Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 3 additions & 2 deletions packages/warden/src/cli/output/tasks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1140,7 +1140,7 @@ 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 the specific extraction code when every hunk extraction fails', 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)
Expand Down Expand Up @@ -1189,7 +1189,8 @@ 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!.error?.code).toBe('extraction_invalid_json');
expect(result.report!.error?.message).not.toContain('authentication');
expect(result.report!.failedExtractions).toBe(1);
expect(result.report!.findings).toEqual([]);
});
Expand Down
18 changes: 18 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,15 @@ function firstAnalysisFailureMessage(hunkFailures: HunkFailure[], code: ErrorCod
return hunkFailures.find((failure) => failure.type === 'analysis' && failure.code === code)?.message;
}

function allFailuresHaveExtractionCode(hunkFailures: HunkFailure[]): ErrorCode | undefined {
const first = hunkFailures[0]?.code;
return first
&& hunkFailures.every((failure) => failure.type === 'extraction' && failure.code === first)
&& isExtractionErrorCode(first)
? first
: undefined;
}

function summarizeRunFailure(args: {
totalHunks: number;
hunkFailures: HunkFailure[];
Expand Down Expand Up @@ -95,6 +105,13 @@ function summarizeRunFailure(args: {
message: `Provider unavailable: all ${totalHunks} chunk${totalHunks === 1 ? '' : 's'} failed to analyze. Warden stopped early.`,
};
}
const extractionCode = allFailuresHaveExtractionCode(hunkFailures);
if (extractionCode) {
return {
code: extractionCode,
message: `Findings extraction failed for all ${totalHunks} chunk${totalHunks === 1 ? '' : 's'} (${extractionCode}).`,
};
}
Comment thread
cursor[bot] marked this conversation as resolved.
return {
code: 'all_hunks_failed',
message:
Expand Down Expand Up @@ -615,6 +632,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
30 changes: 30 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,35 @@ 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');
});
});

describe('severityToAnnotationLevel', () => {
it('maps high to failure', () => {
expect(severityToAnnotationLevel('high')).toBe('failure');
Expand Down
32 changes: 28 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,29 @@ export async function updateSkillCheck(
});
}

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 `Error: ${errorMessage}\n\nHunk failures:\n\`\`\`json\n${JSON.stringify(sanitizedFailures, null, 2)}\n\`\`\``;
}
Comment thread
cursor[bot] marked this conversation as resolved.

/**
* Mark a skill check as failed due to execution error.
*/
Expand All @@ -339,7 +363,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 +374,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
30 changes: 30 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
41 changes: 26 additions & 15 deletions packages/warden/src/sdk/analyze.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { DEFAULT_RETRY_CONFIG, calculateRetryDelay, sleep } from './retry.js';
import { aggregateUsage, emptyUsage, estimateTokens, aggregateAuxiliaryUsage, aggregateAuxiliaryUsageAttribution } from './usage.js';
import { buildHunkSystemPrompt, buildHunkUserPrompt, type PRPromptContext } from './prompt.js';
import { extractFindingsJson, extractFindingsWithLLM, validateFindings } from './extract.js';
import type { ExtractFindingsResult } from './extract.js';
import { postProcessFindings } from './post-process.js';
import { buildFileReports } from './report-files.js';
import { getRuntime, getRuntimeProviderOptions } from './runtimes/index.js';
Expand Down Expand Up @@ -184,27 +185,37 @@ async function parseHunkOutput(
return { findings: validateFindings(extracted.findings, filename), extractionFailed: false, extractionMethod: 'regex' };
}

// Tier 2: Try LLM fallback for malformed output
const fallback = await extractFindingsWithLLM(result.text, {
apiKey: options.apiKey,
runtime: options.runtime,
model: options.auxiliaryModel,
maxRetries: options.auxiliaryMaxRetries,
agentName: skillName,
});

if (fallback.success) {
return { findings: validateFindings(fallback.findings, filename), extractionFailed: false, extractionMethod: 'llm', extractionUsage: fallback.usage };
// Tier 2: Try LLM fallback for malformed output, then retry once because
// structured extraction failures can be transient even when analysis succeeded.
const extractionUsage: UsageStats[] = [];
let lastFailure: Extract<ExtractFindingsResult, { success: false }> | undefined;
for (let attempt = 0; attempt < 2; attempt++) {
const fallback = await extractFindingsWithLLM(result.text, {
apiKey: options.apiKey,
runtime: options.runtime,
model: options.auxiliaryModel,
maxRetries: options.auxiliaryMaxRetries,
agentName: skillName,
});
if (fallback.usage) extractionUsage.push(fallback.usage);
if (fallback.success) {
return {
findings: validateFindings(fallback.findings, filename),
extractionFailed: false,
extractionMethod: 'llm',
extractionUsage: aggregateUsage(extractionUsage),
};
}
lastFailure = fallback;
}

// Both tiers failed - return extraction failure info
return {
findings: [],
extractionFailed: true,
extractionMethod: 'none',
extractionError: fallback.error,
extractionPreview: fallback.preview,
extractionUsage: fallback.usage,
extractionError: lastFailure?.error ?? extracted.error,
extractionPreview: lastFailure?.preview ?? extracted.preview,
extractionUsage: extractionUsage.length > 0 ? aggregateUsage(extractionUsage) : undefined,
};
}

Expand Down
7 changes: 5 additions & 2 deletions packages/warden/src/sdk/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
APIConnectionError,
APIConnectionTimeoutError,
} from '@anthropic-ai/sdk';
import type { ErrorCode } from '../types/index.js';
import type { ErrorCode, HunkFailure } from '../types/index.js';
import { InvalidPiModelSelectorError } from './runtimes/model-selectors.js';
import type { RuntimeName, SkillRunStatus } from './runtimes/types.js';

Expand All @@ -24,11 +24,14 @@ export class SkillRunnerError extends Error {
code?: ErrorCode;
/** Sanitized provider diagnostics safe to attach to telemetry. */
providerContext?: ProviderErrorContext;
constructor(message: string, options?: { cause?: unknown; code?: ErrorCode; providerContext?: ProviderErrorContext }) {
/** Per-hunk diagnostics safe to surface in execution failure reports. */
hunkFailures?: HunkFailure[];
constructor(message: string, options?: { cause?: unknown; code?: ErrorCode; providerContext?: ProviderErrorContext; hunkFailures?: HunkFailure[] }) {
super(message, options);
this.name = 'SkillRunnerError';
if (options?.code) this.code = options.code;
if (options?.providerContext) this.providerContext = options.providerContext;
if (options?.hunkFailures) this.hunkFailures = options.hunkFailures;
}
}

Expand Down
17 changes: 17 additions & 0 deletions packages/warden/src/sdk/extract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,23 @@ describe('extractFindingsWithLLM', () => {
expect(canUseRuntimeAuth({ apiKey: 'test-key', runtime: 'claude' })).toBe(true);
});

it('repairs non-empty output without a findings JSON anchor', async () => {
mockCallHaiku.mockResolvedValue({
success: true,
data: { findings: [] },
usage: { inputTokens: 10, outputTokens: 2, costUSD: 0.001 },
});

const result = await extractFindingsWithLLM('I found no security issues.', { apiKey: 'test-key' });

expect(result).toEqual({
success: true,
findings: [],
usage: { inputTokens: 10, outputTokens: 2, costUSD: 0.001 },
});
expect(mockCallHaiku).toHaveBeenCalledOnce();
});

it('preserves the LLM extraction failure prefix for stable error classification', async () => {
mockCallHaiku.mockResolvedValue({
success: false,
Expand Down
16 changes: 9 additions & 7 deletions packages/warden/src/sdk/extract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,7 @@ const LLM_FALLBACK_MAX_TOKENS = 4096;
const LLM_FALLBACK_TIMEOUT_MS = 30000;

/**
* Truncate text for LLM fallback while preserving the findings JSON.
*
* Caller must ensure findings JSON exists in the text before calling.
* Truncate text for LLM fallback, preserving findings JSON when present.
*/
export function truncateForLLMFallback(rawText: string, maxChars: number): string {
if (rawText.length <= maxChars) {
Expand All @@ -163,6 +161,11 @@ export function truncateForLLMFallback(rawText: string, maxChars: number): strin

const findingsIndex = rawText.match(FINDINGS_JSON_START)?.index ?? -1;

// Without an anchor, preserve the start of the response for best-effort repair.
if (findingsIndex === -1) {
return rawText.slice(0, maxChars) + '\n[... truncated]';
}

// If findings starts within our budget, simple truncation from start preserves it
if (findingsIndex < maxChars - 20) {
return rawText.slice(0, maxChars) + '\n[... truncated]';
Expand Down Expand Up @@ -206,16 +209,15 @@ export async function extractFindingsWithLLM(
};
}

// If no findings anchor exists, there's nothing to extract
if (!FINDINGS_JSON_START.test(rawText)) {
if (!rawText.trim()) {
return {
success: false,
error: 'no_findings_to_extract',
preview: rawText.slice(0, 200),
preview: '',
};
}

// Truncate input while preserving JSON boundaries
// Truncate input while preserving JSON boundaries when present
const truncatedText = truncateForLLMFallback(rawText, LLM_FALLBACK_MAX_CHARS);

const userContent = joinPromptSections([
Expand Down
4 changes: 2 additions & 2 deletions packages/warden/src/sdk/runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -391,8 +391,8 @@ describe('extractFindingsWithLLM', () => {
}
});

it('returns error when no findings pattern exists', async () => {
const result = await extractFindingsWithLLM('some output without findings', 'fake-key');
it('returns error when the response is empty', async () => {
const result = await extractFindingsWithLLM(' ', 'fake-key');
expect(result.success).toBe(false);
if (!result.success) {
expect(result.error).toBe('no_findings_to_extract');
Expand Down
Loading