diff --git a/harness/pstack/claude-code/skills/poteto-mode/playbooks/babysit.md b/harness/pstack/claude-code/skills/poteto-mode/playbooks/babysit.md index 14d5783d4..436a6997f 100644 --- a/harness/pstack/claude-code/skills/poteto-mode/playbooks/babysit.md +++ b/harness/pstack/claude-code/skills/poteto-mode/playbooks/babysit.md @@ -17,7 +17,7 @@ Babysitting starts when the user asks for it, which is normally once a phase or Answer a user question mid-loop and continue. Only an explicit stop ends the loop before the active forge's stop condition. On GitHub, that is `READY` in single or stack mode, or a `WAITING`/`merge-queue` report or `COMPLETE` in queued mode. On Origin, that is the merge-ready state defined above. For a GitHub queued stack, capture the PR list bottom-to-top once and pass the same frozen list to every rearm. Revise the list only for the sanctioned follow-up PR from step 4. Append it at the end, drop the merged owner, and rearm with the corrected snapshot. 7. **Classify CI before any retrigger.** Flake or infrastructure earns one fresh build, never a job retry. One retry only. An identical second failure means it was never flake, so reclassify and read the child logs instead of retrying blind. A failure in code the diff never touches means a stale base, so check with `git merge-base --is-ancestor` before assuming flake. Report a stale base as needing a rebase instead of burning retries. Only a failure in the diff's own code gets a commit. -8. **Bugbot is triaged skeptically, always.** Verify each claim against the code per `../references/bugbot-triage.md`. Fix real findings with a red-first proof in the lowest PR that owns the code, never at the tip unless the owning PR has merged. In that case, use step 4's sanctioned follow-up PR. Per step 2, upstack fixes wait for step 5's next frontier-driven push wave. Push that wave before replying so the reply cites the commit. On Origin, reply with `origin pr thread reply --body-file `. On GitHub, call `gh api --method POST "repos///pulls//comments//replies" --input ` and put the reply body in the JSON file as data. Never interpolate comment text or a reply into a shell command. Dismiss noise with the concrete disproof on the thread. On GitHub, use the watcher's Bugbot pass count. On Origin, derive the pass count from `origin pr thread list` and the review history. From the third pass on, lean toward dismissing documented patterns, still escalating anything touching security, auth, billing, data, or migrations rather than dismissing it yourself. Never churn code to quiet a bot. +8. **Bugbot is triaged skeptically, always.** Verify each claim against the code per `../references/bugbot-triage.md`. Fix real findings with a red-first proof in the lowest PR that owns the code, never at the tip unless the owning PR has merged. In that case, use step 4's sanctioned follow-up PR. Per step 2, upstack fixes wait for step 5's next frontier-driven push wave. Push that wave before replying so the reply cites the commit. On Origin, reply with `origin pr thread reply --body-file `. On GitHub, call `gh api --method POST "repos///pulls//comments//replies" --input ` and put the reply body in the JSON file as data. Never interpolate comment text or a reply into a shell command. Dismiss noise with the concrete disproof on the thread. On GitHub, use the watcher's Bugbot pass count, each thread's `reviewBotPasses`. On Origin, derive the pass count from `origin pr thread list` and the review history. From the third pass on, lean toward dismissing documented patterns, still escalating anything touching security, auth, billing, data, or migrations rather than dismissing it yourself. Never churn code to quiet a bot. 9. **Stop at the human's line.** Owner approval is a wait, not a blocker to fix. Babysitting never authorizes merging. Only an explicit request to merge, land, ship, or merge when ready does. Route that request to Shipping. Surface the escalation and keep working the rest. After GitHub reports `READY`, a queued `WAITING`/`merge-queue` stop, or `COMPLETE`, or after Origin reports the frontier merge-ready, sweep the run's triage decisions once. Offer any team-useful dismissal pattern as a candidate entry in the shared rubric (`../references/bugbot-triage.md`) and its own PR. Never keep it only in private memory. `drive` ends at merge-ready. Landing the stack is `playbooks/shipping.md`. diff --git a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/cli.ts b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/cli.ts index ec8a2f27c..e8974c895 100644 --- a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/cli.ts +++ b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/cli.ts @@ -19,6 +19,12 @@ import { type WatchClock, } from "./policy.ts"; import { renderJson, renderPretty } from "./render.ts"; +import { + DEFAULT_REVIEW_BOTS, + ReviewBotConfigError, + loadReviewBots, + type ReviewBots, +} from "./review-bots.ts"; import type * as T from "./types.ts"; import { nonEmpty, parsePrNumber } from "./types.ts"; export interface CliOptions { @@ -30,6 +36,7 @@ export interface CliOptions { readonly statusOnly: boolean; readonly pretty: boolean; readonly polling: T.PollingOptions; + readonly reviewBots: ReviewBots; } function positiveNumber(value: string): number { const parsed = Number(value); @@ -56,6 +63,14 @@ function prNumber(value: string): T.PrNumber { throw new InvalidArgumentError("must be a positive integer"); } } +function reviewBotFile(value: string): ReviewBots { + try { + return loadReviewBots(value); + } catch (error) { + if (!(error instanceof ReviewBotConfigError)) throw error; + throw new InvalidArgumentError(error.message); + } +} function stackPrList(value: string): T.NonEmpty { const numbers = value.split(",").map((part) => prNumber(part.trim())); if (new Set(numbers).size !== numbers.length) @@ -78,6 +93,7 @@ interface RawOptions { readonly statusOnly: boolean; readonly allowDraft: boolean; readonly pretty: boolean; + readonly reviewBots?: ReviewBots; } export function parseArgs( argv: readonly string[], @@ -128,7 +144,12 @@ export function parseArgs( ) .option("--status-only", "print one status table and exit 0", false) .option("--allow-draft", "do not treat a draft as a merge gate", false) - .option("--pretty", "render human text instead of JSON", false); + .option("--pretty", "render human text instead of JSON", false) + .option( + "--review-bots ", + "review-bot config JSON (default: review-bots.json beside this script)", + reviewBotFile + ); program.parse(argv, { from: "user" }); const raw = program.opts(); if (raw.stackPrs !== undefined && !raw.queuedStack) @@ -148,6 +169,7 @@ export function parseArgs( maxQueryErrors: raw.maxQueryErrors, allowDraft: raw.allowDraft, }, + reviewBots: raw.reviewBots ?? DEFAULT_REVIEW_BOTS, }; } export interface CliRuntime { @@ -207,7 +229,12 @@ export async function main( runtime.stdout(render(verdict)); return verdict.exitCode; } - const dependencies = { reader: runtime.reader, clock: runtime.clock, emit }; + const dependencies = { + reader: runtime.reader, + clock: runtime.clock, + emit, + reviewBots: options.reviewBots, + }; const verdict = options.mode === "queued-stack" && !options.statusOnly ? await runQueued({ dependencies, contexts, options: options.polling }) diff --git a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/github.test.ts b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/github.test.ts index 4e5893838..66ac40bb5 100644 --- a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/github.test.ts +++ b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/github.test.ts @@ -291,8 +291,11 @@ it("annotates Bugbot threads with distinct review-pass counts", () => { }; const threads = buildReviewThreads(parseReviewThreadPage(response).threads); expect(threads).toHaveLength(2); - expect(threads.map((thread) => thread.isBugbot)).toEqual([true, true]); - expect(threads.map((thread) => thread.bugbotReviewPasses)).toEqual([3, 3]); + expect(threads.map((thread) => thread.reviewBot)).toEqual([ + "bugbot", + "bugbot", + ]); + expect(threads.map((thread) => thread.reviewBotPasses)).toEqual([3, 3]); }); describe("review thread pagination", () => { @@ -363,7 +366,7 @@ describe("review thread pagination", () => { ], }); const threads = await resolveReviewThreads(reader, context); - expect(threads.map((item) => item.bugbotReviewPasses)).toEqual([2, 2]); + expect(threads.map((item) => item.reviewBotPasses)).toEqual([2, 2]); }); it("drops resolved threads from later pages too", async () => { diff --git a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/github.ts b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/github.ts index 123a33b19..1327472ca 100644 --- a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/github.ts +++ b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/github.ts @@ -1,4 +1,10 @@ import { spawn } from "node:child_process"; +import { + DEFAULT_REVIEW_BOTS, + countReviewPasses, + reviewBotFor, + type ReviewBots, +} from "./review-bots.ts"; import type * as T from "./types.ts"; import { nonEmpty, parsePrNumber } from "./types.ts"; export const REVIEW_THREADS_QUERY = @@ -326,33 +332,6 @@ function parseComment(value: unknown): T.ReviewComment { createdAt: string(object.createdAt, "review comment.createdAt"), }; } -function isBugbot(comment: T.ReviewComment | null): boolean { - if (comment === null) return false; - const author = (comment.authorLogin ?? "").toLowerCase(); - const body = comment.body.toLowerCase(); - return ( - author.includes("bugbot") || - (author === "cursor" && - [ - "bugbot", - "cursor_automation_id", - "agentic security review", - "description start", - "severity", - ].some((token) => body.includes(token))) - ); -} -function passKey(comment: T.ReviewComment | null): string | null { - if (comment === null) return null; - for (const pattern of [ - /RUN_ID:\s*([a-zA-Z0-9_.:-]+)/, - /CURSOR_AUTOMATION_ID:\s*([a-zA-Z0-9_.:-]+)/, - ]) { - const match = pattern.exec(comment.body); - if (match?.[1]) return match[1]; - } - return null; -} export function parseReviewThreadPage(value: unknown): T.ReviewThreadPage { const reviewThreads = record( at(value, ["data", "repository", "pullRequest", "reviewThreads"]), @@ -384,25 +363,24 @@ export function parseReviewThreadPage(value: unknown): T.ReviewThreadPage { return { threads, endCursor: page.hasNextPage && cursor ? cursor : null }; } export function buildReviewThreads( - threads: readonly T.RawReviewThread[] + threads: readonly T.RawReviewThread[], + bots: ReviewBots = DEFAULT_REVIEW_BOTS ): readonly T.ReviewThread[] { - const keys = new Set(); - let keyless = false; - for (const thread of threads) { - if (!isBugbot(thread.firstComment)) continue; - const key = passKey(thread.firstComment); - if (key === null) keyless = true; - else keys.add(key); - } - const passes = keys.size > 0 ? keys.size : keyless ? 1 : 0; + const passes = countReviewPasses( + threads.map((thread) => thread.firstComment), + bots + ); return threads .filter((thread) => !thread.resolved) - .map(({ id, firstComment }) => ({ - id, - firstComment, - isBugbot: isBugbot(firstComment), - bugbotReviewPasses: passes, - })); + .map(({ id, firstComment }) => { + const reviewBot = reviewBotFor(firstComment, bots)?.id ?? null; + return { + id, + firstComment, + reviewBot, + reviewBotPasses: reviewBot === null ? 0 : (passes.get(reviewBot) ?? 0), + }; + }); } export function parsePullRequest( value: unknown, @@ -634,7 +612,8 @@ export async function resolveChecks( } export async function resolveReviewThreads( reader: T.GitHubReader, - context: T.PrContext + context: T.PrContext, + bots: ReviewBots = DEFAULT_REVIEW_BOTS ): Promise { const threads: T.RawReviewThread[] = []; let after: string | null = null; @@ -643,7 +622,7 @@ export async function resolveReviewThreads( threads.push(...page.threads); after = page.endCursor; } while (after !== null); - return buildReviewThreads(threads); + return buildReviewThreads(threads, bots); } export async function resolveContext(args: { readonly reader: T.GitHubReader; diff --git a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/policy.ts b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/policy.ts index 8179f2d8b..c5ed0b1fc 100644 --- a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/policy.ts +++ b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/policy.ts @@ -3,6 +3,11 @@ import { resolveChecks, resolveReviewThreads, } from "./github.ts"; +import { + DEFAULT_REVIEW_BOTS, + isReviewBotCheck, + type ReviewBots, +} from "./review-bots.ts"; import type * as T from "./types.ts"; import { nonEmpty } from "./types.ts"; export function assessGitHubMerge(args: { @@ -50,24 +55,20 @@ async function mergeAssessment( }), }; } -const AUTOMATION_TOKENS = [ - "bugbot", - "security review", - "pr review automation", - "review automation", -] as const; export async function readSnapshot(args: { readonly reader: T.GitHubReader; readonly context: T.PrContext; readonly pendingHistory: "include" | "omit"; readonly allowDraft: boolean; + readonly reviewBots?: ReviewBots; }): Promise { + const bots = args.reviewBots ?? DEFAULT_REVIEW_BOTS; const facts = await args.reader.pullRequest(args.context); if (facts.state === "MERGED" || facts.mergedAt !== null) return { kind: "merged", context: args.context, facts }; if (facts.state === "CLOSED") return { kind: "closed", context: args.context, facts }; - const threads = await resolveReviewThreads(args.reader, args.context); + const threads = await resolveReviewThreads(args.reader, args.context, bots); const checks = await resolveChecks(args.reader, args.context); const failed = nonEmpty( checks.checks.filter( @@ -130,11 +131,7 @@ export async function readSnapshot(args: { threads, ci, reviewAutomationRunning: checks.checks.some( - (check) => - check.kind === "pending" && - AUTOMATION_TOKENS.some((token) => - check.name.toLowerCase().includes(token) - ) + (check) => check.kind === "pending" && isReviewBotCheck(check.name, bots) ), }; } @@ -348,6 +345,7 @@ export interface RunDependencies { readonly reader: T.GitHubReader; readonly clock: WatchClock; readonly emit: (verdict: T.ProgressVerdict) => void; + readonly reviewBots?: ReviewBots; } const deadlinePassed = ( started: number, @@ -437,6 +435,7 @@ export async function runSimple(args: { context, pendingHistory: "include", allowDraft: args.options.allowDraft, + reviewBots: args.dependencies.reviewBots, }) ); const complete = nonEmpty(rows); @@ -750,6 +749,7 @@ export async function runQueued(args: { context, pendingHistory: "omit", allowDraft: args.options.allowDraft, + reviewBots: args.dependencies.reviewBots, }); const applied = applyQueueSnapshot( state, diff --git a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/render.ts b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/render.ts index 0e9358393..1d4e752fc 100644 --- a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/render.ts +++ b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/render.ts @@ -59,8 +59,8 @@ function threadLine(thread: T.ReviewThread): string { comment?.path ?? "None", comment?.line ?? "None", comment?.authorLogin ?? "None", - `isBugBot=${thread.isBugbot}`, - `bugbotReviewPasses=${thread.bugbotReviewPasses}`, + `reviewBot=${thread.reviewBot ?? "None"}`, + `reviewBotPasses=${thread.reviewBotPasses}`, (comment?.body ?? "").split(/\r?\n/, 1)[0]?.slice(0, 180) ?? "", ].join(" "); } diff --git a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/review-bots.json b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/review-bots.json new file mode 100644 index 000000000..c8f749723 --- /dev/null +++ b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/review-bots.json @@ -0,0 +1,30 @@ +{ + "bots": [ + { + "id": "bugbot", + "authors": [ + { "login": "bugbot", "match": "contains" }, + { + "login": "cursor", + "bodyTokens": [ + "bugbot", + "cursor_automation_id", + "agentic security review", + "description start", + "severity" + ] + } + ], + "passMarkers": [ + { "pattern": "RUN_ID:\\s*([a-zA-Z0-9_.:-]+)" }, + { "pattern": "CURSOR_AUTOMATION_ID:\\s*([a-zA-Z0-9_.:-]+)" } + ], + "runningChecks": [ + "bugbot", + "security review", + "pr review automation", + "review automation" + ] + } + ] +} diff --git a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/review-bots.test.ts b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/review-bots.test.ts new file mode 100644 index 000000000..9b2395f99 --- /dev/null +++ b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/review-bots.test.ts @@ -0,0 +1,238 @@ +import { describe, expect, it } from "bun:test"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { type CliRuntime, main } from "./cli.ts"; +import { fakeReader, passingCheck, pendingCheck } from "./fakes.test-helper.ts"; +import { buildReviewThreads } from "./github.ts"; +import { ReviewBotConfigError, parseReviewBots } from "./review-bots.ts"; +import type { GitHubReader, RawReviewThread, ReviewThread } from "./types.ts"; + +const thread = ( + id: string, + login: string, + body: string, + resolved = false +): RawReviewThread => ({ + id, + resolved, + firstComment: { + authorLogin: login, + body, + path: "a.ts", + line: 1, + createdAt: "now", + }, +}); +const shipped = JSON.parse( + readFileSync(new URL("./review-bots.json", import.meta.url), "utf8") +); +const withExampleBot = { + bots: [ + ...shipped.bots, + { + id: "example", + authors: [{ login: "example-reviewer[bot]" }], + passMarkers: [{ pattern: "" }], + runningChecks: ["Example Review"], + }, + ], +}; +const exampleBots = parseReviewBots(withExampleBot); +const passesByBot = (threads: readonly ReviewThread[]) => + Object.fromEntries( + threads.map((item) => [String(item.reviewBot), item.reviewBotPasses]) + ); + +describe("review-bot detection", () => { + const raw = [ + thread("b1", "cursor", "Bugbot RUN_ID: run-1"), + thread("e1", "example-reviewer", "\nFinding 1."), + thread("e2", "example-reviewer", "\nFinding 2."), + thread("e3", "example-reviewer", "\nOld.", true), + thread("p1", "a-person", "Please rename this."), + ]; + + it("ships a default config that recognises only Bugbot", () => { + expect( + buildReviewThreads(raw).map((item) => [item.id, item.reviewBot]) + ).toEqual([ + ["b1", "bugbot"], + ["e1", null], + ["e2", null], + ["p1", null], + ]); + }); + + it("names each thread's bot from the config", () => { + expect( + buildReviewThreads(raw, exampleBots).map((item) => item.reviewBot) + ).toEqual(["bugbot", "example", "example", null]); + }); + + it("counts passes per bot, including resolved threads", () => { + expect(passesByBot(buildReviewThreads(raw, exampleBots))).toEqual({ + bugbot: 1, + example: 2, + null: 0, + }); + }); + + it("counts one pass for a bot whose findings carry no pass marker", () => { + const keyless = [thread("e1", "example-reviewer", "Finding.")]; + expect(buildReviewThreads(keyless, exampleBots)[0]?.reviewBotPasses).toBe( + 1 + ); + }); +}); + +describe("--review-bots", () => { + const watch = ["--owner", "owner", "--repo", "repo", "--pr", "1"]; + function harness(reader: GitHubReader) { + const stdout: string[] = []; + const stderr: string[] = []; + let now = 0; + const runtime: CliRuntime = { + reader, + clock: { + now: () => (now += 10), + observedAt: () => "2026-11-02T00:00:00.000Z", + sleep: async () => {}, + }, + stdout: (value) => stdout.push(value), + stderr: (value) => stderr.push(value), + }; + return { + runtime, + stderr, + events: () => stdout.map((line) => JSON.parse(line)), + }; + } + function withConfigFile(value: unknown, use: (path: string) => R): R { + const dir = mkdtempSync(join(tmpdir(), "review-bots-")); + const path = join(dir, "review-bots.json"); + writeFileSync(path, JSON.stringify(value)); + try { + return use(path); + } finally { + rmSync(dir, { recursive: true }); + } + } + const exampleRunning = () => + fakeReader({ + fastPath: { + kind: "checks", + checks: [passingCheck(), pendingCheck("Example Review")], + }, + }); + + it("holds READY while a configured bot's check is running", async () => { + await withConfigFile(withExampleBot, async (path) => { + const run = harness(exampleRunning()); + expect( + await main( + [...watch, "--timeout", "5", "--review-bots", path], + run.runtime + ) + ).toBe(5); + expect(run.events().map((event) => event.kind)).toEqual([ + "WAITING", + "TIMEOUT", + ]); + }); + }); + + it("flags review automation from the configured check names", async () => { + const running = async (extra: readonly string[]) => { + const run = harness(exampleRunning()); + await main([...watch, "--status-only", ...extra], run.runtime); + return run.events()[0].rows[0].reviewAutomationRunning; + }; + expect(await running([])).toBe(false); + await withConfigFile(withExampleBot, async (path) => { + expect(await running(["--review-bots", path])).toBe(true); + }); + }); + + it("prints each thread's bot and its pass count", async () => { + await withConfigFile(withExampleBot, async (path) => { + const stdout: string[] = []; + const reader = fakeReader({ + threadPages: [ + { + threads: [ + thread("e1", "example-reviewer", ""), + ], + endCursor: null, + }, + ], + }); + const code = await main([...watch, "--pretty", "--review-bots", path], { + ...harness(reader).runtime, + stdout: (value) => stdout.push(value), + }); + expect(code).toBe(3); + expect(stdout.join("")).toContain( + "e1 a.ts 1 example-reviewer reviewBot=example reviewBotPasses=1" + ); + }); + }); + + it("rejects an unreadable or invalid file as a usage error", async () => { + await withConfigFile({ bots: [{ id: "x" }] }, async (invalid) => { + for (const path of [invalid, `${invalid}.missing`]) { + const reader = fakeReader(); + const run = harness(reader); + expect(await main([...watch, "--review-bots", path], run.runtime)).toBe( + 64 + ); + expect(run.stderr.join("")).toContain(path); + expect(reader.calls).toEqual([]); + } + }); + }); +}); + +describe("review-bot config", () => { + const bot = (fields: Record) => ({ + bots: [{ id: "x", authors: [{ login: "a" }], ...fields }], + }); + it.each([ + ["no bots list", {}], + ["a bot without an id", { bots: [{ authors: [{ login: "a" }] }] }], + ["a bot without authors", bot({ authors: [] })], + [ + "duplicate ids", + { + bots: [ + { id: "x", authors: [{ login: "a" }] }, + { id: "x", authors: [{ login: "b" }] }, + ], + }, + ], + ["an unknown login match", bot({ authors: [{ login: "a", match: "re" }] })], + [ + "a login that is only a [bot] suffix", + bot({ authors: [{ login: "[bot]" }] }), + ], + [ + "an empty body token", + bot({ authors: [{ login: "a", bodyTokens: [" "] }] }), + ], + [ + "a pass marker that is not a regex", + bot({ passMarkers: [{ pattern: "(" }] }), + ], + [ + "a pass marker without a capture group", + bot({ passMarkers: [{ pattern: "run" }] }), + ], + [ + "an unsupported pass marker key", + bot({ passMarkers: [{ key: "commit", pattern: "(x)" }] }), + ], + ["an empty running-check name", bot({ runningChecks: [""] })], + ])("rejects %s", (_label, value) => { + expect(() => parseReviewBots(value)).toThrow(ReviewBotConfigError); + }); +}); diff --git a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/review-bots.ts b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/review-bots.ts new file mode 100644 index 000000000..ef860e587 --- /dev/null +++ b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/review-bots.ts @@ -0,0 +1,171 @@ +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import type * as T from "./types.ts"; + +export interface ReviewBotAuthor { + readonly login: string; + readonly match: "exact" | "contains"; + readonly bodyTokens: readonly string[]; +} +export interface ReviewBot { + readonly id: string; + readonly authors: readonly ReviewBotAuthor[]; + readonly passMarkers: readonly RegExp[]; + readonly runningChecks: readonly string[]; +} +export type ReviewBots = readonly ReviewBot[]; + +export class ReviewBotConfigError extends Error { + constructor(detail: string) { + super(`invalid review-bot config: ${detail}`); + this.name = "ReviewBotConfigError"; + } +} +const normalizeLogin = (login: string): string => + login.toLowerCase().replace(/\[bot\]$/, ""); +function fail(detail: string): never { + throw new ReviewBotConfigError(detail); +} +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} +function nonEmptyString(value: unknown, path: string): string { + if (typeof value !== "string" || value.trim() === "") + fail(`${path} must be a non-empty string`); + return value; +} +function list(value: unknown, path: string): readonly unknown[] { + if (value === undefined) return []; + if (!Array.isArray(value)) fail(`${path} must be a list`); + return value; +} +const lowercaseStrings = (value: unknown, path: string): readonly string[] => + list(value, path).map((item, n) => + nonEmptyString(item, `${path}[${n}]`).toLowerCase() + ); +function parseAuthor(value: unknown, path: string): ReviewBotAuthor { + if (!isRecord(value)) fail(`${path} must be an object`); + const match = value.match ?? "exact"; + if (match !== "exact" && match !== "contains") + fail(`${path}.match must be "exact" or "contains"`); + const login = normalizeLogin(nonEmptyString(value.login, `${path}.login`)); + if (login === "") fail(`${path}.login must name an account`); + return { + login, + match, + bodyTokens: lowercaseStrings(value.bodyTokens, `${path}.bodyTokens`), + }; +} +function parsePassMarker(value: unknown, path: string): RegExp { + if (!isRecord(value)) fail(`${path} must be an object`); + if (value.key !== undefined) fail(`${path}.key is not supported`); + const source = nonEmptyString(value.pattern, `${path}.pattern`); + let pattern: RegExp; + try { + pattern = new RegExp(source); + } catch (error) { + return fail( + `${path}.pattern is not a regular expression: ${error instanceof Error ? error.message : String(error)}` + ); + } + // The empty alternative always matches, so the array length counts groups. + if ((new RegExp(`${source}|`).exec("")?.length ?? 0) < 2) + fail(`${path}.pattern needs a capture group for the pass key`); + return pattern; +} +export function parseReviewBots(value: unknown): ReviewBots { + if (!isRecord(value) || !Array.isArray(value.bots)) + fail("expected an object with a bots list"); + const ids = new Set(); + return value.bots.map((bot, index) => { + const path = `bots[${index}]`; + if (!isRecord(bot)) fail(`${path} must be an object`); + const id = nonEmptyString(bot.id, `${path}.id`); + if (/\s/.test(id)) fail(`${path}.id must not contain whitespace`); + if (ids.has(id)) fail(`${path}.id ${id} is a duplicate`); + ids.add(id); + const authors = list(bot.authors, `${path}.authors`); + if (authors.length === 0) fail(`${path}.authors must not be empty`); + return { + id, + authors: authors.map((author, n) => + parseAuthor(author, `${path}.authors[${n}]`) + ), + passMarkers: list(bot.passMarkers, `${path}.passMarkers`).map( + (marker, n) => parsePassMarker(marker, `${path}.passMarkers[${n}]`) + ), + runningChecks: lowercaseStrings( + bot.runningChecks, + `${path}.runningChecks` + ), + }; + }); +} +export function loadReviewBots(path: string): ReviewBots { + let text: string; + try { + text = readFileSync(path, "utf8"); + } catch (error) { + return fail( + `cannot read ${path}: ${error instanceof Error ? error.message : String(error)}` + ); + } + let value: unknown; + try { + value = JSON.parse(text); + } catch (error) { + return fail( + `${path} is not JSON: ${error instanceof Error ? error.message : String(error)}` + ); + } + return parseReviewBots(value); +} +export const DEFAULT_REVIEW_BOTS: ReviewBots = loadReviewBots( + fileURLToPath(new URL("./review-bots.json", import.meta.url)) +); + +export const isReviewBotCheck = (name: string, bots: ReviewBots): boolean => + bots.some((bot) => + bot.runningChecks.some((token) => name.toLowerCase().includes(token)) + ); +export function reviewBotFor( + comment: T.ReviewComment | null, + bots: ReviewBots +): ReviewBot | null { + if (comment === null) return null; + const login = normalizeLogin(comment.authorLogin ?? ""); + const body = comment.body.toLowerCase(); + const wrote = (author: ReviewBotAuthor): boolean => + (author.match === "contains" + ? login.includes(author.login) + : login === author.login) && + (author.bodyTokens.length === 0 || + author.bodyTokens.some((token) => body.includes(token))); + return bots.find((bot) => bot.authors.some(wrote)) ?? null; +} +function passKey(bot: ReviewBot, comment: T.ReviewComment): string | null { + for (const pattern of bot.passMarkers) { + const key = pattern.exec(comment.body)?.[1]; + if (key) return key; + } + return null; +} +/** + * Review passes per bot id, over every thread including resolved ones: the + * number of distinct pass keys, or 1 when the bot commented without any. + */ +export function countReviewPasses( + comments: readonly (T.ReviewComment | null)[], + bots: ReviewBots +): ReadonlyMap { + const keys = new Map>(); + for (const comment of comments) { + const bot = reviewBotFor(comment, bots); + if (bot === null || comment === null) continue; + const seen = keys.get(bot.id) ?? new Set(); + keys.set(bot.id, seen); + const key = passKey(bot, comment); + if (key !== null) seen.add(key); + } + return new Map([...keys].map(([id, seen]) => [id, Math.max(seen.size, 1)])); +} diff --git a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/types.ts b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/types.ts index 60b9abcac..d58716264 100644 --- a/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/types.ts +++ b/harness/pstack/claude-code/skills/poteto-mode/scripts/watch-pr/types.ts @@ -66,8 +66,8 @@ export interface ReviewComment { export interface ReviewThread { readonly id: string; readonly firstComment: ReviewComment | null; - readonly isBugbot: boolean; - readonly bugbotReviewPasses: number; + readonly reviewBot: string | null; + readonly reviewBotPasses: number; } interface CheckDetails { readonly name: string; diff --git a/pstack/skills/poteto-mode/playbooks/babysit.md b/pstack/skills/poteto-mode/playbooks/babysit.md index 14d5783d4..436a6997f 100644 --- a/pstack/skills/poteto-mode/playbooks/babysit.md +++ b/pstack/skills/poteto-mode/playbooks/babysit.md @@ -17,7 +17,7 @@ Babysitting starts when the user asks for it, which is normally once a phase or Answer a user question mid-loop and continue. Only an explicit stop ends the loop before the active forge's stop condition. On GitHub, that is `READY` in single or stack mode, or a `WAITING`/`merge-queue` report or `COMPLETE` in queued mode. On Origin, that is the merge-ready state defined above. For a GitHub queued stack, capture the PR list bottom-to-top once and pass the same frozen list to every rearm. Revise the list only for the sanctioned follow-up PR from step 4. Append it at the end, drop the merged owner, and rearm with the corrected snapshot. 7. **Classify CI before any retrigger.** Flake or infrastructure earns one fresh build, never a job retry. One retry only. An identical second failure means it was never flake, so reclassify and read the child logs instead of retrying blind. A failure in code the diff never touches means a stale base, so check with `git merge-base --is-ancestor` before assuming flake. Report a stale base as needing a rebase instead of burning retries. Only a failure in the diff's own code gets a commit. -8. **Bugbot is triaged skeptically, always.** Verify each claim against the code per `../references/bugbot-triage.md`. Fix real findings with a red-first proof in the lowest PR that owns the code, never at the tip unless the owning PR has merged. In that case, use step 4's sanctioned follow-up PR. Per step 2, upstack fixes wait for step 5's next frontier-driven push wave. Push that wave before replying so the reply cites the commit. On Origin, reply with `origin pr thread reply --body-file `. On GitHub, call `gh api --method POST "repos///pulls//comments//replies" --input ` and put the reply body in the JSON file as data. Never interpolate comment text or a reply into a shell command. Dismiss noise with the concrete disproof on the thread. On GitHub, use the watcher's Bugbot pass count. On Origin, derive the pass count from `origin pr thread list` and the review history. From the third pass on, lean toward dismissing documented patterns, still escalating anything touching security, auth, billing, data, or migrations rather than dismissing it yourself. Never churn code to quiet a bot. +8. **Bugbot is triaged skeptically, always.** Verify each claim against the code per `../references/bugbot-triage.md`. Fix real findings with a red-first proof in the lowest PR that owns the code, never at the tip unless the owning PR has merged. In that case, use step 4's sanctioned follow-up PR. Per step 2, upstack fixes wait for step 5's next frontier-driven push wave. Push that wave before replying so the reply cites the commit. On Origin, reply with `origin pr thread reply --body-file `. On GitHub, call `gh api --method POST "repos///pulls//comments//replies" --input ` and put the reply body in the JSON file as data. Never interpolate comment text or a reply into a shell command. Dismiss noise with the concrete disproof on the thread. On GitHub, use the watcher's Bugbot pass count, each thread's `reviewBotPasses`. On Origin, derive the pass count from `origin pr thread list` and the review history. From the third pass on, lean toward dismissing documented patterns, still escalating anything touching security, auth, billing, data, or migrations rather than dismissing it yourself. Never churn code to quiet a bot. 9. **Stop at the human's line.** Owner approval is a wait, not a blocker to fix. Babysitting never authorizes merging. Only an explicit request to merge, land, ship, or merge when ready does. Route that request to Shipping. Surface the escalation and keep working the rest. After GitHub reports `READY`, a queued `WAITING`/`merge-queue` stop, or `COMPLETE`, or after Origin reports the frontier merge-ready, sweep the run's triage decisions once. Offer any team-useful dismissal pattern as a candidate entry in the shared rubric (`../references/bugbot-triage.md`) and its own PR. Never keep it only in private memory. `drive` ends at merge-ready. Landing the stack is `playbooks/shipping.md`. diff --git a/pstack/skills/poteto-mode/scripts/watch-pr/cli.ts b/pstack/skills/poteto-mode/scripts/watch-pr/cli.ts index ec8a2f27c..e8974c895 100644 --- a/pstack/skills/poteto-mode/scripts/watch-pr/cli.ts +++ b/pstack/skills/poteto-mode/scripts/watch-pr/cli.ts @@ -19,6 +19,12 @@ import { type WatchClock, } from "./policy.ts"; import { renderJson, renderPretty } from "./render.ts"; +import { + DEFAULT_REVIEW_BOTS, + ReviewBotConfigError, + loadReviewBots, + type ReviewBots, +} from "./review-bots.ts"; import type * as T from "./types.ts"; import { nonEmpty, parsePrNumber } from "./types.ts"; export interface CliOptions { @@ -30,6 +36,7 @@ export interface CliOptions { readonly statusOnly: boolean; readonly pretty: boolean; readonly polling: T.PollingOptions; + readonly reviewBots: ReviewBots; } function positiveNumber(value: string): number { const parsed = Number(value); @@ -56,6 +63,14 @@ function prNumber(value: string): T.PrNumber { throw new InvalidArgumentError("must be a positive integer"); } } +function reviewBotFile(value: string): ReviewBots { + try { + return loadReviewBots(value); + } catch (error) { + if (!(error instanceof ReviewBotConfigError)) throw error; + throw new InvalidArgumentError(error.message); + } +} function stackPrList(value: string): T.NonEmpty { const numbers = value.split(",").map((part) => prNumber(part.trim())); if (new Set(numbers).size !== numbers.length) @@ -78,6 +93,7 @@ interface RawOptions { readonly statusOnly: boolean; readonly allowDraft: boolean; readonly pretty: boolean; + readonly reviewBots?: ReviewBots; } export function parseArgs( argv: readonly string[], @@ -128,7 +144,12 @@ export function parseArgs( ) .option("--status-only", "print one status table and exit 0", false) .option("--allow-draft", "do not treat a draft as a merge gate", false) - .option("--pretty", "render human text instead of JSON", false); + .option("--pretty", "render human text instead of JSON", false) + .option( + "--review-bots ", + "review-bot config JSON (default: review-bots.json beside this script)", + reviewBotFile + ); program.parse(argv, { from: "user" }); const raw = program.opts(); if (raw.stackPrs !== undefined && !raw.queuedStack) @@ -148,6 +169,7 @@ export function parseArgs( maxQueryErrors: raw.maxQueryErrors, allowDraft: raw.allowDraft, }, + reviewBots: raw.reviewBots ?? DEFAULT_REVIEW_BOTS, }; } export interface CliRuntime { @@ -207,7 +229,12 @@ export async function main( runtime.stdout(render(verdict)); return verdict.exitCode; } - const dependencies = { reader: runtime.reader, clock: runtime.clock, emit }; + const dependencies = { + reader: runtime.reader, + clock: runtime.clock, + emit, + reviewBots: options.reviewBots, + }; const verdict = options.mode === "queued-stack" && !options.statusOnly ? await runQueued({ dependencies, contexts, options: options.polling }) diff --git a/pstack/skills/poteto-mode/scripts/watch-pr/github.test.ts b/pstack/skills/poteto-mode/scripts/watch-pr/github.test.ts index 4e5893838..66ac40bb5 100644 --- a/pstack/skills/poteto-mode/scripts/watch-pr/github.test.ts +++ b/pstack/skills/poteto-mode/scripts/watch-pr/github.test.ts @@ -291,8 +291,11 @@ it("annotates Bugbot threads with distinct review-pass counts", () => { }; const threads = buildReviewThreads(parseReviewThreadPage(response).threads); expect(threads).toHaveLength(2); - expect(threads.map((thread) => thread.isBugbot)).toEqual([true, true]); - expect(threads.map((thread) => thread.bugbotReviewPasses)).toEqual([3, 3]); + expect(threads.map((thread) => thread.reviewBot)).toEqual([ + "bugbot", + "bugbot", + ]); + expect(threads.map((thread) => thread.reviewBotPasses)).toEqual([3, 3]); }); describe("review thread pagination", () => { @@ -363,7 +366,7 @@ describe("review thread pagination", () => { ], }); const threads = await resolveReviewThreads(reader, context); - expect(threads.map((item) => item.bugbotReviewPasses)).toEqual([2, 2]); + expect(threads.map((item) => item.reviewBotPasses)).toEqual([2, 2]); }); it("drops resolved threads from later pages too", async () => { diff --git a/pstack/skills/poteto-mode/scripts/watch-pr/github.ts b/pstack/skills/poteto-mode/scripts/watch-pr/github.ts index 123a33b19..1327472ca 100644 --- a/pstack/skills/poteto-mode/scripts/watch-pr/github.ts +++ b/pstack/skills/poteto-mode/scripts/watch-pr/github.ts @@ -1,4 +1,10 @@ import { spawn } from "node:child_process"; +import { + DEFAULT_REVIEW_BOTS, + countReviewPasses, + reviewBotFor, + type ReviewBots, +} from "./review-bots.ts"; import type * as T from "./types.ts"; import { nonEmpty, parsePrNumber } from "./types.ts"; export const REVIEW_THREADS_QUERY = @@ -326,33 +332,6 @@ function parseComment(value: unknown): T.ReviewComment { createdAt: string(object.createdAt, "review comment.createdAt"), }; } -function isBugbot(comment: T.ReviewComment | null): boolean { - if (comment === null) return false; - const author = (comment.authorLogin ?? "").toLowerCase(); - const body = comment.body.toLowerCase(); - return ( - author.includes("bugbot") || - (author === "cursor" && - [ - "bugbot", - "cursor_automation_id", - "agentic security review", - "description start", - "severity", - ].some((token) => body.includes(token))) - ); -} -function passKey(comment: T.ReviewComment | null): string | null { - if (comment === null) return null; - for (const pattern of [ - /RUN_ID:\s*([a-zA-Z0-9_.:-]+)/, - /CURSOR_AUTOMATION_ID:\s*([a-zA-Z0-9_.:-]+)/, - ]) { - const match = pattern.exec(comment.body); - if (match?.[1]) return match[1]; - } - return null; -} export function parseReviewThreadPage(value: unknown): T.ReviewThreadPage { const reviewThreads = record( at(value, ["data", "repository", "pullRequest", "reviewThreads"]), @@ -384,25 +363,24 @@ export function parseReviewThreadPage(value: unknown): T.ReviewThreadPage { return { threads, endCursor: page.hasNextPage && cursor ? cursor : null }; } export function buildReviewThreads( - threads: readonly T.RawReviewThread[] + threads: readonly T.RawReviewThread[], + bots: ReviewBots = DEFAULT_REVIEW_BOTS ): readonly T.ReviewThread[] { - const keys = new Set(); - let keyless = false; - for (const thread of threads) { - if (!isBugbot(thread.firstComment)) continue; - const key = passKey(thread.firstComment); - if (key === null) keyless = true; - else keys.add(key); - } - const passes = keys.size > 0 ? keys.size : keyless ? 1 : 0; + const passes = countReviewPasses( + threads.map((thread) => thread.firstComment), + bots + ); return threads .filter((thread) => !thread.resolved) - .map(({ id, firstComment }) => ({ - id, - firstComment, - isBugbot: isBugbot(firstComment), - bugbotReviewPasses: passes, - })); + .map(({ id, firstComment }) => { + const reviewBot = reviewBotFor(firstComment, bots)?.id ?? null; + return { + id, + firstComment, + reviewBot, + reviewBotPasses: reviewBot === null ? 0 : (passes.get(reviewBot) ?? 0), + }; + }); } export function parsePullRequest( value: unknown, @@ -634,7 +612,8 @@ export async function resolveChecks( } export async function resolveReviewThreads( reader: T.GitHubReader, - context: T.PrContext + context: T.PrContext, + bots: ReviewBots = DEFAULT_REVIEW_BOTS ): Promise { const threads: T.RawReviewThread[] = []; let after: string | null = null; @@ -643,7 +622,7 @@ export async function resolveReviewThreads( threads.push(...page.threads); after = page.endCursor; } while (after !== null); - return buildReviewThreads(threads); + return buildReviewThreads(threads, bots); } export async function resolveContext(args: { readonly reader: T.GitHubReader; diff --git a/pstack/skills/poteto-mode/scripts/watch-pr/policy.ts b/pstack/skills/poteto-mode/scripts/watch-pr/policy.ts index 8179f2d8b..c5ed0b1fc 100644 --- a/pstack/skills/poteto-mode/scripts/watch-pr/policy.ts +++ b/pstack/skills/poteto-mode/scripts/watch-pr/policy.ts @@ -3,6 +3,11 @@ import { resolveChecks, resolveReviewThreads, } from "./github.ts"; +import { + DEFAULT_REVIEW_BOTS, + isReviewBotCheck, + type ReviewBots, +} from "./review-bots.ts"; import type * as T from "./types.ts"; import { nonEmpty } from "./types.ts"; export function assessGitHubMerge(args: { @@ -50,24 +55,20 @@ async function mergeAssessment( }), }; } -const AUTOMATION_TOKENS = [ - "bugbot", - "security review", - "pr review automation", - "review automation", -] as const; export async function readSnapshot(args: { readonly reader: T.GitHubReader; readonly context: T.PrContext; readonly pendingHistory: "include" | "omit"; readonly allowDraft: boolean; + readonly reviewBots?: ReviewBots; }): Promise { + const bots = args.reviewBots ?? DEFAULT_REVIEW_BOTS; const facts = await args.reader.pullRequest(args.context); if (facts.state === "MERGED" || facts.mergedAt !== null) return { kind: "merged", context: args.context, facts }; if (facts.state === "CLOSED") return { kind: "closed", context: args.context, facts }; - const threads = await resolveReviewThreads(args.reader, args.context); + const threads = await resolveReviewThreads(args.reader, args.context, bots); const checks = await resolveChecks(args.reader, args.context); const failed = nonEmpty( checks.checks.filter( @@ -130,11 +131,7 @@ export async function readSnapshot(args: { threads, ci, reviewAutomationRunning: checks.checks.some( - (check) => - check.kind === "pending" && - AUTOMATION_TOKENS.some((token) => - check.name.toLowerCase().includes(token) - ) + (check) => check.kind === "pending" && isReviewBotCheck(check.name, bots) ), }; } @@ -348,6 +345,7 @@ export interface RunDependencies { readonly reader: T.GitHubReader; readonly clock: WatchClock; readonly emit: (verdict: T.ProgressVerdict) => void; + readonly reviewBots?: ReviewBots; } const deadlinePassed = ( started: number, @@ -437,6 +435,7 @@ export async function runSimple(args: { context, pendingHistory: "include", allowDraft: args.options.allowDraft, + reviewBots: args.dependencies.reviewBots, }) ); const complete = nonEmpty(rows); @@ -750,6 +749,7 @@ export async function runQueued(args: { context, pendingHistory: "omit", allowDraft: args.options.allowDraft, + reviewBots: args.dependencies.reviewBots, }); const applied = applyQueueSnapshot( state, diff --git a/pstack/skills/poteto-mode/scripts/watch-pr/render.ts b/pstack/skills/poteto-mode/scripts/watch-pr/render.ts index 0e9358393..1d4e752fc 100644 --- a/pstack/skills/poteto-mode/scripts/watch-pr/render.ts +++ b/pstack/skills/poteto-mode/scripts/watch-pr/render.ts @@ -59,8 +59,8 @@ function threadLine(thread: T.ReviewThread): string { comment?.path ?? "None", comment?.line ?? "None", comment?.authorLogin ?? "None", - `isBugBot=${thread.isBugbot}`, - `bugbotReviewPasses=${thread.bugbotReviewPasses}`, + `reviewBot=${thread.reviewBot ?? "None"}`, + `reviewBotPasses=${thread.reviewBotPasses}`, (comment?.body ?? "").split(/\r?\n/, 1)[0]?.slice(0, 180) ?? "", ].join(" "); } diff --git a/pstack/skills/poteto-mode/scripts/watch-pr/review-bots.json b/pstack/skills/poteto-mode/scripts/watch-pr/review-bots.json new file mode 100644 index 000000000..c8f749723 --- /dev/null +++ b/pstack/skills/poteto-mode/scripts/watch-pr/review-bots.json @@ -0,0 +1,30 @@ +{ + "bots": [ + { + "id": "bugbot", + "authors": [ + { "login": "bugbot", "match": "contains" }, + { + "login": "cursor", + "bodyTokens": [ + "bugbot", + "cursor_automation_id", + "agentic security review", + "description start", + "severity" + ] + } + ], + "passMarkers": [ + { "pattern": "RUN_ID:\\s*([a-zA-Z0-9_.:-]+)" }, + { "pattern": "CURSOR_AUTOMATION_ID:\\s*([a-zA-Z0-9_.:-]+)" } + ], + "runningChecks": [ + "bugbot", + "security review", + "pr review automation", + "review automation" + ] + } + ] +} diff --git a/pstack/skills/poteto-mode/scripts/watch-pr/review-bots.test.ts b/pstack/skills/poteto-mode/scripts/watch-pr/review-bots.test.ts new file mode 100644 index 000000000..9b2395f99 --- /dev/null +++ b/pstack/skills/poteto-mode/scripts/watch-pr/review-bots.test.ts @@ -0,0 +1,238 @@ +import { describe, expect, it } from "bun:test"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { type CliRuntime, main } from "./cli.ts"; +import { fakeReader, passingCheck, pendingCheck } from "./fakes.test-helper.ts"; +import { buildReviewThreads } from "./github.ts"; +import { ReviewBotConfigError, parseReviewBots } from "./review-bots.ts"; +import type { GitHubReader, RawReviewThread, ReviewThread } from "./types.ts"; + +const thread = ( + id: string, + login: string, + body: string, + resolved = false +): RawReviewThread => ({ + id, + resolved, + firstComment: { + authorLogin: login, + body, + path: "a.ts", + line: 1, + createdAt: "now", + }, +}); +const shipped = JSON.parse( + readFileSync(new URL("./review-bots.json", import.meta.url), "utf8") +); +const withExampleBot = { + bots: [ + ...shipped.bots, + { + id: "example", + authors: [{ login: "example-reviewer[bot]" }], + passMarkers: [{ pattern: "" }], + runningChecks: ["Example Review"], + }, + ], +}; +const exampleBots = parseReviewBots(withExampleBot); +const passesByBot = (threads: readonly ReviewThread[]) => + Object.fromEntries( + threads.map((item) => [String(item.reviewBot), item.reviewBotPasses]) + ); + +describe("review-bot detection", () => { + const raw = [ + thread("b1", "cursor", "Bugbot RUN_ID: run-1"), + thread("e1", "example-reviewer", "\nFinding 1."), + thread("e2", "example-reviewer", "\nFinding 2."), + thread("e3", "example-reviewer", "\nOld.", true), + thread("p1", "a-person", "Please rename this."), + ]; + + it("ships a default config that recognises only Bugbot", () => { + expect( + buildReviewThreads(raw).map((item) => [item.id, item.reviewBot]) + ).toEqual([ + ["b1", "bugbot"], + ["e1", null], + ["e2", null], + ["p1", null], + ]); + }); + + it("names each thread's bot from the config", () => { + expect( + buildReviewThreads(raw, exampleBots).map((item) => item.reviewBot) + ).toEqual(["bugbot", "example", "example", null]); + }); + + it("counts passes per bot, including resolved threads", () => { + expect(passesByBot(buildReviewThreads(raw, exampleBots))).toEqual({ + bugbot: 1, + example: 2, + null: 0, + }); + }); + + it("counts one pass for a bot whose findings carry no pass marker", () => { + const keyless = [thread("e1", "example-reviewer", "Finding.")]; + expect(buildReviewThreads(keyless, exampleBots)[0]?.reviewBotPasses).toBe( + 1 + ); + }); +}); + +describe("--review-bots", () => { + const watch = ["--owner", "owner", "--repo", "repo", "--pr", "1"]; + function harness(reader: GitHubReader) { + const stdout: string[] = []; + const stderr: string[] = []; + let now = 0; + const runtime: CliRuntime = { + reader, + clock: { + now: () => (now += 10), + observedAt: () => "2026-11-02T00:00:00.000Z", + sleep: async () => {}, + }, + stdout: (value) => stdout.push(value), + stderr: (value) => stderr.push(value), + }; + return { + runtime, + stderr, + events: () => stdout.map((line) => JSON.parse(line)), + }; + } + function withConfigFile(value: unknown, use: (path: string) => R): R { + const dir = mkdtempSync(join(tmpdir(), "review-bots-")); + const path = join(dir, "review-bots.json"); + writeFileSync(path, JSON.stringify(value)); + try { + return use(path); + } finally { + rmSync(dir, { recursive: true }); + } + } + const exampleRunning = () => + fakeReader({ + fastPath: { + kind: "checks", + checks: [passingCheck(), pendingCheck("Example Review")], + }, + }); + + it("holds READY while a configured bot's check is running", async () => { + await withConfigFile(withExampleBot, async (path) => { + const run = harness(exampleRunning()); + expect( + await main( + [...watch, "--timeout", "5", "--review-bots", path], + run.runtime + ) + ).toBe(5); + expect(run.events().map((event) => event.kind)).toEqual([ + "WAITING", + "TIMEOUT", + ]); + }); + }); + + it("flags review automation from the configured check names", async () => { + const running = async (extra: readonly string[]) => { + const run = harness(exampleRunning()); + await main([...watch, "--status-only", ...extra], run.runtime); + return run.events()[0].rows[0].reviewAutomationRunning; + }; + expect(await running([])).toBe(false); + await withConfigFile(withExampleBot, async (path) => { + expect(await running(["--review-bots", path])).toBe(true); + }); + }); + + it("prints each thread's bot and its pass count", async () => { + await withConfigFile(withExampleBot, async (path) => { + const stdout: string[] = []; + const reader = fakeReader({ + threadPages: [ + { + threads: [ + thread("e1", "example-reviewer", ""), + ], + endCursor: null, + }, + ], + }); + const code = await main([...watch, "--pretty", "--review-bots", path], { + ...harness(reader).runtime, + stdout: (value) => stdout.push(value), + }); + expect(code).toBe(3); + expect(stdout.join("")).toContain( + "e1 a.ts 1 example-reviewer reviewBot=example reviewBotPasses=1" + ); + }); + }); + + it("rejects an unreadable or invalid file as a usage error", async () => { + await withConfigFile({ bots: [{ id: "x" }] }, async (invalid) => { + for (const path of [invalid, `${invalid}.missing`]) { + const reader = fakeReader(); + const run = harness(reader); + expect(await main([...watch, "--review-bots", path], run.runtime)).toBe( + 64 + ); + expect(run.stderr.join("")).toContain(path); + expect(reader.calls).toEqual([]); + } + }); + }); +}); + +describe("review-bot config", () => { + const bot = (fields: Record) => ({ + bots: [{ id: "x", authors: [{ login: "a" }], ...fields }], + }); + it.each([ + ["no bots list", {}], + ["a bot without an id", { bots: [{ authors: [{ login: "a" }] }] }], + ["a bot without authors", bot({ authors: [] })], + [ + "duplicate ids", + { + bots: [ + { id: "x", authors: [{ login: "a" }] }, + { id: "x", authors: [{ login: "b" }] }, + ], + }, + ], + ["an unknown login match", bot({ authors: [{ login: "a", match: "re" }] })], + [ + "a login that is only a [bot] suffix", + bot({ authors: [{ login: "[bot]" }] }), + ], + [ + "an empty body token", + bot({ authors: [{ login: "a", bodyTokens: [" "] }] }), + ], + [ + "a pass marker that is not a regex", + bot({ passMarkers: [{ pattern: "(" }] }), + ], + [ + "a pass marker without a capture group", + bot({ passMarkers: [{ pattern: "run" }] }), + ], + [ + "an unsupported pass marker key", + bot({ passMarkers: [{ key: "commit", pattern: "(x)" }] }), + ], + ["an empty running-check name", bot({ runningChecks: [""] })], + ])("rejects %s", (_label, value) => { + expect(() => parseReviewBots(value)).toThrow(ReviewBotConfigError); + }); +}); diff --git a/pstack/skills/poteto-mode/scripts/watch-pr/review-bots.ts b/pstack/skills/poteto-mode/scripts/watch-pr/review-bots.ts new file mode 100644 index 000000000..ef860e587 --- /dev/null +++ b/pstack/skills/poteto-mode/scripts/watch-pr/review-bots.ts @@ -0,0 +1,171 @@ +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import type * as T from "./types.ts"; + +export interface ReviewBotAuthor { + readonly login: string; + readonly match: "exact" | "contains"; + readonly bodyTokens: readonly string[]; +} +export interface ReviewBot { + readonly id: string; + readonly authors: readonly ReviewBotAuthor[]; + readonly passMarkers: readonly RegExp[]; + readonly runningChecks: readonly string[]; +} +export type ReviewBots = readonly ReviewBot[]; + +export class ReviewBotConfigError extends Error { + constructor(detail: string) { + super(`invalid review-bot config: ${detail}`); + this.name = "ReviewBotConfigError"; + } +} +const normalizeLogin = (login: string): string => + login.toLowerCase().replace(/\[bot\]$/, ""); +function fail(detail: string): never { + throw new ReviewBotConfigError(detail); +} +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} +function nonEmptyString(value: unknown, path: string): string { + if (typeof value !== "string" || value.trim() === "") + fail(`${path} must be a non-empty string`); + return value; +} +function list(value: unknown, path: string): readonly unknown[] { + if (value === undefined) return []; + if (!Array.isArray(value)) fail(`${path} must be a list`); + return value; +} +const lowercaseStrings = (value: unknown, path: string): readonly string[] => + list(value, path).map((item, n) => + nonEmptyString(item, `${path}[${n}]`).toLowerCase() + ); +function parseAuthor(value: unknown, path: string): ReviewBotAuthor { + if (!isRecord(value)) fail(`${path} must be an object`); + const match = value.match ?? "exact"; + if (match !== "exact" && match !== "contains") + fail(`${path}.match must be "exact" or "contains"`); + const login = normalizeLogin(nonEmptyString(value.login, `${path}.login`)); + if (login === "") fail(`${path}.login must name an account`); + return { + login, + match, + bodyTokens: lowercaseStrings(value.bodyTokens, `${path}.bodyTokens`), + }; +} +function parsePassMarker(value: unknown, path: string): RegExp { + if (!isRecord(value)) fail(`${path} must be an object`); + if (value.key !== undefined) fail(`${path}.key is not supported`); + const source = nonEmptyString(value.pattern, `${path}.pattern`); + let pattern: RegExp; + try { + pattern = new RegExp(source); + } catch (error) { + return fail( + `${path}.pattern is not a regular expression: ${error instanceof Error ? error.message : String(error)}` + ); + } + // The empty alternative always matches, so the array length counts groups. + if ((new RegExp(`${source}|`).exec("")?.length ?? 0) < 2) + fail(`${path}.pattern needs a capture group for the pass key`); + return pattern; +} +export function parseReviewBots(value: unknown): ReviewBots { + if (!isRecord(value) || !Array.isArray(value.bots)) + fail("expected an object with a bots list"); + const ids = new Set(); + return value.bots.map((bot, index) => { + const path = `bots[${index}]`; + if (!isRecord(bot)) fail(`${path} must be an object`); + const id = nonEmptyString(bot.id, `${path}.id`); + if (/\s/.test(id)) fail(`${path}.id must not contain whitespace`); + if (ids.has(id)) fail(`${path}.id ${id} is a duplicate`); + ids.add(id); + const authors = list(bot.authors, `${path}.authors`); + if (authors.length === 0) fail(`${path}.authors must not be empty`); + return { + id, + authors: authors.map((author, n) => + parseAuthor(author, `${path}.authors[${n}]`) + ), + passMarkers: list(bot.passMarkers, `${path}.passMarkers`).map( + (marker, n) => parsePassMarker(marker, `${path}.passMarkers[${n}]`) + ), + runningChecks: lowercaseStrings( + bot.runningChecks, + `${path}.runningChecks` + ), + }; + }); +} +export function loadReviewBots(path: string): ReviewBots { + let text: string; + try { + text = readFileSync(path, "utf8"); + } catch (error) { + return fail( + `cannot read ${path}: ${error instanceof Error ? error.message : String(error)}` + ); + } + let value: unknown; + try { + value = JSON.parse(text); + } catch (error) { + return fail( + `${path} is not JSON: ${error instanceof Error ? error.message : String(error)}` + ); + } + return parseReviewBots(value); +} +export const DEFAULT_REVIEW_BOTS: ReviewBots = loadReviewBots( + fileURLToPath(new URL("./review-bots.json", import.meta.url)) +); + +export const isReviewBotCheck = (name: string, bots: ReviewBots): boolean => + bots.some((bot) => + bot.runningChecks.some((token) => name.toLowerCase().includes(token)) + ); +export function reviewBotFor( + comment: T.ReviewComment | null, + bots: ReviewBots +): ReviewBot | null { + if (comment === null) return null; + const login = normalizeLogin(comment.authorLogin ?? ""); + const body = comment.body.toLowerCase(); + const wrote = (author: ReviewBotAuthor): boolean => + (author.match === "contains" + ? login.includes(author.login) + : login === author.login) && + (author.bodyTokens.length === 0 || + author.bodyTokens.some((token) => body.includes(token))); + return bots.find((bot) => bot.authors.some(wrote)) ?? null; +} +function passKey(bot: ReviewBot, comment: T.ReviewComment): string | null { + for (const pattern of bot.passMarkers) { + const key = pattern.exec(comment.body)?.[1]; + if (key) return key; + } + return null; +} +/** + * Review passes per bot id, over every thread including resolved ones: the + * number of distinct pass keys, or 1 when the bot commented without any. + */ +export function countReviewPasses( + comments: readonly (T.ReviewComment | null)[], + bots: ReviewBots +): ReadonlyMap { + const keys = new Map>(); + for (const comment of comments) { + const bot = reviewBotFor(comment, bots); + if (bot === null || comment === null) continue; + const seen = keys.get(bot.id) ?? new Set(); + keys.set(bot.id, seen); + const key = passKey(bot, comment); + if (key !== null) seen.add(key); + } + return new Map([...keys].map(([id, seen]) => [id, Math.max(seen.size, 1)])); +} diff --git a/pstack/skills/poteto-mode/scripts/watch-pr/types.ts b/pstack/skills/poteto-mode/scripts/watch-pr/types.ts index 60b9abcac..d58716264 100644 --- a/pstack/skills/poteto-mode/scripts/watch-pr/types.ts +++ b/pstack/skills/poteto-mode/scripts/watch-pr/types.ts @@ -66,8 +66,8 @@ export interface ReviewComment { export interface ReviewThread { readonly id: string; readonly firstComment: ReviewComment | null; - readonly isBugbot: boolean; - readonly bugbotReviewPasses: number; + readonly reviewBot: string | null; + readonly reviewBotPasses: number; } interface CheckDetails { readonly name: string;