diff --git a/app/harness/HarnessHost.tsx b/app/harness/HarnessHost.tsx index 23c7d9fe..c887d9dd 100644 --- a/app/harness/HarnessHost.tsx +++ b/app/harness/HarnessHost.tsx @@ -31,6 +31,7 @@ import { type IdSessionRepository, type SessionSummary, } from '../../lib/sessionRepository'; +import { decideDetach } from '../../lib/detachTurn'; import { bootCloudSession, readUrlSessionId, @@ -358,6 +359,34 @@ export default function HarnessHost({ authNav }: { authNav?: ReactNode } = {}) { [writeLocalSession], ); + /** + * Host **detach** seam (plan #789, source #766 — backend-agents slice C). + * + * Closing a viewport (unmount, session switch, New/Clear, logout) is NOT "the + * turn is over" — only a user Stop/Esc cancels. The decision is driven by the + * active session's `meta.turnRunId` via the pure `decideDetach` helper: + * - Durable run (id present, post-E): close this viewport's reader only — + * never abort the run, never a server cancel. At slice C there is no + * separate reader object, so this is the seam that stops consuming the + * socially-owned run without cancelling it (slice F wires the real reader + * detach). + * - Legacy tab-owned turn (no id, the slice-C reality): abort the attached + * fetch so a detached busy tab never leaves the 1800s Function running + * with no persisting writer (the host is the only Blob/envelope writer). + * The abort here is the client AbortController for the /api/agent fetch — the + * dedicated `takePendingCancel` path (Stop/Esc) is unchanged and remains the + * only explicit cancel; no server cancel is ever sent on detach. + */ + const detachTurn = useCallback(() => { + const turnRunId = sessionRef.current?.turnRunId; + const decision = decideDetach(turnRunId); + if (decision.kind === 'abort') { + abortRef.current?.abort(); + } + // `close-reader`: no-op at slice C (no durable Workflow reader exists yet); + // slice E/F consume the run identity carried on the envelope. + }, []); + const runPrompt = useCallback( async (prompt: string, opts?: { pushUser?: boolean }) => { const bridge = bridgeRef.current; @@ -704,7 +733,10 @@ export default function HarnessHost({ authNav }: { authNav?: ReactNode } = {}) { return () => { cancelled = true; canvas.removeEventListener('contextmenu', onCanvasContextMenu); - abortRef.current?.abort(); + // Plan #789 (source #766): unmount detaches, not "the turn is over". Only + // aborts the fetch for a legacy tab-owned turn (no turnRunId) so nothing + // burns unpersisted; a durable run (post-E) closes the reader only. + detachTurn(); if (pollRef.current != null) { clearTimeout(pollRef.current); pollRef.current = null; @@ -725,6 +757,7 @@ export default function HarnessHost({ authNav }: { authNav?: ReactNode } = {}) { setUrlSessionId, applySessionModel, foldPendingModelChange, + detachTurn, ]); /** @@ -796,7 +829,9 @@ export default function HarnessHost({ authNav }: { authNav?: ReactNode } = {}) { const onClear = useCallback(() => { if (inflightRef.current || switchInFlightRef.current) return; - abortRef.current?.abort(); + // Plan #789 (source #766): Clear/New detaches the previous session's turn + // rather than declaring it over (abort only the legacy no-run-id fetch). + detachTurn(); const repo = repoRef.current; const bridge = bridgeRef.current; const clearedId = sessionRef.current.id; @@ -869,7 +904,7 @@ export default function HarnessHost({ authNav }: { authNav?: ReactNode } = {}) { } void refreshSessions(); })(); - }, [writeLocalSession, refreshSessions, setUrlSessionId, resolveNewPersona]); + }, [writeLocalSession, refreshSessions, setUrlSessionId, resolveNewPersona, detachTurn]); const onNewSession = useCallback(() => { const repo = repoRef.current; @@ -877,7 +912,8 @@ export default function HarnessHost({ authNav }: { authNav?: ReactNode } = {}) { // Adversarial #642: ack any stale session-switch pending synchronously at // click before the async repo.create + activateSession. bridgeRef.current?.takePendingSessionSwitch(); - abortRef.current?.abort(); + // Plan #789 (source #766): New detaches (abort only the legacy turn). + detachTurn(); void (async () => { const created = await repo.create(); if (created.action !== 'ok') return; // stay on the current session @@ -891,7 +927,7 @@ export default function HarnessHost({ authNav }: { authNav?: ReactNode } = {}) { setUrlSessionId(created.snapshot.id); repo.put(created.snapshot.id, empty); })(); - }, [activateSession, setUrlSessionId, resolveNewPersona]); + }, [activateSession, setUrlSessionId, resolveNewPersona, detachTurn]); const onSwitchSession = useCallback( (id: string) => { @@ -900,7 +936,9 @@ export default function HarnessHost({ authNav }: { authNav?: ReactNode } = {}) { if (!repo || !repo.enabled || !bridge || inflightRef.current) return; if (switchInFlightRef.current) return; // another switch already in-flight if (id === sessionRef.current.id) return; - abortRef.current?.abort(); + // Plan #789 (source #766): switching away detaches this viewport's turn + // (abort only a legacy no-run-id fetch; a durable run keeps running). + detachTurn(); const sourceId = sessionRef.current.id; // generation token — guard against stale get switchInFlightRef.current = true; void (async () => { @@ -918,7 +956,7 @@ export default function HarnessHost({ authNav }: { authNav?: ReactNode } = {}) { } })(); }, - [activateSession, setUrlSessionId], + [activateSession, setUrlSessionId, detachTurn], ); onSwitchSessionRef.current = onSwitchSession; diff --git a/docs/feature-divide.md b/docs/feature-divide.md index f4a213e4..ef6146f9 100644 --- a/docs/feature-divide.md +++ b/docs/feature-divide.md @@ -43,7 +43,8 @@ optional login chrome). | Image bytes (fetch/decode) | **DOM host** | Browser fetch → RGBA → `inv_image_cache_put`; paint stays Wasm | | Math pixels (TeX raster) | **DOM host** | Host MathJax SVG → RGBA → `inv_math_cache_put`; paint stays Wasm | | **Composer + Send** | **Wasm** | Primary input; dynamic absolute-rect from previous-frame measured height: idle hugs one line (~44 px), grows up to cap (124 px), scrolls internally past 120 px content; glyphs inset 5 px from the field border; Send/Stop icon bottom-pinned (`gravity_y = 1.0`) stays on field baseline at all heights (plan #579) | -| **Stop / cancel turn** | **Wasm** control + **DOM** abort | Canvas **Stop** (icon-only ■, plan #457) while busy → pending cancel (protocol v9); host aborts `AbortController` | +| **Stop / cancel turn** | **Wasm** control + **DOM** abort | Canvas **Stop** (icon-only ■, plan #457) while busy → pending cancel (protocol v9); host aborts `AbortController`. **Only a user Stop/Esc cancels** a turn | +| **Turn lifetime + host detach (plan #789, source #766, backend-agents slice C)** | **DOM host** (§ detach seam); the turn itself stays **server-owned** | A viewport close is **not** "the turn is over": the durable turn lives server-side. **unmount / session switch / New-Clear / logout = detach** — the `detachTurn` seam is wired on all four tear-down sites (`lib/detachTurn.ts` `decideDetach` keys on the session's `meta.turnRunId`). **During an in-flight turn only `unmount` actually fires detach**: switch/New/Clear are gated by the pre-existing **#642** inflight lock and their `detachTurn()` runs only on the idle-viewport teardown (a no-op for a live turn at slice C). E/F lifts or justifies those guards when switch-away must detach a durable run. A **durable run** (`turnRunId` present, post-E) survives the tab — the viewport **closes its reader only, never aborts, never sends a server cancel**; a **legacy tab-owned turn** (no `turnRunId`, today's `/api/agent` fetch) still **aborts** on tear-down so no unpersisted 1800 s Function keeps burning with no writer. Reserved carriers `meta.{turnRunId,turnStatus}` ride the envelope ([session-model.md](session-model.md)); **only Stop/Esc cancels** (slice H wires Stop→Workflow cancel; slice F wires the reader attach) | | Busy / error presentation for turns | **Wasm** | EMBER for errors | | Whole-turn `mm:ss` clock (Busy) | **Wasm** (busy row) fed by the **DOM** host | The host owns the only reliable wall-clock (no WASI clock in Wasm) and ticks it ~1 Hz, pushing the elapsed seconds into the Wasm busy row via protocol **v14** `inv_set_turn_elapsed` (plan #567). The canvas appends `Waiting for model… · 0:42` in-canvas while a turn runs; reset to 0 on Ready/Stop/error so no `0:00` lingers. Composer/Stop stay **Wasm** | | 2×4 busy spinner (plan #574) | **Wasm paint** fed by the **DOM** host | **Wasm** paints a 2×4 WARM rectangle grid left of `Waiting for model…` (clockwise pulse; pure LUT `busy_spinner.zig`, zero I/O/alloc in the frame path). **DOM host** drives the pulse phase on the same Busy ticker at **`HARNESS_BUSY_TICK_HZ` = 10 Hz** (`HarnessBridge.setBusyTick` → additive `inv_set_busy_tick`; the v14 `mm:ss` clock is fed every 10th tick ≈ 1 Hz). **Reduced motion** (read fresh at each busy start): the host skips only the per-tick pulse push, grid static at phase 0 — the `mm:ss` **clock keeps ticking** (no reduced-motion clock regression). Idle/Stop/error clears both to 0. Old host + new Wasm degrades to a static grid (busy_tick stays 0) | diff --git a/docs/session-model.md b/docs/session-model.md index f5d64678..c62b201e 100644 --- a/docs/session-model.md +++ b/docs/session-model.md @@ -89,6 +89,8 @@ The **local** blob uses the opaque client snapshot shape: | `activeSandboxId` | **Optional** server/origin sandbox id (Redis-safe opaque). **Session-owned, server-resolved** (P1/GAP-1, #452 + #330): synced as `meta.activeSandboxId` and sent on every `/api/agent` POST as the resolve **override**. The host folds it into the turn and, on success, applies the server's post-turn effective bind — `agentResult.activeSandboxId` (the `meta_sandbox_switch` target) with fallback to `sandboxId` — back as the authoritative binding (never the pre-turn `sandboxId` clobbering a switch); a hard 403 of the **grant-honesty class** (set-but-unusable: `Sandbox access denied.` / selection-required) clears the stale value so the next turn honestly re-resolves from preference / selection. A 403 `Workspace instance is not running.` (a usable grant whose instance is down / softContinue) is **kept** — never silently re-resolved to another grant | | `selectedModel` | **Optional** selected-model id (non-secret printable-ASCII catalog string, e.g. `provider/model`). **Session-owned** (plan #616 / source #610): synced to the cloud record as `meta.selectedModel` (reserved key), so the pick survives a reload and a device-switch adopt. Restore is **by id** after the model catalog is pushed (additive protocol-**v16** host→Wasm set-by-id export, never index math); a stored id missing from the (revoked/changed) catalog → default first-granted. The host also folds a user **Next** cycle into the snapshot via the **pending-model-change** flag (`inv_has_pending_model_change` / `inv_ack_pending_model_change`) observed by the host poll, so a pick persists without waiting for a turn; submit still reads the **live** Wasm selection (`getSelectedModel()`) — the carrier is never a second source of truth on the POST body. `sanitizeModelId` (≤ `MAX_MODEL_ID_LEN` = 128 bytes, printable ASCII) drops a poisoned value to unset — never brick a record | | `usage` | **Optional** last-completed provider token summary (`UsageSummary`, `source === 'provider'`). Rides reserved `meta.usage` as a JSON string (drop-to-unset on poison / non-provider / oversize — never `INVALID_META`). Restore on pull/adopt paints the context slot; **absent = hide**. Capture is live mid-stream (`usage` SSE events from `finish` parts) and reconciled at stream/JSON `done` (the conclusive replace — absent at `done` clears); abort keeps the prior honest in-memory value until the next persist; New/Clear wipe it | +| `turnRunId` | **Optional** durable turn **run id** (Redis-safe opaque, `^[A-Za-z0-9_-]{1,512}$`). **Session-owned** (plan #789 / source #766, backend-agents slice C): synced to the cloud record as the reserved `meta.turnRunId`. Names **at most one** live run behind the session. **Absent = no durable run / idle** (today's Ready). The host **detach** seam keys on its **presence**: a durable run (id present) is owned server-side and survives the tab — a viewport close only **detaches** (closes the reader), it never aborts; a legacy turn with **no** id is still the tab-owned `/api/agent` fetch and **does** abort on tear-down so nothing burns unpersisted. `sanitizeTurnRunId` (wraps the Redis-safe opaque rule) drops a poisoned id to unset — never a 400. At slice C (backend-agents) nothing populates it yet; slice E ships the Workflow owner that writes it. `getRun(runId)` is the source of truth | +| `turnStatus` | **Optional cached hint** of the turn status (`idle` \| `running` \| `cancelling`). **Hint only** — `getRun(runId)` is the authority when a `turnRunId` is present (the tab can detach and nothing polls it). Rides reserved `meta.turnStatus`; `sanitizeTurnStatus` (exact enum accept, ≤ `TURN_STATUS_MAX_BYTES` = 32) drops a poisoned value to unset, never a 400. Absent = unclear/clear | Storage key: `invincible.harness.session.v1`. @@ -173,7 +175,7 @@ for the open tab). | `createdAt` | Epoch ms at mint/backfill — immutable after create | | `updatedAt` | Epoch ms of last accepted write. **New sessions are seeded `0`** (first host PUT with epoch-now ≥ 0 is idempotent-accept, never a spurious 409) | | Cross-user | Other-user id / nonexistent id → **404** (no existence leak) | -| `meta` | **Schema-typed reserved**: `title`, `legacySnapshotId`, `activeSandboxId`, `logicalCwd`, `personaId`, `personaSnapshot`, `transcriptPointer`, `attachedSkills`, `selectedModel`, `usage` — opaque scalars + serialized size cap; nothing else. **Write contract (all keys):** PUT `meta` is the **full desired set** (replace). `upsertEnvelope` stores `input.meta` as-is (`meta: input.meta ?? {}`). **Absent key = clear** that field. There is no PATCH/merge on the store. Mid-turn server writers (`meta_sandbox_switch`, skill inject) **read-copy-override** the existing envelope meta so a one-key update cannot clear siblings. Host `cloudMetaFor` emits every carrier it knows; it folds `attachedSlugs` on every PUT so a rewrite cannot drop the set (omit would clear). `'[]'` is the empty-set **value** for `attachedSkills`, not a third verb. `personaId` is Redis-safe opaque; `personaSnapshot` is the locked-in persona text (≤ `PERSONA_SNAPSHOT_MAX_BYTES` = 512 KiB) and counts toward the raised whole-`meta` budget (**1 MiB**), so it replays on device switch while a mid-session persona edit never rewrites an in-flight session (injection is active — see [docs/personas.md](personas.md)). `attachedSkills` is a **JSON-encoded string** of skill slugs (≤ 32, dedupe): the server stores **slugs only** and re-resolves bodies every turn. **New session / Clear** mints a fresh session, so `attachedSkills` resets there. `transcriptPointer` is a Redis-safe opaque id of the latest **Blob transcript object**. `selectedModel` is a **non-secret** printable-ASCII model id (≤ 128 bytes); poison is **DROPPED to unset** (`sanitizeModelId`) — never a 400 brick. `usage` is a JSON-encoded last-completed provider `UsageSummary`; poison drops to unset (never 400). GET envelope overlays envelope `meta` as that last desired set: valid values win, **absent/poison clears** the transcript field | +| `meta` | **Schema-typed reserved**: `title`, `legacySnapshotId`, `activeSandboxId`, `logicalCwd`, `personaId`, `personaSnapshot`, `transcriptPointer`, `attachedSkills`, `selectedModel`, `usage`, `turnRunId`, `turnStatus` — opaque scalars + serialized size cap; nothing else. **Write contract (all keys):** PUT `meta` is the **full desired set** (replace). `upsertEnvelope` stores `input.meta` as-is (`meta: input.meta ?? {}`). **Absent key = clear** that field. There is no PATCH/merge on the store. Mid-turn server writers (`meta_sandbox_switch`, skill inject) **read-copy-override** the existing envelope meta so a one-key update cannot clear siblings. Host `cloudMetaFor` emits every carrier it knows; it folds `attachedSlugs` on every PUT so a rewrite cannot drop the set (omit would clear). `'[]'` is the empty-set **value** for `attachedSkills`, not a third verb. `personaId` is Redis-safe opaque; `personaSnapshot` is the locked-in persona text (≤ `PERSONA_SNAPSHOT_MAX_BYTES` = 512 KiB) and counts toward the raised whole-`meta` budget (**1 MiB**), so it replays on device switch while a mid-session persona edit never rewrites an in-flight session (injection is active — see [docs/personas.md](personas.md)). `attachedSkills` is a **JSON-encoded string** of skill slugs (≤ 32, dedupe): the server stores **slugs only** and re-resolves bodies every turn. **New session / Clear** mints a fresh session, so `attachedSkills` resets there. `transcriptPointer` is a Redis-safe opaque id of the latest **Blob transcript object**. `selectedModel` is a **non-secret** printable-ASCII model id (≤ 128 bytes); poison is **DROPPED to unset** (`sanitizeModelId`) — never a 400 brick. `usage` is a JSON-encoded last-completed provider `UsageSummary`; poison drops to unset (never 400). **Plan #789 (source #766, backend-agents slice C):** `turnRunId` is the durable turn run id (Redis-safe opaque, `^[A-Za-z0-9_-]{1,512}$`, ≤ `REDIS_SAFE_OPAQUE_ID_MAX` = 512); `turnStatus` is an **optional cached hint** (`idle`\|`running`\|`cancelling`, ≤ `TURN_STATUS_MAX_BYTES` = 32). Both **DROPPED to unset on poison** via the client-safe predicates (`sanitizeTurnRunId` / `sanitizeTurnStatus`) — never a 400 brick — and absent = clear (no durable run / idle). GET envelope overlays envelope `meta` as that last desired set: valid values win, **absent/poison clears** the transcript field | ### Caps (server + host pre-PUT trim) @@ -188,9 +190,12 @@ for the open tab). Host `trimForCloudPut` folds `cwd` + `activeSandboxId` into `meta.{logicalCwd,activeSandboxId}` and `attachedSlugs` into `meta.attachedSkills`, `selectedModel` into `meta.selectedModel`, -and last-completed `usage` into `meta.usage` (JSON string; **absent = clear**), +last-completed `usage` into `meta.usage` (JSON string; **absent = clear**), and the +durable turn carriers `turnRunId` / `turnStatus` into `meta.{turnRunId,turnStatus}` +(plan #789; emit when set, omit = clear), (shared client-safe predicates; a host-absolute cwd / non-Redis-safe id / -non-printable-ASCII model / non-provider usage is dropped to unset), +non-printable-ASCII model / non-provider usage / poisoned or over-`TURN_STATUS_MAX_BYTES` +turn value is dropped to unset), enforces count/byte/body caps (byte accounting includes `meta`), then PUT. `parseCloudSessionSnapshot` restores those from `meta` on pull/adopt (fail-open: a poisoned value drops to unset / [] for `attachedSkills`, never a sticky 400). diff --git a/lib/detachTurn.test.ts b/lib/detachTurn.test.ts new file mode 100644 index 00000000..10f73662 --- /dev/null +++ b/lib/detachTurn.test.ts @@ -0,0 +1,74 @@ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { decideDetach } from './detachTurn'; + +/** + * Host detach seam (plan #789, source #766 — backend-agents slice C). + * + * Closing a viewport (unmount, session switch, New/Clear, logout) must NOT treat + * "the tab went away" as "the turn is over". The guard must distinguish a + * durable server-owned run (meta.turnRunId present, post-E — close reader only) + * from a legacy tab-owned /api/agent fetch (no turnRunId, the slice-C reality — + * abort so no unpersisted 1800s Function keeps burning with no writer). + * + * Neither path sends a server cancel; this decides only the client-side + * AbortController for the attached fetch. Plan-review Major fix: a naïve + * "detach = never abort" would strand a present-but-unbacked busy tab, so the + * decision keys on `turnRunId` PRESENCE, not merely "no live turn". + */ +describe('decideDetach (detach vs abort guard)', () => { + it('no turnRunId → abort (legacy tab-owned turn must still abort so nothing burns unpersisted)', () => { + expect(decideDetach(undefined)).toEqual({ kind: 'abort', reason: 'legacy-turn' }); + }); + + it('empty turnRunId → abort (guard keys on presence; a present-but-empty is legacy)', () => { + expect(decideDetach('')).toEqual({ kind: 'abort', reason: 'legacy-turn' }); + }); + + it('present turnRunId → close-reader only (durable server-owned run survives the tab)', () => { + expect(decideDetach('wf_run_9ax2k')).toEqual({ + kind: 'close-reader', + reason: 'durable-run', + }); + }); + + it('present turnRunId never aborts and never signals a server cancel on either path', () => { + const run = decideDetach('wf_run_a'); + expect(run.kind).toBe('close-reader'); + // No remote-cancel instruction is ever part of the decision — the seam is a + // viewport-close, not a cancel; only Stop/Esc (takePendingCancel) cancels. + expect(run).not.toHaveProperty('serverCancel'); + }); +}); + +describe('HarnessHost wiring lock — tear-down uses detachTurn (PR #790 review L6)', () => { + it('unmount/switch/New/Clear each call detachTurn() (4 sites); the only abortRef.abort() sites are detachTurn, runPrompt, takePendingCancel', () => { + const src = readFileSync( + resolve(import.meta.dirname, '..', 'app/harness/HarnessHost.tsx'), + 'utf-8', + ); + // 1) The detach seam is wired on ALL FOUR tear-down paths. If anyone reverts + // unmount/switch/New/Clear back to a direct abortRef.abort(), this count + // drops to 3 and the durable-run #710 behavior lives only in the + // (untested) cleanup — decideDetach stays green and the seam vanishes. + const detachSites = src.match(/detachTurn\(\);/g) ?? []; + expect(detachSites).toHaveLength(4); + // Anchor the four sites so a rename of both sides can't false-pass. + expect(src).toContain('// Plan #789 (source #766): unmount detaches'); + expect(src).toContain('// Plan #789 (source #766): Clear/New detaches'); + expect(src).toContain('// Plan #789 (source #766): New detaches'); + expect(src).toContain('// Plan #789 (source #766): switching away detaches'); + // 2) The only remaining abortRef.abort() sites are the detachTurn legacy + // path, the runPrompt controller-replace, and the Stop/Esc + // takePendingCancel poll. A misplaced abort on a durable-run path (or a + // revert of the unmount seam back to abort) bumps this count and fails. + const abortSites = src.match(/abortRef\.current\?\.abort\(\);/g) ?? []; + expect(abortSites).toHaveLength(3); + // The three allowed contexts survive — detachTurn itself, runPrompt's + // controller replace, and the poll's takePendingCancel (Stop/Esc) guard. + expect(src).toContain("if (decision.kind === 'abort') {"); + expect(src).toContain('const controller = new AbortController();'); + expect(src).toContain('if (b.takePendingCancel()) {'); + }); +}); diff --git a/lib/detachTurn.ts b/lib/detachTurn.ts new file mode 100644 index 00000000..53f7f2da --- /dev/null +++ b/lib/detachTurn.ts @@ -0,0 +1,46 @@ +/** + * Host **detach** seam (plan #789, source #766 — backend-agents slice C). + * + * Stop treating the tab as the turn lifetime. Closing a viewport (unmount, + * session switch, New/Clear, logout) is a *viewport* event, NOT "the turn is + * over" — the durable turn is owned server-side and survives the tab. Only a + * user Stop/Esc cancels (`takePendingCancel`; slice H wires that to a server + * Workflow cancel). + * + * This pure decision maps a session's current `meta.turnRunId` to whether + * tear-down must abort or only close this viewport's reader: + * + * - **Durable run (`turnRunId` present, post-E):** the run is Workflow-owned and + * keeps its identity on the envelope for the next attached viewport (slice F). + * Tear-down MUST NOT `abort()` the attached fetch as "the turn is over" — it + * closes this viewport's consumption only and never sends a server cancel. + * - **Legacy tab-owned turn (no `turnRunId`, the slice-C reality):** there is no + * durable owner to detach to — the host is the ONLY Blob/envelope writer and + * it just left, so leaving the 1800s `/api/agent` Function running would burn + * inference with nothing persisted. Tear-down degrades to today's `abort()`. + * + * The plan-review Major fix: a naïve "detach = never abort" would strand a + * present-but-unbacked legacy busy tab. Guarding on `turnRunId` **presence** + * (not merely "no live turn") keeps the two cases distinct. No server cancel is + * ever sent on either path — the abort here is the client AbortController for + * the attached fetch, not a Workflow-cancel request. + */ + +/** What a tear-down path should do with an in-flight turn. */ +export type DetachTurnDecision = + | { kind: 'abort'; reason: 'legacy-turn' } + | { kind: 'close-reader'; reason: 'durable-run' }; + +/** + * Decide how tear-down treats the turn, from the session's `turnRunId`. + * + * Absent → the turn is the tab-owned fetch (slice-C reality) and MUST abort so + * no unpersisted inference keeps running with no writer. + * Present → a durable Workflow-owned run: close-reader only, never abort, never + * a server cancel. + */ +export function decideDetach(turnRunId: string | undefined): DetachTurnDecision { + return turnRunId + ? { kind: 'close-reader', reason: 'durable-run' } + : { kind: 'abort', reason: 'legacy-turn' }; +} diff --git a/lib/sessionCloudCaps.test.ts b/lib/sessionCloudCaps.test.ts index 72847887..0d20bfc9 100644 --- a/lib/sessionCloudCaps.test.ts +++ b/lib/sessionCloudCaps.test.ts @@ -4,7 +4,10 @@ import { describe, expect, it } from 'vitest'; import { MAX_MODEL_ID_LEN, STATUS_SLOT_MAX_BYTES, + TURN_STATUS_MAX_BYTES, sanitizeModelId, + sanitizeTurnRunId, + sanitizeTurnStatus, } from './sessionCloudCaps'; import { MAX_MODEL_ID_LEN as BRIDGE_MAX_MODEL_ID_LEN, MAX_STATUS_SLOT_LEN } from './harnessBridge'; @@ -117,3 +120,61 @@ describe('sanitizeModelId (plan #616 — selected-model carrier predicate + cap) expect(sanitizeModelId('a'.repeat(MAX_MODEL_ID_LEN + 1))).toBeUndefined(); }); }); + +describe('turn carriers (plan #789, source #766 — backend-agents C)', () => { + it('sanitizeTurnRunId keeps a valid Redis-safe opaque run id and trims whitespace', () => { + expect(sanitizeTurnRunId('wf_run_9ax2k')).toBe('wf_run_9ax2k'); + expect(sanitizeTurnRunId(' wf_run_1 ')).toBe('wf_run_1'); // trims + expect(sanitizeTurnRunId('a'.repeat(512))).toBe('a'.repeat(512)); // cap-bound accepted + }); + + it('sanitizeTurnRunId drops poison to undefined (drop-to-unset, never a 400)', () => { + for (const bad of [ + undefined, + 42, + '', + ' ', + 'has space', + 'a:b', + '*', + 'a?b', + 'a/b', + 'a.b', + 'x'.repeat(513), // over REDIS_SAFE_OPAQUE_ID_MAX + ]) { + expect(sanitizeTurnRunId(bad)).toBeUndefined(); + } + // reuses the existing Redis-safe opaque rule — 512-char accepted, 513 rejected + expect(sanitizeTurnRunId('a'.repeat(512))).toBeTruthy(); + expect(sanitizeTurnRunId('a'.repeat(513))).toBeUndefined(); + }); + + it('sanitizeTurnStatus accepts exactly idle | running | cancelling', () => { + expect(sanitizeTurnStatus('idle')).toBe('idle'); + expect(sanitizeTurnStatus('running')).toBe('running'); + expect(sanitizeTurnStatus('cancelling')).toBe('cancelling'); + }); + + it('sanitizeTurnStatus drops anything else to undefined (drop-to-unset)', () => { + for (const bad of [ + undefined, + 42, + '', + 'paused', + 'runningg', + 'IDLE', + 'running ', + 'cancelled', + 'x'.repeat(TURN_STATUS_MAX_BYTES + 1), + ]) { + expect(sanitizeTurnStatus(bad)).toBeUndefined(); + } + }); + + it('TURN_STATUS_MAX_BYTES is a NEW generous cap far below any wire/meta budget', () => { + expect(TURN_STATUS_MAX_BYTES).toBe(32); + // The fixed enum literals are ≤ ~10 bytes; the 32-byte ceiling never constrains them. + expect(TURN_STATUS_MAX_BYTES).toBeLessThan(1024 * 1024); + expect(TURN_STATUS_MAX_BYTES).toBeLessThan(4.5 * 1024 * 1024); + }); +}); diff --git a/lib/sessionCloudCaps.ts b/lib/sessionCloudCaps.ts index 612b4d98..750a0933 100644 --- a/lib/sessionCloudCaps.ts +++ b/lib/sessionCloudCaps.ts @@ -5,6 +5,13 @@ */ import { parseInitialCwd } from './agent/workPath'; +const textEncoder = new TextEncoder(); + +/** UTF-8 byte length of a string (client-safe; no Node `Buffer` in this seam). */ +function utf8ByteLength(s: string): number { + return textEncoder.encode(s).length; +} + /** Align with bridge MAX_MSG_LEN (UTF-8 bytes) — native/harness/src/bridge.zig. */ export const HARNESS_SESSION_MAX_MSG_BYTES = 262_144; @@ -308,6 +315,62 @@ export function sanitizeModelId(value: unknown): string | undefined { return s; } +/** + * Optional `meta.turnStatus` enum (plan #789, source #766 — backend-agents C). + * A **cached hint only**: `getRun(runId)` is the source of truth when + * `meta.turnRunId` is present; this value is never treated as authoritative (the + * tab can detach and nothing polls it). Mirrors `sanitizeTurnRunId`'s + * drop-to-unset discipline so a poisoned value omits instead of sticking. + */ +export type TurnStatus = 'idle' | 'running' | 'cancelling'; + +/** + * Max UTF-8 byte length of the reserved `meta.turnStatus` value (plan #789 + * Caps table). NEW generous cap — the enum literals are ≤ ~10 bytes; 32 stays + * well below any wire budget and keeps the enum future-proof. No existing cap + * raised/lowered (a fixed-enum value rides the existing 1 MiB whole-meta budget + * and the 4.5 MB Function ceiling). + */ +export const TURN_STATUS_MAX_BYTES = 32; + +/** + * Client-safe predicate for the reserved session carrier `meta.turnRunId` + * (plan #789). The Workflow/SDK run id is an opaque Redis-safe token, so this + * wraps the existing `isRedisSafeOpaqueId` rule (`^[A-Za-z0-9_-]{1,512}$` via + * `REDIS_SAFE_OPAQUE_ID_MAX`=512) — one bounded rule, no invented charset, and + * it is client-safe (`lib/sessionCloudCaps.ts`). Trims; poison (non-string, + * empty, non-Redis-safe, oversize) → `undefined` (drop-to-unset — never a 400, + * mirroring `sanitizeModelId` / `meta.selectedModel` plan #616). + */ +export function sanitizeTurnRunId(value: unknown): string | undefined { + if (typeof value !== 'string') return undefined; + const s = value.trim(); + if (!s) return undefined; + if (!isRedisSafeOpaqueId(s)) return undefined; + return s; +} + +/** + * Client-safe predicate for the optional reserved `meta.turnStatus` hint + * (plan #789). Exact accept-set `idle | running | cancelling`; anything else + * (non-string, misspelled, over the `TURN_STATUS_MAX_BYTES` byte cap, poisoned) + * → `undefined`. `getRun` is the authority when a `turnRunId` is present; this + * is a cached hint only. Drop-to-unset, never a 400. + */ +export function sanitizeTurnStatus(value: unknown): TurnStatus | undefined { + if ( + value === 'idle' || + value === 'running' || + value === 'cancelling' + ) { + // The fixed enum literals are far under the byte cap (≤ ~10 bytes); the + // guard is structural so a future longer status can never blow the budget. + if (utf8ByteLength(value) > TURN_STATUS_MAX_BYTES) return undefined; + return value; + } + return undefined; +} + /** * Workspace-relative cwd hygiene shared by server validation and host trim/parse. * Keeps only non-empty workspace-relative strings; drops host-absolute, drive/UNC, diff --git a/lib/sessionRepository.test.ts b/lib/sessionRepository.test.ts index 3e6d3c7a..cdc5d223 100644 --- a/lib/sessionRepository.test.ts +++ b/lib/sessionRepository.test.ts @@ -141,6 +141,33 @@ describe('trimForCloudPut', () => { expect(bare.meta).toBeUndefined(); }); + it('plan #789 — folds turnRunId + turnStatus into meta; drops poison (emit when set, omit = clear)', () => { + const out = trimForCloudPut({ + id: 'sess_a', + updatedAt: 7, + messages: [], + turnRunId: 'wf_run_9ax2k', + turnStatus: 'running', + }); + expect(out.meta).toEqual({ turnRunId: 'wf_run_9ax2k', turnStatus: 'running' }); + expect('turnRunId' in out).toBe(false); // carrier carries in meta, not top-level + expect('turnStatus' in out).toBe(false); + + // Poisoned carriers sanitize to undefined → meta omitted entirely (never a 400). + const bad = trimForCloudPut({ + id: 'sess_b', + updatedAt: 7, + messages: [], + turnRunId: 'a:b', + turnStatus: 'paused' as never, + }); + expect(bad.meta).toBeUndefined(); + + // Omitted → no turn carriers in meta. + const bare = trimForCloudPut({ id: 'sess_c', updatedAt: 7, messages: [] }); + expect(bare.meta).toBeUndefined(); + }); + it('normalizes escaping `..` out of meta so a record can never diverge from the request cwd (review #453 residual)', () => { // A P1-legal-on-record `..` is normalized before it is persisted: it drops out // instead of round-tripping `..` into Redis (request sends `.` on any device). @@ -421,6 +448,32 @@ describe('parseCloudSessionSnapshot', () => { expect(bare?.selectedModel).toBeUndefined(); }); + it('plan #789 — restores turnRunId + turnStatus from meta; drops poison to unset', () => { + const out = parseCloudSessionSnapshot({ + id: 'sess_x', + updatedAt: 1, + messages: [{ id: 'm', role: 'user', text: 't', at: 1 }], + meta: { turnRunId: 'wf_run_9ax2k', turnStatus: 'cancelling' }, + }); + expect(out?.turnRunId).toBe('wf_run_9ax2k'); + expect(out?.turnStatus).toBe('cancelling'); + + // Poisoned / invalid → dropped to unset (never a sticky 400). + const bad = parseCloudSessionSnapshot({ + id: 'sess_x', + updatedAt: 1, + messages: [{ id: 'm', role: 'user', text: 't', at: 1 }], + meta: { turnRunId: 'a:b', turnStatus: 'paused' }, + }); + expect(bad?.turnRunId).toBeUndefined(); + expect(bad?.turnStatus).toBeUndefined(); + + // Omitted meta.turnRunId/turnStatus → fields stay undefined (idle today). + const bare = parseCloudSessionSnapshot({ id: 's', updatedAt: 1, messages: [] }); + expect(bare?.turnRunId).toBeUndefined(); + expect(bare?.turnStatus).toBeUndefined(); + }); + it('restores the sticky attachedSlugs from reserved meta.attachedSkills (fail-closed on poison)', () => { const out = parseCloudSessionSnapshot({ id: 'sess_x', @@ -575,6 +628,36 @@ describe('overlayEnvelopeMeta', () => { expect(cleared.attachedSlugs).toBeUndefined(); expect(cleared.personaId).toBeUndefined(); }); + + it('plan #789 — overlays turnRunId/turnStatus from envelope meta; absent/poison clears', () => { + const transcript: SessionSnapshot = { + id: 's', + updatedAt: 1, + messages: [], + turnRunId: 'wf_run_old', + turnStatus: 'running', + }; + // Envelope wins when it carries valid values. + const over = overlayEnvelopeMeta(transcript, { + turnRunId: 'wf_run_new', + turnStatus: 'cancelling', + }); + expect(over.turnRunId).toBe('wf_run_new'); + expect(over.turnStatus).toBe('cancelling'); + + // Absent envelope keys clear the transcript fields (reserved-meta replace). + const cleared = overlayEnvelopeMeta(transcript, { transcriptPointer: 'tx_1' }); + expect(cleared.turnRunId).toBeUndefined(); + expect(cleared.turnStatus).toBeUndefined(); + + // Poison envelope values clear (drop-to-unset), never sticky. + const poisoned = overlayEnvelopeMeta(transcript, { + turnRunId: 'a:b', + turnStatus: 'paused', + }); + expect(poisoned.turnRunId).toBeUndefined(); + expect(poisoned.turnStatus).toBeUndefined(); + }); }); describe('mergeAdoptedUsage (plan #626 test 5)', () => { diff --git a/lib/sessionRepository.ts b/lib/sessionRepository.ts index ddcbb5dc..1db62639 100644 --- a/lib/sessionRepository.ts +++ b/lib/sessionRepository.ts @@ -20,8 +20,11 @@ import { normalizeSessionCwd, parseAttachedSkills, sanitizeModelId, + sanitizeTurnRunId, + sanitizeTurnStatus, serializeAttachedSkills, } from './sessionCloudCaps'; +import type { TurnStatus } from './sessionCloudCaps'; import type { SessionMessage, SessionRole, SessionSnapshot } from './sessionStore'; import { decodeUsageMetaString, @@ -225,6 +228,8 @@ export function parseCloudSessionSnapshot( let activeSandboxId: string | undefined; let personaId: string | undefined; let selectedModel: string | undefined; + let turnRunId: string | undefined; + let turnStatus: TurnStatus | undefined; if (o.meta !== null && typeof o.meta === 'object' && !Array.isArray(o.meta)) { const meta = o.meta as Record; cwd = normalizeSessionCwd(meta.logicalCwd); @@ -244,6 +249,12 @@ export function parseCloudSessionSnapshot( // session's pick by id. `sanitizeModelId` drops a poisoned / invalid value // to unset (restore falls back to the default first-granted model). selectedModel = sanitizeModelId(meta.selectedModel); + // Plan #789 (source #766, backend-agents C): restore the durable turn run + // id + optional status hint from the reserved `meta.{turnRunId,turnStatus}` + // so a reload / device-switch / adopt rebuilds the session's turn identity. + // Poison drops to unset (never a sticky 400 / never bricks the session). + turnRunId = sanitizeTurnRunId(meta.turnRunId); + turnStatus = sanitizeTurnStatus(meta.turnStatus); } const snapshot: SessionSnapshot = { id: o.id, @@ -254,6 +265,8 @@ export function parseCloudSessionSnapshot( if (activeSandboxId !== undefined) snapshot.activeSandboxId = activeSandboxId; if (personaId !== undefined) snapshot.personaId = personaId; if (selectedModel !== undefined) snapshot.selectedModel = selectedModel; + if (turnRunId !== undefined) snapshot.turnRunId = turnRunId; + if (turnStatus !== undefined) snapshot.turnStatus = turnStatus; // Phase 2 (#517): restore the sticky attached-skill set from the reserved // `meta.attachedSkills` JSON-array string (fail-closed → [] on any malformed / // foreign value; never a sticky poison). `[]` restore means detach-all. @@ -317,6 +330,17 @@ export function overlayEnvelopeMeta( delete out.personaId; } + // Plan #789 (source #766, backend-agents C): overlay the durable turn run id + + // optional status hint from the envelope meta. Same reserved-meta replace + // contract — absent or poison clears the transcript field (a valid envelope + // value always wins). + const turnRunId = sanitizeTurnRunId(envMeta.turnRunId); + if (turnRunId !== undefined) out.turnRunId = turnRunId; + else delete out.turnRunId; + const turnStatus = sanitizeTurnStatus(envMeta.turnStatus); + if (turnStatus !== undefined) out.turnStatus = turnStatus; + else delete out.turnStatus; + return out; } @@ -389,6 +413,20 @@ export type CloudPutBody = { * UsageSummary. Absent = clear (hide the context slot). */ usage?: string; + /** + * Plan #789 (source #766, backend-agents C): the durable turn run id + * (Redis-safe opaque token; slice E populates it). Folded from + * `snapshot.turnRunId` via `sanitizeTurnRunId` (drop-to-unset). Absent = + * clear (no durable run / idle). + */ + turnRunId?: string; + /** + * Plan #789 (source #766): an optional cached hint of the turn status + * (`idle | running | cancelling`). Hint only — `getRun` is the authority + * when a `turnRunId` is present. Folded via `sanitizeTurnStatus`. + * Absent = clear. + */ + turnStatus?: TurnStatus; }; }; @@ -445,12 +483,23 @@ export function cloudMetaFor( if (selectedModel !== undefined) meta.selectedModel = selectedModel; const usage = encodeUsageMetaString(snapshot.usage); if (usage !== undefined) meta.usage = usage; + // Plan #789 (source #766, backend-agents C): the durable turn run id + its + // optional status hint ride the reserved `meta.{turnRunId,turnStatus}` so a + // reload / device-switch / adopt rebuilds the session's turn identity. + // Sanitized via the shared client-safe predicates (drop-to-unset on poison — + // never a sticky 400). Absent = clear. + const turnRunId = sanitizeTurnRunId(snapshot.turnRunId); + if (turnRunId !== undefined) meta.turnRunId = turnRunId; + const turnStatus = sanitizeTurnStatus(snapshot.turnStatus); + if (turnStatus !== undefined) meta.turnStatus = turnStatus; return meta.logicalCwd === undefined && meta.activeSandboxId === undefined && meta.personaId === undefined && meta.attachedSkills === undefined && meta.selectedModel === undefined && - meta.usage === undefined + meta.usage === undefined && + meta.turnRunId === undefined && + meta.turnStatus === undefined ? undefined : meta; } diff --git a/lib/sessionStore.test.ts b/lib/sessionStore.test.ts index 2ae77456..d431cc61 100644 --- a/lib/sessionStore.test.ts +++ b/lib/sessionStore.test.ts @@ -416,3 +416,87 @@ describe('selectedModel local sanitize (plan #616)', () => { }); }); +describe('turn carriers local sanitize (plan #789, source #766)', () => { + function installMemoryLocalStorage() { + const map = new Map(); + const ls = { + getItem: (k: string) => (map.has(k) ? map.get(k)! : null), + setItem: (k: string, v: string) => { + map.set(k, String(v)); + }, + removeItem: (k: string) => { + map.delete(k); + }, + clear: () => { + map.clear(); + }, + }; + vi.stubGlobal('localStorage', ls); + return ls; + } + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('LocalStorage load keeps a valid turnRunId + turnStatus; drops poison to unset', () => { + installMemoryLocalStorage(); + const key = 'test-turn-key'; + // Valid Redis-safe run id + valid enum status round-trip. + localStorage.setItem( + key, + JSON.stringify({ + id: 's', + messages: [], + updatedAt: 1, + turnRunId: 'wf_run_9ax2k', + turnStatus: 'running', + }), + ); + const store = new LocalStorageSessionStore(key); + expect(store.load()?.turnRunId).toBe('wf_run_9ax2k'); + expect(store.load()?.turnStatus).toBe('running'); + + // Poisoned turnRunId → undefined (drop-to-unset), never sticks. + const poisonedIds = ['has space', 'a:b', '*', 'x'.repeat(513), 42]; + for (const bad of poisonedIds) { + localStorage.setItem( + key, + JSON.stringify({ id: 's', messages: [], updatedAt: 1, turnRunId: bad }), + ); + const loaded = store.load(); + expect(loaded).not.toBeNull(); + expect(loaded?.turnRunId).toBeUndefined(); + } + + // Poisoned turnStatus → undefined (drop-to-unset); valid enum values survive. + for (const bad of ['paused', 'runningg', 7]) { + localStorage.setItem( + key, + JSON.stringify({ id: 's', messages: [], updatedAt: 1, turnStatus: bad }), + ); + expect(store.load()?.turnStatus).toBeUndefined(); + } + for (const v of ['idle', 'running', 'cancelling']) { + localStorage.setItem( + key, + JSON.stringify({ id: 's', messages: [], updatedAt: 1, turnStatus: v }), + ); + expect(store.load()?.turnStatus).toBe(v); + } + }); + + it('MemorySessionStore round-trips turn carriers; createEmptySession omits them', () => { + const store = new MemorySessionStore(); + store.save({ + ...createEmptySession('z'), + turnRunId: 'wf_run_a', + turnStatus: 'cancelling', + }); + expect(store.load()?.turnRunId).toBe('wf_run_a'); + expect(store.load()?.turnStatus).toBe('cancelling'); + expect(createEmptySession().turnRunId).toBeUndefined(); + expect(createEmptySession().turnStatus).toBeUndefined(); + }); +}); + diff --git a/lib/sessionStore.ts b/lib/sessionStore.ts index 3b148969..8e5cbf5c 100644 --- a/lib/sessionStore.ts +++ b/lib/sessionStore.ts @@ -75,6 +75,24 @@ export type SessionSnapshot = { * poisoned value never sticks or bricks the session. */ selectedModel?: string; + /** + * Plan #789 (source #766, backend-agents C) — the durable turn **run id** for + * this session (a Redis-safe opaque token; slice E populates it once the + * Workflow turn owner ships). Absent = no durable run / idle (today's Ready). + * At slice C nothing populates it yet; the host `detachTurn` seam uses its + * **presence** to decide abort-vs-close-reader on tear-down (a legacy + * tab-owned turn with no run id must still abort so no unpersisted inference + * burns). Sanitized with `sanitizeTurnRunId` on read (drop-to-unset). + */ + turnRunId?: string; + /** + * Plan #789 (source #766) — an optional cached hint of the turn status + * (`idle | running | cancelling`). **Hint only**: `getRun(runId)` is the + * source of truth when a `turnRunId` is present (the tab can detach and + * nothing polls it). Absent = unclear/clear. Sanitized with `sanitizeTurnStatus` + * on read (drop-to-unset). + */ + turnStatus?: import('./sessionCloudCaps').TurnStatus; }; import { @@ -83,6 +101,8 @@ import { isRedisSafeOpaqueId, sanitizeModelId, sanitizeSessionCwd, + sanitizeTurnRunId, + sanitizeTurnStatus, } from './sessionCloudCaps'; import { sanitizeUsageSummary } from './agent/usageSummary'; export { MAX_MODEL_ID_LEN, isRedisSafeOpaqueId, sanitizeSessionCwd } from './sessionCloudCaps'; @@ -182,6 +202,8 @@ export class LocalStorageSessionStore implements SessionStore { attachedSlugs?: unknown; usage?: unknown; selectedModel?: unknown; + turnRunId?: unknown; + turnStatus?: unknown; }; if (!data || typeof data !== 'object' || !Array.isArray(data.messages)) return null; // Tolerant: keep only safe workspace-relative cwd strings (parent #270 / phase 2), @@ -200,6 +222,8 @@ export class LocalStorageSessionStore implements SessionStore { attachedSlugs: rawAttachedSlugs, usage: rawUsage, selectedModel: rawSelectedModel, + turnRunId: rawTurnRunId, + turnStatus: rawTurnStatus, ...rest } = data; const cwd = sanitizeSessionCwd(rawCwd); @@ -214,6 +238,11 @@ export class LocalStorageSessionStore implements SessionStore { const attachedSlugs = sanitizeAttachedSlugs(rawAttachedSlugs); const usage = sanitizeUsageSummary(rawUsage); const selectedModel = sanitizeModelId(rawSelectedModel); + // Plan #789 (source #766): turn carriers sanitize on load — a poisoned + // `turnRunId`/`turnStatus` drops to unset so they never stick or brick the + // session (mirroring `selectedModel`/`usage`). + const turnRunId = sanitizeTurnRunId(rawTurnRunId); + const turnStatus = sanitizeTurnStatus(rawTurnStatus); const out: SessionSnapshot = { ...rest } as SessionSnapshot; if (cwd !== undefined) out.cwd = cwd; if (activeSandboxId !== undefined) out.activeSandboxId = activeSandboxId; @@ -224,6 +253,10 @@ export class LocalStorageSessionStore implements SessionStore { else delete out.usage; if (selectedModel !== undefined) out.selectedModel = selectedModel; else delete out.selectedModel; + if (turnRunId !== undefined) out.turnRunId = turnRunId; + else delete out.turnRunId; + if (turnStatus !== undefined) out.turnStatus = turnStatus; + else delete out.turnStatus; return out; } catch { return null; diff --git a/lib/sessions/sessionStore.test.ts b/lib/sessions/sessionStore.test.ts index be4ec078..3cfc2c87 100644 --- a/lib/sessions/sessionStore.test.ts +++ b/lib/sessions/sessionStore.test.ts @@ -172,17 +172,27 @@ describe('meta — schema-typed reserved (parent #411 lock)', () => { 'attachedSkills', 'selectedModel', 'usage', + // Plan #789 (source #766, backend-agents C): durable turn carriers. + 'turnRunId', + 'turnStatus', ]); for (const k of RESERVED_META_KEYS) { // `attachedSkills` is a JSON-encoded string; `usage` is a JSON UsageSummary - // string. Use a valid value so the reserved-key acceptance loop passes. + // string. Plan #789 turn carriers use their OWN valid values (a Redis-safe + // run id / a real enum literal) so the reserved-key acceptance loop passes + // and also proves those carriers are accepted (a generic 'x' would drop + // turnStatus to unset). const rawMeta: unknown = { [k]: k === 'attachedSkills' ? '[]' : k === 'usage' ? JSON.stringify({ source: 'provider', prompt: 1, completion: 1, total: 2 }) - : 'x', + : k === 'turnRunId' + ? 'wf_run_123' + : k === 'turnStatus' + ? 'running' + : 'x', }; const res = validateSessionRecord( makeRecord({ meta: rawMeta as HarnessSessionRecord['meta'] }), @@ -257,6 +267,95 @@ describe('meta — schema-typed reserved (parent #411 lock)', () => { expect(res.value.meta.selectedModel).toBeUndefined(); } }); + + it('plan #789 — turn carriers accept valid values and DROP poison to unset (never 400)', () => { + // Valid Redis-safe run id + valid enum status survive; trim applied to id. + const ok = validateSessionRecord( + makeRecord({ + meta: { turnRunId: 'wf_run_9ax2k', turnStatus: 'cancelling' } as unknown as HarnessSessionRecord['meta'], + }), + ); + expect(ok.ok).toBe(true); + if (ok.ok) { + expect(ok.value.meta.turnRunId).toBe('wf_run_9ax2k'); + expect(ok.value.meta.turnStatus).toBe('cancelling'); + } + // Trimming: whitespace around a valid Redis-safe id is accepted (sanitize trims). + const trimmed = validateSessionRecord( + makeRecord({ meta: { turnRunId: ' wf_run_1 ' } as unknown as HarnessSessionRecord['meta'] }), + ); + expect(trimmed.ok).toBe(true); + if (trimmed.ok) expect(trimmed.value.meta.turnRunId).toBe('wf_run_1'); + + // Poisoned turnRunId (non-string, non-Redis-safe chars, empty, oversize) drops to unset. + for (const bad of [ + 'has space', + 'a:b', + '*', + 'x'.repeat(513), + 42 as unknown, + '' as unknown, + undefined as unknown, + ]) { + const res = validateSessionRecord( + makeRecord({ meta: { turnRunId: bad } as unknown as HarnessSessionRecord['meta'] }), + ); + expect(res.ok).toBe(true); // drop-to-unset, not a 400 + if (res.ok) { + expect('turnRunId' in res.value.meta).toBe(false); + expect(res.value.meta.turnRunId).toBeUndefined(); + } + } + + // Poisoned turnStatus (misspelled, non-string, over-length) drops to unset. + for (const bad of ['runningg', 'paused', 7 as unknown, 'x'.repeat(33)]) { + const res = validateSessionRecord( + makeRecord({ meta: { turnStatus: bad } as unknown as HarnessSessionRecord['meta'] }), + ); + expect(res.ok).toBe(true); // drop-to-unset, not a 400 + if (res.ok) { + expect('turnStatus' in res.value.meta).toBe(false); + expect(res.value.meta.turnStatus).toBeUndefined(); + } + } + + // All three enum values are accepted. + for (const v of ['idle', 'running', 'cancelling']) { + const res = validateSessionRecord( + makeRecord({ meta: { turnStatus: v } as unknown as HarnessSessionRecord['meta'] }), + ); + expect(res.ok).toBe(true); + if (res.ok) expect(res.value.meta.turnStatus).toBe(v); + } + + // Unknown keys are STILL rejected (reserved-key contract intact; a poisoned + // carrier never leaks into the STRICT unknown-key 400 path). + expect( + validateSessionRecord( + makeRecord({ meta: { notReserved: 1 } as HarnessSessionRecord['meta'] }), + ).ok, + ).toBe(false); + + // Envelope surface accepts/drops the carriers identically (drop-to-unset). + const envOk = validateSessionEnvelope({ + ...makeRecord(), + meta: { turnRunId: 'wf_run_ok', turnStatus: 'running' }, + }); + expect(envOk.ok).toBe(true); + if (envOk.ok) { + expect(envOk.value.meta.turnRunId).toBe('wf_run_ok'); + expect(envOk.value.meta.turnStatus).toBe('running'); + } + const envBad = validateSessionEnvelope({ + ...makeRecord(), + meta: { turnRunId: 'a:b', turnStatus: 'paused' }, + }); + expect(envBad.ok).toBe(true); + if (envBad.ok) { + expect('turnRunId' in envBad.value.meta).toBe(false); + expect('turnStatus' in envBad.value.meta).toBe(false); + } + }); }); describe('validateMetaFields — P1 session-carrier semantic checks (#452)', () => { diff --git a/lib/sessions/sessionStore.ts b/lib/sessions/sessionStore.ts index fabdce3f..bc86ccb6 100644 --- a/lib/sessions/sessionStore.ts +++ b/lib/sessions/sessionStore.ts @@ -39,6 +39,8 @@ import { isRedisSafeOpaqueId, sanitizeModelId, sanitizeSessionCwd, + sanitizeTurnRunId, + sanitizeTurnStatus, } from '../sessionCloudCaps'; export { isRedisSafeOpaqueId } from '../sessionCloudCaps'; import { decodeUsageMetaString } from '../agent/usageSummary'; @@ -71,6 +73,8 @@ export const RESERVED_META_KEYS = [ 'attachedSkills', 'selectedModel', 'usage', + 'turnRunId', + 'turnStatus', ] as const; export type HarnessSessionMetaKey = (typeof RESERVED_META_KEYS)[number]; @@ -413,6 +417,23 @@ export function validateMeta(value: unknown): SessionStoreResult