Skip to content

fix(responses): gracefully handle code-mode view_image calls via unified exec - #4171

Draft
rrmlima wants to merge 1 commit into
lidge-jun:devfrom
rrmlima:fix/code-mode-view-image-guard
Draft

fix(responses): gracefully handle code-mode view_image calls via unified exec#4171
rrmlima wants to merge 1 commit into
lidge-jun:devfrom
rrmlima:fix/code-mode-view-image-guard

Conversation

@rrmlima

@rrmlima rrmlima commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Resolves an abrupt HTTP 502 / response.failed failure when routed models under Code Mode (tool_mode: code_mode_only / features.code_mode_host=true) emit an undeclared client tool call to view_image.
  • In Code Mode, Codex only declares the top-level exec tool. Routed models (e.g. Gemini 3.8 Flash, Claude) occasionally attempt to invoke view_image upon seeing visual assets or file paths in the conversation history.
  • Previously, responses-undeclared-tool-guard.ts failed closed on view_image, killing the turn abruptly.
  • This PR adds view_image to CODE_MODE_HELPER_TOOL_NAMES in src/types/tools.ts, matching the behavior of other Codex client helpers (apply_patch, write_stdin), and compiles it into an informative code-mode text() response informing the model that view_image is not available in Code Mode and guiding it to inspect files programmatically via tools.exec_command or code.

Verification

  • Ran targeted test suites:
    • 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)
  • Ran static analysis & security scans:
    • bun run typecheck (passed with 0 errors)
    • bun run privacy:scan (passed)

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

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

  • Bug Fixes
    • Improved handling of image-view requests in code mode by returning a clear availability message instead of attempting to execute them as shell commands.
    • Error messages may identify the requested image path and reference the supported command interface.
    • Image-view requests are now correctly recognized when supported execution tools are declared, while undeclared requests continue to be reported appropriately.

…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.
@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 10, 2026
@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: dbc874d7-27a0-49e5-af44-a5afb8d279a8

📥 Commits

Reviewing files that changed from the base of the PR and between c15a98c and aabf0b8.

📒 Files selected for processing (4)
  • src/responses/code-mode-helper-compat.ts
  • src/types/tools.ts
  • tests/responses/legacy-shell-compat.test.ts
  • tests/responses/responses-undeclared-tool-guard.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The change adds view_image handling to Code Mode. It returns a compatibility error with the requested path, recognizes view_image during tool normalization, and adds coverage for compatibility and undeclared-tool validation.

Changes

view_image Code Mode compatibility

Layer / File(s) Summary
Handle view_image compatibility errors
src/responses/code-mode-helper-compat.ts, tests/responses/legacy-shell-compat.test.ts
compileCodeModeHelperInput returns an availability error for view_image, includes an optional path, and directs callers to tools.exec_command or code. The test verifies the path and command reference.
Normalize and validate view_image
src/types/tools.ts, tests/responses/responses-undeclared-tool-guard.test.ts
The code-mode helper list recognizes view_image. Tests cover bare exec, direct view_image, and undeclared-tool cases.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to aabf0

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)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: graceful handling of Code Mode view_image calls through unified exec.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ 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.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

@github-actions
github-actions Bot marked this pull request as draft September 10, 2026 01:38
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 54 / 80

이 PR은 Code Mode(tool_mode: code_mode_only / features.code_mode_host)에서 라우팅된 모델이 Codex 클라이언트 도구 view_image를 호출할 때 턴이 502 / response.failed로 끊기던 문제를 막습니다. 지금 dev의 Code Mode는 카탈로그에 최상위 exec만 선언합니다. 그런데 Gemini·Claude 같은 라우팅 모델은 대화에 이미지 경로가 보이면 view_image를 시도합니다. responses-undeclared-tool-guard는 선언되지 않은 이름이면 fail-closed라 턴이 죽습니다.

고치는 방식은 기존 Code Mode helper 패턴과 같습니다. src/types/tools.tsCODE_MODE_HELPER_TOOL_NAMESview_image를 넣어, exec가 선언된 요청에서는 undeclared로 보지 않게 합니다. 그다음 src/responses/code-mode-helper-compat.ts compileCodeModeHelperInput에서 apply_patch/write_stdin처럼 전용 분기를 두고, 실제 tools.view_image를 호출하지 않고 text(...)로 “Code Mode에서는 view_image를 쓸 수 없다”는 안내를 돌려줍니다. 기본 분기로 가면 tools.exec_command에 view_image 인자가 그대로 들어가 더 이상한 실패가 나므로, soft-error 분기는 맞습니다. 테스트는 compile 출력과 undeclared-guard 수용을 각각 잠급니다. 범위가 작고 types/config 분할과도 무관합니다.

다만 제목의 “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 박스는 비어 있습니다.

메인테이너의 판단이 필요한 지점

  • view_image를 helper allowlist에 넣는 설계를 받아들일지, guard 단 soft-fail로 둘지.
  • 안내 문구가 exec_command를 권해도 되는지, 더 보수적으로 “이미지 도구 없음”만 말할지.
  • draft 박스를 채운 뒤 merge할지, 제목/문구만 고친 뒤 바로 받을지.

너의 추천
제목과 soft-fail 안내 문장만 다듬고, 체크리스트를 채운 뒤 merge해도 됩니다. 502로 턴을 죽이는 것보다 text()로 한 번 더 생각하게 하는 쪽이 맞고, 파일 범위도 좁습니다. exec_command로 이미지를 “보라”는 문장만은 꼭 고치세요. types/config 분할과 무관하고 중복 PR로 보이지 않습니다.

이 댓글은 grok-bot이 작성했습니다

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants