Skip to content

fix(runtime): avoid OpenAI tool_search name collision - #3958

Open
liugddx wants to merge 1 commit into
apache:mainfrom
liugddx:fix/3939-tool-search-openai-conflict
Open

fix(runtime): avoid OpenAI tool_search name collision#3958
liugddx wants to merge 1 commit into
apache:mainfrom
liugddx:fix/3939-tool-search-openai-conflict

Conversation

@liugddx

@liugddx liugddx commented Aug 26, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #3939

Maka's deferred-tool connector keeps the internal name tool_search, but the provider-bound OpenAI Responses request uses the collision-free alias maka_tool_search. Provider-facing system text, replayed tool call/result names, repair callbacks, and streamed provider tool results are mapped consistently at the adapter boundary. The alias is reserved so a real tool cannot shadow it.

Verification

  • npm --workspace @maka/core run build
  • npm --workspace @maka/storage run build
  • npm --workspace @maka/mcp run build
  • npm --workspace @maka/computer-use run build
  • npm --workspace @maka/runtime run build
  • node --test "packages/runtime/dist/__tests__/deferred-tools-wire.test.js" "packages/runtime/dist/__tests__/responses-wire-contract.test.js" (20 passed)
  • Biome format/check passed for all changed files

The Responses wire regression uses the real openai-codex / gpt-5.6-sol adapter and @ai-sdk/openai converter. It proves the historical { activated: [...] } result is sent as a generic function output under maka_tool_search, and that provider tool calls/results map back to Maka's internal tool_search identity.

Review focus

The provider alias is a transport-only projection. Runtime persistence, UI copy, activation payloads, and existing history remain on the internal name. A real maka_tool_search tool is rejected at catalog construction to preserve bijective mapping.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex authored the implementation, tests, and review follow-up; the human contributor reviewed the changes and owns the submission.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — provider-bound tool naming changes to avoid an SDK reserved-name collision; internal behavior is unchanged.
  • No

@Astro-Han

Copy link
Copy Markdown
Contributor

I reviewed this PR at exact head 81d8946a6a40c3c78e14afcade10342c10dd6015 (base 6762085e, 3 files +170−6, 1 commit).

Spec: NO-GO — 2×P1 + 1×P2

  • P1 — alias is not bijective when a real tool is named maka_tool_search. packages/runtime/src/model-adapter.ts:259-266,306-310 and packages/runtime/src/tool-availability.ts:138-140 reserve only tool_search in the catalog. If input.tools contains both tool_search and a genuine maka_tool_search, the fixed alias assignment overwrites sdkTools.maka_tool_search, both active names collapse, and every returned alias call (including repair) decodes to Runtime tool_search; the real tool is hidden and misdispatched. A deterministic object-transform repro shows only the connector under the alias and decodes it back to tool_search. This needs a collision-checked per-request mapping or a reserve/reject of the alias with an invariant test.
  • P1 — provider-facing instructions still tell the model to call the unavailable tool_search. The PR forwards input.system unchanged and keeps activation/recovery instructions that say Call tool_search (model-adapter.ts:264-312; packages/runtime/resources/bundled-skills/computer-use/SKILL.md:18; packages/runtime/src/tool-runtime.ts:2835-2839), while OpenAI Responses receives only maka_tool_search. Issue Runtime tool_search conflicts with OpenAI Responses native tool_search #3939 expressly required prompt updates. The deferred-activation workflow can thus direct the next provider step to a name absent from its schema. Preserve Runtime/UI names, but project model-facing instructions and recovery consistently at the provider boundary.
  • P2 — requested replay regression does not exercise the failing converter. packages/runtime/src/__tests__/deferred-tools-wire.test.ts:123-209 uses MockLanguageModelV4; although history contains {activated:[...]}, the mock captures only provider tool definitions and never asserts aliased call/result history nor invokes the real @ai-sdk/openai Responses input converter that raised the original {tools:[...]} validation error. The test would pass if history remapping were removed and the bug returned. It also omits collision/repair/provider-result cases; provider-executed result names remain unmapped at model-adapter.ts:833-849 while calls are decoded at :852-869.

Other aspects (resolved-adapter gating for openai-codex, canonical Runtime history/request identity, activation JSON, ordinary alias round-trips) are correct and no scope creep was found.

Standards: 2×P3 + 1 judgment-only P2

  • P3 — PR body replaces/leaves the required template incomplete. Custom Summary/Root cause/Verification omit the AI-use selection and Checklist required by CONTRIBUTING.md and .github/PULL_REQUEST_TEMPLATE.md.
  • P3 — body does not state whether generative tooling contributed, required for every PR per CONTRIBUTING.md Human ownership and AI attribution.
  • Judgment-only P2 — Primitive Obsession: raw fixed alias comparisons at model-adapter.ts:258-266,857 and tool-availability.ts:29-31 encode an implicit namespace reservation without a catalog invariant, causing the shadow/misdecode risk above.

Title/branch conventions pass and the adapter boundary conforms to ARCHITECTURE.md / runtime README. No trailer finding was made because missing disclosure does not prove substantive AI authorship.

Other checks: git diff --check and changed-file Biome are clean; exact-head test run 33025199552 / job 98364740795 and windows_recovery run 33025199544 are terminal SUCCESS; expected path-filtered checks are green. Post-seal the PR is OPEN / non-draft / MERGEABLE / BLOCKED / REVIEW_REQUIRED, head unchanged, 0 reviews/comments.

What I did not check: full local suite beyond the checks noted.

Gate: exact head has P1/P2 findings and therefore cannot be approved or merged until the alias bijectivity, prompt/projection, and regression-test issues are fixed and the template/AI disclosure is completed.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

@liugddx
liugddx marked this pull request as draft August 27, 2026 00:36
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Aug 27, 2026
@liugddx
liugddx force-pushed the fix/3939-tool-search-openai-conflict branch from 81d8946 to 84e3ee8 Compare August 27, 2026 03:11
@liugddx

liugddx commented Aug 27, 2026

Copy link
Copy Markdown
Member Author

Thanks for the exact-head review. I addressed the in-scope findings in the updated head 84e3ee8: (1) maka_tool_search is now reserved at the Runtime catalog boundary, so the provider alias mapping remains bijective; (2) provider-facing system text and replayed text/tool result content are projected to the alias, while Runtime persistence/UI names remain ool_search; streamed provider tool calls and provider-executed results both map back to the internal name; (3) the regression now exercises the real @ai-sdk/openai Responses converter with openai-codex/gpt-5.6-sol and asserts the emitted wire body. I also restored the required PR template sections, selected substantive AI use, added the Generated-by: Codex trailer, and checked the test/format items. The PR is ready for a fresh exact-head review; there are no inline threads to resolve.

@liugddx

liugddx commented Aug 27, 2026

Copy link
Copy Markdown
Member Author

The reviewed findings are now addressed in head 84e3ee8: alias collision is rejected at the Runtime catalog boundary; provider-facing system text, replayed text/tool results, repair callbacks, and streamed provider calls/results all use a consistent transport-only alias; and the regression uses the real @ai-sdk/openai Responses converter with the openai-codex/gpt-5.6-sol path. The required PR template, AI-use declaration, and Generated-by: Codex trailer are also present. CI will rerun for this head. There are no inline threads to resolve.

@liugddx
liugddx force-pushed the fix/3939-tool-search-openai-conflict branch from 84e3ee8 to 21e10bf Compare August 27, 2026 03:25
@liugddx

liugddx commented Aug 27, 2026

Copy link
Copy Markdown
Member Author

Follow-up after the review pass: head is now 21e10bf. I additionally made the text projection token-aware so an existing maka_tool_search string is not double-prefixed, and the real Responses wire test now covers that case. The provider-result mapping is exercised through the streamed provider-result path, and the alias reservation test prevents shadowing. Local affected tests: 20 passed; runtime build and Biome pass.

@liugddx
liugddx force-pushed the fix/3939-tool-search-openai-conflict branch from 21e10bf to 166204c Compare August 27, 2026 05:19
@liugddx
liugddx marked this pull request as ready for review August 27, 2026 05:20
@liugddx

liugddx commented Aug 27, 2026

Copy link
Copy Markdown
Member Author

Rebased the completed fix onto current upstream/main@4b745fc56; new exact head is 166204c59. This absorbs the intervening Desktop E2E readiness stabilization after the prior unrelated prompt-rail failure. The PR is marked ready for review and fresh test / windows_recovery runs are in progress.

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

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Runtime tool_search conflicts with OpenAI Responses native tool_search

2 participants