mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-14 00:48:18 +00:00
feat(ci): gate enforces the #3745 intent-paragraph + screenshot requirement
CONTRIBUTING.md (#3745) requires every PR to carry a paragraph the author wrote themselves and a screenshot of gbrain actually in use. The gate now checks both, mechanically, before it spends anything on review. What it checks (no LLM, both exported for testing): - hasScreenshot: markdown ``, a bare user-images.githubusercontent or github.com/user-attachments/assets URL, or an <img> tag. Anything inside a fenced code block does not count — pasting the syntax is not attaching the picture. - hasIntentParagraph: >= 40 words of prose left after stripping fenced code, blockquotes, list items, headings, HTML comments, links and the PR template's own boilerplate. Per-character scripts are tokenized per character, so a paragraph written in Chinese counts as one. The two lane consequences: - missing_screenshot OR missing_intent forces close-lane from any recommended lane (exit 1) and skips the model call entirely — closed without review is the documented consequence, so there is nothing to spend a review on. The sticky comment leads with what is missing, how to fix it, and the reopen path; both misses are also recorded in the existing "Mechanical downgrades applied" section. - The model's new advisory intent_authenticity verdict forces needs-maintainer (exit 0) when it reads "ai_generated", and never close-lane on that signal alone. The comment says only that a maintainer will read the paragraph personally; the model's reasoning is consumed and never published at the contributor. Rubric + strict-JSON schema gain intent_authenticity and intent_authenticity_reason, with explicit instructions that rough grammar, terseness and non-native English are evidence of a HUMAN and that "unclear" is the answer whenever the evidence is not clear-cut. test/pr-gate-workflow.test.ts: 63 -> 88 tests, all green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
committed by
Sina Matian
co-authored by
Claude Fable 5
parent
2f65ed8da6
commit
94662eb1e1
@@ -28,12 +28,24 @@ export declare const MAX_ITEMS: number;
|
||||
export declare const NET_SOURCE_LINE_LIMIT: number;
|
||||
export declare const DOWNGRADE_FLAG_IDS: string[];
|
||||
|
||||
/** CONTRIBUTING.md #3745: human-written intent paragraph + screenshot of gbrain in use. */
|
||||
export declare const CONTRIBUTING_URL: string;
|
||||
export declare const INTENT_MIN_WORDS: number;
|
||||
export declare const POLICY_FLAG_IDS: string[];
|
||||
export declare const AI_INTENT_DOWNGRADE: string;
|
||||
export declare function stripCodeFences(body: unknown): string;
|
||||
export declare function hasScreenshot(body: unknown): boolean;
|
||||
export declare function intentWordCount(body: unknown): number;
|
||||
export declare function hasIntentParagraph(body: unknown): boolean;
|
||||
export declare function detectPolicyMisses(body: unknown): RedFlag[];
|
||||
|
||||
export declare function sanitizeModelText(value: unknown, max?: number): string;
|
||||
export declare function sanitizeList(value: unknown, maxItems?: number, maxString?: number): string[];
|
||||
|
||||
export declare function applyMechanicalDowngrades(
|
||||
lane: string,
|
||||
flags: RedFlag[],
|
||||
intentAuthenticity?: string,
|
||||
): { lane: string; downgrades: string[] };
|
||||
|
||||
export interface GhComment {
|
||||
@@ -57,6 +69,7 @@ export declare function renderComment(input: {
|
||||
flags: RedFlag[];
|
||||
neutralReason?: string;
|
||||
downgrades?: string[];
|
||||
policyMisses?: RedFlag[];
|
||||
state?: { hash: string; lane: string };
|
||||
}): string;
|
||||
|
||||
|
||||
+194
-27
@@ -25,6 +25,11 @@
|
||||
* - The lane is NOT purely model-decided: mechanical signals downgrade a
|
||||
* 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.
|
||||
* 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,
|
||||
@@ -44,6 +49,7 @@ const STATE_RE = /<!-- gbrain-pr-gate-state (\{[^\n]*?\}) -->/;
|
||||
const BOT_LOGIN = 'github-actions[bot]';
|
||||
const MODEL = 'claude-sonnet-5';
|
||||
const LANES = ['merge-lane', 'close-lane', 'needs-maintainer'];
|
||||
const INTENT_VERDICTS = ['human', 'ai_generated', 'unclear'];
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// The rubric — the maintainer's standing policy. Keep verbatim-strict.
|
||||
@@ -78,7 +84,11 @@ NEEDS_MAINTAINER (neutral — lane "needs-maintainer"):
|
||||
|
||||
Also produce reviewer_checklist: 3-6 concrete verification steps a human reviewer must do for THIS diff (e.g. 'confirm the claimed bug exists on master at <file>', 'run the eval replay gate — this touches src/core/search/hybrid.ts', 'check engine parity — only pglite-engine.ts modified').
|
||||
|
||||
Output strict JSON: lane (one of "merge-lane", "close-lane", "needs-maintainer"), confidence (0 to 1), reasons[] citing concrete evidence from the diff/description, title_ok (does the title follow the version-first rule stated in the payload), reviewer_checklist[].
|
||||
Also judge intent_authenticity: does the author's own "why I am opening this" paragraph read as written by a human, or as AI-generated / AI-polished text? Telltales of AI text: uniform hedging, vocabulary like "delve", "leverage", "robust", "seamless", perfectly balanced tri-colons, no first-person specifics, no concrete situation, no rough edges. Answer "human", "ai_generated" or "unclear", plus intent_authenticity_reason (one short line).
|
||||
|
||||
This judgment is ADVISORY. It NEVER closes a PR on its own — at most it sends the PR to a human maintainer to read. Rough grammar, terseness, typos and non-native English are evidence of a HUMAN, not of AI. Answer "unclear" whenever the evidence is not clear-cut: wrongly telling a real contributor they did not write their own words is a far worse error than missing an AI-written paragraph.
|
||||
|
||||
Output strict JSON: lane (one of "merge-lane", "close-lane", "needs-maintainer"), confidence (0 to 1), reasons[] citing concrete evidence from the diff/description, title_ok (does the title follow the version-first rule stated in the payload), reviewer_checklist[], intent_authenticity, intent_authenticity_reason.
|
||||
|
||||
Your lane is a RECOMMENDATION. Mechanical signals computed outside this prompt can downgrade merge-lane to needs-maintainer regardless of what you return, so state the honest verdict rather than the one you think will stick.
|
||||
|
||||
@@ -94,8 +104,18 @@ const VERDICT_SCHEMA = {
|
||||
reasons: { type: 'array', items: { type: 'string' } },
|
||||
title_ok: { type: 'boolean' },
|
||||
reviewer_checklist: { type: 'array', items: { type: 'string' } },
|
||||
intent_authenticity: { type: 'string', enum: INTENT_VERDICTS },
|
||||
intent_authenticity_reason: { type: 'string' },
|
||||
},
|
||||
required: ['lane', 'confidence', 'reasons', 'title_ok', 'reviewer_checklist'],
|
||||
required: [
|
||||
'lane',
|
||||
'confidence',
|
||||
'reasons',
|
||||
'title_ok',
|
||||
'reviewer_checklist',
|
||||
'intent_authenticity',
|
||||
'intent_authenticity_reason',
|
||||
],
|
||||
additionalProperties: false,
|
||||
};
|
||||
|
||||
@@ -163,6 +183,85 @@ export function sanitizeList(value, maxItems = MAX_ITEMS, maxString = MAX_STRING
|
||||
return out;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// CONTRIBUTING.md policy (#3745), checked mechanically — no LLM, no judgment
|
||||
// call. Every PR must carry a paragraph the author wrote themselves and a
|
||||
// screenshot of gbrain in use. Missing either is "closed without review,
|
||||
// reopenable once added", so these two are the only flags that can force a
|
||||
// lane rather than merely downgrade one.
|
||||
// ---------------------------------------------------------------------------
|
||||
export const CONTRIBUTING_URL =
|
||||
'https://github.com/garrytan/gbrain/blob/master/CONTRIBUTING.md#human-authored-intent-required-no-exceptions';
|
||||
|
||||
/**
|
||||
* Drop fenced code blocks (``` or ~~~, unterminated fences run to EOF). A
|
||||
* screenshot pasted inside a fence is documentation of the syntax, not proof.
|
||||
*/
|
||||
const FENCE_RE = /^[ \t]{0,3}(`{3,}|~{3,})[^\n]*\n[\s\S]*?(?:^[ \t]{0,3}\1[ \t]*$|$(?![\s\S]))/gm;
|
||||
export const stripCodeFences = (body) => String(body ?? '').replace(FENCE_RE, '\n');
|
||||
|
||||
const SCREENSHOT_RES = [
|
||||
/!\[[^\]]*\]\(\s*\S/, // markdown image embed
|
||||
/<img\b[^>]*>/i, // raw HTML img tag
|
||||
/https:\/\/user-images\.githubusercontent\.com\/\S/i, // legacy paste URL
|
||||
/https:\/\/github\.com\/user-attachments\/assets\/\S/i, // current paste URL
|
||||
];
|
||||
|
||||
export function hasScreenshot(body) {
|
||||
const text = stripCodeFences(body);
|
||||
return SCREENSHOT_RES.some((re) => re.test(text));
|
||||
}
|
||||
|
||||
export const INTENT_MIN_WORDS = 40;
|
||||
|
||||
// Everything a contributor can paste WITHOUT writing a word themselves: code,
|
||||
// quoted logs, checklists, headings, the template's HTML hints, and the
|
||||
// template's own bold prompts (a whole line of `**...**` is a heading in
|
||||
// disguise). What survives is the author's own prose.
|
||||
export function intentWordCount(body) {
|
||||
const prose = stripCodeFences(body)
|
||||
.replace(/<!--[\s\S]*?-->/g, ' ') // HTML comments (the PR template's hints)
|
||||
.replace(/^[ \t]{0,3}#{1,6}[ \t].*$/gm, ' ') // headings
|
||||
.replace(/^[ \t]*\*\*[^\n]*\*\*[ \t]*$/gm, ' ') // bold-only line = template prompt
|
||||
.replace(/^[ \t]{0,3}>.*$/gm, ' ') // blockquotes
|
||||
.replace(/^[ \t]*([-*+]|\d+[.)])[ \t].*$/gm, ' ') // list items
|
||||
.replace(/!?\[[^\]]*\]\([^)]*\)/g, ' ') // links + image embeds
|
||||
.replace(/<[^>]+>/g, ' ') // raw HTML tags
|
||||
.replace(/https?:\/\/\S+/g, ' ') // bare URLs
|
||||
.replace(/`[^`]*`/g, ' ') // inline code
|
||||
// CJK is word-per-character, so space each one out before tokenizing —
|
||||
// otherwise a whole Chinese paragraph counts as a single "word" and a
|
||||
// non-English contributor gets closed for a paragraph they did write.
|
||||
.replace(/[\u3040-\u30ff\u3400-\u4dbf\u4e00-\u9fff\uf900-\ufaff]/gu, ' $& ');
|
||||
return (prose.match(/[\p{L}\p{N}][\p{L}\p{N}'’-]*/gu) ?? []).length;
|
||||
}
|
||||
|
||||
export const hasIntentParagraph = (body) => intentWordCount(body) >= INTENT_MIN_WORDS;
|
||||
|
||||
// Keyed in CONTRIBUTING.md's own order: the paragraph, then the screenshot.
|
||||
export const POLICY_FLAG_IDS = ['missing_intent', 'missing_screenshot'];
|
||||
|
||||
const POLICY_DETAILS = {
|
||||
missing_intent: `no human-written intent paragraph in the PR description (under ${INTENT_MIN_WORDS} words of prose once code, quotes, lists and the template boilerplate are removed) — required by CONTRIBUTING.md (#3745)`,
|
||||
missing_screenshot:
|
||||
'no screenshot of gbrain in use in the PR description — required by CONTRIBUTING.md (#3745)',
|
||||
};
|
||||
|
||||
// Reader-facing version of the same two asks, for the top of the comment.
|
||||
const POLICY_ASKS = {
|
||||
missing_intent:
|
||||
'**A paragraph you wrote yourself** about why you are opening this — what you were doing, what went wrong or what you needed, why it matters to you. Rough grammar is fine and preferred over polish.',
|
||||
missing_screenshot:
|
||||
'**A screenshot of gbrain in use** in that situation — your terminal, your agent session, your logs. Redact private names, keys and brain contents first.',
|
||||
};
|
||||
|
||||
export function detectPolicyMisses(body) {
|
||||
const misses = [];
|
||||
if (!hasIntentParagraph(body)) misses.push({ id: 'missing_intent', detail: POLICY_DETAILS.missing_intent });
|
||||
if (!hasScreenshot(body)) misses.push({ id: 'missing_screenshot', detail: POLICY_DETAILS.missing_screenshot });
|
||||
return misses;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Mechanical red flags (no LLM).
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -281,9 +380,26 @@ export const DOWNGRADE_FLAG_IDS = [
|
||||
'no_test_for_src_change',
|
||||
];
|
||||
|
||||
export function applyMechanicalDowngrades(lane, flags) {
|
||||
if (lane !== 'merge-lane') return { lane, downgrades: [] };
|
||||
const hits = flags.filter((f) => DOWNGRADE_FLAG_IDS.includes(f.id));
|
||||
/**
|
||||
* The one downgrade that is not a red flag: the model read the intent
|
||||
* paragraph as AI-written. It routes to a human and stops there — never to
|
||||
* close-lane, because a false positive tells a real contributor they did not
|
||||
* write their own words. Phrased so the sticky comment can render it verbatim
|
||||
* without accusing anybody of anything.
|
||||
*/
|
||||
export const AI_INTENT_DOWNGRADE =
|
||||
'a maintainer will read the intent paragraph on this PR personally before it merges';
|
||||
|
||||
export function applyMechanicalDowngrades(lane, flags, intentAuthenticity) {
|
||||
// #3745 is a hard requirement, not a recommendation: a missing intent
|
||||
// paragraph or screenshot closes the PR whatever lane was recommended.
|
||||
const policy = flags.filter((f) => POLICY_FLAG_IDS.includes(f.id));
|
||||
if (policy.length > 0) return { lane: 'close-lane', downgrades: policy.map((f) => f.detail) };
|
||||
|
||||
const hits = lane === 'merge-lane' ? flags.filter((f) => DOWNGRADE_FLAG_IDS.includes(f.id)) : [];
|
||||
if (intentAuthenticity === 'ai_generated' && lane !== 'close-lane') {
|
||||
return { lane: 'needs-maintainer', downgrades: [...hits.map((f) => f.detail), AI_INTENT_DOWNGRADE] };
|
||||
}
|
||||
if (hits.length === 0) return { lane, downgrades: [] };
|
||||
return { lane: 'needs-maintainer', downgrades: hits.map((f) => f.detail) };
|
||||
}
|
||||
@@ -494,8 +610,30 @@ const LANE_HEADINGS = {
|
||||
'needs-maintainer': 'NEEDS MAINTAINER — human judgment required',
|
||||
};
|
||||
const LANE_MARKS = { 'merge-lane': '✅', 'close-lane': '❌', 'needs-maintainer': '⚠️' };
|
||||
const POLICY_HEADING = 'CLOSE LANE — the PR description is missing something required';
|
||||
|
||||
export function renderComment({ lane, verdict, titleCheck, flags, neutralReason, downgrades = [], state }) {
|
||||
/** Leads the comment on a #3745 miss: what is missing, how to fix it, how to reopen. */
|
||||
function policyBlock(policyMisses) {
|
||||
const ids = POLICY_FLAG_IDS.filter((id) => policyMisses.some((f) => f.id === id));
|
||||
return [
|
||||
'**Almost there — before this can be reviewed the description needs:**',
|
||||
'',
|
||||
...ids.map((id) => `- ${POLICY_ASKS[id]}`),
|
||||
'',
|
||||
`Edit the description to add that, then reopen. This is not a judgment on the code — the policy is in [CONTRIBUTING.md](${CONTRIBUTING_URL}).`,
|
||||
];
|
||||
}
|
||||
|
||||
export function renderComment({
|
||||
lane,
|
||||
verdict,
|
||||
titleCheck,
|
||||
flags,
|
||||
neutralReason,
|
||||
downgrades = [],
|
||||
policyMisses = [],
|
||||
state,
|
||||
}) {
|
||||
const lines = [MARKER];
|
||||
if (state) lines.push(`${STATE_PREFIX}${JSON.stringify(state)} -->`);
|
||||
lines.push('');
|
||||
@@ -506,24 +644,32 @@ export function renderComment({ lane, verdict, titleCheck, flags, neutralReason,
|
||||
'',
|
||||
);
|
||||
} else {
|
||||
lines.push(`## PR Gate — ${LANE_MARKS[lane]} ${LANE_HEADINGS[lane]}`, '');
|
||||
const heading = policyMisses.length > 0 ? POLICY_HEADING : LANE_HEADINGS[lane];
|
||||
lines.push(`## PR Gate — ${LANE_MARKS[lane]} ${heading}`, '');
|
||||
if (policyMisses.length > 0) lines.push(...policyBlock(policyMisses), '');
|
||||
lines.push(`**Label:** \`${LABELS[lane].name}\` · **Confidence:** ${Number(verdict.confidence) || 0}`, '');
|
||||
lines.push('**Why:**');
|
||||
for (const r of sanitizeList(verdict.reasons)) lines.push(`- ${r}`);
|
||||
if (downgrades.length > 0) {
|
||||
lines.push('', '**Mechanical downgrades applied** (merge-lane → needs-maintainer, regardless of the model verdict):');
|
||||
lines.push('', '**Mechanical downgrades applied** (deterministic, regardless of the model verdict):');
|
||||
for (const d of sanitizeList(downgrades)) lines.push(`- ${d}`);
|
||||
}
|
||||
lines.push('', '**Reviewer checklist:**');
|
||||
for (const c of sanitizeList(verdict.reviewer_checklist)) lines.push(`- [ ] ${c}`);
|
||||
const checklist = sanitizeList(verdict.reviewer_checklist);
|
||||
if (checklist.length > 0) {
|
||||
lines.push('', '**Reviewer checklist:**');
|
||||
for (const c of checklist) lines.push(`- [ ] ${c}`);
|
||||
}
|
||||
lines.push('');
|
||||
}
|
||||
// Policy misses already have two sections of their own; a third copy here
|
||||
// just reads as the machine repeating itself at a first-time contributor.
|
||||
const redFlags = flags.filter((f) => !POLICY_FLAG_IDS.includes(f.id));
|
||||
lines.push(
|
||||
`**Title (version-first rule):** ${titleCheck.ok ? '✅ ok' : `❌ ${titleCheck.reason}`}`,
|
||||
'',
|
||||
`**Mechanical red flags:** ${flags.length ? '' : 'none'}`,
|
||||
`**Mechanical red flags:** ${redFlags.length ? '' : 'none'}`,
|
||||
);
|
||||
for (const f of flags) lines.push(`- ${f.detail}`);
|
||||
for (const f of redFlags) lines.push(`- ${f.detail}`);
|
||||
lines.push(
|
||||
'',
|
||||
'<sub>Strict usefulness gate (#3698). merge-lane / needs-maintainer exit green; close-lane exits red (strong signal, not a hard block — maintainers decide). PR code is never checked out or executed: verdict is from API metadata + a 120KB-capped diff only.</sub>',
|
||||
@@ -545,7 +691,8 @@ export async function runGate(dir, env = process.env, fetchImpl = fetch) {
|
||||
|
||||
const gh = ghClient(env, fetchImpl);
|
||||
const titleCheck = checkTitle(pr.title ?? '');
|
||||
const flags = detectRedFlags({ changedFiles: pr.changed_files ?? files.length, files, diff });
|
||||
const policyMisses = detectPolicyMisses(pr.body);
|
||||
const flags = [...detectRedFlags({ changedFiles: pr.changed_files ?? files.length, files, diff }), ...policyMisses];
|
||||
const existing = await findOwnComment(gh, repo, prNumber);
|
||||
|
||||
const neutral = async (reason) => {
|
||||
@@ -570,27 +717,46 @@ export async function runGate(dir, env = process.env, fetchImpl = fetch) {
|
||||
|
||||
let verdict;
|
||||
let degraded = null;
|
||||
try {
|
||||
verdict = await callAnthropic(apiKey, buildPayload({ pr, files, diff, titleCheck, flags }), fetchImpl);
|
||||
} catch (err) {
|
||||
const detail = String(err?.message ?? err).slice(0, 200);
|
||||
if (err?.kind !== 'refusal' && err?.kind !== 'schema') {
|
||||
return neutral(`Anthropic API unavailable after 2 retries: ${detail}`);
|
||||
}
|
||||
// A refusal or unusable output is NOT a free pass: route to a human.
|
||||
degraded = detail;
|
||||
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.
|
||||
console.log(
|
||||
`PR gate: #3745 policy miss (${policyMisses.map((f) => f.id).join(', ')}) — close-lane without a model call.`,
|
||||
);
|
||||
verdict = {
|
||||
lane: 'needs-maintainer',
|
||||
confidence: 0,
|
||||
reasons: [`No automated verdict — ${detail}. Routed to needs-maintainer rather than skipped.`],
|
||||
reviewer_checklist: ['Classify this PR by hand against the usefulness rubric — the gate could not.'],
|
||||
lane: 'close-lane',
|
||||
confidence: 1,
|
||||
reasons: [
|
||||
'CONTRIBUTING.md requires a human-written intent paragraph and a screenshot of gbrain in use on every PR; this description is missing at least one of them. Reopen once added.',
|
||||
],
|
||||
reviewer_checklist: [],
|
||||
};
|
||||
} else {
|
||||
try {
|
||||
verdict = await callAnthropic(apiKey, buildPayload({ pr, files, diff, titleCheck, flags }), fetchImpl);
|
||||
} catch (err) {
|
||||
const detail = String(err?.message ?? err).slice(0, 200);
|
||||
if (err?.kind !== 'refusal' && err?.kind !== 'schema') {
|
||||
return neutral(`Anthropic API unavailable after 2 retries: ${detail}`);
|
||||
}
|
||||
// A refusal or unusable output is NOT a free pass: route to a human.
|
||||
degraded = detail;
|
||||
verdict = {
|
||||
lane: 'needs-maintainer',
|
||||
confidence: 0,
|
||||
reasons: [`No automated verdict — ${detail}. Routed to needs-maintainer rather than skipped.`],
|
||||
reviewer_checklist: ['Classify this PR by hand against the usefulness rubric — the gate could not.'],
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
// Mechanical overrides beat the LLM: the title verdict is ours, and the
|
||||
// downgrade set below is not negotiable by anything in the PR text.
|
||||
// intent_authenticity is deliberately consumed, never rendered — the reason
|
||||
// string is the model's private working, not something to publish at a
|
||||
// contributor on a public PR.
|
||||
verdict.title_ok = titleCheck.ok;
|
||||
const { lane, downgrades } = applyMechanicalDowngrades(verdict.lane, flags);
|
||||
const { lane, downgrades } = applyMechanicalDowngrades(verdict.lane, flags, verdict.intent_authenticity);
|
||||
verdict.lane = lane;
|
||||
|
||||
const body = renderComment({
|
||||
@@ -599,6 +765,7 @@ export async function runGate(dir, env = process.env, fetchImpl = fetch) {
|
||||
titleCheck,
|
||||
flags,
|
||||
downgrades,
|
||||
policyMisses,
|
||||
state: { hash: inputHash, lane },
|
||||
});
|
||||
await upsertStickyComment(gh, repo, prNumber, existing, body);
|
||||
|
||||
@@ -11,6 +11,11 @@
|
||||
* - Mocked end-to-end runs of runGate() against a stubbed fetch: close-lane
|
||||
* 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.
|
||||
*/
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { readFileSync, existsSync, mkdtempSync, writeFileSync } from 'node:fs';
|
||||
@@ -19,6 +24,10 @@ import { join } from 'node:path';
|
||||
import {
|
||||
checkTitle,
|
||||
detectRedFlags,
|
||||
detectPolicyMisses,
|
||||
hasScreenshot,
|
||||
hasIntentParagraph,
|
||||
intentWordCount,
|
||||
sanitizeModelText,
|
||||
sanitizeList,
|
||||
applyMechanicalDowngrades,
|
||||
@@ -27,6 +36,7 @@ import {
|
||||
parseState,
|
||||
renderComment,
|
||||
runGate,
|
||||
INTENT_MIN_WORDS,
|
||||
MAX_ITEMS,
|
||||
MAX_STRING,
|
||||
} from '../scripts/pr-gate.mjs';
|
||||
@@ -37,6 +47,39 @@ const WORKFLOW = readFileSync(WORKFLOW_PATH, 'utf8');
|
||||
const SCRIPT = readFileSync(SCRIPT_PATH, 'utf8');
|
||||
const MARKER = '<!-- gbrain-pr-gate -->';
|
||||
|
||||
// A #3745-compliant description: a paragraph in the author's own voice (rough
|
||||
// grammar on purpose — the policy prefers it) plus a real screenshot embed.
|
||||
const HUMAN_INTENT = [
|
||||
'I hit this last tuesday syncing my notes repo, about 4k files in it. the run just stopped',
|
||||
'somewhere in the middle and printed nothing at all, no error, so i assumed it had finished.',
|
||||
'next morning half my brain was missing and i had to re-import everything by hand which ate',
|
||||
'most of my day. i dont know this codebase well but the silent exit is the part that got me,',
|
||||
'if it had printed anything at all i would have caught it right away instead of a day later.',
|
||||
].join(' ');
|
||||
const SCREENSHOT_EMBED = '';
|
||||
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');
|
||||
|
||||
/**
|
||||
* Collect every line that belongs to a `run:` script, in all four YAML scalar
|
||||
* spellings: `run: cmd`, `run: |`, `run: >` and their `|-`/`>-`/`|+`/`>+`
|
||||
@@ -180,6 +223,14 @@ describe('pr-gate script rubric pins', () => {
|
||||
expect(SCRIPT).not.toMatch(/["']?temperature["']?\s*:/);
|
||||
expect(SCRIPT).not.toMatch(/["']?top_p["']?\s*:/);
|
||||
});
|
||||
|
||||
test('the rubric asks for intent_authenticity and keeps it advisory (#3745)', () => {
|
||||
expect(SCRIPT).toContain('intent_authenticity');
|
||||
expect(SCRIPT).toContain('intent_authenticity_reason');
|
||||
// The safety rails that keep a false positive from closing a real PR.
|
||||
expect(SCRIPT).toContain('It NEVER closes a PR on its own');
|
||||
expect(SCRIPT).toContain('are evidence of a HUMAN');
|
||||
});
|
||||
});
|
||||
|
||||
describe('checkTitle (version-first rule)', () => {
|
||||
@@ -417,6 +468,112 @@ describe('detectRedFlags (mechanical, no LLM)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('hasScreenshot (#3745, mechanical)', () => {
|
||||
test('accepts all four embed forms GitHub produces', () => {
|
||||
expect(hasScreenshot('here it is:\n\n')).toBe(true);
|
||||
expect(hasScreenshot('https://user-images.githubusercontent.com/1234/98765-abcdef.png')).toBe(true);
|
||||
expect(hasScreenshot('https://github.com/user-attachments/assets/0a1b2c3d-4e5f-6789')).toBe(true);
|
||||
expect(hasScreenshot('<img width="900" alt="run" src="https://example.com/shot.png">')).toBe(true);
|
||||
});
|
||||
|
||||
test('an embed inside a fenced code block does NOT count', () => {
|
||||
// Pasting the syntax is not attaching the picture.
|
||||
expect(hasScreenshot('```md\n\n```')).toBe(false);
|
||||
expect(hasScreenshot('~~~\n<img src="a.png">\nhttps://github.com/user-attachments/assets/x\n~~~')).toBe(false);
|
||||
// An unterminated fence swallows the rest of the body, not just to the next line.
|
||||
expect(hasScreenshot('```\n')).toBe(false);
|
||||
// ...but one real embed outside the fence is enough.
|
||||
expect(
|
||||
hasScreenshot('```\n\n```\n\n'),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
test('claiming a screenshot is not attaching one', () => {
|
||||
expect(hasScreenshot('I attached a screenshot of my terminal, see above.')).toBe(false);
|
||||
expect(hasScreenshot('')).toBe(false);
|
||||
expect(hasScreenshot(undefined)).toBe(false);
|
||||
expect(hasScreenshot(null)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('intent paragraph detector (#3745, mechanical)', () => {
|
||||
const padding = (n: number) => Array.from({ length: n }, (_, i) => `word${i}`);
|
||||
|
||||
test('a one-liner body is not an intent paragraph', () => {
|
||||
expect(hasIntentParagraph('fixes a thing')).toBe(false);
|
||||
expect(hasIntentParagraph('')).toBe(false);
|
||||
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);
|
||||
// Filling only the "what changed" section is still not the intent paragraph.
|
||||
expect(hasIntentParagraph(`${template}\nrenames the flag and updates the docs`)).toBe(false);
|
||||
});
|
||||
|
||||
test('40+ words of the author own prose counts', () => {
|
||||
expect(intentWordCount(HUMAN_INTENT)).toBeGreaterThanOrEqual(INTENT_MIN_WORDS);
|
||||
expect(hasIntentParagraph(HUMAN_INTENT)).toBe(true);
|
||||
expect(hasIntentParagraph(COMPLIANT_BODY)).toBe(true);
|
||||
// The threshold is the documented one, exercised from both sides.
|
||||
expect(hasIntentParagraph(padding(INTENT_MIN_WORDS).join(' '))).toBe(true);
|
||||
expect(hasIntentParagraph(padding(INTENT_MIN_WORDS - 1).join(' '))).toBe(false);
|
||||
});
|
||||
|
||||
test('pasted code, logs, checklists and headings are not prose', () => {
|
||||
const many = padding(80).join(' ');
|
||||
expect(hasIntentParagraph('```\n' + many + '\n```')).toBe(false);
|
||||
expect(hasIntentParagraph(padding(80).map((w) => `> ${w}`).join('\n'))).toBe(false);
|
||||
expect(hasIntentParagraph(padding(80).map((w) => `- ${w}`).join('\n'))).toBe(false);
|
||||
expect(hasIntentParagraph(padding(80).map((w, i) => `${i + 1}. ${w}`).join('\n'))).toBe(false);
|
||||
expect(hasIntentParagraph(`## ${many}`)).toBe(false);
|
||||
expect(hasIntentParagraph(`**${many}**`)).toBe(false);
|
||||
// A wall of links/screenshots is not a paragraph either.
|
||||
expect(hasIntentParagraph(padding(80).map((w) => ``).join(' '))).toBe(false);
|
||||
});
|
||||
|
||||
test('non-English prose counts — the policy asks for rough words, not English', () => {
|
||||
// Per-character scripts must not read as a single "word" and close a PR
|
||||
// whose author did write their own paragraph.
|
||||
const han =
|
||||
'我在同步笔记仓库的时候遇到了这个问题' +
|
||||
',大概有四千个文件。同步到一半就停了' +
|
||||
',没有任何报错信息,所以我以为它已经' +
|
||||
'完成了。第二天早上发现一半的笔记都不' +
|
||||
'见了,只能手动重新导入。';
|
||||
expect(intentWordCount(han)).toBeGreaterThanOrEqual(INTENT_MIN_WORDS);
|
||||
// Diacritics are letters, not separators.
|
||||
expect(hasIntentParagraph(padding(40).map((w) => `${w}ê`).join(' '))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('detectPolicyMisses (#3745)', () => {
|
||||
const ids = (body: unknown) => detectPolicyMisses(body).map((f) => f.id);
|
||||
|
||||
test('a compliant description has no policy misses', () => {
|
||||
expect(detectPolicyMisses(COMPLIANT_BODY)).toEqual([]);
|
||||
});
|
||||
|
||||
test('flags each half independently', () => {
|
||||
expect(ids(HUMAN_INTENT)).toEqual(['missing_screenshot']);
|
||||
expect(ids(`fixes a thing\n\n${SCREENSHOT_EMBED}`)).toEqual(['missing_intent']);
|
||||
expect(ids('')).toEqual(['missing_intent', 'missing_screenshot']);
|
||||
});
|
||||
|
||||
test('every detail names CONTRIBUTING.md and the policy issue', () => {
|
||||
for (const f of detectPolicyMisses('')) {
|
||||
expect(f.detail).toContain('CONTRIBUTING.md');
|
||||
expect(f.detail).toContain('#3745');
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('applyMechanicalDowngrades (lane is not purely model-decided)', () => {
|
||||
const flag = (id: string) => ({ id, detail: `detail for ${id}` });
|
||||
|
||||
@@ -450,6 +607,43 @@ describe('applyMechanicalDowngrades (lane is not purely model-decided)', () => {
|
||||
expect(r.lane).toBe('needs-maintainer');
|
||||
expect(r.downgrades).toHaveLength(2);
|
||||
});
|
||||
|
||||
test.each(['merge-lane', 'needs-maintainer', 'close-lane'])(
|
||||
'a #3745 policy miss forces close-lane from a %s recommendation',
|
||||
(recommended) => {
|
||||
const r = applyMechanicalDowngrades(recommended, [flag('missing_screenshot')]);
|
||||
expect(r.lane).toBe('close-lane');
|
||||
expect(r.downgrades).toEqual(['detail for missing_screenshot']);
|
||||
},
|
||||
);
|
||||
|
||||
test('a policy miss beats every other flag and reports both halves', () => {
|
||||
const r = applyMechanicalDowngrades('merge-lane', [
|
||||
flag('adds_dependency'),
|
||||
flag('missing_intent'),
|
||||
flag('missing_screenshot'),
|
||||
]);
|
||||
expect(r.lane).toBe('close-lane');
|
||||
expect(r.downgrades).toEqual(['detail for missing_intent', 'detail for missing_screenshot']);
|
||||
});
|
||||
|
||||
test('ai_generated intent routes to needs-maintainer and NEVER to close-lane', () => {
|
||||
expect(applyMechanicalDowngrades('merge-lane', [], 'ai_generated').lane).toBe('needs-maintainer');
|
||||
expect(applyMechanicalDowngrades('needs-maintainer', [], 'ai_generated').lane).toBe('needs-maintainer');
|
||||
// A model close-lane for OTHER reasons still stands; the signal never adds one.
|
||||
expect(applyMechanicalDowngrades('close-lane', [], 'ai_generated').lane).toBe('close-lane');
|
||||
// The downgrade reads as a routing note, not an accusation.
|
||||
const r = applyMechanicalDowngrades('merge-lane', [], 'ai_generated');
|
||||
expect(r.downgrades).toHaveLength(1);
|
||||
expect(r.downgrades[0]).toContain('a maintainer will read');
|
||||
expect(r.downgrades[0]).not.toMatch(/AI-generated|AI-polished|did not write/i);
|
||||
});
|
||||
|
||||
test('human / unclear / absent intent verdicts change nothing', () => {
|
||||
expect(applyMechanicalDowngrades('merge-lane', [], 'human').lane).toBe('merge-lane');
|
||||
expect(applyMechanicalDowngrades('merge-lane', [], 'unclear').lane).toBe('merge-lane');
|
||||
expect(applyMechanicalDowngrades('merge-lane', [], undefined).lane).toBe('merge-lane');
|
||||
});
|
||||
});
|
||||
|
||||
describe('sanitizeModelText (LLM output is never raw Markdown)', () => {
|
||||
@@ -560,7 +754,9 @@ function fixtureDir(pr: Record<string, unknown> = {}, files: unknown[] = [], dif
|
||||
JSON.stringify({
|
||||
number: 7,
|
||||
title: 'fix(core): a real fix',
|
||||
body: 'fixes a thing',
|
||||
// #3745-compliant by default so every pre-existing case still exercises
|
||||
// the lane logic rather than tripping the policy gate first.
|
||||
body: COMPLIANT_BODY,
|
||||
changed_files: 2,
|
||||
head: { sha: 'cafebabe' },
|
||||
user: { login: 'contributor' },
|
||||
@@ -782,7 +978,7 @@ describe('runGate end-to-end (mocked fetch)', () => {
|
||||
}, 30_000);
|
||||
|
||||
test('spend guard: unchanged title+body+head_sha skips the LLM and keeps the verdict', async () => {
|
||||
const pr = { title: 'fix(core): a real fix', body: 'fixes a thing', head: { sha: 'cafebabe' } };
|
||||
const pr = { title: 'fix(core): a real fix', body: COMPLIANT_BODY, head: { sha: 'cafebabe' } };
|
||||
const prior = {
|
||||
id: 55,
|
||||
user: { type: 'Bot', login: 'github-actions[bot]' },
|
||||
@@ -802,7 +998,7 @@ describe('runGate end-to-end (mocked fetch)', () => {
|
||||
});
|
||||
|
||||
test('spend guard does not fire when the head sha moved', async () => {
|
||||
const pr = { title: 'fix(core): a real fix', body: 'fixes a thing', head: { sha: 'cafebabe' } };
|
||||
const pr = { title: 'fix(core): a real fix', body: COMPLIANT_BODY, head: { sha: 'cafebabe' } };
|
||||
const prior = {
|
||||
id: 55,
|
||||
user: { type: 'Bot', login: 'github-actions[bot]' },
|
||||
@@ -820,3 +1016,114 @@ describe('runGate end-to-end (mocked fetch)', () => {
|
||||
expect(calls.some((c) => c.url.startsWith('https://api.anthropic.com'))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// The #3745 policy end-to-end: intent paragraph + screenshot are a hard
|
||||
// requirement; the model's authenticity read is advisory only.
|
||||
// ---------------------------------------------------------------------------
|
||||
describe('runGate — CONTRIBUTING.md #3745 policy (mocked fetch)', () => {
|
||||
const SRC_AND_TEST = [
|
||||
{ filename: 'src/a.ts', status: 'modified', additions: 2, deletions: 1 },
|
||||
{ filename: 'test/a.test.ts', status: 'modified', additions: 5, deletions: 0 },
|
||||
];
|
||||
|
||||
test('a compliant description (screenshot + intent) is judged normally', async () => {
|
||||
const { calls, fetchImpl } = stubFetch({ anthropic: () => verdictResponse(CLEAN_VERDICT) });
|
||||
const code = await runGate(fixtureDir({ body: COMPLIANT_BODY }, SRC_AND_TEST), ENV, fetchImpl);
|
||||
expect(code).toBe(0);
|
||||
expect(addedLabels(calls)).toEqual(['gate:merge-lane']);
|
||||
expect(calls.some((c) => c.url.startsWith('https://api.anthropic.com'))).toBe(true);
|
||||
const body: string = postedBody(calls);
|
||||
expect(body).not.toContain('Almost there');
|
||||
expect(body).toContain('MERGE LANE');
|
||||
});
|
||||
|
||||
test('a missing screenshot closes the PR (exit 1) with the friendly fix-it comment', async () => {
|
||||
// No anthropic handler: reaching the model at all throws. A PR that will
|
||||
// be closed unreviewed must not cost a review call.
|
||||
const { calls, fetchImpl } = stubFetch({});
|
||||
const code = await runGate(fixtureDir({ body: HUMAN_INTENT }, 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);
|
||||
|
||||
const body: string = postedBody(calls);
|
||||
expect(body).toContain('Almost there');
|
||||
expect(body).toContain('A screenshot of gbrain in use');
|
||||
expect(body).not.toContain('A paragraph you wrote yourself'); // that half is fine
|
||||
expect(body).toContain('then reopen');
|
||||
expect(body).toContain('not a judgment on the code');
|
||||
expect(body).toContain('CONTRIBUTING.md');
|
||||
// Also recorded where the other deterministic overrides are recorded.
|
||||
expect(body).toContain('Mechanical downgrades applied');
|
||||
expect(body).toContain('#3745');
|
||||
// The fix-it block leads; the rubric heading does not.
|
||||
expect(body.indexOf('Almost there')).toBeLessThan(body.indexOf('**Label:**'));
|
||||
expect(body).not.toContain('fails the strict usefulness rubric');
|
||||
expect(parseState(body)).toMatchObject({ lane: 'close-lane' });
|
||||
});
|
||||
|
||||
test('a missing intent paragraph closes the PR (exit 1)', async () => {
|
||||
const { calls, fetchImpl } = stubFetch({});
|
||||
const code = await runGate(
|
||||
fixtureDir({ body: `fixes a thing\n\n${SCREENSHOT_EMBED}` }, SRC_AND_TEST),
|
||||
ENV,
|
||||
fetchImpl,
|
||||
);
|
||||
expect(code).toBe(1);
|
||||
expect(addedLabels(calls)).toEqual(['gate:close-lane']);
|
||||
const body: string = postedBody(calls);
|
||||
expect(body).toContain('A paragraph you wrote yourself');
|
||||
expect(body).not.toContain('A screenshot of gbrain in use'); // that half is fine
|
||||
expect(body).toContain('then reopen');
|
||||
});
|
||||
|
||||
test('an empty description names both halves', async () => {
|
||||
const { calls, fetchImpl } = stubFetch({});
|
||||
expect(await runGate(fixtureDir({ body: '' }), ENV, fetchImpl)).toBe(1);
|
||||
const body: string = postedBody(calls);
|
||||
expect(body).toContain('A paragraph you wrote yourself');
|
||||
expect(body).toContain('A screenshot of gbrain in use');
|
||||
});
|
||||
|
||||
test('a policy miss overrides even a merge-lane-shaped clean diff', async () => {
|
||||
// Nothing else about this PR is wrong: clean small diff, src + test, good
|
||||
// title. The policy still closes it.
|
||||
const { calls, fetchImpl } = stubFetch({});
|
||||
expect(await runGate(fixtureDir({ body: 'lgtm' }, SRC_AND_TEST), ENV, fetchImpl)).toBe(1);
|
||||
expect(addedLabels(calls)).toEqual(['gate:close-lane']);
|
||||
expect(deletedLabels(calls).sort()).toEqual(['gate:merge-lane', 'gate:needs-maintainer']);
|
||||
});
|
||||
|
||||
test('ai_generated intent routes to needs-maintainer (exit 0) and never accuses', async () => {
|
||||
const { calls, fetchImpl } = stubFetch({
|
||||
anthropic: () =>
|
||||
verdictResponse({
|
||||
...CLEAN_VERDICT,
|
||||
intent_authenticity: 'ai_generated',
|
||||
intent_authenticity_reason: 'uniform hedging, no first-person specifics, no rough edges',
|
||||
}),
|
||||
});
|
||||
const code = await runGate(fixtureDir({ body: COMPLIANT_BODY }, SRC_AND_TEST), ENV, fetchImpl);
|
||||
expect(code).toBe(0);
|
||||
expect(addedLabels(calls)).toEqual(['gate:needs-maintainer']);
|
||||
|
||||
const body: string = postedBody(calls);
|
||||
expect(body).toContain('a maintainer will read the intent paragraph');
|
||||
// Never the accusation, and never the model's private reasoning.
|
||||
expect(body).not.toMatch(/AI-generated|AI-polished|ai_generated|did not write|uniform hedging/i);
|
||||
expect(parseState(body)).toMatchObject({ lane: 'needs-maintainer' });
|
||||
});
|
||||
|
||||
test('a human / unclear intent verdict leaves the lane alone', async () => {
|
||||
for (const intent of ['human', 'unclear']) {
|
||||
const { calls, fetchImpl } = stubFetch({
|
||||
anthropic: () =>
|
||||
verdictResponse({ ...CLEAN_VERDICT, intent_authenticity: intent, intent_authenticity_reason: 'r' }),
|
||||
});
|
||||
const code = await runGate(fixtureDir({ body: COMPLIANT_BODY }, SRC_AND_TEST), ENV, fetchImpl);
|
||||
expect(code).toBe(0);
|
||||
expect(addedLabels(calls)).toEqual(['gate:merge-lane']);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user