mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
Background Claude runs no longer start your MCP servers and hooks (#30939)
* fix(claude): keep Orca's background Claude runs off the user's MCP servers and hooks The model picker and / menu listings, chat naming, and commit message, PR, branch and Jira text generation each start a short Claude run. Every one but the / probe loaded the user's MCP servers and hooks, so each run started every server (one cut off a Notion OAuth refresh and lost the sign-in) and fired SessionStart hooks. Every background run now passes --strict-mcp-config and disableAllHooks. A user's own inline --settings merges into the one settings value Claude keeps, and their --mcp-config still loads. When Claude refuses strict mode because an enterprise MCP config is present (which already keeps user servers out), the run is retried once without the flag. Tracked in STA-11095 and #30411. * fix: centralize background Claude generation and retry policy * fix: rebuild Claude background retries from generated arguments * test: include branch in generation retry fixture * fix: preserve user settings arguments in background generation * test: define retry fixture streams across runtimes
This commit is contained in:
@@ -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)
|
||||
})
|
||||
@@ -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'
|
||||
})
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
|
||||
@@ -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<AgentModelCatalogSuccess> => {
|
||||
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 } : {})
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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<string>((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() }
|
||||
)
|
||||
}
|
||||
})
|
||||
|
||||
@@ -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<InternalTextGenerationResult> {
|
||||
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<InternalTextGenerationResult> =>
|
||||
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<InternalTextGenerationResult> => {
|
||||
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}.` }
|
||||
}
|
||||
@@ -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 } : {})
|
||||
|
||||
@@ -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<DiscoverCommitMessageModelsResult> {
|
||||
}
|
||||
|
||||
export async function discoverModelsLocal(
|
||||
input: DiscoverModelsLocalInput
|
||||
): Promise<DiscoverCommitMessageModelsResult> {
|
||||
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<typeof discoverModelsOnce> => {
|
||||
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 }
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<GenerateConversationNameResult> {
|
||||
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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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</think>\nFix crash')
|
||||
)
|
||||
).toMatchObject({ summary: 'Draft thought</think>' })
|
||||
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')
|
||||
}
|
||||
})
|
||||
|
||||
@@ -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<JiraIssueSummaryGenerationResult> {
|
||||
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 }
|
||||
}
|
||||
|
||||
@@ -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<InternalTextGenerationResult> {
|
||||
const execute = (plan: CommitMessagePlan): Promise<InternalTextGenerationResult> =>
|
||||
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<GenerateCommitMessageResult> {
|
||||
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<GeneratePullRequestFieldsResult<GeneratedPullRequestFields>> {
|
||||
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<GenerateBranchNameResult> {
|
||||
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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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]])
|
||||
}
|
||||
})
|
||||
@@ -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<T>(input: {
|
||||
agentId: string
|
||||
run: (strictMcp: boolean) => Promise<T>
|
||||
failedExit: (result: T) => ClaudeBackgroundFailedExit | undefined
|
||||
}): Promise<T> {
|
||||
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)
|
||||
}
|
||||
@@ -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]`). */
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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. */
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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(
|
||||
{
|
||||
|
||||
@@ -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({
|
||||
|
||||
Reference in New Issue
Block a user