diff --git a/README.md b/README.md index 778955b..b08e60d 100644 --- a/README.md +++ b/README.md @@ -236,6 +236,37 @@ The harness keeps prompts bounded, skips external symlinks, rejects findings outside the reviewed file allowlist, and preserves existing triage fields when a finding is regenerated. +## Branch protection defaults + +`apply-branch-protection` applies the protoLabs recommended branch protection +ruleset to a repo. Two opinions, both off-by-default in GitHub's UI: + +1. **`required_status_checks` is for correctness, not advisory signals.** Drop + LLM review bots (CodeRabbit, protoquinn[bot], etc.) from required checks. + Bots gate via `reviewDecision` — silence shouldn't block merges. +2. **`strict_required_status_checks_policy: false`** for fast-moving repos with + linear PR stacks. Strict mode forces an N×CI-cycle drag on stacked work. + +```bash +# Dry-run against the current repo's main branch +npx @protolabsai/release-tools apply-branch-protection + +# Apply the defaults to a specific repo +npx @protolabsai/release-tools apply-branch-protection \ + --repo protoLabsAI/myrepo \ + --branch main \ + --apply + +# Custom required checks (Rust monorepo) +npx @protolabsai/release-tools apply-branch-protection \ + --required-checks cargo-test,cargo-clippy,cargo-fmt \ + --apply +``` + +See [`docs/branch-protection-defaults.md`](./docs/branch-protection-defaults.md) +for the full rationale, the "when not to use the defaults" list, and the flag +reference. + ## Development ```bash @@ -243,6 +274,7 @@ npm install node bin/rewrite-release-notes.mjs --help node bin/build-updater-manifest.mjs --help node bin/review-code.mjs --help +node bin/apply-branch-protection.mjs --help npm test ``` diff --git a/bin/apply-branch-protection.mjs b/bin/apply-branch-protection.mjs new file mode 100755 index 0000000..9c96391 --- /dev/null +++ b/bin/apply-branch-protection.mjs @@ -0,0 +1,257 @@ +#!/usr/bin/env node +/** + * @license + * Copyright 2026 protoLabs + * SPDX-License-Identifier: Apache-2.0 + * + * Apply the protoLabs recommended branch protection ruleset to a repo. + * + * Defaults: + * - `required_status_checks`: build, test, checks, ci-complete (correctness only) + * - `strict_required_status_checks_policy`: false (don't force PRs to be up-to-date with base) + * - Drop any context that looks like an LLM review bot (CodeRabbit, protoquinn[bot], etc.) + * from required_status_checks. Bots gate via reviewDecision, not status checks. + * + * Driven by the rationale in release-tools#6 (strict policy) and + * release-tools#10 (bot-checks exclusion). + * + * Usage: + * apply-branch-protection [flags] + * + * Flags: + * --repo Target repository. Default: derived from `git remote get-url origin`. + * --branch Protected branch name. Default: main. + * --ruleset-id Apply to an existing ruleset by id. If omitted, looks for a ruleset + * matching --branch by name (e.g. "Protect main") and uses the first hit. + * --required-checks Comma-separated context names. Default: build,test,checks,ci-complete. + * --strict Enable strict_required_status_checks_policy. Default: off (loose). + * --allow-bot-checks Keep contexts whose name looks like an LLM-review bot. Default: drop them. + * --extra-bot-patterns Comma-separated extra case-insensitive substrings to treat as bot. + * --apply PUT the patched ruleset. Without this flag, prints the diff and exits. + * --json Print the would-be PUT body to stdout (combine with --apply to also POST). + * --help Show this help. + * + * Environment: + * GH_TOKEN / GITHUB_TOKEN Optional. If unset, falls back to the locally-authenticated `gh` CLI. + * + * Examples: + * # Dry run — show what would change on the current repo's main branch + * apply-branch-protection + * + * # Apply the recommended defaults to a specific repo + * apply-branch-protection --repo protoLabsAI/protoMaker --branch main --apply + * + * # Custom required checks for a Rust monorepo + * apply-branch-protection --required-checks cargo-test,cargo-clippy,cargo-fmt --apply + * + * # Keep CodeRabbit as required (overrides the bot filter) + * apply-branch-protection --allow-bot-checks --required-checks build,test,checks,CodeRabbit --apply + */ + +import { execFile } from 'node:child_process'; +import { promisify } from 'node:util'; + +import { + DEFAULT_REQUIRED_CHECKS, + applyRecommendedDefaults, + stripReadOnlyFields, +} from '../lib/branch-protection.mjs'; + +const execFileAsync = promisify(execFile); + +if (process.argv.includes('--help') || process.argv.includes('-h')) { + await printHelp(); + process.exit(0); +} + +const args = parseArgs(process.argv.slice(2)); + +try { + await main(args); +} catch (err) { + console.error(`apply-branch-protection: ${err.message}`); + if (process.env.DEBUG) console.error(err.stack); + process.exit(1); +} + +async function main(opts) { + const repo = opts.repo ?? (await detectRepoFromGit()); + if (!repo) { + throw new Error( + 'Could not determine target repo. Pass --repo or run inside a git checkout with origin set.' + ); + } + const [owner, name] = repo.split('/'); + if (!owner || !name) { + throw new Error(`--repo must be in owner/name format, got "${repo}"`); + } + + const branch = opts.branch ?? 'main'; + const requiredChecks = opts.requiredChecks ?? DEFAULT_REQUIRED_CHECKS; + const extraBotPatterns = opts.extraBotPatterns ?? []; + + const rulesetId = opts.rulesetId ?? (await findRulesetIdForBranch(owner, name, branch)); + if (!rulesetId) { + throw new Error( + `No ruleset found for branch "${branch}" on ${repo}. Create one via the UI first, or pass --ruleset-id.` + ); + } + + const before = await ghApi(`repos/${owner}/${name}/rulesets/${rulesetId}`); + const { ruleset: after, diff } = applyRecommendedDefaults(before, { + requiredChecks, + strict: opts.strict, + excludeBots: !opts.allowBotChecks, + extraBotPatterns, + }); + + console.log(`Repo: ${repo}`); + console.log(`Ruleset: ${before.name ?? '(unnamed)'} (id ${rulesetId})`); + console.log(`Branch: ${branch}`); + console.log(''); + console.log('Diff:'); + if (diff.addedContexts.length === 0 && diff.removedContexts.length === 0) { + console.log(' required_status_checks: (no change)'); + } else { + if (diff.addedContexts.length > 0) { + console.log(` + ${diff.addedContexts.join(', ')}`); + } + if (diff.removedContexts.length > 0) { + console.log(` - ${diff.removedContexts.join(', ')}`); + } + } + if (diff.strictBefore !== diff.strictAfter) { + console.log(` strict_required_status_checks_policy: ${diff.strictBefore} → ${diff.strictAfter}`); + } + console.log(''); + + const body = stripReadOnlyFields(after); + + if (opts.json) { + console.log(JSON.stringify(body, null, 2)); + } + + if (!opts.apply) { + console.log('Dry run. Re-run with --apply to PUT this ruleset.'); + return; + } + + await ghApi(`repos/${owner}/${name}/rulesets/${rulesetId}`, { + method: 'PUT', + body, + }); + console.log(`Applied. ${repo} ruleset ${rulesetId} updated.`); +} + +// ── helpers ──────────────────────────────────────────────────────────────── + +function parseArgs(argv) { + const opts = {}; + for (let i = 0; i < argv.length; i++) { + const a = argv[i]; + const next = () => { + const v = argv[i + 1]; + if (v === undefined || v.startsWith('--')) { + throw new Error(`flag ${a} requires a value`); + } + i++; + return v; + }; + switch (a) { + case '--repo': + opts.repo = next(); + break; + case '--branch': + opts.branch = next(); + break; + case '--ruleset-id': + opts.rulesetId = Number(next()); + break; + case '--required-checks': + opts.requiredChecks = next() + .split(',') + .map((s) => s.trim()) + .filter(Boolean); + break; + case '--strict': + opts.strict = true; + break; + case '--allow-bot-checks': + opts.allowBotChecks = true; + break; + case '--extra-bot-patterns': + opts.extraBotPatterns = next() + .split(',') + .map((s) => s.trim()) + .filter(Boolean); + break; + case '--apply': + opts.apply = true; + break; + case '--json': + opts.json = true; + break; + default: + throw new Error(`unknown flag: ${a}`); + } + } + return opts; +} + +async function detectRepoFromGit() { + try { + const { stdout } = await execFileAsync('git', ['remote', 'get-url', 'origin'], { + encoding: 'utf8', + }); + const url = stdout.trim(); + // Handle both SSH (git@github.com:owner/name.git) and HTTPS (https://github.com/owner/name.git) + const m = url.match(/github\.com[:/]([^/]+)\/([^/]+?)(?:\.git)?$/); + return m ? `${m[1]}/${m[2]}` : null; + } catch { + return null; + } +} + +async function findRulesetIdForBranch(owner, name, branch) { + const rulesets = await ghApi(`repos/${owner}/${name}/rulesets`); + // Prefer an exact "Protect " match; fall back to first ruleset that + // mentions the branch in its name. + const exact = rulesets.find((r) => r.name === `Protect ${branch}`); + if (exact) return exact.id; + const loose = rulesets.find((r) => (r.name ?? '').toLowerCase().includes(branch.toLowerCase())); + return loose?.id ?? null; +} + +/** + * Thin wrapper around the locally-authenticated `gh api` CLI. Uses gh because + * it already handles auth, host overrides, and rate-limit retries — and every + * dev environment that runs release-tools also has gh installed. + */ +async function ghApi(endpoint, opts = {}) { + const args = ['api', endpoint]; + if (opts.method && opts.method !== 'GET') { + args.push('-X', opts.method); + } + if (opts.body) { + args.push('--input', '-'); + } + const { stdout } = await execFileAsync('gh', args, { + encoding: 'utf8', + input: opts.body ? JSON.stringify(opts.body) : undefined, + }); + return JSON.parse(stdout); +} + +async function printHelp() { + const fs = await import('node:fs'); + const url = await import('node:url'); + const self = url.fileURLToPath(import.meta.url); + const src = fs.readFileSync(self, 'utf8').split('\n'); + const header = []; + for (const line of src) { + if (line.startsWith('#!')) continue; + header.push(line); + if (line.startsWith(' */')) break; + } + console.log(header.join('\n')); +} diff --git a/docs/branch-protection-defaults.md b/docs/branch-protection-defaults.md new file mode 100644 index 0000000..18ed66a --- /dev/null +++ b/docs/branch-protection-defaults.md @@ -0,0 +1,156 @@ +# Branch Protection Defaults + +A protoLabs convention for branch protection rulesets on GitHub. Codified as +`bin/apply-branch-protection.mjs` so any repo in the org can adopt it with one +command. Pairs with the existing release-notes generator — same package, same +install, same versioning. + +## The two rules + +### 1. `required_status_checks` is for correctness, not advisory signals + +Required status checks should gate on whether **the code works**: build, +test, lint. They should NOT contain LLM review bots (CodeRabbit, protoquinn, +SonarCloud, etc.), because: + +- If the bot is rate-limited or down, every PR blocks until it recovers. +- If the bot's webhook races a fast-merging PR (CI finishes before the bot's + status arrives), the PR sits `mergeStateStatus: BLOCKED` indefinitely until + someone nudges it manually. +- Bots already have a legitimate veto path: a `CHANGES_REQUESTED` review + blocks merge via `reviewDecision`. That's the right level. Comments are + advisory, approvals are advisory, blocking happens explicitly. + +| Belongs in `required_status_checks` | Doesn't belong | +| ----------------------------------- | ------------------------------------------------------- | +| `build` | `CodeRabbit` (LLM advisory review) | +| `test` | `protoquinn[bot]` / any `[bot]` status | +| `checks` (lint/format) | `SonarCloud Code Analysis` | +| Repo-specific rollups (opt-in) | Any external service whose outage shouldn't halt merges | + +### 2. `strict_required_status_checks_policy: false` for fast-moving repos + +GitHub's "strict" mode (the default in the UI) forces a PR's branch to be +up-to-date with the base before merge. For solo-dev or small-team repos with +linear PR stacks, that's an N×CI-cycle drag: merge A → B is behind → rebase B +→ wait for CI → merge B → C is behind → repeat. A 5-PR stack can cost ~25 +minutes of CI-wait instead of ~5 for a single cycle. + +For repos with **1–3 contributors, linear sprint cadence, strong test +coverage**, flip strict to `false`. The actual safety net is the test suite — +stale branches that would break main usually fail their own CI before merge. + +Keep strict `true` on repos with **5+ contributors or frequent parallel +feature branches** where semantic conflicts between stale branches are a real +risk. + +## What this PRESERVES + +Even with `strict: false`: + +- PR required before merging to `main` ✓ +- All required status checks must pass on the PR's last CI run ✓ +- Force-push to `main` blocked ✓ +- Branch deletion blocked ✓ +- Auto-delete head branch on merge ✓ + +What changes: + +- `mergeState: BEHIND` no longer blocks merge. +- The PR can merge whenever its **own** CI is green, regardless of how far + `main` has advanced since the PR opened. + +## Usage + +Install once: + +```bash +npm install -g @protolabsai/release-tools +# or run via npx +npx @protolabsai/release-tools apply-branch-protection --help +``` + +### Dry run (default — safe, prints diff and exits) + +```bash +apply-branch-protection --repo protoLabsAI/myrepo --branch main +``` + +### Apply with defaults (loose + correctness-only + bots filtered) + +```bash +apply-branch-protection --repo protoLabsAI/myrepo --branch main --apply +``` + +### Custom required checks + +```bash +# Rust monorepo +apply-branch-protection \ + --repo protoLabsAI/myrepo \ + --required-checks cargo-test,cargo-clippy,cargo-fmt \ + --apply + +# Add a repo-specific rollup +apply-branch-protection \ + --required-checks build,test,checks,ci-complete \ + --apply +``` + +### Strict mode (5+ contributors / parallel work) + +```bash +apply-branch-protection --branch main --strict --apply +``` + +### Allow bot status checks (opt-out of the bot filter) + +You almost never want this. The flag exists for repos that depend on a +non-LLM bot (e.g. a custom security scanner that exposes itself as a +`[bot]` status), and you've decided it really is a correctness gate. + +```bash +apply-branch-protection \ + --required-checks build,test,checks,security-scanner \ + --allow-bot-checks \ + --apply +``` + +### Extra bot patterns for repo-specific advisory bots + +```bash +apply-branch-protection \ + --extra-bot-patterns acme-llm-reviewer,custom-ai-bot \ + --apply +``` + +## How it works + +1. Resolves the target repo from `--repo`, or from `git remote get-url origin`. +2. Finds the ruleset for the branch by name (`Protect `), or takes + `--ruleset-id` directly. Bails if no matching ruleset exists — create one + in the UI first. +3. Reads the ruleset, computes the recommended shape via + `lib/branch-protection.mjs`, and prints a diff. +4. Without `--apply`, exits. With `--apply`, PUTs the patched ruleset back. + +Read-only fields (`id`, `created_at`, `_links`, etc.) are stripped before the +PUT so GitHub doesn't 422. Known `integration_id` values for kept contexts +are preserved. + +## When NOT to use the defaults + +- **Heavily-contested `main` branches** (5+ contributors making parallel + changes that touch overlapping code). Strict mode catches semantic + conflicts the test suite would miss. +- **Repos where a bot really IS a correctness gate.** E.g. a security + scanner that must clear before deploy. Use `--allow-bot-checks` and + list it explicitly in `--required-checks`. +- **Repos with regulatory requirements** that mandate specific gating + behavior. The defaults are calibrated for fast iteration, not compliance. + +## Related issues + +- [release-tools#6](https://github.com/protoLabsAI/release-tools/issues/6) — original strict/loose recommendation +- [release-tools#10](https://github.com/protoLabsAI/release-tools/issues/10) — bot-checks exclusion +- [protoMaker#3745](https://github.com/protoLabsAI/protoMaker/issues/3745) — first applied case (CodeRabbit dropped from `Protect main`) diff --git a/lib/branch-protection.mjs b/lib/branch-protection.mjs new file mode 100644 index 0000000..6cfc62e --- /dev/null +++ b/lib/branch-protection.mjs @@ -0,0 +1,204 @@ +/** + * @license + * Copyright 2026 protoLabs + * SPDX-License-Identifier: Apache-2.0 + * + * Branch protection ruleset utilities. + * + * Two recommendations are codified here: + * + * 1. Bot reviewers do not belong in `required_status_checks`. Treat them as + * reviewers (they gate via `reviewDecision`), not as CI. Required status + * checks should reflect correctness signals only — build, test, lint. + * See protoLabsAI/release-tools#10. + * + * 2. `strict_required_status_checks_policy` should default to `false` for + * fast-moving, low-conflict repos with linear PR stacks. Strict mode + * forces an N×CI-cycle drag on stacked work. See + * protoLabsAI/release-tools#6. + * + * The exported functions are pure: they take a ruleset JSON shape, return a + * new (or diff) ruleset JSON shape. The thin CLI in `bin/apply-branch-protection.mjs` + * is responsible for I/O (fetching from GitHub, writing back). + */ + +/** + * Default contexts the script recommends as required. Correctness only. + * + * Intentionally minimal — `build`, `test`, and `checks` (lint/format) are + * the universal correctness signals. Repo-specific rollups like + * `ci-complete` should be added explicitly via `--required-checks` because + * not every repo emits them and forcing them silently would BLOCK PRs in + * repos that don't ship that workflow. + */ +export const DEFAULT_REQUIRED_CHECKS = ['build', 'test', 'checks']; + +/** + * Substrings that identify a context as a bot reviewer rather than a CI check. + * Matched case-insensitively against the context name. Extend via the + * `extraBotPatterns` argument on `filterBotChecks`. + */ +export const KNOWN_BOT_PATTERNS = [ + '[bot]', + 'coderabbit', + 'protoquinn', + 'protoava', + 'protojon', + 'sonarcloud', // posts review-style status — treat as advisory +]; + +/** + * Decide whether a status-check context name looks like a bot reviewer. + * + * @param {string} context + * @param {string[]} [extraPatterns] + * @returns {boolean} + */ +export function isBotContext(context, extraPatterns = []) { + if (!context) return false; + const lower = String(context).toLowerCase(); + return [...KNOWN_BOT_PATTERNS, ...extraPatterns].some((p) => + lower.includes(String(p).toLowerCase()) + ); +} + +/** + * Filter a list of required_status_checks entries, dropping any entry whose + * context name matches a known bot pattern. + * + * @param {Array<{ context: string, integration_id?: number }>} checks + * @param {{ extraBotPatterns?: string[] }} [opts] + * @returns {{ kept: typeof checks, removed: typeof checks }} + */ +export function filterBotChecks(checks, opts = {}) { + const extra = opts.extraBotPatterns ?? []; + const kept = []; + const removed = []; + for (const c of checks ?? []) { + if (isBotContext(c.context, extra)) { + removed.push(c); + } else { + kept.push(c); + } + } + return { kept, removed }; +} + +/** + * Merge a desired list of context names into the existing required_status_checks + * entries, preserving any `integration_id` that's already known for that + * context and inserting bare entries for any new ones. + * + * @param {Array<{ context: string, integration_id?: number }>} existing + * @param {string[]} desiredContexts + * @returns {Array<{ context: string, integration_id?: number }>} + */ +export function mergeRequiredChecks(existing, desiredContexts) { + const existingByContext = new Map((existing ?? []).map((c) => [c.context, c])); + return desiredContexts.map((context) => { + const prev = existingByContext.get(context); + return prev ?? { context }; + }); +} + +/** + * Apply the recommended defaults to a ruleset JSON shape. + * + * @param {object} ruleset - The full ruleset object from + * `GET /repos/{owner}/{repo}/rulesets/{id}`. + * @param {object} [opts] + * @param {string[]} [opts.requiredChecks] - Context names to require. Default DEFAULT_REQUIRED_CHECKS. + * @param {boolean} [opts.strict] - strict_required_status_checks_policy. Default false. + * @param {boolean} [opts.excludeBots] - Drop bot-pattern contexts. Default true. + * @param {string[]} [opts.extraBotPatterns] - Additional case-insensitive substrings to treat as bot. + * @returns {{ ruleset: object, diff: { addedContexts: string[], removedContexts: string[], strictBefore: boolean|null, strictAfter: boolean } }} + */ +export function applyRecommendedDefaults(ruleset, opts = {}) { + const requiredChecks = opts.requiredChecks ?? DEFAULT_REQUIRED_CHECKS; + const strict = opts.strict ?? false; + const excludeBots = opts.excludeBots ?? true; + const extraBotPatterns = opts.extraBotPatterns ?? []; + + // Deep clone so callers can compare before/after without aliasing. + const next = JSON.parse(JSON.stringify(ruleset ?? {})); + if (!Array.isArray(next.rules)) next.rules = []; + + let rule = next.rules.find((r) => r.type === 'required_status_checks'); + if (!rule) { + rule = { + type: 'required_status_checks', + parameters: { + do_not_enforce_on_create: true, + required_status_checks: [], + strict_required_status_checks_policy: false, + }, + }; + next.rules.push(rule); + } + if (!rule.parameters) rule.parameters = {}; + const params = rule.parameters; + const beforeChecks = params.required_status_checks ?? []; + const beforeStrict = + typeof params.strict_required_status_checks_policy === 'boolean' + ? params.strict_required_status_checks_policy + : null; + + // Step 1: merge the desired context list (preserving known integration_ids). + let merged = mergeRequiredChecks(beforeChecks, requiredChecks); + + // Step 2: drop bot patterns from anything that survived. Important if a + // caller passes a custom requiredChecks list that happens to include bots. + let removed = []; + if (excludeBots) { + const filtered = filterBotChecks(merged, { extraBotPatterns }); + merged = filtered.kept; + removed = filtered.removed; + } + + params.required_status_checks = merged; + params.strict_required_status_checks_policy = strict; + + const beforeContexts = beforeChecks.map((c) => c.context); + const afterContexts = merged.map((c) => c.context); + const addedContexts = afterContexts.filter((c) => !beforeContexts.includes(c)); + const removedContexts = [ + // Anything dropped because it was a bot, plus anything that wasn't in the + // new desired list. + ...removed.map((c) => c.context), + ...beforeContexts.filter((c) => !afterContexts.includes(c) && !removed.some((r) => r.context === c)), + ]; + + return { + ruleset: next, + diff: { + addedContexts, + removedContexts, + strictBefore: beforeStrict, + strictAfter: strict, + }, + }; +} + +/** + * Strip writable-only fields from a ruleset so the result is acceptable as + * the body of `PUT /repos/{owner}/{repo}/rulesets/{id}`. GitHub rejects + * read-only fields like `id`, `_links`, `current_user_can_bypass`, etc. + * + * @param {object} ruleset + * @returns {object} + */ +export function stripReadOnlyFields(ruleset) { + const READ_ONLY = [ + 'id', + 'node_id', + 'created_at', + 'updated_at', + 'source_type', + 'source', + 'current_user_can_bypass', + '_links', + ]; + const copy = JSON.parse(JSON.stringify(ruleset)); + for (const k of READ_ONLY) delete copy[k]; + return copy; +} diff --git a/package.json b/package.json index 503a8d5..b628dfd 100644 --- a/package.json +++ b/package.json @@ -18,7 +18,8 @@ "bin": { "rewrite-release-notes": "./bin/rewrite-release-notes.mjs", "build-updater-manifest": "./bin/build-updater-manifest.mjs", - "review-code": "./bin/review-code.mjs" + "review-code": "./bin/review-code.mjs", + "apply-branch-protection": "./bin/apply-branch-protection.mjs" }, "files": [ "bin/", @@ -34,7 +35,7 @@ }, "scripts": { "lint": "eslint .", - "smoke": "node bin/rewrite-release-notes.mjs --help && node bin/build-updater-manifest.mjs --help && node bin/review-code.mjs --help", + "smoke": "node bin/rewrite-release-notes.mjs --help && node bin/build-updater-manifest.mjs --help && node bin/review-code.mjs --help && node bin/apply-branch-protection.mjs --help", "test": "node --test test/*.mjs" }, "devDependencies": { diff --git a/test/branch-protection.test.mjs b/test/branch-protection.test.mjs new file mode 100644 index 0000000..aeb5d9d --- /dev/null +++ b/test/branch-protection.test.mjs @@ -0,0 +1,187 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { + DEFAULT_REQUIRED_CHECKS, + applyRecommendedDefaults, + filterBotChecks, + isBotContext, + mergeRequiredChecks, + stripReadOnlyFields, +} from '../lib/branch-protection.mjs'; + +test('isBotContext detects [bot] suffix', () => { + assert.equal(isBotContext('coderabbitai[bot]'), true); + assert.equal(isBotContext('protoquinn[bot]'), true); +}); + +test('isBotContext detects known bot vendor names case-insensitively', () => { + assert.equal(isBotContext('CodeRabbit'), true); + assert.equal(isBotContext('codeRABBIT review'), true); + assert.equal(isBotContext('protoquinn / QA'), true); + assert.equal(isBotContext('SonarCloud Code Analysis'), true); +}); + +test('isBotContext leaves real CI context names alone', () => { + for (const ctx of ['build', 'test', 'checks', 'ci-complete', 'lint', 'typecheck', 'e2e']) { + assert.equal(isBotContext(ctx), false, `expected ${ctx} not to match`); + } +}); + +test('isBotContext supports extraPatterns for repo-specific bots', () => { + assert.equal(isBotContext('custom-llm-review', ['custom-llm']), true); + assert.equal(isBotContext('build', ['custom-llm']), false); +}); + +test('isBotContext is conservative — empty / null / undefined return false', () => { + assert.equal(isBotContext(''), false); + assert.equal(isBotContext(null), false); + assert.equal(isBotContext(undefined), false); +}); + +test('filterBotChecks separates bots from real checks', () => { + const input = [ + { context: 'build', integration_id: 1 }, + { context: 'test', integration_id: 1 }, + { context: 'CodeRabbit', integration_id: 999 }, + { context: 'checks', integration_id: 1 }, + ]; + const { kept, removed } = filterBotChecks(input); + assert.deepEqual( + kept.map((c) => c.context), + ['build', 'test', 'checks'] + ); + assert.deepEqual( + removed.map((c) => c.context), + ['CodeRabbit'] + ); +}); + +test('mergeRequiredChecks preserves integration_id for known contexts', () => { + const existing = [ + { context: 'build', integration_id: 15368 }, + { context: 'test', integration_id: 15368 }, + ]; + const desired = ['build', 'test', 'checks', 'ci-complete']; + const merged = mergeRequiredChecks(existing, desired); + assert.equal(merged.length, 4); + assert.deepEqual(merged[0], { context: 'build', integration_id: 15368 }); + assert.deepEqual(merged[1], { context: 'test', integration_id: 15368 }); + // New contexts have no integration_id — GitHub resolves at apply time. + assert.deepEqual(merged[2], { context: 'checks' }); + assert.deepEqual(merged[3], { context: 'ci-complete' }); +}); + +test('applyRecommendedDefaults drops bot checks and sets strict false by default', () => { + const ruleset = { + id: 12552305, + name: 'Protect main', + rules: [ + { + type: 'required_status_checks', + parameters: { + do_not_enforce_on_create: true, + required_status_checks: [ + { context: 'build', integration_id: 15368 }, + { context: 'test', integration_id: 15368 }, + { context: 'checks', integration_id: 15368 }, + { context: 'CodeRabbit', integration_id: 347564 }, + ], + strict_required_status_checks_policy: true, + }, + }, + ], + }; + + const { ruleset: next, diff } = applyRecommendedDefaults(ruleset); + + const rule = next.rules.find((r) => r.type === 'required_status_checks'); + assert.deepEqual( + rule.parameters.required_status_checks.map((c) => c.context), + DEFAULT_REQUIRED_CHECKS + ); + assert.equal(rule.parameters.strict_required_status_checks_policy, false); + + assert.equal(diff.strictBefore, true); + assert.equal(diff.strictAfter, false); + assert.ok(diff.removedContexts.includes('CodeRabbit')); +}); + +test('applyRecommendedDefaults respects custom required-checks list', () => { + const ruleset = { rules: [] }; + const { ruleset: next, diff } = applyRecommendedDefaults(ruleset, { + requiredChecks: ['build', 'lint', 'typecheck'], + }); + + const rule = next.rules.find((r) => r.type === 'required_status_checks'); + assert.deepEqual( + rule.parameters.required_status_checks.map((c) => c.context), + ['build', 'lint', 'typecheck'] + ); + assert.deepEqual(diff.addedContexts, ['build', 'lint', 'typecheck']); +}); + +test('applyRecommendedDefaults supports --strict opt-in', () => { + const ruleset = { rules: [] }; + const { ruleset: next, diff } = applyRecommendedDefaults(ruleset, { + requiredChecks: ['build'], + strict: true, + }); + const rule = next.rules.find((r) => r.type === 'required_status_checks'); + assert.equal(rule.parameters.strict_required_status_checks_policy, true); + assert.equal(diff.strictAfter, true); +}); + +test('applyRecommendedDefaults supports allow-bot-checks via excludeBots:false', () => { + // The CLI may set this when a caller has explicitly opted in via + // --allow-bot-checks. In that mode, a bot context that the user requested + // explicitly must survive. + const ruleset = { rules: [] }; + const { ruleset: next } = applyRecommendedDefaults(ruleset, { + requiredChecks: ['build', 'CodeRabbit'], + excludeBots: false, + }); + const rule = next.rules.find((r) => r.type === 'required_status_checks'); + assert.deepEqual( + rule.parameters.required_status_checks.map((c) => c.context), + ['build', 'CodeRabbit'] + ); +}); + +test('applyRecommendedDefaults creates the rule if missing', () => { + const ruleset = { rules: [] }; + const { ruleset: next } = applyRecommendedDefaults(ruleset, { + requiredChecks: ['build', 'test'], + }); + const rule = next.rules.find((r) => r.type === 'required_status_checks'); + assert.ok(rule, 'expected required_status_checks rule to be added'); + assert.deepEqual(rule.parameters.required_status_checks.map((c) => c.context), ['build', 'test']); +}); + +test('stripReadOnlyFields removes id and other server-only fields', () => { + const before = { + id: 12552305, + node_id: 'RR_xxx', + name: 'Protect main', + created_at: '2024-01-01', + updated_at: '2026-01-01', + source_type: 'Repository', + source: 'protoLabsAI/protoMaker', + current_user_can_bypass: 'always', + _links: { self: { href: '...' } }, + rules: [{ type: 'required_status_checks', parameters: {} }], + }; + const after = stripReadOnlyFields(before); + assert.equal(after.id, undefined); + assert.equal(after.node_id, undefined); + assert.equal(after.created_at, undefined); + assert.equal(after.updated_at, undefined); + assert.equal(after.source_type, undefined); + assert.equal(after.source, undefined); + assert.equal(after.current_user_can_bypass, undefined); + assert.equal(after._links, undefined); + assert.equal(after.name, 'Protect main'); + assert.ok(after.rules); + // Doesn't mutate the original + assert.equal(before.id, 12552305); +});