Conversation
Contributor
Author
|
/next |
Contributor
概览本PR引入了完整的Agent Client Protocol (ACP) 支持，包括权限对话框系统、会话管理重构、CLI代理进程管理、以及ACP特定的后端服务。这是一个大规模的特性实现，涉及浏览器端和Node端的多个层级。 变更
序列图sequenceDiagram
participant User as 用户
participant UI as 浏览器UI
participant PermBridge as PermissionBridge<br/>(权限桥接)
participant RPC as RPC Service<br/>(权限RPC)
participant NodeACP as Node ACP<br/>(权限调用者)
participant Agent as ACP代理<br/>(CLI进程)
User->>UI: 执行需要权限的操作
UI->>Agent: 发送工具调用请求
Agent->>NodeACP: 请求权限(RequestPermissionRequest)
NodeACP->>RPC: $showPermissionDialog(权限参数)
RPC->>PermBridge: showPermissionDialog()
PermBridge->>UI: 显示权限对话框
UI->>User: 显示权限确认
User->>UI: 选择允许/拒绝
UI->>PermBridge: handleUserDecision()
PermBridge->>RPC: 返回决策
RPC->>NodeACP: 返回AcpPermissionDecision
NodeACP->>Agent: RequestPermissionResponse(允许/拒绝)
Agent->>Agent: 继续执行或中止
sequenceDiagram
participant ChatUI as Chat UI
participant ChatMgr as ChatManager<br/>(会话管理)
participant Registry as SessionProvider<br/>Registry(注册表)
participant LocalProvider as LocalStorage<br/>Provider
participant ACPProvider as ACP<br/>Provider
participant Backend as 后端服务
ChatUI->>ChatMgr: init()
ChatMgr->>Registry: 选择主提供者
alt ACP模式
ChatMgr->>ACPProvider: loadSessions()
ACPProvider->>Backend: aiBackService.listSessions()
Backend-->>ACPProvider: 会话列表
ACPProvider-->>ChatMgr: ISessionModel[]
else 本地模式
ChatMgr->>LocalProvider: loadSessions()
LocalProvider->>LocalProvider: 从localStorage读取
LocalProvider-->>ChatMgr: ISessionModel[]
end
ChatMgr-->>ChatUI: 会话已加载
ChatUI->>ChatMgr: startSession()
alt ACP模式
ChatMgr->>ACPProvider: createSession()
ACPProvider->>Backend: aiBackService.createSession()
Backend-->>ACPProvider: sessionId
ACPProvider-->>ChatMgr: ISessionModel
else 本地模式
ChatMgr->>ChatMgr: 创建本地ChatModel
ChatMgr-->>ChatMgr: 缓存
end
ChatMgr-->>ChatUI: 会话已创建
代码审查工作量评估🎯 5 (Critical) | ⏱️ ~120 minutes 本PR涉及高度复杂的多层级集成：
可能相关的PR
建议标签
建议审查者
✨ Finishing Touches
🧪 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
packages/ai-native/__test__/node/acp/cli-agent-process-manager.test.ts
Outdated
Show resolved
Hide resolved
Comment on lines
+108
to
+127
| listenReadable<IChatProgress>(stream, { | ||
| onData: (data) => { | ||
| progress(data); | ||
| }, | ||
| onEnd: () => { | ||
| chatDeferred.resolve(); | ||
| }, | ||
| onError: (error) => { | ||
| this.messageService.error(error.message); | ||
| this.aiReporter.end(request.sessionId + '_' + request.requestId, { | ||
| message: error.message, | ||
| success: false, | ||
| command, | ||
| }); | ||
| }, | ||
| }); | ||
|
|
||
| await chatDeferred.promise; | ||
| return {}; | ||
| } |
Contributor
There was a problem hiding this comment.
错误时 chatDeferred 未被 resolve，导致 Promise 永远挂起
在 onError 回调中，chatDeferred 没有被 resolve 或 reject，这会导致 invoke 方法永远不会返回，造成调用方阻塞。
🐛 建议修复
listenReadable<IChatProgress>(stream, {
onData: (data) => {
progress(data);
},
onEnd: () => {
chatDeferred.resolve();
},
onError: (error) => {
this.messageService.error(error.message);
this.aiReporter.end(request.sessionId + '_' + request.requestId, {
message: error.message,
success: false,
command,
});
+ chatDeferred.resolve();
},
});📝 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
| listenReadable<IChatProgress>(stream, { | |
| onData: (data) => { | |
| progress(data); | |
| }, | |
| onEnd: () => { | |
| chatDeferred.resolve(); | |
| }, | |
| onError: (error) => { | |
| this.messageService.error(error.message); | |
| this.aiReporter.end(request.sessionId + '_' + request.requestId, { | |
| message: error.message, | |
| success: false, | |
| command, | |
| }); | |
| }, | |
| }); | |
| await chatDeferred.promise; | |
| return {}; | |
| } | |
| listenReadable<IChatProgress>(stream, { | |
| onData: (data) => { | |
| progress(data); | |
| }, | |
| onEnd: () => { | |
| chatDeferred.resolve(); | |
| }, | |
| onError: (error) => { | |
| this.messageService.error(error.message); | |
| this.aiReporter.end(request.sessionId + '_' + request.requestId, { | |
| message: error.message, | |
| success: false, | |
| command, | |
| }); | |
| chatDeferred.resolve(); | |
| }, | |
| }); | |
| await chatDeferred.promise; | |
| return {}; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ai-native/src/browser/chat/default-chat-agent.ts` around lines 108 -
127, The onError handler passed to listenReadable does not settle chatDeferred,
leaving the invoke caller hanging; update the onError callback in
default-chat-agent.ts to both log/report the error (keep
this.messageService.error and aiReporter.end) and then settle chatDeferred
(either call chatDeferred.reject(error) to propagate the failure or
chatDeferred.resolve() if you want a graceful completion), ensuring the invoke
flow (the await chatDeferred.promise and return {}) always resumes; reference
the listenReadable(...) onError, the chatDeferred variable, and the
aiReporter.end(...) call when making the change.
Comment on lines
+346
to
+364
| async createDirectory(request: FileSystemRequest): Promise<FileSystemResponse> { | ||
| const dirPath = this.resolvePath(request.path); | ||
| if (!dirPath) { | ||
| return { | ||
| error: { | ||
| code: ACPErrorCode.SERVER_ERROR, | ||
| message: 'Invalid path', | ||
| data: { path: request.path }, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| try { | ||
| const uri = URI.file(dirPath); | ||
| await this.fileService.createFolder(uri.toString()); | ||
|
|
||
| this.logger?.log(`Directory created: ${dirPath}`); | ||
|
|
||
| return {}; |
Contributor
There was a problem hiding this comment.
createDirectory() 绕过了写权限确认。
Line 346-364 这里直接修改工作区文件系统，没有复用 writeTextFile() 的 permissionCallback 流程。如果 ACP 的权限模型要求所有写操作都要授权，这里就是一个直接的绕过口。
🤖 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/file-system.handler.ts` around lines
346 - 364, createDirectory(...) currently bypasses the ACP write-permission flow
by calling fileService.createFolder(...) directly; before creating the folder
(in the createDirectory method that uses resolvePath, URI.file and
fileService.createFolder), invoke the same permission check used by
writeTextFile (the permissionCallback/authorization flow) and only proceed to
call fileService.createFolder when the permission check grants write access;
ensure you reuse the existing permission-check helper or call the same
permissionCallback path, and preserve logging (this.logger.log) and error
response shape when permission is denied.
packages/ai-native/src/node/acp/handlers/file-system.handler.ts
Outdated
Show resolved
Hide resolved
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 |
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 |
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1773914436.0 |
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
1 participant
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
发布说明
新功能
改进
依赖
@agentclientprotocol/sdk依赖