From 841d094913a7e61bce35cfafea1786050bd3b4f2 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 12 Sep 2026 13:51:18 -0700 Subject: [PATCH 1/6] docs(lifecycle): plan veto-safe application shutdown --- .../2026-09-12-quit-service-lifecycle.md | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-12-quit-service-lifecycle.md diff --git a/docs/superpowers/plans/2026-09-12-quit-service-lifecycle.md b/docs/superpowers/plans/2026-09-12-quit-service-lifecycle.md new file mode 100644 index 000000000..a88fbeb5e --- /dev/null +++ b/docs/superpowers/plans/2026-09-12-quit-service-lifecycle.md @@ -0,0 +1,33 @@ +# Preserve application services when quitting is cancelled + +Status: source revalidation and implementation plan for #919, B02 of #918 / program plan PR #931. This branch begins with this plan; no implementation precedes it. + +Baseline: `115e26fc9c67316a3b0b1b4318f47f7a2bea3606`, current `origin/main` on 2026-09-12. It includes the skills and TLDR changes from #903/#932. It does not include unmerged tmux PR #933 or session-routing PR #935. The quit handlers are unchanged by those merged features. Do not assume either independent recovery slice has merged. + +## Confirmed seam and intended outcome + +`src/main/index.ts` stops workflows from `before-quit`, re-enters quit, then starts MCP, remote, LSP, dictation, caffeinate, watchdog and persistence teardown before a renderer can reject closing. `SessionShutdownGate` correctly waits for `will-quit` before permanent session teardown, but that gate cannot protect disposals elsewhere. `appWindow.ts` implements Keep Editing through Electron's `will-prevent-unload`, and the real editor guard checks current project/AI Workspace buffer state at each unload. + +After a veto, every previously working service must still be usable. After committed shutdown, the app must retain one exact drain attempt, reject new conflicting admission, await required drains, and mark clean/release the process lock only after those drains are confirmed. A rejected or uncertain stop must retain ownership and retry/inspection facilities. Successful drains must not be repeated merely because another drain failed. + +## Delivery boundaries + +The first bounded implementation fixes application-wide disposal composition and required exit drains. It preserves the existing native per-window editor decision UX and the current session shutdown gate's one-way terminal-admission fact. Preparation may flush observations but may not cancel workflows, stop providers, dispose support services, or mark the process clean. + +The complete B02 program ALSO requires a quit-attempt generation, renderer/buffer revision-bound decisions, participant revalidation and a short stable revision frontier before commit. Keep that work explicit under #919. If this branch's first PR does not yet implement that whole preparation protocol, use **Refs #919**, retain the remaining acceptance criteria, and continue its dependent slice. Do not claim that relocating disposals by itself completes B02 or that an old editor approval can authorize a later edit revision. + +## Implementation sequence + +1. Initialize the seven pinned package checkouts and isolated dependencies; run existing shutdown gate and editor unload tests. Inspect all app quit/last-window listeners, service construction/disposal owners, and the new TLDR hook resources. Record which resources exist during partial startup. +2. Build one inspectable application shutdown composition at the real main entry point. Inventory reversible preparation, required producer stops, required persistence drains, support disposal, best-effort diagnostics and final synchronous release. Keep ownership and dependency order explicit in WHY comments beside the wiring. +3. Keep irreversible effects behind committed shutdown. Publish that state before the first destructive await; repeated quit requests join the exact attempt. Audit spawn/recovery, workflow admission, activation and new-window routes so a failed committed shutdown cannot reopen an unusable ordinary workspace. +4. Retain service references until their required stop is confirmed. Await workflow/session ownership release and required stores; do not replace missing evidence with a timer, one event-loop tick or registry removal. Retain completed-stage receipts and retry only unresolved drains. Keep diagnostic/inspection services available while native release remains uncertain. +5. Integrate external control/settings disposal with the same exit gate instead of an independent fire-and-forget `will-quit` listener. Preserve macOS last-window behavior and failed-startup cleanup. Do not introduce a second provider cleanup owner or weaken Codex/native custody. +6. Exercise the real composition through before-quit, actual editor-veto behavior, and will-quit. Verify workflow/MCP/LSP/dictation calls still work after veto; verify exact drain joins, failure/retry, no early clean marker, partial startup, non-macOS last-window ordering and post-commit admission. Mock OS/process boundaries rather than the coordinator's own caller. +7. Run proportionate checks, review every irreversible call site, update this plan for substantive decisions, open a complete linked PR and inspect current-head CI/reviews. Record the revision-bound preparation work accurately in #919/#918. No PR merge without explicit user authorization. + +## Verification and scope rules + +A unit test that calls only `SessionShutdownGate` cannot expose a rogue main `before-quit` disposer. The composed regression must include the caller that owns those service references. A mock stop result proves coordinator sequencing, not native process termination. Native ownership remains governed by each existing service contract and its separate program tests. + +Keep the window-routing repair and tmux changes independent. Do not modify native transcripts, stop live user sessions, or use a live application Quit as a routine test. Local tests must own their processes/state. If a live smoke is later useful, make its isolation and resource ownership concrete first. From e9b88bf57e3a45447b9f7a2adb2eba0b1980ee0c Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 12 Sep 2026 14:22:31 -0700 Subject: [PATCH 2/6] fix: defer application disposal until quit is committed --- ARCHITECTURE.md | 47 +-- docs/architecture/diagrams/shutdown.svg | 2 +- .../2026-09-12-quit-service-lifecycle.md | 35 +- src/main/applicationShutdown.test.ts | 209 ++++++++++++ src/main/applicationShutdown.ts | 164 ++++++++++ src/main/dictation/historyStore.ts | 12 +- src/main/index.ts | 300 ++++++++---------- src/main/ipc/dictation.ts | 59 +++- src/main/ipc/dictationShutdown.test.ts | 99 ++++++ src/main/menu/appMenu.ts | 3 +- src/main/recording/SessionRecorderManager.ts | 2 +- .../RemoteController.integration.test.ts | 37 +++ src/main/remote/RemoteController.ts | 34 +- src/main/sessionShutdownGate.test.ts | 48 ++- src/main/sessionShutdownGate.ts | 30 +- src/main/storage/workspaceFileStore.test.ts | 16 + src/main/storage/workspaceFileStore.ts | 10 + src/main/window/windowRegistry.test.ts | 12 + src/main/window/windowRegistry.ts | 12 + .../workflows/createWorkflowService.test.ts | 15 + src/main/workflows/createWorkflowService.ts | 5 + ...eEditorBeforeUnloadGuard.renderer.test.tsx | 69 ++++ 22 files changed, 969 insertions(+), 251 deletions(-) create mode 100644 src/main/applicationShutdown.test.ts create mode 100644 src/main/applicationShutdown.ts create mode 100644 src/main/ipc/dictationShutdown.test.ts create mode 100644 src/renderer/src/features/global-editor/hooks/useEditorBeforeUnloadGuard.renderer.test.tsx diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 2dcc8de3a..146907901 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -1260,35 +1260,38 @@ Shutdown has vetoes. An unsaved editor can refuse a window close. Workflow shutd ```text sequenceDiagram participant User - participant App as Electron lifecycle - participant WF as WorkflowService + participant App as Application shutdown gate participant Window as Renderer/editor guard - participant SM as Session shutdown gate - participant Aux as Auxiliary services and journals - User->>App: Quit - App->>WF: Stop durable workflow execution - alt Workflow stop cannot finish safely - WF-->>App: Failure, retain application for retry - else Workflow stop completes - App->>Window: Close / beforeunload - alt Unsaved changes veto close - Window-->>App: Keep editing - else Close is allowed - App->>SM: will-quit: killAll and await teardown - alt Owned process teardown fails - SM-->>App: Block quit, report failure - else Teardown completes - App->>Aux: Flush queues, stop servers and helpers - App->>Aux: Mark clean run and release state lock - App-->>User: Application exits - end + participant Exec as Sessions and workflows + participant Aux as Services and persistence + User->>App: Quit and reversible observation flush + App->>Window: Close / beforeunload + alt Unsaved changes veto close + Window-->>App: Keep Editing, all services remain live + else Close is allowed + App->>App: will-quit, publish committed admission + App->>Exec: Begin both execution stops + App->>App: Join admitted startup and dictation work + App->>Exec: Await ownership release + alt Required stop rejects + Exec-->>App: Retain failed owner for explicit retry + App->>App: Keep inspection services and process lock + else Required stops complete + App->>Aux: Await support disposal and admitted write tails + App->>Aux: Await diagnostic flushes, report optional failures + App->>App: Mark clean unless startup failed, release state lock + App-->>User: Application exits end end ``` -Some auxiliary shutdown hooks run earlier or concurrently with these gates. Diagnostic flushes are not all awaited with the same durability guarantee as workflow state and owned-process shutdown. “Clean exit” is a lifecycle result, not proof that every optional debug record reached disk. +[Application shutdown composition](src/main/applicationShutdown.ts) keeps irreversible disposal out of `before-quit`. The existing [terminal gate](src/main/sessionShutdownGate.ts) holds `will-quit` until the complete application drain resolves. Repeated quit requests join one attempt; retries retain completed-stage receipts and invoke only failed stages. Startup publishes workflow ownership before initialization and checks committed admission after asynchronous acquisition. All window creation routes share a committed-shutdown guard. + +Required execution stops retain their native ownership contracts. Workspace and dictation history tails establish settlement of admitted writes; they do not retry failed saves or establish fsync durability. Dictation joins already-admitted batch/preview operations before its history tail is captured. Diagnostic queues are awaited, with failures reported separately. A failed boot retains its process lock through cleanup and does not receive a clean-run marker. + +This is the application disposal repair in B02/#919. It does not yet introduce revision-bound editor approvals, a cross-window preparation generation, or a final persistence acknowledgement frontier. The native per-window close decision UX remains in use. “Clean exit” remains a lifecycle result rather than proof that every optional record reached disk. ### 6.2 Windows, workspace, and restoration diff --git a/docs/architecture/diagrams/shutdown.svg b/docs/architecture/diagrams/shutdown.svg index 37d377161..13f4dcbac 100644 --- a/docs/architecture/diagrams/shutdown.svg +++ b/docs/architecture/diagrams/shutdown.svg @@ -1,2 +1,2 @@ -Auxiliary services and journalsSession shutdown gateRenderer/editor guardWorkflowServiceElectron lifecycleUserAuxiliary services and journalsSession shutdown gateRenderer/editor guardWorkflowServiceElectron lifecycleUseralt[Owned process teardown fails][Teardown completes]alt[Unsaved changes veto close][Close is allowed]alt[Workflow stop cannot finish safely][Workflow stop completes]QuitStop durable workflow executionFailure, retain application for retryClose / beforeunloadKeep editingwill-quit: killAll and await teardownBlock quit, report failureFlush queues, stop servers and helpersMark clean run and release state lockApplication exits +Services and persistenceSessions and workflowsRenderer/editor guardApplication shutdown gateUserServices and persistenceSessions and workflowsRenderer/editor guardApplication shutdown gateUseralt[Required stop rejects][Required stops complete]alt[Unsaved changes veto close][Close is allowed]Quit and reversible observation flushClose / beforeunloadKeep Editing, all services remain livewill-quit, publish committed admissionBegin both execution stopsJoin admitted startup and dictation workAwait ownership releaseRetain failed owner for explicit retryKeep inspection services and process lockAwait support disposal and admitted write tailsAwait diagnostic flushes, report optional failuresMark clean unless startup failed, release state lockApplication exits diff --git a/docs/superpowers/plans/2026-09-12-quit-service-lifecycle.md b/docs/superpowers/plans/2026-09-12-quit-service-lifecycle.md index a88fbeb5e..ca6a5a35d 100644 --- a/docs/superpowers/plans/2026-09-12-quit-service-lifecycle.md +++ b/docs/superpowers/plans/2026-09-12-quit-service-lifecycle.md @@ -1,6 +1,6 @@ # Preserve application services when quitting is cancelled -Status: source revalidation and implementation plan for #919, B02 of #918 / program plan PR #931. This branch begins with this plan; no implementation precedes it. +Status: first B02 disposal repair implemented locally; final verification and PR preparation in progress. Tracking #919 and #918 / program plan PR #931. The branch began with this plan in commit `841d094913a7e61bce35cfafea1786050bd3b4f2`; no implementation preceded it. Baseline: `115e26fc9c67316a3b0b1b4318f47f7a2bea3606`, current `origin/main` on 2026-09-12. It includes the skills and TLDR changes from #903/#932. It does not include unmerged tmux PR #933 or session-routing PR #935. The quit handlers are unchanged by those merged features. Do not assume either independent recovery slice has merged. @@ -31,3 +31,36 @@ The complete B02 program ALSO requires a quit-attempt generation, renderer/buffe A unit test that calls only `SessionShutdownGate` cannot expose a rogue main `before-quit` disposer. The composed regression must include the caller that owns those service references. A mock stop result proves coordinator sequencing, not native process termination. Native ownership remains governed by each existing service contract and its separate program tests. Keep the window-routing repair and tmux changes independent. Do not modify native transcripts, stop live user sessions, or use a live application Quit as a routine test. Local tests must own their processes/state. If a live smoke is later useful, make its isolation and resource ownership concrete first. + + +## Implementation checkpoint + +The application disposal inventory now lives in `applicationShutdown.ts`, installed by the real main entry point. `before-quit` only records reversible preparation and flushes observations; it skips that preparation on re-entry after commitment so it cannot enqueue new work behind the final drains. `SessionShutdownGate` retains its existing terminal-admission facade, but takes the complete application drain rather than inferring an empty inventory from a missing manager. It publishes the exact join before calling potentially reentrant/synchronously throwing shutdown code. Non-macOS last-window closure requests this same quit path; macOS still leaves the app running. + +Execution stops begin together. Stage receipts retain completed work across retry and preserve failed owners. Main keeps workflow/control inspection infrastructure until session and workflow stop contracts resolve. A failed drain holds the exit gate and process lock, with a later Quit retrying unresolved stages. Support disposal, admitted write-tail settlement and optional diagnostic flushing are separate stages. + +Partial startup required more than moving listeners. The workflow factory publishes its owner before `initialize`, whose existing stop contract already closes recovery admission and joins initialization. Main checks committed shutdown after asynchronous resource acquisition and the coordinator joins startup before closing its resource inventory. Failed startup retains the lock through cleanup and does not receive a clean-run marker. A typed workflow-closing outcome caused by committed quit is treated as interrupted startup. The common window factory fences restoration, menu, IPC and external-control creation after commitment. + +The composition audit exposed two concrete owner gaps that this same slice repairs: + +- Dictation's active map excludes stop handlers awaiting batch HTTP. Committed cleanup now fences new operations, cancels active previews, joins admitted start/stop/hotkey work and the independent preview-stop promise, then captures the history write tail. A key lookup or hotkey configuration finishing late cannot revive a resource behind cleanup. +- Remote disposal previously bypassed the enable/disable FIFO and erased its server pointer before a stop resolved. Disposal now closes enable admission, joins that FIFO, prevents a pending enable from publishing a live URL, and keeps the exact server on rejection for an explicit retry. + +The shutdown section of `ARCHITECTURE.md` and its generated preview now describe the implemented boundary. All 42 diagram sources rendered and verified with the pinned documentation tools; only the changed shutdown preview is retained. Five unrelated previews differed when regenerated in this local Chrome environment; those generated changes were discarded. The shutdown SVG was visually inspected independently. + +## Evidence and limits of this slice + +The expanded unit lane passed 57 cases across seven files; the final coordinator/gate/dictation subset passed 19 cases after the drain-order review. The real remote stack suite passed nine system cases, including delayed start and failed transport release. The real renderer editor guard composed with the application listener installer passed its veto/clean-close test. External boundaries in those tests are Electron dispatch, provider HTTP, transport or filesystem operations as appropriate; no live user application was quit. + +Full typecheck, the test-contract checker and all seven pinned-checkout checks passed. Final-source incremental typecheck, application build/entrypoint verification, final targeted checks and PR CI are recorded below as they finish. The first renderer fixture assertion used the wrong buffer field (`text` instead of `currentText`); it was corrected and the renderer test passed. No product behavior was changed to satisfy that assertion. + +This PR uses **Refs #919**, not an issue-closing keyword. B02 remains open for the full revision-bound prepare/commit protocol. In particular: + +- Native per-window decisions are not yet votes bound to a quit generation, renderer generation and buffer revision. Cross-window edits, navigation and changing participants still need final revalidation and a short stable admission frontier. +- `WorkspaceFileStore.drainAdmittedWrites` and `flushHistoryWrites` join their existing admission tails. Each original write still owns its failure receipt. These APIs do not recover an unacknowledged final renderer save, retry a rejected write, or establish fsync durability. +- The remaining frontier must explicitly cover already-admitted control operations and their durable result writes. `createControlHost.dispose()` currently retires bridge registrations synchronously; it is not an awaitable guarantee that every executor operation and `FileControlHistory` append has settled. This slice does not add that missing contract or claim that the clean-run marker proves it. +- Support disposal establishes the current service API's promise. LSP's current dispose requests process termination without independent exit evidence; its process/document ownership audit remains in B04. Optional diagnostics report write errors separately. Native provider custody is not redefined by this coordinator. + +The bounded result is preservation of services on veto and one explicit composition of existing committed-stop/drain contracts, including the two owner gaps above. It is not completion of every quit durability or editor approval invariant in the program. + +The next control frontier audit starts at `src/main/control/createControlHost.ts`, `src/control-sdk/core/executor.ts` and `src/main/control/history/FileControlHistory.ts`. The executor's `active` map is populated only after its admitted intent write, so draining that map alone would miss a request still queued in `exclusive`. Nested waits/batches also call the executor directly. Main's private `operations.start`/`operations.finish` port must keep completion receipts writable while new effectful work is closed; a blanket rejection of all invocation would lose the evidence shutdown needs. Keep these facts in the next focused B02 plan. diff --git a/src/main/applicationShutdown.test.ts b/src/main/applicationShutdown.test.ts new file mode 100644 index 000000000..7ca314452 --- /dev/null +++ b/src/main/applicationShutdown.test.ts @@ -0,0 +1,209 @@ +import { EventEmitter } from 'node:events' +import { describe, expect, it, vi } from 'vitest' +import { installApplicationShutdown, type ApplicationShutdownServices } from './applicationShutdown' + +function deferred() { + let resolve!: () => void + let reject!: (error: unknown) => void + const promise = new Promise((yes, no) => { resolve = yes; reject = no }) + return { promise, resolve, reject } +} + +function harness() { + const events = new EventEmitter() + let veto = false + const app = Object.assign(events, { + quit: vi.fn(() => { + events.emit('before-quit') + if (!veto) events.emit('will-quit', { preventDefault: vi.fn() }) + }), + }) + const sessionStop = vi.fn(async (): Promise => undefined) + const workflowStop = vi.fn(async (_reason: string): Promise => undefined) + const services = { + getSessions: (): ReturnType => ({ killAll: sessionStop }), + getWorkflows: (): ReturnType => ({ stop: workflowStop }), + startupSettled: vi.fn(async (): Promise => undefined), + stopDictation: vi.fn(async (): Promise => undefined), + flushObservations: vi.fn(async (): Promise => undefined), + sweepOwnedProxies: vi.fn(async (): Promise => undefined), + stopBuiltInMcp: vi.fn(async (): Promise => undefined), + stopRemote: vi.fn(async (): Promise => undefined), + stopLsp: vi.fn(async (): Promise => undefined), + stopExternalControl: vi.fn(async (): Promise => undefined), + disposeControl: vi.fn(async (): Promise => undefined), + disposeWorkflowBridge: vi.fn(async (): Promise => undefined), + disposeCaffeinate: vi.fn(async (): Promise => undefined), + stopHeapWatchdog: vi.fn(async (): Promise => undefined), + drainWorkspace: vi.fn(async (): Promise => undefined), + drainDictationHistory: vi.fn(async (): Promise => undefined), + flushGhosts: vi.fn(async (): Promise => undefined), + flushRecordings: vi.fn(async (): Promise => undefined), + flushDictationDebug: vi.fn(async (): Promise => undefined), + flushPasteDebug: vi.fn(async (): Promise => undefined), + stopPerformance: vi.fn(async (): Promise => undefined), + } satisfies ApplicationShutdownServices + const prepare = vi.fn() + const onQuitAllowed = vi.fn() + const onShutdownError = vi.fn() + const onDiagnosticError = vi.fn() + function install(platform: NodeJS.Platform = 'darwin') { + return installApplicationShutdown({ app, services, prepare, onQuitAllowed, onShutdownError, onDiagnosticError, platform }) + } + return { app, services, sessionStop, workflowStop, prepare, onQuitAllowed, onShutdownError, + onDiagnosticError, install, setVeto: (value: boolean) => { veto = value } } +} + +describe('application shutdown composition', () => { + it('keeps every service and recorder usable after repeated editor vetoes', () => { + const h = harness() + const gate = h.install() + h.setVeto(true) + h.app.quit() + h.app.quit() + expect(h.prepare).toHaveBeenCalledTimes(2) + expect(gate.isTerminalShutdownAdmitted()).toBe(false) + expect(h.sessionStop).not.toHaveBeenCalled() + expect(h.workflowStop).not.toHaveBeenCalled() + for (const [name, service] of Object.entries(h.services)) { + if (name.startsWith('get')) continue + expect(service, name).not.toHaveBeenCalled() + } + expect(h.onQuitAllowed).not.toHaveBeenCalled() + }) + + it('starts both execution fences immediately and joins duplicate quit requests', async () => { + const h = harness() + const sessions = deferred() + const workflows = deferred() + h.sessionStop.mockImplementation(() => sessions.promise) + h.workflowStop.mockImplementation(() => workflows.promise) + const gate = h.install() + h.app.quit() + h.app.quit() + expect(gate.isTerminalShutdownAdmitted()).toBe(true) + expect(h.sessionStop).toHaveBeenCalledOnce() + expect(h.workflowStop).toHaveBeenCalledOnce() + expect(h.services.stopBuiltInMcp).not.toHaveBeenCalled() + sessions.resolve() + await sessions.promise + expect(h.onQuitAllowed).not.toHaveBeenCalled() + workflows.resolve() + await vi.waitFor(() => expect(h.onQuitAllowed).toHaveBeenCalledOnce()) + expect(h.sessionStop).toHaveBeenCalledOnce() + expect(h.workflowStop).toHaveBeenCalledOnce() + expect(h.services.disposeControl.mock.invocationCallOrder[0]).toBeGreaterThan( + h.services.stopExternalControl.mock.invocationCallOrder[0]!, + ) + expect(h.prepare).toHaveBeenCalledOnce() + }) + + it('retains inspection services after uncertain execution and retries only the failed owner', async () => { + const h = harness() + h.workflowStop.mockRejectedValueOnce(new Error('Native ownership unconfirmed')) + const gate = h.install() + h.app.quit() + await vi.waitFor(() => expect(h.onShutdownError).toHaveBeenCalledOnce()) + expect(gate.isTerminalShutdownAdmitted()).toBe(true) + expect(h.services.disposeWorkflowBridge).not.toHaveBeenCalled() + expect(h.services.stopBuiltInMcp).not.toHaveBeenCalled() + expect(h.services.stopExternalControl).not.toHaveBeenCalled() + expect(h.onQuitAllowed).not.toHaveBeenCalled() + h.app.quit() + await vi.waitFor(() => expect(h.onQuitAllowed).toHaveBeenCalledOnce()) + expect(h.workflowStop).toHaveBeenCalledTimes(2) + expect(h.sessionStop).toHaveBeenCalledOnce() + expect(h.services.stopDictation).toHaveBeenCalledOnce() + }) + + it('retains completed support receipts when another support service rejects', async () => { + const h = harness() + h.services.stopRemote.mockRejectedValueOnce(new Error('remote stop failed')) + h.install() + h.app.quit() + await vi.waitFor(() => expect(h.onShutdownError).toHaveBeenCalledOnce()) + expect(h.services.disposeControl).not.toHaveBeenCalled() + h.app.quit() + await vi.waitFor(() => expect(h.onQuitAllowed).toHaveBeenCalledOnce()) + expect(h.services.stopRemote).toHaveBeenCalledTimes(2) + expect(h.services.stopLsp).toHaveBeenCalledOnce() + expect(h.services.stopExternalControl).toHaveBeenCalledOnce() + expect(h.sessionStop).toHaveBeenCalledOnce() + }) + + it('holds exit for required write settlement and retries a rejected drain without replaying stops', async () => { + const h = harness() + const writes = deferred() + h.services.drainWorkspace.mockImplementationOnce(() => writes.promise) + h.install() + h.app.quit() + await vi.waitFor(() => expect(h.services.drainWorkspace).toHaveBeenCalledOnce()) + expect(h.onQuitAllowed).not.toHaveBeenCalled() + writes.reject(new Error('pending save failed')) + await vi.waitFor(() => expect(h.onShutdownError).toHaveBeenCalledOnce()) + h.app.quit() + await vi.waitFor(() => expect(h.onQuitAllowed).toHaveBeenCalledOnce()) + expect(h.services.drainWorkspace).toHaveBeenCalledTimes(2) + expect(h.services.drainDictationHistory).toHaveBeenCalledOnce() + expect(h.sessionStop).toHaveBeenCalledOnce() + }) + + it('waits for partial startup and drains an owner published by its admitted initializer', async () => { + const h = harness() + const startup = deferred() + let workflowPublished = false + h.services.getSessions = () => null + h.services.getWorkflows = () => workflowPublished ? { stop: h.workflowStop } : null + h.services.startupSettled.mockImplementation(() => startup.promise) + h.install() + h.app.quit() + expect(h.workflowStop).not.toHaveBeenCalled() + expect(h.services.stopBuiltInMcp).not.toHaveBeenCalled() + workflowPublished = true + startup.resolve() + await vi.waitFor(() => expect(h.onQuitAllowed).toHaveBeenCalledOnce()) + expect(h.workflowStop).toHaveBeenCalledOnce() + expect(h.sessionStop).not.toHaveBeenCalled() + }) + + it('closes an initializing workflow immediately, while startup settlement still gates support disposal', async () => { + const h = harness() + const startup = deferred() + h.services.startupSettled.mockImplementation(() => startup.promise) + h.workflowStop.mockImplementation(() => startup.promise) + h.install() + h.app.quit() + expect(h.workflowStop).toHaveBeenCalledOnce() + expect(h.services.stopBuiltInMcp).not.toHaveBeenCalled() + startup.resolve() + await vi.waitFor(() => expect(h.onQuitAllowed).toHaveBeenCalledOnce()) + }) + + it('keeps macOS last-window closure live and sends other platforms through the same full composition', async () => { + const mac = harness() + mac.install('darwin') + mac.app.emit('window-all-closed') + expect(mac.app.quit).not.toHaveBeenCalled() + expect(mac.services.stopLsp).not.toHaveBeenCalled() + const linux = harness() + linux.install('linux') + linux.app.emit('window-all-closed') + await vi.waitFor(() => expect(linux.onQuitAllowed).toHaveBeenCalledOnce()) + expect(linux.sessionStop).toHaveBeenCalledOnce() + expect(linux.services.stopRemote).toHaveBeenCalledOnce() + }) + + it('reports diagnostic write failures after awaiting them without inventing native ownership uncertainty', async () => { + const h = harness() + const journal = deferred() + h.services.flushGhosts.mockImplementation(() => journal.promise) + h.install() + h.app.quit() + await vi.waitFor(() => expect(h.services.flushGhosts).toHaveBeenCalledOnce()) + expect(h.onQuitAllowed).not.toHaveBeenCalled() + journal.reject(new Error('debug disk full')) + await vi.waitFor(() => expect(h.onQuitAllowed).toHaveBeenCalledOnce()) + expect(h.onDiagnosticError).toHaveBeenCalledWith('ghosts', expect.any(Error)) + expect(h.onShutdownError).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/applicationShutdown.ts b/src/main/applicationShutdown.ts new file mode 100644 index 000000000..0761c046d --- /dev/null +++ b/src/main/applicationShutdown.ts @@ -0,0 +1,164 @@ +import { installSessionShutdownGate, type SessionShutdownGate } from './sessionShutdownGate' + +type Stop = () => void | Promise +type QuitApp = Parameters[0]['app'] & { + on(event: 'before-quit', listener: () => void): unknown +} + +export interface ApplicationShutdownServices { + getSessions(): { killAll(): Promise } | null + getWorkflows(): { stop(reason: string): Promise } | null + /** Settles only after startup can no longer acquire another resource. */ + startupSettled(): Promise + stopDictation: Stop + flushObservations: Stop + sweepOwnedProxies: Stop + stopBuiltInMcp: Stop + stopRemote: Stop + stopLsp: Stop + stopExternalControl: Stop + disposeControl: Stop + disposeWorkflowBridge: Stop + disposeCaffeinate: Stop + stopHeapWatchdog: Stop + drainWorkspace: Stop + drainDictationHistory: Stop + flushGhosts: Stop + flushRecordings: Stop + flushDictationDebug: Stop + flushPasteDebug: Stop + stopPerformance: Stop +} + +interface Stage { + promise: Promise + state: 'pending' | 'complete' | 'failed' +} + +/** + * The composition boundary matters more than the gate alone: a before-quit + * listener elsewhere can dismantle services even when the gate correctly + * honors the editor veto. Keep the complete disposal inventory here and wire + * concrete owners in index.ts; tests exercise this same listener installer. + * + * These receipts describe what each existing service API established. They do + * not upgrade a support-service dispose into observed native process exit, or + * an admission-tail drain into fsync durability. Session/workflow custody stays + * with their evidence-bearing lifecycle implementations. + */ +export function installApplicationShutdown(options: { + app: QuitApp + services: ApplicationShutdownServices + prepare: () => void + onQuitAllowed: () => void + onShutdownError: (error: unknown) => void + onDiagnosticError: (stage: string, error: unknown) => void + platform?: NodeJS.Platform +}): SessionShutdownGate { + const { services } = options + const stages = new Map() + + function run(name: string, action: Stop): Promise { + const prior = stages.get(name) + if (prior) return prior.promise + let resolve!: () => void + let reject!: (error: unknown) => void + const promise = new Promise((yes, no) => { resolve = yes; reject = no }) + const stage: Stage = { promise, state: 'pending' } + stages.set(name, stage) + // Attach rejection handling before action can fail, including while startup + // is still being joined. A rejected receipt is retained for THIS attempt; + // only a later explicit quit retries it. Successful stops are never replayed + // because a different service failed afterward. + void promise.then(() => { stage.state = 'complete' }, () => { stage.state = 'failed' }) + const fail = (error: unknown): void => reject(new Error( + `Shutdown stage ${name}: ${error instanceof Error ? error.message : String(error)}`, + { cause: error }, + )) + try { Promise.resolve(action()).then(resolve, fail) } catch (error) { fail(error) } + return promise + } + + function stopExecution(): Promise[] { + const sessions = services.getSessions() + const workflows = services.getWorkflows() + // Both APIs close mutation admission synchronously before their first await. + // Do not serialize them: a stalled workflow stop cannot prevent cancellation + // of interactive execution (or vice versa). Absent owners get no receipt: + // startup may still be publishing the already-admitted initializer's owner. + return [ + ...(sessions ? [run('sessions', () => sessions.killAll())] : []), + ...(workflows ? [run('workflows', () => workflows.stop('Agent Code is quitting'))] : []), + ] + } + + async function join(promises: Promise[]): Promise { + const results = await Promise.allSettled(promises) + const failures = results.flatMap(result => result.status === 'rejected' ? [result.reason] : []) + if (failures.length) throw new AggregateError(failures, 'Application shutdown has unfinished stages') + } + + async function drain(): Promise { + for (const [name, stage] of stages) if (stage.state === 'failed') stages.delete(name) + const earlyStops = stopExecution() + const dictationStop = run('dictation', services.stopDictation) + // Startup checks the committed gate after asynchronous acquisition. Its + // settlement closes the resource inventory; a missing SessionManager alone + // is not evidence that workflow/MCP/other startup resources never existed. + await services.startupSettled() + await join([...earlyStops, ...stopExecution(), dictationStop]) + await run('observations', services.flushObservations) + await run('proxy-sweep', services.sweepOwnedProxies) + + // Retain inspection/control infrastructure until native owners have drained. + // In particular, a failed WorkflowService.stop must leave its bridge usable + // for inspecting uncertainty instead of advertising a clean shutdown. + await join([ + run('builtin-mcp', services.stopBuiltInMcp), + run('remote', services.stopRemote), + run('lsp', services.stopLsp), + run('external-control', services.stopExternalControl), + ]) + await join([ + run('control', services.disposeControl), + run('workflow-bridge', services.disposeWorkflowBridge), + run('caffeinate', services.disposeCaffeinate), + run('heap-watchdog', services.stopHeapWatchdog), + ]) + await join([ + run('workspace', services.drainWorkspace), + run('dictation-history', services.drainDictationHistory), + ]) + + // Debug artifacts are not execution ownership or workspace save receipts. + // Await their queued work, but report a diagnostic write failure without + // misclassifying it as a provider that may still own a native conversation. + await Promise.all(([ + ['ghosts', services.flushGhosts], + ['recordings', services.flushRecordings], + ['dictation-debug', services.flushDictationDebug], + ['paste-debug', services.flushPasteDebug], + ] satisfies Array<[string, Stop]>).map(async ([name, action]) => { + try { await run(name, action) } + catch (error) { options.onDiagnosticError(name, error) } + })) + await run('performance', services.stopPerformance) + } + + // Preparation remains repeatable and reversible. Chromium may still veto + // after this callback. No service stop, admission fence, or recorder finalize + // belongs on this side of the editor decision. + options.app.on('before-quit', () => { + // Re-entering app.quit after the final drain must not enqueue fresh marks or + // observations behind the persistence snapshot we are about to release. + if (!gate.isTerminalShutdownAdmitted()) options.prepare() + }) + const gate = installSessionShutdownGate({ + app: options.app, + drain, + onQuitAllowed: options.onQuitAllowed, + onShutdownError: options.onShutdownError, + ...(options.platform ? { platform: options.platform } : {}), + }) + return gate +} diff --git a/src/main/dictation/historyStore.ts b/src/main/dictation/historyStore.ts index 7259b10d6..f206fb13b 100644 --- a/src/main/dictation/historyStore.ts +++ b/src/main/dictation/historyStore.ts @@ -274,13 +274,11 @@ function enqueue(operation: () => Promise): Promise { * a stop-handler that is still awaiting `transcribeBatch` — that row was never * enqueued. * - * At shutdown it is **best-effort, exactly like the debug journals**, NOT a - * guarantee. `before-quit` does not gate on the returned promise (doing so - * would mean preventDefault-ing the quit and re-entering it, which is a real - * risk of a hung quit in exchange for at most one bookkeeping row). So a - * dictation finished microseconds before ⌘Q can still be lost. Do not write a - * comment anywhere claiming otherwise — an earlier version of this file did, - * and the guarantee was fictional. + * Committed application shutdown joins already-admitted dictation handlers + * before awaiting this tail, so a batch response cannot enqueue behind that + * snapshot. This remains a settlement receipt: enqueue reports each write's + * rejection to its original caller and keeps this private queue alive. It is + * not proof that a failed write was retried or that the filesystem was fsynced. * * It IS load-bearing for the IPC handlers, which await it to serialise against * in-flight appends. diff --git a/src/main/index.ts b/src/main/index.ts index 0f22341f3..c1a7a5b32 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -29,7 +29,7 @@ import { applicationIdentityCapabilities } from '@main/window/identityControl.js import { conditionBackendCapabilities } from '@main/sessions/conditionControl.js' import { terminalBackendCapabilities } from '@main/sessions/terminalControl.js' import { windowLifecycleControlCapabilities } from '@main/window/lifecycleControl.js' -import { installSessionShutdownGate } from '@main/sessionShutdownGate.js' +import { installApplicationShutdown } from '@main/applicationShutdown.js' import { LspManager } from '@main/lspManager.js' import { compactAllGhostLogs, GhostJournalRegistry } from '@main/ghostJournal.js' import { @@ -71,6 +71,7 @@ import { sendToWindow, sessionsOwnedBy, setGeometryObserver, + setWindowCreationAdmission, setWindowCloseVetoedObserver, setWindowClosedObserver, transferSessions, @@ -120,7 +121,7 @@ import { import { getBuildInfo } from '@main/buildInfo.js' import { createWorkflowService } from '@main/workflows/createWorkflowService.js' import { WorkflowBridge } from '@main/workflows/WorkflowBridge.js' -import type { WorkflowService } from 'workflow-mcp' +import { WorkflowServiceError, type WorkflowService } from 'workflow-mcp' // Main process — thin Electron host. // @@ -185,13 +186,13 @@ const sessionRecorders = isSessionRecordingEnabled() : null if (sessionRecorders) setOutboundObserver(sessionRecorders.observe) // Per-dictation-session debug-dump registry. Mirrors `ghostJournals`: -// constructed before IPC handlers register, flushed on before-quit. See +// constructed before IPC handlers register, flushed after committed shutdown. See // `src/main/dictationJournal.ts` for the on-disk shape and the // rationale for cloning the ghost-journal pattern instead of refactoring // them into a single shared writer. const dictationDebugJournals = new DictationDebugJournalRegistry() // Per-paste debug-dump registry. Same lifecycle as dictationDebugJournals: -// constructed before IPC handlers register, flushed on before-quit, +// constructed before IPC handlers register, flushed after committed shutdown, // pruned on startup. Diagnostic for the "first Enter does nothing" // paste-submit bug; see docs/superpowers/plans/2026-05-11-paste-submit- // harness-findings-and-fix.md for context. @@ -253,8 +254,20 @@ let appRunJournal: AppRunJournal | null = null let workflowService: WorkflowService | null = null let workflowBridge: WorkflowBridge | null = null let codexCliUpdateReserved = false -let workflowShutdownPromise: Promise | null = null -let workflowShutdownComplete = false +// Keep partially initialized owners visible until committed shutdown joins +// startup. A missing SessionManager does not mean no service acquired resources. +let startupTask: Promise | null = null +let startupFailed = false +let disposeExternalControl: (() => Promise) | null = null +let disposeControlHost: (() => void) | null = null +let shutdownWorkspaceStore: WorkspaceFileStore | null = null + +class StartupInterruptedByQuit extends Error {} +function assertStartupOpen(): void { + if (sessionShutdownGate.isTerminalShutdownAdmitted()) { + throw new StartupInterruptedByQuit('Startup interrupted by committed quit') + } +} let sessionForwarder: SessionForwarderControl | null = null // A packaged release needs one executable-level smoke test that stops before @@ -316,9 +329,9 @@ async function runPackagingSmoke(): Promise { // is a window-modal sheet, so a vetoed quit never fires focus at all and the // flag stays latched forever, silently disabling the handoff. // -// The flag is instead cleared by the two paths that actually KNOW the quit -// failed: the sheet's "Keep Editing" branch (via the close-vetoed observer, -// wired in startApp) and the workflow-drain rejection below. +// The flag is instead cleared by the path that actually KNOWS the quit +// was vetoed: the sheet's "Keep Editing" branch (via the close-vetoed observer, +// wired in startApp). A committed drain failure must not resume window handoff. let quitting = false app.on('before-quit', () => { quitting = true }) @@ -338,38 +351,27 @@ if (packagingSmoke) { focusWindow(null) }) - void app.whenReady().then(startApp).catch((err) => { - // A throw out of startApp (toolchain or MCP-host init failure, or a disk - // error while the journal itself starts) would otherwise become an - // unhandledRejection: the process keeps running with no window and never - // quits, so `will-quit` never fires and the state-process lock is leaked - // while THIS pid stays alive. That makes the NEXT launch refuse to start — - // acquireStateProcessLock sees a live owner and shows "Agent Code is already - // running" until the zombie is force-killed. Convert a fatal startup error - // into a clean exit: journal it, flush + release the lock, and quit so a - // relaunch can proceed. We intentionally do NOT write the clean-shutdown - // marker — this run WAS unclean, and a future prior-run classifier should - // see it that way. - console.error('[app] fatal startup error — releasing lock and quitting:', err) - // Record as an INCIDENT (synchronous flush) so the failed boot lands in - // incidents.jsonl and the NEXT launch's classifier can attribute it — a plain - // event would only hit the async events.jsonl that never flushes before quit. - appRunJournal?.recordIncident({ - kind: 'app.startup_failed', - severity: 'fatal', - process: 'main', - error: err, + void app.whenReady().then(() => { + // A quit before readiness must not acquire a fresh resource after the exit + // drain captured an empty inventory. There is no startup task to join yet. + if (sessionShutdownGate.isTerminalShutdownAdmitted()) return + startupTask = startApp().catch(err => { + if (err instanceof StartupInterruptedByQuit) { + appRunJournal?.record({ area: 'app.lifecycle', name: 'app.startup_interrupted_by_quit' }) + return + } + startupFailed = true + console.error('[app] fatal startup error — draining owned resources before quitting:', err) + appRunJournal?.recordIncident({ + kind: 'app.startup_failed', severity: 'fatal', process: 'main', error: err, + }) + // Keep the journal and process lock until partial-startup resources drain. + // Releasing the lock here would permit a second main to start while this + // failed initializer still owned providers or asynchronous state writes. + // The final callback deliberately leaves failed boot without a clean mark. + app.quit() }) - appRunJournal?.stop() - // Null the handle BEFORE app.quit(): quit fires the will-quit handler, whose - // markCleanShutdown() would otherwise write the clean-shutdown marker and make - // this CRASHED boot look CLEAN on the next launch. (The crash-hook path uses - // process.exit, which bypasses will-quit; this path uses app.quit, which does - // NOT — hence the explicit null here, mirroring stateProcessLock below.) - appRunJournal = null - stateProcessLock?.releaseSync() - stateProcessLock = null - app.quit() + return startupTask }) } @@ -435,6 +437,7 @@ async function startApp(): Promise { return } stateProcessLock = lock + assertStartupOpen() appRunJournal = new AppRunJournal({ appVersion: app.getVersion(), // Build provenance (#374): git SHA / branch / dirty / timestamp / mode / @@ -473,6 +476,7 @@ async function startApp(): Promise { // folder as prunable, which is correct because none can be live. if (sessionRecorders) setLiveRecordingDirsProvider(() => sessionRecorders.liveRecordingDirs()) await appRunJournal.start() + assertStartupOpen() appRunJournal.record({ area: 'state.lock', name: 'state_lock.acquired', @@ -647,6 +651,7 @@ async function startApp(): Promise { appRunJournal.record({ area: 'setup.toolchain', name: 'toolchain.start' }) try { await initializeToolchain() + assertStartupOpen() appRunJournal.record({ area: 'setup.toolchain', name: 'toolchain.end' }) } catch (err) { appRunJournal.recordError('toolchain.error', err) @@ -656,14 +661,25 @@ async function startApp(): Promise { try { workflowService = await createWorkflowService({ isCodexCliUpdateReserved: () => codexCliUpdateReserved, + onCreated: service => { workflowService = service }, }) + assertStartupOpen() workflowBridge = new WorkflowBridge(workflowService) // Recovery successors may be created during service.initialize(), before the bridge exists. // Await rehydration so the first renderer query sees the durable lineage owner instead of a // stale parent with a misleading Resume action. await workflowBridge.start() + assertStartupOpen() appRunJournal.record({ area: 'workflows.service', name: 'workflow_service.ready' }) } catch (err) { + // WorkflowService.stop closes recovery admission inside initialize. That + // typed closing outcome is our own committed quit, not a failed boot. Keep + // real initialization/storage failures on the incident path below. + if (sessionShutdownGate.isTerminalShutdownAdmitted() + && err instanceof WorkflowServiceError + && (err.code === 'service-stopping' || err.code === 'service-stopped')) { + throw new StartupInterruptedByQuit('Workflow initialization interrupted by committed quit') + } // Workflow persistence is part of the execution contract, not a cosmetic // renderer enhancement. Starting the MCP host without its durable service // would advertise a toggle that either loses runs or fails every tool call; @@ -676,6 +692,7 @@ async function startApp(): Promise { performanceService.error('app.main.imageCache.cleanup.error', err) appRunJournal?.recordError('image_cache.cleanup.error', err) }) + assertStartupOpen() // Tmux availability is checked once at startup. The cost is a // child-process roundtrip on `tmux -V` — cheap enough to await // before any IPC is wired. Result is cached on the registry; call @@ -695,6 +712,7 @@ async function startApp(): Promise { // same as a machine without tmux installed. No silent // system-tmux usage, no PATH lookup, no sentinel-string trickery. const bundledTmux = await resolveBundledTool('tmux') + assertStartupOpen() tmuxRegistry = new TmuxRegistry({ tmuxBinary: bundledTmux ?? undefined }) const tmuxDetectStarted = performance.now() appRunJournal.record({ @@ -703,6 +721,7 @@ async function startApp(): Promise { data: { bundled: bundledTmux !== null }, }) const tmuxAvailable = await tmuxRegistry.detectAvailability() + assertStartupOpen() appRunJournal.record({ area: 'app.tmux', name: 'tmux.detect.end', @@ -734,6 +753,7 @@ async function startApp(): Promise { try { appRunJournal.record({ area: 'app.tmux', name: 'tmux.recovery.start' }) const raw = await readFile(STATE_FILE, 'utf8') + assertStartupOpen() // workspace.json is wrapped: { workspace: { sessions: {...} } }. // The renderer's saveWorkspace() writes { workspace: workspaceState } // — so persisted sessions live one level deep, not at the root. @@ -779,6 +799,7 @@ async function startApp(): Promise { } } + assertStartupOpen() // Give the host its journal BEFORE start() so a bind failure can record its // mcp.host_start_failed incident — setDependencies() (which also carries the // journal) only runs AFTER start(), because it needs `manager`, so without this @@ -787,6 +808,7 @@ async function startApp(): Promise { appRunJournal.record({ area: 'mcp.host', name: 'mcp_host.start' }) try { await builtInMcpHost.start() + assertStartupOpen() appRunJournal.record({ area: 'mcp.host', name: 'mcp_host.end' }) } catch (err) { appRunJournal.recordError('mcp_host.error', err) @@ -794,6 +816,7 @@ async function startApp(): Promise { } const agentCodeConventionsService = new AgentCodeManagedSkillsService() await agentCodeConventionsService.initialize() + assertStartupOpen() manager = new SessionManager( tmuxAvailable ? tmuxRegistry : null, builtInMcpHost, @@ -871,8 +894,10 @@ async function startApp(): Promise { ...workflowControlCapabilities(activeWorkflowService, invokeTask), ...usageControlCapabilities(), ...applicationIdentityCapabilities(), ...sessionHistoryControlCapabilities(), ...nativeHistoryControlCapabilities(() => conversationService), ...conditionBackendCapabilities(controlManager), ...terminalBackendCapabilities(controlManager), ...windowLifecycleControlCapabilities(), ...externalSettings.capabilities, ]) externalHost = new ExternalControlMcpHost(controlHost.forCaller({ kind: 'external', id: 'agent-code-control' })) + disposeExternalControl = () => externalSettings.dispose() + disposeControlHost = () => controlHost.dispose() await externalSettings.initialize() - app.once('will-quit', () => { void externalSettings.dispose(); controlHost.dispose() }) + assertStartupOpen() const tldrStore = new TldrStore(join(STATE_DIR, 'tldr.json')) const tldrEnforcement = new TldrEnforcement(tldrStore) // Before any session can register: the sweep removes every entry, and each @@ -880,6 +905,7 @@ async function startApp(): Promise { await sweepStaleTldrHookFiles(TLDR_HOOK_RUNTIME_DIR).catch(error => { console.warn('[tldr] stale hook file sweep failed:', error) }) + assertStartupOpen() registerTldrIpc(tldrStore, tldrEnforcement) builtInMcpHost.setDependencies({ tldrStore, @@ -943,11 +969,14 @@ async function startApp(): Promise { }, }) await cliUpdateOrchestrator.loadInitialBehavior() + assertStartupOpen() // WHY the workspace file is read here, before any window exists: it now holds // the window list, so it is what decides how many windows to create. The old // renderer-driven `workspace:load` could not answer that — it required a // renderer, which requires a window. const workspaceFileStore = await WorkspaceFileStore.open() + shutdownWorkspaceStore = workspaceFileStore + assertStartupOpen() // Conversation ledger (docs/decomposition/conversations.md, Stage 3): a // projection of every window's sessions keyed by native id, so the picker // can name and classify conversations after their panes are gone. Boots @@ -957,6 +986,7 @@ async function startApp(): Promise { console.warn('[conversations] ledger unavailable', error) return null }) + assertStartupOpen() if (conversationLedger) { const projectConversations = (windows: readonly PersistedWindow[]) => { void readAgentNameAssignments(AGENT_NAMES_FILE) @@ -1144,144 +1174,74 @@ async function startApp(): Promise { }) } -app.on('before-quit', (event) => { - // WHY Electron quit is gated on WorkflowService.stop(): the durable service - // promises that every published event was appended first, but cancellation - // and the terminal/interrupted marker still require asynchronous file I/O. - // A fire-and-forget stop here would let Electron tear main down between - // those writes, leaving a healthy user-initiated quit indistinguishable from - // a crash. Prevent exactly the first quit, drain once, then re-enter quit - // with the completion flag set so the ordinary lifecycle can finish. - if (workflowService && !workflowShutdownComplete) { - event.preventDefault() - if (!workflowShutdownPromise) { - workflowShutdownPromise = workflowService - .stop('Agent Code is quitting') - .catch(err => { - // An unconfirmed provider may still own descendants. Treating this as a warning and - // immediately calling app.quit() defeats Workflow MCP's fail-closed ownership fence. - // Keep Electron alive, retain the bridge for diagnostics, and let a later quit retry once - // the provider settles (or let the user make an explicit OS-level force-quit decision). - console.error('[workflows] graceful shutdown blocked:', err) - appRunJournal?.recordError('workflow_service.stop.error', err) - workflowShutdownPromise = null - void dialog.showMessageBox({ - type: 'error', - title: 'Agent work is still shutting down', - message: 'Agent Code could not safely quit yet.', - detail: err instanceof Error ? err.message : String(err), - buttons: ['Keep Agent Code Open'], - defaultId: 0, - cancelId: 0, - noLink: true, - }) - throw err - }) - .then(() => { - workflowBridge?.dispose() - workflowShutdownComplete = true - workflowBridge = null - workflowService = null - app.quit() - }) - .catch(() => undefined) - } - return - } - appRunJournal?.record({ area: 'app.lifecycle', name: 'app.before_quit' }) - performanceService.mark('app.main.beforeQuit') - // WHY coalescers drain on the initial quit attempt: their buffers are cheap - // and safe to flush even when a renderer veto keeps the app alive. Terminal - // SessionManager teardown is deliberately deferred to will-quit below, - // because unlike a coalescer flush it cannot be rolled back after Keep Editing. - sessionForwarder?.flush() - void builtInMcpHost.stop() - void remoteController?.dispose() - void lspManager.dispose() - caffeinateController.dispose() - cleanupDictationIpcResources() - stopMainHeapWatchdog() - // Flush pending ghost writes. Fire-and-forget is fine — Electron's - // quit path gives us a tick before teardown. 100 ms queue depth is - // worst-case; in practice drains are empty at quit time because - // streaming is idle. - void ghostJournals.flushAll() - // Same one-tick-before-teardown rationale as ghostJournals; recordings are - // usually mid-stream at quit, so this drain matters more than the ghost one. - void sessionRecorders?.flushAll() - // Same rationale as ghostJournals — Electron gives us one tick before - // teardown. 100 ms queue depth is the worst case; in practice the - // dictation journal is idle at quit unless the user is pressing Fn - // at the exact moment of app shutdown. - void dictationDebugJournals.flushAll() - // Dictation HISTORY is a separate store from the debug journal above, and its - // flush is load-bearing rather than best-effort: `appendEntry` is called - // without await from the stream-stop handler (so a disk write never delays - // the transcript reaching the composer), which means a dictation finished - // seconds before quit can still be in flight right now. Without this the row - // is simply lost, with no error anywhere. See historyStore.ts. - void flushHistoryWrites() - void pasteDebugJournals.flushAll() - performanceService.stop() -}) - -const sessionShutdownGate = installSessionShutdownGate({ +// One composition owns every committed-quit disposer. Electron's before-quit +// precedes renderer unload decisions, so only reversible preparation belongs +// there. In particular, Keep Editing must leave workflow/MCP/LSP/remote/voice +// services intact. The gate holds will-quit until this inventory has settled. +const sessionShutdownGate = installApplicationShutdown({ app, - getManager: () => { - const current = manager - if (!current) return null - return { - killAll: async () => { - await current.killAll() - // WHY a second sweep after killAll: killAll stops the sessions it - // knows about, and each ClaudeSession.stop() already terminates its - // own mitmdump under a deadline. This catches what that snapshot - // cannot — a proxy whose session was mid-start when shutdown began, - // or a stop() that gave up — by asking the kernel which marked - // mitmdumps still have THIS process as parent. Composed here rather - // than inside SessionManager or the gate so neither learns about a - // Claude-specific child process. Never rejects: a failed sweep must - // not hold quit (the gate is fail-closed on rejection), and the - // startup reaper on the next launch is the backstop anyway. - try { - const report = await reapOwnedMitmproxyProcesses() - if (report.owned > 0) { - appRunJournal?.record({ - area: 'proxy', - name: 'proxy.mitmdump.quit_sweep', - severity: 'warn', - data: { ...report }, - }) - } - } catch (err) { - appRunJournal?.recordError('proxy.mitmdump.quit_sweep.error', err) - } - }, - } - }, platform: process.platform, - onLastWindowClosed: () => { - // WHY these provider-neutral resources still stop at last-window close on - // non-macOS: this preserves the established cleanup timing while the - // shutdown gate remains the exclusive owner of session/provider teardown. - // The built-in MCP host intentionally remains app-owned until before-quit. - void remoteController?.dispose() - void lspManager.dispose() - caffeinateController.dispose() + prepare: () => { + appRunJournal?.record({ area: 'app.lifecycle', name: 'app.before_quit' }) + performanceService.mark('app.main.beforeQuit') + sessionForwarder?.flush() + }, + services: { + getSessions: () => manager, + getWorkflows: () => workflowService, + startupSettled: () => startupTask ?? Promise.resolve(), + stopDictation: cleanupDictationIpcResources, + flushObservations: () => sessionForwarder?.flush(), + sweepOwnedProxies: async () => { + // Keep the existing best-effort backstop AFTER managed session teardown. + // This is a kernel sweep of our marked children, not proof that an + // uncertain provider stop succeeded; that stop already gates this stage. + try { + const report = await reapOwnedMitmproxyProcesses() + if (report.owned > 0) appRunJournal?.record({ + area: 'proxy', name: 'proxy.mitmdump.quit_sweep', severity: 'warn', data: { ...report }, + }) + } catch (error) { appRunJournal?.recordError('proxy.mitmdump.quit_sweep.error', error) } + }, + stopBuiltInMcp: () => builtInMcpHost.stop(), + stopRemote: () => remoteController?.dispose(), + stopLsp: () => lspManager.dispose(), + stopExternalControl: () => disposeExternalControl?.(), + disposeControl: () => disposeControlHost?.(), + disposeWorkflowBridge: () => workflowBridge?.dispose(), + disposeCaffeinate: () => caffeinateController.dispose(), + stopHeapWatchdog: stopMainHeapWatchdog, + drainWorkspace: () => shutdownWorkspaceStore?.drainAdmittedWrites(), + drainDictationHistory: flushHistoryWrites, + flushGhosts: () => ghostJournals.flushAll(), + flushRecordings: () => sessionRecorders?.flushAll(), + flushDictationDebug: () => dictationDebugJournals.flushAll(), + flushPasteDebug: () => pasteDebugJournals.flushAll(), + stopPerformance: () => performanceService.stop(), }, onQuitAllowed: () => { appRunJournal?.record({ area: 'app.lifecycle', name: 'app.will_quit' }) - appRunJournal?.markCleanShutdown('will-quit') + if (!startupFailed) appRunJournal?.markCleanShutdown('will-quit') appRunJournal?.stop() stateProcessLock?.releaseSync() stateProcessLock = null }, onShutdownError: error => { - // WHY a rejected terminal drain blocks quit: SessionManager owns exact - // transcript leases and in-flight recovery claims. Exiting while their - // teardown is uncertain recreates the cross-process ownership ambiguity - // this PR is designed to eliminate. A later explicit quit retries. - console.error('[sessions] graceful shutdown blocked:', error) - appRunJournal?.recordError('session_manager.kill_all.error', error) + console.error('[app] graceful shutdown blocked:', error) + appRunJournal?.recordError('app.shutdown.error', error) + if (!app.isReady()) return + void dialog.showMessageBox({ + type: 'error', title: 'Agent work is still shutting down', + message: 'Agent Code could not safely quit yet.', + detail: 'Shutdown is incomplete. Quit again to retry.\n\n' + (error instanceof AggregateError + ? error.errors.map(cause => cause instanceof Error ? cause.message : String(cause)).join('\n') + : error instanceof Error ? error.message : String(error)), + buttons: ['Keep Agent Code Open'], defaultId: 0, cancelId: 0, noLink: true, + }).catch(reportError => console.error('[app] could not show shutdown error:', reportError)) }, + onDiagnosticError: (stage, error) => appRunJournal?.recordError(`app.shutdown.${stage}.error`, error), }) +// Cover menu, IPC, external control and restoration through their shared factory, +// not just the macOS activate callback. Failed committed shutdown is terminal; +// a fresh renderer's unload veto cannot make partially stopped services usable. +setWindowCreationAdmission(() => !sessionShutdownGate.isTerminalShutdownAdmitted()) diff --git a/src/main/ipc/dictation.ts b/src/main/ipc/dictation.ts index 33f1bddae..33d9a7a24 100644 --- a/src/main/ipc/dictation.ts +++ b/src/main/ipc/dictation.ts @@ -1,4 +1,4 @@ -import { app, ipcMain } from 'electron' +import { app, ipcMain, type IpcMainInvokeEvent } from 'electron' import { createHash, randomUUID } from 'node:crypto' import { appendFileSync, writeFileSync } from 'node:fs' import { join } from 'node:path' @@ -58,6 +58,26 @@ type ActiveDictationSession = { } const activeSessions = new Map() +let shutdownAdmitted = false +const pendingOperations = new Set>() + +function trackPending(promise: Promise): Promise { + pendingOperations.add(promise) + void promise.then( + () => { pendingOperations.delete(promise) }, + () => { pendingOperations.delete(promise) }, + ) + return promise +} + +function admittedDictation( + handler: (...args: Args) => Promise, +): (...args: Args) => Promise { + return (...args) => { + if (shutdownAdmitted) return Promise.reject(new Error('Agent Code is shutting down')) + return trackPending(handler(...args)) + } +} // First-8-hex-chars of SHA-256 over a chunk. Used purely as a fingerprint // for cross-process correlation: if the same `sha8` appears in the @@ -175,9 +195,14 @@ export function registerDictationIpc(deps: { ipcMain.handle('dictation:history-clear', async () => clearEntries()) ipcMain.handle('dictation:history-reset-totals', async () => resetTotals()) - ipcMain.handle('dictation:hotkey-configure', async (_evt, params: { binding?: string }) => { + ipcMain.handle('dictation:hotkey-configure', admittedDictation(async (_evt: IpcMainInvokeEvent, params: { binding?: string }) => { try { + if (shutdownAdmitted) throw new Error('Agent Code is shutting down') const result = await configureDictationHotkey(params.binding ?? '') + if (shutdownAdmitted) { + unregisterDictationHotkey() + throw new Error('Agent Code is shutting down') + } if (!result.ok && result.message) { // Durable breadcrumb for the graceful degrade (#495 A4). The // renderer also receives `result.message`, but its console.warn is @@ -201,7 +226,7 @@ export function registerDictationIpc(deps: { message: err instanceof Error ? err.message : 'Could not configure dictation hotkey.', } } - }) + })) // Fire-and-forget journal write from the renderer. We use `ipcMain.on` // (not `handle`) because the renderer side is fire-and-forget; we don't @@ -222,8 +247,8 @@ export function registerDictationIpc(deps: { ipcMain.handle( 'dictation:stream-start', - async ( - evt, + admittedDictation(async ( + evt: IpcMainInvokeEvent, params: { provider: DictationProvider; mimeType?: string; debugSessionId?: string }, ) => { const debugSessionId = params.debugSessionId ?? null @@ -243,6 +268,9 @@ export function registerDictationIpc(deps: { } const apiKey = await readDeepgramApiKeyForRuntime() + // The key lookup may finish after cleanup closed admission and cleared + // the active map. Never publish a late socket/session behind that drain. + if (shutdownAdmitted) return { kind: 'error', message: 'Agent Code is shutting down.' } if (!apiKey) { emit(debugSessionId, 'ERROR', 'stream-start:rejected', { reason: 'missing-api-key', @@ -345,7 +373,7 @@ export function registerDictationIpc(deps: { } return { kind: 'started', id } - }, + }), ) ipcMain.handle( @@ -422,7 +450,7 @@ export function registerDictationIpc(deps: { ipcMain.handle( 'dictation:stream-stop', - async (_evt, params: { id: string; audioDurationMs?: number }) => { + admittedDictation(async (_evt: IpcMainInvokeEvent, params: { id: string; audioDurationMs?: number }) => { const session = activeSessions.get(params.id) if (!session) { return { kind: 'error', message: 'Dictation session is no longer active.' } @@ -456,6 +484,7 @@ export function registerDictationIpc(deps: { return null }) : Promise.resolve(null) + trackPending(streamingStop) if (DICTATION_DUMP_ENABLED) { // eslint-disable-next-line no-console @@ -525,7 +554,8 @@ export function registerDictationIpc(deps: { // putting a disk write between the provider answering and the composer // filling would make dictation feel slower than it is, in exchange for // bookkeeping they cannot see. The store serialises its own writes, and - // `flushHistoryWrites()` on before-quit covers the dictate-then-⌘Q race. + // Committed shutdown joins this handler before draining history writes; + // a renderer veto leaves both the handler and the service intact. // // Raw text, never the -wrapped form: the wrapper is a delivery // concern for the LIVE prompt, and baking today's tag format into every @@ -628,7 +658,7 @@ export function registerDictationIpc(deps: { message: err instanceof Error ? err.message : 'Dictation failed.', } } - }, + }), ) ipcMain.handle('dictation:stream-cancel', async (_evt, params: { id: string }) => { @@ -646,9 +676,18 @@ export function registerDictationIpc(deps: { }) } -export function cleanupDictationIpcResources(): void { +export async function cleanupDictationIpcResources(): Promise { + shutdownAdmitted = true unregisterDictationHotkey() + for (const session of activeSessions.values()) { + if (session.streamingId) deepgramStreaming().cancel(session.streamingId) + } activeSessions.clear() + // A stop handler removes its active entry BEFORE batch HTTP finishes. The + // map alone cannot enumerate pending transcripts/history producers. Join + // those already-admitted handlers before the separate history-store drain; + // its eventual enqueue must precede our final persistence snapshot. + while (pendingOperations.size) await Promise.allSettled([...pendingOperations]) } // WHY the old readDeepgramApiKey() env-only helper is gone: diff --git a/src/main/ipc/dictationShutdown.test.ts b/src/main/ipc/dictationShutdown.test.ts new file mode 100644 index 000000000..dba480e62 --- /dev/null +++ b/src/main/ipc/dictationShutdown.test.ts @@ -0,0 +1,99 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + handlers: new Map Promise>(), + key: vi.fn(), batch: vi.fn(), append: vi.fn(), unregister: vi.fn(), configure: vi.fn(), + preview: { start: vi.fn(), stop: vi.fn(), cancel: vi.fn(), pushChunk: vi.fn() }, +})) +vi.mock('electron', () => ({ app: { getPath: () => '/unused' }, ipcMain: { + handle: (name: string, handler: (...args: any[]) => Promise) => mocks.handlers.set(name, handler), on: vi.fn(), +} })) +vi.mock('@main/dictation/index.js', () => ({ deepgramStreaming: () => mocks.preview, transcribeBatch: mocks.batch })) +vi.mock('@main/dictation/hotkey.js', () => ({ unregisterDictationHotkey: mocks.unregister, configureDictationHotkey: mocks.configure })) +vi.mock('@main/dictation/apiKeyStore.js', () => ({ readDeepgramApiKeyForRuntime: mocks.key, + getDeepgramApiKeyStatus: vi.fn(), setDeepgramApiKey: vi.fn() })) +vi.mock('@main/dictation/historyStore.js', () => ({ appendEntry: mocks.append, + clearEntries: vi.fn(), deleteEntry: vi.fn(), readHistory: vi.fn(), resetTotals: vi.fn() })) +vi.mock('@main/window/windowRegistry.js', () => ({ windowIdFor: () => 'window', sendToWindow: vi.fn() })) + +function deferred() { + let resolve!: (value: T) => void + const promise = new Promise(yes => { resolve = yes }) + return { promise, resolve } +} +function invoke(name: string, params: unknown) { + const handler = mocks.handlers.get(`dictation:${name}`) + if (!handler) throw new Error(`Handler missing: ${name}`) + return handler({ sender: {} }, params) +} + +beforeEach(() => { + vi.resetModules() + vi.resetAllMocks() + mocks.handlers.clear() + mocks.key.mockResolvedValue('test-only-key') + mocks.preview.start.mockReturnValue({ id: 'preview' }) + mocks.preview.stop.mockResolvedValue(null) + mocks.append.mockResolvedValue(undefined) +}) + +async function setup() { + const module = await import('./dictation') + module.registerDictationIpc({ dictationDebugJournals: { get: () => ({ append: vi.fn() }) }, + appRunJournal: { record: vi.fn() } } as never) + return module +} + +describe('dictation IPC shutdown ownership', () => { + it('does not publish a stream after shutdown overtakes key resolution', async () => { + const key = deferred() + mocks.key.mockReturnValue(key.promise) + const module = await setup() + const start = invoke('stream-start', { provider: 'deepgram' }) + const settled = vi.fn() + const shutdown = module.cleanupDictationIpcResources().then(settled) + expect(settled).not.toHaveBeenCalled() + key.resolve('test-only-key') + expect(await start).toMatchObject({ kind: 'error' }) + await shutdown + expect(mocks.preview.start).not.toHaveBeenCalled() + await expect(invoke('stream-start', { provider: 'deepgram' })).rejects.toThrow('shutting down') + }) + + it('cancels active previews and keeps pending batch/history producers inside the drain', async () => { + const module = await setup() + const batch = deferred<{ kind: 'ok'; raw: string }>() + const preview = deferred() + mocks.batch.mockReturnValue(batch.promise) + mocks.preview.stop.mockReturnValue(preview.promise) + const { id } = await invoke('stream-start', { provider: 'deepgram' }) + await invoke('stream-chunk', { id, chunk: new ArrayBuffer(4) }) + const stop = invoke('stream-stop', { id, audioDurationMs: 1000 }) + expect(mocks.batch).toHaveBeenCalledOnce() + // A second utterance is still active while the first batch owns work that + // has already disappeared from activeSessions. Both lifetimes must drain. + await invoke('stream-start', { provider: 'deepgram' }) + const settled = vi.fn() + const shutdown = module.cleanupDictationIpcResources().then(settled) + expect(mocks.preview.cancel).toHaveBeenCalledWith('preview') + batch.resolve({ kind: 'ok', raw: 'recoverable dictation' }) + await stop + expect(mocks.append).toHaveBeenCalledWith(expect.objectContaining({ text: 'recoverable dictation' })) + expect(settled).not.toHaveBeenCalled() + preview.resolve(null) + await shutdown + expect(settled).toHaveBeenCalledOnce() + }) + + it('removes a hotkey installed by a configuration request that finishes after shutdown', async () => { + const module = await setup() + const configured = deferred<{ ok: boolean; binding: string }>() + mocks.configure.mockReturnValue(configured.promise) + const request = invoke('hotkey-configure', { binding: 'Fn' }) + const shutdown = module.cleanupDictationIpcResources() + configured.resolve({ ok: true, binding: 'Fn' }) + expect(await request).toMatchObject({ ok: false }) + await shutdown + expect(mocks.unregister).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/main/menu/appMenu.ts b/src/main/menu/appMenu.ts index 7345eab8c..412707906 100644 --- a/src/main/menu/appMenu.ts +++ b/src/main/menu/appMenu.ts @@ -3,6 +3,7 @@ import type { MenuItemConstructorOptions } from 'electron' import { createAppWindow, + isWindowCreationAllowed, sendToFocusedWindow, zoomFocusedWindow, } from '@main/window/windowRegistry.js' @@ -80,7 +81,7 @@ export function buildAppMenu(): Menu { // windows per press. label: 'New Window', click: () => { - createAppWindow() + if (isWindowCreationAllowed()) createAppWindow() }, }, { type: 'separator' }, diff --git a/src/main/recording/SessionRecorderManager.ts b/src/main/recording/SessionRecorderManager.ts index 29298ea0f..c24dcfafa 100644 --- a/src/main/recording/SessionRecorderManager.ts +++ b/src/main/recording/SessionRecorderManager.ts @@ -503,7 +503,7 @@ export class SessionRecorderManager { return this.stop(sessionId) } - /** Drain + finalize every recording. Called on before-quit (mirrors + /** Drain + finalize every recording. Called after committed producer shutdown (mirrors * ghostJournals.flushAll). */ async flushAll(): Promise { for (const { timer } of this.pendingStops.values()) clearTimeout(timer) diff --git a/src/main/remote/RemoteController.integration.test.ts b/src/main/remote/RemoteController.integration.test.ts index 0cf80a3fd..42660e67c 100644 --- a/src/main/remote/RemoteController.integration.test.ts +++ b/src/main/remote/RemoteController.integration.test.ts @@ -51,6 +51,43 @@ afterEach(async () => { }) describe('RemoteController lifecycle', () => { + it('joins an admitted slow enable and prevents publication after disposal', async () => { + let release!: () => void + const starting = new Promise(resolve => { release = resolve }) + const start = vi.fn(async () => { await starting; return { url: 'http://127.0.0.1:12345' } }) + const stop = vi.fn(async () => undefined) + controller = new RemoteController({ manager: makeManager() as never, stateDir: dir, + createTransport: () => ({ start, stop }) }) + const enabled = controller.enable().catch(error => error) + await vi.waitFor(() => expect(start).toHaveBeenCalledOnce()) + const settled = vi.fn() + const disposal = controller.dispose() + void disposal.then(settled) + expect(controller.dispose()).toBe(disposal) + await expect(controller.enable()).rejects.toThrow('shutting down') + expect(settled).not.toHaveBeenCalled() + release() + expect(await enabled).toBeInstanceOf(Error) + await disposal + expect(controller.getStatus()).toMatchObject({ enabled: false, url: null }) + expect(stop).toHaveBeenCalledOnce() + }) + + it('retains the exact failed transport owner for a later disposal retry', async () => { + const stop = vi.fn<() => Promise>() + .mockRejectedValueOnce(new Error('transport release unconfirmed')).mockResolvedValue(undefined) + const createTransport = vi.fn(() => ({ + start: async () => ({ url: 'http://127.0.0.1:12345' }), stop, + })) + controller = new RemoteController({ manager: makeManager() as never, stateDir: dir, createTransport }) + await controller.enable() + await expect(controller.dispose()).rejects.toThrow('transport release unconfirmed') + await expect(controller.enable()).rejects.toThrow('shutting down') + await controller.dispose() + expect(stop).toHaveBeenCalledTimes(2) + expect(createTransport).toHaveBeenCalledOnce() + }) + it('starts disabled with no URL', () => { expect(controller.getStatus()).toMatchObject({ enabled: false, url: null }) }) diff --git a/src/main/remote/RemoteController.ts b/src/main/remote/RemoteController.ts index 880567cf0..8c9d6bf65 100644 --- a/src/main/remote/RemoteController.ts +++ b/src/main/remote/RemoteController.ts @@ -131,6 +131,8 @@ export class RemoteController extends EventEmitter { * the whole class: later calls observe the state their predecessors left * and no-op when it already matches. */ private chain: Promise = Promise.resolve() + private disposalAdmitted = false + private disposalPromise: Promise | null = null constructor(private readonly deps: RemoteControllerDeps) { super() @@ -153,13 +155,15 @@ export class RemoteController extends EventEmitter { } async enable(mode: RemoteTransportMode = 'lan'): Promise { + if (this.disposalAdmitted) throw new Error('Remote access is shutting down') return this.runExclusive(async () => { + if (this.disposalAdmitted) throw new Error('Remote access is shutting down') // Same mode already live: idempotent no-op. Different mode: clean // switch — every socket drops (URL and reachability change anyway; // phones reconnect via their feed's backoff, and paired tokens // survive because pairing is durable state). - if (this.server && this.url) { - if (this.transport === mode) return this.getStatus() + if (this.server) { + if (this.url && this.transport === mode) return this.getStatus() await this.teardownLive() this.emit('status-changed', this.getStatus()) } @@ -225,6 +229,7 @@ export class RemoteController extends EventEmitter { this.transport = mode this.server.on('clients-changed', () => this.emitStatus()) const { url } = await this.server.start() + if (this.disposalAdmitted) throw new Error('Remote access is shutting down') this.url = url } catch (err) { // Whatever partially came up, tear it ALL down so a retry starts @@ -295,9 +300,22 @@ export class RemoteController extends EventEmitter { return revoked } - async dispose(): Promise { - await this.teardownLive() - this.removeAllListeners() + dispose(): Promise { + this.disposalAdmitted = true + if (this.disposalPromise) return this.disposalPromise + // Disposal must join the SAME FIFO as enable/disable. Stopping outside it + // could observe no server while a slow enable was still preparing its + // secret/transport, then return before that initializer published a server. + const disposal = this.runExclusive(async () => { + await this.teardownLive() + this.removeAllListeners() + }) + this.disposalPromise = disposal + void disposal.catch(() => { + // Keep failed resource owners below, but allow an explicit shutdown retry. + if (this.disposalPromise === disposal) this.disposalPromise = null + }) + return disposal } /** Build the transport for a mode. LAN needs nothing; tunnel resolves the @@ -321,11 +339,13 @@ export class RemoteController extends EventEmitter { private async teardownLive(): Promise { const server = this.server - this.server = null this.url = null + if (server) await server.stop() + // A rejection is not release evidence. Keep this exact server reachable so + // a retry cannot mistake a cleared registry field for successful teardown. + this.server = null this.transport = null this.pairing = null - if (server) await server.stop() this.feedSource?.dispose() this.feedSource = null } diff --git a/src/main/sessionShutdownGate.test.ts b/src/main/sessionShutdownGate.test.ts index 7ce15bcb3..6311355b0 100644 --- a/src/main/sessionShutdownGate.test.ts +++ b/src/main/sessionShutdownGate.test.ts @@ -52,6 +52,36 @@ function createFakeApp(): { } describe('installSessionShutdownGate', () => { + it('holds synchronous throws behind the committed gate and allows an explicit retry', async () => { + const fake = createFakeApp() + const failure = new Error('synchronous stop failure') + const drain = vi.fn<() => Promise>().mockImplementationOnce(() => { throw failure }) + .mockResolvedValue(undefined) + const onShutdownError = vi.fn() + const gate = installSessionShutdownGate({ app: fake.app, drain, onShutdownError, onQuitAllowed: vi.fn() }) + expect(() => fake.emitWillQuit()).not.toThrow() + await vi.waitFor(() => expect(onShutdownError).toHaveBeenCalledWith(failure)) + expect(gate.isTerminalShutdownAdmitted()).toBe(true) + expect(fake.app.quit).not.toHaveBeenCalled() + fake.emitWillQuit() + await vi.waitFor(() => expect(fake.app.quit).toHaveBeenCalledOnce()) + expect(drain).toHaveBeenCalledTimes(2) + }) + + it('publishes the exact join before a stop can synchronously request quit again', async () => { + const fake = createFakeApp() + const pending = deferred() + const drain = vi.fn(() => { + fake.emitWillQuit() + return pending.promise + }) + installSessionShutdownGate({ app: fake.app, drain, onQuitAllowed: vi.fn() }) + fake.emitWillQuit() + expect(drain).toHaveBeenCalledOnce() + pending.resolve() + await vi.waitFor(() => expect(fake.app.quit).toHaveBeenCalledOnce()) + }) + it('leaves the manager usable when Keep Editing prevents will-quit', async () => { const fake = createFakeApp() const onQuitAllowed = vi.fn() @@ -68,7 +98,7 @@ describe('installSessionShutdownGate', () => { const gate = installSessionShutdownGate({ app: fake.app, - getManager: () => manager, + drain: () => manager.killAll(), onQuitAllowed, }) @@ -87,7 +117,7 @@ describe('installSessionShutdownGate', () => { const manager = { killAll: vi.fn(() => teardown.promise) } const onQuitAllowed = vi.fn() - installSessionShutdownGate({ app: fake.app, getManager: () => manager, onQuitAllowed }) + installSessionShutdownGate({ app: fake.app, drain: () => manager.killAll(), onQuitAllowed }) const first = fake.emitWillQuit() const duplicate = fake.emitWillQuit() @@ -99,6 +129,7 @@ describe('installSessionShutdownGate', () => { teardown.resolve() await teardown.promise await Promise.resolve() + await Promise.resolve() expect(fake.app.quit).toHaveBeenCalledOnce() expect(onQuitAllowed).not.toHaveBeenCalled() @@ -112,18 +143,15 @@ describe('installSessionShutdownGate', () => { const fake = createFakeApp() const teardown = deferred() const manager = { killAll: vi.fn(() => teardown.promise) } - const onLastWindowClosed = vi.fn() installSessionShutdownGate({ app: fake.app, - getManager: () => manager, + drain: () => manager.killAll(), onQuitAllowed: vi.fn(), platform: 'linux', - onLastWindowClosed, }) fake.emitWindowAllClosed() - expect(onLastWindowClosed).toHaveBeenCalledOnce() expect(fake.app.quit).toHaveBeenCalledOnce() expect(manager.killAll).not.toHaveBeenCalled() @@ -134,24 +162,22 @@ describe('installSessionShutdownGate', () => { teardown.resolve() await teardown.promise await Promise.resolve() + await Promise.resolve() expect(fake.app.quit).toHaveBeenCalledTimes(2) }) it('keeps macOS last-window closure outside application teardown', () => { const fake = createFakeApp() const manager = { killAll: vi.fn(async () => undefined) } - const onLastWindowClosed = vi.fn() installSessionShutdownGate({ app: fake.app, - getManager: () => manager, + drain: () => manager.killAll(), onQuitAllowed: vi.fn(), platform: 'darwin', - onLastWindowClosed, }) fake.emitWindowAllClosed() - expect(onLastWindowClosed).not.toHaveBeenCalled() expect(fake.app.quit).not.toHaveBeenCalled() expect(manager.killAll).not.toHaveBeenCalled() }) @@ -162,7 +188,7 @@ describe('installSessionShutdownGate', () => { const manager = { killAll: vi.fn(() => teardown.promise) } const gate = installSessionShutdownGate({ app: fake.app, - getManager: () => manager, + drain: () => manager.killAll(), onQuitAllowed: vi.fn(), platform: 'darwin', }) diff --git a/src/main/sessionShutdownGate.ts b/src/main/sessionShutdownGate.ts index ae4d3335d..93b5bbad1 100644 --- a/src/main/sessionShutdownGate.ts +++ b/src/main/sessionShutdownGate.ts @@ -8,16 +8,11 @@ interface SessionShutdownApp { quit(): void } -interface SessionShutdownManager { - killAll(): Promise -} - interface SessionShutdownGateOptions { app: SessionShutdownApp - getManager: () => SessionShutdownManager | null + drain: () => Promise onQuitAllowed: () => void platform?: NodeJS.Platform - onLastWindowClosed?: () => void onShutdownError?: (error: unknown) => void } @@ -50,7 +45,6 @@ export function installSessionShutdownGate( // and allow exit while the first call still awaited physical provider // stops. Keeping manager access out of this branch makes the gate below the // sole owner of the exact teardown promise on every platform. - options.onLastWindowClosed?.() options.app.quit() }) @@ -63,17 +57,6 @@ export function installSessionShutdownGate( return } - const manager = options.getManager() - if (!manager) { - // WHY absence is already terminal: packaging smoke and failed startup can - // legitimately quit before SessionManager construction. Inventing an - // async gate there would hold Electron for work that cannot exist. - terminalShutdownAdmitted = true - shutdownComplete = true - options.onQuitAllowed() - return - } - // WHY this is will-quit rather than before-quit: Electron emits // before-quit before BrowserWindow's beforeunload/will-prevent-unload // decision. A user choosing Keep Editing never reaches will-quit, so this @@ -94,8 +77,14 @@ export function installSessionShutdownGate( // recovery/replacement claims terminal before awaiting provider stops. // Starting a second teardown is unnecessary, while clearing that fence to // roll back a veto would be unsafe because some stops may already be done. - shutdownPromise = manager - .killAll() + // Publish the join BEFORE calling application code. A stop implementation + // can throw synchronously or re-enter app.quit(); neither may bypass the + // gate or start another drain. The drain still starts synchronously so its + // producer admission fences close on this same JavaScript turn. + let resolve!: () => void + let reject!: (error: unknown) => void + const drain = new Promise((yes, no) => { resolve = yes; reject = no }) + shutdownPromise = drain .then(() => { shutdownComplete = true options.app.quit() @@ -107,6 +96,7 @@ export function installSessionShutdownGate( shutdownPromise = null options.onShutdownError?.(error) }) + try { options.drain().then(resolve, reject) } catch (error) { reject(error) } }) return { diff --git a/src/main/storage/workspaceFileStore.test.ts b/src/main/storage/workspaceFileStore.test.ts index 2ac64d6d3..652c60183 100644 --- a/src/main/storage/workspaceFileStore.test.ts +++ b/src/main/storage/workspaceFileStore.test.ts @@ -53,6 +53,22 @@ describe('workspace persistence ordering', () => { unlink.mockReset().mockResolvedValue(undefined) }) + it('keeps shutdown waiting for the final admitted rename without inventing another save', async () => { + const publication = deferred() + rename.mockImplementationOnce(() => publication.promise) + const store = await WorkspaceFileStore.open() + const save = store.saveSlice('w1', slice(['agent']), NO_GEOMETRY) + await vi.waitFor(() => expect(rename).toHaveBeenCalledOnce()) + const settled = vi.fn() + const drain = store.drainAdmittedWrites().then(settled) + await Promise.resolve() + expect(settled).not.toHaveBeenCalled() + publication.resolve() + await Promise.all([save, drain]) + expect(settled).toHaveBeenCalledOnce() + expect(writeFile).toHaveBeenCalledOnce() + }) + it('commits overlapping saves in admission order', async () => { const firstWriteGate = deferred() const tempContents = new Map() diff --git a/src/main/storage/workspaceFileStore.ts b/src/main/storage/workspaceFileStore.ts index eeba7b405..166a5f3ff 100644 --- a/src/main/storage/workspaceFileStore.ts +++ b/src/main/storage/workspaceFileStore.ts @@ -122,6 +122,16 @@ export class WorkspaceFileStore { } } + /** + * Join saves admitted before this call. Their individual IPC receipts still + * carry publication failures; this tail is settlement, not a new save or an + * fsync guarantee. Revision-bound final renderer saves need the B02 prepare + * protocol and must not be inferred merely from reaching will-quit. + */ + async drainAdmittedWrites(): Promise { + await this.saveTail + } + /** The windows to restore at startup, in file order. */ windows(): readonly PersistedWindow[] { return this.file.windows diff --git a/src/main/window/windowRegistry.test.ts b/src/main/window/windowRegistry.test.ts index a8039e0b1..b53e02dc5 100644 --- a/src/main/window/windowRegistry.test.ts +++ b/src/main/window/windowRegistry.test.ts @@ -46,6 +46,18 @@ describe('window registry routing', () => { built.length = 0 }) + it('rejects every window factory call after committed shutdown before allocating native chrome', () => { + let committed = false + registry.setWindowCreationAdmission(() => !committed) + registry.createAppWindow() + expect(built).toHaveLength(1) + committed = true + expect(registry.isWindowCreationAllowed()).toBe(false) + expect(() => registry.createAppWindow()).toThrow('shutting down') + expect(() => registry.createAppWindow({ windowId: 'restored' })).toThrow('shutting down') + expect(built).toHaveLength(1) + }) + it('sends a session event only to the window that owns the session', () => { const left = registry.createAppWindow() const right = registry.createAppWindow() diff --git a/src/main/window/windowRegistry.ts b/src/main/window/windowRegistry.ts index 820d66523..206874214 100644 --- a/src/main/window/windowRegistry.ts +++ b/src/main/window/windowRegistry.ts @@ -343,11 +343,22 @@ function copyStringLength( // Lifecycle // --------------------------------------------------------------------------- +let windowCreationAdmission = (): boolean => true + +export function setWindowCreationAdmission(admit: () => boolean): void { + windowCreationAdmission = admit +} + +export function isWindowCreationAllowed(): boolean { + return windowCreationAdmission() +} + export function createAppWindow(options?: { windowId?: WindowId bounds?: WindowBounds | null fullScreen?: boolean }): WindowId { + if (!windowCreationAdmission()) throw new Error('Agent Code is shutting down; new windows are unavailable') const id = options?.windowId ?? randomUUID() const window = buildAppWindow({ bounds: options?.bounds ?? null, @@ -607,6 +618,7 @@ export function sendToSessionWindow( /** Test-only reset. Vitest module state persists across files in a worker. */ export function resetWindowRegistryForTests(): void { + windowCreationAdmission = () => true windows.clear() focusOrder.length = 0 sessionOwners.clear() diff --git a/src/main/workflows/createWorkflowService.test.ts b/src/main/workflows/createWorkflowService.test.ts index 31d498d9e..038ea77c0 100644 --- a/src/main/workflows/createWorkflowService.test.ts +++ b/src/main/workflows/createWorkflowService.test.ts @@ -46,6 +46,7 @@ describe('createWorkflowService', () => { vi.unstubAllEnvs() }) + it('lets WorkflowService acquire storage ownership before initialization without eagerly constructing Codex', async () => { const service = await createWorkflowService() @@ -79,4 +80,18 @@ describe('createWorkflowService', () => { sessionSourceHome: '/tmp/agent-code-home/.codex', }) }) + it('publishes the exact workflow owner before initialization can block or reject', async () => { + let reject!: (error: unknown) => void + serviceInitialize.mockImplementationOnce(() => new Promise((_resolve, no) => { reject = no })) + const onCreated = vi.fn() + const creation = createWorkflowService({ onCreated }) + expect(onCreated).toHaveBeenCalledOnce() + expect(onCreated.mock.calls[0]![0].initialize).toBe(serviceInitialize) + reject(new Error('store repair failed')) + await expect(creation).rejects.toThrow('store repair failed') + // The caller still owns this exact object after a rejected factory promise; + // it can invoke WorkflowService's existing initialization-aware stop path. + expect(onCreated).toHaveBeenCalledOnce() + }) + }) diff --git a/src/main/workflows/createWorkflowService.ts b/src/main/workflows/createWorkflowService.ts index f6737d5d1..a9f182f2b 100644 --- a/src/main/workflows/createWorkflowService.ts +++ b/src/main/workflows/createWorkflowService.ts @@ -16,6 +16,7 @@ import { WorkflowSourceApprovalStore } from '@main/workflows/WorkflowSourceAppro export async function createWorkflowService(options: { isCodexCliUpdateReserved?: () => boolean + onCreated?: (service: WorkflowService) => void } = {}): Promise { const workflowStateRoot = join(app.getPath('userData'), 'workflows') const store = new FileWorkflowStore(workflowStateRoot) @@ -106,6 +107,10 @@ export async function createWorkflowService(options: { network: false, }, }) + // Publish ownership before initialize can recover work or fail halfway. + // WorkflowService.stop() already joins its initializer and fences recovery; + // main must be able to reach that owner during a partial-startup quit. + options.onCreated?.(service) await service.initialize() return service } diff --git a/src/renderer/src/features/global-editor/hooks/useEditorBeforeUnloadGuard.renderer.test.tsx b/src/renderer/src/features/global-editor/hooks/useEditorBeforeUnloadGuard.renderer.test.tsx new file mode 100644 index 000000000..e01d36fdf --- /dev/null +++ b/src/renderer/src/features/global-editor/hooks/useEditorBeforeUnloadGuard.renderer.test.tsx @@ -0,0 +1,69 @@ +import { EventEmitter } from 'node:events' +import { act, renderHook, waitFor } from '@testing-library/react' +import { expect, it, vi } from 'vitest' +import { installApplicationShutdown, type ApplicationShutdownServices } from '@main/applicationShutdown' +import { useGlobalEditorStore } from '../store' +import { useEditorBeforeUnloadGuard } from './useEditorBeforeUnloadGuard' + +it('keeps the real editor veto ahead of all application service disposal', async () => { + useGlobalEditorStore.setState({ byCwd: {}, cwdRecency: [], activeCwd: null }) + const hook = renderHook(useEditorBeforeUnloadGuard) + const events = new EventEmitter() + let windowsClosed = false + const app = Object.assign(events, { + quit: () => { + events.emit('before-quit') + if (!windowsClosed) { + // Electron/Chromium is the external boundary: dispatch the actual + // renderer hook, then emulate Keep Editing honoring its veto. This + // catches a disposer in the real application listener composition, + // which a test of SessionShutdownGate alone could never exercise. + const unload = new Event('beforeunload', { cancelable: true }) + window.dispatchEvent(unload) + if (unload.defaultPrevented) return + windowsClosed = true + } + events.emit('will-quit', { preventDefault: vi.fn() }) + }, + }) + let executionStopped = false + const stop = vi.fn(async () => { executionStopped = true }) + const supportStop = vi.fn(async () => undefined) + const services: ApplicationShutdownServices = { + getSessions: () => ({ killAll: stop }), getWorkflows: () => ({ stop }), + startupSettled: async () => undefined, + stopDictation: supportStop, flushObservations: supportStop, sweepOwnedProxies: supportStop, + stopBuiltInMcp: supportStop, stopRemote: supportStop, stopLsp: supportStop, + stopExternalControl: supportStop, disposeControl: supportStop, + disposeWorkflowBridge: supportStop, disposeCaffeinate: supportStop, stopHeapWatchdog: supportStop, + drainWorkspace: supportStop, drainDictationHistory: supportStop, + flushGhosts: supportStop, flushRecordings: supportStop, flushDictationDebug: supportStop, + flushPasteDebug: supportStop, stopPerformance: supportStop, + } + const onQuitAllowed = vi.fn() + const gate = installApplicationShutdown({ app, services, prepare: vi.fn(), onQuitAllowed, + onShutdownError: vi.fn(), onDiagnosticError: vi.fn() }) + try { + act(() => { + useGlobalEditorStore.getState().openFile({ cwd: '/repo', path: 'draft.ts', text: 'base', mtimeMs: 1, diskVersion: 'base' }) + useGlobalEditorStore.getState().updateFileText('/repo', 'draft.ts', 'unsaved revision') + }) + app.quit() + expect(windowsClosed).toBe(false) + expect(executionStopped).toBe(false) + expect(supportStop).not.toHaveBeenCalled() + expect(gate.isTerminalShutdownAdmitted()).toBe(false) + expect(useGlobalEditorStore.getState().byCwd['/repo']!.openFiles['draft.ts']!.currentText).toBe('unsaved revision') + + // Further editing remains possible after veto. Returning to the saved + // baseline makes the later unload clean; no cached approval is fabricated. + act(() => { useGlobalEditorStore.getState().updateFileText('/repo', 'draft.ts', 'base') }) + app.quit() + await waitFor(() => expect(onQuitAllowed).toHaveBeenCalledOnce()) + expect(executionStopped).toBe(true) + expect(gate.isTerminalShutdownAdmitted()).toBe(true) + } finally { + hook.unmount() + useGlobalEditorStore.setState({ byCwd: {}, cwdRecency: [], activeCwd: null }) + } +}) From 3d382fb744b54e7682fd6c6f80f75ad4c6c26aab Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 12 Sep 2026 14:32:58 -0700 Subject: [PATCH 3/6] fix(dictation): cancel owned transcription when quit commits --- ARCHITECTURE.md | 2 +- .../2026-09-12-quit-service-lifecycle.md | 11 ++++++- src/main/dictation/controller.ts | 3 ++ src/main/ipc/dictation.ts | 28 ++++++++++++++-- src/main/ipc/dictationShutdown.test.ts | 32 +++++++++++++++++-- 5 files changed, 69 insertions(+), 7 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 146907901..a72c3cca5 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -1289,7 +1289,7 @@ sequenceDiagram [Application shutdown composition](src/main/applicationShutdown.ts) keeps irreversible disposal out of `before-quit`. The existing [terminal gate](src/main/sessionShutdownGate.ts) holds `will-quit` until the complete application drain resolves. Repeated quit requests join one attempt; retries retain completed-stage receipts and invoke only failed stages. Startup publishes workflow ownership before initialization and checks committed admission after asynchronous acquisition. All window creation routes share a committed-shutdown guard. -Required execution stops retain their native ownership contracts. Workspace and dictation history tails establish settlement of admitted writes; they do not retry failed saves or establish fsync durability. Dictation joins already-admitted batch/preview operations before its history tail is captured. Diagnostic queues are awaited, with failures reported separately. A failed boot retains its process lock through cleanup and does not receive a clean-run marker. +Required execution stops retain their native ownership contracts. Workspace and dictation history tails establish settlement of admitted writes; they do not retry failed saves or establish fsync durability. Dictation aborts owned batch HTTP and joins admitted handlers/hotkey work before capturing its history tail. It cancels active/stopping previews and fences late optional observations; the pinned preview cancellation API can abandon a pending stop promise, which shutdown must not await. Diagnostic queues are awaited, with failures reported separately. A failed boot retains its process lock through cleanup and does not receive a clean-run marker. This is the application disposal repair in B02/#919. It does not yet introduce revision-bound editor approvals, a cross-window preparation generation, or a final persistence acknowledgement frontier. The native per-window close decision UX remains in use. “Clean exit” remains a lifecycle result rather than proof that every optional record reached disk. diff --git a/docs/superpowers/plans/2026-09-12-quit-service-lifecycle.md b/docs/superpowers/plans/2026-09-12-quit-service-lifecycle.md index ca6a5a35d..34a99485a 100644 --- a/docs/superpowers/plans/2026-09-12-quit-service-lifecycle.md +++ b/docs/superpowers/plans/2026-09-12-quit-service-lifecycle.md @@ -43,7 +43,7 @@ Partial startup required more than moving listeners. The workflow factory publis The composition audit exposed two concrete owner gaps that this same slice repairs: -- Dictation's active map excludes stop handlers awaiting batch HTTP. Committed cleanup now fences new operations, cancels active previews, joins admitted start/stop/hotkey work and the independent preview-stop promise, then captures the history write tail. A key lookup or hotkey configuration finishing late cannot revive a resource behind cleanup. +- Dictation's active map excludes stop handlers awaiting batch HTTP. Committed cleanup now fences new operations, aborts owned batch HTTP, joins admitted start/stop/hotkey work, cancels active/stopping previews and fences their late observations, then captures the history write tail. A key lookup or hotkey configuration finishing late cannot revive a resource behind cleanup. - Remote disposal previously bypassed the enable/disable FIFO and erased its server pointer before a stop resolved. Disposal now closes enable admission, joins that FIFO, prevents a pending enable from publishing a live URL, and keeps the exact server on rejection for an explicit retry. The shutdown section of `ARCHITECTURE.md` and its generated preview now describe the implemented boundary. All 42 diagram sources rendered and verified with the pinned documentation tools; only the changed shutdown preview is retained. Five unrelated previews differed when regenerated in this local Chrome environment; those generated changes were discarded. The shutdown SVG was visually inspected independently. @@ -64,3 +64,12 @@ This PR uses **Refs #919**, not an issue-closing keyword. B02 remains open for t The bounded result is preservation of services on veto and one explicit composition of existing committed-stop/drain contracts, including the two owner gaps above. It is not completion of every quit durability or editor approval invariant in the program. The next control frontier audit starts at `src/main/control/createControlHost.ts`, `src/control-sdk/core/executor.ts` and `src/main/control/history/FileControlHistory.ts`. The executor's `active` map is populated only after its admitted intent write, so draining that map alone would miss a request still queued in `exclusive`. Nested waits/batches also call the executor directly. Main's private `operations.start`/`operations.finish` port must keep completion receipts writable while new effectful work is closed; a blanket rejection of all invocation would lose the evidence shutdown needs. Keep these facts in the next focused B02 plan. + + +## Final provider-boundary review + +The first app build and entrypoint verification passed. Review of the actual pinned voice package then exposed why merely awaiting dictation promises was insufficient: batch HTTP has no default application deadline, and preview `cancel()` removes the session before `finalizeSession` can resolve an earlier `stop()` promise. The final implementation propagates an AbortSignal through the real main controller to the pinned HTTP provider, aborts owned batch work on committed quit, and joins its handler. It cancels previews through the existing API and suppresses late optional debug writes instead of awaiting an abandoned preview-stop promise. Already-completed batch results may still enqueue history before their handler settles; the subsequent tail includes those writes. + +A fourth dictation regression exercises IPC → real controller → pinned provider → abortable HTTP boundary. The coordinator/dictation review lane passed 13 cases, including cancellation and an intentionally unresolved optional preview promise. This adds one unique unit case to the previous totals (58 unit, nine remote system, one renderer). Repeat final-source checks/build after this substantive correction; do not cite the earlier app build as verification of the cancellation change. + +New problem records from the composition audit: #941 remote disposal (implemented), #942 pending dictation cleanup (implemented), #943 control admission/result drain (follow-up, not implemented). The first two are independently closeable by this PR; #919/#943/#918 remain open. The conventions require these separate issue records even though the implementation shares the quit-safety PR. diff --git a/src/main/dictation/controller.ts b/src/main/dictation/controller.ts index 359bc3bf9..072727ad2 100644 --- a/src/main/dictation/controller.ts +++ b/src/main/dictation/controller.ts @@ -33,6 +33,8 @@ export type DictationBatchInput = { * expose multilingual dictation later without a controller refactor. */ language?: string onTrace?: (event: SpeechTraceEvent) => void + /** The owning operation cancels HTTP when its consumer is permanently gone. */ + signal?: AbortSignal } // Discriminated union, same shape as the cleanup we landed on the @@ -74,6 +76,7 @@ function runProvider(input: DictationBatchInput): Promise { audio, ...(input.language ? { language: input.language } : {}), ...(input.onTrace ? { onTrace: input.onTrace } : {}), + ...(input.signal ? { signal: input.signal } : {}), } return transcribeDeepgram({}, opts) } diff --git a/src/main/ipc/dictation.ts b/src/main/ipc/dictation.ts index 33d9a7a24..4b7f7a87f 100644 --- a/src/main/ipc/dictation.ts +++ b/src/main/ipc/dictation.ts @@ -60,6 +60,8 @@ type ActiveDictationSession = { const activeSessions = new Map() let shutdownAdmitted = false const pendingOperations = new Set>() +const pendingTranscriptions = new Set() +const stoppingPreviews = new Set() function trackPending(promise: Promise): Promise { pendingOperations.add(promise) @@ -149,7 +151,9 @@ export function registerDictationIpc(deps: { event: string, data?: Record, ): void => { - if (!debugSessionId) return + // Preview cancellation may abandon an unresolved package stop promise. + // Late optional observations must not append behind the final debug flush. + if (shutdownAdmitted || !debugSessionId) return deps.dictationDebugJournals .get(debugSessionId) .append({ layer, event, ...(data !== undefined ? { data } : {}) }) @@ -484,7 +488,13 @@ export function registerDictationIpc(deps: { return null }) : Promise.resolve(null) - trackPending(streamingStop) + if (streamingId) { + stoppingPreviews.add(streamingId) + const forgetPreview = (): void => { stoppingPreviews.delete(streamingId) } + void streamingStop.then(forgetPreview, forgetPreview) + } + const batchAbort = new AbortController() + pendingTranscriptions.add(batchAbort) if (DICTATION_DUMP_ENABLED) { // eslint-disable-next-line no-console @@ -517,6 +527,7 @@ export function registerDictationIpc(deps: { }) const startedAt = Date.now() const outcome = await transcribeBatch({ + signal: batchAbort.signal, provider: session.provider, apiKey: session.apiKey, audio, @@ -657,6 +668,8 @@ export function registerDictationIpc(deps: { kind: 'error', message: err instanceof Error ? err.message : 'Dictation failed.', } + } finally { + pendingTranscriptions.delete(batchAbort) } }), ) @@ -679,6 +692,17 @@ export function registerDictationIpc(deps: { export async function cleanupDictationIpcResources(): Promise { shutdownAdmitted = true unregisterDictationHotkey() + // Quit has committed and the originating composer is gone. Cancel owned + // batch HTTP and join the handler, rather than letting an unbounded provider + // response hold application exit. A response that already completed can + // still enqueue history before the handler settles and its tail is drained. + for (const operation of pendingTranscriptions) operation.abort() + // The pinned preview cancel() drops the session before finalizeSession can + // resolve an earlier stop() promise. It is explicitly an abandonment API, + // not a joinable completion receipt. Cancel both active and stopping previews + // and fence their late debug observations instead of awaiting that promise. + for (const id of stoppingPreviews) deepgramStreaming().cancel(id) + stoppingPreviews.clear() for (const session of activeSessions.values()) { if (session.streamingId) deepgramStreaming().cancel(session.streamingId) } diff --git a/src/main/ipc/dictationShutdown.test.ts b/src/main/ipc/dictationShutdown.test.ts index dba480e62..edf95beb0 100644 --- a/src/main/ipc/dictationShutdown.test.ts +++ b/src/main/ipc/dictationShutdown.test.ts @@ -31,6 +31,7 @@ beforeEach(() => { vi.resetModules() vi.resetAllMocks() mocks.handlers.clear() + vi.unstubAllGlobals() mocks.key.mockResolvedValue('test-only-key') mocks.preview.start.mockReturnValue({ id: 'preview' }) mocks.preview.stop.mockResolvedValue(null) @@ -60,7 +61,7 @@ describe('dictation IPC shutdown ownership', () => { await expect(invoke('stream-start', { provider: 'deepgram' })).rejects.toThrow('shutting down') }) - it('cancels active previews and keeps pending batch/history producers inside the drain', async () => { + it('cancels previews without joining abandoned stop promises and drains admitted batch/history producers', async () => { const module = await setup() const batch = deferred<{ kind: 'ok'; raw: string }>() const preview = deferred() @@ -76,13 +77,15 @@ describe('dictation IPC shutdown ownership', () => { const settled = vi.fn() const shutdown = module.cleanupDictationIpcResources().then(settled) expect(mocks.preview.cancel).toHaveBeenCalledWith('preview') + expect(mocks.batch.mock.calls[0]![0].signal.aborted).toBe(true) batch.resolve({ kind: 'ok', raw: 'recoverable dictation' }) await stop expect(mocks.append).toHaveBeenCalledWith(expect.objectContaining({ text: 'recoverable dictation' })) - expect(settled).not.toHaveBeenCalled() - preview.resolve(null) await shutdown expect(settled).toHaveBeenCalledOnce() + // The native preview cancel contract may never settle preview.promise. + // That optional promise cannot hold committed application exit. + preview.resolve(null) }) it('removes a hotkey installed by a configuration request that finishes after shutdown', async () => { @@ -96,4 +99,27 @@ describe('dictation IPC shutdown ownership', () => { await shutdown expect(mocks.unregister).toHaveBeenCalledTimes(2) }) + it('propagates committed cancellation through the real controller and pinned provider HTTP path', async () => { + const { transcribeBatch } = await import('../dictation/controller') + mocks.batch.mockImplementation(transcribeBatch) + let signal: AbortSignal | undefined + vi.stubGlobal('fetch', vi.fn((_url: unknown, init: RequestInit) => { + signal = init.signal as AbortSignal + return new Promise((_resolve, reject) => { + signal!.addEventListener('abort', () => reject(new DOMException('Quit cancelled HTTP', 'AbortError')), { once: true }) + }) + })) + try { + const module = await setup() + const { id } = await invoke('stream-start', { provider: 'deepgram' }) + await invoke('stream-chunk', { id, chunk: new ArrayBuffer(4) }) + const stop = invoke('stream-stop', { id, audioDurationMs: 1000 }) + expect(signal?.aborted).toBe(false) + await module.cleanupDictationIpcResources() + expect(signal?.aborted).toBe(true) + expect(await stop).toMatchObject({ kind: 'error' }) + expect(mocks.append).not.toHaveBeenCalled() + } finally { vi.unstubAllGlobals() } + }) + }) From 9778d51382b58ec8970ac4d27a8c81cecba7f2e2 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 19 Sep 2026 18:36:56 -0700 Subject: [PATCH 4/6] fix(shutdown): commit the stopExtensions stage the merge left unstaged The merge that placed main's extension runtime in the committed-quit composition staged index.ts but not applicationShutdown.ts, so the services object passed a stage the interface did not declare. Local `tsc -b` was incremental and stale; CI caught it. Co-Authored-By: Claude Opus 5 (1M context) --- src/main/applicationShutdown.test.ts | 22 +++++++++++++++++++ src/main/applicationShutdown.ts | 9 +++++++- ...eEditorBeforeUnloadGuard.renderer.test.tsx | 2 +- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/main/applicationShutdown.test.ts b/src/main/applicationShutdown.test.ts index 7ca314452..7514f3a66 100644 --- a/src/main/applicationShutdown.test.ts +++ b/src/main/applicationShutdown.test.ts @@ -42,6 +42,7 @@ function harness() { flushDictationDebug: vi.fn(async (): Promise => undefined), flushPasteDebug: vi.fn(async (): Promise => undefined), stopPerformance: vi.fn(async (): Promise => undefined), + stopExtensions: vi.fn(async (): Promise => undefined), } satisfies ApplicationShutdownServices const prepare = vi.fn() const onQuitAllowed = vi.fn() @@ -166,6 +167,27 @@ describe('application shutdown composition', () => { expect(h.sessionStop).not.toHaveBeenCalled() }) + it('stops extensions only after startup settles, and before any support service (merge with #577)', async () => { + // Startup creates the extension runtime. A stop issued before startup + // settled would complete against nothing, keep that receipt, and let the + // runtime startup publishes later escape disposal. + const h = harness() + const startup = deferred() + const extensions = deferred() + h.services.startupSettled.mockImplementation(() => startup.promise) + h.services.stopExtensions.mockImplementation(() => extensions.promise) + h.install() + h.app.quit() + expect(h.services.stopExtensions).not.toHaveBeenCalled() + startup.resolve() + await vi.waitFor(() => expect(h.services.stopExtensions).toHaveBeenCalledOnce()) + // Extensions run user code; support services stay up until they stop. + expect(h.services.stopBuiltInMcp).not.toHaveBeenCalled() + extensions.resolve() + await vi.waitFor(() => expect(h.onQuitAllowed).toHaveBeenCalledOnce()) + expect(h.services.stopBuiltInMcp).toHaveBeenCalledOnce() + }) + it('closes an initializing workflow immediately, while startup settlement still gates support disposal', async () => { const h = harness() const startup = deferred() diff --git a/src/main/applicationShutdown.ts b/src/main/applicationShutdown.ts index 0761c046d..550cb5b7a 100644 --- a/src/main/applicationShutdown.ts +++ b/src/main/applicationShutdown.ts @@ -28,6 +28,10 @@ export interface ApplicationShutdownServices { flushDictationDebug: Stop flushPasteDebug: Stop stopPerformance: Stop + /** Background extension runtimes (#577). An execution owner like sessions + * and workflows: extensions run user code in hidden windows, so they stop + * in the first wave, beside them, and gate the support services after. */ + stopExtensions: Stop } interface Stage { @@ -106,7 +110,10 @@ export function installApplicationShutdown(options: { // settlement closes the resource inventory; a missing SessionManager alone // is not evidence that workflow/MCP/other startup resources never existed. await services.startupSettled() - await join([...earlyStops, ...stopExecution(), dictationStop]) + // Extensions stop only after startup settles: startup creates the + // runtime, and `run` keeps the first receipt, so an early stop against a + // not-yet-published runtime would complete and let the real one escape. + await join([...earlyStops, ...stopExecution(), dictationStop, run('extensions', services.stopExtensions)]) await run('observations', services.flushObservations) await run('proxy-sweep', services.sweepOwnedProxies) diff --git a/src/renderer/src/features/global-editor/hooks/useEditorBeforeUnloadGuard.renderer.test.tsx b/src/renderer/src/features/global-editor/hooks/useEditorBeforeUnloadGuard.renderer.test.tsx index e01d36fdf..0e51816b6 100644 --- a/src/renderer/src/features/global-editor/hooks/useEditorBeforeUnloadGuard.renderer.test.tsx +++ b/src/renderer/src/features/global-editor/hooks/useEditorBeforeUnloadGuard.renderer.test.tsx @@ -38,7 +38,7 @@ it('keeps the real editor veto ahead of all application service disposal', async disposeWorkflowBridge: supportStop, disposeCaffeinate: supportStop, stopHeapWatchdog: supportStop, drainWorkspace: supportStop, drainDictationHistory: supportStop, flushGhosts: supportStop, flushRecordings: supportStop, flushDictationDebug: supportStop, - flushPasteDebug: supportStop, stopPerformance: supportStop, + flushPasteDebug: supportStop, stopPerformance: supportStop, stopExtensions: supportStop, } const onQuitAllowed = vi.fn() const gate = installApplicationShutdown({ app, services, prepare: vi.fn(), onQuitAllowed, From 9309b9d502e94b3d3f37ac84b1d96349c26cc8d9 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 19 Sep 2026 18:42:48 -0700 Subject: [PATCH 5/6] fix(shutdown): a failed quit offers a retry that actually re-quits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review of #945. When a committed quit fails, will-quit has been admitted and every window is gone, so the dialog is the only control the application still has. It said "Quit again to retry" while offering one button, "Keep Agent Code Open", and discarded the response. On macOS a user can quit again from the Dock; on Windows and Linux the menu bar belongs to a window, window creation is fenced after commitment, and focusWindow has nothing to focus — so a transient stop failure stranded the process and its state lock with no reachable action, even though a second attempt would have succeeded. The dialog now offers Retry Quit (default) and Keep Agent Code Open, and calls app.quit() on a retry. Escape maps to "keep open", never a retry: a retry can kill live sessions. Retrying is safe by construction — applicationShutdown drops failed stages at the next drain and keeps the completed ones. It lives in its own module because index.ts cannot be imported by a test (it builds the whole application on import), and this is behaviour rather than presentation. Four tests drive it: the retry quits, waiting does not, an aggregate failure reports every cause, and a dialog that throws never escapes into the shutdown path. Co-Authored-By: Claude Opus 5 (1M context) --- src/main/index.ts | 15 +++--- src/main/quitFailureDialog.test.ts | 52 +++++++++++++++++++ src/main/quitFailureDialog.ts | 80 ++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 8 deletions(-) create mode 100644 src/main/quitFailureDialog.test.ts create mode 100644 src/main/quitFailureDialog.ts diff --git a/src/main/index.ts b/src/main/index.ts index 6f100f49a..29fa9d111 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -42,6 +42,7 @@ import { conditionBackendCapabilities } from '@main/sessions/conditionControl.js import { terminalBackendCapabilities } from '@main/sessions/terminalControl.js' import { windowLifecycleControlCapabilities } from '@main/window/lifecycleControl.js' import { installApplicationShutdown } from '@main/applicationShutdown.js' +import { presentQuitFailure } from '@main/quitFailureDialog.js' import { LspManager } from '@main/lspManager.js' import { compactAllGhostLogs, GhostJournalRegistry } from '@main/ghostJournal.js' import { @@ -1460,14 +1461,12 @@ const sessionShutdownGate = installApplicationShutdown({ console.error('[app] graceful shutdown blocked:', error) appRunJournal?.recordError('app.shutdown.error', error) if (!app.isReady()) return - void dialog.showMessageBox({ - type: 'error', title: 'Agent work is still shutting down', - message: 'Agent Code could not safely quit yet.', - detail: 'Shutdown is incomplete. Quit again to retry.\n\n' + (error instanceof AggregateError - ? error.errors.map(cause => cause instanceof Error ? cause.message : String(cause)).join('\n') - : error instanceof Error ? error.message : String(error)), - buttons: ['Keep Agent Code Open'], defaultId: 0, cancelId: 0, noLink: true, - }).catch(reportError => console.error('[app] could not show shutdown error:', reportError)) + // Every window is gone by now, so this dialog is the only reachable + // control the application still has; quitFailureDialog.ts owns what it + // offers and acts on the answer (#945 Codex review). + // A lambda, not `dialog` itself: Electron's showMessageBox is overloaded + // (with and without a parent window), and this call has no window left. + void presentQuitFailure({ showMessageBox: options => dialog.showMessageBox(options) }, app, error) }, onDiagnosticError: (stage, error) => appRunJournal?.recordError(`app.shutdown.${stage}.error`, error), }) diff --git a/src/main/quitFailureDialog.test.ts b/src/main/quitFailureDialog.test.ts new file mode 100644 index 000000000..17ff87497 --- /dev/null +++ b/src/main/quitFailureDialog.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it, vi } from 'vitest' + +import { presentQuitFailure, quitFailureDetail, type QuitFailureDialogHost } from './quitFailureDialog' + +// #945 Codex review: when a committed quit fails, `will-quit` has already been +// admitted and every window is gone. This dialog is the only control the +// application still has, so its buttons — and whether the answer is acted on — +// decide whether a transient stop failure strands the process and its state +// lock. The earlier version said "Quit again to retry" while offering only +// "Keep Agent Code Open" and discarding the response; on Windows and Linux the +// menu bar belongs to a window and window creation is fenced after +// commitment, so there was nothing left to quit with. + +type Options = Parameters[0] + +function host(response: number) { + return { showMessageBox: vi.fn(async (_options: Options) => ({ response })) } +} + +describe('a failed committed quit', () => { + it('offers a retry that actually re-quits, with retry as the default button', async () => { + const dialog = host(0) + const app = { quit: vi.fn() } + await presentQuitFailure(dialog, app, new Error('workflow stop timed out')) + const options = dialog.showMessageBox.mock.calls[0]![0] + expect(options.buttons).toEqual(['Retry Quit', 'Keep Agent Code Open']) + expect(options.defaultId).toBe(0) + expect(app.quit).toHaveBeenCalledOnce() + }) + + it('does nothing when the user chooses to wait, and Escape means wait', async () => { + const dialog = host(1) + const app = { quit: vi.fn() } + await presentQuitFailure(dialog, app, new Error('workflow stop timed out')) + // Escape must never re-enter a quit that can kill live sessions. + expect(dialog.showMessageBox.mock.calls[0]![0].cancelId).toBe(1) + expect(app.quit).not.toHaveBeenCalled() + }) + + it('reports every cause of an aggregate failure, since one stage can fail per owner', () => { + const detail = quitFailureDetail(new AggregateError([new Error('workflows: timed out'), new Error('sessions: EBUSY')])) + expect(detail).toContain('workflows: timed out') + expect(detail).toContain('sessions: EBUSY') + }) + + it('never lets a failing dialog throw into the shutdown path', async () => { + const dialog = { showMessageBox: vi.fn(async (_options: Options) => { throw new Error('no display') }) } + const app = { quit: vi.fn() } + await expect(presentQuitFailure(dialog, app, new Error('boom'))).resolves.toBeUndefined() + expect(app.quit).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/quitFailureDialog.ts b/src/main/quitFailureDialog.ts new file mode 100644 index 000000000..e4b49380f --- /dev/null +++ b/src/main/quitFailureDialog.ts @@ -0,0 +1,80 @@ +/** + * What the user is told, and what they can DO, when a committed quit could not + * finish (#945 Codex review). + * + * WHY this is its own module rather than a closure in index.ts: by the time + * this runs, `will-quit` has been admitted and every window is gone. The only + * remaining way to reach the application is this dialog's buttons, so which + * buttons exist — and whether their answer is acted on — is load-bearing + * behavior, not presentation. index.ts cannot be imported by a test (it builds + * the whole application on import), so the decision lives here where it can be + * driven directly. + * + * WHAT WAS WRONG: the dialog said "Quit again to retry" while offering exactly + * one button, "Keep Agent Code Open", and discarded the response. On macOS a + * user can quit again from the Dock; on Windows and Linux the menu bar belongs + * to a window, window creation is fenced after commitment, and `focusWindow` + * has nothing to focus. So a transient stop failure — a workflow provider that + * needed one more moment — stranded the process and its state lock with no + * reachable action, even though a second attempt would have succeeded. + * + * The retry is safe by construction: `applicationShutdown` drops FAILED stages + * at the start of the next drain and keeps completed ones, so quitting again + * re-runs only what did not finish. + */ + +/** The parts of Electron's `dialog` and `app` this needs, so a test supplies + * plain objects instead of booting Electron. */ +export type QuitFailureDialogHost = { + showMessageBox(options: { + type: 'error' + title: string + message: string + detail: string + buttons: string[] + defaultId: number + cancelId: number + noLink: boolean + }): Promise<{ response: number }> +} +export type QuitFailureApp = { quit(): void } + +export function quitFailureDetail(error: unknown): string { + const causes = error instanceof AggregateError + ? error.errors.map(cause => (cause instanceof Error ? cause.message : String(cause))).join('\n') + : error instanceof Error + ? error.message + : String(error) + return `Shutdown is incomplete. Retry when you are ready; anything that already stopped stays stopped.\n\n${causes}` +} + +/** + * Shows the failure and acts on the answer. Resolves once the user has + * answered (or immediately if the dialog itself fails), so callers can await + * it in tests; production calls it fire-and-forget. + */ +export async function presentQuitFailure( + host: QuitFailureDialogHost, + app: QuitFailureApp, + error: unknown, +): Promise { + try { + const { response } = await host.showMessageBox({ + type: 'error', + title: 'Agent work is still shutting down', + message: 'Agent Code could not safely quit yet.', + detail: quitFailureDetail(error), + // Retry first and as the default: the common case is a provider that + // needed another moment, and pressing Return should try again rather + // than park an application the user has already asked to close. + buttons: ['Retry Quit', 'Keep Agent Code Open'], + defaultId: 0, + // Escape means "not now", never "try again": a retry can kill sessions. + cancelId: 1, + noLink: true, + }) + if (response === 0) app.quit() + } catch (dialogError) { + console.error('[app] could not show shutdown error:', dialogError) + } +} From f19c0ed5746153b49331f8e8769fa35ead282f8a Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 19 Sep 2026 18:57:01 -0700 Subject: [PATCH 6/6] fix(shutdown): one quit-failure dialog at a time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex delta review of #945, reproduced against the real gate: the gate clears its shutdown promise when a drain fails, so a second quit can fail and report while the first dialog is still unanswered. That opened a second dialog with its own live Retry — two ways to re-enter shutdown, stacked over an application that has no windows left. One presentation is held at module scope; a report arriving while it is open resolves against it instead of opening another. Ownership is cleared before a retry, so the quit it triggers can report its own failure rather than be swallowed as a duplicate. The test holds the first dialog unanswered, reports a second failure, and expects exactly one showMessageBox; it fails without the guard. Co-Authored-By: Claude Opus 5 (1M context) --- src/main/quitFailureDialog.test.ts | 20 +++++++++++++++ src/main/quitFailureDialog.ts | 40 +++++++++++++++++++++++++++--- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/main/quitFailureDialog.test.ts b/src/main/quitFailureDialog.test.ts index 17ff87497..4f929b4e4 100644 --- a/src/main/quitFailureDialog.test.ts +++ b/src/main/quitFailureDialog.test.ts @@ -43,6 +43,26 @@ describe('a failed committed quit', () => { expect(detail).toContain('sessions: EBUSY') }) + it('shows one dialog at a time, so a second failure cannot stack a second Retry', async () => { + // The gate clears its shutdown promise when a drain fails, so a second + // quit can fail while the first dialog is still unanswered. + let answer!: (value: { response: number }) => void + const pending = new Promise<{ response: number }>(resolve => { answer = resolve }) + const dialog = { showMessageBox: vi.fn(async (_options: Options) => await pending) } + const app = { quit: vi.fn() } + const first = presentQuitFailure(dialog, app, new Error('first')) + const second = presentQuitFailure(dialog, app, new Error('second')) + expect(dialog.showMessageBox).toHaveBeenCalledOnce() + answer({ response: 1 }) + await Promise.all([first, second]) + expect(dialog.showMessageBox).toHaveBeenCalledOnce() + expect(app.quit).not.toHaveBeenCalled() + // Once answered, a later failure can present again. + const third = presentQuitFailure(host(1), app, new Error('third')) + await third + expect(app.quit).not.toHaveBeenCalled() + }) + it('never lets a failing dialog throw into the shutdown path', async () => { const dialog = { showMessageBox: vi.fn(async (_options: Options) => { throw new Error('no display') }) } const app = { quit: vi.fn() } diff --git a/src/main/quitFailureDialog.ts b/src/main/quitFailureDialog.ts index e4b49380f..c4a9ebabc 100644 --- a/src/main/quitFailureDialog.ts +++ b/src/main/quitFailureDialog.ts @@ -48,15 +48,44 @@ export function quitFailureDetail(error: unknown): string { return `Shutdown is incomplete. Retry when you are ready; anything that already stopped stays stopped.\n\n${causes}` } +/** + * One presentation at a time. + * + * WHY (#945 Codex delta review, reproduced against the real gate): the gate + * clears its shutdown promise when a drain fails, so a second quit can fail + * and report while the first dialog is still unanswered. That opened a second + * dialog with its own live Retry button — two ways to re-enter shutdown, and + * a stack of modals over an application with no windows. A later report while + * one is open is dropped: it is the same failure being retried, and the open + * dialog already offers the only two answers. + */ +let presenting: Promise | null = null + /** * Shows the failure and acts on the answer. Resolves once the user has - * answered (or immediately if the dialog itself fails), so callers can await - * it in tests; production calls it fire-and-forget. + * answered (or immediately if the dialog itself fails, or if a presentation + * is already open), so callers can await it in tests; production calls it + * fire-and-forget. */ export async function presentQuitFailure( host: QuitFailureDialogHost, app: QuitFailureApp, error: unknown, +): Promise { + if (presenting) return await presenting + const run = presentOnce(host, app, error) + presenting = run + try { + await run + } finally { + presenting = null + } +} + +async function presentOnce( + host: QuitFailureDialogHost, + app: QuitFailureApp, + error: unknown, ): Promise { try { const { response } = await host.showMessageBox({ @@ -73,7 +102,12 @@ export async function presentQuitFailure( cancelId: 1, noLink: true, }) - if (response === 0) app.quit() + // Ownership is cleared before the retry, so the quit this triggers can + // report its own failure rather than being swallowed as a duplicate. + if (response === 0) { + presenting = null + app.quit() + } } catch (dialogError) { console.error('[app] could not show shutdown error:', dialogError) }