diff --git a/core/scripts/check-arch-docs.mjs b/core/scripts/check-arch-docs.mjs index 5f1fbc95b..8fb029383 100644 --- a/core/scripts/check-arch-docs.mjs +++ b/core/scripts/check-arch-docs.mjs @@ -123,6 +123,22 @@ export function filesNeedDocs({ ref = baseRef(), map = null } = {}) { return { ok: missing.length === 0, missing, files }; } +// #1076: the trailer this gate looks for is read from `git log ..HEAD`, and on a merge-queue +// entry that range holds the squash of the pull request title and body — the branch commits are not in +// it. So "add a trailer" sent people to the commit they had just pushed, and the entry went red again. +// The refusal names both places, and says why the body counts. +export function failureLines(missing, root = ROOT) { + const lines = ['arch-docs: FAIL — owned file(s) changed without subsystem doc update:']; + for (const m of missing) lines.push(` ${m.file} → ${m.subsystem} (${relative(root, m.doc)})`); + lines.push('Fix it one of two ways:'); + lines.push(' - update the subsystem doc in docs/subsystems/ in this pull request, or'); + lines.push(' - add a line `Docs-not-needed: ` to the pull request body AND to the branch commit.'); + lines.push(' The merge queue squashes the title and body into the commit it judges, so a trailer'); + lines.push(' that lives only in a branch commit is gone exactly when main is judged'); + lines.push(' (AGENTS.md → Docs-not-needed: why both places).'); + return lines; +} + function main() { let result; try { @@ -136,11 +152,7 @@ function main() { console.log(`arch-docs: OK (${files.length} changed file(s))`); process.exit(0); } - console.error('arch-docs: FAIL — owned file(s) changed without subsystem doc update:'); - for (const m of missing) { - console.error(` ${m.file} → ${m.subsystem} (${relative(ROOT, m.doc)})`); - } - console.error('Update the subsystem doc in docs/subsystems/ or add a "Docs-not-needed:" trailer.'); + for (const line of failureLines(missing)) console.error(line); process.exit(1); } diff --git a/core/test/check-arch-docs.test.js b/core/test/check-arch-docs.test.js index e756d65b6..05e8085f0 100644 --- a/core/test/check-arch-docs.test.js +++ b/core/test/check-arch-docs.test.js @@ -49,3 +49,16 @@ test('invalid base ref fails closed', async () => { assert.throws(() => filesNeedDocs({ ref: bad }), /cannot diff against/); assert.throws(() => hasDocsNotNeededTrailer(bad), /cannot diff against/); }); + +test('#1076: the refusal names both places the trailer has to appear', async () => { + // The gate reads `git log ..HEAD`; on a merge-queue entry that is the squashed pull request + // title and body, so a branch-only trailer is gone exactly when main is judged. The old text said + // "add a trailer" and sent the author to the commit they had just pushed. + const { failureLines } = await import('../scripts/check-arch-docs.mjs'); + assert.equal(typeof failureLines, 'function', 'the refusal text is readable without running the gate'); + const text = failureLines([{ file: 'plugins/social/test/social.test.js', subsystem: 'social', doc: join(tmpdir(), 'docs/subsystems/social.md') }]).join('\n'); + assert.match(text, /pull request body/); + assert.match(text, /branch commit/); + assert.match(text, /squash/i); + assert.match(text, /Docs-not-needed:/); +}); diff --git a/docs/autonomous-dev.md b/docs/autonomous-dev.md index fcb473969..93361a6db 100644 --- a/docs/autonomous-dev.md +++ b/docs/autonomous-dev.md @@ -299,7 +299,7 @@ and the `test` job never report there and the entry waits until the queue's time that is intended, and they must stay out of the required checks, or every entry waits until the queue's timeout. - The `arch-docs` step fetches `origin/main` and deepens until `git merge-base origin/main HEAD` works. Emulated on a one-commit shallow checkout of a merge-queue-shaped commit (local bare remote): it falls back to deepening and - `arch-docs: OK`. With a group size above 1, `arch-docs` sees the union of the grouped PRs; keep the group size at 1. + `arch-docs: OK`. With a group size above 1, `arch-docs` sees the union of the grouped PRs; keep the group size at 1. The trailer it looks for has to be in the pull request body as well as in the branch commit: on a queue entry the range it logs is the squashed body, and the refusal says so (#1076). - Observed since the queue was enabled: `merge_group` runs on `gh-readonly-queue/main/pr--` pass and the PR merges (e.g. #217 → `38c00b4`, #235 → `7a3ccb3`). **Read-only checks of the live repository (2026-10-15, rulesets re-read 2026-10-18; `gh api` GET with a maintainer login):** diff --git a/docs/subsystems/install.md b/docs/subsystems/install.md index de30a8274..df404f882 100644 --- a/docs/subsystems/install.md +++ b/docs/subsystems/install.md @@ -31,7 +31,7 @@ Tooling for setup checks, the architecture-docs gate and the container image (#3 | File | Role | | --- | --- | -| `core/scripts/check-arch-docs.mjs` | owned_paths ↔ docs gate. | +| `core/scripts/check-arch-docs.mjs` | owned_paths ↔ docs gate. Its refusal names both places a `Docs-not-needed:` trailer has to appear — the pull request body and the branch commit — because the gate reads `git log ..HEAD` and, on a merge-queue entry, that range is the squashed body (#1076). | | `core/test/check-arch-docs.test.js` | Tests for the gate. | | `core/test/install-doctor.test.js` | Install doctor tests. | | `plugins/README.md` | Plugin list. | diff --git a/patches/ds-bot/files/scripts/check-arch-docs.mjs b/patches/ds-bot/files/scripts/check-arch-docs.mjs index 5f1fbc95b..8fb029383 100644 --- a/patches/ds-bot/files/scripts/check-arch-docs.mjs +++ b/patches/ds-bot/files/scripts/check-arch-docs.mjs @@ -123,6 +123,22 @@ export function filesNeedDocs({ ref = baseRef(), map = null } = {}) { return { ok: missing.length === 0, missing, files }; } +// #1076: the trailer this gate looks for is read from `git log ..HEAD`, and on a merge-queue +// entry that range holds the squash of the pull request title and body — the branch commits are not in +// it. So "add a trailer" sent people to the commit they had just pushed, and the entry went red again. +// The refusal names both places, and says why the body counts. +export function failureLines(missing, root = ROOT) { + const lines = ['arch-docs: FAIL — owned file(s) changed without subsystem doc update:']; + for (const m of missing) lines.push(` ${m.file} → ${m.subsystem} (${relative(root, m.doc)})`); + lines.push('Fix it one of two ways:'); + lines.push(' - update the subsystem doc in docs/subsystems/ in this pull request, or'); + lines.push(' - add a line `Docs-not-needed: ` to the pull request body AND to the branch commit.'); + lines.push(' The merge queue squashes the title and body into the commit it judges, so a trailer'); + lines.push(' that lives only in a branch commit is gone exactly when main is judged'); + lines.push(' (AGENTS.md → Docs-not-needed: why both places).'); + return lines; +} + function main() { let result; try { @@ -136,11 +152,7 @@ function main() { console.log(`arch-docs: OK (${files.length} changed file(s))`); process.exit(0); } - console.error('arch-docs: FAIL — owned file(s) changed without subsystem doc update:'); - for (const m of missing) { - console.error(` ${m.file} → ${m.subsystem} (${relative(ROOT, m.doc)})`); - } - console.error('Update the subsystem doc in docs/subsystems/ or add a "Docs-not-needed:" trailer.'); + for (const line of failureLines(missing)) console.error(line); process.exit(1); } diff --git a/patches/ds-bot/files/test/check-arch-docs.test.js b/patches/ds-bot/files/test/check-arch-docs.test.js index e756d65b6..05e8085f0 100644 --- a/patches/ds-bot/files/test/check-arch-docs.test.js +++ b/patches/ds-bot/files/test/check-arch-docs.test.js @@ -49,3 +49,16 @@ test('invalid base ref fails closed', async () => { assert.throws(() => filesNeedDocs({ ref: bad }), /cannot diff against/); assert.throws(() => hasDocsNotNeededTrailer(bad), /cannot diff against/); }); + +test('#1076: the refusal names both places the trailer has to appear', async () => { + // The gate reads `git log ..HEAD`; on a merge-queue entry that is the squashed pull request + // title and body, so a branch-only trailer is gone exactly when main is judged. The old text said + // "add a trailer" and sent the author to the commit they had just pushed. + const { failureLines } = await import('../scripts/check-arch-docs.mjs'); + assert.equal(typeof failureLines, 'function', 'the refusal text is readable without running the gate'); + const text = failureLines([{ file: 'plugins/social/test/social.test.js', subsystem: 'social', doc: join(tmpdir(), 'docs/subsystems/social.md') }]).join('\n'); + assert.match(text, /pull request body/); + assert.match(text, /branch commit/); + assert.match(text, /squash/i); + assert.match(text, /Docs-not-needed:/); +});