Skip to content

feat(claude-cli): serve a tool catalog through the capture-only bridge - #5795

Closed
RobinBially wants to merge 2 commits into
lidge-jun:devfrom
RobinBially:codex/claude-cli-tool-bridge
Closed

RobinBially wants to merge 2 commits into
lidge-jun:devfrom
RobinBially:codex/claude-cli-tool-bridge

Conversation

@RobinBially

@RobinBially RobinBially commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • New capability. A request that carries a tool catalog is now served through the shared capture-only MCP bridge instead of coming back as text only. claude-cli is launched with --mcp-config plus an exact --allowedTools list, the isolated stdio server advertises that catalog, system/init must report it as connected or the turn fails closed, and a captured call returns to the client under the request's wire name. Approval, sandboxing and execution stay with the external client, and a request without a catalog keeps the previous text-only argument shape.
  • Maintainer-facing move. tool-bridge.ts and mcp-server.ts move from src/adapters/codebuddy/ to src/adapters/coding-agent/, because both harnesses speak the same capture contract. The family modules keep supplying only their own arguments, child environment and MCP server path. The private compiled-binary entrypoint follows the move (__codebuddy-mcp -> __coding-agent-mcp); bridge failure text is family-neutral, and what a client sees still names its provider (Invalid Claude Code tool catalog: ...).
  • The registry row, the provider guide and the two structure docs describe the bridge instead of the previous "text and reasoning only" limitation. Refs [Feature]: CodeBuddy capture-only tool bridge and account-roster model discovery (direct merge requested) #5146 (the bridge half of that request; account-roster discovery stays with feat(codebuddy): discover the account roster through the CLI help surface #5147).

Verification

  • Live against the installed CLI (Claude Code 2.1.281, signed-in subscription, Sonnet 5): system/init reports mcp_servers: [{ name: "opencodex", status: "connected", source: "dynamic" }] plus the advertised catalog tools, the model's call arrives as mcp__opencodex__echo, and the process parks on the never-answering capture handler after message_stop (no result frame ever arrives). A full adapter turn with a read_file catalog finished in 3.8 s: tool_call_start (read_file), two argument deltas, tool_call_end, then done { stopReason: "tool_use", endTurn: false, usage: input 2 / output 58 / cache-creation 1366 }.
  • Review follow-up, measured on 2.1.282 (fresh npm i @anthropic-ai/claude-code@latest in a scratch prefix, driven against a two-tool MCP probe): --allowedTools as one comma-joined argument permits the second listed name (permission_denials: []), while allowing only the first name turns the same prompted call into permission_denials: [{ tool_name: "mcp__probe__echo_b", ... }] and it never reaches the server. --max-turns does exist — --max-turns <turns>, "maximum number of agentic turns in non-interactive mode" — but it is hidden from --help and is deliberately not passed: against the capture-only server it ends the turn with a terminal result frame of subtype: "error_max_turns" while the process stays parked behind the never-answering MCP child, so the bound remains turn.ts terminating the tree at message_stop. The adapter comment and the sibling test now record that measurement instead of claiming no such flag exists.
  • bun run typecheck clean.
  • Focused run: bun test tests/providers/claude-cli-tool-bridge.test.ts tests/providers/claude-cli-adapter.test.ts tests/providers/codebuddy-tool-bridge.test.ts tests/providers/codebuddy-tool-bridge-turn.test.ts tests/adapters/adapter-tool-conformance.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts — 100 pass / 0 fail. tests/providers/codebuddy-mcp-server.test.ts separately compiles the standalone binary and drives the renamed __coding-agent-mcp entrypoint — 5 pass / 0 fail.
  • bun run structure:check, bun run privacy:scan and bun run skill:surface:check pass.
  • Full suite (environment exception, documented as the repo requires). bun run test cannot complete on this machine today: the 4x PARALLEL lane aborts with a Bun worker SIGSEGV — the runner itself reports "This indicates a bug in Bun or in a native addon, not in the test itself" — at a file the diff does not touch. Two attempts died at different files (tests/providers/command-code-tool-text.test.ts: passes standalone 40/40; tests/codex-integration/codex-account-label.test.ts: passes standalone 6/6). tests/codex-integration/codex-inject-integration.test.ts additionally stalls, and stalls identically on a pristine origin/dev worktree (git worktree add .tmp/control-dev --detach origin/dev). I therefore drove the suite through the same runner in 100-file lanes instead. The lanes that reported totals: 28,079 pass / 59 skipped / 119 distinct failing tests. Every one of those failures is pre-existing here rather than caused by this branch: the 16 test files they come from produce byte-identical pass/fail counts on a pristine origin/dev worktree (git worktree add .tmp/control-dev --detach origin/dev; 0 differing files out of 16 compared). They are the WebSocket steering/injection suites (tests/responses/ws-native-injection.test.ts 21/24, ws-native-steering 17/16, ws-steering-completion 21/14, ws-native-result-continuations 50/7, ws-steering-stability 27/1), the local-CA/picker TLS tests, a few Chat SSE/refusal cases and the Codex-config/gather load cases. Coverage left to CI: the lanes for tests/providers/cursor, tests/windows, tests/vision, tests/videos and tests/web-search report every file but their runner process wedges on teardown in this environment, and tests/codex-integration/codex-inject-integration.test.ts wedges on the pristine control worktree too. The failures are the pre-existing environment ones: tests/ci-workflows/test-runner.test.ts, tests/claude-integration/claude-intercept-local-ca.test.ts, tests/clients/integrations-state.test.ts, tests/adapters/openai/openai-provider-option-e2e.test.ts, tests/claude-integration/claude-picker-listener.test.ts and tests/cli/update-stop-first.test.ts fail with identical counts on the pristine origin/dev control worktree, and the remaining names are the known load-dependent Codex-config/gather cases. Coverage left to CI: the two lanes above that this runner cannot execute here.

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:

  • Required local validation passed; commands, results, and any full-suite exception are documented.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • New Features
    • Claude CLI requests can expose supplied tools through an isolated capture-only bridge. Captured tool calls are returned for the external client to handle; requests without tools remain text-and-reasoning only.
  • Bug Fixes
    • Invalid tool catalogs and unexpected bridge initialization responses now stop the turn with an error.
  • Documentation
    • Updated provider guidance to explain tool bridging, isolation, and client-side execution.

@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: db353aa9-d2b7-4818-987a-55c95194d0d6

📥 Commits

Reviewing files that changed from the base of the PR and between 1b45e10 and a5c5ccd.

📒 Files selected for processing (3)
  • src/adapters/claude-cli/adapter.ts
  • src/providers/registry/entries-extended.ts
  • tests/providers/claude-cli-adapter.test.ts

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


📝 Walkthrough

Walkthrough

The shared capture-only MCP bridge is generalized for coding-agent CLI adapters. Claude CLI now uses the bridge for requests with a tool catalog. CodeBuddy and its existing tests use the shared implementation.

Changes

Capture-only MCP tool bridge

Layer / File(s) Summary
Generalize the shared capture bridge
src/adapters/coding-agent/tool-bridge.ts, src/adapters/coding-agent/mcp-server.ts
The bridge and MCP server use coding-agent names and shared tool limits. The bridge adds a helper that builds turn input from a bridge and server path.
Wire the shared bridge to CodeBuddy
src/adapters/codebuddy/adapter.ts, src/adapters/coding-agent/turn.ts, src/cli/index.ts, structure/providers-and-adapters.md, tests/providers/codebuddy-*
CodeBuddy uses the shared bridge and prompt directive. The private MCP invocation and CLI entrypoint use coding-agent names. Existing tests now reference the shared API and limits.
Connect Claude CLI requests to the bridge
src/adapters/claude-cli/adapter.ts, tests/providers/claude-cli-tool-bridge.test.ts, tests/providers/claude-cli-adapter.test.ts, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json, docs-site/src/content/docs/guides/providers.md, src/providers/registry/entries-extended.ts, structure/adapters/registry.md
Claude CLI builds bridge input for requests with a tool catalog, stages the bridge directive, and passes the bridge to the shared turn. Tests cover captured calls, cleanup, requests without tools, and fail-closed errors. Documentation describes the capture-only bridge and client-side execution.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ClaudeCLIAdapter
  participant CodingAgentTurn
  participant ClaudeCodeCLI
  participant CodingAgentMCPServer
  Client->>ClaudeCLIAdapter: Submit request with tool catalog
  ClaudeCLIAdapter->>CodingAgentTurn: Pass bridge input and staged prompt
  CodingAgentTurn->>ClaudeCodeCLI: Start CLI with MCP configuration
  ClaudeCodeCLI->>CodingAgentMCPServer: Connect and invoke advertised tool
  CodingAgentMCPServer->>CodingAgentTurn: Return captured tool call
  CodingAgentTurn->>Client: Return call using request wire name
Loading

Merge Risk: ⚪ Minimal · up to a5c5c

The new tool bridge does not materially expand access to the signed-in account, and no merge-blocking issue remains established. Merge after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 27.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 12 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding tool-catalog support to the claude-cli provider through a capture-only bridge.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the enhancement New feature or request label Sep 24, 2026
@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ Required local validation passed; commands, results, and any full-suite exception are documented.
  • ✅ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

@RobinBially

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Pull request base or head changed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

The Claude Code CLI provider answered every turn tools-disabled, so a request
that carried a tool catalog came back as text only. Arming the capture bridge
the CodeBuddy rows already use closes that gap: the CLI is launched with
--mcp-config plus the exact --allowedTools list, advertises the request's
catalog from the isolated stdio server, and a captured call is returned to the
client, which keeps approval, sandboxing and execution.

The bridge and its MCP server move from src/adapters/codebuddy/ to
src/adapters/coding-agent/. Both harnesses speak the same contract (verified
against CodeBuddy Code and Claude Code 2.1.281: the init frame reports the
server as connected and renders calls as mcp__<server>__<tool>), so the family
modules now supply only their own arguments, child environment and MCP server
path. The private compiled-binary entrypoint follows the move
(__codebuddy-mcp -> __coding-agent-mcp), bridge failures read family-neutrally,
and the error a client sees still names its provider.

A catalog turn stages the same private prompt file as before, now with the
bridge directive folded in, and requests without a catalog keep the previous
text-only argument shape.
@RobinBially
RobinBially force-pushed the codex/claude-cli-tool-bridge branch from 90f20ab to 1b45e10 Compare September 24, 2026 19:08
@RobinBially

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-actions
github-actions Bot marked this pull request as ready for review September 24, 2026 19:25
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 60 / 80

이 풀리퀘스트는 바탕이 dev예요. Claude Code CLI 제공자는 요청에 도구 목록이 있어도 글만 돌려줬어요. 이제는 CodeBuddy와 같은 받아적기 전용 다리를 써요. 목록이 있으면 그 목록만 보여주는 MCP 서버를 따로 띄워요. 모델이 고른 호출은 실행하지 않고, 요청에 적힌 이름으로 클라이언트에게 돌려줘요. 승인, 샌드박스, 실행은 바깥 클라이언트가 해요. 목록이 없는 요청은 예전처럼 글만 돌려줘요.

다리 파일은 src/adapters/codebuddy/에서 src/adapters/coding-agent/로 옮겨요. 두 CLI가 같은 약속을 쓰기 때문이에요. 시작 프레임에 서버가 connected로 나와야 하고, 호출 이름은 mcp__opencodex__도구예요. 각 가족 모듈은 자기 인자, 자식 환경, MCP 서버 경로만 넣어요. 컴파일된 프로그램의 숨은 시작 이름은 __codebuddy-mcp에서 __coding-agent-mcp로 바뀌어요. types.ts와 config.ts는 안 건드려요. 같은 내용의 다른 열린 글은 없어요. 계정 목록 찾기는 #5147에 그대로 있어요.

라인 - src/adapters/claude-cli/adapter.ts 98–99행 — 주석이 "도구 통로가 없어서 한 번의 -p는 반복될 수 없다"고 적혀 있어요. 목록이 있는 턴에는 통로가 있어요. 그 턴을 끊는 곳은 src/adapters/coding-agent/turn.ts 593–605행이에요. message_stop이 오면 프로세스를 끝내요. 받아적기 서버는 호출에 답을 안 해서, CLI는 그 뒤에 멈춰 있어요. 98–99행만 읽으면 그 끊기가 없어도 안전해 보여요.

라인 - src/providers/registry/entries-extended.ts 1490행 — note 들여쓰기가 두 칸이에요. 바로 위 defaultMaxOutputTokens와 같은 파일의 다른 note(1397행, 1422행)는 네 칸이에요.

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

구독 계정으로 이 프록시를 열어도 되는지는 레지스트리 노트가 이미 맡긴 판단이에요. 이 글부터는 그 계정으로 도구 호출 제안도 돌아요. 실행은 클라이언트가 해요.

Claude Code 2.1.281에 턴을 한 번으로 막는 플래그가 있는지도 봐 주세요. 주석은 그런 플래그가 없다고 해요. 없다면 message_stop에서 프로세스를 끝내는 일이 반복을 막는 장치예요.

도구가 여러 개일 때 --allowedTools는 쉼표로 이은 한 인자예요 (turn.ts 266행). 직접 확인은 도구 하나(read_file)였어요. CodeBuddy는 이미 이 모양을 써요. Claude가 쉼표를 이름 경계로 자르는지만 확인해 주세요.

너의 추천

방향은 맞아요. 바탕은 dev예요. 닫을 중복 글은 없어요. 잘못된 목록은 400 tool_catalog_invalid로 거절하고, 서버가 connected로 안 보이면 턴을 닫아요. 실행은 이 프로세스 안에서 안 일어나요.

넣기 전에 두 곳만 고치면 좋겠어요. 98–99행 주석은 "message_stop에서 프로세스를 끝내서 다음 턴으로 안 넘어간다"로 적기. 1490행 note는 네 칸으로 맞추기.

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

… note

The adapter comment claimed the CLI exposes no --max-turns. It does carry one, but
hidden from --help, and it is not the bound this leg relies on: against a
capture-only server on 2.1.282 the flag produced a terminal result frame of
subtype error_max_turns while the process stayed open behind the never-answering
MCP child. The comment now names the real bound (turn.ts terminating the tree at
message_stop) and the test records the same measured reason.

Also indents the claude-cli registry note to the four spaces its neighbours use.
@github-actions
github-actions Bot marked this pull request as draft September 24, 2026 20:39
@RobinBially

Copy link
Copy Markdown
Contributor Author

Both line notes are fixed in a5c5ccd, and I ran the two measurements you asked for.

Multi-tool --allowedTools (turn.ts 266) — the comma form holds. --help documents the option as "Comma or space-separated list of tool names to allow", and a live turn against a two-tool catalog with one comma-joined argv token (mcp__probe__echo_a,mcp__probe__echo_b) called the second name and finished with permission_denials: []. The control run is the discriminating half: with only mcp__probe__echo_a allowed, the same prompt's call to mcp__probe__echo_b came back as permission_denials: [{"tool_name":"mcp__probe__echo_b",...}] and never reached the MCP server.

A flag that bounds a turn — one exists, and my comment was wrong about it. --max-turns <turns> ("maximum number of agentic turns in non-interactive mode") is in the CLI but hidden from --help; the string is present in the 2.1.226, 2.1.241 and 2.1.282 binaries, which is why the adapter's claim and the test's rationale were both wrong. It does not replace the termination in turn.ts, and we deliberately do not pass it: against a capture-only server on 2.1.282 the flag ended the turn with a terminal result frame of subtype: "error_max_turns" while the process stayed open behind the never-answering MCP child — the same parked state as without the flag, only with an extra frame the shared mapper would have to special-case. Without a catalog there is no tool channel, so a single -p turn cannot loop either way. adapter.ts now names the real bound (turn.ts terminating the tree at message_stop) and the sibling test records the measured reason instead of "no such flag".

On the two judgment calls: the subscription-authorization question stays where the registry note already puts it (flagged for maintainer review, unchanged by this commit), and roster discovery remains out of scope here — it stays in #5147.

Verification on this head: bun run typecheck; 74 pass / 0 fail across claude-cli-adapter, claude-cli-tool-bridge, codebuddy-tool-bridge, codebuddy-tool-bridge-turn; bun run structure:check; bun run privacy:scan.

@github-actions
github-actions Bot marked this pull request as ready for review September 24, 2026 20:52
@RobinBially

Copy link
Copy Markdown
Contributor Author

Closing as superseded before review spending continues.

The bridge gets the idea right — the client keeps tool ownership and the CLI only proposes calls — but it is built on the wrong substrate: a one-shot claude -p invocation with --tools "", a replaced system prompt and no session, wired to a compiled __coding-agent-mcp entrypoint through --mcp-config/--allowedTools. That invocation shape is what makes the shipped claude-cli provider unusable in practice, and adding a tool catalog to it does not change that.

The subscription route is being rebuilt on the Claude Agent SDK instead — the same substrate meridian uses: a real managed session per conversation, prompt caching, the harness's own tool/permission machinery, and in-process MCP servers for client tools instead of an argv-level bridge. When that lands, client tools come back through the SDK's own mechanism and this argv path disappears rather than being extended.

No maintainer time was spent on this besides the priority review; thanks for that review — the --max-turns correction and the --allowedTools measurement both carry over into the rebuild.

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

Labels

enhancement New feature or request review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants