Repository navigation
fix(quit): Cmd-Q with local terminals asks how to quit (Keep Sessions default, End Sessions Keep Layout, End Everything); Don't ask again writes app.quitBehavior (QUIT P0) - #29984
Merged
Conversation
…eep Sessions the default (QUIT P0, red)
… (default), End Sessions Keep Layout, End Everything, Cancel; Don't ask again writes app.quitBehavior (QUIT P0) Lawrence 2026-11-01: "cmd q needs to prompt for what type of quit". Takes back aba72a3 (cx-7vu, 2026-10-11, Lawrence 2026-10-07 "no confirm dialogs"), which made QuitPolicy.decide quit with Keep and no question unless incognito programs ran, so app.quitBehavior "ask" (the default) never asked.
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 02:04 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on 654a733 to compare; do not merge until green; see lane-rules Default changesThis pull request changes 1 default at
Signed off on this commit by Merge receipt: merged as Regenerate the snapshot with Shipped in:
|
Contributor
Author
|
Review (claude-review-subagent-29984): FIX at 6bfec69 Scope: BlockingB1. The four buttons do not fit in the dialog's fixed 356 pt button row
Approximate widths at 12 pt:
With 3 x 4 pt spacing, the total is about 500 pt in a 356 pt row. The old 3-button row was about 340 pt and only just fit. German ("Sitzungen beenden, Layout behalten", "Alles beenden", ...) is wider still. Nothing in Fix options, choose one, then attach a real capture (
B2. The repo's own quit acceptance scripts still assert "Cmd-Q never asks" (cx-7vu), and this PR does not update them
These are not CI jobs, but they are the checks that prove this area. Update them in this PR to expect the question and press Checked and correct
Non-blocking
|
…stack inside it, the default on top (QUIT P0 review, red)
…n top, each the card's width (the quit choice's four buttons); the quit scripts answer the quit dialog's Keep (QUIT P0 review)
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 02:49 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 02:49 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="plans/cmux-next/quit-persistence.md">
<violation number="1" location="plans/cmux-next/quit-persistence.md:21">
P2: This update lists "End Everything" as a direct dialog choice with no confirmation step. §4.1 and decision D1 in this same file put it behind a confirmation. Either state the confirmation here or correct the design section to match the shipped behavior.</violation>
<violation number="2" location="plans/cmux-next/quit-persistence.md:21">
P2: This update says Cmd-Q uses one cmux dialog, but §1.1 still describes a system `NSAlert` followed by a second alert. Update §1.1 to match the current flow so the plan does not document contradictory behavior.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Tests/CmuxNextDesignTests/CmuxDialogButtonFitTests.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Tests/CmuxNextDesignTests/CmuxDialogButtonFitTests.swift:19">
P2: This test lays out the dialog without a superview, so `view.bounds` probably stays zero and the card-containment assertions cannot pass. Attach the view to a sized host and pin it before calling `layoutSubtreeIfNeeded()`, as CmuxDialogLightContrastTests does.</violation>
</file>
<file name="scripts/cmux-next/quit-launch-bound.py">
<violation number="1" location="scripts/cmux-next/quit-launch-bound.py:368">
P2: The probe loop does not handle the app exiting without a prompt. `self.call` goes through `bench.app_client`, which retries for 30 s and then raises `SystemExit`, and that is not caught by `except (OSError, ValueError)`. The run then aborts with a misleading 'control socket did not answer' error instead of recording the quit. Open one client before the loop and close it in `finally`, so a closed connection raises a catchable `OSError`.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Sources/CmuxNextDesign/Dialog/CmuxDialogView.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextDesign/Dialog/CmuxDialogView.swift:234">
P2: The horizontal row includes a leading spacer, so it needs `buttons.count` spacing gaps, not `buttons.count - 1`; near the cutoff, buttons that do not fit stay on one line and may be compressed or overflow. Include the spacer's gap in the fit calculation.</violation>
</file>
<file name="scripts/cmux-next/dialogs-undo-live.py">
<violation number="1" location="scripts/cmux-next/dialogs-undo-live.py:206">
P3: The module docstring still says this case has “no dialog” and “quits at once,” but this test now waits for and accepts the Keep Sessions prompt. Update the quit description so the script’s help text matches its behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| Update 2026-11-01 (QUIT P0, #29984): from 2026-10-11 (cx-7vu, aba72a3fbae, Lawrence 2026-10-07 "no confirm dialogs") | ||
| until #29984, an interactive quit asked nothing and kept the sessions. Lawrence 2026-11-01 asked for the prompt again | ||
| ("cmd q needs to prompt for what type of quit"): with local terminals and `app.quitBehavior` "ask", Cmd-Q asks Keep | ||
| Sessions Running (default) / End Sessions, Keep Layout / End Everything / Cancel in one cmux dialog (the four buttons |
There was a problem hiding this comment.
P2: This update lists "End Everything" as a direct dialog choice with no confirmation step. §4.1 and decision D1 in this same file put it behind a confirmation. Either state the confirmation here or correct the design section to match the shipped behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At plans/cmux-next/quit-persistence.md, line 21:
<comment>This update lists "End Everything" as a direct dialog choice with no confirmation step. §4.1 and decision D1 in this same file put it behind a confirmation. Either state the confirmation here or correct the design section to match the shipped behavior.</comment>
<file context>
@@ -15,6 +15,12 @@ the "quit = plain NSAlert" decision; the default choice is Keep sessions running
+Update 2026-11-01 (QUIT P0, #29984): from 2026-10-11 (cx-7vu, aba72a3fbae, Lawrence 2026-10-07 "no confirm dialogs")
+until #29984, an interactive quit asked nothing and kept the sessions. Lawrence 2026-11-01 asked for the prompt again
+("cmd q needs to prompt for what type of quit"): with local terminals and `app.quitBehavior` "ask", Cmd-Q asks Keep
+Sessions Running (default) / End Sessions, Keep Layout / End Everything / Cancel in one cmux dialog (the four buttons
+stack when they do not fit on one line); "Don't ask again" writes the choice. Scripts, signals and power off never ask.
+
</file context>
Suggested change
| Sessions Running (default) / End Sessions, Keep Layout / End Everything / Cancel in one cmux dialog (the four buttons | |
| Sessions Running (default) / End Sessions, Keep Layout / End Everything (after a confirmation) / Cancel in one cmux dialog (the four buttons |
|
|
||
| @Test func buttonsThatDoNotFitStackInsideTheCardWithTheDefaultOnTop() { | ||
| let view = CmuxDialogView(spec: Self.quitSpec) | ||
| view.layoutSubtreeIfNeeded() |
There was a problem hiding this comment.
P2: This test lays out the dialog without a superview, so view.bounds probably stays zero and the card-containment assertions cannot pass. Attach the view to a sized host and pin it before calling layoutSubtreeIfNeeded(), as CmuxDialogLightContrastTests does.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Tests/CmuxNextDesignTests/CmuxDialogButtonFitTests.swift, line 19:
<comment>This test lays out the dialog without a superview, so `view.bounds` probably stays zero and the card-containment assertions cannot pass. Attach the view to a sized host and pin it before calling `layoutSubtreeIfNeeded()`, as CmuxDialogLightContrastTests does.</comment>
<file context>
@@ -0,0 +1,44 @@
+
+ @Test func buttonsThatDoNotFitStackInsideTheCardWithTheDefaultOnTop() {
+ let view = CmuxDialogView(spec: Self.quitSpec)
+ view.layoutSubtreeIfNeeded()
+ let card = view.bounds
+ #expect(card.width > 0 && card.height > 0)
</file context>
| ask_end = time.time() + 10 | ||
| while time.time() < ask_end: | ||
| try: | ||
| asking = self.call("debug.quit", {}).get("result") or {} |
There was a problem hiding this comment.
P2: The probe loop does not handle the app exiting without a prompt. self.call goes through bench.app_client, which retries for 30 s and then raises SystemExit, and that is not caught by except (OSError, ValueError). The run then aborts with a misleading 'control socket did not answer' error instead of recording the quit. Open one client before the loop and close it in finally, so a closed connection raises a catchable OSError.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/quit-launch-bound.py, line 368:
<comment>The probe loop does not handle the app exiting without a prompt. `self.call` goes through `bench.app_client`, which retries for 30 s and then raises `SystemExit`, and that is not caught by `except (OSError, ValueError)`. The run then aborts with a misleading 'control socket did not answer' error instead of recording the quit. Open one client before the loop and close it in `finally`, so a closed connection raises a catchable `OSError`.</comment>
<file context>
@@ -361,6 +361,17 @@ def quit(self, mode, pages, out, label):
+ ask_end = time.time() + 10
+ while time.time() < ask_end:
+ try:
+ asking = self.call("debug.quit", {}).get("result") or {}
+ except (OSError, ValueError):
+ break # the app is gone or going: nothing asks
</file context>
| row.translatesAutoresizingMaskIntoConstraints = false | ||
| row.spacing = Metrics.space2 | ||
| let needed = buttons.reduce(0) { $0 + $1.intrinsicContentSize.width } | ||
| + row.spacing * CGFloat(max(buttons.count - 1, 0)) |
There was a problem hiding this comment.
P2: The horizontal row includes a leading spacer, so it needs buttons.count spacing gaps, not buttons.count - 1; near the cutoff, buttons that do not fit stay on one line and may be compressed or overflow. Include the spacer's gap in the fit calculation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Sources/CmuxNextDesign/Dialog/CmuxDialogView.swift, line 234:
<comment>The horizontal row includes a leading spacer, so it needs `buttons.count` spacing gaps, not `buttons.count - 1`; near the cutoff, buttons that do not fit stay on one line and may be compressed or overflow. Include the spacer's gap in the fit calculation.</comment>
<file context>
@@ -228,6 +223,31 @@ public final class CmuxDialogView: NSView {
+ row.translatesAutoresizingMaskIntoConstraints = false
+ row.spacing = Metrics.space2
+ let needed = buttons.reduce(0) { $0 + $1.intrinsicContentSize.width }
+ + row.spacing * CGFloat(max(buttons.count - 1, 0))
+ guard needed > width else {
+ row.orientation = .horizontal
</file context>
Suggested change
| + row.spacing * CGFloat(max(buttons.count - 1, 0)) | |
| + + row.spacing * CGFloat(buttons.count) |
| Update 2026-11-01 (QUIT P0, #29984): from 2026-10-11 (cx-7vu, aba72a3fbae, Lawrence 2026-10-07 "no confirm dialogs") | ||
| until #29984, an interactive quit asked nothing and kept the sessions. Lawrence 2026-11-01 asked for the prompt again | ||
| ("cmd q needs to prompt for what type of quit"): with local terminals and `app.quitBehavior` "ask", Cmd-Q asks Keep | ||
| Sessions Running (default) / End Sessions, Keep Layout / End Everything / Cancel in one cmux dialog (the four buttons |
There was a problem hiding this comment.
P2: This update says Cmd-Q uses one cmux dialog, but §1.1 still describes a system NSAlert followed by a second alert. Update §1.1 to match the current flow so the plan does not document contradictory behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At plans/cmux-next/quit-persistence.md, line 21:
<comment>This update says Cmd-Q uses one cmux dialog, but §1.1 still describes a system `NSAlert` followed by a second alert. Update §1.1 to match the current flow so the plan does not document contradictory behavior.</comment>
<file context>
@@ -15,6 +15,12 @@ the "quit = plain NSAlert" decision; the default choice is Keep sessions running
+Update 2026-11-01 (QUIT P0, #29984): from 2026-10-11 (cx-7vu, aba72a3fbae, Lawrence 2026-10-07 "no confirm dialogs")
+until #29984, an interactive quit asked nothing and kept the sessions. Lawrence 2026-11-01 asked for the prompt again
+("cmd q needs to prompt for what type of quit"): with local terminals and `app.quitBehavior` "ask", Cmd-Q asks Keep
+Sessions Running (default) / End Sessions, Keep Layout / End Everything / Cancel in one cmux dialog (the four buttons
+stack when they do not fit on one line); "Don't ask again" writes the choice. Scripts, signals and power off never ask.
+
</file context>
| check("quit-setup-program", loop_pid, loop_pid) | ||
| reply = rpc("debug.quit", {"open": True}) | ||
| print("quit open:", json.dumps(reply)[:200]) | ||
| # QUIT P0: with a local terminal, Cmd-Q asks how to quit; its default (Keep Sessions) keeps the program running. |
There was a problem hiding this comment.
P3: The module docstring still says this case has “no dialog” and “quits at once,” but this test now waits for and accepts the Keep Sessions prompt. Update the quit description so the script’s help text matches its behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/dialogs-undo-live.py, line 206:
<comment>The module docstring still says this case has “no dialog” and “quits at once,” but this test now waits for and accepts the Keep Sessions prompt. Update the quit description so the script’s help text matches its behavior.</comment>
<file context>
@@ -203,13 +203,19 @@ def alive(pid):
check("quit-setup-program", loop_pid, loop_pid)
reply = rpc("debug.quit", {"open": True})
print("quit open:", json.dumps(reply)[:200])
+ # QUIT P0: with a local terminal, Cmd-Q asks how to quit; its default (Keep Sessions) keeps the program running.
asked = False
deadline = time.time() + 20
</file context>
lawrencecchen
added a commit
that referenced
this pull request
Nov 1, 2026
…pp quits P0 journey quit-leaves-no-helpers (perf v2.1, job 2769005f506cef2d9a476d02, cmux24): 5 s after a normal quit the app's acpmux daemon and the cmux-tui daemon's cmux-browser-host still ran. Port of cp/lag-quit-helpers (267b25d) and the stop rule of cp/redesign-quit-helpers to feat, written for feat's acpmux: perf's daemon_lifecycle.rs and cmux-owner-liveness crate do not exist here; feat's own stop is daemon/unused_exit.rs. Root cause. The app starts acpmux detached (setsid, adopted by launchd) so durable agents outlive a quit, and nothing ties it to the app's exit: on a home of its own it stops only after 60 min unused, on the shared ~/.acpmux never. The browser host waits its 5 min idle delay after the app's provider left. Fix, the way a Chromium child ends when the browser process goes: - The launcher names the app (ACPMUX_OWNER_APP_PID). acpmux follows it with one thread blocked in the kernel (kqueue EVFILT_PROC NOTE_EXIT on macOS, a pidfd on Linux; daemon/owner_exit.rs, no polling). - After the app exited, unused_exit waits OWNER_EXIT_GRACE (2 s) with no client instead of its grace, on any home, and stops only when Hub::in_use finds nothing: a running agent or agent host, a turn, a queued prompt, a pending permission, a background task, a peer link keep it, and it asks again every 60 s. So Keep Sessions Running (#29984) keeps working agents. While the app runs nothing changes; a daemon with no owner named (the CLI's, a service unit's, a Cloud machine's) is unchanged. Agents do not inherit the variable. - cmux-browser-host: the provider leaving (an edge: connected, then not) with no session and no agent connection exits the host at once; a session keeps it, with the whole delay after it. The daemon starts it again on the next agent connect or provider request. The app does nothing new at quit and nothing on its main thread: the helpers see its exit. Tests: acpmux tests/daemon_lifecycle.rs quit_helpers_* (real binary, fake owner app), unused_exit quit_helpers_* (paused clock), owner_exit unit tests; cmux-browser-host idle_exit quit_helpers_*; AcpmuxOwnerAppPidTests.
Contributor
Author
|
Review (claude-review-subagent-29984): FIX at 1ac555c Scope: the full PR over merge base 8922fc2 (4 commits). This round I looked closely at 325b2a3 (red) and 1ac555c (fix). I read the code only. I did not build or run tests. BlockingB1. Two more quit scripts are wrong after this change (same class as the previous B2; I missed the first one in round 1)
Checked and correct (this round)
Non-blocking
|
… the second-quit check waits for the dialog; stacked dialog buttons tab top to bottom (review FIX on #29984)
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 03:25 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 03:25 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
added a commit
that referenced
this pull request
Nov 1, 2026
… the second-quit check waits for the dialog; stacked dialog buttons tab top to bottom (review FIX on #29984)
There was a problem hiding this comment.
1 existing issue remains and 2 new issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/fleet-quit-persistence.sh">
<violation number="1" location="scripts/fleet-quit-persistence.sh:219">
P3: The second-Cmd-Q variant never checks that the quit dialog appeared. If the 15-second poll times out, the script still sends the second quit and only `second-quit-keeps` reports a failure. Track an `asked` flag like step 5 and add a `check second-quit-dialog-shown` so a missing prompt is reported as the cause.</violation>
</file>
<file name="scripts/cmux-next/quit-hidden-chromium-live.py">
<violation number="1" location="scripts/cmux-next/quit-hidden-chromium-live.py:1116">
P3: The default id is taken from `prompt.default` without checking it against the dialog's buttons. In the incognito-only sheet, `prompt.default` can name a button the dialog does not show (for example "keep" with buttons [quit, cancel]), so the press is a no-op and the run waits until SIGKILL. Choose the first candidate that is present in `buttons`, falling back to buttons[0].</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
View guided diff | Turn on auto-fix | Re-trigger cubic
| # A second Cmd-Q while the quit dialog shows answers its default (Keep). It is | ||
| # sent once the dialog shows: before that the quit is still reading its facts. | ||
| rpc_ok debug.quit '{"open":true}' >/dev/null | ||
| for i in $(seq 1 15); do |
There was a problem hiding this comment.
P3: The second-Cmd-Q variant never checks that the quit dialog appeared. If the 15-second poll times out, the script still sends the second quit and only second-quit-keeps reports a failure. Track an asked flag like step 5 and add a check second-quit-dialog-shown so a missing prompt is reported as the cause.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/fleet-quit-persistence.sh, line 219:
<comment>The second-Cmd-Q variant never checks that the quit dialog appeared. If the 15-second poll times out, the script still sends the second quit and only `second-quit-keeps` reports a failure. Track an `asked` flag like step 5 and add a `check second-quit-dialog-shown` so a missing prompt is reported as the cause.</comment>
<file context>
@@ -213,8 +213,14 @@ rpc_ok action.run '{"id":"selectWorkspaceByNumber","args":{"index":1}}' >/dev/nu
+# A second Cmd-Q while the quit dialog shows answers its default (Keep). It is
+# sent once the dialog shows: before that the quit is still reading its facts.
rpc_ok debug.quit '{"open":true}' >/dev/null
+for i in $(seq 1 15); do
+ rpc debug.quit '{}' 2 2>/dev/null | grep -q '"asking": *true' && break
+ alive "$APP_PID" || break
</file context>
| buttons = [b.get("id") for b in sheet.get("buttons") or []] | ||
| choice = next((b for b in buttons if "keep" in str(b)), buttons[0] if buttons else None) | ||
| # The default (Keep Sessions Running, id "keep"), never another id that merely contains "keep". | ||
| choice = (sheet.get("prompt") or {}).get("default") or ("keep" if "keep" in buttons else (buttons[0] if buttons else None)) |
There was a problem hiding this comment.
P3: The default id is taken from prompt.default without checking it against the dialog's buttons. In the incognito-only sheet, prompt.default can name a button the dialog does not show (for example "keep" with buttons [quit, cancel]), so the press is a no-op and the run waits until SIGKILL. Choose the first candidate that is present in buttons, falling back to buttons[0].
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/cmux-next/quit-hidden-chromium-live.py, line 1116:
<comment>The default id is taken from `prompt.default` without checking it against the dialog's buttons. In the incognito-only sheet, `prompt.default` can name a button the dialog does not show (for example "keep" with buttons [quit, cancel]), so the press is a no-op and the run waits until SIGKILL. Choose the first candidate that is present in `buttons`, falling back to buttons[0].</comment>
<file context>
@@ -1112,7 +1112,8 @@ def one_run(index):
buttons = [b.get("id") for b in sheet.get("buttons") or []]
- choice = next((b for b in buttons if "keep" in str(b)), buttons[0] if buttons else None)
+ # The default (Keep Sessions Running, id "keep"), never another id that merely contains "keep".
+ choice = (sheet.get("prompt") or {}).get("default") or ("keep" if "keep" in buttons else (buttons[0] if buttons else None))
run.note(f"quit sheet asked ({buttons}); pressing {choice}")
rpc("debug.quit", {"press": choice}, timeout=5)
</file context>
Suggested change
| choice = (sheet.get("prompt") or {}).get("default") or ("keep" if "keep" in buttons else (buttons[0] if buttons else None)) | |
| choice = next((c for c in [(sheet.get("prompt") or {}).get("default"), "keep"] if c in buttons), buttons[0] if buttons else None) |
|
Found 9 test failures on Blacksmith runners: Failures
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Contributor
Author
|
Review (claude-review-subagent-29984): SHIP at 132fa6e Scope: the full PR over merge base 8922fc2 (5 commits). This round I looked closely at 132fa6e. I read the code only. I did not build or run tests. BlockingNone. Round-2 findings: all fixed
Evidence (verified)
Carried forward from rounds 1-2 (unchanged)
Non-blocking
|
…or and the End Everything guard, b52ff17): the session choice moves into QuitRules.decide; a remembered End Everything still asks with its own dialog
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 03:58 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 03:58 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 04:06 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
added a commit
that referenced
this pull request
Nov 1, 2026
…pp quits (#30090) * test(quit): red: the app's idle acpmux and its browser host outlive the app's quit P0 journey quit-leaves-no-helpers (perf v2.1, job 2769005f506cef2d9a476d02, cmux24): 5 s after a normal quit the app's acpmux daemon and the cmux-tui daemon's cmux-browser-host still ran. Port of cp/lag-quit-helpers 3ce76b2 to feat, where the acpmux half is new (perf's daemon_lifecycle.rs does not exist here). On feat the app starts its acpmux detached (AcpmuxDaemonLauncher, setsid) and nothing ties it to the app's exit: a daemon on a home of its own stops only after 60 min unused (daemon/unused_exit.rs), the shared ~/.acpmux never. The feat linger probe (bench-startup.py watch_linger) could not see this: it lists only /Contents/MacOS/ processes, and acpmux runs from Contents/Resources/bin. - acpmux tests/daemon_lifecycle.rs owner_exit_a_daemon_stops_soon_after_its_app_is_gone_and_its_last_client_left: the real binary with ACPMUX_OWNER_APP_PID = a fake app (a sleep child) still runs 15 s after the app is gone and its last client left. owner_exit_a_running_turn_keeps_the_daemon_after_its_app_is_gone: a guard (Keep Sessions Running, #29984): a turn in progress keeps it. - cmux-browser-host idle_exit a_host_whose_app_provider_left_with_nothing_else_exits_at_once: the host waits its whole 5 min delay after the app's provider left. provider_left is a stub here (the old behavior) so the test compiles. * test(quit): one name prefix (quit_helpers_) for the quit helper tests, so one fleet filter runs them all * test(quit): red: the app's acpmux spawn environment names no owner app The daemon can follow its app only if the app names itself: on feat AcpmuxDaemonLauncher.spawnEnvironment sets no ACPMUX_OWNER_APP_PID (git grep: no hit in Packages/). AcpmuxOwnerAppPidTests: the spawn environment names this app's pid over an inherited value, signed spawn too. * fix(quit): the app's idle acpmux and its browser host stop when the app quits P0 journey quit-leaves-no-helpers (perf v2.1, job 2769005f506cef2d9a476d02, cmux24): 5 s after a normal quit the app's acpmux daemon and the cmux-tui daemon's cmux-browser-host still ran. Port of cp/lag-quit-helpers (267b25d) and the stop rule of cp/redesign-quit-helpers to feat, written for feat's acpmux: perf's daemon_lifecycle.rs and cmux-owner-liveness crate do not exist here; feat's own stop is daemon/unused_exit.rs. Root cause. The app starts acpmux detached (setsid, adopted by launchd) so durable agents outlive a quit, and nothing ties it to the app's exit: on a home of its own it stops only after 60 min unused, on the shared ~/.acpmux never. The browser host waits its 5 min idle delay after the app's provider left. Fix, the way a Chromium child ends when the browser process goes: - The launcher names the app (ACPMUX_OWNER_APP_PID). acpmux follows it with one thread blocked in the kernel (kqueue EVFILT_PROC NOTE_EXIT on macOS, a pidfd on Linux; daemon/owner_exit.rs, no polling). - After the app exited, unused_exit waits OWNER_EXIT_GRACE (2 s) with no client instead of its grace, on any home, and stops only when Hub::in_use finds nothing: a running agent or agent host, a turn, a queued prompt, a pending permission, a background task, a peer link keep it, and it asks again every 60 s. So Keep Sessions Running (#29984) keeps working agents. While the app runs nothing changes; a daemon with no owner named (the CLI's, a service unit's, a Cloud machine's) is unchanged. Agents do not inherit the variable. - cmux-browser-host: the provider leaving (an edge: connected, then not) with no session and no agent connection exits the host at once; a session keeps it, with the whole delay after it. The daemon starts it again on the next agent connect or provider request. The app does nothing new at quit and nothing on its main thread: the helpers see its exit. Tests: acpmux tests/daemon_lifecycle.rs quit_helpers_* (real binary, fake owner app), unused_exit quit_helpers_* (paused clock), owner_exit unit tests; cmux-browser-host idle_exit quit_helpers_*; AcpmuxOwnerAppPidTests. * fix(quit): review: no daemon a command starts follows the app; kqueue waits without re-adding; docs Review of 6418b9e (SHIP, findings 1, 3, 4, 5): - spawn_detached (CLI auto-start) and chat_fork's harness drop ACPMUX_OWNER_APP_PID: only the app's launcher names an owner, so a daemon a descendant starts never follows the app that started an older one. - owner_exit (macOS): after EINTR the kevent call passes no change, not the same change with flags 0. - unused_exit docs: ACPMUX_UNUSED_EXIT_SECS=0 means no grace; the owner rule can still stop the daemon. - owner_exit unit tests take the quit_helpers_ prefix, so the fleet filter runs them.
…sion question (Ask is not a remembered choice; QUIT P0), the remembered-choice cases unchanged
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 04:28 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 04:28 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Author
|
Review (claude-review-subagent-29984): SHIP at b3c6d56 Scope: the full PR over the new merge base 654a733. This round I looked closely at merge cbde57d (origin/feat-cmux-next, which brought b52ff17: the QuitRules refactor and the remembered End Everything guard) and at b3c6d56. I read the code only. I did not build or run tests. BlockingNone. The merge kept feat's End Everything guard
The test change is honest
Catalog, scripts and evidence
Non-blocking (carried forward)
|
Contributor
Author
|
Check classification at b3c6d56 (author):
|
lawrencecchen
added
the
ready-to-land
lawrencecchen
added a commit
that referenced
this pull request
Nov 1, 2026
…y workspace (port of feat b52ff17 to v2.2.12-opt) On 2026-10-31 18:46 a Cmd-Q typed mid-sentence, with app.quitBehavior end-everything set, quit with no dialog and closed all 900 local workspaces. With the quit question of #29984 a person can store End Everything from the dialog, so this line needs the guard too (review FIX on #30182). - An interactive quit with End Everything remembered asks "Quit and close every workspace?" (Return keeps sessions running, Escape cancels, End Everything has no key). After the unsaved-changes question it ends the sessions but keeps the workspaces. - A scripted quit with no flag maps End Everything to End Sessions, Keep Layout. --end-everything and the File menu item run as before. - The files are feat's at 88dcb89 (b52ff17 + #29984): QuitPolicy, QuitAlert, QuitAlertContent, QuitStrings, Quit.xcstrings, QuitChoice, readiness-soak.py, update-e2e.py, QuitChoiceTests, QuitEndEverythingGuardTests. - Port difference: feat's CmuxNextDaemonService/QuitRules.swift holds six types; here they are one file per type (QuitProgram, QuitAgentFacts, QuitFacts, QuitPrompt, QuitDecision, QuitRules), byte-identical code, so no new multi-type file. (cherry picked from commit b52ff17, files split)
This branch was previously deployed
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
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.
What
QUIT P0 (Lawrence 2026-11-01 01:05Z: "after quitting and reopening, i lose all the workspaces and terminals ... cmd q needs to prompt for what type of quit"). A person's Cmd-Q (menu, Dock) with local terminals now asks how to quit, in one standard cmux dialog (
CmuxDialogSpec, key hints):app.quitBehavior(Settings > General > When Quitting), which then applies with no question. A remembered End Everything deletes the local workspaces at every later Cmd-Q with no question: this is the same as setting "End Everything" in Settings today, and the choice is shown on the destructive (red, no key) button; Settings > When Quitting takes it back.The four buttons do not fit on one line of the 380 pt dialog card (review finding), so
CmuxDialogViewnow stacks a button row that does not fit: each button the card's width, the default (Keep) on top, as NSAlert does. Dialogs whose buttons fit are unchanged.Scripts (
cmux app quit, the socket), SIGTERM/SIGINT/SIGHUP and power off never ask. With no local terminal there is nothing to choose: it quits with Keep. The unsaved-changes question stays the quit's only question when it shows (#17501). End Sessions and End Everything are destructive buttons with no key.History (why this reverses a decision)
Takes back aba72a3 (cx-7vu, 2026-10-11, from Lawrence's 2026-10-07 "no confirm dialogs"): it made
QuitPolicy.decidequit with Keep and no question unless incognito programs ran, soapp.quitBehavior: "ask"(the default) never asked. Lawrence's newest words (2026-11-01 01:05Z) ask for the prompt; the newest words win. That commit is older than 7 days, so no take-back hold applies.Cause (STATUS 2718)
quit-reopen-handoffreproduces that handoff on the fleet (log linedaemon <pid> runs build b3b8b44...; handing off to be22bd3b...), and there every workspace, terminal, the holder and the scrollback came back. The holder-mode startup pass that can end a shell is fixed separately (fix/p0-quit-reopen-holder, risk:high).Tests
QuitChoiceTests(CmuxNextAppTests): red commit 8316ec3, fix 6bfec69. Because the feat base does not compile (NativePopupHost, fix(next): menus opened inside a native popup keep the popup open (bookmark bubble's Folder picker) #28704; fix base red: Takes back #28704: NSMenu tracking goes through one seam adapter (compile + assume_isolated/observer rows) #29856), the proof ran on scratch refs = the base red: Takes back #28704: NSMenu tracking goes through one seam adapter (compile + assume_isolated/observer rows) #29856 head 87682a6 plus these commits (never merged):cmdQWithLocalTerminalsAsksWithKeepAsTheDefault()recorded an issue at QuitChoiceTests.swift:15 ("quit with no question"); the 5 guard tests pass on the base too.Test run with 6 tests in 1 suite passed.cmux-ci compile-check --app --package CmuxNext --recipe-version 2of 808b04d: b67588d83ea2b504d8f83090 passed (CmuxNext, app, cmux-tui; the log compiles QuitPolicy.swift and QuitChoiceTests.swift).Journeys (kit cp/journey-quit-reopen-restore@7d7593f260e, independent review SHIP; a REAL Cmd-Q by CuaDriver to the key window, whole-host lease) on the dogfood build 9276e456:
quit-reopen-restoreRED 3/3 (cmux22, cmux23) andquit-reopen-handoffRED 2/2 (cmux19), each ONLY at the dialog checkquit_asked(logquit origin=interactive behavior=askthenquit choice=keep); every restore check passed (same daemon or a logged handoff, holder kept, workspaces, terminals, focus and scrollback 1..400 back). Green rounds on this fix's build (job b18f6c6e6283198241db0eb2) follow.CmuxDialogButtonFitTests(CmuxNextDesignTests): red 325b2a3, fix 1ac555c; on the scratch ref: red 4297aca step 2df0dc59f812dd14e73a0a32 (aws-m4pro-1): FAILED at CmuxDialogButtonFitTests.swift:25/28 (the base squeezes the buttons to 0 and 99 pt and the default is not on top); green 3eac1f0 step a87cf09a4e9d66c2ea92b510 (aws-m4pro-1): CmuxDialogButtonFitTests 2/2 and QuitChoiceTests 6/6 passed. Compile of 3eac1f0: 9b7f249e686afab9d998cf93 passed.Scripts that assumed Cmd-Q never asks now answer the dialog's Keep:
scripts/fleet-quit-persistence.sh(checkquit-asks),scripts/cmux-next/dialogs-undo-live.py(quit-asks),scripts/cmux-next/quit-launch-bound.py(interactive and wedged modes).plans/cmux-next/quit-persistence.mdrecords the change.Journey rounds on feat DEV builds (kit a1f077f line, whole-host, REAL Cmd-Q): base 66cbc3a8 (feat 8922fc2 + base red: Takes back #28704: NSMenu tracking goes through one seam adapter (compile + assume_isolated/observer rows) #29856): no dialog (red). Fix 8faff1dd (this PR on that base): the dialog shows with Keep default and is answered in 5/5 rounds (cmux21); every restore check passes in 5/5 (same daemon pid, holder, 4 workspaces, 5 terminals, focus on the split, scrollback 1..400).
Known open fault, NOT this PR's, tracked separately (lead 03:09Z): in 4 of those 5 rounds the first line typed into the restored, focused terminal right after the reopen was lost (keys handled by the window's responder, nothing reached the PTY). The base build shows it too (1 of 3), the dogfood line does not (0 of 5). Its own red-first fix PR follows; the journey keeps the step (not weakened).
Risk
risk:normalby the classifier (Swift UI logic, copy, tests; no new delete: End Everything's delete is existing code). Treated with high-tier evidence anyway, because a destructive quit choice is now one click from Cmd-Q: an independent review receipt at the exact head and a green compile. Ships behind the existingapp.quitBehaviorsetting ("keep" restores the old no-question quit).Localization
One key changed:
quit.button.quitEverything("Quit Everything") is nowquit.button.endKeepLayout("End Sessions, Keep Layout"), with the translations of the existing Settings choicesettings.choice.quitEndKeepLayout(all 20 locales; the 8 required ones marked translated).scripts/localization_catalog.py check: no new parity error.Changelog
Observed / Not run