-
Notifications
You must be signed in to change notification settings - Fork 407
feat(storage): read durable Session context refs #4182
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
Changes from all commits
da7c481
a13b3c1
2715ee2
d87bba8
407a687
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 |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| { | ||
| "epoch": 67, | ||
| "files": [ | ||
| "packages/runtime-host/src/protocol/hosted-execution.ts", | ||
| "packages/runtime-host/src/protocol/message.ts", | ||
| "packages/runtime-host/src/protocol/turn.ts" | ||
| ], | ||
| "reason": "Widens Host result decoding for a durable context reference variant while client admission rejects Host-owned refs; this reader-only slice emits no new wire frames" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -355,7 +355,7 @@ function decodeTurnStartInput(value: unknown): TurnStartInput { | |
| return { | ||
| sessionId: requireEntityId(record.sessionId, 'sessionId'), | ||
| turnId: requireEntityId(record.turnId, 'turnId'), | ||
| content: decodeMessageContent(record.content, skillIds.length > 0), | ||
| content: decodeMessageAdmissionContent(record.content, skillIds.length > 0), | ||
| ...(skillIds.length > 0 ? { skillIds } : {}), | ||
| ...(record.turnOrchestration !== undefined | ||
| ? { turnOrchestration: decodeTurnOrchestration(record.turnOrchestration) } | ||
|
|
@@ -431,14 +431,16 @@ export function decodeMessageContent(value: unknown, allowEmptyText = false): Me | |
| if (attachment.bytes > MAX_ATTACHMENT_BYTES) { | ||
| throw invalidProtocolFrame('Invalid AttachmentRef bytes'); | ||
| } | ||
| if (attachment.ref.kind === 'session_file') { | ||
| if (attachment.ref.kind === 'session_file' || attachment.ref.kind === 'session_context') { | ||
|
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] This decoder is also used by turn.message.submit, so the reader-first widening lets a peer persist arbitrary or dangling session_context references before the claimed writer cutover. Please use a direction-specific decoder or admission rule, or validate the active capability and referenced record before accepting this durable input.
Member
Author
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. Fixed in 91817d6. Client admission now uses a direction-specific decoder that rejects Host-owned session_context attachments for turn.start, turn.message.submit, and hosted execution, while snapshot and result decoding continues to accept the ref kind. |
||
| requireEntityId(attachment.ref.sessionId, 'AttachmentRef sessionId'); | ||
| } | ||
| const path = | ||
| const identity = | ||
| attachment.ref.kind === 'external_file' | ||
| ? attachment.ref.absolutePath | ||
| : attachment.ref.relativePath; | ||
| requireUtf8String(path, 'AttachmentRef path', ATTACHMENT_PATH_MAX_BYTES, false); | ||
| : attachment.ref.kind === 'session_context' | ||
| ? attachment.ref.refId | ||
| : attachment.ref.relativePath; | ||
| requireUtf8String(identity, 'AttachmentRef identity', ATTACHMENT_PATH_MAX_BYTES, false); | ||
| } | ||
| if ((content.quotes?.length ?? 0) > TURN_MESSAGE_QUOTE_MAX_COUNT) { | ||
| throw invalidProtocolFrame('Invalid Message quotes'); | ||
|
|
@@ -456,6 +458,18 @@ export function decodeMessageContent(value: unknown, allowEmptyText = false): Me | |
| return content; | ||
| } | ||
|
|
||
| /** Client-authored Messages cannot claim Host-owned Session context references. */ | ||
| export function decodeMessageAdmissionContent( | ||
| value: unknown, | ||
| allowEmptyText = false, | ||
| ): MessageContent { | ||
| const content = decodeMessageContent(value, allowEmptyText); | ||
| if (content.attachments?.some((attachment) => attachment.ref.kind === 'session_context')) { | ||
| throw invalidProtocolFrame('Session context references are Host-owned'); | ||
| } | ||
| return content; | ||
| } | ||
|
|
||
| function requireUtf8String( | ||
| value: unknown, | ||
| label: string, | ||
|
|
||
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.
[P1] Adding session_context to the global StorageRef union makes it flow through conversation copy, but rewriteStorageRef only rewrites session_file. A copied message therefore retains the source sessionId, and the target Session later fails to hydrate it with session_mismatch; Session retirement also does not release these references. Please add lifecycle handling or fail these operations explicitly until that owner exists.
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.
Fixed in 91817d6 for the reader-only slice. Exact conversation copy now rejects source-owned session_context refs, and Session removal checks context usage under admission and fails before the tombstone if any refs exist or if the Store is unavailable. The stacked writer PR will replace these guards with copy and retire lifecycle handling.