Skip to content

bug(lifecycle): preserve services when an editor vetoes quitting #919

Closed

Description

@Juliusolsson05

Status (quality-loop audit, 2026-09-25)

Landed: #945 (d5856257): installApplicationShutdown runs disposal only after the quit is committed; regression test applicationShutdown.test.ts:60.

Remaining: Editor votes are not bound to a revision or generation: an approval from window A survives an edit in window B before the quit commits.

Acceptance: A two-window real-quit test: approve in A, edit in B, commit the quit, and the unsaved edit is protected. Re-grade to P1 if it reproduces.

Original report

Expected behavior

Quit preparation is reversible; an editor veto preserves working services and restores admission. Irreversible shutdown begins only after current editor decisions are committed.

Source finding and impact

The before-quit handler stops and clears workflow services, then stops MCP, remote, LSP, dictation and other facilities before renderer beforeunload can veto closure. Session teardown is deferred separately. A veto can leave an open application with disposed services.

Evidence and environment

Inspected source at 552914f5610518458f944fa1bdaf63f243685bd1 on 2026-09-12 in the local macOS workspace. This finding has not yet been reproduced through a running application or the package integration suite.

Regression and acceptance

Exercise the real quit composition with a dirty editor veto and verify new workflow/MCP/LSP/dictation operations remain usable. Also invalidate an earlier close approval when another window changes a buffer.

The resolving PR must protect the observable contract and document any remaining uncertainty. This planning pass makes no runtime changes.

Refs #918; planning finding F02.

Implementation loop status (2026-09-12)

Implemented on fix/quit-service-lifecycle, source commit e9b88bf5, based on 115e26fc9c67316a3b0b1b4318f47f7a2bea3606. The branch's first commit is focused plan 841d0949; worktree .worktrees/quit-service-lifecycle.

The first B02 slice composes irreversible disposal behind committed quit, joins startup, fences window creation, tracks completed/failed drain stages, retains the process lock through failure, joins pending dictation producers, and serializes remote disposal with enable/disable while retaining failed server ownership. External-control disposal no longer runs in an independent fire-and-forget listener. Services remain live after a veto in the composed test.

Verification so far: 57 unit cases, nine remote system cases, one real-editor-guard/composition renderer case; the post-review coordinator/gate/dictation subset passed 19 cases. Full typecheck, contract and all seven pinned-checkout checks passed. The shutdown diagram was regenerated, verified and visually inspected. The full application build is still running in the renderer phase under heavy local load; final-source incremental checks and the PR/CI handoff remain pending. No live user application was quit and nothing has merged.

This is a Refs #919 slice. Remaining acceptance includes generation/revision-bound editor decisions, participant revalidation and the final save/control-operation receipt frontier. Workspace/history admission-tail settlement is not fsync or retry of failed saves. The control executor's active map excludes pre-receipt admission work; nested waits/batches and the private operations.finish path must be covered without rejecting the completion evidence needed during shutdown. Details and remaining contracts are in the focused plan. Do not close #919 from this first disposal repair.

Activity

  1. Juliusolsson05 commented on Sep 12, 2026

    @Juliusolsson05
    OwnerAuthor

    PR #945 is ready for review at 3d382fb7 (both CI gates green; final-source test:package passed). It is not merged. It delivers the first bounded B02 slice (#941, #942). The remaining B02 work is tracked in #943.

  2. Juliusolsson05 commented on Sep 20, 2026

    @Juliusolsson05
    OwnerAuthor

    Audit note (2026-09-20): the bug as written is fixed; a named remainder survives, so narrowing rather than closing.

    Fixed by PR #945 (d5856257). There is now one composition — installApplicationShutdown (src/main/applicationShutdown.ts:57-) — owning the full disposal inventory (workflows, MCP, remote, LSP, dictation, external control, extensions), and it runs only behind committed quit. src/main/index.ts:1449-1453 states that rule, and :313-315 records that main's former workflow before-quit flags are gone. The one surviving before-quit handler is the extension-runtime pause, documented as reversible and resumed by Keep Editing (:1430-1447).

    Regression cover: applicationShutdown.test.ts:60 — "keeps every service and recorder usable after repeated editor vetoes" — plus ~20 further cases.

    What did not ship, and what this issue should now mean: revision/generation-bound editor votes, participant revalidation, and the final save / control-operation receipt frontier. In this issue's own words, "invalidate an earlier close approval when another window changes a buffer". PR #945's body says so explicitly, and its tests are composed unit/renderer/system — the real quit composition with a dirty editor veto has not been exercised in a running app.

    Note for anyone following the trail: the #918 audit initially pointed that remainder at #943, but #943 has since been closed as fixed (it was the drain finding, which did ship). So this issue is where the approval-revalidation frontier lives. Narrowed to that.

  3. Juliusolsson05 commented on Sep 25, 2026

    @Juliusolsson05
    OwnerAuthor

    Quality-loop re-audit (2026-09-25): the remaining scenario has no stale-vote path in the current source.

    The remainder claims that an approval from window A survives an edit in window B before the quit commits. Reading origin/main 54287d90:

    • No stored vote. The "vote" is Chromium's beforeunload, answered per window at that window's own close. useEditorBeforeUnloadGuard reads useGlobalEditorStore and the AI Workspace buffers when the event fires, not from a cached decision. appWindow.ts's will-prevent-unload handler shows that window's dialog and either lets it unload or calls onCloseVetoed. No approval is recorded anywhere for later reuse.
    • Windows close one at a time. Electron closes them sequentially during app.quit(), and will-quit (where sessionShutdownGate admits teardown) fires only after all of them have closed. Window A's "Discard Changes and Leave" destroys A, so A can't receive later edits. An edit in B while A's dialog is up lands in B's own renderer store. It is then judged by B's own beforeunload when B's turn comes.
    • Buffers are not shared. Buffers live in each renderer's store, so an edit in B cannot be covered by A's approval.

    A stale approval would need a cached cross-window decision, and none exists. The acceptance test (a real two-window quit) needs a running Electron app, which this loop does not launch. Labelled needs-evidence. Close it if the CI system tier or a manual two-window quit confirms that B still prompts after A has discarded. Re-open as P1 if B's edit is lost.

    🤖 Generated with Claude Code

  4. Juliusolsson05 commented on Sep 27, 2026

    @Juliusolsson05
    OwnerAuthor

    Closing: fixed by #945; the narrowed follow-up has no code path (triage 2026-09-27, main a70403ef).

    • The fix: since fix(lifecycle): preserve services when Quit is cancelled #945 (d5856257), services are shut down only after a quit is really committed. installApplicationShutdown (applicationShutdown.ts:60) runs after the editor veto has been resolved, and applicationShutdown.test.ts:60 covers it.
    • The narrowed remainder: a "stale approval across windows". The 2026-09-25 re-audit found no path for it. The guard reads live store state on every beforeunload (useEditorBeforeUnloadGuard.ts:8,27), and no decision is cached between windows.

    Reopen when (as P1): a two-window quit ever loses window B's unsaved edits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-evidenceNeeds a recording/real use to settlesev:P2Real bug with a workaroundtype:bugSomething works wrong

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions