Skip to content

fix(responses): normalize default-wrapped namespaced tool calls - #5230

Closed
ildunari wants to merge 1 commit into
lidge-jun:devfrom
ildunari:fix/muse-undeclared-client-tool-callback
Closed

ildunari wants to merge 1 commit into
lidge-jun:devfrom
ildunari:fix/muse-undeclared-client-tool-callback

Conversation

@ildunari

@ildunari ildunari commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Normalize a routed Responses client callback when a provider wraps the complete request-declared flattened namespace identity in default., such as default.mcp__codex_app__send_message_to_thread.
  • Strip that wrapper only when the full suffix contains the flattened namespace delimiter and is an exact member of the current declared-tool set. Explicit default. / default__ identities still win; namespace-dropping guesses and unknown suffixes still fail closed.
  • Add the measured Muse callback fixture, streaming coverage, negative controls, transport documentation, and an architecture decision record.

Verification

  • bun test tests/responses/responses-default-namespace-emit-normalize.test.ts tests/responses/responses-undeclared-tool-guard.test.ts tests/responses/responses-parser.test.ts tests/responses/bare-echo-alias.test.ts tests/responses/responses-custom-tool-stream-consistency.test.ts — 222 passed, 0 failed.
  • bun run typecheck — passed.
  • bun run structure:index && bun run structure:check — passed; the index remained clean.
  • bun run privacy:scan — passed.
  • bun run test:changed — 23,554 passed, 38 skipped, 3 failed. The failures are outside this patch: one Codex shim probe timeout and two remote-workspace executable-inode assertions.
  • The same three failures reproduce on an untouched detached checkout of current dev (3f9fe3fa2): 11 passed, 3 failed across the two affected files. The focused Responses suite above is green after rebasing onto that commit.

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

  • Local CI green. The full/import-graph runs retain the three confirmed current-dev baseline failures documented above.
  • Branch is based on the latest dev commit at push time (3f9fe3fa2).
  • All correct Codex and CodeRabbit findings fixed. Automated review has not run yet.
  • Ready-for-review confirmation. Keep this PR in draft until the repository-wide baseline failures and review findings are resolved or maintainers classify them.

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.

@coderabbitai

coderabbitai Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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 Sep 20, 2026
@github-actions

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.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 66 / 80

이 PR은 Muse가 도구 이름 앞에 default.를 붙였을 때, 그 앞부분을 떼고 요청에 있던 이름으로 돌려보냅니다. 예는 default.mcp__codex_app__send_message_to_thread입니다. 점 뒤를 떼었을 때 그 문자열이 이번 요청의 도구 목록에 있고, 그 안에 __가 있을 때만 뗍니다. default.send_message_to_thread처럼 가운데 이름만 남은 호출은 예전처럼 거절합니다. default.default__로 직접 등록한 이름이 있으면 그 이름이 이깁니다.

베이스는 dev입니다. 이 글을 쓰는 지금 dev 끝은 97aaf8c736d0입니다. #5135입니다. 패키지 버전은 2.60.0입니다. 이 브랜치는 3f9fe3fa 위에 있습니다. tip보다 커밋 4개가 뒤입니다. 겹치는 파일은 없고, GitHub는 머지 가능으로 봅니다. 고친 코드는 src/types/tools.ts입니다. types.tsconfig.ts는 안 바꿉니다. 상태는 드래프트이고, 준비 체크 네 칸이 비어 있습니다.

경로 단위 블로커 없음. 이름 칸의 default.를 떼는 길과, 스트림으로 같은 이름을 내보내는 길은 테스트가 있습니다. 막는 쪽은 이름을 통과시킵니다. 내보내는 쪽은 점 때문에 스키마에 안 맞는 이름을 같은 normalizeDeclaredToolName으로 고칩니다.

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

도구 이름 자체에 __가 있으면, 네임스페이스를 뺀 짧은 이름도 이번 조건에 걸립니다. 그 짧은 이름은 목록에 같이 들어 있습니다. default.그런이름이 짧은 이름으로 통과하는 것은 이 PR이 적은 규칙과 같습니다. #4176에서 막던 범위와 같게 두고 싶으면 조건을 더 좁혀야 합니다.

namespace 칸이 default이고 이름 칸에 전체 이름이 있는 호출은 그대로 거절됩니다. 잡은 콜백이 이름 앞의 default.인지, namespace 칸인지 테스트만으로는 안 보입니다.

작성자는 test:changed 실패 3건이 지금 dev에서도 난다고 적었습니다. 이 조회에서 다시 돌리지는 않았습니다.

너의 추천

코드 방향은 맞습니다. 드래프트로 두세요. 준비 체크와 CI를 채우기 전에 머지하지 마세요. tip의 커밋 4개는 이 파일과 안 겹칩니다. types.ts/config.ts 분할과 무관해서 이 PR을 닫지 마세요.

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

@lidge-jun

lidge-jun commented Sep 23, 2026

Copy link
Copy Markdown
Owner

Closing as superseded. The changes from this PR (head b4c839b8f1f8, by @ildunari) were carried with author credit into #5542, which was consolidated into #5608. #5608 merged to dev as 9579bbe. The carry was cherry-picked or reworked, not merged, so this branch's own commit history is not part of dev. I compared this head against current dev and found its behavior and tests present. That covers default-wrapped namespaced tool-call normalization and its regression coverage. The original standalone ADR filename was not retained separately; this closure records the carried implementation, not byte-for-byte preservation of that documentation file.

This fix is on dev only. It is not in the stable v2.63.0 release and will ship in a later release. Thank you for the contribution.

@lidge-jun lidge-jun closed this Sep 23, 2026
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.

2 participants