From 99e80d37dc2e066071654bef0f68a872d7e7856d Mon Sep 17 00:00:00 2001 From: Justin <9146678+brickfrog@users.noreply.github.com> Date: Thu, 9 Jul 2026 14:02:39 -0500 Subject: [PATCH 1/2] fix: forward CHOIR_AGENT_TOKEN to codex mcp-stdio via env_vars --- .choir/config.toml | 22 ++++++-------- src/workspace/launch.mbt | 11 +++++++ src/workspace/workspace_test.mbt | 51 ++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 13 deletions(-) diff --git a/.choir/config.toml b/.choir/config.toml index 2fa552f7..791b5f48 100644 --- a/.choir/config.toml +++ b/.choir/config.toml @@ -6,19 +6,15 @@ verify_cmd = "moon check --target native && moon test --target native" # Per-agent model selection (choir-w5m7). Passthrough to `codex --model `. # dev (gpt-5.3-codex-spark) PARKED: spark could not drive the agentic TDD-leaf # protocol (mis-sequenced Red->Done, false verify-pass, exit-255 build) on first -# trial. gpt-5.6-luna PARKED too (2026-07-09): every codex dev leaf spawned -# under luna had its `choir mcp-stdio` MCP subprocess launch with a completely -# empty CHOIR_* environment (confirmed via /proc//environ), unlike gpt-5.5 -# leaves and unlike root's own Claude-Code-spawned mcp-stdio process, which both -# correctly inherit CHOIR_AGENT_TOKEN etc. Missing token -> registration -# silently rejected server-side (serve_authenticate_register_request) but -# mcp_perform_registration_handshake only checks for a non-empty response, not -# success, so the bridge proceeds as if authenticated and the next real -# tools/call (tdd_intent et al) then fails opaquely as "server restarting, -# retry" -- see choir bead for the write-up. Root cause not fully pinned -# (whether it's luna specifically or the whole 5.6 family under some codex -# runtime mode), so reverting dev to the known-good gpt-5.5 baseline rather -# than guessing at terra. Effort is the real quota lever — see choir-mn0l. +# trial. gpt-5.6-luna was parked on 2026-07-09 blaming it for codex leaves' +# `choir mcp-stdio` subprocess launching with an empty CHOIR_* environment; +# that was disproven the same day — gpt-5.5 leaves had identically scrubbed +# env. Real cause: codex's shell_environment_policy never forwards the shell +# environment into MCP stdio subprocesses, and choir's codex MCP entry set no +# per-server env forwarding (choir-ghl9, fixed via +# `mcp_servers.choir.env_vars` in codex_mcp_override_args). Model was never at +# fault; re-tiering dev beyond gpt-5.5 is the operator's call once the fix +# proves out. Effort is the real quota lever — see choir-mn0l. [model.codex] default = "gpt-5.5" diff --git a/src/workspace/launch.mbt b/src/workspace/launch.mbt index b0561ce3..5b7c668e 100644 --- a/src/workspace/launch.mbt +++ b/src/workspace/launch.mbt @@ -154,12 +154,23 @@ pub fn codex_mcp_override_args( ] } let args_lit = toml_string_array_literal(elems) + // codex's shell_environment_policy scrubs the shell environment before + // spawning MCP stdio subprocesses, so `choir mcp-stdio` never inherits + // CHOIR_* ambiently (choir-ghl9). `env_vars` is codex's names-only forward + // list: values come from codex's own environment at spawn time and never + // ride this argv (argv leaks into ps/proc; the token is a secret). Identity + // and the socket path come from the per-leaf .choir/config.local.toml in + // `cwd` plus the --role/--name args above, so the token is the only + // env-sourced input registration needs. + let env_vars_lit = toml_string_array_literal(["CHOIR_AGENT_TOKEN"]) let args = [ "-c", "mcp_servers.choir.command=\"choir\"", "-c", "mcp_servers.choir.args=" + args_lit, "-c", + "mcp_servers.choir.env_vars=" + env_vars_lit, + "-c", "mcp_servers.choir.cwd=\"" + escape_toml_string(workspace_path) + "\"", ] if semble_pilot { diff --git a/src/workspace/workspace_test.mbt b/src/workspace/workspace_test.mbt index ab29451d..8b109ad3 100644 --- a/src/workspace/workspace_test.mbt +++ b/src/workspace/workspace_test.mbt @@ -825,6 +825,57 @@ test "codex_mcp_override_args is argv-shaped Toml without sh" { assert_false(args[3].contains("-lc")) } +///| +test "codex_mcp_override_args forwards registration env by name only" { + for role in [@types.Role::TL, @types.Role::Dev, @types.Role::Worker] { + let args = @workspace.codex_mcp_override_args(role, "main.leaf", "/proj") + // codex does not inherit the shell environment into MCP stdio + // subprocesses; the names-only `env_vars` forward list is the one + // mechanism that gets CHOIR_AGENT_TOKEN there without the value. + assert_true( + args.contains("mcp_servers.choir.env_vars=[\"CHOIR_AGENT_TOKEN\"]"), + ) + for arg in args { + // The token value must never ride argv: no `env` value-map entry + // (`mcp_servers.choir.env.KEY="..."` / `env={...}`) and no + // KEY=value assignment for the token anywhere. + assert_false(arg.contains("mcp_servers.choir.env.")) + assert_false(arg.contains("mcp_servers.choir.env=")) + assert_false(arg.contains("CHOIR_AGENT_TOKEN=")) + } + } +} + +///| +test "codex launch argv never carries a token assignment" { + let spec = @types.SpawnSpec::{ + slug: "codex-env-fwd", + role: @types.Role::Dev, + agent_type: @types.AgentType::Codex, + task: "do work", + task_contract: None, + isolation: @types.IsolationMode::Worktree, + parent_id: "root", + parent_branch: "main", + reuse_workspace: false, + workspace_slug: None, + reuse_git_branch: None, + spawn_depth: 0, + automerge: None, + review_mode: None, + merge_strategy: None, + wave_id: None, + } + let cmd = @workspace.agent_launch_command(spec, "/proj/worktree") + let body = launch_cmd_body_contains_setsid( + cmd, + project_dir="/proj/worktree", + agent_id="main.codex-env-fwd", + ) + assert_true(body.contains("mcp_servers.choir.env_vars")) + assert_false(body.contains("CHOIR_AGENT_TOKEN=")) +} + ///| test "codex_mcp_override_args includes semble server when pilot is enabled" { let args = @workspace.codex_mcp_override_args( From 4a52d6eccb5651d15de3b6543cbff91413ae0e31 Mon Sep 17 00:00:00 2001 From: Justin <9146678+brickfrog@users.noreply.github.com> Date: Thu, 9 Jul 2026 14:21:38 -0500 Subject: [PATCH 2/2] fix: correct MCP env-scrub attribution, drop redundant argv test --- .choir/config.toml | 8 +++++--- src/workspace/launch.mbt | 8 +++++--- src/workspace/workspace_test.mbt | 30 ------------------------------ 3 files changed, 10 insertions(+), 36 deletions(-) diff --git a/.choir/config.toml b/.choir/config.toml index 791b5f48..015b8085 100644 --- a/.choir/config.toml +++ b/.choir/config.toml @@ -9,9 +9,11 @@ verify_cmd = "moon check --target native && moon test --target native" # trial. gpt-5.6-luna was parked on 2026-07-09 blaming it for codex leaves' # `choir mcp-stdio` subprocess launching with an empty CHOIR_* environment; # that was disproven the same day — gpt-5.5 leaves had identically scrubbed -# env. Real cause: codex's shell_environment_policy never forwards the shell -# environment into MCP stdio subprocesses, and choir's codex MCP entry set no -# per-server env forwarding (choir-ghl9, fixed via +# env. Real cause: codex's MCP stdio launcher builds server environments from +# scratch (env_clear() + DEFAULT_ENV_VARS whitelist in rmcp-client's +# create_env_for_mcp_server; shell_environment_policy only affects shell/exec +# tools, not MCP), and choir's codex MCP entry set no per-server env +# forwarding (choir-ghl9, fixed via # `mcp_servers.choir.env_vars` in codex_mcp_override_args). Model was never at # fault; re-tiering dev beyond gpt-5.5 is the operator's call once the fix # proves out. Effort is the real quota lever — see choir-mn0l. diff --git a/src/workspace/launch.mbt b/src/workspace/launch.mbt index 5b7c668e..582fff10 100644 --- a/src/workspace/launch.mbt +++ b/src/workspace/launch.mbt @@ -154,9 +154,11 @@ pub fn codex_mcp_override_args( ] } let args_lit = toml_string_array_literal(elems) - // codex's shell_environment_policy scrubs the shell environment before - // spawning MCP stdio subprocesses, so `choir mcp-stdio` never inherits - // CHOIR_* ambiently (choir-ghl9). `env_vars` is codex's names-only forward + // codex's MCP stdio launcher builds each server's environment from scratch + // (`env_clear()` + a DEFAULT_ENV_VARS whitelist in rmcp-client's + // `create_env_for_mcp_server`; `shell_environment_policy` only governs + // shell/exec tools, not MCP), so `choir mcp-stdio` never inherits CHOIR_* + // ambiently (choir-ghl9). `env_vars` is codex's names-only forward // list: values come from codex's own environment at spawn time and never // ride this argv (argv leaks into ps/proc; the token is a secret). Identity // and the socket path come from the per-leaf .choir/config.local.toml in diff --git a/src/workspace/workspace_test.mbt b/src/workspace/workspace_test.mbt index 8b109ad3..7e642782 100644 --- a/src/workspace/workspace_test.mbt +++ b/src/workspace/workspace_test.mbt @@ -846,36 +846,6 @@ test "codex_mcp_override_args forwards registration env by name only" { } } -///| -test "codex launch argv never carries a token assignment" { - let spec = @types.SpawnSpec::{ - slug: "codex-env-fwd", - role: @types.Role::Dev, - agent_type: @types.AgentType::Codex, - task: "do work", - task_contract: None, - isolation: @types.IsolationMode::Worktree, - parent_id: "root", - parent_branch: "main", - reuse_workspace: false, - workspace_slug: None, - reuse_git_branch: None, - spawn_depth: 0, - automerge: None, - review_mode: None, - merge_strategy: None, - wave_id: None, - } - let cmd = @workspace.agent_launch_command(spec, "/proj/worktree") - let body = launch_cmd_body_contains_setsid( - cmd, - project_dir="/proj/worktree", - agent_id="main.codex-env-fwd", - ) - assert_true(body.contains("mcp_servers.choir.env_vars")) - assert_false(body.contains("CHOIR_AGENT_TOKEN=")) -} - ///| test "codex_mcp_override_args includes semble server when pilot is enabled" { let args = @workspace.codex_mcp_override_args(