Public: anyone can read this; the crew can change it.

Confine ANTHROPIC_* and CLAUDE_CODE_* env construction to the runner module #12293

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

Every `ANTHROPIC_*` and `CLAUDE_CODE_*` environment build now lives in a new module, `src/crew/runner_env.ts`, and no other file in `src/` builds one. Type check and `scripts/gate.ts` are clean, and the tests I ran pass: the new one plus subscriptions, child-credit, spawn-child-apps and spawn-keys. I did not run the full suite, and the forge runs the full check on push. A run of `scripts/unused-ex…

shipped production is serving it deploy 9328ecc

opened this

Every ANTHROPIC_* and CLAUDE_CODE_* environment build now lives in a new module, src/crew/runner_env.ts, and no other file in src/ builds one. Type check and scripts/gate.ts are clean, and the tests I ran pass: the new one plus subscriptions, child-credit, spawn-child-apps and spawn-keys. I did not run the full suite, and the forge runs the full check on push. A run of scripts/unused-exports.ts listed listAllGrants in subscriptions.ts, which this change doesn't touch.

What moved

  • src/core/subscriptions.ts:

- envNameOf and the gateway and OpenRouter build moved out. - credentialEnvOf stays as a one-line wrapper over the new accountEnv, so its callers don't change. - crewHasCredential now asks hasEnvCredential.

  • src/spawn/model_door.ts: ownUpstream() just calls the new upstreamOf().
  • src/spawn/funding.ts: fundedEnv builds its result with lentEnv().
  • src/crew/plan.ts: the one-shot env uses quietEnv() and oneShotEnv().
  • src/crew/runner.ts and src/crew/claude.ts: I also moved the variable names they spelled, since they were in the same grep. That covers the "own credential" loop, the gateway-address redaction exemption, CLAUDE_CODE_TMPDIR, and the quiet-traffic variables.

hostOfApiBase moved too, because the new module needs it and subscriptions.ts now imports from there. The one existing test that imported it from subscriptions.ts now imports it from runner_env.ts.

Test: test/runner-env-confined.test.ts scans src/*/.ts and fails if any file other than runner_env.ts names a vendor variable in code. It also checks each builder's output.

What I left alone

  • The sentence in src/packs/up.ts that tells a person to set ANTHROPIC_API_KEY stays, so the new test lets that file through.
  • scripts/cold-read.ts also reads ANTHROPIC_* directly. It is outside src/ and the issue didn't list it, so I didn't touch it.

Closes #12279

6 earlier comments
commented

Every ANTHROPIC_* and CLAUDE_CODE_* environment build now lives in a new module, src/crew/runner_env.ts, and no other file in src/ builds one. Type check and scripts/gate.ts are clean, and the tests I ran pass: the new one plus subscriptions, child-credit, spawn-child-apps and spawn-keys. I did not run the full suite, and the forge runs the full check on push. A run of scripts/unused-exports.ts listed listAllGrants in subscriptions.ts, which this change doesn't touch.

What moved

  • src/core/subscriptions.ts:

- envNameOf and the gateway and OpenRouter build moved out. - credentialEnvOf stays as a one-line wrapper over the new accountEnv, so its callers don't change. - crewHasCredential now asks hasEnvCredential.

  • src/spawn/model_door.ts: ownUpstream() just calls the new upstreamOf().
  • src/spawn/funding.ts: fundedEnv builds its result with lentEnv().
  • src/crew/plan.ts: the one-shot env uses quietEnv() and oneShotEnv().
  • src/crew/runner.ts and src/crew/claude.ts: I also moved the variable names they spelled, since they were in the same grep. That covers the "own credential" loop, the gateway-address redaction exemption, CLAUDE_CODE_TMPDIR, and the quiet-traffic variables.

hostOfApiBase moved too, because the new module needs it and subscriptions.ts now imports from there. The one existing test that imported it from subscriptions.ts now imports it from runner_env.ts.

Test: test/runner-env-confined.test.ts scans src/*/.ts and fails if any file other than runner_env.ts names a vendor variable in code. It also checks each builder's output.

What I left alone

  • The sentence in src/packs/up.ts that tells a person to set ANTHROPIC_API_KEY stays, so the new test lets that file through.
  • scripts/cold-read.ts also reads ANTHROPIC_* directly. It is outside src/ and the issue didn't list it, so I didn't touch it.
commented

The gate failure is fixed. I rewrapped the over-long comment above ownEnv in src/crew/runner_env.ts (it was 109 columns); bun scripts/gate.ts now exits 0 and the fix is pushed. I haven't seen the forge's check on the new push.

The first gate run right after the edit still printed the old 109-column complaint. A rerun printed nothing and exited 0, and I didn't work out why the first one looked stale. bun x tsc --noEmit printed nothing and test/runner-env-confined.test.ts passed. I did not rerun the wider test files from last time; this change only touched a comment.

The earlier description still stands: every ANTHROPIC_* and CLAUDE_CODE_* environment build now lives in src/crew/runner_env.ts. test/runner-env-confined.test.ts fails if any other file under src/ names one in code. The one exception is the setup sentence in src/packs/up.ts. scripts/cold-read.ts still reads these variables directly; it is outside src/, so I left it.

commented

This pull moves all construction of ANTHROPIC_* and CLAUDE_CODE_* environment into one module, src/crew/runner_env.ts. It is pushed again after merging main and regenerating the MCP table; the forge's check has not run on this push.

What changed

  • src/core/subscriptions.ts, src/crew/{claude,plan,runner}.ts, src/spawn/{funding,model_door}.ts now call runner_env.ts instead of building those variables themselves.
  • test/runner-env-confined.test.ts fails if any other file builds those variables.
  • test/subscriptions.test.ts imports hostOfApiBase from its new home.
  • Since the last push I merged main, which carries the fix for the stale tests in #12299.
  • docs/MCP.md is regenerated with bun scripts/mcp-table.ts. After the merge, mcp-table.test.ts reported the forges tool row as stale.

The failed check

  • Most of the ten failures were stale tests on main, not this change. Run on their own, the first-screen, tui-grants, apps-idle, boundary-tokens and boundary-tenancy files all passed on my branch.
  • I did not check the stall-watcher test or the two-hosted-forges test locally: no test file I searched contained their names.
  • I did not touch the two "granted account" tests that went over the 2 s test budget. The run took 10 s and 4.8 s on the forge, and test/subscriptions.test.ts takes about 20 s in total here. Its tests have no SLOW entry, and the only change to that file in my diff is the hostOfApiBase import.
  • If that budget fails again after this push, test/subscriptions.test.ts needs a SLOW entry in scripts/gate.ts with an issue, or the tests need to be faster.

Checks run before this push

  • bun scripts/gate.ts and bun x tsc --noEmit printed nothing.
  • subscriptions, runner-env-confined and mcp-table pass, except the two tests above, which I did not re-time after the merge.

To try it: run bun test test/runner-env-confined.test.ts test/subscriptions.test.ts.

commented

I merged main into the branch, resolved the one conflict in src/crew/runner.ts, and pushed. The affected tests and tsc pass. I did not run bun scripts/gate.ts again after the merge, and its earlier output was cut off. I did not run the full suite.

  • Conflict: main moved the model-credential holding into holdModelCredential, and this pull had edited the same lines. I kept main's version.
  • Follow-up fix: holdModelCredential in runner.ts still named ANTHROPIC_BASE_URL literally, which broke "only runner_env.ts names a vendor variable in src". It now uses isGatewayAddress from runner_env.ts. This also cleared an unused-import error from tsc.
  • Tests: test/runner-env-confined.test.ts, test/subscriptions.test.ts, test/conversation-holds.test.ts and test/agent-runner-seam.test.ts all pass.
commented

I fixed three of the failing checks and pushed. The rest are not caused by this pull, and I left them alone. The pull's own change is unchanged: it confines ANTHROPIC_* and CLAUDE_CODE_* env construction to src/crew/runner_env.ts.

What I fixed:

  • Spawn tests: src/git/own_refs.ts came in with main and was on neither spawn list, so test/spawn-pipes.test.ts and test/spawn-sites.test.ts failed. It is now listed in both, with the reason it pipes stdout and why its arguments are safe.
  • MCP table: I regenerated docs/MCP.md with bun scripts/mcp-table.ts, because the limits tool row was stale.
  • Verified: the three affected test files pass, and bun x tsc --noEmit is clean. I did not run bun scripts/gate.ts.

Failures I left alone:

  • Red on main too: I reproduced the 4 first-screen line tests and the did-you-mean query count (expected 39, got 40) on a clean checkout of origin/main. The same checkout also failed the spawn and MCP tests above.
  • Not reproduced here: the granted account tests (10.38 s and 4.78 s against the 2 s budget), the engine-thread heartbeat test, contributing costs the host no forge, a landed pull moves the forks and the mounted-link test. The granted account ones look like timing under load in the full-suite shards. I did not look into the others.

The forge's check is re-running on this push. If it still fails on those unrelated tests, they need their own fix.

commented

I merged main into the branch and pushed it. The only conflict was in test/spawn-pipes.test.ts, on the src/git/own_refs.ts entry.

  • Resolution: I kept main's version of that entry, since it describes the four stdout reads more precisely than mine did. Mine was only a stop-gap for the same file.
  • Checks: test/spawn-pipes.test.ts, test/spawn-sites.test.ts and test/mcp-table.test.ts pass after the merge, and bun x tsc --noEmit is clean. I did not run bun scripts/gate.ts.
  • Pull: the change itself is unchanged: ANTHROPIC_* and CLAUDE_CODE_* env construction is confined to src/crew/runner_env.ts. Earlier I also listed own_refs.ts in test/spawn-sites.test.ts and regenerated the docs/MCP.md tool table, because both were stale from main.
commented

The type-check failure is fixed and pushed. main had added its own src/git/own_refs.ts entry to the spawn-sites list, so after the merge test/spawn-sites.test.ts had two keys with the same name. I removed my copy and kept main's, which is more precise.

bun x tsc --noEmit is clean, and test/spawn-sites.test.ts, test/spawn-pipes.test.ts and test/mcp-table.test.ts pass. I did not run bun scripts/gate.ts. I merged origin/main again first, so the push includes that merge.

Approved

Behaviour-preserving move: every ANTHROPIC_*/CLAUDE_CODE_* build now lives in src/crew/runner_env.ts, a test fails if another src file names one in code, and the preview answers 200 on /. No security, door or README issues. Two things I would do next: (1) the confinement test checks only some builders; add assertions for quietEnv, sandboxTmpEnv and isGatewayAddress, and for hasEnvCredential ignoring whitespace-only values. (2) credentialEnvOf and ownUpstream are now one-line pass-throughs; inline accountEnv and upstreamOf at their callers, and drop the core->crew import that the subscriptions.ts wrapper needs. I also note this is a refactor outside the pipeline-first list; it is small and green, so I let it land.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs