Pre-launch — Gluecron is in final validation. Public signups and git hosting for non-owner users open after launch review.
ccantynz/Gluecron.com
CodeIssuesDiscussionsWikiPull RequestsProjectsCommitsActionsReleasesContributorsPulse● GatesSecuritySettingsDeploymentsPipelineInsightsAgents✨ Explain✨ Ask AI✨ Workspace✨ Spec✨ Tests▓ Debt Map✨ NL Search🏛 Archaeology
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.

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