diff --git a/README.md b/README.md index 0286622..3a7a053 100644 --- a/README.md +++ b/README.md @@ -154,10 +154,15 @@ Repo → **Settings** → **Webhooks** → **Add webhook** | Content type | **`application/json`** (not `x-www-form-urlencoded`) | | Secret | Copy from Hare Settings for that watched repo | | SSL | Enable | -| Events | **Let me select individual events** → **Pull requests** only | +| Events | **Let me select individual events** → **Pull requests** and **Issue comments** | | Active | On | -`Just the push event` is ignored. Hare only handles `pull_request` actions: `opened`, `synchronize`, `reopened`, `ready_for_review`. +`Just the push event` is ignored. Hare handles: + +- `pull_request`: `opened`, `synchronize`, `reopened`, `ready_for_review` +- `issue_comment` / `pull_request_review_comment`: a comment that mentions **`@hare-bot`** (re-review, `force`). Comments from `hare-bot` itself are ignored so reviews do not loop. + +On GitHub, comment `@hare-bot please re-review` on the PR Conversation tab to run again on the current HEAD. After save, GitHub sends a `ping`. A green check means the secret and URL are correct. @@ -243,7 +248,7 @@ src/ engine.ts Orchestrates load → review → post → status reviewer.ts Prompt, context paths, accuracy filters github.ts GitHub REST (PRs, files, reviews, statuses) - webhook.ts HMAC verification + pull_request dispatch + webhook.ts HMAC + pull_request + @hare-bot comment dispatch format.ts Review markdown, Hare gate, REQUEST_CHANGES db.ts Watched repos, PRs, reviews, findings routes/ diff --git a/package.json b/package.json index d70f384..87c5752 100644 --- a/package.json +++ b/package.json @@ -26,8 +26,8 @@ "preview:stop": "node scripts/preview.mjs stop", "typecheck": "tsc --noEmit", "check:auth": "node scripts/check-auth-invariant.mjs", - "test": "node --test 'scripts/**/*.test.mjs' && node --experimental-strip-types --test src/lib/app-data/app-data.test.ts src/lib/app-data/readiness-schedule.test.ts src/lib/auth/gate-identity.test.ts src/lib/auth/sign-in-gate.test.ts src/lib/hare/migrations.test.ts src/lib/hare/queue.test.ts", - "test:ci": "node --experimental-strip-types --test src/lib/app-data/app-data.test.ts src/lib/app-data/readiness-schedule.test.ts src/lib/auth/gate-identity.test.ts src/lib/auth/sign-in-gate.test.ts src/lib/hare/migrations.test.ts src/lib/hare/queue.test.ts", + "test": "node --test 'scripts/**/*.test.mjs' && node --experimental-strip-types --test src/lib/app-data/app-data.test.ts src/lib/app-data/readiness-schedule.test.ts src/lib/auth/gate-identity.test.ts src/lib/auth/sign-in-gate.test.ts src/lib/hare/migrations.test.ts src/lib/hare/queue.test.ts src/lib/hare/webhook.test.ts", + "test:ci": "node --experimental-strip-types --test src/lib/app-data/app-data.test.ts src/lib/app-data/readiness-schedule.test.ts src/lib/auth/gate-identity.test.ts src/lib/auth/sign-in-gate.test.ts src/lib/hare/migrations.test.ts src/lib/hare/queue.test.ts src/lib/hare/webhook.test.ts", "lint": "eslint .", "format": "prettier --write ." }, diff --git a/src/lib/hare/mention.ts b/src/lib/hare/mention.ts new file mode 100644 index 0000000..69a688f --- /dev/null +++ b/src/lib/hare/mention.ts @@ -0,0 +1,8 @@ +/** Conversation or inline comment that asks hare-bot to (re)review. */ +export function mentionsHareBot(body: string | null | undefined): boolean { + return /@hare-bot\b/i.test(body ?? ""); +} + +export function isBotCommentAuthor(login: string | null | undefined): boolean { + return /^hare-bot$/i.test((login ?? "").trim()); +} diff --git a/src/lib/hare/queue.ts b/src/lib/hare/queue.ts index 9541488..f7fdc76 100644 --- a/src/lib/hare/queue.ts +++ b/src/lib/hare/queue.ts @@ -39,7 +39,15 @@ export function isStaleRunning(createdAt: string | null | undefined): boolean { export function enqueueReview(job: ReviewJob): boolean { const s = state(); const key = jobKey(job); - if (s.inflight.has(key) || s.pending.some((p) => jobKey(p) === key)) return false; + const waiting = s.pending.find((p) => jobKey(p) === key); + if (waiting) { + if (job.force) waiting.force = true; + return false; + } + if (s.inflight.has(key)) { + if (job.force) s.pending.push({ ...job, force: true }); + return job.force === true; + } s.pending.push(job); pump(); return true; diff --git a/src/lib/hare/webhook.test.ts b/src/lib/hare/webhook.test.ts new file mode 100644 index 0000000..45348a4 --- /dev/null +++ b/src/lib/hare/webhook.test.ts @@ -0,0 +1,29 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { isBotCommentAuthor, mentionsHareBot } from "./mention.ts"; + +describe("mentionsHareBot", () => { + it("matches a conversation ping", () => { + assert.equal( + mentionsHareBot("@hare-bot please re-review — OG asset is now committed"), + true, + ); + }); + it("is case-insensitive", () => { + assert.equal(mentionsHareBot("Hey @Hare-Bot look at this"), true); + }); + it("ignores other bots and bare words", () => { + assert.equal(mentionsHareBot("hare-bot without an at-sign"), false); + assert.equal(mentionsHareBot("@dependabot rebase"), false); + assert.equal(mentionsHareBot(""), false); + assert.equal(mentionsHareBot(null), false); + }); +}); + +describe("isBotCommentAuthor", () => { + it("skips hare-bot so review comments do not loop", () => { + assert.equal(isBotCommentAuthor("hare-bot"), true); + assert.equal(isBotCommentAuthor("Hare-Bot"), true); + assert.equal(isBotCommentAuthor("ginxx009"), false); + }); +}); diff --git a/src/lib/hare/webhook.ts b/src/lib/hare/webhook.ts index 55d4b24..25e77dd 100644 --- a/src/lib/hare/webhook.ts +++ b/src/lib/hare/webhook.ts @@ -1,5 +1,7 @@ import { createHmac, timingSafeEqual } from "node:crypto"; -import { findWatchedByRepo, upsertPull } from "./db"; +import { findWatchedByRepo, getConnection, upsertPull } from "./db"; +import { getPull } from "./github"; +import { isBotCommentAuthor, mentionsHareBot } from "./mention"; import { enqueueReview } from "./queue"; export function verifyGithubSignature( @@ -20,6 +22,7 @@ export function verifyGithubSignature( } } + type GhWebhookPull = { number: number; title: string; @@ -36,6 +39,61 @@ type GhWebhookPull = { updated_at?: string; }; +const PR_ACTIONS = new Set(["opened", "synchronize", "reopened", "ready_for_review"]); +const COMMENT_EVENTS = new Set(["issue_comment", "pull_request_review_comment"]); + +async function queueWatchers(input: { + payload: string; + signature: string | null; + owner: string; + repo: string; + number: number; + pull: GhWebhookPull; + force: boolean; +}): Promise<{ status: number; queued: number; message: string }> { + const watchers = await findWatchedByRepo(input.owner, input.repo); + const matched = watchers.filter((w) => + verifyGithubSignature(w.webhookSecret, input.payload, input.signature), + ); + if (matched.length === 0) { + return { status: 401, queued: 0, message: "invalid signature" }; + } + + let queued = 0; + for (const watcher of matched) { + if (!watcher.autoReview) continue; + await upsertPull(watcher.userId, { + owner: input.owner, + repo: input.repo, + number: input.number, + title: input.pull.title, + body: input.pull.body, + author: input.pull.user?.login ?? "unknown", + state: input.pull.state, + draft: Boolean(input.pull.draft), + htmlUrl: input.pull.html_url, + headSha: input.pull.head.sha, + baseSha: input.pull.base.sha, + headRef: input.pull.head.ref, + baseRef: input.pull.base.ref, + additions: input.pull.additions ?? 0, + deletions: input.pull.deletions ?? 0, + changedFiles: input.pull.changed_files ?? 0, + isDemo: false, + githubUpdatedAt: input.pull.updated_at ?? new Date().toISOString(), + }); + enqueueReview({ + userId: watcher.userId, + owner: input.owner, + repo: input.repo, + number: input.number, + force: input.force, + }); + queued += 1; + } + return { status: 200, queued, message: `queued ${queued}` }; +} + export async function handleGithubWebhook( payload: string, signature: string | null, @@ -44,73 +102,133 @@ export async function handleGithubWebhook( if (eventName === "ping") { return { status: 200, body: { ok: true, message: "Hare webhook ready" } }; } - if (eventName && eventName !== "pull_request") { - return { status: 200, body: { ok: true, message: "ignored event" } }; - } - let data: { - action?: string; - repository?: { name?: string; owner?: { login?: string } }; - pull_request?: GhWebhookPull; - }; + let data: Record; try { - data = JSON.parse(payload) as typeof data; + data = JSON.parse(payload) as Record; } catch { return { status: 400, body: { ok: false, message: "invalid json" } }; } - const action = data.action ?? ""; - if (!["opened", "synchronize", "reopened", "ready_for_review"].includes(action)) { - return { status: 200, body: { ok: true, message: "ignored action" } }; + const repository = data.repository as + | { name?: string; owner?: { login?: string } } + | undefined; + const owner = repository?.owner?.login; + const repo = repository?.name; + if (!owner || !repo) { + return { status: 400, body: { ok: false, message: "missing repository" } }; } - const owner = data.repository?.owner?.login; - const repo = data.repository?.name; - const pr = data.pull_request; - if (!owner || !repo || !pr) { - return { status: 400, body: { ok: false, message: "missing pull request" } }; - } - - const watchers = await findWatchedByRepo(owner, repo); - const matched = watchers.filter((w) => - verifyGithubSignature(w.webhookSecret, payload, signature), - ); - const targets = matched.length > 0 ? matched : []; - if (targets.length === 0) { - return { status: 401, body: { ok: false, message: "invalid signature" } }; - } - - let queued = 0; - for (const watcher of targets) { - if (!watcher.autoReview) continue; - await upsertPull(watcher.userId, { + if (!eventName || eventName === "pull_request") { + const action = String(data.action ?? ""); + if (!PR_ACTIONS.has(action)) { + return { status: 200, body: { ok: true, message: "ignored action" } }; + } + const pr = data.pull_request as GhWebhookPull | undefined; + if (!pr) { + return { status: 400, body: { ok: false, message: "missing pull request" } }; + } + const result = await queueWatchers({ + payload, + signature, owner, repo, number: pr.number, - title: pr.title, - body: pr.body, - author: pr.user?.login ?? "unknown", - state: pr.state, - draft: Boolean(pr.draft), - htmlUrl: pr.html_url, - headSha: pr.head.sha, - baseSha: pr.base.sha, - headRef: pr.head.ref, - baseRef: pr.base.ref, - additions: pr.additions ?? 0, - deletions: pr.deletions ?? 0, - changedFiles: pr.changed_files ?? 0, - isDemo: false, - githubUpdatedAt: pr.updated_at ?? new Date().toISOString(), + pull: pr, + force: false, }); - enqueueReview({ - userId: watcher.userId, + return { + status: result.status, + body: { ok: result.status === 200, message: result.message }, + }; + } + + if (COMMENT_EVENTS.has(eventName)) { + const action = String(data.action ?? ""); + if (action !== "created" && action !== "edited") { + return { status: 200, body: { ok: true, message: "ignored action" } }; + } + const comment = data.comment as + | { body?: string; user?: { login?: string } } + | undefined; + if (isBotCommentAuthor(comment?.user?.login)) { + return { status: 200, body: { ok: true, message: "ignored own comment" } }; + } + if (!mentionsHareBot(comment?.body)) { + return { status: 200, body: { ok: true, message: "no @hare-bot mention" } }; + } + + let number: number | null = null; + if (eventName === "issue_comment") { + const issue = data.issue as + | { number?: number; pull_request?: unknown } + | undefined; + if (!issue?.pull_request) { + return { status: 200, body: { ok: true, message: "not a pull request comment" } }; + } + number = typeof issue.number === "number" ? issue.number : null; + } else { + const pr = data.pull_request as { number?: number } | undefined; + number = typeof pr?.number === "number" ? pr.number : null; + } + if (!number) { + return { status: 400, body: { ok: false, message: "missing pull request number" } }; + } + + const watchers = await findWatchedByRepo(owner, repo); + const matched = watchers.filter((w) => + verifyGithubSignature(w.webhookSecret, payload, signature), + ); + if (matched.length === 0) { + return { status: 401, body: { ok: false, message: "invalid signature" } }; + } + + const conn = await getConnection(matched[0]!.userId); + if (!conn) { + return { status: 200, body: { ok: false, message: "GitHub is not connected" } }; + } + + let live; + try { + live = await getPull(conn.token, owner, repo, number); + } catch (err) { + return { + status: 200, + body: { + ok: false, + message: err instanceof Error ? err.message : "could not load pull request", + }, + }; + } + + const result = await queueWatchers({ + payload, + signature, owner, repo, - number: pr.number, + number, + pull: { + number: live.number, + title: live.title, + body: live.body, + state: live.state, + draft: live.draft, + html_url: live.html_url, + user: live.user, + head: live.head, + base: live.base, + additions: live.additions, + deletions: live.deletions, + changed_files: live.changed_files, + updated_at: live.updated_at, + }, + force: true, }); - queued += 1; + return { + status: result.status, + body: { ok: result.status === 200, message: result.message }, + }; } - return { status: 200, body: { ok: true, message: `queued ${queued}` } }; + return { status: 200, body: { ok: true, message: "ignored event" } }; } diff --git a/src/routes/_app/settings.tsx b/src/routes/_app/settings.tsx index cd45a5a..6a278a7 100644 --- a/src/routes/_app/settings.tsx +++ b/src/routes/_app/settings.tsx @@ -128,7 +128,7 @@ jobs:

Webhook

- After you publish this app, add a repository webhook for Pull requests pointing at this URL. Use the secret shown for a watched repo. + After you publish this app, add a repository webhook pointing at this URL. Enable **Pull requests** and **Issue comments**. Use the secret shown for a watched repo.