CtrlK
BlogDocsLog inGet started
Tessl Logo

project-conventions

Core conventions and patterns for ExcelMcp

46

Quality

32%

Does it follow best practices?

Impact

Pending

No eval scenarios have been run

SecuritybySnyk

Passed

No known issues

Optimize this skill with Tessl

npx tessl skill review --optimize ./.squad/skills/project-conventions/SKILL.md
SKILL.md
Quality
Evals
Security

Context

ExcelMcp is a Windows-only toolset for programmatic Excel automation via COM interop, with TWO equal entry points: MCP Server (for AI assistants) and CLI (for scripting/agents). .NET/C# codebase.

Patterns

Exception Propagation (CRITICAL)

Core commands NEVER wrap batch.Execute() in try-catch that returns error results. Let exceptions propagate naturally — batch.Execute() handles them via TaskCompletionSource.

// ✅ CORRECT
return await batch.Execute((ctx, ct) => {
    // operation
    return ValueTask.FromResult(new OperationResult { Success = true });
});

// ❌ WRONG — double-wraps, loses stack context
try { return await batch.Execute(...); }
catch (Exception ex) { return new OperationResult { Success = false, ErrorMessage = ex.Message }; }

COM Object Cleanup

ALL dynamic COM objects must be released in finally blocks using ComUtilities.Release(ref obj!). NEVER use catch blocks to swallow exceptions.

Success Flag Invariant

Success == trueErrorMessage == null || ErrorMessage == "". Set Success in try block, always false in catch.

Error Handling

  • Core Commands: Let exceptions propagate through batch.Execute()
  • MCP Server: Return JSON with isError: true for business errors; throw McpException for validation
  • CLI: Wrap Core calls in try-catch, display with AnsiConsole.MarkupLine
  • When evolving failure envelopes, preserve the legacy error field for compatibility and add richer diagnostics additively (errorMessage, isError, exceptionType, hresult, innerError) instead of renaming contracts in place.
  • If CLI/MCP parity needs richer diagnostics, add them to the shared ServiceResponse transport first so both entry points receive the same failure detail.

MCP Schema Discoverability

  • For hand-written MCP tools, add [Description("...")] attributes to each exposed parameter that needs to appear in the published schema.
  • XML /// <param> comments are not enough by themselves for MCP client discoverability; verify the live schema with an integration test that calls ListToolsAsync().
  • When fixing LLM discoverability bugs, preserve runtime aliases if needed, but make the canonical parameter pair explicit in both the parameter descriptions and the shared skill doc.

Testing

  • Framework: xUnit with integration tests ONLY (no unit tests)
  • Test location: tests/ExcelMcp.Core.Tests/, tests/ExcelMcp.McpServer.Tests/, tests/ExcelMcp.CLI.Tests/
  • Run command: dotnet test tests/ExcelMcp.Core.Tests --filter "Feature=<name>&RunType!=OnDemand"
  • TDD: Write test FIRST → RED → implement → GREEN
  • NEVER share test files between tests — each test creates unique files
  • ALWAYS verify actual Excel state, not just success flags (round-trip validation)
  • For range write bugs, test payload shape explicitly: rectangular wide writes, jagged rows, and create-sheet-then-write-non-A1 flows. Don’t infer COM limits from jagged input failures.
  • When hardening diagnostics regressions, centralize failure-envelope assertions in the shared test harness so every regression checks the same contract: success=false, isError=true, error == errorMessage, expected exceptionType, and scenario-specific presence/absence of errorCategory, hresult, and innerError.
  • For CLI parity tests, assert exit code 1 on business-error paths (for example, missing sheet or invalid input). If the setup command itself returns non-JSON stdout, treat that as startup/harness noise and surface raw stdout/stderr in the helper exception instead of misclassifying it as a contract failure.

Bug Report Triage For Tests

  • Check the live tool surface before treating a report as a missing-feature bug; verify ServiceAction coverage and current MCP tool docs first.
  • Classify each report item before writing tests: regression in promised behavior, discoverability/documentation gap, or new feature request.
  • Regressions get exact failing workflow tests first at Core and MCP layers.
  • Existing capabilities with weak discoverability get positive end-to-end coverage before any API expansion.
  • New features get acceptance tests only after the public API shape is agreed.

Code Style

  • Analyzer: TreatWarningsAsErrors=true with .NET analyzers
  • Naming: PascalCase for public, camelCase for params → auto-converts to snake_case in MCP
  • One public class per file, file name = class name
  • Partial classes for 15+ methods (split by feature domain)
  • No emojis in LLM-consumed content (XML docs, skill .md files)

File Structure

src/ExcelMcp.ComInterop/  — COM patterns, STA threading, sessions
src/ExcelMcp.Core/         — Excel business logic, commands
src/ExcelMcp.Service/      — Session management, command routing
src/ExcelMcp.McpServer/    — MCP protocol tools
src/ExcelMcp.CLI/          — Command-line interface
src/ExcelMcp.Generators*/  — Source generators
tests/                     — Integration tests (no unit tests)
skills/shared/             — Single source of truth for docs/prompts

Formatting Surface Split

Before classifying a formatting bug as missing functionality, check both range and range_format.

  • range owns value/formula-adjacent display formats such as set-number-format
  • range_format owns visual styling and layout actions such as format-range, auto-fit-columns, and auto-fit-rows

If the capability already exists under one of those tools, treat it as a discoverability/API-shape issue first, not a backend feature gap.

For real batching work, prefer existing list-of-objects patterns over ad hoc JSON blobs. IRangeEditCommands.Sort(... List<SortColumn> ...) is a precedent that the shared surface can carry structured collections cleanly.

Anti-Patterns

  • Unit tests — NEVER write unit tests. Integration tests only for COM interop.
  • RefreshAll() — NEVER use. Use individual queryTable.Refresh(false) (synchronous).
  • Catch-and-swallow — NEVER catch exceptions in Core commands to return error results.
  • Dual test fixtures — NEVER use both IClassFixture<T> AND [Collection("...")].
  • Manual ScreenUpdating — ExcelWriteGuard handles this automatically.
  • Suppressing EnableEvents — Data Model operations depend on them.
  • Assuming Excel has a hidden range-width cap — if set-values fails with ArgumentOutOfRangeException, inspect for jagged List<List<object?>> input before blaming COM.

Triage Pattern

Before assigning a bug to Core, check three things in order:

  1. Existing integration coverage for the exact shape or a close analogue.
  2. Whether the capability already exists under a different tool or action name.
  3. Whether the failure is more likely in MCP/service argument binding, docs/skills discoverability, or true COM/Core behavior.

Use this especially for reports that claim a hard product limit or a missing feature. Wide-range failures and formatting gaps are often mis-triaged when tests or tool surfaces already cover the scenario elsewhere.

Repository
sbroenne/mcp-server-excel
Last updated
Created

Is this your skill?

If you maintain this skill, you can claim it as your own. Once claimed, you can manage eval scenarios, bundle related skills, attach documentation or rules, and ensure cross-agent compatibility.