fix: expand ${VAR_NAME} brace syntax in MCP config env var expansion

[coleam00]

Summary

• Problem: expandEnvVarsInRecord in packages/providers/src/mcp/config.ts only expanded bare $VAR_NAME references; brace-style ${VAR_NAME} — the standard form in Docker Compose, shell scripts, and most MCP server documentation — was silently passed through as a literal string.
• Why it matters: Users following standard MCP server setup guides (which use ${VAR_NAME}) received broken API tokens and database URLs at runtime with no error or warning.
• What changed: Single regex updated to a non-capturing alternation that handles both forms; 4 new targeted tests; docs and CLAUDE.md updated to document both forms. PIV workflow command files added to .archon/ as part of the AI-layer evolution.
• What did not change: No other MCP loading logic, no new imports, no schema changes, no database changes.

UX Journey

Before

User writes mcp.json:   { "github": { "env": { "TOKEN": "${MY_API_KEY}" } } }
Archon loads config ──▶ expandEnvVarsInRecord runs
                        regex: /\$([A-Z_][A-Z0-9_]*)/g  ← excludes {
                        "${MY_API_KEY}" passes through unexpanded ← BUG
MCP server starts ─────▶ TOKEN = "${MY_API_KEY}"  ← literal string, auth fails

After

User writes mcp.json:   { "github": { "env": { "TOKEN": "${MY_API_KEY}" } } }
Archon loads config ──▶ expandEnvVarsInRecord runs
                        regex: /\$(?:\{([A-Z_][A-Z0-9_]*)\}|([A-Z_][A-Z0-9_]*))/g
                        "${MY_API_KEY}" [expands correctly]
MCP server starts ─────▶ TOKEN = "actual_api_key_value"  ← correct

Architecture Diagram

Before / After

  WorkflowExecutor (dag-executor.ts)
       │
       ▼
  loadMcpConfig (providers/src/mcp/config.ts)
       │
       ▼
  expandEnvVarsInRecord     [~ regex updated to support both $VAR and ${VAR}]
       │
       ▼
  process.env / injected envSource

Connection inventory:

From	To	Status	Notes
dag-executor.ts	loadMcpConfig	unchanged	call site unchanged
expandEnvVarsInRecord	process.env	unchanged	env lookup unchanged
expandEnvVarsInRecord	regex	modified	alternation added for brace form

Label Snapshot

• Risk: risk: low
• Size: size: XS
• Scope: providers
• Module: providers:mcp

Change Metadata

• Change type: bug
• Primary scope: providers

Linked Issue

• Closes #

Validation Evidence (required)

bun run validate

• Bundled defaults: PASS (36 commands, 20 workflows — up to date)
• Bundled skill: PASS (21 files — up to date)
• Type check: PASS — all 10 packages clean
• Lint: PASS — zero warnings, zero errors
• Format: PASS
• Tests: PASS — ~3,600+ tests across all packages, 0 failures, 7 skipped

Package breakdown: @archon/git 142, @archon/cli 226, @archon/providers 381, @archon/core 831, @archon/web 158, @archon/paths 170, @archon/isolation 251, @archon/server 233, @archon/adapters 344, @archon/workflows 930.

Security Impact (required)

• New permissions/capabilities? No
• New external network calls? No
• Secrets/tokens handling changed? No — same env lookup, broader match pattern only
• File system access scope changed? No

Compatibility / Migration

• Backward compatible? Yes — bare $VAR_NAME expansion is unchanged; only adds support for ${VAR_NAME}
• Config/env changes? No
• Database migration needed? No

Human Verification (required)

• Verified scenarios: regex alternation unit-tested (4 new tests cover env expansion, mixed syntax in one value, missing-var reporting, headers expansion)
• Edge cases checked: unclosed brace ${VAR (falls through unexpanded), missing vars in brace form (empty string + missingVars list), mixed forms in same string
• What was not verified: live end-to-end with a running MCP server process

Side Effects / Blast Radius (required)

• Affected subsystems/workflows: MCP config loading only (expandEnvVarsInRecord in @archon/providers)
• Potential unintended effects: None — the alternation is additive; existing bare $VAR paths are unchanged
• Guardrails: result.missingVars list already surfaces undefined vars to callers

Rollback Plan (required)

• Fast rollback: revert packages/providers/src/mcp/config.ts regex to single-capture form
• Feature flags: none
• Observable failure symptoms: MCP servers receiving literal ${VAR} strings instead of resolved values

Risks and Mitigations

• Risk: Regex partially matches ${ without closing } — LOW. The alternation requires \{...\} to be complete; an unclosed brace falls through to the bare-var branch or passes through unexpanded. Covered by the missing-var test.
• Risk: as string vs ?? '' ESLint conflict — RESOLVED. The @typescript-eslint/non-nullable-type-assertion-style + @typescript-eslint/no-non-null-assertion rules together prohibit both as T and !. Resolution: braced ?? bare ?? '' with an inline comment explaining the '' branch is unreachable. This pattern is now documented in CLAUDE.md for future implementers.

AI-Layer Changes (PIV run)

This branch was implemented and validated by a PIV (Plan-Implement-Validate) run. The following AI-layer files were evolved based on findings:

• .archon/commands/piv-plan.md — Added ESLint constraint verification step to Phase 2 ANALYZE and Phase 4 VERIFY, so future plan authors check TypeScript construct compatibility with project ESLint rules before recommending them in GOTCHA sections. Added cross-package test rationale note to the Testing Strategy table template.
• CLAUDE.md — Added "Type-assertion dead-end (Archon-specific)" section under ESLint Guidelines documenting that @typescript-eslint/non-nullable-type-assertion-style + @typescript-eslint/no-non-null-assertion prohibit both as T and !; the compliant resolution is ?? fallback with an inline comment.
• .archon/commands/piv-*.md + .archon/workflows/piv-system-evolution.yaml — PIV workflow command files and the system-evolution workflow added to the repo.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4ac71bce-3eb1-4989-bd99-caa4f4b4fde9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch archon/task-test-piv-1612

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coleam00]

Closing — this PR was produced by an end-to-end test run of the piv-system-evolution workflow, not a real contribution. The MCP ${VAR} fix itself is correct and may be reopened/reused separately.