-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[agent] fix: refuse lossy media conversion at the final adapter boundary #4562
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
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,20 @@ | ||
| # Direct implementation: media admission | ||
|
|
||
| This fifth layer follows PR #4539. ChatGPT authored the production changes and regression | ||
| tests directly in an isolated worktree, rather than handing this implementation to the prior | ||
| native authoring session. The outcome closes the silent-success part of F5, not native media | ||
| transport: recognized audio/file inputs either stay on an existing native wire or receive an | ||
| explicit conversion error. Legacy function-image conversion also refuses instead of losing | ||
| its result. The canonical current contract is in | ||
| [adapter registry](../../../structure/adapters/registry.md#untranslated-input-media). | ||
|
|
||
| The pure scanner inspects typed content arrays only. The registry owns final translated | ||
| build/runTurn/local-completion admission, and Chat owns rejection before a lossy projection. | ||
| No new fetch, decoding, credential access, provider capability declarations or vendor CLI | ||
| permissions are introduced. Desired regression coverage includes unchanged native Responses | ||
| and Azure bodies, final hook ordering, typed runTurn error, legacy media failure, and real HTTP | ||
| rejection with zero upstream sends. Public Pi documentation records the pending behavior. | ||
|
|
||
| The connected Mac runs no product verification by explicit user instruction. Tests are | ||
| written for hosted CI; their presence alone is not a passing result. This direct layer does | ||
| not reuse another session's PABCD identity or claim unperformed formal phase transitions. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| import type { ProviderAdapter } from "./base"; | ||
| import { untranslatedInputMediaMessage, untranslatedResponsesInputMedia } from "../responses/input-media"; | ||
|
|
||
| /** | ||
| * Refuse unrepresentable input at the final translated-adapter boundary. The registry | ||
| * applies this after wire resolution; Responses passthrough (including Azure) opts | ||
| * out because it uses the original body rather than the lossy normalized content. | ||
| */ | ||
| export function withInputMediaGuard<T extends ProviderAdapter>(adapter: T): T { | ||
| const build = adapter.buildRequest.bind(adapter); | ||
| adapter.buildRequest = (parsed, incoming) => { | ||
| const kind = untranslatedResponsesInputMedia(parsed._rawBody); | ||
| if (kind) throw new Error(untranslatedInputMediaMessage(kind)); | ||
| return build(parsed, incoming); | ||
| }; | ||
|
|
||
| const runTurn = adapter.runTurn?.bind(adapter); | ||
| if (runTurn) { | ||
| adapter.runTurn = async (parsed, incoming, emit) => { | ||
| const kind = untranslatedResponsesInputMedia(parsed._rawBody); | ||
| if (kind) { | ||
| emit({ | ||
| type: "error", | ||
| status: 400, | ||
| errorType: "invalid_request_error", | ||
| code: "unsupported_input_modality", | ||
| retryable: false, | ||
| message: untranslatedInputMediaMessage(kind), | ||
| }); | ||
| return; | ||
| } | ||
| await runTurn(parsed, incoming, emit); | ||
| }; | ||
| } | ||
|
|
||
| const localTerminal = adapter.localTerminal?.bind(adapter); | ||
| if (localTerminal) { | ||
| // This hook is outside the builder's error catch. Decline its success shortcut; | ||
| // the ordinary buildRequest path then returns the established client-safe 400. | ||
| adapter.localTerminal = parsed => untranslatedResponsesInputMedia(parsed._rawBody) | ||
| ? undefined | ||
| : localTerminal(parsed); | ||
| } | ||
| return adapter; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| /** Input kinds for which the normalized request has no lossless content carrier. */ | ||
| export type UntranslatedInputMedia = "audio" | "file"; | ||
|
|
||
| type RecordValue = Record<string, unknown>; | ||
|
|
||
| function isRecord(value: unknown): value is RecordValue { | ||
| return value !== null && typeof value === "object" && !Array.isArray(value); | ||
| } | ||
|
|
||
| function mediaKind(value: unknown): UntranslatedInputMedia | undefined { | ||
| if (!isRecord(value)) return undefined; | ||
| if (value.type === "input_audio" || value.type === "audio") return "audio"; | ||
| if (value.type === "input_file" || value.type === "file" || value.type === "document") return "file"; | ||
| // A file-id-only image is not pixels: translated adapters cannot dereference it. | ||
| if (value.type === "input_image" && typeof value.file_id === "string" && value.file_id.length > 0 | ||
| && !(typeof value.image_url === "string" && value.image_url.length > 0)) return "file"; | ||
| return undefined; | ||
| } | ||
|
|
||
| function contentMedia(content: unknown): UntranslatedInputMedia | undefined { | ||
| if (!Array.isArray(content)) return undefined; | ||
| for (const part of content) { | ||
| const kind = mediaKind(part); | ||
| if (kind) return kind; | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| /** | ||
| * Inspect only typed input items and their content arrays, never strings, tool | ||
| * arguments, schema properties, or arbitrary nested objects. No payload is copied, | ||
| * decoded, fetched or included in the returned value. | ||
| */ | ||
| export function untranslatedResponsesInputMedia(body: unknown): UntranslatedInputMedia | undefined { | ||
| if (!isRecord(body) || !Array.isArray(body.input)) return undefined; | ||
| for (const item of body.input) { | ||
| if (!isRecord(item)) continue; | ||
| const direct = mediaKind(item); | ||
| if (direct) return direct; | ||
| if (item.type === "function_call_output" || item.type === "custom_tool_call_output") { | ||
| const kind = contentMedia(item.output); | ||
| if (kind) return kind; | ||
| } else if (item.type === "message" || item.type === undefined) { | ||
| const kind = contentMedia(item.content); | ||
| if (kind) return kind; | ||
|
Comment on lines
+43
to
+45
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.
When a Responses request carries a Codex AGENTS.md reference: src/AGENTS.md:L19-L19 Useful? React with 👍 / 👎. |
||
| } | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| /** Used only when Chat is actually projected, not on the native Chat fast path. */ | ||
| export function untranslatedChatInputMedia(body: unknown): UntranslatedInputMedia | undefined { | ||
| if (!isRecord(body) || !Array.isArray(body.messages)) return undefined; | ||
| for (const message of body.messages) { | ||
| if (!isRecord(message)) continue; | ||
| const kind = contentMedia(message.content); | ||
| if (kind) return kind; | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| /** Fixed vocabulary only: never interpolate filenames, URLs or client metadata. */ | ||
| export function untranslatedInputMediaMessage(kind: UntranslatedInputMedia): string { | ||
| return `OpenCodex cannot translate ${kind} input on this route. Use a native input wire that supports the attachment, or convert it to text first.`; | ||
| } | ||
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.
When the same request activates the web-search or image bridge, those loops call this guarded builder at
src/web-search/loop.ts:467andsrc/images/loop.ts:575before the normal core build-error catch. Because the guard throws a plainError, both loops classify it as502 Provider unreachableat lines 607 and 702; the runTurn image path similarly discards the emitted 400 status atsrc/images/loop.ts:758. A known nonretryable client input error is therefore reported as an upstream outage, inviting retries and misleading diagnostics. Use a shared typed client-input failure that the bridges preserve, or perform admission before entering them.AGENTS.md reference: src/AGENTS.md:L17-L19
Useful? React with 👍 / 👎.