Skip to content

fix(codex): sidecar lifecycle — persistent tracking, reattach on restore, conservative reaping (kata ynfn, da92) - #641

Merged
danshapiro merged 18 commits into
mainfrom
the-usual/codex-sidecar-lifecycle
Aug 11, 2026
Merged

fix(codex): sidecar lifecycle — persistent tracking, reattach on restore, conservative reaping (kata ynfn, da92)#641
danshapiro merged 18 commits into
mainfrom
the-usual/codex-sidecar-lifecycle

Conversation

@danshapiro

Copy link
Copy Markdown
Owner

Context

A freshell server restart exposed two bugs in the codex terminal-pane sidecar lifecycle.

Kata ynfn — orphan leak: freshell spawns a detached codex app-server sidecar per codex pane; on restart the new server had no memory of them, restore spawned fresh sidecars, and surviving old ones were orphaned forever (~20 accumulated on the host).

Kata da92 — restore blind spot: a sidecar mid-agent-turn survives restart holding its codex thread as active writer; restore's fresh-sidecar resume is then rejected with JSON-RPC -32600 "already has an active writer", so the pane fails to restore and manual codex resume fails too.

What this PR builds

All changes in the Rust server, primarily crates/freshell-codex + freshell-server + freshell-ws tests:

  • Durable sidecar record store (rust-codex-sidecars) recording pid, ws port, session/thread id, and process-identity evidence for every sidecar freshell spawns; atomic writes under ~/.freshell.
  • Process-identity verification so a recycled pid can never be mistaken for a recorded sidecar.
  • Store-gated detach at spawn ("no store, no behavior change" guarantee).
  • Boot-time reconciler with claim-by-session: restored panes REATTACH to a surviving verified sidecar via codex --remote ws://127.0.0.1:<port> (in-flight turn preserved), with fresh-spawn fallback when no usable sidecar exists.
  • Conservative tree-aware reap sweep for tracked-but-unclaimed sidecars: identity re-verified before every kill signal, whole process tree torn down, ambiguous states never signalled, /proc writer-evidence check errs toward leaving processes alive, plus a race guard so cleanup can never kill a sidecar just reclaimed by a restoring pane.
  • Shutdown retention of adopted sidecars + end-to-end websocket reattach test + server wiring.

Invariant: after restart every tracked sidecar is reattached, reaped, or intentionally retained with a recorded reason — never silently orphaned.

Process & evidence

  • Built via the-usual workflow: plan at docs/plans/2026-08-10-codex-sidecar-lifecycle.md, 10 load-bearing assumptions checked (7 falsified and fixed pre-execution), 4-round plan review PASSED, 11/11 tasks executed with per-task review, whole-branch review "Ready to merge: Yes", independent delta review PASSED with zero blockers.
  • Post-rebase gates (rebased onto origin/main 44030ca): cargo fmt/clippy/test --workspace all pass; npm run check green except 3 client-side failures proven pre-existing: test/e2e/pane-context-menu-stability.test.tsx (2 tests) fails identically on origin/main 44030ca, and one FreshAgentView flake passes in focused rerun. Branch touches no client files.
  • Follow-up katas filed: 4g2a (Node-server da92 parity), wfah (other lane's orphan leak).
  • Fixes kata ynfn and kata da92.

🤖 Generated with Amplifier

Co-Authored-By: Amplifier 240397093+microsoft-amplifier@users.noreply.github.com

danshapiro and others added 18 commits August 11, 2026 02:48
…conservative reaping (katas ynfn+da92)

Plan reconstructed from the crashed write-plan session's recovered step
prompt, kata bodies, and surviving exploration reports (provenance note in
the document header).

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…ar lifecycle plan

Ten assumptions validated (3 verified, 7 falsified); fixes applied:
thread/read (not loaded/list) as the mid-turn discriminator with an
fd-writer-evidence arm for unreachable survivors; tree-aware verified kill
helper (sidecars have children in their own pgids); reconciler re-keyed by
ownership_id with writer-aware claims and duplicate-session accounting;
Retained rows stay claimable so late restores still reattach; freshagent-lane
orphaning recorded as a scoped follow-up; SAFE-11 step corrected to
verify-unchanged + recorded documentary deviation; measured cargo baseline
(ws: 2 pre-existing failures) and store-hardening notes (loud disablement,
O_CLOEXEC lock fd, systemd KillMode caveat) recorded. Evidence ledger:
.worktrees/.the-usual-logs/codex-sidecar-lifecycle/load-bearing-ledger.md

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…e the claim path async

Independent review found two executable-spec defects: (1) Task 9's required
threadStatuses fixture knob was absent from the plan's named files, Task 9's
commit, and would break Task 11's binding files audit — the fixture is now in
the File Structure table, Task 9's Files list, a new Step 0 (backward-
compatible default-idle knob), and Task 9's git add + commit message;
(2) claim_for_session's writer-preference ws probe was spec'd sync behind a
sync factory inside async plan_create — claim_for_session and
select_codex_runtime are now async and CodexRuntimeFactory returns a boxed
future that plan_create awaits, with lock-snapshot-before-await guidance and
a deterministic probe-fallback note in Task 5's duplicate test. Fresh-eyes
pass recorded at the end of the plan.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…erification gates

Two verification-gate defects found by the second independent review:
- Tasks 3/4 ran `--test codex_managed_launch_e2e` without
  `-- --ignored --test-threads=1`; the binary's only test is #[ignore]-gated,
  so the gate ran zero tests. Both invocations now include the flags.
- Every whole-package `cargo test -p freshell-ws` gate aborted at
  auto_resume_e2e under cargo fail-fast (baseline covered 3 of ~44 binaries).
  Baseline bullet now mandates --no-fail-fast on all ws gates, a pre-Task-1
  full-suite baseline recording, and failure-set-equality semantics for
  "baseline-identical"; Task 7 Step 4 and Task 11 Step 1 updated.

Self-review re-run recorded in "Fresh-eyes review pass (iteration 2)".
…ose the claim/sweep race

Four blocking executable-spec defects from the third independent review:
(1) Task 11's binding safety audit is scoped to the code diff
(crates/ + test/fixtures/) so the plan document itself — which legitimately
contains the forbidden kill-pattern strings — no longer trips it;
(2) Task 3's detach (kill_on_drop(false) + process_group(0)) is now store-
and platform-gated with a #[cfg(unix)] guard, a disabled-store backstop
test, and a matching Task 10 retention gate (record-less sidecars are never
retained); (3) claim/sweep TOCTOU closed: held-membership is the single
source of truth, the sweep re-checks it under lock immediately before any
signal (new SkippedClaimedDuringSweep outcome + deterministic guard test);
(4) kill_verified_sidecar_tree is spec'd async (tokio sleeps, no locks
across awaits). Self-review re-run recorded in the iteration-3 section.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…cars)

Schema-v1 JSON records (pid, starttime, cmdline, ws url, ownership id,
session/terminal ids, state) in ~/.freshell/rust-codex-sidecars/, written
atomically (sibling tmp + fsync + rename, tabs_persist.rs precedent) under a
flock single-writer with the PaneLedger disabled-fallback and per-row
quarantine policies. Deliberately a distinct store from Node's
~/.freshell/codex-sidecars/ (rust-session-cache.json anti-two-writer
precedent). Groundwork for kata ynfn/da92.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
(pid, starttime, cmdline) capture from /proc and a four-way verdict
(Verified/Dead/Mismatch/Unverifiable). Stale pids are never trusted: only
Verified may ever be signalled; environ tags are not required (YAMA can hide
them for reparented orphans). starttime parsing duplicated with provenance
from session_lease.rs (kata ynfn groundwork).

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…racked sidecars from server death

SpawnedCodexAppServerRuntime writes a verified (pid, starttime, cmdline,
ws url) record on successful spawn, enriches it at adopt, and removes it on
explicit shutdown. Tracked spawns (Linux + enabled store) switch
kill_on_drop(true) -> false + process_group(0): a dying server no longer
silently kills or silently orphans them — an unclean death leaves a TRACKED
record for boot reconciliation (kata ynfn). Untracked spawns (disabled store
or non-Linux) keep today's attached kill_on_drop backstop: a record-less
sidecar must never outlive the server.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
New note_session_id seam (default no-op on the runtime trait): plan_create
notes resume ids at plan time; the freshell-ws proxy-event router notes
captured thread candidates beside mark_candidate_persisted. Records now carry
the session id restore-time reattach keys on (katas ynfn/da92).

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Loads rust-codex-sidecars records at boot, prunes Dead records and removes
Mismatch ones without ever signalling (pid reuse is never trusted), and holds
Verified survivors as one-shot claimable by codex session id, re-verifying at
claim time (katas ynfn/da92).

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…stead of spawning

ReattachedCodexAppServerRuntime implements CodexLaunchRuntime over a claimed
record: ensure_ready re-verifies identity and probes the existing listener
(3s budget, fail-fast into the structural fresh-spawn fallback via the plan
retry loop). Unusable-but-verified survivors are reaped; mismatched pids are
never signalled. Teardown kills only after re-verification (kata da92).

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
CodexRuntimeFactory now receives the pure launch plan and returns a boxed
future that async plan_create awaits (the claim's duplicate arm awaits a
bounded ws writer probe); the global manager's factory claims a verified
surviving sidecar for resume plans via the reconciler and mints
ReattachedCodexAppServerRuntime, else the spawn runtime.
Claim consumption makes fresh-spawn fallback structural through the existing
plan retry loop (kata da92).

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…r's store-None arm

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…(da92)

terminal.create{restore:true, sessionRef} against a tracked surviving fake
app-server routes the TUI's thread/resume to the SURVIVOR (mid-turn state
preserved, pid untouched); with no tracked survivor the fresh-spawn path is
byte-compatible with today, and the scripted -32600 active-writer rejection
is confined to that fresh path — the incident shape, now guarded.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…urn (ynfn)

sweep_unclaimed reaps only sidecars freshell recorded AND re-verified by
(pid, starttime, cmdline) at kill time, tree-aware (descendants snapshotted
and verified before any signal); mid-turn survivors (thread/read status
active — loaded alone is NOT mid-turn) are retained with a recorded reason
and stay claimable by late restores; unreachable-but-writer-holding survivors
are retained, not killed; mismatched/unverifiable pids are never signalled.
Invariant encoded in
restart_reconciliation_leaves_no_sidecar_silently_orphaned: every tracked
sidecar (including session-id duplicates) ends reattached, reaped, or
retained-with-reason. The repo-owned fake app-server fixture gains a
per-thread threadStatuses knob so tests can script thread/read status.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…ecars at shutdown

Boot constructs the flock'd rust-codex-sidecars store, reconciles records
(prune stale, hold verified survivors claimable), and arms the grace-delayed
conservative sweep (FRESHELL_CODEX_SIDECAR_REAP_GRACE_MS, default 30m —
restores arrived 18m post-boot in the incident). Graceful shutdown now
RETAINS adopted codex sidecars with a recorded reason instead of killing them
(kata ynfn: surviving restarts is a feature); unadopted mid-plan sidecars are
still torn down. SAFE-11 expectations updated with recorded rationale.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…conservative loaded-list parsing, retention/adopt state fidelity

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
origin/main added WsState.handshake_settings (CFG-12) while this branch
was in flight; initialize it in codex_sidecar_reattach_e2e's spawn_server
the same way the other freshell-ws e2e tests do.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
@danshapiro
danshapiro merged commit fc40185 into main Aug 11, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant