Skip to content

Fix: support VAR=value env prefixes in agent command override - #5363

Closed
carlosbaraza wants to merge 2 commits into
stablyai:mainfrom
carlosbaraza:fix/agent-command-override-env-prefix
Closed

carlosbaraza wants to merge 2 commits into
stablyai:mainfrom
carlosbaraza:fix/agent-command-override-env-prefix

Conversation

@carlosbaraza

@carlosbaraza carlosbaraza commented Jun 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes the "Branch auto-name failed" error (CLAUDE_CODE_NO_FLICKER=1 not found on PATH. Install Claude to use AI commit messages.) that occurs whenever the agent command override contains anything other than a bare binary — e.g. an environment-variable prefix.

The agent command override (shared by AI commit messages, branch auto-naming, and PR-field generation) is tokenized and spawned via argv with no shell. The first token was always treated as the binary, so an override like CLAUDE_CODE_NO_FLICKER=1 claude tried to exec a file literally named CLAUDE_CODE_NO_FLICKER=1 → ENOENT → the error above.

Fix: parse leading VAR=value tokens as POSIX-style environment assignments (letter/underscore-led name, split on the first =) before resolving the binary, then merge them over the spawn environment. So CLAUDE_CODE_NO_FLICKER=1 claude now runs claude with CLAUDE_CODE_NO_FLICKER=1 in its environment.

Why this approach (and not a shell)

The tokenizer is deliberately no-shell (see comment in commit-message-prompt.ts) to avoid $VAR/glob/injection surface across macOS/Linux/Windows. Env-assignment prefixes are the common, safe case that previously broke; parsing them keeps the argv-only model with no new injection or cross-platform surface. Shell-only constructs (aliases, functions) still require an absolute path — they cannot work without a shell, and that is unchanged.

Scope

  • extractLeadingEnvAssignments() helper peels leading VAR=value tokens.
  • Wired into both planAgentBinary (preset overrides) and planCustomCommand (fully custom commands).
  • plan.env threaded through the local spawn, model discovery, and the SSH relay (the relay handler already merges params.env over process.env).

Screenshots

No visual change. (The only user-visible difference is the error no longer appearing for valid env-prefixed overrides.)

Testing

  • pnpm lint — clean for changed files; the remaining max-lines errors are pre-existing in unrelated files (MobileHero.tsx, CodexUsagePane.tsx, …) and present on main.
  • pnpm typecheck — no new errors from this change; one pre-existing unrelated error in folder-workspace-composer-submit.ts exists on main (verified via git stash).
  • pnpm test — added/updated unit + spawn-level integration tests; all relevant suites pass (120 tests). The integration test was confirmed to fail against the old implementation (binary was CLAUDE_CODE_NO_FLICKER=1) and pass with the fix.
  • pnpm build — not run: build:desktop runs typecheck first, which fails on the pre-existing unrelated error above.
  • Added or updated high-quality tests that would catch regressions

Tests added:

  • extractLeadingEnvAssignments unit tests (peeling, multiple assignments, stop-at-binary, = in value, empty value, invalid names, no-assignment).
  • planCustomCommand + planCommitMessageGeneration/planAgentBinary env-prefix cases.
  • generateCommitMessageFromContext spawn-level test asserting the real binary is spawned with the env var applied (not the VAR=value token).

AI Review Report

Reviewed for correctness and cross-platform behavior. The change is argv-only — env vars are passed via spawn's env option, which behaves identically on macOS/Linux/Windows; no path separators, shortcuts, or shell strings are introduced. Windows .cmd/PATH resolution (resolveCliCommand / getSpawnArgsForWindows) is unchanged and operates on the now-correct binary. SSH path was checked end-to-end: plan.env is forwarded in the exec payload and the relay handler (agent-exec-handler.ts) already merges it over process.env. Verified the fix does not alter behavior when no assignments are present (env key is omitted; existing toEqual tests unchanged).

Security Audit

Command execution: assignments come from the user's own settings and are passed as a child-process env map — never to a shell — so this introduces no shell-injection vector and does not expand $VAR, globs, or substitutions. Values may contain = (split on first = only). No secrets, auth, IPC, or path-handling changes. Env precedence is user-assignment-over-base, matching POSIX VAR=val cmd semantics.

Notes

Aliases / shell functions remain unsupported by design (no shell); users should point the override at an absolute binary path. Pre-existing lint/typecheck failures on main are unrelated to this change and called out above.

ELI5

Agent command overrides like VAR=1 claude broke branch auto-name and similar features (PATH “not found”). Env prefixes are parsed correctly so the real binary runs with those variables.

The agent command override (used for AI commit messages, branch
auto-naming, and PR-field generation) is tokenized and spawned via argv
with no shell. The first token was always treated as the binary, so an
override like `CLAUDE_CODE_NO_FLICKER=1 claude` tried to exec a file
literally named `CLAUDE_CODE_NO_FLICKER=1` and failed with:

  CLAUDE_CODE_NO_FLICKER=1 not found on PATH. Install Claude...

Parse leading `VAR=value` tokens as POSIX-style environment assignments
(letter/underscore-led name, split on first `=`) before resolving the
binary, then merge them over the spawn environment. Stays argv-only — no
shell — so there's no new injection or cross-platform surface. Applies to
preset overrides and fully custom commands, and threads through local
spawn, model discovery, and the SSH relay (which already merges env).

Shell-only constructs (aliases, functions) still require an absolute path
since there is no shell; that is unchanged.
@coderabbitai

coderabbitai Bot commented Jun 14, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The change adds POSIX-style VAR=value leading token support to agent command overrides. A new exported extractLeadingEnvAssignments helper in commit-message-prompt.ts parses leading assignment tokens from a command token list and returns them as a Record<string, string> plus the remaining tokens. CustomCommandPlan's success variant gains an optional env field. CommitMessagePlan is extended with the same optional env field; planAgentBinary and planCommitMessageGeneration are updated to extract and propagate these assignments. In commit-message-text-generation.ts, both discoverCommitMessageModelsLocal and runLocalPlan now merge plan.env over the base environment when spawning subprocesses. SshGitProvider.executeCommitMessagePlan conditionally forwards plan.env to the remote non-interactive exec payload.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change: adding support for VAR=value environment variable prefixes in agent command overrides.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed All required sections are present and sufficiently detailed, with testing, AI review, security, and notes covered.

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/main/providers/ssh-git-provider.ts (1)

162-165: ⚡ Quick win

Add a regression assertion for plan.env passthrough in SSH provider tests.

This forwarding path is critical for remote parity. Please extend src/main/providers/ssh-git-provider.test.ts to assert agent.execNonInteractive receives env when plan.env is set.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: c8cebd95-3124-468d-94bb-4d020f61fee6

📥 Commits

Reviewing files that changed from the base of the PR and between c377ec1 and 17ce14c.

📒 Files selected for processing (7)
  • src/main/providers/ssh-git-provider.ts
  • src/main/text-generation/commit-message-text-generation.test.ts
  • src/main/text-generation/commit-message-text-generation.ts
  • src/shared/commit-message-plan.test.ts
  • src/shared/commit-message-plan.ts
  • src/shared/commit-message-prompt.test.ts
  • src/shared/commit-message-prompt.ts

Comment on lines +289 to +291
const spawnEnv = planned.plan.env
? { ...(env ?? process.env), ...planned.plan.env }
: (env ?? process.env)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

In Node.js child_process.spawn on Windows, how are duplicate environment keys that differ only by case (e.g., PATH vs Path) handled, and which value wins?

💡 Result:

In Node.js, when using child_process.spawn on Windows, duplicate environment keys that differ only by case are handled by a sanitization process that removes the duplicates [1]. Specifically, Node.js sorts the environment keys lexicographically and filters them so that only one entry remains for each case-insensitive key [1]. When multiple variants of a key exist (e.g., PATH and Path), the entry that comes first in lexicographic order (based on the original key string) is the one that is kept and passed to the spawned subprocess [2][1]. This behavior was implemented to address issues where certain applications or system processes become confused by the presence of duplicate environment variables on Windows, which natively treats environment variable names as case-insensitive [1]. Consequently, if you provide an environment object to spawn, you should ensure that only the desired version of any case-variant key is included, or be aware that the lexicographically first variant will effectively win [2][3].

Citations:


🏁 Script executed:

# Find and examine the file
find . -name "commit-message-text-generation.ts" -type f

Repository: stablyai/orca

Length of output: 118


🏁 Script executed:

# Check lines around 289-291 and 495-497 in the identified file
if [ -f "src/main/text-generation/commit-message-text-generation.ts" ]; then
  echo "=== Lines 280-300 ===" && sed -n '280,300p' "src/main/text-generation/commit-message-text-generation.ts" | cat -n
  echo ""
  echo "=== Lines 485-510 ===" && sed -n '485,510p' "src/main/text-generation/commit-message-text-generation.ts" | cat -n
fi

Repository: stablyai/orca

Length of output: 2409


🏁 Script executed:

# Search for any existing platform checks related to env or PATH in the file
rg -n "process\.platform|win32|darwin|linux" src/main/text-generation/commit-message-text-generation.ts

Repository: stablyai/orca

Length of output: 208


🏁 Script executed:

# Check if there are any existing PATH normalization utilities in the codebase
rg -n "PATH|path.*normalize|env.*case" --type ts --type js -A 2 | head -100

Repository: stablyai/orca

Length of output: 6881


Normalize PATH/Path environment key on Windows when merging plan.env.

These merges can result in both PATH and Path keys in the spawned environment. Node.js spawn on Windows sanitizes environment variables and keeps the lexicographically first key; PATH will win, causing any Path override to be silently ignored during process spawning.

-const spawnEnv = plan.env ? { ...(env ?? process.env), ...plan.env } : (env ?? process.env)
+let spawnEnv = plan.env ? { ...(env ?? process.env), ...plan.env } : (env ?? process.env)
+if (process.platform === 'win32' && plan.env) {
+  const pathOverride = plan.env.PATH ?? plan.env.Path
+  if (pathOverride !== undefined) {
+    delete spawnEnv.PATH
+    delete spawnEnv.Path
+    spawnEnv.Path = pathOverride
+  }
+}

Also applies to: 495-497

* upstream/main: (341 commits)
  Show and Filter Automation-Created Workspaces (stablyai#5697)
  Fix premature project header truncation
  Send review notes to any running agent of a worktree (stablyai#5654)
  Fix workspace sidebar appearance setting (stablyai#5725)
  Highlight the open file in the Source Control sidebar (stablyai#5667)
  release: v1.4.80-rc.1
  Fix premature Codex completion notifications (stablyai#5682)
  Improve Codex startup prompt reliability (stablyai#5686)
  Keep hibernated agent completions visible (stablyai#5689)
  Move Prompt Cache Timer setting to Agents pane, simplify expired state (stablyai#5691)
  release: v1.4.80-rc.0
  Skip GitHub CLI preflight checks if no GitHub projects exist
  fix: restore font family keyboard navigation in settings
  Update README downloads badge
  fix(landing): make GitHub CLI setup nudge dismissible and provider-aware (stablyai#5688)
  Fix nested workspace card indentation (stablyai#5684)
  Fix flaky e2e tests (stablyai#5680)
  Replace plus icon with play button on quick command entry (stablyai#5674)
  Use managed Pi and OMP extensions (stablyai#5681)
  Narrow computer-use e2e trigger (stablyai#5685)
  ...

# Conflicts:
#	src/main/text-generation/commit-message-text-generation.test.ts
#	src/main/text-generation/commit-message-text-generation.ts
@ghost

ghost commented Jun 18, 2026

Copy link
Copy Markdown

Ready to review this PR? Stage has broken it down into 3 individual chapters for you:

Title
1 Implement environment assignment parsing utility
2 Integrate env parsing into command planning
3 Apply environment variables to process execution
Open in Stage

Chapters generated by Stage for commit 8fc6632 on Jun 18, 2026 8:43pm UTC.

@nwparker nwparker added the bug Something isn't working label Jul 27, 2026
@nwparker

Copy link
Copy Markdown
Contributor

Closing as superseded by merged #22427, which carries the reviewed fix and regression coverage. Thank you for the original contribution.

@nwparker nwparker closed this Sep 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants