fix(ci): gate policy check survives an API outage; merge master; drop NUL separators

Three fixes on the strict PR usefulness gate (#3698), plus the merge that
brings in the policy it enforces.

1. The mechanical policy check now outlives the model. The
   ANTHROPIC_API_KEY guard used to sit above detectPolicyMisses, so a PR
   with no intent paragraph and no screenshot got a green NEUTRAL skip
   whenever the key was absent or Anthropic was down — "wait for a 500"
   was a documented way past the one hard requirement. The #3745 branch
   now sits above the key guard and the spend guard: a policy miss is
   close-lane + the friendly fix-it comment + exit 1 with no API
   dependency at all. A compliant PR that hits a missing key or a dead
   API keeps the round-1 NEUTRAL behavior unchanged (loud comment,
   ::warning::, exit 0, stale gate:* labels cleared) — and the NEUTRAL
   comment now says plainly that the *usefulness verdict* did not run,
   while still reporting the title check and mechanical red flags it was
   able to compute without a model.

2. Merged origin/master, which carries #3745's CONTRIBUTING.md section
   and .github/pull_request_template.md. No conflicts: this branch never
   touched VERSION / package.json / CHANGELOG.md, so master's 0.42.72.1
   carried through untouched — the feature branch adds no version bump.
   The test's inlined pull_request_template fallback (only needed while
   the branch predated the merge) is gone; it now reads the real file, so
   growing the template's own prose past the 40-word bar fails here
   instead of silently letting an untouched template through. The
   CONTRIBUTING_URL deep link is pinned against a GitHub-style slug of
   every heading in the merged CONTRIBUTING.md, with the slugger itself
   pinned so it cannot "pass" against an anchor GitHub never generates.

3. hashInputs joined its three fields with literal NUL bytes, which made
   grep treat the whole of scripts/pr-gate.mjs as binary — any future
   grep-based CI guard over that file would have matched nothing and
   passed silently. Replaced with JSON.stringify of the tuple: still
   unforgeable (each field is quoted and escaped), still stable by
   construction, and printable. `grep -c hashInputs scripts/pr-gate.mjs`
   now returns 2 instead of nothing. Existing sticky-comment state hashes
   are invalidated once, costing one re-verdict per open PR.

Tests: 95 pass / 0 fail in test/pr-gate-workflow.test.ts. The no-API-key
policy-miss case was verified to fail against the pre-fix ordering.
Verified live against the Anthropic API: HTTP 200, strict JSON, all seven
required keys, merge-lane on a compliant fixture.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

.github/workflows/pr-gate.yml

@@ -14,11 +14,15 @@ name: PR Gate
 #   every ${{ }} is env-bound; run: scripts use plain env vars.
 # - Only the issues API is used (comments + labels), so issues:write is the
 #   single write grant; the checkout drops its credentials.
-# - If ANTHROPIC_API_KEY is missing or the API is unreachable, the script
-#   NEUTRAL-skips loudly (sticky comment + warning annotation, exit 0) and
-#   CLEARS any stale gate:* label — never a silent green, never a red X for a
-#   missing secret, never a stale verdict. A model REFUSAL is not a skip: it
-#   routes to needs-maintainer so refusing is not a way to dodge the gate.
+# - The mechanical CONTRIBUTING.md #3745 check (intent paragraph + screenshot)
+#   runs BEFORE any API dependency, so a PR missing either still lands in
+#   close-lane during an Anthropic outage — an outage is not a way through.
+# - If ANTHROPIC_API_KEY is missing or the API is unreachable on an otherwise
+#   compliant PR, the script NEUTRAL-skips loudly (sticky comment + warning
+#   annotation, exit 0) and CLEARS any stale gate:* label — never a silent
+#   green, never a red X for a missing secret, never a stale verdict. A model
+#   REFUSAL is not a skip: it routes to needs-maintainer so refusing is not a
+#   way to dodge the gate.
 # Pinned by test/pr-gate-workflow.test.ts.
 
 on:

scripts/pr-gate.mjs

@@ -26,14 +26,17 @@
  *   merge-lane recommendation to needs-maintainer, so a persuasive PR body
  *   cannot talk itself into the fast lane.
  * - CONTRIBUTING.md's #3745 requirement (a human-written intent paragraph AND
- *   a screenshot of gbrain in use) is checked mechanically. Missing either
- *   forces close-lane with no model call — that is the documented consequence.
+ *   a screenshot of gbrain in use) is checked mechanically, BEFORE anything
+ *   that can fail: no model, and therefore no API key and no network. Missing
+ *   either forces close-lane — that is the documented consequence, and an
+ *   Anthropic outage must not become a way past it.
  *   The model's separate intent_authenticity read is advisory only: at most it
  *   forces needs-maintainer, and it never appears in the comment.
  * - A refusal or unparseable output routes to needs-maintainer, never to a
  *   green NEUTRAL — a deterministic refusal must not be a way to dodge the
- *   verdict. Only infrastructure failure (missing key, API down) is NEUTRAL,
- *   and NEUTRAL clears stale gate:* labels so no stale verdict survives.
+ *   verdict. Only infrastructure failure (missing key, API down) on an
+ *   otherwise-compliant PR is NEUTRAL, and NEUTRAL clears stale gate:* labels
+ *   so no stale verdict survives.
  *
  * No dependencies — global fetch only (Node 18+).
  */
@@ -582,9 +585,13 @@ async function setLaneLabel(gh, repo, prNumber, lane) {
 // The verdict only depends on title + body + head sha, so if those are
 // unchanged since the last sticky comment there is nothing new to classify.
 // ---------------------------------------------------------------------------
+// JSON.stringify is the separator: it quotes and escapes each field, so no
+// title or body can forge a boundary, and the tuple order is fixed by the
+// literal. Literal NUL bytes did the same job but made the whole file "binary"
+// to grep, which silently defeats any grep-based CI guard over it.
 export function hashInputs(pr) {
   return createHash('sha256')
-    .update(`${pr.title ?? ''}${pr.body ?? ''}${pr.head?.sha ?? ''}`)
+    .update(JSON.stringify([pr.title ?? '', pr.body ?? '', pr.head?.sha ?? '']))
     .digest('hex')
     .slice(0, 16);
 }
@@ -640,7 +647,7 @@ export function renderComment({
   if (neutralReason) {
     lines.push('## PR Gate — NEUTRAL (skipped)', '', `**Reason:** ${sanitizeModelText(neutralReason)}`, '');
     lines.push(
-      'The gate did not run, so there is no verdict and any previous `gate:*` label was cleared. This is a loud skip, not a pass.',
+      'The **usefulness verdict did not run**, so there is no lane and any previous `gate:*` label was cleared. This is a loud skip, not a pass. The mechanical checks below need no model: they ran, and the CONTRIBUTING.md intent-paragraph + screenshot requirement passed — a miss there is close-lane whether or not the model is reachable.',
       '',
     );
   } else {
@@ -702,24 +709,16 @@ export async function runGate(dir, env = process.env, fetchImpl = fetch) {
     return 0;
   };
 
-  const apiKey = env.ANTHROPIC_API_KEY;
-  if (!apiKey) return neutral('ANTHROPIC_API_KEY is not configured for this run — verdict skipped.');
-
-  // Spend guard: identical inputs to the last verdict → reuse it, no LLM call.
   const inputHash = hashInputs(pr);
-  const prev = parseState(existing?.body);
-  if (prev && prev.hash === inputHash && LANES.includes(prev.lane)) {
-    console.log(
-      `PR gate: title+body+head_sha unchanged (${inputHash}) since the last verdict — skipping the LLM call, keeping ${prev.lane}.`,
-    );
-    return prev.lane === 'close-lane' ? 1 : 0;
-  }
-
   let verdict;
   let degraded = null;
   if (policyMisses.length > 0) {
-    // Closed without review is the documented consequence, so don't spend a
-    // review call proving it. The comment leads with the fix, not the verdict.
+    // ORDER IS LOAD-BEARING: this branch sits ABOVE the API-key guard and the
+    // model call. #3745 is fully mechanical, so a missing key or a dead
+    // Anthropic must not turn "closed without review" into a green NEUTRAL —
+    // that would make an outage the way through the one hard requirement.
+    // Closed without review is also the documented consequence, so don't spend
+    // a review call proving it. The comment leads with the fix, not the verdict.
     console.log(
       `PR gate: #3745 policy miss (${policyMisses.map((f) => f.id).join(', ')}) — close-lane without a model call.`,
     );
@@ -732,6 +731,20 @@ export async function runGate(dir, env = process.env, fetchImpl = fetch) {
       reviewer_checklist: [],
     };
   } else {
+    const apiKey = env.ANTHROPIC_API_KEY;
+    if (!apiKey) {
+      return neutral('ANTHROPIC_API_KEY is not configured for this run — the usefulness verdict was skipped.');
+    }
+
+    // Spend guard: identical inputs to the last verdict → reuse it, no LLM call.
+    const prev = parseState(existing?.body);
+    if (prev && prev.hash === inputHash && LANES.includes(prev.lane)) {
+      console.log(
+        `PR gate: title+body+head_sha unchanged (${inputHash}) since the last verdict — skipping the LLM call, keeping ${prev.lane}.`,
+      );
+      return prev.lane === 'close-lane' ? 1 : 0;
+    }
+
     try {
       verdict = await callAnthropic(apiKey, buildPayload({ pr, files, diff, titleCheck, flags }), fetchImpl);
     } catch (err) {

test/pr-gate-workflow.test.ts

@@ -12,10 +12,14 @@
  *   exit code, marker-hijack, sanitization, truncation, refusal routing,
  *   NEUTRAL label clearing, label swap, and the input-hash spend guard.
  * - The CONTRIBUTING.md #3745 policy: the mechanical screenshot + intent
- *   detectors (all four embed forms, the in-code-fence negative, the empty
- *   template, non-English prose), the forced close-lane both halves produce,
- *   the friendly fix-it comment, and the advisory-only ai_generated route to
- *   needs-maintainer that must never accuse or close.
+ *   detectors (all four embed forms, the in-code-fence negative, the real
+ *   .github/pull_request_template.md, non-English prose), the forced
+ *   close-lane both halves produce, the friendly fix-it comment, its deep link
+ *   resolving to a heading that actually exists in CONTRIBUTING.md, and the
+ *   advisory-only ai_generated route to needs-maintainer that must never
+ *   accuse or close.
+ * - The policy check outliving the model: a miss closes the PR with no API key
+ *   and through a 500, while a compliant PR keeps the loud NEUTRAL skip.
  */
 import { describe, test, expect } from 'bun:test';
 import { readFileSync, existsSync, mkdtempSync, writeFileSync } from 'node:fs';
@@ -36,6 +40,7 @@ import {
   parseState,
   renderComment,
   runGate,
+  CONTRIBUTING_URL,
   INTENT_MIN_WORDS,
   MAX_ITEMS,
   MAX_STRING,
@@ -59,26 +64,12 @@ const HUMAN_INTENT = [
 const SCREENSHOT_EMBED = '![my terminal](https://github.com/user-attachments/assets/0a1b2c3d-4e5f-6789)';
 const COMPLIANT_BODY = `${HUMAN_INTENT}\n\n${SCREENSHOT_EMBED}\n`;
 
-// .github/pull_request_template.md as of #3745, for branches that predate it.
-const PR_TEMPLATE_FALLBACK = [
-  '**Why are you opening this? (human-written, required)**',
-  '',
-  '<!-- Write this yourself. Not AI-generated, not AI-polished. What were you',
-  '     doing, what went wrong or what you needed, why it matters to you.',
-  '     Rough grammar is fine. PRs without this are closed unreviewed. -->',
-  '',
-  '',
-  '**Screenshot of gbrain in use (required)**',
-  '',
-  '<!-- Your terminal / agent session / logs showing the real need this fixes.',
-  '     Redact private names, keys, and brain contents first. -->',
-  '',
-  '',
-  '**What changed**',
-  '',
-  '',
-  '**How it was tested**',
-].join('\n');
+// The real #3745 artifacts the gate enforces. Read from disk, never inlined:
+// a fallback copy would keep passing after the originals drifted.
+const CONTRIBUTING_PATH = join(import.meta.dir, '..', 'CONTRIBUTING.md');
+const PR_TEMPLATE_PATH = join(import.meta.dir, '..', '.github', 'pull_request_template.md');
+const CONTRIBUTING = readFileSync(CONTRIBUTING_PATH, 'utf8');
+const PR_TEMPLATE = readFileSync(PR_TEMPLATE_PATH, 'utf8');
 
 /**
  * Collect every line that belongs to a `run:` script, in all four YAML scalar
@@ -219,6 +210,14 @@ describe('pr-gate script rubric pins', () => {
     expect(SCRIPT).toContain(MARKER);
   });
 
+  test('the script is greppable as text — no NUL bytes anywhere', () => {
+    // One literal \0 makes grep treat the whole file as binary, so any future
+    // grep-based CI guard over it silently matches nothing instead of failing.
+    expect(SCRIPT).not.toMatch(/\u0000/);
+    // ...and so does this test file, or the guard reintroduces what it forbids.
+    expect(readFileSync(import.meta.path, 'utf8')).not.toMatch(/\u0000/);
+  });
+
   test('never passes sampling params (rejected with 400 on claude-sonnet-5)', () => {
     expect(SCRIPT).not.toMatch(/["']?temperature["']?\s*:/);
     expect(SCRIPT).not.toMatch(/["']?top_p["']?\s*:/);
@@ -505,16 +504,13 @@ describe('intent paragraph detector (#3745, mechanical)', () => {
     expect(hasIntentParagraph(undefined)).toBe(false);
   });
 
-  test('the PR template with nothing filled in does not count', () => {
-    // Read the real template when it is present (this branch may predate the
-    // #3745 merge that adds it), so growing the template's own prose past the
-    // bar — which would let an untouched template pass — fails here.
-    const templatePath = join(import.meta.dir, '..', '.github', 'pull_request_template.md');
-    const template = existsSync(templatePath) ? readFileSync(templatePath, 'utf8') : PR_TEMPLATE_FALLBACK;
-    expect(hasIntentParagraph(template)).toBe(false);
-    expect(hasScreenshot(template)).toBe(false);
+  test('the real PR template with nothing filled in does not count', () => {
+    // Against .github/pull_request_template.md itself: growing the template's
+    // own prose past the bar would let an untouched template pass the gate.
+    expect(hasIntentParagraph(PR_TEMPLATE)).toBe(false);
+    expect(hasScreenshot(PR_TEMPLATE)).toBe(false);
     // Filling only the "what changed" section is still not the intent paragraph.
-    expect(hasIntentParagraph(`${template}\nrenames the flag and updates the docs`)).toBe(false);
+    expect(hasIntentParagraph(`${PR_TEMPLATE}\nrenames the flag and updates the docs`)).toBe(false);
   });
 
   test('40+ words of the author own prose counts', () => {
@@ -574,6 +570,39 @@ describe('detectPolicyMisses (#3745)', () => {
   });
 });
 
+describe('CONTRIBUTING.md deep link (#3745)', () => {
+  // GitHub's heading-anchor slug: lowercase, drop everything outside
+  // [word chars, hyphen, space], collapse spaces to hyphens.
+  const githubAnchor = (heading: string) =>
+    heading.toLowerCase().replace(/[^\w\- ]+/g, '').trim().replace(/ +/g, '-');
+
+  test('the anchor the gate links to is a real heading in CONTRIBUTING.md', () => {
+    // A deep link that 404s to the top of the file is the whole comment's
+    // call to action pointing at nothing.
+    const [url, anchor] = CONTRIBUTING_URL.split('#');
+    expect(url).toBe('https://github.com/garrytan/gbrain/blob/master/CONTRIBUTING.md');
+    expect(anchor).toBeTruthy();
+    const anchors = [...CONTRIBUTING.matchAll(/^#{1,6} +(.+?)\s*$/gm)].map((m) => githubAnchor(m[1]));
+    expect(anchors).toContain(anchor);
+  });
+
+  test('the slugger matches GitHub on the heading shapes in this file', () => {
+    // Guards the guard: a slugger that dropped punctuation handling would
+    // "pass" the test above against an anchor GitHub never generates.
+    expect(githubAnchor('Human-authored intent (required, no exceptions)')).toBe(
+      'human-authored-intent-required-no-exceptions',
+    );
+    expect(githubAnchor('Setup')).toBe('setup');
+  });
+
+  test('CONTRIBUTING.md states the policy the gate enforces', () => {
+    expect(CONTRIBUTING).toContain('## Human-authored intent (required, no exceptions)');
+    expect(CONTRIBUTING).toContain('A paragraph you wrote yourself');
+    expect(CONTRIBUTING).toMatch(/screenshot showing gbrain actually being used/i);
+    expect(CONTRIBUTING).toMatch(/closed without review/i);
+  });
+});
+
 describe('applyMechanicalDowngrades (lane is not purely model-decided)', () => {
   const flag = (id: string) => ({ id, detail: `detail for ${id}` });
 
@@ -1054,6 +1083,7 @@ describe('runGate — CONTRIBUTING.md #3745 policy (mocked fetch)', () => {
     expect(body).toContain('then reopen');
     expect(body).toContain('not a judgment on the code');
     expect(body).toContain('CONTRIBUTING.md');
+    expect(body).toContain(CONTRIBUTING_URL); // the deep link, anchor included
     // Also recorded where the other deterministic overrides are recorded.
     expect(body).toContain('Mechanical downgrades applied');
     expect(body).toContain('#3745');
@@ -1115,6 +1145,59 @@ describe('runGate — CONTRIBUTING.md #3745 policy (mocked fetch)', () => {
     expect(parseState(body)).toMatchObject({ lane: 'needs-maintainer' });
   });
 
+  // The policy check is mechanical, so it must outlive the model. If an
+  // outage downgraded a policy miss to a green NEUTRAL, "wait for Anthropic to
+  // 500" would be the documented way past the one hard requirement.
+  test('a policy miss closes the PR with NO API key — an outage is not a way through', async () => {
+    const { calls, fetchImpl } = stubFetch({}); // no anthropic handler: any call throws
+    const code = await runGate(
+      fixtureDir({ body: HUMAN_INTENT }, SRC_AND_TEST),
+      { ...ENV, ANTHROPIC_API_KEY: undefined },
+      fetchImpl,
+    );
+    expect(code).toBe(1);
+    expect(addedLabels(calls)).toEqual(['gate:close-lane']);
+    const body: string = postedBody(calls);
+    expect(body).toContain('Almost there');
+    expect(body).toContain('A screenshot of gbrain in use');
+    expect(body).not.toContain('NEUTRAL');
+  });
+
+  test('a policy miss closes the PR when the API 500s, without reaching the model', async () => {
+    const { calls, fetchImpl } = stubFetch({ anthropic: () => jsonResponse({ error: 'boom' }, 500) });
+    const code = await runGate(fixtureDir({ body: '' }, SRC_AND_TEST), ENV, fetchImpl);
+    expect(code).toBe(1);
+    expect(addedLabels(calls)).toEqual(['gate:close-lane']);
+    expect(calls.some((c) => c.url.startsWith('https://api.anthropic.com'))).toBe(false);
+    expect(postedBody(calls)).not.toContain('NEUTRAL');
+  });
+
+  test('a COMPLIANT PR with no API key still NEUTRAL-skips, reporting what it could compute', async () => {
+    const { calls, fetchImpl } = stubFetch({});
+    const code = await runGate(
+      // Bad title + a src change with no test: both mechanical, both computable
+      // without the model.
+      fixtureDir({ title: 'Update README.md', body: COMPLIANT_BODY }, [
+        { filename: 'src/core/thing.ts', status: 'modified', additions: 12, deletions: 0 },
+      ]),
+      { ...ENV, ANTHROPIC_API_KEY: undefined },
+      fetchImpl,
+    );
+    expect(code).toBe(0);
+    expect(addedLabels(calls)).toEqual([]);
+    expect(deletedLabels(calls).sort()).toEqual([
+      'gate:close-lane',
+      'gate:merge-lane',
+      'gate:needs-maintainer',
+    ]);
+    const body: string = postedBody(calls);
+    expect(body).toContain('NEUTRAL');
+    expect(body).toContain('usefulness verdict did not run');
+    expect(body).toContain('neither version-first'); // the mechanical title check
+    expect(body).toContain('#3665'); // the mechanical red flag
+    expect(body).not.toContain('Almost there'); // nothing to fix in the description
+  });
+
   test('a human / unclear intent verdict leaves the lane alone', async () => {
     for (const intent of ['human', 'unclear']) {
       const { calls, fetchImpl } = stubFetch({