diff --git a/src/ExcelMcp.Core/Commands/PowerQuery/IPowerQueryCommands.cs b/src/ExcelMcp.Core/Commands/PowerQuery/IPowerQueryCommands.cs index c032e08a..73928430 100644 --- a/src/ExcelMcp.Core/Commands/PowerQuery/IPowerQueryCommands.cs +++ b/src/ExcelMcp.Core/Commands/PowerQuery/IPowerQueryCommands.cs @@ -23,11 +23,11 @@ namespace Sbroenne.ExcelMcp.Core.Commands; /// /// TARGET CELL: targetCellAddress places tables without clearing sheet. /// TIMEOUT: 5 min auto-timeout for refresh/load. For network queries, use timeout=120 or higher. -/// timeout=0 is INVALID - must be greater than zero. +/// timeout=0 or omitted uses the 5 min default. /// [ServiceCategory("powerquery", "PowerQuery")] [McpTool("powerquery", Title = "Power Query Operations", Destructive = true, Category = "query", - Description = "Power Query M code and data loading. TEST-FIRST WORKFLOW: 1. evaluate (test M code without persisting) 2. create/update (store validated query) 3. refresh/load-to (load data to destination). IF CREATE FAILS: Use evaluate for detailed M engine error. DATETIME: Always include Table.TransformColumnTypes() for explicit column types. DESTINATIONS: worksheet (default), data-model (for DAX), both, connection-only. M-CODE: Auto-formatted via powerqueryformatter.com. TARGET CELL: targetCellAddress places tables without clearing sheet. TIMEOUT: 5 min auto-timeout. For network queries, use timeout=120 or higher. timeout=0 is INVALID.")] + Description = "Power Query M code and data loading. TEST-FIRST WORKFLOW: 1. evaluate (test M code without persisting) 2. create/update (store validated query) 3. refresh/load-to (load data to destination). IF CREATE FAILS: Use evaluate for detailed M engine error. DATETIME: Always include Table.TransformColumnTypes() for explicit column types. DESTINATIONS: worksheet (default), data-model (for DAX), both, connection-only. M-CODE: Auto-formatted via powerqueryformatter.com. TARGET CELL: targetCellAddress places tables without clearing sheet. TIMEOUT: 5 min auto-timeout. For network queries, use timeout=120 or higher. timeout=0 or omitted uses the 5 min default.")] public interface IPowerQueryCommands { /// diff --git a/src/ExcelMcp.Core/Commands/PowerQuery/PowerQueryCommands.Refresh.cs b/src/ExcelMcp.Core/Commands/PowerQuery/PowerQueryCommands.Refresh.cs index bcb77348..dc8b9190 100644 --- a/src/ExcelMcp.Core/Commands/PowerQuery/PowerQueryCommands.Refresh.cs +++ b/src/ExcelMcp.Core/Commands/PowerQuery/PowerQueryCommands.Refresh.cs @@ -29,7 +29,7 @@ public PowerQueryRefreshResult Refresh(IExcelBatch batch, string queryName, Time if (timeout <= TimeSpan.Zero) { - throw new ArgumentOutOfRangeException(nameof(timeout), "Timeout must be greater than zero."); + timeout = TimeSpan.FromMinutes(5); // Default timeout when not specified } using var timeoutCts = new CancellationTokenSource(timeout); diff --git a/tests/ExcelMcp.Core.Tests/Integration/Commands/PowerQuery/PowerQueryCommandsTests.RefreshErrors.cs b/tests/ExcelMcp.Core.Tests/Integration/Commands/PowerQuery/PowerQueryCommandsTests.RefreshErrors.cs index 71355357..fe0bf656 100644 --- a/tests/ExcelMcp.Core.Tests/Integration/Commands/PowerQuery/PowerQueryCommandsTests.RefreshErrors.cs +++ b/tests/ExcelMcp.Core.Tests/Integration/Commands/PowerQuery/PowerQueryCommandsTests.RefreshErrors.cs @@ -233,6 +233,68 @@ public void Refresh_ValidDataModelQuery_Succeeds() Assert.False(result.HasErrors); } + /// + /// Regression test: Refresh with TimeSpan.Zero timeout should use the default 5-minute timeout + /// instead of throwing ArgumentOutOfRangeException. + /// + /// BUG: The source generator produces `args.Timeout ?? default(TimeSpan)` = TimeSpan.Zero + /// when no timeout is supplied (CLI without --timeout, MCP without timeout parameter). + /// The Core method used to throw ArgumentOutOfRangeException("Timeout must be greater than zero."). + /// FIX: TimeSpan.Zero now falls back to 5-minute default. + /// + [Fact] + public void Refresh_ZeroTimeout_UsesDefaultAndSucceeds() + { + // Arrange - Create a valid worksheet query + var testExcelFile = _fixture.CreateTestFile(); + var queryName = "ZeroTimeoutQuery"; + + var validMCode = @"let + Source = #table({""Name""}, {{""Test""}}) +in + Source"; + + using var batch = ExcelSession.BeginBatch(testExcelFile); + + _powerQueryCommands.Create(batch, queryName, validMCode, PowerQueryLoadMode.LoadToTable); + batch.Save(); + + // Act - Refresh with TimeSpan.Zero (the exact value generated when timeout is omitted) + var result = _powerQueryCommands.Refresh(batch, queryName, TimeSpan.Zero); + + // Assert - Should succeed, not throw ArgumentOutOfRangeException + Assert.True(result.Success, $"Refresh failed: {result.ErrorMessage}"); + Assert.False(result.HasErrors); + } + + /// + /// Regression test: Refresh with negative timeout should use the default 5-minute timeout. + /// + [Fact] + public void Refresh_NegativeTimeout_UsesDefaultAndSucceeds() + { + // Arrange - Create a valid worksheet query + var testExcelFile = _fixture.CreateTestFile(); + var queryName = "NegativeTimeoutQuery"; + + var validMCode = @"let + Source = #table({""Name""}, {{""Test""}}) +in + Source"; + + using var batch = ExcelSession.BeginBatch(testExcelFile); + + _powerQueryCommands.Create(batch, queryName, validMCode, PowerQueryLoadMode.LoadToTable); + batch.Save(); + + // Act - Refresh with negative timeout + var result = _powerQueryCommands.Refresh(batch, queryName, TimeSpan.FromSeconds(-1)); + + // Assert - Should succeed, not throw ArgumentOutOfRangeException + Assert.True(result.Success, $"Refresh failed: {result.ErrorMessage}"); + Assert.False(result.HasErrors); + } + /// /// Verifies that refresh throws when query is connection-only (no QueryTable or DataModel connection). /// Connection-only queries have no mechanism to refresh - they're only query definitions.