mirror of
https://github.com/sbroenne/mcp-server-excel.git
synced 2026-09-19 07:53:08 +08:00
Stefan Broenner
299ee68703
* feat(range): Add locale-aware number formatting with separate actions PROBLEM: - Format codes like '\$#,##0.00' interpreted incorrectly on non-US locales - German Excel treated '.' as thousands separator, ',' as decimal - LLM API had confusing design with 'formatCode OR numberFormatCategory' SOLUTION: 1. Use NumberFormatLocal instead of NumberFormat (interprets locale correctly) 2. Use LocaleAwareFormatBuilder to generate format codes using CultureInfo 3. Separate into two clean actions for better LLM usability: - SetNumberFormat: Structured params only (numberFormatCategory required) - SetNumberFormatCustom: Raw formatCode only (expert use) NEW FILES: - NumberFormatOptions.cs: Enums and LocaleAwareFormatBuilder MODIFIED: - RangeCommands.NumberFormat.cs: NumberFormatLocal + SetNumberFormatStructured - ExcelRangeTool.cs: Two separate actions, cleaner parameter routing - ToolActions.cs: SetNumberFormatCustom enum added - ActionExtensions.cs: set-number-format-custom mapping - audit-core-coverage.ps1: Added SetNumberFormatCustom to known exceptions - check-mcp-core-implementations.ps1: Added SetNumberFormatCustom to known exceptions VERIFIED: - German locale shows correct formats: EUR#.##0,00, 0,0%, #.##0,00 - Build passes with 0 warnings - All pre-commit checks pass * docs(range): Add LLM guidance for number formatting actions - Add [Description] attribute to excel_range tool with number formatting guidance - Create excel_range.md prompt with detailed action comparison - Create ExcelRangePrompts.cs to expose prompt via MCP Guidance helps LLMs choose between: - SetNumberFormat: RECOMMENDED, locale-aware, structured parameters - SetNumberFormatCustom: Expert use, raw format codes, locale-sensitive * docs: Update instructions to use XML documentation instead of [Description] MCP SDK now extracts tool descriptions from XML documentation (/// <summary>) rather than [Description] attributes. Updated files: - mcp-server-guide.instructions.md: XML docs guidance and examples - critical-rules.instructions.md: Rule 18 now shows XML doc patterns - bug-fixing-checklist.instructions.md: Reference XML docs not [Description] - ExcelRangeTool.cs: Move description to XML docs, remove [Description] * docs: Add guidance to ask user before closing when showExcel=true When Excel is visible (showExcel=true), the user is actively watching. LLM should ask before closing instead of auto-closing, since user may want to inspect results, make manual changes, or continue working. Updated files: - Program.cs: Server instructions with new 'ASK BEFORE CLOSING' section - ExcelFileTool.cs: XML docs with same guidance * feat: Add operation tracking to prevent premature session close - Add BeginOperation/EndOperation tracking in SessionManager - Block Close action when operations are still running - Add ValidateClose method with CloseValidationResult record - Enhanced List action to return activeOperations, canClose, isExcelVisible - Add force parameter to CloseSession for emergency override - Track showExcel flag per session for visibility status - Updated server instructions about automatic operation tracking - Fixed missing FileAction.List mapping in ActionExtensions Tests: - 18 new unit tests for SessionManager operation tracking - 4 new MCP integration tests for ExcelFileTool operation tracking This prevents data loss from closing sessions while parallel operations are still running. The server now automatically tracks operations and returns a blocking error with activeOperations count if close is attempted while operations are in progress. * feat(datamodel): add DeleteTable action to remove orphaned Data Model tables - Add DeleteTable to DataModelAction enum and ToActionString mapping - Add DeleteTable method to IDataModelCommands interface - Implement DeleteTable using SourceWorkbookConnection.Delete() pattern (ModelTable is read-only, must delete via associated connection) - Add DeleteTable handler in ExcelDataModelTool - Add 3 integration tests covering success and error scenarios This allows cleanup of orphaned tables when Power Query is deleted and recreated with loadDestination: data-model. * fix(connection): correct error message for Power Query connection deletion Changed error message from outdated 'pq-delete' to correct 'excel_powerquery with action Delete' when user attempts to delete a Power Query connection via excel_connection tool. * Simplify number formatting: auto-LCID for date/time, streamlined docs - Add EnsureDateTimeLcid() to automatically prefix date/time formats with [$-409] - LLMs no longer need to know about LCID - just use 'mm/dd/yyyy' directly - Currency symbols ($, €, £, ¥) are literal - no special handling needed - Simplified excel_range.md from 95 lines to 28 lines (one table, essentials only) * Simplify telemetry: remove redundant TrackPageView - Remove TrackPageView call (Usage blades don't work for console apps) - Keep TrackEvent for tool usage analytics (customEvents table) - Keep TrackRequest for performance metrics (Performance blade) - Update documentation to reflect simplified telemetry model * Track unhandled exceptions and file context in telemetry - Add try-catch in Program.Main() to track MCP SDK/transport errors - Pass excelPath to TrackToolInvocation for file-level analytics grouping - Ensures telemetry is flushed before exit on unhandled exceptions * docs: fix showExcel default guidance and incorrect action names - Fix ServerInstructions to clarify showExcel:false is DEFAULT (not showExcel:true) - Update tool description to emphasize default is hidden/faster mode - Fix incorrect pivottable actions: 'create' with sourceType -> 'create-from-table'/'create-from-datamodel' - Remove non-existent 'create-bulk' action reference from namedrange examples - Fix NamedRangeCommands comment to match actual exposed operations - Update VS Code extension copilot-instructions with correct defaults and actions * docs: enhance copilot instructions for Excel MCP usage clarity * Merge origin/main: keep operation tracking features from this branch * fix: address GitHub Advanced Security warnings - Fix constant condition in ConnectionCommands (check HasValue before switch) - Add using statement for SessionManager in Dispose_CleansUpAllTracking test - Suppress CA3003 (Path.Combine) in test code - controlled inputs - Suppress CA1031 (generic catch) in test cleanup - best effort disposal - Use .Where() instead of if-in-foreach for filtering
ExcelMcp Tests
⚠️ No Traditional Unit Tests: ExcelMcp has no unit tests. Integration tests ARE our unit tests because Excel COM cannot be meaningfully mocked. See
docs/ADR-001-NO-UNIT-TESTS.mdfor full architectural rationale.
Quick Start
# Development (fast feedback - excludes VBA tests)
dotnet test --filter "Category=Integration&RunType!=OnDemand&Feature!=VBA&Feature!=VBATrust"
# Pre-commit (comprehensive - excludes VBA tests)
dotnet test --filter "Category=Integration&RunType!=OnDemand&Feature!=VBA&Feature!=VBATrust"
# Session/batch changes (MANDATORY when modifying session/batch code)
dotnet test --filter "RunType=OnDemand"
# VBA tests (manual only - requires VBA trust enabled)
dotnet test --filter "(Feature=VBA|Feature=VBATrust)&RunType!=OnDemand"
Documentation
For complete testing guidance, see:
- Testing Strategy - Quick reference, templates, common mistakes
- Critical Rules - Mandatory development rules (Rule 14: SaveAsync)
Test Architecture
tests/
├── ExcelMcp.Core.Tests/ # Core business logic (Unit + Integration)
├── ExcelMcp.McpServer.Tests/ # MCP protocol layer (Unit + Integration)
├── ExcelMcp.CLI.Tests/ # CLI wrapper (Unit + Integration)
└── ExcelMcp.ComInterop.Tests/ # COM utilities (Unit + OnDemand)
Test Categories
| Category | Speed | Requirements | Run By Default |
|---|---|---|---|
| Unit | Fast (2-5 sec) | None | ✅ Yes (CI/CD) |
| Integration | Medium (10-20 min) | Excel + Windows | ✅ Yes (local) |
| OnDemand | Slow (3-5 min) | Excel + Windows | ❌ No (explicit only) |
Feature-Specific Tests
# Test specific feature only
dotnet test --filter "Feature=PowerQuery&RunType!=OnDemand"
dotnet test --filter "Feature=DataModel&RunType!=OnDemand"
dotnet test --filter "Feature=Tables&RunType!=OnDemand"
dotnet test --filter "Feature=PivotTables&RunType!=OnDemand"
dotnet test --filter "Feature=Ranges&RunType!=OnDemand"
dotnet test --filter "Feature=Connections&RunType!=OnDemand"
When to Run Which Tests
| Scenario | Command |
|---|---|
| Daily development | dotnet test --filter "Category=Integration&RunType!=OnDemand&Feature!=VBA" |
| Before commit | dotnet test --filter "Category=Integration&RunType!=OnDemand&Feature!=VBA" |
| Modified session/batch code | dotnet test --filter "RunType=OnDemand" (see Rule 3) |
| VBA development | dotnet test --filter "(Feature=VBA|Feature=VBATrust)&RunType!=OnDemand" |
VBA Testing
Why VBA Tests Are Excluded by Default
VBA tests are excluded from normal test runs because:
- Stable codebase - VBA features are mature with minimal changes
- Performance - Excluding VBA tests makes integration tests ~25% faster (10-15 min vs 15-20 min)
- Special requirements - VBA tests require VBA trust enabled in Excel settings
- Opt-in model - Explicit testing when VBA code changes, rather than every commit
When to Run VBA Tests
Run VBA tests manually when:
- Modifying VBA-related code (ScriptCommands, VbaTrustDetection)
- Adding new VBA features
- Before releasing VBA-related changes
- Troubleshooting VBA-specific issues
How to Run VBA Tests
# Run ONLY VBA tests
dotnet test --filter "(Feature=VBA|Feature=VBATrust)&RunType!=OnDemand"
# Run ALL tests including VBA (takes longer)
dotnet test --filter "Category=Integration&RunType!=OnDemand"
VBA Test Files
All VBA tests are tagged with [Trait("Feature", "VBA")] or [Trait("Feature", "VBATrust")]:
tests/ExcelMcp.Core.Tests/Integration/Commands/Script/
- ScriptCommandsTests.cs
- ScriptCommandsTests.Lifecycle.cs
- VbaTrustDetectionTests.ScriptCommands.cs
- VbaTrustDetectionTests.cs
tests/ExcelMcp.CLI.Tests/Integration/Commands/
- ScriptAndSetupCommandsTests.cs
VBA Trust Setup
VBA tests require VBA trust enabled in Excel:
# Enable VBA trust (required for VBA tests)
Set-ItemProperty -Path "HKCU:\Software\Microsoft\Office\16.0\Excel\Security" -Name "AccessVBOM" -Value 1
# Verify setting
Get-ItemProperty -Path "HKCU:\Software\Microsoft\Office\16.0\Excel\Security" -Name "AccessVBOM"
Security Note: Only enable VBA trust in development environments. Production systems should keep this disabled.
Key Principles
- ✅ File Isolation - Each test creates unique file (no sharing)
- ✅ Binary Assertions - Pass OR fail, never "accept both"
- ✅ Verify Excel State - Always verify actual Excel state after operations
- ❌ No SaveAsync - Unless testing persistence (see Rule 14)
Getting Help
- Test failures: Check test output for detailed error messages
- Excel issues: Ensure Excel 2016+ installed and activated
- Session/batch issues: Run OnDemand tests to verify cleanup
- Writing tests: See Testing Strategy