-
Notifications
You must be signed in to change notification settings - Fork 0
Fix GitHub workflow configuration mismatches and add prevention guidelines #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…aming Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com>
sbroenne
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot can you update your coding instructions to make sure this never happens again
…ync issues Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com>
Updated The new section includes:
This ensures workflows stay synchronized with project configuration during future upgrades or rebranding efforts. |
* Add Excel diagnostics and helper classes for enhanced error reporting and automation - Implemented ExcelDiagnostics class for comprehensive error reporting, including detailed context for Excel operations. - Added ExcelHelper class to manage Excel COM automation with improved resource management and security checks. - Created project file for ExcelMcp.CLI with necessary dependencies and metadata. - Developed main program logic for command-line interface, including argument validation and command handling for various Excel operations. - Included detailed help and usage instructions for users, along with examples and requirements. * Add integration and unit tests for ExcelMcp CLI and MCP Server - Implemented integration tests for ScriptCommands and SheetCommands to validate VBA script operations and worksheet manipulations. - Created unit tests for argument validation and Excel file checks in the ExcelMcp CLI. - Added project files for test projects with necessary dependencies. - Developed integration tests for ExcelMcp Server tools to ensure proper functionality of resource-based commands. - Included cleanup logic in tests to manage temporary files and directories. * Update documentation for .NET 10 compatibility and workflow changes * Remove build and release workflows from GitHub Actions * Refine release strategy documentation for MCP Server, CLI, and combined releases * Apply Sbroenne prefix to all project namespaces, simplify CLI executable name, and implement .NET best practices (#1) * Initial plan * Apply Sbroenne prefix to all project namespaces and solution file - Rename ExcelMcp.sln to Sbroenne.ExcelMcp.sln - Update all CLI namespaces from ExcelMcp.* to Sbroenne.ExcelMcp.CLI.* - Add AssemblyName and RootNamespace properties to all projects - Fix using statements in CLI to reference correct namespaces - Update Program.cs to use CLI Commands instead of Core Commands Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Fix old repository and product naming references - Update Directory.Build.props: Product name from ExcelCLI to Sbroenne.ExcelMcp - Update Directory.Build.props: Repository URLs from ExcelCLI to mcp-server-excel - Update Program.cs: Repository links and branding from ExcelCLI to ExcelMcp.CLI - Update documentation: Replace ExcelCLI with Sbroenne.ExcelMcp/ExcelMcp.CLI - Update workflows: Fix executable name and help text validation - Update CONTRIBUTING, SECURITY, CLI, AUTHOR, COPILOT docs Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Complete project cleanup and NuGet best practices - Remove legacy ExcelMcp.Tests folder (not in solution) - Update TEST_GUIDE.md with correct test structure - Add PackageReadmeFile to CLI project (includes CLI.md) - Fix .NET version reference from 8.0 to 10.0 in Program.cs - Fix release workflow paths: net8.0 → net10.0 - Fix release workflow DLL names with Sbroenne prefix - Ensure all NuGet metadata follows best practices Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Add comprehensive naming review summary documentation - Document all naming convention changes - Provide migration notes for developers - List all best practices compliance items - Include before/after examples - Summary statistics of changes made Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Simplify CLI executable name to 'excelcli.exe' - Change AssemblyName from Sbroenne.ExcelMcp.CLI to excelcli - Update build-cli.yml workflow for new executable name - Update release-cli.yml with excelcli.exe references - Update all documentation (INSTALLATION, CLI, DEVELOPMENT, etc.) - Update Program.cs usage examples to use 'excelcli' command - Update NAMING_REVIEW_SUMMARY.md with new executable name Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Apply .NET best practices improvements - Add XML documentation generation to Core library - Add version information to Core library (2.0.0) - Enable deterministic builds for reproducible binaries - Enable package validation for NuGet packages (CLI and MCP Server) - Create comprehensive DOTNET_BEST_PRACTICES_REVIEW.md document All changes maintain backward compatibility and enhance professional quality. Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Update all documentation for simplified CLI executable name - Update CLI.md: Change zip file name and path references to excelcli - Update COMMANDS.md: All 80+ command examples now use 'excelcli' command - Update excel-powerquery-vba-copilot-instructions.md: All command examples updated - Update .github/copilot-instructions.md: All command examples and workflows updated All documentation now consistently references 'excelcli' as the executable name (156 command references updated). Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Refocus excel-powerquery-vba-copilot-instructions.md on MCP server - Changed from CLI command examples to MCP server conversational workflows - Updated title and introduction to focus on AI-assisted development - Replaced CLI commands with 6 resource-based MCP tools (excel_file, excel_powerquery, excel_worksheet, excel_parameter, excel_cell, excel_vba) - Added conversational workflow examples (Power Query refactoring, VBA enhancement, etc.) - Updated use cases to emphasize AI development vs scripted automation - Added MCP server installation and configuration instructions - Added example prompts for GitHub Copilot integration - Clarified distinction between MCP server (development) and CLI (automation) The file now properly guides users on using the MCP server for AI-powered Excel development workflows. Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Remove intermediary documentation files - Remove NAMING_REVIEW_SUMMARY.md (documented changes during PR, no longer needed) - Remove DOTNET_BEST_PRACTICES_REVIEW.md (review completed, current state documented elsewhere) - Remove .github/BRANCH_PROTECTION.md (setup guide, not needed for current state) - Remove .github/CODEQL_SETUP_FIX.md (troubleshooting doc, issue resolved) - Remove .github/SECURITY_SETUP.md (setup guide, security already configured) All removed files documented the transition process but aren't needed to document the current project state. Essential documentation remains: - README.md (project overview and quick start) - SECURITY.md (security policy) - docs/CONTRIBUTING.md (contribution guidelines) - docs/CLI.md, docs/COMMANDS.md (usage documentation) - .github/copilot-instructions.md (AI assistant integration) Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Add XML documentation for command interfaces and implementations * Fix GitHub workflow configuration mismatches and add prevention guidelines (#2) * Initial plan * Fix GitHub workflow configuration issues - .NET version and package naming Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Add GitHub workflows configuration management guidelines to prevent sync issues Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com> * Enhance GitHub workflows: update CLI and MCP Server build paths, improve test output messages, and adjust dependency review settings * Update MCP Server and CLI project files for improved architecture support and documentation clarity
Fixed two critical bugs in PowerQuery operations when column structure changes: Bug #1: Column structure doesn't update when query M code changes - QueryTable.PreserveColumnInfo was preventing column updates - Changed default to false in PowerQueryHelpers.cs Bug #2: Columns accumulate when using delete+recreate workflow - Deleting QueryTable left data on worksheet causing visual concatenation - Added worksheet clearing logic before creating new QueryTables Implementation: - PowerQueryHelpers.cs: Changed PreserveColumnInfo = false (was true) - PowerQueryHelpers.cs: Added RemoveQueryTablesFromSheet() helper method - PowerQueryCommands.Lifecycle.cs: Enhanced UpdateAsync with complete fix - Delete existing QueryTables - Clear worksheet data (prevents accumulation) - Create fresh QueryTable with new column structure - PowerQueryCommands.LoadConfig.cs: Enhanced SetLoadToTableAsync with same fix - Delete existing QueryTables - Clear worksheet data (prevents accumulation) - Create fresh QueryTable Tests: - Added 2 regression tests validating both bugs are fixed - Test 1: Update_QueryColumnStructure_UpdatesWorksheetColumns (UpdateAsync path) - Test 2: Update_QueryColumnStructureWithDeleteRecreate_AccumulatesColumns (SetLoadToTableAsync path) - Both tests passing - validates fix works across both code paths Closes #133
Fixed two critical bugs in PowerQuery operations when column structure changes: Bug #1: Column structure doesn't update when query M code changes - QueryTable.PreserveColumnInfo was preventing column updates - Changed default to false in PowerQueryHelpers.cs Bug #2: Columns accumulate when using delete+recreate workflow - Deleting QueryTable left data on worksheet causing visual concatenation - Added worksheet clearing logic before creating new QueryTables Implementation: - PowerQueryHelpers.cs: Changed PreserveColumnInfo = false (was true) - PowerQueryHelpers.cs: Added RemoveQueryTablesFromSheet() helper method - PowerQueryCommands.Lifecycle.cs: Enhanced UpdateAsync with complete fix - Delete existing QueryTables - Clear worksheet data (prevents accumulation) - Create fresh QueryTable with new column structure - PowerQueryCommands.LoadConfig.cs: Enhanced SetLoadToTableAsync with same fix - Delete existing QueryTables - Clear worksheet data (prevents accumulation) - Create fresh QueryTable Tests: - Added 2 regression tests validating both bugs are fixed - Test 1: Update_QueryColumnStructure_UpdatesWorksheetColumns (UpdateAsync path) - Test 2: Update_QueryColumnStructureWithDeleteRecreate_AccumulatesColumns (SetLoadToTableAsync path) - Both tests passing - validates fix works across both code paths Closes #133 Co-authored-by: Stefan Broenner <stefan.broenner@microsoft.comm>
…ccumulation Co-authored-by: sbroenne <3026464+sbroenne@users.noreply.github.com>
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)
* 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)
Problem
GitHub workflows were not executing correctly due to configuration mismatches from the namespace rebrand (
ExcelMcp→Sbroenne.ExcelMcp) and .NET version upgrade (8 → 10). While the path filters were correct, four critical configuration issues prevented workflows from building and releasing successfully.Issues Fixed
1. .NET SDK Version Mismatch (CRITICAL)
Three workflows were configured to use .NET 8.0.x while the project requires .NET 10.0.x:
Files affected:
.github/workflows/release-cli.yml.github/workflows/codeql.yml.github/workflows/publish-nuget.ymlImpact: Build workflows would fail immediately with SDK version errors since .NET 8 SDK cannot compile projects targeting net10.0.
2. Incorrect Executable Name in Build Verification
The MCP Server build workflow was checking for the wrong executable name:
File affected:
.github/workflows/build-mcp-server.ymlImpact: Build verification would always fail even when builds succeeded, creating false negatives.
3. Wrong NuGet Package Name References
Package operations used the old package ID instead of the rebranded name:
Files affected:
.github/workflows/publish-nuget.yml(5 references).github/workflows/release-mcp-server.yml(3 references)Impact: Package verification and publish steps would fail, and users would receive incorrect installation instructions.
4. Outdated Runtime Requirements in Documentation
Release notes claimed ".NET 8.0 runtime" while the project actually requires ".NET 10.0 runtime":
Files affected:
.github/workflows/release-cli.yml(2 locations).github/workflows/release-mcp-server.yml(2 locations)Impact: Users would install the wrong runtime version and experience installation failures.
5. Prevention Guidelines Added
Added comprehensive GitHub Workflows Configuration Management section to
.github/copilot-instructions.mdto prevent similar issues in the future:These guidelines ensure workflows stay synchronized with project configuration during future upgrades or rebranding efforts.
Root Cause
The project underwent two major changes:
ExcelMcp→Sbroenne.ExcelMcpWhile the codebase (.csproj files, Assembly names, Package IDs) was fully updated, the CI/CD workflow configurations were not completely synchronized with these changes.
Changes Summary
Verification
✅ All workflows now use .NET 10.0.x SDK
✅ Executable name matches csproj AssemblyName (
Sbroenne.ExcelMcp.McpServer)✅ Package name matches csproj PackageId (
Sbroenne.ExcelMcp.McpServer)✅ Runtime requirements correctly state .NET 10.0
✅ Path filters verified correct (were never the issue)
✅ Prevention guidelines added to copilot-instructions.md
Expected Results
After merging, workflows will:
Plus: Future configuration changes will have clear guidelines and validation scripts to prevent similar synchronization issues.
Testing Recommendations
Created from VS Code via the GitHub Pull Request extension.
Original prompt
please review alll my github workflows - i think they are using the wrong paths and this is why they do not fire
[Chronological Review: The conversation began with the user requesting to rename the project to "ExcelMcp.CLI" and update all documentation accordingly. The user then asked to ensure that the README focused on the MCP server, with CLI details in a separate file. The user expressed a desire to have different build and release processes for the MCP Server and CLI, and to split installation instructions accordingly. The user later specified a preference for using a specific approach for the MCP Server, including prerequisites for .NET 10 SDK and installation via winget. The user requested direct updates to files, avoiding the use of Get-ChildItem, and raised a question about naming conventions for solutions and namespaces. The user noted that some documentation was out of date and requested a review of the project for outdated links and installation instructions. The conversation continued with the user reporting build errors and requesting the addition of XML documentation. The user then asked for a review of GitHub workflows, suspecting incorrect paths were causing them not to fire.][Intent Mapping:
[Technical Inventory:
[Code Archaeology:
[Progress Assessment:
[Context Validation: All critical information for continuation is captured, including user requests for documentation updates, project renaming, and workflow adjustments.]
[Recent Commands Analysis:
1. Conversation Overview: - Primary Objectives: The user aims to rename the project to "ExcelMcp.CLI," update documentation, ensure proper build and release processes for MCP Server and CLI, and address build errors. - Session Context: The conversation has evolved from initial renaming requests to detailed discussions about documentation, workflows, and best practices. - User Intent Evolution: The user has shifted from broad project renaming to specific concerns about documentation accuracy, build processes, and adherence to naming conventions.- Technical Foundation:
- .NET 10 SDK: Required as a prerequisite for the MCP Server.
- GitHub Actions: Used for CI/CD workflows, with a focus on separating processes for MCP Server and CLI.
- Codebase Status:
- d:\source\mcp-server-excel.github\workflows\build-mcp-server.yml:
- Purpose: Defines the build process for the MCP Server.
- Current State: Needs review for path correctness and separation of build processes.
- d:\source\mcp-server-excel\docs\COMMANDS.md:
- Purpose: Contain...
Created from VS Code via the GitHub Pull Request extension.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.