mirror of
https://github.com/sbroenne/mcp-server-excel.git
synced 2026-09-19 07:53:08 +08:00
9ed4f6131a
* Rename Core methods to match MCP enum names and add naming check to audit script * Add naming consistency check to audit script with documented exceptions * Add Success flag violation detection and fix remaining violation (Rule 0) - Created scripts/check-success-flag.ps1 for automated detection - Created scripts/fix-success-flags.ps1 for automated fixes - Updated pre-commit.ps1 to run Success flag check (5th check) - Enhanced CRITICAL-RULES.md Rule 0 with detailed patterns - Fixed last violation in DataModelCommands.Refresh.cs - Created SUCCESS-FLAG-VIOLATIONS-PREVENTION.md documentation Enforcement: Pre-commit hook now blocks commits with Success=true + ErrorMessage violations * Fix hyperlink address assertion to include trailing slash for URL normalization * Remove outdated documentation files for MCP exception handling, success flag violations, formatting guidance, and prevention strategies * Update README files to enhance descriptions and clarify features of ExcelMcp, including expanded tool capabilities and improved examples for user guidance. * Remove console output from fixture initialization in test classes for cleaner test execution * docs: correct tool count from 12 to 11 across all documentation - Update architecture-patterns.instructions.md with accurate 11-tool list - Fix tool names: excel_batch (not begin/commit as separate), excel_namedrange (not excel_parameter) - Add missing excel_pivottable to instructions - Update all 3 READMEs (main, MCP Server, VS Code extension) to say '11 specialized tools' - Remove incorrect '12. Total: 12 tools' line, replace with 'Total: 11 tools with 166 actions' Actual tools verified from code: 1. excel_batch, 2. excel_connection, 3. excel_datamodel, 4. excel_file 5. excel_namedrange, 6. excel_pivottable, 7. excel_powerquery, 8. excel_range 9. excel_table, 10. excel_vba, 11. excel_worksheet * fix: add git safe.directory config for self-hosted runner Fixes 'dubious ownership' error in integration-tests workflow when dorny/test-reporter tries to run git ls-files on self-hosted Azure VM. The repository is checked out by GitHub Actions runner service (NETWORK SERVICE) but workflow steps run as azureuser, causing Git to reject operations. Solution: Add git config --global --add safe.directory step after checkout. --------- Co-authored-by: Stefan Broenner <stefan.broenner@microsoft.comm>
104 lines
3.9 KiB
PowerShell
104 lines
3.9 KiB
PowerShell
#!/usr/bin/env pwsh
|
|
<#
|
|
.SYNOPSIS
|
|
Checks for Success = true with ErrorMessage violations (Rule 0)
|
|
|
|
.DESCRIPTION
|
|
Scans Core Commands for the critical bug pattern:
|
|
- result.Success = true (set optimistically)
|
|
- catch block sets result.ErrorMessage
|
|
- but FORGETS to set result.Success = false
|
|
|
|
This causes Success=true responses with error messages, confusing LLMs.
|
|
|
|
.EXAMPLE
|
|
.\check-success-flag.ps1
|
|
|
|
.NOTES
|
|
Part of pre-commit validation. See CRITICAL-RULES.md Rule 0.
|
|
#>
|
|
|
|
$ErrorActionPreference = "Stop"
|
|
$rootDir = Split-Path -Parent $PSScriptRoot
|
|
|
|
Write-Host "🔍 Checking for Success = true with ErrorMessage violations (Rule 0)..." -ForegroundColor Cyan
|
|
Write-Host ""
|
|
|
|
$violations = @()
|
|
|
|
# Scan all Core Commands files
|
|
Get-ChildItem -Path "$rootDir\src\ExcelMcp.Core\Commands" -Filter "*.cs" -Recurse | ForEach-Object {
|
|
$lines = Get-Content $_.FullName
|
|
|
|
for ($i = 0; $i -lt $lines.Count; $i++) {
|
|
# Look for Success = true
|
|
if ($lines[$i] -match '\.Success\s*=\s*true') {
|
|
$successLine = $i + 1
|
|
|
|
# Check next 30 lines for ErrorMessage = "something" (not null/empty)
|
|
for ($j = $i + 1; $j -lt [Math]::Min($i + 30, $lines.Count); $j++) {
|
|
# Skip if we hit another Success assignment
|
|
if ($lines[$j] -match '\.Success\s*=') {
|
|
break
|
|
}
|
|
|
|
# Found ErrorMessage being set to non-null value
|
|
if ($lines[$j] -match '\.ErrorMessage\s*=\s*["\$]' -and
|
|
$lines[$j] -notmatch '= null' -and
|
|
$lines[$j] -notmatch '= string.Empty' -and
|
|
$lines[$j] -notmatch '= ""') {
|
|
|
|
$errorLine = $j + 1
|
|
|
|
# Check if Success is set to false between these lines
|
|
$hasSuccessFalse = $false
|
|
for ($k = $i + 1; $k -lt $j; $k++) {
|
|
if ($lines[$k] -match '\.Success\s*=\s*false') {
|
|
$hasSuccessFalse = $true
|
|
break
|
|
}
|
|
}
|
|
|
|
if (-not $hasSuccessFalse) {
|
|
$violations += [PSCustomObject]@{
|
|
File = $_.FullName.Replace("$rootDir\", "")
|
|
SuccessLine = $successLine
|
|
ErrorMessageLine = $errorLine
|
|
SuccessCode = $lines[$i].Trim()
|
|
ErrorCode = $lines[$j].Trim()
|
|
}
|
|
}
|
|
break
|
|
}
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
# Report results
|
|
if ($violations.Count -eq 0) {
|
|
Write-Host "✅ No violations found - all Success flags match reality!" -ForegroundColor Green
|
|
exit 0
|
|
}
|
|
|
|
Write-Host "❌ Found $($violations.Count) Rule 0 violations!" -ForegroundColor Red
|
|
Write-Host ""
|
|
Write-Host "Pattern: Success = true followed by ErrorMessage = ... without Success = false" -ForegroundColor Yellow
|
|
Write-Host ""
|
|
|
|
foreach ($v in $violations) {
|
|
Write-Host "❌ $($v.File)" -ForegroundColor Red
|
|
Write-Host " Line $($v.SuccessLine): $($v.SuccessCode)" -ForegroundColor Yellow
|
|
Write-Host " Line $($v.ErrorMessageLine): $($v.ErrorCode)" -ForegroundColor Yellow
|
|
Write-Host " → Missing: result.Success = false; before ErrorMessage" -ForegroundColor Magenta
|
|
Write-Host ""
|
|
}
|
|
|
|
Write-Host "Action Required:" -ForegroundColor Yellow
|
|
Write-Host " 1. In each catch block, add: result.Success = false;" -ForegroundColor Yellow
|
|
Write-Host " 2. Or move: result.Success = true; to INSIDE try block (after work succeeds)" -ForegroundColor Yellow
|
|
Write-Host " 3. See CRITICAL-RULES.md Rule 0 for pattern examples" -ForegroundColor Yellow
|
|
Write-Host ""
|
|
Write-Host "See: .github/instructions/CRITICAL-RULES.md Rule 0" -ForegroundColor Gray
|
|
|
|
exit 1 |