fix(linux): commit quit teardown at will-quit

[OmerFarukOruc]

Summary

Fixes #939.

On Linux, a quit that starts but does not finish leaves the app permanently wedged: the tray icon is gone (so it cannot be closed) and the single-instance lock is still held (so it cannot be reopened). The process sits idle at ~0% CPU with its entire context already disposed.

The root cause is that before-quit was treated as a point of no return. It is not — it is cancellable and re-entrant. The patch set registered app.on('before-quit', () => codexLinuxDestroyTray()) at module top level, so every quit attempt destroyed the tray, including ones that were later vetoed. It also latched codexLinuxQuitInProgress on with no reset, and ended the will-quit drain in a second, cancellable app.quit() — which re-fires before-quit and can be vetoed again, stalling the quit forever with the tray already gone.

This change makes will-quit the single commit point for irreversible app state. An accepted Linux quit first enters a reversible ATTEMPTING state. markAppQuitting() and quick-chat disposal are deferred until will-quit; a later before-quit veto clears the attempt before either mutation runs. After will-quit, teardown is irreversible and process exit is guaranteed.

• Move tray teardown off before-quit and onto will-quit — including upstream's own before-quit tray-destroy listener, which the previous fix attempt missed. Without that, relocating only our listener would not have fixed the bug.
• Replace the sticky codexLinuxQuitInProgress latch with explicit ATTEMPTING and COMMITTED states plus a one-shot explicit-quit ticket. ATTEMPTING suppresses close-to-tray and other launch actions without mutating upstream quit state; the commit callback performs markAppQuitting() only when will-quit fires.
• Patch the real SQLite backfill progress-window close listener. It prevents the outer close and calls app.quit() recursively; Electron ignores that nested quit while the first quit is in progress, so this was a reachable pre-commit cancellation path. During an accepted Linux quit attempt the progress window now closes normally instead of cancelling and re-entering quit.
• End the Linux drain finalizer in app.exit(0) / process.exit(0) instead of a cancellable app.quit(), with an unconditional drain timeout. Browser::Exit() bypasses both the before-quit and will-quit veto paths, so a committed quit can no longer be stalled by any listener.
• Arm an 8s unref()d hard-exit watchdog at will-quit, in the same commit step that runs the deferred upstream mutation and destroys the tray. It backstops teardown once the audited listener graph reaches its commit point.
• Require a live tray for close-to-tray. canHideLastWindowToTray() reported the tray setting, not tray liveness, so the window could be hidden into a destroyed tray. The gate is now fail-closed on Linux: no live tray means a real close.
• Remove the duplicate quit-state helper copy that was being injected at depth 1 inside the minified startup function, where it shadowed the module-scope copy and was never written — making the launch-action quit guards dead code. The rebuilt bundle now contains exactly one copy.
• Remove the dead appVar/directHandler branch (it always received null) and the legacy second-instance migration path, per the "no legacy patch paths" rule.
• Make the prompt-bypass patch report its own drift. Its warning keyed on a showMessageBoxSync dialog shape that upstream has since localized, so it could never fire. It now recognizes the confirmation site through canQuitWithoutPrompt() or through the showMessageBoxSync and markQuitApproved() pair, so a rename of either anchor surfaces as drift instead of silently recording this required-upstream patch as applied.

Validation

Review-fix validation, run against the amended tree:

Check	Result
node --test scripts/patch-linux-window-ui.test.js	386/386 pass
node --test linux-features/*/test.js	560/560 pass
bash tests/scripts_smoke.sh	All script smoke tests passed
node scripts/ci/validate-patch-report.js codex-app/.codex-linux/patch-report.json	Required upstream profile passed

Real-DMG rebuild. MAX_BUILD_THREADS=0 make build-app-fresh refreshed the upstream DMG and rebuilt end to end against 26.707.72221 (Electron 42.1.0), which is newer than the 26.707.71524 build requested in the review. DMG SHA-256: 40e34814e74e30943c209ebd4da94cd4de3581a52c5bffbe2bcf2e488d6361c6.

• Acceptance verdict accepted_with_warnings, with zero required blockers. The two warnings are unrelated optional drift in automation-update-eager-tool and linux-tooltip-window-controls-collision.
• All six lifecycle descriptors, including the new linux-sqlite-backfill-quit-close, report applied with required-upstream policy.
• The emitted app.asar contains the reversible attempt state, deferred markAppQuitting() callbacks, the will-quit commit callback, and the SQLite close guard. It contains no renderer will-prevent-unload or onbeforeunload handler; the sole beforeunload string is a Computer Use dialog enum.

Runtime reachability proof. A clean Electron 42.1.0 reproduction of the audited SQLite listener graph recorded before-quit -> close-outer -> timeout, with no will-quit. The nested app.quit() did not restart the lifecycle and the outer close remained prevented. This confirms that cancellation was reachable and that merely narrowing the old test would not fix the bug.

Tests added. The synthetic “fully recoverable” test called out in review is gone. Its replacement carries one mutable upstream state object through a later before-quit veto and retry, proving the veto leaves isAppQuitting false, quick-chat undisposed, the tray alive, and close-to-tray usable. A separate behavioral test matches the actual SQLite close listener and proves it no longer prevents an accepted Linux quit. Existing tests still cover the committed watchdog/force-exit path, ticket expiry, drift reporting, and byte-identical reapplication.

Known risk / scope. The proof and patch target the complete cancellation graph present in 26.707.72221: the close-to-tray listener, the SQLite backfill progress listener, the upstream quit confirmation, and the upstream drain listener. A future upstream window or renderer veto is drift and requires a fresh audit; this PR does not claim recoverability for listener shapes absent from the validated bundle.

Checklist

• This pull request is ready for review and is no longer a draft.
• I followed CONTRIBUTING.md, kept the change focused, edited source files rather than generated output, and removed unrelated changes.
• If this fixes upstream drift, it targets only the latest CODEX.DMG and removes obsolete fallback code and tests from the affected area.
• I added or updated the relevant tests, ran the validation listed above, and confirmed the required CI checks pass.
• I reviewed the final diff with a coding agent at maximum reasoning effort, addressed all findings, and reran the relevant tests.

[Yo-DDV]

Hi @ilysenko, I found one blocking lifecycle regression in the final diff.

In scripts/patches/impl/main-process/quit-lifecycle.js (the generated branches at lines 69, 82, and 88), the hard-exit watchdog is armed before Electron has finished dispatching the remaining before-quit listeners and before window beforeunload handlers have had a chance to cancel. I reproduced that ordering against the generated helper: a later listener called preventDefault(), codexLinuxQuitCommitted remained false and the tray stayed alive initially, but the watchdog remained armed. Firing it then destroyed the tray and called app.exit(0) / process.exit(0).

Electron v42.1.0 explicitly allows before-quit and window beforeunload to cancel app.quit(), while app.exit() bypasses those hooks. This turns a canceled pre-commit quit into forced termination eight seconds later. It contradicts the PR's recoverability invariant and can discard user state.

Please keep the watchdog unarmed until the chosen commit point, or add a proven reset/disarm path for pre-commit vetoes, and cover a later-listener or window cancellation after the patched listener has run. I also tested the PR merged onto the current main: the 383 patch tests, 565 feature tests, and script smoke suite all pass. I did not find another blocker in the tray, launch-action, patch-drift, cleanup, or current-main integration paths.

[OmerFarukOruc]

@Yo-DDV You are right. The pre-commit arms are gone.

No disarm path existed. codexLinuxQuitWatchdogTimer only ever got assigned, never cleared. The one clearTimeout under scripts/patches/impl/ sits in browser.js:469 and belongs to xdg-open. Once any of those three branches armed the timer, it fired.

Why they were there. An earlier max-reasoning review raised the inverse hole: accept the quit at before-quit, let a window veto its own close, will-quit never fires, and nothing backstops an app that already marked itself quitting. I hedged by arming at the accepted branch. That same reviewer later walked the rebuilt ASAR and cleared the scenario as unreachable in the current bundle. markAppQuitting() disables the close-to-tray veto before any window closes. The SQLite-backfill window reissues app.quit() itself. The bootstrap-installer veto is macOS-only. The bundle carries no renderer beforeunload cancellation and no will-prevent-unload handler. So the arms guarded nothing reachable, and they opened the hazard you reproduced.

What changed. I dropped the arm call from the literal beforeQuitPatch branch and from the semantic beforeQuitRegex replacement. Both sites still inject the bypass expression, so the prompt bypass survives untouched. I deleted acceptedPromptRegex and its .replace() block: with the arm gone it emitted text byte-identical to its match while still marking a required-upstream patch as applied. I deleted the second drift warning too, since it keyed on markQuitApproved(),typeof codexLinuxArmQuitWatchdog, a string the patch no longer emits.

Grep of the rebuilt app.asar. codexLinuxArmQuitWatchdog = 2 (the definition, plus the single call inside codexLinuxCommitQuit, which runs on will-quit). markQuitApproved(),typeof codexLinuxArmQuitWatchdog = 0. codexLinuxQuitCommitted=!1 = 1. The accepted before-quit branch is back to a bare commit with the bypass prefix intact:

codexLinuxShouldBypassQuitPrompt())||e||r.canQuitWithoutPrompt()||t||!s&&!l){_=!0,i.markAppQuitting();return}

Why #939 stays closed without a pre-commit watchdog. Two independent things hold it. The Linux drain finalizer ends in app.exit(0) then process.exit(0), and Browser::Exit() skips both HandleBeforeQuit() and NotifyAndShutdown() (browser.cc#L125-L148), so no listener gets a second chance to stall a committed quit. And codexLinuxCommitQuit still arms the watchdog at will-quit, which covers anything that hangs during teardown.

The one cost. A quit accepted at before-quit that then stalls before will-quit fires now has no backstop. That stall has one reachable shape, and it is an Electron runtime fault: Electron closes the BrowserWindows, a renderer never finishes its beforeunload/unload close handshake, and will-quit never fires. No application code in the rebuilt bundle holds a quit open at that point. An unresponsive renderer still can. I took that trade over forcing termination on a quit the user cancelled.

Tests. scripts/patch-linux-window-ui.test.js gains "a later before-quit veto keeps an accepted explicit quit fully recoverable", which drives your exact sequence: accept the quit, let a later before-quit listener call preventDefault(), advance timers past the 8s window, then assert the tray survives and app.exit never runs. Restore a pre-commit arm and it fails. Remove it and it passes.

One more thing from the same pass. The prompt-bypass drift warning could never fire. It keyed on the showMessageBoxSync({type:`warning`,buttons:[`Quit`,`Cancel`] literal, and upstream has since localized those buttons, so the AND was always false on a required-upstream patch. It now recognizes the confirmation site through canQuitWithoutPrompt() or through the showMessageBoxSync and markQuitApproved() pair, so a rename of either anchor surfaces as drift instead of a silent already-applied.

Validation.

check	result
node --test scripts/patch-linux-window-ui.test.js	385 pass
node --test linux-features/*/test.js	560 pass
bash tests/scripts_smoke.sh	pass
bash -n install.sh, launcher template, scripts/lib/*.sh, 4 build scripts	pass
cargo check -p codex-update-manager	clean
cargo test -p codex-update-manager	242 pass

./install.sh ./Codex.dmg exits 0 with verdict accepted_with_warnings and "blockers": []. The only warning is the pre-existing linux-tooltip-window-controls-collision, which drifted before this change and stays untouched by it. No patch in this change reports drift.

Thank you for running it merged onto current main. Your 565 feature tests against my 560 tells me main gained tests after I branched.

[ilysenko]

Thanks for fixing the pre-commit watchdog issue. One lifecycle blocker remains.

The new test claims that a later before-quit veto leaves the app fully recoverable, but the upstream handler has already called markAppQuitting(). In the current 26.707.71524 bundle, that sets WindowManager.isAppQuitting = true and disposes the quick-chat controller. The test then checks close-to-tray using a separate mock with isAppQuitting: false, masking the real post-veto state.

Please either avoid/recover that irreversible upstream state before the commit point, or prove that this veto is unreachable in the current bundle and narrow the recoverability claim and test accordingly. The regression coverage should carry the actual post-markAppQuitting() state into the post-veto behavior check.

[joshyorko]

@OmerFarukOruc Just reviewed , came to same conculsion as Gary @ilysenko . Lets GTM. YOLO

[OmerFarukOruc]

@ilysenko Addressed in 6c666b4. Your review was correct: the old test recorded markAppQuitting() on one mock, then asserted recoverability through a separate isAppQuitting: false mock.

I refreshed the upstream DMG and rebuilt against 26.707.72221 (Electron 42.1.0), newer than the requested 26.707.71524. The real bundle audit found a reachable cancellation path: the SQLite backfill progress window prevents its outer close and calls app.quit() recursively. A clean Electron 42.1.0 reproduction recorded before-quit -> close-outer -> timeout, with no will-quit; the nested quit is ignored while the first quit is in progress.

The patch now uses reversible ATTEMPTING state before commit. Both accepted confirmation branches defer markAppQuitting() and quick-chat disposal in a commit callback. A later before-quit veto clears the attempt on the event microtask. will-quit changes the state to COMMITTED, runs the callback once, destroys the tray, and arms the watchdog. The SQLite backfill close listener now allows its window to close during an accepted Linux attempt instead of cancelling and re-entering quit.

I removed the synthetic “fully recoverable” test. Its replacement carries one mutable upstream state object through veto and retry, proving the veto leaves isAppQuitting false, quick-chat undisposed, the tray alive, and close-to-tray usable. A separate behavioral test matches the actual SQLite close listener.

Validation:

• Fresh DMG acceptance: accepted_with_warnings, zero required blockers; all six lifecycle descriptors are applied under required-upstream.
• node --test scripts/patch-linux-window-ui.test.js: 386/386 pass.
• node --test linux-features/*/test.js: 560/560 pass.
• bash tests/scripts_smoke.sh: pass.
• Required patch-report validation: pass.

I also narrowed the PR description to the listener graph audited in 26.707.72221 and removed the old blanket recoverability claim.