fix(keyword-routing): resolve i18n, error handling, and observability issues#1278
Closed
ItzArona wants to merge 13 commits into
Closed
fix(keyword-routing): resolve i18n, error handling, and observability issues#1278ItzArona wants to merge 13 commits into
ItzArona wants to merge 13 commits into
Conversation
…r redis invalidation path
…ain rendering - Created keyword routing settings page with master toggle, rule management UI - Implemented add/edit/delete dialogs with validation and confirmation - Added rule list table with priority, keyword, source/target models, case sensitivity - Integrated cache refresh functionality - Added navigation entry with filter icon - Completed i18n for all 5 locales (zh-CN, zh-TW, en, ja, ru) - Enhanced LogicTraceTab to display keyword routing decisions in logs - Updated provider-chain-formatter to include keyword routing timeline - Fixed optional chain lint issues in keyword-routing actions Components: - master-toggle.tsx: System-wide keyword routing enable/disable - add-rule-dialog.tsx: Create new routing rules with validation - edit-rule-dialog.tsx: Edit existing rules with current values - rule-list-table.tsx: Display rules sorted by priority with inline actions - refresh-cache-button.tsx: Clear keyword routing cache - page.tsx: Main settings page with sections i18n coverage: - keywordRouting.json: All UI strings for 5 locales - provider-chain.json: Decision chain rendering strings - nav.json: Navigation label Related: Task 8 - UI implementation for keyword model routing feature
… issues Core fixes: - Add i18n for all validation errors (14 keys × 5 languages) - Refactor actions to use getTranslations instead of hardcoded Chinese - Add errorCode field to all error responses (PERMISSION_DENIED, NOT_FOUND, etc.) - Extract validation constants to shared module (single source of truth) - Enhance observability: record audit when rule matches but target equals source - Add comprehensive authorization tests (18 test cases) - Fix test mocks for next-intl/server compatibility This resolves code review findings from the ultracode workflow. All critical and high-priority issues fixed. Tests: 24/24 action tests pass, typecheck and lint pass. Resolves: i18n compliance, error code mapping, validation constant duplication.
📝 Walkthrough通用概览该 PR 实现了完整的关键词模型路由功能，允许系统根据请求中的关键词自动改写目标模型，包括数据库迁移、核心匹配引擎、REST API、前端设置界面以及端到端的测试覆盖。 Changes关键词路由完整功能实现
代码审查复杂度评估🎯 4 (Complex) | ⏱️ ~75 minutes 该 PR 涉及多个子系统的深度集成：关键词匹配引擎的并发控制与补跑机制、请求处理管道中的模型改写与缓冲区重建、完整的 API 层设计、前端表单校验与状态管理、以及跨越 10 种语言的国际化配置。虽然变更高度结构化且测试完整，但需要理解从数据库、缓存、请求拦截到前端的端到端流程。 可能关联的 PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
|
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
Author
|
ai发神经乱发pr，对不起 |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR introduces keyword-based model routing: admins can configure keyword routing rules to rewrite the requested model before provider selection, controlled by a new system setting and backed by a hot-reloadable in-memory cache.
Changes:
- Add
enableKeywordModelRoutingsystem setting with DB migration + degradation ladder support. - Implement keyword routing rules CRUD (DB table + repository + server actions + v1 API) and a caching engine with local/Redis invalidation.
- Integrate keyword routing into the proxy guard pipeline and surface routing audit info in provider chain UI/timeline.
Reviewed changes
Copilot reviewed 90 out of 91 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/repository/system-config-update-missing-columns.test.ts | Updates degradation tests to cover the new system setting column and ladder ordering. |
| tests/unit/repository/system-config-degradation-ladder.test.ts | Extends column ladder assertions to include enableKeywordModelRouting. |
| tests/unit/repository/keyword-routing-events.test.ts | Adds repository unit tests for keyword routing rule persistence + event emission. |
| tests/unit/proxy/keyword-routing-guard.test.ts | Adds unit tests for the proxy keyword routing guard behavior and audit propagation. |
| tests/unit/proxy/guard-pipeline-keyword-routing.test.ts | Verifies keyword routing guard placement in the guard pipeline. |
| tests/unit/lib/message-extractor-keyword-routing.test.ts | Adds unit tests for extracting scan texts used by keyword routing. |
| tests/unit/lib/keyword-routing/matcher.test.ts | Adds unit tests for keyword routing matching semantics. |
| tests/unit/lib/keyword-routing/engine.test.ts | Adds unit tests for the keyword routing cache engine reload/invalidation behavior. |
| tests/unit/lib/hot-reload-singleton.test.ts | Extends singleton pattern tests to include the keyword routing engine. |
| tests/unit/lib/emit-event.test.ts | Adds tests for emitting keyword routing update events and Redis invalidation. |
| tests/unit/lib/config/system-settings-cache.test.ts | Adds cache helper test for isKeywordModelRoutingEnabled. |
| tests/unit/actions/system-config-save.test.ts | Ensures enableKeywordModelRouting is validated and saved via system config action. |
| tests/unit/actions/keyword-routing.test.ts | Adds tests for keyword routing server actions (CRUD, validation, authz, cache stats). |
| tests/setup.ts | Adds global next-intl/server mock and action compat mapping for keyword routing. |
| tests/api/v1/system/system-config.test.ts | Updates API system-config test fixture to include the new setting field. |
| tests/api/v1/keyword-routing/keyword-routing.test.ts | Adds API tests for keyword routing REST endpoints and OpenAPI docs. |
| tests/api/v1/keyword-routing/keyword-routing.crud.test.ts | Adds “evidence” test asserting keyword routing CRUD coverage via source inspection. |
| tests/api/v1/keyword-routing/keyword-routing.authz.test.ts | Adds API authorization tests for keyword routing endpoints. |
| src/types/system-config.ts | Adds enableKeywordModelRouting to settings types. |
| src/types/message.ts | Extends provider chain item type to include keyword routing audit metadata. |
| src/types/audit-log.ts | Adds keyword_routing_rule audit category. |
| src/repository/system-config.ts | Adds new column to fallback settings and degradation ladder; supports updating the new field. |
| src/repository/keyword-routing-rules.ts | Implements DB repository for keyword routing rules with event emission. |
| src/repository/_shared/transformers.ts | Adds transformer default for enableKeywordModelRouting. |
| src/repository/_shared/transformers.test.ts | Updates transformer tests for the new setting default. |
| src/lib/validation/schemas.ts | Adds enableKeywordModelRouting to update system settings validation schema. |
| src/lib/validation/keyword-routing-constants.ts | Introduces shared constants for keyword routing validation bounds. |
| src/lib/utils/provider-chain-formatter.ts | Adds timeline formatting for keyword routing audit metadata. |
| src/lib/redis/pubsub.ts | Adds Redis channel constant for keyword routing rules invalidation. |
| src/lib/message-extractor.ts | Adds keyword routing scan text extraction helpers. |
| src/lib/keyword-routing/matcher.ts | Adds matching logic for keyword routing rules. |
| src/lib/keyword-routing/engine.ts | Adds cache engine singleton with reload queueing + event/Redis invalidation. |
| src/lib/event-emitter.ts | Adds keywordRoutingRulesUpdated event to global emitter. |
| src/lib/emit-event.ts | Adds emitKeywordRoutingRulesUpdated to broadcast local + Redis invalidation. |
| src/lib/config/system-settings-cache.ts | Adds cached accessor isKeywordModelRoutingEnabled and default behavior. |
| src/lib/config/index.ts | Re-exports isKeywordModelRoutingEnabled. |
| src/lib/api/v1/schemas/system-config.ts | Exposes enableKeywordModelRouting via v1 system config schema. |
| src/lib/api/v1/schemas/keyword-routing.ts | Adds OpenAPI/Zod schemas for keyword routing REST API. |
| src/lib/api/v1/schemas/audit-logs.ts | Extends audit log category schema to include keyword routing rule category. |
| src/lib/api-client/v1/actions/keyword-routing.ts | Adds v1 client wrappers for keyword routing endpoints. |
| src/drizzle/schema.ts | Adds keyword_routing_rules table and system setting column to schema. |
| src/app/v1/_lib/proxy/session.ts | Stores and propagates keyword routing audit info onto provider chain items. |
| src/app/v1/_lib/proxy/keyword-routing-guard.ts | Implements the proxy guard to rewrite models based on keyword matches. |
| src/app/v1/_lib/proxy/guard-pipeline.ts | Registers the keyword routing guard step into the chat pipeline. |
| src/app/v1/[...route]/route.ts | Warms up keyword routing engine on startup (non-blocking). |
| src/app/api/v1/resources/keyword-routing/router.ts | Registers keyword routing REST routes in the v1 OpenAPI router. |
| src/app/api/v1/resources/keyword-routing/handlers.ts | Implements keyword routing route handlers bridging to server actions. |
| src/app/api/v1/_root/app.ts | Mounts the keyword routing router in the v1 API app. |
| src/app/api/admin/system-config/route.ts | Passes enableKeywordModelRouting through the admin system-config route. |
| src/app/[locale]/settings/keyword-routing/page.tsx | Adds settings page for keyword routing management. |
| src/app/[locale]/settings/keyword-routing/_components/rule-list-table.tsx | Adds UI table to view/update/delete routing rules. |
| src/app/[locale]/settings/keyword-routing/_components/refresh-cache-button.tsx | Adds UI to refresh keyword routing cache and show basic stats. |
| src/app/[locale]/settings/keyword-routing/_components/master-toggle.tsx | Adds UI master toggle backed by system settings. |
| src/app/[locale]/settings/keyword-routing/_components/keyword-routing-skeleton.tsx | Adds skeleton/loading UI for keyword routing settings page. |
| src/app/[locale]/settings/keyword-routing/_components/edit-rule-dialog.tsx | Adds UI dialog to edit a routing rule. |
| src/app/[locale]/settings/keyword-routing/_components/add-rule-dialog.tsx | Adds UI dialog to create a routing rule. |
| src/app/[locale]/settings/_lib/nav-items.ts | Adds keyword routing entry to settings navigation. |
| src/app/[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsx | Displays keyword routing audit info in provider chain trace UI. |
| src/app/[locale]/dashboard/audit-logs/_components/audit-logs-view.tsx | Adds keyword routing rule category to audit log filters. |
| src/actions/system-config.ts | Adds enableKeywordModelRouting to system settings save action payload. |
| src/actions/keyword-routing.ts | Implements admin-only server actions for keyword routing CRUD + cache operations + auditing. |
| src/actions/audit-logs.ts | Adds keyword routing category to audit actions. |
| messages/zh-TW/settings/nav.json | Adds keyword routing nav translation. |
| messages/zh-TW/settings/keywordRouting.json | Adds zh-TW translations for keyword routing settings UI. |
| messages/zh-TW/settings/index.ts | Registers keyword routing settings translations bundle. |
| messages/zh-TW/provider-chain.json | Adds zh-TW provider chain/timeline translations for keyword routing. |
| messages/zh-TW/auditLogs.json | Adds zh-TW audit log category/action labels for keyword routing. |
| messages/zh-CN/settings/nav.json | Adds keyword routing nav translation. |
| messages/zh-CN/settings/keywordRouting.json | Adds zh-CN translations for keyword routing settings UI. |
| messages/zh-CN/settings/index.ts | Registers keyword routing settings translations bundle. |
| messages/zh-CN/provider-chain.json | Adds zh-CN provider chain/timeline translations for keyword routing. |
| messages/zh-CN/auditLogs.json | Adds zh-CN audit log category/action labels for keyword routing. |
| messages/ru/settings/nav.json | Adds keyword routing nav translation. |
| messages/ru/settings/keywordRouting.json | Adds ru translations for keyword routing settings UI. |
| messages/ru/settings/index.ts | Registers keyword routing settings translations bundle. |
| messages/ru/provider-chain.json | Adds ru provider chain/timeline translations for keyword routing. |
| messages/ru/auditLogs.json | Adds ru audit log category/action labels for keyword routing. |
| messages/ja/settings/nav.json | Adds keyword routing nav translation. |
| messages/ja/settings/keywordRouting.json | Adds ja translations for keyword routing settings UI. |
| messages/ja/settings/index.ts | Registers keyword routing settings translations bundle. |
| messages/ja/provider-chain.json | Adds ja provider chain/timeline translations for keyword routing. |
| messages/ja/auditLogs.json | Adds ja audit log category/action labels for keyword routing. |
| messages/en/settings/nav.json | Adds keyword routing nav translation. |
| messages/en/settings/keywordRouting.json | Adds en translations for keyword routing settings UI. |
| messages/en/settings/index.ts | Registers keyword routing settings translations bundle. |
| messages/en/provider-chain.json | Adds en provider chain/timeline translations for keyword routing. |
| messages/en/auditLogs.json | Adds en audit log category/action labels for keyword routing. |
| drizzle/meta/_journal.json | Records the new migration in Drizzle journal. |
| drizzle/0106_stale_demogoblin.sql | Creates keyword_routing_rules table and adds enable_keyword_model_routing column. |
Comments suppressed due to low confidence (5)
src/lib/keyword-routing/engine.ts:1
setupEventListener()is async but is called withoutawait/voidhandling. If it ever rejects (e.g., an unexpected throw before the internal try/catch), this can surface as an unhandled promise rejection in Node (which may crash the process depending on runtime settings). Consider explicitly suppressing/handling the returned promise (e.g.,void this.setupEventListener().catch(...)) or refactoring the listener setup to be non-async with a safe “fire-and-forget” pattern.
src/repository/keyword-routing-rules.ts:1- This repository module is a DB/data-access layer, but adding
"use server"makes Next treat the file as a Server Actions module. That can impose Server Actions constraints on exports and increases the risk of accidental client invocation / bundling issues later. Recommendation: remove"use server"here and keep Server Actions directives limited to action entrypoints (e.g.,src/actions/*).
tests/api/v1/keyword-routing/keyword-routing.crud.test.ts:1 - This test asserts coverage by reading another test file’s source and matching strings. This is brittle (renames/refactors break it without behavior changes) and doesn’t validate runtime behavior. Prefer deleting this test or replacing it with behavioral assertions (e.g., keep the existing API tests as the source of truth, or add a focused test that exercises the specific CRUD paths you want guaranteed).
tests/unit/lib/keyword-routing/matcher.test.ts:1 - Corrected spelling of 'DIALOGE' to 'DIALOGUE' (typo in test data, repeated across assertions).
tests/unit/lib/keyword-routing/engine.test.ts:1 - Corrected spelling of 'DIALOGE' to 'DIALOGUE' (typo in test data).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a keyword-based model routing feature, allowing requests to be dynamically routed to a target model when prompts match configured keywords. It includes database schema updates, localized translation files, server actions, REST API endpoints, a proxy guard integrated into the chat pipeline, and a management UI. The review feedback highlights two critical robustness issues: missing null/undefined checks on the request message object in both the ProxyKeywordRoutingGuard and the extractKeywordRoutingTexts function, which could lead to runtime TypeError crashes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+61
to
+62
| // 提取待扫描文本（系统提示词 + 最后一条用户消息） | ||
| const texts = extractKeywordRoutingTexts(session.request.message); |
Contributor
There was a problem hiding this comment.
问题分析：
在调用 extractKeywordRoutingTexts(session.request.message) 之前，未对 session.request.message 进行空值校验。如果请求体解析失败、为空，或者模型是通过 URL/Header 等其他方式传入导致 session.request.message 为 null 或 undefined，此处将会抛出 TypeError 异常，导致代理请求中断。
解决方案：
建议在提取文本前，增加对 session.request.message 的防御性空值校验，确保系统在异常请求下能够安全降级放行。
Suggested change
| // 提取待扫描文本（系统提示词 + 最后一条用户消息） | |
| const texts = extractKeywordRoutingTexts(session.request.message); | |
| if (!session.request.message) { | |
| return null; | |
| } | |
| // 提取待扫描文本（系统提示词 + 最后一条用户消息） | |
| const texts = extractKeywordRoutingTexts(session.request.message); |
Comment on lines
+223
to
+225
| const systemTexts: string[] = []; | ||
| const lastUserTexts: string[] = []; | ||
|
|
Contributor
There was a problem hiding this comment.
问题分析：
如果传入的 message 为 null 或 undefined，在执行 "system" in message 时会抛出 TypeError 异常。为了保证函数的健壮性，应当在函数入口处进行防御性空值校验。
解决方案：
在函数开头增加对 message 的空值判断，若为空则直接返回空的扫描结果。
const systemTexts: string[] = [];
const lastUserTexts: string[] = [];
if (!message) {
return { systemTexts, lastUserTexts };
}![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: 6e4f55263a
ℹ️ 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".
Comment on lines
+31
to
+32
| if (result.ok) { | ||
| toast.success(isEnabled ? t("keywordRouting.enable") : t("keywordRouting.disable")); |
There was a problem hiding this comment.
When an admin enables or disables a keyword routing rule, the PATCH succeeds but the component never updates local state or calls router.refresh(). Because the switch is controlled by the server-provided rule.isEnabled prop, the UI stays on the old value until a manual page reload, making successful status changes look like they did not apply.
Useful? React with 👍 / 👎.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+62
to
+77
| const actions = await import("@/actions/keyword-routing"); | ||
| const result = await callAction( | ||
| c, | ||
| actions.deleteKeywordRoutingRuleAction, | ||
| [params.data.id] as never[], | ||
| c.get("auth") | ||
| ); | ||
| if (!result.ok) return actionError(c, result); | ||
| return noContentResponse(); | ||
| } | ||
|
|
||
| export async function refreshKeywordRoutingCache(c: Context): Promise<Response> { | ||
| const actions = await import("@/actions/keyword-routing"); | ||
| const result = await callAction(c, actions.refreshKeywordRoutingCacheAction, [], c.get("auth")); | ||
| if (!result.ok) return actionError(c, result); | ||
| return jsonResponse(result.data); |
There was a problem hiding this comment.
null return maps both auth failure and exceptions to 403
getKeywordRoutingCacheStats returns null for two different reasons: (1) the user is not an admin, and (2) any exception in the try block. The handler maps both to a 403 "Admin access is required" response. If getSession() throws due to a transient infrastructure issue (session store unavailable, context propagation error), an admin user receives a 403 instead of a 500, making an infrastructure failure indistinguishable from an authorization failure.
The action should return a proper ActionResult<...> like the other actions in this file — using { ok: false, error: ..., errorCode: "PERMISSION_DENIED" } for auth failures and letting exceptions propagate so callAction or the handler can return 500.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/api/v1/resources/keyword-routing/handlers.ts
Line: 62-77
Comment:
**`null` return maps both auth failure and exceptions to 403**
`getKeywordRoutingCacheStats` returns `null` for two different reasons: (1) the user is not an admin, and (2) any exception in the `try` block. The handler maps both to a 403 "Admin access is required" response. If `getSession()` throws due to a transient infrastructure issue (session store unavailable, context propagation error), an admin user receives a 403 instead of a 500, making an infrastructure failure indistinguishable from an authorization failure.
The action should return a proper `ActionResult<...>` like the other actions in this file — using `{ ok: false, error: ..., errorCode: "PERMISSION_DENIED" }` for auth failures and letting exceptions propagate so `callAction` or the handler can return 500.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+78
to
+93
| /** | ||
| * 获取所有关键词路由规则列表 | ||
| */ | ||
| export async function listKeywordRoutingRules(): Promise<repo.KeywordRoutingRule[]> { | ||
| try { | ||
| const session = await getSession(); | ||
| if (session?.user.role !== "admin") { | ||
| logger.warn("[KeywordRoutingAction] Unauthorized access attempt"); | ||
| return []; | ||
| } | ||
|
|
||
| return await repo.getAllKeywordRoutingRules(); | ||
| } catch (error) { | ||
| logger.error("[KeywordRoutingAction] Failed to list keyword routing rules:", error); | ||
| return []; | ||
| } |
There was a problem hiding this comment.
listKeywordRoutingRules silently swallows errors, breaking the ActionResult contract
This is the only action in the file that returns a raw value (KeywordRoutingRule[]) instead of an ActionResult. When a DB error occurs, the catch block returns [], and callAction wraps it as { ok: true, data: [] }, so the API handler returns HTTP 200 { items: [] }. This is indistinguishable from a successful response with no rules. It also means the if (!result.ok) return actionError(c, result) guard in the handler can never trigger for this action — authentication or DB failures are silently promoted to 200 responses.
Every other action in this file correctly returns ActionResult with distinct error codes. This one should follow the same pattern.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/actions/keyword-routing.ts
Line: 78-93
Comment:
**`listKeywordRoutingRules` silently swallows errors, breaking the `ActionResult` contract**
This is the only action in the file that returns a raw value (`KeywordRoutingRule[]`) instead of an `ActionResult`. When a DB error occurs, the `catch` block returns `[]`, and `callAction` wraps it as `{ ok: true, data: [] }`, so the API handler returns `HTTP 200 { items: [] }`. This is indistinguishable from a successful response with no rules. It also means the `if (!result.ok) return actionError(c, result)` guard in the handler can never trigger for this action — authentication or DB failures are silently promoted to 200 responses.
Every other action in this file correctly returns `ActionResult` with distinct error codes. This one should follow the same pattern.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+125
to
+128
| // Fallback to string matching for backward compatibility | ||
| if (detail.includes("不存在") || detail.includes("not found")) return 404; | ||
| if (detail.includes("权限")) return 403; | ||
| return 400; |
There was a problem hiding this comment.
The PR explicitly removes hardcoded Chinese strings from error handling, yet the getActionErrorStatus fallback still matches on "不存在" and "权限". These strings only match Chinese-locale error messages, so zh-TW, en, ja, and ru users would hit the default 400 branch instead. Since all new code paths supply errorCode, these fallbacks are dead code that can be removed.
Suggested change
| // Fallback to string matching for backward compatibility | |
| if (detail.includes("不存在") || detail.includes("not found")) return 404; | |
| if (detail.includes("权限")) return 403; | |
| return 400; | |
| return 500; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/api/v1/resources/keyword-routing/handlers.ts
Line: 125-128
Comment:
**Hardcoded Chinese strings in fallback error detection contradict the i18n goal**
The PR explicitly removes hardcoded Chinese strings from error handling, yet the `getActionErrorStatus` fallback still matches on `"不存在"` and `"权限"`. These strings only match Chinese-locale error messages, so zh-TW, en, ja, and ru users would hit the default `400` branch instead. Since all new code paths supply `errorCode`, these fallbacks are dead code that can be removed.
```suggestion
return 500;
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| try { | ||
| const { eventEmitter } = await import("@/lib/event-emitter"); | ||
| eventEmitter.emitKeywordRoutingRulesUpdated(); | ||
| } catch { |
Contributor
There was a problem hiding this comment.
[Medium] [ERROR-SILENT] Empty catch blocks silently swallow cache invalidation failures
Why this is a problem: Both catch blocks in emitKeywordRoutingRulesUpdated are empty (comment-only). If the local event emission or Redis pub/sub publish fails, the failure is completely invisible. When cache invalidation is silently lost, keyword routing rules may remain stale until the next restart, and there is no diagnostic signal to identify why.
The existing emitRequestFiltersUpdated function in this same file demonstrates the correct pattern - it imports a logger and uses logger?.warn?.() to record failures. The new function should follow that same pattern for observability consistency.
Suggested fix:
export async function emitKeywordRoutingRulesUpdated(): Promise<void> {
if (typeof process \!== "undefined" && process.env.NEXT_RUNTIME \!== "edge") {
let logger: typeof import("@/lib/logger").logger | undefined;
try {
({ logger } = await import("@/lib/logger"));
} catch {
// ignore - degrade without logger
}
try {
const { eventEmitter } = await import("@/lib/event-emitter");
eventEmitter.emitKeywordRoutingRulesUpdated();
logger?.info?.("[emitKeywordRoutingRulesUpdated] Local event emitted");
} catch (error) {
logger?.warn?.("[emitKeywordRoutingRulesUpdated] Failed to emit local event", { error });
}
try {
const { CHANNEL_KEYWORD_ROUTING_RULES_UPDATED, publishCacheInvalidation } = await import(
"@/lib/redis/pubsub"
);
await publishCacheInvalidation(CHANNEL_KEYWORD_ROUTING_RULES_UPDATED);
logger?.info?.("[emitKeywordRoutingRulesUpdated] Redis pub/sub publish attempted");
} catch (error) {
logger?.warn?.("[emitKeywordRoutingRulesUpdated] Failed to publish to Redis", { error });
}
}
}
Contributor
There was a problem hiding this comment.
Code Review Summary
This PR adds keyword-based model routing to the proxy pipeline with comprehensive test coverage (10 test files), proper i18n across 5 languages, and clean integration into the existing guard chain. The architecture is sound: matcher -> engine (singleton cache with hot-reload) -> guard (model rewrite before provider selection). Error handling follows fail-open semantics for the proxy path, which is appropriate.
One medium-severity observability issue found: empty catch blocks in the new event emission function miss the logging pattern already established by emitRequestFiltersUpdated in the same file.
PR Size: XL
- Lines changed: 11,616 (11,572 additions + 44 deletions)
- Files changed: 91
Note: The raw size is inflated by auto-generated Drizzle migration snapshots (~4,600 lines), i18n translation files (~1,500 lines across 5 locales), and 10 test files (~1,800 lines). The core application logic is approximately 10 source files (~1,200 lines). A split is not recommended as the changes are cohesive.
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 |
Medium Priority Issues (Should Fix)
- [ERROR-SILENT]
emitKeywordRoutingRulesUpdateduses empty catch blocks (src/lib/emit-event.ts:62,71) - Both catch blocks are comment-only, silently swallowing event emission and Redis pub/sub failures. The existingemitRequestFiltersUpdatedin the same file demonstrates the correct pattern withlogger?.warn?.(). See inline comment for suggested fix.
Review Coverage
- Logic and correctness - Guard chain ordering verified (keywordRouting after requestFilter, before rateLimit/provider)
- Security (OWASP Top 10) - Admin-only auth enforced at action and API middleware layers
- Error handling - Fail-open proxy path appropriate; observability gap in event emission
- Type safety - Strong typing throughout; Zod schemas shared between API and action layers
- Documentation accuracy - Chinese comments accurately describe code behavior
- Test coverage - 10 test files covering engine, matcher, guard, message extraction, authz, actions, repository events, and pipeline integration
- Code clarity - Well-structured with clear separation of concerns
Automated review by Claude AI
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (7)
tests/api/v1/keyword-routing/keyword-routing.crud.test.ts (1)
5-10: ⚡ Quick win当前用例只校验源码字符串，无法验证 CRUD 行为本身。
Line 6 读取
keyword-routing.test.ts文本并断言toContain，会产生“假覆盖”：路由逻辑坏了也可能通过，文案改动却会失败。建议删掉这个元测试，或改为直接通过callV1Route断言真实响应语义。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/api/v1/keyword-routing/keyword-routing.crud.test.ts` around lines 5 - 10, The current test "keyword routing tests cover REST CRUD" just reads the source via readFileSync and uses toContain, which is a meta-test that doesn't verify CRUD behavior; remove this meta assertion (the readFileSync + expect(...toContain...) checks) and replace it with real HTTP assertions using the callV1Route helper: invoke endpoints like "/api/v1/keyword-routing-rules" and "/api/v1/keyword-routing-rules/cache:refresh" via callV1Route, assert expected status codes and response bodies for list/create/update/delete flows (and cache refresh), and keep or add setup/teardown data to make the CRUD assertions deterministic. Ensure you remove references to the previous readFileSync usage and update the test name or assertions accordingly so the test verifies actual route semantics rather than source text.tests/unit/repository/system-config-update-missing-columns.test.ts (1)
351-399: ⚡ Quick win补齐“默认开启”语义的断言，避免回归漏检。
该用例标题明确验证
enable_thinking_effort_conflict_rectifier缺列时“默认开启”，但当前没有直接断言这个返回值。建议补一条断言把测试目标闭环。建议修改
@@ expect(selectMock).toHaveBeenCalledTimes(3); expect(result.siteTitle).toBe("Claude Code Hub"); expect(result.enableHttp2).toBe(true); + expect(result.enableThinkingEffortConflictRectifier).toBe(true);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/repository/system-config-update-missing-columns.test.ts` around lines 351 - 399, The test is missing an assertion that verifies the expected default-on behaviour for the missing column; update the test that calls getSystemSettings so it asserts the returned result.enableThinkingEffortConflictRectifier is true (use the existing result variable and the same expect(...) style used for siteTitle and enableHttp2) to close the verification loop that the column defaults to enabled when absent.src/lib/emit-event.ts (1)
54-75: ⚡ Quick win建议统一错误处理，提升可观测性。
新增的
emitKeywordRoutingRulesUpdated函数使用了静默错误处理（空 catch 块），而同文件中较新的emitRequestFiltersUpdated（80-107 行）采用了基于 logger 的错误处理模式。建议将此函数改为与emitRequestFiltersUpdated一致的错误处理方式，以便在事件发射或 Redis 发布失败时记录日志，提升系统可观测性。♻️ 建议的改进
/** * 触发 keywordRoutingRulesUpdated 事件 */ export async function emitKeywordRoutingRulesUpdated(): Promise<void> { if (typeof process !== "undefined" && process.env.NEXT_RUNTIME !== "edge") { + let logger: typeof import("`@/lib/logger`").logger | undefined; + try { + ({ logger } = await import("`@/lib/logger`")); + } catch { + // 忽略导入错误 (silent degrade) + } + try { const { eventEmitter } = await import("`@/lib/event-emitter`"); eventEmitter.emitKeywordRoutingRulesUpdated(); - } catch { - // 忽略导入错误 + logger?.info?.("[emitKeywordRoutingRulesUpdated] Local event emitted"); + } catch (error) { + logger?.warn?.("[emitKeywordRoutingRulesUpdated] Failed to emit local event", { error }); } try { const { CHANNEL_KEYWORD_ROUTING_RULES_UPDATED, publishCacheInvalidation } = await import( "`@/lib/redis/pubsub`" ); await publishCacheInvalidation(CHANNEL_KEYWORD_ROUTING_RULES_UPDATED); - } catch { - // 忽略导入错误 + logger?.info?.("[emitKeywordRoutingRulesUpdated] Redis pub/sub publish attempted"); + } catch (error) { + logger?.warn?.("[emitKeywordRoutingRulesUpdated] Failed to publish to Redis", { error }); } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/emit-event.ts` around lines 54 - 75, The new emitKeywordRoutingRulesUpdated function currently swallows import/publish errors with empty catch blocks; change its error handling to match emitRequestFiltersUpdated by catching errors and logging them (use the same logger/processLogger used in this file) when calling eventEmitter.emitKeywordRoutingRulesUpdated() and when awaiting publishCacheInvalidation(CHANNEL_KEYWORD_ROUTING_RULES_UPDATED), so each catch should log a clear contextual message and the error object rather than being empty.src/app/[locale]/settings/_lib/nav-items.ts (1)
55-60: 💤 Low value图标重复使用可能导致视觉混淆。
新增的 "keyword-routing" 导航项与 "request-filters"（第 71 行）使用了相同的
filter图标。虽然两者在语义上都与过滤/路由相关，但在导航菜单中使用相同图标可能降低可辨识度。建议考虑为关键词路由选择更具区分度的图标（如
route、git-branch、shuffle等），或确认重复使用是有意为之的设计决策。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`[locale]/settings/_lib/nav-items.ts around lines 55 - 60, The new nav item object for href "/settings/keyword-routing" (labelKey "nav.keywordRouting", label "Keyword Routing") reuses iconName "filter", which duplicates the "request-filters" nav entry; change the iconName to a more distinct value (for example "route", "git-branch", or "shuffle") in src/app/[locale]/settings/_lib/nav-items.ts by updating the keyword-routing nav item's iconName property to the chosen unique iconName (or document confirmation if the duplicate was intentional).src/app/[locale]/settings/keyword-routing/_components/add-rule-dialog.tsx (1)
84-84: 💤 Low value考虑在对话框关闭时重置表单状态。
当前实现中，对话框的
onOpenChange绑定到setOpen，但表单状态在用户取消或关闭对话框时不会重置。这意味着用户再次打开对话框时会看到之前的输入内容。这可能是有意为之的 UX 设计（保留草稿），也可能是疏忽。如果希望每次打开对话框时都从空白状态开始，建议添加重置逻辑：
可选的重置逻辑
- <Dialog open={open} onOpenChange={setOpen}> + <Dialog open={open} onOpenChange={(newOpen) => { + setOpen(newOpen); + if (!newOpen) resetForm(); + }}>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`[locale]/settings/keyword-routing/_components/add-rule-dialog.tsx at line 84, The dialog's onOpenChange is directly bound to setOpen so when the Dialog closes the form state isn't cleared; update the handler used by Dialog (leave the open prop as-is) to intercept close events and reset the form state — e.g., replace onOpenChange={setOpen} with a small handler that calls setOpen(value) and when value is false also calls your form reset function (reset(), setFormState(initialState), or clear controlled inputs) inside the AddRuleDialog component so the form is cleared each time the Dialog closes.src/app/[locale]/settings/keyword-routing/_components/refresh-cache-button.tsx (1)
29-29: ⚡ Quick win类型断言不安全，应使用正确的类型定义。
当前使用
as { stats: { ruleCount: number } }进行类型断言，这会绕过 TypeScript 的类型检查。如果refreshKeywordRoutingCacheAction的返回类型发生变化，此处不会得到编译时错误提示。建议在
src/lib/api-client/v1/actions/keyword-routing.ts中为该 action 定义明确的返回类型，然后在此处使用类型守卫或直接依赖类型推导。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`[locale]/settings/keyword-routing/_components/refresh-cache-button.tsx at line 29, The code unsafely asserts result.data as { stats: { ruleCount: number } }; instead, give refreshKeywordRoutingCacheAction an explicit return type (e.g., Promise<{ data: { stats: { ruleCount: number } } }>) and update its signature so callers get proper types, then remove the inline "as" assertion here and either rely on the inferred property (const count = result.data.stats.ruleCount) or add a small type guard/null check (if (!result?.data?.stats) throw or handle) before reading ruleCount to avoid runtime errors.src/app/[locale]/settings/keyword-routing/_components/rule-list-table.tsx (1)
140-140: ⚡ Quick win建议使用区域感知的日期格式化。
当前使用硬编码的日期格式字符串
"yyyy-MM-dd HH:mm:ss"，这对所有语言环境都显示相同格式。不同区域的用户可能期望不同的日期/时间格式（例如美国用户习惯MM/dd/yyyy）。考虑使用
next-intl的useFormatter()或根据timeZone配置区域特定的格式字符串，以提供更好的国际化体验。♻️ 建议的改进方案
// 方案 1: 使用 next-intl 的 useFormatter import { useFormatter } from "next-intl"; const format = useFormatter(); // ... {format.dateTime(new Date(rule.createdAt), { year: 'numeric', month: '2-digit', day: '2-digit', hour: '2-digit', minute: '2-digit', second: '2-digit', timeZone })} // 方案 2: 基于语言环境的格式映射 const dateFormats: Record<string, string> = { 'en': 'MM/dd/yyyy HH:mm:ss', 'zh-CN': 'yyyy-MM-dd HH:mm:ss', 'zh-TW': 'yyyy-MM-dd HH:mm:ss', 'ja': 'yyyy/MM/dd HH:mm:ss', 'ru': 'dd.MM.yyyy HH:mm:ss', }; const locale = useLocale(); const dateFormat = dateFormats[locale] || 'yyyy-MM-dd HH:mm:ss';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`[locale]/settings/keyword-routing/_components/rule-list-table.tsx at line 140, The code formats dates with a hardcoded pattern via formatInTimeZone(new Date(rule.createdAt), timeZone, "yyyy-MM-dd HH:mm:ss"); update this to use a locale-aware formatter: import and call next-intl's useFormatter() (e.g. const format = useFormatter()) and replace the formatInTimeZone call with format.dateTime(new Date(rule.createdAt), { year:'numeric', month:'2-digit', day:'2-digit', hour:'2-digit', minute:'2-digit', second:'2-digit', timeZone }) so the output respects the user's locale/timeZone; alternatively, if you prefer mapping by locale use useLocale() to pick a localized format string and pass that to your formatter—ensure changes touch the expression using rule.createdAt and timeZone and replace the hardcoded format string there.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/actions/keyword-routing.ts`:
- Around line 81-93: The code in listKeywordRoutingRules (and the other affected
keyword-routing handlers) is swallowing backend exceptions by returning []/null
on errors or unauthorized checks; instead, for listKeywordRoutingRules keep the
authorization check but throw an explicit 403/Unauthorized error when
session?.user.role !== "admin", and in the try/catch either remove the catch so
exceptions from repo.getAllKeywordRoutingRules() bubble up (causing a 500) or
rethrow the caught error after logging (do not return []/null); apply the same
change to the other keyword-routing functions that currently return []/null in
their catch blocks so real failures surface to callers.
- Around line 62-64: The length check for description must match the API's
.trim().max(...) semantics: instead of comparing fields.description.length
directly, check the trimmed length (e.g. use fields.description?.trim().length
or compute const desc = fields.description?.trim(); and then if (desc != null &&
desc.length > DESCRIPTION_MAX_LENGTH) return t("descriptionMaxLength", { max:
DESCRIPTION_MAX_LENGTH });) so leading/trailing whitespace doesn't change
validation; keep the same DESCRIPTION_MAX_LENGTH and translation call.
In `@src/app/`[locale]/settings/keyword-routing/_components/edit-rule-dialog.tsx:
- Around line 39-48: The effect that fills the form (useEffect) only depends on
rule, so when the edit dialog is closed and reopened without rule changing the
local form state (keyword, sourceModel, targetModel, caseSensitive, priority,
description) can retain stale values; update the useEffect to also depend on the
dialog visibility prop (e.g., open or isOpen) and run when the dialog opens so
that on open you call setKeyword, setSourceModel, setTargetModel,
setCaseSensitive, setPriority, setDescription using rule (or sensible defaults
when rule is undefined) to guarantee a fresh form each time the dialog is
opened.
- Around line 66-73: The priority parsing silently truncates decimals and falls
back to 0; change the validation in edit-rule-dialog (where parsedPriority is
computed before calling updateKeywordRoutingRuleAction) to enforce a strict
integer within the allowed range: parse with Number.parseInt and then check that
String(parsedPriority) === String(priority).replace(/^\s+|\s+$/g,'') (or use a
/^\-?\d+$/ test) and that parsedPriority falls inside the allowed range from
messages (use the i18n keys keywordRouting.validation.priorityInteger and
keywordRouting.validation.priorityRange for error strings); on validation
failure call toast.error(...) with the appropriate i18n message and return early
so updateKeywordRoutingRuleAction is not invoked. Ensure you still trim other
fields as before and only call updateKeywordRoutingRuleAction when the priority
passes these checks.
In `@src/app/`[locale]/settings/keyword-routing/_components/master-toggle.tsx:
- Around line 19-32: The toast currently displays raw result.error in
handleToggle; instead use result.errorCode to produce an i18n message: import
and initialize tErrors = useTranslations("errors") and call
getErrorMessage(tErrors, result.errorCode) when result.errorCode exists, falling
back to t("saveFailed") otherwise, and pass that string to toast.error; keep the
existing setIsEnabled/toast.success behavior for the success path and leave
saveSystemSettings as-is.
In `@src/app/api/v1/resources/keyword-routing/router.ts`:
- Around line 35-52: The OpenAPI problemResponses constant is missing a 500
entry (handler can return OPERATION_FAILED -> 500); update the problemResponses
object (symbol: problemResponses) to include a 500 response with a suitable
description (e.g., "Internal server error.") and the same content/schema as the
other entries (application/problem+json using ProblemJsonSchema) so the spec
matches the OPERATION_FAILED behavior.
In `@src/lib/keyword-routing/engine.ts`:
- Around line 34-79: The async setupEventListener has a race with destroy: add a
boolean flag (e.g., this.isDestroyed) initialized false, set it true at the
start of destroy(), and use it inside setupEventListener to avoid registering
handlers after destruction; specifically, before calling eventEmitter.on(...)
and before storing this.eventEmitterCleanup or this.redisPubSubCleanup (and
immediately call/off any returned cleanup if isDestroyed became true after the
async import/subscribe), and ensure the handler variable remains the same
reference so any immediate cleanup can off() it; update constructor/
setupEventListener/ destroy to use this.isDestroyed and perform the immediate
cleanup-if-destroyed logic rather than only storing cleanup callbacks.
In `@src/lib/message-extractor.ts`:
- Around line 220-264: The extractKeywordRoutingTexts function is missing Gemini
fields: add handling so message.systemInstruction (treat as system text) and
message.contents (treat like messages) are scanned. Concretely: in
extractKeywordRoutingTexts, if typeof message.systemInstruction === "string"
push it into systemTexts (or pass through extractSystemText if appropriate); if
Array.isArray(message.contents) call collectRoleScanTexts(message.contents,
systemTexts, lastUserTexts); else if typeof message.contents === "string" push
it into lastUserTexts. This ensures Gemini's systemInstruction and contents are
included in keyword routing scans.
In `@tests/setup.ts`:
- Around line 13-22: Update the test mock so the async signatures of next-intl
v4 are preserved: change getTranslations, getLocale and getMessages to return
via vi.fn(async () => ...); specifically, make getTranslations a vi.fn(async ()
=> async (key: string, params?: Record<string, unknown>) => { ... }) so the
translator function is async, and change getLocale and getMessages to
vi.fn(async () => "en") and vi.fn(async () => ({})) respectively; then scan the
repo for other mocks of next-intl and convert them similarly.
---
Nitpick comments:
In `@src/app/`[locale]/settings/_lib/nav-items.ts:
- Around line 55-60: The new nav item object for href
"/settings/keyword-routing" (labelKey "nav.keywordRouting", label "Keyword
Routing") reuses iconName "filter", which duplicates the "request-filters" nav
entry; change the iconName to a more distinct value (for example "route",
"git-branch", or "shuffle") in src/app/[locale]/settings/_lib/nav-items.ts by
updating the keyword-routing nav item's iconName property to the chosen unique
iconName (or document confirmation if the duplicate was intentional).
In `@src/app/`[locale]/settings/keyword-routing/_components/add-rule-dialog.tsx:
- Line 84: The dialog's onOpenChange is directly bound to setOpen so when the
Dialog closes the form state isn't cleared; update the handler used by Dialog
(leave the open prop as-is) to intercept close events and reset the form state —
e.g., replace onOpenChange={setOpen} with a small handler that calls
setOpen(value) and when value is false also calls your form reset function
(reset(), setFormState(initialState), or clear controlled inputs) inside the
AddRuleDialog component so the form is cleared each time the Dialog closes.
In
`@src/app/`[locale]/settings/keyword-routing/_components/refresh-cache-button.tsx:
- Line 29: The code unsafely asserts result.data as { stats: { ruleCount: number
} }; instead, give refreshKeywordRoutingCacheAction an explicit return type
(e.g., Promise<{ data: { stats: { ruleCount: number } } }>) and update its
signature so callers get proper types, then remove the inline "as" assertion
here and either rely on the inferred property (const count =
result.data.stats.ruleCount) or add a small type guard/null check (if
(!result?.data?.stats) throw or handle) before reading ruleCount to avoid
runtime errors.
In `@src/app/`[locale]/settings/keyword-routing/_components/rule-list-table.tsx:
- Line 140: The code formats dates with a hardcoded pattern via
formatInTimeZone(new Date(rule.createdAt), timeZone, "yyyy-MM-dd HH:mm:ss");
update this to use a locale-aware formatter: import and call next-intl's
useFormatter() (e.g. const format = useFormatter()) and replace the
formatInTimeZone call with format.dateTime(new Date(rule.createdAt), {
year:'numeric', month:'2-digit', day:'2-digit', hour:'2-digit',
minute:'2-digit', second:'2-digit', timeZone }) so the output respects the
user's locale/timeZone; alternatively, if you prefer mapping by locale use
useLocale() to pick a localized format string and pass that to your
formatter—ensure changes touch the expression using rule.createdAt and timeZone
and replace the hardcoded format string there.
In `@src/lib/emit-event.ts`:
- Around line 54-75: The new emitKeywordRoutingRulesUpdated function currently
swallows import/publish errors with empty catch blocks; change its error
handling to match emitRequestFiltersUpdated by catching errors and logging them
(use the same logger/processLogger used in this file) when calling
eventEmitter.emitKeywordRoutingRulesUpdated() and when awaiting
publishCacheInvalidation(CHANNEL_KEYWORD_ROUTING_RULES_UPDATED), so each catch
should log a clear contextual message and the error object rather than being
empty.
In `@tests/api/v1/keyword-routing/keyword-routing.crud.test.ts`:
- Around line 5-10: The current test "keyword routing tests cover REST CRUD"
just reads the source via readFileSync and uses toContain, which is a meta-test
that doesn't verify CRUD behavior; remove this meta assertion (the readFileSync
+ expect(...toContain...) checks) and replace it with real HTTP assertions using
the callV1Route helper: invoke endpoints like "/api/v1/keyword-routing-rules"
and "/api/v1/keyword-routing-rules/cache:refresh" via callV1Route, assert
expected status codes and response bodies for list/create/update/delete flows
(and cache refresh), and keep or add setup/teardown data to make the CRUD
assertions deterministic. Ensure you remove references to the previous
readFileSync usage and update the test name or assertions accordingly so the
test verifies actual route semantics rather than source text.
In `@tests/unit/repository/system-config-update-missing-columns.test.ts`:
- Around line 351-399: The test is missing an assertion that verifies the
expected default-on behaviour for the missing column; update the test that calls
getSystemSettings so it asserts the returned
result.enableThinkingEffortConflictRectifier is true (use the existing result
variable and the same expect(...) style used for siteTitle and enableHttp2) to
close the verification loop that the column defaults to enabled when absent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: df16cc76-9de0-4f97-af03-f7493f82993f
📒 Files selected for processing (91)
drizzle/0106_stale_demogoblin.sqldrizzle/meta/0106_snapshot.jsondrizzle/meta/_journal.jsonmessages/en/auditLogs.jsonmessages/en/provider-chain.jsonmessages/en/settings/index.tsmessages/en/settings/keywordRouting.jsonmessages/en/settings/nav.jsonmessages/ja/auditLogs.jsonmessages/ja/provider-chain.jsonmessages/ja/settings/index.tsmessages/ja/settings/keywordRouting.jsonmessages/ja/settings/nav.jsonmessages/ru/auditLogs.jsonmessages/ru/provider-chain.jsonmessages/ru/settings/index.tsmessages/ru/settings/keywordRouting.jsonmessages/ru/settings/nav.jsonmessages/zh-CN/auditLogs.jsonmessages/zh-CN/provider-chain.jsonmessages/zh-CN/settings/index.tsmessages/zh-CN/settings/keywordRouting.jsonmessages/zh-CN/settings/nav.jsonmessages/zh-TW/auditLogs.jsonmessages/zh-TW/provider-chain.jsonmessages/zh-TW/settings/index.tsmessages/zh-TW/settings/keywordRouting.jsonmessages/zh-TW/settings/nav.jsonsrc/actions/audit-logs.tssrc/actions/keyword-routing.tssrc/actions/system-config.tssrc/app/[locale]/dashboard/audit-logs/_components/audit-logs-view.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsxsrc/app/[locale]/settings/_lib/nav-items.tssrc/app/[locale]/settings/keyword-routing/_components/add-rule-dialog.tsxsrc/app/[locale]/settings/keyword-routing/_components/edit-rule-dialog.tsxsrc/app/[locale]/settings/keyword-routing/_components/keyword-routing-skeleton.tsxsrc/app/[locale]/settings/keyword-routing/_components/master-toggle.tsxsrc/app/[locale]/settings/keyword-routing/_components/refresh-cache-button.tsxsrc/app/[locale]/settings/keyword-routing/_components/rule-list-table.tsxsrc/app/[locale]/settings/keyword-routing/page.tsxsrc/app/api/admin/system-config/route.tssrc/app/api/v1/_root/app.tssrc/app/api/v1/resources/keyword-routing/handlers.tssrc/app/api/v1/resources/keyword-routing/router.tssrc/app/v1/[...route]/route.tssrc/app/v1/_lib/proxy/guard-pipeline.tssrc/app/v1/_lib/proxy/keyword-routing-guard.tssrc/app/v1/_lib/proxy/session.tssrc/drizzle/schema.tssrc/lib/api-client/v1/actions/keyword-routing.tssrc/lib/api-client/v1/openapi-types.gen.tssrc/lib/api/v1/schemas/audit-logs.tssrc/lib/api/v1/schemas/keyword-routing.tssrc/lib/api/v1/schemas/system-config.tssrc/lib/config/index.tssrc/lib/config/system-settings-cache.tssrc/lib/emit-event.tssrc/lib/event-emitter.tssrc/lib/keyword-routing/engine.tssrc/lib/keyword-routing/matcher.tssrc/lib/message-extractor.tssrc/lib/redis/pubsub.tssrc/lib/utils/provider-chain-formatter.tssrc/lib/validation/keyword-routing-constants.tssrc/lib/validation/schemas.tssrc/repository/_shared/transformers.test.tssrc/repository/_shared/transformers.tssrc/repository/keyword-routing-rules.tssrc/repository/system-config.tssrc/types/audit-log.tssrc/types/message.tssrc/types/system-config.tstests/api/v1/keyword-routing/keyword-routing.authz.test.tstests/api/v1/keyword-routing/keyword-routing.crud.test.tstests/api/v1/keyword-routing/keyword-routing.test.tstests/api/v1/system/system-config.test.tstests/setup.tstests/unit/actions/keyword-routing.test.tstests/unit/actions/system-config-save.test.tstests/unit/lib/config/system-settings-cache.test.tstests/unit/lib/emit-event.test.tstests/unit/lib/hot-reload-singleton.test.tstests/unit/lib/keyword-routing/engine.test.tstests/unit/lib/keyword-routing/matcher.test.tstests/unit/lib/message-extractor-keyword-routing.test.tstests/unit/proxy/guard-pipeline-keyword-routing.test.tstests/unit/proxy/keyword-routing-guard.test.tstests/unit/repository/keyword-routing-events.test.tstests/unit/repository/system-config-degradation-ladder.test.tstests/unit/repository/system-config-update-missing-columns.test.ts
Comment on lines
+62
to
+64
| if (fields.description != null && fields.description.length > DESCRIPTION_MAX_LENGTH) { | ||
| return t("descriptionMaxLength", { max: DESCRIPTION_MAX_LENGTH }); | ||
| } |
There was a problem hiding this comment.
description 的长度校验应与 API Schema 保持同一语义。
Line 62 直接用原始长度比较，未 trim()；而 API 入口是 .trim().max(...)。同一输入在 Action 与 API 可能得到不同校验结果。
建议修改
- if (fields.description != null && fields.description.length > DESCRIPTION_MAX_LENGTH) {
+ if (
+ fields.description != null &&
+ fields.description.trim().length > DESCRIPTION_MAX_LENGTH
+ ) {
return t("descriptionMaxLength", { max: DESCRIPTION_MAX_LENGTH });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (fields.description != null && fields.description.length > DESCRIPTION_MAX_LENGTH) { | |
| return t("descriptionMaxLength", { max: DESCRIPTION_MAX_LENGTH }); | |
| } | |
| if ( | |
| fields.description != null && | |
| fields.description.trim().length > DESCRIPTION_MAX_LENGTH | |
| ) { | |
| return t("descriptionMaxLength", { max: DESCRIPTION_MAX_LENGTH }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/actions/keyword-routing.ts` around lines 62 - 64, The length check for
description must match the API's .trim().max(...) semantics: instead of
comparing fields.description.length directly, check the trimmed length (e.g. use
fields.description?.trim().length or compute const desc =
fields.description?.trim(); and then if (desc != null && desc.length >
DESCRIPTION_MAX_LENGTH) return t("descriptionMaxLength", { max:
DESCRIPTION_MAX_LENGTH });) so leading/trailing whitespace doesn't change
validation; keep the same DESCRIPTION_MAX_LENGTH and translation call.
Comment on lines
+81
to
+93
| export async function listKeywordRoutingRules(): Promise<repo.KeywordRoutingRule[]> { | ||
| try { | ||
| const session = await getSession(); | ||
| if (session?.user.role !== "admin") { | ||
| logger.warn("[KeywordRoutingAction] Unauthorized access attempt"); | ||
| return []; | ||
| } | ||
|
|
||
| return await repo.getAllKeywordRoutingRules(); | ||
| } catch (error) { | ||
| logger.error("[KeywordRoutingAction] Failed to list keyword routing rules:", error); | ||
| return []; | ||
| } |
There was a problem hiding this comment.
不要用空数组或 null 吞掉后端异常。
Line 90-93 与 Line 399-402 在异常时返回成功态值（[]/null），会把真实故障伪装成“空数据/无权限”，下游 API 可能返回 200 或 403，而不是 500。
建议修改
export async function listKeywordRoutingRules(): Promise<repo.KeywordRoutingRule[]> {
try {
@@
} catch (error) {
logger.error("[KeywordRoutingAction] Failed to list keyword routing rules:", error);
- return [];
+ throw error;
}
}
@@
export async function getKeywordRoutingCacheStats() {
try {
@@
} catch (error) {
logger.error("[KeywordRoutingAction] Failed to get cache stats:", error);
- return null;
+ throw error;
}
}Also applies to: 391-402
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/actions/keyword-routing.ts` around lines 81 - 93, The code in
listKeywordRoutingRules (and the other affected keyword-routing handlers) is
swallowing backend exceptions by returning []/null on errors or unauthorized
checks; instead, for listKeywordRoutingRules keep the authorization check but
throw an explicit 403/Unauthorized error when session?.user.role !== "admin",
and in the try/catch either remove the catch so exceptions from
repo.getAllKeywordRoutingRules() bubble up (causing a 500) or rethrow the caught
error after logging (do not return []/null); apply the same change to the other
keyword-routing functions that currently return []/null in their catch blocks so
real failures surface to callers.
Comment on lines
+39
to
+48
| useEffect(() => { | ||
| if (rule) { | ||
| setKeyword(rule.keyword); | ||
| setSourceModel(rule.sourceModel || ""); | ||
| setTargetModel(rule.targetModel); | ||
| setCaseSensitive(rule.caseSensitive); | ||
| setPriority(String(rule.priority)); | ||
| setDescription(rule.description || ""); | ||
| } | ||
| }, [rule]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 验证点：是否仅在 [rule] 变化时同步，未包含 open
rg -n 'useEffect\\(\\s*\\(\\)\\s*=>[\\s\\S]*\\[rule\\]\\s*\\)' src/app/[locale]/settings/keyword-routing/_components/edit-rule-dialog.tsx -U
rg -n 'onOpenChange\\(false\\)' src/app/[locale]/settings/keyword-routing/_components/edit-rule-dialog.tsxRepository: ding113/claude-code-hub
Length of output: 49
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/app/[locale]/settings/keyword-routing/_components/edit-rule-dialog.tsx"
echo "== useEffect blocks around state sync =="
rg -n "useEffect\\(" "$FILE" -n
echo
echo "== show lines 1-120 =="
sed -n '1,120p' "$FILE" | cat -n
echo
echo "== search for open / onOpenChange usage =="
rg -n "open|onOpenChange|setOpen" "$FILE" -nRepository: ding113/claude-code-hub
Length of output: 5692
编辑弹窗复开时未重置表单状态，可能提交脏数据
useEffect 仅在 rule 变化时回填（依赖数组为 [rule]）。当弹窗关闭/复开但 rule 引用未变时，表单本地 state 不会重新同步，可能沿用上次未保存的输入并被误提交。
建议修复（示例）
- useEffect(() => {
- if (rule) {
- setKeyword(rule.keyword);
- setSourceModel(rule.sourceModel || "");
- setTargetModel(rule.targetModel);
- setCaseSensitive(rule.caseSensitive);
- setPriority(String(rule.priority));
- setDescription(rule.description || "");
- }
- }, [rule]);
+ useEffect(() => {
+ if (!open) return;
+ setKeyword(rule.keyword);
+ setSourceModel(rule.sourceModel || "");
+ setTargetModel(rule.targetModel);
+ setCaseSensitive(rule.caseSensitive);
+ setPriority(String(rule.priority));
+ setDescription(rule.description || "");
+ }, [open, rule]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/`[locale]/settings/keyword-routing/_components/edit-rule-dialog.tsx
around lines 39 - 48, The effect that fills the form (useEffect) only depends on
rule, so when the edit dialog is closed and reopened without rule changing the
local form state (keyword, sourceModel, targetModel, caseSensitive, priority,
description) can retain stale values; update the useEffect to also depend on the
dialog visibility prop (e.g., open or isOpen) and run when the dialog opens so
that on open you call setKeyword, setSourceModel, setTargetModel,
setCaseSensitive, setPriority, setDescription using rule (or sensible defaults
when rule is undefined) to guarantee a fresh form each time the dialog is
opened.
Comment on lines
+66
to
+73
| const parsedPriority = Number.parseInt(priority, 10); | ||
| const result = await updateKeywordRoutingRuleAction(rule.id, { | ||
| keyword: keyword.trim(), | ||
| sourceModel: sourceModel.trim() || null, | ||
| targetModel: targetModel.trim(), | ||
| caseSensitive, | ||
| priority: Number.isNaN(parsedPriority) ? 0 : parsedPriority, | ||
| description: description.trim() || null, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 验证点：是否存在 parseInt + NaN->0 的静默回退模式
rg -n 'parseInt\\(|Number\\.isNaN\\(parsedPriority\\)\\s*\\?\\s*0' src/app/[locale]/settings/keyword-routing/_components/edit-rule-dialog.tsx
rg -n '"priorityInteger"|"priorityRange"' messages/*/settings/keywordRouting.jsonRepository: ding113/claude-code-hub
Length of output: 1190
优先级解析会静默截断/回退 0，建议严格整数校验并使用 i18n 提示
- 当前代码用
Number.parseInt(priority, 10)会截断小数（如1.9→1），且Number.isNaN(parsedPriority) ? 0 : parsedPriority对非法输入静默回退为0，可能导致路由匹配优先级/规则顺序异常。 messages/*/settings/keywordRouting.json已包含keywordRouting.validation.priorityInteger/keywordRouting.validation.priorityRange，建议在校验失败时 toast.error + return，避免写入错误优先级。
建议修复（示例）
- const parsedPriority = Number.parseInt(priority, 10);
+ const parsedPriority = Number(priority);
+ if (!Number.isInteger(parsedPriority)) {
+ toast.error(t("keywordRouting.validation.priorityInteger"));
+ return;
+ }
const result = await updateKeywordRoutingRuleAction(rule.id, {
keyword: keyword.trim(),
sourceModel: sourceModel.trim() || null,
targetModel: targetModel.trim(),
caseSensitive,
- priority: Number.isNaN(parsedPriority) ? 0 : parsedPriority,
+ priority: parsedPriority,
description: description.trim() || null,
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/`[locale]/settings/keyword-routing/_components/edit-rule-dialog.tsx
around lines 66 - 73, The priority parsing silently truncates decimals and falls
back to 0; change the validation in edit-rule-dialog (where parsedPriority is
computed before calling updateKeywordRoutingRuleAction) to enforce a strict
integer within the allowed range: parse with Number.parseInt and then check that
String(parsedPriority) === String(priority).replace(/^\s+|\s+$/g,'') (or use a
/^\-?\d+$/ test) and that parsedPriority falls inside the allowed range from
messages (use the i18n keys keywordRouting.validation.priorityInteger and
keywordRouting.validation.priorityRange for error strings); on validation
failure call toast.error(...) with the appropriate i18n message and return early
so updateKeywordRoutingRuleAction is not invoked. Ensure you still trim other
fields as before and only call updateKeywordRoutingRuleAction when the priority
passes these checks.
Comment on lines
+19
to
+32
| function handleToggle(checked: boolean) { | ||
| startTransition(async () => { | ||
| const result = await saveSystemSettings({ | ||
| enableKeywordModelRouting: checked, | ||
| }); | ||
|
|
||
| if (result.ok) { | ||
| setIsEnabled(checked); | ||
| toast.success(checked ? t("enabled") : t("disabled")); | ||
| } else { | ||
| toast.error(result.error || t("saveFailed")); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 描述: 检查 saveSystemSettings 实现，确认其返回结构
# 查找 saveSystemSettings 定义
rg -nP -A 20 'export\s+(async\s+)?function\s+saveSystemSettings' --type ts
# 查找相关类型定义
rg -nP 'interface.*Result|type.*Result' src/lib/api-client/v1/actions/system-config.ts -A 5Repository: ding113/claude-code-hub
Length of output: 2412
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) 读取 master-toggle.tsx 相关片段
FILE="src/app/[locale]/settings/keyword-routing/_components/master-toggle.tsx"
if [ -f "$FILE" ]; then
echo "==== $FILE ===="
sed -n '1,120p' "$FILE" | nl -ba | sed -n '1,120p'
else
echo "Missing file: $FILE"
fi
# 2) 定位 toActionResult 与其返回类型
echo "==== toActionResult definition ===="
rg -n "toActionResult" -S src --type ts --type tsx
# 3) 读取 system-config 里的 saveSystemSettings 实现附近 + 返回结果结构
echo "==== src/lib/api-client/v1/actions/system-config.ts ===="
sed -n '1,120p' src/lib/api-client/v1/actions/system-config.ts | nl -ba
# 4) 读取 toActionResult 具体实现（缩小范围到定义处的文件/行附近）
# 从上一步 rg 输出中取第一个 toActionResult 定义文件（或直接用 rg 再抓）
echo "==== toActionResult implementation (first matches) ===="
rg -n "function toActionResult|export function toActionResult" src --type ts -S | head -n 20Repository: ding113/claude-code-hub
Length of output: 210
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==== locate master-toggle.tsx ===="
git ls-files | rg -n "master-toggle\.tsx$" || true
fd -a "master-toggle\.tsx" . || true
FILE="$(fd -a "master-toggle\.tsx" . | head -n 1 || true)"
if [ -z "${FILE:-}" ]; then
echo "master-toggle.tsx not found"
exit 0
fi
echo "==== read $FILE (first 200 lines with line numbers) ===="
cat -n "$FILE" | sed -n '1,220p'
echo "==== locate toActionResult definition ===="
rg -n "toActionResult" src --type ts --type tsx | head -n 50
# Try to find ActionResult / result shape types
echo "==== locate ActionResult types ===="
rg -n "ActionResult|toActionResult<|ok:|errorCode|error\s*:" src --type ts --type tsx | head -n 80
echo "==== read api-client system-config.ts (first 200 lines) ===="
sed -n '1,220p' src/lib/api-client/v1/actions/system-config.ts | cat -n
echo "==== read src/actions/system-config.ts (around saveSystemSettings) ===="
rg -n "export async function saveSystemSettings" src/actions/system-config.ts || true
sed -n '1,140p' src/actions/system-config.ts | cat -nRepository: ding113/claude-code-hub
Length of output: 2214
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# locate toActionResult implementation and result types
echo "==== find toActionResult usages/definition ===="
rg -n "toActionResult" src -S -g "*.{ts,tsx}" | head -n 50
echo "==== find ActionResult / ok / errorCode shape types ===="
rg -n "type\s+ActionResult|interface\s+ActionResult|ActionResult<|errorCode|result\.error|result\.ok|ok:\s*boolean|error:" src -S -g "*.{ts,tsx}" | head -n 120
echo "==== read candidate toActionResult file(s) ===="
TOREAD="$(rg -n "function\s+toActionResult|export\s+function\s+toActionResult" src -S -g "*.{ts,tsx}" | head -n 5 | cut -d: -f1 | sort -u)"
if [ -z "${TOREAD:-}" ]; then
echo "No toActionResult implementation found in first pass."
else
for f in $TOREAD; do
echo "---- $f (first 220 lines) ----"
sed -n '1,220p' "$f" | cat -n
done
fi
echo "==== read src/lib/api-client/v1/actions/system-config.ts (around saveSystemSettings) ===="
sed -n '1,120p' src/lib/api-client/v1/actions/system-config.ts | cat -n
rg -n "saveSystemSettings" src/lib/api-client/v1/actions/system-config.ts -S -g "*.{ts,tsx}"Repository: ding113/claude-code-hub
Length of output: 26996
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) 读取 ApiError / getApiErrorMessageKey 实现
rg -n "function\s+getApiErrorMessageKey|export\s+function\s+getApiErrorMessageKey" src/lib/api-client/v1/errors* -S --type ts
FILES="$(rg -n "getApiErrorMessageKey" src/lib/api-client/v1/errors* -S --type ts | cut -d: -f1 | sort -u)"
for f in $FILES; do
echo "==== read $f (first 220 lines) ===="
sed -n '1,220p' "$f" | cat -n
done
# 2) 读取 ActionResult 类型，确认字段名
rg -n "export\s+type\s+ActionResult|export\s+interface\s+ActionResult|ActionResult<" src/lib/api-client/v1/actions -S --type ts
TYPES="$(rg -n "export\s+(type|interface)\s+ActionResult" src/lib/api-client/v1/actions -S --type ts | cut -d: -f1 | sort -u)"
for f in $TYPES; do
echo "==== read $f (first 220 lines) ===="
sed -n '1,220p' "$f" | cat -n
done
# 3) 查找翻译键：saveFailed、enabled/disabled，以及 validation.* 在哪个 namespace
rg -n "saveFailed|enabled\"|disabled\"|validation\.permissionDenied|validation\.updateFailed" messages -S --type json || true
rg -n "\"settings\.keywordRouting\.masterToggle\"|keywordRouting\.masterToggle" messages -S --type json || true
# 4) 读取相关 messages/ru 文件（只读匹配到的文件）
RU_FILES="$(rg -n "keywordRouting" messages/ru -S --type json | cut -d: -f1 | sort -u | head -n 20)"
for f in $RU_FILES; do
echo "==== read $f (first 220 lines) ===="
sed -n '1,220p' "$f" | cat -n
doneRepository: ding113/claude-code-hub
Length of output: 235
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==== getApiErrorMessageKey implementation ===="
RG_OUT="$(rg -n "export function getApiErrorMessageKey" src/lib/api-client/v1 -S --type ts)"
echo "$RG_OUT"
FILE="$(echo "$RG_OUT" | head -n 1 | cut -d: -f1)"
sed -n '1,220p' "$FILE" | cat -n
echo "==== ActionResult type definition ===="
RG_TYPES="$(rg -n "export (type|interface) ActionResult" src/lib/api-client/v1/actions -S --type ts)"
echo "$RG_TYPES"
# read all candidate files that mention ActionResult
CAND_FILES="$(echo "$RG_TYPES" | cut -d: -f1 | sort -u)"
for f in $CAND_FILES; do
echo "---- $f (first 220 lines) ----"
sed -n '1,220p' "$f" | cat -n
done
echo "==== messages for keywordRouting masterToggle namespace and validation keys ===="
# namespace used by component:
# useTranslations("settings.keywordRouting.masterToggle")
rg -n "\"settings\\.keywordRouting\\.masterToggle\"|keywordRouting\\.masterToggle" messages -S --type json || true
# keywordRouting related files
KR_FILES="$(rg -n "keywordRouting" messages -S --type json | cut -d: -f1 | sort -u)"
echo "$KR_FILES" | head -n 50
# read likely ru file(s) that contain keywordRouting
RU_KR_FILES="$(rg -n "keywordRouting" messages/ru -S --type json | cut -d: -f1 | sort -u)"
for f in $(echo "$RU_KR_FILES" | head -n 10); do
echo "---- $f (first 260 lines) ----"
sed -n '1,260p' "$f" | cat -n
done
# read validation.* keys if they exist in the same ru file(s)
for f in $(echo "$RU_KR_FILES" | head -n 10); do
if rg -n "\"validation\." "$f" -S --type json >/dev/null 2>&1; then
echo "---- $f (validation.* matches) ----"
rg -n "\"validation\." "$f" -S --type json | head -n 80
fi
doneRepository: ding113/claude-code-hub
Length of output: 19665
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==== find masterToggle namespace keys ===="
rg -n "settings\.keywordRouting\.masterToggle" messages -S --type json || true
rg -n "\"enabled\"|\"disabled\"|\"saveFailed\"" messages -S --type json | head -n 80 || true
echo "==== locate getErrorMessage (or similar) that maps errorCode to i18n ===="
rg -n "getErrorMessage|errorCodeTo|get.*Message.*Code|translate.*errorCode" src -S --type ts --type tsx | head -n 80
# Search for common helper names in api-client/v1/errors
echo "==== read errors.ts for message mapping helpers ===="
sed -n '1,260p' src/lib/api-client/v1/errors.ts | cat -n
echo "==== find getErrorMessage usages ===="
rg -n "getErrorMessage\(" src --type ts --type tsx | head -n 80
echo "==== locate translation keys for PERMISSION_DENIED etc ===="
# Find where these tokens appear in code or messages. Use message files.
rg -n "\"PERMISSION_DENIED\"|\"NOT_FOUND\"|\"OPERATION_FAILED\"|\"INVALID_FORMAT\"|\"VALIDATION_ERROR\"" messages -S --type json || trueRepository: ding113/claude-code-hub
Length of output: 5671
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==== locate getErrorMessage / errorCode -> i18n mapping helpers ===="
rg -n "getErrorMessage|errorCodeTo|map.*errorCode|ErrorMessage" src -S --type ts || true
rg -n "getErrorMessage|errorCodeTo|map.*errorCode|ErrorMessage" src -S --type tsx || true
echo "==== read messages/ru/settings/keywordRouting.json ===="
FILE="messages/ru/settings/keywordRouting.json"
if [ -f "$FILE" ]; then
wc -l "$FILE"
sed -n '1,220p' "$FILE" | cat -n
else
echo "Missing $FILE"
fi
echo "==== search validation.* keys in keywordRouting ru file ===="
if [ -f "messages/ru/settings/keywordRouting.json" ]; then
rg -n "\"validation\.\"" messages/ru/settings/keywordRouting.json || true
rg -n "\"validation\":" messages/ru/settings/keywordRouting.json || true
rg -n "validation" messages/ru/settings/keywordRouting.json || true
fi
echo "==== search how other components render ActionResult errors ===="
rg -n "toast\.error\(([^)]*result\.errorCode|result\.errorCode|result\.error\b)" src -S --type ts --type tsx || true
rg -n "result\.error\s*\|\|\s*t\(" src -S --type ts --type tsx | head -n 50 || trueRepository: ding113/claude-code-hub
Length of output: 30655
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==== keyword-routing components using getErrorMessage / errorCode ===="
rg -n "keyword-routing" src/app/[locale]/settings/keyword-routing -S --type ts --type tsx || true
rg -n "getErrorMessage\(|errorCode" src/app/[locale]/settings/keyword-routing -S --type ts --type tsx || true
echo "==== read a representative keyword-routing file that handles save errors ===="
# list keyword-routing files
fd -e ts -e tsx -e js -e jsx "src/app/[locale]/settings/keyword-routing" . >/dev/null 2>&1 || true
ls -R src/app/[locale]/settings/keyword-routing 2>/dev/null | head -n 200 || trueRepository: ding113/claude-code-hub
Length of output: 543
🏁 Script executed:
#!/bin/bash
set -euo pipefail
DIR="src/app/[locale]/settings/keyword-routing/_components"
echo "==== search toast.error usage in keyword-routing ===="
rg -n "toast\.error\(|toast\.success\(" "$DIR" -S
echo "==== search errorCode usage in keyword-routing ===="
rg -n "errorCode|getErrorMessage" "$DIR" -S
echo "==== read each component (first 220 lines) ===="
for f in $(ls "$DIR" 2>/dev/null); do
echo "---- $DIR/$f (first 240 lines) ----"
sed -n '1,240p' "$DIR/$f" | cat -n
doneRepository: ding113/claude-code-hub
Length of output: 2693
🏁 Script executed:
#!/bin/bash
set -euo pipefail
DIR="src/app/[locale]/settings/keyword-routing/_components"
echo "==== search t(\"validation\"...) in keyword-routing ===="
rg -n "t\\(\"validation\\.|validation\\." "$DIR" -S --type ts --type tsx || true
echo "==== search getErrorMessage usage in keyword-routing ===="
rg -n "getErrorMessage\\(" "$DIR" -S --type ts --type tsx || trueRepository: ding113/claude-code-hub
Length of output: 254
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/lib/utils/error-messages.ts"
if [ -f "$FILE" ]; then
wc -l "$FILE"
sed -n '1,260p' "$FILE" | cat -n
rg -n "function\s+getErrorMessage|export function getErrorMessage|getErrorMessage\\(" "$FILE"
rg -n "PERMISSION_DENIED|NOT_FOUND|OPERATION_FAILED|INVALID_FORMAT|INVALID_FORMAT|VALIDATION_ERROR|NETWORK_ERROR" "$FILE" -S || true
# show relevant mapping tables near those tokens
rg -n "const\s+ERROR_CODES|ERROR_CODES|MESSAGES|map|ERROR_MESSAGE" "$FILE" -S || true
else
echo "Missing $FILE"
fi
# also inspect where it is used with tErrors, e.g. lib/hooks/use-api-mutation.ts
FILE2="src/lib/hooks/use-api-mutation.ts"
if [ -f "$FILE2" ]; then
sed -n '1,120p' "$FILE2" | cat -n
fiRepository: ding113/claude-code-hub
Length of output: 13421
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "\"PERMISSION_DENIED\"|\"NOT_FOUND\"|\"OPERATION_FAILED\"|\"INVALID_FORMAT\"|\"INVALID_TOKEN\"|\"NETWORK_ERROR\"|\"INTERNAL_ERROR\"" messages -S --type json | head -n 200
# also check if there is errors.json namespace per locale
ls messages | rg "^en|^zh-CN|^zh-TW|^ja|^ru" | head -n 20 || true
for loc in en zh-CN zh-TW ja ru; do
f="messages/$loc/errors.json"
if [ -f "$f" ]; then
echo "==== $f (matches) ===="
rg -n "\"PERMISSION_DENIED\"|\"NOT_FOUND\"|\"OPERATION_FAILED\"|\"INVALID_FORMAT\"" "$f" -S --type json || true
fi
doneRepository: ding113/claude-code-hub
Length of output: 3452
master-toggle.tsx 的错误 toast 直接展示 result.error，未按 errorCode 走 i18n 翻译
saveSystemSettings 失败时通过 toActionResult 返回 { ok: false, error: error.message, errorCode }。当前 master-toggle.tsx 在第 29 行使用 toast.error(result.error || t("saveFailed"))，把服务端的 error.message 直接作为用户可见文案展示，可能不满足“用户可见字符串必须 i18n”的要求；应优先使用 result.errorCode 走统一翻译。
建议改为按 errorCode 翻译（例如使用 useTranslations("errors") + getErrorMessage），并在缺失时回退到 t("saveFailed")：
import { useTranslations } from "next-intl";
import { getErrorMessage } from "`@/lib/utils/error-messages`";
const tErrors = useTranslations("errors");
toast.error(result.errorCode ? getErrorMessage(tErrors, result.errorCode) : t("saveFailed"));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/`[locale]/settings/keyword-routing/_components/master-toggle.tsx
around lines 19 - 32, The toast currently displays raw result.error in
handleToggle; instead use result.errorCode to produce an i18n message: import
and initialize tErrors = useTranslations("errors") and call
getErrorMessage(tErrors, result.errorCode) when result.errorCode exists, falling
back to t("saveFailed") otherwise, and pass that string to toast.error; keep the
existing setIsEnabled/toast.success behavior for the success path and leave
saveSystemSettings as-is.
Source: Coding guidelines
Comment on lines
+35
to
+52
| const problemResponses = { | ||
| 400: { | ||
| description: "Invalid request.", | ||
| content: { "application/problem+json": { schema: ProblemJsonSchema } }, | ||
| }, | ||
| 401: { | ||
| description: "Authentication required.", | ||
| content: { "application/problem+json": { schema: ProblemJsonSchema } }, | ||
| }, | ||
| 403: { | ||
| description: "Admin access required.", | ||
| content: { "application/problem+json": { schema: ProblemJsonSchema } }, | ||
| }, | ||
| 404: { | ||
| description: "Keyword routing rule not found.", | ||
| content: { "application/problem+json": { schema: ProblemJsonSchema } }, | ||
| }, | ||
| } as const; |
There was a problem hiding this comment.
OpenAPI 错误响应缺少 500，和处理器行为不一致。
这里的 problemResponses 只声明了 400/401/403/404，但处理器会在 OPERATION_FAILED 场景返回 500。文档与真实响应不一致，会导致客户端类型和集成预期偏差。
建议修改
const problemResponses = {
@@
description: "Keyword routing rule not found.",
content: { "application/problem+json": { schema: ProblemJsonSchema } },
},
+ 500: {
+ description: "Server operation failed.",
+ content: { "application/problem+json": { schema: ProblemJsonSchema } },
+ },
} as const;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const problemResponses = { | |
| 400: { | |
| description: "Invalid request.", | |
| content: { "application/problem+json": { schema: ProblemJsonSchema } }, | |
| }, | |
| 401: { | |
| description: "Authentication required.", | |
| content: { "application/problem+json": { schema: ProblemJsonSchema } }, | |
| }, | |
| 403: { | |
| description: "Admin access required.", | |
| content: { "application/problem+json": { schema: ProblemJsonSchema } }, | |
| }, | |
| 404: { | |
| description: "Keyword routing rule not found.", | |
| content: { "application/problem+json": { schema: ProblemJsonSchema } }, | |
| }, | |
| } as const; | |
| const problemResponses = { | |
| 400: { | |
| description: "Invalid request.", | |
| content: { "application/problem+json": { schema: ProblemJsonSchema } }, | |
| }, | |
| 401: { | |
| description: "Authentication required.", | |
| content: { "application/problem+json": { schema: ProblemJsonSchema } }, | |
| }, | |
| 403: { | |
| description: "Admin access required.", | |
| content: { "application/problem+json": { schema: ProblemJsonSchema } }, | |
| }, | |
| 404: { | |
| description: "Keyword routing rule not found.", | |
| content: { "application/problem+json": { schema: ProblemJsonSchema } }, | |
| }, | |
| 500: { | |
| description: "Server operation failed.", | |
| content: { "application/problem+json": { schema: ProblemJsonSchema } }, | |
| }, | |
| } as const; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/api/v1/resources/keyword-routing/router.ts` around lines 35 - 52, The
OpenAPI problemResponses constant is missing a 500 entry (handler can return
OPERATION_FAILED -> 500); update the problemResponses object (symbol:
problemResponses) to include a 500 response with a suitable description (e.g.,
"Internal server error.") and the same content/schema as the other entries
(application/problem+json using ProblemJsonSchema) so the spec matches the
OPERATION_FAILED behavior.
Comment on lines
+34
to
+79
| constructor() { | ||
| this.setupEventListener(); | ||
| } | ||
|
|
||
| private async setupEventListener(): Promise<void> { | ||
| if (typeof process !== "undefined" && process.env.NEXT_RUNTIME !== "edge") { | ||
| try { | ||
| const { eventEmitter } = await import("@/lib/event-emitter"); | ||
| const handler = () => { | ||
| logger.info("[KeywordRoutingRuleCache] Received update event, reloading..."); | ||
| void this.reload(); | ||
| }; | ||
| eventEmitter.on("keywordRoutingRulesUpdated", handler); | ||
| logger.info("[KeywordRoutingRuleCache] Subscribed to local eventEmitter"); | ||
|
|
||
| this.eventEmitterCleanup = () => { | ||
| eventEmitter.off("keywordRoutingRulesUpdated", handler); | ||
| }; | ||
|
|
||
| try { | ||
| const { CHANNEL_KEYWORD_ROUTING_RULES_UPDATED, subscribeCacheInvalidation } = | ||
| await import("@/lib/redis/pubsub"); | ||
| const cleanup = await subscribeCacheInvalidation( | ||
| CHANNEL_KEYWORD_ROUTING_RULES_UPDATED, | ||
| handler | ||
| ); | ||
| if (cleanup) { | ||
| this.redisPubSubCleanup = cleanup; | ||
| logger.info("[KeywordRoutingRuleCache] Subscribed to Redis pub/sub channel"); | ||
| } | ||
| } catch (error) { | ||
| logger.warn("[KeywordRoutingRuleCache] Failed to subscribe to Redis pub/sub", { error }); | ||
| } | ||
| } catch (error) { | ||
| logger.warn("[KeywordRoutingRuleCache] Failed to setup event listener", { error }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| destroy(): void { | ||
| this.eventEmitterCleanup?.(); | ||
| this.eventEmitterCleanup = null; | ||
|
|
||
| this.redisPubSubCleanup?.(); | ||
| this.redisPubSubCleanup = null; | ||
| } |
There was a problem hiding this comment.
异步订阅初始化和 destroy() 之间有竞态。
Line 35 直接启动异步初始化，但 Line 73-L79 的 destroy() 只能清理“已经注册完成”的句柄。若实例在动态 import() 或 Redis subscribeCacheInvalidation() 返回前就被销毁，后续仍会把监听器挂上，导致重复 reload 和订阅泄漏。
可参考的修复方向
class KeywordRoutingRuleCache {
private rules: KeywordRoutingRule[] = [];
private lastReloadTime = 0;
private isLoading = false;
private activeReloadPromise: Promise<void> | null = null; // 合并并发 reload
private reloadRequestedWhileLoading = false; // reload 期间收到的补跑请求
private eventEmitterCleanup: (() => void) | null = null;
private redisPubSubCleanup: (() => void) | null = null;
+ private destroyed = false;
constructor() {
- this.setupEventListener();
+ void this.setupEventListener();
}
private async setupEventListener(): Promise<void> {
if (typeof process !== "undefined" && process.env.NEXT_RUNTIME !== "edge") {
try {
const { eventEmitter } = await import("`@/lib/event-emitter`");
+ if (this.destroyed) {
+ return;
+ }
const handler = () => {
logger.info("[KeywordRoutingRuleCache] Received update event, reloading...");
void this.reload();
};
eventEmitter.on("keywordRoutingRulesUpdated", handler);
@@
try {
const { CHANNEL_KEYWORD_ROUTING_RULES_UPDATED, subscribeCacheInvalidation } =
await import("`@/lib/redis/pubsub`");
const cleanup = await subscribeCacheInvalidation(
CHANNEL_KEYWORD_ROUTING_RULES_UPDATED,
handler
);
+ if (this.destroyed) {
+ eventEmitter.off("keywordRoutingRulesUpdated", handler);
+ cleanup?.();
+ return;
+ }
if (cleanup) {
this.redisPubSubCleanup = cleanup;
logger.info("[KeywordRoutingRuleCache] Subscribed to Redis pub/sub channel");
}
} catch (error) {
@@
}
destroy(): void {
+ this.destroyed = true;
this.eventEmitterCleanup?.();
this.eventEmitterCleanup = null;
this.redisPubSubCleanup?.();
this.redisPubSubCleanup = null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/keyword-routing/engine.ts` around lines 34 - 79, The async
setupEventListener has a race with destroy: add a boolean flag (e.g.,
this.isDestroyed) initialized false, set it true at the start of destroy(), and
use it inside setupEventListener to avoid registering handlers after
destruction; specifically, before calling eventEmitter.on(...) and before
storing this.eventEmitterCleanup or this.redisPubSubCleanup (and immediately
call/off any returned cleanup if isDestroyed became true after the async
import/subscribe), and ensure the handler variable remains the same reference so
any immediate cleanup can off() it; update constructor/ setupEventListener/
destroy to use this.isDestroyed and perform the immediate cleanup-if-destroyed
logic rather than only storing cleanup callbacks.
Comment on lines
+220
to
+264
| export function extractKeywordRoutingTexts( | ||
| message: Record<string, unknown> | ||
| ): KeywordRoutingScanTexts { | ||
| const systemTexts: string[] = []; | ||
| const lastUserTexts: string[] = []; | ||
|
|
||
| // 1. 提取 system 字段（Claude 格式，string 或 content block 数组） | ||
| if ("system" in message) { | ||
| systemTexts.push(...extractSystemText(message.system)); | ||
| } | ||
|
|
||
| // 2. 提取 instructions 字段（Codex / Response API 格式） | ||
| if (typeof message.instructions === "string") { | ||
| systemTexts.push(message.instructions); | ||
| } | ||
|
|
||
| // 3. 提取 messages 数组（Claude / OpenAI Chat 格式） | ||
| if (Array.isArray(message.messages)) { | ||
| collectRoleScanTexts(message.messages, systemTexts, lastUserTexts); | ||
| } | ||
|
|
||
| // 4. 提取 input 字段（Codex / Response API 格式，数组或纯字符串） | ||
| if (Array.isArray(message.input)) { | ||
| collectRoleScanTexts(message.input, systemTexts, lastUserTexts); | ||
| } else if (typeof message.input === "string") { | ||
| // input 为纯字符串时属于用户输入，进入 lastUserTexts | ||
| lastUserTexts.push(message.input); | ||
| } | ||
|
|
||
| // 5. 提取图片接口等顶层 prompt 字段（string 或 string 数组） | ||
| if (typeof message.prompt === "string") { | ||
| lastUserTexts.push(message.prompt); | ||
| } else if (Array.isArray(message.prompt)) { | ||
| for (const item of message.prompt) { | ||
| if (typeof item === "string") { | ||
| lastUserTexts.push(item); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // 过滤空字符串 | ||
| return { | ||
| systemTexts: systemTexts.filter((t) => t.length > 0), | ||
| lastUserTexts: lastUserTexts.filter((t) => t.length > 0), | ||
| }; |
There was a problem hiding this comment.
Gemini 请求在这里会被完全漏扫。
这段提取逻辑只处理 system、instructions、messages、input 和 prompt。message.systemInstruction / message.contents 没有进入 systemTexts 或 lastUserTexts，所以 Gemini 格式请求上的关键词路由会静默失效。至少需要补上这两个字段的提取，或者在入口处明确排除 Gemini 并记录可观测告警。
依据提供的堆栈上下文，关键词路由是接入代理入口的通用能力，而不是只覆盖单一消息格式。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/message-extractor.ts` around lines 220 - 264, The
extractKeywordRoutingTexts function is missing Gemini fields: add handling so
message.systemInstruction (treat as system text) and message.contents (treat
like messages) are scanned. Concretely: in extractKeywordRoutingTexts, if typeof
message.systemInstruction === "string" push it into systemTexts (or pass through
extractSystemText if appropriate); if Array.isArray(message.contents) call
collectRoleScanTexts(message.contents, systemTexts, lastUserTexts); else if
typeof message.contents === "string" push it into lastUserTexts. This ensures
Gemini's systemInstruction and contents are included in keyword routing scans.
Comment on lines
+13
to
+22
| vi.mock("next-intl/server", () => ({ | ||
| getTranslations: vi.fn(() => (key: string, params?: Record<string, unknown>) => { | ||
| if (params) { | ||
| return `${key}:${JSON.stringify(params)}`; | ||
| } | ||
| return key; | ||
| }), | ||
| getLocale: vi.fn(() => "en"), | ||
| getMessages: vi.fn(() => ({})), | ||
| })); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In next-intl v4, are getTranslations, getLocale, and getMessages from "next-intl/server" async APIs that should be awaited?
💡 Result:
Yes, in next-intl v4, getTranslations, getLocale, and getMessages from "next-intl/server" are async functions that must be awaited when used [1][2][3]. These functions are provided specifically for use in environments where React hooks cannot be called, such as async Server Components, Server Actions, Route Handlers, and metadata generation [1][4]. Because they are asynchronous, you must use the await keyword to resolve their return values before using them [1][2][5]. Example usage: import {getTranslations, getLocale, getMessages} from 'next-intl/server'; export default async function Page { const t = await getTranslations('Index'); const locale = await getLocale; const messages = await getMessages; return
{t('title')}
; }Citations:
- 1: https://next-intl.dev/docs/environments/server-client-components
- 2: https://v4.next-intl.dev/docs/usage/configuration
- 3: https://next-intl.dev/docs/usage/configuration
- 4: https://next-intl.dev/docs/environments/actions-metadata-route-handlers
- 5: https://next-intl.dev/docs/getting-started/app-router
建议把 next-intl/server 在 mock 中的返回签名保持为异步（async），避免掩盖未 await 的调用。next-intl v4 的 getTranslations/getLocale/getMessages 为 async API；当前 tests/setup.ts（13-22）将其同步返回，可能导致遗漏 await 的实现被测试“误通过”。将三者改为 vi.fn(async () => ...)，并同步排查项目中其他对该模块的同类 mock。
建议修改
vi.mock("next-intl/server", () => ({
- getTranslations: vi.fn(() => (key: string, params?: Record<string, unknown>) => {
+ getTranslations: vi.fn(async () => (key: string, params?: Record<string, unknown>) => {
if (params) {
return `${key}:${JSON.stringify(params)}`;
}
return key;
}),
- getLocale: vi.fn(() => "en"),
- getMessages: vi.fn(() => ({})),
+ getLocale: vi.fn(async () => "en"),
+ getMessages: vi.fn(async () => ({})),
}));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| vi.mock("next-intl/server", () => ({ | |
| getTranslations: vi.fn(() => (key: string, params?: Record<string, unknown>) => { | |
| if (params) { | |
| return `${key}:${JSON.stringify(params)}`; | |
| } | |
| return key; | |
| }), | |
| getLocale: vi.fn(() => "en"), | |
| getMessages: vi.fn(() => ({})), | |
| })); | |
| vi.mock("next-intl/server", () => ({ | |
| getTranslations: vi.fn(async () => (key: string, params?: Record<string, unknown>) => { | |
| if (params) { | |
| return `${key}:${JSON.stringify(params)}`; | |
| } | |
| return key; | |
| }), | |
| getLocale: vi.fn(async () => "en"), | |
| getMessages: vi.fn(async () => ({})), | |
| })); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/setup.ts` around lines 13 - 22, Update the test mock so the async
signatures of next-intl v4 are preserved: change getTranslations, getLocale and
getMessages to return via vi.fn(async () => ...); specifically, make
getTranslations a vi.fn(async () => async (key: string, params?: Record<string,
unknown>) => { ... }) so the translator function is async, and change getLocale
and getMessages to vi.fn(async () => "en") and vi.fn(async () => ({}))
respectively; then scan the repo for other mocks of next-intl and convert them
similarly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements Keyword Model Routing — a new proxy feature that inspects incoming request messages for user-defined keywords and transparently rewrites the target model before forwarding to the upstream provider. Includes database schema, rule engine, proxy guard pipeline integration, REST API with OpenAPI docs, settings UI with full i18n, and comprehensive test coverage.
Also resolves i18n compliance, error handling, and observability issues found during code review.
Problem
Users need the ability to automatically route requests to different models based on keywords found in the message content. For example, when a prompt contains "gpt-4", automatically route to the GPT-4 provider regardless of the originally requested model. This allows keyword-based model switching without client-side changes.
Solution
Architecture
The feature integrates into the existing proxy guard pipeline as a new guard step (
keywordRouting) positioned betweenrequestFilterandrateLimit:When the feature is enabled via the
enableKeywordModelRoutingsystem setting, the guard:Key Components
keyword-routing-guard.tsmatcher.tsengine.tsmessage-extractor.tskeyword-routing-rules.tskeyword-routing-constants.tsChanges
Database (Migration #106)
keyword_routing_rulestable with columns: id, keyword, source_model, target_model, case_sensitive, priority, description, is_enabled, created_at, updated_at(is_enabled, priority)andcreated_atenable_keyword_model_routingboolean column onsystem_settingsProxy Pipeline
ProxyKeywordRoutingGuardinserted intoCHAT_PIPELINEbetweenrequestFilterandrateLimitREST API (
/api/v1/keyword-routing)errorCodefields (PERMISSION_DENIED, NOT_FOUND, VALIDATION_ERROR, OPERATION_FAILED)Settings UI
/settings/keyword-routingwith:i18n (5 languages: en, zh-CN, zh-TW, ja, ru)
messages/*/settings/keywordRouting.json— 87 keys per language (UI labels, validation messages, descriptions)messages/*/auditLogs.json— 6 audit log keys per languagemessages/*/provider-chain.json— 11 provider chain keys per languagemessages/*/settings/nav.json— navigation entrygetTranslations()instead of hardcoded stringsServer Actions
keyword-routing.tsactions with full i18n error messagesTests (~35 describe/it blocks, ~1000+ test lines)
keyword-routing.authz.test.ts— 18 authorization test cases (401/403 scenarios)engine.test.ts— Rule engine unit tests (cache warmup, reload queuing, priority matching)matcher.test.ts— Keyword matcher tests (case sensitivity, regex, multi-match)keyword-routing-guard.test.ts— Proxy guard integration testskeyword-routing-events.test.ts— Repository event and cache invalidation testsmessage-extractor-keyword-routing.test.ts— Text extraction from various message formatskeyword-routing.test.ts— Action layer testsSupporting Changes
openapi-types.gen.ts— Auto-generated API client types for keyword routingsystem-config.ts— Repository and cache support for the new settingemit-event.ts— Event emission for keyword routing audit eventsprovider-chain-formatter.ts— Formatting support for keyword routing decisionsCode Review Fixes (from final commit)
Critical — i18n Compliance
getTranslations()callsHigh — Error Handling
errorCodefields to all error responsesMedium — Observability
Description enhanced by Claude AI
Greptile Summary
This PR introduces a complete keyword-based model routing subsystem — DB schema, cache engine, proxy guard, CRUD API, and i18n-compliant UI — on top of fixing prior i18n/error-handling issues. The core routing logic (engine, matcher, guard) is well-designed and closely mirrors the existing sensitive-word detector pattern.
keyword-routing-guard.ts) intercepts each non-Gemini, non-multipart request, scans system prompts and the last user message for matching keywords, and rewritessession.request.model+ the JSON buffer in-place before provider selection.engine.ts) maintains an in-memory sorted list of enabled rules, subscribes to both local EventEmitter and Redis pub/sub for cross-worker invalidation, and uses a queued-reload pattern to avoid lost updates.requireAuth("admin")middleware, with structurederrorCodefields driving HTTP status mapping in handlers.Confidence Score: 3/5
The proxy guard and cache engine are solid, but two issues in the action/handler layer should be addressed before merging: the
listKeywordRoutingRulesaction silently promotes DB errors to successful empty responses, and thegetKeywordRoutingCacheStatsaction maps both auth failures and exceptions to the same 403 response.The core routing logic (engine, matcher, guard) is well-designed and follows established patterns in the codebase. The two issues that lower the score are in the API surface:
listKeywordRoutingRulesreturns a raw array instead of anActionResult, so any DB error during listing returnsHTTP 200 { items: [] }with no indication of failure; andgetKeywordRoutingCacheStatsconflates infrastructure exceptions with access-denied by mapping both to a 403 error. Both could mislead operators diagnosing production issues.src/actions/keyword-routing.ts(list action pattern) andsrc/app/api/v1/resources/keyword-routing/handlers.ts(cache stats handler and fallback error strings) need the most attention.Important Files Changed
listKeywordRoutingRulescorrectly returnActionResult; the list action silently swallows DB errors and returns an empty array.getKeywordRoutingCacheStatshandler maps both auth failures and exceptions to 403; the fallbackgetActionErrorStatusstill contains hardcoded Chinese strings despite the PR's i18n goal.activeReloadPromiseis well-designed; subscribes to both local EventEmitter and Redis pub/sub for cross-worker invalidation.toLowerCaseon large texts. Correctly skips disabled rules and empty-keyword rules as defense against dirty data.sourceModelto null.requireAuth("admin")middleware on all routes.keyword_routing_rulestable andenable_keyword_model_routingcolumn tosystem_settings. UsesIF NOT EXISTSguards; indexes match schema definitions.Sequence Diagram
sequenceDiagram participant Client participant GuardPipeline participant KeywordRoutingGuard participant Engine as KeywordRoutingEngine participant ProviderSelector Client->>GuardPipeline: POST /v1/chat/completions (model: A) GuardPipeline->>KeywordRoutingGuard: ensure(session) KeywordRoutingGuard->>Engine: isEmpty()? alt cache empty → skip Engine-->>KeywordRoutingGuard: true KeywordRoutingGuard-->>GuardPipeline: null (passthrough) else rules loaded Engine-->>KeywordRoutingGuard: false KeywordRoutingGuard->>KeywordRoutingGuard: isKeywordModelRoutingEnabled()? alt master switch off KeywordRoutingGuard-->>GuardPipeline: null (passthrough) else switch on KeywordRoutingGuard->>Engine: match(texts, model-A) Engine-->>KeywordRoutingGuard: MatchResult rule → model-B KeywordRoutingGuard->>KeywordRoutingGuard: rewrite session.request.model KeywordRoutingGuard->>KeywordRoutingGuard: setKeywordRoutingAudit KeywordRoutingGuard-->>GuardPipeline: null (model rewritten) end end GuardPipeline->>ProviderSelector: select provider for model-B ProviderSelector-->>Client: responsePrompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(keyword-routing): resolve i18n, erro..." | Re-trigger Greptile