feat: bounded auto-resume + loud exited-pane banner for crashed agent CLIs (katas 7w4h, xkhx) - #560
Merged
Merged
Conversation
…dation findings Stage-2 validation of 24 assumptions (18 verified, 4 falsified) against the actual repo. Falsifications fixed: - exited panes clear terminalId -> lifecycle slice re-keyed by paneId with lastTerminalId frame-matching (Tasks 6/7, D-2) - second exit-hook site in fenced freshagent crate -> REST panes documented out of auto-resume scope (Task 2) - codex/opencode/amplifier session ids discovered at first prompt -> per- provider provenance documented in D-5 - terminal-pane BindingRow launch fields are None -> Task 4 derives params from state.settings Verified-with-amendment: SpawnGate acquire added to the respawn seam; lease holder_conn + release discipline pinned; binding-still-Bound pre-respawn guard; corrected-flag semantics fixed with mandatory reconcile-after- replacement pin test; fold payload mirrors pane-reconcile.ts:428-436.
…ash-during-launch path, lease-shaped driver trait Independent cross-model review (iteration 1) found three major executable defects in the agent-crash-resilience plan; all fixed: 1. Stale auto-resume notice masked the settled error bar: recordTerminalExit now clears any notice (pinned by a new slice test), Task 7's false 'frames guarantee degradation' note replaced with a mandated TTL-expiry re-render backstop for silent settles. 2. Crash-during-launch path analyzed: recordTerminalExit moved to the top of the matched terminal.exit path (before the pendingDurableReplacement / exitedDuringLaunch early returns), showExitBanner extended to agent panes settled 'error' with a recorded non-zero exit (new scenario 6), and the subscriber-only exit fan-out concern closed via the verified synthesized-exit replay on attach. 3. Task 5 driver trait reshaped to the real asymmetric lease API: claim_session(+create_request_id) / complete_claim(new_terminal_id)->bool / fail_claim, plus pre_respawn_guard distinguishing session_owned_live from pane_closed; hub sketch, driver bullets, and unit-test list updated with two new scenarios (pane_closed guard, lease_completion_lost).
…NTIL survive semantics, test/unit/client test paths
1. Task 9 e2e test 4: fold FAKE_CRASH_UNTIL into the Step 1 fixture with
explicit precedence over FAKE_CRASH_MODE (crash while invocation <= N,
then SURVIVE — never falling through to the 'clean' exit-0 default),
rewrite test 4's arrangement comment with a non-vacuous liveness
assertion, and drop the incoherent once+pre-seed alternative.
2. Tasks 6/7/8 + File Structure table: move all 22 client test path
references from src/{store,components}/ to the verified repo
convention test/unit/client/{store,components}/ and replace the
conditional placement hedges with the verified convention statement.
Adds a 'Fresh-Eyes Review Fixes (iteration 2 addendum)' section with the
re-run self-review.
…s in Task 6/7 test code
The iteration-2 fix relocated the fully-written client test files to
test/unit/client/{store,components}/ but left relative './' import
specifiers inside the provided test code, making both tasks' red->green
verification gates unachievable (module-not-found before AND after
implementation):
- Task 6 slice test now imports '@/store/terminalLifecycleSlice'
(repo convention: tsconfig.json '@/*' alias, matching sibling
test/unit/client/store/turnCompletionSlice.test.ts)
- Task 7 banner test now imports '@/components/TerminalExitBanner'
The '../store/terminalLifecycleSlice' import inside the
src/components/TerminalExitBanner.tsx implementation sketch is correct
as-is and untouched. Iteration-3 addendum documents the fixes and the
re-run self-review.
Task 2 of the agent-crash-resilience lane (Lane D1): - WsState gains `auto_resume_tx: UnboundedSender<CrashEvent>`; the receiver is consumed by the auto-resume hub (Task 5) — until then tests drain it directly and other construction sites drop it (best-effort sends). - The handle_create inline `on_exit` closure is extracted verbatim into `build_pty_exit_hook(ExitHookDeps, ...)` so the Task 4 respawn seam can reuse the exact hook for respawned generations. Only additions: probe + create_request_id reads BEFORE finish/retire mutate state, the `finished` capture from `finish_pty_exit`, and the gated CrashEvent send (natural exits only — user kills return false and never send). - `CrashEvent` promoted pub (rides the public WsState field); its dead_code allow removed. The 6 remaining allows in auto_resume.rs stay (consumers land in Task 5) with updated comments; module doc now records the WS-created-only coverage boundary (REST/freshagent panes out of scope). - New integration test `auto_resume_events.rs`: natural nonzero exit sends one CrashEvent with code/mode/createRequestId/lifetime; user kill sends none. Harness gains `spawn_server_with_specs_and_auto_resume_rx`. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…uto-resume 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
… spawn Task 4 of the agent-crash-resilience lane: extracts the connection-independent respawn seam the Task 5 auto-resume orchestrator will call. Mirrors handle_create's CLI branch step-for-step (resume argv via resolve_coding_cli_command with LaunchIntent::Resume, SpawnGate acquire, spawn_blocking registry.create with the SAME createRequestId as the dead generation, shared build_pty_exit_hook so respawned generations report their own crashes, set_meta/identity/ledger-binding/locator-arm bookkeeping, terminals.changed broadcast). Shared helpers extracted (behavior-identical for handle_create): cli_provider_settings (codingCli.providers[mode] with the codex strip) and plan_codex_managed_launch (DEV-0006 S4 flag-gated codex plan). Known deviation (brief-sanctioned): FRESHELL_TAB_ID/FRESHELL_PANE_ID are wire fields not derivable from the pane ledger, so auto-resumed generations omit them. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…ing/replaced frames
Task 5 of the agent-crash-resilience lane: the hub that consumes the PTY
exit hook's CrashEvents and drives the full recovery pipeline —
decide() (Task 1 policy) → terminal.status{recovering} frame → backoff
sleep → post-sleep guards (live session owner / pane closed) → headless
sessionRef lease claim (full bounded ingress discipline, minted holder
conn id) → respawn_agent_terminal (Task 4 seam) → lease completion →
terminal.replaced frame (Task 3). Every settle path logs
terminal.auto_resume.settled with its reason.
- auto_resume.rs: spawn_hub_with_driver + AutoResumeDriver trait (lease-
shaped: complete_claim/fail_claim distinct; claim/complete are async —
the production impl awaits the kill→confirm lease discipline) +
WsAutoResumeDriver production impl + pub spawn_auto_resume_hub[_with_
delays]. All six Task-1 dead_code allows removed (now live).
- terminal.rs: kill_session_ref_holder_and_confirm /
confirm_pid_dead_within_500ms widened to pub(crate) for the driver's
lease paths.
- pane_ledger.rs: is_enabled() accessor — an ENABLED ledger's None from
bound_session_ref_for_terminal means retired (pane_closed); a disabled
ledger's None means nothing.
- main.rs: hub spawned from the boot path (spawn_idle_monitor precedent);
the Task-2 dropped-receiver placeholder removed.
- tests: 9 fake-driver unit scenarios (paused time) + auto_resume_e2e.rs
(real registry: 3 spawns then settle exited, recovering/replaced frames
on a subscribed client, and the mandatory reconcile-after-replacement
pin: old terminalId reconciles to an attach verdict naming the NEW live
terminal with corrected absent). Harness gained a hub-ON spawn variant
with injected delays (in-process env writes would leak across tests).
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…ling Ephemeral (never-persisted) terminalLifecycle slice keyed by paneId with a lastTerminalId frame-matching key, recorded at the top of TerminalView's matched terminal.exit path (before the pendingDurableReplacement / exitedDuringLaunch early returns) so every exit path captures it. terminal.status 'recovering' frames and the new terminal.replaced frame match old-terminalId frames via lastTerminalId; replacement folds the new terminalId into the pane via applyReconcileAttach (incl. serverInstanceId). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…agent panes
Task 7 of the crash-resilience lane (D1 user-facing surface):
- TerminalExitBanner: pure presentational role=alert error bar
('process exited (code N)', codeless post-reload variant) with a real
<button> aria-labelled 'Relaunch <mode> session', plus the role=status
auto-resume notice strip (recovering/resumed).
- TerminalView wiring: agent-pane-only render conditions covering settled
'exited' (non-zero or unknown exit) AND settled 'error' with a recorded
non-zero exit (crash-before-attach-ready via failLaunch); clean exits and
shells stay quiet. Relaunch dispatches resetPaneForReconcileCreate
(intent 'respawn', same sessionRef) so the pane resumes its session.
- Deterministic TTL backstop: one scheduled re-render at notice expiry so a
'recovering' notice orphaned by a silent settle degrades to the alert
without waiting for an unrelated re-render.
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
… replacement Lane D1 Task 8. Pins with a real-store harness (tabs/panes/settings/ turnCompletion/terminalLifecycle reducers + terminalDetachMiddleware): - terminal.idle on the NEW terminalId chimes exactly once after the fold (fresh per-terminalId idle-dedupe baseline; replay at same 'at' deduped) - terminal.idle on the OLD terminalId is dropped by the owner-lookup guard (no false chime, no baseline consumed) - resolvePaneActivity reads via the NEW terminalId immediately after the fold; a leftover record on the dead old id cannot wedge the pane busy - the fold emits NO terminal.detach for the old id Production change (test 4 was red): add applyReconcileAttach to the detach-middleware skip list — the terminal.replaced fold rebinds the pane while the old terminal is already exited server-side, so a detach would only draw a server error (and in the reconcile flow the fold runs on a fresh connection before any attach, so no live subscription can leak). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…et, relaunch Task 9 of the agent-crash-resilience lane: end-to-end proof of every user story with a real RustServer + real browser. - fixtures/fake-crashing-claude-cli.mjs: fake claude CLI with FAKE_CRASH_MODE=once|always|clean and FAKE_CRASH_UNTIL=N (precedence: crash while invocation <= N, then SURVIVE, bypassing the mode checks so the 'clean' default can never exit-0 the surviving invocation). - specs/agent-crash-autoresume-rust.spec.ts: four tests — crash → bounded auto-resume with --resume <same id> + role=status notice; always-crash → exactly 3 invocations + role=alert 'process exited (code 1)' + Relaunch button; clean exit stays quiet; Relaunch drives invocation 4 with --resume <same id>, clears the alert, and the pane stays live >=1s. - playwright.config.ts: RUST_ONLY_SPECS + rust-chromium testMatch appends. All 4 tests pass against the Tasks 1-8 feature implementation. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…slice Full-suite gate (Task 10) surfaced 331 failures across 44 pre-existing client test files: they build partial Redux stores (no terminalLifecycle reducer) and render TerminalView, whose render-path selectors read root.terminalLifecycle.byPaneId unguarded -> TypeError. Fix mirrors the repo's paneRuntimeActivity defensive-access convention: the *From selectors accept `undefined` slice state and the root wrappers mark the slice optional, degrading to `undefined` (no banner/notice). Production behavior is unchanged - store.ts always mounts the reducer. Red->green: new regression pin in terminalLifecycleSlice.test.ts failed with the exact production signature, passes after the fix; TerminalView.renderer.test.tsx (previously failing) is green again. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
… shows no phantom crash banner
The exit-banner Relaunch handler dispatched resetPaneForReconcileCreate but
never cleared the pane's terminalLifecycle entry. If the relaunch create was
rejected (pane settles 'error' with no new terminal.exit), the PREVIOUS
crash's exit record still satisfied the 'error' && exitRecord &&
exitCode !== 0 arm of showExitBanner, resurrecting a stale
"process exited (code 1)" for what is actually a launch failure.
Relaunch now also dispatches clearTerminalLifecycle({ paneId }); a genuine
crash-during-relaunch still repopulates via recordTerminalExit. Test pins
both the record clearing on click and the quiet 'error' settle afterwards.
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…omments - auto_resume.rs emit_recovering/emit_replaced: serialize failures now tracing::error! instead of being silently swallowed. - auto_resume.rs auto_resume_delays(): warn when FRESHELL_AUTO_RESUME_DELAYS_MS is set but unparseable and the override falls back to defaults (parse semantics unchanged). - auto_resume.rs attempts map: document that retaining exhausted/pane-closed entries is deliberate (eviction would refill the retry budget), not a leak. - terminal.rs missing-probe fallback: document that lifetime_ms = i64::MAX is a deliberate "treat as healthy / fresh budget" sentinel, not "unknown". - terminalLifecycleSlice.ts header: reconcile the denylist wording with store.ts's allowlist note (slices allowlisted; pane fields deny-stripped). - terminalDetachMiddleware.ts: name BOTH applyReconcileAttach dispatch sites (crash fold + reconnect reconcile) and each one's detach-skip safety reason. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> # Conflicts: # crates/freshell-ws/src/terminal.rs # test/e2e-browser/playwright.config.ts
…a enn3) Post-merge integration of origin/main (83c5541) into the crash-resilience lane: - respawn_agent_terminal: pass the now-required cancel receiver to SpawnGate::acquire (gate relocated to freshell-freshagent by #559 with a cancellable acquire). Auto-resume is server-initiated with no connection to die, so it holds a never-fired sender (the REST-door convention). - Pin the interaction: a queue-full spawn gate rejects a respawn loudly (LaunchUnresolvable, queue_rejections == 1) instead of spawning — a crash-loop storm is exactly what the gate bounds. Verified RED via a gate-bypass mutation before landing. - Test harness fixups: add main's create_dedupe/shutdown_started fields to the lane's WsState builders, and the lane's auto_resume_tx field to main's restore_spawn_gate/rest_ws_shared_gate builders. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
… is never parsed Council MEDIUM fix (7w4h/xkhx review, Sam + tester-breaker): the client parsed attempt/max/exit out of the recovering notice's English text (TerminalView regexes with invented ?? 2 / ?? 1 fallbacks), so rewording UI copy would silently change behavior. - terminal.status: add OPTIONAL maxAttempts/exitCode fields (TS type + Rust TerminalStatus with skip_serializing_if), populated by the auto-resume emit_recovering path. - Client reads the FIELDS; both regexes and the fallbacks are deleted. reason prose is purely presentational now. - Contract: npm run contract:generate (ws-server-messages.schema.json), Rust pin added to freshell-protocol roundtrip (terminal.status recovering fixture), ws e2e pin strengthened (maxAttempts/exitCode asserted on the wire). npm run test:port green (38 passed). - Red-first: exitBanner client test sends a recovering frame with DELIBERATELY reworded prose + typed fields; failed on the regex code (banner showed invented 1/2 exit 1), passes reading fields (1/3, 137). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
… silently end auto-resume Council MEDIUM fix (7w4h/xkhx review, crusty): main.rs discarded the hub task's JoinHandle, so a future panic in the driver would silently end auto-resume forever — reinstating the exact overnight-grey-pane incident this feature prevents. The hub now self-supervises: the supervisor task owns the crash-event receiver and attempts map OUTSIDE a catch_unwind boundary, so a driver panic drops only the in-flight body future. The panic is logged ERROR (terminal.auto_resume.hub_panicked) and the loop restarted after a bounded escalating backoff (1s/5s/30s/60s cap, counter reset after a 30s-healthy run) — a hot-panicking driver cannot spin, and auto-resume is never permanently lost. (Respawning with a fresh channel would NOT work: PTY exit hooks clone the sender at hook-build time; owning rx across the unwind is the load-bearing part.) Red-first: hub_survives_driver_panic_and_processes_subsequent_crashes — a one-shot injected driver panic on event 1, then event 2 must still be respawned. Failed on the unsupervised hub (10s timeout, second event never processed); passes under supervision (~1s). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
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
1 participant
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.
katas 7w4h + xkhx — crash resilience for coding-agent terminal panes (motivating incident: an overnight claude pane crashed exit-1 at 03:57 and sat silently grey for 6 hours).
(1) Bounded server-side auto-resume: non-zero agent CLI exits retry the provider resume with backoff and user-visible typed notices; never for clean exits, plain shells, user kills, or idle-reaped terminals (discriminator is structural — council-verified); retry exhaustion settles exited; respawn cap bounds crash loops; auto-resume respawns acquire the shared spawn gate (mutation-verified pin).
(2) Loud exited-pane banner: "process exited (code N)" + one-click Relaunch resuming the SAME session (never silently fresh — resume path hardcodes the session id).
(3) Status integrity: chime/dedupe survive server-driven terminal replacement (pinned).
Council review (7-lens): LAND, zero blockers; two MEDIUM fixes applied post-review: auto-resume notices now carry typed attempt/maxAttempts/exitCode frame fields (client regexes on prose deleted; contract regenerated, both pins updated, test:port 38/38) and the auto-resume hub is now self-supervising (catch_unwind + bounded escalating restart backoff — a driver panic can no longer silently end auto-resume). Remaining non-blocking council findings tracked as kata znhn.
Verification: cargo workspace 2091 green, coordinated JS green (4327/4640/350), lint clean, e2e 19/19 incl. full restore-contract-wall.