Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 107 additions & 4 deletions .github/workflows/docs-drift-check.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,13 @@ name: Docs Drift Check
# because they document it through the SDK, which does not depend on the implementing
# package. The comment now also states what the run could NOT see — the derivation being
# read past its precision, with its silence taken for absence, is what #9192 records.
#
# And since #9519 it states WHICH TREE it read. On `pull_request`, actions/checkout gives
# the MERGE of base and head, so the row set is a fact about a commit that exists on no
# branch a reader can name — while the comment's own re-derivation command sent them to
# run the tool on their own worktree. A reader who did that got a different list and
# reported a correct row as a false positive, and the investigation of the non-existent
# defect cost a full round. Same posture, one more thing the run knows and used not to say.

on:
pull_request:
Expand DownExpand Up@@ -98,7 +105,89 @@ jobs:
const crossCutting = data.crossCuttingSymbols || [];
const weak = data.weakAnchorsDropped || [];
const coarse = data.packageMentionDocs || [];
const rederive = `node scripts/docs-audit/affected-docs.mjs --json origin/${baseRef}`;
// ── WHICH TREE THIS WAS COMPUTED ON (#9519) ─────────────────────────────
// The row set is a fact about two commits and this comment named neither.
// Pages are read with `readFileSync` from the WORKING TREE, and on a
// `pull_request` run `actions/checkout` gives us merge(base, head) — a tree
// that exists on no branch a reader can name and that GitHub drops once the
// PR closes. The change set is a diff whose base was published only as
// `origin/<base>`, a moving NAME. The re-derivation command below then sent
// the reader to run the tool against THEIR tree, so a page that gained or
// lost an anchor token on the base branch after their worktree was cut
// produced a row they could not reproduce, with nothing here to say why.
//
// Measured: a reader did exactly that, grepped, found nothing, and reported
// a correct row as a false positive; the follow-up then investigated a
// defect class that does not exist in this tool — the anchor set is derived
// fresh per run, with no cache, index or snapshot anywhere — at the cost of
// a full round. Naming the tree is this file's own #9192 posture (say what
// the run could not see) applied to one more thing the run already knows.
//
// The mapper change behind this is strictly ADDITIVE: `computedOn` is read
// off git at the emit boundary, after every derivation has finished, so no
// row above depends on it. Verified byte-for-byte against the pre-change
// mapper on three refs — the `computedOn` block is the entire diff.
const computedOn = data.computedOn || {};
const parents = Array.isArray(computedOn.headParents) ? computedOn.headParents : [];
// Parent ORDER on `refs/pull/N/merge` (base first, head second) is GitHub's
// convention, not git's, so it is CHECKED against the event payload rather
// than assumed — the pair is labelled only when the payload says which is
// which, and stays unlabelled otherwise. A confidently wrong label would
// send a reader off to rebuild the mirror image of the tree.
const prHeadSha = context.payload.pull_request.head.sha;
const mergedHead = parents.find(p => p === prHeadSha) || null;
const mergedBase = mergedHead ? (parents.find(p => p !== mergedHead) || null) : null;
// The COMMIT the mapper measured its diff from — the three-dot merge-base it
// had already resolved, not `origin/<base>` re-read later. Naming the commit
// is what makes the command replayable from any clone; naming the branch is
// what made it a trap.
const diffBase = computedOn.diffBase || null;
const rederive = `node scripts/docs-audit/affected-docs.mjs --json ${diffBase || `origin/${baseRef}`}`;
const treeBlock = (() => {
// ⛔ Never degrade to silence here. An unnamed tree is the exact state this
// block exists to end, so a missing identity is SAID, not omitted.
if (!computedOn.head) {
return ['', '> ⚠️ This run could not identify the commit it read, so the list above cannot be'
+ ' tied to a tree. Re-derive against your own checkout and compare by hand.'];
}
const lines = [
'',
'<details><summary>Which tree this was computed on</summary>',
'',
mergedHead && mergedBase
? `This run read \`content/docs\` from \`${computedOn.head}\` — the merge of head`
+ ` \`${mergedHead}\` into base \`${mergedBase}\`, which is what \`actions/checkout\``
+ ` gives a \`pull_request\` run. **Not** the PR head.`
: `This run read \`content/docs\` from \`${computedOn.head}\`.`,
'',
`A worktree cut from an older \`${baseRef}\` holds a different \`content/docs\`, so re-deriving`
+ ` there can legitimately return a different list — that is a **different tree, not a wrong`
+ ` row**. To answer on the same tree:`,
'',
'```sh',
];
if (mergedHead && mergedBase) {
lines.push(
'# while this PR is open — GitHub drops the merge commit once it closes',
`git fetch origin ${computedOn.head} && git checkout ${computedOn.head}`,
'# afterwards, rebuild it from the two parents, which stay fetchable',
`git fetch origin ${mergedBase} ${mergedHead} && git checkout -B drift-repro ${mergedBase} && git merge --no-ff ${mergedHead}`,
'',
);
} else {
lines.push(`git fetch origin ${computedOn.head} && git checkout ${computedOn.head}`);
}
lines.push(rederive, '```');
// A sha that misidentifies the tree is worse than no sha, so the one
// condition under which it does is stated right where the sha is.
if (computedOn.dirty === true) {
lines.push('', '⚠️ That checkout carried **uncommitted changes**, so the commit above does not fully identify what was read.');
} else if (computedOn.dirty === null) {
lines.push('', '⚠️ This run could not check whether its checkout was clean, so the commit above may not fully identify what was read.');
}
lines.push('</details>');
return lines;
})();
const limits = [];
if (anchorless.length) limits.push(`**${anchorless.length}** changed file(s) yielded no anchor (\`${anchorless.slice(0, 3).join('`, `')}\`${anchorless.length > 3 ? ', …' : ''}) — pages documenting those are invisible to this run`);
if (crossCutting.length) limits.push(`**${crossCutting.length}** cross-cutting symbol(s) contributed no route anchor: \`${crossCutting.join('`, `')}\``);
Expand DownExpand Up@@ -137,7 +226,13 @@ jobs:
const headline = anchorList.length === 0
? `Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from **${pkgs.length}** changed package(s)), so **this run has no opinion** about the docs.`
: `**${anchorList.length}** anchor(s) derived from **${pkgs.length}** changed package(s); no hand-written page names any of them. ✅`;
body = [marker, '### 📓 Docs Drift Check', headline, ...limitsBlock].join('\n');
// The tree identity is rendered here only when anchors WERE derived: that
// is the run with an opinion ("no page names any of them"), and a reader
// whose own tree does yield a row is owed the reason. With no anchors the
// run has no opinion to disagree with, and the bytes would be pure noise on
// every docs-tooling-only PR (#9037 — every byte here is relayed).
body = [marker, '### 📓 Docs Drift Check', headline, ...limitsBlock,
...(anchorList.length ? treeBlock : [])].join('\n');
} else {
const detail = (data.detail || []).reduce((m, d) => (m[d.doc] = d.via, m), {});
const row = d => `- \`${d}\`${detail[d] ? ` _(via ${detail[d].join(', ')})_` : ''}`;
Expand All@@ -154,7 +249,7 @@ jobs:
// fidelity is one command away, never lost.
body.push(
'',
`**${editable.length}** hand-written doc(s) name something this change touched — list omitted above ${EDITABLE_ROW_CAP} rows. Re-derive: \`node scripts/docs-audit/affected-docs.mjs --json origin/${baseRef}\`.`,
`**${editable.length}** hand-written doc(s) name something this change touched — list omitted above ${EDITABLE_ROW_CAP} rows. Re-derive on the tree named below: \`${rederive}\`.`,
);
if (readOnly.length) {
body.push(
Expand DownExpand Up@@ -186,13 +281,21 @@ jobs:
}
}
body.push(...limitsBlock);
body.push(...treeBlock);
body.push(
'',
'> Advisory only, and a **precision-first** one (#9192): a page is listed because it names a',
'> symbol, wire route or SDK method this diff touched — not because it mentions a changed',
'> package. Each row says which anchor put it there, so a wrong row is reportable rather than',
'> merely annoying. To re-verify, run the `docs-accuracy-audit` workflow scoped to these files:',
'> `node scripts/docs-audit/affected-docs.mjs origin/' + baseRef + '` → pass the list as `args.docs`.',
// Pinned to the commit the mapper measured from, and pointed at the tree
// named above (#9519): run this against a different tree and a different
// list is the CORRECT answer — which is exactly how a right row once got
// reported as a wrong one.
'> `node scripts/docs-audit/affected-docs.mjs ' + (diffBase || `origin/${baseRef}`) + '` → pass the list as',
computedOn.head
? '> `args.docs`, on the commit named under **Which tree this was computed on**.'
: '> `args.docs`.',
);
body = body.join('\n');
}
Expand Down
121 changes: 121 additions & 0 deletions scripts/docs-audit/affected-docs.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -153,6 +153,11 @@ const args = process.argv.slice(2);
const asJson = args.includes('--json');
const all = args.includes('--all');
const sinceRef = args.find((a) => !a.startsWith('--')) || 'origin/main';
// The commit the change set is actually measured FROM, published in `computedOn`
// below (#9519). Declared up here so both `emit` call sites can read it — the `--all`
// arm returns long before §2 assigns it, and a `let` in §2 would put that arm in the
// temporal dead zone. `null` is the honest answer for `--all`: it diffs nothing.
let diffBaseRef = null;

// --- 0. classifier constants -------------------------------------------------
// Declared up here, ahead of the `--self-test` short-circuit below, because `const` is
Expand DownExpand Up@@ -515,6 +520,9 @@ let baseRef = sinceRef;
if (threeDot) {
try { baseRef = sh(`git merge-base ${sinceRef} HEAD`).trim() || sinceRef; } catch { /* keep sinceRef */ }
}
// Publish it (#9519). READ-ONLY of a value §2 has already settled — this line adds no
// input to any derivation, it only lets the answer say what it was measured from.
diffBaseRef = baseRef;

/**
* A test file — it observes behaviour rather than defining it, so changing one cannot
Expand DownExpand Up@@ -1727,6 +1735,46 @@ function selfTest() {
];
for (const [lit, want, label] of literalCases) check('literalAnchorsFromLines', label, lit, want, lits.has(lit));

// ── `computedOn` (#9519): the record that names WHICH TREE the answer is about ──
// Pinned on the pure shaper, so these stay hermetic; the probing wrapper reads real
// git state by construction. Two properties carry the field's whole value: a merge
// commit's parents must survive as a PAIR — that pair is the only durable handle on
// an ephemeral `refs/pull/N/merge` tree — and "could not tell" must never be
// flattened into "checked, clean".
const mergeParents = '097fe96e1228f7da71f87e8f5ed95ae2739b53f1 047457ca3a8757012043460b8ded6090cbc9b114';
const computedOnCases = [
// [label, want, got]
['a merge commit keeps BOTH parents, in order',
JSON.stringify(mergeParents.split(' ')), JSON.stringify(computedOnFrom('m', mergeParents, 'b', '').headParents)],
['an ordinary commit has exactly one, trailing newline stripped',
JSON.stringify(['p1']), JSON.stringify(computedOnFrom('m', 'p1\n', 'b', '').headParents)],
['a root commit has none — never a [""] entry',
JSON.stringify([]), JSON.stringify(computedOnFrom('m', '', 'b', '').headParents)],
['a failed parent probe degrades to [] rather than throwing',
JSON.stringify([]), JSON.stringify(computedOnFrom('m', null, 'b', '').headParents)],
['the head sha is trimmed', 'abc', computedOnFrom('abc\n', 'p', 'b', '').head],
['a failed head probe is null, never the empty string', null, computedOnFrom(null, 'p', 'b', '').head],
['`--all` diffs nothing, so it names no base', null, computedOnFrom('m', 'p', null, '').diffBase],
['a clean checkout is dirty=false', false, computedOnFrom('m', 'p', 'b', '').dirty],
['a modified page is dirty=true', true, computedOnFrom('m', 'p', 'b', ' M content/docs/x.mdx\n').dirty],
['an UNTRACKED page counts too — walk() reads the filesystem, not the index',
true, computedOnFrom('m', 'p', 'b', '?? content/docs/new.mdx\n').dirty],
['a failed status probe is null — "could not tell" is not "checked, clean"',
null, computedOnFrom('m', 'p', 'b', null).dirty],
['the record carries exactly the four declared members',
'head,headParents,diffBase,dirty', Object.keys(computedOnFrom('m', 'p', 'b', '')).join(',')],
];
for (const [label, want, got] of computedOnCases) check('computedOnFrom', label, 'computedOn', want, got);

// PRESENCE, not merely shape. The field is worth nothing unless it reaches the JSON
// the workflow renders, and a rename or a dropped line there returns the comment to
// the unnamed-tree state this field exists to end — with every pin above still green.
// Read from source because the emitter writes to stdout under module-level flags and
// cannot be called hermetically.
const ownSource = readFileSync(new URL(import.meta.url), 'utf8');
check('emit', 'the emitted JSON actually carries `computedOn`', 'affected-docs.mjs',
true, /\bcomputedOn:\s*computedOnIdentity\(/.test(ownSource));

if (failed) {
console.error(`\n✗ affected-docs self-test failed (${failed} case(s)).`);
process.exit(1);
Expand DownExpand Up@@ -2072,6 +2120,75 @@ emit(
},
);

/**
* Shape the `computedOn` record from raw git answers. PURE — every probe lives in
* `computedOnIdentity` below — so `--self-test` can pin the shape with no repo state.
*
* @param {string|null} head `git rev-parse HEAD`
* @param {string|null} parentLine `git log -1 --format=%P HEAD` — space-separated
* @param {string|null} diffBase the resolved commit the diff was measured from
* @param {string|null} porcelain `git status --porcelain`; null when the probe failed
*/
function computedOnFrom(head, parentLine, diffBase, porcelain) {
const one = (v) => (typeof v === 'string' && v.trim() ? v.trim() : null);
return {
head: one(head),
// A root commit has no parents and a failed probe answered nothing: both are the
// empty list, never a `['']` entry that reads downstream as a real commit.
headParents: typeof parentLine === 'string' ? parentLine.trim().split(/\s+/).filter(Boolean) : [],
diffBase: one(diffBase),
// "Could not tell" and "checked, clean" are DIFFERENT answers and must not render
// alike — the same distinction this tool's output draws everywhere else.
dirty: typeof porcelain === 'string' ? porcelain.trim().length > 0 : null,
};
}

/**
* Name the tree this run's answer is a fact ABOUT (#9519).
*
* The row set is a function of two commits and, until this field, the JSON named
* neither by anything stable. The pages are read with `readFileSync` from the WORKING
* TREE (`docTexts`, §3b) — not from any ref — and the change set is a diff whose base
* was published only as `sinceRef`, a moving NAME (`origin/main`), never a commit.
*
* On a `pull_request` run `actions/checkout` checks out merge(base, head), so the
* advisory is a fact about a tree that exists on no branch the reader can name and that
* GitHub drops once the PR closes. `headParents` is the durable handle on it: both
* parents stay fetchable, and re-merging them rebuilds the same tree.
*
* `diffBase` is the merge-base §2 already resolved, not `sinceRef` re-read here, and
* that distinction is what makes it reproducible: the diff is three-dot, so re-running
* with `origin/main` a day later measures from the same merge-base while re-running
* with THIS sha measures from it by construction — even from a clone whose `origin/main`
* has moved. Naming the commit is what makes the command replayable; naming the branch
* is what made it a trap.
*
* Measured cost of leaving all of it unsaid: a reader re-derived in a worktree cut from
* an older `main`, one page had gained an anchor token on `main` in between, and a
* correct row was reported as a false positive. The follow-up then investigated a defect
* class that does not exist in this tool — the anchor set is derived fresh per run, with
* no cache, index or snapshot anywhere — and cost a full round.
*
* `dirty` is this field's own correctness guard, not decoration: the tool reads the
* working tree, so with uncommitted changes present the shas do NOT identify what was
* read. A sha that misidentifies the tree is worse than no sha — the same defect, now
* wearing a credential.
*
* ⛔ Read-only, and deliberately evaluated HERE, at the emit boundary after every
* derivation has finished, so it cannot participate in deriving anything. Every probe
* degrades to `null` rather than throwing: this is a courtesy label on an advisory and
* must never be the reason a scan fails.
*/
function computedOnIdentity() {
const probe = (cmd) => { try { return sh(cmd); } catch { return null; } };
return computedOnFrom(
probe('git rev-parse HEAD'),
probe('git log -1 --format=%P HEAD'),
diffBaseRef === null ? null : probe(`git rev-parse --verify --quiet ${JSON.stringify(`${diffBaseRef}^{commit}`)}`),
probe('git status --porcelain'),
);
}

function emit(docList, changedPackages, summary, detail, skipped = {}, anchorInfo = {}) {
const { testFilesSkipped = 0, scriptFilesSkipped = 0, devOnlyManifestsSkipped = 0 } = skipped;
const {
Expand All@@ -2085,6 +2202,10 @@ function emit(docList, changedPackages, summary, detail, skipped = {}, anchorInf
{
summary,
sinceRef: all ? null : sinceRef,
// WHICH TREE THIS ANSWER IS A FACT ABOUT (#9519). `sinceRef` above is a NAME
// and names move; the pages were read from the WORKING TREE, which no field
// named at all. See `computedOnIdentity` for what each member is for.
computedOn: computedOnIdentity(),
changedPackages,
// The FULL set, release-owned pages included — this is what feeds the audit
// workflow's `args.docs`, and #4920 requires those pages to stay audited.
Expand Down
Loading