Resume validation: disk-existence gate before resuming coding-CLI sessions - #591
Merged
Conversation
…ings Nine validators ran against the plan's load-bearing assumptions (ledger in .worktrees/.the-usual-logs/resume-validation/). Falsified assumptions fixed: locator error-accumulator contract (false-Absent IO paths), cold-index coverage via direct amplifier/claude locators, spawn_blocking probe calls, gate moved after the D7 liveness guard, mandatory accepted_session_ref guard + Start threading at door 3, notice-triggered client ref clear for codex/opencode, corrected protocol-mirror mechanics (shared/ws-protocol.ts + regenerated schema), and claude coverage honestly rescoped to same-boot deletions. Design decisions AD-1..AD-4 recorded.
Independent cross-model plan review found two blocking executable-plan defects; both fixed: 1. Door 3 (Task 8) gate ordering: the REST create pipeline puts the amplifier ensure (:895) BEFORE the REST D7 live-session guard (:968-988) and D8 lease, so the gate cannot be protected by after-D7 placement like door 1 - a gate fire would falsify D7-REST's applicability filter, bypass its loud CONFLICT reject, and retire the Bound ledger row of a RUNNING session (live zero-turn codex has no rollout on disk). Added a MANDATORY in-gate liveness precondition reusing the D7-REST guard's own consult, a passthrough branch in the wiring snippet, and a live-session test pinning reject preservation and never-invoked on_stale_resume. 2. Task 6 in-gate liveness snippet was a sync is_some_and closure that cannot host the async half of D7's join (fresh-agent sidecar has_live_session(sid).await) - the sidecar arm was silently droppable while all planned tests passed. Reshaped to an async-capable let (both arms load-bearing, copy D7's join verbatim) and added integration case 6 pinning the sidecar arm (sidecar-only liveness, registry empty, probe Absent, legacy carrier - fail open). Self-Review record, placeholder scan, and DEVIATIONS pinning-tests text updated to match (cases 4-6, door-3 liveness precondition).
…er fresheyes round 2 Fresheyes iteration 2 found the door-3 in-gate liveness precondition reused the REST D7 guard's registry-only consult, which is PTY-scoped and blind to sessions live inside the fresh-agent sidecars — the exact live-zero-turn-codex-with-no-rollout hazard the plan cites. Task 8 now mandates a two-arm join: the D7-REST registry consult PLUS a new injected async SidecarLivenessProbe (FreshAgentState::with_sidecar_liveness, built in main.rs from the same sidecar instances the WS door consults, with_* builder to avoid the FreshOpencodeState Arc cycle), a step-4 sidecar-live pinning test that fails RED if the arm is dropped, and matching updates to the Interfaces, main.rs wiring snippet, Self-Review attestations, and the DEVIATIONS pinning-test list.
Round-3 fresheyes review found two major blocking issues in the plan; both are resolved: 1. Task 8 main.rs wiring was written against a code shape that does not exist (cloned a never-named 'existence_probe' binding; pinned the builder chain to :408-411, before pane_ledger exists at :527). The wiring now specifies a pure hoist of the inline WsState session_existence initializer into a named binding plus ONE new final consuming rebinding after pane_ledger and the hoisted probe exist, with an inline soundness argument (all door-3 consumers clone after that site; the :260 FreshOpencodeState capture predates every door-3-relevant builder by existing design; OnceLock setters at :494/:542 unaffected). 2. The plan never reconciled the gate with the designed amplifier never-used-stub GC / ensure-after-GC restore contract. Now recorded as an explicit accepted deviation AD-5 (Design Notes) with rationale: a GC'd never-used stub is indistinguishable on disk from the incident's stale id, the outcome for a never-typed pane is an equivalent empty session either way, and reconcile's amplifier Absent carve-out (parking decision) is untouched. Cross-referenced in the Global Constraints fail-open invariant, the Door 2 'Absent is near-impossible' framing (now scoped to non-amplifier modes), Task 1's policy comment + amplifier test, Task 6 integration case 1, the Task 9 DEVIATIONS entry, and the Self-Review Record.
- Add AmplifierSessionAnswer enum with three states: Present, Absent, Unreadable - Implement session_on_disk() function for read-only session existence scanning - Scan ALL project slugs under amplifier_home/projects/ (not just cwd-derived slug) - Tri-state semantics with errors-seen accumulator: - Present: session dir found anywhere (short-circuits) - Absent: projects/ missing OR scanned without errors/hits - Unreadable: projects/ unreadable OR per-entry errors with no hit (fail-open) - 8 comprehensive tests covering present, absent, divergent slug, and 5 permission/access scenarios - All tests passing, clippy clean
… file to honor 1K-line cap Moved the eight session_on_disk test functions (and their helpers SCAN_COUNTER, scan_temp_home, running_as_root) from amplifier_stub.rs (1049 → 906 lines) into a new sibling module amplifier_stub_scan_tests.rs (148 lines), wired up via #[path] attribute per the pane_ledger_tests.rs pattern. All 8 tests verified running and passing: - session_on_disk_present_under_cwd_slug - session_on_disk_present_under_divergent_slug - session_on_disk_absent_when_store_readable - session_on_disk_absent_when_projects_dir_missing - session_on_disk_unreadable_projects_dir_fails_open - session_on_disk_unreadable_project_subdir_fails_open - session_on_disk_listable_not_traversable_projects_fails_open - session_on_disk_matches_a_file_only_as_absent Clippy: clean (no warnings). Generated with Amplifier (https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Two problems, one probe change (plan Task 3): 1. Stale warm snapshots: a session created moments before a restart can be missing from the snapshot, and peek() serves snapshots regardless of TTL. The claude/opencode arms already had by-id fallbacks; amplifier and codex now get them too, so Absent for all four validated providers is POSITIVE absence. The codex fallback is a NEW gate-safe tri-state walk (errors-seen accumulator), deliberately NOT the fail-soft locate_codex_rollout (silent None on per-entry errors => false Absent). 2. Cold index at boot (A1): the snapshot is None every boot and the warm sweep is a detached spawn, so restore-time creates race it. exists() now runs the cheap amplifier/claude by-id locators when cold; codex/opencode stay Unknown (AD-4: the ~1s codex walk is warm-Absent adjudication only). The root-missing => ProviderUnavailable pre-check is untouched. New locator logic lives in a focused existence_by_id.rs (existence.rs is already large) and is re-exported as existence::*; tests live in a sibling existence_resume_validation_tests.rs included from existence.rs's test module (amplifier_stub scan_tests precedent). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…d in the pane Additive optional field mirrored Rust protocol -> shared/ws-protocol.ts -> regenerated contract schema (57-entry inventory unchanged, Node never sets it). Client writes the notice into the pane's xterm like the reconcile notice and clears the persisted sessionRef/resumeSessionId so codex/opencode gate-fired spawns (no sessionRef on the frame) don't re-fire the gate on every restart; a frame that also carries a sessionRef keeps the existing overwrite fold. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
… existence Door 1: handle_create consults the disk-existence probe (via the pure resume_gate policy) before turning a WIRE-derived resume id into resume argv. Positive absence => provider-shaped fresh-spawn fallback (claude: fresh UUID + Start + prealloc; amplifier: fresh UUID keep Resume; codex/opencode: drop resume), operator notice on terminal.created, and retire_missing on the pane ledger. Unknown/ProviderUnavailable and live sessions (registry + async fresh-agent sidecar arms) always fail open. Placed AFTER the D7 cross-mode liveness guard and BEFORE the amplifier ensure_session re-stub; the sync helper runs in spawn_blocking. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…istence
Door 2 of the resume-validation feature: respawn_agent_terminal now runs
the shared validate_wire_resume helper (spawn_blocking, A13) before
turning the cached session id into resume argv. On positive absence the
respawn proceeds fresh under a minted id, the stale ledger row is
retired SessionMissing, a terminal.status{recovering, reason} frame is
broadcast (headless path has no out sink), and a tracing::warn is
logged. The gate replaces the resume_session_id LOCAL (V8 \u00a7A9) so all
post-spawn bookkeeping (registry row, identity upsert, record_binding,
amplifier ensure_session) records the FRESH id \u2014 no respawn loop.
Present/Unknown proceed byte-for-byte as today (fail-open invariant).
AD-3 (stale locator lease stays bound to the fresh terminal) accepted
and documented at the wiring site.
\U0001f916 Generated with [Amplifier](https://github.com/microsoft/amplifier)
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
… existence Door 3: before the REST create pipeline (POST /api/tabs, /api/panes/:id/split, /api/panes/:id/respawn) turns a cached resume id into resume argv, consult the disk-existence probe; on POSITIVE absence, spawn fresh (claude -> minted v4 + Start, amplifier -> minted v4 + Resume, codex/opencode -> no resume), clear the stale wire ref, retire the ledger row via on_stale_resume, and inject the stale-resume notice as paneContent.reconcileNotice. - validate_rest_resume in terminal_tabs.rs (sync; run in spawn_blocking, A13), gated by the MANDATORY two-arm in-gate liveness precondition: the D7-REST guard's own registry consult (shared, not reimplemented) + the injected async sidecar_liveness probe (PTY-scoped consult is blind to sidecar-live sessions). A LIVE candidate skips the gate byte-for-byte. - FreshAgentState: resume_probe / on_stale_resume / sidecar_liveness fields + consuming with_* builders (with_opencode_locator shape; None = today's behavior). - main.rs: hoist the session_existence probe out of the WsState literal into a named binding (arms byte-identical) and add the door-3 wiring as the LAST fresh_agent_state rebinding (needs pane_ledger + the hoisted probe). Fail-open invariant preserved: Unknown/ProviderUnavailable and unvalidated providers proceed unchanged; probe unwired = passthrough. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
… validation Appends DEV-0010 to port/oracle/DEVIATIONS.md recording the deliberate divergence from the Node reference: resume session ids are validated against the provider's on-disk session store at all three spawn doors (WS terminal.create, headless auto-resume respawn, freshagent REST create), spawning fresh with an operator notice and retiring the pane-ledger binding on positive absence, failing OPEN on unknown/unreadable stores. Entry status: proposed (pending antagonist adjudication per the ledger's entry rules). Full verification sweep (Task 9): - cargo fmt --all -- --check: clean - cargo clippy (6 crates, -D warnings): clean - cargo test freshell-platform/sessions/protocol: all green (439 passed) - cargo test freshell-server/ws/freshagent: all green (1226 passed). One load flake on the first run only: freshell-ws pane_ledger::tests::new_locked_degrades_to_disabled_when_another_holder_exists (flock EWOULDBLOCK under parallel load); passed isolated and in the full re-run. Pre-existing lock-contention test, unrelated to this feature's changes. - npm run test:unit (coordinated): 372 files / 4379 tests, all green. The known pre-existing terminal-font-settings.test.tsx failure from main did NOT reproduce in this run. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…llers via exists_for_gate Fresh-eyes review (iteration 1) found a blocking async-runtime regression: the codex by-id rollout walk (~1s warm-cache) was attached to the shared IndexExistenceProbe::exists(), and while all three spawn doors wrap the probe in spawn_blocking (A13), the pre-existing reconcile-path callers - derive_verdicts (reconcile.rs:318) and build_snapshot (reconcile_freshagent.rs:138) - run exists() inline on the async runtime, per dead codex pane, on every WS ready, twice under the warming deferral. That violates the reconcile path's documented ~250ms sync-IO budget (see the opencode locator doc in existence.rs). Fix: split the probe API along the caller boundary. - SessionExistenceProbe gains a DEFAULTED exists_for_gate() (same shape as the existing defaulted ever_observed_on_disk), delegating to exists(), so NoIndexProbe and all seven test fakes are unchanged. - IndexExistenceProbe::exists() drops the codex warm-Absent walk; the new exists_for_gate() override runs base exists() and adjudicates a warm codex Absent through the rollout locator (Present => observed-set + Present, Unreadable => Unknown). AD-4 preserved: cold codex answers Unknown from exists(), and the walk only fires on Absent, so it still never runs on the cold path. The cheap amplifier scan and the claude/opencode fallbacks stay on exists() (pre-existing budgeted IO). - Gate callers retargeted: validate_wire_resume (doors 1+2, resume_validation.rs) and the door-3 ResumeProbeFn closure (main.rs) now consult exists_for_gate(); the reconcile callers stay on exists(). - Tests: the three probe-consulting codex fallback tests retargeted to exists_for_gate; new regression test reconcile_exists_never_runs_codex_walk_gate_variant_does pins that exists() answers warm-Absent codex WITHOUT invoking the locator while exists_for_gate() on the same probe invokes it and answers Present. Trade-off (deliberate): reconcile's codex warm-Absent no longer gets the walk rescue, restoring pre-branch behavior there (a stale snapshot can transiently answer Absent until the kicked refresh lands) in exchange for keeping the reconcile handshake inside its sync IO budget. Verified: cargo fmt; cargo check --tests (server/ws/freshagent); existence + resume_validation unit suites; resume_validation_gate, pane_reconcile, auto_resume_respawn, pane_reconcile_freshagent integration suites; clippy -D warnings (server/ws) - all green.
…findings Validation pass (9 verified, 2 falsified, 1 accepted residual; full trial rebase ran green end-to-end in a sandbox clone): - fix branch commit counts to 18/17/'17 or 18' (docs plan commit at tip) - rewrite Task 2's test observation to main's RecordingBinder seam (PIN 2 is binder-gated; no ledger-query API exists) + pin that the OR-folded flag must not re-key main's mint - Task 1: GIT_EDITOR=true continues, main.rs may auto-merge (verify content either way), mid-rebase check carve-out for the known E0063, keep mut accepted_session_ref, cite the trial's exact 3 in-pattern errors - Task 4: sync wait_for_captured_argv, drop unproducible ever_observed_on_disk, CODEX_HOME temp-dir hygiene, textual gate-above-plan check on any manual relocation (false-green window) - Task 5: fill auto_resume_cancels read idiom + frame-capture facts - Task 7: 2h coordinator wait bound, baseline-attribution rule for client gates (Rust-side baselines verified green on main), expected clippy lint
…commit branch shape) Fresh-eyes iteration 4 caught that the plan-hardening commit 0e077fd4b made the branch 19 commits while Task 1's verification gates still pinned 18/17/"17 or 18" and claimed the tip was 948e52b5f. Updated: background facts, Step 1/Step 3 wc -l expectations, Step 9 replay count, Task 1 final audit, Task 7 Step 5 range (18-24), and the validation-pass count references.
…ure N, express gates relative to it) Fresh-eyes review caught the count-staleness defect a third time: each plan-fix commit invalidated the hard-coded totals (19 pinned, actual 20). Task 1 Step 1 now measures N and structurally verifies 17 feature commits under a docs-only top run; Steps 3/9, the final check, Task 7 Step 5, and the validation-pass paragraph are expressed relative to N, so amending this plan can no longer break its own gates.
Main #584 added should_preallocate_fresh_claude keyed on 'no resume id'; a gate-fired claude fallback carries a minted id and silently skipped the pre-spawn ledger binding. Thread claude_fresh_prealloc through RestResumeOutcome and OR it into the prealloc decision.
…CLI codex tests Post S5.e the flag defaults ON; unpinned codex-terminal tests would spawn the real codex binary (226s retry burn on codex-less hosts). Mirrors 0b3082c's pin pattern. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…aunch New host-gated e2e: a definitively-absent codex resume id at the WS door is gated (fresh spawn + notice) and never reaches managed planning — no resume tokens in the captured argv, so a stale id can't burn a sidecar planning slot. Uses S5's dispatcher/CODEX_CMD/argv-capture seam.
…ngle-counted One Recovering frame per gate-fired respawn, no synthetic breaker settle, auto_resume_cancels untouched. The gate reduces breaker trips (kills missing-session flap at the source) without masquerading as one.
Records the gate-before-plan ordering at all three doors and the PIN 2 prealloc threading; verifies no stale 'default OFF' comments and no client double-messaging between the created-notice and #582 banners.
…eturn to satisfy clippy type_complexity Mechanical fix for the one clippy lint on the merged tree (predicted by the reconciliation plan's 2026-07-30 trial rebase). No behavior change. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…@2641ada38 (#589 reconciliation)
…ORDER red test, corrected baselines (28 commits), storm-pin load-attribution and node_modules guards
…tProbe in Arc::new to match spawn_server_with_probe's Arc<dyn SessionExistenceProbe> parameter; replace mispredicting harness-signature contingency with the real contract (fresheyes iteration 6)
…(fresheyes iteration 7) — Task 4 probe now Absent-but-observed (ever_observed=true) so the claude zero-turn carve-out cannot swallow the mutation-proof and the never-gated pin actually bites; Task 3 payloads carry required shell field, Interfaces/self-review corrected
…_id_from_wire origin semantics
…mplifier prealloc)
…reconcile resume-validation gate-before-plan with #589
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Incident
Freshell resumed a cached Amplifier session ID that never existed on disk. The CLI silently fabricated an empty 'resumed' session instead of failing or creating a fresh one. This breaks users' assumptions about continuity and leaves them stranded in a broken context.
Mechanism
Pure resume-gate policy in freshell-platform with disk existence probes:
Disk existence probes — kept off hot reconnect paths:
Resume-gate policy — three spawn doors (all gated):
State tracking — RetiredReason::SessionMissing + retire_missing in pane ledger
User feedback — optional notice on terminal.created; client stale-ref clearing
Contract
Reconciliation
Rebased over codex-managed-launch S5 (default-ON) with:
Gate placement reconciled per deviation ledger; see commit 'docs: reconcile deviation ledger with off-permit codex gate placement'.
Verification
Note
Partially delivers the Rust-parity follow-up recorded in #583.
Deferred (non-blocking)