fix(verify-pr): keep the verdict line a plain paragraph and check the comment footer (#16002)

* fix(verify-pr): keep the verdict line a plain paragraph and check the comment footer

Three verification rounds on PR #15984 shipped their opening verdict as a
single Markdown heading: the round title and the whole ~950-character
verdict paragraph shared one `#` line, which GitHub renders as one giant
H1 — a wall of bold text instead of a readable lead. All three comments
also went out without the attribution footer the publish contract
requires.

The skill only said "verdict line first" without pinning the Markdown
shape, and the consistency checker only checked that the verdict word
and head OID appear in the first ten lines, so both defects passed every
gate.

- The skill now pins the verdict line to a plain paragraph (a round
  title may lead it as plain text, never as a heading), with the
  measured #15984 example and a template.
- Local rounds assemble the exact posted body — report content plus the
  resolved footer — into comment.md, post that file, and read the
  comment back against it byte for byte; report.md stays footer-free by
  contract.
- The checker warns when the lead line is a heading carrying the
  verdict or an over-long heading (GFM indents of up to three spaces
  included), and when comment.md lacks a well-formed attribution
  footer. Warnings, not errors: a cosmetic defect must not make the CI
  publisher refuse an otherwise consistent verdict.
- Pin tests reproduce the shipped shape from the real round-3 comment,
  and the workflow meta-test pins the new skill text.

* fix(verify-pr): accept CRLF line endings in the comment-body checks

Review of #16002 reproduced a Windows-authored comment.md failing the
anchored footer regex: splitting only on \n left a stray \r on the
footer line, so a valid attribution was reported missing and the local
publish gate sent the artifact back for unnecessary repair. GitHub
renders CRLF and LF identically.

Normalize CRLF (and lone CR) to LF before the line-based checks on
report.md and comment.md, and pin LF/CRLF parity: the same valid footer
passes under both endings, a missing footer and a heading lead still
warn under CRLF, and the reviewer's CLI probe is covered end to end.
This commit is contained in:
Shaojin Wen 2026-10-31 19:02:46 +08:00 • committed by GitHub
parent d811674c55
commit 35daec35ea
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 281 additions and 17 deletions

View file

@ -390,10 +390,11 @@ cloned repository's settings file cannot choose where evidence is published.
Upload blobs
with `gh api -X PUT repos/.../contents/<path> --input -` carrying a
JSON payload — `-f content="$(base64 …)"` blows past ARG_MAX around 100 KB.
Keep the report.md contract in the comment body: verdict line first,
collapsed `<details>` Chinese summary immediately after, images referenced
by the same kebab-case names the files carry. End the comment with the
attribution footer, the same shape `/review` posts:
Keep the report.md contract in the comment body: verdict line first — a
plain paragraph, never a `#` heading, which renders the whole verdict as
one giant H1 — collapsed `<details>` Chinese summary immediately after,
images referenced by the same kebab-case names the files carry. End the
comment with the attribution footer, the same shape `/review` posts:
```text
_— <model> via Qwen Code /verify-pr (v<cli-version>; skill <skill-hash>)_
@ -407,10 +408,15 @@ the first 12 characters of
hash of the instructions this round actually loaded, which needs no git
history. When the model cannot be resolved, omit the footer rather than
post an empty identity — and never claim the CI lane's `sandboxed
verification` signature for a workstation round. The local path has no
publisher enforcing the fail/mismatch rule, so run
`node scripts/verify-pr-check.mjs <artifact dir>` and clear its errors before
publishing, then state the verdict word from `verdict.txt` verbatim. A published
verification` signature for a workstation round. Assemble the exact comment
body — report content plus the resolved footer — into `comment.md` in the
artifact directory and post that file: it is what the read-back below
compares against, and it gives the checker a place to verify the footer
actually landed. The local path has no publisher enforcing the fail/mismatch
rule, so run `node scripts/verify-pr-check.mjs <artifact dir>` and clear its
errors and its verdict-shape and footer warnings before publishing (a
missing footer is legitimate only when the model could not be resolved),
then state the verdict word from `verdict.txt` verbatim. A published
filename is spent: raw.githubusercontent serves a URL from cache, so
overwriting `02-matrix.png` leaves every earlier link showing the old image.
Republish as `02-matrix-v2.png`, update the body and every comment that quotes
@ -433,10 +439,12 @@ Measured example: while a round ran, a bot review with 4 Criticals landed
against the previous head 20 minutes before the post. It was not read, the
report went out as ready to merge, every blocking finding in it reproduced
on the current head, and the comment had to be revised in place to "Not
ready yet". After posting, read the body back and compare it with the
local file byte for byte, and PATCH in place on any difference: GitHub
rewrites the six-character escape text for NUL and SOH (a backslash, `u`,
then `0000` or `0001`) into a caret form, inside code spans and fences too.
ready yet". After posting, read the body back and compare it with
`comment.md` byte for byte — not with `report.md`, which carries no footer
and would differ on every clean post — and PATCH in place on any difference:
GitHub rewrites the six-character escape text for NUL and SOH (a backslash,
`u`, then `0000` or `0001`) into a caret form, inside code spans and fences
too.
**Run both halves of that guard through `scripts/verify-pr-guard.mjs`, because
the hand-run version fails open.** `--pr <n> --repo <owner>/<repo> --snapshot
@ -1518,6 +1526,11 @@ workflow globs). It must contain:
it: both publish paths append the footer at comment time — the workflow
from its own run metadata, a local round per the publish rule — so a
footer written into the report would appear twice.
- `comment.md` — local rounds only: the exact comment body as posted, the
report content plus the resolved attribution footer. The workflow
assembles its own body and appends its own footer, so it neither writes
nor needs this file; a local round posts it, and the checker verifies the
footer on it because `report.md` deliberately has none.
- `verdict.txt` — exactly one word: `merge-ready` | `findings` | `blocked` |
`inconclusive`. Anything else is discarded by the workflow.
- `assertions.json` — `{"pass": <int>, "fail": <int>, "total": <int>}`,
@ -1607,7 +1620,19 @@ either path.
1. **Verdict line first**, with assertion totals and the verified head OID
(`git rev-parse HEAD^2` in the lane — not the metadata snapshot's, which
may have drifted; the `headRefOid` from `context.json` locally, which
prepare pinned to the clean tree the round verified).
prepare pinned to the clean tree the round verified). The verdict line is
a **plain paragraph, never a heading**: a `#` prefix makes GitHub render
the entire paragraph as one giant H1. A round title may lead the same
line as plain text, but the line must not start with `#`:
```text
PR <n> round <r> verification — **Verdict: <word>** — <pass>/<total> scripted assertions passed, <fail> failed. Verified head: `<40-hex head OID>` …
```
Measured example: three rounds of PR #15984 shipped the whole verdict
paragraph as one H1 because the title and the verdict shared a single
`#` line — the comment opened as a wall of giant bold text.
2. **中文摘要** in a collapsed `<details>` block, **immediately after the
verdict**: verdict, A/B 结论, findings, 未覆盖范围. Collapsed, so it costs a
reader who does not want it exactly one line; placed here rather than at

View file

@ -4986,8 +4986,20 @@ describe('qwen-triage verify hardening round 2', () => {
);
expect(flat).toContain('only `updated_at` moves');
expect(flat).toContain('Keep the guard and the post in separate commands');
// #15984 rounds 1-3: the title and the whole verdict paragraph shared
// one `#` line, which GitHub renders as a single giant H1, and all three
// comments shipped without the attribution footer. The verdict line is
// pinned to a plain paragraph, and the posted body is assembled as
// comment.md so the read-back and the footer check compare against one
// local file — report.md carries no footer by contract.
expect(flat).toContain(
'read the body back and compare it with the local file byte for byte',
'The verdict line is a **plain paragraph, never a heading**',
);
expect(flat).toContain(
'Assemble the exact comment body — report content plus the resolved footer — into `comment.md`',
);
expect(flat).toContain(
'read the body back and compare it with `comment.md` byte for byte',
);
// #13163 R7: a merge carried two review fixes in 507 hand-written lines

View file

@ -45,6 +45,9 @@ function makeArtifactDir(files, evidence = []) {
writeFileSync(join(dir, 'report.md'), files.report);
writeFileSync(join(dir, 'verdict.txt'), files.verdict);
writeFileSync(join(dir, 'assertions.json'), files.assertions);
if (files.comment !== undefined) {
writeFileSync(join(dir, 'comment.md'), files.comment);
}
if (evidence.length > 0) {
mkdirSync(join(dir, 'evidence'));
for (const name of evidence) {
@ -413,6 +416,139 @@ describe('checkMutationCoverage', () => {
});
});
// Three rounds of PR #15984 opened with the title and the whole verdict
// paragraph merged into one `#` line. The Markdown is structurally valid;
// GitHub renders it as a single giant H1, a wall of bold text. The shape is
// a warning, not an error: a cosmetic defect must not make the CI publisher
// refuse an otherwise consistent verdict.
describe('verdict line shape', () => {
const shippedLead = `# PR 15984 round 3 verification — **Verdict: merge-ready** — 63/63 scripted assertions passed, 0 failed. Verified head: \`${HEAD}\`.`;
it('warns when the verdict paragraph is a heading, without failing the round', () => {
const result = check({ report: `${shippedLead}\n` });
expect(result.errors).toEqual([]);
expect(result.warnings.join('\n')).toMatch(
/report\.md opens with a heading carrying the verdict/,
);
});
it('accepts the same line as a plain paragraph', () => {
const result = check({ report: `${shippedLead.slice(2)}\n` });
expect(result.warnings).toEqual([]);
});
it('accepts a short title heading that carries no verdict', () => {
const result = check({ report: `# Report\n\nmerge-ready · ${HEAD}\n` });
expect(result.warnings.join('\n')).not.toMatch(/opens with a heading/);
});
it('warns on an over-long heading even without the verdict word', () => {
const result = check({
report: `# ${'t'.repeat(130)}\n\nmerge-ready · ${HEAD}\n`,
});
expect(result.warnings.join('\n')).toMatch(
/opens with a heading of 132 characters/,
);
});
it('keeps the shape verdicts under CRLF line endings', () => {
const heading = check({ report: `${shippedLead}\r\n` });
expect(heading.warnings.join('\n')).toMatch(
/report\.md opens with a heading carrying the verdict/,
);
const plain = check({ report: `${shippedLead.slice(2)}\r\n` });
expect(plain.warnings).toEqual([]);
});
// GFM renders an ATX heading indented up to three spaces as a heading
// (verified against the GitHub markdown API); four spaces are a code
// block. The indent must not be a way around the shape check.
it('warns on a verdict heading indented up to three spaces', () => {
for (const indent of [' ', ' ', ' ']) {
const result = check({ report: `${indent}${shippedLead}\n` });
expect(result.warnings.join('\n')).toMatch(
/report\.md opens with a heading carrying the verdict/,
);
}
const codeBlock = check({ report: ` ${shippedLead}\n` });
expect(codeBlock.warnings.join('\n')).not.toMatch(/opens with a heading/);
});
});
// report.md deliberately carries no footer — both publish paths append it at
// comment time — so the footer is checkable only on comment.md, the exact
// body a local round posts. All three PR #15984 rounds shipped without it.
describe('comment.md footer', () => {
const footer =
'_— test-model via Qwen Code /verify-pr (v0.1.2; skill 0123456789ab)_';
const body = `merge-ready · 4 passed · 0 failed · 4 total · ${HEAD}\n`;
it('accepts a body ending in the resolved footer', () => {
const result = check({ comment: `${body}\n${footer}\n` });
expect(result.warnings).toEqual([]);
});
// PR #16002 review: a Windows-authored comment.md with CRLF line endings
// failed the anchored footer regex on the stray \r, and the local publish
// gate sent a valid artifact back for unnecessary repair. GitHub renders
// CRLF and LF identically; the checker has to agree.
it('accepts the same footer with CRLF line endings (LF/CRLF parity)', () => {
const lf = check({ comment: `${body}\n${footer}\n` });
const crlf = check({
comment: `${body}\n${footer}\n`.replace(/\n/g, '\r\n'),
});
expect(lf.warnings).toEqual([]);
expect(crlf.warnings).toEqual([]);
});
it('still flags a missing footer and a heading lead under CRLF', () => {
const missing = check({ comment: body.replace(/\n/g, '\r\n') });
expect(missing.warnings.join('\n')).toMatch(
/does not end with the attribution footer/,
);
const heading = check({
comment: `# PR 15984 round 3 verification — **Verdict: merge-ready** — ${HEAD}\r\n\r\n${footer}\r\n`,
});
expect(heading.warnings.join('\n')).toMatch(
/comment\.md opens with a heading carrying the verdict/,
);
});
it('warns when the footer is missing', () => {
const result = check({ comment: body });
expect(result.errors).toEqual([]);
expect(result.warnings.join('\n')).toMatch(
/comment\.md does not end with the attribution footer/,
);
});
it('warns on a malformed footer', () => {
for (const bad of [
'_— test-model via Qwen Code /verify-pr (v0.1.2; skill xyz)_',
'_— test-model via Qwen Code /verify-pr (0.1.2; skill 0123456789ab)_',
'— test-model via Qwen Code /verify-pr (v0.1.2; skill 0123456789ab)',
]) {
const result = check({ comment: `${body}\n${bad}\n` });
expect(result.warnings.join('\n')).toMatch(
/does not end with the attribution footer/,
);
}
});
it('checks the lead shape on the posted body too', () => {
const result = check({
comment: `# PR 15984 round 3 verification — **Verdict: merge-ready** — ${HEAD}\n\n${footer}\n`,
});
expect(result.warnings.join('\n')).toMatch(
/comment\.md opens with a heading carrying the verdict/,
);
});
it('stays silent when the round has no comment.md, which the workflow never writes', () => {
expect(check({}).warnings).toEqual([]);
});
});
// The CLI is the path a maintainer runs, so the exit codes are the contract.
describe.skipIf(process.platform === 'win32')('check (CLI)', () => {
const run = (dir) =>
@ -437,6 +573,42 @@ describe.skipIf(process.platform === 'win32')('check (CLI)', () => {
expect(bad.stdout).toContain('failed: verdict=merge-ready');
});
// The CI publisher parses the last stdout line; shape and footer warnings
// must stay above it and keep the exit code at 0.
it('warns without failing when comment.md lacks the footer', () => {
const dir = makeArtifactDir(
validFiles({ comment: `merge-ready · ${HEAD}\n` }),
['01-ab-cells.png'],
);
const result = run(dir);
expect(result.status).toBe(0);
expect(result.stdout).toContain(
'warn comment.md does not end with the attribution footer',
);
expect(result.stdout).toContain(
'ok: verdict=merge-ready (4 passed · 0 failed · 4 total)',
);
});
// The reviewer's CLI probe: the same valid footer, only the line endings
// changed to CRLF, must not produce the footer warning.
it('accepts a CRLF comment.md with a valid footer end to end', () => {
const dir = makeArtifactDir(
validFiles({
comment: `merge-ready · ${HEAD}\r\n\r\n_— test-model via Qwen Code /verify-pr (v0.26.4; skill 0123456789ab)_\r\n`,
}),
['01-ab-cells.png'],
);
const result = run(dir);
expect(result.status).toBe(0);
expect(result.stdout).not.toContain(
'does not end with the attribution footer',
);
expect(result.stdout).toContain(
'ok: verdict=merge-ready (4 passed · 0 failed · 4 total)',
);
});
it('names the legacy snapshot when the round has none', () => {
const root = mkdtempSync(join(tmpdir(), 'verify-pr-check-snapshot-'));
tmpDirs.push(root);

View file

@ -37,6 +37,16 @@ export const ATTRIBUTIONS = ['pr', 'harness', 'environment'];
const KEBAB_PNG = /^[a-z0-9]+(?:-[a-z0-9]+)*\.png$/;
const OID = /^[0-9a-f]{7,40}$/;
// GFM renders an ATX heading indented up to three spaces as a heading too
// (four spaces become a code block), so the indent is part of the shape,
// not a way around it.
const HEADING = /^ {0,3}#{1,6}\s/;
// A heading is a short title; past this it is a paragraph wearing a heading's
// font size. GitHub renders an H1 at 2em, so a long heading line is a wall of
// giant text however well-formed the Markdown is.
const HEADING_TITLE_MAX = 120;
const FOOTER =
/^_— .+ via Qwen Code \/verify-pr \(v[^;]+; skill [0-9a-f]{12}\)_$/;
const HELP = `Usage: node scripts/verify-pr-check.mjs <artifact-dir>
@ -53,8 +63,11 @@ Checks a local round against the rules the CI publisher enforces:
and lists at least one commit (an empty set makes the claim vacuous)
and warns (without failing) when the report does not lead with its verdict and
head OID, when evidence/ carries no kebab-case PNG capture, or when a round with
failures carries no failures ledger at all.
head OID, when the verdict line is a heading (GitHub renders the whole
paragraph as one giant H1), when evidence/ carries no kebab-case PNG capture,
when a round with failures carries no failures ledger at all, or when
comment.md — the exact body a local round posts, optional — repeats the
heading shape or lacks the attribution footer.
Exit codes: 0 no errors, 1 errors found, 2 usage.
`;
@ -135,6 +148,29 @@ export function checkMutationCoverage(text) {
return { errors };
}
// GitHub renders CRLF and LF identically, so a Windows-authored artifact
// must not fail the line-anchored checks on a stray \r.
function normalizeNewlines(text) {
return text.replace(/\r\n?/g, '\n');
}
/**
* The verdict line is a plain paragraph. Three rounds of PR #15984 opened
* with `# PR … — **Verdict: …** — <the whole verdict paragraph>` on one line,
* which GitHub renders as a single giant H1 — structurally valid Markdown,
* visually a wall of bold text. A heading carrying the verdict, or any
* over-long heading in the lead, is that defect's shape.
*/
function leadShapeWarnings(text, verdict, label) {
const first = text.split('\n').find((line) => line.trim() !== '');
if (!first || !HEADING.test(first)) return [];
const holdsVerdict = verdict !== null && first.includes(verdict);
if (!holdsVerdict && first.length <= HEADING_TITLE_MAX) return [];
return [
`${label} opens with a heading${holdsVerdict ? ' carrying the verdict' : ` of ${first.length} characters`}: GitHub renders the whole line as one giant H1 — the verdict line is a plain paragraph (no leading #), and a heading is a short title on its own line`,
];
}
/**
* Pure check over already-read artifacts. `files` carries raw text or null;
* `evidence` is the PNG file names under evidence/.
@ -268,7 +304,8 @@ export function checkArtifacts({ files, evidence = [], expectedHead = null }) {
errors.push(...checkMutationCoverage(files.mutations).errors);
if (files.report && files.report.trim() !== '') {
const lead = files.report
const report = normalizeNewlines(files.report);
const lead = report
.split('\n')
.filter((line) => line.trim() !== '')
.slice(0, 10);
@ -289,6 +326,23 @@ export function checkArtifacts({ files, evidence = [], expectedHead = null }) {
'report.md does not name a 40-character head OID in its first 10 non-empty lines',
);
}
warnings.push(...leadShapeWarnings(report, verdict, 'report.md'));
}
// comment.md is the exact body a local round posts: report content plus
// the attribution footer. report.md deliberately carries no footer (both
// publish paths append it at comment time), so this is the only artifact
// where the footer's presence is checkable. The workflow assembles its own
// body and never writes this file, so its absence is not a defect.
if (files.comment && files.comment.trim() !== '') {
const comment = normalizeNewlines(files.comment);
warnings.push(...leadShapeWarnings(comment, verdict, 'comment.md'));
const lines = comment.split('\n').filter((line) => line.trim() !== '');
if (!FOOTER.test(lines[lines.length - 1] ?? '')) {
warnings.push(
'comment.md does not end with the attribution footer `_— <model> via Qwen Code /verify-pr (v<version>; skill <12-hex>)_`: append the resolved footer before posting — omit it only when the model cannot be resolved',
);
}
}
if (evidence.length === 0) {
@ -329,6 +383,7 @@ function main(argv) {
verdict: readIfExists(join(dir, 'verdict.txt')),
assertions: readIfExists(join(dir, 'assertions.json')),
mutations: readIfExists(join(dir, 'mutations.json')),
comment: readIfExists(join(dir, 'comment.md')),
};
const evidenceDir = join(dir, 'evidence');
const evidence = existsSync(evidenceDir)