[agent] fix: carry tool-result images and state unsupported modalities instead of dropping them

devlog/_plan/260914_provider_parity_stack/040_phase4_modality_fidelity.md

@@ -145,7 +145,7 @@ than a drop because it can fail schema validation upstream.
             // bounded marker keeps the turn well-formed and tells the model an
             // attachment it cannot see was sent; the previous code produced a text
             // part whose text was undefined.
-            if (p.type === "video") return { type: "text", text: "[video omitted: unsupported by this provider]" };
+            if (p.type === "video") return { type: "text", text: "[video omitted: the translated Chat route has no video mapping]" };
             return { type: "text", text: (p as OcxTextContent).text };
           });
 ```

devlog/_plan/260914_provider_parity_stack/050_residuals.md

@@ -20,17 +20,29 @@ provider B, and a cache lifetime. `src/responses/reasoning-replay-cache.ts`
 already solves a narrower version of this inside one provider's session and is the
 natural starting point. It is a design unit, not a line change.
 
-## R2 — real audio transport in the translated IR (from F5)
+## R2 — real audio/file transport, and any adapter-level refusal (from F5)
 
-Phase 4 preserves the *presence* of an audio attachment and explicitly does not add
-audio support. `OcxContentPart` has no audio member, no adapter consumes one, and
-per-provider audio capability is not recorded anywhere in the catalog —
-`src/providers/registry.ts:1062` notes exactly this when it omits audio from the
-Baseten hints.
+**F5 is PRESENCE-ONLY and is not fixed.** Phase 4 records that an audio attachment
+existed and explicitly does not add audio support. `OcxContentPart` has no audio
+member, no adapter consumes one, and per-provider audio capability is not recorded
+anywhere in the catalog — `src/providers/registry.ts:1062` notes exactly this when it
+omits audio from the Baseten hints.
 
-Adding it means a carrier type, capability data for 93 providers, and a wire mapping
-per vendor. Guessing any one of those produces a request that fails at call time
-instead of a modality that works.
+Two things are residual, not delivered:
+
+- **Transport.** A carrier type, capability data across the provider set, and a wire
+  mapping per vendor. Guessing any one of those produces a request that fails at call
+  time instead of a modality that works.
+- **Refusal.** There is no adapter-level rejection of audio. By final dispatch the part
+  is already a text marker, so every adapter continues. Doing this properly needs a
+  typed unsupported-modality signal that survives to final adapter dispatch — including
+  `runTurn`, compaction and sidecar paths — while raw Responses passthrough stays
+  untouched. An early throw in the shared parser is not acceptable: raw passthrough
+  runs through `parseRequest` before the adapter forwards `_rawBody`.
+
+`input_file` keeps its existing filename-only marker, and Chat inbound has no file or
+audio translation at all, so a Chat request can lose media before the Responses parser
+sees it. Neither is addressed here.
 
 ## R3 — Kiro remote images stay uninlined
 

scripts/test-layout/layout.json

@@ -167,6 +167,8 @@
     }
   },
   "explicit": {
+    "chat-responses-control-integration.test.ts": "responses",
+    "coding-agent-tool-result-images.test.ts": "adapters",
     "hub-usage.test.ts": "server",
     "client-hub-usage.test.ts": "clients",
     "cli-usage-hub.test.ts": "cli",
@@ -787,6 +789,7 @@
     "kiro-oauth.test.ts": "providers/kiro",
     "kiro-pool-rank.test.ts": "providers/kiro",
     "kiro-reasoning-roundtrip.test.ts": "providers/kiro",
+    "kiro-remote-image.test.ts": "providers/kiro",
     "kiro-retry.test.ts": "providers/kiro",
     "kiro-review-regressions.test.ts": "providers/kiro",
     "kiro-stream.test.ts": "providers/kiro",
@@ -988,6 +991,7 @@
     "openai-chat-system-order.test.ts": "adapters/openai",
     "openai-chat-tool-result-images.test.ts": "adapters/openai",
     "openai-chat-url.test.ts": "adapters/openai",
+    "openai-chat-video-part.test.ts": "adapters/openai",
     "openai-provider-option-e2e.test.ts": "adapters/openai",
     "openai-provider-option-migration.test.ts": "adapters/openai",
     "openai-provider-option-startup.test.ts": "adapters/openai",
@@ -1013,6 +1017,7 @@
     "owned-service-home.test.ts": "server",
     "package-tree-integrity.test.ts": "ci-workflows",
     "parallel-tool-calls-optin.test.ts": "codex-integration",
+    "parser-content-audio.test.ts": "responses",
     "passive-route-linker.test.ts": "server",
     "passthrough-abort.test.ts": "responses",
     "passthrough-headers.test.ts": "responses",

src/adapters/coding-agent/protocol.ts

@@ -409,7 +409,24 @@ export function buildConversationInput(parsed: OcxParsedRequest): string[] {
   const historyMessages = nonDev.slice(0, -1);
   const currentMessage = nonDev[nonDev.length - 1]!;
 
-  const imageBlocks: WireContentPart[] = [];
+  // History images are collected BEFORE the current message's so the attached blocks
+  // follow conversation order. The projected prose says "Prior conversation context"
+  // then "Current user request", so emitting current-turn images first contradicted
+  // the text the model reads alongside them.
+  const historyImageBlocks: WireContentPart[] = [];
+  for (const msg of historyMessages) {
+    if (!Array.isArray(msg.content)) continue;
+    // Tool results carry images too — a screenshot returned by a tool was previously
+    // flattened to the literal text "[image]" and the carrier discarded.
+    if (msg.role !== "user" && msg.role !== "toolResult") continue;
+    for (const part of msg.content) {
+      if (part.type !== "image") continue;
+      const img = imagePart(part.imageUrl);
+      if (img) historyImageBlocks.push(img);
+    }
+  }
+
+  const currentImageBlocks: WireContentPart[] = [];
   let currentRequestText = "";
 
   if (currentMessage.role === "user") {
@@ -421,34 +438,42 @@ export function buildConversationInput(parsed: OcxParsedRequest): string[] {
         if (part.type === "text") textParts.push(part.text);
         else if (part.type === "image") {
           const image = imagePart(part.imageUrl);
-          if (image) imageBlocks.push(image);
+          if (image) currentImageBlocks.push(image);
+          else textParts.push("[image omitted: unsupported reference]");
         } else {
           textParts.push("[video]");
         }
       }
       currentRequestText = textParts.join("\n");
     }
   } else if (currentMessage.role === "toolResult") {
-    const text = typeof currentMessage.content === "string"
-      ? currentMessage.content
-      : currentMessage.content.map(p => (p.type === "text" ? p.text : "[image]")).join("");
+    let text: string;
+    if (typeof currentMessage.content === "string") {
+      text = currentMessage.content;
+    } else {
+      const segments: string[] = [];
+      for (const part of currentMessage.content) {
+        if (part.type === "text") { segments.push(part.text); continue; }
+        if (part.type === "image") {
+          // Carry the real image instead of flattening it to a marker. The provenance
+          // note stays so the prose still reads coherently and the model can tell which
+          // attachment the tool produced; the bytes travel as an image block, never as text.
+          const image = imagePart(part.imageUrl);
+          if (image) { currentImageBlocks.push(image); segments.push("[image attached below]"); }
+          else segments.push("[image omitted: unsupported reference]");
+          continue;
+        }
+        segments.push("[video]");
+      }
+      text = segments.join("");
+    }
     const status = currentMessage.isError ? " (error)" : "";
     currentRequestText = `TOOL RESULT (call_id: ${currentMessage.toolCallId})${status}:\n${text}\n\nPlease proceed based on the above tool result.`;
   } else {
     currentRequestText = formatMessageForHistory(currentMessage);
   }
 
-  // Also collect any images from history messages so multimodal attachments are never dropped:
-  for (const msg of historyMessages) {
-    if (msg.role === "user" && Array.isArray(msg.content)) {
-      for (const part of msg.content) {
-        if (part.type === "image") {
-          const img = imagePart(part.imageUrl);
-          if (img) imageBlocks.push(img);
-        }
-      }
-    }
-  }
+  const imageBlocks: WireContentPart[] = [...historyImageBlocks, ...currentImageBlocks];
 
   let historyText = historyMessages.map(formatMessageForHistory).filter(Boolean).join("\n\n");
   if (historyText.length > MAX_PROJECTED_HISTORY_CHARS) {

src/adapters/kiro-images.ts

@@ -35,6 +35,36 @@ export function extractKiroImages(content: string | OcxContentPart[]): KiroImage
   return out;
 }
 
+/**
+ * Count images Kiro cannot inline, so the loss is never silent.
+ *
+ * Kiro's wire carries base64 bytes only, so a remote reference genuinely cannot be
+ * sent, and this proxy does not fetch one on a request path. Such a part used to be
+ * dropped with neither bytes nor any trace that an attachment existed. Counting them
+ * lets the payload builder attach a bounded marker instead.
+ *
+ * The count is all that crosses: a remote image URL can carry a signed token, so the
+ * URL itself is never echoed into prose.
+ */
+export function countKiroUninlinableImages(content: string | OcxContentPart[]): number {
+  if (typeof content === "string") return 0;
+  let count = 0;
+  for (const p of content) {
+    if (p.type !== "image") continue;
+    // Keyed on the scheme, not on parse success: a malformed data URL also fails
+    // parseDataUrlImage, and labelling that "remote reference" would misstate the cause.
+    if (!p.imageUrl.startsWith("data:")) count++;
+  }
+  return count;
+}
+
+/** Bounded, content-free marker for images Kiro could not inline. */
+export function kiroUninlinableImageMarker(count: number): string {
+  if (count <= 0) return "";
+  if (count === 1) return "[image omitted: remote image references are not supported by this provider]";
+  return "[" + String(count) + " images omitted: remote image references are not supported by this provider]";
+}
+
 /**
  * Conservative POLICY caps for the CodeWhisperer GenerateAssistantResponse payload,
  * whose limits are undocumented. Derived from adjacent AWS surfaces

src/adapters/kiro/payload.ts

@@ -22,7 +22,12 @@ import {
 } from "../kiro-constants";
 import { EMPTY_EXEC_OUTPUT_MESSAGE, annotateCodeModeHostFailure, normalizeEmptyExecToolResultText } from "../exec-tool-result-normalize";
 import { identifyRoutedModel } from "../identity";
-import { extractKiroImages, type KiroImage } from "../kiro-images";
+import {
+  countKiroUninlinableImages,
+  extractKiroImages,
+  kiroUninlinableImageMarker,
+  type KiroImage,
+} from "../kiro-images";
 import { convertKiroToolContext } from "../kiro-tools";
 import { createKiroToolNameRegistry, mapModelId, normalizeToolId, stableConversationId } from "../kiro-wire";
 import { buildNonOpenAIToolCatalogNudgeFromNames, isBareShellBridgeTool, isCodexCodeModeExecTool } from "../tool-catalog-nudge";
@@ -233,9 +238,13 @@ export function buildKiroPayload(
     // Original-message adjacency matters even when a turn is collapsed or skipped below.
     if (msg.role !== "toolResult") finishAdjacentResult();
     if (msg.role === "user" || msg.role === "developer") {
-      const text = userContentText((msg as { content: string | OcxContentPart[] }).content);
-      const images = extractKiroImages((msg as { content: string | OcxContentPart[] }).content);
-      pushUser(text, images);
+      const content = (msg as { content: string | OcxContentPart[] }).content;
+      const images = extractKiroImages(content);
+      // Kiro inlines base64 bytes only. A remote reference used to vanish with neither
+      // bytes nor a trace; attach a bounded, URL-free marker so the loss is visible.
+      const marker = kiroUninlinableImageMarker(countKiroUninlinableImages(content));
+      const text = userContentText(content);
+      pushUser(marker ? (text ? text + "\n" + marker : marker) : text, images);
     } else if (msg.role === "assistant") {
       const aMsg = msg as OcxAssistantMessage;
       const text = (aMsg.content || [])
@@ -281,16 +290,29 @@ export function buildKiroPayload(
       const annotatedExecText = normalizedExecText === undefined && codeModeExecName !== undefined
         ? annotateCodeModeHostFailure(text, execOptions)
         : undefined;
-      const resultText = normalizedExecText ?? annotatedExecText ?? (text.trim() ? text : KIRO_EMPTY_TOOL_RESULT_MESSAGE);
+      const uninlinableMarker = kiroUninlinableImageMarker(countKiroUninlinableImages(tr.content));
+      // Appended to the SELECTED result text, not to `text`: when an exec normalization
+      // fires, resultText below takes normalizedExecText/annotatedExecText instead, and
+      // a marker attached to `text` would be dropped — reinstating the silent loss this
+      // exists to remove.
+      const chosenText = normalizedExecText ?? annotatedExecText ?? (text.trim() ? text : KIRO_EMPTY_TOOL_RESULT_MESSAGE);
+      const resultText = uninlinableMarker
+        ? (chosenText ? chosenText + "\n" + uninlinableMarker : uninlinableMarker)
+        : chosenText;
       const images = extractKiroImages(tr.content);
       const toolUseId = normalizeToolId(tr.toolCallId);
       const call = priorCalls.get(toolUseId);
       if (!call || call.rawId !== tr.toolCallId) {
         throw new Error(`Kiro history contains an orphaned tool result for call ${JSON.stringify(tr.toolCallId)}`);
       }
       // Keep real whitespace and failed wrappers, but no empty-success wrapper boilerplate.
-      const rawGroupText = text.length > 0 && (!text.trim() || normalizedExecText !== EMPTY_EXEC_OUTPUT_MESSAGE)
+      const rawGroupBase = text.length > 0 && (!text.trim() || normalizedExecText !== EMPTY_EXEC_OUTPUT_MESSAGE)
         ? (annotatedExecText ?? text) : undefined;
+      // The grouping path rebuilds a collapsed turn's content from these texts, so the
+      // marker has to ride along here too or an adjacent-result turn loses it.
+      const rawGroupText = uninlinableMarker
+        ? (rawGroupBase ? rawGroupBase + "\n" + uninlinableMarker : uninlinableMarker)
+        : rawGroupBase;
       const last = turns.at(-1);
       if (
         adjacentResult?.rawId === tr.toolCallId

src/adapters/openai-chat.ts

@@ -108,6 +108,17 @@ function openAIChatTransport(provider: OcxProviderConfig): {
   return { url, headers, hasCredential };
 }
 
+/**
+ * The translated Chat route has no video mapping: this adapter does not implement one,
+ * and the marker records that fact so the payload is not dropped in silence.
+ *
+ * The wording is deliberately about opencodex's own translation, not the provider or
+ * model. An earlier revision said "unsupported by this provider", which attributed an
+ * opencodex mapping limit to upstream capability the proxy has not established. Native
+ * Chat passthrough and Google inline video are unaffected by this route.
+ */
+const VIDEO_UNSUPPORTED_MARKER = "[video omitted: the translated Chat route has no video mapping]";
+
 /**
  * Build a provider request from an inbound Chat Completions body without translating it
  * through the Responses contract. This is deliberately a whitelist: Chat-only caller
@@ -784,11 +795,29 @@ function messagesToChatFormat(parsed: OcxParsedRequest, provider: OcxProviderCon
         } else if (typeof msg.content === "string") {
           chatMsg = { role: "user", content: msg.content };
         } else if (!hasImages) {
-          chatMsg = { role: "user", content: parts!.map(p => (p as OcxTextContent).text).join("") };
+          // A video part has no `text`, so joining it produced "" and the whole message
+          // was dropped: a video-only or text-plus-video turn vanished silently. OpenAI's
+          // Chat Completions wire has no video content part, so state the omission
+          // instead of losing it. Scoped to this adapter's wire, not a claim about video
+          // support in general — native Chat passthrough and Google inline video are
+          // unaffected.
+          chatMsg = {
+            role: "user",
+            content: parts!.map(p => (p.type === "video"
+              ? VIDEO_UNSUPPORTED_MARKER
+              : (p as OcxTextContent).text)).join(""),
+          };
         } else {
-          const chatParts = parts!.map(p => p.type === "image"
-            ? { type: "image_url", image_url: { url: p.imageUrl, ...(p.detail ? { detail: p.detail } : {}) } }
-            : { type: "text", text: (p as OcxTextContent).text });
+          const chatParts = parts!.map(p => {
+            if (p.type === "image") {
+              return { type: "image_url", image_url: { url: p.imageUrl, ...(p.detail ? { detail: p.detail } : {}) } };
+            }
+            // Previously this produced { type: "text", text: undefined } for a video
+            // part — a malformed part, worse than a drop because it can fail upstream
+            // schema validation.
+            if (p.type === "video") return { type: "text", text: VIDEO_UNSUPPORTED_MARKER };
+            return { type: "text", text: (p as OcxTextContent).text };
+          });
           chatMsg = { role: "user", content: chatParts };
         }
         if (pendingToolCalls.length > 0) deferredBarrierMessages.push(chatMsg);