Skip to content

Commit 84ba4a8

Browse files
test(cli): read the emitJson exit count off the source in the two os validate --json pins (#18880)
Fixes #18848 Clause-②: no — the diff is two files under `packages/cli/test/`, no governed surface (`docs/adr/**`, `.claude/**`, `skills/**`, `AGENTS.md`, `CLAUDE.md`). Declared from the diff, and it agrees with the claim comment's declaration from the card. `packages/cli/src/commands/validate.ts` is **untouched** — read only, never written. The seventh exit is correct and stays. ## What was actually wrong `validate.ts` has **7** `await emitJson(` exits; two nightly-only pins asserted **6**. | reading | value | |:--|:--| | `await emitJson(` exits in `validate.ts` at `b0b5f31cc6` | **7** (`:286 :364 :434 :517 :554 :661 :762`) | | control at `095c7f60ae^` (pre-#18769) | **6** | | what both pins asserted | `toHaveLength(6)` | | exits carrying `warnings:` | **7 of 7** | | exits carrying `conversions:` | **7 of 7** | ⇒ both CONTRACTS these two files exist for still hold. Only the COUNT was stale. Re-measured independently here and the dispatching seat's figures reproduce exactly. The seventh exit came with #18769 and is legitimate. Its payload carries both keys. ## The choice: DERIVED, not `7` The claim asked for the option taken and the cost of the rejected one. **Rejected — `toHaveLength(7)`.** It is one character of work and it is wrong for a measurable reason. It re-arms the identical trap for exit eight, and the trap is not theoretical: it just cost a night of red `main` and was about to cost a duplicate p1 card. Its cost is not only that it rots, but **where** it rots — see the tier reading below. And it is strictly weaker: ablation C is a `validate.ts` with eight call sites where a pin of `7` reads **green** while the file has an exit no assertion in the family covers. **Taken — read the count off the source.** The extractor must produce one payload literal per `emitJson` call site the file actually has: ``` callSites = every `emitJson` call in SRC, matched with a word boundary, optional whitespace, then the opening paren expect(literals).toHaveLength(callSites.length) ``` Both sides read raw source, which keeps them symmetric: a commented-out `await emitJson(` is counted by the extractor and by the pattern alike. The one asymmetric case — prose naming the call with its paren but no `await` — reddens, and that is the accepted price for not importing a comment masker into these two files. Nothing is skipped, disabled, quarantined or deleted. The count assertion is still there, the success/failure partition is still there, the contract negative is still there: - `toHaveLength(6)` → `toBeGreaterThanOrEqual(6)`. The **one integer left**, demoted from an equality to a floor. Six is the population the #12047 / #12125 rulings were made over, not a count of today; it rots only in the direction that has to be reviewed anyway (exits being *removed*), and it floors the derived pair away from the single vacuum they share — an `emitJson` renamed out of existence takes both sides to zero and every assertion here with them. **I write no `7` anywhere in this diff.** - `valid: false` × 5 → `literals.length - 1`, beside `valid: true` × 1. Derived, and the two together now also assert the PARTITION: an exit carrying neither literal reddens, which the two frozen integers never checked. ## The tier, measured in BOTH directions Not inferred from the workflow YAML — collected, on this branch, in `packages/cli`: | `OS_TEST_TIERS` | files vitest collects | these two pins among them | |:--|--:|:--| | unset (pull request · merge queue · local default) | 266 | **0** | | `nightly` | 68 | **2** — both, in the `integration` project | ⇒ the pins are not "a red nobody looks at". In the queue population they are **not collected at all**. No pull request and no merge-queue run could have reddened on this, which is why #18769's own CI was green and correct. That is a measurement for #18520, not a new card — it is in the report for the seat. ## Red → green, in the nightly tier itself Not a substitute: the tier the pins actually live in, reached locally with `OS_TEST_TIERS=nightly`. **Before** (at `b0b5f31cc6`, pre-fix): ``` FAIL |integration| test/validate-json-failure-conversions.e2e.test.ts AssertionError: the `emitJson` exit count moved — a new exit must carry `conversions` too: expected [ …(7) ] to have a length of 6 but got 7 FAIL |integration| test/validate-json-failure-warnings.e2e.test.ts AssertionError: … expected [ …(7) ] to have a length of 6 but got 7 Test Files 2 failed (2) Tests 2 failed | 5 passed | 16 skipped (23) ``` This is also the **first runner-level reproduction** of this card: the filing seat stated it had no install and relayed the PR run's result instead. **After:** ``` Test Files 4 passed (4) Tests 14 passed | 36 skipped (50) ``` Four files, because the run also carries the two `build-json-failure-*` siblings — see acceptance notes. ## Ablation — four legs, mutation proven on disk, restore proven byte-exact Run from the **committed** fix. The mutation is confined to my own two test files (the `SRC` seed), driven by env var; `validate.ts` is never written. `trap … EXIT INT TERM`, absolute paths, and the restore is verified by `git diff HEAD` empty **and** `git hash-object` equal to the HEAD blob hash (non-empty, both files). | leg | mutation | expected | observed | |:--|:--|:--|:--| | **A** | an 8th exit, `await`, carrying both keys — an *honest addition* | green | **green** — 2 files, 7 passed | | **B** | an 8th exit missing `warnings:` | warnings red, conversions green | **exactly that** — `expected [ Array(1) ] to deeply equal []` | | **C** | an 8th call site spelled **without** `await` | derived equality red | **both red** — ``an `emitJson(` call site the payload extractor could not read: expected [ …(7) ] to have a length of 8 but got 7`` | | **D** | source truncated to 5 exits | floor red | **both red** — `expected 5 to be greater than or equal to 6` | Leg **A** is the fix doing its job: a hard-coded `7` is red here. Leg **C** is the capability the rejected option does not have at all — the extractor reports 7, so `toHaveLength(7)` passes while the file carries an exit no assertion covers. Leg **B** proves deriving the count weakened nothing: the contract still bites over the *added* exit, and the two files isolate from each other. Leg **D** proves the remaining integer is live. A first ablation attempt exited 127 on a wrong vitest path — the mutation landed but nothing ran, so those readings were void and discarded rather than reported. The table above is the re-run. ## Changeset `skip-changeset`. Measured, not assumed — nothing published moves: - `packages/cli` `files[]` is `["dist", "README.md", "CHANGELOG.md"]`; `tsconfig.build.json` has `include: ["src"]`, so `test/` is outside the compiled tree entirely. - `npm pack --dry-run --json`: **0** published paths under `test/` or `src/`. Instrument lit by the positive control `README.md`, which is present. - ⚠️ The `dist/` positive control was **not lit** in the worktree where that manifest was taken (no build yet at that point). The `include: ["src"]` boundary is what carries the negative, and the closure build reading is in the report. ## Acceptance notes — noted, not filed - **The same rotting shape sits on two more pins, green today.** `test/build-json-failure-warnings.e2e.test.ts` and `test/build-json-failure-conversions.e2e.test.ts` pin `toHaveLength(11)` / `10` / `1` over `compile.ts`. Measured: `compile.ts` has 11 literals, 10 `success: false`, 1 `success: true` — **green right now**, and one honest exit away from repeating this exact p1, again only at night. ⛔ Deliberately **not** touched here: `compile.ts` is a #18779 carrier and is fenced for this card, and a p1 with a cron clock is not the place to double the review surface. Offered to #18520 as a measurement rather than as a new card. - **`validate.ts:661` carries a comment that the seventh exit made false** — it says the ordering site is read by "every one of the six exits" and that "a seventh exit cannot be added with a different member order". A doc nit inside a read-only file; carrier: the next PR that touches `validate.ts`. ## An instruction conflict, named rather than quietly resolved Triage comment `5723031597` pinned the dispatch order: land the two-line literal repair first, and take the "does an integer pin belong here at all" question as a second, separate stroke. The claim comment `5724808732` instead hands the choice to the deliverer and says ⛔ not to reflexively hard-code `7`. I took the claim's instruction, because the thing triage was protecting against is not in play: the derived form is the *same edit in the same assertion slot*, measured and proven in this run, and it delays the red `main` by nothing. The design question triage deferred — whether these files belong in the core tier, or the tier's path filter should name `validate.ts` — is untouched here and remains the cli lane's call. ## Verification `OS_TEST_TIERS` measured in both directions · red→green in the nightly tier · four ablation legs · gate families derived with `scripts/pm/dispatch-gates.mjs` and reconciled with `--ran`. Full readings, including anything that came back NOT MEASURED, are in the `os-dev-report` comment on #18848. --- _Generated by [Claude Code](https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 600b1e2 commit 84ba4a8

2 files changed

Lines changed: 94 additions & 6 deletions

File tree

‎packages/cli/test/validate-json-failure-conversions.e2e.test.ts‎

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -410,11 +410,55 @@ describe('#12125 — the contract is exhaustive over `validate.ts`, not just ove
410410
expect(found[1]).toContain('conversions: conversionNotices');
411411
});
412412

413-
it('all 6 `emitJson` exits carry `conversions` — 5 failure exits and the success payload', () => {
413+
it('every `emitJson` exit carries `conversions` — the exit count is READ OFF the source, never an integer', () => {
414414
const literals = payloadLiterals(SRC);
415-
expect(literals, 'the `emitJson` exit count moved — a new exit must carry `conversions` too').toHaveLength(6);
416-
expect(literals.filter((p) => p.includes('valid: false'))).toHaveLength(5);
415+
416+
// ⭐ #18848 — this line read `toHaveLength(6)`, and that integer is what
417+
// put `main` in the red without a run to say so. `validate.ts` gained a
418+
// seventh, entirely legitimate exit (#18769, the per-package author-time
419+
// rule pass) whose payload carries this key like every other; the CONTRACT
420+
// below held throughout and only the COUNT was stale. And because this
421+
// file is nightly-tier by NAME (`*.e2e.test.ts` — `scripts/nightly-tiers.mjs`),
422+
// no pull request and no merge-queue run could collect it: measured on the
423+
// branch that landed this, `OS_TEST_TIERS` unset collects 266 files here
424+
// with ZERO of them this one, and `=nightly` collects 68 with this one in
425+
// it. An integer that only a cron can read is a pin with no reader at the
426+
// moment it matters.
427+
//
428+
// ⛔ So the count is not re-pinned to 7 — that just re-arms the same trap
429+
// for exit eight. It is DERIVED: the extractor read every `emitJson` call
430+
// site the file has, whatever today's number is. An honest new exit stays
431+
// green here and is still held to the contract below and to the
432+
// `warningsSoFar()` loop further down; a needle that stopped matching one
433+
// reddens HERE instead of quietly turning that negative into a vacuous
434+
// pass over fewer exits than the file has.
435+
//
436+
// Both sides read RAW source, which is what keeps them symmetric: a
437+
// commented-out `await emitJson(` is counted by the extractor and by the
438+
// pattern alike. The one asymmetric case — prose naming the call with its
439+
// paren but no `await` — reddens, and that is the accepted price for not
440+
// importing a comment masker into this file.
441+
const callSites = SRC.match(/\bemitJson\s*\(/g) ?? [];
442+
expect(
443+
literals,
444+
'an `emitJson(` call site the payload extractor could not read — the contract below would skip it',
445+
).toHaveLength(callSites.length);
446+
447+
// The one integer left, and it rots only in the direction that has to be
448+
// reviewed anyway: exits being REMOVED. Six is the population the ruling
449+
// above was made over, not a count of today. It is also the floor under
450+
// the one vacuum the derived pair shares — an `emitJson` renamed out of
451+
// existence takes BOTH sides to zero and every assertion here with them.
452+
expect(
453+
literals.length,
454+
'`os validate --json` publishes fewer exits than the ruling above was made over',
455+
).toBeGreaterThanOrEqual(6);
456+
457+
// Exactly one success payload; every other exit is a failure exit. The
458+
// second count is derived from the length, so the two together also assert
459+
// the PARTITION — an exit carrying neither literal reddens.
417460
expect(literals.filter((p) => p.includes('valid: true'))).toHaveLength(1);
461+
expect(literals.filter((p) => p.includes('valid: false'))).toHaveLength(literals.length - 1);
418462

419463
const bare = literals.filter((p) => !p.includes('conversions:'));
420464
expect(

‎packages/cli/test/validate-json-failure-warnings.e2e.test.ts‎

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -495,11 +495,55 @@ describe('#12047 — the contract is exhaustive over `validate.ts`, not just ove
495495
expect(found[1]).toContain('warnings: warningsSoFar()');
496496
});
497497

498-
it('all 6 `emitJson` exits carry `warnings` — 5 failure exits and the success payload', () => {
498+
it('every `emitJson` exit carries `warnings` — the exit count is READ OFF the source, never an integer', () => {
499499
const literals = payloadLiterals(SRC);
500-
expect(literals, 'the `emitJson` exit count moved — a new exit must carry `warnings` too').toHaveLength(6);
501-
expect(literals.filter((p) => p.includes('valid: false'))).toHaveLength(5);
500+
501+
// ⭐ #18848 — this line read `toHaveLength(6)`, and that integer is what
502+
// put `main` in the red without a run to say so. `validate.ts` gained a
503+
// seventh, entirely legitimate exit (#18769, the per-package author-time
504+
// rule pass) whose payload carries this key like every other; the CONTRACT
505+
// below held throughout and only the COUNT was stale. And because this
506+
// file is nightly-tier by NAME (`*.e2e.test.ts` — `scripts/nightly-tiers.mjs`),
507+
// no pull request and no merge-queue run could collect it: measured on the
508+
// branch that landed this, `OS_TEST_TIERS` unset collects 266 files here
509+
// with ZERO of them this one, and `=nightly` collects 68 with this one in
510+
// it. An integer that only a cron can read is a pin with no reader at the
511+
// moment it matters.
512+
//
513+
// ⛔ So the count is not re-pinned to 7 — that just re-arms the same trap
514+
// for exit eight. It is DERIVED: the extractor read every `emitJson` call
515+
// site the file has, whatever today's number is. An honest new exit stays
516+
// green here and is still held to the contract below and to the
517+
// `warningsSoFar()` loop further down; a needle that stopped matching one
518+
// reddens HERE instead of quietly turning that negative into a vacuous
519+
// pass over fewer exits than the file has.
520+
//
521+
// Both sides read RAW source, which is what keeps them symmetric: a
522+
// commented-out `await emitJson(` is counted by the extractor and by the
523+
// pattern alike. The one asymmetric case — prose naming the call with its
524+
// paren but no `await` — reddens, and that is the accepted price for not
525+
// importing a comment masker into this file.
526+
const callSites = SRC.match(/\bemitJson\s*\(/g) ?? [];
527+
expect(
528+
literals,
529+
'an `emitJson(` call site the payload extractor could not read — the contract below would skip it',
530+
).toHaveLength(callSites.length);
531+
532+
// The one integer left, and it rots only in the direction that has to be
533+
// reviewed anyway: exits being REMOVED. Six is the population the ruling
534+
// above was made over, not a count of today. It is also the floor under
535+
// the one vacuum the derived pair shares — an `emitJson` renamed out of
536+
// existence takes BOTH sides to zero and every assertion here with them.
537+
expect(
538+
literals.length,
539+
'`os validate --json` publishes fewer exits than the ruling above was made over',
540+
).toBeGreaterThanOrEqual(6);
541+
542+
// Exactly one success payload; every other exit is a failure exit. The
543+
// second count is derived from the length, so the two together also assert
544+
// the PARTITION — an exit carrying neither literal reddens.
502545
expect(literals.filter((p) => p.includes('valid: true'))).toHaveLength(1);
546+
expect(literals.filter((p) => p.includes('valid: false'))).toHaveLength(literals.length - 1);
503547

504548
const bare = literals.filter((p) => !p.includes('warnings:'));
505549
expect(

0 commit comments

Comments
 (0)