From 17ce14c50ad9fa5a82936c43a43208f26b29ecab Mon Sep 17 00:00:00 2001 From: Carlos Baraza Haro Date: Sun, 14 Jun 2026 16:59:22 +0200 Subject: [PATCH] Support VAR=value env prefixes in agent command override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/main/providers/ssh-git-provider.ts | 3 + .../commit-message-text-generation.test.ts | 46 +++++++++++++ .../commit-message-text-generation.ts | 13 +++- src/shared/commit-message-plan.test.ts | 62 ++++++++++++++++++ src/shared/commit-message-plan.ts | 29 +++++++-- src/shared/commit-message-prompt.test.ts | 65 +++++++++++++++++++ src/shared/commit-message-prompt.ts | 49 ++++++++++++-- 7 files changed, 252 insertions(+), 15 deletions(-) diff --git a/src/main/providers/ssh-git-provider.ts b/src/main/providers/ssh-git-provider.ts index 477555df9e60..176003318d13 100644 --- a/src/main/providers/ssh-git-provider.ts +++ b/src/main/providers/ssh-git-provider.ts @@ -159,6 +159,9 @@ export class SshGitProvider implements IGitProvider { cwd, stdin: plan.stdinPayload, timeoutMs, + // Why: forward leading `VAR=value` env assignments from the override so + // the remote CLI runs with the same environment as a local spawn. + ...(plan.env ? { env: plan.env } : {}), operation }, undefined, diff --git a/src/main/text-generation/commit-message-text-generation.test.ts b/src/main/text-generation/commit-message-text-generation.test.ts index d640cc160e04..eb56f53cb2f7 100644 --- a/src/main/text-generation/commit-message-text-generation.test.ts +++ b/src/main/text-generation/commit-message-text-generation.test.ts @@ -856,6 +856,52 @@ describe('generateCommitMessageFromContext', () => { ) }) + it('spawns the real binary with leading VAR=value override tokens applied to the env', async () => { + const listeners = new Map void>() + const child = { + pid: 123, + kill: vi.fn(), + stdout: { on: vi.fn((event, callback) => listeners.set(`stdout:${event}`, callback)) }, + stderr: { on: vi.fn((event, callback) => listeners.set(`stderr:${event}`, callback)) }, + stdin: { end: vi.fn() }, + on: vi.fn((event, callback) => listeners.set(event, callback)) + } + spawnMock.mockReturnValue(child as never) + + const pending = generateCommitMessageFromContext( + { + branch: 'main', + stagedSummary: 'M\tREADME.md', + stagedPatch: '+hello' + }, + { + agentId: 'claude', + model: 'sonnet', + agentCommandOverride: 'CLAUDE_CODE_NO_FLICKER=1 claude' + }, + { + kind: 'local', + cwd: '/repo' + } + ) + + listeners.get('stdout:data')?.(Buffer.from('Add README note\n')) + listeners.get('close')?.(0) + + await expect(pending).resolves.toMatchObject({ + success: true, + message: 'Add README note' + }) + // Why: the env-assignment prefix must become an env var, not the binary. + expect(spawnMock).toHaveBeenCalledWith( + 'claude', + expect.not.arrayContaining(['CLAUDE_CODE_NO_FLICKER=1']), + expect.objectContaining({ + env: expect.objectContaining({ CLAUDE_CODE_NO_FLICKER: '1' }) + }) + ) + }) + it('keeps local commit-message and pull-request cancellation lanes separate', async () => { const children: { kill: ReturnType diff --git a/src/main/text-generation/commit-message-text-generation.ts b/src/main/text-generation/commit-message-text-generation.ts index bfa1ff6cc7e5..e3b23b125964 100644 --- a/src/main/text-generation/commit-message-text-generation.ts +++ b/src/main/text-generation/commit-message-text-generation.ts @@ -256,7 +256,8 @@ function planModelDiscovery( binary: command.binary, args: [...command.prefixArgs, ...modelDiscovery.args], stdinPayload: null, - label: spec.label + label: spec.label, + ...(Object.keys(command.env).length ? { env: command.env } : {}) } } } @@ -277,13 +278,17 @@ export async function discoverCommitMessageModelsLocal( return new Promise((resolve) => { let child: ChildProcess - const spawnEnv = env ?? process.env try { const planned = planModelDiscovery(spec, agentCommandOverride) if (!planned.ok) { resolve({ success: false, error: planned.error }) return } + // Why: merge env-assignment prefixes from the override so model discovery + // runs the real binary with the same environment as generation. + const spawnEnv = planned.plan.env + ? { ...(env ?? process.env), ...planned.plan.env } + : (env ?? process.env) const resolvedBinary = process.platform === 'win32' ? resolveCliCommand(planned.plan.binary, { @@ -487,7 +492,9 @@ async function runLocalPlan( return new Promise((resolve) => { let child: ChildProcess try { - const spawnEnv = env ?? process.env + // Why: `plan.env` carries leading `VAR=value` assignments from the agent + // command override; merge them over the base env so they reach the CLI. + const spawnEnv = plan.env ? { ...(env ?? process.env), ...plan.env } : (env ?? process.env) const resolvedBinary = process.platform === 'win32' ? resolveCliCommand(binary, { pathEnv: spawnEnv.PATH ?? spawnEnv.Path ?? null }) diff --git a/src/shared/commit-message-plan.test.ts b/src/shared/commit-message-plan.test.ts index 2aa20a4cd632..a7a4f9cab805 100644 --- a/src/shared/commit-message-plan.test.ts +++ b/src/shared/commit-message-plan.test.ts @@ -333,6 +333,68 @@ describe('planCommitMessageGeneration', () => { }) }) + it('parses leading env assignments in a preset override into plan.env, spawning the real binary', () => { + const result = planCommitMessageGeneration( + { + agentId: 'claude', + model: 'sonnet', + agentCommandOverride: 'CLAUDE_CODE_NO_FLICKER=1 claude' + }, + 'PROMPT' + ) + + expect(result).toMatchObject({ + ok: true, + plan: { + binary: 'claude', + env: { CLAUDE_CODE_NO_FLICKER: '1' } + } + }) + // Why: the env-assignment token must not leak into the binary or argv. + if (result.ok) { + expect(result.plan.binary).toBe('claude') + expect(result.plan.args).not.toContain('CLAUDE_CODE_NO_FLICKER=1') + } + }) + + it('keeps env assignments before a wrapper binary in the override', () => { + const result = planCommitMessageGeneration( + { + agentId: 'codex', + model: 'gpt-5.4-mini', + agentCommandOverride: 'FOO=bar npx codex' + }, + 'PROMPT' + ) + + expect(result).toMatchObject({ + ok: true, + plan: { + binary: 'npx', + env: { FOO: 'bar' } + } + }) + if (result.ok) { + expect(result.plan.args.slice(0, 2)).toEqual(['codex', 'exec']) + } + }) + + it('omits plan.env when the override carries no env assignments', () => { + const result = planCommitMessageGeneration( + { + agentId: 'codex', + model: 'gpt-5.4-mini', + agentCommandOverride: 'npx codex' + }, + 'PROMPT' + ) + + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.plan).not.toHaveProperty('env') + } + }) + it('rejects invalid preset agent command overrides before spawning', () => { const result = planCommitMessageGeneration( { diff --git a/src/shared/commit-message-plan.ts b/src/shared/commit-message-plan.ts index bef816dd7207..1ade131b3728 100644 --- a/src/shared/commit-message-plan.ts +++ b/src/shared/commit-message-plan.ts @@ -3,7 +3,11 @@ import { getCommitMessageModel, isCustomAgentId } from './commit-message-agent-spec' -import { planCustomCommand, tokenizeCustomCommandTemplate } from './commit-message-prompt' +import { + extractLeadingEnvAssignments, + planCustomCommand, + tokenizeCustomCommandTemplate +} from './commit-message-prompt' import type { TuiAgent } from './types' // Why: planning is a pure transformation from "user request + prompt text" @@ -28,6 +32,12 @@ export type CommitMessagePlan = { stdinPayload: string | null /** Human-readable label used in error prefixes (e.g. "Claude failed: ..."). */ label: string + /** + * Environment variables parsed from leading `VAR=value` tokens in the agent + * command override / custom command. Merged over the spawn environment. + * Present only when assignments were supplied. + */ + env?: Record } export type CommitMessagePlanResult = @@ -37,21 +47,24 @@ export type CommitMessagePlanResult = export function planAgentBinary( defaultBinary: string, commandOverride: string | undefined -): { ok: true; binary: string; prefixArgs: string[] } | { ok: false; error: string } { +): + | { ok: true; binary: string; prefixArgs: string[]; env: Record } + | { ok: false; error: string } { const command = commandOverride?.trim() if (!command) { - return { ok: true, binary: defaultBinary, prefixArgs: [] } + return { ok: true, binary: defaultBinary, prefixArgs: [], env: {} } } const tokenized = tokenizeCustomCommandTemplate(command) if (!tokenized.ok) { return { ok: false, error: `Agent command override is invalid: ${tokenized.error}` } } - const [binary, ...prefixArgs] = tokenized.tokens + const { env, rest } = extractLeadingEnvAssignments(tokenized.tokens) + const [binary, ...prefixArgs] = rest if (!binary) { return { ok: false, error: 'Agent command override must start with a binary name.' } } - return { ok: true, binary, prefixArgs } + return { ok: true, binary, prefixArgs, env } } function planAdditionalAgentArgs( @@ -126,7 +139,8 @@ export function planCommitMessageGeneration( stdinPayload: planned.stdinPayload, // Why: a custom command has no friendly name, so the binary doubles // as the label in error prefixes ("ollama failed: ..."). - label: planned.binary + label: planned.binary, + ...(planned.env ? { env: planned.env } : {}) } } } @@ -180,7 +194,8 @@ export function planCommitMessageGeneration( binary: command.binary, args: [...command.prefixArgs, ...args], stdinPayload: spec.promptDelivery === 'stdin' ? prompt : null, - label: spec.label + label: spec.label, + ...(Object.keys(command.env).length ? { env: command.env } : {}) } } } diff --git a/src/shared/commit-message-prompt.test.ts b/src/shared/commit-message-prompt.test.ts index 436075b397fc..2ae87857c41e 100644 --- a/src/shared/commit-message-prompt.test.ts +++ b/src/shared/commit-message-prompt.test.ts @@ -3,6 +3,7 @@ import { buildCommitPrompt, cleanGeneratedCommitMessage, extractAgentErrorMessage, + extractLeadingEnvAssignments, planCustomCommand, STAGED_DIFF_BYTE_BUDGET, tokenizeCustomCommandTemplate, @@ -220,12 +221,76 @@ describe('tokenizeCustomCommandTemplate', () => { }) }) +describe('extractLeadingEnvAssignments', () => { + it('peels a single leading VAR=value token off the front', () => { + const r = extractLeadingEnvAssignments(['CLAUDE_CODE_NO_FLICKER=1', 'claude', '-p']) + expect(r).toEqual({ env: { CLAUDE_CODE_NO_FLICKER: '1' }, rest: ['claude', '-p'] }) + }) + + it('peels multiple consecutive assignments', () => { + const r = extractLeadingEnvAssignments(['FOO=bar', 'BAZ=qux', 'claude']) + expect(r).toEqual({ env: { FOO: 'bar', BAZ: 'qux' }, rest: ['claude'] }) + }) + + it('stops at the first non-assignment token (later VAR=val stays an arg)', () => { + const r = extractLeadingEnvAssignments(['FOO=bar', 'claude', 'KEY=val']) + expect(r).toEqual({ env: { FOO: 'bar' }, rest: ['claude', 'KEY=val'] }) + }) + + it('splits on the first = so values may contain =', () => { + const r = extractLeadingEnvAssignments(['URL=https://x?a=b', 'claude']) + expect(r).toEqual({ env: { URL: 'https://x?a=b' }, rest: ['claude'] }) + }) + + it('allows empty values', () => { + const r = extractLeadingEnvAssignments(['EMPTY=', 'claude']) + expect(r).toEqual({ env: { EMPTY: '' }, rest: ['claude'] }) + }) + + it('does not treat =value or a digit-led name as an assignment', () => { + expect(extractLeadingEnvAssignments(['=foo', 'claude'])).toEqual({ + env: {}, + rest: ['=foo', 'claude'] + }) + expect(extractLeadingEnvAssignments(['1AB=x', 'claude'])).toEqual({ + env: {}, + rest: ['1AB=x', 'claude'] + }) + }) + + it('returns an empty env when there are no assignments', () => { + expect(extractLeadingEnvAssignments(['claude', '-p'])).toEqual({ + env: {}, + rest: ['claude', '-p'] + }) + }) +}) + describe('planCustomCommand', () => { it('routes prompt via stdin when {prompt} is absent', () => { const r = planCustomCommand('claude -p', 'COMMIT MSG') expect(r).toEqual({ ok: true, binary: 'claude', args: ['-p'], stdinPayload: 'COMMIT MSG' }) }) + it('peels leading env assignments and spawns the real binary, not VAR=value', () => { + const r = planCustomCommand('CLAUDE_CODE_NO_FLICKER=1 claude -p', 'COMMIT MSG') + expect(r).toEqual({ + ok: true, + binary: 'claude', + args: ['-p'], + stdinPayload: 'COMMIT MSG', + env: { CLAUDE_CODE_NO_FLICKER: '1' } + }) + }) + + it('errors when the command is only env assignments', () => { + const r = planCustomCommand('FOO=bar', 'PROMPT') + expect(r.ok).toBe(false) + if (!r.ok) { + expect(r.error).toMatch(/binary name/i) + } + }) + it('substitutes {prompt} as a whole token via argv', () => { const r = planCustomCommand('codex exec {prompt}', 'PROMPT') expect(r).toEqual({ ok: true, binary: 'codex', args: ['exec', 'PROMPT'], stdinPayload: null }) diff --git a/src/shared/commit-message-prompt.ts b/src/shared/commit-message-prompt.ts index f8aeb495a1aa..864772856cfa 100644 --- a/src/shared/commit-message-prompt.ts +++ b/src/shared/commit-message-prompt.ts @@ -236,8 +236,44 @@ export function tokenizeCustomCommandTemplate(template: string): TokenizeCustomC return { ok: true, tokens } } +// Why: POSIX `VAR=value cmd` env-assignment prefix. We spawn via argv (no +// shell), so without this a token like `CLAUDE_CODE_NO_FLICKER=1` would be +// treated as the binary name and fail with ENOENT. Names follow the shell +// rule: a letter/underscore followed by word chars, up to the first `=`. +const ENV_ASSIGNMENT_PATTERN = /^[A-Za-z_][A-Za-z0-9_]*=/ + +/** + * Splits leading `VAR=value` tokens off the front of a tokenized command into + * an environment map, returning the remaining tokens (binary + argv). Stops at + * the first non-assignment token, so a `KEY=val` appearing after the binary is + * preserved as a normal argument. + */ +export function extractLeadingEnvAssignments(tokens: string[]): { + env: Record + rest: string[] +} { + const env: Record = {} + let i = 0 + for (; i < tokens.length; i++) { + const token = tokens[i] + if (!ENV_ASSIGNMENT_PATTERN.test(token)) { + break + } + const eq = token.indexOf('=') + env[token.slice(0, eq)] = token.slice(eq + 1) + } + return { env, rest: tokens.slice(i) } +} + export type CustomCommandPlan = - | { ok: true; binary: string; args: string[]; stdinPayload: string | null } + | { + ok: true + binary: string + args: string[] + stdinPayload: string | null + /** Present only when the command carried leading `VAR=value` assignments. */ + env?: Record + } | { ok: false; error: string } /** @@ -257,25 +293,28 @@ export function planCustomCommand(template: string, prompt: string): CustomComma if (tokenized.tokens.length === 0) { return { ok: false, error: 'Custom command is empty.' } } - const [binary, ...rest] = tokenized.tokens + const { env, rest: commandTokens } = extractLeadingEnvAssignments(tokenized.tokens) + const [binary, ...rest] = commandTokens if (!binary) { return { ok: false, error: 'Custom command must start with a binary name.' } } + const envFields = Object.keys(env).length ? { env } : {} const substitute = (token: string): string => token.includes(CUSTOM_PROMPT_PLACEHOLDER) ? token.split(CUSTOM_PROMPT_PLACEHOLDER).join(prompt) : token - const usesPlaceholder = tokenized.tokens.some((t) => t.includes(CUSTOM_PROMPT_PLACEHOLDER)) + const usesPlaceholder = commandTokens.some((t) => t.includes(CUSTOM_PROMPT_PLACEHOLDER)) if (usesPlaceholder) { return { ok: true, binary: substitute(binary), args: rest.map(substitute), - stdinPayload: null + stdinPayload: null, + ...envFields } } - return { ok: true, binary, args: rest, stdinPayload: prompt } + return { ok: true, binary, args: rest, stdinPayload: prompt, ...envFields } } // Why: agent CLIs (Codex, Claude) prefix their stdout/stderr with config