fix(opencode): harden preflight provider checks
This commit is contained in:
parent
55dfc5db14
commit
8db61d4860
3 changed files with 297 additions and 23 deletions
|
|
@ -5,6 +5,7 @@ import {
|
|||
type ExecFileOptions,
|
||||
type ExecOptions,
|
||||
spawn,
|
||||
spawnSync,
|
||||
type SpawnOptions,
|
||||
} from 'child_process';
|
||||
import { existsSync, readFileSync } from 'fs';
|
||||
|
|
@ -21,12 +22,25 @@ function execFileAsync(
|
|||
options: ExecFileOptions = {}
|
||||
): Promise<{ stdout: string; stderr: string }> {
|
||||
return new Promise((resolve, reject) => {
|
||||
const { timeout, killSignal, ...execOptions } = options;
|
||||
const timeoutMs = typeof timeout === 'number' && timeout > 0 ? timeout : 0;
|
||||
const timeoutSignal = normalizeKillSignal(killSignal);
|
||||
let child: ChildProcess | null = null;
|
||||
let settled = false;
|
||||
let stdoutText = '';
|
||||
let stderrText = '';
|
||||
let timeoutHandle: ReturnType<typeof setTimeout> | null = null;
|
||||
const cleanup = (): void => {
|
||||
if (timeoutHandle) {
|
||||
clearTimeout(timeoutHandle);
|
||||
timeoutHandle = null;
|
||||
}
|
||||
untrackCliProcess(child);
|
||||
};
|
||||
child = execFile(cmd, args, options, (err, stdout, stderr) => {
|
||||
child = execFile(cmd, args, execOptions, (err, stdout, stderr) => {
|
||||
if (settled) {
|
||||
return;
|
||||
}
|
||||
settled = true;
|
||||
cleanup();
|
||||
if (err) {
|
||||
|
|
@ -41,6 +55,33 @@ function execFileAsync(
|
|||
});
|
||||
if (!settled) {
|
||||
trackCliProcess(child);
|
||||
if (timeoutMs > 0) {
|
||||
child.stdout?.on('data', (chunk: Buffer | string) => {
|
||||
stdoutText += chunk.toString();
|
||||
});
|
||||
child.stderr?.on('data', (chunk: Buffer | string) => {
|
||||
stderrText += chunk.toString();
|
||||
});
|
||||
timeoutHandle = setTimeout(() => {
|
||||
if (settled) {
|
||||
return;
|
||||
}
|
||||
settled = true;
|
||||
cleanup();
|
||||
killProcessTree(child, timeoutSignal);
|
||||
const error = new Error(
|
||||
`Command timed out after ${timeoutMs}ms: ${cmd} ${args.join(' ')}`
|
||||
);
|
||||
Object.assign(error, {
|
||||
killed: true,
|
||||
signal: timeoutSignal,
|
||||
stdout: stdoutText,
|
||||
stderr: stderrText,
|
||||
});
|
||||
reject(error);
|
||||
}, timeoutMs);
|
||||
timeoutHandle.unref?.();
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
|
@ -55,13 +96,26 @@ function execShellAsync(
|
|||
options: ExecOptions = {}
|
||||
): Promise<{ stdout: string; stderr: string }> {
|
||||
return new Promise((resolve, reject) => {
|
||||
const { timeout, killSignal, ...execOptions } = options;
|
||||
const timeoutMs = typeof timeout === 'number' && timeout > 0 ? timeout : 0;
|
||||
const timeoutSignal = normalizeKillSignal(killSignal);
|
||||
let child: ChildProcess | null = null;
|
||||
let settled = false;
|
||||
let stdoutText = '';
|
||||
let stderrText = '';
|
||||
let timeoutHandle: ReturnType<typeof setTimeout> | null = null;
|
||||
const cleanup = (): void => {
|
||||
if (timeoutHandle) {
|
||||
clearTimeout(timeoutHandle);
|
||||
timeoutHandle = null;
|
||||
}
|
||||
untrackCliProcess(child);
|
||||
};
|
||||
// eslint-disable-next-line sonarjs/os-command, security/detect-child-process -- cmd from known binaryPath+args, not user input (Windows EINVAL fallback)
|
||||
child = exec(cmd, options, (err, stdout, stderr) => {
|
||||
child = exec(cmd, execOptions, (err, stdout, stderr) => {
|
||||
if (settled) {
|
||||
return;
|
||||
}
|
||||
settled = true;
|
||||
cleanup();
|
||||
if (err)
|
||||
|
|
@ -72,6 +126,31 @@ function execShellAsync(
|
|||
});
|
||||
if (!settled) {
|
||||
trackCliProcess(child);
|
||||
if (timeoutMs > 0) {
|
||||
child.stdout?.on('data', (chunk: Buffer | string) => {
|
||||
stdoutText += chunk.toString();
|
||||
});
|
||||
child.stderr?.on('data', (chunk: Buffer | string) => {
|
||||
stderrText += chunk.toString();
|
||||
});
|
||||
timeoutHandle = setTimeout(() => {
|
||||
if (settled) {
|
||||
return;
|
||||
}
|
||||
settled = true;
|
||||
cleanup();
|
||||
killProcessTree(child, timeoutSignal);
|
||||
const error = new Error(`Command timed out after ${timeoutMs}ms: ${cmd}`);
|
||||
Object.assign(error, {
|
||||
killed: true,
|
||||
signal: timeoutSignal,
|
||||
stdout: stdoutText,
|
||||
stderr: stderrText,
|
||||
});
|
||||
reject(error);
|
||||
}, timeoutMs);
|
||||
timeoutHandle.unref?.();
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
|
@ -385,5 +464,66 @@ export function killProcessTree(
|
|||
}
|
||||
}
|
||||
|
||||
child.kill(signal);
|
||||
const childPid = child.pid;
|
||||
const descendants = getDescendantProcessIds(childPid);
|
||||
const targetSignal = signal ?? 'SIGTERM';
|
||||
for (const pid of [childPid, ...descendants.reverse()]) {
|
||||
try {
|
||||
process.kill(pid, targetSignal);
|
||||
} catch {
|
||||
// Best-effort - process may have already exited.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function normalizeKillSignal(signal: ExecFileOptions['killSignal']): NodeJS.Signals {
|
||||
return typeof signal === 'string' ? (signal as NodeJS.Signals) : 'SIGTERM';
|
||||
}
|
||||
|
||||
function getDescendantProcessIds(parentPid: number): number[] {
|
||||
if (process.platform === 'win32') {
|
||||
return [];
|
||||
}
|
||||
|
||||
try {
|
||||
const result = spawnSync('ps', ['-axo', 'pid=,ppid='], {
|
||||
encoding: 'utf8',
|
||||
windowsHide: true,
|
||||
});
|
||||
if (result.error || result.status !== 0 || typeof result.stdout !== 'string') {
|
||||
return [];
|
||||
}
|
||||
|
||||
const childrenByParent = new Map<number, number[]>();
|
||||
for (const line of result.stdout.split('\n')) {
|
||||
const match = line.trim().match(/^(\d+)\s+(\d+)$/);
|
||||
if (!match) {
|
||||
continue;
|
||||
}
|
||||
const pid = Number(match[1]);
|
||||
const ppid = Number(match[2]);
|
||||
const children = childrenByParent.get(ppid);
|
||||
if (children) {
|
||||
children.push(pid);
|
||||
} else {
|
||||
childrenByParent.set(ppid, [pid]);
|
||||
}
|
||||
}
|
||||
|
||||
const descendants: number[] = [];
|
||||
const stack = [...(childrenByParent.get(parentPid) ?? [])];
|
||||
const seen = new Set<number>();
|
||||
while (stack.length > 0) {
|
||||
const pid = stack.pop();
|
||||
if (!pid || seen.has(pid) || pid === process.pid) {
|
||||
continue;
|
||||
}
|
||||
seen.add(pid);
|
||||
descendants.push(pid);
|
||||
stack.push(...(childrenByParent.get(pid) ?? []));
|
||||
}
|
||||
return descendants;
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -282,6 +282,40 @@ describe('OpenCode semantic model gauntlet report helpers', () => {
|
|||
expect(getRunOutcome(category)).toBe('provider-infra-blocked');
|
||||
});
|
||||
|
||||
it('classifies OpenCode quota, auth, and rate failures as provider infrastructure', () => {
|
||||
const stages = createPassingStages({ launchBootstrap: false });
|
||||
const diagnostics = [
|
||||
'OpenCode error: Free usage exceeded. Please subscribe to Go to continue.',
|
||||
'OpenCode session_error: token refresh failed for provider profile',
|
||||
'OpenCode provider returned authentication_failed',
|
||||
'OpenCode provider returned 429 rate limited, retry later',
|
||||
'OpenCode auth failed with HTTP 403',
|
||||
];
|
||||
|
||||
for (const diagnostic of diagnostics) {
|
||||
const category = classifyGauntletFailure({ diagnostics: [diagnostic], stages });
|
||||
|
||||
expect(category, diagnostic).toBe('provider-infra');
|
||||
expect(isCountedForRecommendation(category), diagnostic).toBe(false);
|
||||
expect(getRunOutcome(category), diagnostic).toBe('provider-infra-blocked');
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps transcript failures without provider signals as model behavior', () => {
|
||||
const stages = createPassingStages({ peerRelayAB: false });
|
||||
const category = classifyGauntletFailure({
|
||||
diagnostics: [
|
||||
'runId=abc429def',
|
||||
'transcript: peer relay reply missing expected taskRef',
|
||||
],
|
||||
stages,
|
||||
});
|
||||
|
||||
expect(category).toBe('model-behavior');
|
||||
expect(isCountedForRecommendation(category)).toBe(true);
|
||||
expect(getRunOutcome(category)).toBe('behavioral-fail');
|
||||
});
|
||||
|
||||
it('does not promote a single perfect run to Recommended', () => {
|
||||
const qualified = isModelQualified({
|
||||
averageScore: 100,
|
||||
|
|
@ -1348,6 +1382,49 @@ function isHardProtocolFailure(stages: RunGauntletResult['stages']): boolean {
|
|||
);
|
||||
}
|
||||
|
||||
const PROVIDER_INFRA_TEXT_PATTERNS = [
|
||||
'free usage exceeded',
|
||||
'usage exceeded',
|
||||
'quota exhausted',
|
||||
'quota exceeded',
|
||||
'subscribe to go',
|
||||
'insufficient credits',
|
||||
'requires more credits',
|
||||
'can only afford',
|
||||
'key limit exceeded',
|
||||
'total limit exceeded',
|
||||
'rate limit',
|
||||
'rate limited',
|
||||
'too many requests',
|
||||
'model cooldown',
|
||||
'capacity exceeded',
|
||||
'provider overloaded',
|
||||
'token refresh failed',
|
||||
'authentication_failed',
|
||||
'authentication failed',
|
||||
'unauthorized',
|
||||
'forbidden',
|
||||
'invalid api key',
|
||||
'missing credentials',
|
||||
'not logged in',
|
||||
'login required',
|
||||
'does not have access',
|
||||
'no endpoints found',
|
||||
'selected model',
|
||||
'not found in the live provider catalog',
|
||||
'unable to connect',
|
||||
'provider unavailable',
|
||||
];
|
||||
|
||||
const PROVIDER_INFRA_HTTP_STATUS_PATTERN = /\b(?:401|402|403|429)\b/;
|
||||
|
||||
function hasProviderInfraSignal(text: string): boolean {
|
||||
return (
|
||||
PROVIDER_INFRA_TEXT_PATTERNS.some((pattern) => text.includes(pattern)) ||
|
||||
PROVIDER_INFRA_HTTP_STATUS_PATTERN.test(text)
|
||||
);
|
||||
}
|
||||
|
||||
function classifyGauntletFailure(input: {
|
||||
diagnostics: readonly string[];
|
||||
stages: RunGauntletResult['stages'];
|
||||
|
|
@ -1356,23 +1433,7 @@ function classifyGauntletFailure(input: {
|
|||
if (!text && !isHardProtocolFailure(input.stages)) {
|
||||
return 'none';
|
||||
}
|
||||
if (
|
||||
[
|
||||
'key limit exceeded',
|
||||
'total limit exceeded',
|
||||
'requires more credits',
|
||||
'can only afford',
|
||||
'rate limit',
|
||||
'429',
|
||||
'402',
|
||||
'no endpoints found',
|
||||
'selected model',
|
||||
'not found in the live provider catalog',
|
||||
'unable to connect',
|
||||
'provider unavailable',
|
||||
'insufficient credits',
|
||||
].some((pattern) => text.includes(pattern))
|
||||
) {
|
||||
if (hasProviderInfraSignal(text)) {
|
||||
return 'provider-infra';
|
||||
}
|
||||
if (
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@
|
|||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import path from 'path';
|
||||
import { EventEmitter } from 'events';
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach, vi, type Mock } from 'vitest';
|
||||
|
||||
|
|
@ -12,6 +13,7 @@ vi.mock('child_process', async (importOriginal) => {
|
|||
return {
|
||||
...actual,
|
||||
spawn: vi.fn(),
|
||||
spawnSync: vi.fn(),
|
||||
execFile: vi.fn(),
|
||||
exec: vi.fn(),
|
||||
};
|
||||
|
|
@ -22,6 +24,7 @@ import * as child from 'child_process';
|
|||
import {
|
||||
execCli,
|
||||
killTrackedCliProcesses,
|
||||
killProcessTree,
|
||||
quoteWindowsCmdArg,
|
||||
spawnCli,
|
||||
} from '@main/utils/childProcess';
|
||||
|
|
@ -250,6 +253,7 @@ describe('cli child process helpers', () => {
|
|||
|
||||
it('kills tracked CLI processes on shutdown', () => {
|
||||
setPlatform('linux');
|
||||
const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => true);
|
||||
const fakeChild = {
|
||||
pid: 123,
|
||||
kill: vi.fn(),
|
||||
|
|
@ -259,10 +263,14 @@ describe('cli child process helpers', () => {
|
|||
};
|
||||
(child.spawn as unknown as Mock).mockReturnValue(fakeChild);
|
||||
|
||||
spawnCli('/usr/bin/claude', ['--version']);
|
||||
killTrackedCliProcesses('SIGTERM');
|
||||
try {
|
||||
spawnCli('/usr/bin/claude', ['--version']);
|
||||
killTrackedCliProcesses('SIGTERM');
|
||||
|
||||
expect(fakeChild.kill).toHaveBeenCalledWith('SIGTERM');
|
||||
expect(killSpy).toHaveBeenCalledWith(123, 'SIGTERM');
|
||||
} finally {
|
||||
killSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it('untracks CLI processes after close', () => {
|
||||
|
|
@ -495,5 +503,70 @@ describe('cli child process helpers', () => {
|
|||
stderr: 'bun: not found',
|
||||
});
|
||||
});
|
||||
|
||||
it('kills the launcher process tree on manual execFile timeout', async () => {
|
||||
setPlatform('darwin');
|
||||
vi.useFakeTimers();
|
||||
const execFileMock = child.execFile as unknown as Mock;
|
||||
const spawnSyncMock = child.spawnSync as unknown as Mock;
|
||||
const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => true);
|
||||
const childProcess = new EventEmitter() as EventEmitter & {
|
||||
pid: number;
|
||||
stdout: EventEmitter;
|
||||
stderr: EventEmitter;
|
||||
};
|
||||
childProcess.pid = 100;
|
||||
childProcess.stdout = new EventEmitter();
|
||||
childProcess.stderr = new EventEmitter();
|
||||
spawnSyncMock.mockReturnValue({
|
||||
status: 0,
|
||||
stdout: ['100 1', '101 100', '102 101', '103 100'].join('\n'),
|
||||
});
|
||||
execFileMock.mockImplementation(() => childProcess);
|
||||
|
||||
try {
|
||||
const result = execCli('/tmp/cli-dev', ['runtime', 'status'], { timeout: 100 });
|
||||
const expectation = expect(result).rejects.toMatchObject({
|
||||
killed: true,
|
||||
signal: 'SIGTERM',
|
||||
stdout: 'partial stdout',
|
||||
stderr: 'partial stderr',
|
||||
});
|
||||
childProcess.stdout.emit('data', Buffer.from('partial stdout'));
|
||||
childProcess.stderr.emit('data', Buffer.from('partial stderr'));
|
||||
await vi.advanceTimersByTimeAsync(100);
|
||||
|
||||
await expectation;
|
||||
expect(execFileMock.mock.calls[0][2]).not.toHaveProperty('timeout');
|
||||
expect(killSpy.mock.calls.map(([pid]) => pid)).toEqual(
|
||||
expect.arrayContaining([100, 101, 102, 103])
|
||||
);
|
||||
} finally {
|
||||
killSpy.mockRestore();
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('killProcessTree', () => {
|
||||
it('kills POSIX descendants discovered from ps output', () => {
|
||||
setPlatform('darwin');
|
||||
const spawnSyncMock = child.spawnSync as unknown as Mock;
|
||||
const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => true);
|
||||
spawnSyncMock.mockReturnValue({
|
||||
status: 0,
|
||||
stdout: ['200 1', '201 200', '202 201'].join('\n'),
|
||||
});
|
||||
|
||||
try {
|
||||
killProcessTree({ pid: 200 } as any, 'SIGKILL');
|
||||
|
||||
expect(killSpy.mock.calls.map(([pid]) => pid)).toEqual(
|
||||
expect.arrayContaining([200, 201, 202])
|
||||
);
|
||||
} finally {
|
||||
killSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue