Skip to content

fix(responses): spell \0 as \x00 in tool-schema patterns - #5939

Closed
rhomat27 wants to merge 1 commit into
lidge-jun:devfrom
rhomat27:fix/responses-nul-escape
Closed

rhomat27 wants to merge 1 commit into
lidge-jun:devfrom
rhomat27:fix/responses-nul-escape

Conversation

@rhomat27

@rhomat27 rhomat27 commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Meta's Responses API (meta-muse, meta-model) rejects \0 inside a regex character class: Invalid JSON schema: "^[^\\0]*$" is not a "regex". It accepts the equivalent \x00.
  • Claude Code's built-in Artifact tool ships ^[^\0]*$ on its file-path parameters. With the Claude intercept, every routed Claude Code turn to a Meta model returned 400, and Claude Code silently fell back to a native model.
  • stripUnicodePropertyPatterns now also rewrites an unescaped \0 that is not followed by a digit to \x00, in the same pass that drops \p{…}. The constraint is kept, not dropped. \0 followed by a digit (a Python octal escape) and an escaped \\0 are left alone.

Verification

  • Direct probe through the proxy against meta-muse/muse-spark-1.3-contributor, one function tool per request: ^[a-z]*$ 200, ^[^\0]*$ 400, ^[^\x00]*$ 200, ^[^\u0000]*$ 200, \0 200.
  • claude -p --model claude-ocx-meta-muse--muse-spark-1.3-contributor (Artifact tool present): 400 before this change, 200 after, for both meta-muse and meta-model.
  • bun test tests/adapters/openai/openai-chat-hardening.test.ts tests/responses/openai-responses-passthrough.test.ts: 291 pass, 0 fail, including the new regression test.
  • tsc --noEmit: exit 0.
  • bun scripts/test.ts --changed=dev: 619/1758 files selected; 56 failures, all in tests/lab/*. The same 56 fail identically on unmodified dev (2255aff) on this machine.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. (None 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

  • Bug Fixes
    • Schema patterns with a standalone \0 escape are now converted to an equivalent form for improved compatibility. Patterns containing unsupported Unicode property escapes continue to be removed.

Meta's Responses API rejects `\0` inside a character class ("is not a
\"regex\"") but accepts the equivalent `\x00`. Claude Code's Artifact tool
ships `^[^\0]*$` on its file-path parameters, so every routed Claude Code
turn to meta-muse/meta-model returned 400 and Claude Code silently fell back
to a native model. Rewrite unescaped `\0` (not followed by a digit, which
would be a Python octal escape) to `\x00` in the same pass that strips
`\p{...}`; the constraint is kept, not dropped.
@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@coderabbitai

coderabbitai Bot commented Sep 26, 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: 0bfd7c49-0eb0-45ed-aa25-2fba4dc8f6ed

📥 Commits

Reviewing files that changed from the base of the PR and between bb3f3c2 and cb2c061.

📒 Files selected for processing (2)
  • src/adapters/responses-tool-schema.ts
  • tests/adapters/openai/openai-chat-hardening.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

Walkthrough

The schema stripper now rewrites eligible \0 escapes in scalar pattern constraints to \x00. It continues to remove patterns with unsupported Unicode property escapes. Tests cover escape handling, schema preservation, and equivalent matching.

Changes

Schema pattern handling

Layer / File(s) Summary
Rewrite NUL escapes in scalar patterns
src/adapters/responses-tool-schema.ts, tests/adapters/openai/openai-chat-hardening.test.ts
The stripper rewrites unescaped \0 escapes that are not followed by a digit, unless the pattern contains an unsupported Unicode property escape. Tests check octal escapes, escaped backslashes, preservation of other schema fields and the input object, and representative pattern matches.

Priority: ⬆️ High

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to cb2c0

The schema rewrite preserves escaped literals in the reviewed path, and no actionable merge-blocking risk is established.

Architecture Summary

Architecture risk: 🟡 Medium · up to cb2c0

The change affects 2 systems.

Changed systems: src, tests

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 1 changed file maps to changed impact.
  • observed — tests (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in src/adapters/responses-tool-schema.ts: Adds rewriteNulEscapes, which converts each unescaped \0 not followed by a digit to \x00 and returns the original string when no conversion is needed.
  • observed — Modified behavior in src/adapters/responses-tool-schema.ts: Updates the stripUnicodePropertyPatterns documentation to include NUL escape rewriting alongside Unicode property escape removal and to describe the affected destination compatibility.
  • observed — Modified behavior in src/adapters/responses-tool-schema.ts: For scalar pattern values outside name bags, the stripper still removes patterns containing Unicode property escapes. Otherwise, it rewrites NUL escapes and clones the containing schema only when the pattern changes.
  • observed — Modified behavior in tests/adapters/openai/openai-chat-hardening.test.ts: Added coverage for rewriting \0 in schema patterns to \x00, while preserving octal escapes, escaped backslashes, other schema fields, and the input object; checks equivalent matching on representative strings.

Reliability and maintainability

  • inferred — Risk-relevant change factors for src: blast_radius_1; direct_dependents_1
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main change: rewriting \0 as \x00 in response tool-schema patterns.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files.
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.
✨ 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 github-actions Bot added the bug Something isn't working label Sep 26, 2026
@github-actions

github-actions Bot commented Sep 26, 2026 •

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

  • ⬜ 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.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

@github-actions
github-actions Bot marked this pull request as draft September 26, 2026 13:06
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 66 / 80

Claude Code의 Artifact 도구는 파일 경로에 ^[^\0]*$라는 규칙을 붙입니다. \0은 널 문자, 즉 빈 칸 하나를 가리킵니다. Meta의 Responses API는 대괄호 안의 \0을 정규식으로 인정하지 않고 400을 돌려줍니다. 같은 빈 칸인 \x00은 받아 줍니다. 그래서 Claude Code 대화를 meta-muse나 meta-model로 보내면 매번 400이 났고, Claude Code는 오류를 보여 주지 않은 채 원래 모델로 돌아갔습니다.

이 PR은 도구 스키마의 pattern을 손보는 자리에서, 이스케이프되지 않은 \0을 \x00으로 바꿉니다. 바로 뒤에 숫자가 오면 파이썬이 8진수로 읽으므로 그대로 둡니다. \\0처럼 백슬래시가 이스케이프된 것도 그대로 둡니다. 규칙을 지우지 않고 표기만 바꿉니다. \p{…}가 들어 있는 패턴은 예전처럼 통째로 뺍니다.

바탕은 dev입니다. types.ts와 config.ts를 나누는 변경은 아닙니다. 같은 수정을 하는 다른 열린 PR은 없습니다.

라인 - src/adapters/responses-tool-schema.ts rewriteNulEscapes. 뒤 글자 검사는 0부터 9까지입니다. 파이썬 8진수는 0부터 7까지만 이어집니다. \08과 \09는 빈 칸 다음에 8이나 9가 오는 식인데, 이 함수는 고치지 않습니다. 대괄호 안에 있으면 Meta가 또 400을 줄 수 있습니다. Claude Code가 보내는 식은 ^[^\0]*$라서 이번 고장에는 걸리지 않습니다.

라인 - stripUnicodePropertyPatterns. patternProperties, not, oneOf, if, contains, $defs 아래는 들어가지 않습니다. 그 안의 규칙을 빼면 허용과 거절이 뒤집힐 수 있어서, 빼는 일과 바꾸는 일을 같이 건너뜁니다. \0을 \x00으로 바꾸는 일은 규칙을 느슨하게 만들지 않습니다. 그 안에 ^[^\0]*$가 있으면 Meta는 여전히 400입니다. Artifact 파일 경로는 보통 properties 안의 pattern이라, 지금 막힌 요청은 이 칸 밖에 있습니다.

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

자동 테스트는 바꾼 문자열이 자바스크립트에서 같은 글을 거르는지 확인합니다. Meta가 400을 주는지 200을 주는지는 작성자가 프록시로 직접 찍어 본 기록입니다. CI는 그 서버를 다시 치지 않습니다.

이 PR은 아직 초안입니다. 준비 체크리스트 네 칸이 비어 있습니다. 본문에는 관련 테스트 291개와 tsc --noEmit이 통과했다고 적혀 있습니다. bun scripts/test.ts --changed=dev의 실패 56개는 tests/lab/*이고, 고치지 않은 dev에서도 같다고 했습니다.

너의 추천

Artifact 경로를 고치는 이 변경은 두세요. \08과 위 보존 칸까지 넓히는 일은 이번 고장과 따로입니다. 다른 PR은 닫지 마세요. 체크리스트를 채운 뒤 초안을 풀면 됩니다.

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

lidge-jun added a commit that referenced this pull request Sep 26, 2026
)

Six focused fixes from the assigned bug batch remain as separate attributed commits.

| PR | Change | Author |
| --- | --- | --- |
| #5969 | Preserve Meta Muse tool-choice semantics and reject unsupported selectors before dispatch. | shawnkim |
| #5944 | Remove unsupported hosted web-search declarations for Xiaomi MiMo destinations. | codingbo |
| #5938 | Restart the Windows service child after unexpected exits, including exit 0, while reserving the intentional stay-out code. | codingbo |
| #5935 | Reject Claude message-thread state on translated routes so the client resends full history. | kaladinhonor |
| #5939 | Rewrite standalone `\\0` escapes in Meta tool-schema patterns to equivalent `\\x00`. | boblob6969 |
| #5951 | Preserve Kiro-reported credits across stream attempts and in the usage ledger. | codingbo |

A separate integration commit keeps upstream-controlled Kiro event-type text out of opt-in debug logs. The Kiro stream retains the previously landed bounded HTTP-error text when combined with credit metering.

Left out: #5977. Independent security review found that its local read capability authenticates the request but not the HTTP response. A substituted listener could return a shape-valid forged `protected` verdict. A correct server proof bound to the nonce, endpoint, and body is outside this batch. Both its source commit and status-validation follow-up were reverted in new commits; its test and layout entries are gone. The source PR remains open.

Co-authored-by: shawnkim <shawnkim@markncompany.co.kr>
Co-authored-by: codingbo <cnsdbo@163.com>
Co-authored-by: kaladinhonor <266145786+kaladinhonor@users.noreply.github.com>
Co-authored-by: boblob6969 <boblob6969@icloud.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Thanks! This landed on dev through bug-PR merge train batch 9B, #5985 (merge bf04176). Your change is one commit on dev with you as the author and a Co-authored-by trailer. Closing since the content is now on dev.

@lidge-jun lidge-jun closed this Sep 26, 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