feat: add mpp-vscode support

[phodal]

Summary by CodeRabbit

Release Notes

• New Features
  • Added structured plan management system for multi-step task organization and progress tracking
  • Integrated file context selection in editor for better code awareness
  • Introduced VS Code extension with chat interface and workspace integration
  • Added workspace file search and recent files browsing capabilities
  • Implemented IDE server for external tool integration

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Introduces a comprehensive plan management system replacing task-boundary tracking, with new data models (AgentPlan, PlanTask, PlanStep), a plan-aware tool (PlanManagementTool), state service with listeners, markdown parsing, UI components, file search capabilities, and a complete VSCode extension with chat webview, IDE server integration, and diff management.

Changes

Cohort / File(s)	Summary
Plan Domain Models mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/plan/\*	Added TaskStatus and PlanPhase enums; introduced AgentPlan, PlanTask, PlanStep data classes with status tracking, progress calculation, and markdown serialization; created CodeFileLink for link extraction and MarkdownPlanParser for plan parsing/formatting.
Plan State Management mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/plan/PlanStateService.kt, PlanUpdateListener.kt	Introduced PlanStateService with reactive StateFlow for current plan, methods to create/update/clear plans, task/step management, and listener-driven event notifications; added PlanUpdateListener interface and DefaultPlanUpdateListener for event subscription.
Plan Tool Implementation mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/tool/impl/PlanManagementTool.kt	Added new PlanManagementTool replacing TaskBoundaryTool with support for CREATE, UPDATE, COMPLETE_STEP, FAIL_STEP, VIEW actions; includes PlanManagementParams, PlanManagementSchema, and execution logic with plan state integration and validation.
Tool Orchestration mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/orchestrator/ToolOrchestrator.kt, tool/registry/BuiltinToolsProvider.kt	Renamed executeTaskBoundaryTool to executePlanManagementTool; updated tool types and parameters from TaskBoundaryTool/Params to PlanManagementTool/Params; replaced TaskBoundaryTool with PlanManagementTool in registry.
Removed Task Boundary Tool mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/tool/impl/TaskBoundaryTool.kt	Deleted entire TaskBoundaryTool implementation including TaskStatus enum, TaskBoundaryParams, schema, and invocation logic.
Documentation & Templates mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/CodingAgentTemplate.kt	Updated task completion strategy with planning-first approach; added plan workflow documentation in English and Chinese; restructured step execution guidance.
Executor Configuration mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/executor/CodingAgentExecutor.kt	Added maxWaitTimeoutMs field to AsyncShellConfig (default 120,000 ms) for maximum async shell execution timeout; propagated timeout to OrchestratorContext.
Editor Extensions mpp-core/src/commonMain/kotlin/cc/unitmesh/devins/editor/EditorCallbacks.kt	Added FileContext data class and onSubmit overload accepting file context list; maintains backward compatibility with existing onSubmit(text).
Markdown Parser Relocation mpp-core/src/commonMain/kotlin/cc/unitmesh/markdown/MarkdownTextParser.kt	Moved MarkdownTextParser from devins.idea.renderer.markdown package to cc.unitmesh.markdown; updated imports in MarkdownInlineRenderer, JewelMarkdownRenderer, MarkdownTable.
Plan UI Components mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/\*, PlanPanel.kt	Added PlanPanel composable with plan rendering, task cards, step items with status indicators; introduced TaskStatus.planColor property and TaskStatus.planIcon composable extension.
Composer Renderer mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt	Integrated plan state tracking; added updatePlanFromToolCall to handle CREATE/UPDATE/COMPLETE_STEP/FAIL_STEP actions; wired plan tool calls into UI state.
File Search & Context mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/context/\*, SelectedFileItem.kt, FileChip.kt, FileSearchPopup.kt, TopToolbar.kt, WorkspaceFileSearchProvider.kt	Added SelectedFileItem with path truncation; created FileChip/FileChipExpanded for file display; introduced FileSearchProvider interface and FileSearchPopup composable; implemented WorkspaceFileSearchProvider with file indexing and recent files; added TopToolbar for file context management.
Editor Input Enhancement mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/DevInEditorInput.kt, BottomToolbar.kt	Integrated FileSearchProvider for file context selection; added getFileContexts and buildAndSendMessage for file-aware messaging; added prompt enhancement controls to BottomToolbar with isEnhancing state.
UI Utilities mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/icons/AutoDevComposeIcons.kt	Added AutoAwesome icon alias.
mpp-core Tests mpp-core/src/commonTest/kotlin/cc/unitmesh/agent/plan/\*, tool/impl/PlanManagementToolTest.kt, ToolRegistryTest.kt	Added comprehensive test suites for MarkdownPlanParser, PlanStateService, and PlanManagementTool; added test for plan tool registration.
File System Integration mpp-core/src/jvmMain/kotlin/cc/unitmesh/devins/filesystem/DefaultFileSystem.jvm.kt	Added verbose debug logging for searchFiles method including timing and result counts.
Async Result Handling mpp-core/src/jvmTest/kotlin/cc/unitmesh/agent/tool/impl/DocQLReturnAllTest.kt	Extended ToolResult handling to include Pending state for both returnAll branches.
VSCode Extension Setup mpp-vscode/package.json, tsconfig.json, vitest.config.ts	Added extension manifest with metadata, commands, sidebar, chat view, configuration, keybindings, language definitions, build scripts, and dependencies.
VSCode Extension Core mpp-vscode/src/extension.ts	Implemented extension activation and deactivation; wired logging, status bar, diff management, IDE server, commands, chat view, completions, and initial welcome notification.
VSCode Services mpp-vscode/src/services/\*	Implemented ConfigManager for ~/.autodev/config.yaml loading; StatusBarManager with state animation; DiffContentProvider and DiffManager for diff UI; IDEServer with HTTP API for MCP protocol.
VSCode Providers mpp-vscode/src/providers/chat-view.ts, devins-completion.ts	Created ChatViewProvider integrating LLM service, CodingAgent, and webview messaging; implemented DevInsCompletionProvider for command/@/$ completions.
mpp-core Bridge mpp-vscode/src/bridge/mpp-core.ts	Exposed TypeScript wrappers for LLMService, CompletionManager, DevInsCompiler, ToolRegistry, VSCodeRenderer, CodingAgent; unified public API for AI tooling.
VSCode Utilities mpp-vscode/src/utils/logger.ts, test/mocks/vscode.ts	Added createLogger for timestamp-aware logging; created comprehensive VSCode API mocks (Uri, EventEmitter, window, workspace, commands).
VSCode Tests mpp-vscode/test/\\.ts`	Added test suites for mpp-core bridge, ChatViewProvider, DevInsCompletionProvider, DiffManager, IDEServer, and StatusBarManager; included mocks for VSCode and extension modules.
VSCode Syntax & Config mpp-vscode/syntaxes/\*, README.md	Added TextMate grammar for DevIns language (syntax highlighting); created language configuration; added README with architecture and development guidance.
Webview HTML & Build mpp-vscode/webview/index.html, package.json, tsconfig.json, vite.config.ts	Set up webview project structure with React, Vite bundler, TypeScript config, and build scripts.
Webview UI Components mpp-vscode/webview/src/components/\*	Implemented Chat UI: ChatInput with file context/completions/enhancement, CompletionPopup, DevInInput with syntax highlighting, FileChip variants, FileSearchPopup, MessageList, ModelSelector, TopToolbar; added SketchRenderer subsystem with CodeBlockRenderer, DiffRenderer, MarkdownRenderer, TerminalRenderer, ThinkingRenderer, ToolCallRenderer.
Webview Sketch Renderers mpp-vscode/webview/src/components/sketch/\*	Implemented specialized renderers: SketchRenderer dispatcher; MarkdownRenderer for markdown content; CodeBlockRenderer for code blocks; DiffRenderer for patches; TerminalRenderer for shell output; ThinkingRenderer for reasoning; ToolCallRenderer for tool execution details.
Webview Styling mpp-vscode/webview/src/styles/index.css, components/\*.css	Added global styles (CSS reset, design system variables), component-specific stylesheets for Chat, Input, Completions, File chips, Timeline, and all sketch renderers.
Webview Hooks & Types mpp-vscode/webview/src/hooks/useVSCode.ts, types/timeline.ts, utils/codeFence.ts	Implemented useVSCode and useVSCodeState hooks for extension messaging; defined ExtensionMessage/WebviewMessage contracts; created timeline types (Message, ToolCall, Terminal, Task, Error items); added CodeBlock parser utilities.
Webview App mpp-vscode/webview/src/App.tsx, main.tsx	Implemented main App component with Timeline rendering, agent state management, extension integration, config/token/file tracking, and action handlers; bootstrapped React app entry.

Sequence Diagram

sequenceDiagram
    actor User
    participant VSCode as VSCode<br/>Extension
    participant Agent as Coding<br/>Agent
    participant PlanSvc as Plan State<br/>Service
    participant UI as Webview<br/>UI
    
    User->>VSCode: Submit task
    VSCode->>Agent: execute(input)
    Agent->>Agent: Process & invoke tool
    Agent->>VSCode: Call plan tool<br/>(CREATE)
    VSCode->>PlanSvc: createPlanFromMarkdown()
    PlanSvc->>PlanSvc: Parse, create AgentPlan<br/>notify listeners
    PlanSvc-->>VSCode: AgentPlan
    VSCode->>UI: Render plan
    UI->>User: Display tasks/steps
    
    loop Execute Plan Steps
        Agent->>Agent: Execute step
        Agent->>VSCode: Call plan tool<br/>(COMPLETE_STEP)
        VSCode->>PlanSvc: completeStep(taskId, stepId)
        PlanSvc->>PlanSvc: Update step status,<br/>refresh plan
        PlanSvc-->>VSCode: Updated plan
        VSCode->>UI: Re-render with progress
        UI->>User: Show step completed
    end
    
    Agent->>VSCode: Execution complete
    VSCode->>UI: Render final result
    UI->>User: Display summary

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120+ minutes

Specific areas requiring attention:

• Plan State Management: PlanStateService listener notifications, reactive StateFlow updates, and state consistency across create/update/clear operations (PlanStateService.kt)
• Plan Parsing & Serialization: MarkdownPlanParser regex patterns, markdown-to-model and model-to-markdown conversions, and edge case handling for nested structures (MarkdownPlanParser.kt, PlanTask.kt, PlanStep.kt)
• Tool Orchestration Changes: Replacement of TaskBoundaryTool with PlanManagementTool in ToolOrchestrator; verify parameter mapping and invocation logic (ToolOrchestrator.kt, PlanManagementTool.kt)
• VSCode Extension Messaging: Complex message contracts between webview and extension; verify message routing, error handling, and state synchronization (ChatViewProvider.ts, useVSCode.ts, extension.ts)
• File Search & Indexing: WorkspaceFileSearchProvider indexing logic, deduplication, depth/file limits, and error handling during recursive traversal (WorkspaceFileSearchProvider.kt)
• Webview Component Integration: Nested renderers (SketchRenderer→CodeBlockRenderer, etc.), state lifting, and callback propagation for actions (SketchRenderer.tsx, Timeline.tsx)
• IDE Server API: HTTP endpoint definitions for diff/file operations, auth token validation, and environment variable syncing (IDEServer.ts)
• Plan UI Synchronization: ComposeRenderer plan state tracking and integration with tool call updates (ComposeRenderer.kt, PlanPanel.kt)

Possibly related PRs

• feat: add basic MPP IDEA support #471 #473: Modifies ToolOrchestrator.kt with SubAgent execution handling; overlaps with this PR's tool orchestration changes for plan management.
• Feat task tracking UI 959ce #466: Changes task/tracking code paths and adds task state models in UI; directly related to plan/task management integration in ComposeRenderer and task model updates.

Poem

🐰 Whiskers twitching with delight!
A plan emerges, structured right—
From tasks to steps, a parse so neat,
VSCode chats make work complete!
Markdown blooms, webviews shine—
This feature branch? Simply divine! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 47.09% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat: add mpp-vscode support' clearly and specifically describes the main change: adding VSCode extension support for the mpp (multiplatform project) framework.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[phodal]

@copilot 修复 merge 问题

[Copilot]

Pull request overview

This PR adds comprehensive VSCode extension support for AutoDev, implementing a feature-rich coding assistant based on Kotlin Multiplatform technology. The extension includes a React-based webview UI, backend services for diff management and IDE server communication, and integration with the mpp-core Kotlin/JS module.

Key Changes:

• React + TypeScript webview with Timeline-based chat architecture
• VSCode extension services (diff management, IDE server, status bar)
• Bridge to mpp-core for LLM services, completion management, and coding agent
• DevIns language support with syntax highlighting and auto-completion

Reviewed changes

Copilot reviewed 104 out of 105 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
mpp-vscode/package.json	Main extension manifest with commands, views, and configuration
mpp-vscode/webview/package.json	Webview dependencies (React, Vite, TypeScript)
mpp-vscode/tsconfig.json	TypeScript configuration for extension
mpp-vscode/webview/tsconfig.json	TypeScript configuration for webview
mpp-vscode/webview/vite.config.ts	Vite build configuration for webview
mpp-vscode/vitest.config.ts	Vitest test configuration
mpp-vscode/src/extension.ts	Extension entry point with activation logic
mpp-vscode/src/bridge/mpp-core.ts	TypeScript bridge to Kotlin/JS mpp-core module
mpp-vscode/src/services/*.ts	IDE server, diff manager, status bar, config manager
mpp-vscode/src/providers/*.ts	Chat view provider and DevIns completion provider
mpp-vscode/webview/src/App.tsx	Main React application component
mpp-vscode/webview/src/components/*.tsx	React UI components (Timeline, ChatInput, ModelSelector, etc.)
mpp-vscode/webview/src/components/sketch/*.tsx	Specialized renderers for different content types
mpp-vscode/test/**/*.test.ts	Unit tests for services and providers
mpp-vscode/syntaxes/*.json	DevIns language syntax definitions

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

[nitpick] The display name 'DevIn' has inconsistent capitalization compared to other entries. Should this be 'Devin' to match the typical naming pattern?

[Copilot]

Missing type annotation for the any parameter in onAction. Consider using a more specific type:

onAction?: (action: string, data: Record<string, unknown>) => void;

[Copilot]

The any type is used multiple times for action data parameters. Consider defining a union type for all possible actions:

type ActionData = 
  | { action: 'rerun-tool'; data: { toolName: string; params: string } }
  | { action: 'accept-diff'; data: { diff: string } }
  | { action: 'run-command'; data: { command: string } }
  // ... other actions

[Copilot]

[nitpick] CSS custom properties are used without fallback values in several places. While VSCode themes should provide these, consider adding fallback values for better robustness:

background: var(--vscode-editorWidget-background, #252526);

[Copilot]

Security: dangerouslySetInnerHTML is used with user input. While escapeHtml function is used to sanitize, ensure this covers all XSS attack vectors. The current implementation looks correct, but consider using a well-tested library like DOMPurify for additional security.

[Copilot]

Unused variable JsCompressionConfig.

[Copilot]

Unused variable basePath.

[Copilot]

Unused variable nonce.

[coderabbitai]

Actionable comments posted: 19

🧹 Nitpick comments (78)

mpp-vscode/test/bridge/mpp-core.test.ts (3)

104-113: Test doesn't verify meaningful clearing behavior.

The test calls clearHistory() on a service that already has empty history, then asserts it's empty. This doesn't prove clearHistory actually clears anything.

Consider populating history before clearing:

   it('should clear history', async () => {
     const { LLMService } = await import('../../src/bridge/mpp-core');
     const service = new LLMService({
       provider: 'openai',
       model: 'gpt-4',
       apiKey: 'test-key'
     });
+    // Populate history first by calling streamMessage or directly
+    await service.streamMessage('test message', () => {});
+    expect(service.getHistory().length).toBeGreaterThan(0);
     service.clearHistory();
     expect(service.getHistory()).toEqual([]);
   });

---

129-134: Missing instantiation test for consistency.

Other classes like CompletionManager and ToolRegistry have an instantiation test. Consider adding one here for consistency:

 describe('DevInsCompiler', () => {
   it('should be importable', async () => {
     const { DevInsCompiler } = await import('../../src/bridge/mpp-core');
     expect(DevInsCompiler).toBeDefined();
   });
+
+  it('should create instance', async () => {
+    const { DevInsCompiler } = await import('../../src/bridge/mpp-core');
+    const compiler = new DevInsCompiler();
+    expect(compiler).toBeDefined();
+  });
 });

---

156-168: Consider verifying actual mock return values.

The mock returns specific values (['gpt-4', 'gpt-3.5-turbo'] and ['OPENAI', 'ANTHROPIC']), but the tests only verify the return type is an array. Stronger assertions would catch issues with the bridge layer's data transformation:

   it('should get available models', async () => {
     const { getAvailableModels } = await import('../../src/bridge/mpp-core');
     const models = getAvailableModels('openai');
     expect(Array.isArray(models)).toBe(true);
+    expect(models).toContain('gpt-4');
+    expect(models).toContain('gpt-3.5-turbo');
   });

   it('should get all providers', async () => {
     const { getAllProviders } = await import('../../src/bridge/mpp-core');
     const providers = getAllProviders();
     expect(Array.isArray(providers)).toBe(true);
+    expect(providers).toContain('OPENAI');
+    expect(providers).toContain('ANTHROPIC');
   });

mpp-vscode/webview/src/components/MessageList.tsx (4)

74-78: Prefer a stable key over array index for messages

Using index as the React key can cause subtle UI glitches if messages are ever inserted/removed anywhere but the end of the list. Consider adding a stable identifier to Message (e.g., id: string) and using that as the key.

---

50-55: Auto-scroll may override user scrolling history

Unconditionally setting scrollTop = scrollHeight on every messages change will snap the view to the bottom even when the user has scrolled up to read older messages. Consider only auto-scrolling when the user is already near the bottom, or when the newest message is from the assistant.

---

64-68: Suggestions look clickable but are inert and lack semantics

The suggestions are styled as clickable pills but rendered as plain <span> elements with no click handlers or keyboard accessibility. Consider:

• Wiring them to callbacks (e.g., onSuggestionClick) and using <button> elements, or
• At least adding role="button" and tabIndex={0} plus keyboard handlers if you keep spans.

---

26-32: Add an accessible cue for the streaming indicator

The three-dot streaming indicator is purely visual. For screen readers, consider adding an aria-label or visually hidden text (e.g., “AutoDev is responding…”) tied to the same conditional so assistive tech users get equivalent feedback.

mpp-vscode/webview/src/components/sketch/DiffRenderer.css (1)

1-129: Styles are solid; consider routing remaining hardcoded colors through shared tokens

The diff viewer styling (container, actions, states, applied overlay) is coherent and leans on VSCode theming tokens, which is good. If you want stricter design‑token discipline later, consider moving the remaining raw colors (e.g. #45a049, #da190b, white and the hex fallbacks) into shared CSS variables so they’re easier to tweak across components, but this is non‑blocking.

mpp-vscode/webview/src/components/sketch/TerminalRenderer.css (1)

34-42: Avoid hardcoded color: white for status badges.

Per coding guidelines, hardcoded colors should be avoided. Use a CSS variable with fallback for consistency with the rest of the file.

 .terminal-status.success {
   background-color: var(--vscode-testing-iconPassed, #4caf50);
-  color: white;
+  color: var(--vscode-button-foreground, #ffffff);
 }
 
 .terminal-status.error {
   background-color: var(--vscode-testing-iconFailed, #f44336);
-  color: white;
+  color: var(--vscode-button-foreground, #ffffff);
 }

mpp-vscode/webview/src/components/ChatInput.css (1)

47-50: Potential low-contrast hover state.

The .toolbar-button:hover uses --selection-background which may already be the same as the surrounding toolbar background (.input-toolbar also uses --selection-background). This could result in no visible hover effect beyond the opacity change.

Consider using a different variable or adding a subtle differentiation:

 .toolbar-button:hover:not(:disabled) {
   opacity: 1;
-  background: var(--selection-background);
+  background: var(--list-hover-background, var(--selection-background));
 }

mpp-vscode/webview/src/hooks/useVSCode.ts (1)

104-111: Consider validating message structure in onMessage handler.

The onMessage handler casts event.data directly to ExtensionMessage without validation. Malicious or malformed messages could cause runtime errors. Consider adding a type guard.

 const onMessage = useCallback((handler: (message: ExtensionMessage) => void) => {
   const listener = (event: MessageEvent<ExtensionMessage>) => {
+    // Validate message has expected structure
+    if (event.data && typeof event.data.type === 'string') {
       handler(event.data);
+    }
   };

   window.addEventListener('message', listener);
   return () => window.removeEventListener('message', listener);
 }, []);

mpp-vscode/src/bridge/mpp-core.ts (1)

440-449: Minor code duplication: model config creation.

The JsModelConfig creation logic at lines 441-449 duplicates lines 64-73 in LLMService. Consider extracting a helper function for consistency.

// Add helper function at module level
function createJsModelConfig(config: ModelConfig): any {
  const providerName = ProviderTypes[config.provider.toLowerCase()] || config.provider.toUpperCase();
  return new JsModelConfig(
    providerName,
    config.model,
    config.apiKey,
    config.temperature ?? 0.7,
    config.maxTokens ?? 8192,
    config.baseUrl ?? ''
  );
}

mpp-vscode/README.md (1)

108-111: Avoid hardcoded local filesystem paths in docs

The build step uses a machine-specific path:

cd /Volumes/source/ai/autocrud

Consider switching to a repo-relative or placeholder path (e.g., cd /path/to/autocrud or from the monorepo root) so other contributors can follow the instructions directly.

mpp-vscode/test/providers/chat-view.test.ts (2)

5-91: Good structural tests; consider adding behavior assertions for message flow

The suite validates construction and public method presence, but doesn’t assert behavior (e.g., that sendMessage uses LLMService.streamMessage or that errors/history clear are posted back to the webview). Adding a couple of focused tests around these flows would better protect the extension’s core chat UX while still using your existing mocks.

---

94-141: Private HTML helper coverage is solid; optional: assert asset URI wiring

The HTML tests nicely cover CSP, root container, and fallback inline UI markers via the private getHtmlContent. If you want slightly stronger regression protection, you could also assert that webview.asWebviewUri is called with the expected dist/webview asset paths so bundle layout changes don’t silently break the React path.

mpp-vscode/webview/src/components/sketch/ThinkingRenderer.tsx (1)

9-59: Component logic looks good; consider exporting the props type

ThinkingRenderer is clean and satisfies the guidelines (no inline colors, CSS-based styling). If you plan to reuse the props type elsewhere (e.g., in a higher-level sketch renderer API), consider exporting ThinkingRendererProps so consumers can import a single source of truth for the props shape.

mpp-vscode/test/services/status-bar.test.ts (1)

7-98: Solid smoke tests; consider fake timers for time-based behavior

The tests do a good job ensuring StatusBarManager constructs and its public methods don’t throw against mocked vscode APIs. To harden against regressions in the animation and temporary-message logic, consider using vitest fake timers to assert that showMessage eventually restores the original text and that setState('thinking'/'streaming') starts (and dispose stops) the animation interval.

mpp-vscode/webview/src/components/sketch/ToolCallRenderer.tsx (1)

10-126: Renderer is well-structured; consider using fullParams for rerun/formatting

The renderer cleanly maps success to icon/CSS state and safely parses params with a fallback. Since ToolCallInfo also exposes fullParams/fullOutput, consider preferring fullParams ?? params for formatParams and for the rerun payload, and optionally fullOutput ?? output for display. That would keep the UI concise while ensuring reruns and detailed views use the complete data rather than any truncated variant.

mpp-vscode/webview/src/components/sketch/CodeBlockRenderer.tsx (1)

9-87: Code block UX is good; tighten action typing and consider a clipboard fallback

The component provides a nice “copy / insert / apply” UX and follows the styling guideline (no inline colors). Two optional improvements:

• Narrow onAction to a string union like 'insert' | 'apply' with a typed payload to avoid accidental misuse from callers.
• For environments where navigator.clipboard is unavailable or denied, consider invoking onAction?.('insert', …) or surfacing a small inline error instead of only logging to the console so users still have a way to get the code.

mpp-vscode/test/services/ide-server.test.ts (1)

90-124: Endpoint describe block is effectively a placeholder

The “IDEServer API Endpoints” tests only assert that IDEServer is defined, so they won’t fail even if all HTTP handlers are removed. Either extend these to call the handlers (in an integration-style suite) or drop/rename this block so expectations match what’s actually tested.

mpp-vscode/webview/src/components/CompletionPopup.css (1)

27-34: Avoid redundant .completion-item.selected background rule

.completion-item:hover, .completion-item.selected and the later .completion-item.selected both set background. Because the latter wins, the .completion-item.selected part of the first rule is redundant; you can drop it to make the cascade clearer.

mpp-vscode/webview/src/components/FileSearchPopup.css (1)

89-96: Tidy up duplicated .file-item.selected background rules

You set background for .file-item:hover, .file-item.selected and then override it in a later .file-item.selected rule. The first rule could just be .file-item:hover without changing behavior.

mpp-vscode/test/providers/devins-completion.test.ts (1)

5-143: Good coverage of trigger characters; consider asserting item shape

The tests nicely cover import/instantiation and that /, @, $ prefixes yield non-empty completion arrays while regular text yields none. To catch regressions more precisely, you might additionally assert a couple of properties on returned items (e.g., label format or kind) so changes in built‑in lists or matching logic are surfaced earlier.

mpp-vscode/test/services/diff-manager.test.ts (1)

140-157: “should emit events on diff changes” test never triggers a change

In the DiffManager suite you attach a listener and assert that events is empty, but you never invoke any method (showDiff, acceptDiff, cancelDiff, etc.) that would fire onDidChange. As written, the test doesn’t validate the event pipeline. Either invoke a diff‑changing method and assert the emitted events, or rename/remove the test to match its behavior.

mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/plan/CodeFileLink.kt (1)

28-41: Regex is simple and works, but may over/under‑match in edge cases

LINK_PATTERN will happily treat any markdown link (including https://...) as a CodeFileLink, and it won’t handle file paths containing ) characters. That’s fine if your plan steps only contain simple [label](relative/path.ext) links, but if you expect more varied markdown, consider tightening the pattern (e.g., to prefer path‑like targets) or documenting this limitation explicitly.

mpp-vscode/webview/src/components/sketch/TerminalRenderer.tsx (1)

25-39: Handle copy timeout cleanup to avoid stale state updates

handleCopy sets a 2s timeout that calls setCopied(false) but doesn’t clear that timeout if the component unmounts in the meantime, which can lead to “setState on unmounted component” warnings. Consider storing the timeout ID in a ref and clearing it in a cleanup (or using an effect tied to copied) so the timer can’t fire after unmount.

Also applies to: 60-75, 103-113

mpp-vscode/webview/src/components/DevInInput.tsx (2)

11-18: Export DevInInputProps if it’s part of the public surface

If callers outside this module are expected to type the component props (as implied by the summary), declare it as:

export interface DevInInputProps {
  value: string;
  onChange: (value: string) => void;
  onKeyDown?: (e: KeyboardEvent<HTMLTextAreaElement>) => void;
  onTriggerCompletion?: (trigger: '/' | '@' | '$', position: number) => void;
  placeholder?: string;
  disabled?: boolean;
}

Otherwise, keeping it internal is fine.

---

69-87: dangerouslySetInnerHTML usage is XSS-safe as written; consider documenting this

Static analysis warns about dangerouslySetInnerHTML, but here all user input goes through escapeHtml before pattern highlighting, and the replacements only wrap the already-escaped text in <span> tags. That prevents script injection.

To avoid regressions, consider adding a short comment near getHighlightedHtml noting that:

• All user content must be passed through escapeHtml first.
• Highlighting must never insert raw user strings into the HTML template.

No code change is strictly required.

Also applies to: 93-96

mpp-core/src/commonTest/kotlin/cc/unitmesh/agent/plan/MarkdownPlanParserTest.kt (1)

65-75: Align “checkmark symbol” test with the ✓ marker

The test name suggests covering the ✓ marker, but the sample uses [x], which is already exercised in should parse step status markers correctly. To better match the documentation in TaskStatus and strengthen coverage, consider changing the step to:

val markdown = """
    1. Task with checkmark
       - [✓] Completed with checkmark
""".trimIndent()

So that both x and ✓ are explicitly tested.

mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/plan/TaskStatus.kt (1)

22-44: Consider making toMarker an instance method on TaskStatus

The logic is sound, but declaring fun TaskStatus.toMarker() inside the companion is a bit unconventional and requires importing the companion’s extension. Defining it as an instance method keeps usage simple:

@Serializable
enum class TaskStatus {
    TODO, IN_PROGRESS, COMPLETED, FAILED, BLOCKED;

    fun toMarker(): String = when (this) {
        COMPLETED -> "✓"
        FAILED -> "!"
        IN_PROGRESS -> "*"
        TODO -> " "
        BLOCKED -> "B"
    }

    companion object {
        fun fromMarker(marker: String): TaskStatus = when (marker.trim().lowercase()) {
            "x", "✓" -> COMPLETED
            "!" -> FAILED
            "*" -> IN_PROGRESS
            "", " " -> TODO
            else -> TODO
        }
    }
}

Not mandatory, but a bit more idiomatic.

mpp-vscode/src/extension.ts (2)

52-66: Return Promises from async diff commands so VS Code can observe failures

diffManager.acceptDiff / cancelDiff are asynchronous; the command handlers invoke them but don’t return the resulting Promise. This can hide rejections from VS Code and from callers.

Consider returning the Promise from the handlers:

context.subscriptions.push(
  vscode.commands.registerCommand('autodev.acceptDiff', (uri?: vscode.Uri) => {
    const docUri = uri ?? vscode.window.activeTextEditor?.document.uri;
    if (docUri && docUri.scheme === DIFF_SCHEME) {
      return diffManager.acceptDiff(docUri);
    }
  }),
  vscode.commands.registerCommand('autodev.cancelDiff', (uri?: vscode.Uri) => {
    const docUri = uri ?? vscode.window.activeTextEditor?.document.uri;
    if (docUri && docUri.scheme === DIFF_SCHEME) {
      return diffManager.cancelDiff(docUri);
    }
  })
);

This lets VS Code handle errors and completion correctly.

---

15-41: Watch for circular dependency between extension.ts and diff-manager.ts around DIFF_SCHEME

extension.ts defines and exports DIFF_SCHEME, while also importing DiffManager/DiffContentProvider from ./services/diff-manager. If diff-manager.ts in turn imports DIFF_SCHEME from this file and uses it during module initialization (rather than only inside functions), that can lead to fragile circular import behavior.

If that pattern exists, consider moving DIFF_SCHEME into a small shared module (e.g., ./constants) imported by both extension.ts and diff-manager.ts, so neither file depends on the other at load time.

mpp-vscode/webview/src/utils/codeFence.ts (1)

50-51: Language ID regex doesn't match all valid identifiers.

The regex \w+ only matches word characters (letters, digits, underscores) but language identifiers like c++ or c# won't be captured correctly.

-        const langMatch = trimmedLine.match(/^```(\w+)?/);
+        const langMatch = trimmedLine.match(/^```([a-zA-Z0-9+#_-]+)?/);

mpp-vscode/src/providers/devins-completion.ts (1)

53-61: Remove or use the completionManager field.

The completionManager is instantiated but never used in provideCompletionItems or any helper method. This is dead code that should either be removed or integrated.

If dynamic completions are not needed yet, remove the unused field:

 export class DevInsCompletionProvider implements vscode.CompletionItemProvider {
-  private completionManager: CompletionManager | undefined;
-
-  constructor() {
-    try {
-      this.completionManager = new CompletionManager();
-    } catch (e) {
-      // mpp-core not available, use built-in completions only
-    }
-  }
+  // Built-in completions only; dynamic completions can be added later

Or implement the integration if intended.

mpp-vscode/src/services/config-manager.ts (2)

118-158: Consider using async file operations.

The load() method is marked async but uses synchronous fs.existsSync, fs.mkdirSync, and fs.readFileSync. In a VSCode extension context, blocking the main thread during config load could degrade responsiveness.

 static async load(): Promise<AutoDevConfigWrapper> {
   try {
-    // Ensure directory exists
-    if (!fs.existsSync(this.CONFIG_DIR)) {
-      fs.mkdirSync(this.CONFIG_DIR, { recursive: true });
-    }
-
-    if (!fs.existsSync(this.CONFIG_FILE)) {
-      return this.createEmpty();
-    }
-
-    const content = fs.readFileSync(this.CONFIG_FILE, 'utf-8');
+    // Ensure directory exists
+    await fs.promises.mkdir(this.CONFIG_DIR, { recursive: true });
+
+    try {
+      await fs.promises.access(this.CONFIG_FILE);
+    } catch {
+      return this.createEmpty();
+    }
+
+    const content = await fs.promises.readFile(this.CONFIG_FILE, 'utf-8');
     const parsed = yaml.parse(content);

---

83-88: Missing MCP server validation in filter.

The Kotlin implementation filters with !it.value.disabled && it.value.validate(), but this implementation only checks disabled. Invalid server configurations (e.g., missing both command and url) could be returned as enabled.

Consider adding validation:

 getEnabledMcpServers(): Record<string, McpServerConfig> {
   const mcpServers = this.getMcpServers();
   return Object.fromEntries(
-    Object.entries(mcpServers).filter(([_, server]) => !server.disabled)
+    Object.entries(mcpServers).filter(([_, server]) => 
+      !server.disabled && (server.command || server.url)
+    )
   );
 }

mpp-vscode/webview/src/components/FileSearchPopup.tsx (1)

20-25: Consider reusing SelectedFile interface from FileChip.tsx.

FileItem has identical fields to SelectedFile (imported on line 10). This creates maintenance overhead if fields need to change.

-interface FileItem {
-  name: string;
-  path: string;
-  relativePath: string;
-  isDirectory: boolean;
-}
+// Use SelectedFile from FileChip.tsx
+type FileItem = SelectedFile;

mpp-vscode/webview/src/components/FileChip.tsx (1)

42-46: Add aria-label to remove button for accessibility.

The button has a title attribute which provides a tooltip, but aria-label is preferred for screen reader announcements.

-      <button className="file-chip-remove" onClick={onRemove} title="Remove from context">
+      <button className="file-chip-remove" onClick={onRemove} title="Remove from context" aria-label={`Remove ${file.name} from context`}>

mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/context/FileChip.kt (1)

160-171: Consider adding click feedback for the remove button.

The remove icon uses clickable without an indication, which means no visual feedback on click. Consider adding a ripple or hover effect for better UX.

         Icon(
             imageVector = AutoDevComposeIcons.Close,
             contentDescription = "Remove from context",
             modifier = Modifier
                 .size(16.dp)
-                .clickable(onClick = onRemove),
+                .clickable(
+                    interactionSource = remember { MutableInteractionSource() },
+                    indication = null, // or use LocalIndication.current for ripple
+                    onClick = onRemove
+                ),
             tint = if (isHovered) {
                 MaterialTheme.colorScheme.onSurface
             } else {
                 MaterialTheme.colorScheme.onSurface.copy(alpha = 0.4f)
             }
         )

mpp-vscode/webview/src/components/sketch/SketchRenderer.tsx (1)

42-46: Consider using a more stable key than index.

Using index as a key can cause issues with React reconciliation if blocks are inserted or reordered during streaming. Consider generating a stable identifier, such as combining block position with a hash of the content prefix.

       {blocks.map((block, index) => {
         const isLastBlock = index === blocks.length - 1;
         const blockIsComplete = block.isComplete && (isComplete || !isLastBlock);

         return (
-          <div key={index} className="sketch-block">
+          <div key={`${index}-${block.languageId}-${block.text.slice(0, 20)}`} className="sketch-block">
             {renderBlock(block, blockIsComplete, onAction)}
           </div>
         );
       })}

mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt (1)

241-246: Consider adding error logging for invalid plan markdown.

When planMarkdown is blank for CREATE/UPDATE actions, the code silently does nothing. Consider logging a warning for debugging purposes.

             "CREATE", "UPDATE" -> {
-                if (planMarkdown.isNotBlank()) {
-                    _currentPlan = MarkdownPlanParser.parseToPlan(planMarkdown)
+                if (planMarkdown.isNotBlank()) {
+                    _currentPlan = MarkdownPlanParser.parseToPlan(planMarkdown)
+                } else {
+                    // Consider logging: plan markdown is empty for $action
                 }
             }

mpp-vscode/webview/src/App.tsx (2)

119-137: Direct mutation of timeline item - potential React state issue.

The toolResult handler mutates item.toolCall directly within the spread array. While this works because the outer array is spread, it's safer to create a new object for the modified item to avoid subtle React reconciliation issues.

       case 'toolResult':
         if (msg.data) {
           setAgentState(prev => {
             const timeline = [...prev.timeline];
             // Find the last tool call and update it
             for (let i = timeline.length - 1; i >= 0; i--) {
               if (timeline[i].type === 'tool_call') {
                 const item = timeline[i] as ToolCallTimelineItem;
-                item.toolCall = {
-                  ...item.toolCall,
-                  success: msg.data?.success as boolean | undefined,
-                  output: msg.data?.output as string | undefined,
-                  summary: msg.data?.summary as string | undefined
-                };
+                timeline[i] = {
+                  ...item,
+                  toolCall: {
+                    ...item.toolCall,
+                    success: msg.data?.success as boolean | undefined,
+                    output: msg.data?.output as string | undefined,
+                    summary: msg.data?.summary as string | undefined
+                  }
+                };
                 break;
               }
             }
             return { ...prev, timeline };
           });
         }
         break;

---

321-345: Potential memory leak in handlePromptOptimize.

The event listener added via window.addEventListener may not be cleaned up if the component unmounts before the timeout fires or the response arrives. Consider using AbortController or cleanup in a useEffect.

   const handlePromptOptimize = useCallback(async (prompt: string): Promise<string> => {
     return new Promise((resolve) => {
       postMessage({ type: 'action', action: 'optimizePrompt', data: { prompt } });

+      let resolved = false;
       const handler = (event: MessageEvent) => {
         const msg = event.data;
         if (msg.type === 'promptOptimized' && msg.data?.optimizedPrompt) {
+          resolved = true;
           window.removeEventListener('message', handler);
           resolve(msg.data.optimizedPrompt as string);
         } else if (msg.type === 'promptOptimizeFailed') {
+          resolved = true;
           window.removeEventListener('message', handler);
           resolve(prompt);
         }
       };
       window.addEventListener('message', handler);

       setTimeout(() => {
-        window.removeEventListener('message', handler);
-        resolve(prompt);
+        if (!resolved) {
+          window.removeEventListener('message', handler);
+          resolve(prompt);
+        }
       }, 30000);
     });
   }, [postMessage]);

mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/plan/MarkdownPlanParser.kt (1)

113-119: Consider handling empty task list edge case.

The formatToMarkdown function works correctly but returns an empty string for an empty task list. This is probably fine, but worth documenting or handling explicitly if callers expect specific behavior.

     /**
      * Format a list of tasks back to markdown.
+     * Returns an empty string if no tasks are provided.
      */
     fun formatToMarkdown(tasks: List<PlanTask>): String {
+        if (tasks.isEmpty()) return ""
         val sb = StringBuilder()
         tasks.forEachIndexed { index, task ->
             sb.append(task.toMarkdown(index + 1))
         }
         return sb.toString()
     }

mpp-vscode/webview/src/components/sketch/DiffRenderer.tsx (1)

24-37: Consider memoizing parsed diff and handling CRLF/empty input more defensively

parseDiff(diffContent) runs on every render and assumes \n only; mixed \r\n input will leak \r into content, and large diffs may be slightly wasteful to re-parse on each re-render. You could useMemo around parseDiff and strip trailing \r per line (or normalize line endings) to keep rendering clean and predictable, especially when integrating with other tools that emit CRLF patches.

Also applies to: 88-100

mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/context/TopToolbar.kt (1)

32-38: remember for effectiveSearchProvider should take fileSearchProvider as a key

effectiveSearchProvider is created with remember { fileSearchProvider ?: DefaultFileSearchProvider }, so if a parent ever changes fileSearchProvider at runtime, this composable will keep using the initial instance. Prefer remember(fileSearchProvider) { ... } so injected providers can be swapped predictably (tests, previews, different workspaces).

mpp-vscode/webview/src/components/ModelSelector.tsx (1)

50-64: Optional: add type="button" and basic ARIA hints for better accessibility

Since the trigger and list items are plain <button> elements, adding type="button" on the trigger and aria-haspopup="listbox" / aria-expanded={isOpen} would avoid accidental form submission and improve screen reader behavior without changing runtime logic.

Also applies to: 66-108

mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/DevInEditorInput.kt (2)

108-110: remember for effectiveSearchProvider should depend on fileSearchProvider

val effectiveSearchProvider = remember { fileSearchProvider ?: WorkspaceFileSearchProvider() } will ignore future changes to fileSearchProvider. If a different provider is ever passed (tests, alternate workspace), it won’t be picked up. Use remember(fileSearchProvider) { fileSearchProvider ?: WorkspaceFileSearchProvider() } to keep behavior intuitive.

---

111-139: Confirm that built-in commands should also carry file context and clear selections

buildAndSendMessage always appends DevIns file commands for selectedFiles, calls callbacks?.onSubmit(fullText, getFileContexts()), then clears selectedFiles. This path is now used for:

• Desktop Enter send
• Mobile IME send
• Bottom toolbar send
• Auto-executed completions for /init, /clear, /help

If the intent is that all messages, including these built-ins, should be contextual and also clear any selected files, this is fine. If some commands (e.g. /clear or /help) should ignore file context and/or not reset selections, you may want separate helpers for “normal” messages vs. meta-commands.

Also applies to: 315-316, 435-443, 553-560, 631-639

mpp-vscode/webview/src/components/CompletionPopup.tsx (1)

58-97: Be mindful of global keydown listener interactions with other components

The popup attaches a window-level keydown listener while isOpen. This is fine, but if other parts of the webview also rely on global shortcuts, there’s a chance of conflicts. If this becomes an issue, you could:

• Restrict handling to specific keys and conditions (as you already do), and/or
• Gate the listener behind a higher-level “active input” flag so only the focused editor’s popup consumes navigation keys.

mpp-vscode/webview/src/components/Timeline.tsx (1)

63-77: Prefer explicit timeline item types instead of inline { success: boolean; message: string }

TaskCompleteItemRenderer and ErrorItemRenderer currently accept structural { success: boolean; message: string } / { message: string }, even though TimelineItem already carries discriminated variants. Using the specific timeline item types here (e.g. TaskCompleteTimelineItem, ErrorTimelineItem) would tighten typing and keep these renderers aligned with any future schema changes.

Also applies to: 133-145

mpp-vscode/src/services/ide-server.ts (1)

194-203: Port file is not cleaned up on stop.

The port file written in writePortFile() is never deleted when the server stops. This leaves stale connection info (including the auth token) on disk, which could cause confusion for external tools or represent a minor security concern.

   async stop(): Promise<void> {
     if (this.server) {
       return new Promise((resolve) => {
         this.server!.close(() => {
           this.log('IDE Server stopped');
+          this.deletePortFile();
           resolve();
         });
       });
     }
   }

+  private async deletePortFile(): Promise<void> {
+    if (this.portFile) {
+      try {
+        await fs.unlink(this.portFile);
+        this.log(`Port file deleted: ${this.portFile}`);
+      } catch {
+        // Ignore if file doesn't exist
+      }
+    }
+  }

mpp-core/src/commonTest/kotlin/cc/unitmesh/agent/tool/impl/PlanManagementToolTest.kt (1)

51-74: Consider adding edge case tests for invalid indices.

The COMPLETE_STEP and FAIL_STEP tests use valid indices. Consider adding tests for out-of-bounds taskIndex or stepIndex to ensure graceful error handling.

@Test
fun `should return error for invalid task index on COMPLETE_STEP`() = runTest {
    val tool = createTool()
    
    // Create a plan first
    val createParams = PlanManagementParams(
        action = "CREATE",
        planMarkdown = "1. Task\n   - [ ] Step 1"
    )
    tool.createInvocation(createParams).execute(ToolExecutionContext())
    
    // Try to complete with invalid task index
    val params = PlanManagementParams(action = "COMPLETE_STEP", taskIndex = 99, stepIndex = 1)
    val result = tool.createInvocation(params).execute(ToolExecutionContext())
    
    assertIs<ToolResult.Error>(result)
}

mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/plan/AgentPlan.kt (2)

119-137: idCounter is not thread-safe in Kotlin Multiplatform context.

The idCounter uses a simple ++idCounter which is not atomic. In concurrent scenarios (e.g., multiple coroutines creating plans), this could produce duplicate IDs. The same pattern exists in PlanTask.kt and PlanStep.kt per the provided snippets.

For thread-safe ID generation, consider using AtomicLong (on JVM) or a platform-specific solution:

 companion object {
-    private var idCounter = 0L
+    private val idCounter = kotlinx.atomicfu.atomic(0L)
     
     fun generateId(): String {
-        return "plan_${++idCounter}_${Clock.System.now().toEpochMilliseconds()}"
+        return "plan_${idCounter.incrementAndGet()}_${Clock.System.now().toEpochMilliseconds()}"
     }
 }

Alternatively, rely solely on timestamps with random components (similar to RendererModels.kt pattern) to avoid shared mutable state.

---

21-22: Mutable list in data class can cause serialization and copy issues.

Using MutableList as a property in a data class can lead to unexpected behavior with copy() (shallow copy shares the list) and may complicate serialization. Consider using an immutable List with builder methods that return new instances, or document the intended mutation pattern.

If mutation is required, consider encapsulating it:

data class AgentPlan(
    val id: String,
    private val _tasks: MutableList<PlanTask> = mutableListOf(),
    // ...
) {
    val tasks: List<PlanTask> get() = _tasks
    
    fun addTask(task: PlanTask): AgentPlan {
        _tasks.add(task)
        touch()
        return this
    }
}

mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/plan/PlanTask.kt (3)

3-3: Unused import of toMarker.

TaskStatus.Companion.toMarker is imported but not used in this file. Only fromMarker is used on line 146.

-import cc.unitmesh.agent.plan.TaskStatus.Companion.toMarker

---

149-153: Same thread-safety concern as AgentPlan.generateId().

The idCounter pattern here has the same non-atomic increment issue. Consider applying a consistent fix across AgentPlan, PlanTask, and PlanStep (as noted in the external snippets).

---

83-86: Silent no-op when step ID is not found.

updateStepStatus silently ignores invalid stepId values. This may be intentional, but consider returning a Boolean or throwing to help callers detect programming errors.

-    fun updateStepStatus(stepId: String, newStatus: TaskStatus) {
-        steps.find { it.id == stepId }?.updateStatus(newStatus)
+    fun updateStepStatus(stepId: String, newStatus: TaskStatus): Boolean {
+        val step = steps.find { it.id == stepId } ?: return false
+        step.updateStatus(newStatus)
         updateStatusFromSteps()
+        return true
     }

mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/plan/PlanStep.kt (3)

29-29: Mutable var in data class affects equals/hashCode stability.

Using var status in a data class means objects may produce different hash codes over their lifetime. If PlanStep is used as a key in a Map or stored in a Set, status changes could break collection invariants.

Consider either:

1. Making status immutable and returning new instances via copy(status = newStatus)
2. Excluding status from data class generation and managing it separately

---

79-79: Emoji in regex may cause issues on WASM platform.

The STEP_PATTERN regex includes ✓ which, per coding guidelines, should be avoided for WASM platform compatibility. Consider using only ASCII alternatives like x, X, or other markers.

-        private val STEP_PATTERN = Regex("^\\s*-\\s*\\[\\s*([xX!*✓]?)\\s*]\\s*(.*)")
+        private val STEP_PATTERN = Regex("^\\s*-\\s*\\[\\s*([xX!*]?)\\s*]\\s*(.*)")

As per coding guidelines, avoid emoji and UTF-8 in code for WASM platform.

---

113-122: idCounter increment is not thread-safe; consider using atomic operations.

The ++idCounter operation is not atomic and can cause duplicate IDs in concurrent scenarios. Additionally, the ID generation pattern is inconsistent with RendererModels.kt which uses Platform.getCurrentTimestamp() with a random component.

Consider using AtomicLong or aligning with the existing pattern in RendererModels.kt:

-        private var idCounter = 0L
-        
-        private fun generateId(): String {
-            return "step_${++idCounter}_${currentTimeMillis()}"
-        }
+        private val idCounter = kotlinx.atomicfu.atomic(0L)
+        
+        private fun generateId(): String {
+            return "step_${idCounter.incrementAndGet()}_${currentTimeMillis()}"
+        }

mpp-vscode/src/services/status-bar.ts (1)

95-103: VSCode icon spin modifiers may conflict with manual animation.

The $(loading~spin), $(sync~spin), and $(gear~spin) icons use VSCode's built-in ~spin modifier which already provides animation. Cycling through these frames via setInterval may create a jarring double-animation effect.

Consider either:

1. Using non-spinning icons and relying on your frame animation
2. Using a single spinning icon without manual frame cycling

   private getThinkingIcon(): string {
-    const frames = ['$(loading~spin)', '$(sync~spin)', '$(gear~spin)'];
-    return frames[this.animationFrame % frames.length];
+    return '$(loading~spin)';
   }

mpp-vscode/syntaxes/DevIns.tmLanguage.json (1)

19-24: FrontMatter end pattern may not match correctly.

The end pattern (^|\\G)-{3}|\\.{3}\\s*$ seems to have precedence issues. The | operator in regex has low precedence, so this matches either (^|\\G)-{3} or \\.{3}\\s*$, but the intent appears to be matching --- or ... at line start.

Consider grouping:

-      "end": "(^|\\G)-{3}|\\.{3}\\s*$"
+      "end": "(^|\\G)(-{3}|\\.{3})\\s*$"

mpp-vscode/webview/src/components/TopToolbar.tsx (1)

30-41: Move ContextIndicator outside the component to prevent recreation on each render.

Defining ContextIndicator inside TopToolbar causes a new function reference on every render, which can lead to unnecessary re-renders of child components.

+const ContextIndicator: React.FC<{
+  autoAddCurrentFile: boolean;
+  onToggleAutoAdd?: () => void;
+}> = ({ autoAddCurrentFile, onToggleAutoAdd }) => (
+  <button
+    className={`context-indicator ${autoAddCurrentFile ? 'active' : ''}`}
+    onClick={onToggleAutoAdd}
+    title={autoAddCurrentFile ? 'Auto-add current file: ON' : 'Auto-add current file: OFF'}
+  >
+    <svg width="12" height="12" viewBox="0 0 16 16" fill="currentColor">
+      <path d="M14 4.5V14a2 2 0 0 1-2 2H4a2 2 0 0 1-2-2V2a2 2 0 0 1 2-2h5.5L14 4.5zm-3 0A1.5 1.5 0 0 1 9.5 3V1H4a1 1 0 0 0-1 1v12a1 1 0 0 0 1 1h8a1 1 0 0 0 1-1V4.5h-2z"/>
+    </svg>
+    <span className="indicator-dot" />
+  </button>
+);
+
 export const TopToolbar: React.FC<TopToolbarProps> = ({
   selectedFiles, onAddFile, onRemoveFile, onClearFiles, autoAddCurrentFile = true, onToggleAutoAdd
 }) => {
   // ...
-  const ContextIndicator = () => ( /* ... */ );

Then update usage to <ContextIndicator autoAddCurrentFile={autoAddCurrentFile} onToggleAutoAdd={onToggleAutoAdd} />.

mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/PlanPanel.kt (2)

71-78: Unusual pattern: planIcon() explicitly returns Unit while rendering content.

The function signature fun TaskStatus.planIcon(): Unit is misleading. It renders an Icon composable but explicitly declares Unit return type. Consider either:

1. Remove the explicit return type (Kotlin infers Unit)
2. Rename to PlanIcon() following Compose naming conventions for component functions

 @Composable
-fun TaskStatus.planIcon(): Unit = when (this) {
+fun TaskStatus.PlanIcon() {
+  when (this) {
     TaskStatus.TODO -> Icon(Icons.Default.RadioButtonUnchecked, null, tint = planColor)
     // ...
+  }
 }

---

199-221: Animation resources created even when not used.

The infiniteTransition and angle animation are created for every PlanStepItem, but only used when step.status == TaskStatus.IN_PROGRESS. This creates unnecessary animation overhead for most steps.

Consider conditionally creating the animation:

 @Composable
 private fun PlanStepItem(step: PlanStep, onClick: (() -> Unit)?) {
-    val infiniteTransition = rememberInfiniteTransition()
-    val angle by infiniteTransition.animateFloat(
-        initialValue = 0f,
-        targetValue = 360f,
-        animationSpec = infiniteRepeatable(animation = tween(2000, easing = LinearEasing), repeatMode = RepeatMode.Restart)
-    )
+    val angle = if (step.status == TaskStatus.IN_PROGRESS) {
+        val infiniteTransition = rememberInfiniteTransition()
+        infiniteTransition.animateFloat(
+            initialValue = 0f,
+            targetValue = 360f,
+            animationSpec = infiniteRepeatable(animation = tween(2000, easing = LinearEasing), repeatMode = RepeatMode.Restart)
+        ).value
+    } else 0f

mpp-vscode/webview/src/components/ChatInput.tsx (1)

157-165: Keyboard handling incomplete when completion popup is open.

When completionOpen is true, the function returns early without preventing default. This means Enter key events will still bubble up. Consider letting the CompletionPopup component handle its own keyboard events or explicitly handle arrow keys/Enter for completion navigation here.

 const handleKeyDown = (e: KeyboardEvent<HTMLTextAreaElement>) => {
-  // Don't submit if completion popup is open
-  if (completionOpen) return;
+  // Let completion popup handle its keyboard events
+  if (completionOpen) {
+    // Prevent submit on Enter when popup is open
+    if (e.key === 'Enter') {
+      e.preventDefault();
+    }
+    return;
+  }

   if (e.key === 'Enter' && !e.shiftKey) {
     e.preventDefault();
     handleSubmit();
   }
 };

mpp-vscode/webview/src/components/FileChip.css (3)

20-22: Hardcoded fallback color violates coding guidelines.

The fallback #73c991 is a hardcoded color. As per coding guidelines, colors should always use design tokens for consistency across platforms.

Consider removing the fallback or using another CSS variable:

 .file-chip.directory {
-  border-color: var(--vscode-gitDecoration-untrackedResourceForeground, #73c991);
+  border-color: var(--vscode-gitDecoration-untrackedResourceForeground, var(--foreground));
 }

Based on coding guidelines: "DO NOT hardcode colors... Always use design tokens."

---

32-34: Same hardcoded fallback color issue.

 .file-chip.directory .file-chip-icon {
-  color: var(--vscode-gitDecoration-untrackedResourceForeground, #73c991);
+  color: var(--vscode-gitDecoration-untrackedResourceForeground, var(--foreground));
 }

---

69-72: Hardcoded RGBA fallback color.

The rgba(255, 0, 0, 0.2) fallback is a hardcoded color.

 .file-chip-remove:hover {
   opacity: 1 !important;
-  background: var(--vscode-inputValidation-errorBackground, rgba(255, 0, 0, 0.2));
+  background: var(--vscode-inputValidation-errorBackground);
 }

If the fallback is essential, consider defining a CSS custom property at the root level.

mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/context/SelectedFileItem.kt (1)

28-39: Inconsistent truncation logic between truncatedPath and truncatePath.

The truncatedPath property uses takeLast(4) with a 40-char threshold, while the standalone truncatePath function uses takeLast(3) with a configurable maxLength (default 30). The doc comment for both says "last 3-4 parts" which is ambiguous.

Consider consolidating the logic to avoid duplication and ensure consistency:

 val truncatedPath: String
     get() {
-        val parentPath = relativePath.substringBeforeLast("/", "")
-        if (parentPath.isEmpty()) return ""
-        if (parentPath.length <= 40) return parentPath
-
-        val parts = parentPath.split("/")
-        if (parts.size <= 2) return "...$parentPath"
-
-        val keepParts = parts.takeLast(4)
-        return "...${keepParts.joinToString("/")}"
+        return truncatePath(relativePath, maxLength = 40)
     }

And update truncatePath to handle the logic consistently (adjusting takeLast count as needed).

Also applies to: 61-75

mpp-vscode/src/providers/chat-view.ts (2)

577-603: Using Node.js fs module instead of VSCode API.

The openMcpConfig method uses Node.js fs.promises directly. For consistency and better cross-platform support in VSCode extensions, prefer vscode.workspace.fs.

 private async openMcpConfig(): Promise<void> {
   try {
     const homeDir = process.env.HOME || process.env.USERPROFILE || '';
     const mcpConfigPath = `${homeDir}/.autodev/mcp.json`;
+    const uri = vscode.Uri.file(mcpConfigPath);

-    const fs = await import('fs').then(m => m.promises);
     try {
-      await fs.access(mcpConfigPath);
+      await vscode.workspace.fs.stat(uri);
     } catch {
       const defaultConfig = { mcpServers: {} };
-      await fs.mkdir(`${homeDir}/.autodev`, { recursive: true });
-      await fs.writeFile(mcpConfigPath, JSON.stringify(defaultConfig, null, 2));
+      const dirUri = vscode.Uri.file(`${homeDir}/.autodev`);
+      await vscode.workspace.fs.createDirectory(dirUri);
+      await vscode.workspace.fs.writeFile(uri, Buffer.from(JSON.stringify(defaultConfig, null, 2)));
     }

-    const uri = vscode.Uri.file(mcpConfigPath);
     await vscode.window.showTextDocument(uri);

---

972-979: Generated nonce is not used in CSP.

The getNonce() method generates a nonce, but it's called at line 884 and never actually used in the Content-Security-Policy header (line 892). The CSP uses 'unsafe-inline' instead.

Either remove the unused getNonce() call or implement proper nonce-based CSP:

-  <meta http-equiv="Content-Security-Policy" content="default-src 'none'; style-src ${webview.cspSource} 'unsafe-inline'; script-src ${webview.cspSource} 'unsafe-inline';">
+  <meta http-equiv="Content-Security-Policy" content="default-src 'none'; style-src ${webview.cspSource} 'nonce-${nonce}'; script-src ${webview.cspSource} 'nonce-${nonce}';">

Then add nonce="${nonce}" to inline script tags.

mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/context/WorkspaceFileSearchProvider.kt (1)

47-84: Potential race condition in buildIndex().

If buildIndex() is called concurrently from multiple coroutines, the check at lines 63-67 may pass for both before either sets INDEXING, causing duplicate indexing work. Consider using a mutex or atomic state transition.

+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
+
 class WorkspaceFileSearchProvider(
     private val recentFilesProvider: RecentFilesProvider? = null
 ) : FileSearchProvider {
+    private val indexMutex = Mutex()
 
     // ... existing fields ...
 
-    suspend fun buildIndex() = withContext(Dispatchers.Default) {
+    suspend fun buildIndex() = indexMutex.withLock {
+        withContext(Dispatchers.Default) {
             // ... existing implementation ...
+        }
     }

mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/context/FileSearchPopup.kt (1)

95-113: Debounce implementation may produce out-of-order results.

Rapid typing triggers multiple coroutines; the last to complete wins regardless of query order. Consider canceling previous search jobs or using a MutableStateFlow with debounce().

// Alternative: Track and cancel previous job
private var searchJob: Job? = null

fun performSearch(query: String) {
    if (query.length < 2 || currentWorkspace == null || indexingState != IndexingState.READY) {
        searchResults = emptyList()
        isLoading = false
        return
    }

    isLoading = true
    searchJob?.cancel()
    searchJob = scope.launch {
        delay(150)
        try {
            searchResults = searchProvider.searchFiles(query)
        } catch (e: Exception) {
            searchResults = emptyList()
        } finally {
            isLoading = false
        }
    }
}

mpp-vscode/test/mocks/vscode.ts (1)

14-17: Uri.parse may throw on malformed URIs.

The new URL(value) constructor throws for invalid URLs. Consider adding try-catch or documenting this behavior for test authors.

   parse: (value: string) => {
-    const url = new URL(value);
-    return { scheme: url.protocol.replace(':', ''), path: url.pathname, fsPath: url.pathname, toString: () => value };
+    try {
+      const url = new URL(value);
+      return { scheme: url.protocol.replace(':', ''), path: url.pathname, fsPath: url.pathname, toString: () => value };
+    } catch {
+      // Fallback for non-URL strings (e.g., "file.txt")
+      return { scheme: 'file', path: value, fsPath: value, toString: () => value };
+    }
   }

mpp-vscode/src/services/diff-manager.ts (1)

250-259: Type assertion on tab.input may be fragile.

The cast to { modified?: vscode.Uri; original?: vscode.Uri } assumes the diff input structure. Consider using optional chaining with more defensive checks.

     for (const tabGroup of vscode.window.tabGroups.all) {
       for (const tab of tabGroup.tabs) {
-        const input = tab.input as { modified?: vscode.Uri; original?: vscode.Uri };
-        if (input && input.modified?.toString() === rightDocUri.toString()) {
+        const input = tab.input;
+        if (input && typeof input === 'object' && 'modified' in input) {
+          const diffInput = input as { modified?: vscode.Uri };
+          if (diffInput.modified?.toString() === rightDocUri.toString()) {
             await vscode.window.tabGroups.close(tab);
             return;
+          }
         }
       }
     }

mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/tool/impl/PlanManagementTool.kt (1)

70-84: Redundant parsing in updatePlan().

Line 74 parses planMarkdown into tasks, but if no plan exists (line 75), createPlanFromMarkdown is called which parses the markdown again internally. Consider reusing the parsed tasks.

     private fun updatePlan(): ToolResult {
         if (params.planMarkdown.isBlank()) {
             return ToolResult.Error("planMarkdown is required for UPDATE", ToolErrorType.MISSING_REQUIRED_PARAMETER.code)
         }
         val tasks = MarkdownPlanParser.parse(params.planMarkdown)
         if (planStateService.currentPlan.value == null) {
-            val plan = planStateService.createPlanFromMarkdown(params.planMarkdown)
+            val plan = planStateService.createPlan(tasks)
             return ToolResult.Success("Plan created with ${plan.taskCount} tasks.\n\n${plan.toMarkdown()}",
                 mapOf("plan_id" to plan.id, "task_count" to plan.taskCount.toString()))
         }
         // ... rest unchanged
     }

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Stricter validation for taskIndex/stepIndex would avoid mis-targeting steps

In executePlanManagementTool, both indices are derived as:

val taskIndex = when (val v = params["taskIndex"]) {
    is Number -> v.toInt()
    is String -> v.toIntOrNull() ?: 0
    else -> 0
}

This means a bad value like "abc" silently becomes 0, and missing indices are indistinguishable from an explicit 0. For plan operations this could accidentally act on the first task/step instead of failing fast.

Consider:

• Treating parse failures as an error (ToolResult.Error("taskIndex must be an integer")), and
• Using a sentinel (e.g. -1) or nullable field in PlanManagementParams to represent “no index provided”, if the tool supports actions that don’t require indices.

This will make plan actions safer and easier to debug when the LLM or caller sends bad parameters.

Also applies to: 383-392

🤖 Prompt for AI Agents

mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/orchestrator/ToolOrchestrator.kt
around lines 678-709 (and also adjust same pattern at 383-392): the current
parsing of taskIndex/stepIndex silently maps invalid or missing values to 0
which can accidentally target the first task/step; instead, validate inputs and
fail fast on parse errors and distinguish “not provided” from an explicit 0 — if
params contain a non-null value that is a String attempt to parse it and return
ToolResult.Error("taskIndex must be an integer") or ToolResult.Error("stepIndex
must be an integer") on parse failure, and for absent keys or nulls pass a
sentinel (e.g. -1) or use nullable Ints in PlanManagementParams (update the
PlanManagementParams type accordingly), then construct the invocation with the
corrected/tightened taskIndex/stepIndex values so bad input is rejected rather
than defaulting to 0.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Thread-safety concern with listener collection.

The mutableListOf is not thread-safe. If addListener/removeListener are called from different coroutines while notifyX methods iterate, a ConcurrentModificationException can occur.

Consider using a thread-safe collection:

-    private val listeners = mutableListOf<PlanUpdateListener>()
+    private val listeners = mutableListOf<PlanUpdateListener>()
+    private val listenersLock = Any()

Then synchronize access in addListener, removeListener, and notification methods, or use CopyOnWriteArrayList if available on all target platforms.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/plan/PlanStateService.kt
around line 26, the listeners field is initialized with mutableListOf which is
not thread-safe; replace it with a thread-safe collection (e.g.,
java.util.concurrent.CopyOnWriteArrayList when available on targets) or wrap
access with a mutex/synchronized block, and update addListener, removeListener
and all notification methods to use the chosen synchronization strategy (either
operate directly on the CopyOnWriteArrayList or acquire the mutex before
mutating/iterating) to prevent ConcurrentModificationException from concurrent
coroutine access.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

StateFlow may not emit updates when mutating in-place.

StateFlow uses structural equality to determine if a new value should be emitted. Mutating plan.tasks in-place (via clear() and addAll()) without reassigning _currentPlan.value means collectors won't receive update notifications since the object reference hasn't changed.

Consider creating a new AgentPlan instance or explicitly re-emitting:

 fun updatePlan(tasks: List<PlanTask>) {
     val plan = _currentPlan.value
     if (plan != null) {
         plan.tasks.clear()
         plan.tasks.addAll(tasks)
+        _currentPlan.value = plan  // Force re-emission
         notifyPlanUpdated(plan)
     } else {
         createPlan(tasks)
     }
 }

Note: Even with re-assignment, StateFlow checks equality. You may need to create a copy or use a different state holder pattern for mutable data.

🤖 Prompt for AI Agents

In mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/plan/PlanStateService.kt
around lines 62 to 71, the code mutates the existing AgentPlan.tasks in-place so
StateFlow may not emit updates because the value reference doesn't change;
instead create a new AgentPlan instance (or a copy with the updated tasks) and
assign it to _currentPlan.value so the StateFlow sees a new value, and then call
notifyPlanUpdated with that new instance (or otherwise re-emit the updated
plan).

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Same StateFlow mutation issue in addTask.

Similar to updatePlan, calling plan.addTask(task) mutates the existing plan without re-emitting the StateFlow value.

Apply the same fix pattern as suggested for updatePlan.

🤖 Prompt for AI Agents

In mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/plan/PlanStateService.kt
around lines 84 to 88, addTask currently mutates the existing Plan instance
(plan.addTask(task)) which won't re-emit the StateFlow; instead create an
updated Plan instance (e.g., copy the plan with a new tasks list that includes
the new task or use an immutable builder), then assign that new Plan to the
StateFlow (or call notifyPlanUpdated with the new Plan) so the StateFlow value
is replaced rather than mutated in-place.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Replace println with a proper logging framework.

Using println for logging is not production-ready and should be replaced with a proper logging framework (e.g., SLF4J, Kotlin logging) to enable:

• Log level control (debug/info/warn/error)
• Conditional enabling/disabling
• Proper output redirection
• Structured logging

Consider using a logging framework like this:

private val logger = LoggerFactory.getLogger(DefaultFileSystem::class.java)

actual override fun searchFiles(pattern: String, maxDepth: Int, maxResults: Int): List<String> {
    return try {
        logger.debug("searchFiles called: pattern={}, projectPath={}", pattern, projectPath)
        val projectRoot = Path.of(projectPath)
        if (!projectRoot.exists() || !projectRoot.isDirectory()) {
            logger.warn("Project root does not exist or is not a directory")
            return emptyList()
        }

🤖 Prompt for AI Agents

In
mpp-core/src/jvmMain/kotlin/cc/unitmesh/devins/filesystem/DefaultFileSystem.jvm.kt
around lines 108 to 111, replace the println calls with a proper logger: add a
private SLF4J LoggerFactory logger for DefaultFileSystem, use logger.debug for
the "searchFiles called" message with parameterized placeholders, and use
logger.warn for the "Project root does not exist or is not a directory" message;
ensure imports are added and no println() calls remain so logging can be
controlled by log levels and configuration.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Operator precedence issue in needsConfig logic.

The && has higher precedence than ||, causing incorrect evaluation. The current logic is: (A && B.includes('X')) || B.includes('Y') instead of the intended A && (B.includes('X') || B.includes('Y')).

   // Check if we need to show config prompt
-  const needsConfig = agentState.timeline.length === 0 &&
-    agentState.currentStreamingContent.includes('No configuration found') ||
-    agentState.currentStreamingContent.includes('Configuration Required');
+  const needsConfig = agentState.timeline.length === 0 && (
+    agentState.currentStreamingContent.includes('No configuration found') ||
+    agentState.currentStreamingContent.includes('Configuration Required')
+  );

📝 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

	// Check if we need to show config prompt
	const needsConfig = agentState.timeline.length === 0 &&
	agentState.currentStreamingContent.includes('No configuration found') ||
	agentState.currentStreamingContent.includes('Configuration Required');
	// Check if we need to show config prompt
	const needsConfig = agentState.timeline.length === 0 && (
	agentState.currentStreamingContent.includes('No configuration found') ||
	agentState.currentStreamingContent.includes('Configuration Required')
	);

🤖 Prompt for AI Agents

In mpp-vscode/webview/src/App.tsx around lines 362 to 365, the boolean
expression for needsConfig mixes && and || without grouping so it evaluates as
(A && B.includes('No configuration found')) || B.includes('Configuration
Required') instead of the intended A && (B.includes('No configuration found') ||
B.includes('Configuration Required')). Fix by adding parentheses around the two
includes checks: ensure the timeline-empty check is ANDed with the combined OR
of the two includes calls, so the overall condition is timeline.length === 0 &&
(currentStreamingContent.includes(...) ||
currentStreamingContent.includes(...)).

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Add ARIA attributes for accessibility.

The popup lacks accessibility markup. Screen reader users won't understand the component's purpose or be able to navigate it effectively.

Key additions needed:

• Dialog: role="dialog" and aria-modal="true"
• Input: aria-label="Search files" or visible label
• List: role="listbox" with aria-label
• Items: role="option" and aria-selected

Example for the popup container:

-    <div className="file-search-popup-overlay" onClick={onClose}>
-      <div className="file-search-popup" onClick={e => e.stopPropagation()}>
+    <div className="file-search-popup-overlay" onClick={onClose} role="presentation">
+      <div className="file-search-popup" onClick={e => e.stopPropagation()} role="dialog" aria-modal="true" aria-label="Search files">

📝 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

	return (
	<div className="file-search-popup-overlay" onClick={onClose}>
	<div className="file-search-popup" onClick={e => e.stopPropagation()}>
	<div className="file-search-header">
	<input ref={inputRef} type="text" className="file-search-input" placeholder="Search files..."
	value={searchQuery} onChange={e => setSearchQuery(e.target.value)} onKeyDown={handleKeyDown} />
	</div>
	<div className="file-search-list" ref={listRef}>
	{isLoading && <div className="file-search-loading">Searching...</div>}
	{!isLoading && filteredItems.length === 0 && (
	<div className="file-search-empty">{searchQuery.length >= 2 ? 'No files found' : 'Type to search files'}</div>
	)}
	{!isLoading && searchQuery.length < 2 && recentFiles.length > 0 && (
	<div className="file-search-section-title">Recent Files</div>
	)}
	{filteredItems.map((item, index) => (
	<FileSearchItem key={item.path} item={item} isSelected={index === selectedIndex}
	onClick={() => { onSelectFile(item); setSearchQuery(''); onClose(); }}
	onMouseEnter={() => setSelectedIndex(index)} />
	))}
	</div>
	</div>
	</div>
	);
	return (
	<div className="file-search-popup-overlay" onClick={onClose} role="presentation">
	<div className="file-search-popup" onClick={e => e.stopPropagation()} role="dialog" aria-modal="true" aria-label="Search files">
	<div className="file-search-header">
	<input ref={inputRef} type="text" className="file-search-input" placeholder="Search files..."
	value={searchQuery} onChange={e => setSearchQuery(e.target.value)} onKeyDown={handleKeyDown} />
	</div>
	<div className="file-search-list" ref={listRef}>
	{isLoading && <div className="file-search-loading">Searching...</div>}
	{!isLoading && filteredItems.length === 0 && (
	<div className="file-search-empty">{searchQuery.length >= 2 ? 'No files found' : 'Type to search files'}</div>
	)}
	{!isLoading && searchQuery.length < 2 && recentFiles.length > 0 && (
	<div className="file-search-section-title">Recent Files</div>
	)}
	{filteredItems.map((item, index) => (
	<FileSearchItem key={item.path} item={item} isSelected={index === selectedIndex}
	onClick={() => { onSelectFile(item); setSearchQuery(''); onClose(); }}
	onMouseEnter={() => setSelectedIndex(index)} />
	))}
	</div>
	</div>
	</div>
	);

🤖 Prompt for AI Agents

In mpp-vscode/webview/src/components/FileSearchPopup.tsx around lines 130 to
153, the popup is missing ARIA markup which makes it inaccessible; update the
outer popup container to include role="dialog" and aria-modal="true", add
aria-label (or visible label) to the input (e.g. aria-label="Search files"),
mark the list container with role="listbox" and an appropriate aria-label, and
ensure each FileSearchItem is rendered with role="option" and
aria-selected={isSelected} (and an id so you can use aria-activedescendant on
the listbox if you want keyboard focus tracking); keep click/keyboard behavior
the same but wire the selectedIndex boolean to the aria-selected attribute so
screen readers know the current selection.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Missing onAction prop on placeholder renderers.

The mermaid, devin, and plan cases use CodeBlockRenderer but don't pass the onAction callback. This means actions like "Insert" or "Apply" won't work for these block types.

     case 'mermaid':
     case 'mmd':
       // TODO: Implement MermaidRenderer
       return (
         <CodeBlockRenderer
           code={block.text}
           language="mermaid"
           isComplete={isComplete}
+          onAction={onAction}
         />
       );

     case 'devin':
       // TODO: Implement DevInRenderer
       return (
         <CodeBlockRenderer
           code={block.text}
           language="devin"
           isComplete={isComplete}
+          onAction={onAction}
         />
       );

     case 'plan':
       // TODO: Implement PlanRenderer
       return (
         <CodeBlockRenderer
           code={block.text}
           language="plan"
           isComplete={isComplete}
+          onAction={onAction}
         />
       );

🤖 Prompt for AI Agents

In mpp-vscode/webview/src/components/sketch/SketchRenderer.tsx around lines 108
to 137, the mermaid, devin, and plan cases render CodeBlockRenderer without the
onAction prop so user actions (Insert/Apply) won't be handled; update each of
those returns to pass the same onAction handler used elsewhere in this component
(e.g., onAction={onAction} or the correct handler name in scope), keeping
existing props (code, language, isComplete) intact so the CodeBlockRenderer
receives onAction for interactive behavior.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Same hardcoded color issue in error items.

Line 141 also uses a hardcoded rgba() value. Apply the same fix as suggested for task-complete items.

 .timeline-item.error {
   ...
-  background-color: rgba(244, 67, 54, 0.1);
+  background-color: color-mix(in srgb, var(--vscode-testing-iconFailed, #f44336) 10%, transparent);
   border: 1px solid var(--vscode-testing-iconFailed, #f44336);
   ...
 }

🤖 Prompt for AI Agents

In mpp-vscode/webview/src/components/Timeline.css around lines 134 to 144, the
.timeline-item.error rule uses a hardcoded background-color rgba(244, 67, 54,
0.1); — replace that hardcoded rgba with the theme variable approach used for
task-complete items: derive the error color from --vscode-testing-iconFailed
(with a fallback like #f44336) and apply a 10% alpha (for example by introducing
a small helper css var for the RGB value or using color-mix with the var),
leaving the border as-is so the color source is consistent with the rest of the
UI.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

TaskInfo status is missing 'PLANNING' value.

The TypeScript TaskInfo.status type excludes 'PLANNING' which exists in the Kotlin TaskStatus enum (in RendererModels.kt). This mismatch could cause runtime issues when the backend sends a task with PLANNING status.

 export interface TaskInfo {
   taskName: string;
-  status: 'WORKING' | 'COMPLETED' | 'CANCELLED' | 'BLOCKED';
+  status: 'PLANNING' | 'WORKING' | 'COMPLETED' | 'CANCELLED' | 'BLOCKED';
   summary: string;
   timestamp?: number;
 }

📝 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

	export interface TaskInfo {
	taskName: string;
	status: 'WORKING' | 'COMPLETED' | 'CANCELLED' | 'BLOCKED';
	summary: string;
	timestamp?: number;
	}
	export interface TaskInfo {
	taskName: string;
	status: 'PLANNING' | 'WORKING' | 'COMPLETED' | 'CANCELLED' | 'BLOCKED';
	summary: string;
	timestamp?: number;
	}

🤖 Prompt for AI Agents

In mpp-vscode/webview/src/types/timeline.ts around lines 34 to 39, the
TaskInfo.status union type omits the 'PLANNING' value present in the backend
Kotlin TaskStatus enum; update the status type to include 'PLANNING' so it
becomes 'WORKING' | 'COMPLETED' | 'CANCELLED' | 'BLOCKED' | 'PLANNING' to match
the backend and avoid runtime mismatches.