[lawrencecchen]

What

QUIT P0 (Lawrence 2026-11-01: "after quitting and reopening, i lose all the workspaces and terminals"). In holder mode (cmux-tui __holder, on every channel), a new owner's startup pass ended the person's shell in two cases:

1. A terminal whose registry row was not exactly running (a previous owner that stopped mid-adoption: a version handoff at the app's reopen, a crash) was recorded ended (adopt_from_holder_or_end), and then
2. release_unclaimed_holder_terminals sent CLOSE (2 s hang-up) to EVERY holder terminal no surface re-attached, a terminal with no registry row too. A failed re-attach ended the shell the same way.

Now:

• A row left adopting re-attaches like a running one.
• A re-attach that fails while the holder lists the terminal and its row is still running or adopting keeps the row and the shell (adopt_on_holder's failure paths now detach the shell instead of closing it), and the tab shows adopting (PendingTerminal::Adopting), not a live terminal that takes no input. The next start re-attaches it (the failed attach already took the holder's listed entry, so this process cannot retry it).
• After the pass the holder ends only terminals whose row exited or was tombstoned (the shell exited, or the person closed the tab). A terminal with no registry row stays held, on purpose: a registry that comes back thin (the P0 itself) reads "no row" for every terminal with no read error, so "absent = ended" would close every shell in exactly the failure this fixes. A visible "Running in background" list (pty-holder.md section 5) is a follow-up.

Tests (red first, fleet cargo on macOS, scripts/ci/cmux-tui-rust-check.sh test <filter>)

• holder_terminals a_restart_reattaches_a_holder_terminal_whose_row_was_left_adopting (2 holder cat terminals, SIGKILL the daemon, set one row adopting, restart: both shells kept by pid, screens kept, input works).
  • First branch fix/p0-quit-reopen-holder: red 3e9146e, step 6dfde043d7436ab48f80fb89 (aws-m4pro-2): FAILED at holder_terminals.rs:175, the terminal came back as "— session restored (previous process ended; it ran /bin/cat) —" with a NEW shell: the base ends the person's shell. Green 6c746e5, step 7e88d4e215446ab8f43257dc (aws-m4pro-4): 1 test run, 1 passed.
• holder_terminals a_restart_closes_only_holder_terminals_whose_row_ended (a tombstoned row's shell is closed; a terminal with no registry row keeps its shell).
• This branch: red 48fac40 step b54edea31a2b8a815cab67d5 (aws-m4pro-2): the adopting test FAILED at holder_terminals.rs:175 as on the first branch (a second test there failed at its own set-up, a foreign-key refusal of a row DELETE; that test was replaced). Head cc4b006 (after the review FIX): step 7d7f0cd255c5c7ad5dcd5579 (aws-m4pro-9, macOS, filter a_restart_): a_restart_reattaches_a_holder_terminal_whose_row_was_left_adopting PASS, a_restart_closes_a_holder_terminal_whose_row_was_tombstoned_and_keeps_the_rest PASS, all holder_terminals PASS; one acpmux test (durable_agents restart_reattach) failed once and passed on its rerun, not in this diff. Compile-check of cc4b006: 1e94d277757362b79298f099 passed (CmuxNext, app, cmux-tui).
• Clippy step 6298c55e7fe041d7e44d0356 stopped on a BASE red: acpmux/src/hub/processes.rs:256 redundant closure (feat base autofix 704abda), fixed on feat by base red: acpmux clippy redundant_closure in process_values #30009 (55361f1). Clippy for this PR's crates: step 2ab060fade549d7c4c993c40 (aws-m4pro-6) on scratch ref ffe8515 (this head merged with feat, which has base red: acpmux clippy redundant_closure in process_values #30009; never merged): clippy -D warnings PASS (cmux-tui-rust-check summary (mode clippy): clippy pass).
• Not covered by a test: the failed re-attach branch (no seam to inject an attach error without a test-only hook).

Risk

risk:high (lead 01:2xZ): this decides whether a person's running shell ends at a daemon start. Kept shells are user data. Needs an independent review receipt at the exact head and a green compile of the head.

Owners

PL-daemon holder (plans/cmux-next/pty-holder.md): told in STATUS. DAEMON SKEW owner (a619d54a5cbae4d7d) read the diff: no conflict with its host-mode design.

Changelog

• Fixed: a reopen after an update or a daemon restart no longer ends terminals whose re-attach was interrupted or failed.

Observed / Not run

• Observed: red and green of the first test above (fleet logs); prepush gate (no new violations); rustfmt --check.
• Not run yet: a journey on a build with this fix.

Open follow-ups (DAEMON SKEW design review)

• A Retry for a tab held as adopting (needs a fresh holder list); a UI and End path for no-row held shells; a seam to test the failed re-attach branch; a future holder ADOPT handoff must never end a held shell.

[lawrencecchen]

Design review (DAEMON SKEW owner, for the handoff part). Not a code-review receipt; that comes separately. Read at 004297f, code only, nothing built here.

The design is right for the version handoff:

• The launch-time handoff (DaemonService.start -> handOffIfStale -> shutdown-daemon without end_terminals) keeps the registry, the frontend_projections table and every holder shell by design (STATUS 2726). Before this PR, the new owner's startup pass was the one place where a handoff could end a shell: a row left adopting, or a failed re-attach, then release_unclaimed closing every unclaimed terminal. This PR removes both paths.
• Keeping a no-row terminal held is the correct side. A thin registry (the P0 itself) reads "no row" with no error, so "absent = ended" would close every shell in exactly the case this fixes.
• PendingTerminal::Adopting for a held terminal whose re-attach failed is the right visible state: the tab no longer looks live while input goes nowhere.

Open items. They are follow-ups and do not hold this PR:

1. A held terminal whose re-attach failed stays "adopting" until the next owner start. With "Quit and Keep Sessions", that can be days. A person-visible way out is needed: a Retry on the tab that re-lists the holder (the failed attach took the listed entry, so this needs a fresh HELLO/list), or the "Running in background" list. Please put it in the PR's follow-ups.
2. No-row held terminals have no UI and no end path except a person's explicit End, and that End is not built either. Same follow-up, same list.
3. The failed re-attach branch has no test. A seam (an injected attach error through HolderAdoption) would let a test pin it. Agreed that it can come later; the PR says so.
4. DAEMON SKEW b1 (moving a stale terminal host at a safe moment) is host mode only. It does not touch these files. A holder-mode ADOPT handoff (terminal-host-model-DESIGN.md section 4, "still open") must reuse this rule: it never ends a held shell on any error path.

[cubic-dev-ai]

1 issue found across 3 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="cmux-tui/crates/cmux-tui-core/src/mux/holder_adoption.rs">

<violation number="1" location="cmux-tui/crates/cmux-tui-core/src/mux/holder_adoption.rs:82">
P1: `is_ok_and` turns a holder-service lookup error into `held == false`; the subsequent adoption error then falls through to `mark_terminal_ended`, potentially ending a shell the holder still runs. Treat lookup failure as unknown and retain the row.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[cubic-dev-ai]

P1: is_ok_and turns a holder-service lookup error into held == false; the subsequent adoption error then falls through to mark_terminal_ended, potentially ending a shell the holder still runs. Treat lookup failure as unknown and retain the row.

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 cmux-tui/crates/cmux-tui-core/src/mux/holder_adoption.rs, line 82:

<comment>`is_ok_and` turns a holder-service lookup error into `held == false`; the subsequent adoption error then falls through to `mark_terminal_ended`, potentially ending a shell the holder still runs. Treat lookup failure as unknown and retain the row.</comment>

<file context>
@@ -41,17 +63,47 @@ impl Mux {
+        };
+        crate::holder_runtime::HolderService::may_run(host_root)
+            && crate::holder_runtime::HolderService::for_root(host_root)
+                .is_ok_and(|service| service.lists(id.as_bytes()))
+    }
+
</file context>

Suggested change

	.is_ok_and(|service| service.lists(id.as_bytes()))
	.map_or(true, |service| service.lists(id.as_bytes()))

[github-actions]

cmux status _{(one comment, edited in place; each line is updated by the job that owns it)}

Review cc4b006: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr29997 -f ref=fix/p0-quit-reopen-holder2 -f platform=macos -f backend_mode=local

red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on cba94d5 to compare; do not merge until green; see lane-rules

Shipped in:

• cmux NEXT nightly 1.0.0-nightly.4130052256501 (28aa638015)

[cursor]

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

[cubic-dev-ai]

All reported issues were addressed across 1 file (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[lawrencecchen]

Review (claude-review-subagent-29997): FIX at 457f76f
reviewer-session: e0cd4651-544b-4e36-9024-92a17a570369
author-session: a8c03bd88d9f5386c
R9: fail (REVIEW.md section 9, "errors swallowed or reported as success": the new keep branch logs "keep holder terminal" and shows the tab as adopting after Surface::adopt_on_holder has already SIGKILLed the shell; see blocking finding 2)
perf claim: none

I read the code only. I did not build or run anything. Scope: git diff 22abf5bc757..457f76f972b (3 commits: 48fac40 red, 004297f fix, 457f76f test fix-up). I read the fleet logs of b54edea3, 8cfd3f5a and 6298c55e. Step cbd3653c (green at this head) was still queued.

Blocking

1. At this head, holder_terminals.rs does not compile. Commit 457f76f only adds lines (+128, -0). It does not replace the old test. It pastes a second copy of the block. These items are now defined twice: a_restart_reattaches_a_holder_terminal_whose_row_was_left_adopting (lines 336 and 464), edit_registry (386, 514), two_holder_terminals (396, 524) and exits_within (415, 543). The result is E0428, so the whole holder_terminals test binary fails to build. The old a_restart_closes_only_holder_terminals_whose_row_ended test (line 558, with the DELETE FROM terminal_hosts that fails on the foreign key) is also still in the file. Step cbd3653c will fail at compile and will not prove anything. Fix: remove the first copy of each duplicate and the old DELETE test. Keep one copy of each helper, the adopting test and the new tombstoned test. Then run the green step again.

2. The new "a failed re-attach keeps the shell" branch also runs after errors that already ended the shell. held is read before adopt_from_holder. Then every Err takes the keep branch: it returns Ok(()) and sets PendingTerminal::Adopting. But some of those errors have already ended the shell or recorded its end:

   • In surface/holder_spawn.rs:121-124 and :156-160, Surface::adopt_on_holder sends service.close(terminal.chan, 0) (SIGKILL with no grace) and untracks the channel when the Kitty reservation or run_on_holder fails. When that happens, the PR logs "keep holder terminal ... for the next start" and the tab shows "adopting" until the next owner start. That can be days with "Quit and Keep Sessions". The shell is already dead and input goes nowhere. On the base, the same failure records missing_record, which is in RESPAWN_REASONS, so the person gets a respawned shell at once. For this path the PR is worse than the base, and both its doc comment and its PR text are wrong.
   • In finish_terminal_adoption (terminal_adoption.rs:687-692), if the surface is already dead (a shell that ended while no daemon ran: the holder's EXIT can land between attach and commit), it persists the exit (row Exited) and then bails. The keep branch then puts an Adopting marker on an exited terminal and skips detach_exited_terminal_topology and record_terminal_end. A kept tab then reads "adopting" instead of "exited". If persist_terminal_exit planned a respawn, set_pending_terminal_as (a HashMap::insert keyed by public id) overwrites the Respawning marker. The respawn worker clears only Respawning markers (supervisor.rs:135, terminal_respawn.rs:396), so the stale Adopting marker stays and terminal_is_respawning returns false while the respawn runs.

   Minimal root-cause fix:

   • (a) After an Err, read the row again and keep only if it is still Running or Adopting. Otherwise fall through to mark_terminal_ended, which already handles an Exited row: it clears the pending markers, detaches and records the end.
   • (b) Make adopt_on_holder's two failure paths leave the shell on the holder (service.detach(chan) + untrack, the same thing a daemon shutdown does) instead of close(chan, 0). The comment there ("the startup pass no longer lists it: end it") is the old policy that this PR reverses. If you choose not to do (b), then the keep branch must not run for those errors, and the PR text must stop saying that a failed re-attach keeps the shell.

   This branch has no test. A small seam would cover the first bullet: the codebase already has injected-failure seams such as consume_terminal_adoption_insert_failure.

Checked and correct

• Lock order (release_unclaimed). self.listed.lock().recover_poison().drain().collect() is one statement, so the listed guard drops before the loop. The ended closure takes workspace_registry only after that, with no holder lock held. link_or_launch(false) takes connecting only while it connects and releases it before the drain. The caller (adopt_terminal_hosts, the end of the snapshot loop) holds no lock. I found no deadlock.
• holder_lists before adopt_from_holder. Its side effects are the same as before (lists -> link_or_launch(false), which never launches). The first connect fills listed once (had_link == false). It reads listed without removing anything. If no holder runs, may_run is false and nothing changes. With a stale socket file, each row now tries two failed connects instead of one. That is negligible.
• Adopting rows with no incarnation or a stale one. adopt_from_holder still returns Ok(false) when incarnation is None. transition_terminal_lifecycle(Adopting, Some(row incarnation)) is Adopting -> Adopting with the same incarnation, which validate_terminal_transition allows. finish_terminal_adoption checks the incarnation again. Rows that a host-mode pass left adopting and that still have a record are handled in the records loop (handled_terminals), so they never reach this function. I found no wrong re-attach.
• Which rows close a shell. The snapshot loop skips Tombstoned, and Exited rows continue, so both reach release_unclaimed unclaimed and are closed. A Launching row, or a row whose adopt_from_holder returned Ok(false), is marked Exited in this same pass and is then closed. That matches the base. Exited | Tombstoned is the right set to close.
• Next start. A kept terminal (no row, or failed attach) is listed again at the next owner's HELLO (listed is per process), so it is re-attached or kept again. Nothing wedges. The PendingTerminal::Adopting map is in memory only, so a new process starts empty.
• set_pending_terminal there. It takes workspace_registry and then pending_terminals, each for a moment. The caller holds no lock. It is safe.
• Red reason. The base turns the left-adopting row into missing-host-record -> missing_record (a respawn reason), so the tab gets a new shell. That is exactly the line-175 failure in 6dfde043 and b54edea3 ("session restored (previous process ended; it ran /bin/cat)"). The red failed for the right reason. 7e88d4e2 is a valid green of that test on the first branch.

Non-blocking

• Tests.
  • The new tombstoned test also passes on the base, because the base closes every unclaimed terminal. It guards against keeping too much. It is not a regression test for this fix. Say so in the PR body.
  • The "no row keeps its shell" behavior, which is the main change in release_unclaimed, now has no test. To get it back, run PRAGMA foreign_keys = OFF on the test connection before the DELETE (SQLite enforces foreign keys per connection), or delete the resource_terminals row first.
  • The adopting test copies edit_registry inline. Use the helper.
  • Timing is fine: 20 s deadlines for a 2 s hang-up grace, and the 3 s fixed sleep leaves with the old test.
  • Clean-up is fine: track records every shell and holder pid, and Drop SIGKILLs them and removes the directory.
• A tab that is kept adopting and that the person closes does not end its holder shell until the next start. after_terminal_close -> terminate_discovered_terminal_host_in handles only host records. The row is tombstoned, so the next start's release_unclaimed closes the shell. pty-holder.md section 5 says an explicit close sends CLOSE. Add this to the follow-up list together with the Retry/"Running in background" item that the design review raised.
• holder_lists turns a for_root error into "not held" (is_ok_and) (cubic P1). adopt_from_holder gets the same error, so the base already ended the row in this case and there is no regression. The safe side for "unknown" would be to keep. Do this together with blocking finding 2(a).
• Ok(false) with held == true (for example, a Running row with no incarnation) still marks the row ended, and the same pass then closes the shell. I believe holder rows always carry an incarnation, so I did not block on this. Consider keeping these rows as well.
• Style.
  • self.workspace_registry.lock().unwrap() in ended panics on a poisoned registry. Nearby startup code uses unwrap_or_else(PoisonError::into_inner).
  • The "keep unclaimed holder terminal {}" log prints the channel. The terminal id (TerminalId::from_bytes(id).to_hex()) would let someone match it to a registry row.
  • rustfmt: the diff looks formatted.
• Clippy is not proven for this diff. 6298c55e stopped at the known base error acpmux/src/hub/processes.rs:256 (redundant closure) and reported "could not compile acpmux". After that failure cargo may not have finished checking cmux-tui-core or cmux-tui. Run clippy again when the base is fixed, or limit it to -p cmux-tui-core -p cmux-tui.
• Risk label. The PR has risk:normal, but its own body says risk:high (whether a person's running shell ends). It needs risk:high and the receipt at the exact head.

[cursor]

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

[lawrencecchen]

Check classification (author): the clippy step 6298c55e7fe041d7e44d0356 stopped at a base red, acpmux/src/hub/processes.rs:256 redundant closure (not in this diff; from feat base autofix 704abda). Its fix is #29995. Holder green at cc4b006: step 7d7f0cd255c5c7ad5dcd5579 (aws-m4pro-9) passed both new holder tests.

[lawrencecchen]

Review (claude-review-subagent-29997): SHIP at cc4b006
reviewer-session: e0cd4651-544b-4e36-9024-92a17a570369
author-session: a8c03bd88d9f5386c
R9: pass
perf claim: none

I read the code only. I did not build or run anything. Scope: git diff 22abf5bc757..cc4b006e88e (4 commits). This round I re-read the delta from 457f76f (my earlier FIX) and checked it against the whole diff. I read the fleet logs of 7d7f0cd2 and 1e94d277.

Earlier blocking findings: fixed

1. Test file compiles. holder_terminals.rs now has one copy each of a_restart_reattaches_a_holder_terminal_whose_row_was_left_adopting, edit_registry, two_holder_terminals, exits_within and a_restart_closes_a_holder_terminal_whose_row_was_tombstoned_and_keeps_the_rest. The DELETE FROM terminal_hosts test is gone, and the file has no duplicate items. Green 7d7f0cd2 at this head (aws-m4pro-9) compiled and passed all four cmux-tui::holder_terminals a_restart_* tests. Its one failure, acpmux::durable_agents ...a_restart_reattaches_live_agents..., failed in the full run and passed alone ("FLAKY"). It is outside this diff.
2. The keep branch no longer reports a kept shell that is gone.
   • In Surface::adopt_on_holder, both failure paths (the Kitty reservation and run_on_holder) now send service.detach(chan) + untrack instead of close(chan, 0). The holder handles DETACH (cmux-holder/src/holder.rs:908) by clearing sent, held and exit_sent. It keeps the shell and counts no poison-pill strike, so the next owner's HELLO lists the shell again. adopt_on_holder has only one caller (adopt_from_holder), so no other path changes. Holders older than wire v3 ignore DETACH (link.rs:370). After untrack -> forget, their frames are dropped until the next start. That matches what a daemon shutdown already does.
   • The keep branch now also needs row_still_live (a fresh read: Running or Adopting). A dead surface in finish_terminal_adoption has already persisted Exited, so the row is not live. The code falls through to mark_terminal_ended, which clears the markers, detaches and records the end, as the base did. The Respawning-overwrite case is gone with it. A poisoned lock or an unreadable row counts as live (the side that keeps the shell). No row counts as not live, and mark_terminal_ended is a no-op without a row.
   • Errors before the re-attach (the transition_terminal_lifecycle commit, restored_terminal_binding) leave the entry in listed. Its row is still Running or Adopting, so release_unclaimed keeps it.

Checked again (unchanged from the first round, still correct)

• Locks. listed is drained in one statement before ended takes workspace_registry. row_still_live and set_pending_terminal take the registry lock for a moment, and their caller holds no lock. I found no deadlock.
• holder_lists before the re-attach. It never launches a holder (link_or_launch(false)) and removes nothing from listed.
• Adopting rows. A row with no incarnation does not re-attach. Adopting -> Adopting needs the same incarnation, and finish_terminal_adoption checks it again.
• What closes a shell. Only Exited | Tombstoned rows. That includes a row this same pass marked ended.
• Next start. A kept terminal is listed again and re-attached or kept again. Nothing wedges.
• Red reason. 6dfde043 and b54edea3 failed at line 175 because the base turned the adopting row into missing_record and started a new shell. That is the right reason.
• ended now uses let Ok(registry) = ... else { return false }, so a poisoned lock keeps the shell. The keep log names the terminal id and the channel.

Non-blocking (follow-ups; none holds this PR)

• No way out of a re-attach that fails every time. Such a terminal now stays "adopting" at every start, because DETACH counts no strike. Before, it ended and respawned. The person can close the tab: that tombstones the row, and the next start closes the shell. Closing the tab does not end the shell before then, because terminate_discovered_terminal_host_in handles only host records. Put both in the Retry / "Running in background" follow-up that the design review already asked for.
• A held terminal with no row stays held with no UI, which is a deliberate trade-off. That case and the failed-re-attach branch have no test. The no-row test could come back with PRAGMA foreign_keys = OFF on the test connection before the DELETE.
• The tombstoned test also passes on the base. It guards against keeping too much. It is not a red-first test for the fix. The red-first test is the adopting test.
• A finish_terminal_adoption failure with a live surface (for example, a topology insert failure) leaves a surfaceless runtime attached until the next start, with an Adopting marker. If that shell exits in this process, I found no path that clears the marker. This case is rare, and the base respawned a second shell next to the orphan, which was worse.
• Clippy for these crates is still not proven. The last clippy step stopped at the known base error acpmux/src/hub/processes.rs:256. 1e94d277 is a Swift package compile, not Rust clippy. Run -p cmux-tui-core -p cmux-tui clippy, or run it again when the base is fixed.
• Label. gh pr view still shows risk:normal, but the PR text and the author say risk:high. Set risk:high so that this receipt is the evidence that gates the PR.

[lawrencecchen]

Correction (author): the acpmux clippy base red is fixed on feat by #30009 (55361f1), not #29995.

[lawrencecchen]

Check classification at cc4b006 (author):

• clippy (changed cmux-tui packages) (job 124732735490): BASE red. The only error is crates/acpmux/src/hub/processes.rs:256:71 redundant closure, not in this diff; fixed on feat by base red: acpmux clippy redundant_closure in process_values #30009 (55361f1). Clippy of this head merged with feat: fleet step 2ab060fade549d7c4c993c40 (scratch ref ffe8515, never merged).
• rerun (job 124732785833): INFRASTRUCTURE. The defaults-guard rerun step got HTTP 403 API rate limit exceeded; this PR changes no defaults.
• Receipt: fix(cmux-tui): a new owner never ends a shell the holder still runs (an adopting row re-attaches, a failed re-attach keeps the shell, only ended rows are closed) (QUIT P0) #29997 (comment) (SHIP at this head). Red/green and compile: see the description.

[blacksmith-sh]

Found 1 test failure on Blacksmith runners:

Failure

Test	View Logs
test_no_runner_picker_returns_a_hosted_mac (main.NoHostedMacInNightlies.test_no_run ner_picker_returns_a_hosted_mac)/ test_no_runner_picker_returns_a_hosted_mac (main.NoHostedMacInNightlies.test_no_run ner_picker_returns_a_hosted_mac)	View Logs

^{Need help on this PR? Tag @codesmith-bot with what you need.}