Skip to content

Commit c7dc089

Browse files
fix(cli): os build's text face renders every author-time advisory its summary line counts (#18857)
Fixes #18780 Clause-②: no `os build`'s text face counted per-package author-time advisories it never printed: `⚠ 4 author-time warning(s) — see above` standing over a list of 3. The list grows to the count. ⚠️ **This is a RESUMED delivery.** Two commits were already on this branch from a run whose container was killed before it opened a PR. They carried **no** verification — no test reading, no ablation, no changeset measurement, no gate sweep survived. They were re-read adversarially and re-measured from scratch; three defects in them are corrected in this PR and are listed below. ## The card's reading, re-derived rather than relayed The card recorded the 4-vs-3 count as the #18677 deliverer's, explicitly not re-driven by the filing seat. It reproduces exactly. Measured on `examples/app-multi-package`, `compile.ts` at this branch's merge-base `ad1f94e8ec`, CLI run from source, `NO_COLOR=1`: ``` os build exit 0 3 rendered advisory entries summary: "4 author-time warning(s) — see above" os build --json exit 0 warnings: 4, of which 1 carries a `package 'ID' — ` prefix os validate exit 0 4 rendered advisory entries (no "see above" sentence on that face at all) ``` With `compile.ts` at this branch's HEAD, same fixture: `os build` renders **4**, summary reads **4**, `--json` still carries **4**. The mutation was proven on disk by blob hash before each reading and the restore proven by an empty `git diff HEAD` — not by an editing command's exit code. ⭐ One correction to the card's framing, offered rather than assumed: `os validate` renders its four through a *different* printer — one line per advisory, no closing `rule: ID at PATH` line — and it has no summary sentence to keep honest. So "the two doors disagree on the rendered list" is right, but only `os build` can carry this defect at all. `os lint` cannot either: `see above` appears in exactly one place in `packages/cli/src`, and it is `compile.ts`. ## Which side moves, and what the chosen side costs — measured The card deliberately did not rule on which side moves. **The list moves, not the count**, and the cost of that is measured rather than argued: | face | before | after | delta | |:--|:--|:--|:--| | single-package stack (no `packages[]`) | 2038 bytes | 2038 bytes | the two clocks only — `Load time: Nms`, `Build complete (Nms)` | | union-level author-time FAILURE | 3590 bytes | 3590 bytes | `Load time: Nms` only | | multi-package stack | 3 entries under a count of 4 | 4 entries under a count of 4 | the block renders below the `Running author-time rules per package (N)...` step line, and gains the per-package entry | So the only rendered byte this moves is the one the card exists to move. #18769 held `os build`'s text output byte-identical on purpose; that hold is honoured everywhere except the defect itself. Shrinking the count instead would have made the text face report 3 while its own `--json` and `os validate` both report 4 — the false-clean direction #11529 named one list over, and it would have needed a second binding to count a rendering rather than a set. ## The pin, and the two controls `packages/cli/test/build-text-face-advisory-count.test.ts` asserts an **equality read from one run**, not a number: the integer in the summary line, the count of entries rendered above it, and the length of `--json`'s `warnings`. A fixture that raises a different number of advisories keeps passing; a face that counts a set it did not print cannot. Both directions were run, each from a committed tree, each with the mutation proven on disk and the restore proven clean: - **fails before** — `compile.ts` reverted to its pre-fix blob, pin unchanged: `Tests 2 failed | 3 passed`, on `summary said 3; rendered list: … expected 2 to be 3` and on `this per-package finding rides --json and the text face never prints it`. - **passes after** — at HEAD: `Tests 5 passed`. - **the lit control can fail, and fails on the condition it names** — strip `packages[]` out of the fixture and the equality assertions all stay GREEN while `the fixture reaches the per-package pass and raises a survivor there` goes RED: `1 failed | 4 passed`. That is exactly the vacuous pass the control exists to refuse. ## What was rewritten in the pre-existing diff 1. **A raw ESC byte in the pin.** `stripAnsi` carried a literal 0x1B inside its regex literal — the class `check:nul-bytes` rejects, invisible in every reader. Rewritten as an escape; byte-identical at runtime. 2. **A red that predated this branch.** The new once-guard is a call site in `compile.ts`, and `validate-build-gate-parity.test.ts` has held a CLOSED roster since #18491 — every bare-identifier call site must land in exactly one ledger. It was in none, so that file failed twice (`every call site … is classified`, and the parity gap derived from the same set). The roster and the assertion are both present at this branch's merge-base and the name is absent there, so the red was carried by the two commits this branch started from, not introduced by merging `main`. Classified as `NOT_A_GATE` under the presentation reason, with the argument written next to it: the guard decides WHEN `printAuthoringAdvisories` is called and nothing else, and `validate.ts` has nothing to wire because it has no "see above" sentence. 3. **A changeset sentence stricter than its own measurement.** It claimed the single-package captures "differ only in the `Build complete (Nms)` timer". Re-measured: they differ in **two** clocks — `Load time: Nms` as well. Both are clocks, so the claim survives; the sentence now says what was actually measured. ## What was verified and kept - **Every exit between step 3b and the summary flushes.** Enumerated: the union-failure text branch, the per-package-failure text branch, the continuing path, and the catch-all. The `--json` branches return early through the guard, and `isExitSignal` re-throws ahead of the catch's flush so no face double-prints. - **`^ {4}rule: ` really does tell an advisory from an error.** `printAuthoringAdvisories` closes each entry with a four-space line; `printAuthoringRuleErrors` and `printDocIssueErrors` both indent theirs by six. - **The `--json` leg's `where`-typeof filter isolates `ruleAdvisories` exactly.** None of the other five members of `warningsSoFar()` carries a `where` key — `DocIssue`, `NavContributionGroupDiagnostic`, `PermissionSetNameCollisionDiagnostic`, the capability-provider pairs and the plain-string undeclared-key list were each read. So the third assertion is sound in general, not only on this fixture. ## Changeset level, measured on the built `dist` ⛔ Not inferred from `files[]` and not from the file's look: - positive control — `author-time warning(s) — see above` is present in **1** published file, `packages/cli/dist/commands/compile.js`; - negative control — a token nothing in the tree carries is present in **0**; - `dist/commands/compile.js` sha256 `4999075…` with `compile.ts` at its pre-fix blob, `f1bbb59…` with the fix — **published bytes move**; - restoring and rebuilding a third time reproduces `f1bbb59…` exactly, which is what makes the middle reading a measurement rather than build noise. `@objectstack/cli` is public and versioned and ships `dist`, so a changeset is owed. `patch`: a bug fix in a released package, `Clause-②: no` — no schema key, no closed-set member, no published export, no registry entry, and no payload key, exit code or `--json` byte moves. `check-widening-tells --declaration no` over this diff reports no file on any declared surface (3 NOT MEASURED, 0 tells). ## Verification run on this branch `origin/main` is merged in (not rebased); both pre-existing commits are still ancestors, verified by `git merge-base --is-ancestor` exit 0 on each — a positive reading, which is self-proving in any checkout. - `pnpm --filter @objectstack/cli exec vitest run --project unit` — **214 files, 3048 tests, 0 failed** - `pnpm --filter @objectstack/cli exec vitest run --project integration` — **51 files, 427 tests, 0 failed** - the six nightly-tier `os build` pins, run under `OS_TEST_TIERS=nightly` because the queue population excludes them by name — `build-json-advisory-parity`, `build-json-undeclared-key-parity`, `build-json-failure-warnings`, `build-multi-package-artifact`, `compile-artifact-packages`, `build-docs-step-count`: **6 files, 41 tests, 0 failed** - `pnpm --filter @objectstack/cli typecheck` — exit 0. Note `tsconfig.json` includes `src` only, so the test layer is judged by the `check:test-typecheck` half of that script, which reports the layer compiling under `tsconfig.test.json`. - `pnpm --filter '@objectstack/cli^...' build` — exit 0 (the dependency closure). - the gate families `scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derives from this branch's own change set, each exit code captured before any pipe. ⚠️ Two earlier readings were contention artefacts and are recorded as such rather than as failures: four `scaffold-emission-typechecks` cases reddened in a run that was competing with a gate sweep and was then killed at a timeout, and both pass in the clean full run above. ## Acceptance notes - `check:type-check-debt` was killed by the OOM killer (exit 137 inside its full-repo build, gate exit 3) during a contended sweep. Its own failure text says that is **not** a pass and **not** a finding — nothing was measured. It is re-run alone in the final sweep and the reading is in the report. - Noted, not filed: `printDocIssueErrors` and `printAuthoringRuleErrors` render identical six-space `rule:` lines, so a text-face assertion that keys on that indent alone cannot tell a doc error from a rule error. Nothing in this PR depends on it, no open PR is heading for that file, and there is no carrier — recorded here rather than as a card. --- _Generated by [Claude Code](https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent be7aeb8 commit c7dc089

4 files changed

Lines changed: 398 additions & 8 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
`os build`'s text face prints every author-time advisory its own summary line counts — the closing `N author-time warning(s) — see above` no longer stands over a shorter list (#18780).
6+
7+
Clause-②: no
8+
9+
`compile.ts` rendered the advisory block at step 3b, inline, straight off the union rule run. Step 3b-ii — the ADR-0130 D4 pass that runs the same rule table once per `packages[]` entry — then appended its survivors to the **same** `ruleAdvisories` binding, and the summary line at the foot of the command counts that binding. So on a multi-package project the count was the complete set and the printed list was the union's alone, and the sentence pointing at it sent the reader back up to find a warning that had never been printed.
10+
11+
Measured at 17.4.0 on `examples/app-multi-package`, exit 0 on every face:
12+
13+
```
14+
os build 3 advisory entries · ⚠ 4 author-time warning(s) — see above
15+
os build --json warnings: 4 <- the count was already right
16+
os validate 4 advisory entries <- since #18769
17+
```
18+
19+
- **The list moves, not the count.** #11529 settled this axis one list over: the summary counts the whole set and the printer NAMES what it withheld, because a count quietly shrunk to match a short list is the false-clean direction — it deletes a finding from the text face of the command that ships while `--json` and `os validate` keep reporting it. The fourth advisory now prints.
20+
- **What an author sees change**: on a stack that declares `packages[]`, the advisory block is rendered after the `Running author-time rules per package (N)...` step line instead of before it, and it now carries the per-package findings — the ones whose `where` reads `package '<id>' — …`. A stack with no `packages[]` is unchanged — measured on a single-package fixture, the before/after captures are 2038 bytes each and differ only in the run's two clocks, `Load time: Nms` and `Build complete (Nms)`: its list was already complete, and the block still precedes every later step line.
21+
- **Still ONE printer call.** The block is deferred to the point where the list is complete rather than printed twice, so the 50-entry cap and its `… and N more … not shown` notice keep judging one list. A second `printAuthoringAdvisories` for the survivors alone would have given the cap a second budget and the notice a second, partial total.
22+
- **The author-time rule FAILURE faces keep their advisories.** A union-level failure exits before the per-package pass runs, so its block is byte-for-byte what it was; the per-package failure face now prints the per-package advisories too, which its own `--json` twin has published since #11772.
23+
24+
No payload key, no exit code and no `--json` byte moves: `warnings` already carried all four, which is how the mismatch was measurable in the first place.

‎packages/cli/src/commands/compile.ts‎

Lines changed: 65 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,48 @@ export default class Compile extends Command {
172172
...navGroupWarnings,
173173
...permissionSetCollisionWarnings,
174174
];
175+
// [#18780] ONE rendering of the author-time advisory block, from the
176+
// COMPLETE list — hoisted here for the same reason the lists above are.
177+
//
178+
// The block used to be printed inline at step 3b, BEFORE step 3b-ii
179+
// appended the per-package survivors to `ruleAdvisories`. So on a
180+
// multi-package stack this command's summary line counted a set strictly
181+
// larger than the one it pointed at. Measured on
182+
// `examples/app-multi-package` at 17.4.0: `⚠ 4 author-time warning(s) —
183+
// see above` standing over a list of THREE, while `--json` carried all
184+
// four and `os validate` — which renders its advisory list once, at the
185+
// end, after the same per-package append — printed all four (#18769). The
186+
// reader is sent back up to find a warning that was never printed, and the
187+
// direction reads as "I must have missed it".
188+
//
189+
// ⛔ THE COUNT IS NOT THE SIDE THAT MOVES. #11529 settled that axis one
190+
// list over: the summary counts the whole set and the PRINTER names what
191+
// it withheld, because a count quietly shrunk to match a short list is the
192+
// false-clean direction — it deletes a finding from the text face of the
193+
// command that ships, while `--json` and `os validate` keep reporting it.
194+
// So the list grows to the count.
195+
//
196+
// ⛔ AND IT STAYS ONE PRINTER CALL. A second `printAuthoringAdvisories`
197+
// for the survivors alone would hand the 50-entry cap a second budget and
198+
// its truncation notice a second, partial total — two locally-honest
199+
// notices for one list, which is #11529's defect wearing its own fix.
200+
//
201+
// Deferring the call is what the guard below is for: every text face that
202+
// used to be DOWNSTREAM of the old inline site flushes the block itself,
203+
// so both author-time rule failures still print their advisories ahead of
204+
// their error list, and the catch-all still prints them when a rule throws
205+
// inside the per-package pass — the one window between the two sites.
206+
let advisoriesPrinted = false;
207+
const printAdvisoriesOnce = (): void => {
208+
if (advisoriesPrinted || flags.json || ruleAdvisories.length === 0) return;
209+
advisoriesPrinted = true;
210+
console.log('');
211+
// #11529 — rendered by ONE printer, which also names the remainder when
212+
// the list is cut. The loop used to sit inline here and stop dead at 50
213+
// with no notice, so a truncated report read exactly like a complete
214+
// one. See `printAuthoringAdvisories` for the measurement.
215+
printAuthoringAdvisories(ruleAdvisories);
216+
};
175217
// [#12125] The ADR-0087 D2 conversion notices, hoisted for the SAME reason
176218
// and under the SAME ruling as the four lists above — one field over. The
177219
// notices were computed at step 2 (below) and reached the terminal SUCCESS
@@ -366,14 +408,6 @@ export default class Compile extends Command {
366408
const { errors: ruleErrors, advisories } = splitBySeverity(findings);
367409
ruleAdvisories = advisories;
368410

369-
if (ruleAdvisories.length > 0 && !flags.json) {
370-
console.log('');
371-
// #11529 — rendered by ONE printer, which also names the remainder when
372-
// the list is cut. The loop used to sit inline here and stop dead at 50
373-
// with no notice, so a truncated report read exactly like a complete
374-
// one. See `printAuthoringAdvisories` for the measurement.
375-
printAuthoringAdvisories(ruleAdvisories);
376-
}
377411
if (ruleErrors.length > 0) {
378412
// Every failing rule reports at once — see the note in `validate.ts`.
379413
if (flags.json) {
@@ -384,6 +418,11 @@ export default class Compile extends Command {
384418
);
385419
this.exit(1);
386420
}
421+
// [#18780] This exit is UPSTREAM of the per-package append below — a
422+
// union-level `error` refuses before that pass runs — so the list
423+
// flushed here is the union's alone, byte-for-byte what this face
424+
// printed when the call sat inline above.
425+
printAdvisoriesOnce();
387426
console.log('');
388427
printError(`Author-time rules failed (${ruleErrors.length} issue${ruleErrors.length > 1 ? 's' : ''})`);
389428
// [#11642] `--json` on this same exit publishes every one of them as
@@ -465,6 +504,10 @@ export default class Compile extends Command {
465504
);
466505
this.exit(1);
467506
}
507+
// [#18780] Downstream of the append, so this flush carries the
508+
// per-package advisories too — the same list `warningsSoFar()` has
509+
// published on this exit's `--json` twin since #11772.
510+
printAdvisoriesOnce();
468511
console.log('');
469512
printError(
470513
`Author-time rules failed inside the artifact's packages (${perPackageErrors.length} issue${perPackageErrors.length > 1 ? 's' : ''})`,
@@ -473,6 +516,12 @@ export default class Compile extends Command {
473516
this.exit(1);
474517
}
475518
}
519+
// [#18780] The continuing path — and the only one the summary line at
520+
// the foot of this command is reachable from. `ruleAdvisories` is
521+
// complete here on BOTH shapes: a stack with `packages[]` has just had
522+
// the survivors appended, and one without skips the block entirely and
523+
// arrives with the union list the summary already counted.
524+
printAdvisoriesOnce();
476525

477526
// 3b-bis. [#14553] Navigation contributions whose `group` names no group
478527
// in the target app. RUNS ON EVERY BUILD, artifact or not — the block
@@ -946,6 +995,14 @@ export default class Compile extends Command {
946995
await emitJson({ success: false, error: error.message, ...errorCodeFields(error), warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true });
947996
this.exit(1);
948997
}
998+
// [#18780] The one window the three flushes above do not cover: a throw
999+
// between step 3b's split and step 3b-ii's append — a rule throwing
1000+
// inside the per-package pass. The inline call this replaced had already
1001+
// rendered the union list by then, so flushing here keeps that path's
1002+
// output rather than shortening it. Every other throw on this face is
1003+
// downstream of a flush and the guard makes this a no-op; a throw
1004+
// upstream of step 3b finds the list empty and renders nothing.
1005+
printAdvisoriesOnce();
9491006
// [#15547] `resolveConfigPath()` already wrote its refusal and hint lines
9501007
// to stderr before throwing, so this face has nothing left to render —
9511008
// and `this.error()` below is NOT a no-op for it: it re-renders the same

0 commit comments

Comments
 (0)