fix(codex): strip image_gen namespace tools - #1328
Conversation
📝 WalkthroughWalkthrough扩展 Codex provider 覆写对 ChangesCodex 图像生成工具处理
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the Codex provider overrides logic to support image generation tools declared within the input.additional_tools field (Responses Lite format) and as namespace tools (e.g., image_gen). It introduces helper functions to identify, extract, and filter these tool definitions and choices, ensuring consistent behavior when enabling or disabling image generation capabilities. Additionally, comprehensive unit tests have been added to verify these new behaviors and audit logging. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/codex/provider-overrides.ts`:
- Around line 378-383: 在调用 applyImageGenerationToolChoicePreference 前先判断
imageGeneration 是否为 null；为 null 时跳过整个调用及其参数构造，避免执行
findImageGenerationToolReference 和 hasAvailableTool 的全量扫描。保持非 null 偏好下现有调用行为不变。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d3adcdc2-c019-4f14-9aea-82a4b9981bf7
📒 Files selected for processing (2)
src/lib/codex/provider-overrides.tstests/unit/proxy/codex-provider-overrides.test.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83989621ca
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR. The namespace-detection gap in the Codex image-generation provider override is closed cleanly: detection, Responses Lite additional_tools stripping, expanded tool_choice handling, and a new audit path are all internally consistent and well-tested. The copy-on-write discipline and the inherit short-circuit (avoiding tools/input reads) are preserved correctly.
PR Size: L
- Lines changed: 517 (494 additions / 23 deletions)
- Files changed: 3
- Note: Technically L by line count, but ~317 of the 494 added lines are unit tests. The substantive production change is ~195 lines in a single module (
provider-overrides.ts) plus a 3-line Biome-driven test refactor. This is one cohesive fix with appropriate coverage and does not warrant splitting.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Review Notes
- Force-on path correctly reuses the existing same-shape reference (
findImageGenerationToolReference) forallowed_toolsinjection instead of always emitting the legacy{ type: "image_generation" }form. The reference is normalized (onlytype/name), which matches the test expectations and is appropriate for anallowed_toolsentry. - Force-off path strips image tools from both top-level
toolsandinput[].additional_tools, drops emptied carriers, and preserves unrelated namespaces, historicalimage_generation_callitems, andincludefields — verified by dedicated tests. isImageGenerationToolChoicecovers the additional shapes (namespace object vianame/namespace, nestedtool,function.name) andsummarizeImageGenerationToolChoiceemits matching audit values; the two stay consistent across all parameterized forms.- The
inheritshort-circuit (if (imageGeneration !== null)) correctly skips tool/input scans in both the direct override path and the audit wrapper; the getter-based regression test confirms zero reads. - The audit
changesarray is typed generically (Array<{ path: string; ... }>), so the newinput.additional_tools.image_generationentry is type-safe and the sole consumer (forwarder.ts) persists it opaquely. No exported signatures changed. - The OpenAPI test refactor (extracting
publicStatusOperation?.parametersto a local) is the correct fix for Biome 2.5.3's redundant optional-chaining access rule.
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean (pure synchronous transform, no I/O)
- Type safety - Clean
- Documentation accuracy - Clean (docstring updated to reflect
input.additional_tools) - Test coverage - Adequate
- Code clarity - Good
Automated review by Claude AI
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
2 participants
Summary
image_gennamespacetoolsand Responses Liteinput[].additional_toolswhen a provider forces image generation offtool_choiceandallowed_toolsreferences while preserving unrelated tools and response historyProblem
CCH's provider override only recognized the legacy
{ "type": "image_generation" }declaration. Newer Codex requests can expose image generation as{ "type": "namespace", "name": "image_gen" }, including inside Responses Liteadditional_toolscarriers. As a result, a provider configured to force image generation off could still receive the image namespace and forward it upstream, where accounts without image access reject the request.Related
{ "type": "image_generation" }declaration, so newer Codex requests exposing image generation as anamespacetool (including inside Responses Liteadditional_tools) bypassed the force-off path and leaked upstream.service_tier) that feat: add codex image generation provider override #1307 extended and that this fix operates within.Solution
When image generation is forced off, this change removes only image tool declarations and matching tool choices. It keeps other namespaces, ordinary tools, historical
image_generation_callinput items, andincludefields intact. Emptyadditional_toolscarriers are removed. The transformation remains copy-on-write.When image generation is forced on, existing namespace declarations are reused for
allowed_toolsinstead of injecting a mismatched legacy reference.Key implementation points in
src/lib/codex/provider-overrides.ts:toImageGenerationToolReferencenow recognizes both{ type: "image_generation" }and{ type: "namespace", name: "image_gen" }.applyInputImageGenerationToolPreferencestrips image tools frominput[].additional_toolsand drops emptied carriers.isImageGenerationToolChoicecovers additionaltool_choiceshapes (namespace object, nestedtool,function.name), andsummarizeImageGenerationToolChoiceemits matching audit values.findImageGenerationToolReference) forallowed_toolsinjection rather than always inserting the legacy form.This follows the complete-blocking behavior introduced in Sub2API v0.1.151 (
d3a1835e).Changes
src/lib/codex/provider-overrides.ts(+168/-16) - namespace detection, Responses Liteadditional_toolsstripping, expandedtool_choicehandling, and a newinput.additional_tools.image_generationaudit path.tests/unit/proxy/codex-provider-overrides.test.ts(+284) - force-on idempotency, same-shapeallowed_toolsreuse, force-off stripping across top-level and Responses Lite carriers, multipletool_choiceforms, and a negative test ensuring an unrelatedimagegenfunction is not misidentified.No breaking changes - internal helpers only; no exported signatures, schema, or migrations affected.
Testing
Automated Tests
tests/unit/proxy/codex-provider-overrides.test.ts, +284)Validation
bun run lintbun run lint:fixbun run typecheckLC_ALL=C bun run test(743 files passed, 6731 tests passed, 13 skipped)bun run buildManual Testing
No UI changes; this is proxy-pipeline logic. To verify end-to-end: send a Codex Responses request carrying
{ type: "namespace", name: "image_gen" }(intoolsorinput[].additional_tools) through a provider withcodexImageGenerationPreference=false, and confirm the namespace (and any matchingtool_choice) is stripped before the request is forwarded upstream.Checklist
Description enhanced by Claude AI
Follow-up fixes
toolsorinputapplyCodexProviderOverridesWithAuditto cover the real forwarding pathtests/api/api-openapi-spec.test.ts)Follow-up validation: full Biome 2.5.3 check passed; local full suite
6732passed /13skipped; production build passed.Greptile Summary
This PR closes a gap in the Codex provider image-generation override: the existing logic only recognised the legacy
{ type: "image_generation" }declaration, so requests using the newer{ type: "namespace", name: "image_gen" }form — including inside Responses Liteinput[].additional_toolscarriers — could slip through a force-off provider and reach upstream accounts that reject image access. The fix adds namespace detection across all relevant shapes, strips image tools from both the top-leveltoolsarray and embeddedadditional_toolscarriers, normalises associatedtool_choicereferences, and reuses the same-shape reference (legacy or namespace) when force-on injects intoallowed_tools.toImageGenerationToolReference/isImageGenerationTool: now recognises{ type: \"image_generation\" },{ type: \"namespace\", name: \"image_gen\" }, and{ type: \"namespace\", namespace: \"image_gen\" }, covering both field spellings for the namespace form.applyInputImageGenerationToolPreference: new function that strips image tools frominput[].additional_toolscarriers on force-off; carriers with no remaining tools are dropped, others are preserved.isImageGenerationToolChoice/summarizeImageGenerationToolChoice: extended to handle namespace objects and the nested-toolwrapping; the depth check is a single one-level unwrap (callsisImageGenerationTool, not itself), so there is no recursive risk.beforeInputImageGeneration/afterInputImageGenerationscans are now guarded behind the early-exithitcheck, eliminating unnecessary property reads when all preferences are inherited.Confidence Score: 5/5
Safe to merge — the change is scoped entirely to internal helper functions, preserves copy-on-write semantics, and carries comprehensive test coverage across all new code paths.
Both gaps flagged in the previous review round are correctly addressed.
isImageGenerationToolChoicecallsisImageGenerationTool(not itself), eliminating any recursive risk.toImageGenerationToolReferencehandles bothnameandnamespacefield spellings uniformly so filtering is consistent everywhere. The 284 new test lines cover force-on idempotency, force-off stripping across both carriers, same-shape reference reuse, multiple tool_choice shapes, and the inherit-path early-exit. No exported API, schema, or migration is touched.No files require special attention.
Important Files Changed
Reviews (2): Last reviewed commit: "fix(codex): narrow image tool choice mat..." | Re-trigger Greptile