From efb5f79215a8f4f8dc8cad61369cc2f95eefd793 Mon Sep 17 00:00:00 2001 From: Evan Jacobson Date: Mon, 31 Aug 2026 12:20:49 -0600 Subject: [PATCH 1/4] 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(), }); From 36f25a9ff3f883cca9c6fb50d20cb1acaba9989a Mon Sep 17 00:00:00 2001 From: Evan Jacobson Date: Tue, 1 Sep 2026 15:15:08 -0600 Subject: [PATCH 2/4] (chore) lint --- .../components/app-builder/AppBuilderChat.tsx | 40 ++- .../components/app-builder/ProjectManager.ts | 122 ++++++--- .../components/app-builder/PromptInput.tsx | 3 + .../__tests__/ProjectManager.test.ts | 244 ++++++++++++++++++ .../__tests__/preview-polling.test.ts | 2 + .../project-manager/__tests__/store.test.ts | 3 + .../__tests__/streaming.test.ts | 1 + .../project-manager/sessions/session-store.ts | 2 + .../project-manager/sessions/v2/streaming.ts | 16 +- .../app-builder/project-manager/store.ts | 2 + .../app-builder/project-manager/types.ts | 4 + .../app-builder/app-builder-client.test.ts | 38 +++ .../src/lib/app-builder/app-builder-client.ts | 41 ++- .../app-builder/app-builder-service.test.ts | 239 +++++++++++++++++ .../lib/app-builder/app-builder-service.ts | 145 ++++++++--- .../github-migration-service.test.ts | 121 +++++++++ .../app-builder/github-migration-service.ts | 25 +- 17 files changed, 933 insertions(+), 115 deletions(-) create mode 100644 apps/web/src/components/app-builder/project-manager/__tests__/ProjectManager.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 diff --git a/apps/web/src/components/app-builder/AppBuilderChat.tsx b/apps/web/src/components/app-builder/AppBuilderChat.tsx index d77ba5a8be..8057886705 100644 --- a/apps/web/src/components/app-builder/AppBuilderChat.tsx +++ b/apps/web/src/components/app-builder/AppBuilderChat.tsx @@ -508,7 +508,15 @@ 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, + isConnecting, + isInterrupting, + model: projectModel, + sessions, + pendingNewSession, + isRecoveringSession, + } = state; const messagesEndRef = useRef(null); const scrollContainerRef = useRef(null); @@ -532,14 +540,14 @@ export function AppBuilderChat({ organizationId }: AppBuilderChatProps) { setVisibleSessionCount(DEFAULT_VISIBLE_SESSIONS); }, [manager]); - // Clear the submit-count once the awaited new session has arrived, or if - // the request failed (isStreaming drops back to false with no new session). + // Clear the submit-count once the awaited new session has arrived, or once + // neither connection nor streaming is in progress after a failed request. useEffect(() => { if (sessionCountAtSubmit === null) return; - if (sessions.length > sessionCountAtSubmit || !isStreaming) { + if (sessions.length > sessionCountAtSubmit || (!isStreaming && !isConnecting)) { setSessionCountAtSubmit(null); } - }, [sessions.length, sessionCountAtSubmit, isStreaming]); + }, [sessions.length, sessionCountAtSubmit, isStreaming, isConnecting]); // Fetch eligibility to check if user can use App Builder const personalEligibilityQuery = useQuery({ @@ -691,7 +699,7 @@ export function AppBuilderChat({ organizationId }: AppBuilderChatProps) { // until the new session object actually arrives in state. setSessionCountAtSubmit(sessions.length); } - manager.sendMessage(value, images, selectedModel || undefined); + await manager.sendMessage(value, images, selectedModel || undefined); // PromptInput clears itself internally after successful submit setMessageUuid(uuidv4()); }, @@ -743,7 +751,7 @@ export function AppBuilderChat({ organizationId }: AppBuilderChatProps) { variant="ghost" size="icon" onClick={handleNewChatToggle} - disabled={isStreaming} + disabled={isStreaming || isConnecting || isRecoveringSession} className={pendingNewSession ? 'text-primary bg-primary/10 h-8 w-8' : 'h-8 w-8'} aria-label="New chat" > @@ -751,7 +759,11 @@ export function AppBuilderChat({ organizationId }: AppBuilderChatProps) { - {pendingNewSession ? 'Cancel new chat' : 'New chat'} + {isRecoveringSession + ? 'A new chat is required' + : pendingNewSession + ? 'Cancel new chat' + : 'New chat'} @@ -839,13 +851,15 @@ export function AppBuilderChat({ organizationId }: AppBuilderChatProps) { placeholder={ isStreaming ? 'Building...' - : pendingNewSession - ? 'What would you like to change?' - : 'Describe changes to your app...' + : isConnecting + ? 'Connecting...' + : pendingNewSession + ? 'What would you like to change?' + : 'Describe changes to your app...' } disabled={(!hasAnyMessages && !pendingNewSession) || isBlocked} - isSubmitting={isStreaming} - onInterrupt={handleInterrupt} + isSubmitting={isStreaming || isConnecting} + onInterrupt={isStreaming ? handleInterrupt : undefined} isInterrupting={isInterrupting} onImagesChange={handleImagesChange} models={modelOptions} diff --git a/apps/web/src/components/app-builder/ProjectManager.ts b/apps/web/src/components/app-builder/ProjectManager.ts index 1b2bb96284..424288df1a 100644 --- a/apps/web/src/components/app-builder/ProjectManager.ts +++ b/apps/web/src/components/app-builder/ProjectManager.ts @@ -41,7 +41,7 @@ export type ProjectManager = { destroyed: boolean; subscribe: (listener: () => void) => () => void; getState: () => ProjectState; - sendMessage: (message: string, images?: Images, model?: string) => void; + sendMessage: (message: string, images?: Images, model?: string) => Promise; interrupt: () => void; setCurrentIframeUrl: (url: string | null) => void; setGitRepoFullName: (repoFullName: string) => void; @@ -64,7 +64,8 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag let pendingInitialStreamingStart = false; let pendingReconnect = false; let hasStartedInitialStreaming = false; - let sessionUnsubscribes: Array<() => void> = []; + const sessionUnsubscribes = new Map void>(); + let sessionCreationOperation = 0; const initialState = createInitialState( project.deployment_id ?? null, @@ -101,17 +102,24 @@ 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 { const unsubscribe = session.subscribe(() => { - const active = getActiveSession(); - const isStreaming = active?.getState().isStreaming ?? false; - store.setState({ isStreaming }); + if (destroyed || getActiveSession() !== session) return; + const { isStreaming, isConnecting } = session.getState(); + store.setState({ isStreaming, isConnecting }); }); - sessionUnsubscribes.push(unsubscribe); + sessionUnsubscribes.set(session, unsubscribe); + } + + function unsubscribeFromSession(session: AppBuilderSession): void { + sessionUnsubscribes.get(session)?.(); + sessionUnsubscribes.delete(session); } /** @@ -123,12 +131,17 @@ 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) { @@ -173,6 +186,7 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag const currentActive = getActiveSession(); if (currentActive) { currentActive.info.ended_at = new Date().toISOString(); + unsubscribeFromSession(currentActive); currentActive.destroy(); } @@ -198,7 +212,9 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag const currentSessions = store.getState().sessions; store.setState({ sessions: [...currentSessions, newSession], - isStreaming: true, + isStreaming: false, + isConnecting: true, + isRecoveringSession: false, }); cloudAgentSessionId = newSessionId; @@ -259,15 +275,25 @@ 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; + if (activeProjectSessionInfo.worker_version === 'v2') { + store.setState({ isConnecting: true }); + } } else { + if (cloudAgentSessionId) { + store.setState({ pendingNewSession: true, isRecoveringSession: true }); + } startPreviewPollingIfNeeded(); } @@ -303,16 +329,15 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag return store.getState(); } - function sendMessage(message: string, images?: Images, model?: string): void { + async function sendMessage(message: string, images?: Images, model?: string): Promise { if (store.getState().pendingNewSession) { - sendMessageAsNewSession(message, images, model); - return; + return sendMessageAsNewSession(message, images, model); } const activeSession = getActiveSession(); if (!activeSession) { logger.logWarn('Cannot send message: no active session'); - return; + throw new Error('Cannot send message: no active session'); } if (model) { @@ -320,7 +345,7 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag } const effectiveModel = model ?? store.getState().model; - void activeSession.sendMessage(message, images, effectiveModel); + return activeSession.sendMessage(message, images, effectiveModel); } /** @@ -328,10 +353,14 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag * Calls sendMessage tRPC mutation with forceNewSession:true, then delegates * to handleSessionChanged to create the new session object and begin streaming. */ - function sendMessageAsNewSession(message: string, images?: Images, model?: string): void { + async function sendMessageAsNewSession( + message: string, + images?: Images, + model?: string + ): Promise { if (destroyed) { logger.logWarn('Cannot start new session: ProjectManager is destroyed'); - return; + throw new Error('Cannot start new session: ProjectManager is destroyed'); } if (model) { @@ -339,8 +368,10 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag } const effectiveModel = model ?? store.getState().model; + const isRecoveringSession = store.getState().isRecoveringSession; - store.setState({ pendingNewSession: false, isStreaming: true }); + const operation = ++sessionCreationOperation; + store.setState({ pendingNewSession: false, isStreaming: false, isConnecting: true }); const mutationPromise = organizationId ? trpcClient.organizations.appBuilder.sendMessage.mutate({ @@ -359,19 +390,25 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag forceNewSession: true, }); - void mutationPromise - .then(result => { - if (destroyed) return; - handleSessionChanged(result.cloudAgentSessionId, { - text: message, - images, - }); - }) - .catch((err: Error) => { - if (destroyed) return; - logger.logError('Failed to start new session', err); - store.setState({ isStreaming: false }); + try { + const result = await mutationPromise; + if (destroyed || operation !== sessionCreationOperation) return; + handleSessionChanged(result.cloudAgentSessionId, { + text: message, + images, }); + } catch (err) { + if (!destroyed && operation === sessionCreationOperation) { + logger.logError('Failed to start new session', err); + store.setState({ + pendingNewSession: true, + isRecoveringSession, + isStreaming: false, + isConnecting: false, + }); + } + throw err; + } } function interrupt(): void { @@ -423,26 +460,29 @@ 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; } - store.setState({ pendingNewSession: false }); + store.setState({ pendingNewSession: false, isRecoveringSession: false }); } function destroy(): void { if (destroyed) return; destroyed = true; - for (const unsub of sessionUnsubscribes) { + sessionCreationOperation++; + + for (const unsub of sessionUnsubscribes.values()) { unsub(); } - sessionUnsubscribes = []; + sessionUnsubscribes.clear(); for (const session of store.getState().sessions) { session.destroy(); diff --git a/apps/web/src/components/app-builder/PromptInput.tsx b/apps/web/src/components/app-builder/PromptInput.tsx index a5c4d9e6d4..1094517650 100644 --- a/apps/web/src/components/app-builder/PromptInput.tsx +++ b/apps/web/src/components/app-builder/PromptInput.tsx @@ -117,6 +117,7 @@ const BottomBar = memo(function BottomBar({ onValueChange={onModelChange} isLoading={isLoadingModels} error={modelsError} + disabled={disabled || isSubmitting} placeholder="Model" className="w-56 bg-zinc-800 text-zinc-400 hover:bg-zinc-700 hover:text-zinc-200" /> @@ -162,6 +163,7 @@ const BottomBar = memo(function BottomBar({ onClick={onInterrupt} disabled={isInterrupting} className="h-9 w-9" + aria-label="Stop building" > @@ -173,6 +175,7 @@ const BottomBar = memo(function BottomBar({ onClick={onSubmit} disabled={isSubmitDisabled} className="h-9 w-9" + aria-label="Send message" > 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..0fdbf4bc56 --- /dev/null +++ b/apps/web/src/components/app-builder/project-manager/__tests__/ProjectManager.test.ts @@ -0,0 +1,244 @@ +import type { StoredMessage } from '@/components/cloud-agent-next/types'; +import type { ProjectWithMessages } from '@/lib/app-builder/types'; +import { createProjectManager, type ProjectManagerConfig } from '../../ProjectManager'; +import { createSessionStore, type SessionStore } from '../sessions/session-store'; +import { createV1Session } from '../sessions/v1/v1-session'; +import { createV2Session, type CreateV2SessionConfig } from '../sessions/v2/v2-session'; +import type { V2Session } from '../sessions/types'; + +jest.mock('../preview-polling', () => ({ + startPreviewPolling: jest.fn(() => ({ isPolling: true, stop: jest.fn() })), +})); +jest.mock('../deployments', () => ({ deploy: jest.fn() })); +jest.mock('../sessions/v1/v1-session', () => ({ createV1Session: jest.fn() })); +jest.mock('../sessions/v2/v2-session', () => ({ createV2Session: jest.fn() })); + +type TestSession = V2Session & { + store: SessionStore; + capturedListeners: Array<() => void>; +}; + +const createdSessions: TestSession[] = []; +const mockCreateV1Session = jest.mocked(createV1Session); +const mockCreateV2Session = jest.mocked(createV2Session); + +function createTestSession(config: CreateV2SessionConfig): TestSession { + const store = createSessionStore(config.initialMessages); + const capturedListeners: Array<() => void> = []; + const session: TestSession = { + type: 'v2', + info: config.info, + getState: store.getState, + subscribe: listener => { + capturedListeners.push(listener); + return store.subscribe(listener); + }, + getChildSessionMessages: store.getChildSessionMessages, + sendMessage: jest.fn(async () => {}), + interrupt: jest.fn(async () => {}), + startInitialStreaming: jest.fn(), + connectToExistingSession: jest.fn(() => store.setState({ isConnecting: true })), + loadMessages: jest.fn(), + destroy: jest.fn(), + store, + capturedListeners, + }; + createdSessions.push(session); + return session; +} + +function createProject(): ProjectWithMessages { + return { + id: 'project-1', + session_id: 'session-old', + deployment_id: null, + model_id: null, + git_repo_full_name: null, + messages: [], + sessions: [ + { + id: 'session-old', + cloud_agent_session_id: 'session-old', + worker_version: 'v2', + ended_at: null, + title: null, + initiated: true, + prepared: true, + }, + ], + } as unknown as ProjectWithMessages; +} + +function createTrpcClient(sendMessage: jest.Mock = jest.fn()) { + return { + appBuilder: { + sendMessage: { mutate: sendMessage }, + interruptSession: { mutate: jest.fn(async () => ({ success: true })) }, + }, + organizations: { + appBuilder: { + sendMessage: { mutate: jest.fn() }, + interruptSession: { mutate: jest.fn(async () => ({ success: true })) }, + }, + }, + }; +} + +function createManager( + sendMessage: jest.Mock = jest.fn(), + project: ProjectWithMessages = createProject() +) { + const trpcClient = createTrpcClient(sendMessage); + const manager = createProjectManager({ + project, + organizationId: null, + trpcClient: trpcClient as unknown as ProjectManagerConfig['trpcClient'], + }); + return { manager, trpcClient }; +} + +function flushNotifications(): Promise { + return new Promise(resolve => setTimeout(resolve, 0)); +} + +describe('ProjectManager session handoff', () => { + beforeEach(() => { + createdSessions.length = 0; + jest.clearAllMocks(); + mockCreateV1Session.mockImplementation(() => { + throw new Error('Unexpected V1 session'); + }); + mockCreateV2Session.mockImplementation(createTestSession); + }); + + it('uses a connecting guard without claiming an existing idle session is streaming', async () => { + const { manager } = createManager(); + const activeSession = createdSessions[0]; + + expect(manager.getState()).toMatchObject({ isConnecting: true, isStreaming: false }); + + const unsubscribe = manager.subscribe(jest.fn()); + await Promise.resolve(); + + expect(activeSession.connectToExistingSession).toHaveBeenCalledWith('session-old'); + expect(manager.getState()).toMatchObject({ isConnecting: true, isStreaming: false }); + + activeSession.store.setState({ isConnecting: false, isStreaming: false }); + await flushNotifications(); + + expect(manager.getState()).toMatchObject({ isConnecting: false, isStreaming: false }); + unsubscribe(); + manager.destroy(); + }); + + it('ignores a queued notification owned by the destroyed session after handoff', async () => { + const sendMessage = jest.fn(async () => ({ cloudAgentSessionId: 'session-new' })); + const { manager } = createManager(sendMessage); + const oldSession = createdSessions[0]; + + manager.requestNewSession(); + await manager.sendMessage('continue building'); + + const newSession = createdSessions[1]; + expect(oldSession.destroy).toHaveBeenCalledTimes(1); + expect(manager.getState()).toMatchObject({ isConnecting: true, isStreaming: false }); + + newSession.store.setState({ isConnecting: false, isStreaming: true }); + await flushNotifications(); + expect(manager.getState().isStreaming).toBe(true); + + newSession.store.setState({ isStreaming: false }); + oldSession.capturedListeners[0]?.(); + + expect(manager.getState().isStreaming).toBe(true); + await flushNotifications(); + expect(manager.getState().isStreaming).toBe(false); + manager.destroy(); + }); + + it('propagates replacement mutation failure and clears the non-interruptible guard', async () => { + const error = new Error('replacement failed'); + const { manager } = createManager(jest.fn(async () => Promise.reject(error))); + + manager.requestNewSession(); + + await expect(manager.sendMessage('retryable prompt')).rejects.toBe(error); + expect(manager.getState()).toMatchObject({ + pendingNewSession: true, + isRecoveringSession: false, + isConnecting: false, + isStreaming: false, + }); + manager.destroy(); + }); + + it('reconnects the canonical session instead of a newer orphan row', async () => { + const project = createProject(); + project.sessions = [ + { ...project.sessions[0]!, ended_at: '2026-08-31T00:00:00.000Z' }, + { + ...project.sessions[0]!, + id: 'orphan-row', + cloud_agent_session_id: 'orphan-session', + }, + ]; + const { manager } = createManager(jest.fn(), project); + + manager.subscribe(jest.fn()); + await Promise.resolve(); + + expect(createdSessions[0]?.info.cloud_agent_session_id).toBe('orphan-session'); + expect(createdSessions[0]?.connectToExistingSession).not.toHaveBeenCalled(); + expect(createdSessions[1]?.info.cloud_agent_session_id).toBe('session-old'); + expect(createdSessions[1]?.connectToExistingSession).toHaveBeenCalledWith('session-old'); + manager.destroy(); + }); + + it('requires a replacement when the canonical session is missing and preserves recovery on failure', async () => { + const project = createProject(); + project.sessions = [ + { + ...project.sessions[0]!, + id: 'orphan-row', + cloud_agent_session_id: 'orphan-session', + }, + ]; + const error = new Error('replacement failed'); + const { manager } = createManager( + jest.fn(async () => Promise.reject(error)), + project + ); + + expect(manager.getState()).toMatchObject({ + pendingNewSession: true, + isRecoveringSession: true, + isConnecting: false, + }); + manager.cancelNewSession(); + expect(manager.getState().pendingNewSession).toBe(true); + + await expect(manager.sendMessage('recover this project')).rejects.toBe(error); + expect(manager.getState()).toMatchObject({ + pendingNewSession: true, + isRecoveringSession: true, + isConnecting: false, + }); + manager.destroy(); + }); + + it('does not apply replacement completion after the manager is destroyed', async () => { + let resolveMutation: ((result: { cloudAgentSessionId: string }) => void) | undefined; + const mutation = new Promise<{ cloudAgentSessionId: string }>(resolve => { + resolveMutation = resolve; + }); + const { manager } = createManager(jest.fn(() => mutation)); + + manager.requestNewSession(); + const submission = manager.sendMessage('late prompt'); + manager.destroy(); + resolveMutation?.({ cloudAgentSessionId: 'session-late' }); + await submission; + + expect(createdSessions).toHaveLength(1); + }); +}); 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..6168079a17 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 @@ -12,6 +12,7 @@ function createMockStore(): ProjectStore & { stateUpdates: Array> = []; let currentState: ProjectState = { isStreaming: false, + isConnecting: false, isInterrupting: false, previewUrl: null, previewStatus: 'idle', @@ -21,6 +22,7 @@ function createMockStore(): ProjectStore & { stateUpdates: Array { const state = createInitialState(null, null, null); expect(state.isStreaming).toBe(false); + expect(state.isConnecting).toBe(false); expect(state.previewUrl).toBeNull(); expect(state.previewStatus).toBe('idle'); expect(state.deploymentId).toBeNull(); @@ -52,6 +53,7 @@ describe('createInitialState', () => { describe('createProjectStore', () => { const initialState: ProjectState = { isStreaming: false, + isConnecting: false, isInterrupting: false, previewUrl: null, previewStatus: 'idle', @@ -61,6 +63,7 @@ describe('createProjectStore', () => { gitRepoFullName: null, sessions: [], pendingNewSession: false, + isRecoveringSession: false, }; describe('getState', () => { diff --git a/apps/web/src/components/app-builder/project-manager/__tests__/streaming.test.ts b/apps/web/src/components/app-builder/project-manager/__tests__/streaming.test.ts index 9cf4e08fc0..45ec7e176b 100644 --- a/apps/web/src/components/app-builder/project-manager/__tests__/streaming.test.ts +++ b/apps/web/src/components/app-builder/project-manager/__tests__/streaming.test.ts @@ -31,6 +31,7 @@ function createMockStore(): { getState: () => ({ messages, isStreaming, + isConnecting: false, questionRequestIds: new Map(), childSessionMessages, }), diff --git a/apps/web/src/components/app-builder/project-manager/sessions/session-store.ts b/apps/web/src/components/app-builder/project-manager/sessions/session-store.ts index fff05dacd3..6620388636 100644 --- a/apps/web/src/components/app-builder/project-manager/sessions/session-store.ts +++ b/apps/web/src/components/app-builder/project-manager/sessions/session-store.ts @@ -10,6 +10,7 @@ export type SessionState = { messages: TMessage[]; isStreaming: boolean; + isConnecting: boolean; questionRequestIds: Map; /** Messages from child/subagent sessions, keyed by child session ID */ childSessionMessages: Map; @@ -32,6 +33,7 @@ export function createSessionStore(initialMessages: TMessage[]): Sessi let state: SessionState = { messages: initialMessages, isStreaming: false, + isConnecting: false, questionRequestIds: new Map(), childSessionMessages: new Map(), }; 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..6ad95732ae 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 @@ -177,10 +177,10 @@ export function createV2StreamingCoordinator(config: V2StreamingConfig): V2Strea onSessionStatusChanged: status => { if (status.type === 'idle') { - store.setState({ isStreaming: false }); + store.setState({ isStreaming: false, isConnecting: false }); onStreamComplete?.(); } else if (status.type === 'busy') { - store.setState({ isStreaming: true }); + store.setState({ isStreaming: true, isConnecting: false }); } }, @@ -194,11 +194,11 @@ export function createV2StreamingCoordinator(config: V2StreamingConfig): V2Strea onError: (error, _sessionId) => { logger.logError('V2 EventProcessor error', new Error(error)); - store.setState({ isStreaming: false }); + store.setState({ isStreaming: false, isConnecting: false }); }, onStreamingChanged: streaming => { - store.setState({ isStreaming: streaming }); + store.setState({ isStreaming: streaming, isConnecting: false }); // onStreamComplete is called from onSessionStatusChanged (idle) — not here, // to avoid triggering preview polling twice per stream completion. }, @@ -232,7 +232,7 @@ export function createV2StreamingCoordinator(config: V2StreamingConfig): V2Strea if (state.status === 'error' || state.status === 'disconnected') { // Force-complete all in-flight messages so they don't appear stuck in streaming state processor?.forceCompleteAll(); - store.setState({ isStreaming: false }); + store.setState({ isStreaming: false, isConnecting: false }); if (state.status === 'disconnected') { onStreamComplete?.(); } @@ -469,14 +469,14 @@ export function createV2StreamingCoordinator(config: V2StreamingConfig): V2Strea } logger.log('Connecting to existing V2 session', { sessionId }); - store.setState({ isStreaming: true }); + store.setState({ isConnecting: true }); void (async () => { try { await connectWs(sessionId); } catch (err) { logger.logError('Failed to connect to existing V2 session', err); - store.setState({ isStreaming: false }); + store.setState({ isStreaming: false, isConnecting: false }); } })(); } @@ -503,7 +503,7 @@ export function createV2StreamingCoordinator(config: V2StreamingConfig): V2Strea // Force-complete all in-flight messages so they don't appear stuck in streaming state processor?.forceCompleteAll(); - store.setState({ isStreaming: false }); + store.setState({ isStreaming: false, isConnecting: false }); } /** 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..7f0757cfb4 100644 --- a/apps/web/src/components/app-builder/project-manager/store.ts +++ b/apps/web/src/components/app-builder/project-manager/store.ts @@ -23,6 +23,7 @@ export function createInitialState( ): ProjectState { return { isStreaming: false, + isConnecting: false, isInterrupting: false, previewUrl: null, previewStatus: 'idle', @@ -32,6 +33,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..8ce3caf9d4 100644 --- a/apps/web/src/components/app-builder/project-manager/types.ts +++ b/apps/web/src/components/app-builder/project-manager/types.ts @@ -16,6 +16,8 @@ export type PreviewStatus = 'idle' | 'building' | 'running' | 'error'; export type ProjectState = { /** Derived from active session — true if active session is streaming */ isStreaming: boolean; + /** True while the active session is connecting and its streaming status is unknown */ + isConnecting: boolean; isInterrupting: boolean; previewUrl: string | null; previewStatus: PreviewStatus; @@ -29,6 +31,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..1aa8baa861 --- /dev/null +++ b/apps/web/src/lib/app-builder/app-builder-client.test.ts @@ -0,0 +1,38 @@ +jest.mock('@/lib/config.server', () => ({ + APP_BUILDER_URL: 'https://app-builder.example.com', + APP_BUILDER_AUTH_TOKEN: 'worker-auth-token', +})); + +import { AppBuilderError, migrateToGithub } from './app-builder-client'; + +const request = { + githubRepo: 'owner/repo', + userId: '00000000-0000-4000-8000-000000000000', +}; + +describe('migrateToGithub', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('returns a typed failure envelope for a non-2xx response', async () => { + const failure = { + success: false as const, + error: 'push_failed' as const, + message: 'push rejected https://oauth2:secret-token@github.com/owner/repo.git', + }; + jest.spyOn(global, 'fetch').mockResolvedValue(Response.json(failure, { status: 500 })); + + await expect(migrateToGithub('project-1', request)).resolves.toEqual(failure); + }); + + it('does not attach an untrusted response body to parsing errors', async () => { + const sensitiveBody = 'https://oauth2:secret-token@github.com/owner/repo.git'; + jest.spyOn(global, 'fetch').mockResolvedValue(new Response(sensitiveBody, { status: 502 })); + + const error = await migrateToGithub('project-1', request).catch(value => value); + + expect(error).toBeInstanceOf(AppBuilderError); + expect(String(error)).not.toContain(sensitiveBody); + }); +}); 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..d990e65c7d 100644 --- a/apps/web/src/lib/app-builder/app-builder-client.ts +++ b/apps/web/src/lib/app-builder/app-builder-client.ts @@ -285,24 +285,39 @@ export async function migrateToGithub( const baseUrl = getBaseUrl(); const endpoint = `${baseUrl}/apps/${encodeURIComponent(projectId)}/migrate-to-github`; - const response = await fetch(endpoint, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - ...(APP_BUILDER_AUTH_TOKEN && { Authorization: `Bearer ${APP_BUILDER_AUTH_TOKEN}` }), - }, - body: JSON.stringify(config), - }); + let response: Response; + try { + response = await fetch(endpoint, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + ...(APP_BUILDER_AUTH_TOKEN && { Authorization: `Bearer ${APP_BUILDER_AUTH_TOKEN}` }), + }, + body: JSON.stringify(config), + }); + } catch { + throw new AppBuilderError('App Builder GitHub migration request failed', undefined, endpoint); + } - if (!response.ok) { - const errorText = await response.text().catch(() => 'Unknown error'); + let data: unknown; + try { + data = await response.json(); + } catch { throw new AppBuilderError( - `Failed to migrate project ${projectId} to GitHub: ${response.status} ${response.statusText} - ${errorText}`, + 'App Builder GitHub migration returned an invalid response', response.status, endpoint ); } - const data = await response.json(); - return MigrateToGithubResponseSchema.parse(data); + const parsed = MigrateToGithubResponseSchema.safeParse(data); + if (!parsed.success || (!response.ok && parsed.data.success)) { + throw new AppBuilderError( + 'App Builder GitHub migration returned an invalid response', + response.status, + endpoint + ); + } + + 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..74a3db4847 --- /dev/null +++ b/apps/web/src/lib/app-builder/app-builder-service.test.ts @@ -0,0 +1,239 @@ +import { beforeAll, beforeEach, describe, expect, it, jest } from '@jest/globals'; +import { and, eq, isNull } from 'drizzle-orm'; +import { cleanupDbForTest, db, pool } from '@/lib/drizzle'; +import { + app_builder_projects, + app_builder_project_sessions, + AppBuilderSessionReason, +} from '@kilocode/db/schema'; +import { insertTestUser } from '@/tests/helpers/user.helper'; +import type * as serviceModule from './app-builder-service'; + +const mockPrepareSession = + jest.fn<(...args: unknown[]) => Promise<{ cloudAgentSessionId: string }>>(); +const mockInitiateFromPreparedSession = jest.fn< + (input: { cloudAgentSessionId: string }) => Promise<{ + cloudAgentSessionId: string; + executionId: string; + status: 'started'; + streamUrl: string; + messageId: string; + delivery: 'sent'; + }> +>(); +const mockInterruptSession = jest.fn<(...args: unknown[]) => Promise<{ success: boolean }>>(); +const mockCleanupSession = jest.fn<(...args: unknown[]) => Promise<{ success: boolean }>>(); +const mockGetSession = jest.fn<(...args: unknown[]) => Promise>>(); + +jest.mock('@/lib/cloud-agent-next/cloud-agent-client', () => ({ + createAppBuilderCloudAgentNextClient: () => ({ + prepareSession: mockPrepareSession, + initiateFromPreparedSession: mockInitiateFromPreparedSession, + interruptSession: mockInterruptSession, + cleanupSession: mockCleanupSession, + getSession: mockGetSession, + }), +})); + +let getProject: typeof serviceModule.getProject; +let sendMessage: typeof serviceModule.sendMessage; + +beforeAll(async () => { + ({ getProject, sendMessage } = await import('./app-builder-service')); +}); + +beforeEach(async () => { + await cleanupDbForTest(); + jest.clearAllMocks(); + mockInterruptSession.mockResolvedValue({ success: true }); + mockCleanupSession.mockResolvedValue({ success: true }); + mockGetSession.mockImplementation(async sessionId => ({ + sessionId, + userId: 'test-user', + execution: null, + preparedAt: Date.now(), + initiatedAt: Date.now(), + timestamp: Date.now(), + version: 1, + })); + mockInitiateFromPreparedSession.mockImplementation(async ({ cloudAgentSessionId }) => ({ + cloudAgentSessionId, + executionId: `execution-${cloudAgentSessionId}`, + status: 'started', + streamUrl: `/sessions/${cloudAgentSessionId}/stream`, + messageId: `message-${cloudAgentSessionId}`, + delivery: 'sent', + })); +}); + +async function createProjectFixture() { + const user = await insertTestUser(); + const [project] = await db + .insert(app_builder_projects) + .values({ + created_by_user_id: user.id, + owned_by_user_id: user.id, + session_id: 'session-a', + title: 'Snapshot project', + model_id: 'test-model', + git_repo_full_name: 'kilo/snapshot-project', + }) + .returning(); + + await db.insert(app_builder_project_sessions).values({ + project_id: project.id, + cloud_agent_session_id: 'session-a', + reason: AppBuilderSessionReason.Initial, + worker_version: 'v2', + }); + + return { project, owner: { type: 'user' as const, id: user.id } }; +} + +async function waitForProjectSessionReadBlockedBy(lockingPid: number): Promise { + const deadline = Date.now() + 5_000; + while (Date.now() < deadline) { + const result = await pool.query<{ waiting: boolean }>( + `SELECT EXISTS ( + SELECT 1 + FROM pg_locks + WHERE relation = 'app_builder_project_sessions'::regclass + AND NOT granted + AND $1::integer = ANY(pg_blocking_pids(pid)) + ) AS waiting`, + [lockingPid] + ); + if (result.rows[0]?.waiting) return; + await new Promise(resolve => setTimeout(resolve, 10)); + } + throw new Error('App Builder project read did not block on expected table lock'); +} + +describe('App Builder session consistency', () => { + it('marks a definitively missing canonical session as unprepared for recovery', async () => { + const { project, owner } = await createProjectFixture(); + mockGetSession.mockRejectedValueOnce({ + shape: { data: { code: 'NOT_FOUND', httpStatus: 404 } }, + }); + + const result = await getProject(project.id, owner, 'test-token'); + + expect(result.sessions).toEqual([ + expect.objectContaining({ + cloud_agent_session_id: 'session-a', + initiated: null, + prepared: false, + }), + ]); + }); + + it('allows only one concurrent new-session request to claim the canonical pointer', async () => { + const { project, owner } = await createProjectFixture(); + const bothPrepared = Promise.withResolvers(); + + mockPrepareSession.mockImplementation(async () => { + const callNumber = mockPrepareSession.mock.calls.length; + if (callNumber === 2) bothPrepared.resolve(); + await bothPrepared.promise; + return { cloudAgentSessionId: `session-${callNumber}` }; + }); + + const results = await Promise.allSettled([ + sendMessage({ + projectId: project.id, + owner, + message: 'first concurrent message', + authToken: 'test-token', + forceNewSession: true, + }), + sendMessage({ + projectId: project.id, + owner, + message: 'second concurrent message', + authToken: 'test-token', + forceNewSession: true, + }), + ]); + + const fulfilled = results.filter(result => result.status === 'fulfilled'); + const rejected = results.filter(result => result.status === 'rejected'); + expect(fulfilled).toHaveLength(1); + expect(rejected).toHaveLength(1); + expect(rejected[0]?.reason).toMatchObject({ code: 'CONFLICT' }); + + const [persistedProject] = await db + .select({ sessionId: app_builder_projects.session_id }) + .from(app_builder_projects) + .where(eq(app_builder_projects.id, project.id)); + const activeSessions = await db + .select({ sessionId: app_builder_project_sessions.cloud_agent_session_id }) + .from(app_builder_project_sessions) + .where( + and( + eq(app_builder_project_sessions.project_id, project.id), + isNull(app_builder_project_sessions.ended_at) + ) + ); + + expect(activeSessions).toEqual([{ sessionId: persistedProject.sessionId }]); + expect(['session-1', 'session-2']).toContain(persistedProject.sessionId); + + const losingSessionId = persistedProject.sessionId === 'session-1' ? 'session-2' : 'session-1'; + expect(mockInterruptSession).toHaveBeenCalledWith(losingSessionId); + expect(mockCleanupSession).toHaveBeenCalledWith(losingSessionId); + + const losingRows = await db + .select() + .from(app_builder_project_sessions) + .where(eq(app_builder_project_sessions.cloud_agent_session_id, losingSessionId)); + expect(losingRows).toHaveLength(0); + }); + + it('returns a coherent project and session snapshot across a concurrent rollover', async () => { + const { project, owner } = await createProjectFixture(); + const lockClient = await pool.connect(); + let projectPromise: ReturnType | null = null; + + try { + await lockClient.query('BEGIN'); + await lockClient.query('LOCK TABLE app_builder_project_sessions IN ACCESS EXCLUSIVE MODE'); + const pidResult = await lockClient.query<{ pid: number }>('SELECT pg_backend_pid() AS pid'); + const lockingPid = pidResult.rows[0]?.pid; + if (lockingPid === undefined) throw new Error('Could not determine locking database process'); + + projectPromise = getProject(project.id, owner, 'test-token'); + await waitForProjectSessionReadBlockedBy(lockingPid); + + await lockClient.query( + 'UPDATE app_builder_project_sessions SET ended_at = now() WHERE project_id = $1 AND cloud_agent_session_id = $2', + [project.id, 'session-a'] + ); + await lockClient.query( + `INSERT INTO app_builder_project_sessions + (project_id, cloud_agent_session_id, reason, worker_version) + VALUES ($1, $2, $3, 'v2')`, + [project.id, 'session-b', AppBuilderSessionReason.UserInitiated] + ); + await lockClient.query('UPDATE app_builder_projects SET session_id = $2 WHERE id = $1', [ + project.id, + 'session-b', + ]); + await lockClient.query('COMMIT'); + + const result = await projectPromise; + expect(result.session_id).toBe('session-b'); + expect(result.sessions).toEqual([ + expect.objectContaining({ + cloud_agent_session_id: 'session-a', + ended_at: expect.any(String), + }), + expect.objectContaining({ cloud_agent_session_id: 'session-b', ended_at: null }), + ]); + expect(mockGetSession).toHaveBeenCalledWith('session-b'); + } finally { + await lockClient.query('ROLLBACK').catch(() => undefined); + lockClient.release(); + if (projectPromise) await projectPromise.catch(() => undefined); + } + }, 20_000); +}); 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..25082f5436 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. */ @@ -89,20 +102,35 @@ function parseWorkerVersion(value: string | null): WorkerVersion | null { } /** - * Fetch all sessions for a project, ordered by created_at ascending. + * Fetch the project and all of its sessions in one statement so the canonical + * session pointer and session history come from the same PostgreSQL snapshot. */ -async function getProjectSessions(projectId: string): Promise { +async function getProjectSnapshot( + projectId: string, + owner: Owner +): Promise<{ project: AppBuilderProject; sessions: ProjectSessionInfo[] }> { + const ownerCondition = + owner.type === 'org' + ? eq(app_builder_projects.owned_by_organization_id, owner.id) + : eq(app_builder_projects.owned_by_user_id, owner.id); + const rows = await db .select({ - id: app_builder_project_sessions.id, - cloud_agent_session_id: app_builder_project_sessions.cloud_agent_session_id, - worker_version: app_builder_project_sessions.worker_version, - created_at: app_builder_project_sessions.created_at, - ended_at: app_builder_project_sessions.ended_at, - v1_title: cliSessions.title, - v2_title: cli_sessions_v2.title, + project: app_builder_projects, + session: { + id: app_builder_project_sessions.id, + cloud_agent_session_id: app_builder_project_sessions.cloud_agent_session_id, + worker_version: app_builder_project_sessions.worker_version, + ended_at: app_builder_project_sessions.ended_at, + v1_title: cliSessions.title, + v2_title: cli_sessions_v2.title, + }, }) - .from(app_builder_project_sessions) + .from(app_builder_projects) + .leftJoin( + app_builder_project_sessions, + eq(app_builder_projects.id, app_builder_project_sessions.project_id) + ) .leftJoin( cliSessions, eq(app_builder_project_sessions.cloud_agent_session_id, cliSessions.cloud_agent_session_id) @@ -114,18 +142,34 @@ async function getProjectSessions(projectId: string): Promise ({ - id: row.id, - cloud_agent_session_id: row.cloud_agent_session_id, - worker_version: parseWorkerVersion(row.worker_version) ?? 'v1', - ended_at: row.ended_at, - title: row.v1_title ?? row.v2_title ?? null, - initiated: null, - prepared: null, - })); + const project = rows[0]?.project; + if (!project) { + throw new TRPCError({ + code: 'NOT_FOUND', + message: 'Project not found', + }); + } + + const sessions = rows.flatMap(({ session }) => { + if (!session.id || !session.cloud_agent_session_id || !session.worker_version) return []; + + return [ + { + id: session.id, + cloud_agent_session_id: session.cloud_agent_session_id, + worker_version: parseWorkerVersion(session.worker_version) ?? 'v1', + ended_at: session.ended_at, + title: session.v1_title ?? session.v2_title ?? null, + initiated: null, + prepared: null, + } satisfies ProjectSessionInfo, + ]; + }); + + return { project, sessions }; } /** @@ -306,16 +350,29 @@ async function createCloudAgentNextSession( cloudAgentSessionId: newSessionId, }); - await db.transaction(async tx => { + const claimedSession = await db.transaction(async tx => { + const [claimedProject] = await tx + .update(app_builder_projects) + .set({ session_id: newSessionId }) + .where( + and( + eq(app_builder_projects.id, projectId), + eq(app_builder_projects.session_id, currentSessionId) + ) + ) + .returning({ id: app_builder_projects.id }); + + if (!claimedProject) return false; + await tx .update(app_builder_project_sessions) .set({ ended_at: sql`now()` }) - .where(eq(app_builder_project_sessions.cloud_agent_session_id, currentSessionId)); - - await tx - .update(app_builder_projects) - .set({ session_id: newSessionId }) - .where(eq(app_builder_projects.id, projectId)); + .where( + and( + eq(app_builder_project_sessions.project_id, projectId), + eq(app_builder_project_sessions.cloud_agent_session_id, currentSessionId) + ) + ); await tx.insert(app_builder_project_sessions).values({ project_id: projectId, @@ -323,8 +380,35 @@ async function createCloudAgentNextSession( reason: toSessionReason(reason), worker_version: REQUIRED_WORKER_VERSION, }); + + return true; }); + if (!claimedSession) { + try { + await client.interruptSession(newSessionId); + } catch (error) { + errorExceptInTest( + 'Failed to interrupt superseded App Builder session', + { cloudAgentSessionId: newSessionId, projectId }, + error + ); + } + + const cleanupResult = await client.cleanupSession(newSessionId); + if (!cleanupResult.success) { + errorExceptInTest('Failed to clean up superseded App Builder session', { + cloudAgentSessionId: newSessionId, + projectId, + }); + } + + throw new TRPCError({ + code: 'CONFLICT', + message: 'Project session changed while creating a new session.', + }); + } + return { cloudAgentSessionId: newSessionId, executionId: result.executionId, @@ -518,10 +602,7 @@ export async function getProject( owner: Owner, authToken: string ): Promise { - const project = await getProjectWithOwnershipCheck(projectId, owner); - - // Fetch all sessions for this project - const sessions = await getProjectSessions(projectId); + const { project, sessions } = await getProjectSnapshot(projectId, owner); // Session state for the active session (populated below). // Messages are only eagerly loaded for the active session; ended legacy v1 @@ -560,7 +641,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..86a940903c --- /dev/null +++ b/apps/web/src/lib/app-builder/github-migration-service.test.ts @@ -0,0 +1,121 @@ +jest.mock('@/lib/drizzle', () => ({ + db: { + update: jest.fn(() => ({ + set: jest.fn(() => ({ + where: jest.fn(() => ({ + returning: jest + .fn() + .mockResolvedValue([{ id: 'project-1', deployment_id: null, session_id: 'session-1' }]), + })), + })), + })), + }, +})); +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(), +})); +jest.mock('@/lib/app-builder/app-builder-client', () => { + class AppBuilderError extends Error { + constructor( + message: string, + public statusCode?: number + ) { + super(message); + } + } + + return { + AppBuilderError, + migrateToGithub: jest.fn(), + }; +}); + +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'; + +const sensitiveUrl = 'https://oauth2:secret-token@github.com/owner/repo.git'; +const params = { + projectId: 'project-1', + owner: { type: 'user' as const, id: 'user-1' }, + userId: 'user-1', + repoFullName: 'owner/repo', +}; + +describe('migrateProjectToGitHub error logging', () => { + beforeEach(() => { + jest.clearAllMocks(); + jest.mocked(getProjectWithOwnershipCheck).mockResolvedValue({} as never); + jest.mocked(getIntegrationForOwner).mockResolvedValue({ + id: 'integration-1', + platform_installation_id: 'installation-1', + } as never); + jest.mocked(getRepositoryDetails).mockResolvedValue({ + fullName: 'owner/repo', + cloneUrl: 'https://github.com/owner/repo.git', + htmlUrl: 'https://github.com/owner/repo', + isEmpty: true, + isPrivate: true, + }); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it.each([ + ['push_failed', 'push_failed'], + ['invalid_request', 'internal_error'], + ['token_failed', 'internal_error'], + ['internal_error', 'internal_error'], + ] as const)( + 'maps Worker %s to %s without logging its message', + async (workerError, publicError) => { + jest.mocked(appBuilderClient.migrateToGithub).mockResolvedValue({ + success: false, + error: workerError, + message: `push rejected ${sensitiveUrl}`, + }); + const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + + await expect(migrateProjectToGitHub(params)).resolves.toEqual({ + success: false, + error: publicError, + }); + + expect(errorSpy).toHaveBeenCalledWith(`Migration failed (${publicError}):`, { + source: 'app_builder_response', + workerError, + }); + expect(JSON.stringify(errorSpy.mock.calls)).not.toContain(sensitiveUrl); + } + ); + + it('logs a safe transport representation without the original error', async () => { + jest + .mocked(appBuilderClient.migrateToGithub) + .mockRejectedValue(new Error(`request failed for ${sensitiveUrl}`)); + const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + + await expect(migrateProjectToGitHub(params)).resolves.toEqual({ + success: false, + error: 'internal_error', + }); + + expect(errorSpy).toHaveBeenCalledWith('Migration failed (internal_error):', { + source: 'app_builder_request', + }); + expect(JSON.stringify(errorSpy.mock.calls)).not.toContain(sensitiveUrl); + }); +}); 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..9fda7aecd3 100644 --- a/apps/web/src/lib/app-builder/github-migration-service.ts +++ b/apps/web/src/lib/app-builder/github-migration-service.ts @@ -23,9 +23,12 @@ import type { class MigrationError extends Error { constructor( public readonly code: MigrateToGitHubErrorCode, - options?: ErrorOptions + public readonly logDetails?: { + source: 'github_repository_lookup' | 'app_builder_request' | 'app_builder_response'; + workerError?: 'invalid_request' | 'internal_error' | 'token_failed' | 'push_failed'; + } ) { - super(`Migration failed: ${code}`, options); + super(`Migration failed: ${code}`); this.name = 'MigrationError'; } } @@ -211,8 +214,8 @@ export async function migrateProjectToGitHub( try { repoDetails = await getRepositoryDetails(integration.platform_installation_id, repoFullName); - } catch (error) { - throw new MigrationError('internal_error', { cause: error }); + } catch { + throw new MigrationError('internal_error', { source: 'github_repository_lookup' }); } if (!repoDetails) { @@ -232,11 +235,17 @@ 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', + { + source: 'app_builder_response', + workerError: migrateResult.error, + } + ); } } catch (error) { if (error instanceof MigrationError) throw error; - throw new MigrationError('push_failed', { cause: error }); + throw new MigrationError('internal_error', { source: 'app_builder_request' }); } // 5. Update deployment if exists @@ -273,8 +282,8 @@ export async function migrateProjectToGitHub( .where(eq(app_builder_projects.id, projectId)); if (error instanceof MigrationError) { - if (error.cause) { - console.error(`Migration failed (${error.code}):`, error.cause); + if (error.logDetails) { + console.error(`Migration failed (${error.code}):`, error.logDetails); } return { success: false, error: error.code }; } From 35385ecbea867d515f551a215e5cd07f24f802a1 Mon Sep 17 00:00:00 2001 From: Evan Jacobson Date: Wed, 2 Sep 2026 10:32:45 -0600 Subject: [PATCH 3/4] fix(app-builder): isolate Kilo user ID validation --- .../components/app-builder/AppBuilderChat.tsx | 40 +-- .../components/app-builder/ProjectManager.ts | 122 +++------ .../components/app-builder/PromptInput.tsx | 3 - .../__tests__/ProjectManager.test.ts | 244 ------------------ .../__tests__/preview-polling.test.ts | 2 - .../project-manager/__tests__/store.test.ts | 3 - .../__tests__/streaming.test.ts | 1 - .../project-manager/sessions/session-store.ts | 2 - .../project-manager/sessions/v2/streaming.ts | 16 +- .../app-builder/project-manager/store.ts | 2 - .../app-builder/project-manager/types.ts | 4 - .../app-builder/app-builder-client.test.ts | 38 --- .../src/lib/app-builder/app-builder-client.ts | 41 +-- .../app-builder/app-builder-service.test.ts | 239 ----------------- .../lib/app-builder/app-builder-service.ts | 145 +++-------- .../github-migration-service.test.ts | 121 --------- .../app-builder/github-migration-service.ts | 25 +- services/app-builder/src/api-schemas.test.ts | 32 +++ services/app-builder/src/api-schemas.ts | 2 +- 19 files changed, 148 insertions(+), 934 deletions(-) delete mode 100644 apps/web/src/components/app-builder/project-manager/__tests__/ProjectManager.test.ts delete mode 100644 apps/web/src/lib/app-builder/app-builder-client.test.ts delete mode 100644 apps/web/src/lib/app-builder/app-builder-service.test.ts delete 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 8057886705..d77ba5a8be 100644 --- a/apps/web/src/components/app-builder/AppBuilderChat.tsx +++ b/apps/web/src/components/app-builder/AppBuilderChat.tsx @@ -508,15 +508,7 @@ function SessionMessages({ export function AppBuilderChat({ organizationId }: AppBuilderChatProps) { // Get state and manager from ProjectSession context const { manager, state } = useProject(); - const { - isStreaming, - isConnecting, - isInterrupting, - model: projectModel, - sessions, - pendingNewSession, - isRecoveringSession, - } = state; + const { isStreaming, isInterrupting, model: projectModel, sessions, pendingNewSession } = state; const messagesEndRef = useRef(null); const scrollContainerRef = useRef(null); @@ -540,14 +532,14 @@ export function AppBuilderChat({ organizationId }: AppBuilderChatProps) { setVisibleSessionCount(DEFAULT_VISIBLE_SESSIONS); }, [manager]); - // Clear the submit-count once the awaited new session has arrived, or once - // neither connection nor streaming is in progress after a failed request. + // Clear the submit-count once the awaited new session has arrived, or if + // the request failed (isStreaming drops back to false with no new session). useEffect(() => { if (sessionCountAtSubmit === null) return; - if (sessions.length > sessionCountAtSubmit || (!isStreaming && !isConnecting)) { + if (sessions.length > sessionCountAtSubmit || !isStreaming) { setSessionCountAtSubmit(null); } - }, [sessions.length, sessionCountAtSubmit, isStreaming, isConnecting]); + }, [sessions.length, sessionCountAtSubmit, isStreaming]); // Fetch eligibility to check if user can use App Builder const personalEligibilityQuery = useQuery({ @@ -699,7 +691,7 @@ export function AppBuilderChat({ organizationId }: AppBuilderChatProps) { // until the new session object actually arrives in state. setSessionCountAtSubmit(sessions.length); } - await manager.sendMessage(value, images, selectedModel || undefined); + manager.sendMessage(value, images, selectedModel || undefined); // PromptInput clears itself internally after successful submit setMessageUuid(uuidv4()); }, @@ -751,7 +743,7 @@ export function AppBuilderChat({ organizationId }: AppBuilderChatProps) { variant="ghost" size="icon" onClick={handleNewChatToggle} - disabled={isStreaming || isConnecting || isRecoveringSession} + disabled={isStreaming} className={pendingNewSession ? 'text-primary bg-primary/10 h-8 w-8' : 'h-8 w-8'} aria-label="New chat" > @@ -759,11 +751,7 @@ export function AppBuilderChat({ organizationId }: AppBuilderChatProps) { - {isRecoveringSession - ? 'A new chat is required' - : pendingNewSession - ? 'Cancel new chat' - : 'New chat'} + {pendingNewSession ? 'Cancel new chat' : 'New chat'} @@ -851,15 +839,13 @@ export function AppBuilderChat({ organizationId }: AppBuilderChatProps) { placeholder={ isStreaming ? 'Building...' - : isConnecting - ? 'Connecting...' - : pendingNewSession - ? 'What would you like to change?' - : 'Describe changes to your app...' + : pendingNewSession + ? 'What would you like to change?' + : 'Describe changes to your app...' } disabled={(!hasAnyMessages && !pendingNewSession) || isBlocked} - isSubmitting={isStreaming || isConnecting} - onInterrupt={isStreaming ? handleInterrupt : undefined} + isSubmitting={isStreaming} + onInterrupt={handleInterrupt} isInterrupting={isInterrupting} onImagesChange={handleImagesChange} models={modelOptions} diff --git a/apps/web/src/components/app-builder/ProjectManager.ts b/apps/web/src/components/app-builder/ProjectManager.ts index 424288df1a..1b2bb96284 100644 --- a/apps/web/src/components/app-builder/ProjectManager.ts +++ b/apps/web/src/components/app-builder/ProjectManager.ts @@ -41,7 +41,7 @@ export type ProjectManager = { destroyed: boolean; subscribe: (listener: () => void) => () => void; getState: () => ProjectState; - sendMessage: (message: string, images?: Images, model?: string) => Promise; + sendMessage: (message: string, images?: Images, model?: string) => void; interrupt: () => void; setCurrentIframeUrl: (url: string | null) => void; setGitRepoFullName: (repoFullName: string) => void; @@ -64,8 +64,7 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag let pendingInitialStreamingStart = false; let pendingReconnect = false; let hasStartedInitialStreaming = false; - const sessionUnsubscribes = new Map void>(); - let sessionCreationOperation = 0; + let sessionUnsubscribes: Array<() => void> = []; const initialState = createInitialState( project.deployment_id ?? null, @@ -102,24 +101,17 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag } function getActiveSession(): AppBuilderSession | undefined { - if (!cloudAgentSessionId) return undefined; - return store - .getState() - .sessions.find(session => session.info.cloud_agent_session_id === cloudAgentSessionId); + const sessions = store.getState().sessions; + return sessions[sessions.length - 1]; } function subscribeToSession(session: AppBuilderSession): void { const unsubscribe = session.subscribe(() => { - if (destroyed || getActiveSession() !== session) return; - const { isStreaming, isConnecting } = session.getState(); - store.setState({ isStreaming, isConnecting }); + const active = getActiveSession(); + const isStreaming = active?.getState().isStreaming ?? false; + store.setState({ isStreaming }); }); - sessionUnsubscribes.set(session, unsubscribe); - } - - function unsubscribeFromSession(session: AppBuilderSession): void { - sessionUnsubscribes.get(session)?.(); - sessionUnsubscribes.delete(session); + sessionUnsubscribes.push(unsubscribe); } /** @@ -131,17 +123,12 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag const sessionInfos = proj.sessions; if (sessionInfos.length === 0) return []; - 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 activeInfo = + sessionInfos.find(s => s.ended_at === null) ?? sessionInfos[sessionInfos.length - 1]; const sessions: AppBuilderSession[] = []; - for (const info of orderedSessionInfos) { + for (const info of sessionInfos) { const isActive = info.id === activeInfo?.id; if (!isActive) { @@ -186,7 +173,6 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag const currentActive = getActiveSession(); if (currentActive) { currentActive.info.ended_at = new Date().toISOString(); - unsubscribeFromSession(currentActive); currentActive.destroy(); } @@ -212,9 +198,7 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag const currentSessions = store.getState().sessions; store.setState({ sessions: [...currentSessions, newSession], - isStreaming: false, - isConnecting: true, - isRecoveringSession: false, + isStreaming: true, }); cloudAgentSessionId = newSessionId; @@ -275,25 +259,15 @@ 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.session_id - ? project.sessions.find(s => s.cloud_agent_session_id === project.session_id) - : undefined; + const activeProjectSessionInfo = + project.sessions.find(s => s.ended_at === null) ?? + project.sessions[project.sessions.length - 1]; - if (activeProjectSessionInfo?.prepared === true && activeProjectSessionInfo.initiated === false) { + if (activeProjectSessionInfo?.initiated === false) { pendingInitialStreamingStart = true; - } else if ( - cloudAgentSessionId && - activeProjectSessionInfo && - activeProjectSessionInfo.prepared !== false - ) { + } else if (cloudAgentSessionId) { pendingReconnect = true; - if (activeProjectSessionInfo.worker_version === 'v2') { - store.setState({ isConnecting: true }); - } } else { - if (cloudAgentSessionId) { - store.setState({ pendingNewSession: true, isRecoveringSession: true }); - } startPreviewPollingIfNeeded(); } @@ -329,15 +303,16 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag return store.getState(); } - async function sendMessage(message: string, images?: Images, model?: string): Promise { + function sendMessage(message: string, images?: Images, model?: string): void { if (store.getState().pendingNewSession) { - return sendMessageAsNewSession(message, images, model); + sendMessageAsNewSession(message, images, model); + return; } const activeSession = getActiveSession(); if (!activeSession) { logger.logWarn('Cannot send message: no active session'); - throw new Error('Cannot send message: no active session'); + return; } if (model) { @@ -345,7 +320,7 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag } const effectiveModel = model ?? store.getState().model; - return activeSession.sendMessage(message, images, effectiveModel); + void activeSession.sendMessage(message, images, effectiveModel); } /** @@ -353,14 +328,10 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag * Calls sendMessage tRPC mutation with forceNewSession:true, then delegates * to handleSessionChanged to create the new session object and begin streaming. */ - async function sendMessageAsNewSession( - message: string, - images?: Images, - model?: string - ): Promise { + function sendMessageAsNewSession(message: string, images?: Images, model?: string): void { if (destroyed) { logger.logWarn('Cannot start new session: ProjectManager is destroyed'); - throw new Error('Cannot start new session: ProjectManager is destroyed'); + return; } if (model) { @@ -368,10 +339,8 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag } const effectiveModel = model ?? store.getState().model; - const isRecoveringSession = store.getState().isRecoveringSession; - const operation = ++sessionCreationOperation; - store.setState({ pendingNewSession: false, isStreaming: false, isConnecting: true }); + store.setState({ pendingNewSession: false, isStreaming: true }); const mutationPromise = organizationId ? trpcClient.organizations.appBuilder.sendMessage.mutate({ @@ -390,25 +359,19 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag forceNewSession: true, }); - try { - const result = await mutationPromise; - if (destroyed || operation !== sessionCreationOperation) return; - handleSessionChanged(result.cloudAgentSessionId, { - text: message, - images, - }); - } catch (err) { - if (!destroyed && operation === sessionCreationOperation) { - logger.logError('Failed to start new session', err); - store.setState({ - pendingNewSession: true, - isRecoveringSession, - isStreaming: false, - isConnecting: false, + void mutationPromise + .then(result => { + if (destroyed) return; + handleSessionChanged(result.cloudAgentSessionId, { + text: message, + images, }); - } - throw err; - } + }) + .catch((err: Error) => { + if (destroyed) return; + logger.logError('Failed to start new session', err); + store.setState({ isStreaming: false }); + }); } function interrupt(): void { @@ -460,29 +423,26 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag if (currentActive) { currentActive.info.ended_at = new Date().toISOString(); } - store.setState({ pendingNewSession: true, isRecoveringSession: false }); + store.setState({ pendingNewSession: true }); } function cancelNewSession(): void { if (destroyed) return; - if (store.getState().isRecoveringSession) return; const currentActive = getActiveSession(); if (currentActive) { currentActive.info.ended_at = null; } - store.setState({ pendingNewSession: false, isRecoveringSession: false }); + store.setState({ pendingNewSession: false }); } function destroy(): void { if (destroyed) return; destroyed = true; - sessionCreationOperation++; - - for (const unsub of sessionUnsubscribes.values()) { + for (const unsub of sessionUnsubscribes) { unsub(); } - sessionUnsubscribes.clear(); + sessionUnsubscribes = []; for (const session of store.getState().sessions) { session.destroy(); diff --git a/apps/web/src/components/app-builder/PromptInput.tsx b/apps/web/src/components/app-builder/PromptInput.tsx index 1094517650..a5c4d9e6d4 100644 --- a/apps/web/src/components/app-builder/PromptInput.tsx +++ b/apps/web/src/components/app-builder/PromptInput.tsx @@ -117,7 +117,6 @@ const BottomBar = memo(function BottomBar({ onValueChange={onModelChange} isLoading={isLoadingModels} error={modelsError} - disabled={disabled || isSubmitting} placeholder="Model" className="w-56 bg-zinc-800 text-zinc-400 hover:bg-zinc-700 hover:text-zinc-200" /> @@ -163,7 +162,6 @@ const BottomBar = memo(function BottomBar({ onClick={onInterrupt} disabled={isInterrupting} className="h-9 w-9" - aria-label="Stop building" > @@ -175,7 +173,6 @@ const BottomBar = memo(function BottomBar({ onClick={onSubmit} disabled={isSubmitDisabled} className="h-9 w-9" - aria-label="Send message" > 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 deleted file mode 100644 index 0fdbf4bc56..0000000000 --- a/apps/web/src/components/app-builder/project-manager/__tests__/ProjectManager.test.ts +++ /dev/null @@ -1,244 +0,0 @@ -import type { StoredMessage } from '@/components/cloud-agent-next/types'; -import type { ProjectWithMessages } from '@/lib/app-builder/types'; -import { createProjectManager, type ProjectManagerConfig } from '../../ProjectManager'; -import { createSessionStore, type SessionStore } from '../sessions/session-store'; -import { createV1Session } from '../sessions/v1/v1-session'; -import { createV2Session, type CreateV2SessionConfig } from '../sessions/v2/v2-session'; -import type { V2Session } from '../sessions/types'; - -jest.mock('../preview-polling', () => ({ - startPreviewPolling: jest.fn(() => ({ isPolling: true, stop: jest.fn() })), -})); -jest.mock('../deployments', () => ({ deploy: jest.fn() })); -jest.mock('../sessions/v1/v1-session', () => ({ createV1Session: jest.fn() })); -jest.mock('../sessions/v2/v2-session', () => ({ createV2Session: jest.fn() })); - -type TestSession = V2Session & { - store: SessionStore; - capturedListeners: Array<() => void>; -}; - -const createdSessions: TestSession[] = []; -const mockCreateV1Session = jest.mocked(createV1Session); -const mockCreateV2Session = jest.mocked(createV2Session); - -function createTestSession(config: CreateV2SessionConfig): TestSession { - const store = createSessionStore(config.initialMessages); - const capturedListeners: Array<() => void> = []; - const session: TestSession = { - type: 'v2', - info: config.info, - getState: store.getState, - subscribe: listener => { - capturedListeners.push(listener); - return store.subscribe(listener); - }, - getChildSessionMessages: store.getChildSessionMessages, - sendMessage: jest.fn(async () => {}), - interrupt: jest.fn(async () => {}), - startInitialStreaming: jest.fn(), - connectToExistingSession: jest.fn(() => store.setState({ isConnecting: true })), - loadMessages: jest.fn(), - destroy: jest.fn(), - store, - capturedListeners, - }; - createdSessions.push(session); - return session; -} - -function createProject(): ProjectWithMessages { - return { - id: 'project-1', - session_id: 'session-old', - deployment_id: null, - model_id: null, - git_repo_full_name: null, - messages: [], - sessions: [ - { - id: 'session-old', - cloud_agent_session_id: 'session-old', - worker_version: 'v2', - ended_at: null, - title: null, - initiated: true, - prepared: true, - }, - ], - } as unknown as ProjectWithMessages; -} - -function createTrpcClient(sendMessage: jest.Mock = jest.fn()) { - return { - appBuilder: { - sendMessage: { mutate: sendMessage }, - interruptSession: { mutate: jest.fn(async () => ({ success: true })) }, - }, - organizations: { - appBuilder: { - sendMessage: { mutate: jest.fn() }, - interruptSession: { mutate: jest.fn(async () => ({ success: true })) }, - }, - }, - }; -} - -function createManager( - sendMessage: jest.Mock = jest.fn(), - project: ProjectWithMessages = createProject() -) { - const trpcClient = createTrpcClient(sendMessage); - const manager = createProjectManager({ - project, - organizationId: null, - trpcClient: trpcClient as unknown as ProjectManagerConfig['trpcClient'], - }); - return { manager, trpcClient }; -} - -function flushNotifications(): Promise { - return new Promise(resolve => setTimeout(resolve, 0)); -} - -describe('ProjectManager session handoff', () => { - beforeEach(() => { - createdSessions.length = 0; - jest.clearAllMocks(); - mockCreateV1Session.mockImplementation(() => { - throw new Error('Unexpected V1 session'); - }); - mockCreateV2Session.mockImplementation(createTestSession); - }); - - it('uses a connecting guard without claiming an existing idle session is streaming', async () => { - const { manager } = createManager(); - const activeSession = createdSessions[0]; - - expect(manager.getState()).toMatchObject({ isConnecting: true, isStreaming: false }); - - const unsubscribe = manager.subscribe(jest.fn()); - await Promise.resolve(); - - expect(activeSession.connectToExistingSession).toHaveBeenCalledWith('session-old'); - expect(manager.getState()).toMatchObject({ isConnecting: true, isStreaming: false }); - - activeSession.store.setState({ isConnecting: false, isStreaming: false }); - await flushNotifications(); - - expect(manager.getState()).toMatchObject({ isConnecting: false, isStreaming: false }); - unsubscribe(); - manager.destroy(); - }); - - it('ignores a queued notification owned by the destroyed session after handoff', async () => { - const sendMessage = jest.fn(async () => ({ cloudAgentSessionId: 'session-new' })); - const { manager } = createManager(sendMessage); - const oldSession = createdSessions[0]; - - manager.requestNewSession(); - await manager.sendMessage('continue building'); - - const newSession = createdSessions[1]; - expect(oldSession.destroy).toHaveBeenCalledTimes(1); - expect(manager.getState()).toMatchObject({ isConnecting: true, isStreaming: false }); - - newSession.store.setState({ isConnecting: false, isStreaming: true }); - await flushNotifications(); - expect(manager.getState().isStreaming).toBe(true); - - newSession.store.setState({ isStreaming: false }); - oldSession.capturedListeners[0]?.(); - - expect(manager.getState().isStreaming).toBe(true); - await flushNotifications(); - expect(manager.getState().isStreaming).toBe(false); - manager.destroy(); - }); - - it('propagates replacement mutation failure and clears the non-interruptible guard', async () => { - const error = new Error('replacement failed'); - const { manager } = createManager(jest.fn(async () => Promise.reject(error))); - - manager.requestNewSession(); - - await expect(manager.sendMessage('retryable prompt')).rejects.toBe(error); - expect(manager.getState()).toMatchObject({ - pendingNewSession: true, - isRecoveringSession: false, - isConnecting: false, - isStreaming: false, - }); - manager.destroy(); - }); - - it('reconnects the canonical session instead of a newer orphan row', async () => { - const project = createProject(); - project.sessions = [ - { ...project.sessions[0]!, ended_at: '2026-08-31T00:00:00.000Z' }, - { - ...project.sessions[0]!, - id: 'orphan-row', - cloud_agent_session_id: 'orphan-session', - }, - ]; - const { manager } = createManager(jest.fn(), project); - - manager.subscribe(jest.fn()); - await Promise.resolve(); - - expect(createdSessions[0]?.info.cloud_agent_session_id).toBe('orphan-session'); - expect(createdSessions[0]?.connectToExistingSession).not.toHaveBeenCalled(); - expect(createdSessions[1]?.info.cloud_agent_session_id).toBe('session-old'); - expect(createdSessions[1]?.connectToExistingSession).toHaveBeenCalledWith('session-old'); - manager.destroy(); - }); - - it('requires a replacement when the canonical session is missing and preserves recovery on failure', async () => { - const project = createProject(); - project.sessions = [ - { - ...project.sessions[0]!, - id: 'orphan-row', - cloud_agent_session_id: 'orphan-session', - }, - ]; - const error = new Error('replacement failed'); - const { manager } = createManager( - jest.fn(async () => Promise.reject(error)), - project - ); - - expect(manager.getState()).toMatchObject({ - pendingNewSession: true, - isRecoveringSession: true, - isConnecting: false, - }); - manager.cancelNewSession(); - expect(manager.getState().pendingNewSession).toBe(true); - - await expect(manager.sendMessage('recover this project')).rejects.toBe(error); - expect(manager.getState()).toMatchObject({ - pendingNewSession: true, - isRecoveringSession: true, - isConnecting: false, - }); - manager.destroy(); - }); - - it('does not apply replacement completion after the manager is destroyed', async () => { - let resolveMutation: ((result: { cloudAgentSessionId: string }) => void) | undefined; - const mutation = new Promise<{ cloudAgentSessionId: string }>(resolve => { - resolveMutation = resolve; - }); - const { manager } = createManager(jest.fn(() => mutation)); - - manager.requestNewSession(); - const submission = manager.sendMessage('late prompt'); - manager.destroy(); - resolveMutation?.({ cloudAgentSessionId: 'session-late' }); - await submission; - - expect(createdSessions).toHaveLength(1); - }); -}); 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 6168079a17..c13c7cebb7 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 @@ -12,7 +12,6 @@ function createMockStore(): ProjectStore & { stateUpdates: Array> = []; let currentState: ProjectState = { isStreaming: false, - isConnecting: false, isInterrupting: false, previewUrl: null, previewStatus: 'idle', @@ -22,7 +21,6 @@ function createMockStore(): ProjectStore & { stateUpdates: Array { const state = createInitialState(null, null, null); expect(state.isStreaming).toBe(false); - expect(state.isConnecting).toBe(false); expect(state.previewUrl).toBeNull(); expect(state.previewStatus).toBe('idle'); expect(state.deploymentId).toBeNull(); @@ -53,7 +52,6 @@ describe('createInitialState', () => { describe('createProjectStore', () => { const initialState: ProjectState = { isStreaming: false, - isConnecting: false, isInterrupting: false, previewUrl: null, previewStatus: 'idle', @@ -63,7 +61,6 @@ describe('createProjectStore', () => { gitRepoFullName: null, sessions: [], pendingNewSession: false, - isRecoveringSession: false, }; describe('getState', () => { diff --git a/apps/web/src/components/app-builder/project-manager/__tests__/streaming.test.ts b/apps/web/src/components/app-builder/project-manager/__tests__/streaming.test.ts index 45ec7e176b..9cf4e08fc0 100644 --- a/apps/web/src/components/app-builder/project-manager/__tests__/streaming.test.ts +++ b/apps/web/src/components/app-builder/project-manager/__tests__/streaming.test.ts @@ -31,7 +31,6 @@ function createMockStore(): { getState: () => ({ messages, isStreaming, - isConnecting: false, questionRequestIds: new Map(), childSessionMessages, }), diff --git a/apps/web/src/components/app-builder/project-manager/sessions/session-store.ts b/apps/web/src/components/app-builder/project-manager/sessions/session-store.ts index 6620388636..fff05dacd3 100644 --- a/apps/web/src/components/app-builder/project-manager/sessions/session-store.ts +++ b/apps/web/src/components/app-builder/project-manager/sessions/session-store.ts @@ -10,7 +10,6 @@ export type SessionState = { messages: TMessage[]; isStreaming: boolean; - isConnecting: boolean; questionRequestIds: Map; /** Messages from child/subagent sessions, keyed by child session ID */ childSessionMessages: Map; @@ -33,7 +32,6 @@ export function createSessionStore(initialMessages: TMessage[]): Sessi let state: SessionState = { messages: initialMessages, isStreaming: false, - isConnecting: false, questionRequestIds: new Map(), childSessionMessages: new Map(), }; 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 6ad95732ae..7785b2fe89 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 @@ -177,10 +177,10 @@ export function createV2StreamingCoordinator(config: V2StreamingConfig): V2Strea onSessionStatusChanged: status => { if (status.type === 'idle') { - store.setState({ isStreaming: false, isConnecting: false }); + store.setState({ isStreaming: false }); onStreamComplete?.(); } else if (status.type === 'busy') { - store.setState({ isStreaming: true, isConnecting: false }); + store.setState({ isStreaming: true }); } }, @@ -194,11 +194,11 @@ export function createV2StreamingCoordinator(config: V2StreamingConfig): V2Strea onError: (error, _sessionId) => { logger.logError('V2 EventProcessor error', new Error(error)); - store.setState({ isStreaming: false, isConnecting: false }); + store.setState({ isStreaming: false }); }, onStreamingChanged: streaming => { - store.setState({ isStreaming: streaming, isConnecting: false }); + store.setState({ isStreaming: streaming }); // onStreamComplete is called from onSessionStatusChanged (idle) — not here, // to avoid triggering preview polling twice per stream completion. }, @@ -232,7 +232,7 @@ export function createV2StreamingCoordinator(config: V2StreamingConfig): V2Strea if (state.status === 'error' || state.status === 'disconnected') { // Force-complete all in-flight messages so they don't appear stuck in streaming state processor?.forceCompleteAll(); - store.setState({ isStreaming: false, isConnecting: false }); + store.setState({ isStreaming: false }); if (state.status === 'disconnected') { onStreamComplete?.(); } @@ -469,14 +469,14 @@ export function createV2StreamingCoordinator(config: V2StreamingConfig): V2Strea } logger.log('Connecting to existing V2 session', { sessionId }); - store.setState({ isConnecting: true }); + store.setState({ isStreaming: true }); void (async () => { try { await connectWs(sessionId); } catch (err) { logger.logError('Failed to connect to existing V2 session', err); - store.setState({ isStreaming: false, isConnecting: false }); + store.setState({ isStreaming: false }); } })(); } @@ -503,7 +503,7 @@ export function createV2StreamingCoordinator(config: V2StreamingConfig): V2Strea // Force-complete all in-flight messages so they don't appear stuck in streaming state processor?.forceCompleteAll(); - store.setState({ isStreaming: false, isConnecting: false }); + store.setState({ isStreaming: false }); } /** 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 7f0757cfb4..c41cef4393 100644 --- a/apps/web/src/components/app-builder/project-manager/store.ts +++ b/apps/web/src/components/app-builder/project-manager/store.ts @@ -23,7 +23,6 @@ export function createInitialState( ): ProjectState { return { isStreaming: false, - isConnecting: false, isInterrupting: false, previewUrl: null, previewStatus: 'idle', @@ -33,7 +32,6 @@ 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 8ce3caf9d4..870bb02d2a 100644 --- a/apps/web/src/components/app-builder/project-manager/types.ts +++ b/apps/web/src/components/app-builder/project-manager/types.ts @@ -16,8 +16,6 @@ export type PreviewStatus = 'idle' | 'building' | 'running' | 'error'; export type ProjectState = { /** Derived from active session — true if active session is streaming */ isStreaming: boolean; - /** True while the active session is connecting and its streaming status is unknown */ - isConnecting: boolean; isInterrupting: boolean; previewUrl: string | null; previewStatus: PreviewStatus; @@ -31,8 +29,6 @@ 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 deleted file mode 100644 index 1aa8baa861..0000000000 --- a/apps/web/src/lib/app-builder/app-builder-client.test.ts +++ /dev/null @@ -1,38 +0,0 @@ -jest.mock('@/lib/config.server', () => ({ - APP_BUILDER_URL: 'https://app-builder.example.com', - APP_BUILDER_AUTH_TOKEN: 'worker-auth-token', -})); - -import { AppBuilderError, migrateToGithub } from './app-builder-client'; - -const request = { - githubRepo: 'owner/repo', - userId: '00000000-0000-4000-8000-000000000000', -}; - -describe('migrateToGithub', () => { - afterEach(() => { - jest.restoreAllMocks(); - }); - - it('returns a typed failure envelope for a non-2xx response', async () => { - const failure = { - success: false as const, - error: 'push_failed' as const, - message: 'push rejected https://oauth2:secret-token@github.com/owner/repo.git', - }; - jest.spyOn(global, 'fetch').mockResolvedValue(Response.json(failure, { status: 500 })); - - await expect(migrateToGithub('project-1', request)).resolves.toEqual(failure); - }); - - it('does not attach an untrusted response body to parsing errors', async () => { - const sensitiveBody = 'https://oauth2:secret-token@github.com/owner/repo.git'; - jest.spyOn(global, 'fetch').mockResolvedValue(new Response(sensitiveBody, { status: 502 })); - - const error = await migrateToGithub('project-1', request).catch(value => value); - - expect(error).toBeInstanceOf(AppBuilderError); - expect(String(error)).not.toContain(sensitiveBody); - }); -}); 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 d990e65c7d..7042393af3 100644 --- a/apps/web/src/lib/app-builder/app-builder-client.ts +++ b/apps/web/src/lib/app-builder/app-builder-client.ts @@ -285,39 +285,24 @@ export async function migrateToGithub( const baseUrl = getBaseUrl(); const endpoint = `${baseUrl}/apps/${encodeURIComponent(projectId)}/migrate-to-github`; - let response: Response; - try { - response = await fetch(endpoint, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - ...(APP_BUILDER_AUTH_TOKEN && { Authorization: `Bearer ${APP_BUILDER_AUTH_TOKEN}` }), - }, - body: JSON.stringify(config), - }); - } catch { - throw new AppBuilderError('App Builder GitHub migration request failed', undefined, endpoint); - } - - let data: unknown; - try { - data = await response.json(); - } catch { - throw new AppBuilderError( - 'App Builder GitHub migration returned an invalid response', - response.status, - endpoint - ); - } + const response = await fetch(endpoint, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + ...(APP_BUILDER_AUTH_TOKEN && { Authorization: `Bearer ${APP_BUILDER_AUTH_TOKEN}` }), + }, + body: JSON.stringify(config), + }); - const parsed = MigrateToGithubResponseSchema.safeParse(data); - if (!parsed.success || (!response.ok && parsed.data.success)) { + if (!response.ok) { + const errorText = await response.text().catch(() => 'Unknown error'); throw new AppBuilderError( - 'App Builder GitHub migration returned an invalid response', + `Failed to migrate project ${projectId} to GitHub: ${response.status} ${response.statusText} - ${errorText}`, response.status, endpoint ); } - return parsed.data; + const data = await response.json(); + return MigrateToGithubResponseSchema.parse(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 deleted file mode 100644 index 74a3db4847..0000000000 --- a/apps/web/src/lib/app-builder/app-builder-service.test.ts +++ /dev/null @@ -1,239 +0,0 @@ -import { beforeAll, beforeEach, describe, expect, it, jest } from '@jest/globals'; -import { and, eq, isNull } from 'drizzle-orm'; -import { cleanupDbForTest, db, pool } from '@/lib/drizzle'; -import { - app_builder_projects, - app_builder_project_sessions, - AppBuilderSessionReason, -} from '@kilocode/db/schema'; -import { insertTestUser } from '@/tests/helpers/user.helper'; -import type * as serviceModule from './app-builder-service'; - -const mockPrepareSession = - jest.fn<(...args: unknown[]) => Promise<{ cloudAgentSessionId: string }>>(); -const mockInitiateFromPreparedSession = jest.fn< - (input: { cloudAgentSessionId: string }) => Promise<{ - cloudAgentSessionId: string; - executionId: string; - status: 'started'; - streamUrl: string; - messageId: string; - delivery: 'sent'; - }> ->(); -const mockInterruptSession = jest.fn<(...args: unknown[]) => Promise<{ success: boolean }>>(); -const mockCleanupSession = jest.fn<(...args: unknown[]) => Promise<{ success: boolean }>>(); -const mockGetSession = jest.fn<(...args: unknown[]) => Promise>>(); - -jest.mock('@/lib/cloud-agent-next/cloud-agent-client', () => ({ - createAppBuilderCloudAgentNextClient: () => ({ - prepareSession: mockPrepareSession, - initiateFromPreparedSession: mockInitiateFromPreparedSession, - interruptSession: mockInterruptSession, - cleanupSession: mockCleanupSession, - getSession: mockGetSession, - }), -})); - -let getProject: typeof serviceModule.getProject; -let sendMessage: typeof serviceModule.sendMessage; - -beforeAll(async () => { - ({ getProject, sendMessage } = await import('./app-builder-service')); -}); - -beforeEach(async () => { - await cleanupDbForTest(); - jest.clearAllMocks(); - mockInterruptSession.mockResolvedValue({ success: true }); - mockCleanupSession.mockResolvedValue({ success: true }); - mockGetSession.mockImplementation(async sessionId => ({ - sessionId, - userId: 'test-user', - execution: null, - preparedAt: Date.now(), - initiatedAt: Date.now(), - timestamp: Date.now(), - version: 1, - })); - mockInitiateFromPreparedSession.mockImplementation(async ({ cloudAgentSessionId }) => ({ - cloudAgentSessionId, - executionId: `execution-${cloudAgentSessionId}`, - status: 'started', - streamUrl: `/sessions/${cloudAgentSessionId}/stream`, - messageId: `message-${cloudAgentSessionId}`, - delivery: 'sent', - })); -}); - -async function createProjectFixture() { - const user = await insertTestUser(); - const [project] = await db - .insert(app_builder_projects) - .values({ - created_by_user_id: user.id, - owned_by_user_id: user.id, - session_id: 'session-a', - title: 'Snapshot project', - model_id: 'test-model', - git_repo_full_name: 'kilo/snapshot-project', - }) - .returning(); - - await db.insert(app_builder_project_sessions).values({ - project_id: project.id, - cloud_agent_session_id: 'session-a', - reason: AppBuilderSessionReason.Initial, - worker_version: 'v2', - }); - - return { project, owner: { type: 'user' as const, id: user.id } }; -} - -async function waitForProjectSessionReadBlockedBy(lockingPid: number): Promise { - const deadline = Date.now() + 5_000; - while (Date.now() < deadline) { - const result = await pool.query<{ waiting: boolean }>( - `SELECT EXISTS ( - SELECT 1 - FROM pg_locks - WHERE relation = 'app_builder_project_sessions'::regclass - AND NOT granted - AND $1::integer = ANY(pg_blocking_pids(pid)) - ) AS waiting`, - [lockingPid] - ); - if (result.rows[0]?.waiting) return; - await new Promise(resolve => setTimeout(resolve, 10)); - } - throw new Error('App Builder project read did not block on expected table lock'); -} - -describe('App Builder session consistency', () => { - it('marks a definitively missing canonical session as unprepared for recovery', async () => { - const { project, owner } = await createProjectFixture(); - mockGetSession.mockRejectedValueOnce({ - shape: { data: { code: 'NOT_FOUND', httpStatus: 404 } }, - }); - - const result = await getProject(project.id, owner, 'test-token'); - - expect(result.sessions).toEqual([ - expect.objectContaining({ - cloud_agent_session_id: 'session-a', - initiated: null, - prepared: false, - }), - ]); - }); - - it('allows only one concurrent new-session request to claim the canonical pointer', async () => { - const { project, owner } = await createProjectFixture(); - const bothPrepared = Promise.withResolvers(); - - mockPrepareSession.mockImplementation(async () => { - const callNumber = mockPrepareSession.mock.calls.length; - if (callNumber === 2) bothPrepared.resolve(); - await bothPrepared.promise; - return { cloudAgentSessionId: `session-${callNumber}` }; - }); - - const results = await Promise.allSettled([ - sendMessage({ - projectId: project.id, - owner, - message: 'first concurrent message', - authToken: 'test-token', - forceNewSession: true, - }), - sendMessage({ - projectId: project.id, - owner, - message: 'second concurrent message', - authToken: 'test-token', - forceNewSession: true, - }), - ]); - - const fulfilled = results.filter(result => result.status === 'fulfilled'); - const rejected = results.filter(result => result.status === 'rejected'); - expect(fulfilled).toHaveLength(1); - expect(rejected).toHaveLength(1); - expect(rejected[0]?.reason).toMatchObject({ code: 'CONFLICT' }); - - const [persistedProject] = await db - .select({ sessionId: app_builder_projects.session_id }) - .from(app_builder_projects) - .where(eq(app_builder_projects.id, project.id)); - const activeSessions = await db - .select({ sessionId: app_builder_project_sessions.cloud_agent_session_id }) - .from(app_builder_project_sessions) - .where( - and( - eq(app_builder_project_sessions.project_id, project.id), - isNull(app_builder_project_sessions.ended_at) - ) - ); - - expect(activeSessions).toEqual([{ sessionId: persistedProject.sessionId }]); - expect(['session-1', 'session-2']).toContain(persistedProject.sessionId); - - const losingSessionId = persistedProject.sessionId === 'session-1' ? 'session-2' : 'session-1'; - expect(mockInterruptSession).toHaveBeenCalledWith(losingSessionId); - expect(mockCleanupSession).toHaveBeenCalledWith(losingSessionId); - - const losingRows = await db - .select() - .from(app_builder_project_sessions) - .where(eq(app_builder_project_sessions.cloud_agent_session_id, losingSessionId)); - expect(losingRows).toHaveLength(0); - }); - - it('returns a coherent project and session snapshot across a concurrent rollover', async () => { - const { project, owner } = await createProjectFixture(); - const lockClient = await pool.connect(); - let projectPromise: ReturnType | null = null; - - try { - await lockClient.query('BEGIN'); - await lockClient.query('LOCK TABLE app_builder_project_sessions IN ACCESS EXCLUSIVE MODE'); - const pidResult = await lockClient.query<{ pid: number }>('SELECT pg_backend_pid() AS pid'); - const lockingPid = pidResult.rows[0]?.pid; - if (lockingPid === undefined) throw new Error('Could not determine locking database process'); - - projectPromise = getProject(project.id, owner, 'test-token'); - await waitForProjectSessionReadBlockedBy(lockingPid); - - await lockClient.query( - 'UPDATE app_builder_project_sessions SET ended_at = now() WHERE project_id = $1 AND cloud_agent_session_id = $2', - [project.id, 'session-a'] - ); - await lockClient.query( - `INSERT INTO app_builder_project_sessions - (project_id, cloud_agent_session_id, reason, worker_version) - VALUES ($1, $2, $3, 'v2')`, - [project.id, 'session-b', AppBuilderSessionReason.UserInitiated] - ); - await lockClient.query('UPDATE app_builder_projects SET session_id = $2 WHERE id = $1', [ - project.id, - 'session-b', - ]); - await lockClient.query('COMMIT'); - - const result = await projectPromise; - expect(result.session_id).toBe('session-b'); - expect(result.sessions).toEqual([ - expect.objectContaining({ - cloud_agent_session_id: 'session-a', - ended_at: expect.any(String), - }), - expect.objectContaining({ cloud_agent_session_id: 'session-b', ended_at: null }), - ]); - expect(mockGetSession).toHaveBeenCalledWith('session-b'); - } finally { - await lockClient.query('ROLLBACK').catch(() => undefined); - lockClient.release(); - if (projectPromise) await projectPromise.catch(() => undefined); - } - }, 20_000); -}); 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 25082f5436..779d4d2d56 100644 --- a/apps/web/src/lib/app-builder/app-builder-service.ts +++ b/apps/web/src/lib/app-builder/app-builder-service.ts @@ -72,19 +72,6 @@ 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. */ @@ -102,35 +89,20 @@ function parseWorkerVersion(value: string | null): WorkerVersion | null { } /** - * Fetch the project and all of its sessions in one statement so the canonical - * session pointer and session history come from the same PostgreSQL snapshot. + * Fetch all sessions for a project, ordered by created_at ascending. */ -async function getProjectSnapshot( - projectId: string, - owner: Owner -): Promise<{ project: AppBuilderProject; sessions: ProjectSessionInfo[] }> { - const ownerCondition = - owner.type === 'org' - ? eq(app_builder_projects.owned_by_organization_id, owner.id) - : eq(app_builder_projects.owned_by_user_id, owner.id); - +async function getProjectSessions(projectId: string): Promise { const rows = await db .select({ - project: app_builder_projects, - session: { - id: app_builder_project_sessions.id, - cloud_agent_session_id: app_builder_project_sessions.cloud_agent_session_id, - worker_version: app_builder_project_sessions.worker_version, - ended_at: app_builder_project_sessions.ended_at, - v1_title: cliSessions.title, - v2_title: cli_sessions_v2.title, - }, + id: app_builder_project_sessions.id, + cloud_agent_session_id: app_builder_project_sessions.cloud_agent_session_id, + worker_version: app_builder_project_sessions.worker_version, + created_at: app_builder_project_sessions.created_at, + ended_at: app_builder_project_sessions.ended_at, + v1_title: cliSessions.title, + v2_title: cli_sessions_v2.title, }) - .from(app_builder_projects) - .leftJoin( - app_builder_project_sessions, - eq(app_builder_projects.id, app_builder_project_sessions.project_id) - ) + .from(app_builder_project_sessions) .leftJoin( cliSessions, eq(app_builder_project_sessions.cloud_agent_session_id, cliSessions.cloud_agent_session_id) @@ -142,34 +114,18 @@ async function getProjectSnapshot( cli_sessions_v2.cloud_agent_session_id ) ) - .where(and(eq(app_builder_projects.id, projectId), ownerCondition)) + .where(eq(app_builder_project_sessions.project_id, projectId)) .orderBy(asc(app_builder_project_sessions.created_at)); - const project = rows[0]?.project; - if (!project) { - throw new TRPCError({ - code: 'NOT_FOUND', - message: 'Project not found', - }); - } - - const sessions = rows.flatMap(({ session }) => { - if (!session.id || !session.cloud_agent_session_id || !session.worker_version) return []; - - return [ - { - id: session.id, - cloud_agent_session_id: session.cloud_agent_session_id, - worker_version: parseWorkerVersion(session.worker_version) ?? 'v1', - ended_at: session.ended_at, - title: session.v1_title ?? session.v2_title ?? null, - initiated: null, - prepared: null, - } satisfies ProjectSessionInfo, - ]; - }); - - return { project, sessions }; + return rows.map(row => ({ + id: row.id, + cloud_agent_session_id: row.cloud_agent_session_id, + worker_version: parseWorkerVersion(row.worker_version) ?? 'v1', + ended_at: row.ended_at, + title: row.v1_title ?? row.v2_title ?? null, + initiated: null, + prepared: null, + })); } /** @@ -350,29 +306,16 @@ async function createCloudAgentNextSession( cloudAgentSessionId: newSessionId, }); - const claimedSession = await db.transaction(async tx => { - const [claimedProject] = await tx - .update(app_builder_projects) - .set({ session_id: newSessionId }) - .where( - and( - eq(app_builder_projects.id, projectId), - eq(app_builder_projects.session_id, currentSessionId) - ) - ) - .returning({ id: app_builder_projects.id }); - - if (!claimedProject) return false; - + await db.transaction(async tx => { await tx .update(app_builder_project_sessions) .set({ ended_at: sql`now()` }) - .where( - and( - eq(app_builder_project_sessions.project_id, projectId), - eq(app_builder_project_sessions.cloud_agent_session_id, currentSessionId) - ) - ); + .where(eq(app_builder_project_sessions.cloud_agent_session_id, currentSessionId)); + + await tx + .update(app_builder_projects) + .set({ session_id: newSessionId }) + .where(eq(app_builder_projects.id, projectId)); await tx.insert(app_builder_project_sessions).values({ project_id: projectId, @@ -380,35 +323,8 @@ async function createCloudAgentNextSession( reason: toSessionReason(reason), worker_version: REQUIRED_WORKER_VERSION, }); - - return true; }); - if (!claimedSession) { - try { - await client.interruptSession(newSessionId); - } catch (error) { - errorExceptInTest( - 'Failed to interrupt superseded App Builder session', - { cloudAgentSessionId: newSessionId, projectId }, - error - ); - } - - const cleanupResult = await client.cleanupSession(newSessionId); - if (!cleanupResult.success) { - errorExceptInTest('Failed to clean up superseded App Builder session', { - cloudAgentSessionId: newSessionId, - projectId, - }); - } - - throw new TRPCError({ - code: 'CONFLICT', - message: 'Project session changed while creating a new session.', - }); - } - return { cloudAgentSessionId: newSessionId, executionId: result.executionId, @@ -602,7 +518,10 @@ export async function getProject( owner: Owner, authToken: string ): Promise { - const { project, sessions } = await getProjectSnapshot(projectId, owner); + const project = await getProjectWithOwnershipCheck(projectId, owner); + + // Fetch all sessions for this project + const sessions = await getProjectSessions(projectId); // Session state for the active session (populated below). // Messages are only eagerly loaded for the active session; ended legacy v1 @@ -641,7 +560,7 @@ export async function getProject( err ); sessionInitiated = null; - sessionPrepared = isDefinitiveSessionNotFoundError(err) ? false : null; + sessionPrepared = 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 deleted file mode 100644 index 86a940903c..0000000000 --- a/apps/web/src/lib/app-builder/github-migration-service.test.ts +++ /dev/null @@ -1,121 +0,0 @@ -jest.mock('@/lib/drizzle', () => ({ - db: { - update: jest.fn(() => ({ - set: jest.fn(() => ({ - where: jest.fn(() => ({ - returning: jest - .fn() - .mockResolvedValue([{ id: 'project-1', deployment_id: null, session_id: 'session-1' }]), - })), - })), - })), - }, -})); -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(), -})); -jest.mock('@/lib/app-builder/app-builder-client', () => { - class AppBuilderError extends Error { - constructor( - message: string, - public statusCode?: number - ) { - super(message); - } - } - - return { - AppBuilderError, - migrateToGithub: jest.fn(), - }; -}); - -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'; - -const sensitiveUrl = 'https://oauth2:secret-token@github.com/owner/repo.git'; -const params = { - projectId: 'project-1', - owner: { type: 'user' as const, id: 'user-1' }, - userId: 'user-1', - repoFullName: 'owner/repo', -}; - -describe('migrateProjectToGitHub error logging', () => { - beforeEach(() => { - jest.clearAllMocks(); - jest.mocked(getProjectWithOwnershipCheck).mockResolvedValue({} as never); - jest.mocked(getIntegrationForOwner).mockResolvedValue({ - id: 'integration-1', - platform_installation_id: 'installation-1', - } as never); - jest.mocked(getRepositoryDetails).mockResolvedValue({ - fullName: 'owner/repo', - cloneUrl: 'https://github.com/owner/repo.git', - htmlUrl: 'https://github.com/owner/repo', - isEmpty: true, - isPrivate: true, - }); - }); - - afterEach(() => { - jest.restoreAllMocks(); - }); - - it.each([ - ['push_failed', 'push_failed'], - ['invalid_request', 'internal_error'], - ['token_failed', 'internal_error'], - ['internal_error', 'internal_error'], - ] as const)( - 'maps Worker %s to %s without logging its message', - async (workerError, publicError) => { - jest.mocked(appBuilderClient.migrateToGithub).mockResolvedValue({ - success: false, - error: workerError, - message: `push rejected ${sensitiveUrl}`, - }); - const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); - - await expect(migrateProjectToGitHub(params)).resolves.toEqual({ - success: false, - error: publicError, - }); - - expect(errorSpy).toHaveBeenCalledWith(`Migration failed (${publicError}):`, { - source: 'app_builder_response', - workerError, - }); - expect(JSON.stringify(errorSpy.mock.calls)).not.toContain(sensitiveUrl); - } - ); - - it('logs a safe transport representation without the original error', async () => { - jest - .mocked(appBuilderClient.migrateToGithub) - .mockRejectedValue(new Error(`request failed for ${sensitiveUrl}`)); - const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); - - await expect(migrateProjectToGitHub(params)).resolves.toEqual({ - success: false, - error: 'internal_error', - }); - - expect(errorSpy).toHaveBeenCalledWith('Migration failed (internal_error):', { - source: 'app_builder_request', - }); - expect(JSON.stringify(errorSpy.mock.calls)).not.toContain(sensitiveUrl); - }); -}); 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 9fda7aecd3..44051a3415 100644 --- a/apps/web/src/lib/app-builder/github-migration-service.ts +++ b/apps/web/src/lib/app-builder/github-migration-service.ts @@ -23,12 +23,9 @@ import type { class MigrationError extends Error { constructor( public readonly code: MigrateToGitHubErrorCode, - public readonly logDetails?: { - source: 'github_repository_lookup' | 'app_builder_request' | 'app_builder_response'; - workerError?: 'invalid_request' | 'internal_error' | 'token_failed' | 'push_failed'; - } + options?: ErrorOptions ) { - super(`Migration failed: ${code}`); + super(`Migration failed: ${code}`, options); this.name = 'MigrationError'; } } @@ -214,8 +211,8 @@ export async function migrateProjectToGitHub( try { repoDetails = await getRepositoryDetails(integration.platform_installation_id, repoFullName); - } catch { - throw new MigrationError('internal_error', { source: 'github_repository_lookup' }); + } catch (error) { + throw new MigrationError('internal_error', { cause: error }); } if (!repoDetails) { @@ -235,17 +232,11 @@ export async function migrateProjectToGitHub( }); if (!migrateResult.success) { - throw new MigrationError( - migrateResult.error === 'push_failed' ? 'push_failed' : 'internal_error', - { - source: 'app_builder_response', - workerError: migrateResult.error, - } - ); + throw new MigrationError('push_failed', { cause: migrateResult }); } } catch (error) { if (error instanceof MigrationError) throw error; - throw new MigrationError('internal_error', { source: 'app_builder_request' }); + throw new MigrationError('push_failed', { cause: error }); } // 5. Update deployment if exists @@ -282,8 +273,8 @@ export async function migrateProjectToGitHub( .where(eq(app_builder_projects.id, projectId)); if (error instanceof MigrationError) { - if (error.logDetails) { - console.error(`Migration failed (${error.code}):`, error.logDetails); + if (error.cause) { + console.error(`Migration failed (${error.code}):`, error.cause); } return { success: false, error: error.code }; } 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(), }); From 0e50ad641e51e0344408a24e42e25c344b5515d6 Mon Sep 17 00:00:00 2001 From: Evan Jacobson Date: Wed, 2 Sep 2026 11:13:52 -0600 Subject: [PATCH 4/4] fix(app-builder): restore validation-only branch scope --- .../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 +- 15 files changed, 23 insertions(+), 668 deletions(-) delete mode 100644 apps/web/src/components/app-builder/project-manager/__tests__/ProjectManager.test.ts delete mode 100644 apps/web/src/components/app-builder/project-manager/__tests__/v2-streaming.test.ts delete mode 100644 apps/web/src/lib/app-builder/app-builder-client.test.ts delete mode 100644 apps/web/src/lib/app-builder/app-builder-service.test.ts delete mode 100644 apps/web/src/lib/app-builder/github-migration-service.test.ts diff --git a/apps/web/src/components/app-builder/AppBuilderChat.tsx b/apps/web/src/components/app-builder/AppBuilderChat.tsx index 29e06aae11..d77ba5a8be 100644 --- a/apps/web/src/components/app-builder/AppBuilderChat.tsx +++ b/apps/web/src/components/app-builder/AppBuilderChat.tsx @@ -508,14 +508,7 @@ 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, - isRecoveringSession, - } = state; + const { isStreaming, isInterrupting, model: projectModel, sessions, pendingNewSession } = state; const messagesEndRef = useRef(null); const scrollContainerRef = useRef(null); @@ -750,7 +743,7 @@ export function AppBuilderChat({ organizationId }: AppBuilderChatProps) { variant="ghost" size="icon" onClick={handleNewChatToggle} - disabled={isStreaming || isRecoveringSession} + disabled={isStreaming} className={pendingNewSession ? 'text-primary bg-primary/10 h-8 w-8' : 'h-8 w-8'} aria-label="New chat" > @@ -758,11 +751,7 @@ export function AppBuilderChat({ organizationId }: AppBuilderChatProps) { - {isRecoveringSession - ? 'A new chat is required' - : pendingNewSession - ? 'Cancel new chat' - : 'New chat'} + {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 ea00f5a97b..1b2bb96284 100644 --- a/apps/web/src/components/app-builder/ProjectManager.ts +++ b/apps/web/src/components/app-builder/ProjectManager.ts @@ -101,10 +101,8 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag } function getActiveSession(): AppBuilderSession | undefined { - if (!cloudAgentSessionId) return undefined; - return store - .getState() - .sessions.find(session => session.info.cloud_agent_session_id === cloudAgentSessionId); + const sessions = store.getState().sessions; + return sessions[sessions.length - 1]; } function subscribeToSession(session: AppBuilderSession): void { @@ -125,16 +123,12 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag const sessionInfos = proj.sessions; if (sessionInfos.length === 0) return []; - const activeInfo = project.session_id - ? sessionInfos.find(s => s.cloud_agent_session_id === project.session_id) - : undefined; + const activeInfo = + sessionInfos.find(s => s.ended_at === null) ?? sessionInfos[sessionInfos.length - 1]; - const orderedSessionInfos = activeInfo - ? [...sessionInfos.filter(info => info.id !== activeInfo.id), activeInfo] - : sessionInfos; const sessions: AppBuilderSession[] = []; - for (const info of orderedSessionInfos) { + for (const info of sessionInfos) { const isActive = info.id === activeInfo?.id; if (!isActive) { @@ -205,7 +199,6 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag store.setState({ sessions: [...currentSessions, newSession], isStreaming: true, - isRecoveringSession: false, }); cloudAgentSessionId = newSessionId; @@ -266,22 +259,15 @@ 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.session_id - ? project.sessions.find(s => s.cloud_agent_session_id === project.session_id) - : undefined; + const activeProjectSessionInfo = + project.sessions.find(s => s.ended_at === null) ?? + project.sessions[project.sessions.length - 1]; - if (activeProjectSessionInfo?.prepared === true && activeProjectSessionInfo.initiated === false) { + if (activeProjectSessionInfo?.initiated === false) { pendingInitialStreamingStart = true; - } else if ( - cloudAgentSessionId && - activeProjectSessionInfo && - activeProjectSessionInfo.prepared !== false - ) { + } else if (cloudAgentSessionId) { pendingReconnect = true; } else { - if (cloudAgentSessionId) { - store.setState({ pendingNewSession: true, isRecoveringSession: true }); - } startPreviewPollingIfNeeded(); } @@ -353,7 +339,6 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag } const effectiveModel = model ?? store.getState().model; - const isRecoveringSession = store.getState().isRecoveringSession; store.setState({ pendingNewSession: false, isStreaming: true }); @@ -385,11 +370,7 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag .catch((err: Error) => { if (destroyed) return; logger.logError('Failed to start new session', err); - store.setState({ - pendingNewSession: true, - isRecoveringSession, - isStreaming: false, - }); + store.setState({ isStreaming: false }); }); } @@ -442,12 +423,11 @@ export function createProjectManager(config: ProjectManagerConfig): ProjectManag if (currentActive) { currentActive.info.ended_at = new Date().toISOString(); } - store.setState({ pendingNewSession: true, isRecoveringSession: false }); + store.setState({ pendingNewSession: true }); } 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 deleted file mode 100644 index 99e532f727..0000000000 --- a/apps/web/src/components/app-builder/project-manager/__tests__/ProjectManager.test.ts +++ /dev/null @@ -1,262 +0,0 @@ -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 43b7981dbb..c13c7cebb7 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,7 +21,6 @@ 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 deleted file mode 100644 index 8b454231fa..0000000000 --- a/apps/web/src/components/app-builder/project-manager/__tests__/v2-streaming.test.ts +++ /dev/null @@ -1,114 +0,0 @@ -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 94a7d87f70..7785b2fe89 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,6 +469,7 @@ 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 be880bfb77..c41cef4393 100644 --- a/apps/web/src/components/app-builder/project-manager/store.ts +++ b/apps/web/src/components/app-builder/project-manager/store.ts @@ -32,7 +32,6 @@ 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 642e8c1b7d..870bb02d2a 100644 --- a/apps/web/src/components/app-builder/project-manager/types.ts +++ b/apps/web/src/components/app-builder/project-manager/types.ts @@ -29,8 +29,6 @@ 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 deleted file mode 100644 index 329f6c9beb..0000000000 --- a/apps/web/src/lib/app-builder/app-builder-client.test.ts +++ /dev/null @@ -1,78 +0,0 @@ -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 56282a0e07..7042393af3 100644 --- a/apps/web/src/lib/app-builder/app-builder-client.ts +++ b/apps/web/src/lib/app-builder/app-builder-client.ts @@ -294,24 +294,15 @@ 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}`, + `Failed to migrate project ${projectId} to GitHub: ${response.status} ${response.statusText} - ${errorText}`, response.status, endpoint ); } - if (!parsed.success) { - throw parsed.error; - } - - return parsed.data; + const data = await response.json(); + return MigrateToGithubResponseSchema.parse(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 deleted file mode 100644 index 705eb6be55..0000000000 --- a/apps/web/src/lib/app-builder/app-builder-service.test.ts +++ /dev/null @@ -1,29 +0,0 @@ -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 c6d2bc4694..779d4d2d56 100644 --- a/apps/web/src/lib/app-builder/app-builder-service.ts +++ b/apps/web/src/lib/app-builder/app-builder-service.ts @@ -72,19 +72,6 @@ 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. */ @@ -573,7 +560,7 @@ export async function getProject( err ); sessionInitiated = null; - sessionPrepared = isDefinitiveSessionNotFoundError(err) ? false : null; + sessionPrepared = 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 deleted file mode 100644 index e448e97f54..0000000000 --- a/apps/web/src/lib/app-builder/github-migration-service.test.ts +++ /dev/null @@ -1,102 +0,0 @@ -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 e34fc7bb32..44051a3415 100644 --- a/apps/web/src/lib/app-builder/github-migration-service.ts +++ b/apps/web/src/lib/app-builder/github-migration-service.ts @@ -232,14 +232,11 @@ export async function migrateProjectToGitHub( }); if (!migrateResult.success) { - throw new MigrationError( - migrateResult.error === 'push_failed' ? 'push_failed' : 'internal_error', - { cause: migrateResult } - ); + throw new MigrationError('push_failed', { cause: migrateResult }); } } catch (error) { if (error instanceof MigrationError) throw error; - throw new MigrationError('internal_error', { cause: error }); + throw new MigrationError('push_failed', { cause: error }); } // 5. Update deployment if exists