Handle malformed MCP JSON config files gracefully#14537
Handle malformed MCP JSON config files gracefully#14537mitchdenny merged 7 commits intorelease/13.2from
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14537Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14537" |
There was a problem hiding this comment.
Pull request overview
This pull request adds graceful error handling for malformed or empty JSON configuration files in the Aspire CLI's agent initialization commands (aspire mcp init and aspire agent init). Previously, encountering malformed JSON in MCP configuration files (e.g., .vscode/mcp.json) would crash the CLI with an unhandled JsonReaderException.
Changes:
- Added error handling to wrap
JsonNode.Parse()calls inApply*ConfigurationAsyncmethods across 4 agent environment scanners with descriptive error messages - Modified
AgentInitCommandto catchInvalidOperationExceptionand display user-friendly error messages while continuing with remaining applicators - Added localized error message resource string
MalformedConfigFileErrorin English with pending translations for 13 languages
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Cli/Agents/VsCode/VsCodeAgentEnvironmentScanner.cs | Added try-catch blocks around JsonNode.Parse in ApplyAspireMcpConfigurationAsync and ApplyPlaywrightMcpConfigurationAsync |
| src/Aspire.Cli/Agents/CopilotCli/CopilotCliAgentEnvironmentScanner.cs | Added try-catch blocks around JsonNode.Parse in ApplyMcpConfigurationAsync and ApplyPlaywrightMcpConfigurationAsync |
| src/Aspire.Cli/Agents/ClaudeCode/ClaudeCodeAgentEnvironmentScanner.cs | Added try-catch blocks around JsonNode.Parse in ApplyAspireMcpConfigurationAsync and ApplyPlaywrightMcpConfigurationAsync |
| src/Aspire.Cli/Agents/OpenCode/OpenCodeAgentEnvironmentScanner.cs | Added try-catch blocks around JsonNode.Parse in ApplyMcpConfigurationAsync and ApplyPlaywrightMcpConfigurationAsync |
| src/Aspire.Cli/Commands/AgentInitCommand.cs | Added try-catch for InvalidOperationException in applicator loop to display errors and continue |
| src/Aspire.Cli/Resources/AgentCommandStrings.resx | Added MalformedConfigFileError resource string |
| src/Aspire.Cli/Resources/AgentCommandStrings.Designer.cs | Generated property accessor for MalformedConfigFileError |
| src/Aspire.Cli/Resources/xlf/*.xlf (13 files) | Added MalformedConfigFileError entries marked as "new" (pending translation) |
| tests/Aspire.Cli.Tests/Agents/VsCodeAgentEnvironmentScannerTests.cs | Added 3 test cases covering malformed JSON, empty files, and file preservation |
Files not reviewed (1)
- src/Aspire.Cli/Resources/AgentCommandStrings.Designer.cs: Language not supported
🎬 CLI E2E Test RecordingsThe following terminal recordings are available for commit
📹 Recordings uploaded automatically from CI run #22322998782 |
JamesNK
left a comment
There was a problem hiding this comment.
I don’t like the duplication. There should a shared method for reading MCP file.
|
What does the user see after this change? Review showed a success message after catching error. |
When 'aspire mcp init' or 'aspire agent init' encounters an MCP config file (e.g., .vscode/mcp.json) containing empty or malformed JSON, the CLI previously crashed with an unhandled JsonReaderException. This change wraps JsonNode.Parse() calls in try-catch blocks in all four agent environment scanner Apply methods (VsCode, CopilotCli, ClaudeCode, OpenCode). On malformed JSON, a descriptive InvalidOperationException is thrown with the file path, which is caught by AgentInitCommand and displayed as a user-friendly error. The malformed file is NOT overwritten, preserving the user's content so they can fix it manually. Fixes #14394 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… files When a malformed JSON config file is encountered, now displays: - The error message identifying the file - An explicit 'Skipping update of ...' message - Returns a non-zero exit code (1) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds AgentInitCommand_WithMalformedMcpJson_ShowsErrorAndExitsNonZero which: 1. Creates a .vscode folder with a malformed mcp.json 2. Runs 'aspire agent init' and selects VS Code configuration 3. Verifies the error message about malformed JSON appears 4. Verifies the 'Skipping' message appears 5. Verifies the command exits with non-zero exit code 6. Verifies the original malformed file was NOT overwritten Also adds WaitForErrorPrompt and CreateMalformedMcpConfig helpers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The agent init command shows two multi-select prompts: 1. Agent environments (VS Code, OpenCode, Claude Code) 2. Additional options (skill files, Playwright) The test was missing handling for the second prompt, causing a timeout. Spectre.Console MultiSelectionPrompt requires at least one selection, so we select the first skill file option. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move duplicated JSON config file reading and server-check logic from 4 scanner files into a shared McpConfigFileHelper class with two methods: - HasServerConfigured: sync read + parse + check (for Has* methods) - ReadConfigAsync: async read + parse with error handling (for Apply*) Both accept an optional preprocessContent delegate for JSONC support (used by OpenCode scanner). This removes ~360 lines of duplicated boilerplate across VsCode, CopilotCli, ClaudeCode, and OpenCode scanners. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When some applicators fail due to malformed JSON config files, display a warning message 'Configuration completed with errors' instead of silently returning a non-zero exit code. The success message is only shown when all applicators succeed. Also update the E2E test to verify the new warning message is displayed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
32e1f2b to
6145644
Compare
…nners Address review feedback to add test coverage for malformed JSON handling across all agent environment scanners, not just VsCode. Each scanner now has tests for malformed JSON, empty files, and file preservation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/deployment-test |
|
🚀 Deployment tests starting on PR #14537... This will deploy to real Azure infrastructure. Results will be posted here when complete. |
|
✅ Deployment E2E Tests passed Summary: 23 passed, 0 failed, 0 cancelled Passed Tests
🎬 Terminal Recordings
|
| return new JsonObject(); | ||
| } | ||
|
|
||
| var content = await File.ReadAllTextAsync(configFilePath, cancellationToken); |
There was a problem hiding this comment.
Is there a chance of a race condition here where the file get's deleted right after the above check? Should this be wrapped in a try catch?
|
|
||
| try | ||
| { | ||
| var content = File.ReadAllText(configFilePath); |
There was a problem hiding this comment.
Same question as the one bellow. Is it possible to have a race condition here?
* Handle malformed MCP JSON config files gracefully When 'aspire mcp init' or 'aspire agent init' encounters an MCP config file (e.g., .vscode/mcp.json) containing empty or malformed JSON, the CLI previously crashed with an unhandled JsonReaderException. This change wraps JsonNode.Parse() calls in try-catch blocks in all four agent environment scanner Apply methods (VsCode, CopilotCli, ClaudeCode, OpenCode). On malformed JSON, a descriptive InvalidOperationException is thrown with the file path, which is caught by AgentInitCommand and displayed as a user-friendly error. The malformed file is NOT overwritten, preserving the user's content so they can fix it manually. Fixes #14394 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add explicit skip message and non-zero exit code for malformed config files When a malformed JSON config file is encountered, now displays: - The error message identifying the file - An explicit 'Skipping update of ...' message - Returns a non-zero exit code (1) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add E2E test for malformed MCP JSON config handling Adds AgentInitCommand_WithMalformedMcpJson_ShowsErrorAndExitsNonZero which: 1. Creates a .vscode folder with a malformed mcp.json 2. Runs 'aspire agent init' and selects VS Code configuration 3. Verifies the error message about malformed JSON appears 4. Verifies the 'Skipping' message appears 5. Verifies the command exits with non-zero exit code 6. Verifies the original malformed file was NOT overwritten Also adds WaitForErrorPrompt and CreateMalformedMcpConfig helpers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix E2E test: handle additional options prompt in agent init The agent init command shows two multi-select prompts: 1. Agent environments (VS Code, OpenCode, Claude Code) 2. Additional options (skill files, Playwright) The test was missing handling for the second prompt, causing a timeout. Spectre.Console MultiSelectionPrompt requires at least one selection, so we select the first skill file option. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Extract shared McpConfigFileHelper to reduce duplication Move duplicated JSON config file reading and server-check logic from 4 scanner files into a shared McpConfigFileHelper class with two methods: - HasServerConfigured: sync read + parse + check (for Has* methods) - ReadConfigAsync: async read + parse with error handling (for Apply*) Both accept an optional preprocessContent delegate for JSONC support (used by OpenCode scanner). This removes ~360 lines of duplicated boilerplate across VsCode, CopilotCli, ClaudeCode, and OpenCode scanners. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Display partial success warning when agent init encounters errors When some applicators fail due to malformed JSON config files, display a warning message 'Configuration completed with errors' instead of silently returning a non-zero exit code. The success message is only shown when all applicators succeed. Also update the E2E test to verify the new warning message is displayed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add malformed JSON tests for CopilotCli, ClaudeCode, and OpenCode scanners Address review feedback to add test coverage for malformed JSON handling across all agent environment scanners, not just VsCode. Each scanner now has tests for malformed JSON, empty files, and file preservation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Mitch Denny <mitch@mitchdeny.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>


Summary
When
aspire mcp initoraspire agent initencounters an MCP config file (e.g.,.vscode/mcp.json) containing empty or malformed JSON, the CLI previously crashed with an unhandledJsonReaderException.Changes
JsonNode.Parse()calls inApply*ConfigurationAsyncmethods with try-catch forJsonException. On malformed JSON, a descriptiveInvalidOperationExceptionis thrown with the file path.InvalidOperationExceptionfromApplyAsyncand displays a user-friendly error message, then continues with remaining applicators.MalformedConfigFileErrortoAgentCommandStringsfor the error message.VsCodeAgentEnvironmentScannerTestscovering malformed JSON, empty files, and verifying the file is not overwritten.Key design decision
The malformed file is NOT overwritten. The user may have valuable content they accidentally broke (e.g., a missing quote). The error message tells them which file has invalid JSON so they can fix it manually.
Fixes #14394