From 5cb67a57ba465f00a4ba267b963b65e25d9ea90b Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Wed, 29 Jul 2026 14:21:16 -0700 Subject: [PATCH 01/17] docs: add implementation plan for znhn-bccd-followups --- docs/plans/2026-07-29-znhn-bccd-followups.md | 1894 ++++++++++++++++++ 1 file changed, 1894 insertions(+) create mode 100644 docs/plans/2026-07-29-znhn-bccd-followups.md 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..569f591e7 --- /dev/null +++ b/docs/plans/2026-07-29-znhn-bccd-followups.md @@ -0,0 +1,1894 @@ +# 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, 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 acquires a permit. + +**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, paneId)` traversal helper (`panesSlice.ts:618`). +- Produces (used by Tasks 11–12): `export type CrashTrace = { exitCode: number; resumedAtMs: number }` in `paneTypes.ts`; `TerminalPaneContent.crashTrace?: CrashTrace`; actions `setPaneCrashTrace({ paneId, crashTrace })` and `clearPaneCrashTrace({ paneId })`; 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 + }) +``` + +- [ ] **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 `setPaneReconcileNotice`/`clearPaneReconcileNotice` at `:2080-2096` exactly, same traversal helper): + +```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<{ paneId: string; crashTrace: CrashTrace }> + ) { + const content = findReconcileTerminalContent(state, action.payload.paneId) + if (content) content.crashTrace = action.payload.crashTrace + }, + clearPaneCrashTrace(state, action: PayloadAction<{ paneId: string }>) { + const content = findReconcileTerminalContent(state, action.payload.paneId) + if (content && content.crashTrace) delete content.crashTrace + }, +``` +(Match the helper's real signature — if `clearPaneReconcileNotice` calls it differently, copy that call shape.) 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). 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({ + paneId: paneIdRef.current, + crashTrace: { exitCode: msg.exitCode, resumedAtMs: Date.now() }, + }) + ) +``` +`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({ 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) + } + }) +``` + +- [ ] **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 now acquires a single spawn-gate permit in the REST send-keys cold path (rejection reuses the 429/503 envelopes); the WS materialize door stays deliberately ungated (D-D ruled on REST-reachable forks; single-flighted singleton bounds it — comment in opencode_ws.rs). (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 | +| 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 + frame-driven clear; 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; kata comment records the supersession | +| bccd 4 sidecar cold-start gating (evaluate-and-decide) | 4 | DECIDED: gate it (D-7); tests pin 429-when-full and queue-behind-held-permit; WS door deliberately out of scope with comment | +| 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 (znhn 6, bccd 4) are decided and implemented/pinned in-plan. **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; NoopSpawner for the sidecar gate rejection — the gate rejection fires before any spawn, and the queue-behind-permit test proves the door is on the gate). The WS-door opencode cold-start is a deliberate, documented scope decision (council D-D ruled on REST), not a silent deferral — recorded in code comment + kata comment. + +**2. Placeholder scan:** the remaining `` / "KEEP the existing payload" markers in Tasks 3 and 4 are deliberate **splice anchors into existing test bodies quoted by line number** — the implementer copies working in-repo code rather than this plan duplicating (and drifting from) it; every NEW behavior has complete code. Test skeletons in Tasks 6–7 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 Tasks 3, 4 with matching signatures. ✓ +- `spawn_gate_error_response(err, retry_after: Duration)` `pub(crate)` — Task 2 defines, Task 4 consumes with `(err, g.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. ✓ +- `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). + From d0e9bfb5d763cc70fb9d4ffa57e8b8fe089d8ac9 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Wed, 29 Jul 2026 15:15:21 -0700 Subject: [PATCH 02/17] docs(plan): incorporate load-bearing validation findings (znhn-bccd-followups) Validation wave results (7 verified, 6 falsified, 2 accepted-deferred): - D-7 REVERSED: do not gate the opencode cold-start (A10: worst-case permit hold ~50-70s vs 10s door waits; singleton already bounds forks) - D-3/Task 9: reconnect backstop clears stale recovering notices (A3: missed broadcast frames were a forever-lying notice) - D-4/Task 8: registry-validate cancel ids; take_cancel emits the settle frame (A5: poisoned-flag path defused) - D-1/Task 9: corrected clean-exit rationale; allowlist pinned as the out-of-order barrier (A1/A6) - Task 7: thread cfg.healthy_lifetime_ms into the eviction condition; between-thresholds test (A8) - Tasks 9/10: clear settle state on exit/replacement (A15 stale resumeCycles leak) - Notes: Task 3 byte-identical payload (A9), Task 12 knob margins (A7), D-2 createRequestId verification record (A2) --- docs/plans/2026-07-29-znhn-bccd-followups.md | 321 +++++++++++-------- 1 file changed, 190 insertions(+), 131 deletions(-) diff --git a/docs/plans/2026-07-29-znhn-bccd-followups.md b/docs/plans/2026-07-29-znhn-bccd-followups.md index 569f591e7..066137baa 100644 --- a/docs/plans/2026-07-29-znhn-bccd-followups.md +++ b/docs/plans/2026-07-29-znhn-bccd-followups.md @@ -5,9 +5,9 @@ > 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, D-C revisit tripwire). +**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 acquires a permit. +**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). @@ -30,31 +30,31 @@ ### D-1. Settle frame shape (znhn item 3) -`RuntimeStatus` gains a third variant `Exited` (serialized `"exited"`), carried on the **existing** `terminal.status` frame — no new server frame type, so `SERVER_MESSAGE_TYPES` stays at 57. `TerminalStatus` already has optional `reason`/`exitCode`/`attempt`/`maxAttempts` fields; it gains one optional `resumeCycles` (breaker settles only). The frame is broadcast with the OLD (crashed) terminal id — the client already matches old ids via `selectLastTerminalIdFrom` (`TerminalView.tsx:4357`). Every settle path for agent-mode events emits it (uniform "every settle is loud"); the clean-exit settle frame is harmless client-side (no notice exists; the alert condition requires a non-zero exit record). +`RuntimeStatus` gains a third variant `Exited` (serialized `"exited"`), carried on the **existing** `terminal.status` frame — no new server frame type, so `SERVER_MESSAGE_TYPES` stays at 57. `TerminalStatus` already has optional `reason`/`exitCode`/`attempt`/`maxAttempts` fields; it gains one optional `resumeCycles` (breaker settles only). The frame is broadcast with the OLD (crashed) terminal id — the client already matches old ids via `selectLastTerminalIdFrom` (`TerminalView.tsx:4357`). Every settle path for agent-mode events emits it (uniform "every settle is loud"). The clean-exit settle frame is harmless client-side for VERIFIED reasons (validation A1/A6): the settle-frame handler is lifecycle-slice-only (content-inert), and the client's `terminal.status` content-write allowlist (`TerminalView.tsx:4378-4381`, admits only `running|recovering`) blocks `'exited'` from ever touching pane content or pane status; the only writer of `'exited'` pane status (`TerminalView.tsx:4487`) dispatches `recordTerminalExit` (`:4432`) FIRST in the same synchronous handler, so a clean exit records code 0 before any alert evaluation — the codeless-alert branch (`TerminalView.tsx:5232-5239`, which fires when NO exit record exists) is unreachable from a settle frame alone. Stated plainly: cross-channel ordering (`terminal.exit` on the per-connection sink vs the settle frame on the broadcast bus, merged by an unbiased `select!` — `terminal.rs:325-334`) is NOT guaranteed, so the content-write allowlist is the load-bearing barrier — Task 9 pins it with an out-of-order settle-frame test. ### D-2. Flap circuit breaker thresholds (znhn item 2) -A **bounded circuit breaker**, not escalating backoff: the hub is fully serialized (one task; a backoff sleep delays every pane's resume — `auto_resume.rs:213-217`), so per-pane escalating sleeps of 30s+ would starve all other panes. Chosen semantics: a **cycle** = one successful auto-resume (a `terminal.replaced` emission). Cycles are recorded per `createRequestId` with wall-clock timestamps, pruned to a rolling window at each crash, and **never reset by healthy generations** (that is the cross-reset bound the council asked for — it also bounds the out-of-band-`kill` resurrection loop, which is indistinguishable from a crash). Defaults: **5 cycles per rolling hour** (`AUTO_RESUME_DEFAULT_MAX_CYCLES = 5`, `AUTO_RESUME_DEFAULT_CYCLE_WINDOW_MS = 3_600_000`), env-overridable (`FRESHELL_AUTO_RESUME_MAX_CYCLES`, `FRESHELL_AUTO_RESUME_CYCLE_WINDOW_MS`) for e2e. When a crash arrives with cycles ≥ max, `decide` settles `flap_circuit_breaker`; the settle frame carries `resumeCycles` and the client renders "`` crashed N times — auto-resume paused". Relaunch stays available; a re-crash after manual relaunch re-settles immediately until the window drains — bounded and loud, never infinite and silent (user ruling). For e2e testability two more knobs become env-overridable: the hub healthy-lifetime (`FRESHELL_AUTO_RESUME_HEALTHY_LIFETIME_MS`, default 30_000) and the registry respawn liveness window (`FRESHELL_RESPAWN_LIVENESS_WINDOW_MS`, wired in `main.rs` through the existing `pub fn set_respawn_liveness_window_ms` setter, `registry.rs:743`) — without the latter, sub-30s flap cycles trip the registry generation cap (3) before the breaker can ever fire. +A **bounded circuit breaker**, not escalating backoff: the hub is fully serialized (one task; a backoff sleep delays every pane's resume — `auto_resume.rs:213-217`), so per-pane escalating sleeps of 30s+ would starve all other panes. Chosen semantics: a **cycle** = one successful auto-resume (a `terminal.replaced` emission). Cycles are recorded per `createRequestId` with wall-clock timestamps, pruned to a rolling window at each crash, and **never reset by healthy generations** (that is the cross-reset bound the council asked for — it also bounds the out-of-band-`kill` resurrection loop, which is indistinguishable from a crash). Defaults: **5 cycles per rolling hour** (`AUTO_RESUME_DEFAULT_MAX_CYCLES = 5`, `AUTO_RESUME_DEFAULT_CYCLE_WINDOW_MS = 3_600_000`), env-overridable (`FRESHELL_AUTO_RESUME_MAX_CYCLES`, `FRESHELL_AUTO_RESUME_CYCLE_WINDOW_MS`) for e2e. When a crash arrives with cycles ≥ max, `decide` settles `flap_circuit_breaker`; the settle frame carries `resumeCycles` and the client renders "`` crashed N times — auto-resume paused". Relaunch stays available; a re-crash after manual relaunch re-settles immediately until the window drains — bounded and loud, never infinite and silent (user ruling). For e2e testability two more knobs become env-overridable: the hub healthy-lifetime (`FRESHELL_AUTO_RESUME_HEALTHY_LIFETIME_MS`, default 30_000) and the registry respawn liveness window (`FRESHELL_RESPAWN_LIVENESS_WINDOW_MS`, wired in `main.rs` through the existing `pub fn set_respawn_liveness_window_ms` setter, `registry.rs:743`) — without the latter, sub-30s flap cycles trip the registry generation cap (3) before the breaker can ever fire. Verification record (A2): manual Relaunch preserves `createRequestId` — `resetPaneForReconcileCreate` keeps it (`panesSlice.ts:1932-1934`, "PRESERVING createRequestId (D4)") and the WS create stamps it onto the new generation (`terminal.rs:2247` → `:2260`) — so breaker history survives manual Relaunch, as this design requires. Opencode durable-replacement and recovery-plan restore mint fresh ids — genuinely new identities; the budget refill there is accepted. ### D-3. Persistent crash trace replaces the ephemeral "resumed" strip (znhn item 1) -The trace lives on **pane content** (`TerminalPaneContent.crashTrace`) because pane-content persistence is a **denylist** (`stripTransientSessionFields` spreads `...rest`) — a new field persists automatically, no `persistMiddleware` change, no `PANES_SCHEMA_VERSION` bump (schemas use `.passthrough()`; absent = safe `undefined`). The ephemeral `terminalLifecycle` slice cannot host it (slice-level persistence is an allowlist that deliberately excludes it). The `kind: 'resumed'` notice is retired — `foldTerminalReplacement` now clears the notice and the trace is the post-resume indicator. The trace renders as `role="status"` + `data-testid="crash-trace"` (NOT `role="alert"` — four e2e tests assert `getByRole('alert')).toHaveCount(0)` on the happy path). With settle frames on every path (D-1) and terminal.exit already clearing notices, the recovering notice is fully frame-driven, so **both halves of the 30s TTL apparatus are deleted** (selector filter + `TerminalView` re-render timer + the `AUTO_RESUME_NOTICE_TTL_MS` constant). +The trace lives on **pane content** (`TerminalPaneContent.crashTrace`) because pane-content persistence is a **denylist** (`stripTransientSessionFields` spreads `...rest`) — a new field persists automatically, no `persistMiddleware` change, no `PANES_SCHEMA_VERSION` bump (schemas use `.passthrough()`; absent = safe `undefined`). The ephemeral `terminalLifecycle` slice cannot host it (slice-level persistence is an allowlist that deliberately excludes it). The `kind: 'resumed'` notice is retired — `foldTerminalReplacement` now clears the notice and the trace is the post-resume indicator. The trace renders as `role="status"` + `data-testid="crash-trace"` (NOT `role="alert"` — four e2e tests assert `getByRole('alert')).toHaveCount(0)` on the happy path). With settle frames on every path (D-1) and terminal.exit already clearing notices, the recovering notice is fully frame-driven, so **both halves of the 30s TTL apparatus are deleted** (selector filter + `TerminalView` re-render timer + the `AUTO_RESUME_NOTICE_TTL_MS` constant). **Backstop decision (added at validation — A3 falsified the no-backstop design):** the settle/replaced frames ride a fire-and-forget bounded broadcast (`main.rs:210`, cap 1024, no replay), and lagged receivers are force-closed with 4008 (`terminal.rs:407-423`) — so a missed frame is possible and, uncorrected, a stale recovering notice would lie FOREVER (it masks the exit alert, `TerminalExitBanner.tsx:16`). Because every missed-frame path necessarily passes through a WS reconnect (lag forces a close; disconnects reconnect), the client clears stale recovering notices on WS reconnect (`clearRecoveringNotices`, Task 9). Frames stay the primary mechanism; reconnect-clear is the backstop; no TTL returns. ### D-4. Cancel affordance flow (znhn item 2) -New client message `terminal.autoResumeCancel { terminalId }` (the OLD terminal id from the recovering frame). The WS handler (a) inserts the id into a new `WsState.auto_resume_cancels` set and (b) **broadcasts the settle frame immediately** (reason `"auto-resume cancelled"`) so the notice clears on click, not after the backoff sleep. The hub's post-sleep guard consumes the flag first (`take_cancel`) and settles silently (log only — the frame already went out). A cancel with no pending resume leaves a small string in the set (bounded: one entry per click, consumed on the next crash of that id or never) — accepted. The Rust `ClientMessage` match is exhaustive with no catch-all, so the new variant, its handler, the `WsState` field, protocol pins, and the Node-server case arm (`server/ws-handler.ts` `default:` sends `UNKNOWN_MESSAGE` — a no-op arm is required) all land in ONE commit (Task 8). +New client message `terminal.autoResumeCancel { terminalId }` (the OLD terminal id from the recovering frame). The WS handler **validates the terminalId against the registry first** (the kill-handler precedent): an unknown id is ignored with a log — no insert, no broadcast — which kills both unbounded client-controlled set growth and spoofed settle broadcasts (validation A5 falsified the no-validation design). For a known id the handler (a) inserts it into a new `WsState.auto_resume_cancels` set and (b) **broadcasts the settle frame immediately** (reason `"auto-resume cancelled"`) so the notice clears on click, not after the backoff sleep. The hub's post-sleep guard consumes the flag first (`take_cancel`) and **ALSO emits the settle frame** (same reason, `"auto-resume cancelled"` — idempotent client-side: `recordAutoResumeSettled` on an already-settled entry is a no-op-shaped overwrite) instead of settling silently: a late-consumed or pre-seeded cancel is always loud and can never strand a recovering notice (the poisoned-flag path — a cancel pre-seeded on a live id silently suppressing that terminal's FUTURE resume — is thereby defused). Residual: a validated cancel with no pending resume leaves one registry-known id in the set until the next crash of that id consumes it — bounded by the registry and always settled loudly when consumed. The Rust `ClientMessage` match is exhaustive with no catch-all, so the new variant, its handler, the `WsState` field, protocol pins, and the Node-server case arm (`server/ws-handler.ts` `default:` sends `UNKNOWN_MESSAGE` — a no-op arm is required) all land in ONE commit (Task 8). ### D-5. Retry-After (bccd item 1) -`Retry-After: ` header (the council's ask, HTTP convention) **plus** a `retryAfterMs` body field (house convention — session-lease `SESSION_RESERVED`; the MCP bridge surfaces only `message` text, so the prose hint stays too). Value: the gate wait bound `rest_gate.timeout` (default 10s) — the queue drains within roughly one timeout window; no other duration exists at the rejection point. Scope: the 429 (`SPAWN_QUEUE_FULL`) only, per the kata. `spawn_gate_error_response` gains a `retry_after: Duration` parameter and becomes `pub(crate)` (Task 4 reuses it). +`Retry-After: ` header (the council's ask, HTTP convention) **plus** a `retryAfterMs` body field (house convention — session-lease `SESSION_RESERVED`; the MCP bridge surfaces only `message` text, so the prose hint stays too). Value: the gate wait bound `rest_gate.timeout` (default 10s) — the queue drains within roughly one timeout window; no other duration exists at the rejection point. Scope: the 429 (`SPAWN_QUEUE_FULL`) only, per the kata. `spawn_gate_error_response` gains a `retry_after: Duration` parameter and stays private to `terminal_tabs.rs` (the planned `pub(crate)` widening existed solely for Task 4's gate reuse, dropped with D-7's reversal). ### D-6. Deterministic burst-test pin (bccd item 2) The flaky `queued_total() >= 8` is replaced by **pre-holding the gate's single permit before firing the burst**: while the budget is fully held, the fast path cannot fire, so `queued_total()` reaches **exactly 16** (deterministic), and **zero** requests may complete (`!h.is_finished()`) — that pins max-in-flight ≤ budget without probabilistic counters. This mirrors the established re-acquire precedent (`abort_burst_rest_creates_stay_gated...`, `terminal_tabs.rs:4104`). No gate instrumentation is added: a peak-in-flight gauge would require changing `acquire`'s return type to an RAII newtype across ~15 call/test sites — YAGNI. -### D-7. Opencode sidecar cold-start gating (bccd item 4) — DECISION: gate it +### D-7. Opencode sidecar cold-start gating (bccd item 4) — DECISION REVERSED at validation: do NOT gate -The sidecar cold-start is a REST-reachable process fork; the gate's semantics are "one server-wide budget for every fork door". The cold-start arm of `send_keys` (`crates/freshell-freshagent/src/lib.rs:1586-1596`) acquires **one** permit via `acquire_uncancellable` around `manager.create_session(...)` (which runs `ensure_started` = spawn + bounded health wait). Warm sends take the `durable_id` branch and never touch the gate. Singleton-ness (double-mutex single-flight) means at most one fork ever happens; the permit is held across a bounded wait — the known hazard (holding a permit while blocked on the `running` mutex held by a permit-less WS caller) is deadlock-free and bounded, accepted. The **WS** materialize door (`opencode_ws.rs:542`) stays ungated with a deliberate code comment: council D-D ruled on REST-reachable forks, and the shared singleton bounds it — recorded in the kata comment (Task 13). +The plan-time decision was to gate the cold-start arm of `send_keys`; validation (A10) FALSIFIED its load-bearing "bounded wait" premise with arithmetic. Worst-case permit hold ≈ **50–70s**: `health_timeout = 20_000ms` (`serve.rs:303`) + `request_timeout = 30_000ms` (`serve.rs:308,546` — the POST /session runs under the permit), and the serialized `running`-mutex queue adds ~20s per failing holder — vs the 10s gate waits at every other door. Worse, k cold first-sends would hold k permits while queued on the singleton mutex, so k ≥ budget cold panes starve ALL spawn doors. Meanwhile the double-mutex single-flight already bounds actual sidecar forks to AT MOST ONE server-wide — so the gate adds starvation without reducing fork concurrency. Moving the acquire inside the single-flight would invert lock order (cycle hazard), and plumbing the gate into `freshell-opencode`'s spawn site is a disproportionate cross-crate change. **Council D-D evaluate-and-decide outcome: the singleton bounds the fork; gating starves.** Both doors — the `send_keys` cold arm (`crates/freshell-freshagent/src/lib.rs:1586-1596`) AND the WS materialize door (`opencode_ws.rs:542`) — get deliberate code comments recording this decision + arithmetic (Task 4, comments-only); the kata comment records it too (Task 13). ### D-8. Cancel-sender pin test (bccd item 3) — superseded by construction @@ -62,7 +62,7 @@ After Task 1 the REST door holds **no cancel sender at all** — `acquire_uncanc ### D-9. Relaunch-mid-respawn orphan race (znhn item 6) — resolved structurally -The race's UI trigger was the notice TTL: with `FRESHELL_AUTO_RESUME_DELAYS_MS > 30000` the recovering notice expired before the backoff finished, exposing the alert bar (with Relaunch) while a respawn was still planned. Tasks 6+9 delete the TTL and make the notice frame-driven, so the alert bar can no longer appear while a resume is pending, at any delay value. A unit test pins this (Task 9, step 1), and the residual server-side ordering (REST-door relaunch during backoff) remains covered by the existing `session_owned_live` guard — which now also emits a settle frame. Kata comment records the resolution (Task 13). +The race's UI trigger was the notice TTL: with `FRESHELL_AUTO_RESUME_DELAYS_MS > 30000` the recovering notice expired before the backoff finished, exposing the alert bar (with Relaunch) while a respawn was still planned. Tasks 6+9 delete the TTL and make the notice frame-driven, so the alert bar can no longer appear while a resume is pending, at any delay value. A unit test pins this (Task 9, step 1), and the residual server-side ordering (REST-door relaunch during backoff) remains covered by the existing `session_owned_live` guard — which now also emits a settle frame. Kata comment records the resolution (Task 13). **Accepted residuals (recorded at validation, with the D-3 reconnect backstop):** (a) after a reconnect during a still-pending resume, the reconnect-clear removes the recovering notice, so the exit alert may transiently appear until `terminal.replaced` folds it — honest and accepted (the alternative was a forever-lying notice); (b) a missed `terminal.replaced` frame loses that crash's trace entry — accepted. ### D-10. Patience window honesty (znhn item 1b) @@ -75,12 +75,12 @@ One sentence appended to `docs/plans/2026-07-27-agent-crash-resilience.md` §D-5 | File | Change | Task | |---|---|---| | `crates/freshell-freshagent/src/spawn_gate.rs` | Modify: add `acquire_uncancellable` + 3 pin tests | 1 | -| `crates/freshell-freshagent/src/terminal_tabs.rs` | Modify: migrate REST acquire; `spawn_gate_error_response` signature + `pub(crate)` + Retry-After; D-C marker; burst test rewrite; 429 header test | 1, 2, 3 | +| `crates/freshell-freshagent/src/terminal_tabs.rs` | Modify: migrate REST acquire; `spawn_gate_error_response` signature (stays private) + Retry-After; D-C marker; burst test rewrite; 429 header test | 1, 2, 3 | | `crates/freshell-ws/src/terminal.rs` | Modify: migrate respawn acquire; `terminal.autoResumeCancel` match arm + handler | 1, 8 | | `crates/freshell-codex/src/launch_plan.rs` | Modify: D-C marker on flag doc comment | 2 | | `docs/plans/2026-07-27-rest-spawn-gate.md` | Modify: D-C tripwire note | 2 | -| `crates/freshell-freshagent/src/lib.rs` | Modify: gate opencode cold-start in `send_keys`; tests | 4 | -| `crates/freshell-freshagent/src/opencode_ws.rs` | Modify: deliberate-ungated comment | 4 | +| `crates/freshell-freshagent/src/lib.rs` | Modify: comment-only — deliberate do-not-gate record in the `send_keys` cold arm (D-7 reversed) | 4 | +| `crates/freshell-freshagent/src/opencode_ws.rs` | Modify: comment-only — deliberate-ungated record at the WS materialize door (D-7 reversed) | 4 | | `crates/freshell-protocol/src/server_messages.rs` | Modify: `RuntimeStatus::Exited`, `TerminalStatus.resume_cycles` | 5 | | `crates/freshell-protocol/src/client_messages.rs` | Modify: `TerminalAutoResumeCancel` variant + struct, `CLIENT_MESSAGE_TYPES` 29→30 | 8 | | `crates/freshell-protocol/tests/roundtrip.rs` | Modify: settle-frame + cancel roundtrip tests | 5, 8 | @@ -105,7 +105,7 @@ One sentence appended to `docs/plans/2026-07-27-agent-crash-resilience.md` §D-5 | `test/e2e-browser/fixtures/fake-crashing-claude-cli.mjs` | Modify: `FAKE_CRASH_LIVE_MS` flap mode | 12 | | `test/e2e-browser/specs/agent-crash-autoresume-rust.spec.ts` | Modify: 3 new tests | 12 | -Anything not in this table is out of scope. Scope check: the two katas share the spawn gate (znhn item 4 IS the enabler for bccd items 2–4) and the auto-resume protocol work is one coherent chain — one plan, strictly ordered tasks, each independently testable. +Anything not in this table is out of scope. Scope check: the two katas share the spawn gate (znhn item 4 IS the enabler for bccd items 2–3; bccd item 4 was decided at validation as do-not-gate, D-7) and the auto-resume protocol work is one coherent chain — one plan, strictly ordered tasks, each independently testable. --- @@ -158,7 +158,7 @@ Expected: all green. (Full-suite gates run in Task 13; `npm test` is coordinator **Interfaces:** - Consumes: `SpawnGate::acquire(&self, timeout: Duration, cancel: &mut watch::Receiver) -> Result` (existing). -- Produces: `pub async fn acquire_uncancellable(&self, timeout: Duration) -> Result` — used by Tasks 3 and 4. +- Produces: `pub async fn acquire_uncancellable(&self, timeout: Duration) -> Result` — consumed by this task's two door migrations (REST + auto-resume respawn, znhn#4/bccd#3) and by Task 3's test pre-hold. - [ ] **Step 1: Write the failing pin tests** — add to `mod tests` in `spawn_gate.rs` (the module already has `cancel_pair()` at `:212` and uses `Arc`, `Duration`, `tokio::time::sleep`): @@ -296,7 +296,7 @@ git commit -m "feat(spawn-gate): acquire_uncancellable owns the never-fired canc **Interfaces:** - Consumes: `crate::fail_json_code(StatusCode, &str, String) -> Response` (lib.rs:1336); `RestSpawnGate { gate, timeout }`. -- Produces: `pub(crate) fn spawn_gate_error_response(err: SpawnGateError, retry_after: std::time::Duration) -> Response` — reused by Task 4. 429 body gains `retryAfterMs: number`; 429 response gains `Retry-After: ` header. +- Produces: `fn spawn_gate_error_response(err: SpawnGateError, retry_after: std::time::Duration) -> Response` (stays private — D-5). 429 body gains `retryAfterMs: number`; 429 response gains `Retry-After: ` header. - [ ] **Step 1: Extend the failing test** — in `queue_cap_exceeded_rest_create_is_429_spawn_queue_full` (`terminal_tabs.rs:3913`), after the existing status/code assertions, add assertions on the header and body field. The test configures its gate timeout via `state.set_spawn_gate(gate, )`; assert against that same duration (if the test uses `Duration::from_secs(30)`, expect `"30"` and `30000`): @@ -326,7 +326,7 @@ Expected: FAIL — no `retry-after` header / no `retryAfterMs` field (or compile /// 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`). -pub(crate) fn spawn_gate_error_response( +fn spawn_gate_error_response( err: crate::spawn_gate::SpawnGateError, retry_after: std::time::Duration, ) -> Response { @@ -475,6 +475,8 @@ git commit -m "feat(rest): Retry-After on SPAWN_QUEUE_FULL 429 + D-C revisit tri } ``` +Payload note (validated A9): determinism of "zero completions while the permit is held" was verified for the EXISTING payload specifically — every pre-gate early-return passes for exactly the `shell_create_body()` request + auth the current burst tests use. The rewritten test must keep the request payload AND auth byte-identical to the existing burst tests' `shell_create_body()`; any payload change re-opens the pre-gate early-return question. + - [ ] **Step 2: Run to verify it fails/compiles honestly** Run: `cargo test -p freshell-freshagent fifteen_plus_rest_create_burst -- --nocapture` @@ -494,107 +496,57 @@ git commit -m "test(rest): burst test pins max-in-flight <= budget deterministic --- -### Task 4: Gate the opencode sidecar cold-start (bccd 4) +### Task 4: Opencode sidecar cold-start — deliberate DO-NOT-GATE comments (bccd 4, D-7 reversed) **Files:** -- Modify: `crates/freshell-freshagent/src/lib.rs` (`send_keys` cold-start arm at `:1583-1596`; inline tests near `:2694`) +- Modify: `crates/freshell-freshagent/src/lib.rs` (comment in the `send_keys` cold-start arm at `:1583-1596`) - Modify: `crates/freshell-freshagent/src/opencode_ws.rs` (comment near `:542`) **Interfaces:** -- Consumes: `FreshAgentState::spawn_gate() -> Option` (`lib.rs:310`), `acquire_uncancellable` (Task 1), `pub(crate) spawn_gate_error_response(err, retry_after)` (Task 2), test harness `FreshAgentState::set_manager_for_test` (`lib.rs:536`) + `NoopSpawner`/`FakeAllocator`/`NoopEventSource`, template test `rest_send_keys_materialization_records_binding` (`lib.rs:2694`). -- Produces: the cold-start fork consults the shared spawn gate; rejection returns the same 429/503 envelopes as the create door. +- Consumes: D-7 (DECISION REVERSED at validation — do NOT gate; the singleton bounds the fork, gating starves). +- Produces: two deliberate code comments recording the evaluate-and-decide outcome + arithmetic at both fork doors. No behavior change, no gate acquisition, no new tests. -- [ ] **Step 1: Write the failing tests** — inline in `lib.rs`'s test module, modeled byte-for-byte on the harness of `rest_send_keys_materialization_records_binding` (`:2694` — same state/pane/router setup, `timeout: 0`, no real `opencode` binary): +**TDD note:** this is a comments-only task — there is no behavior to pin, so no red test is required (documented decision, not a silent deferral; see D-7 and the Self-Review). -```rust - #[tokio::test] - async fn opencode_cold_start_send_keys_is_gated_and_queue_full_is_429() { - // Same setup as rest_send_keys_materialization_records_binding, but - // the gate has zero permits and zero queue slots: the cold-start - // (no durable_id) send-keys must be rejected BY THE GATE before any - // sidecar work happens. - // - state.set_spawn_gate( - Arc::new(crate::spawn_gate::SpawnGate::new(0, 0)), - std::time::Duration::from_millis(50), - ); - // - assert_eq!(status, 429); - assert_eq!(body["code"], "SPAWN_QUEUE_FULL"); - assert!(body["retryAfterMs"].is_u64()); - } - - #[tokio::test] - async fn opencode_cold_start_queues_behind_the_held_spawn_permit() { - // Gate (1, 64) with the permit pre-held by the test: the cold-start - // send-keys must QUEUE (queued_total == 1) and complete only after - // release — proof the fork door actually flows through the gate. - // - let gate = Arc::new(crate::spawn_gate::SpawnGate::new(1, 64)); - state.set_spawn_gate(Arc::clone(&gate), std::time::Duration::from_secs(5)); - let held = gate - .acquire_uncancellable(std::time::Duration::from_secs(1)) - .await - .unwrap(); - let task = tokio::spawn(/* the send-keys request future */); - for _ in 0..200 { - if gate.queued_total() == 1 { break; } - tokio::time::sleep(std::time::Duration::from_millis(5)).await; - } - assert_eq!(gate.queued_total(), 1, "cold start must wait on the gate"); - drop(held); - let response = task.await.unwrap(); - // - } -``` -Also assert in the second test (or a third, if cleaner) that a pane **with** `durable_id` (warm path) does NOT consult the gate: with the permit held, the warm send-keys response must not be 429/503-`SPAWN_TIMEOUT`. - -- [ ] **Step 2: Run to verify failure** - -Run: `cargo test -p freshell-freshagent opencode_cold_start` -Expected: FAIL — cold start currently never touches the gate (first test gets a non-429; second never queues). - -- [ ] **Step 3: Implement** — in `send_keys` (`lib.rs`), inside the cold-start `else` arm (`:1586`), immediately before `manager.create_session(...)`: +- [ ] **Step 1: Add the REST-door comment** — in `send_keys` (`lib.rs`), at the top of the cold-start `else` arm (`:1586`), immediately before `manager.create_session(...)`: ```rust - // bccd item 4 (council enn3 D-D follow-up): the sidecar - // cold-start is a REST-reachable process fork — it must consult - // the same server-wide spawn gate as every other fork door. - // Singleton double-mutex single-flight means at most one fork - // ever happens, so this is a single-permit acquire held only - // across the cold-start (spawn + bounded health wait). Warm - // sends take the durable_id branch and never reach this arm. - let _spawn_permit = match state.spawn_gate() { - Some(g) => match g.gate.acquire_uncancellable(g.timeout).await { - Ok(permit) => Some(permit), - Err(err) => { - return crate::terminal_tabs::spawn_gate_error_response(err, g.timeout) - } - }, - None => None, // unwired (unit-test states) = legacy ungated - }; -``` -(The permit drops at the end of the `else` block — after `create_session` returns.) Ensure `spawn_gate_error_response` is reachable: Task 2 made it `pub(crate)`; if `terminal_tabs` is not already a visible module path, adjust to the crate's actual module layout (compiler-guided). - -Add the deliberate-scope comment in `opencode_ws.rs` at the materialize cold-start (`:542` area): + // 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. +``` + +- [ ] **Step 2: Add the WS-door comment** — in `opencode_ws.rs` at the materialize cold-start (`:542` area): ```rust - // Deliberately ungated (bccd item 4 scope): council D-D ruled on - // REST-reachable forks; this WS materialize path shares the same - // single-flighted singleton manager, so at most one sidecar fork - // exists server-wide. Revisit if the WS door grows fork fan-out. + // 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. ``` -- [ ] **Step 4: Run to verify pass** +- [ ] **Step 3: Verify comments-only** — `git diff` shows only comment lines; then run: -Run: `cargo test -p freshell-freshagent && cargo fmt --check && cargo clippy -p freshell-freshagent --all-targets -- -D warnings` -Expected: PASS. +Run: `cargo test -p freshell-freshagent 2>&1 | tail -3 && cargo fmt --check && cargo clippy -p freshell-freshagent --all-targets -- -D warnings` +Expected: PASS (no behavior change). -- [ ] **Step 5: Commit** +- [ ] **Step 4: Commit** ```bash git add crates/freshell-freshagent/src/lib.rs crates/freshell-freshagent/src/opencode_ws.rs -git commit -m "feat(opencode): sidecar cold-start acquires a spawn-gate permit (bccd#4)" +git commit -m "docs(opencode): record the do-not-gate decision at both sidecar fork doors (bccd#4, D-7 reversed)" ``` --- @@ -789,7 +741,7 @@ Trait addition (`AutoResumeDriver`): } ``` -Hub body: at **every** `driver.log_settled(...)` site (`:240` decide-settle inside the `ev.mode != "shell"` guard, `:264` pre-respawn guard, `:268` session_lease_held, `:295` lease_completion_lost, `:301` respawn_failed), add `driver.emit_settled(&ev.terminal_id, , None);` immediately before the `log_settled` call. (Task 7 threads a real `resume_cycles` value into the breaker settle; Task 8 adds the one settle site that must NOT emit.) +Hub body: at **every** `driver.log_settled(...)` site (`:240` decide-settle inside the `ev.mode != "shell"` guard, `:264` pre-respawn guard, `:268` session_lease_held, `:295` lease_completion_lost, `:301` respawn_failed), add `driver.emit_settled(&ev.terminal_id, , None);` immediately before the `log_settled` call. (Task 7 threads a real `resume_cycles` value into the breaker settle; Task 8 adds the user-cancel settle site, which ALSO emits — reason `"auto-resume cancelled"`, idempotent with the handler's immediate frame.) - [ ] **Step 4: Run to verify pass** @@ -946,7 +898,7 @@ Hub changes: None => (0, 0), }; ``` -- Eviction branch (`:242-246`): **reset attempts only, keep cycles** — replace `attempts.remove(k)` with `if let Some(h) = attempts.get_mut(k) { h.attempts = 0; }` (the existing "never evicted on exhaustion" posture is preserved; cycles must survive healthy resets by design). +- Eviction branch (`:242-246`): **reset attempts only, keep cycles** — replace `attempts.remove(k)` with `if let Some(h) = attempts.get_mut(k) { h.attempts = 0; }` (the existing "never evicted on exhaustion" posture is preserved; cycles must survive healthy resets by design). **AND replace the const in the eviction-branch CONDITION at `:242`** — `ev.lifetime_ms >= AUTO_RESUME_HEALTHY_LIFETIME_MS` becomes `ev.lifetime_ms >= cfg.healthy_lifetime_ms` (the same config value threaded into `decide` — validated A8: with an env of 500ms, a const/cfg split leaves stale attempts after a breaker settle and drains the budget one retry early). The supervisor panic-health site (`auto_resume.rs:169`) deliberately STAYS on the compile-time const — add a code comment there documenting the split (panic-health is orthogonal to attempts/cycles and must not follow the e2e knob). - Attempt write (`:251`): `attempts.entry(key.clone()).or_default().attempts = attempt;` - Breaker settle: in the settle branch, thread the count — `driver.emit_settled(&ev.terminal_id, reason, if reason == "flap_circuit_breaker" { Some(recent_cycles) } else { None });` - On successful respawn (the `emit_replaced` arm): `attempts.entry(key.clone()).or_default().cycles.push(crate::terminal::now_ms());` (re-fetch the entry — the earlier borrow ended before the awaits). @@ -1001,6 +953,17 @@ Hub changes: // reset) → verify the successful resumes still accumulated cycles by // tripping the breaker at the configured max. } + + #[tokio::test] + 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 condition (:242) must treat this as healthy: after + // the crash, the entry's attempts are 0 (evicted/reset) and the next + // resume starts at attempt 1. The planned 60_000 lifetimes in the + // tests above CANNOT detect a const/cfg split — this one can. + } ``` (Write the bodies against the existing `FakeDriver` helpers — `drain()`, the crash-event constructor, and frame accessors are at `:893-1076`; follow `healthy_generation_resets_attempts` (`:1145`) for the event/lifetime idiom. FakeDriver gains a `respawn_count()` helper if one doesn't already exist.) @@ -1034,7 +997,7 @@ git commit -m "feat(auto-resume): flap-loop circuit breaker — bounded and loud **Interfaces:** - Consumes: `emit_settled`/settle frame (Tasks 5–6); dispatch pattern of `handle_client_text` (`terminal.rs:483-520`, exhaustive match, arms return `bool`); `handle_kill(kill: TerminalKill, ws_tx: &mut WsSink, state: &WsState) -> bool` (`:3847`) as the shape template. -- Produces (used by Tasks 9, 12): client→server message `{ type: 'terminal.autoResumeCancel', terminalId: string }`; on receipt the server broadcasts the settle frame immediately (reason `"auto-resume cancelled"`) and the hub's pending resume aborts silently; `WsState.auto_resume_cancels: Arc>>`; driver method `fn take_cancel(&self, terminal_id: &str) -> bool`. +- Produces (used by Tasks 9, 12): client→server message `{ type: 'terminal.autoResumeCancel', terminalId: string }`; on receipt the server VALIDATES the id against the registry (unknown → log + ignore: no insert, no broadcast — D-4, validated A5), then broadcasts the settle frame immediately (reason `"auto-resume cancelled"`); the hub's `take_cancel` arm aborts the pending resume and ALSO emits the settle frame (same reason — idempotent client-side), so a late-consumed or pre-seeded cancel can never strand a recovering notice; `WsState.auto_resume_cancels: Arc>>`; driver method `fn take_cancel(&self, terminal_id: &str) -> bool`. - [ ] **Step 1: Write the failing protocol tests** — @@ -1081,9 +1044,11 @@ Enum variant (in the `#[serde(tag = "type")]` enum, matching neighbors' style): ```rust /// Pending user cancels for planned auto-resumes, keyed by the OLD - /// (crashed) terminal id (znhn item 2). Inserted by the WS handler, - /// consumed by the hub's post-sleep guard. Bounded: one entry per - /// cancel click, removed on consumption. + /// (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>>, ``` Fix every `WsState { ... }` literal the compiler reports (production wiring in `freshell-server/src/main.rs`, harness literals in `crates/freshell-ws/tests/common/mod.rs:157,233,313,390,475,556,636,718`, and any unit-test literals) with `auto_resume_cancels: Default::default(),`. @@ -1101,9 +1066,17 @@ Handler (near `handle_kill`): ```rust /// 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 hub consumes the flag and settles -/// silently (this frame is the loud half). +/// 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() @@ -1138,8 +1111,13 @@ Hub: FIRST post-sleep guard (before `pre_respawn_guard`): ```rust if driver.take_cancel(&ev.terminal_id) { - // The cancel handler already broadcast the settle - // frame — log only, no second frame. + // 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; } @@ -1149,18 +1127,34 @@ Hub: FIRST post-sleep guard (before `pre_respawn_guard`): ```rust #[tokio::test] - async fn user_cancel_during_backoff_aborts_the_respawn_silently() { + 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 NO - // settle frame of its own (the WS handler's immediate frame is the - // loud half). + // 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. // ; driver.set_cancelled("t1") BEFORE sending // the crash event (the flag is checked post-sleep). - // after drain(): respawn_count == 0; settled_frames() is EMPTY; + // after drain(): respawn_count == 0; settled_frames() contains + // ("t1", "auto-resume cancelled", None); // log records contain ("t1", "user_cancelled"). } ``` -Run: `cargo test -p freshell-ws auto_resume && cargo test -p freshell-protocol` +And the handler-side validation test (in `terminal.rs`'s tests or the WS integration harness, following the kill-handler unknown-id precedent): + +```rust + #[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. + } +``` +The immediate handler-side broadcast for a KNOWN id stays exactly as specified in Step 3 — the click-latency story is unchanged. + +Run: `cargo test -p freshell-ws auto_resume && cargo test -p freshell-ws terminal && cargo test -p freshell-protocol` Expected: PASS. - [ ] **Step 5: TS + Node side + contract regen** — @@ -1215,14 +1209,17 @@ git commit -m "feat(auto-resume): terminal.autoResumeCancel — user opts out of **Interfaces:** - Consumes: settle frame `terminal.status { status: 'exited', terminalId, resumeCycles? }` (Tasks 5–8); `terminal.autoResumeCancel` send (Task 8); `WsClient.send(msg: unknown)` (`src/lib/ws-client.ts:681`; TerminalView already holds `const ws = useMemo(() => getWsClient(), [])` at `:622` and calls e.g. `ws.send({ type: 'terminal.detach', terminalId: tid })` at `:2889`; tests mock it via the hoisted `wsMocks.send`). -- Produces (used by Tasks 10–12): slice action `recordAutoResumeSettled({ paneId, resumeCycles?, at })`; `PaneLifecycleEntry.settle?: { resumeCycles?: number; at: number }`; selector `selectResumeCycles(root, paneId)`; `TerminalExitBannerProps.onCancelAutoResume`; NO TTL anywhere (constant, selector filter, and re-render timer all deleted). +- Produces (used by Tasks 10–12): slice action `recordAutoResumeSettled({ paneId, resumeCycles?, at })`; `PaneLifecycleEntry.settle?: { resumeCycles?: number; at: number }`; selector `selectResumeCycles(root, paneId)`; slice action `clearRecoveringNotices()` (the D-3 reconnect backstop — clears every stale `kind: 'recovering'` notice); `TerminalExitBannerProps.onCancelAutoResume`; NO TTL anywhere (constant, selector filter, and re-render timer all deleted). **Hard rule (D-1, validated):** the new settle-frame handling dispatches ONLY into `terminalLifecycleSlice` — it must NEVER write pane content or pane status (`updateContent`/`updateTab`); the `terminal.status` content-write allowlist (`running|recovering` only, `TerminalView.tsx:4378-4381`) is the load-bearing barrier against out-of-order settle frames and stays untouched. - [ ] **Step 1: Write the failing tests** — Replace the TTL-degradation test (`TerminalView.exitBanner.test.tsx:363-391`) with the frame-driven pair (this is ALSO the znhn item 6 pin — the alert can never appear while a resume is pending, at any delay value): ```tsx - it('keeps the recovering notice up indefinitely without a settle frame (no timer degradation — znhn#6 pin)', 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() // seed: lifecycle { lastTerminalId: 'term-crashed', exit: { exitCode: 1, at: Date.now() }, // notice: { kind: 'recovering', attempt: 1, maxAttempts: 2, exitCode: 1, at: Date.now() } }, status 'exited' @@ -1234,6 +1231,30 @@ Replace the TTL-degradation test (`TerminalView.exitBanner.test.tsx:363-391`) wi vi.useRealTimers() }) + it('an out-of-order exited settle frame never touches pane content or status (D-1 allowlist pin)', async () => { + // seed: a LIVE pane — content.terminalId === 'term-live', status 'running', + // lifecycle.lastTerminalId 'term-live' (no terminal.exit received yet). + await act(async () => { + messageHandler!({ type: 'terminal.status', terminalId: 'term-live', status: 'exited', reason: 'retries_exhausted' }) + }) + // 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. + // walk the store: pane content.status still 'running', terminalId still 'term-live' + expect(screen.queryByRole('alert')).toBeNull() + }) + + it('a recovering notice does not survive a reconnect (D-3 backstop pin)', async () => { + // seed the recovering notice as above; then fire the ws reconnect + // callback the harness captured (the same hook TerminalView registers + // via ws.onReconnect). + await act(async () => { + reconnectHandler!() + }) + expect(screen.queryByText(/auto-resuming/)).toBeNull() + }) + it('clears the recovering notice the moment the settle frame arrives', async () => { // same seed; then: await act(async () => { @@ -1256,6 +1277,8 @@ Slice tests (`terminalLifecycleSlice.test.ts`): replace the TTL-expiry case with ```ts it('selectActiveNoticeFrom returns the notice with no TTL — settles are frame-driven', () => { /* seed notice with at: 0; expect returned regardless of now */ }) it('recordAutoResumeSettled clears the notice and records resumeCycles', () => { /* dispatch; expect entry.notice undefined, entry.settle = { resumeCycles: 3, at } */ }) + it('clearRecoveringNotices clears every recovering notice (D-3 reconnect backstop)', () => { /* seed two panes with recovering notices + one settle; dispatch; expect both notices undefined, settle/exit untouched */ }) + it('recordTerminalExit clears prior settle state (stale resumeCycles cannot leak into a later crash banner)', () => { /* seed entry.settle = { resumeCycles: 5, at: 1 }; dispatch recordTerminalExit; expect entry.settle undefined */ }) ``` Banner test (`TerminalExitBanner.test.tsx`): the recovering-notice case now also asserts the cancel button (`getByRole('button', { name: 'Cancel auto-resume for claude' })`). @@ -1297,7 +1320,22 @@ export const selectActiveNotice = (root: { terminalLifecycle?: TerminalLifecycle export const selectResumeCycles = (root: { terminalLifecycle?: TerminalLifecycleState }, paneId: string) => root.terminalLifecycle?.byPaneId[paneId]?.settle?.resumeCycles ``` -- Export `recordAutoResumeSettled` with the other actions. +- Second new reducer — the D-3 reconnect backstop: + +```ts + // 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 entry of Object.values(state.byPaneId)) { + if (entry?.notice?.kind === 'recovering') delete entry.notice + } + }, +``` +- `recordTerminalExit` additionally deletes any prior `entry.settle` (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. +- Export `recordAutoResumeSettled` and `clearRecoveringNotices` with the other actions. - [ ] **Step 4: Implement TerminalView + banner** — - Delete the TTL re-render block (`TerminalView.tsx:603-619`: the `setNoticeExpiryTick` state + effect) and the `AUTO_RESUME_NOTICE_TTL_MS` import; change `selectActiveNotice(s, paneId, Date.now())` → `selectActiveNotice(s, paneId)`. @@ -1314,7 +1352,15 @@ export const selectResumeCycles = (root: { terminalLifecycle?: TerminalLifecycle ) } ``` -(Guard the existing `updateContent({ status })`/`updateTab({ status })` branch so a settle frame for a dead old terminal doesn't touch live content — it already only fires when `msg.terminalId === tid`, which is cleared on exit; verify and leave as-is.) +**Hard rule (D-1, validated A1/A6):** this settle handling dispatches ONLY into `terminalLifecycleSlice` — never `updateContent`/`updateTab`. Do NOT rely on `content.terminalId` having been cleared by `terminal.exit` first: cross-channel ordering is unguaranteed (unbiased `select!`, `terminal.rs:325-334`). The existing content-write allowlist (`msg.status === 'running' || msg.status === 'recovering'`, `:4378-4381`) is the load-bearing barrier that keeps an out-of-order `'exited'` frame away from pane content/status — leave it exactly as-is; the new out-of-order pin test from Step 1 locks it. +- Reconnect backstop (D-3): in the established reconnect handling (`ws.onReconnect` handler area, `TerminalView.tsx:4934-4998` — wire it where reconnect is already handled), dispatch `clearRecoveringNotices()`: + +```tsx + // 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()) +``` - Banner wiring: pass `onCancelAutoResume` at the mount (`:5333-5356`): ```tsx @@ -1424,6 +1470,13 @@ git commit -m "feat(client): frame-driven auto-resume notices — delete the 30s // 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** @@ -1473,7 +1526,7 @@ Field on `TerminalPaneContent` (after `reconcileEpoch`): ``` (Match the helper's real signature — if `clearPaneReconcileNotice` calls it differently, copy that call shape.) 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). Narrow `AutoResumeNotice.kind` to `'recovering'` and update any test-harness types that referenced `'resumed'`. +`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(...)`: @@ -1780,6 +1833,8 @@ if (liveMs > 0) { }) ``` +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** @@ -1842,7 +1897,7 @@ Expected: all PASS; the restore-contract-wall must stay green with ZERO `test.fa ```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 now acquires a single spawn-gate permit in the REST send-keys cold path (rejection reuses the 429/503 envelopes); the WS materialize door stays deliberately ungated (D-D ruled on REST-reachable forks; single-flighted singleton bounds it — comment in opencode_ws.rs). (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." +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** @@ -1862,29 +1917,33 @@ Expected: branch pushed. Do NOT open a PR — landing happens outside this workf | 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 | -| 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 + frame-driven clear; e2e cancel test exercises the settle frame end-to-end | +| 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; kata comment records the supersession | -| bccd 4 sidecar cold-start gating (evaluate-and-decide) | 4 | DECIDED: gate it (D-7); tests pin 429-when-full and queue-behind-held-permit; WS door deliberately out of scope with comment | +| 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 (znhn 6, bccd 4) are decided and implemented/pinned in-plan. **No unresolved coverage gaps.** +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; NoopSpawner for the sidecar gate rejection — the gate rejection fires before any spawn, and the queue-behind-permit test proves the door is on the gate). The WS-door opencode cold-start is a deliberate, documented scope decision (council D-D ruled on REST), not a silent deferral — recorded in code comment + kata comment. +**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 Tasks 3 and 4 are deliberate **splice anchors into existing test bodies quoted by line number** — the implementer copies working in-repo code rather than this plan duplicating (and drifting from) it; every NEW behavior has complete code. Test skeletons in Tasks 6–7 name their exact harness templates by line. No "TBD"/"add error handling"/"similar to Task N" anywhere. +**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 Tasks 3, 4 with matching signatures. ✓ -- `spawn_gate_error_response(err, retry_after: Duration)` `pub(crate)` — Task 2 defines, Task 4 consumes with `(err, g.timeout)`. ✓ +- `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`. ✓ From 7de061891c8a1032f212b218733dbb93efb9dfec Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Wed, 29 Jul 2026 15:36:20 -0700 Subject: [PATCH 03/17] docs(plan): fix two blocking fresheyes findings (znhn-bccd-followups) Fresh-eyes review iteration 1 flagged two executable plan defects: - Task 10: setPaneCrashTrace/clearPaneCrashTrace were specified against a paneId-only helper call, but the panes-tree traversal helpers take positional (state, tabId, paneId). Payloads now carry { tabId, paneId }, the reducer bodies call findReconcileTerminalContent(state, action.payload.tabId, action.payload.paneId), and both TerminalView dispatch sites (terminal.replaced handler, banner onDismissCrashTrace) pass tabId. Also corrected the 'same traversal helper' claim: the reconcile-notice reducers use findReconcilePaneContent (:631); crashTrace uses its TerminalPaneContent-narrowed sibling findReconcileTerminalContent (:618). - Task 9: the D-3 reconnect-backstop test referenced reconnectHandler!(), which the existing harness never captures (wsMocks.onReconnect is a bare mockReturnValue). Added an explicit harness-extension step: declare reconnectHandler next to messageHandler and capture it in beforeEach via wsMocks.onReconnect.mockImplementation, mirroring the messageHandler capture. --- docs/plans/2026-07-29-znhn-bccd-followups.md | 43 +++++++++++++++----- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/docs/plans/2026-07-29-znhn-bccd-followups.md b/docs/plans/2026-07-29-znhn-bccd-followups.md index 066137baa..4087ab3cd 100644 --- a/docs/plans/2026-07-29-znhn-bccd-followups.md +++ b/docs/plans/2026-07-29-znhn-bccd-followups.md @@ -1247,8 +1247,8 @@ Replace the TTL-degradation test (`TerminalView.exitBanner.test.tsx:363-391`) wi it('a recovering notice does not survive a reconnect (D-3 backstop pin)', async () => { // seed the recovering notice as above; then fire the ws reconnect - // callback the harness captured (the same hook TerminalView registers - // via ws.onReconnect). + // callback captured via the harness extension described below (the same + // hook TerminalView registers via ws.onReconnect). await act(async () => { reconnectHandler!() }) @@ -1272,6 +1272,25 @@ Replace the TTL-degradation test (`TerminalView.exitBanner.test.tsx:363-391`) wi ``` (Use the file's existing `makeStore`/`renderPane` harness at `:27-185`; explicit `cleanup()` stays in `afterEach`.) +**Harness extension (required for the D-3 backstop test):** the harness does not yet capture the reconnect callback — `wsMocks.onReconnect` is only `vi.fn().mockReturnValue(() => {})` (`:31`), and only `messageHandler` gets a capturing `mockImplementation` in `beforeEach` (`:198-201`). Mirror that capture: next to the `messageHandler` declaration (`:97`) add + +```ts +let reconnectHandler: (() => void) | null = null +``` + +and in `beforeEach`, alongside the existing `wsMocks.onMessage.mockImplementation(...)` capture, add + +```ts + wsMocks.onReconnect.mockImplementation((callback: () => void) => { + reconnectHandler = callback + return () => { + reconnectHandler = null + } + }) +``` + +(TerminalView registers a zero-arg callback and keeps the returned unsubscribe: `unsubReconnect = ws.onReconnect(() => { ... })` at `TerminalView.tsx:4934`; `type ReconnectHandler = () => void`, `ws-client.ts:17`.) + Slice tests (`terminalLifecycleSlice.test.ts`): replace the TTL-expiry case with: ```ts @@ -1424,8 +1443,8 @@ git commit -m "feat(client): frame-driven auto-resume notices — delete the 30s - 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, paneId)` traversal helper (`panesSlice.ts:618`). -- Produces (used by Tasks 11–12): `export type CrashTrace = { exitCode: number; resumedAtMs: number }` in `paneTypes.ts`; `TerminalPaneContent.crashTrace?: CrashTrace`; actions `setPaneCrashTrace({ paneId, crashTrace })` and `clearPaneCrashTrace({ paneId })`; 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` ``. +- 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** — @@ -1507,24 +1526,24 @@ Field on `TerminalPaneContent` (after `reconcileEpoch`): crashTrace?: CrashTrace ``` -`panesSlice.ts` (mirror `setPaneReconcileNotice`/`clearPaneReconcileNotice` at `:2080-2096` exactly, same traversal helper): +`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<{ paneId: string; crashTrace: CrashTrace }> + action: PayloadAction<{ tabId: string; paneId: string; crashTrace: CrashTrace }> ) { - const content = findReconcileTerminalContent(state, action.payload.paneId) + const content = findReconcileTerminalContent(state, action.payload.tabId, action.payload.paneId) if (content) content.crashTrace = action.payload.crashTrace }, - clearPaneCrashTrace(state, action: PayloadAction<{ paneId: string }>) { - const content = findReconcileTerminalContent(state, action.payload.paneId) + 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 }, ``` -(Match the helper's real signature — if `clearPaneReconcileNotice` calls it differently, copy that call shape.) Export both actions at `:2236-2237` alongside the others; import `CrashTrace` from `./paneTypes`. +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'`. @@ -1533,11 +1552,13 @@ Field on `TerminalPaneContent` (after `reconcileEpoch`): ```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 @@ -1558,7 +1579,7 @@ Banner mount gains: (terminalContent.status === 'exited' && (exitRecord ? exitRecord.exitCode !== 0 : true)) || (terminalContent.status === 'error' && Boolean(exitRecord && exitRecord.exitCode !== 0)) } - onDismissCrashTrace={() => dispatch(clearPaneCrashTrace({ paneId }))} + onDismissCrashTrace={() => dispatch(clearPaneCrashTrace({ tabId, paneId }))} ``` (Hoist the two settled-dead sub-expressions into a `const settledDead = ...` used by both `showExitBanner` and the prop — DRY.) From b45981faba14fdc15e70bcefa543733eb5bd511f Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Wed, 29 Jul 2026 16:45:57 -0700 Subject: [PATCH 04/17] feat(spawn-gate): acquire_uncancellable owns the never-fired cancel wart (znhn#4, bccd#3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- crates/freshell-freshagent/src/spawn_gate.rs | 71 +++++++++++++++++++ .../freshell-freshagent/src/terminal_tabs.rs | 15 ++-- crates/freshell-ws/src/terminal.rs | 16 ++--- 3 files changed, 83 insertions(+), 19 deletions(-) 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..2f1379380 100644 --- a/crates/freshell-freshagent/src/terminal_tabs.rs +++ b/crates/freshell-freshagent/src/terminal_tabs.rs @@ -1049,17 +1049,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), diff --git a/crates/freshell-ws/src/terminal.rs b/crates/freshell-ws/src/terminal.rs index d41e24ed0..e9c1fad4f 100644 --- a/crates/freshell-ws/src/terminal.rs +++ b/crates/freshell-ws/src/terminal.rs @@ -2906,18 +2906,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, From e2e9cbc3094a1fa773fc7ea02c7245cd5fd9fe88 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Wed, 29 Jul 2026 16:51:48 -0700 Subject: [PATCH 05/17] feat(rest): Retry-After on SPAWN_QUEUE_FULL 429 + D-C revisit tripwire (bccd#1, bccd#5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- crates/freshell-codex/src/launch_plan.rs | 4 + .../freshell-freshagent/src/terminal_tabs.rs | 81 ++++++++++++++----- docs/plans/2026-07-27-rest-spawn-gate.md | 2 + 3 files changed, 68 insertions(+), 19 deletions(-) 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/terminal_tabs.rs b/crates/freshell-freshagent/src/terminal_tabs.rs index 2f1379380..545e701bd 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", @@ -1071,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)); } } } @@ -1265,6 +1280,14 @@ async fn settle_gated_create(inputs: GatedSettleInputs) -> Result 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 From 3c9fdce78795cc72828b3f4e860b3ca89f88abae Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Wed, 29 Jul 2026 16:53:32 -0700 Subject: [PATCH 06/17] test(rest): burst test pins max-in-flight <= budget deterministically (bccd#2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- .../freshell-freshagent/src/terminal_tabs.rs | 46 +++++++++++++------ 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/crates/freshell-freshagent/src/terminal_tabs.rs b/crates/freshell-freshagent/src/terminal_tabs.rs index 545e701bd..750f58f1f 100644 --- a/crates/freshell-freshagent/src/terminal_tabs.rs +++ b/crates/freshell-freshagent/src/terminal_tabs.rs @@ -4012,17 +4012,25 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn fifteen_plus_rest_create_burst_is_bounded_and_all_complete() { - // Concurrency-1 gate: at most ONE request may hold the spawn permit - // at a time, so a 16-burst must serialize through the gate — the - // queued_total counter proves the burst actually queued (bounded - // in-flight) instead of spawning in parallel, and every request - // still completes (FIFO drain, nothing dropped). + // Deterministic pin (kata bccd item 2, council enn3): pre-holding the + // single permit forces EVERY burst request through the queue — + // queued_total() reaches exactly 16 (the fast path cannot fire while + // the budget is held), and ZERO requests may complete while the + // budget is exhausted. That pins max-in-flight <= budget without the + // probabilistic `queued_total >= 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(); @@ -4030,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"); From e44696c3d140a050d9cfdb832ac103e725743e4e Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Wed, 29 Jul 2026 16:56:16 -0700 Subject: [PATCH 07/17] docs(opencode): record the do-not-gate decision at both sidecar fork doors (bccd#4, D-7 reversed) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- crates/freshell-freshagent/src/lib.rs | 14 ++++++++++++++ crates/freshell-freshagent/src/opencode_ws.rs | 6 ++++++ 2 files changed, 20 insertions(+) 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) => { From cdb6e914e8e54c1e57923eb0e9f5e7fc38c8bb66 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Wed, 29 Jul 2026 17:13:34 -0700 Subject: [PATCH 08/17] feat(protocol): RuntimeStatus::Exited settle frame + resumeCycles field (znhn#3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- .../freshell-protocol/src/server_messages.rs | 12 +++++++++ crates/freshell-protocol/tests/roundtrip.rs | 26 +++++++++++++++++++ crates/freshell-ws/src/auto_resume.rs | 1 + port/contract/ws-server-messages.schema.json | 6 ++++- shared/ws-protocol.ts | 6 ++++- 5 files changed, 49 insertions(+), 2 deletions(-) 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/roundtrip.rs b/crates/freshell-protocol/tests/roundtrip.rs index d25e4f5fb..0ca9d9e50 100644 --- a/crates/freshell-protocol/tests/roundtrip.rs +++ b/crates/freshell-protocol/tests/roundtrip.rs @@ -484,3 +484,29 @@ 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); +} diff --git a/crates/freshell-ws/src/auto_resume.rs b/crates/freshell-ws/src/auto_resume.rs index cbd4e9eea..c1d379b22 100644 --- a/crates/freshell-ws/src/auto_resume.rs +++ b/crates/freshell-ws/src/auto_resume.rs @@ -617,6 +617,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) => { 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/shared/ws-protocol.ts b/shared/ws-protocol.ts index 5af6f7ff1..d73dfa3e4 100644 --- a/shared/ws-protocol.ts +++ b/shared/ws-protocol.ts @@ -772,7 +772,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 +781,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. From 0690dc0ea4504497192fafa4e1e17b2a916f6962 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Wed, 29 Jul 2026 17:25:53 -0700 Subject: [PATCH 09/17] feat(auto-resume): settle frames on every silent settle path (znhn#3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- crates/freshell-ws/src/auto_resume.rs | 130 ++++++++++++++++++++++++++ 1 file changed, 130 insertions(+) diff --git a/crates/freshell-ws/src/auto_resume.rs b/crates/freshell-ws/src/auto_resume.rs index c1d379b22..28f56b984 100644 --- a/crates/freshell-ws/src/auto_resume.rs +++ b/crates/freshell-ws/src/auto_resume.rs @@ -237,6 +237,7 @@ async fn run_hub_body( match decide(&ctx, delays) { AutoResumeDecision::SettleExited { reason } => { if ev.mode != "shell" { + driver.emit_settled(&ev.terminal_id, reason, None); driver.log_settled(&ev.terminal_id, reason); } if reason == "clean_exit" || ev.lifetime_ms >= AUTO_RESUME_HEALTHY_LIFETIME_MS { @@ -261,10 +262,12 @@ async fn run_hub_body( 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); 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"); continue; } @@ -292,12 +295,14 @@ async fn run_hub_body( // 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"); } } @@ -373,6 +378,11 @@ 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); fn log_settled(&self, terminal_id: &str, reason: &str); } @@ -649,6 +659,27 @@ 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 log_settled(&self, terminal_id: &str, reason: &str) { tracing::info!(terminal_id, reason, "terminal.auto_resume.settled"); } @@ -880,6 +911,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 @@ -908,6 +942,7 @@ mod tests { completes: Vec::new(), fails: Vec::new(), settled: Vec::new(), + settled_frames: Vec::new(), })), } } @@ -961,6 +996,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 { @@ -1060,6 +1099,13 @@ 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 log_settled(&self, terminal_id: &str, reason: &str) { self.lock() .settled @@ -1199,6 +1245,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)] @@ -1219,6 +1270,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)] @@ -1240,6 +1295,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)] @@ -1260,6 +1319,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)] @@ -1288,6 +1351,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)] @@ -1319,6 +1386,69 @@ 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 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, 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, 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 From 544901fa57b9a73263f3cb89537bd1c0b9df76fd Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Wed, 29 Jul 2026 17:39:31 -0700 Subject: [PATCH 10/17] =?UTF-8?q?feat(auto-resume):=20flap-loop=20circuit?= =?UTF-8?q?=20breaker=20=E2=80=94=20bounded=20and=20loud=20(znhn#2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- crates/freshell-server/src/main.rs | 11 + crates/freshell-ws/src/auto_resume.rs | 452 +++++++++++++++++++++++--- 2 files changed, 415 insertions(+), 48 deletions(-) diff --git a/crates/freshell-server/src/main.rs b/crates/freshell-server/src/main.rs index d5ff3048d..e1a8633e0 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). diff --git a/crates/freshell-ws/src/auto_resume.rs b/crates/freshell-ws/src/auto_resume.rs index 28f56b984..1ddcf873a 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,39 +304,63 @@ 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, None); + 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; + } } } } 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, @@ -291,6 +402,14 @@ 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 @@ -692,7 +811,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 @@ -704,7 +823,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)] @@ -720,14 +846,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 @@ -738,7 +875,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 @@ -753,7 +890,7 @@ mod tests { ..ctx() }; assert_eq!( - decide(&c, &DELAYS), + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), AutoResumeDecision::SettleExited { reason: "clean_exit" } @@ -767,7 +904,7 @@ mod tests { ..ctx() }; assert_eq!( - decide(&c, &DELAYS), + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), AutoResumeDecision::SettleExited { reason: "not_agent_mode" } @@ -778,7 +915,7 @@ mod tests { ..ctx() }; assert_eq!( - decide(&c, &DELAYS), + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), AutoResumeDecision::SettleExited { reason: "not_agent_mode" } @@ -790,7 +927,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}" ); } @@ -803,7 +943,7 @@ mod tests { ..ctx() }; assert_eq!( - decide(&c, &DELAYS), + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), AutoResumeDecision::SettleExited { reason: "no_resumable_identity" } @@ -813,7 +953,7 @@ mod tests { ..ctx() }; assert_eq!( - decide(&c, &DELAYS), + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), AutoResumeDecision::SettleExited { reason: "no_create_request_id" } @@ -827,7 +967,7 @@ mod tests { ..ctx() }; assert_eq!( - decide(&c, &DELAYS), + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), AutoResumeDecision::SettleExited { reason: "respawn_cap_exhausted" } @@ -841,7 +981,7 @@ mod tests { ..ctx() }; assert_eq!( - decide(&c, &DELAYS), + decide(&c, &DELAYS, AUTO_RESUME_HEALTHY_LIFETIME_MS), AutoResumeDecision::SettleExited { reason: "retries_exhausted" } @@ -858,7 +998,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 @@ -1125,7 +1298,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; @@ -1146,7 +1319,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(); @@ -1194,7 +1367,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(); @@ -1232,7 +1405,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(); @@ -1258,7 +1431,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(); @@ -1282,7 +1455,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(); @@ -1308,7 +1481,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(); @@ -1333,7 +1506,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(); @@ -1363,7 +1536,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)) @@ -1398,13 +1571,196 @@ mod tests { ); } + #[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 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, 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(); @@ -1425,7 +1781,7 @@ mod tests { // 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, 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(); @@ -1464,7 +1820,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)) From 2edbbc9d755562e9c07c413bf556c391ac8bb8d1 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:02:09 -0700 Subject: [PATCH 11/17] =?UTF-8?q?feat(auto-resume):=20terminal.autoResumeC?= =?UTF-8?q?ancel=20=E2=80=94=20user=20opts=20out=20of=20an=20in-flight=20r?= =?UTF-8?q?esume=20(znhn#2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- .../freshell-protocol/src/client_messages.rs | 14 +++- crates/freshell-protocol/tests/inventory.rs | 12 ++-- crates/freshell-protocol/tests/roundtrip.rs | 12 ++++ crates/freshell-server/src/main.rs | 1 + crates/freshell-ws/src/auto_resume.rs | 59 +++++++++++++++ crates/freshell-ws/src/codex_association.rs | 1 + crates/freshell-ws/src/lib.rs | 8 +++ .../freshell-ws/src/opencode_association.rs | 1 + crates/freshell-ws/src/terminal.rs | 72 ++++++++++++++++++- .../tests/claude_session_rebind.rs | 1 + .../tests/codex_managed_launch_e2e.rs | 1 + .../tests/codex_session_ref_resume.rs | 1 + crates/freshell-ws/tests/common/mod.rs | 8 +++ .../freshell-ws/tests/cross_kind_liveness.rs | 1 + .../tests/diag01_lifecycle_events.rs | 1 + .../tests/freshagent_claude_attach.rs | 1 + .../tests/freshagent_claude_kill_interrupt.rs | 1 + .../tests/freshagent_session_lease.rs | 1 + crates/freshell-ws/tests/hello_timeout.rs | 1 + crates/freshell-ws/tests/keepalive.rs | 1 + crates/freshell-ws/tests/max_payload.rs | 1 + .../tests/opencode_switch_rebind.rs | 1 + crates/freshell-ws/tests/origin_policy.rs | 1 + crates/freshell-ws/tests/pane_reconcile.rs | 1 + .../tests/pane_reconcile_freshagent.rs | 1 + .../freshell-ws/tests/rest_ws_shared_gate.rs | 1 + .../freshell-ws/tests/restore_spawn_gate.rs | 1 + .../tests/safe08_restore_diagnostics.rs | 1 + .../freshell-ws/tests/term09_output_queue.rs | 1 + port/contract/ws-message-inventory.json | 3 +- port/contract/ws-protocol.schema.json | 39 +++++++++- server/ws-handler.ts | 6 ++ shared/ws-protocol.ts | 8 +++ 33 files changed, 251 insertions(+), 12 deletions(-) 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/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 0ca9d9e50..004b34d9c 100644 --- a/crates/freshell-protocol/tests/roundtrip.rs +++ b/crates/freshell-protocol/tests/roundtrip.rs @@ -510,3 +510,15 @@ fn terminal_status_exited_settle_frame_roundtrips() { 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 e1a8633e0..bfee56cad 100644 --- a/crates/freshell-server/src/main.rs +++ b/crates/freshell-server/src/main.rs @@ -558,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 1ddcf873a..95102b564 100644 --- a/crates/freshell-ws/src/auto_resume.rs +++ b/crates/freshell-ws/src/auto_resume.rs @@ -370,6 +370,17 @@ 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) { @@ -502,6 +513,8 @@ pub(crate) trait AutoResumeDriver: Send + 'static { /// 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); } @@ -799,6 +812,14 @@ impl AutoResumeDriver for WsAutoResumeDriver { } } + 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"); } @@ -1073,6 +1094,8 @@ 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, claim_ok: bool, complete_ok: bool, panic_next_recovering: bool, @@ -1104,6 +1127,7 @@ mod tests { cap_exhausted: false, session: Some(("claude".into(), "sess-1".into(), None)), guard: None, + cancels: std::collections::HashSet::new(), claim_ok: true, complete_ok: true, panic_next_recovering: false, @@ -1145,6 +1169,9 @@ 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()); + } /// (old_terminal_id, attempt, max_attempts) fn recovering_calls(&self) -> Vec<(String, u32, u32)> { @@ -1279,6 +1306,9 @@ mod tests { 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 @@ -1754,6 +1784,35 @@ mod tests { ); } + #[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 guard_abort_emits_a_settle_frame() { // pane_closed guard-abort must broadcast the settle frame so the 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 e9c1fad4f..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 @@ -3840,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 @@ -4773,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), @@ -4865,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` @@ -4980,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/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/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 d73dfa3e4..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, From ef406b7454475819af00d3b18403d22f86f06639 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:11:45 -0700 Subject: [PATCH 12/17] =?UTF-8?q?feat(client):=20frame-driven=20auto-resum?= =?UTF-8?q?e=20notices=20=E2=80=94=20delete=20the=2030s=20TTL,=20add=20can?= =?UTF-8?q?cel=20(znhn#2,#3,#6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- src/components/TerminalExitBanner.tsx | 15 ++- src/components/TerminalView.tsx | 51 ++++++--- src/store/terminalLifecycleSlice.ts | 61 +++++++--- .../components/TerminalExitBanner.test.tsx | 17 ++- .../TerminalView.exitBanner.test.tsx | 107 ++++++++++++++++-- .../store/terminalLifecycleSlice.test.ts | 49 ++++++-- 6 files changed, 246 insertions(+), 54 deletions(-) diff --git a/src/components/TerminalExitBanner.tsx b/src/components/TerminalExitBanner.tsx index 0b4dad62a..8c462380a 100644 --- a/src/components/TerminalExitBanner.tsx +++ b/src/components/TerminalExitBanner.tsx @@ -10,19 +10,30 @@ export interface TerminalExitBannerProps { exitCode: number | null notice: AutoResumeNotice | null onRelaunch: () => void + onCancelAutoResume: () => void } -export function TerminalExitBanner({ mode, exitCode, notice, onRelaunch }: TerminalExitBannerProps) { +export function TerminalExitBanner({ mode, exitCode, notice, onRelaunch, onCancelAutoResume }: TerminalExitBannerProps) { if (notice) { const verb = notice.kind === 'recovering' ? 'auto-resuming' : 'auto-resumed' return (
{mode} crashed (exit {notice.exitCode}) — {verb}, attempt {notice.attempt}/{notice.maxAttempts} + {notice.kind === 'recovering' && ( + + )}
) } diff --git a/src/components/TerminalView.tsx b/src/components/TerminalView.tsx index b2a6f2a16..97e2b8cbb 100644 --- a/src/components/TerminalView.tsx +++ b/src/components/TerminalView.tsx @@ -32,10 +32,11 @@ 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, @@ -604,19 +605,11 @@ 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)) // All hooks MUST be called before any conditional returns const ws = useMemo(() => getWsClient(), []) @@ -4374,6 +4367,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' @@ -4932,6 +4942,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, @@ -5351,6 +5365,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/terminalLifecycleSlice.ts b/src/store/terminalLifecycleSlice.ts index 085a7f28a..037b3e620 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,11 +47,15 @@ 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 @@ -63,13 +68,41 @@ const slice = createSlice({ e.notice = { kind: 'resumed', attempt, maxAttempts, exitCode, at } 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 +113,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/unit/client/components/TerminalExitBanner.test.tsx b/test/unit/client/components/TerminalExitBanner.test.tsx index 33c2870f9..4e485ed4d 100644 --- a/test/unit/client/components/TerminalExitBanner.test.tsx +++ b/test/unit/client/components/TerminalExitBanner.test.tsx @@ -2,6 +2,8 @@ import { describe, it, expect, vi, afterEach } from 'vitest' import { render, screen, fireEvent, cleanup } from '@testing-library/react' import { TerminalExitBanner } from '@/components/TerminalExitBanner' +const 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 +11,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 +20,28 @@ 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', () => { + it('renders a recovering notice with a cancel button instead of the error bar while auto-resume is in flight', () => { + const onCancel = vi.fn() render( {}} />) + onRelaunch={noop} 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 a resumed notice', () => { render( {}} />) + onRelaunch={noop} onCancelAutoResume={noop} />) expect(screen.getByRole('status')).toHaveTextContent('claude crashed (exit 1) — auto-resumed, attempt 2/2') }) }) diff --git a/test/unit/client/components/TerminalView.exitBanner.test.tsx b/test/unit/client/components/TerminalView.exitBanner.test.tsx index 3c78d5af9..4a73d2046 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 @@ -199,6 +200,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 +367,116 @@ 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() + }) - expect(screen.queryByText('claude crashed (exit 1) — auto-resuming, attempt 1/2')).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) + + 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('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 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/terminalLifecycleSlice.test.ts b/test/unit/client/store/terminalLifecycleSlice.test.ts index 7c410a9ce..1d028b725 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,10 +16,37 @@ 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('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('fold clears the exit record, sets a resumed notice, and advances lastTerminalId', () => { @@ -26,7 +54,7 @@ describe('terminalLifecycleSlice', () => { 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')).toEqual({ kind: 'resumed', attempt: 1, maxAttempts: 2, exitCode: 1, at: 2000 }) expect(selectLastTerminalIdFrom(s, 'p1')).toBe('t2') }) @@ -36,7 +64,7 @@ describe('terminalLifecycleSlice', () => { // 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 })) 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 +75,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', () => { From e0c7d193a410f289f86ce0cc0b0435f6343c9872 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:27:53 -0700 Subject: [PATCH 13/17] feat(client): persistent dismissible crash trace on pane content (znhn#1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note: the plan's no-code-change persistence premise held for the persistMiddleware strip (denylist) and persistedState load (passthrough), but normalizePaneContent is a whitelist reachable from hydratePanes — the trace needed an explicit passthrough there to actually survive reload. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- .../2026-07-27-agent-crash-resilience.md | 1 + src/components/TerminalExitBanner.tsx | 99 +++++++++++++------ src/components/TerminalView.tsx | 23 ++++- src/store/paneTypes.ts | 13 +++ src/store/panesSlice.ts | 37 +++++++ src/store/terminalLifecycleSlice.ts | 8 +- .../components/TerminalExitBanner.test.tsx | 41 ++++++-- .../TerminalView.exitBanner.test.tsx | 76 +++++++++++++- .../client/store/panesPersistence.test.ts | 52 ++++++++++ .../store/terminalLifecycleSlice.test.ts | 22 +++-- 10 files changed, 318 insertions(+), 54 deletions(-) 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/src/components/TerminalExitBanner.tsx b/src/components/TerminalExitBanner.tsx index 8c462380a..c8f5c26c4 100644 --- a/src/components/TerminalExitBanner.tsx +++ b/src/components/TerminalExitBanner.tsx @@ -1,56 +1,95 @@ // 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 onRelaunch: () => void onCancelAutoResume: () => void + onDismissCrashTrace: () => void } -export function TerminalExitBanner({ mode, exitCode, notice, onRelaunch, onCancelAutoResume }: TerminalExitBannerProps) { +export function TerminalExitBanner({ + mode, + exitCode, + notice, + crashTrace, + settledDead, + 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} - {notice.kind === 'recovering' && ( - - )} +
) } - 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 97e2b8cbb..89ce11c93 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, @@ -4414,6 +4416,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 @@ -5244,12 +5255,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 ( @@ -5350,6 +5360,9 @@ function TerminalView({ tabId, paneId, paneContent, hidden }: TerminalViewProps) mode={terminalContent.mode ?? 'agent'} exitCode={exitRecord?.exitCode ?? null} notice={activeNotice ?? null} + crashTrace={terminalContent.crashTrace ?? null} + settledDead={settledDead} + 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 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..bf6862bca 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') { @@ -2095,6 +2113,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 +2270,8 @@ export const { resetFreshAgentPaneForReconcileCreate, setPaneReconcileNotice, clearPaneReconcileNotice, + setPaneCrashTrace, + clearPaneCrashTrace, setDeadSessionAdjudication, resolveDeadSessionEntry, clearDeadSessionAdjudication, diff --git a/src/store/terminalLifecycleSlice.ts b/src/store/terminalLifecycleSlice.ts index 037b3e620..50b3cab62 100644 --- a/src/store/terminalLifecycleSlice.ts +++ b/src/store/terminalLifecycleSlice.ts @@ -62,10 +62,14 @@ const slice = createSlice({ 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 diff --git a/test/unit/client/components/TerminalExitBanner.test.tsx b/test/unit/client/components/TerminalExitBanner.test.tsx index 4e485ed4d..0fc7bb3f5 100644 --- a/test/unit/client/components/TerminalExitBanner.test.tsx +++ b/test/unit/client/components/TerminalExitBanner.test.tsx @@ -3,6 +3,12 @@ import { render, screen, fireEvent, cleanup } from '@testing-library/react' import { TerminalExitBanner } from '@/components/TerminalExitBanner' const noop = () => {} +const baseProps = { + crashTrace: null, + onRelaunch: noop, + onCancelAutoResume: noop, + onDismissCrashTrace: noop, +} describe('TerminalExitBanner', () => { // This repo's vitest setup does not auto-cleanup between tests (globals off); @@ -11,7 +17,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' }) @@ -20,16 +26,16 @@ 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 with a cancel button instead of the error bar while auto-resume is in flight', () => { 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. @@ -38,10 +44,27 @@ describe('TerminalExitBanner', () => { expect(onCancel).toHaveBeenCalledTimes(1) }) - it('renders a resumed notice', () => { - render() - expect(screen.getByRole('status')).toHaveTextContent('claude crashed (exit 1) — auto-resumed, attempt 2/2') + 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() }) }) diff --git a/test/unit/client/components/TerminalView.exitBanner.test.tsx b/test/unit/client/components/TerminalView.exitBanner.test.tsx index 4a73d2046..c8ddb7784 100644 --- a/test/unit/client/components/TerminalView.exitBanner.test.tsx +++ b/test/unit/client/components/TerminalView.exitBanner.test.tsx @@ -120,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 } } } @@ -135,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 } }), @@ -460,6 +462,78 @@ describe('TerminalView exited-pane error banner', () => { 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({ 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/terminalLifecycleSlice.test.ts b/test/unit/client/store/terminalLifecycleSlice.test.ts index 1d028b725..46bf330df 100644 --- a/test/unit/client/store/terminalLifecycleSlice.test.ts +++ b/test/unit/client/store/terminalLifecycleSlice.test.ts @@ -49,20 +49,28 @@ describe('terminalLifecycleSlice', () => { expect(selectResumeCycles({ terminalLifecycle: s }, 'p1')).toBeUndefined() }) - it('fold clears the exit record, sets a resumed notice, and advances lastTerminalId', () => { + 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')).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')).toBeUndefined() expect(selectExitRecordFrom(s, 'p1')).toEqual({ exitCode: 1, at: 2000 }) From 0876e46ca76d226f5e9ac121e5fc1077b8e137b5 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:32:24 -0700 Subject: [PATCH 14/17] feat(client): honest Relaunch copy + circuit-breaker banner (znhn#5, znhn#2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- src/components/TerminalExitBanner.tsx | 16 ++++++++-- src/components/TerminalView.tsx | 8 +++++ .../components/TerminalExitBanner.test.tsx | 21 +++++++++++++ .../TerminalView.exitBanner.test.tsx | 30 +++++++++++++++++++ 4 files changed, 73 insertions(+), 2 deletions(-) diff --git a/src/components/TerminalExitBanner.tsx b/src/components/TerminalExitBanner.tsx index c8f5c26c4..c158c749c 100644 --- a/src/components/TerminalExitBanner.tsx +++ b/src/components/TerminalExitBanner.tsx @@ -14,6 +14,12 @@ export interface TerminalExitBannerProps { 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 @@ -25,6 +31,8 @@ export function TerminalExitBanner({ notice, crashTrace, settledDead, + resumeCycles, + canResume, onRelaunch, onCancelAutoResume, onDismissCrashTrace, @@ -55,14 +63,18 @@ export function TerminalExitBanner({ role="alert" className="flex items-center justify-between gap-2 border-t border-destructive/30 bg-destructive/15 px-3 py-1.5 text-sm text-destructive" > - process exited{exitCode !== null ? ` (code ${exitCode})` : ''} + + {resumeCycles != null + ? `${mode} crashed ${resumeCycles} times — auto-resume paused` + : `process exited${exitCode !== null ? ` (code ${exitCode})` : ''}`} + ) diff --git a/src/components/TerminalView.tsx b/src/components/TerminalView.tsx index 89ce11c93..ea893b9e5 100644 --- a/src/components/TerminalView.tsx +++ b/src/components/TerminalView.tsx @@ -43,6 +43,7 @@ import { selectActiveNotice, selectExitRecord, selectLastTerminalIdFrom, + selectResumeCycles, } from '@/store/terminalLifecycleSlice' import { TerminalExitBanner } from '@/components/TerminalExitBanner' import { dismissTabGreen } from '@/store/turnCompletionAttention' @@ -612,6 +613,9 @@ function TerminalView({ tabId, paneId, paneContent, hidden }: TerminalViewProps) // 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(), []) @@ -5362,6 +5366,10 @@ function TerminalView({ tabId, paneId, paneContent, hidden }: TerminalViewProps) 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 diff --git a/test/unit/client/components/TerminalExitBanner.test.tsx b/test/unit/client/components/TerminalExitBanner.test.tsx index 0fc7bb3f5..833223604 100644 --- a/test/unit/client/components/TerminalExitBanner.test.tsx +++ b/test/unit/client/components/TerminalExitBanner.test.tsx @@ -5,6 +5,8 @@ import { TerminalExitBanner } from '@/components/TerminalExitBanner' const noop = () => {} const baseProps = { crashTrace: null, + resumeCycles: null, + canResume: false, onRelaunch: noop, onCancelAutoResume: noop, onDismissCrashTrace: noop, @@ -67,4 +69,23 @@ describe('TerminalExitBanner', () => { ) 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 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 c8ddb7784..de929a648 100644 --- a/test/unit/client/components/TerminalView.exitBanner.test.tsx +++ b/test/unit/client/components/TerminalView.exitBanner.test.tsx @@ -551,6 +551,36 @@ describe('TerminalView exited-pane error banner', () => { 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 From 74918c5c4fd448008112d312634dd6176f97c833 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:59:01 -0700 Subject: [PATCH 15/17] test(e2e): crash trace survives reload; breaker banner; cancel clears immediately (znhn#1,#2,#3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- .../fixtures/fake-crashing-claude-cli.mjs | 13 +- .../specs/agent-crash-autoresume-rust.spec.ts | 120 +++++++++++++++++- 2 files changed, 126 insertions(+), 7 deletions(-) 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) } From 5a5b0ad8b2de7830260f0c298085954a93e1df8c Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Wed, 29 Jul 2026 19:35:32 -0700 Subject: [PATCH 16/17] fix(client): fresh reconcile clears the stale crash trace (znhn#1 fresh-eyes) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- src/store/panesSlice.ts | 5 +++++ .../client/store/panesSlice.reconcile.test.ts | 15 +++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/store/panesSlice.ts b/src/store/panesSlice.ts index bf6862bca..877fe2529 100644 --- a/src/store/panesSlice.ts +++ b/src/store/panesSlice.ts @@ -1997,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. 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 { From 9a76924202e089d48a31db5fd5a8eb5efbd9dd84 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Wed, 29 Jul 2026 19:35:55 -0700 Subject: [PATCH 17/17] fix(auto-resume): clean up unconsumed cancel entries on every settle/replaced tail (znhn#2 fresh-eyes) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A cancel landing after the hub's post-sleep take_cancel check (or for a terminal that settles without ever reaching the Resume arm) leaked in WsState.auto_resume_cancels forever. Every settle tail and the respawn match tail now consume the entry, so 'removed on consumption' holds on every path. Too late to abort at those points — cleanup, not abort. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- crates/freshell-ws/src/auto_resume.rs | 96 +++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/crates/freshell-ws/src/auto_resume.rs b/crates/freshell-ws/src/auto_resume.rs index 95102b564..cf76ab930 100644 --- a/crates/freshell-ws/src/auto_resume.rs +++ b/crates/freshell-ws/src/auto_resume.rs @@ -356,6 +356,13 @@ async fn run_hub_body( } } } + // 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"); @@ -386,11 +393,17 @@ async fn run_hub_body( { 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 { @@ -436,6 +449,13 @@ async fn run_hub_body( 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); } } } @@ -1096,6 +1116,11 @@ mod tests { 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, @@ -1128,6 +1153,7 @@ mod tests { 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, @@ -1172,6 +1198,14 @@ mod tests { 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)> { @@ -1257,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) @@ -1813,6 +1858,57 @@ mod tests { 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