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. Users writing ${VAR_NAME} in MCP config env/headers fields had their variables silently left unexpanded and never reported in missingVars.
• Why it matters: MCP server configs commonly use shell-style brace syntax (e.g. ${GITHUB_TOKEN}). Without this fix, secrets are silently not injected, causing MCP servers to start with missing credentials — no error surfaced.
• What changed: Replaced the single-group regex with a two-group alternation that matches both $VAR and ${VAR} forms. Added 4 targeted tests and updated the MCP servers doc.
• What did not change: All other expandEnvVarsInRecord logic — error handling, missingVars tracking, caller sites — is unchanged. Bare $VAR behavior is identical.

UX Journey

Before

User writes in MCP config:    env: { TOKEN: "${GITHUB_TOKEN}" }
Archon expands env vars  -->  TOKEN = ""  (silently, no error)
MCP server starts        -->  TOKEN is empty, auth fails
User sees no warning     -->  missingVars = [] (brace form not detected)

After

User writes in MCP config:    env: { TOKEN: "${GITHUB_TOKEN}" }
Archon expands env vars  -->  TOKEN = "<value of GITHUB_TOKEN>"  [fixed]
MCP server starts        -->  TOKEN is set, auth succeeds
If GITHUB_TOKEN is unset -->  missingVars = ["GITHUB_TOKEN"]  (reported)  [fixed]

Architecture Diagram

Before

packages/providers/src/mcp/config.ts
  expandEnvVarsInRecord()
    regex: /\$([A-Z_][A-Z0-9_]*)/g   ← bare $VAR only

After

packages/providers/src/mcp/config.ts
  expandEnvVarsInRecord()
    regex: /\$(?:\{([A-Z_][A-Z0-9_]*)\}|([A-Z_][A-Z0-9_]*))/g  [~]
    callback: braced ?? bare ?? ''                                [~]

Connection inventory:

From	To	Status	Notes
config.ts:expandEnvVarsInRecord	env source	unchanged	same lookup logic
config.ts:expandEnvVarsInRecord	missingVars	modified	now tracks brace-form missing vars too
dag-executor.test.ts	loadMcpConfig	modified	+4 brace-syntax test cases
mcp-servers.md	doc	modified	both forms documented

Label Snapshot

• Risk: risk: low
• Size: size: XS
• 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

Check	Status
Bundled defaults	PASS (36 commands, 20 workflows)
Bundled skill	PASS (21 files)
Type check	PASS (all 10 packages)
Lint	PASS (0 warnings, 0 errors)
Format	PASS
Tests	PASS (0 failures across all batches: 2,651+ tests)

• Evidence provided: Full bun run validate run post code-review fix
• No commands skipped

Security Impact (required)

• New permissions/capabilities? No
• New external network calls? No
• Secrets/tokens handling changed? Yes — brace-form env vars in MCP configs now expand correctly (previously silently dropped). No tokens logged or exposed.
• File system access scope changed? No
• Risk: Zero. The fix closes a security gap where credentials were silently not injected. No new capability is added; the behavior now matches user intent.

Compatibility / Migration

• Backward compatible? Yes — bare $VAR behavior is unchanged
• Config/env changes? No
• Database migration needed? No

Human Verification (required)

• Verified scenarios: Regex replacement confirmed correct via 4 new unit tests covering env, headers, mixed $VAR/${VAR}, and missing-var reporting
• Edge cases checked: Mixed bare+brace in same string; missing brace-form vars correctly added to missingVars
• What was not verified: Live MCP server integration (unit tests are definitive for the regex behavior)

Side Effects / Blast Radius (required)

• Affected subsystems: MCP config loading only (expandEnvVarsInRecord)
• Potential unintended effects: None — the two-group alternation is a strict superset of the old pattern with no nested quantifiers (no backtracking risk)
• Guardrails: missingVars reporting unchanged; existing bare-$VAR tests all pass

Rollback Plan (required)

• Fast rollback: Revert the one-line regex change in packages/providers/src/mcp/config.ts
• Feature flags: None
• Observable failure symptoms: MCP servers receiving empty env vars when users use ${VAR} syntax

Risks and Mitigations

• Risk: Regex catastrophic backtracking
  • Mitigation: Alternation is on a fixed prefix \$; both branches are [A-Z_][A-Z0-9_]* — no nested quantifiers possible. No backtracking risk.

[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: 59222645-fa99-4eb8-a404-2b1ecc651a39

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-fix-issue-1612-dryrun2

---

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