fix(cli): os lint runs the per-package author-time rule pass the other two doors already ran

.changeset/18778-lint-per-package-authoring-pass.md

@@ -0,0 +1,38 @@
+---
+'@objectstack/cli': minor
+---
+
+`os lint` runs the per-package author-time rule pass the other two doors already ran
+
+`os build` has run the author-time rule table a second time, once per
+`packages[]` entry with that package's body as the stack and the artifact's own
+`packages[]` as resolution context, since #16611; `os validate` joined it in
+#18677. `os lint` ran the union fold and stopped, so every finding that pass
+produces — "exactly the set the union could not see", in the build command's own
+words — was reported by the command that ships and invisible on the fastest of
+the three doors. All three now call the one shared pass.
+
+Measured on a two-package project whose union run is clean and whose per-package
+run is not (one package owns an object, a sibling package owns the view that
+displays its field):
+
+| | before | after |
+|---|---|---|
+| `os build --json` | warnings 1 | warnings 1 |
+| `os lint --json` | total 0, exit 0 | total 1, exit 0 |
+| `os lint --json --strict` | exit 0 | exit 1 |
+
+**BREAKING** — `os lint --strict` can now fail a project it passed before. A
+per-package finding is a finding this door could not see, `--strict` is
+documented as "treat warnings as errors", and the verdict moves with it. The
+default face is unchanged in the measurement above, and the severity mapping is
+`os lint`'s own: an `error` fails the run, a `warning` fails it only under
+`--strict`, an `info` stays a suggestion. Nothing is refused here that `os build`
+does not already refuse, so the pre-flight is narrowed to the bar the command
+that ships already holds and never past it. A run that must keep its old verdict
+drops `--strict`; a project that wants to keep it fixes what the pass reports,
+which is the same thing `os build` has been reporting all along.
+
+Clause-②: yes (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) Nothing an author writes changes: no spec key, export, config field or payload key is removed, renamed or added. What moved is which stacks one CLI command's existing rule table is run over, so `objectstack migrate meta` has nothing to rewrite and the ledger has nothing to record. -->

packages/cli/src/commands/lint.ts

@@ -15,7 +15,11 @@ import { scoreMetadata } from '../lint/score.js';
 import { checkHookBodyLowering } from '../lint/hook-body-lowering.js';
 import { lowerCallables } from '../utils/lower-callables.js';
 import { authoringRuleUnionStack } from '../utils/stack-collections.js';
-import { artifactPackages, packageBodyAsStack } from '../utils/artifact-packages.js';
+import {
+  artifactPackages,
+  packageBodyAsStack,
+  runPerPackageAuthoringRules,
+} from '../utils/artifact-packages.js';
 import { runMetadataEval } from '../lint/metadata-eval.js';
 import { DEFAULT_METADATA_EVAL_CORPUS } from '../lint/corpus.js';
 import {
@@ -666,15 +670,73 @@ export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue
   // so `lowered` already carries the folded collections and re-folding it here
   // would be a second call that could only ever return by identity.
   const { lowered, loweredHookRefs } = lowerCallables(stack as Record<string, unknown>);
-  for (const f of runAuthoringRules('lint', {
+  const unionFindings = runAuthoringRules('lint', {
     normalized: stack,
     parsed: lowered,
     sduiManifest: opts.sduiManifest,
     // [#16546] Same ref set `os build` computes from the same normalized
     // input — what lets `validateReadonlyHookWrites` / `validateHookBodyWrites`
     // report `hooks[i].handler` here byte-identically to `os build`.
     loweredHookRefs,
-  })) {
+  });
+
+  // ── The SAME rule table, once per PACKAGE (ADR-0130 D4, #18778) ──
+  //
+  // The second half of the run above, and the half THIS door ran without.
+  // `os build` has run it since #16611 and `os validate` since #18677; `os
+  // lint` ran the union fold and stopped. `compile.ts` step 3b-ii says what
+  // survives the de-duplication is "exactly the set the union could not see" ⇒
+  // that whole set was findings `os build` reported and this command
+  // structurally could not.
+  //
+  // ⚠️ The reading that hid it for two cards is the one the imports above
+  // invite: this file DOES call `artifactPackages` and `packageBodyAsStack` —
+  // for the intra-package duplicate-name advisory (#17821), which is `os
+  // lint`'s OWN rubric and not the shared table. ⛔ A count is not a reading.
+  //
+  // ⛔ Not a second copy of the loop. This file already runs ONE per-package
+  // walk of its own (the advisory above), so "write the loop here, it is
+  // already the shape" is the live temptation at this door specifically — and
+  // it is the one `utils/artifact-packages.ts`' header forbids by name: what
+  // drifts between two hand-written loops is the VERDICT (the de-duplication
+  // key, the severity split, the `where` prefix), not the package reading.
+  //
+  // Settled from the repo's own statements, ⛔ not assumed: `authoring-rules.ts`
+  // calls the three commands "three doors in ONE wall" and holds a gate to its
+  // weakest door; the union fold landed HERE (#17069/#17528) for this exact
+  // false-clean direction, in this file's own words — "the whole table reported
+  // nothing and `os lint` returned no finding of any severity for a project
+  // `os build` refuses"; and the pre-registry hand-wired subset was removed
+  // because "a pre-flight that disagrees with the gate in both directions is
+  // worse than no pre-flight". This is that same sentence, on the INPUT.
+  //
+  // The severity face is `os lint`'s own and is unchanged: a per-package
+  // finding is mapped by the same expression the union findings are, so an
+  // `error` fails the run, a `warning` fails it under `--strict` and an `info`
+  // stays a suggestion. ⛔ No severity judgement is made here — a per-package
+  // `error` is one `os build` ALREADY refuses, so this narrows `os lint` to the
+  // bar the command that ships holds, never past it.
+  //
+  // Skipped entirely for a stack with no `packages[]` (`packageCount` 0): one
+  // package by definition, already judged whole by the union run above.
+  const perPackageFindings = runPerPackageAuthoringRules({
+    command: 'lint',
+    // The LOWERED view, exactly as the `parsed` tier above is handed it and as
+    // both other doors hand their parse: `lowerCallables` re-maps
+    // `packages[*].manifest`, so this is the same per-package body `os build`
+    // walks. ⛔ Not `stack` — that would judge un-lowered package bodies here
+    // and lowered ones there, which is #16095 one layer in.
+    parsed: lowered,
+    // De-duplicated against the run above, on the UNPREFIXED finding, so what
+    // reaches the list below is the set the union could not see.
+    unionFindings,
+    sduiManifest: opts.sduiManifest,
+    loweredHookRefs,
+  }).findings;
+
+  // ⛔ ONE mapping for both halves. A second copy of this expression is how one
+  // list comes to render `info` as `suggestion` and the other does not.
+  for (const f of [...unionFindings, ...perPackageFindings]) {
     issues.push({
       severity: f.severity === 'info' ? 'suggestion' : f.severity,
       rule: f.rule,

packages/cli/src/utils/artifact-packages.ts

@@ -29,6 +29,14 @@
  * {@link runPerPackageAuthoringRules} — for the same reason one layer out: the
  * `os build` door ran it and the `os validate` door did not, and a second copy
  * of the loop is how that asymmetry would come back.
+ *
+ * ⚠️ #18778 measured that the asymmetry was 2 of 3 doors, not 1 of 2: `os lint`
+ * ran the union fold and stopped as well, and the reading that hid it is the
+ * one this module's own header invites — `lint.ts` DOES import both seams
+ * above, for its intra-package duplicate-name advisory (#17821), so a sweep
+ * that scores a door by symbol presence scores it as covered. ⛔ A count is not
+ * a reading: what matters is which loop the symbols feed. All three doors call
+ * the pass now.
  */
 
 import {
@@ -132,20 +140,23 @@ export function packageBodyAsStack(
 
 /**
  * The author-time rule table, run ONCE PER PACKAGE and de-duplicated against a
- * union run — the pass `os build` has run since #16611 and `os validate` did
- * not (#18677).
+ * union run — the pass `os build` has run since #16611, `os validate` did not
+ * (#18677) and `os lint` did not (#18778).
  *
- * ## Why it lives here and not in one of the two commands
+ * ## Why it lives here and not in one of the commands
  *
  * It is the THIRD entry to owe the shape the module header describes, and the
  * header's fence binds it: the only ways to reach `compile.ts`' loop from
- * `validate.ts` are to import one oclif command from another — pulling the
- * lowerer and the docs sweep into every `os validate` invocation — or to write
- * a second copy. ⛔ The second copy is what must not happen, and here it would
- * not be the `{index,id,body}` reading that drifted but the VERDICT: two loops
- * choosing their own de-duplication key, their own severity split or their own
- * `where` prefix is how one door comes to report a different set from the other
- * while both look right. That is the defect #18677 is, one layer down.
+ * another command are to import one oclif command from another — pulling the
+ * lowerer and the docs sweep into every `os validate` / `os lint` invocation —
+ * or to write a second copy. ⛔ The second copy is what must not happen, and
+ * here it would not be the `{index,id,body}` reading that drifted but the
+ * VERDICT: two loops choosing their own de-duplication key, their own severity
+ * split or their own `where` prefix is how one door comes to report a
+ * different set from another while both look right. That is the defect #18677
+ * is, one layer down — and #18778 is the measurement that the fence held: the
+ * third door reached the SAME pass, and nothing about the pass moved but the
+ * shape it hands back.
  *
  * ## What the asymmetry was, measured
  *
@@ -187,14 +198,30 @@ export function runPerPackageAuthoringRules(run: {
 }): {
   /** How many package entries were walked — 0 means the pass did not run. */
   packageCount: number;
+  /**
+   * Every surviving finding, `where`-prefixed, in walk order — the SAME set as
+   * `errors` ∪ `advisories`, not a second computation of it (#18778).
+   *
+   * The two doors that hold an ARTIFACT to the bar need the severity SPLIT:
+   * an `error` refuses the run and an advisory rides the warnings list, so
+   * `compile.ts` and `validate.ts` read the two arrays below. `os lint` has no
+   * such split — it maps every finding of every severity onto ONE `issues`
+   * list through its own `info` → `suggestion` face and lets `--strict` decide
+   * what fails — so re-joining the halves at that door would put all errors
+   * before all advisories and silently re-order a list the union run above it
+   * produces in rule order. This member is that door's shape, produced by the
+   * one loop rather than by a caller stitching the halves back together.
+   */
+  findings: AuthoringFinding[];
   errors: Array<{ package: string } & AuthoringFinding>;
   advisories: AuthoringFinding[];
 } {
   const artifactPackageEntries = run.parsed.packages;
   const packageEntries = artifactPackages(run.parsed);
+  const findings: AuthoringFinding[] = [];
   const errors: Array<{ package: string } & AuthoringFinding> = [];
   const advisories: AuthoringFinding[] = [];
-  if (packageEntries.length === 0) return { packageCount: 0, errors, advisories };
+  if (packageEntries.length === 0) return { packageCount: 0, findings, errors, advisories };
 
   const alreadyReported = new Set(run.unionFindings.map(findingKey));
   for (const pkg of packageEntries) {
@@ -206,13 +233,19 @@ export function runPerPackageAuthoringRules(run: {
       loweredHookRefs: run.loweredHookRefs,
     }).filter((f) => !alreadyReported.has(findingKey(f)));
     for (const f of pkgFindings) alreadyReported.add(findingKey(f));
+    // ⛔ ONE prefixer, applied to every member of all three lists. The `where`
+    // prefix is the only thing that identifies a finding as per-package on any
+    // door's face, and a second spelling of it is the drift this module exists
+    // to foreclose — one layer smaller than the second copy of the loop its
+    // header forbids, and invisible in exactly the same way.
+    const prefixed = (f: AuthoringFinding): AuthoringFinding => ({
+      ...f,
+      where: `package '${pkg.id}' — ${f.where}`,
+    });
+    findings.push(...pkgFindings.map(prefixed));
     const split = splitBySeverity(pkgFindings);
-    advisories.push(
-      ...split.advisories.map((a) => ({ ...a, where: `package '${pkg.id}' — ${a.where}` })),
-    );
-    errors.push(
-      ...split.errors.map((e) => ({ ...e, package: pkg.id, where: `package '${pkg.id}' — ${e.where}` })),
-    );
+    advisories.push(...split.advisories.map(prefixed));
+    errors.push(...split.errors.map((e) => ({ ...prefixed(e), package: pkg.id })));
   }
-  return { packageCount: packageEntries.length, errors, advisories };
+  return { packageCount: packageEntries.length, findings, errors, advisories };
 }