Add robust error handling and execution order validation

Co-authored-by: DonJayamanne <[email protected]>

Author: Copilot

src/standalone/chat/installPackageTool.node.ts

@@ -34,14 +34,20 @@ export class InstallPackagesTool extends BaseTool<IInstallPackageParams> impleme
         // Track cell execution for all notebooks
         this.disposables.push(
             vscode.workspace.onDidChangeNotebookDocument((e) => {
-                for (const change of e.cellChanges) {
-                    const cell = change.cell;
-                    if (
-                        cell.kind === vscode.NotebookCellKind.Code &&
-                        typeof cell.executionSummary?.executionOrder === 'number'
-                    ) {
-                        this.executedNotebooks.add(e.notebook);
+                try {
+                    for (const change of e.cellChanges) {
+                        const cell = change.cell;
+                        if (
+                            cell.kind === vscode.NotebookCellKind.Code &&
+                            typeof cell.executionSummary?.executionOrder === 'number' &&
+                            cell.executionSummary.executionOrder > 0
+                        ) {
+                            this.executedNotebooks.add(e.notebook);
+                            break; // Once we find one executed cell, no need to check others
+                        }
                     }
+                } catch (error) {
+                    console.warn('Error processing notebook document change:', error);
                 }
             })
         );
@@ -53,19 +59,31 @@ export class InstallPackagesTool extends BaseTool<IInstallPackageParams> impleme
     }
 
     private initializeExecutedNotebooks(): void {
-        // Check all currently open notebooks for executed cells
-        for (const notebook of vscode.workspace.notebookDocuments) {
-            if (this.hasAnyExecutedCells(notebook)) {
-                this.executedNotebooks.add(notebook);
+        try {
+            // Check all currently open notebooks for executed cells
+            for (const notebook of vscode.workspace.notebookDocuments) {
+                if (this.hasAnyExecutedCells(notebook)) {
+                    this.executedNotebooks.add(notebook);
+                }
             }
+        } catch (error) {
+            // If there's an error during initialization, log it but don't fail
+            console.warn('Error initializing executed notebooks:', error);
         }
     }
 
     private hasAnyExecutedCells(notebook: vscode.NotebookDocument): boolean {
-        return notebook.getCells().some(cell => 
-            cell.kind === vscode.NotebookCellKind.Code &&
-            typeof cell.executionSummary?.executionOrder === 'number'
-        );
+        try {
+            return notebook.getCells().some(cell => 
+                cell.kind === vscode.NotebookCellKind.Code &&
+                typeof cell.executionSummary?.executionOrder === 'number' &&
+                cell.executionSummary.executionOrder > 0
+            );
+        } catch (error) {
+            // If there's an error accessing cells, assume no execution
+            console.warn('Error checking executed cells:', error);
+            return false;
+        }
     }
 
     private hasExecutedCells(notebook: vscode.NotebookDocument): boolean {

src/standalone/chat/installPackageTool.node.unit.test.ts

@@ -89,6 +89,26 @@ suite('InstallPackagesTool Unit Tests', () => {
         assert.isTrue(hasExecuted, 'Should return true for notebook with executed cells');
     });
 
+    test('Should not detect cells with execution order 0', () => {
+        // Setup notebook with cell that has execution order 0 (not executed)
+        const mockNonExecutedCell = {
+            kind: vscode.NotebookCellKind.Code,
+            executionSummary: { executionOrder: 0 }
+        } as vscode.NotebookCell;
+        
+        mockCells.push(mockNonExecutedCell);
+        
+        installPackagesTool = new InstallPackagesTool(
+            instance(kernelProvider),
+            instance(controllerRegistration),
+            instance(installationManager)
+        );
+        
+        // hasExecutedCells should return false for cell with execution order 0
+        const hasExecuted = (installPackagesTool as any).hasExecutedCells(mockNotebook);
+        assert.isFalse(hasExecuted, 'Should return false for cell with execution order 0');
+    });
+
     test('Should track cell execution via event listener', () => {
         let onDidChangeCallback: (e: vscode.NotebookDocumentChangeEvent) => void;
 
@@ -170,6 +190,29 @@ suite('InstallPackagesTool Unit Tests', () => {
         assert.isFalse((installPackagesTool as any).hasExecutedCells(mockNotebook));
     });
 
+    test('Should handle errors gracefully during initialization', () => {
+        // Setup notebook that throws error when getCells is called
+        const mockErrorNotebook = {
+            uri: vscode.Uri.file('/test/error-notebook.ipynb'),
+            getCells: sinon.stub().throws(new Error('Test error'))
+        } as any;
+        
+        mockWorkspace.notebookDocuments = [mockErrorNotebook];
+        
+        // Should not throw error during initialization
+        assert.doesNotThrow(() => {
+            installPackagesTool = new InstallPackagesTool(
+                instance(kernelProvider),
+                instance(controllerRegistration),
+                instance(installationManager)
+            );
+        }, 'Should handle initialization errors gracefully');
+        
+        // Should return false for error case
+        const hasExecuted = (installPackagesTool as any).hasExecutedCells(mockErrorNotebook);
+        assert.isFalse(hasExecuted, 'Should return false when error occurs');
+    });
+
     test('Should dispose of event listeners properly', () => {
         const disposeSpy = sinon.spy();
         (mockWorkspace.onDidChangeNotebookDocument as sinon.SinonStub).returns({ dispose: disposeSpy });