ci(gate): gate-verdict job enforces the GATE: PASS rule at the PR head (TASK-595)

.github/workflows/gate-verdict.yml

@@ -0,0 +1,44 @@
+name: Gate verdict
+
+# Enforces the seat-gate rule in docs/development/external-review.md: a PR that is
+# not docs-only or test-only needs a `GATE: PASS` from the gate account at its exact
+# head. The logic is scripts/gate-verdict.sh, unit-tested by
+# scripts/__tests__/gate-verdict.test.mjs. This file only says when to run it.
+#
+# The script and its tests are read from the PR checkout, so a PR that edits them
+# grades itself. Pin both to main's copy (a follow-up, not in #3051) before this
+# check is made required.
+#
+# Not a required check yet. Making it one is a repo-settings step for the operator.
+
+on:
+  pull_request:
+    types: [opened, synchronize, reopened, ready_for_review, edited]
+  pull_request_review:
+    types: [submitted, edited, dismissed]
+
+permissions:
+  contents: read
+  pull-requests: read
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.pull_request.number }}
+  cancel-in-progress: true
+
+jobs:
+  gate-verdict:
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          persist-credentials: false
+      - name: Script unit tests
+        run: node --test scripts/__tests__/gate-verdict.test.mjs
+      - name: Docs or tests only, or a seat gate PASS at this head
+        env:
+          GH_TOKEN: ${{ github.token }}
+          REPO: ${{ github.repository }}
+          PR_NUMBER: ${{ github.event.pull_request.number }}
+          HEAD_SHA: ${{ github.event.pull_request.head.sha }}
+        run: bash scripts/gate-verdict.sh

docs/development/external-review.md

@@ -93,14 +93,32 @@ can't tell us whether it's wrong or whether we are.
   as the second check. It reads each changed file in full context, including
   the callers and readers of whatever changed. For example, a new setting must
   have something that reads it. The verdict names the head SHA it checked out
-  and the commands it ran. A PASS without both does not count. Full suites
-  stay in CI, so run focused tests only.
+  and the commands it ran. `gate-verdict` enforces the SHA. The commands are not
+  machine-checked, so the reviewer lists them. Full suites stay in CI, so run
+  focused tests only.
 
   The writer names the tier in the PR body. A reviewer can raise it but
   never lower it. A seat verdict is a review from the seat account at the exact head
-  whose last line is `GATE: PASS`, and the press tooling reads only those. A
-  CodeRabbit review is never a verdict. It only lets a docs-only or test-only
-  PR skip the seat gate.
+  whose first line opens with `GATE: PASS` and names the head's first seven characters,
+  and whose final non-blank line is exactly `GATE: PASS` as stored, with the credit line
+  above it (review-checklist rules 44 and 51). The press reads that line as stored, so a
+  footer or bold markup below the sentinel fails the PASS. `gate-verdict` finds a verdict
+  with footers and markup removed, at either end, so a `GATE: HOLD` at either end blocks a
+  PASS before it at the same head, and a footer under the HOLD does not hide it. A
+  CodeRabbit review is never a verdict. It only lets a docs-only or test-only PR skip the
+  seat gate.
+
+- **`gate-verdict` checks this rule on every PR head.** `.github/workflows/gate-verdict.yml`
+  runs `scripts/gate-verdict.sh`. A PR passes as Tier 0 when every changed file is docs
+  or test code and the file list covers `changed_files`. A rename counts both its old
+  and its new path. Outside `docs/`, `SKILL.md` files, skill trees, `k8s/helm/commonly/configs/`,
+  and `CLAUDE.md`, `AGENTS.md` and `REVIEW.md` are behaviour, not docs. Any other PR
+  needs a verdict from the gate account at its exact head whose first line opens with
+  `GATE: PASS` and names the head SHA, and whose final non-blank line, as stored, is `GATE: PASS`. The latest
+  verdict at that head wins. The job does not count the Tier 2 second
+  gate. It runs the script from the PR head, so a PR that edits the script grades itself
+  until the script is pinned to main, and it is not a required check until the operator
+  makes it one.
 
 ## Keeping a PR out of automatic review
 

scripts/__tests__/gate-verdict.test.mjs

@@ -0,0 +1,292 @@
+import test, { after } from 'node:test';
+import assert from 'node:assert/strict';
+import { spawnSync } from 'node:child_process';
+import { chmodSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { dirname, join } from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+// Runs scripts/gate-verdict.sh with a fake `gh` on PATH. The fake answers the three REST
+// calls from JSON fixtures and applies the script's own --jq filter with jq, so the
+// filter the workflow ships is the one under test.
+const SCRIPT = join(dirname(fileURLToPath(import.meta.url)), '..', 'gate-verdict.sh');
+const HEAD = 'a'.repeat(40);
+const OLD = 'b'.repeat(40);
+
+const FAKE_GH = `#!/usr/bin/env bash
+set -euo pipefail
+if [ -n "\${FAKE_GH_FAIL:-}" ]; then echo "HTTP 500" >&2; exit 1; fi
+path=""; expr=""
+while [ $# -gt 0 ]; do
+  case "$1" in
+    api|--paginate) shift ;;
+    --jq) expr="$2"; shift 2 ;;
+    *) path="$1"; shift ;;
+  esac
+done
+case "$path" in
+  */files) fixture="$FAKE_FILES" ;;
+  */reviews) fixture="$FAKE_REVIEWS" ;;
+  */pulls/[0-9]*) fixture="$FAKE_PR" ;;
+  *) echo "unexpected path: $path" >&2; exit 2 ;;
+esac
+jq -r "$expr" "$fixture"
+`;
+
+const bin = mkdtempSync(join(tmpdir(), 'gate-verdict-gh-'));
+writeFileSync(join(bin, 'gh'), FAKE_GH);
+chmodSync(join(bin, 'gh'), 0o755);
+after(() => rmSync(bin, { recursive: true, force: true }));
+
+let nextId = 1000;
+const review = (body, { login = 'lilyshen0722', commit = HEAD, state = 'COMMENTED' } = {}) =>
+  ({ id: nextId++, user: { login }, commit_id: commit, state, submitted_at: '2026-10-31T07:30:00Z', body });
+const PASS = `GATE: PASS @ ${HEAD}\n\nRan the focused tests.\n\nGATE: PASS`;
+const FAIL = 'Found a defect in the rollback path.\n\nGATE: FAIL';
+const HOLD = 'The rollback path needs a test.\n\nGATE: HOLD';
+
+const CODE_FILES = ['backend/services/byoHandoffService.ts'];
+const TIER0_FILES = [
+  'docs/development/external-review.md',
+  'backend/__tests__/unit/services/byoHandoffService.test.ts',
+  'docs/adr/ADR-011-shell-first-pre-gtm.md',
+];
+
+// A changed file is a path, or a { filename, previous_filename } pair for a rename.
+const entry = (file) => (typeof file === 'string' ? { filename: file } : file);
+
+function run({ files = [], changedFiles = files.length, reviews = [], env = {} } = {}) {
+  const fixtures = mkdtempSync(join(tmpdir(), 'gate-verdict-fx-'));
+  try {
+    const filesPath = join(fixtures, 'files.json');
+    const reviewsPath = join(fixtures, 'reviews.json');
+    const prPath = join(fixtures, 'pr.json');
+    writeFileSync(filesPath, JSON.stringify(files.map(entry)));
+    writeFileSync(reviewsPath, JSON.stringify(reviews));
+    writeFileSync(prPath, JSON.stringify({ changed_files: changedFiles }));
+    return spawnSync('bash', [SCRIPT], {
+      encoding: 'utf8',
+      env: {
+        PATH: `${bin}:${process.env.PATH}`,
+        REPO: 'Team-Commonly/commonly',
+        PR_NUMBER: '3050',
+        HEAD_SHA: HEAD,
+        FAKE_FILES: filesPath,
+        FAKE_REVIEWS: reviewsPath,
+        FAKE_PR: prPath,
+        ...env,
+      },
+    });
+  } finally {
+    rmSync(fixtures, { recursive: true, force: true });
+  }
+}
+
+test('docs and test files only are Tier 0: no verdict needed', () => {
+  const r = run({ files: TIER0_FILES });
+  assert.equal(r.status, 0, r.stderr);
+  assert.match(r.stdout, /Tier 0/);
+});
+
+test('a dot-directory markdown file is not docs: it needs a gate', () => {
+  assert.equal(run({ files: ['.github/PULL_REQUEST_TEMPLATE.md', '.claude/skills/x/SKILL.md'] }).status, 1);
+});
+
+test('a dot-directory under docs is not docs either: it needs a gate', () => {
+  // Built from parts, as docReferences.test.js builds its fixtures: a literal doc path here would
+  // be a pointer that the doc-reference scan follows to a file that is not there.
+  assert.equal(run({ files: [['docs', '.claude', 'skills', 'x', 'SKILL.md'].join('/')] }).status, 1);
+  assert.equal(run({ files: [['docs', '.hidden', 'notes.md'].join('/')] }).status, 1);
+});
+
+test('one code file among docs and tests still needs a gate', () => {
+  const r = run({ files: [...TIER0_FILES, ...CODE_FILES] });
+  assert.equal(r.status, 1);
+  assert.match(r.stdout, /no GATE verdict from lilyshen0722/);
+});
+
+test('an empty file list is not Tier 0', () => {
+  assert.equal(run({ files: [] }).status, 1);
+});
+
+test('a code PR with no verdict is blocked', () => {
+  const r = run({ files: CODE_FILES });
+  assert.equal(r.status, 1);
+  assert.match(r.stdout, /no GATE verdict/);
+});
+
+test('a GATE: PASS from the gate account at the head passes', () => {
+  const r = run({ files: CODE_FILES, reviews: [review(PASS)] });
+  assert.equal(r.status, 0, r.stderr);
+  assert.match(r.stdout, new RegExp(`GATE: PASS at ${HEAD}`));
+});
+
+test('the account match ignores case', () => {
+  assert.equal(run({ files: CODE_FILES, reviews: [review(PASS, { login: 'LilyShen0722' })] }).status, 0);
+});
+
+test('a PASS at an older head does not count', () => {
+  assert.equal(run({ files: CODE_FILES, reviews: [review(PASS, { commit: OLD })] }).status, 1);
+});
+
+test('a PASS from another account does not count, CodeRabbit included', () => {
+  const reviews = [review(PASS, { login: 'coderabbitai' }), review(PASS, { login: 'sharpen-bot' })];
+  assert.equal(run({ files: CODE_FILES, reviews }).status, 1);
+});
+
+test('GATE: PASS must be the last line of the review', () => {
+  const body = `GATE: PASS @ ${HEAD}\n\nRan the focused tests.\n\nGATE: PASS\n\nFollow-up: none.`;
+  assert.equal(run({ files: CODE_FILES, reviews: [review(body)] }).status, 1);
+});
+
+test('trailing spaces and CRLF on the last line are ignored', () => {
+  const body = `GATE: PASS @ ${HEAD}\r\n\r\nRan the tests\r\n\r\nGATE: PASS  \r\n`;
+  assert.equal(run({ files: CODE_FILES, reviews: [review(body)] }).status, 0);
+});
+
+test('a FAIL after a PASS at the same head blocks it', () => {
+  const r = run({ files: CODE_FILES, reviews: [review(PASS), review(FAIL)] });
+  assert.equal(r.status, 1);
+  assert.match(r.stdout, /last line is 'GATE: FAIL'/);
+});
+
+test('a PASS after a FAIL at the same head passes', () => {
+  assert.equal(run({ files: CODE_FILES, reviews: [review(FAIL), review(PASS)] }).status, 0);
+});
+
+test('a HOLD after a PASS at the same head blocks it', () => {
+  const r = run({ files: CODE_FILES, reviews: [review(PASS), review(HOLD)] });
+  assert.equal(r.status, 1);
+  assert.match(r.stdout, /last line is 'GATE: HOLD'/);
+});
+
+test('a PASS after a HOLD at the same head passes', () => {
+  assert.equal(run({ files: CODE_FILES, reviews: [review(HOLD), review(PASS)] }).status, 0);
+});
+
+test('markup on the first line is ignored, but a bold last line is not the sentinel', () => {
+  const body = `**GATE: PASS @ ${HEAD}**\n\nRan the tests.\n\n**GATE: PASS**`;
+  const r = run({ files: CODE_FILES, reviews: [review(body)] });
+  assert.equal(r.status, 1);
+  assert.match(r.stdout, /last line is '\*\*GATE: PASS\*\*'/);
+});
+
+test('a credit line below the sentinel fails the PASS; the same line above it passes', () => {
+  const below = `GATE: PASS @ ${HEAD}\n\nRan the focused tests.\n\nGATE: PASS\n\nWritten by Sable, a Commonly agent`;
+  const r = run({ files: CODE_FILES, reviews: [review(below)] });
+  assert.equal(r.status, 1);
+  assert.match(r.stdout, /last line is 'Written by Sable, a Commonly agent'/);
+  const above = `GATE: PASS @ ${HEAD}\n\nRan the focused tests.\n\nWritten by Sable, a Commonly agent\n\nGATE: PASS`;
+  assert.equal(run({ files: CODE_FILES, reviews: [review(above)] }).status, 0);
+});
+
+test('a footer under a HOLD does not hide it from the PASS before it', () => {
+  const pass = review(PASS);
+  const hold = review('The rollback path needs a test.\n\nGATE: HOLD\n\nPod thread: https://commonly.me/v2/pods/6a692a1be833c668acdb84cf');
+  const r = run({ files: CODE_FILES, reviews: [pass, hold] });
+  assert.equal(r.status, 1);
+  assert.match(r.stdout, new RegExp(`review ${hold.id}\\)`));
+});
+
+test('only the exact last line GATE: PASS passes, so near-misses block', () => {
+  for (const last of ['GATE: PASSED', 'GATE:PASS', 'gate: pass', 'GATE: PASS (with notes)']) {
+    const body = `GATE: PASS @ ${HEAD}\n\nRan the focused tests.\n\n${last}`;
+    const r = run({ files: CODE_FILES, reviews: [review(body)] });
+    assert.equal(r.status, 1, last);
+  }
+});
+
+test('a dismissed PASS does not count', () => {
+  assert.equal(run({ files: CODE_FILES, reviews: [review(PASS, { state: 'DISMISSED' })] }).status, 1);
+});
+
+test('a bare PASS that names no head SHA does not count', () => {
+  const r = run({ files: CODE_FILES, reviews: [review('GATE: PASS\n\nRan the focused tests.\n\nGATE: PASS')] });
+  assert.equal(r.status, 1);
+  assert.match(r.stdout, /first line is 'GATE: PASS'/);
+});
+
+test('a first line that declares HOLD blocks a PASS on the last line', () => {
+  const body = `GATE: HOLD @ ${HEAD}\n\nThe rollback path needs a test.\n\nGATE: PASS`;
+  const r = run({ files: CODE_FILES, reviews: [review(body)] });
+  assert.equal(r.status, 1);
+  assert.match(r.stdout, /first line is 'GATE: HOLD @/);
+});
+
+test('a HOLD whose last line is prose still blocks a PASS before it', () => {
+  const hold = `GATE: HOLD @ ${HEAD}\n\nThe rollback path needs a test.`;
+  const r = run({ files: CODE_FILES, reviews: [review(PASS), review(hold)] });
+  assert.equal(r.status, 1);
+  assert.match(r.stdout, /first line is 'GATE: HOLD @/);
+});
+
+test('a PASS whose last line is prose does not count', () => {
+  const body = `GATE: PASS @ ${HEAD}\n\nRan the focused tests.\n\nFollow-up: none.`;
+  const r = run({ files: CODE_FILES, reviews: [review(body)] });
+  assert.equal(r.status, 1);
+  assert.match(r.stdout, /last line is 'Follow-up: none\.'/);
+});
+
+test('the first line must say GATE: PASS as a word, so near-misses block', () => {
+  for (const first of ['GATE: PASSED', 'GATE:PASS', 'gate: pass', `Checked out ${HEAD}`]) {
+    const body = `${first} @ ${HEAD}\n\nRan the focused tests.\n\nGATE: PASS`;
+    assert.equal(run({ files: CODE_FILES, reviews: [review(body)] }).status, 1, first);
+  }
+});
+
+test('a PASS that names only an older SHA does not count', () => {
+  const r = run({ files: CODE_FILES, reviews: [review(`GATE: PASS @ ${OLD}\n\nRan the tests.\n\nGATE: PASS`)] });
+  assert.equal(r.status, 1);
+});
+
+test('a rename out of a code path into docs is not Tier 0', () => {
+  const doc = ['docs', 'old-service.md'].join('/');
+  const r = run({ files: [{ filename: doc, previous_filename: 'backend/services/foo.ts' }] });
+  assert.equal(r.status, 1);
+  assert.match(r.stdout, /no GATE verdict/);
+});
+
+test('a rename out of a code path into a test directory is not Tier 0', () => {
+  const files = [{ filename: 'backend/services/__tests__/foo.ts', previous_filename: 'backend/services/foo.ts' }];
+  assert.equal(run({ files }).status, 1);
+});
+
+test('a rename within docs stays Tier 0', () => {
+  const [oldDoc, newDoc] = [['docs', 'old.md'], ['docs', 'new.md']].map((parts) => parts.join('/'));
+  assert.equal(run({ files: [{ filename: newDoc, previous_filename: oldDoc }] }).status, 0);
+});
+
+test('skill and config markdown is behaviour, not docs', () => {
+  const behaviour = [
+    'backend/commonly-bundled-skills/github/SKILL.md',
+    'cli/skills/commonly/SKILL.md',
+    'plugins/commonly-agent-plugin/skills/commonly/SKILL.md',
+    'fleet-skills/testing/SKILL.md',
+    'k8s/helm/commonly/configs/SKILLS_CATALOG.md',
+  ];
+  for (const file of behaviour) {
+    assert.equal(run({ files: [file] }).status, 1, file);
+  }
+});
+
+test('agent instruction files are behaviour, not docs', () => {
+  for (const file of ['CLAUDE.md', 'AGENTS.md', 'REVIEW.md', 'backend/CLAUDE.md']) {
+    assert.equal(run({ files: [file] }).status, 1, file);
+  }
+});
+
+test('markdown outside the skill and config trees is still Tier 0, docs/skills included', () => {
+  const files = ['README.md', 'backend/TESTING.md', 'frontend/design-system/README.md', 'docs/skills/SKILLS_CATALOG.md'];
+  assert.equal(run({ files }).status, 0);
+});
+
+test('a file list shorter than changed_files is not Tier 0', () => {
+  const r = run({ files: TIER0_FILES, changedFiles: 4 });
+  assert.equal(r.status, 1);
+  assert.match(r.stdout, /no GATE verdict/);
+});
+
+test('an API failure fails closed', () => {
+  const r = run({ files: CODE_FILES, reviews: [review(PASS)], env: { FAKE_GH_FAIL: '1' } });
+  assert.notEqual(r.status, 0);
+});