fix(gateway): persist media metadata in agent.request transcripts#86936
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 7, 2026, 1:50 AM ET / 05:50 UTC. Summary PR surface: Source +37, Tests +197. Total +234 across 12 files. Reproducibility: yes. at source level. Current main parses Review metrics: 1 noteworthy metric.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Land after redacted node/iOS share or node-event proof shows the persisted and reloaded transcript keeps ordered Do we have a high-confidence way to reproduce the issue? Yes at source level. Current main parses Is this the best way to solve the issue? Yes, this is the best fix shape I found: it reuses shared inbound-media persistence and SessionManager-backed transcript helpers instead of adding a raw JSONL write or display-only update. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 094c0d421faf. Label changesLabel justifications:
Evidence reviewedPR surface: Source +37, Tests +197. Total +234 across 12 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (3 earlier review cycles)
|
|
ClawSweeper PR egg 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
There was a problem hiding this comment.
Pull request overview
This PR aims to ensure media attachments received via agent.request node events are no longer lost from session history by persisting inbound image data and emitting transcript updates that include MediaPath / MediaPaths metadata (matching the behavior established for chat.send).
Changes:
- Exposes
resolveSessionFilePath,saveMediaBuffer, andemitSessionTranscriptUpdatevia theserver-node-events.runtime.tsbarrel for easier mocking. - Adds
agent.requesthelpers inserver-node-events.tsto persist inline images, pass through offloaded refs, and emit transcript updates with media fields. - Adds regression tests covering inline images, mixed inline + offloaded, and text-only agent requests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/gateway/server-node-events.runtime.ts | Re-exports additional runtime helpers so tests can mock transcript/media behavior. |
| src/gateway/server-node-events.ts | Persists inbound media for agent.request and emits transcript updates with MediaPath(s) metadata. |
| src/gateway/server-node-events.test.ts | Adds regression tests validating transcript update emission for agent.request attachments. |
Comments suppressed due to low confidence (3)
src/gateway/server-node-events.ts:358
persistAgentRequestImages()appendsoffloadedRefsafter inlineimages, but the parser providesimageOrderspecifically to preserve the original inline/offloaded image ordering. If offloaded images appear before inline ones (or are interleaved), the resultingMediaPath/MediaPathsordering will be incorrect. Consider acceptingimageOrderand interleaving saved inline/offloaded entries the same waypersistChatSendImages()does insrc/gateway/server-methods/chat.ts.
async function persistAgentRequestImages(params: {
images: Array<{ type: "image"; data: string; mimeType: string }>;
offloadedRefs: Array<{ id: string; path: string; mimeType: string }>;
logGateway: NodeEventContext["logGateway"];
}): Promise<SavedMediaEntry[]> {
if (params.images.length === 0 && params.offloadedRefs.length === 0) {
return [];
}
const saved: SavedMediaEntry[] = [];
for (const img of params.images) {
try {
saved.push(await saveMediaBuffer(Buffer.from(img.data, "base64"), img.mimeType, "inbound"));
} catch (err) {
params.logGateway.warn(
`agent.request: failed to persist inbound image (${img.mimeType}): ${formatForLog(err)}`,
);
}
}
for (const ref of params.offloadedRefs) {
saved.push({ id: ref.id, path: ref.path, size: 0, contentType: ref.mimeType });
}
return saved;
src/gateway/server-node-events.ts:377
emitAgentRequestTranscript()always callsresolveSessionFilePath(sessionId, undefined, ...), ignoring any existingentry.sessionFile. If the session is configured to use a custom/rotated transcript filename, this can emit updates against the wrong.jsonlpath and further desync history. Consider passing the loaded session entry’ssessionFileintoresolveSessionFilePath(and/or agentId) similar toresolveTranscriptPath()inserver-methods/chat.ts.
try {
const sessionsDir = storePath ? path.dirname(storePath) : undefined;
transcriptPath = resolveSessionFilePath(
sessionId,
undefined,
sessionsDir ? { sessionsDir } : undefined,
);
src/gateway/server-node-events.ts:405
emitSessionTranscriptUpdate()only broadcasts a change; it does not write to the transcript JSONL. In this handler, no subsequent rewrite/appending step is updating the on-disk user entry to includeMediaPath/MediaPaths, so the transcript will still contain plain text and media will remain orphaned on reload. Consider adding an on-disk rewrite step (similar torewriteChatSendUserTurnMediaPaths()insrc/gateway/server-methods/chat.ts) after the user message is persisted, and emit the update based on the rewritten/persisted message.
emitSessionTranscriptUpdate({
sessionFile: transcriptPath,
sessionKey: canonicalKey,
message: {
role: "user" as const,
content: message,
timestamp: now,
...mediaFields,
},
});
48732cb to
cf74ea1
Compare
328b202 to
0e3071e
Compare
|
@peterdsp thanks for the PR. ClawSweeper is still waiting on real behavior proof before this can move forward. Useful proof can be a screenshot, short video, terminal output, copied live output, linked artifact, or redacted logs that show the changed behavior after the fix. Please redact private tokens, phone numbers, private endpoints, customer data, and anything else sensitive. Once proof is added to the PR body or a comment, ClawSweeper or a maintainer can re-check it. |
0e3071e to
ea14584
Compare
Route inline and offloaded agent.request images through the canonical user-turn transcript recorder across embedded, CLI, and ACP runtimes. Share ordered media persistence with chat.send and cover media-only empty-reply turns. Co-authored-by: Petros Dhespollari <[email protected]>
ea14584 to
8ed7873
Compare
|
Land-ready verification for reviewed head
Known proof gaps: none for the changed gateway/agent transcript path. |
|
Merged via squash.
|
…enclaw#86936) * fix(gateway): persist agent request transcript media Route inline and offloaded agent.request images through the canonical user-turn transcript recorder across embedded, CLI, and ACP runtimes. Share ordered media persistence with chat.send and cover media-only empty-reply turns. Co-authored-by: Petros Dhespollari <[email protected]> * fix(gateway): avoid transcript media shadowing --------- Co-authored-by: Peter Steinberger <[email protected]>
…enclaw#86936) * fix(gateway): persist agent request transcript media Route inline and offloaded agent.request images through the canonical user-turn transcript recorder across embedded, CLI, and ACP runtimes. Share ordered media persistence with chat.send and cover media-only empty-reply turns. Co-authored-by: Petros Dhespollari <[email protected]> * fix(gateway): avoid transcript media shadowing --------- Co-authored-by: Peter Steinberger <[email protected]>
What Problem This Solves
Images sent by the iOS Share Extension enter the gateway through the node
agent.requestevent. The gateway parses the attachment and the active agent can inspect it, but the durable user turn stored only the caption text. After history reload, the transcript no longer knew which media belonged to that turn.Closes #60339.
What Is Stored Where
media://inbound/<id>record and file path; inline base64 images are now saved there before dispatch.MediaPath,MediaPaths,MediaType, andMediaTypes. Those fields point at the inbound media files; the transcript does not duplicate image bytes.Why The Existing Path Was Broken
parseMessageWithAttachmentscorrectly produced inline model images, offloaded media references, and their original order.agentCommandFromIngressreceived those model inputs, but the canonical user-turn recorder received text only. The model could therefore process the image during the live turn while session history persisted no media metadata.The earlier implementation in this PR called
emitSessionTranscriptUpdateafter guessing a transcript path. That helper broadcasts a display update; it is not the canonical durable append path. It could also create a second projection rather than enriching the single user turn, and it did not cover ACP-backed sessions.Why This Change Was Made
chat.sendandagent.request, preserving mixed inline/offloaded order.transcriptMediainto the agent command boundary while keeping the original user caption separate from model-onlymedia://markers.User Impact
Shared images remain attached to the correct user turn after transcript reload. Text-only requests are unchanged, and image bytes are not duplicated into session JSONL.
Evidence
git diff --checkrun_d555f247ea33on behavior headea145844428c5763f29cdd6a5d4402d1d328c578: 204 gateway tests and 133 agent tests passed across the selected Vitest shards.8ed7873fe74d21e1534ff5361d2fb9ac709b2150in run28843912996; the only post-test follow-up was a shadowed callback-parameter rename.