From 7cf8b6ac52f2b2d0c7b6233e0ffc1217a80e2a57 Mon Sep 17 00:00:00 2001 From: Brennan Benson Date: Sun, 12 Jul 2026 15:41:28 -0700 Subject: [PATCH] fix(runtime): drain terminal admissions before removal --- config/reliability-gates.jsonc | 10 ++ src/main/runtime/orca-runtime.test.ts | 35 +++++ src/main/runtime/orca-runtime.ts | 209 ++++++++++++++++---------- 3 files changed, 176 insertions(+), 78 deletions(-) diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 70da4dd1c86..4ec97a8503d 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -3698,6 +3698,7 @@ "terminal.stop awaits stopAndWait and fresh post-stop liveness before acknowledging provider shutdown", "provider shutdown rejection fails terminal.stop closed", "project removal fences paired terminal admission through provider stop and repo mutation", + "project removal drains create-first in-flight admissions before its liveness snapshot", "targeted terminal stop omits foreground probes for unrelated sessions" ] }, @@ -3765,6 +3766,15 @@ "result": "passed", "durationSeconds": 12, "summary": "1236 tests passed with relay-generation identity, stale-spawn retained ownership, bounded renderer maps/retry work, VM cleanup success ordering, atomic runtime removal admission, and targeted liveness probes." + }, + { + "date": "2026-07-12", + "runner": "local", + "platform": "macos", + "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts", + "result": "passed", + "durationSeconds": 18, + "summary": "664 runtime tests passed, including removal-first rejection and create-first admission drain before provider liveness snapshots." } ], "runtimeBudget": { diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 148c5ebeb9b..8ce1a18165c 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -22266,6 +22266,41 @@ describe('OrcaRuntimeService', () => { expect(removeProject).toHaveBeenCalledWith(TEST_REPO_ID) }) + it('drains a create-first spawn before project removal snapshots liveness', async () => { + const removeProject = vi.fn() + const runtime = new OrcaRuntimeService({ ...store, removeProject } as never) + const spawnResult = deferred<{ id: string }>() + const stopAndWait = vi.fn(async (ptyId: string) => { + runtime.onPtyExit(ptyId, -1) + return true + }) + const processLists = [[{ id: 'pty-late', cwd: '/tmp/worktree-a', title: 'Shell' }], []] + const spawn = vi.fn(() => spawnResult.promise) + runtime.setPtyController({ + spawn, + write: () => true, + kill: () => false, + stopAndWait, + getForegroundProcess: async () => null, + listProcesses: async () => processLists.shift() ?? [] + }) + syncSinglePty(runtime, 'pty-late') + + const create = runtime.createTerminal(`id:${TEST_WORKTREE_ID}`) + await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()) + const removal = runtime.removeProject(`id:${TEST_REPO_ID}`) + await Promise.resolve() + expect(removeProject).not.toHaveBeenCalled() + expect(processLists).toHaveLength(2) + + spawnResult.resolve({ id: 'pty-late' }) + await expect(create).resolves.toMatchObject({ ptyId: 'pty-late' }) + await expect(removal).resolves.toEqual({ removed: true }) + + expect(stopAndWait).toHaveBeenCalledWith('pty-late') + expect(removeProject).toHaveBeenCalledWith(TEST_REPO_ID) + }) + it('rejects terminal.stop when provider shutdown is not accepted', async () => { const runtime = new OrcaRuntimeService(store) runtime.setPtyController({ diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index f94d8547fc5..010483b6d10 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -2227,6 +2227,7 @@ export class OrcaRuntimeService { // Why: repo removal must make verified stop + metadata removal atomic with // respect to paired/mobile terminal creation, or a late spawn is orphaned. private terminalAdmissionBlockedWorktreeIds = new Set() + private terminalAdmissionsByWorktreeId = new Map>>() private titleObservationSequence = 0 private headlessTerminals = new Map() private ptyOutputSequenceById = new Map() @@ -12200,6 +12201,47 @@ export class OrcaRuntimeService { return updated } + private beginTerminalAdmission(worktreeId: string): () => void { + if (this.terminalAdmissionBlockedWorktreeIds.has(worktreeId)) { + throw new Error('terminal_admission_blocked_for_project_removal') + } + let resolveDone = (): void => {} + const done = new Promise((resolve) => { + resolveDone = resolve + }) + const admissions = this.terminalAdmissionsByWorktreeId.get(worktreeId) ?? new Set() + admissions.add(done) + this.terminalAdmissionsByWorktreeId.set(worktreeId, admissions) + let released = false + return () => { + if (released) { + return + } + released = true + admissions.delete(done) + if (admissions.size === 0) { + this.terminalAdmissionsByWorktreeId.delete(worktreeId) + } + resolveDone() + } + } + + private async drainTerminalAdmissions(worktreeIds: readonly string[]): Promise { + const pending = worktreeIds.flatMap((id) => [ + ...(this.terminalAdmissionsByWorktreeId.get(id) ?? []) + ]) + await Promise.all(pending) + } + + private async withTerminalAdmission(worktreeId: string, action: () => Promise): Promise { + const release = this.beginTerminalAdmission(worktreeId) + try { + return await action() + } finally { + release() + } + } + async removeProject(repoSelector: string): Promise<{ removed: true }> { if (!this.store?.removeProject) { throw new Error('runtime_unavailable') @@ -12215,6 +12257,10 @@ export class OrcaRuntimeService { this.terminalAdmissionBlockedWorktreeIds.add(id) } try { + // Why: a create that passed admission before the block can still have a + // provider spawn in flight. Let it register first so the fresh stop + // snapshot sees and terminates it. + await this.drainTerminalAdmissions(worktreeIds) // Why: hold admission closed across both the fresh liveness proof and // repo mutation. A separate terminal.stop RPC leaves an inter-call gap. for (const id of worktreeIds) { @@ -17597,31 +17643,31 @@ export class OrcaRuntimeService { tabId, agentTeamsPlan?.env ) - if (this.terminalAdmissionBlockedWorktreeIds.has(workspace.id)) { - throw new Error('terminal_admission_blocked_for_project_removal') - } - const result = await this.ptyController.spawn({ - cols: 120, - rows: 40, - cwd, - command: sequencedStartupCommand - ? launchOpts.command - : (agentTeamsPlan?.command ?? launchOpts.command), - commandDelivery: 'provider', - startupCommandDelivery: launchOpts.startupCommandDelivery, - env, - envToDelete: agentTeamsPlan?.envToDelete, - telemetry: launchOpts.telemetry, - connectionId: workspace.connectionId, - worktreeId: workspace.id, - preAllocatedHandle, - tabId, - leafId, - ...(launchOpts.sessionId ? { sessionId: launchOpts.sessionId } : {}), - ...(launchOpts.persistHostSessionBinding ? { persistHostSessionBinding: true } : {}) + const result = await this.withTerminalAdmission(workspace.id, async () => { + const spawned = await this.ptyController!.spawn!({ + cols: 120, + rows: 40, + cwd, + command: sequencedStartupCommand + ? launchOpts.command + : (agentTeamsPlan?.command ?? launchOpts.command), + commandDelivery: 'provider', + startupCommandDelivery: launchOpts.startupCommandDelivery, + env, + envToDelete: agentTeamsPlan?.envToDelete, + telemetry: launchOpts.telemetry, + connectionId: workspace.connectionId, + worktreeId: workspace.id, + preAllocatedHandle, + tabId, + leafId, + ...(launchOpts.sessionId ? { sessionId: launchOpts.sessionId } : {}), + ...(launchOpts.persistHostSessionBinding ? { persistHostSessionBinding: true } : {}) + }) + this.registerPreAllocatedHandleForPty(spawned.id, preAllocatedHandle) + this.registerPty(spawned.id, workspace.id, workspace.connectionId) + return spawned }) - this.registerPreAllocatedHandleForPty(result.id, preAllocatedHandle) - this.registerPty(result.id, workspace.id, workspace.connectionId) const pty = this.getOrCreatePtyWorktreeRecord(result.id) if (pty) { if (launchOpts.title) { @@ -17719,48 +17765,55 @@ export class OrcaRuntimeService { // Why: terminal creation is a renderer-side Zustand store operation (like // browser tab creation). The main process sends a request, the renderer // creates the tab and replies with the tabId so we can resolve the handle. - const reply = await new Promise<{ tabId: string; title: string }>((resolve, reject) => { - const timer = setTimeout(() => { - ipcMain.removeListener('terminal:tabCreateReply', handler) - reject(new Error('Terminal creation timed out')) - }, 10_000) + const createRendererTerminal = async (): Promise<{ + reply: { tabId: string; title: string } + handle: string + }> => { + const reply = await new Promise<{ tabId: string; title: string }>((resolve, reject) => { + const timer = setTimeout(() => { + ipcMain.removeListener('terminal:tabCreateReply', handler) + reject(new Error('Terminal creation timed out')) + }, 10_000) - const handler = ( - event: Electron.IpcMainEvent, - r: { requestId: string; tabId?: string; title?: string; error?: string } - ): void => { - if (event.sender !== win.webContents || r.requestId !== requestId) { - return + const handler = ( + event: Electron.IpcMainEvent, + r: { requestId: string; tabId?: string; title?: string; error?: string } + ): void => { + if (event.sender !== win.webContents || r.requestId !== requestId) { + return + } + clearTimeout(timer) + ipcMain.removeListener('terminal:tabCreateReply', handler) + if (r.error) { + reject(new Error(r.error)) + } else { + resolve({ tabId: r.tabId!, title: r.title ?? launchOpts.title ?? '' }) + } } - clearTimeout(timer) - ipcMain.removeListener('terminal:tabCreateReply', handler) - if (r.error) { - reject(new Error(r.error)) - } else { - resolve({ tabId: r.tabId!, title: r.title ?? launchOpts.title ?? '' }) - } - } - ipcMain.on('terminal:tabCreateReply', handler) - win.webContents.send('terminal:requestTabCreate', { - requestId, - worktreeId, - command: launchOpts.command, - cwd, - ...(launchOpts.env ? { env: launchOpts.env } : {}), - ...(launchOpts.launchConfig ? { launchConfig: launchOpts.launchConfig } : {}), - ...(launchOpts.launchToken ? { launchToken: launchOpts.launchToken } : {}), - ...(launchOpts.launchAgent ? { launchAgent: launchOpts.launchAgent } : {}), - startupCommandDelivery: launchOpts.startupCommandDelivery, - title: launchOpts.title, - activate: presentation === 'focused', - ...(presentation ? { presentation } : {}) + ipcMain.on('terminal:tabCreateReply', handler) + win.webContents.send('terminal:requestTabCreate', { + requestId, + worktreeId, + command: launchOpts.command, + cwd, + ...(launchOpts.env ? { env: launchOpts.env } : {}), + ...(launchOpts.launchConfig ? { launchConfig: launchOpts.launchConfig } : {}), + ...(launchOpts.launchToken ? { launchToken: launchOpts.launchToken } : {}), + ...(launchOpts.launchAgent ? { launchAgent: launchOpts.launchAgent } : {}), + startupCommandDelivery: launchOpts.startupCommandDelivery, + title: launchOpts.title, + activate: presentation === 'focused', + ...(presentation ? { presentation } : {}) + }) }) - }) - // Why: the renderer created the tab immediately, but the graph sync that - // populates this.leaves may not have arrived yet. Wait for the leaf to - // appear so we can return a valid handle the caller can use right away. - const handle = await this.waitForTerminalHandle(reply.tabId) + // Why: keep admission active until graph sync publishes the created PTY; + // project removal can then drain this create before its liveness snapshot. + return { reply, handle: await this.waitForTerminalHandle(reply.tabId) } + } + const { reply, handle } = workspace + ? await this.withTerminalAdmission(workspace.id, createRendererTerminal) + : await createRendererTerminal() return { handle, tabId: reply.tabId, @@ -18638,23 +18691,23 @@ export class OrcaRuntimeService { const leafId = randomUUID() const preAllocatedHandle = this.createPreAllocatedTerminalHandle() const paneKey = makePaneKey(parentTabId, leafId) - if (this.terminalAdmissionBlockedWorktreeIds.has(workspace.id)) { - throw new Error('terminal_admission_blocked_for_project_removal') - } - const result = await this.ptyController.spawn({ - cols: 120, - rows: 40, - cwd: workspace.path, - command: opts.command, - commandDelivery: 'provider', - env: this.buildTerminalWorkspaceEnv(workspace, opts.env ?? {}, paneKey, parentTabId), - envToDelete: opts.envToDelete, - connectionId: workspace.connectionId, - worktreeId: workspace.id, - preAllocatedHandle + const result = await this.withTerminalAdmission(workspace.id, async () => { + const spawned = await this.ptyController!.spawn!({ + cols: 120, + rows: 40, + cwd: workspace.path, + command: opts.command, + commandDelivery: 'provider', + env: this.buildTerminalWorkspaceEnv(workspace, opts.env ?? {}, paneKey, parentTabId), + envToDelete: opts.envToDelete, + connectionId: workspace.connectionId, + worktreeId: workspace.id, + preAllocatedHandle + }) + this.registerPreAllocatedHandleForPty(spawned.id, preAllocatedHandle) + this.registerPty(spawned.id, workspace.id, workspace.connectionId) + return spawned }) - this.registerPreAllocatedHandleForPty(result.id, preAllocatedHandle) - this.registerPty(result.id, workspace.id, workspace.connectionId) const createdPty = this.getOrCreatePtyWorktreeRecord(result.id) if (createdPty) { createdPty.tabId = parentTabId