From 1095e360c0ee1f906bdc1ad07cd68f790a95331c Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 17:51:31 -0700 Subject: [PATCH] fix(runtime): release the bootstrap latch when the post-create read throws MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `tabsByWorktree` row read that decides between release and park sat outside the try. Measured with `useAppStore.getState` throwing once after a successful create: the dispatch rejects with the latch still in `creating`, and `releaseWebRuntimeInitialTerminalBootstrapOnMirrorFrame` only ever clears `awaiting-mirror` — so no mirror frame can rescue it and every later dispatch for that workspace returns false without creating, until environment teardown. --- ...nitial-terminal-bootstrap-dispatch.test.ts | 52 +++++++++++++++++++ ...ime-initial-terminal-bootstrap-dispatch.ts | 25 +++++---- 2 files changed, 66 insertions(+), 11 deletions(-) create mode 100644 src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.test.ts diff --git a/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.test.ts b/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.test.ts new file mode 100644 index 00000000000..1f1ee5f5bd9 --- /dev/null +++ b/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.test.ts @@ -0,0 +1,52 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useAppStore } from '@/store' +import { dispatchWebRuntimeInitialTerminalBootstrap } from './web-runtime-initial-terminal-bootstrap-dispatch' +import { + isWebRuntimeInitialTerminalBootstrapInFlight, + releaseWebRuntimeInitialTerminalBootstrapOnMirrorFrame, + resetWebRuntimeInitialTerminalBootstrapForTests +} from './web-runtime-initial-terminal-bootstrap' + +const createTerminal = vi.hoisted(() => vi.fn()) +vi.mock('./web-runtime-session', () => ({ createWebRuntimeSessionTerminal: createTerminal })) + +// What this pins: a throw AFTER the create resolves must not leave the latch in `creating`. +// `releaseWebRuntimeInitialTerminalBootstrapOnMirrorFrame` only ever clears `awaiting-mirror`, so +// a stranded `creating` claim survives every later mirror frame and makes the workspace refuse to +// bootstrap a terminal for the rest of the environment's life. + +const ENV_ID = 'env-bootstrap-dispatch' +const WORKTREE_ID = 'repo-1::/w/one' + +describe('dispatchWebRuntimeInitialTerminalBootstrap', () => { + beforeEach(() => { + resetWebRuntimeInitialTerminalBootstrapForTests() + createTerminal.mockReset() + }) + + afterEach(() => { + resetWebRuntimeInitialTerminalBootstrapForTests() + vi.restoreAllMocks() + }) + + it('releases the latch when the post-create store read throws', async () => { + createTerminal.mockResolvedValue({ status: 'created' }) + const getState = vi.spyOn(useAppStore, 'getState').mockImplementation(() => { + throw new Error('store read blew up') + }) + + await expect(dispatchWebRuntimeInitialTerminalBootstrap(ENV_ID, WORKTREE_ID)).rejects.toThrow( + 'store read blew up' + ) + getState.mockRestore() + + expect(isWebRuntimeInitialTerminalBootstrapInFlight(ENV_ID, WORKTREE_ID)).toBe(false) + // A mirror frame cannot rescue a stranded `creating` claim, so the latch had to release itself. + releaseWebRuntimeInitialTerminalBootstrapOnMirrorFrame(ENV_ID, WORKTREE_ID) + expect(isWebRuntimeInitialTerminalBootstrapInFlight(ENV_ID, WORKTREE_ID)).toBe(false) + + createTerminal.mockResolvedValue({ status: 'created' }) + await dispatchWebRuntimeInitialTerminalBootstrap(ENV_ID, WORKTREE_ID) + expect(createTerminal).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.ts b/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.ts index d4f097a2a68..ad554d1f1c6 100644 --- a/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.ts +++ b/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.ts @@ -31,22 +31,25 @@ export async function dispatchWebRuntimeInitialTerminalBootstrap( return false } let outcome: WebRuntimeTerminalCreateOutcome + // Why the row read is inside the try too: a throw between the create and the latch decision + // left `creating` held forever — the mirror-frame release only ever clears `awaiting-mirror`, + // so every later dispatch for that workspace refused without creating until teardown. try { outcome = await createWebRuntimeSessionTerminal({ worktreeId, environmentId, activate: true }) + // Why check the outcome: the create reports RPC and network failures as `{ status: 'failed' }` + // rather than throwing, so the catch below never sees them. Both arms report the same way. + if (outcome.status === 'failed') { + endWebRuntimeInitialTerminalBootstrap(environmentId, worktreeId) + return false + } + if (Object.hasOwn(useAppStore.getState().tabsByWorktree, worktreeId)) { + endWebRuntimeInitialTerminalBootstrap(environmentId, worktreeId) + } else { + markWebRuntimeInitialTerminalBootstrapAwaitingMirror(environmentId, worktreeId) + } } catch (error) { endWebRuntimeInitialTerminalBootstrap(environmentId, worktreeId) throw error } - // Why check the outcome: the create reports RPC and network failures as `{ status: 'failed' }` - // rather than throwing, so the catch above never sees them. Both arms report the same way. - if (outcome.status === 'failed') { - endWebRuntimeInitialTerminalBootstrap(environmentId, worktreeId) - return false - } - if (Object.hasOwn(useAppStore.getState().tabsByWorktree, worktreeId)) { - endWebRuntimeInitialTerminalBootstrap(environmentId, worktreeId) - } else { - markWebRuntimeInitialTerminalBootstrapAwaitingMirror(environmentId, worktreeId) - } return true }