diff --git a/.project-docs/30-worklog/tasks/20260817-makelore-module-access-6f2a91c4.md b/.project-docs/30-worklog/tasks/20260817-makelore-module-access-6f2a91c4.md index fe2c40f..8746f52 100644 --- a/.project-docs/30-worklog/tasks/20260817-makelore-module-access-6f2a91c4.md +++ b/.project-docs/30-worklog/tasks/20260817-makelore-module-access-6f2a91c4.md @@ -8,7 +8,7 @@ - Worktree: D:\Datas\OthersProjects\makelore-module-access-6f2a91c4 - Base commit: f7171a471ab1a39380ef666b1e1be9a1f689e43f - Owner: codex -- Status: Ready For Integration +- Status: In Progress ## Scope @@ -32,17 +32,20 @@ - Implemented a shared four-field policy normalizer, Main-owned `/api/auth/me` projection, startup/login/refresh policy hydration, chooser disable state, and direct-route guard. - Bumped the persisted authentication state to schema version 2 so existing installations normalize the new policy field during upgrade; a new login falls back to all-enabled rather than inheriting another account's cached policy. - Disabled Programming routes no longer initialize providers, and all four disabled module families redirect before `MainLayout` or module workspaces mount. +- Corrected the first-review lifecycle gaps: Provider initialization now waits for authenticated policy hydration, a terminal current-user 401 clears both Main and Renderer session state, and global `/settings` remains available when Code is disabled. ## Verification - Red: `module-navigation.test.tsx` failed at `toBeDisabled()` while the static module definition remained enabled. -- Green: focused Vitest (`module-navigation`, `auth-store`, `auth-routes`, `app-module-provider-gate`) — 58 passed. -- Full Vitest — 175 files, 2036 tests passed. The first sandboxed run had one environmental `EPERM` because the test could not create worktree `.tmp`; the unrestricted rerun of the same suite passed completely. +- Green after first review corrections: focused Vitest (`module-navigation`, `auth-store`, `auth-routes`, `app-module-provider-gate`) — 64 passed. +- Full Vitest — 175 files, 2042 tests passed. An earlier sandboxed run before the review corrections had one environmental `EPERM` because the test could not create worktree `.tmp`; every unrestricted full-suite rerun passed completely. - TypeScript `tsc --noEmit` — passed. - Scoped ESLint for all changed TypeScript/TSX files — passed. - Vite production build — passed (Renderer, Electron Main, and preload); existing chunk-size/dynamic-import warnings remain unchanged. - `git diff --check` — passed. - Electron E2E was not extended because the shared fixture deliberately bypasses authentication and cannot express a Main-owned Works `/api/auth/me` policy; the user-visible chooser and direct-route behavior are covered at rendered App/Router seams. +- First independent Sol review — FAIL: found a cold-start Programming provider race, terminal `/api/auth/me` 401 fallback, and global `/settings` misclassification. The task returned to In Progress for corrections and re-review. +- Regression-first correction: the cold-start test failed before the Provider gate fix (`initProviders` called once while auth was unresolved), then passed after the fix. Added startup/login/refresh 401 lifecycle, Main-session clear, and Code-disabled global-settings coverage. ## Follow-ups diff --git a/electron/api/routes/auth.ts b/electron/api/routes/auth.ts index 811a4b4..404631e 100644 --- a/electron/api/routes/auth.ts +++ b/electron/api/routes/auth.ts @@ -466,7 +466,7 @@ async function handleSessionActivity( }); } -async function handleCurrentUser(res: ServerResponse): Promise { +async function handleCurrentUser(res: ServerResponse, ctx: HostApiContext): Promise { const accessToken = await getValidWorksSquareAccessToken({ forceRefresh: false }); if (!accessToken) { sendJson(res, 401, { success: false, error: '登录已过期，请重新授权。' }); @@ -481,6 +481,13 @@ async function handleCurrentUser(res: ServerResponse): Promise { }); const payload = await readResponsePayload(response); if (!response.ok) { + if (response.status === 401) { + clearWorksSquareSession(); + await Promise.allSettled([ + flushWorksSquareSessionPersistence(), + clearManagedWorksSquareRuntimeBestEffort(ctx, 'terminal current-user lookup'), + ]); + } sendJson(res, response.status === 401 ? 401 : 502, { success: false, error: response.status === 401 @@ -649,7 +656,7 @@ export async function handleAuthRoutes( } if (url.pathname === '/api/auth/me' && req.method === 'GET') { - await handleCurrentUser(res); + await handleCurrentUser(res, ctx); return true; } diff --git a/src/App.tsx b/src/App.tsx index 26e6875..5dba06e 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -35,6 +35,7 @@ import { AI_MODULE_SELECTION_PATH, getGuardedAiModuleForPath, isAiModuleAllowed, + isProgrammingProviderRoute, } from './lib/ai-modules'; import { useUserSyncStore } from './stores/user-sync'; import { flushPendingAgentSessionSync } from '@/lib/agent-session-sync'; @@ -122,10 +123,6 @@ function getReturnPath(location: ReturnType): string { return `${location.pathname}${location.search}`; } -function isProgrammingRoute(pathname: string): boolean { - return getGuardedAiModuleForPath(pathname) === 'programming'; -} - function ProtectedLayout({ authReady, authRequired, @@ -321,10 +318,24 @@ function App() { useEffect(() => { if (rendererOnlyPreview) return; if (!setupReady) return; - if (!programmingModuleAllowed) return; - if (!isProgrammingRoute(location.pathname)) return; + if (authRequired && !authInitialized) return; + if (authRequired && !authenticated) return; + if (!isProgrammingProviderRoute(location.pathname)) return; + if ( + getGuardedAiModuleForPath(location.pathname) === 'programming' + && !programmingModuleAllowed + ) return; initProviders(); - }, [initProviders, location.pathname, programmingModuleAllowed, rendererOnlyPreview, setupReady]); + }, [ + authInitialized, + authRequired, + authenticated, + initProviders, + location.pathname, + programmingModuleAllowed, + rendererOnlyPreview, + setupReady, + ]); useEffect(() => { if (rendererOnlyPreview) return; diff --git a/src/lib/ai-modules.ts b/src/lib/ai-modules.ts index 002c3e4..f9e0298 100644 --- a/src/lib/ai-modules.ts +++ b/src/lib/ai-modules.ts @@ -76,6 +76,10 @@ const PROGRAMMING_ROUTE_PREFIXES = [ '/projects', '/sessions', '/models', +] as const; + +const PROGRAMMING_PROVIDER_ROUTE_PREFIXES = [ + ...PROGRAMMING_ROUTE_PREFIXES, '/settings', ] as const; @@ -87,6 +91,10 @@ export function isAiModuleAllowed(moduleId: AiModuleId, access: ModuleAccess): b return access[moduleAccessKeyById[moduleId]]; } +export function isProgrammingProviderRoute(pathname: string): boolean { + return PROGRAMMING_PROVIDER_ROUTE_PREFIXES.some((route) => matchesRoute(pathname, route)); +} + export function getGuardedAiModuleForPath(pathname: string): AiModuleId | null { if (pathname === '/image-canvas' || pathname.startsWith('/image-canvas/') diff --git a/src/stores/auth.ts b/src/stores/auth.ts index d10c953..b95a1eb 100644 --- a/src/stores/auth.ts +++ b/src/stores/auth.ts @@ -236,7 +236,8 @@ async function readCurrentModuleAccess(fallback: ModuleAccess): Promise('/api/auth/me'); if (!response.success) return fallback; return normalizeModuleAccess(response.moduleAccess); - } catch { + } catch (error) { + if (isTerminalAuthError(error)) throw error; return fallback; } } @@ -347,9 +348,25 @@ export const useAuthStore = create()( return; } - const moduleAccess = await readCurrentModuleAccess( - normalizeModuleAccess(state.moduleAccess), - ); + let moduleAccess: ModuleAccess; + try { + moduleAccess = await readCurrentModuleAccess( + normalizeModuleAccess(state.moduleAccess), + ); + } catch (error) { + if (!isCurrentAuthSessionEpoch(operationEpoch)) return; + if (isTerminalAuthError(error)) { + advanceAuthSessionEpoch(); + set({ + initialized: true, + loading: false, + error: '登录已过期，请重新授权。', + ...getClearedSession(), + }); + return; + } + moduleAccess = normalizeModuleAccess(state.moduleAccess); + } if (!isCurrentAuthSessionEpoch(operationEpoch)) return; set({ initialized: true, loading: false, error: null, moduleAccess }); }, @@ -383,7 +400,10 @@ export const useAuthStore = create()( }); } catch (error) { if (!isCurrentAuthSessionEpoch(operationEpoch)) return; - const message = error instanceof Error ? error.message : String(error); + const terminal = isTerminalAuthError(error); + const message = terminal + ? '登录已过期，请重新授权。' + : (error instanceof Error ? error.message : String(error)); advanceAuthSessionEpoch(); set({ loading: false, error: message, ...getClearedSession() }); throw new Error(message, { cause: error }); @@ -431,8 +451,10 @@ export const useAuthStore = create()( return session.accessToken; } catch (error) { if (!isCurrentAuthSessionEpoch(operationEpoch)) return null; - const message = error instanceof Error ? error.message : String(error); const terminal = isTerminalAuthError(error); + const message = terminal + ? '登录已过期，请重新授权。' + : (error instanceof Error ? error.message : String(error)); if (terminal) advanceAuthSessionEpoch(); set({ initialized: true, diff --git a/tests/unit/app-module-provider-gate.test.tsx b/tests/unit/app-module-provider-gate.test.tsx index 64be61e..33e0831 100644 --- a/tests/unit/app-module-provider-gate.test.tsx +++ b/tests/unit/app-module-provider-gate.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, waitFor } from '@testing-library/react'; +import { act, render, screen, waitFor } from '@testing-library/react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { MemoryRouter, Outlet } from 'react-router-dom'; import App from '@/App'; @@ -27,6 +27,10 @@ vi.mock('@/pages/ModuleSelection', () => ({ ModuleSelection: () =>
Module chooser
, })); +vi.mock('@/pages/Settings', () => ({ + Settings: () =>
Global settings
, +})); + describe('App programming provider initialization gate', () => { const initProviders = vi.fn(); @@ -84,6 +88,37 @@ describe('App programming provider initialization gate', () => { await waitFor(() => expect(initProviders).toHaveBeenCalledTimes(1)); }); + it('waits for the startup module policy before initializing programming providers', async () => { + useAuthStore.setState({ + initialized: false, + moduleAccess: { + programming: true, + design: true, + learning: true, + robot: true, + }, + }); + + await renderAt('/opencode-chat'); + + expect(initProviders).not.toHaveBeenCalled(); + + act(() => { + useAuthStore.setState({ + initialized: true, + moduleAccess: { + programming: false, + design: true, + learning: true, + robot: true, + }, + }); + }); + + expect(await screen.findByText('Module chooser')).toBeInTheDocument(); + expect(initProviders).not.toHaveBeenCalled(); + }); + it.each([ ['/opencode-chat', 'programming'], ['/image-canvas', 'design'], @@ -107,4 +142,20 @@ describe('App programming provider initialization gate', () => { expect(initProviders).not.toHaveBeenCalled(); } }); + + it('keeps global settings available when Programming is disabled', async () => { + useAuthStore.setState({ + moduleAccess: { + programming: false, + design: true, + learning: true, + robot: true, + }, + }); + + await renderAt('/settings'); + + expect(await screen.findByText('Global settings')).toBeInTheDocument(); + expect(screen.queryByText('Module chooser')).not.toBeInTheDocument(); + }); }); diff --git a/tests/unit/auth-routes.test.ts b/tests/unit/auth-routes.test.ts index ec851f0..448459a 100644 --- a/tests/unit/auth-routes.test.ts +++ b/tests/unit/auth-routes.test.ts @@ -145,6 +145,34 @@ describe('auth host api routes', () => { }); }); + it('clears the Main session when the current-user lookup is unauthorized', async () => { + storeWorksSquareSession({ + accessToken: 'expired-access-token', + refreshToken: 'expired-refresh-token', + expiresAt: Date.now() + 60_000, + lastActiveAt: Date.now(), + }); + vi.stubGlobal('fetch', vi.fn().mockResolvedValueOnce( + new Response(JSON.stringify({ detail: 'upstream secret' }), { status: 401 }), + )); + const response = createResponse(); + + await handleAuthRoutes( + createRequest('GET'), + response.res, + new URL('http://127.0.0.1:13210/api/auth/me'), + {} as never, + ); + + expect(response.statusCode).toBe(401); + expect(response.json()).toEqual({ + success: false, + error: '登录已过期，请重新授权。', + }); + expect(JSON.stringify(response.json())).not.toContain('upstream secret'); + expect(getWorksSquareSessionSnapshot()).toBeNull(); + }); + it('exchanges username and AES-encrypted password through the app SSO token endpoint', async () => { const fetchMock = vi.fn().mockResolvedValueOnce( new Response(JSON.stringify({ diff --git a/tests/unit/auth-store.test.ts b/tests/unit/auth-store.test.ts index 13f9c87..8c14ddd 100644 --- a/tests/unit/auth-store.test.ts +++ b/tests/unit/auth-store.test.ts @@ -150,6 +150,100 @@ describe('auth store', () => { }); }); + it('clears restored auth when the current-user policy lookup is unauthorized', async () => { + hostApiFetchMock + .mockResolvedValueOnce({ + success: true, + session: { + accessToken: 'persisted-access-token', + tokenType: 'Bearer', + expiresAt: Date.now() + 60_000, + lastActiveAt: Date.now(), + canRefresh: true, + }, + }) + .mockRejectedValueOnce(Object.assign(new Error('Unauthorized'), { + details: { status: 401 }, + })); + useAuthStore.setState({ + authBase: 'https://biz.nianxx.cn/auth/', + accessToken: 'persisted-access-token', + tokenType: 'Bearer', + expiresAt: Date.now() + 60_000, + lastActiveAt: Date.now(), + canRefresh: true, + moduleAccess: { + programming: true, + design: false, + learning: true, + robot: true, + }, + user: { + username: 'zhangsan', + userId: '1', + tenantId: null, + deptId: null, + authorities: [], + }, + }); + + await useAuthStore.getState().init(); + + expect(useAuthStore.getState()).toMatchObject({ + initialized: true, + loading: false, + error: '登录已过期，请重新授权。', + accessToken: null, + user: null, + moduleAccess: { + programming: true, + design: true, + learning: true, + robot: true, + }, + }); + }); + + it('rejects a new login when its current-user policy lookup is unauthorized', async () => { + hostApiFetchMock + .mockResolvedValueOnce({ + success: true, + token: { + access_token: 'access-token', + token_type: 'Bearer', + username: 'zhangsan', + user_id: '1', + }, + session: { + accessToken: 'access-token', + tokenType: 'Bearer', + expiresAt: Date.now() + 60_000, + lastActiveAt: Date.now(), + canRefresh: true, + }, + }) + .mockRejectedValueOnce(Object.assign(new Error('Unauthorized'), { + details: { status: 401 }, + })); + + await expect(useAuthStore.getState().loginWithBrowser()).rejects.toThrow( + '登录已过期，请重新授权。', + ); + + expect(useAuthStore.getState()).toMatchObject({ + initialized: false, + loading: false, + accessToken: null, + user: null, + moduleAccess: { + programming: true, + design: true, + learning: true, + robot: true, + }, + }); + }); + it('surfaces browser authorization failures and does not keep a partial session', async () => { hostApiFetchMock.mockResolvedValueOnce({ success: false, @@ -541,6 +635,52 @@ describe('auth store', () => { expect(useAuthStore.getState().user?.username).toBe('zhangsan'); }); + it('clears auth when a refreshed session cannot read the current user', async () => { + hostApiFetchMock + .mockResolvedValueOnce({ + success: true, + session: { + accessToken: 'new-access-token', + tokenType: 'Bearer', + expiresAt: Date.now() + 120_000, + lastActiveAt: Date.now(), + canRefresh: true, + }, + }) + .mockRejectedValueOnce(Object.assign(new Error('Unauthorized'), { + details: { status: 401 }, + })); + useAuthStore.setState({ + initialized: true, + accessToken: 'old-access-token', + tokenType: 'Bearer', + expiresAt: Date.now() + 1_000, + lastActiveAt: Date.now(), + canRefresh: true, + user: { + username: 'zhangsan', + userId: '1', + tenantId: null, + deptId: null, + authorities: [], + }, + }); + + await expect(useAuthStore.getState().refreshSession()).resolves.toBeNull(); + + expect(useAuthStore.getState()).toMatchObject({ + error: '登录已过期，请重新授权。', + accessToken: null, + user: null, + moduleAccess: { + programming: true, + design: true, + learning: true, + robot: true, + }, + }); + }); + it('restores an expired access token when Main reports it can refresh', async () => { const lastActiveAt = Date.now() - 24 * 60 * 60 * 1000; hostApiFetchMock