diff --git a/src/main/providers/ssh-git-noninteractive-provider.ts b/src/main/providers/ssh-git-noninteractive-provider.ts index 0550b1bba39..c0d8df0573f 100644 --- a/src/main/providers/ssh-git-noninteractive-provider.ts +++ b/src/main/providers/ssh-git-noninteractive-provider.ts @@ -29,7 +29,8 @@ export class SshGitNoninteractiveProvider extends SshGitReadProvider { stdin: plan.stdinPayload, ...(plan.env ? { env: plan.env } : {}), timeoutMs, - operation + operation, + shell: true }, undefined, operation @@ -100,6 +101,7 @@ export class SshGitNoninteractiveProvider extends SshGitReadProvider { timeoutMs: number env?: Record operation?: string + shell?: boolean }, signal?: AbortSignal, operation?: string diff --git a/src/main/providers/ssh-git-provider-commit-message.test.ts b/src/main/providers/ssh-git-provider-commit-message.test.ts index be024b643c8..70d31d6a4f2 100644 --- a/src/main/providers/ssh-git-provider-commit-message.test.ts +++ b/src/main/providers/ssh-git-provider-commit-message.test.ts @@ -179,7 +179,8 @@ describe('SshGitProvider', () => { stdin: null, env: { FLAG: 'literal $HOME' }, timeoutMs: 60_000, - operation: 'commit-message' + operation: 'commit-message', + shell: true }, { timeoutMs: 65_000 } ) @@ -228,7 +229,8 @@ describe('SshGitProvider', () => { cwd: '/home/user/repo', stdin: null, timeoutMs: 60_000, - operation: 'commit-message' + operation: 'commit-message', + shell: true }, { timeoutMs: 65_000 } ) @@ -241,7 +243,8 @@ describe('SshGitProvider', () => { cwd: '/home/user/repo', stdin: null, timeoutMs: 60_000, - operation: 'pull-request-fields' + operation: 'pull-request-fields', + shell: true }, { timeoutMs: 65_000 } ) diff --git a/src/main/text-generation/command-environment-process.test.ts b/src/main/text-generation/command-environment-process.test.ts index bdc04daca3f..3ae17c6e60d 100644 --- a/src/main/text-generation/command-environment-process.test.ts +++ b/src/main/text-generation/command-environment-process.test.ts @@ -155,6 +155,9 @@ describe('environment-prefixed commands with real child processes', () => { it('discovers models through the same override environment', async () => { await expect( discoverCommitMessageModelsLocal('opencode', target.env, override, { cwd: folder }) - ).resolves.toMatchObject({ success: true, models: [{ id: 'anthropic/claude-sonnet-4' }] }) + ).resolves.toMatchObject({ + success: true, + models: [{ id: 'default' }, { id: 'anthropic/claude-sonnet-4' }] + }) }) }) diff --git a/src/main/text-generation/commit-message-agent-environment.ts b/src/main/text-generation/commit-message-agent-environment.ts index 3c5523bc420..588a56058e1 100644 --- a/src/main/text-generation/commit-message-agent-environment.ts +++ b/src/main/text-generation/commit-message-agent-environment.ts @@ -51,7 +51,7 @@ function readInheritedOrShellEnvVar(name: string, sourceName?: string): string | function prepareShellConfigDirEnv(agentId: string): { ok: true; env?: NodeJS.ProcessEnv } | null { const configVar = - agentId === 'opencode' + agentId === 'opencode' || agentId === 'opencode2' ? 'OPENCODE_CONFIG_DIR' : agentId === 'pi' || agentId === 'omp' ? 'PI_CODING_AGENT_DIR' @@ -66,7 +66,7 @@ function prepareShellConfigDirEnv(agentId: string): { ok: true; env?: NodeJS.Pro // the Pi one (and vice versa). PI_CODING_AGENT_DIR is the binary-facing var // both kinds consume — see src/main/pi/titlebar-extension-service.ts. const sourceVar = - agentId === 'opencode' + agentId === 'opencode' || agentId === 'opencode2' ? 'ORCA_OPENCODE_SOURCE_CONFIG_DIR' : agentId === 'pi' ? 'ORCA_PI_SOURCE_AGENT_DIR' 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 fa2b8869a37..41bef79fb18 100644 --- a/src/main/text-generation/commit-message-model-discovery-policy.ts +++ b/src/main/text-generation/commit-message-model-discovery-policy.ts @@ -57,6 +57,12 @@ export function finalizeModelDiscoveryOutput( } return { success: false, error: `${spec.label} returned no available models.` } } + if (spec.id === 'opencode' || spec.id === 'opencode2') { + const configuredDefault = spec.models.find((model) => model.id === 'default') + if (configuredDefault && !models.some((model) => model.id === 'default')) { + models = [configuredDefault, ...models] + } + } // A sentinel model in the static spec (for example `default`) means the CLI // should keep its configured provider even when discovery lists concrete models. const defaultModelId = diff --git a/src/main/text-generation/opencode-generation-events.test.ts b/src/main/text-generation/opencode-generation-events.test.ts new file mode 100644 index 00000000000..894f70e679f --- /dev/null +++ b/src/main/text-generation/opencode-generation-events.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, it } from 'vitest' +import { + generateCommitMessageFromContext, + generateBranchNameFromContext +} from './commit-message-text-generation' +import { finalizeModelDiscoveryOutput } from './commit-message-model-discovery-policy' +import { getCommitMessageAgentSpec } from '../../shared/commit-message-agent-spec' + +const text = (answer: string): string => + JSON.stringify({ type: 'text', part: { id: 'answer', text: answer } }) +const context = { branch: 'main', stagedSummary: 'M\tfile.ts', stagedPatch: '+new' } +const params = { agentId: 'opencode', model: 'default' } as const + +describe('OpenCode generation event handling across remote execution', () => { + it('extracts only the answer from a JSON stream', async () => { + const result = await generateCommitMessageFromContext(context, params, { + kind: 'remote', + cwd: '/repo', + missingBinaryLocation: 'remote PATH', + execute: async () => ({ + stdout: `${JSON.stringify({ type: 'tool_use', part: { text: 'tool output' } })}\n${text('fix: output only the answer')}`, + stderr: '', + exitCode: 0, + timedOut: false + }) + }) + expect(result).toEqual({ + success: true, + message: 'fix: output only the answer', + agentLabel: 'OpenCode' + }) + }) + + it.each([0, 1])('does not accept an error event with exit code %s', async (exitCode) => { + const result = await generateCommitMessageFromContext(context, params, { + kind: 'remote', + cwd: '/repo', + missingBinaryLocation: 'remote PATH', + execute: async () => ({ + stdout: JSON.stringify({ + type: 'error', + error: { type: 'provider.no-route', message: 'Model unavailable' } + }), + stderr: '', + exitCode, + timedOut: false + }) + }) + expect(result).toMatchObject({ + success: false, + error: expect.stringContaining('Model unavailable') + }) + }) + + it('retries the v2 flag rejection on the same execution host', async () => { + const argv: string[][] = [] + const result = await generateBranchNameFromContext( + { firstPrompt: 'Fix generation' }, + { ...params, model: 'fixture/chat', thinkingLevel: 'high' }, + { + kind: 'remote', + cwd: '/repo', + missingBinaryLocation: 'remote PATH', + execute: async (plan) => { + argv.push(plan.args) + return argv.length === 1 + ? { + stdout: 'Help', + stderr: 'ERROR\nUnrecognized flag: --variant in command opencode run', + exitCode: 1, + timedOut: false + } + : { stdout: text('fix-generation'), stderr: '', exitCode: 0, timedOut: false } + } + } + ) + expect(result).toMatchObject({ success: true, slug: 'fix-generation' }) + expect(argv[1]).toContain('fixture/chat#high') + expect(argv[1]).not.toContain('--variant') + }) + + it.each(['opencode', 'opencode2'] as const)( + 'retains Config default when %s discovery lists explicit models', + (agent) => { + const spec = getCommitMessageAgentSpec(agent) + if (!spec) { + throw new Error('missing spec') + } + expect(finalizeModelDiscoveryOutput(spec, 'fixture/chat\n', '', 0)).toMatchObject({ + success: true, + defaultModelId: 'default', + models: [{ id: 'default' }, { id: 'fixture/chat' }] + }) + } + ) +}) diff --git a/src/main/text-generation/source-control-agent-failure.ts b/src/main/text-generation/source-control-agent-failure.ts index 3bf549f663a..c924343e94d 100644 --- a/src/main/text-generation/source-control-agent-failure.ts +++ b/src/main/text-generation/source-control-agent-failure.ts @@ -9,6 +9,7 @@ import { type AgentGenerationFailureOutput } from './agent-failure-output' import type { InternalTextGenerationResult } from './source-control-text-generation-types' +import { parseOpenCodeGenerationOutput } from '../../shared/opencode-generation-output' export function formatAgentCliFailureMessage( label: string, @@ -45,17 +46,35 @@ export function finalizeFromAgentOutput(args: { emptyResultName: string includeLocalMacDnsHint?: boolean includeStdoutDetail?: boolean + outputFormat?: 'opencode-json' }): InternalTextGenerationResult { const { code, stdout, stderr, label, emptyResultName } = args + const parsed = + args.outputFormat === 'opencode-json' ? parseOpenCodeGenerationOutput(stdout) : null if (code !== 0) { console.error('[commit-message] Generator failed:', { label, exitCode: code, stdout, stderr }) return { success: false, - error: formatAgentCliFailureMessage(label, stdout, stderr, code, args), + error: formatAgentCliFailureMessage( + label, + stdout, + parsed && !parsed.ok && parsed.error !== 'OpenCode returned invalid JSON events.' + ? parsed.error + : stderr, + code, + args + ), failureOutput: captureFailureOutput(label, code, stdout, stderr) } } - const cleaned = cleanGeneratedCommitMessage(stdout) + if (parsed && !parsed.ok) { + return { + success: false, + error: sanitizeAgentFailureDetail(parsed.error) ?? 'OpenCode reported an error.', + failureOutput: captureFailureOutput(label, code, stdout, stderr) + } + } + const cleaned = cleanGeneratedCommitMessage(parsed?.ok ? parsed.text : stdout) if (cleaned) { return { success: true, rawOutput: cleaned, agentLabel: label } } diff --git a/src/main/text-generation/source-control-local-process.ts b/src/main/text-generation/source-control-local-process.ts index da1a2f9a139..4800868d068 100644 --- a/src/main/text-generation/source-control-local-process.ts +++ b/src/main/text-generation/source-control-local-process.ts @@ -186,6 +186,7 @@ export function runLocalSourceControlPlan(input: { stdout, stderr, label: plan.label, + outputFormat: plan.outputFormat, emptyResultName: input.emptyResultName, includeStdoutDetail: operation !== 'branch-name' }) diff --git a/src/main/text-generation/source-control-remote-generation.ts b/src/main/text-generation/source-control-remote-generation.ts index ece1a6de1d8..871345787a9 100644 --- a/src/main/text-generation/source-control-remote-generation.ts +++ b/src/main/text-generation/source-control-remote-generation.ts @@ -66,6 +66,7 @@ export async function runRemoteSourceControlPlan(input: { stdout: result.stdout, stderr: result.stderr, label: plan.label, + outputFormat: plan.outputFormat, emptyResultName: input.emptyResultName, includeLocalMacDnsHint: false, includeStdoutDetail: operation !== 'branch-name' 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 a061201ac8c..298d2ac2e39 100644 --- a/src/main/text-generation/source-control-text-generation-requests.ts +++ b/src/main/text-generation/source-control-text-generation-requests.ts @@ -28,6 +28,7 @@ import type { ResolvedSourceControlAiGenerationParams } from '../../shared/sourc 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 type { @@ -61,21 +62,29 @@ async function executeGenerationPlan(input: { operation: TextGenerationOperation spawnAgent: SpawnSourceControlAgent }): Promise { - const result = await (input.target.kind === 'remote' - ? runRemoteSourceControlPlan({ - plan: input.plan, - target: input.target, - emptyResultName: input.emptyResultName, - operation: input.operation - }) - : runLocalPlanForAgent({ - agentId: input.params.agentId, - plan: input.plan, - target: input.target, - emptyResultName: input.emptyResultName, - operation: input.operation, - spawnAgent: input.spawnAgent - })) + 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. diff --git a/src/relay/agent-exec-handler.ts b/src/relay/agent-exec-handler.ts index 152e8bd72fb..6a4cf295a6b 100644 --- a/src/relay/agent-exec-handler.ts +++ b/src/relay/agent-exec-handler.ts @@ -1,4 +1,5 @@ import { mergeCommandEnvironment } from '../shared/command-environment' +import { PromiseSettlementWaiters } from '../shared/promise-settlement-waiters' import { spawn, type ChildProcess } from 'node:child_process' import { existsSync } from 'node:fs' import { delimiter, join } from 'node:path' @@ -6,6 +7,7 @@ import type { RelayDispatcher, RequestContext } from './dispatcher' import { applyTerminalGitCredentialPromptGuard } from '../shared/terminal-git-credential-guard' import { mergeGitConfigEnvProtocol } from '../shared/git-credential-prompt-env' import { terminateRelaySubprocessTree } from './subprocess-tree-termination' +import { resolveLoginShellEnvironment } from '../main/startup/login-shell-environment' const DEFAULT_TIMEOUT_MS = 60_000 const MAX_TIMEOUT_MS = 5 * 60 * 1000 @@ -76,6 +78,7 @@ type ExecParams = { timeoutMs: unknown env: unknown operation: unknown + shell: unknown } type CancelParams = { @@ -88,7 +91,7 @@ function laneKeyFor(cwd: string, operation: unknown): string { return JSON.stringify([op, cwd]) } -type InFlightExec = { child: ChildProcess; cancel: () => void } +type InFlightExec = { child?: ChildProcess; cancel: () => void } type ExecResult = { stdout: string @@ -113,10 +116,6 @@ export class AgentExecHandler { // operation lanes let cancel target only the user-visible job that stopped. private inFlightByLane = new Map() - private laneKey(cwd: string, operation: unknown): string { - return laneKeyFor(cwd, operation) - } - constructor(dispatcher: RelayDispatcher) { dispatcher.onRequest('agent.execNonInteractive', (p, context) => this.exec(p as ExecParams, context) @@ -126,7 +125,7 @@ export class AgentExecHandler { private async cancel(params: CancelParams): Promise<{ canceled: boolean }> { const cwd = typeof params.cwd === 'string' ? params.cwd : '' - const entry = this.inFlightByLane.get(this.laneKey(cwd, params.operation)) + const entry = this.inFlightByLane.get(laneKeyFor(cwd, params.operation)) if (!entry) { return { canceled: false } } @@ -144,19 +143,56 @@ export class AgentExecHandler { const stdinPayload = typeof params.stdin === 'string' ? params.stdin : null const requestedTimeout = typeof params.timeoutMs === 'number' ? params.timeoutMs : DEFAULT_TIMEOUT_MS - const timeoutMs = Math.max(1_000, Math.min(MAX_TIMEOUT_MS, requestedTimeout)) + const deadline = Date.now() + Math.max(1_000, Math.min(MAX_TIMEOUT_MS, requestedTimeout)) const extraEnv = params.env && typeof params.env === 'object' && !Array.isArray(params.env) ? (params.env as Record) : null - const baseEnv = mergeCommandEnvironment( - process.env, - extraEnv ? {} : undefined, - process.platform - ) + let hostEnv = process.env + if (params.shell === true) { + const timeoutError = new Error('Profile resolution exceeded the request deadline') + const controller = new AbortController() + const key = laneKeyFor(cwd ?? '', params.operation) + const pending = { cancel: (): void => controller.abort() } + this.inFlightByLane.get(key)?.cancel() + this.inFlightByLane.set(key, pending) + context?.signal?.addEventListener('abort', pending.cancel, { once: true }) + if (context?.signal?.aborted) { + pending.cancel() + } + try { + hostEnv = await new PromiseSettlementWaiters( + resolveLoginShellEnvironment({ env: process.env }) + ).wait({ + signal: controller.signal, + timeoutMs: Math.max(1, deadline - Date.now()), + createTimeoutError: () => timeoutError + }) + } catch (error) { + if (error === timeoutError) { + return { stdout: '', stderr: '', exitCode: null, timedOut: true } + } + if (controller.signal.aborted) { + return { stdout: '', stderr: '', exitCode: null, timedOut: false, canceled: true } + } + throw error + } finally { + context?.signal?.removeEventListener('abort', pending.cancel) + if (this.inFlightByLane.get(key) === pending) { + this.inFlightByLane.delete(key) + } + } + if (controller.signal.aborted) { + return { stdout: '', stderr: '', exitCode: null, timedOut: false, canceled: true } + } + } + if (Date.now() >= deadline) { + return { stdout: '', stderr: '', exitCode: null, timedOut: true } + } + const baseEnv = mergeCommandEnvironment(hostEnv, extraEnv ? {} : undefined, process.platform) const overrides = mergeCommandEnvironment({}, extraEnv ?? undefined, process.platform) const spawnEnv = Object.fromEntries( - Object.entries(mergeGitConfigEnvProtocol(baseEnv ?? process.env, overrides)).filter( + Object.entries(mergeGitConfigEnvProtocol(baseEnv ?? hostEnv, overrides)).filter( (entry): entry is [string, string] => typeof entry[1] === 'string' ) ) @@ -171,6 +207,10 @@ export class AgentExecHandler { let child try { const { spawnCmd, spawnArgs } = getWindowsSafeSpawn(binary, args, spawnEnv) + if (Date.now() >= deadline) { + resolve({ stdout: '', stderr: '', exitCode: null, timedOut: true }) + return + } child = spawn(spawnCmd, spawnArgs, { cwd, env: spawnEnv, @@ -195,7 +235,7 @@ export class AgentExecHandler { let timedOut = false let canceled = false let settled = false - const laneKey = typeof cwd === 'string' ? this.laneKey(cwd, params.operation) : '' + const laneKey = typeof cwd === 'string' ? laneKeyFor(cwd, params.operation) : '' let entry: InFlightExec | null = null let timer: ReturnType | null = null let detachChildListeners = (): void => {} @@ -226,22 +266,10 @@ export class AgentExecHandler { // that process until timeout because future cancelExec calls reach only // the newest map entry. this.inFlightByLane.get(laneKey)?.cancel() - entry = { - child, - cancel: cancelCurrent - } + entry = { child, cancel: cancelCurrent } this.inFlightByLane.set(laneKey, entry) } - timer = setTimeout(() => { - timedOut = true - // Why: tree-kill because some CLIs trap SIGTERM and continue streaming; - // also Windows wraps `.cmd` shims in cmd.exe, so the immediate child - // is not the real node.exe process. - terminateRelaySubprocessTree(child) - finish({ stdout, stderr, exitCode: null, timedOut, canceled }) - }, timeoutMs) - const onStdoutData = (chunk: Buffer): void => { stdoutBytes += chunk.byteLength if (stdoutBytes > MAX_OUTPUT_BYTES) { @@ -258,18 +286,10 @@ export class AgentExecHandler { } stderr += chunk.toString('utf-8') } - const onError = (error: Error): void => { - finish({ - stdout, - stderr, - exitCode: null, - timedOut, - spawnError: error.message - }) - } - const onClose = (code: number | null): void => { + const onError = (error: Error): void => + finish({ stdout, stderr, exitCode: null, timedOut, spawnError: error.message }) + const onClose = (code: number | null): void => finish({ stdout, stderr, exitCode: code, timedOut, canceled }) - } child.stdout?.on('data', onStdoutData) child.stderr?.on('data', onStderrData) child.on('error', onError) @@ -281,6 +301,19 @@ export class AgentExecHandler { child.off('close', onClose) } + const expireCurrent = (): void => { + timedOut = true + // Why: wrappers and signal-trapping CLIs require terminating the whole tree. + terminateRelaySubprocessTree(child) + finish({ stdout, stderr, exitCode: null, timedOut, canceled }) + } + const remainingTimeoutMs = deadline - Date.now() + if (remainingTimeoutMs <= 0) { + expireCurrent() + return + } + timer = setTimeout(expireCurrent, remainingTimeoutMs) + if (context?.signal) { if (context.signal.aborted) { cancelCurrent() diff --git a/src/relay/agent-exec-shell-environment.test.ts b/src/relay/agent-exec-shell-environment.test.ts new file mode 100644 index 00000000000..a369251635a --- /dev/null +++ b/src/relay/agent-exec-shell-environment.test.ts @@ -0,0 +1,356 @@ +import { execFile, spawn } from 'node:child_process' +import { existsSync } from 'node:fs' +import type * as FileSystem from 'node:fs' +import type * as ChildProcess from 'node:child_process' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { resolveLoginShellEnvironment } from '../main/startup/login-shell-environment' +import { createFakeChild, createHandlers, requestContext } from './agent-exec-handler-test-harness' + +vi.mock('node:child_process', async (importOriginal) => ({ + ...(await importOriginal()), + spawn: vi.fn(), + execFile: vi.fn() +})) +vi.mock('../main/startup/login-shell-environment', () => ({ + resolveLoginShellEnvironment: vi.fn() +})) + +vi.mock('node:fs', async (importOriginal) => { + const original = await importOriginal() + return { ...original, existsSync: vi.fn(original.existsSync) } +}) + +describe('relay headless generation shell environment', () => { + afterEach(() => { + vi.useRealTimers() + vi.clearAllMocks() + }) + it('uses the execution host profile PATH and retains explicit command overrides', async () => { + vi.mocked(resolveLoginShellEnvironment).mockResolvedValue({ + PATH: '/profile/bin:/usr/bin', + MODEL: 'profile' + }) + const child = createFakeChild() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: This fake provides the streams and lifecycle used by the relay. + vi.mocked(spawn).mockReturnValue(child as never) + const handlers = createHandlers() + const pending = handlers.get('agent.execNonInteractive')?.( + { binary: 'opencode', args: ['run'], cwd: '/repo', shell: true, env: { MODEL: 'override' } }, + requestContext() + ) + await vi.waitFor(() => expect(spawn).toHaveBeenCalled()) + child.emit('close', 0) + await expect(pending).resolves.toMatchObject({ exitCode: 0 }) + expect(spawn).toHaveBeenLastCalledWith( + 'opencode', + ['run'], + expect.objectContaining({ + env: expect.objectContaining({ PATH: '/profile/bin:/usr/bin', MODEL: 'override' }) + }) + ) + }) + + it('does not start generation after cancellation during profile resolution', async () => { + vi.mocked(spawn).mockClear() + let resolveProfile: (env: NodeJS.ProcessEnv) => void = () => {} + vi.mocked(resolveLoginShellEnvironment).mockReturnValue( + new Promise((resolve) => { + resolveProfile = resolve + }) + ) + const handlers = createHandlers() + const pending = handlers.get('agent.execNonInteractive')?.( + { binary: 'opencode', args: ['run'], cwd: '/repo', operation: 'commit-message', shell: true }, + requestContext() + ) + await expect( + handlers.get('agent.cancelExec')?.( + { cwd: '/repo', operation: 'commit-message' }, + requestContext() + ) + ).resolves.toEqual({ canceled: true }) + resolveProfile({ PATH: '/profile/bin' }) + await expect(pending).resolves.toMatchObject({ canceled: true }) + expect(spawn).not.toHaveBeenCalled() + }) +}) + +describe('relay generation deadline includes profile resolution', () => { + afterEach(() => { + vi.useRealTimers() + vi.clearAllMocks() + }) + + it('counts both five-second primary and fallback profile probes in the generation budget', async () => { + vi.useFakeTimers() + vi.mocked(resolveLoginShellEnvironment).mockImplementation( + () => + new Promise((resolve) => + setTimeout(() => { + setTimeout(() => resolve({ PATH: '/profile/bin' }), 5_000) + }, 5_000) + ) + ) + const child = createFakeChild() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The existing fake supplies the relay streams and lifecycle. + vi.mocked(spawn).mockReturnValue(child as never) + const handlers = createHandlers() + const pending = handlers.get('agent.execNonInteractive')?.( + { binary: 'opencode', args: ['run'], cwd: '/repo', shell: true, timeoutMs: 12_000 }, + requestContext() + ) + let result: unknown + void pending?.then((value) => { + result = value + }) + await vi.advanceTimersByTimeAsync(10_000) + expect(spawn).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(1_999) + expect(result).toBeUndefined() + await vi.advanceTimersByTimeAsync(1) + expect(result).toMatchObject({ timedOut: true, exitCode: null }) + child.emit('close', 0) + await pending + }) + + it('settles at the request deadline while the shared profile probe remains pending', async () => { + vi.useFakeTimers() + let resolveProfile: (env: NodeJS.ProcessEnv) => void = () => {} + vi.mocked(resolveLoginShellEnvironment).mockReturnValue( + new Promise((resolve) => { + resolveProfile = resolve + }) + ) + const child = createFakeChild() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The existing fake supplies the relay streams and lifecycle. + vi.mocked(spawn).mockReturnValue(child as never) + const handlers = createHandlers() + const pending = handlers.get('agent.execNonInteractive')?.( + { binary: 'opencode', args: ['run'], cwd: '/repo', shell: true, timeoutMs: 1_000 }, + requestContext() + ) + let result: unknown + void pending?.then((value) => { + result = value + }) + await vi.advanceTimersByTimeAsync(1_000) + try { + expect(result).toMatchObject({ timedOut: true, exitCode: null }) + expect(spawn).not.toHaveBeenCalled() + } finally { + resolveProfile({ PATH: '/profile/bin' }) + await vi.advanceTimersByTimeAsync(0) + child.emit('close', 0) + await pending + } + expect(spawn).not.toHaveBeenCalled() + }) + + it.each(['cancel', 'abort'] as const)( + 'settles %s immediately while the shared profile probe remains pending', + async (kind) => { + vi.useFakeTimers() + let resolveProfile: (env: NodeJS.ProcessEnv) => void = () => {} + vi.mocked(resolveLoginShellEnvironment).mockReturnValue( + new Promise((resolve) => { + resolveProfile = resolve + }) + ) + const handlers = createHandlers() + const controller = new AbortController() + const pending = handlers.get('agent.execNonInteractive')?.( + { + binary: 'opencode', + args: ['run'], + cwd: '/repo', + operation: 'commit-message', + shell: true + }, + { ...requestContext(), signal: controller.signal } + ) + let result: unknown + void pending?.then((value) => { + result = value + }) + if (kind === 'cancel') { + await handlers.get('agent.cancelExec')?.( + { cwd: '/repo', operation: 'commit-message' }, + requestContext() + ) + } else { + controller.abort() + } + await vi.advanceTimersByTimeAsync(0) + try { + expect(result).toMatchObject({ canceled: true, timedOut: false }) + expect(spawn).not.toHaveBeenCalled() + } finally { + resolveProfile({ PATH: '/profile/bin' }) + await vi.advanceTimersByTimeAsync(0) + await pending + } + } + ) +}) + +describe('relay deadline boundaries and profile failures', () => { + afterEach(() => { + vi.useRealTimers() + vi.clearAllMocks() + }) + + it('retains the final millisecond of the request budget without resetting the minimum timeout', async () => { + vi.useFakeTimers() + vi.mocked(resolveLoginShellEnvironment).mockImplementation( + () => new Promise((resolve) => setTimeout(() => resolve({ PATH: '/profile/bin' }), 999)) + ) + const child = createFakeChild() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The existing fake supplies the relay streams and lifecycle. + vi.mocked(spawn).mockReturnValue(child as never) + const handlers = createHandlers() + const pending = handlers.get('agent.execNonInteractive')?.( + { binary: 'opencode', args: ['run'], cwd: '/repo', shell: true, timeoutMs: 1_000 }, + requestContext() + ) + let result: unknown + void pending?.then((value) => { + result = value + }) + await vi.advanceTimersByTimeAsync(999) + expect(spawn).toHaveBeenCalledTimes(1) + expect(result).toBeUndefined() + await vi.advanceTimersByTimeAsync(1) + expect(result).toMatchObject({ timedOut: true }) + child.emit('close', 0) + await pending + }) + + it('does not spawn when profile settlement consumes the whole request budget', async () => { + vi.useFakeTimers() + vi.mocked(resolveLoginShellEnvironment).mockImplementation( + () => new Promise((resolve) => setTimeout(() => resolve({ PATH: '/profile/bin' }), 1_000)) + ) + const child = createFakeChild() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The existing fake supplies the relay streams and lifecycle. + vi.mocked(spawn).mockReturnValue(child as never) + const handlers = createHandlers() + const pending = handlers.get('agent.execNonInteractive')?.( + { binary: 'opencode', args: ['run'], cwd: '/repo', shell: true, timeoutMs: 1_000 }, + requestContext() + ) + let result: unknown + void pending?.then((value) => { + result = value + }) + await vi.advanceTimersByTimeAsync(1_000) + try { + expect(result).toMatchObject({ timedOut: true }) + expect(spawn).not.toHaveBeenCalled() + } finally { + child.emit('close', 0) + await pending + } + }) + + it('preserves a profile rejection and removes the pending cancellation lane', async () => { + const failure = new Error('profile resolution refused') + vi.mocked(resolveLoginShellEnvironment).mockRejectedValue(failure) + const handlers = createHandlers() + const pending = handlers.get('agent.execNonInteractive')?.( + { binary: 'opencode', args: ['run'], cwd: '/repo', operation: 'commit-message', shell: true }, + requestContext() + ) + await expect(pending).rejects.toBe(failure) + expect(spawn).not.toHaveBeenCalled() + await expect( + handlers.get('agent.cancelExec')?.( + { cwd: '/repo', operation: 'commit-message' }, + requestContext() + ) + ).resolves.toEqual({ canceled: false }) + }) +}) + +describe('relay deadline covers synchronous command startup', () => { + afterEach(() => { + vi.useRealTimers() + vi.mocked(existsSync).mockReset() + vi.clearAllMocks() + }) + + it('does not spawn after a Windows PATH lookup exhausts the request budget', async () => { + vi.useFakeTimers() + vi.setSystemTime(0) + vi.mocked(resolveLoginShellEnvironment).mockImplementation(() => { + vi.setSystemTime(999) + return Promise.resolve({ PATH: 'C:\\slow-network-bin' }) + }) + vi.mocked(existsSync).mockImplementation(() => { + vi.setSystemTime(1_001) + return false + }) + const child = createFakeChild() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The existing fake supplies the relay streams and lifecycle. + vi.mocked(spawn).mockReturnValue(child as never) + const originalPlatform = process.platform + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + try { + const pending = createHandlers().get('agent.execNonInteractive')?.( + { binary: 'opencode', args: ['run'], cwd: 'C:\\repo', shell: true, timeoutMs: 1_000 }, + requestContext() + ) + await vi.advanceTimersByTimeAsync(0) + try { + expect(existsSync).toHaveBeenCalled() + expect(spawn).not.toHaveBeenCalled() + await expect(pending).resolves.toMatchObject({ timedOut: true, exitCode: null }) + } finally { + child.emit('close', 0) + await pending + } + } finally { + Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) + } + }) + + it('times out immediately when spawning consumes the remaining request budget', async () => { + vi.useFakeTimers() + vi.setSystemTime(0) + vi.mocked(existsSync).mockReturnValue(false) + const child = createFakeChild() + vi.mocked(spawn).mockImplementation(() => { + vi.setSystemTime(1_001) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The existing fake supplies the relay streams and lifecycle. + return child as never + }) + const handlers = createHandlers() + const pending = handlers.get('agent.execNonInteractive')?.( + { binary: 'opencode', args: ['run'], cwd: '/repo', timeoutMs: 1_000 }, + requestContext() + ) + let result: unknown + void pending?.then((value) => { + result = value + }) + await vi.advanceTimersByTimeAsync(0) + try { + expect(spawn).toHaveBeenCalledTimes(1) + expect(result).toMatchObject({ timedOut: true, exitCode: null }) + if (process.platform === 'win32') { + expect(execFile).toHaveBeenCalledWith( + 'taskkill', + ['/pid', String(child.pid), '/T', '/F'], + expect.any(Function) + ) + } else { + expect(child.kill).toHaveBeenCalled() + } + await expect( + handlers.get('agent.cancelExec')?.({ cwd: '/repo' }, requestContext()) + ).resolves.toEqual({ canceled: false }) + } finally { + child.emit('close', 0) + await pending + } + }) +}) diff --git a/src/shared/command-option-occurrence.ts b/src/shared/command-option-occurrence.ts new file mode 100644 index 00000000000..6dee668829b --- /dev/null +++ b/src/shared/command-option-occurrence.ts @@ -0,0 +1,38 @@ +function matchesOption(token: string, aliases: readonly string[]): boolean { + return aliases.some( + (alias) => + token === alias || + token.startsWith(`${alias}=`) || + (alias.startsWith('-') && + !alias.startsWith('--') && + token.startsWith(alias) && + token.length > alias.length) + ) +} + +export function findOptionOccurrence( + tokens: string[], + aliases: readonly string[], + stopAtTerminator: boolean +): { index: number; consumed: number; value?: string } | null { + for (let index = 0; index < tokens.length; index += 1) { + const token = tokens[index] + if (stopAtTerminator && token === '--') { + break + } + if (!matchesOption(token, aliases)) { + continue + } + const nextToken = tokens[index + 1] + const consumesNext = + aliases.includes(token) && nextToken !== undefined && !nextToken.startsWith('-') + const alias = aliases.find((name) => matchesOption(token, [name])) + const value = consumesNext + ? nextToken + : alias && token !== alias + ? token.slice(alias.length + (token[alias.length] === '=' ? 1 : 0)) + : undefined + return { index, consumed: consumesNext ? 2 : 1, value } + } + return null +} diff --git a/src/shared/commit-message-agent-spec.test.ts b/src/shared/commit-message-agent-spec.test.ts index 553f734a971..a4e52b604f2 100644 --- a/src/shared/commit-message-agent-spec.test.ts +++ b/src/shared/commit-message-agent-spec.test.ts @@ -550,7 +550,7 @@ describe('buildArgs (OpenCode)', () => { '--agent', 'build', '--format', - 'default' + 'json' ]) expect(args).not.toContain(prompt) expect(args).not.toContain('') @@ -571,7 +571,7 @@ describe('buildArgs (OpenCode)', () => { '--agent', 'build', '--format', - 'default', + 'json', '--variant', 'high' ]) @@ -604,7 +604,7 @@ describe('buildArgs (OpenCode 2)', () => { '--agent', 'build', '--format', - 'default' + 'json' ]) expect(args).not.toContain(prompt) expect(args).not.toContain('') @@ -625,7 +625,7 @@ describe('buildArgs (OpenCode 2)', () => { '--agent', 'build', '--format', - 'default' + 'json' ]) expect(args).not.toContain('--variant') }) diff --git a/src/shared/commit-message-agent-specs-primary.ts b/src/shared/commit-message-agent-specs-primary.ts index 729228824bf..ce7b35afcfb 100644 --- a/src/shared/commit-message-agent-specs-primary.ts +++ b/src/shared/commit-message-agent-specs-primary.ts @@ -161,32 +161,25 @@ export function buildPrimaryCommitMessageAgentSpecs({ promptDelivery: 'stdin', buildArgs: ({ model, thinkingLevel }) => [ 'run', - '--model', - model, + ...(model && model !== 'default' ? ['--model', model] : []), '--agent', 'build', '--format', - 'default', + 'json', ...(thinkingLevel ? ['--variant', thinkingLevel] : []) ], singletonOptions: [['--model', '-m'], ['--agent'], ['--format'], ['--variant']], modelSource: 'dynamic', modelDiscovery: { binary: 'opencode', args: ['models'], parse: parseLineModels }, models: [ - { - // Why: OpenCode's hosted GPT models can require workspace billing even - // when `opencode models` lists them. This free model is available in - // discovery and works as a usable out-of-the-box default. - id: 'opencode/deepseek-v4-flash-free', - label: 'OpenCode DeepSeek V4 Flash Free' - }, + { id: 'default', label: 'Config default' }, { id: 'opencode/gpt-5.4-mini', label: 'OpenCode GPT 5.4 Mini', ...withOpenAiThinking('gpt-5.4-mini') } ], - defaultModelId: 'opencode/deepseek-v4-flash-free' + defaultModelId: 'default' }, opencode2: { id: 'opencode2', @@ -195,25 +188,26 @@ export function buildPrimaryCommitMessageAgentSpecs({ promptDelivery: 'stdin', buildArgs: ({ model, thinkingLevel }) => [ 'run', - '--model', - thinkingLevel ? `${model}#${thinkingLevel}` : model, + ...(model && model !== 'default' + ? ['--model', thinkingLevel ? `${model}#${thinkingLevel}` : model] + : []), '--agent', 'build', '--format', - 'default' + 'json' ], singletonOptions: [['--model', '-m'], ['--agent'], ['--format']], modelSource: 'dynamic', modelDiscovery: { binary: 'opencode2', args: ['models'], parse: parseLineModels }, models: [ - { id: 'opencode/deepseek-v4-flash-free', label: 'OpenCode DeepSeek V4 Flash Free' }, + { id: 'default', label: 'Config default' }, { id: 'opencode/gpt-5.4-mini', label: 'OpenCode GPT 5.4 Mini', ...withOpenAiThinking('gpt-5.4-mini') } ], - defaultModelId: 'opencode/deepseek-v4-flash-free' + defaultModelId: 'default' }, pi: { id: 'pi', diff --git a/src/shared/commit-message-plan.test.ts b/src/shared/commit-message-plan.test.ts index 9e499ae4a95..435c4b30c3d 100644 --- a/src/shared/commit-message-plan.test.ts +++ b/src/shared/commit-message-plan.test.ts @@ -77,12 +77,13 @@ describe('planCommitMessageGeneration', () => { '--agent', 'build', '--format', - 'default', + 'json', '--variant', 'high' ], stdinPayload: 'PROMPT', - label: 'OpenCode' + label: 'OpenCode', + outputFormat: 'opencode-json' } }) }) @@ -109,10 +110,11 @@ describe('planCommitMessageGeneration', () => { '--agent', 'build', '--format', - 'default' + 'json' ], stdinPayload: 'PROMPT', - label: 'OpenCode' + label: 'OpenCode', + outputFormat: 'opencode-json' } }) }) @@ -481,7 +483,7 @@ describe('planCommitMessageGeneration', () => { expect(result).toMatchObject({ ok: true, plan: { - args: ['run', '--model', 'opencode/gpt-5.5', '--agent', 'build', '--format', 'default'], + args: ['run', '--model', 'opencode/gpt-5.5', '--agent', 'build', '--format', 'json'], stdinPayload: 'PROMPT' } }) @@ -496,7 +498,7 @@ describe('planCommitMessageGeneration', () => { expect(result).toMatchObject({ ok: true, plan: { - args: ['run', '-m', 'opencode/gpt-5.5', '--agent', 'build', '--format', 'default'] + args: ['run', '-m', 'opencode/gpt-5.5', '--agent', 'build', '--format', 'json'] } }) }) @@ -546,7 +548,7 @@ describe('planCommitMessageGeneration', () => { '--agent', 'build', '--format', - 'default', + 'json', '--share' ], stdinPayload: 'PROMPT' @@ -567,7 +569,7 @@ describe('planCommitMessageGeneration', () => { expect(result).toMatchObject({ ok: true, plan: { - args: ['run', '--model', 'opencode/first', '--agent', 'build', '--format', 'default'] + args: ['run', '--model', 'opencode/first', '--agent', 'build', '--format', 'json'] } }) }) @@ -610,7 +612,7 @@ describe('planCommitMessageGeneration', () => { '--agent', 'build', '--format', - 'default' + 'json' ] } }) @@ -641,7 +643,7 @@ describe('planCommitMessageGeneration', () => { '--agent', 'build', '--format', - 'default' + 'json' ] } }) @@ -662,7 +664,7 @@ describe('planCommitMessageGeneration', () => { ok: true, plan: { binary: 'opencode', - args: ['run', '--model', 'opencode/from-recipe', '--agent', 'build', '--format', 'default'] + args: ['run', '--model', 'opencode/from-recipe', '--agent', 'build', '--format', 'json'] } }) }) @@ -785,3 +787,57 @@ describe('backslash mode reaches every command the user can type (#11375)', () = expect(plan.ok && plan.plan.args).toContain('/my dir') }) }) + +describe('OpenCode format metadata respects option terminators', () => { + it.each(['opencode', 'opencode2'] as const)( + 'ignores literal recipe format values for %s', + (agentId) => { + const result = planCommitMessageGeneration( + { + agentId, + model: 'opencode/gpt-5.4-mini', + agentArgs: '--format default -- --format json' + }, + 'PROMPT' + ) + expect(result.ok).toBe(true) + if (!result.ok) { + throw new Error(result.error) + } + expect(result.plan.args).toContain('--') + expect(result.plan.outputFormat).toBeUndefined() + } + ) + + it('ignores literal equals-form flags after a command override terminator', () => { + const result = planCommitMessageGeneration( + { + agentId: 'opencode', + model: 'opencode/gpt-5.4-mini', + agentCommandOverride: 'opencode --format default -- --format=json' + }, + 'PROMPT' + ) + expect(result.ok).toBe(true) + if (!result.ok) { + throw new Error(result.error) + } + expect(result.plan.outputFormat).toBeUndefined() + }) + + it('retains JSON metadata for the active option before a literal default value', () => { + const result = planCommitMessageGeneration( + { + agentId: 'opencode', + model: 'opencode/gpt-5.4-mini', + agentArgs: '--format=json -- --format default' + }, + 'PROMPT' + ) + expect(result.ok).toBe(true) + if (!result.ok) { + throw new Error(result.error) + } + expect(result.plan.outputFormat).toBe('opencode-json') + }) +}) diff --git a/src/shared/commit-message-plan.ts b/src/shared/commit-message-plan.ts index 1d0bf1e02de..8fe541c6028 100644 --- a/src/shared/commit-message-plan.ts +++ b/src/shared/commit-message-plan.ts @@ -1,3 +1,4 @@ +import { findOptionOccurrence } from './command-option-occurrence' import { planAgentBinary } from './agent-command-plan' export { planAgentBinary } from './agent-command-plan' import type { CommandTemplateBackslash } from './commit-message-prompt' @@ -8,6 +9,7 @@ import { } from './commit-message-agent-spec' import { planCustomCommand, tokenizeCustomCommandTemplate } from './commit-message-prompt' import type { TuiAgent } from './tui-agent' +import { mergeOpenCodeGenerationArgs } from './opencode-generation-command' // Why: planning is a pure transformation from "user request + prompt text" // into "spawn-ready binary + argv". Keeping it in shared lets both the local @@ -37,6 +39,7 @@ export type CommitMessagePlan = { label: string /** Leading command assignments, applied on the execution host. */ env?: Record + outputFormat?: 'opencode-json' } export type CommitMessagePlanResult = @@ -60,39 +63,6 @@ function planAdditionalAgentArgs( const DEFAULT_SINGLETON_OPTIONS: readonly (readonly string[])[] = [['--model']] -function matchesOption(token: string, aliases: readonly string[]): boolean { - return aliases.some( - (alias) => - token === alias || - token.startsWith(`${alias}=`) || - (alias.startsWith('-') && - !alias.startsWith('--') && - token.startsWith(alias) && - token.length > alias.length) - ) -} - -function findOptionOccurrence( - tokens: string[], - aliases: readonly string[], - stopAtTerminator: boolean -): { index: number; consumed: number } | null { - for (let index = 0; index < tokens.length; index += 1) { - const token = tokens[index] - if (stopAtTerminator && token === '--') { - break - } - if (!matchesOption(token, aliases)) { - continue - } - const nextToken = tokens[index + 1] - const consumesNext = - aliases.includes(token) && nextToken !== undefined && !nextToken.startsWith('-') - return { index, consumed: consumesNext ? 2 : 1 } - } - return null -} - function applyRecipeOptionOverride(args: { generatedArgs: string[] recipeArgs: string[] @@ -307,13 +277,24 @@ export function planCommitMessageGeneration( promptDelivery: spec.promptDelivery, prompt: argvPrompt }) + const generationArgs = mergeOpenCodeGenerationArgs( + input.agentId, + command.binary, + merged.prefixArgs, + args + ) + const formatOption = + input.agentId === 'opencode' || input.agentId === 'opencode2' + ? findOptionOccurrence(generationArgs, ['--format'], true) + : null return { ok: true, plan: { binary: command.binary, - args: [...merged.prefixArgs, ...args], + args: generationArgs, stdinPayload: spec.promptDelivery === 'stdin' ? prompt : null, label: spec.label, + ...(formatOption?.value === 'json' ? { outputFormat: 'opencode-json' as const } : {}), ...(command.env ? { env: command.env } : {}) } } diff --git a/src/shared/opencode-generation-command.test.ts b/src/shared/opencode-generation-command.test.ts new file mode 100644 index 00000000000..945300a1c12 --- /dev/null +++ b/src/shared/opencode-generation-command.test.ts @@ -0,0 +1,94 @@ +import { describe, expect, it } from 'vitest' +import { planCommitMessageGeneration } from './commit-message-plan' +import { openCodeVariantRetryPlan } from './opencode-generation-command' + +const rejection = 'Unrecognized flag: --variant in command opencode run' +describe('OpenCode generation commands', () => { + it.each(['opencode', 'opencode.exe', 'opencode.cmd', 'opencode2'])( + 'keeps run before launch flags for %s', + (binary) => { + const result = planCommitMessageGeneration( + { agentId: 'opencode', model: 'default', agentCommandOverride: `${binary} --auto` }, + 'PROMPT' + ) + expect(result).toMatchObject({ + ok: true, + plan: { + args: ['run', '--auto', '--agent', 'build', '--format', 'json'], + stdinPayload: 'PROMPT', + outputFormat: 'opencode-json' + } + }) + } + ) + + it('keeps explicit formatted output recipes on their requested output format', () => { + const result = planCommitMessageGeneration( + { agentId: 'opencode', model: 'default', agentArgs: '--format default' }, + 'PROMPT' + ) + if (!result.ok) { + throw new Error(result.error) + } + expect(result.plan.outputFormat).toBeUndefined() + }) + + it('retries only the precise v2 argv rejection, retaining the prompt and selected model', () => { + const result = planCommitMessageGeneration( + { agentId: 'opencode', model: 'fixture/chat', thinkingLevel: 'high' }, + 'PROMPT' + ) + if (!result.ok) { + throw new Error(result.error) + } + expect(openCodeVariantRetryPlan(result.plan, rejection)).toMatchObject({ + args: ['run', '--model', 'fixture/chat#high', '--agent', 'build', '--format', 'json'], + stdinPayload: 'PROMPT' + }) + expect(openCodeVariantRetryPlan(result.plan, 'provider rejected the request')).toBeNull() + expect(openCodeVariantRetryPlan(result.plan, 'Unrecognized flag: --other')).toBeNull() + }) + it.each([ + '--model=fixture/chat --variant=high', + '-mfixture/chat --variant=high', + '--variant high --model=fixture/chat', + '--variant=high -m fixture/chat' + ])('retries the chosen recipe model and variant in %s', (agentArgs) => { + const result = planCommitMessageGeneration( + { agentId: 'opencode', model: 'other/model', thinkingLevel: 'low', agentArgs }, + 'PROMPT' + ) + if (!result.ok) { + throw new Error(result.error) + } + const retry = openCodeVariantRetryPlan(result.plan, rejection) + expect(retry?.args.join(' ')).toContain('fixture/chat#high') + expect(retry?.args.join(' ')).not.toContain('--variant') + expect(retry?.stdinPayload).toBe('PROMPT') + }) + + it('does not treat prompt tokens after the terminator as options', () => { + expect( + openCodeVariantRetryPlan( + { + binary: 'opencode', + label: 'OpenCode', + stdinPayload: null, + args: ['run', '--model=fixture/chat', '--', '--variant=high'] + }, + rejection + ) + ).toBeNull() + expect( + openCodeVariantRetryPlan( + { + binary: 'opencode', + label: 'OpenCode', + stdinPayload: null, + args: ['run', '--variant=high', '--', '--model=fixture/chat'] + }, + rejection + ) + ).toBeNull() + }) +}) diff --git a/src/shared/opencode-generation-command.ts b/src/shared/opencode-generation-command.ts new file mode 100644 index 00000000000..524cbe5bdbe --- /dev/null +++ b/src/shared/opencode-generation-command.ts @@ -0,0 +1,44 @@ +import { findOptionOccurrence } from './command-option-occurrence' +import type { CommitMessagePlan } from './commit-message-plan' + +export function mergeOpenCodeGenerationArgs( + agentId: string, + binary: string, + prefixArgs: string[], + args: string[] +): string[] { + if ( + (agentId === 'opencode' || agentId === 'opencode2') && + /(?:^|[\\/])opencode2?(?:\.(?:cmd|exe))?$/i.test(binary) && + args[0] === 'run' && + !prefixArgs.includes('--') + ) { + return ['run', ...prefixArgs, ...args.slice(1)] + } + return [...prefixArgs, ...args] +} + +export function openCodeVariantRetryPlan( + plan: CommitMessagePlan, + stderr: string +): CommitMessagePlan | null { + // Retry only an argv rejection, which happens before a model turn starts. + if (!stderr.includes('Unrecognized flag: --variant in command opencode run')) { + return null + } + const variant = findOptionOccurrence(plan.args, ['--variant'], true) + const model = findOptionOccurrence(plan.args, ['--model', '-m'], true) + if (!variant?.value || !model?.value) { + return null + } + const args = [...plan.args] + const replacement = `${model.value.split('#')[0]}#${variant.value}` + if (model.consumed === 2) { + args[model.index + 1] = replacement + } else { + const token = args[model.index] + args[model.index] = `${token.slice(0, token.length - model.value.length)}${replacement}` + } + args.splice(variant.index, variant.consumed) + return { ...plan, args } +} diff --git a/src/shared/opencode-generation-output.test.ts b/src/shared/opencode-generation-output.test.ts new file mode 100644 index 00000000000..1d0e8c4d9ce --- /dev/null +++ b/src/shared/opencode-generation-output.test.ts @@ -0,0 +1,84 @@ +import { describe, expect, it } from 'vitest' +import { parseOpenCodeGenerationOutput } from './opencode-generation-output' + +const frame = (type: string, text: string, id = 'answer'): string => + JSON.stringify({ type, part: { id, text } }) + +describe('OpenCode generation event output', () => { + it('uses the final answer after a tool step', () => { + const output = [ + frame('step_start', ''), + frame('text', 'I will inspect the staged diff.'), + frame('tool_use', 'git diff'), + frame('step_finish', 'tool-calls'), + frame('step_start', ''), + frame('text', 'fix: parse the final answer') + ].join('\n') + expect(parseOpenCodeGenerationOutput(output)).toEqual({ + ok: true, + text: 'fix: parse the final answer' + }) + }) + it('keeps assistant text and ignores tool, reasoning, progress and warning events', () => { + const output = [ + frame('step_start', 'starting'), + frame('reasoning', 'reasoning'), + frame('tool_use', 'git diff output'), + JSON.stringify({ type: 'warning', message: 'notice' }), + frame('text', 'fix: generate messages'), + frame('step_finish', 'done') + ].join('\n') + expect(parseOpenCodeGenerationOutput(output)).toEqual({ + ok: true, + text: 'fix: generate messages' + }) + }) + + it('replaces repeated parts and preserves multiple answer parts', () => { + expect( + parseOpenCodeGenerationOutput( + [frame('text', 'partial'), frame('text', 'subject'), frame('text', 'body', 'body')].join( + '\r\n' + ) + ) + ).toEqual({ ok: true, text: 'subject\nbody' }) + }) + + it.each([ + { name: 'UnknownError', data: { message: 'Model unavailable' } }, + { type: 'provider.no-route', message: 'Unsupported package' } + ])('reports errors instead of accepting a partial answer', (error) => { + const output = `${frame('text', 'partial')}\n${JSON.stringify({ type: 'error', error })}` + expect(parseOpenCodeGenerationOutput(output)).toEqual({ + ok: false, + error: error.message ?? error.data?.message + }) + }) + + it.each([ + { error: { name: 'MessageOutputLengthError', data: {} }, expected: 'MessageOutputLengthError' }, + { + error: { name: 'ProviderError', data: { retryable: false } }, + expected: 'ProviderError' + }, + { + error: { name: 'ProviderError', message: 'Provider rejected the request', data: {} }, + expected: 'Provider rejected the request' + } + ])('reports named errors without a data message', ({ error, expected }) => { + const output = `${frame('text', 'partial')}\n${JSON.stringify({ type: 'error', error })}` + expect(parseOpenCodeGenerationOutput(output)).toEqual({ ok: false, error: expected }) + }) + + it.each([ + '{broken', + '{"title":"not an event"}', + 'null', + JSON.stringify({ type: 'error', error: { name: 'ProviderError', data: { message: 42 } } }) + ])('rejects malformed events %s', (output) => { + expect(parseOpenCodeGenerationOutput(output)).toEqual({ + ok: false, + error: 'OpenCode returned invalid JSON events.' + }) + }) +}) diff --git a/src/shared/opencode-generation-output.ts b/src/shared/opencode-generation-output.ts new file mode 100644 index 00000000000..8a3ba24e494 --- /dev/null +++ b/src/shared/opencode-generation-output.ts @@ -0,0 +1,53 @@ +import { z } from 'zod' + +const eventSchema = z.object({ + type: z.string(), + part: z.object({ id: z.string().optional(), text: z.string().optional() }).optional(), + error: z + .object({ + name: z.string().optional(), + message: z.string().optional(), + data: z.object({ message: z.string().optional() }).optional() + }) + .optional() +}) + +export function parseOpenCodeGenerationOutput( + stdout: string +): { ok: true; text: string } | { ok: false; error: string } { + const parts = new Map() + let anonymousPart = 0 + for (const line of stdout.split(/\r?\n/)) { + if (!line.trim()) { + continue + } + let value: unknown + try { + value = JSON.parse(line) + } catch { + return { ok: false, error: 'OpenCode returned invalid JSON events.' } + } + const parsed = eventSchema.safeParse(value) + if (!parsed.success) { + return { ok: false, error: 'OpenCode returned invalid JSON events.' } + } + const event = parsed.data + if (event.type === 'step_start') { + parts.clear() + } + if (event.type === 'error') { + return { + ok: false, + error: + event.error?.data?.message ?? + event.error?.message ?? + event.error?.name ?? + 'OpenCode reported an error.' + } + } + if (event.type === 'text' && event.part?.text !== undefined) { + parts.set(event.part.id ?? `anonymous-${anonymousPart++}`, event.part.text) + } + } + return { ok: true, text: [...parts.values()].join('\n') } +}