Skip to content

Commit 424316b

Browse files
fix(ci): let the two changeset gates see a RENAMED changeset (#7045) (#7106)
`check-empty-changeset.mjs` and `check-adr-0087-registration.mjs` both enumerated the diff with `--diff-filter=AM`. Git's rename detection is on by default, so a changeset renamed and modified in one commit reports as `R` and both filters dropped the row entirely: an emptied changeset, or a newly declared-breaking one with no ADR-0087 disposition, walked past its gate unseen. `--diff-filter=AMR`, reading the branch-point side at the PRE-RENAME path (field 2 of an `R` row, not field 3) — the pattern `check-changeset-no-major.mjs` landed first in PR #7048 for the third member of the family. A pure rename compares equal and stays exempt, so moving a stock changeset still costs nothing. One thing the reference did not need: the base-side read is guarded by `isChangesetFile(basePath)`. Git pairs renames by CONTENT, so an `R` row can arrive as `.changeset/README.md -> .changeset/x.md`, and README declares nothing by definition — inheriting an exemption from it would have been free. Claude-Session: https://claude.ai/code/session_01F8q5J1MQyocgtNspb15fSn Co-authored-by: Claude <noreply@anthropic.com>
1 parent c27f29f commit 424316b

2 files changed

Lines changed: 335 additions & 23 deletions

File tree

‎scripts/check-adr-0087-registration.mjs‎

Lines changed: 158 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,36 @@
7070
// the PR diff, `git grep`, this gate's log, and `--list` -- while rendering as
7171
// nothing in a published changelog. It is not hidden from anyone who reviews.
7272
//
73+
// ## Which diff rows are judged (#7045)
74+
//
75+
// "Newly ADDS (or newly turns breaking)" is a statement about diff STATUS, and
76+
// the enumeration has to name every status a changeset can arrive under:
77+
//
78+
// A breaking at head -> judged (a new declaration)
79+
// M breaking at head, NOT breaking at base -> judged (turned breaking)
80+
// M breaking at head, already breaking at base -> skipped (inherited stock)
81+
// R breaking at head, NOT breaking at the OLD path-> judged (renamed into it)
82+
// R breaking at head, already breaking there -> skipped (stock, moved)
83+
//
84+
// The last two rows arrived late. Git's rename detection is on by default
85+
// (`diff.renames`, since git 2.9), so a changeset renamed AND turned breaking in
86+
// one commit reports as `R`, and this file passed `--diff-filter=AM` until #7045
87+
// -- which drops the `R` row wholesale. A declared-breaking changeset arriving
88+
// that way carried no ADR-0087 disposition and was never asked for one, which is
89+
// #6148 again through a door nobody had checked. Measured on git 2.43.0, renaming
90+
// `.changeset/old.md` to `.changeset/new.md` while adding the breaking declaration
91+
// reports `R077<TAB>.changeset/old.md<TAB>.changeset/new.md`, and the same diff
92+
// under `AM` prints nothing at all.
93+
//
94+
// The fix is `AMR` plus reading the base side at the OLD path -- field 2 of an
95+
// `R` row, not field 3. It costs nothing: a PURE move of an already-breaking
96+
// stock changeset compares breaking-at-base and stays skipped, so no author is
97+
// asked to re-dispose of somebody else's declaration for renaming a file.
98+
// `check-changeset-no-major.mjs` made the same one-letter correction first
99+
// (#7005 / PR #7048); this gate and `check-empty-changeset.mjs` followed in
100+
// #7045, separately, because each of the three owns its own fixtures and its own
101+
// messages.
102+
//
73103
// ## Why the escape hatch is the MAJORITY path, and why that is fine (measured)
74104
//
75105
// Measured over the last 400 first-parent commits on main: 32 newly-added
@@ -1304,14 +1334,24 @@ export function scan({ cwd, base, head = 'HEAD' }) {
13041334
let pkgsCache = null;
13051335
const packages = () => (pkgsCache ??= workspacePackagesAt(head, cwd));
13061336

1307-
const out = git(['diff', '--name-status', '--diff-filter=AM', from, head, '--', '.changeset/*.md'], cwd);
1337+
// `AMR`, not `AM`: see "Which diff rows are judged" in the header (#7045). An
1338+
// `R` row is how a declared-breaking changeset used to arrive unseen.
1339+
const out = git(['diff', '--name-status', '--diff-filter=AMR', from, head, '--', '.changeset/*.md'], cwd);
13081340
const problems = [];
13091341
const judged = [];
13101342
const skipped = [];
13111343

13121344
for (const line of out.split('\n')) {
13131345
if (!line.trim()) continue;
1314-
const [status, file] = line.split('\t');
1346+
const fields = line.split('\t');
1347+
// `R` is `R<score>\t<old path>\t<new path>`; `A` and `M` are `<status>\t<path>`.
1348+
// One character wide, because the `R` letter carries a similarity score and
1349+
// `=== 'R'` against the whole field would never match.
1350+
const status = fields[0][0];
1351+
const file = status === 'R' ? fields[2] : fields[1];
1352+
// The path the branch-point side is READ at: the same one for `M`, the
1353+
// PRE-RENAME one for `R`. Field 2 either way.
1354+
const basePath = fields[1];
13151355
if (!file || !isChangesetFile(file)) continue;
13161356

13171357
const headText = showOrNull(head, file, cwd);
@@ -1321,11 +1361,23 @@ export function scan({ cwd, base, head = 'HEAD' }) {
13211361
const decl = breakingDeclaration(parsed);
13221362
if (!decl.breaking) { skipped.push(file); continue; }
13231363

1324-
// A changeset that was ALREADY breaking at base is inherited, not introduced.
1325-
// Same philosophy as check-empty-changeset: this gate judges what a PR brings,
1326-
// never the stock it forked from.
1327-
if (status === 'M') {
1328-
const baseText = showOrNull(from, file, cwd);
1364+
// A changeset that was ALREADY breaking at the branch point is inherited, not
1365+
// introduced. Same philosophy as check-empty-changeset: this gate judges what a
1366+
// PR brings, never the stock it forked from.
1367+
//
1368+
// `R` is read at `basePath` -- its pre-rename name -- which is the only thing
1369+
// the rename status changes here (#7045). The statuses are spelled out rather
1370+
// than written `!== 'A'` so that widening the filter again some day cannot
1371+
// silently grant an unlisted status the inheritance exemption; an unknown
1372+
// status falls through and gets JUDGED, which is the safe direction.
1373+
//
1374+
// `isChangesetFile(basePath)` because git pairs renames by CONTENT, not by
1375+
// name: an `R` row can arrive as `.changeset/README.md -> .changeset/x.md`,
1376+
// and README is documentation that declares nothing. Inheriting "already
1377+
// breaking" from it would exempt a genuinely new breaking changeset. For `M`
1378+
// the guard is a no-op -- `basePath` is the path already accepted above.
1379+
if ((status === 'M' || status === 'R') && isChangesetFile(basePath)) {
1380+
const baseText = showOrNull(from, basePath, cwd);
13291381
if (baseText !== null && breakingDeclaration(parseChangeset(baseText)).breaking) {
13301382
skipped.push(file);
13311383
continue;
@@ -1776,8 +1828,13 @@ function selfTest() {
17761828
/**
17771829
* Build a two-commit repo: base carries the ledger + a breaking changeset in
17781830
* stock (so the convention assertion is satisfied), head adds `files`.
1831+
*
1832+
* `baseFiles` puts extra files on the BASE commit, and a `null` in `files`
1833+
* deletes one at head. Together they are how a RENAME is expressed (#7045):
1834+
* git infers `R` from a delete plus an add of similar content, so the fixture
1835+
* has to be able to say both halves.
17791836
*/
1780-
const build = ({ baseIds = ['old-entry-one', 'old-entry-two'], headIds = null, files = {}, pkgs = null }) => {
1837+
const build = ({ baseIds = ['old-entry-one', 'old-entry-two'], headIds = null, files = {}, baseFiles = {}, pkgs = null }) => {
17811838
const dir = mkdtempSync(join(tmpdir(), 'adr0087-'));
17821839
const w = (rel, text) => {
17831840
mkdirSync(dirname(join(dir, rel)), { recursive: true });
@@ -1795,6 +1852,7 @@ function selfTest() {
17951852
for (const [name, p] of Object.entries(pkgs ?? { '@objectstack/spec': { dir: 'packages/spec', private: false } })) {
17961853
w(`${p.dir}/package.json`, JSON.stringify({ name, version: '1.0.0', ...(p.private ? { private: true } : {}) }));
17971854
}
1855+
for (const [rel, text] of Object.entries(baseFiles)) w(rel, text);
17981856
git(['add', '-A'], dir);
17991857
git(['commit', '-qm', 'base'], dir);
18001858
const base = git(['rev-parse', 'HEAD'], dir).trim();
@@ -1803,7 +1861,10 @@ function selfTest() {
18031861
w(LEDGER_SOURCES[0], REG(headIds));
18041862
w(SPEC_CHANGES, SPEC_CHANGES_JSON(headIds));
18051863
}
1806-
for (const [rel, text] of Object.entries(files)) w(rel, text);
1864+
for (const [rel, text] of Object.entries(files)) {
1865+
if (text === null) rmSync(join(dir, rel));
1866+
else w(rel, text);
1867+
}
18071868
git(['add', '-A'], dir);
18081869
// `--allow-empty`: some cases deliberately add nothing at head (the "this PR
18091870
// touches no changeset" and "inherited changeset" shapes), and an empty commit
@@ -2120,6 +2181,94 @@ function selfTest() {
21202181
green('G6 inherited breaking changeset not re-judged', run(r));
21212182
}
21222183

2184+
// ---- The `R` rows (#7045) -------------------------------------------------
2185+
//
2186+
// `--diff-filter=AM` -- what this gate passed until #7045 -- drops an `R` row
2187+
// wholesale, so R15 below was GREEN with an undisposed breaking declaration
2188+
// sitting in the diff. Both cases carry a CONTROL asserting that git really
2189+
// emitted `R`: rename detection is a SIMILARITY score, and a short body degrades
2190+
// to add-plus-delete, at which point R15 is an ordinary `A` case that would have
2191+
// been caught before the fix too -- green for a reason that is not the fix.
2192+
const RENAMEABLE_BODY =
2193+
'a summary line, with a body long enough that git scores the move below as a\n' +
2194+
'rename rather than as an add plus a delete. The `R` status is the entire\n' +
2195+
'subject of these two cases, so the fixture may not be allowed to degrade.\n';
2196+
/** The `R` row for `old -> new` in this repo's diff, or null. */
2197+
const renameRow = (dir, base, oldPath, newPath) =>
2198+
git(['diff', '--name-status', base, 'HEAD', '--', '.changeset/*.md'], dir)
2199+
.split('\n')
2200+
.find((l) => new RegExp(`^R\\d+\t${oldPath}\t${newPath}$`).test(l)) ?? null;
2201+
2202+
// ---- R15: a changeset RENAMED AND turned breaking in the same commit ------
2203+
// The #7045 shape, and R1's own shape with a `git mv` bolted on: a declaration
2204+
// this PR introduces, arriving at a path that did not exist at the branch point,
2205+
// saying nothing about the ledger.
2206+
{
2207+
const r = mk({
2208+
baseFiles: { '.changeset/pending.md': CS({ bumps: [['@objectstack/spec', 'patch']], body: RENAMEABLE_BODY }) },
2209+
files: {
2210+
'.changeset/pending.md': null,
2211+
'.changeset/pending-renamed.md': CS({ bumps: [['@objectstack/spec', 'major']], body: `**BREAKING** ${RENAMEABLE_BODY}` }),
2212+
},
2213+
});
2214+
assert(
2215+
renameRow(r.dir, r.base, '\\.changeset/pending\\.md', '\\.changeset/pending-renamed\\.md') !== null,
2216+
`R15 control: git must really report this as \`R\`, or the case below is an ordinary \`A\` -- got ${JSON.stringify(git(['diff', '--name-status', r.base, 'HEAD', '--', '.changeset/*.md'], r.dir))}`,
2217+
);
2218+
red('R15 the #7045 shape (renamed AND turned breaking, no disposition)', run(r), [
2219+
/pending-renamed/,
2220+
/no `adr-0087:` disposition marker/,
2221+
/#6148/,
2222+
]);
2223+
}
2224+
2225+
// ---- G10: a PURE rename of an ALREADY-breaking stock changeset ------------
2226+
// The paired control, and the reason `AMR` costs nothing: the declaration was
2227+
// already at the branch point, so moving the file asks nobody to dispose of it
2228+
// again. This case can only be green through the `R` path -- were the rename to
2229+
// degrade to add-plus-delete the new path would arrive as `A`, and `A` never
2230+
// reads the base side at all, so it would be RED.
2231+
{
2232+
const r = mk({ files: {} });
2233+
git(['mv', '.changeset/stock-breaking.md', '.changeset/stock-breaking-moved.md'], r.dir);
2234+
git(['commit', '-qm', 'move the stock changeset'], r.dir);
2235+
assert(
2236+
renameRow(r.dir, r.base, '\\.changeset/stock-breaking\\.md', '\\.changeset/stock-breaking-moved\\.md') !== null,
2237+
'G10 control: git must really report the move as `R`',
2238+
);
2239+
const res = run(r);
2240+
green('G10 a pure move of an inherited breaking changeset is not re-judged', res);
2241+
assert(
2242+
res.skipped.includes('.changeset/stock-breaking-moved.md'),
2243+
`G10: ...and it is reported as SKIPPED at its new path, not silently absent -- got ${JSON.stringify(res.skipped)}`,
2244+
);
2245+
}
2246+
2247+
// ---- R16: an `R` row whose BASE side is README.md inherits NOTHING --------
2248+
// Git pairs renames by CONTENT, not by name, so an `R` row can arrive as
2249+
// `.changeset/README.md -> .changeset/x.md` (measured, git 2.43.0). README is
2250+
// documentation and declares nothing, so reading "already breaking at base" off
2251+
// it would exempt a genuinely new breaking changeset. Delete the
2252+
// `isChangesetFile(basePath)` guard in the scan and this case goes green.
2253+
{
2254+
const BREAKING_README = `# Changesets\n\n**BREAKING** ${RENAMEABLE_BODY}`;
2255+
const r = mk({
2256+
baseFiles: { '.changeset/README.md': BREAKING_README },
2257+
files: {
2258+
'.changeset/README.md': null,
2259+
'.changeset/was-the-readme.md': `---\n'@objectstack/spec': major\n---\n\n${BREAKING_README}`,
2260+
},
2261+
});
2262+
assert(
2263+
renameRow(r.dir, r.base, '\\.changeset/README\\.md', '\\.changeset/was-the-readme\\.md') !== null,
2264+
`R16 control: git must really pair the new changeset with README.md -- got ${JSON.stringify(git(['diff', '--name-status', r.base, 'HEAD', '--', '.changeset/*.md'], r.dir))}`,
2265+
);
2266+
red('R16 a breaking changeset paired with README.md by rename detection is still judged', run(r), [
2267+
/was-the-readme/,
2268+
/no `adr-0087:` disposition marker/,
2269+
]);
2270+
}
2271+
21232272
// ---- Input assertions (#4690) --------------------------------------------
21242273
{
21252274
const r = mk({ files: {} });

0 commit comments

Comments
 (0)