Conversation
Contributor
Author
|
/next |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough在 Node 与 Browser 两端加入 ACP agent/CLI 客户端、进程管理、文件与终端处理器，新增权限调用器/桥接/RPC、浏览器侧权限对话 UI 与对话管理，扩展聊天输入/视图/注册表、偏好与类型，并添加大量单元测试覆盖。 ChangesACP Agent 模式与权限对话集成
Sequence Diagram(s)（已在隐藏审查栈中提供高层示意序列图，涵盖 Node 权限请求 -> 浏览器 RPC -> 桥接 -> UI -> 决策 -> 回传 的交互。） Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/ai-native/src/browser/chat/chat-multi-diff-source.ts (1)
101-103:⚠️ Potential issue | 🟠 Major使用
Event.signal()消除双重断言的类型隐患。Line 102 的
as unknown as Event<void>双重断言会掩盖类型不匹配，应改用框架提供的事件适配工具。由于onDidChange的消费方不需要CodeBlockData的具体值，仅关心更新事件本身，最简洁的方案是使用Event.signal()直接将事件转换为Event<void>。建议修改示例
- // 这里event类型错误不影响 - onDidChange: this.baseApplyService.onCodeBlockUpdate as unknown as Event<void>, + // 使用 Event.signal 将 Event<CodeBlockData> 转换为 Event<void> + onDidChange: Event.signal(this.baseApplyService.onCodeBlockUpdate),若需保留事件消息但仅转发信号，也可用
Event.map()的替代方案。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/chat/chat-multi-diff-source.ts` around lines 101 - 103, The current assignment uses a double-cast "as unknown as Event<void>" for onDidChange; replace that unsafe cast by adapting baseApplyService.onCodeBlockUpdate via the framework event helper: use Event.signal(this.baseApplyService.onCodeBlockUpdate) to produce an Event<void> (or use Event.map if you need to preserve payload while only forwarding a signal). Update the onDidChange assignment to call Event.signal with the source event (baseApplyService.onCodeBlockUpdate) instead of the double assertion.packages/ai-native/src/browser/layout/ai-layout.tsx (1)
25-42:⚠️ Potential issue | 🟠 Major将 Hook 调用移到条件返回之前，确保 Hook 顺序一致
Line 25-36 的条件返回导致 Line 39-42 的
useMemo只在shouldShowFullLayout为 true 时执行。这违反了 React Hooks 规则：Hook 必须在每次渲染时以相同的顺序调用，不能在条件分支中。当shouldShowFullLayout的值在两次渲染间变化时，React 会抛出运行时错误。建议：将
defaultRightSize的初始化移到条件返回之前，或改用简单赋值替代useMemo（因为designLayoutConfig通常不会改变）。建议修改
// 判断是否应该显示完整布局 const shouldShowFullLayout = !isMobileDevice(); + // 正常模式：渲染完整布局 + const defaultRightSize = designLayoutConfig.useMergeRightWithLeftPanel ? 0 : 49; + // 移动端模式：只渲染 AI_CHAT_VIEW_ID，添加 mobile class if (!shouldShowFullLayout) { return ( <SlotRenderer slot={AI_CHAT_VIEW_ID} isTabbar={true} defaultSize={layout['AI-Chat']?.currentId ? layout['AI-Chat']?.size || 360 : 0} maxResize={420} minResize={280} minSize={0} /> ); } - - // 正常模式：渲染完整布局 - const defaultRightSize = useMemo( - () => (designLayoutConfig.useMergeRightWithLeftPanel ? 0 : 49), - [designLayoutConfig.useMergeRightWithLeftPanel], - );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/layout/ai-layout.tsx` around lines 25 - 42, The useMemo hook for defaultRightSize is called only when shouldShowFullLayout is true, violating React Hooks order; move the useMemo call for defaultRightSize (or replace it with a plain calculation) above the conditional return that renders SlotRenderer so that useMemo/designLayoutConfig is invoked on every render; ensure references to defaultRightSize, useMemo, designLayoutConfig, shouldShowFullLayout, SlotRenderer and AI_CHAT_VIEW_ID remain consistent after moving.packages/ai-native/src/browser/chat/chat-edit-resource.ts (1)
52-56:⚠️ Potential issue | 🟡 Minor补齐
side/id参数校验，避免错误查询默认落到右侧内容。当前当
side非left（含缺失/非法）时会直接走updatedCode分支，可能掩盖无效 URI 并返回错误内容。建议仅接受left/right，否则返回空串。🔧 建议修改
- async provideEditorDocumentModelContent(uri: URI, encoding?: string | undefined): Promise<string> { + async provideEditorDocumentModelContent(uri: URI, encoding?: string | undefined): Promise<string> { // Get the content from the base apply service based on the uri query parameters - const { id, side } = uri.getParsedQuery(); + const { id, side } = uri.getParsedQuery() as { id?: string; side?: string }; + if (!id || (side !== 'left' && side !== 'right')) { + return ''; + } const codeBlocks = this.baseApplyService.getSessionCodeBlocks(); const codeBlock = codeBlocks?.find((block) => block.toolCallId === id); - const content = side === 'left' ? codeBlock?.originalCode : codeBlock?.updatedCode; - return content || ''; + return side === 'left' ? (codeBlock?.originalCode ?? '') : (codeBlock?.updatedCode ?? ''); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/chat/chat-edit-resource.ts` around lines 52 - 56, The current logic uses side !== 'left' to select updatedCode which causes invalid/missing side or id to default to the right side; update the code around uri.getParsedQuery(), baseApplyService.getSessionCodeBlocks(), and the codeBlock lookup to strictly validate both id and side: ensure id is present, only accept side === 'left' or side === 'right', return '' immediately for any other values, find the codeBlock by toolCallId === id and if not found return '', and only then return codeBlock.originalCode for 'left' or codeBlock.updatedCode for 'right'.packages/ai-native/src/browser/ai-core.contribution.ts (1)
301-310:⚠️ Potential issue | 🟠 Major不要把
chatManagerService.init()变成未处理的后台任务。Line 309 去掉
await后，初始化失败会变成未处理 rejection，会话恢复也会和后续依赖方产生竞态。除非这里已经有独立的 readiness 机制，否则应继续等待它完成；如果是有意异步化，也至少要显式catch。建议修改
- this.chatManagerService.init(); + await this.chatManagerService.init();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/ai-core.contribution.ts` around lines 301 - 310, The call to chatManagerService.init() inside initialize is now fire-and-forget and can produce unhandled rejections and race conditions; change it to await chatManagerService.init() so initialize waits for completion (or, if intentional, explicitly handle errors by adding a .catch(...) that logs/fails gracefully and exposes readiness) — update the initialize method to either await chatManagerService.init() or wrap chatManagerService.init() with an explicit catch that surfaces errors; locate this in the initialize function alongside chatInternalService.init() and chatProxyService.registerDefaultAgent() and ensure AI_CHAT_VIEW_ID / AI_CHAT_CONTAINER_ID layout registration remains unchanged.packages/ai-native/src/browser/chat/chat.view.tsx (1)
815-829:⚠️ Potential issue | 🟠 MajorLine 816 的非空断言无法防止从 storage 恢复时
request为 undefined 导致的崩溃Line 816 先使用可选链
sessionModel?.getRequest(...)再用非空断言!掩盖，但renderReply函数签名中request: ChatRequestModel是必需参数。当从 storage 恢复的 assistant 消息缺少对应的 request 时，非空断言会让 undefined 通过类型检查，随后在renderReply内部访问request.requestId时触发运行时异常。需要移除非空断言并添加空值检查，当 request 为 undefined 时调用
renderSimpleMarkdownReply作为后备方案：🔧 建议修改
- const request = aiChatService.sessionModel?.getRequest(msg.requestId)!; + const request = aiChatService.sessionModel?.getRequest(msg.requestId); // 从storage恢复时，request为undefined if (request && !request.response.isComplete) { setLoading(true); } - await renderReply({ - msgId: msg.id, - relationId: msg.relationId!, - message: msg.content, - agentId: msg.agentId, - command: msg.agentCommand, - startTime: msg.replyStartTime!, - request, - }); + if (request) { + await renderReply({ + msgId: msg.id, + relationId: msg.relationId!, + message: msg.content, + agentId: msg.agentId, + command: msg.agentCommand, + startTime: msg.replyStartTime!, + request, + }); + } else if (msg.content) { + await renderSimpleMarkdownReply({ + relationId: msg.relationId!, + chunk: msg.content, + }); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/chat/chat.view.tsx` around lines 815 - 829, The code uses a non-null assertion on the result of aiChatService.sessionModel?.getRequest(msg.requestId) (the local variable request) but renderReply requires a ChatRequestModel and will throw if request is actually undefined during storage restore; remove the trailing "!" non-null assertion, add an explicit check for request before calling renderReply, and when request is undefined call renderSimpleMarkdownReply with the assistant message (preserving relationId/agentId/command/startTime as appropriate); also keep the existing setLoading logic only when request exists and !request.response.isComplete so you don't set loading for the fallback path.
🟠 Major comments (30)
packages/core-common/src/log.ts-214-216 (1)
214-216:⚠️ Potential issue | 🟠 Major恢复可控的 debug 开关，当前改动会让调试日志永久关闭
Line 215 被注释后，
this.isEnable在当前类中不再有任何赋值为true的路径，导致verbose/debug/log/warn/info全部不可达（仅error还能输出）。这会显著削弱排障能力。建议修复
constructor(namespace?: string) { if (typeof process !== 'undefined' && process.env && process.env.KTLOG_SHOW_DEBUG) { - // this.isEnable = true; + this.isEnable = true; } this.namespace = namespace || ''; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core-common/src/log.ts` around lines 214 - 216, The debug switch was effectively disabled by commenting out the assignment so this.isEnable never becomes true; restore the conditional that sets this.isEnable = true when process.env.KTLOG_SHOW_DEBUG is present (check the environment in the same block using typeof process !== 'undefined' and process.env) so the class-level flag (this.isEnable) can re-enable verbose/debug/log/warn/info methods (only error currently reachable); update the block that references process.env.KTLOG_SHOW_DEBUG to assign this.isEnable = true and keep the surrounding safety checks intact.packages/ai-native/src/browser/acp/permission.handler.ts-60-68 (1)
60-68:⚠️ Potential issue | 🟠 Major权限存储的异步初始化没有被等待。
Line 60-68 在构造函数里 fire-and-forget
initStorage()，但后面的requestPermission()/saveRules()已经假定 storage 可用了。首个请求如果来得够快，已存规则不会生效，用户点 “always” 之后的新规则也可能直接丢掉。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/acp/permission.handler.ts` around lines 60 - 68, The constructor calls initStorage() without waiting, so permissionStorage may be undefined when requestPermission() or saveRules() run; update the class to ensure initialization is awaited by creating and storing an initialization Promise (e.g., this.initPromise = this.initStorage()) or by making initStorage set a ready Promise, then await that Promise at the start of requestPermission() and saveRules() before accessing permissionStorage; reference the existing symbols: constructor, initStorage(), permissionStorage, storageProvider(STORAGE_NAMESPACE.AI_NATIVE), loadRules(), requestPermission(), and saveRules() so the methods reliably wait for storage to be initialized.packages/ai-native/src/browser/acp/permission.handler.ts-117-120 (1)
117-120:⚠️ Potential issue | 🟠 Major“始终允许/拒绝” 现在保存的是错误的 rule 维度。
Line 119 把
optionId传给addRule()，而 Line 260-278 又把它直接当成pattern存下来，并把kind固定成'write'。结果保存出来的 rule 类似allow_always => allow，后续checkRules()用真实路径/标题去匹配时永远命中不到，read/command规则也都会被误归类。建议修改
private pendingRequests = new Map< string, { resolve: (decision: PermissionDecision) => void; timeout: NodeJS.Timeout; + request: PermissionRequest; } >(); ... this.pendingRequests.set(requestId, { resolve, timeout, + request, }); ... if (always) { - this.addRule(requestId, optionId, allow ? 'allow' : 'reject'); + this.addRule(pending.request, allow ? 'allow' : 'reject'); } ... - private addRule(requestId: string, pattern: string, decision: 'allow' | 'reject'): void { - // Extract pattern from request - // This is a placeholder - actual implementation should extract from the request + private addRule(request: PermissionRequest, decision: 'allow' | 'reject'): void { + const pattern = + request.toolCall.locations?.length + ? request.toolCall.locations.map((l) => l.path).join(',') + : request.toolCall.title || ''; + const rule: PermissionRule = { id: uuid(), pattern, - kind: 'write', // Should be extracted from actual request + kind: request.toolCall.kind || 'read', decision, always: true, createdAt: Date.now(), };Also applies to: 260-278
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/acp/permission.handler.ts` around lines 117 - 120, The "always allow/reject" branch currently calls addRule(requestId, optionId, ...) and the storage logic (around the addRule implementation and the block at the 260-278 region) treats optionId as the rule pattern and hardcodes kind='write', which produces rules like "allow_always => allow" that never match in checkRules() and misclassifies read/command rules; change addRule calls and the rule-persistence code so that the stored rule uses the actual resource pattern/descriptor (e.g., the request's path/title or the resolved option pattern) and the correct kind (read/command/write) derived from the original request/option, not the optionId string; update addRule(requestId, optionId, ...) usage to pass the resolved pattern and kind (or have addRule resolve them from requestId/optionId) and ensure checkRules() matching will then succeed.packages/ai-native/src/browser/chat/chat-model.ts-315-329 (1)
315-329:⚠️ Potential issue | 🟠 Major
title新增后还没有进入持久化。Line 317-329 把标题加进了构造参数和 getter，但
toJSON()仍然没带上它。只要会话经过 storage/provider 恢复，标题就会丢失，这个 session 标题能力就只在当前内存实例里有效。建议修改
toJSON() { return { sessionId: this.sessionId, + title: this.title, modelId: this.modelId, history: this.history, requests: this.requests, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/chat/chat-model.ts` around lines 315 - 329, The new private field `#title` and its getter are not persisted because toJSON() does not include the title, so session restores lose it; update the ChatModel's serialization to include title (e.g., add title: this.#title to the object returned by toJSON()) and ensure any static fromJSON/restore factory (or constructor usage when deserializing) reads that title back into `#title` so restored sessions preserve the title; reference the constructor, `#title/title` getter and toJSON()/fromJSON() (or restore method) to locate where to add the write/read of the title.packages/ai-native/__test__/node/acp/cli-agent-process-manager.test.ts-40-46 (1)
40-46:⚠️ Potential issue | 🟠 Major这里把
cwd写死成/tmp，会让用例在非 POSIX 环境直接失败。如果 CI 覆盖 Windows，这个断言路径不存在。这里最好改成
os.tmpdir()或测试夹具目录。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/__test__/node/acp/cli-agent-process-manager.test.ts` around lines 40 - 46, The test hardcodes '/tmp' as the cwd which breaks on non-POSIX systems; update the second startAgent call in the test (in cli-agent-process-manager.test.ts) to use a cross-platform temp directory instead of '/tmp' — for example call processManager.startAgent('node', ['-e', 'setInterval(() => {}, 1000)'], {}, os.tmpdir()) or use the test fixture/tmpDir helper if available; ensure you import/require os (or the fixture) at the top so the test runs on Windows CI as well.packages/ai-native/src/node/acp/acp-cli-client.service.ts-216-234 (1)
216-234:⚠️ Potential issue | 🟠 Major挂起请求现在既不会超时，也不会在
close()时被 reject。
requestTimeoutMs定义了但没用；close()也没有像handleDisconnect()那样清空pendingRequests。只要 agent 漏一条响应，或者关闭发生在请求飞行中，调用方 Promise 就会永久悬挂。Also applies to: 248-270
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/node/acp/acp-cli-client.service.ts` around lines 216 - 234, The close() method currently clears connection state but neither rejects pendingRequests nor uses requestTimeoutMs, causing in-flight requests to hang; update close() (and mirror the logic used in handleDisconnect()) to iterate over this.pendingRequests and reject each Promise with an appropriate Error (e.g., "connection closed"), then clear the pendingRequests map/array; additionally, ensure the request-sending code (e.g., the method that registers entries in this.pendingRequests) honors this.requestTimeoutMs by starting a timer per request that rejects and cleans up the pendingRequests entry on timeout so callers never hang (apply same fix pattern to the other close-like block around the 248-270 region).packages/ai-native/src/node/acp/acp-cli-client.service.ts-250-253 (1)
250-253:⚠️ Potential issue | 🟠 Major断连后仍然可能继续向旧流写数据。
handleDisconnect()只把connected置成false，但保留了旧的stdin/stdout引用；而发送路径只检查this.stdin是否非空。断连后的新请求不会立即失败，而是继续写向失效 transport。Also applies to: 273-281, 433-454
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/node/acp/acp-cli-client.service.ts` around lines 250 - 253, The code allows writes to stale stdin/stdout after disconnect because handleDisconnect() only sets connected=false but leaves this.stdin/this.stdout references intact and sendRequest() only checks this.stdin non-null; update handleDisconnect() (and any disconnect handlers referenced around sendRequest, the logic at lines near 273-281 and 433-454) to null out/clear this.stdin and this.stdout (or replace with a closed stream/erroring stub) and/or make sendRequest() validate this.connected in addition to this.stdin before writing, so new requests fail fast; ensure any send paths (e.g., sendRequest<T>) re-check this.connected and throw immediately if disconnected to prevent writing to an invalid transport.packages/ai-native/src/browser/acp/permission-dialog-container.module.less-1-13 (1)
1-13:⚠️ Potential issue | 🟠 Major这个容器现在不是模态的。
根节点
pointer-events: none会让弹窗外区域继续把点击落到下面的页面。对权限确认这种交互来说，这会破坏模态语义，也容易在授权过程中触发背景操作。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/acp/permission-dialog-container.module.less` around lines 1 - 13, 根节点 .dialogContainer 现在使用 pointer-events: none 导致容器不是模态的——点击会穿透到页面底层；把根容器改为能拦截事件（移除 pointer-events: none 或改为 pointer-events: auto）以确保弹窗外区域不会把点击传递到背景，并在需要时在 .dialogContainer 内添加一个覆盖层（半透明背景）用于捕获和处理外部点击（例如用于取消或阻止点击），同时保留对弹窗内容的点击（目前的 > * 规则可按需保留或调整以确保子元素正常接收事件）。packages/ai-native/src/node/acp/acp-cli-client.service.ts-257-269 (1)
257-269:⚠️ Potential issue | 🟠 Major不要把 ACP 原始 payload 直接打到日志里。
这里会记录完整 params / message 片段，里面很容易带上用户 prompt、文件内容、认证信息或命令参数。默认日志级别下直接落盘，隐私和合规风险都太高。
Also applies to: 296-304
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/node/acp/acp-cli-client.service.ts` around lines 257 - 269, The current logging in send request writes full params/JSON (in logger?.log and logger?.debug) which may leak prompts, files, or credentials; update the send flow (references: logger?.log call, logger?.debug call, pendingRequests set, message/json creation, and stdin!.write) to avoid emitting raw payloads: log only non-sensitive metadata such as method and id (and optionally param keys without values), or replace params/json with a fixed placeholder like "[REDACTED]" at info/default levels; if you need payload visibility keep it behind a debug/trace-only sanitized formatter that strips values (or truncates and hashes) before logging, and ensure the code paths around the message/json and logger calls (including the similar block at lines 296-304) are changed accordingly.packages/ai-native/src/browser/chat/session-provider-registry.ts-83-94 (1)
83-94:⚠️ Potential issue | 🟠 Major重复注册时，旧 disposer 会误删新的 provider。
这里返回的
dispose()只按provider.id删除。先注册 A、再用同一 id 注册 B 之后，如果 A 的 disposer 晚一点执行，会把 B 也一起删掉。建议修复
return { dispose: () => { - this.providers.delete(provider.id); + if (this.providers.get(provider.id) === provider) { + this.providers.delete(provider.id); + } }, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/chat/session-provider-registry.ts` around lines 83 - 94, The dispose returned by registerProvider currently deletes by provider.id and can remove a newer provider with the same id; change it to capture the registered instance (or a unique token) at registration time and, in the returned dispose(), only delete this.providers.delete(provider.id) if this.providers.get(provider.id) === capturedInstance (or token matches); update registerProvider (and the IDisposable returned) so the disposer validates the stored instance before removing to avoid deleting a newer provider registered under the same id.packages/ai-native/src/node/acp/cli-agent-process-manager.ts-202-207 (1)
202-207:⚠️ Potential issue | 🟠 Major当前的进程组杀死逻辑存在缺陷，无法彻底清理子进程树。
代码尝试用
process.kill(-pid)杀死进程组，但子进程配置为detached: false。在这种情况下，子进程会继承父进程的进程组ID，而不是成为自己进程组的leader。因此process.kill(-pid)的负PID操作会失败（ESRCH错误），虽然代码确实有fallback逻辑（第270行捕获异常后改用单进程kill），但单进程kill只会清理直接子进程，无法杀死孙进程及更深层级的子进程。如果需要可靠地控制整个进程树，应该考虑使用
detached: true创建新的进程组。虽然代码注释说明了选择detached: false是为了等待子进程退出，但实际上即使设置detached: true，仍可通过监听exit事件正常等待进程退出，不会影响现有的等待逻辑。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/node/acp/cli-agent-process-manager.ts` around lines 202 - 207, The current child spawn uses detached: false so process.kill(-pid) can't target a new process group; change spawn(...) to use detached: true so the child becomes leader of its own process group, keep stdio as pipes and do NOT call child.unref() so the parent can still wait; retain the existing exit/close listeners (e.g., where the code currently awaits childProcess 'exit'/'close') to wait for termination, and update the group-kill logic to call process.kill(-childProcess.pid, signal) to terminate the whole group; keep the existing fallback single-process kill and platform-safe handling (Windows taskkill) for environments that don't support negative PIDs.packages/ai-native/src/node/acp/handlers/terminal.handler.ts-149-160 (1)
149-160:⚠️ Potential issue | 🟠 Major输出截断状态现在是不可信的。
缓冲区超限后这里按字符
slice()，不是按字节裁剪；而getTerminalOutput()又靠“当前 buffer 是否仍然超限”来推断truncated。结果就是旧输出明明已经被丢弃，返回仍可能是false。Also applies to: 209-216
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/node/acp/handlers/terminal.handler.ts` around lines 149 - 160, The truncation logic uses string.slice() (character-based) and doesn't set the truncated flag, causing getTerminalOutput() to incorrectly report truncated=false; update the onData handler (and the similar block around the other listener) to trim the buffer by bytes not characters: compute how many bytes to keep relative to terminalSession.outputByteLimit, convert the buffer to a byte-aware form (e.g., Buffer) to slice the tail by byte length, assign the resulting utf8 string back to terminalSession.outputBuffer, and set a terminalSession.truncated (or equivalent) boolean when any trimming occurs so getTerminalOutput() can reliably detect truncation. Ensure you update both the onData block that references terminalSession.outputBuffer and the repeated block at lines ~209-216.packages/ai-native/src/browser/chat/session-provider-registry.ts-119-121 (1)
119-121:⚠️ Potential issue | 🟠 Major
getProviderBySessionId()现在没有按 source 做路由。这里把完整
sessionId原样传给getProvider()，但接口和注释约定的是基于 source 选择 provider。只要某个实现按'local' | 'acp'判断canHandle()，这里就会直接找不到 provider。建议修复
-import { ISessionProvider, SessionProviderDomain } from './session-provider'; +import { ISessionProvider, SessionProviderDomain, parseSessionId } from './session-provider'; @@ getProviderBySessionId(sessionId: string): ISessionProvider | undefined { - const provider = this.getProvider(sessionId); + const { source } = parseSessionId(sessionId); + const provider = this.getProvider(source); return provider; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/chat/session-provider-registry.ts` around lines 119 - 121, getProviderBySessionId currently forwards the full sessionId to getProvider, but the registry should route by the session "source" (e.g., 'local' | 'acp') so implementations that base canHandle() on source fail to match; change getProviderBySessionId to extract the source from sessionId (e.g., parse by a delimiter such as ':' or use the established sessionId format to obtain the source token) and then call getProvider(source) or iterate providers and call provider.canHandle(source) to locate the correct ISessionProvider; update getProviderBySessionId to return that provider instead of passing the whole sessionId through.packages/ai-native/src/browser/components/mention-input/mention-input.tsx-97-99 (1)
97-99:⚠️ Potential issue | 🟠 Major
optionsBottomPosition这段状态目前没有真正生效。这里只初始化成
0，但整个文件里没有任何地方调用setOptionsBottomPosition。因此PermissionDialogWidget会一直收到固定的bottom={0}，和 footer / selector 的实际高度脱钩。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/components/mention-input/mention-input.tsx` around lines 97 - 99, The state optionsBottomPosition is never updated, so PermissionDialogWidget always receives bottom={0}; add logic to measure the actual footer/selector height and update setOptionsBottomPosition accordingly: attach a ref to the footer/selector element (the DOM node that determines the bottom offset), on mount measure its clientHeight and call setOptionsBottomPosition(height) and also subscribe to changes via a ResizeObserver (or window resize) to update it dynamically; ensure you cleanup the observer on unmount. Update any JSX to pass the measured optionsBottomPosition to PermissionDialogWidget and remove the hardcoded 0 usage.packages/ai-native/src/browser/chat/chat.internal.service.ts-128-135 (1)
128-135:⚠️ Potential issue | 🟠 Major避免
clearSessionModel先删后建留下半失效状态。当前会先清理旧会话，再
await startSession()。如果新会话创建失败，服务会停在“旧会话已清理 / 新会话未建立”的中间态，而#sessionModel仍可能保留旧值。这里至少要在失败时显式清空/回退当前会话，并补上 loading 的生命周期。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/chat/chat.internal.service.ts` around lines 128 - 135, clearSessionModel currently clears the old session first then awaits startSession, risking a half-invalid state if startSession fails; change it so you either create the new session before removing the old one or perform a safe swap with try/catch and explicit rollback/clearing: call this._onWillClearSession, set this.#sessionModel to a "loading" or null sentinel, then await this.chatManagerService.startSession() inside a try block, on success replace `#sessionModel` and call this.chatManagerService.clearSession(oldSessionId) and this._onChangeSession; on failure ensure `#sessionModel` is explicitly null/cleared (or restored to a previous safe state) and emit appropriate change/error lifecycle events so the UI never sees “old cleared but new missing” state.packages/ai-native/src/browser/chat/chat.internal.service.ts-86-95 (1)
86-95:⚠️ Potential issue | 🟠 Major不要在后端未实现时仍然广播“模式切换成功”。
这里用了可选调用。
setSessionMode缺失时，await会直接得到undefined，但_onModeChange.fire(modeId)仍然会执行。UI 会以为模式已经切走，后端实际却没变。🛠️ 建议修改
async setSessionMode(modeId: string): Promise<void> { const sessionId = this.#sessionModel?.sessionId; if (!sessionId) { throw new Error('No active session'); } - await this.aiBackService.setSessionMode?.(sessionId, modeId); + if (!this.aiBackService.setSessionMode) { + throw new Error('setSessionMode is not supported by the current backend'); + } + await this.aiBackService.setSessionMode(sessionId, modeId); // 切换成功后通知前端 UI 同步更新当前模式 this._onModeChange.fire(modeId); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/chat/chat.internal.service.ts` around lines 86 - 95, The method setSessionMode currently always fires _onModeChange.fire(modeId) even when aiBackService.setSessionMode is missing (optional) and therefore does nothing; change setSessionMode to first verify aiBackService.setSessionMode exists and await its call (or capture its result) and only call this._onModeChange.fire(modeId) when the backend call actually executed successfully (or the function exists and resolved without error); if aiBackService.setSessionMode is undefined, either throw or return without firing to avoid informing the UI of a non-existent backend change (refer to the setSessionMode method and aiBackService.setSessionMode and _onModeChange.fire symbols).packages/ai-native/src/browser/chat/chat.internal.service.ts-72-77 (1)
72-77:⚠️ Potential issue | 🟠 Major在
onStorageInit回调里补上await this.createSessionModel()。现在回调已经是
async，但无会话分支仍然直接丢掉 Promise。startSession()一旦失败会变成未处理拒绝，而且 storage init 会在会话真正建立前提前结束。🛠️ 建议修改
if (sessions.length > 0) { await this.activateSession(sessions[sessions.length - 1].sessionId); } else { - this.createSessionModel(); + await this.createSessionModel(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/chat/chat.internal.service.ts` around lines 72 - 77, The onStorageInit callback currently awaits activateSession when sessions exist but does not await the Promise returned by createSessionModel() in the "no sessions" branch, causing unhandled rejections if startSession() fails and finishing storage init prematurely; update the onStorageInit handler so it awaits this.createSessionModel() (i.e., replace the bare call with await this.createSessionModel()) to ensure session creation completes before storage init resolves and to propagate errors properly from createSessionModel/startSession.packages/ai-native/src/browser/chat/chat.internal.service.ts-119-125 (1)
119-125:⚠️ Potential issue | 🟠 Major
createSessionModel需要用finally回收 loading 状态。
startSession()抛错时，Line 121 打开的 loading 永远不会关闭，界面会一直卡在 loading。🛠️ 建议修改
async createSessionModel() { - // this.__isSessionLoading = true; this._onSessionLoadingChange.fire(true); - this.#sessionModel = await this.chatManagerService.startSession(); - this._onChangeSession.fire(this.#sessionModel.sessionId); - // this.__isSessionLoading = false; - this._onSessionLoadingChange.fire(false); + try { + this.#sessionModel = await this.chatManagerService.startSession(); + this._onChangeSession.fire(this.#sessionModel.sessionId); + } finally { + this._onSessionLoadingChange.fire(false); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/chat/chat.internal.service.ts` around lines 119 - 125, createSessionModel sets loading true then awaits chatManagerService.startSession but never resets loading if startSession throws; wrap the await in a try/finally so _onSessionLoadingChange.fire(false) always runs, assign this.#sessionModel and fire _onChangeSession inside the try after the await, and rethrow or let the error propagate after the finally so callers still see failures.packages/ai-native/src/node/acp/acp-cli-back.service.ts-261-299 (1)
261-299:⚠️ Potential issue | 🟠 Major恢复历史时不要把 chunk 直接当成完整消息。
这里把
user_message_chunk/agent_message_chunk每条通知都直接push成一条消息了。会话恢复后，同一条用户消息或 agent 回复会被切成很多碎片消息，后面的 summary、follow-up 和上下文窗口都会错位。至少要先把连续 chunk 合并成完整 message，再返回给上层。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/node/acp/acp-cli-back.service.ts` around lines 261 - 299, convertSessionUpdatesToMessages currently treats each user_message_chunk/agent_message_chunk as a full message, causing fragmented messages after restoration; change it to buffer and merge consecutive chunk notifications for the same logical message by concatenating update.content.text (preserve order) and using the first/earliest timestamp, then push a single {role, content, timestamp} when the sequence ends (detect end by a non-chunk update or a boundary change in role/session id/sequence indicator on notification.update); update logic in convertSessionUpdatesToMessages to accumulate chunks (for both 'user_message_chunk' and 'agent_message_chunk') and flush the combined message instead of pushing every chunk separately.packages/ai-native/src/browser/components/permission-dialog-widget.tsx-35-41 (1)
35-41:⚠️ Potential issue | 🟠 Major
Enter键读取过期的焦点索引
keydown事件监听在 effect 中注册，但依赖数组仅包含[dialogs.length, dialogs]，没有focusedIndex。当用户按 ArrowUp/ArrowDown 改变焦点时，focusedIndex状态会更新，但 effect 不会重新运行，导致handleKeyboard仍然持有旧闭包中的focusedIndex。虽然 ArrowUp/ArrowDown 通过函数式 setState 能正常工作，但第 59 行Enter键读取的focusedIndex仍是过期值，会错误提交。建议：将
focusedIndex加入依赖数组，或用useCallback包装handleKeyboard并依赖focusedIndex，或用 ref 保存最新值。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/components/permission-dialog-widget.tsx` around lines 35 - 41, The keydown handler (handleKeyboard) is capturing a stale focusedIndex because the effect that registers it only depends on [dialogs.length, dialogs]; update the effect so the listener always sees the latest focusedIndex by either adding focusedIndex to the useEffect dependency array, or memoizing handleKeyboard with useCallback that lists focusedIndex in its deps, or storing focusedIndex in a ref and reading from that ref inside handleKeyboard; locate handleKeyboard, the useEffect that adds/removes the 'keydown' listener, and the focusedIndex state to apply one of these fixes.packages/ai-native/src/browser/acp/permission-dialog.view.tsx-40-61 (1)
40-61:⚠️ Potential issue | 🟠 Major倒计时在新请求或重新打开时不会重置。
remainingTime只在首次挂载时用timeout初始化，不会在后续变化时重置。当组件被复用到新的requestId或visible再次变为true时，会继承之前的倒计时状态（通常是 0）。由于倒计时效果在第 45 行检查if (!visible || remainingTime <= 0)，如果状态未重置，新弹窗会立即满足退出条件，导致 auto-reject。🔧 建议修改
const [remainingTime, setRemainingTime] = useState(timeout); + + useEffect(() => { + if (visible) { + setRemainingTime(timeout); + } + }, [visible, timeout, requestId]); // Countdown timer useEffect(() => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/acp/permission-dialog.view.tsx` around lines 40 - 61, The countdown state remainingTime is only initialized from timeout once and never reset for new requests or when visible toggles, causing immediate auto-reject; add an effect that resets remainingTime to timeout whenever visible becomes true or requestId changes (e.g. useEffect(() => { if (visible) setRemainingTime(timeout); }, [visible, requestId, timeout])) and keep the existing countdown effect (which references remainingTime, visible, requestId, onClose) so the timer restarts correctly for each new request.packages/ai-native/src/browser/components/ChatMentionInput.tsx-450-455 (1)
450-455:⚠️ Potential issue | 🟠 Major
useMemo依赖数组不完整
defaultMentionInputFooterOptions使用了modeOptions和currentMode，但它们没有被包含在依赖数组中。这会导致当 mode 相关状态变化时，footer 配置不会更新。🔧 建议修复
- [iconService, handleShowMCPConfig, handleShowRules, props.disableModelSelector, props.sessionModelId], + [iconService, handleShowMCPConfig, handleShowRules, props.disableModelSelector, props.sessionModelId, modeOptions, currentMode],Also applies to: 530-530
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/components/ChatMentionInput.tsx` around lines 450 - 455, The useMemo that computes defaultMentionInputFooterOptions (type FooterConfig) is missing dependencies, so changes to modeOptions or currentMode won't update the memoized value; update the dependency array of the useMemo that defines defaultMentionInputFooterOptions to include modeOptions and currentMode (and any other values used inside like modeOptions[0]) so the footer config recomputes when those values change—look for the useMemo that returns { modeOptions, defaultMode: modeOptions[0]?.id || 'default', currentMode, showModeSelector: modeOptions.length > 1 } and add modeOptions and currentMode to its dependency list.packages/ai-native/src/browser/acp/permission-dialog-container.tsx-238-245 (1)
238-245:⚠️ Potential issue | 🟠 Major
handleDialogClose也需要通知 bridge service关闭对话框时应该通知
permissionBridgeService用户取消了操作，否则后端会一直等待响应。🔧 建议修复
// 处理对话框关闭 const handleDialogClose = useCallback(() => { if (dialogs.length === 0) { return; } const requestId = dialogs[0].requestId; + // 通知 bridge service 用户取消了操作 + permissionBridgeService.handleDialogClose(requestId); functionComponentDialogManager.removeDialog(requestId); - }, [dialogs]); + }, [dialogs, permissionBridgeService]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/acp/permission-dialog-container.tsx` around lines 238 - 245, The dialog close handler (handleDialogClose) currently only removes the dialog via functionComponentDialogManager.removeDialog(requestId) and does not inform the backend; update handleDialogClose to also call the permissionBridgeService cancellation notification with the same requestId (e.g., permissionBridgeService.notifyDialogCancelled(requestId) or the appropriate cancel method provided by permissionBridgeService) so the backend stops waiting for a response, and guard the call if permissionBridgeService is undefined; keep the removeDialog call and ensure the notification runs before or immediately after removing the dialog.packages/ai-native/src/browser/index.ts-203-206 (1)
203-206:⚠️ Potential issue | 🟠 Major
DefaultChatAgentToken的绑定和模式分支不一致。
ChatAgentPromptProvider已经按supportsAgentMode分支，但这里始终把DefaultChatAgentToken绑定到AcpChatAgent。如果本地模式还需要保留，这里会让非 ACP 场景也解析到 ACP 实现。Also applies to: 232-239
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/index.ts` around lines 203 - 206, DefaultChatAgentToken is always bound to AcpChatAgent which conflicts with ChatAgentPromptProvider's supportsAgentMode branching and forces non-ACP scenarios to resolve to the ACP implementation; change the binding so DefaultChatAgentToken resolves conditionally based on supportsAgentMode (the same condition used by ChatAgentPromptProvider) and preserve the local-mode binding (e.g., map DefaultChatAgentToken to the local implementation when supportsAgentMode is false); update both binding sites (the one shown and the similar block around lines 232-239) to use the conditional/resolution logic so non-ACP flows get the correct agent implementation.packages/ai-native/src/node/acp/acp-agent.service.ts-483-485 (1)
483-485:⚠️ Potential issue | 🟠 Major权限弹窗和取消请求都可能落到错误的 session 上。
requestPermission()取的是this.sessionInfo?.sessionId，拒绝工具调用时也取消this.sessionInfo.sessionId。但真正触发 tool call 的是当前通知对应的notification.sessionId。一旦存在多 session，或者createSession()后还没更新sessionInfo，这里就会把权限请求/取消发到别的会话甚至空 session。🔁 建议修复
把当前通知的
sessionId一路透传到权限请求和取消路径，例如：- async requestPermission(toolCallUpdate: ToolCallUpdate): Promise<PermissionResult> { + async requestPermission(sessionId: string, toolCallUpdate: ToolCallUpdate): Promise<PermissionResult> { const request: RequestPermissionRequest = { - sessionId: this.sessionInfo?.sessionId || '', + sessionId,- this.handleToolCallWithPermission(update, stream, pendingToolCalls); + this.handleToolCallWithPermission(notification.sessionId, update, stream, pendingToolCalls);- const result = await this.requestPermission(toolCallUpdate); + const result = await this.requestPermission(sessionId, toolCallUpdate); ... - await this.cancelRequest(this.sessionInfo.sessionId); + await this.cancelRequest(sessionId);Also applies to: 511-527, 642-674
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/node/acp/acp-agent.service.ts` around lines 483 - 485, The permission popup and cancel actions use this.sessionInfo?.sessionId, which can target the wrong or empty session when multiple sessions exist or sessionInfo is stale; update the flow so the notification's sessionId is passed through to requestPermission and any cancel logic: modify handleToolCallWithPermission (and related code paths referenced around lines 511-527 and 642-674) to accept an explicit sessionId argument (use notification.sessionId) and propagate that sessionId into requestPermission(), the rejection/cancel handler, and any createSession-related callbacks instead of reading this.sessionInfo; ensure requestPermission() signature and callers accept the sessionId parameter and that cancel actions use the same sessionId value.packages/ai-native/src/browser/acp/permission-bridge.service.ts-124-145 (1)
124-145:⚠️ Potential issue | 🟠 Major
cancelRequest()现在会把取消态错误地上报成timeout。
cancelRequest()直接复用了handleDialogClose()，而handleDialogClose()固定返回{ type: 'timeout' }。这样上游拿不到真实的取消结果，取消和超时会被混成一种状态。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/acp/permission-bridge.service.ts` around lines 124 - 145, The cancelRequest currently reuses handleDialogClose which always produces a PermissionDecision of { type: 'timeout' }, so cancelled requests are misreported as timeouts; change cancelRequest to produce and dispatch a distinct cancel decision (e.g., { type: 'cancel' } or { type: 'user_cancel' }) and perform the same cleanup (clearTimeout, delete from pendingDecisions and activeDialogs, fire onPermissionResult, resolve/reject the pending promise) or refactor the shared cleanup into a helper like finalizeRequest(requestId, decision) that both handleDialogClose and cancelRequest call to ensure correct decision is reported.packages/ai-native/src/browser/chat/chat-manager.service.ts-117-134 (1)
117-134:⚠️ Potential issue | 🟠 Major保存会话时把
title丢掉了。
fromJSON()已经会恢复item.title，但这里序列化时没有写回去。只要触发一次saveSessions()，已有标题就会被覆盖掉。💾 建议修复
private toSessionData(model: ChatModel): ISessionModel { return { sessionId: model.sessionId, modelId: model.modelId, + title: model.title, history: model.history.toJSON(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/chat/chat-manager.service.ts` around lines 117 - 134, toSessionData currently omits the session title so saving sessions overwrites existing titles; update the toSessionData method to include the title field (e.g., add title: model.title) in the returned ISessionModel object so fromJSON() can restore item.title correctly; locate the toSessionData function in chat-manager.service.ts and append the title property to the returned object alongside sessionId, modelId, history, and requests.packages/ai-native/src/browser/chat/chat-manager.service.ts-160-164 (1)
160-164:⚠️ Potential issue | 🟠 Major预加载进缓存的 session 没有挂上自动保存监听。
startSession()和getSession()在放入缓存后都会调用listenSession()，但init()这里没有。这样启动时恢复出来的会话后续触发history.onMessageAdditionalChange时不会自动持久化。🔄 建议修复
savedSessions.forEach((session) => { this.#sessionModels.set(session.sessionId, session); + this.listenSession(session); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/chat/chat-manager.service.ts` around lines 160 - 164, Preloaded sessions created via fromJSON and inserted into the `#sessionModels` map in init() are not registered with listenSession(), so later history.onMessageAdditionalChange events won't trigger auto-save; update the init() flow that loops over savedSessions (the block where savedSessions.forEach(session => this.#sessionModels.set(session.sessionId, session))) to call listenSession(session) (or the equivalent registration method) after adding each session to `#sessionModels`, mirroring what startSession() and getSession() do.packages/ai-native/src/node/acp/acp-agent.service.ts-453-460 (1)
453-460:⚠️ Potential issue | 🟠 Major这里会对无
update的通知直接解引用。同文件的
loadSession()已经把notification.update当成可空处理了，这里却直接访问update.sessionUpdate。只要 agent 推来同 session 的非 update 通知，这个回调就会抛异常并中断当前流。🛡️ 建议修复
): void { const update = notification.update; + if (!update) { + return; + } switch (update.sessionUpdate) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/node/acp/acp-agent.service.ts` around lines 453 - 460, handleNotification dereferences notification.update without null-check which throws when non-update notifications arrive; change handleNotification (handling SessionNotification) to guard for a missing update at the top (e.g. if (!notification.update) { /* ignore or handle non-update notification and return */ }) or use a safe-access pattern before reading update.sessionUpdate so the stream isn't broken by non-update notifications; mirror the nullable handling used in loadSession() and ensure any early return still properly maintains stream/pendingToolCalls semantics.packages/ai-native/src/browser/index.ts-119-123 (1)
119-123:⚠️ Potential issue | 🟠 Major
AINativeBrowserContribution被重复注册了。前面已经注册过一次了。Contribution 类通常会注册命令、菜单和监听器，重复注入很容易带来重复 side effect。
🧹 建议修复
- AINativeBrowserContribution, AcpPermissionDialogContribution, PermissionDialogManager, AcpPermissionBridgeService,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-native/src/browser/index.ts` around lines 119 - 123, AINativeBrowserContribution is registered twice in the exported contribution list causing duplicate side effects; remove the duplicate entry so AINativeBrowserContribution appears only once (leave other exports like AcpPermissionDialogContribution, PermissionDialogManager, AcpPermissionBridgeService intact) and verify any registration/DI container usage only consumes the single AINativeBrowserContribution symbol to avoid double command/menu/listener registration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: dab2d7ed-03ed-4eeb-aec1-41abd2767dec
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (60)
packages/ai-native/__test__/browser/chat/chat-manager.service.test.tspackages/ai-native/__test__/node/acp/cli-agent-process-manager.test.tspackages/ai-native/package.jsonpackages/ai-native/src/browser/acp/acp-permission-rpc.service.tspackages/ai-native/src/browser/acp/index.tspackages/ai-native/src/browser/acp/permission-bridge.service.tspackages/ai-native/src/browser/acp/permission-dialog-container.module.lesspackages/ai-native/src/browser/acp/permission-dialog-container.tsxpackages/ai-native/src/browser/acp/permission-dialog.module.lesspackages/ai-native/src/browser/acp/permission-dialog.view.tsxpackages/ai-native/src/browser/acp/permission.handler.tspackages/ai-native/src/browser/ai-core.contribution.tspackages/ai-native/src/browser/chat/acp-chat-agent.tspackages/ai-native/src/browser/chat/acp-session-provider.tspackages/ai-native/src/browser/chat/apply.service.tspackages/ai-native/src/browser/chat/chat-agent.service.tspackages/ai-native/src/browser/chat/chat-agent.view.service.tspackages/ai-native/src/browser/chat/chat-edit-resource.tspackages/ai-native/src/browser/chat/chat-manager.service.tspackages/ai-native/src/browser/chat/chat-model.tspackages/ai-native/src/browser/chat/chat-multi-diff-source.tspackages/ai-native/src/browser/chat/chat-proxy.service.tspackages/ai-native/src/browser/chat/chat.api.service.tspackages/ai-native/src/browser/chat/chat.feature.registry.tspackages/ai-native/src/browser/chat/chat.internal.service.tspackages/ai-native/src/browser/chat/chat.render.registry.tspackages/ai-native/src/browser/chat/chat.view.tsxpackages/ai-native/src/browser/chat/default-chat-agent.tspackages/ai-native/src/browser/chat/local-storage-provider.tspackages/ai-native/src/browser/chat/session-provider-registry.tspackages/ai-native/src/browser/chat/session-provider.tspackages/ai-native/src/browser/components/ChatHistory.tsxpackages/ai-native/src/browser/components/ChatMentionInput.tsxpackages/ai-native/src/browser/components/mention-input/mention-input.tsxpackages/ai-native/src/browser/components/mention-input/types.tspackages/ai-native/src/browser/components/permission-dialog-widget.module.lesspackages/ai-native/src/browser/components/permission-dialog-widget.tsxpackages/ai-native/src/browser/index.tspackages/ai-native/src/browser/layout/ai-layout.tsxpackages/ai-native/src/common/acp-types.tspackages/ai-native/src/common/agent-types.tspackages/ai-native/src/common/index.tspackages/ai-native/src/common/prompts/empty-prompt-provider.tspackages/ai-native/src/node/acp/acp-agent.service.tspackages/ai-native/src/node/acp/acp-cli-back.service.tspackages/ai-native/src/node/acp/acp-cli-client.service.tspackages/ai-native/src/node/acp/acp-permission-caller.service.tspackages/ai-native/src/node/acp/cli-agent-process-manager.tspackages/ai-native/src/node/acp/handlers/agent-request.handler.tspackages/ai-native/src/node/acp/handlers/constants.tspackages/ai-native/src/node/acp/handlers/file-system.handler.tspackages/ai-native/src/node/acp/handlers/terminal.handler.tspackages/ai-native/src/node/acp/index.tspackages/ai-native/src/node/index.tspackages/core-browser/src/ai-native/ai-config.service.tspackages/core-common/src/log.tspackages/core-common/src/storage.tspackages/core-common/src/types/ai-native/index.tspackages/startup/entry/sample-modules/ai-native/ai-native.contribution.tspackages/startup/entry/web/server.ts
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1773654797.0 |
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1773716644.0 |
Contributor
Author
|
/next |
1 similar comment
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1773819640.0 |
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1773827963.0 |
Contributor
Author
|
/next |
1 similar comment
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1773914436.0 |
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1774233236.0 |
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1774269438.0 |
Contributor
Author
|
/next |
1 similar comment
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1774430680.0 |
Member
|
👍🏻 |
Contributor
Author
|
/next |
1 similar comment
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1775806869.0 |
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1775815562.0 |
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1779073145.0 |
- Remove the slashCommand useEffect in MentionInput that duplicated the slash tag alongside the custom event insertion path - Guard CodeBlockWrapperInput to skip rendering the command prop tag when it was already extracted from the message text Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ervice Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1779079561.0 |
- Add @opensumi/di mock to prevent DI container errors in unit tests - Make AcpPermissionHandler.initStorage() lazy (ensureInitialized pattern) - Fix auditLog assertion to match actual timestamp format - Fix getSmartTitle assertion for undefined kind Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1779086995.0 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (30)
packages/ai-native/src/browser/acp/components/AcpChatHistory.tsx-75-79 (1)
75-79:⚠️ Potential issue | 🟠 Major | ⚡ Quick win标题重命名入口缺失，编辑能力当前不可触发。
handleTitleEdit已定义，但列表项标题没有任何事件触发该状态切换，导致onHistoryItemChange路径在 UI 上不可达。可参考的修复
-<span id={`chat-history-item-title-${item.id}`} className={styles.chat_history_item_title}> +<span + id={`chat-history-item-title-${item.id}`} + className={styles.chat_history_item_title} + onDoubleClick={() => handleTitleEdit(item)} +> {item.title} </span>Also applies to: 173-177
🤖 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 `@packages/ai-native/src/browser/acp/components/AcpChatHistory.tsx` around lines 75 - 79, The title rename entry is not reachable because handleTitleEdit is never wired to the UI; wire the handler to the title element(s) so clicking/activating a history item toggles setHistoryTitleEditable for that item and thus enables the onHistoryItemChange path. Specifically, attach handleTitleEdit (or an inline call to setHistoryTitleEditable) to the title render of each IChatHistoryItem (the element that displays item.title) and ensure the same wiring exists for the other occurrence referenced around lines 173-177 so the edit UI becomes reachable.packages/ai-native/src/browser/acp/components/AcpChatHistory.tsx-162-166 (1)
162-166:⚠️ Potential issue | 🟠 Major | ⚡ Quick win历史项仅支持鼠标点击，键盘用户无法操作。
当前交互元素使用
div且仅绑定onClick，会阻断键盘导航/触发。可参考的修复
<div key={item.id} className={cls(styles.chat_history_item, item.id === currentId ? styles.chat_history_item_selected : '')} onClick={() => handleHistoryItemSelect(item)} + role='button' + tabIndex={disabled ? -1 : 0} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + handleHistoryItemSelect(item); + } + }} >🤖 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 `@packages/ai-native/src/browser/acp/components/AcpChatHistory.tsx` around lines 162 - 166, The chat history item is a plain div with only onClick, blocking keyboard users; update the element rendered for items in AcpChatHistory (the div using className with styles.chat_history_item and comparing item.id to currentId) to be keyboard-accessible: either render a semantic <button> or add role="button" and tabIndex={0}, wire an onKeyDown handler that calls handleHistoryItemSelect(item) when Enter or Space is pressed, and include an appropriate aria-selected or aria-current based on currentId so keyboard and assistive tech can operate it.packages/ai-native/src/browser/acp/permission.handler.ts-121-124 (1)
121-124:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift“始终允许/拒绝” 规则写入了错误的匹配信息。
handleUserResponse()把optionId传给addRule()，而addRule()又把它当成pattern存起来，并且把kind硬编码成'write'。这样保存下来的规则既匹配不到原始工具请求，也会把非写操作错误归类成写操作，后续自动授权结果会失真。Also applies to: 263-279
🤖 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 `@packages/ai-native/src/browser/acp/permission.handler.ts` around lines 121 - 124, handleUserResponse is currently calling addRule(requestId, optionId, ...) which causes addRule to store optionId as the rule pattern and hardcode kind='write'; update handleUserResponse (and the similar block at 263-279) to pass the actual request pattern and request kind into addRule instead of optionId, e.g. extract the original request pattern/identifier and its operation kind from the request object and call addRule(requestId, /*pattern=*/request.patternOrMatchValue, /*kind=*/request.kind, allow ? 'allow' : 'reject'); then update addRule's signature/usage to accept and persist the correct pattern and kind so saved rules match the original requests and preserve the real operation type rather than always using 'write'.packages/ai-native/src/browser/acp/components/AcpChatMentionInput.tsx-764-773 (1)
764-773:⚠️ Potential issue | 🟠 Major | ⚡ Quick winACP 专属 slash 命令现在能展示，但选中后不会生效。
slashCommands里把acpSlashCommands也拼进来了，但handleSlashSelect()只从chatFeatureRegistry查命令。选中 ACP-only 命令时commandModel会是undefined，最终不会写入theme/agentId/command。🤖 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 `@packages/ai-native/src/browser/acp/components/AcpChatMentionInput.tsx` around lines 764 - 773, handleSlashSelect currently only looks up commands via chatFeatureRegistry.getSlashCommandBySlashName, so ACP-only commands (merged into slashCommands via acpSlashCommands) return undefined and don't apply; update handleSlashSelect to fallback to lookup in acpSlashCommands when commandModel is undefined (use the same nameWithSlash key), then call props.setTheme(nameWithSlash) and setAgentId/setCommand from whichever model you found (commandModel or the acp command), using optional chaining/null checks to avoid crashes.packages/ai-native/src/browser/acp/permission.handler.ts-61-72 (1)
61-72:⚠️ Potential issue | 🟠 Major | ⚡ Quick win权限规则首次命中时可能还没加载完成。
ensureInitialized()只是 fire-and-forget 地调用initStorage()，requestPermission()紧接着就执行checkRules()。首个权限请求到来时，持久化规则很容易还没从 storage 读出来，自动决策会被直接绕过；后续saveRules()也有机会在permissionStorage尚未就绪时执行。可参考的调整
+ private initializing?: Promise<void>; + - private ensureInitialized(): void { - if (this.initialized) { - return; - } - this.initialized = true; - this.initStorage(); + private async ensureInitialized(): Promise<void> { + if (this.initialized) { + return; + } + this.initializing ??= this.initStorage().then(() => { + this.initialized = true; + }); + await this.initializing; } private async initStorage(): Promise<void> { this.permissionStorage = await this.storageProvider(STORAGE_NAMESPACE.AI_NATIVE); this.loadRules(); }然后在
requestPermission()/getRules()/removeRule()/clearRules()等入口统一await它。Also applies to: 77-85
🤖 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 `@packages/ai-native/src/browser/acp/permission.handler.ts` around lines 61 - 72, ensureInitialized currently calls initStorage() fire-and-forget so the first requestPermission/checkRules may run before permissionStorage and loadRules complete; make initialization awaitable by turning ensureInitialized into an async method that awaits initStorage (or maintain a promise property like this.initPromise = this.initStorage()), then update all public entry points (requestPermission, getRules, removeRule, clearRules, and any callers of checkRules/saveRules/loadRules) to await the initialization promise or call await ensureInitialized() before touching permissionStorage or rules; reference symbols: ensureInitialized, initStorage, initPromise (or initialized), permissionStorage, loadRules, saveRules, requestPermission, checkRules, getRules, removeRule, clearRules, storageProvider, STORAGE_NAMESPACE.AI_NATIVE.packages/ai-native/src/browser/acp/components/AcpChatViewHeader.tsx-162-188 (1)
162-188:⚠️ Potential issue | 🟠 Major | ⚡ Quick win会话切换后会遗留旧的消息监听。
初始
sessionModel.history.onMessageChange()被直接push到toDispose，但后续切换会话时只会释放previousMessageChangeDisposable。这样首个会话的监听会一直挂着，旧会话消息变化仍会触发刷新，而且切换次数越多泄漏越明显。🤖 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 `@packages/ai-native/src/browser/acp/components/AcpChatViewHeader.tsx` around lines 162 - 188, The initial onMessageChange subscription is pushed directly into toDispose so it never gets cleared when sessions switch; instead, assign the result of aiChatService.sessionModel.history.onMessageChange(...) to previousMessageChangeDisposable for the initial subscription (same symbol used later), push a disposable that calls previousMessageChangeDisposable?.dispose() into the DisposableCollection, and on aiChatService.onChangeSession dispose the previousMessageChangeDisposable before replacing it with the new subscription; ensure all uses reference getHistoryList, previousMessageChangeDisposable, aiChatService.onChangeSession and aiChatService.sessionModel.history.onMessageChange so no stray listeners remain.packages/ai-native/src/browser/acp/permission.handler.ts-224-247 (1)
224-247:⚠️ Potential issue | 🟠 Major | ⚡ Quick win自动命中的规则没有从当前请求里选择真实的
optionId。这里命中规则后直接返回固定的
'allow_always'/'reject_always'，完全没用request.options。只要后端给的 option id 不是这两个固定字符串，buildPermissionResponse()生成的就是无效选择。packages/ai-native/src/browser/acp/components/AcpChatInput.tsx-448-459 (1)
448-459:⚠️ Potential issue | 🟠 Major | ⚡ Quick win展开逻辑里有空指针崩溃风险。
document.querySelector('#ai_chat_left_container')可能返回null，当前ele!会在容器不存在时直接抛错，导致输入框无法展开。这里需要先判空再计算高度。可参考的调整
if (!expand) { - const ele = document.querySelector('`#ai_chat_left_container`'); - const maxHeight = ele!.clientHeight - 68 - (theme ? 32 : 0) - 16; + const ele = document.querySelector<HTMLElement>('`#ai_chat_left_container`'); + if (!ele) { + return; + } + const maxHeight = ele.clientHeight - 68 - (theme ? 32 : 0) - 16; setInputHeight(maxHeight); } else {🤖 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 `@packages/ai-native/src/browser/acp/components/AcpChatInput.tsx` around lines 448 - 459, The expand handler handleExpandClick uses document.querySelector('`#ai_chat_left_container`') and assumes the element exists via the non-null assertion (ele!), which can throw if the container is missing; update handleExpandClick to null-check the result of document.querySelector before accessing clientHeight and compute maxHeight only when ele is truthy, otherwise fall back to defaultHeight (or a safe height) and still toggle setIsExpand/setShowExpand accordingly; remove the non-null assertion and use the safe value when calling setInputHeight so expansion never crashes.packages/ai-native/src/browser/acp/components/AcpChatMentionInput.tsx-590-627 (1)
590-627:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
defaultModel的兜底值不在候选列表里。这里的兜底值是
deepseek-r1，但上面的候选值是DeepSeek-R1-0528/claude_sonnet4/qwen-plus-latest。在没有sessionModelId和偏好设置时，选择器会落到一个无效值，后续也可能把无效 model 透传下去。可参考的调整
defaultModel: - props.sessionModelId || preferenceService.get<string>(AINativeSettingSectionsId.ModelID) || 'deepseek-r1', + props.sessionModelId || + preferenceService.get<string>(AINativeSettingSectionsId.ModelID) || + 'DeepSeek-R1-0528',🤖 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 `@packages/ai-native/src/browser/acp/components/AcpChatMentionInput.tsx` around lines 590 - 627, The fallback defaultModel value 'deepseek-r1' is not present in the modelOptions values, causing an invalid selection; update the defaultModel logic in AcpChatMentionInput (the defaultModel expression) to use a valid option value (e.g., 'DeepSeek-R1-0528') or derive the fallback from modelOptions (e.g., modelOptions[0].value) and/or normalize incoming IDs (props.sessionModelId and preferenceService.get) to match option values; ensure any comparison/assignment uses the exact value strings found in modelOptions so the selector never receives an unknown model id.packages/ai-native/src/browser/acp/components/AcpChatMentionInput.tsx-148-155 (1)
148-155:⚠️ Potential issue | 🟠 Major | ⚡ Quick win切换到无默认输入的命令时需要清空
defaultInput。这里仅在
providerDefaultInput返回值时更新状态。从一个会预填内容的 slash 命令切到不提供默认输入的命令时，旧的预填文本会残留到下一次输入里。可参考的调整
+ setDefaultInput(''); if (findCommandHandler?.providerDefaultInput) { const editor = monacoCommandRegistry.getActiveCodeEditor(); Promise.resolve(findCommandHandler.providerDefaultInput(value, editor)).then((input) => { - if (input) { - setDefaultInput(input); - } + setDefaultInput(input || ''); }); }🤖 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 `@packages/ai-native/src/browser/acp/components/AcpChatMentionInput.tsx` around lines 148 - 155, 当切换到不提供默认输入的命令时需要清空默认输入；在 AcpChatMentionInput 里针对 findCommandHandler?.providerDefaultInput 的异步调用，如果 providerDefaultInput 不存在或返回 falsy 值，要明确调用 setDefaultInput('')（或 null 取决于现有 defaultInput 类型）以清除旧的预填文本。定位到使用 monacoCommandRegistry.getActiveCodeEditor() 并 Promise.resolve(findCommandHandler.providerDefaultInput(value, editor)).then(...) 的代码路径，修改 then 分支为：若返回值有内容则 setDefaultInput(input)，否则调用 setDefaultInput('')；同时在 findCommandHandler 不存在的情况下也应调用 setDefaultInput('') 以保证状态一致。packages/ai-native/src/browser/acp/components/AcpChatViewWrapper.tsx-166-180 (1)
166-180:⚠️ Potential issue | 🟠 Major | ⚡ Quick win轮询超时分支不会让 loading 结束。
这里达到上限后只再次
setInitState({ initialized: true })，但sessionReady仍然是false；而下面只有initialized && sessionReady才会渲染children。如果sessionModel一直没有准备好，界面会永久卡在 loading。修复方向
if (pollCount >= MAX_POLL_COUNT) { clearInterval(interval); - setInitState({ initialized: true }); + setTimedOut(true); }更稳妥的做法是把这里切到明确的失败/重试态，而不是重复设置已经为
true的initialized。🤖 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 `@packages/ai-native/src/browser/acp/components/AcpChatViewWrapper.tsx` around lines 166 - 180, The timeout branch only re-sets initialized and leaves sessionReady false, so loading never ends; modify the timeout handling in the polling block (referencing pollCount, MAX_POLL_COUNT, interval, aiChatService.sessionModel, setSessionReady, setInitState) to mark an explicit failure/retry state instead of just initialized — for example call setInitState({ initialized: true, failed: true }) (and clearInterval(interval) as already done); also ensure any waiting state is cleared (optionally call setSessionReady(false) if needed) and update the consumer logic to render a retry/error UI when initState.failed is true.packages/ai-native/src/browser/chat/chat-proxy.service.acp.ts-55-58 (1)
55-58:⚠️ Potential issue | 🟠 Major | ⚡ Quick win回退注册后未回写
agentDisposable，后续无法正确清理当前 agent。
registerFallbackAgent里注册了新 agent，但没有更新this.agentDisposable，再次切换时可能无法释放最新注册项。💡 建议修复
registerFallbackAgent(): void { this.agentDisposable?.dispose(); - this.addDispose(this.chatAgentService.registerAgent(this.defaultChatAgent)); + const disposable = this.chatAgentService.registerAgent(this.defaultChatAgent); + this.agentDisposable = disposable; + this.addDispose(disposable); queueMicrotask(() => { this.chatAgentService.updateAgent(ChatProxyService.AGENT_ID, {}); }); }🤖 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 `@packages/ai-native/src/browser/chat/chat-proxy.service.acp.ts` around lines 55 - 58, registerFallbackAgent currently disposes the previous agent but doesn't store the Disposable returned from chatAgentService.registerAgent, so subsequent switches can't clean up the newly registered agent; change the flow to capture the Disposable returned by chatAgentService.registerAgent(this.defaultChatAgent), assign it to this.agentDisposable, and then pass that same Disposable to this.addDispose (instead of calling addDispose on the call directly) so the instance field is kept up-to-date for future disposal; keep the existing initial dispose and queueMicrotask behavior in registerFallbackAgent.packages/ai-native/src/browser/chat/acp-chat-agent.ts-151-161 (1)
151-161:⚠️ Potential issue | 🟠 Major | ⚡ Quick win避免把
undefined消息写入history。首轮会话时
history可能为空，当前会把undefined包进数组并发送到后端，容易触发请求参数异常。💡 建议修复
- const lastmessage = history[history.length - 1]; + const lastmessage = history[history.length - 1]; ... - history: [lastmessage], + history: lastmessage ? [lastmessage] : [],🤖 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 `@packages/ai-native/src/browser/chat/acp-chat-agent.ts` around lines 151 - 161, The code may push an undefined lastmessage into the history sent to aiBackService.requestStream when history is empty; update the call that builds the history argument (used around lastmessage and the requestStream invocation) to only include lastmessage when it is defined (e.g., conditionally build history as [] or [lastmessage] or filter out falsy entries) so the backend never receives undefined entries.packages/ai-native/src/browser/chat/chat.view.acp.tsx-836-849 (1)
836-849:⚠️ Potential issue | 🟠 Major | ⚡ Quick win恢复历史时将可能为
undefined的request传入必填链路注释已说明从 storage 恢复时
request可能是undefined，但这里仍传给renderReply（其request为必填），后续访问request.requestId/子组件依赖时有运行时风险。🤖 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 `@packages/ai-native/src/browser/chat/chat.view.acp.tsx` around lines 836 - 849, The code fetches request via aiChatService.sessionModel?.getRequest(msg.requestId) but still unconditionally passes it to renderReply which expects a non-optional request, risking runtime errors; fix by guarding or normalizing before calling: either (A) only call renderReply when request is defined (move await renderReply inside the if that checks request and adjust setLoading accordingly), or (B) change renderReply’s API (and all callers) to accept request?: RequestType and handle undefined inside renderReply, and update any downstream uses of request.requestId to check for existence; locate aiChatService.sessionModel?.getRequest, the local request variable, and the renderReply invocation to implement one of these fixes.packages/ai-native/src/browser/chat/chat.internal.service.acp.ts-73-81 (1)
73-81:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
createSessionModel缺少失败兜底，可能导致加载态卡死
startSession()抛错时，_onSessionLoadingChange(false)不会触发，输入区可能一直被禁用。建议用try/finally包裹 loading 状态。建议修复
override async createSessionModel() { - this._onSessionLoadingChange.fire(true); - this._sessionModel = await this.chatManagerService.startSession(); - const acpManager = this.chatManagerService as AcpChatManagerService; - this.setAvailableCommands(acpManager.getAvailableCommands()); - this._onSessionModelChange.fire(this._sessionModel); - this._onChangeSession.fire(this._sessionModel.sessionId); - this._onSessionLoadingChange.fire(false); + this._onSessionLoadingChange.fire(true); + try { + this._sessionModel = await this.chatManagerService.startSession(); + const acpManager = this.chatManagerService as AcpChatManagerService; + this.setAvailableCommands(acpManager.getAvailableCommands()); + this._onSessionModelChange.fire(this._sessionModel); + this._onChangeSession.fire(this._sessionModel.sessionId); + } finally { + this._onSessionLoadingChange.fire(false); + } }🤖 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 `@packages/ai-native/src/browser/chat/chat.internal.service.acp.ts` around lines 73 - 81, createSessionModel currently sets _onSessionLoadingChange(true) then awaits chatManagerService.startSession() but never ensures _onSessionLoadingChange(false) on errors; wrap the session start and subsequent setup (assigning _sessionModel, casting to AcpChatManagerService, calling setAvailableCommands, firing _onSessionModelChange and _onChangeSession) in a try block and move the _onSessionLoadingChange.fire(false) into a finally so loading state is always cleared; optionally catch/log the error from startSession before rethrowing or handling to avoid silent failures.packages/ai-native/src/browser/chat/chat.view.acp.tsx-1042-1054 (1)
1042-1054:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
currentMessages可能为undefined，这里会直接崩溃
aiChatService.sessionModel?.history.getMessages()可能返回undefined，后续[...currentMessages]和currentMessages.map会抛异常。建议修复
- const currentMessages = aiChatService.sessionModel?.history.getMessages(); - const latestUserMessage = [...currentMessages].find((m) => m.role === ChatMessageRole.User); + const currentMessages = aiChatService.sessionModel?.history.getMessages() ?? []; + const latestUserMessage = [...currentMessages].reverse().find((m) => m.role === ChatMessageRole.User);🤖 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 `@packages/ai-native/src/browser/chat/chat.view.acp.tsx` around lines 1042 - 1054, currentMessages can be undefined from aiChatService.sessionModel?.history.getMessages(), causing [...currentMessages] and currentMessages.map to throw; guard the value before use by defaulting to an empty array (or early-return) when getMessages() returns undefined, e.g. obtain const currentMessages = aiChatService.sessionModel?.history.getMessages() ?? []; then use that safe array for finding latestUserMessage, calling cleanAttachedTextWrapper, setCurrentTitle(currentTitle) and building messages via currentMessages.map so latestUserMessage, setCurrentTitle and messages usage are protected from undefined.packages/ai-native/src/browser/chat/get-default-agent-type.ts-23-33 (1)
23-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick win缺少对非法
agentType的兜底，可能导致下游配置构建崩溃偏好里若出现未知
agentType，getAgentConfig()会返回undefined，下游对象展开会直接报错。建议修复
export function getDefaultAgentType(preferenceService: PreferenceService): ACPAgentType { - return preferenceService.get<ACPAgentType>('ai.native.agent.defaultType', DEFAULT_AGENT_TYPE); + const preferred = preferenceService.get<string>('ai.native.agent.defaultType', DEFAULT_AGENT_TYPE); + return (preferred in DEFAULT_AGENT_CONFIGS ? preferred : DEFAULT_AGENT_TYPE) as ACPAgentType; } export function getAgentConfig(preferenceService: PreferenceService, agentType: ACPAgentType): AgentConfig { const configs = preferenceService.get<Record<string, AgentConfig>>(AINativeSettingSectionsId.AgentConfigs, {}); - return configs[agentType] || DEFAULT_AGENT_CONFIGS[agentType]; + return configs[agentType] || DEFAULT_AGENT_CONFIGS[agentType] || DEFAULT_AGENT_CONFIGS[DEFAULT_AGENT_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 `@packages/ai-native/src/browser/chat/get-default-agent-type.ts` around lines 23 - 33, getAgentConfig currently assumes the supplied agentType maps to a config and can return undefined when preferenceService.get(AINativeSettingSectionsId.AgentConfigs, {}) doesn't contain that key; update getAgentConfig to validate the agentType (ACPAgentType) against the user configs and DEFAULT_AGENT_CONFIGS and provide a safe fallback (e.g. use DEFAULT_AGENT_CONFIGS[DEFAULT_AGENT_TYPE] or DEFAULT_AGENT_CONFIGS[agentType] if present) so the function never returns undefined; reference getAgentConfig, preferenceService.get, AINativeSettingSectionsId.AgentConfigs, DEFAULT_AGENT_CONFIGS and DEFAULT_AGENT_TYPE to locate where to add the check and fallback.packages/ai-native/src/browser/chat/chat.view.acp.tsx-995-1000 (1)
995-1000:⚠️ Potential issue | 🟠 Major | ⚡ Quick win异步错误不会被当前
try/catch捕获
aiChatService.createSessionModel()是 async，未await时 reject 不会进入catch，会产生未处理 Promise 错误。建议修复
- const handleNewChat = React.useCallback(() => { + const handleNewChat = React.useCallback(async () => { if (aiChatService.sessionModel?.history.getMessages().length > 0) { try { - aiChatService.createSessionModel(); + await aiChatService.createSessionModel(); } catch (error) { - messageService.error(error.message); + messageService.error((error as Error).message); } } }, [aiChatService]);🤖 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 `@packages/ai-native/src/browser/chat/chat.view.acp.tsx` around lines 995 - 1000, The try/catch won’t catch rejections because aiChatService.createSessionModel() returns a Promise; change the call so the rejection is handled (either await aiChatService.createSessionModel() from an async function or attach .catch(error => messageService.error(error.message))), keeping the existing messageService.error usage; locate the call to aiChatService.createSessionModel and update it to await the promise (or use .then/.catch) and ensure the surrounding handler is async or error-handling is attached so rejected promises are not left unhandled.packages/ai-native/src/browser/chat/pick-workspace-dir.ts-57-69 (1)
57-69:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
roots数组为空时访问roots[0]会导致运行时错误当
isMultiRootWorkspaceOpened为 true 但tryGetRoots()返回空数组时，Line 67 访问roots[0]会抛出异常。🐛 建议修复：添加空数组检查
if (workspaceService.isMultiRootWorkspaceOpened) { const roots = workspaceService.tryGetRoots(); + if (roots.length === 0) { + return ''; + } const choose = await quickPick.show( roots.map((file) => new URI(file.uri).codeUri.fsPath), { placeholder: localize('chat.selectCWDForACP') }, ); if (choose) { return choose; } // User cancelled: fall back to first root and notify const fallback = new URI(roots[0].uri).codeUri.fsPath;🤖 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 `@packages/ai-native/src/browser/chat/pick-workspace-dir.ts` around lines 57 - 69, When isMultiRootWorkspaceOpened is true but workspaceService.tryGetRoots() returns an empty array, roots[0] access will throw; add a guard after obtaining roots (check roots.length === 0) and handle that case by notifying the user via messageService.info/messageService.error using a localized message (e.g. formatLocalize('chat.noWorkspaceRoots')) and returning a safe fallback (e.g. process.cwd() or an empty string) instead of accessing roots[0]; update the block around workspaceService.tryGetRoots(), quickPick.show, and the fallback logic accordingly so you never index into roots when it is empty.packages/ai-native/src/browser/components/ChatHistory.acp.tsx-184-193 (1)
184-193:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnter 提交后 onBlur 会触发取消，存在竞态条件
当用户按下 Enter 键时，
onPressEnter先执行handleTitleEditComplete，然后 Input 失去焦点触发onBlur，调用handleTitleEditCancel。这会导致编辑完成后状态被覆盖。🐛 建议修复：在 onBlur 中检查是否已完成编辑
+const editCompletedRef = useRef<Record<string, boolean>>({}); + const handleTitleEditComplete = useCallback( (item: IChatHistoryItem, newTitle: string) => { + editCompletedRef.current[item.id] = true; setHistoryTitleEditable({ [item.id]: false, }); onHistoryItemChange(item, newTitle); + // 重置标记 + setTimeout(() => { + editCompletedRef.current[item.id] = false; + }, 0); }, [onHistoryItemChange], ); const handleTitleEditCancel = useCallback( (item: IChatHistoryItem) => { + // 如果已通过 Enter 完成编辑，跳过取消 + if (editCompletedRef.current[item.id]) { + return; + } setHistoryTitleEditable({ [item.id]: false, }); }, [], );🤖 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 `@packages/ai-native/src/browser/components/ChatHistory.acp.tsx` around lines 184 - 193, The Input's onBlur races with onPressEnter: onPressEnter calls handleTitleEditComplete then blur fires and calls handleTitleEditCancel, overwriting the completed edit; to fix, add a short-lived "commit" guard that handleTitleEditComplete sets (e.g., editingCommitted boolean on component state or inputRef) and have onBlur first check that guard and no-op if editingCommitted is true, then clear the guard after handling; update references around the Input, handleTitleEditComplete and handleTitleEditCancel to use this guard so blur doesn't cancel a just-committed edit.packages/ai-native/src/browser/components/acp/MentionInput.tsx-807-864 (1)
807-864:⚠️ Potential issue | 🟠 Major | ⚡ Quick win粘贴处理中潜在的 HTML 注入风险
Line 829-833 中，
processedLine通过字符串替换生成 HTML，然后使用innerHTML赋值。虽然已经调用了getData('text/plain')，但如果粘贴的纯文本包含<script>或其他 HTML 标签，仍会被解释为 HTML。🛡️ 建议使用 textContent 或转义 HTML
lines.forEach((line, index) => { - // 处理行首空格，将每个空格转换为 - const processedLine = line.replace(/^[ ]+/g, (match) => { - const span = document.createElement('span'); - span.innerHTML = ' '.repeat(match.length); - return span.innerHTML; - }); - - // 创建一个临时容器来保持 HTML 内容 - const container = document.createElement('span'); - container.innerHTML = processedLine; - - // 将容器的内容添加到文档片段 - while (container.firstChild) { - fragment.appendChild(container.firstChild); - } + // 处理行首空格 + const leadingSpaces = line.match(/^[ ]+/); + if (leadingSpaces) { + // 为行首空格创建不间断空格 + const spaceSpan = document.createElement('span'); + spaceSpan.textContent = '\u00A0'.repeat(leadingSpaces[0].length); + fragment.appendChild(spaceSpan); + // 添加剩余文本 + const textNode = document.createTextNode(line.slice(leadingSpaces[0].length)); + fragment.appendChild(textNode); + } else { + const textNode = document.createTextNode(line); + fragment.appendChild(textNode); + } // 如果不是最后一行，添加换行符 if (index < lines.length - 1) { fragment.appendChild(document.createElement('br')); } });🤖 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 `@packages/ai-native/src/browser/components/acp/MentionInput.tsx` around lines 807 - 864, The paste handler currently assigns HTML via container.innerHTML using processedLine (lines ~829–833), which risks HTML injection; instead build DOM using text nodes and textContent: replace the innerHTML usage by creating text nodes (document.createTextNode) or set span.textContent for leading-space spans (use '\u00A0' repeated) so pasted content is treated as plain text, append those nodes to fragment, keep the existing logic for <br> insertion and cursor movement (range, fragment, lastNode) and then call handleInput() as before.packages/ai-native/src/browser/contrib/terminal/ai-terminal.service.ts-188-192 (1)
188-192:⚠️ Potential issue | 🟠 Major | ⚡ Quick win点击不同操作时传入了“当前匹配器”，可能与目标 handler 不匹配。
现在
id已按点击项分发，但execute仍固定用action.matcher。点击非当前匹配 action 时，这个参数可能错误。💡建议修改
onClickItem: (id: string) => { const handler = this.inlineChatFeatureRegistry.getTerminalHandler(id); if (handler) { - handler.execute(output, input || '', action.matcher); + handler.execute(output, input || '', id === action.action.id ? action.matcher : undefined); } },🤖 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 `@packages/ai-native/src/browser/contrib/terminal/ai-terminal.service.ts` around lines 188 - 192, The click handler is passing the outer scope action.matcher into handler.execute which can be the wrong matcher for the clicked item; update the call in onClickItem to pass the matcher associated with the clicked item/handler instead of action.matcher (e.g., use the matcher property or lookup on the returned handler from inlineChatFeatureRegistry.getTerminalHandler(id) or perform a matcher lookup by id) so handler.execute(output, input || '', handlerMatcher) is invoked with the correct matcher for that handler.packages/ai-native/src/browser/components/permission-dialog-widget.module.less-69-69 (1)
69-69:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
word-break: break-word已被弃用，需替换。将其改为
overflow-wrap: anywhere;以保持断行表现同时符合现代 CSS 标准。🤖 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 `@packages/ai-native/src/browser/components/permission-dialog-widget.module.less` at line 69, Replace the deprecated CSS property `word-break: break-word` in permission-dialog-widget.module.less with the modern equivalent `overflow-wrap: anywhere;` in the same selector(s) to preserve line-breaking behavior while complying with current CSS standards.packages/ai-native/src/node/acp/acp-agent.service.ts-404-419 (1)
404-419:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift不要用单个
currentNotificationHandler覆盖并发请求。新请求会直接覆盖旧的 handler；更糟的是，旧 stream 结束时还会把当前字段清空。结果是并发 session/request 时，
dispose()只能回收最后一次记录的订阅，较早的通知订阅会泄漏，清理也可能串到错误的 stream。建议至少按sessionId/request 维度跟踪 handler，并在 cleanup 时按引用删除。🤖 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 `@packages/ai-native/src/node/acp/acp-agent.service.ts` around lines 404 - 419, The code currently overwrites a single currentNotificationHandler for concurrent requests (symbols: currentNotificationHandler, unsubscribe, stream.onEnd, stream.onError, request.sessionId), causing leaked/incorrect cleanup; change to track handlers in a map keyed by sessionId or request id (e.g., notificationHandlers: Map<string, Handler>), store the handler under that key instead of assigning currentNotificationHandler, and in stream.onEnd/stream.onError call the handler's unsubscribe and then delete only that key from the map (do not null a global field). Also update dispose() to accept a session/request key or to iterate the map to remove/unsubscribe specific entries so cleanup is per-session/request rather than single global replacement.packages/ai-native/src/node/acp/handlers/terminal.handler.ts-95-99 (1)
95-99:⚠️ Potential issue | 🟠 Major | ⚡ Quick win不要把原始命令和参数直接打到日志里。
这里会把
command、args、cwd原样写入 node 日志，权限拒绝分支也一样。终端命令里很容易带 token、密钥、访问路径等敏感信息，落盘后就是长期泄露面。建议只记录sessionId/terminalId/ allow-deny 结果，必要时做显式脱敏。Also applies to: 107-127, 135-139
🤖 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 `@packages/ai-native/src/node/acp/handlers/terminal.handler.ts` around lines 95 - 99, The handler is logging raw terminal inputs (command/args/cwd) which may contain secrets; update AcpTerminalHandler logging in createTerminal (and the similar blocks at the ranges around 107-127 and 135-139) to stop emitting the raw command, args or cwd and instead log only safe identifiers and outcomes (e.g., request.sessionId, terminalId when created, and the allow/deny decision); if detailed context is required, perform explicit redaction/sanitization before logging (never JSON.stringify(raw args/command) into logs) and ensure logger calls reflect only the sanitized values and the decision/result.packages/ai-native/src/node/acp/acp-cli-back.service.ts-281-318 (1)
281-318:⚠️ Potential issue | 🟠 Major | ⚡ Quick win历史回放把 message chunk 当成了完整消息。
user_message_chunk/agent_message_chunk从命名和实时流逻辑看都是分片；这里直接push会把一次历史回复拆成很多条消息。加载旧 session 后，聊天记录会碎片化，后续摘要、重放和 UI 展示都会失真。更稳妥的做法是按 message id 聚合；如果协议里没有 id，至少要把连续同角色的 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 `@packages/ai-native/src/node/acp/acp-cli-back.service.ts` around lines 281 - 318, convertSessionUpdatesToMessages currently treats 'user_message_chunk' and 'agent_message_chunk' as full messages causing fragmented history; update it to aggregate chunks into complete messages by grouping chunks that share the same message identifier (e.g. update.message_id or update.id) and concatenating their text parts before pushing a single message; if no message id exists, merge consecutive chunks with the same role (user vs assistant) by appending content.text and using the earliest timestamp for the combined message; refer to convertSessionUpdatesToMessages, SessionNotification, and the update.sessionUpdate values 'user_message_chunk' / 'agent_message_chunk' when implementing the aggregation.packages/ai-native/src/node/acp/acp-cli-back.service.ts-119-123 (1)
119-123:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
createSession()首次调用会额外创建一个隐藏 session。冷启动时这里先走
ensureAgentInitialized()，而initializeAgent()内部已经newSession()过一次；随后又调用agentService.createSession()再开一个。这样首个真实会话会多耗一次 agent 资源，而且sessionInfo还会停留在那个没有返回给调用方的 session 上。🐛 建议修改
async createSession( config: AgentProcessConfig, ): Promise<{ sessionId: string; availableCommands: AvailableCommand[] }> { - await this.ensureAgentInitialized(config); return this.agentService.createSession(config); }Also applies to: 126-132
packages/ai-native/src/common/prompts/empty-prompt-provider.ts-20-27 (1)
20-27:⚠️ Potential issue | 🟠 Major | ⚡ Quick win无上下文分支的条件写反了。
这里在“没有任何上下文”时，只要当前编辑器里有文件，就会继续走
buildACPPrompt()并追加Current filesection；这和类注释里的“直接返回userMessage”相反，也会把未显式附加的文件内容隐式带进 ACP prompt。🐛 建议修改
async provideContextPrompt(context: SerializedContext, userMessage: string): Promise<string> { const hasContextFields = context.globalRules.length > 0 || context.attachedFolders.length > 0 || context.attachedFiles.length > 0 || context.attachedRules.length > 0; if (!hasContextFields) { - const currentFileInfo = await this.getACPCurrentFileInfo(); - const hasCurrentFile = - currentFileInfo && !context.attachedFiles.some((file) => file.path === currentFileInfo.path); - if (!hasCurrentFile) { - return userMessage; - } + return userMessage; } return this.buildACPPrompt(context, userMessage); }🤖 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 `@packages/ai-native/src/common/prompts/empty-prompt-provider.ts` around lines 20 - 27, The no-context branch logic is inverted: inside the block that checks hasContextFields you fetch currentFileInfo via getACPCurrentFileInfo() and compute hasCurrentFile, but you return userMessage when there is NO current file; instead return early when there IS a current file so you don't implicitly include it in buildACPPrompt(). Change the inner condition to return userMessage when hasCurrentFile is truthy (i.e. invert the if (!hasCurrentFile) to if (hasCurrentFile)), keeping the currentFileInfo and attachedFiles check using context.attachedFiles as-is.packages/startup/entry/sample-modules/ai-native/ai-native.contribution.ts-616-616 (1)
616-616:⚠️ Potential issue | 🟠 Major | ⚡ Quick win这里缺少空值保护，可能触发运行时异常。
Line [616] 直接读取
capabilities.supportsAgentMode，当配置未提供capabilities时会抛错。🛡️ 建议修改
- if (this.aiNativeConfigService.capabilities.supportsAgentMode) { + if (this.aiNativeConfigService.capabilities?.supportsAgentMode) {🤖 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 `@packages/startup/entry/sample-modules/ai-native/ai-native.contribution.ts` at line 616, The code reads this.aiNativeConfigService.capabilities.supportsAgentMode without null checks; change the conditional to guard against missing capabilities (e.g., use optional chaining or an explicit check) so it becomes this.aiNativeConfigService.capabilities?.supportsAgentMode or (this.aiNativeConfigService.capabilities && this.aiNativeConfigService.capabilities.supportsAgentMode) to avoid runtime exceptions when capabilities is undefined; update the conditional at the if that references supportsAgentMode in ai-native.contribution.ts accordingly.packages/startup/entry/sample-modules/ai-native/WelcomePage.tsx-33-49 (1)
33-49:⚠️ Potential issue | 🟠 Major | ⚡ Quick win示例卡片用
div + onClick违反无障碍访问标准，阻断键盘导航第 34、38、42、46 行的四个示例卡片当前仅支持鼠标交互，键盘用户和屏幕阅读器用户无法等价访问。建议改为语义化
<button>元素，既提升无障碍性，也无需额外处理键盘事件。修复建议
- <div className={styles.sample_card} onClick={() => handleSampleClick('Explain my code')}> + <button + type="button" + className={styles.sample_card} + onClick={() => handleSampleClick('Explain my code')} + > <Icon className={getIcon('search')} /> <span>Explain my code</span> - </div> + </button>同样修改第 38、42、46 行的其他三个卡片。现有 CSS 样式与
<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 `@packages/startup/entry/sample-modules/ai-native/WelcomePage.tsx` around lines 33 - 49, The sample cards in WelcomePage.tsx use non-semantic <div> with onClick (elements using styles.sample_card and invoking handleSampleClick via Icon/getIcon) which breaks keyboard and screen-reader accessibility; replace each clickable <div> with a semantic <button> (preserving styles.sample_card, Icon and the onClick handler to call handleSampleClick) so keyboard activation and focus handling work out-of-the-box and no extra key handlers or CSS changes are required.
🧹 Nitpick comments (7)
packages/ai-native/src/browser/acp/components/AcpChatInput.tsx (1)
399-405: ⚡ Quick win不要在
useMemo里触发状态更新。这里在 render 过程中调用
setIsShowOptions(false)，在 React 严格模式下很容易变成重复 render 或难定位的警告。把“展开时关闭选项”的副作用移到useEffect，useMemo只保留纯计算即可。可参考的调整
+ useEffect(() => { + if (isExpand) { + setIsShowOptions(false); + } + }, [isExpand]); + const optionsBottomPosition = useMemo(() => { - const customBottom = INSTRUCTION_BOTTOM + inputHeight; - if (isExpand) { - setIsShowOptions(false); - } - return customBottom; + return INSTRUCTION_BOTTOM + inputHeight; }, [inputHeight]);🤖 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 `@packages/ai-native/src/browser/acp/components/AcpChatInput.tsx` around lines 399 - 405, The optionsBottomPosition useMemo in AcpChatInput performs a state update (calls setIsShowOptions(false)) during render; move that side effect into a useEffect that watches isExpand (and inputHeight if needed) and only keep pure computation inside useMemo (optionsBottomPosition = compute customBottom using INSTRUCTION_BOTTOM + inputHeight). Replace the setIsShowOptions call from the useMemo block with a separate useEffect that triggers when isExpand becomes true and invokes setIsShowOptions(false).packages/ai-native/src/browser/components/acp/MentionInput.tsx (2)
148-162: 💤 Low value自定义 Hook 定义在组件内部，每次渲染都会重新创建
useDebouncehook 定义在组件函数体内，这意味着每次渲染都会创建新的 hook 定义。虽然 React 会保持 hook 调用顺序的一致性，但这种模式不符合 React Hooks 的最佳实践。♻️ 建议将 useDebounce 提取到组件外部
+// 移到文件顶部，组件外部 +const useDebounce = <T,>(value: T, delay: number): T => { + const [debouncedValue, setDebouncedValue] = React.useState<T>(value); + + React.useEffect(() => { + const handler = setTimeout(() => { + setDebouncedValue(value); + }, delay); + + return () => { + clearTimeout(handler); + }; + }, [value, delay]); + + return debouncedValue; +}; export const MentionInput: React.FC<...> = ({...}) => { - const useDebounce = <T,>(value: T, delay: number): T => { - // ... - };🤖 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 `@packages/ai-native/src/browser/components/acp/MentionInput.tsx` around lines 148 - 162, The custom hook useDebounce is currently defined inside the MentionInput component causing it to be recreated on each render; move the useDebounce<T>(value: T, delay: number): T function to the module/top-level (outside the MentionInput component) so it is defined once, preserve its generic signature and React.useEffect/React.useState usage, and update any references inside MentionInput to call the top-level useDebounce; ensure React is imported where the hook now lives and export the hook if other modules need it.
1281-1287: 💤 Low valueuseCallback 依赖包含未使用的
selectedModel
handleModelChange回调的依赖数组包含selectedModel，但函数体内只使用了setSelectedModel和onSelectionChange。♻️ 移除多余依赖
const handleModelChange = React.useCallback( (value: string) => { setSelectedModel(value); onSelectionChange?.(value); }, - [selectedModel, onSelectionChange], + [onSelectionChange], );🤖 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 `@packages/ai-native/src/browser/components/acp/MentionInput.tsx` around lines 1281 - 1287, The useCallback for handleModelChange lists selectedModel in its dependency array even though the callback only uses setSelectedModel and onSelectionChange; remove selectedModel from the dependency array so it becomes [onSelectionChange] (or include only the actual external values used) to avoid an unnecessary dependency and potential stale/misleading lint warnings; update the dependency list for handleModelChange accordingly while keeping setSelectedModel (state setter) omitted as it's stable.packages/ai-native/src/browser/components/ChatHistory.acp.tsx (1)
53-57: 💤 Low valueuseCallback 依赖数组包含未使用的变量
多个
useCallback的依赖数组包含未实际使用的状态变量，这会导致不必要的函数重建。♻️ 建议清理依赖数组
const handleSearchChange = useCallback( (event: React.ChangeEvent<HTMLInputElement>) => { setSearchValue(event.target.value); }, - [searchValue], + [], ); const handleHistoryItemSelect = useCallback( (item: IChatHistoryItem) => { onHistoryItemSelect(item); setSearchValue(''); }, - [onHistoryItemSelect, searchValue], + [onHistoryItemSelect], ); const handleTitleEdit = useCallback( (item: IChatHistoryItem) => { setHistoryTitleEditable({ [item.id]: true, }); }, - [historyTitleEditable], + [], );Also applies to: 61-66, 70-76, 80-88, 91-98
🤖 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 `@packages/ai-native/src/browser/components/ChatHistory.acp.tsx` around lines 53 - 57, The useCallback hooks include unused state variables in their dependency arrays causing needless remounts; for each callback (e.g., handleSearchChange) remove unused dependencies (like searchValue) and keep only variables actually referenced inside the function (for example setSearchValue or external handlers such as onSearch/onClear); update the other callbacks called out (handleSearchClear, handleSearchSubmit, handleToggleMessages, handleSelectMessage) the same way so each useCallback's dependency array contains only the symbols it reads.packages/ai-native/src/browser/preferences/schema.ts (1)
170-195: ⚡ Quick win
AgentConfigs建议补充必填约束，避免无效配置进入运行期。建议把
command设为必填字段，尽早在配置校验阶段拦截错误。💡建议修改
additionalProperties: { type: 'object', + required: ['command'], properties: { command: { type: 'string',🤖 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 `@packages/ai-native/src/browser/preferences/schema.ts` around lines 170 - 195, The schema currently allows agent config objects without a command, so add a required constraint to the object schema to force callers to provide a command; specifically, update the additionalProperties object definition in the preferences schema (the object that defines properties command, args, streaming, description) to include required: ['command'] so validation fails early for missing command in AgentConfigs.packages/core-common/src/types/ai-native/acp-types.ts (1)
1-35: 🏗️ Heavy lift缩小
@ts-nocheck的作用范围，防止隐藏本地代码的类型错误。第 1 行的文件级
@ts-nocheck会禁用整个文件的类型检查，包括本地类型定义和接口。建议仅在 SDK 桥接的 import 和 export 语句处添加@ts-expect-error TS1479，保留其余代码的类型安全检查。文件包含多个来自
@agentclientprotocol/sdk的导入/导出语句（第 26 行和第 87 行），需要在各自位置添加定点抑制。🤖 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 `@packages/core-common/src/types/ai-native/acp-types.ts` around lines 1 - 35, Remove the file-level "// `@ts-nocheck`" and instead apply targeted suppression only where interacting with the ESM-only SDK: place "// `@ts-expect-error` TS1479" immediately above the import list from '`@agentclientprotocol/sdk`' and above any re-export lines that reference '`@agentclientprotocol/sdk`' (the CJS-compatible re-export bridge and the specific export statements). This preserves full TypeScript checking for local types/interfaces while silencing only the TS1479 diagnostics for the SDK import/export operations.tools/playwright/src/tests/search-view.test.ts (1)
88-88: ⚡ Quick win避免固定 sleep，改为条件等待提升稳定性。
Line 88 的
waitForTimeout(1000)容易在慢机/快机下分别导致不稳定或浪费时间。建议用expect.poll()等待树节点加载完成。⏱️ 建议修改
- await app.page.waitForTimeout(1000); + await expect + .poll(async () => { + const node = await search.getTreeNodeByIndex(1); + return !!node; + }) + .toBeTruthy();🤖 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 `@tools/playwright/src/tests/search-view.test.ts` at line 88, 在 search-view.test.ts 中不要使用硬编码 sleep 调用 app.page.waitForTimeout(1000); 改为基于条件的等待（例如使用 expect.poll 或 page.waitForSelector/locator.waitFor）来等待树节点渲染完成：定位到代表树节点的选择器（使用同一测试内的 locator/selector 逻辑）并用 expect.poll(() => /* 查询节点存在或节点数量 */) 或 page.waitForSelector(selector, { state: 'visible' }) 替换该 sleep，以提升稳定性并避免在快机/慢机上出错或浪费时间。
The else branch in getItems was using a shadowed const folders (string[]) instead of the outer MentionItem[] variable, causing a type error when passed to expandFolderPaths which expects string[]. Renamed the intermediate variable to folderPaths and assigned the expanded result to folders. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ChatReply: when condition was inverted — button was always rendered and then overwritten. Now properly checks !when or when matches before rendering. Also adds missing AgentProcessConfig import in test. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@packages/ai-native/src/browser/components/acp/ChatReply.tsx`:
- Around line 366-398: The followupNode useMemo builds an array with null/false
entries so an empty container can still render; update the React.useMemo for
followupNode (which iterates request.response.followups) to filter out falsy
nodes before returning (e.g., build nodes via .reduce or
.map(...).filter(Boolean)) and return null when the resulting nodes array is
empty; keep existing behavior for reply branch (chatApiService.sendMessage /
chatAgentService.parseMessage) and the when-check (contextKeyService.match) but
ensure the final return is either a non-empty array of Fragment-wrapped nodes or
null.
- Around line 257-271: In ChatReply.tsx inside the block handling
request.response.isComplete (where aiReporter.end is called), change the
hard-coded success: true to reflect whether the response actually failed by
checking request.response.errorDetails (or similar error flag used elsewhere,
e.g., errorDetails?.message) and set success: false when an error exists; also
ensure assistantMessage uses the appropriate text (responseText or
errorDetails.message) and keep the existing fields (replytime, isStop, command,
agentId, messageId, sessionId) unchanged when calling aiReporter.end.
- Around line 445-449: The custom chat AIRole renderer is being passed
chunk.content (the whole object) while the main markdown renderer uses
markdown.value; update the chatAIRoleRender invocation
(chatRenderRegistry.chatAIRoleRender / ChatAIRoleRender) to receive the same
value shape as the main path (pass markdown.value or the same property used by
renderMarkdown/ChatMarkdown) so custom renderers get the consistent string input
instead of the full chunk.content object.
- Around line 197-199: The deferred (async) branch that resolves the component
via chatAgentViewService.getChatComponentDeferred(props.component) doesn't pass
props.messageId into the mounted component, causing inconsistent props versus
the synchronous branch; update the deferred.promise.then handler (the callback
that receives { component: Component, initialProps }) to include messageId when
calling setNode — i.e., pass {...initialProps, value: props.value, messageId:
props.messageId} (or merge messageId into initialProps) so the component
receives the same props in both paths.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3c797867-ba99-456c-86d8-db663fac5474
📒 Files selected for processing (3)
packages/ai-native/__test__/node/acp-cli-back.test.tspackages/ai-native/src/browser/components/ChatMentionInput.acp.tsxpackages/ai-native/src/browser/components/acp/ChatReply.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/ai-native/test/node/acp-cli-back.test.ts
- packages/ai-native/src/browser/components/ChatMentionInput.acp.tsx
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1779092698.0 |
Add 242 tests across 8 test suites covering: - acp-cli-client.service.ts (NDJSON transport, JSON-RPC) - acp-cli-process-manager.ts (process lifecycle) - acp-permission-caller.service.ts (permission routing, option sorting) - acp-agent.service.ts (session management, notifications) - agent-request.handler.ts (request routing delegation) - file-system.handler.ts (file ops, workspace sandboxing) - terminal.handler.ts (PTY terminal lifecycle) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The async branch of getChatComponentDeferred omitted messageId, causing inconsistent props compared to the synchronous path. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Member
|
👍👍👍 |
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Remove unused imports (AcpAgentServiceToken, SimpleMessage) - Remove unused mockStream variable - Fix property shorthand lint error - Add tests for history, images, error handling, and content conversion Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
harry seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
1 similar comment
|
harry seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Contributor
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 `@packages/ai-native/src/browser/mcp/tools/fileSearch.ts`:
- Line 76: The call to preferenceService.get used for the followSymlinks option
lacks an explicit generic type, allowing non-boolean values to propagate into
the search `find` parameters; update the call in fileSearch.ts to use
preferenceService.get<boolean> for the followSymlinks retrieval (the symbol to
change is the expression using followSymlinks and preferenceService.get) so the
returned value is typed as boolean and then fall back to the default true when
the getter returns undefined.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6731ae55-be22-405b-8365-d4192c7fcf39
📒 Files selected for processing (13)
packages/addons/src/browser/file-search.contribution.tspackages/ai-native/src/browser/mcp/tools/fileSearch.tspackages/ai-native/src/browser/mcp/tools/grepSearch.tspackages/core-common/src/settings/search.tspackages/extension/src/browser/vscode/api/main.thread.workspace.tspackages/file-search/src/common/file-search.tspackages/file-search/src/node/file-search.service.tspackages/i18n/src/common/en-US.lang.tspackages/i18n/src/common/zh-CN.lang.tspackages/search/src/browser/search-preferences.tspackages/search/src/browser/search.service.tspackages/search/src/common/content-search.tspackages/search/src/node/content-search.service.ts
| useGitIgnore: true, | ||
| noIgnoreParent: true, | ||
| fuzzyMatch: true, | ||
| followSymlinks: this.preferenceService.get('search.followSymlinks') ?? true, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
请帮我检索 OpenSumi 当前版本中 PreferenceService.get 的类型签名（返回值类型与泛型约束），并确认 search.followSymlinks 的偏好项声明类型是否为 boolean。
💡 Result:
在 OpenSumi 当前版本中，PreferenceService.get 的类型签名如下 [1]： get(preferenceName: string, defaultValue?: T, resourceUri?: string, overrideIdentifier?: string): T | undefined; 该方法使用泛型 T 来约束返回值类型，若未提供默认值且未找到对应配置，则返回 undefined [1]。 关于 search.followSymlinks 偏好项，OpenSumi 的偏好设置系统与 VS Code 高度兼容 [2]。在 OpenSumi 中，search.followSymlinks 的声明类型确实为 boolean [3][4]。该配置项用于控制在进行文件搜索时是否跟随符号链接（symbolic links）[4]。
Citations:
- 1: https://www.npmjs.com/package/@opensumi/ide-preferences
- 2: https://opensumi.com/en/docs/develop/basic-design/core-idea/
- 3: Search results show duplicate files when "search.followSymlinks" is set to false in Visual Studio Code 1.77.3 microsoft/vscode#180087
- 4: https://www.youtube.com/watch?v=ONctjCQUk4Y
🏁 Script executed:
#!/bin/bash
# Find and examine the fileSearch.ts file
find . -name "fileSearch.ts" -type fRepository: opensumi/core
Length of output: 114
🏁 Script executed:
#!/bin/bash
# Look at the specific file around line 76
if [ -f "packages/ai-native/src/browser/mcp/tools/fileSearch.ts" ]; then
wc -l packages/ai-native/src/browser/mcp/tools/fileSearch.ts
fiRepository: opensumi/core
Length of output: 116
🏁 Script executed:
#!/bin/bash
# Read the file content around line 76
if [ -f "packages/ai-native/src/browser/mcp/tools/fileSearch.ts" ]; then
sed -n '70,85p' packages/ai-native/src/browser/mcp/tools/fileSearch.ts
fiRepository: opensumi/core
Length of output: 587
🏁 Script executed:
#!/bin/bash
# Search for all preferenceService.get calls in the file
if [ -f "packages/ai-native/src/browser/mcp/tools/fileSearch.ts" ]; then
rg "preferenceService\.get" packages/ai-native/src/browser/mcp/tools/fileSearch.ts -n
fiRepository: opensumi/core
Length of output: 143
🏁 Script executed:
#!/bin/bash
# Check for PreferenceService interface definition and get<T> signature
rg "interface.*PreferenceService|get<\w+>" --type typescript -A 2 | head -50Repository: opensumi/core
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Get all preferenceService.get calls with context from the file
sed -n '1,118p' packages/ai-native/src/browser/mcp/tools/fileSearch.ts | grep -n "preferenceService"Repository: opensumi/core
Length of output: 203
🏁 Script executed:
#!/bin/bash
# Read the full file to understand the context better
cat -n packages/ai-native/src/browser/mcp/tools/fileSearch.ts | head -80Repository: opensumi/core
Length of output: 3881
🏁 Script executed:
#!/bin/bash
# Search for PreferenceService interface definition
rg "interface PreferenceService" -A 10 -t ts -t tsxRepository: opensumi/core
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Search for get method in PreferenceService across codebase
rg "get\s*<\w+>\s*\(" --type ts --type tsx | head -20Repository: opensumi/core
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Search for PreferenceService interface definition
rg "interface PreferenceService" -A 15Repository: opensumi/core
Length of output: 1215
🏁 Script executed:
#!/bin/bash
# Search for get method in PreferenceService
rg "get\s*<" -A 2 | head -40Repository: opensumi/core
Length of output: 3358
🏁 Script executed:
#!/bin/bash
# Look for other get calls with type parameter in the codebase
rg "\.get<" | head -20Repository: opensumi/core
Length of output: 2935
🏁 Script executed:
#!/bin/bash
# Check how preferenceService.get is typically used in similar files
rg "preferenceService\.get\(" --context 2 | head -50Repository: opensumi/core
Length of output: 4948
🏁 Script executed:
#!/bin/bash
# Get the full PreferenceService interface including get method signature
sed -n '1,100p' packages/core-browser/src/preferences/types.tsRepository: opensumi/core
Length of output: 2869
🏁 Script executed:
#!/bin/bash
# Look for more of the get method signature
rg "get\(" packages/core-browser/src/preferences/types.ts -A 10Repository: opensumi/core
Length of output: 39
🏁 Script executed:
#!/antml/bash
# Search for Boolean() wrapping in preference handling patterns
rg "Boolean\(" packages/scm/src/browser/scm-model.ts -B 2 -A 2Repository: opensumi/core
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Check if there are any other follow-up usages of get without type parameter
rg "\.get\(" packages/ --type-not md | grep -v "\.get<" | head -30Repository: opensumi/core
Length of output: 3253
🏁 Script executed:
#!/bin/bash
# Check the actual usage pattern with type guards/conversions
rg "!!.*preferenceService\.get" -B 1 -A 1Repository: opensumi/core
Length of output: 2033
🏁 Script executed:
#!/bin/bash
# Look for Boolean() wrapper patterns in the codebase
rg "Boolean\(" packages/ -B 2 -A 2 | head -50Repository: opensumi/core
Length of output: 3345
🏁 Script executed:
#!/bin/bash
# Check other search-related preference usage patterns
rg "search\." packages/ | grep preferenceService | head -20Repository: opensumi/core
Length of output: 632
🏁 Script executed:
#!/bin/bash
# Verify the search.followSymlinks preference schema/declaration
rg "search\.followSymlinks" -B 5 -A 5Repository: opensumi/core
Length of output: 9047
为 followSymlinks 添加显式的泛型类型约束。
Line 76 当前缺少泛型类型参数，与同 PR 中 packages/addons/src/browser/file-search.contribution.ts 的调用点不一致。建议改为 get<boolean> 以确保类型安全，防止偏好层异常值透传到 find 参数。
建议修改
- followSymlinks: this.preferenceService.get('search.followSymlinks') ?? true,
+ followSymlinks: this.preferenceService.get<boolean>('search.followSymlinks') ?? true,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| followSymlinks: this.preferenceService.get('search.followSymlinks') ?? true, | |
| followSymlinks: this.preferenceService.get<boolean>('search.followSymlinks') ?? true, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ai-native/src/browser/mcp/tools/fileSearch.ts` at line 76, The call
to preferenceService.get used for the followSymlinks option lacks an explicit
generic type, allowing non-boolean values to propagate into the search `find`
parameters; update the call in fileSearch.ts to use
preferenceService.get<boolean> for the followSymlinks retrieval (the symbol to
change is the expression using followSymlinks and preferenceService.get) so the
returned value is typed as boolean and then fall back to the default true when
the getter returns undefined.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Types
Background or solution
Changelog
Summary by CodeRabbit
New Features
Improvements
Tests