feat(google): enable Gemini inline image output - #355
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
8069565 to
b106701
Compare
This comment was marked as outdated.
This comment was marked as outdated.
230ba5f to
d665a3b
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
d665a3b to
1700fa3
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
R5 findings fixed: 2. OAuth preflight signal propagation: wrap getValidAccessToken in abortableRace against linkedSignal.signal so client abort / timeout during token refresh surfaces as 499/504 instead of hanging. 5. HTTP-layer payload limit: add Content-Length pre-check in parseResponse before response.json() buffers the body. Codex P2-1: Reject malformed CCA inlineData.data (non-string types were forwarded as fake b64_json). Codex P2-2: Fix duplicate 'neither' bullet in JA/KO/RU/ZH locale docs. Codex P2-3: Exempt gemini-*-image chat models from routed catalog media-generation filter (shouldExposeRoutedModel). Codex P2-4: Map CCA safety blocks (finishReason SAFETY/BLOCKLIST/etc) to non-retryable 400 instead of retried 502. Finding 3 (file: URL local-only) addressed in review response as a design positioning decision, not a code bug.
9602aef to
df6a745
Compare
R5 follow-up — all blockers addressed (
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
Re-review @ df6a745
Verdict: request changes. Tip closes most earlier Wibias/R5 code blockers and the 14:07 Codex batch. It is still not merge-ready. Remaining work below must ship in this PR before merge; optional follow-ups are not accepted.
Fixed on tip (do not reopen)
| Area | Status |
|---|---|
| Registry destination pin + sink-host regression | Fixed |
| CCA Images fallback documented (EN + locales) + CCA-after-OpenAI-auth-fail | Fixed |
| Empty/whitespace prompt → 400 before OAuth | Fixed |
materializeInlineImage try/catch → structured error |
Fixed |
Exclusive wx create + UUID collision retry |
Fixed |
| Full PNG 8-byte magic; throw on unrecognized | Fixed |
| Artifact retention/prune | Fixed |
pathToFileURL for local file: syntax |
Fixed (local only — remote still open below) |
ANTIGRAVITY_REQUEST_UA, linked timeout finally, body-read 499/504 parent-first |
Fixed |
OAuth login → 401 vs refresh → 502; deadline + abortableRace around preflight |
Fixed |
CCA envelope Array.isArray(parts) + non-object skip |
Fixed |
Safety finish reasons → non-retryable 400 (partial — missing RECITATION) |
Mostly fixed |
Non-string inlineData.data not forwarded as b64_json |
Partial — type check only; base64/magic still missing |
| Locale duplicate “neither” bullets | Fixed |
| Keep Gemini image chat in catalog | Partial — exemption too broad |
Rebase vs dev |
Fixed — ahead 8 / behind 0 |
| Draft cleared | Fixed |
Keep registry pin, OAuth redaction, timeouts, private artifact permissions, and retention unchanged while fixing the items below.
Remaining blockers (required in this PR)
-
Admission bearer must allow CCA-only Images requests —
src/server/images.ts
validateForwardAdmissionCredential()runs before the CCA-only branch and rejects validAuthorization: Bearer <OPENCODEX_API_AUTH_TOKEN>for non-loopback clients. CCA never forwards that bearer; it uses stored Google OAuth. Bypass/strip the admission check when selecting CCA. Never send the proxy admission bearer upstream. -
Reject
n !== 1for CCA unless real multi-image support exists —src/server/images.ts
CCA envelope ignoresnand can succeed with fewer images than requested. Reject unsupportednbefore OAuth or generation (or implement proper multi-image and enforce returned count). -
Treat
RECITATIONas a permanent CCA content-filter —src/server/images.ts
AddRECITATIONtoCCA_BLOCKING_FINISH_REASONS. Google adapter already maps it tocontent_filter; omitting it here turns paid refusals into retried 502s. -
Strictly validate every CCA
inlineData.databeforeb64_json—src/server/images.ts
Require bounded base64 and a supported image magic signature before returning success. Non-string was tightened; invalid base64 / fake bytes can still become a “successful” Images payload that fails in Codex. -
Replace broad
gemini+imageheuristics with explicit allowlists —src/codex/catalog/parsing.ts+src/adapters/google.ts
Current/gemini/i && /image/iresurrects standalone media-gen IDs (e.g.gemini-3-pro-image) into the picker /responseModalitiespath. Allowlist known chat-capable provider/model pairs only; keep media-generation IDs filtered. -
Replace proxy-local
file:links with an authenticated artifact HTTP route —src/adapters/google.ts(+ server route)
pathToFileURLis fine for same-host clients; remote/container clients cannot open host filesystem paths and paths should not leak. Serve artifacts via an authenticated, traversal-safe HTTP route using opaque IDs for both local and remote clients. -
Enforce Gemini response / SSE-frame size limits before full buffering or JSON parse —
src/adapters/google.ts
Encoded per-image check and non-streamContent-Lengthhelp, but missing/lyingContent-Lengthstill fully buffers viaresponse.json(), and a single oversized SSE frame is still parsed first. Cap response/frame size before buffering/parsing.
Focused regressions (keep minimal)
- Admission-bearer CCA-only request succeeds; bearer never appears upstream
n > 1rejected (or multi-image correct) before OAuthRECITATION→ non-retryable content-filter, not 502- Malformed base64 + fake image bytes → sanitized failure, not
b64_json - Negative catalog cases: media-gen IDs stay filtered; allowlisted chat image models stay exposed
- Remote client can fetch artifact via opaque authenticated route (no server path in markdown)
- Oversized non-stream / chunked SSE frame rejected before full parse
Do not expand into a large test matrix beyond these behaviors.
Process gates
| Gate | Status |
|---|---|
| Base branch | dev (correct) |
Divergence vs dev |
OK — ahead 8 / behind 0 |
| Cross-platform CI on exact tip | Still open — only enforce-target / label / CodeRabbit |
| Maintainer security review (OAuth/CCA + new artifact route) | Still required |
| Unresolved review threads | Still open — resolve/reply after tip matches |
After fixes
Land blockers 1–7 + the focused regressions above, run exact-head Cross-platform CI green, then ping for maintainer security review.
Rebuilt on current dev (terminal-truth parser rework). - Parse inlineData parts in both streaming and non-streaming Google adapter paths, materializing images to OPENCODEX_HOME/artifacts with async writes, per-image (50 MiB) and per-response (100 MiB) decoded byte budgets, 0o700/0o600 permissions, and full-UUID filenames. - Restrict responseModalities=["TEXT","IMAGE"] to explicit image-capable models (gemini-3.1-flash-image class) — non-image Gemini, Vertex, Claude/GPT-on-Antigravity, and thinking models are unaffected. - Whitelist responseModalities in compileGenerationConfig so the setting survives CCA/Vertex wire compilation. - Add gemini-3.1-flash-image to the Antigravity model catalog (wire IDs, picker, context windows). - CCA /v1/images/generations fallback: gated to generations only, uses signalWithTimeout, caps response via arrayBuffer (IMAGES_RESPONSE_MAX_BYTES), redacts errors via safeAntigravityHttpErrorMessage. - Markdown paths escape spaces/parentheses; tests cover special-char dirs. - Update model-count pins in provider-registry-parity and antigravity-wire.
Replace lidge-jun#355's standalone artifacts.ts with lidge-jun#424's fuller version (adds downloadImageToArtifact, SSRF protection, HTTPS enforcement). Also bring in destination-policy.ts exports needed by the module. Removes lidge-jun#355's duplicate guessExtFromMagic — lidge-jun#424's version is the canonical one.
- Rewrite CCA tests to intercept registry host (not local URL) + sink-host regression - Strip home directory from artifact paths in google adapter markdown output - Add try/catch around materializeInlineImage to prevent stream abort on bad parts - Reject empty/whitespace prompts with 400 before OAuth token refresh - Document CCA /v1/images/generations fallback in codex-integration guide (+ locales) - Replace hardcoded artifact path in adapters docs with platform-agnostic wording - Add exclusive create (flag: wx) to artifact writes - guessExtFromMagic throws on unrecognized format (no silent png fallback)
- Emit file: URI instead of ~/ for artifact paths (resolvable + privacy) - Remove imagen-4.0-generate-001 from IMAGE_CAPABLE_MODELS (wrong API) - Try CCA fallback when OpenAI candidate exists but auth fails - Return 502 on OAuth refresh failure (not misleading 400 'none configured') - Reuse ANTIGRAVITY_REQUEST_UA instead of hard-coded UA - Wrap fetch+body-read in try/finally for linkedSignal.cleanup() - Catch body-read timeout/abort → 504/499 - Preserve CCA 4xx statuses instead of collapsing to 502 - Add UUID collision retry on exclusive-create (flag: wx)
- pruneOldArtifacts() caps artifacts/ to 200 files, deleting oldest by mtime - Runs after each successful write in materializeInlineImage + downloadImageToArtifact - Best-effort: errors caught and warned, never fails the write
- Use pathToFileURL for standard file: URIs (Windows + RFC8089 compliant) - Validate CCA envelope: Array.isArray(parts) + per-part object check - Map OAuthLoginRequiredError → 401 (login required), not 502 (refresh failed) - Fix body-read timeout: check linkedSignal.signal.aborted before parent signal - Remove Imagen from adapters.md docs - Document CCA fallback also fires after OpenAI auth failure (+ locales) - Document artifacts emitted as file: URIs
R4 findings fixed: 1. Body-read client cancellation now reports 499 (not 504): signalWithTimeout propagates parent abort into the linked signal, so both are aborted on client cancel. Check parent signal first in the body-read catch block. 2. OAuth preflight now runs inside the image deadline: Move signalWithTimeout creation before getValidAccessToken so token refresh and project discovery are bounded by timeoutMs and can be cancelled by client abort. Add 499/504 checks after OAuth preflight. 4. PNG magic bytes: validate complete 8-byte signature (89 50 4E 47 0D 0A 1A 0A) instead of just the first 4 bytes. 5. Inline payload size pre-check: reject oversized base64 strings in the adapter before normalization copies them, preventing large allocations before the decoded-size guard runs. Finding 3 (file: URL for remote clients) is a design discussion, not a code bug — addressed in PR review response.
R5 findings fixed: 2. OAuth preflight signal propagation: wrap getValidAccessToken in abortableRace against linkedSignal.signal so client abort / timeout during token refresh surfaces as 499/504 instead of hanging. 5. HTTP-layer payload limit: add Content-Length pre-check in parseResponse before response.json() buffers the body. Codex P2-1: Reject malformed CCA inlineData.data (non-string types were forwarded as fake b64_json). Codex P2-2: Fix duplicate 'neither' bullet in JA/KO/RU/ZH locale docs. Codex P2-3: Exempt gemini-*-image chat models from routed catalog media-generation filter (shouldExposeRoutedModel). Codex P2-4: Map CCA safety blocks (finishReason SAFETY/BLOCKLIST/etc) to non-retryable 400 instead of retried 502. Finding 3 (file: URL local-only) addressed in review response as a design positioning decision, not a code bug.
… takeover Address remaining Wibias blockers: admission bearer allows CCA/keyed without forwarding the secret, CCA n=1 + RECITATION + validated base64, explicit image model allowlists, authenticated opaque artifact HTTP route, and size caps with focused regressions.
df6a745 to
168a516
Compare
Maintainer takeoverRebased onto latest
Waiting on fresh CI + bot review before squash-merge. Author @tizerluo will be credited on merge. |
There was a problem hiding this comment.
💡 Codex Review
opencodex/src/adapters/google.ts
Line 532 in 168a516
Cap SSE frames before waiting for a newline
When a compromised or stalled Google upstream sends one very large data: frame without a newline, this append grows buffer without limit because MAX_SSE_FRAME_BYTES is checked only later in handleDataLine(), after split("\n") produces a complete line. Fresh evidence after the earlier buffering finding is that the new frame-size check remains unreachable for an unterminated frame; enforce the cap while accumulating bytes/chars and add a no-newline regression.
AGENTS.md reference: AGENTS.md:L93-L95
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!ARTIFACT_ID_RE.test(name)) { | ||
| throw new Error("artifact filename is not a valid opaque id"); | ||
| } | ||
| return `${ARTIFACT_HTTP_PREFIX}/${name}`; |
There was a problem hiding this comment.
When Codex CLI/App receives this Markdown outside a document served by the proxy, the root-relative /v1/opencodex/artifacts/... URL resolves against the client's own UI/current context rather than the OpenCodex server, so the newly added HTTP route still cannot render the generated image, especially for remote or containerized proxies. Fresh evidence beyond the earlier remote-client comment is that the replacement now emits only a path with no scheme or authority; construct a client-resolvable URL using the proxy's externally reachable origin.
Useful? React with 👍 / 👎.
| `gemini-2.0-flash-preview-image-generation`, or any model id matching both `gemini` and `image`), | ||
| the adapter sends `responseModalities: ["TEXT", "IMAGE"]`. Returned `inlineData` parts are | ||
| materialized to the `artifacts/` subdirectory of the configured OpenCodex config directory and | ||
| surfaced to the client as a markdown image link referencing a `file:` URI | ||
| (``). Each image is capped at 50 MB and each response at 100 MB of decoded |
There was a problem hiding this comment.
The documented behavior no longer matches this change: the adapter now uses an exact three-model allowlist rather than accepting any ID containing gemini and image, and emitted links now target /v1/opencodex/artifacts/... rather than a file: URI. Users following this reference may select unsupported models and will troubleshoot the wrong artifact mechanism; update the model list and link description to match the implementation.
AGENTS.md reference: AGENTS.md:L96-L97
Useful? React with 👍 / 👎.
| export function readArtifactBytes(id: string): { bytes: Buffer; contentType: string } | null { | ||
| const path = resolveArtifactPath(id); | ||
| if (!path) return null; | ||
| const bytes = readFileSync(path); |
There was a problem hiding this comment.
When a client retrieves an artifact near the supported 50 MiB limit, readFileSync() blocks Bun's JavaScript event loop while loading the entire file, and the route then copies that buffer again with new Uint8Array(artifact.bytes). A few concurrent image views can therefore retain roughly twice the artifact size per request and stall unrelated proxy traffic; serve the file through an asynchronous or streaming body such as Bun.file instead of materializing two in-memory copies.
Useful? React with 👍 / 👎.
| } | ||
| return formatErrorResponse(502, "upstream_error", "CCA image generation failed: OAuth token refresh failed"); | ||
| } | ||
| const project = getOAuthCredentialProjectId("google-antigravity"); |
There was a problem hiding this comment.
When the active Google Antigravity account is switched while token refresh is in progress, getValidAccessToken() can return account A's token and this later lookup can reload account B's project ID. The request then combines credentials from different accounts, producing authorization failures or charging/selecting the wrong project when account A can access it; extend the account-scoped access snapshot to carry the matching project ID and use both values atomically.
AGENTS.md reference: AGENTS.md:L83-L89
Useful? React with 👍 / 👎.
| if (Number.isFinite(contentLength) && contentLength > MAX_RESPONSE_BYTES) { | ||
| return [{ type: "error", message: `google response too large (content-length ${contentLength} exceeds ${MAX_RESPONSE_BYTES} bytes)` }]; |
There was a problem hiding this comment.
When Google declares a Content-Length above the cap, this early return leaves response.body uncancelled. A large or indefinitely streaming upstream body can therefore keep its fetch connection and buffers alive until garbage collection even though the client has already received an adapter error, allowing repeated oversized responses to exhaust upstream connection slots; explicitly cancel the body before returning and add a cancellation regression for this header-only rejection path.
AGENTS.md reference: AGENTS.md:L93-L95
Useful? React with 👍 / 👎.
Include gemini-3.1-flash-image in the OAuth preset migration assertion so Cross-platform CI matches the Antigravity picker seed.
When Antigravity is logged in but has no Cloud Code Assist project id, surface a project-discovery 400 instead of falling through to the generic provider-not-configured message.
Cancel oversized non-stream bodies on the Content-Length path, cap SSE accumulation before a newline, stream artifacts via Bun.file, and sync the adapters reference docs with the explicit allowlist and opaque HTTP route.
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
Maintainer review @ 379cb3c9
Verdict: approve.
All Wibias R5 blockers are addressed on tip (rebased onto latest dev):
- Admission bearer allows CCA/keyed Images without forwarding the proxy secret
- CCA rejects
n !== 1 RECITATIONis a non-retryable content filter- Strict base64 + magic validation before CCA
b64_json - Explicit Gemini image-capable allowlists (catalog + adapter); media-gen IDs stay filtered
- Authenticated opaque
/v1/opencodex/artifacts/<id>route (nofile:/ host paths) - Pre-parse size caps for non-stream bodies + SSE accumulation/frames
Tip also includes focused regressions, docs sync, Bun.file artifact streaming, Content-Length cancel, and a clear CCA project-discovery error.
Security review
No medium/high/critical findings in the changed image/artifact/auth paths. Admission non-forwarding, artifact path containment, base64/magic validation, and allowlist gating look solid.
Process
- Cross-platform CI green on exact tip
- CodeRabbit completed; tip Codex P2s from the takeover batch addressed
- Remaining Codex notes (absolute artifact origin, OAuth snapshot atomicity) are non-blocking follow-ups
Ready to squash-merge with credit to @tizerluo.
Merged — thank you @tizerluo!Huge thanks for this PR. Gemini inline image output is a meaningful capability for Codex users, and you carried it through a long review loop with a ton of care (CCA Images fallback, artifacts, catalog wiring, and many hardening passes). That work is genuinely appreciated. Squash-merged to Welcome contribution — looking forward to more from you if you feel like it. |
Reconcile with lidge-jun#355 Gemini inline artifact helpers: keep DNS-pinned downloads and HTTP artifact serving APIs in a shared artifacts module.
…akes These suites measured 5.6–7.5s on windows-latest under the default 5s harness budget after the lidge-jun#355 merge; give them the same 20s ceiling used elsewhere.
4 participants
Summary
/v1/images/generationsfallback (primary): When no OpenAI upstream is configured, Codex's built-inimage_genskill now falls back to CCA's dedicated image model (gemini-3.1-flash-image). This is the standard Codex workflow — the model calls the tool, codex-rs POSTs to the Images API, and the proxy serves it via CCA.inlineDataparsing: For users who selectgemini-3.1-flash-imagedirectly as their chat model,inlineDataparts in the response are materialized to~/.opencodex/artifacts/and emitted as markdown.responseModalities: ["TEXT", "IMAGE"]togenerationConfigfor non-thinking Gemini models.gemini-3.1-flash-imageto the CCA model catalog (fromFetchAvailableModels.imageGenerationModelIds).responseModalitieswhen a thinking level is active (thinking models reject IMAGE modality).How it works (Codex workflow)
No model switching required. Works with just a CCA login (
ocx login google-antigravity).Verification
bun run typecheck— cleanbun test tests/images/gemini-inline.test.ts— 8/8 passbun test tests/google-adapter.test.ts tests/google-vertex-stream.test.ts— 17/17 passbun run privacy:scan— passedPOST /v1/images/generations {"prompt":"A cute green frog on a lily pad"}→ 2MB JPEG returned in standard{created, data:[{b64_json}]}formatPOST /v1/chat/completionswith modelgemini-3.1-flash-image→markdown with materialized 515KB JPEGNote:
bun run test(full suite) has 24 pre-existing failures ondevin catalog/discovery tests, unrelated to this change.Checklist
Summary by CodeRabbit
gemini-3.1-flash-imagemodel.