Files
sbroenne__mcp-server-excel/scripts/check-success-flag.ps1
T
Stefan Broenner 9ed4f6131a docs: correct tool count from 12 to 11 + fix integration test workflow (#103)
* 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>
2025-11-03 09:16:02 +00:00

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