diff --git a/src/main/claude/claude-background-run-isolation-real-cli.test.ts b/src/main/claude/claude-background-run-isolation-real-cli.test.ts new file mode 100644 index 00000000000..228bc56fc2b --- /dev/null +++ b/src/main/claude/claude-background-run-isolation-real-cli.test.ts @@ -0,0 +1,67 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { runProcess } from '@orca/process-host' +import { withCliRuntimeOnPath } from '../../shared/node-cli-command-resolution' +import { claudeBackgroundArgs } from '../../shared/claude-background-run-isolation' +import { CLAUDE_MODEL_LIST_ARGS } from '../../shared/claude-model-list-probe' +import { realClaudeAvailable, realClaudeCommand } from './claude-real-cli-availability-test-support' + +// Pins the contract with Claude itself: its `mcp_status` and `get_settings` answers (no turn spent) +// in a throwaway home whose own settings allowlist its user-scope server. +const root = mkdtempSync(join(tmpdir(), 'orca-claude-isolation-')) +afterEach(() => rmSync(root, { recursive: true, force: true })) + +async function loaded(args: string[]): Promise<{ servers: unknown; hooksOff: unknown }> { + const request = (id: string) => + `${JSON.stringify({ type: 'control_request', request_id: id, request: { subtype: id } })}\n` + const { stdout } = await runProcess({ + program: realClaudeCommand, + args, + cwd: root, + input: request('mcp_status') + request('get_settings'), + timeoutMs: 60_000, + env: withCliRuntimeOnPath(realClaudeCommand, { + ...process.env, + HOME: root, + CLAUDE_CONFIG_DIR: join(root, 'config') + }) + }) + const answers = new Map( + stdout.split('\n').flatMap((line) => { + const response = line.startsWith('{') ? JSON.parse(line).response : undefined + return response?.request_id ? [[response.request_id, response.response]] : [] + }) + ) + return { + servers: answers.get('mcp_status')?.mcpServers?.map((server: { name: string }) => server.name), + hooksOff: answers.get('get_settings')?.effective?.disableAllHooks ?? false + } +} + +describe.skipIf(!realClaudeAvailable)('background Claude runs with the real CLI', () => { + it('load none of the user MCP servers or hooks, and keep a --mcp-config the user passed', async () => { + const server = { type: 'stdio', command: '/bin/cat', args: [] } + mkdirSync(join(root, 'config')) + writeFileSync( + join(root, 'config', '.claude.json'), + JSON.stringify({ mcpServers: { user: server } }) + ) + writeFileSync( + join(root, 'config', 'settings.json'), + JSON.stringify({ allowedMcpServers: [{ serverName: 'user' }, { serverName: 'explicit' }] }) + ) + writeFileSync(join(root, 'mcp.json'), JSON.stringify({ mcpServers: { explicit: server } })) + const listing = CLAUDE_MODEL_LIST_ARGS.slice(0, -claudeBackgroundArgs().length) + + expect(await loaded(listing)).toEqual({ servers: ['user'], hooksOff: false }) + expect(await loaded([...listing, ...claudeBackgroundArgs()])).toEqual({ + servers: [], + hooksOff: true + }) + expect( + await loaded([...listing, ...claudeBackgroundArgs(), '--mcp-config', join(root, 'mcp.json')]) + ).toEqual({ servers: ['explicit'], hooksOff: true }) + }, 180_000) +}) diff --git a/src/main/claude/claude-command-catalog-probe.ts b/src/main/claude/claude-command-catalog-probe.ts index 6899810995a..4397db87fcb 100644 --- a/src/main/claude/claude-command-catalog-probe.ts +++ b/src/main/claude/claude-command-catalog-probe.ts @@ -1,5 +1,4 @@ import type { AgentSessionSlashCommand } from '../../shared/agent-session-wire' -import { CLAUDE_MODEL_LIST_ARGS } from '../../shared/claude-model-list-probe' import { assertJsonTextStructureWithinLimits } from '../../shared/json-text-structure-limit' import { runClaudeListing, type ClaudeModelCatalogProbeDeps } from './claude-model-catalog-probe' import { ClaudeSlashCommandCatalog } from './claude-slash-command-catalog' @@ -14,15 +13,6 @@ const CLAUDE_COMMAND_LIST_STDIN = `${JSON.stringify({ request: { subtype: 'initialize' } })}\n` -// Hooks and MCP servers only slow the answer: a SessionStart hook would run on every refresh, -// and every configured MCP server would start just to be stopped. -const CLAUDE_COMMAND_LIST_ARGS = [ - ...CLAUDE_MODEL_LIST_ARGS, - '--strict-mcp-config', - '--settings', - JSON.stringify({ disableAllHooks: true }) -] - const INITIALIZE_JSON_LIMITS = { structuralTokens: 64 * 1024, nestingDepth: 16 } as const export type ClaudeCommandCatalogProbe = (request: { @@ -38,7 +28,6 @@ export function createClaudeCommandCatalogProbe( const stdout = await runClaudeListing(deps, { accountHomePath, cwd, - args: CLAUDE_COMMAND_LIST_ARGS, stdin: CLAUDE_COMMAND_LIST_STDIN, site: 'claude-command-catalog-probe' }) diff --git a/src/main/claude/claude-model-catalog-probe.test.ts b/src/main/claude/claude-model-catalog-probe.test.ts index 5a12ff794ba..7fd12f41a77 100644 --- a/src/main/claude/claude-model-catalog-probe.test.ts +++ b/src/main/claude/claude-model-catalog-probe.test.ts @@ -250,3 +250,17 @@ describe('Claude catalog availability', () => { } }) }) + +// Only completed failed exits supply process facts to the shared retry policy. +it.each([null, 0, 1])('enterprise retry uses the actual exit code %s', async (exitCode) => { + const stderr = 'You cannot use --strict-mcp-config when an enterprise MCP config is present' + const runListing = vi.fn(async () => LISTED) + runListing.mockRejectedValueOnce( + new AgentModelCatalogProbeError(stderr, 'exit', false, { exitCode, stdout: '', stderr }) + ) + const result = createClaudeModelCatalogProbe({ ...probeDeps(), runListing })(HOME('/homes/a')) + await (exitCode === 1 + ? expect(result).resolves.toMatchObject({ models: [{ id: 'sonnet' }] }) + : expect(result).rejects.toMatchObject({ failedExit: { exitCode } })) + expect(runListing).toHaveBeenCalledTimes(exitCode === 1 ? 2 : 1) +}) diff --git a/src/main/claude/claude-model-catalog-probe.ts b/src/main/claude/claude-model-catalog-probe.ts index 63b1926bc96..8c990e5aaf1 100644 --- a/src/main/claude/claude-model-catalog-probe.ts +++ b/src/main/claude/claude-model-catalog-probe.ts @@ -1,9 +1,10 @@ import { - CLAUDE_MODEL_LIST_ARGS, + buildClaudeModelListArgs, CLAUDE_MODEL_LIST_STDIN, parseClaudeModelList } from '../../shared/claude-model-list-probe' import { requireLegacyAgentSessionAccountHome } from '../../shared/agent-session-account-home' +import { runClaudeBackgroundAttempts } from '../../shared/claude-background-run-isolation' import { parseClaudeModels } from '../../shared/commit-message-model-parsers' import { AgentModelCatalogUnavailableError } from '../native-chat/agent-model-catalog/agent-model-catalog-unavailable' import { claudeConfigDirEnvPatch } from './claude-config-dir-pin' @@ -33,7 +34,6 @@ export async function runClaudeListing( deps: ClaudeModelCatalogProbeDeps, request: { accountHomePath: string - args: readonly string[] stdin: string site: string cwd?: string @@ -46,15 +46,34 @@ export async function runClaudeListing( ...base, ...claudeConfigDirEnvPatch(request.accountHomePath, { env: base }) })) - return (deps.runListing ?? runAgentModelCatalogListing)( - { - command, - args: [...request.args], - stdin: request.stdin, - ...(request.cwd ? { cwd: request.cwd } : {}) - }, - { site: request.site, inheritedEnv: env, signal: request.signal } - ) + const run = (args: readonly string[]) => + (deps.runListing ?? runAgentModelCatalogListing)( + { + command, + args: [...args], + stdin: request.stdin, + ...(request.cwd ? { cwd: request.cwd } : {}) + }, + { site: request.site, inheritedEnv: env, signal: request.signal } + ) + const result = await runClaudeBackgroundAttempts({ + agentId: 'claude', + run: (strictMcp) => + run(buildClaudeModelListArgs(strictMcp)).catch((error: unknown) => { + if (error instanceof AgentModelCatalogProbeError) { + return error + } + throw error + }), + failedExit: (outcome) => + outcome instanceof AgentModelCatalogProbeError && outcome.reason === 'exit' + ? outcome.failedExit + : undefined + }) + if (result instanceof AgentModelCatalogProbeError) { + throw result + } + return result } /** @@ -68,7 +87,6 @@ export function createClaudeModelCatalogProbe( return async (accountHome, options): Promise => { const stdout = await runClaudeListing(deps, { accountHomePath: requireLegacyAgentSessionAccountHome(accountHome).path, - args: CLAUDE_MODEL_LIST_ARGS, stdin: CLAUDE_MODEL_LIST_STDIN, site: 'claude-model-catalog-probe', ...(options?.signal ? { signal: options.signal } : {}) diff --git a/src/main/native-chat/agent-model-catalog/agent-model-catalog-probe-runner.test.ts b/src/main/native-chat/agent-model-catalog/agent-model-catalog-probe-runner.test.ts index 744c13dafa6..8cd8a254672 100644 --- a/src/main/native-chat/agent-model-catalog/agent-model-catalog-probe-runner.test.ts +++ b/src/main/native-chat/agent-model-catalog/agent-model-catalog-probe-runner.test.ts @@ -58,7 +58,11 @@ describe('the shared catalog probe runner', () => { { ...node("process.stderr.write('not signed in');process.exit(3)"), env: NODE_ENV }, { site: 'test-listing' } ) - ).rejects.toMatchObject({ reason: 'exit', message: expect.stringContaining('not signed in') }) + ).rejects.toMatchObject({ + reason: 'exit', + message: expect.stringContaining('not signed in'), + failedExit: { exitCode: 3, stdout: '', stderr: 'not signed in' } + }) }) it('stops a listing that outlives its budget before rejecting', async () => { diff --git a/src/main/native-chat/agent-model-catalog/agent-model-catalog-probe-runner.ts b/src/main/native-chat/agent-model-catalog/agent-model-catalog-probe-runner.ts index 643586ca8dd..b9a910de4a8 100644 --- a/src/main/native-chat/agent-model-catalog/agent-model-catalog-probe-runner.ts +++ b/src/main/native-chat/agent-model-catalog/agent-model-catalog-probe-runner.ts @@ -1,3 +1,4 @@ +import type { ClaudeBackgroundFailedExit } from '../../../shared/claude-background-run-isolation' import type { PipedProcessSpawner } from '@orca/process-host/process-spec' // The one way a catalog probe runs an agent CLI: a disposable child under the provider supervisor, // one budget for the whole listing, bounded output, and a teardown awaited on every path. A probe @@ -32,7 +33,8 @@ export class AgentModelCatalogProbeError extends Error { message: string, readonly reason: AgentModelCatalogProbeFailure, /** The listing's own executable was not found, as the spawn or its supervisor reported. */ - readonly executableMissing = false + readonly executableMissing = false, + readonly failedExit?: ClaudeBackgroundFailedExit ) { super(message) this.name = 'AgentModelCatalogProbeError' @@ -92,12 +94,17 @@ export async function runAgentModelCatalogListing( let onAbort = (): void => {} try { return await new Promise((resolve, reject) => { - const fail = (message: string, reason: AgentModelCatalogProbeFailure): void => + const fail = ( + message: string, + reason: AgentModelCatalogProbeFailure, + failedExit?: ClaudeBackgroundFailedExit + ): void => reject( new AgentModelCatalogProbeError( message, reason, - reason === 'spawn' && managed.executableMissing + reason === 'spawn' && managed.executableMissing, + failedExit ) ) onAbort = () => fail(`${launch.command} listing stopped`, 'stopped') @@ -132,7 +139,8 @@ export async function runAgentModelCatalogListing( const detail = stderrDetail() fail( `${launch.command} exited ${code ?? 'by signal'}${detail ? `: ${detail}` : ''}`, - 'exit' + 'exit', + { exitCode: code, stdout: output.text(), stderr: managed.stderrTail() } ) } }) diff --git a/src/main/text-generation/agent-generation-request.ts b/src/main/text-generation/agent-generation-request.ts new file mode 100644 index 00000000000..4eae6591d30 --- /dev/null +++ b/src/main/text-generation/agent-generation-request.ts @@ -0,0 +1,104 @@ +import { runClaudeBackgroundAttempts } from '../../shared/claude-background-run-isolation' +import { + cleanGeneratedCommitMessage, + stripPrefilledReasoningPreamble, + type CommandTemplateBackslash +} from '../../shared/commit-message-prompt' +import { isCustomAgentId } from '../../shared/commit-message-agent-spec' +import { + planCommitMessageGeneration, + type CommitMessagePlan +} from '../../shared/commit-message-plan' +import type { ResolvedSourceControlAiGenerationParams } from '../../shared/source-control-ai' +import { openCodeVariantRetryPlan } from '../../shared/opencode-generation-command' +import { runLocalPlanForAgent } from './source-control-local-generation' +import { runRemoteSourceControlPlan } from './source-control-remote-generation' +import type { + CommitMessageGenerationTarget, + InternalTextGenerationResult, + SpawnSourceControlAgent, + TextGenerationOperation +} from './source-control-text-generation-types' + +type GenerateParams = ResolvedSourceControlAiGenerationParams + +export function commandBackslashMode( + target: CommitMessageGenerationTarget, + platform: NodeJS.Platform = process.platform +): CommandTemplateBackslash { + return platform === 'win32' && target.kind === 'local' && !target.wslDistro ? 'literal' : 'escape' +} + +export async function runGenerationRequest(input: { + params: GenerateParams + prompt: string + target: CommitMessageGenerationTarget + emptyResultName: string + operation: TextGenerationOperation + spawnAgent: SpawnSourceControlAgent +}): Promise { + const params = { ...input.params, backslash: commandBackslashMode(input.target) } + const planned = planCommitMessageGeneration(params, input.prompt) + if (!planned.ok) { + return { success: false, error: planned.error } + } + const plan = planned.plan + const execute = (plan: CommitMessagePlan): Promise => + input.target.kind === 'remote' + ? runRemoteSourceControlPlan({ + plan, + target: input.target, + emptyResultName: input.emptyResultName, + operation: input.operation + }) + : runLocalPlanForAgent({ + agentId: input.params.agentId, + plan, + target: input.target, + emptyResultName: input.emptyResultName, + operation: input.operation, + spawnAgent: input.spawnAgent + }) + let result = await runClaudeBackgroundAttempts({ + agentId: input.params.agentId, + run: (strictMcp): Promise => { + if (strictMcp) { + return execute(plan) + } + const retry = planCommitMessageGeneration(params, input.prompt, { strictMcp }) + return retry.ok + ? execute(retry.plan) + : Promise.resolve({ success: false, error: retry.error }) + }, + failedExit: (outcome) => (outcome.success ? undefined : outcome.failureOutput) + }) + if ( + input.operation !== 'jira-issue-summary' && + !result.success && + input.params.agentId === 'opencode' + ) { + const retry = openCodeVariantRetryPlan(plan, result.failureOutput?.stderr ?? '') + if (retry) { + result = await execute(retry) + } + } + // Why: only a custom command runs a raw model whose chat template can swallow + // the opening think tag; a built-in agent's message may just mention the tag. + // PR fields are JSON, so they strip only when parsing fails instead. + if ( + !result.success || + !isCustomAgentId(input.params.agentId) || + input.operation === 'pull-request-fields' || + input.operation === 'jira-issue-summary' + ) { + return result + } + const answer = stripPrefilledReasoningPreamble(result.rawOutput) + if (answer === result.rawOutput) { + return result + } + const rawOutput = cleanGeneratedCommitMessage(answer) + return rawOutput + ? { ...result, rawOutput } + : { success: false, error: `${plan.label} returned an empty ${input.emptyResultName}.` } +} diff --git a/src/main/text-generation/commit-message-model-discovery-policy.ts b/src/main/text-generation/commit-message-model-discovery-policy.ts index 41bef79fb18..00c6485da63 100644 --- a/src/main/text-generation/commit-message-model-discovery-policy.ts +++ b/src/main/text-generation/commit-message-model-discovery-policy.ts @@ -1,3 +1,4 @@ +import { buildClaudeModelListArgs } from '../../shared/claude-model-list-probe' import type { CommandTemplateBackslash } from '../../shared/commit-message-prompt' import type { CommitMessagePlan } from '../../shared/commit-message-plan' import { planAgentBinary } from '../../shared/commit-message-plan' @@ -75,7 +76,8 @@ export function finalizeModelDiscoveryOutput( export function planModelDiscovery( spec: AgentModelProbeSpec, agentCommandOverride?: string, - backslash: CommandTemplateBackslash = 'escape' + backslash: CommandTemplateBackslash = 'escape', + strictMcp = true ): { ok: true; plan: CommitMessagePlan } | { ok: false; error: string } { const modelDiscovery = spec.modelDiscovery if (!modelDiscovery) { @@ -85,11 +87,15 @@ export function planModelDiscovery( if (!command.ok) { return command } + const args = + spec.id === 'claude' + ? buildClaudeModelListArgs(strictMcp, command.prefixArgs) + : modelDiscovery.args return { ok: true, plan: { binary: command.binary, - args: [...command.prefixArgs, ...modelDiscovery.args], + args: [...command.prefixArgs, ...args], stdinPayload: modelDiscovery.stdinPayload ?? null, label: spec.label, ...(command.env ? { env: command.env } : {}) diff --git a/src/main/text-generation/commit-message-model-discovery.ts b/src/main/text-generation/commit-message-model-discovery.ts index c327541c106..22f1d369191 100644 --- a/src/main/text-generation/commit-message-model-discovery.ts +++ b/src/main/text-generation/commit-message-model-discovery.ts @@ -1,7 +1,15 @@ import { isMissingProviderExecutable } from '../provider-process/provider-executable-missing' import { mergeCommandEnvironment } from '../../shared/command-environment' import type { CommandTemplateBackslash } from '../../shared/commit-message-prompt' -import { getAgentModelProbeSpec } from '../../shared/agent-model-probe-spec' +import { + getAgentModelProbeSpec, + type AgentModelProbeSpec +} from '../../shared/agent-model-probe-spec' +import type { CommitMessagePlan } from '../../shared/commit-message-plan' +import { + runClaudeBackgroundAttempts, + type ClaudeBackgroundFailedExit +} from '../../shared/claude-background-run-isolation' import type { TuiAgent } from '../../shared/tui-agent' import { resolveCodexHomeProcessLockKeyForSpawnEnv } from '../codex-cli/codex-home-process-lock' import { supervisedProviderSpawnFailure } from '../provider-process/provider-spawn-failure-report' @@ -28,7 +36,7 @@ export type CommitMessageModelDiscoveryLocalOptions = { wslDistro?: string } -export async function discoverModelsLocal(input: { +type DiscoverModelsLocalInput = { agentId: TuiAgent env: NodeJS.ProcessEnv | undefined agentCommandOverride?: string @@ -37,7 +45,11 @@ export async function discoverModelsLocal(input: { spawnAgent: SpawnSourceControlAgent /** The command actually spawned, when the caller replaces the plan's; a missing one is typed. */ binary?: string -}): Promise { +} + +export async function discoverModelsLocal( + input: DiscoverModelsLocalInput +): Promise { const spec = getAgentModelProbeSpec(input.agentId) if (!spec) { return { success: false, error: `Agent "${input.agentId}" does not support model discovery.` } @@ -55,8 +67,30 @@ export async function discoverModelsLocal(input: { planned.plan.env, input.options.wslDistro ? 'linux' : process.platform ) + const attempt = await runClaudeBackgroundAttempts({ + agentId: input.agentId, + run: (strictMcp): ReturnType => { + const rebuilt = strictMcp + ? planned + : planModelDiscovery(spec, input.agentCommandOverride, input.backslash, strictMcp) + return rebuilt.ok + ? discoverModelsOnce(input, spec, rebuilt.plan, env) + : Promise.resolve({ result: { success: false as const, error: rebuilt.error } }) + }, + failedExit: (outcome) => (outcome.result.success ? undefined : outcome.failedExit) + }) + return attempt.result +} - const binary = input.binary ?? planned.plan.binary +async function discoverModelsOnce( + input: DiscoverModelsLocalInput, + spec: AgentModelProbeSpec, + plan: CommitMessagePlan, + env: NodeJS.ProcessEnv | undefined +): Promise<{ result: DiscoverCommitMessageModelsResult; failedExit?: ClaudeBackgroundFailedExit }> { + let failedExit: ClaudeBackgroundFailedExit | undefined + + const binary = input.binary ?? plan.binary const couldNotStart = `${spec.label} model discovery could not be started. Check the agent CLI configuration and try again.` const startFailure = (error: unknown): DiscoverCommitMessageModelsResult => ({ success: false, @@ -75,17 +109,17 @@ export async function discoverModelsLocal(input: { try { child = input.spawnAgent({ binary, - args: planned.plan.args, + args: plan.args, cwd: input.options.cwd, env: input.options.wslDistro ? input.env : env, - commandEnv: planned.plan.env, + commandEnv: plan.env, wslDistro: input.options.wslDistro, - stdinMode: planned.plan.stdinPayload === null ? 'ignore' : 'pipe', + stdinMode: plan.stdinPayload === null ? 'ignore' : 'pipe', useCwdForNative: false }) - if (planned.plan.stdinPayload !== null) { + if (plan.stdinPayload !== null) { child.stdin?.on?.('error', () => {}) - child.stdin?.end(planned.plan.stdinPayload) + child.stdin?.end(plan.stdinPayload) } } catch (error) { markProcessClosed() @@ -174,6 +208,9 @@ export async function discoverModelsLocal(input: { onError(spawnFailure.error) return } + if (!settled && !outputLimitExceeded) { + failedExit = { exitCode: code, stdout, stderr } + } finish( outputLimitExceeded ? { success: false, error: `${spec.label} returned too much model data.` } @@ -197,10 +234,11 @@ export async function discoverModelsLocal(input: { }) return { result, processClosed } } - return input.agentId === 'codex' + const result = await (input.agentId === 'codex' ? runCodexProcessWithHomeLock( - resolveCodexHomeProcessLockKeyForSpawnEnv(env, input.options.wslDistro, planned.plan.env), + resolveCodexHomeProcessLockKeyForSpawnEnv(env, input.options.wslDistro, plan.env), startDiscovery ) - : startDiscovery().result + : startDiscovery().result) + return { result, failedExit } } diff --git a/src/main/text-generation/commit-message-text-generation-model-discovery.test.ts b/src/main/text-generation/commit-message-text-generation-model-discovery.test.ts index 968237b7b1b..861d7245430 100644 --- a/src/main/text-generation/commit-message-text-generation-model-discovery.test.ts +++ b/src/main/text-generation/commit-message-text-generation-model-discovery.test.ts @@ -175,7 +175,10 @@ describe('discoverCommitMessageModelsLocal on the Windows direct-child path', () 'stream-json', '--output-format', 'stream-json', - '--verbose' + '--verbose', + '--strict-mcp-config', + '--settings', + '{"disableAllHooks":true}' ]) expect(spawnMock.mock.calls[0]![2]).toMatchObject({ windowsHide: true, diff --git a/src/main/text-generation/conversation-name-generation-request.ts b/src/main/text-generation/conversation-name-generation-request.ts index c7ac529efd5..e9d28c6208a 100644 --- a/src/main/text-generation/conversation-name-generation-request.ts +++ b/src/main/text-generation/conversation-name-generation-request.ts @@ -4,14 +4,10 @@ import { sanitizeGeneratedConversationName, type ConversationNameContext } from '../../shared/conversation-name-generation' -import { planCommitMessageGeneration } from '../../shared/commit-message-plan' import type { ResolvedSourceControlAiGenerationParams } from '../../shared/source-control-ai' import { renderSourceControlActionCommandTemplate } from '../../shared/source-control-ai-actions' import { captureAgentGenerationFailureOutput } from './agent-failure-output' -import { - commandBackslashMode, - executeGenerationPlan -} from './source-control-text-generation-requests' +import { runGenerationRequest } from './agent-generation-request' import type { CommitMessageGenerationTarget, GenerateConversationNameResult, @@ -24,7 +20,7 @@ export async function generateConversationName(input: { target: CommitMessageGenerationTarget spawnAgent: SpawnSourceControlAgent }): Promise { - const { context, params, target } = input + const { context, params } = input const firstPrompt = clampConversationNameFirstPrompt(context.firstPrompt) const basePrompt = buildConversationNamePrompt({ firstPrompt }) const prompt = @@ -34,17 +30,10 @@ export async function generateConversationName(input: { firstPrompt }) : buildConversationNamePrompt({ firstPrompt }, params.customPrompt ?? '') - const planned = planCommitMessageGeneration( - { ...params, backslash: commandBackslashMode(target) }, - prompt - ) - if (!planned.ok) { - return { success: false, error: planned.error } - } - const result = await executeGenerationPlan({ + const result = await runGenerationRequest({ ...input, - plan: planned.plan, + prompt, emptyResultName: 'chat name', operation: 'conversation-name' }) @@ -59,7 +48,11 @@ export async function generateConversationName(input: { success: false, error: 'Generated chat name was empty after sanitization.', failureOutput: - captureAgentGenerationFailureOutput(planned.plan.label, 0, result.rawOutput, '') ?? - undefined + captureAgentGenerationFailureOutput( + result.agentLabel ?? params.agentId, + 0, + result.rawOutput, + '' + ) ?? undefined } } diff --git a/src/main/text-generation/jira-issue-summary-text-generation.test.ts b/src/main/text-generation/jira-issue-summary-text-generation.test.ts index 83cc6fdf12e..d78acc153cf 100644 --- a/src/main/text-generation/jira-issue-summary-text-generation.test.ts +++ b/src/main/text-generation/jira-issue-summary-text-generation.test.ts @@ -1,4 +1,14 @@ import { describe, expect, it } from 'vitest' +import { ChildProcess } from 'node:child_process' +import { PassThrough } from 'node:stream' +import { generateCommitMessage } from './source-control-text-generation-requests' +import type { + CommitMessageGenerationTarget, + SpawnSourceControlAgent +} from './source-control-text-generation-types' +import { getAgentModelProbeSpec } from '../../shared/agent-model-probe-spec' +import { planModelDiscovery } from './commit-message-model-discovery-policy' +import { planCommitMessageGeneration } from '../../shared/commit-message-plan' import type { CommitMessagePlan } from '../../shared/commit-message-plan' import { generateJiraIssueSummaryFromContext } from './jira-issue-summary-text-generation' @@ -95,3 +105,167 @@ describe('generateJiraIssueSummaryFromContext', () => { }) }) }) + +// One owner must cover Jira and ordinary generation through both existing transports. +it.each(['local', 'remote'] as const)( + 'retries the same Claude invocation for Jira and commits on %s', + async (kind) => { + for (const operation of ['jira-issue-summary', 'commit-message'] as const) { + const calls: { args: string[]; other: unknown }[] = [] + const response = (args: string[]) => { + const refused = args.includes('--strict-mcp-config') + return { + stdout: refused ? '' : 'Fix crash', + stderr: refused + ? 'You cannot use --strict-mcp-config when an enterprise MCP config is present' + : '', + exitCode: refused ? 1 : 0, + timedOut: false + } + } + const spawnAgent: SpawnSourceControlAgent = ({ args, ...other }) => { + calls.push({ args, other }) + const child = new (class extends ChildProcess { + override stdin = new PassThrough() + override stdout = new PassThrough() + override stderr = new PassThrough() + })() + queueMicrotask(() => { + const output = response(args) + child.stdout.emit('data', Buffer.from(output.stdout)) + child.stderr.emit('data', Buffer.from(output.stderr)) + child.emit('close', output.exitCode) + }) + return child + } + const target: CommitMessageGenerationTarget = + kind === 'local' + ? { kind, cwd: '/repo' } + : { + kind, + cwd: '/repo', + missingBinaryLocation: 'remote PATH', + execute: async ({ args, ...other }, cwd, timeout, op) => { + expect(op).toBe(operation) + calls.push({ args, other: { ...other, cwd, timeout, op } }) + return response(args) + } + } + const params = { agentId: 'claude' as const, model: 'haiku' } + const result = + operation === 'jira-issue-summary' + ? await generateJiraIssueSummaryFromContext( + { description: 'Crash on save' }, + params, + target, + spawnAgent + ) + : await generateCommitMessage({ + context: { branch: 'main', stagedSummary: 'M file', stagedPatch: '+fix' }, + params, + target, + spawnAgent + }) + expect(result.success).toBe(true) + expect(calls).toHaveLength(2) + expect(calls[1]).toEqual({ + ...calls[0], + args: calls[0].args.filter((arg) => arg !== '--strict-mcp-config') + }) + } + } +) + +it('preserves Jira custom output and OpenCode one-attempt behavior', async () => { + expect( + await generateJiraIssueSummaryFromContext( + { description: 'Crash' }, + { agentId: 'custom', model: '', customAgentCommand: 'agent' }, + remoteTarget(async () => 'Draft thought\nFix crash') + ) + ).toMatchObject({ summary: 'Draft thought' }) + let attempts = 0 + const result = await generateJiraIssueSummaryFromContext( + { description: 'Crash' }, + { agentId: 'opencode', model: 'provider/model', thinkingLevel: 'high' }, + { + kind: 'remote', + cwd: '/repo', + missingBinaryLocation: 'remote PATH', + execute: async () => { + attempts++ + return { stdout: '', stderr: 'Unknown argument: variant', exitCode: 1, timedOut: false } + } + } + ) + expect(result.success).toBe(false) + expect(attempts).toBe(1) +}) + +it.each(['\'{"model":"haiku"}\'', 'provider-settings.json'])( + 'uses existing settings override for generation and discovery: %s', + (settings) => { + const override = `claude --settings ${settings}` + const spec = getAgentModelProbeSpec('claude') + if (!spec) { + throw new Error('Missing Claude spec') + } + const requests = [ + planModelDiscovery(spec, override), + planCommitMessageGeneration( + { agentId: 'claude', model: 'haiku', agentCommandOverride: override }, + 'prompt' + ), + planCommitMessageGeneration( + { agentId: 'claude', model: 'haiku', agentArgs: `--settings ${settings}` }, + 'prompt' + ) + ] + for (const planned of requests) { + if (!planned.ok) { + throw new Error(planned.error) + } + expect(planned.plan.args.filter((arg) => arg === '--settings')).toHaveLength(1) + expect(planned.plan.args[planned.plan.args.indexOf('--settings') + 1]).toBe( + settings.replaceAll("'", '') + ) + expect(planned.plan.args).not.toContain('{"disableAllHooks":true}') + } + } +) + +it('rebuilds without inspecting prompt-like, optional or user strict tokens', async () => { + for (const agentArgs of [ + "--append-subagent-system-prompt '--strict-mcp-config'", + '--resume --strict-mcp-config saved-session', + '--mcp-config custom.json --strict-mcp-config' + ]) { + const calls: CommitMessagePlan[] = [] + const params = { agentId: 'claude' as const, model: 'haiku', agentArgs } + const result = await generateJiraIssueSummaryFromContext({ description: 'Crash' }, params, { + kind: 'remote', + cwd: '/repo', + missingBinaryLocation: 'remote PATH', + execute: async (plan) => { + calls.push(plan) + return { + stdout: '', + stderr: 'You cannot use --strict-mcp-config when an enterprise MCP config is present', + exitCode: 1, + timedOut: false + } + } + }) + expect(result.success).toBe(false) + expect(calls).toHaveLength(2) + const first = planCommitMessageGeneration(params, calls[0].stdinPayload ?? '') + const second = planCommitMessageGeneration(params, calls[0].stdinPayload ?? '', { + strictMcp: false + }) + if (!first.ok || !second.ok) { + throw new Error('Invalid fixture') + } + expect(calls).toEqual([first.plan, second.plan]) + expect(calls[1].args).toContain('--strict-mcp-config') + } +}) diff --git a/src/main/text-generation/jira-issue-summary-text-generation.ts b/src/main/text-generation/jira-issue-summary-text-generation.ts index 27a0978e2c8..4326bc50084 100644 --- a/src/main/text-generation/jira-issue-summary-text-generation.ts +++ b/src/main/text-generation/jira-issue-summary-text-generation.ts @@ -1,4 +1,3 @@ -import { planCommitMessageGeneration } from '../../shared/commit-message-plan' import { buildJiraIssueSummaryPrompt, sanitizeGeneratedJiraIssueSummary, @@ -8,9 +7,7 @@ import { import type { ResolvedSourceControlAiGenerationParams } from '../../shared/source-control-ai' import { spawnSourceControlAgent } from './source-control-agent-launch' import { cancelLocalGeneration } from './source-control-generation-lanes' -import { runLocalPlanForAgent } from './source-control-local-generation' -import { runRemoteSourceControlPlan } from './source-control-remote-generation' -import { commandBackslashMode } from './source-control-text-generation-requests' +import { runGenerationRequest } from './agent-generation-request' import type { CommitMessageGenerationTarget, SpawnSourceControlAgent @@ -24,29 +21,14 @@ export async function generateJiraIssueSummaryFromContext( spawnAgent: SpawnSourceControlAgent = spawnSourceControlAgent ): Promise { const prompt = buildJiraIssueSummaryPrompt(context) - const planned = planCommitMessageGeneration( - { ...params, backslash: commandBackslashMode(target) }, - prompt - ) - if (!planned.ok) { - return { success: false, error: planned.error } - } - const result = - target.kind === 'remote' - ? await runRemoteSourceControlPlan({ - plan: planned.plan, - target, - emptyResultName: 'summary', - operation: 'jira-issue-summary' - }) - : await runLocalPlanForAgent({ - agentId: params.agentId, - plan: planned.plan, - target, - emptyResultName: 'summary', - operation: 'jira-issue-summary', - spawnAgent - }) + const result = await runGenerationRequest({ + params, + prompt, + target, + spawnAgent, + emptyResultName: 'summary', + operation: 'jira-issue-summary' + }) if (!result.success) { return { success: false, error: result.error, canceled: result.canceled } } diff --git a/src/main/text-generation/source-control-text-generation-requests.ts b/src/main/text-generation/source-control-text-generation-requests.ts index 367e67c76fa..f94d344dc0d 100644 --- a/src/main/text-generation/source-control-text-generation-requests.ts +++ b/src/main/text-generation/source-control-text-generation-requests.ts @@ -16,29 +16,21 @@ import { } from '../../shared/branch-name-from-work' import { cleanGeneratedCommitMessage, - stripPrefilledReasoningPreamble, - type CommandTemplateBackslash + stripPrefilledReasoningPreamble } from '../../shared/commit-message-prompt' import { isCustomAgentId } from '../../shared/commit-message-agent-spec' -import { - planCommitMessageGeneration, - type CommitMessagePlan -} from '../../shared/commit-message-plan' import type { ResolvedSourceControlAiGenerationParams } from '../../shared/source-control-ai' import { formatLinkedIssueTemplateValue } from '../../shared/source-control-ai-action-variables' import { renderSourceControlActionCommandTemplate } from '../../shared/source-control-ai-actions' import { captureAgentGenerationFailureOutput } from './agent-failure-output' -import { openCodeVariantRetryPlan } from '../../shared/opencode-generation-command' -import { runLocalPlanForAgent } from './source-control-local-generation' -import { runRemoteSourceControlPlan } from './source-control-remote-generation' +import { runGenerationRequest } from './agent-generation-request' +export { commandBackslashMode } from './agent-generation-request' import type { CommitMessageGenerationTarget, GenerateBranchNameResult, GenerateCommitMessageResult, GeneratePullRequestFieldsResult, - InternalTextGenerationResult, - SpawnSourceControlAgent, - TextGenerationOperation + SpawnSourceControlAgent } from './source-control-text-generation-types' type GenerateParams = ResolvedSourceControlAiGenerationParams @@ -47,71 +39,13 @@ export function trimGeneratedCommitMessage(message: string): string { return message.replace(/\s+$/, '') } -export function commandBackslashMode( - target: CommitMessageGenerationTarget, - platform: NodeJS.Platform = process.platform -): CommandTemplateBackslash { - return platform === 'win32' && target.kind === 'local' && !target.wslDistro ? 'literal' : 'escape' -} - -export async function executeGenerationPlan(input: { - params: GenerateParams - plan: CommitMessagePlan - target: CommitMessageGenerationTarget - emptyResultName: string - operation: TextGenerationOperation - spawnAgent: SpawnSourceControlAgent -}): Promise { - const execute = (plan: CommitMessagePlan): Promise => - input.target.kind === 'remote' - ? runRemoteSourceControlPlan({ - plan, - target: input.target, - emptyResultName: input.emptyResultName, - operation: input.operation - }) - : runLocalPlanForAgent({ - agentId: input.params.agentId, - plan, - target: input.target, - emptyResultName: input.emptyResultName, - operation: input.operation, - spawnAgent: input.spawnAgent - }) - let result = await execute(input.plan) - if (!result.success && input.params.agentId === 'opencode') { - const retry = openCodeVariantRetryPlan(input.plan, result.failureOutput?.stderr ?? '') - if (retry) { - result = await execute(retry) - } - } - // Why: only a custom command runs a raw model whose chat template can swallow - // the opening think tag; a built-in agent's message may just mention the tag. - // PR fields are JSON, so they strip only when parsing fails instead. - if ( - !result.success || - !isCustomAgentId(input.params.agentId) || - input.operation === 'pull-request-fields' - ) { - return result - } - const answer = stripPrefilledReasoningPreamble(result.rawOutput) - if (answer === result.rawOutput) { - return result - } - const rawOutput = cleanGeneratedCommitMessage(answer) - return rawOutput - ? { ...result, rawOutput } - : { success: false, error: `${input.plan.label} returned an empty ${input.emptyResultName}.` } -} - export async function generateCommitMessage(input: { context: CommitMessageDraftContext params: GenerateParams target: CommitMessageGenerationTarget spawnAgent: SpawnSourceControlAgent }): Promise { - const { context, params, target } = input + const { context, params } = input const basePrompt = buildCommitMessagePrompt(context, '') const prompt = params.commandInputTemplate !== undefined @@ -123,16 +57,9 @@ export async function generateCommitMessage(input: { linkedIssue: formatLinkedIssueTemplateValue(context.linkedIssue) }) : buildCommitMessagePrompt(context, params.customPrompt ?? '') - const planned = planCommitMessageGeneration( - { ...params, backslash: commandBackslashMode(target) }, - prompt - ) - if (!planned.ok) { - return { success: false, error: planned.error } - } - const result = await executeGenerationPlan({ + const result = await runGenerationRequest({ ...input, - plan: planned.plan, + prompt, emptyResultName: 'message', operation: 'commit-message' }) @@ -156,7 +83,7 @@ export async function generatePullRequestFields(input: { target: CommitMessageGenerationTarget spawnAgent: SpawnSourceControlAgent }): Promise> { - const { context, params, target } = input + const { context, params } = input const basePrompt = buildPullRequestFieldsPrompt(context, '') const prompt = params.commandInputTemplate !== undefined @@ -172,20 +99,9 @@ export async function generatePullRequestFields(input: { linkedIssue: formatLinkedIssueTemplateValue(context.linkedIssue) }) : buildPullRequestFieldsPrompt(context, params.customPrompt ?? '') - const planned = planCommitMessageGeneration( - { ...params, backslash: commandBackslashMode(target) }, - prompt - ) - if (!planned.ok) { - return { - success: false, - error: planned.error, - branchChangedByPreparation: context.branchChangedByPreparation - } - } - const result = await executeGenerationPlan({ + const result = await runGenerationRequest({ ...input, - plan: planned.plan, + prompt, emptyResultName: 'details', operation: 'pull-request-fields' }) @@ -237,7 +153,7 @@ export async function generateBranchName(input: { target: CommitMessageGenerationTarget spawnAgent: SpawnSourceControlAgent }): Promise { - const { context, params, target } = input + const { context, params } = input const basePrompt = buildBranchNamePrompt(context) const prompt = params.commandInputTemplate !== undefined @@ -247,16 +163,9 @@ export async function generateBranchName(input: { assistantMessage: context.assistantMessage ?? '' }) : buildBranchNamePrompt(context, params.customPrompt ?? '') - const planned = planCommitMessageGeneration( - { ...params, backslash: commandBackslashMode(target) }, - prompt - ) - if (!planned.ok) { - return { success: false, error: planned.error } - } - const result = await executeGenerationPlan({ + const result = await runGenerationRequest({ ...input, - plan: planned.plan, + prompt, emptyResultName: 'branch name', operation: 'branch-name' }) @@ -270,7 +179,11 @@ export async function generateBranchName(input: { success: false, error: 'Generated branch name was empty after sanitization.', failureOutput: - captureAgentGenerationFailureOutput(planned.plan.label, 0, result.rawOutput, '') ?? - undefined + captureAgentGenerationFailureOutput( + result.agentLabel ?? params.agentId, + 0, + result.rawOutput, + '' + ) ?? undefined } } diff --git a/src/shared/claude-background-run-isolation.test.ts b/src/shared/claude-background-run-isolation.test.ts new file mode 100644 index 00000000000..9b9da9ebc08 --- /dev/null +++ b/src/shared/claude-background-run-isolation.test.ts @@ -0,0 +1,25 @@ +import { expect, it, vi } from 'vitest' +import { runClaudeBackgroundAttempts } from './claude-background-run-isolation' + +it('rebuilds only after a canonical nonzero completed exit, at most twice', async () => { + const canonical = 'You cannot use --strict-mcp-config when an enterprise MCP config is present' + for (const [agentId, exitCode, stderr, eligible] of [ + ['claude', 1, canonical, true], + [ + 'claude', + 1, + 'Failed reading enterprise MCP config; --strict-mcp-config was requested.', + false + ], + ['claude', 0, canonical, false], + ['claude', null, canonical, false], + ['codex', 1, canonical, false] + ] as const) { + const result = { exitCode, stdout: '', stderr } + const run = vi.fn(async (_strictMcp: boolean) => result) + expect( + await runClaudeBackgroundAttempts({ agentId, run, failedExit: (outcome) => outcome }) + ).toBe(result) + expect(run.mock.calls).toEqual(eligible ? [[true], [false]] : [[true]]) + } +}) diff --git a/src/shared/claude-background-run-isolation.ts b/src/shared/claude-background-run-isolation.ts new file mode 100644 index 00000000000..4f0bf6dbfe7 --- /dev/null +++ b/src/shared/claude-background-run-isolation.ts @@ -0,0 +1,34 @@ +const ENTERPRISE_REFUSAL = + 'You cannot use --strict-mcp-config when an enterprise MCP config is present' + +/** Background policy is generated separately from user arguments. */ +export function claudeBackgroundArgs(strictMcp = true, userArgs: readonly string[] = []): string[] { + // A literal option value also preserves existing hooks; never interpret user argument grammar. + const hasSettings = userArgs.some((arg) => arg === '--settings' || arg.startsWith('--settings=')) + return [ + ...(strictMcp ? ['--strict-mcp-config'] : []), + ...(hasSettings ? [] : ['--settings', '{"disableAllHooks":true}']) + ] +} + +export type ClaudeBackgroundFailedExit = { exitCode: number | null; stdout: string; stderr: string } + +/** Rebuild once without generated strict mode, only after Claude's completed enterprise-policy refusal. */ +export async function runClaudeBackgroundAttempts(input: { + agentId: string + run: (strictMcp: boolean) => Promise + failedExit: (result: T) => ClaudeBackgroundFailedExit | undefined +}): Promise { + const result = await input.run(true) + const failure = input.failedExit(result) + if ( + input.agentId !== 'claude' || + !failure || + typeof failure.exitCode !== 'number' || + failure.exitCode === 0 || + !`${failure.stdout}\n${failure.stderr}`.includes(ENTERPRISE_REFUSAL) + ) { + return result + } + return input.run(false) +} diff --git a/src/shared/claude-model-list-probe.ts b/src/shared/claude-model-list-probe.ts index 113d69aa859..d6d2307ef21 100644 --- a/src/shared/claude-model-list-probe.ts +++ b/src/shared/claude-model-list-probe.ts @@ -1,3 +1,4 @@ +import { claudeBackgroundArgs } from './claude-background-run-isolation' import { assertJsonTextStructureWithinLimits } from './json-text-structure-limit' // Why: the Claude CLI has no model-listing subcommand (`claude models` starts a @@ -12,14 +13,22 @@ export const CLAUDE_MODEL_LIST_STDIN = `${JSON.stringify({ })}\n` // Why: --print rejects stream-json output unless --verbose is also set. -export const CLAUDE_MODEL_LIST_ARGS = [ - '-p', - '--input-format', - 'stream-json', - '--output-format', - 'stream-json', - '--verbose' -] +export function buildClaudeModelListArgs( + strictMcp = true, + userArgs: readonly string[] = [] +): string[] { + return [ + '-p', + '--input-format', + 'stream-json', + '--output-format', + 'stream-json', + '--verbose', + ...claudeBackgroundArgs(strictMcp, userArgs) + ] +} + +export const CLAUDE_MODEL_LIST_ARGS = buildClaudeModelListArgs() export type ClaudeListedModel = { /** Value the CLI accepts for `--model` and `/model` (e.g. `opus[1m]`). */ diff --git a/src/shared/commit-message-agent-spec.test.ts b/src/shared/commit-message-agent-spec.test.ts index 18635420b4c..b5880aab3aa 100644 --- a/src/shared/commit-message-agent-spec.test.ts +++ b/src/shared/commit-message-agent-spec.test.ts @@ -1,3 +1,4 @@ +import { claudeBackgroundArgs } from './claude-background-run-isolation' import { afterEach, describe, expect, it, vi } from 'vitest' import { COMMIT_MESSAGE_AGENT_SPECS, @@ -227,7 +228,8 @@ describe('buildArgs (Claude)', () => { '--model', 'haiku', '--permission-mode', - 'plan' + 'plan', + ...claudeBackgroundArgs() ]) }) @@ -246,7 +248,8 @@ describe('buildArgs (Claude)', () => { '--permission-mode', 'plan', '--effort', - 'high' + 'high', + ...claudeBackgroundArgs() ]) }) @@ -395,7 +398,8 @@ describe('model discovery parsers', () => { 'stream-json', '--output-format', 'stream-json', - '--verbose' + '--verbose', + ...claudeBackgroundArgs() ]) const payload = JSON.parse(discovery?.stdinPayload ?? '') as { type?: string diff --git a/src/shared/commit-message-agent-spec.ts b/src/shared/commit-message-agent-spec.ts index 35f5b4c119c..d8fe4023b43 100644 --- a/src/shared/commit-message-agent-spec.ts +++ b/src/shared/commit-message-agent-spec.ts @@ -54,7 +54,13 @@ export type CommitMessageAgentSpec = { binary: string /** Where the prompt is delivered. Large diffs go via stdin to avoid argv limits. */ promptDelivery: 'argv' | 'stdin' - buildArgs: (params: { prompt: string; model: string; thinkingLevel?: string }) => string[] + buildArgs: (params: { + prompt: string + model: string + thinkingLevel?: string + strictMcp?: boolean + userArgs?: readonly string[] + }) => string[] /** Alias groups the CLI accepts at most once. Recipe CLI arguments repeating one * replace the generated flag instead of being appended: yargs-based CLIs collapse * a repeated flag into an array and crash. Defaults to the model flag alone. */ diff --git a/src/shared/commit-message-agent-specs-primary.ts b/src/shared/commit-message-agent-specs-primary.ts index 7a2aa70c979..c8b888260b3 100644 --- a/src/shared/commit-message-agent-specs-primary.ts +++ b/src/shared/commit-message-agent-specs-primary.ts @@ -1,3 +1,4 @@ +import { claudeBackgroundArgs } from './claude-background-run-isolation' import type { TuiAgent } from './tui-agent' import type { CommitMessageAgentSpec, @@ -35,7 +36,7 @@ export function buildPrimaryCommitMessageAgentSpecs({ // Why: diffs can be large and `claude -p` reads from stdin natively when no // positional prompt is provided. promptDelivery: 'stdin', - buildArgs: ({ model, thinkingLevel }) => [ + buildArgs: ({ model, thinkingLevel, strictMcp, userArgs }) => [ '-p', '--output-format', 'text', @@ -43,8 +44,10 @@ export function buildPrimaryCommitMessageAgentSpecs({ model, '--permission-mode', 'plan', - ...(thinkingLevel ? ['--effort', thinkingLevel] : []) + ...(thinkingLevel ? ['--effort', thinkingLevel] : []), + ...claudeBackgroundArgs(strictMcp, userArgs) ], + singletonOptions: [['--model']], modelSource: 'dynamic', // Why: the Claude CLI has no listing subcommand; one list_models control // request over --print stream-json returns the /model picker catalog. diff --git a/src/shared/commit-message-plan.test.ts b/src/shared/commit-message-plan.test.ts index 3fcdfc5cbf6..de20fcc9603 100644 --- a/src/shared/commit-message-plan.test.ts +++ b/src/shared/commit-message-plan.test.ts @@ -48,7 +48,10 @@ describe('planCommitMessageGeneration', () => { '--permission-mode', 'plan', '--effort', - 'high' + 'high', + '--strict-mcp-config', + '--settings', + '{"disableAllHooks":true}' ], stdinPayload: 'PROMPT', label: 'Claude' @@ -56,6 +59,19 @@ describe('planCommitMessageGeneration', () => { }) }) + it('preserves a literal --settings prompt value without generating hooks settings', () => { + const result = planCommitMessageGeneration( + { agentId: 'claude', model: 'sonnet', agentArgs: '--append-system-prompt "--settings"' }, + 'PROMPT' + ) + expect(result.ok && result.plan.args.slice(-3)).toEqual([ + '--strict-mcp-config', + '--append-system-prompt', + '--settings' + ]) + expect(result.ok && result.plan.args).not.toContain('{"disableAllHooks":true}') + }) + it('plans OpenCode run with prompt on stdin and model variant', () => { const result = planCommitMessageGeneration( { diff --git a/src/shared/commit-message-plan.ts b/src/shared/commit-message-plan.ts index 8fe541c6028..f758474a686 100644 --- a/src/shared/commit-message-plan.ts +++ b/src/shared/commit-message-plan.ts @@ -189,7 +189,8 @@ function insertAdditionalAgentArgs(args: { export function planCommitMessageGeneration( input: CommitMessagePlanInput, - prompt: string + prompt: string, + options: { strictMcp?: boolean } = {} ): CommitMessagePlanResult { if (isCustomAgentId(input.agentId)) { const command = input.customAgentCommand?.trim() ?? '' @@ -250,11 +251,6 @@ export function planCommitMessageGeneration( } const argvPrompt = spec.promptDelivery === 'argv' ? prompt : '' - const baseArgs = spec.buildArgs({ - prompt: argvPrompt, - model: input.model, - thinkingLevel: input.thinkingLevel - }) const agentArgs = planAdditionalAgentArgs(input.agentArgs, input.backslash) if (!agentArgs.ok) { return agentArgs @@ -263,6 +259,13 @@ export function planCommitMessageGeneration( if (!command.ok) { return { ok: false, error: command.error } } + const baseArgs = spec.buildArgs({ + prompt: argvPrompt, + model: input.model, + thinkingLevel: input.thinkingLevel, + strictMcp: options.strictMcp, + userArgs: [...command.prefixArgs, ...agentArgs.args] + }) // Why: repeating a singleton flag makes yargs-based CLIs parse it as an array and // crash (OpenCode's `model.split('/')`). User values replace Orca's, never stack. const merged = applySingletonOptionOverrides({