From 9698511e858e7e2fad7d55499c7089f2943169fc Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Fri, 24 Jul 2026 15:22:57 +0800 Subject: [PATCH] fix: prevent review ledger merge conflicts --- .gitattributes | 4 + .github/workflows/ci.yml | 3 + AGENTS.md | 2 +- docs/branch-review-ledger.md | 2 + docs/codex-review-protocol.md | 2 +- package.json | 3 +- scripts/check-branch-review-ledger.mjs | 125 +++++++++++++++++++++++++ 7 files changed, 138 insertions(+), 3 deletions(-) create mode 100644 scripts/check-branch-review-ledger.mjs diff --git a/.gitattributes b/.gitattributes index 7fc3c1ecc..71e17286c 100644 --- a/.gitattributes +++ b/.gitattributes @@ -7,3 +7,7 @@ *.pdf binary *.png binary *.webp binary + +# Review records are append-only. Concurrent branches should retain both sets +# of rows instead of stopping on an add/add conflict at the shared table tail. +docs/branch-review-ledger.md merge=union diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 680b02f32..9be3e7e74 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -202,6 +202,9 @@ jobs: - name: Gate-manifest self-test run: npm run check:gate-manifest + - name: Branch review ledger integrity + run: npm run check:branch-review-ledger + - name: Codebase index coverage run: npm run docs:check-index diff --git a/AGENTS.md b/AGENTS.md index 3682fca06..50a3bd579 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -143,7 +143,7 @@ Review routing: - `branch-cleanup`: Use only when the prompt explicitly asks for branch cleanup/hygiene or branch deletion candidates. Apply `docs/branch-cleanup-guide.md` and the review ledger before inspecting branch diffs. - `pr-ci-fix`: Confirmation-required for this repo. GitHub/GitLab API calls, PR comments, CI reruns, commits, and pushes require explicit user approval and must respect the upload/handoff rules. Exception: an explicit `Run PR` sweep carries this approval (see "## Run PR shortcut"). -When a branch or PR review completes, record the reviewed branch/ref, HEAD SHA, date, scope, outcome, and checks in `docs/branch-review-ledger.md`. +When a branch or PR review completes, append the reviewed branch/ref, HEAD SHA, date, scope, outcome, and checks to `docs/branch-review-ledger.md`. The ledger is append-only: never edit or delete an existing record; append a correction or superseding record instead. Its `merge=union` attribute preserves concurrent appends, and `npm run check:branch-review-ledger` blocks conflict markers, exact duplicate records, or loss of that merge protection. diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index d3f05d9f8..f63ed7ff1 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -2,6 +2,8 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD has not changed. +This file is append-only. Never rewrite or delete an existing review record; append a correction or superseding record instead. Git uses the `union` merge driver for this file so concurrent appended records are retained automatically. After merging, keep all distinct records and remove exact duplicates only. + ## Lookup Procedure 1. Identify the target branch or ref from the user request. If no target is named, use the current branch. diff --git a/docs/codex-review-protocol.md b/docs/codex-review-protocol.md index fda9de1db..5117780b7 100644 --- a/docs/codex-review-protocol.md +++ b/docs/codex-review-protocol.md @@ -30,7 +30,7 @@ Use this protocol for every Codex review, audit, bug hunt, PR review, release-re - During an automatic resolve task, work only existing unresolved Codex threads. Do not start a new review, add standalone findings, or request another review. - After fixing or fully dispositioning a thread, start the reply with ``; the workflow will close that exact thread. Do not use the marker when human input or new authorization is required, and leave that blocked thread open with a concise reason. - Ask before any OpenAI, Supabase, GitHub/GitLab, hosted CI, or provider-backed workflow. -- After any completed branch/PR review, update `docs/branch-review-ledger.md` with date, branch/ref, HEAD, scope, outcome, and checks. This ledger append is allowed even during a pure review. +- After any completed branch/PR review, append to `docs/branch-review-ledger.md` with date, branch/ref, HEAD, scope, outcome, and checks. The ledger is append-only: never edit or delete an existing record; append a correction or superseding record instead. This ledger append is allowed even during a pure review. ## Severity Guide diff --git a/package.json b/package.json index 9335c003a..afaeb300c 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,7 @@ "test:e2e:visual": "node scripts/run-playwright.mjs --config=playwright.visual.config.ts", "test:cross-tenant:staging": "node scripts/run-tsx.mjs scripts/test-cross-tenant-staging.ts", "verify:cheap": "node scripts/run-heavy.mjs --npm-script verify:cheap:internal", - "verify:cheap:internal": "npm run check:runtime && npm run check:github-actions && npm run check:ci-scope && npm run check:ci-triage && npm run check:pr-policy && npm run check:gate-manifest && npm run sitemap:check && npm run docs:check-index && npm run check:knip && npm run check:maintainability-budgets && npm run brand:check && npm run check:therapy-data-index && npm run check:type-scale && npm run check:icon-scale && npm run check:design-system-contract && npm run check:migration-role && npm run check:function-grants && npm run check:owner-scope && npm run lint && npm run typecheck && npm run test", + "verify:cheap:internal": "npm run check:runtime && npm run check:github-actions && npm run check:ci-scope && npm run check:ci-triage && npm run check:pr-policy && npm run check:gate-manifest && npm run check:branch-review-ledger && npm run sitemap:check && npm run docs:check-index && npm run check:knip && npm run check:maintainability-budgets && npm run brand:check && npm run check:therapy-data-index && npm run check:type-scale && npm run check:icon-scale && npm run check:design-system-contract && npm run check:migration-role && npm run check:function-grants && npm run check:owner-scope && npm run lint && npm run typecheck && npm run test", "verify:pr-local": "node scripts/verify-pr-local.mjs", "verify:ui": "npm run check:runtime && npm run test:e2e:pr", "verify:release": "npm run check:runtime && npm run lint && npm run typecheck && npm run test && npm run build && npm run test:e2e && npm run check:production-readiness && npm run governance:release && npm run eval:quality:release", @@ -58,6 +58,7 @@ "check:ci-scope": "node scripts/ci-change-scope.mjs --self-test", "check:ci-triage": "node scripts/ci-triage.mjs --self-test", "check:gate-manifest": "node scripts/check-gate-manifest.mjs", + "check:branch-review-ledger": "node scripts/check-branch-review-ledger.mjs --self-test && node scripts/check-branch-review-ledger.mjs", "check:pr-policy": "node scripts/pr-policy.mjs --self-test && node scripts/check-pr-policy-workflow.mjs", "check:env-parity": "node scripts/check-env-parity.mjs", "sweep:branch-ledger": "node scripts/sweep-branch-ledger.mjs", diff --git a/scripts/check-branch-review-ledger.mjs b/scripts/check-branch-review-ledger.mjs new file mode 100644 index 000000000..7fc8cc126 --- /dev/null +++ b/scripts/check-branch-review-ledger.mjs @@ -0,0 +1,125 @@ +#!/usr/bin/env node +/** + * Prevent the append-only branch review ledger from becoming a recurring merge + * hazard. The union driver preserves concurrent appends; this gate catches a + * missing attribute, accidentally committed conflict markers, and exact duplicate + * records before they can land on the shared branch. + */ +import { execFileSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const root = path.join(path.dirname(fileURLToPath(import.meta.url)), ".."); +const LEDGER_PATH = "docs/branch-review-ledger.md"; +const PROTOCOL_PATH = "docs/codex-review-protocol.md"; + +const conflictMarker = /^(?:<{7}(?: .*)?|={7}|>{7}(?: .*)?)\r?$/gm; +const datedTableRow = /^\| \d{4}-\d{2}-\d{2} \|/; + +export function validateLedger({ ledger, mergeAttribute, protocol }) { + const failures = []; + + if (mergeAttribute !== "union") { + failures.push(`${LEDGER_PATH} must resolve to merge=union (found ${JSON.stringify(mergeAttribute || "unset")}).`); + } + + const markers = [...ledger.matchAll(conflictMarker)]; + if (markers.length > 0) { + const lines = markers.map((match) => ledger.slice(0, match.index).split(/\r?\n/).length); + failures.push(`conflict marker(s) found at ledger line(s): ${lines.join(", ")}.`); + } + + const rows = ledger.split(/\r?\n/).filter((line) => datedTableRow.test(line)); + const seen = new Set(); + const duplicates = []; + for (const row of rows) { + if (seen.has(row)) duplicates.push(row); + seen.add(row); + } + if (duplicates.length > 0) { + failures.push(`${duplicates.length} exact duplicate review record(s) found.`); + } + + if (!ledger.includes("This file is append-only.")) { + failures.push(`${LEDGER_PATH} is missing its append-only editing contract.`); + } + if (!protocol.includes("The ledger is append-only:")) { + failures.push(`${PROTOCOL_PATH} is missing its append-only reviewer instruction.`); + } + + return { failures, recordCount: rows.length }; +} + +function effectiveMergeAttribute() { + const output = execFileSync("git", ["check-attr", "merge", "--", LEDGER_PATH], { + cwd: root, + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }).trim(); + return output.match(/:\s*merge:\s*(\S+)$/)?.[1] ?? ""; +} + +function assert(condition, label) { + if (!condition) throw new Error(`self-test failed: ${label}`); +} + +function selfTest() { + const valid = { + ledger: "# Ledger\n\nThis file is append-only.\n| 2026-07-24 | branch | head | scope | outcome | checks |\n", + mergeAttribute: "union", + protocol: "The ledger is append-only: append corrections.", + }; + + assert(validateLedger(valid).failures.length === 0, "valid ledger passes"); + assert( + validateLedger({ ...valid, mergeAttribute: "" }).failures.some((failure) => failure.includes("merge=union")), + "missing union attribute fails", + ); + assert( + validateLedger({ ...valid, ledger: `${valid.ledger}<<<<<<< ours\n` }).failures.some((failure) => + failure.includes("conflict marker"), + ), + "conflict marker fails", + ); + const row = "| 2026-07-24 | branch | head | scope | outcome | checks |"; + assert( + validateLedger({ ...valid, ledger: `This file is append-only.\n${row}\n${row}\n` }).failures.some((failure) => + failure.includes("duplicate"), + ), + "exact duplicate record fails", + ); + assert( + validateLedger({ ...valid, protocol: "append records" }).failures.some((failure) => + failure.includes("reviewer instruction"), + ), + "missing protocol contract fails", + ); + + console.log("branch-review-ledger self-test passed."); +} + +function main() { + if (process.argv.includes("--self-test")) { + selfTest(); + return; + } + + const result = validateLedger({ + ledger: readFileSync(path.join(root, LEDGER_PATH), "utf8"), + mergeAttribute: effectiveMergeAttribute(), + protocol: readFileSync(path.join(root, PROTOCOL_PATH), "utf8"), + }); + + if (result.failures.length > 0) { + console.error("Branch review ledger guard failed:"); + for (const failure of result.failures) console.error(`- ${failure}`); + process.exit(1); + } + + console.log( + `Branch review ledger guard passed: ${result.recordCount} table records, union merge active, no conflict markers or exact duplicates.`, + ); +} + +main();