(window): ask before a window close ends the app

[abate]

Stacked on #479 (9f9c57f, on main 421f77f). Until #479 is merged, this PR's diff includes its four commits; it will be rebased on main once #479 lands.

Merged with the reviewed #479. unsaved-guard.js keeps this PR's REASON_RANK / entry.upgrade and adds #479's late-save wait: every upgrade (reload to close to quit) carries a token, the page sends it back with unsaved-check-reason-done (after the write for a reload it already approved, at once otherwise), and main waits for that upgrade's own report, bounded, released when the renderer is gone, and skipped when the check ended unanswered. While a joined check waits, it stays the one in flight, so a close arriving then joins it instead of showing a second question. will-prevent-unload skips the reload of a check a close or quit joined (check.entry.reason). In public/file-panel.js the answeredReloads branch runs before the early return for an unknown id, and answeredReloads.add comes before unsavedCheckResult.

Changelog. This PR's entry is under Unreleased / Changed; the released #540 line under v0.0.93 is left as published.

Problem

The window's close button sits next to the strip's controls; a stray click ended the app and every session in it.

Change

• The question. A window close (close button, Alt+F4, the window manager's close) asks "Close Switchboard?" in the in-page choice dialog, with Cancel focused. On macOS it reads "Close the window?" and says Switchboard stays in the Dock: closing the window there does not quit, but its sessions still stop.

  • It rides on the unsaved-edits guard: the window's close event asks the renderer with reason 'close' instead of 'quit'.
  • The renderer adds the confirm for 'close' only, and only when the unsaved-edits dialog was not shown (Save or Discard there is already a decision to close).
  • Another choice dialog that is open (Archive folder) is cancelled first, so a close is never answered with a silent Cancel.

• A busy page is asked, not skipped. A 'close' check has no bound before the page acknowledges it: a page busy when the close arrives shows the question once it is free. A page still starting (no listener yet) gets the check again once it registers one: the preload sends unsaved-ready, main re-sends the pending unacknowledged check to that page with its current reason, and the renderer ignores an id it already has.

• Not asked again:

  • ☰ → Quit (Exit on Windows) and an update install.
  • A Windows logoff: query-session-end approves the quit first.
  • A quit from outside (SIGTERM, a Linux or macOS logout, an update install) that joins a pending close question. The guard upgrades the check's reason (unsaved-check-reason, 'reload' < 'close' < 'quit'), the renderer closes the close question as a yes (or never shows it), and a check not yet acknowledged gets the quit's 2.5 s bound. The unsaved-edits dialog stays, as for any quit.

• A page that stops answering is destroyed in two cases only:

  • The user closes the window again:

    • while the check has gone unacknowledged for 2.5 s;
    • or, once the page has acknowledged, after a ping has gone unanswered for 1.5 s. The ping is sent by a second close, and a page that is alive answers it in milliseconds.

    A double click does nothing, neither delay destroys anything by itself, and a quit or an install never counts as a second close.

  • Electron reports it unresponsive while the check is not acknowledged, or a ping is unanswered.

  Electron does not report a page that hung before the close, because the guard holds the close: a single-close journey timed out after 60 s (run 38070586412). So a single close of a hung window waits, and the second close is what ends it. The changelog entry of (window): close a window whose page stopped answering #540 says so.

• A quit that the page never acknowledges goes ahead after 2.5 s; a page still busy about 7.5 s after the quit is destroyed with its pending working-set save ((window): close a window whose page stopped answering #540).

• Native dialogs. While the page shows a native alert, confirm or prompt, nothing is forced, and a quit's bound becomes a 5-minute ceiling.

• Focus. Cancel returns the focus to where it was.

• Docs: .ai/contexts/window-frame.md ("Closing the window", "A window that stops answering", "A native dialog pauses the bounds"), viewer-panel.md and docs/e2e.md.

Tests

• test/unsaved-guard.test.js:
  • a close sends 'close', and has no bound before the ack;
  • unresponsive ends an unacknowledged or pinged check, and leaves one being answered;
  • the ping on a second close, the overdue rule, and that a quit never pings;
  • the reason upgrade, plus the quit bound it brings;
  • the native-dialog hold and ceiling, and the pong sender check.
• test/dom-file-panel-unsaved-guard.test.js:
  • close asks; Cancel, Enter and Escape keep the window;
  • quit does not ask;
  • a close over unsaved edits asks once;
  • a quit joining the close question closes it as a yes;
  • the macOS wording;
  • native dialogs;
  • focus.
• test/dom-choice-dialog.test.js: closeWith.
• e2e/hung-window.spec.js (from (window): close a window whose page stopped answering #540) now closes twice: the hung window goes on the second close, and the journey exits in 7.8 s.
• e2e/close-window.spec.js: a real window close shows the question, Cancel keeps the window and a terminal that still runs commands, and Close exits with code 0; Quit from the menu exits with code 0 without asking.
• Each fix was mutation-checked: removing it fails a test.
• The ready re-send: tests first (5165954, run 38140789371 red on exactly its three tests on all four test jobs), then the fix (4ced00b); the head's run is linked in the latest comment.

taskkill without /F sends a close and gets the question. /F, or an installer that escalates to it, ends the process without the quit cleanup, as before (documented in window-frame.md).

🤖 Generated with Claude Code

Round 2026-10-11 (nit from the 13:07 re-check)

A check is forgotten only once answered, so one main sends again on unsaved-ready during the exit write is ignored instead of starting a second write. Tests first: 62a9064, red run 38147038161 (exactly the new test, all four test jobs; the rebased 6d61200 differs only by the #479 changelog line). Fix: 8e1f08f, green run 38147558889.

[devsuitup]

Review at a72aa4e. The confirmation itself is wired through the existing unsaved-edits check, which is the right place. One point blocks, found by tracing the code.

Blocking

1. A renderer that hangs after the ACK makes the window impossible to close (public/file-panel.js onUnsavedCheck, unsaved-guard.js unsaved-check-ack). The renderer sends the ACK first, and the ACK cancels the main-side timeout. If the renderer then blocks (a stuck confirm, a hung page), neither a result nor a timeout ever arrives, and both the close and the deliberate quit stay pending. The weakness existed for unsaved edits; this PR extends it to every close, including a window with nothing unsaved. Fix: after the ACK, keep a longer bound (or re-arm the timer), or let a second close request override the pending one. Test: ACK, never answer, then check that a second close or the bound ends it.

Non-blocking

• The new confirm('Close Switchboard?') has no test for its default focus or Enter/Escape behaviour (test/dom-file-panel-unsaved-guard.test.js: the confirmation is stubbed). Enter approving a close by accident is plausible and not verified.
• The CHANGELOG entry must end with (#478).

393 tests in the nine related files pass locally on Node 24. Not verified: Node 20/22 with c8, and the native dialog.

[jbr-sekoia]

Tested on Linux (Ubuntu, GNOME on Wayland), PR head a72aa4e.

Setup. An isolated instance with fixture HOME and plain-shell sessions, launched through Playwright _electron. window.confirm was replaced in the renderer by a stub that records its message and returns a chosen answer. The native GTK dialog's look and its Enter/Escape defaults are therefore not covered.

Window close (BrowserWindow.close(), the path the compositor's close request takes), with a running shell

• Cancel. confirm('Close Switchboard?') is asked once. The window stays, and the shell PTY keeps the same PID. Works.
• OK. The process exits 0.7 s later, and the shell is gone. Works.

☰ → Quit (role: 'quit'). It exits in 0.2 s with no question, as the PR describes.

Close over unsaved edits (dirty file tab). "Unsaved file edits" is shown, and "Close Switchboard?" is not asked. Cancel keeps the app and the edit, and Discard exits at once. Works.

Blocking point: the renderer ACKs, then hangs. This reproduces live. The hang was a 60 s busy loop in place of confirm. During it, none of these closed the app:

• a second close();
• app.quit() (the Quit menu path);
• Ctrl+Q sent as an input event;
• SIGTERM to the main process, which was still alive 10 s later.

No unresponsive event fired. When the loop returned No, the pending close and the pending quit were both dropped. Only killing a process ended it: SIGKILL of the renderer (render-process-gone, then exit in 0.2 s) or of the main process. The PR extends this state from "while the unsaved-edits dialog is open" to "while any window close is being confirmed".

Found alongside, in the existing guard: the 2.5 s bound when the renderer hangs before the ACK. The bound fires and calls win.close() again, but the window does not close: Chromium waits for the hung renderer's beforeunload listener. A 30 s hang exited at about 30 s, and a 75 s hang at about 75 s. This predates the PR, which only changes the reason string in unsaved-guard.js. It contradicts "a renderer that does not acknowledge the check within 2.5 s still lets the window close" in window-frame.md and viewer-panel.md.

Not run:

• a physical click on the native close button: on Linux it is drawn by Electron's OpaqueFrameView, outside the page, and calls the same close();
• Alt+F4 and a real Ctrl+Q: both need focus.

The PR's test files pass: test/unsaved-guard.test.js and test/dom-file-panel-unsaved-guard.test.js, 26 tests.

[abate]

Thanks, and thanks @jbr-sekoia for the live reproduction and for finding the pre-ack bound problem. Rebased on current main (eaf59ff); the fixes are in 150b213, on top of the rebased original commit.

Blocking: a renderer that hangs after the ACK

I took the second option: a repeated request ends the pending check, but only once the renderer has shown it cannot answer.

• Probe: a close or a quit that arrives while an acknowledged check is pending sends unsaved-ping. That covers a second close and before-quit, which is the Quit menu, and SIGTERM, which reached before-quit and was held there in the live test. The preload answers with unsaved-pong, a listener added with onUnsavedCheck that runs on the renderer's main thread, so a hung page cannot answer.
• No answer within 1.5 s (DEFAULT_PROBE_MS): the check ends as a yes and the window is destroyed. The pending close and quit then go through.
• With an answer: the question stays up and the user decides. A later request checks again.
• The page stays able to answer while it asks. The close question is no longer a native confirm(), which blocks the page, but the in-page choice dialog. The unsaved-edits dialog is in the page too. So a second close while either dialog is open never forces the close, and never throws away edits the user is being asked about.

The 2.5 s bound before the ACK (found in the live test, predating this PR): when it fires, the window is now destroyed instead of closed. close() waited on the hung page's beforeunload, which is why the window stayed open until the hang ended. window-frame.md and viewer-panel.md now describe this, and the new "A window that stops answering" section in window-frame.md covers the probe.

Tests (test/unsaved-guard.test.js):

• An ACK, then a second close with no pong: the window is destroyed, not closed.
• An ACK, then a second close answered by a pong: no destroy and no close, and a later close pings again.
• An ACK, then a quit with no pong: the quit goes through and the window is destroyed.
• Before the ACK, a second close sends no ping.
• When the bound fires, the window is destroyed.
• The shipped preload answers a ping once the check handler is registered.

Non-blocking

• Enter/Escape: the close question is now showChoiceDialog with initialFocus: 'cancel'. A test checks that Cancel has the focus, Enter on it keeps the app, Escape keeps the app, and Close proceeds, with no native dialog shown.
• CHANGELOG: the entry ends with (#478), and mentions Cancel being selected and how a hung window can be ended.

Each part was removed in turn, and each removal fails a test: the probe on close, the probe on quit, the destroy, the forced bound, the pong handling, the ACK condition and the dialog focus.

The related suites pass (646). On the full suite, the only failures are the CLI-state canary, test-pr and sandbox-wrapper tests, which fail the same way on main in my sandboxed environment. Not run: a live Electron, so the native frame's close button, Alt+F4 and a real SIGTERM are covered only by the guard tests.

#479 changes the same onUnsavedCheck handler (the exit flush), so whichever merges second has a small textual conflict there.

[devsuitup]

Review at 150b213, rebased on current main. Thanks: the earlier blocker is fixed (a renderer that stops answering after the ACK is now probed with a ping and the close recovers), and so are the Cancel/Enter/Escape tests and the changelog suffix (#478). 409 tests in 12 related files pass locally on Node 24. One point blocks until it is shown not to happen; the reviewer rated it plausible, not reproduced, and I re-read the path.

Blocking (to verify)

1. The probe can destroy the window while a native dialog is waiting for the user (unsaved-guard.js, probe ~83 and end ~98; public/viewer-panel.js ~477, public/file-panel.js). On a deliberate quit, beforeQuit pings the renderer and, if no pong comes back within probeMs (1.5 s), calls entry.finish(true, true) and then win.destroy(). The check itself waits on native dialogs: the confirm('Close Switchboard?'), and the overwrite confirmation shown while Save is waiting. If the renderer does not answer the ping while such a dialog is open (it depends on whether the modal's nested loop dispatches the preload's ipcRenderer.on('unsaved-ping') handler), the window is destroyed after 1.5 s with the dialog unanswered, and unsaved edits are lost; for the close prompt, the confirmation disappears by itself. Please verify in a real Electron run, and add a test through the real handler: with a dialog open, a slow answer (longer than probeMs) must not force the close. If the pong is not served during a modal, pause the probe while a dialog is open (the renderer can tell main when it opens and closes one).

Non-blocking

• Focus is not restored after Cancel (public/file-panel.js ~807).
• Rationale comments at unsaved-guard.js ~35 and ~94 go beyond the one-line pointer allowed in code.

Not run: native Electron behaviour, a real Windows shutdown, Node 20/22 with c8, lint, fresh CI, the combination with #479 and #441. CI on this head is not approved yet.

[abate]

Thanks. Rebased on 805d064, the commit before #499: my token cannot push a branch that contains the .github/workflows change, so the branch is four commits behind main. None of those four touches these files. The conflicts with #490/#496 in viewer-panel.md were documentation only. The fix is in 62fe805.

Blocking (to verify)

1. The probe can destroy the window while a native dialog is open.
   • Not verified in Electron. Electron does not start in my sandboxed session: it segfaults under Xvfb, which itself crashes, and with the headless ozone platform. What follows is reasoned from Chromium's model of synchronous dialogs, not run.
   • The premise holds, though not on the path cited. alert, confirm and prompt block the page's main thread, and the preload's unsaved-ping listener runs on that thread, so it cannot answer. Since (touched): open every clicked file in Touched, and drop the separate file tab #490 the check's own path shows no native dialog. The close question is the in-page choice dialog (since 150b213), and the unsaved-edits Save goes through handleChangesSave and saveTouchedStash, which report a refusal in the dialog. Other code does show native dialogs, though: the sidebar's delete and hide confirms, the resume guard, the viewer's overwrite and delete confirms. A close or quit that arrives while one is up could not even be acknowledged, so it hit the 2.5 s bound, or the 1.5 s probe once acknowledged, and the window was destroyed with the dialog unanswered.
   • Fix.
     • reportNativeDialogs (file-panel.js, run by initFilePanel) wraps window.alert, confirm and prompt, and sends unsaved-dialog true before the call and false after it, through the new preload unsavedDialog. A bare confirm(…) resolves to the wrapper too.
     • In main, while that page has a dialog up, neither bound runs. A check waits unacknowledged, and a probe sends its ping but starts no timer.
     • When the dialog closes, the bound that applies starts again from zero.
     • A crash or a navigation of the page clears its flag, so a dialog that never reported its end cannot pause the bounds for good. A flag reported by another page does not count.
   • Cost: while a native dialog is up, a quit (SIGTERM included) waits for the user to answer it. A Windows logoff approves the quit first, so it does not wait.
   • Tests:
     • test/dom-file-panel-unsaved-guard.test.js wires the shipped file-panel.js to the shipped guard. A close that arrives during a page confirm runs no 2.5 s bound until the dialog is answered, and then the page shows its dialog. A second close during another confirm pings but runs no 1.5 s bound. The pong clears it, and Save then closes the window, with no destroy and the edits on disk.
     • test/unsaved-guard.test.js adds a slow answer past probeMs with a dialog open, which is not forced, then a dialog that opens during a probe and a page that hangs after it closes, which is still ended. It also covers a check sent behind a dialog, a flag from another page or one cleared by a crash or a navigation, and the preload channel.
   • Described in window-frame.md, "A native dialog pauses the bounds".

Non-blocking

• Focus after Cancel. The close question passes returnFocus to showChoiceDialog, and the unsaved-edits dialog restores the element focused before it on Cancel or Escape. Tested for both.
• Comments. The rationale above askFull is now a one-line pointer. The /* the timer ends the check */ in probe is gone: a ping that cannot be sent now ends the check at once, as a failed unsaved-check send already did. Tested.
• CHANGELOG: the entry now says a confirmation box waiting for an answer is not ended.

Each fix was removed in turn, and each removal fails a test: ignoring the flag, no re-arm when the dialog closes, never clearing the flag, the crash and navigation resets, any sender counting, the probe token and the pong re-arm, no wrapper, no false report, the preload channel, both focus restores, the failed-ping end.

The full suite has 17 failures, the same 17 as upstream/main in my sandboxed environment: the CLI-state canary, test-pr isolation and four sandbox-wrapper tests. Lint: 0 errors. Not run: Electron itself, Windows, macOS, the combination with #479 and #441.

[devsuitup]

Review at 62fe805. Thanks for the quick turnaround and the careful write-up. Both earlier blockers are addressed in the code, and the tests catch it when each fix is removed (checked locally by mutation: removing the dialog pause, win.destroy(), the quit exemption or the native-dialog wrapper each turns tests red). 62fe805 merges cleanly into current main (75ab0ad); the two PR test files pass on the merge (39/39).

Earlier findings

• Renderer hangs after the ACK: resolved; a repeated close or quit now probes the page (unsaved-guard.js:98-114, :120-158). A single close after an ACK still waits for a repeat, as documented.
• Enter/Escape on the close question: resolved (public/file-panel.js:725-735, initialFocus: 'cancel').
• (#478) suffix: resolved (CHANGELOG.md:12).
• Probe vs. a native dialog: resolved in code (public/file-panel.js:709-722; unsaved-guard.js:24-30, :61-66, :142-143); not yet seen in a live Electron run.
• Focus after Cancel: resolved (public/file-panel.js:625, :678, :732).
• Rationale comments: resolved.
• Pre-ACK bound that did not close a hung window: resolved (unsaved-guard.js:64, :116-118).

Major

1. The changelog entry lands in the released v0.0.90 section on merge (CHANGELOG.md:12). v0.0.90 was released after your base and moved the old ## Unreleased items under ## v0.0.90 — 2026-10-09. Merged into main, the hunk applies by context with no conflict and the entry ends up under ## v0.0.90 → ### Changed; the changelog check only looks for the file in the diff, so it stays green. Since you cannot rebase past #499, we can move it under ## Unreleased → ### Changed when merging — please confirm that is fine with you.

2. A window close is silently cancelled while another choice dialog is open (public/file-panel.js:725-735 with public/choice-dialog.js:25). showChoiceDialog returns null at once if a .choice-dialog-overlay exists, and confirmWindowClose reads null as Cancel. With the Archive folder dialog open (public/sidebar.js:1189), Alt+F4 or the taskbar's Close window shows nothing and the window stays, with no feedback. Tell "already open" apart from "cancelled" (dismiss the other dialog first, or show the close question anyway), and add a test: with a choice dialog open, a close check still shows "Close Switchboard?" or resolves deliberately, never as a cancel.

3. Evidence for the bug-fix part, and integration coverage. The PR also fixes a bug present on main (the pre-ACK bound now destroys a hung window instead of calling close() on it). For bug fixes we ask for (a) a CI run that fails before the fix — the regression test pushed alone, failing in a required job, then the fix — and (b) integration coverage where feasible: all the new coverage drives a fake BrowserWindow, while the required e2e job already runs Playwright Electron journeys. Could you add an e2e/ journey for close → Cancel keeps the window and the PTY, close → Close exits, and ☰ → Quit does not ask, and provide the CI-red step for the hung-window fix? If either is not possible in CI, please say concretely why; e2e runs green on this head.

Minor

4. unsaved-guard.js:64, :116-118: a close during startup (before initFilePanel registers onUnsavedCheck) or while the page is busy for more than 2.5 s gets no ACK; the bound then forces the close and destroy() ends every session without the new question, where it used to go through close(). Consider forcing only once the page has also failed a ping, or document that the question is skipped there in window-frame.md.
5. main.js:3262: confirmQuit (updater install) goes through ask() and drops forced, so a page hung before the ACK still blocks the window close after quitAndInstall(). Predates this PR; reusing askFull and end() would make the paths consistent.
6. unsaved-guard.js:63: the native-dialog pause has no upper bound, so SIGTERM or a Linux/macOS logout wait indefinitely while a native confirm is open. Consider a long ceiling (minutes) for a quit that does not come from the user.

Suggestions

• unsaved-guard.js:18-21: unsaved-pong does not check event.sender; only entry.wc should be able to answer.
• On macOS the app stays in the Dock after the window closes (main.js:3468), so "Close Switchboard?" is slightly off there.

Not verified: native Electron behaviour, a real Windows shutdown, macOS, and the combination with #479 and #441.

Changes requested: 0 blocking, 3 major, 3 minor, 2 suggestions.

[abate]

Rebased on current main (75ab0ad), now that my token can push a branch that contains the workflow change of #499. New head 6fda14c; the patch is the same as 62fe805, with no conflicts. Full suite and lint at the new head: only the failures this sandboxed environment has on main.

[devsuitup]

Thanks for the rebase. git range-diff of 62fe805 against 6fda14c shows the same patch (only context lines move), so the findings of the review at 62fe805 stand.

One thing the rebase did not fix: the (#478) changelog line is still under ## v0.0.90 — 2026-10-09, a released section, in CHANGELOG.md at 6fda14c. Now that you can push on top of current main, please move it under ## Unreleased (same subsection).

[abate]

Thanks. Based on #540 now, on current main (a02f96c). #540 is the hung-window fix split out, as asked in Major 3. The earlier three commits of this PR are squashed into 44db05e on top of it, because they rewrote the same lines #540 now changes. This round is 93e05e8, plus 8ec5ef5 for the e2e fixture.

Major

1. Changelog. The line is under ## Unreleased → ### Changed.
2. A close cancelled by another choice dialog.
   • showChoiceDialog takes replace, and confirmWindowClose passes it. A dialog already open, such as Archive folder, ends as a cancel, so nothing it offered is done, and the close question shows.
   • Cancel on the close question gives the focus back to where it was before either dialog.
   • Every other caller keeps the old behaviour: a second dialog yields to the open one.
   • Tests: with the Archive dialog open, a close check shows "Close Switchboard?", the other dialog resolves null, and Cancel then Close answer no then yes. A second test checks that a dialog without replace still yields.
3. CI evidence and integration coverage.
   • (a) Split. The pre-ACK fix is now (window): close a window whose page stopped answering #540, its own PR. Its test commit f1e68a9 failed in CI on exactly the three new tests, on every test job (run 38039912120). The fix commit 1fb50e9 is green (run 38040440728); both runs are listed on (window): close a window whose page stopped answering #540.
     • It also changes the fix, which covers your Minor 4. A window whose page never acknowledges is closed the usual way first, so a page that is only starting up or busy closes as before. It is destroyed only if that close has not finished 3 s later (DEFAULT_CLOSE_GRACE_MS).
   • (b) e2e. e2e/close-window.spec.js has two journeys:
     • BrowserWindow.close() from main brings up the question. Cancel keeps the window, the terminal is still listed by get-active-sessions and still runs a command that writes a file, and a second close answered Close ends the process.
     • Clicking the menu's quit role item ends the process with no question. If it asked, the process would not exit.
     • CI run 38041869326: both journeys pass in the e2e job, and every other job is green.
     • The fixture needed one change (8ec5ef5). Its teardown called app.process() and app.close() on an app the journey had already quit, and Playwright throws there. It now keeps the process from launch, and skips the trace, the screenshot and the close once that process has exited. docs/e2e.md says so. Both journeys are listed in docs/e2e.md. I did not run the mutation runs that docs/e2e.md lists for the other journeys.

Minor
4. Forced close during startup or a busy page. Fixed as described under 3a. Pages that cannot acknowledge in 2.5 s close without the question, as before this PR; window-frame.md says so.
5. confirmQuit and forced. confirmQuit now uses askFull. An install the page never acknowledges is approved, and the window is destroyed if its close has not finished 3 s later, so quitAndInstall() still closes it the usual way first. Tested.
6. No limit on the native-dialog pause. The pause now lasts at most 5 minutes (DEFAULT_DIALOG_CEILING_MS), for every request; a quit from the user while a dialog is open waits that long too. At the ceiling, a check that was not acknowledged closes the window (destroyed after the grace period), and an unanswered probe destroys it. Tests cover the quit (SIGTERM path) and the probe.

Suggestions

• Pong sender. unsaved-pong counts only when event.sender is the probed window's webContents. Test: a pong from another sender leaves the bound running.
• macOS wording. Left unchanged. The window close there also stops the sessions, since the window's closed handler kills every PTY, so "Every session running in it stops" holds; only the app stays in the Dock.

Mutation check: each of these changes was removed in turn and each removal fails a test:

• replace in confirmWindowClose;
• the cancel of the open dialog;
• replace honoured only when asked;
• the focus carried past the replaced dialog;
• the sender check;
• the ceiling;
• the bound after an unacknowledged install;
• the grace period instead of an immediate destroy;
• no bound for an acknowledged check with no probe.

Full suite at 93e05e8: only the failures this sandboxed environment has on main (the CLI-state canary, test-pr isolation, the four sandbox-wrapper tests that need #483). Lint has 0 errors.

Not run locally: Electron, which segfaults in this environment, so the e2e journeys were checked only in CI. Also not run: a real hung renderer, a real SIGTERM with a native dialog open, macOS and Windows.

[devsuitup]

Re-review at 8ec5ef5 — request changes (1 major inherited from #540, 4 minor, 4 suggestions). The #478 commits (44db05e, 93e05e8, 8ec5ef5) close every finding from the 62fe805 review except the macOS wording.

Previous findings

Finding	Status
M1 changelog under a released section	closed (CHANGELOG.md:7-8, Unreleased / Changed)
M2 close silently cancelled over another choice dialog	closed (choice-dialog.js:29-33 replace, file-panel.js:726-737; test 10 fails on 44db05e)
M3a e2e coverage	closed (e2e/close-window.spec.js, 2/2 in job 114183554769)
M3b red before the fix	closed via #540 (run 38039912120 on f1e68a9 fails the 3 new tests, green on 1fb50e9)
m4 startup/busy page closes without the question	documented only — see Major
m5 confirmQuit drops the forced end	closed (test 49)
m6 native-dialog pause unbounded	closed (5-min ceiling unsaved-guard.js:67, tests 46/47)
S pong from any page	closed (unsaved-guard.js:21-24, test 48)
S macOS wording ("Close Switchboard?" while the app stays in the Dock)	open

Local replays of the head's two test files (Node 22): head 50/0, main 29/21, 1fb50e9 32/18, 44db05e 45/5 — each 93e05e8 fix has a test that fails without it. CI green on 8ec5ef5 (runs 38041869326 / 38041869325).

Major — a busy page loses the question and is destroyed (unsaved-guard.js:124-134, :67; CHANGELOG.md:8)
Inherited from #540 and extended: a page busy more than 2.5 s does not acknowledge the check, the close proceeds without the question and the window is destroyed ~5.5 s after the click; #478 adds a second fixed bound, the 1.5 s probe, which destroys without calling close() first. Renderer stalls of 8–10 s happen today on tab reveal (#526), so a stray click on the close button during one — the very case this PR is for — ends every session with no question, and drops the debounced persistWorkingSet (and #479's flush once it lands). The head test a page that never acknowledges is closed, and destroyed when that close does not finish asserts exactly this. The changelog says closing "asks for confirmation first" unconditionally. Please settle it in #540 (e.g. key on Electron's unresponsive/responsive), and for the 'close' reason do not force before an acknowledgement unless the user closes again or the page is reported unresponsive; qualify the changelog line.

Minor

1. .ai/contexts/window-frame.md:258, CHANGELOG.md:8: on Windows Electron's quit role reads "Exit" and has no Ctrl+Q accelerator (window-frame.js:45 sets none), so "Ctrl+Q" / "Quit" are wrong there.
2. unsaved-guard.js:136-150 (plausible): an external quit (SIGTERM, Linux/macOS logout) arriving while "Close Switchboard?" is shown joins the pending check; the page answers the ping so no bound runs, and Cancel drops the quit (if (!proceed) return;).
3. unsaved-guard.js:67, :103-122 (plausible): the 1.5 s probe treats a busy page as hung when a SIGTERM or an updater install counts as the second close.
4. window-frame.md:318 (and :276): the "not run: Electron does not start in the sandboxed session" paragraph is authoring history, stale now that e2e runs Electron; two lines are not wrapped like the rest.

Suggestions

• e2e/close-window.spec.js:40, :63 only check the process exited; assert exitCode === 0 so a crash cannot pass.
• taskkill without /F now shows the question; a tool that then escalates to /F (possibly the NSIS running-app check, not verified) skips the quit cleanup — worth documenting.
• #479 conflicts here (preload.js, file-panel.js, unsaved-guard.js); its flush runs for any reason but 'reload', so keep it after confirmWindowClose when resolving.
• macOS wording, as above.

The e2e fixture change (8ec5ef5) neither hides a teardown failure nor leaves Electron running: Playwright's close() calls app.quit(), and a journey failing before exit still goes through capture, close, wait, then forced kill.

Not run here: native dialogs in Electron, a real Windows shutdown/taskkill, macOS, the combination with #479/#441.

[abate]

Addressed at ea62821, rebased on #540 (fba2056). The #478 part is now a single commit, because the guard was reworked.

Major (a busy page loses the question and is destroyed).

• A 'close' check has no bound before the acknowledgement: a page busy when the close arrives shows the question once it is free.
• The 1.5 s probe no longer destroys anything by itself. A second close pings the page, and a double click is answered in milliseconds. The window is forced (a destroy) only:
  • when Electron reports the page unresponsive while the check is unacknowledged or a ping is unanswered;
  • when the user closes again after a ping has gone unanswered for 1.5 s.
• An acknowledged check with no ping out ignores unresponsive, since the user may be reading the question.
• The test a page that never acknowledges is closed, and destroyed when that close does not finish is replaced by a window close the renderer has not acknowledged waits: a busy page is asked once it is free, plus the unresponsive, ping and overdue tests.
• The changelog says a busy window asks once it is free, and that a stopped one is ended on the system's report or on a second close.

Minor 1 (Exit / Ctrl+Q). The changelog says "Quit, or Exit on Windows", and window-frame.md gives the accelerators per platform.

Minor 2 (an external quit dropped by Cancel). A quit that joins a pending close check now upgrades its reason. Main sends unsaved-check-reason (id, 'quit'; ranks 'reload' < 'close' < 'quit'), the same channel #479 uses. The renderer then closes the close question as a yes (closeWith on the choice dialog), or never shows it, and an unacknowledged check gets the quit's 2.5 s bound. The unsaved-edits dialog stays, so Cancel there still keeps the app, as for any quit. Covered by a guard test and a DOM test.

Minor 3 (SIGTERM/updater as the second close). beforeQuit and confirmQuit no longer probe. Only the window's own close event counts, and a test pins that a quit and an install never ping.

Minor 4. The stale "not run" paragraph is gone, and the sections are rewrapped and rewritten for the new rules.

Suggestions.

• e2e/close-window.spec.js asserts exitCode === 0 and no signal on both journeys.
• taskkill / taskkill /F are documented in window-frame.md ("A window that stops answering").
• macOS: the question reads "Close the window?" and says Switchboard stays in the Dock (closed still stops the sessions there), with a DOM test.
• (restore): save the open sessions when the app closes #479: same channel name, so when this is rebased on (restore): save the open sessions when the app closes #479 its flush stays after confirmWindowClose. Merge order is (sandbox): protect repositories below a symlinked Additional Directory #538, (restore): save the open sessions when the app closes #479, (window): close a window whose page stopped answering #540, then this PR.

Mutation checks. 18 single mutations of the new guard, renderer and dialog code each fail a test. The one exception is the renderer ignoring a reason for an id it no longer tracks, which only avoids a stale map entry.

Locally (Node 22). Full suite 5110 pass / 17 fail, all of them the sandboxed-session baseline (CLI-state canary, 4 sandbox-wrapper, 12 test-pr "isolated" tests). The touched files pass 73/73.

[devsuitup]

Re-review at 08fea1c — request changes, wording only (3 minor). The behaviour is acceptable; what is left is docs and changelog that do not match the code. Every required check is green (run 38071187146, e2e job 114268758666); locally (Node 22) unsaved-guard, dom-file-panel-unsaved-guard and dom-choice-dialog 75/75.

On ea62821 the required e2e was red (hung-window.spec.js:14 timed out, run 38070586412) while it passed on #540's head: a window already flagged unresponsive and then closed once never ended. 08fea1c settles it as a design choice: a 'close' check unacknowledged after 2.5 s is marked overdue and the next close destroys the window (probeOrForce), a double click does not; new test "closing again a page that left the check unacknowledged for the bound ends it; a double click does not", and the hung-window journey now closes twice. window-frame.md:293-297 states that a single close on a hung page never ends it. The other 8ec5ef5 points (Windows "Exit"/no Ctrl+Q, external quit dropped on Cancel, a quit counted as the second close, stale paragraph, exitCode 0, taskkill, macOS wording) are closed.

Minor

1. window-frame.md:267 says a page "still starting" shows the question once it is free. A close before initFilePanel (app.js:1939; the window is shown at creation) sends an unsaved-check nobody listens to yet, so the question never shows; a second close more than 2.5 s later destroys the window without it (no longer dead for the session, as on ea62821). Say so, or have the renderer re-ask an unacknowledged check once ready (did-finish-load or a ready signal).
2. CHANGELOG.md:11 (#540's entry) says the window "closes as before, and once the system reports it not responding … closed anyway". With #478 the guard holds a window close before beforeunload, so Electron never reports the page unresponsive and a single close waits; whichever of #540/#478 lands second should fix that line.
3. The quit path still calls win.close() after the 2.5 s bound (beforeQuit, confirmQuit), so per my #540 review a page still busy ~7.5 s after a quit is destroyed; the window-frame paragraph "A check the page never acknowledged for a quit…" should say it. (This does not apply to a window close before the acknowledgement.)

Still open: the #479 conflict. Not run: Electron locally, native dialogs, macOS, the combination with #479.

[abate]

Follow-up at 08fea1c. CI on ea62821 showed something the design had not covered: with a 'close' check held, Electron does not report a page that hung before the close as unresponsive. The single-close hang journey timed out after 60 s (run 38070586412). On #540 alone the close goes ahead after the bound, so Electron reports the page about 5 s later and the window is destroyed.

So for an unacknowledged close, a second close counts once the check has gone 2.5 s without an acknowledgement (entry.overdue). The marker destroys nothing by itself, a quicker repeat such as a double click does nothing, and a native dialog in the page holds it. After an acknowledgement, the ping rule stays as before.

e2e/hung-window.spec.js now closes twice, at 2 s and at 6 s, and the app exits with code 0 after 7.8 s. CI is green on 08fea1c (run 38071187146), including the close-window journeys with their exitCode === 0 asserts. The PR body and window-frame.md say this.

This is the trade-off you asked for: a single close of a hung window does nothing until the user closes it again, and a busy window is never closed without the question.

[abate]

Rebased on the reviewed #479, which is stacked on main 24ac041. Main already merged #479 at f39d5ea through #555, so #479 now carries only its late-save wait.

Major (the conflict with #479): resolved as you described.

• askFull keeps REASON_RANK / entry.upgrade. Every upgrade still sends unsaved-check-reason while the check is pending in main, including when the renderer has already answered a reload.
• (restore): save the open sessions when the app closes #479's new wait also applies to every upgrade. Main holds the joined request until the page sends unsaved-check-reason-done.
  • The renderer sends it after the late write for an approved reload, and at once otherwise.
  • The wait is bounded by the check timeout. It is skipped for a declined check, and for one that ended unanswered (ended).
• will-prevent-unload reads check.entry.reason, so a reload that a close or quit joined is not reloaded first.
• file-panel.js: the answeredReloads branch runs before the early return for an unknown id, and answeredReloads.add comes before unsavedCheckResult.
• Tests: this PR's files carry (restore): save the open sessions when the app closes #479's tests, including the bounded, declined and hung cases. One test of this PR, the close joined by a quit, now has the page report done, as a real page does.

Minor 2, now a nit (a resent check during the exit write): a check is now forgotten only after unsavedCheckResult. A resend on unsaved-ready during the write is ignored by the existing has(id) guard.

• Tests first: 62a9064, red run 38147038161, on exactly a check sent again while the page writes the app state for it is not asked or written a second time, all four test jobs. The rebased 6d61200 differs from it only by (restore): save the open sessions when the app closes #479's changelog line.
• Fix: 8e1f08f, green run 38147558889, every job including e2e.

Nit 3 (5165954's message): not changed yet. That commit is now c119179, and its body still describes the fix, which is in 75e71f2. I will reword it when this branch is rebased onto main after #479 lands, so CI does not have to run again just for a message.

Local: the full suite fails only the 13 known sandbox-baseline tests, and lint has 0 errors.

[devsuitup]

Review of 8e1f08f: changes requested.

• "One write for one check" is fixed (verified). file-panel.js:196 drops the id only after unsavedCheckResult, and :175 ignores a duplicate. Locally, 6d61200 fails 1 of 103 (:398, 2 instead of 1) and 8e1f08f passes 103 of 103.
• The "still starting" round fails first, then passes. c119179 fails 3 of 102 on assertions. 75e71f2 passes 102 of 102. CI is green on 8e1f08f.
• Major: CHANGELOG lands in a released section. Same problem as on (restore): save the open sessions when the app closes #479. Merged onto 79e2589, the whole ### Changed entry, the rewrite of the (window): close a window whose page stopped answering #540 line and the (restore): save the open sessions when the app closes #479 extension all land under the released ## v0.0.93. Please rebase onto 79e2589 and put the entries under Unreleased.
• Minor: the (restore): save the open sessions when the app closes #479 race shows here too. A replay on 8e1f08f gives unsaved-check 2 close, so "Close Switchboard?" can show for up to 2.5 s while an approved quit is held. Fixing it in (restore): save the open sessions when the app closes #479 covers this PR.
• Nit: one done releases every wait for an id (replayed main-side). With unsaved-check-reason-done at unsaved-guard.js:60-63, the sequence reload → close joins → quit joins → result → one done ends in quit() before the late save reports. It needs a close and a quit within one IPC round trip, so it is rare.
• Searched, nothing found.
  • No quit hangs: every path is bounded.
  • A busy acknowledged window is destroyed only through probeOrForce, which this PR introduces; it is not on main.
  • A quit during the close dialog proceeds.
  • Logoff is fine.
  • Comments are one-line pointers.
• c119179's commit message still describes the fix; reword it at the rebase, as you said.

[abate]

Addressed at 24aa642, rebased onto #479's new head 9f9c57f (main 421f77f).

Major (changelog in a released section). The (#478) entry is under Unreleased / Changed. The released #540 line under v0.0.93 is restored to its published text; the Unreleased entry already says that a window that stopped responding needs a second close.

Minor (#479 race: "Close Switchboard?" during a held approved quit). Fixed in #479 (9f9c57f): a joined check stays in flight until its late-save wait ends, so a window close in between joins it. The rebased #479 test a close or quit arriving while a joined quit waits for the late save joins it instead of asking again runs here with this PR's guard and emits exactly that close: one unsaved-check, no second question.

Nit (one report releases every wait for an id). Each upgrade now carries a token (unsaved-check-reason: id, reason, token); the page echoes it in unsaved-check-reason-done (id, token), and main releases only that upgrade's wait. Tests (tests-first commit 1969b7c):

• each upgrade of a check is waited for by its own report, not by the first one to arrive: reload, a close and a quit join, answer, the quit upgrade's report alone does not quit; the close upgrade's report then does.
• the page reports each upgrade with the token it came with (renderer: the second upgrade reports at once, the first after its save).
• the preload passes an upgrade token through, and back with the report.
  Red: run 38217972611 fails exactly those three on all four test jobs. Green: 24aa642, run 38218674005, every job including e2e. Mutations (main sends no token; one report releases all; preload drops the token either way; renderer drops it) each fail a test.

c119179's message. Reworded during the rebase (now 977b852): it describes the tests and the contract they pin, not the fix.

Locally: full suite fails only the 13 sandbox-baseline tests; lint 0 errors.

[devsuitup]

Re-review at 24aa642 (stacked on #479 at 9f9c57f). Changes requested. The only blocking item is the CHANGELOG; the code is ready.

Previous review (8e1f08f)

#	Finding	Status
1	Major: CHANGELOG entries in a released section	Reopened by the v0.0.95 cut. At 24aa642 the entry sits under Unreleased relative to 421f77f. Main is now abc9a54: ## Unreleased was renamed ## v0.0.95 — 2026-10-12 and a new, empty ## Unreleased opened above it. A trial merge onto main conflicts only on #479's line. The ### Changed (#478) entry merges without a conflict but lands under ## v0.0.95. Please rebase onto main (after #479) and put both entries under the new ## Unreleased. The restored #540 line under v0.0.93 now matches its published text.
2	Minor: #479 race	Closed for a single upgrade (fixed in #479 at 9f9c57f). Two upgrades still leave a gap; see the new nit below.
3	Nit: one "done" releases every wait for an id	Closed. Each wait has its own token (unsaved-guard.js:59-82, :95-97, :133), carried through preload.js:170,175 and public/file-panel.js:158-159.
4	c119179's message described the fix	Closed. That commit is now 977b852, and its body describes only the tests.

New nit: with two upgrades, the first upgrade's report clears the in-flight state while the second upgrade is still waiting (unsaved-guard.js:99-108)

Each upgrade adds a link to joined.asked, and each link clears inflight when it resolves (:106). The close link therefore clears it before the quit link has received its own report. This contradicts viewer-panel.md:167: "Until then the joined question stays the one in flight".

Replay with the test file's setup():

1. reload;
2. a close and a quit join it;
3. answer yes;
4. send unsaved-check-reason-done for the close's token only;
5. call confirmQuit (an install).

Result: main sends a second unsaved-check ([4,"quit"]), and quit() runs while that question is still pending.

The window is short, about one IPC round trip, or the difference between the two 2.5 s bounds on a hung page. Suggested fix: clear inflight only from the last link, for example with a pending-upgrade counter on joined, and add a test where the reports arrive in that order.

Red, then green (local, Node 22, unsaved-guard / dom-file-panel-unsaved-guard / dom-choice-dialog tests)

• At the tests-only commit 1969b7c: 92/95. The 3 failures are on assertions, at dom-file-panel-unsaved-guard.test.js:361 and unsaved-guard.test.js:372 / :390.
• At 24aa642: 95/95. The two older assertions loosened to args.slice(0, 2) follow from the new IPC argument.

Other checks

• CI on 24aa642 is green (run 38218674005).
• On the merged tree, the full suite under Node 22 gives 5397 pass and 1 fail. The failure is the changelog/version check, caused by the CHANGELOG item above.
• eslint on the touched files: 0 errors.
• Comments are one-line pointers only.

Not run: Electron or e2e locally, a real hung renderer.

[abate]

Addressed review of 24aa642. Rebased onto #479 at 80c87e3. Backup of the old head: abate:backup-478-24aa642.

Point	Change
1. CHANGELOG in a released section	The ### Changed (#478) entry is under the new ## Unreleased, above ### Fixed, in every commit that carries it (ba0d87e onward). git diff 80c87e3 22e6996 -- CHANGELOG.md adds only that group; ## v0.0.95 and older are byte-identical to main, the #540 line included.
New nit: first upgrade's report clears inflight while a second upgrade waits	Tests only, dd9c5d9: "a check joined by two upgrades stays in flight until the last upgrade reports" (the review's replay: reload; close and quit join; yes; report for the close's token only; confirmQuit; expects one unsaved-check and no quit until the quit's token reports). Fix, 22e6996: joined.upgrades counts pending upgrade links; each link decrements it and only the last clears inflight (unsaved-guard.js askFull). viewer-panel.md now says the joined question stays in flight until the last upgrade joined to it has reported.

Red, then green (CI on devsuitup):

• dd9c5d9 (tests only): Test run 38256932514 failed on the new assertion (expected: 1, actual: 2, the second unsaved-check), all four test jobs; Build & Release 38256932550 cancelled.
• 22e6996 (fix): Test run 38257806779 green incl. e2e; Build & Release 38257806860 green.

Local, Node 22: full suite 5465 pass, 13 fail (sandbox-only: CLI-state canary and 12 test-pr "isolated" tests); unsaved-guard, dom-file-panel-unsaved-guard, dom-choice-dialog and changelog tests 154/154. eslint on touched files: 0 errors.

Rebased commits: ba0d87e, b456c67, 1efd2f1, dc26fa6, 26eb265, 70937d4, d9e7798 (former 7bfe4d8, 977b852, 00aa634, b6ae309, ce07939, 1969b7c, 24aa642). The rebase conflict in 70937d4 (test/unsaved-guard.test.js, next to #479's new test from 80c87e3) was resolved by keeping both tests unchanged.

[devsuitup]

Re-review at 22e6996 (stacked on #479 at 80c87e3). Approved.

#	Previous finding (24aa642)	Status
1	Blocking: CHANGELOG entries under the released v0.0.95	Closed. The ### Changed (#478) entry and the #479 line both sit under the new ## Unreleased, and nothing under ## v0.0.95 changes. The changelog check is green.
2	Nit: with two upgrades, the first report cleared inflight	Closed. unsaved-guard.js:95 counts upgrades on the joined check, and :107-108 clears inflight only from the last link. viewer-panel.md:167 is updated to match.

Red, then green (local, Node 22)

• At dd9c5d9, unsaved-guard.test.js is 54/55. The new test fails on its assertion at test/unsaved-guard.test.js:420 (2 !== 1: a second question was sent).
• At 22e6996 it is 55/55, and unsaved-guard + dom-file-panel-unsaved-guard + dom-choice-dialog is 97/97.

Other checks

• Range-diff against 24aa642: the old commits differ only in their CHANGELOG hunks, and there are two new commits (test, then fix).
• origin/main (61bee93) is an ancestor of the head, so merging the stack is a fast-forward.
• CI on 22e6996 is green, including every required test job. The new commits add no code comments.

Merge order, for us: our pending #589 (the modal stack and the dialog queue) conflicts with this PR:

• public/choice-dialog.js: replace/closeWith against the new queue;
• public/file-panel.js: showUnsavedEditsDialog's finish;
• the unsaved-guard tests;
• CHANGELOG.md.

#589 will be reworked on top of #479 and #478, not the other way round.

Not run: Electron or e2e locally, a real hung renderer.

[devsuitup]

#479 is merged into main as 45d8037 (squash). This branch still carries #479's original commits, so it now conflicts with main.

Dropping them makes it clean. I checked this locally: rebasing only this PR's own commits onto main applies without conflict, and the resulting tree is byte-identical to 22e6996, the head I approved.

git fetch origin
git rebase --onto origin/main 80c87e3 <your-478-branch>
git push --force-with-lease

(80c87e3 is #479's last head.) Once you push, I will check that the tree still matches 22e6996 and merge after CI.

[abate]

Rebased with git rebase --onto 5a9376a 80c87e3 onto main 5a9376a. New head: b3aaad6 (was 22e6996; backup branch backup-478-22e6996 on the fork).

Two CHANGELOG ## Unreleased conflicts (main's new entries), resolved keeping all lines; released sections are unchanged from main. The #478 ### Changed entry sits under ## Unreleased.

Tree comparison git diff 22e6996 b3aaad6: the PR's own files are identical to 22e6996 except CHANGELOG.md, which now also carries main's entries (#593, #579). Everything else that differs comes from main's own commits (#579, #591, #593, #588 and others).

Tests on the new head: the full suite has 14 failures, all environmental. They are the CLI-state canary and 13 test-pr "isolated" tests. 12 of those are the known sandbox failures. The 13th (4177, new on main with #591) fails with Cannot find module '@playwright/test' because the shared node_modules lacks it. unsaved-guard, dom-file-panel-unsaved-guard, dom-choice-dialog and the changelog tests pass.

CI on b3aaad6: Test run 38371228767 success, Build & Release run 38371229326 success.

[devsuitup]

Rebase verified at b3aaad6: git range-diff 80c87e3..22e6996 origin/main..b3aaad6 shows the nine commits with identical code; the only differences are CHANGELOG/docs context from main's new entries. git diff origin/main b3aaad6 touches the same 13 files as the approved 22e6996 (1141/80 vs 1143/80, the two lines being CHANGELOG). CI green on the head. Approval of 22e6996 carried over.