From efb5f79215a8f4f8dc8cad61369cc2f95eefd793 Mon Sep 17 00:00:00 2001 From: Evan Jacobson Date: Mon, 31 Aug 2026 12:20:49 -0600 Subject: [PATCH] fix(app-builder): repair migrations and stale sessions --- .../components/app-builder/AppBuilderChat.tsx | 17 +- .../components/app-builder/ProjectManager.ts | 44 ++- .../__tests__/ProjectManager.test.ts | 262 ++++++++++++++++++ .../__tests__/preview-polling.test.ts | 1 + .../project-manager/__tests__/store.test.ts | 1 + .../__tests__/v2-streaming.test.ts | 114 ++++++++ .../project-manager/sessions/v2/streaming.ts | 1 - .../app-builder/project-manager/store.ts | 1 + .../app-builder/project-manager/types.ts | 2 + .../app-builder/app-builder-client.test.ts | 78 ++++++ .../src/lib/app-builder/app-builder-client.ts | 17 +- .../app-builder/app-builder-service.test.ts | 29 ++ .../lib/app-builder/app-builder-service.ts | 15 +- .../github-migration-service.test.ts | 102 +++++++ .../app-builder/github-migration-service.ts | 7 +- services/app-builder/src/api-schemas.test.ts | 32 +++ services/app-builder/src/api-schemas.ts | 2 +- 17 files changed, 701 insertions(+), 24 deletions(-) create mode 100644 apps/web/src/components/app-builder/project-manager/__tests__/ProjectManager.test.ts create mode 100644 apps/web/src/components/app-builder/project-manager/__tests__/v2-streaming.test.ts create mode 100644 apps/web/src/lib/app-builder/app-builder-client.test.ts create mode 100644 apps/web/src/lib/app-builder/app-builder-service.test.ts create mode 100644 apps/web/src/lib/app-builder/github-migration-service.test.ts create mode 100644 services/app-builder/src/api-schemas.test.ts diff --git a/apps/web/src/components/app-builder/AppBuilderChat.tsx b/apps/web/src/components/app-builder/AppBuilderChat.tsx index d77ba5a8be..29e06aae11 100644 --- a/apps/web/src/components/app-builder/AppBuilderChat.tsx +++ b/apps/web/src/components/app-builder/AppBuilderChat.tsx @@ -508,7 +508,14 @@ function SessionMessages({ export function AppBuilderChat({ organizationId }: AppBuilderChatProps) { // Get state and manager from ProjectSession context const { manager, state } = useProject(); - const { isStreaming, isInterrupting, model: projectModel, sessions, pendingNewSession } = state; + const { + isStreaming, + isInterrupting, + model: projectModel, + sessions, + pendingNewSession, + isRecoveringSession, + } = state; const messagesEndRef = useRef(null); const scrollContainerRef = useRef(null); @@ -743,7 +750,7 @@ export function AppBuilderChat({ organizationId }: AppBuilderChatProps) { variant="ghost" size="icon" onClick={handleNewChatToggle} - disabled={isStreaming} + disabled={isStreaming || isRecoveringSession} className={pendingNewSession ? 'text-primary bg-primary/10 h-8 w-8' : 'h-8 w-8'} aria-label="New chat" > @@ -751,7 +758,11 @@ export function AppBuilderChat({ organizationId }: AppBuilderChatProps) { - {pendingNewSession ? 'Cancel new chat' : 'New chat'} + {isRecoveringSession + ? 'A new chat is required' + : pendingNewSession + ? 'Cancel new chat' + : 'New chat'} diff --git a/apps/web/src/components/app-builder/ProjectManager.ts b/apps/web/src/components/app-builder/ProjectManager.ts index 1b2bb96284..ea00f5a97b 100644 --- a/apps/web/src/components/app-builder/ProjectManager.ts +++ b/apps/web/src/components/app-builder/ProjectManager.ts @@ -101,8 +101,10 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag } function getActiveSession(): AppBuilderSession | undefined { - const sessions = store.getState().sessions; - return sessions[sessions.length - 1]; + if (!cloudAgentSessionId) return undefined; + return store + .getState() + .sessions.find(session => session.info.cloud_agent_session_id === cloudAgentSessionId); } function subscribeToSession(session: AppBuilderSession): void { @@ -123,12 +125,16 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag const sessionInfos = proj.sessions; if (sessionInfos.length === 0) return []; - const activeInfo = - sessionInfos.find(s => s.ended_at === null) ?? sessionInfos[sessionInfos.length - 1]; + const activeInfo = project.session_id + ? sessionInfos.find(s => s.cloud_agent_session_id === project.session_id) + : undefined; + const orderedSessionInfos = activeInfo + ? [...sessionInfos.filter(info => info.id !== activeInfo.id), activeInfo] + : sessionInfos; const sessions: AppBuilderSession[] = []; - for (const info of sessionInfos) { + for (const info of orderedSessionInfos) { const isActive = info.id === activeInfo?.id; if (!isActive) { @@ -199,6 +205,7 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag store.setState({ sessions: [...currentSessions, newSession], isStreaming: true, + isRecoveringSession: false, }); cloudAgentSessionId = newSessionId; @@ -259,15 +266,22 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag // Determine if the active session needs initial streaming from the backend session info. // `initiated` lives on ProjectSessionInfo (routing data), not on SessionDisplayInfo. - const activeProjectSessionInfo = - project.sessions.find(s => s.ended_at === null) ?? - project.sessions[project.sessions.length - 1]; + const activeProjectSessionInfo = project.session_id + ? project.sessions.find(s => s.cloud_agent_session_id === project.session_id) + : undefined; - if (activeProjectSessionInfo?.initiated === false) { + if (activeProjectSessionInfo?.prepared === true && activeProjectSessionInfo.initiated === false) { pendingInitialStreamingStart = true; - } else if (cloudAgentSessionId) { + } else if ( + cloudAgentSessionId && + activeProjectSessionInfo && + activeProjectSessionInfo.prepared !== false + ) { pendingReconnect = true; } else { + if (cloudAgentSessionId) { + store.setState({ pendingNewSession: true, isRecoveringSession: true }); + } startPreviewPollingIfNeeded(); } @@ -339,6 +353,7 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag } const effectiveModel = model ?? store.getState().model; + const isRecoveringSession = store.getState().isRecoveringSession; store.setState({ pendingNewSession: false, isStreaming: true }); @@ -370,7 +385,11 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag .catch((err: Error) => { if (destroyed) return; logger.logError('Failed to start new session', err); - store.setState({ isStreaming: false }); + store.setState({ + pendingNewSession: true, + isRecoveringSession, + isStreaming: false, + }); }); } @@ -423,11 +442,12 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag if (currentActive) { currentActive.info.ended_at = new Date().toISOString(); } - store.setState({ pendingNewSession: true }); + store.setState({ pendingNewSession: true, isRecoveringSession: false }); } function cancelNewSession(): void { if (destroyed) return; + if (store.getState().isRecoveringSession) return; const currentActive = getActiveSession(); if (currentActive) { currentActive.info.ended_at = null; diff --git a/apps/web/src/components/app-builder/project-manager/__tests__/ProjectManager.test.ts b/apps/web/src/components/app-builder/project-manager/__tests__/ProjectManager.test.ts new file mode 100644 index 0000000000..99e532f727 --- /dev/null +++ b/apps/web/src/components/app-builder/project-manager/__tests__/ProjectManager.test.ts @@ -0,0 +1,262 @@ +import type { ProjectWithMessages, SessionDisplayInfo } from '@/lib/app-builder/types'; +import type { AppBuilderSession } from '../types'; + +const mockStartPreviewPolling = jest.fn((_config?: unknown) => ({ + isPolling: true, + stop: jest.fn(), +})); +const mockSessions = new Map(); + +function makeSession(info: SessionDisplayInfo): AppBuilderSession { + const session = { + type: 'v2' as const, + info, + getState: jest.fn(() => ({ + messages: [], + isStreaming: false, + questionRequestIds: new Map(), + childSessionMessages: new Map(), + })), + subscribe: jest.fn(() => () => {}), + getChildSessionMessages: jest.fn(() => []), + sendMessage: jest.fn(async () => {}), + interrupt: jest.fn(async () => {}), + startInitialStreaming: jest.fn(), + connectToExistingSession: jest.fn(), + loadMessages: jest.fn(), + destroy: jest.fn(), + } satisfies AppBuilderSession; + mockSessions.set(info.cloud_agent_session_id ?? info.id, session); + return session; +} + +jest.mock('../preview-polling', () => ({ + startPreviewPolling: (config: unknown) => mockStartPreviewPolling(config), +})); +jest.mock('../deployments', () => ({ deploy: jest.fn() })); +jest.mock('../sessions/v2/v2-session', () => ({ + createV2Session: (config: { info: SessionDisplayInfo }) => makeSession(config.info), +})); +jest.mock('../sessions/v1/v1-session', () => ({ + createV1Session: (config: { info: SessionDisplayInfo }) => makeSession(config.info), +})); + +import { createProjectManager } from '../../ProjectManager'; + +function makeProject( + sessions: ProjectWithMessages['sessions'], + sessionId: string | null = 'canonical-session' +): ProjectWithMessages { + return { + id: 'project-1', + session_id: sessionId, + deployment_id: null, + model_id: 'test-model', + git_repo_full_name: null, + messages: [], + sessions, + } as unknown as ProjectWithMessages; +} + +function makeSessionInfo( + id: string, + overrides: Partial = {} +): ProjectWithMessages['sessions'][number] { + return { + id, + cloud_agent_session_id: id, + worker_version: 'v2', + ended_at: null, + title: null, + initiated: true, + prepared: true, + ...overrides, + }; +} + +function makeTrpcClient() { + return { + appBuilder: { + sendMessage: { + mutate: jest.fn(async () => ({ + cloudAgentSessionId: 'replacement-session', + workerVersion: 'v2' as const, + })), + }, + }, + }; +} + +async function flushMicrotasks(): Promise { + await Promise.resolve(); + await Promise.resolve(); +} + +describe('createProjectManager session recovery', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockSessions.clear(); + }); + + it('reconnects the canonical session instead of a newer orphan row', async () => { + const trpcClient = makeTrpcClient(); + const manager = createProjectManager({ + project: makeProject([ + makeSessionInfo('canonical-session', { ended_at: '2026-08-31T00:00:00.000Z' }), + makeSessionInfo('orphan-session'), + ]), + trpcClient: trpcClient as never, + organizationId: null, + }); + + manager.subscribe(() => {}); + await flushMicrotasks(); + + expect(mockSessions.get('canonical-session')?.connectToExistingSession).toHaveBeenCalledWith( + 'canonical-session' + ); + expect(mockSessions.get('orphan-session')?.connectToExistingSession).not.toHaveBeenCalled(); + expect(manager.getState().sessions.at(-1)?.info.cloud_agent_session_id).toBe( + 'canonical-session' + ); + manager.destroy(); + }); + + it.each([ + ['unprepared', [makeSessionInfo('canonical-session', { initiated: null, prepared: false })]], + ['orphaned', [makeSessionInfo('orphan-session')]], + ])('recovers an %s canonical session through forceNewSession', async (_label, sessions) => { + const trpcClient = makeTrpcClient(); + const manager = createProjectManager({ + project: makeProject(sessions), + trpcClient: trpcClient as never, + organizationId: null, + }); + + expect(manager.getState().pendingNewSession).toBe(true); + expect(manager.getState().isRecoveringSession).toBe(true); + expect(mockStartPreviewPolling).toHaveBeenCalled(); + + manager.subscribe(() => {}); + await flushMicrotasks(); + for (const session of mockSessions.values()) { + expect(session.connectToExistingSession).not.toHaveBeenCalled(); + expect(session.startInitialStreaming).not.toHaveBeenCalled(); + } + + manager.sendMessage('Recover this project'); + await flushMicrotasks(); + + expect(trpcClient.appBuilder.sendMessage.mutate).toHaveBeenCalledWith({ + projectId: 'project-1', + message: 'Recover this project', + images: undefined, + model: 'test-model', + forceNewSession: true, + }); + expect(mockSessions.get('replacement-session')?.connectToExistingSession).toHaveBeenCalledWith( + 'replacement-session' + ); + manager.destroy(); + }); + + it('starts a prepared, uninitiated canonical session', async () => { + const manager = createProjectManager({ + project: makeProject([ + makeSessionInfo('canonical-session', { initiated: false, prepared: true }), + ]), + trpcClient: makeTrpcClient() as never, + organizationId: null, + }); + + manager.subscribe(() => {}); + await flushMicrotasks(); + + expect(mockSessions.get('canonical-session')?.startInitialStreaming).toHaveBeenCalled(); + expect(mockSessions.get('canonical-session')?.connectToExistingSession).not.toHaveBeenCalled(); + manager.destroy(); + }); + + it('fails open and reconnects when canonical session state is unknown', async () => { + const manager = createProjectManager({ + project: makeProject([ + makeSessionInfo('canonical-session', { initiated: null, prepared: null }), + ]), + trpcClient: makeTrpcClient() as never, + organizationId: null, + }); + + manager.subscribe(() => {}); + await flushMicrotasks(); + + expect(mockSessions.get('canonical-session')?.connectToExistingSession).toHaveBeenCalledWith( + 'canonical-session' + ); + expect(manager.getState().pendingNewSession).toBe(false); + manager.destroy(); + }); + + it('keeps stale-session recovery retryable when replacement creation fails', async () => { + const trpcClient = makeTrpcClient(); + trpcClient.appBuilder.sendMessage.mutate.mockRejectedValueOnce(new Error('Unavailable')); + const manager = createProjectManager({ + project: makeProject([ + makeSessionInfo('canonical-session', { initiated: null, prepared: false }), + ]), + trpcClient: trpcClient as never, + organizationId: null, + }); + + manager.sendMessage('Recover this project'); + await flushMicrotasks(); + + expect(manager.getState()).toMatchObject({ + pendingNewSession: true, + isRecoveringSession: true, + isStreaming: false, + }); + manager.destroy(); + }); + + it('does not allow mandatory stale-session recovery to be cancelled', () => { + const manager = createProjectManager({ + project: makeProject([ + makeSessionInfo('canonical-session', { initiated: null, prepared: false }), + ]), + trpcClient: makeTrpcClient() as never, + organizationId: null, + }); + + manager.cancelNewSession(); + + expect(manager.getState()).toMatchObject({ + pendingNewSession: true, + isRecoveringSession: true, + }); + manager.destroy(); + }); + + it('keeps an ordinary new chat cancellable when session creation fails', async () => { + const trpcClient = makeTrpcClient(); + trpcClient.appBuilder.sendMessage.mutate.mockRejectedValueOnce(new Error('Unavailable')); + const manager = createProjectManager({ + project: makeProject([makeSessionInfo('canonical-session')]), + trpcClient: trpcClient as never, + organizationId: null, + }); + + manager.requestNewSession(); + manager.sendMessage('Start another chat'); + await flushMicrotasks(); + + expect(manager.getState()).toMatchObject({ + pendingNewSession: true, + isRecoveringSession: false, + isStreaming: false, + }); + + manager.cancelNewSession(); + expect(manager.getState().pendingNewSession).toBe(false); + manager.destroy(); + }); +}); diff --git a/apps/web/src/components/app-builder/project-manager/__tests__/preview-polling.test.ts b/apps/web/src/components/app-builder/project-manager/__tests__/preview-polling.test.ts index c13c7cebb7..43b7981dbb 100644 --- a/apps/web/src/components/app-builder/project-manager/__tests__/preview-polling.test.ts +++ b/apps/web/src/components/app-builder/project-manager/__tests__/preview-polling.test.ts @@ -21,6 +21,7 @@ function createMockStore(): ProjectStore & { stateUpdates: Array { gitRepoFullName: null, sessions: [], pendingNewSession: false, + isRecoveringSession: false, }; describe('getState', () => { diff --git a/apps/web/src/components/app-builder/project-manager/__tests__/v2-streaming.test.ts b/apps/web/src/components/app-builder/project-manager/__tests__/v2-streaming.test.ts new file mode 100644 index 0000000000..8b454231fa --- /dev/null +++ b/apps/web/src/components/app-builder/project-manager/__tests__/v2-streaming.test.ts @@ -0,0 +1,114 @@ +import type { V2SessionState } from '../sessions/types'; +import type { V2StreamingConfig } from '../sessions/v2/streaming'; + +const mockConnect = jest.fn(); + +jest.mock('@/lib/cloud-agent-next/websocket-manager', () => ({ + createWebSocketManager: jest.fn(() => ({ + connect: mockConnect, + disconnect: jest.fn(), + })), +})); +jest.mock('@/lib/cloud-agent-next/processor', () => ({ + createEventProcessor: jest.fn(() => ({ + processEvent: jest.fn(), + forceCompleteAll: jest.fn(), + clear: jest.fn(), + })), +})); +jest.mock('@/lib/constants', () => ({ + CLOUD_AGENT_NEXT_WS_URL: 'https://cloud-agent.example.com', +})); + +import { createV2StreamingCoordinator } from '../sessions/v2/streaming'; + +function makeStore() { + let state: V2SessionState = { + messages: [], + isStreaming: false, + questionRequestIds: new Map(), + childSessionMessages: new Map(), + }; + const updates: Array> = []; + + return { + updates, + getState: () => state, + setState: jest.fn((partial: Partial) => { + updates.push(partial); + state = { ...state, ...partial }; + }), + subscribe: jest.fn(() => () => {}), + updateMessages: jest.fn(), + setQuestionRequestId: jest.fn(), + updateChildSessionMessages: jest.fn(), + getChildSessionMessages: jest.fn(() => []), + }; +} + +function makeTrpcClient() { + return { + appBuilder: { + startSession: { + mutate: jest.fn(async () => ({ cloudAgentSessionId: 'canonical-session' })), + }, + sendMessage: { + mutate: jest.fn(async () => ({ + cloudAgentSessionId: 'canonical-session', + workerVersion: 'v2' as const, + })), + }, + }, + }; +} + +describe('V2 reconnect streaming state', () => { + beforeEach(() => { + jest.clearAllMocks(); + jest.spyOn(global, 'fetch').mockImplementation(async () => + Promise.resolve( + new Response(JSON.stringify({ ticket: 'stream-ticket' }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }) + ) + ); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + function makeCoordinator() { + const store = makeStore(); + const coordinator = createV2StreamingCoordinator({ + projectId: 'project-1', + organizationId: null, + trpcClient: makeTrpcClient() as unknown as V2StreamingConfig['trpcClient'], + store, + cloudAgentSessionId: 'canonical-session', + }); + return { coordinator, store }; + } + + it('does not optimistically mark a reconnect as streaming', async () => { + const { coordinator, store } = makeCoordinator(); + + coordinator.connectToExistingSession('canonical-session'); + await new Promise(resolve => setImmediate(resolve)); + + expect(store.updates).not.toContainEqual({ isStreaming: true }); + expect(mockConnect).toHaveBeenCalled(); + }); + + it('keeps optimistic streaming state for sends and starts', () => { + const send = makeCoordinator(); + const start = makeCoordinator(); + + send.coordinator.sendMessage('Build it'); + start.coordinator.startInitialStreaming(); + + expect(send.store.updates).toContainEqual({ isStreaming: true }); + expect(start.store.updates).toContainEqual({ isStreaming: true }); + }); +}); diff --git a/apps/web/src/components/app-builder/project-manager/sessions/v2/streaming.ts b/apps/web/src/components/app-builder/project-manager/sessions/v2/streaming.ts index 7785b2fe89..94a7d87f70 100644 --- a/apps/web/src/components/app-builder/project-manager/sessions/v2/streaming.ts +++ b/apps/web/src/components/app-builder/project-manager/sessions/v2/streaming.ts @@ -469,7 +469,6 @@ export function createV2StreamingCoordinator(config: V2StreamingConfig): V2Strea } logger.log('Connecting to existing V2 session', { sessionId }); - store.setState({ isStreaming: true }); void (async () => { try { diff --git a/apps/web/src/components/app-builder/project-manager/store.ts b/apps/web/src/components/app-builder/project-manager/store.ts index c41cef4393..be880bfb77 100644 --- a/apps/web/src/components/app-builder/project-manager/store.ts +++ b/apps/web/src/components/app-builder/project-manager/store.ts @@ -32,6 +32,7 @@ export function createInitialState( gitRepoFullName, sessions: [], pendingNewSession: false, + isRecoveringSession: false, }; } diff --git a/apps/web/src/components/app-builder/project-manager/types.ts b/apps/web/src/components/app-builder/project-manager/types.ts index 870bb02d2a..642e8c1b7d 100644 --- a/apps/web/src/components/app-builder/project-manager/types.ts +++ b/apps/web/src/components/app-builder/project-manager/types.ts @@ -29,6 +29,8 @@ export type ProjectState = { sessions: AppBuilderSession[]; /** True while the user has clicked "New Chat" but hasn't sent the first message yet */ pendingNewSession: boolean; + /** True when a missing backend session requires the next message to create a replacement */ + isRecoveringSession: boolean; }; export type StateListener = () => void; diff --git a/apps/web/src/lib/app-builder/app-builder-client.test.ts b/apps/web/src/lib/app-builder/app-builder-client.test.ts new file mode 100644 index 0000000000..329f6c9beb --- /dev/null +++ b/apps/web/src/lib/app-builder/app-builder-client.test.ts @@ -0,0 +1,78 @@ +jest.mock('@/lib/config.server', () => ({ + APP_BUILDER_URL: 'https://app-builder.example.com', + APP_BUILDER_AUTH_TOKEN: 'test-token', +})); + +import { AppBuilderError, migrateToGithub } from './app-builder-client'; + +describe('migrateToGithub', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('returns a typed failure envelope from a non-2xx response', async () => { + jest + .spyOn(global, 'fetch') + .mockResolvedValue( + new Response( + JSON.stringify({ success: false, error: 'push_failed', message: 'sensitive detail' }), + { status: 502 } + ) + ); + + await expect( + migrateToGithub('project-id', { + githubRepo: 'kilocode/example', + userId: 'user_2abc123', + }) + ).resolves.toEqual({ + success: false, + error: 'push_failed', + message: 'sensitive detail', + }); + }); + + it('rejects a malformed non-2xx response', async () => { + jest + .spyOn(global, 'fetch') + .mockResolvedValue(new Response(JSON.stringify({ error: 'push_failed' }), { status: 502 })); + + const result = migrateToGithub('project-id', { + githubRepo: 'kilocode/example', + userId: 'user_2abc123', + }); + + await expect(result).rejects.toMatchObject({ statusCode: 502 }); + await expect(result).rejects.toBeInstanceOf(AppBuilderError); + }); + + it('rejects a non-JSON non-2xx response without exposing its body', async () => { + jest + .spyOn(global, 'fetch') + .mockResolvedValue(new Response('sensitive detail', { status: 502 })); + + const result = migrateToGithub('project-id', { + githubRepo: 'kilocode/example', + userId: 'user_2abc123', + }); + + await expect(result).rejects.toMatchObject({ statusCode: 502 }); + await expect(result).rejects.not.toThrow('sensitive detail'); + }); + + it('rejects a success envelope returned with a non-2xx status without exposing its body', async () => { + jest.spyOn(global, 'fetch').mockResolvedValue( + new Response(JSON.stringify({ success: true, message: 'sensitive detail' }), { + status: 500, + }) + ); + + const result = migrateToGithub('project-id', { + githubRepo: 'kilocode/example', + userId: 'user_2abc123', + }); + + await expect(result).rejects.toBeInstanceOf(AppBuilderError); + await expect(result).rejects.not.toThrow('sensitive detail'); + }); +}); diff --git a/apps/web/src/lib/app-builder/app-builder-client.ts b/apps/web/src/lib/app-builder/app-builder-client.ts index 7042393af3..56282a0e07 100644 --- a/apps/web/src/lib/app-builder/app-builder-client.ts +++ b/apps/web/src/lib/app-builder/app-builder-client.ts @@ -294,15 +294,24 @@ export async function migrateToGithub( body: JSON.stringify(config), }); + const data = await response.json().catch(() => undefined); + const parsed = MigrateToGithubResponseSchema.safeParse(data); + + if (parsed.success && !parsed.data.success) { + return parsed.data; + } + if (!response.ok) { - const errorText = await response.text().catch(() => 'Unknown error'); throw new AppBuilderError( - `Failed to migrate project ${projectId} to GitHub: ${response.status} ${response.statusText} - ${errorText}`, + `Failed to migrate project ${projectId} to GitHub: ${response.status} ${response.statusText}`, response.status, endpoint ); } - const data = await response.json(); - return MigrateToGithubResponseSchema.parse(data); + if (!parsed.success) { + throw parsed.error; + } + + return parsed.data; } diff --git a/apps/web/src/lib/app-builder/app-builder-service.test.ts b/apps/web/src/lib/app-builder/app-builder-service.test.ts new file mode 100644 index 0000000000..705eb6be55 --- /dev/null +++ b/apps/web/src/lib/app-builder/app-builder-service.test.ts @@ -0,0 +1,29 @@ +import { TRPCClientError } from '@trpc/client'; +import { isDefinitiveSessionNotFoundError } from './app-builder-service'; + +describe('isDefinitiveSessionNotFoundError', () => { + it.each([ + new TRPCClientError('Missing', { + result: { + error: { + code: -32004, + message: 'Missing', + data: { code: 'NOT_FOUND', httpStatus: 404 }, + }, + }, + }), + { code: 'NOT_FOUND' }, + { data: { httpStatus: 404 } }, + { shape: { data: { code: 'NOT_FOUND' } } }, + ])('classifies definitive tRPC not-found errors', error => { + expect(isDefinitiveSessionNotFoundError(error)).toBe(true); + }); + + it.each([ + new Error('Not Found'), + { data: { code: 'INTERNAL_SERVER_ERROR', httpStatus: 500 } }, + { data: { code: 'TIMEOUT' } }, + ])('leaves transient or unstructured failures unknown', error => { + expect(isDefinitiveSessionNotFoundError(error)).toBe(false); + }); +}); diff --git a/apps/web/src/lib/app-builder/app-builder-service.ts b/apps/web/src/lib/app-builder/app-builder-service.ts index 779d4d2d56..c6d2bc4694 100644 --- a/apps/web/src/lib/app-builder/app-builder-service.ts +++ b/apps/web/src/lib/app-builder/app-builder-service.ts @@ -72,6 +72,19 @@ export type { const REQUIRED_WORKER_VERSION = 'v2' satisfies WorkerVersion; +export function isDefinitiveSessionNotFoundError(error: unknown): boolean { + if (typeof error !== 'object' || error === null) return false; + + const trpcError = error as { + code?: unknown; + data?: { code?: unknown; httpStatus?: unknown }; + shape?: { data?: { code?: unknown; httpStatus?: unknown } }; + }; + const data = trpcError.data ?? trpcError.shape?.data; + + return trpcError.code === 'NOT_FOUND' || data?.code === 'NOT_FOUND' || data?.httpStatus === 404; +} + /** * Construct the git URL for an App Builder project. */ @@ -560,7 +573,7 @@ export async function getProject( err ); sessionInitiated = null; - sessionPrepared = null; + sessionPrepared = isDefinitiveSessionNotFoundError(err) ? false : null; } } else if (activeSession) { // Active session is a legacy v1 session — fetch its messages from R2 so diff --git a/apps/web/src/lib/app-builder/github-migration-service.test.ts b/apps/web/src/lib/app-builder/github-migration-service.test.ts new file mode 100644 index 0000000000..e448e97f54 --- /dev/null +++ b/apps/web/src/lib/app-builder/github-migration-service.test.ts @@ -0,0 +1,102 @@ +jest.mock('@/lib/drizzle', () => ({ + db: { update: jest.fn() }, +})); +jest.mock('@/lib/app-builder/app-builder-client', () => ({ + migrateToGithub: jest.fn(), +})); +jest.mock('@/lib/app-builder/project-ownership', () => ({ + getProjectWithOwnershipCheck: jest.fn(), +})); +jest.mock('@/lib/integrations/db/platform-integrations', () => ({ + getIntegrationForOwner: jest.fn(), +})); +jest.mock('@/lib/integrations/platforms/github/adapter', () => ({ + fetchGitHubInstallationDetails: jest.fn(), + fetchGitHubRepositories: jest.fn(), + getInstallationSettingsUrl: jest.fn(), + getRepositoryDetails: jest.fn(), +})); + +import { db } from '@/lib/drizzle'; +import * as appBuilderClient from '@/lib/app-builder/app-builder-client'; +import { getProjectWithOwnershipCheck } from '@/lib/app-builder/project-ownership'; +import { getIntegrationForOwner } from '@/lib/integrations/db/platform-integrations'; +import { getRepositoryDetails } from '@/lib/integrations/platforms/github/adapter'; +import { migrateProjectToGitHub } from './github-migration-service'; + +describe('migrateProjectToGitHub Worker error mapping', () => { + const cleanupSet = jest.fn(); + + beforeEach(() => { + jest.resetAllMocks(); + jest.spyOn(console, 'error').mockImplementation(() => undefined); + + jest.mocked(getProjectWithOwnershipCheck).mockResolvedValue({} as never); + jest.mocked(getIntegrationForOwner).mockResolvedValue({ + id: 'integration-id', + platform_installation_id: 'installation-id', + } as never); + jest.mocked(getRepositoryDetails).mockResolvedValue({ + fullName: 'kilocode/example', + cloneUrl: 'https://github.com/kilocode/example.git', + htmlUrl: 'https://github.com/kilocode/example', + isEmpty: true, + isPrivate: true, + }); + + const claim = { + set: jest.fn().mockReturnValue({ + where: jest.fn().mockReturnValue({ + returning: jest + .fn() + .mockResolvedValue([{ deployment_id: null, session_id: 'session-id' }]), + }), + }), + }; + cleanupSet.mockReturnValue({ where: jest.fn().mockResolvedValue(undefined) }); + /* eslint-disable drizzle/enforce-update-with-where -- This configures the query mock. */ + jest + .mocked(db.update) + .mockReturnValueOnce(claim as never) + .mockReturnValueOnce({ set: cleanupSet } as never); + /* eslint-enable drizzle/enforce-update-with-where */ + }); + + it.each([ + ['push_failed', 'push_failed'], + ['invalid_request', 'internal_error'], + ['token_failed', 'internal_error'], + ['internal_error', 'internal_error'], + ] as const)('maps Worker %s to public %s', async (workerError, publicError) => { + jest.mocked(appBuilderClient.migrateToGithub).mockResolvedValue({ + success: false, + error: workerError, + message: 'sensitive detail', + }); + + await expect( + migrateProjectToGitHub({ + projectId: 'project-id', + owner: { type: 'user', id: 'user_2abc123' }, + userId: 'user_2abc123', + repoFullName: 'kilocode/example', + }) + ).resolves.toEqual({ success: false, error: publicError }); + expect(cleanupSet).toHaveBeenCalledWith({ migrated_at: null }); + }); + + it('maps transport errors to internal_error', async () => { + jest + .mocked(appBuilderClient.migrateToGithub) + .mockRejectedValue(new Error('sensitive transport detail')); + + await expect( + migrateProjectToGitHub({ + projectId: 'project-id', + owner: { type: 'user', id: 'user_2abc123' }, + userId: 'user_2abc123', + repoFullName: 'kilocode/example', + }) + ).resolves.toEqual({ success: false, error: 'internal_error' }); + }); +}); diff --git a/apps/web/src/lib/app-builder/github-migration-service.ts b/apps/web/src/lib/app-builder/github-migration-service.ts index 44051a3415..e34fc7bb32 100644 --- a/apps/web/src/lib/app-builder/github-migration-service.ts +++ b/apps/web/src/lib/app-builder/github-migration-service.ts @@ -232,11 +232,14 @@ export async function migrateProjectToGitHub( }); if (!migrateResult.success) { - throw new MigrationError('push_failed', { cause: migrateResult }); + throw new MigrationError( + migrateResult.error === 'push_failed' ? 'push_failed' : 'internal_error', + { cause: migrateResult } + ); } } catch (error) { if (error instanceof MigrationError) throw error; - throw new MigrationError('push_failed', { cause: error }); + throw new MigrationError('internal_error', { cause: error }); } // 5. Update deployment if exists diff --git a/services/app-builder/src/api-schemas.test.ts b/services/app-builder/src/api-schemas.test.ts new file mode 100644 index 0000000000..5a30e41476 --- /dev/null +++ b/services/app-builder/src/api-schemas.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from 'vitest'; +import { MigrateToGithubRequestSchema } from './api-schemas'; + +describe('MigrateToGithubRequestSchema', () => { + const request = { + githubRepo: 'kilocode/example', + userId: 'user_2abc123', + }; + + it('accepts non-empty text user IDs', () => { + expect(MigrateToGithubRequestSchema.parse(request)).toEqual(request); + }); + + it('rejects empty user IDs', () => { + expect(() => MigrateToGithubRequestSchema.parse({ ...request, userId: '' })).toThrow(); + }); + + it('keeps organization IDs UUID-only', () => { + expect(() => + MigrateToGithubRequestSchema.parse({ ...request, orgId: 'org_2abc123' }) + ).toThrow(); + expect( + MigrateToGithubRequestSchema.parse({ + ...request, + orgId: '123e4567-e89b-42d3-a456-426614174000', + }) + ).toEqual({ + ...request, + orgId: '123e4567-e89b-42d3-a456-426614174000', + }); + }); +}); diff --git a/services/app-builder/src/api-schemas.ts b/services/app-builder/src/api-schemas.ts index 9c1b452abe..399718296e 100644 --- a/services/app-builder/src/api-schemas.ts +++ b/services/app-builder/src/api-schemas.ts @@ -141,7 +141,7 @@ export type DeleteErrorResponse = z.infer; export const MigrateToGithubRequestSchema = z.object({ githubRepo: z.string().regex(/^[^/]+\/[^/]+$/, 'Must be in "owner/repo" format'), - userId: z.string().uuid(), + userId: z.string().min(1), orgId: z.string().uuid().optional(), });