skills/sbroenne__mcp-server-excel
1
0
Fork 0
mirror of https://github.com/sbroenne/mcp-server-excel.git synced 2026-09-19 07:53:08 +08:00
Code Issues Packages Projects Releases Wiki Activity
160 Commits 13 Branches 184 Tags
v1.4.21
Commit Graph

3 Commits

Author SHA1 Message Date
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)
 2025-11-18 17:58:43 +00:00
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 21d4c58.

* fix: return JSON errors from OpenSessionAsync instead of throwing

Changed OpenSessionAsync to catch exceptions from CreateSessionAsync and return JSON with success=false instead of letting them bubble up as unhandled exceptions.

Fixes the generic 'An error occurred invoking excel_file' message shown when file was already open or other session creation errors occurred.

Files changed:
- ExcelFileTool.cs: Wrapped CreateSessionAsync in try-catch, return JSON errors

* refactor: Replace McpException with standard .NET exceptions

CRITICAL INSIGHT (Phase 8): Discovered that McpException usage was unnecessary complexity.
Since we catch ALL exceptions before the MCP SDK sees them and return JSON responses,
the SDK never encounters McpException - it gets our JSON error response instead.

CHANGES:
- Removed McpException catch blocks from all 11 tool files
- Updated ExcelToolsBase to throw ArgumentException (validation) and InvalidOperationException (state)
- Replaced McpException throws in:
  * ExcelFileTool (5 throws)
  * ExcelConnectionTool (17 throws)
  * All 9 'unknown action' switch branches
  * ExcelWorksheetTool invalid visibility

ARCHITECTURAL BENEFIT:
- Removed unnecessary coupling to ModelContextProtocol.McpException
- Single catch (Exception ex) block now handles all cases
- Uses standard .NET exception types for clearer semantics:
  * ArgumentException for parameter validation
  * InvalidOperationException for invalid state
- Simpler, more idiomatic C# code
- Same error handling quality (descriptive JSON responses)

REMAINING:
- ~86 parameter validation throws still using McpException (will be caught as Exception)
- These still work correctly but should be migrated to ArgumentException in future PRs
- Build: 0 warnings, 0 errors

RATIONALE:
The MCP SDK wraps our tool invocations with try-catch. Our code catches exceptions
BEFORE the SDK sees them and returns JSON with isError=true. Therefore, whether we
throw McpException, ArgumentException, or any other Exception is irrelevant to the
SDK - it receives JSON in all cases. Using standard .NET exceptions is cleaner,
doesn't add SDK coupling, and makes the code more maintainable.

* refactor: Complete migration to ArgumentException for ALL parameter validations

Complete removal of McpException pattern across all 11 tool files:
- ExcelDataModelTool: 23 throws (all measure/table/relationship parameters)
- ExcelWorksheetTool: 16 throws (all sheet/color/visibility parameters)
- ExcelVbaTool: 10 throws (all module/path parameters)
- ExcelTableTool: 5 throws (all table/column/format parameters)
- ExcelQueryTableTool: 10 throws (all query table parameters)
- ExcelRangeTool: 2 throws (shift direction validation)
- ExcelPivotTableTool: 4 throws (aggregation/sort direction validation)
- ExcelPowerQueryTool: 12 throws (completed in prior batch)
- ExcelNamedRangeTool: 9 throws (completed in prior batch)
- ExcelConnectionTool: 17 throws (completed in prior batch)
- ExcelFileTool: 5 throws (completed in prior batch)

TOTAL MIGRATION: 117/~117 parameter validation throws replaced

Architecture Insight:
- Standard .NET exceptions (ArgumentException for invalid input)
- Single catch (Exception ex) block handles all cases
- No SDK coupling - clean separation of concerns
- Build: 0 warnings, 0 errors (maintained throughout)

Breaking Simplification:
- Removed unnecessary McpException abstraction layer
- SDK-side exception handling wraps all tool invocations
- No behavior change to callers (still returns JSON with isError=true)
- Cleaner, more maintainable code with standard .NET patterns

* refactor: Simplify async command invocation by removing unnecessary lambda expressions

* fix: update .gitignore to exclude Jekyll generated files

* Fix MCP Server tests: tool discovery and enum completeness

- Fixed ToolDiscoveryTests: Removed non-existent 'excel_batch' from expected tools list
- Fixed ActionEnumCompletenessTests: Added FileAction.Open/Save/Close mappings to ActionExtensions
- Fixed tool naming: Changed 'excel_conditional_format' to 'excel_conditionalformat' (matches naming convention)
- Enum cleanup: Removed unused BatchAction enum and its ToActionString() mapping

Test results: 60 passing (was 58), 9 failing (was 11)
Remaining failures are pre-existing issues (Power Query COM errors, invalid session handling)

* Update documentation: 12 tools with 168 operations

- Added excel_conditionalformat tool (2 actions: add-rule, clear-rules)
- Removed excel_batch from tool lists (doesn't exist)
- Updated operation counts: 166 → 168
- Updated tool counts: 11 → 12
- Added conditional formatting examples to CLI README
- Updated all READMEs: main, MCP Server, CLI, VS Code extension, GitHub pages
- Updated range action counts: 45 → 43 (moved conditional formatting to dedicated tool)
- Added excel_querytable (8 actions) to complete tool list
- Updated excel_file action count: 3 → 6 (open, save, close, create-empty, close-workbook, test)

* Delete incorrect MCP Server tests expecting exceptions

Removed tests that expected McpException for invalid sessions/missing parameters.
Per CRITICAL-RULES.md Rule 17, MCP tools should return JSON with success=false
for business errors, NOT throw exceptions.

Tests deleted:
- DetailedErrorMessageTests.cs (entire file - 8 tests)
- ExcelMcpServerTests.ExcelWorksheet_InvalidSession_ShouldThrowError

Test results: 59 passing (was 60), 2 failing (was 9)
Remaining 2 failures are legitimate Power Query COM errors (RPC failures)

* Fix ExcelBatch async disposal and timeout handling

- Replace synchronous Task.Wait() with async DisposeAsync pattern
- Add proper CancellationTokenSource disposal in finally blocks
- Use ConfigureAwait(false) for library code
- Fix timeout handling to properly cancel operations
- Add comprehensive disposal tests with timeout scenarios

* Remove ExportAsync methods - consolidate with ViewAsync

All three command types (PowerQuery, VBA, Connection) now use ViewAsync
instead of ExportAsync to return code/data directly rather than writing
to files. This simplifies the API by eliminating duplicate functionality.

Changes:
- Removed ExportAsync from IPowerQueryCommands, IVbaCommands, IConnectionCommands interfaces
- Removed ExportAsync implementations from all three command files
- Updated CLI commands to use ViewAsync instead (return code directly)
- Updated MCP Server tools to use ViewAsync instead (return JSON with code)
- Updated VBA test to use ViewAsync instead of ExportAsync
- Updated POWERQUERY-FUTURE-STATE-SPEC.md to remove ExportAsync references

Tests now verify code is returned in ViewAsync result rather than written to file.

* WIP: CLI async-to-sync conversion attempt - reverting to clean state

* Save WIP

* Align MCP tooling with updated actions

* - Refactored tools to reduce cognizant load on LLM
- Fix stability issues when working on multiple files at the same time

* Increase STA thread cleanup timeout from 3 to 10 seconds to prevent potential deadlocks during disposal

* Refactor QueryTable handling to use unified PowerQueryHelpers

- Updated QueryTableCommand to utilize PowerQueryHelpers for creating QueryTables.
- Introduced ConnectionHelpers for retrieving connection strings and command texts.
- Consolidated QueryTable creation options into PowerQueryHelpers.QueryTableCreateOptions.
- Removed redundant methods and classes related to QueryTable options.
- Enhanced error handling and validation for connection strings.
- Updated tests to reflect changes in QueryTable creation and options application.

* Revert "Refactor QueryTable handling to use unified PowerQueryHelpers"

This reverts commit 3603afcb71.

* Switch PowerQuery flows to inline M code

* Fix formula string formatting in JSON deserialization tests

* Fix test failures: formula syntax, PowerQuery Update, and QueryTable API usage

- Fixed JSON syntax in formula tests (removed extra quotes in array)
- Fixed raw string literal quote handling in Excel formulas
- Fixed PowerQuery Update to preserve worksheet data:
  * Capture QueryTable destination and ResultRange before deletion
  * Clear only ResultRange (not entire sheet)
  * Recreate at same location with clearEntireSheet: false
  * Eliminates legacy clearEntireSheet anti-pattern
- Fixed QueryTable tests to pass M code directly instead of file paths:
  * Removed file I/O operations (7 occurrences)
  * Aligned with PowerQueryCommands.Create() API signature

All 9 originally failing tests now pass (2 formula + 7 QueryTable/PowerQuery).

* Include mcp.json in version control

- Added .vscode/mcp.json to .gitignore exceptions
- Tracks MCP server configuration for project

* Add sheet movement and cross-workbook operations: Move, CopyToWorkbook, MoveToWorkbook
 2025-11-17 09:27:59 +00:00
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
 2025-11-14 16:13:22 +00:00