From 054c93a62ba46cf197b6aff047dd3bd747f2e17e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 00:32:27 +0000 Subject: [PATCH] =?UTF-8?q?fix(tooling):=20=E5=8F=91=E5=B8=83=E8=AF=B4?= =?UTF-8?q?=E6=98=8E=E9=A1=B5=E7=95=99=E5=9C=A8=E5=AE=A1=E8=AE=A1=E8=8C=83?= =?UTF-8?q?=E5=9B=B4=E5=86=85,=E4=BD=86=E9=99=8D=E7=BA=A7=E4=B8=BA?= =?UTF-8?q?=E5=8F=AA=E8=AF=BB=E9=80=9A=E9=81=93=20=E2=80=94=E2=80=94=20?= =?UTF-8?q?=E6=8A=A5=20finding,=E4=B8=8D=E6=94=B9=E7=9B=98=20(#4920)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `docs-accuracy-audit` 的交付物是就地改写 mdx,而它的范围包含 `content/docs/releases/**` 9 页 —— AGENTS.md「Documentation Guardrails」 明确禁止代码 PR 编辑这些页面。跑一轮 full audit,产出的正是那条 guardrail 要拦的 PR。 裁决是不从范围里删(那会让读者最多的页面永远没人审,且在生成清单之外 再造一份「审计覆盖哪些文档」的定义 —— #4851 刚为此付过账),只分流交付物: 路径前缀 `content/docs/releases/`(guardrail 路径列原文,VM 内可判定)把 这 9 页导向只读评审通道 —— 禁改的 prompt、没有 fixesApplied 的 finding schema、findings → 立 issue。 沉默跳过被同样否掉:只读页零结果按名判红,agent 自陈 filesEdited 则点名 文件判红,汇总恒定输出 `releases (read-only): N finding(s) — file issues, do not edit`(N=0 也输出)。`check:docs-audit-scope` 把前缀锚到 AGENTS.md 的 guardrail 行,并用 stub agent 真跑工作流来验证分流仍然生效;self-test 把分流从内存副本里删掉,要求该检查判红。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018iARDqtrhQgz6fVHDeDkbQ --- .changeset/docs-audit-releases-read-only.md | 22 ++ .claude/workflows/docs-accuracy-audit.js | 250 +++++++++++-- scripts/docs-audit/README.md | 46 ++- scripts/docs-audit/check-audit-scope.mjs | 389 +++++++++++++++++++- 4 files changed, 673 insertions(+), 34 deletions(-) create mode 100644 .changeset/docs-audit-releases-read-only.md diff --git a/.changeset/docs-audit-releases-read-only.md b/.changeset/docs-audit-releases-read-only.md new file mode 100644 index 0000000000..d90f3c2900 --- /dev/null +++ b/.changeset/docs-audit-releases-read-only.md @@ -0,0 +1,22 @@ +--- +--- + +tooling: audit release-owned docs read-only instead of editing them (#4920) + +`content/docs/releases/**` stays in the `docs-accuracy-audit` scope but is now reviewed +**read-only**: those 9 pages get a review prompt that forbids edits and a finding schema +with no `fixesApplied`, and their deliverable is a list of evidence-backed findings to +file as issues. AGENTS.md marks the directory RELEASE-OWNED — release notes are compiled +centrally at release time — while the audit's deliverable is an in-place mdx rewrite, so +a full audit used to open exactly the PR that guardrail exists to stop. + +Excluding them was rejected: it would leave the most-read pages permanently unaudited +and add a second definition of the audit's scope next to the generated one (#4851). +Silence was rejected too — a release page that produces no result, or whose agent admits +it edited the file, fails the run by name, and the summary always carries +`releases (read-only): N finding(s) — file issues, do not edit`, zero findings included. + +`pnpm check:docs-audit-scope` now also anchors the prefix to AGENTS.md's guardrail row, +requires release pages to still be in scope, and verifies the read-only fork by running +the workflow against stub agents; the self-test mutates the fork away and requires that +check to go red. diff --git a/.claude/workflows/docs-accuracy-audit.js b/.claude/workflows/docs-accuracy-audit.js index ed2bd01497..65c424c462 100644 --- a/.claude/workflows/docs-accuracy-audit.js +++ b/.claude/workflows/docs-accuracy-audit.js @@ -4,7 +4,7 @@ export const meta = { whenToUse: 'Periodic or change-scoped documentation accuracy verification. Pass args.docs = [paths] to scope (e.g. output of scripts/docs-audit/affected-docs.mjs); omit for a full audit of every hand-written doc.', phases: [ { title: 'Scope Preflight', detail: 'resolve every doc path on disk; abort naming any that does not exist' }, - { title: 'Audit & Fix', detail: 'one agent per doc: read, locate implementation, apply evidence-backed edits' }, + { title: 'Audit & Fix', detail: 'one agent per doc: read, locate implementation, apply evidence-backed edits — except release-owned pages, which are reviewed read-only and produce findings to file as issues' }, { title: 'Adversarial Verify', detail: 'second agent re-checks each applied fix against code, repairs regressions' }, ], } @@ -214,6 +214,37 @@ const ALL_HANDWRITTEN = [ ] // +// --- Release-owned pages are IN SCOPE but READ-ONLY (#4920) ------------------- +// +// AGENTS.md "Documentation Guardrails" — the row whose path column is exactly the +// prefix below — and CLAUDE.md's second ⛔ rule say the same thing: +// +// `content/docs/releases/` | RELEASE-OWNED | Never edit in a code PR. +// Release notes are written centrally at release time, compiled from changesets +// + the ADR-0087 registries — not accreted a row per PR. +// +// This workflow's stated deliverable is an in-place mdx rewrite (see RULES below), +// so a full audit walked straight into that prohibition: 9 release pages in scope, +// each handed to an agent told to Edit it, and the follow-up PR from a run was +// precisely the PR the guardrail exists to stop. +// +// The ruling on #4920 was NOT to drop them from scope. Dropping them would leave +// some of the most-read pages in the docs permanently unaudited, and would create a +// SECOND definition of "which docs does this workflow cover" alongside the generated +// block above — #4851 had just finished paying for what happens when one subject has +// two hand-kept lists. So the scope is unchanged and only the DELIVERABLE forks: +// findings to file as issues, instead of edits written to disk. +// +// The fork has to be decidable inside the workflow VM (no filesystem, no require, no +// import — see the note on the generated block), which a path prefix is. And the +// prefix is not a curation of the guardrail, it is the guardrail's own path column +// copied verbatim, so there is still exactly one definition of "release-owned". +// `scripts/docs-audit/check-audit-scope.mjs` anchors the two together and goes red if +// AGENTS.md stops marking this exact path RELEASE-OWNED, if this constant stops +// matching the row, or if the derived scope stops containing release pages at all. +const RELEASE_OWNED_PREFIX = 'content/docs/releases/' +const isReleaseOwned = (doc) => doc.startsWith(RELEASE_OWNED_PREFIX) + // Scope resolution. Omitting `args` entirely is the legitimate "audit // everything" invocation; supplying `args` but not a usable `args.docs` array // is a CALLER BUG and must say so. @@ -242,7 +273,15 @@ if (args !== undefined && args !== null) { } } const DOCS = args && Array.isArray(args.docs) && args.docs.length ? args.docs : ALL_HANDWRITTEN +const WRITABLE_DOCS = DOCS.filter((d) => !isReleaseOwned(d)) +const READONLY_DOCS = DOCS.filter(isReleaseOwned) log(`scope: ${DOCS.length} doc(s)${DOCS === ALL_HANDWRITTEN ? ' — FULL audit (no args.docs given)' : ''}`) +if (READONLY_DOCS.length) { + log( + ` of which ${READONLY_DOCS.length} release-owned page(s) under ${RELEASE_OWNED_PREFIX} — ` + + 'audited READ-ONLY: findings only, never edited (AGENTS.md Documentation Guardrails; #4920)', + ) +} // --- Scope preflight: every path in scope must resolve to a real file --------- // @@ -365,6 +404,19 @@ const RULES = `HARD RULES: 6. Make minimal, precise edits — fix what is wrong, leave correct prose alone. 7. Verify code samples, CLI commands, API method names, config keys, env vars, file paths, enum values, and links against the actual implementation.` +// The read-only counterpart of RULES, for release-owned pages. Rule 1 of RULES +// ("Edit the doc FILE IN PLACE … the edits to disk are the real deliverable") is +// exactly what must not happen here, so this is a separate text rather than RULES +// with a caveat bolted on — an agent handed both a "you must edit" and a "you must +// not edit" instruction resolves the contradiction however it likes. +const READONLY_RULES = `HARD RULES — THIS PAGE IS RELEASE-OWNED AND READ-ONLY: +1. DO NOT edit, create, move, rename or delete this file or ANY file under ${RELEASE_OWNED_PREFIX}. No Edit, no Write, no shell command that modifies the working tree. These pages are RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release notes are written centrally at release time, compiled from changesets + the ADR-0087 registries, and a code PR that edits them is the exact PR that guardrail exists to stop. Leaving the file untouched is not a partial result — it is the correct result. +2. Your deliverable is the FINDING LIST. Each finding gets filed as an issue by the caller of this workflow, by someone who has not read the page and will not redo your research: name the location, state what is wrong, and say what it should say instead. +3. EVERY finding must be backed by implementation you actually read — cite file:line under packages/. If you cannot find code that contradicts the page, it is NOT a finding. Put the suspicion under 'unresolved' instead; an unresolved item is a real, useful outcome here. +4. Do not fabricate APIs, flags, paths or features, and do not report wording, tone, formatting or structure preferences. Implementation accuracy only. +5. A release page is a HISTORICAL record: it describes what shipped in a given version. "The current API differs" is therefore not automatically an error. Classify each finding: 'never-true' (the page was wrong when written), 'no-longer-true' (accurate for its version, but the page states it as present tense / current behaviour and now misleads), or 'ambiguous' (cannot tell without the release's own history). The three want different fixes, and only you have the evidence to tell them apart. +6. Auto-generated reference docs (content/docs/references/) are out of scope and so is every page outside the target — do not wander.` + // `docExists` is required and reported by the agent that actually opens the file — // the preflight above is a separate call path, and #4868's lesson is that a self-check // running somewhere other than the real path proves nothing about the real path. It is @@ -388,6 +440,36 @@ const FIX_LOG_SCHEMA = { }, } +// The read-only channel's output. Deliberately NOT shaped like FIX_LOG_SCHEMA: there +// is no `fixesApplied`/`fixCount` to report zero of, because "0 fixes" is the value +// #4851 showed to be indistinguishable from "nothing was there". A release page's +// result is a list of findings whose length is the count — one source of truth, no +// self-reported tally to disagree with it. +// +// `filesEdited` is required and must come back false. The VM cannot see the working +// tree, so this is the agent's own admission — but an agent that admits it edited a +// release-owned page fails the run by name, which beats discovering the edit in review +// (or not discovering it). +const FINDING_SCHEMA = { + type: 'object', additionalProperties: false, + required: ['doc', 'docExists', 'filesEdited', 'implementationFound', 'findings', 'unresolved', 'notes'], + properties: { + doc: { type: 'string' }, + docExists: { type: 'boolean' }, + filesEdited: { type: 'boolean' }, + implementationFound: { type: 'boolean' }, + findings: { type: 'array', items: { type: 'object', additionalProperties: false, + required: ['kind', 'category', 'location', 'inaccuracy', 'suggestedFix', 'evidence'], + properties: { + kind: { type: 'string', enum: ['never-true', 'no-longer-true', 'ambiguous'] }, + category: { type: 'string', enum: ['broken-example', 'inaccurate-api', 'outdated-path', 'outdated-env', 'security-model', 'fabricated-feature', 'broken-link', 'naming-drift', 'enum-drift', 'other'] }, + location: { type: 'string' }, inaccuracy: { type: 'string' }, suggestedFix: { type: 'string' }, evidence: { type: 'string' }, + } } }, + unresolved: { type: 'array', items: { type: 'string' } }, + notes: { type: 'string' }, + }, +} + const VERDICT_SCHEMA = { type: 'object', additionalProperties: false, required: ['doc', 'fixesReviewed', 'confirmed', 'correctionsMade', 'regressionsFound', 'buildSafe', 'residualInaccuracies'], @@ -426,6 +508,39 @@ PROCEDURE: A doc with no real inaccuracies should return fixCount 0 — do not invent changes. The edits you write to disk ARE the deliverable.` } +function readOnlyReviewPrompt(doc) { + return `You are reviewing a single RELEASE-OWNED ObjectStack documentation page for IMPLEMENTATION ACCURACY. This page is audited READ-ONLY: you report, you do not fix. + +TARGET DOC (do not modify): ${doc} + +${PACKAGE_MAP} + +${HOUSE_FACTS} + +${READONLY_RULES} + +PROCEDURE: +1. Read the entire doc (${doc}). If that path does not exist, STOP: return docExists + false, an empty findings array, and say so in notes. Do NOT substitute a similar + path, and do not report "no inaccuracies" — a file you could not open was not + reviewed, and the two must never be reported the same way. +2. For each technical claim — code sample, CLI command, client/server API call, + method/type name, config key, enum value, env var, file path, route, link — LOCATE + the backing implementation under packages/ (Grep/Glob/Read; ripgrep via Bash is + fine) and confirm whether the page matches reality. +3. Record every contradiction you can evidence as a finding, with its kind + (never-true / no-longer-true / ambiguous), where on the page it is, what is wrong, + what it should say instead, and the file:line you read. Anything suspected but not + evidenced goes under 'unresolved'. +4. Do NOT edit the file. Return filesEdited false. If you edited it by reflex, revert + it and say so in notes — this run will fail on purpose rather than carry an edit to + a release-owned page into a PR. + +A page with no evidenced inaccuracies returns an empty findings array — do not invent +findings to look productive. The finding list IS the deliverable; each entry becomes an +issue.` +} + function verifyPrompt(doc, fixLog) { return `You are the ADVERSARIAL VERIFIER for an implementation-accuracy fix just applied to an ObjectStack doc. Assume the previous agent may have over-corrected or introduced errors. @@ -452,28 +567,84 @@ Return the verdict.` } phase('Audit & Fix') -log(`Auditing ${DOCS.length} hand-written doc(s) (pipelined: audit -> adversarial verify per doc)`) +log( + `Auditing ${DOCS.length} hand-written doc(s): ${WRITABLE_DOCS.length} editable ` + + `(audit -> adversarial verify per doc)` + + (READONLY_DOCS.length ? `, ${READONLY_DOCS.length} release-owned (read-only review, findings only)` : ''), +) +// One pipeline over the whole scope, two deliverables. Routing by `isReleaseOwned` +// inside the stages — rather than by running two pipelines, or by filtering the +// release pages out up front — is deliberate: there is no code path here on which a +// doc in scope produces no result at all, which is the shape a "skip" would take. const results = await pipeline( DOCS, - (doc) => agent(auditPrompt(doc), { label: `audit:${doc.replace('content/docs/', '')}`, phase: 'Audit & Fix', schema: FIX_LOG_SCHEMA }), - (fixLog, doc) => { - if (!fixLog) return null - return agent(verifyPrompt(doc, fixLog), { label: `verify:${doc.replace('content/docs/', '')}`, phase: 'Adversarial Verify', schema: VERDICT_SCHEMA }) - .then((v) => ({ doc, fixLog, verdict: v })) + (doc) => isReleaseOwned(doc) + ? agent(readOnlyReviewPrompt(doc), { label: `review:${doc.replace('content/docs/', '')}`, phase: 'Audit & Fix', schema: FINDING_SCHEMA }) + : agent(auditPrompt(doc), { label: `audit:${doc.replace('content/docs/', '')}`, phase: 'Audit & Fix', schema: FIX_LOG_SCHEMA }), + (auditLog, doc) => { + if (!auditLog) return null + // No adversarial verifier for release-owned pages: the verifier's job is to + // re-check APPLIED EDITS and repair over-corrections, and there are none. The + // guard against a bad finding is that it must carry file:line evidence and is + // read by a human before it becomes an issue. + if (isReleaseOwned(doc)) return { doc, readOnly: true, findingLog: auditLog } + return agent(verifyPrompt(doc, auditLog), { label: `verify:${doc.replace('content/docs/', '')}`, phase: 'Adversarial Verify', schema: VERDICT_SCHEMA }) + .then((v) => ({ doc, readOnly: false, fixLog: auditLog, verdict: v })) } ) const clean = results.filter(Boolean) -const totalFixes = clean.reduce((n, r) => n + (r.fixLog?.fixCount || 0), 0) -const totalRepairs = clean.reduce((n, r) => n + (r.verdict?.correctionsMade?.length || 0), 0) -const totalResidual = clean.reduce((n, r) => n + (r.verdict?.residualInaccuracies?.length || 0), 0) +const edited = clean.filter((r) => !r.readOnly) +const reviewed = clean.filter((r) => r.readOnly) +const totalFixes = edited.reduce((n, r) => n + (r.fixLog?.fixCount || 0), 0) +const totalRepairs = edited.reduce((n, r) => n + (r.verdict?.correctionsMade?.length || 0), 0) +const totalResidual = edited.reduce((n, r) => n + (r.verdict?.residualInaccuracies?.length || 0), 0) +const totalFindings = reviewed.reduce((n, r) => n + (r.findingLog?.findings?.length || 0), 0) + +// The read-only channel's headline, emitted BEFORE any of the failure paths below so +// it survives a failing run. #4920's rejected option was deleting the release pages +// from scope; a run that says nothing about them is that option, reached by accident. +// So this line is unconditional whenever release pages are in scope — including when +// the count is zero, which is a reviewed-and-clean result, not an absence. +if (READONLY_DOCS.length) { + log(`releases (read-only): ${totalFindings} finding(s) — file issues, do not edit`) +} + +// A release page that produced NO result was silently skipped, which is exactly the +// outcome the read-only channel exists to prevent. `results.filter(Boolean)` above is +// where such a doc would vanish without a trace, so reconcile against the scope by name. +const skippedReadOnly = READONLY_DOCS.filter((d) => !reviewed.some((r) => r.doc === d)) +if (skippedReadOnly.length) { + throw new Error( + `[docs-accuracy-audit] ${skippedReadOnly.length} of ${READONLY_DOCS.length} release-owned ` + + 'page(s) in scope produced no review result:\n ' + skippedReadOnly.join('\n ') + + '\n\nThese pages are in scope precisely so they are not skipped (#4920): the audit does ' + + 'not edit them, it reports findings on them. A run that neither edits nor reports has ' + + 'dropped them from the audit — the option that ruling rejected. Re-run the workflow.', + ) +} + +// A release-owned page the agent admits it edited. The edit is already on disk, so the +// run fails naming the files rather than letting them ride along into a PR — the exact +// PR AGENTS.md's Documentation Guardrails forbid. +const illegallyEdited = reviewed.filter((r) => r.findingLog?.filesEdited === true).map((r) => r.doc) +if (illegallyEdited.length) { + throw new Error( + `[docs-accuracy-audit] ${illegallyEdited.length} release-owned page(s) were EDITED by their ` + + 'read-only review agent:\n ' + illegallyEdited.join('\n ') + + '\n\n`' + RELEASE_OWNED_PREFIX + '` is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): ' + + 'release notes are compiled centrally at release time and must never be edited by a code PR. ' + + 'Revert these files (`git checkout -- `) before doing anything else with this run; the ' + + 'findings are still in the result, and belong in issues.', + ) +} // The preflight said every path resolved; the agents that actually opened the files // are the authority on whether that was true. If they disagree, the run did NOT audit // what it claims to have audited — say so by failing, after logging the work that did // land (the edits are already on disk) rather than returning a summary that reads green. -const ghosts = clean.filter((r) => r.fixLog?.docExists === false).map((r) => r.doc) +const ghosts = clean.filter((r) => (r.readOnly ? r.findingLog : r.fixLog)?.docExists === false).map((r) => r.doc) if (ghosts.length) { log(`audited ${clean.length - ghosts.length} doc(s), ${totalFixes} fix(es), ${totalRepairs} verifier repair(s) before failing`) throw new Error( @@ -488,20 +659,51 @@ if (ghosts.length) { return { docsProcessed: clean.length, docsDropped: DOCS.length - clean.length, - docsWithChanges: clean.filter((r) => (r.fixLog?.fixCount || 0) > 0 || (r.verdict?.correctionsMade?.length || 0) > 0).length, + docsWithChanges: edited.filter((r) => (r.fixLog?.fixCount || 0) > 0 || (r.verdict?.correctionsMade?.length || 0) > 0).length, totalFixesApplied: totalFixes, totalVerifierRepairs: totalRepairs, totalResidualForFollowup: totalResidual, - docsMissingVerifier: clean.filter((r) => !r.verdict).map((r) => r.doc), - perDoc: clean.map((r) => ({ - doc: r.doc, - fixes: r.fixLog?.fixCount || 0, - docExists: r.fixLog?.docExists, - implFound: r.fixLog?.implementationFound, - confirmed: r.verdict?.confirmed, - repairs: r.verdict?.correctionsMade?.length || 0, - regressions: r.verdict?.regressionsFound || [], - buildSafe: r.verdict?.buildSafe, - residual: r.verdict?.residualInaccuracies || [], - })), + docsMissingVerifier: edited.filter((r) => !r.verdict).map((r) => r.doc), + // The release-owned channel, reported separately and in full: its deliverable is not + // a diff, it is this list, and it is only worth anything if someone files it. + releaseOwnedReadOnly: { + prefix: RELEASE_OWNED_PREFIX, + docsReviewed: reviewed.length, + findings: totalFindings, + action: `releases (read-only): ${totalFindings} finding(s) — file issues, do not edit`, + why: 'AGENTS.md "Documentation Guardrails": content/docs/releases/ is RELEASE-OWNED — never edited by a code PR. In scope, read-only (#4920).', + perDoc: reviewed.map((r) => ({ + doc: r.doc, + docExists: r.findingLog?.docExists, + implFound: r.findingLog?.implementationFound, + findings: r.findingLog?.findings || [], + unresolved: r.findingLog?.unresolved || [], + notes: r.findingLog?.notes, + })), + }, + // Both channels appear here, in DIFFERENT shapes on purpose: a read-only entry has + // no `fixes` key to read as `0`, so it cannot be mistaken for a page that was audited + // and found clean. + perDoc: clean.map((r) => r.readOnly + ? { + doc: r.doc, + channel: 'read-only', + docExists: r.findingLog?.docExists, + implFound: r.findingLog?.implementationFound, + findings: (r.findingLog?.findings || []).length, + unresolved: r.findingLog?.unresolved || [], + note: 'release-owned — file issues, do not edit', + } + : { + doc: r.doc, + channel: 'edit', + fixes: r.fixLog?.fixCount || 0, + docExists: r.fixLog?.docExists, + implFound: r.fixLog?.implementationFound, + confirmed: r.verdict?.confirmed, + repairs: r.verdict?.correctionsMade?.length || 0, + regressions: r.verdict?.regressionsFound || [], + buildSafe: r.verdict?.buildSafe, + residual: r.verdict?.residualInaccuracies || [], + }), } diff --git a/scripts/docs-audit/README.md b/scripts/docs-audit/README.md index 01542bd19d..2d8ebb5f39 100644 --- a/scripts/docs-audit/README.md +++ b/scripts/docs-audit/README.md @@ -119,6 +119,48 @@ and each audit agent reports `docExists` from the read path itself, so a preflig was wrong cannot be laundered into a green summary. The gate covers the default list, the preflight covers the caller's list, and the read path checks both. +### Release-owned pages are in scope, and read-only (#4920) + +The derived scope contains `content/docs/releases/**` (9 pages), and AGENTS.md's +Documentation Guardrails forbid a code PR from editing those pages at all. Since the +audit's deliverable is an in-place mdx rewrite, a full audit used to walk straight into +that prohibition — and open exactly the PR the guardrail exists to stop. + +They are **not** excluded. Excluding them would leave some of the most-read pages in the +docs permanently unaudited, and would put a second definition of "docs this workflow +covers" next to the generated block — #4851 is the bill for one subject with two +hand-kept lists. Instead the **deliverable** forks, on a path prefix (`content/docs/ +releases/`, which is the guardrail's own path column, decidable inside the workflow VM): + +| | editable docs | release-owned pages | +|:--|:--|:--| +| prompt | audit + **fix in place** | review, **never edit** | +| output schema | `fixesApplied` / `fixCount` | `findings[]` + `filesEdited` | +| adversarial verifier | yes — re-checks applied edits | n/a, nothing was applied | +| deliverable | the diff | findings → **file as issues** | + +Each finding carries `kind` (`never-true` / `no-longer-true` / `ambiguous` — a release +page is a historical record, so "the current API differs" is not automatically an +error), where on the page it is, what it should say instead, and `file:line` evidence. +The run summary reports them under `releaseOwnedReadOnly` and logs +`releases (read-only): N finding(s) — file issues, do not edit`. + +Three failure modes are made loud rather than silent, because "audited nothing" and +"audited, found nothing" must never look alike: + +- a release page whose review returns **no result** fails the run by name — that is the + exclusion option arrived at by accident; +- a review agent reporting `filesEdited: true` fails the run naming the file to revert; +- the read-only headline is logged whenever release pages are in scope, **including at + zero findings** (reviewed-and-clean is a result, absence is not). + +`pnpm check:docs-audit-scope` enforces the whole contract: AGENTS.md must still mark +that exact path RELEASE-OWNED, the workflow's `RELEASE_OWNED_PREFIX` must still match +that row, the scope must still contain release pages, and the fork must still work — +checked by **running** the workflow against stub agents and inspecting which prompt and +schema each doc gets, not by grepping for a keyword. `--self-test` then mutates the fork +out of an in-memory copy and requires that check to go red. + ## 2. CI gate — `.github/workflows/docs-drift-check.yml` On any PR that touches `packages/**`, runs `affected-docs.mjs` against the base branch @@ -141,7 +183,9 @@ Workflow({ name: 'docs-accuracy-audit' }) ``` It edits files in place (frontmatter preserved, no moves) and returns a per-doc log of -fixes, verifier repairs, and residual items that couldn't be confirmed against code. +fixes, verifier repairs, and residual items that couldn't be confirmed against code — +**except** for `content/docs/releases/**`, which is reviewed read-only and returns +findings to file as issues (see [1b](#release-owned-pages-are-in-scope-and-read-only-4920)). Always follow a run with the docs build gate: ```bash diff --git a/scripts/docs-audit/check-audit-scope.mjs b/scripts/docs-audit/check-audit-scope.mjs index 4b2991bc6d..72cc30466b 100644 --- a/scripts/docs-audit/check-audit-scope.mjs +++ b/scripts/docs-audit/check-audit-scope.mjs @@ -2,10 +2,14 @@ // Keep the `docs-accuracy-audit` workflow's default scope list DERIVED from the // filesystem instead of hand-kept — and fail loudly the moment the two disagree. // +// It also holds the second half of that scope's contract (#4920): `content/docs/ +// releases/**` is IN the scope but is a READ-ONLY target, and this script is what +// keeps that true — see "release-owned pages" below. +// // Usage: // node scripts/docs-audit/check-audit-scope.mjs # verify; exit 1 naming every drifted entry // node scripts/docs-audit/check-audit-scope.mjs --write # regenerate the block in place -// node scripts/docs-audit/check-audit-scope.mjs --self-test # pin the parser/renderer/differ (no repo state needed) +// node scripts/docs-audit/check-audit-scope.mjs --self-test # pin the parser/renderer/differ + the read-only routing // // ## Why this exists (#4851) // @@ -50,11 +54,39 @@ // `--write` regenerates the inline block from that derivation, so the array is a // generated artifact that happens to live inside a hand-written file. Hand-editing // it is never necessary and this check will reject it. +// +// ## Release-owned pages: in scope, read-only (#4920) +// +// The derived scope contains `content/docs/releases/**`, and AGENTS.md's Documentation +// Guardrails forbid a code PR from editing those pages at all. The audit workflow's +// deliverable is an in-place mdx rewrite, so for those 9 pages the two rules collided +// head-on: a full audit produced exactly the PR the guardrail exists to stop. +// +// The ruling was to keep them in scope and fork the DELIVERABLE — the workflow reviews +// them read-only and emits findings to file as issues. Excluding them instead would +// have created a second definition of "docs this workflow covers" next to the generated +// block, and #4851 is the bill for one subject with two hand-kept lists. +// +// That leaves three things that can quietly break, so this script checks all three: +// +// 1. the guardrail itself moves or is reworded in AGENTS.md, and the workflow keeps +// protecting a path nothing declares any more; +// 2. the workflow's `RELEASE_OWNED_PREFIX` stops matching that guardrail's path; +// 3. the routing is refactored away, and release pages silently rejoin the editable +// channel — the failure with no symptom until a PR edits a release note. +// +// (3) is checked by RUNNING the workflow against stub agents and inspecting which +// prompt and schema each doc actually gets, not by grepping for a keyword: a check +// that reads source text would pass on any refactor that keeps the words and drops +// the behaviour. `--self-test` then mutates the routing out of an in-memory copy and +// requires that check to go red, because a guard nobody has ever seen fail is a guard +// nobody has tested (#4868). import { execFileSync } from 'node:child_process'; import { readFileSync, writeFileSync } from 'node:fs'; import { join, dirname } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { createContext, runInContext } from 'node:vm'; const HERE = dirname(fileURLToPath(import.meta.url)); const REPO_ROOT = execFileSync('git', ['rev-parse', '--show-toplevel'], { cwd: HERE }) @@ -62,15 +94,19 @@ const REPO_ROOT = execFileSync('git', ['rev-parse', '--show-toplevel'], { cwd: H .trim(); const WORKFLOW_REL = '.claude/workflows/docs-accuracy-audit.js'; +const AGENTS_REL = 'AGENTS.md'; const BEGIN = '// '; const END = '// '; -const args = process.argv.slice(2); +/** + * The release-owned boundary — the path column of AGENTS.md's RELEASE-OWNED guardrail + * row, verbatim. Not a curation of it: `assertGuardrailAnchored` fails if AGENTS.md + * stops declaring exactly this, so the rule and its enforcement cannot drift apart. + */ +export const RELEASE_OWNED_PREFIX = 'content/docs/releases/'; +export const isReleaseOwned = (doc) => doc.startsWith(RELEASE_OWNED_PREFIX); -if (args.includes('--self-test')) { - selfTest(); - process.exit(0); -} +const args = process.argv.slice(2); // --- block extraction / rendering ------------------------------------------- @@ -193,10 +229,213 @@ function deriveDocs() { return docs; } +// --- release-owned pages: the rule, the constant, the routing ---------------- + +/** + * AGENTS.md must still declare exactly this path RELEASE-OWNED. The workflow's + * read-only fork is an ENFORCEMENT of that row; if the row is renamed, moved or + * softened, the enforcement is protecting a rule that no longer says what it is + * quoting, and that must be noticed here rather than by a reader years later. + */ +export function findGuardrailRow(agentsMd) { + return ( + agentsMd + .split('\n') + .find((line) => line.includes(`\`${RELEASE_OWNED_PREFIX}\``) && line.includes('RELEASE-OWNED')) ?? null + ); +} + +/** The prefix the workflow actually routes on. */ +export function parseReleaseOwnedPrefix(source) { + const m = source.match(/const RELEASE_OWNED_PREFIX = '([^']*)'/); + if (!m) { + throw new Error( + `${WORKFLOW_REL}: no \`const RELEASE_OWNED_PREFIX = '...'\` declaration. That constant is ` + + `how the workflow tells release-owned pages (read-only, findings only) from editable ones; ` + + `without it every page in scope is editable, including ${RELEASE_OWNED_PREFIX}** — the ` + + `collision #4920 was filed for. Restore it.`, + ); + } + return m[1]; +} + +/** + * Run the workflow the way it really runs — free globals, stub agents — and report + * what each doc in scope was actually handed. + * + * The workflow body uses top-level `await` and a top-level `return`, so its runner + * evaluates it as a function body with `log`/`phase`/`agent`/`pipeline`/`args` supplied + * as globals; `export const meta` is lifted out separately. This mirrors that shape + * closely enough to exercise the real routing expressions, which is the point — the + * alternative, matching source text, cannot tell a working fork from a dead one. + */ +async function runWorkflow(source, { workflowArgs, respond }) { + const logs = []; + const calls = []; + const context = createContext({ + console: { log() {}, error() {} }, + args: workflowArgs, + budget: { remaining: () => Number.POSITIVE_INFINITY }, + workflow: {}, + log: (m) => logs.push(String(m)), + phase: () => {}, + parallel: async (items, fn) => Promise.all(items.map(fn)), + // Two-stage pipeline, sequential: order does not matter to any assertion here and + // sequencing keeps a failing case readable. + pipeline: async (items, stage1, stage2) => { + const out = []; + for (const item of items) out.push(await stage2(await stage1(item), item)); + return out; + }, + agent: async (prompt, opts = {}) => { + calls.push({ prompt, ...opts }); + return respond({ prompt, ...opts }); + }, + }); + const body = source.replace(/^export const meta =/m, 'const meta ='); + try { + const result = await runInContext(`(async () => {\n${body}\n})()`, context, { + filename: WORKFLOW_REL, + }); + return { result, logs, calls, error: null }; + } catch (e) { + return { result: null, logs, calls, error: e }; + } +} + +/** A minimal object satisfying a workflow schema's `required` list. */ +function stubFor(schema, overrides = {}) { + const bools = { docExists: true, implementationFound: true, buildSafe: true, filesEdited: false }; + const out = {}; + for (const key of schema?.required ?? []) { + const type = schema.properties?.[key]?.type; + out[key] = + type === 'string' ? '' : type === 'number' ? 0 : type === 'array' ? [] : type === 'boolean' ? bools[key] ?? true : {}; + } + return { ...out, ...overrides }; +} + +/** Echo the preflight's own path list back as fully present. */ +function preflightResponse(prompt) { + const head = prompt.lastIndexOf('PATHS ('); + const paths = prompt + .slice(prompt.indexOf('\n', head) + 1) + .split('\n') + .map((s) => s.trim()) + .filter(Boolean); + return { command: 'stub', present: paths, missing: [] }; +} + +const PROBE_EDITABLE = 'content/docs/api/index.mdx'; +const PROBE_RELEASE = `${RELEASE_OWNED_PREFIX}v9.mdx`; + +/** + * Every way the read-only channel can be broken, checked by observing a real run. + * Returns a list of human-readable problems; empty means the fork is intact. + */ +export async function checkReadOnlyRouting(source) { + const problems = []; + const docs = [PROBE_EDITABLE, PROBE_RELEASE]; + const audits = (calls) => calls.filter((c) => c.phase === 'Audit & Fix'); + + // 1. Routing: which prompt and which schema does each doc get? + const run = await runWorkflow(source, { + workflowArgs: { docs }, + respond: ({ prompt, phase, schema }) => + phase === 'Scope Preflight' ? preflightResponse(prompt) : stubFor(schema, { doc: '' }), + }); + if (run.error) { + problems.push(`the workflow threw on a clean two-doc run: ${run.error.message}`); + return problems; + } + + const seen = audits(run.calls); + if (seen.length !== docs.length) { + problems.push(`expected ${docs.length} audit-phase agent(s), saw ${seen.length}`); + return problems; + } + const releaseCall = seen.find((c) => String(c.label).includes('releases/')); + const editableCall = seen.find((c) => !String(c.label).includes('releases/')); + + if (!releaseCall) { + problems.push(`${PROBE_RELEASE} was handed to no audit-phase agent at all — a page in scope that produces nothing has been dropped from the audit, which is the outcome #4920 rejected`); + } else { + // The editable channel's rule 1 is "Edit the doc FILE IN PLACE"; its presence in a + // release page's prompt IS the bug, whatever else the prompt says. + if (releaseCall.prompt.includes('Edit the doc FILE IN PLACE')) { + problems.push(`${PROBE_RELEASE} was given the EDITABLE audit prompt ("Edit the doc FILE IN PLACE") — release notes are RELEASE-OWNED and must never be edited by a code PR (AGENTS.md; #4920)`); + } + if (!releaseCall.prompt.includes('READ-ONLY') || !releaseCall.prompt.includes(`DO NOT edit`)) { + problems.push(`${PROBE_RELEASE}'s prompt does not tell the agent the page is read-only`); + } + // The schema is the structural half: a read-only channel that still reports + // `fixesApplied` is one Edit call away from writing to a release page. + const req = releaseCall.schema?.required ?? []; + if (req.includes('fixesApplied') || !req.includes('filesEdited')) { + problems.push(`${PROBE_RELEASE} was given the edit-log schema (fixesApplied), not the finding schema (filesEdited)`); + } + } + + if (!editableCall) { + problems.push(`${PROBE_EDITABLE} was handed to no audit-phase agent`); + } else if (!editableCall.prompt.includes('Edit the doc FILE IN PLACE')) { + problems.push(`${PROBE_EDITABLE} lost the editable audit prompt — the read-only fork must not swallow ordinary docs`); + } + + // 2. The run summary must SAY so. A silent read-only channel is indistinguishable + // from having excluded the pages, which is the option that was rejected. + const headline = 'releases (read-only): 0 finding(s) — file issues, do not edit'; + if (!run.logs.some((l) => l.includes(headline))) { + problems.push(`no run-summary line "${headline}" — findings on release pages have to be visible enough to file, or the audit of those pages produced nothing anyone can act on`); + } + const readOnly = run.result?.releaseOwnedReadOnly; + if (!readOnly || readOnly.docsReviewed !== 1) { + problems.push(`the result's releaseOwnedReadOnly section did not report the 1 release page reviewed (got ${JSON.stringify(readOnly?.docsReviewed)})`); + } + const entry = (run.result?.perDoc ?? []).find((d) => d.doc === PROBE_RELEASE); + if (!entry || entry.channel !== 'read-only') { + problems.push(`${PROBE_RELEASE} is not marked channel:"read-only" in perDoc (got ${JSON.stringify(entry?.channel)})`); + } else if ('fixes' in entry) { + problems.push(`${PROBE_RELEASE}'s perDoc entry carries a \`fixes\` count — a read-only page reporting "0 fixes" reads exactly like an audited-and-clean one (#4851)`); + } + + // 3. No result at all for a release page must FAIL the run, not shrink the summary. + const skipped = await runWorkflow(source, { + workflowArgs: { docs }, + respond: ({ prompt, phase, schema, label }) => + phase === 'Scope Preflight' + ? preflightResponse(prompt) + : String(label).includes('releases/') + ? null + : stubFor(schema, { doc: '' }), + }); + if (!skipped.error || !/produced no review result/.test(skipped.error.message)) { + problems.push('a release page whose review returned nothing did not fail the run — it was silently dropped from the summary instead'); + } + + // 4. An agent that admits it edited a release page must fail the run by name. + const edited = await runWorkflow(source, { + workflowArgs: { docs }, + respond: ({ prompt, phase, schema, label }) => + phase === 'Scope Preflight' + ? preflightResponse(prompt) + : stubFor(schema, { doc: '', filesEdited: String(label).includes('releases/') }), + }); + if (!edited.error || !edited.error.message.includes(PROBE_RELEASE)) { + problems.push('a read-only agent reporting filesEdited:true did not fail the run naming the page — an edit to a release note would ride into the PR unannounced'); + } + + return problems; +} + // --- main -------------------------------------------------------------------- try { - main(); + if (args.includes('--self-test')) { + await selfTest(); + process.exit(0); + } + await main(); } catch (e) { // A structural failure (markers gone, list unparseable, derivation empty) is a // RED result with a readable reason — never a stack trace, and never a pass. @@ -204,7 +443,7 @@ try { process.exit(1); } -function main() { +async function main() { const workflowPath = join(REPO_ROOT, WORKFLOW_REL); const source = readFileSync(workflowPath, 'utf8'); const derived = deriveDocs(); @@ -230,6 +469,7 @@ function main() { console.log( `✓ docs-accuracy-audit scope is in sync with content/docs/: ${listed.length} hand-written doc(s).`, ); + await checkReleaseOwned(source, derived); return; } @@ -264,6 +504,66 @@ function main() { process.exit(1); } +/** + * The release-owned half of the contract: the rule still says it, the workflow still + * encodes the same path, the pages are still in scope, and the read-only fork still + * works on a real run. + */ +async function checkReleaseOwned(source, derived) { + const guardrail = findGuardrailRow(readFileSync(join(REPO_ROOT, AGENTS_REL), 'utf8')); + if (!guardrail) { + console.error( + `✗ ${AGENTS_REL}: no Documentation Guardrails row marking \`${RELEASE_OWNED_PREFIX}\` RELEASE-OWNED.\n\n` + + ` ${WORKFLOW_REL} routes that exact prefix down a read-only channel BECAUSE of that row\n` + + ` (#4920). If the guardrail moved, was renamed or was softened, the workflow is now\n` + + ` enforcing a rule the repo no longer states — update both together, in that order.\n`, + ); + process.exit(1); + } + + const prefix = parseReleaseOwnedPrefix(source); + if (prefix !== RELEASE_OWNED_PREFIX) { + console.error( + `✗ ${WORKFLOW_REL}: RELEASE_OWNED_PREFIX is "${prefix}", but ${AGENTS_REL} marks\n` + + ` "${RELEASE_OWNED_PREFIX}" RELEASE-OWNED. The workflow would review the wrong set of pages\n` + + ` read-only — and edit the release notes it no longer recognises.\n`, + ); + process.exit(1); + } + + // The pages must still BE in scope. Zero of them is not "nothing to protect": it is + // option A from #4920 (exclude releases from the audit), which was rejected — the + // most-read pages in the docs would go permanently unaudited, silently. + const inScope = derived.filter(isReleaseOwned); + if (!inScope.length) { + console.error( + `✗ no ${RELEASE_OWNED_PREFIX}** page is in the audit scope.\n\n` + + ` Release pages are meant to be IN scope and READ-ONLY (#4920): audited, never edited,\n` + + ` findings filed as issues. An empty set means either the pages moved, or they were\n` + + ` excluded from the scope — the option that ruling rejected, because it leaves the\n` + + ` most-read pages in the docs unaudited with nothing to say so.\n`, + ); + process.exit(1); + } + + const problems = await checkReadOnlyRouting(source); + if (problems.length) { + console.error(`✗ ${WORKFLOW_REL}: the read-only channel for ${RELEASE_OWNED_PREFIX}** is broken.\n`); + for (const p of problems) console.error(` - ${p}`); + console.error( + `\n Observed by running the workflow against stub agents. Release notes are RELEASE-OWNED\n` + + ` (${AGENTS_REL}: "${guardrail.trim().slice(0, 96)}…"); the audit reviews them and reports,\n` + + ` it never edits them (#4920).\n`, + ); + process.exit(1); + } + + console.log( + `✓ release-owned pages are in scope and read-only: ${inScope.length} page(s) under ` + + `${RELEASE_OWNED_PREFIX} review-only (findings → issues, never edited).`, + ); +} + // --- self-test --------------------------------------------------------------- /** @@ -272,7 +572,7 @@ function main() { * no repo state — so a regression here fails on its own PR rather than being * discovered the next time a directory is renamed. */ -function selfTest() { +async function selfTest() { let failed = 0; let total = 0; const check = (label, want, got) => { @@ -341,6 +641,77 @@ function selfTest() { check('a renamed directory shows up on BOTH sides', 1, renamed.dead.length); check('…and its new home is flagged as unlisted', 1, renamed.unlisted.length); + // --- release-owned pages: in scope, read-only (#4920) ---------------------- + // + // The predicate and the guardrail parser are hermetic. The three cases after them + // deliberately are NOT: they run the REAL workflow, because the thing being pinned + // is that release pages take the read-only fork on the real path, and a fixture + // proves nothing about that (#4868 — a self-check running somewhere other than the + // real path proves nothing about the real path). + check('the prefix routes release pages', [true, true], [ + isReleaseOwned('content/docs/releases/v9.mdx'), + isReleaseOwned('content/docs/releases/index.mdx'), + ]); + check('…and nothing else', [false, false, false], [ + isReleaseOwned('content/docs/api/index.mdx'), + // Neither a sibling directory whose name merely starts the same way… + isReleaseOwned('content/docs/releases-notes/v9.mdx'), + // …nor a page that only mentions releases deeper in its path. + isReleaseOwned('content/docs/deployment/releases/v9.mdx'), + ]); + check( + 'the AGENTS.md guardrail row is found by path + RELEASE-OWNED', + true, + findGuardrailRow('| `content/docs/releases/` | **RELEASE-OWNED** | ❌ Never edit in a code PR. |') !== null, + ); + check( + 'a row that no longer says RELEASE-OWNED is not the guardrail', + null, + findGuardrailRow('| `content/docs/releases/` | generated | see the release process |'), + ); + throws( + 'a workflow without the prefix constant throws', + () => parseReleaseOwnedPrefix('const ALL_HANDWRITTEN = []'), + 'no `const RELEASE_OWNED_PREFIX', + ); + + const workflowSource = readFileSync(join(REPO_ROOT, WORKFLOW_REL), 'utf8'); + + // (1) Still in scope. #4920's rejected option was deleting these pages from the + // audit; that would show up right here, as an empty list. + check( + 'release pages are still IN the audit scope', + true, + parseBlock(workflowSource).filter(isReleaseOwned).length > 0, + ); + + // (2) …and routed read-only, observed on a real run of the workflow. + check('release pages take the read-only channel', [], await checkReadOnlyRouting(workflowSource)); + + // (3) Mutations. A guard that has never been seen to fail is a guard nobody has + // tested — so break the fork two ways in memory and require each to go red. + const mutants = [ + // The fork itself: every doc becomes editable, release notes included. + ['routing removed', 'doc.startsWith(RELEASE_OWNED_PREFIX)', 'false'], + // The fork survives but says nothing, which reads exactly like the pages having + // been excluded — the outcome the ruling rejected. + ['read-only headline removed', 'releases (read-only): ${totalFindings} finding(s)', 'audited ${totalFindings} page(s)'], + ]; + for (const [label, from, to] of mutants) { + total++; + const mutated = workflowSource.replace(from, to); + if (mutated === workflowSource) { + console.error(` ✗ mutation "${label}" did not apply — it cannot prove anything. Update the mutation to match the current source.`); + failed++; + continue; + } + const problems = await checkReadOnlyRouting(mutated); + if (!problems.length) { + console.error(` ✗ mutation "${label}": checkReadOnlyRouting stayed GREEN with the read-only channel broken`); + failed++; + } + } + if (failed) { console.error(`\n✗ check-audit-scope self-test failed (${failed} case(s)).`); process.exit(1);