Conversation
…ied exec When running under Code Mode (tool_mode: code_mode_only / features.code_mode_host=true), routed models (such as Gemini 3.8 Flash or Claude) occasionally emit undeclared top-level client tool calls to view_image when encountering visual assets. Previously, responses-undeclared-tool-guard.ts failed closed with HTTP 502 / response.failed, abruptly terminating the turn. This patch: 1. Adds view_image to CODE_MODE_HELPER_TOOL_NAMES in src/types/tools.ts so the guard normalizes it to exec. 2. Synthesizes a friendly code-mode informative message via text() inside compileCodeModeHelperInput in src/responses/code-mode-helper-compat.ts, allowing the model to self-correct and proceed via exec_command/code without aborting the session. 3. Adds regression tests in tests/responses/legacy-shell-compat.test.ts and tests/responses/responses-undeclared-tool-guard.test.ts.
|
✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change adds Changesview_image Code Mode compatibility
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to Code Mode now responds gracefully when a routed model emits view_image, directing it to available file-inspection options instead of failing the response. No current merge-blocking risk remains. 🚥 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 |
⏳ DRAFT
What to do
Review readiness checklist
0/4 boxes ticked. This PR stays in draft until every box above is ticked. |
리뷰 · 우선순위 54 / 80이 PR은 Code Mode( 고치는 방식은 기존 Code Mode helper 패턴과 같습니다. 다만 제목의 “via unified exec”는 실제 동작과 조금 어긋납니다. 이 패치는 view_image를 exec로 실행하지 않고, 실행을 막고 안내 문장만 줍니다. 또 안내가 “tools.exec_command로 시각 자산을 보라”고 하는데, exec_command는 view_image처럼 이미지를 읽어 모델에 넣는 경로가 아닙니다. 모델이 같은 실패를 다른 도구로 반복할 수 있습니다. apply_patch/write_stdin은 진짜 nested helper이고 view_image는 Codex 클라이언트 전용 도구라, “helper로 승격해 수용 + soft fail”이 제품적으로 맞는지는 한 번 더 보면 좋습니다. 대안으로 undeclared guard에서 view_image만 별도 soft-fail 메시지를 주는 길도 있지만, 지금 패턴 재사용이 일관됩니다. 라인 문제: PR 제목 - “via unified exec”는 오해의 소지가 있습니다. soft-fail / text() 안내가 본체이니 제목을 그에 맞게 고치는 편이 좋습니다. code-mode-helper-compat.ts · view_image 분기 안내문 - “Inspect … using tools.exec_command or code”는 이미지 조회와 도구 능력이 맞지 않습니다. “Code Mode에서는 이미지 도구가 없다. 경로가 파일이면 exec_command로 메타데이터/텍스트만 확인하고, 픽셀이 필요하면 Code Mode 밖 세션을 쓰라”처럼 능력을 나누어 쓰는 편이 덜 헷갈립니다. types/tools.ts · CODE_MODE_HELPER_TOOL_NAMES - view_image를 apply_patch급 helper 집합에 넣는 순간, 이름 정규화/수용 경로 전부에 영향이 갑니다. 테스트는 undeclared 수용만 잠그고, 실제 custom-tool-compat 실행 경로에서 text() soft-fail이 한 번이라도 end-to-end로 도는지(가드 통과 후 compile까지) 확인 기록이 본문에 더 있으면 좋습니다. 지금 추가 테스트는 compile 단위와 guard 단위입니다. draft 체크리스트 0/4 - 본문 verification은 있으나 review-ready 박스는 비어 있습니다. 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
This does not implement view_image: it accepts an otherwise undeclared call and emits only text(), without reading or returning an image. That distinction needs to remain explicit.
The generated assertion that view_image is universally unavailable in Code Mode is not established by the request's bare exec declaration. That declaration also does not describe the downstream host's full nested-helper capabilities. Use a request-scoped, truthful unsupported-call diagnostic or an explicitly supported/negotiated helper mapping; do not invent host capabilities or imply image inspection took place.
The two new tests cover the compiler and name guard independently, not their composition through actual streaming/non-streaming restore paths. Add end-to-end bridge assertions for the emitted exec payload and tool-call identity, plus directly declared view_image, namespaced view_image, non-code-mode exec and malformed arguments. Preserve directly declared/namespaced calls unchanged and keep provider strings serialized only as data. Document that any chosen fallback is a tool-error compatibility policy, not image support.
The PR is based on merge base 71a0c308, not the current dev tree; the large two-tree diff is base drift, not evidence that your four-file authored patch intentionally rolls changes back. Rebase and verify the exact-head suite before requesting integration. No subject code was executed locally.
|
This draft and #4412 cover the same code-mode Please don't merge the #4412 proposes the same remap as |
Lane R of the contributor carry train, the serialized responses/core lane: code-mode view_image through unified exec (#4455 by jeongjin0, also carrying the duplicate #4171 by rrmlima), routed effort ladders from models.dev with a refused-rung replay (#4409 by yxr1995-maker), and web-search continuations bound to the serving API key (#4387 by luvs01). Cross-platform CI run 34746891233 concluded success on 2c28886, the exact head merged here, and it covers every link because the lane is cumulative. #4475 and #4488 carry no ci check of their own; their head commits carry [skip ci] by design, under the owner-authorized tip-only CI economy for this batch. The fourth planned link, #4086 by Eleven-is-cool, is not here because it is already on dev as d6723f7 with its own Co-authored-by trailer. The lane attempted the carry first and found a modify/delete conflict on structure/04_transports-and-sidecars.md, which the #4276 SSOT restructure had removed; the landed version is a superset of the branch. All four source authors are credited by Co-authored-by trailers in the landed commits.
|
Closing as a duplicate that has been landed with your credit attached. The code-mode view_image fix reached dev through #4475, merged inside lane R's tip #4489 (merge commit 3f76ce4). #4455 by @jeongjin0 was the eight-file superset of your four files, so that carry is what landed — and the landed commit names you in a Co-authored-by trailer alongside them, because your pull request identified the same defect and the CHANGES_REQUESTED findings on it were folded into the carry rather than discarded. Nothing here is a judgment on this branch; it is bookkeeping so the queue reflects dev. |
…ip ci] A routed provider that echoes the nested helper name emitted `view_image` instead of the declared code-mode `exec`, and the undeclared-tool guard failed the turn. `view_image` now joins the helper names the guard admits behind a bare `exec` declaration, and the compiler turns the call into `await tools.view_image(...)`, surfacing the returned `image_url` through `image()` and falling back to `text()` when the host returns no image. The `default.view_image` spelling some providers invent is stripped to the bare helper first. Explicit `path` wins over the `file_path`, `file` and `image_path` aliases, in that order. Carries lidge-jun#4455 by jeongjin0 and the overlapping four-file subset in lidge-jun#4171 by rrmlima, both answering lidge-jun#4412. Folds in the review findings recorded on lidge-jun#4171: the compat path executes the helper rather than emitting a text-only stub, it asserts nothing about `view_image` being unavailable in code mode, and the composition cases the review named are covered end to end — a namespaced `view_image` keeps its full wire name, a flat-bridge catalog that declares `exec` beside a bare `exec_command` is never rewritten, and malformed arguments still reach nested validation as data. Co-authored-by: Jeongjin Shin <80797980+jeongjin0@users.noreply.github.com> Co-authored-by: rrmlima <137737127+rrmlima@users.noreply.github.com>
Handle view_image and invented default-prefixed helper names through the declared exec tool. Preserve explicit tool identities, serialized arguments, image output, and original detail. Cover bridge and native Responses streaming repair plus catalog boundaries. Related to the unmerged code-mode image proposal lidge-jun#4171; this implementation emits the actual image instead of a text-only fallback. Co-authored-by: Rafael Moreira <rrmlima@gmail.com>
4 participants
Summary
HTTP 502 / response.failedfailure when routed models under Code Mode (tool_mode: code_mode_only/features.code_mode_host=true) emit an undeclared client tool call toview_image.exectool. Routed models (e.g. Gemini 3.8 Flash, Claude) occasionally attempt to invokeview_imageupon seeing visual assets or file paths in the conversation history.responses-undeclared-tool-guard.tsfailed closed onview_image, killing the turn abruptly.view_imagetoCODE_MODE_HELPER_TOOL_NAMESinsrc/types/tools.ts, matching the behavior of other Codex client helpers (apply_patch,write_stdin), and compiles it into an informative code-modetext()response informing the model thatview_imageis not available in Code Mode and guiding it to inspect files programmatically viatools.exec_commandor code.Verification
bun test tests/responses/legacy-shell-compat.test.ts(7 pass, 0 fail)bun test tests/responses/responses-undeclared-tool-guard.test.ts(83 pass, 0 fail)bun test tests/adapters/tool-catalog-nudge.test.ts(20 pass, 0 fail)bun test tests/responses/responses-custom-tool-repair.test.ts(55 pass, 0 fail)bun test tests/responses/apply-patch-envelope.test.ts(18 pass, 0 fail)bun run typecheck(passed with 0 errors)bun run privacy:scan(passed)Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit