fix(responses): carry bare tool-name echo acceptance (#4729) - #4792
Conversation
…4679) Providers in the muse family (Command Code route included) sometimes echo a namespaced tool call by its bare name - spawn_agent, list_agents, exec - instead of the declared <namespace>__<name> spelling. The fail-closed undeclared-tool guard then kills the stream mid-turn and the client re-sends the entire turn. buildToolBridgeMaps now also registers a namespaced tool's BARE name as an echo alias (declaredToolNames + toolNsMap + toolParameterSchemas) when exactly one authorized identity claims it, mirroring the dotted alias handling: bare names claimed by two identities, or ones equal to another tool's canonical/dotted spelling, stay poisoned and undeclared. Bare-declared (no-namespace) functions participate as owners too, aligning with the tool_choice bareNameCounts rule. The restore-path contract comment (passthrough-dispatch.ts, moved from core.ts) now distinguishes ordinary bare aliases from the exec exception.
tests/responses/bare-echo-alias.test.ts, added by the bare-echo alias change, resolves to no domain: its basename does not match any regex seed in scripts/test-layout/layout.json (the responses seed matches apply|chat|citation|continuation|eventstream|legacy|namespace|passthrough| responses|sse|thought|ws prefixes, not bare-), and it has no explicit entry. That fails two root guards that a scoped bun test tests/responses/ run never executes: - tests/test-layout.test.ts:21 reports it in unresolved. - tests/test-layout-tooling.test.ts:247 requires layout.explicit and tests/fixtures/test-layout-expected.json to be identical, and reports the file in unresolvedNew. Add the basename to both tables, as #4774 did for its own new test file. Co-authored-by: Hulian Felipe Muller Buligon <hulian@MacBook-Pro-de-Hulian.local>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
|
✅ Deterministic PR hygiene checks passed. |
Lock the existing exec-helper exclusion at the bridge-map boundary so a namespaced helper cannot become an authorized bare echo. Co-authored-by: Hulian Felipe Muller Buligon <hulian@MacBook-Pro-de-Hulian.local>
The bare-name carry intentionally adds the unambiguous safe alias to the bridge map. Keep the parser contract fixture aligned with the fail-closed alias map. Co-authored-by: Hulian Felipe Muller Buligon <hulian@MacBook-Pro-de-Hulian.local>
Keep the parser fixture aligned with the bridge map's unambiguous bare-name contract for the safe namespaced function. Co-authored-by: Hulian Felipe Muller Buligon <hulian@MacBook-Pro-de-Hulian.local>
A namespaced custom exec from another catalog remains a caller-declared tool. Only collaboration-surface helper spellings are excluded from bare echo aliases, which preserves the existing mcp__functions.exec contract. Co-authored-by: Hulian Felipe Muller Buligon <hulian@MacBook-Pro-de-Hulian.local>
The selected-tool bridge map retains the unambiguous safe alias, so the allowed_tools parser fixture must assert that alias alongside canonical and dotted spellings. Co-authored-by: Hulian Felipe Muller Buligon <hulian@MacBook-Pro-Hulian.local>
The allowed_tools parser branch returns the same safe bare alias in declaredToolNames as in toolNsMap. Co-authored-by: Hulian Felipe Muller Buligon <hulian@MacBook-Pro-Hulian.local>
리뷰 · 우선순위 72 / 80이 PR(#4792)은 기여자 PR #4729를 지금 지금 이전 grok-bot 리뷰(#4729, 우선순위 70)에서 지적했던 세 가지가 이 레인에서 상당 부분 고쳐졌습니다. (1) 이름공간 없는 bare function도 상태 요약: 타깃은 라인 collaboration.ts BARE_ECHO_EXCLUDED_NAMES 가드 - 제외가 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39d06495cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // `mcp__functions.exec`) remains an ordinary caller-declared tool. | ||
| if ( | ||
| bareAliasOwners.get(t.name) === JSON.stringify([t.namespace, t.name]) | ||
| && !(t.namespace === "collaboration" && BARE_ECHO_EXCLUDED_NAMES.has(t.name)) |
There was a problem hiding this comment.
When an adapter-routed request declares only a namespaced mcp__remote.exec with no bare exec, this condition now adds exec to declaredToolNames. normalizeDeclaredToolName consequently rewrites an undeclared provider apply_patch, exec_command, or similar helper call to exec, after which toolNsMap routes it to the MCP tool instead of rejecting it. Keep the unconditional bare alias out of helper normalization for every namespaced exec—the legacy explicit bare-selector path below can remain—or carry namespace provenance into normalization.
Useful? React with 👍 / 👎.
| // Bare echo alias (`name` with no namespace spelling, #4679): same tool identity as | ||
| // the flattened wire name, so a provider that drops the namespace prefix still | ||
| // restores against this entry. Ambiguous bare names were resolved to null above; | ||
| // skipping them falls back to the spellings every provider can still echo. |
There was a problem hiding this comment.
This changes shared Responses authorization and restoration semantics by accepting another provider-emitted tool spelling, but the commit does not update the owning structure documentation. Record the bare-alias collision and helper-exclusion invariant in structure/transports/responses.md so the architecture contract stays synchronized with the implementation.
AGENTS.md reference: src/AGENTS.md:L10-L11
Useful? React with 👍 / 👎.
| @@ -0,0 +1,132 @@ | |||
| import { describe, expect, test } from "bun:test"; | |||
There was a problem hiding this comment.
The commit message explicitly identifies this change as a carry and supersession of #4729, but the commit object contains no Co-authored-by: trailer; the checklist prose claiming attribution does not register the original contributor or survive as machine-readable credit. Add a real trailer naming the source author before landing the carry.
AGENTS.md reference: AGENTS.md:L288-L292
Useful? React with 👍 / 👎.
…lidge-jun#4792) Maintainer integration of the carried lidge-jun#4729 implementation. Exact head 39d0649 passed Cross-platform CI run 35070233415 and React Doctor 35070233385. The carry preserves original author attribution and includes the parser/layout contract fixes found by exact hosted CI. Host-owned rebase and merge; no local suite, typecheck, build, or install was run.
1 participant
Summary
Carries the implementation from #4729 onto the current dev integration head and adds the missing tests layout registration required by the repository SSOT guards.
Verification
Checklist