fix(lifecycle): preserve services when Quit is cancelled - #945
Merged
Merged
Conversation
Juliusolsson05
marked this pull request as ready for review
September 12, 2026 21:47
This was referenced Sep 12, 2026
Main gained three quit-time owners after this branch was cut, and each had to be placed in the committed-shutdown composition rather than left on before-quit, which runs before the renderer's unload vote: - The extension runtime (#577). Its PAUSE stays on before-quit: it is the one preparation that must finish before windows unload, and it is reversible (Keep Editing resumes it). Its disposal is a new committed stage, `stopExtensions`, which runs beside sessions and workflows — extensions run user code — and only AFTER startup settles, because startup creates the runtime and an earlier stop would complete against nothing and keep that receipt. - The performance monitor and trace drain (#958), with its 1.8s/2s bounded race, folded into the `performance` stage. Main also ran monitor/watchdog/performanceService stops in will-quit; those are already stages here, so the duplicates are dropped. - tmux recovery now goes through restoration's canonical envelope decoder. A quit committed mid-read must not authorize killing tmux sessions, so the startup assertion moved inside the reader, where it becomes the decoder's read-failure path and withholds cleanup. `workflowShutdownPromise`/`Complete` are gone with the before-quit handler that used them; workflow stop is a committed stage. The shutdown diagram was rewritten as a flowchart in main's current convention (accTitle/accDescr/scope, #954) and says what the composition actually does: the editor decides first and Keep Editing leaves every service live; only a committed quit stops execution; a failed stop keeps the app and its process lock. Re-rendered with the pinned toolchain; every other SVG restored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cancelling Quit can leave Agent Code open after workflows, MCP, LSP, remote access and dictation have already stopped. Move those disposals into one committed-quit composition: an editor veto preserves services; a committed quit joins execution stops, startup, supporting services and admitted write tails before releasing the process lock.
The terminal gate publishes its exact drain before calling shutdown code. Repeated requests join it, completed stages are retained across retry, and failed owners hold the exit boundary. All window creation routes reject after commitment. Partial startup publishes workflow ownership before initialization and retains the lock through cleanup. Dictation cancels owned batch HTTP, joins admitted handlers/hotkey work and cancels optional previews without awaiting an abandoned stop promise. Remote disposal joins its enable/disable queue and retains a failed server owner.
Validation: 58 distinct focused unit cases across seven files, nine remote system cases and one real-editor-guard/application-composition renderer case; the final changed-path lane passed 21 cases. Full typecheck plus final-source incremental typecheck, contract and seven pinned-checkout checks passed. The application build/entrypoint verification (
npm run test:package) passed on the final source3d382fb7, and both CI gates (quality and minimum-Node fixture) are green on that head. The shutdown diagram was regenerated, verified and visually inspected. No live user app or microphone/provider session was used; system tests own their state/transports.The first bounded B02 repair is independently based on
115e26fc. Its focused plan is the branch's first commit (841d0949); implementation ise9b88bf5plus cancellation review3d382fb7. No provider package pin changed and no signed release was produced.Fixes #941. Fixes #942. Refs #919, #943 and #918.
B02 remains open: revision-bound editor votes, participant revalidation and final save/control-operation receipt frontiers are not implemented here. Write-tail settlement does not imply successful retries or fsync. Current support-service disposal promises do not manufacture native process-exit evidence. The pinned preview cancel API deliberately abandons a pending stop promise; the host cancels it and fences late observations. The follow-up control drain must include pre-receipt admission, nested waits/batches and internal completion writes, as recorded in #943 and the focused plan.
🤖 Generated with Claude Code
https://claude.ai/code/session_01FcJNPX21SkzfKhRFxftriT