fix(providers): expand ${VAR_NAME} brace syntax in MCP config env vars

[coleam00]

Summary

• Problem: expandEnvVarsInRecord in packages/providers/src/mcp/config.ts only matched bare $VAR_NAME syntax via /\$([A-Z_][A-Z0-9_]*)/g. The ${VAR_NAME} brace form — used in shell scripts, Docker Compose, and official MCP server documentation — was silently left unexpanded and never reported in missingVars.
• Why it matters: Users copying MCP config examples from official docs (which use ${VAR}) get silent failures — the variable reference appears literally in the config instead of the env value, with no warning.
• What changed: Regex replaced with a two-group alternation that matches both ${VAR} (group 1) and $VAR (group 2); 5 new tests added; MCP servers docs updated to show both forms; one CLAUDE.md planning-convention note added.
• What did not change: Expansion is still restricted to env and headers fields only — command and args are intentionally not expanded (security boundary preserved for both bare and brace forms).

UX Journey

Before

User writes MCP config:
  { "db": { "env": { "DATABASE_URL": "${DATABASE_URL}" } } }

Archon loads config ──▶ expandEnvVarsInRecord
                         regex /\$([A-Z_][A-Z0-9_]*)/ — no match on ${...}
                         value stays: "${DATABASE_URL}"   ← silently wrong
                         missingVars: []                  ← no warning either

MCP server receives literal "${DATABASE_URL}" as env value — connection fails

After

User writes MCP config:
  { "db": { "env": { "DATABASE_URL": "${DATABASE_URL}" } } }

Archon loads config ──▶ expandEnvVarsInRecord
                         regex /\$(?:\{([A-Z_][A-Z0-9_]*)\}|([A-Z_][A-Z0-9_]*))/ — matches!
                         braced group fires: varName = "DATABASE_URL"
                         [value set] / [missingVars entry added if unset]

MCP server receives correct env value — connection succeeds

Architecture Diagram

Before

mcp/config.ts::expandEnvVarsInRecord
  regex: /\$([A-Z_][A-Z0-9_]*)/g   ← bare $VAR only
  → expands env/headers values
  → skips ${VAR} silently

After

mcp/config.ts::expandEnvVarsInRecord
  regex: /\$(?:\{([A-Z_][A-Z0-9_]*)\}|([A-Z_][A-Z0-9_]*))/g   [~]
  → expands env/headers values (bare $VAR unchanged)
  → [+] also expands ${VAR} brace form
  → [+] reports ${MISSING_VAR} in missingVars

Connection inventory:

From	To	Status	Notes
dag-executor	mcp/config.ts::expandEnvVarsInRecord	modified	Brace-form expansion added
dag-executor.test.ts	loadMcpConfig	modified	5 new brace-form tests
mcp-servers.md	—	modified	Docs updated with both syntaxes
CLAUDE.md	—	modified	Planning-convention note added

Label Snapshot

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

Change Metadata

• Change type: bug
• Primary scope: providers

Linked Issue

• Closes MCP env var expansion misses ${VAR} brace syntax #1612

Validation Evidence (required)

bun run validate

All checks pass:

• Bundled defaults: PASS (36 commands, 20 workflows — up to date)
• Bundled skill: PASS (21 files — up to date)
• Type check: PASS (all 10 packages)
• Lint: PASS (0 warnings, --max-warnings 0 enforced)
• Format: PASS (Prettier clean)
• Tests: PASS (3,712+ tests across all packages, 0 failures)

5 new targeted tests for brace-form expansion:

1. expands ${VAR_NAME} brace-form in env values
2. expands ${VAR_NAME} brace-form in headers values
3. replaces undefined brace-form vars with empty string and reports them
4. expands mixed bare and brace-form vars in the same string
5. does not expand brace-form vars in command or args fields (security boundary)

Security Impact (required)

• New permissions/capabilities? No
• New external network calls? No
• Secrets/tokens handling changed? No — expansion scope is unchanged (env/headers only); the same env vars that were accessible before are accessible now, just with an additional syntax
• File system access scope changed? No
• The command and args security boundary is explicitly tested to confirm brace form is also NOT expanded there

Compatibility / Migration

• Backward compatible? Yes — bare $VAR_NAME behaviour is unchanged; ${VAR_NAME} was previously a no-op (silently passed through), now it expands
• Config/env changes? No
• Database migration needed? No

Human Verification (required)

• Verified scenarios: regex alternation compiles and matches both forms; missing brace var correctly reported in missingVars; command/args field security boundary confirmed by test
• Edge cases checked: mixed bare+brace in a single string value; undefined brace var (empty string + missingVars); headers field (not just env)
• What was not verified: end-to-end with a live MCP server process (unit tests cover the expansion logic; MCP server behavior is outside this fix's scope)

Side Effects / Blast Radius (required)

• Affected subsystems/workflows: MCP config loading only (packages/providers/src/mcp/config.ts)
• Potential unintended effects: None — the old bare-form path is unmodified; brace form was previously a no-op
• Guardrails/monitoring: missingVars reporting is preserved and extended to brace form, so users get the same warnings they would for bare-form missing vars

Rollback Plan (required)

• Fast rollback command/path: git revert ec328a23 — single-commit revert, no migration needed
• Feature flags or config toggles: None
• Observable failure symptoms: MCP server env values containing literal ${VAR} strings instead of resolved values

Risks and Mitigations

• Risk: Regex catastrophic backtracking
  • Mitigation: Two-group alternation with (?:...) non-capturing outer group — linear time, no nested quantifiers. Reviewed and verified safe.

Summary by CodeRabbit

• Documentation

  • Updated MCP server configuration guide to clarify environment variable expansion.
  • Updated internal documentation notes on plan insertion point definitions.

• New Features

  • MCP configuration now supports both $VAR_NAME and ${VAR_NAME} formats for environment variable expansion in config values.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9f9abbeb-4495-4c55-912f-e89e6fc5818c

📥 Commits

Reviewing files that changed from the base of the PR and between 0adec22 and 41f695b.

📒 Files selected for processing (4)

• CLAUDE.md
• packages/docs-web/src/content/docs/guides/mcp-servers.md
• packages/providers/src/mcp/config.ts
• packages/workflows/src/dag-executor.test.ts

---

📝 Walkthrough

Walkthrough

This PR extends MCP environment variable expansion to support both $VAR_NAME and ${VAR_NAME} syntax forms. The core implementation updates a regex in the expansion function to capture both patterns, tests validate expansion behavior including undefined variable handling and field-specific non-expansion, and user documentation is updated to reflect the new capability.

Changes

MCP Environment Variable Expansion

Layer / File(s)	Summary
Environment variable expansion implementation packages/providers/src/mcp/config.ts	Regex in expandEnvVarsInRecord updated to match either ${VAR} braced or $VAR unbraced forms, with doc comment clarifying both syntax patterns are now supported and missing variables yielding empty strings while being recorded.
Brace-form expansion test coverage packages/workflows/src/dag-executor.test.ts	New loadMcpConfig test cases validate ${VAR_NAME} expansion in env and headers, empty-string substitution for undefined variables with missingVars reporting, mixed bare and brace syntax within single strings, and confirmation that ${...} is not expanded in command and args fields.
User-facing MCP documentation packages/docs-web/src/content/docs/guides/mcp-servers.md	MCP config guide updated to state that env and headers support both $UPPER_CASE_VAR and ${UPPER_CASE_VAR} syntax; postgres example revised to use ${DATABASE_URL} and regex rules clarified to match both patterns.

Development Guidelines

Layer / File(s)	Summary
Plan insertion point guidance CLAUDE.md	Added note that plan insertion points should be defined using stable text anchors (e.g., referencing a specific test block) instead of raw line numbers, since lines shift with code edits.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• coleam00/Archon#1459: Both PRs touch the shared MCP config loader (packages/providers/src/mcp/config.ts) used by workflow MCP support—feat(workflows): support Codex MCP nodes #1459 adds Codex MCP nodes and placeholder expansion via loadMcpConfig, while this PR extends that same env/header expansion logic to also support ${VAR_NAME} and updates the corresponding dag-executor tests.

Poem

🐰 Braces now dance with dollars fine,
${VAR} and $VAR both align,
Environment vars take their stance,
In MCP configs, all expand!
The test suite hops, docs now gleam,
Variable expansion dreams! ✨

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch archon/task-fix-issue-1612-dryrun3

---

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.}

[Wirasm]

Review Summary

Verdict: ready-to-merge

Clean, focused fix. The ${VAR_NAME} brace-form env var expansion is correctly implemented — sound regex, proper type narrowing, no backtracking risk, and all edge cases tested. Docs are updated in the same PR. One minor doc comment nitpick (see below); nothing blocking.

Blocking issues

None.

Suggested fixes

None.

Minor / nice-to-have

• packages/providers/src/mcp/config.ts:19: The function doc comment mentions $VAR_NAME but omits ${VAR_NAME}, even though the implementation supports both forms. Consider updating to "Expand $VAR_NAME and ${VAR_NAME} references" for consistency with the updated mcp-servers.md docs. (non-blocking)

Compliments

• Test coverage — Five well-scoped tests covering happy paths, undefined vars, mixed bare+brace syntax, and the command/args security boundary. Good boundary assertion.
• CLAUDE.md addition — The new planning-convention note on using stable text anchors over raw line numbers is exactly the kind of institutional knowledge worth capturing. Well spotted.
• Docs in same PR — mcp-servers.md updated to show both forms with the braced syntax in the example. No doc drift.

---

Reviewed via maintainer-review-pr workflow (Pi/Minimax). Aspects run: code-review, test-coverage, comment-quality, docs-impact.