[nwparker]

	Files	Added	Deleted	Net
Test	5	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​358	$\color{#cf222e}{\Huge{\mathbf{−}}}$​2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​356
Prod	13	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​159	$\color{#cf222e}{\Huge{\mathbf{−}}}$​58	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​101

ELI5

A command such as VAR=value agent should run the agent with that variable, instead of looking for a program named VAR=value.

What Changed

Extract leading assignments after command tokenization and carry them as data through generation and model discovery. Merge variables on the execution host, fold Windows environment names consistently, apply WSL assignments after login startup and keep Codex home locks tied to the effective home.

Why

Using the existing argv planner avoids shell evaluation and keeps literal values, Windows casing and remote-host ownership explicit. The command planner is split into a small module to stay within the file-size gate.

Linked Issue

Related to @carlosbaraza’s #5363.

Visual Proof

Historical hidden-renderer proof from audit baseline 6ae5ef2d0031. A synthetic agent returns the fixed response through production Source Control UI; no paid agent request. These images illustrate unchanged behavior after the current-main merge.

Before:

After:

Testing

• Automated regressions added and run
• Historical hidden-renderer fixture evidence attached

262 tests pass across 24 files on macOS; Node typecheck and changed-code quality pass. After the review follow-up, 26 focused environment tests and changed-code quality pass. The combined-source suite with the SSH login-shell change exercises the real parser and relay handler: 292 pass, two Windows-only cases skip. Earlier audit included Windows/Linux process fixtures; this pass does not claim native WSL, full SSH transport or paid-agent integration.

All tests used ORCA_BACKGROUND_LAUNCH=1; no visible test window or real agent account was launched. Full lint, test suite and release packaging remain CI coverage.

Review

Current-head CI (54da185953d7) is green, including required PR Checks verify, changed-spec E2E and Docker SSH E2E. One targeted retry cleared an unrelated scan-cache call-count failure; the unchanged failing case passed locally, and the environment parent’s full runtime suite passed 1,294 tests with one skip.

Pullfrog’s current-head review at 54da185953d7 is clean. The Windows casing thread is resolved: deliberate normalization remains, tests no longer assume inherited key spelling, and mixed-case ComSpec preservation is covered.

Independently reviewed literal argv/environment handling, host ownership, Windows casing, WSL forwarding, cancellation/cleanup and mixed-version fallback. The original integration base was 996f9cc3068c.

Agent skill upstream boundary

• Not applicable; no upstream skill content copied.

Notes

The SSH login-shell PR #22428 builds on this PR and must merge after it. Literal assignment values are never shell-expanded; variables after the executable remain arguments.

Checklist

• Scoped before/after behavior and approach explained
• Before/after proof attached
• Correctness, security, performance, platform and remote behavior reviewed
• Full CI checks complete

[coderabbitai]

Warning

Review limit reached

Next included review available in 24 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 839df32f-56b0-44b2-8c0a-6be030682abb

📥 Commits

Reviewing files that changed from the base of the PR and between 0b16a31 and 54da185.

📒 Files selected for processing (18)

• src/main/codex-cli/codex-home-process-lock.ts
• src/main/providers/ssh-git-noninteractive-provider.ts
• src/main/providers/ssh-git-provider-commit-message.test.ts
• src/main/text-generation/command-environment-process.test.ts
• src/main/text-generation/command-environment-wsl.test.ts
• src/main/text-generation/commit-message-model-discovery-policy.ts
• src/main/text-generation/commit-message-model-discovery.ts
• src/main/text-generation/source-control-agent-launch.ts
• src/main/text-generation/source-control-local-generation.ts
• src/main/text-generation/source-control-local-process.ts
• src/main/text-generation/source-control-text-generation-types.ts
• src/relay/agent-exec-handler.test.ts
• src/relay/agent-exec-handler.ts
• src/shared/agent-command-plan.ts
• src/shared/command-environment.test.ts
• src/shared/command-environment.ts
• src/shared/commit-message-plan.ts
• src/shared/commit-message-prompt.ts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[nwparker]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[pullfrog]

ℹ️ No critical issues — one minor suggestion inline, plus two nitpicks.

Reviewed changes

• Leading assignment parser — new command-environment.ts extracts NAME=value prefixes as data (extractLeadingEnvAssignments) and merges them platform-aware (mergeCommandEnvironment, which uppercases all keys on win32).
• Plan plumbing — planAgentBinary (moved to agent-command-plan.ts) and planCustomCommand now strip leading assignments and carry them as env through CommitMessagePlan and planModelDiscovery.
• Local execution — native merges plan.env into the child env; WSL leaves env unmerged and applies assignments in the guest via /usr/bin/env KEY=value binary ... after the login shell.
• Remote execution — SshGitNoninteractiveProvider forwards plan.env; the relay AgentExecHandler folds it over process.env and re-runs the Git indexed-config protocol merge.
• Codex home lock — resolveCodexHomeProcessLockKeyForSpawnEnv now accepts commandEnv and prefers an explicitly assigned guest CODEX_HOME.
• Tests — parser/merge units, real-child native tests, a mocked WSL argv test, the relay casing case, and the SSH provider forwarding case.

The env-once-per-path diagram holds: native callers always pre-merge env and the native branch ignores commandEnv, WSL does the opposite, so an assignment is applied exactly once. I verified every production caller of runLocalSourceControlPlan/discoverModelsLocal/runLocalPlanForAgent — none drops an assignment.

ℹ️ Nitpicks

• SpawnSourceControlAgent.commandEnv is consumed only by the WSL branch of spawnSourceControlAgent; on native it is dead input, and the invariant that both callers pre-merge plan.env into env is implicit. A one-line comment on the type or the native branch would keep a future caller from assuming commandEnv alone works natively.
• The WSL test mocks wslAwareSpawn, so it asserts the argv it hands over rather than literal preservation — the shell-safety that actually keeps $HOME/$(...) from expanding lives in resolveCommand/quotePosixShell and isn't exercised. The process test covers native, but the title's WSL claim is untested end-to-end (the PR body already notes no native WSL run).

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

The delta since the prior pullfrog review (5170a86) is the test-and-comment commit 54da185; no production behavior changed.

• Windows environment-key coverage — a new relay test pins mixed-case override folding (Path → PATH, token → TOKEN) and verifies the indexed Git-config protocol discards stale inherited indices (GIT_CONFIG_KEY_1/VALUE_1) when a smaller override count arrives. I confirmed it fails against the pre-fix mergeGitConfigEnvProtocol(process.env, extraEnv) and passes at head.
• Inherited-env assertion normalized — the pass-through test now stubs a dedicated ORCA_EXEC_INHERITED value instead of spreading ...process.env, so it no longer depends on Windows key spelling.
• ComSpec preservation pinned — the merge unit test now asserts a mixed-case inherited key survives as COMSPEC on win32.
• Native/WSL contract comment — SpawnSourceControlAgent.commandEnv documents that WSL applies assignments in the guest while native callers pre-merge them into env.

The prior inline concern (whole-environment uppercasing on Windows) was answered as deliberate normalization, with the affected assertion normalized and a regression test added, so that thread is retired. The five touched test files pass (34 tests).

  ^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}