-
Notifications
You must be signed in to change notification settings - Fork 387
feat(runtime-host): write Read images to Session context #4184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,7 @@ import { | |
| archivedToolResultContainsConversationOwnedReferences, | ||
| cloneConversationRuntimeLedger, | ||
| collectConversationCopyLinkedChildReferences, | ||
| collectConversationCopySessionContextRefIds, | ||
| createConversationCopySlice, | ||
| prepareConversationRuntimeLedgerCopy, | ||
| type ConversationRuntimeLedgerCopyPlan, | ||
|
|
@@ -54,6 +55,7 @@ import { | |
| authenticateInteractiveTaskLedgerWriter, | ||
| type InteractiveTaskLedgerWriter, | ||
| } from '@maka/storage/task-ledger-authority'; | ||
| import type { InteractiveContextOffloadWriter } from '@maka/storage/context-offload-store'; | ||
| import type { | ||
| OperationOutcome, | ||
| SessionConversationCopyInput, | ||
|
|
@@ -99,6 +101,10 @@ export interface HostSessionRevisionCoordinatorOptions { | |
| readonly stores: ExecutionStoresWriter<'interactive'>; | ||
| readonly artifacts: InteractiveArtifactStoreWriter; | ||
| readonly taskLedger: InteractiveTaskLedgerWriter; | ||
| readonly contextOffload?: Pick< | ||
| InteractiveContextOffloadWriter, | ||
| 'copyReferences' | 'retireSession' | ||
| >; | ||
| readonly manager: SessionManager; | ||
| readonly admission: SessionAdmissionGate; | ||
| readonly continuity: SessionContinuityCoordinator; | ||
|
|
@@ -489,6 +495,28 @@ export class HostSessionRevisionCoordinator { | |
| ) | ||
| .map(({ descriptor, serializedResult }) => [descriptor.artifactId, serializedResult]), | ||
| ); | ||
| const sourceContextRefIds = collectConversationCopySessionContextRefIds({ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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. |
||
| sourceSessionId: input.sourceSessionId, | ||
| copiedMessages: slice.messages, | ||
| plan, | ||
| }); | ||
| if (sourceContextRefIds.length > 0 && !this.options.contextOffload) { | ||
| throw new Error('Session context copy authority is unavailable'); | ||
| } | ||
| const contextCopy = | ||
| sourceContextRefIds.length === 0 | ||
| ? { ok: true as const, copied: [] } | ||
| : await this.options.contextOffload!.copyReferences({ | ||
| sourceSessionId: input.sourceSessionId, | ||
| targetSessionId: input.targetSessionId, | ||
| references: sourceContextRefIds.map((sourceRefId) => ({ | ||
| sourceRefId, | ||
| targetOwner: { kind: 'read_image_snapshot', ownerId: sourceRefId }, | ||
| })), | ||
| }); | ||
| if (!contextCopy.ok) { | ||
| throw new Error(`Session context references could not be copied: ${contextCopy.reason}`); | ||
| } | ||
| const artifactCopy = await this.#artifacts.copyConversationArtifacts({ | ||
| sourceSessionId: input.sourceSessionId, | ||
| targetSessionId: input.targetSessionId, | ||
|
|
@@ -511,6 +539,9 @@ export class HostSessionRevisionCoordinator { | |
| targetSessionId: input.targetSessionId, | ||
| artifactIds: artifactCopy.artifactIds, | ||
| relativePaths: artifactCopy.relativePaths, | ||
| contextRefs: new Map( | ||
| contextCopy.copied.map(({ sourceRefId, targetRefId }) => [sourceRefId, targetRefId]), | ||
| ), | ||
| linkedChildren: | ||
| kind === 'side_conversation' | ||
| ? { | ||
|
|
@@ -803,6 +834,7 @@ export class HostSessionRevisionCoordinator { | |
| { | ||
| artifacts: this.#artifacts, | ||
| taskLedger: this.#taskLedger, | ||
| ...(this.options.contextOffload ? { contextOffload: this.options.contextOffload } : {}), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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. |
||
| purgeOperationalState: (sessionId) => | ||
| this.#stores.purgeConversationOperationalState(sessionId), | ||
| }, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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.