Skip to content

fix(protocols): enforce explicit-empty tool catalogs across delivery paths - #7058

Open
luvs01 wants to merge 4 commits into
lidge-jun:devfrom
luvs01:codex/explicit-empty-catalog-followup-20261013
Open

luvs01 wants to merge 4 commits into
lidge-jun:devfrom
luvs01:codex/explicit-empty-catalog-followup-20261013

Conversation

@luvs01

@luvs01 luvs01 commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Follow up on the explicit-empty catalog finding in #7030. Managed native Chat/Messages and translated delivery paths reject client tool output outside a current request's explicit-empty catalog with undeclared_tool_call, before emitting a tool call or successful completion.
  • Snapshot the current request's declarations before replay and hosted-tool injection, and use the same catalog for output validation and name normalization. Preserve omitted/partial catalogs, normal text, current legacy declarations, caller-forward behavior, and internal hosted search/image/video work.
  • Add 218 HTTP JSON/SSE regression cases, 36 Advisor composition cases, and 12 Antigravity composition cases and update the canonical protocol documentation. This is a separate follow-up originally based on dev at f5f73ff3970b3b238c4e2fade29ba3f64914f637; fix(responses): preserve tool catalog boundaries across inbound wires #7030 is already merged.

Verification

Conflict maintenance at 3d431f3fc04472203ae17b7c01cbbb61c6615704 (Linux, repository wrapper, pinned Bun 1.4.3):

  • Normally merged the confirmed conflicting dev commit 9b31677c4704215e8f3db1c321a38b36b40062a9. The sole text conflict was in request preparation: preserve direct decoding before the client-catalog snapshot, and recover current legacy Chat function names when the direct settings-only body omits them. Both parents' test registrations and the new layout shard configuration are retained. The other 25 existing feature-file deltas are unchanged.
  • Added 12 Antigravity direct/bridge composition cases for JSON/SSE, empty/omitted/partial catalogs, current/unrelated legacy declarations and normal text. The simple conflict resolution failed the two current-legacy cases; the final composition file passed 15 tests, 0 failures, 150 assertions with bun scripts/test.ts --parallel=4 --timeout 60000 tests/responses/protocol-antigravity-direct-chat.test.ts.
  • The expanded focused command below passed 1,774 tests, 0 failures, 10,124 assertions across 65 files. bun run typecheck, bun run structure:check, bun run privacy:scan, and git diff 9b31677c4704215e8f3db1c321a38b36b40062a9 HEAD --check passed. Frozen docs install and docs build passed: 577 pages, 81,200 internal links.
  • Import-connected validation with bun scripts/test.ts --changed=9b31677c4704215e8f3db1c321a38b36b40062a9 --parallel=4 --timeout 60000 --bail=6: 3,366 passed, 1 skipped, 10 failed, 24,761 assertions across 126 files, then stopped early. All ten failures reproduced on the exact unmodified pinned base: the two baseline files below gave 40 passed, 8 failed, 180 assertions; separately, bun scripts/test.ts --parallel=1 --timeout 60000 tests/codex-integration/native-codex-toggle.test.ts gave 12 passed, 2 failed, 32 assertions. Coordinator temporary-directory ownership is rejected in this sandbox, and the non-loopback HTTP probe receives 403 instead of 401. No ownership, permission, network, or safety guard was changed; these scenarios remain unvalidated beyond the failing checks.
  • The full local suite and macOS/Windows validation remain unrun. Existing native-path deferrals remain unchanged. Independent security review remains incomplete: the previously blocked attempt was not retried or replaced, and no independent approval is claimed. Human review remains required. Keep Draft.
  • Current-head CI: Ubuntu PR CI succeeded for this exact head: 15 successful jobs, 8 conditional skips, including all four Ubuntu test shards and the final aggregate. Catalog async contracts, React Doctor, PR hygiene, PR Labeler and Enforce PR target branch also succeeded for this head. Skipped platforms are not validated.

Original implementation validation at e4944b07066f8734f25d3b619b6caa69edc2e7e8 (Linux, repository wrapper, pinned Bun 1.4.3):

  • Focused protocol, native transport, hosted-tool, replay, layout, architecture, and Qoder adapter tests: 1,210 passed, 0 failed, 7,211 assertions across 39 files.
  • bun scripts/test.ts --parallel=4 --timeout 60000 tests/server/server-key-failover-e2e.test.ts: 63 passed, 0 failed, 457 assertions. Covers the newer dev key-pool recovery behavior in the shared run-turn path.
  • bun run typecheck, bun run structure:check, bun run privacy:scan, and git diff --cached --check: passed.
  • In docs-site: bun install --frozen-lockfile and bun run build: passed; 569 pages and 79,403 internal links checked. The first build could not create Astro's default home-directory settings; the successful build used a temporary XDG_CONFIG_HOME within the checkout.
  • bun run test:changed --parallel=4 --timeout 60000 --bail=6 with Bun 1.4.3 explicitly selected: 3,066 passed, 1 skipped, 8 failed, 22,507 assertions across 111 files, then stopped early. The six Desktop first-party guard failures and two loopback integration failures also reproduce on unmodified dev at the base above (40 passed, 8 failed, 180 assertions for those two files). The environment rejects its temporary-directory ownership for coordinator setup; the non-loopback HTTP probe receives 403 instead of the expected 401. No ownership, permission, network, or safety guard was changed. Those scenarios are not validated past the failing checks.
  • The full local suite and macOS/Windows validation were not run. The import-connected run already stops on baseline environment failures; this Draft uses the documented focused-test scope exception. Per current repository CI policy, PR CI runs the fast Ubuntu subset; full cross-platform coverage remains outstanding.
  • Independent security review is incomplete: the independent automated review attempt was blocked by the review environment and did not finish. It has not been retried or replaced through another route. No independent security approval is claimed; human review remains required. Keep this PR in Draft.
  • Original implementation-head CI for e4944b07066f8734f25d3b619b6caa69edc2e7e8: Ubuntu PR CI succeeded: 15 successful jobs, 8 conditional skips, including all four Ubuntu test shards. This run does not validate the skipped macOS/Windows jobs. Catalog async contracts, React Doctor, PR hygiene, PR Labeler, and Enforce PR target branch also succeeded for this head.
Current merged-head focused test command (65 files)
bun scripts/test.ts --parallel=4 --timeout 60000 \
  tests/responses/explicit-empty-tool-catalog.test.ts \
  tests/responses/explicit-empty-tool-catalog-history.test.ts \
  tests/responses/responses-undeclared-tool-guard.test.ts \
  tests/responses/responses-undeclared-tool-guard-inbound.test.ts \
  tests/responses/passthrough-deferred-tools.test.ts \
  tests/responses/chat-completions-deferred-tools.test.ts \
  tests/responses/protocol-direct-encoders-messages.test.ts \
  tests/responses/protocol-direct-encoders-chat.test.ts \
  tests/responses/protocol-direct-encoders-stall-disabled.test.ts \
  tests/responses/protocol-direct-encoders-local-upstream.test.ts \
  tests/server/inference-client-encoder-delivery.test.ts \
  tests/responses/responses-run-turn-web-search.test.ts \
  tests/images/loop.test.ts \
  tests/images/loop-reasoning-replay.test.ts \
  tests/images/z-handler-activation.test.ts \
  tests/videos/plan-video.test.ts \
  tests/web-search/web-search-passthrough-bridge.test.ts \
  tests/web-search/web-search-run-turn-loop.test.ts \
  tests/web-search/web-search-bridge-replay.test.ts \
  tests/responses/responses-core-modules.test.ts \
  tests/lab/core-lab-boundary.test.ts \
  tests/test-layout.test.ts \
  tests/test-layout-tooling.test.ts \
  tests/ci-workflows/structure-ssot.test.ts \
  tests/ci-workflows/file-size-ratchet.test.ts \
  tests/providers/qoder-adapter.test.ts \
  tests/providers/qoder-scaffold-guard.test.ts \
  tests/responses/chat-native-combo.test.ts \
  tests/responses/chat-native-decline-reason.test.ts \
  tests/responses/chat-native-image-normalization.test.ts \
  tests/responses/chat-native-spend.test.ts \
  tests/responses/chat-native-lenient-finish.test.ts \
  tests/responses/chat-native-sse-stall.test.ts \
  tests/responses/chat-native-developer-role.test.ts \
  tests/claude-integration/messages-native-scope.test.ts \
  tests/claude-integration/messages-native-oauth.test.ts \
  tests/claude-integration/messages-native.test.ts \
  tests/claude-integration/messages-native-decline-trace.test.ts \
  tests/claude-integration/messages-native-opaque-state.test.ts \
  tests/server/server-key-failover-e2e.test.ts \
  tests/adapters/anthropic/anthropic-client-identity.test.ts \
  tests/clients/desktop-app-server-shim-launcher.test.ts \
  tests/codex-integration/codex-history-pending-wal.test.ts \
  tests/lib/socks5-tls-lifecycle.test.ts \
  tests/service/service-log.test.ts \
  tests/update/update-standalone.test.ts \
  tests/advisor/advisor-authority-transport.test.ts \
  tests/advisor/advisor-consult.test.ts \
  tests/advisor/advisor-context.test.ts \
  tests/advisor/advisor-continuation-limit.test.ts \
  tests/advisor/advisor-core-boundary.test.ts \
  tests/advisor/advisor-guard.test.ts \
  tests/advisor/advisor-internal-authority.test.ts \
  tests/advisor/advisor-plan.test.ts \
  tests/advisor/advisor-responses-wiring.test.ts \
  tests/advisor/advisor-settings.test.ts \
  tests/advisor/advisor-state.test.ts \
  tests/adapters/anthropic/anthropic-beta-allowlist.test.ts \
  tests/responses/responses-compaction-http-isolation.test.ts \
  tests/responses/responses-compaction-routing.test.ts \
  tests/responses/empty-completion-core.test.ts \
  tests/responses/empty-completion-guard.test.ts \
  tests/responses/empty-completion-hardening.test.ts \
  tests/responses/protocol-antigravity-direct-eligibility.test.ts \
  tests/responses/protocol-chat-ir-parity.test.ts
Current baseline comparison command (unmodified pinned dev)
bun scripts/test.ts --parallel=1 --timeout 60000 \
  tests/claude-integration/claude-desktop-first-party-guards.test.ts \
  tests/server/loopback-listener-integration.test.ts

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.

Summary by CodeRabbit

  • Bug Fixes
    • Requests with an explicitly empty tool catalog now refuse client tool calls not declared for the current turn, returning an undeclared_tool_call error before delivery.
    • This behavior applies to managed Chat and Messages responses, translated and direct responses, and final tool output from hosted search, image, and video flows.
    • Omitted or partial catalogs retain their existing behavior, and hosted internal operations and caller-forward routes are unaffected.
  • Documentation
    • Clarified when empty catalogs trigger refusals and how those errors appear in responses.

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: 9b6c9a7a-8196-4c99-a417-914a61a92b27
📥 Commits

Reviewing files that changed from the base of the PR and between 9b31677 and 3d431f3.

📒 Files selected for processing: 28
  • docs-site/src/content/docs/reference/proxy-formats.md
  • scripts/test-layout/layout.json
  • src/images/loop.ts
  • src/lib/empty-tool-catalog.ts
  • src/protocols/encoders/messages.ts
  • src/server/chat-native-sse.ts
  • src/server/chat-native.ts
  • src/server/messages-native-tool-guard.ts
  • src/server/messages-native.ts
  • src/server/responses/adapter-delivery.ts
  • src/server/responses/request-prepare.ts
  • src/server/responses/run-turn-execution.ts
  • src/server/responses/sidecar-execution.ts
  • src/web-search/loop.ts
  • structure/advisor.md
  • structure/data-planes/images.md
  • structure/data-planes/inbound-compat.md
  • structure/data-planes/protocol-paths.md
  • structure/data-planes/search.md
  • structure/providers-and-adapters.md
  • structure/transports/inventory.md
  • structure/transports/responses-wire-shapes.md
  • structure/transports/responses.md
  • tests/advisor/advisor-responses-wiring.test.ts
  • tests/fixtures/test-layout-expected.json
  • tests/responses/explicit-empty-tool-catalog-history.test.ts
  • tests/responses/explicit-empty-tool-catalog.test.ts
  • tests/responses/protocol-antigravity-direct-chat.test.ts

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

📝 Walkthrough

The change adds explicit empty client tool-catalog enforcement across managed native Chat and Messages, translated Responses delivery, and final hosted-search, image, and video output. Undeclared client tool calls return undeclared_tool_call; caller-forward routes and internal hosted operations remain exempt.

Changes: Explicit Empty Tool Catalog Enforcement

Layer / File(s) Summary
Catalog capture and translated delivery
src/lib/empty-tool-catalog.ts, src/server/responses/request-prepare.ts, src/server/responses/adapter-delivery.ts, src/server/responses/run-turn-execution.ts, src/protocols/encoders/messages.ts, tests/responses/explicit-empty-tool-catalog*.test.ts, tests/fixtures/test-layout-expected.json, scripts/test-layout/layout.json, structure/transports/responses*.md, structure/data-planes/protocol-paths.md
Request preparation captures the current-turn catalog restriction. Translated streaming and buffered delivery guard client tool events and preserve refusal codes. Regression tests cover current versus historical declarations, response formats, and delivery lanes.
Native Chat and Messages guards
src/server/chat-native*.ts, src/server/messages-native*.ts, src/server/messages-native-tool-guard.ts, tests/responses/explicit-empty-tool-catalog.test.ts, tests/responses/protocol-antigravity-direct-chat.test.ts, docs-site/src/content/docs/reference/proxy-formats.md, structure/data-planes/inbound-compat.md, structure/data-planes/protocol-paths.md
Native Chat and Messages reject client tool calls for explicitly empty catalogs in JSON and SSE responses. Chat tracks streamed tool names, including fragmented names; Messages distinguishes client tool calls from server-tool blocks.
Hosted output propagation and validation
src/images/loop.ts, src/web-search/loop.ts, src/server/responses/sidecar-execution.ts, tests/responses/explicit-empty-tool-catalog*.test.ts, tests/advisor/advisor-responses-wiring.test.ts, structure/advisor.md, structure/data-planes/images.md, structure/data-planes/search.md, structure/providers-and-adapters.md, structure/transports/inventory.md
Image, video, and web-search paths receive the client catalog restriction and guard final client output. Tests cover hosted outputs and Advisor wiring; internal hosted operations retain their existing behavior.

Priority: ⬇️ Low

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Request as prepareResponsesRequest
  participant Delivery as deliverAdapterResponse
  participant Guard as guardEmptyToolCatalogStream
  participant Client as Client
  Request->>Delivery: current-turn catalog restriction
  Delivery->>Guard: streamed adapter events and restriction
  Guard->>Client: accepted events or undeclared_tool_call refusal
Loading

Suggested reviewers: lidge-jun

Merge Risk: ⚪ Minimal · up to 3d431

No identified issue with this change currently blocks merging; complete the planned human review before taking it out of Draft.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 30.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 16 files. (12 skipped… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 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 primary change: enforcing explicit-empty tool catalogs across native, translated, direct-encoder, and hosted delivery paths.
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 30.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 16 files. (12 skipped: 12 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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 bug Something isn't working label Oct 13, 2026
Preserve both sets of test registrations in the two conflicting layout maps. Retain the existing feature changes and upstream updates.
Keep advisor interception before client catalog validation and retain both sidecar request fields. Add focused composition coverage for buffered, bridged and direct delivery.
@luvs01
luvs01 marked this pull request as ready for review October 14, 2026 00:59

This branch has not been deployed

No deployments
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.

1 participant