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