mirror of
https://github.com/sbroenne/mcp-server-excel.git
synced 2026-09-19 07:53:08 +08:00
v1.4.23
99 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Stefan Broenner
|
d72d8f8d1b |
Fix/remove-telemetry-diagnostics (#247)
* chore: remove telemetry diagnostic output
* chore: update packages to .NET 10 and MCP SDK 0.4.1-preview.1
- Microsoft.Extensions.Hosting: 9.0.10 -> 10.0.0
- Microsoft.Extensions.Logging: 9.0.10 -> 10.0.0
- Microsoft.Extensions.Configuration: 9.0.10 -> 10.0.0
- Microsoft.Extensions.DependencyInjection: 9.0.10 -> 10.0.0
- System.Text.Json: 9.0.10 -> 10.0.0
- ModelContextProtocol: 0.4.0-preview.3 -> 0.4.1-preview.1
All packages forward-compatible with .NET 8.0 runtime.
No breaking changes - verified build passes with 0 warnings.
* refactor: migrate MCP tools from [Description] attributes to XML documentation
- Convert all 12 tool files to use XML docs (/// <summary>, /// <param>)
- XML docs processed by MCP SDK's XmlToDescriptionGenerator source generator
- Remove all CA1861 pragma warnings and [SuppressMessage] attributes (not needed)
- Remove dead code: ThrowUnknownAction() and invalid action error handlers
- Add GlobalUsings.cs for Chart and Range namespaces
- Update mcp-server-guide.instructions.md with NO EMOJIS rule
The XmlToDescriptionGenerator converts XML comments to [Description] attributes
at compile time, providing better IDE support and documentation integration.
* feat(mcp): add tool categories and remove outdated LLM guidance
- Add [McpMeta("category", "...")] to all 12 tools for better LLM discovery
- Remove unhelpful LLM guidance (suggestedNextActions, retryGuidance, workflowHint, operationContext)
- Update ExcelResourceProvider to use excel_file sessions
Follows principle: state facts, not guidance - LLMs figure out next steps
* fix: add prominent warning to prevent premature session close
- Update ServerInstructions in Program.cs to explicitly warn LLMs:
- WAIT for ALL pending tool calls before closing
- Closing while operations running causes failures
- Expand excel_file tool description with detailed session lifecycle
- Addresses race condition where close was called during parallel operations
* feat: add showExcel parameter to watch Excel during automation
- Add showExcel parameter to excel_file open action
- When true, Excel window becomes visible during operations
- Perfect for debugging, demos, and learning what AI is doing
Implementation:
- ExcelFileTool: Added showExcel parameter to ExcelFile() and OpenSessionAsync()
- SessionManager: Added showExcel parameter to CreateSession()
- ExcelSession: Added BeginBatch overload with showExcel parameter
- ExcelBatch: Sets Excel.Visible based on showExcel flag
Server instructions:
- Added prominent session close warnings (wait for all operations)
- LLM instructed to proactively offer showExcel for complex tasks
- Example prompt: 'Would you like me to show Excel so you can watch?'
Documentation updated:
- README.md: User-facing section with natural language examples
- FEATURES.md: Added to File Operations section
- INSTALLATION.md: Added tip after test installation step
- gh-pages/index.md: Added tip to Interactive Development feature
- McpServer README: Added to AI-Powered Workflows list
- VS Code extension copilot-instructions.md: LLM guidance
|
||
|
Stefan Broenner
|
073f25450b |
refactor: simplify telemetry - always enabled, build-time connection string (#246)
* refactor: simplify telemetry - always enabled, build-time connection string - Remove opt-out functionality (EXCELMCP_TELEMETRY_OPTOUT) - Embed AppInsights connection string at build time via MSBuild - Add Directory.Build.props.user for local dev connection string - Remove runtime environment variable lookup for connection string - Update release workflows to pass connection string to build - Remove obsolete test.runsettings file - Simplify IsEnabled to always return true * refactor: consolidate prompts and improve tool descriptions - Reduce prompts from 14 to 2 (keep only high-value guides) - Remove 13 orphaned .md content files - Fix ExcelWorksheetTool incorrect 'save' action reference - Improve ExcelRangeTool and ExcelNamedRangeTool descriptions - Slim ServerInstructions to essential info only - Remove redundant telemetry flush (SDK handles automatically) * refactor(telemetry): disable dependency tracking for HTTP calls in telemetry configuration * test hook * fix: correct Application Insights telemetry context - Set Component.Version explicitly to override SDK auto-detection (was picking up Excel COM interop 15.0.0.0 instead of app 1.0.0) - Anonymize cloud_RoleInstance (was exposing machine name) - Now all telemetry context values are privacy-safe: - application_Version: from assembly metadata - cloud_RoleName: fixed 'ExcelMcp.McpServer' - cloud_RoleInstance: anonymous hash 'instance-xxxx' - user_Id: anonymous hash of machine identity - session_Id: random per session * fix: telemetry improvements for Users/Sessions analytics - Remove debug mode from telemetry (simplify code) - Fix PageView tracking to include duration (required for User Flows) - Always override application_Version (SDK picks up Excel 15.0.0.0) - Add explicit Flush() call in smoke tests for reliable telemetry delivery - Fix smoke test shutdown to use graceful shutdown before cancellation Telemetry now correctly populates: - requests table (with duration, success, resultCode) - pageViews table (with duration for User Flows) - user_Id and session_Id (for Users/Sessions blades) - application_Version (MCP Server version, not Excel) |
||
|
Stefan Broenner
|
403ecfaf73 |
fix: replace empty catch blocks with specific exception types (#244)
* fix: replace empty catch blocks with specific exception types (#213) - NamedRangeCommands.Operations.cs: Replace empty catch blocks with COMException handlers for COM interop resilience - PowerQueryCommands.Lifecycle.cs: Replace generic Exception catch with COMException for accurate error handling - ExcelComSmokeTests.cs: Replace empty catches with InvalidOperationException and Win32Exception handlers for process cleanup Addresses CodeQL alerts: cs/empty-catch-block, cs/catch-of-all-exceptions Fixes #213 * fix: replace additional empty catch blocks with specific exception types - VbaCommands.cs: Replace empty catch and generic catch with SecurityException for registry access operations - PowerQueryCommands.Lifecycle.cs: Replace empty catch with COMException for CommandText property access - PivotTableCommands.Lifecycle.cs: Replace empty catches with COMException for field count access (RowFields, ColumnFields, DataFields, PageFields) - DataModelCommands.Read.cs: Replace empty catches with COMException and RuntimeBinderException for FormatInformation/FormatString access All catches now have specific exception types and explanatory comments. * chore: configure CodeQL suppressions for intentional patterns Suppress CodeQL alerts for legitimate exception handling patterns: - MCP Tools: catch(Exception) required for JSON error responses (MCP protocol) - CLI Commands: catch(Exception) for user-friendly error messages - COM Interop: exception handling at session boundaries for cleanup - Dynamic COM: late binding is standard pattern for Excel automation - GC.Collect: required for COM object cleanup per Microsoft guidance - Test paths: Path.Combine safe in isolated test directories - Generated code: Regex generator output triggers false positives These patterns are intentional by design, not bugs to fix. |
||
|
Stefan Broenner
|
f44c23e478 |
feat(telemetry): Switch to Application Insights SDK for full Usage analytics (#243)
Replace OpenTelemetry with Application Insights SDK to enable Users, Sessions, Funnels, and User Flows analytics in Azure portal. Changes: - Remove OpenTelemetry packages - Use Microsoft.ApplicationInsights SDK directly (v2.23.0) - Add ExcelMcpTelemetryInitializer for User.Id/Session.Id context - Simplify ExcelMcpTelemetry to use TelemetryClient.TrackEvent() - Convert to static SensitiveDataRedactor - Update .NET SDK to 8.0.416 Fixes #239 |
||
|
Stefan Broenner
|
d7a2d3d24b |
feat: Add Application Insights telemetry via OpenTelemetry (#240)
- Add OpenTelemetry + Azure Monitor for usage tracking and unhandled exceptions - Implement SensitiveDataRedactingProcessor to protect file paths/connection strings - Add telemetry to all 12 MCP tools via ExecuteToolAction pattern - Support opt-out via EXCELMCP_TELEMETRY_OPTOUT environment variable - Add debug mode via EXCELMCP_DEBUG_TELEMETRY for local testing (console output) - Build-time connection string injection in release workflow - Add Bicep templates for Azure Application Insights deployment - Add 29 tests (28 unit + 1 integration) for telemetry components - Update developer docs with telemetry setup instructions Closes #239 |
||
|
Stefan Broenner
|
d2fcc5f527 |
Fix Rule 1b violation in PowerQuery List() (#238)
* Fix Rule 1b violation in PowerQuery List() outer catch block - Remove catch block creating fake 'Error Query' entries - Replace with continue statement to skip problematic queries - Prevents exception suppression (Rule 1b compliance) - Allows fault-tolerant listing: one bad query doesn't break entire list - All 16 PowerQuery tests pass * Remove inefficient try-catch in PowerQuery Refresh() - Remove inner try-catch that suppressed COMException - Let exceptions propagate naturally (Rule 1b) - Remove complex fallback logic checking Success flag - Simpler, more efficient: just throw on error - All 16 PowerQuery tests pass * Enforce sequential test execution and remove obsolete test - Add xunit.runner.json to all test projects (Core, CLI, McpServer) - Configure parallelizeAssembly: false, maxParallelThreads: 1 - Remove obsolete VbaCommandsTests.Trust.TrustScope.cs (skipped test) - Ensures stable test execution for Excel COM interop |
||
|
Stefan Broenner
|
2828175d8c |
Fix PowerQuery List() returning 'Error Query' for workbooks with manual tables (#237)
Fixes #236 Problem: - List() returned fake 'Error Query' entries when workbooks contained manually created Excel tables (ListObjects without QueryTables) - Accessing listObject.QueryTable on manual tables throws 0x800A03EC - Outer catch block suppressed exceptions and created fake entries Solution: - Add specific try-catch for QueryTable access that continues on 0x800A03EC - Manual tables are now skipped gracefully during IsConnectionOnly detection - Queries return with correct names and formulas Changes: - PowerQueryCommands.Lifecycle.cs: Handle 0x800A03EC for QueryTable access - Tests: Add regression test with user file + synthetic test - Tests: Add diagnostic test to pinpoint exception location Test results: - All 16 PowerQuery tests pass - New regression tests verify fix works |
||
|
Stefan Broenner
|
0b6003ec09 |
docs: align all READMEs to MCP-first messaging, add centralized FEATURES.md (#235)
feat(table): add get-data action (CLI + MCP) with visible-only filtering tests(tables): add GetData coverage (unfiltered + filtered) chore: update readme-management instruction counts to 12 tools / 172 operations |
||
|
Stefan Broenner
|
6b3a557870 |
refactor: Migrate entire codebase to exception-based pattern (void mutations, no OperationResult suppression) (#233)
* Refactor MCP tools to share error handling
* Refactor Core Commands to remove exception suppression
Per CRITICAL-RULES.md Rule 1b:
- Removed catch blocks that suppress exceptions by returning OperationResult { Success = false }
- Let exceptions propagate naturally through batch.Execute()
- Kept finally blocks for COM resource cleanup
- Removed orphaned try blocks from methods with no COM cleanup needs
Modified files:
- RangeCommands.AutoFit.cs (2 methods)
- RangeCommands.Advanced.cs (5 methods)
- RangeCommands.Validation.cs (3 methods)
- RangeCommands.Formatting.cs (3 methods)
- PivotTableCommands.GrandTotals.cs (1 method)
- OlapPivotTableFieldStrategy.cs (2 methods)
- RegularPivotTableFieldStrategy.cs (1 method)
Architecture: batch.Execute() ALREADY captures exceptions via TaskCompletionSource.
Inner try-catch suppresses exceptions and loses stack context.
Exception propagation now happens at correct layer (batch, not method).
* Update copilot-instructions: Add surgical testing guidance and exception propagation pattern
CRITICAL additions:
- Integration tests take 45+ MINUTES for full suite
- ALWAYS use surgical testing with Feature filters (2-5 min per feature)
- Core Commands NEVER wrap batch.Execute() in catch blocks
- Let exceptions propagate naturally - batch.Execute() handles via TaskCompletionSource
- ONLY use finally blocks for COM cleanup, NO catch blocks returning OperationResult
This prevents 45+ minute test runs and documents correct exception handling pattern per CRITICAL-RULES.md Rule 1b.
* Remove additional violating catch blocks from PivotTable commands
Found and removed 7 more catch blocks that violated Rule 1b:
- PivotTableCommands.Subtotals.SetSubtotals
- PivotTableCommands.Grouping.GroupByDate
- PivotTableCommands.Grouping.GroupByNumeric
- PivotTableCommands.CalculatedFields.CreateCalculatedField
- PivotTableCommands.Analysis.GetData
- PivotTableCommands.Analysis.SetFieldFilter
- PivotTableCommands.Analysis.SortField
All were inside batch.Execute() and returning OperationResult { Success = false }.
Removed catch blocks, kept finally blocks for COM cleanup.
Exceptions now propagate naturally through batch.Execute().
* refactor(core): remove violating catch block from TableCommands.DataModel.cs
Removed catch block that violated Rule 1b (CRITICAL-RULES.md).
AddToDataModel now lets exceptions propagate through batch.Execute().
Pattern removed:
catch (Exception ex) {
return new OperationResult { Success = false, ErrorMessage = ... };
}
Rationale:
- batch.Execute() captures exceptions via TaskCompletionSource
- Inner catch suppresses exceptions and loses stack context
- Exceptions should originate from batch layer, not method layer
Part of comprehensive audit removing all violating catch blocks from Core Commands.
* test(core): fix Range style tests to expect exceptions after architectural changes
Updated 2 tests to expect exceptions instead of OperationResult { Success = false }:
- SetStyle_InvalidStyleName_ThrowsException (was ReturnsError)
- GetStyle_InvalidRange_ThrowsException (was ReturnsError)
Why Changed:
After removing violating catch blocks (Rule 1b), SetStyle() and GetStyle()
no longer catch Excel COM exceptions. batch.Execute() captures exceptions via
TaskCompletionSource and re-throws them.
Pattern:
Methods WITHOUT pre-validation → Excel COM throws → batch.Execute() captures →
Exception propagates to caller → Test must expect exception
Verified Clean:
- Tests with pre-validation (Sheet.Move, NamedRange.CreateBulk) still expect
Success=false correctly
- Tests with business rules (OLAP calculated fields) correctly expect Success=false
- Only methods letting Excel COM throw directly needed test updates
All 2 tests now pass with architectural fix in place.
* refactor: remove async wrappers and bulk operations
Remove ViewAsync, CreateBulk methods and update documentation
* refactor(chart): Convert Chart commands to exception-based pattern
- IChartCommands: Convert 14 method signatures (OperationResult/ChartSeriesResult/ChartListResult → void/SeriesInfo/List<ChartInfo>)
- IChartStrategy: Convert 3 data operation signatures (SetSourceRange→void, AddSeries→SeriesInfo, RemoveSeries→void)
- RegularChartStrategy: Remove OperationResult wrappers, return data types directly
- PivotChartStrategy: Convert error returns to throw NotSupportedException with helpful messages
- ChartCommands implementations: Update DataSource/Lifecycle/Appearance to match new signatures
- Remove unnecessary Models using directives
CRUD ops return void (throw on error), Query ops return data types, Exceptions replace Success/ErrorMessage wrappers.
Pattern validated: Core commands build successfully with 0 errors/warnings.
* fix(chart): Fix COM cleanup in PivotChartStrategy to use finally blocks
- SetSourceRange: Move COM Release to finally block ensuring cleanup even if exception occurs
- AddSeries: Move COM Release to finally block ensuring cleanup even if exception occurs
- RemoveSeries: Move COM Release to finally block ensuring cleanup even if exception occurs
- Declare COM objects as nullable dynamic? variables before try-catch-finally
Prevents COM object leaks when exceptions occur during PivotTable name retrieval.
Finally blocks guarantee cleanup happens regardless of try/catch outcome.
Follows proper COM interop resource management pattern from excel-com-interop.instructions.md.
* refactor(chart): Remove empty catch blocks from PivotChartStrategy
- GetInfo: Remove empty catch blocks, keep only try-finally for COM cleanup
- GetDetailedInfo: Remove empty catch block, keep only try-finally for COM cleanup
Empty catch blocks are unnecessary when finally blocks handle cleanup.
Finally blocks execute regardless of exception occurrence.
Cleaner code pattern: try { operations } finally { cleanup }
* refactor(chart): Fix COM cleanup in RegularChartStrategy
- GetInfo: Move seriesCollection to try-finally without empty catch
- GetDetailedInfo: Move seriesCollection to try-finally, keep only justified catch blocks for optional properties
- Remove COM Release from try block, move to finally block
- Keep catch blocks only where fallback values make sense (HasLegend=false, etc.)
Ensures COM objects always released even if exceptions occur.
Empty catch blocks removed where no fallback action needed.
* refactor(chart): Simplify PivotChartStrategy exception messages
- SetSourceRange: Remove COM object lookup, just throw NotSupportedException
- AddSeries: Remove COM object lookup, just throw NotSupportedException
- RemoveSeries: Remove COM object lookup, just throw NotSupportedException
LLM already knows context - no need for complex COM operations to get PivotTable name.
Simpler, cleaner, no COM cleanup needed for error-only code paths.
* refactor(chart): Convert CLI ChartCommand to exception-based pattern
- Removed Result wrapper pattern from all Execute methods
- Added try-catch exception handling with proper exit codes (0=success, 1=error)
- Query operations: return data via WriteJson
- CRUD operations: void with WriteInfo success messages
- Removed WriteResult helper method (no longer needed)
- Removed unused Models using statement
- All 14 Execute methods converted:
* ExecuteList (List<ChartInfo>)
* ExecuteRead (ChartInfoResult)
* ExecuteCreateFromRange (ChartCreateResult)
* ExecuteCreateFromPivotTable (ChartCreateResult)
* ExecuteDelete (void)
* ExecuteMove (void)
* ExecuteSetSourceRange (void)
* ExecuteAddSeries (SeriesInfo)
* ExecuteRemoveSeries (void)
* ExecuteSetChartType (void)
* ExecuteSetTitle (void)
* ExecuteSetAxisTitle (void)
* ExecuteShowLegend (void)
* ExecuteSetStyle (void)
Part of Phase 2.2: Chart CLI Updates
* refactor(chart): Convert MCP ExcelChartTool to exception-based pattern
- Removed Result wrapper anonymous object serialization
- Query operations: serialize data types directly (List<ChartInfo>, ChartInfoResult, ChartCreateResult, SeriesInfo)
- CRUD operations (void): return success message JSON after completion
- Exception handling: ExecuteToolAction wrapper catches and serializes exceptions
- All 14 actions converted:
* List → serialize List<ChartInfo>
* Read → serialize ChartInfoResult
* CreateFromRange → serialize ChartCreateResult
* CreateFromPivotTable → serialize ChartCreateResult
* Delete → success message JSON
* Move → success message JSON
* SetSourceRange → success message JSON
* AddSeries → serialize SeriesInfo
* RemoveSeries → success message JSON
* SetChartType → success message JSON
* SetTitle → success message JSON
* SetAxisTitle → success message JSON
* ShowLegend → success message JSON
* SetStyle → success message JSON
Part of Phase 2.3: Chart MCP Tool Updates
* refactor(chart): Convert MCP ExcelChartTool to exception-based pattern
- Removed Result wrapper anonymous object serialization
- Query operations: serialize data types directly (List<ChartInfo>, ChartInfoResult, ChartCreateResult, SeriesInfo)
- CRUD operations (void): return success message JSON after completion
- Exception handling: ExecuteToolAction wrapper catches and serializes exceptions
- All 14 actions converted:
* List → serialize List<ChartInfo>
* Read → serialize ChartInfoResult
* CreateFromRange → serialize ChartCreateResult
* CreateFromPivotTable → serialize ChartCreateResult
* Delete → success message JSON
* Move → success message JSON
* SetSourceRange → success message JSON
* AddSeries → serialize SeriesInfo
* RemoveSeries → success message JSON
* SetChartType → success message JSON
* SetTitle → success message JSON
* SetAxisTitle → success message JSON
* ShowLegend → success message JSON
* SetStyle → success message JSON
Part of Phase 2.3: Chart MCP Tool Updates
* refactor(namedrange): Convert NamedRange commands to exception pattern (all layers)
Core Changes:
- INamedRangeCommands: List returns List<NamedRangeInfo>, Read returns NamedRangeValue, CRUD operations void
- Added NamedRangeValue class (POCO for Read operation)
- All methods throw exceptions on error (InvalidOperationException, ArgumentException)
- Parameter validation enforced (255 char limit, no duplicates)
CLI Changes:
- All Execute methods use try-catch with exit codes (0=success, 1=exception, -1=validation)
- List/Get serialize data directly via WriteJson
- Create/Update/Delete/Write use WriteInfo for success messages
- Removed WriteResult helper method
- Removed unused using directive
MCP Server Changes:
- List serializes List<NamedRangeInfo> directly
- Read serializes NamedRangeValue directly
- Write/Create/Update/Delete wrap void operations in lambda with dummy return
- All CRUD ops return success message JSON with operation-specific messages
- Delete includes workflowHint about #NAME? errors
Test Changes (12 tests converted, all passing):
- Lifecycle.cs: 4 tests - List accesses data directly, Create/Delete void with verification
- Validation.cs: 6 tests - Parameter validation with Assert.Throws for errors
- Values.cs: 3 tests - Write/Read void/data operations with verification
All layers build successfully with 0 errors, 0 warnings
All 12 NamedRange tests passing (100%)
Phase 3 (NamedRange) complete - 6 Core methods + 12 tests converted
* refactor(conditionalformat): Convert ConditionalFormatting commands to exception pattern
Core Changes:
- IConditionalFormattingCommands: AddRule and ClearRules return void
- All methods throw exceptions on error (InvalidOperationException, ArgumentException)
- Removed OperationResult wrappers, use dummy return 0 for batch.Execute
- Removed unused using directives
CLI Changes:
- ExecuteAddRule/ExecuteClearRules use try-catch with exit codes (0=success, 1=exception, -1=validation)
- WriteInfo for success messages
- Removed WriteResult helper method
- Removed unused using directive
MCP Server Changes:
- AddRule/ClearRules wrap void operations in lambda with dummy return
- Return success message JSON with operation-specific messages
- Keep workflowHints for formatting verification guidance
All layers build successfully with 0 errors, 0 warnings
No tests exist for ConditionalFormatting commands
Phase 4.1 (ConditionalFormatting) complete - 2 Core methods converted
* Refactor: Convert Connection, File, VBA subsystems from OperationResult to exception-throwing pattern
- Connection: IConnectionCommands methods now return void, throw on error
- File: CreateEmpty returns void, throws on error
- VBA: Trust, Import, Delete, Run operations throw exceptions
- CLI layer: Updated error handling for void operations
- MCP layer: Responses serialized as {success: true} for void operations
- Test infrastructure: Updated 14 test helper files to check exceptions
Mutation operations now follow .NET convention of throwing exceptions instead of returning error-wrapped results. Read operations continue returning typed results.
All three layers (Core, CLI, MCP) coordinated for consistent behavior.
* Merge branch 'feature/shared-tool-error-handling' of https://github.com/sbroenne/mcp-server-excel into feature/shared-tool-error-handling
* Refactor: Convert Sheet subsystem from OperationResult to exception-throwing pattern
Convert 12 Sheet mutation methods to void signatures across all layers:
- ISheetCommands: Create, Rename, Copy, Delete, Move, CopyToWorkbook, MoveToWorkbook, SetTabColor, ClearTabColor, SetVisibility, Show, Hide, VeryHide
- SheetCommands (Core): All implementations use batch.Execute() with return 0, exceptions propagate
- SheetCommand (CLI): All void operations wrapped in try-catch with success/error messages
- ExcelWorksheetTool (MCP): All 10 void methods use WithSession() + return 0 + JSON serialization
Production code compiles cleanly. MCP smoke test: PASSED. Tests require separate refactoring for void method assertions.
* Refactor: Convert Table subsystem from OperationResult to exception-throwing pattern (17 void methods, Core+CLI+MCP layers)
- ITableCommands: 17 mutation methods converted to void signatures with exception documentation
- Core layer: 17 implementations across 7 partial files use batch.Execute() return 0 pattern
- CLI layer: 10 void handlers use try-catch-serialize pattern with success/error responses
- MCP layer: 10 void tool methods use try-catch-JSON pattern (fixed 8 broken WithSession<T> calls)
- All production layers compile cleanly: 0 errors, 0 warnings
- MCP smoke test PASSED: all 11 tools working correctly
- Test layer errors (44) deferred: void assertion issues to be fixed separately
- Pattern matches Sheet/Connection/File/VBA subsystem conversions
* feat: Convert PowerQuery subsystem mutation methods to void pattern (Delete, Create, Update, LoadTo, RefreshAll)
* Convert DataModel subsystem to exception-throwing pattern
- Interface: 7 void mutation methods (DeleteMeasure, DeleteRelationship, Refresh (2 overloads), CreateMeasure, UpdateMeasure, CreateRelationship, UpdateRelationship)
- Core: Converted Write.cs (6 mutations) and Refresh.cs (2 methods) to void pattern with batch.Execute((ctx, ct) => { ...code...; return 0; })
- CLI: 7 handlers with try-catch-console JSON output
- MCP: 7 tool methods with try-catch-JSON error handling
- Removed unused Core.Models imports from Write.cs and Refresh.cs
- Build: 0 errors, 0 warnings across Core, CLI, MCP layers
Note: Test failures expected (44 in Core, 50+ in other test files due to void assertion issues) - these are deferred per testing strategy (Rule 14)
* Merge branch 'feature/shared-tool-error-handling' of https://github.com/sbroenne/mcp-server-excel into feature/shared-tool-error-handling
* fix: Complete void test migration - 84/84 errors fixed
Complete migration of 84 test compilation errors to exception-based pattern.
This completes the integration of void Execute infrastructure with test layer updates.
✅ BUILD VERIFIED: 0 errors, 0 warnings
✅ PRE-COMMIT PASSED: All quality gates passing
Test Migrations (84 total):
- Sheet tests: 19 errors (Move, TabColor, Visibility, Lifecycle)
- Helper fixtures: 4 errors (PowerQuery, Table, DataModel, PivotTable)
- PowerQueryCommandsTests: 19 errors (Create, Update, Delete, LoadTo calls)
- DataModelCommandsTests: 12 errors (CreateMeasure, UpdateMeasure, CreateRelationship, DeleteRelationship)
- DataModelTestsFixture: 1 error (var assignment from void CreateMeasure)
- TableCommandsTests: 2 errors (Create calls in tests and fixture)
- PivotTableCommandsTests: 3 errors (Create calls from Table/DataModel)
- PivotTableCommandsTests.OlapFields: 2 errors (CreateMeasure calls)
Pattern Applied (Two types):
1. CS0815 'Cannot assign void': Remove 'var result =' and 'Assert.True(result.Success)' lines
2. Remove unnecessary success checks - void methods throw on error instead
Infrastructure:
- void Execute(Action<ExcelContext, CancellationToken>) in IExcelBatch/ExcelBatch
- Delegates to Execute<T>(Func<...>) returning dummy 0
- Non-breaking enhancement for clean void method support
Verification:
✅ Build: 0 errors, 0 warnings
✅ Pre-commit checks: All passed
- COM leak detection: 0 leaks
- Success flag validation: All consistent
- MCP implementation coverage: 100%
- MCP server smoke test: Passed
✅ Feature branch active (feature/shared-tool-error-handling)
* docs: Add void test migration completion summary
* refactor: Convert 11 Range void mutations to fire-and-forget pattern across all layers
Complete refactoring of Range command mutations to void pattern:
CORE LAYER (11 void mutations):
- RangeCommands.Search.cs: Replace, Sort
- RangeCommands.Formatting.cs: SetStyle, FormatRange
- RangeCommands.Validation.cs: ValidateRange, RemoveValidation
- RangeCommands.AutoFit.cs: AutoFitColumns, AutoFitRows
- RangeCommands.Advanced.cs: MergeCells, UnmergeCells, SetCellLock
INTERFACE:
- IRangeCommands.cs: All 11 signatures changed from OperationResult to void
MCP TOOL LAYER:
- ExcelRangeTool.cs: All 11 calls updated to use WithSession<object?>() pattern
* Explicit type parameter resolves CS0411 type inference on void returns
* Returns { Success = true } JSON directly for all void mutations
* Lambda formatting fixed (IDE0055 compliance)
CLI LAYER:
- RangeCommand.cs: All 11 command handlers adapted
* Removed var result = ... assignments
* Call void methods directly
* Return { Success = true } JSON on completion
TEST LAYER (14+ files):
- RangeCommandsTests: All test files updated
* Removed var result = ... assignments
* Removed Assert.True(result.Success, ...) assertions
* Void methods throw on failure; silence = success
BUILD STATUS: ✅ Clean - 0 errors, 0 warnings
LEGITIMATE OperationResult METHODS (NOT CONVERTED):
- SetValues, SetFormulas, Copy, SetNumberFormat, AddHyperlink
- These data mutations need validation and result reporting
PATTERN:
- Void mutations: Fire-and-forget with exception-based error handling
- Exceptions propagate naturally through batch.Execute() layer
- No OperationResult wrapping needed for simple write operations
* chore: Remove unused imports and temporary documentation files
- Clean up unused using directives across Table and PivotTable commands
- Remove temporary documentation files
- Maintain code style consistency
* fix: Update MartinCostello.Logging.XUnit package version to 0.7.0
* fix(chart): Suppress CodeQL false positives for dynamic COM interop in Strategy pattern
- Add pragma warning disable CS8604 for all Strategy method calls
- CodeQL cannot verify dynamic Excel COM objects have required methods at compile-time
- All methods verified to exist in both RegularChartStrategy and PivotChartStrategy
- False positives: SetSourceRange, AddSeries, RemoveSeries, GetInfo, GetDetailedInfo
- Addresses code scanning alerts #2631-2635
|
||
|
Stefan Broenner
|
e512204732 |
feat: Add comprehensive Chart operations support (Regular Charts and PivotCharts) (#230)
* feat: Add comprehensive Chart operations support (Regular Charts and PivotCharts) Implements complete chart management functionality across all three layers: - Core Commands: 15 methods for chart lifecycle, data source, and appearance operations - MCP Server Tool: excel_chart with 15 actions exposed to LLM agents - CLI Commands: chart command with 15 actions for command-line automation Core Implementation (940+ lines): - IChartCommands.cs: Interface defining 15 chart operations - ChartCommands.Lifecycle.cs (500+ lines): List, Read, CreateFromRange, CreateFromPivotTable, Delete, Move - ChartCommands.DataSource.cs (200+ lines): SetSourceRange, AddSeries, RemoveSeries - ChartCommands.Appearance.cs (240+ lines): SetChartType, SetTitle, SetAxisTitle, ShowLegend, SetStyle - Strategy Pattern: RegularChartStrategy and PivotChartStrategy (220+ lines each) - ChartType.cs: 70+ chart type enum values (Column, Bar, Line, Pie, Area, Scatter, etc.) - ChartResults.cs: 4 result classes (ChartListResult, ChartInfoResult, ChartCreateResult, ChartUpdateResult) MCP Server Integration (473 lines): - ExcelChartTool.cs: Main tool with 15 actions - ChartAction enum: 15 values (List, Read, CreateFromRange, CreateFromPivotTable, Delete, Move, SetSourceRange, AddSeries, RemoveSeries, SetChartType, SetTitle, SetAxisTitle, ShowLegend, SetStyle) - ToActionString() extension: Maps ChartAction to kebab-case strings CLI Integration (368 lines): - ChartCommand.cs: Command<Settings> pattern with 15 actions - Registered in Program.cs with DI support Testing (27 tests + smoke test): - ChartCommandsTests.Lifecycle.cs: 9 tests covering chart creation and management - ChartCommandsTests.Appearance.cs: 10 tests covering appearance customization - ChartCommandsTests.DataSource.cs: 8 tests covering data source operations - CoreCommandsCoverageTests.cs: Automated verification of enum/method coverage - McpServerSmokeTests.cs: End-to-end smoke test for excel_chart tool (now includes Chart!) - ToolDiscoveryTests.cs: Discovery verification for excel_chart Features: - Regular Charts: Static charts from Excel ranges (A1:D10) - PivotCharts: Dynamic charts linked to PivotTables - 70+ chart types supported (Column, Bar, Line, Pie, Area, Scatter, Bubble, Stock, Surface, Radar, etc.) - Full positioning control (left, top, width, height in points) - Series management (add/remove data series) - Appearance customization (titles, axes, legends, styles) - Round-trip validation in all tests Architecture: - Strategy Pattern: Separates Regular vs PivotChart logic - Batch-aware: All operations use IExcelBatch for performance - COM-safe: Proper resource cleanup with try/finally blocks - Range object fix: SetSourceData requires Range object, not string Documentation: - specs/CHART-API-SPECIFICATION.md: 400+ line comprehensive specification - All methods fully documented with XML comments - Examples provided for all operation types Builds successfully with 0 warnings, 0 errors. All 27 Core tests pass. Smoke test passes (validates end-to-end workflow with all 13 tools). 100% Core method coverage verified. MCP tool discoverable and functional. * fix: Update Chart test assertions to match Excel SERIES formula behavior - SetSourceRange tests now check for Sheet1 + column references ($D$, $E$, $A$, $B$) instead of simple ranges - AddSeries_InvalidSeriesIndex now expects COMException for invalid index instead of Success=false - All 26 Chart integration tests now pass (9 lifecycle, 8 data source, 9 appearance) - Fixes #229 * refactor: Rewrite Connection tests to accurately document Excel COM behavior - TEXT/WEB connections: Create works successfully - OLEDB connections: Fail with ArgumentException (Excel COM Add2() limitation) - ODBC connections: Create succeeds even without valid DSN - Duplicate names: Excel handles gracefully (prevents or auto-renames) - Removed confusing 'test always passes' logic - Added clear documentation about connection type support - Fixed exception propagation in CreateConnection helper (removed InvalidOperationException wrapping) All 21 Connection tests now pass (1 skipped). * feat(connection): Add ODC import and enhance SetProperties with connection string/command text/description updates FEATURE ENHANCEMENTS: - Add ImportFromOdc action to import Office Data Connection (.odc) files - Expose connectionString, commandText, description parameters in SetProperties (was behavior-only) - Add exception handler for Excel 0x800A03EC error (ODC connection string updates blocked) - Block TEXT/WEB connection creation via Create (redirect to Power Query) - Add import-odc action to CLI with --odc-file parameter IMPLEMENTATION: - New ConnectionCommands.ImportOdc.cs with AddFromFile COM API - Enhanced UpdateConnectionProperties to handle all 7 properties (connection string, command text, description + 4 behavior properties) - MCP Tool: SetProperties accepts newConnectionString, newCommandText, newDescription parameters - CLI: ExecuteImportOdc method with OdcFilePath setting - Exception scoping: Only catches 0x800A03EC when updating connection string TESTING (13 new tests): - ConnectionCommandsTests.ImportOdc.cs: * ImportFromOdc_ValidFile_ImportsConnection * ImportFromOdc_MissingFile_ReturnsError * ImportFromOdc_DuplicateImport_AddsSecondConnection * ImportFromOdc_ThenUpdateCommandText_RefreshSucceeds (proves functional query change) * ImportFromOdc_ThenUpdateCommandText_ConnectionUpdated * ImportFromOdc_ThenUpdateDescription_ConnectionUpdated * CreateConnection_ThenUpdateConnectionString_Succeeds (proves ODBC connections allow updates) - ConnectionCommandsTests.Create.cs: * Create_TextConnection_ThrowsNotSupportedException (TEXT blocked) * Create_WebConnection_ThrowsNotSupportedException (WEB blocked) * Create_OleDbConnection_ThrowsArgumentException (Excel COM limitation) * Create_OdbcConnection_ReturnsSuccess (ODBC works) * Updated Create_DuplicateName and Create_WithDescription to use ODBC - Helper: SQLiteDatabaseHelper for reliable OLEDB testing DOCUMENTATION: - Updated README tool counts: 10 actions (was 9) - Updated architecture-patterns.instructions.md: 11 tools (corrected from 12) - Updated excel_connection tool description: ODC import, TEXT/WEB removal guidance - Added Directory.Packages.props: Microsoft.Data.Sqlite package - Removed outdated vscode-extension/INSTALL.md (content merged to main README) KEY FINDINGS: - ODC-imported connections: Connection string updates blocked (0x800A03EC) - Programmatically-created ODBC connections: Connection string updates work - Command text updates are functional (refresh executes new query, not cosmetic) - TEXT/WEB connections now blocked from Create (use Power Query or ODC import) * refactor: Remove ODC import functionality (blocked by Windows Defender ASR) - Removed ImportFromOdc action from ConnectionAction enum and all implementations - Deleted ConnectionCommands.ImportOdc.cs (implementation file) - Deleted ConnectionCommandsTests.ImportOdc.cs (all test files) - Removed import-odc from MCP tool, CLI, and interface - Removed OdcFilePath CLI parameter - Fixed Delete and View tests to use ODBC connections instead of TEXT - Updated Refresh tests to use simpler ODBC connections - Updated all READMEs: connection count 10→9 actions, 155→154 total actions REASON: Excel's AddFromFile() COM API creates .lnk shortcuts in 'My Data Sources' folder, triggering Windows Defender Attack Surface Reduction (ASR) rule 'Block Office applications from creating executable content'. This blocks all users with default Defender policies (common in enterprise). ALTERNATIVE APPROACH: Users can manually create connections via excel_connection create action with connection strings, or use Power Query (excel_powerquery) for data imports. * feat: Enhance Excel tools with timeout safeguards and error handling - Added timeout safeguards to `ExcelPivotTableTool` and `ExcelPowerQueryTool` to handle long-running operations, returning suggested next actions on timeout. - Updated `CreateFromDataModel` and `LoadToPowerQueryAsync` methods to include detailed error messages and operation context in case of timeouts. - Removed unused SQLite database helper class and replaced it with ACE OLEDB helper functions for better integration testing. - Updated connection tests to utilize the new ACE OLEDB helper methods, ensuring proper creation and refreshing of connections. - Adjusted README to reflect the current number of supported operations (154) in the Excel MCP Server. |
||
|
Stefan Broenner
|
72c968c0ba |
feat: Add cross-workbook worksheet operations (copy-to-workbook, move-to-workbook) (#231)
Implements GitHub issue #187 - Enable copying and moving worksheets between different Excel workbooks. **Architecture Changes:** - Extended IExcelBatch to support multiple workbooks in single Excel.Application instance - Added Workbooks property (IReadOnlyDictionary<string, dynamic>) and GetWorkbook(string) method - Modified ExcelBatch constructor to accept string[] workbookPaths and open all in shared COM context - Updated ExcelSession.BeginBatch to accept params string[] filePaths **Core Implementation:** - Added CopyToWorkbook and MoveToWorkbook methods to ISheetCommands - Implemented using Excel COM Worksheet.Copy and Worksheet.Move APIs - Extended List() and Create() with optional filePath parameter for multi-workbook scenarios - Uses batch.GetWorkbook(filePath) to target specific workbook in multi-file batches **MCP Server:** - Updated ExcelWorksheetTool to create temporary multi-file batches for cross-workbook operations - Added comprehensive cross-workbook documentation to tool description - Updated excel_worksheet.md prompt with workflow examples and guidance **Tests:** - Converted 10 cross-workbook tests from error-handling to positive validation - Updated 18 test files for new ExcelBatch constructor signature (array parameter) - All 9 cross-workbook integration tests passing **Documentation:** - Updated README.md: 13→16 worksheet actions, added cross-workbook capability - Updated CLI README: Added copy-to-workbook/move-to-workbook examples - Updated GitHub Pages: 158 operations (fixed inconsistencies), comprehensive action tables - Updated vscode-extension README: Already correct (16 actions) **Key Benefits:** - Single Excel.Application for multiple workbooks = proper COM object sharing - No RCW isolation issues between batches - Clean multi-workbook API: ExcelSession.BeginBatch(file1, file2, file3) - LLMs now have clear guidance on when/how to use cross-workbook operations |
||
|
Copilot
|
ffbef07a2e |
Add tests and documentation for Connection Delete action (#228)
* Initial plan * Add comprehensive tests for Connection Delete action and update documentation Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Add MCP Server integration tests for Connection Delete action Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> |
||
|
Stefan Broenner
|
76f77a7240 | Fix integration tests: Add missing batch-test-static.xlsx file (#225) | ||
|
Stefan Broenner
|
9e7d532196 |
refactor: eliminate exception wrapping across all command layers (205+ catch blocks removed) (#224)
* refactor: centralize Excel shutdown logic and add early path validation
- Extract SaveWorkbookWithTimeout() to eliminate code duplication (~70 lines)
- Create ExcelShutdownService for unified workbook close and Excel quit
- Remove GC.Collect() calls per Microsoft guidance (RCW finalizers handle cleanup)
- Add early path length validation (218 char limit) before Excel COM operations
- Convert COM path errors to PathTooLongException with clear messages
- Update all exception catch blocks to handle nested AggregateException
- Add Microsoft.Extensions.Resilience for COM busy retry with exponential backoff
- Update documentation to reflect batch API patterns and remove deprecated references
Benefits:
- Fail fast: Path validation before spawning Excel process
- DRY: Single shutdown implementation used by ExcelBatch, ExcelSession, FileCommands
- Performance: No forced GC, automatic RCW cleanup
- Resilience: 6-attempt retry for transient COM busy errors
- Clear errors: PathTooLongException → JSON error messages for LLMs
* refactor: remove unnecessary try-catch blocks for cleaner code
* Potential fix for code scanning alert no. 2575: Generic catch clause
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Potential fix for code scanning alert no. 2576: Generic catch clause
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* refactor: clean up Excel process termination logic in tests
* refactor: update documentation for ExcelSession tests to clarify cleanup process
* Refactor Excel Tool Classes and Tests
- Removed unused variables and comments in ExcelNamedRangeTool, ExcelPowerQueryTool, ExcelRangeTool, ExcelVbaTool, and ExcelWorksheetTool to clean up the code.
- Updated tests in ExcelComSmokeTests, ExcelBatchTests, ExcelSessionTests, and SessionManagerTests to improve readability and maintainability by eliminating unnecessary variable assignments.
- Consolidated sheet command tests into a single file, SheetCommandsTests.Move.cs, and removed redundant tests from the previous SheetCommandsTests.MoveAndCrossWorkbook.cs file.
- Ensured all tests maintain functionality while improving structure and clarity.
* refactor: streamline loading logic in Power Query commands
* refactor: update Power Query refresh methods to enforce timeout and improve error handling
* refactor: remove unused result properties and exception wrapping from PowerQuery commands
- Removed IsRetryable, RetryGuidance, OperationContext from ResultBase
These fields were never consumed by CLI, MCP Server, or LLM layers
- Refactored PowerQuery commands to let exceptions bubble naturally
Removed outer try-catch wrapping from 6 partial files:
- PowerQueryCommands.Create.cs
- PowerQueryCommands.View.cs
- PowerQueryCommands.Update.cs
- PowerQueryCommands.Lifecycle.cs
- PowerQueryCommands.Refresh.cs
- PowerQueryCommands.LoadTo.cs
- Pattern: batch.Execute() already captures exceptions via TaskCompletionSource
No need to wrap at method level. Keep finally blocks for COM cleanup.
Test Results:
- PowerQuery: 14/14 passed
- CLI: 2/2 passed
- Connection: 13/13 passed
- MCP: 44/46 passed (2 pre-existing PivotTable enum failures)
- Build: 0 errors, 0 warnings
Architecture validation confirms sound design:
- Exception propagation works end-to-end
- COM resource cleanup via finally blocks still functions
- No loss of functionality
* refactor: remove exception wrapping from all remaining Core commands
Applied the same pattern across 21 additional command files:
- Connection (3 files)
- VBA (3 files)
- Table (6 files)
- Sheet (4 files)
- Range (4 files)
- DataModel (3 files)
- PivotTable (6 files)
- NamedRange (2 files)
- ConditionalFormat (1 file)
Changes:
- Removed outer try-catch wrappers around batch.Execute() calls
- Preserved all finally blocks for critical COM resource cleanup
- Preserved inner try-catch for specific exception handling
- Exceptions now flow naturally to batch.Execute() via TaskCompletionSource
Statistics:
- 21 files refactored (+ 8 from PowerQuery = 29 total)
- 41 methods updated
- 553 net lines removed (982 deleted, 429 inserted)
- 0 compilation errors
- Build: SUCCESS
Test Results (spot checks):
- PowerQuery: 14/14 PASSED
- Worksheets: 36/36 PASSED
Architecture validated across entire Core command layer.
* refactor: remove all suppressions from RegularPivotTableFieldStrategy.cs
- Removed 11 exception-suppressing catch blocks (Success=false returns)
- Preserved all finally blocks for COM resource cleanup
- Methods refactored: ListFields, AddRowField, AddColumnField, AddValueField,
AddFilterField, RemoveField, SetFieldName, SetFieldFunction, SetFieldFormat,
SetFieldFilter, SortField, SetLayout, SetSubtotals, SetGrandTotals
- Exceptions now flow naturally to callers
- Build: 0 errors confirmed
- Architecture validated - batch.Execute() TaskCompletionSource properly receives exceptions
* refactor: remove exception suppressions from OlapPivotTableFieldStrategy.cs
- Removed all exception-suppressing catch blocks (Success=false returns)
- Preserved safe fallback patterns (continue in loop, conditional rethrrows)
- Methods affected: ListFields and related field manipulation methods
- Build: 0 errors confirmed
- 17 safe fallback catches remaining (architectural patterns - KEEP)
- OlapPivotTableFieldStrategy refactoring complete
* refactor: remove exception suppression from PowerQuery commands (21 patterns)
- PowerQueryCommands.LoadTo.cs: 6 patterns removed
* Parameter validation (targetCellAddress) → ArgumentException
* Query not found → InvalidOperationException
* Sheet access, table overlap, cell data validation → InvalidOperationException
* Removed catch block suppressing Data Model exceptions
- PowerQueryCommands.Update.cs: 4 patterns removed
* Query name validation → ArgumentException
* Empty M code validation → ArgumentException
* Query not found → InvalidOperationException
* Removed catch block suppressing refresh exceptions
- PowerQueryCommands.View.cs: 2 patterns removed
* Query name validation → ArgumentException
* Query not found → InvalidOperationException
- PowerQueryCommands.Refresh.cs: 2 patterns removed
* Query name validation → ArgumentException
* Query not found with suggestion → InvalidOperationException
- PowerQueryCommands.Lifecycle.cs: 4 patterns removed
* GetLoadConfig validation → ArgumentException
* GetLoadConfig query not found → InvalidOperationException
* Delete validation → ArgumentException
* Delete query not found → InvalidOperationException
- PivotTableCommandsTests: 3 tests corrected
* Fixed GetInfo_NonExistent, Delete_NonExistent to use Assert.Throws
* Updated CreateFromDataModel_NoDataModel error message expectation
All exceptions now propagate naturally through batch.Execute().
Build: 0 errors, 0 warnings
Tests: All 14 PowerQuery tests pass, 3 PivotTable tests pass
* refactor: remove exception suppression from Range commands (61 patterns)
Converted all error suppression patterns in RangeCommands to proper exception propagation:
**Files Fixed (61 patterns total):**
- RangeCommands.Editing.cs: 21 patterns removed
* Copy, CopyValues, CopyFormulas: Convert null checks → InvalidOperationException, remove catch blocks
* InsertCells, DeleteCells, InsertRows, DeleteRows, InsertColumns, DeleteColumns: Convert null checks → InvalidOperationException, remove catch blocks
- RangeCommands.Search.cs: 8 patterns removed
* Find: Convert null check → InvalidOperationException, remove catch block
* Replace: Convert null check → InvalidOperationException, remove catch block
* Sort: Convert null checks + validations → InvalidOperationException/ArgumentException, remove catch block
- RangeCommands.NumberFormat.cs: 8 patterns removed
* GetNumberFormats, SetNumberFormat: Convert null checks → InvalidOperationException, remove catch blocks
* SetNumberFormats: Convert null checks + validation errors → InvalidOperationException/ArgumentException, remove catch blocks
- RangeCommands.Hyperlinks.cs: 8 patterns removed
* AddHyperlink, RemoveHyperlink, ListHyperlinks, GetHyperlink: Convert null checks → InvalidOperationException, remove catch blocks
- RangeCommands.Values.cs: 4 patterns removed
* GetValues, SetValues: Convert null checks → InvalidOperationException
* Convert specific COM exceptions (E_OUTOFMEMORY) → throw InvalidOperationException (not suppress)
- RangeCommands.Formulas.cs: 4 patterns removed
* GetFormulas, SetFormulas: Convert null checks → InvalidOperationException
* Convert specific COM exceptions (E_OUTOFMEMORY) → throw InvalidOperationException (not suppress)
- RangeCommands.Discovery.cs: 6 patterns removed
* GetUsedRange, GetCurrentRegion, GetInfo: Convert null checks → InvalidOperationException, remove catch blocks
- RangeHelpers.cs: 2 patterns removed
* ClearRange helper: Convert null check → InvalidOperationException, remove catch block
**Pattern Applied:**
- Parameter validation (BEFORE batch.Execute()): null → ArgumentException
- Not found / Invalid state (INSIDE batch.Execute()): null → InvalidOperationException
- Specific COM exceptions: Rethrow as InvalidOperationException with context (not suppress)
- Remove all catch blocks that return OperationResult { Success = false }
All exceptions now propagate naturally through batch.Execute().
Build: 0 errors, 0 warnings
* refactor: remove exception suppression from Range commands (61 patterns)
Converted all error suppression patterns in RangeCommands to proper exception propagation:
**Files Fixed (61 patterns total):**
- RangeCommands.Editing.cs: 21 patterns removed
* Copy, CopyValues, CopyFormulas: Convert null checks → InvalidOperationException, remove catch blocks
* InsertCells, DeleteCells, InsertRows, DeleteRows, InsertColumns, DeleteColumns: Convert null checks → InvalidOperationException, remove catch blocks
- RangeCommands.Search.cs: 8 patterns removed
* Find: Convert null check → InvalidOperationException, remove catch block
* Replace: Convert null check → InvalidOperationException, remove catch block
* Sort: Convert null checks + validations → InvalidOperationException/ArgumentException, remove catch block
- RangeCommands.NumberFormat.cs: 8 patterns removed
* GetNumberFormats, SetNumberFormat: Convert null checks → InvalidOperationException, remove catch blocks
* SetNumberFormats: Convert null checks + validation errors → InvalidOperationException/ArgumentException, remove catch blocks
- RangeCommands.Hyperlinks.cs: 8 patterns removed
* AddHyperlink, RemoveHyperlink, ListHyperlinks, GetHyperlink: Convert null checks → InvalidOperationException, remove catch blocks
- RangeCommands.Values.cs: 4 patterns removed
* GetValues, SetValues: Convert null checks → InvalidOperationException
* Convert specific COM exceptions (E_OUTOFMEMORY) → throw InvalidOperationException (not suppress)
- RangeCommands.Formulas.cs: 4 patterns removed
* GetFormulas, SetFormulas: Convert null checks → InvalidOperationException
* Convert specific COM exceptions (E_OUTOFMEMORY) → throw InvalidOperationException (not suppress)
- RangeCommands.Discovery.cs: 6 patterns removed
* GetUsedRange, GetCurrentRegion, GetInfo: Convert null checks → InvalidOperationException, remove catch blocks
- RangeHelpers.cs: 2 patterns removed
* ClearRange helper: Convert null check → InvalidOperationException, remove catch block
**Pattern Applied:**
- Parameter validation (BEFORE batch.Execute()): null → ArgumentException
- Not found / Invalid state (INSIDE batch.Execute()): null → InvalidOperationException
- Specific COM exceptions: Rethrow as InvalidOperationException with context (not suppress)
- Remove all catch blocks that return OperationResult { Success = false }
All exceptions now propagate naturally through batch.Execute().
Build: 0 errors, 0 warnings
* refactor(core): Convert DataModel validation errors to exceptions (31 patterns)
- DataModelCommands.Write.cs: 18 patterns fixed
- DeleteMeasure, DeleteRelationship, CreateMeasure, UpdateMeasure
- CreateRelationship, UpdateRelationship
- NoDataModelTables, MeasureNotFound, TableNotFound, RelationshipNotFound
- All validation errors before batch.Execute() → InvalidOperationException
- DataModelCommands.Read.cs: 10 patterns fixed
- ListMeasures, Read, ListRelationships, ListColumns, ReadTable, ReadInfo
- NoDataModelTables, MeasureNotFound, TableNotFound
- All validation errors → InvalidOperationException
- DataModelCommands.Refresh.cs: 2 patterns fixed
- NoDataModelTables, TableNotFound → InvalidOperationException
- Kept Model.Refresh() catch (line 71) - valid recoverable COM exception
Part of exception propagation refactoring. Let batch.Execute() handle
exceptions via TaskCompletionSource for proper STA thread propagation.
Related: PowerQuery (SHA
|
||
|
Stefan Broenner
|
ae71b7cff9 |
feat(pivottable): Complete PivotTable expansion (Issue #219) (#223)
* feat(pivottable): Add date grouping by Days/Months/Quarters/Years (Task 1 of Issue #219) Implements GroupByDate operation for PivotTable fields with support for all 4 date intervals. Excel automatically creates hierarchical groupings (e.g., Months + Years) for proper time-based analysis. CRITICAL DISCOVERY - NumberFormat Requirement: - Source data MUST be formatted with date NumberFormat BEFORE creating PivotTable - Without proper formatting, dates stored as serial numbers (e.g., 45672) with 'Standard' format - Excel silently fails to group plain numbers - grouping requires date-formatted data - Solution: Apply NumberFormat = 'm/d/yyyy' to source range before PivotTable creation Implementation: - Added DateGroupingInterval enum (Days, Months, Quarters, Years) - Implemented GroupByDate in RegularPivotTableFieldStrategy using Range.Group() - Uses PivotField.DataRange.Cells[1,1] for single cell grouping - Periods array: [Seconds, Minutes, Hours, Days, Months, Quarters, Years] - Hierarchical grouping: Months/Quarters automatically include Years - MCP Server: Added GroupByDate action to ExcelPivotTableTool with dateGroupingInterval parameter Infrastructure Changes: - Added xUnit logging support via MartinCostello.Logging.XUnit package - Added IExcelBatch.Logger property with proper STA thread affinity - Logger passed through batch operations for diagnostics Test Coverage (4 new tests, all passing): - GroupByDate_DaysInterval_CreatesDailyGroups - GroupByDate_MonthsInterval_CreatesMonthlyGroups - GroupByDate_QuartersInterval_CreatesQuarterlyGroups - GroupByDate_YearsInterval_CreatesYearlyGroups All 38 PivotTable tests passing. Technical Details: - Date serial: 45672 (Double) = 2025-01-15 (days since 1900-01-01) - DataRange for Row/Column fields = 'Items in the field' - Group() called on single cell, not entire range - Error logging preserved with #pragma CA1848 suppression Documentation: - XML docs on interface and implementation with NumberFormat requirement - Code examples showing proper date formatting Related: #219 (Task 1 complete) * feat(pivottable): Add numeric grouping + remove misleading Async naming (Task 2 of Issue #219) Implements GroupByNumeric operation for numeric field range grouping. Creates groups like 0-100, 100-200, 200-300 for age groups, price ranges, score bands. Implementation: - Uses Range.Group() with Start, End, By parameters - By parameter specifies interval size (e.g., 100) - Start/End nullable → null = auto-detect field min/max - Periods parameter ignored for numeric fields (only for dates) - Type casting required: double? → object for COM interop - VB.NET compatibility: Renamed 'end' parameter to 'endValue' BREAKING CHANGE (Code Quality Improvement): - Removed misleading 'Async' suffix from 21 synchronous methods in ExcelPivotTableTool - Methods are synchronous (no async/await/Task) but were incorrectly named - User observation: 'why do we have Async - everything is Sync!' (correct!) - Affected: List, Read, Create*, Delete, Refresh, *Field, GroupBy* methods - Impact: Code now accurately reflects synchronous implementation - No API change: Method names were internal/private Documentation Updates: - Rewrote mcp-server-guide.instructions.md to remove all Async references - Clarified that all action methods are synchronous - Tool signatures are 'async Task<string>' only for MCP SDK requirements - Updated all code examples to show synchronous patterns Test Coverage (3 new tests, all passing): - GroupByNumeric_AutoRange_CreatesNumericGroups (interval=100, auto min/max) - GroupByNumeric_CustomRange_CreatesNumericGroups (0-1000, interval=200) - GroupByNumeric_SmallInterval_CreatesFineGrainedGroups (interval=50) - Fixed aggregation function: Use AggregationFunction.Count for text fields Technical Details: - Type casting: start.HasValue ? (object)start.Value : true - COM interop: Excel expects object type for Group() parameters - VB.NET interop: 'end' keyword conflict → renamed to 'endValue' - OLAP: Not supported (returns clear error with Power Pivot guidance) All 41 PivotTable tests passing (38 existing + 3 new). Related: #219 (Task 2 complete) * feat(pivottable): Add calculated fields with formulas Implements Task 3 of Issue #219: CreateCalculatedField operation FEATURES: - Regular PivotTables: Full support via CalculatedFields.Add() API - OLAP PivotTables: NotSupported (use DAX measures instead) - Formula syntax: =FieldName1 operator FieldName2 (e.g., =Revenue-Cost) - Operators: + - * / ^ () IMPLEMENTATION: - IPivotTableFieldStrategy: CreateCalculatedField interface method - RegularPivotTableFieldStrategy: Uses pivot.CalculatedFields().Add() - OlapPivotTableFieldStrategy: Returns NotSupported with DAX workflow hint - PivotTableCommands.CalculatedFields.cs: Routes to correct strategy - PivotTableAction.CreateCalculatedField: New enum value - ExcelPivotTableTool: MCP Server integration with formula parameter - PivotFieldResult: Added Formula property for result tracking TESTS: - 4 new regular PivotTable tests (multiplication, subtraction, complex, addition) - 1 new OLAP test using DataModelTestsFixture - Tests cover: formulas with different operators and parentheses - OLAP test verifies NotSupported behavior with proper error messages COMPATIBILITY: - All 41 existing PivotTable tests passing (no regressions) - Total: 46 passing tests (41 existing + 5 new) Closes Task 3 of #219 * feat(pivottable): Add layout and subtotals control (Task 4 of Issue #219) Implements Task 4 of Issue #219: SetLayout and SetSubtotals operations FEATURES: - Layout control: Compact (default), Tabular, Outline forms - Subtotals control: Show/hide automatic subtotals per field - Full support for both regular and OLAP PivotTables LAYOUT FORMS: - Compact (0): All row fields in single column with indentation - Tabular (1): Each field in separate column, subtotals at bottom - Outline (2): Each field in separate column, subtotals at top IMPLEMENTATION: - IPivotTableFieldStrategy: SetLayout and SetSubtotals interface methods - RegularPivotTableFieldStrategy: Uses RowAxisLayout() and field.Subtotals[1] - OlapPivotTableFieldStrategy: Full support for both operations - PivotTableCommands.Layout.cs: Routes to correct strategy - PivotTableCommands.Subtotals.cs: Routes to correct strategy - PivotTableAction: SetLayout and SetSubtotals enum values - ExcelPivotTableTool: MCP Server integration with layout and subtotalsVisible parameters TESTS: - 6 new tests covering all layout forms and subtotals scenarios - Tests for both regular and OLAP PivotTables - Round-trip verification tests COMPATIBILITY: - All 46 existing PivotTable tests passing (no regressions) - Total: 52 passing tests (46 existing + 6 new) CLOSES #219: All 4 tasks complete - Task 1: Date grouping (committed |
||
|
Copilot
|
42f01582bc |
feat: Support adding pre-existing DAX measures to OLAP PivotTables (#221)
* Initial plan * feat: Support adding pre-existing measures to OLAP PivotTables - AddValueField now detects if fieldName refers to existing measure - Supports measure formats: "Name", "[Measures].[Name]", or CubeField name - Mode 1: Add existing measure (new) - just set Orientation = xlDataField - Mode 2: Create measure from column (existing) - auto-create DAX measure - Added IsExistingMeasure() helper to check Data Model measures - Added 2 new tests for pre-existing measure scenarios Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * docs: Update documentation for OLAP add-value-field dual mode support - Updated IPivotTableCommands.cs XML comments - Updated excel_pivottable.md with OLAP modes explanation - Updated ExcelPivotTableTool.cs description attribute - Clarified difference between existing measure vs auto-create modes - Added workflow guidance for both scenarios Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * fix: Increase Excel COM cleanup timeout from 30s to 60s - Changed Join() timeout in ExcelBatch.Dispose() from 30 to 60 seconds - Updated log messages and exception text to reflect new timeout - Addresses intermittent test failures in SessionManagerTests Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> |
||
|
Stefan Broenner
|
7c51ebbcab |
feat: Full CRUD support for OLAP PivotTable value fields (#217) (#218)
* feat: Full CRUD support for OLAP PivotTable value fields (#217) Implements complete CRUD operations for OLAP PivotTable value fields: CREATE: - AddValueField auto-creates DAX measures with proper aggregation - Supports SUM, COUNT, AVERAGE, MIN, MAX functions - Handles both numeric and non-numeric columns UPDATE: - SetFieldFunction: Change aggregation function by modifying DAX formula - SetFieldFormat: Change number format via FormatInformation property * Currency formats ($#,##0.00) * Percentage formats (0.00%) * Decimal formats (#,##0.00) * Whole number formats (#,##0) READ: - ListFields: Returns all fields including value fields with measures - Field info includes measure details (formula, format) DELETE: - RemoveField: Deletes measures when removing value fields Key Implementation: - ParseDaxFormula/GenerateDaxFormula for formula manipulation - ModifyFormatObject for in-place format property modification - FormatInformation uses GET-AND-MODIFY pattern (not assignment) Tests: All 7 OLAP PivotTable tests passing (2m 34s) - 2 CREATE tests (SUM, COUNT aggregations) - 1 UPDATE function test - 1 UPDATE format test - 3 other field operation tests Closes #217 * style: Update padding and font sizes for improved layout in hero section and content area |
||
|
Stefan Broenner
|
a6672c8ffb |
Fix #215: Add helpful error message for invalid excel_file action (#216)
- Add specific catch block for ArgumentException containing 'action' - Return structured JSON with: - Clear error message listing valid actions - Array of valid action values - Workflow hint explaining correct save pattern - Update tool [Description] to warn about missing 'Save' action - Update excel_file.md prompt to document session lifecycle - Add regression test for error message clarity Fixes unhelpful 'An error occurred invoking excel_file' message when LLMs attempt to use invalid action values like 'Save' instead of the correct 'Close' with save=true parameter. |
||
|
Stefan Broenner
|
48c7b58ee7 |
Fix SessionManager disposal bug (issue #209 Phase 2 - 1 of 21) (#212)
* Update package versions and clean up unnecessary usings in tests * Fix disposal bug in SessionManager.CreateSession (issue #209 Phase 2) Implements try-finally with null sentinel pattern to ensure IExcelBatch is disposed on error paths. BEFORE: If TryAdd() threw exception, batch leaked. AFTER: Batch disposed in finally block unless successfully added to dictionary (ownership transfer via batch = null). Tests: All 25 SessionManager tests pass (3min 12sec). Relates to issue #209 - cs/dispose-not-called-on-throw alert. --------- Co-authored-by: Stefan Broenner <stefan.broenner@microsoft.comm> |
||
|
Stefan Broenner
|
9d9d10aa5c |
Remove Save action and add save parameter to Close (#207) (#208)
- Remove FileAction.Save from enum (breaking change) - Add save parameter to CloseSession (default: false) - Make Close action atomic: saves then closes when save=true - Update SessionManager.CloseSession to accept save parameter - Remove separate SaveSession method (no longer needed) - Update tool description to reflect explicit save requirement - Update all tests to use new CloseSession signature - Fix CLI SessionService to throw NotSupportedException for Save BREAKING CHANGE: LLMs must now explicitly pass save:true to persist changes. This prevents race conditions and makes the save operation atomic with close. Fixes #207 |
||
|
Stefan Broenner
|
11c10d1ce1 |
Refactor FileCommands.CreateEmpty to bypass batch API (#205) (#206)
- Replace ExcelSession.CreateNew() with direct COM automation - Add CreateNewWorkbookOnStaThread() for lightweight file creation - Remove session overhead and metadata comment injection - Improve performance by avoiding batch API for simple operations - Remove unused ExcelSession import Fixes #205 |
||
|
Stefan Broenner
|
fc9a78d7aa |
Add table style completions and presets for LLM discoverability (#203)
Fixes #202 ## Changes ### 1. Table Style Completions - Created table_styles.md with 60+ preset table styles - Added tableStyle parameter handler to ExcelCompletionHandler - Provides autocomplete for Light (1-21), Medium (1-28), Dark (1-11) styles ### 2. Enhanced Prompt Documentation - Expanded excel_table.md with style categories and naming patterns - Added popular style recommendations (Medium2, Medium9, Light9, Dark1) - Documented style behavior and empty string behavior ### 3. TableStylePresets Class - Created TableStylePresets.cs mirroring NumberFormatPresets pattern - Documented 23+ common table styles as constants with descriptions - Provides IntelliSense for developers ### 4. Updated Tests - Updated test files to use TableStylePresets.Medium2 constant - Added CompletionHandlerTests for autocomplete verification - All 22 tests pass (18 table + 4 completion tests) ## Test Results - Build: 0 warnings, 0 errors - Feature=Tables: 18 tests passed - CompletionHandler: 4 tests passed ## Benefits - LLMs get autocomplete suggestions for table styles - Consistent with existing NumberFormatPresets/PivotTableStylePresets - Better developer experience (IntelliSense) - Complete documentation for table styling |
||
|
Stefan Broenner
|
809f2555ee |
Fix: Use 1-based arrays for Excel COM range operations (#200)
Fixes #199 ## Root Cause SetFormulas and SetValues were creating 0-based C# arrays, but Excel COM requires 1-based arrays for multi-cell range assignments. This caused E_OUTOFMEMORY (0x8007000E) errors with misleading 'out of memory' message. ## Changes - RangeCommands.SetFormulas: Use Array.CreateInstance with [1,1] lower bounds - RangeCommands.SetValues: Use Array.CreateInstance with [1,1] lower bounds - Added regression test: SetFormulas_WideHorizontalRange_NoOutOfMemoryError - Added regression test: SetValues_WideHorizontalRange_NoOutOfMemoryError ## Test Results All 62 Range tests pass (60 existing + 2 new regression tests) ## Impact - Before: Multi-cell range operations failed with cryptic error - After: All range operations work correctly - Backwards compatible: All existing tests pass |
||
|
Stefan Broenner
|
0eb11c04e9 |
Remove redundant QueryTable and ListExcelSources features (#197)
* Update README to streamline API safety section and enhance clarity * Update robots.txt to enhance sitemap visibility and prioritize crawling for Googlebot and Bingbot * Remove obsolete PowerQuery integration tests for LoadTo and timeout scenarios * Refactor using directives across multiple command files for consistency * Remove redundant ListExcelSources and Unload methods (#133) Removed ListExcelSources method from PowerQuery commands as it duplicates functionality already available via separate excel_table(list) and excel_namedrange(list) tool calls. LLMs can easily chain these operations. Also removed Unload method as the interface signature was removed during earlier refactoring but enum/tool routing remained. **Changes:** - Removed IPowerQueryCommands.ListExcelSources() interface method - Removed PowerQueryCommands.ListExcelSources() implementation (~95 lines) - Removed PowerQueryAction.ListExcelSources enum value - Removed action mapping for 'list-excel-sources' from ActionExtensions - Removed MCP Server tool routing for ListExcelSources (~15 lines) - Removed PowerQueryAction.Unload enum value - Removed action mapping for 'unload' from ActionExtensions - Removed MCP Server tool routing for Unload (~20 lines) - Converted 10 PowerQuery tests to inline M code (removed .pq file dependencies) **Total cleanup:** ~140 lines removed **Related to #133:** Code cleanup discovered both bugs already fixed: - Bug #1 (Column Preservation): Tests pass - Update handles column changes - Bug #2 (ListObject Accumulation): Delete() removes ListObjects properly - All regression tests passing (2/2) * Remove redundant QueryTable functionality QueryTable functionality duplicates existing Power Query + Table + Connection tool capabilities. LLMs can compose workflows from existing tools without needing a separate QueryTable abstraction. **Redundancy Analysis:** - CreateFromQuery → excel_powerquery(action: 'create', loadDestination: 'worksheet') - CreateFromConnection → excel_connection(action: 'loadto') - List → excel_table(list) + excel_powerquery(list) - Refresh → excel_powerquery(refresh) or excel_connection(refresh) - Delete → excel_powerquery(delete) or excel_table(delete) - UpdateProperties → Rarely needed, can be set during creation **Files Deleted:** - src/ExcelMcp.Core/Commands/QueryTable/ directory (IQueryTableCommands.cs, QueryTableCommands.cs, QueryTableCommands.Lifecycle.cs) - src/ExcelMcp.Core/Models/QueryTableTypes.cs - src/ExcelMcp.CLI/Commands/QueryTable/ directory - src/ExcelMcp.McpServer/Tools/ExcelQueryTableTool.cs - tests/ExcelMcp.Core.Tests/Integration/Commands/QueryTable/ directory (4 test files) - src/ExcelMcp.McpServer/Prompts/Content/excel_querytable.md - specs/QUERYTABLE-SUPPORT-SPECIFICATION.md **Code Changes:** - Removed QueryTableAction enum from ToolActions.cs - Removed ToActionString mapping from ActionExtensions.cs - Removed CLI command registrations from Program.cs - Updated tool count: 12 → 11 in all README files - Updated action count: 163 → 155 in all README files **Total cleanup:** ~800 lines removed (Core: ~400, CLI: ~150, MCP: ~100, Tests: ~150) **Same rationale as ListExcelSources removal:** LLMs don't need convenience wrappers - they can chain operations from focused, single-purpose tools. * Update documentation to remove QueryTable references - GitHub Pages: Update tool count (12→11) and action count (163→155) - Copilot instructions: Remove QueryTable from architecture patterns - MCP prompts: Remove QueryTable guidance from tool selection, user patterns, server quirks - MCP prompts: Update excel_connection and excel_powerquery to remove QueryTable references - Specs: Remove QueryTableTool from session redesign tracking - ComInterop: Remove unused FindQueryTable() helper (~70 lines) - Delete ExcelQueryTablePrompts.cs (no longer needed) - VS Code extension: Update dev dependencies (TypeScript types, eslint) |
||
|
Stefan Broenner
|
14fbfe2247 |
Fix MCP server crashes and CreateEmpty workflow failures (#192) (#193)
Three critical fixes: 1. COM Cleanup Hardening (ExcelBatch.cs) - Wrap workbook.Close() and excel.Quit() in try/catch blocks - Catch COMException and MissingMemberException - Log warnings instead of crashing server - Fixes: RPC_E_DISCONNECTED (0x80010108) unhandled exceptions 2. Add Error Messages to Worksheet Tool (ExcelWorksheetTool.cs) - List action now includes errorMessage in JSON response - Create action now includes errorMessage in JSON response - LLMs can now diagnose failures properly 3. Fix CreateEmpty Save (FileCommands.cs) - Call ctx.Book.Save() before batch disposal - Ensures Sheet1 rename and comment are persisted - Fixes: corrupted workbook state after CreateEmpty Tests Added (FileCommandsTests.CreateEmptyThenList.cs): - CreateEmpty_ThenOpenAndList_ReturnsSheet1 - CreateEmpty_ThenOpenAndListMultipleTimes_ConsistentResults - CreateEmpty_ThenOpenAndCreateMoreSheets_AllSheetsVisible All 15 file-related tests pass. Fixes #192 Co-authored-by: Stefan Broenner <stefan.broenner@microsoft.comm> |
||
|
Stefan Broenner
|
1e996bea6d |
Remove file-based operations and refactor VBA to accept code strings (#191)
* Remove file-based operations and refactor VBA to accept code strings Fixes #190 Changes: - Removed Connection.Import and Connection.UpdateProperties (JSON file-based operations) - Removed DataModel.ExportMeasure (DAX file export) - Refactored VBA Import/Update to accept code strings instead of file paths - Core layer now accepts vbaCode string parameter - CLI layer handles file I/O (reads files and passes code strings) - MCP Server layer accepts vbaCode parameter directly - Deleted PathValidator.cs and removed Security namespace references - Updated all documentation (READMEs, GitHub Pages) to reflect 163 operations (down from 168) Build status: ✅ 0 warnings, 0 errors Breaking changes: - VBA Import/Update signature changed in Core layer (file path → code string) - Connection and DataModel commands removed (file-based operations) * Fix VBA tests to pass code strings instead of file paths The VBA command signature changed to accept code strings directly rather than file paths. Updated all VBA trust tests to pass vbaCode strings instead of creating temporary files and passing file paths. Tests fixed: - ScriptCommands_Import_WithTrustEnabled_WorksCorrectly - ScriptCommands_Export_WithTrustEnabled_WorksCorrectly - ScriptCommands_Run_WithTrustEnabled_WorksCorrectly - ScriptCommands_Delete_WithTrustEnabled_WorksCorrectly - ScriptCommands_View_WithTrustEnabled_WorksCorrectly - ScriptCommands_Update_WithTrustEnabled_WorksCorrectly All 7 VBA tests now pass. |
||
|
Stefan Broenner
|
df928e95f4 |
Refactor (#188)
* feat: align CLI with MCP Server session API pattern MAJOR CHANGE: CLI now uses MCP-aligned session terminology - Old: batch-begin, batch-commit, batch-list - New: open, save, list (primary commands) - Legacy batch commands still work but map to new session commands IMPLEMENTATION: ✓ BatchCommands: Renamed Begin() → Open(), Commit() → Save() ✓ Program.cs: Updated routing for new command names ✓ Program.cs: Added backward compatibility for old batch commands ✓ Program.cs: Updated help text to show session lifecycle ✓ Program.cs: Banner now says 'MCP-aligned Session API' ✓ CommandHelper: Updated --batch-id → --session-id parameter detection ✓ Error messages: Updated terminology from 'batch' to 'session' ✓ All file operations: Updated comments and output messages BACKWARD COMPATIBILITY: ✓ Old batch-begin/batch-commit/batch-list commands still work ✓ --batch-id flag still accepted (mapped to --session-id internally) ✓ Both terminologies coexist during transition TESTING: ✓ CLI builds: 0 warnings, 0 errors ✓ MCP smoke test passed ✓ Help text displays new session commands and deprecation notice DOCUMENTATION: ✓ CLI README already updated in previous commit with session API examples ✓ Help output now clearly shows primary commands and deprecation path This aligns the CLI completely with the MCP Server's session-based API, making both interfaces consistent and simplifying user experience. * feat: split save/close commands - save keeps session open, close discards changes * refactor: remove backward compatibility layer - legacy GetBatch method and usage * fix: improve Range command error messages - use RangeHelpers specific errors - All Range operations now use RangeHelpers.ResolveRange() with specificError out parameter - Instead of cryptic '0x800A03EC' COM error, users now get helpful messages like: * 'Sheet \\'Validation_Summary\\' not found. Available sheets: Sheet1, Sheet2' * 'Named range \\'Sales\\' not found. Available named ranges: Parameters, Region' - Includes suggestions to use excel_worksheet or excel_namedrange tools - Affects: Get/Set Values/Formulas, Copy, Clear, Insert, Delete, Find, Replace, Sort, Hyperlinks, NumberFormat operations * fix: simplify COM error messages - remove LLM guidance Removed verbose step-by-step guidance from error messages. LLMs don't need to be told what to do, just what failed. Changed from: 'This usually means: (1) Sheet doesn't exist, (2) Range invalid, or (3) Session not open. Use excel_worksheet(action: list) to verify...' To: 'Cannot read range X on sheet Y: Insufficient memory to continue the execution of the program' Files changed: - RangeCommands.Values.cs: Simplified GetValuesAsync and SetValuesAsync COM error messages - RangeCommands.Formulas.cs: Simplified GetFormulasAsync and SetFormulasAsync COM error messages - ExcelToolsBase.cs: Simplified session validation error - just state facts, no instructions * docs: add error message style guidance - facts not instructions Added Best Practice #9 and Error Message Style section to mcp-server-guide. Key principle: Error messages should state facts (what failed, why), not prescribe solutions. LLMs are intelligent agents that determine next steps - they don't need step-by-step guidance. Example: ❌ Wrong: 'This usually means: (1) Sheet doesn't exist... Use excel_worksheet(list)...' ✅ Correct: 'Cannot read range X on sheet Y: Insufficient memory' This matches the simplifications in commit |
||
|
Stefan Broenner
|
efe6d6a735 |
Feature/session-api-redesign-spec (#185)
* Spec: redesign session API for open/save/close model * docs: add implementation notes for session API redesign and lifecycle management * feat: implement session API foundation - Add SessionManager in Core layer (shared by MCP Server and CLI) - Add WithSessionAsync helper method in ExcelToolsBase - Implement Open/Save/Close actions in excel_file tool - Mark WithBatchAsync as obsolete - Document migration pattern for remaining tools Core and ComInterop layers unchanged (IExcelBatch, batchId preserved) MCP Server exposes sessionId to users/LLMs * Fix double disposal race condition and Excel process leaks in SessionManager CRITICAL FIXES: - Fix double disposal race condition using Interlocked.CompareExchange - Fix Excel process leaks on timeout with force-kill fallback - Fix two-session disposal hang with sequential disposal + timeouts - Migrate to ILogger pattern for proper test diagnostics DISPOSAL IMPROVEMENTS: - ExcelBatch: Atomic disposal check prevents duplicate disposal - SessionManager: Sequential disposal with 10s timeout per session - Excel.Quit() timeout wrapper with Task.Run + CancellationToken - Force-kill Excel processes on timeout (Process.GetProcessesByName) LOGGER MIGRATION: - Replace Debug.WriteLine/Console.WriteLine with ILogger<ExcelBatch> - Comprehensive thread ID tracking throughout disposal flow - Proper dependency injection pattern for testability TEST IMPROVEMENTS: - DisposalVerificationTest: Verify Interlocked fix prevents double disposal - SessionManagerTests: Comprehensive lifecycle and multi-session tests - All tests pass reliably in ~18s (was timing out at 30s) - Zero Excel process leaks verified ARCHITECTURE: - Move SessionManager from Core to ComInterop (correct layer) - Remove batch-of-one pattern, use session API exclusively - Update all MCP tools to require sessionId parameter - Better than NetOffice: they don't handle COM deadlocks at all AUDIT SCRIPTS: - Add FileAction session operations to knownExceptions (Open, Save, Close) - These are MCP-specific session management actions, not Core methods - Update both audit-core-coverage.ps1 and check-mcp-core-implementations.ps1 Fixes #139 (double disposal bug) Fixes #140 (Excel process leaks) Fixes #141 (two-session hang) * test: re-enable comprehensive smoke test for MCP tools with session API * refactor: eliminate redundant guidance, remove obsolete batch mode, preserve critical hints CHANGES: - Removed 70% of redundant prompts, next-step suggestions, and elicitations - Eliminated ALL batch mode references (API redesigned to Open/Save/Close sessions) - Preserved only 7 critical server-specific hints (hidden sheets, #NAME? errors, connection-only detection, etc.) DELETED FILES (12 total): - ExcelBatchModePrompts.cs (obsolete batch mode guidance) - excel_batch.md (obsolete tool prompt) - 6 elicitation files (batch_workflow, dax_measure, powerquery_import, querytable_creation, range_formatting, table_creation) - 5 redundant completion files (alignment, colors, error styles, format codes) CLEANED TOOL FILES (11 total, 130+ methods): - ExcelPowerQueryTool.cs - Retained 5 critical hints (Issue #170, Unload, LoadConfig, Delete side effects, Refresh timeout) - ExcelRangeTool.cs - Removed 227 lines of guidance, retained core patterns - ExcelTableTool.cs - Removed 126 lines of guidance, retained core patterns - 8 remaining tool files - Cleaned batch mode references, removed generic guidance CLEANED PROMPT FILES (12 total): - Removed batch mode references from all 8 affected tool prompts - Removed deleted file references from tool prompts and loaders - Condensed tool_selection_guide.md by 80% (250→50 lines) - Rewrote server_quirks.md and user_request_patterns.md (removed batch sections) FIXED LOADERS: - ExcelElicitationPrompts.cs - Removed 5 methods for deleted elicitations - ExcelCompletionHandler.cs - Removed references to 5 deleted completion files - ExcelScenarioPrompts.cs - Replaced all 5 batch workflows with current Open/Save/Close patterns QUALITY METRICS: ✓ Build: 0 warnings, 0 errors ✓ All tool files clean (verified with grep) ✓ No broken references remain ✓ All guidance now server-specific only ✓ Ready for merge to main * docs: update all documentation with correct tool/operation counts and session lifecycle CHANGES ACROSS ALL DOCUMENTATION: - Updated tool count: 12 → 11 specialized tools (batch mode removed) - Updated operation count: 165 → 166 operations (new open/save/close actions) - Replaced obsolete batch mode documentation with current session lifecycle pattern FILES UPDATED: - README.md (main project) - src/ExcelMcp.McpServer/README.md (MCP Server NuGet) - src/ExcelMcp.CLI/README.md (CLI tool documentation) - gh-pages/index.md (GitHub Pages landing) - vscode-extension/README.md (VS Code extension) KEY CHANGES: ✓ CLI: Removed batch mode (batch-begin/batch-commit) - 3 obsolete commands ✓ CLI: Added session lifecycle (open/save/close) - 3 new core operations ✓ CLI: Updated command count from 150 to 166 operations ✓ CLI: Restructured category table from 13 to 11 categories ✓ All tools: Updated to reflect 11 tools, 166 operations consistently ✓ Examples: Updated batch mode examples to show open/save/close pattern VERIFICATION: ✓ All counts match live codebase (audit script verified: 160 Core + 6 MCP-specific) ✓ Session lifecycle accurately reflects current API design ✓ Documentation consistent across all 5 locations |
||
|
Copilot
|
c0474b350d |
Add comprehensive MCP client discovery support for Claude, Cursor, Cline, and Windsurf (#175)
* Initial plan * Add comprehensive MCP client discovery support - Add example configs for Claude, Cursor, Cline, Windsurf, VS Code - Update README with MCP Registry badge and multi-client setup - Expand INSTALLATION.md with detailed per-client instructions - Create examples/mcp-configs/ with ready-to-use config files - Add setup guide covering all major MCP clients Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> |
||
|
Stefan Broenner
|
85ca40073b |
Fix Issue #170: LoadTo detects existing sheets and returns clear error (#171)
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. |
||
|
Stefan Broenner
|
c9643b4979 |
Fix #167: Excel validation InputTitle/InputMessage not returned + dropdown list guidance (#168)
Root Cause: - Excel COM API requires validation.ShowInput=true BEFORE setting InputTitle/InputMessage - Otherwise Excel silently ignores these properties Fix Applied: - Reordered property assignments in ValidateRangeAsync (lines 67-72) - Refactored GetValidationAsync to read properties into variables first (defensive) Critical Discovery - List Validation Dropdowns: - Comma-separated strings do NOT create dropdown arrows in Excel - Only worksheet range references create actual dropdowns - Updated documentation to guide users: write values to range first, then reference it Documentation Updates: - ExcelRangeTool.cs: Updated formula1 parameter description for list validation - data_validation.md: Added step-by-step dropdown workflow with warning - excel_range.md: Added to common mistakes list Tests Added: - ValidateRange_WithInputMessage_ReturnsSuccess: End-to-end validation workflow - GetValidation_WithInputMessage_ReturnsInputTitleAndMessage: InputTitle/InputMessage verification - Both tests use proper dropdown syntax (worksheet range reference) - Tests pass (15s total, 0 warnings) User Impact: - InputTitle and InputMessage now work correctly - Clear guidance on creating proper dropdown lists - Prevents confusion about comma-separated vs range-based validation |
||
|
Stefan Broenner
|
8665b97ee3 |
fix: VS Code extension build - move XML cleanup to after Publish (#166)
Fixes MSB3030 error in GitHub Actions by changing MSBuild target timing from AfterTargets="Build" to AfterTargets="Publish". XML files are now deleted after publish completes instead of before, preventing "file not found" errors during the publish copy phase. |
||
|
Stefan Broenner
|
8a80ef13f5 |
refactor: Complete Strategy Pattern implementation with test organization alignment
Complete Strategy Pattern implementation for PivotTable field operations, aligning code architecture with test organization and adding critical build optimizations. Changes: - Implemented IPivotTableFieldStrategy interface with 11 field operation methods - Created RegularPivotTableFieldStrategy for PivotFields API (825 lines) - Created OlapPivotTableFieldStrategy for CubeFields API with CreatePivotFields() fix - Added PivotTableFieldStrategyFactory for runtime strategy selection - Refactored PivotTableCommands from 962 → 779 lines (-183 lines) - Added Category trait to 9 Regular tests and 3 OLAP tests - Added Release build optimization configuration to 4 projects - Updated documentation with NetOffice library reference - Pre-commit validation: All 30 tests passing, 0 COM leaks, MCP smoke test passed Files changed: 19 files (+2,055, -759) Tests: 30/30 passing (6m 26s) Build: 0 warnings, 0 errors |
||
|
Stefan Broenner
|
7e1db06b0a |
Fix catastrophic OLAP PivotTable COM lifecycle bug (#162)
* Fix PivotTable operations: defensive error handling + OLAP CubeFields support
PROBLEM:
1. PivotTable List crashing with 0x800A03EC on real workbooks when accessing properties
2. Data Model (OLAP) PivotTables returning 0 fields - CATASTROPHIC BUG
ROOT CAUSE:
1. No defensive error handling when accessing PivotTable properties that may fail
2. Excel uses completely different API for OLAP PivotTables:
- Regular PivotTables: use PivotFields collection
- Data Model/OLAP PivotTables: use CubeFields collection
- Per Microsoft docs: 'In OLAP PivotTables, PivotFields do not exist until
the corresponding CubeField is added to the PivotTable'
SOLUTION:
1. Defensive Error Handling (PivotTableCommands.Lifecycle.cs):
- Wrapped TableRange2, SourceData, field counts, RefreshDate in try-catch
- Returns safe defaults on COMException instead of crashing
- Improved error messages with workflow hints
2. OLAP/CubeFields Support (PivotTableCommands.Fields.cs):
- Refactored ListFieldsAsync to detect OLAP vs regular PivotTables
- Check pivot.CubeFields.Count > 0 to determine OLAP type
- New ListCubeFieldsAsync: iterates CubeFields for OLAP PivotTables
- New ListRegularFieldsAsync: iterates PivotFields for regular PivotTables
- OLAP field names include hierarchy: '[TableName].[FieldName]'
3. MCP Server (ExcelPivotTableTool.cs):
- Updated error messages to suggest common causes
- Improved workflow hints for troubleshooting
4. SetValuesAsync Documentation (IRangeCommands.cs, RANGE-API-SPECIFICATION.md):
- Added CRITICAL warning about range address format
- Must specify full range like 'A1:D11', not just 'A1'
- Excel COM doesn't reliably auto-expand from single cell addresses
TEST COVERAGE:
- PivotTableRealisticFixture: Creates workbook with 3 PivotTable types
* Range-based PivotTable (standard)
* Table-based PivotTable (standard)
* Data Model PivotTable (OLAP/CubeFields)
- 8 comprehensive tests in PivotTableCommandsTests.Realistic.cs
- ALL 8 TESTS PASSING (100% success rate)
BACKWARDS COMPATIBILITY:
- Fully backwards compatible
- Regular PivotTable behavior unchanged
- OLAP PivotTables now work correctly (were broken before)
DISCOVERY PROCESS:
- Microsoft Learn docs revealed CubeFields requirement
- Stack Overflow searches found no relevant examples
- mcp_microsoft_doc search provided key insight about OLAP architecture
- Excel's dual PivotTable architecture not well documented
FILES CHANGED:
- src/ExcelMcp.Core/Commands/PivotTable/PivotTableCommands.Lifecycle.cs
- src/ExcelMcp.Core/Commands/PivotTable/PivotTableCommands.Fields.cs
- src/ExcelMcp.Core/Commands/Range/IRangeCommands.cs
- src/ExcelMcp.McpServer/Tools/ExcelPivotTableTool.cs
- specs/RANGE-API-SPECIFICATION.md
- tests/ExcelMcp.Core.Tests/Helpers/PivotTableRealisticFixture.cs
- tests/ExcelMcp.Core.Tests/Commands/PivotTable/PivotTableCommandsTests.Realistic.cs
* Fix PivotTable OLAP/Data Model support - field operations now work
CRITICAL FIXES:
1. GetAsync: Now detects OLAP and routes to CubeFields vs PivotFields
2. GetFieldForManipulation(): New helper handles OLAP field access
3. Add/Remove/SetName methods: Now work with Data Model PivotTables
FIXED METHODS:
- GetAsync (Lifecycle.cs)
- AddRowFieldAsync, AddColumnFieldAsync, AddValueFieldAsync, AddFilterFieldAsync
- RemoveFieldAsync, SetFieldNameAsync
TODO:
- SetFieldFunctionAsync, SetFieldFormatAsync (use DataFields)
- Comprehensive OLAP field operation tests
ALL TESTS PASSING (8/8)
* Fix SetFieldFunction and SetFieldFormat for OLAP support
Both methods use DataFields collection which works for OLAP, but had
fallback code using pivot.PivotFields.Item() for error reporting.
Changed to use GetFieldForManipulation() helper for OLAP-aware access.
Also fixed data type detection in SetFieldFunctionAsync to skip
DetectFieldDataType() for OLAP (returns 'Cube' instead).
ALL TESTS PASSING (8/8)
* Fix SetFieldFilter and SortField for OLAP support
Both methods had direct pivot.PivotFields.Item() calls that failed
for OLAP PivotTables.
Changed to use GetFieldForManipulation() helper.
SetFieldFilterAsync: Now filters OLAP fields via CubeField access
SortFieldAsync: Now sorts OLAP fields via CubeField access
ALL 10 FIELD MANIPULATION METHODS NOW OLAP-AWARE
ALL TESTS PASSING (8/8)
* Fix OLAP PivotTable field operations - add COM lifecycle handling
- Fixed GetFieldForManipulation() COM lifecycle bug: Don't release cubeField when returning dependent pivotField
- Updated RemoveFieldAsync() to handle OLAP-specific removal logic (CubeField vs PivotField)
- Added 11 comprehensive OLAP field operation tests (1 passing - SortField proves fix works)
- Tests reveal RemoveField needs additional investigation for complete OLAP support
The core OLAP architecture fix is complete and working. SortField test passing validates that:
- GetFieldForManipulation correctly retrieves fields from Data Model PivotTables
- Field manipulation operations work on OLAP PivotFields
- COM lifecycle is properly managed (no RCW separation errors)
Remaining work: Refine RemoveField OLAP implementation and test cleanup logic.
* Refactor PivotTable tests: eliminate duplicates and consolidate structure
- Deleted duplicate PivotTableCommandsTests.Realistic.cs from Commands/PivotTable/
- Moved PivotTableCommandsTests.OlapFields.cs to Integration/Commands/PivotTable/
- Removed Collection attribute (no longer needed - tests run independently)
- Simplified test names: DataModelPivotTable_* -> OlapPivot_* (clearer, shorter)
- Removed redundant assertions (kept essential binary assertions only)
- Removed empty Commands/PivotTable/ directory
Test organization now follows established pattern:
- All PivotTable tests in Integration/Commands/PivotTable/
- OlapFields tests standalone (not partial class - uses different fixture)
- Clear naming: AddRowField_OlapPivot_AddsFieldToRows
- Clean hierarchy: Integration -> Commands -> PivotTable
Build: 0 warnings, 0 errors
Tests: SortField_OlapPivot_SortsFieldSuccessfully PASSES (validates refactoring)
---------
Co-authored-by: Stefan Broenner <stefan.broenner@microsoft.comm>
|
||
|
Copilot
|
85ee61f9de |
Fix PivotTable tool discovery: Remove partial keyword blocking MCP SDK reflection (#158)
* Initial plan * Fix: Remove partial keyword from ExcelPivotTableTool to enable tool discovery Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Add runtime assembly scanning test to catch tool discovery issues Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Update documentation: 11 tools → 12 tools (include PivotTable) Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Update all documentation references: 11 tools → 12 tools Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> |
||
|
Copilot
|
a66561b092 | Fix: Add missing MCP SDK discovery attributes to PivotTable tool (#156) | ||
|
Stefan Broenner
|
8d23bce14c |
SEO: Complete optimization + GitHub Pages reorganization (#154)
* docs: add GitHub Pages landing page links to distribution channels Strategic placement for SEO backlinks: - ✅ NuGet package READMEs (MCP Server + CLI): Subtle 'Learn more' links - ✅ VS Code Marketplace README: Link after Quick Start section - ❌ Main repo README: No link (comprehensive docs, not landing page) Rationale: - GitHub Pages = landing page for discovery/first impressions - Distribution channels = authoritative backlinks for SEO - Main README = comprehensive documentation for developers * docs: reorganize GitHub Pages to gh-pages/ and add Jekyll plugins Reorganization: - ✅ Moved all GitHub Pages files from docs/ to gh-pages/ - ✅ docs/ now clean - only contains project documentation - ✅ gh-pages/ contains: _layouts/, assets/, _config.yml, Gemfile, index.md, robots.txt, favicon.ico Jekyll SEO improvements: - ✅ _config.yml: Enable jekyll-sitemap and jekyll-seo-tag plugins - ✅ Gemfile: Dependencies for local development (github-pages gem) - ✅ default.html: Add {% seo %} tag for automatic SEO optimization - ✅ Removed manual sitemap.xml (Jekyll auto-generates it) README updates (strategic placement for SEO backlinks): - ✅ NuGet packages (MCP Server + CLI): Subtle 'Learn more' links - ✅ VS Code Marketplace: Link after Quick Start section - ❌ Main README: No link (comprehensive docs, not landing page) Benefits: - Clean separation: docs/ = project docs, gh-pages/ = website - Sitemap automatically generated and kept up-to-date by Jekyll - SEO meta tags automatically optimized by jekyll-seo-tag - Better search engine indexing and discoverability Note: GitHub Pages settings need update to use /gh-pages as source directory * seo: complete SEO optimization for 100% coverage Front matter & metadata: - ✅ Unique title, description, keywords per page - ✅ Canonical URL support for duplicate content prevention - ✅ Dynamic meta tags from Jekyll front matter Performance optimizations: - ✅ Preconnect hints for external resources - ✅ Preload critical CSS and images - ✅ Mobile-first responsive design (existing) - ✅ Smooth scrolling for better UX Internal linking: - ✅ Quick navigation menu with anchor links - ✅ Cross-references between sections - ✅ Internal links from use cases to examples/installation Heading structure: - ✅ Proper H1-H6 hierarchy verified - ✅ H1 in layout (page title) - ✅ H2 for main sections - ✅ H3 for subsections SEO Coverage: 100% Complete - [x] Image alt text - [x] Structured data (JSON-LD) - [x] Social media meta tags - [x] Robots.txt - [x] Auto-generated sitemap (Jekyll) - [x] Link descriptions - [x] Jekyll SEO plugin - [x] Unique page titles - [x] Unique meta descriptions - [x] Heading structure - [x] Mobile optimization - [x] Page load speed (preload/preconnect) - [x] Internal linking - [x] Canonical URLs - [x] HTTPS (GitHub Pages default) --------- Co-authored-by: Stefan Broenner <stefan.broenner@microsoft.comm> |
||
|
Copilot
|
b156c6af2d |
Add batch mode to CLI for RPA workflows (75-90% faster multi-operation execution) (#153)
* Initial plan * Add basic batch commands to CLI (begin, commit, list) Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Add batch-id parameter support to CLI commands with CommandHelper Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Add batch mode documentation to CLI README Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Add batch mode example scripts and documentation Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> |
||
|
Stefan Broenner
|
321aa694f4 |
feat: Improve SEO and add comprehensive installation guide (#144)
* Refactor README files: Simplify language in ExcelMcp.ComInterop and remove ExcelMcp.Core documentation * chore: Remove references to COMMANDS.md in documentation and workflows, update to README.md or component documentation * chore: Update documentation references to CLI and remove obsolete CLI.md file * feat(cli): implement 38 missing command wrappers and sync online help Implementation (38 commands): - Range: Add 33 commands (copy-values/formulas, insert/delete cells/rows/columns, find/replace/sort, discovery, hyperlinks, styles/validation, merge/autofit, advanced) - Table: Add 2 commands (get/set-column-format) - QueryTable: Add 3 commands (create-from-connection/query, update-properties) - PivotTable: Document 7 blocked methods with technical context Routing & Help: - Add 38 routing entries in Program.cs switch statement - Update ShowHelp() function with all 38 commands: * Range: Reorganize into 8 subsections for clarity * Table: Add 2 column format commands * QueryTable: Add 3 create/update commands - Grow Program.cs from 554 to 608 lines (+9.7%) Documentation: - Update CLI README: 109 → 147 commands - Update category counts: Range (11→44), Table (21→23), QueryTable (5→8) - Update MCP Server README: CLI reference now shows 147 commands - Document PivotTable limitations in code comments Status: 38/45 commands complete (84.4%) - Implemented: 38 (Range: 33, Table: 2, QueryTable: 3) - Blocked: 7 (PivotTable methods awaiting file manipulation support) Fixes: - Correct property names in RangeCommands (Address vs Url, ScreenTip vs Tooltip) - Fix hyperlink removal logic (target address calculation) - Replace ToLowerInvariant() with StringComparison.OrdinalIgnoreCase for CA1862 - Ensure consistent formatting throughout Zero compilation errors, all tests passing. * refactor(release): unify MCP Server and CLI releases, optimize copilot instructions Release Workflow Changes: - Rename release-mcp-server.yml to handle both MCP Server and CLI - Change tag pattern: mcp-v* → v* (unified versioning) - MCP Server and CLI always release together with same version - Single workflow builds and publishes both NuGet packages - GitHub release includes both ZIP packages - Delete obsolete workflows: * release-cli.yml (merged into unified workflow) * release-core.yml (no longer publishing Core to NuGet) * release-cominterop.yml (no longer publishing ComInterop to NuGet) Copilot Instructions Optimization: - copilot-instructions.md: 184→132 lines (-28%) * Replace verbose navigation with priority-based structure * Consolidate code patterns (remove redundant comments) * Compress 28 lesson paragraphs → 7 one-line takeaways * Simplify path-specific explanation - critical-rules.instructions.md: Add usage-based priority table * Group rules by context: Every Edit, Writing Code, Writing Tests, Before Commit, PR Process, Rare * Replace flat list with contextual grouping for 10x faster scanning * Add 'Why Critical' column for each rule Benefits: - Simplified release process: One tag (v1.2.3) releases both packages - Version consistency: MCP Server and CLI stay in sync - Reduced CI/CD complexity: 3 fewer workflows to maintain - Better LLM efficiency: 28% less token usage in main instructions - Faster rule lookup: O(1) with priority table vs linear scan * docs: update release documentation for unified v* versioning - Updated RELEASE-STRATEGY.md: Removed separate Core/ComInterop/MCP/CLI sections, added unified MCP+CLI release section - Updated NUGET-GUIDE.md: Changed to unified v* tag pattern, combined package documentation - Updated MCP_REGISTRY_PUBLISHING.md: Changed mcp-v* to v* tag pattern - All docs now reflect simplified release strategy (MCP Server + CLI always released together) * docs: update development-workflow instructions for unified versioning - Changed tag pattern from separate mcp-v*/cli-v* to unified v* - Reflects simplified release strategy (MCP Server + CLI together) * Remove redundant CheckIfOpen action FileAction.CheckIfOpen was redundant because ExcelBatch already checks if files are locked before opening Excel COM (via FileAccessValidator. ValidateFileNotLocked). Operations fail gracefully with clear error messages when files are open, eliminating need for pre-flight checks. Changes: - Remove CheckIfOpen from FileAction enum - Remove CheckIfOpen implementation (75 lines) - Remove CheckIfOpen from ActionExtensions mapping - Update audit scripts to remove CheckIfOpen exceptions - Update excel_file.md prompt to document automatic lock detection - Update ExcelFileTool [Description] to remove CheckIfOpen guidance Benefits: - Simpler architecture with less mental load for LLMs - One less action to learn and maintain - Automatic error handling with actionable user guidance - Consistent with batch mechanism that already handles this Fixes: MCP Server test failures (missing enum mapping) * Optimize instruction files for expert LLM audience - Eliminate duplication across all 13 instruction files - Remove beginner content and explanatory 'why' sections - Establish expert tone throughout (assumes C#, Excel, COM knowledge) - Consolidate test commands to testing-strategy (single source of truth) - Cross-reference instead of duplicating content (WithExcel, SaveAsync, workflows) - Major rewrites: bug-fixing (416→82), documentation-structure (355→86), mcp-llm-guidance (414→152), mcp-server-guide (282→192) - Result: Most files now under 200 lines, focused on project-specific patterns only * Update MCP Server README: emphasize RPA/CLI capabilities and simplify installation - Add RPA (Robotic Process Automation) terminology for CLI automation use cases - Remove specific command counts that change frequently - Simplify installation section to prioritize VS Code Extension - Add CLI example use case for version control workflows - Update resources section with RPA/CI/CD focus * feat: improve SEO with GitHub Pages docs and enhanced server.json description * feat: Improve SEO and add comprehensive installation guide - Add GitHub repository topics (20 relevant tags for discoverability) - Update GitHub repository description for better search results - Enable GitHub Pages with docs/index.md landing page - Enhance MCP Registry server.json description - Improve NuGet package metadata with comprehensive tags - Add REPL developer callout and interactive development messaging - Create comprehensive INSTALLATION.md guide - Fix VS Code marketplace extension ID (sbroenne.excel-mcp) - Add Critical Rule 20: Never commit automatically - Update README and docs to mention both MCP Server and CLI prominently --------- Co-authored-by: Stefan Broenner <stefan.broenner@microsoft.comm> |
||
|
Stefan Broenner
|
7a123732e8 |
Add LLM guidance for PowerQuery Create vs Update action selection (#142)
Fixes issue where LLMs incorrectly used Create action on existing queries instead of Update. - Added Create_DuplicateQueryName_ReturnsError test to verify Create properly rejects duplicates - Updated excel_powerquery.md with 'When to use create vs update' and 'Common mistakes' sections - Updated ExcelPowerQueryTool.cs description with prominent 'WHEN TO USE CREATE vs UPDATE' guidance - Updated powerquery_import.md elicitation to include query existence check as REQUIRED step Create action correctly fails with 'Query X already exists' error per CRUD semantics. This guidance helps LLMs select the correct action based on query existence. Related to #140 |
||
|
Stefan Broenner
|
3074254391 |
Fix: Power Query Update merges instead of replaces M code (Issue #140) (#141)
Root Cause:
UpdateAsync was setting query.Formula property which was merging/concatenating
new M code with existing code instead of replacing it, causing severely
corrupted Power Query definitions with duplicate let blocks and invalid syntax.
Fix:
Changed UpdateAsync to delete and recreate the query to ensure clean replacement:
- Delete old query completely
- Create new query with updated M code
- Preserves QueryTable recreation logic for column structure changes
Why Tests Didn't Catch It:
Existing tests only checked result.Success flag without verifying actual M code
content. Tests passed even though feature was broken because they didn't do
round-trip validation (update -> view -> verify content replaced).
Changes:
1. Updated testing-strategy.instructions.md with new Content Replacement
Validation section documenting the missing test pattern
2. Fixed UpdateAsync to delete+recreate instead of setting Formula property
3. Added 2 comprehensive regression tests:
- Update_ExistingQuery_ReplacesNotMergesMCode: Validates single update
- Update_MultipleSequentialUpdates_EachReplacesCompletely: Validates no
compounding corruption
Test Results:
- All 3 Update tests pass (1 original + 2 new regression tests)
- Regression tests verify old content removed and only new content present
- Sequential update test proves no compound merging
Fixes #140
Co-authored-by: Stefan Broenner <stefan.broenner@microsoft.comm>
|
||
|
Stefan Broenner
|
d1aebbb25b |
feat: MCP Server optimization - accurate tool descriptions and improved LLM guidance (#139)
* Fix all build errors and warnings
- Fixed culture-aware string operations (CA1310, CA1305, CA1304, CA1311)
- Added CultureInfo.InvariantCulture to ToString() calls
- Added StringComparison.Ordinal to string comparisons
- Used char overloads for StartsWith/EndsWith where applicable
- Fixed performance issues (CA1860, CA1861, CA1866, CA1869)
- Replaced .Any() with .Count comparisons
- Extracted inline arrays to static readonly fields
- Applied SuppressMessage for complex conditional arrays
- Cached JsonSerializerOptions instances
- Fixed reserved keyword conflicts (CA1716)
- Renamed Get/Set to GetValue/SetValue in INamedRangeCommands
- Fixed unused parameters (IDE0060, CA1725)
- Removed unused timeout parameters across MCP tools
- Removed unused promptName, crossFilterDirection parameters
- Renamed ambiguous parameters (function -> aggregationFunction)
- Fixed identifier resolution errors in CLI commands
- Qualified property references in TableCommands.cs
- Qualified property references in PivotTableCommands.cs
- Qualified property references in VbaCommands.cs
- Fixed obsolete API usage
- Removed IFormatProvider from Enum.ToString() calls
- Updated test call sites for renamed methods
Build now succeeds with 0 errors and 0 warnings (TreatWarningsAsErrors=true)
* feat: Implement QueryTable management commands and enhance PivotTable commands
- Added QueryTableCommands class with methods for listing, getting, refreshing, and deleting QueryTables.
- Enhanced PivotTableCommands with additional methods for getting, deleting, listing fields, and adding/removing fields.
- Updated Program.cs to integrate new QueryTable commands into the CLI.
- Modified README.md to reflect the new QueryTable commands and updated command counts.
- Adjusted test projects to suppress specific warnings related to XML documentation.
* Fix CA2016 and suppress IDE0008, SYSLIB1096, SYSLIB1054
- CA2016: Forward CancellationToken to async methods (6 fixes in ExcelSession.cs)
* Task.Run calls now include cancellationToken parameter
* Execute/ExecuteAsync/SaveAsync calls include timeout and cancellationToken
- IDE0008: Disabled 'Use explicit type instead of var' warning
* Changed severity from suggestion to none in .editorconfig
* User preference: use var keyword throughout codebase
- SYSLIB1096/SYSLIB1054: Suppressed instead of fixed
* GeneratedComInterface incompatible with legacy COM interop (IOleMessageFilter)
* LibraryImport fails with COM interface marshalling
* Modern source generators don't work with legacy COM P/Invoke
* Added suppressions to .editorconfig with detailed comments
Build: 0 warnings, 0 errors
Tests: Smoke test passes (all 11 MCP tools working)
Analyzers: CA2016, IDE0008, SYSLIB1096, SYSLIB1054 all clean
* fix: Update array initialization syntax to use brackets for consistency
* refactor: Update conditional checks to use pattern matching for improved readability
* test: Update ParallelBatches test to clarify SaveAsync omission reasons
* Remove confusing 'Phase 1' terminology from PowerQuery codebase
- Updated all source files to replace 'Phase 1' with 'Atomic Operations'
- Cleaned up 9 source files (CLI, Core, MCP Server)
- Deleted 4 diagnostic/research test files and documentation
- Updated test file header comments
- All 21 PowerQuery integration tests passing
The 'Phase 1' terminology made it appear there was legacy code, when these
are actually the current production methods (CreateAsync, UpdateMCodeAsync,
LoadToAsync, UnloadAsync, UpdateAndRefreshAsync, RefreshAllAsync).
* Remove flaky ParallelBatches_VerifyDataIntegrity test
This test was causing intermittent failures in CI due to Windows file
locking issues with parallel SaveAsync calls. The functionality is
already covered by other tests:
- SaveAsync_PersistsChanges_ToWorkbook (single save)
- ParallelBatches_10ConcurrentBatches_NoExcelProcessLeak (parallel batches)
- CompleteWorkflow_CreateModifyReadSave_AllOperationsSucceed (workflow)
We know Excel can save files - this test was redundant.
* Restore PowerQuery column structure regression tests
* Rename PowerQueryCommandsTests.Phase1.cs to PowerQueryCommandsTests.AtomicOps.cs
More descriptive name for tests covering atomic operations (CreateAsync, LoadToAsync,
UpdateMCodeAsync, UnloadAsync, UpdateAndRefreshAsync, RefreshAllAsync).
Removes last 'Phase1' reference from test files.
* fix: prevent column accumulation in QueryTable creation
- Add worksheet clearing before QueryTable creation in CreateQueryTableForQuery helper
- Prevents column accumulation when queries are deleted and recreated
- Fixes regression test Update_QueryColumnStructureWithDeleteRecreate_NoAccumulation
- Consistent with DELETE->CLEAR->CREATE pattern in UpdateAndRefreshAsync
- Also added worksheet clearing to LoadToAsync method in PowerQueryCommands.Advanced.cs
Changes:
- PowerQueryCommands.Lifecycle.cs: Clear worksheet in CreateQueryTableForQuery before creating QueryTable
- PowerQueryCommands.Advanced.cs: Clear worksheet in LoadToAsync before creating QueryTable (both code paths)
- ExcelQueryTableBehaviorDiagnostics.cs: New OnDemand diagnostic tests validating QueryTable behavior
- testing-strategy.instructions.md: Updated with OnDemand diagnostic test documentation
Validation:
- Both regression tests now pass (Update_QueryColumnStructure_UpdatesWorksheetColumns, Update_QueryColumnStructureWithDeleteRecreate_NoAccumulation)
- Diagnostic tests empirically validate DELETE->UPDATE->RECREATE pattern prevents RPC timeout
- Production code matches validated patterns from diagnostic tests
* Fix Data Model validation in CreateFromDataModelAsync
Every workbook has a Model object, but it may be empty. Changed validation from checking if model is null (always false) to checking if modelTables.Count == 0, which correctly detects when no tables exist in the Data Model.
Fixes test: CreateFromDataModel_NoDataModel_ReturnsError
* feat: Add comprehensive AI workflow hints to all MCP tools (186 methods)
Add contextual workflow hints and suggested next actions to all 10 MCP server tools,
providing LLMs with actionable guidance after every operation.
SUMMARY:
- 186 methods enhanced across 10 tools
- workflowHint: Context-aware status messages
- suggestedNextActions: 3 actionable next steps per operation
- CA1861 suppression added to all tools
TOOLS ENHANCED:
- ExcelRangeTool: 44 methods (largest - autofit, merge, formatting, validation)
- PivotTableTool: 41 methods (field management, data analysis)
- TableTool: 23 methods (Excel tables with filtering, sorting)
- ExcelPowerQueryTool: 17 methods (M code, atomic operations)
- ExcelDataModelTool: 17 methods (DAX measures, relationships)
- ExcelWorksheetTool: 11 methods (lifecycle, tab colors, visibility)
- ExcelConnectionTool: 13 methods (OLEDB, ODBC, Text connections)
- ExcelNamedRangeTool: 7 methods (parameters)
- ExcelQueryTableTool: 6 methods (legacy query tables)
- ExcelVbaTool: 7 methods (VBA automation)
WORKFLOW HINT FEATURES:
- Success/error-aware messaging
- Dynamic counting from result properties
- Conditional hints based on operation outcomes
- Cross-tool workflow suggestions
- Batch mode detection and recommendations
- Operation-specific timeout guidance
TECHNICAL PATTERNS:
- List truncation with separate counters (Take(10) + TotalMatches)
- Conditional suggestedNextActions based on result data
- Parameter interpolation with complex expressions
- Type-safe array construction (new object?[] for mixed types)
- Property name disambiguation for dynamic results
MCP COMPLIANCE:
- All tools return JSON for business errors (not exceptions)
- McpException reserved for parameter validation only
- Error handling follows MCP specification
- Documentation updated (mcp-server-guide.instructions.md)
BUILD STATUS:
- 0 warnings, 0 errors
- All 8 projects built successfully
- Code formatted with dotnet format
BREAKING CHANGES: None
BACKWARD COMPATIBILITY: Fully maintained
This enhancement significantly improves the MCP server's usefulness for AI coding
agents by providing clear, actionable guidance after every Excel operation.
* refactor: Clean up tool documentation - remove LLM guidance from C# code
DOCUMENTATION CLEANUP:
- Removed 'LLM Usage Patterns' sections from 10 tool files (~250 lines)
- Simplified class summaries to concise technical API documentation (3-7 lines)
- Preserved technical constraints, prerequisites, and architectural notes
- LLM guidance remains in prompt files (Prompts/Content/*.md) - single source of truth
Files refactored:
- ExcelPowerQueryTool.cs (48→4 lines class + 24→1 method description)
- ExcelWorksheetTool.cs (20→4 lines)
- ExcelVbaTool.cs (17→5 lines)
- ExcelDataModelTool.cs (52→3 lines)
- ExcelTableTool.cs (22→7 lines)
- ExcelRangeTool.cs (27→10 lines)
- ExcelQueryTableTool.cs (22→5 lines)
- ExcelNamedRangeTool.cs (17→7 lines)
- ExcelFileTool.cs (7→4 lines)
RATIONALE:
- C# XML comments are for developers, not LLMs
- MCP protocol doesn't send C# comments to LLMs
- Prompt files (.md) are the authoritative source for LLM behavior
- Eliminates duplication and maintenance burden
- See: .github/instructions/mcp-llm-guidance.instructions.md
- Analysis: TOOL-DOCUMENTATION-CLEANUP.md
BUILD:
- Build passes: 0 errors, 0 warnings
- All tools function correctly after changes
Co-committed changes from earlier work:
- Fixed audit script false positives in completeness check
- Renamed PivotTableTool.cs → ExcelPivotTableTool.cs
- Renamed TableTool.cs → ExcelTableTool.cs
- Updated test references for renamed tools
* docs: Remove LLM guidance from tool code documentation
Cleaned up tool file documentation by removing 'LLM Usage Patterns' sections and simplifying class summaries to focus on technical descriptions. Preserved essential technical constraints and prerequisites. Ensured all LLM guidance is maintained in prompt files.
* Enhance MCP prompt files with elicitation references and related tool cross-references
What: Enhanced all 12 MCP tool prompt files with workflow optimization guidance
Changes:
- Added elicitation references (⚠️ BEFORE CALLING sections)
* excel_powerquery → powerquery_import.md
* excel_table → table_creation.md
* excel_range → data_validation.md, range_formatting.md
* excel_datamodel → dax_measure.md
* excel_querytable → querytable_creation.md
* excel_batch → batch_workflow.md
- Added 'Related tools' section to all 12 prompt files
* Cross-references to complementary tools
* Batch mode optimization reminders (75-90% faster)
* Clear workflow sequences (e.g., powerquery → datamodel → pivottable)
- Fixed naming inconsistency
* excel_namedrange.md: Changed header from 'excel_parameter Tool' to 'excel_namedrange Tool'
Why: Improves LLM workflow planning by:
- Proactive parameter gathering (check elicitation BEFORE calling)
- Better tool selection (see related tools upfront)
- Performance optimization (batch mode reminders)
- Reduced round-trips (complete info gathered first)
Impact: LLMs receive comprehensive workflow guidance without duplication
- C# code = Developer reference (technical constraints)
- Prompt files = LLM guidance (workflow optimization + tool relationships)
- Elicitations = Pre-flight checklists (complete parameter gathering)
Files modified: 12 prompt files in src/ExcelMcp.McpServer/Prompts/Content/
* feat: Update tool descriptions to ensure accuracy and enhance guidance for LLMs
* fix: resolve Copilot and GitHub Advanced Security code review comments
Addressed 17 automated code review comments from PR #139:
Documentation fixes (2):
- Removed improper /// <inheritdoc/> from test constructors and methods
that don't override anything (McpServerSmokeTests.cs, ExcelMcpServerTests.cs)
Performance improvements (3):
- Replaced .AsSpan(0, 8).ToString() with [..8] for better string slicing
performance in PowerQueryCommandsTests.cs (lines 131, 158, 284)
Null safety fix (1):
- Added explicit variable extraction after null checks in ExcelWorksheetTool.cs
to satisfy code analyzer (redValue, greenValue, blueValue)
Code style improvements (2):
- Replaced foreach loop with .Select() LINQ in ExcelTableTool.cs:401
- Combined nested if statements in ExcelTableTool.cs:681
Generic catch clause suppressions (4):
- Added #pragma warning disable CA1031 with justifications for diagnostic
test catch blocks in ExcelQueryTableBehaviorDiagnostics.cs
Path.Combine warnings suppression (5):
- Added file-level #pragma warning disable CA3003 for test code with
controlled temp directory paths in ExcelQueryTableBehaviorDiagnostics.cs
Build verification: 0 warnings, 0 errors
* docs: add PR review comment workflow to instructions
Added comprehensive guidance for checking and fixing automated PR review comments:
1. Updated copilot-instructions.md:
- Added 'After creating PR' to Common Tasks section
- Added PR Review Comments to Key Lessons Learned
2. Updated development-workflow.instructions.md:
- Added complete PR Review Comment Workflow section
- Documented common automated reviewers (Copilot, GitHub Advanced Security)
- Listed common code quality issues and how to fix them
- Added GitHub CLI and mcp_github tool examples
3. Added CRITICAL-RULES.md Rule 19:
- Made PR review comment checks mandatory (5-10 min)
- Provided step-by-step process for handling automated reviews
- Included example from PR #139 (17 comments fixed)
Why this matters:
- Automated reviewers (Copilot, github-advanced-security) catch common issues early
- Fixing issues before human review speeds up PR approval
- Prevents technical debt accumulation
- Improves code quality and maintainability
This workflow is now part of the standard PR process for all contributors.
* fix: Address CodeQL notices
Combined nested if statements in VbaCommands.cs and enhanced suppression justifications for CodeQL reviewers in diagnostic test code.
* refactor: simplify prompts - remove duplication with tool descriptions
Prompts now focus only on server-specific quirks and action disambiguation per MCP-LLM-Guidance instructions. Tool descriptions already cover:
- What tools do (purpose/use cases)
- When to use which tool (tool selection)
- Load destinations and parameters
- Performance guidance (batch mode)
- Related tools
Prompts are now SHORT (10-15 lines vs 50-100) focusing on:
- Action disambiguation (which action for which scenario)
- Server quirks (validation=execution, one batch per file, etc.)
- Non-obvious behavior LLMs need to know
Files trimmed:
- excel_powerquery.md: 98 lines → 12 lines
- excel_batch.md: 65 lines → 11 lines
- excel_table.md: 45 lines → 10 lines
- excel_datamodel.md: 42 lines → 11 lines
Follows guidance: LLMs already know Excel and programming, they need to know THIS server's quirks.
* fix: remove unimplemented 'eval' action from PowerQueryAction enum
The 'eval' action was defined in PowerQueryAction enum and had a handler in
ExcelPowerQueryTool, but no corresponding EvalAsync method exists in
IPowerQueryCommands interface. This would cause runtime exceptions if called.
Changes:
- Removed PowerQueryAction.Eval from ToolActions.cs
- Removed 'eval' mapping from ActionExtensions.cs
- Removed switch case and EvalPowerQueryAsync handler from ExcelPowerQueryTool.cs
- Added check-mcp-core-implementations.ps1 to validate all enum actions have Core methods
- Updated pre-commit.ps1 to run the new validation
The new pre-commit check prevents this issue by ensuring every MCP Tool enum
action has a matching *Async method in the corresponding Core Commands interface.
* refactor: remove broken EvalAsync from Core and CLI
The eval functionality was incomplete and unnecessary:
- Creates temp queries with side effects (not pure evaluation)
- Refresh failures conflate syntax errors with connectivity issues
- LLMs already know M code syntax, don't need Excel validation
Changes:
- Core: Removed IPowerQueryCommands.EvalAsync interface method
- Core: Removed PowerQueryCommands.EvalAsync implementation (60 lines)
- CLI: Removed IPowerQueryCommands.Eval interface method
- CLI: Removed PowerQueryCommands.Eval implementation (50 lines)
- CLI: Removed pq-verify command routing in Program.cs
- Scripts: Added known exceptions to check-mcp-core-implementations.ps1
This completes the eval removal started in previous commit which removed
the MCP Server layer. The feature is now completely removed from all layers.
* fix: add known exceptions to MCP-Core validation script
Enhanced check-mcp-core-implementations.ps1 to handle documented exceptions:
- FileAction.CloseWorkbook - Handled by MCP server directly
- TableAction.ApplyFilterValues, SortMulti - Composite operations
These are intentional gaps where MCP tools implement functionality without
corresponding Core methods (documented in CORE-METHOD-RENAMING-SUMMARY.md)
* test: add regression test for UpdateMCode + Refresh load config preservation
- Added UpdateMCodeThenRefresh_QueryLoadedToSheet_PreservesLoadConfiguration test
- Validates that UpdateMCode followed by Refresh preserves load configuration
- Test PASSES - confirms bug is not in Core layer
- User reported: UpdateMCode + Refresh causes query to become connection-only
- Investigation result: Core implementation is correct, bug may be user perception or MCP layer interpretation
Test workflow:
1. Create query loaded to worksheet
2. Verify load config (LoadToTable mode)
3. UpdateMCode with new M code
4. Refresh query data
5. Verify load config still LoadToTable (not connection-only)
6. Verify target sheet still set
Related to user report from 2025-01-28
* spec: simplify PowerQuery API - remove atomic vs granular distinction
BREAKING CHANGE: UpdateAsync now auto-refreshes, UpdateAndRefreshAsync removed
User Report (2025-01-28):
- UpdateMCode + Refresh leaves stale data (design footgun)
- Atomic vs granular distinction confuses LLMs
- LLMs naturally expect 'update' to be complete operation
Design Problems:
- Artificial atomic vs granular distinction
- UpdateMCode without auto-refresh = stale worksheet data
- Requires explicit second call to Refresh (easy to forget)
New Design Philosophy:
- Each operation does complete, intuitive thing
- update = Update M code AND refresh data (ONE operation)
- No incomplete operations that require second call
Changes:
- UpdateMCodeAsync → UpdateAsync (clearer name, auto-refreshes)
- Remove UpdateAndRefreshAsync (now redundant)
- Update workflow hints to remove 'call refresh next'
Why Break Compatibility:
- Atomic operations added recently (Phase 1, not widely used)
- Fixing footgun NOW prevents long-term confusion
- LLM-optimized API more important than backward compat
Implementation Plan:
- 14 todos covering Core, MCP, CLI, tests, docs
- See updated POWERQUERY-FUTURE-STATE-SPEC.md for details
* refactor(core,mcp,cli,tests,docs): simplify PowerQuery API - Update always refreshes
BREAKING CHANGE: PowerQuery Update operation now automatically refreshes data
This commit completes the PowerQuery API simplification by updating all layers
of the codebase to maintain consistency and eliminate the stale data footgun.
Core Changes:
- Renamed UpdateMCodeAsync -> UpdateAsync in IPowerQueryCommands
- UpdateAsync now calls RefreshAsync automatically after updating M code formula
- Removed UpdateAndRefreshAsync method (redundant - UpdateAsync does both)
- Error handling: Success only if BOTH update and refresh succeed
- Error message format: 'M code updated but refresh failed: {details}' for partial failures
MCP Changes:
- PowerQueryAction enum: UpdateMCode -> Update, removed UpdateAndRefresh (12 values, was 13)
- ActionExtensions: 'update-mcode' -> 'update' mapping, removed 'update-and-refresh'
- ExcelPowerQueryTool: UpdateMCodePowerQueryAsync -> UpdatePowerQueryAsync
- ExcelPowerQueryTool: Removed UpdateAndRefreshPowerQueryAsync method
- Tool description updated: Removed references to granular vs complete operation distinction
- Prompt file (excel_powerquery.md) updated: Simplified action list
- Workflow hints updated: 'M code updated and data refreshed for query. Query is current.'
- Removed stale suggestions to 'call refresh next' (now auto-refreshes)
CLI Changes:
- PowerQueryCommands: UpdateMCode -> Update method
- Removed: UpdateAndRefresh method (entire implementation deleted)
- Program.cs routing: 'pq-update-mcode' -> 'pq-update'
- Removed: 'pq-update-and-refresh' routing
- Help text simplified: 4 operations (was 5)
- Messages emphasize complete operation: 'Updated M code and refreshed data'
Test Changes:
- PowerQueryCommandsTests.cs: All UpdateMCodeAsync -> UpdateAsync (6 occurrences)
- PowerQueryCommandsTests.cs: All UpdateAndRefreshAsync -> UpdateAsync (2 occurrences)
- PowerQueryCommandsTests.AtomicOps.cs: Updated method calls and comments
- Removed 'atomic' terminology from test comments
- Simplified test logic where separate update+refresh pattern existed
Documentation Changes:
- COMMANDS.md updated: pq-update-mcode -> pq-update, removed pq-update-and-refresh
- Workflow examples updated to reflect simplified command set
- Phase 1 operations list updated (4 operations, was 5)
Rationale:
- LLM-optimized: Complete operations are more intuitive than fine-grained control
- Eliminates stale data footgun (update M code without refreshing data)
- Simpler mental model: 'update' means 'make the query current with new code and fresh data'
- Consistent with Create operation (also loads data by default)
Impact:
- MCP Server: 'update' action now complete operation
- CLI: 'pq-update' command now complete operation
- Tests: All passing with simplified API
- Documentation: Reflects complete operation model
Related: specs/POWERQUERY-FUTURE-STATE-SPEC.md (commit
|
||
|
Stefan Broenner
|
2a08a04a35 |
feat: Power Query Phase 1 & 2 Complete - Atomic Operations + Documentation Cleanup
Complete refactoring of Power Query API with Phase 1 atomic operations, Phase 2 integrations, and comprehensive cleanup. **Phase 1 - Atomic Operations (6 methods):** - ✅ Create: Import + load in single atomic operation - ✅ UpdateMCode: Update M code without refresh - ✅ Unload: Convert to connection-only - ✅ UpdateAndRefresh: Update M code + refresh atomically - ✅ RefreshAll: Refresh all queries in one operation - ❌ ValidateSyntax: Removed (Excel COM doesn't support) **Phase 2 - Integration:** - ✅ MCP Server: All 6 Phase 1 actions functional - ✅ CLI: All 6 commands implemented - ✅ 25/25 integration tests passing (100% success rate) **Phase 3 - Documentation & Cleanup:** - ✅ All MCP prompts updated for Phase 1 consistency - ✅ Removed non-functional Errors action (COM API limitation) - ✅ 10 files cleaned across Core, MCP Server, and CLI layers **Quality Verification:** - ✅ All pre-commit checks passing (COM leaks, coverage, naming, success flags, smoke test) - ✅ Build passing (0 warnings, 0 errors) - ✅ 100% test success rate maintained - ✅ Coverage: 101.9% (154 Core methods → 157 enum values) **Breaking Changes:** - Removed ValidateSyntax action (never worked - Excel COM limitation) - Removed Errors action (returned hardcoded messages - COM doesn't expose error details) Ready for production use. |
||
|
Copilot
|
8e3b238e45 |
Fix: Remove regex validation preventing numeric column names in Excel tables (#137)
* Initial plan * Fix: Remove regex validation preventing numeric column names Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Update table prompt to document numeric column name support Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> |
||
|
Stefan Broenner
|
7637d29f83 |
Fix PowerQuery column structure bugs (#133) (#134)
Fixed two critical bugs in PowerQuery operations when column structure changes: Bug #1: Column structure doesn't update when query M code changes - QueryTable.PreserveColumnInfo was preventing column updates - Changed default to false in PowerQueryHelpers.cs Bug #2: Columns accumulate when using delete+recreate workflow - Deleting QueryTable left data on worksheet causing visual concatenation - Added worksheet clearing logic before creating new QueryTables Implementation: - PowerQueryHelpers.cs: Changed PreserveColumnInfo = false (was true) - PowerQueryHelpers.cs: Added RemoveQueryTablesFromSheet() helper method - PowerQueryCommands.Lifecycle.cs: Enhanced UpdateAsync with complete fix - Delete existing QueryTables - Clear worksheet data (prevents accumulation) - Create fresh QueryTable with new column structure - PowerQueryCommands.LoadConfig.cs: Enhanced SetLoadToTableAsync with same fix - Delete existing QueryTables - Clear worksheet data (prevents accumulation) - Create fresh QueryTable Tests: - Added 2 regression tests validating both bugs are fixed - Test 1: Update_QueryColumnStructure_UpdatesWorksheetColumns (UpdateAsync path) - Test 2: Update_QueryColumnStructureWithDeleteRecreate_AccumulatesColumns (SetLoadToTableAsync path) - Both tests passing - validates fix works across both code paths Closes #133 Co-authored-by: Stefan Broenner <stefan.broenner@microsoft.comm> |
||
|
Copilot
|
910986ceaf |
Add timeout support to all MCP tools (#132)
* Initial plan * Add timeout parameter to all MCP tools Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Add timeout support to core commands (PivotTable, QueryTable, VBA) Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Fix culture-specific timeout test assertions Make timeout exception message assertions culture-agnostic by using regex patterns that accept both comma and period as decimal separators. This fixes test failures when running in environments with different locale settings. Changed from exact string match ("0,00166") to regex pattern (@"0[.,]0+1\d+") that matches both European ("0,00166") and US ("0.00166") number formats. Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> |
||
|
Copilot
|
93f73fcb85 |
Implement complete QueryTable support for Excel MCP Server with comprehensive tests and LLM guidance (#130)
* Checkpoint from VS Code for coding agent session * Add FindQueryTable method to ComUtilities.cs Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Add QueryTable Core Commands implementation (Phase 1) Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Add QueryTable MCP Server integration (Phase 2) Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Add comprehensive QueryTable integration tests Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Add comprehensive LLM guidance for QueryTable MCP tool Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * fix: QueryTable tests - use file-based M code import with connection-only loading All 15 QueryTable tests now pass successfully: - Fixed ImportAsync calls to use temporary .pq files instead of inline M code - Added loadDestination: 'connection-only' to prevent auto-refresh hangs - Resolved namespace conflict with System.IO.File.WriteAllTextAsync - Optimized List test to use single batch for all operations Test Results: ✅ 15/15 tests passed (3.38 minutes total) ✅ List operations: 2 tests ✅ Lifecycle operations: 5 tests ✅ Operations: 8 tests Pattern applied: 1. Write M code to temp file: Path.Combine(_tempDir, 'Query.pq') 2. Import as connection-only: prevents data refresh during test setup 3. Create QueryTable: validates QueryTable creation and operations 4. Single batch: all operations in one batch for efficiency * refactor: Remove unnecessary load config restore logic from PowerQuery UpdateAsync Excel automatically preserves load configuration when updating query.Formula property. The restore logic (capture before, restore after) was defensive coding that added complexity without benefit. Changes: - Removed GetLoadConfigAsync call before update - Removed SetLoadToTableAsync restore logic after update - Simplified UpdateAsync to just update the formula - Added comment explaining Excel's automatic preservation behavior Evidence: - Test Update_QueryLoadedToSheet_PreservesLoadConfiguration passes WITHOUT restore - Verified by temporarily disabling restore - test still passes - All 12 PowerQuery integration tests pass Benefits: - 30% less code in UpdateAsync method - Faster execution (no unnecessary GetLoadConfig + SetLoadToTable calls) - Clearer intent - just update the formula - Eliminates 2 batch operations per update Discovered via LLM investigation of reported issue that update turns queries into connection-only (issue was NOT true, but led to finding this optimization). --------- Co-authored-by: Stefan Broenner <stefan.broenner@microsoft.comm> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> |
||
|
Stefan Broenner
|
6e50cc0f8c |
fix: Fix connection refresh test failures and Success flag violations
Fix test failures in ConnectionCommandsTests and Success flag violations. - Fixed Success flag violations where Success=true was set with ErrorMessage populated (Rule 0) - Removed test with incorrect Excel COM expectations about file validation timing - Added documentation of actual Excel COM behavior for TEXT connections - Simplified ExcelConnectionTool documentation (MCP auto-discovers actions) - All connection refresh tests now pass (5/5) - All pre-commit checks pass (COM leaks, coverage, success flags, MCP smoke test) |
||
|
Stefan Broenner
|
a1d5323f1f |
feat: Add LLM-configurable timeout parameters to MCP tools
Add LLM-configurable timeout parameters to MCP tools - Add timeout parameter to BatchSessionTool (save operations, 2min default) - Add timeout parameter to PowerQueryTool (refresh operations, 5min default) - Add timeout parameter to ExcelConnectionTool (connection operations, 2min default) - Add timeout parameter to ExcelDataModelTool (data model operations, 2min default) Core Infrastructure: - Add timeout overloads to IPowerQueryCommands, IConnectionCommands, IDataModelCommands - Implement timeout-aware methods in Core commands - Update ExcelBatch timeout handling for SaveAsync operations LLM Benefits: - No artificial timeout limits - trust LLM intelligence - Operation-specific defaults with LLM override capability - Proper timeout conversion and enhanced error messages Implementation: - MCP Parameter: double? timeout = null (minutes) - Core Methods: TimeSpan? timeout parameter - Execute Calls: timeout ?? TimeSpan.FromMinutes(default) - Proven across 4 tools, ready for systematic rollout Script Fix: - Fix audit-core-coverage.ps1 to handle method overloads correctly - Count unique method names instead of total method signatures Status: 40% complete (4 of 10 MCP tools) |
||
|
Stefan Broenner
|
70acaf3fb3 |
fix: Add timeout protection to SaveAsync for batch commit operations (#125)
- IExcelBatch.SaveAsync: Added timeout parameter (default null) - ExcelBatch.SaveAsync: Implemented 5-minute default timeout with Task.WaitAsync - BatchSessionTool.CommitBatchAsync: Added TimeoutException enrichment with LLM guidance - Fixes batch commit hanging indefinitely when save operations take too long - Timeout behavior consistent with Execute/ExecuteAsync patterns Closes issue: Batch commit never finishing with save requested Co-authored-by: Stefan Broenner <stefan.broenner@microsoft.comm> |