fix: address PR #40 review comments

- Add error handling for plan markdown parsing in JewelRenderer
- Restore OutputDisplay in IdeaTerminalRenderer (was commented out)
- Fix root-level files edge case in IdeaFileChangeItem
- Cancel planStateObserverJob in IdeaAgentViewModel.dispose()
- Add dispose() method to ChatViewProvider for cleanup
- Add type guard validation for PlanData in App.tsx
- Replace println with Logger in IdeaFileChangeSummary
- Simplify StateFlow update in PlanStateService (use direct reassignment)

Author: phodal

mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/plan/PlanStateService.kt

@@ -94,31 +94,31 @@ class PlanStateService {
         val plan = _currentPlan.value ?: return
         val task = plan.getTask(taskId) ?: return
         task.updateStatus(status)
-        // Trigger StateFlow update by creating a new plan instance with updated timestamp
-        _currentPlan.value = plan.copy(updatedAt = kotlinx.datetime.Clock.System.now().toEpochMilliseconds())
+        // Trigger StateFlow update by reassigning the plan object
+        _currentPlan.value = plan
         notifyTaskUpdated(task)
     }
-    
+
     /**
      * Complete a step within a task.
      */
     fun completeStep(taskId: String, stepId: String) {
         val plan = _currentPlan.value ?: return
         plan.completeStep(taskId, stepId)
-        // Trigger StateFlow update by creating a new plan instance with updated timestamp
-        _currentPlan.value = plan.copy(updatedAt = kotlinx.datetime.Clock.System.now().toEpochMilliseconds())
+        // Trigger StateFlow update by reassigning the plan object
+        _currentPlan.value = plan
         notifyStepCompleted(taskId, stepId)
     }
-    
+
     /**
      * Update a step's status.
      */
     fun updateStepStatus(taskId: String, stepId: String, status: TaskStatus) {
         val plan = _currentPlan.value ?: return
         val task = plan.getTask(taskId) ?: return
         task.updateStepStatus(stepId, status)
-        // Trigger StateFlow update by creating a new plan instance with updated timestamp
-        _currentPlan.value = plan.copy(updatedAt = kotlinx.datetime.Clock.System.now().toEpochMilliseconds())
+        // Trigger StateFlow update by reassigning the plan object
+        _currentPlan.value = plan
         notifyTaskUpdated(task)
     }
 

mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt

@@ -323,9 +323,14 @@ class JewelRenderer : BaseRenderer() {
         when (action) {
             "CREATE", "UPDATE" -> {
                 if (planMarkdown.isNotBlank()) {
-                    val plan = MarkdownPlanParser.parseToPlan(planMarkdown)
-                    jewelRendererLogger.info("Parsed plan: ${plan.tasks.size} tasks")
-                    _currentPlan.value = plan
+                    try {
+                        val plan = MarkdownPlanParser.parseToPlan(planMarkdown)
+                        jewelRendererLogger.info("Parsed plan: ${plan.tasks.size} tasks")
+                        _currentPlan.value = plan
+                    } catch (e: Exception) {
+                        jewelRendererLogger.warn("Failed to parse plan markdown", e)
+                        // Keep previous valid plan on parse failure
+                    }
                 }
             }
             "COMPLETE_STEP" -> {

mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/sketch/IdeaTerminalRenderer.kt

@@ -77,9 +77,9 @@ fun IdeaTerminalRenderer(
         // Command display
         CommandDisplay(command = command, isDangerous = isDangerous, dangerReason = dangerReason)
         // Output (if available)
-//        if (showOutput && executionResult != null) {
-//            OutputDisplay(result = executionResult!!)
-//        }
+        if (showOutput && executionResult != null) {
+            OutputDisplay(result = executionResult!!)
+        }
     }
 }
 

mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.kt

@@ -576,6 +576,7 @@ class IdeaAgentViewModel(
 
     override fun dispose() {
         currentJob?.cancel()
+        planStateObserverJob?.cancel()
         coroutineScope.cancel()
     }
 }

mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeItem.kt

@@ -89,8 +89,8 @@ fun IdeaFileChangeItem(
             )
 
             // Parent path
-            val parentPath = change.filePath.substringBeforeLast('/')
-            if (parentPath.isNotEmpty()) {
+            val parentPath = change.filePath.substringBeforeLast('/', "")
+            if (parentPath.isNotEmpty() && parentPath != change.filePath) {
                 Text(
                     text = parentPath.substringAfterLast('/'),
                     style = JewelTheme.defaultTextStyle.copy(

mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeSummary.kt

@@ -23,6 +23,7 @@ import cc.unitmesh.agent.diff.FileChangeTracker
 import cc.unitmesh.devins.ui.compose.theme.AutoDevColors
 import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.application.runWriteAction
+import com.intellij.openapi.diagnostic.Logger
 import com.intellij.openapi.fileEditor.FileDocumentManager
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.vfs.LocalFileSystem
@@ -32,6 +33,8 @@ import org.jetbrains.jewel.ui.component.IconButton
 import org.jetbrains.jewel.ui.component.Text
 import org.jetbrains.jewel.ui.icons.AllIconsKeys
 
+private val logger = Logger.getInstance("IdeaFileChangeSummary")
+
 /**
  * File Change Summary Component for IntelliJ IDEA using Jewel components.
  *
@@ -254,7 +257,7 @@ private fun undoChange(project: Project, change: FileChange) {
                     }
                 }
             } catch (e: Exception) {
-                println("Failed to undo change for ${change.filePath}: ${e.message}")
+                logger.error("Failed to undo change for ${change.filePath}", e)
             }
         }
     }

mpp-vscode/src/providers/chat-view.ts

@@ -21,7 +21,7 @@ const KotlinCC = MppCore.cc.unitmesh;
 /**
  * Chat View Provider for the sidebar webview
  */
-export class ChatViewProvider implements vscode.WebviewViewProvider {
+export class ChatViewProvider implements vscode.WebviewViewProvider, vscode.Disposable {
   private webviewView: vscode.WebviewView | undefined;
   private codingAgent: any = null;
   private llmService: any = null;
@@ -32,6 +32,17 @@ export class ChatViewProvider implements vscode.WebviewViewProvider {
   private editorChangeDisposable: vscode.Disposable | undefined;
   private planStateUnsubscribe: (() => void) | null = null;
 
+  /**
+   * Dispose of resources when the provider is no longer needed.
+   */
+  dispose(): void {
+    if (this.planStateUnsubscribe) {
+      this.planStateUnsubscribe();
+      this.planStateUnsubscribe = null;
+    }
+    this.editorChangeDisposable?.dispose();
+  }
+
   constructor(
     private readonly context: vscode.ExtensionContext,
     private readonly log: (message: string) => void

mpp-vscode/webview/src/App.tsx

@@ -16,6 +16,22 @@ import { useVSCode, ExtensionMessage } from './hooks/useVSCode';
 import type { AgentState, ToolCallInfo, TerminalOutput, ToolCallTimelineItem } from './types/timeline';
 import './App.css';
 
+/**
+ * Type guard to validate PlanData structure from extension messages.
+ */
+function isPlanData(data: unknown): data is PlanData {
+  if (!data || typeof data !== 'object') return false;
+  const plan = data as Record<string, unknown>;
+  return (
+    typeof plan.planId === 'string' &&
+    typeof plan.title === 'string' &&
+    typeof plan.totalSteps === 'number' &&
+    typeof plan.completedSteps === 'number' &&
+    typeof plan.progressPercent === 'number' &&
+    Array.isArray(plan.tasks)
+  );
+}
+
 interface ConfigState {
   availableConfigs: ModelConfig[];
   currentConfigName: string | null;
@@ -260,8 +276,8 @@ const App: React.FC = () => {
 
       // Plan update from mpp-core PlanStateService
       case 'planUpdate':
-        if (msg.data) {
-          setCurrentPlan(msg.data as unknown as PlanData);
+        if (msg.data && isPlanData(msg.data)) {
+          setCurrentPlan(msg.data);
         } else {
           setCurrentPlan(null);
         }