From eef9b3447cc5ebea86fe58507897db8931362c09 Mon Sep 17 00:00:00 2001 From: Codex Date: Thu, 23 Jul 2026 23:43:33 -0700 Subject: [PATCH 1/8] feat(protocol): pane.reconcile frames, paneReconcileV1 capability, reconcile error codes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Additive wire surface for the reconciliation handshake (design doc 2026-07-22-reconciliation-handshake-design.md §4): - ClientMessage::PaneReconcileRequest (pane.reconcile.request), parse-tolerant per-pane claims so malformed entries get 'invalid' verdicts, never a frame parse failure - ServerMessage::PaneReconcileResult (pane.reconcile.result) with per-pane verdict enum attach|respawn|fresh|dead_session|retry|invalid - HelloCapabilities.paneReconcileV1 + Ready.capabilities advertisement (Option + skip_serializing_if: clean-boot handshake bytes unchanged) - ErrorCode::{ReconcileTooLarge,ReconcileUnavailable} - ws-message-inventory.json 27→28 client / 52→53 server types protocolVersion stays 7. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- .../freshell-protocol/src/client_messages.rs | 61 ++++- crates/freshell-protocol/src/common.rs | 8 + .../freshell-protocol/src/server_messages.rs | 80 +++++- crates/freshell-protocol/tests/inventory.rs | 18 +- .../freshell-protocol/tests/pane_reconcile.rs | 240 ++++++++++++++++++ port/contract/ws-message-inventory.json | 8 +- 6 files changed, 401 insertions(+), 14 deletions(-) create mode 100644 crates/freshell-protocol/tests/pane_reconcile.rs diff --git a/crates/freshell-protocol/src/client_messages.rs b/crates/freshell-protocol/src/client_messages.rs index 9f1a21f99..2d245fa27 100644 --- a/crates/freshell-protocol/src/client_messages.rs +++ b/crates/freshell-protocol/src/client_messages.rs @@ -70,11 +70,13 @@ pub enum ClientMessage { FreshAgentKill(FreshAgentKill), #[serde(rename = "freshAgent.fork")] FreshAgentFork(FreshAgentFork), + #[serde(rename = "pane.reconcile.request")] + PaneReconcileRequest(PaneReconcileRequest), } /// The exact `type` discriminants of every client→server message, in the frozen /// inventory's order. This is the T0 conformance checklist. -pub const CLIENT_MESSAGE_TYPES: [&str; 27] = [ +pub const CLIENT_MESSAGE_TYPES: [&str; 28] = [ "claude.activity.list", "client.diagnostic", "codex.activity.list", @@ -92,6 +94,7 @@ pub const CLIENT_MESSAGE_TYPES: [&str; 27] = [ "freshAgent.send", "hello", "opencode.activity.list", + "pane.reconcile.request", "ping", "terminal.attach", "terminal.codex.candidate.persisted", @@ -113,6 +116,11 @@ pub struct HelloCapabilities { pub terminal_output_batch_v1: Option, #[serde(skip_serializing_if = "Option::is_none")] pub ui_screenshot_v1: Option, + /// Reconciliation handshake opt-in (design §4.1). A client that sets this + /// MAY send `pane.reconcile.request` once the `ready` it receives + /// advertises the capability back (§4.2). Absent for the frozen client. + #[serde(skip_serializing_if = "Option::is_none")] + pub pane_reconcile_v1: Option, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -327,6 +335,57 @@ pub struct UiScreenshotResult { pub width: Option, } +// --- pane.reconcile.request --------------------------------------------------- + +/// One pane's identity claims, as presented by a reconciling client +/// (reconciliation-handshake design §4.3). Every field is a HINT to be +/// validated, never trusted. All fields are parse-tolerant: a malformed entry +/// must still deserialize so the server can answer it with an `invalid` +/// verdict (total cardinality, §8) instead of failing the whole frame. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ReconcilePane { + /// OPAQUE to the server; echoed verbatim on the verdict. `""` when the + /// client omitted it (the entry is then `invalid`). + #[serde(default)] + pub pane_key: String, + /// v1: `"terminal"` only (fresh-agent extension deferred, §12). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub kind: Option, + /// `TerminalMode` string as persisted (`"shell"`, `"claude"`, …). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub mode: Option, + /// The pane's stable creation key — required by contract (§5.5); an entry + /// without one is `invalid`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub create_request_id: Option, + /// Last known live handle. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub terminal_id: Option, + /// Locality hint, informational only. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub server_instance_id: Option, + /// Optional identity claim. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub session_ref: Option, + /// Optional legacy single-key claim. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub resume_session_id: Option, + /// Informational only — never trusted. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub status: Option, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PaneReconcileRequest { + /// Client-minted, echoed verbatim; correlation only. + pub reconcile_id: String, + /// Flat list — no tree, no tab structure. Cap: 200 entries (an over-cap + /// request is answered with `error{RECONCILE_TOO_LARGE}`). + pub panes: Vec, +} + // --- codingcli.* ------------------------------------------------------------ #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] diff --git a/crates/freshell-protocol/src/common.rs b/crates/freshell-protocol/src/common.rs index 06b5f9377..57b826efc 100644 --- a/crates/freshell-protocol/src/common.rs +++ b/crates/freshell-protocol/src/common.rs @@ -86,6 +86,14 @@ pub enum ErrorCode { RateLimited, Unauthorized, ProtocolMismatch, + /// Reconciliation handshake (`pane.reconcile.request`, design §4.3): the + /// request presented more than the 200-pane cap — answered with this error + /// instead of ever being silently truncated. + ReconcileTooLarge, + /// Reconciliation handshake (design §8): the verdict derivation itself + /// failed (poisoned lock, index handle error). The client keeps its state + /// and may re-send. + ReconcileUnavailable, } /// The three coding-agent providers (`claude | codex | opencode`). diff --git a/crates/freshell-protocol/src/server_messages.rs b/crates/freshell-protocol/src/server_messages.rs index e69217745..ae464588f 100644 --- a/crates/freshell-protocol/src/server_messages.rs +++ b/crates/freshell-protocol/src/server_messages.rs @@ -66,6 +66,8 @@ pub enum ServerMessage { OpencodeActivityListResponse(OpencodeActivityListResponse), #[serde(rename = "opencode.activity.updated")] OpencodeActivityUpdated(OpencodeActivityUpdated), + #[serde(rename = "pane.reconcile.result")] + PaneReconcileResult(PaneReconcileResult), #[serde(rename = "perf.logging")] PerfLogging(PerfLogging), #[serde(rename = "pong")] @@ -124,7 +126,7 @@ pub enum ServerMessage { /// The exact `type` discriminants of every server→client message, in the frozen /// inventory's order. This is the T0 conformance checklist. -pub const SERVER_MESSAGE_TYPES: [&str; 52] = [ +pub const SERVER_MESSAGE_TYPES: [&str; 53] = [ "claude.activity.list.response", "claude.activity.updated", "codex.activity.list.response", @@ -150,6 +152,7 @@ pub const SERVER_MESSAGE_TYPES: [&str; 52] = [ "freshAgent.session.materialized", "opencode.activity.list.response", "opencode.activity.updated", + "pane.reconcile.result", "perf.logging", "pong", "ready", @@ -592,6 +595,64 @@ pub struct FreshAgentSessionMaterialized { pub session_ref: Option, } +// --- pane.reconcile.result ---------------------------------------------------- + +/// Authoritative per-pane verdict (reconciliation-handshake design §4.4). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ReconcileVerdict { + Attach, + Respawn, + Fresh, + DeadSession, + Retry, + Invalid, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PaneVerdict { + /// Echoed verbatim, 1:1 with request order. + pub pane_key: String, + pub verdict: ReconcileVerdict, + /// `attach` only: the live terminal to attach to. + #[serde(skip_serializing_if = "Option::is_none")] + pub terminal_id: Option, + /// attach: authoritative identity; respawn: THE identity to resume with; + /// dead_session: the claimed-but-missing identity, for the error UI. + #[serde(skip_serializing_if = "Option::is_none")] + pub session_ref: Option, + /// Present iff the server overrode a differing client claim. + #[serde(skip_serializing_if = "Option::is_none")] + pub corrected: Option, + /// fresh / dead_session / retry / invalid: machine-readable code. + #[serde(skip_serializing_if = "Option::is_none")] + pub reason: Option, + /// retry only. + #[serde(skip_serializing_if = "Option::is_none")] + pub retry_after_ms: Option, + /// Row 2b (invariant I6): a newer duplicate generation exists for the same + /// `createRequestId`; the client stays on its live attachment and this + /// merely flags the duplicate `terminalId`. + #[serde(skip_serializing_if = "Option::is_none")] + pub duplicate: Option, +} + +/// Sent ONLY in response to `pane.reconcile.request` — the server never +/// volunteers it (frozen-client inertness gate 3, design §3). +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PaneReconcileResult { + /// Echoed from the request. + pub reconcile_id: String, + /// This server process's boot. + pub boot_id: String, + pub server_instance_id: String, + /// Cardinality invariant: `verdicts.len() == panes.len()`, matched 1:1 by + /// `paneKey` — a malformed entry gets `invalid`, never omission. + pub verdicts: Vec, +} + // --- lifecycle / misc ------------------------------------------------------- #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -604,6 +665,17 @@ pub struct Pong { pub timestamp: String, } +/// Server capability advertisement on `ready` (reconciliation-handshake design +/// §4.2). Populated **iff** the connection's `hello` carried the matching +/// client capability — today's frozen client never opts in, so the emitted +/// clean-boot handshake stays byte-for-byte unchanged. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ReadyCapabilities { + #[serde(skip_serializing_if = "Option::is_none")] + pub pane_reconcile_v1: Option, +} + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct Ready { @@ -612,6 +684,12 @@ pub struct Ready { pub boot_id: Option, #[serde(skip_serializing_if = "Option::is_none")] pub server_instance_id: Option, + /// Reconciliation-handshake advertisement (§4.2): `Some` only when the + /// client's `hello` opted in via `capabilities.paneReconcileV1`. A client + /// must not send `pane.reconcile.request` unless the `ready` it just + /// received advertised the capability. + #[serde(skip_serializing_if = "Option::is_none")] + pub capabilities: Option, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] diff --git a/crates/freshell-protocol/tests/inventory.rs b/crates/freshell-protocol/tests/inventory.rs index deb3c2634..60d26339b 100644 --- a/crates/freshell-protocol/tests/inventory.rs +++ b/crates/freshell-protocol/tests/inventory.rs @@ -31,12 +31,12 @@ fn client_types_match_inventory_exactly() { let inv = inventory(); assert_eq!( inv["clientToServer"]["count"].as_u64(), - Some(27), - "inventory declares 27 client→server types" + Some(28), + "inventory declares 28 client→server types" ); let expected = json_type_set(&inv["clientToServer"]["types"]); let actual: BTreeSet = CLIENT_MESSAGE_TYPES.iter().map(|s| s.to_string()).collect(); - assert_eq!(actual.len(), 27, "crate declares 27 client types (no dups)"); + assert_eq!(actual.len(), 28, "crate declares 28 client types (no dups)"); assert_eq!( actual, expected, "CLIENT_MESSAGE_TYPES must equal the frozen inventory (no missing/extra)" @@ -48,12 +48,12 @@ fn server_types_match_inventory_exactly() { let inv = inventory(); assert_eq!( inv["serverToClient"]["count"].as_u64(), - Some(52), - "inventory declares 52 server→client types" + Some(53), + "inventory declares 53 server→client types" ); let expected = json_type_set(&inv["serverToClient"]["types"]); let actual: BTreeSet = SERVER_MESSAGE_TYPES.iter().map(|s| s.to_string()).collect(); - assert_eq!(actual.len(), 52, "crate declares 52 server types (no dups)"); + assert_eq!(actual.len(), 53, "crate declares 53 server types (no dups)"); assert_eq!( actual, expected, "SERVER_MESSAGE_TYPES must equal the frozen inventory (no missing/extra)" @@ -61,14 +61,14 @@ fn server_types_match_inventory_exactly() { } #[test] -fn combined_surface_is_79() { +fn combined_surface_is_81() { let all = all_message_types(); - assert_eq!(all.len(), 79, "27 client + 52 server = 79 discriminants"); + assert_eq!(all.len(), 81, "28 client + 53 server = 81 discriminants"); // sorted + unique let unique: BTreeSet<&str> = all.iter().copied().collect(); assert_eq!( unique.len(), - 79, + 81, "no discriminant collides across directions" ); } diff --git a/crates/freshell-protocol/tests/pane_reconcile.rs b/crates/freshell-protocol/tests/pane_reconcile.rs new file mode 100644 index 000000000..d2d9701a9 --- /dev/null +++ b/crates/freshell-protocol/tests/pane_reconcile.rs @@ -0,0 +1,240 @@ +//! Reconciliation-handshake wire frames (Phase 1 of +//! `docs/plans/2026-07-22-reconciliation-handshake-design.md` §4). +//! +//! Additive protocol surface only: `pane.reconcile.request` (client→server), +//! `pane.reconcile.result` (server→client), the `paneReconcileV1` capability on +//! `hello` + its advertisement on `ready`, and the two reconcile error codes. +//! `protocolVersion` stays 7 (§4.5 / fence "No protocolVersion bump"). + +use freshell_protocol::{ + ClientMessage, ErrorCode, PaneReconcileResult, PaneVerdict, ReadyCapabilities, + ReconcileVerdict, ServerMessage, SessionLocator, +}; +use serde_json::json; + +// --- capability (hello) ------------------------------------------------------ + +#[test] +fn hello_capabilities_parse_pane_reconcile_v1() { + let wire = json!({ + "type": "hello", + "protocolVersion": 7, + "token": "t", + "capabilities": { "paneReconcileV1": true } + }); + let msg: ClientMessage = serde_json::from_value(wire).expect("hello parses"); + let ClientMessage::Hello(hello) = msg else { + panic!("expected hello"); + }; + assert_eq!( + hello.capabilities.and_then(|c| c.pane_reconcile_v1), + Some(true) + ); +} + +#[test] +fn hello_capabilities_omit_pane_reconcile_v1_when_absent() { + // Frozen-client shape: no paneReconcileV1 anywhere. Round-trip must not + // invent the field (skip_serializing_if). + let wire = json!({ + "type": "hello", + "protocolVersion": 7, + "token": "t", + "capabilities": { "terminalOutputBatchV1": true } + }); + let msg: ClientMessage = serde_json::from_value(wire.clone()).expect("hello parses"); + let back = serde_json::to_value(&msg).expect("serializes"); + assert_eq!(back, wire); +} + +// --- advertisement (ready) --------------------------------------------------- + +#[test] +fn ready_capabilities_field_is_omitted_when_none() { + // The pinned clean-boot handshake bytes must be unchanged for a frozen + // client (§3): `capabilities: None` must not appear on the wire at all. + let ready = freshell_protocol::Ready { + timestamp: "2026-07-22T00:00:00.000Z".to_string(), + boot_id: Some("boot-1".to_string()), + server_instance_id: Some("srv-1".to_string()), + capabilities: None, + }; + let wire = serde_json::to_value(ServerMessage::Ready(ready)).expect("serializes"); + assert!( + wire.get("capabilities").is_none(), + "ready must omit capabilities when not negotiated: {wire}" + ); +} + +#[test] +fn ready_capabilities_advertise_pane_reconcile_v1_when_negotiated() { + let ready = freshell_protocol::Ready { + timestamp: "2026-07-22T00:00:00.000Z".to_string(), + boot_id: Some("boot-1".to_string()), + server_instance_id: Some("srv-1".to_string()), + capabilities: Some(ReadyCapabilities { + pane_reconcile_v1: Some(true), + }), + }; + let wire = serde_json::to_value(ServerMessage::Ready(ready)).expect("serializes"); + assert_eq!(wire["capabilities"], json!({ "paneReconcileV1": true })); +} + +// --- pane.reconcile.request -------------------------------------------------- + +#[test] +fn reconcile_request_parses_full_pane_claims() { + let wire = json!({ + "type": "pane.reconcile.request", + "reconcileId": "rec-8f2k", + "panes": [ + { + "paneKey": "tab3:paneA", + "kind": "terminal", + "mode": "amplifier", + "createRequestId": "cr-1", + "terminalId": "term-1", + "serverInstanceId": "srv-old", + "sessionRef": { "provider": "amplifier", "sessionId": "s-1" }, + "resumeSessionId": "s-1", + "status": "running" + } + ] + }); + let msg: ClientMessage = serde_json::from_value(wire).expect("request parses"); + let ClientMessage::PaneReconcileRequest(req) = msg else { + panic!("expected pane.reconcile.request"); + }; + assert_eq!(req.reconcile_id, "rec-8f2k"); + assert_eq!(req.panes.len(), 1); + let pane = &req.panes[0]; + assert_eq!(pane.pane_key, "tab3:paneA"); + assert_eq!(pane.kind.as_deref(), Some("terminal")); + assert_eq!(pane.mode.as_deref(), Some("amplifier")); + assert_eq!(pane.create_request_id.as_deref(), Some("cr-1")); + assert_eq!(pane.terminal_id.as_deref(), Some("term-1")); + assert_eq!(pane.server_instance_id.as_deref(), Some("srv-old")); + assert_eq!( + pane.session_ref, + Some(SessionLocator { + provider: "amplifier".to_string(), + session_id: "s-1".to_string(), + }) + ); + assert_eq!(pane.resume_session_id.as_deref(), Some("s-1")); + assert_eq!(pane.status.as_deref(), Some("running")); +} + +#[test] +fn reconcile_request_tolerates_malformed_pane_entries() { + // A malformed entry (no createRequestId, unknown kind, even a missing + // paneKey) must still PARSE — the server answers it with an `invalid` + // verdict (§8 total cardinality), never a frame-level parse failure. + let wire = json!({ + "type": "pane.reconcile.request", + "reconcileId": "rec-1", + "panes": [ {}, { "paneKey": "p2", "kind": "fresh-agent" } ] + }); + let msg: ClientMessage = serde_json::from_value(wire).expect("malformed panes still parse"); + let ClientMessage::PaneReconcileRequest(req) = msg else { + panic!("expected pane.reconcile.request"); + }; + assert_eq!(req.panes.len(), 2); + assert_eq!(req.panes[0].pane_key, ""); + assert!(req.panes[0].create_request_id.is_none()); + assert_eq!(req.panes[1].kind.as_deref(), Some("fresh-agent")); +} + +// --- pane.reconcile.result --------------------------------------------------- + +#[test] +fn reconcile_result_serializes_verdicts_with_optional_fields_omitted() { + let result = ServerMessage::PaneReconcileResult(PaneReconcileResult { + reconcile_id: "rec-8f2k".to_string(), + boot_id: "boot-1".to_string(), + server_instance_id: "srv-1".to_string(), + verdicts: vec![ + PaneVerdict { + pane_key: "tab3:paneA".to_string(), + verdict: ReconcileVerdict::Attach, + terminal_id: Some("term-1".to_string()), + session_ref: Some(SessionLocator { + provider: "amplifier".to_string(), + session_id: "s-1".to_string(), + }), + corrected: Some(true), + reason: None, + retry_after_ms: None, + duplicate: None, + }, + PaneVerdict { + pane_key: "tab3:paneB".to_string(), + verdict: ReconcileVerdict::Fresh, + terminal_id: None, + session_ref: None, + corrected: None, + reason: Some("no_recoverable_identity".to_string()), + retry_after_ms: None, + duplicate: None, + }, + ], + }); + let wire = serde_json::to_value(&result).expect("serializes"); + assert_eq!(wire["type"], "pane.reconcile.result"); + assert_eq!(wire["reconcileId"], "rec-8f2k"); + assert_eq!(wire["bootId"], "boot-1"); + assert_eq!(wire["serverInstanceId"], "srv-1"); + assert_eq!(wire["verdicts"][0]["verdict"], "attach"); + assert_eq!(wire["verdicts"][0]["terminalId"], "term-1"); + assert_eq!(wire["verdicts"][0]["corrected"], true); + // Optional fields absent from the JSON entirely, not null. + assert!(wire["verdicts"][0].get("reason").is_none()); + assert!(wire["verdicts"][1].get("terminalId").is_none()); + assert_eq!(wire["verdicts"][1]["verdict"], "fresh"); + assert_eq!(wire["verdicts"][1]["reason"], "no_recoverable_identity"); +} + +#[test] +fn reconcile_verdict_wire_names_are_snake_case() { + for (verdict, name) in [ + (ReconcileVerdict::Attach, "attach"), + (ReconcileVerdict::Respawn, "respawn"), + (ReconcileVerdict::Fresh, "fresh"), + (ReconcileVerdict::DeadSession, "dead_session"), + (ReconcileVerdict::Retry, "retry"), + (ReconcileVerdict::Invalid, "invalid"), + ] { + assert_eq!(serde_json::to_value(verdict).unwrap(), json!(name)); + } +} + +#[test] +fn retry_verdict_carries_retry_after_ms() { + let verdict = PaneVerdict { + pane_key: "p".to_string(), + verdict: ReconcileVerdict::Retry, + terminal_id: None, + session_ref: None, + corrected: None, + reason: Some("index_warming".to_string()), + retry_after_ms: Some(2000), + duplicate: None, + }; + let wire = serde_json::to_value(&verdict).expect("serializes"); + assert_eq!(wire["retryAfterMs"], 2000); + assert_eq!(wire["reason"], "index_warming"); +} + +// --- error codes -------------------------------------------------------------- + +#[test] +fn reconcile_error_codes_are_screaming_snake_case() { + assert_eq!( + serde_json::to_value(ErrorCode::ReconcileTooLarge).unwrap(), + json!("RECONCILE_TOO_LARGE") + ); + assert_eq!( + serde_json::to_value(ErrorCode::ReconcileUnavailable).unwrap(), + json!("RECONCILE_UNAVAILABLE") + ); +} diff --git a/port/contract/ws-message-inventory.json b/port/contract/ws-message-inventory.json index 1e73de537..715973979 100644 --- a/port/contract/ws-message-inventory.json +++ b/port/contract/ws-message-inventory.json @@ -1,6 +1,6 @@ { "clientToServer": { - "count": 27, + "count": 28, "types": [ "claude.activity.list", "client.diagnostic", @@ -19,6 +19,7 @@ "freshAgent.send", "hello", "opencode.activity.list", + "pane.reconcile.request", "ping", "terminal.attach", "terminal.codex.candidate.persisted", @@ -34,7 +35,7 @@ "description": "Auto-generated from shared/ws-protocol.ts. DO NOT EDIT BY HAND. Regenerate with `npm run contract:generate`. `type` discriminants for every message in each direction, resolved from the ClientMessage and ServerMessage union types via the TypeScript type checker.", "generator": "port/contract/generate-ws-contract.ts", "serverToClient": { - "count": 52, + "count": 53, "types": [ "claude.activity.list.response", "claude.activity.updated", @@ -61,6 +62,7 @@ "freshAgent.session.materialized", "opencode.activity.list.response", "opencode.activity.updated", + "pane.reconcile.result", "perf.logging", "pong", "ready", @@ -91,6 +93,6 @@ ] }, "source": "shared/ws-protocol.ts", - "title": "freshell WebSocket message inventory — T0 conformance surface", + "title": "freshell WebSocket message inventory \u2014 T0 conformance surface", "wsProtocolVersion": 7 } From 6077a2b3caa2f4f1210824f8e5908385e05bf084 Mon Sep 17 00:00:00 2001 From: Codex Date: Thu, 23 Jul 2026 23:50:26 -0700 Subject: [PATCH 2/8] feat(terminal): stamp createRequestId atomically with registry insert + newest-generation accessors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reconciliation handshake design §5.1: the pane's stable creation key is now a field of the inserted TerminalShared row, set under the same registry lock that inserts it — no observer can ever see a row without its key (§9.1 test 5, proven by a concurrent insert/reader race test). - TerminalRegistry::create gains create_request_id (WS path stamps terminal.create.requestId; REST ingress passes None per §5.5 precondition 2) - newest_live_by_create_request_id / newest_by_create_request_id: newest generation first, live-only vs exited-inclusive (§5.2 derivation inputs) - is_live / probe: run-status + identity-probe row per terminal (crate-boundary identity path for REST-created resumes, design assumption 1) - §5.4 backstop detector: ≥2 live PTYs on one key emits ws.reconcile.duplicate_pty warn — the duplicate-writer shape made loud - register_headless: public headless seam so cross-crate (freshell-ws) wire tests can seed live/exited generations deterministically 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- .../freshell-freshagent/src/terminal_tabs.rs | 3 + crates/freshell-terminal/src/registry.rs | 423 +++++++++++++++++- .../freshell-ws/src/amplifier_association.rs | 1 + .../freshell-ws/src/opencode_association.rs | 1 + crates/freshell-ws/src/terminal.rs | 4 + 5 files changed, 408 insertions(+), 24 deletions(-) diff --git a/crates/freshell-freshagent/src/terminal_tabs.rs b/crates/freshell-freshagent/src/terminal_tabs.rs index 01a61b95e..46aabdae2 100644 --- a/crates/freshell-freshagent/src/terminal_tabs.rs +++ b/crates/freshell-freshagent/src/terminal_tabs.rs @@ -871,6 +871,9 @@ pub(crate) async fn spawn_terminal_pane( stream_id, &mode, resume_session_id.as_deref(), + // REST ingress mints no createRequestId (reconciliation design §5.5 + // precondition 2 — booked for the Phase-3 adoption change). + None, None, on_exit, ) { diff --git a/crates/freshell-terminal/src/registry.rs b/crates/freshell-terminal/src/registry.rs index af06b4899..d3f3f5d23 100644 --- a/crates/freshell-terminal/src/registry.rs +++ b/crates/freshell-terminal/src/registry.rs @@ -223,6 +223,12 @@ struct TerminalShared { mode: String, /// The session id a CLI pane resumed from (feeds the directory `sessionRef`). resume_session_id: Option, + /// The pane's stable creation key (`terminal.create.requestId`), stamped + /// ATOMICALLY with the registry insert — it is a field of the inserted row, + /// so no observer can ever see a row without its key (reconciliation + /// handshake design §5.1). `None` for creates that carried no key (e.g. + /// REST ingress, which mints none — design §5.5 precondition 2). + create_request_id: Option, /// Attached connections, keyed by connection id (multi-client fan-out, `§7.3`). subscribers: HashMap, } @@ -309,6 +315,28 @@ struct TerminalHandle { pty: Option, } +/// Registration options for a terminal record with NO backing PTY. +/// +/// This is the registry's headless seam (reconciliation-handshake design §9.1: +/// "the registry supports headless terminals for exactly this"): crate tests — +/// including `freshell-ws`'s wire-level reconcile tests, which sit across the +/// crate boundary and cannot reach the private [`TerminalShared`] — seed +/// live/exited terminal generations deterministically without spawning real +/// children. Never called on a production path (production terminals are only +/// ever registered by [`TerminalRegistry::create`], which spawns the PTY). +#[derive(Debug, Clone, Default)] +pub struct HeadlessTerminal { + pub terminal_id: String, + pub stream_id: String, + /// `TerminalMode` string; empty is normalized to `"shell"`. + pub mode: String, + pub resume_session_id: Option, + /// The pane's stable creation key (see [`TerminalRegistry::create`]). + pub create_request_id: Option, + /// Explicit creation timestamp (epoch ms); `None` = now. + pub created_at: Option, +} + struct RegistryInner { terminals: HashMap, /// Run-monotonic inventory revision (`terminals.changed.revision`, `§7.5`). Only @@ -517,6 +545,7 @@ impl TerminalRegistry { stream_id: String, mode: &str, resume_session_id: Option<&str>, + create_request_id: Option<&str>, ring_max_bytes: Option, on_exit: Option, ) -> io::Result<()> { @@ -545,6 +574,7 @@ impl TerminalRegistry { description: None, mode: mode.to_string(), resume_session_id: resume_session_id.map(str::to_string), + create_request_id: create_request_id.map(str::to_string), subscribers: HashMap::new(), })); @@ -593,6 +623,11 @@ impl TerminalRegistry { pid = pid.unwrap_or(0), "terminal.created" ); + // §5.4 backstop: two live PTYs on one createRequestId is the + // duplicate-writer data-loss shape — make it loud. + if let Some(key) = create_request_id { + self.warn_on_duplicate_live_ptys(key); + } Ok(()) } @@ -1090,6 +1125,186 @@ impl TerminalRegistry { .collect() } + /// Register a terminal record with NO backing PTY — see [`HeadlessTerminal`] + /// for exactly who this seam exists for. The row (including its + /// `create_request_id` stamp) is inserted atomically under the registry + /// lock, same as [`Self::create`]. + pub fn register_headless(&self, opts: HeadlessTerminal) { + let created_at = opts.created_at.unwrap_or_else(now_ms); + let mode = if opts.mode.is_empty() { + "shell".to_string() + } else { + opts.mode + }; + let create_request_id = opts.create_request_id.clone(); + let shared = Arc::new(Mutex::new(TerminalShared { + terminal_id: opts.terminal_id.clone(), + stream_id: opts.stream_id, + replay: VecDeque::new(), + replay_chars: 0, + max_replay_chars: self.scrollback_max_bytes().max(0) as usize, + scanner: BarrierScanner::new(), + head_seq: 0, + status: TerminalRunStatus::Running, + exit_code: None, + created_at, + last_activity_at: created_at, + cols: 120, + rows: 30, + geometry_epoch: 1, + cwd: None, + title: "Shell".to_string(), + description: None, + mode, + resume_session_id: opts.resume_session_id, + create_request_id, + subscribers: HashMap::new(), + })); + { + let mut inner = self.inner.lock().expect("registry lock"); + inner.terminals.insert( + opts.terminal_id.clone(), + TerminalHandle { shared, pty: None }, + ); + inner.revision += 1; + } + if let Some(key) = opts + .create_request_id + .or_else(|| self.probe_create_request_id(&opts.terminal_id)) + { + self.warn_on_duplicate_live_ptys(&key); + } + } + + /// Terminals (of any status) matching a `createRequestId`, NEWEST + /// generation first (`created_at` desc, `terminal_id` desc tie-break). + fn terminals_by_create_request_id(&self, key: &str) -> Vec<(String, TerminalRunStatus)> { + let shareds: Vec>> = { + let inner = self.inner.lock().expect("registry lock"); + inner + .terminals + .values() + .map(|h| Arc::clone(&h.shared)) + .collect() + }; + let mut rows: Vec<(i64, String, TerminalRunStatus)> = shareds + .iter() + .filter_map(|shared| { + let s = shared.lock().expect("terminal lock"); + if s.create_request_id.as_deref() == Some(key) { + Some((s.created_at, s.terminal_id.clone(), s.status)) + } else { + None + } + }) + .collect(); + rows.sort_by(|a, b| b.0.cmp(&a.0).then_with(|| b.1.cmp(&a.1))); + rows.into_iter() + .map(|(_, id, status)| (id, status)) + .collect() + } + + /// The newest **live** terminal for a `createRequestId` — the idempotency + /// keystone (design §7) and the single-flight create-dedupe key (§5.4). + /// Exited generations are excluded; a key whose every generation has + /// exited returns `None`. + pub fn newest_live_by_create_request_id(&self, key: &str) -> Option { + self.terminals_by_create_request_id(key) + .into_iter() + .find(|(_, status)| *status == TerminalRunStatus::Running) + .map(|(id, _)| id) + } + + /// The newest terminal for a `createRequestId` INCLUDING exited + /// generations — used by the verdict derivation (§5.2 step 2) to recover a + /// retired terminal's identity before declaring `fresh`. + pub fn newest_by_create_request_id(&self, key: &str) -> Option { + self.terminals_by_create_request_id(key) + .into_iter() + .next() + .map(|(id, _)| id) + } + + /// Whether a terminal is registered AND currently running (an exited-but- + /// retained record is NOT live — contrast [`Self::is_running`], which only + /// checks registration). + pub fn is_live(&self, terminal_id: &str) -> bool { + let shared = { + let inner = self.inner.lock().expect("registry lock"); + inner + .terminals + .get(terminal_id) + .map(|h| Arc::clone(&h.shared)) + }; + match shared { + Some(shared) => { + shared.lock().expect("terminal lock").status == TerminalRunStatus::Running + } + None => false, + } + } + + /// One terminal's [`IdentityProbeRow`] (mode / status / registry-side + /// resume id) — the per-terminal getter the reconcile derivation uses to + /// resolve identity across the crate boundary for REST-created resumes + /// (design §2 assumption 1's acceptance check). + pub fn probe(&self, terminal_id: &str) -> Option { + let shared = { + let inner = self.inner.lock().expect("registry lock"); + inner + .terminals + .get(terminal_id) + .map(|h| Arc::clone(&h.shared)) + }; + shared.map(|shared| { + let s = shared.lock().expect("terminal lock"); + IdentityProbeRow { + terminal_id: s.terminal_id.clone(), + mode: s.mode.clone(), + status: s.status, + created_at: s.created_at, + resume_session_id: s.resume_session_id.clone(), + } + }) + } + + /// A terminal's stamped `createRequestId`, if any. + fn probe_create_request_id(&self, terminal_id: &str) -> Option { + let shared = { + let inner = self.inner.lock().expect("registry lock"); + inner + .terminals + .get(terminal_id) + .map(|h| Arc::clone(&h.shared)) + }; + shared.and_then(|shared| { + shared + .lock() + .expect("terminal lock") + .create_request_id + .clone() + }) + } + + /// §5.4 backstop detector (always-on, capability-independent): if a key + /// now has two or more LIVE terminals, make it loud — two live PTYs on one + /// `createRequestId` means two JSONL writers on one session file. + fn warn_on_duplicate_live_ptys(&self, key: &str) { + let live: Vec = self + .terminals_by_create_request_id(key) + .into_iter() + .filter(|(_, status)| *status == TerminalRunStatus::Running) + .map(|(id, _)| id) + .collect(); + if live.len() >= 2 { + tracing::warn!( + create_request_id = %key, + terminal_ids = ?live, + "ws.reconcile.duplicate_pty" + ); + } + } + /// The current `terminals.changed.revision` (run-monotonic, `§7.5`). pub fn revision(&self) -> i64 { self.inner.lock().expect("registry lock").revision @@ -1345,6 +1560,7 @@ mod tests { None, None, None, + None, ) .expect("spawn /bin/sh -c 'sleep 30'"); @@ -1400,6 +1616,7 @@ mod tests { Some("sess-codex-resume-1"), None, None, + None, ) .expect("spawn /bin/sh -c 'sleep 30'"); @@ -1551,34 +1768,14 @@ mod tests { /// `inventory()`'s tie-break) without racing real `now_ms()` resolution /// under load -- see `inventory_lists_running_terminals_sorted_and_reflects_revision`. fn insert_headless_at(&self, terminal_id: &str, stream_id: &str, created_at: i64) { - let shared = Arc::new(Mutex::new(TerminalShared { + self.register_headless(HeadlessTerminal { terminal_id: terminal_id.to_string(), stream_id: stream_id.to_string(), - replay: VecDeque::new(), - replay_chars: 0, - max_replay_chars: self.scrollback_max_bytes().max(0) as usize, - scanner: BarrierScanner::new(), - head_seq: 0, - status: TerminalRunStatus::Running, - exit_code: None, - created_at, - last_activity_at: created_at, - cols: 120, - rows: 30, - geometry_epoch: 1, - cwd: None, - title: "Shell".to_string(), - description: None, mode: "shell".to_string(), resume_session_id: None, - subscribers: HashMap::new(), - })); - let mut inner = self.inner.lock().unwrap(); - inner.terminals.insert( - terminal_id.to_string(), - TerminalHandle { shared, pty: None }, - ); - inner.revision += 1; + create_request_id: None, + created_at: Some(created_at), + }); } /// Test-only: force a terminal's `lastActivityAt` to an arbitrary value so @@ -1969,6 +2166,7 @@ mod tests { "shell", None, None, + None, Some(Box::new(move |code| { // Mirrors the production wiring (`freshell-ws`'s on_exit hook): // the reader thread calls `finish_pty_exit` on natural exit. @@ -2036,6 +2234,7 @@ mod tests { None, None, None, + None, ) .expect("spawn /bin/sh -c 'sleep 30'"); assert!(reg.is_running(&terminal_id)); @@ -2457,4 +2656,180 @@ mod tests { "retained {retained_chars} chars must not exceed the {cap}-char cap" ); } + + // ------------------------------------------------------------------ + // Reconciliation handshake (design §5.1): createRequestId stamped + // atomically with the registry insert + the two newest-generation + // accessors + the ≥2-live-PTYs-per-key backstop detector. + // ------------------------------------------------------------------ + + fn headless(reg: &TerminalRegistry, id: &str, key: Option<&str>, created_at: i64) { + reg.register_headless(HeadlessTerminal { + terminal_id: id.to_string(), + stream_id: format!("S-{id}"), + mode: "claude".to_string(), + resume_session_id: None, + create_request_id: key.map(str::to_string), + created_at: Some(created_at), + }); + } + + #[test] + fn create_stamps_create_request_id_visible_via_newest_live_accessor() { + let reg = TerminalRegistry::new(); + let spec = SpawnSpec { + program: "/bin/sh".into(), + args: vec!["-c".into(), "sleep 30".into()], + env_overrides: std::collections::BTreeMap::new(), + cwd: Some("/tmp".into()), + cols: 80, + rows: 24, + }; + let env = std::collections::BTreeMap::new(); + reg.create( + &spec, + &env, + "T-crid".to_string(), + "S-crid".to_string(), + "shell", + None, + Some("cr-stamp-1"), + None, + None, + ) + .expect("spawn /bin/sh"); + + assert_eq!( + reg.newest_live_by_create_request_id("cr-stamp-1"), + Some("T-crid".to_string()) + ); + reg.kill("T-crid"); + } + + #[test] + fn newest_live_by_key_prefers_newest_generation_and_excludes_exited() { + let reg = TerminalRegistry::new(); + headless(®, "gen1", Some("cr-k"), 1_000); + headless(®, "gen2", Some("cr-k"), 2_000); + // Exit the OLD generation: newest live is gen2. + reg.finish_pty_exit("gen1", 0); + assert_eq!( + reg.newest_live_by_create_request_id("cr-k"), + Some("gen2".to_string()) + ); + // Exit the newest too: no live generation left. + reg.finish_pty_exit("gen2", 0); + assert_eq!(reg.newest_live_by_create_request_id("cr-k"), None); + // ... but the exited-inclusive accessor still finds the NEWEST one. + assert_eq!( + reg.newest_by_create_request_id("cr-k"), + Some("gen2".to_string()) + ); + assert_eq!(reg.newest_by_create_request_id("cr-unknown"), None); + } + + #[test] + fn is_live_distinguishes_running_exited_and_unknown() { + let reg = TerminalRegistry::new(); + headless(®, "T-live", None, 1_000); + headless(®, "T-dead", None, 1_000); + reg.finish_pty_exit("T-dead", 1); + assert!(reg.is_live("T-live")); + assert!(!reg.is_live("T-dead")); + assert!(!reg.is_live("T-ghost")); + } + + #[test] + fn probe_returns_identity_row_with_mode_and_resume_id() { + let reg = TerminalRegistry::new(); + reg.register_headless(HeadlessTerminal { + terminal_id: "T-probe".to_string(), + stream_id: "S-probe".to_string(), + mode: "codex".to_string(), + resume_session_id: Some("sess-9".to_string()), + create_request_id: Some("cr-probe".to_string()), + created_at: None, + }); + let row = reg.probe("T-probe").expect("registered"); + assert_eq!(row.mode, "codex"); + assert_eq!(row.resume_session_id.as_deref(), Some("sess-9")); + assert_eq!(row.status, TerminalRunStatus::Running); + assert!(reg.probe("T-ghost").is_none()); + } + + /// §5.4 backstop detector: whenever a create completes and the key now has + /// two or more LIVE terminals, a `ws.reconcile.duplicate_pty` warn event + /// makes the violation loud instead of a silent second JSONL writer. + #[test] + fn second_live_terminal_on_one_key_emits_duplicate_pty_warning() { + let (events, _guard) = tracing_capture::capture(); + let reg = TerminalRegistry::new(); + headless(®, "dup1", Some("cr-dup"), 1_000); + { + let captured = events.lock().unwrap(); + assert!( + !captured + .iter() + .any(|e| e.message == "ws.reconcile.duplicate_pty"), + "one live terminal must not trip the detector" + ); + } + headless(®, "dup2", Some("cr-dup"), 2_000); + let captured = events.lock().unwrap(); + let warn = captured + .iter() + .find(|e| e.message == "ws.reconcile.duplicate_pty") + .expect("two live PTYs on one createRequestId must emit the detector event"); + assert_eq!( + warn.fields.get("create_request_id").map(String::as_str), + Some("cr-dup") + ); + } + + /// §5.1 atomic-stamp insert-edge interleave (§9.1 test 5): the key is part + /// of the row inserted under the registry lock, so an observer's + /// `newest_live_by_create_request_id` sees either no row or the + /// row-with-key — never a row that later gains its key. + #[test] + fn concurrent_inserts_never_expose_a_row_without_its_key() { + let reg = TerminalRegistry::new(); + let stop = Arc::new(std::sync::atomic::AtomicBool::new(false)); + + let reader = { + let reg = reg.clone(); + let stop = Arc::clone(&stop); + std::thread::spawn(move || { + let mut observed = Vec::new(); + while !stop.load(Ordering::Relaxed) { + if let Some(id) = reg.newest_live_by_create_request_id("cr-race") { + assert!( + id == "race1" || id == "race2", + "by-key lookup must only ever see fully-stamped rows, got {id}" + ); + observed.push(id); + } + } + observed + }) + }; + + let w1 = { + let reg = reg.clone(); + std::thread::spawn(move || headless(®, "race1", Some("cr-race"), 1_000)) + }; + let w2 = { + let reg = reg.clone(); + std::thread::spawn(move || headless(®, "race2", Some("cr-race"), 2_000)) + }; + w1.join().unwrap(); + w2.join().unwrap(); + stop.store(true, Ordering::Relaxed); + reader.join().unwrap(); + + // After both inserts, the newest generation (by created_at) wins. + assert_eq!( + reg.newest_live_by_create_request_id("cr-race"), + Some("race2".to_string()) + ); + } } diff --git a/crates/freshell-ws/src/amplifier_association.rs b/crates/freshell-ws/src/amplifier_association.rs index 7767f8d4a..985e8d8d7 100644 --- a/crates/freshell-ws/src/amplifier_association.rs +++ b/crates/freshell-ws/src/amplifier_association.rs @@ -394,6 +394,7 @@ mod tests { None, None, None, + None, ) .expect("spawn a real shell for the test PTY"); state diff --git a/crates/freshell-ws/src/opencode_association.rs b/crates/freshell-ws/src/opencode_association.rs index 382160bd4..4698a07e2 100644 --- a/crates/freshell-ws/src/opencode_association.rs +++ b/crates/freshell-ws/src/opencode_association.rs @@ -429,6 +429,7 @@ mod tests { None, None, None, + None, ) .expect("spawn a real shell for the test PTY"); state diff --git a/crates/freshell-ws/src/terminal.rs b/crates/freshell-ws/src/terminal.rs index 574533fb3..7ac3f602c 100644 --- a/crates/freshell-ws/src/terminal.rs +++ b/crates/freshell-ws/src/terminal.rs @@ -1084,6 +1084,10 @@ async fn handle_create(create: TerminalCreate, ws_tx: &mut WsSink, state: &WsSta stream_id, &mode, resume_session_id.as_deref(), + // The pane's stable creation key, stamped atomically with the registry + // insert (reconciliation design §5.1) — capability-independent and + // inert on its own; only the §5.4 dedupe branch is gated. + Some(&create.request_id), None, on_exit, ) { From c2b546a2c4fb33f0ecd4f61cd5b63ba711b8dec9 Mon Sep 17 00:00:00 2001 From: Codex Date: Thu, 23 Jul 2026 23:53:35 -0700 Subject: [PATCH 3/8] =?UTF-8?q?feat(terminal):=20respawn-generation=20cap?= =?UTF-8?q?=20per=20createRequestId=20(=C2=A77.5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A respawn ↔ instant-exit loop (corrupt JSONL, CLI dying on resume) would make the reconcile derivation re-issue 'respawn' forever. The registry now counts consecutive generations that exited inside a liveness window (default 30s, cap 3); respawn_exhausted(key) lets the derivation return dead_session(reason='respawn_exhausted') instead — restoring the 'at most one respawn' convergence bound as a guarantee. A generation that survives the window resets the counter; user-initiated kills never count. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- crates/freshell-terminal/src/registry.rs | 135 ++++++++++++++++++++++- 1 file changed, 134 insertions(+), 1 deletion(-) diff --git a/crates/freshell-terminal/src/registry.rs b/crates/freshell-terminal/src/registry.rs index d3f3f5d23..d30d8628a 100644 --- a/crates/freshell-terminal/src/registry.rs +++ b/crates/freshell-terminal/src/registry.rs @@ -342,6 +342,12 @@ struct RegistryInner { /// Run-monotonic inventory revision (`terminals.changed.revision`, `§7.5`). Only /// its monotonic increase is asserted by the oracle, not the value. revision: i64, + /// Respawn-generation counters per `createRequestId` (reconciliation design + /// §7.5): consecutive generations that exited WITHIN the liveness window. + /// Reset to 0 whenever a generation survives the window. Read by + /// [`TerminalRegistry::respawn_exhausted`], which turns an infinite + /// respawn ↔ instant-exit loop into a terminal `dead_session` verdict. + respawn_generations: HashMap, } /// Shared, cheaply-cloneable owner of all live terminals, keyed by `terminalId`. @@ -353,6 +359,11 @@ struct RegistryInner { /// hasn't been called yet (e.g. before the boot-time settings load completes). const DEFAULT_AUTO_KILL_IDLE_MINUTES: i64 = 15; +/// Reconciliation §7.5 defaults: a resumed CLI that exits within 30s of +/// spawn, 3 generations in a row, is a respawn ↔ instant-exit loop. +const DEFAULT_RESPAWN_LIVENESS_WINDOW_MS: i64 = 30_000; +const DEFAULT_RESPAWN_GENERATION_CAP: i64 = 3; + #[derive(Clone)] pub struct TerminalRegistry { inner: Arc>, @@ -379,6 +390,14 @@ pub struct TerminalRegistry { /// Captured into each new terminal's `max_replay_chars` at [`Self::create`] /// time (TERM-13) -- see [`compute_scrollback_max_bytes`]. scrollback_max_bytes: Arc, + /// Reconciliation §7.5: a generation that exits within this window of its + /// creation counts toward the respawn cap; one that survives it resets the + /// counter. Atomic (mirrors `auto_kill_idle_minutes`) so tests can shrink + /// it without sleeping. + respawn_liveness_window_ms: Arc, + /// Reconciliation §7.5: consecutive short-lived generations after which + /// [`Self::respawn_exhausted`] fires. + respawn_generation_cap: Arc, } impl Default for TerminalRegistry { @@ -401,11 +420,16 @@ impl TerminalRegistry { inner: Arc::new(Mutex::new(RegistryInner { terminals: HashMap::new(), revision: 0, + respawn_generations: HashMap::new(), })), conn_seq: Arc::new(AtomicU64::new(1)), active_connections: Arc::new(AtomicI64::new(0)), auto_kill_idle_minutes: Arc::new(AtomicI64::new(DEFAULT_AUTO_KILL_IDLE_MINUTES)), scrollback_max_bytes: Arc::new(AtomicI64::new(DEFAULT_MAX_SCROLLBACK_CHARS)), + respawn_liveness_window_ms: Arc::new(AtomicI64::new( + DEFAULT_RESPAWN_LIVENESS_WINDOW_MS, + )), + respawn_generation_cap: Arc::new(AtomicI64::new(DEFAULT_RESPAWN_GENERATION_CAP)), } } @@ -463,6 +487,31 @@ impl TerminalRegistry { self.scrollback_max_bytes.load(Ordering::Relaxed) } + /// Reconciliation §7.5: shrink/grow the liveness window a generation must + /// survive to reset the respawn counter (tests use small values). + pub fn set_respawn_liveness_window_ms(&self, ms: i64) { + self.respawn_liveness_window_ms.store(ms, Ordering::Relaxed); + } + + /// Reconciliation §7.5: how many consecutive short-lived generations + /// exhaust a key. + pub fn set_respawn_generation_cap(&self, cap: i64) { + self.respawn_generation_cap.store(cap, Ordering::Relaxed); + } + + /// Reconciliation §7.5: whether this `createRequestId` has hit the + /// respawn-generation cap — the verdict derivation returns + /// `dead_session(reason='respawn_exhausted')` instead of another + /// `respawn`, restoring §7's "at most one respawn" bound as a guarantee. + pub fn respawn_exhausted(&self, create_request_id: &str) -> bool { + let cap = self.respawn_generation_cap.load(Ordering::Relaxed).max(1) as u32; + let inner = self.inner.lock().expect("registry lock"); + inner + .respawn_generations + .get(create_request_id) + .is_some_and(|count| *count >= cap) + } + /// `enforceIdleKills()` (`terminal-registry.ts:1406-1425`): auto-kill every /// DETACHED **running** terminal idle beyond the configured threshold. /// `auto_kill_idle_minutes() <= 0` is legacy's disabled state -- a no-op. @@ -972,9 +1021,12 @@ impl TerminalRegistry { if s.status == TerminalRunStatus::Exited { return false; } + let now = now_ms(); s.status = TerminalRunStatus::Exited; s.exit_code = Some(exit_code); - s.last_activity_at = now_ms(); + s.last_activity_at = now; + let respawn_key = s.create_request_id.clone(); + let lifetime_ms = now.saturating_sub(s.created_at); let exit = ServerMessage::TerminalExit(TerminalExit { exit_code, terminal_id: terminal_id.to_string(), @@ -984,6 +1036,19 @@ impl TerminalRegistry { } s.subscribers.clear(); drop(s); + // Reconciliation §7.5: a generation that died inside the liveness + // window counts toward the respawn cap; one that survived it resets + // the counter (a healthy resume is not penalized). Natural exits only + // — a user-initiated `kill` removes the record without passing here. + if let Some(key) = respawn_key { + let window = self.respawn_liveness_window_ms.load(Ordering::Relaxed); + let mut inner = self.inner.lock().expect("registry lock"); + if lifetime_ms < window { + *inner.respawn_generations.entry(key).or_insert(0) += 1; + } else { + inner.respawn_generations.remove(&key); + } + } tracing::info!(terminal_id = %terminal_id, exit_code = exit_code, "terminal.exited"); true } @@ -2832,4 +2897,72 @@ mod tests { Some("race2".to_string()) ); } + + // ------------------------------------------------------------------ + // Respawn-generation cap (design §7.5): a respawn ↔ instant-exit loop + // must converge to a terminal dead_session verdict instead of thrashing. + // ------------------------------------------------------------------ + + #[test] + fn respawn_cap_exhausts_after_n_short_lived_generations() { + let reg = TerminalRegistry::new(); + reg.set_respawn_liveness_window_ms(10_000); + reg.set_respawn_generation_cap(3); + + for gen in 1..=3 { + let id = format!("cap-gen{gen}"); + // created_at = now → the exit below is inside the liveness window. + headless(®, &id, Some("cr-cap"), now_ms()); + assert!( + !reg.respawn_exhausted("cr-cap"), + "cap must not fire before generation {gen} exits" + ); + reg.finish_pty_exit(&id, 1); + } + assert!( + reg.respawn_exhausted("cr-cap"), + "3 short-lived generations must exhaust the cap" + ); + // An unrelated key is unaffected. + assert!(!reg.respawn_exhausted("cr-other")); + } + + #[test] + fn healthy_generation_resets_the_respawn_counter() { + let reg = TerminalRegistry::new(); + reg.set_respawn_liveness_window_ms(10_000); + reg.set_respawn_generation_cap(3); + + for gen in 1..=2 { + let id = format!("reset-gen{gen}"); + headless(®, &id, Some("cr-reset"), now_ms()); + reg.finish_pty_exit(&id, 1); + } + // A generation that SURVIVED the liveness window (created long ago) + // exits: the counter resets — a healthy resume is not penalized. + headless(®, "reset-healthy", Some("cr-reset"), now_ms() - 60_000); + reg.finish_pty_exit("reset-healthy", 0); + assert!(!reg.respawn_exhausted("cr-reset")); + + // The next two short-lived exits count from zero again. + for gen in 3..=4 { + let id = format!("reset-gen{gen}"); + headless(®, &id, Some("cr-reset"), now_ms()); + reg.finish_pty_exit(&id, 1); + } + assert!( + !reg.respawn_exhausted("cr-reset"), + "only 2 short-lived generations since the healthy reset" + ); + } + + #[test] + fn exits_without_a_create_request_id_never_count() { + let reg = TerminalRegistry::new(); + reg.set_respawn_liveness_window_ms(10_000); + reg.set_respawn_generation_cap(1); + headless(®, "keyless", None, now_ms()); + reg.finish_pty_exit("keyless", 1); + assert!(!reg.respawn_exhausted("")); + } } From 79f7ffba3710e4c6abdd81d42f7eddff5a9c4c01 Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 24 Jul 2026 00:09:42 -0700 Subject: [PATCH 4/8] feat(ws): reconcile verdict derivation, existence probe, capability negotiation, WS handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The server half of the reconciliation handshake (design §4/§5/§8): - existence.rs: SessionExistenceProbe trait with defined semantics — Present/ Absent require a known consulted provider; unknown provider → Absent (never Unknown); Unknown reserved for a cold index on a known provider. NoIndexProbe fallback for boots with no provider home. - reconcile.rs: pure, read-only derive_verdicts over registry × identity × disk (§5.2/§5.3) — decision-table unit tests cover rows 1, 2, 2b, 3, 4, 4b, 5, 6, 8, 9, 10 plus §9.1-named tests: cardinality/opacity with hostile paneKeys, pure-read idempotency, exited-only-generations never attach, duplicate createRequestId flagged (§5.5), respawn_exhausted → dead_session (§7.5), REST-created-resume crate-boundary identity (assumption 1's Phase-1 acceptance check), trust-boundary corrected:true. - ready capability advertisement gated on hello.capabilities.paneReconcileV1 (§4.2): non-negotiating handshake stays byte-identical (frozen-client inertness at source). - handle_pane_reconcile: 200-pane cap → RECONCILE_TOO_LARGE, catch_unwind → RECONCILE_UNAVAILABLE (both carrying reconcileId), result only in response to a request on a negotiated connection. - retry verdict ships the documented §8.0 placeholder (index_warming + retryAfterMs) — the retry-mechanism decision is recorded OPEN for the user. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- crates/freshell-server/src/main.rs | 4 + .../freshell-ws/src/amplifier_association.rs | 1 + crates/freshell-ws/src/existence.rs | 106 +++ crates/freshell-ws/src/lib.rs | 63 +- .../freshell-ws/src/opencode_association.rs | 1 + crates/freshell-ws/src/reconcile.rs | 698 ++++++++++++++++++ crates/freshell-ws/src/terminal.rs | 72 ++ .../tests/codex_managed_launch_e2e.rs | 1 + .../tests/codex_session_ref_resume.rs | 1 + .../tests/diag01_lifecycle_events.rs | 1 + .../tests/freshagent_claude_kill_interrupt.rs | 1 + crates/freshell-ws/tests/hello_timeout.rs | 1 + crates/freshell-ws/tests/keepalive.rs | 1 + crates/freshell-ws/tests/max_payload.rs | 1 + crates/freshell-ws/tests/origin_policy.rs | 1 + .../tests/safe08_restore_diagnostics.rs | 1 + .../tests/session_identity_frames.rs | 1 + .../freshell-ws/tests/term09_output_queue.rs | 1 + 18 files changed, 955 insertions(+), 1 deletion(-) create mode 100644 crates/freshell-ws/src/existence.rs create mode 100644 crates/freshell-ws/src/reconcile.rs diff --git a/crates/freshell-server/src/main.rs b/crates/freshell-server/src/main.rs index ab005c17b..a2fd9a6a8 100644 --- a/crates/freshell-server/src/main.rs +++ b/crates/freshell-server/src/main.rs @@ -352,6 +352,10 @@ async fn main() -> ExitCode { identity: terminal_identity.clone(), amplifier_locator: amplifier_locator.clone(), opencode_locator: opencode_locator.clone(), + // Reconciliation handshake disk-truth probe: honest `Unknown` answers + // until the index-backed probe below replaces this (no provider home => + // stays the no-index fallback). + session_existence: std::sync::Arc::new(freshell_ws::existence::NoIndexProbe::default()), auth_token: Arc::clone(&auth_token), // Shared (not moved) so `GET /api/health` reports the SAME `instanceId`. server_instance_id: Arc::clone(&server_instance_id), diff --git a/crates/freshell-ws/src/amplifier_association.rs b/crates/freshell-ws/src/amplifier_association.rs index 985e8d8d7..8a061a5a5 100644 --- a/crates/freshell-ws/src/amplifier_association.rs +++ b/crates/freshell-ws/src/amplifier_association.rs @@ -286,6 +286,7 @@ mod tests { term09: crate::backpressure::Term09Config::default(), config_fallback: None, amplifier_locator: Some(StdArc::new(AmplifierLocator::new(amplifier_home))), + session_existence: std::sync::Arc::new(crate::existence::NoIndexProbe::default()), opencode_locator: None, }; (state, rx) diff --git a/crates/freshell-ws/src/existence.rs b/crates/freshell-ws/src/existence.rs new file mode 100644 index 000000000..bc88eaa44 --- /dev/null +++ b/crates/freshell-ws/src/existence.rs @@ -0,0 +1,106 @@ +//! `SessionExistence` — the reconcile derivation's disk-truth input +//! (reconciliation-handshake design §5.1, second additive piece). +//! +//! Answers "does `provider:sessionId` exist on disk?" with DEFINED semantics: +//! +//! * [`SessionExistence::Present`] / [`SessionExistence::Absent`] require a +//! **known provider** whose index has been consulted. +//! * An **unknown provider** returns `Absent` (surfacing as `fresh`/`invalid` +//! downstream), never `Unknown`. +//! * [`SessionExistence::Unknown`] is reserved strictly for a *cold index on a +//! known provider* (boot sweep not finished / index unavailable) — it is +//! what makes the `retry` verdict honest instead of guessing (§5.3 row 5). +//! +//! Trait-shaped so crate tests inject a fake; the real implementation is +//! backed by the shared `freshell_sessions::directory_index::SessionIndex` +//! and constructed in `freshell-server::main`, cloned into [`crate::WsState`] +//! — the exact precedent of the `identity` and locator handles. + +use std::sync::Arc; + +/// Disk-existence answer for one `provider:sessionId`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SessionExistence { + /// The identity exists on disk (known provider, consulted index). + Present, + /// The identity does not exist on disk — ALSO the answer for an unknown + /// provider (never `Unknown`). + Absent, + /// Known provider, but the index cannot answer yet (cold/unavailable). + Unknown, +} + +/// The reconcile derivation's read-only view of disk session truth. +pub trait SessionExistenceProbe: Send + Sync { + /// Does `provider:sessionId` exist on disk? See the module doc for the + /// Present/Absent/Unknown contract. + fn exists(&self, provider: &str, session_id: &str) -> SessionExistence; + + /// Whether this identity has EVER been observed on disk by this probe. + /// Gates `dead_session` (§5.3 rows 4/4b): a data-loss-shaped verdict is + /// only raised for an identity disk has some memory of — a never-observed + /// (stale/typo) claim falls through to `fresh`. + fn ever_observed(&self, provider: &str, session_id: &str) -> bool; +} + +/// The no-index fallback (mirrors `session_index: None` in +/// `freshell-server::main` when no provider home resolves): every query on a +/// KNOWN provider is honestly `Unknown` (the index does not exist, so nothing +/// can be asserted); unknown providers are `Absent` per the contract. +pub struct NoIndexProbe { + known_providers: Vec, +} + +impl NoIndexProbe { + pub fn new(known_providers: Vec) -> Self { + Self { known_providers } + } +} + +impl Default for NoIndexProbe { + /// The four disk-indexed providers of `freshell-server::main`'s + /// `SessionIndex` construction. + fn default() -> Self { + Self::new( + ["claude", "codex", "opencode", "amplifier"] + .into_iter() + .map(str::to_string) + .collect(), + ) + } +} + +impl SessionExistenceProbe for NoIndexProbe { + fn exists(&self, provider: &str, _session_id: &str) -> SessionExistence { + if self.known_providers.iter().any(|p| p == provider) { + SessionExistence::Unknown + } else { + SessionExistence::Absent + } + } + + fn ever_observed(&self, _provider: &str, _session_id: &str) -> bool { + false + } +} + +/// Shared handle type carried on [`crate::WsState`]. +pub type SharedExistenceProbe = Arc; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn no_index_probe_is_unknown_for_known_provider_and_absent_for_unknown() { + let probe = NoIndexProbe::default(); + assert_eq!(probe.exists("claude", "s1"), SessionExistence::Unknown); + assert_eq!(probe.exists("amplifier", "s1"), SessionExistence::Unknown); + // Unknown provider → Absent, NEVER Unknown (design §5.1 / change #4c). + assert_eq!( + probe.exists("not-a-provider", "s1"), + SessionExistence::Absent + ); + assert!(!probe.ever_observed("claude", "s1")); + } +} diff --git a/crates/freshell-ws/src/lib.rs b/crates/freshell-ws/src/lib.rs index c712587ca..dd1ab2cea 100644 --- a/crates/freshell-ws/src/lib.rs +++ b/crates/freshell-ws/src/lib.rs @@ -22,10 +22,12 @@ pub mod amplifier_association; pub mod backpressure; +pub mod existence; pub mod identity; pub(crate) mod invariants; pub mod opencode_association; pub mod origin; +pub mod reconcile; pub mod screenshot; pub mod tabs; pub mod tabs_persist; @@ -206,6 +208,13 @@ pub struct WsState { /// `SessionDirectoryState::session_index`'s `Option` convention) -- every /// [`crate::amplifier_association`] entry point no-ops in that case. pub amplifier_locator: Option>, + /// Reconciliation handshake (design §5.1): the disk-truth probe behind the + /// `pane.reconcile.request` verdict derivation — "does `provider:sessionId` + /// exist on disk?" with defined Present/Absent/Unknown semantics. Backed by + /// the shared session index in `freshell-server::main` (the exact precedent + /// of `identity` and the locator handles); [`crate::existence::NoIndexProbe`] + /// when no provider home resolves. + pub session_existence: crate::existence::SharedExistenceProbe, /// The opencode terminal-pane session locator (restore-across-restart fix, /// `docs/plans/2026-07-18-opencode-terminal-restore-spec.md`): correlates a /// fresh opencode PTY's first Enter/submit (or a row written at spawn) with @@ -321,12 +330,27 @@ pub fn spawn_idle_monitor( /// would lose scrollback). On a truly fresh boot the registry is empty, so this stays /// byte-identical to the clean-boot handshake the oracle's T0/determinism tiers pin. pub fn build_handshake(state: &WsState) -> Vec { + build_handshake_with_capabilities(state, false) +} + +/// [`build_handshake`], parameterized on the connection's negotiated +/// `hello.capabilities.paneReconcileV1` (reconciliation design §4.2): the +/// `ready.capabilities` advertisement is emitted **only when the client's +/// `hello` opted in** — today's frozen client doesn't, so the emitted +/// handshake stays byte-for-byte identical to the pinned clean-boot shape. +pub fn build_handshake_with_capabilities( + state: &WsState, + pane_reconcile_v1: bool, +) -> Vec { let boot_id = state.boot_id.as_ref().clone(); let mut messages = vec![ ServerMessage::Ready(Ready { timestamp: now_iso(), boot_id: Some(boot_id.clone()), server_instance_id: Some(state.server_instance_id.as_ref().clone()), + capabilities: pane_reconcile_v1.then_some(freshell_protocol::ReadyCapabilities { + pane_reconcile_v1: Some(true), + }), }), ServerMessage::SettingsUpdated(SettingsUpdated { settings: state.settings.as_ref().clone(), @@ -509,8 +533,17 @@ async fn handle_socket( } } + // Reconciliation handshake negotiation (§4.1/§4.2): the `ready` + // advertisement is gated on the client's `hello` opt-in, so a frozen + // client's handshake stays byte-for-byte unchanged. + let pane_reconcile_v1 = value + .get("capabilities") + .and_then(|c| c.get("paneReconcileV1")) + .and_then(|v| v.as_bool()) + .unwrap_or(false); + // Authenticated: emit the ordered handshake. - for msg in build_handshake(&state) { + for msg in build_handshake_with_capabilities(&state, pane_reconcile_v1) { let json = match serde_json::to_string(&msg) { Ok(json) => json, Err(_) => return, @@ -546,6 +579,7 @@ async fn handle_socket( bcast_rx, terminal_output_batch_v1, ui_screenshot_v1, + pane_reconcile_v1, origin_kind, ) .await; @@ -667,6 +701,7 @@ mod tests { config_fallback: None, amplifier_locator: None, opencode_locator: None, + session_existence: std::sync::Arc::new(crate::existence::NoIndexProbe::default()), } } @@ -719,6 +754,32 @@ mod tests { ); } + /// Reconciliation §4.2: the `ready.capabilities` advertisement is emitted + /// ONLY for a hello that opted in — the default handshake stays + /// byte-identical to the pinned clean-boot shape (frozen-client inertness + /// at the source). + #[test] + fn handshake_advertises_pane_reconcile_only_when_negotiated() { + let s = state(); + let negotiated = build_handshake_with_capabilities(&s, true); + let ready = serde_json::to_value(&negotiated[0]).unwrap(); + assert_eq!( + ready["capabilities"], + serde_json::json!({ "paneReconcileV1": true }) + ); + + let default = build_handshake(&s); + let ready = serde_json::to_value(&default[0]).unwrap(); + assert!( + ready.get("capabilities").is_none(), + "non-negotiating hello must not change ready's shape: {ready}" + ); + // Same shape as an explicit `false` negotiation. + let unnegotiated = build_handshake_with_capabilities(&s, false); + let ready2 = serde_json::to_value(&unnegotiated[0]).unwrap(); + assert!(ready2.get("capabilities").is_none()); + } + #[test] fn handshake_is_ordered_with_shared_bootid() { let msgs = build_handshake(&state()); diff --git a/crates/freshell-ws/src/opencode_association.rs b/crates/freshell-ws/src/opencode_association.rs index 4698a07e2..d16434551 100644 --- a/crates/freshell-ws/src/opencode_association.rs +++ b/crates/freshell-ws/src/opencode_association.rs @@ -273,6 +273,7 @@ mod tests { config_fallback: None, amplifier_locator: None, opencode_locator: Some(StdArc::new(OpencodeLocator::new(data_home))), + session_existence: std::sync::Arc::new(crate::existence::NoIndexProbe::default()), }; (state, rx) } diff --git a/crates/freshell-ws/src/reconcile.rs b/crates/freshell-ws/src/reconcile.rs new file mode 100644 index 000000000..b82eef02e --- /dev/null +++ b/crates/freshell-ws/src/reconcile.rs @@ -0,0 +1,698 @@ +//! Reconciliation-handshake verdict derivation (design §5) — the pure, +//! read-only function behind `pane.reconcile.request`. +//! +//! One protocol replaces the N ad-hoc client-side latches: the client +//! *presents* its pane view; this module answers with an authoritative +//! per-pane verdict derived from the terminal registry × identity registry × +//! disk session index. The derivation MUTATES NOTHING — receiving the same +//! request 1 or N times, on 1 or N sockets, is indistinguishable from +//! receiving it once (the idempotency keystone, §7.2). + +use freshell_protocol::{PaneVerdict, ReconcilePane, ReconcileVerdict, SessionLocator}; +use freshell_terminal::TerminalRegistry; + +use crate::existence::{SessionExistence, SessionExistenceProbe}; +use crate::identity::TerminalIdentityRegistry; + +/// §4.3: an over-cap request is answered with `error{RECONCILE_TOO_LARGE}`, +/// never silently truncated. +pub const MAX_RECONCILE_PANES: usize = 200; + +/// §5.3 row 5 placeholder cadence for `retry(index_warming)` (the retry +/// mechanism itself is an OPEN user decision, design §8.0 — this implements +/// the documented placeholder). +pub const RETRY_AFTER_MS: i64 = 2000; + +/// The read-only inputs of the derivation (§5.1) — all shared handles that +/// already live on [`crate::WsState`]. +pub struct ReconcileDeps<'a> { + pub registry: &'a TerminalRegistry, + pub identity: &'a TerminalIdentityRegistry, + pub existence: &'a dyn SessionExistenceProbe, +} + +/// Derive one verdict per presented pane, 1:1 by `paneKey`, order preserved +/// (§8 total cardinality: a malformed entry gets `invalid{reason}`, never +/// omission). Pure read — no server state is mutated. +pub fn derive_verdicts(deps: &ReconcileDeps<'_>, panes: &[ReconcilePane]) -> Vec { + let mut seen_keys: std::collections::HashSet<&str> = std::collections::HashSet::new(); + panes + .iter() + .map(|pane| { + // §5.5 server-side contract enforcement: two panes in ONE request + // carrying the same createRequestId would each drive a create — + // flag the duplicate instead of emitting two actionable verdicts. + if let Some(key) = pane.create_request_id.as_deref().filter(|k| !k.is_empty()) { + if !seen_keys.insert(key) { + return invalid(pane, "duplicate_create_request_id"); + } + } + verdict_for_pane(deps, pane) + }) + .collect() +} + +fn invalid(pane: &ReconcilePane, reason: &str) -> PaneVerdict { + PaneVerdict { + pane_key: pane.pane_key.clone(), + verdict: ReconcileVerdict::Invalid, + terminal_id: None, + session_ref: None, + corrected: None, + reason: Some(reason.to_string()), + retry_after_ms: None, + duplicate: None, + } +} + +fn base(pane: &ReconcilePane, verdict: ReconcileVerdict) -> PaneVerdict { + PaneVerdict { + pane_key: pane.pane_key.clone(), + verdict, + terminal_id: None, + session_ref: None, + corrected: None, + reason: None, + retry_after_ms: None, + duplicate: None, + } +} + +/// `corrected: true` iff the server overrode a DIFFERING client claim (§5.2): +/// requires a claim to be present and the server's ref to be present and +/// different — the server-wins rule that retires the client's `matchScore` +/// guessing. +fn corrected_flag(claim: Option<&SessionLocator>, server: Option<&SessionLocator>) -> Option { + match (claim, server) { + (Some(c), Some(s)) if c != s => Some(true), + _ => None, + } +} + +/// §5.2 `resolve_authoritative_ref`: server memory wins (even retired), then +/// the retired identity of the newest generation for the key — including the +/// registry-side `resume_session_id` path for REST-created resumes that never +/// reached the WS-owned identity registry (design assumption 1's acceptance +/// check) — then the client's claims, promoted by ONE uniform rule. +fn resolve_authoritative_ref( + deps: &ReconcileDeps<'_>, + pane: &ReconcilePane, + key: &str, +) -> Option { + // 1. Identity registry by the client's terminalId (retired entries included). + if let Some(tid) = pane.terminal_id.as_deref() { + if let Some(sref) = deps.identity.session_ref_for(tid) { + return Some(sref); + } + } + // 2. The newest generation (INCLUDING exited) for this key. + if let Some(newest) = deps.registry.newest_by_create_request_id(key) { + if let Some(sref) = deps.identity.session_ref_for(&newest) { + return Some(sref); + } + // Crate-boundary path: REST-created resumes carry identity only on + // the registry row (`IdentityProbeRow.resume_session_id`). + if let Some(row) = deps.registry.probe(&newest) { + if row.mode != "shell" { + if let Some(rsid) = row.resume_session_id.filter(|s| !s.is_empty()) { + return Some(SessionLocator { + provider: row.mode, + session_id: rsid, + }); + } + } + } + } + // 3. The client's structured claim (validated against disk by the caller). + if let Some(sref) = pane.session_ref.clone() { + return Some(sref); + } + // 4. ONE uniform promotion rule: {provider: mode, sessionId: resumeSessionId}. + let mode = pane + .mode + .as_deref() + .filter(|m| !m.is_empty() && *m != "shell")?; + let rsid = pane + .resume_session_id + .as_deref() + .filter(|s| !s.is_empty())?; + Some(SessionLocator { + provider: mode.to_string(), + session_id: rsid.to_string(), + }) +} + +fn attach( + deps: &ReconcileDeps<'_>, + pane: &ReconcilePane, + terminal_id: String, + duplicate: Option, +) -> PaneVerdict { + let server_ref = deps.identity.session_ref_for(&terminal_id); + let corrected = corrected_flag(pane.session_ref.as_ref(), server_ref.as_ref()); + PaneVerdict { + terminal_id: Some(terminal_id), + session_ref: server_ref, + corrected, + duplicate, + ..base(pane, ReconcileVerdict::Attach) + } +} + +fn verdict_for_pane(deps: &ReconcileDeps<'_>, pane: &ReconcilePane) -> PaneVerdict { + // §5.3 row 10 protocol hygiene: malformed entries get an explicit reason. + if pane.pane_key.is_empty() { + return invalid(pane, "missing_pane_key"); + } + match pane.kind.as_deref() { + Some("terminal") => {} + Some(_) => return invalid(pane, "unsupported_kind"), + None => return invalid(pane, "missing_kind"), + } + let Some(key) = pane + .create_request_id + .as_deref() + .filter(|k| !k.is_empty()) + .map(str::to_string) + else { + return invalid(pane, "missing_create_request_id"); + }; + + // Rows 1/2/2b: any LIVE terminal wins. + let t1 = deps.registry.newest_live_by_create_request_id(&key); + let t2 = pane + .terminal_id + .as_deref() + .filter(|tid| deps.registry.is_live(tid)) + .map(str::to_string); + match (t1, t2) { + // Row 2b (invariant I6): the client is live-attached to T while a + // newer duplicate generation T′ exists for the same key — keep the + // client on T, flag T′, never silently switch. + (Some(t1), Some(t2)) if t1 != t2 => return attach(deps, pane, t2, Some(t1)), + (Some(t1), _) => return attach(deps, pane, t1, None), + (None, Some(t2)) => return attach(deps, pane, t2, None), + (None, None) => {} + } + + // No live terminal for this key — recover a retired identity if one exists. + let Some(sref) = resolve_authoritative_ref(deps, pane, &key) else { + // Row 8: shells are stateless by design; row 9: CLI with nothing to + // resume becomes an explicit, labeled fresh — never a surprise. + if pane.mode.as_deref() == Some("shell") { + return base(pane, ReconcileVerdict::Fresh); + } + return PaneVerdict { + reason: Some("no_recoverable_identity".to_string()), + ..base(pane, ReconcileVerdict::Fresh) + }; + }; + + match deps.existence.exists(&sref.provider, &sref.session_id) { + SessionExistence::Present => { + // §7.5: a respawn ↔ instant-exit loop converges to a terminal, + // actionable dead_session instead of thrashing forever. + if deps.registry.respawn_exhausted(&key) { + return PaneVerdict { + session_ref: Some(sref), + reason: Some("respawn_exhausted".to_string()), + ..base(pane, ReconcileVerdict::DeadSession) + }; + } + let corrected = corrected_flag(pane.session_ref.as_ref(), Some(&sref)); + PaneVerdict { + session_ref: Some(sref), + corrected, + ..base(pane, ReconcileVerdict::Respawn) + } + } + SessionExistence::Absent => { + // dead_session is gated on the identity having been SEEN on disk + // at least once — never a data-loss-shaped verdict for an + // identity disk has no memory of (§5.3 rows 4/4b). + if deps + .existence + .ever_observed(&sref.provider, &sref.session_id) + { + PaneVerdict { + session_ref: Some(sref), + reason: Some("session_not_on_disk".to_string()), + ..base(pane, ReconcileVerdict::DeadSession) + } + } else { + PaneVerdict { + reason: Some("identity_never_observed".to_string()), + ..base(pane, ReconcileVerdict::Fresh) + } + } + } + SessionExistence::Unknown => PaneVerdict { + reason: Some("index_warming".to_string()), + retry_after_ms: Some(RETRY_AFTER_MS), + ..base(pane, ReconcileVerdict::Retry) + }, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use freshell_terminal::registry::HeadlessTerminal; + use std::collections::HashMap; + use std::sync::Mutex; + + /// §5.1 test fake: per-key existence answers + an observed-history set. + #[derive(Default)] + struct FakeProbe { + answers: Mutex>, + observed: Mutex>, + } + + impl FakeProbe { + fn set(&self, provider: &str, session_id: &str, existence: SessionExistence) { + self.answers + .lock() + .unwrap() + .insert(format!("{provider}:{session_id}"), existence); + if existence == SessionExistence::Present { + self.observed + .lock() + .unwrap() + .insert(format!("{provider}:{session_id}")); + } + } + + fn mark_observed(&self, provider: &str, session_id: &str) { + self.observed + .lock() + .unwrap() + .insert(format!("{provider}:{session_id}")); + } + } + + impl SessionExistenceProbe for FakeProbe { + fn exists(&self, provider: &str, session_id: &str) -> SessionExistence { + *self + .answers + .lock() + .unwrap() + .get(&format!("{provider}:{session_id}")) + .unwrap_or(&SessionExistence::Absent) + } + + fn ever_observed(&self, provider: &str, session_id: &str) -> bool { + self.observed + .lock() + .unwrap() + .contains(&format!("{provider}:{session_id}")) + } + } + + struct Fixture { + registry: TerminalRegistry, + identity: TerminalIdentityRegistry, + probe: FakeProbe, + } + + impl Fixture { + fn new() -> Self { + Self { + registry: TerminalRegistry::new(), + identity: TerminalIdentityRegistry::new(), + probe: FakeProbe::default(), + } + } + + fn deps(&self) -> ReconcileDeps<'_> { + ReconcileDeps { + registry: &self.registry, + identity: &self.identity, + existence: &self.probe, + } + } + + fn headless(&self, id: &str, key: Option<&str>, mode: &str, created_at: i64) { + self.registry.register_headless(HeadlessTerminal { + terminal_id: id.to_string(), + stream_id: format!("S-{id}"), + mode: mode.to_string(), + resume_session_id: None, + create_request_id: key.map(str::to_string), + created_at: Some(created_at), + }); + } + + fn one(&self, pane: ReconcilePane) -> PaneVerdict { + let verdicts = derive_verdicts(&self.deps(), &[pane]); + assert_eq!(verdicts.len(), 1); + verdicts.into_iter().next().unwrap() + } + } + + fn pane(key: &str) -> ReconcilePane { + ReconcilePane { + pane_key: format!("pk-{key}"), + kind: Some("terminal".to_string()), + mode: Some("claude".to_string()), + create_request_id: Some(key.to_string()), + terminal_id: None, + server_instance_id: None, + session_ref: None, + resume_session_id: None, + status: None, + } + } + + fn sref(provider: &str, id: &str) -> SessionLocator { + SessionLocator { + provider: provider.to_string(), + session_id: id.to_string(), + } + } + + // --- decision table §5.3 -------------------------------------------- + + /// Row 1: a live terminal under this createRequestId wins over anything + /// the client claims (closes the interrupted-respawn orphan). + #[test] + fn row1_live_terminal_under_key_yields_attach() { + let f = Fixture::new(); + f.headless("T-live", Some("cr-1"), "claude", 1_000); + f.identity + .upsert("T-live", Some("claude"), Some("s-1"), None, 1); + + let mut p = pane("cr-1"); + p.terminal_id = Some("T-stale-handle".to_string()); // dead handle + let v = f.one(p); + assert_eq!(v.verdict, ReconcileVerdict::Attach); + assert_eq!(v.terminal_id.as_deref(), Some("T-live")); + assert_eq!(v.session_ref, Some(sref("claude", "s-1"))); + } + + /// Row 2: no match by key, but the presented terminalId is live → + /// attach(T) with the server's corrected identity. + #[test] + fn row2_live_presented_terminal_yields_attach_with_corrected_ref() { + let f = Fixture::new(); + f.headless("T-2", None, "claude", 1_000); + f.identity + .upsert("T-2", Some("claude"), Some("s-real"), None, 1); + + let mut p = pane("cr-2"); + p.terminal_id = Some("T-2".to_string()); + p.session_ref = Some(sref("claude", "s-wrong")); // contradicting claim + let v = f.one(p); + assert_eq!(v.verdict, ReconcileVerdict::Attach); + assert_eq!(v.terminal_id.as_deref(), Some("T-2")); + assert_eq!(v.session_ref, Some(sref("claude", "s-real"))); + assert_eq!(v.corrected, Some(true), "server overrode a differing claim"); + } + + /// Row 2b (§9.1 test 12, invariant I6): live-attached T + newer duplicate + /// T′ for the same key → keep the client on T, flag T′, never switch. + #[test] + fn row2b_both_live_prefers_clients_terminal_and_flags_duplicate() { + let f = Fixture::new(); + f.headless("T-mine", Some("cr-2b"), "claude", 1_000); + f.headless("T-newer", Some("cr-2b"), "claude", 2_000); + + let mut p = pane("cr-2b"); + p.terminal_id = Some("T-mine".to_string()); + let v = f.one(p); + assert_eq!(v.verdict, ReconcileVerdict::Attach); + assert_eq!( + v.terminal_id.as_deref(), + Some("T-mine"), + "never silently switch a live attachment" + ); + assert_eq!(v.duplicate.as_deref(), Some("T-newer")); + } + + /// Row 3 (+ §9.1 test 11): terminal exited; its RETIRED identity drives + /// respawn when the session is Present on disk. + #[test] + fn row3_exited_terminal_with_retired_identity_yields_respawn() { + let f = Fixture::new(); + f.headless("T-3", Some("cr-3"), "claude", 1_000); + f.identity + .upsert("T-3", Some("claude"), Some("s-3"), None, 1); + f.registry.finish_pty_exit("T-3", 0); + f.identity.retire("T-3"); + f.probe.set("claude", "s-3", SessionExistence::Present); + + let mut p = pane("cr-3"); + p.terminal_id = Some("T-3".to_string()); + let v = f.one(p); + assert_eq!(v.verdict, ReconcileVerdict::Respawn); + assert_eq!(v.session_ref, Some(sref("claude", "s-3"))); + } + + /// §9.1 test 11 second half + row 4b: an identity the index has NEVER + /// observed → fresh(identity_never_observed), never dead_session. + #[test] + fn row4b_never_observed_identity_yields_fresh_not_dead_session() { + let f = Fixture::new(); + let mut p = pane("cr-4b"); + p.session_ref = Some(sref("claude", "s-typo")); + // Probe default: Absent + never observed. + let v = f.one(p); + assert_eq!(v.verdict, ReconcileVerdict::Fresh); + assert_eq!(v.reason.as_deref(), Some("identity_never_observed")); + } + + /// Row 4/7: Absent but EVER seen on disk → explicit dead_session. + #[test] + fn row4_absent_but_ever_observed_yields_dead_session() { + let f = Fixture::new(); + f.probe.mark_observed("claude", "s-gone"); + let mut p = pane("cr-4"); + p.session_ref = Some(sref("claude", "s-gone")); + let v = f.one(p); + assert_eq!(v.verdict, ReconcileVerdict::DeadSession); + assert_eq!(v.reason.as_deref(), Some("session_not_on_disk")); + assert_eq!( + v.session_ref, + Some(sref("claude", "s-gone")), + "dead_session carries the claimed-but-missing identity for the error UI" + ); + } + + /// Row 5 (§9.1 test 6): cold index on a known provider → honest retry, + /// never dead_session, never optimistic respawn. + #[test] + fn row5_unknown_existence_yields_retry_with_backoff() { + let f = Fixture::new(); + f.probe.set("claude", "s-cold", SessionExistence::Unknown); + let mut p = pane("cr-5"); + p.session_ref = Some(sref("claude", "s-cold")); + let v = f.one(p); + assert_eq!(v.verdict, ReconcileVerdict::Retry); + assert_eq!(v.reason.as_deref(), Some("index_warming")); + assert_eq!(v.retry_after_ms, Some(RETRY_AFTER_MS)); + } + + /// Row 6: no terminalId at all, just a claim that IS on disk → respawn + /// (restore-after-persist-cycle). The claim matched, so no `corrected`. + #[test] + fn row6_claim_present_on_disk_yields_respawn_without_corrected() { + let f = Fixture::new(); + f.probe.set("codex", "s-6", SessionExistence::Present); + let mut p = pane("cr-6"); + p.mode = Some("codex".to_string()); + p.session_ref = Some(sref("codex", "s-6")); + let v = f.one(p); + assert_eq!(v.verdict, ReconcileVerdict::Respawn); + assert_eq!(v.session_ref, Some(sref("codex", "s-6"))); + assert_eq!(v.corrected, None); + } + + /// Row 8: shells are stateless by design — plain fresh. + #[test] + fn row8_shell_pane_with_nothing_yields_fresh() { + let f = Fixture::new(); + let mut p = pane("cr-8"); + p.mode = Some("shell".to_string()); + let v = f.one(p); + assert_eq!(v.verdict, ReconcileVerdict::Fresh); + assert_eq!(v.reason, None); + } + + /// Row 9: CLI pane with no recoverable identity — an explicit, labeled + /// fresh (never a surprise grey pane). + #[test] + fn row9_cli_pane_with_no_identity_yields_labeled_fresh() { + let f = Fixture::new(); + let v = f.one(pane("cr-9")); + assert_eq!(v.verdict, ReconcileVerdict::Fresh); + assert_eq!(v.reason.as_deref(), Some("no_recoverable_identity")); + } + + /// Row 10: malformed entries → invalid{reason}, never omission. + #[test] + fn row10_malformed_entries_yield_invalid_with_reasons() { + let f = Fixture::new(); + + let mut no_key = pane("cr-10a"); + no_key.pane_key = String::new(); + assert_eq!(f.one(no_key).reason.as_deref(), Some("missing_pane_key")); + + let mut no_crid = pane("cr-10b"); + no_crid.create_request_id = None; + let v = f.one(no_crid); + assert_eq!(v.verdict, ReconcileVerdict::Invalid); + assert_eq!(v.reason.as_deref(), Some("missing_create_request_id")); + + let mut bad_kind = pane("cr-10c"); + bad_kind.kind = Some("fresh-agent".to_string()); + let v = f.one(bad_kind); + assert_eq!(v.verdict, ReconcileVerdict::Invalid); + assert_eq!(v.reason.as_deref(), Some("unsupported_kind")); + } + + // --- beyond the table: §9.1-named tests ------------------------------- + + /// §9.1 test 3: N panes in → N verdicts out, paneKey echoed VERBATIM + /// (hostile strings included), order preserved. + #[test] + fn cardinality_and_opacity_hold_for_hostile_pane_keys() { + let f = Fixture::new(); + let hostile = r#"tab"3:\pane {} 💥 \u0000"#; + let mut p1 = pane("cr-a"); + p1.pane_key = hostile.to_string(); + let p2 = pane("cr-b"); + let mut p3 = pane("cr-c"); + p3.create_request_id = None; // malformed — still answered + + let verdicts = derive_verdicts(&f.deps(), &[p1, p2, p3]); + assert_eq!(verdicts.len(), 3); + assert_eq!(verdicts[0].pane_key, hostile); + assert_eq!(verdicts[1].pane_key, "pk-cr-b"); + assert_eq!(verdicts[2].verdict, ReconcileVerdict::Invalid); + } + + /// §9.1 test 4a (derivation half): the derivation is a deterministic pure + /// read — the same request twice yields identical verdicts. + #[test] + fn same_request_twice_yields_identical_verdicts() { + let f = Fixture::new(); + f.headless("T-idem", Some("cr-idem"), "claude", 1_000); + f.identity + .upsert("T-idem", Some("claude"), Some("s-i"), None, 1); + let panes = vec![pane("cr-idem"), pane("cr-other")]; + let first = derive_verdicts(&f.deps(), &panes); + let second = derive_verdicts(&f.deps(), &panes); + assert_eq!(first, second); + } + + /// §9.1 test 9 (spawn-failed / exited exclusion): a key whose only + /// generations have EXITED must re-derive respawn/fresh — never a phantom + /// attach to a dead handle. + #[test] + fn exited_only_generations_never_yield_attach() { + let f = Fixture::new(); + f.headless("T-dead", Some("cr-dead"), "claude", 1_000); + f.registry.finish_pty_exit("T-dead", 1); + let v = f.one(pane("cr-dead")); + assert_ne!(v.verdict, ReconcileVerdict::Attach); + } + + /// §9.1 test 14 (§5.5 contract): duplicate createRequestId within ONE + /// request — the duplicate is flagged invalid, the first is answered. + #[test] + fn duplicate_create_request_id_in_one_request_is_flagged() { + let f = Fixture::new(); + let mut p1 = pane("cr-dup"); + p1.pane_key = "first".to_string(); + let mut p2 = pane("cr-dup"); + p2.pane_key = "second".to_string(); + let verdicts = derive_verdicts(&f.deps(), &[p1, p2]); + assert_ne!(verdicts[0].verdict, ReconcileVerdict::Invalid); + assert_eq!(verdicts[1].verdict, ReconcileVerdict::Invalid); + assert_eq!( + verdicts[1].reason.as_deref(), + Some("duplicate_create_request_id") + ); + } + + /// §9.1 test 15 (§7.5): the respawn-generation cap converts an infinite + /// respawn loop into dead_session(respawn_exhausted); the healthy-reset + /// half lives in `freshell-terminal`'s cap tests. + #[test] + fn respawn_exhausted_key_yields_dead_session_not_another_respawn() { + let f = Fixture::new(); + f.registry.set_respawn_liveness_window_ms(60_000); + f.registry.set_respawn_generation_cap(2); + f.probe.set("claude", "s-loop", SessionExistence::Present); + for gen in 1..=2 { + let id = format!("T-loop{gen}"); + f.headless(&id, Some("cr-loop"), "claude", now_ms_for_test()); + f.identity + .upsert(&id, Some("claude"), Some("s-loop"), None, gen); + f.registry.finish_pty_exit(&id, 1); + f.identity.retire(&id); + } + let v = f.one(pane("cr-loop")); + assert_eq!(v.verdict, ReconcileVerdict::DeadSession); + assert_eq!(v.reason.as_deref(), Some("respawn_exhausted")); + assert_eq!(v.session_ref, Some(sref("claude", "s-loop"))); + } + + /// Design assumption 1's Phase-1 ACCEPTANCE CHECK: a REST-created resumed + /// terminal (registry-side `resume_session_id`, NO identity-registry + /// entry) reconciles to respawn with the correct sessionRef — the + /// derivation reads identity across the crate boundary. + #[test] + fn rest_created_resume_resolves_identity_from_registry_row() { + let f = Fixture::new(); + f.registry.register_headless(HeadlessTerminal { + terminal_id: "T-rest".to_string(), + stream_id: "S-rest".to_string(), + mode: "codex".to_string(), + resume_session_id: Some("s-rest".to_string()), + create_request_id: Some("cr-rest".to_string()), + created_at: Some(1_000), + }); + // NO identity-registry entry — the WS-owned registry never saw this + // create. The terminal has exited (server restart shape). + f.registry.finish_pty_exit("T-rest", 0); + f.probe.set("codex", "s-rest", SessionExistence::Present); + + let mut p = pane("cr-rest"); + p.mode = Some("codex".to_string()); + let v = f.one(p); + assert_eq!(v.verdict, ReconcileVerdict::Respawn); + assert_eq!( + v.session_ref, + Some(sref("codex", "s-rest")), + "identity must resolve via the registry-side resume_session_id" + ); + } + + /// §9.1 test 8 trust boundary, respawn shape: the claim contradicts the + /// retired server identity → server ref + corrected: true. + #[test] + fn contradicting_claim_is_corrected_on_respawn() { + let f = Fixture::new(); + f.headless("T-tb", Some("cr-tb"), "claude", 1_000); + f.identity + .upsert("T-tb", Some("claude"), Some("s-server"), None, 1); + f.registry.finish_pty_exit("T-tb", 0); + f.identity.retire("T-tb"); + f.probe.set("claude", "s-server", SessionExistence::Present); + + let mut p = pane("cr-tb"); + p.terminal_id = Some("T-tb".to_string()); + p.session_ref = Some(sref("claude", "s-client-guess")); + let v = f.one(p); + assert_eq!(v.verdict, ReconcileVerdict::Respawn); + assert_eq!(v.session_ref, Some(sref("claude", "s-server"))); + assert_eq!(v.corrected, Some(true)); + } + + fn now_ms_for_test() -> i64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_millis() as i64) + .unwrap_or(0) + } +} diff --git a/crates/freshell-ws/src/terminal.rs b/crates/freshell-ws/src/terminal.rs index 7ac3f602c..613e82248 100644 --- a/crates/freshell-ws/src/terminal.rs +++ b/crates/freshell-ws/src/terminal.rs @@ -103,12 +103,14 @@ fn map_shell(shell: Shell) -> ShellType { /// shared broadcast bus) until the socket closes. `socket` has already had the /// connect handshake written by the caller; `bcast_rx` is this connection's /// subscription to the server→client broadcast bus. +#[allow(clippy::too_many_arguments)] pub async fn run( socket: WebSocket, state: &WsState, mut bcast_rx: tokio::sync::broadcast::Receiver, terminal_output_batch_v1: bool, ui_screenshot_v1: bool, + pane_reconcile_v1: bool, origin_kind: &'static str, ) { let (mut ws_tx, mut ws_rx) = socket.split(); @@ -238,6 +240,7 @@ pub async fn run( conn_id, &conn_sink, terminal_output_batch_v1, + pane_reconcile_v1, ) .await { @@ -400,6 +403,7 @@ pub async fn run( /// Parse + dispatch one inbound client text frame. Returns `false` to close the /// connection (only on an unrecoverable send failure). +#[allow(clippy::too_many_arguments)] async fn handle_client_text( text: &str, ws_tx: &mut WsSink, @@ -407,6 +411,7 @@ async fn handle_client_text( conn_id: u64, conn_sink: &FrameSink, terminal_output_batch_v1: bool, + pane_reconcile_v1: bool, ) -> bool { // Accept-and-strip: unknown/unparseable frames are ignored (matches the // runtime's tolerance; the handshake already gated auth). @@ -628,6 +633,17 @@ async fn handle_client_text( // (`crate::now_iso`, the same clock `build_handshake`'s `ready.timestamp` // uses). No correlation id on either side -- the client matches by type, // not by request/response pairing. + ClientMessage::PaneReconcileRequest(request) => { + // Answered ONLY on a connection that negotiated the capability + // (§4.2's "may I send?" gate); anything else is accept-and-strip + // ignored, exactly like an unknown frame — the frozen client's + // byte-inertness does not depend on this, since it never sends + // the request at all (§3). + if pane_reconcile_v1 { + return handle_pane_reconcile(request, ws_tx, state).await; + } + true + } ClientMessage::Ping => { send( ws_tx, @@ -1387,6 +1403,60 @@ pub fn broadcast_sessions_changed(state: &WsState) { /// Send the reference's `sendError` frame for a failed `terminal.create` /// (`ws-handler.ts:2606-2614`): `{ code, message, requestId }`. +/// `pane.reconcile.request` (reconciliation design §5): a PURE READ over the +/// terminal registry × identity registry × disk index — one verdict per +/// presented pane. Safe to receive N times on N sockets (§7); the only error +/// paths are the explicit `RECONCILE_TOO_LARGE` cap and the +/// `RECONCILE_UNAVAILABLE` derivation-failure frame, both carrying the +/// `reconcileId` for correlation. +async fn handle_pane_reconcile( + request: freshell_protocol::PaneReconcileRequest, + ws_tx: &mut WsSink, + state: &WsState, +) -> bool { + if request.panes.len() > crate::reconcile::MAX_RECONCILE_PANES { + return send_create_error( + ws_tx, + ErrorCode::ReconcileTooLarge, + format!( + "pane.reconcile.request presented {} panes (cap {})", + request.panes.len(), + crate::reconcile::MAX_RECONCILE_PANES + ), + &request.reconcile_id, + ) + .await; + } + let deps = crate::reconcile::ReconcileDeps { + registry: &state.registry, + identity: &state.identity, + existence: state.session_existence.as_ref(), + }; + // §8 frame-level failure: a panicking derivation (poisoned lock) must + // surface as an explicit error frame, never silence. + let verdicts = match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + crate::reconcile::derive_verdicts(&deps, &request.panes) + })) { + Ok(verdicts) => verdicts, + Err(_) => { + return send_create_error( + ws_tx, + ErrorCode::ReconcileUnavailable, + "reconcile derivation failed; keep current state and re-send".to_string(), + &request.reconcile_id, + ) + .await; + } + }; + let result = ServerMessage::PaneReconcileResult(freshell_protocol::PaneReconcileResult { + reconcile_id: request.reconcile_id, + boot_id: state.boot_id.as_ref().clone(), + server_instance_id: state.server_instance_id.as_ref().clone(), + verdicts, + }); + send(ws_tx, &result).await +} + async fn send_create_error( ws_tx: &mut WsSink, code: ErrorCode, @@ -2281,6 +2351,7 @@ mod terminals_changed_tests { config_fallback: None, amplifier_locator: None, opencode_locator: None, + session_existence: std::sync::Arc::new(crate::existence::NoIndexProbe::default()), }; (state, rx) } @@ -2483,6 +2554,7 @@ mod terminal_meta_created_tests { config_fallback: None, amplifier_locator: None, opencode_locator: None, + session_existence: std::sync::Arc::new(crate::existence::NoIndexProbe::default()), }; (state, rx) } diff --git a/crates/freshell-ws/tests/codex_managed_launch_e2e.rs b/crates/freshell-ws/tests/codex_managed_launch_e2e.rs index 1a1315a4d..9598ef0a3 100644 --- a/crates/freshell-ws/tests/codex_managed_launch_e2e.rs +++ b/crates/freshell-ws/tests/codex_managed_launch_e2e.rs @@ -150,6 +150,7 @@ async fn spawn_server() -> (String, freshell_terminal::TerminalRegistry) { config_fallback: None, amplifier_locator: None, opencode_locator: None, + session_existence: std::sync::Arc::new(freshell_ws::existence::NoIndexProbe::default()), }; let router = freshell_ws::router(state); diff --git a/crates/freshell-ws/tests/codex_session_ref_resume.rs b/crates/freshell-ws/tests/codex_session_ref_resume.rs index bcaf5d494..a5c77cae0 100644 --- a/crates/freshell-ws/tests/codex_session_ref_resume.rs +++ b/crates/freshell-ws/tests/codex_session_ref_resume.rs @@ -143,6 +143,7 @@ async fn spawn_server() -> (String, freshell_terminal::TerminalRegistry) { config_fallback: None, amplifier_locator: None, opencode_locator: None, + session_existence: std::sync::Arc::new(freshell_ws::existence::NoIndexProbe::default()), }; let router = freshell_ws::router(state); diff --git a/crates/freshell-ws/tests/diag01_lifecycle_events.rs b/crates/freshell-ws/tests/diag01_lifecycle_events.rs index 2bf7e9871..4dc750958 100644 --- a/crates/freshell-ws/tests/diag01_lifecycle_events.rs +++ b/crates/freshell-ws/tests/diag01_lifecycle_events.rs @@ -167,6 +167,7 @@ async fn spawn_server(ping_interval_ms: u64) -> String { config_fallback: None, amplifier_locator: None, opencode_locator: None, + session_existence: std::sync::Arc::new(freshell_ws::existence::NoIndexProbe::default()), }; let router = freshell_ws::router(state); diff --git a/crates/freshell-ws/tests/freshagent_claude_kill_interrupt.rs b/crates/freshell-ws/tests/freshagent_claude_kill_interrupt.rs index 05659ad8b..dfd4348dd 100644 --- a/crates/freshell-ws/tests/freshagent_claude_kill_interrupt.rs +++ b/crates/freshell-ws/tests/freshagent_claude_kill_interrupt.rs @@ -202,6 +202,7 @@ async fn spawn_server() -> String { config_fallback: None, amplifier_locator: None, opencode_locator: None, + session_existence: std::sync::Arc::new(freshell_ws::existence::NoIndexProbe::default()), }; let router = freshell_ws::router(state); diff --git a/crates/freshell-ws/tests/hello_timeout.rs b/crates/freshell-ws/tests/hello_timeout.rs index 58afce8f5..00b5adaad 100644 --- a/crates/freshell-ws/tests/hello_timeout.rs +++ b/crates/freshell-ws/tests/hello_timeout.rs @@ -87,6 +87,7 @@ async fn spawn_server(hello_timeout_ms: u64) -> String { config_fallback: None, amplifier_locator: None, opencode_locator: None, + session_existence: std::sync::Arc::new(freshell_ws::existence::NoIndexProbe::default()), }; let router = freshell_ws::router(state); diff --git a/crates/freshell-ws/tests/keepalive.rs b/crates/freshell-ws/tests/keepalive.rs index 7d676a7cf..6dc46c18d 100644 --- a/crates/freshell-ws/tests/keepalive.rs +++ b/crates/freshell-ws/tests/keepalive.rs @@ -88,6 +88,7 @@ async fn spawn_server( config_fallback: None, amplifier_locator: None, opencode_locator: None, + session_existence: std::sync::Arc::new(freshell_ws::existence::NoIndexProbe::default()), }; let router = freshell_ws::router(state); diff --git a/crates/freshell-ws/tests/max_payload.rs b/crates/freshell-ws/tests/max_payload.rs index aa209b6e2..0f64dc073 100644 --- a/crates/freshell-ws/tests/max_payload.rs +++ b/crates/freshell-ws/tests/max_payload.rs @@ -88,6 +88,7 @@ async fn spawn_server(ws_max_payload_bytes: usize) -> String { config_fallback: None, amplifier_locator: None, opencode_locator: None, + session_existence: std::sync::Arc::new(freshell_ws::existence::NoIndexProbe::default()), }; let router = freshell_ws::router(state); diff --git a/crates/freshell-ws/tests/origin_policy.rs b/crates/freshell-ws/tests/origin_policy.rs index 7555a7f2f..69f4a9626 100644 --- a/crates/freshell-ws/tests/origin_policy.rs +++ b/crates/freshell-ws/tests/origin_policy.rs @@ -78,6 +78,7 @@ async fn spawn_server(allowed_origins: Vec) -> (String, String) { config_fallback: None, amplifier_locator: None, opencode_locator: None, + session_existence: std::sync::Arc::new(freshell_ws::existence::NoIndexProbe::default()), }; let router = freshell_ws::router(state); diff --git a/crates/freshell-ws/tests/safe08_restore_diagnostics.rs b/crates/freshell-ws/tests/safe08_restore_diagnostics.rs index a03fc1eef..aa14ac8f1 100644 --- a/crates/freshell-ws/tests/safe08_restore_diagnostics.rs +++ b/crates/freshell-ws/tests/safe08_restore_diagnostics.rs @@ -174,6 +174,7 @@ async fn spawn_server() -> String { config_fallback: None, amplifier_locator: None, opencode_locator: None, + session_existence: std::sync::Arc::new(freshell_ws::existence::NoIndexProbe::default()), }; let router = freshell_ws::router(state); diff --git a/crates/freshell-ws/tests/session_identity_frames.rs b/crates/freshell-ws/tests/session_identity_frames.rs index 98dce9570..fb15307c7 100644 --- a/crates/freshell-ws/tests/session_identity_frames.rs +++ b/crates/freshell-ws/tests/session_identity_frames.rs @@ -131,6 +131,7 @@ async fn spawn_server() -> (String, freshell_terminal::TerminalRegistry) { config_fallback: None, amplifier_locator: None, opencode_locator: None, + session_existence: std::sync::Arc::new(freshell_ws::existence::NoIndexProbe::default()), }; let router = freshell_ws::router(state); diff --git a/crates/freshell-ws/tests/term09_output_queue.rs b/crates/freshell-ws/tests/term09_output_queue.rs index 4ebd7aa1c..7e9b80309 100644 --- a/crates/freshell-ws/tests/term09_output_queue.rs +++ b/crates/freshell-ws/tests/term09_output_queue.rs @@ -81,6 +81,7 @@ async fn spawn_server(term09: Term09Config) -> String { config_fallback: None, amplifier_locator: None, opencode_locator: None, + session_existence: std::sync::Arc::new(freshell_ws::existence::NoIndexProbe::default()), }; let router = freshell_ws::router(state); From 70fd000c920daed3062a8555e7cf074221719b13 Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 24 Jul 2026 00:16:27 -0700 Subject: [PATCH 5/8] =?UTF-8?q?feat(ws):=20single-flight=20create-dedupe?= =?UTF-8?q?=20+=20=C2=A79.1=20wire-level=20reconcile=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The council's sole blocker-class finding (§0/§5.4): two reconciling browser tabs that both receive 'respawn' for one createRequestId after a restart both fire terminal.create — absent a guard that is two live PTYs on one key, i.e. two JSONL writers on one session file. On paneReconcileV1 connections ONLY, handle_create now consults newest_live_by_create_request_id first and ADOPTS the existing live terminal (emits terminal.created naming the existing terminalId, spawns nothing). The frozen client never negotiates the capability, so its create flow is byte-for-byte unchanged — proven by the frozen_client_create_path_is_unchanged_no_dedupe wire test. Wire tests (tests/pane_reconcile.rs, real axum + tokio-tungstenite on ephemeral ports): §9.1.1 negotiation + unchanged ready, §9.1.3 cardinality/ opacity with hostile paneKeys, §9.1.4 idempotency incl. the Incident-2 create-interrupted-before-created regression converging to attach, §9.1.7 RECONCILE_TOO_LARGE with reconcileId, §9.1.8 trust boundary corrected:true, §9.1.10 adopt-vs-legacy both halves, and non-negotiating-connection inertness. IdentityProbeRow gains cwd so the adopt frame echoes the existing terminal's working directory. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- crates/freshell-terminal/src/registry.rs | 5 + crates/freshell-ws/src/invariants.rs | 1 + crates/freshell-ws/src/terminal.rs | 42 +- crates/freshell-ws/tests/pane_reconcile.rs | 535 +++++++++++++++++++++ 4 files changed, 581 insertions(+), 2 deletions(-) create mode 100644 crates/freshell-ws/tests/pane_reconcile.rs diff --git a/crates/freshell-terminal/src/registry.rs b/crates/freshell-terminal/src/registry.rs index d30d8628a..1a81735ee 100644 --- a/crates/freshell-terminal/src/registry.rs +++ b/crates/freshell-terminal/src/registry.rs @@ -282,6 +282,9 @@ pub struct IdentityProbeRow { /// (e.g. REST-created resumes, whose creates can't reach the WS-owned /// identity registry across the crate boundary). pub resume_session_id: Option, + /// The terminal's working directory — carried so the §5.4 adopt branch can + /// echo the EXISTING terminal's cwd on its `terminal.created` frame. + pub cwd: Option, } /// One terminal's row for the REST terminal directory (`registry.list()` as consumed @@ -1100,6 +1103,7 @@ impl TerminalRegistry { status: s.status, created_at: s.created_at, resume_session_id: s.resume_session_id.clone(), + cwd: s.cwd.clone(), } }) .collect() @@ -1329,6 +1333,7 @@ impl TerminalRegistry { status: s.status, created_at: s.created_at, resume_session_id: s.resume_session_id.clone(), + cwd: s.cwd.clone(), } }) } diff --git a/crates/freshell-ws/src/invariants.rs b/crates/freshell-ws/src/invariants.rs index 63d4b0ba9..ca46b6418 100644 --- a/crates/freshell-ws/src/invariants.rs +++ b/crates/freshell-ws/src/invariants.rs @@ -95,6 +95,7 @@ mod tests { status, created_at, resume_session_id: resume_session_id.map(str::to_string), + cwd: None, } } diff --git a/crates/freshell-ws/src/terminal.rs b/crates/freshell-ws/src/terminal.rs index 613e82248..6aeedbab2 100644 --- a/crates/freshell-ws/src/terminal.rs +++ b/crates/freshell-ws/src/terminal.rs @@ -467,7 +467,9 @@ async fn handle_client_text( true } ClientMessage::ClientDiagnostic(_) => true, - ClientMessage::TerminalCreate(create) => handle_create(create, ws_tx, state).await, + ClientMessage::TerminalCreate(create) => { + handle_create(create, ws_tx, state, pane_reconcile_v1).await + } ClientMessage::TerminalAttach(attach) => { handle_attach(attach, state, conn_id, conn_sink, terminal_output_batch_v1); true @@ -758,7 +760,43 @@ fn codex_create_uses_managed_launch(mode: &str, flag_value: Option<&str>) -> boo /// `terminal.create` — spawn + register the PTY in the shared registry (owned by no /// connection), then reply `terminal.created`. Create does NOT attach; the client /// sends `terminal.attach` next. -async fn handle_create(create: TerminalCreate, ws_tx: &mut WsSink, state: &WsState) -> bool { +async fn handle_create( + create: TerminalCreate, + ws_tx: &mut WsSink, + state: &WsState, + pane_reconcile_v1: bool, +) -> bool { + // Single-flight create-dedupe (reconciliation design §5.4, the council's + // two-tab double-respawn blocker): on `paneReconcileV1` connections ONLY, + // a create whose `createRequestId` already has a live terminal ADOPTS it — + // `terminal.created` names the EXISTING terminal and spawns nothing, so + // two reconciling connections that both received `respawn` for one key + // converge to one PTY (one JSONL writer per session file). The frozen + // client never negotiates the capability, so its create flow is + // byte-for-byte unchanged (§11 fence). + if pane_reconcile_v1 { + if let Some(existing) = state + .registry + .newest_live_by_create_request_id(&create.request_id) + { + tracing::info!( + terminal_id = %existing, + create_request_id = %create.request_id, + "terminal.create.adopted" + ); + let created = ServerMessage::TerminalCreated(TerminalCreated { + created_at: now_ms(), + request_id: create.request_id, + terminal_id: existing.clone(), + clear_codex_durability: None, + cwd: state.registry.probe(&existing).and_then(|row| row.cwd), + restore_error: None, + session_ref: state.identity.session_ref_for(&existing), + }); + return send(ws_tx, &created).await; + } + } + // `terminalId` via UUID (nanoid-alphabet-compatible for the oracle validator); // `streamId` via UUIDv4 (the reference's randomUUID()). let terminal_id = Uuid::new_v4().simple().to_string(); diff --git a/crates/freshell-ws/tests/pane_reconcile.rs b/crates/freshell-ws/tests/pane_reconcile.rs new file mode 100644 index 000000000..25738ac72 --- /dev/null +++ b/crates/freshell-ws/tests/pane_reconcile.rs @@ -0,0 +1,535 @@ +//! Reconciliation-handshake wire tests (design §9.1) — raw-WS +//! (tokio-tungstenite) integration against an in-process axum server, +//! following the `hello_timeout.rs` / `session_identity_frames.rs` harness +//! convention: ephemeral loopback ports, never a fixed one. +//! +//! Covered here (crate-level, wire): +//! * 9.1.1 negotiation — no capability → no `ready.capabilities`, handshake +//! shape unchanged; capability → advertised. +//! * 9.1.3 cardinality + opacity over the wire. +//! * 9.1.4 idempotency — (a) same request twice → identical verdicts; +//! (b) create → disconnect before reading `terminal.created` → reconnect → +//! re-present without `terminalId` → `attach` (row 1, the Incident-2 +//! regression at protocol level). +//! * 9.1.7 limits — 201 panes → `RECONCILE_TOO_LARGE` carrying the +//! `reconcileId`. +//! * 9.1.8 trust boundary — contradicting claim → server ref + corrected. +//! * 9.1.10 single-flight create-dedupe — negotiated connection adopts the +//! existing live terminal for a key; a non-negotiating connection keeps the +//! legacy spawn path. +//! * inertness — a non-negotiating connection's `pane.reconcile.request` is +//! accept-and-strip ignored. + +use std::sync::Arc; +use std::time::Duration; + +use futures_util::{SinkExt, StreamExt}; +use tokio::net::TcpListener; +use tokio_tungstenite::tungstenite::Message as WsMessage; + +use freshell_ws::WsState; + +const AUTH_TOKEN: &str = "s3cr3t-token-abcdef"; + +fn test_settings_value() -> serde_json::Value { + serde_json::json!({ + "ai": {}, + "codingCli": { "enabledProviders": [], "mcpServer": true, "providers": {} }, + "editor": { "externalEditor": "auto" }, + "extensions": { "disabled": [] }, + "freshAgent": { "defaultPlugins": [], "enabled": false, "providers": {} }, + "logging": { "debug": false }, + "network": { "configured": true, "host": "127.0.0.1" }, + "panes": { "defaultNewPane": "ask" }, + "safety": { "autoKillIdleMinutes": 15 }, + "sidebar": { + "autoGenerateTitles": true, + "excludeFirstChatMustStart": false, + "excludeFirstChatSubstrings": [] + }, + "terminal": { "scrollback": 10000 } + }) +} + +struct Server { + url: String, + registry: freshell_terminal::TerminalRegistry, + identity: freshell_ws::identity::TerminalIdentityRegistry, +} + +/// Real axum server on an ephemeral loopback port. Returns handles to the +/// SHARED registry + identity registry so tests can seed generations +/// deterministically (the §9.1 headless convention). +async fn spawn_server() -> Server { + let auth_token = Arc::new(AUTH_TOKEN.to_string()); + let broadcast_tx = Arc::new(tokio::sync::broadcast::channel::(64).0); + let settings = + Arc::new(serde_json::from_value(test_settings_value()).expect("valid settings fixture")); + let registry = freshell_terminal::TerminalRegistry::new(); + let identity = freshell_ws::identity::TerminalIdentityRegistry::new(); + + let state = WsState { + identity: identity.clone(), + auth_token: Arc::clone(&auth_token), + server_instance_id: Arc::new("srv-test".to_string()), + boot_id: Arc::new("boot-test".to_string()), + settings, + broadcast_tx: Arc::clone(&broadcast_tx), + fresh_codex: freshell_freshagent::FreshCodexState::new( + Arc::clone(&auth_token), + Arc::clone(&broadcast_tx), + serde_json::json!({ "freshAgent": { "enabled": false } }), + ), + fresh_claude: freshell_freshagent::FreshClaudeState::new(Arc::clone(&broadcast_tx)), + fresh_opencode: freshell_freshagent::FreshOpencodeState::new( + freshell_freshagent::FreshAgentState::new( + Arc::clone(&auth_token), + Arc::clone(&broadcast_tx), + ), + ), + registry: registry.clone(), + tabs: freshell_ws::tabs::TabsRegistry::new(), + screenshots: freshell_ws::screenshot::ScreenshotBroker::new(Arc::clone(&broadcast_tx)), + terminals_revision: Arc::new(std::sync::atomic::AtomicI64::new(0)), + sessions_revision: Arc::new(std::sync::atomic::AtomicI64::new(0)), + cli_commands: Arc::new(Vec::new()), + shutdown: Arc::new(tokio::sync::Notify::new()), + ping_interval_ms: 30_000, + hello_timeout_ms: 5_000, + allowed_origins: Arc::new(freshell_ws::origin::default_allowed_origins()), + ws_max_payload_bytes: 16 * 1024 * 1024, + term09: freshell_ws::backpressure::Term09Config::default(), + config_fallback: None, + amplifier_locator: None, + opencode_locator: None, + session_existence: std::sync::Arc::new(freshell_ws::existence::NoIndexProbe::default()), + }; + + let router = freshell_ws::router(state); + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("bind ephemeral loopback port"); + let addr = listener.local_addr().expect("local addr"); + tokio::spawn(async move { + let _ = axum::serve(listener, router).await; + }); + + Server { + url: format!("ws://{addr}/ws"), + registry, + identity, + } +} + +type TestWs = + tokio_tungstenite::WebSocketStream>; + +/// Connect + hello (optionally negotiating `paneReconcileV1`), consuming the +/// 4-frame handshake. Returns the socket and the parsed `ready` frame. +async fn connect(url: &str, pane_reconcile_v1: bool) -> (TestWs, serde_json::Value) { + let (mut ws, _resp) = tokio_tungstenite::connect_async(url) + .await + .expect("ws connect"); + let mut hello = serde_json::json!({ + "type": "hello", + "token": AUTH_TOKEN, + "protocolVersion": freshell_protocol::WS_PROTOCOL_VERSION, + }); + if pane_reconcile_v1 { + hello["capabilities"] = serde_json::json!({ "paneReconcileV1": true }); + } + ws.send(WsMessage::Text(hello.to_string())) + .await + .expect("send hello"); + + let mut ready = serde_json::Value::Null; + for _ in 0..4u8 { + let msg = tokio::time::timeout(Duration::from_secs(5), ws.next()) + .await + .expect("handshake message within timeout") + .expect("stream not ended") + .expect("no ws error"); + if let WsMessage::Text(text) = &msg { + let value: serde_json::Value = serde_json::from_str(text).expect("json frame"); + if value["type"] == serde_json::json!("ready") { + ready = value; + } + } + } + assert!(!ready.is_null(), "handshake must contain ready"); + (ws, ready) +} + +/// Read text frames until one with `type == wanted` arrives (bounded). +async fn next_frame_of_type(ws: &mut TestWs, wanted: &str) -> serde_json::Value { + for _ in 0..30u8 { + let msg = tokio::time::timeout(Duration::from_secs(5), ws.next()) + .await + .unwrap_or_else(|_| panic!("timed out waiting for a {wanted} frame")) + .expect("stream not ended") + .expect("no ws error"); + if let WsMessage::Text(text) = &msg { + let value: serde_json::Value = serde_json::from_str(text).expect("json frame"); + if value["type"] == serde_json::json!(wanted) { + return value; + } + } + } + panic!("no {wanted} frame within 30 messages"); +} + +fn reconcile_request(reconcile_id: &str, panes: serde_json::Value) -> WsMessage { + WsMessage::Text( + serde_json::json!({ + "type": "pane.reconcile.request", + "reconcileId": reconcile_id, + "panes": panes, + }) + .to_string(), + ) +} + +fn headless(server: &Server, id: &str, key: Option<&str>, mode: &str, created_at: i64) { + server + .registry + .register_headless(freshell_terminal::registry::HeadlessTerminal { + terminal_id: id.to_string(), + stream_id: format!("S-{id}"), + mode: mode.to_string(), + resume_session_id: None, + create_request_id: key.map(str::to_string), + created_at: Some(created_at), + }); +} + +// --- 9.1.1 negotiation -------------------------------------------------------- + +#[tokio::test] +async fn hello_without_capability_gets_unchanged_ready_and_with_it_gets_advertised() { + let server = spawn_server().await; + + // Frozen-client shape: no capability → `ready` carries NO capabilities + // key at all (byte-level inertness of the advertisement). + let (_ws, ready) = connect(&server.url, false).await; + assert!( + ready.get("capabilities").is_none(), + "non-negotiating ready must not carry capabilities: {ready}" + ); + + let (_ws, ready) = connect(&server.url, true).await; + assert_eq!( + ready["capabilities"], + serde_json::json!({ "paneReconcileV1": true }) + ); +} + +// --- inertness ------------------------------------------------------------------ + +#[tokio::test] +async fn non_negotiating_connection_gets_no_reconcile_response() { + let server = spawn_server().await; + let (mut ws, _ready) = connect(&server.url, false).await; + + ws.send(reconcile_request( + "rec-inert", + serde_json::json!([{ "paneKey": "p", "kind": "terminal", "createRequestId": "cr-x" }]), + )) + .await + .expect("send request"); + // A ping after the request: if the very next frame is the pong, the + // request was accept-and-strip ignored (nothing was sent for it). + ws.send(WsMessage::Text( + serde_json::json!({ "type": "ping" }).to_string(), + )) + .await + .expect("send ping"); + + let frame = next_frame_of_type(&mut ws, "pong").await; + assert_eq!(frame["type"], "pong"); +} + +// --- 9.1.3 cardinality + opacity ------------------------------------------------ + +#[tokio::test] +async fn reconcile_round_trip_preserves_cardinality_order_and_hostile_pane_keys() { + let server = spawn_server().await; + headless(&server, "T-live", Some("cr-live"), "claude", 1_000); + server + .identity + .upsert("T-live", Some("claude"), Some("s-live"), None, 1); + + let (mut ws, _ready) = connect(&server.url, true).await; + let hostile = "tab\"3:\\pane {} 💥"; + ws.send(reconcile_request( + "rec-1", + serde_json::json!([ + { "paneKey": hostile, "kind": "terminal", "mode": "claude", "createRequestId": "cr-live" }, + { "paneKey": "p2", "kind": "terminal", "mode": "shell", "createRequestId": "cr-shell" }, + { "paneKey": "p3" } + ]), + )) + .await + .expect("send request"); + + let result = next_frame_of_type(&mut ws, "pane.reconcile.result").await; + assert_eq!(result["reconcileId"], "rec-1"); + assert_eq!(result["bootId"], "boot-test"); + assert_eq!(result["serverInstanceId"], "srv-test"); + let verdicts = result["verdicts"].as_array().expect("verdicts array"); + assert_eq!(verdicts.len(), 3, "N panes in → N verdicts out"); + assert_eq!(verdicts[0]["paneKey"], hostile); + assert_eq!(verdicts[0]["verdict"], "attach"); + assert_eq!(verdicts[0]["terminalId"], "T-live"); + assert_eq!( + verdicts[0]["sessionRef"], + serde_json::json!({ "provider": "claude", "sessionId": "s-live" }) + ); + assert_eq!(verdicts[1]["paneKey"], "p2"); + assert_eq!(verdicts[1]["verdict"], "fresh"); + assert_eq!(verdicts[2]["paneKey"], "p3"); + assert_eq!(verdicts[2]["verdict"], "invalid"); +} + +// --- 9.1.4 idempotency ----------------------------------------------------------- + +#[tokio::test] +async fn same_request_twice_on_one_socket_returns_identical_verdicts() { + let server = spawn_server().await; + headless(&server, "T-i", Some("cr-i"), "claude", 1_000); + server + .identity + .upsert("T-i", Some("claude"), Some("s-i"), None, 1); + + let (mut ws, _ready) = connect(&server.url, true).await; + let panes = serde_json::json!([ + { "paneKey": "pk", "kind": "terminal", "mode": "claude", "createRequestId": "cr-i" } + ]); + ws.send(reconcile_request("rec-a", panes.clone())) + .await + .expect("send"); + let first = next_frame_of_type(&mut ws, "pane.reconcile.result").await; + ws.send(reconcile_request("rec-a", panes)) + .await + .expect("send again"); + let second = next_frame_of_type(&mut ws, "pane.reconcile.result").await; + assert_eq!(first["verdicts"], second["verdicts"]); +} + +/// 9.1.4(b) — the Incident-2 regression at protocol level: respawn verdict → +/// `terminal.create` → disconnect BEFORE reading `terminal.created` → +/// reconnect → re-present the pane WITHOUT a terminalId → row 1 `attach` to +/// the already-spawned terminal (never a second spawn). +#[tokio::test] +async fn interrupted_create_converges_to_attach_on_the_next_reconcile() { + let server = spawn_server().await; + let (mut ws, _ready) = connect(&server.url, true).await; + + ws.send(WsMessage::Text( + serde_json::json!({ + "type": "terminal.create", + "requestId": "cr-interrupted", + "mode": "shell", + "shell": "system", + }) + .to_string(), + )) + .await + .expect("send create"); + // Disconnect WITHOUT reading terminal.created (the interruption point). + drop(ws); + + // The spawn is discoverable via the write-ahead key stamp — poll the + // SHARED registry until the create lands (bounded). + let mut spawned = None; + for _ in 0..100u8 { + if let Some(id) = server + .registry + .newest_live_by_create_request_id("cr-interrupted") + { + spawned = Some(id); + break; + } + tokio::time::sleep(Duration::from_millis(50)).await; + } + let spawned = spawned.expect("terminal.create must have spawned a keyed terminal"); + + // Reconnect and re-present from persisted state only (no terminalId). + let (mut ws, _ready) = connect(&server.url, true).await; + ws.send(reconcile_request( + "rec-2", + serde_json::json!([ + { "paneKey": "pk", "kind": "terminal", "mode": "shell", "createRequestId": "cr-interrupted" } + ]), + )) + .await + .expect("send request"); + let result = next_frame_of_type(&mut ws, "pane.reconcile.result").await; + assert_eq!(result["verdicts"][0]["verdict"], "attach"); + assert_eq!(result["verdicts"][0]["terminalId"], spawned.as_str()); + + server.registry.kill(&spawned); +} + +// --- 9.1.7 limits ---------------------------------------------------------------- + +#[tokio::test] +async fn over_cap_request_is_answered_with_reconcile_too_large() { + let server = spawn_server().await; + let (mut ws, _ready) = connect(&server.url, true).await; + + let panes: Vec = (0..201) + .map(|i| { + serde_json::json!({ + "paneKey": format!("p{i}"), + "kind": "terminal", + "createRequestId": format!("cr-{i}") + }) + }) + .collect(); + ws.send(reconcile_request("rec-too-big", serde_json::json!(panes))) + .await + .expect("send request"); + + let error = next_frame_of_type(&mut ws, "error").await; + assert_eq!(error["code"], "RECONCILE_TOO_LARGE"); + assert_eq!( + error["requestId"], "rec-too-big", + "the error must carry the reconcileId for correlation" + ); +} + +// --- 9.1.8 trust boundary --------------------------------------------------------- + +#[tokio::test] +async fn contradicting_claim_is_answered_with_server_ref_and_corrected() { + let server = spawn_server().await; + headless(&server, "T-tb", Some("cr-tb"), "claude", 1_000); + server + .identity + .upsert("T-tb", Some("claude"), Some("s-server"), None, 1); + + let (mut ws, _ready) = connect(&server.url, true).await; + ws.send(reconcile_request( + "rec-tb", + serde_json::json!([{ + "paneKey": "pk", + "kind": "terminal", + "mode": "claude", + "createRequestId": "cr-tb", + "terminalId": "T-tb", + "sessionRef": { "provider": "claude", "sessionId": "s-client-guess" } + }]), + )) + .await + .expect("send request"); + + let result = next_frame_of_type(&mut ws, "pane.reconcile.result").await; + let verdict = &result["verdicts"][0]; + assert_eq!(verdict["verdict"], "attach"); + assert_eq!( + verdict["sessionRef"], + serde_json::json!({ "provider": "claude", "sessionId": "s-server" }) + ); + assert_eq!(verdict["corrected"], true); +} + +// --- 9.1.10 single-flight create-dedupe -------------------------------------------- + +/// Change #1 (the council's two-tab double-respawn blocker): on a +/// `paneReconcileV1` connection, a `terminal.create` for a key that already +/// has a live terminal ADOPTS it — `terminal.created` names the EXISTING +/// terminalId and nothing is spawned. Exactly one live PTY per key. +#[tokio::test] +async fn negotiated_create_for_existing_key_adopts_instead_of_spawning() { + let server = spawn_server().await; + let (mut ws1, _ready) = connect(&server.url, true).await; + + ws1.send(WsMessage::Text( + serde_json::json!({ + "type": "terminal.create", + "requestId": "cr-adopt", + "mode": "shell", + "shell": "system", + }) + .to_string(), + )) + .await + .expect("send create 1"); + let created1 = next_frame_of_type(&mut ws1, "terminal.created").await; + let first_id = created1["terminalId"].as_str().expect("id").to_string(); + + // Second reconciling connection (the second browser tab) fires the SAME + // createRequestId — both were told `respawn` for the same key. + let (mut ws2, _ready) = connect(&server.url, true).await; + ws2.send(WsMessage::Text( + serde_json::json!({ + "type": "terminal.create", + "requestId": "cr-adopt", + "mode": "shell", + "shell": "system", + }) + .to_string(), + )) + .await + .expect("send create 2"); + let created2 = next_frame_of_type(&mut ws2, "terminal.created").await; + assert_eq!( + created2["terminalId"].as_str(), + Some(first_id.as_str()), + "the adopt branch must name the EXISTING terminal, not spawn a second" + ); + assert_eq!(created2["requestId"], "cr-adopt"); + + // ≤ 1 live PTY for the key — the data-loss shape stays closed. + assert_eq!( + server.registry.newest_live_by_create_request_id("cr-adopt"), + Some(first_id.clone()) + ); + let inventory = server.registry.inventory(); + let live_for_key = inventory + .iter() + .filter(|t| t.terminal_id == first_id) + .count(); + assert_eq!(live_for_key, 1); + assert_eq!( + inventory.len(), + 1, + "exactly one terminal exists after both creates" + ); + + server.registry.kill(&first_id); +} + +/// The other half of change #1's fence: a NON-negotiating (frozen-client) +/// connection keeps the legacy spawn path byte-for-byte — same requestId, +/// second spawn (today's behavior, untouched). +#[tokio::test] +async fn frozen_client_create_path_is_unchanged_no_dedupe() { + let server = spawn_server().await; + let (mut ws, _ready) = connect(&server.url, false).await; + + let create = serde_json::json!({ + "type": "terminal.create", + "requestId": "cr-legacy", + "mode": "shell", + "shell": "system", + }); + ws.send(WsMessage::Text(create.to_string())) + .await + .expect("send create 1"); + let created1 = next_frame_of_type(&mut ws, "terminal.created").await; + ws.send(WsMessage::Text(create.to_string())) + .await + .expect("send create 2"); + let created2 = next_frame_of_type(&mut ws, "terminal.created").await; + + let id1 = created1["terminalId"].as_str().expect("id1").to_string(); + let id2 = created2["terminalId"].as_str().expect("id2").to_string(); + assert_ne!( + id1, id2, + "the frozen client's blind re-create behavior must be unchanged" + ); + + server.registry.kill(&id1); + server.registry.kill(&id2); +} From c6d6c9d48dd6c9773d693004173181a56eb8c1bd Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 24 Jul 2026 00:22:24 -0700 Subject: [PATCH 6/8] feat(server): index-backed SessionExistence probe over the shared SessionIndex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The real disk-truth probe behind the reconcile derivation (design §5.1): - SessionIndex gains sync, non-blocking peek() + is_fresh() — the published (fresh or stale) snapshot without ever waiting on a sweep - IndexExistenceProbe: unknown provider → Absent never Unknown; cold index → honest Unknown + detached background refresh; stale snapshot answers now but kicks a refresh so re-queries converge — §9.1 test 13 proves a session written to disk AFTER a cold read resolves Present on re-query (no latched stale Absent), against a REAL SessionIndex over a temp claude home - ever_observed is a monotone per-boot observed-set fed from every snapshot read — it survives the session later disappearing from disk, gating dead_session vs fresh(identity_never_observed) - main.rs wires the probe from the same shared index the History surfaces read (session_index construction moved above WsState, no behavior change); NoIndexProbe fallback when no provider home resolves 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- crates/freshell-server/src/existence.rs | 238 ++++++++++++++++++ crates/freshell-server/src/main.rs | 87 ++++--- crates/freshell-server/src/sessions.rs | 1 + .../freshell-sessions/src/directory_index.rs | 18 +- 4 files changed, 304 insertions(+), 40 deletions(-) create mode 100644 crates/freshell-server/src/existence.rs diff --git a/crates/freshell-server/src/existence.rs b/crates/freshell-server/src/existence.rs new file mode 100644 index 000000000..f93321c29 --- /dev/null +++ b/crates/freshell-server/src/existence.rs @@ -0,0 +1,238 @@ +//! The index-backed [`SessionExistenceProbe`] (reconciliation-handshake +//! design §5.1): "does `provider:sessionId` exist on disk?" answered from the +//! SAME shared [`SessionIndex`] the History/session-directory surfaces read. +//! +//! Semantics (the design's defined contract): +//! * unknown provider → `Absent`, **never** `Unknown` (change #4c); +//! * known provider + no published snapshot (cold index) → `Unknown` — and a +//! background `snapshot()` refresh is kicked so a re-query converges; +//! * known provider + published snapshot → `Present`/`Absent` from the +//! snapshot; a STALE snapshot also kicks a background refresh, so a +//! `provider:sessionId` written to disk after a cold read resolves +//! `Present` on re-query — never a latched stale `Absent` (§9.1 test 13). +//! +//! `ever_observed` gates `dead_session` (§5.3 rows 4/4b): every snapshot read +//! feeds a monotone observed-set, so "disk has seen this identity at least +//! once (this boot)" survives the session later disappearing from disk. + +use std::collections::HashSet; +use std::sync::{Arc, Mutex}; + +use freshell_sessions::directory_index::SessionIndex; +use freshell_ws::existence::{SessionExistence, SessionExistenceProbe}; + +/// The disk-indexed providers of `main.rs`'s `SessionIndex` construction — +/// the "known provider" set of the probe contract. +const KNOWN_PROVIDERS: [&str; 4] = ["claude", "codex", "opencode", "amplifier"]; + +pub struct IndexExistenceProbe { + index: Arc, + /// `provider:sessionId` keys ever seen in ANY snapshot this boot. + observed: Mutex>, +} + +impl IndexExistenceProbe { + pub fn new(index: Arc) -> Self { + Self { + index, + observed: Mutex::new(HashSet::new()), + } + } + + /// Kick a detached background refresh (never blocks the caller). No-op + /// outside a tokio runtime — the WS handler always runs inside one. + fn kick_refresh(&self) { + if let Ok(handle) = tokio::runtime::Handle::try_current() { + let index = Arc::clone(&self.index); + handle.spawn(async move { + let _ = index.snapshot().await; + }); + } + } + + fn record_observed(&self, items: &[freshell_sessions::directory_index::IndexedSession]) { + let mut observed = self.observed.lock().expect("observed set lock"); + for item in items { + observed.insert(item.key()); + } + } +} + +impl SessionExistenceProbe for IndexExistenceProbe { + fn exists(&self, provider: &str, session_id: &str) -> SessionExistence { + if !KNOWN_PROVIDERS.contains(&provider) { + return SessionExistence::Absent; + } + // Keep the answer converging: any non-fresh state kicks a detached + // refresh so a re-query (the client's reconnect-and-re-present loop) + // eventually reads current disk truth. + if !self.index.is_fresh() { + self.kick_refresh(); + } + match self.index.peek() { + None => SessionExistence::Unknown, + Some(items) => { + self.record_observed(&items); + let hit = items + .iter() + .any(|s| s.provider == provider && s.session_id == session_id); + if hit { + SessionExistence::Present + } else { + SessionExistence::Absent + } + } + } + } + + fn ever_observed(&self, provider: &str, session_id: &str) -> bool { + self.observed + .lock() + .expect("observed set lock") + .contains(&format!("{provider}:{session_id}")) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use freshell_sessions::directory_index::{ClaudeSource, SessionSource}; + use std::time::Duration; + + fn temp_claude_home(tag: &str) -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!( + "freshell-existence-{tag}-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + std::fs::create_dir_all(dir.join("projects/proj")).expect("mkdir claude home"); + dir + } + + fn write_session(claude_home: &std::path::Path, session_id: &str) { + // Minimal claude transcript that passes the R10b cwd gate: one line + // carrying `cwd` + timestamps; the file stem is the session id. + let line = serde_json::json!({ + "type": "user", + "message": "hello", + "uuid": "msg-1", + "cwd": "/tmp/proj", + "timestamp": "2026-07-22T10:00:00.000Z" + }); + std::fs::write( + claude_home + .join("projects/proj") + .join(format!("{session_id}.jsonl")), + format!("{line}\n"), + ) + .expect("write session fixture"); + } + + fn probe_over(home: &std::path::Path) -> (IndexExistenceProbe, Arc) { + let index = Arc::new(SessionIndex::with_ttl_and_cache_path( + vec![Arc::new(ClaudeSource::new(home.to_path_buf())) as Arc], + Duration::from_millis(50), + None, // no persistent parse-cache — fully isolated temp home + )); + (IndexExistenceProbe::new(Arc::clone(&index)), index) + } + + #[test] + fn unknown_provider_is_absent_never_unknown() { + let home = temp_claude_home("unknown-provider"); + let (probe, _index) = probe_over(&home); + assert_eq!( + probe.exists("not-a-provider", "s1"), + SessionExistence::Absent + ); + let _ = std::fs::remove_dir_all(&home); + } + + #[tokio::test] + async fn cold_index_is_unknown_for_known_provider() { + let home = temp_claude_home("cold"); + let (probe, _index) = probe_over(&home); + // Nothing published yet — honest Unknown, never a guessed Absent. + assert_eq!(probe.exists("claude", "s-cold"), SessionExistence::Unknown); + let _ = std::fs::remove_dir_all(&home); + } + + /// §9.1 test 13 — real-index staleness: a `provider:sessionId` written to + /// disk AFTER a cold read must resolve `Present` on re-query; a stale + /// `Absent` must never latch. + #[tokio::test] + async fn session_written_after_cold_read_resolves_present_on_requery() { + let home = temp_claude_home("staleness"); + let (probe, index) = probe_over(&home); + let session_id = "5f0c2a1e-9b7d-4c3a-8e21-0d9f6b4a7c11"; + + // Cold read: Unknown (kicks a background refresh of the EMPTY home). + assert_eq!( + probe.exists("claude", session_id), + SessionExistence::Unknown + ); + index.warm().await; + // Warmed empty home: honestly Absent. + assert_eq!(probe.exists("claude", session_id), SessionExistence::Absent); + + // The session appears on disk AFTER that Absent answer. + write_session(&home, session_id); + + // Re-query until the stale-kicked refresh publishes it (bounded). + let mut last = SessionExistence::Absent; + for _ in 0..100u8 { + last = probe.exists("claude", session_id); + if last == SessionExistence::Present { + break; + } + tokio::time::sleep(Duration::from_millis(60)).await; + } + assert_eq!( + last, + SessionExistence::Present, + "a re-query must converge to Present — no latched stale Absent" + ); + assert!(probe.ever_observed("claude", session_id)); + let _ = std::fs::remove_dir_all(&home); + } + + /// The observed-set is monotone: once seen on disk, an identity stays + /// `ever_observed` even after its file disappears — exactly what gates + /// `dead_session` vs `fresh(identity_never_observed)`. + #[tokio::test] + async fn ever_observed_survives_the_session_disappearing_from_disk() { + let home = temp_claude_home("observed"); + let (probe, index) = probe_over(&home); + let session_id = "7a1b3c5d-2e4f-4a6b-9c8d-1e2f3a4b5c6d"; + write_session(&home, session_id); + index.warm().await; + assert_eq!( + probe.exists("claude", session_id), + SessionExistence::Present + ); + + std::fs::remove_file( + home.join("projects/proj") + .join(format!("{session_id}.jsonl")), + ) + .expect("delete session file"); + + let mut last = SessionExistence::Present; + for _ in 0..100u8 { + last = probe.exists("claude", session_id); + if last == SessionExistence::Absent { + break; + } + tokio::time::sleep(Duration::from_millis(60)).await; + } + assert_eq!(last, SessionExistence::Absent); + assert!( + probe.ever_observed("claude", session_id), + "the observed-set must remember identities disk has seen" + ); + let _ = std::fs::remove_dir_all(&home); + } +} diff --git a/crates/freshell-server/src/main.rs b/crates/freshell-server/src/main.rs index a2fd9a6a8..1766c0d2f 100644 --- a/crates/freshell-server/src/main.rs +++ b/crates/freshell-server/src/main.rs @@ -19,6 +19,7 @@ mod boot; mod checkpoints; mod diag; +mod existence; mod extensions; mod files; mod instance_id; @@ -348,14 +349,57 @@ async fn main() -> ExitCode { .with_cli_commands(Arc::clone(&cli_commands)) .with_amplifier_locator(amplifier_locator.clone()) .with_opencode_locator(opencode_locator.clone()); + // Batch B: `session_directory` no longer re-walks + re-parses every + // transcript on every request -- it reads a cached, TTL-refreshed + // `SessionIndex`. Batch C adds `CodexSource` (file-based, same shape as + // `ClaudeSource`) and `OpencodeSource` (direct-listed from + // `opencode.db`) alongside claude. `None` home -> no index -> the prior + // empty-page behavior. + // + // FRESHELL_HOME root-alignment fix: provider transcript sources must + // resolve against the REAL home, never the (possibly `FRESHELL_HOME`- + // overridden) isolated config root `home` above -- see + // `session_directory::provider_home` for the full rationale. + // + // Fourth source: `AmplifierSource` (`crates/freshell-sessions/src/amplifier.rs`, + // a faithful port of `server/coding-cli/providers/amplifier.ts`'s + // discovery/parse -- file-based, same shape as `ClaudeSource`/`CodexSource`). + // `amplifier_home` lives in that module (not `session_directory.rs`, whose + // internals are out of scope for this change) but resolves the SAME + // `AMPLIFIER_HOME` env / `/.amplifier` default convention + // `claude_home`/`codex_home` use, against the same `provider_home()` root. + let session_index = session_directory::provider_home().as_ref().map(|h| { + Arc::new(freshell_sessions::directory_index::SessionIndex::new(vec![ + Arc::new(freshell_sessions::directory_index::ClaudeSource::new( + session_directory::claude_home(h), + )) as Arc, + Arc::new(freshell_sessions::directory_index::CodexSource::new( + session_directory::codex_home(h), + )) as Arc, + Arc::new(freshell_sessions::directory_index::OpencodeSource::new( + freshell_sessions::parse::default_opencode_data_home(), + )) as Arc, + Arc::new(freshell_sessions::amplifier::AmplifierSource::new( + freshell_sessions::amplifier::amplifier_home(h), + )) as Arc, + ])) + }); + let ws_state = WsState { identity: terminal_identity.clone(), amplifier_locator: amplifier_locator.clone(), opencode_locator: opencode_locator.clone(), - // Reconciliation handshake disk-truth probe: honest `Unknown` answers - // until the index-backed probe below replaces this (no provider home => - // stays the no-index fallback). - session_existence: std::sync::Arc::new(freshell_ws::existence::NoIndexProbe::default()), + // Reconciliation handshake disk-truth probe (design §5.1): backed by + // the SAME shared session index the History surfaces read; the + // no-index fallback (honest `Unknown` on known providers) when no + // provider home resolves — mirrors `session_index`'s own `Option` + // convention. + session_existence: match &session_index { + Some(index) => std::sync::Arc::new(existence::IndexExistenceProbe::new( + std::sync::Arc::clone(index), + )), + None => std::sync::Arc::new(freshell_ws::existence::NoIndexProbe::default()), + }, auth_token: Arc::clone(&auth_token), // Shared (not moved) so `GET /api/health` reports the SAME `instanceId`. server_instance_id: Arc::clone(&server_instance_id), @@ -429,41 +473,6 @@ async fn main() -> ExitCode { // the coding-CLI sessions from the isolated home's provider transcript dirs, // reusing `freshell-sessions` parsers. Replaces the earlier empty-page stub. // - // Batch B: `session_directory` no longer re-walks + re-parses every - // transcript on every request -- it reads a cached, TTL-refreshed - // `SessionIndex`. Batch C adds `CodexSource` (file-based, same shape as - // `ClaudeSource`) and `OpencodeSource` (direct-listed from - // `opencode.db`) alongside claude. `None` home -> no index -> the prior - // empty-page behavior. - // - // FRESHELL_HOME root-alignment fix: provider transcript sources must - // resolve against the REAL home, never the (possibly `FRESHELL_HOME`- - // overridden) isolated config root `home` above -- see - // `session_directory::provider_home` for the full rationale. - // - // Fourth source: `AmplifierSource` (`crates/freshell-sessions/src/amplifier.rs`, - // a faithful port of `server/coding-cli/providers/amplifier.ts`'s - // discovery/parse -- file-based, same shape as `ClaudeSource`/`CodexSource`). - // `amplifier_home` lives in that module (not `session_directory.rs`, whose - // internals are out of scope for this change) but resolves the SAME - // `AMPLIFIER_HOME` env / `/.amplifier` default convention - // `claude_home`/`codex_home` use, against the same `provider_home()` root. - let session_index = session_directory::provider_home().as_ref().map(|h| { - Arc::new(freshell_sessions::directory_index::SessionIndex::new(vec![ - Arc::new(freshell_sessions::directory_index::ClaudeSource::new( - session_directory::claude_home(h), - )) as Arc, - Arc::new(freshell_sessions::directory_index::CodexSource::new( - session_directory::codex_home(h), - )) as Arc, - Arc::new(freshell_sessions::directory_index::OpencodeSource::new( - freshell_sessions::parse::default_opencode_data_home(), - )) as Arc, - Arc::new(freshell_sessions::amplifier::AmplifierSource::new( - freshell_sessions::amplifier::amplifier_home(h), - )) as Arc, - ])) - }); // Warm the cache in the background so the first real request never pays // the cold full-sweep cost. The scan itself runs in `spawn_blocking` // (inside `SessionIndex::snapshot`), so this never delays serving other diff --git a/crates/freshell-server/src/sessions.rs b/crates/freshell-server/src/sessions.rs index ec4c1462f..27bb29b56 100644 --- a/crates/freshell-server/src/sessions.rs +++ b/crates/freshell-server/src/sessions.rs @@ -421,6 +421,7 @@ mod tests { None, None, None, + None, ) .expect("spawn headless test terminal"); } diff --git a/crates/freshell-sessions/src/directory_index.rs b/crates/freshell-sessions/src/directory_index.rs index d16707219..825e68736 100644 --- a/crates/freshell-sessions/src/directory_index.rs +++ b/crates/freshell-sessions/src/directory_index.rs @@ -741,6 +741,23 @@ impl SessionIndex { } } + /// Sync, non-blocking read of the last PUBLISHED snapshot — fresh or + /// stale — or `None` when the cache is truly cold (nothing ever + /// published). This is the reconciliation handshake's disk-truth read + /// (`freshell-server`'s `SessionExistenceProbe` impl): a sync caller that + /// must never wait on a sweep. `None` is what makes the probe's `Unknown` + /// honest; pair with [`Self::is_fresh`] to decide whether to kick a + /// background `snapshot()` refresh. + pub fn peek(&self) -> Option>> { + self.any_cached() + } + + /// Whether the published snapshot (if any) is within the TTL window — + /// the sync freshness companion to [`Self::peek`]. + pub fn is_fresh(&self) -> bool { + self.fresh_cached().is_some() + } + /// The cached snapshot, if present and within the TTL window. A brief, /// non-async lock: never held across an await point. fn fresh_cached(&self) -> Option>> { @@ -898,7 +915,6 @@ impl SessionIndex { pub async fn warm(&self) { let _ = self.snapshot().await; } - } /// Free-function form of the save-decision gate (see From 9550df9718511f46e8de444f3ed184c6c1e71a9a Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 24 Jul 2026 00:29:53 -0700 Subject: [PATCH 7/8] feat(ws,terminal): keyed-create reservation closes the concurrent-spawn window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The §5.4 adopt branch alone is check-then-spawn: the spawn takes milliseconds and the row only becomes observable at insert, so two truly concurrent creates for one createRequestId could BOTH pass newest_live_by check and both spawn. The registry now carries an in-flight keyed-create reservation (begin/end_keyed_create); a negotiated handle_create claims the key before spawning, and a racing create that finds the key reserved briefly waits and re-checks — adopting the winner's terminal. RAII guard releases on every exit path (spawn error, socket close); bounded fail-open after ~5s with the §5.4 backstop detector still making any residual duplicate loud. Frozen-client path untouched (claim only taken on paneReconcileV1 connections). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- crates/freshell-terminal/src/registry.rs | 51 ++++++++++++++++ crates/freshell-ws/src/terminal.rs | 75 ++++++++++++++++++------ 2 files changed, 107 insertions(+), 19 deletions(-) diff --git a/crates/freshell-terminal/src/registry.rs b/crates/freshell-terminal/src/registry.rs index 1a81735ee..695b66438 100644 --- a/crates/freshell-terminal/src/registry.rs +++ b/crates/freshell-terminal/src/registry.rs @@ -401,6 +401,13 @@ pub struct TerminalRegistry { /// Reconciliation §7.5: consecutive short-lived generations after which /// [`Self::respawn_exhausted`] fires. respawn_generation_cap: Arc, + /// §5.4 single-flight: `createRequestId`s with a keyed create currently + /// in flight (claimed before the spawn, released after the insert). The + /// spawn takes milliseconds and the row only becomes observable at + /// insert, so without this reservation two truly concurrent creates for + /// one key could BOTH pass the `newest_live_by_create_request_id` check + /// and both spawn — the exact duplicate-writer shape the dedupe closes. + keyed_create_inflight: Arc>>, } impl Default for TerminalRegistry { @@ -433,6 +440,7 @@ impl TerminalRegistry { DEFAULT_RESPAWN_LIVENESS_WINDOW_MS, )), respawn_generation_cap: Arc::new(AtomicI64::new(DEFAULT_RESPAWN_GENERATION_CAP)), + keyed_create_inflight: Arc::new(Mutex::new(std::collections::HashSet::new())), } } @@ -1356,6 +1364,26 @@ impl TerminalRegistry { }) } + /// §5.4 single-flight claim: reserve `key` for an in-flight keyed create. + /// `false` means another create currently holds the reservation — the + /// caller should re-check for a live terminal (adopt) instead of + /// spawning. Pair with [`Self::end_keyed_create`]. + pub fn begin_keyed_create(&self, key: &str) -> bool { + self.keyed_create_inflight + .lock() + .expect("keyed-create inflight lock") + .insert(key.to_string()) + } + + /// Release a [`Self::begin_keyed_create`] reservation (success OR failure + /// — the spawn's outcome is discoverable via the registry itself). + pub fn end_keyed_create(&self, key: &str) { + self.keyed_create_inflight + .lock() + .expect("keyed-create inflight lock") + .remove(key); + } + /// §5.4 backstop detector (always-on, capability-independent): if a key /// now has two or more LIVE terminals, make it loud — two live PTYs on one /// `createRequestId` means two JSONL writers on one session file. @@ -2961,6 +2989,29 @@ mod tests { ); } + /// §5.4 single-flight claim: the in-flight keyed-create reservation that + /// closes the check-then-spawn window between two truly concurrent + /// creates for one key (the spawn itself takes milliseconds; the row only + /// becomes observable at insert). + #[test] + fn keyed_create_claim_is_exclusive_until_released() { + let reg = TerminalRegistry::new(); + assert!(reg.begin_keyed_create("cr-claim"), "first claim wins"); + assert!( + !reg.begin_keyed_create("cr-claim"), + "a second concurrent create must NOT also claim the key" + ); + assert!( + reg.begin_keyed_create("cr-other"), + "unrelated keys are free" + ); + reg.end_keyed_create("cr-claim"); + assert!( + reg.begin_keyed_create("cr-claim"), + "a released key is claimable again" + ); + } + #[test] fn exits_without_a_create_request_id_never_count() { let reg = TerminalRegistry::new(); diff --git a/crates/freshell-ws/src/terminal.rs b/crates/freshell-ws/src/terminal.rs index 6aeedbab2..aa8eb9fab 100644 --- a/crates/freshell-ws/src/terminal.rs +++ b/crates/freshell-ws/src/terminal.rs @@ -757,6 +757,21 @@ fn codex_create_uses_managed_launch(mode: &str, flag_value: Option<&str>) -> boo mode == "codex" && freshell_codex::launch_plan::codex_managed_launch_enabled(flag_value) } +/// RAII release of a §5.4 keyed-create reservation +/// ([`freshell_terminal::TerminalRegistry::begin_keyed_create`]): dropped on +/// EVERY exit path of `handle_create`'s spawn — success, spawn error, or the +/// task being cancelled by a socket close. +struct KeyedCreateGuard { + registry: freshell_terminal::TerminalRegistry, + key: String, +} + +impl Drop for KeyedCreateGuard { + fn drop(&mut self) { + self.registry.end_keyed_create(&self.key); + } +} + /// `terminal.create` — spawn + register the PTY in the shared registry (owned by no /// connection), then reply `terminal.created`. Create does NOT attach; the client /// sends `terminal.attach` next. @@ -774,28 +789,50 @@ async fn handle_create( // converge to one PTY (one JSONL writer per session file). The frozen // client never negotiates the capability, so its create flow is // byte-for-byte unchanged (§11 fence). + let mut keyed_create_guard: Option = None; if pane_reconcile_v1 { - if let Some(existing) = state - .registry - .newest_live_by_create_request_id(&create.request_id) - { - tracing::info!( - terminal_id = %existing, - create_request_id = %create.request_id, - "terminal.create.adopted" - ); - let created = ServerMessage::TerminalCreated(TerminalCreated { - created_at: now_ms(), - request_id: create.request_id, - terminal_id: existing.clone(), - clear_codex_durability: None, - cwd: state.registry.probe(&existing).and_then(|row| row.cwd), - restore_error: None, - session_ref: state.identity.session_ref_for(&existing), - }); - return send(ws_tx, &created).await; + // Claim loop: adopt a live terminal if one exists; otherwise RESERVE + // the key before spawning (the spawn takes milliseconds and the row + // only becomes observable at insert, so check-then-spawn alone would + // let two truly concurrent creates both pass the check). A racing + // create that finds the key reserved waits briefly and re-checks — + // it then adopts the winner's terminal. Bounded: after ~5s it + // proceeds to spawn anyway (fail-open; the §5.4 backstop detector + // makes any residual duplicate loud). + for _ in 0..500u16 { + if let Some(existing) = state + .registry + .newest_live_by_create_request_id(&create.request_id) + { + tracing::info!( + terminal_id = %existing, + create_request_id = %create.request_id, + "terminal.create.adopted" + ); + let created = ServerMessage::TerminalCreated(TerminalCreated { + created_at: now_ms(), + request_id: create.request_id, + terminal_id: existing.clone(), + clear_codex_durability: None, + cwd: state.registry.probe(&existing).and_then(|row| row.cwd), + restore_error: None, + session_ref: state.identity.session_ref_for(&existing), + }); + return send(ws_tx, &created).await; + } + if state.registry.begin_keyed_create(&create.request_id) { + keyed_create_guard = Some(KeyedCreateGuard { + registry: state.registry.clone(), + key: create.request_id.clone(), + }); + break; + } + tokio::time::sleep(std::time::Duration::from_millis(10)).await; } } + // Released on EVERY exit path of the spawn below (RAII), success or error; + // the outcome itself is discoverable through the registry. + let _keyed_create_guard = keyed_create_guard; // `terminalId` via UUID (nanoid-alphabet-compatible for the oracle validator); // `streamId` via UUIDv4 (the reference's randomUUID()). From 5de61ec5002e7c71a23ea8764100e142a48f0b75 Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 24 Jul 2026 00:35:03 -0700 Subject: [PATCH 8/8] =?UTF-8?q?test(e2e):=20=C2=A79.2=20synthetic-client?= =?UTF-8?q?=20reconcile=20proof=20against=20the=20real=20Rust=20server?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PW-RUST specs (raw Node WebSocket synthetic client, no SPA — the §9.2 posture) on the HARNESS-01 owned RustServer fixture: 1. Server restart (Incident 1/2 shape, doubling as the WSL-restart equivalence and refreshed-browser analogue): shell pane → fresh, resumed CLI pane with an on-disk fixture claude session → respawn with the correct sessionRef; completing the respawn converges the next reconcile to attach on the spawned terminal. 2. Dead session: a session the index HAS observed disappears from disk → explicit dead_session(session_not_on_disk) carrying the identity; the session directory is untouched (sibling intact, nothing recreated). 3. Two concurrent reconciling connections (change #1 blocker regression): after a restart both tabs reconcile the same createRequestId, both fire terminal.create — exactly one live PTY afterwards via /api/terminals, second create adopted the first's terminalId. All three green against target/release/freshell-server (25.8s). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- .../specs/reconcile-handshake-rust.spec.ts | 332 ++++++++++++++++++ 1 file changed, 332 insertions(+) create mode 100644 test/e2e-browser/specs/reconcile-handshake-rust.spec.ts diff --git a/test/e2e-browser/specs/reconcile-handshake-rust.spec.ts b/test/e2e-browser/specs/reconcile-handshake-rust.spec.ts new file mode 100644 index 000000000..a6805a103 --- /dev/null +++ b/test/e2e-browser/specs/reconcile-handshake-rust.spec.ts @@ -0,0 +1,332 @@ +import fs from 'node:fs/promises' +import path from 'node:path' +import { test, expect } from '@playwright/test' +import WebSocket from 'ws' +import { RustServer } from '../helpers/rust-server.js' +import type { TestServerInfo } from '../helpers/test-server.js' + +/** + * RECONCILE-HANDSHAKE (PW-RUST, design §9.2) — synthetic-client proof of the + * reconciliation-on-connect handshake against the REAL Rust server. + * + * The synthetic client is a raw Node `WebSocket` inside the spec (no SPA + * involvement), driving the real server + a real fixture home directory — + * exactly the §9.2 posture. Scenarios: + * + * 1. Server restart (Incident 1/2 shape, also the WSL-restart equivalence + * and the refreshed-browser analogue: the re-present is built from + * persisted-shape pane claims only): shell pane → `fresh`, resumed-CLI + * pane with an on-disk fixture session → `respawn` with the correct + * `sessionRef`; completing the respawn converges the next reconcile to + * `attach` — exactly one live PTY per createRequestId. + * 2. Dead session: a session the index HAS observed disappears from disk → + * explicit `dead_session`, and the session directory is untouched. + * 3. Two concurrent reconciling connections (change #1 — the council's + * two-tab double-respawn blocker): both fire `terminal.create` for the + * SAME createRequestId after a restart; ≤1 live PTY afterwards and the + * second create took the adopt branch (same terminalId). + * + * Frozen-client inertness at e2e level (§9.2 scenario 6) is the EXISTING + * PW-RUST suite passing unchanged against this server build — not a spec in + * this file. + */ + +const FIXTURE_SESSION_A = '5f0c2a1e-9b7d-4c3a-8e21-0d9f6b4a7c11' +const FIXTURE_SESSION_B = '7a1b3c5d-2e4f-4a6b-9c8d-1e2f3a4b5c6d' + +/** Minimal claude transcript that the session index accepts (carries `cwd`). */ +function claudeSessionLine(): string { + return ( + JSON.stringify({ + type: 'user', + message: 'hello from the reconcile fixture', + uuid: 'msg-1', + cwd: '/tmp/reconcile-proj', + timestamp: '2026-07-22T10:00:00.000Z', + }) + '\n' + ) +} + +async function seedClaudeSessions(homeDir: string, sessionIds: string[]): Promise { + const projDir = path.join(homeDir, '.claude', 'projects', 'reconcile-proj') + await fs.mkdir(projDir, { recursive: true }) + for (const id of sessionIds) { + await fs.writeFile(path.join(projDir, `${id}.jsonl`), claudeSessionLine(), 'utf8') + } +} + +type Frame = Record & { type: string } + +/** Raw synthetic client: connect + hello (negotiating paneReconcileV1). */ +class SyntheticClient { + private ws: WebSocket + private frames: Frame[] = [] + private waiters: Array<{ match: (f: Frame) => boolean; resolve: (f: Frame) => void }> = [] + + private constructor(ws: WebSocket) { + this.ws = ws + ws.on('message', (data) => { + let frame: Frame + try { + frame = JSON.parse(String(data)) as Frame + } catch { + return + } + this.frames.push(frame) + for (let i = this.waiters.length - 1; i >= 0; i--) { + if (this.waiters[i].match(frame)) { + const [waiter] = this.waiters.splice(i, 1) + waiter.resolve(frame) + } + } + }) + } + + static async connect(info: TestServerInfo): Promise { + const ws = new WebSocket(info.wsUrl) + await new Promise((resolve, reject) => { + ws.once('open', () => resolve()) + ws.once('error', reject) + }) + const client = new SyntheticClient(ws) + client.send({ + type: 'hello', + protocolVersion: 7, + token: info.token, + capabilities: { paneReconcileV1: true }, + }) + const ready = await client.waitFor((f) => f.type === 'ready') + expect((ready as { capabilities?: { paneReconcileV1?: boolean } }).capabilities?.paneReconcileV1).toBe(true) + return client + } + + send(frame: Record): void { + this.ws.send(JSON.stringify(frame)) + } + + waitFor(match: (f: Frame) => boolean, timeoutMs = 15_000): Promise { + const seen = this.frames.find(match) + if (seen) return Promise.resolve(seen) + return new Promise((resolve, reject) => { + const timer = setTimeout( + () => reject(new Error(`timed out waiting for frame (have: ${this.frames.map((f) => f.type).join(', ')})`)), + timeoutMs, + ) + this.waiters.push({ + match, + resolve: (f) => { + clearTimeout(timer) + resolve(f) + }, + }) + }) + } + + /** + * One reconcile round-trip. The retry verdict is part of the protocol + * (cold index → `retry(index_warming)`); `reconcileUntilStable` below is + * the client-side re-request loop the design prescribes. + */ + async reconcile(panes: Array>): Promise { + const reconcileId = `rec-${Math.random().toString(36).slice(2)}` + this.send({ type: 'pane.reconcile.request', reconcileId, panes }) + const result = await this.waitFor( + (f) => f.type === 'pane.reconcile.result' && (f as { reconcileId?: string }).reconcileId === reconcileId, + ) + return (result as { verdicts: Frame[] }).verdicts + } + + /** Re-request while any verdict is `retry` (bounded), per §5.3 row 5. */ + async reconcileUntilStable(panes: Array>): Promise { + let verdicts: Frame[] = [] + for (let attempt = 0; attempt < 40; attempt++) { + verdicts = await this.reconcile(panes) + if (!verdicts.some((v) => (v as { verdict?: string }).verdict === 'retry')) return verdicts + await new Promise((r) => setTimeout(r, 250)) + } + return verdicts + } + + async createTerminal(requestId: string): Promise { + this.send({ type: 'terminal.create', requestId, mode: 'shell', shell: 'system' }) + return this.waitFor( + (f) => + (f.type === 'terminal.created' && (f as { requestId?: string }).requestId === requestId) || + (f.type === 'error' && (f as { requestId?: string }).requestId === requestId), + ) + } + + close(): void { + try { + this.ws.close() + } catch { + /* already closed */ + } + } +} + +async function listTerminals(info: TestServerInfo): Promise> { + const res = await fetch(`${info.baseUrl}/api/terminals`, { + headers: { 'x-auth-token': info.token }, + }) + expect(res.ok).toBe(true) + const body = (await res.json()) as { terminals?: Array<{ terminalId: string; status: string }> } + return body.terminals ?? (body as unknown as Array<{ terminalId: string; status: string }>) +} + +test.describe('reconcile handshake (rust server, synthetic client)', () => { + test('restart: shell → fresh, resumed CLI → respawn with correct sessionRef, then attach', async () => { + const server = new RustServer({ + setupHome: async (home) => seedClaudeSessions(home, [FIXTURE_SESSION_A]), + }) + const info = await server.start() + try { + // Pre-restart: a real shell PTY owned by pane cr-shell. + const before = await SyntheticClient.connect(info) + const created = await before.createTerminal('cr-shell') + expect(created.type).toBe('terminal.created') + before.close() + + // The restart: registry emptied, disk intact — the SAME shape as a + // WSL restart (§9.2 scenario 3) and, because the re-present below is + // built from persisted-shape claims only, the refreshed-browser + // analogue (§9.2 scenario 4). + await server.restart() + + const after = await SyntheticClient.connect(info) + const verdicts = await after.reconcileUntilStable([ + // Persisted shell pane: stale terminalId, nothing to resume. + { + paneKey: 'tab1:shell', + kind: 'terminal', + mode: 'shell', + createRequestId: 'cr-shell', + terminalId: String(created.terminalId), + }, + // Persisted CLI pane claiming the on-disk fixture session. + { + paneKey: 'tab1:cli', + kind: 'terminal', + mode: 'claude', + createRequestId: 'cr-cli', + sessionRef: { provider: 'claude', sessionId: FIXTURE_SESSION_A }, + }, + ]) + + expect(verdicts[0].verdict).toBe('fresh') + expect(verdicts[1].verdict).toBe('respawn') + expect(verdicts[1].sessionRef).toEqual({ provider: 'claude', sessionId: FIXTURE_SESSION_A }) + + // Complete the respawn (the create is keyed by the pane's + // createRequestId; shell mode keeps the spec environment-independent — + // identity naming is pinned at crate level). + const respawned = await after.createTerminal('cr-cli') + expect(respawned.type).toBe('terminal.created') + + // Convergence: the next reconcile hits row 1 → attach. + const second = await after.reconcileUntilStable([ + { paneKey: 'tab1:cli', kind: 'terminal', mode: 'claude', createRequestId: 'cr-cli' }, + ]) + expect(second[0].verdict).toBe('attach') + expect(second[0].terminalId).toBe(respawned.terminalId) + after.close() + } finally { + await server.stop() + } + }) + + test('dead session: an observed session gone from disk → explicit dead_session, disk untouched', async () => { + const server = new RustServer({ + setupHome: async (home) => seedClaudeSessions(home, [FIXTURE_SESSION_A, FIXTURE_SESSION_B]), + }) + const info = await server.start() + try { + const client = await SyntheticClient.connect(info) + const pane = { + paneKey: 'tab1:dead', + kind: 'terminal', + mode: 'claude', + createRequestId: 'cr-dead', + sessionRef: { provider: 'claude', sessionId: FIXTURE_SESSION_B }, + } + // First reconcile observes the session on disk. + const first = await client.reconcileUntilStable([pane]) + expect(first[0].verdict).toBe('respawn') + + // The session disappears from disk (external deletion). + const projDir = path.join(info.homeDir, '.claude', 'projects', 'reconcile-proj') + await fs.rm(path.join(projDir, `${FIXTURE_SESSION_B}.jsonl`)) + + // Re-present until the index refresh lands: explicit dead_session, + // never a silent grey pane and never a fresh that hides data loss. + let verdicts: Frame[] = [] + await expect + .poll( + async () => { + verdicts = await client.reconcileUntilStable([pane]) + return verdicts[0].verdict + }, + { timeout: 30_000 }, + ) + .toBe('dead_session') + expect(verdicts[0].reason).toBe('session_not_on_disk') + expect(verdicts[0].sessionRef).toEqual({ provider: 'claude', sessionId: FIXTURE_SESSION_B }) + + // Disk untouched: the sibling session file still exists, nothing was + // recreated or deleted by the server. + const remaining = await fs.readdir(projDir) + expect(remaining).toEqual([`${FIXTURE_SESSION_A}.jsonl`]) + client.close() + } finally { + await server.stop() + } + }) + + test('two concurrent reconciling connections converge to exactly one live PTY per key', async () => { + const server = new RustServer() + const info = await server.start() + try { + // Pre-restart terminal for the key, so the post-restart shape is the + // real incident shape (both tabs re-present a pane that HAD a live + // terminal before the restart). + const before = await SyntheticClient.connect(info) + await before.createTerminal('cr-two') + before.close() + await server.restart() + + // Two browser tabs, both reconciling. + const [tab1, tab2] = await Promise.all([ + SyntheticClient.connect(info), + SyntheticClient.connect(info), + ]) + const pane = { paneKey: 'tab:two', kind: 'terminal', mode: 'shell', createRequestId: 'cr-two' } + const [v1, v2] = await Promise.all([ + tab1.reconcileUntilStable([pane]), + tab2.reconcileUntilStable([pane]), + ]) + // Deterministic pure read: both tabs get the SAME verdict. + expect(v1[0].verdict).toBe(v2[0].verdict) + expect(v1[0].verdict).toBe('fresh') // shell: nothing to resume + + // Both fire terminal.create for the same key — the single-flight + // dedupe must yield ONE PTY, the second create adopting the first. + const [c1, c2] = await Promise.all([ + tab1.createTerminal('cr-two'), + tab2.createTerminal('cr-two'), + ]) + expect(c1.type).toBe('terminal.created') + expect(c2.type).toBe('terminal.created') + expect(c1.terminalId).toBe(c2.terminalId) + + // ≤1 live PTY for the key, asserted via the REST directory (§9.2.7). + const terminals = await listTerminals(info) + const running = terminals.filter((t) => t.status === 'running') + expect(running).toHaveLength(1) + expect(running[0].terminalId).toBe(c1.terminalId) + tab1.close() + tab2.close() + } finally { + await server.stop() + } + }) +})