feat(local-ai): expose compact service gateway and request logs#210
feat(local-ai): expose compact service gateway and request logs#210awsl233777 wants to merge 11 commits into
Conversation
📝 WalkthroughWalkthroughTask-local OpenAI/Claude provider config is now stored and resolved per task, local bridge requests can fail over across upstreams, and the web UI adds a Local AI surface, Kilo webview host state, locale updates, and matching tests. ChangesTask-local provider and orchestration surface
Locale strings for Local AI, Kilo, and readiness copy
Provider, webview, and UI tests
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web-ui/modules/task-kilo-webview-host.mjs (1)
193-228: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
postMessagepre-normalizes with a default-only config, silently stripping the raw message for anything outside the hardcoded allow-list.
postMessage()callsnormalizeTaskKiloWebviewMessage(message)without passing the realbridgeConfig, so it only checks membership against the hardcodedKILO_ALLOWED_WEBVIEW_MESSAGE_TYPES/KILO_BLOCKED_WEBVIEW_MESSAGE_TYPES.normalizeTaskKiloWebviewMessageonly attachesmessage: rawwhenallowedis true (Line 190). For any type that is only allowed via the documentedallowedMessageTypesextension innormalizeTaskKiloBridgeConfig(Lines 35-57), this first pass returnsallowed:falseand drops the payload. Downstream,handleTaskKiloWebviewMessagere-normalizes using the realstate.webviewBridgeConfig(which could allow the type), but by then it only receivesdecision.message || decision— i.e. the decision object itself, not the original payload (text/content fields are gone).This defeats the purpose of the
allowedMessageTypesextension mechanism for any type not already hardcoded. It's not actively triggered by current in-app usage (the handler's own type dispatch is a closed set), but it's a latent correctness gap in a documented extension point.♻️ Proposed fix: always retain the raw message on the decision object
export function normalizeTaskKiloWebviewMessage(message, bridgeConfig = {}) { const raw = message && typeof message === 'object' ? message : {}; const type = String(raw.type || '').trim(); const config = normalizeTaskKiloBridgeConfig(bridgeConfig); const allowedSet = new Set(config.allowedMessageTypes); const blockedSet = new Set(config.blockedMessageTypes); if (!type) { return { allowed: false, blocked: false, type: '', reason: 'missing-type' }; } if (blockedSet.has(type)) { - return { allowed: false, blocked: true, type, reason: 'blocked-sensitive-host-capability' }; + return { allowed: false, blocked: true, type, reason: 'blocked-sensitive-host-capability', message: raw }; } if (!allowedSet.has(type)) { - return { allowed: false, blocked: false, type, reason: 'not-whitelisted' }; + return { allowed: false, blocked: false, type, reason: 'not-whitelisted', message: raw }; } return { allowed: true, blocked: false, type, reason: '', message: raw }; }Also applies to: 175-191
🤖 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 `@web-ui/modules/task-kilo-webview-host.mjs` around lines 193 - 228, `createTaskKiloVsCodeApiShim` is dropping the original payload because `postMessage()` normalizes with no bridge config before `handleTaskKiloWebviewMessage` can apply the real `state.webviewBridgeConfig`. Update `normalizeTaskKiloWebviewMessage` so it always preserves the raw `message` on the returned decision object, even when `allowed` is false, and keep `postMessage()` passing through that full decision so later re-normalization in `handleTaskKiloWebviewMessage` can still access the original fields.
🤖 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 `@web-ui/modules/app.methods.task-orchestration.mjs`:
- Around line 246-257: The background refresh in loadTaskOrchestrationOverview()
is overwriting in-progress edits in state.webviewBridgeDraft while the
device-settings panel is open. Update the sync path around
syncTaskWebviewBridgeDraft() and the state.providerSetupSelection reset so they
are skipped or merged when the settings panel is active, instead of blindly
replacing the current draft on each polling refresh.
---
Nitpick comments:
In `@web-ui/modules/task-kilo-webview-host.mjs`:
- Around line 193-228: `createTaskKiloVsCodeApiShim` is dropping the original
payload because `postMessage()` normalizes with no bridge config before
`handleTaskKiloWebviewMessage` can apply the real `state.webviewBridgeConfig`.
Update `normalizeTaskKiloWebviewMessage` so it always preserves the raw
`message` on the returned decision object, even when `allowed` is false, and
keep `postMessage()` passing through that full decision so later
re-normalization in `handleTaskKiloWebviewMessage` can still access the original
fields.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 215367cd-cfb5-4ee2-9733-a66c9e4d0a6a
📒 Files selected for processing (27)
cli.jstests/e2e/test-task-orchestration.jstests/unit/config-tabs-ui.test.mjstests/unit/i18n-locales.test.mjstests/unit/session-tab-switch-performance.test.mjstests/unit/task-codex-exec-windows.test.mjstests/unit/web-ui-behavior-parity.test.mjstests/unit/web-ui-logic.test.mjsweb-ui/app.jsweb-ui/index.htmlweb-ui/modules/app.computed.main-tabs.mjsweb-ui/modules/app.methods.navigation.mjsweb-ui/modules/app.methods.task-orchestration.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/modules/task-kilo-webview-host.mjsweb-ui/partials/index/layout-header.htmlweb-ui/partials/index/panel-local-ai.htmlweb-ui/partials/index/panel-orchestration.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/session-helpers.mjsweb-ui/styles.cssweb-ui/styles/local-ai-service.cssweb-ui/styles/task-orchestration.css
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.0)
tests/e2e/test-task-orchestration.js
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] 123-142: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(scriptPath, const http = require('http'); const fs = require('fs'); const portFile = process.argv[2]; const status = Number(process.argv[3] || 401); const message = process.argv[4] || 'Invalid token'; const server = http.createServer((req, res) => { req.resume(); req.on('end', () => { const body = JSON.stringify({ error: { message } }); res.writeHead(status, { 'Content-Type': 'application/json; charset=utf-8', 'Content-Length': Buffer.byteLength(body, 'utf-8') }); res.end(body, 'utf-8'); }); }); server.listen(0, '127.0.0.1', () => { fs.writeFileSync(portFile, String(server.address().port), 'utf-8'); }); process.on('SIGTERM', () => server.close(() => process.exit(0))); process.on('SIGINT', () => server.close(() => process.exit(0)));, 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 151-151: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(portFile, 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 186-186: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(configDir, 'codexmate-init.json'), JSON.stringify({ version: 1, mode: 'task-openai-chat-e2e' }), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] 229-229: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(configDir, 'codexmate-init.json'), JSON.stringify({ version: 1, mode: 'task-openai-chat-fallback-e2e' }), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 230-256: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(configDir, 'config.toml'), [
'model = "gpt-5.3-codex"',
'model_provider = "local"',
'',
'[model_providers.local]',
'name = "Local Codex"',
'base_url = "http://127.0.0.1:9/v1"',
'wire_api = "responses"',
'preferred_auth_method = "sk-codex-tab-secret"',
'models = ["gpt-5.3-codex"]',
'',
'[model_providers.bad-chat]',
'name = "Bad Chat"',
base_url = "${badBaseUrl}/v1",
'wire_api = "chat_completions"',
'preferred_auth_method = "sk-bad-token"',
'models = ["bad-model"]',
'',
'[model_providers.new-api-chat]',
'name = "New API Chat"',
base_url = "${fallbackBaseUrl}/v1",
'wire_api = "chat_completions"',
'preferred_auth_method = "sk-task-e2e-secret"',
'temperature = 0.7',
'models = ["glm-5.2"]',
''
].join('\n'), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 257-279: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(taskConfigDir, 'task-openai-chat-provider.json'), JSON.stringify({
selectedProvider: 'bad-chat',
selectedModels: { 'bad-chat': 'bad-model', 'new-api-chat': 'glm-5.2' },
providers: {
'bad-chat': {
name: 'Bad Chat',
base_url: ${badBaseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-bad-token',
models: ['bad-model'],
selected_model: 'bad-model'
},
'new-api-chat': {
name: 'New API Chat',
base_url: ${fallbackBaseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-task-e2e-secret',
temperature: 0.7,
models: ['glm-5.2'],
selected_model: 'glm-5.2'
}
}
}, null, 2), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
🪛 HTMLHint (1.9.2)
web-ui/partials/index/panel-local-ai.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🪛 Stylelint (17.14.0)
web-ui/styles.css
[error] 14-14: Expected "url('./styles/local-ai-service.css')" to be "'./styles/local-ai-service.css'" (import-notation)
(import-notation)
🔇 Additional comments (51)
cli.js (12)
252-252: LGTM!
12525-12542: LGTM!
16096-16097: LGTM!
16190-16208: LGTM!
16225-16308: LGTM!
16320-16400: LGTM!
16402-16467: LGTM!
16469-16574: LGTM!
16575-16628: LGTM!
16867-16893: LGTM!
17020-17036: LGTM!
16938-16988: 🩺 Stability & AvailabilityNo issue:
registerAbortreplaces the previous handler, so fallback attempts do not stack abort callbacks.> Likely an incorrect or invalid review comment.tests/e2e/test-task-orchestration.js (5)
121-180: LGTM!
182-223: LGTM!
225-281: LGTM!
485-502: LGTM!
824-852: LGTM!web-ui/modules/task-kilo-webview-host.mjs (2)
1-57: LGTM!
230-387: LGTM!web-ui/app.js (1)
479-488: LGTM!Also applies to: 562-562, 620-622
web-ui/index.html (1)
23-23: LGTM!web-ui/modules/app.methods.task-orchestration.mjs (2)
1-5: LGTM!Also applies to: 32-54, 332-510, 618-1131
356-375: 🎯 Functional Correctness
item.keymatches the backend shape here.localAiServiceStatus.providerOptionsincludeskey, andselectedProvideris resolved against that same field, so this prefill path is fine.> Likely an incorrect or invalid review comment.web-ui/modules/i18n/locales/en.mjs (1)
135-165: LGTM!Also applies to: 247-247, 997-1003, 1014-1064, 1080-1115, 1124-1126, 1149-1171, 1192-1200, 1228-1241, 1276-1298
web-ui/modules/i18n/locales/ja.mjs (2)
136-166: 📐 Maintainability & Code Quality | ⚡ Quick winNew Local AI Service / rebuilding strings left untranslated (still English).
Lines 136-166 add the new
tab.localAi,localAi.*,orchestration.rebuilding.*, andorchestration.openai.status.presentkeys, but the Japanese values are copy-pasted English text (e.g.'title.localAi': 'Local AI Service','localAi.body': 'Add Claude or OpenAI-compatible providers...') rather than Japanese translations. Every other new block added in this same file (readiness, kilo, toast, quick-entry copy) is properly translated to Japanese, so this looks like an isolated oversight for this one block.zh.mjs's equivalent block is correctly localized for comparison.
248-248: LGTM!Also applies to: 986-992, 1001-1063, 1086-1124, 1138-1160, 1181-1190, 1217-1230, 1265-1287
web-ui/modules/i18n/locales/vi.mjs (2)
96-126: 📐 Maintainability & Code Quality | ⚡ Quick winNew Local AI Service / rebuilding strings left untranslated (still English).
Lines 96-126 add the new
tab.localAi,localAi.*,orchestration.rebuilding.*, andorchestration.openai.status.presentkeys, but the Vietnamese values are the raw English text (e.g.'localAi.title': 'Local AI Service','localAi.body': 'Add Claude or OpenAI-compatible providers...') rather than Vietnamese translations. All other new blocks added in this file (readiness, kilo, toast/privacy, quick-entry copy) are properly translated to Vietnamese, so this appears to be an isolated oversight for this one block.zh.mjs's equivalent block is correctly localized for comparison.
262-262: LGTM!Also applies to: 333-348, 1132-1138, 1149-1209, 1232-1270, 1293-1301, 1329-1342, 1377-1399
web-ui/modules/i18n/locales/zh.mjs (1)
135-165: LGTM!Also applies to: 247-247, 996-1002, 1013-1063, 1079-1125, 1148-1170, 1191-1199, 1227-1240, 1275-1297
web-ui/modules/app.methods.navigation.mjs (1)
8-21: LGTM!web-ui/partials/index/layout-header.html (2)
327-339: 🎯 Functional Correctness | ⚡ Quick winHardcoded "providers" text bypasses i18n.
The provider count suffix is a literal English string instead of a translation key, so it won't localize for ja/vi/zh-tw/zh users even though this PR adds locale coverage for the Local AI tab.
🌐 Proposed fix
<div class="side-item-meta"> <span>{{ t('side.localAi.meta') }}</span> - <span>{{ taskOrchestration.localAiServiceStatus && taskOrchestration.localAiServiceStatus.providerOptions ? taskOrchestration.localAiServiceStatus.providerOptions.length : 0 }} providers</span> + <span>{{ t('side.localAi.providerCount', { count: taskOrchestration.localAiServiceStatus && taskOrchestration.localAiServiceStatus.providerOptions ? taskOrchestration.localAiServiceStatus.providerOptions.length : 0 }) }}</span> </div>
76-85: LGTM!web-ui/partials/index/panel-local-ai.html (1)
1-115: LGTM!web-ui/styles.css (1)
14-14: LGTM!web-ui/styles/local-ai-service.css (1)
1-183: LGTM!web-ui/partials/index/panel-orchestration.html (3)
9-38: LGTM!
41-76: LGTM!Also applies to: 82-131
105-110: 🩺 Stability & AvailabilityGuard the
.trim()calls if these fields can ever be unset. The current defaults are empty strings, so this only becomes a crash risk if a later code path assignsnullorundefined.web-ui/modules/app.computed.main-tabs.mjs (1)
508-517: 🎯 Functional CorrectnessNo sync issue here
taskOrchestrationProviderConfigRequired()matchestaskOpenAiChatConfigRequired(), so the header state stays aligned.> Likely an incorrect or invalid review comment.web-ui/session-helpers.mjs (1)
89-89: LGTM!Also applies to: 145-150
web-ui/modules/i18n/locales/zh-tw.mjs (1)
135-165: LGTM!Also applies to: 247-247, 996-1002, 1013-1063, 1079-1125, 1148-1170, 1191-1199, 1227-1240, 1275-1297
tests/unit/config-tabs-ui.test.mjs (3)
9-25: LGTM!
211-220: 🗄️ Data Integrity & IntegrationConfirm
localAiServiceStatusandopenAiChatStatusstay in sync.The Local AI panel reads provider state from
taskOrchestration.localAiServiceStatus.providerOptions(Line 218), while the task-detail/kilo-webview snapshot code paths (perweb-ui-logic.test.mjs) read from a separately-namedtaskOrchestration.openAiChatStatus. Since both surfaces manage the same underlying OpenAI/Claude provider store, please confirm these two status objects are populated from a single source of truth (e.g., both refreshed by the sametask-overviewresponse) rather than independently, to avoid the Local AI tab and task tab showing inconsistent provider readiness.
97-210: LGTM!Also applies to: 221-242
tests/unit/i18n-locales.test.mjs (1)
196-213: LGTM!Also applies to: 228-250
tests/unit/session-tab-switch-performance.test.mjs (1)
170-192: LGTM!tests/unit/task-codex-exec-windows.test.mjs (1)
139-192: LGTM!Also applies to: 194-240, 241-279, 280-311, 313-332, 364-380
tests/unit/web-ui-behavior-parity.test.mjs (2)
794-810: 🎯 Functional CorrectnessVerify
previewTaskPlan/previewTaskPlanFromChataren't double-counted across allowlists.
'previewTaskPlan' + 'FromChat'is allowlisted as an "extra" (newly-present) method key at Line 806, but'previewTaskPlan'and'previewTaskPlan' + 'FromChat'are also added to the "missing" (allowed-absent) allowlist at Lines 846-847. If both lists diff against the same baseline snapshot, a method name can't legitimately be both newly-added and missing at once — please confirm this isn't a stale/copy-pasted allowlist entry masking a genuine rename/removal that should only appear in one list.Also applies to: 845-847
821-821: LGTM!Also applies to: 923-924
tests/unit/web-ui-logic.test.mjs (2)
2185-2261: 🎯 Functional Correctness | ⚡ Quick winConfirm Local AI Service tab doesn't reuse task-chat-specific save wording.
saveLocalAiProvideris exercised here in the task-orchestration detail context and produces the toast'orchestration.openai.config.saveSuccess'("任务聊天 Provider 已保存" / "task chat Provider saved" — Line 2213/2261). The same method is also wired topanel-local-ai.html's form (@submit.prevent="saveLocalAiProvider", perconfig-tabs-ui.test.mjs), and the newlocalAi.*locale block added in this PR has no dedicated save-success/failure strings. If the toast text isn't context-aware, users saving a provider from the dedicated "Local AI Service" tab would see a confusing "task chat provider saved" message instead of copy referencing the Local AI Service they're actually configuring.
18-24: LGTM!Also applies to: 1140-1140, 1308-1345, 1392-1417, 1419-1467, 1477-1477, 1509-1510, 1546-1546, 1695-1697, 1784-1785, 1826-2051, 2082-2105, 2132-2132, 2368-2368
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web-ui/partials/index/layout-header.html (1)
326-336: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHardcoded "providers" string bypasses i18n.
Unlike the adjacent
t('side.localAi.meta'), the provider-count suffix is a raw literalproviders, so it won't be translated for ja/vi/zh-tw/zh locales even though this PR adds locale entries for Local AI Service copy elsewhere.🌐 Proposed fix
- <span>{{ taskOrchestration.localAiServiceStatus && taskOrchestration.localAiServiceStatus.providerOptions ? taskOrchestration.localAiServiceStatus.providerOptions.length : 0 }} providers</span> + <span>{{ t('side.localAi.providerCount', { count: taskOrchestration.localAiServiceStatus && taskOrchestration.localAiServiceStatus.providerOptions ? taskOrchestration.localAiServiceStatus.providerOptions.length : 0 }) }}</span>(requires adding a
side.localAi.providerCountkey with pluralization support to the locale files)🤖 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 `@web-ui/partials/index/layout-header.html` around lines 326 - 336, The Local AI provider-count label in layout-header.html is hardcoded as a literal string, so it bypasses i18n. Update the template around the local-ai tab to use a translation key instead of appending "providers" directly, and wire it to a new side.localAi.providerCount locale entry with pluralization support across the existing locale files. Keep the change localized to the local-ai tab markup and the related locale resources.
🤖 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 `@web-ui/modules/app.methods.navigation.mjs`:
- Around line 445-449: The navigation intent clearing logic is using the wrong
tab value after the orchestration redirect. In the `targetTab` flow inside the
navigation method, make sure the later `clearMainTabSwitchIntent` calls use
`targetTab` instead of `normalizedTab`, so the `orchestration` to `local-ai`
mapping is respected and the intent/classes are cleared correctly.
---
Outside diff comments:
In `@web-ui/partials/index/layout-header.html`:
- Around line 326-336: The Local AI provider-count label in layout-header.html
is hardcoded as a literal string, so it bypasses i18n. Update the template
around the local-ai tab to use a translation key instead of appending
"providers" directly, and wire it to a new side.localAi.providerCount locale
entry with pluralization support across the existing locale files. Keep the
change localized to the local-ai tab markup and the related locale resources.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 20f6ec67-2856-4ceb-a6e5-c4cf71c85f2e
📒 Files selected for processing (11)
tests/unit/config-tabs-ui.test.mjstests/unit/session-tab-switch-performance.test.mjstests/unit/web-ui-runtime-navigation-regression.test.mjsweb-ui/modules/app.methods.navigation.mjsweb-ui/partials/index/layout-header.htmlweb-ui/partials/index/panel-dashboard.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/session-helpers.mjsweb-ui/styles/layout-shell.cssweb-ui/styles/local-ai-service.cssweb-ui/styles/navigation-panels.css
✅ Files skipped from review due to trivial changes (1)
- web-ui/partials/index/panel-dashboard.html
🚧 Files skipped from review as they are similar to previous changes (3)
- web-ui/styles/local-ai-service.css
- tests/unit/config-tabs-ui.test.mjs
- web-ui/session-helpers.mjs
📜 Review details
🔇 Additional comments (8)
web-ui/modules/app.methods.navigation.mjs (1)
8-21: LGTM!web-ui/partials/index/layout-header.html (3)
66-74: LGTM!
76-84: LGTM!
316-325: LGTM!web-ui/styles/layout-shell.css (1)
377-389: LGTM!web-ui/styles/navigation-panels.css (1)
308-323: LGTM!tests/unit/session-tab-switch-performance.test.mjs (1)
170-214: LGTM!tests/unit/web-ui-runtime-navigation-regression.test.mjs (1)
340-358: LGTM!
awsl233777
changed the title
|
Implemented the Local AI Gateway direction for this round. What changed:
Validation:
Screenshot evidence was produced locally for:
CodeRabbit was not checked or triggered. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web-ui/partials/index/panel-local-ai.html (1)
22-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRepeated deep-chained null guards reduce readability.
Lines 29, 35, and 39 each repeat the same
taskOrchestration.localAiServiceStatus && ...gateway && ...gateway.Xchain with a hardcoded default. Extracting a computed property (e.g.localAiGateway) returning a normalized object with defaults would remove this duplication and reduce risk of the chains drifting out of sync as more gateway fields are added.Example refactor
-<code>{{ (taskOrchestration.localAiServiceStatus && taskOrchestration.localAiServiceStatus.gateway && taskOrchestration.localAiServiceStatus.gateway.chatCompletionsPath) || '/bridge/local/v1/chat/completions' }}</code> +<code>{{ localAiGateway.chatCompletionsPath }}</code>computed: { localAiGateway() { const gateway = this.taskOrchestration.localAiServiceStatus?.gateway || {}; return { chatCompletionsPath: gateway.chatCompletionsPath || '/bridge/local/v1/chat/completions', basePath: gateway.basePath || '/bridge/local/v1', modelsPath: gateway.modelsPath || '/bridge/local/v1/models' }; } }🤖 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 `@web-ui/partials/index/panel-local-ai.html` around lines 22 - 46, The local AI endpoint template repeats the same deep null-check chain for gateway paths in multiple bindings, which hurts readability and makes the defaults easy to drift. Refactor the panel rendering around a single computed property or helper such as localAiGateway that normalizes taskOrchestration.localAiServiceStatus.gateway with fallback values, then update the bindings for chatCompletionsPath, basePath, and modelsPath to read from that shared object.
🤖 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.
Nitpick comments:
In `@web-ui/partials/index/panel-local-ai.html`:
- Around line 22-46: The local AI endpoint template repeats the same deep
null-check chain for gateway paths in multiple bindings, which hurts readability
and makes the defaults easy to drift. Refactor the panel rendering around a
single computed property or helper such as localAiGateway that normalizes
taskOrchestration.localAiServiceStatus.gateway with fallback values, then update
the bindings for chatCompletionsPath, basePath, and modelsPath to read from that
shared object.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1c9c1b8d-e07a-4e70-9512-f88be72e7cb6
📒 Files selected for processing (12)
cli.jscli/local-bridge.jstests/e2e/test-task-orchestration.jstests/unit/config-tabs-ui.test.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/panel-local-ai.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/styles/local-ai-service.css
✅ Files skipped from review due to trivial changes (1)
- web-ui/modules/i18n/locales/zh-tw.mjs
🚧 Files skipped from review as they are similar to previous changes (7)
- web-ui/styles/local-ai-service.css
- web-ui/modules/i18n/locales/vi.mjs
- web-ui/modules/i18n/locales/zh.mjs
- cli.js
- web-ui/modules/i18n/locales/en.mjs
- tests/unit/config-tabs-ui.test.mjs
- web-ui/modules/i18n/locales/ja.mjs
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.0)
tests/e2e/test-task-orchestration.js
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
🔇 Additional comments (12)
web-ui/partials/index/panel-local-ai.html (4)
48-71: LGTM!
122-163: LGTM! Password field usestype="password"withautocomplete="new-password", and JS-side validation is already handled insaveLocalAiProvider.
164-165: LGTM!
10-20: 🎯 Functional CorrectnessNo fallback to
openAiChatStatusis needed here.task-overviewalways supplieslocalAiServiceStatus, and this panel is intentionally driven by local-provider state only, so the readiness badge and provider list should stay keyed totaskOrchestration.localAiServiceStatus.> Likely an incorrect or invalid review comment.cli/local-bridge.js (4)
106-109: LGTM!
195-208: LGTM!
157-161: 🎯 Functional CorrectnessRemove this warning:
preferred_auth_methodis the stored API key here, not an auth enum. The non-codexmate/openai-bridgeproviders in this repo write secrets likesk-*intopreferred_auth_method, and this branch correctly forwards that value asBearer <token>.> Likely an incorrect or invalid review comment.
745-836: 🩺 Stability & AvailabilityNo streaming regression here This passthrough already buffered via
proxyRequestJsonand returnedapplication/json, so it did not preserve SSE or upstream headers before this change.> Likely an incorrect or invalid review comment.tests/e2e/test-task-orchestration.js (4)
2-2: LGTM!
326-352: LGTM!
514-554: LGTM!
876-921: LGTM!
awsl233777
force-pushed
the
feat/local-ai-service-provider-console
branch
from
6e8ba30 to
63dcf74
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
web-ui/modules/app.methods.navigation.mjs (1)
445-449: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winNav intent still cleared with the wrong tab value.
The
orchestration→local-airemap setsstate.intent = 'local-ai'viasetMainTabSwitchIntent(targetTab)(line 461), but the intent is still cleared usingnormalizedTab(still'orchestration') at lines 492, 514, and 531. SinceclearMainTabSwitchIntentbails out whenexpectedTab !== state.intent(lines 411-413), the guard never clears when navigating away fromorchestration, leaving nav-intent DOM classes and pending state stuck.This is the same issue flagged in a prior review round and remains unaddressed.
🐛 Proposed fix
this.scheduleAfterFrame(() => { - this.clearMainTabSwitchIntent(normalizedTab); + this.clearMainTabSwitchIntent(targetTab); }); return; }Apply the same
targetTabsubstitution at the other twoclearMainTabSwitchIntent(normalizedTab)call sites (lines 514 and 531).🤖 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 `@web-ui/modules/app.methods.navigation.mjs` around lines 445 - 449, The navigation intent is still being cleared with the pre-remap tab value, so the orchestration-to-local-ai flow leaves state stuck. Update the remaining clearMainTabSwitchIntent call sites in the navigation method to use the same targetTab value that is passed to setMainTabSwitchIntent, matching the existing remap logic in the main tab switch path. Use the identifiers normalizedTab, targetTab, setMainTabSwitchIntent, and clearMainTabSwitchIntent to locate and align the affected branches.
🤖 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.
Duplicate comments:
In `@web-ui/modules/app.methods.navigation.mjs`:
- Around line 445-449: The navigation intent is still being cleared with the
pre-remap tab value, so the orchestration-to-local-ai flow leaves state stuck.
Update the remaining clearMainTabSwitchIntent call sites in the navigation
method to use the same targetTab value that is passed to setMainTabSwitchIntent,
matching the existing remap logic in the main tab switch path. Use the
identifiers normalizedTab, targetTab, setMainTabSwitchIntent, and
clearMainTabSwitchIntent to locate and align the affected branches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 647d6976-90a6-4068-b6fa-7cec5c30c2c1
📒 Files selected for processing (32)
cli.jscli/local-bridge.jstests/e2e/test-task-orchestration.jstests/unit/config-tabs-ui.test.mjstests/unit/i18n-locales.test.mjstests/unit/session-tab-switch-performance.test.mjstests/unit/task-codex-exec-windows.test.mjstests/unit/web-ui-behavior-parity.test.mjstests/unit/web-ui-logic.test.mjstests/unit/web-ui-runtime-navigation-regression.test.mjsweb-ui/app.jsweb-ui/index.htmlweb-ui/modules/app.computed.main-tabs.mjsweb-ui/modules/app.methods.navigation.mjsweb-ui/modules/app.methods.task-orchestration.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/modules/task-kilo-webview-host.mjsweb-ui/partials/index/layout-header.htmlweb-ui/partials/index/panel-dashboard.htmlweb-ui/partials/index/panel-local-ai.htmlweb-ui/partials/index/panel-orchestration.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/session-helpers.mjsweb-ui/styles.cssweb-ui/styles/layout-shell.cssweb-ui/styles/local-ai-service.cssweb-ui/styles/navigation-panels.cssweb-ui/styles/task-orchestration.css
✅ Files skipped from review due to trivial changes (1)
- web-ui/index.html
🚧 Files skipped from review as they are similar to previous changes (25)
- web-ui/partials/index/panel-dashboard.html
- web-ui/styles/navigation-panels.css
- web-ui/session-helpers.mjs
- tests/unit/web-ui-runtime-navigation-regression.test.mjs
- web-ui/app.js
- tests/unit/session-tab-switch-performance.test.mjs
- web-ui/styles/layout-shell.css
- web-ui/partials/index/layout-header.html
- web-ui/partials/index/panel-orchestration.html
- tests/unit/i18n-locales.test.mjs
- web-ui/modules/i18n/locales/en.mjs
- tests/unit/config-tabs-ui.test.mjs
- web-ui/modules/i18n/locales/zh-tw.mjs
- tests/unit/task-codex-exec-windows.test.mjs
- web-ui/styles/local-ai-service.css
- cli/local-bridge.js
- web-ui/modules/i18n/locales/vi.mjs
- tests/unit/web-ui-behavior-parity.test.mjs
- web-ui/modules/i18n/locales/zh.mjs
- cli.js
- web-ui/modules/i18n/locales/ja.mjs
- web-ui/modules/task-kilo-webview-host.mjs
- web-ui/modules/app.computed.main-tabs.mjs
- tests/unit/web-ui-logic.test.mjs
- web-ui/modules/app.methods.task-orchestration.mjs
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.0)
tests/e2e/test-task-orchestration.js
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] 124-143: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(scriptPath, const http = require('http'); const fs = require('fs'); const portFile = process.argv[2]; const status = Number(process.argv[3] || 401); const message = process.argv[4] || 'Invalid token'; const server = http.createServer((req, res) => { req.resume(); req.on('end', () => { const body = JSON.stringify({ error: { message } }); res.writeHead(status, { 'Content-Type': 'application/json; charset=utf-8', 'Content-Length': Buffer.byteLength(body, 'utf-8') }); res.end(body, 'utf-8'); }); }); server.listen(0, '127.0.0.1', () => { fs.writeFileSync(portFile, String(server.address().port), 'utf-8'); }); process.on('SIGTERM', () => server.close(() => process.exit(0))); process.on('SIGINT', () => server.close(() => process.exit(0)));, 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 152-152: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(portFile, 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 187-187: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(configDir, 'codexmate-init.json'), JSON.stringify({ version: 1, mode: 'task-openai-chat-e2e' }), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] 230-230: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(configDir, 'codexmate-init.json'), JSON.stringify({ version: 1, mode: 'task-openai-chat-fallback-e2e' }), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 231-257: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(configDir, 'config.toml'), [
'model = "gpt-5.3-codex"',
'model_provider = "local"',
'',
'[model_providers.local]',
'name = "Local Codex"',
'base_url = "http://127.0.0.1:9/v1"',
'wire_api = "responses"',
'preferred_auth_method = "sk-codex-tab-secret"',
'models = ["gpt-5.3-codex"]',
'',
'[model_providers.bad-chat]',
'name = "Bad Chat"',
base_url = "${badBaseUrl}/v1",
'wire_api = "chat_completions"',
'preferred_auth_method = "sk-bad-token"',
'models = ["bad-model"]',
'',
'[model_providers.new-api-chat]',
'name = "New API Chat"',
base_url = "${fallbackBaseUrl}/v1",
'wire_api = "chat_completions"',
'preferred_auth_method = "sk-task-e2e-secret"',
'temperature = 0.7',
'models = ["glm-5.2"]',
''
].join('\n'), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 258-280: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(taskConfigDir, 'task-openai-chat-provider.json'), JSON.stringify({
selectedProvider: 'bad-chat',
selectedModels: { 'bad-chat': 'bad-model', 'new-api-chat': 'glm-5.2' },
providers: {
'bad-chat': {
name: 'Bad Chat',
base_url: ${badBaseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-bad-token',
models: ['bad-model'],
selected_model: 'bad-model'
},
'new-api-chat': {
name: 'New API Chat',
base_url: ${fallbackBaseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-task-e2e-secret',
temperature: 0.7,
models: ['glm-5.2'],
selected_model: 'glm-5.2'
}
}
}, null, 2), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
🪛 HTMLHint (1.9.2)
web-ui/partials/index/panel-local-ai.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🪛 Stylelint (17.14.0)
web-ui/styles.css
[error] 14-14: Expected "url('./styles/local-ai-service.css')" to be "'./styles/local-ai-service.css'" (import-notation)
(import-notation)
🔇 Additional comments (9)
web-ui/modules/app.methods.navigation.mjs (1)
14-14: LGTM!tests/e2e/test-task-orchestration.js (4)
122-181: Mock server lifecycle looks solid.Startup polling, stderr capture, and graceful SIGTERM→timeout→SIGKILL teardown in
close()are well structured. Static-analysis flags onrequire('child_process')/ non-literal fs paths here are not applicable since all paths derive from the test's owntmpHome, not external input.
209-282: Fallback config writers look correct and consistent.
writeOpenAiChatConfigandwriteOpenAiChatFallbackConfigcorrectly mirrorconfig.tomlprovider entries with thetask-openai-chat-provider.jsonselection metadata, matching the assertions made later (assertOpenAiRequests,assertOpenAiRunPayload).
326-352: LGTM!
876-921: 🩺 Stability & AvailabilityVerify failover retry loop can't spuriously fail on timing.
The loop asserts
x-codexmate-upstream-provider === 'new-api-chat'on every one of the (up to) 4 iterations (Line 889) while only using the loop to wait forx-codexmate-attempted-providersto include'bad-chat'(Line 890). If the freeze/failover logic incli/local-bridge.jsever skips retrying the previously-failed provider on a later iteration but still reports the healthy provider without ever recordingbad-chatinattempted-providers(e.g., due to a circuit-breaker that permanently excludes it after the very first failure without repopulating the header), this loop would exhaust its 4 attempts and fail at Line 895. This is likely fine given the routing design described in the PR, but the coupling between this test and internal freeze/attempt-header semantics incli/local-bridge.js(not in this review batch) should be double-checked for flakiness.web-ui/partials/index/panel-local-ai.html (3)
1-46: LGTM!
84-114: 🎯 Functional Correctness | ⚡ Quick winVerify
provider.keyis actually populated on provider objects.The
v-foruses:key="provider.key"(Line 85), but the e2e assertions for bothproviderOptionsandrouteProviderOptions(intests/e2e/test-task-orchestration.js) only ever checkname,type, andready— neverkey. If the underlying provider objects don't carry akeyfield, every item's key resolves toundefined, which Vue treats as "no key" without any dev warning, risking stale/mismatched DOM (and input state) when the provider list order or contents change (e.g., after adding the Claude provider mid-session, as the E2E test does).Consider keying on a guaranteed-unique field such as
provider.name(or a composite ofname+type) instead.💡 Suggested fix
- <article v-for="provider in (taskOrchestration.localAiServiceStatus.routeProviderOptions && taskOrchestration.localAiServiceStatus.routeProviderOptions.length ? taskOrchestration.localAiServiceStatus.routeProviderOptions : taskOrchestration.localAiServiceStatus.providerOptions)" :key="provider.key" class="provider-card" :data-provider-type="provider.type"> + <article v-for="provider in (taskOrchestration.localAiServiceStatus.routeProviderOptions && taskOrchestration.localAiServiceStatus.routeProviderOptions.length ? taskOrchestration.localAiServiceStatus.routeProviderOptions : taskOrchestration.localAiServiceStatus.providerOptions)" :key="provider.key || provider.name" class="provider-card" :data-provider-type="provider.type">
115-163: LGTM!web-ui/styles.css (1)
14-14: LGTM!
awsl233777
changed the title
|
Implemented the recommended A+B direction for this round. What changed:
Validation:
Screenshot evidence produced locally:
CodeRabbit was not checked or triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
web-ui/partials/index/panel-local-ai.html (1)
19-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRepeated readiness null-check expression.
taskOrchestration.localAiServiceStatus && taskOrchestration.localAiServiceStatus.readyis duplicated across the hero badge and endpoint card. A computed property (e.g.localAiReady) would reduce duplication.Also applies to: 33-33
🤖 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 `@web-ui/partials/index/panel-local-ai.html` around lines 19 - 21, The Local AI readiness null-check is duplicated in the template, so replace repeated `taskOrchestration.localAiServiceStatus && taskOrchestration.localAiServiceStatus.ready` usage with a single computed property such as `localAiReady` and reference it in both the hero badge and endpoint card. Update the `panel-local-ai` template bindings to use that shared computed value for both the class and label conditions so the readiness logic lives in one place and stays consistent.
🤖 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 `@web-ui/partials/index/panel-local-ai.html`:
- Around line 159-193: The empty state in panel-local-ai.html can render before
the first overview fetch completes because the current v-if only checks
localAiServiceStatus.providerOptions length. Update the conditional around the
provider list/empty-state to account for taskOrchestration.loading (and any
initial undefined state) so the empty-state only shows after loading has
finished and providerOptions is confirmed empty. Use the existing
taskOrchestration.loading and localAiServiceStatus/providerOptions bindings to
keep the initial render from flashing “no providers.”
- Around line 25-55: The readiness indicator in local-ai-endpoint-card is only
changing text, while local-ai-live-dot stays visually green even when
taskOrchestration.localAiServiceStatus.ready is false. Update the template in
panel-local-ai.html so the dot element also gets a readiness-based class/state,
similar to how local-ai-badge-good is conditionally applied, and then wire the
CSS in local-ai-service.css to render the pulse/color differently for ready
versus incomplete.
---
Nitpick comments:
In `@web-ui/partials/index/panel-local-ai.html`:
- Around line 19-21: The Local AI readiness null-check is duplicated in the
template, so replace repeated `taskOrchestration.localAiServiceStatus &&
taskOrchestration.localAiServiceStatus.ready` usage with a single computed
property such as `localAiReady` and reference it in both the hero badge and
endpoint card. Update the `panel-local-ai` template bindings to use that shared
computed value for both the class and label conditions so the readiness logic
lives in one place and stays consistent.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 983d7a7e-36d5-402d-9350-9cf6f2bae6c5
📒 Files selected for processing (11)
cli.jstests/e2e/test-task-orchestration.jstests/unit/config-tabs-ui.test.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/panel-local-ai.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/styles/local-ai-service.css
✅ Files skipped from review due to trivial changes (2)
- web-ui/modules/i18n/locales/ja.mjs
- web-ui/modules/i18n/locales/zh-tw.mjs
🚧 Files skipped from review as they are similar to previous changes (5)
- web-ui/modules/i18n/locales/vi.mjs
- tests/unit/config-tabs-ui.test.mjs
- web-ui/modules/i18n/locales/en.mjs
- web-ui/modules/i18n/locales/zh.mjs
- cli.js
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.0)
tests/e2e/test-task-orchestration.js
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] 208-222: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(taskConfigDir, 'local-ai-gateway.json'), JSON.stringify({
selectedProvider: 'local-ai-chat',
selectedModels: { 'local-ai-chat': 'glm-5.2' },
providers: {
'local-ai-chat': {
name: 'Local AI Chat',
base_url: ${baseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-local-ai-secret',
temperature: 0.7,
models: ['glm-5.2', 'glm-5.2-flash'],
selected_model: 'glm-5.2'
}
}
}, null, 2), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] 267-289: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(taskConfigDir, 'local-ai-gateway.json'), JSON.stringify({
selectedProvider: 'bad-chat',
selectedModels: { 'bad-chat': 'bad-model', 'local-ai-chat': 'glm-5.2' },
providers: {
'bad-chat': {
name: 'Bad Chat',
base_url: ${badBaseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-bad-token',
models: ['bad-model'],
selected_model: 'bad-model'
},
'local-ai-chat': {
name: 'Local AI Chat',
base_url: ${fallbackBaseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-local-ai-secret',
temperature: 0.7,
models: ['glm-5.2'],
selected_model: 'glm-5.2'
}
}
}, null, 2), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
🔇 Additional comments (10)
tests/e2e/test-task-orchestration.js (7)
157-170: LGTM!
200-223: LGTM!
259-290: LGTM!
331-335: LGTM!
549-582: LGTM!
584-594: LGTM!
917-965: LGTM!web-ui/partials/index/panel-local-ai.html (2)
196-235: LGTM!
105-140: 🎯 Functional CorrectnessThis panel is intentionally mock-ready; the hardcoded rows are descriptive example content, not live trace output.
> Likely an incorrect or invalid review comment.web-ui/styles/local-ai-service.css (1)
1-432: LGTM!
awsl233777
changed the title
|
Implemented the selected 方案 1 / Calm Console layout. What changed:
Validation:
Screenshot evidence produced locally:
CodeRabbit was not checked or triggered. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web-ui/styles/local-ai-service.css (1)
140-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded hex colors bypass the theme token system.
The rest of the file consistently uses
var(--color-*)tokens, but.local-ai-live-dot::before(line 145) and.local-ai-trace-row span(line 294) use raw hex colors (#22a05a,#168a45). This breaks theme consistency (e.g. dark mode) established elsewhere in this stylesheet.♻️ Suggested fix
.local-ai-live-dot::before { content: ""; width: 8px; height: 8px; border-radius: 50%; - background: `#22a05a`; - box-shadow: 0 0 0 6px rgba(34, 160, 90, 0.12); + background: var(--color-success, `#22a05a`); + box-shadow: 0 0 0 6px color-mix(in srgb, var(--color-success, `#22a05a`) 12%, transparent); }.local-ai-trace-row span { justify-self: end; - color: `#168a45`; + color: var(--color-success, `#168a45`); font-size: 12px; font-weight: 850; }Also applies to: 292-297
🤖 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 `@web-ui/styles/local-ai-service.css` around lines 140 - 147, Replace the hardcoded hex colors in the local AI status styles with existing theme tokens so the stylesheet stays consistent with the rest of the file. Update the `.local-ai-live-dot::before` rule and the `.local-ai-trace-row span` styling to use `var(--color-*)` values instead of `#22a05a` and `#168a45`, reusing the appropriate green token or defining a matching token if needed. Keep the same visual intent, but route all color values through the theme token system.
🤖 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.
Nitpick comments:
In `@web-ui/styles/local-ai-service.css`:
- Around line 140-147: Replace the hardcoded hex colors in the local AI status
styles with existing theme tokens so the stylesheet stays consistent with the
rest of the file. Update the `.local-ai-live-dot::before` rule and the
`.local-ai-trace-row span` styling to use `var(--color-*)` values instead of
`#22a05a` and `#168a45`, reusing the appropriate green token or defining a
matching token if needed. Keep the same visual intent, but route all color
values through the theme token system.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 211eaa16-4695-48e8-a5ef-b9d0ccf8c70c
📒 Files selected for processing (8)
web-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/panel-local-ai.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/styles/local-ai-service.css
🚧 Files skipped from review as they are similar to previous changes (6)
- web-ui/partials/index/panel-local-ai.html
- web-ui/modules/i18n/locales/en.mjs
- web-ui/modules/i18n/locales/zh.mjs
- web-ui/modules/i18n/locales/zh-tw.mjs
- web-ui/modules/i18n/locales/ja.mjs
- web-ui/modules/i18n/locales/vi.mjs
📜 Review details
🔇 Additional comments (2)
web-ui/styles/local-ai-service.css (2)
5-59: LGTM!Also applies to: 96-415
16-21: 🎯 Functional CorrectnessThese selectors are scoped to the Local AI panel, so the leakage concern doesn’t apply.
> Likely an incorrect or invalid review comment.
awsl233777
force-pushed
the
feat/local-ai-service-provider-console
branch
2 times, most recently
from
586af60 to
ef09f77
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
web-ui/partials/index/panel-local-ai.html (1)
56-58: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win"Form Open" button doesn't actually toggle the form closed.
Text switches to
localAi.action.formOpenwhen open, but the click handler still callsopenTaskProviderSetupunconditionally — clicking again won't close the form via this control, only via the separate Cancel button. Consider toggling directly.💡 Proposed fix
- <button type="button" class="primary-button" `@click`="openTaskProviderSetup" aria-controls="local-ai-provider-form" :aria-expanded="taskOrchestration.providerSetupOpen ? 'true' : 'false'"> + <button type="button" class="primary-button" `@click`="taskOrchestration.providerSetupOpen ? closeTaskProviderSetup() : openTaskProviderSetup()" aria-controls="local-ai-provider-form" :aria-expanded="taskOrchestration.providerSetupOpen ? 'true' : 'false'"> {{ taskOrchestration.providerSetupOpen ? t('localAi.action.formOpen') : t('localAi.action.newProvider') }} </button>🤖 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 `@web-ui/partials/index/panel-local-ai.html` around lines 56 - 58, The button in panel-local-ai.html uses openTaskProviderSetup unconditionally, so when taskOrchestration.providerSetupOpen is already true it cannot toggle the form closed. Update the click handling on this primary-button to switch behavior based on providerSetupOpen: openTaskProviderSetup when closed and the corresponding close/toggle action when open, keeping the existing aria-expanded and label logic in sync.tests/e2e/test-task-orchestration.js (1)
361-387: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueOptional: guard the response stream against mid-flight errors.
requestJsonhandlesreqerrors and request timeouts, but not aresstream error. If the upstream socket drops after headers arrive, neitherresolvenorrejectfires and the promise hangs until the outer harness timeout. Since this helper is now used inside a polling loop, adding ares.on('error', reject)keeps failures fast and observable.♻️ Proposed guard
}, (res) => { let raw = ''; res.setEncoding('utf-8'); + res.on('error', reject); res.on('data', chunk => { raw += chunk; });🤖 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/e2e/test-task-orchestration.js` around lines 361 - 387, The requestJson helper can hang if the response stream fails after headers are received because only req errors and timeouts are handled. Update requestJson to also listen for res stream errors and reject the Promise when they occur, using the existing request/response flow in requestJson so polling failures surface immediately instead of waiting for the outer harness timeout.
🤖 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 `@web-ui/partials/index/panel-local-ai.html`:
- Around line 127-133: The endpoint and model input placeholders in the local AI
panel are hardcoded English strings, which breaks i18n consistency with the rest
of the form. Update the placeholders in the relevant input fields within the
panel-local-ai template to use translated keys via t(...) like the existing
provider name placeholder, and add the corresponding locale entries for the
endpoint and model examples.
---
Nitpick comments:
In `@tests/e2e/test-task-orchestration.js`:
- Around line 361-387: The requestJson helper can hang if the response stream
fails after headers are received because only req errors and timeouts are
handled. Update requestJson to also listen for res stream errors and reject the
Promise when they occur, using the existing request/response flow in requestJson
so polling failures surface immediately instead of waiting for the outer harness
timeout.
In `@web-ui/partials/index/panel-local-ai.html`:
- Around line 56-58: The button in panel-local-ai.html uses
openTaskProviderSetup unconditionally, so when
taskOrchestration.providerSetupOpen is already true it cannot toggle the form
closed. Update the click handling on this primary-button to switch behavior
based on providerSetupOpen: openTaskProviderSetup when closed and the
corresponding close/toggle action when open, keeping the existing aria-expanded
and label logic in sync.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dddd2ee6-80f9-4c43-ba71-bb3b4cf305fc
📒 Files selected for processing (32)
cli.jscli/local-bridge.jstests/e2e/test-task-orchestration.jstests/unit/config-tabs-ui.test.mjstests/unit/i18n-locales.test.mjstests/unit/session-tab-switch-performance.test.mjstests/unit/task-codex-exec-windows.test.mjstests/unit/web-ui-behavior-parity.test.mjstests/unit/web-ui-logic.test.mjstests/unit/web-ui-runtime-navigation-regression.test.mjsweb-ui/app.jsweb-ui/index.htmlweb-ui/modules/app.computed.main-tabs.mjsweb-ui/modules/app.methods.navigation.mjsweb-ui/modules/app.methods.task-orchestration.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/modules/task-kilo-webview-host.mjsweb-ui/partials/index/layout-header.htmlweb-ui/partials/index/panel-dashboard.htmlweb-ui/partials/index/panel-local-ai.htmlweb-ui/partials/index/panel-orchestration.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/session-helpers.mjsweb-ui/styles.cssweb-ui/styles/layout-shell.cssweb-ui/styles/local-ai-service.cssweb-ui/styles/navigation-panels.cssweb-ui/styles/task-orchestration.css
✅ Files skipped from review due to trivial changes (3)
- web-ui/styles/navigation-panels.css
- web-ui/index.html
- web-ui/modules/i18n/locales/zh.mjs
🚧 Files skipped from review as they are similar to previous changes (23)
- web-ui/partials/index/panel-dashboard.html
- web-ui/app.js
- tests/unit/web-ui-runtime-navigation-regression.test.mjs
- web-ui/styles/layout-shell.css
- tests/unit/session-tab-switch-performance.test.mjs
- web-ui/modules/app.methods.navigation.mjs
- tests/unit/web-ui-behavior-parity.test.mjs
- tests/unit/i18n-locales.test.mjs
- web-ui/session-helpers.mjs
- tests/unit/config-tabs-ui.test.mjs
- web-ui/partials/index/layout-header.html
- web-ui/partials/index/panel-orchestration.html
- cli/local-bridge.js
- web-ui/modules/i18n/locales/zh-tw.mjs
- web-ui/modules/i18n/locales/en.mjs
- tests/unit/task-codex-exec-windows.test.mjs
- web-ui/modules/task-kilo-webview-host.mjs
- cli.js
- tests/unit/web-ui-logic.test.mjs
- web-ui/modules/i18n/locales/vi.mjs
- web-ui/modules/app.computed.main-tabs.mjs
- web-ui/modules/i18n/locales/ja.mjs
- web-ui/modules/app.methods.task-orchestration.mjs
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.0)
tests/e2e/test-task-orchestration.js
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] 124-143: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(scriptPath, const http = require('http'); const fs = require('fs'); const portFile = process.argv[2]; const status = Number(process.argv[3] || 401); const message = process.argv[4] || 'Invalid token'; const server = http.createServer((req, res) => { req.resume(); req.on('end', () => { const body = JSON.stringify({ error: { message } }); res.writeHead(status, { 'Content-Type': 'application/json; charset=utf-8', 'Content-Length': Buffer.byteLength(body, 'utf-8') }); res.end(body, 'utf-8'); }); }); server.listen(0, '127.0.0.1', () => { fs.writeFileSync(portFile, String(server.address().port), 'utf-8'); }); process.on('SIGTERM', () => server.close(() => process.exit(0))); process.on('SIGINT', () => server.close(() => process.exit(0)));, 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 152-152: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(portFile, 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 187-187: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(configDir, 'codexmate-init.json'), JSON.stringify({ version: 1, mode: 'task-openai-chat-e2e' }), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] 208-222: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(taskConfigDir, 'local-ai-gateway.json'), JSON.stringify({
selectedProvider: 'local-ai-chat',
selectedModels: { 'local-ai-chat': 'glm-5.2' },
providers: {
'local-ai-chat': {
name: 'Local AI Chat',
base_url: ${baseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-local-ai-secret',
temperature: 0.7,
models: ['glm-5.2', 'glm-5.2-flash'],
selected_model: 'glm-5.2'
}
}
}, null, 2), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 224-238: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(taskConfigDir, 'task-openai-chat-provider.json'), JSON.stringify({
selectedProvider: 'new-api-chat',
selectedModels: { 'new-api-chat': 'glm-5.2' },
providers: {
'new-api-chat': {
name: 'New API Chat',
base_url: ${baseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-task-e2e-secret',
temperature: 0.7,
models: ['glm-5.2', 'glm-5.2-flash'],
selected_model: 'glm-5.2'
}
}
}, null, 2), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 246-246: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(configDir, 'codexmate-init.json'), JSON.stringify({ version: 1, mode: 'task-openai-chat-fallback-e2e' }), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 247-265: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(configDir, 'config.toml'), [
'model = "gpt-5.3-codex"',
'model_provider = "local"',
'',
'[model_providers.local]',
'name = "Local Codex"',
'base_url = "http://127.0.0.1:9/v1"',
'wire_api = "responses"',
'preferred_auth_method = "sk-codex-tab-secret"',
'models = ["gpt-5.3-codex"]',
'',
'[model_providers.codex-coupling-trap]',
'name = "Codex Coupling Trap"',
'base_url = "http://127.0.0.1:9/v1"',
'wire_api = "chat_completions"',
'preferred_auth_method = "sk-codex-tab-secret"',
'models = ["codex-trap-model"]',
''
].join('\n'), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 267-289: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(taskConfigDir, 'local-ai-gateway.json'), JSON.stringify({
selectedProvider: 'bad-chat',
selectedModels: { 'bad-chat': 'bad-model', 'local-ai-chat': 'glm-5.2' },
providers: {
'bad-chat': {
name: 'Bad Chat',
base_url: ${badBaseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-bad-token',
models: ['bad-model'],
selected_model: 'bad-model'
},
'local-ai-chat': {
name: 'Local AI Chat',
base_url: ${fallbackBaseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-local-ai-secret',
temperature: 0.7,
models: ['glm-5.2'],
selected_model: 'glm-5.2'
}
}
}, null, 2), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 291-313: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(taskConfigDir, 'task-openai-chat-provider.json'), JSON.stringify({
selectedProvider: 'bad-chat',
selectedModels: { 'bad-chat': 'bad-model', 'new-api-chat': 'glm-5.2' },
providers: {
'bad-chat': {
name: 'Bad Chat',
base_url: ${badBaseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-bad-token',
models: ['bad-model'],
selected_model: 'bad-model'
},
'new-api-chat': {
name: 'New API Chat',
base_url: ${fallbackBaseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-task-e2e-secret',
temperature: 0.7,
models: ['glm-5.2'],
selected_model: 'glm-5.2'
}
}
}, null, 2), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
🪛 HTMLHint (1.9.2)
web-ui/partials/index/panel-local-ai.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🪛 Stylelint (17.14.0)
web-ui/styles.css
[error] 14-14: Expected "url('./styles/local-ai-service.css')" to be "'./styles/local-ai-service.css'" (import-notation)
(import-notation)
🔇 Additional comments (10)
web-ui/partials/index/panel-local-ai.html (2)
62-96: Empty-state can still flash before data loads.The provider-list conditional still only checks
providerOptions && ...length, with no guard fortaskOrchestration.loading/initial undefined state, so "no providers" can flash before the first overview fetch resolves.
1-153: HTMLHint doctype hint is a false positive.This is a Vue partial included into
index.html, not a standalone document; no doctype is expected here.Source: Linters/SAST tools
web-ui/styles/local-ai-service.css (1)
1-286: LGTM!tests/e2e/test-task-orchestration.js (6)
2-2: LGTM!
122-181: LGTM!
183-240: LGTM!Also applies to: 242-315
327-342: LGTM!
549-595: LGTM!
917-965: LGTM!web-ui/styles.css (1)
14-14: LGTM!
awsl233777
force-pushed
the
feat/local-ai-service-provider-console
branch
from
ef09f77 to
ab398be
Compare
|
Pushed Changed:
Validation:
CodeRabbit not triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
web-ui/styles/local-ai-service.css (3)
85-94: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNon-standard font-weight values.
font-weight: 760(line 90) andfont-weight: 740(line 188) aren't standard weight steps; static (non-variable) fonts will just snap to the nearest defined weight (usually 700 or 800), making these specific values misleading/no-op in most environments.💅 Suggested fix
.local-ai-button { min-height: 38px; border-radius: 12px; padding: 9px 14px; font-size: 13px; - font-weight: 760; + font-weight: 700; letter-spacing: -0.01em;.local-ai-brief-item code { display: inline-flex; max-width: 100%; margin-top: 12px; border-radius: 12px; padding: 8px 10px; background: var(--color-brand-dark); color: `#ffe5ce`; font-size: 13px; - font-weight: 740; + font-weight: 700;Also applies to: 179-190
🤖 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 `@web-ui/styles/local-ai-service.css` around lines 85 - 94, The local-ai-service button styles use non-standard font-weight values that will be ignored or snapped in many fonts, so update the font-weight declarations in the local-ai-button and related local AI service CSS rules to standard numeric steps that are supported consistently. Review the affected selectors in local-ai-service.css, especially local-ai-button and the other nearby local AI service button/text styles referenced in the comment, and replace the 760/740 values with an appropriate standard weight such as 700 or 800 to match the intended visual emphasis.
145-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded hex colors bypass the design-token system.
.status-pill.success/.local-ai-badge-good(Lines 145-150),.status-pill.warn(Lines 152-156), and the code chip text color#ffe5ce(Line 186) use raw hex/rgba values while the rest of the file consistently usesvar(--color-*)tokens. This will diverge from theme changes (e.g., dark mode) that the token-based rules automatically pick up.Also applies to: 179-190
🤖 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 `@web-ui/styles/local-ai-service.css` around lines 145 - 156, Hardcoded color values are bypassing the design-token system in the local AI service styles. Update the selectors around .local-ai-service-badges .local-ai-badge-good, .status-pill.success, and .status-pill.warn to use existing var(--color-*) tokens instead of raw hex/rgba values, and apply the same token-based cleanup to the code chip text styling in the related 179-190 rules. Keep the intent of the current success/warn visuals, but express all colors through theme tokens so dark mode and future palette changes continue to work automatically.
283-291: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent focus state: use
:focus-visiblefor consistency.
.local-ai-upstream-carduses:focuswhile.local-ai-button(Lines 104-107) uses:focus-visible. Mouse clicks will now show a persistent outline on this card, unlike buttons elsewhere in the panel.💅 Suggested fix
-.local-ai-upstream-card:focus { +.local-ai-upstream-card:focus-visible { outline: 2px solid color-mix(in srgb, var(--color-brand-dark) 32%, transparent); outline-offset: 3px; }🤖 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 `@web-ui/styles/local-ai-service.css` around lines 283 - 291, The `.local-ai-upstream-card` focus styling is inconsistent with `.local-ai-button` because it uses `:focus` instead of `:focus-visible`, which causes mouse clicks to show the outline. Update the focus rule on `.local-ai-upstream-card` in the same stylesheet to use `:focus-visible` so it matches the panel’s other interactive elements and only appears for keyboard focus.
🤖 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 `@web-ui/partials/index/panel-local-ai.html`:
- Line 24: The aria-label on the Local AI summary section is hardcoded English
and should be localized like the rest of the panel. Update the
local-ai-brief-row markup in panel-local-ai.html to route the label through the
existing t(...) translation helper so the accessible text is translated
consistently with other user-facing copy.
---
Nitpick comments:
In `@web-ui/styles/local-ai-service.css`:
- Around line 85-94: The local-ai-service button styles use non-standard
font-weight values that will be ignored or snapped in many fonts, so update the
font-weight declarations in the local-ai-button and related local AI service CSS
rules to standard numeric steps that are supported consistently. Review the
affected selectors in local-ai-service.css, especially local-ai-button and the
other nearby local AI service button/text styles referenced in the comment, and
replace the 760/740 values with an appropriate standard weight such as 700 or
800 to match the intended visual emphasis.
- Around line 145-156: Hardcoded color values are bypassing the design-token
system in the local AI service styles. Update the selectors around
.local-ai-service-badges .local-ai-badge-good, .status-pill.success, and
.status-pill.warn to use existing var(--color-*) tokens instead of raw hex/rgba
values, and apply the same token-based cleanup to the code chip text styling in
the related 179-190 rules. Keep the intent of the current success/warn visuals,
but express all colors through theme tokens so dark mode and future palette
changes continue to work automatically.
- Around line 283-291: The `.local-ai-upstream-card` focus styling is
inconsistent with `.local-ai-button` because it uses `:focus` instead of
`:focus-visible`, which causes mouse clicks to show the outline. Update the
focus rule on `.local-ai-upstream-card` in the same stylesheet to use
`:focus-visible` so it matches the panel’s other interactive elements and only
appears for keyboard focus.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7d14df91-46d5-4de1-abea-81ae5e87e23b
📒 Files selected for processing (32)
cli.jscli/local-bridge.jstests/e2e/test-task-orchestration.jstests/unit/config-tabs-ui.test.mjstests/unit/i18n-locales.test.mjstests/unit/session-tab-switch-performance.test.mjstests/unit/task-codex-exec-windows.test.mjstests/unit/web-ui-behavior-parity.test.mjstests/unit/web-ui-logic.test.mjstests/unit/web-ui-runtime-navigation-regression.test.mjsweb-ui/app.jsweb-ui/index.htmlweb-ui/modules/app.computed.main-tabs.mjsweb-ui/modules/app.methods.navigation.mjsweb-ui/modules/app.methods.task-orchestration.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/modules/task-kilo-webview-host.mjsweb-ui/partials/index/layout-header.htmlweb-ui/partials/index/panel-dashboard.htmlweb-ui/partials/index/panel-local-ai.htmlweb-ui/partials/index/panel-orchestration.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/session-helpers.mjsweb-ui/styles.cssweb-ui/styles/layout-shell.cssweb-ui/styles/local-ai-service.cssweb-ui/styles/navigation-panels.cssweb-ui/styles/task-orchestration.css
✅ Files skipped from review due to trivial changes (1)
- web-ui/index.html
🚧 Files skipped from review as they are similar to previous changes (25)
- web-ui/partials/index/panel-dashboard.html
- web-ui/styles/layout-shell.css
- web-ui/session-helpers.mjs
- web-ui/app.js
- tests/unit/web-ui-runtime-navigation-regression.test.mjs
- tests/unit/i18n-locales.test.mjs
- tests/unit/session-tab-switch-performance.test.mjs
- web-ui/styles/navigation-panels.css
- tests/unit/config-tabs-ui.test.mjs
- web-ui/modules/i18n/locales/ja.mjs
- tests/unit/task-codex-exec-windows.test.mjs
- web-ui/modules/i18n/locales/zh.mjs
- web-ui/partials/index/panel-orchestration.html
- web-ui/partials/index/layout-header.html
- web-ui/modules/i18n/locales/zh-tw.mjs
- web-ui/modules/i18n/locales/vi.mjs
- web-ui/modules/task-kilo-webview-host.mjs
- web-ui/modules/i18n/locales/en.mjs
- tests/unit/web-ui-behavior-parity.test.mjs
- cli/local-bridge.js
- tests/unit/web-ui-logic.test.mjs
- web-ui/modules/app.methods.navigation.mjs
- cli.js
- web-ui/modules/app.computed.main-tabs.mjs
- web-ui/modules/app.methods.task-orchestration.mjs
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.0)
tests/e2e/test-task-orchestration.js
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] 124-143: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(scriptPath, const http = require('http'); const fs = require('fs'); const portFile = process.argv[2]; const status = Number(process.argv[3] || 401); const message = process.argv[4] || 'Invalid token'; const server = http.createServer((req, res) => { req.resume(); req.on('end', () => { const body = JSON.stringify({ error: { message } }); res.writeHead(status, { 'Content-Type': 'application/json; charset=utf-8', 'Content-Length': Buffer.byteLength(body, 'utf-8') }); res.end(body, 'utf-8'); }); }); server.listen(0, '127.0.0.1', () => { fs.writeFileSync(portFile, String(server.address().port), 'utf-8'); }); process.on('SIGTERM', () => server.close(() => process.exit(0))); process.on('SIGINT', () => server.close(() => process.exit(0)));, 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 152-152: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(portFile, 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 187-187: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(configDir, 'codexmate-init.json'), JSON.stringify({ version: 1, mode: 'task-openai-chat-e2e' }), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] 208-222: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(taskConfigDir, 'local-ai-gateway.json'), JSON.stringify({
selectedProvider: 'local-ai-chat',
selectedModels: { 'local-ai-chat': 'glm-5.2' },
providers: {
'local-ai-chat': {
name: 'Local AI Chat',
base_url: ${baseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-local-ai-secret',
temperature: 0.7,
models: ['glm-5.2', 'glm-5.2-flash'],
selected_model: 'glm-5.2'
}
}
}, null, 2), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 224-238: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(taskConfigDir, 'task-openai-chat-provider.json'), JSON.stringify({
selectedProvider: 'new-api-chat',
selectedModels: { 'new-api-chat': 'glm-5.2' },
providers: {
'new-api-chat': {
name: 'New API Chat',
base_url: ${baseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-task-e2e-secret',
temperature: 0.7,
models: ['glm-5.2', 'glm-5.2-flash'],
selected_model: 'glm-5.2'
}
}
}, null, 2), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 246-246: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(configDir, 'codexmate-init.json'), JSON.stringify({ version: 1, mode: 'task-openai-chat-fallback-e2e' }), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 247-265: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(configDir, 'config.toml'), [
'model = "gpt-5.3-codex"',
'model_provider = "local"',
'',
'[model_providers.local]',
'name = "Local Codex"',
'base_url = "http://127.0.0.1:9/v1"',
'wire_api = "responses"',
'preferred_auth_method = "sk-codex-tab-secret"',
'models = ["gpt-5.3-codex"]',
'',
'[model_providers.codex-coupling-trap]',
'name = "Codex Coupling Trap"',
'base_url = "http://127.0.0.1:9/v1"',
'wire_api = "chat_completions"',
'preferred_auth_method = "sk-codex-tab-secret"',
'models = ["codex-trap-model"]',
''
].join('\n'), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 267-289: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(taskConfigDir, 'local-ai-gateway.json'), JSON.stringify({
selectedProvider: 'bad-chat',
selectedModels: { 'bad-chat': 'bad-model', 'local-ai-chat': 'glm-5.2' },
providers: {
'bad-chat': {
name: 'Bad Chat',
base_url: ${badBaseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-bad-token',
models: ['bad-model'],
selected_model: 'bad-model'
},
'local-ai-chat': {
name: 'Local AI Chat',
base_url: ${fallbackBaseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-local-ai-secret',
temperature: 0.7,
models: ['glm-5.2'],
selected_model: 'glm-5.2'
}
}
}, null, 2), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 291-313: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(taskConfigDir, 'task-openai-chat-provider.json'), JSON.stringify({
selectedProvider: 'bad-chat',
selectedModels: { 'bad-chat': 'bad-model', 'new-api-chat': 'glm-5.2' },
providers: {
'bad-chat': {
name: 'Bad Chat',
base_url: ${badBaseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-bad-token',
models: ['bad-model'],
selected_model: 'bad-model'
},
'new-api-chat': {
name: 'New API Chat',
base_url: ${fallbackBaseUrl}/v1,
wire_api: 'chat_completions',
preferred_auth_method: 'sk-task-e2e-secret',
temperature: 0.7,
models: ['glm-5.2'],
selected_model: 'glm-5.2'
}
}
}, null, 2), 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
🪛 HTMLHint (1.9.2)
web-ui/partials/index/panel-local-ai.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🪛 Stylelint (17.14.0)
web-ui/styles.css
[error] 14-14: Expected "url('./styles/local-ai-service.css')" to be "'./styles/local-ai-service.css'" (import-notation)
(import-notation)
🔇 Additional comments (8)
web-ui/styles/local-ai-service.css (1)
1-345: Structure, layout grids, responsive breakpoints, and token usage otherwise look solid and match the panel-local-ai.html consumer structure.web-ui/partials/index/panel-local-ai.html (3)
93-96: Previously flagged issues now resolved.The empty-state loading race and untranslated endpoint/model placeholders flagged in earlier reviews are fixed here —
v-else-ifnow guards on!taskOrchestration.loading(Line 93), and placeholders uset('placeholder.apiEndpointExample')/t('placeholder.modelExample')(Lines 128, 132).Also applies to: 128-128, 132-132
1-22: LGTM!
45-150: LGTM!tests/e2e/test-task-orchestration.js (3)
2-2: LGTM!Also applies to: 122-181, 183-240, 242-315, 327-341, 361-389
917-965: 🎯 Functional CorrectnessNo issue here:
cli.jsemits the fallback log line, andcli/local-bridge.jsalready setsX-Codexmate-Attempted-Providerson bridge responses.> Likely an incorrect or invalid review comment.
549-595: 🎯 Functional CorrectnessNo issue:
local-ai-service-overview,/bridge/local/v1/*, andx-codexmate-upstream-providerare implemented incli.js/cli/local-bridge.js.web-ui/styles.css (1)
14-14: LGTM!
| </div> | ||
| </section> | ||
|
|
||
| <section class="local-ai-brief-row" aria-label="Local AI gateway summary"> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Untranslated aria-label.
aria-label="Local AI gateway summary" is hardcoded English, inconsistent with the rest of the panel which routes all user-facing copy through t(...).
💡 Proposed fix
- <section class="local-ai-brief-row" aria-label="Local AI gateway summary">
+ <section class="local-ai-brief-row" :aria-label="t('localAi.brief.ariaLabel')">📝 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.
| <section class="local-ai-brief-row" aria-label="Local AI gateway summary"> | |
| <section class="local-ai-brief-row" :aria-label="t('localAi.brief.ariaLabel')"> |
🤖 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 `@web-ui/partials/index/panel-local-ai.html` at line 24, The aria-label on the
Local AI summary section is hardcoded English and should be localized like the
rest of the panel. Update the local-ai-brief-row markup in panel-local-ai.html
to route the label through the existing t(...) translation helper so the
accessible text is translated consistently with other user-facing copy.
|
Pushed Changed:
Validation:
CodeRabbit not manually triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@web-ui/styles/local-ai-service.css`:
- Around line 169-185: Add the missing shrink rule to the
`.local-ai-summary-meta` grid item used by `.local-ai-gateway-summary` so long
localized text cannot force the two-column layout wider than intended. Update
the same responsive sizing rule set that already includes
`.local-ai-summary-main` and `.local-ai-summary-meta > div` so
`.local-ai-summary-meta` itself also gets `min-width: 0`, keeping the nested
grid from overflowing.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f6b0a1eb-5ff3-4eb6-b3b1-ebb3d1c4b050
📒 Files selected for processing (4)
tests/unit/config-tabs-ui.test.mjsweb-ui/partials/index/panel-local-ai.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/styles/local-ai-service.css
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/unit/config-tabs-ui.test.mjs
- web-ui/partials/index/panel-local-ai.html
📜 Review details
🔇 Additional comments (2)
web-ui/styles/local-ai-service.css (2)
158-162: LGTM!
335-366: LGTM!
| .local-ai-gateway-summary { | ||
| display: grid; | ||
| grid-template-columns: minmax(260px, 1.2fr) minmax(320px, 1.8fr); | ||
| gap: 18px; | ||
| } | ||
|
|
||
| .local-ai-summary-main, | ||
| .local-ai-summary-meta > div { | ||
| min-width: 0; | ||
| } | ||
|
|
||
| .local-ai-summary-meta { | ||
| display: grid; | ||
| grid-template-columns: repeat(2, minmax(0, 1fr)); | ||
| gap: 14px; | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm no ancestor rule already resets min-width for nested grid items.
rg -n 'local-ai-brief-item|local-ai-summary-meta|min-width' web-ui/styles/local-ai-service.cssRepository: SakuraByteCore/codexmate
Length of output: 512
🏁 Script executed:
#!/bin/bash
sed -n '150,190p' web-ui/styles/local-ai-service.css
printf '\n---\n'
sed -n '332,360p' web-ui/styles/local-ai-service.cssRepository: SakuraByteCore/codexmate
Length of output: 1486
Add min-width: 0 to .local-ai-summary-meta
.local-ai-summary-meta is the direct grid item inside .local-ai-gateway-summary, so long localized policy/trace text can still push the two-column layout wider than intended. Add it to the same shrink rule:
🩹 Proposed fix
.local-ai-summary-main,
+.local-ai-summary-meta,
.local-ai-summary-meta > div {
min-width: 0;
}📝 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.
| .local-ai-gateway-summary { | |
| display: grid; | |
| grid-template-columns: minmax(260px, 1.2fr) minmax(320px, 1.8fr); | |
| gap: 18px; | |
| } | |
| .local-ai-summary-main, | |
| .local-ai-summary-meta > div { | |
| min-width: 0; | |
| } | |
| .local-ai-summary-meta { | |
| display: grid; | |
| grid-template-columns: repeat(2, minmax(0, 1fr)); | |
| gap: 14px; | |
| } | |
| .local-ai-gateway-summary { | |
| display: grid; | |
| grid-template-columns: minmax(260px, 1.2fr) minmax(320px, 1.8fr); | |
| gap: 18px; | |
| } | |
| .local-ai-summary-main, | |
| .local-ai-summary-meta, | |
| .local-ai-summary-meta > div { | |
| min-width: 0; | |
| } | |
| .local-ai-summary-meta { | |
| display: grid; | |
| grid-template-columns: repeat(2, minmax(0, 1fr)); | |
| gap: 14px; | |
| } |
🤖 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 `@web-ui/styles/local-ai-service.css` around lines 169 - 185, Add the missing
shrink rule to the `.local-ai-summary-meta` grid item used by
`.local-ai-gateway-summary` so long localized text cannot force the two-column
layout wider than intended. Update the same responsive sizing rule set that
already includes `.local-ai-summary-main` and `.local-ai-summary-meta > div` so
`.local-ai-summary-meta` itself also gets `min-width: 0`, keeping the nested
grid from overflowing.
|
Pushed Changed:
Validation:
CodeRabbit not manually triggered. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/e2e/test-task-orchestration.js (1)
597-604: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: also cover the rename path.
This new edit-flow test keeps
originalNameandnameidentical ('claude-local'), validating key preservation and field updates but not an actual rename (originalNamediffering fromname). Since the commit introduces provider editing generally, consider adding a follow-up case where the name changes to ensure the backend correctly resolves the old entry viaoriginalNameand persists it under the new name.🤖 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/e2e/test-task-orchestration.js` around lines 597 - 604, The save-local-ai-provider edit-flow test currently only covers updating an existing provider without changing its name, so it misses the rename path. Add a follow-up case in test-task-orchestration.js around the save-local-ai-provider call to set originalName and name to different values, then verify the backend resolves the old provider by originalName and persists the edited provider under the new name while preserving the expected fields. Use the existing save-local-ai-provider helper and the claude-local test setup to keep the new rename coverage close to the current edit-flow assertions.
🤖 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.
Nitpick comments:
In `@tests/e2e/test-task-orchestration.js`:
- Around line 597-604: The save-local-ai-provider edit-flow test currently only
covers updating an existing provider without changing its name, so it misses the
rename path. Add a follow-up case in test-task-orchestration.js around the
save-local-ai-provider call to set originalName and name to different values,
then verify the backend resolves the old provider by originalName and persists
the edited provider under the new name while preserving the expected fields. Use
the existing save-local-ai-provider helper and the claude-local test setup to
keep the new rename coverage close to the current edit-flow assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0bfa3fea-c8d6-4382-bc61-d93f6b2f5f2a
📒 Files selected for processing (8)
cli.jstests/e2e/test-task-orchestration.jstests/unit/config-tabs-ui.test.mjstests/unit/web-ui-logic.test.mjsweb-ui/modules/app.methods.task-orchestration.mjsweb-ui/partials/index/panel-local-ai.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/styles/local-ai-service.css
🚧 Files skipped from review as they are similar to previous changes (6)
- web-ui/partials/index/panel-local-ai.html
- tests/unit/config-tabs-ui.test.mjs
- web-ui/styles/local-ai-service.css
- cli.js
- web-ui/modules/app.methods.task-orchestration.mjs
- tests/unit/web-ui-logic.test.mjs
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.0)
tests/e2e/test-task-orchestration.js
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
🔇 Additional comments (1)
tests/e2e/test-task-orchestration.js (1)
597-611: LGTM!
|
Pushed Changed:
Validation:
CodeRabbit not manually triggered. |
|
Pushed Changed:
Validation:
CodeRabbit not manually triggered. |
|
Pushed Changed:
Validation:
CodeRabbit not manually triggered. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
web-ui/modules/app.methods.task-orchestration.mjs (2)
657-662: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider skipping the model refetch on each poll.
loadTaskOrchestrationOverviewruns on the 4s polling loop while a run is active, sofetchLocalAiGatewayModels({ silent: true })fires a/modelsGET every cycle even though detected models rarely change. Consider only fetching when the model list is empty or the gateway endpoint/readiness actually changed. Note: thesilentoption is passed butfetchLocalAiGatewayModelsnever reads it.🤖 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 `@web-ui/modules/app.methods.task-orchestration.mjs` around lines 657 - 662, The polling path in loadTaskOrchestrationOverview is refetching local AI gateway models on every cycle via fetchLocalAiGatewayModels even though the models rarely change, and the silent option is unused. Update the task orchestration flow to skip this GET unless localAiDetectedModels is empty or the localAiServiceStatus/readiness or gateway endpoint has actually changed, and either remove or wire up the silent parameter in fetchLocalAiGatewayModels so the call sites reflect real behavior.
436-475: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLocalize the model-fetch error strings.
state.localAiModelsErroris set to hardcoded English ('Models endpoint unavailable','No models returned','Models endpoint timed out','Failed to fetch models','Models endpoint returned invalid JSON'). Every other user-facing message in this module routes throughshowTaskOrchestrationMessagewith i18n keys, so these will render untranslated when this field is displayed. Prefer storing an i18n key (or a translated string viathis.t) instead of raw text.🤖 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 `@web-ui/modules/app.methods.task-orchestration.mjs` around lines 436 - 475, The model-fetch path in `fetchLocalAiGatewayModels` is assigning hardcoded English strings directly to `state.localAiModelsError`, which bypasses the module’s i18n flow. Update the error handling in this method to store an i18n key or translate via `this.t` for every message (`Models endpoint unavailable`, `No models returned`, `Models endpoint timed out`, `Failed to fetch models`, `Models endpoint returned invalid JSON`, and the HTTP error case) so any UI displaying `localAiModelsError` renders localized text consistently.
🤖 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.
Nitpick comments:
In `@web-ui/modules/app.methods.task-orchestration.mjs`:
- Around line 657-662: The polling path in loadTaskOrchestrationOverview is
refetching local AI gateway models on every cycle via fetchLocalAiGatewayModels
even though the models rarely change, and the silent option is unused. Update
the task orchestration flow to skip this GET unless localAiDetectedModels is
empty or the localAiServiceStatus/readiness or gateway endpoint has actually
changed, and either remove or wire up the silent parameter in
fetchLocalAiGatewayModels so the call sites reflect real behavior.
- Around line 436-475: The model-fetch path in `fetchLocalAiGatewayModels` is
assigning hardcoded English strings directly to `state.localAiModelsError`,
which bypasses the module’s i18n flow. Update the error handling in this method
to store an i18n key or translate via `this.t` for every message (`Models
endpoint unavailable`, `No models returned`, `Models endpoint timed out`,
`Failed to fetch models`, `Models endpoint returned invalid JSON`, and the HTTP
error case) so any UI displaying `localAiModelsError` renders localized text
consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fd5d9690-c536-4af0-a7ef-2e941aaecf3d
📒 Files selected for processing (13)
tests/unit/config-tabs-ui.test.mjstests/unit/web-ui-behavior-parity.test.mjstests/unit/web-ui-logic.test.mjsweb-ui/app.jsweb-ui/modules/app.computed.main-tabs.mjsweb-ui/modules/app.methods.task-orchestration.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/panel-local-ai.htmlweb-ui/res/web-ui-render.precompiled.js
✅ Files skipped from review due to trivial changes (1)
- web-ui/modules/i18n/locales/zh.mjs
🚧 Files skipped from review as they are similar to previous changes (10)
- web-ui/app.js
- web-ui/partials/index/panel-local-ai.html
- tests/unit/web-ui-behavior-parity.test.mjs
- web-ui/modules/i18n/locales/en.mjs
- web-ui/modules/i18n/locales/vi.mjs
- tests/unit/config-tabs-ui.test.mjs
- web-ui/modules/i18n/locales/zh-tw.mjs
- web-ui/modules/i18n/locales/ja.mjs
- tests/unit/web-ui-logic.test.mjs
- web-ui/modules/app.computed.main-tabs.mjs
📜 Review details
🔇 Additional comments (6)
web-ui/modules/app.methods.task-orchestration.mjs (6)
33-58: LGTM!
81-97: LGTM!
379-419: LGTM!
482-527: LGTM!
539-612: LGTM!
1182-1238: LGTM!
awsl233777
changed the title
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web-ui/modules/app.methods.task-orchestration.mjs (1)
426-480: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUnguarded concurrent fetch can overwrite fresh model list with stale data.
fetchLocalAiGatewayModelshas no protection against overlapping invocations. It's fired-and-forgotten (void this.fetchLocalAiGatewayModels(...)) fromloadTaskOrchestrationOverviewon every poll cycle (every 4s persyncTaskOrchestrationPolling), while its own default timeout is 5000ms — so a newer call can start before an older one resolves. If the older (stale) request resolves after the newer one, itsstate.localAiDetectedModels/state.localAiModelsError/state.localAiModelsLoadingwrites silently clobber the fresher result. The file already has a proven fix for exactly this pattern via thedetailRequestTokencounter inloadTaskRunDetail(Lines 939-983); the same technique should be applied here.🔧 Proposed fix using a request token
async fetchLocalAiGatewayModels(options = {}) { const state = this.ensureTaskOrchestrationState(); const status = state.localAiServiceStatus && typeof state.localAiServiceStatus === 'object' ? state.localAiServiceStatus : {}; if (!status.ready) { state.localAiDetectedModels = []; state.localAiModelsError = ''; return []; } const modelsUrl = String(this.localAiGatewayModelsUrl || '').trim(); if (!modelsUrl || typeof fetchImpl !== 'function') { state.localAiDetectedModels = []; state.localAiModelsError = 'Models endpoint unavailable'; return []; } + const requestToken = (state.localAiModelsRequestToken || 0) + 1; + state.localAiModelsRequestToken = requestToken; state.localAiModelsLoading = true; state.localAiModelsError = ''; const timeoutMs = Number.isFinite(Number(options.timeoutMs)) ? Number(options.timeoutMs) : 5000; const controller = typeof AbortController === 'function' ? new AbortController() : null; const timeoutId = controller ? setTimeout(() => controller.abort(), timeoutMs) : 0; try { const res = await fetchImpl(modelsUrl, { method: 'GET', headers: { Accept: 'application/json' }, ...(controller ? { signal: controller.signal } : {}) }); const bodyText = typeof res.text === 'function' ? await res.text() : ''; let payload = {}; if (bodyText.trim()) { try { payload = JSON.parse(bodyText); } catch (_error) { throw new Error('Models endpoint returned invalid JSON'); } } if (!res.ok) { const message = payload && payload.error ? String(payload.error) : `Models endpoint returned HTTP ${res.status}`; throw new Error(message); } const models = normalizeLocalAiGatewayModelIds(payload); + if (state.localAiModelsRequestToken !== requestToken) return models; state.localAiDetectedModels = models; state.localAiModelsError = models.length ? '' : 'No models returned'; return models; } catch (error) { + if (state.localAiModelsRequestToken !== requestToken) return []; state.localAiDetectedModels = []; state.localAiModelsError = error && error.name === 'AbortError' ? 'Models endpoint timed out' : (error && error.message ? error.message : 'Failed to fetch models'); return []; } finally { if (timeoutId) clearTimeout(timeoutId); - state.localAiModelsLoading = false; + if (state.localAiModelsRequestToken === requestToken) { + state.localAiModelsLoading = false; + } } },Also applies to: 654-662
🤖 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 `@web-ui/modules/app.methods.task-orchestration.mjs` around lines 426 - 480, `fetchLocalAiGatewayModels` can race with overlapping poll-triggered calls and let stale responses overwrite newer model data. Add a request token/counter to this method, similar to `loadTaskRunDetail`’s `detailRequestToken`, and only apply `state.localAiDetectedModels`, `state.localAiModelsError`, and `state.localAiModelsLoading` updates if the current token is still the latest invocation. Use the unique symbols `fetchLocalAiGatewayModels`, `loadTaskOrchestrationOverview`, and `syncTaskOrchestrationPolling` to locate the polling path and ensure older in-flight requests are ignored.
🤖 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.
Outside diff comments:
In `@web-ui/modules/app.methods.task-orchestration.mjs`:
- Around line 426-480: `fetchLocalAiGatewayModels` can race with overlapping
poll-triggered calls and let stale responses overwrite newer model data. Add a
request token/counter to this method, similar to `loadTaskRunDetail`’s
`detailRequestToken`, and only apply `state.localAiDetectedModels`,
`state.localAiModelsError`, and `state.localAiModelsLoading` updates if the
current token is still the latest invocation. Use the unique symbols
`fetchLocalAiGatewayModels`, `loadTaskOrchestrationOverview`, and
`syncTaskOrchestrationPolling` to locate the polling path and ensure older
in-flight requests are ignored.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4a090863-3b67-4407-8a00-d699ea5d767c
📒 Files selected for processing (13)
cli.jscli/local-bridge.jstests/unit/config-tabs-ui.test.mjstests/unit/local-bridge-claude-target-api.test.mjstests/unit/web-ui-logic.test.mjsweb-ui/modules/app.methods.task-orchestration.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/panel-local-ai.htmlweb-ui/res/web-ui-render.precompiled.js
✅ Files skipped from review due to trivial changes (1)
- web-ui/modules/i18n/locales/ja.mjs
🚧 Files skipped from review as they are similar to previous changes (9)
- tests/unit/config-tabs-ui.test.mjs
- web-ui/partials/index/panel-local-ai.html
- web-ui/modules/i18n/locales/en.mjs
- web-ui/modules/i18n/locales/zh.mjs
- web-ui/modules/i18n/locales/vi.mjs
- cli/local-bridge.js
- cli.js
- tests/unit/web-ui-logic.test.mjs
- web-ui/modules/i18n/locales/zh-tw.mjs
📜 Review details
🔇 Additional comments (4)
tests/unit/local-bridge-claude-target-api.test.mjs (1)
203-305: LGTM!web-ui/modules/app.methods.task-orchestration.mjs (3)
439-439: 📐 Maintainability & Code Quality | ⚡ Quick winUntranslated error strings break i18n consistency.
fetchLocalAiGatewayModelssetsstate.localAiModelsErrorto plain English literals (e.g.'Models endpoint unavailable','No models returned','Models endpoint timed out'), while the rest of the file consistently routes user-facing text throught()/i18n keys (seeformatTaskNodeDependencies/formatTaskNodeLogs/formatTaskNodeOutputTextand all theshowTaskOrchestrationMessage(..., 'orchestration.toast...')calls). If this error string is rendered in the UI it will not be localized.Also applies to: 459-459, 464-464, 468-468, 472-474
286-378: LGTM!Also applies to: 482-612, 614-1181, 1217-1260
1182-1215: 🎯 Functional CorrectnessNo change needed.
taskOrchestrationWorkspacePathis already defined in the main-tabs computed state, andstartNewTaskWorkspaceSession()falls back to the current workspace anyway.abortis intentionally routed to the currently selected orchestration run; this webview contract only sends the messagetype, not a run/session id.> Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@web-ui/partials/index/panel-local-ai.html`:
- Around line 54-56: The request log in the panel-local-ai template mixes
translated and hardcoded labels, because the provider label uses
t('localAi.client.requestLog.provider') while the model label is still hardcoded
as “model:”. Update the span in the request log section to use the same i18n
pattern as the rest of this panel, adding or reusing the appropriate translation
key for the model label and referencing the existing template bindings around
item.provider, item.model, and item.at.
- Line 54: The request log provider label in the local AI panel is using the
initial provider selection, which can be wrong after failover. Update the
display logic in the panel-local-ai template to show the serving upstream first
by using item.upstreamProvider when present, and fall back to item.provider only
if it is missing. Keep the change focused on the provider span in the local AI
request log row.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9c026306-2d7d-49c5-820d-8915d6a1a9a7
📒 Files selected for processing (16)
cli.jscli/local-bridge.jstests/unit/config-tabs-ui.test.mjstests/unit/local-bridge-claude-target-api.test.mjstests/unit/web-ui-behavior-parity.test.mjstests/unit/web-ui-logic.test.mjsweb-ui/modules/app.computed.main-tabs.mjsweb-ui/modules/app.methods.task-orchestration.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/panel-local-ai.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/styles/local-ai-service.css
✅ Files skipped from review due to trivial changes (3)
- web-ui/modules/i18n/locales/en.mjs
- web-ui/modules/i18n/locales/vi.mjs
- web-ui/modules/i18n/locales/zh-tw.mjs
🚧 Files skipped from review as they are similar to previous changes (10)
- tests/unit/local-bridge-claude-target-api.test.mjs
- tests/unit/web-ui-behavior-parity.test.mjs
- tests/unit/config-tabs-ui.test.mjs
- web-ui/modules/i18n/locales/ja.mjs
- web-ui/styles/local-ai-service.css
- web-ui/modules/app.computed.main-tabs.mjs
- web-ui/modules/i18n/locales/zh.mjs
- cli.js
- tests/unit/web-ui-logic.test.mjs
- web-ui/modules/app.methods.task-orchestration.mjs
📜 Review details
🔇 Additional comments (5)
cli/local-bridge.js (4)
164-186: LGTM!
206-253: LGTM!
269-281: LGTM!
878-936: LGTM!web-ui/partials/index/panel-local-ai.html (1)
45-66: 🩺 Stability & AvailabilityNo action needed
localAiRequestLogandlocalAiGatewayModelsUrlare both defined as computed properties, andlocalAiRequestLogfalls back to[], so this block is safe as written.> Likely an incorrect or invalid review comment.
| <span class="local-ai-request-duration">{{ item.durationMs }}ms</span> | ||
| </div> | ||
| <div class="local-ai-request-log-meta"> | ||
| <span v-if="item.provider">{{ t('localAi.client.requestLog.provider') }}: {{ item.provider }}</span> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== panel-local-ai.html ==\n'
grep -n "item\.provider\|item\.upstreamProvider\|requestLog.provider" -n web-ui/partials/index/panel-local-ai.html || true
printf '\n== cli/local-bridge.js relevant lines ==\n'
grep -n "requestLog.provider\|upstreamProvider\|X-Codexmate-Upstream-Provider\|failover\|entry.name" cli/local-bridge.js || true
printf '\n== surrounding context in cli/local-bridge.js ==\n'
python3 - <<'PY'
from pathlib import Path
p = Path('cli/local-bridge.js')
text = p.read_text().splitlines()
for i, line in enumerate(text, 1):
if any(k in line for k in ['requestLog.provider', 'upstreamProvider', 'X-Codexmate-Upstream-Provider', 'failover', 'entry.name']):
start = max(1, i-8)
end = min(len(text), i+12)
print(f"\n--- lines {start}-{end} around line {i} ---")
for j in range(start, end+1):
print(f"{j:4d}: {text[j-1]}")
PY
printf '\n== surrounding context in web-ui/partials/index/panel-local-ai.html ==\n'
python3 - <<'PY'
from pathlib import Path
p = Path('web-ui/partials/index/panel-local-ai.html')
text = p.read_text().splitlines()
for i, line in enumerate(text, 1):
if 'item.provider' in line or 'item.upstreamProvider' in line:
start = max(1, i-6)
end = min(len(text), i+8)
print(f"\n--- lines {start}-{end} around line {i} ---")
for j in range(start, end+1):
print(f"{j:4d}: {text[j-1]}")
PYRepository: SakuraByteCore/codexmate
Length of output: 37325
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
files = [
'cli/local-bridge.js',
'web-ui/partials/index/panel-local-ai.html',
]
for file in files:
p = Path(file)
if not p.exists():
print(f'MISSING: {file}')
continue
print(f'\n== {file} ==')
text = p.read_text().splitlines()
keys = ['requestLog.provider', 'upstreamProvider', 'X-Codexmate-Upstream-Provider', 'item.provider', 'item.upstreamProvider', 'failover', 'entry.name']
for i, line in enumerate(text, 1):
if any(k in line for k in keys):
start = max(1, i-6)
end = min(len(text), i+10)
print(f'--- around line {i} ---')
for j in range(start, end+1):
print(f'{j:4d}: {text[j-1]}')
print()
PYRepository: SakuraByteCore/codexmate
Length of output: 29019
Display the serving upstream instead of the initial pick. item.provider is set before failover, so this can mislabel retries; use item.upstreamProvider || item.provider here.
💡 Proposed fix
- <span v-if="item.provider">{{ t('localAi.client.requestLog.provider') }}: {{ item.provider }}</span>
+ <span v-if="item.upstreamProvider || item.provider">{{ t('localAi.client.requestLog.provider') }}: {{ item.upstreamProvider || item.provider }}</span>📝 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.
| <span v-if="item.provider">{{ t('localAi.client.requestLog.provider') }}: {{ item.provider }}</span> | |
| <span v-if="item.upstreamProvider || item.provider">{{ t('localAi.client.requestLog.provider') }}: {{ item.upstreamProvider || item.provider }}</span> |
🤖 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 `@web-ui/partials/index/panel-local-ai.html` at line 54, The request log
provider label in the local AI panel is using the initial provider selection,
which can be wrong after failover. Update the display logic in the
panel-local-ai template to show the serving upstream first by using
item.upstreamProvider when present, and fall back to item.provider only if it is
missing. Keep the change focused on the provider span in the local AI request
log row.
| <span v-if="item.provider">{{ t('localAi.client.requestLog.provider') }}: {{ item.provider }}</span> | ||
| <span v-if="item.model">model: {{ item.model }}</span> | ||
| <span>{{ item.at }}</span> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Hardcoded model: label breaks i18n consistency.
Line 54 routes the provider label through t('localAi.client.requestLog.provider'), but Line 55 hardcodes model: in English. Route it through a translation key like the rest of the form, matching the i18n fixes applied earlier in this panel.
💡 Proposed fix
- <span v-if="item.model">model: {{ item.model }}</span>
+ <span v-if="item.model">{{ t('localAi.client.requestLog.model') }}: {{ item.model }}</span>📝 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.
| <span v-if="item.provider">{{ t('localAi.client.requestLog.provider') }}: {{ item.provider }}</span> | |
| <span v-if="item.model">model: {{ item.model }}</span> | |
| <span>{{ item.at }}</span> | |
| <span v-if="item.provider">{{ t('localAi.client.requestLog.provider') }}: {{ item.provider }}</span> | |
| <span v-if="item.model">{{ t('localAi.client.requestLog.model') }}: {{ item.model }}</span> | |
| <span>{{ item.at }}</span> |
🤖 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 `@web-ui/partials/index/panel-local-ai.html` around lines 54 - 56, The request
log in the panel-local-ai template mixes translated and hardcoded labels,
because the provider label uses t('localAi.client.requestLog.provider') while
the model label is still hardcoded as “model:”. Update the span in the request
log section to use the same i18n pattern as the rest of this panel, adding or
reusing the appropriate translation key for the model label and referencing the
existing template bindings around item.provider, item.model, and item.at.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cli/local-bridge.js (2)
764-770: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply configured model mapping to native
/responsesrequests.The passthrough loop maps
chat/completionstoattemptEntry.model, but this native Responses path forwards the client model unchanged. With an upstream model configured, Responses clients can still hit upstream “unknown model” failures.Proposed mapping fix
} - const responsesRequest = parsed.value; + const responsesRequest = parsed.value && typeof parsed.value === 'object' && !Array.isArray(parsed.value) + ? { ...parsed.value } + : parsed.value; + if (responsesRequest && typeof responsesRequest === 'object' && !Array.isArray(responsesRequest)) { + if (entry.model) responsesRequest.model = entry.model; + if (responsesRequest.temperature === undefined && entry.temperature !== null && entry.temperature !== undefined) { + responsesRequest.temperature = entry.temperature; + } + } const wantsSse = !!(responsesRequest && responsesRequest.stream);🤖 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 `@cli/local-bridge.js` around lines 764 - 770, The native Responses passthrough in local-bridge.js is forwarding the client-supplied model unchanged, unlike the chat/completions path that applies the configured upstream model mapping. Update the /responses handling around parsed.value and proxyRequestJson so the request model is rewritten through the same mapping logic used by the passthrough loop (for example via attemptEntry.model or the shared model-mapping helper) before sending to the upstream responses endpoint. Keep the change localized to the responses request construction so both streaming and non-streaming Requests use the mapped model consistently.
879-950: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftSplit
audio/*andimages/*out of the JSON proxy. This branch routesaudio/*,images/*, andchat/completionsthroughproxyRequestJson(), so it parses every body as JSON and always writes JSON back. That breaks multipart uploads and buffers/mislabells streamed or binary responses. Narrow the matcher to JSON-only endpoints, or add raw/streaming passthrough that preserves content type.🤖 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 `@cli/local-bridge.js` around lines 879 - 950, The proxy branch in local-bridge should not treat audio/* and images/* the same as JSON endpoints, because it currently forces request parsing through proxyRequestJson() and always responds with JSON. Update the matcher around the failover/proxy path to exclude binary or multipart routes and handle audio/images with raw or streamed passthrough instead, preserving the upstream content type and response bytes. Keep the JSON-specific shaping only for chat/completions and similar JSON-only paths, using the existing helpers like proxyRequestJson(), joinApiUrl(), and shouldFailover() to locate the logic.
🤖 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 `@cli/local-bridge.js`:
- Around line 693-695: The route check in the local bridge path matcher is too
permissive because `startsWith('v1')` accepts longer prefixes like `v10` and
`v1evil`. Update the `pathname`/`suffix` validation in the local bridge routing
logic so `v1` is matched only as a full path segment in the same place where
`isCodexLocalBridgeSuffix(suffix)` is used, and reject any non-segment matches
before forwarding upstream.
- Around line 190-193: The redactText helper in local-bridge.js only masks a
limited set of secret field names, so common variants like access_token,
refresh_token, id_token, and client_secret can still appear in bodyPreview.
Expand the redaction logic in redactText to cover these additional JSON key
names before preview logging, and keep the existing bearer/sk- masking intact so
all logged previews consistently hide token-like values.
---
Outside diff comments:
In `@cli/local-bridge.js`:
- Around line 764-770: The native Responses passthrough in local-bridge.js is
forwarding the client-supplied model unchanged, unlike the chat/completions path
that applies the configured upstream model mapping. Update the /responses
handling around parsed.value and proxyRequestJson so the request model is
rewritten through the same mapping logic used by the passthrough loop (for
example via attemptEntry.model or the shared model-mapping helper) before
sending to the upstream responses endpoint. Keep the change localized to the
responses request construction so both streaming and non-streaming Requests use
the mapped model consistently.
- Around line 879-950: The proxy branch in local-bridge should not treat audio/*
and images/* the same as JSON endpoints, because it currently forces request
parsing through proxyRequestJson() and always responds with JSON. Update the
matcher around the failover/proxy path to exclude binary or multipart routes and
handle audio/images with raw or streamed passthrough instead, preserving the
upstream content type and response bytes. Keep the JSON-specific shaping only
for chat/completions and similar JSON-only paths, using the existing helpers
like proxyRequestJson(), joinApiUrl(), and shouldFailover() to locate the logic.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cab2f308-dcfb-4f32-b39a-eec415474774
📒 Files selected for processing (2)
cli/local-bridge.jstests/unit/local-bridge-claude-target-api.test.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/local-bridge-claude-target-api.test.mjs
| const redactText = (value) => String(value || '') | ||
| .replace(/("(?:api[_-]?key|token|authorization|password|secret)"\s*:\s*")[^"]*(")/ig, '$1***$2') | ||
| .replace(/(Bearer\s+)[A-Za-z0-9._~+\-/=]+/ig, '$1***') | ||
| .replace(/\bsk-[A-Za-z0-9._-]+\b/g, 'sk-***'); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redact common token key variants before logging previews.
access_token, refresh_token, id_token, and client_secret are not matched here, so they can leak into bodyPreview.
Proposed redaction hardening
const redactText = (value) => String(value || '')
- .replace(/("(?:api[_-]?key|token|authorization|password|secret)"\s*:\s*")[^"]*(")/ig, '$1***$2')
+ .replace(/("(?:api[_-]?key|x[_-]?api[_-]?key|authorization|password|secret|client[_-]?secret|access[_-]?token|refresh[_-]?token|id[_-]?token|session[_-]?token|auth[_-]?token)"\s*:\s*")[^"]*(")/ig, '$1***$2')
.replace(/(Bearer\s+)[A-Za-z0-9._~+\-/=]+/ig, '$1***')
.replace(/\bsk-[A-Za-z0-9._-]+\b/g, 'sk-***');📝 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.
| const redactText = (value) => String(value || '') | |
| .replace(/("(?:api[_-]?key|token|authorization|password|secret)"\s*:\s*")[^"]*(")/ig, '$1***$2') | |
| .replace(/(Bearer\s+)[A-Za-z0-9._~+\-/=]+/ig, '$1***') | |
| .replace(/\bsk-[A-Za-z0-9._-]+\b/g, 'sk-***'); | |
| const redactText = (value) => String(value || '') | |
| .replace(/("(?:api[_-]?key|x[_-]?api[_-]?key|authorization|password|secret|client[_-]?secret|access[_-]?token|refresh[_-]?token|id[_-]?token|session[_-]?token|auth[_-]?token)"\s*:\s*")[^"]*(")/ig, '$1***$2') | |
| .replace(/(Bearer\s+)[A-Za-z0-9._~+\-/=]+/ig, '$1***') | |
| .replace(/\bsk-[A-Za-z0-9._-]+\b/g, 'sk-***'); |
🤖 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 `@cli/local-bridge.js` around lines 190 - 193, The redactText helper in
local-bridge.js only masks a limited set of secret field names, so common
variants like access_token, refresh_token, id_token, and client_secret can still
appear in bodyPreview. Expand the redaction logic in redactText to cover these
additional JSON key names before preview logging, and keep the existing
bearer/sk- masking intact so all logged previews consistently hide token-like
values.
79a51ff to
785a422
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web-ui/session-helpers.mjs (1)
82-91: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep the orchestration tab checks aligned with the new
local-aialias.switchMainTab()now normalizes'orchestration'to'local-ai', soenteringOrchestrationTabandnextTab !== 'orchestration'no longer fire during normal tab switches. That leavesincludeDetailoff for the renamed tab and stops orchestration polling as soon as you switch into it; the same stale'orchestration'guard also exists inweb-ui/modules/app.methods.task-orchestration.mjs:1207.🤖 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 `@web-ui/session-helpers.mjs` around lines 82 - 91, Align the orchestration tab logic with the new local-ai alias in switchMainTab() and the task-orchestration polling code. Update the tab checks so the renamed tab is treated as active when nextTab is normalized from orchestration to local-ai, and replace any remaining nextTab !== 'orchestration' style guard in the orchestration methods (including the one in app.methods.task-orchestration.mjs) so includeDetail and polling continue to work for the renamed tab.cli/local-bridge.js (1)
799-868: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftUse the same provider failover policy for
/responses.This branch retries only the initially selected provider; network errors or failed chat-completions fallback return immediately instead of trying the next healthy upstream.
🤖 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 `@cli/local-bridge.js` around lines 799 - 868, The /responses path in local-bridge.js is only retrying the initially selected upstream, so provider failover is not applied consistently. Update the logic around retryLocalBridgeUpstreamRequest, shouldFallbackFromUpstreamResponses, and the chat/completions fallback so that both network failures and fallback conversion failures continue through the same provider selection policy used elsewhere instead of returning immediately. Keep the fix localized to the upstreamResponsesResult handling and the subsequent chatResult path, and ensure the next healthy provider is attempted before surfacing an error.
🤖 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 `@cli/local-bridge.js`:
- Around line 307-309: The upstream error extraction in local-bridge.js only
reads parsed.error as a string, so OpenAI-style responses with error.message end
up logged as empty. Update the error parsing logic around the JSON.parse
handling to detect object-shaped error payloads, especially
parsed.error.message, and fall back to the existing string path when needed.
Keep the extracted message trimmed to the same length limit before assigning it
to item.error, and preserve the current try/catch behavior in the request
logging flow.
- Around line 239-242: The request summary built in the payload handling logic
can leak secrets because it appends raw `messages[].content` and `input` text
even when `bodyPreview` is redacted. Update the summary construction in the code
path that uses `payload.messages` and `payload.input` to redact or omit
sensitive snippets before pushing them into `summaryParts`, preserving only safe
metadata or a sanitized preview.
- Around line 63-72: The route predicate in isCodexLocalBridgeSuffix is too
permissive for embeddings and can match invalid suffixes like embeddingsevil.
Tighten the embeddings check so it only accepts the exact embeddings endpoint or
embeddings followed by a valid path separator/query boundary, matching the
existing style used for responses, chat/completions, and models. Keep the fix
localized to isCodexLocalBridgeSuffix and adjust the embeddings branch
accordingly.
- Around line 796-801: The /responses forwarding path in local-bridge.js is
passing responsesRequest through unchanged, so the configured upstream model
mapping is never applied. Update the branch around proxyRequestJson and
retryLocalBridgeUpstreamRequest to transform the parsed request before sending
it upstream, using the same model-mapping logic already used for other request
types so a configured upstream model overrides the client model. Locate the fix
in the responses handling block that builds upstreamResponsesUrl and sends body:
responsesRequest.
In `@web-ui/styles/local-ai-service.css`:
- Around line 226-228: The grid-column rule is targeting the wrong selector, so
it never matches the actual request section element. Update the selector in
local-ai-service.css from the data-component-style name to the real class used
by panel-local-requests.html, namely local-ai-request-section, so the styling
applies to the section rendered by that component.
- Around line 285-289: The body-preview styling is targeting a nested code
element that does not exist for the local request detail preview, so the
wrapping rules never apply. Update the selector in local-ai-service.css to
target the actual .local-ai-request-body element used by the preview inside
.local-ai-request-detail, and keep the existing block, pre-wrap, and full-width
behavior there so the body text wraps correctly instead of overflowing.
---
Outside diff comments:
In `@cli/local-bridge.js`:
- Around line 799-868: The /responses path in local-bridge.js is only retrying
the initially selected upstream, so provider failover is not applied
consistently. Update the logic around retryLocalBridgeUpstreamRequest,
shouldFallbackFromUpstreamResponses, and the chat/completions fallback so that
both network failures and fallback conversion failures continue through the same
provider selection policy used elsewhere instead of returning immediately. Keep
the fix localized to the upstreamResponsesResult handling and the subsequent
chatResult path, and ensure the next healthy provider is attempted before
surfacing an error.
In `@web-ui/session-helpers.mjs`:
- Around line 82-91: Align the orchestration tab logic with the new local-ai
alias in switchMainTab() and the task-orchestration polling code. Update the tab
checks so the renamed tab is treated as active when nextTab is normalized from
orchestration to local-ai, and replace any remaining nextTab !== 'orchestration'
style guard in the orchestration methods (including the one in
app.methods.task-orchestration.mjs) so includeDetail and polling continue to
work for the renamed tab.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 79008214-b605-4fd8-ad88-de0a89f580ec
📒 Files selected for processing (22)
cli.jscli/local-bridge.jstests/unit/config-tabs-ui.test.mjstests/unit/local-bridge-claude-target-api.test.mjstests/unit/web-ui-behavior-parity.test.mjstests/unit/web-ui-logic.test.mjsweb-ui/app.jsweb-ui/index.htmlweb-ui/modules/app.computed.main-tabs.mjsweb-ui/modules/app.methods.navigation.mjsweb-ui/modules/app.methods.task-orchestration.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/layout-header.htmlweb-ui/partials/index/panel-local-ai.htmlweb-ui/partials/index/panel-local-requests.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/session-helpers.mjsweb-ui/styles/local-ai-service.css
✅ Files skipped from review due to trivial changes (2)
- web-ui/index.html
- web-ui/modules/i18n/locales/ja.mjs
🚧 Files skipped from review as they are similar to previous changes (14)
- web-ui/app.js
- tests/unit/web-ui-behavior-parity.test.mjs
- web-ui/modules/app.methods.navigation.mjs
- tests/unit/config-tabs-ui.test.mjs
- tests/unit/local-bridge-claude-target-api.test.mjs
- web-ui/modules/i18n/locales/zh-tw.mjs
- web-ui/modules/i18n/locales/en.mjs
- web-ui/partials/index/layout-header.html
- cli.js
- web-ui/modules/i18n/locales/vi.mjs
- web-ui/modules/app.computed.main-tabs.mjs
- web-ui/modules/i18n/locales/zh.mjs
- tests/unit/web-ui-logic.test.mjs
- web-ui/modules/app.methods.task-orchestration.mjs
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.0)
cli/local-bridge.js
[warning] 156-156: Avoid using the initial state variable in setState
Context: setTimeout(resolve, delay)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🪛 HTMLHint (1.9.2)
web-ui/partials/index/panel-local-requests.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🔇 Additional comments (11)
web-ui/partials/index/panel-local-requests.html (5)
21-21: 📐 Maintainability & Code Quality | ⚡ Quick winUntranslated
aria-label.
aria-label="Local service request configuration"is hardcoded English while the rest of this panel routes copy throught(...). This is the same concern raised previously on the sibling gateway-summary markup (now relocated here).💡 Proposed fix
- <section class="local-ai-brief-row" aria-label="Local service request configuration"> + <section class="local-ai-brief-row" :aria-label="t('localAi.requests.briefAriaLabel')">
76-76: 🎯 Functional Correctness | ⚡ Quick winProvider label should show the serving upstream, not the initial pick.
item.provider(Line 76) andlocalAiSelectedRequestLogItem.provider(Line 96) reflect the provider chosen before failover. If failover/retry occurs, this mislabels which upstream actually served the request. This is the same issue previously flagged on the (now relocated) request-log markup — preferitem.upstreamProvider || item.provider.💡 Proposed fix
- <span v-if="item.provider">{{ t('localAi.client.requestLog.provider') }}: {{ item.provider }}</span> + <span v-if="item.upstreamProvider || item.provider">{{ t('localAi.client.requestLog.provider') }}: {{ item.upstreamProvider || item.provider }}</span>- <dd>{{ localAiSelectedRequestLogItem.provider || '-' }}</dd> + <dd>{{ localAiSelectedRequestLogItem.upstreamProvider || localAiSelectedRequestLogItem.provider || '-' }}</dd>Also applies to: 96-96
77-77: 📐 Maintainability & Code Quality | ⚡ Quick winHardcoded
model:label breaks i18n consistency.Line 76 routes the provider label through
t('localAi.client.requestLog.provider'), but Line 77 hardcodesmodel:in English. Same unresolved concern previously flagged, now relocated to this file.💡 Proposed fix
- <span v-if="item.model">model: {{ item.model }}</span> + <span v-if="item.model">{{ t('localAi.client.requestLog.model') }}: {{ item.model }}</span>
1-1: Doctype hint is a false positive.HTMLHint's
doctype-firstcomplaint applies since this is a partial fragment injected into a parent document, not a standalone HTML page.Source: Linters/SAST tools
55-58: LGTM!Also applies to: 60-118
web-ui/partials/index/panel-local-ai.html (2)
63-76: LGTM!
125-129: LGTM!web-ui/styles/local-ai-service.css (1)
230-284: LGTM!Also applies to: 349-354, 416-429, 474-582
cli/local-bridge.js (3)
222-225: Duplicate: broaden token-field redaction variants.This still misses common JSON secret fields such as
access_token,refresh_token,id_token, andclient_secret.
725-727: Duplicate: requirev1to be a full path segment.
startsWith('v1')still accepts/bridge/local/v10/...and/bridge/local/v1evil.
38-38: LGTM!Also applies to: 53-61, 129-167, 259-306, 310-317, 332-345, 760-763, 779-784, 828-836, 892-924, 943-985
| function isCodexLocalBridgeSuffix(suffix) { | ||
| const normalized = normalizeCodexLocalSuffix(`/bridge/local/${suffix || ''}`); | ||
| return !normalized | ||
| || normalized === 'responses' | ||
| || normalized === 'chat/completions' | ||
| || normalized === 'models' | ||
| || normalized.startsWith('models?') | ||
| || normalized.startsWith('images/') | ||
| || normalized.startsWith('audio/') | ||
| || normalized.startsWith('embeddings'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Require embeddings to match as an endpoint.
normalized.startsWith('embeddings') accepts invalid paths like /bridge/local/embeddingsevil and forwards them upstream.
Proposed route predicate tightening
function isCodexLocalBridgeSuffix(suffix) {
const normalized = normalizeCodexLocalSuffix(`/bridge/local/${suffix || ''}`);
return !normalized
|| normalized === 'responses'
|| normalized === 'chat/completions'
|| normalized === 'models'
- || normalized.startsWith('models?')
|| normalized.startsWith('images/')
|| normalized.startsWith('audio/')
- || normalized.startsWith('embeddings');
+ || normalized === 'embeddings';
}🤖 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 `@cli/local-bridge.js` around lines 63 - 72, The route predicate in
isCodexLocalBridgeSuffix is too permissive for embeddings and can match invalid
suffixes like embeddingsevil. Tighten the embeddings check so it only accepts
the exact embeddings endpoint or embeddings followed by a valid path
separator/query boundary, matching the existing style used for responses,
chat/completions, and models. Keep the fix localized to isCodexLocalBridgeSuffix
and adjust the embeddings branch accordingly.
| const parsed = JSON.parse(Buffer.isBuffer(chunk) ? chunk.toString('utf-8') : String(chunk)); | ||
| item.error = typeof parsed.error === 'string' ? parsed.error.slice(0, 240) : ''; | ||
| } catch (_) { } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Extract object-shaped upstream errors too.
OpenAI-style errors usually use error.message, so the request log can show an empty error for failed upstream calls.
Proposed error extraction fix
const parsed = JSON.parse(Buffer.isBuffer(chunk) ? chunk.toString('utf-8') : String(chunk));
- item.error = typeof parsed.error === 'string' ? parsed.error.slice(0, 240) : '';
+ const errorValue = parsed && parsed.error;
+ item.error = typeof errorValue === 'string'
+ ? errorValue.slice(0, 240)
+ : String((errorValue && (errorValue.message || errorValue.type)) || '').slice(0, 240);📝 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.
| const parsed = JSON.parse(Buffer.isBuffer(chunk) ? chunk.toString('utf-8') : String(chunk)); | |
| item.error = typeof parsed.error === 'string' ? parsed.error.slice(0, 240) : ''; | |
| } catch (_) { } | |
| const parsed = JSON.parse(Buffer.isBuffer(chunk) ? chunk.toString('utf-8') : String(chunk)); | |
| const errorValue = parsed && parsed.error; | |
| item.error = typeof errorValue === 'string' | |
| ? errorValue.slice(0, 240) | |
| : String((errorValue && (errorValue.message || errorValue.type)) || '').slice(0, 240); | |
| } catch (_) { } |
🤖 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 `@cli/local-bridge.js` around lines 307 - 309, The upstream error extraction in
local-bridge.js only reads parsed.error as a string, so OpenAI-style responses
with error.message end up logged as empty. Update the error parsing logic around
the JSON.parse handling to detect object-shaped error payloads, especially
parsed.error.message, and fall back to the existing string path when needed.
Keep the extracted message trimmed to the same length limit before assigning it
to item.error, and preserve the current try/catch behavior in the request
logging flow.
| const responsesRequest = parsed.value; | ||
| const wantsSse = !!(responsesRequest && responsesRequest.stream); | ||
| const upstreamResponsesUrl = joinApiUrl(upstreamBase, 'responses'); | ||
| const upstreamResponsesResult = await retryTransientRequest(() => proxyRequestJson(upstreamResponsesUrl, { | ||
| const upstreamResponsesResult = await retryLocalBridgeUpstreamRequest(entry, () => proxyRequestJson(upstreamResponsesUrl, { | ||
| method: 'POST', | ||
| body: responsesRequest, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Apply configured model mapping to /responses requests.
A filled upstream model should override the client model, but this branch forwards/converts the original responsesRequest unchanged.
Proposed mapping fix
- const responsesRequest = parsed.value;
+ let responsesRequest = parsed.value;
+ if (responsesRequest && typeof responsesRequest === 'object' && !Array.isArray(responsesRequest)) {
+ responsesRequest = { ...responsesRequest };
+ if (entry.model) responsesRequest.model = entry.model;
+ if (responsesRequest.temperature === undefined && entry.temperature !== null && entry.temperature !== undefined) {
+ responsesRequest.temperature = entry.temperature;
+ }
+ }
const wantsSse = !!(responsesRequest && responsesRequest.stream);📝 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.
| const responsesRequest = parsed.value; | |
| const wantsSse = !!(responsesRequest && responsesRequest.stream); | |
| const upstreamResponsesUrl = joinApiUrl(upstreamBase, 'responses'); | |
| const upstreamResponsesResult = await retryTransientRequest(() => proxyRequestJson(upstreamResponsesUrl, { | |
| const upstreamResponsesResult = await retryLocalBridgeUpstreamRequest(entry, () => proxyRequestJson(upstreamResponsesUrl, { | |
| method: 'POST', | |
| body: responsesRequest, | |
| let responsesRequest = parsed.value; | |
| if (responsesRequest && typeof responsesRequest === 'object' && !Array.isArray(responsesRequest)) { | |
| responsesRequest = { ...responsesRequest }; | |
| if (entry.model) responsesRequest.model = entry.model; | |
| if (responsesRequest.temperature === undefined && entry.temperature !== null && entry.temperature !== undefined) { | |
| responsesRequest.temperature = entry.temperature; | |
| } | |
| } | |
| const wantsSse = !!(responsesRequest && responsesRequest.stream); | |
| const upstreamResponsesUrl = joinApiUrl(upstreamBase, 'responses'); | |
| const upstreamResponsesResult = await retryLocalBridgeUpstreamRequest(entry, () => proxyRequestJson(upstreamResponsesUrl, { | |
| method: 'POST', | |
| body: responsesRequest, |
🤖 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 `@cli/local-bridge.js` around lines 796 - 801, The /responses forwarding path
in local-bridge.js is passing responsesRequest through unchanged, so the
configured upstream model mapping is never applied. Update the branch around
proxyRequestJson and retryLocalBridgeUpstreamRequest to transform the parsed
request before sending it upstream, using the same model-mapping logic already
used for other request types so a configured upstream model overrides the client
model. Locate the fix in the responses handling block that builds
upstreamResponsesUrl and sends body: responsesRequest.
| .local-ai-request-info { | ||
| grid-column: 1 / -1; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n "local-ai-request-info|local-ai-request-section|data-component=\"local-ai-request-info\"|local-ai-calm-console" web-ui/styles/local-ai-service.css web-ui -SRepository: SakuraByteCore/codexmate
Length of output: 1484
Use the actual section class here
.local-ai-request-info is only a data-component value on panel-local-requests.html; the element’s class is local-ai-request-section, so this grid-column: 1 / -1 rule never applies. Change the selector to .local-ai-request-section.
🤖 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 `@web-ui/styles/local-ai-service.css` around lines 226 - 228, The grid-column
rule is targeting the wrong selector, so it never matches the actual request
section element. Update the selector in local-ai-service.css from the
data-component-style name to the real class used by panel-local-requests.html,
namely local-ai-request-section, so the styling applies to the section rendered
by that component.
| .local-ai-brief-item .local-ai-request-body code { | ||
| display: block; | ||
| white-space: pre-wrap; | ||
| width: 100%; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files and inspect the exact surrounding lines.
git ls-files 'web-ui/styles/local-ai-service.css' 'web-ui/**/*.html' | sed -n '1,200p'
printf '\n--- local-ai-service.css (around lines 260-310) ---\n'
sed -n '260,310p' web-ui/styles/local-ai-service.css
printf '\n--- panel-local-requests.html: selector references ---\n'
rg -n 'local-ai-request-body|local-ai-brief-item|local-ai-request-detail|local-ai-request' web-ui -g '!*node_modules*'
printf '\n--- panel-local-requests.html (around matched lines) ---\n'
sed -n '1,260p' web-ui/panel-local-requests.htmlRepository: SakuraByteCore/codexmate
Length of output: 11989
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- panel-local-requests.html (lines 80-120) ---\n'
sed -n '80,120p' web-ui/partials/index/panel-local-requests.html
printf '\n--- local-ai-service.css (lines 1-120) ---\n'
sed -n '1,120p' web-ui/styles/local-ai-service.css
printf '\n--- local-ai-service.css (lines 520-610) ---\n'
sed -n '520,610p' web-ui/styles/local-ai-service.cssRepository: SakuraByteCore/codexmate
Length of output: 7798
Scope the body-preview rule to .local-ai-request-body
In web-ui/partials/index/panel-local-requests.html, the body preview is rendered as <pre class="local-ai-request-body"> inside .local-ai-request-detail, so .local-ai-brief-item .local-ai-request-body code never matches that element. The preview keeps the default <pre> whitespace behavior and can overflow instead of wrapping.
Proposed fix
-.local-ai-brief-item .local-ai-request-body code {
+.local-ai-request-body code {
display: block;
white-space: pre-wrap;
width: 100%;
}📝 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.
| .local-ai-brief-item .local-ai-request-body code { | |
| display: block; | |
| white-space: pre-wrap; | |
| width: 100%; | |
| } | |
| .local-ai-request-body code { | |
| display: block; | |
| white-space: pre-wrap; | |
| width: 100%; | |
| } |
🤖 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 `@web-ui/styles/local-ai-service.css` around lines 285 - 289, The body-preview
styling is targeting a nested code element that does not exist for the local
request detail preview, so the wrapping rules never apply. Update the selector
in local-ai-service.css to target the actual .local-ai-request-body element used
by the preview inside .local-ai-request-detail, and keep the existing block,
pre-wrap, and full-width behavior there so the body text wraps correctly instead
of overflowing.
785a422 to
c94f00d
Compare
awsl233777
changed the title
awsl233777
force-pushed
the
feat/local-ai-service-provider-console
branch
from
c94f00d to
38a5aec
Compare
awsl233777
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web-ui/modules/app.methods.task-orchestration.mjs (1)
663-670: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winBackground/poll refresh unconditionally wipes fetched local AI models.
state.localAiDetectedModels/localAiModelsErrorare reset to empty on every successfulloadTaskOrchestrationOverview()call, including silent background polling and the "请求" page's explicit refresh button. UnlikeproviderSetupDraft/webviewBridgeDraft(which merge/preserve current values viasyncTaskProviderSelectionFromStatus/syncTaskWebviewBridgeDraft, addressing the earlier "background refresh overwrites in-progress edits" issue), this field has no such guard — a user who fetched models viafetchLocalAiGatewayModels()and is mid-selection on the provider form will silently lose the populated dropdown as soon as any overview refresh fires.♻️ Suggested fix: only clear when provider identity actually changes
- state.localAiDetectedModels = []; - state.localAiModelsError = ''; + const previousProviderKey = state.localAiServiceStatus && state.localAiServiceStatus.selectedProvider; + const nextProviderKey = res && res.localAiServiceStatus && res.localAiServiceStatus.selectedProvider; + if (previousProviderKey !== nextProviderKey) { + state.localAiDetectedModels = []; + state.localAiModelsError = ''; + }Also applies to: 690-698
🤖 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 `@web-ui/modules/app.methods.task-orchestration.mjs` around lines 663 - 670, The overview refresh logic is wiping fetched local AI models on every successful load, which can clear the provider form during background polling or manual refresh. Update loadTaskOrchestrationOverview() so state.localAiDetectedModels and state.localAiModelsError are only reset when the active provider identity actually changes, using the same kind of preservation pattern already used by syncTaskProviderSelectionFromStatus and syncTaskWebviewBridgeDraft. Keep fetchLocalAiGatewayModels() results intact across ordinary refreshes, and only clear them when a real provider switch makes the cached models invalid.web-ui/session-helpers.mjs (1)
82-91: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
local-ai/local-requestsnever restart orchestration polling
switchMainTab('orchestration')now lands onlocal-ai, soenteringOrchestrationTabis dead here andincludeDetailis alwaysfalseon this path. More importantly,syncTaskOrchestrationPolling()still returns unlessthis.mainTab === 'orchestration', so any timer stopped on tab change never comes back while viewinglocal-aiorlocal-requests. Update the polling gate to the new tab set.🤖 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 `@web-ui/session-helpers.mjs` around lines 82 - 91, The polling logic in switchMainTab now maps orchestration to local-ai, but syncTaskOrchestrationPolling still gates on this.mainTab === 'orchestration', so polling never restarts from local-ai or local-requests. Update the tab checks in session-helpers.mjs to use the new tab set consistently, remove or replace the now-dead enteringOrchestrationTab branch, and make sure the restart condition in syncTaskOrchestrationPolling matches local-ai/local-requests so timers resume after tab changes.
🧹 Nitpick comments (2)
web-ui/partials/index/panel-local-requests.html (2)
43-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated status-pill class logic.
The
item.status >= 200 && item.status < 400 ? 'success' : 'warn'ternary is duplicated between the list row (Line 43) and the detail header (Line 62). Consider extracting a small helper (e.g.,localAiRequestStatusTone(status)) alongside the other request-log helpers inapp.methods.task-orchestration.mjs.Also applies to: 62-62
🤖 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 `@web-ui/partials/index/panel-local-requests.html` at line 43, The status-to-tone ternary is duplicated in the request log UI, so extract it into a shared helper instead of repeating it. Add a small function like localAiRequestStatusTone(status) alongside the other request-log helpers in app.methods.task-orchestration.mjs, then update both the list row and detail header bindings in the local requests panel to call that helper for the status-pill class.
33-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
role="listitem"on native<button>may conflict with implicit button semantics.Each list row is a
<button>with an explicitrole="listitem". Per ARIA guidance, an explicit role generally overrides an element's implicit role, so assistive tech may announce these as list items rather than actionable buttons, even though click/keyboard activation still works via the native<button>. Consider droppingrole="listitem"(since<button>inside arole="list"container doesn't require it) or restructuring so the list semantics live on a wrapping non-interactive element.🤖 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 `@web-ui/partials/index/panel-local-requests.html` around lines 33 - 53, The local request rows in panel-local-requests.html use a native button element with an explicit role="listitem", which overrides the button’s implicit semantics. Update the button rendered by localAiRequestLog to remove the conflicting role, or move the list semantics to a non-interactive wrapper if true list structure is needed. Keep the existing button behavior and bindings in localAiRequestLogItemKey, selectLocalAiRequestLogItem, and the current active/status classes unchanged.
🤖 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 `@web-ui/partials/index/panel-local-requests.html`:
- Around line 40-51: The request log template in panel-local-requests should
avoid rendering raw undefined values and should localize the model label. Update
the local-ai request display logic so the `item.durationMs` field in the request
header/footer falls back to a placeholder like `-` when missing, and replace the
hardcoded `model:` text with the appropriate `t('localAi.client.requestLog...')`
translation key used alongside `localAi.client.requestLog.provider`. Apply the
same fix wherever the request log item is rendered so
`local-ai-request-duration` and the `item.model` label behave consistently.
---
Outside diff comments:
In `@web-ui/modules/app.methods.task-orchestration.mjs`:
- Around line 663-670: The overview refresh logic is wiping fetched local AI
models on every successful load, which can clear the provider form during
background polling or manual refresh. Update loadTaskOrchestrationOverview() so
state.localAiDetectedModels and state.localAiModelsError are only reset when the
active provider identity actually changes, using the same kind of preservation
pattern already used by syncTaskProviderSelectionFromStatus and
syncTaskWebviewBridgeDraft. Keep fetchLocalAiGatewayModels() results intact
across ordinary refreshes, and only clear them when a real provider switch makes
the cached models invalid.
In `@web-ui/session-helpers.mjs`:
- Around line 82-91: The polling logic in switchMainTab now maps orchestration
to local-ai, but syncTaskOrchestrationPolling still gates on this.mainTab ===
'orchestration', so polling never restarts from local-ai or local-requests.
Update the tab checks in session-helpers.mjs to use the new tab set
consistently, remove or replace the now-dead enteringOrchestrationTab branch,
and make sure the restart condition in syncTaskOrchestrationPolling matches
local-ai/local-requests so timers resume after tab changes.
---
Nitpick comments:
In `@web-ui/partials/index/panel-local-requests.html`:
- Line 43: The status-to-tone ternary is duplicated in the request log UI, so
extract it into a shared helper instead of repeating it. Add a small function
like localAiRequestStatusTone(status) alongside the other request-log helpers in
app.methods.task-orchestration.mjs, then update both the list row and detail
header bindings in the local requests panel to call that helper for the
status-pill class.
- Around line 33-53: The local request rows in panel-local-requests.html use a
native button element with an explicit role="listitem", which overrides the
button’s implicit semantics. Update the button rendered by localAiRequestLog to
remove the conflicting role, or move the list semantics to a non-interactive
wrapper if true list structure is needed. Keep the existing button behavior and
bindings in localAiRequestLogItemKey, selectLocalAiRequestLogItem, and the
current active/status classes unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 394b0202-a3f1-4a47-bf7c-7d9cc752fc47
📒 Files selected for processing (23)
cli.jscli/local-bridge.jstests/unit/config-tabs-ui.test.mjstests/unit/local-bridge-claude-target-api.test.mjstests/unit/session-tab-switch-performance.test.mjstests/unit/web-ui-behavior-parity.test.mjstests/unit/web-ui-logic.test.mjsweb-ui/app.jsweb-ui/index.htmlweb-ui/modules/app.computed.main-tabs.mjsweb-ui/modules/app.methods.navigation.mjsweb-ui/modules/app.methods.task-orchestration.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/layout-header.htmlweb-ui/partials/index/panel-local-ai.htmlweb-ui/partials/index/panel-local-requests.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/session-helpers.mjsweb-ui/styles/local-ai-service.css
🚧 Files skipped from review as they are similar to previous changes (17)
- web-ui/index.html
- web-ui/app.js
- web-ui/partials/index/layout-header.html
- web-ui/partials/index/panel-local-ai.html
- web-ui/modules/app.methods.navigation.mjs
- tests/unit/config-tabs-ui.test.mjs
- tests/unit/web-ui-behavior-parity.test.mjs
- tests/unit/local-bridge-claude-target-api.test.mjs
- web-ui/modules/i18n/locales/en.mjs
- web-ui/modules/i18n/locales/zh.mjs
- web-ui/styles/local-ai-service.css
- web-ui/modules/i18n/locales/ja.mjs
- web-ui/modules/i18n/locales/vi.mjs
- web-ui/modules/app.computed.main-tabs.mjs
- web-ui/modules/i18n/locales/zh-tw.mjs
- cli.js
- tests/unit/web-ui-logic.test.mjs
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.1)
cli/local-bridge.js
[warning] 156-156: Avoid using the initial state variable in setState
Context: setTimeout(resolve, delay)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🪛 HTMLHint (1.9.2)
web-ui/partials/index/panel-local-requests.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🔇 Additional comments (13)
cli/local-bridge.js (9)
63-72:normalized.startsWith('embeddings')on Line 72 still accepts invalid suffixes likeembeddingseviland forwards them upstream. This matches the earlier finding and remains unaddressed.
222-225:redactTextstill omits common token variants (access_token,refresh_token,id_token,client_secret), so they can leak intobodyPreview. Same as the prior finding.
239-242:summarystill pushes rawmessages[].content/input(Lines 240, 242) withoutredactText, so secrets can surface even whenbodyPreviewis redacted. Same as the prior finding.
305-309: Line 308 still reads only stringparsed.error, so OpenAI-styleerror.messagepayloads log an empty error. Same as the prior finding.
725-727: Line 727 still uses!suffix.startsWith('v1'), which accepts/bridge/local/v10/...and/bridge/local/v1eviland forwards them upstream. This was previously marked addressed, but the current snapshot shows the permissive check is still present.
796-801: The/responsesbranch still forwardsresponsesRequestunchanged (Line 796), so a configured upstreammodel/temperaturenever overrides the client request. Same as the prior finding.
139-167: LGTM!
269-317: LGTM!
943-986: LGTM!web-ui/partials/index/panel-local-requests.html (1)
1-1: Doctype hint is a false positive.This is an HTML partial included into a larger document, not a standalone page; the
doctype-firstHTMLHint rule doesn't apply here.web-ui/modules/app.methods.task-orchestration.mjs (2)
32-61: LGTM!Also applies to: 437-492, 704-763, 1243-1299
1226-1241: 🩺 Stability & AvailabilityPolling guard still has a reachable orchestration state
mainTabcan still be'orchestration'via nav-state restore, and it’s only rewritten toconfigwhentaskOrchestrationTabEnabledis false.> Likely an incorrect or invalid review comment.tests/unit/session-tab-switch-performance.test.mjs (1)
170-242: LGTM!
| <span class="local-ai-request-log-head"> | ||
| <span class="local-ai-request-method">{{ item.method }}</span> | ||
| <code>{{ item.path }}</code> | ||
| <span :class="['status-pill', item.status >= 200 && item.status < 400 ? 'success' : 'warn']">{{ item.status || '-' }}</span> | ||
| <span class="local-ai-request-duration">{{ item.durationMs }}ms</span> | ||
| </span> | ||
| <span class="local-ai-request-log-meta"> | ||
| <span v-if="item.provider">{{ t('localAi.client.requestLog.provider') }}: {{ item.provider }}</span> | ||
| <span v-if="item.model">model: {{ item.model }}</span> | ||
| <span>{{ item.at }}</span> | ||
| </span> | ||
| <span v-if="item.summary" class="local-ai-request-summary">{{ item.summary }}</span> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Unset durationMs/model renders literal undefinedms/-.
When item.durationMs is not yet populated (e.g., request still in flight or logging omitted it), {{ item.durationMs }}ms (Lines 44, 75) will literally render undefinedms instead of a placeholder. The model label at Lines 48 and 70 is also hardcoded in English rather than using t(...), unlike the surrounding labels (t('localAi.client.requestLog.provider'), etc.), which will look inconsistent in non-English locales given this PR explicitly ships zh/ja/vi locale updates.
🩹 Suggested fixes
- <span class="local-ai-request-duration">{{ item.durationMs }}ms</span>
+ <span class="local-ai-request-duration">{{ item.durationMs != null ? `${item.durationMs}ms` : '-' }}</span>- <span v-if="item.model">model: {{ item.model }}</span>
+ <span v-if="item.model">{{ t('localAi.client.requestLog.model') }}: {{ item.model }}</span>- <dt>model</dt>
+ <dt>{{ t('localAi.client.requestLog.model') }}</dt>Also applies to: 64-80
🤖 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 `@web-ui/partials/index/panel-local-requests.html` around lines 40 - 51, The
request log template in panel-local-requests should avoid rendering raw
undefined values and should localize the model label. Update the local-ai
request display logic so the `item.durationMs` field in the request
header/footer falls back to a placeholder like `-` when missing, and replace the
hardcoded `model:` text with the appropriate `t('localAi.client.requestLog...')`
translation key used alongside `localAi.client.requestLog.provider`. Apply the
same fix wherever the request log item is rendered so
`local-ai-request-duration` and the `item.model` label behave consistently.
1 participant
@awsl
Summary
本地服务navigation section with sibling entries服务and请求.服务focused on Local AI provider/gateway configuration.POST /chat,GET /models, and copy action.请求; keep that page focused on request items and selected request detail.服务and请求; refresh pulls latest data explicitly./v1path tolerance, and ignored upstream-error retries.Validation
npm run lint— passed, 579 files.npm run test:unit— passed, 763 tests.npm run test:e2e— passed.服务: status, Base URL,POST /chat,GET /models, copy action.请求no longer renders client config / gateway summary.502 -> 502 -> 200.Screenshot/manual evidence produced
tmp/e2e-gateway-compact/screens/01-service-gateway-compact.pngtmp/e2e-gateway-compact/screens/02-service-copy-action.pngtmp/e2e-gateway-compact/screens/03-requests-without-client-config.pngtmp/e2e-gateway-compact/screens/04-requests-selected-detail.pngtmp/e2e-gateway-compact/screens/05-mock-upstream-retry-trace.pngNotes
Summary by CodeRabbit
New Features
Bug Fixes