Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/cli/args.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@ describe('parseCliArgs', () => {
expect(result.options.skill).toBe('security-review');
});

it('ignores a leading script delimiter before run arguments', () => {
const result = parseCliArgs(['--', '-vvv', 'src/', '--skill', 'code-review']);
expect(result.command).toBe('run');
expect(result.options.targets).toEqual(['src/']);
expect(result.options.skill).toBe('code-review');
expect(result.options.verbose).toBe(3);
});

it('parses multiple file targets', () => {
const result = parseCliArgs(['file1.ts', 'file2.ts', '--skill', 'security-review']);
expect(result.options.targets).toEqual(['file1.ts', 'file2.ts']);
Expand Down Expand Up @@ -176,6 +184,11 @@ describe('parseCliArgs', () => {
expect(result.options.verbose).toBe(2);
});

it('parses repeated shorthand verbosity', () => {
const result = parseCliArgs(['-vvv']);
expect(result.options.verbose).toBe(3);
});

it('parses --verbose flag', () => {
const result = parseCliArgs(['--verbose']);
expect(result.options.verbose).toBe(1);
Expand Down
12 changes: 7 additions & 5 deletions src/cli/args.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,15 +249,17 @@ function resolveColorOption(values: { color?: boolean; 'no-color'?: boolean }):
}

export function parseCliArgs(argv: string[] = process.argv.slice(2)): ParsedArgs {
const args = argv[0] === '--' ? argv.slice(1) : argv;

// Count -v flags before parsing (parseArgs doesn't handle multiple -v well)
let verboseCount = 0;
const filteredArgv = argv.filter((arg) => {
if (arg === '-v' || arg === '--verbose') {
verboseCount++;
const filteredArgv = args.filter((arg) => {
if (/^-v+$/.test(arg)) {
verboseCount += arg.length - 1;
return false;
}
if (arg === '-vv') {
verboseCount += 2;
if (arg === '--verbose') {
verboseCount++;
return false;
}
return true;
Expand Down
75 changes: 75 additions & 0 deletions src/sdk/analyze.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,37 @@ function makeContextWithTwoHunks(): EventContext {
};
}

function makeContextWithOneHunk(): EventContext {
return {
eventType: 'pull_request',
action: 'opened',
repository: { owner: 'o', name: 'r', fullName: 'o/r', defaultBranch: 'main' },
repoPath: '/tmp/repo',
pullRequest: {
number: 1,
title: 'Test PR',
body: '',
author: 'test',
baseBranch: 'main',
headBranch: 'feature',
headSha: 'head',
baseSha: 'base',
files: [{
filename: 'src/example.ts',
status: 'modified',
additions: 1,
deletions: 1,
patch: [
'@@ -10,1 +10,1 @@',
'-old10',
'+new10',
].join('\n'),
chunks: 1,
}],
},
};
}

describe('filterOutOfRangeFindings', () => {
const hunkRange = { start: 10, end: 20 };

Expand Down Expand Up @@ -420,6 +451,50 @@ describe('runSkill', () => {
vi.restoreAllMocks();
});

it('preserves candidate findings when verification is interrupted', async () => {
const controller = new AbortController();
const runSkillMock = vi.fn()
.mockResolvedValueOnce({
result: {
status: 'success',
text: JSON.stringify({
findings: [makeFinding(10, 'candidate-finding')],
}),
errors: [],
usage: makeUsage(),
},
})
.mockImplementationOnce(async () => {
controller.abort();
throw makeAbortError();
});
vi.mocked(getRuntime).mockReturnValue({
name: 'claude',
runSkill: runSkillMock,
runAuxiliary: vi.fn(),
runSynthesis: vi.fn(),
} as unknown as Runtime);

const report = await runSkill(
{
name: 'security-review',
description: 'Security review.',
prompt: 'Return findings as JSON.',
},
makeContextWithOneHunk(),
{ abortController: controller },
);

expect(runSkillMock).toHaveBeenCalledTimes(2);
expect(report.findings).toEqual([
expect.objectContaining({
title: 'Finding at line 10',
location: { path: 'src/example.ts', startLine: 10 },
}),
]);
expect(report.files?.[0]?.findings).toBe(1);
});

it('preserves partial findings when the shared circuit opens mid-run', async () => {
const controller = new AbortController();
const circuitBreaker = new ProviderFailureCircuitBreaker({
Expand Down
22 changes: 6 additions & 16 deletions src/sdk/verify.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ describe('verifyFindings', () => {
expect(result.findings).toEqual([finding]);
});

it('drops unverified findings when verification is already aborted', async () => {
it('keeps candidate findings when verification is already aborted', async () => {
const runtime = mockRuntime('{"verdict":"keep"}');
vi.mocked(getRuntime).mockReturnValue(runtime);
const abortController = new AbortController();
Expand All @@ -257,17 +257,12 @@ describe('verifyFindings', () => {
onFindingProcessing,
});

expect(result.findings).toEqual([]);
expect(result.findings).toEqual([finding]);
expect(runtime.runSkill).not.toHaveBeenCalled();
expect(onFindingProcessing).toHaveBeenCalledWith({
stage: 'verification',
action: 'rejected',
finding,
reason: 'verification aborted before start',
});
expect(onFindingProcessing).not.toHaveBeenCalled();
});

it('drops unverified findings when verification aborts before verdict', async () => {
it('keeps candidate findings when verification aborts before verdict', async () => {
const abortController = new AbortController();
const runtime: Runtime = {
name: 'claude',
Expand All @@ -289,13 +284,8 @@ describe('verifyFindings', () => {
onFindingProcessing,
});

expect(result.findings).toEqual([]);
expect(onFindingProcessing).toHaveBeenCalledWith({
stage: 'verification',
action: 'rejected',
finding,
reason: 'verification aborted before verdict',
});
expect(result.findings).toEqual([finding]);
expect(onFindingProcessing).not.toHaveBeenCalled();
});

it('propagates authentication errors reported by the verifier runtime', async () => {
Expand Down
20 changes: 6 additions & 14 deletions src/sdk/verify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,18 +220,10 @@ function notifyVerdict(
}
}

function rejectUnverifiedFinding(
options: VerifyFindingsOptions,
finding: Finding,
reason: string
): VerificationTaskResult {
options.onFindingProcessing?.({
stage: 'verification',
action: 'rejected',
finding,
reason,
});
return { finding: undefined };
function keepFindingAfterInterruptedVerification(finding: Finding): VerificationTaskResult {
// An abort is inconclusive, not a verifier rejection. Preserve candidates so
// interrupted runs report the partial findings already collected.
return { finding };
}

/**
Expand All @@ -254,7 +246,7 @@ export async function verifyFindings(
VERIFICATION_CONCURRENCY,
async (finding) => {
if (options.abortController?.signal.aborted) {
return rejectUnverifiedFinding(options, finding, 'verification aborted before start');
return keepFindingAfterInterruptedVerification(finding);
}

try {
Expand Down Expand Up @@ -284,7 +276,7 @@ export async function verifyFindings(
return { finding: next ?? undefined, usage: result?.usage };
} catch (error) {
if (isAbortRequested(error, options.abortController)) {
return rejectUnverifiedFinding(options, finding, 'verification aborted before verdict');
return keepFindingAfterInterruptedVerification(finding);
}

if (error instanceof WardenAuthenticationError) {
Expand Down
Loading