Fix Issue #170: LoadTo detects existing sheets and returns clear error (#171)

sbroenne · web-flow · commit 85ca40073ba0 · 2025-11-10T17:48:05.000Z
LoadTo silently failed when target worksheet existed - Excel's CreateQueryTableForQuery doesn't update query load configuration when sheet exists. Now detects conflicts and returns clear error requiring explicit sheet deletion before retry. Includes 6 regression tests, fixes CLI pack --no-build error, and corrects pre-commit script path warning.
diff --git a/scripts/check-mcp-core-implementations.ps1 b/scripts/check-mcp-core-implementations.ps1
@@ -32,7 +32,7 @@ $knownExceptions = @{
 
 # Define mappings: Enum -> Core Interface File
 $mappings = @{
-    "FileAction" = "src/ExcelMcp.Core/Commands/File/IFileCommands.cs"
+    "FileAction" = "src/ExcelMcp.Core/Commands/IFileCommands.cs"
     "PowerQueryAction" = "src/ExcelMcp.Core/Commands/PowerQuery/IPowerQueryCommands.cs"
     "WorksheetAction" = "src/ExcelMcp.Core/Commands/Sheet/ISheetCommands.cs"
     "TableAction" = "src/ExcelMcp.Core/Commands/Table/ITableCommands.cs"
diff --git a/src/ExcelMcp.CLI/ExcelMcp.CLI.csproj b/src/ExcelMcp.CLI/ExcelMcp.CLI.csproj
@@ -45,9 +45,6 @@
     <DebugType>none</DebugType>
     <DebugSymbols>false</DebugSymbols>
 
-    <!-- Generate XML docs for analyzers but exclude from output -->
-    <GenerateDocumentationFile>true</GenerateDocumentationFile>
-
     <!-- Additional optimization flags -->
     <Optimize>true</Optimize>
     <TieredCompilation>true</TieredCompilation>
@@ -65,6 +62,14 @@
     <Message Text="Removed XML documentation and PDB files from Release build" Importance="high" />
   </Target>
 
+  <!-- Prevent dotnet pack/publish from expecting XML docs after they're deleted -->
+  <Target Name="RemoveDocumentationFileFromPublish" BeforeTargets="ComputeFilesToPublish">
+    <ItemGroup>
+      <ResolvedFileToPublish Remove="@(ResolvedFileToPublish)" Condition="'%(Extension)' == '.xml'" />
+      <_IntermediateAssembly Remove="@(_IntermediateAssembly)" Condition="'%(Extension)' == '.xml'" />
+    </ItemGroup>
+  </Target>
+
   <ItemGroup>
     <PackageReference Include="Spectre.Console" />
     <PackageReference Include="Microsoft.CodeAnalysis.NetAnalyzers">
diff --git a/src/ExcelMcp.Core/Commands/PowerQuery/PowerQueryCommands.Lifecycle.cs b/src/ExcelMcp.Core/Commands/PowerQuery/PowerQueryCommands.Lifecycle.cs
@@ -1028,7 +1028,46 @@ public async Task<PowerQueryLoadResult> LoadToAsync(
                     switch (loadMode)
                     {
                         case PowerQueryLoadMode.LoadToTable:
-                            sheet = ctx.Book.Worksheets.Item(targetSheet!);
+                            // Check if sheet already exists - require explicit deletion by user
+                            // Bug fix for Issue #170: Prevent silent failures when sheet exists
+                            dynamic? worksheetsCheck = null;
+                            try
+                            {
+                                worksheetsCheck = ctx.Book.Worksheets;
+                                try
+                                {
+                                    dynamic? existingSheet = worksheetsCheck.Item(targetSheet!);
+                                    if (existingSheet != null)
+                                    {
+                                        ComUtilities.Release(ref existingSheet!);
+                                        result.Success = false;
+                                        result.ErrorMessage = $"Cannot load query to sheet '{targetSheet}': worksheet already exists.";
+                                        return result;
+                                    }
+                                }
+                                catch (COMException)
+                                {
+                                    // Sheet doesn't exist - this is expected, continue
+                                }
+                            }
+                            finally
+                            {
+                                ComUtilities.Release(ref worksheetsCheck!);
+                            }
+
+                            // Create new sheet for query data
+                            dynamic? worksheetsLoadToTable = null;
+                            try
+                            {
+                                worksheetsLoadToTable = ctx.Book.Worksheets;
+                                sheet = worksheetsLoadToTable.Add();
+                                sheet.Name = targetSheet;
+                            }
+                            finally
+                            {
+                                ComUtilities.Release(ref worksheetsLoadToTable!);
+                            }
+
                             queryTable = CreateQueryTableForQuery(sheet, query);
                             queryTable.Refresh(false);
                             result.ConfigurationApplied = true;
@@ -1072,7 +1111,46 @@ public async Task<PowerQueryLoadResult> LoadToAsync(
                             break;
 
                         case PowerQueryLoadMode.LoadToBoth:
-                            sheet = ctx.Book.Worksheets.Item(targetSheet!);
+                            // Check if sheet already exists - require explicit deletion by user
+                            // Bug fix for Issue #170: Prevent silent failures when sheet exists
+                            dynamic? worksheetsCheckBoth = null;
+                            try
+                            {
+                                worksheetsCheckBoth = ctx.Book.Worksheets;
+                                try
+                                {
+                                    dynamic? existingSheet = worksheetsCheckBoth.Item(targetSheet!);
+                                    if (existingSheet != null)
+                                    {
+                                        ComUtilities.Release(ref existingSheet!);
+                                        result.Success = false;
+                                        result.ErrorMessage = $"Cannot load query to sheet '{targetSheet}': worksheet already exists.";
+                                        return result;
+                                    }
+                                }
+                                catch (COMException)
+                                {
+                                    // Sheet doesn't exist - this is expected, continue
+                                }
+                            }
+                            finally
+                            {
+                                ComUtilities.Release(ref worksheetsCheckBoth!);
+                            }
+
+                            // Create new sheet for query data
+                            dynamic? worksheetsLoadToBoth = null;
+                            try
+                            {
+                                worksheetsLoadToBoth = ctx.Book.Worksheets;
+                                sheet = worksheetsLoadToBoth.Add();
+                                sheet.Name = targetSheet;
+                            }
+                            finally
+                            {
+                                ComUtilities.Release(ref worksheetsLoadToBoth!);
+                            }
+
                             queryTable = CreateQueryTableForQuery(sheet, query);
                             queryTable.Refresh(false);
 
diff --git a/src/ExcelMcp.McpServer/Prompts/Content/excel_powerquery.md b/src/ExcelMcp.McpServer/Prompts/Content/excel_powerquery.md
@@ -3,17 +3,24 @@
 **Action disambiguation**:
 - create: Import NEW query (FAILS if query already exists - use update instead)
 - update: Update EXISTING query M code + refresh data (use this if query exists)
-- load-to: Applies destination + refreshes (not just config change)
+- load-to: Applies destination + refreshes (not just config change) - CHECKS for sheet conflicts
 - unload: Removes data but keeps query definition (inverse of load-to)
 
 **When to use create vs update**:
 - Query doesn't exist? → Use create
 - Query already exists? → Use update (create will error "already exists")
 - Not sure? → Check with list action first, then use update if exists or create if new
 
+**LoadTo with existing sheets (Issue #170 fix)**:
+- If target worksheet already exists, LoadTo returns clear error
+- User must delete existing sheet first using excel_worksheet action='Delete'
+- Then retry LoadTo - ensures explicit user control over data deletion
+- Example workflow: LoadTo fails → Delete sheet manually → Retry LoadTo succeeds
+
 **Common mistakes**:
 - Using create on existing query → ERROR "Query 'X' already exists" (should use update)
 - Using update on new query → ERROR "Query 'X' not found" (should use create)
+- Calling LoadTo without checking if sheet exists (will error if sheet exists)
 
 **Server-specific quirks**:
 - Validation = execution: M code only validated when data loads/refreshes
diff --git a/src/ExcelMcp.McpServer/Tools/ExcelPowerQueryTool.cs b/src/ExcelMcp.McpServer/Tools/ExcelPowerQueryTool.cs
@@ -31,6 +31,11 @@ LOAD DESTINATIONS (loadDestination parameter):
 - 'both': Load to BOTH worksheet AND Data Model
 - 'connection-only': Don't load data (M code imported but not executed)
 
+⚠️ SHEET NAME CONFLICTS (LoadTo action):
+- If a worksheet with the target name already exists, LoadTo returns an error
+- User must delete the existing sheet first using excel_worksheet action='Delete'
+- Then retry LoadTo - this ensures explicit user control over data deletion
+
 WHEN TO USE CREATE vs UPDATE:
 - Create: For NEW queries only (FAILS with 'already exists' error if query exists)
 - Update: For EXISTING queries (updates M code + refreshes data)
@@ -445,6 +450,10 @@ private static async Task<string> LoadToPowerQueryAsync(
             save: true,
             async (batch) => await commands.LoadToAsync(batch, queryName, loadMode, targetSheet));
 
+        // Detect sheet conflict error and provide specific guidance
+        var isSheetConflict = !result.Success &&
+                             result.ErrorMessage?.Contains("worksheet already exists", StringComparison.OrdinalIgnoreCase) == true;
+
         // Add workflow hints
         var inBatch = !string.IsNullOrEmpty(batchId);
         var destinationName = loadMode switch
@@ -467,7 +476,9 @@ private static async Task<string> LoadToPowerQueryAsync(
             result.RowsLoaded,
             workflowHint = result.Success
                 ? $"Query '{queryName}' now loaded to {destinationName}. Data refreshed with {result.RowsLoaded} rows."
-                : $"Failed to load query '{queryName}': {result.ErrorMessage}",
+                : isSheetConflict
+                    ? $"Cannot load query '{queryName}': sheet '{targetSheet ?? queryName}' already exists. Delete it first, then retry LoadTo."
+                    : $"Failed to load query '{queryName}': {result.ErrorMessage}",
             suggestedNextActions = result.Success
                 ? (loadMode == PowerQueryLoadMode.LoadToDataModel || loadMode == PowerQueryLoadMode.LoadToBoth
                     ? new[]
@@ -484,12 +495,19 @@ private static async Task<string> LoadToPowerQueryAsync(
                         "Use excel_table 'create' to convert range to Excel Table for filtering/sorting",
                         inBatch ? "Load more queries in this batch" : "Loading multiple queries? Use excel_batch for efficiency"
                     ])
-                :
-                [
-                    "Check if query name is correct with excel_powerquery 'list'",
-                    "Verify query is connection-only with excel_powerquery 'get-load-config'",
-                    "Review error message for specific issue"
-                ]
+                : isSheetConflict
+                    ?
+                    [
+                        $"Use excel_worksheet action='Delete' sheetName='{targetSheet ?? queryName}' to delete the existing sheet",
+                        $"Then retry: excel_powerquery action='LoadTo' queryName='{queryName}' loadDestination='{loadDestination ?? "worksheet"}' targetSheet='{targetSheet ?? queryName}'",
+                        "Or rename the target sheet if you want to keep existing data"
+                    ]
+                    :
+                    [
+                        "Check if query name is correct with excel_powerquery 'list'",
+                        "Verify query is connection-only with excel_powerquery 'get-load-config'",
+                        "Review error message for specific issue"
+                    ]
         }, ExcelToolsBase.JsonOptions);
     }
 
diff --git a/tests/ExcelMcp.Core.Tests/Integration/Commands/PowerQuery/PowerQueryCommandsTests.LoadDestination.cs b/tests/ExcelMcp.Core.Tests/Integration/Commands/PowerQuery/PowerQueryCommandsTests.LoadDestination.cs
@@ -215,17 +215,14 @@ public async Task LoadTo_ConnectionOnlyToTable_LoadsDataSuccessfully()
         var mCodeFile = CreateUniqueTestQueryFile(nameof(LoadTo_ConnectionOnlyToTable_LoadsDataSuccessfully));
         var targetSheet = "LoadSheet";
 
+        // Act - Create connection-only query and LoadTo (all in one batch)
+        await using var batch = await ExcelSession.BeginBatchAsync(testFile);
+
         // Create connection-only query first
-        await using (var setupBatch = await ExcelSession.BeginBatchAsync(testFile))
-        {
-            await _powerQueryCommands.CreateAsync(
-                setupBatch, queryName, mCodeFile, PowerQueryLoadMode.ConnectionOnly);
-            await _sheetCommands.CreateAsync(setupBatch, targetSheet);
-            await setupBatch.SaveAsync();
-        }
+        await _powerQueryCommands.CreateAsync(
+            batch, queryName, mCodeFile, PowerQueryLoadMode.ConnectionOnly);
 
-        // Act
-        await using var batch = await ExcelSession.BeginBatchAsync(testFile);
+        // LoadTo should create sheet and load data
         var loadResult = await _powerQueryCommands.LoadToAsync(
             batch, queryName, PowerQueryLoadMode.LoadToTable, targetSheet);
 
diff --git a/tests/ExcelMcp.Core.Tests/Integration/Commands/PowerQuery/PowerQueryCommandsTests.LoadToExistingSheet.cs b/tests/ExcelMcp.Core.Tests/Integration/Commands/PowerQuery/PowerQueryCommandsTests.LoadToExistingSheet.cs
