fix(rectifier): detect 'signature: Field required' error and trigger rectifier#594
Conversation
…rectifier - Extend detectThinkingSignatureRectifierTrigger to match 'signature: Field required' - Add Rule 72 for friendly error message when signature field is missing - Add comprehensive test cases for the new detection logic This fixes an issue where switching from non-Anthropic to Anthropic channels with thinking blocks missing signature fields would fail without proper handling. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 Walkthrough概述本PR增加了对思考块中缺失签名字段的检测和处理。通过添加新的测试用例、检测启发式方法和错误处理规则，扩展了思考块签名验证的场景覆盖。 变更清单
代码审查工作量评估🎯 2 (Simple) | ⏱️ ~12 分钟 可能相关的PR
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @ding113, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the handling of specific API errors that arise from channel switching, particularly when Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively addresses an unhandled error that occurs when a thinking block is missing the required signature field, a scenario common when switching between different channel types. The changes include extending the rectifier's detection logic, adding a new user-friendly error rule, and providing comprehensive unit tests. The implementation is solid and the tests are thorough. I've identified one medium-severity issue regarding a priority conflict in the error rules, which should be addressed to ensure long-term maintainability.
| matchType: "regex" as const, | ||
| isDefault: true, | ||
| isEnabled: true, | ||
| priority: 72, |
There was a problem hiding this comment.
There's a priority conflict with another error rule. This new rule is assigned priority: 72, which is already used by another rule for 'Codex model reasoning effort mismatch' (lines 824-840).
Both rules share the same category 'thinking_error'. While their patterns are currently mutually exclusive, using non-unique priorities is not a good practice and can lead to unpredictable behavior if overlapping rules are added in the future. It also makes the rule set harder to maintain.
Please assign a unique priority to this new rule. For example, you could use 73.
| priority: 72, | |
| priority: 73, |
github-actions
Bot
added
bug
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/repository/error-rules.ts (1)
738-738: Issue 编号占位符需更新。注释中的
Issue #xxx看起来是占位符，建议替换为实际关联的 Issue 编号以保持追溯性。建议修改
- // Issue #xxx: Missing signature field in thinking block + // Issue #594: Missing signature field in thinking block
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
📒 Files selected for processing (3)
src/app/v1/_lib/proxy/thinking-signature-rectifier.test.tssrc/app/v1/_lib/proxy/thinking-signature-rectifier.tssrc/repository/error-rules.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use emoji characters in any code, comments, or string literals
Files:
src/app/v1/_lib/proxy/thinking-signature-rectifier.test.tssrc/repository/error-rules.tssrc/app/v1/_lib/proxy/thinking-signature-rectifier.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
All new features must have unit test coverage of at least 80%
Files:
src/app/v1/_lib/proxy/thinking-signature-rectifier.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: All user-facing strings must use i18n (5 languages supported: zh-CN, zh-TW, en, ja, ru). Never hardcode display text
Use path alias@/to reference files in./src/directory
Format code with Biome using: double quotes, trailing commas, 2-space indent, 100 character line width
Files:
src/app/v1/_lib/proxy/thinking-signature-rectifier.test.tssrc/repository/error-rules.tssrc/app/v1/_lib/proxy/thinking-signature-rectifier.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer named exports over default exports
Files:
src/app/v1/_lib/proxy/thinking-signature-rectifier.test.tssrc/repository/error-rules.tssrc/app/v1/_lib/proxy/thinking-signature-rectifier.ts
src/**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Source-adjacent tests should be placed in
src/**/*.test.tsalongside source files
Files:
src/app/v1/_lib/proxy/thinking-signature-rectifier.test.ts
src/app/v1/_lib/proxy/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
The proxy pipeline processes requests through a GuardPipeline with sequential guards: auth, sensitive, client, model, version, probe, session, warmup, requestFilter, rateLimit, provider, providerRequestFilter, messageContext
Files:
src/app/v1/_lib/proxy/thinking-signature-rectifier.test.tssrc/app/v1/_lib/proxy/thinking-signature-rectifier.ts
src/repository/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Drizzle ORM for data access in the repository layer
Files:
src/repository/error-rules.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: NieiR
Repo: ding113/claude-code-hub PR: 573
File: src/actions/model-prices.ts:275-335
Timestamp: 2026-01-10T06:20:04.478Z
Learning: In the `ding113/claude-code-hub` repository, Server Actions (files under `src/actions/*.ts`) currently return hardcoded Chinese error messages directly. This is a codebase-wide architectural decision that applies to all action files (e.g., model-prices.ts, users.ts, system-config.ts). Changing this pattern requires a coordinated i18n refactor across all Server Actions, which should be handled as a separate unified task rather than piecemeal changes in individual PRs.
📚 Learning: 2026-01-05T03:01:39.354Z
Learnt from: NieiR
Repo: ding113/claude-code-hub PR: 539
File: src/types/user.ts:158-170
Timestamp: 2026-01-05T03:01:39.354Z
Learning: In TypeScript interfaces, explicitly document and enforce distinct meanings for null and undefined. Example: for numeric limits like limitTotalUsd, use 'number | null | undefined' when null signifies explicitly unlimited (e.g., matches DB schema or special UI logic) and undefined signifies 'inherit default'. This pattern should be consistently reflected in type definitions across related fields to preserve semantic clarity between database constraints and UI behavior.
Applied to files:
src/app/v1/_lib/proxy/thinking-signature-rectifier.test.tssrc/repository/error-rules.tssrc/app/v1/_lib/proxy/thinking-signature-rectifier.ts
📚 Learning: 2026-01-10T06:20:04.478Z
Learnt from: NieiR
Repo: ding113/claude-code-hub PR: 573
File: src/actions/model-prices.ts:275-335
Timestamp: 2026-01-10T06:20:04.478Z
Learning: In the `ding113/claude-code-hub` repository, Server Actions (files under `src/actions/*.ts`) currently return hardcoded Chinese error messages directly. This is a codebase-wide architectural decision that applies to all action files (e.g., model-prices.ts, users.ts, system-config.ts). Changing this pattern requires a coordinated i18n refactor across all Server Actions, which should be handled as a separate unified task rather than piecemeal changes in individual PRs.
Applied to files:
src/repository/error-rules.ts
🧬 Code graph analysis (1)
src/app/v1/_lib/proxy/thinking-signature-rectifier.test.ts (1)
src/app/v1/_lib/proxy/thinking-signature-rectifier.ts (2)
detectThinkingSignatureRectifierTrigger(18-69)rectifyAnthropicRequestMessage(80-198)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: pr-review
- GitHub Check: pr-description
- GitHub Check: Docker Build Test
- GitHub Check: 🌐 API Tests
- GitHub Check: check-codex-status
- GitHub Check: dev-build-deploy
🔇 Additional comments (4)
src/app/v1/_lib/proxy/thinking-signature-rectifier.ts (1)
53-61: 检测逻辑实现正确，复用现有触发类型合理。新增的检测条件能够捕获
xxx.signature: Field required格式的错误消息，复用invalid_signature_in_thinking_block触发类型使整流逻辑保持一致。检测顺序正确：在显式 invalid signature 检查之后、通用 invalid_request 检查之前。src/repository/error-rules.ts (1)
740-743: 正则表达式大小写敏感性考虑。当前 pattern
signature.*Field required使用默认的大小写敏感匹配。根据测试文件中的变体用例（如"Signature: field required"），若需兼容不同大小写的错误消息，建议使用大小写不敏感的正则表达式。请确认 Claude API 返回的错误消息格式是否始终使用
Field required（大写 F），或是否存在小写变体。如果存在变体，可以考虑修改 pattern：可选修改：使用大小写不敏感正则
- pattern: "signature.*Field required", + pattern: "(?i)signature.*field required",或者修改为更宽松的模式：
- pattern: "signature.*Field required", + pattern: "[Ss]ignature.*[Ff]ield required",src/app/v1/_lib/proxy/thinking-signature-rectifier.test.ts (2)
44-68: 测试用例覆盖全面。新增的检测测试覆盖了多种
signature: Field required错误格式变体：
- 脱敏路径格式 (
***.***.***.***.***.signature)- 数组索引格式 (
content[0].signature)- 点号分隔格式 (
messages.1.content.0.signature)- 大小写混合 (
Signature: field required)这些用例能有效验证检测逻辑的健壮性。
151-194: 整流场景集成测试设计合理。此测试完整模拟了从非 Anthropic 渠道切换到 Anthropic 渠道的场景：
- thinking block 存在但缺少 signature 字段
- thinking 启用且包含 tool_use
- 验证了 thinking block 移除、顶层 thinking 字段删除等关键行为
测试断言覆盖了
applied、removedThinkingBlocks、content 转换和顶层 thinking 移除等关键结果。
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| type: "thinking_error", | ||
| message: | ||
| "thinking block 缺少必需的 signature 字段。" + | ||
| "该问题通常发生在从非 Anthropic 渠道切换到 Anthropic 渠道时。" + |
There was a problem hiding this comment.
[Medium] [STANDARD-VIOLATION] Hardcoded user-facing error message (missing i18n)
src/repository/error-rules.ts:752
Why this is a problem: CLAUDE.md requires "i18n Required - All user-facing strings must use i18n (5 languages supported). Never hardcode display text". The new override returns a Chinese-only message:
\"thinking block 缺少必需的 signature 字段。\" + ...
This override is returned to clients via the error-rule override path, so it will not respect the caller's locale.
Suggested fix:
// Prefer returning a stable error code and translating per-request locale.
// 1) Add a new code (e.g. THINKING_SIGNATURE_FIELD_REQUIRED) to ERROR_CODES
// 2) Add translations in messages/{zh-CN,zh-TW,en,ja,ru}/errors.json
// 3) When applying override.response in the proxy error handler, translate with:
// const locale = await getLocale();
// const message = await getErrorMessageServer(locale, code);
message: "THINKING_SIGNATURE_FIELD_REQUIRED",
There was a problem hiding this comment.
Code Review Summary
This PR extends the thinking-signature rectifier to detect the signature: Field required failure mode and adds a new default error rule plus unit tests. The main issue is that the newly introduced override error message is hardcoded and not localized, which conflicts with the repo i18n requirement.
PR Size: S
- Lines changed: 101
- Files changed: 3
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 1 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
- None.
High Priority Issues (Should Fix)
src/repository/error-rules.ts:752[Medium] [STANDARD-VIOLATION] Hardcoded user-facing error message without i18n (Confidence: 80).
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
- Identified PR
#594and applied labelsize/S(101 lines, 3 files). - Left 1 inline review comment:
src/repository/error-rules.ts:752— [Medium][STANDARD-VIOLATION]hardcoded user-facing override message without i18n. - Submitted the required PR review summary via
gh pr review(no other diff-line issues met the ≥80 confidence threshold).
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR. The implementation correctly extends the thinking signature rectifier to detect the signature: Field required error pattern and provides a friendly error message via Rule 72.
PR Size: S
- Lines changed: 101
- Files changed: 3
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate (11 tests covering detection logic and rectification scenarios)
- Code clarity - Good
Notes
The implementation follows existing patterns in the codebase:
- Detection logic in
detectThinkingSignatureRectifierTriggeruses case-insensitive string matching consistent with other detection patterns - Error rule in
DEFAULT_ERROR_RULESfollows the same structure as existing rules - Test coverage includes edge cases for various error message formats
Automated review by Claude AI
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
* PR: 修复 Edge Runtime 下 `process.once` 构建告警（AsyncTaskManager 导入链） (#589) * fix: skip async task manager init on edge * fix: avoid static async task manager import * test: cover edge runtime task scheduling * chore: document edge runtime process.once fix * chore: record edge runtime warning baseline * fix: drop NEXT_PHASE and lazy-init async task manager * test: isolate NEXT_RUNTIME in cloud price sync tests * docs: stabilize edge process.once repro baseline * docs: make rollback instructions hashless * docs: add grep checklist for edge warning audit * chore: run regression gate and align docs * test: cover edge runtime guard on register * Update src/lib/async-task-manager.ts 补充 NEXT_PHASE === "phase-production-build" 检查 Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * chore: format code (fix-edge-runtime-process-once-bee7e19) * PR：i18n settings 拆分与翻译质量门禁 (#588) * refactor(i18n): split settings json into smaller files * refactor(i18n): load settings from split module * refactor(i18n): remove legacy settings.json * chore(i18n): update sync-settings-keys for split layout * test(i18n): add split settings guards * chore: align biome schema version * chore(i18n): document messages loading contract * chore(i18n): add settings split verification notes * chore: format code (refactor-i18n-split-settings-3f48fec) * chore: fix i18n request formatting * chore: format code (refactor-i18n-split-settings-a1eff62) * fix: replace settings placeholder translations * chore: verify settings sync script is idempotent * test: run i18n settings split guards * test: add audit for zh-CN placeholder settings strings * chore: apply biome formatting * chore: document manual i18n settings verification * fix: translate all providers filter in ja * fix: translate all providers filter in zh-TW * fix: translate providers section copy in zh-TW * fix: translate providers section copy in ja * feat: extend placeholder audit output * feat: add allowlist for placeholder audit * docs: define i18n translation quality rules * chore: add i18n audit fail commands * docs: add i18n PR checklist * chore: format i18n audit tests * fix: translate dashboard placeholders * fix: translate myUsage placeholders * fix: enforce locale-specific parentheses * fix: start translating provider form strings * fix: translate provider form strings * fix: translate provider guide content * test: add ja dashboard parentheses guard * test: add zh-TW dashboard parentheses guard * test: add zh-TW myUsage parentheses guard * chore: translate ja provider form strings * chore: translate zh-TW provider form strings * chore: translate ja providers guide * chore: translate zh-TW providers guide * chore: refine zh-TW dashboard strings * chore: translate ja providers strings * chore: translate zh-TW providers strings * chore: refine zh-TW api test strings * chore: translate zh-TW settings small modules * chore: translate ja settings small modules * chore: clear i18n placeholders in settings * chore: format code (refactor-i18n-split-settings-2437d19) * test: fix biome formatting in i18n test * chore: verify Biome lint gate (I18NE-030) * chore: add messages emoji audit script (I18NE-010) * fix: remove emoji from messages warnings (I18NE-040) * test: add messages no-emoji audit gate (I18NE-050) * docs: add zh-CN i18n docs (I18NE-020) * docs: add messages no-emoji rule (I18NE-060) * chore: run full regression checks (I18NE-070) * docs: add i18n PR evidence template (I18NE-080) * fix: make messages no-emoji audit path-sep safe * docs: add bun alias for messages no-emoji audit * fix: detect keycap and flag emoji sequences in i18n message audits * fix(provider): allow removing custom whitelisted models (#592) (#593) * fix(rectifier): detect 'signature: Field required' error and trigger rectifier (#594) - Extend detectThinkingSignatureRectifierTrigger to match 'signature: Field required' - Add Rule 72 for friendly error message when signature field is missing - Add comprehensive test cases for the new detection logic This fixes an issue where switching from non-Anthropic to Anthropic channels with thinking blocks missing signature fields would fail without proper handling. * feat(users): increase provider group length to 200 (#591) close #590 * feat(usage-doc): update OpenCode config example with GPT-5.2 and Gemini v1beta (#597) - Add OpenAI GPT-5.2 model configuration with reasoningEffort options - Add GPT-5.2-small variant using medium reasoning effort - Fix Gemini baseURL to use /v1beta endpoint - Update i18n strings to reflect different baseURLs per provider * feat: auto-complete Codex session identifiers (#599) * fix: Codex session completion must not inject metadata (#601) * feat: auto-complete Codex session identifiers * fix: avoid Codex metadata injection --------- Co-authored-by: YangQing-Lin <56943790+YangQing-Lin@users.noreply.github.com> Co-authored-by: Hwwwww-dev <47653238+Hwwwww-dev@users.noreply.github.com>
1 participant
Summary
detectThinkingSignatureRectifierTriggerto matchsignature: Field requirederrorProblem
When switching from non-Anthropic to Anthropic channels, thinking blocks may lack the required
signaturefield, causing Claude API to return:Previously, this error was not detected by the thinking signature rectifier, so:
Related Issues:
signatureinthinkingblock #518 - Extends the original "Invalidsignatureinthinkingblock" error handling to also cover the missing signature caseSolution
Extend rectifier detection (thinking-signature-rectifier.ts:53-61):
signature+field requiredpatterninvalid_signature_in_thinking_blockAdd error rule (error-rules.ts: Rule 72):
signature.*Field requiredChanges
Core Changes
src/app/v1/_lib/proxy/thinking-signature-rectifier.ts- Add detection for missing signature field patternsrc/repository/error-rules.ts- Add Rule 72 for friendly error messageSupporting Changes
src/app/v1/_lib/proxy/thinking-signature-rectifier.test.ts- Comprehensive tests for the new detection logicTesting
Automated Tests
Checklist
Description enhanced by Claude AI