Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
5 changes: 4 additions & 1 deletion packages/warden/src/action/triggers/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,10 @@ export async function executeTrigger(
if (report.error) {
throw (
result.error ??
new SkillRunnerError(report.error.message, { code: report.error.code })
new SkillRunnerError(report.error.message, {
code: report.error.code,
hunkFailures: report.hunkFailures,
})
);
}

Expand Down
30 changes: 20 additions & 10 deletions packages/warden/src/cli/output/tasks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1140,27 +1140,35 @@ describe('runSkillTask all-hunks-fail synthesis', () => {
expect(onSkillError).not.toHaveBeenCalled();
});

it('triggers all_hunks_failed when every hunk succeeded at SDK level but extraction failed for all', async () => {
it('reports extraction codes when every hunk has an extraction failure', async () => {
// Regression test for the if/else mutual-exclusion change: each hunk
// contributes to either failedHunks OR failedExtractions, not both.
// If every hunk fails extraction (SDK call succeeds, parsing fails)
// then failedHunks is 0 — naive `failedHunks === totalHunks` checks
// would silently produce a "0 findings" run. Detection must sum both.
const fakeHunk = {
hunk: { newStart: 1, newCount: 10 },
} as unknown as HunkWithContext;
const fakeHunks = [
{ hunk: { newStart: 1, newCount: 10 } },
{ hunk: { newStart: 20, newCount: 5 } },
] as unknown as HunkWithContext[];
const hunkFailures: HunkFailure[] = [
{
type: 'extraction',
filename: 'a.ts',
lineRange: '1-10',
code: 'extraction_invalid_json',
message: 'invalid_json',
code: 'extraction_llm_timeout',
message: 'timed out',
},
{
type: 'extraction',
filename: 'a.ts',
lineRange: '20-24',
code: 'extraction_llm_failed',
message: 'malformed response',
},
];

vi.spyOn(sdkRunner, 'prepareFiles').mockReturnValue({
files: [{ filename: 'a.ts', hunks: [fakeHunk] }],
files: [{ filename: 'a.ts', hunks: fakeHunks }],
skippedFiles: [],
});

Expand All @@ -1169,7 +1177,7 @@ describe('runSkillTask all-hunks-fail synthesis', () => {
findings: [],
usage: { inputTokens: 0, outputTokens: 0, costUSD: 0 },
failedHunks: 0,
failedExtractions: 1,
failedExtractions: 2,
hunkFailures,
};
vi.spyOn(sdkRunner, 'analyzeFile').mockResolvedValue(failedFileResult);
Expand All @@ -1189,8 +1197,10 @@ describe('runSkillTask all-hunks-fail synthesis', () => {
const result = await runSkillTask(options, 1, noopCallbacks());

expect(result.report).toBeDefined();
expect(result.report!.error?.code).toBe('all_hunks_failed');
expect(result.report!.failedExtractions).toBe(1);
expect(result.report!.error?.code).toBe('extraction_llm_timeout');
expect(result.report!.error?.message).toContain('extraction_llm_timeout, extraction_llm_failed');
expect(result.report!.error?.message).not.toContain('authentication');
expect(result.report!.failedExtractions).toBe(2);
expect(result.report!.findings).toEqual([]);
});

Expand Down
22 changes: 22 additions & 0 deletions packages/warden/src/cli/output/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* Reporter spec: specs/reporters.md
*/

import { isExtractionErrorCode } from '../../types/index.js';
import type { SkillReport, SeverityThreshold, ConfidenceThreshold, Finding, UsageStats, EventContext, HunkFailure, AuxiliaryUsageMap, ErrorCode, HunkTrace } from '../../types/index.js';
import type { SkillDefinition } from '../../config/schema.js';
import { Sentry, emitSkillMetrics, logger } from '../../sentry.js';
Expand Down Expand Up @@ -64,6 +65,18 @@ function firstAnalysisFailureMessage(hunkFailures: HunkFailure[], code: ErrorCod
return hunkFailures.find((failure) => failure.type === 'analysis' && failure.code === code)?.message;
}

function extractionFailureCodes(hunkFailures: HunkFailure[]): ErrorCode[] | undefined {
if (
hunkFailures.length === 0
|| !hunkFailures.every(
(failure) => failure.type === 'extraction' && isExtractionErrorCode(failure.code),
)
) {
return undefined;
}
return [...new Set(hunkFailures.map((failure) => failure.code))];
}

function summarizeRunFailure(args: {
totalHunks: number;
hunkFailures: HunkFailure[];
Expand Down Expand Up @@ -95,6 +108,14 @@ function summarizeRunFailure(args: {
message: `Provider unavailable: all ${totalHunks} chunk${totalHunks === 1 ? '' : 's'} failed to analyze. Warden stopped early.`,
};
}
const extractionCodes = extractionFailureCodes(hunkFailures);
const primaryExtractionCode = extractionCodes?.[0];
if (primaryExtractionCode) {
return {
code: primaryExtractionCode,
message: `Findings extraction failed for all ${totalHunks} chunk${totalHunks === 1 ? '' : 's'} (${extractionCodes.join(', ')}).`,
};
}
Comment thread
cursor[bot] marked this conversation as resolved.
return {
code: 'all_hunks_failed',
message:
Expand Down Expand Up @@ -615,6 +636,7 @@ export async function runSkillTask(
const runnerError = new SkillRunnerError(error.message, {
code: error.code,
providerContext: error.providerContext,
hunkFailures: allHunkFailures,
});
return { name, report: errorReport, error: runnerError, failOn, minConfidence };
}
Expand Down
55 changes: 55 additions & 0 deletions packages/warden/src/output/github-checks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
aggregateSeverityCounts,
createCoreCheck,
updateCoreCheck,
failSkillCheck,
} from './github-checks.js';
import type { Finding, SkillReport } from '../types/index.js';

Expand Down Expand Up @@ -52,6 +53,60 @@ describe('check details URL', () => {
});
});

describe('failSkillCheck', () => {
it('includes sanitized hunk diagnostics in the check summary', async () => {
const update = vi.fn().mockResolvedValue({ data: {} });
const error = Object.assign(new Error('Extraction failed with api_key=secret-value'), {
hunkFailures: [{
type: 'extraction',
filename: 'src/auth.ts',
lineRange: '10-20',
code: 'extraction_invalid_json',
message: 'Invalid response with token=secret-value',
preview: 'api_key=secret-value malformed output',
}],
});

await failSkillCheck(
{ checks: { update } } as never,
123,
error,
{ owner: 'getsentry', repo: 'sentry', headSha: 'abc123' }
);

const summary = update.mock.calls[0]![0].output.summary as string;
expect(summary).toContain('src/auth.ts');
expect(summary).toContain('extraction_invalid_json');
expect(summary).toContain('[redacted]');
expect(summary).not.toContain('secret-value');
});

it('bounds diagnostics to the GitHub check summary byte limit', async () => {
const update = vi.fn().mockResolvedValue({ data: {} });
const error = Object.assign(new Error('Extraction failed'), {
hunkFailures: Array.from({ length: 200 }, (_, index) => ({
type: 'extraction',
filename: `src/file-${index}.ts`,
lineRange: `${index + 1}`,
code: 'extraction_invalid_json',
message: 'Malformed output',
preview: '🔥'.repeat(500),
})),
});

await failSkillCheck(
{ checks: { update } } as never,
123,
error,
{ owner: 'getsentry', repo: 'sentry', headSha: 'abc123' }
);

const summary = update.mock.calls[0]![0].output.summary as string;
expect(Buffer.byteLength(summary, 'utf8')).toBeLessThanOrEqual(65000);
expect(summary).toContain('[diagnostics truncated]');
});
});

describe('severityToAnnotationLevel', () => {
it('maps high to failure', () => {
expect(severityToAnnotationLevel('high')).toBe('failure');
Expand Down
54 changes: 50 additions & 4 deletions packages/warden/src/output/github-checks.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import type { Octokit } from '@octokit/rest';
import { SEVERITY_ORDER, filterFindings } from '../types/index.js';
import type { Severity, SeverityThreshold, ConfidenceThreshold, Finding, SkillReport, UsageStats, AuxiliaryUsageMap } from '../types/index.js';
import { HunkFailureSchema, SEVERITY_ORDER, filterFindings } from '../types/index.js';
import type { Severity, SeverityThreshold, ConfidenceThreshold, Finding, SkillReport, UsageStats, AuxiliaryUsageMap, HunkFailure } from '../types/index.js';
import { formatDuration, formatCost, formatTokens, totalUsageCost, totalUsageStats } from '../cli/output/formatters.js';
import { escapeHtml } from '../utils/index.js';
import { sanitizeErrorMessage } from '../sdk/errors.js';

/**
* GitHub Check annotation for inline code comments.
Expand Down Expand Up @@ -330,6 +331,51 @@ export async function updateSkillCheck(
});
}

const MAX_CHECK_SUMMARY_BYTES = 65000;
const TRUNCATION_NOTICE = '\n\n[diagnostics truncated]';

function truncateCheckSummary(summary: string): string {
if (Buffer.byteLength(summary, 'utf8') <= MAX_CHECK_SUMMARY_BYTES) return summary;

const contentBudget = MAX_CHECK_SUMMARY_BYTES - Buffer.byteLength(TRUNCATION_NOTICE, 'utf8');
let low = 0;
let high = summary.length;
while (low < high) {
const midpoint = Math.ceil((low + high) / 2);
if (Buffer.byteLength(summary.slice(0, midpoint), 'utf8') <= contentBudget) {
low = midpoint;
} else {
high = midpoint - 1;
}
}
return summary.slice(0, low) + TRUNCATION_NOTICE;
}

function failureSummary(error: unknown): string {
const errorMessage = sanitizeErrorMessage(error instanceof Error ? error.message : String(error));
const rawFailures = error && typeof error === 'object' && 'hunkFailures' in error
? (error as { hunkFailures?: unknown }).hunkFailures
: undefined;
const parsedFailures = Array.isArray(rawFailures)
? rawFailures.flatMap((failure) => {
const parsed = HunkFailureSchema.safeParse(failure);
return parsed.success ? [parsed.data] : [];
})
: [];
if (parsedFailures.length === 0) return `Error: ${errorMessage}`;

const sanitizedFailures: HunkFailure[] = parsedFailures.map((failure) => ({
...failure,
filename: sanitizeErrorMessage(failure.filename),
lineRange: sanitizeErrorMessage(failure.lineRange),
message: sanitizeErrorMessage(failure.message),
preview: failure.preview ? sanitizeErrorMessage(failure.preview).slice(0, 500) : undefined,
}));
return truncateCheckSummary(
`Error: ${errorMessage}\n\nHunk failures:\n\`\`\`json\n${JSON.stringify(sanitizedFailures, null, 2)}\n\`\`\``,
);
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
}
Comment thread
cursor[bot] marked this conversation as resolved.

/**
* Mark a skill check as failed due to execution error.
*/
Expand All @@ -339,7 +385,7 @@ export async function failSkillCheck(
error: unknown,
options: CheckOptions
): Promise<void> {
const errorMessage = error instanceof Error ? error.message : String(error);
const summary = failureSummary(error);

await octokit.checks.update({
owner: options.owner,
Expand All @@ -350,7 +396,7 @@ export async function failSkillCheck(
completed_at: new Date().toISOString(),
output: {
title: 'Skill execution failed',
summary: `Error: ${errorMessage}`,
summary,
Comment thread
cursor[bot] marked this conversation as resolved.
},
});
}
Expand Down
66 changes: 66 additions & 0 deletions packages/warden/src/sdk/analyze.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,36 @@ describe('analyzeFile', () => {
vi.restoreAllMocks();
});

it('retries extraction once after a transient auxiliary failure', async () => {
Comment thread
sentry-warden[bot] marked this conversation as resolved.
const runAuxiliary = vi.fn()
.mockResolvedValueOnce({ success: false, error: 'malformed tool call' })
.mockResolvedValueOnce({ success: true, data: { findings: [] } });
vi.mocked(getRuntime).mockReturnValue({
name: 'pi',
runSkill: vi.fn().mockResolvedValue({
result: {
status: 'success',
text: 'No security issues found.',
errors: [],
usage: makeUsage(),
},
}),
runAuxiliary,
runSynthesis: vi.fn(),
} as unknown as Runtime);

const result = await analyzeFile(
{ name: 'security-review', description: 'Security review.', prompt: 'Return findings as JSON.' },
makePreparedFile(),
'/tmp/repo',
{ runtime: 'pi' },
);

expect(runAuxiliary).toHaveBeenCalledTimes(2);
expect(result.failedExtractions).toBe(0);
expect(result.hunkFailures).toEqual([]);
});

it('treats runtime aborts as interruption instead of retry failures', async () => {
const controller = new AbortController();
const runSkill = vi.fn(async () => {
Expand Down Expand Up @@ -701,6 +731,42 @@ describe('runSkill', () => {
vi.restoreAllMocks();
});

it('reports all-extraction failures without authentication guidance', async () => {
vi.mocked(getRuntime).mockReturnValue({
name: 'pi',
runSkill: vi.fn().mockResolvedValue({
result: {
status: 'success',
text: 'No security issues found.',
errors: [],
usage: makeUsage(),
},
}),
runAuxiliary: vi.fn().mockResolvedValue({
success: false,
error: 'malformed tool call',
}),
runSynthesis: vi.fn(),
} as unknown as Runtime);

await expect(runSkill(
{
name: 'security-review',
description: 'Security review.',
prompt: 'Return findings as JSON.',
},
makeContextWithOneHunk(),
{ runtime: 'pi', verifyFindings: false },
)).rejects.toMatchObject({
code: 'extraction_llm_failed',
message: expect.not.stringContaining('authentication'),
hunkFailures: [expect.objectContaining({
type: 'extraction',
code: 'extraction_llm_failed',
})],
});
});

it('records runtime on reports with no hunks to analyze', async () => {
const context = makeContextWithOneHunk();
const report = await runSkill(
Expand Down
Loading
Loading