fix(parallel): give upstream MCPs the broker's environment - #91
Conversation
The parallel broker spawned each upstream with cmd.Env set to only the spec's env block from mcp.json, so PATH, HOME and everything else were gone. Any upstream that shells out failed: the git MCP's analyze_change returned `exec: "git": executable file not found in $PATH` for every call routed through triagent-parallel__call, while the same tool invoked directly by Claude Code worked because Claude Code layers the env block onto its own environment. Build the upstream env the same way: os.Environ() with the spec's env layered on top, spec winning on conflict so each upstream keeps its own telemetry tool prefix. The broker's TRIAGENT_MCP_PARALLEL_UPSTREAMS blob is dropped since it carries every upstream's secrets and means nothing to a child. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TnKDV9gdifLNNJQ8SbpSFV
There was a problem hiding this comment.
🟡 Changes recommended
The upstream registry env var can currently be reintroduced via spec overrides, undermining the intended guarantee that the secrets-bearing blob never reaches child processes.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes the triagent-parallel broker’s upstream spawning so upstream MCP subprocesses inherit the broker’s environment (e.g., PATH, HOME) while allowing the upstream spec’s env block to override specific variables. This aligns parallel-routed tool calls with the behavior of directly launched MCPs, unblocking upstreams that shell out to external binaries (e.g., git fetch, kubectl).
Changes:
- Build upstream subprocess environment from
os.Environ()with the upstream spec’senvlayered on top (spec wins on conflict). - Filter
TRIAGENT_MCP_PARALLEL_UPSTREAMSout of child environments to avoid leaking the upstream registry blob. - Add unit tests covering inheritance/override behavior and exclusion of the upstream registry env var.
File summaries
| File | Description |
|---|---|
| pkg/mcp/parallel/registry.go | Changes upstream subprocess env construction to inherit broker env with spec overrides, and introduces helper to filter sensitive broker-only env. |
| pkg/mcp/parallel/registry_test.go | Adds tests validating PATH inheritance, override precedence, and non-leakage of the upstream registry blob. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
upstreamEnv filtered TRIAGENT_MCP_PARALLEL_UPSTREAMS out of the broker's inherited environment but not out of the spec's override block, so the docstring's guarantee that the secrets-bearing registry never reaches a child held only for one of the two sources. Preflight never writes the key into an upstream's env block today, but the guarantee should not depend on that. Skip the key in both loops and extend the test to put it in the spec too. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TnKDV9gdifLNNJQ8SbpSFV
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, matches the described failure mode, and is covered by new unit tests asserting the key env invariants.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
Description
Every tool call routed through
triagent-parallel__callto an upstream that shells out failed, while the same tool invoked directly by Claude Code worked. The broker spawned each upstream withcmd.Envset to only the spec's env block frommcp.json, so PATH, HOME and everything else were gone, and the git MCP'sanalyze_changereturnedexec: "git": executable file not found in $PATH. This PR builds the upstream env the way Claude Code does for a directly launched MCP: the broker's own environment with the spec's env block layered on top.Changes
os.Environ(); the spec's env wins on conflict so each upstream keeps its own telemetry tool prefix rather than the broker's.TRIAGENT_MCP_PARALLEL_UPSTREAMSblob is filtered out of children, since it carries every upstream's secrets and means nothing to them.Related
Testing
Two new unit tests on
buildUpstreamCommandcover PATH inheritance, spec override precedence, and exclusion of the upstreams blob; both failed before the change and pass after.make test-go(race) andmake lintare clean. To reproduce the original failure, run anytriagent-parallel__callagainst atriagent-git-*upstream on a v0.4.2 launcher.🤖 Generated with Claude Code
https://claude.ai/code/session_01TnKDV9gdifLNNJQ8SbpSFV