From 07264c16a3d8b341c04bfc109e5bbd727f507c1b Mon Sep 17 00:00:00 2001 From: chiperman <34889778+chiperman@users.noreply.github.com> Date: Tue, 29 Sep 2026 18:15:21 +0800 Subject: [PATCH 1/3] fix(responses): display hosted image results in local Codex clients - Project hosted image results into final assistant messages backed by validated local artifacts. - Preserve upstream continuation state and add bounded delivery, synthetic tests, and documentation. --- .../content/docs/guides/codex-integration.md | 16 ++ scripts/test-layout/layout.json | 2 + src/server/responses-hosted-image-display.ts | 179 ++++++++++++++++ src/server/responses/passthrough-delivery.ts | 16 ++ structure/INDEX.md | 2 +- structure/data-planes/images.md | 21 ++ structure/manifest.json | 2 +- structure/transports/responses-wire-shapes.md | 2 + tests/fixtures/hosted-image-display.ts | 5 + tests/fixtures/test-layout-expected.json | 2 + .../responses-hosted-image-delivery.test.ts | 119 +++++++++++ .../responses-hosted-image-display.test.ts | 198 ++++++++++++++++++ 12 files changed, 562 insertions(+), 2 deletions(-) create mode 100644 src/server/responses-hosted-image-display.ts create mode 100644 tests/fixtures/hosted-image-display.ts create mode 100644 tests/responses/responses-hosted-image-delivery.test.ts create mode 100644 tests/responses/responses-hosted-image-display.test.ts diff --git a/docs-site/src/content/docs/guides/codex-integration.md b/docs-site/src/content/docs/guides/codex-integration.md index 7ddd7fe18ff..4d8df0891a9 100644 --- a/docs-site/src/content/docs/guides/codex-integration.md +++ b/docs-site/src/content/docs/guides/codex-integration.md @@ -225,6 +225,22 @@ caller, including one that posts them directly, and a model turn records no hist opencodex decides this by reading Codex own config itself, so the switch does not depend on the injected URL or on anything a client sends, and turning it off takes effect without a restart. +### Hosted image results in Codex App + +When a routed Responses provider returns a completed hosted `image_generation_call` +with base64 image data, opencodex saves the validated image under its local +`artifacts/` directory and delivers a final assistant image message to a locally +connected Codex client. The image stays outside the collapsible progress section. +This applies to streaming and non-streaming Responses, without another generation +request or a change to the selected provider. + +This display compatibility requires loopback admission and a recognized Codex client. +Remote and generic API clients retain the provider's hosted response format. +Partial previews and URL-only results are not rendered by this compatibility layer. +Artifacts use the existing retention limit, so save images you want to keep before +older files are pruned. An invalid image or a failed local write produces a visible +failure message instead of a broken image link. + ### Built-in image generation (`image_gen`) Codex's built-in `image_gen` tool does not go through `/v1/responses` — the codex-rs extension diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 2cd657d98f6..42d865ec0d1 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -2,6 +2,8 @@ "version": 1, "root": "tests", "explicit": { + "responses-hosted-image-delivery.test.ts": "responses", + "responses-hosted-image-display.test.ts": "responses", "cursor-local-installer.test.ts": "providers/cursor", "codex-quota-query-backoff.test.ts": "codex-integration", "pnpm-command-isolation.test.ts": "update", "provider-antigravity-quota-retry.test.ts": "providers", "project-config-warning-snapshot.test.ts": "codex-integration", "codex-quota-auto-refresh-generation.test.ts": "codex-integration", "codex-account-clear-paused.test.ts": "codex-integration", diff --git a/src/server/responses-hosted-image-display.ts b/src/server/responses-hosted-image-display.ts new file mode 100644 index 00000000000..c69c9a8733d --- /dev/null +++ b/src/server/responses-hosted-image-display.ts @@ -0,0 +1,179 @@ +import { mkdirSync, writeFileSync, unlinkSync } from "node:fs"; +import { createHash, randomUUID } from "node:crypto"; +import { join } from "node:path"; +import { getConfigDir } from "../config"; +import { recordOwnedConfigPath } from "../lib/config-ownership"; +import { + chargeImageBudget, createImageBudget, decodeValidatedImageBase64, + getArtifactsDir, MAX_ENCODED_BYTES_PER_IMAGE, pruneArtifacts, sniffImageExtension, +} from "../images/artifacts"; +import { sseDataPayload, replaceSseDataPayload, type SseBlockRewrite } from "./sse-payload-rewrite"; + +type Row = Record; +const object = (v: unknown): v is Row => !!v && typeof v === "object" && !Array.isArray(v); +const originators = new Set(["codex_cli_rs", "Codex Desktop", "codex_app", "codex_work_desktop"]); + +/** Local filesystem links are only appropriate for the loopback Codex client. */ +export function isLocalCodexImageClient(headers: Headers, admissionKind?: string, inboundWire?: string): boolean { + return admissionKind === "loopback" + && (inboundWire === undefined || inboundWire === "responses") + && (originators.has(headers.get("originator") ?? "") + || /^codex(?:[_ /-]|$)/i.test(headers.get("user-agent") ?? "")); +} + +/** Client branch only: raw hosted items remain intact in the upstream replay cache. */ +export function createHostedImageDisplayRewrite(): SseBlockRewrite & { json(text: string): string } { + const budget = createImageBudget(); + const states = new Map(); + let extraSequence = 0; + let wroteArtifact = false; + + function stateFor(item: Row, index: number) { + const key = typeof item.id === "string" ? item.id : "index:" + index; + let state = states.get(key); + if (state) return state; + // Bound metadata independently of the existing 100 MiB decoded-image budget. + if (states.size >= 128) throw new RangeError("hosted image result count exceeds local display limit"); + const id = "msg_ocx_img_" + createHash("sha256").update(key).digest("hex").slice(0, 32); + state = { added: false, done: false, message: { + // Persist image results outside the client's collapsible progress messages. + id, type: "message", role: "assistant", phase: "final_answer", status: "in_progress", content: [], + ...(item.internal_chat_message_metadata_passthrough === undefined ? {} : { + internal_chat_message_metadata_passthrough: item.internal_chat_message_metadata_passthrough, + }), + } }; + states.set(key, state); + return state; + } + + function complete(item: Row, index: number) { + const state = stateFor(item, index); + if (state.message.status !== "in_progress") return state; + let text = item.status === "completed" + ? "The completed image result has no supported image data for local display." + : "Image generation did not complete; no image is available to display."; + let status = "incomplete"; + if (item.status === "completed" && typeof item.result === "string") { + try { + if (item.result.length > MAX_ENCODED_BYTES_PER_IMAGE) throw new RangeError("image too large"); + const bytes = decodeValidatedImageBase64(item.result); + chargeImageBudget(budget, bytes.length); + const dir = getArtifactsDir(); + recordOwnedConfigPath(getConfigDir(), dir); + mkdirSync(dir, { recursive: true, mode: 0o700 }); + const candidate = join(dir, "img-codex-" + randomUUID() + "." + sniffImageExtension(bytes)); + // Bounded synchronous write: block rewriters cannot await. Never use upstream ids as paths. + try { + writeFileSync(candidate, bytes, { flag: "wx", mode: 0o600 }); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "EEXIST") { + try { unlinkSync(candidate); } catch { /* best-effort partial-write cleanup */ } + } + throw error; + } + wroteArtifact = true; + text = "![Generated image](<" + candidate + ">)"; + status = "completed"; + } catch { + // Do not expose image data, filesystem errors, or credentials in a generated message. + text = "The image result could not be saved for local display."; + } + } + state.message = { ...state.message, status, content: [{ type: "output_text", text, annotations: [] }] }; + return state; + } + + function doneEvents(item: Row, index: number): Row[] { + const state = complete(item, index); + if (state.done) return []; + const { message } = state; + const common = { item_id: message.id, output_index: index, content_index: 0 }; + const part = message.content[0]; + const events: Row[] = []; + if (!state.added) events.push({ type: "response.output_item.added", output_index: index, + item: { ...message, status: "in_progress", content: [] } }); + events.push( + { type: "response.content_part.added", ...common, part: { ...part, text: "" } }, + { type: "response.output_text.delta", ...common, delta: part.text }, + { type: "response.output_text.done", ...common, text: part.text }, + { type: "response.content_part.done", ...common, part }, + { type: "response.output_item.done", output_index: index, item: message }, + ); + state.added = state.done = true; + return events; + } + + function snapshot(response: Row, terminal: boolean, events?: Row[]): Row { + if (!Array.isArray(response.output)) return response; + // Reject oversized snapshots before saving artifacts that cannot be delivered. + const keys = new Set(states.keys()); + for (const [index, item] of response.output.entries()) { + if (!object(item) || item.type !== "image_generation_call") continue; + keys.add(typeof item.id === "string" ? item.id : "index:" + index); + if (keys.size > 128) throw new RangeError("hosted image result count exceeds local display limit"); + } + let changed = false; + const output = response.output.map((item: unknown, index: number) => { + if (!object(item) || item.type !== "image_generation_call") return item; + changed = true; + if (terminal) { + if (events) events.push(...doneEvents(item, index)); + return complete(item, index).message; + } + return stateFor(item, index).message; + }); + return changed ? { ...response, output } : response; + } + + const rewrite = ((block: string): readonly string[] => { + const payload = sseDataPayload(block); + if (!payload || payload === "[DONE]") return [block]; + let event: Row; + try { event = JSON.parse(payload); } catch { return [block]; } + if (!object(event)) return [block]; + let result: Row[] = [event]; + const index = Number.isSafeInteger(event.output_index) && event.output_index >= 0 ? event.output_index : 0; + if (object(event.item) && event.item.type === "image_generation_call") { + if (event.type === "response.output_item.added") { + const state = stateFor(event.item, index); + result = state.added ? [] : [{ ...event, item: state.message }]; + state.added = true; + } else if (event.type === "response.output_item.done") { + result = doneEvents(event.item, index); + } + } else if (typeof event.type === "string" && event.type.startsWith("response.image_generation_call.")) { + // Hosted progress/partial-image frames refer to the replaced item; never expose mismatched ids. + result = []; + } else if (object(event.response)) { + const terminal = ["response.completed", "response.incomplete", "response.failed"].includes(event.type); + const injected: Row[] = []; + const response = snapshot(event.response, terminal, injected); + if (response !== event.response) result = [...injected, { ...event, response }]; + } + const sequence = Number.isSafeInteger(event.sequence_number) ? event.sequence_number + extraSequence : undefined; + const shift = extraSequence; + extraSequence += result.length - 1; + return result.map((value, offset) => { + if (value === event && shift === 0 && result.length === 1) return block; + const next = sequence === undefined ? value : { ...value, sequence_number: sequence + offset }; + const data = JSON.stringify(next); + return value.type === event.type ? replaceSseDataPayload(block, data) : "event: " + value.type + "\ndata: " + data; + }); + }) as SseBlockRewrite & { json(text: string): string }; + + rewrite.json = (text: string) => { + let response: unknown; + try { response = JSON.parse(text); } catch { return text; } + if (!object(response)) return text; + const next = snapshot(response, true); + return next === response ? text : JSON.stringify(next); + }; + rewrite.dispose = () => { + states.clear(); + if (wroteArtifact) { + wroteArtifact = false; + pruneArtifacts(); + } + }; + return rewrite; +} diff --git a/src/server/responses/passthrough-delivery.ts b/src/server/responses/passthrough-delivery.ts index 4c039f0d55c..9a1dbb8f500 100644 --- a/src/server/responses/passthrough-delivery.ts +++ b/src/server/responses/passthrough-delivery.ts @@ -97,6 +97,7 @@ import { PLAINTEXT_V2_AGENT_MESSAGE_RESTORE_OVERFLOW_MESSAGE, } from "../../responses/plaintext-v2-agent-messages"; import { createResponsesFieldBackfillBlockRewrite } from "./responses-field-backfill"; +import { createHostedImageDisplayRewrite, isLocalCodexImageClient } from "../responses-hosted-image-display"; import { createResponsesFunctionToolRepairBlockRewrite } from "../responses-function-tool-repair"; import { createUndeclaredToolCallGuardBlockRewrite, @@ -743,6 +744,10 @@ export async function deliverPassthroughResponse( ) : undefined, rememberPlaintextBlock, + // Display-only projection must follow every continuation-cache observer. + isLocalCodexImageClient(req.headers, options.admission?.kind, options.inboundWire) + ? createHostedImageDisplayRewrite() + : undefined, ].filter((rewrite): rewrite is NonNullable => rewrite !== undefined); const clientBlockRewrite = blockRewrites.length > 0 ? composeSseBlockRewrites(...blockRewrites) @@ -1022,6 +1027,17 @@ export async function deliverPassthroughResponse( JSON.parse(grokUpstreamEchoEnabled ? clientJson : text) as { id?: unknown; output?: unknown; status?: unknown; model?: unknown }, ); } catch { /* non-JSON despite content-type; recording is best-effort */ } + if (isLocalCodexImageClient(req.headers, options.admission?.kind, options.inboundWire)) { + const imageDisplay = createHostedImageDisplayRewrite(); + try { + clientJson = imageDisplay.json(clientJson); + } catch (error) { + if (error instanceof RangeError) { + return formatErrorResponse(502, "upstream_error", "hosted image result count exceeds local display limit"); + } + throw error; + } finally { imageDisplay.dispose?.(); } + } // #875: the transport-neutral reliability policy forced a bounded JSON // upstream for a client that asked for SSE. Reframe the completed JSON // as the canonical terminal SSE sequence (created → output_item.done → diff --git a/structure/INDEX.md b/structure/INDEX.md index a0dbbf3ea74..a841fce20b8 100644 --- a/structure/INDEX.md +++ b/structure/INDEX.md @@ -43,7 +43,7 @@ The wire surfaces a client actually talks to. | [`transports/responses-spend.md`](transports/responses-spend.md) | Credential-hop and durable spend reservations, and what a spent budget tells the client. | | [`transports/streaming-health.md`](transports/streaming-health.md) | Heartbeat and stall deadlines, plus the opt-in WebSocket transport. | | [`transports/inventory.md`](transports/inventory.md) | The per-provider transport table and diagnostic outbound safety. | -| [`data-planes/images.md`](data-planes/images.md) | Standalone image generation and edit relay. | +| [`data-planes/images.md`](data-planes/images.md) | Standalone image generation/edit relay and local hosted Responses image display. | | [`data-planes/search.md`](data-planes/search.md) | Hosted search relay and exact account selectors. | | [`data-planes/inbound-compat.md`](data-planes/inbound-compat.md) | Chat Completions inbound, Anthropic-shaped clients, and JSON-upstream streaming clients. | | [`data-planes/protocol-paths.md`](data-planes/protocol-paths.md) | Shared protocol vocabulary, declared feature dispositions, the ingress-by-upstream baseline, plan/trace shapes, and protocol settings. | diff --git a/structure/data-planes/images.md b/structure/data-planes/images.md index 97636d2674f..cd23c8050dd 100644 --- a/structure/data-planes/images.md +++ b/structure/data-planes/images.md @@ -16,6 +16,27 @@ Codex Spark exception; standalone Images retain the separate relay contract belo Shared parsing and streaming follow the [request-copy](../transports/byte-accounting.md#request-copy-accounting) and [stream-buffer accounting](../transports/byte-accounting.md#stream-buffer-accounting) contracts. Response-attached WebSocket telemetry follows the [stage record identity contract](../transports/responses-wire-shapes.md#passthrough-sse-stream-shapes-314). +## Hosted Responses image display + +For loopback-admitted Codex Responses clients, +`src/server/responses-hosted-image-display.ts` projects hosted `image_generation_call` +results into assistant messages with `phase: final_answer` and Markdown links to local +artifacts. The final phase keeps the image outside collapsible progress output. +`src/server/responses/passthrough-delivery.ts` applies this projection after continuation +observers, on the client branch only, for SSE, JSON and JSON synthesized into SSE. +Hosted items in the continuation cache retain their upstream representation; other +client rewrites keep their existing cache policy. Generic, remotely admitted and non-Responses +clients do not receive this filesystem projection. + +Image data must pass the shared base64, format and byte-budget checks in +`src/images/artifacts.ts`. Files use random names, exclusive creation and mode 0600; +the artifact directory uses mode 0700 on creation. Per-response display state is capped +at 128 image items, with the existing 50 MiB per-image and 100 MiB aggregate decoded +limits. Exceeding the item cap returns HTTP 502 before streaming or a failed SSE +terminal after streaming has started. Retention runs after the batch. Failed or invalid results produce a bounded +display message without reflecting payloads or filesystem errors. URL results and +partial-image previews are not downloaded or rendered by this projection. + ## Standalone Images Codex's local `image_gen.imagegen` tool makes a second Images request after the model calls it: diff --git a/structure/manifest.json b/structure/manifest.json index 0057a6464e4..9bc2e553cfc 100644 --- a/structure/manifest.json +++ b/structure/manifest.json @@ -235,7 +235,7 @@ "path": "data-planes/images.md", "tier": 3, "title": "Images Data Plane", - "scope": "Standalone image generation and edit relay.", + "scope": "Standalone image generation/edit relay and local hosted Responses image display.", "documents": [ "src/server/" ] diff --git a/structure/transports/responses-wire-shapes.md b/structure/transports/responses-wire-shapes.md index 7944955f31b..9f209e57838 100644 --- a/structure/transports/responses-wire-shapes.md +++ b/structure/transports/responses-wire-shapes.md @@ -366,6 +366,8 @@ destination receives unchanged whenever a caller sends one itself. ## Passthrough SSE stream shapes (#314) +Local Codex hosted-image results use the [client-only image projection](../data-planes/images.md#hosted-responses-image-display) after continuation-cache observers. It emits consistent message lifecycle events and terminal snapshots with `phase: final_answer`, suppresses replaced hosted-image progress frames, and adjusts numeric sequence numbers for inserted or removed events. Both relay shapes apply the same projection. + Native passthrough SSE has TWO shapes, selected per request in `src/server/responses/core.ts`: diff --git a/tests/fixtures/hosted-image-display.ts b/tests/fixtures/hosted-image-display.ts new file mode 100644 index 00000000000..04653096c53 --- /dev/null +++ b/tests/fixtures/hosted-image-display.ts @@ -0,0 +1,5 @@ +// Synthetic one-pixel PNG. No captured request, provider, account, or image data. +export const item = { + id: 'ig_synthetic', type: 'image_generation_call', status: 'completed', + result: 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+jRZkAAAAASUVORK5CYII=', +}; diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 784f61f6887..ab70a5d66d1 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1,4 +1,6 @@ { + "responses-hosted-image-delivery.test.ts": "responses", + "responses-hosted-image-display.test.ts": "responses", "cursor-local-installer.test.ts": "providers/cursor", "codex-quota-query-backoff.test.ts": "codex-integration", "link-relay-bound-transport.test.ts": "clients", diff --git a/tests/responses/responses-hosted-image-delivery.test.ts b/tests/responses/responses-hosted-image-delivery.test.ts new file mode 100644 index 00000000000..d6ba2065c5e --- /dev/null +++ b/tests/responses/responses-hosted-image-delivery.test.ts @@ -0,0 +1,119 @@ +import { expect, test } from 'bun:test'; +import { readFileSync, statSync } from 'node:fs'; +import { deliverPassthroughResponse, setRelayPlatformForTests } from "../../src/server/responses/passthrough-delivery"; +import { createTranslatorBudget } from "../../src/lib/translator-budget"; +import { createTempHome, type TempHome } from "../helpers/temp-home"; +import { beforeEach, afterEach } from "bun:test"; +let home: TempHome; +beforeEach(() => { home = createTempHome("hosted-image-delivery-"); }); +afterEach(() => { setRelayPlatformForTests(undefined); home.remove(); }); + +import { item } from "../fixtures/hosted-image-display"; +const finalMessage = { id: 'msg_final_fixture', type: 'message', role: 'assistant', status: 'completed', phase: 'final_answer', content: [{ type: 'output_text', text: 'Done.', annotations: [] }] }; +const response = { id: 'resp_image_fixture', object: 'response', model: 'fixture', created_at: 1790666489, status: 'completed', output: [item, finalMessage] }; +const nl = String.fromCharCode(10); + +const cases: Array<{ platform: 'darwin' | 'linux' | 'win32'; format: string; client: string; cache: string }> = []; +for (const platform of ['darwin', 'linux', 'win32'] as const) { + for (const format of ['sse', 'json', 'json-to-sse']) { + for (const client of ['local', 'generic', 'remote']) cases.push({ platform, format, client, cache: 'raw' }); + } + for (const cache of ['plaintext', 'envelope']) cases.push({ platform, format: 'sse', client: 'local', cache }); +} +for (const platform of ['darwin', 'linux', 'win32'] as const) { + for (const format of ['sse', 'json', 'json-to-sse']) cases.push({ platform, format, client: 'local', cache: 'overflow' }); +} +for (const { platform, format, client, cache } of cases) { + const local = client === 'local'; + test(platform + '/' + format + '/' + client + '/' + cache, async () => { + setRelayPlatformForTests(platform); + const upstream = new AbortController(); + const budget = createTranslatorBudget(); + const remembered: any[] = []; + let resolveRemembered!: () => void; + const didRemember = new Promise(resolve => { resolveRemembered = resolve; }); + const currentResponse = cache === 'overflow' + ? { ...response, output: Array.from({ length: 129 }, (_, i) => ({ ...item, id: 'ig_limit_' + i, status: 'failed', result: null })) } + : response; + const payload = format !== 'sse' ? JSON.stringify(currentResponse) : [ + { type: 'response.created', response: { ...response, status: 'in_progress', output: [] } }, + { type: 'response.output_item.added', output_index: 0, item: { ...item, status: 'in_progress', result: null } }, + { type: 'response.output_item.done', output_index: 0, item }, + { type: 'response.output_item.added', output_index: 1, item: { ...finalMessage, status: 'in_progress', content: [] } }, + { type: 'response.output_item.done', output_index: 1, item: finalMessage }, + { type: 'response.completed', response: currentResponse }, + ].map(e => 'data: ' + JSON.stringify(e) + nl + nl).join('') + 'data: [DONE]' + nl + nl; + const req = new Request('http://127.0.0.1:10100/v1/responses', { + headers: client !== 'generic' ? { originator: 'Codex Desktop' } : {}, + }); + const native = { + upstreamResponse: new Response(payload, { headers: { 'content-type': format === 'sse' ? 'text/event-stream' : 'application/json' } }), + upstream, request: req, connectMs: 1000, + imageGenCallAliases: new Map(), selfNamedNamespaceScrubAuthorization: new Set(), authorizedBareNamespaceToolAliases: new Map(), + routedCustomToolNames: new Set(), routedCustomToolRepairNames: new Set(), declaredWireToolNames: new Set(), + routedToolSearchNames: new Set(), functionRepairSchemas: new Map(), declaredNamelessClientCallTypes: new Set(), + providerExecutedCallTypes: new Set(), declaredBareWireToolNames: new Set(), undeclaredToolGuardActive: false, + rememberPassthroughResponse: true, rememberPassthroughResponseChecked: (value: any) => { remembered.push(value); resolveRemembered(); }, + normalizeFunctionCompletionJson: (value: string) => value, + outboundRequestBody: '{}', + }; + try { + const result = await deliverPassthroughResponse( + { req, config: { providers: {}, streamMode: 'auto' }, logCtx: {}, options: { admission: { kind: client === 'remote' ? 'configured' : 'loopback', source: 'loopback' } } } as any, + { authCtx: { kind: 'none' } } as any, + { parsed: { stream: format === 'sse', modelId: 'fixture', _rawBody: { previous_response_id: 'resp_synthetic_prior' } }, clientRequestedStream: format !== 'json', translatorBudget: budget, + route: { provider: { adapter: 'openai-responses', ...(cache === 'envelope' ? { baseUrl: 'https://api.x.ai/v1' } : {}) }, providerName: 'fixture', modelId: 'fixture', staticPolicy: { model: format === 'json-to-sse' ? { responsesUpstreamStreaming: false } : {} } } } as any, + { requestBindings: new Map() } as any, + {} as any, + { plaintextV2AgentMessageToolNames: new Set(cache === 'plaintext' ? ['synthetic_agent'] : []), plaintextV2AgentMessageAliasedToolNames: new Set(), + routedMuseToolNameAliases: new Map(), routedNamespaceToolAliases: new Map(), + commitReasoningReplayServingRoute: () => {}, recordTerminalOutcomes: false } as any, + native as any, + ); + if (cache === 'overflow' && format !== 'sse') { + expect(result.status).toBe(502); + const body = await result.text(); + expect(body).toContain('hosted image result count exceeds local display limit'); + expect(body).not.toContain(home.root); + return; + } + expect(result.status).toBe(200); + expect(result.headers.get('content-type')?.includes('text/event-stream')).toBe(format !== 'json'); + const body = await result.text(); + if (cache === 'overflow') { + const terminal = body.split(nl).filter(l => l.startsWith('data: {')) + .map(l => JSON.parse(l.slice(6))).find(e => e.type === 'response.failed'); + expect(terminal).toBeDefined(); + expect(body).not.toContain(item.result); + return; + } + const completed = format === 'json' ? JSON.parse(body) : body.split(nl).filter(l => l.startsWith('data: {')) + .map(l => JSON.parse(l.slice(6))).find(e => e.type === 'response.completed')?.response; + expect(completed?.output[0]?.type).toBe(local ? 'message' : 'image_generation_call'); + if (local) { + expect(completed.output[0].phase).toBe('final_answer'); + expect(completed.output[1]).toEqual(finalMessage); + if (format !== 'json') { + const lifecycle = body.split(nl).filter(l => l.startsWith('data: {')).map(l => JSON.parse(l.slice(6))) + .filter(e => e.item?.id === completed.output[0].id); + // Bounded JSON synthesis intentionally emits only output_item.done. + if (format === 'sse') expect(lifecycle.some(e => e.type === 'response.output_item.added')).toBe(true); + expect(lifecycle.some(e => e.type === 'response.output_item.done')).toBe(true); + for (const event of lifecycle) expect(event.item.phase).toBe('final_answer'); + } + const path = completed.output[0].content[0].text.match(/<([^>]+)>/)?.[1]; + expect(path).toBeDefined(); + expect(readFileSync(path)).toEqual(Buffer.from(item.result, 'base64')); + if (process.platform !== 'win32') expect(statSync(path).mode & 0o777).toBe(0o600); + expect(body).not.toContain(item.result); + } + else expect(completed.output[0].result).toBe(item.result); + await didRemember; + expect(remembered.some(r => r.output?.[0]?.type === 'image_generation_call')).toBe(true); + expect(JSON.stringify(remembered)).not.toContain('img-codex-'); + } finally { + upstream.abort(); + budget.dispose(); + } + }); +} diff --git a/tests/responses/responses-hosted-image-display.test.ts b/tests/responses/responses-hosted-image-display.test.ts new file mode 100644 index 00000000000..f024dfd0e15 --- /dev/null +++ b/tests/responses/responses-hosted-image-display.test.ts @@ -0,0 +1,198 @@ +import { expect, test } from 'bun:test'; +import { existsSync, readFileSync, readdirSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; + +import { isAbsolute } from "node:path"; +import { beforeEach, afterEach } from "bun:test"; +import { createTempHome, type TempHome } from "../helpers/temp-home"; +import { item } from "../fixtures/hosted-image-display"; +import { createHostedImageDisplayRewrite as factory, isLocalCodexImageClient } from "../../src/server/responses-hosted-image-display"; +let home: TempHome; +beforeEach(() => { home = createTempHome("hosted-image-display-"); }); +afterEach(() => { home.remove(); }); + +const block = (event: unknown) => 'data: ' + JSON.stringify(event); +const events = (blocks: readonly string[]) => blocks.map(b => JSON.parse(b.split('data: ')[1])); + +test('completed hosted image becomes a visible message pointing to identical saved bytes', () => { + const rewrite = factory(); + const result = events(rewrite(block({ type: 'response.output_item.done', output_index: 0, item }))); + const message = result.find(e => e.type === 'response.output_item.done')?.item; + expect(message?.type).toBe('message'); + expect(message?.phase).toBe('final_answer'); + const path = message.content[0].text.match(/!\[.*?\]\(<(.*?)>\)/)?.[1]; + expect(isAbsolute(path)).toBe(true); + expect(readFileSync(path)).toEqual(Buffer.from(item.result, 'base64')); + const terminal = events(rewrite(block({ type: 'response.completed', response: { status: 'completed', output: [item] } }))); + expect(terminal.at(-1).response.output[0]).toEqual(message); + expect(terminal.filter(e => e.type === 'response.output_item.done')).toHaveLength(0); + rewrite.dispose?.(); +}); + +test('added/done/terminal have one consistent message identity and monotonic sequence numbers', () => { + const rewrite = factory(); + const values = [ + { type: 'response.output_item.added', sequence_number: 0, output_index: 0, item: { ...item, status: 'in_progress', result: null } }, + { type: 'response.image_generation_call.in_progress', sequence_number: 1, output_index: 0, item_id: item.id }, + { type: 'response.output_item.done', sequence_number: 2, output_index: 0, item }, + { type: 'response.completed', sequence_number: 3, response: { status: 'completed', output: [item] } }, + ]; + const result = values.flatMap(e => events(rewrite(block(e)))); + expect(result.map(e => e.sequence_number)).toEqual(result.map((_, i) => i)); + expect(result.filter(e => e.type === 'response.output_item.added')).toHaveLength(1); + expect(result.filter(e => e.type === 'response.output_item.done')).toHaveLength(1); + const done = result.find(e => e.type === 'response.output_item.done').item; + expect(result[0].item.phase).toBe('final_answer'); + expect(done.phase).toBe('final_answer'); + expect(result[0].item.id).toBe(done.id); + expect(result.at(-1).response.output[0]).toEqual(done); + expect(result.find(e => e.type === 'response.output_text.delta').delta).toBe(done.content[0].text); + rewrite.dispose?.(); +}); + +test('terminal-only responses emit the missing complete message lifecycle', () => { + const rewrite = factory(); + const result = events(rewrite(block({ type: 'response.completed', response: { status: 'completed', output: [item] } }))); + expect(result.map(e => e.type)).toEqual([ + 'response.output_item.added', 'response.content_part.added', 'response.output_text.delta', + 'response.output_text.done', 'response.content_part.done', 'response.output_item.done', 'response.completed', + ]); + expect(result[0].item.phase).toBe('final_answer'); + expect(result.at(-1).response.output[0].phase).toBe('final_answer'); + rewrite.dispose?.(); +}); + +test('invalid image bytes produce a visible failure, never false success or raw data', () => { + const rewrite = factory(); + const result = events(rewrite(block({ type: 'response.output_item.done', output_index: 0, item: { ...item, result: 'invalid-secret-looking-data' } }))); + const message = result.at(-1).item; + expect(message.status).toBe('incomplete'); + expect(message.content[0].text).toContain('could not be saved'); + expect(JSON.stringify(result)).not.toContain('invalid-secret-looking-data'); + rewrite.dispose?.(); +}); + +test('text and ordinary tool calls remain byte-identical', () => { + const rewrite = factory(); + for (const value of [ + { type: 'response.output_item.done', item: { type: 'function_call', name: 'imagegen', namespace: 'image_gen', arguments: '{}' } }, + { type: 'response.output_text.delta', delta: '你好', sequence_number: 4 }, + { type: 'response.completed', response: { status: 'completed', output: [] }, sequence_number: 5 }, + ]) expect(rewrite(block(value))).toEqual([block(value)]); + expect(rewrite('data: [DONE]')).toEqual(['data: [DONE]']); + rewrite.dispose?.(); +}); + +test('non-streaming results have the same visible artifact contract', () => { + const rewrite = factory(); + const result = JSON.parse(rewrite.json(JSON.stringify({ status: 'completed', output: [item] }))); + expect(result.output[0].type).toBe('message'); + expect(result.output[0].phase).toBe('final_answer'); + expect(result.output[0].content[0].text).toContain('![Generated image]'); + rewrite.dispose?.(); +}); + +test('filesystem paths are never enabled for remote or generic API clients', async () => { + const codex = new Headers({ originator: 'Codex Desktop' }); + expect(isLocalCodexImageClient(codex, 'loopback')).toBe(true); + expect(isLocalCodexImageClient(codex, 'configured')).toBe(false); + expect(isLocalCodexImageClient(new Headers(), 'loopback')).toBe(false); + expect(isLocalCodexImageClient(codex, 'loopback', 'anthropic')).toBe(false); + expect(isLocalCodexImageClient(codex)).toBe(false); + expect(isLocalCodexImageClient(new Headers({ 'user-agent': 'codex_cli_rs/1.0' }), 'loopback')).toBe(true); + expect(isLocalCodexImageClient(new Headers({ 'user-agent': 'not-codex' }), 'loopback')).toBe(false); +}); + +test('multiple images keep distinct identities and create one artifact per image', () => { + const rewrite = factory(); + try { + const images = [item, { ...item, id: 'ig_second' }]; + const response = { status: 'completed', output: images }; + const first = JSON.parse(rewrite.json(JSON.stringify(response))); + const second = JSON.parse(rewrite.json(JSON.stringify(response))); + expect(first).toEqual(second); + expect(first.output[0].id).not.toBe(first.output[1].id); + expect(first.output[0].content).not.toEqual(first.output[1].content); + expect(readdirSync(home.path('artifacts'))).toHaveLength(2); + expect(response.output).toEqual(images); + for (const message of first.output) expect(message.phase).toBe('final_answer'); + } finally { rewrite.dispose?.(); } +}); + +test('a failed disk write returns a bounded message without leaking filesystem details', () => { + writeFileSync(home.path('artifacts'), 'not a directory'); + const rewrite = factory(); + try { + const output = JSON.parse(rewrite.json(JSON.stringify({ output: [item] }))).output[0]; + expect(output.status).toBe('incomplete'); + expect(output.content[0].text).toBe('The image result could not be saved for local display.'); + expect(JSON.stringify(output)).not.toContain(home.root); + expect(JSON.stringify(output)).not.toContain(item.result); + } finally { rewrite.dispose?.(); } +}); + +test('incomplete and failed terminals retain their status without claiming an image', () => { + for (const status of ['incomplete', 'failed']) { + const rewrite = factory(); + try { + const result = events(rewrite(block({ + type: 'response.' + status, + response: { status, output: [{ ...item, status, result: null }] }, + }))); + const response = result.at(-1).response; + expect(response.status).toBe(status); + expect(response.output[0].status).toBe('incomplete'); + expect(response.output[0].phase).toBe('final_answer'); + expect(response.output[0].content[0].text).not.toContain('!['); + } finally { rewrite.dispose?.(); } + } +}); + +test('preserves routing metadata on the displayed message', () => { + const rewrite = factory(); + try { + const metadata = { synthetic: true }; + const response = JSON.parse(rewrite.json(JSON.stringify({ + output: [{ ...item, internal_chat_message_metadata_passthrough: metadata }], + }))); + expect(response.output[0].internal_chat_message_metadata_passthrough).toEqual(metadata); + } finally { rewrite.dispose?.(); } +}); + +test('completed URL-only or missing image data is reported without claiming generation failed', () => { + for (const extra of [{ url: 'https://example.test/image.png' }, {}]) { + const rewrite = factory(); + try { + const unsupported = { ...item, result: null, ...extra }; + const added = events(rewrite(block({ type: 'response.output_item.added', output_index: 0, + item: { ...unsupported, status: 'in_progress' } })))[0].item; + const done = events(rewrite(block({ type: 'response.output_item.done', output_index: 0, item: unsupported }))).at(-1).item; + expect(done.id).toBe(added.id); + expect(done.content[0].text).toBe('The completed image result has no supported image data for local display.'); + expect(done.status).toBe('incomplete'); + expect(JSON.parse(rewrite.json(JSON.stringify({ output: [unsupported] }))).output[0]).toEqual(done); + expect(JSON.stringify(done)).not.toContain('https://'); + } finally { rewrite.dispose?.(); } + } +}); + +test('oversized JSON snapshots are rejected before creating any image artifacts', () => { + const rewrite = factory(); + try { + const output = Array.from({ length: 129 }, (_, i) => ({ ...item, id: 'ig_limit_' + i })); + expect(() => rewrite.json(JSON.stringify({ output }))).toThrow('hosted image result count exceeds local display limit'); + expect(existsSync(home.path('artifacts'))).toBe(false); + } finally { rewrite.dispose?.(); } +}); + +test('non-image JSON and malformed SSE pass through unchanged', () => { + const rewrite = factory(); + try { + for (const text of ['not json', 'null', '[]', '{ "output": [] }']) { + expect(rewrite.json(text)).toBe(text); + } + for (const value of ['data: {', ': heartbeat', 'data: null', 'data: []']) { + expect(rewrite(value)).toEqual([value]); + } + } finally { rewrite.dispose?.(); } +}); From 23e67487e5055dea66de0c81d2cec5de54cf9830 Mon Sep 17 00:00:00 2001 From: chiperman <34889778+chiperman@users.noreply.github.com> Date: Tue, 29 Sep 2026 18:48:12 +0800 Subject: [PATCH 2/3] fix(responses): redact local image paths from replayed history - Replace generated image paths with opaque artifact references before request dispatch. - Cover full-history replay and pruned artifacts with regression tests and document the display boundary. --- .../content/docs/guides/codex-integration.md | 5 +++ src/server/responses-hosted-image-display.ts | 26 +++++++++++- src/server/responses/request-prepare.ts | 4 ++ structure/data-planes/images.md | 8 ++++ structure/transports/responses-wire-shapes.md | 4 ++ .../responses-hosted-image-delivery.test.ts | 42 +++++++++++++++++++ .../responses-hosted-image-display.test.ts | 35 +++++++++++++++- 7 files changed, 121 insertions(+), 3 deletions(-) diff --git a/docs-site/src/content/docs/guides/codex-integration.md b/docs-site/src/content/docs/guides/codex-integration.md index 4d8df0891a9..fea39ee3fad 100644 --- a/docs-site/src/content/docs/guides/codex-integration.md +++ b/docs-site/src/content/docs/guides/codex-integration.md @@ -234,6 +234,11 @@ connected Codex client. The image stays outside the collapsible progress section This applies to streaming and non-streaming Responses, without another generation request or a change to the selected provider. +When a client replays these generated image messages as assistant history, opencodex +replaces its generated local image links with opaque artifact references before +forwarding that history. This protects the display paths without changing the image +message already shown in the app. It does not redact unrelated user-supplied paths. + This display compatibility requires loopback admission and a recognized Codex client. Remote and generic API clients retain the provider's hosted response format. Partial previews and URL-only results are not rendered by this compatibility layer. diff --git a/src/server/responses-hosted-image-display.ts b/src/server/responses-hosted-image-display.ts index c69c9a8733d..cf572a0687e 100644 --- a/src/server/responses-hosted-image-display.ts +++ b/src/server/responses-hosted-image-display.ts @@ -5,7 +5,7 @@ import { getConfigDir } from "../config"; import { recordOwnedConfigPath } from "../lib/config-ownership"; import { chargeImageBudget, createImageBudget, decodeValidatedImageBase64, - getArtifactsDir, MAX_ENCODED_BYTES_PER_IMAGE, pruneArtifacts, sniffImageExtension, + artifactHttpUrl, getArtifactsDir, MAX_ENCODED_BYTES_PER_IMAGE, pruneArtifacts, sniffImageExtension, } from "../images/artifacts"; import { sseDataPayload, replaceSseDataPayload, type SseBlockRewrite } from "./sse-payload-rewrite"; @@ -13,6 +13,30 @@ type Row = Record; const object = (v: unknown): v is Row => !!v && typeof v === "object" && !Array.isArray(v); const originators = new Set(["codex_cli_rs", "Codex Desktop", "codex_app", "codex_work_desktop"]); +/** Full client replay can include display-only messages even without their generated ids. */ +export function redactHostedImageDisplayPaths(body: unknown): void { + if (!object(body) || !Array.isArray(body.input)) return; + const prefix = join(getArtifactsDir(), "img-codex-"); + const generatedLink = /!\[Generated image\]\(<([^>\r\n]+)>\)/g; + const redact = (text: string) => text.replace(generatedLink, (link, path: string) => { + if (!path.startsWith(prefix) + || !/^[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}\.(png|jpg|webp|gif)$/.test(path.slice(prefix.length))) return link; + // No file read or existence check: history may outlive artifact retention. + return "![Generated image](<" + artifactHttpUrl(path) + ">)"; + }); + for (const item of body.input) { + if (!object(item) || item.role !== "assistant" + || (item.type !== undefined && item.type !== "message")) continue; + if (typeof item.content === "string") item.content = redact(item.content); + else if (Array.isArray(item.content)) { + for (const part of item.content) { + if (object(part) && ["output_text", "input_text", "text"].includes(part.type) + && typeof part.text === "string") part.text = redact(part.text); + } + } + } +} + /** Local filesystem links are only appropriate for the loopback Codex client. */ export function isLocalCodexImageClient(headers: Headers, admissionKind?: string, inboundWire?: string): boolean { return admissionKind === "loopback" diff --git a/src/server/responses/request-prepare.ts b/src/server/responses/request-prepare.ts index 60e0ed6fa38..d96ca4f83d3 100644 --- a/src/server/responses/request-prepare.ts +++ b/src/server/responses/request-prepare.ts @@ -146,6 +146,8 @@ import { conversationCarriesUploadedFiles, } from "./account-change-state"; +import { redactHostedImageDisplayPaths } from "../responses-hosted-image-display"; + /** Parses, selects, and admits one request without changing the dispatch policy. */ export async function prepareResponsesRequest( requestContext: Pick, @@ -168,6 +170,8 @@ export async function prepareResponsesRequest( } return decodeRequestErrorResponse(err, "responses"); } + // Remove display-only artifact paths before helpers, combo dispatch and upstream replay. + redactHostedImageDisplayPaths(body); observeCacheDiagnosticInbound( logCtx, body, diff --git a/structure/data-planes/images.md b/structure/data-planes/images.md index cd23c8050dd..704d69cf417 100644 --- a/structure/data-planes/images.md +++ b/structure/data-planes/images.md @@ -28,6 +28,14 @@ Hosted items in the continuation cache retain their upstream representation; oth client rewrites keep their existing cache policy. Generic, remotely admitted and non-Responses clients do not receive this filesystem projection. +Full-history assistant messages can replay the display Markdown without their generated +item ids. At Responses request preparation, exact generated links under the current +artifact directory become opaque artifact HTTP references before routing and helper +dispatch. This does not read files and still applies after artifact pruning; unrelated +paths, user messages and tool payloads retain their original content. Local display +uses filesystem links because remote Markdown media has a separate client safety gate; +HTTP references here are for upstream context, not a claim of desktop HTTP rendering. + Image data must pass the shared base64, format and byte-budget checks in `src/images/artifacts.ts`. Files use random names, exclusive creation and mode 0600; the artifact directory uses mode 0700 on creation. Per-response display state is capped diff --git a/structure/transports/responses-wire-shapes.md b/structure/transports/responses-wire-shapes.md index 9f209e57838..5bac49d67ce 100644 --- a/structure/transports/responses-wire-shapes.md +++ b/structure/transports/responses-wire-shapes.md @@ -368,6 +368,10 @@ destination receives unchanged whenever a caller sends one itself. Local Codex hosted-image results use the [client-only image projection](../data-planes/images.md#hosted-responses-image-display) after continuation-cache observers. It emits consistent message lifecycle events and terminal snapshots with `phase: final_answer`, suppresses replaced hosted-image progress frames, and adjusts numeric sequence numbers for inserted or removed events. Both relay shapes apply the same projection. +Responses request preparation replaces exact generated artifact links in replayed assistant +messages with opaque HTTP references before dispatch, so full client history does not +forward these display-only filesystem paths. The raw hosted continuation cache stays unchanged. + Native passthrough SSE has TWO shapes, selected per request in `src/server/responses/core.ts`: diff --git a/tests/responses/responses-hosted-image-delivery.test.ts b/tests/responses/responses-hosted-image-delivery.test.ts index d6ba2065c5e..8c631629a08 100644 --- a/tests/responses/responses-hosted-image-delivery.test.ts +++ b/tests/responses/responses-hosted-image-delivery.test.ts @@ -9,10 +9,52 @@ beforeEach(() => { home = createTempHome("hosted-image-delivery-"); }); afterEach(() => { setRelayPlatformForTests(undefined); home.remove(); }); import { item } from "../fixtures/hosted-image-display"; +import { handleResponses } from "../../src/server/responses"; +import { createHostedImageDisplayRewrite } from "../../src/server/responses-hosted-image-display"; +import { acquireOwnedSpendHome } from "../helpers/owned-spend-home"; +import { artifactHttpUrl } from "../../src/images/artifacts"; +import type { OcxConfig } from "../../src/types"; const finalMessage = { id: 'msg_final_fixture', type: 'message', role: 'assistant', status: 'completed', phase: 'final_answer', content: [{ type: 'output_text', text: 'Done.', annotations: [] }] }; const response = { id: 'resp_image_fixture', object: 'response', model: 'fixture', created_at: 1790666489, status: 'completed', output: [item, finalMessage] }; const nl = String.fromCharCode(10); +test('full client history replay removes generated artifact paths before upstream dispatch', async () => { + const release = acquireOwnedSpendHome(); + const originalFetch = globalThis.fetch; + const rewrite = createHostedImageDisplayRewrite(); + const requests: any[] = []; + try { + const message = JSON.parse(rewrite.json(JSON.stringify({ output: [item] }))).output[0]; + const displayedText = message.content[0].text; + const path = displayedText.match(/<([^>]+)>/)[1]; + globalThis.fetch = (async (_url: unknown, init?: RequestInit) => { + requests.push(JSON.parse(String(init?.body))); + return Response.json({ id: 'resp_replay_fixture', status: 'completed', output: [] }); + }) as typeof fetch; + const config = { port: 0, defaultProvider: 'fixture', providers: { fixture: { + adapter: 'openai-responses', baseUrl: 'https://fixture.test/v1', authMode: 'key', apiKey: 'fixture-key', + } } } as OcxConfig; + for (const content of [message.content, displayedText]) { + // Clients may omit the synthetic item id when serializing their full history. + const result = await handleResponses(new Request('http://localhost/v1/responses', { + method: 'POST', headers: { 'content-type': 'application/json', originator: 'Codex Desktop' }, + body: JSON.stringify({ model: 'fixture/fixture-model', stream: false, + input: [{ role: 'assistant', content }, { role: 'user', content: 'Describe the previous result.' }] }), + }), config, { model: '', provider: '' }, { admission: { kind: 'loopback', source: 'loopback' }, inboundWire: 'responses' }); + await result.text(); + expect(result.status).toBe(200); + } + expect(requests).toHaveLength(2); + for (const request of requests) { + expect(JSON.stringify(request.input)).not.toContain(home.root); + expect(JSON.stringify(request.input)).toContain(artifactHttpUrl(path)); + expect(JSON.stringify(request.input)).toContain('Describe the previous result.'); + } + expect(message.content[0].text).toBe(displayedText); + expect(readFileSync(path)).toEqual(Buffer.from(item.result, 'base64')); + } finally { globalThis.fetch = originalFetch; rewrite.dispose?.(); release(); } +}); + const cases: Array<{ platform: 'darwin' | 'linux' | 'win32'; format: string; client: string; cache: string }> = []; for (const platform of ['darwin', 'linux', 'win32'] as const) { for (const format of ['sse', 'json', 'json-to-sse']) { diff --git a/tests/responses/responses-hosted-image-display.test.ts b/tests/responses/responses-hosted-image-display.test.ts index f024dfd0e15..830a01490c3 100644 --- a/tests/responses/responses-hosted-image-display.test.ts +++ b/tests/responses/responses-hosted-image-display.test.ts @@ -1,12 +1,12 @@ import { expect, test } from 'bun:test'; -import { existsSync, readFileSync, readdirSync, writeFileSync } from 'node:fs'; +import { existsSync, readFileSync, readdirSync, unlinkSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { isAbsolute } from "node:path"; import { beforeEach, afterEach } from "bun:test"; import { createTempHome, type TempHome } from "../helpers/temp-home"; import { item } from "../fixtures/hosted-image-display"; -import { createHostedImageDisplayRewrite as factory, isLocalCodexImageClient } from "../../src/server/responses-hosted-image-display"; +import { createHostedImageDisplayRewrite as factory, isLocalCodexImageClient, redactHostedImageDisplayPaths } from "../../src/server/responses-hosted-image-display"; let home: TempHome; beforeEach(() => { home = createTempHome("hosted-image-display-"); }); afterEach(() => { home.remove(); }); @@ -196,3 +196,34 @@ test('non-image JSON and malformed SSE pass through unchanged', () => { } } finally { rewrite.dispose?.(); } }); + +test('replay redaction survives pruning and preserves unrelated paths, roles and tool payloads', () => { + const rewrite = factory(); + try { + const message = JSON.parse(rewrite.json(JSON.stringify({ output: [item] }))).output[0]; + const text = message.content[0].text; + const path = text.match(/<([^>]+)>/)[1]; + unlinkSync(path); + const unrelated = '![Generated image](<' + home.path('private.png') + '>)'; + const unowned = text.replace('img-codex-', 'img-other-'); + const body = { input: [ + { role: 'assistant', content: [{ type: 'input_text', text: text + '\n' + unrelated + '\n' + unowned }] }, + { role: 'user', content: text }, + { type: 'function_call_output', output: text }, + ] }; + redactHostedImageDisplayPaths(body); + const cleaned = body.input[0].content as Array<{ text: string }>; + expect(cleaned[0].text).not.toContain(path); + expect(cleaned[0].text).toContain('/v1/opencodex/artifacts/img-codex-'); + expect(cleaned[0].text).toContain(unrelated); + expect(cleaned[0].text).toContain(unowned); + expect(body.input[1].content).toBe(text); + expect(body.input[2].output).toBe(text); + const first = JSON.stringify(body); + redactHostedImageDisplayPaths(body); + expect(JSON.stringify(body)).toBe(first); + for (const malformed of [null, [], {}, { input: [null, false, { role: 'assistant', content: [null, {}] }] }]) { + expect(() => redactHostedImageDisplayPaths(malformed)).not.toThrow(); + } + } finally { rewrite.dispose?.(); } +}); From e9b710fd90ef73d2bc0a860336a0e3713c12114d Mon Sep 17 00:00:00 2001 From: chiperman <34889778+chiperman@users.noreply.github.com> Date: Tue, 29 Sep 2026 19:31:00 +0800 Subject: [PATCH 3/3] fix(responses): preserve image events without an identity --- src/server/responses-hosted-image-display.ts | 6 ++- structure/data-planes/images.md | 4 +- .../responses-hosted-image-display.test.ts | 40 +++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/src/server/responses-hosted-image-display.ts b/src/server/responses-hosted-image-display.ts index cf572a0687e..497011cdf9f 100644 --- a/src/server/responses-hosted-image-display.ts +++ b/src/server/responses-hosted-image-display.ts @@ -156,8 +156,10 @@ export function createHostedImageDisplayRewrite(): SseBlockRewrite & { json(text try { event = JSON.parse(payload); } catch { return [block]; } if (!object(event)) return [block]; let result: Row[] = [event]; - const index = Number.isSafeInteger(event.output_index) && event.output_index >= 0 ? event.output_index : 0; - if (object(event.item) && event.item.type === "image_generation_call") { + const hasIndex = Number.isSafeInteger(event.output_index) && event.output_index >= 0; + const index = hasIndex ? event.output_index : 0; + if (object(event.item) && event.item.type === "image_generation_call" + && (hasIndex || typeof event.item.id === "string")) { if (event.type === "response.output_item.added") { const state = stateFor(event.item, index); result = state.added ? [] : [{ ...event, item: state.message }]; diff --git a/structure/data-planes/images.md b/structure/data-planes/images.md index 704d69cf417..3fcdcf73248 100644 --- a/structure/data-planes/images.md +++ b/structure/data-planes/images.md @@ -26,7 +26,9 @@ artifacts. The final phase keeps the image outside collapsible progress output. observers, on the client branch only, for SSE, JSON and JSON synthesized into SSE. Hosted items in the continuation cache retain their upstream representation; other client rewrites keep their existing cache policy. Generic, remotely admitted and non-Responses -clients do not receive this filesystem projection. +clients do not receive this filesystem projection. Individual image item events without +a string item id or a valid non-negative integer output index pass through without +allocating display state, so unidentified items cannot collide at a synthetic index. Full-history assistant messages can replay the display Markdown without their generated item ids. At Responses request preparation, exact generated links under the current diff --git a/tests/responses/responses-hosted-image-display.test.ts b/tests/responses/responses-hosted-image-display.test.ts index 830a01490c3..1f8c9c657cf 100644 --- a/tests/responses/responses-hosted-image-display.test.ts +++ b/tests/responses/responses-hosted-image-display.test.ts @@ -50,6 +50,46 @@ test('added/done/terminal have one consistent message identity and monotonic seq rewrite.dispose?.(); }); +test('image events without an identity pass through without consuming shared state', () => { + const rewrite = factory(); + try { + for (const output_index of [undefined, -1, 0.5, '0', null]) { + for (const id of [undefined, 123]) { + for (const type of ['response.output_item.added', 'response.output_item.done']) { + const value = block({ type, output_index, item: { ...item, id } }); + expect(rewrite(value)).toEqual([value]); + } + } + } + expect(existsSync(home.path('artifacts'))).toBe(false); + const result = events(rewrite(block({ type: 'response.output_item.done', output_index: 0, + item: { ...item, id: undefined } }))); + expect(result.at(-1).item.type).toBe('message'); + expect(result.at(-1).output_index).toBe(0); + expect(readdirSync(home.path('artifacts'))).toHaveLength(1); + } finally { rewrite.dispose?.(); } +}); + +test('image events with either an index or a string id retain distinct display state', () => { + const rewrite = factory(); + try { + const results = [ + { output_index: 1, item: { ...item, id: undefined } }, + { output_index: 2, item: { ...item, id: undefined } }, + { item }, + { item: { ...item, id: 'ig_second' } }, + ].map(value => { + const added = events(rewrite(block({ ...value, type: 'response.output_item.added' })))[0].item; + const done = events(rewrite(block({ ...value, type: 'response.output_item.done' }))).at(-1).item; + expect(done.id).toBe(added.id); + expect(done.type).toBe('message'); + return done.id; + }); + expect(new Set(results).size).toBe(4); + expect(readdirSync(home.path('artifacts'))).toHaveLength(4); + } finally { rewrite.dispose?.(); } +}); + test('terminal-only responses emit the missing complete message lifecycle', () => { const rewrite = factory(); const result = events(rewrite(block({ type: 'response.completed', response: { status: 'completed', output: [item] } })));