fix: respect case-sensitive provider model ids#1011
Conversation
📝 WalkthroughWalkthrough将规则/模型标识键的归一化行为从小写化改为仅修剪空白：构建身份字符串时不再对 Changes
Estimated code review effort🎯 3 (中等) | ⏱️ ~20 分钟 Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![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 updates the model rule and redirection logic to be case-sensitive. By removing the .toLowerCase() transformation during rule identity generation and schema validation, the system now allows multiple rules or redirects that differ only by case (e.g., "GLM-5" and "glm-5"). Corresponding unit and UI tests have been added to verify this behavior. I have no feedback to provide.
![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: 67e0d21436
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| function getRuleIdentity(rule: Pick<AllowedModelRule, "matchType" | "pattern">): string { | ||
| return `${rule.matchType}:${rule.pattern.trim().toLowerCase()}`; | ||
| // exact/prefix/suffix/contains/regex 的运行时匹配都区分大小写，这里去重也必须保持一致。 | ||
| return `${rule.matchType}:${rule.pattern.trim()}`; |
There was a problem hiding this comment.
Switching rule identity to matchType + pattern makes GLM-5 and glm-5 valid distinct rules, but the quick picker still collapses selections by lowercased keys (getModelKey/handleToggleModel in model-multi-select.tsx). In this mixed-case scenario, toggling one selected item removes both entries because removal filters by the shared lowercase key, so users can silently lose rules after interacting with the picker even though the editor now allows them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ff94e4f. ModelMultiSelect now keeps its internal keys case-sensitive as well, so selected exact rules like GLM-5 and glm-5 no longer collapse in the picker. Added a regression test in tests/unit/settings/providers/model-multi-select.test.tsx that verifies toggling one mixed-case entry preserves the other.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR.
PR Size: S
- Lines changed: 125
- Files changed: 8
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Automated review by Codex AI
There was a problem hiding this comment.
- Reviewed PR
#1011(“fix: respect case-sensitive provider model ids”):120additions,5deletions,8files → Size S - Applied PR label
size/S - Submitted the required PR review summary comment via
gh pr review(no diff-scoped issues met the reporting threshold, so no inline comments were posted)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
🧪 测试结果
总体结果: ✅ 所有测试通过 |
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
* fix: relax provider routing rule limits * feat: improve provider detection template selection (#1003) * feat: improve provider detection templates * fix: address provider detection review feedback * chore: format code (feat-provider-detect-templates-02e4d1f) * test: cover provider detection fallback retries * chore: format code (feat-provider-detect-templates-4dc1701) * fix: tighten provider detection validation --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * fix: honor disabled error rules during queued reloads (#1006) * fix: honor disabled error rules during queued reloads * fix: propagate matched rule metadata in hedge errors * fix: avoid hot retries on error rule reload failure * fix: avoid redundant error-rule reload passes * fix: await queued error rule reload completion * fix: prevent STREAM_PROCESSING_ERROR from premature agent pool eviction (#1009) * fix: prevent STREAM_PROCESSING_ERROR from premature agent pool eviction The agent pool cleanup timer was destroying HTTP agents after 5min TTL with no awareness of in-flight streaming requests, causing ClientDestroyedError and ERR_HTTP2_STREAM_ERROR for long-running responses. These errors were also misclassified because isTransportError did not recognize them. Three fixes: 1. Add UND_ERR_DESTROYED, UND_ERR_CLOSED, and HTTP/2 errors to isTransportError() so they classify as STREAM_UPSTREAM_ABORTED 2. Add reference counting (activeRequests) to AgentPool to prevent eviction of agents with in-flight requests (30min hard upper bound) 3. Wire up releaseAgent lifecycle in forwarder/response-handler to decrement refcount when streams complete Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: harden agent pool refcount against cross-generation misattribution Addresses bot review feedback on #1009 covering four root causes that still let STREAM_PROCESSING_ERROR leak through under concurrency: - Track dispatcher generation ids so releaseAgent can ignore stale releases from old dispatchers after markUnhealthy / 30-min hard expire / LRU eviction rebuilds the same cacheKey - Increment activeRequests for pending-creation waiters, not just the creator, so concurrent bursts don't prematurely evict the shared agent once the first request completes - Move agent acquisition inside forwarder's fetch try block so any exception between acquire and fetch still releases the refcount - Clear proxyConfig after a successful proxy-to-direct fallback to prevent session.releaseAgent double-decrementing the shared agent - Add ERR_HTTP2_STREAM_ERROR to transport error codes so wrapped HTTP/2 errors on the cause chain are detected by isTransportError Adds regression coverage for concurrent pending-creation waiter counting (Promise.all with blocked createAgent), stale dispatcher generation release safety, and HTTP/2 cause-chain detection. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: respect case-sensitive provider model ids (#1011) * fix: respect case-sensitive provider model ids * fix: keep model picker ids case-sensitive * chore: update package dependencies to use major version ranges * fix: address code review findings from PR #1014 - Use nullish coalescing (??) instead of logical OR (||) for parsed.content in test-service.ts to preserve empty string as valid content - Wrap parseResponse() in try-catch to prevent parse exceptions from masking HTTP status codes as network_error - Add releaseSessionAgent() to early-return branches in response-handler.ts to prevent agent pool reference count leaks - Fix error-rule-detector reload() to always queue a fresh pass when called during an active reload, preventing stale snapshot reads - Remove pre-clearing of isInitialized in event handler to avoid retry storms when database is unavailable - Use ICU {max, number} format in i18n messages for locale-aware number formatting of rule limits across all 5 languages - Remove emoji from test comments and fix pool cleanup in finally block Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: format code (dev-56698a2) --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Summary
Fix case-sensitive duplicate detection in provider allowlist and model redirect editors/schemas, so that model IDs differing only in case (e.g.
GLM-5vsglm-5) can coexist as separate rules.Problem
The runtime matching logic in
model-pattern-matcher.tsandallowed-model-rules.tsis case-sensitive --GLM-5andglm-5are treated as distinct model IDs. However, the duplicate-detection code in both the Zod validation schemas and the editor UI components applied.toLowerCase()before comparing rule identities. This meant:GLM-5(exact) andglm-5(exact) in the allowlist editor would incorrectly flag them as duplicatesRelated Issues/PRs:
getRuleIdentitySolution
Remove
.toLowerCase()from the identity/dedup key computation in four locations so that editor validation matches runtime matching semantics:provider-allowed-model-schema.ts(2 schemas) andprovider-model-redirect-schema.ts(1 schema)allowed-model-rule-editor.tsxandmodel-redirect-editor.tsx(getRuleIdentityfunctions)Changes
Core Changes
src/lib/provider-allowed-model-schema.ts-- Remove.toLowerCase()from duplicate-check keys in bothINPUT_LIST_SCHEMAandLIST_SCHEMAsrc/lib/provider-model-redirect-schema.ts-- Remove.toLowerCase()from duplicate-check key inLIST_SCHEMAsrc/app/[locale]/settings/providers/_components/allowed-model-rule-editor.tsx-- Remove.toLowerCase()fromgetRuleIdentitysrc/app/[locale]/settings/providers/_components/model-redirect-editor.tsx-- Remove.toLowerCase()fromgetRuleIdentityTest Coverage
tests/unit/lib/provider-allowed-model-schema.test.ts-- VerifyGLM-5andglm-5can coexist in allowlist schematests/unit/lib/provider-model-redirect-schema.test.ts-- VerifyGLM-5andglm-5can coexist as redirect sourcestests/unit/settings/providers/allowed-model-rule-editor.test.tsx-- Verify editor accepts both cased variants without errortests/unit/settings/providers/model-redirect-editor.test.tsx-- Verify redirect editor accepts both cased variantsTesting
bunx vitest run tests/unit/lib/provider-allowed-model-schema.test.ts tests/unit/lib/provider-model-redirect-schema.test.ts tests/unit/settings/providers/allowed-model-rule-editor.test.tsx tests/unit/settings/providers/model-redirect-editor.test.tsx bunx vitest run tests/unit/proxy/provider-selector-allowed-model-rules.test.ts bun run typecheck bun run lint bun run build bun run testChecklist
Description enhanced by Claude AI
Greptile Summary
This PR removes
.toLowerCase()from duplicate-detection keys in schema validation and editor UI components so that model IDs differing only in case (e.g.GLM-5vsglm-5) can coexist as separate rules, aligning editor semantics with the runtime's case-sensitive matching behavior. ThenormalizeAllowedModelRulestransform (which runs before schema dedup inPROVIDER_ALLOWED_MODEL_RULE_INPUT_LIST_SCHEMA) was confirmed to only trim whitespace — it does not lowercase — so the fix is effective end-to-end.Confidence Score: 5/5
Safe to merge — targeted one-line removals of
.toLowerCase()across five files, all consistent with runtime behavior and covered by new tests.All changed sites apply the same fix symmetrically. The
normalizeAllowedModelRulestransform (which precedes schema dedup) was confirmed to only trim whitespace, not lowercase, so the fix is effective end-to-end. New tests cover the primary regression case in every affected layer. No P0/P1 findings.No files require special attention.
Important Files Changed
.toLowerCase()from dedup keys in bothINPUT_LIST_SCHEMAandLIST_SCHEMA; dedup logic is now case-sensitive and consistent with runtime behavior..toLowerCase()from redirect rule dedup key; case-sensitive matching now applies consistently at both validation and runtime layers..toLowerCase()fromgetRuleIdentity; handles add, edit, remove, move, and model-picker path — all consistent after the fix..toLowerCase()fromgetRuleIdentity; edit-while-delete test confirms index-based save still works correctly under the case-sensitive key scheme..toLowerCase()fromgetModelKey; search filtering intentionally remains case-insensitive (UX), while selection/dedup keys are now case-sensitive.PROVIDER_ALLOWED_MODEL_RULE_INPUT_LIST_SCHEMA;PROVIDER_ALLOWED_MODEL_RULE_LIST_SCHEMA(also fixed) has no parallel case-sensitivity test, but both schemas use identical dedup logic so the fix is verifiable by inspection.PROVIDER_MODEL_REDIRECT_RULE_LIST_SCHEMA; covers the redirect schema fix directly.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["User enters model ID\n(e.g. GLM-5 or glm-5)"] --> B["Editor UI\ngetRuleIdentity(rule)\nmatchType + ':' + pattern.trim()"] B --> C{Duplicate check\ncase-sensitive} C -- "GLM-5 ≠ glm-5 ✓" --> D["Rule accepted\nonChange called"] C -- "GLM-5 = GLM-5 ✗" --> E["Error: duplicate rule"] D --> F["Zod Schema Validation\nmatchType + ':' + pattern.trim()"] F --> G{Dedup refine\ncase-sensitive} G -- "GLM-5 ≠ glm-5 ✓" --> H["Schema valid\nRules persisted"] G -- "duplicate detected ✗" --> I["Validation error"] H --> J["Runtime matching\nmodel-pattern-matcher.ts\ncase-sensitive"] J -- "GLM-5 matches GLM-5" --> K["Request allowed"] J -- "glm-5 matches glm-5" --> K style C fill:#22c55e,color:#fff style G fill:#22c55e,color:#fff style J fill:#22c55e,color:#fffReviews (2): Last reviewed commit: "fix: keep model picker ids case-sensitiv..." | Re-trigger Greptile