fix(cli): os lint's hand-written checks and scoreMetadata judge the stack an option-B project declares, instead of passing a stack they never opened (#17528)

.changeset/17528-lint-handwritten-checks-package-fold.md

@@ -0,0 +1,60 @@
+---
+'@objectstack/cli': patch
+---
+
+`os lint`'s own rubric and `os lint --score` judge the stack an ADR-0130 D4 / option-B project actually declares, instead of reporting `✓ All checks passed` on a stack they never opened
+
+`lintConfig` runs two families: the shared author-time rule registry and
+`os lint`'s **own** hand-written checks — naming, labels, empty field maps, the
+intra-package duplicate advisory, hook-body lowering and the data-model
+conventions. The registry learned to resolve `packages[]` earlier; the
+hand-written family and `scoreMetadata`, which reaches the same function, still
+read the **top level only**. On an option-B project (every definition inside
+`packages[]`, none flattened up) they were handed an empty stack.
+
+Measured through the real binary, on one object authored two ways — the same
+metadata, differing only in where it is declared:
+
+```
+packages[]   os lint            exit 0   ✓ All checks passed
+                                         Metadata quality: 100/100  (A)
+
+top level    os lint            exit 0   ⚠ Label "order" should start with an uppercase letter
+                                           convention/label-case        at objects[0].label
+                                         ℹ Object "ob_order" has no nameField and no name-like field …
+                                           object/missing-name-field    at objects[0].fields
+                                         Metadata quality: 96/100  (A)
+```
+
+and after, on the same two projects:
+
+```
+packages[]   os lint            exit 0   ⚠ convention/label-case        at objects[0].label
+                                         ℹ object/missing-name-field    at objects[0].fields
+                                         Metadata quality: 96/100  (A)
+
+top level    os lint            exit 0   — byte-identical to before
+```
+
+The score is the sharper half. `100/100 (A)` with every count at zero is
+byte-for-byte the verdict a genuinely clean project gets, on a rubric that had
+judged nothing — the same indistinguishability a swallowed linter crash used to
+produce, arriving through the input instead.
+
+**The fix folds once, at `lintConfig`'s entry, with the existing helper.**
+`authoringRuleUnionStack` (`utils/stack-collections.ts`) is this package's one
+resolution rule for a package-owned collection and it is present-wins: a key the
+top level already carries wins, because in today's additive shape that array
+already *is* the union. So a multi-package artifact is judged once, never twice,
+and a stack whose top level carries its collections — every stack the platform
+emits today — is returned by identity and lints byte-identically to before.
+
+**This does not change what `scoreMetadata` scores.** It already scored the whole
+project: its schema half reports `packages.0.manifest.objects.0: …` on an
+option-B stack with no fold anywhere, and on today's additive multi-package shape
+its lint half already read the flattened union across every package. The fold
+makes the option-B shape agree with the additive one.
+
+⛔ No authorable key, spec schema, published export or accept set moves.
+`os build` rejects and accepts exactly what it did; `os lint`'s own `error`
+severity remains a lint verdict, not a publish gate.

packages/cli/src/commands/lint.ts

@@ -174,19 +174,75 @@ export interface LintConfigOptions {
    * JSX gate does full component/prop validation; absent, it stays parse-level.
    * The `os lint` command resolves it; `scoreMetadata` deliberately does not —
    * the scorer is a pure function of a stack and must not read the filesystem.
+   *
+   * ⚠️ That sentence is about the FILESYSTEM and about nothing else. It is not
+   * a decision about WHICH stack the scorer judges: the stack it is handed is
+   * still resolved to the collections the author declared, in either ADR-0130
+   * D4 shape, by the fold at `lintConfig`'s entry below. ⛔ Do not read it as a
+   * warrant for the scorer seeing an unfolded stack.
    */
   sduiManifest?: unknown;
 }
 
+/**
+ * `os lint`'s whole rubric: the hand-written checks below plus the shared
+ * author-time rule registry (#4409), over the stack the author DECLARED.
+ *
+ * ## Why the fold is here, at the entry, and not at the registry call alone
+ *
+ * #17069 taught the registry call inside this function to resolve `packages[]`
+ * and scoped itself the way `compile.ts` scopes it, writing down exactly what
+ * it left behind: "the hand-written checks above and `scoreMetadata` (which
+ * reaches `lintConfig` through `lint/score.js`) keep reading the caller's own
+ * stack". This is the other half of that staged change, not a reversal of it.
+ *
+ * The half it left was the sharper one. Under ADR-0130 D4 / option B every
+ * definition lives in `packages[]` and the top level carries none, so the
+ * hand-written family read an EMPTY stack: a project with a lower-case object
+ * label and no name field got `✓ All checks passed`, while the byte-identical
+ * top-level spelling of the same metadata got `convention/label-case` and
+ * `object/missing-name-field`. `scoreMetadata` reaches this same function, so
+ * an option-B project's metadata-quality rubric was computed over nothing and
+ * published `100/100 (A)` — the #15658 shape ("the linter found nothing" and
+ * "the linter never ran" collapsed into the better-looking one) arriving
+ * through the INPUT this time rather than through a swallowed crash.
+ *
+ * ## Why {@link authoringRuleUnionStack} and not a second fold
+ *
+ * `stack-collections.ts` is the ONE place this package resolves a
+ * package-owned collection, and its rule is present-wins: a key the top level
+ * already carries wins, because in today's additive shape that array already
+ * IS the union. A stack that carries its collections comes back BY IDENTITY,
+ * so every single-package project lints byte-identically to before. A second
+ * fold written here could not have that property without re-deriving it, and
+ * two folds that disagree is a worse defect than the one being removed.
+ *
+ * ## What this does NOT decide
+ *
+ * It does not choose between a per-project and a per-package metadata score.
+ * `scoreMetadata` already scores the whole project and always has: its schema
+ * half parses the artifact whole and reports `packages.0.manifest.objects.0:
+ * …` on an option-B stack today, with no fold anywhere, and on today's
+ * additive multi-package shape its lint half already reads the flattened union
+ * across every package. This fold makes the option-B shape agree with the
+ * additive one — the same thing computed, over a stack that is no longer
+ * empty — and raises no new scoping question.
+ */
 export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue[] {
+  // The stack as DECLARED, whichever ADR-0130 D4 shape it arrived in. ⛔ Never
+  // read `config.KEY` below this line — a check that skipped the fold is
+  // exactly the defect #17528 removed, and it would be invisible in every
+  // other check, because the run still exits 0 and still prints a score.
+  const stack: any = authoringRuleUnionStack(config as Record<string, unknown>);
+
   const issues: LintIssue[] = [];
 
   const push = (issue: LintIssue | null) => {
     if (issue) issues.push(issue);
   };
 
   // ── Objects ──
-  const objects: any[] = Array.isArray(config.objects) ? config.objects : [];
+  const objects: any[] = Array.isArray(stack.objects) ? stack.objects : [];
 
   for (let i = 0; i < objects.length; i++) {
     const obj = objects[i];
@@ -244,7 +300,7 @@ export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue
   }
 
   // ── Views ──
-  const views: any[] = Array.isArray(config.views) ? config.views : [];
+  const views: any[] = Array.isArray(stack.views) ? stack.views : [];
   for (let i = 0; i < views.length; i++) {
     const view = views[i];
     const viewPath = `views[${i}]`;
@@ -259,7 +315,7 @@ export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue
   }
 
   // ── Apps ──
-  const apps: any[] = Array.isArray(config.apps) ? config.apps : [];
+  const apps: any[] = Array.isArray(stack.apps) ? stack.apps : [];
   for (let i = 0; i < apps.length; i++) {
     const app = apps[i];
     const appPath = `apps[${i}]`;
@@ -273,7 +329,7 @@ export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue
   }
 
   // ── Flows ──
-  const flows: any[] = Array.isArray(config.flows) ? config.flows : [];
+  const flows: any[] = Array.isArray(stack.flows) ? stack.flows : [];
   for (let i = 0; i < flows.length; i++) {
     const flow = flows[i];
     const flowPath = `flows[${i}]`;
@@ -283,7 +339,7 @@ export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue
   }
 
   // ── Agents ──
-  const agents: any[] = Array.isArray(config.agents) ? config.agents : [];
+  const agents: any[] = Array.isArray(stack.agents) ? stack.agents : [];
   for (let i = 0; i < agents.length; i++) {
     const agent = agents[i];
     const agentPath = `agents[${i}]`;
@@ -309,7 +365,7 @@ export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue
   //
   // Objects are already prefix-*enforced* (error) in defineStack; views are
   // object-derived; `doc` has its own build lint — so they are excluded here.
-  const ns: string | undefined = config.manifest?.namespace;
+  const ns: string | undefined = stack.manifest?.namespace;
 
   // Bare-named UI/automation types that share the generic registry namespace.
   // Data-driven so a new bare-named type is one line.
@@ -369,7 +425,7 @@ export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue
   ];
 
   for (const { key, label, registryKey } of PREFIXED_TYPES) {
-    const items: any[] = Array.isArray(config[key]) ? config[key] : [];
+    const items: any[] = Array.isArray(stack[key]) ? stack[key] : [];
     // First occurrence of each registry key → its index, so a later duplicate
     // can point back at the original declaration.
     const firstSeen = new Map<string, number>();
@@ -427,7 +483,7 @@ export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue
   // Scoped to configs that declare a manifest — a bare metadata fragment (no
   // package identity) has nowhere to hang an engines range.
   {
-    const manifest = config.manifest as Record<string, any> | undefined;
+    const manifest = stack.manifest as Record<string, any> | undefined;
     const hasRange =
       typeof manifest?.engines?.protocol === 'string' ||
       typeof manifest?.engines?.platform === 'string' ||
@@ -456,7 +512,7 @@ export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue
   //
   // Reads FUNCTION values, so it must run on the normalized input before any
   // Zod parse — which is where `lintConfig` already sits.
-  issues.push(...checkHookBodyLowering(config as Record<string, unknown>));
+  issues.push(...checkHookBodyLowering(stack as Record<string, unknown>));
 
   // ── Data-model best practices (relationships / master-detail / roll-ups) ──
   // Cross-object rules that encode the conventions in ADR-0035 and the
@@ -523,14 +579,19 @@ export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue
   // a stack that still carries them comes back BY IDENTITY and every
   // single-package project lints exactly as before.
   //
-  // Scoped to this call, as it is in `compile.ts`: the hand-written checks
-  // above and `scoreMetadata` (which reaches `lintConfig` through
-  // `lint/score.js`) keep reading the caller's own stack, so nothing about
-  // what this function returns for a top-level stack moves.
-  const { lowered, loweredHookRefs } = lowerCallables(config as Record<string, unknown>);
+  // ⭐ #17069 scoped its fold to THIS CALL, as `compile.ts` scopes its own, and
+  // named the two things it deliberately left reading the caller's own stack:
+  // "the hand-written checks above and `scoreMetadata`". #17528 is the other
+  // half of that staged change — the fold now happens ONCE, at this function's
+  // entry, so every check in it and every caller of it (the `os lint` command
+  // and `scoreMetadata`) judge one stack. ⛔ There is still exactly one fold
+  // and one helper: `lowerCallables` shallow-clones the top level it is handed,
+  // 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', {
-    normalized: authoringRuleUnionStack(config as Record<string, unknown>),
-    parsed: authoringRuleUnionStack(lowered),
+    normalized: stack,
+    parsed: lowered,
     sduiManifest: opts.sduiManifest,
     // [#16546] Same ref set `os build` computes from the same normalized
     // input — what lets `validateReadonlyHookWrites` / `validateHookBodyWrites`

packages/cli/test/fixtures/option-b-collection-zoo.ts

@@ -115,6 +115,38 @@ export const PROBE_HOOK_OBJECT = 'probe_order';
 export const PROBE_SEED_DATASET = 'probe_account';
 export const PROBE_LOCALE = 'en';
 
+/**
+ * [#17528] The one deliberately LINT-DIRTY value in this fixture: a label that
+ * `os lint`'s HAND-WRITTEN family has something to say about.
+ *
+ * It is here for the same reason `PROBE_FEDERATED_DATASOURCE` is — "watching
+ * the map required carrying one". `lintConfig`'s own rubric (naming, labels,
+ * structure, data-model conventions) reports nothing at all about a CLEAN
+ * stack, so on this zoo as it stood the reader returned the identical four
+ * findings in BOTH shapes while reading an empty one under option B: every
+ * finding came from the shared rule registry, which #17069 had already taught
+ * to fold. A probe row counted over that return value would have been green
+ * before the fix and green after it — a row that cannot go red.
+ *
+ * So exactly one label is lower-cased, on the MODULE package's federated
+ * object, which makes the row a CROSS-PACKAGE resolution rather than a
+ * within-body read. Labels are display strings: nothing in this fixture, the
+ * probe or the pin keys on one, and `ObjectSchema` does not constrain their
+ * case — the whole effect is one `convention/label-case` warning.
+ *
+ * ⛔ Do not "fix" this label. Restoring its case deletes the only signal two
+ * probe rows have.
+ */
+export const PROBE_DIRTY_LABEL = 'probe federated order';
+/**
+ * [#17528] The hand-written `os lint` rule {@link PROBE_DIRTY_LABEL} provokes.
+ *
+ * Named here, beside the value that provokes it, so the provocation and the
+ * rule that observes it cannot drift apart. It is one of the two rules the
+ * card's own repro measured going missing on a `packages[]`-only project.
+ */
+export const PROBE_DIRTY_LABEL_RULE = 'convention/label-case';
+
 // ─── Package 1: the App ─────────────────────────────────────────────────────
 
 const coreStack = (): ObjectStackDefinition =>
@@ -241,7 +273,8 @@ const ordersStack = (): ObjectStackDefinition =>
       // makes the two collections separately observable through one function.
       {
         name: PROBE_FEDERATED_OBJECT,
-        label: 'Probe Federated Order',
+        // [#17528] Deliberately lower-case — see `PROBE_DIRTY_LABEL`.
+        label: PROBE_DIRTY_LABEL,
         pluralLabel: 'Probe Federated Orders',
         sharingModel: 'private',
         datasource: PROBE_FEDERATED_DATASOURCE,

packages/cli/test/fixtures/option-b-reader-probe.ts

@@ -81,6 +81,12 @@ import { lowerCallables } from '../../src/utils/lower-callables.js';
 // `lowerCallables` is: it ships inside `@objectstack/cli`, so the row below
 // measures the reader in this checkout rather than a built artifact.
 import { collectMetadataStats, type MetadataStats } from '../../src/utils/format.js';
+// [#17528] `os lint`'s OWN rubric, and the metadata-quality scorer that reaches
+// it through `lint/score.js`. Reached as SOURCE, by relative path, for the same
+// reason the two imports above are: both ship inside `@objectstack/cli`, so the
+// rows below measure the readers in this checkout, not a built artifact.
+import { lintConfig } from '../../src/commands/lint.js';
+import { scoreMetadata } from '../../src/lint/score.js';
 // [#15006] The CLI's OWN reads of a package-owned collection. Until that card
 // these were inline expressions inside oclif command bodies — no exported
 // reader, nothing a probe could call — which is why the four rows they carry
@@ -98,6 +104,7 @@ import {
 import {
   PACKAGE_OWNED_COLLECTION_KEYS,
   PROBE_DEFAULT_PERMISSION_SET,
+  PROBE_DIRTY_LABEL_RULE,
   PROBE_FEDERATED_OBJECT,
   PROBE_FUNCTION,
   PROBE_FUNCTION_EFFECT,
@@ -513,6 +520,57 @@ export async function measureShape(project: unknown, projectRoot: string): Promi
     summaryItems,
   ));
 
+  // [#17528] `os lint`'s OWN rubric — the hand-written checks at the top of
+  // `lintConfig` (naming, labels, structure, name-field suggestions). #17069
+  // folded the RULE REGISTRY inside that same function and scoped itself
+  // explicitly to that call, naming the two things it left reading the caller's
+  // own stack: this family, and `scoreMetadata`. Under option B both read an
+  // EMPTY stack, so a project with a lint-dirty label got `✓ All checks passed`
+  // while the byte-identical top-level spelling of the same metadata reported
+  // it.
+  //
+  // Counted over the reader's RETURN VALUE, per this file's rule — never over
+  // `project.<collection>`, which would be a second copy of the read the row
+  // watches.
+  //
+  // ⚠️ Narrowed to the ONE rule the zoo provokes on purpose, and the narrowing
+  // is what makes the row a measurement at all: every other finding `lintConfig`
+  // returns on this fixture comes from the rule registry, which already folds,
+  // so an unfiltered count is identical in both shapes before AND after the fix
+  // — a row that can never go red. `PROBE_DIRTY_LABEL` is the label that
+  // provokes it, carried by the MODULE package so this is a cross-package
+  // resolution. ⛔ This row is enumeration, not coverage of the family: the
+  // byte-parity of the WHOLE family across both shapes is pinned separately, in
+  // `test/lint-handwritten-checks-package-fold.test.ts`.
+  const handWritten = lintConfig(project as Record<string, unknown>)
+    .filter((issue) => issue.rule === PROBE_DIRTY_LABEL_RULE);
+  rows.push(countRow(
+    'B2/B3 · cli lint hand-written rubric (lintConfig — naming, labels, structure) · objects',
+    handWritten.length,
+  ));
+
+  // [#17528] The metadata-quality SCORER, the second call site into the reader
+  // above (`lint/score.js`, reached by `os lint --score` and the metadata eval).
+  // It is the sharper half: the linter RAN and read an empty stack, so an
+  // option-B project's rubric was computed over nothing and published
+  // `100/100 (A)` — the #15658 shape, "the linter found nothing" and "the linter
+  // never ran" collapsed into the better-looking one, arriving through the INPUT
+  // rather than through a swallowed crash.
+  //
+  // A separate row from the one above because it is a separate DOOR: the scorer
+  // normalizes its input first (`normalizeStackInput`) and answers a score
+  // rather than a finding list, so a fold that reached `lintConfig` but not this
+  // path would still publish a clean number. Counted over `MetadataScore.issues`
+  // — the reader's return value — with the score rendered beside it.
+  const score = scoreMetadata(project);
+  const scoredHandWritten = score.issues
+    .filter((issue) => issue.rule === PROBE_DIRTY_LABEL_RULE);
+  rows.push(row(
+    'B2/B3 · cli metadata-quality rubric (scoreMetadata — os lint --score, metadata eval) · objects',
+    `${scoredHandWritten.length} hand-written finding(s) · score ${score.score}/${score.grade}`,
+    scoredHandWritten.length === 0,
+  ));
+
   // ── B2 · `os verify`'s readers (#15229) ──────────────────────────────────
   //
   // `os verify` has exactly ONE door — `loadConfig` (`verify.ts:92`) — and it