mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-14 00:48:18 +00:00
fix(ci): gate — CommonMark info-string rule, hash the real model payload, close the YAML scanner gap (blind review round 5)
Three defects a second blind reviewer found that survived round 5. All three
reproduced against cf7a1616 before the fix and are pinned after it.
1. The fence scanner still falsely closed compliant descriptions. CommonMark 4.5
forbids a backtick in a BACKTICK fence's info string, so ```foo`bar is an
ordinary paragraph. The scanner opened a fence on it, never found a closer,
and stripped the body to EOF:
body = '```foo`bar' + three sentences of real prose + an image embed
before: intentWordCount 0, misses [missing_intent, missing_screenshot]
after: intentWordCount 52, misses []
Same class as round 5's closing-length bug and the same cost — opening a
block CommonMark would not open deletes the author's prose exactly the way
closing one late did. Tilde fences keep the permissive rule (4.5 restricts
backtick fences only).
2. The spend guard hashed less than the model consumes. hashInputs covered
title, modelBody, head.sha, exemption and policy ids — but the payload also
carries the changed-file list and the diff, and the workflow degrades the
diff to a marker line when the API 406s on a huge one. So a run that
classified with no diff cached a diff-blind verdict, and the next run — real
diff in hand, same title/body/sha — matched the hash and was served that
verdict permanently (measured: anthropicCalls 1 across both runs). The
assembled payload is now folded in as a fixed-width digest, inside the JSON
tuple where quoting still makes a forged boundary impossible. Round 5's
property is re-pinned: a policy fix past the model's body cap invalidates
even when the payload is byte-identical.
3. The workflow's ${{ }}-in-run scanner missed a legal commented block header.
`run: | # shell block` is valid YAML — js-yaml puts the following
interpolation in the script — but the header fell through to the
single-line branch, which captured `| # shell block` as the whole command
and never looked at the block body. The rule protecting against shell
injection reported clean over an interpolating workflow. The scanner now
handles a trailing comment on the header, and scans the comment text too
rather than leaving itself a hiding place. Guarded against js-yaml's actual
parse, not against the scanner's own opinion.
Also corrected one overstated claim in the file's security block: a BARE url in
a sanitized string still autolinks under GFM. That is a self-labelled link and
the deliberate stopping point — the escaping targets the masking characters so
it cannot forge `` or `[click to approve](…)` — but "no live
links" was too strong for what the code does.
Self-audit: all 13 interpolations into the sticky comment are either literals in
this file or pass sanitizeModelText/sanitizeList; the only unsanitized one
(titleCheck.reason) is a hardcoded literal and renders inside a code span. Every
rendered statement matches behavior the code performs. The four realistic-human
fixtures and the three new fence cases all PASS; empty, "fixes bug", the
unfilled template, ten words of lorem and a fenced-away screenshot all still
FAIL.
test/pr-gate-workflow.test.ts 158 -> 163 pass, 0 fail. Six mutations (the
CommonMark rule, the tilde exemption, the payload digest, the runGate wiring,
the scanner's comment group, the header-comment scan) each fail at least one
test. typecheck, actionlint, verify (34/34) green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
committed by
Sina Matian
Sina Matian
co-authored by
Claude Fable 5
Claude Fable 5
parent
f87488ff36
commit
d610a845a8
@@ -69,6 +69,8 @@ export declare function isOwnComment(comment: GhComment | null | undefined): boo
|
||||
|
||||
export declare function hashInputs(
|
||||
pr: PrIdentity & { title?: string; body?: string; head?: { sha?: string } },
|
||||
/** The assembled model payload (changed files + diff). runGate always passes it. */
|
||||
payload?: string,
|
||||
): string;
|
||||
export declare function parseState(body: unknown): { hash: string; lane?: string } | null;
|
||||
|
||||
|
||||
+53
-11
@@ -43,12 +43,16 @@
|
||||
* the marker gets a fresh bot comment instead of a hijacked one.
|
||||
* - EVERY string that is not a literal in THIS file is sanitized before it
|
||||
* reaches Markdown (no HTML comments, no renderable HTML, no live @mentions,
|
||||
* no live image embeds or links, no block markers, no newlines, length- and
|
||||
* count-capped). Markdown counts as much as HTML here: `` and
|
||||
* `[click to approve](…)` forge a green verdict with no angle brackets at
|
||||
* no image embeds, no LABELLED links, no block markers, no newlines, length-
|
||||
* and count-capped). Markdown counts as much as HTML here: ``
|
||||
* and `[click to approve](…)` forge a green verdict with no angle brackets at
|
||||
* all. That includes the mechanical red-flag details: two of them
|
||||
* interpolate PR filenames, and a filename may legally contain a newline, so
|
||||
* they are attacker-controlled too.
|
||||
* Deliberate stopping point: a BARE url left in a sanitized string still
|
||||
* autolinks under GFM. That is a self-labelled link — the reader sees exactly
|
||||
* where it goes — which is why the escaping targets the MASKING characters
|
||||
* (`[`/`]`) rather than mangling every URL a model legitimately cites.
|
||||
* - parseState only reads the state block the bot itself wrote (line 2 of a
|
||||
* marker-leading comment). A block appearing anywhere else in the body is
|
||||
* somebody else's text and is ignored, so hostile content cannot forge a
|
||||
@@ -278,6 +282,16 @@ export const POLICY_SCAN_MAX = 16384;
|
||||
|
||||
const FENCE_OPEN_RE = /^[ \t]{0,3}(`{3,}|~{3,})([^\n]*)$/;
|
||||
|
||||
/**
|
||||
* CommonMark 4.5: a BACKTICK fence's info string may not contain a backtick,
|
||||
* because ``` `foo` ``` on its own line has to stay an ordinary paragraph with
|
||||
* inline code in it. A TILDE fence's info string may contain anything.
|
||||
*
|
||||
* Only ever asked at the OPENING site. A closing fence may carry no info string
|
||||
* at all, so the rule is already subsumed there.
|
||||
*/
|
||||
const opensFence = (m) => m[1][0] === '~' || !m[2].includes('`');
|
||||
|
||||
/**
|
||||
* Drop fenced code blocks (``` or ~~~, unterminated fences run to EOF). A
|
||||
* screenshot pasted inside a fence is documentation of the syntax, not proof.
|
||||
@@ -290,6 +304,11 @@ const FENCE_OPEN_RE = /^[ \t]{0,3}(`{3,}|~{3,})([^\n]*)$/;
|
||||
* A compliant PR that documented fence syntax then failed the intent check and
|
||||
* was closed. (The scanner is also linear, which retires the superlinear-
|
||||
* backtracking hazard the 16KB cap was sized against.)
|
||||
*
|
||||
* Opening too eagerly is the same false-positive class and the same cost: every
|
||||
* line to EOF disappears, the intent paragraph with it, and a compliant
|
||||
* contributor gets a red X. Both rules below therefore err toward NOT opening a
|
||||
* block that CommonMark would not open.
|
||||
*/
|
||||
export const stripCodeFences = (body) => {
|
||||
const out = [];
|
||||
@@ -301,7 +320,7 @@ export const stripCodeFences = (body) => {
|
||||
if (m && m[1][0] === fence.char && m[1].length >= fence.len && m[2].trim() === '') fence = null;
|
||||
continue; // fenced content and the fences themselves are not prose
|
||||
}
|
||||
if (m) {
|
||||
if (m && opensFence(m)) {
|
||||
fence = { char: m[1][0], len: m[1].length };
|
||||
continue;
|
||||
}
|
||||
@@ -829,8 +848,9 @@ async function setLaneLabel(gh, repo, prNumber, lane) {
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Spend guard: `edited` + `synchronize` amplify a single PR into many runs.
|
||||
// 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.
|
||||
// The verdict is a function of the model payload (and of the mechanical policy
|
||||
// outcome), so if that payload is byte-identical to the one behind 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
|
||||
@@ -847,14 +867,31 @@ async function setLaneLabel(gh, repo, prNumber, lane) {
|
||||
// from the full (16KB-capped) body, so its outcome is hashed alongside the
|
||||
// truncated text — otherwise adding the missing screenshot past 6KB would leave
|
||||
// the hash unchanged and the cached close-lane would be served forever.
|
||||
//
|
||||
// "What the run consumes" is the WHOLE model payload, not just the body. The
|
||||
// changed-file list and the diff are in it too, and the workflow degrades the
|
||||
// diff to a one-line marker when the API 406s on a huge one. Hashing only the
|
||||
// body made that degradation permanent: run 1 fetched no diff and cached a
|
||||
// diff-blind verdict, run 2 had the real diff, matched the hash, and served the
|
||||
// diff-blind verdict forever. So the assembled payload is folded in as a
|
||||
// fixed-width digest — inside the tuple, where JSON.stringify's quoting still
|
||||
// makes a forged boundary impossible.
|
||||
export const MODEL_BODY_MAX = 6000;
|
||||
export const modelBody = (pr) => (pr?.body ?? '(empty)').slice(0, MODEL_BODY_MAX);
|
||||
|
||||
export function hashInputs(pr) {
|
||||
/**
|
||||
* @param payload the exact string buildPayload() hands the model. Omitted only
|
||||
* by unit tests comparing two prs against each other; runGate always passes
|
||||
* it, pinned by the diff-unavailable→available test.
|
||||
*/
|
||||
export function hashInputs(pr, payload = '') {
|
||||
const exemption = policyExemption(pr) ?? '';
|
||||
const policy = exemption ? [] : detectPolicyMisses(pr?.body).map((f) => f.id);
|
||||
const payloadDigest = createHash('sha256').update(String(payload ?? '')).digest('hex');
|
||||
return createHash('sha256')
|
||||
.update(JSON.stringify([pr.title ?? '', modelBody(pr), pr.head?.sha ?? '', exemption, policy]))
|
||||
.update(
|
||||
JSON.stringify([pr.title ?? '', modelBody(pr), pr.head?.sha ?? '', exemption, policy, payloadDigest]),
|
||||
)
|
||||
.digest('hex')
|
||||
.slice(0, 16);
|
||||
}
|
||||
@@ -1038,7 +1075,12 @@ export async function runGate(dir, env = process.env, fetchImpl = fetch) {
|
||||
return 0;
|
||||
};
|
||||
|
||||
const inputHash = hashInputs(pr);
|
||||
// Built once, unconditionally, and hashed: the spend guard must key on the
|
||||
// bytes the model actually sees. Building it on the policy-miss path too
|
||||
// (where no model call happens) keeps ONE hash convention across both paths —
|
||||
// two conventions is how a cached verdict gets served to the wrong inputs.
|
||||
const payload = buildPayload({ pr, files, diff, titleCheck, flags });
|
||||
const inputHash = hashInputs(pr, payload);
|
||||
let verdict;
|
||||
let degraded = null;
|
||||
if (policyMisses.length > 0) {
|
||||
@@ -1069,13 +1111,13 @@ export async function runGate(dir, env = process.env, fetchImpl = fetch) {
|
||||
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}.`,
|
||||
`PR gate: model payload 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);
|
||||
verdict = await callAnthropic(apiKey, payload, fetchImpl);
|
||||
} catch (err) {
|
||||
const detail = String(err?.message ?? err).slice(0, 200);
|
||||
if (err?.kind !== 'refusal' && err?.kind !== 'schema') {
|
||||
|
||||
+154
-20
@@ -13,9 +13,15 @@
|
||||
* used to red-X (bullet-point prose, non-native English, a terse bug report,
|
||||
* a body that is mostly a stack trace) are pinned as PASSING forever, with
|
||||
* the zero-effort bodies that must still fail beside them.
|
||||
* - CommonMark fence matching in BOTH directions: a closing fence longer than
|
||||
* its opener closes, and a backtick fence whose info string contains a
|
||||
* backtick never opens (4.5). Opening a block CommonMark would not open
|
||||
* strips the author's prose to EOF — the same red X as closing one late.
|
||||
* - 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.
|
||||
* NEUTRAL label clearing, label swap, and the spend guard — which keys on the
|
||||
* whole model payload, so a verdict reached while the diff was unavailable is
|
||||
* not served back once the real diff arrives.
|
||||
* - The CONTRIBUTING.md #3745 policy: the mechanical screenshot + intent
|
||||
* detectors (all four embed forms, the in-code-fence negative, the real
|
||||
* .github/pull_request_template.md, non-English prose), the forced
|
||||
@@ -27,6 +33,7 @@
|
||||
* and through a 500, while a compliant PR keeps the loud NEUTRAL skip.
|
||||
*/
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { safeLoad as yamlLoad } from 'js-yaml';
|
||||
import { readFileSync, existsSync, mkdtempSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
@@ -85,16 +92,24 @@ const PR_TEMPLATE = readFileSync(PR_TEMPLATE_PATH, 'utf8');
|
||||
/**
|
||||
* Collect every line that belongs to a `run:` script, in EVERY YAML block
|
||||
* scalar spelling: `run: cmd`, `run: |`, `run: >`, the `-`/`+` chomping
|
||||
* indicators, and the numeric indentation indicator in either order (`|2-`
|
||||
* and `|-2` are both legal headers). A spelling the scanner cannot see hides
|
||||
* interpolation from the env-binding rule, which is exactly how that rule rots.
|
||||
* indicators, the numeric indentation indicator in either order (`|2-` and
|
||||
* `|-2` are both legal headers), and a trailing comment after the header
|
||||
* (`run: | # shell block` is legal YAML — js-yaml parses it as a block, pinned
|
||||
* below). A spelling the scanner cannot see hides interpolation from the
|
||||
* env-binding rule, which is exactly how that rule rots: the comment spelling
|
||||
* used to fall through to the single-line branch, which captured the HEADER
|
||||
* (`| # shell block`) as if it were the whole command and never looked at the
|
||||
* block body at all — a clean report over an interpolating workflow.
|
||||
*/
|
||||
function runBlockLines(yaml: string): string[] {
|
||||
const lines = yaml.split('\n');
|
||||
const out: string[] = [];
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
const block = lines[i].match(/^(\s*)(?:-\s+)?run:\s*[|>][0-9]*[-+]?[0-9]*\s*$/);
|
||||
const block = lines[i].match(/^(\s*)(?:-\s+)?run:\s*[|>][0-9]*[-+]?[0-9]*([ \t]+#.*)?\s*$/);
|
||||
if (block) {
|
||||
// A `${{ }}` in a YAML comment is inert (it is not part of the scalar),
|
||||
// but scan it anyway rather than leave the scanner a hiding place.
|
||||
if (block[2]) out.push(block[2]);
|
||||
const baseIndent = block[1].length;
|
||||
for (let j = i + 1; j < lines.length; j++) {
|
||||
if (lines[j].trim() === '') continue;
|
||||
@@ -186,6 +201,38 @@ describe('pr-gate workflow security pins', () => {
|
||||
}
|
||||
});
|
||||
|
||||
test('the run: scanner sees a block header carrying a trailing comment', () => {
|
||||
// Guards the guard against REALITY, not against the scanner's own opinion.
|
||||
// `run: | # shell block` is a legal block header, and the interpolation on
|
||||
// the next line really does end up in the script — so a purely cosmetic
|
||||
// formatting edit must not be able to blind the env-binding rule above.
|
||||
const yaml = [
|
||||
'jobs:',
|
||||
' x:',
|
||||
' steps:',
|
||||
' - run: | # shell block',
|
||||
' echo ${{ github.event.pull_request.title }}',
|
||||
].join('\n');
|
||||
const parsed = yamlLoad(yaml) as { jobs: { x: { steps: { run: string }[] } } };
|
||||
expect(parsed.jobs.x.steps[0].run).toContain('${{'); // YAML really puts it in the script…
|
||||
expect(runBlockLines(yaml).join('\n')).toContain('${{'); // …and the scanner really sees it.
|
||||
// The comment composes with every chomping/indentation spelling.
|
||||
for (const header of ['|', '>', '|-', '>2-', '|+2']) {
|
||||
const y = [
|
||||
'jobs:',
|
||||
' x:',
|
||||
' steps:',
|
||||
` - run: ${header} # note`,
|
||||
' echo ${{ github.head_ref }}',
|
||||
].join('\n');
|
||||
expect(runBlockLines(y).join('\n')).toContain('${{');
|
||||
}
|
||||
// A `${{ }}` inside the header comment is inert YAML, but it is scanned
|
||||
// anyway — the scanner is not left a hiding place.
|
||||
const inComment = ['jobs:', ' x:', ' steps:', ' - run: | # ${{ github.head_ref }}', ' echo hi'].join('\n');
|
||||
expect(runBlockLines(inComment).join('\n')).toContain('${{');
|
||||
});
|
||||
|
||||
test('all actions are SHA-pinned', () => {
|
||||
const uses = [...WORKFLOW.matchAll(/uses:\s*(\S+)/g)].map((m) => m[1]);
|
||||
expect(uses.length).toBeGreaterThan(0);
|
||||
@@ -676,6 +723,35 @@ describe('stripCodeFences (CommonMark fence matching)', () => {
|
||||
// ```` ```js ```` opens; a second ` ```js ` line is content, not a closer.
|
||||
expect(stripCodeFences('```js\nhidden\n```js\nstill hidden')).not.toContain('still hidden');
|
||||
});
|
||||
|
||||
test('a backtick fence info string may not contain a backtick (CommonMark 4.5)', () => {
|
||||
// The other half of the false-positive class above. Opening a block that
|
||||
// CommonMark never opens costs exactly what closing one late costs: every
|
||||
// line to EOF disappears, the intent paragraph with it, red X on a body
|
||||
// GitHub renders perfectly.
|
||||
const backtickInfo = `\`\`\`foo\`bar\n${prose}`;
|
||||
expect(stripCodeFences(backtickInfo)).toContain('real human intent paragraph');
|
||||
expect(hasIntentParagraph(backtickInfo)).toBe(true);
|
||||
|
||||
// A TILDE fence has no such restriction — this one really does open.
|
||||
const tildeInfo = `~~~foo\`bar\n${prose}`;
|
||||
expect(stripCodeFences(tildeInfo)).not.toContain('real human intent paragraph');
|
||||
expect(hasIntentParagraph(tildeInfo)).toBe(false);
|
||||
|
||||
// And a backtick-free info string still opens a backtick fence, as always.
|
||||
expect(stripCodeFences(`\`\`\`js\n${prose}`)).not.toContain('real human intent paragraph');
|
||||
expect(stripCodeFences('```js\nhidden\n```\nvisible')).toContain('visible');
|
||||
expect(stripCodeFences('```js\nhidden\n```\nvisible')).not.toContain('hidden');
|
||||
});
|
||||
|
||||
test('the reported false positive, end to end: prose + screenshot after such a line', () => {
|
||||
// Verbatim shape of the repro: a line whose info string carries a backtick,
|
||||
// then real prose, then a real embed. Both #3745 halves are present in the
|
||||
// rendered description, so the gate must report neither as missing.
|
||||
const body = `\`\`\`foo\`bar\n${prose}\n${SCREENSHOT_EMBED}`;
|
||||
expect(intentWordCount(body)).toBeGreaterThanOrEqual(INTENT_MIN_WORDS);
|
||||
expect(detectPolicyMisses(body)).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('hasScreenshot (#3745, mechanical)', () => {
|
||||
@@ -1359,6 +1435,27 @@ describe('isOwnComment / hashInputs / parseState', () => {
|
||||
expect(detectPolicyMisses(before.body).map((f) => f.id)).toEqual(['missing_screenshot']);
|
||||
expect(detectPolicyMisses(after.body)).toEqual([]);
|
||||
expect(hashInputs(before)).not.toBe(hashInputs(after));
|
||||
// Round 5's property, re-pinned with the payload term present: the policy
|
||||
// outcome must still invalidate even when the model payload is identical.
|
||||
expect(modelBody(before)).toBe(modelBody(after)); // same 6KB window
|
||||
expect(hashInputs(before, 'identical payload')).not.toBe(hashInputs(after, 'identical payload'));
|
||||
});
|
||||
|
||||
// The model reads the changed-file list and the diff too, and the workflow
|
||||
// degrades the diff to a marker line when the API 406s on a huge one. Hashing
|
||||
// only the PR fields froze that: a run that classified with no diff cached its
|
||||
// verdict, and the next run — real diff in hand — matched the hash and served
|
||||
// the diff-blind verdict forever.
|
||||
test('the hash covers the model payload, not just the PR fields', () => {
|
||||
const pr = { title: 't', body: COMPLIANT_BODY, head: { sha: 'abc' } };
|
||||
const noDiff = '--- UNTRUSTED DIFF ---\n[diff unavailable from the GitHub API]';
|
||||
const realDiff = '--- UNTRUSTED DIFF ---\ndiff --git a/src/a.ts b/src/a.ts\n+real';
|
||||
expect(hashInputs(pr, noDiff)).toBe(hashInputs(pr, noDiff));
|
||||
expect(hashInputs(pr, noDiff)).not.toBe(hashInputs(pr, realDiff));
|
||||
// A changed FILE LIST with the same diff is a different payload too.
|
||||
expect(hashInputs(pr, `added src/b.ts\n${realDiff}`)).not.toBe(hashInputs(pr, realDiff));
|
||||
// …and the payload cannot silently drop out: omitting it is its own input.
|
||||
expect(hashInputs(pr, noDiff)).not.toBe(hashInputs(pr));
|
||||
});
|
||||
|
||||
test('state round-trips through the rendered comment', () => {
|
||||
@@ -1664,26 +1761,63 @@ describe('runGate end-to-end (mocked fetch)', () => {
|
||||
expect(calls.filter((c) => c.url.startsWith('https://api.anthropic.com'))).toHaveLength(3);
|
||||
}, 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: COMPLIANT_BODY, head: { sha: 'cafebabe' } };
|
||||
const prior = {
|
||||
id: 55,
|
||||
user: { type: 'Bot', login: 'github-actions[bot]' },
|
||||
body: renderComment({
|
||||
lane: 'close-lane',
|
||||
verdict: { confidence: 0.9, reasons: ['drive-by refactor'], reviewer_checklist: ['c'] },
|
||||
titleCheck: { ok: true },
|
||||
flags: [],
|
||||
state: { hash: hashInputs(pr), lane: 'close-lane' },
|
||||
}),
|
||||
};
|
||||
const { calls, fetchImpl } = stubFetch({ comments: [prior] }); // no anthropic handler: any call throws
|
||||
const code = await runGate(fixtureDir(pr), ENV, fetchImpl);
|
||||
test('spend guard: an unchanged PR skips the LLM and keeps the verdict', async () => {
|
||||
// Round-tripped through the gate's OWN state block rather than a hash
|
||||
// recomputed here: hand-building the expected hash would re-implement
|
||||
// runGate's payload assembly in the test and pin the test's idea of the
|
||||
// inputs instead of the gate's.
|
||||
const dir = fixtureDir({ title: 'fix(core): a real fix', body: COMPLIANT_BODY, head: { sha: 'cafebabe' } });
|
||||
const comments: any[] = [];
|
||||
const first = stubFetch({
|
||||
comments,
|
||||
persistComments: true,
|
||||
anthropic: () => verdictResponse({ ...CLEAN_VERDICT, lane: 'close-lane', reasons: ['drive-by refactor'] }),
|
||||
});
|
||||
expect(await runGate(dir, ENV, first.fetchImpl)).toBe(1);
|
||||
expect(comments).toHaveLength(1);
|
||||
|
||||
// Same dir, same everything. No anthropic handler: any call throws.
|
||||
const { calls, fetchImpl } = stubFetch({ comments });
|
||||
const code = await runGate(dir, ENV, fetchImpl);
|
||||
expect(code).toBe(1); // the stored close-lane verdict still holds
|
||||
expect(calls.some((c) => c.url.startsWith('https://api.anthropic.com'))).toBe(false);
|
||||
expect(calls.some((c) => c.method === 'PATCH' || c.method === 'POST')).toBe(false); // nothing rewritten
|
||||
});
|
||||
|
||||
test('spend guard does not serve a diff-blind verdict once the diff is available', async () => {
|
||||
// The workflow degrades to `[diff unavailable …]` when the GitHub API 406s
|
||||
// on a huge diff. Run 1 therefore classifies with NO diff. Run 2 has the
|
||||
// real one: same title, same body, same head sha — only the payload moved,
|
||||
// and that alone has to buy a second verdict. Otherwise the diff-blind
|
||||
// verdict is the permanent one.
|
||||
const comments: any[] = [];
|
||||
const { calls, fetchImpl } = stubFetch({
|
||||
comments,
|
||||
persistComments: true,
|
||||
anthropic: () => verdictResponse(CLEAN_VERDICT),
|
||||
});
|
||||
const files = [
|
||||
{ filename: 'src/a.ts', status: 'modified', additions: 2, deletions: 1 },
|
||||
{ filename: 'test/a.test.ts', status: 'modified', additions: 5, deletions: 0 },
|
||||
];
|
||||
const REAL_DIFF = 'diff --git a/src/a.ts b/src/a.ts\n@@ -1 +1 @@\n-old\n+new\n';
|
||||
const anthropicCalls = () => calls.filter((c) => c.url.startsWith('https://api.anthropic.com')).length;
|
||||
|
||||
const unavailable = '[diff unavailable from the GitHub API — too large or unfetchable]\n';
|
||||
await runGate(fixtureDir({}, files, unavailable), ENV, fetchImpl);
|
||||
expect(anthropicCalls()).toBe(1);
|
||||
expect(comments).toHaveLength(1); // the diff-blind verdict is cached
|
||||
|
||||
await runGate(fixtureDir({}, files, REAL_DIFF), ENV, fetchImpl);
|
||||
expect(anthropicCalls()).toBe(2); // …and is NOT what run 2 gets served
|
||||
|
||||
// Control: a third run on the SAME payload still short-circuits. The guard
|
||||
// was fixed, not switched off.
|
||||
calls.length = 0;
|
||||
await runGate(fixtureDir({}, files, REAL_DIFF), ENV, fetchImpl);
|
||||
expect(anthropicCalls()).toBe(0);
|
||||
});
|
||||
|
||||
// "Exactly one gate:* label" is only true if a failed label call can be
|
||||
// repaired. The sticky comment carries the cached state that makes a rerun
|
||||
// short-circuit, so writing it BEFORE the labels are reconciled turns one
|
||||
|
||||
Reference in New Issue
Block a user