fix(connection): resolve ${secret:...} refs in stdio env before spawn (closes #65)

[rodaddy]

Root cause

For stdio services, ${secret:...} references in the service's env (and args) reached the spawned child unresolved on any connect path that did not resolve them upstream.

• src/connection/transport.ts:82 spreads service.env raw into the child mergedEnv.
• src/connection/transport.ts:83 runs only ${VAR} expansion, and only on args -- never the secret resolver.

So a literal ${secret:...} env value (or a ${secret:...} arg that expandEnvVars blanks to empty, since no such shell var exists) landed in the child. Observed on the n8n service: n8n-mcp's getN8nApiConfig() validates N8N_API_URL with zod .url(), the literal ref fails validation, and all 15 n8n management tools report the misleading "n8n API not configured". (This is the true root cause behind rtech-mcps#19; source: remote->local was a red herring -- source is daemon routing, not env-source.)

Why some paths worked and some didn't

resolveServiceSecretRefs was already called on the direct CLI path (resolveDirectServiceConfig) and in the daemon pool (src/daemon/pool.ts:140). But the pool's HTTP/WebSocket stdio fallback constructors -- connectFallback (pool.ts:435) and connectWsFallback (pool.ts:359) -- build a stdio config from raw fb.env / fb.args and call connectToService with no resolution. That is a proven leak in the current tree, and it means resolution was caller-dependent rather than guaranteed.

The fix

Resolve at the single universal stdio chokepoint -- connectToService in src/connection/client.ts -- which every stdio spawn passes through (direct path, daemon pool, and both fallbacks). One surgical change closes every path instead of patching each caller.

• Uses the same resolveServiceSecretRefs(name, config, resolver) + VaultwardenSecretResolver pattern the working paths use.
• Idempotent: resolveServiceSecretRefs short-circuits via hasSecretRefs when no ${secret: remains, so callers that already resolve upstream incur zero extra Vaultwarden lookups -- no double resolution.
• Resolver is injectable (options.resolver) for testing; default stays VaultwardenSecretResolver.
• Log labels intentionally keep the unresolved command/args so resolved secret values never reach logs.

Test Plan

New fixture tests/fixtures/env-echo-mcp-server.ts (an MCP server whose echo_env tool returns its own process.env + argv) plus tests/connection/client.test.ts cases:

1. A ${secret:...} env value reaches the child resolved (mocked resolver) -- child never sees ${secret:.
2. A ${secret:...} arg is resolved before spawn.
3. A ref-free config triggers zero resolver calls (idempotency guard).

bun test tests/connection/client.test.ts
# 10 pass / 0 fail

Mutation-verified: reverting the fix (resolved.* -> service.*) makes the child read back the literal ${secret:RTech MCPs - foo#fields.N8N_API_URL} env and a stripped --token= arg -- both new tests fail. This reproduces #65 exactly.

Full suite: bun test -> 1080 pass / 0 fail across three repeated runs. bun run typecheck clean.

Verifying n8n lights up after

With this merged, on a host where the n8n stdio service uses ${secret:...} in its env, the child receives the real N8N_API_URL/N8N_API_KEY, zod .url() validation passes, and the 15 n8n management tools stop reporting "n8n API not configured":

mcp2cli n8n <management-tool>   # previously "n8n API not configured", now works

Closes #65.

🤖 Generated with Claude Code