fix(adapters,engine): tell a lost mux session apart from an exited CLI - #522
Conversation
|
Warning Review limit reached
Next review available in: 52 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe change detects vanished multiplexer sessions during crash handling. It records ChangesSession-loss diagnostics
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GenericAdapter
participant Multiplexer
participant Escalation
participant Journal
GenericAdapter->>Multiplexer: Probe session existence after a crash
Multiplexer-->>GenericAdapter: Return confirmed presence or absence
GenericAdapter->>Escalation: Provide SessionResult.session_vanished
Escalation-->>Journal: Record diagnostic reason without changing routing
GenericAdapter->>Journal: Emit session-vanished lifecycle data
Possibly related issues
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/bmad_loop/adapters/generic.py (1)
298-311: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace
getattrwith direct attribute access.
getattr(self, "session_name")with no default has the same failure behavior asself.session_name. Both raiseAttributeErrorif the attribute is missing, so the "fail loud, no default" intent in the comment holds either way. Use direct attribute access; it is equally safe and more idiomatic.🔧 Proposed fix
self._note_lifecycle( handle.task_id, "session-vanished", - session=getattr(self, "session_name"), + session=self.session_name, status=status, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bmad_loop/adapters/generic.py` around lines 298 - 311, In the vanished-session branch of the lifecycle handling, replace getattr(self, "session_name") with direct self.session_name access when passing the session value to _note_lifecycle. Preserve the existing fail-loud behavior and all other arguments unchanged.Source: Linters/SAST tools
CHANGELOG.md (1)
163-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the
CHANGELOG.mdentry terse and imperative.The entry uses a long narrative and a declarative opening. Replace it with a short imperative summary that names the diagnostic fields and states that routing is unchanged.
Proposed wording
-- **A lost multiplexer session no longer reads as an agent that crashed (`#489`).** A window is - equally gone when the CLI exits and when something destroys the whole session under the run ... +- **Improve crash diagnosis when the multiplexer no longer reports a session (`#489`).** Include + the diagnostic in crash reasons, `session-end`/`dev-decision` journal entries, and + `session-vanished` lifecycle breadcrumbs. Preserve environment-fault composition and retry routing.As per coding guidelines,
CHANGELOG.mdentries must be underUnreleasedand remain terse, scannable, and imperative.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CHANGELOG.md` around lines 163 - 172, Rewrite the CHANGELOG entry as a terse, imperative summary under the Unreleased section. Name the affected diagnostic fields—crash verdict, operator-facing reason, session_vanished journal entry, and session-vanished lifecycle breadcrumb—and explicitly state that routing is unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/tui-guide.md`:
- Around line 223-224: Update the `session-vanished` diagnostic description in
the event list to say “the mux no longer reported the session during the run”
instead of asserting that the mux lost the session, preserving the wording as an
unconfirmed negative lookup.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Around line 163-172: Rewrite the CHANGELOG entry as a terse, imperative
summary under the Unreleased section. Name the affected diagnostic fields—crash
verdict, operator-facing reason, session_vanished journal entry, and
session-vanished lifecycle breadcrumb—and explicitly state that routing is
unchanged.
In `@src/bmad_loop/adapters/generic.py`:
- Around line 298-311: In the vanished-session branch of the lifecycle handling,
replace getattr(self, "session_name") with direct self.session_name access when
passing the session value to _note_lifecycle. Preserve the existing fail-loud
behavior and all other arguments unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4fb9255d-fbe8-4b02-a0b0-0e7bc43550aa
📒 Files selected for processing (15)
CHANGELOG.mddocs/FEATURES.mddocs/tui-guide.mdsrc/bmad_loop/adapters/base.pysrc/bmad_loop/adapters/generic.pysrc/bmad_loop/adapters/multiplexer.pysrc/bmad_loop/adapters/tmux_base.pysrc/bmad_loop/engine.pysrc/bmad_loop/escalation.pysrc/bmad_loop/sweep.pytests/test_engine.pytests/test_escalation.pytests/test_generic_tmux.pytests/test_plugin_workflows.pytests/test_sweep.py
Sessions complete on a hook Stop or on window death, and `list_window_ids` answers [] for both "the CLI exited" and "the whole session is gone" — a missing session exits non-zero and degrades to the same empty list. So a session destroyed under the run (an external reaper, a concurrent prune or stop, an operator kill-session, a server crash, the host sleeping) scored `crashed` exactly like a clean CLI exit, and the retry/defer reason an operator reads said only `dev session crashed`, pointing at the agent when the host was at fault. Ask `has_session` once a crash verdict is already reached, from the single `_final` chokepoint all five crash sites funnel through. Safe to ask that late: run()'s teardown kills the window, never the session. The answer rides `SessionResult.session_vanished` into the reason text, the `dev-decision` journal entry, and a `session-vanished` lifecycle breadcrumb. Worded as the observation, not the conclusion — the probe cannot tell who destroyed the session. Composed into `env_fault_pause_reason` so a lost session whose log also matches a transport pattern keeps both diagnoses instead of the env-fault branch swallowing one. A session reaped after flushing its result still scores `completed` and is not diagnosed: it produced something. Diagnosis only; routing is unchanged and a retry re-creates the session. Unit tests get a mux stand-in: the probe was the first unstubbed `self.mux` access on the crash path, so `make_dev_adapter`'s adapters were reaching the host multiplexer for real — scoring eight existing crash tests `session_vanished` and writing breadcrumbs, against that file's "unit tests need no tmux" contract. Refs bmad-code-org#489
7296287 to
264d84a
Compare
|
Addressed the CodeRabbit findings in 264d84a: the tui-guide breadcrumb description no longer overclaims ("no longer reported the session during the run"), and the CHANGELOG entry is rewritten terse and imperative. The |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 264d84a655
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # its own exit rather than the window dying — the label stays truthful | ||
| # there because it reports what the mux answered, not how the window | ||
| # ended. | ||
| vanished = status == "crashed" and self._session_vanished() |
There was a problem hiding this comment.
When the crashed fallback comes from the SessionEnd arm in wait_for_completion, the CLI has explicitly announced its exit; if the mux session is also absent by the time this probe runs, this stamps session_vanished=True and produces a reason claiming that the window disappearance is not evidence the CLI exited. That misdiagnoses exactly the exited-CLI case this change is meant to distinguish. Pass the crash origin into _final, or skip this probe for the SessionEnd path, so only a crash inferred from a failed window-liveness check receives this label.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Verified against the code at 264d84a — declining this one. The conjunction it describes (SessionEnd received AND session absent at probe time) can only arise when a host-level actor destroyed the session, and that is precisely when the suffix must fire:
- A normal CLI exit can never make
_session_vanished()true. Window 0 is a parked shell ("Window 0 is a plain shell so the session survives task windows closing",tmux_base.py), andrun()'s teardown kills only the window, never the session (comment in_session_vanished). So reaching this corner requires an independent destroyer — mux server crash, external reaper, operator/concurrentkill-session, host sleep — the exact candidates enumerated ingeneric.py's probe comment. - In every one of those cases, "the multiplexer no longer reports the session" is a true and operator-relevant fact that an announced CLI exit cannot explain. The likeliest route into the corner is causal, not coincidental: the destroyer HUPs the CLI, whose hook flushes
SessionEndon the way down. Skipping the probe on theSessionEndarm would make exactly that case read as a plain CLI crash — reintroducing the psmux can destroy a live session out from under a run (psmux#546) — decide the exposure and whether the reconcile can tell #489 misdiagnosis this PR fixes. - The suffix withdraws an inference; it does not assert the CLI failed to exit. That wording is deliberate on both sides of the seam:
session_failure_reason's docstring ("states what the evidence withdraws, not what it proves") and_final's comment, which names theSessionEndarm explicitly as considered. Routing is unchanged either way, and theSessionEndreceipt remains on disk in the run'sevents/directory (SignalWatcher never unlinks consumed event files), so no evidence is lost to the operator.
Threading the crash origin into _final to vary one explanation string in a double-fault corner would trade real plumbing for a marginal wording refinement; if anything, the improvement would be phrasing that acknowledges both facts, not narrowing the probe's scope.
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@dracic — heads up, I pushed a merge commit ( Both conflicts were docs-only and purely additive:
No code conflicts, and nothing of yours was rewritten — Verified locally on the merge commit: 4976 passed, 45 skipped, 5 xfailed; Happy to revert if you'd rather resolve it yourself. |
|
@codex review |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_engine.py`:
- Around line 7113-7120: Add an assertion that len(adapter.sessions) equals 2
after engine.run() in tests/test_engine.py lines 7113-7120, tests/test_sweep.py
lines 1646-1653, and tests/test_sweep.py lines 1663-1670, confirming both retry
sessions were consumed.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: becc8dd2-208a-4ccc-ad1b-f1163df8a112
📒 Files selected for processing (9)
CHANGELOG.mddocs/FEATURES.mddocs/tui-guide.mdsrc/bmad_loop/adapters/generic.pysrc/bmad_loop/adapters/multiplexer.pysrc/bmad_loop/adapters/tmux_base.pysrc/bmad_loop/engine.pytests/test_engine.pytests/test_sweep.py
🚧 Files skipped from review as they are similar to previous changes (6)
- src/bmad_loop/adapters/multiplexer.py
- docs/tui-guide.md
- CHANGELOG.md
- src/bmad_loop/adapters/tmux_base.py
- docs/FEATURES.md
- src/bmad_loop/engine.py
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The three bmad-code-org#489 diagnostic tests queue two crashed results but asserted only over the journal entries produced, so a regression that stopped after the first crash still passed: `all(...)` holds over a one-element list and `[-1]` reads the only decision there is. Ablating the engine to a single attempt kept every original assertion green. Assert both attempts are spent, matching the sibling env-fault test in test_sweep.py that already pins the same thing.
|
@coderabbitai — following up on the second nitpick from the first-round review body (the one collapsed under "🧹 Nitpick comments", Declining "Replace The premise is right as far as it goes: Applied the proposed diff verbatim and ran the repo's pinned typechecker:
Reverted; |
|
@codex review |
|
Tip For best results, initiate chat on the files or code changes.
I withdraw the nitpick. The other addressed items need no further action. (ᵔᴥᵔ) ✏️ Learnings added
You are interacting with an AI system. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 389a87bf32
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def session_failure_reason(role: str, result: SessionResult) -> str: | ||
| """The reason text for a non-completed session: ``<role> session <status>``, | ||
| plus the lost-session diagnosis (#489). |
There was a problem hiding this comment.
When the mux disappears during a plain, non-environment-fault _fix_phase session, this new reason builder is never used: _fix_phase leaves reason set to the original verification failure and ultimately returns an empty DEFER, so its callers defer with that stale verification reason instead of the vanished-session diagnosis. This affects repair sessions after a clean review or with review disabled; incorporate session_failure_reason("fix", result) into the non-completed fix path so the operator-facing defer explains why the repair did not run.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Verified against the code — the gap is real, but declining it for this PR: the root cause is pre-existing and _fix_phase is not part of this change.
This PR does not touch _fix_phase. git diff origin/main...389a87b -- src/bmad_loop/engine.py is exactly four hunks: the session_failure_reason import, the blocking-workflow _defer reason (~1538), session_vanished on the dev-decision entry (~1759), and _session_end_extras (~4016). _fix_phase begins at 4730 and is untouched.
The empty DEFER predates the change. return Decision(Action.DEFER) with no reason is already on origin/main at line 4811. The reason-substitution behavior the callers exhibit is therefore what the fix path did before #489 was addressed at all — reverting this PR entirely would not change it.
What the fix path already gets from this PR: the session-vanished lifecycle breadcrumb (adapter-side, role-agnostic), and session_vanished on the fix session's own session-end journal entry — _fix_phase calls _run_session(..., role="dev", ...), and every session-end funnels through _session_end_extras, whose comment names the roles deliberately: "EVERY role — dev, review, fix, migration, triage, injected workflows". So the greppable forensic record for a lost fix session is complete; what is missing is the operator-facing reason string on the exhaustion path, plus session_vanished on the fix-decision entry.
That narrower gap was already caught in this PR's pre-merge review and triaged as a follow-up (residual R1), precisely because closing it means touching _fix_phase reason plumbing that this diagnosis-only change deliberately stays out of — routing is unchanged here by design. It is getting filed as its own issue rather than widened into this PR.
Good catch on the substance; wrong PR to land it in.
There was a problem hiding this comment.
Reversing my earlier decline — this finding is in scope and is now fixed in 9bc39ef. My reasoning above was wrong and worth naming, since it was wrong in a way that would have shipped a half-delivered contract.
I graded the finding by provenance: _fix_phase is not in the diff, the bare Decision(Action.DEFER) already exists on main, therefore not this PR's problem. Both facts are true and neither is the right test. Applied consistently that rule says a change fixing a class of bug is finished once it has fixed the lines it happened to touch.
The right test is whether the PR delivers its stated purpose on this path, and its own FEATURES.md entry is explicit about what that purpose is: "the retry/defer reason an operator reads said only dev session crashed — pointing at the agent when the host was at fault … now … says so in the reason." Ran the scenario — two repair sessions destroyed by the mux, budget exhausted:
task.defer_reason -> "verify commands kept failing after clean review"
That is the #489 misdiagnosis one layer further out, and a worse instance of it: the original at least named a crashed session, while this blames the tree's verify commands for repairs that never ran. I also read past the disconfirming evidence already in the diff — this PR had rewritten a defer reason on another path (the blocking-workflow _defer → session_failure_reason), which settles that reason-string propagation was in scope. fix was an inconsistency in the PR's own coverage, not adjacent work.
The fix: carry the last non-completed session's own failure into the exhaustion DEFER. The callers already do fix.reason or "<fallback>", so no caller changes, and the verify-centric wording is untouched when the repair actually ran and only verify failed. fix-decision also gains session_vanished, matching dev-decision.
- verify commands kept failing after clean review
+ fix session crashed: the multiplexer no longer reports the session, so the
+ window's disappearance is not evidence the CLI exited
Two tests, both ablated: the new one fails with exactly the old string when the change is reverted, and a guard pin proves the repair-actually-ran path keeps its own wording. 4978 passed, pyright 0, trunk clean.
Thanks for holding the line on this one — the P2 was right and my triage was not.
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
`_fix_phase` returned a bare DEFER on budget exhaustion, and both callers substitute verify-centric text for an empty reason. So a repair session the multiplexer destroyed was filed as `verify commands kept failing after clean review` — the bmad-code-org#489 misdiagnosis one layer out, blaming the tree for repairs that never ran, where the original at least named a crashed session. Carry the last non-completed session's own failure into the DEFER. The callers' `fix.reason or ...` fallback picks it up, so no caller changes and the wording is untouched when the repair actually ran and only verify failed. `fix-decision` gains `session_vanished` for parity with `dev-decision`. Diagnosis only; routing is unchanged.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_engine.py (1)
7122-7128: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPin one journal record per consumed session.
Both tests use
all(...)on non-empty lists. A regression that drops a later journal record can still pass.
tests/test_engine.py#L7122-L7128: assertlen(decisions) == 2andlen(ends) == 2before checking the diagnostic fields.tests/test_engine.py#L7234-L7235: assertlen(fixes) == 2before checkingsession_vanished.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_engine.py` around lines 7122 - 7128, Strengthen the journal assertions in tests/test_engine.py at lines 7122-7128 by requiring exactly two decisions and exactly two session-end records before checking their fields; at lines 7234-7235, require exactly two fixes before validating session_vanished. Preserve the existing diagnostic assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tests/test_engine.py`:
- Around line 7122-7128: Strengthen the journal assertions in
tests/test_engine.py at lines 7122-7128 by requiring exactly two decisions and
exactly two session-end records before checking their fields; at lines
7234-7235, require exactly two fixes before validating session_vanished.
Preserve the existing diagnostic assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a64cf9ba-445f-42db-aecc-03f6864cf2f3
📒 Files selected for processing (3)
src/bmad_loop/engine.pytests/test_engine.pytests/test_sweep.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_sweep.py
`all(...)` over a non-empty list is vacuous about length, so a regression that dropped a later dev-decision, session-end or fix-decision still passed while the session-count assertions held: those pin that the sessions RAN, not that each was recorded. Count the records too.
|
@coderabbitai accepted and fixed in Verified the counts before asserting them ( - assert decisions and all(d["session_vanished"] is True for d in decisions)
+ assert len(decisions) == 2 and all(d["session_vanished"] is True for d in decisions)
- assert ends and all(e["session_vanished"] is True for e in ends)
+ assert len(ends) == 2 and all(e["session_vanished"] is True for e in ends)
- assert fixes and all(f["session_vanished"] is True for f in fixes)
+ assert len(fixes) == 2 and all(f["session_vanished"] is True for f in fixes)Left the two 4978 passed, pyright 0 errors, trunk clean. |
|
@codex review |
|
Tip For best results, initiate chat on the files or code changes.
Thanks for the precise fix and validation results. 🐇 If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The floor was never written down anywhere — no constant, no gate, no docs sentence — so the only floor a reader could derive was the one implied by the argv grammar, which is far older than anything the project tests. Reading it that way advertises support for tmux releases nobody has verified, so say the real number and say explicitly that the grammar is not the floor. Nothing is enforced here: tmux is still selected on the presence of the binary alone. psmux's version gate is a separate requirement for unrelated reasons (recycled-PID kills up to 3.3.6) and is untouched. Also records the psmux has-session foreign-server residual, which bmad-code-org#522 made user-facing: one server per session means a -t read naming a dead session can be answered by another server. A wrong True drops the lost-session diagnosis rather than inventing one, and needs an operator-created name collision.
2 participants
Addresses Q1 of #489 (the shared-ctl-session/untagged-fallback question, Q2, stays out — it is gated on #419)
Problem
Sessions complete on a hook
Stopevent or on window death — a hard invariant. But_window_aliveis a membership test overlist_window_ids(session), and that list is empty for two different worlds: the window died inside a live session (the CLI exited), and the session itself no longer exists. Both scoredcrashed, so a session destroyed under a run (an external reaper such as psmux/psmux#546, this tool's own prune/stop, an operatorkill-session, a mux server crash, a sleeping host) presented as an ordinary CLI crash — the reason an operator reads said onlydev session crashed, pointing at the agent when the host was at fault.Approach
Once a crash verdict is already reached,
_finalaskshas_session— the only call that separates the two worlds. The answer is a diagnostic label, never a routing input:SessionResult.session_vanished, stamped only when the verdict is alreadycrashed— never to reach a verdict, and never on a read-back upgrade tocompleted(a session reaped after flushing its result did produce something)session_failure_reason(… session crashed: the multiplexer no longer reports the session, so the window's disappearance is not evidence the CLI exited), adopted at the dev/review deciders, the blocking-workflow defer, and the sweep migration/triage sitessession-endentry via the_session_end_extraschokepoint (besideenv_fault), plusdev-decisionsession-vanishedbreadcrumb insession-lifecycle.jsonlcarrying the session name and verdictThe wording states what the evidence withdraws, not what it proves: the weak-False contract (
False= "the backend did not confirm the session"; transport failure raisesMultiplexerError, never returnsFalse) is now declared on theTerminalMultiplexer.has_sessionseam.MultiplexerErrorfrom the probe degrades to "not vanished" — the same "unknown is not dead" rule the liveness probe follows.Routing is deliberately untouched:
_ensure_sessionre-creates the session, so a retry already self-heals. Adapters with no session to lose (opencode-http) are inert via a constant-Falsebase hook.Testing
status == "crashed"gate or the env-fault composition fails the covering tests; the gate test reads the final status, so a regression to gating on the fallback also failssession-end+dev-decisionjournal fields (True on a vanished crash; absent/False on a plain crash), the read-back-upgrade skip, the non-crash skip, the post-kill-reconcile pass-through of a flagged verdict, and the blocking-workflow defer reason end to end_UnitMux);uv run pytestgreen,uv run pyrightclean but for the pre-existingplatform_utilwin32 pair,trunk checkcleanSummary by CodeRabbit
Bug Fixes
Documentation