Uh oh!
There was an error while loading. Please reload this page.
fix(runtime): reload completed sessions after sandbox boundary decisions - #1609
Merged
Merged
Conversation
A completed session containing a sandbox boundary request and decision could not be reopened. Both are control-only state deltas, the legacy read-model projection claimed neither shape, and the resulting hard `unsupported_event` diagnostics made RuntimeReadModel reject the whole projection — which fails every getSessionView caller, not just the transcript: branching, revising and any turn-scoped action go with it. Claim both, and downgrade AiSdkFlow's unmapped-SessionEvent guard to a soft `unmapped_session_event` diagnostic so one unknown event stays observable without making a session unreadable. A projection-coverage contract keyed on BackendSessionEvent['type'] now fails to compile when a new variant has no sample, and fails at runtime when the projection does not claim one. Fixes#1607
Claiming the key alone let any shape ride in under a control-fact name. Only a well-formed request or decision is a canonical fact; anything else stays an `unsupported_event`, matching how isPlanProposalStateDelta guards its own claim. Drop the `unmapped_session_event` downgrade. Turning an unknown event from a hard failure into a soft diagnostic is a policy decision about projection failure handling, not part of reloading a session after a boundary decision, and it would hide a future projection gap. The compile-time coverage contract already catches a new SessionEvent variant before it can reach a ledger. Cover deny alongside allow in the durable round trip, and use real boundary payloads in the projection tests.
This was referenced Jul 29, 2026
…ject A partial claim was the worst of both: it paid the cost of rejecting a malformed ledger while still admitting one. A boundary fact now has to match what AiSdkFlow actually emits — every field, the system/user identity, and the tool-call reference — with the expansion checked by the authoritative validator rather than a local guess. Eight table-driven counter-examples cover the fields, the identity and the reference. The coverage table only proved its keys existed. `provider_retry: []` compiled and passed, and the `error` entry had drifted to testing `complete`. Each entry now holds a `subject` typed to its own key, so neither is expressible, and companions moved to explicit before/after. That let `error` hold a real error event again: the contract filters mapped events through AgentRun's own ledger-admission rule instead of restating it, so it now says what it means — every event a reader can meet has to project. Reported by Codex review of #1609.
Uh oh!
There was an error while loading. Please reload this page.
Astro-Han added a commit
that referenced
this pull request
Jul 29, 2026
…#1618) * refactor(runtime): give read-model diagnostics one severity authority RuntimeReadModel decided which projection diagnostics are fatal by restating their codes, so the projection declared the diagnostics and its caller declared what they mean. Move that decision next to the codes as a table keyed by `RuntimeEventReadModelDiagnosticCode`: a new diagnostic cannot compile without saying whether it means a user-visible row may be missing. The caller and the persisted-compat test now ask that authority instead of listing codes. No behavior change — the table restates today's hard set exactly. * fix(runtime): isolate an unclaimed control fact from the session view One RuntimeEvent the projection did not claim made an entire session unreadable. The catch-all emitted a hard `unsupported_event`, RuntimeReadModel threw on it, and the whole projection went with it — so getMessages, listTurns, branching, revising and every turn-scoped action failed over a fact that owns no chat row. #1607 was one instance; #1609 claimed those two shapes but left the amplification in place. Split the catch-all on the RuntimeEvent's own structure: `content` is its message payload, `actions` its control intent. Every row this projection emits from an unclaimed shape would have come from content, so a content-bearing event stays hard — "a message is never silently dropped" is the invariant the hard failure exists for. A control-only fact has nothing to lose, so it becomes `unclaimed_control_fact` and degrades the view instead of discarding it. A projector that tried to build a row and failed still reports its own hard diagnostic, so this softens nothing that attempted a message. A future gap is still caught before a user meets it. The projection-coverage contract now asserts on the unclaimed codes at either severity rather than the hard one alone, so a new SessionEvent variant with no claim still fails CI, and AiSdkFlow's exhaustiveness guard is what a variant becomes: a content-free control fact that lands on the degradable side by construction. Fixes#1613 * fix(runtime): claim every action field the read-model projection can meet The soft path rests on a premise that was not machine-checked: an unclaimed content-free event degrades the view instead of withholding it, which is only safe while no unclaimed action can owe a row. `content === undefined` does not prove that on its own — permissionDecision, tokenUsage and the terminal fact all produce rows, and runtime-event-backfill already writes a content-free event that becomes a visible `permission_decision`. What actually holds the rule up is claim coverage, so make coverage the thing that is proven. The SessionEvent contract only covers events built by `mapSessionEventToRuntimeEvent`; tool-runtime, terminal-run-commit and the backfill write RuntimeEvents directly, so a new action field on those paths was invisible to it. A second contract keyed on `RuntimeEventActions` gives every field a reachable sample typed to its own key: a new field cannot compile without one and cannot pass without being claimed. Writing it found three fields the projection never claimed — `artifactDelta`, `transferToAgent` and `runtimeProtocol`, the last of which real emitters already write. All three are control-only, so claim them, and say in the fallback what the rule actually depends on. * test(runtime): lock the unclaimed predicate and the caller's hard policy Two regressions the suite could not see. The unmapped-SessionEvent test compared the raw code string, so dropping `unclaimed_control_fact` from `isUnclaimedRuntimeEventDiagnostic` would have quietly narrowed the coverage contract to `unsupported_event` with every test still green; it now filters through the predicate itself. And the hard side was only asserted inside the projector, so a caller that stopped enforcing the policy went unnoticed: append a content-bearing unclaimed event to a completed run's ledger and getSessionView must still refuse the view — the counterpart of the soft reproduction beside it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A completed session that contains a sandbox boundary request and decision could not be reopened.
AiSdkFlowmaps both to control-onlystateDeltaRuntimeEvents (ai-sdk-flow.ts:303-337) — no content, no recognised action — andprojectRuntimeEventsToStoredMessagesclaimed neither shape, so each produced a hardunsupported_eventdiagnostic andRuntimeReadModelrejected the entire projection.The blast radius is wider than the transcript.
getSessionViewis the sole authority behindgetMessages,listTurns,branchFromTurn,branchBeforeTurn,reviseBeforeTurn,requireTurnForActionandrequireUserMessageForTurn, so an affected session could not be read, branched, revised, or acted on at all.The projection now claims a well-formed boundary request or decision and produces no message row for it. The claim matches exactly what
AiSdkFlowemits — every field, the system/user identity, and the tool-call reference, with the expansion checked byvalidateSandboxBoundaryExpansionrather than a local guess. Anything short of that stays anunsupported_event: a partial claim would be the worst of both, paying the cost of rejecting a malformed ledger while still admitting one. Sandbox enforcement, settlement, persistence and the boundary schema are untouched.Beyond the fix itself,
stateDeltaisRecord<string, unknown>while the projection is a closed whitelist, so nothing forced the two to stay in sync — the actual reason #1581 could introduce this. A projection-coverage contract keyed onBackendSessionEvent['type']now fails to compile when a variant has no sample, and fails at runtime when the projection does not claim one.Fixes#1607
Verification
packages/runtimefull suite: 2767 passed, 0 failed, 9 skipped.getMessagesround trip) turn red together.subject, an emptysubject, and asubjectborrowed from another variant each fail compilation (TS2741 / TS2322). An earlier revision only caught the first of those.npm run format,npm run lint,npm run build,npm run typecheckacross all workspaces: clean.Root cause
Open-ended write side (
stateDelta: Record<string, unknown>), closed whitelist read side, and a hard failure when they disagree. #1581 replaced the whole permission vocabulary across 30+ commits without touching the projection, and nothing caught it.The failure was also invisible until after the turn ended: an active run is served from the in-flight projection cache (
runtime-read-model.ts:98-139) and never re-projects the durable ledger, so approving the boundary, running the tool and rendering the turn all behaved correctly. Only reopening the session hit the ledger path.Review focus
Two deliberate scope decisions.
No Desktop E2E. The issue suggested one. The
sandbox-boundaryfixture injectsE2eFixtureStatedirectly and never reachesRuntimeEventStoreorRuntimeReadModel, and the fake backend cannot emit boundary events, so an E2E along that path would re-test the existing UI takeover assertions (sandbox-boundary-takeover.spec.ts) rather than the regression. The durableSessionManager.getMessagesround trip covers it directly, for allow and deny. Making the E2E meaningful would mean building boundary-emission into the fake backend — worth doing on its own terms, not as a smokescreen here.Non-terminal
errorcontent left as is. The coverage contract surfaced it as a third candidate, butAgentRundeliberately keeps non-terminal error RuntimeEvents out of the ledger (agent-run.ts:487), verified against a realSessionManagerround trip whose ledger holds only the user text and one terminal failed event. The projection's hard diagnostic there is the matching defensive assertion, so the contract sample reflects the shape a reader actually finds.Three related gaps found during the analysis are not addressed here and are tracked separately:
AiSdkFlow's exhaustiveness guard (ai-sdk-flow.ts:507-521) emits a control-onlystateDeltafor any unknownSessionEvent, which the projection rejects as hard — so that "safe" fallback escalates one ignorable event into an unreadable session. Whether it should degrade is a policy decision about projection failure handling, not part of this fix. The compile-time coverage contract already catches a new variant well before it can reach a ledger.backfillMissingRuntimeEventsonly fires on an empty ledger, so an intact 10-message session is discarded wholesale over one unclaimed event. Relaxing this trades against its purpose — preventing silently dropped messages — and deserves a separate decision. Same underlying question as (1).RuntimeKernel.sandboxBoundaryRequestOwnersis an in-memory map bound to a backend generation (runtime-kernel.ts:1911), andactivePermissionOverlayEventcovers only the permission actions, so the response authority is gone once the host restarts. Recovery then settles every persisted pending request asdenywithclosureReason: 'host_restarted'(session-manager.ts:915-918) and the turn is marked failed. That is correctly fail-closed, but from the user's side the prompt vanishes and the task is interrupted with no way to resume it.