feat(ws): graceful restore/resume S1 — restores queue instead of dying under load - #589
Merged
Conversation
… validation findings Load-bearing assumption review (10 verified, 5 falsified) drives these fixes: - A13: add restore_spawn_gate.rs, create_protection.rs, rest_ws_shared_gate.rs as Modify + Task 4 step rewriting the four restore-path gate-timeout pins to timeout-free mechanisms (gate Timeout unreachable for restore class) - A12: narrow the P1 hoist to pure rungs + codex prep; claude P0.4 ladder stays at its original post-adopt/D8 position (hoisting it converts adoptions into loud RESTORE_UNAVAILABLE in the reconcile race) - A4: fresh-planned codex restores (no resume session id) excluded from pre-gate planning (45s candidate-capture timer would fire while parked) - A6: wrap ensure_ready's connect_async probes in the remaining SIDECAR_START_BUDGET timeout so plan-budget holds are truly bounded - A5: periodic warn logging on long unbounded gate waits/holds (restores the ops signal the removed 10s timeout provided); residual recorded - A8: discard_sync uses Handle::try_current(), never panics in Drop - A10/V6-N1: storm fairness pin restructured to deterministic park/release waves; harness pins mcpServer:false (tsx unresolvable in bare worktrees) - honest structural-bound arithmetic (~251s/plan, ~2.2h cap-64 worst case) Ledger: .the-usual-logs/graceful-restore-resume-s1/load-bearing-ledger.md
…cpServer pin with node-deps precondition Independent review found two blocking defects sharing one root cause: 1. The Task 5 V6-N1 'pin mcpServer: false' mitigation was fictional: settings.codingCli.mcpServer is never read in the create path (declared protocol/settings.rs:80, defaulted server/settings.rs:44, whitelisted settings_store.rs:1467-1471 — no consumer), and handle_create runs generate_mcp_injection unconditionally for every non-shell mode (terminal.rs:2043-2063), requiring node_modules/tsx/dist/loader.mjs (mcp_inject.rs:123-128). Rewrote the V6-N1 ground rule: the pin is a documented no-op; the real mechanism is installed node deps. 2. Verification gates self-contradicted: the plan asserted codex_session_ref_resume fails in this bare worktree while Tasks 3/4/6 expected it (and full ws/workspace suites) green, with no dependency-install step. Added a Global Constraints environment precondition (test -f node_modules/tsx/dist/loader.mjs || npm ci --no-audit --no-fund; precedent: 2026-07-24-rust-attach-viewport.md:26, 2026-07-29-rebind-review-polish.md:1207-1210) and wired it into Task 3 Step 1, Task 4 Step 9, and Task 6 Step 2.
…ing pin invocation in Task 3 Step 1 The plan invoked the ordering pin as an integration test (cargo test -p freshell-ws --test terminal_create_ordering_tests), but it is a lib unit-test module (src/terminal_create_ordering_tests.rs, declared via #[path] in terminal.rs:72-73), so that command errors with 'no test target' and the step's Expected: all PASS was unachievable. The built-in fallback also searched the wrong directory (tests/). Fixed: invoke via --lib with the terminal_create_ordering filter (verified live: 1 passed) and point the fallback at grep -rn terminal_create_ordering crates/freshell-ws/src/.
…al teardowns in the restore-storm suite The independent plan review found the Task 5 restore_storm.rs design's shared shutdown_calls counter is contaminated across serialized tests: killing ADOPTED codex terminals only QUEUES their manager teardowns (kill joins the PTY reader thread whose exit hook sends to the detached worker), so the increments land AFTER the test returns and TEST_LOCK is released, corrupting the next test's exact-count discard-arm asserts. Fixes (plan-only, Task 5): - New ground rule + harness helper drain_adopted_teardowns(c, N): the two adopting tests (mandate storm: 8, deterministic-failure: 7) drain the queued teardowns to their exact totals before releasing TEST_LOCK. Deterministic — every send is already queued when kill_all() returns. - New ground rule + storm_rt() helper: all storm tests run on ONE shared never-dropped tokio runtime (#[test] + block_on) instead of per-test #[tokio::test] runtimes, because the manager's teardown worker is armed lazily ONCE on the first arming runtime and dies with it — later notify_terminal_exit sends are silently swallowed and the drain would time out (ground-truthed: launch_lifecycle.rs:467/:513/:602-612/ :632-643, pty.rs:440-453, terminal.rs:1333-1337). - Converted the five sketch test fns to the shared-runtime shape and added the two drain calls after their final kill_all() asserts.
…the sidecar budget Supersedes the fail-fast half of D-C-REVISIT for LaunchClass::Restore (spec 2026-07-30-graceful-restore-resume §4 P2, §9.2): restore-class waiters queue with no wall-clock death, bounded by queue cap (default 64, FRESHELL_CODEX_PLAN_QUEUE_CAP) and cancellation. Concurrency bound 2 and the Interactive 30s fail-fast are unchanged.
set_global_codex_launch_manager_for_tests (set-once, mirrors the proxy event-sink seam) unblocks WS-level fake-runtime injection for the S1 restore-storm pins; discard_sync is the Drop-safe teardown the prepared- launch RAII guard needs (Handle::try_current-guarded: never panics outside runtime context).
…-preserving) Seam for graceful restore/resume S1: the restore path will run this derivation BEFORE the spawn-gate permit. Only the pure rungs move; the claude P0.4 ladder stays in handle_create AFTER the adopt/D8 arms (A12: it reads mutable liveness state that presumes those arms ran first). No behavior change; pins claude_restore_unavailable / codex_session_ref_resume stay green.
…ermit Graceful restore/resume S1, P1: prepare_launch (resume-identity derivation + codex managed plan, LaunchClass::Restore) runs before spawn_gate.acquire, so permits cover only fast mode-uniform PTY-spawn->settle work — codex planning can no longer starve shell/claude/opencode restores. The restore gate wait is now acquire_unbounded (cancel-aware, no 10s death); QueueFull still fails loud as RATE_LIMITED. Prepared-but-unadopted sidecars are discarded on EVERY early exit via the PreparedCodexLaunch RAII guard. Scoped per the load-bearing review: only resume-planned codex restores are prepared pre-gate (A4: fresh plans arm a 45s candidate-capture timer at proxy start — no-session codex restores keep today's on-permit Interactive planning); the claude P0.4 ladder stays post-adopt/attach in handle_create (A12); the four gate-timeout restore pins are rewritten timeout-free (A13); warn-level slow-wait/slow-hold tracing replaces the deleted 10s ops signal (A5 accepted residual). D-GATE-SOFT generalized: the gate may not kill a live pane; now contention may not kill a restore. Permit scope spawn->settle unchanged (da5d9b5c class pinned by permit_released_only_after_work_completes).
…, prepared-sidecar discard Spec §8 integration pins for graceful restore/resume S1: 8 codex + 4 shell burst settles all 12 with zero user-facing errors, shells settle before the codex backlog drains (planning off-permit), plan concurrency <= 2; deterministic plan failure stays loud for that create only; disconnect/ shutdown/queue-full paths tear prepared sidecars down; plan-queue overflow maps to RATE_LIMITED.
…l restore/resume S1)
pull Bot
pushed a commit
to HinchK/freshell
that referenced
this pull request
Jul 31, 2026
…@2641ada38 (danshapiro#589 reconciliation)
pull Bot
pushed a commit
to HinchK/freshell
that referenced
this pull request
Jul 31, 2026
…— case-7b ORDER red test, corrected baselines (28 commits), storm-pin load-attribution and node_modules guards
pull Bot
pushed a commit
to HinchK/freshell
that referenced
this pull request
Jul 31, 2026
…lwaysAbsentProbe 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)
pull Bot
pushed a commit
to HinchK/freshell
that referenced
this pull request
Jul 31, 2026
…base plan (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
pull Bot
pushed a commit
to HinchK/freshell
that referenced
this pull request
Jul 31, 2026
…reconcile resume-validation gate-before-plan with danshapiro#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.
Slice 1 of docs/plans/2026-07-30-graceful-restore-resume.md — server-only, protocol-invisible.
Built via the-usual (6/6 tasks, per-task review, whole-branch review approved; independent delta review PASSED round 1 with zero blocking issues). Full gates green at tip: fmt/clippy/check, workspace tests (known pre-existing pane_ledger flake excepted), storm suite deterministic.
Generated with Amplifier
Co-Authored-By: Amplifier 240397093+microsoft-amplifier@users.noreply.github.com