diff --git a/crates/freshell-codex/src/launch_plan.rs b/crates/freshell-codex/src/launch_plan.rs index 679f8870b..01f851fc1 100644 --- a/crates/freshell-codex/src/launch_plan.rs +++ b/crates/freshell-codex/src/launch_plan.rs @@ -52,6 +52,10 @@ pub const CODEX_REMOTE_NON_LOOPBACK_MESSAGE: &str = /// launches. Council fence: S4's wiring is FLAG-GATED, default OFF — legacy's proxy path /// exists to feed durability binding (S5), so the launch mechanism ships dark until S5's /// consumers land; S5 + the flag-default flip land together. +/// +/// D-C-REVISIT(FRESHELL_CODEX_MANAGED_LAUNCH): flipping this default ON must +/// revisit the ~226s REST permit-hold (grep for D-C-REVISIT; decision in +/// docs/plans/2026-07-27-rest-spawn-gate.md §D-C). pub const FRESHELL_CODEX_MANAGED_LAUNCH_ENV: &str = "FRESHELL_CODEX_MANAGED_LAUNCH"; /// Whether the managed-launch flag value enables the S4 wiring. Only the exact string diff --git a/crates/freshell-freshagent/src/lib.rs b/crates/freshell-freshagent/src/lib.rs index ca83086bd..5afa88c99 100644 --- a/crates/freshell-freshagent/src/lib.rs +++ b/crates/freshell-freshagent/src/lib.rs @@ -1587,6 +1587,20 @@ async fn send_keys( // COLD-START + create the durable session. `create_session` runs `ensure_started` // (spawn serve → bounded health wait — the DEV-0001 fix, NO warm-proxy) then // `POST /session`. Success here IS the cold-start-clean fingerprint. + // + // bccd item 4 (council enn3 D-D evaluate-and-decide) — DELIBERATELY + // UNGATED. Decision reversed at plan validation: gating this + // cold-start would hold a spawn permit for a worst-case ~50-70s + // (health_timeout 20_000ms serve.rs:303 + request_timeout 30_000ms + // serve.rs:308,546 under the permit; the serialized `running`-mutex + // queue adds ~20s per failing holder) vs the 10s gate waits at + // every other door — and k cold first-sends queued on the singleton + // mutex would hold k permits, starving ALL spawn doors. The + // double-mutex single-flight already bounds actual sidecar forks + // to AT MOST ONE server-wide: the gate would add starvation + // without reducing fork concurrency. Moving the acquire inside + // the single-flight would invert lock order (cycle hazard). + // Decision record: docs/plans/2026-07-29-znhn-bccd-followups.md §D-7. let created = match manager .create_session(None, None, pane.cwd.as_deref()) .await diff --git a/crates/freshell-freshagent/src/opencode_ws.rs b/crates/freshell-freshagent/src/opencode_ws.rs index 3cf7bf427..bf0efe94b 100644 --- a/crates/freshell-freshagent/src/opencode_ws.rs +++ b/crates/freshell-freshagent/src/opencode_ws.rs @@ -557,6 +557,12 @@ impl FreshOpencodeState { // Already materialized: THE continuity fix — reuse it, no new session. real_id } else { + // Deliberately ungated (bccd item 4, council D-D evaluate-and-decide; + // same decision as the REST send-keys cold arm in lib.rs): the + // single-flighted singleton manager bounds sidecar forks to AT MOST + // ONE server-wide, and gating would starve the spawn budget on + // ~50-70s worst-case cold-start holds (see the lib.rs comment for + // the arithmetic). Revisit if the sidecar ever grows fork fan-out. let created = match manager.create_session(None, None, cwd.as_deref()).await { Ok(created) => created, Err(err) => { diff --git a/crates/freshell-freshagent/src/spawn_gate.rs b/crates/freshell-freshagent/src/spawn_gate.rs index d06ea71e8..aef4d3580 100644 --- a/crates/freshell-freshagent/src/spawn_gate.rs +++ b/crates/freshell-freshagent/src/spawn_gate.rs @@ -186,6 +186,22 @@ impl SpawnGate { } } + /// Acquire a spawn permit with no caller-side cancellation (kata znhn + /// item 4). Two doors have no connection whose death should cancel the + /// wait — the REST door and the auto-resume respawn door. They used to + /// mint never-fired watch channels at every call site; that wart belongs + /// to the gate. The never-fired sender now lives HERE, held across the + /// acquire, so `Cancelled` is unreachable by construction (kata bccd + /// item 3: no caller-side sender exists to drop). The timeout still + /// bounds the wait. + pub async fn acquire_uncancellable( + &self, + timeout: Duration, + ) -> Result { + let (_cancel_tx, mut cancel_rx) = tokio::sync::watch::channel(false); + self.acquire(timeout, &mut cancel_rx).await + } + pub fn queued_total(&self) -> u64 { self.queued_total.load(Ordering::Relaxed) } @@ -213,6 +229,61 @@ mod tests { watch::channel(false) } + #[tokio::test] + async fn acquire_uncancellable_waits_for_a_permit_and_never_cancels() { + let gate = Arc::new(SpawnGate::new(1, 64)); + let (_tx, mut rx) = cancel_pair(); + let held = gate + .acquire(Duration::from_secs(1), &mut rx) + .await + .expect("holder"); + let g2 = Arc::clone(&gate); + let waiter = + tokio::spawn(async move { g2.acquire_uncancellable(Duration::from_secs(5)).await }); + // Deterministic queue barrier (established idiom in this module). + for _ in 0..200 { + if gate.queued_total() == 1 { + break; + } + tokio::time::sleep(Duration::from_millis(5)).await; + } + assert_eq!(gate.queued_total(), 1, "uncancellable waiter must queue"); + drop(held); + assert!( + waiter.await.unwrap().is_ok(), + "waiter acquires after release" + ); + assert_eq!(gate.cancellations(), 0, "Cancelled must be unreachable"); + } + + #[tokio::test] + async fn acquire_uncancellable_times_out_as_timeout_not_cancelled() { + let gate = SpawnGate::new(1, 64); + let (_tx, mut rx) = cancel_pair(); + let _held = gate + .acquire(Duration::from_secs(1), &mut rx) + .await + .expect("holder"); + let err = gate + .acquire_uncancellable(Duration::from_millis(50)) + .await + .unwrap_err(); + assert_eq!(err, SpawnGateError::Timeout); + assert_eq!(gate.timeouts(), 1); + assert_eq!(gate.cancellations(), 0); + } + + #[tokio::test] + async fn acquire_uncancellable_rejects_queue_full_loudly() { + let gate = SpawnGate::new(0, 0); + let err = gate + .acquire_uncancellable(Duration::from_millis(50)) + .await + .unwrap_err(); + assert_eq!(err, SpawnGateError::QueueFull); + assert_eq!(gate.queue_rejections(), 1); + } + #[tokio::test] async fn bounds_concurrency_to_n_and_all_complete() { // Spawn N+K creates, assert max in-flight == N, all complete. diff --git a/crates/freshell-freshagent/src/terminal_tabs.rs b/crates/freshell-freshagent/src/terminal_tabs.rs index 3e01e4fd7..750f58f1f 100644 --- a/crates/freshell-freshagent/src/terminal_tabs.rs +++ b/crates/freshell-freshagent/src/terminal_tabs.rs @@ -34,7 +34,8 @@ use std::collections::HashSet; use axum::extract::{Path, Query, State}; use axum::http::{HeaderMap, StatusCode}; -use axum::response::Response; +use axum::response::{IntoResponse, Response}; +use axum::Json; use serde_json::{json, Value}; use uuid::Uuid; @@ -580,28 +581,42 @@ fn codex_launch_error_response( /// REST mapping of a spawn-gate rejection (WS analogue: /// `spawn_gate_error_parts` in freshell-ws/src/terminal.rs). -/// QueueFull -> 429: the caller should back off and retry. -/// Timeout -> 503: spawn capacity unavailable right now. -/// The retry guidance lives in the MESSAGE because the MCP bridge -/// (server/mcp/freshell-tool.ts) surfaces only the message text, not the -/// HTTP status. Body key is `code`+`message` (never `error`) so the MCP -/// http-client's `data.error || data.message` precedence keeps showing the -/// human message. -fn spawn_gate_error_response(err: crate::spawn_gate::SpawnGateError) -> Response { +/// QueueFull -> 429 with Retry-After (bccd item 1): header for HTTP +/// convention + `retryAfterMs` body field (house convention, session-lease +/// SESSION_RESERVED). The retry guidance ALSO stays in the MESSAGE because +/// the MCP bridge (server/mcp/freshell-tool.ts) surfaces only message text. +/// Timeout -> 503: spawn capacity unavailable right now. +/// Body key is `code`+`message` (never `error`). +fn spawn_gate_error_response( + err: crate::spawn_gate::SpawnGateError, + retry_after: std::time::Duration, +) -> Response { match err { - crate::spawn_gate::SpawnGateError::QueueFull => crate::fail_json_code( - StatusCode::TOO_MANY_REQUESTS, - "SPAWN_QUEUE_FULL", - "Too many concurrent terminal spawns; retry shortly".to_string(), - ), + crate::spawn_gate::SpawnGateError::QueueFull => { + let secs = retry_after.as_secs().max(1); + ( + StatusCode::TOO_MANY_REQUESTS, + [( + axum::http::header::RETRY_AFTER, + axum::http::HeaderValue::from(secs), + )], + Json(json!({ + "status": "error", + "code": "SPAWN_QUEUE_FULL", + "message": "Too many concurrent terminal spawns; retry shortly", + "retryAfterMs": retry_after.as_millis() as u64, + })), + ) + .into_response() + } crate::spawn_gate::SpawnGateError::Timeout => crate::fail_json_code( StatusCode::SERVICE_UNAVAILABLE, "SPAWN_TIMEOUT", "Timed out waiting for a terminal spawn slot".to_string(), ), - // Unreachable on the REST door: the handler holds its cancel - // sender (never fired, never dropped) across the whole acquire. - // Mapped like Timeout so an impossible arm still fails safe. + // Unreachable since acquire_uncancellable (znhn item 4): no cancel + // sender exists on this door at all. Mapped like Timeout so an + // impossible arm still fails safe. crate::spawn_gate::SpawnGateError::Cancelled => crate::fail_json_code( StatusCode::SERVICE_UNAVAILABLE, "SPAWN_TIMEOUT", @@ -1049,17 +1064,14 @@ pub(crate) async fn spawn_terminal_pane( // server wiring keep legacy behavior. let spawn_permit = match state.spawn_gate() { Some(rest_gate) => { - // The gate's acquire is cancellable via a watch channel (the WS - // door wires its per-connection cancel signal). REST has no such - // signal: hold a live, never-fired sender for the whole acquire - // (dropping it early would read as "connection gone" => - // Cancelled). If the HTTP request itself is dropped while - // QUEUED, axum drops this future and the gate's queue-slot - // guard reclaims the slot — nothing has been spawned yet. - let (_cancel_tx, mut cancel_rx) = tokio::sync::watch::channel(false); + // Uncancellable acquire (kata znhn item 4): REST has no + // connection whose death should cancel the wait — the gate owns + // that semantics now. If the HTTP request is dropped while + // QUEUED, axum drops this future and the gate's queue-slot guard + // reclaims the slot — nothing has been spawned yet. match rest_gate .gate - .acquire(rest_gate.timeout, &mut cancel_rx) + .acquire_uncancellable(rest_gate.timeout) .await { Ok(permit) => Some(permit), @@ -1074,7 +1086,7 @@ pub(crate) async fn spawn_terminal_pane( let _ = freshell_sessions::amplifier_stub::gc_stub_if_unused(&stub.session_dir); } - return Err(spawn_gate_error_response(err)); + return Err(spawn_gate_error_response(err, rest_gate.timeout)); } } } @@ -1268,6 +1280,14 @@ async fn settle_gated_create(inputs: GatedSettleInputs) -> Result= 8` lower bound (the fast path skips + // the counter). Mirrors the re-acquire precedent at + // `abort_burst_rest_creates_stay_gated...`. let state = state_with_registry(); let registry = state.terminal_registry.clone().unwrap(); let gate = Arc::new(crate::spawn_gate::SpawnGate::new(1, 64)); state.set_spawn_gate(Arc::clone(&gate), std::time::Duration::from_secs(30)); let router = app(state); + let held = gate + .acquire_uncancellable(std::time::Duration::from_secs(1)) + .await + .expect("test pre-hold of the single permit"); + let mut handles = Vec::new(); for _ in 0..16 { let r = router.clone(); @@ -3990,20 +4038,32 @@ mod tests { post(r, "/api/tabs", shell_create_body(), true).await })); } + + // Every request must queue behind the held permit — exact, not + // probabilistic. + for _ in 0..600 { + if gate.queued_total() == 16 { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(5)).await; + } + assert_eq!( + gate.queued_total(), + 16, + "all 16 burst requests must queue while the permit is held" + ); + assert!( + handles.iter().all(|h| !h.is_finished()), + "no request may complete while the budget is fully held (max-in-flight <= budget)" + ); + + drop(held); let mut terminal_ids = Vec::new(); for h in handles { let (status, body) = h.await.expect("request task"); assert_eq!(status, StatusCode::OK, "{body}"); terminal_ids.push(body["data"]["terminalId"].as_str().unwrap().to_string()); } - // With 16 near-simultaneous arrivals and 1 permit, the overwhelming - // majority must have queued. (The fast path skips the counter when - // the queue is momentarily empty, hence >= 8, not == 15.) - assert!( - gate.queued_total() >= 8, - "burst did not queue through the gate: queued_total={}", - gate.queued_total() - ); assert_eq!(gate.queue_rejections(), 0, "no loud rejections expected"); assert_eq!(gate.timeouts(), 0, "no permit-wait timeouts expected"); diff --git a/crates/freshell-protocol/src/client_messages.rs b/crates/freshell-protocol/src/client_messages.rs index 2e9f8fed7..861635b60 100644 --- a/crates/freshell-protocol/src/client_messages.rs +++ b/crates/freshell-protocol/src/client_messages.rs @@ -28,6 +28,8 @@ pub enum ClientMessage { TerminalCodexCandidatePersisted(TerminalCodexCandidatePersisted), #[serde(rename = "terminal.attach")] TerminalAttach(TerminalAttach), + #[serde(rename = "terminal.autoResumeCancel")] + TerminalAutoResumeCancel(TerminalAutoResumeCancel), #[serde(rename = "terminal.detach")] TerminalDetach(TerminalDetach), #[serde(rename = "terminal.input")] @@ -81,7 +83,7 @@ pub enum ClientMessage { /// The exact `type` discriminants of every client→server message, in the frozen /// inventory's order. This is the T0 conformance checklist. -pub const CLIENT_MESSAGE_TYPES: [&str; 29] = [ +pub const CLIENT_MESSAGE_TYPES: [&str; 30] = [ "amplifier.activity.list", "claude.activity.list", "client.diagnostic", @@ -103,6 +105,7 @@ pub const CLIENT_MESSAGE_TYPES: [&str; 29] = [ "pane.reconcile.request", "ping", "terminal.attach", + "terminal.autoResumeCancel", "terminal.codex.candidate.persisted", "terminal.create", "terminal.detach", @@ -245,6 +248,15 @@ pub struct TerminalCodexCandidatePersisted { pub terminal_id: String, } +/// znhn item 2: the user opts out of an in-flight auto-resume ("stop +/// trying, leave it dead"). Carries the OLD (crashed) terminal id — the +/// same id the recovering `terminal.status` frame was broadcast with. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct TerminalAutoResumeCancel { + pub terminal_id: String, +} + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct TerminalAttach { diff --git a/crates/freshell-protocol/src/server_messages.rs b/crates/freshell-protocol/src/server_messages.rs index 25d6dfd18..f1b50ae75 100644 --- a/crates/freshell-protocol/src/server_messages.rs +++ b/crates/freshell-protocol/src/server_messages.rs @@ -247,6 +247,12 @@ pub enum SessionRepairEvent { pub enum RuntimeStatus { Running, Recovering, + /// The auto-resume SETTLE frame (kata znhn item 3): broadcast with the + /// OLD terminal id whenever a planned auto-resume settles without a + /// replacement (guard-abort, retries exhausted, flap circuit breaker, + /// user cancel) so the client clears the recovering notice on a FRAME, + /// never on a timer. + Exited, } /// Terminal lifecycle status in the inventory (`running | exited`). @@ -1114,6 +1120,12 @@ pub struct TerminalStatus { pub exit_code: Option, #[serde(skip_serializing_if = "Option::is_none")] pub reason: Option, + /// Flap-circuit-breaker settle frames only: successful auto-resumes + /// inside the rolling window. The client renders the "crashed N times" + /// banner from this FIELD — `reason` prose is presentational and must + /// never be parsed (council 7w4h/xkhx). + #[serde(skip_serializing_if = "Option::is_none")] + pub resume_cycles: Option, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] diff --git a/crates/freshell-protocol/tests/inventory.rs b/crates/freshell-protocol/tests/inventory.rs index 93d06b1d3..3e546274b 100644 --- a/crates/freshell-protocol/tests/inventory.rs +++ b/crates/freshell-protocol/tests/inventory.rs @@ -31,12 +31,12 @@ fn client_types_match_inventory_exactly() { let inv = inventory(); assert_eq!( inv["clientToServer"]["count"].as_u64(), - Some(29), - "inventory declares 29 client→server types" + Some(30), + "inventory declares 30 client→server types" ); let expected = json_type_set(&inv["clientToServer"]["types"]); let actual: BTreeSet = CLIENT_MESSAGE_TYPES.iter().map(|s| s.to_string()).collect(); - assert_eq!(actual.len(), 29, "crate declares 29 client types (no dups)"); + assert_eq!(actual.len(), 30, "crate declares 30 client types (no dups)"); assert_eq!( actual, expected, "CLIENT_MESSAGE_TYPES must equal the frozen inventory (no missing/extra)" @@ -61,14 +61,14 @@ fn server_types_match_inventory_exactly() { } #[test] -fn combined_surface_is_86() { +fn combined_surface_is_87() { let all = all_message_types(); - assert_eq!(all.len(), 86, "29 client + 57 server = 86 discriminants"); + assert_eq!(all.len(), 87, "30 client + 57 server = 87 discriminants"); // sorted + unique let unique: BTreeSet<&str> = all.iter().copied().collect(); assert_eq!( unique.len(), - 86, + 87, "no discriminant collides across directions" ); } diff --git a/crates/freshell-protocol/tests/roundtrip.rs b/crates/freshell-protocol/tests/roundtrip.rs index d25e4f5fb..004b34d9c 100644 --- a/crates/freshell-protocol/tests/roundtrip.rs +++ b/crates/freshell-protocol/tests/roundtrip.rs @@ -484,3 +484,41 @@ fn terminal_input_blocked_unknown_terminal_roundtrips_and_conforms() { other => panic!("expected TerminalInputBlocked, got {other:?}"), } } + +#[test] +fn terminal_status_exited_settle_frame_roundtrips() { + // znhn item 3: the auto-resume SETTLE frame — status 'exited' on the + // existing terminal.status message, with the typed resumeCycles field + // (flap-circuit-breaker settles only). + let msg = ServerMessage::TerminalStatus(TerminalStatus { + status: RuntimeStatus::Exited, + terminal_id: "t1".into(), + attempt: None, + max_attempts: None, + exit_code: None, + reason: Some("pane_closed".into()), + resume_cycles: Some(3), + }); + let json = serde_json::to_value(&msg).unwrap(); + assert_eq!(json["type"], "terminal.status"); + assert_eq!(json["status"], "exited"); + assert_eq!(json["resumeCycles"], 3); + assert!( + json.get("attempt").is_none(), + "None fields are skip-serialized" + ); + let back: ServerMessage = serde_json::from_value(json).unwrap(); + assert_eq!(back, msg); +} + +#[test] +fn terminal_auto_resume_cancel_roundtrips() { + // znhn item 2: the user opts out of an in-flight auto-resume. + let json = serde_json::json!({"type": "terminal.autoResumeCancel", "terminalId": "t1"}); + let msg: ClientMessage = serde_json::from_value(json.clone()).unwrap(); + match &msg { + ClientMessage::TerminalAutoResumeCancel(c) => assert_eq!(c.terminal_id, "t1"), + other => panic!("wrong variant: {other:?}"), + } + assert_eq!(serde_json::to_value(&msg).unwrap(), json); +} diff --git a/crates/freshell-server/src/main.rs b/crates/freshell-server/src/main.rs index d5ff3048d..bfee56cad 100644 --- a/crates/freshell-server/src/main.rs +++ b/crates/freshell-server/src/main.rs @@ -290,6 +290,17 @@ async fn main() -> ExitCode { // `freshell_ws::spawn_idle_monitor` for the periodic sweep this feeds. registry.set_auto_kill_idle_minutes(settings.safety.auto_kill_idle_minutes); freshell_ws::spawn_idle_monitor(registry.clone(), std::time::Duration::from_secs(30)); + // e2e knob (kata znhn item 2): sub-second flap cycles would trip the + // registry generation cap (3 per 30s liveness window) before the hub's + // circuit breaker can ever fire. Production default unchanged. + if let Some(ms) = std::env::var("FRESHELL_RESPAWN_LIVENESS_WINDOW_MS") + .ok() + .and_then(|raw| raw.trim().parse::().ok()) + .filter(|v| *v > 0) + { + registry.set_respawn_liveness_window_ms(ms); + tracing::info!(ms, "respawn_liveness_window_override"); + } // TERM-13 fix: honor `settings.terminal.scrollback` at boot (the Rust // registry previously used a fixed 8MiB replay-log cap for every // terminal, ignoring the configured value entirely). @@ -547,6 +558,7 @@ async fn main() -> ExitCode { tokio::sync::mpsc::unbounded_channel::(); let ws_state = WsState { auto_resume_tx, + auto_resume_cancels: Default::default(), activity: Some(activity_hub.clone()), identity: terminal_identity.clone(), opencode_locator: opencode_locator.clone(), diff --git a/crates/freshell-ws/src/auto_resume.rs b/crates/freshell-ws/src/auto_resume.rs index cbd4e9eea..cf76ab930 100644 --- a/crates/freshell-ws/src/auto_resume.rs +++ b/crates/freshell-ws/src/auto_resume.rs @@ -27,6 +27,75 @@ pub(crate) const AUTO_RESUME_DEFAULT_DELAYS_MS: [u64; 2] = [2_000, 10_000]; /// `DEFAULT_RESPAWN_LIVENESS_WINDOW_MS` in freshell-terminal). pub(crate) const AUTO_RESUME_HEALTHY_LIFETIME_MS: i64 = 30_000; +/// Flap circuit breaker (kata znhn item 2, user ruling: bounded-and-loud, +/// never infinite-and-silent). A "cycle" is one SUCCESSFUL auto-resume. +/// Cycles are pruned to a rolling window at each crash and are NEVER reset +/// by healthy generations — that is the cross-reset bound (it also bounds +/// the out-of-band `kill` resurrection loop). When a crash arrives with +/// cycles >= max, settle exited instead of resuming; Relaunch stays +/// available. +pub(crate) const AUTO_RESUME_DEFAULT_MAX_CYCLES: u32 = 5; +pub(crate) const AUTO_RESUME_DEFAULT_CYCLE_WINDOW_MS: i64 = 3_600_000; + +fn env_parse(name: &str, default: T) -> T { + std::env::var(name) + .ok() + .and_then(|raw| raw.trim().parse::().ok()) + .filter(|v| *v > T::default()) + .unwrap_or(default) +} + +pub(crate) fn auto_resume_max_cycles() -> u32 { + env_parse( + "FRESHELL_AUTO_RESUME_MAX_CYCLES", + AUTO_RESUME_DEFAULT_MAX_CYCLES, + ) +} +pub(crate) fn auto_resume_cycle_window_ms() -> i64 { + env_parse( + "FRESHELL_AUTO_RESUME_CYCLE_WINDOW_MS", + AUTO_RESUME_DEFAULT_CYCLE_WINDOW_MS, + ) +} +/// e2e knob: shrinking this lets tests exercise healthy-reset flap loops in +/// milliseconds. Production default matches the frozen 30s semantics. +pub(crate) fn auto_resume_healthy_lifetime_ms() -> i64 { + env_parse( + "FRESHELL_AUTO_RESUME_HEALTHY_LIFETIME_MS", + AUTO_RESUME_HEALTHY_LIFETIME_MS, + ) +} + +/// Hub policy knobs, resolved once at spawn (env-overridable for e2e). +#[derive(Debug, Clone)] +pub(crate) struct HubConfig { + pub delays: Vec, + pub healthy_lifetime_ms: i64, + pub max_cycles: u32, + pub cycle_window_ms: i64, +} + +impl HubConfig { + pub(crate) fn from_env() -> Self { + Self { + delays: auto_resume_delays(), + healthy_lifetime_ms: auto_resume_healthy_lifetime_ms(), + max_cycles: auto_resume_max_cycles(), + cycle_window_ms: auto_resume_cycle_window_ms(), + } + } +} + +/// Per-createRequestId resume history. `attempts` is the consecutive +/// fast-fail budget (reset by a healthy generation); `cycles` is the +/// wall-clock record of every successful auto-resume, pruned to the rolling +/// window — deliberately NOT reset by healthy generations. +#[derive(Debug, Default, Clone)] +pub(crate) struct ResumeHistory { + pub attempts: u32, + pub cycles: Vec, +} + /// Crash notification from the PTY exit hook. Only sent for NATURAL exits /// (`finish_pty_exit` returned `true`) — user kills never produce one. /// `pub` (not `pub(crate)`): it rides the public `WsState.auto_resume_tx` @@ -52,6 +121,11 @@ pub(crate) struct CrashContext<'a> { pub prior_attempts: u32, /// `registry.respawn_exhausted(create_request_id)` — outer loop bound. pub cap_exhausted: bool, + /// Successful auto-resumes inside the rolling window (flap breaker, + /// znhn item 2) — NEVER reset by healthy generations. + pub recent_cycles: u32, + /// Breaker threshold (cfg.max_cycles). + pub max_cycles: u32, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -60,7 +134,11 @@ pub(crate) enum AutoResumeDecision { SettleExited { reason: &'static str }, } -pub(crate) fn decide(ctx: &CrashContext<'_>, delays: &[u64]) -> AutoResumeDecision { +pub(crate) fn decide( + ctx: &CrashContext<'_>, + delays: &[u64], + healthy_lifetime_ms: i64, +) -> AutoResumeDecision { use AutoResumeDecision::SettleExited; if ctx.exit_code == 0 { return SettleExited { @@ -82,12 +160,19 @@ pub(crate) fn decide(ctx: &CrashContext<'_>, delays: &[u64]) -> AutoResumeDecisi reason: "no_resumable_identity", }; } + // Flap circuit breaker (znhn item 2): checked BEFORE the healthy-reset — + // a flap loop is exactly the case where every generation looks healthy. + if ctx.recent_cycles >= ctx.max_cycles { + return SettleExited { + reason: "flap_circuit_breaker", + }; + } if ctx.cap_exhausted { return SettleExited { reason: "respawn_cap_exhausted", }; } - let effective_prior = if ctx.lifetime_ms >= AUTO_RESUME_HEALTHY_LIFETIME_MS { + let effective_prior = if ctx.lifetime_ms >= healthy_lifetime_ms { 0 } else { ctx.prior_attempts @@ -143,7 +228,7 @@ const HUB_SUPERVISOR_BACKOFF_MS: &[u64] = &[1_000, 5_000, 30_000, 60_000]; pub(crate) fn spawn_hub_with_driver( driver: D, mut rx: tokio::sync::mpsc::UnboundedReceiver, - delays: Vec, + cfg: HubConfig, ) -> tokio::task::JoinHandle<()> { tokio::spawn(async move { // SUPERVISOR: `rx` and the attempts map are owned HERE, outside the @@ -152,20 +237,22 @@ pub(crate) fn spawn_hub_with_driver( // in every PTY exit hook) and the retry bookkeeping both survive the // restart. (Respawning with a fresh channel would NOT work: exit // hooks clone the sender at hook-build time.) - let mut attempts: std::collections::HashMap = std::collections::HashMap::new(); + let mut attempts: std::collections::HashMap = + std::collections::HashMap::new(); let mut consecutive_panics: u32 = 0; loop { let body_started = std::time::Instant::now(); - let body = std::panic::AssertUnwindSafe(run_hub_body( - &driver, - &mut rx, - &delays, - &mut attempts, - )); + let body = + std::panic::AssertUnwindSafe(run_hub_body(&driver, &mut rx, &cfg, &mut attempts)); match futures_util::FutureExt::catch_unwind(body).await { // Channel closed: every sender dropped (server shutdown). Ok(()) => return, Err(panic) => { + // Deliberate const (NOT cfg.healthy_lifetime_ms): panic + // supervision health is orthogonal to the attempts/cycles + // policy and must not follow the e2e knob — a shrunken + // env value would let a hot-panicking driver reset its + // own backoff. if body_started.elapsed().as_millis() as i64 >= AUTO_RESUME_HEALTHY_LIFETIME_MS { consecutive_panics = 0; @@ -202,14 +289,14 @@ pub(crate) fn spawn_hub_with_driver( async fn run_hub_body( driver: &D, rx: &mut tokio::sync::mpsc::UnboundedReceiver, - delays: &[u64], - attempts: &mut std::collections::HashMap, + cfg: &HubConfig, + attempts: &mut std::collections::HashMap, ) { { // Retaining exhausted / pane-closed entries is DELIBERATE (not a // leak): evicting on exhaustion would refill the retry budget for an // immediate manual-Relaunch re-crash. - let max_attempts = delays.len() as u32; + let max_attempts = cfg.delays.len() as u32; // Design note (serialization): handling events sequentially in ONE // task means a backoff sleep delays other panes' resumes by up to // 10s worst-case. Acceptable at v1 — crashes are rare, the budget is @@ -217,38 +304,70 @@ async fn run_hub_body( // (one respawn in flight, ever). while let Some(ev) = rx.recv().await { let sref = driver.resumable_session_ref(&ev.terminal_id); + // Prune the cycle record to the rolling window BEFORE deciding + // (znhn item 2): recent_cycles feeds the breaker threshold. + let now = crate::terminal::now_ms(); + let (prior_attempts, recent_cycles) = match &ev.create_request_id { + Some(k) => { + let h = attempts.entry(k.clone()).or_default(); + h.cycles.retain(|t| now - *t <= cfg.cycle_window_ms); + (h.attempts, h.cycles.len() as u32) + } + None => (0, 0), + }; let ctx = CrashContext { exit_code: ev.exit_code, mode: &ev.mode, create_request_id: ev.create_request_id.as_deref(), has_resumable_identity: sref.is_some(), lifetime_ms: ev.lifetime_ms, - prior_attempts: ev - .create_request_id - .as_deref() - .and_then(|k| attempts.get(k).copied()) - .unwrap_or(0), + prior_attempts, cap_exhausted: ev .create_request_id .as_deref() .map(|k| driver.cap_exhausted(k)) .unwrap_or(true), + recent_cycles, + max_cycles: cfg.max_cycles, }; - match decide(&ctx, delays) { + match decide(&ctx, &cfg.delays, cfg.healthy_lifetime_ms) { AutoResumeDecision::SettleExited { reason } => { if ev.mode != "shell" { + driver.emit_settled( + &ev.terminal_id, + reason, + if reason == "flap_circuit_breaker" { + Some(recent_cycles) + } else { + None + }, + ); driver.log_settled(&ev.terminal_id, reason); } - if reason == "clean_exit" || ev.lifetime_ms >= AUTO_RESUME_HEALTHY_LIFETIME_MS { + if reason == "clean_exit" || ev.lifetime_ms >= cfg.healthy_lifetime_ms { if let Some(k) = &ev.create_request_id { - attempts.remove(k); + // Reset attempts only, KEEP cycles: the breaker's + // cross-reset bound requires cycles to survive + // healthy generations (znhn item 2; validated A8: + // this condition must use the SAME configured + // healthy-lifetime as `decide`). + if let Some(h) = attempts.get_mut(k) { + h.attempts = 0; + } } } + // Fresh-eyes fix: a cancel whose terminal settles without + // ever reaching the Resume arm's take_cancel check would + // otherwise leak in auto_resume_cancels forever — the + // "removed on consumption" invariant must hold on EVERY + // settle tail. Consumed silently: the pane is already + // settled, there is nothing left to abort. + let _ = driver.take_cancel(&ev.terminal_id); } AutoResumeDecision::Resume { attempt, delay_ms } => { let (provider, session_id, cwd) = sref.expect("checked by decide"); let key = ev.create_request_id.clone().expect("checked by decide"); - attempts.insert(key.clone(), attempt); + attempts.entry(key.clone()).or_default().attempts = attempt; driver.emit_recovering( &ev.terminal_id, &ev.mode, @@ -258,14 +377,33 @@ async fn run_hub_body( ); tokio::time::sleep(std::time::Duration::from_millis(delay_ms)).await; // Guards AFTER the sleep — the world may have moved on. + if driver.take_cancel(&ev.terminal_id) { + // D-4 (validated A5): re-emit the settle frame here + // too. The handler's immediate frame covers the + // click-latency story; THIS frame guarantees a + // late-consumed or pre-seeded cancel is loud and can + // never strand a recovering notice. Idempotent + // client-side (recordAutoResumeSettled). + driver.emit_settled(&ev.terminal_id, "auto-resume cancelled", None); + driver.log_settled(&ev.terminal_id, "user_cancelled"); + continue; + } if let Some(reason) = driver.pre_respawn_guard(&provider, &session_id, &ev.terminal_id) { + driver.emit_settled(&ev.terminal_id, reason, None); driver.log_settled(&ev.terminal_id, reason); + // Cancel-set hygiene (fresh-eyes fix): a cancel that + // landed after the take_cancel check above must not + // leak — every settle tail cleans it up. + let _ = driver.take_cancel(&ev.terminal_id); continue; } if !driver.claim_session(&provider, &session_id, &key).await { + driver.emit_settled(&ev.terminal_id, "session_lease_held", None); driver.log_settled(&ev.terminal_id, "session_lease_held"); + // Cancel-set hygiene (see the guard tail above). + let _ = driver.take_cancel(&ev.terminal_id); continue; } let spec = RespawnSpec { @@ -288,19 +426,36 @@ async fn run_hub_body( attempt, max_attempts, ); + // One successful auto-resume = one breaker + // cycle (znhn item 2). Re-fetch the entry — + // the earlier borrow ended before the awaits. + attempts + .entry(key.clone()) + .or_default() + .cycles + .push(crate::terminal::now_ms()); } else { // Binding raced away between claim and completion; the // driver already killed its own orphan child. No // terminal.replaced — the pane stays settled exited. + driver.emit_settled(&ev.terminal_id, "lease_completion_lost", None); driver.log_settled(&ev.terminal_id, "lease_completion_lost"); } } Err(err) => { driver.fail_claim(&provider, &session_id, &key); tracing::warn!(terminal_id = %ev.terminal_id, error = %err, "terminal.auto_resume.respawn_failed"); + driver.emit_settled(&ev.terminal_id, "respawn_failed", None); driver.log_settled(&ev.terminal_id, "respawn_failed"); } } + // Cancel-set hygiene (fresh-eyes fix): a cancel landing + // DURING the respawn await — after the post-sleep + // take_cancel check — would otherwise leak forever. Too + // late to abort (the resume already ran); clean up on + // every tail of the respawn match (replaced / + // lease_completion_lost / respawn_failed). + let _ = driver.take_cancel(&ev.terminal_id); } } } @@ -373,6 +528,13 @@ pub(crate) trait AutoResumeDriver: Send + 'static { max_attempts: u32, ); fn emit_replaced(&self, old: &str, new: &str, exit_code: i64, attempt: u32, max_attempts: u32); + /// Broadcast the settle frame — `terminal.status { status: 'exited' }` + /// for the OLD terminal id (znhn item 3). Every agent-mode settle emits + /// it: the client clears the recovering notice on a FRAME, never on a + /// timer. `resume_cycles` is Some only for flap-circuit-breaker settles. + fn emit_settled(&self, terminal_id: &str, reason: &str, resume_cycles: Option); + /// Consume a pending user cancel for this terminal id (znhn item 2). + fn take_cancel(&self, terminal_id: &str) -> bool; fn log_settled(&self, terminal_id: &str, reason: &str); } @@ -617,6 +779,7 @@ impl AutoResumeDriver for WsAutoResumeDriver { reason: Some(format!( "{mode} crashed (exit {exit_code}) — auto-resuming, attempt {attempt}/{max_attempts}" )), + resume_cycles: None, }); match serde_json::to_string(&msg) { Ok(json) => { @@ -648,6 +811,35 @@ impl AutoResumeDriver for WsAutoResumeDriver { } } + fn emit_settled(&self, terminal_id: &str, reason: &str, resume_cycles: Option) { + let msg = + freshell_protocol::ServerMessage::TerminalStatus(freshell_protocol::TerminalStatus { + status: freshell_protocol::RuntimeStatus::Exited, + terminal_id: terminal_id.to_string(), + attempt: None, + max_attempts: None, + exit_code: None, + reason: Some(reason.to_string()), + resume_cycles: resume_cycles.map(i64::from), + }); + match serde_json::to_string(&msg) { + Ok(json) => { + let _ = self.state.broadcast_tx.send(json); + } + Err(err) => { + tracing::error!(terminal_id, error = %err, "terminal.auto_resume.settled_frame_serialize_failed"); + } + } + } + + fn take_cancel(&self, terminal_id: &str) -> bool { + self.state + .auto_resume_cancels + .lock() + .expect("auto_resume_cancels lock") + .remove(terminal_id) + } + fn log_settled(&self, terminal_id: &str, reason: &str) { tracing::info!(terminal_id, reason, "terminal.auto_resume.settled"); } @@ -660,7 +852,7 @@ pub fn spawn_auto_resume_hub( state: crate::WsState, rx: tokio::sync::mpsc::UnboundedReceiver, ) -> tokio::task::JoinHandle<()> { - spawn_auto_resume_hub_with_delays(state, rx, auto_resume_delays()) + spawn_hub_with_driver(WsAutoResumeDriver { state }, rx, HubConfig::from_env()) } /// [`spawn_auto_resume_hub`] with an explicit backoff schedule. The @@ -672,7 +864,14 @@ pub fn spawn_auto_resume_hub_with_delays( rx: tokio::sync::mpsc::UnboundedReceiver, delays: Vec, ) -> tokio::task::JoinHandle<()> { - spawn_hub_with_driver(WsAutoResumeDriver { state }, rx, delays) + spawn_hub_with_driver( + WsAutoResumeDriver { state }, + rx, + HubConfig { + delays, + ..HubConfig::from_env() + }, + ) } #[cfg(test)] @@ -688,14 +887,25 @@ mod tests { lifetime_ms: 5_000, prior_attempts: 0, cap_exhausted: false, + recent_cycles: 0, + max_cycles: AUTO_RESUME_DEFAULT_MAX_CYCLES, } } const DELAYS: [u64; 2] = [2_000, 10_000]; + fn test_cfg(delays: Vec) -> HubConfig { + HubConfig { + delays, + healthy_lifetime_ms: AUTO_RESUME_HEALTHY_LIFETIME_MS, + max_cycles: AUTO_RESUME_DEFAULT_MAX_CYCLES, + cycle_window_ms: AUTO_RESUME_DEFAULT_CYCLE_WINDOW_MS, + } + } + #[test] fn nonzero_agent_exit_resumes_with_schedule() { assert_eq!( - decide(&ctx(), &DELAYS), + decide(&ctx(), &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), AutoResumeDecision::Resume { attempt: 1, delay_ms: 2_000 @@ -706,7 +916,7 @@ mod tests { ..ctx() }; assert_eq!( - decide(&c, &DELAYS), + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), AutoResumeDecision::Resume { attempt: 2, delay_ms: 10_000 @@ -721,7 +931,7 @@ mod tests { ..ctx() }; assert_eq!( - decide(&c, &DELAYS), + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), AutoResumeDecision::SettleExited { reason: "clean_exit" } @@ -735,7 +945,7 @@ mod tests { ..ctx() }; assert_eq!( - decide(&c, &DELAYS), + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), AutoResumeDecision::SettleExited { reason: "not_agent_mode" } @@ -746,7 +956,7 @@ mod tests { ..ctx() }; assert_eq!( - decide(&c, &DELAYS), + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), AutoResumeDecision::SettleExited { reason: "not_agent_mode" } @@ -758,7 +968,10 @@ mod tests { for mode in AUTO_RESUME_MODES { let c = CrashContext { mode, ..ctx() }; assert!( - matches!(decide(&c, &DELAYS), AutoResumeDecision::Resume { .. }), + matches!( + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), + AutoResumeDecision::Resume { .. } + ), "mode {mode}" ); } @@ -771,7 +984,7 @@ mod tests { ..ctx() }; assert_eq!( - decide(&c, &DELAYS), + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), AutoResumeDecision::SettleExited { reason: "no_resumable_identity" } @@ -781,7 +994,7 @@ mod tests { ..ctx() }; assert_eq!( - decide(&c, &DELAYS), + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), AutoResumeDecision::SettleExited { reason: "no_create_request_id" } @@ -795,7 +1008,7 @@ mod tests { ..ctx() }; assert_eq!( - decide(&c, &DELAYS), + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), AutoResumeDecision::SettleExited { reason: "respawn_cap_exhausted" } @@ -809,7 +1022,7 @@ mod tests { ..ctx() }; assert_eq!( - decide(&c, &DELAYS), + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), AutoResumeDecision::SettleExited { reason: "retries_exhausted" } @@ -826,7 +1039,40 @@ mod tests { ..ctx() }; assert_eq!( - decide(&c, &DELAYS), + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), + AutoResumeDecision::Resume { + attempt: 1, + delay_ms: 2_000 + } + ); + } + + #[test] + fn flap_circuit_breaker_settles_when_cycles_reach_max() { + let c = CrashContext { + lifetime_ms: i64::MAX, // healthy — attempts would reset + recent_cycles: 5, + max_cycles: 5, + ..ctx() + }; + assert_eq!( + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), + AutoResumeDecision::SettleExited { + reason: "flap_circuit_breaker" + } + ); + } + + #[test] + fn cycles_below_max_still_resume_even_when_healthy_reset_applies() { + let c = CrashContext { + lifetime_ms: i64::MAX, + recent_cycles: 4, + max_cycles: 5, + ..ctx() + }; + assert_eq!( + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), AutoResumeDecision::Resume { attempt: 1, delay_ms: 2_000 @@ -868,6 +1114,13 @@ mod tests { cap_exhausted: bool, session: Option<(String, String, Option)>, guard: Option<&'static str>, + /// Pending user cancels (znhn item 2) — consumed by `take_cancel`. + cancels: std::collections::HashSet, + /// Test knob: when true, `respawn` inserts the spec's OLD terminal id + /// into `cancels` — simulates a user cancel landing DURING the + /// respawn await, i.e. after the hub's post-sleep take_cancel check + /// (the leak window the fresh-eyes review flagged). + insert_cancel_on_respawn: bool, claim_ok: bool, complete_ok: bool, panic_next_recovering: bool, @@ -879,6 +1132,9 @@ mod tests { completes: Vec, fails: Vec, settled: Vec<(String, String)>, + /// (terminal_id, reason, resume_cycles) — settle FRAMES broadcast + /// (znhn item 3), distinct from the `settled` log records. + settled_frames: Vec<(String, String, Option)>, } /// Records every orchestrator effect; each knob is mutable mid-test so @@ -896,6 +1152,8 @@ mod tests { cap_exhausted: false, session: Some(("claude".into(), "sess-1".into(), None)), guard: None, + cancels: std::collections::HashSet::new(), + insert_cancel_on_respawn: false, claim_ok: true, complete_ok: true, panic_next_recovering: false, @@ -907,6 +1165,7 @@ mod tests { completes: Vec::new(), fails: Vec::new(), settled: Vec::new(), + settled_frames: Vec::new(), })), } } @@ -936,6 +1195,17 @@ mod tests { fn set_panic_next_recovering(&self, v: bool) { self.lock().panic_next_recovering = v; } + fn set_cancelled(&self, terminal_id: &str) { + self.lock().cancels.insert(terminal_id.to_string()); + } + fn set_insert_cancel_on_respawn(&self, v: bool) { + self.lock().insert_cancel_on_respawn = v; + } + /// Pending (unconsumed) cancel entries — the leak the fresh-eyes + /// review flagged: must drain to zero on every settle/replaced tail. + fn pending_cancels(&self) -> usize { + self.lock().cancels.len() + } /// (old_terminal_id, attempt, max_attempts) fn recovering_calls(&self) -> Vec<(String, u32, u32)> { @@ -960,6 +1230,10 @@ mod tests { fn settled_reasons(&self) -> Vec { self.lock().settled.iter().map(|(_, r)| r.clone()).collect() } + /// (terminal_id, reason, resume_cycles) settle FRAMES (znhn item 3). + fn settled_frames(&self) -> Vec<(String, String, Option)> { + self.lock().settled_frames.clone() + } } impl AutoResumeDriver for FakeDriver { @@ -1017,6 +1291,17 @@ mod tests { let result = { let mut s = self.lock(); s.respawns.push(req.clone()); + if s.insert_cancel_on_respawn { + // Simulate a user cancel landing DURING the respawn — + // after the hub's post-sleep take_cancel check. The hub + // must still clean this entry up on the replaced tail. + let old_tid = s + .recovering + .last() + .map(|(tid, _, _)| tid.clone()) + .unwrap_or_default(); + s.cancels.insert(old_tid); + } s.respawn_result.clone() }; std::future::ready(result) @@ -1059,6 +1344,16 @@ mod tests { .replaced .push((old.to_string(), new.to_string(), attempt)); } + fn emit_settled(&self, terminal_id: &str, reason: &str, resume_cycles: Option) { + self.lock().settled_frames.push(( + terminal_id.to_string(), + reason.to_string(), + resume_cycles, + )); + } + fn take_cancel(&self, terminal_id: &str) -> bool { + self.lock().cancels.remove(terminal_id) + } fn log_settled(&self, terminal_id: &str, reason: &str) { self.lock() .settled @@ -1078,7 +1373,7 @@ mod tests { async fn crash_resumes_after_first_backoff_and_emits_frames() { let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); let fake = FakeDriver::healthy(); // identity present, cap ok, claim ok, respawn -> Ok("t-new") - let _hub = spawn_hub_with_driver(fake.clone(), rx, vec![2_000, 10_000]); + let _hub = spawn_hub_with_driver(fake.clone(), rx, test_cfg(vec![2_000, 10_000])); tx.send(crash("t1", 1, "claude", Some("cr-1"), 5_000)) .unwrap(); tokio::task::yield_now().await; @@ -1099,7 +1394,7 @@ mod tests { // crash again -> settled("retries_exhausted"), NO third respawn. let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); let fake = FakeDriver::healthy(); - let _hub = spawn_hub_with_driver(fake.clone(), rx, vec![2_000, 10_000]); + let _hub = spawn_hub_with_driver(fake.clone(), rx, test_cfg(vec![2_000, 10_000])); tx.send(crash("t1", 1, "claude", Some("cr-1"), 1_000)) .unwrap(); @@ -1147,7 +1442,7 @@ mod tests { // attempt resets to 1 with the first delay again. let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); let fake = FakeDriver::healthy(); - let _hub = spawn_hub_with_driver(fake.clone(), rx, vec![2_000, 10_000]); + let _hub = spawn_hub_with_driver(fake.clone(), rx, test_cfg(vec![2_000, 10_000])); tx.send(crash("t1", 1, "claude", Some("cr-1"), 1_000)) .unwrap(); @@ -1185,7 +1480,7 @@ mod tests { let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); let fake = FakeDriver::healthy(); fake.set_guard(Some("session_owned_live")); - let _hub = spawn_hub_with_driver(fake.clone(), rx, vec![2_000, 10_000]); + let _hub = spawn_hub_with_driver(fake.clone(), rx, test_cfg(vec![2_000, 10_000])); tx.send(crash("t1", 1, "claude", Some("cr-1"), 1_000)) .unwrap(); @@ -1198,6 +1493,11 @@ mod tests { fake.settled_reasons(), vec!["session_owned_live".to_string()] ); + // znhn #3: even the "silent" guard-abort broadcasts the settle frame. + assert_eq!( + fake.settled_frames(), + vec![("t1".to_string(), "session_owned_live".to_string(), None)] + ); } #[tokio::test(start_paused = true)] @@ -1206,7 +1506,7 @@ mod tests { // the backoff): no respawn, no claim, settled("pane_closed"). let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); let fake = FakeDriver::healthy(); - let _hub = spawn_hub_with_driver(fake.clone(), rx, vec![2_000, 10_000]); + let _hub = spawn_hub_with_driver(fake.clone(), rx, test_cfg(vec![2_000, 10_000])); tx.send(crash("t1", 1, "claude", Some("cr-1"), 1_000)) .unwrap(); @@ -1218,6 +1518,10 @@ mod tests { assert!(fake.respawn_calls().is_empty()); assert!(fake.claim_calls().is_empty()); assert_eq!(fake.settled_reasons(), vec!["pane_closed".to_string()]); + assert_eq!( + fake.settled_frames(), + vec![("t1".to_string(), "pane_closed".to_string(), None)] + ); } #[tokio::test(start_paused = true)] @@ -1226,7 +1530,7 @@ mod tests { let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); let fake = FakeDriver::healthy(); fake.set_claim_ok(false); - let _hub = spawn_hub_with_driver(fake.clone(), rx, vec![2_000, 10_000]); + let _hub = spawn_hub_with_driver(fake.clone(), rx, test_cfg(vec![2_000, 10_000])); tx.send(crash("t1", 1, "claude", Some("cr-1"), 1_000)) .unwrap(); @@ -1239,6 +1543,10 @@ mod tests { fake.settled_reasons(), vec!["session_lease_held".to_string()] ); + assert_eq!( + fake.settled_frames(), + vec![("t1".to_string(), "session_lease_held".to_string(), None)] + ); } #[tokio::test(start_paused = true)] @@ -1248,7 +1556,7 @@ mod tests { let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); let fake = FakeDriver::healthy(); fake.set_respawn_result(Err("spawn failed".into())); - let _hub = spawn_hub_with_driver(fake.clone(), rx, vec![2_000, 10_000]); + let _hub = spawn_hub_with_driver(fake.clone(), rx, test_cfg(vec![2_000, 10_000])); tx.send(crash("t1", 1, "claude", Some("cr-1"), 1_000)) .unwrap(); @@ -1259,6 +1567,10 @@ mod tests { assert_eq!(fake.fail_calls(), vec!["cr-1".to_string()]); assert!(fake.complete_calls().is_empty()); assert_eq!(fake.settled_reasons(), vec!["respawn_failed".to_string()]); + assert_eq!( + fake.settled_frames(), + vec![("t1".to_string(), "respawn_failed".to_string(), None)] + ); } #[tokio::test(start_paused = true)] @@ -1269,7 +1581,7 @@ mod tests { let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); let fake = FakeDriver::healthy(); fake.set_complete_ok(false); - let _hub = spawn_hub_with_driver(fake.clone(), rx, vec![2_000, 10_000]); + let _hub = spawn_hub_with_driver(fake.clone(), rx, test_cfg(vec![2_000, 10_000])); tx.send(crash("t1", 1, "claude", Some("cr-1"), 1_000)) .unwrap(); @@ -1287,6 +1599,10 @@ mod tests { fake.settled_reasons(), vec!["lease_completion_lost".to_string()] ); + assert_eq!( + fake.settled_frames(), + vec![("t1".to_string(), "lease_completion_lost".to_string(), None)] + ); } #[tokio::test(start_paused = true)] @@ -1295,7 +1611,7 @@ mod tests { // exit_code=0 / mode="shell" — zero respawn calls, zero recovering frames. let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); let fake = FakeDriver::healthy(); - let _hub = spawn_hub_with_driver(fake.clone(), rx, vec![2_000, 10_000]); + let _hub = spawn_hub_with_driver(fake.clone(), rx, test_cfg(vec![2_000, 10_000])); fake.set_cap_exhausted(true); tx.send(crash("t1", 1, "claude", Some("cr-1"), 1_000)) @@ -1318,6 +1634,332 @@ mod tests { assert!(fake.respawn_calls().is_empty()); assert!(fake.recovering_calls().is_empty()); assert!(fake.claim_calls().is_empty()); + // znhn #3: agent-mode settles are LOUD (frame emitted), shell is not. + assert_eq!( + fake.settled_frames(), + vec![ + ("t1".to_string(), "respawn_cap_exhausted".to_string(), None), + ("t2".to_string(), "no_resumable_identity".to_string(), None), + ("t3".to_string(), "clean_exit".to_string(), None), + ], + "shell-mode settles must NOT emit a settle frame" + ); + } + + #[tokio::test(start_paused = true)] + async fn flap_loop_trips_the_circuit_breaker_and_settles_loud() { + // 3 healthy flap cycles (lifetime >= healthy: attempts reset each + // time — pre-breaker this loops forever), then crash #4 must settle + // with the breaker reason + typed cycle count, and respawn nothing. + let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); + let fake = FakeDriver::healthy(); + let cfg = HubConfig { + max_cycles: 3, + healthy_lifetime_ms: 1, + ..test_cfg(vec![2_000, 10_000]) + }; + let _hub = spawn_hub_with_driver(fake.clone(), rx, cfg); + + for _ in 0..3 { + tx.send(crash("t1", 1, "claude", Some("cr-1"), 60_000)) + .unwrap(); + drain().await; + tokio::time::advance(std::time::Duration::from_millis(2_000)).await; + drain().await; + } + assert_eq!(fake.respawn_calls().len(), 3); + + tx.send(crash("t1", 1, "claude", Some("cr-1"), 60_000)) + .unwrap(); + drain().await; + tokio::time::advance(std::time::Duration::from_millis(2_000)).await; + drain().await; + assert_eq!(fake.respawn_calls().len(), 3, "no 4th respawn"); + assert_eq!( + fake.settled_frames().last().unwrap(), + &( + "t1".to_string(), + "flap_circuit_breaker".to_string(), + Some(3) + ) + ); + assert_eq!( + fake.recovering_calls().len(), + 3, + "no recovering frame for the breaker settle" + ); + } + + #[tokio::test(start_paused = true)] + async fn cycle_window_prunes_old_cycles_and_the_loop_may_continue() { + // max_cycles 2, cycle_window_ms 1 — every prior cycle is stale + // (wall-clock) by the time the next crash arrives, so the breaker + // never trips: 4 crash/resume rounds all succeed. + let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); + let fake = FakeDriver::healthy(); + let cfg = HubConfig { + max_cycles: 2, + cycle_window_ms: 1, + healthy_lifetime_ms: 1, + ..test_cfg(vec![2_000, 10_000]) + }; + let _hub = spawn_hub_with_driver(fake.clone(), rx, cfg); + + for _ in 0..4 { + // Real (not virtual) sleep: cycle timestamps are wall-clock, so + // >1ms of real time must pass for the window to prune them. + std::thread::sleep(std::time::Duration::from_millis(10)); + tx.send(crash("t1", 1, "claude", Some("cr-1"), 60_000)) + .unwrap(); + drain().await; + tokio::time::advance(std::time::Duration::from_millis(2_000)).await; + drain().await; + } + assert_eq!(fake.respawn_calls().len(), 4, "breaker must never trip"); + assert_eq!(fake.replaced_calls().len(), 4); + assert!(fake.settled_frames().is_empty()); + } + + #[tokio::test(start_paused = true)] + async fn healthy_generations_reset_attempts_but_never_cycles() { + // Healthy crashes reset the attempt budget (each resume is attempt 1) + // while the cycle record accumulates and trips the breaker at max. + let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); + let fake = FakeDriver::healthy(); + let cfg = HubConfig { + max_cycles: 2, + ..test_cfg(vec![2_000, 10_000]) + }; + let _hub = spawn_hub_with_driver(fake.clone(), rx, cfg); + + // Fast-fail crash: attempt 1. + tx.send(crash("t1", 1, "claude", Some("cr-1"), 1_000)) + .unwrap(); + drain().await; + tokio::time::advance(std::time::Duration::from_millis(2_000)).await; + drain().await; + // Healthy crash: attempts reset — attempt 1 again (not 2). + tx.send(crash("t2", 1, "claude", Some("cr-1"), 60_000)) + .unwrap(); + drain().await; + tokio::time::advance(std::time::Duration::from_millis(2_000)).await; + drain().await; + assert_eq!( + fake.recovering_calls(), + vec![("t1".into(), 1u32, 2u32), ("t2".into(), 1u32, 2u32)] + ); + assert_eq!(fake.respawn_calls().len(), 2); + + // Two successful resumes accumulated DESPITE the healthy reset: + // crash #3 trips the breaker. + tx.send(crash("t3", 1, "claude", Some("cr-1"), 60_000)) + .unwrap(); + drain().await; + assert_eq!(fake.respawn_calls().len(), 2, "breaker blocks the 3rd"); + assert_eq!( + fake.settled_frames().last().unwrap(), + &( + "t3".to_string(), + "flap_circuit_breaker".to_string(), + Some(2) + ) + ); + } + + #[tokio::test(start_paused = true)] + async fn eviction_and_decide_agree_on_the_configured_healthy_lifetime() { + // Between-thresholds pin (validated A8): cfg.healthy_lifetime_ms = + // 500, generation lifetime 1_000ms — ABOVE the config but BELOW the + // 30_000 compile-time const. Both the decide-time reset AND the + // eviction branch must treat this as healthy. 60_000 lifetimes + // CANNOT detect a const/cfg split — this one can. + let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); + let fake = FakeDriver::healthy(); + let cfg = HubConfig { + max_cycles: 100, + healthy_lifetime_ms: 500, + ..test_cfg(vec![2_000, 10_000]) + }; + let _hub = spawn_hub_with_driver(fake.clone(), rx, cfg); + + // Two fast-fail crashes drain the budget to attempt 2. + tx.send(crash("t1", 1, "claude", Some("cr-1"), 100)) + .unwrap(); + drain().await; + tokio::time::advance(std::time::Duration::from_millis(2_000)).await; + drain().await; + tx.send(crash("t2", 1, "claude", Some("cr-1"), 100)) + .unwrap(); + drain().await; + tokio::time::advance(std::time::Duration::from_millis(10_000)).await; + drain().await; + assert_eq!(fake.respawn_calls().len(), 2); + + // Between-thresholds crash: healthy per CFG (1_000 >= 500) — decide + // must reset to attempt 1, NOT settle retries_exhausted (which the + // 30_000 const would produce). + tx.send(crash("t3", 1, "claude", Some("cr-1"), 1_000)) + .unwrap(); + drain().await; + tokio::time::advance(std::time::Duration::from_millis(2_000)).await; + drain().await; + assert_eq!(fake.respawn_calls().len(), 3); + + // Eviction-branch pin: a between-thresholds SETTLE must reset the + // attempts entry (cfg agreement), so the NEXT fast crash is attempt 1. + fake.set_cap_exhausted(true); + tx.send(crash("t4", 1, "claude", Some("cr-1"), 1_000)) + .unwrap(); + drain().await; + fake.set_cap_exhausted(false); + tx.send(crash("t5", 1, "claude", Some("cr-1"), 100)) + .unwrap(); + drain().await; + tokio::time::advance(std::time::Duration::from_millis(2_000)).await; + drain().await; + assert_eq!( + fake.recovering_calls(), + vec![ + ("t1".into(), 1u32, 2u32), + ("t2".into(), 2u32, 2u32), + ("t3".into(), 1u32, 2u32), + ("t5".into(), 1u32, 2u32), + ], + "t5 must start a fresh budget: the eviction branch reset attempts on t4's settle" + ); + } + + #[tokio::test(start_paused = true)] + async fn user_cancel_during_backoff_aborts_the_respawn_and_settles_loud() { + // Crash schedules a resume; the cancel lands during the backoff. + // The hub must consume the flag, respawn NOTHING, and EMIT the + // settle frame itself (D-4, validated A5): the take_cancel arm is + // loud so a late-consumed or pre-seeded cancel can never strand a + // recovering notice. Idempotent with the WS handler's immediate + // frame — the client folds duplicates. + let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); + let fake = FakeDriver::healthy(); + // Pre-seed the cancel BEFORE the crash event (the flag is checked + // post-sleep, so a pre-seeded flag exercises the late-consume path). + fake.set_cancelled("t1"); + let _hub = spawn_hub_with_driver(fake.clone(), rx, test_cfg(vec![2_000, 10_000])); + + tx.send(crash("t1", 1, "claude", Some("cr-1"), 1_000)) + .unwrap(); + drain().await; + tokio::time::advance(std::time::Duration::from_millis(2_000)).await; + drain().await; + assert!(fake.respawn_calls().is_empty(), "cancel aborts the respawn"); + assert!(fake.claim_calls().is_empty()); + assert_eq!( + fake.settled_frames(), + vec![("t1".to_string(), "auto-resume cancelled".to_string(), None)] + ); + assert_eq!(fake.settled_reasons(), vec!["user_cancelled".to_string()]); + } + + #[tokio::test(start_paused = true)] + async fn a_cancel_for_a_settling_terminal_is_cleaned_up_not_leaked() { + // Fresh-eyes fix: a cancel whose terminal settles WITHOUT reaching + // the post-sleep take_cancel check (here: decide settles on + // cap_exhausted, no Resume arm at all) must still be removed from + // the set — "removed on consumption" has to hold on every path. + let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); + let fake = FakeDriver::healthy(); + fake.set_cap_exhausted(true); + fake.set_cancelled("t1"); + let _hub = spawn_hub_with_driver(fake.clone(), rx, test_cfg(vec![2_000, 10_000])); + + tx.send(crash("t1", 1, "claude", Some("cr-1"), 1_000)) + .unwrap(); + drain().await; + assert_eq!( + fake.settled_frames(), + vec![("t1".to_string(), "respawn_cap_exhausted".to_string(), None)] + ); + assert_eq!( + fake.pending_cancels(), + 0, + "the stale cancel entry must be cleaned up on the settle tail" + ); + } + + #[tokio::test(start_paused = true)] + async fn a_cancel_landing_during_the_respawn_is_cleaned_up_on_the_replaced_tail() { + // Fresh-eyes fix: a cancel that lands AFTER the hub's post-sleep + // take_cancel check (simulated: inserted during the respawn await) + // used to leak in auto_resume_cancels forever. The replaced tail + // must remove it. (It is too late to abort — the resume already + // happened — so cleanup, not abort, is the correct semantics.) + let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); + let fake = FakeDriver::healthy(); + fake.set_insert_cancel_on_respawn(true); + let _hub = spawn_hub_with_driver(fake.clone(), rx, test_cfg(vec![2_000, 10_000])); + + tx.send(crash("t1", 1, "claude", Some("cr-1"), 1_000)) + .unwrap(); + drain().await; + tokio::time::advance(std::time::Duration::from_millis(2_000)).await; + drain().await; + assert_eq!(fake.replaced_calls().len(), 1, "the resume completed"); + assert_eq!( + fake.pending_cancels(), + 0, + "the late cancel entry must be cleaned up on the replaced tail" + ); + } + + #[tokio::test(start_paused = true)] + async fn guard_abort_emits_a_settle_frame() { + // pane_closed guard-abort must broadcast the settle frame so the + // client clears the recovering notice deterministically (znhn #3). + let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); + let fake = FakeDriver::healthy(); + let _hub = spawn_hub_with_driver(fake.clone(), rx, test_cfg(vec![2_000, 10_000])); + + tx.send(crash("t1", 1, "claude", Some("cr-1"), 1_000)) + .unwrap(); + drain().await; + fake.set_guard(Some("pane_closed")); + tokio::time::advance(std::time::Duration::from_millis(2_000)).await; + drain().await; + let settled = fake.settled_frames(); + assert_eq!( + settled, + vec![("t1".to_string(), "pane_closed".to_string(), None)] + ); + } + + #[tokio::test(start_paused = true)] + async fn retries_exhausted_emits_a_settle_frame() { + // Same shape as second_crash_uses_second_delay_then_exhausts: after + // the budget drains, the final crash must broadcast a settle frame. + let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); + let fake = FakeDriver::healthy(); + let _hub = spawn_hub_with_driver(fake.clone(), rx, test_cfg(vec![2_000, 10_000])); + + tx.send(crash("t1", 1, "claude", Some("cr-1"), 1_000)) + .unwrap(); + drain().await; + tokio::time::advance(std::time::Duration::from_millis(2_000)).await; + drain().await; + tx.send(crash("t-new", 1, "claude", Some("cr-1"), 1_000)) + .unwrap(); + drain().await; + tokio::time::advance(std::time::Duration::from_millis(10_000)).await; + drain().await; + assert_eq!(fake.respawn_calls().len(), 2); + + tx.send(crash("t-new2", 1, "claude", Some("cr-1"), 1_000)) + .unwrap(); + drain().await; + let settled = fake.settled_frames(); + assert!( + settled + .iter() + .any(|(t, r, _)| t == "t-new2" && r == "retries_exhausted"), + "exhaustion must be a LOUD settle frame: {settled:?}" + ); } /// Council MEDIUM fix (crusty, 7w4h/xkhx review): a driver panic must not @@ -1333,7 +1975,7 @@ mod tests { let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); let fake = FakeDriver::healthy(); fake.set_panic_next_recovering(true); - let _hub = spawn_hub_with_driver(fake.clone(), rx, vec![10]); + let _hub = spawn_hub_with_driver(fake.clone(), rx, test_cfg(vec![10])); // Event 1: the driver panics mid-processing (inside emit_recovering). tx.send(crash("t1", 1, "claude", Some("cr-1"), 5_000)) diff --git a/crates/freshell-ws/src/codex_association.rs b/crates/freshell-ws/src/codex_association.rs index b017cfdab..fd823095e 100644 --- a/crates/freshell-ws/src/codex_association.rs +++ b/crates/freshell-ws/src/codex_association.rs @@ -279,6 +279,7 @@ mod tests { ), broadcast_tx: StdArc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( StdArc::clone(&auth_token), StdArc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/src/lib.rs b/crates/freshell-ws/src/lib.rs index 52cf6588d..8d857cac5 100644 --- a/crates/freshell-ws/src/lib.rs +++ b/crates/freshell-ws/src/lib.rs @@ -104,6 +104,13 @@ pub struct WsState { /// (Task 5); until then tests drain it directly (construction sites /// without a consumer drop the receiver — sends are best-effort). pub auto_resume_tx: tokio::sync::mpsc::UnboundedSender, + /// Pending user cancels for planned auto-resumes, keyed by the OLD + /// (crashed) terminal id (znhn item 2). Inserted by the WS handler + /// ONLY after registry validation (unknown ids never enter — D-4), + /// consumed by the hub's post-sleep guard, which re-emits the settle + /// frame so a consumed cancel is always loud. Bounded: one + /// registry-known entry per cancel click, removed on consumption. + pub auto_resume_cancels: std::sync::Arc>>, /// The freshcodex WS fresh-agent slice: the post-handshake loop dispatches /// `freshAgent.create` / `freshAgent.send` (codex) here, which spawns the codex /// app-server sidecar and broadcasts `freshAgent.created` / `freshAgent.send.accepted` @@ -773,6 +780,7 @@ mod tests { settings: Arc::new(test_settings()), broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/src/opencode_association.rs b/crates/freshell-ws/src/opencode_association.rs index e2d2b4f50..bdb4be220 100644 --- a/crates/freshell-ws/src/opencode_association.rs +++ b/crates/freshell-ws/src/opencode_association.rs @@ -265,6 +265,7 @@ mod tests { ), broadcast_tx: StdArc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( StdArc::clone(&auth_token), StdArc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/src/terminal.rs b/crates/freshell-ws/src/terminal.rs index d41e24ed0..29553ac25 100644 --- a/crates/freshell-ws/src/terminal.rs +++ b/crates/freshell-ws/src/terminal.rs @@ -60,9 +60,9 @@ use freshell_platform::{ }; use freshell_protocol::{ ClientMessage, ErrorCode, ErrorMsg, Pong, ServerMessage, SessionLocator, Shell, TerminalAttach, - TerminalCreate, TerminalCreated, TerminalIdOnly, TerminalInputBlocked, - TerminalInputBlockedReason, TerminalKill, TerminalMetaRecord, TerminalMetaUpdated, - TerminalResize, + TerminalAutoResumeCancel, TerminalCreate, TerminalCreated, TerminalIdOnly, + TerminalInputBlocked, TerminalInputBlockedReason, TerminalKill, TerminalMetaRecord, + TerminalMetaUpdated, TerminalResize, }; use freshell_terminal::{build_child_env_from_process, FrameSink}; @@ -694,6 +694,10 @@ async fn handle_client_text( handle_detach(&detach.terminal_id, ws_tx, state, conn_id).await } ClientMessage::TerminalKill(kill) => handle_kill(kill, ws_tx, state).await, + ClientMessage::TerminalAutoResumeCancel(cancel) => { + handle_auto_resume_cancel(cancel, state); + true + } // freshAgent.create / freshAgent.send (codex + claude slices): dispatch to the // shared provider state as a DETACHED task so the cold sidecar spawn + the live // turn never block this connection's select loop (which must keep fanning out @@ -2906,18 +2910,14 @@ pub async fn respawn_agent_terminal( // doors. (The per-connection CreateRateLimiter is connection-loop-local // and does not apply here.) // - // The gate's acquire is cancellable via a watch channel (the WS restore - // door wires its per-connection cancel signal; kata enn3). Auto-resume - // is server-initiated with no connection to die, so — like the REST - // door (`terminal_tabs.rs` rest gate) — hold a never-fired sender for - // the acquire's duration; the timeout still bounds the wait. - let (_respawn_cancel_tx, mut respawn_cancel_rx) = tokio::sync::watch::channel(false); + // Uncancellable acquire (kata znhn item 4): auto-resume is + // server-initiated with no connection to die; the timeout still bounds + // the wait. let _spawn_permit = match state .spawn_gate - .acquire( - std::time::Duration::from_millis(state.create_protect.spawn_timeout_ms), - &mut respawn_cancel_rx, - ) + .acquire_uncancellable(std::time::Duration::from_millis( + state.create_protect.spawn_timeout_ms, + )) .await { Ok(permit) => permit, @@ -3844,6 +3844,39 @@ async fn handle_detach( /// live-pinned 2026-07-14 in the kill re-probe, `kill-orig-r16.json`: the invalid /// kill draws an `error` frame on the original; the port previously dropped it /// silently). +/// znhn item 2: flag the pending resume for the hub's post-sleep guard AND +/// settle the client IMMEDIATELY — the notice must clear on click, not +/// after the backoff sleep completes. The id is VALIDATED against the +/// registry first (D-4, kill-handler precedent): an unknown id is ignored +/// with a log — no insert, no broadcast — so the set cannot grow without +/// bound and spoofed ids cannot broadcast settle frames. The hub consumes +/// the flag post-sleep and re-emits the settle frame (idempotent), so a +/// late-consumed cancel is always loud. +fn handle_auto_resume_cancel(cancel: TerminalAutoResumeCancel, state: &WsState) { + if !state.registry.exists(&cancel.terminal_id) { + tracing::info!(terminal_id = %cancel.terminal_id, "terminal.auto_resume.cancel_unknown_id_ignored"); + return; + } + state + .auto_resume_cancels + .lock() + .expect("auto_resume_cancels lock") + .insert(cancel.terminal_id.clone()); + let msg = freshell_protocol::ServerMessage::TerminalStatus(freshell_protocol::TerminalStatus { + status: freshell_protocol::RuntimeStatus::Exited, + terminal_id: cancel.terminal_id.clone(), + attempt: None, + max_attempts: None, + exit_code: None, + reason: Some("auto-resume cancelled".to_string()), + resume_cycles: None, + }); + if let Ok(json) = serde_json::to_string(&msg) { + let _ = state.broadcast_tx.send(json); + } + tracing::info!(terminal_id = %cancel.terminal_id, "terminal.auto_resume.user_cancelled"); +} + async fn handle_kill(kill: TerminalKill, ws_tx: &mut WsSink, state: &WsState) -> bool { if kill_and_broadcast(state, &kill.terminal_id) { // P1.8 trigger (e): explicit user close — best-effort retire of the @@ -4777,6 +4810,7 @@ mod terminals_changed_tests { ), broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), @@ -4869,6 +4903,33 @@ mod terminals_changed_tests { Err(tokio::sync::broadcast::error::TryRecvError::Empty) )); } + + #[tokio::test] + async fn cancel_with_an_unknown_terminal_id_is_ignored_and_the_set_does_not_grow() { + // D-4 (validated A5): unknown id -> log + return. Assert (a) no + // settle frame is broadcast, (b) state.auto_resume_cancels stays + // EMPTY — the set is bounded by registry-known ids, a client cannot + // grow it with spoofed ids or pre-poison a future resume. + let (state, mut rx) = state_with_bus(); + handle_auto_resume_cancel( + TerminalAutoResumeCancel { + terminal_id: "spoofed-id".to_string(), + }, + &state, + ); + assert!(matches!( + rx.try_recv(), + Err(tokio::sync::broadcast::error::TryRecvError::Empty) + )); + assert!( + state + .auto_resume_cancels + .lock() + .expect("auto_resume_cancels lock") + .is_empty(), + "unknown ids must never enter the cancel set" + ); + } } /// DEV-0008 create-time slice (`port/oracle/DEVIATIONS.md`): `terminal.meta.updated` @@ -4984,6 +5045,7 @@ mod terminal_meta_created_tests { ), broadcast_tx: std::sync::Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( std::sync::Arc::clone(&auth_token), std::sync::Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/claude_session_rebind.rs b/crates/freshell-ws/tests/claude_session_rebind.rs index 6e52ba18d..73897eecc 100644 --- a/crates/freshell-ws/tests/claude_session_rebind.rs +++ b/crates/freshell-ws/tests/claude_session_rebind.rs @@ -149,6 +149,7 @@ async fn spawn_server_returning_state( settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/codex_managed_launch_e2e.rs b/crates/freshell-ws/tests/codex_managed_launch_e2e.rs index fb6a01668..e6b8fefbf 100644 --- a/crates/freshell-ws/tests/codex_managed_launch_e2e.rs +++ b/crates/freshell-ws/tests/codex_managed_launch_e2e.rs @@ -125,6 +125,7 @@ async fn spawn_server() -> (String, freshell_terminal::TerminalRegistry) { settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/codex_session_ref_resume.rs b/crates/freshell-ws/tests/codex_session_ref_resume.rs index 0c9b733c5..a957d8885 100644 --- a/crates/freshell-ws/tests/codex_session_ref_resume.rs +++ b/crates/freshell-ws/tests/codex_session_ref_resume.rs @@ -118,6 +118,7 @@ async fn spawn_server() -> (String, freshell_terminal::TerminalRegistry) { settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/common/mod.rs b/crates/freshell-ws/tests/common/mod.rs index f42d13f97..8bb618c0d 100644 --- a/crates/freshell-ws/tests/common/mod.rs +++ b/crates/freshell-ws/tests/common/mod.rs @@ -129,6 +129,7 @@ pub async fn spawn_server_with_specs( settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), @@ -205,6 +206,7 @@ pub async fn spawn_server_with_specs_and_auto_resume_rx( settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), @@ -285,6 +287,7 @@ pub async fn spawn_server_with_specs_and_auto_resume_hub( settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), @@ -362,6 +365,7 @@ pub async fn spawn_server_with_specs_and_state( settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), @@ -447,6 +451,7 @@ pub async fn spawn_server_with_ledger( settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), @@ -528,6 +533,7 @@ pub async fn spawn_server_with_specs_and_activity( settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), @@ -608,6 +614,7 @@ pub async fn spawn_server_with_specs_activity_and_codex_locator( settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), @@ -686,6 +693,7 @@ pub async fn spawn_server_with_create_protect( settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/cross_kind_liveness.rs b/crates/freshell-ws/tests/cross_kind_liveness.rs index 6fcaa6b36..234a51449 100644 --- a/crates/freshell-ws/tests/cross_kind_liveness.rs +++ b/crates/freshell-ws/tests/cross_kind_liveness.rs @@ -253,6 +253,7 @@ async fn spawn_server() -> (String, freshell_terminal::TerminalRegistry) { settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex, fresh_claude, fresh_opencode, diff --git a/crates/freshell-ws/tests/diag01_lifecycle_events.rs b/crates/freshell-ws/tests/diag01_lifecycle_events.rs index bc10d19a2..bf7b41642 100644 --- a/crates/freshell-ws/tests/diag01_lifecycle_events.rs +++ b/crates/freshell-ws/tests/diag01_lifecycle_events.rs @@ -142,6 +142,7 @@ async fn spawn_server(ping_interval_ms: u64) -> String { settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/freshagent_claude_attach.rs b/crates/freshell-ws/tests/freshagent_claude_attach.rs index b28416b0c..ae12f6e4e 100644 --- a/crates/freshell-ws/tests/freshagent_claude_attach.rs +++ b/crates/freshell-ws/tests/freshagent_claude_attach.rs @@ -180,6 +180,7 @@ async fn spawn_server() -> String { settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/freshagent_claude_kill_interrupt.rs b/crates/freshell-ws/tests/freshagent_claude_kill_interrupt.rs index 47610f4a5..0bb6584fe 100644 --- a/crates/freshell-ws/tests/freshagent_claude_kill_interrupt.rs +++ b/crates/freshell-ws/tests/freshagent_claude_kill_interrupt.rs @@ -177,6 +177,7 @@ async fn spawn_server() -> String { settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/freshagent_session_lease.rs b/crates/freshell-ws/tests/freshagent_session_lease.rs index bd02f640f..618f25001 100644 --- a/crates/freshell-ws/tests/freshagent_session_lease.rs +++ b/crates/freshell-ws/tests/freshagent_session_lease.rs @@ -202,6 +202,7 @@ async fn spawn_server() -> String { settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/hello_timeout.rs b/crates/freshell-ws/tests/hello_timeout.rs index 4e0682853..71d14344f 100644 --- a/crates/freshell-ws/tests/hello_timeout.rs +++ b/crates/freshell-ws/tests/hello_timeout.rs @@ -62,6 +62,7 @@ async fn spawn_server(hello_timeout_ms: u64) -> String { settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/keepalive.rs b/crates/freshell-ws/tests/keepalive.rs index 4898e1048..5e5e6ebd6 100644 --- a/crates/freshell-ws/tests/keepalive.rs +++ b/crates/freshell-ws/tests/keepalive.rs @@ -63,6 +63,7 @@ async fn spawn_server( settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/max_payload.rs b/crates/freshell-ws/tests/max_payload.rs index fd908b420..03b6ebac5 100644 --- a/crates/freshell-ws/tests/max_payload.rs +++ b/crates/freshell-ws/tests/max_payload.rs @@ -63,6 +63,7 @@ async fn spawn_server(ws_max_payload_bytes: usize) -> String { settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/opencode_switch_rebind.rs b/crates/freshell-ws/tests/opencode_switch_rebind.rs index 248c24777..1fdf1970a 100644 --- a/crates/freshell-ws/tests/opencode_switch_rebind.rs +++ b/crates/freshell-ws/tests/opencode_switch_rebind.rs @@ -219,6 +219,7 @@ async fn spawn_server_returning_state( settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/origin_policy.rs b/crates/freshell-ws/tests/origin_policy.rs index 72617f473..33ca090d0 100644 --- a/crates/freshell-ws/tests/origin_policy.rs +++ b/crates/freshell-ws/tests/origin_policy.rs @@ -53,6 +53,7 @@ async fn spawn_server(allowed_origins: Vec) -> (String, String) { settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/pane_reconcile.rs b/crates/freshell-ws/tests/pane_reconcile.rs index 8181be531..7016ff45e 100644 --- a/crates/freshell-ws/tests/pane_reconcile.rs +++ b/crates/freshell-ws/tests/pane_reconcile.rs @@ -131,6 +131,7 @@ async fn spawn_server_with_probe( settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/pane_reconcile_freshagent.rs b/crates/freshell-ws/tests/pane_reconcile_freshagent.rs index 91efcbeb4..e374b834c 100644 --- a/crates/freshell-ws/tests/pane_reconcile_freshagent.rs +++ b/crates/freshell-ws/tests/pane_reconcile_freshagent.rs @@ -205,6 +205,7 @@ async fn spawn_server_with_probe(probe: Arc) -> Server { settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/rest_ws_shared_gate.rs b/crates/freshell-ws/tests/rest_ws_shared_gate.rs index 985717d7a..e928ad58b 100644 --- a/crates/freshell-ws/tests/rest_ws_shared_gate.rs +++ b/crates/freshell-ws/tests/rest_ws_shared_gate.rs @@ -117,6 +117,7 @@ async fn spawn_combined_server( settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/restore_spawn_gate.rs b/crates/freshell-ws/tests/restore_spawn_gate.rs index 8d70bedcf..e2f155098 100644 --- a/crates/freshell-ws/tests/restore_spawn_gate.rs +++ b/crates/freshell-ws/tests/restore_spawn_gate.rs @@ -102,6 +102,7 @@ async fn spawn_server( settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/safe08_restore_diagnostics.rs b/crates/freshell-ws/tests/safe08_restore_diagnostics.rs index eb3af8b05..ef7820294 100644 --- a/crates/freshell-ws/tests/safe08_restore_diagnostics.rs +++ b/crates/freshell-ws/tests/safe08_restore_diagnostics.rs @@ -149,6 +149,7 @@ async fn spawn_server() -> String { settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/crates/freshell-ws/tests/term09_output_queue.rs b/crates/freshell-ws/tests/term09_output_queue.rs index 3d593c88c..e5bb29c22 100644 --- a/crates/freshell-ws/tests/term09_output_queue.rs +++ b/crates/freshell-ws/tests/term09_output_queue.rs @@ -56,6 +56,7 @@ async fn spawn_server(term09: Term09Config) -> String { settings, broadcast_tx: Arc::clone(&broadcast_tx), auto_resume_tx: tokio::sync::mpsc::unbounded_channel().0, + auto_resume_cancels: Default::default(), fresh_codex: freshell_freshagent::FreshCodexState::new( Arc::clone(&auth_token), Arc::clone(&broadcast_tx), diff --git a/docs/plans/2026-07-27-agent-crash-resilience.md b/docs/plans/2026-07-27-agent-crash-resilience.md index 279c7cf65..2aa79551a 100644 --- a/docs/plans/2026-07-27-agent-crash-resilience.md +++ b/docs/plans/2026-07-27-agent-crash-resilience.md @@ -65,6 +65,7 @@ The respawned terminal gets a NEW terminalId (`Uuid::new_v4()` per create, `crat ### D-5. Retry budget semantics - Schedule `AUTO_RESUME_DELAYS_MS = [2_000, 10_000]` (2 retries max), shaped after the repo's bounded-retry exemplar (`activity.rs:80-88` `lane_retry_delay_ms`: index = attempts-so-far, `None` = exhausted-and-loud). Env override `FRESHELL_AUTO_RESUME_DELAYS_MS="2000,10000"` (tests set `"50,100"`). +- Patience-window honesty (council 7w4h/xkhx follow-up): the total patience window is ~12s of backoff (2s + 10s) plus spawn time — outage-class causes (provider down, expired auth) will exhaust the budget and settle loudly. By design: auto-resume survives crashes, not outages. - Attempts are counted per `createRequestId` in the orchestrator, **reset when the crashed generation lived ≥ 30s** (mirrors `DEFAULT_RESPAWN_LIVENESS_WINDOW_MS` — a healthy resume is not penalized; tomorrow's crash of an overnight pane starts at attempt 1). - The registry's respawn-generation cap (`respawn_exhausted`, cap 3/30s — mutated by every natural exit in `finish_pty_exit`) is consulted as an **outer guard**, composing with client-driven reconcile respawns: whoever exhausts generations first, the pane converges to `exited`. - Guards before each respawn (all post-sleep): D7 live-session (`registry.live_terminal_for_session_ref` — never a second `--resume ` writer), sessionRef lease (`claim_session_ref` — never race a concurrent client create; VERIFIED: the lease is a registry-owned map keyed `provider\0sessionId`, connection-independent, and the identical object both the WS create ingress `terminal.rs:1149` and REST ingress contend on), and **binding-still-Bound** (re-check `pane_ledger.bound_session_ref_for_terminal` returns a live binding — a user who closed the pane during the backoff retires it via `retire_closed`, `terminal.rs:2716-2730`; if retired, settle `pane_closed`; this also bounds the crash-microseconds-before-kill race). diff --git a/docs/plans/2026-07-27-rest-spawn-gate.md b/docs/plans/2026-07-27-rest-spawn-gate.md index ad3a0a8d4..4ad11368b 100644 --- a/docs/plans/2026-07-27-rest-spawn-gate.md +++ b/docs/plans/2026-07-27-rest-spawn-gate.md @@ -112,6 +112,8 @@ documented here, and MUST be revisited if/when S5 flips the default ON (likely a separate sidecar budget covering both doors). Task 9's report carries this forward. +> Tripwire (added 2026-07-29, kata bccd item 5): grep `D-C-REVISIT(FRESHELL_CODEX_MANAGED_LAUNCH)` — marker comments sit at the REST call site (`terminal_tabs.rs`) and on the flag const (`launch_plan.rs`) so the default flip cannot ship without hitting this decision. + **D-D. Codex sidecar evaluation.** (The kata has NO numbered items — its sidecar mention is the un-numbered aside *"Also noted: the codex-sidecar launch path bypasses the gate similarly"*, and it is NOT in the kata's diff --git a/docs/plans/2026-07-29-znhn-bccd-followups.md b/docs/plans/2026-07-29-znhn-bccd-followups.md new file mode 100644 index 000000000..4087ab3cd --- /dev/null +++ b/docs/plans/2026-07-29-znhn-bccd-followups.md @@ -0,0 +1,1974 @@ +# Agent Auto-Resume & REST Spawn-Gate Council Follow-Ups (katas znhn + bccd) — Implementation Plan + +> **For agentic workers:** This plan is executed task-by-task by the +> workflow's execute stage: a fresh implementer per task, with a spec + +> quality review after each task. Steps use checkbox (`- [ ]`) syntax +> for tracking. + +**Goal:** Implement both council follow-up katas as one batch — kata `znhn` (persistent crash trace, flap-loop circuit breaker + cancel affordance, settle frames for guard-aborted auto-resumes, uncancellable spawn-gate acquire, Relaunch copy) and kata `bccd` (Retry-After on the 429, deterministic burst-test pin, cancel-sender pin superseded by construction, opencode sidecar cold-start gating decision (evaluate-and-decide — decided: do NOT gate, D-7), D-C revisit tripwire). + +**Architecture:** Server-side, the auto-resume hub (`crates/freshell-ws/src/auto_resume.rs`) gains a settle frame (a new `RuntimeStatus::Exited` on the existing `terminal.status` broadcast) emitted on every silent settle path, a cross-reset flap circuit breaker (rolling-window cycle counter in the hub's attempts map), and a user-cancel door (new client message `terminal.autoResumeCancel`). Client-side, the 30s notice-TTL guessing apparatus is deleted (frames are now deterministic), the ephemeral "resumed" strip is replaced by a **persistent, dismissible crash trace stored on pane content** (persists automatically — pane-content persistence is a denylist), and the exit banner gains cancel/dismiss affordances plus honest copy. The spawn gate gains `acquire_uncancellable` (the dummy-channel wart moves into the gate), the REST 429 gains `Retry-After`, and the opencode sidecar cold-start stays deliberately UNGATED (D-7 reversed at validation — the single-flighted singleton bounds the fork; gating would starve the budget; recorded in code comments at both doors). + +**Tech Stack:** Rust (axum, tokio, serde; crates `freshell-ws`, `freshell-freshagent`, `freshell-protocol`, `freshell-terminal`, `freshell-server`, `freshell-codex`), React + Redux Toolkit + TypeScript client, Vitest unit tests, Playwright e2e (`rust-chromium` project), frozen WS contract (`port/contract/*.json` + Rust inventory pins). + +## Global Constraints + +- Worktree: `/home/dan/code/freshell/.worktrees/znhn-bccd-followups`, branch `feat/znhn-bccd-followups`, based on `origin/main` @ `d2388a09` or newer. All paths below are relative to the worktree root. +- **Frozen contract rule:** any change to `shared/ws-protocol.ts` or `crates/freshell-protocol/src/*` requires `npm run contract:generate` and committing the regenerated `port/contract/ws-protocol.schema.json`, `port/contract/ws-server-messages.schema.json`, `port/contract/ws-message-inventory.json` **plus** the Rust pins (`CLIENT_MESSAGE_TYPES`/`SERVER_MESSAGE_TYPES` arrays and the hardcoded counts in `crates/freshell-protocol/tests/inventory.rs`) **in the same commit**. `npm run test:port` must be green. Additive changes do NOT bump `WS_PROTOCOL_VERSION` (precedent: commits `60bfdcad`, `eef9b344` — version stayed at 7). +- The `reason` field on notice frames is presentational prose and must NEVER be parsed by the client — all client rendering reads typed fields (pinned by `test/unit/client/components/TerminalView.exitBanner.test.tsx:393-425`). +- The spawn-gate tracing target stays the literal string `"freshell_ws::spawn_gate"` (e2e log greps depend on it). +- Ports: e2e servers use kernel-ephemeral ports (`RustServer` helper) — **NEVER 3001/3002**. The user's LIVE server runs on 3002: never restart it, never use broad kill patterns (`pkill -f freshell` etc. is forbidden), no synthetic load on this shared host. +- A11y: real ` + + ) + } +``` +(The `verb` ternary disappears here — Task 10 retires the `'resumed'` kind; until Task 10 lands, keep the ternary if any test still exercises `'resumed'`.) + +- [ ] **Step 5: Run to verify pass** + +Run: `npx vitest run test/unit/client/components/TerminalView.exitBanner.test.tsx test/unit/client/store/terminalLifecycleSlice.test.ts test/unit/client/components/TerminalExitBanner.test.tsx && npm run lint 2>&1 | tail -3` +Expected: PASS, lint clean. + +- [ ] **Step 6: Commit** + +```bash +git add src/store/terminalLifecycleSlice.ts src/components/TerminalView.tsx src/components/TerminalExitBanner.tsx test/unit/client +git commit -m "feat(client): frame-driven auto-resume notices — delete the 30s TTL, add cancel (znhn#2,#3,#6)" +``` + +--- + +### Task 10: Persistent crash trace on pane content (znhn 1) + +**Files:** +- Modify: `src/store/paneTypes.ts:71-103` (`TerminalPaneContent`) +- Modify: `src/store/panesSlice.ts` (reducers near `setPaneReconcileNotice`/`clearPaneReconcileNotice` at `:2080-2096`, exports at `:2236-2237`) +- Modify: `src/store/terminalLifecycleSlice.ts` (`foldTerminalReplacement`) +- Modify: `src/components/TerminalView.tsx` (`terminal.replaced` handler `:4392-4423`, `showExitBanner` `:5209-5215`, banner mount) +- Modify: `src/components/TerminalExitBanner.tsx` +- Modify: `test/unit/client/store/panesPersistence.test.ts`, `test/unit/client/components/TerminalView.exitBanner.test.tsx`, `test/unit/client/components/TerminalExitBanner.test.tsx`, `test/unit/client/store/terminalLifecycleSlice.test.ts` +- Modify: `docs/plans/2026-07-27-agent-crash-resilience.md` (§D-5, one sentence) + +**Interfaces:** +- Consumes: `terminal.replaced` frame (existing); denylist persistence (`stripTransientSessionFields` — do NOT add `crashTrace` there); `findReconcileTerminalContent(state, tabId, paneId)` traversal helper (module-private, `panesSlice.ts:618`) — the `TerminalPaneContent`-narrowed sibling of the `findReconcilePaneContent(state, tabId, paneId)` helper (`:631`) that the reconcile-notice reducers use; both take positional `(state, tabId, paneId)`. +- Produces (used by Tasks 11–12): `export type CrashTrace = { exitCode: number; resumedAtMs: number }` in `paneTypes.ts`; `TerminalPaneContent.crashTrace?: CrashTrace`; actions `setPaneCrashTrace({ tabId, paneId, crashTrace })` and `clearPaneCrashTrace({ tabId, paneId })` (payloads carry `tabId` because the panes-tree traversal helpers are keyed `(state, tabId, paneId)`, exactly like `setPaneReconcileNotice`/`clearPaneReconcileNotice`); banner props `crashTrace: CrashTrace | null`, `onDismissCrashTrace: () => void`; trace UI = `role="status"` + `data-testid="crash-trace"`, copy `"{mode} crashed (exit {N}) & auto-resumed at {HH:MM}"`, dismiss button aria-label `` `Dismiss ${mode} crash notice` ``. + +- [ ] **Step 1: Write the failing tests** — + +`panesPersistence.test.ts` (follow the file's existing round-trip idiom at `:207`/`:362`): + +```ts + it('crashTrace persists across a panes round-trip (denylist keeps new fields)', () => { + // seed a terminal pane whose content includes + // crashTrace: { exitCode: 1, resumedAtMs: 1_753_760_220_000 } + // run the same persist -> load cycle as the durable-identity test at :362 + // expect the loaded pane content to still carry the exact crashTrace + }) +``` + +`TerminalView.exitBanner.test.tsx`: + +```tsx + it('terminal.replaced writes a persistent crash trace onto pane content and shows the trace strip', async () => { + // seed: recovering notice for 'term-crashed', pane status 'running' + await act(async () => { + messageHandler!({ type: 'terminal.replaced', oldTerminalId: 'term-crashed', newTerminalId: 'term-new', exitCode: 1, attempt: 1, maxAttempts: 2 }) + }) + const trace = screen.getByTestId('crash-trace') + expect(trace).toHaveTextContent(/claude crashed \(exit 1\) & auto-resumed at \d{2}:\d{2}/) + expect(trace).toHaveAttribute('role', 'status') + expect(screen.queryByRole('alert')).toBeNull() + // and the store now carries it on pane CONTENT (persisted home): + // walk store.getState().panes... leaf content.crashTrace === { exitCode: 1, resumedAtMs: } + }) + + it('dismissing the crash trace clears it from pane content', async () => { + // seed pane content with crashTrace directly via makeStore + fireEvent.click(screen.getByRole('button', { name: 'Dismiss claude crash notice' })) + expect(screen.queryByTestId('crash-trace')).toBeNull() + }) +``` + +`terminalLifecycleSlice.test.ts`: + +```ts + it('foldTerminalReplacement clears the notice (the persistent crash trace replaces the resumed strip)', () => { + // seed entry with a recovering notice; dispatch foldTerminalReplacement; + // expect entry.notice undefined, entry.exit undefined, lastTerminalId advanced + }) + it('a replacement clears prior settle state (stale resumeCycles cannot leak into a later crash banner)', () => { + // seed entry.settle = { resumeCycles: 5, at: 1 }; dispatch + // foldTerminalReplacement; expect entry.settle undefined — pairs with + // Task 9's recordTerminalExit pin (validated A15: nothing else ever + // deletes the settle state, and the REST-door relaunch/reconcile never + // advances lastTerminalId). + }) +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `npx vitest run test/unit/client/store/panesPersistence.test.ts test/unit/client/components/TerminalView.exitBanner.test.tsx test/unit/client/store/terminalLifecycleSlice.test.ts` +Expected: FAIL. + +- [ ] **Step 3: Implement** — + +`paneTypes.ts` (above `TerminalPaneContent`): + +```ts +/** Persistent crash trace (kata znhn item 1): "crashed & auto-resumed" — + * survives reload (pane-content persistence is a denylist) until the user + * dismisses it or the pane closes. */ +export type CrashTrace = { + /** Exit code of the crashed generation. */ + exitCode: number + /** Wall-clock ms when the auto-resume succeeded. */ + resumedAtMs: number +} +``` +Field on `TerminalPaneContent` (after `reconcileEpoch`): + +```ts + /** znhn item 1: persisted deliberately — do NOT add to + * stripTransientSessionFields. Absent on old layouts = no trace. */ + crashTrace?: CrashTrace +``` + +`panesSlice.ts` (mirror the payload and call shape of `setPaneReconcileNotice`/`clearPaneReconcileNotice` at `:2080-2096` — `{ tabId, paneId }` payloads, positional `(state, action.payload.tabId, action.payload.paneId)` call. Those reducers use `findReconcilePaneContent` (`:631`); use its `TerminalPaneContent`-narrowed sibling `findReconcileTerminalContent` (`:618`, same positional signature) because `crashTrace` exists only on `TerminalPaneContent`): + +```ts + // znhn item 1: persistent crash trace — written on terminal.replaced, + // cleared only by user dismissal (pane close deletes the pane node). + setPaneCrashTrace( + state, + action: PayloadAction<{ tabId: string; paneId: string; crashTrace: CrashTrace }> + ) { + const content = findReconcileTerminalContent(state, action.payload.tabId, action.payload.paneId) + if (content) content.crashTrace = action.payload.crashTrace + }, + clearPaneCrashTrace(state, action: PayloadAction<{ tabId: string; paneId: string }>) { + const content = findReconcileTerminalContent(state, action.payload.tabId, action.payload.paneId) + if (content && content.crashTrace) delete content.crashTrace + }, +``` +Export both actions at `:2236-2237` alongside the others; import `CrashTrace` from `./paneTypes`. + +`terminalLifecycleSlice.ts` — in `foldTerminalReplacement`, replace the `kind: 'resumed'` notice assignment with `delete e.notice` (keep the `delete e.exit` + `lastTerminalId` advance), and **also `delete e.settle`** (stale-settle leak fix, validated A15: the REST-door relaunch/reconcile never clears/advances `lastTerminalId` and nothing else deletes the entry's `settle`, so without this a stale breaker `resumeCycles` could leak into a LATER crash's alert copy). Narrow `AutoResumeNotice.kind` to `'recovering'` and update any test-harness types that referenced `'resumed'`. + +`TerminalView.tsx` — in the `terminal.replaced` handler (`:4392-4423`), after `foldTerminalReplacement(...)`: + +```tsx + dispatch( + setPaneCrashTrace({ + tabId, + paneId: paneIdRef.current, + crashTrace: { exitCode: msg.exitCode, resumedAtMs: Date.now() }, + }) + ) +``` +(`tabId` is a `TerminalView` prop, in scope everywhere; this same handler already passes it to `applyReconcileAttach({ tabId, paneId: paneIdRef.current, ... })` at `:4415-4420`.) +`showExitBanner` (`:5209-5215`) gains the trace condition: + +```tsx + const showExitBanner = Boolean( + isAgentPane && ( + activeNotice || + terminalContent.crashTrace || + (terminalContent.status === 'exited' && (exitRecord ? exitRecord.exitCode !== 0 : true)) || + (terminalContent.status === 'error' && exitRecord && exitRecord.exitCode !== 0) + ) + ) +``` +Banner mount gains: + +```tsx + crashTrace={terminalContent.crashTrace ?? null} + settledDead={ + (terminalContent.status === 'exited' && (exitRecord ? exitRecord.exitCode !== 0 : true)) || + (terminalContent.status === 'error' && Boolean(exitRecord && exitRecord.exitCode !== 0)) + } + onDismissCrashTrace={() => dispatch(clearPaneCrashTrace({ tabId, paneId }))} +``` +(Hoist the two settled-dead sub-expressions into a `const settledDead = ...` used by both `showExitBanner` and the prop — DRY.) + +`TerminalExitBanner.tsx` — new props + third branch (precedence: notice → alert → trace): + +```tsx +import type { AutoResumeNotice } from '../store/terminalLifecycleSlice' +import type { CrashTrace } from '../store/paneTypes' + +export interface TerminalExitBannerProps { + mode: string + exitCode: number | null + notice: AutoResumeNotice | null + crashTrace: CrashTrace | null + settledDead: boolean + onRelaunch: () => void + onCancelAutoResume: () => void + onDismissCrashTrace: () => void +} +``` +After the notice branch, wrap the existing alert in `if (settledDead) { ...existing alert JSX... }`, then: + +```tsx + if (crashTrace) { + const d = new Date(crashTrace.resumedAtMs) + const hh = String(d.getHours()).padStart(2, '0') + const mm = String(d.getMinutes()).padStart(2, '0') + return ( +
+ + {mode} crashed (exit {crashTrace.exitCode}) & auto-resumed at {hh}:{mm} + + +
+ ) + } + return null +``` + +`docs/plans/2026-07-27-agent-crash-resilience.md` — append one bullet to §D-5 (after the schedule bullet at `:67`): + +```markdown +- Patience-window honesty (council 7w4h/xkhx follow-up): the total patience window is ~12s of backoff (2s + 10s) plus spawn time — outage-class causes (provider down, expired auth) will exhaust the budget and settle loudly. By design: auto-resume survives crashes, not outages. +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `npx vitest run test/unit/client && npm run lint 2>&1 | tail -3` +Expected: PASS (fix any collateral in `TerminalExitBanner.test.tsx` — the pure-component tests now need the new required props; add `crashTrace: null, settledDead: true/false, onCancelAutoResume: () => {}, onDismissCrashTrace: () => {}` to their prop fixtures). + +- [ ] **Step 5: Commit** + +```bash +git add src/store src/components docs/plans/2026-07-27-agent-crash-resilience.md test/unit/client +git commit -m "feat(client): persistent dismissible crash trace on pane content (znhn#1)" +``` + +--- + +### Task 11: Honest banner copy — Relaunch + circuit-breaker (znhn 5 + znhn 2 client tail) + +**Files:** +- Modify: `src/components/TerminalExitBanner.tsx` (alert branch) +- Modify: `src/components/TerminalView.tsx` (banner mount props) +- Modify: `test/unit/client/components/TerminalExitBanner.test.tsx`, `test/unit/client/components/TerminalView.exitBanner.test.tsx` + +**Interfaces:** +- Consumes: `selectResumeCycles(root, paneId)` (Task 9); `resetPaneForReconcileCreate`'s provider-match rule (`panesSlice.ts:1960-1976` — Relaunch resumes the same conversation ONLY when `sessionRef.provider === content.mode`, else it loudly degrades to fresh). +- Produces: banner props `resumeCycles: number | null`, `canResume: boolean`; alert copy `"{mode} crashed {N} times — auto-resume paused"` when `resumeCycles != null`; button text `"Relaunch — resumes this conversation"` when `canResume`, plain `"Relaunch"` otherwise; aria-label stays `` `Relaunch ${mode} session` `` (e2e locators depend on it). + +- [ ] **Step 1: Write the failing tests** (`TerminalExitBanner.test.tsx`): + +```tsx + it('says the relaunch resumes the same conversation when the sessionRef matches', () => { + render() + const btn = screen.getByRole('button', { name: 'Relaunch claude session' }) + expect(btn).toHaveTextContent('Relaunch — resumes this conversation') + }) + + it('keeps plain Relaunch copy when no matching sessionRef exists (degrades to fresh)', () => { + // canResume={false} → text exactly 'Relaunch' + }) + + it('renders the circuit-breaker banner from the typed resumeCycles field', () => { + // resumeCycles={5} → alert text 'claude crashed 5 times — auto-resume paused' + }) +``` +And in `TerminalView.exitBanner.test.tsx`: a settle frame with `resumeCycles: 3` followed by the alert asserting `'claude crashed 3 times — auto-resume paused'`; plus a case asserting `canResume` derives from the seeded `sessionRef` (`withSessionRef: true` in the harness's `makeStore`). + +- [ ] **Step 2: Run to verify failure** + +Run: `npx vitest run test/unit/client/components/TerminalExitBanner.test.tsx test/unit/client/components/TerminalView.exitBanner.test.tsx` +Expected: FAIL. + +- [ ] **Step 3: Implement** — `TerminalExitBanner.tsx` props gain `resumeCycles: number | null` and `canResume: boolean`; the alert branch becomes: + +```tsx + if (settledDead) { + return ( +
+ + {resumeCycles != null + ? `${mode} crashed ${resumeCycles} times — auto-resume paused` + : `process exited${exitCode !== null ? ` (code ${exitCode})` : ''}`} + + +
+ ) + } +``` +`TerminalView.tsx` mount: + +```tsx + resumeCycles={useAppSelectorValue /* see below */} + canResume={Boolean( + terminalContent.sessionRef && terminalContent.sessionRef.provider === terminalContent.mode + )} +``` +where the cycles value comes from a top-level `const resumeCycles = useAppSelector((s) => selectResumeCycles(s, paneId)) ?? null` next to the existing `exitRecord`/`activeNotice` selectors (hooks stay top-level — never inline in JSX). + +- [ ] **Step 4: Run to verify pass + lint** + +Run: `npx vitest run test/unit/client && npm run lint 2>&1 | tail -3` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add src/components test/unit/client +git commit -m "feat(client): honest Relaunch copy + circuit-breaker banner (znhn#5, znhn#2)" +``` + +--- + +### Task 12: E2E — crash trace survives reload, breaker banner, cancel clears immediately + +**Files:** +- Modify: `test/e2e-browser/fixtures/fake-crashing-claude-cli.mjs` (flap mode) +- Modify: `test/e2e-browser/specs/agent-crash-autoresume-rust.spec.ts` (3 new tests; adjust any copy assertions the earlier tasks changed) + +**Interfaces:** +- Consumes: `bootRig(prefix, behaviorEnv)` (`spec:144-160` — owns a `RustServer` on an ephemeral port, installs the fake CLI, seeds `FRESHELL_AUTO_RESUME_DELAYS_MS: '100,200'` which `behaviorEnv` may override), `createClaudePane`, `autoResumeNotice(page)` (`spec:177`), `readArgvLog`, `teardownRig`, `connect(page, info)`; env knobs from Task 7; the reload-flush gotcha (persist is 500ms-debounced; `page.reload()` fires `pagehide` which flushes). +- Produces: green e2e coverage for the three headline behaviors; fixture env `FAKE_CRASH_LIVE_MS`. + +- [ ] **Step 1: Fixture flap mode** — in `fake-crashing-claude-cli.mjs`, in the `always` behavior branch, honor a new env: + +```js +// FAKE_CRASH_LIVE_MS=N — with FAKE_CRASH_MODE=always: stay alive N ms, then +// exit 1 (a "healthy flap": long enough to reset the retry budget when the +// server's healthy-lifetime knob is shrunk below N). +const liveMs = Number(process.env.FAKE_CRASH_LIVE_MS || '0') +if (liveMs > 0) { + setTimeout(() => process.exit(1), liveMs) + // keep the event loop alive exactly like the SURVIVE path does +} else { + process.exit(1) +} +``` +(Splice into the fixture's existing structure — reuse its existing "stay alive" mechanism from the `once`/SURVIVE path rather than inventing a new one.) + +- [ ] **Step 2: Write the three tests** (append to the `test.describe` in `agent-crash-autoresume-rust.spec.ts`, following the rig/teardown pattern of the existing four): + +```ts + test('a persistent crash trace survives reload and is dismissible', async ({ page, e2eServerKind }) => { + expect(e2eServerKind).toBe('rust') + test.setTimeout(240_000) + let rig: Rig | undefined + try { + rig = await bootRig('trace', { FAKE_CRASH_MODE: 'once' }) + await createClaudePane(page, rig.info) + + const trace = page.getByTestId('crash-trace') + await expect(trace).toBeVisible({ timeout: 30_000 }) + await expect(trace).toHaveText(/claude crashed \(exit 1\) & auto-resumed at \d{2}:\d{2}/) + await expect(page.getByRole('alert')).toHaveCount(0) + + // The morning-user scenario: the trace survives a reload. + await page.reload() + await connect(page, rig.info) + await expect(page.getByTestId('crash-trace')).toBeVisible({ timeout: 30_000 }) + + // Dismiss → gone, and STAYS gone across another reload. + await page.getByRole('button', { name: 'Dismiss claude crash notice' }).click() + await expect(page.getByTestId('crash-trace')).toHaveCount(0) + await page.reload() + await connect(page, rig.info) + await expect(page.locator('.xterm').first()).toBeVisible({ timeout: 30_000 }) + await expect(page.getByTestId('crash-trace')).toHaveCount(0) + } finally { + await teardownRig(rig) + } + }) + + test('a flap loop trips the circuit breaker: settles with the crashed-N-times banner', async ({ page, e2eServerKind }) => { + expect(e2eServerKind).toBe('rust') + test.setTimeout(240_000) + let rig: Rig | undefined + try { + rig = await bootRig('flap', { + FAKE_CRASH_MODE: 'always', + FAKE_CRASH_LIVE_MS: '1000', + FRESHELL_AUTO_RESUME_DELAYS_MS: '100,200', + // Each 1s generation counts as "healthy" (budget resets — the + // forever-loop precondition) and stays under the registry window so + // the generation cap never preempts the breaker. + FRESHELL_AUTO_RESUME_HEALTHY_LIFETIME_MS: '500', + FRESHELL_RESPAWN_LIVENESS_WINDOW_MS: '500', + FRESHELL_AUTO_RESUME_MAX_CYCLES: '3', + }) + await createClaudePane(page, rig.info) + + const alert = page.getByRole('alert').filter({ hasText: 'claude crashed 3 times — auto-resume paused' }) + await expect(alert).toBeVisible({ timeout: 60_000 }) + + // Bounded: 1 original + 3 auto-resumes, then nothing more. + await expect(async () => { + expect((await readArgvLog(rig!.argvLog)).length).toBe(4) + }).toPass({ timeout: 15_000 }) + await page.waitForTimeout(3_000) + expect((await readArgvLog(rig.argvLog)).length, 'breaker must stay open').toBe(4) + await expect(page.getByRole('button', { name: 'Relaunch claude session' })).toBeVisible() + } finally { + await teardownRig(rig) + } + }) + + test('cancel clears the recovering notice immediately and no respawn happens', async ({ page, e2eServerKind }) => { + expect(e2eServerKind).toBe('rust') + test.setTimeout(240_000) + let rig: Rig | undefined + try { + // Long backoff = a wide window where the OLD behavior would have lied + // for 30s (znhn#3) and no window at all for the alert bar (znhn#6). + rig = await bootRig('cancel', { FAKE_CRASH_MODE: 'always', FRESHELL_AUTO_RESUME_DELAYS_MS: '8000,8000' }) + await createClaudePane(page, rig.info) + + await expect(autoResumeNotice(page)).toBeVisible({ timeout: 30_000 }) + await page.getByRole('button', { name: 'Cancel auto-resume for claude' }).click() + + // Settle frame, not TTL: the notice clears within seconds, the loud + // alert takes its place. + await expect(autoResumeNotice(page)).toHaveCount(0, { timeout: 3_000 }) + await expect(page.getByRole('alert').filter({ hasText: 'process exited (code 1)' })).toBeVisible({ timeout: 5_000 }) + + // The planned respawn was guard-aborted: still only 1 invocation. + await page.waitForTimeout(10_000) + expect((await readArgvLog(rig.argvLog)).length, 'cancel must abort the planned respawn').toBe(1) + } finally { + await teardownRig(rig) + } + }) +``` + +Margin note for the flap test (deferred assumption A7): the breaker-banner text assertion is the primary proof. If the argv-count assertion (`length == 4`) proves flaky on CI, raise `FAKE_CRASH_LIVE_MS` (e.g. 1000 → 2000) keeping `FRESHELL_AUTO_RESUME_MAX_CYCLES=3` — tune the knobs, never weaken the assertions. + +- [ ] **Step 3: Reconcile the four existing tests with the new UI** — run the whole spec and fix assertions that the feature legitimately changed (expected: the `once` test's resumed-strip expectations now match the crash trace via the `/auto-resum/` status filter; the Relaunch test's button locator is by aria-label and unchanged; alert-count-0 assertions hold because the trace is `role="status"`). Do NOT weaken assertions — update copy expectations only where this plan changed the copy. + +- [ ] **Step 4: Run** + +```bash +cargo build --release -p freshell-server +npx playwright test --config test/e2e-browser/playwright.config.ts --project=rust-chromium test/e2e-browser/specs/agent-crash-autoresume-rust.spec.ts +``` +Expected: all 7 tests PASS (4 existing + 3 new), each on its own ephemeral-port server. + +- [ ] **Step 5: Commit** + +```bash +git add test/e2e-browser +git commit -m "test(e2e): crash trace survives reload; breaker banner; cancel clears immediately (znhn#1,#2,#3)" +``` + +--- + +### Task 13: Full gates, kata comments, push (NO PR) + +**Files:** none beyond incidental fixes surfaced by the gates. + +**Interfaces:** +- Consumes: everything above. +- Produces: a pushed branch `feat/znhn-bccd-followups`; kata comments recording the decisions; NO PR. + +- [ ] **Step 1: Rust gates** + +```bash +cargo fmt --check +cargo clippy --workspace --all-targets -- -D warnings +cargo test --workspace +``` +Expected: all PASS. + +- [ ] **Step 2: Node gates (coordinator-aware)** + +```bash +npm run test:status # if the gate is HELD, wait and re-check — do not bypass +FRESHELL_TEST_SUMMARY='znhn+bccd follow-ups' env -u FRESHELL_BIND_HOST npm test +npm run test:port +npm run lint +``` +Expected: all PASS. + +- [ ] **Step 3: Release build + e2e lane (ephemeral ports only)** + +```bash +cargo build --release -p freshell-server +npx playwright test --config test/e2e-browser/playwright.config.ts --project=rust-chromium \ + test/e2e-browser/specs/agent-crash-autoresume-rust.spec.ts \ + test/e2e-browser/specs/rest-spawn-gate-rust.spec.ts \ + test/e2e-browser/specs/restore-contract-wall-rust.spec.ts +``` +Expected: all PASS; the restore-contract-wall must stay green with ZERO `test.fail()` pins (it currently has zero — closed by PR #577; do not add any). + +- [ ] **Step 4: Kata comments (decision record)** + +```bash +kata comment znhn -m "Follow-ups landed on feat/znhn-bccd-followups: (1) persistent crash trace on pane content — survives reload, dismissible, role=status data-testid=crash-trace; patience-window sentence added to crash-resilience plan D-5. (2) flap circuit breaker: 5 successful auto-resumes per createRequestId per rolling hour (FRESHELL_AUTO_RESUME_MAX_CYCLES / _CYCLE_WINDOW_MS), settles 'flap_circuit_breaker' with typed resumeCycles + 'crashed N times — auto-resume paused' banner; cancel button on the recovering notice sends terminal.autoResumeCancel (settle frame emitted immediately). Escalating backoff rejected: the hub is serialized, long sleeps would starve other panes. (3) settle frame = RuntimeStatus 'exited' on terminal.status, emitted on EVERY silent settle path; client 30s TTL apparatus deleted. (4) SpawnGate::acquire_uncancellable added, both dummy-channel callers migrated. (5) Relaunch copy: 'Relaunch — resumes this conversation' when sessionRef.provider matches (copy stays plain 'Relaunch' on the provider-mismatch degrade path). (6) orphan race resolved structurally: with the TTL gone the alert bar cannot appear while a resume is pending at ANY delay value — pinned by the no-timer-degradation unit test; the in-flight window is covered by the session_owned_live guard which now also emits a settle frame." + +kata comment bccd -m "Follow-ups landed on feat/znhn-bccd-followups: (1) 429 SPAWN_QUEUE_FULL now carries Retry-After header + retryAfterMs body field (value = gate wait bound, default 10s). (2) burst test deflaked: test pre-holds the single permit, so queued_total()==16 exactly and zero requests may complete while the budget is held — pins max-in-flight <= budget deterministically. (3) cancel-sender pin superseded BY CONSTRUCTION: acquire_uncancellable (znhn#4) owns the never-fired sender inside the gate, so no caller-side sender exists to drop; semantics pinned by acquire_uncancellable_waits_for_a_permit_and_never_cancels / _times_out_as_timeout_not_cancelled / _rejects_queue_full_loudly. (4) opencode sidecar cold-start gating: evaluate-and-decide outcome REVERSED at plan validation — do NOT gate. Measured bounds: gating would hold a permit ~50-70s worst case (health_timeout 20s + request_timeout 30s under the permit; serialized running-mutex adds ~20s per failing holder) vs 10s waits at every other door, and k>=budget cold first-sends queued on the singleton mutex would starve ALL spawn doors — while the double-mutex single-flight already bounds sidecar forks to AT MOST ONE server-wide, so gating adds starvation without reducing fork concurrency. Deliberate do-not-gate comments recorded at BOTH doors (lib.rs send-keys cold arm + opencode_ws.rs materialize). (5) D-C tripwire: grep D-C-REVISIT(FRESHELL_CODEX_MANAGED_LAUNCH) — markers at the REST call site + the flag const, note appended to the rest-spawn-gate plan doc." +``` + +- [ ] **Step 5: Push the branch — NO PR** + +```bash +git log --oneline origin/main..HEAD # review: one focused commit per task +git push -u origin feat/znhn-bccd-followups +``` +Expected: branch pushed. Do NOT open a PR — landing happens outside this workflow with the final review verdict. + +--- + +## Self-Review (performed at plan time) + +**1. Spec coverage — every kata item has a covering task:** + +| Item | Task(s) | Production outcome proved by | +|---|---|---| +| znhn 1 crash trace + patience sentence | 10 (+12) | e2e: trace visible after auto-resume, survives reload, dismissible; doc sentence in crash-resilience plan D-5 | +| znhn 2 circuit breaker + cancel affordance | 7, 8, 9, 11 (+12) | e2e: breaker banner after 3 flaps, argv log pinned at 4; cancel clears notice <3s and respawn count stays 1; cancel HARDENED (D-4, validated A5): registry validation pinned by the unknown-id-ignored test, loud `take_cancel` pinned by the hub settle-frame test; between-thresholds hub test pins the eviction/decide cfg agreement (A8) | +| znhn 3 settle frame for guard-aborts (deletes TTL apparatus) | 5, 6, 9 | hub tests: settle frame on every silent path; client tests: no-timer-degradation-while-connected + frame-driven clear + out-of-order allowlist pin (D-1) + reconnect-backstop pin (`clearRecoveringNotices`, D-3); e2e cancel test exercises the settle frame end-to-end | +| znhn 4 acquire_uncancellable + migrate both callers | 1 | gate unit pins; both call sites migrated (compile-verified; REST + respawn suites green) | +| znhn 5 Relaunch copy | 11 | component tests incl. the provider-mismatch degrade path (copy stays honest) | +| znhn 6 orphan race (evaluate-and-decide) | 9 (pin test), 13 (kata comment) | resolved structurally by TTL deletion — D-9 records the reasoning | +| bccd 1 Retry-After on 429 | 2 | REST unit test asserts header + retryAfterMs | +| bccd 2 burst-test deflake → deterministic max-in-flight pin | 3 | rewritten test, 10× determinism run | +| bccd 3 cancel-sender pin | 1 (superseded — D-8) | new-API semantics pinned instead; consumers are Task 1's two door migrations + Task 3's test pre-hold; kata comment records the supersession | +| bccd 4 sidecar cold-start gating (evaluate-and-decide) | 4 | DECIDED: do NOT gate (D-7 reversed at validation with measured bounds — ~50-70s worst-case permit hold vs 10s waits; singleton already bounds the fork to one); deliberate comments at both doors (lib.rs cold arm + opencode_ws.rs) | +| bccd 5 D-C revisit tripwire | 2 | grep-able markers at both sites + plan-doc note | +| Cross-cutting: contract regen + both pins same-commit | 5, 8 | `test:port`, `cargo test -p freshell-protocol`, restore-contract-wall zero pins (13) | + +No item is deferred; both evaluate-and-decide items are decided in-plan — znhn 6 resolved structurally and pinned, bccd 4 decided as do-not-gate (D-7 reversed at validation) and documented in code comments at both doors. **No unresolved coverage gaps.** + +**1b. No silent deferrals:** every user-facing behavior lands with a production path and an e2e or integration proof (table above). The only test doubles are the established ones (FakeDriver for hub logic — production driver covered by `auto_resume_e2e.rs` + Playwright). Task 4 is a DOCUMENTED DECISION, not a silent deferral: D-7 was reversed at validation with measured bounds (the singleton already bounds the fork to one; gating would starve the spawn budget on ~50-70s worst-case holds), and the outcome is comments-only — deliberate do-not-gate comments at BOTH sidecar fork doors plus the kata comment. Comments-only work needs no red test, so no test double stands in for a behavior there. + +**2. Placeholder scan:** the remaining "KEEP the existing payload" markers in Task 3 are deliberate **splice anchors into existing test bodies quoted by line number** — the implementer copies working in-repo code (byte-identical `shell_create_body()` payload + auth, per the validated A9 note) rather than this plan duplicating (and drifting from) it; every NEW behavior has complete code. (Task 4's former `` anchors are gone with its rewrite to comments-only.) Test skeletons in Tasks 6–9 name their exact harness templates by line. No "TBD"/"add error handling"/"similar to Task N" anywhere. + +**3. Type consistency check (cross-task):** +- `acquire_uncancellable(timeout: Duration) -> Result` — defined Task 1, consumed by Task 1's two door migrations (REST + respawn) and Task 3's test pre-hold with matching signatures (Task 4 no longer consumes it — comments-only after the D-7 reversal). ✓ +- `spawn_gate_error_response(err, retry_after: Duration)` — Task 2 changes the signature; it stays PRIVATE to `terminal_tabs.rs` (the pub(crate) widening was dropped with D-7's reversal); sole caller is the REST door at `:1077` with `(err, rest_gate.timeout)`. ✓ +- `TerminalStatus.resume_cycles: Option` / TS `resumeCycles?: number` — Task 5 defines; Task 6 `emit_settled(resume_cycles: Option)` maps via `i64::from`; Task 8 handler passes `None`; Task 9 reads `msg.resumeCycles`; Task 11 renders it. ✓ +- `recordAutoResumeSettled({ paneId, resumeCycles?, at })` + `selectResumeCycles` — Task 9 defines, Task 11 consumes. ✓ +- `clearRecoveringNotices()` (no payload) — Task 9 defines the reducer, exports it, and dispatches it from the `ws.onReconnect` handling (`TerminalView.tsx:4934-4998`); the reconnect-backstop unit test consumes the same name. ✓ +- `take_cancel` settle emission — Task 8's hub arm calls `emit_settled(&ev.terminal_id, "auto-resume cancelled", None)`, the SAME signature Task 6 defines and the SAME reason string the WS handler broadcasts (idempotent pair); the hub test asserts the `("t1", "auto-resume cancelled", None)` tuple. ✓ +- Eviction cfg threading — Task 7 threads `cfg.healthy_lifetime_ms` (from `HubConfig`) into BOTH `decide`'s `healthy_lifetime_ms` param and the eviction-branch condition at `:242`; only the supervisor panic-health site (`:169`) keeps the compile-time const (documented split); the between-thresholds test pins the agreement. ✓ +- Settle-state clearing — Task 9's `recordTerminalExit` and Task 10's `foldTerminalReplacement` both `delete entry.settle` (A15 leak fix); each has a unit pin ("stale resumeCycles cannot leak into a later crash banner"). ✓ +- `CrashTrace { exitCode, resumedAtMs }` — Task 10 defines; banner + e2e (`crash-trace` testid, `Dismiss ${mode} crash notice`) consume the same names. ✓ +- Cancel wire: `{ type: 'terminal.autoResumeCancel', terminalId }` identical in Rust serde rename (Task 8), TS schema (Task 8), client send (Task 9), e2e button flow (Task 12). ✓ +- Env knob names identical across Task 7 (definitions) and Task 12 (e2e rig): `FRESHELL_AUTO_RESUME_MAX_CYCLES`, `FRESHELL_AUTO_RESUME_CYCLE_WINDOW_MS`, `FRESHELL_AUTO_RESUME_HEALTHY_LIFETIME_MS`, `FRESHELL_RESPAWN_LIVENESS_WINDOW_MS`. ✓ +- Banner props evolve across Tasks 9→10→11 (each task lists the full prop set it leaves behind); final shape: `{ mode, exitCode, notice, crashTrace, settledDead, resumeCycles, canResume, onRelaunch, onCancelAutoResume, onDismissCrashTrace }`. ✓ + +**Known ordering hazards handled:** Task 5 (server-side enum widening) compiles workspace-wide because `ClientMessage` is untouched there; the exhaustive client-message match means the new variant + handler + WsState field + pins all land atomically in Task 8. The `'resumed'` notice kind survives until Task 10 retires it, so Task 9 keeps the verb ternary if any test still needs it (noted inline). + diff --git a/port/contract/ws-message-inventory.json b/port/contract/ws-message-inventory.json index b79796709..f1c691cab 100644 --- a/port/contract/ws-message-inventory.json +++ b/port/contract/ws-message-inventory.json @@ -1,6 +1,6 @@ { "clientToServer": { - "count": 29, + "count": 30, "types": [ "amplifier.activity.list", "claude.activity.list", @@ -23,6 +23,7 @@ "pane.reconcile.request", "ping", "terminal.attach", + "terminal.autoResumeCancel", "terminal.codex.candidate.persisted", "terminal.create", "terminal.detach", diff --git a/port/contract/ws-protocol.schema.json b/port/contract/ws-protocol.schema.json index 630b3c306..7e458ac2f 100644 --- a/port/contract/ws-protocol.schema.json +++ b/port/contract/ws-protocol.schema.json @@ -2,7 +2,7 @@ "description": "Auto-generated from shared/ws-protocol.ts. DO NOT EDIT BY HAND. Regenerate with `npm run contract:generate`. Each entry in `schemas` is a self-contained JSON Schema for one exported Zod schema. The wire contract is frozen for the Rust port — changing it is out of scope.", "generator": "port/contract/generate-ws-contract.ts", "jsonSchemaDialect": "https://json-schema.org/draft/2020-12/schema", - "schemaCount": 66, + "schemaCount": 67, "schemas": { "AmplifierActivityListResponseSchema": { "$schema": "https://json-schema.org/draft/2020-12/schema", @@ -987,6 +987,24 @@ ], "type": "object" }, + { + "additionalProperties": false, + "properties": { + "terminalId": { + "minLength": 1, + "type": "string" + }, + "type": { + "const": "terminal.autoResumeCancel", + "type": "string" + } + }, + "required": [ + "type", + "terminalId" + ], + "type": "object" + }, { "additionalProperties": false, "properties": { @@ -4377,6 +4395,25 @@ ], "type": "object" }, + "TerminalAutoResumeCancelSchema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "additionalProperties": false, + "properties": { + "terminalId": { + "minLength": 1, + "type": "string" + }, + "type": { + "const": "terminal.autoResumeCancel", + "type": "string" + } + }, + "required": [ + "type", + "terminalId" + ], + "type": "object" + }, "TerminalCodexCandidatePersistedSchema": { "$schema": "https://json-schema.org/draft/2020-12/schema", "additionalProperties": false, diff --git a/port/contract/ws-server-messages.schema.json b/port/contract/ws-server-messages.schema.json index 4eeda8af1..d69456d3d 100644 --- a/port/contract/ws-server-messages.schema.json +++ b/port/contract/ws-server-messages.schema.json @@ -3015,10 +3015,14 @@ "reason": { "type": "string" }, + "resumeCycles": { + "type": "number" + }, "status": { "enum": [ "running", - "recovering" + "recovering", + "exited" ], "type": "string" }, diff --git a/server/ws-handler.ts b/server/ws-handler.ts index 0784d39ec..ff4dc38e3 100644 --- a/server/ws-handler.ts +++ b/server/ws-handler.ts @@ -3783,6 +3783,12 @@ export class WsHandler { return } + case 'terminal.autoResumeCancel': + // Rust-only feature: agent auto-resume lives in freshell-ws. The + // Node server has no auto-resume hub — accept and ignore so a valid + // client message never triggers UNKNOWN_MESSAGE. + return + default: this.sendError(ws, { code: 'UNKNOWN_MESSAGE', message: 'Unknown message type' }) return diff --git a/shared/ws-protocol.ts b/shared/ws-protocol.ts index 5af6f7ff1..86072889c 100644 --- a/shared/ws-protocol.ts +++ b/shared/ws-protocol.ts @@ -356,6 +356,13 @@ export const TerminalDetachSchema = z.object({ terminalId: z.string().min(1), }) +export const TerminalAutoResumeCancelSchema = z.object({ + type: z.literal('terminal.autoResumeCancel'), + /** The OLD (crashed) terminal id from the recovering notice frame. */ + terminalId: z.string().min(1), +}) +export type TerminalAutoResumeCancelMessage = z.infer + export const TerminalInputSchema = z.object({ type: z.literal('terminal.input'), terminalId: z.string().min(1), @@ -659,6 +666,7 @@ export const ClientMessageSchema = z.discriminatedUnion('type', [ TerminalCreateSchema, TerminalCodexCandidatePersistedSchema, TerminalAttachSchema, + TerminalAutoResumeCancelSchema, TerminalDetachSchema, TerminalInputSchema, TerminalResizeSchema, @@ -772,7 +780,7 @@ export type TerminalExitMessage = { export type TerminalStatusMessage = { type: 'terminal.status' terminalId: string - status: 'running' | 'recovering' + status: 'running' | 'recovering' | 'exited' reason?: string attempt?: number /** Auto-resume 'recovering' frames only: the bounded retry budget. The @@ -781,6 +789,10 @@ export type TerminalStatusMessage = { maxAttempts?: number /** Auto-resume 'recovering' frames only: the crashed generation's exit code. */ exitCode?: number + /** Flap-circuit-breaker settle frames ('exited') only: successful + * auto-resumes inside the rolling window — the typed source for the + * "crashed N times" banner. */ + resumeCycles?: number } /** Lane D1: server-initiated crash auto-resume replaced a pane's terminal. diff --git a/src/components/TerminalExitBanner.tsx b/src/components/TerminalExitBanner.tsx index 0b4dad62a..c158c749c 100644 --- a/src/components/TerminalExitBanner.tsx +++ b/src/components/TerminalExitBanner.tsx @@ -1,45 +1,107 @@ // Lane D1: loud exited-pane presentation for coding-agent terminals. -// - recovering/resumed notice (server-driven auto-resume in flight/succeeded) -// - error bar + Relaunch after the pane settles exited (non-zero exit). +// - recovering notice (server-driven auto-resume in flight) + cancel +// - error bar + Relaunch after the pane settles exited (non-zero exit) +// - persistent, dismissible crash trace after a successful auto-resume +// (kata znhn item 1 — replaces the ephemeral 'resumed' strip). // Pure presentational: props in, callbacks out — TerminalView owns the render // conditions and the relaunch dispatch. import type { AutoResumeNotice } from '../store/terminalLifecycleSlice' +import type { CrashTrace } from '../store/paneTypes' export interface TerminalExitBannerProps { mode: string exitCode: number | null notice: AutoResumeNotice | null + crashTrace: CrashTrace | null + settledDead: boolean + /** Flap-circuit-breaker settles only (znhn item 2): successful auto-resumes + * inside the rolling window, from the settle frame's TYPED field. */ + resumeCycles: number | null + /** znhn item 5: honest Relaunch copy — true when the pane's sessionRef can + * resume this conversation (provider matches mode). */ + canResume: boolean onRelaunch: () => void + onCancelAutoResume: () => void + onDismissCrashTrace: () => void } -export function TerminalExitBanner({ mode, exitCode, notice, onRelaunch }: TerminalExitBannerProps) { +export function TerminalExitBanner({ + mode, + exitCode, + notice, + crashTrace, + settledDead, + resumeCycles, + canResume, + onRelaunch, + onCancelAutoResume, + onDismissCrashTrace, +}: TerminalExitBannerProps) { if (notice) { - const verb = notice.kind === 'recovering' ? 'auto-resuming' : 'auto-resumed' return (
- {mode} crashed (exit {notice.exitCode}) — {verb}, attempt {notice.attempt}/{notice.maxAttempts} + {mode} crashed (exit {notice.exitCode}) — auto-resuming, attempt {notice.attempt}/{notice.maxAttempts} +
) } - return ( -
- process exited{exitCode !== null ? ` (code ${exitCode})` : ''} - +
+ ) + } + if (crashTrace) { + const d = new Date(crashTrace.resumedAtMs) + const hh = String(d.getHours()).padStart(2, '0') + const mm = String(d.getMinutes()).padStart(2, '0') + return ( +
- Relaunch - -
- ) + + {mode} crashed (exit {crashTrace.exitCode}) & auto-resumed at {hh}:{mm} + + + + ) + } + return null } diff --git a/src/components/TerminalView.tsx b/src/components/TerminalView.tsx index b2a6f2a16..ea893b9e5 100644 --- a/src/components/TerminalView.tsx +++ b/src/components/TerminalView.tsx @@ -20,6 +20,8 @@ import { RECONCILE_NOTICE_FRESH_BY_RACE, repairCodexIdentityMismatch, resetPaneForReconcileCreate, + setPaneCrashTrace, + clearPaneCrashTrace, splitPane, updatePaneContent, updatePaneTitle, @@ -32,14 +34,16 @@ import { updateSettingsLocal } from '@/store/settingsSlice' import { clearPaneRuntimeActivity } from '@/store/paneRuntimeActivitySlice' import { recordTurnComplete } from '@/store/turnCompletionSlice' import { - AUTO_RESUME_NOTICE_TTL_MS, + clearRecoveringNotices, clearTerminalLifecycle, foldTerminalReplacement, recordAutoResumeRecovering, + recordAutoResumeSettled, recordTerminalExit, selectActiveNotice, selectExitRecord, selectLastTerminalIdFrom, + selectResumeCycles, } from '@/store/terminalLifecycleSlice' import { TerminalExitBanner } from '@/components/TerminalExitBanner' import { dismissTabGreen } from '@/store/turnCompletionAttention' @@ -604,19 +608,14 @@ function TerminalView({ tabId, paneId, paneContent, hidden }: TerminalViewProps) // terminalId — the exit handler clears paneContent.terminalId, so an exited // pane has no terminal id to key by. const exitRecord = useAppSelector((s) => selectExitRecord(s, paneId)) - const activeNotice = useAppSelector((s) => selectActiveNotice(s, paneId, Date.now())) - // Deterministic notice→alert degradation: a 'recovering' notice orphaned by - // a SILENT auto-resume settle (respawn_failed / session_lease_held / - // session_owned_live / pane_closed — none emits a frame) would otherwise - // only flip to the error bar "whenever something else re-renders". Schedule - // exactly one re-render at TTL expiry so selectActiveNotice re-evaluates. - const [, setNoticeExpiryTick] = useState(0) - useEffect(() => { - if (!activeNotice) return - const delay = Math.max(0, activeNotice.at + AUTO_RESUME_NOTICE_TTL_MS - Date.now() + 1) - const timer = window.setTimeout(() => setNoticeExpiryTick((n) => n + 1), delay) - return () => window.clearTimeout(timer) - }, [activeNotice]) + // Frame-driven notice (znhn item 3): every settle path now broadcasts a + // terminal.status{exited} settle frame, so the old 30s TTL guessing + // apparatus (selector filter + expiry re-render timer) is deleted. A + // missed frame is corrected by the reconnect backstop (D-3) below. + const activeNotice = useAppSelector((s) => selectActiveNotice(s, paneId)) + // Flap-circuit-breaker settle count (znhn item 2) — typed field, feeds the + // "crashed N times — auto-resume paused" alert copy. + const resumeCycles = useAppSelector((s) => selectResumeCycles(s, paneId)) ?? null // All hooks MUST be called before any conditional returns const ws = useMemo(() => getWsClient(), []) @@ -4374,6 +4373,23 @@ function TerminalView({ tabId, paneId, paneContent, hidden }: TerminalViewProps) at: Date.now(), })) } + // Settle frame (znhn item 3): the deterministic end of an + // auto-resume story — clears the recovering notice on a FRAME, + // never a timer. Hard rule (D-1, validated A1/A6): dispatches ONLY + // into terminalLifecycleSlice — NEVER pane content/status. Cross- + // channel ordering vs terminal.exit is unguaranteed (unbiased + // select!, terminal.rs:325-334); the running|recovering content- + // write allowlist below is the load-bearing barrier that keeps an + // out-of-order 'exited' frame away from pane content. + if (statusMine && msg.status === 'exited') { + dispatch( + recordAutoResumeSettled({ + paneId: paneIdRef.current, + resumeCycles: msg.resumeCycles, + at: Date.now(), + }) + ) + } if (msg.terminalId === tid) { if ( msg.status === 'running' @@ -4404,6 +4420,15 @@ function TerminalView({ tabId, paneId, paneContent, hidden }: TerminalViewProps) maxAttempts: msg.maxAttempts, at: Date.now(), })) + // znhn item 1: the persistent, dismissible crash trace lives on + // pane CONTENT (persistence is a denylist — it survives reload). + dispatch( + setPaneCrashTrace({ + tabId, + paneId: paneIdRef.current, + crashTrace: { exitCode: msg.exitCode, resumedAtMs: Date.now() }, + }) + ) // Fold the new terminalId into this pane via the ONE reducer built // for server-supplied rebinds (mirrors pane-reconcile.ts:428-436). // applyReconcileAttach unconditionally overwrites serverInstanceId @@ -4932,6 +4957,10 @@ function TerminalView({ tabId, paneId, paneContent, hidden }: TerminalViewProps) }) unsubReconnect = ws.onReconnect(() => { + // D-3 backstop: any missed settle/replaced frame necessarily passed + // through this reconnect (bounded broadcast, no replay; lag force- + // closes the socket). Stale recovering notices must not survive it. + dispatch(clearRecoveringNotices()) const tid = terminalIdRef.current if (debugRef.current) log.debug('[TRACE resumeSessionId] onReconnect', { paneId: paneIdRef.current, @@ -5230,12 +5259,11 @@ function TerminalView({ tabId, paneId, paneContent, hidden }: TerminalViewProps) // (agent process died), same alert. Plain launch failures (create // rejected — no exit record) keep today's presentation. const isAgentPane = Boolean(terminalContent.mode && terminalContent.mode !== 'shell') + const settledDead = + (terminalContent.status === 'exited' && (exitRecord ? exitRecord.exitCode !== 0 : true)) || + (terminalContent.status === 'error' && Boolean(exitRecord && exitRecord.exitCode !== 0)) const showExitBanner = Boolean( - isAgentPane && ( - activeNotice || - (terminalContent.status === 'exited' && (exitRecord ? exitRecord.exitCode !== 0 : true)) || - (terminalContent.status === 'error' && exitRecord && exitRecord.exitCode !== 0) - ) + isAgentPane && (activeNotice || terminalContent.crashTrace || settledDead) ) return ( @@ -5336,6 +5364,13 @@ function TerminalView({ tabId, paneId, paneContent, hidden }: TerminalViewProps) mode={terminalContent.mode ?? 'agent'} exitCode={exitRecord?.exitCode ?? null} notice={activeNotice ?? null} + crashTrace={terminalContent.crashTrace ?? null} + settledDead={settledDead} + resumeCycles={resumeCycles} + canResume={Boolean( + terminalContent.sessionRef && terminalContent.sessionRef.provider === terminalContent.mode + )} + onDismissCrashTrace={() => dispatch(clearPaneCrashTrace({ tabId, paneId }))} onRelaunch={() => { // Discard the OLD crash's lifecycle entry: if the relaunch // create is rejected (pane settles 'error' with no new @@ -5351,6 +5386,15 @@ function TerminalView({ tabId, paneId, paneContent, hidden }: TerminalViewProps) sessionRef: terminalContent.sessionRef, })) }} + onCancelAutoResume={() => { + // znhn item 2: the recovering frame carries the OLD terminal id + // — the same id the server keys the pending resume by. + const lastTid = selectLastTerminalIdFrom( + appStore.getState().terminalLifecycle, + paneId + ) + if (lastTid) ws.send({ type: 'terminal.autoResumeCancel', terminalId: lastTid }) + }} /> )} diff --git a/src/store/paneTypes.ts b/src/store/paneTypes.ts index 470398546..a9b31fc43 100644 --- a/src/store/paneTypes.ts +++ b/src/store/paneTypes.ts @@ -68,6 +68,16 @@ export function normalizeFreshAgentEffortOverride(value: unknown): string | unde * Terminal pane content with full lifecycle management. * Each terminal pane owns its backend terminal process. */ +/** Persistent crash trace (kata znhn item 1): "crashed & auto-resumed" — + * survives reload (pane-content persistence is a denylist) until the user + * dismisses it or the pane closes. */ +export type CrashTrace = { + /** Exit code of the crashed generation. */ + exitCode: number + /** Wall-clock ms when the auto-resume succeeded. */ + resumedAtMs: number +} + export type TerminalPaneContent = { kind: 'terminal' /** Backend terminal ID (undefined until created) */ @@ -100,6 +110,9 @@ export type TerminalPaneContent = { pendingReconcile?: 'respawn' | 'fresh' /** VOLATILE fold counter. Incremented by applyReconcileAttach / resetPaneForReconcileCreate so a fold on an already-mounted pane (same createRequestId — never re-minted) re-fires TerminalView's create-or-attach effect (Task 12 adds it to the dep array). Stripped from persistence (Task 8). */ reconcileEpoch?: number + /** znhn item 1: persisted deliberately — do NOT add to + * stripTransientSessionFields. Absent on old layouts = no trace. */ + crashTrace?: CrashTrace } /** diff --git a/src/store/panesSlice.ts b/src/store/panesSlice.ts index 45f6dc9ba..877fe2529 100644 --- a/src/store/panesSlice.ts +++ b/src/store/panesSlice.ts @@ -5,6 +5,7 @@ import { normalizeFreshAgentModelSelection, normalizeFreshAgentPendingLocalEcho, type DeadSessionEntry, + type CrashTrace, type FreshAgentPaneContent, type LivePaneContentInput, type PanesState, @@ -60,6 +61,16 @@ function readRestoreError(value: unknown): RestoreError | undefined { /** * Normalize pane content to the full persisted/runtime shape. */ +/** Shape guard for the persisted crash trace (znhn item 1). */ +function isCrashTrace(value: unknown): value is CrashTrace { + return ( + typeof value === 'object' + && value !== null + && typeof (value as { exitCode?: unknown }).exitCode === 'number' + && typeof (value as { resumedAtMs?: unknown }).resumedAtMs === 'number' + ) +} + function normalizePaneContent( rawInput: PaneContentInput | PaneContent | Record, previous?: PaneContent, @@ -100,6 +111,13 @@ function normalizePaneContent( ? input.pendingReconcile : undefined, reconcileEpoch: typeof input.reconcileEpoch === 'number' ? input.reconcileEpoch : undefined, + // znhn item 1: the persistent crash trace must survive the hydrate + // normalize (this function is a whitelist — without this line the + // "survives reload" property silently dies here even though the + // persistMiddleware strip and persistedState load both keep it). + ...(isCrashTrace((input as { crashTrace?: unknown }).crashTrace) + ? { crashTrace: (input as { crashTrace: CrashTrace }).crashTrace } + : {}), } } if (input.kind === 'browser') { @@ -1979,6 +1997,11 @@ export const panesSlice = createSlice({ content.sessionRef = undefined content.resumeSessionId = undefined content.codexDurability = undefined + // znhn item 1 (fresh-eyes fix): a fresh create is a genuinely NEW + // identity-less conversation — the persisted "crashed & auto-resumed" + // trace belongs to the retired session and must not leak onto it. + // The 'respawn' branch deliberately KEEPS it (same conversation). + content.crashTrace = undefined } content.pendingReconcile = intent // A1 fix: same-createRequestId folds are only observable via the epoch bump. @@ -2095,6 +2118,23 @@ export const panesSlice = createSlice({ content.reconcileNotice = undefined }, + // znhn item 1: persistent crash trace — written on terminal.replaced, + // cleared only by user dismissal (pane close deletes the pane node). + setPaneCrashTrace: ( + state, + action: PayloadAction<{ tabId: string; paneId: string; crashTrace: CrashTrace }> + ) => { + const content = findReconcileTerminalContent(state, action.payload.tabId, action.payload.paneId) + if (content) content.crashTrace = action.payload.crashTrace + }, + clearPaneCrashTrace: ( + state, + action: PayloadAction<{ tabId: string; paneId: string }> + ) => { + const content = findReconcileTerminalContent(state, action.payload.tabId, action.payload.paneId) + if (content && content.crashTrace) delete content.crashTrace + }, + /** Council rule 12: dead_session is a UI state, not a deletion — panes wait for the user. */ setDeadSessionAdjudication: (state, action: PayloadAction) => { state.deadSessionAdjudication = action.payload @@ -2235,6 +2275,8 @@ export const { resetFreshAgentPaneForReconcileCreate, setPaneReconcileNotice, clearPaneReconcileNotice, + setPaneCrashTrace, + clearPaneCrashTrace, setDeadSessionAdjudication, resolveDeadSessionEntry, clearDeadSessionAdjudication, diff --git a/src/store/terminalLifecycleSlice.ts b/src/store/terminalLifecycleSlice.ts index 085a7f28a..50b3cab62 100644 --- a/src/store/terminalLifecycleSlice.ts +++ b/src/store/terminalLifecycleSlice.ts @@ -6,8 +6,6 @@ // slice is never added to that allowlist, so it is never persisted. import { createSlice, type PayloadAction } from '@reduxjs/toolkit' -export const AUTO_RESUME_NOTICE_TTL_MS = 30_000 - export interface TerminalExitRecord { exitCode: number; at: number } export interface AutoResumeNotice { kind: 'recovering' | 'resumed' @@ -21,6 +19,9 @@ export interface PaneLifecycleEntry { lastTerminalId?: string exit?: TerminalExitRecord notice?: AutoResumeNotice + /** Settle frame record (znhn item 3) — resumeCycles is present only for + * flap-circuit-breaker settles and feeds the "crashed N times" banner. */ + settle?: { resumeCycles?: number; at: number } } interface TerminalLifecycleState { @@ -46,30 +47,66 @@ const slice = createSlice({ // Fresh-eyes fix: an exit is always NEWER truth than any notice. Without // this, the exhaustion path (last crash -> settle, which emits no frame) // leaves the previous 'resumed' notice masking the role=alert error bar - // for the 30s TTL — a success-toned banner on a dead pane. Clearing here - // makes the alert show immediately on the final crash; a genuine - // in-flight resume re-sets the notice when its `recovering` frame lands - // (which always follows the exit, per Task 5's emit order). + // — a success-toned banner on a dead pane. Clearing here makes the alert + // show immediately on the final crash; a genuine in-flight resume + // re-sets the notice when its `recovering` frame lands (which always + // follows the exit, per Task 5's emit order). delete e.notice + // Stale-settle leak fix (validated A15): a new crash must never inherit + // an earlier breaker settle's resumeCycles, or the alert would read + // "crashed N times — auto-resume paused" on a non-breaker crash. + delete e.settle }, recordAutoResumeRecovering(state, a: PayloadAction<{ paneId: string; attempt: number; maxAttempts: number; exitCode: number; at: number }>) { const { paneId, ...n } = a.payload entry(state, paneId).notice = { kind: 'recovering', ...n } }, foldTerminalReplacement(state, a: PayloadAction<{ paneId: string; newTerminalId: string; exitCode: number; attempt: number; maxAttempts: number; at: number }>) { - const { paneId, newTerminalId, exitCode, attempt, maxAttempts, at } = a.payload + const { paneId, newTerminalId } = a.payload const e = entry(state, paneId) delete e.exit // pane is alive again — no error bar - e.notice = { kind: 'resumed', attempt, maxAttempts, exitCode, at } + // znhn item 1: the ephemeral 'resumed' strip is retired — the + // persistent crash trace on pane content is the post-resume indicator. + delete e.notice + // Stale-settle leak fix (validated A15, pairs with recordTerminalExit). + delete e.settle e.lastTerminalId = newTerminalId }, + // Settle frame (terminal.status status:'exited') — the deterministic + // replacement for the old 30s TTL guess (znhn item 3). + recordAutoResumeSettled( + state, + action: PayloadAction<{ paneId: string; resumeCycles?: number; at: number }> + ) { + const e = entry(state, action.payload.paneId) + delete e.notice + e.settle = { + at: action.payload.at, + ...(action.payload.resumeCycles !== undefined + ? { resumeCycles: action.payload.resumeCycles } + : {}), + } + }, + // D-3 backstop (validated): the settle/replaced frames are fire-and-forget + // on a bounded broadcast (no replay; lagged receivers are force-closed), + // so every missed-frame path necessarily passes through a WS reconnect. + // Clearing stale recovering notices on reconnect makes a lying notice + // impossible; frames stay the primary mechanism. No TTL returns. + clearRecoveringNotices(state) { + for (const e of Object.values(state.byPaneId)) { + if (e?.notice?.kind === 'recovering') delete e.notice + } + }, clearTerminalLifecycle(state, a: PayloadAction<{ paneId: string }>) { delete state.byPaneId[a.payload.paneId] }, }, }) -export const { recordTerminalExit, recordAutoResumeRecovering, foldTerminalReplacement, clearTerminalLifecycle } = slice.actions +export const { + recordTerminalExit, recordAutoResumeRecovering, foldTerminalReplacement, + recordAutoResumeSettled, clearRecoveringNotices, clearTerminalLifecycle, +} = slice.actions export default slice.reducer // Selectors tolerate an absent slice state (`s?.`): many pre-existing client @@ -80,13 +117,15 @@ export default slice.reducer // include the reducer (store.ts), so this never changes runtime behavior. export const selectExitRecordFrom = (s: TerminalLifecycleState | undefined, paneId: string) => s?.byPaneId[paneId]?.exit export const selectLastTerminalIdFrom = (s: TerminalLifecycleState | undefined, paneId: string) => s?.byPaneId[paneId]?.lastTerminalId -export const selectActiveNoticeFrom = (s: TerminalLifecycleState | undefined, paneId: string, now: number) => { - const n = s?.byPaneId[paneId]?.notice - return n && now - n.at <= AUTO_RESUME_NOTICE_TTL_MS ? n : undefined -} +// No TTL (znhn item 3): notices are frame-driven — cleared by settle frames, +// terminal.replaced folds, terminal.exit, or the reconnect backstop. +export const selectActiveNoticeFrom = (s: TerminalLifecycleState | undefined, paneId: string) => + s?.byPaneId[paneId]?.notice // Root-state wrappers — match the RootState typing convention of the sibling // selectors in this directory (see turnCompletionSlice.ts for the pattern): export const selectExitRecord = (root: { terminalLifecycle?: TerminalLifecycleState }, paneId: string) => selectExitRecordFrom(root.terminalLifecycle, paneId) -export const selectActiveNotice = (root: { terminalLifecycle?: TerminalLifecycleState }, paneId: string, now: number) => - selectActiveNoticeFrom(root.terminalLifecycle, paneId, now) +export const selectActiveNotice = (root: { terminalLifecycle?: TerminalLifecycleState }, paneId: string) => + selectActiveNoticeFrom(root.terminalLifecycle, paneId) +export const selectResumeCycles = (root: { terminalLifecycle?: TerminalLifecycleState }, paneId: string) => + root.terminalLifecycle?.byPaneId[paneId]?.settle?.resumeCycles diff --git a/test/e2e-browser/fixtures/fake-crashing-claude-cli.mjs b/test/e2e-browser/fixtures/fake-crashing-claude-cli.mjs index 86e098123..3c9839d55 100644 --- a/test/e2e-browser/fixtures/fake-crashing-claude-cli.mjs +++ b/test/e2e-browser/fixtures/fake-crashing-claude-cli.mjs @@ -9,6 +9,9 @@ // once — invocation #1 prints output then exits 1; later invocations stay alive // always — every invocation prints then exits 1 immediately // clean — prints then exits 0 (the default when neither env is set) +// FAKE_CRASH_LIVE_MS=N — with FAKE_CRASH_MODE=always: stay alive N ms, then +// exit 1 (a "healthy flap": long enough to reset the retry budget when the +// server's healthy-lifetime knob is shrunk below N). // Every invocation appends {pid,t,argv} to FAKE_CLAUDE_ARGV_LOG (JSONL) and // bumps the invocation counter in FAKE_CRASH_STATE_FILE. import fs from 'node:fs' @@ -39,7 +42,15 @@ if (crashUntil > 0) { const mode = process.env.FAKE_CRASH_MODE || 'clean' if (mode === 'always' || (mode === 'once' && invocation === 1)) { process.stdout.write('fake-claude: simulated crash\r\n') - process.exit(1) + const liveMs = Number(process.env.FAKE_CRASH_LIVE_MS || '0') + if (liveMs > 0) { + // Flap mode (kata znhn item 2 e2e): stay alive liveMs, THEN exit 1 — + // keep the event loop alive exactly like the SURVIVE path does. + setTimeout(() => process.exit(1), liveMs) + process.stdin.resume() + } else { + process.exit(1) + } } if (mode === 'clean') { process.stdout.write('fake-claude: clean exit\r\n') diff --git a/test/e2e-browser/specs/agent-crash-autoresume-rust.spec.ts b/test/e2e-browser/specs/agent-crash-autoresume-rust.spec.ts index c34eb6de3..7c9ca1bbc 100644 --- a/test/e2e-browser/specs/agent-crash-autoresume-rust.spec.ts +++ b/test/e2e-browser/specs/agent-crash-autoresume-rust.spec.ts @@ -175,9 +175,15 @@ async function createClaudePane(page: Page, info: TestServerInfo): Promise page.getByRole('status').filter({ hasText: /auto-resum/ }) +/** ONLY the in-flight recovering notice (znhn#1 retired the ephemeral + * 'resumed' strip; the persistent crash trace says "auto-resumed at"). */ +const recoveringNotice = (page: Page) => page.getByRole('status').filter({ hasText: /auto-resuming/ }) + test.describe('agent crash auto-resume (rust only)', () => { // Pay any cold cargo release build inside a generous HOOK timeout, not a // test timeout (donor: recover-my-panes-rust.spec.ts's beforeAll). With @@ -216,8 +222,10 @@ test.describe('agent crash auto-resume (rust only)', () => { ).toBe(true) }).toPass({ timeout: 30_000 }) - // UI: the auto-resume notice is visible (the 'resumed' notice persists - // for its 30s TTL, so this cannot race the 100ms recovering window)... + // UI: the auto-resume surface is visible (znhn#1: the persistent crash + // trace — "crashed & auto-resumed at HH:MM" — replaced the ephemeral + // resumed strip and persists until dismissed, so this cannot race the + // 100ms recovering window)... await expect(autoResumeNotice(page)).toBeVisible({ timeout: 15_000 }) // ...and the pane is back to a live terminal: no role=alert error bar, // and the claude pane's content settles on a running terminal. @@ -341,12 +349,112 @@ test.describe('agent crash auto-resume (rust only)', () => { // Genuinely LIVE: the argv log stays at EXACTLY 4 invocations for >=1s // (a clean exit-0 would re-settle the pane; a crash would append - // invocation 5), and neither the alert bar nor an auto-resume notice - // reappears in that window. + // invocation 5), and neither the alert bar nor an in-flight recovering + // notice reappears in that window. (The persistent crash trace from the + // earlier successful auto-resumes legitimately remains — znhn#1 — so + // the assertion targets the RECOVERING notice specifically.) await page.waitForTimeout(1_000) expect((await readArgvLog(rig.argvLog)).length, 'invocation 4 must stay alive').toBe(4) await expect(page.getByRole('alert')).toHaveCount(0) - await expect(autoResumeNotice(page)).toHaveCount(0) + await expect(recoveringNotice(page)).toHaveCount(0) + } finally { + await teardownRig(rig) + } + }) + + test('a persistent crash trace survives reload and is dismissible', async ({ page, e2eServerKind }) => { + expect(e2eServerKind).toBe('rust') + test.setTimeout(240_000) + let rig: Rig | undefined + try { + rig = await bootRig('trace', { FAKE_CRASH_MODE: 'once' }) + await createClaudePane(page, rig.info) + + const trace = page.getByTestId('crash-trace') + await expect(trace).toBeVisible({ timeout: 30_000 }) + await expect(trace).toHaveText(/claude crashed \(exit 1\) & auto-resumed at \d{2}:\d{2}/) + await expect(page.getByRole('alert')).toHaveCount(0) + + // The morning-user scenario: the trace survives a reload. + await page.reload() + await connect(page, rig.info) + await expect(page.getByTestId('crash-trace')).toBeVisible({ timeout: 30_000 }) + + // Dismiss → gone, and STAYS gone across another reload. + await page.getByRole('button', { name: 'Dismiss claude crash notice' }).click() + await expect(page.getByTestId('crash-trace')).toHaveCount(0) + await page.reload() + await connect(page, rig.info) + await expect(page.locator('.xterm').first()).toBeVisible({ timeout: 30_000 }) + await expect(page.getByTestId('crash-trace')).toHaveCount(0) + } finally { + await teardownRig(rig) + } + }) + + test('a flap loop trips the circuit breaker: settles with the crashed-N-times banner', async ({ page, e2eServerKind }) => { + expect(e2eServerKind).toBe('rust') + test.setTimeout(240_000) + let rig: Rig | undefined + try { + rig = await bootRig('flap', { + FAKE_CRASH_MODE: 'always', + FAKE_CRASH_LIVE_MS: '1000', + FRESHELL_AUTO_RESUME_DELAYS_MS: '100,200', + // Each 1s generation counts as "healthy" (budget resets — the + // forever-loop precondition) and stays under the registry window so + // the generation cap never preempts the breaker. + FRESHELL_AUTO_RESUME_HEALTHY_LIFETIME_MS: '500', + FRESHELL_RESPAWN_LIVENESS_WINDOW_MS: '500', + FRESHELL_AUTO_RESUME_MAX_CYCLES: '3', + }) + await createClaudePane(page, rig.info) + + const alert = page.getByRole('alert').filter({ hasText: 'claude crashed 3 times — auto-resume paused' }) + await expect(alert).toBeVisible({ timeout: 60_000 }) + + // Bounded: 1 original + 3 auto-resumes, then nothing more. + await expect(async () => { + expect((await readArgvLog(rig!.argvLog)).length).toBe(4) + }).toPass({ timeout: 15_000 }) + await page.waitForTimeout(3_000) + expect((await readArgvLog(rig.argvLog)).length, 'breaker must stay open').toBe(4) + await expect(page.getByRole('button', { name: 'Relaunch claude session' })).toBeVisible() + } finally { + await teardownRig(rig) + } + }) + + test('cancel clears the recovering notice immediately and no respawn happens', async ({ page, e2eServerKind }) => { + expect(e2eServerKind).toBe('rust') + test.setTimeout(240_000) + let rig: Rig | undefined + try { + // Long backoff = a wide window where the OLD behavior would have lied + // for 30s (znhn#3) and no window at all for the alert bar (znhn#6). + // FAKE_CRASH_LIVE_MS keeps invocation 1 alive ~5s so the pane-creation + // choreography fully settles BEFORE the crash: the cancel click then + // lands early in the 8s backoff (observed: a crash mid-choreography + // pushed the click past the first backoff, so attempt 1 had already + // respawned before the cancel could land). + rig = await bootRig('cancel', { + FAKE_CRASH_MODE: 'always', + FAKE_CRASH_LIVE_MS: '5000', + FRESHELL_AUTO_RESUME_DELAYS_MS: '8000,8000', + }) + await createClaudePane(page, rig.info) + + await expect(recoveringNotice(page)).toBeVisible({ timeout: 30_000 }) + await page.getByRole('button', { name: 'Cancel auto-resume for claude' }).click() + + // Settle frame, not TTL: the notice clears within seconds, the loud + // alert takes its place. + await expect(recoveringNotice(page)).toHaveCount(0, { timeout: 3_000 }) + await expect(page.getByRole('alert').filter({ hasText: 'process exited (code 1)' })).toBeVisible({ timeout: 5_000 }) + + // The planned respawn was guard-aborted: still only 1 invocation. + await page.waitForTimeout(10_000) + expect((await readArgvLog(rig.argvLog)).length, 'cancel must abort the planned respawn').toBe(1) } finally { await teardownRig(rig) } diff --git a/test/unit/client/components/TerminalExitBanner.test.tsx b/test/unit/client/components/TerminalExitBanner.test.tsx index 33c2870f9..833223604 100644 --- a/test/unit/client/components/TerminalExitBanner.test.tsx +++ b/test/unit/client/components/TerminalExitBanner.test.tsx @@ -2,6 +2,16 @@ import { describe, it, expect, vi, afterEach } from 'vitest' import { render, screen, fireEvent, cleanup } from '@testing-library/react' import { TerminalExitBanner } from '@/components/TerminalExitBanner' +const noop = () => {} +const baseProps = { + crashTrace: null, + resumeCycles: null, + canResume: false, + onRelaunch: noop, + onCancelAutoResume: noop, + onDismissCrashTrace: noop, +} + describe('TerminalExitBanner', () => { // This repo's vitest setup does not auto-cleanup between tests (globals off); // sibling suites (DeadSessionPanel.test.tsx) call cleanup() explicitly. @@ -9,7 +19,7 @@ describe('TerminalExitBanner', () => { it('renders a loud error bar with the exit code and an accessible relaunch button', () => { const onRelaunch = vi.fn() - render() + render() const bar = screen.getByRole('alert') expect(bar).toHaveTextContent('process exited (code 1)') const btn = screen.getByRole('button', { name: 'Relaunch claude session' }) @@ -18,23 +28,64 @@ describe('TerminalExitBanner', () => { }) it('renders without a code when the exit code is unknown (post-reload)', () => { - render( {}} />) + render() expect(screen.getByRole('alert')).toHaveTextContent('process exited') expect(screen.getByRole('alert')).not.toHaveTextContent('(code') }) - it('renders a recovering notice instead of the error bar while auto-resume is in flight', () => { - render( { + const onCancel = vi.fn() + render( {}} />) + onCancelAutoResume={onCancel} />) expect(screen.queryByRole('alert')).toBeNull() expect(screen.getByRole('status')).toHaveTextContent('claude crashed (exit 1) — auto-resuming, attempt 1/2') + // znhn item 2: the user can opt out of the in-flight auto-resume. + const cancel = screen.getByRole('button', { name: 'Cancel auto-resume for claude' }) + fireEvent.click(cancel) + expect(onCancel).toHaveBeenCalledTimes(1) + }) + + it('renders the persistent crash trace (role=status, NOT alert) with a dismiss button', () => { + // znhn item 1: the trace replaces the ephemeral 'resumed' strip. It must + // NOT be role=alert — e2e happy paths assert alert count 0. + const onDismiss = vi.fn() + // 2026-07-29T03:37:00 local — assert on the derived HH:MM. + const resumedAtMs = new Date(2026, 6, 29, 9, 5).getTime() + render() + expect(screen.queryByRole('alert')).toBeNull() + const trace = screen.getByTestId('crash-trace') + expect(trace).toHaveAttribute('role', 'status') + expect(trace).toHaveTextContent('claude crashed (exit 1) & auto-resumed at 09:05') + fireEvent.click(screen.getByRole('button', { name: 'Dismiss claude crash notice' })) + expect(onDismiss).toHaveBeenCalledTimes(1) + }) + + it('renders nothing when there is no notice, no settled death, and no trace', () => { + const { container } = render( + + ) + expect(container).toBeEmptyDOMElement() + }) + + it('labels Relaunch honestly when the sessionRef can resume the conversation (znhn#5)', () => { + render() + const btn = screen.getByRole('button', { name: 'Relaunch claude session' }) + expect(btn).toHaveTextContent('Relaunch — resumes this conversation') + }) + + it('keeps plain Relaunch copy when no matching sessionRef exists (degrades to fresh)', () => { + render() + const btn = screen.getByRole('button', { name: 'Relaunch claude session' }) + expect(btn).toHaveTextContent(/^Relaunch$/) }) - it('renders a resumed notice', () => { - render( {}} />) - expect(screen.getByRole('status')).toHaveTextContent('claude crashed (exit 1) — auto-resumed, attempt 2/2') + it('renders the circuit-breaker banner from the typed resumeCycles field (znhn#2)', () => { + render() + expect(screen.getByRole('alert')).toHaveTextContent('claude crashed 5 times — auto-resume paused') + // Relaunch stays available — bounded and loud, never dead-ended. + expect(screen.getByRole('button', { name: 'Relaunch claude session' })).toBeInTheDocument() }) }) diff --git a/test/unit/client/components/TerminalView.exitBanner.test.tsx b/test/unit/client/components/TerminalView.exitBanner.test.tsx index 3c78d5af9..de929a648 100644 --- a/test/unit/client/components/TerminalView.exitBanner.test.tsx +++ b/test/unit/client/components/TerminalView.exitBanner.test.tsx @@ -6,7 +6,7 @@ import tabsReducer from '@/store/tabsSlice' import panesReducer from '@/store/panesSlice' import settingsReducer, { defaultSettings } from '@/store/settingsSlice' import connectionReducer from '@/store/connectionSlice' -import terminalLifecycleReducer, { AUTO_RESUME_NOTICE_TTL_MS, selectExitRecordFrom } from '@/store/terminalLifecycleSlice' +import terminalLifecycleReducer, { selectExitRecordFrom } from '@/store/terminalLifecycleSlice' import { updatePaneContent } from '@/store/panesSlice' import { resetPersistedLayoutCacheForTests, resetPersistFlushListenersForTests } from '@/store/persistMiddleware' import type { PaneNode, TerminalPaneContent } from '@/store/paneTypes' @@ -95,6 +95,7 @@ class MockResizeObserver { } let messageHandler: ((msg: any) => void) | null = null +let reconnectHandler: (() => void) | null = null let requestAnimationFrameSpy: ReturnType | null = null let cancelAnimationFrameSpy: ReturnType | null = null @@ -119,10 +120,11 @@ interface StoreOptions { mode?: string status?: TerminalPaneContent['status'] withSessionRef?: boolean + crashTrace?: { exitCode: number; resumedAtMs: number } lifecycle?: { lastTerminalId?: string exit?: { exitCode: number; at: number } - notice?: { kind: 'recovering' | 'resumed'; attempt: number; maxAttempts: number; exitCode: number; at: number } + notice?: { kind: 'recovering'; attempt: number; maxAttempts: number; exitCode: number; at: number } } } @@ -134,6 +136,7 @@ function makeStore(opts: StoreOptions = {}) { status: opts.status ?? 'exited', mode: mode as TerminalPaneContent['mode'], shell: 'system', + ...(opts.crashTrace ? { crashTrace: opts.crashTrace } : {}), ...(opts.withSessionRef === false ? {} : { sessionRef: { provider: mode, sessionId: SESSION_ID } }), @@ -199,6 +202,12 @@ describe('TerminalView exited-pane error banner', () => { messageHandler = callback return () => { messageHandler = null } }) + wsMocks.onReconnect.mockImplementation((callback: () => void) => { + reconnectHandler = callback + return () => { + reconnectHandler = null + } + }) requestAnimationFrameSpy = vi.spyOn(window, 'requestAnimationFrame').mockImplementation((cb: FrameRequestCallback) => { cb(0) return 1 @@ -360,36 +369,218 @@ describe('TerminalView exited-pane error banner', () => { expect(screen.queryByRole('alert')).toBeNull() }) - it('degrades an orphaned recovering notice to the alert deterministically at TTL expiry (silent settle backstop)', async () => { + it('keeps the recovering notice up while the socket stays connected without a settle frame (no timer degradation — znhn#6 pin)', async () => { + // Scope (D-3, validated): "no timer degradation" holds WHILE CONNECTED. + // A disconnect/reconnect clears stale notices via the reconnect backstop + // (tested below) — missed-frame paths always pass through a reconnect. vi.useFakeTimers() const at = Date.now() const { store, paneContent } = makeStore({ mode: 'claude', status: 'exited', lifecycle: { + lastTerminalId: 'term-crashed', exit: { exitCode: 1, at }, notice: { kind: 'recovering', attempt: 1, maxAttempts: 2, exitCode: 1, at }, }, }) await renderPane(store, paneContent) - // While the notice is active: notice strip, no alert. (Text-anchored: - // TerminalView also renders an unrelated role='status' offline strip in - // this harness; the banner's role='status' semantics are covered by - // TerminalExitBanner.test.tsx.) + await act(async () => { + vi.advanceTimersByTime(120_000) + }) + expect(screen.getByText('claude crashed (exit 1) — auto-resuming, attempt 1/2')).toBeInTheDocument() + expect(screen.queryByRole('alert')).toBeNull() + vi.useRealTimers() + }) + + it('an out-of-order exited settle frame never touches pane content or status (D-1 allowlist pin)', async () => { + // Cross-channel ordering (broadcast settle vs per-connection + // terminal.exit) is NOT guaranteed (unbiased select!, + // terminal.rs:325-334): the settle handler is lifecycle-only, and the + // running|recovering content-write allowlist must block 'exited' from + // pane content/status. + const { store, paneContent } = makeStore({ + mode: 'claude', + status: 'running', + lifecycle: { lastTerminalId: 'term-live' }, + }) + const contentWithTid = { ...paneContent, terminalId: 'term-live' } + act(() => { + store.dispatch(updatePaneContent({ tabId: TAB, paneId: PANE, content: contentWithTid })) + }) + await renderPane(store, paneState(store)) + + await act(async () => { + messageHandler!({ type: 'terminal.status', terminalId: 'term-live', status: 'exited', reason: 'retries_exhausted' }) + }) + + const content = paneState(store) + expect(content.status).toBe('running') + expect(content.terminalId).toBe('term-live') expect(screen.queryByRole('alert')).toBeNull() + }) + + it('a recovering notice does not survive a reconnect (D-3 backstop pin)', async () => { + const at = Date.now() + const { store, paneContent } = makeStore({ + mode: 'claude', + status: 'exited', + lifecycle: { + lastTerminalId: 'term-crashed', + exit: { exitCode: 1, at }, + notice: { kind: 'recovering', attempt: 1, maxAttempts: 2, exitCode: 1, at }, + }, + }) + await renderPane(store, paneContent) expect(screen.getByText('claude crashed (exit 1) — auto-resuming, attempt 1/2')).toBeInTheDocument() + expect(reconnectHandler).not.toBeNull() - // No frame ever arrives (respawn_failed / lease-held / owned-live settle - // silently). The scheduled re-render must flip notice → alert on its own. await act(async () => { - vi.advanceTimersByTime(AUTO_RESUME_NOTICE_TTL_MS + 2) + reconnectHandler!() + }) + expect(screen.queryByText(/auto-resuming/)).toBeNull() + }) + + it('clears the recovering notice the moment the settle frame arrives', async () => { + const at = Date.now() + const { store, paneContent } = makeStore({ + mode: 'claude', + status: 'exited', + lifecycle: { + lastTerminalId: 'term-crashed', + exit: { exitCode: 1, at }, + notice: { kind: 'recovering', attempt: 1, maxAttempts: 2, exitCode: 1, at }, + }, }) + await renderPane(store, paneContent) - expect(screen.queryByText('claude crashed (exit 1) — auto-resuming, attempt 1/2')).toBeNull() + await act(async () => { + messageHandler!({ type: 'terminal.status', terminalId: 'term-crashed', status: 'exited', reason: 'pane_closed' }) + }) + expect(screen.queryByText(/auto-resuming/)).toBeNull() expect(screen.getByRole('alert')).toHaveTextContent('process exited (code 1)') }) + it('terminal.replaced writes a persistent crash trace onto pane content and shows the trace strip', async () => { + const at = Date.now() + const { store, paneContent } = makeStore({ + mode: 'claude', + status: 'running', + lifecycle: { + lastTerminalId: 'term-crashed', + notice: { kind: 'recovering', attempt: 1, maxAttempts: 2, exitCode: 1, at }, + }, + }) + const { rerender } = render( + + + + ) + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) + expect(messageHandler).not.toBeNull() + + await act(async () => { + messageHandler!({ type: 'terminal.replaced', oldTerminalId: 'term-crashed', newTerminalId: 'term-new', exitCode: 1, attempt: 1, maxAttempts: 2 }) + }) + + // The store now carries it on pane CONTENT (the persisted home): + const content = paneState(store) + expect(content.crashTrace?.exitCode).toBe(1) + expect(typeof content.crashTrace?.resumedAtMs).toBe('number') + + // Re-render with the updated content (in production the parent passes + // fresh store content on every render). + rerender( + + + + ) + const trace = screen.getByTestId('crash-trace') + expect(trace).toHaveTextContent(/claude crashed \(exit 1\) & auto-resumed at \d{2}:\d{2}/) + expect(trace).toHaveAttribute('role', 'status') + expect(screen.queryByRole('alert')).toBeNull() + }) + + it('dismissing the crash trace clears it from pane content', async () => { + const { store, paneContent } = makeStore({ + mode: 'claude', + status: 'running', + crashTrace: { exitCode: 1, resumedAtMs: Date.now() }, + }) + const { rerender } = render( + + + + ) + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) + + expect(screen.getByTestId('crash-trace')).toBeInTheDocument() + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: 'Dismiss claude crash notice' })) + }) + expect(paneState(store).crashTrace).toBeUndefined() + rerender( + + + + ) + expect(screen.queryByTestId('crash-trace')).toBeNull() + }) + + it('cancel button sends terminal.autoResumeCancel with the old terminal id', async () => { + const at = Date.now() + const { store, paneContent } = makeStore({ + mode: 'claude', + status: 'exited', + lifecycle: { + lastTerminalId: 'term-crashed', + exit: { exitCode: 1, at }, + notice: { kind: 'recovering', attempt: 1, maxAttempts: 2, exitCode: 1, at }, + }, + }) + await renderPane(store, paneContent) + + fireEvent.click(screen.getByRole('button', { name: 'Cancel auto-resume for claude' })) + expect(wsMocks.send).toHaveBeenCalledWith({ type: 'terminal.autoResumeCancel', terminalId: 'term-crashed' }) + }) + + it('renders the circuit-breaker banner from the typed resumeCycles settle field (znhn#2)', async () => { + const at = Date.now() + const { store, paneContent } = makeStore({ + mode: 'claude', + status: 'exited', + lifecycle: { + lastTerminalId: 'term-crashed', + exit: { exitCode: 1, at }, + notice: { kind: 'recovering', attempt: 1, maxAttempts: 2, exitCode: 1, at }, + }, + }) + await renderPane(store, paneContent) + + await act(async () => { + messageHandler!({ + type: 'terminal.status', + terminalId: 'term-crashed', + status: 'exited', + reason: 'flap_circuit_breaker', + resumeCycles: 3, + }) + }) + + expect(screen.getByRole('alert')).toHaveTextContent('claude crashed 3 times — auto-resume paused') + // canResume derives from the seeded sessionRef (provider matches mode): + // the button copy is honest about resuming this conversation. + expect(screen.getByRole('button', { name: 'Relaunch claude session' })) + .toHaveTextContent('Relaunch — resumes this conversation') + }) + it('renders the recovering notice from the frame FIELDS — prose is presentational, never parsed', async () => { // Council MEDIUM fix (7w4h/xkhx review): the client must read // attempt/maxAttempts/exitCode from the terminal.status frame's typed diff --git a/test/unit/client/store/panesPersistence.test.ts b/test/unit/client/store/panesPersistence.test.ts index f8a1f269e..8ad329d48 100644 --- a/test/unit/client/store/panesPersistence.test.ts +++ b/test/unit/client/store/panesPersistence.test.ts @@ -424,6 +424,58 @@ describe('Panes Persistence Integration', () => { expect(restored.resumeSessionId).toBeUndefined() // always stripped; re-derived from sessionRef at create time }) + it('crashTrace persists across a panes round-trip (denylist keeps new fields)', () => { + // znhn item 1: the crash trace lives on pane content BECAUSE the + // pane-content persistence strip is a denylist — a new field persists by + // default with no persistMiddleware change. This pins that property. + const store1 = configureStore({ + reducer: { + tabs: tabsReducer, + panes: panesReducer, + }, + middleware: (getDefault) => getDefault().concat(persistMiddleware as any), + }) + + store1.dispatch(addTab({ mode: 'claude' })) + const tabId = store1.getState().tabs.tabs[0].id + store1.dispatch(initLayout({ + tabId, + content: { + kind: 'terminal', + mode: 'claude', + shell: 'system', + createRequestId: 'req-trace', + status: 'running', + crashTrace: { exitCode: 1, resumedAtMs: 1_753_760_220_000 }, + } as any, + })) + + vi.runAllTimers() + const persistedTabs = loadPersistedTabs() + const persistedPanes = loadPersistedPanes() + + const store2 = configureStore({ + reducer: { + tabs: tabsReducer, + panes: panesReducer, + }, + middleware: (getDefault) => getDefault().concat(persistMiddleware as any), + }) + if (persistedTabs?.tabs) { + store2.dispatch(hydrateTabs(persistedTabs.tabs)) + } + if (persistedPanes) { + store2.dispatch(hydratePanes(persistedPanes)) + } + + const restoredLayout = store2.getState().panes.layouts[tabId] + expect(restoredLayout).toBeDefined() + expect(restoredLayout.type).toBe('leaf') + const restored = (restoredLayout as any).content + expect(restored.kind).toBe('terminal') + expect(restored.crashTrace).toEqual({ exitCode: 1, resumedAtMs: 1_753_760_220_000 }) + }) + it('flushes pending writes on visibility change', () => { const store = configureStore({ reducer: { diff --git a/test/unit/client/store/panesSlice.reconcile.test.ts b/test/unit/client/store/panesSlice.reconcile.test.ts index 27c663054..f124e6456 100644 --- a/test/unit/client/store/panesSlice.reconcile.test.ts +++ b/test/unit/client/store/panesSlice.reconcile.test.ts @@ -135,6 +135,21 @@ describe('reconcile reducers', () => { expect(c.reconcileNotice).toBe('Started fresh (identity_never_observed).') }) + it('resetPaneForReconcileCreate(fresh) clears a stale crashTrace; respawn keeps it (znhn#1)', () => { + // Fresh-eyes finding: fresh = a genuinely NEW identity-less conversation — + // the old "crashed & auto-resumed" trace belongs to the retired session + // and must not leak onto it. Respawn resumes the SAME conversation, so + // its trace legitimately stays. + const trace = { exitCode: 1, resumedAtMs: 1_753_760_220_000 } + const freshState = stateWithTerminalPane({ crashTrace: trace, sessionRef: { provider: 'claude', sessionId: 'gone' } }) + const fresh = panesReducer(freshState, resetPaneForReconcileCreate({ tabId: 'tab1', paneId: 'p1', intent: 'fresh', reason: 'identity_never_observed' })) + expect(terminalContent(fresh, 'tab1', 'p1').crashTrace).toBeUndefined() + + const respawnState = stateWithTerminalPane({ crashTrace: trace, sessionRef: { provider: 'claude', sessionId: 'keep' } }) + const respawn = panesReducer(respawnState, resetPaneForReconcileCreate({ tabId: 'tab1', paneId: 'p1', intent: 'respawn', sessionRef: { provider: 'claude', sessionId: 'keep' } })) + expect(terminalContent(respawn, 'tab1', 'p1').crashTrace).toEqual(trace) + }) + it('resetPaneForReconcileCreate(respawn) with provider mismatch degrades loudly to fresh', () => { const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) try { diff --git a/test/unit/client/store/terminalLifecycleSlice.test.ts b/test/unit/client/store/terminalLifecycleSlice.test.ts index 7c410a9ce..46bf330df 100644 --- a/test/unit/client/store/terminalLifecycleSlice.test.ts +++ b/test/unit/client/store/terminalLifecycleSlice.test.ts @@ -1,9 +1,10 @@ import { describe, it, expect } from 'vitest' import reducer, { recordTerminalExit, recordAutoResumeRecovering, foldTerminalReplacement, - clearTerminalLifecycle, selectExitRecordFrom, selectActiveNoticeFrom, + clearTerminalLifecycle, recordAutoResumeSettled, clearRecoveringNotices, + selectExitRecordFrom, selectActiveNoticeFrom, selectLastTerminalIdFrom, selectExitRecord, selectActiveNotice, - AUTO_RESUME_NOTICE_TTL_MS, + selectResumeCycles, } from '@/store/terminalLifecycleSlice' const empty = reducer(undefined, { type: '@@init' }) @@ -15,28 +16,63 @@ describe('terminalLifecycleSlice', () => { expect(selectLastTerminalIdFrom(s, 'p1')).toBe('t1') // frame-matching key survives TerminalView clearing its own terminalId }) - it('records a recovering notice and expires it after the TTL', () => { - const s = reducer(empty, recordAutoResumeRecovering({ paneId: 'p1', attempt: 1, maxAttempts: 2, exitCode: 1, at: 1000 })) - expect(selectActiveNoticeFrom(s, 'p1', 1000 + AUTO_RESUME_NOTICE_TTL_MS - 1)?.kind).toBe('recovering') - expect(selectActiveNoticeFrom(s, 'p1', 1000 + AUTO_RESUME_NOTICE_TTL_MS + 1)).toBeUndefined() + it('selectActiveNoticeFrom returns the notice with no TTL — settles are frame-driven', () => { + // znhn item 3: the 30s TTL guessing apparatus is deleted; a notice stays + // active until a settle/replaced frame (or reconnect backstop) clears it. + const s = reducer(empty, recordAutoResumeRecovering({ paneId: 'p1', attempt: 1, maxAttempts: 2, exitCode: 1, at: 0 })) + expect(selectActiveNoticeFrom(s, 'p1')?.kind).toBe('recovering') }) - it('fold clears the exit record, sets a resumed notice, and advances lastTerminalId', () => { + it('recordAutoResumeSettled clears the notice and records resumeCycles', () => { + let s = reducer(empty, recordAutoResumeRecovering({ paneId: 'p1', attempt: 1, maxAttempts: 2, exitCode: 1, at: 1000 })) + s = reducer(s, recordAutoResumeSettled({ paneId: 'p1', resumeCycles: 3, at: 2000 })) + expect(selectActiveNoticeFrom(s, 'p1')).toBeUndefined() + expect(selectResumeCycles({ terminalLifecycle: s }, 'p1')).toBe(3) + }) + + it('clearRecoveringNotices clears every recovering notice (D-3 reconnect backstop)', () => { + let s = reducer(empty, recordAutoResumeRecovering({ paneId: 'p1', attempt: 1, maxAttempts: 2, exitCode: 1, at: 1000 })) + s = reducer(s, recordAutoResumeRecovering({ paneId: 'p2', attempt: 2, maxAttempts: 2, exitCode: 137, at: 1000 })) + s = reducer(s, recordTerminalExit({ paneId: 'p3', terminalId: 't3', exitCode: 1, at: 1000 })) + s = reducer(s, recordAutoResumeSettled({ paneId: 'p4', resumeCycles: 5, at: 1000 })) + s = reducer(s, clearRecoveringNotices()) + expect(selectActiveNoticeFrom(s, 'p1')).toBeUndefined() + expect(selectActiveNoticeFrom(s, 'p2')).toBeUndefined() + // exit + settle records are untouched — only notices clear. + expect(selectExitRecordFrom(s, 'p3')).toEqual({ exitCode: 1, at: 1000 }) + expect(selectResumeCycles({ terminalLifecycle: s }, 'p4')).toBe(5) + }) + + it('recordTerminalExit clears prior settle state (stale resumeCycles cannot leak into a later crash banner)', () => { + let s = reducer(empty, recordAutoResumeSettled({ paneId: 'p1', resumeCycles: 5, at: 1 })) + s = reducer(s, recordTerminalExit({ paneId: 'p1', terminalId: 't1', exitCode: 1, at: 2 })) + expect(selectResumeCycles({ terminalLifecycle: s }, 'p1')).toBeUndefined() + }) + + it('foldTerminalReplacement clears the notice (the persistent crash trace replaces the resumed strip)', () => { + // znhn item 1: the 'resumed' notice kind is retired — the dismissible + // crash trace on pane content is the post-resume indicator. let s = reducer(empty, recordTerminalExit({ paneId: 'p1', terminalId: 't1', exitCode: 1, at: 1000 })) s = reducer(s, recordAutoResumeRecovering({ paneId: 'p1', attempt: 1, maxAttempts: 2, exitCode: 1, at: 1000 })) s = reducer(s, foldTerminalReplacement({ paneId: 'p1', newTerminalId: 't2', exitCode: 1, attempt: 1, maxAttempts: 2, at: 2000 })) expect(selectExitRecordFrom(s, 'p1')).toBeUndefined() // pane is alive again — no error bar - expect(selectActiveNoticeFrom(s, 'p1', 2000)).toEqual({ kind: 'resumed', attempt: 1, maxAttempts: 2, exitCode: 1, at: 2000 }) + expect(selectActiveNoticeFrom(s, 'p1')).toBeUndefined() expect(selectLastTerminalIdFrom(s, 'p1')).toBe('t2') }) - it('a later exit clears any active notice (exhaustion must not be masked by a stale resumed strip)', () => { - // fold sets a 'resumed' notice; the replacement then crashes and the hub - // settles retries_exhausted WITHOUT emitting any frame — the exit record - // must surface the alert immediately, not after the 30s TTL. - let s = reducer(empty, foldTerminalReplacement({ paneId: 'p1', newTerminalId: 't2', exitCode: 1, attempt: 2, maxAttempts: 2, at: 1000 })) + it('a replacement clears prior settle state (stale resumeCycles cannot leak into a later crash banner)', () => { + // Pairs with the recordTerminalExit pin above (validated A15): nothing + // else ever deletes the settle state, and the REST-door relaunch/ + // reconcile never advances lastTerminalId. + let s = reducer(empty, recordAutoResumeSettled({ paneId: 'p1', resumeCycles: 5, at: 1 })) + s = reducer(s, foldTerminalReplacement({ paneId: 'p1', newTerminalId: 't2', exitCode: 1, attempt: 1, maxAttempts: 2, at: 2000 })) + expect(selectResumeCycles({ terminalLifecycle: s }, 'p1')).toBeUndefined() + }) + + it('a later exit clears any active notice (exhaustion must not be masked by a stale strip)', () => { + let s = reducer(empty, recordAutoResumeRecovering({ paneId: 'p1', attempt: 1, maxAttempts: 2, exitCode: 1, at: 1000 })) s = reducer(s, recordTerminalExit({ paneId: 'p1', terminalId: 't2', exitCode: 1, at: 2000 })) - expect(selectActiveNoticeFrom(s, 'p1', 2000)).toBeUndefined() + expect(selectActiveNoticeFrom(s, 'p1')).toBeUndefined() expect(selectExitRecordFrom(s, 'p1')).toEqual({ exitCode: 1, at: 2000 }) }) @@ -47,10 +83,11 @@ describe('terminalLifecycleSlice', () => { // mirroring the paneRuntimeActivity defensive-access convention. const bare = {} as Parameters[0] expect(selectExitRecord(bare, 'p1')).toBeUndefined() - expect(selectActiveNotice(bare, 'p1', Date.now())).toBeUndefined() + expect(selectActiveNotice(bare, 'p1')).toBeUndefined() + expect(selectResumeCycles(bare, 'p1')).toBeUndefined() expect(selectExitRecordFrom(undefined, 'p1')).toBeUndefined() expect(selectLastTerminalIdFrom(undefined, 'p1')).toBeUndefined() - expect(selectActiveNoticeFrom(undefined, 'p1', 0)).toBeUndefined() + expect(selectActiveNoticeFrom(undefined, 'p1')).toBeUndefined() }) it('clearTerminalLifecycle wipes the pane entry', () => {