Pre-launch — Gluecron is in final validation. Public signups and git hosting for non-owner users open after launch review.
CodeIssuesPull RequestsActionsSecurityInsightsSettings
✨ AI
More
Blame · Line-by-line history

ai-review-trio.ts

Each line is annotated with the commit that last touched it. Click any SHA to jump to that commit and see the surrounding change.

ai-review-trio.tsBlame626 lines · 1 contributor
422a2d4Claude1/**
2 * Three-Claude parallel PR review — security / correctness / style.
3 *
4 * Where `ai-review.ts` runs a single Claude pass with a generalist
5 * prompt, this module fans out three concurrent calls — each with a
6 * narrow persona and a stricter remit. The three verdicts are inserted
7 * as separate PR comments (one per reviewer) plus a top-level summary
8 * comment that highlights disagreements.
9 *
10 * When the personas disagree on the same file/line (one says fail, one
11 * says pass), that's a SIGNAL for a human reviewer — surfaced both in
12 * the summary comment and as a yellow callout strip in `pulls.tsx`.
13 *
14 * Hard rules (mirrors `ai-review.ts`):
15 * - Never throws at the boundary. Anthropic, JSON parse, and DB
16 * failures all fail-closed: the reviewer in question lands a
17 * verdict of `fail` with an empty findings list so a human still
18 * sees the attempt.
19 * - All DB writes are best-effort with breadcrumb logging.
20 * - Uses the shared Anthropic client (`getAnthropic` from `ai-client`)
21 * and the shared `audit()` helper from `notify`.
22 *
23 * Wiring: `ai-review.ts`'s `triggerAiReview()` consults
24 * `isTrioReviewEnabled()` (env `AI_TRIO_REVIEW_ENABLED=1`). When on,
25 * it delegates the whole AI review to `runTrioReview()` instead of the
26 * single-Claude path.
27 */
28
29import { eq, and, like } from "drizzle-orm";
30import { db } from "../db";
31import { pullRequests, prComments } from "../db/schema";
32import { getAnthropic, MODEL_SONNET, parseJsonResponse } from "./ai-client";
33import { audit } from "./notify";
34import { recordAiCost, extractUsage } from "./ai-cost-tracker";
35
36// ---------------------------------------------------------------------------
37// Public types
38// ---------------------------------------------------------------------------
39
40export type TrioPersona = "security" | "correctness" | "style";
41
42export type Verdict = "pass" | "fail";
43
44export interface TrioFinding {
45 severity: "low" | "medium" | "high" | "critical" | string;
46 file: string | null;
47 line: number | null;
48 issue: string;
49 fix: string;
50}
51
52export interface TrioVerdict {
53 persona: TrioPersona;
54 verdict: Verdict;
55 findings: TrioFinding[];
56 /** Raw text from Claude — kept for the comment body + debugging. */
57 rawText: string;
58 /** Anthropic latency in ms, observational only. */
59 latencyMs: number;
60 /**
61 * True when the call/parse failed and we synthesised a fail-closed
62 * verdict. The summary comment marks these so humans aren't misled.
63 */
64 failed: boolean;
65}
66
67export interface TrioDisagreement {
68 file: string;
69 line: number | null;
70 /** Personas that returned `fail` for this file/line. */
71 failingPersonas: TrioPersona[];
72 /** Personas that returned `pass` (i.e. had no finding here). */
73 passingPersonas: TrioPersona[];
74}
75
76export interface TrioReviewResult {
77 securityVerdict: TrioVerdict;
78 correctnessVerdict: TrioVerdict;
79 styleVerdict: TrioVerdict;
80 disagreements: TrioDisagreement[];
81}
82
83export interface RunTrioReviewOpts {
84 pullRequestId: string;
85 /** Resolved SHA of the PR head — recorded in the audit metadata. */
86 headSha: string;
87 /** Unified diff text. Will be truncated to `DIFF_BYTE_CAP`. */
88 diff: string;
89 /** Optional repository id for audit attribution. */
90 repositoryId?: string | null;
91 /** Optional override of the model id (tests may want haiku). */
92 model?: string;
93}
94
95// ---------------------------------------------------------------------------
96// Marker constants
97// ---------------------------------------------------------------------------
98
99/**
100 * Per-reviewer marker embedded in each persona's PR comment body.
101 * Used by `pulls.tsx` to render the three cards as a single grid.
102 */
103export const TRIO_COMMENT_MARKER: Record<TrioPersona, string> = {
104 security: "<!-- ai-trio:security -->",
105 correctness: "<!-- ai-trio:correctness -->",
106 style: "<!-- ai-trio:style -->",
107};
108
109/** Marker embedded in the trio summary comment. */
110export const TRIO_SUMMARY_MARKER = "<!-- ai-trio:summary -->";
111
112/** Cap on diff size we feed each reviewer — matches `ai-review.ts`. */
113const DIFF_BYTE_CAP = 100_000;
114
115/** Per-call max tokens. Findings are short JSON — 3k is plenty. */
116const MAX_TOKENS = 3072;
117
118// ---------------------------------------------------------------------------
119// Persona prompts — kept terse so each Claude stays in lane.
120// ---------------------------------------------------------------------------
121
122const SYSTEM_PROMPT_BASE = `You are a focused code reviewer on a pull request. Respond with ONLY valid JSON matching this exact shape:
123
124{
125 "verdict": "pass" | "fail",
126 "findings": [
127 {
128 "severity": "low" | "medium" | "high" | "critical",
129 "file": "path/to/file.ts",
130 "line": 42,
131 "issue": "short description of the problem",
132 "fix": "concrete suggested fix"
133 }
134 ]
135}
136
137Rules:
138- Return "verdict": "fail" if you found ANY finding worth flagging at your remit. Otherwise "pass" with an empty findings array.
139- "line" is the line number in the NEW file (right side of the diff), or null when you can't pin it.
140- Stay strictly inside your remit — do not flag issues that belong to another reviewer.
141- No prose outside the JSON. No code fences.`;
142
143const PERSONA_PROMPT: Record<TrioPersona, string> = {
144 security: `You are the SECURITY reviewer. Be paranoid. Find security issues:
145- SQL/NoSQL injection, command injection, path traversal
146- XSS (reflected, stored, DOM-based) and HTML-escaping gaps
147- Broken auth: missing session/token checks, IDOR, privilege escalation
148- Secret leaks (API keys, tokens, passwords committed to source)
149- Unsafe deserialization (eval, Function, untrusted JSON.parse into prototypes)
150- Crypto misuse (weak hashes, missing HMAC verification, IV reuse)
151- CSRF / SSRF / open redirects
152
153Do NOT flag style, naming, or non-security bugs.
154
155${SYSTEM_PROMPT_BASE}`,
156
157 correctness: `You are the CORRECTNESS reviewer. Find logic bugs:
158- Null/undefined dereference risks where input may not be guaranteed
159- Race conditions, missing await, unhandled promise rejection
160- Off-by-one errors in loops, slices, range checks
161- Missing error handling at system boundaries (fs, network, DB)
162- Wrong operator (== vs ===, & vs &&), inverted conditions
163- Resource leaks (unclosed handles, missing cleanup on error)
164- Type coercion bugs, NaN propagation, integer overflow
165
166Do NOT flag style, naming, security, or readability.
167
168${SYSTEM_PROMPT_BASE}`,
169
170 style: `You are the STYLE reviewer. Find readability + maintainability issues:
171- Inconsistent or unclear naming (vars, functions, types)
172- Missing JSDoc / docstring on newly-added public APIs
173- Functions over ~80 lines or cyclomatic complexity hotspots
174- Magic numbers without named constants
175- Duplicated code blocks that should be extracted
176- Deeply nested conditionals that hurt readability
177
178Do NOT flag security, correctness bugs, or trivial formatting (let the linter do that).
179
180${SYSTEM_PROMPT_BASE}`,
181};
182
183// ---------------------------------------------------------------------------
184// Test seam — let tests inject canned persona outputs instead of calling
185// the real Anthropic API. Pass `null` to reset.
186// ---------------------------------------------------------------------------
187
188export type PersonaRunner = (args: {
189 persona: TrioPersona;
190 diff: string;
191 model: string;
192}) => Promise<{ text: string; inputTokens: number; outputTokens: number }>;
193
194let _runnerOverride: PersonaRunner | null = null;
195
196export function __setPersonaRunnerForTests(fn: PersonaRunner | null): void {
197 _runnerOverride = fn;
198}
199
200// ---------------------------------------------------------------------------
201// Enablement
202// ---------------------------------------------------------------------------
203
204/**
205 * Whether the trio review path is on. Off by default; flip with
206 * `AI_TRIO_REVIEW_ENABLED=1`. Independent from `ANTHROPIC_API_KEY` —
207 * callers still need a key for the actual API calls, but tests can
208 * toggle the flag without a real key via the runner override.
209 */
210export function isTrioReviewEnabled(): boolean {
211 return process.env.AI_TRIO_REVIEW_ENABLED === "1";
212}
213
214/**
215 * Has trio already run on this PR? Detected by a prior summary marker.
216 */
217export async function alreadyTrioReviewed(prId: string): Promise<boolean> {
218 try {
219 const [row] = await db
220 .select({ id: prComments.id })
221 .from(prComments)
222 .where(
223 and(
224 eq(prComments.pullRequestId, prId),
225 eq(prComments.isAiReview, true),
226 like(prComments.body, `%${TRIO_SUMMARY_MARKER}%`)
227 )
228 )
229 .limit(1);
230 return !!row;
231 } catch {
232 return false;
233 }
234}
235
236// ---------------------------------------------------------------------------
237// Core runner
238// ---------------------------------------------------------------------------
239
240/**
241 * Run all three personas in parallel against the same diff, compute
242 * disagreements, persist four prComments (3 verdicts + 1 summary), and
243 * audit the outcome.
244 *
245 * Returns the structured trio result. Never throws.
246 */
247export async function runTrioReview(
248 opts: RunTrioReviewOpts
249): Promise<TrioReviewResult> {
250 const model = opts.model || MODEL_SONNET;
251 const diff =
252 opts.diff.length > DIFF_BYTE_CAP
253 ? opts.diff.slice(0, DIFF_BYTE_CAP)
254 : opts.diff;
255
256 // 1. Fan out the three persona calls.
257 const personas: TrioPersona[] = ["security", "correctness", "style"];
258 const [securityVerdict, correctnessVerdict, styleVerdict] = await Promise.all(
259 personas.map((p) => runOnePersona({ persona: p, diff, model }))
260 );
261
262 // 2. Compute disagreements.
263 const disagreements = computeDisagreements({
264 securityVerdict,
265 correctnessVerdict,
266 styleVerdict,
267 });
268
269 const result: TrioReviewResult = {
270 securityVerdict,
271 correctnessVerdict,
272 styleVerdict,
273 disagreements,
274 };
275
276 // 3. Persist comments (best-effort).
277 await persistTrioComments({
278 pullRequestId: opts.pullRequestId,
279 result,
280 });
281
282 // 4. Audit.
283 try {
284 await audit({
285 action: "ai.review.trio",
286 targetType: "pull_request",
287 targetId: opts.pullRequestId,
288 repositoryId: opts.repositoryId ?? null,
289 metadata: {
290 headSha: opts.headSha,
291 security: securityVerdict.verdict,
292 correctness: correctnessVerdict.verdict,
293 style: styleVerdict.verdict,
294 disagreements: disagreements.length,
295 failed: [
296 securityVerdict.failed ? "security" : null,
297 correctnessVerdict.failed ? "correctness" : null,
298 styleVerdict.failed ? "style" : null,
299 ].filter(Boolean),
300 },
301 });
302 } catch {
303 /* audit is observational only */
304 }
305
306 return result;
307}
308
309// ---------------------------------------------------------------------------
310// One persona — single Anthropic call + fail-closed JSON parse.
311// ---------------------------------------------------------------------------
312
313async function runOnePersona(args: {
314 persona: TrioPersona;
315 diff: string;
316 model: string;
317}): Promise<TrioVerdict> {
318 const t0 = Date.now();
319 let text = "";
320 let inputTokens = 0;
321 let outputTokens = 0;
322 let failed = false;
323
324 try {
325 if (_runnerOverride) {
326 const out = await _runnerOverride({
327 persona: args.persona,
328 diff: args.diff,
329 model: args.model,
330 });
331 text = out.text || "";
332 inputTokens = out.inputTokens || 0;
333 outputTokens = out.outputTokens || 0;
334 } else {
335 const client = getAnthropic();
336 const message = await client.messages.create({
337 model: args.model,
338 max_tokens: MAX_TOKENS,
339 system: PERSONA_PROMPT[args.persona],
340 messages: [
341 {
342 role: "user",
343 content: `Review this diff at your remit (${args.persona}). Return JSON only.\n\n\`\`\`diff\n${args.diff}\n\`\`\``,
344 },
345 ],
346 });
347 text =
348 message.content[0]?.type === "text" ? message.content[0].text : "";
349 const usage = extractUsage(message);
350 inputTokens = usage.input;
351 outputTokens = usage.output;
352 }
353 } catch (err) {
354 failed = true;
355 text = `__error__:${err instanceof Error ? err.message : String(err)}`;
356 }
357
358 // Best-effort cost capture (skipped when call failed and we have no usage).
359 if (!failed && (inputTokens || outputTokens)) {
360 try {
361 await recordAiCost({
362 model: args.model,
363 inputTokens,
364 outputTokens,
365 category: "ai_review",
366 sourceKind: "pull_request",
367 });
368 } catch {
369 /* observational */
370 }
371 }
372
373 const parsed = parseJsonResponse<{
374 verdict?: unknown;
375 findings?: unknown;
376 }>(text);
377
378 let verdict: Verdict = "fail"; // fail-closed default
379 let findings: TrioFinding[] = [];
380
381 if (parsed && typeof parsed === "object") {
382 if (parsed.verdict === "pass") verdict = "pass";
383 else if (parsed.verdict === "fail") verdict = "fail";
384 if (Array.isArray(parsed.findings)) {
385 findings = parsed.findings
386 .map((f) => normaliseFinding(f))
387 .filter((f): f is TrioFinding => !!f);
388 }
389 } else if (!failed) {
390 // Call succeeded but JSON parse failed → fail-closed.
391 failed = true;
392 }
393
394 return {
395 persona: args.persona,
396 verdict,
397 findings,
398 rawText: text,
399 latencyMs: Date.now() - t0,
400 failed,
401 };
402}
403
404function normaliseFinding(raw: unknown): TrioFinding | null {
405 if (!raw || typeof raw !== "object") return null;
406 const r = raw as Record<string, unknown>;
407 const issue =
408 typeof r.issue === "string"
409 ? r.issue
410 : typeof r.description === "string"
411 ? r.description
412 : "";
413 if (!issue) return null;
414 return {
415 severity:
416 typeof r.severity === "string" && r.severity.length > 0
417 ? r.severity
418 : "medium",
419 file: typeof r.file === "string" && r.file.length > 0 ? r.file : null,
420 line:
421 typeof r.line === "number" && Number.isInteger(r.line) && r.line > 0
422 ? r.line
423 : null,
424 issue,
425 fix: typeof r.fix === "string" ? r.fix : "",
426 };
427}
428
429// ---------------------------------------------------------------------------
430// Disagreement detection — file/line pairs where one persona flags `fail`
431// and another would have said `pass` (i.e. no finding at that location).
432// ---------------------------------------------------------------------------
433
434export function computeDisagreements(args: {
435 securityVerdict: TrioVerdict;
436 correctnessVerdict: TrioVerdict;
437 styleVerdict: TrioVerdict;
438}): TrioDisagreement[] {
439 const verdicts: TrioVerdict[] = [
440 args.securityVerdict,
441 args.correctnessVerdict,
442 args.styleVerdict,
443 ];
444
445 // Group findings by file (+ optional line) and record which personas
446 // hit each location.
447 const byKey = new Map<
448 string,
449 {
450 file: string;
451 line: number | null;
452 failingPersonas: Set<TrioPersona>;
453 }
454 >();
455
456 for (const v of verdicts) {
457 for (const f of v.findings) {
458 const file = f.file;
459 if (!file) continue; // can't disagree about an unattributable finding
460 const key = `${file}::${f.line ?? ""}`;
461 let bucket = byKey.get(key);
462 if (!bucket) {
463 bucket = { file, line: f.line, failingPersonas: new Set() };
464 byKey.set(key, bucket);
465 }
466 bucket.failingPersonas.add(v.persona);
467 }
468 }
469
470 const allPersonas: TrioPersona[] = ["security", "correctness", "style"];
471 const disagreements: TrioDisagreement[] = [];
472
473 for (const bucket of byKey.values()) {
474 // Disagreement = at least one persona flagged AND at least one
475 // persona did not. (All three flagging the same location is
476 // unanimous agreement, not a disagreement.)
477 if (
478 bucket.failingPersonas.size === 0 ||
479 bucket.failingPersonas.size === allPersonas.length
480 ) {
481 continue;
482 }
483 disagreements.push({
484 file: bucket.file,
485 line: bucket.line,
486 failingPersonas: Array.from(bucket.failingPersonas).sort() as TrioPersona[],
487 passingPersonas: allPersonas.filter(
488 (p) => !bucket.failingPersonas.has(p)
489 ),
490 });
491 }
492
493 // Stable sort: file then line then severity.
494 disagreements.sort((a, b) => {
495 if (a.file !== b.file) return a.file < b.file ? -1 : 1;
496 return (a.line ?? 0) - (b.line ?? 0);
497 });
498
499 return disagreements;
500}
501
502// ---------------------------------------------------------------------------
503// Comment persistence — 3 per-reviewer + 1 summary.
504// ---------------------------------------------------------------------------
505
506async function persistTrioComments(args: {
507 pullRequestId: string;
508 result: TrioReviewResult;
509}): Promise<void> {
510 // Need the PR's author id to satisfy `prComments.authorId NOT NULL`.
511 // (`ai-review.ts` uses the same pattern.)
512 let authorId: string | null = null;
513 try {
514 const [pr] = await db
515 .select({ authorId: pullRequests.authorId })
516 .from(pullRequests)
517 .where(eq(pullRequests.id, args.pullRequestId))
518 .limit(1);
519 if (pr) authorId = pr.authorId;
520 } catch {
521 /* tolerate */
522 }
523 if (!authorId) return; // can't post comments without an author id
524
525 const verdicts: TrioVerdict[] = [
526 args.result.securityVerdict,
527 args.result.correctnessVerdict,
528 args.result.styleVerdict,
529 ];
530
531 for (const v of verdicts) {
532 const body = renderPersonaCommentBody(v);
533 try {
534 await db.insert(prComments).values({
535 pullRequestId: args.pullRequestId,
536 authorId,
537 isAiReview: true,
538 body,
539 });
540 } catch (err) {
541 console.error(
542 `[ai-review-trio] persona ${v.persona} comment insert failed for PR ${args.pullRequestId}:`,
543 err instanceof Error ? err.message : err
544 );
545 }
546 }
547
548 // Top-level summary.
549 try {
550 await db.insert(prComments).values({
551 pullRequestId: args.pullRequestId,
552 authorId,
553 isAiReview: true,
554 body: renderSummaryCommentBody(args.result),
555 });
556 } catch (err) {
557 console.error(
558 `[ai-review-trio] summary insert failed for PR ${args.pullRequestId}:`,
559 err instanceof Error ? err.message : err
560 );
561 }
562}
563
564// ---------------------------------------------------------------------------
565// Comment body rendering — markdown that's also re-parseable.
566// ---------------------------------------------------------------------------
567
568function renderPersonaCommentBody(v: TrioVerdict): string {
569 const marker = TRIO_COMMENT_MARKER[v.persona];
570 const heading = `${marker}\n## AI ${v.persona[0].toUpperCase() + v.persona.slice(1)} Review — ${v.verdict === "pass" ? "Pass" : "Fail"}`;
571 if (v.failed) {
572 return `${heading}\n\n_AI review call failed; treating as fail-closed. A human reviewer should look at this PR._`;
573 }
574 if (v.findings.length === 0) {
575 return `${heading}\n\nNo ${v.persona} issues detected.`;
576 }
577 const lines = v.findings.map((f) => {
578 const loc = f.file
579 ? `\`${f.file}${f.line ? `:${f.line}` : ""}\``
580 : "_(unattributed)_";
581 return `- **${f.severity}** ${loc} — ${f.issue}${f.fix ? ` _Fix: ${f.fix}_` : ""}`;
582 });
583 return `${heading}\n\n${lines.join("\n")}`;
584}
585
586function renderSummaryCommentBody(r: TrioReviewResult): string {
587 const verdictLine = (v: TrioVerdict): string =>
588 `- **${v.persona}**: ${v.verdict === "pass" ? "✓ pass" : "✗ fail"}${v.failed ? " _(call failed)_" : ""} — ${v.findings.length} finding(s)`;
589
590 const disagreementLines =
591 r.disagreements.length === 0
592 ? "_All three reviewers agree on every flagged location._"
593 : r.disagreements
594 .map((d) => {
595 const loc = `\`${d.file}${d.line ? `:${d.line}` : ""}\``;
596 return `- ${loc} — ${d.failingPersonas.join(", ")} say ✗, ${d.passingPersonas.join(", ")} say ✓`;
597 })
598 .join("\n");
599
600 return [
601 TRIO_SUMMARY_MARKER,
602 "## AI Trio Review",
603 "",
604 "Three independent reviewers ran in parallel — security, correctness, style.",
605 "",
606 "### Verdicts",
607 verdictLine(r.securityVerdict),
608 verdictLine(r.correctnessVerdict),
609 verdictLine(r.styleVerdict),
610 "",
611 "### Disagreements",
612 disagreementLines,
613 ].join("\n");
614}
615
616// ---------------------------------------------------------------------------
617// Test-only exports.
618// ---------------------------------------------------------------------------
619
620export const __test = {
621 PERSONA_PROMPT,
622 normaliseFinding,
623 renderPersonaCommentBody,
624 renderSummaryCommentBody,
625 DIFF_BYTE_CAP,
626};