diff --git a/packages/warden/src/output/dedup.ts b/packages/warden/src/output/dedup.ts index 051bc1a1..5f518a45 100644 --- a/packages/warden/src/output/dedup.ts +++ b/packages/warden/src/output/dedup.ts @@ -5,7 +5,7 @@ import type { Confidence, Finding, Severity, UsageStats } from '../types/index.j import { findingLine } from '../types/index.js'; import { escapeHtml } from '../utils/index.js'; import { getRuntime } from '../sdk/runtimes/index.js'; -import { applyMergeGroups, canUseRuntimeAuth } from '../sdk/extract.js'; +import { applyMergeGroups, canUseRuntimeAuth, FINDING_COMPARISON_MAX_TOKENS } from '../sdk/extract.js'; import type { AuxiliaryCallOptions } from '../sdk/extract.js'; import { buildJsonOutputSection, @@ -538,7 +538,7 @@ Return [] if none are duplicates.`), schema: DuplicateMatchesSchema, model: options.model, effort: options.effort, - maxTokens: 512, + maxTokens: FINDING_COMPARISON_MAX_TOKENS, maxRetries: options.maxRetries, }); @@ -788,7 +788,7 @@ Singletons (findings with no duplicates) should not appear in any group. schema: ConsolidationGroupsSchema, model: options.model, effort: options.effort, - maxTokens: 512, + maxTokens: FINDING_COMPARISON_MAX_TOKENS, maxRetries: options.maxRetries, }); diff --git a/packages/warden/src/sdk/extract.ts b/packages/warden/src/sdk/extract.ts index 571887b1..aa37ffc9 100644 --- a/packages/warden/src/sdk/extract.ts +++ b/packages/warden/src/sdk/extract.ts @@ -17,6 +17,9 @@ import { const ExtractedFindingSchema = FindingSchema.omit({ sourceSnippet: true }); +// Reasoning and the JSON answer share the output budget on Pi providers. +export const FINDING_COMPARISON_MAX_TOKENS = 8_192; + /** Pattern to match the start of findings JSON (allows whitespace after brace) */ export const FINDINGS_JSON_START = /\{\s*"findings"/; @@ -576,11 +579,12 @@ Singletons should not appear. Return [] if no findings describe the same issue.` schema: MergeGroupsSchema, model: options?.model, effort: options?.effort, - maxTokens: 512, + maxTokens: FINDING_COMPARISON_MAX_TOKENS, maxRetries: options?.maxRetries, }); if (!result.success) { + console.warn(`LLM cross-location consolidation failed, keeping all findings: ${result.error}`); return { findings, mergedCount: 0, usage: result.usage }; } diff --git a/packages/warden/src/sdk/runtimes/__fixtures__/sentry-python-7780-dedup.json b/packages/warden/src/sdk/runtimes/__fixtures__/sentry-python-7780-dedup.json new file mode 100644 index 00000000..633792c0 --- /dev/null +++ b/packages/warden/src/sdk/runtimes/__fixtures__/sentry-python-7780-dedup.json @@ -0,0 +1,38 @@ +{ + "source": "https://github.com/getsentry/sentry-python/pull/7780", + "findings": [ + { + "id": "YNL-KUW", + "severity": "high", + "confidence": "high", + "title": "A local SERVER_NAME marks public WSGI requests as localhost", + "description": "The WSGI middleware passes `SERVER_NAME` as `url_host` to `is_localhost()`. Since `SERVER_NAME` can describe the internal server and any local input makes the helper return true, a deployment with `SERVER_NAME=localhost` or `127.0.0.1` will mark requests as localhost even when `HTTP_HOST` and the client IP are public. If the inbound localhost filter uses this attribute, it can discard production traffic. Use the effective public request host instead, and add a regression test with a local `SERVER_NAME` and public `HTTP_HOST` and client IP.", + "location": { + "path": "sentry_sdk/integrations/wsgi.py", + "startLine": 24 + } + }, + { + "id": "XNA-MC6", + "severity": "high", + "confidence": "high", + "title": "Local SERVER_NAME marks public WSGI requests as localhost", + "description": "In `wsgi.py`, `is_localhost(url_host=environ[\"SERVER_NAME\"])` treats the internal bind name as the request host, so `SERVER_NAME=localhost|127.0.0.1` with a public `HTTP_HOST` still sets `sentry.is_localhost=True` and can drop production traffic via inbound filters. Pass the public host from `get_host(...)` (or omit `url_host` when `HTTP_HOST` is set), and add a regression case with local `SERVER_NAME` + public Host/IP expecting False.", + "location": { + "path": "tests/integrations/wsgi/test_wsgi.py", + "startLine": 1361 + } + }, + { + "id": "89W-DUX", + "severity": "medium", + "confidence": "high", + "title": "Client-controlled X-Forwarded-For forces localhost client_ip", + "description": "is_localhost() uses get_client_ip(), which prefers the first X-Forwarded-For hop with no use_x_forwarded_for gate, so a client can send X-Forwarded-For: 127.0.0.1 and get sentry.is_localhost=true. Prefer REMOTE_ADDR unless forwarded headers are explicitly trusted.", + "location": { + "path": "sentry_sdk/integrations/wsgi.py", + "startLine": 24 + } + } + ] +} diff --git a/packages/warden/src/sdk/runtimes/pi.test.ts b/packages/warden/src/sdk/runtimes/pi.test.ts index 47d11d23..2b08623e 100644 --- a/packages/warden/src/sdk/runtimes/pi.test.ts +++ b/packages/warden/src/sdk/runtimes/pi.test.ts @@ -8,6 +8,10 @@ import { createAgentSession, } from '@earendil-works/pi-coding-agent'; import { piRuntime } from './pi.js'; +import { mergeCrossLocationFindings } from '../extract.js'; +import { consolidateBatchFindings, deduplicateFindings, findingToExistingComment } from '../../output/dedup.js'; +import { FindingSchema } from '../../types/index.js'; +import dedupFixture from './__fixtures__/sentry-python-7780-dedup.json' with { type: 'json' }; import { createCheckoutFileTools } from './pi-file-tools.js'; import { configureWardenOffline, @@ -835,6 +839,7 @@ describe('piRuntime structured calls', () => { piMocks.listeners = []; piMocks.resourceLoaderOptions = []; piMocks.customTools = []; + piMocks.streamFunction.mockReset(); piMocks.session.agent.streamFunction = piMocks.streamFunction; piMocks.session.prompt.mockImplementation(async () => emitSuccessfulRun( assistantMessage({ content: [{ type: 'text', text: '{"ok":true}' }] }) @@ -843,6 +848,81 @@ describe('piRuntime structured calls', () => { piMocks.modelRuntime.getModels.mockReturnValue([piMocks.model]); }); + it.each(['batch', 'cross-location', 'existing-comments'] as const)( + 'deduplicates PR findings with reasoning longer than 512 tokens during %s comparison', + async (kind) => { + const findings = z.tuple([FindingSchema, FindingSchema, FindingSchema]).parse(dedupFixture.findings); + const [original, duplicate, independent] = findings; + const output = kind === 'existing-comments' + ? '[{"findingIndex":1,"existingIndex":1}]' + : '[[1,2]]'; + // Reproduce a provider that spends the old 512-token budget before it can emit JSON. + piMocks.session.prompt.mockImplementation(async () => { + await piMocks.session.agent.streamFunction(piMocks.model, { messages: [] }, {}); + }); + piMocks.streamFunction.mockImplementation((_model, _context, options: { maxTokens?: number }) => { + const truncated = (options.maxTokens ?? 0) < 2_048; + emitSuccessfulRun(assistantMessage({ + stopReason: truncated ? 'length' : 'stop', + content: [{ type: 'text', text: truncated ? '' : output }], + })); + }); + const options = { runtime: 'pi' as const, model: 'openai/gpt-test', effort: 'high' as const }; + + if (kind === 'existing-comments') { + const result = await deduplicateFindings( + [duplicate, independent], [findingToExistingComment(original)!], options, + ); + expect(result.newFindings.map((finding) => finding.id)).toEqual([independent.id]); + expect(result.duplicateActions).toMatchObject([{ originalFindingId: duplicate.id, matchType: 'semantic' }]); + } else { + const result = kind === 'batch' + ? await consolidateBatchFindings(findings, options) + : await mergeCrossLocationFindings(findings, options); + expect(result.findings.map((finding) => finding.id)).toEqual([original.id, independent.id]); + expect(result.findings[0]?.additionalLocations).toContainEqual(duplicate.location); + } + }, + ); + + it.each(['auxiliary', 'synthesis'] as const)('reports output truncation and its budget for %s calls', async (kind) => { + piMocks.session.prompt.mockImplementation(async () => emitSuccessfulRun(assistantMessage({ + stopReason: 'length', + content: [{ type: 'text', text: '[{"finding' }], + usage: { ...assistantMessage().usage, output: 512 }, + }))); + let spans: TraceSpan[] | undefined; + await Sentry.startSpan({ op: 'test', name: 'truncated comparison' }, async (span) => { + const recorder = startTraceRecorder(span); + const request = { + prompt: 'Compare findings', schema: z.array(z.unknown()), model: 'openai/gpt-test', maxTokens: 512, + }; + const result = await withTraceRecorder(recorder, () => kind === 'auxiliary' + ? piRuntime.runAuxiliary({ ...request, task: 'deduplication' }) + : piRuntime.runSynthesis({ ...request, task: 'consolidation' })); + expect(result).toMatchObject({ + success: false, + error: expect.stringContaining('output token limit'), + usage: { outputTokens: 512 }, + }); + if (!result.success) { + expect(result.error).toContain('finish_reason=length'); + expect(result.error).toContain('max_tokens=512'); + expect(result.error).toContain('output_tokens=512'); + expect(result.error).toContain('gpt-test-2026'); + } + spans = recorder?.snapshot(); + }); + expect(spans).toEqual(expect.arrayContaining([expect.objectContaining({ + op: 'gen_ai.invoke_agent', + attributes: expect.objectContaining({ + 'error.type': 'output_limit', + 'gen_ai.request.max_tokens': 512, + 'gen_ai.response.finish_reasons': ['length'], + }), + })])); + }); + it.each(['auxiliary', 'synthesis'] as const)('honors the output-token limit on %s provider requests', async (kind) => { piMocks.session.prompt.mockImplementation(async () => { await piMocks.session.agent.streamFunction(piMocks.model, { messages: [] }, { diff --git a/packages/warden/src/sdk/runtimes/pi.ts b/packages/warden/src/sdk/runtimes/pi.ts index 8a176711..4c97f708 100644 --- a/packages/warden/src/sdk/runtimes/pi.ts +++ b/packages/warden/src/sdk/runtimes/pi.ts @@ -471,6 +471,7 @@ function statusFromPiMessage(message: AssistantMessage, hitMaxTurns: boolean): S case 'stop': return 'success'; case 'length': + return 'output_limit'; case 'toolUse': return 'provider_error'; case 'error': @@ -484,15 +485,25 @@ function statusFromPiMessage(message: AssistantMessage, hitMaxTurns: boolean): S } } -function normalizePiResult(run: PiPromptResult): SkillRunResult | undefined { +function normalizePiResult(run: PiPromptResult, maxTokens?: number): SkillRunResult | undefined { const message = run.lastAssistant; if (!message) { return undefined; } const errors = message.errorMessage ? [message.errorMessage] : []; + const status = statusFromPiMessage(message, run.hitMaxTurns); + if (status === 'output_limit') { + const details = [ + 'finish_reason=length', + `model=${message.responseModel ?? message.model}`, + ...(maxTokens !== undefined ? [`max_tokens=${maxTokens}`] : []), + `output_tokens=${message.usage.output}`, + ]; + errors.unshift(`Pi response reached the output token limit (${details.join(', ')}). Increase the budget to allow reasoning and a complete answer.`); + } return { - status: statusFromPiMessage(message, run.hitMaxTurns), + status, text: textFromAssistant(message), errors, usage: run.usage, @@ -918,6 +929,7 @@ async function runStructured( ...(request.agentName ? { 'gen_ai.agent.name': request.agentName } : {}), ...(request.task ? { 'warden.ai.task': request.task } : {}), ...(request.model ? { 'gen_ai.request.model': request.model } : {}), + ...(request.maxTokens !== undefined ? { 'gen_ai.request.max_tokens': request.maxTokens } : {}), 'gen_ai.output.type': 'json', }, }, @@ -943,7 +955,7 @@ async function runStructured( timeout: request.timeout, parentSpan: span, }); - const result = normalizePiResult(run); + const result = normalizePiResult(run, request.maxTokens); if (!result) { span.setAttribute('error.type', 'missing_response'); return { success: false, error: 'Pi runtime returned no response', usage: run.usage }; diff --git a/packages/warden/src/sdk/runtimes/types.ts b/packages/warden/src/sdk/runtimes/types.ts index 94e9bf64..b265dd34 100644 --- a/packages/warden/src/sdk/runtimes/types.ts +++ b/packages/warden/src/sdk/runtimes/types.ts @@ -26,6 +26,7 @@ export type SkillRunStatus = | 'provider_error' | 'auth_error' | 'turn_limit' + | 'output_limit' | 'budget_limit' | 'aborted' | 'structured_output_error';