fix: prevent orphaned Power Query connections during worksheet loading

[sbroenne]

Summary

Fixes #298

Power Query load-to-worksheet operations were creating orphaned connections with generic names like Connection, Connection1, etc. These connections accumulate over time and cannot be easily managed.

Root Cause

When loading Power Query results to a worksheet, the code was using ListObjects.Add() with a connection string, which caused Excel to create a new connection with a generic name instead of reusing/creating a properly named connection.

Solution

Changed the implementation to:

1. Use Connections.Add2() to create properly named connections (Query - {queryName}) BEFORE calling ListObjects.Add()
2. Pass the connection OBJECT to ListObjects.Add() instead of a connection string
3. For LoadToBoth mode, create TWO properly named connections:
   • Worksheet: Query - {queryName}
   • Data Model: Query - {queryName} (Data Model)
4. Updated pattern matching in Delete/Unload/List/GetLoadConfig to recognize the new naming pattern

Files Changed

Core Implementation:

• PowerQueryCommands.LoadTo.cs - Use Add2 pattern for worksheet loading
• PowerQueryCommands.Create.cs - Updated LoadToBoth case to use dual connections
• PowerQueryCommands.Lifecycle.cs - Updated pattern matching for new naming convention
• PowerQueryHelpers.cs - Enhanced orphan detection logic

Tests:

• PowerQueryCommandsTests.WorksheetCleanup.cs (NEW) - 14 tests for clean slate verification
• PowerQueryComApiBehaviorTests.cs - Added Scenario 17 proving the Add2 approach

Test Coverage (74+ tests)

Test Suite	Tests	Status
PowerQuery Worksheet Cleanup	14	✅ All passing
PowerQuery Lifecycle Cleanup	9	✅ All passing
PowerQuery Core	26	✅ All passing
Connection	25	✅ All passing

Scenarios Tested

• Create with all load modes (ConnectionOnly, LoadToTable, LoadToDataModel, LoadToBoth)
• Delete cleanup verification
• LoadTo on existing queries
• Mode transitions (ConnectionOnly → LoadToBoth)
• Refresh/Update operations maintain proper naming
• Unload then reload cycles
• Dual connection verification for LoadToBoth

Backwards Compatibility

✅ Fully backwards compatible - no breaking changes to API

Related Fix

This also enables users to delete existing orphaned connections via excel_connection delete action. The orphan detection logic now correctly identifies:

• Generic-named connections (Connection, Connection1, etc.) - always orphaned
• Query - {name} connections without corresponding queries - orphaned

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

The best fix for this issue is to replace Path.Combine with Path.Join on line 216. This makes path joining safer by ensuring none of the earlier arguments are dropped if any later ones are absolute paths (which could accidentally happen in future edits). Functionally, Path.Join does not behave differently from Path.Combine for all relative segments, so the change preserves behavior. The only code to change is the one line constructing the sourceFile path on line 216. This file is using Path without an explicit import, so we need to ensure a using directive for System.IO is available, but it’s likely already provided in the unseen snippet; if not, add it at the top.

---

The best way to fix this problem is to replace the call to Path.Combine with Path.Join. Path.Join always joins the arguments with path separators, and does not treat absolute paths specially, avoiding the risk present in Path.Combine. Specifically, change line 224 in tests/ExcelMcp.Core.Tests/Integration/Commands/Connection/ConnectionCommandsTests.Delete.cs from:

var testFile = Path.Combine(_fixture.TempDir, $"OrphanedPQ_{Guid.NewGuid():N}.xlsx");

to

var testFile = Path.Join(_fixture.TempDir, $"OrphanedPQ_{Guid.NewGuid():N}.xlsx");

No new imports or helper definitions are necessary, since the surrounding code already uses the Path class.

The problem can be fixed by replacing the use of Path.Combine with Path.Join for the flagged path-building calls. Path.Join combines path segments without dropping earlier segments if a later segment turns out to be absolute—making its semantics safer for this scenario. In the code snippet shown, update all lines using Path.Combine relevant to this code region (lines 216, 224, 251, and 259). No additional method changes or variable definitions are needed; ensure that System.IO.Path is imported (which is implicitly available unless excluded), as both methods come from the same namespace.

To fix the problem, replace calls to Path.Combine that join potentially non-absolute directory paths with filenames with corresponding calls to Path.Join. In C# (since .NET Core 2.1), Path.Join is available and does not drop earlier segments if later arguments are absolute, ensuring path joining behaves predictably. Specifically, in tests/ExcelMcp.Core.Tests/Integration/Commands/Connection/ConnectionCommandsTests.Delete.cs, replace the call on line 259:

var testFile = Path.Combine(_fixture.TempDir, $"OrphanedPQ2_{Guid.NewGuid():N}.xlsx");

with

var testFile = Path.Join(_fixture.TempDir, $"OrphanedPQ2_{Guid.NewGuid():N}.xlsx");

No additional imports are needed if System.IO.Path is accessible (as it is in C# test files), so only the minimal change needs to be made to the line in question.

To fix the issue, replace the call to Path.Combine(AppContext.BaseDirectory, "TestData", "MSXI Baseline.xlsx") with Path.Join(AppContext.BaseDirectory, "TestData", "MSXI Baseline.xlsx"), which is guaranteed to preserve all path segments regardless of input. This change should be made on line 286 of tests/ExcelMcp.Core.Tests/Integration/Commands/Connection/ConnectionCommandsTests.Delete.cs. You should ensure that System.IO.Path is available (no new using directives needed, as it's in the global namespace and already referenced by usage of Path.Combine). No other functional change is necessary.

---

To address this issue, replace the use of Path.Combine on line 364 with Path.Join. Path.Join is designed for straightforward path composition and will simply concatenate the path segments using the correct directory separator, without any "drop" behavior for absolute paths. Given the arguments are all well-controlled and relative (except for the first), this change is a drop-in replacement and does not affect existing logic.

Needed:

• Ensure using System.IO; is present if Path.Join is not directly accessible, but in this context, it can be assumed or added if missing.
• On line 364, replace Path.Combine(AppContext.BaseDirectory, "TestData", "MSXI Baseline.xlsx") with Path.Join(AppContext.BaseDirectory, "TestData", "MSXI Baseline.xlsx").

No other code or method changes are required.

---

The fix is to replace Path.Combine calls in the highlighted regions with Path.Join. Path.Join safely concatenates all arguments using the directory separator, without dropping earlier segments if a later one is absolute. This change should be applied to all instances flagged by CodeQL, specifically the ones shown on lines 372 and similarly on line 364 (which are structurally identical and should both be updated for consistency). No changes to functionality are expected, and there is no need for new imports since System.IO.Path is already in use in the file. Only the relevant Path.Combine calls need to be changed; all else remains the same.

The best way to fix this problem is to narrow the catch clause from Exception to only those exception types expected from COM interop operations, most notably COMException. This prevents suppressing unrelated errors but still handles anticipated failures from the Excel API interface. Specifically, the change should be applied to the catch (Exception ex) block on line 1666 by replacing it with catch (COMException ex). This will require ensuring the file includes a using System.Runtime.InteropServices; directive, but as we see, line 8 already includes this import.

No other edits outside the shown code are needed, and no additional method definitions or variable changes are required.

---

To address the generic catch clause, replace all broad catch (Exception ex) and catch (Exception ex2) statements with specific catch blocks for exceptions that are likely to be thrown by COM interop and Excel COM calls, such as COMException, and possibly ArgumentException or others if warranted. In addition, add a fallback catch block for unexpected exception types which logs the full diagnostics. Edit lines 1666–1669 and 1685–1688 in the file, replacing them with more specific catches. You should add using System.Runtime.InteropServices; as needed (this is already present), and ensure all appropriate exceptions are handled explicitly.

---

To fix the problem, replace the overly broad catch (Exception ex) block with specific catch clauses for likely/expected exception types (COMException, etc.), and where truly unexpected exceptions occur, either let them bubble up or log and rethrow. In this diagnostic test context, the most expected failure is a COMException, so we should catch that explicitly and handle/log accordingly. If other exception types are expected (e.g., InvalidOperationException), those can also be caught and logged. For everything else, either don't catch, or optionally catch, log, and rethrow after logging. The necessary change is within the catch block from lines 1726-1729 of PowerQueryComApiBehaviorTests.cs.

No new methods or field definitions are required, but you must ensure that System.Runtime.InteropServices.COMException is accessible (it is, via the existing using System.Runtime.InteropServices; line).

---