Skip to content

fix(server): enforce D7 live-session guard + resume lease on REST resume paths (ks38)#540

Merged
danshapiro merged 9 commits into
mainfrom
fix/rest-resume-live-guard
Jul 27, 2026
Merged

fix(server): enforce D7 live-session guard + resume lease on REST resume paths (ks38)#540
danshapiro merged 9 commits into
mainfrom
fix/rest-resume-live-guard

Conversation

@danshapiro

Copy link
Copy Markdown
Owner

Fixes kata issue ks38 — the REST POST /api/tabs resume path lacked the D7 live-session guard that the WebSocket create path enforces (crates/freshell-ws/src/terminal.rs:1357-1410). Through REST, a resume onto a session already running live could spawn a second CLI writer onto the same session (two-JSONL-writers corruption).

The fix:

  • Extracts the guard into a shared helper used by both the WS and REST paths
  • Enforces it on all three REST entry points (POST /api/tabs, POST /api/panes/:id/split, POST /api/panes/:id/respawn) returning a 409 RESTORE_UNAVAILABLE-style error
  • Adds the same short-lived per-session resume lease the WS path takes so two racing resume requests cannot both win
  • New tests prove REST resume onto a live (mode, sessionId) is refused while resume onto a non-live session still works

Results: 459 tests pass, fmt/clippy clean. Independent fresh-eyes delta review verdict: APPROVED, ready to merge.

Discovered during real-world recovery from an unexpected host reboot on 2026-07-26.

References: Kata ks38

danshapiro and others added 9 commits July 27, 2026 00:35
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

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

Falsified assumptions fixed:
- The in-process tabs-sync restore caller no longer exists (deleted by
  2ed6b94) - plan now names the three real REST callers only.
- Plain check-then-spawn is NOT parity with WS: the shipped client
  always negotiates paneReconcileV1, so the WS create path holds the
  D8 per-sessionRef lease in production. New Task 5 claims the same
  registry lease at the REST choke point (conservative 409 arms, RAII
  release, claim completed into a binding on success).

Also: definitive Debug-derive instruction for TerminalIdentityRegistry,
corrected pane_ops helper range (1007-1021), removed the ks38 respawn
misquote, renumbered verification/land tasks to 6/7.

Ledger: .the-usual-logs/rest-resume-live-guard/load-bearing-ledger.md
(16 verified, 2 falsified-and-fixed, 1 acceptable).

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

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

Fresh-eyes review found the rest_create_resume_completes_claim_into_binding
test vacuous: a late claim_session_ref probe is answered BoundElsewhere by
the row-join arm (registry.rs:1771-1773) from the Running row's
resume_session_id stamp alone, so the prescribed red state (Acquired) was
unachievable and the green test never proved complete_session_ref_claim ran.
The test now asserts bound_terminal_for_session_ref (registry.rs:2007, pub
test probe; only complete_session_ref_claim writes that map) is None before
the spawn and Some(tid) after, with the expected-red text and Task 5's
consumed-interfaces list updated to match.

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

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

Extracts the D7 liveness join (identity arm behind a new
SessionIdentityLookup trait + registry-row arm) into
TerminalRegistry::live_session_owner so the WS create guard and the
REST spawn pipeline can share one predicate instead of copies.
Part of ks38.

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

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

cargo fmt --all --check and clippy's doc_lazy_continuation (-D warnings)
both failed on 8c800c9's additions: two over-wide assert_eq! lines in
the registry tests and a doc-comment paragraph that clippy read as a
lazy list continuation. Mechanical fixes only; no behavior change.
Part of ks38.

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

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

TerminalIdentityRegistry now implements the freshell-terminal
SessionIdentityLookup seam, and the terminal.create D7 block delegates
its two-arm liveness join to TerminalRegistry::live_session_owner.
Behavior-preserving: live_session_ref_guard and the restore-unavailable
suites pin the wire contract. Part of ks38.

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

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

POST /api/tabs (and every spawn_terminal_pane caller) now refuses a
sessionRef resume onto a (mode, sessionId) owned by a Running terminal
with 409 {code: RESTORE_UNAVAILABLE}, via the shared
TerminalRegistry::live_session_owner predicate -- both arms: registry
rows plus the identity registry injected from freshell-server through
the new SessionIdentityLookup seam. Same message text as the WS guard;
refusal happens before any side effect, so no rollback is needed.

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

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Respawn/split with a sessionRef onto a live (mode, sessionId) are
refused 409 RESTORE_UNAVAILABLE through the shared spawn choke point;
respawn-resume succeeds once the owner exits; and respawn has NO
self-exemption -- the pane's own detached-but-running predecessor
counts as the live owner (two-writers doctrine). Part of ks38.

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

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…n-ref lease

The REST choke point now claims the registry's per-sessionRef lease
(claim_session_ref) after the D7 guard and completes it into a binding
on success, closing the REST-x-REST / REST-x-WS duplicate-writer race
the check-then-spawn guard alone leaves open. Conservative arms:
Held/BoundElsewhere/ExpiredNeedsKill all answer 409 RESTORE_UNAVAILABLE;
RAII release on every failure path. Part of ks38.

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

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
@danshapiro
danshapiro merged commit 70c43c6 into main Jul 27, 2026
2 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