-
Notifications
You must be signed in to change notification settings - Fork 39
Description
🔍 Smoke Test Investigation - Run #36
Summary
The Smoke Copilot workflow failed immediately when the Copilot CLI attempted to parse the --additional-mcp-config argument. The JSON contains double-quoted environment variable values (e.g., "GH_AW_SAFE_OUTPUTS_CONFIG":""{...}""), creating invalid JSON at position 557. The CLI exits before the agent can start.
Failure Details
- Run: 18792218718
- Commit: a160185
- Branch: copilot/add-additional-mcp-server-support
- Trigger: workflow_dispatch
- Duration: 1.0 minutes
- Copilot Version: 0.0.350
Root Cause Analysis
Primary Error
Invalid JSON in --additional-mcp-config: Expected ',' or '}' after property value in JSON at position 557 (line 1 column 558)
The Smoking Gun
Looking at the generated --additional-mcp-config command (from workflow logs):
{
"mcpServers": {
"safe_outputs": {
"env": {
"GH_AW_SAFE_OUTPUTS_CONFIG": ""{\"create_issue\":{\"max\":1,\"min\":1},\"missing_tool\":{}}""
}
}
}
}Notice the doubled quotes: "GH_AW_SAFE_OUTPUTS_CONFIG":""
There are two quote characters after the colon instead of one, making this invalid JSON.
Expected vs Actual
| Component | Expected | Actual | Status |
|---|---|---|---|
| Field name | "GH_AW_SAFE_OUTPUTS_CONFIG" |
"GH_AW_SAFE_OUTPUTS_CONFIG" |
✅ |
| After colon | : |
: |
✅ |
| Value start | " (single quote) |
"" (double quote) |
❌ |
| Value content | {\"create_issue\":...} |
{\"create_issue\":...} |
✅ |
| Value end | " (single quote) |
"" (double quote) |
❌ |
Position Analysis
The error at position 557 (column 558) corresponds to the character immediately after the doubled opening quotes, where the JSON parser expects either:
- A string value character
- Or a closing quote
But instead finds { which is invalid at that position.
Failed Jobs and Errors
| Job | Status | Conclusion | Duration |
|---|---|---|---|
| activation | ✅ | success | 2s |
| agent | ❌ | failure | 26s |
| detection | ⏭️ | skipped | - |
| create_issue | ❌ | failure | 3s |
| missing_tool | ⏭️ | skipped | - |
Error Location: workflow-logs/agent/23_Execute GitHub Copilot CLI.txt:21
Full Command Context:
copilot --add-dir /tmp/ --add-dir /tmp/gh-aw/ --add-dir /tmp/gh-aw/agent/ \
--log-level all --log-dir /tmp/gh-aw/.copilot/logs/ \
--disable-builtin-mcps --allow-tool github --allow-tool safeoutputs \
--additional-mcp-config '{"mcpServers":{...}}' \ # ← Invalid JSON here
--prompt "$COPILOT_CLI_INSTRUCTION" 2>&1 | tee /tmp/gh-aw/agent-stdio.logInvestigation Findings
Environment Variable Context
The actual environment variable is correctly formatted:
GH_AW_SAFE_OUTPUTS_CONFIG='{"create_issue":{"max":1,"min":1},"missing_tool":{}}'But when this is embedded into the MCP config JSON, the quoting goes wrong:
"GH_AW_SAFE_OUTPUTS_CONFIG":""{\"create_issue\":{\"max\":1,\"min\":1},\"missing_tool\":{}}"" Root Cause
This suggests a quoting/escaping bug in the code that builds the MCP configuration JSON. When an environment variable value is itself a JSON string, the code appears to:
- ✅ Correctly escape the JSON content (
{\"create_issue\":...}) - ❌ Incorrectly add double quotes around the escaped value
Likely Code Pattern (hypothetical):
// Somewhere in pkg/workflow/mcp*.go or copilot_engine.go:
// WRONG: Adds quotes even though value already includes quotes
envValue := fmt.Sprintf(`"%s"`, escapeJSON(rawValue)) // Creates ""value""
// RIGHT: Should handle JSON-valued env vars specially
if isJSON(rawValue) {
envValue = escapeJSON(rawValue) // Already includes quotes
} else {
envValue = fmt.Sprintf(`"%s"`, escape(rawValue))
}Why This Happened on This Branch
The branch copilot/add-additional-mcp-server-support has been actively working on MCP configuration handling, including:
- Updating how env vars are passed to MCP servers
- Handling different types of env var values
- JSON escaping for shell safety
The commit message mentions "Merge main, format, recompile" - this failure appeared after workflow recompilation, suggesting the underlying Go code has the bug and recompiling exposed it.
Comparison with Related Issues
| Issue | Pattern | Error Position | Root Cause | Status |
|---|---|---|---|---|
| #2270 | Bad escaped character | Position 433 | Backslash escaping issue | Closed |
| #2280 | Malformed config in env var | N/A | Malformed GH_AW_SAFE_OUTPUTS_CONFIG | Closed |
| #2296 | Invalid schema type: local |
N/A | Schema incompatibility | Closed |
| #2267 | File path vs JSON | N/A | Claude expects file path | Closed |
| #2262 | Base64 vs JSON | N/A | Base64 when JSON expected | Closed |
| This | Doubled quotes | Position 557 | Double-quoting env var values | New |
This is a new pattern not seen in previous investigations.
Recommended Actions
Critical Priority ⚠️
-
Find the code that builds env section of MCP config
- Likely location:
pkg/workflow/mcp.goorpkg/workflow/copilot_engine.go - Search for where
GH_AW_SAFE_OUTPUTS_CONFIGis added to the JSON - Look for quote-wrapping logic
- File: Likely
pkg/workflow/mcp.goaround line 200-400
- Likely location:
-
Fix the double-quoting issue
// Identify if env var value is already JSON func isJSONValue(val string) bool { var js json.RawMessage return json.Unmarshal([]byte(val), &js) == nil } // When building env section: for key, value := range envVars { if isJSONValue(value) { // Value is JSON - it already has quotes, just escape for shell envJSON[key] = value } else { // Value is plain string - wrap in quotes envJSON[key] = fmt.Sprintf(`"%s"`, value) } }
-
Add validation test
func TestMCPConfigWithJSONEnvVars(t *testing.T) { config := buildMCPConfig(map[string]string{ "PLAIN_VAR": "simple value", "JSON_VAR": `{"key":"value"}`, }) // Parse the generated JSON to ensure it's valid var parsed map[string]interface{} err := json.Unmarshal([]byte(config), &parsed) assert.NoError(t, err, "Generated MCP config must be valid JSON") }
High Priority
-
Review all env var handling in MCP config
grep -n "env\[" pkg/workflow/*.go grep -n "EnvVar" pkg/workflow/*.go
- Check if other env vars have the same issue
- Ensure consistent handling across all MCP servers
-
Add pre-flight JSON validation
// Before passing to Copilot CLI: func validateMCPConfigJSON(config string) error { var test map[string]interface{} if err := json.Unmarshal([]byte(config), &test); err != nil { // Log the problematic JSON for debugging log.Printf("Invalid MCP config JSON: %s", config) return fmt.Errorf("MCP config is not valid JSON: %w", err) } return nil }
-
Check the git history on this branch
git log --oneline copilot/add-additional-mcp-server-support -- pkg/workflow/mcp*.go git log --oneline copilot/add-additional-mcp-server-support -- pkg/workflow/copilot*.go
- Identify when the double-quoting was introduced
- Review the PR/commit that added env var handling for MCP servers
Medium Priority
-
Add integration test with real Copilot CLI
- Create test that actually invokes
copilot --additional-mcp-config - Verify it accepts the generated JSON without errors
- Include env vars with JSON values in test cases
- Create test that actually invokes
-
Document env var value types in MCP config
## Environment Variable Handling in MCP Config When building MCP server configurations: - Plain string values: Wrap in quotes - JSON values: Do not wrap (already quoted) - Escaped values: Handle shell escaping separately from JSON quoting
-
Consider using json.Marshal for safety
// Instead of string concatenation, use Go's JSON marshaling: type MCPEnv map[string]interface{} env := MCPEnv{ "PLAIN_VAR": "value", "JSON_VAR": json.RawMessage(`{"key":"value"}`), // Preserves JSON structure } envBytes, _ := json.Marshal(env)
Prevention Strategies
-
JSON Validation Pipeline
- Validate all generated MCP config JSON before passing to CLI
- Add assertion in tests:
json.Unmarshal(generatedConfig) must succeed - Log the full JSON when validation fails for debugging
-
Type-Safe Configuration Building
- Use structs and
json.Marshal()instead of string concatenation - Define
MCPServerConfigstruct with proper field types - Let Go's JSON encoder handle all escaping
- Use structs and
-
Integration Testing
- Add smoke test that verifies CLI accepts generated configs
- Test with variety of env var value types (strings, JSON, paths, URLs)
- Run actual Copilot CLI in CI to catch config issues early
-
Code Review Checklist
- When modifying MCP config generation: "Did you test with JSON-valued env vars?"
- When adding string concatenation: "Should this use json.Marshal instead?"
- When changing quoting logic: "Is there a test for this edge case?"
Technical Details
Full MCP Config Structure (Truncated for Clarity)
{
"mcpServers": {
"github": {
"type": "local",
"command": "docker",
"args": [...],
"env": {
"GITHUB_PERSONAL_ACCESS_TOKEN": "***" // ← Plain string, correctly quoted
},
"tools": ["*"]
},
"safe_outputs": {
"type": "local",
"command": "node",
"args": ["/tmp/gh-aw/safe-outputs/mcp-server.cjs"],
"env": {
"GH_AW_SAFE_OUTPUTS": "/tmp/gh-aw/safeoutputs/outputs.jsonl", // ← Plain string, correctly quoted
"GH_AW_SAFE_OUTPUTS_CONFIG": ""{...}"" // ← JSON string, INCORRECTLY double-quoted
},
"tools": ["*"]
}
}
}Observation: Plain string env vars (like GITHUB_PERSONAL_ACCESS_TOKEN) are correctly quoted. Only the JSON-valued env var (GH_AW_SAFE_OUTPUTS_CONFIG) has the doubled quotes.
Character-by-Character Analysis at Position 557
Position: 550 555 560
...:"GH_AW_SAFE_OUTPUTS_CONFIG":""{\"create_issue\"...
↑↑
|└── Position 557: Where error detected
└─── Extra quote character
The JSON parser reads:
- Key:
"GH_AW_SAFE_OUTPUTS_CONFIG"✅ - Colon:
:✅ - Value start:
"✅ - Unexpected: Another
"❌ (position 557) - Parser expects string content, gets
{❌ - Error: Expected
,or}after property value
Historical Context
This is part of ongoing work on the copilot/add-additional-mcp-server-support branch to enhance MCP configuration support. Related patterns:
| Date | Run | Pattern | Branch | Status |
|---|---|---|---|---|
| Oct 24 05:03 | 18770131006 | Bad escaped character (pos 433) | copilot/update-copilot-agent-engine | Closed #2270 |
| Oct 24 11:07 | 18777690706 | Malformed config in env var | copilot/update-copilot-agent-engine | Closed #2280 |
| Oct 24 16:02 | 18785278805 | Invalid schema type:local | copilot/update-copilot-agent-engine | Closed #2296 |
| Oct 24 21:10 | 18792218718 | Double-quoted JSON env var | copilot/add-additional-mcp-server-support | This issue |
All issues relate to MCP configuration generation, suggesting this is a complex area that needs systematic testing and type-safe implementation.
Commit Context
Commit: a160185
Message: "Merge main, format, recompile, and fix package.json"
Author: copilot-swe-agent[bot]
Files Changed: 22 (mostly recompiled workflow lock files)
This suggests:
- The Go code was changed (either on this branch or merged from main)
- Workflows were recompiled with the updated code
- The bug exists in the current Go codebase for building MCP configs
Related Issues
- [smoke-detector] 🔍 Smoke Test Investigation - Smoke Copilot: Bad Escaped Character in MCP Config JSON #2270 - Bad escaped character in MCP config JSON (similar error location, different cause)
- [smoke-detector] 🔍 Smoke Test Investigation - Smoke Copilot: Safe-Outputs MCP Crashes Due to Malformed Config JSON #2280 - Malformed safe-outputs config in env var (related to same config field)
- [smoke-detector] 🔍 Smoke Test Investigation - Copilot CLI Rejects MCP Config with 'type': 'local' Field - Run #32 #2296 - Invalid MCP schema with type:local (schema validation issue)
- [smoke-detector] 🔍 Smoke Test Investigation - Smoke Claude: Invalid MCP Configuration Argument #2267 - Claude expects file path instead of JSON (different CLI requirements)
- [smoke-detector] 🔍 Smoke Test Investigation - Smoke Copilot: Base64 MCP Config vs JSON Expected #2262 - Base64 instead of JSON (workflow compilation issue)
Pattern ID: COPILOT_DOUBLE_QUOTED_ENV_VAR_IN_MCP_CONFIG
Severity: Critical
Category: Configuration Error - JSON Escaping
Investigation stored: /tmp/gh-aw/cache-memory/investigations/2025-10-24-18792218718.json
AI generated by Smoke Detector - Smoke Test Failure Investigator
AI generated by Smoke Detector - Smoke Test Failure Investigator