feat(runtime-host): write Read images to Session context - #4184
feat(runtime-host): write Read images to Session context#4184likun666661 wants to merge 1 commit into
Conversation
91817d6 to
407a687
Compare
15e4434 to
a003af7
Compare
a003af7 to
a6b6e8d
Compare
| { | ||
| artifacts: this.#artifacts, | ||
| taskLedger: this.#taskLedger, | ||
| ...(this.options.contextOffload ? { contextOffload: this.options.contextOffload } : {}), |
There was a problem hiding this comment.
[P1] Keep the preparing copy durable when context cleanup is unavailable
If copyReferences commits and the Host crashes before publication, a restart can reach recover() with contextOffload omitted because the Store failed to open. The discard path then purges the remaining sidecars and deletes the stable Session metadata without retiring the context refs or recording pending cleanup.
Those refs become unreachable, continue consuming quota, and keep their blobs live indefinitely. Please inject the same unavailable-retirement sentinel used by HostSessionRetirementCoordinator so the discard remains pending, or durably enqueue context cleanup before deleting the metadata.
| ) | ||
| .map(({ descriptor, serializedResult }) => [descriptor.artifactId, serializedResult]), | ||
| ); | ||
| const sourceContextRefIds = collectConversationCopySessionContextRefIds({ |
There was a problem hiding this comment.
[P1] Include archived image refs in the context copy
sourceContextRefIds scans messages and RuntimeEvents, but an archived Read result exposes only its placeholder there; the actual session_context ref is inside archivePreflight.serializedResult. archivedToolResultContainsConversationOwnedReferences also still recognizes only session_file images.
The archive can therefore be copied unchanged and the target committed with the source sessionId. Expanding it in the target then returns session_mismatch, and retiring the source makes the result permanently unavailable. Please collect and copy refs from archived image results and rewrite them before publication, or reject this copy shape.
| for (const item of value) visit(item); | ||
| return; | ||
| } | ||
| for (const item of Object.values(value)) visit(item); |
There was a problem hiding this comment.
[P2] Do not interpret opaque tool payloads as storage refs
This structural walk visits complete tool args and JSON results. A valid opaque object that merely has the exact shape { kind: 'session_context', sessionId, refId } is therefore copied even though rewriteToolResultContent intentionally leaves opaque JSON unchanged.
A nonexistent refId makes the whole branch or revision fail with not_found; an existing one adds an unused target ref while the payload still points to the source. Please collect only the canonical attachment and result fields that rewriteStorageRef will actually rewrite.
| readonly bytes: Uint8Array; | ||
| readonly mimeType: string; | ||
| }) => | ||
| createReadImageSnapshotStore(openedContextOffloadStore, input.sessionId).snapshot({ |
There was a problem hiding this comment.
[P2] Release snapshots after known result-commit failures
After snapshot() succeeds, persistence of the tool-result RuntimeEvent can still fail. This production wiring exposes only the put path, and the context releaseReference authority is not used anywhere in the result-commit path.
An observed failure therefore leaves a live-Session orphan consuming logical quota until retirement. A hard-crash orphan is unavoidable, but known commit failures should best-effort release the returned ref, for example through a compensation handle around result publication.
me2seeks
left a comment
me2seeks
left a comment
There was a problem hiding this comment.
Requesting changes for four product-reachable ownership/lifecycle boundaries:
- Snapshot ownership must use a Runtime-owned durable operation identity, not a provider-scoped toolCallId.
- Revision/branch collection must follow the selected copy projection and must not retain refs from excluded later turns.
- A known T2/result-commit failure needs best-effort snapshot compensation (existing inline thread).
- Preparing-copy recovery must retain a durable cleanup anchor while the context store is unavailable (existing inline thread).
I am treating archived-result handling under the current pruning thresholds, inactive backup/export integration, and physical GC scheduling as follow-ups rather than blockers for this PR.
| sessionId, | ||
| turnId: ctx.turnId, | ||
| name: basename(path), | ||
| ownerId: ctx.toolCallId, |
There was a problem hiding this comment.
[blocking] Scope the snapshot owner to the durable tool operation
The context store enforces uniqueness on (session_id, owner_kind, owner_id), but this toolCallId is provider-supplied. Runtime only treats that value as unique together with invocationId: buildToolOperationId derives the durable ID from both, and the runtime schema uses (invocation_id, provider_tool_call_id). If an adapter reuses a call ID in a later invocation of the same Session, a different image hits identity_conflict. Please key the snapshot with Runtime's durable operation identity (or another explicitly Session-global derived ID), keeping the provider call ID only as correlation metadata.
| for (const item of Object.values(value)) visit(item); | ||
| }; | ||
| visit(input.copiedMessages); | ||
| visit(input.plan.inlineRuntimeEvents); |
There was a problem hiding this comment.
[blocking] Collect only refs in the selected copy projection
plan.inlineRuntimeEvents contains every source.events entry, while copyTurnIds and plan.runs describe the selected history. A normal branch from an earlier turn after a later image Read therefore copies the later image ref into the target even though that event is not published there. The new Session then consumes logical quota and retention for an invisible ref. Please collect from the same typed, selected projection that the rewrite will publish, rather than from the complete source event set.
3 participants
Stacked on #4182. Part of #4071.
What changed
Why this shape
The Store is an index over offloaded Agent context, not another Artifact container. SQLite owns the metadata and reference lifecycle; managed values remain KV-separated on disk. This PR activates one canonical producer only, so rollback is the writer switch while the reader remains compatible.
Verification