fix(codex): sidecar lifecycle — persistent tracking, reattach on restore, conservative reaping (kata ynfn, da92) - #641
Merged
Conversation
…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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 resumefails too.What this PR builds
All changes in the Rust server, primarily
crates/freshell-codex + freshell-server + freshell-wstests:rust-codex-sidecars) recording pid, ws port, session/thread id, and process-identity evidence for every sidecar freshell spawns; atomic writes under~/.freshell.codex --remote ws://127.0.0.1:<port>(in-flight turn preserved), with fresh-spawn fallback when no usable sidecar exists.Invariant: after restart every tracked sidecar is reattached, reaped, or intentionally retained with a recorded reason — never silently orphaned.
Process & evidence
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.cargo fmt/clippy/test --workspaceall pass;npm run checkgreen 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.🤖 Generated with Amplifier
Co-Authored-By: Amplifier 240397093+microsoft-amplifier@users.noreply.github.com