feat: add search.followSymlinks preference to control symlink followi…#4739
feat: add search.followSymlinks preference to control symlink followi…#4739harryecho996 wants to merge 97 commits into
Conversation
- Restore original components to pre-ACP state (ChatHistory, ChatMentionInput, mention-input) - Create ACP-specific components in acp/components/ directory - AcpChatMentionInput: recursive workspace file loading (limit 50) - AcpChatHistory: no delete button (server-managed sessions) - Register ACP components via IChatAgentViewService contribution point - Dynamic component selection in chat.view.tsx based on supportsAgentMode flag - Add design document: docs/plans/2026-04-07-acp-components-refactor.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…g logs Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Re-add 8 test files covering chat agent service, chat model, diff computer, multi-line decoration, tree-sitter, and MCP servers. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…dropdown When a slash command is selected from the MentionInput dropdown, only a visual DOM tag was inserted but the parent's command state was never updated. This caused AcpChatAgent to receive an empty command string, skipping the custom invoke handler. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Removed 10 verbose console.log/logger.log debug statements and 3 noisy dispose lifecycle logs across the ACP module. Error and warn level logs are preserved. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… mention input - Remove dead code (unreachable return) and add null-safe fallbacks in acp-session-provider.ts - Remove duplicate `localize` import in ChatMentionInput.acp.tsx Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ing space Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- 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>
- 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>
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>
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>
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>
|
/next |
|
🎉 PR Next publish successful! 3.9.1-next-1779163609.0 |
There was a problem hiding this comment.
Actionable comments posted: 6
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (39)
packages/ai-native/src/browser/acp/permission-bridge.service.ts-143-145 (1)
143-145:⚠️ Potential issue | 🟠 Major | ⚡ Quick win取消请求不应被标记为超时。
cancelRequest走handleDialogClose后统一返回type: 'timeout',会把主动取消与真实超时混淆,影响上游策略判断与埋点统计。可参考修改
+ private resolveDecision(requestId: string, decision: PermissionDecision): void { + const pending = this.pendingDecisions.get(requestId); + if (!pending) { + return; + } + clearTimeout(pending.timeout); + this.pendingDecisions.delete(requestId); + this.activeDialogs.delete(requestId); + this.onPermissionResult.fire({ requestId, decision }); + pending.resolve(decision); + } + handleDialogClose(requestId: string): void { - const pending = this.pendingDecisions.get(requestId); - if (!pending) { - return; - } - - clearTimeout(pending.timeout); - this.pendingDecisions.delete(requestId); - - const decision: PermissionDecision = { type: 'timeout' }; - - this.activeDialogs.delete(requestId); - this.onPermissionResult.fire({ requestId, decision }); - pending.resolve(decision); + this.resolveDecision(requestId, { type: 'timeout' }); } cancelRequest(requestId: string): void { - this.handleDialogClose(requestId); + this.resolveDecision(requestId, { type: 'cancelled' }); }🤖 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-bridge.service.ts` around lines 143 - 145, The current cancelRequest(requestId) calls handleDialogClose(requestId) which causes the unified response to be labeled type: 'timeout' and conflates user-initiated cancels with real timeouts; change the flow so cancelRequest produces a distinct reason (e.g., type: 'cancel' or 'user_cancel') instead of 'timeout' by either updating handleDialogClose to accept an explicit reason parameter (e.g., handleDialogClose(requestId, reason)) and passing 'cancel' from cancelRequest, or adding a new method (e.g., handleCancelRequest(requestId)) that mirrors handleDialogClose but sets the response type to 'cancel', and ensure any upstream completion/emit/path uses the new cancel type for metrics/strategy.packages/ai-native/src/browser/mcp/base-apply.service.ts-192-193 (1)
192-193:⚠️ Potential issue | 🟠 Major | ⚡ Quick win补空值兜底,当前仍可能在运行时抛错。
这里即使做了可选链,
sessionAdditionals仍可能是undefined,随后调用.values()会直接异常。建议修复
- const sessionAdditionals = sessionModel?.history?.sessionAdditionals; - return Array.from(sessionAdditionals.values()) + const sessionAdditionals = sessionModel?.history?.sessionAdditionals; + if (!sessionAdditionals) { + return []; + } + return Array.from(sessionAdditionals.values())🤖 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/base-apply.service.ts` around lines 192 - 193, sessionAdditionals may be undefined despite the optional chaining, so calling .values() can throw; update the code that computes sessionAdditionals (the variable sessionAdditionals derived from sessionModel?.history?.sessionAdditionals) to provide a safe default (e.g. an empty Map or empty iterable) when it's null/undefined and then call Array.from(...) on that safe default, ensuring the return is an empty array instead of throwing at runtime.packages/ai-native/src/browser/chat/chat.view.registry.ts-45-49 (1)
45-49:⚠️ Potential issue | 🟠 Major | ⚡ Quick win避免
when()异常中断视图选择。当前任一贡献的
when()抛错会中断getActiveChatView(),导致后续候选都无法参与选择。建议修复
getActiveChatView(): ChatViewContribution | null { for (const c of this.contributions) { - if (!c.when || c.when()) { - return c; - } + try { + if (!c.when || c.when()) { + return c; + } + } catch { + continue; + } } return null; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-native/src/browser/chat/chat.view.registry.ts` around lines 45 - 49, getActiveChatView currently iterates contributions and calls each contribution.when(), but if a when() throws it will abort selection; wrap the when() call in a try/catch inside getActiveChatView (the method in chat.view.registry.ts) and treat any exception as a false/skip for that contribution, logging the error if appropriate, then continue to the next contribution so subsequent candidates can be evaluated; ensure you reference the contributions array and the when() predicate on ChatViewContribution when applying the fix.packages/ai-native/src/browser/chat/session-provider-registry.ts-88-93 (1)
88-93:⚠️ Potential issue | 🟠 Major | ⚡ Quick win覆盖注册场景下,旧 disposable 可能误删新 Provider。
同一
id被新 Provider 覆盖后,旧返回句柄dispose()仍会delete(id),会把当前生效的新 Provider 一并删掉。建议修复
registerProvider(provider: ISessionProvider): IDisposable { @@ - this.providers.set(provider.id, provider); + this.providers.set(provider.id, provider); 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 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/session-provider-registry.ts` around lines 88 - 93, 当前实现把 provider 放入 this.providers 后返回的 dispose 闭包直接 delete(provider.id),在同 id 被新 provider 覆盖后旧的 dispose 会误删新注册的 provider。修改 dispose 实现使其在删除前校验当前 map 中的值是否仍然是要删除的实例(通过引用比较 this.providers.get(provider.id) === provider),仅在匹配时才执行 this.providers.delete(provider.id);在 session-provider-registry 的注册逻辑(调用 this.providers.set(provider.id, provider) 并返回 dispose 的位置)添加该比较以确保不会删除已被新覆盖的 provider。packages/ai-native/src/browser/chat/local-storage-provider.ts-31-36 (1)
31-36:⚠️ Potential issue | 🟠 Major | ⚡ Quick win会话模式兼容与存储数据健壮性需要一起补齐。
canHandle与注释不一致(未兼容无前缀),且sessionModels未校验数组类型就直接filter/find,本地数据异常时会抛错。🔧 建议修改
canHandle(mode: string): boolean { - return mode === 'local'; + return mode === 'local' || mode === ''; } @@ async loadSessions(): Promise<ISessionModel[]> { const storage = await this.getStorage(); - const sessionsModelData = storage.get<ISessionModel[]>('sessionModels', []); + const raw = storage.get<unknown>('sessionModels', []); + const sessionsModelData = Array.isArray(raw) ? (raw as ISessionModel[]) : []; // 过滤掉空消息历史的会话 return sessionsModelData.filter((item) => item.history?.messages?.length > 0); } @@ async loadSession(sessionId: string): Promise<ISessionModel | undefined> { const storage = await this.getStorage(); - const sessionsModelData = storage.get<ISessionModel[]>('sessionModels', []); + const raw = storage.get<unknown>('sessionModels', []); + const sessionsModelData = Array.isArray(raw) ? (raw as ISessionModel[]) : []; return sessionsModelData.find((item) => item.sessionId === sessionId); }Also applies to: 43-55
🤖 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/local-storage-provider.ts` around lines 31 - 36, Update canHandle to match the comment by accepting 'local' prefixes and also the legacy no-prefix case (e.g., return true when mode is falsy or startsWith('local') or equals 'local'); additionally, before using sessionModels with array operations (filter/find) validate it (e.g., if (!Array.isArray(sessionModels)) set sessionModels = [] or bail) so malformed local storage won't throw—apply these checks where sessionModels is read/filtered (references: canHandle and any code blocks using sessionModels.filter / sessionModels.find).packages/ai-native/src/browser/components/acp/mention-input.module.less-3-4 (1)
3-4:⚠️ Potential issue | 🟠 Major | ⚡ Quick win空样式块会触发 stylelint 错误。
这里是空规则,需删除或补齐样式声明,否则会命中
block-no-empty。🔧 建议修改
-.popover_icon { -}🤖 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/mention-input.module.less` around lines 3 - 4, The .popover_icon CSS rule is empty and triggers stylelint's block-no-empty; either remove the empty .popover_icon { } rule from mention-input.module.less or add the necessary style declarations for .popover_icon (e.g., padding/margin/display/etc.) so the rule is not empty; update the file to eliminate the empty block referencing the .popover_icon selector.packages/ai-native/src/browser/chat/pick-workspace-dir.ts-57-69 (1)
57-69:⚠️ Potential issue | 🟠 Major | ⚡ Quick win多根工作区的空 roots 场景会在取消时崩溃。
当
roots为空且用户取消选择时,回退逻辑直接读取首项会抛异常。请先判空再回退。🔧 建议修改
if (workspaceService.isMultiRootWorkspaceOpened) { const roots = workspaceService.tryGetRoots(); + if (!roots.length) { + messageService.info(localize('chat.noWorkspaceRootForACP')); + return ''; + } const choose = await quickPick.show( roots.map((file) => new URI(file.uri).codeUri.fsPath), { placeholder: localize('chat.selectCWDForACP') }, );🤖 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, The cancel path assumes roots has at least one entry and will throw when roots is empty; update the block around workspaceService.isMultiRootWorkspaceOpened / workspaceService.tryGetRoots() / quickPick.show to check that roots is non-empty before using roots[0] — if roots.length === 0 handle the empty-roots case (e.g. show a message via messageService.info/formatLocalize and return a safe default or undefined) instead of dereferencing roots[0]; ensure the quickPick fallback logic only accesses new URI(roots[0].uri) when roots.length > 0.packages/ai-native/src/node/acp/acp-permission-caller.service.ts-62-74 (1)
62-74:⚠️ Potential issue | 🟠 Major | ⚡ Quick win权限绕过开关未限制环境,生产可被直接放行。
当前只要
SKIP_PERMISSION_CHECK=true就无条件放行,建议至少限制为非生产环境可用,并记录告警日志。🔧 建议修改
- const skipPermissionCheck = process.env.SKIP_PERMISSION_CHECK === 'true'; + const skipPermissionCheck = + process.env.NODE_ENV !== 'production' && process.env.SKIP_PERMISSION_CHECK === 'true'; if (skipPermissionCheck) { + this.logger.warn('[ACP Permission Caller] Permission check skipped by env flag'); const allowOptionId = this.findAllowOptionId(request.options); return { outcome: {🤖 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-permission-caller.service.ts` around lines 62 - 74, The current SKIP_PERMISSION_CHECK bypass allows unconditional approval when SKIP_PERMISSION_CHECK='true'; change the guard in the early-return block so it only bypasses in non-production environments (e.g., check process.env.NODE_ENV !== 'production' or a dedicated isProd flag) and emit a warning log when the bypass is used (use the service logger, e.g., this.logger.warn or similar) before returning; keep using findAllowOptionId(request.options) and the same return shape but restrict it to non-production and log the event.packages/ai-native/src/browser/chat/chat.input.registry.ts-87-90 (1)
87-90:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
when()缺少异常隔离,单个贡献异常会阻断整个输入选择。Line 89 直接调用
c.when();若某个贡献抛异常,getActiveChatInput()会整体失败。建议隔离异常并继续尝试下一个贡献。建议修复
getActiveChatInput(): ChatInputContribution | null { for (const c of this.contributions) { - if (!c.when || c.when()) { - return c; - } + try { + if (!c.when || c.when()) { + return c; + } + } catch { + continue; + } } return null; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-native/src/browser/chat/chat.input.registry.ts` around lines 87 - 90, The getActiveChatInput() method currently calls each contribution's when() directly which can throw and abort selection; update getActiveChatInput to call c.when() inside a try/catch, treat any thrown error as a false condition (skip that contribution), optionally log the error for debugging (e.g., via console.error or your module logger), and continue iterating contributions so a single bad contribution cannot break the whole selection.packages/ai-native/src/browser/chat/chat-proxy.service.acp.ts-55-58 (1)
55-58:⚠️ Potential issue | 🟠 Major | ⚡ Quick winfallback 注册后未更新
agentDisposable,后续切换会遗留旧 agent。Line 57 只
addDispose(...),但没有把新 disposable 赋给this.agentDisposable。这会让下一次registerFallbackAgent()释放不到当前 agent。建议修复
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 previous agent but calls addDispose(this.chatAgentService.registerAgent(this.defaultChatAgent)) without saving the returned disposable, so future calls can't dispose the new agent; update registerFallbackAgent to capture the disposable returned from chatAgentService.registerAgent (e.g. const disp = this.chatAgentService.registerAgent(this.defaultChatAgent)), assign it to this.agentDisposable, and then call this.addDispose(disp) (keeping the existing queueMicrotask context) so the new agent can be disposed on subsequent switches.packages/ai-native/src/browser/chat/chat.internal.service.ts-67-90 (1)
67-90:⚠️ Potential issue | 🟠 Major | ⚡ Quick win在会话未初始化时会触发运行时崩溃。
Line 68、Line 72、Line 80、Line 90 直接读取
this._sessionModel.sessionId,在init()的异步初始化完成前调用这些方法会抛异常。建议统一加会话可用性保护,避免空引用崩溃。建议修复
+ private requireSessionModel(): ChatModel { + if (!this._sessionModel) { + throw new Error('Chat session is not initialized yet'); + } + return this._sessionModel; + } createRequest(input: string, agentId: string, images?: string[], command?: string) { - return this.chatManagerService.createRequest(this._sessionModel.sessionId, input, agentId, command, images); + const session = this.requireSessionModel(); + return this.chatManagerService.createRequest(session.sessionId, input, agentId, command, images); } sendRequest(request: ChatRequestModel, regenerate = false) { - const result = this.chatManagerService.sendRequest(this._sessionModel.sessionId, request, regenerate); + const session = this.requireSessionModel(); + const result = this.chatManagerService.sendRequest(session.sessionId, request, regenerate); if (regenerate) { this._onRegenerateRequest.fire(); } return result; } cancelRequest() { - this.chatManagerService.cancelRequest(this._sessionModel.sessionId); + const session = this.requireSessionModel(); + this.chatManagerService.cancelRequest(session.sessionId); this._onCancelRequest.fire(); } async clearSessionModel(sessionId?: string) { - sessionId = sessionId || this._sessionModel.sessionId; + const session = this.requireSessionModel(); + sessionId = sessionId || session.sessionId;🤖 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.ts` around lines 67 - 90, Multiple methods (createRequest, sendRequest, cancelRequest, clearSessionModel) read this._sessionModel.sessionId without checking that _sessionModel exists, causing crashes when the async init hasn't completed; add a guard or helper (e.g., private ensureSessionInitialized or throwIfNoSession) that verifies this._sessionModel is set (or starts a session via chatManagerService.startSession) and call it at the start of createRequest, sendRequest, cancelRequest, and clearSessionModel so they either await/initialize the session or throw a clear error instead of dereferencing undefined; keep createSessionModel as the initializer and ensure _onChangeSession is fired only after _sessionModel is set.packages/ai-native/src/browser/chat/acp-session-provider.ts-33-64 (1)
33-64:⚠️ Potential issue | 🟠 Major | ⚡ Quick win创建会话后未失效会话列表缓存,导致新会话可能不可见
Line [72]~Line [74] 会直接返回
loadedSessionsResult,但 Line [33]~Line [64] 新建会话后没有更新该缓存。先加载列表再创建会话时,列表会持续返回旧数据。建议修复
async createSession(title?: string): Promise<ISessionModel> { @@ // 新创建的 Session 不需要 load,直接加入缓存 this.loadedSessionMap.set(sessionId, sessionModel); + // 失效会话列表缓存,确保下次 loadSessions 能看到新会话 + this.loadedSessionsResult = null; return sessionModel;Also applies to: 71-74
🤖 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-session-provider.ts` around lines 33 - 64, The createSession implementation adds the new session to loadedSessionMap but does not update the cached sessions list returned by loadedSessionsResult (causing callers to still see stale lists); modify createSession (or immediately after it creates sessionModel) to also update or invalidate the cached list used by loadedSessionsResult so the newly created acp:{sessionId} appears in subsequent calls—either push the new session into the cached array that loadedSessionsResult returns or clear the cache so the next list fetch rebuilds it; reference createSession, sessionModel, loadedSessionMap and loadedSessionsResult when making this update.packages/ai-native/src/browser/acp/permission-dialog.view.tsx-40-61 (1)
40-61:⚠️ Potential issue | 🟠 Major | ⚡ Quick win倒计时没有在新请求打开时重置
Line [40] 只在首次渲染初始化
remainingTime。当同一组件承载新的requestId/timeout时,倒计时会复用旧值,导致自动拒绝时间异常。建议修复
const [remainingTime, setRemainingTime] = useState(timeout); + + useEffect(() => { + if (visible) { + setRemainingTime(timeout); + } + }, [visible, requestId, timeout]);🤖 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-dialog.view.tsx` around lines 40 - 61, The countdown state remainingTime is only initialized once with useState(timeout) so when a new requestId or timeout arrives the timer keeps the old value; update remainingTime whenever requestId or timeout (and visible becoming true) changes by resetting setRemainingTime(timeout) and recreate the interval logic accordingly inside the useEffect tied to [visible, requestId, timeout, onClose] (remove remainingTime from the dependency to avoid locking the effect), and ensure the interval is cleared on unmount or when requestId changes so onClose(requestId) uses the correct id.packages/ai-native/src/browser/acp/components/AcpChatHistory.tsx-162-166 (1)
162-166:⚠️ Potential issue | 🟠 Major | ⚡ Quick win这些主操作现在只能靠鼠标触发。
Line 162的历史项和Line 273的“新建聊天”入口都是可点击元素,但没有可聚焦语义,也没有 Enter/Space 键盘处理。键盘用户将无法切换会话或新建对话,这会直接阻断主流程。建议改成<button>,或至少补上role="button"、tabIndex={0}和键盘事件。Also applies to: 273-276
🤖 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 clickable history item and the "new chat" entry in AcpChatHistory.tsx are currently plain divs (the element rendered for each item using key={item.id} and the "new chat" entry around lines ~162 and ~273) and thus not keyboard-accessible; update both to be real interactive controls or add full accessible behavior: either change the elements to <button> so they are focusable and handle clicks, or keep the div but add role="button", tabIndex={0}, and an onKeyDown handler that triggers the same handler as onClick when Enter or Space is pressed (e.g., call handleHistoryItemSelect(item) and the new-chat handler). Also ensure focus/selected state is exposed (aria-selected or aria-pressed as appropriate) and that className usage (styles.chat_history_item, styles.chat_history_item_selected) remains applied for keyboard focus.packages/ai-native/src/browser/chat/chat-model.ts-303-315 (1)
303-315:⚠️ Potential issue | 🟠 Major | ⚡ Quick win新加的
title没有进入持久化模型。
Line 309已经把title存进ChatModel,而 session 模型契约也新增了title,但toJSON()仍然没有把它序列化出去。这样会话一旦保存/恢复,ACP 返回的标题就会丢失,历史列表又会退回到消息内容推导。Also applies to: 526-532
🤖 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-model.ts` around lines 303 - 315, The new ChatModel field title is not being persisted—update the ChatModel serialization/deserialization to include it: add the title property to the object returned by toJSON() (and any other serializer used around lines 526-532), and ensure the corresponding deserializer/constructor (e.g., the ChatModel constructor or a static fromJSON/deserialize method) reads title back into this.#title so saved/restored sessions preserve the title; reference ChatModel, title, toJSON(), and the deserialization path that restores session state.packages/ai-native/src/browser/acp/components/AcpChatViewHeader.tsx-68-70 (1)
68-70:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
createSessionModel()的异步错误无法被捕获
createSessionModel()是异步方法,但第 68 行和第 88 行都是同步调用它而未await,导致 Promise 拒绝会逃逸成未处理的异步错误,messageService.error()不会执行。建议:
- 第 68 行(在
handleSwitchWorkspaceDir中):改为await aiChatService.createSessionModel();- 第 88 行(在
handleNewChat中):先将回调改为async,再添加await🤖 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 68 - 70, The async method createSessionModel() is being called without awaiting inside handleSwitchWorkspaceDir and handleNewChat so rejected Promises escape and messageService.error(...) never runs; update handleSwitchWorkspaceDir to call await aiChatService.createSessionModel(); and make the handleNewChat callback async (e.g., async function or async arrow) then await aiChatService.createSessionModel() there as well so errors are properly caught and messageService.error(...) executes.packages/ai-native/src/browser/layout/ai-layout.tsx-21-39 (1)
21-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick win不要在条件返回前调用 Hook。
useMemo在第 39 行于条件分支之后被调用。即使isMobileDevice()在实际运行中不会变化,这种结构仍然违反 React Hooks 规则(Hooks 必须在条件判断之前调用)。将defaultRightSize的计算移至第 25 行的条件检查之前,或改用普通表达式,确保 Hook 调用顺序恒定。🤖 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/layout/ai-layout.tsx` around lines 21 - 39, The useMemo call for defaultRightSize is placed after a conditional return, violating React Hooks rules; move the useMemo invocation (or replace it with a plain calculation) so that defaultRightSize is computed before the early return that uses shouldShowFullLayout (which is derived from isMobileDevice()), ensuring the Hook useMemo is called unconditionally every render; update references to layout/AI-Chat and keep AI_CHAT_VIEW_ID/shouldShowFullLayout logic intact while relocating useMemo above the mobile check.packages/ai-native/src/browser/contrib/terminal/ai-terminal.service.ts-179-191 (1)
179-191:⚠️ Potential issue | 🟠 Major | ⚡ Quick win点击其它操作时会把错误的 matcher 传给 handler。
现在
operationList展示的是所有 detection action,但onClickItem回调中handler.execute()仍然把最初命中的action.matcher传给任意被点击的 handler。用户如果点击了另一项 action,handler 收到的是错配上下文。应按id从detectionActions中查找对应 action 的 matcher,或调整execute()的调用方式。🤖 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 179 - 191, The onClickItem currently always passes the outer-scoped action.matcher to handler.execute, causing the wrong matcher for other items; fix by looking up the clicked action in the detectionActions list (from inlineChatFeatureRegistry.getTerminalActions / detectionActions) using the given id and pass that action's matcher to handler.execute (or fallback to ''/undefined if not found) when wiring terminalDecorations.addZoneDecoration's operationList/onClickItem; ensure you still obtain the handler via getTerminalHandler(id) before calling execute.packages/ai-native/src/browser/chat/acp-chat-agent.ts-168-190 (1)
168-190:⚠️ Potential issue | 🟠 Major | ⚡ Quick win异常路径被吞掉了,而且会重复 reject/重复报错。
onError已经 reject 过chatDeferred;await chatDeferred.promise抛进catch后,这里又再次reject,最后还return {}。结果是调用方看不到失败,某些路径还会出现重复 toast/重复上报。🤖 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 168 - 190, The code double-rejects and swallows the original error: keep the reject in the listenReadable onError handler (which calls chatDeferred.reject) but stop rejecting again in the outer catch and don't return an empty success value; instead in the catch block only perform non-duplicative logging/reporting if needed (use this.messageService.error/this.aiReporter.end only if not already done) and rethrow the error so the caller sees the failure. Refer to listenReadable, chatDeferred, this.messageService.error, this.aiReporter.end, sessionId, request.requestId and command to locate where to remove the duplicate chatDeferred.reject and ensure the outer catch rethrows rather than rejecting/returning {}.packages/ai-native/src/browser/chat/chat-manager.service.acp.ts-74-87 (1)
74-87:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
createSession()失败时没有回退路径。这段只有“返回空模型”的分支,没有“异常回退”的分支。只要 ACP provider 抛错,
startSession()就会直接 reject,用户连本地会话都起不来。🤖 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-manager.service.acp.ts` around lines 74 - 87, Wrap the call to this.mainProvider.createSession() inside a try/catch in startSession so provider exceptions don't bubble out; on catch, log the error and create a local fallback ChatModel (e.g. construct a minimal session object, set this.availableCommands = [] or preserved defaults, convert via this.fromAcpJSON or a new createLocalSession helper), then store it in this.sessionModels and call this.listenSession on that model before returning it; ensure the code still handles the successful path that uses sessionData.extension.availableCommands and models[0].packages/ai-native/src/browser/chat/acp-chat-agent.ts-150-160 (1)
150-160:⚠️ Potential issue | 🟠 Major | ⚡ Quick win空历史时这里会把
undefined发进history。新会话或无历史时,
history[history.length - 1]是undefined,当前代码会把它包成[lastmessage]传给后端。这样很容易在序列化或服务端校验时把首条请求打坏。💡 建议修改
- 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 150 - 160, The code computes lastmessage = history[history.length - 1] and always passes history: [lastmessage] into aiBackService.requestStream, which will send undefined when history is empty; change the payload so you only include the last message if it exists (e.g. compute a historyPayload that is history.length ? [lastmessage] : [] or omit the history field entirely) before calling requestStream (refer to lastmessage, history, and aiBackService.requestStream to locate the change).packages/ai-native/src/browser/components/acp/ChatReply.tsx-445-449 (1)
445-449:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
ChatAIRoleRender这里拿到的参数类型和上面不一致。
ChatReply传的是markdown.value字符串,这里却把整个IMarkdownString直接塞给了content。一旦注册了自定义 role renderer,这条通知路径很容易渲染成[object Object]或直接出错。💡 建议修改
- renderContent = <ChatAIRoleRender content={chunk.content} />; + renderContent = <ChatAIRoleRender content={chunk.content.value} />;🤖 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/ChatReply.tsx` around lines 445 - 449, chatAIRoleRender is being passed the whole IMarkdownString (chunk.content) instead of the markdown string used elsewhere; update the ChatReply rendering branch so that when chatRenderRegistry.chatAIRoleRender is present you extract the string (e.g. markdown.value or the same string used for ChatMarkdown) and pass that as the content prop to ChatAIRoleRender (reference ChatReply, chatRenderRegistry.chatAIRoleRender, ChatAIRoleRender, chunk.content and markdown.value) to avoid rendering [object Object] or runtime errors.packages/ai-native/src/browser/acp/permission-dialog-container.tsx-279-300 (1)
279-300:⚠️ Potential issue | 🟠 Major | ⚡ Quick win这里还留着调试底色,而且主体背景色变量少了一个右括号。
外层半透明红色会直接出现在正式权限弹窗里;同时内层
backgroundColor的 CSS 变量字符串无效,背景会回退成默认值。两处都会让权限 UI 看起来是坏的。💡 建议修改
<div style={{ position: 'absolute', bottom: '100%', left: 0, right: 0, zIndex: 1000, marginBottom: 8, - backgroundColor: 'rgba(255, 0, 0, 0.2)', }} > @@ border: '1px solid var(--kt-popover-border-color, var(--popover-border-color))', boxShadow: 'var(--kt-popover-shadow, 0 4px 12px rgba(0, 0, 0, 0.15))', padding: '8px', outline: 'none', - backgroundColor: 'var(--kt-popover-background, var(--popover-background, var(--app-background))', + backgroundColor: 'var(--kt-popover-background, var(--popover-background, var(--app-background)))', maxHeight: '200px',🤖 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-dialog-container.tsx` around lines 279 - 300, Remove the temporary debug background and fix the malformed CSS variable in the popover container: delete the outer div's backgroundColor: 'rgba(255, 0, 0, 0.2)' (the translucent red) and correct the inner style on the element referenced by containerRef by closing the missing parenthesis so backgroundColor reads something like "var(--kt-popover-background, var(--popover-background, var(--app-background)))". Ensure no other debug styling remains and that containerRef’s style uses the corrected variable string.packages/ai-native/src/browser/acp/components/AcpChatInput.tsx-399-405 (1)
399-405:⚠️ Potential issue | 🟠 Major | ⚡ Quick win不要在
useMemo里触发setState。
optionsBottomPosition在渲染阶段调用setIsShowOptions(false),这属于 render-time state update。展开输入框时这里很容易出现 React 警告,严重时还会造成重复渲染。把"展开时关闭 options"挪到单独的useEffect([isExpand])里更稳。🤖 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 useMemo computing optionsBottomPosition currently calls setIsShowOptions(false) (when isExpand) which causes a render-time state update; move that side-effect into a separate useEffect that depends on isExpand so the memo only computes customBottom = INSTRUCTION_BOTTOM + inputHeight and the effect handles closing options when isExpand becomes true (keep optionsBottomPosition derived from inputHeight in the useMemo and add useEffect(() => { if (isExpand) setIsShowOptions(false) }, [isExpand])).packages/ai-native/src/browser/chat/chat-manager.service.acp.ts-27-33 (1)
27-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick win在
startSession()中为createSession()调用添加错误处理。第 76 行的
await this.mainProvider.createSession()缺少 try-catch 保护。若调用抛出异常,错误会直接传播而无法降级到本地 ChatModel。loadSessionList()方法(第 45-60 行)已正确实现了相同操作的错误处理模式,startSession()应该补充相同的 try-catch 机制:建议修复
override async startSession(): Promise<ChatModel> { if (this.aiNativeConfig.capabilities.supportsAgentMode && this.mainProvider?.createSession) { try { const sessionData = await this.mainProvider.createSession(); if (sessionData.extension?.availableCommands) { this.availableCommands = sessionData.extension.availableCommands; } const models = this.fromAcpJSON([sessionData]); if (models.length > 0) { const model = models[0]; this.sessionModels.set(model.sessionId, model); this.listenSession(model); return model; } } catch (error) { // 降级到本地实现 } } const model = new ChatModel(this.chatFeatureRegistry); this.sessionModels.set(model.sessionId, model); this.listenSession(model); return model; }🤖 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-manager.service.acp.ts` around lines 27 - 33, startSession currently calls await this.mainProvider.createSession() without error handling; wrap that call (and subsequent processing: checking sessionData.extension.availableCommands, converting with fromAcpJSON, setting sessionModels, and calling listenSession) in a try-catch so any thrown error falls back to creating a local ChatModel. Mirror the pattern used in loadSessionList: if createSession succeeds use the created model, otherwise in the catch block (or when no model produced) instantiate new ChatModel, add it to this.sessionModels and call listenSession before returning. Ensure references: startSession, mainProvider.createSession, fromAcpJSON, availableCommands, sessionModels, listenSession, and ChatModel.packages/ai-native/src/browser/components/ChatHistory.acp.tsx-179-183 (1)
179-183:⚠️ Potential issue | 🟠 Major | ⚡ Quick win标题编辑入口缺失,当前实现无法进入编辑态。
handleTitleEdit已实现,但渲染分支里没有任何事件触发它,导致标题只能展示、无法编辑。🔧 建议修改
-<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={(e) => { + e.stopPropagation(); + handleTitleEdit(item); + }} +> {item.title} </span>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-native/src/browser/components/ChatHistory.acp.tsx` around lines 179 - 183, The title display branch currently lacks any event to enter edit mode; wire the existing handleTitleEdit so the UI can toggle editable state: add an event handler (e.g., onClick or onDoubleClick) to the span with id `chat-history-item-title-${item.id}` and/or the element using `styles.chat_history_item_title` to call `handleTitleEdit(item.id)` (or the signature used by the function) so that `historyTitleEditable[item.id]` becomes true and the edit input branch is shown.packages/ai-native/__test__/browser/acp/permission.handler.test.ts-1-1 (1)
1-1:⚠️ Potential issue | 🟠 Major | ⚡ Quick win修复导入路径:测试文件导入了未构建的
lib产物。该文件从
../../../lib/browser/acp/permission.handler导入,但lib目录不存在——lib仅在执行tsc构建后才由src生成。若 CI 直接运行单测而未预构建,会触发Cannot find module错误。需改为src路径。🔧 建议修改
-import { AcpPermissionHandler, PermissionDecision } from '../../../lib/browser/acp/permission.handler'; +import { AcpPermissionHandler, PermissionDecision } from '../../../src/browser/acp/permission.handler';注:包内其他测试文件(如
acp-permission-rpc.service.test.ts、permission-dialog-container.test.ts)也存在相同问题,使用了lib导入路径,建议一并修正。🤖 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/__test__/browser/acp/permission.handler.test.ts` at line 1, 测试文件从未构建的 `lib` 产物导入导致 CI 在未执行 tsc 时找不到模块;把 import 源从 ../../../lib/... 改为对应的 src 路径(即使用源代码导入而非构建产物)以修复,具体在测试中修改导入语句引用的符号 AcpPermissionHandler 和 PermissionDecision,从 ../../../lib/browser/acp/permission.handler 改为 ../../../src/browser/acp/permission.handler;同时检查并修正包内其它测试(如 acp-permission-rpc.service.test.ts、permission-dialog-container.test.ts)中相同的 lib->src 导入问题。packages/ai-native/src/browser/components/ChatMentionInput.acp.tsx-454-537 (1)
454-537:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
footerConfig的 memo 依赖不完整,模式切换后 UI 不会同步。这个对象里读取了
currentMode、modeOptions、aiNativeConfigService.capabilities和默认模型配置,但依赖数组没有覆盖这些值。结果是 mode 切换成功后,底部选择器/按钮配置仍可能停留在首帧状态。🤖 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/ChatMentionInput.acp.tsx` around lines 454 - 537, The memo for defaultMentionInputFooterOptions misses dependencies so its FooterConfig doesn't update when currentMode, modeOptions, aiNativeConfigService.capabilities or default model sources change; update the useMemo dependency array for defaultMentionInputFooterOptions to include currentMode, modeOptions, aiNativeConfigService.capabilities (or aiNativeConfigService), props.sessionModelId, preferenceService (or the getter used), and AINativeSettingSectionsId.ModelID so the memo recalculates when mode or model settings change; locate the useMemo that defines defaultMentionInputFooterOptions and add those symbols to its dependency list (also include handleImageUpload if used inside buttons) to ensure UI syncs on mode switch.packages/ai-native/src/browser/chat/chat.view.acp.tsx-870-880 (1)
870-880:⚠️ Potential issue | 🟠 Major | ⚡ Quick win恢复历史时把
hasUserSentMessage重置为false会盖住已恢复的消息列表。当注册了
chatWelcomePageRender时,这里先清空再恢复历史,但标志位一直保持false,最终仍然会渲染欢迎页而不是刚恢复出来的消息。建议根据历史是否为空来设置这个状态,或者在recover()完成后置回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/chat/chat.view.acp.tsx` around lines 870 - 880, The effect currently clears content and unconditionally calls setHasUserSentMessage(false) before calling recover, which causes chatWelcomePageRender to show even when recover restores messages; instead, don't force-reset the flag or update it after recover completes: remove the unconditional setHasUserSentMessage(false) and, inside the recover completion (or its promise/callback that receives restored messages), setHasUserSentMessage(restoredHistory && restoredHistory.length > 0) (or derive from aiChatService.sessionModel/message list) so the flag reflects whether history was actually recovered; keep using the existing CancellationTokenSource token passed to recover.packages/ai-native/src/node/acp/acp-cli-back.service.ts-281-318 (1)
281-318:⚠️ Potential issue | 🟠 Major | ⚡ Quick win按 chunk 直接入库会把一条历史消息拆成多条。
这里每收到一个
user_message_chunk/agent_message_chunk就push一条消息,恢复后的会话会被切成很多碎片气泡。建议按角色/消息边界先聚合 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 pushes a new message for every user_message_chunk/agent_message_chunk causing one logical message to be split into many; modify convertSessionUpdatesToMessages to aggregate consecutive text chunks per role before pushing: iterate updates, detect chunk types ('user_message_chunk' and 'agent_message_chunk') from update.sessionUpdate, append content.text to a buffer string for the current role, and only push a single message object when the role changes or at the end of the loop (carry over/merge timestamps as appropriate, e.g., use the first chunk's timestamp or the last chunk's timestamp); ensure non-chunk updates still flush any buffered message before handling other cases.packages/ai-native/src/node/acp/acp-cli-client.service.ts-486-492 (1)
486-492:⚠️ Potential issue | 🟠 Major | ⚡ Quick win保持 JSON-RPC
error.code为数字。这里直接透传
err.code,底层如果抛出 Node 风格错误(比如ENOENT、EACCES),会把字符串写进 JSON-RPCerror.code,协议层就变成无效 payload。建议统一使用数值 code,并把原始错误码放到data里。🤖 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-client.service.ts` around lines 486 - 492, The catch block that calls this.sendMessage is passing err.code directly into the JSON-RPC error.code field which can be a string (e.g. Node errno) and break the protocol; change the payload construction in the catch (the block that builds the { jsonrpc:'2.0', id: message.id, error: {...} } object) to always set error.code to a numeric value (e.g. use err.code if typeof err.code === 'number' otherwise a safe numeric fallback like -32603) and move the original err.code (and any other non-protocol fields like err.syscall or err.errno) into error.data so the original info is preserved without violating JSON-RPC.packages/ai-native/src/browser/chat/chat.view.acp.tsx-760-766 (1)
760-766:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
@code占位符解析在 Windows/URI 路径上会截断文件路径。这里用
split(':')解析filePath:Ln-m,遇到C:\...或带 scheme 的路径时会把真正的文件名拆坏,最终附加到上下文里的引用会错。建议从末尾的:L标记反向解析行号范围。🤖 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 760 - 766, The current parsing of filePathWithLineRange (using filePathWithLineRange.split(':') and then extracting lineRange) breaks on Windows drive letters or URIs because it splits the first colon; update the logic in the block around filePathWithLineRange and range so you parse the line-range from the end (e.g. find the last ':' or use a regex that captures an optional trailing ":<start>-<end>" or ":{Ln-m}" group) and then set filePath to the substring before that last colon and parse startLine/endLine from the trailing segment to compute range; keep using filePathWithLineRange, filePath, lineRange and range variable names to locate where to change.packages/ai-native/src/browser/acp/permission.handler.ts-61-78 (1)
61-78:⚠️ Potential issue | 🟠 Major | ⚡ Quick win初始化是异步的,但首个请求没有等它完成。
ensureInitialized()只触发了initStorage()就继续往下走,所以第一次requestPermission()会在规则尚未加载时做决策;如果这时又走到saveRules(),permissionStorage也还没准备好。🤖 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 - 78, ensureInitialized is synchronous but initStorage is async, so requestPermission may run before permissionStorage and rules are ready; change ensureInitialized to return/await a Promise (e.g., make ensureInitialized async and await initStorage or store an initialization Promise like this.initPromise = this.initPromise ?? this.initStorage()) and only mark initialized after initStorage completes; update callers such as requestPermission to await ensureInitialized() so permissionStorage, loadRules, and subsequent saveRules operate after initialization completes (refer to ensureInitialized, initStorage, permissionStorage, loadRules, saveRules, and requestPermission).packages/ai-native/src/node/acp/acp-agent.service.ts-195-225 (1)
195-225:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
availableCommands这里会混入其他 session 的通知。临时订阅没有按
sessionId过滤,而且下面还额外等待了 2 秒收集更新。只要这段时间里别的 session 也发出available_commands_update,返回结果就会被污染。🤖 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 195 - 225, availableCommands is being polluted because tempHandler (registered via this.clientService.onNotification) doesn't filter by sessionId and you rely on a fixed 2s sleep; change the logic in tempHandler to only accept notifications whose update.sessionUpdate === 'available_commands_update' AND whose sessionId matches the session returned by this.clientService.newSession (use the session id from res/newSession), replace the blind 2s wait with a promise that resolves when the matching notification(s) for that session arrive (with a timeout fallback), and ensure you always call unsubscribe (the value returned by this.clientService.onNotification) in a finally block to remove the temporary handler; reference tempHandler, availableCommands, this.clientService.onNotification, and this.clientService.newSession when locating where to apply these changes.packages/ai-native/src/node/acp/cli-agent-process-manager.ts-135-136 (1)
135-136:⚠️ Potential issue | 🟠 Major | ⚡ Quick win配置比较遗漏了
args和env。这里现在只比较
command和cwd,startAgent()会在启动参数或环境变量变化时错误复用旧进程。对 CLI agent 来说,这会把后续请求跑在过期配置上。🤖 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/cli-agent-process-manager.ts` around lines 135 - 136, isConfigSame currently only compares command and cwd, which lets startAgent() reuse a process when args or env changed; update isConfigSame to also compare args (order and values) and env (same set of keys and equal values) against this.currentArgs and this.currentEnv. Use a deterministic equality: check array length then element-by-element for args, and for env check same keys length and for each key this.currentEnv[key] === env[key]; return false on any mismatch so a new process is spawned when args or environment differ.packages/ai-native/src/node/acp/cli-agent-process-manager.ts-153-181 (1)
153-181:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift缺少“启动中”互斥,单例进程会被并发打穿。
两个并发
startAgent()都可能在currentProcess仍为空时进入createAgentProcess()。后一个实例会覆盖前一个引用,但前一个子进程不会被清理,最后就会留下游离进程。🤖 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/cli-agent-process-manager.ts` around lines 153 - 181, startAgent can be entered concurrently and must guard against multiple creations: add a "starting" mutex (e.g. a private isStarting boolean or a startingPromise) checked at the top of startAgent to await or short-circuit concurrent callers, set it immediately before calling createAgentProcess and clear it in a finally block; ensure that createAgentProcess result is only assigned to currentProcess/currentCommand/currentCwd if the caller still owns the startup (avoid overwriting another caller's successful start), and if createAgentProcess throws or the startup is abandoned kill/cleanup the child (and call stopAgentInternal if needed) so no orphaned processes remain; reference startAgent, createAgentProcess, stopAgentInternal, currentProcess, currentCommand, currentCwd, isProcessRunning, and isConfigSame when making the changes.packages/ai-native/src/node/acp/acp-agent.service.ts-233-247 (1)
233-247:⚠️ Potential issue | 🟠 Major | ⚡ Quick win已有连接时直接返回,会吞掉后续配置变更。
这里只要
currentProcessId已存在,就不会再把新的command / args / env / workspaceDir传给processManager.startAgent()。同一个服务实例下切换工作区或 agent 配置后,后续请求仍会复用旧进程。🤖 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 233 - 247, ensureConnected currently returns early when this.currentProcessId exists, causing new config (command/args/env/workspaceDir) to be ignored; modify ensureConnected to detect config changes and restart or start a new agent when needed. Specifically, compare the incoming AgentProcessConfig (command, args, env, workspaceDir) against the config used to start the running process (store the last-started config on the class, e.g., lastAgentConfig); if they differ, call processManager.startAgent(...) again, call clientService.setTransport(...) and clientService.initialize(), and update this.currentProcessId and the stored config; otherwise keep the fast path returning this.currentProcessId. Ensure the logic references ensureConnected, this.currentProcessId, processManager.startAgent, clientService.setTransport, clientService.initialize, and a new stored lastAgentConfig field.packages/ai-native/src/browser/acp/permission.handler.ts-47-53 (1)
47-53:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift“始终允许/拒绝”规则保存错了维度。
这里保存规则时传入的是
requestId和optionId,而落库时又把kind硬编码成'write'。结果保存出来的规则既不对应真实文件/命令,也无法匹配非写操作,用户刚选的“always”基本不会在下一次请求里生效。Also applies to: 121-123, 263-273
🤖 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 47 - 53, The "always allow/deny" rule persistence is storing the wrong dimensions: instead of hardcoding kind='write' and using requestId, update the save/persist logic to record the actual operation kind (use the request.kind/operation kind) and the correct resource identifier (e.g., fileId or commandId / the optionId semantics) so stored rules match future requests; find and update the code paths that create rules (the pendingRequests handling, the rule creation/save function referenced around the pendingRequests map and at the other affected blocks) to stop hardcoding 'write', use the real request.kind and the resource id used by the matcher, and ensure the matcher code that loads rules (lines noted 121-123 and 263-273) uses the same fields to compare so "always" choices will be applied correctly.packages/ai-native/src/browser/chat/chat.internal.service.acp.ts-73-80 (1)
73-80:⚠️ Potential issue | 🟠 Major | ⚡ Quick win加载态没有放进
finally。
startSession()或后面的命令刷新一旦抛错,这里就不会再发出false,界面会一直停在 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); + 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 - 80, The loading flag in createSessionModel is not placed in a finally block, so if chatManagerService.startSession() or subsequent calls throw the UI stays stuck in loading; update createSessionModel to set this._onSessionLoadingChange.fire(true) before the await, then move the this._onSessionLoadingChange.fire(false) into a finally block so it always runs; ensure the try block still assigns this._sessionModel and fires this._onSessionModelChange and this._onChangeSession, and reference createSessionModel, chatManagerService.startSession, this._sessionModel, and this._onSessionLoadingChange.fire when making the change.
| return new Promise((resolve) => { | ||
| // Set up timeout | ||
| const timeout = setTimeout(() => { | ||
| this.pendingRequests.delete(requestId); | ||
| this.logger.warn(`Permission request timed out: ${request.toolCall.title}`); | ||
| resolve({ type: 'timeout' }); | ||
| }, request.timeout ?? this.defaultTimeout); | ||
|
|
||
| this.pendingRequests.set(requestId, { | ||
| resolve, | ||
| timeout, | ||
| }); | ||
|
|
||
| // Show permission dialog | ||
| this.showPermissionDialog(requestId, request); | ||
| }); |
There was a problem hiding this comment.
当前实现下,未命中规则的权限请求只能超时。
这里既没有真正弹 UI,也没有把内部生成的 requestId 暴露给能调用 handleUserResponse() 的一侧。正常交互路径里,这类请求无法被批准或拒绝,只能等超时或外部取消。
Also applies to: 213-222
🤖 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 87 -
102, Unmatched permission requests currently only time out because no UI is
shown for them and the internally generated requestId isn't exposed so
handleUserResponse can't resolve them; change the request flow so you always
call showPermissionDialog(requestId, request) for every request and expose the
generated requestId to the caller (or return a small object { requestId, promise
} from the permission request function) so external code can call
handleUserResponse(requestId, response). Ensure pendingRequests.set(requestId, {
resolve, timeout }) remains, and implement handleUserResponse(requestId,
response) to look up pendingRequests, clear the timeout, call resolve with the
appropriate result, and delete pendingRequests so requests can be
approved/denied/cancelled instead of only timing out; update any callers to
accept the exposed requestId if necessary.
| const layoutService = useInjectable<IMainLayoutService>(IMainLayoutService); | ||
| const msgHistoryManager = aiChatService.sessionModel?.history; | ||
| if (!msgHistoryManager) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l packages/ai-native/src/browser/chat/chat.view.acp.tsxRepository: opensumi/core
Length of output: 116
🏁 Script executed:
sed -n '130,160p' packages/ai-native/src/browser/chat/chat.view.acp.tsxRepository: opensumi/core
Length of output: 2125
🏁 Script executed:
# Search for all hook declarations in the file to understand the hook order
rg -n "use[A-Z]|useState|useEffect|useContext|useCallback|useRef|useMemo|useInjectable" packages/ai-native/src/browser/chat/chat.view.acp.tsx | head -40Repository: opensumi/core
Length of output: 2937
不要在这里提前 return,这会破坏 Hook 顺序。
sessionModel 如果首帧为空、后续再初始化成功,这个分支会让组件前后两次 render 调用的 hooks 数量不同,运行时会直接报错。当前代码在行 142 之后立即返回,但行 147 之后还有大量 hooks(useRef、useState、useReducer、useEffect 等)。把判空分支挪到所有 hooks 之后,或者放进 JSX 分支里处理。
🤖 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 142 -
146, The early return when msgHistoryManager is falsy (using
aiChatService.sessionModel?.history) breaks hook order because more hooks run
later in the component; move the null check after all hooks are declared or
handle it inside the JSX render branch instead of returning before hooks like
useRef/useState/useReducer/useEffect are called — locate the
useInjectable<IMainLayoutService>(IMainLayoutService) and the msgHistoryManager
assignment and ensure any conditional render based on aiChatService.sessionModel
or msgHistoryManager happens only after all hooks have been invoked (or wrapped
inside the returned JSX).
| const text = e.clipboardData.getData('text/plain'); | ||
|
|
||
| // 处理文本,保留换行和缩进 | ||
| const processedText = text | ||
| .replace(/\t/g, ' ') | ||
| .replace(/\n\s*\n/g, '\n\n') | ||
| .replace(/[ \t]+$/gm, ''); | ||
|
|
||
| const selection = window.getSelection(); | ||
| if (!selection || !selection.rangeCount) { | ||
| return; | ||
| } | ||
|
|
||
| const range = selection.getRangeAt(0); | ||
| range.deleteContents(); | ||
|
|
||
| // 将处理后的文本按行分割 | ||
| const lines = processedText.split('\n'); | ||
| const fragment = document.createDocumentFragment(); | ||
|
|
||
| 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; |
There was a problem hiding this comment.
不要把剪贴板纯文本重新喂给 innerHTML。
这里拿到的是 text/plain,但后面又用 container.innerHTML = processedLine 解析了一次。用户只要粘贴 <img onerror=...> 这类文本,就会被当成真实 DOM 插进去,形成可达的 DOM/XSS 注入面。
建议修改
- const container = document.createElement('span');
- container.innerHTML = processedLine;
+ const container = document.createElement('span');
+ container.textContent = line;
+ // 缩进和换行请继续用 Text/BR 节点拼装,不要重新走 innerHTML 解析🤖 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 - 837, The paste handler in MentionInput.tsx is assigning processed plain
text to container.innerHTML (see processedText, processedLine,
container.innerHTML) which enables XSS; instead, preserve whitespace but insert
only text nodes: avoid using innerHTML and create Text nodes
(document.createTextNode or element.textContent) and explicit nbsp characters
for leading spaces, then append those nodes into the fragment and range. Update
the code paths that build fragment/lines (lines.forEach, fragment,
range.deleteContents()) to construct safe DOM via createTextNode and
element.textContent rather than parsing via innerHTML.
| private static currentRpcClient: IAcpPermissionService | null = null; | ||
|
|
||
| private clientId: string | undefined; | ||
|
|
||
| /** | ||
| * 设置连接 clientId | ||
| * | ||
| * 注意:框架调用 setConnectionClientId 后才设置 rpcClient, | ||
| * 因此需要使用微任务延迟赋值,确保 rpcClient 已经准备好 | ||
| */ | ||
| setConnectionClientId(clientId: string): void { | ||
| this.clientId = clientId; | ||
|
|
||
| Promise.resolve().then(() => { | ||
| AcpPermissionCallerManager.currentRpcClient = this.client || null; | ||
| }); | ||
| } |
There was a problem hiding this comment.
全局静态 RPC 客户端会导致跨连接权限串线。
currentRpcClient 在多连接下会被后连接覆盖,而请求侧优先使用它,权限弹窗可能发到错误连接,存在越权风险。建议按连接维度维护客户端映射,并在请求时按连接精确路由,避免全局单例游标。
Also applies to: 77-81
🤖 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-permission-caller.service.ts` around
lines 31 - 47, The static shared currentRpcClient causes cross-connection
leakage; change AcpPermissionCallerManager to store a map from clientId to
IAcpPermissionService (e.g., a private static rpcClients: Map<string,
IAcpPermissionService>) and update setConnectionClientId to register this.client
under the provided clientId (using Promise.resolve().then to keep the current
async behavior) instead of assigning to currentRpcClient; then update any
request paths that read AcpPermissionCallerManager.currentRpcClient (including
the code around lines referenced 77-81) to look up the RPC client by the active
connection's clientId (falling back to null) so permission checks are routed
per-connection rather than via a global singleton.
| // Resolve symlinks for both the resolved path and workspace directory | ||
| let realResolvedPath: string; | ||
| let realWorkspaceDir: string; | ||
| try { | ||
| realResolvedPath = fs.realpathSync(resolvedPath); | ||
| } catch (error) { | ||
| // If the path doesn't exist yet (e.g., new file for write), use the resolved path as-is | ||
| realResolvedPath = resolvedPath; | ||
| } | ||
| try { | ||
| realWorkspaceDir = fs.realpathSync(this.workspaceDir); | ||
| } catch (error) { | ||
| this.logger?.warn(`Cannot resolve workspace directory: ${this.workspaceDir}`); | ||
| return null; | ||
| } | ||
|
|
||
| // Compute the relative path and ensure it does not escape workspace | ||
| const relativePath = path.relative(realWorkspaceDir, realResolvedPath); | ||
|
|
||
| // Reject if relative path equals '..' or starts with '..' + separator | ||
| if (relativePath === '..' || relativePath.startsWith(`..${path.sep}`)) { | ||
| this.logger?.warn(`Path outside workspace rejected: ${inputPath}`); | ||
| return null; | ||
| } | ||
|
|
||
| return realResolvedPath; |
There was a problem hiding this comment.
resolvePath() 还能被“工作区内符号链接 + 新文件名”绕出沙箱。
当目标文件还不存在时,这里直接退回 resolvedPath,不会再解析父目录的真实路径。比如工作区里有个指向外部目录的 symlink,link/new.txt 会通过校验,但后续写入实际落到工作区外,和注释里的“拒绝路径穿越/工作区外路径”不一致。
🤖 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/file-system.handler.ts` around lines
421 - 446, The current resolvePath logic falls back to using resolvedPath when
the target doesn't exist, allowing "workspace symlink + new filename" to escape
the sandbox; change it so that when fs.realpathSync(resolvedPath) fails you
instead resolve the real path of the parent directory (use
path.dirname(resolvedPath)), recompute the candidate realResolvedPath by joining
the parent realpath with path.basename(resolvedPath), then perform the same
relative check against the realWorkspaceDir; update resolvePath to use these
symbols (resolvedPath, realResolvedPath, realWorkspaceDir, workspaceDir) so
symlinked directories are resolved even for new files and paths outside the
workspace are correctly rejected.
Two issues prevented the followSymlinks preference from working: 1. The setting was not registered in the settings UI whitelist (defaultSettingSections), so it was invisible in the preferences panel. Added it with localized description for searchability. 2. doSearch() used UIState.isFollowSymlinks which was initialized from stale browserStorage (recoverUIState) and could not be overridden by the preference value. Now doSearch() reads directly from searchPreferences to ensure the setting takes immediate effect. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
/next |
This is an internal development tool that should not be published to npm. Adding private: true prevents lerna from attempting to publish it, fixing the E404 error during CI publish. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ublish" This reverts commit b010e3b.
|
/next |
1 similar comment
|
/next |
|
🎉 PR Next publish successful! 3.9.1-next-1779262847.0 |
…ng in search
Types
Background or solution
Changelog
Summary by CodeRabbit