Skip to content

fix: pass provider launch env vars to agents#2135

Merged
jschwxrz merged 2 commits into
mainfrom
jona/eng-1294-environment-variables-not-working
May 20, 2026
Merged

fix: pass provider launch env vars to agents#2135
jschwxrz merged 2 commits into
mainfrom
jona/eng-1294-environment-variables-not-working

Conversation

@jschwxrz
Copy link
Copy Markdown
Collaborator

No description provided.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 20, 2026

Greptile Summary

This PR expands the AGENT_ENV_VARS allowlist in pty-env.ts with ~22 new entries covering provider-specific config, model selectors, base URLs, proxy settings, and tool paths so they are forwarded into agent PTY environments. A companion test verifies the new vars are passed through while hook-owned and agent-internal vars remain blocked.

  • pty-env.ts: Adds entries for ANTHROPIC_AUTH_TOKEN, ANTHROPIC_BASE_URL, ANTHROPIC_MODEL, GOOSE_*, OPENAI_MODEL/ORGANIZATION/PROJECT, OPENCODE_MODEL, GEMINI_MODEL, GROK_CODE_XAI_API_KEY, BAILIAN_CODING_PLAN_API_KEY, CLAUDE_CONFIG_DIR, CODEX_HOME, AMP_TOOLBOX, EDITOR, ALL_PROXY, and several Google/Vertex URL vars.
  • pty-env.test.ts: Adds a pass-through test that seeds process.env with all newly listed vars and asserts they appear in the built env, while asserting that CLAUDE_PROJECT_DIR, CODEX_ACCESS_TOKEN, GOOSE_TERMINAL, and TOOLBOX_ACTION are correctly excluded.

Confidence Score: 4/5

Safe to merge — the change is an additive allowlist expansion with no modifications to forwarding logic.

The forwarding logic in buildAgentEnv is untouched; only the list of env-var names picked up from process.env grows. The new test covers the happy path and hook-var exclusions are explicitly asserted. The two observations are non-blocking style points.

No files require special attention.

Important Files Changed

Filename Overview
src/main/core/pty/pty-env.ts Adds ~22 new entries to AGENT_ENV_VARS covering provider config, model names, proxy vars, and tool paths; logic in buildAgentEnv is unchanged.
src/main/core/pty/pty-env.test.ts Adds a new pass-through test for the new vars; test is placed inside the Windows-specific describe block even though it covers cross-platform behaviour.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([process.env]) --> B{agentApiVars?}
    B -- true --> C[Pick keys from AGENT_ENV_VARS]
    B -- false --> D[skip]
    C --> E[Apply providerVars overlay]
    D --> E
    E --> F{hook defined?}
    F -- yes --> G[Inject EMDASH_HOOK_PORT / PTY_ID / TOKEN]
    F -- no --> H[skip]
    G --> I([Final agent env])
    H --> I
Loading
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/main/core/pty/pty-env.test.ts:61-98
**Test placed inside wrong describe block**

The new test `'passes through documented provider launch environment variables'` is nested inside `describe('pty env Windows shell handling')`, but it tests cross-platform env-var forwarding logic, not Windows shell behaviour. A reader scanning that describe block will expect only Windows-specific cases; this test will be overlooked or confuse future maintainers. It should live in its own `describe` block (e.g. `describe('buildAgentEnv provider env forwarding', ...)`) or alongside the existing `buildAgentEnv` tests.

### Issue 2 of 2
src/main/core/pty/pty-env.ts:34
**`EDITOR` forwarded alongside API keys and model vars**

`EDITOR` is a general-purpose Unix convention for the user's preferred interactive text editor (e.g. `code --wait`, `vim`, `emacs`). Forwarding it to every agent process is unlikely to cause harm in practice, but it sits oddly among API keys, model selectors, and provider URLs. If no agent CLI actually needs to know the user's `$EDITOR`, this is unnecessary noise in the forwarded environment; if it is needed, a short comment explaining why would help future reviewers.

Reviews (1): Last reviewed commit: "fix: pass provider launch env vars to ag..." | Re-trigger Greptile

Comment thread src/main/core/pty/pty-env.test.ts
Comment thread src/main/core/pty/pty-env.ts Outdated
@jschwxrz jschwxrz merged commit e031bff into main May 20, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant