[smoke-detector] 🔍 Smoke Test Investigation - Smoke Copilot: Bad Escaped Character in MCP Config JSON

# 🔍 Smoke Test Investigation - Run #18770131006

## Summary
The Smoke Copilot workflow failed immediately when the Copilot CLI attempted to parse the `--additional-mcp-config` argument. The CLI received **malformed JSON with bad escaped characters** at position 433 (line 20 column 43), causing it to exit with a JSON parsing error before any agent execution could begin.

## Failure Details
- **Run**: [#18770131006](https://github.com/githubnext/gh-aw/actions/runs/18770131006)
- **Commit**: b223cc90c47a038af4e74c5a5813d4a9f14175bc
- **Branch**: copilot/update-copilot-agent-engine
- **Trigger**: workflow_dispatch
- **Duration**: 1.0 minutes
- **Failed Job**: agent (24s duration)

## Root Cause Analysis

### Primary Error
```
Invalid JSON in --additional-mcp-config: Bad escaped character in JSON at position 433 (line 20 column 43)
```

The error occurred when the Copilot CLI attempted to parse the inline JSON passed via `--additional-mcp-config`. Unlike issue #2267 (Claude expecting file path) or #2262 (base64 vs JSON), this is a **JSON escaping error** within the JSON string itself.

### Investigation Findings

**The Problem**: Invalid JSON Escaping in MCP Config

The commit b223cc90c47a038af4e74c5a5813d4a9f14175bc updated tests for `--additional-mcp-config`, but the actual MCP config JSON being generated contains invalid escape sequences that cause the Copilot CLI's JSON parser to fail.

| Component | Expected | Actual | Status |
|-----------|----------|--------|--------|
| Copilot --additional-mcp-config | Valid JSON string | JSON with bad escape at pos 433 | ❌ Invalid |
| JSON Parser | Properly escaped characters | Bad escaped character | ❌ Parse Error |

**No Agent Logs Generated**: The agent did not produce any log files because the Copilot CLI failed during initialization before the agent could execute:
- Directory checked: `/tmp/gh-aw/.copilot/logs/`
- Result: "No log files found in directory"
- This confirms the CLI crashed before agent startup

### Error Location Analysis

From the log at `/tmp/gh-aw/aw-mcp/logs/run-18770131006/agent-stdio.log/agent-stdio.log`:
```
Invalid JSON in --additional-mcp-config: Bad escaped character in JSON at position 433 (line 20 column 43)
```

**Position 433, line 20 column 43** suggests the error is in the environment variable section of the MCP config JSON, likely in the `GITHUB_PERSONAL_ACCESS_TOKEN` or similar field.

## Failed Jobs and Errors

### Job Sequence
1. ✅ **activation** - succeeded (2s)
2. ❌ **agent** - failed (24s) - Copilot CLI couldn't parse malformed JSON
3. ❌ **create_issue** - failed (4s) - Dependent on agent job
4. ⏭️ **missing_tool** - skipped
5. ⏭️ **detection** - skipped

### Error Details

**Location**: `/tmp/gh-aw/agent-stdio.log:1`

```
Invalid JSON in --additional-mcp-config: Bad escaped character in JSON at position 433 (line 20 column 43)
```

**Context**: The Copilot CLI validates the `--additional-mcp-config` argument immediately upon startup. When it encounters invalid JSON (bad escape sequence), it exits with error code 1 before any agent execution.

## Commit Analysis

**Commit**: b223cc90c47a038af4e74c5a5813d4a9f14175bc  
**Author**: copilot-swe-agent[bot]  
**Co-authored-by**: pelikhan  
**Message**: "Update tests for --additional-mcp-config argument"

**Description from commit**:
- Fix TestCopilotEngineRenderMCPConfigWithGitHub to check execution steps
- Fix TestCopilotEngineRenderMCPConfigWithGitHubAndPlaywright
- Update HTTP MCP integration tests to check --additional-mcp-config
- All copilot-specific tests are now passing

**Key Observation**: The tests pass but the actual workflow fails. This suggests the test fixtures may not accurately reflect the real MCP config JSON being generated in production workflows.

## Comparison with Related Issues

| Issue | Engine | Branch | Problem | Root Cause |
|-------|--------|--------|---------|------------|
| #2262 | Copilot | copilot/update-copilot-engine-cli-config | Expected JSON, got base64 | Workflow not recompiled |
| #2267 | Claude | copilot/update-engines-cli-arguments | Expected file path, got JSON | Different CLI semantics |
| **This** | **Copilot** | **copilot/update-copilot-agent-engine** | **Valid JSON expected, got bad escaping** | **Invalid escape sequence in JSON** |

All three issues are related to MCP configuration handling but have different root causes on different branches.

## Recommended Actions

### Critical Priority
- [ ] **Investigate the validateAndCompactJSON function**
  - Location: `pkg/workflow/engine_shared_helpers.go:387-411`
  - Recent changes removed backslash escaping before `$` for env vars
  - This may have created invalid JSON escape sequences
  - **Rationale**: This function is the most likely source of the bad escaping

- [ ] **Identify what's at position 433 in the generated JSON**
  ```bash
  # Debug by logging the full JSON before passing to CLI
  echo "$MCP_JSON" | head -c 500 | tail -c 100
  ```
  - Determine which field contains the bad escape
  - **Rationale**: Need to know exactly what's malformed

- [ ] **Add JSON validation before passing to Copilot CLI**
  ```go
  // In copilot_engine.go, before executing CLI:
  if _, err := json.Marshal(mcpConfig); err != nil {
      return fmt.Errorf("invalid MCP config JSON: %w", err)
  }
  ```
  - **Rationale**: Fail fast with clear error message

### High Priority
- [ ] **Review escaping logic for environment variables**
  - The `validateAndCompactJSON` function skips backslashes before `$`
  - This works for shell quoting but may create invalid JSON
  - Need separate handling: shell escaping vs JSON escaping
  - **Rationale**: Conflating shell and JSON escaping causes issues

- [ ] **Add integration tests with real environment variable patterns**
  ```go
  func TestCopilotMCPConfigWithEnvVars(t *testing.T) {
      // Test with: ${GITHUB_TOKEN}, \${ESCAPED}, etc.
      // Verify JSON is valid AND shell-safe
  }
  ```
  - **Rationale**: Current tests don't catch this escaping issue

- [ ] **Compare test fixtures with production workflow JSON**
  - Extract actual MCP config from failing workflow
  - Compare with test fixture JSON
  - Identify differences
  - **Rationale**: Tests pass but production fails = fixture mismatch

### Medium Priority
- [ ] **Consider using heredoc or file-based approach for MCP config**
  ```bash
  cat > /tmp/mcp-config.json <<'EOF'
  {JSON_HERE}
  EOF
  copilot --additional-mcp-config "$(cat /tmp/mcp-config.json)"
  ```
  - Eliminates shell escaping concerns
  - **Rationale**: Simpler, more reliable than inline JSON

- [ ] **Document JSON escaping requirements**
  - Explain difference between shell escaping and JSON escaping
  - Document how validateAndCompactJSON handles both
  - **Rationale**: Prevent similar issues in the future

## Prevention Strategies

1. **JSON Validation in CI**: Add pre-flight check that parses generated JSON
   ```go
   // Before running Copilot CLI:
   var test map[string]interface{}
   if err := json.Unmarshal([]byte(mcpJSON), &test); err != nil {
       return fmt.Errorf("MCP config is not valid JSON: %w", err)
   }
   ```

2. **Integration Tests with Real Workflows**: Run smoke tests in PR CI when engine code changes

3. **Separate Shell and JSON Escaping**:
   - Use Go's `json.Marshal()` for JSON correctness
   - Use shell escaping only for shell safety
   - Don't conflate the two concerns

4. **Log Generated JSON**: Add debug logging of the full MCP config JSON before passing to CLI for easier debugging

## Technical Details

### Expected Behavior
```bash
copilot --additional-mcp-config '{
  "mcpServers": {
    "github": {
      "env": {
        "GITHUB_PERSONAL_ACCESS_TOKEN": "${GITHUB_MCP_SERVER_TOKEN}"
      }
    }
  }
}'
```
Valid JSON with properly escaped characters.

### Actual Behavior
```bash
copilot --additional-mcp-config '{JSON with bad escape at position 433}'
```
JSON parser fails due to invalid escape sequence.

### Likely Culprit Code

From `engine_shared_helpers.go` (based on issue #2267's context):
```go
// Look ahead - if next char is $, skip the backslash (for env var refs)
if i+1 < len(jsonStr) && jsonStr[i+1] == '$' && inString {
    // Skip the backslash - we're in a single-quoted context so no escaping needed
    continue
}
```

This code removes backslashes before `$` signs, which may be creating invalid JSON when the JSON needs those backslashes for proper escaping.

## Pattern Storage

**Investigation saved to**: `/tmp/gh-aw/cache-memory/investigations/2025-10-24-18770131006.json`  
**Pattern ID**: COPILOT_INVALID_JSON_ESCAPED_CHAR

---

**Investigation Metadata:**
- **Investigator**: Smoke Detector
- **Investigation Run**: [#18770149208](https://github.com/githubnext/gh-aw/actions/runs/18770149208)
- **Pattern ID**: COPILOT_INVALID_JSON_ESCAPED_CHAR
- **Severity**: Critical
- **Is Flaky**: No
- **Related Patterns**: COPILOT_BASE64_MCP_CONFIG (#2262), CLAUDE_INVALID_MCP_CONFIG_ARG (#2267)

**Labels**: smoke-test, investigation, copilot, configuration, mcp, critical, json-escaping

> AI generated by [Smoke Detector - Smoke Test Failure Investigator](https://github.com/githubnext/gh-aw/actions/runs/18770149208)




> AI generated by [Smoke Detector - Smoke Test Failure Investigator](https://github.com/githubnext/gh-aw/actions/runs/18770149208)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[smoke-detector] 🔍 Smoke Test Investigation - Smoke Copilot: Bad Escaped Character in MCP Config JSON #2270

🔍 Smoke Test Investigation - Run #18770131006

Summary

Failure Details

Root Cause Analysis

Primary Error

Investigation Findings

Error Location Analysis

Failed Jobs and Errors

Job Sequence

Error Details

Commit Analysis

Comparison with Related Issues

Recommended Actions

Critical Priority

High Priority

Medium Priority

Prevention Strategies

Technical Details

Expected Behavior

Actual Behavior

Likely Culprit Code

Pattern Storage

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Component	Expected	Actual	Status
Copilot --additional-mcp-config	Valid JSON string	JSON with bad escape at pos 433	❌ Invalid
JSON Parser	Properly escaped characters	Bad escaped character	❌ Parse Error

Issue	Engine	Branch	Problem	Root Cause
#2262	Copilot	copilot/update-copilot-engine-cli-config	Expected JSON, got base64	Workflow not recompiled
#2267	Claude	copilot/update-engines-cli-arguments	Expected file path, got JSON	Different CLI semantics
This	Copilot	copilot/update-copilot-agent-engine	Valid JSON expected, got bad escaping	Invalid escape sequence in JSON

[smoke-detector] 🔍 Smoke Test Investigation - Smoke Copilot: Bad Escaped Character in MCP Config JSON #2270

Description

🔍 Smoke Test Investigation - Run #18770131006

Summary

Failure Details

Root Cause Analysis

Primary Error

Investigation Findings

Error Location Analysis

Failed Jobs and Errors

Job Sequence

Error Details

Commit Analysis

Comparison with Related Issues

Recommended Actions

Critical Priority

High Priority

Medium Priority

Prevention Strategies

Technical Details

Expected Behavior

Actual Behavior

Likely Culprit Code

Pattern Storage

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions