Skip to content

Commit 70c43c6

Browse files
fix(server): enforce D7 live-session guard + resume lease on REST resume paths (ks38) (#540)
* docs: add implementation plan for rest-resume-live-guard 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> * docs(plan): apply load-bearing validation findings to the rest-resume-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> * docs(plan): make Task 5's claim-completion test observe the bindings 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> * feat(terminal): add shared D7 live-session-owner predicate to TerminalRegistry 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> * style(terminal): fix fmt and clippy doc lint in Task 1's live_session_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> * refactor(ws): route the D7 create guard through the shared live_session_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> * fix(freshagent): enforce the D7 live-session guard on the REST spawn 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> * test(freshagent): pin D7 REST guard coverage on respawn and split routes 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> * fix(freshagent): serialize REST sessionRef resumes with the D8 session-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> --------- Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
1 parent cab6c95 commit 70c43c6

7 files changed

Lines changed: 2057 additions & 27 deletions

File tree

crates/freshell-freshagent/src/lib.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,13 @@ pub struct FreshAgentState {
116116
/// unchanged (terminal-mode routes 503 instead of touching a registry
117117
/// that was never given to them).
118118
pub(crate) terminal_registry: Option<freshell_terminal::TerminalRegistry>,
119+
/// Read-only session-identity lookup (in production: the WS-side
120+
/// TerminalIdentityRegistry behind the freshell-terminal
121+
/// SessionIdentityLookup seam, wired by freshell-server). Powers the
122+
/// identity arm of the REST D7 live-session guard. `None` (unwired)
123+
/// narrows the guard to the registry-row arm.
124+
pub(crate) session_identity:
125+
Option<Arc<dyn freshell_terminal::registry::SessionIdentityLookup>>,
119126
/// paneId -> terminal pane record (Slice 1 `mode:'shell'` terminals
120127
/// created via `POST /api/tabs`). Disjoint from `panes` (fresh-agent-only)
121128
/// and `content_panes` (browser/editor) -- a pane id appears in exactly
@@ -238,6 +245,7 @@ impl FreshAgentState {
238245
opencode: Arc::new(tokio::sync::Mutex::new(None)),
239246
sessions_revision: Arc::new(AtomicI64::new(0)),
240247
terminal_registry: None,
248+
session_identity: None,
241249
terminal_panes: Arc::new(Mutex::new(HashMap::new())),
242250
content_panes: Arc::new(Mutex::new(HashMap::new())),
243251
tabs: Arc::new(Mutex::new(HashMap::new())),
@@ -388,6 +396,20 @@ impl FreshAgentState {
388396
self
389397
}
390398

399+
/// D7 live-session guard (REST rung): wire in the read-only
400+
/// session-identity lookup (in production the WS-side
401+
/// `TerminalIdentityRegistry`, injected by `freshell-server`'s `main.rs`)
402+
/// so `spawn_terminal_pane` can probe the identity arm of
403+
/// [`freshell_terminal::TerminalRegistry::live_session_owner`]. Unwired
404+
/// (`None`), the guard degrades to the registry-row arm only.
405+
pub fn with_session_identity(
406+
mut self,
407+
identity: Arc<dyn freshell_terminal::registry::SessionIdentityLookup>,
408+
) -> Self {
409+
self.session_identity = Some(identity);
410+
self
411+
}
412+
391413
/// SESSION-09 fix-forward: replace this state's own `sessions_revision`
392414
/// counter with a SHARED one -- in production, `freshell-server` wires
393415
/// this to the SAME `Arc<AtomicI64>` as `freshell_ws::WsState::sessions_revision`
@@ -1257,6 +1279,17 @@ fn fail_json(status: StatusCode, message: String) -> Response {
12571279
.into_response()
12581280
}
12591281

1282+
/// `fail_json` + a machine-readable code, matching how the WS side keys
1283+
/// errors (`error["code"] == "RESTORE_UNAVAILABLE"`). Envelope is additive:
1284+
/// `{status:"error", code, message}`.
1285+
pub(crate) fn fail_json_code(status: StatusCode, code: &str, message: String) -> Response {
1286+
(
1287+
status,
1288+
Json(json!({ "status": "error", "code": code, "message": message })),
1289+
)
1290+
.into_response()
1291+
}
1292+
12601293
/// The error status the original maps serve failures to (`agentRouteErrorStatus`): a
12611294
/// bounded cold-start failure / transport error is a 5xx; everything else 500 here.
12621295
fn serve_error_status(err: &ServeError) -> StatusCode {

0 commit comments

Comments
 (0)