diff --git a/README.md b/README.md index 54824ff..12481d8 100644 --- a/README.md +++ b/README.md @@ -60,12 +60,19 @@ Dependencies: **none**. Node ≥ 22.13 (uses `node:sqlite`, global `fetch`). "routerUrl": "http://localhost:5497", "model": "anthropic/claude-sonnet-4-6", "tenantId": "nirlab", - "publicBaseUrl": "http://localhost:5498" + "publicBaseUrl": "http://localhost:5498", + "admin_token": "", + "botUsername": "shre-reviewer" } ``` Bot identity: Gitea user `shre-reviewer` (created via `gitea admin user create`). +`admin_token` powers the 403 self-heal: when a status/comment post returns 403 +(a repo auto-wired by the admin default hook where the bot has no access yet), +the service adds `shre-reviewer` as collaborator (write) via the admin token +and retries the post once. Without `admin_token` the 403 surfaces as before. + ## Deploy (Mac dev tier) - `launchd/ai.granthi.review.plist` → `~/Library/LaunchAgents/`, KeepAlive service on :5498, running from the `~/Services/granthi-review` clone. @@ -83,9 +90,15 @@ Bot identity: Gitea user `shre-reviewer` (created via `gitea admin user create`) ### Known v1 limitations -- The review queue is in-memory: a service restart drops queued/in-flight reviews, leaving that sha's `granthi-review` status at `pending`. Re-push (or push an empty commit) to re-trigger. - Bare pushes get status + ledger only (no Gitea commit-comment API). -- Reviews for repos outside the pilot set require the bot (`shre-reviewer`) to have write access before statuses/comments can post; grant collaborator write when adding repos. + +### Hardening (fix/review-hardening) + +- **Full push-range review**: pushes are reviewed as ONE compare diff `before...after` (web compare `.diff` route — the 1.27 API has no compare diff variant). New-branch pushes (`before` = zero sha) fall back to the head commit diff. No more first-20-commits slicing under a head-sha status. +- **Truncated diff ≠ pass**: when the 60KB cap truncates the diff, the prompt and the status description both say `partial review (diff truncated)` and the verdict is capped at `warn` (a confirmed-critical `fail` still fails). +- **Diff fetch failure ≠ pass**: any diff-fetch failure (throw, non-200, empty body for a non-empty push/PR) short-circuits to status `warning` — `review unavailable — diff fetch failed (not blocking)` — with a ledger row. A diffless prompt is never sent. +- **Crash-safe queue**: every accepted job is persisted to sqlite (`queue_jobs`) before the pending status posts; on startup, rows that never reached a final state are re-enqueued, so a restart no longer strands a sha at `pending`. +- **403 self-heal**: see `admin_token` above. `scripts/wire-repos.mjs` wires the webhook + bot collaborator across ALL repos on the peer (idempotent; re-runnable). ## Adding the Reviews tab later (Granthi overlay) diff --git a/scripts/wire-repos.mjs b/scripts/wire-repos.mjs new file mode 100644 index 0000000..ecc68d6 --- /dev/null +++ b/scripts/wire-repos.mjs @@ -0,0 +1,95 @@ +#!/usr/bin/env node +// Wire EVERY repo on the peer forge for granthi-review: +// - create the push+pull_request webhook -> (HMAC secret from the +// service config) on every repo that lacks it (idempotent: skipped when a +// hook with that exact URL already exists) +// - add the bot (shre-reviewer) as collaborator (write) so status/comment +// posts don't 403 +// Re-runnable at any time. Uses the admin-scoped token (config admin_token). +// +// Usage: node scripts/wire-repos.mjs [--dry-run] + +import { readFileSync } from 'node:fs'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; + +const CONFIG_DIR = process.env.GRANTHI_REVIEW_HOME || join(homedir(), '.granthi-review'); +const cfg = JSON.parse(readFileSync(join(CONFIG_DIR, 'config.json'), 'utf8')); + +const base = (cfg.forgeBaseUrl || 'http://localhost:3030').replace(/\/$/, ''); +const adminToken = cfg.admin_token || process.env.GITEA_CLAUDE_SIA_TOKEN; +const secret = cfg.webhookSecret; +const botUsername = cfg.botUsername || 'shre-reviewer'; +const hookUrl = process.env.GRANTHI_REVIEW_HOOK_URL || 'http://host.docker.internal:5498/webhook'; +const dryRun = process.argv.includes('--dry-run'); + +if (!adminToken) { console.error('no admin token (config admin_token or $GITEA_CLAUDE_SIA_TOKEN)'); process.exit(1); } +if (!secret) { console.error('no webhookSecret in config'); process.exit(1); } + +async function api(method, path, body, ok = [200, 201, 204]) { + const res = await fetch(`${base}${path}`, { + method, + headers: { + Authorization: `token ${adminToken}`, + ...(body ? { 'Content-Type': 'application/json' } : {}) + }, + body: body ? JSON.stringify(body) : undefined + }); + if (!ok.includes(res.status)) { + const text = await res.text().catch(() => ''); + throw new Error(`${method} ${path} -> ${res.status}: ${text.slice(0, 200)}`); + } + if (res.status === 204) return null; + return res.json().catch(() => null); +} + +async function allRepos() { + const repos = []; + for (let page = 1; ; page++) { + const out = await api('GET', `/api/v1/repos/search?limit=50&page=${page}`); + const batch = out?.data || []; + repos.push(...batch); + if (batch.length < 50) break; + } + return repos; +} + +const repos = await allRepos(); +let hooksCreated = 0, hooksPresent = 0, collabAdded = 0, skipped = 0, errors = 0; + +for (const r of repos) { + const full = r.full_name; + const [owner, name] = full.split('/'); + try { + if (r.archived || r.empty || r.mirror) { skipped++; console.log(`skip ${full} (${r.archived ? 'archived' : r.mirror ? 'mirror' : 'empty'})`); continue; } + const hooks = await api('GET', `/api/v1/repos/${full}/hooks?limit=50`) || []; + const has = hooks.some(h => h?.config?.url === hookUrl); + if (has) { + hooksPresent++; + } else if (dryRun) { + console.log(`would wire ${full}`); + } else { + await api('POST', `/api/v1/repos/${full}/hooks`, { + type: 'gitea', + active: true, + events: ['push', 'pull_request'], + config: { url: hookUrl, content_type: 'json', secret } + }); + hooksCreated++; + console.log(`wired ${full}`); + } + // collaborator write for the bot (idempotent PUT); skip bot-owned repos + if (owner !== botUsername && !dryRun) { + await api('PUT', `/api/v1/repos/${full}/collaborators/${botUsername}`, { permission: 'write' }); + collabAdded++; + } + } catch (e) { + errors++; + console.error(`ERROR ${full}: ${e.message}`); + } +} + +console.log(`\ntotal repos: ${repos.length}`); +console.log(`hooks created: ${hooksCreated}, already wired: ${hooksPresent}, skipped (archived/empty/mirror): ${skipped}`); +console.log(`collaborator (write) ensured: ${collabAdded}, errors: ${errors}`); +process.exit(errors ? 2 : 0); diff --git a/src/config.js b/src/config.js index 9454c7b..7ce4076 100644 --- a/src/config.js +++ b/src/config.js @@ -19,7 +19,8 @@ const DEFAULTS = { digestDir: join(CONFIG_DIR, 'digests'), maxDiffBytes: 60 * 1024, routerTimeoutMs: 240000, - digestLastN: 20 + digestLastN: 20, + botUsername: 'shre-reviewer' }; export function loadConfig() { @@ -28,6 +29,9 @@ export function loadConfig() { fileCfg = JSON.parse(readFileSync(join(CONFIG_DIR, 'config.json'), 'utf8')); } catch { /* config file optional; env/defaults apply */ } const cfg = { ...DEFAULTS, ...fileCfg }; + // admin-scoped token (config key: admin_token) — used ONLY to self-heal bot + // permissions (add the bot as collaborator when a status/comment post 403s). + cfg.adminToken = fileCfg.admin_token || ''; // env overrides if (process.env.GRANTHI_REVIEW_PORT) cfg.port = Number(process.env.GRANTHI_REVIEW_PORT); if (process.env.GRANTHI_REVIEW_TOKEN) cfg.botToken = process.env.GRANTHI_REVIEW_TOKEN; diff --git a/src/lib/access.js b/src/lib/access.js new file mode 100644 index 0000000..c9dfd28 --- /dev/null +++ b/src/lib/access.js @@ -0,0 +1,19 @@ +// Self-healing bot access: new repos wired by the admin default hook 403 the +// bot's status/comment posts because shre-reviewer is not a collaborator yet. +// On a 403 we add the bot as collaborator (write) using the admin-scoped +// token, then retry the original call exactly once. + +export async function withCollabRetry(fn, { adminGitea, owner, repo, botUsername, log }) { + try { + return await fn(); + } catch (e) { + if (e?.status !== 403) throw e; + if (!adminGitea) { + log?.(`[${owner}/${repo}] 403 from forge and no admin_token configured — cannot self-heal collaborator access`); + throw e; + } + log?.(`[${owner}/${repo}] 403 from forge — adding ${botUsername} as collaborator (write) and retrying once`); + await adminGitea.addCollaborator(owner, repo, botUsername, 'write'); + return fn(); + } +} diff --git a/src/lib/db.js b/src/lib/db.js index e3b9aae..0133cc2 100644 --- a/src/lib/db.js +++ b/src/lib/db.js @@ -26,7 +26,18 @@ const DDL = [ ts TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')) )`, `CREATE INDEX IF NOT EXISTS idx_reviews_repo_ts ON reviews(repo, ts DESC)`, - `CREATE INDEX IF NOT EXISTS idx_reviews_sha ON reviews(sha)` + `CREATE INDEX IF NOT EXISTS idx_reviews_sha ON reviews(sha)`, + `CREATE TABLE IF NOT EXISTS queue_jobs ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + repo TEXT NOT NULL, + sha TEXT NOT NULL, + job TEXT NOT NULL, + state TEXT NOT NULL DEFAULT 'queued', + outcome TEXT, + created_ts TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')), + finished_ts TEXT + )`, + `CREATE INDEX IF NOT EXISTS idx_queue_jobs_state ON queue_jobs(state)` ]; export function openDb(dbPath) { @@ -55,6 +66,37 @@ export function insertReview(db, r) { return res.lastInsertRowid; } +// --- persistent work queue (crash-safe: rows written before the pending +// status posts; rows that never reach 'done' are re-enqueued on startup) --- + +export function enqueueJob(db, job) { + const res = db.prepare( + `INSERT INTO queue_jobs (repo, sha, job) VALUES (?, ?, ?)` + ).run(`${job.owner}/${job.repo}`, String(job.sha), JSON.stringify(job)); + return res.lastInsertRowid; +} + +export function finishJob(db, id, outcome = null) { + db.prepare( + `UPDATE queue_jobs SET state = 'done', outcome = ?, + finished_ts = strftime('%Y-%m-%dT%H:%M:%fZ','now') WHERE id = ?` + ).run(outcome, id); +} + +// Startup reconciliation: rows still 'queued' belonged to a previous process +// that died mid-review (their posted status is stuck at 'pending'). +export function recoverPendingJobs(db) { + const rows = db.prepare( + `SELECT id, job FROM queue_jobs WHERE state != 'done' ORDER BY id ASC` + ).all(); + const out = []; + for (const row of rows) { + try { out.push({ id: row.id, job: JSON.parse(row.job) }); } + catch { finishJob(db, row.id, 'unparseable-job'); } + } + return out; +} + export function listReviews(db, repo, limit = 50) { return db.prepare( 'SELECT * FROM reviews WHERE repo = ? ORDER BY id DESC LIMIT ?' diff --git a/src/lib/gitea.js b/src/lib/gitea.js index 7a8fcd1..001efde 100644 --- a/src/lib/gitea.js +++ b/src/lib/gitea.js @@ -41,6 +41,20 @@ export class GiteaClient { return this.req('GET', `/api/v1/repos/${owner}/${repo}/compare/${before}...${after}`); } + // Full-range diff for a push (base...head) in ONE request. Gitea 1.27's API + // has no compare .diff variant, but the web compare route serves .diff and + // accepts `Authorization: token` (verified live on 1.27.1). + getCompareDiff(owner, repo, before, after) { + return this.req('GET', `/${owner}/${repo}/compare/${before}...${after}.diff`, { raw: true }); + } + + // Add a collaborator (requires owner/admin token). 204 on success. + addCollaborator(owner, repo, username, permission = 'write') { + return this.req('PUT', `/api/v1/repos/${owner}/${repo}/collaborators/${username}`, { + body: { permission }, ok: [204] + }); + } + // File content at HEAD (returns null when absent) async getFileHead(owner, repo, path, maxBytes = 8192) { try { diff --git a/src/lib/prompt.js b/src/lib/prompt.js index aae3fe3..94e61a6 100644 --- a/src/lib/prompt.js +++ b/src/lib/prompt.js @@ -36,7 +36,7 @@ Rules: if (conventions) { user += `\nRepo conventions (excerpt from CLAUDE.md / CONTRIBUTING.md — weigh style/design against these):\n---\n${conventions}\n---\n`; } - if (truncated) user += `\nNOTE: the diff below was TRUNCATED at the size cap; judge only what you see and do not penalize for the missing tail.\n`; + if (truncated) user += `\nNOTE: this is a partial review (diff truncated) — the diff below was cut at the size cap. Judge only what you see and do not penalize for the missing tail; unseen code is unreviewed, not approved.\n`; user += `\nUnified diff to review:\n\`\`\`diff\n${diff}\n\`\`\`\n\nReturn the JSON object now.`; return [ { role: 'system', content: system }, diff --git a/src/lib/review.js b/src/lib/review.js index b8cc669..43bc700 100644 --- a/src/lib/review.js +++ b/src/lib/review.js @@ -5,19 +5,26 @@ import { chat } from './router.js'; import { extractJson } from './jsonExtract.js'; -import { normalizeReview } from './verdict.js'; +import { normalizeReview, applyTruncationCap } from './verdict.js'; import { parseTrailers } from './trailers.js'; import { buildReviewPrompt } from './prompt.js'; import { renderScorecardMarkdown } from './render.js'; import { insertReview } from './db.js'; +import { withCollabRetry } from './access.js'; const CONTEXT = 'granthi-review'; +const ZERO_SHA = /^0+$/; -export async function runReview({ cfg, gitea, db, job, withGlobal, log }) { +export async function runReview({ cfg, gitea, adminGitea, db, job, withGlobal, log }) { const { owner, repo, sha } = job; const fullRepo = `${owner}/${repo}`; const dashboardUrl = `${cfg.publicBaseUrl}/review/${owner}/${repo}`; const t0 = Date.now(); + // status/comment posters that self-heal a 403 (bot not yet collaborator on + // repos auto-wired by the admin default hook) via the admin token, once. + const retryCtx = { adminGitea, owner, repo, botUsername: cfg.botUsername, log }; + const postStatus = (body) => withCollabRetry(() => gitea.postStatus(owner, repo, sha, body), retryCtx); + const postComment = (index, md) => withCollabRetry(() => gitea.postIssueComment(owner, repo, index, md), retryCtx); // For PR events the webhook only carries title/body; pull the PR's commits // so Co-Authored-By trailer attribution sees real commit messages. @@ -33,26 +40,36 @@ export async function runReview({ cfg, gitea, db, job, withGlobal, log }) { // 0. pending status (best-effort) await safe(log, 'pending-status', () => - gitea.postStatus(owner, repo, sha, { + postStatus({ state: 'pending', context: CONTEXT, description: 'AI review in progress…', target_url: dashboardUrl })); - // 1. diff + // 1. diff — any fetch failure (throw, non-200, or empty body for a job that + // has commits) short-circuits to a 'warning' status. A diffless prompt would + // make the model "review" nothing and pass; never send one. let diff = '', truncated = false, diffBytes = 0; try { diff = await fetchDiff({ gitea, job }); - diffBytes = Buffer.byteLength(diff); - if (diffBytes > cfg.maxDiffBytes) { - diff = Buffer.from(diff).subarray(0, cfg.maxDiffBytes).toString('utf8'); - truncated = true; - log(`[${fullRepo}@${sha.slice(0, 8)}] diff truncated ${diffBytes} -> ${cfg.maxDiffBytes} bytes`); - } } catch (e) { - return await failOpen({ cfg, gitea, db, job, log, dashboardUrl, t0, error: `diff fetch failed: ${e.message}` }); + return await failOpen({ + cfg, db, job, log, postStatus, dashboardUrl, t0, + error: `diff fetch failed: ${e.message}`, + description: 'review unavailable — diff fetch failed (not blocking)' + }); } - if (!diff.trim()) { - return await failOpen({ cfg, gitea, db, job, log, dashboardUrl, t0, error: 'empty diff' , state: 'success', description: 'empty diff — nothing to review'}); + if (!diff || !diff.trim()) { + return await failOpen({ + cfg, db, job, log, postStatus, dashboardUrl, t0, + error: 'diff fetch failed: empty diff body for a non-empty push/PR', + description: 'review unavailable — diff fetch failed (not blocking)' + }); + } + diffBytes = Buffer.byteLength(diff); + if (diffBytes > cfg.maxDiffBytes) { + diff = Buffer.from(diff).subarray(0, cfg.maxDiffBytes).toString('utf8'); + truncated = true; + log(`[${fullRepo}@${sha.slice(0, 8)}] diff truncated ${diffBytes} -> ${cfg.maxDiffBytes} bytes (partial review)`); } // 2. conventions @@ -98,9 +115,13 @@ export async function runReview({ cfg, gitea, db, job, withGlobal, log }) { } }); if (!review) { - return await failOpen({ cfg, gitea, db, job, log, dashboardUrl, t0, error: `review unavailable: ${llmError}` }); + return await failOpen({ cfg, db, job, log, postStatus, dashboardUrl, t0, error: `review unavailable: ${llmError}` }); } + // A truncated diff can never yield a clean 'pass' — part of the change was + // not reviewed. Cap at 'warn' (a confirmed-critical 'fail' still fails). + applyTruncationCap(review, truncated); + // 4. status + comment const stateByVerdict = { pass: 'success', warn: 'success', fail: 'failure' }; const state = stateByVerdict[review.verdict]; @@ -109,14 +130,15 @@ export async function runReview({ cfg, gitea, db, job, withGlobal, log }) { warn: `Grade ${review.grade} (${review.overall}/100) — pass with warnings (${review.findings.length} finding(s))`, fail: `Grade ${review.grade} (${review.overall}/100) — BLOCKED: confirmed critical finding (admin merge = override)` }; + const description = truncated + ? `partial review (diff truncated) — ${descByVerdict[review.verdict]}` + : descByVerdict[review.verdict]; await safe(log, 'final-status', () => - gitea.postStatus(owner, repo, sha, { - state, context: CONTEXT, description: descByVerdict[review.verdict], target_url: dashboardUrl - })); + postStatus({ state, context: CONTEXT, description, target_url: dashboardUrl })); const md = renderScorecardMarkdown({ repo: fullRepo, sha, review, dashboardUrl, truncated }); if (job.prIndex) { - await safe(log, 'pr-comment', () => gitea.postIssueComment(owner, repo, job.prIndex, md)); + await safe(log, 'pr-comment', () => postComment(job.prIndex, md)); } else { // Gitea has no commit-comment API endpoint; bare pushes get status + // ledger + dashboard only (README documents this). @@ -137,24 +159,25 @@ export async function runReview({ cfg, gitea, db, job, withGlobal, log }) { return { ok: true, verdict: review.verdict }; } -async function fetchDiff({ gitea, job }) { +// Push reviews cover the FULL push range in ONE compare diff (base=before, +// head=after) — never a per-commit subset, so the head status can't claim +// success for commits that were silently dropped. New-branch pushes +// (before = 0000…) fall back to the head commit's diff. +export async function fetchDiff({ gitea, job }) { const { owner, repo } = job; if (job.prIndex) return gitea.getPrDiff(owner, repo, job.prIndex); - // push: per-commit diffs for the pushed shas (payload-provided), capped later - const parts = []; - for (const sha of job.commitShas.slice(0, 20)) { - try { parts.push(await gitea.getCommitDiff(owner, repo, sha)); } - catch (e) { parts.push(`# diff for ${sha} unavailable: ${e.message}\n`); } + if (!job.before || ZERO_SHA.test(job.before)) { + return gitea.getCommitDiff(owner, repo, job.sha); } - return parts.join('\n'); + return gitea.getCompareDiff(owner, repo, job.before, job.sha); } -async function failOpen({ cfg, gitea, db, job, log, dashboardUrl, t0, error, state = 'warning', description }) { +async function failOpen({ cfg, db, job, log, postStatus, dashboardUrl, t0, error, state = 'warning', description }) { const { owner, repo, sha } = job; const fullRepo = `${owner}/${repo}`; log(`[${fullRepo}@${sha.slice(0, 8)}] fail-open: ${error}`); await safe(log, 'fail-open-status', () => - gitea.postStatus(owner, repo, sha, { + postStatus({ state, context: CONTEXT, description: description || `review unavailable — not blocking (${error.slice(0, 180)})`, target_url: dashboardUrl diff --git a/src/lib/verdict.js b/src/lib/verdict.js index cedd638..70f272e 100644 --- a/src/lib/verdict.js +++ b/src/lib/verdict.js @@ -62,6 +62,13 @@ export function normalizeReview(raw) { return review; } +// A truncated diff means part of the change was never reviewed: a clean +// 'pass' cannot be claimed. Cap at 'warn'; a confirmed-critical 'fail' stands. +export function applyTruncationCap(review, truncated) { + if (truncated && review && review.verdict === 'pass') review.verdict = 'warn'; + return review; +} + function clampScore(v) { const n = Number(v); if (!Number.isFinite(n)) return 0; diff --git a/src/server.js b/src/server.js index e588859..59b6d8f 100644 --- a/src/server.js +++ b/src/server.js @@ -1,6 +1,6 @@ import { createServer } from 'node:http'; import { loadConfig } from './config.js'; -import { openDb, listReviews } from './lib/db.js'; +import { openDb, listReviews, enqueueJob, finishJob, recoverPendingJobs } from './lib/db.js'; import { GiteaClient } from './lib/gitea.js'; import { ReviewQueue } from './lib/queue.js'; import { verifySignature, jobFromWebhook } from './lib/webhook.js'; @@ -24,8 +24,34 @@ if (!cfg.webhookSecret) { console.error('missing webhookSecret in config'); proc const db = openDb(cfg.dbPath); const gitea = new GiteaClient({ baseUrl: cfg.forgeBaseUrl, token: cfg.botToken }); +// admin-scoped client: only used to self-heal bot collaborator access on 403 +const adminGitea = cfg.adminToken + ? new GiteaClient({ baseUrl: cfg.forgeBaseUrl, token: cfg.adminToken }) + : null; const queue = new ReviewQueue(); +// Run one persisted job through the pipeline, marking the queue row done +// whatever the outcome (runReview itself posts a final or fail-open status). +function dispatchJob(queueRowId, job) { + return queue.enqueue(`${job.owner}/${job.repo}`, (withGlobal) => + runReview({ cfg, gitea, adminGitea, db, job, withGlobal, log }) + ).then( + (r) => finishJob(db, queueRowId, r?.ok ? (r.verdict || 'ok') : `error:${String(r?.error || '').slice(0, 200)}`), + (e) => { + log(`review pipeline error for ${job.owner}/${job.repo}@${job.sha}: ${e.stack || e.message}`); + finishJob(db, queueRowId, `crash:${String(e.message || e).slice(0, 200)}`); + } + ); +} + +// Startup reconciliation: re-enqueue persisted jobs a previous process never +// finished (their commit status is stuck at 'pending' otherwise). +const recovered = recoverPendingJobs(db); +if (recovered.length) { + log(`recovering ${recovered.length} unfinished review job(s) from previous run`); + for (const { id, job } of recovered) dispatchJob(id, job); +} + const server = createServer((req, res) => { const url = new URL(req.url, 'http://x'); try { @@ -78,11 +104,11 @@ function handleWebhook(req, res) { return json(res, 200, { accepted: false, reason: 'not reviewable' }); } log(`webhook: accepted ${job.event} ${job.owner}/${job.repo}@${String(job.sha).slice(0, 10)}${job.prIndex ? ' PR#' + job.prIndex : ''}`); + // persist BEFORE acking/enqueueing so a restart can re-run it, then // respond immediately; review runs async on the queue + const queueRowId = enqueueJob(db, job); json(res, 202, { accepted: true }); - queue.enqueue(`${job.owner}/${job.repo}`, (withGlobal) => - runReview({ cfg, gitea, db, job, withGlobal, log }) - ).catch(e => log(`review pipeline error for ${job.owner}/${job.repo}@${job.sha}: ${e.stack || e.message}`)); + dispatchJob(queueRowId, job); }); req.on('error', e => log('webhook req error:', e.message)); } diff --git a/test/access.test.js b/test/access.test.js new file mode 100644 index 0000000..d1d01bc --- /dev/null +++ b/test/access.test.js @@ -0,0 +1,84 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { withCollabRetry } from '../src/lib/access.js'; + +const err = (status, msg = `http ${status}`) => Object.assign(new Error(msg), { status }); +const noLog = () => {}; + +function adminStub() { + const calls = []; + return { + calls, + addCollaborator(owner, repo, username, permission) { + calls.push({ owner, repo, username, permission }); + return Promise.resolve(); + } + }; +} + +test('403 adds collaborator (write) then retries once and succeeds', async () => { + const admin = adminStub(); + let attempts = 0; + const out = await withCollabRetry(async () => { + attempts++; + if (attempts === 1) throw err(403); + return 'ok'; + }, { adminGitea: admin, owner: 'nirpa', repo: 'newrepo', botUsername: 'shre-reviewer', log: noLog }); + assert.equal(out, 'ok'); + assert.equal(attempts, 2); + assert.deepEqual(admin.calls, [{ owner: 'nirpa', repo: 'newrepo', username: 'shre-reviewer', permission: 'write' }]); +}); + +test('non-403 errors are rethrown without touching collaborators', async () => { + const admin = adminStub(); + await assert.rejects( + () => withCollabRetry(() => Promise.reject(err(500)), { + adminGitea: admin, owner: 'o', repo: 'r', botUsername: 'b', log: noLog + }), + /http 500/ + ); + assert.equal(admin.calls.length, 0); +}); + +test('403 with no admin client rethrows the original 403', async () => { + await assert.rejects( + () => withCollabRetry(() => Promise.reject(err(403)), { + adminGitea: null, owner: 'o', repo: 'r', botUsername: 'b', log: noLog + }), + (e) => e.status === 403 + ); +}); + +test('retry happens exactly once: second 403 propagates', async () => { + const admin = adminStub(); + let attempts = 0; + await assert.rejects( + () => withCollabRetry(() => { attempts++; return Promise.reject(err(403)); }, { + adminGitea: admin, owner: 'o', repo: 'r', botUsername: 'b', log: noLog + }), + (e) => e.status === 403 + ); + assert.equal(attempts, 2); + assert.equal(admin.calls.length, 1); +}); + +test('addCollaborator failure propagates (no infinite loop)', async () => { + const admin = { + addCollaborator: () => Promise.reject(err(422, 'bad permission')) + }; + let attempts = 0; + await assert.rejects( + () => withCollabRetry(() => { attempts++; return Promise.reject(err(403)); }, { + adminGitea: admin, owner: 'o', repo: 'r', botUsername: 'b', log: noLog + }), + /bad permission/ + ); + assert.equal(attempts, 1); +}); + +test('success path never consults the admin client', async () => { + const out = await withCollabRetry(() => Promise.resolve(42), { + adminGitea: undefined, owner: 'o', repo: 'r', botUsername: 'b', log: noLog + }); + assert.equal(out, 42); +}); diff --git a/test/fetchdiff.test.js b/test/fetchdiff.test.js new file mode 100644 index 0000000..54b9283 --- /dev/null +++ b/test/fetchdiff.test.js @@ -0,0 +1,66 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { fetchDiff } from '../src/lib/review.js'; + +function giteaStub() { + const calls = []; + return { + calls, + getPrDiff: (o, r, i) => { calls.push(['pr', o, r, i]); return Promise.resolve('PRDIFF'); }, + getCommitDiff: (o, r, sha) => { calls.push(['commit', o, r, sha]); return Promise.resolve('HEADDIFF'); }, + getCompareDiff: (o, r, before, after) => { calls.push(['compare', o, r, before, after]); return Promise.resolve('RANGEDIFF'); } + }; +} + +const HEAD = 'a'.repeat(40); +const BEFORE = 'b'.repeat(40); + +test('PR jobs use the PR diff', async () => { + const g = giteaStub(); + const out = await fetchDiff({ gitea: g, job: { owner: 'o', repo: 'r', prIndex: 5, sha: HEAD, before: null } }); + assert.equal(out, 'PRDIFF'); + assert.deepEqual(g.calls, [['pr', 'o', 'r', 5]]); +}); + +test('push uses ONE compare diff over the full before...after range', async () => { + const g = giteaStub(); + const out = await fetchDiff({ + gitea: g, + job: { + owner: 'o', repo: 'r', prIndex: null, sha: HEAD, before: BEFORE, + // 25 commits in the payload: must NOT fall back to per-commit slicing + commitShas: Array.from({ length: 25 }, (_, i) => String(i).padStart(40, '0')) + } + }); + assert.equal(out, 'RANGEDIFF'); + assert.deepEqual(g.calls, [['compare', 'o', 'r', BEFORE, HEAD]]); +}); + +test('new-branch push (before = zero sha) falls back to head commit diff', async () => { + const g = giteaStub(); + const out = await fetchDiff({ + gitea: g, + job: { owner: 'o', repo: 'r', prIndex: null, sha: HEAD, before: '0'.repeat(40), commitShas: [HEAD] } + }); + assert.equal(out, 'HEADDIFF'); + assert.deepEqual(g.calls, [['commit', 'o', 'r', HEAD]]); +}); + +test('missing before also falls back to head commit diff', async () => { + const g = giteaStub(); + const out = await fetchDiff({ + gitea: g, + job: { owner: 'o', repo: 'r', prIndex: null, sha: HEAD, before: null, commitShas: [HEAD] } + }); + assert.equal(out, 'HEADDIFF'); + assert.deepEqual(g.calls, [['commit', 'o', 'r', HEAD]]); +}); + +test('compare failure propagates (no silent placeholder diff)', async () => { + const g = giteaStub(); + g.getCompareDiff = () => Promise.reject(new Error('boom 500')); + await assert.rejects( + () => fetchDiff({ gitea: g, job: { owner: 'o', repo: 'r', prIndex: null, sha: HEAD, before: BEFORE } }), + /boom 500/ + ); +}); diff --git a/test/persistqueue.test.js b/test/persistqueue.test.js new file mode 100644 index 0000000..3b3d3f2 --- /dev/null +++ b/test/persistqueue.test.js @@ -0,0 +1,51 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { openDb, enqueueJob, finishJob, recoverPendingJobs } from '../src/lib/db.js'; + +const JOB = (over = {}) => ({ + event: 'push', owner: 'nirpa', repo: 'demo', sha: 'a'.repeat(40), + ref: 'refs/heads/main', before: 'b'.repeat(40), prIndex: null, prTitle: null, + pusher: 'nirpa', authors: ['nirpa'], commitShas: ['a'.repeat(40)], + commitMessages: ['msg'], ...over +}); + +test('enqueued jobs that never finish are recovered on startup', () => { + const db = openDb(':memory:'); + const id1 = enqueueJob(db, JOB({ sha: '1'.repeat(40) })); + const id2 = enqueueJob(db, JOB({ sha: '2'.repeat(40) })); + const id3 = enqueueJob(db, JOB({ sha: '3'.repeat(40), prIndex: 7 })); + finishJob(db, id2, 'pass'); + + const pending = recoverPendingJobs(db); + assert.equal(pending.length, 2); + assert.deepEqual(pending.map(p => p.id), [id1, id3]); + // job payload round-trips intact + assert.equal(pending[0].job.sha, '1'.repeat(40)); + assert.equal(pending[1].job.prIndex, 7); + assert.deepEqual(pending[0].job.commitMessages, ['msg']); +}); + +test('finished jobs are not recovered again', () => { + const db = openDb(':memory:'); + const id = enqueueJob(db, JOB()); + finishJob(db, id, 'warn'); + assert.equal(recoverPendingJobs(db).length, 0); +}); + +test('unparseable persisted job is marked done, not re-run forever', () => { + const db = openDb(':memory:'); + db.prepare(`INSERT INTO queue_jobs (repo, sha, job) VALUES ('a/b', 'c', '{broken')`).run(); + assert.equal(recoverPendingJobs(db).length, 0); + // and it is now terminal + assert.equal(recoverPendingJobs(db).length, 0); + const row = db.prepare(`SELECT state, outcome FROM queue_jobs`).get(); + assert.equal(row.state, 'done'); + assert.equal(row.outcome, 'unparseable-job'); +}); + +test('recovery is ordered oldest first', () => { + const db = openDb(':memory:'); + const ids = [enqueueJob(db, JOB()), enqueueJob(db, JOB()), enqueueJob(db, JOB())]; + const pending = recoverPendingJobs(db); + assert.deepEqual(pending.map(p => p.id), ids); +}); diff --git a/test/verdict.test.js b/test/verdict.test.js index 378484a..3614711 100644 --- a/test/verdict.test.js +++ b/test/verdict.test.js @@ -78,3 +78,30 @@ test('normalizeReview tolerates junk', () => { assert.equal(r.verdict, 'pass'); assert.equal(r.findings.length, 0); }); + +// --- applyTruncationCap --- +import { applyTruncationCap } from '../src/lib/verdict.js'; + +test('truncation caps pass at warn', () => { + const r = { verdict: 'pass' }; + applyTruncationCap(r, true); + assert.equal(r.verdict, 'warn'); +}); + +test('truncation leaves warn as warn', () => { + const r = { verdict: 'warn' }; + applyTruncationCap(r, true); + assert.equal(r.verdict, 'warn'); +}); + +test('truncation does not downgrade fail', () => { + const r = { verdict: 'fail' }; + applyTruncationCap(r, true); + assert.equal(r.verdict, 'fail'); +}); + +test('no truncation leaves pass untouched', () => { + const r = { verdict: 'pass' }; + applyTruncationCap(r, false); + assert.equal(r.verdict, 'pass'); +});