From a457602042af56e4313b3e28e070613e79e8029a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 19 Aug 2026 19:36:44 +0000 Subject: [PATCH 1/3] fix(scripts): keep packages.count true across --union-into, and assert it on read `check-cross-package-test-inputs.mjs --union-into` appended to `packages.items` and never touched `packages.count`, so the document it handed to `partition-test-shards.mjs` said `count: 0` while carrying two items. Inert only because nothing reads `count` -- but the consumer's stated job is to assert this payload's shape loudly so a `turbo ls` upgrade becomes a red step naming the cause rather than a silently empty shard, and it was being fed a document that contradicts itself. `count` is turbo's own field, not this script's invention: `turbo ls --output=json` emits `{packageManager, packages:{count, items}}` and keeps count === items.length (measured on turbo 2.10.10 across the bare, --filter and --affected forms). So it is maintained, not deleted. Both halves ship together: - the writer reconciles `count` with `items.length` before writing; - the reader (`readPackageItems()`) refuses a payload whose `count` disagrees with `items.length`, naming the contradiction. A payload with no `count` is accepted -- a redundant field's absence cannot mis-shard anything, its disagreement can. Both `--self-test` suites pin the invariant, so it cannot rot back. No change to which packages are selected, sharded or tested. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja --- scripts/check-cross-package-test-inputs.mjs | 36 +++++++++ scripts/partition-test-shards.mjs | 84 ++++++++++++++++++--- 2 files changed, 111 insertions(+), 9 deletions(-) diff --git a/scripts/check-cross-package-test-inputs.mjs b/scripts/check-cross-package-test-inputs.mjs index f115f4a006..89e05e03ec 100644 --- a/scripts/check-cross-package-test-inputs.mjs +++ b/scripts/check-cross-package-test-inputs.mjs @@ -1009,6 +1009,27 @@ function expectedInputs(globs) { return ['$TURBO_DEFAULT$', '!dist/**', '!coverage/**', '!.turbo/**', ...globs.map((g) => `$TURBO_ROOT$/${g}`)]; } +/** + * `turbo ls --output=json` emits `packages.count` beside `packages.items`, and + * keeps the two equal -- measured on turbo 2.10.10, all of the bare, `--filter` + * and `--affected` forms agree. So `count` is TURBO's field, not this script's + * invention, and a document we have appended to is a valid `turbo ls` payload + * only while the count moves with the array. + * + * Nothing reads `count` today, which is exactly what makes it cheap to keep + * true and expensive to leave stale: the consumer is partition-test-shards.mjs, + * whose stated posture is to assert this payload's shape LOUDLY so an + * experimental-command upgrade becomes a red step naming the cause rather than + * a silently empty shard. A hand-mutated document that contradicts itself about + * its own size is the input to that assertion. The reader now checks the + * agreement (`readPackageItems()` there), so this is a checked invariant across + * the two scripts rather than a convention someone has to remember. + */ +function reconcilePackageCount(packages) { + packages.count = packages.items.length; + return packages; +} + /** * Layer A. Adds any declaring package whose globs the diff touches to the * package list ci.yml is about to shard, so the scan runs on the PR that @@ -1045,6 +1066,11 @@ function unionInto(listPath, changedPath) { items.push({ name, path: join(REPO_ROOT, dir) }); added.push(`${name} (declared glob matched ${hit})`); } + // The push above changed the list's size, so the size the document DECLARES + // has to move with it -- see reconcilePackageCount(). Unconditional rather + // than `if (added.length)`: the invariant is a property of the document we + // write, not of whether this particular run had anything to add. + reconcilePackageCount(parsed.packages); writeFileSync(listPath, JSON.stringify(parsed)); if (added.length) { console.log('Cross-package scans pulled into this run because the diff touched their declared inputs:'); @@ -1371,6 +1397,16 @@ function selfTest() { ok('a single-file glob does not cover the directory it sits in', !coversDirectory('scripts', ['scripts/check-nul-bytes.mjs'])); ok('a directory that does not exist is covered by nothing', !coversDirectory('scripts/no-such-dir-9763', ['**'])); + // `--union-into`'s output document. `packages.count` is turbo's field and the + // append changes the size it describes, so the two are one operation -- these + // pin the half of the cross-script invariant this side owns (the reader's + // half is partition-test-shards.mjs `--self-test`). + const counted = (packages) => reconcilePackageCount(packages).count; + ok('count follows an appended item', counted({ count: 0, items: [{ name: 'a', path: 'p' }, { name: 'b', path: 'q' }] }) === 2); + ok('a correct count is left correct', counted({ count: 1, items: [{ name: 'a', path: 'p' }] }) === 1); + ok('count follows an empty list down', counted({ count: 7, items: [] }) === 0); + ok('the reconciliation never invents items', reconcilePackageCount({ count: 0, items: [] }).items.length === 0); + const failed = cases.filter((c) => !c.cond); for (const c of cases) console.log(`${c.cond ? 'ok ' : 'FAIL'} ${c.label}`); if (failed.length) { diff --git a/scripts/partition-test-shards.mjs b/scripts/partition-test-shards.mjs index 7619ffcc37..a34002d26a 100644 --- a/scripts/partition-test-shards.mjs +++ b/scripts/partition-test-shards.mjs @@ -28,8 +28,9 @@ // node scripts/partition-test-shards.mjs --self-test // // is the output of `turbo ls [--affected] --output=json` -// (shape: {packages:{items:[{name,path}]}}; `turbo ls` is marked experimental, -// so the shape is asserted loudly below rather than defaulted around). +// (shape: {packages:{count,items:[{name,path}]}}; `turbo ls` is marked +// experimental, so the payload is asserted loudly in readPackageItems() below +// rather than defaulted around). // Prints the selected shard's package names, one per line -- possibly zero // lines, which the caller must treat as "nothing to run", NOT as "no filter": // a `turbo run test` with no --filter args runs the entire workspace. @@ -78,6 +79,51 @@ export function partition(items, shardCount) { return bins; } +// Reads the package list out of a `turbo ls --output=json` payload, asserting +// two independent properties. They fail for different reasons and both are +// loud, because the failure this whole file guards against is the quiet one -- +// a shard that tested nothing and went green. +// +// SHAPE -- `packages.items` must be an array. `turbo ls` is experimental, so +// an upgrade that renames or restructures this becomes a red step +// naming the cause rather than an empty shard. +// SIZE -- when the payload carries turbo's own `packages.count`, it must +// equal `items.length`. turbo never breaks this itself (measured on +// 2.10.10: the bare, `--filter` and `--affected` forms all agree), +// so a payload that DOES has been hand-mutated or truncated between +// turbo and here and is not trustworthy about how many packages +// this shard is meant to see. There is exactly one such mutator in +// this repo -- `--union-into` in check-cross-package-test-inputs.mjs, +// which appends the cross-package scans the dependency graph cannot +// reach -- and it maintains `count`. This assertion is what makes +// that a checked fact instead of a convention: it wrote a `count: 0` +// document alongside two items for as long as nobody looked. +// +// A payload carrying NO `count` is accepted on purpose. The field is redundant +// with the array, so its ABSENCE cannot mis-shard anything, while its +// DISAGREEMENT can; requiring it would turn a turbo upgrade that merely dropped +// a field nobody reads into a red Test Core on every PR. Note this is a +// redundancy check, not lenient parsing -- a `count` that is present and wrong +// is rejected, never repaired. +export function readPackageItems(parsed, listPath) { + const items = parsed?.packages?.items; + if (!Array.isArray(items)) { + throw new Error( + `${listPath}: expected \`turbo ls --output=json\` shape {packages:{items:[...]}} -- ` + + 'did an experimental-command upgrade change the output?' + ); + } + const count = parsed.packages.count; + if (count !== undefined && count !== items.length) { + throw new Error( + `${listPath}: packages.count is ${JSON.stringify(count)} but packages.items holds ` + + `${items.length} -- the payload contradicts itself about its own size, so it has ` + + 'been hand-mutated or truncated since `turbo ls` wrote it. Refusing to shard it.' + ); + } + return items; +} + function selfTest() { const mk = (name, weight) => ({ name, weight }); // Coverage + determinism: every package lands in exactly one bin, and two @@ -101,6 +147,32 @@ function selfTest() { if (empty.some((bin) => bin.names.length > 0)) throw new Error('empty input produced packages'); const sparse = partition([mk('only', 5)], 3); if (sparse.flatMap((bin) => bin.names).join() !== 'only') throw new Error('sparse input lost the package'); + + // Payload assertions. The document reaching this script has two writers -- + // `turbo ls` and `--union-into` in check-cross-package-test-inputs.mjs -- so + // "count agrees with items" is a cross-script invariant; this is its reading + // half (the writing half is that script's own `--self-test`). + const threw = (fn) => { + try { + fn(); + return false; + } catch { + return true; + } + }; + const doc = (packages) => ({ packageManager: 'pnpm9', packages }); + const two = [{ name: 'a', path: 'p' }, { name: 'b', path: 'q' }]; + if (readPackageItems(doc({ count: 2, items: two }), 'f').length !== 2) throw new Error('payload: a consistent list was rejected'); + if (readPackageItems(doc({ items: two }), 'f').length !== 2) throw new Error('payload: a list with no count was rejected'); + if (readPackageItems(doc({ count: 0, items: [] }), 'f').length !== 0) throw new Error('payload: a legitimately empty list was rejected'); + // The exact document `--union-into` used to write: two items, count still 0. + if (!threw(() => readPackageItems(doc({ count: 0, items: two }), 'f'))) throw new Error('payload: count 0 beside 2 items was accepted'); + if (!threw(() => readPackageItems(doc({ count: 3, items: two }), 'f'))) throw new Error('payload: an over-count was accepted'); + if (!threw(() => readPackageItems(doc({ count: '2', items: two }), 'f'))) throw new Error('payload: a non-numeric count was accepted'); + if (!threw(() => readPackageItems(doc({ count: 2 }), 'f'))) throw new Error('payload: a missing items array was accepted'); + if (!threw(() => readPackageItems(doc({ count: 0, items: {} }), 'f'))) throw new Error('payload: a non-array items was accepted'); + if (!threw(() => readPackageItems({}, 'f'))) throw new Error('payload: a document with no packages key was accepted'); + console.log('partition-test-shards: self-test OK'); } @@ -131,13 +203,7 @@ function main() { if (shardIndex > shardCount) throw new Error(`--shard ${shardSpec}: index exceeds count`); const parsed = JSON.parse(readFileSync(listPath, 'utf8')); - const items = parsed?.packages?.items; - if (!Array.isArray(items)) { - throw new Error( - `${listPath}: expected \`turbo ls --output=json\` shape {packages:{items:[...]}} -- ` + - 'did an experimental-command upgrade change the output?' - ); - } + const items = readPackageItems(parsed, listPath); const weighted = []; for (const it of items) { if (typeof it?.name !== 'string' || typeof it?.path !== 'string') { From 0e827d941169127082a1fe7e5ba131442926663c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 19 Aug 2026 19:38:12 +0000 Subject: [PATCH 2/3] refactor(scripts): fold the count reconciliation into the single write path A `reconcile(); write();` pair re-creates the original defect the moment someone adds a second write path -- "appended to items but forgot to move count" stays a reachable state. Reconciling inside the serializer makes it unreachable: `unionInto()` has exactly one `writeFileSync` and it has no other source of bytes. The self-test cases now assert on the parsed-back document, so they pin what actually lands on disk rather than an intermediate object. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja --- scripts/check-cross-package-test-inputs.mjs | 35 +++++++++++++-------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/scripts/check-cross-package-test-inputs.mjs b/scripts/check-cross-package-test-inputs.mjs index 89e05e03ec..bdd2f265c5 100644 --- a/scripts/check-cross-package-test-inputs.mjs +++ b/scripts/check-cross-package-test-inputs.mjs @@ -1024,10 +1024,17 @@ function expectedInputs(globs) { * its own size is the input to that assertion. The reader now checks the * agreement (`readPackageItems()` there), so this is a checked invariant across * the two scripts rather than a convention someone has to remember. + * + * Reconciling inside the SERIALIZER rather than as a statement beside the write + * is the point: `unionInto()` has exactly one `writeFileSync`, and it has no + * other source of bytes, so "appended to `items` but forgot to move `count`" is + * not a state this script can reach. A separate `reconcile(); write();` pair + * would have re-created the original defect the first time someone added a + * second write path. */ -function reconcilePackageCount(packages) { - packages.count = packages.items.length; - return packages; +function serializePackageList(parsed) { + parsed.packages.count = parsed.packages.items.length; + return JSON.stringify(parsed); } /** @@ -1067,11 +1074,9 @@ function unionInto(listPath, changedPath) { added.push(`${name} (declared glob matched ${hit})`); } // The push above changed the list's size, so the size the document DECLARES - // has to move with it -- see reconcilePackageCount(). Unconditional rather - // than `if (added.length)`: the invariant is a property of the document we - // write, not of whether this particular run had anything to add. - reconcilePackageCount(parsed.packages); - writeFileSync(listPath, JSON.stringify(parsed)); + // moves with it -- serializePackageList() is the only way this function turns + // `parsed` into bytes, precisely so that cannot be skipped. + writeFileSync(listPath, serializePackageList(parsed)); if (added.length) { console.log('Cross-package scans pulled into this run because the diff touched their declared inputs:'); for (const a of added) console.log(` + ${a}`); @@ -1401,11 +1406,15 @@ function selfTest() { // append changes the size it describes, so the two are one operation -- these // pin the half of the cross-script invariant this side owns (the reader's // half is partition-test-shards.mjs `--self-test`). - const counted = (packages) => reconcilePackageCount(packages).count; - ok('count follows an appended item', counted({ count: 0, items: [{ name: 'a', path: 'p' }, { name: 'b', path: 'q' }] }) === 2); - ok('a correct count is left correct', counted({ count: 1, items: [{ name: 'a', path: 'p' }] }) === 1); - ok('count follows an empty list down', counted({ count: 7, items: [] }) === 0); - ok('the reconciliation never invents items', reconcilePackageCount({ count: 0, items: [] }).items.length === 0); + // These run the real serializer -- the one and only source of the bytes + // `unionInto()` writes -- and assert on the parsed-back document, so they pin + // what lands on disk rather than an intermediate object. + const written = (packages) => JSON.parse(serializePackageList({ packageManager: 'pnpm9', packages })).packages; + ok('count follows an appended item', written({ count: 0, items: [{ name: 'a', path: 'p' }, { name: 'b', path: 'q' }] }).count === 2); + ok('a correct count is left correct', written({ count: 1, items: [{ name: 'a', path: 'p' }] }).count === 1); + ok('count follows an empty list down', written({ count: 7, items: [] }).count === 0); + ok('the write never invents items', written({ count: 0, items: [] }).items.length === 0); + ok('the write leaves turbo\'s other fields alone', JSON.parse(serializePackageList({ packageManager: 'pnpm9', packages: { count: 0, items: [] } })).packageManager === 'pnpm9'); const failed = cases.filter((c) => !c.cond); for (const c of cases) console.log(`${c.cond ? 'ok ' : 'FAIL'} ${c.label}`); From a0b947783d83b83be005037560eeeec60b76a0d3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 19 Aug 2026 19:40:30 +0000 Subject: [PATCH 3/3] ci(lint): run the shard partitioner's self-test `scripts/partition-test-shards.mjs --self-test` existed but nothing ever invoked it -- not package.json, not any workflow -- so every assertion in it evaluated never, the partitioner's determinism and coverage pins included. A pin nobody runs is not a weaker pin, it is no pin, and the reader-side refusal added in this branch needs a live one or it rots the same way the writer's count did. Invoked as `node` rather than a `pnpm check:*` alias because root package.json is declared territory of the @changesets/cli v3 lane (#9465), matching the release-rehearsal self-test step above it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja --- .github/workflows/lint.yml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index b46dff47ee..77f38923b3 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -1079,6 +1079,26 @@ jobs: - name: Cross-package test inputs run: pnpm check:cross-package-test-inputs + # The READING half of the gate above (#10046). `--union-into` appends the + # cross-package scans to the `turbo ls` document ci.yml is about to shard, + # and `scripts/partition-test-shards.mjs` is the only thing that reads it. + # That script asserts the payload shape loudly on purpose — an + # experimental-command upgrade should become a red step naming the cause + # rather than a silently empty shard — but it had carried a `--self-test` + # that NOTHING ran since it was written, so every assertion in it, the + # partitioner's determinism and coverage pins included, evaluated never. + # A pin nobody runs is not a weaker pin, it is no pin: `--union-into` wrote + # a `count: 0` document alongside two items for as long as nobody looked, + # and the reader-side refusal that now catches that needs a live pin of its + # own or it rots the same way. + # Invoked as `node` rather than through a `pnpm check:*` alias for the same + # reason as the release-rehearsal self-test above: that alias belongs in + # root package.json, declared territory of the @changesets/cli v3 lane + # (#9465) while it runs. dispatch-gates.mjs derives gate families from + # either spelling. Pure functions, no IO, milliseconds. + - name: Shard partitioner self-test + run: node scripts/partition-test-shards.mjs --self-test + # The inventory of `packages/**` tests coupled to `examples/**` (#8754). # Sibling of the gate above, on the axis it cannot see: that one detects # tests whose FILESYSTEM READS escape their package, this one detects