sbroenne
diff --git a/‎.github/copilot-instructions.md‎
Lines changed: 36 additions & 16 deletions b/‎.github/copilot-instructions.md‎
Lines changed: 36 additions & 16 deletions
diff --git a/‎.github/instructions/critical-rules.instructions.md‎
Lines changed: 58 additions & 0 deletions b/‎.github/instructions/critical-rules.instructions.md‎
Lines changed: 58 additions & 0 deletions
diff --git a/‎.github/instructions/excel-com-interop.instructions.md‎
Lines changed: 74 additions & 0 deletions b/‎.github/instructions/excel-com-interop.instructions.md‎
Lines changed: 74 additions & 0 deletions
diff --git a/‎Directory.Packages.props‎
Lines changed: 2 additions & 2 deletions b/‎Directory.Packages.props‎
Lines changed: 2 additions & 2 deletions
@@ -38,34 +38,52 @@
 
 ### Test Commands
 ```bash
-# Fast feedback (excludes VBA)
+# ⚠️ CRITICAL: Integration tests take 45+ MINUTES for full suite
+# ALWAYS use surgical testing - test only what you changed!
+
+# Fast feedback (excludes VBA) - Still takes 10-15 minutes
 dotnet test --filter "Category=Integration&RunType!=OnDemand&Feature!=VBA&Feature!=VBATrust"
 
+# Surgical testing - Feature-specific (2-5 minutes per feature)
+dotnet test --filter "Feature=PowerQuery&RunType!=OnDemand"
+dotnet test --filter "Feature=Ranges&RunType!=OnDemand"
+dotnet test --filter "Feature=PivotTables&RunType!=OnDemand"
+
 # Session/batch changes (MANDATORY)
 dotnet test --filter "RunType=OnDemand"
 ```
 
 ### Code Patterns
 ```csharp
-// Core: Always use batch parameter
-public async Task<OperationResult> MethodAsync(IExcelBatch batch, string arg1)
+// Core: NEVER wrap batch.Execute() in try-catch that returns error result
+// Let exceptions propagate naturally - batch.Execute() handles them via TaskCompletionSource
+public DataType Method(IExcelBatch batch, string arg1)
 {
-    return await batch.Execute((ctx, ct) => {
-        // Use ctx.Book for workbook access
-        return ValueTask.FromResult(new OperationResult { Success = true });
+    return batch.Execute((ctx, ct) => {
+        dynamic? item = null;
+        try {
+            // Operation code here
+            item = ctx.Book.SomeObject;
+            // For CRUD: return void (throws on error)
+            // For queries: return actual data
+            return someData;
+        }
+        finally {
+            // ✅ ONLY finally blocks for COM cleanup
+            ComUtilities.Release(ref item!);
+        }
+        // ❌ NO catch blocks that return error results
     });
 }
 
+
 // CLI: Wrap Core calls
 public int Method(string[] args)
 {
     try {
-        var task = Task.Run(async () => {
-            await using var batch = await ExcelSession.BeginBatchAsync(filePath);
-            return await _coreCommands.MethodAsync(batch, arg1);
-        });
-        var result = task.GetAwaiter().GetResult();
-        return result.Success ? 0 : 1;
+        using var batch = ExcelSession.BeginBatch(filePath);
+        _coreCommands.Method(batch, arg1);
+        return 0;
     } catch (Exception ex) {
         AnsiConsole.MarkupLine($"[red]Error:[/] {ex.Message.EscapeMarkup()}");
         return 1;
@@ -74,11 +92,11 @@ public int Method(string[] args)
 
 // Tests: Use batch API
 [Fact]
-public async Task TestMethod()
+public void TestMethod()
 {
-    await using var batch = await ExcelSession.BeginBatchAsync(_testFile);
-    var result = await _commands.MethodAsync(batch, args);
-    Assert.True(result.Success, $"Failed: {result.ErrorMessage}");
+    using var batch = ExcelSession.BeginBatch(_testFile);
+    var result = _commands.Method(batch, args);
+    Assert.NotNull(result); // Or other appropriate assertion
 }
 ```
 
@@ -106,6 +124,8 @@ public async Task TestMethod()
 
 **PR Review:** Check automated comments immediately (Copilot, GitHub Security). Fix before human review.
 
+**Surgical Testing:** Integration tests take 45+ minutes. ALWAYS test only the feature you changed using `--filter "Feature=<name>"`.
+
 ---
 
 ## 📚 How Path-Specific Instructions Work
 
@@ -55,6 +55,7 @@ applyTo: "**"
 **When Writing Code:**
 | Rule | Action | Why Critical |
 |------|--------|--------------|
+| 22. COM cleanup | ALWAYS use try-finally, NEVER swallow exceptions | Prevents leaks and silent failures |
 | 7. COM API | Use Excel COM first, validate docs | Prevents wrong dependencies |
 | 9. GitHub search | Search OTHER repos for VBA/COM examples FIRST | Learn from working code |
 | 2. NotImplementedException | Never use, full implementation only | No placeholders allowed |
@@ -663,5 +664,62 @@ mcp_github_github_pull_request_read(method="get_review_comments", owner="sbroenn
 
 **Example:** PR #139 had 17 automated review comments - all fixed in one commit before human review.
 
+---
+
+## Rule 22: COM Cleanup Must Use Finally Blocks (CRITICAL)
+
+**ALWAYS use try-finally for COM object cleanup. NEVER swallow exceptions with empty catch blocks.**
+
+```csharp
+// ❌ WRONG: Swallows exception, sets fallback value
+try
+{
+    dynamic pivotLayout = chart.PivotLayout;
+    dynamic pivotTable = pivotLayout.PivotTable;
+    name = pivotTable.Name?.ToString() ?? string.Empty;
+    ComUtilities.Release(ref pivotTable!);  // Won't execute if exception occurs!
+    ComUtilities.Release(ref pivotLayout!);
+}
+catch
+{
+    name = "(unknown)";  // Swallows exception!
+}
+
+// ✅ CORRECT: Finally ensures cleanup, exceptions propagate
+dynamic? pivotLayout = null;
+dynamic? pivotTable = null;
+try
+{
+    pivotLayout = chart.PivotLayout;
+    pivotTable = pivotLayout.PivotTable;
+    name = pivotTable.Name?.ToString() ?? string.Empty;
+}
+finally
+{
+    if (pivotTable != null) ComUtilities.Release(ref pivotTable!);
+    if (pivotLayout != null) ComUtilities.Release(ref pivotLayout!);
+}
+// Exception propagates naturally, COM objects always released
+```
+
+**Why Critical:**
+- Finally blocks execute **regardless** of exceptions
+- COM objects leak if Release() not reached before exception
+- Swallowing exceptions hides real problems
+- Empty catch blocks are code smell - remove them
+- Let exceptions propagate naturally to batch.Execute()
+
+**Pattern Requirements:**
+1. Declare COM objects as `dynamic?` nullable before try block
+2. Initialize to `null`
+3. Acquire COM objects in try block
+4. Release in finally block with null checks
+5. **NO catch blocks** unless specific exception handling needed
+6. **NEVER** catch just to set fallback values like "(unknown)"
+
+**See Also:**
+- Rule 1b: Exception propagation pattern
+- excel-com-interop.instructions.md for complete patterns
+
 ---
 ---
@@ -158,6 +158,80 @@ Overall Quit Timeout: 30 seconds (outer)
 - **6-attempt retry**: Handles transient COM busy states within the 30s window
 - **10s thread join**: Quick verification that cleanup finished (not a primary timeout mechanism)
 
+## COM Object Cleanup Pattern (CRITICAL)
+
+**ALWAYS use try-finally for COM object cleanup. NEVER use catch blocks to swallow exceptions.**
+
+### ❌ WRONG Patterns
+
+```csharp
+// WRONG #1: COM cleanup in try block (won't execute if exception occurs)
+try
+{
+    dynamic pivotLayout = chart.PivotLayout;
+    dynamic pivotTable = pivotLayout.PivotTable;
+    name = pivotTable.Name?.ToString() ?? string.Empty;
+    ComUtilities.Release(ref pivotTable!);  // ❌ Won't execute if exception above!
+    ComUtilities.Release(ref pivotLayout!);
+}
+catch
+{
+    name = "(unknown)";  // ❌ Swallows exception, causes COM leak
+}
+
+// WRONG #2: Empty catch block (swallows exceptions silently)
+try
+{
+    dynamic item = GetItem();
+    // ... operations ...
+    ComUtilities.Release(ref item!);
+}
+catch
+{
+    // ❌ Empty catch - swallows exception, no cleanup
+}
+```
+
+### ✅ CORRECT Pattern
+
+```csharp
+// CORRECT: Finally block ensures cleanup regardless of exceptions
+dynamic? pivotLayout = null;
+dynamic? pivotTable = null;
+try
+{
+    pivotLayout = chart.PivotLayout;
+    pivotTable = pivotLayout.PivotTable;
+    name = pivotTable.Name?.ToString() ?? string.Empty;
+}
+finally
+{
+    // ✅ ALWAYS executes - exception or no exception
+    if (pivotTable != null) ComUtilities.Release(ref pivotTable!);
+    if (pivotLayout != null) ComUtilities.Release(ref pivotLayout!);
+}
+// ✅ Exception propagates naturally to batch.Execute()
+```
+
+**Pattern Requirements:**
+1. **Declare COM objects as `dynamic?` nullable** before try block
+2. **Initialize to `null`**
+3. **Acquire COM objects in try block**
+4. **Release in finally block** with null checks
+5. **NO catch blocks** unless specific exception handling required
+6. **NEVER catch to set fallback values** - let exceptions propagate
+
+**Why This Matters:**
+- Finally blocks execute **regardless** of exceptions (try succeeds or fails)
+- COM objects leak if Release() not reached before exception
+- Swallowing exceptions with catch blocks hides real problems from batch.Execute()
+- Empty catch blocks are code smell - remove them entirely
+- Let exceptions propagate naturally to batch.Execute() for proper error handling
+
+**See Also:**
+- CRITICAL-RULES.md Rule 22 for complete requirements
+- CRITICAL-RULES.md Rule 1b for exception propagation pattern
+
 ## Critical COM Issues
 
 ### 1. Excel Collections Are 1-Based
 
@@ -30,9 +30,9 @@
     <PackageVersion Include="xunit" Version="2.9.3" />
     <PackageVersion Include="xunit.runner.visualstudio" Version="3.1.5" />
     <PackageVersion Include="coverlet.collector" Version="6.1.0" />
-    <PackageVersion Include="MartinCostello.Logging.XUnit" Version="0.5.1" />
+    <PackageVersion Include="MartinCostello.Logging.XUnit" Version="0.7.0" />
     <!-- Code Analysis and Security -->
     <PackageVersion Include="Microsoft.CodeAnalysis.Analyzers" Version="3.3.5" />
     <PackageVersion Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="10.0.100" />
   </ItemGroup>
-</Project>
+</Project>