From 5c5e1b907f434e45d906f9ebaedd60fe060c305a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 02:27:10 +0000 Subject: [PATCH 1/6] fix(cli): the per-package de-duplication key ignores the top-level collection index, so an echo no longer survives it (#18779) Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 Co-authored-by: Claude --- packages/cli/src/commands/compile.ts | 18 +- packages/cli/src/commands/lint.ts | 23 +- packages/cli/src/commands/validate.ts | 24 +- packages/cli/src/utils/artifact-packages.ts | 118 ++++++-- .../lint-per-package-authoring-parity.test.ts | 13 +- .../lint-per-package-authoring-seam.test.ts | 24 +- .../per-package-dedup-positional-echo.test.ts | 267 ++++++++++++++++++ ...idate-per-package-authoring-parity.test.ts | 16 +- ...alidate-per-package-authoring-seam.test.ts | 25 +- 9 files changed, 463 insertions(+), 65 deletions(-) create mode 100644 packages/cli/test/per-package-dedup-positional-echo.test.ts diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index 5d282be680..0f708f8fa9 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -452,8 +452,22 @@ export default class Compile extends Command { // DE-DUPLICATED against the union run, because the union contains // every package's items: without this, a two-package project reports // every finding twice and the author cannot tell a real per-package - // finding from an echo. What survives the filter is exactly the set - // the union could not see. + // finding from an echo. + // + // ⚠️ [#18779] This comment used to end "What survives the filter is + // exactly the set the union could not see", and that was FALSE for + // as long as the de-duplication key carried the POSITIONAL `path`: + // a package body re-bases its collections from 0, so one finding got + // two keys and its echo survived the very filter described here. The + // sentence was quoted as authority by #18677 and #18778 without the + // definition being opened, and copied into the `os validate` and + // `os lint` doors as each was wired. `findingKey` now neutralises + // the top-level collection index, so what survives is the set of + // per-package findings no union finding already carried under the + // same rule, `where`, message and non-top-level position. ⛔ Do not + // re-inflate that to "exactly the set the union could not see" — + // `utils/artifact-packages.ts` states the bound and why it is + // narrower than that sentence. // // [#16611] Each package's stack is handed the artifact's `packages[]` // as RESOLUTION CONTEXT — see `packageBodyAsStack`. The list read here diff --git a/packages/cli/src/commands/lint.ts b/packages/cli/src/commands/lint.ts index 5b2eb1af02..1ccf9274a9 100644 --- a/packages/cli/src/commands/lint.ts +++ b/packages/cli/src/commands/lint.ts @@ -684,10 +684,19 @@ export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue // // 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. + // lint` ran the union fold and stopped. Every finding this pass yields is + // therefore one `os build` reported and this command structurally could not. + // + // ⚠️ [#18779] This paragraph used to size that gap by quoting `compile.ts` + // step 3b-ii — "exactly the set the union could not see" — and that sentence + // was FALSE when it was copied here: the de-duplication key carried the + // POSITIONAL `path`, so a package-local finding and its flattened twin got + // two keys and the ECHO survived. Part of every survivor set was therefore + // something this door's own union run ALREADY reported. The key was + // corrected in `utils/artifact-packages.ts`; the gap this door closed is + // real and its direction is unchanged, but ⛔ do not re-derive its size from + // that sentence — it was quoted, never measured, by the two cards that + // wired the second and third doors. // // ⚠️ The reading that hid it for two cards is the one the imports above // invite: this file DOES call `artifactPackages` and `packageBodyAsStack` — @@ -727,8 +736,10 @@ export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue // 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. + // De-duplicated against the run above, on the UNPREFIXED finding — so what + // reaches the list below is what that run did not already carry under the + // same rule, `where`, message and non-top-level position (#18779; the key + // used to compare the top-level index too, and let the echo through). unionFindings, sduiManifest: opts.sduiManifest, loweredHookRefs, diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index 340cec6253..dd1947b1a9 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -391,13 +391,23 @@ export default class Validate extends Command { // // `os build` has run it since #16611; `os validate` ran the union // fold and stopped, importing neither `artifactPackages` nor - // `packageBodyAsStack`. `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. Same FALSE-CLEAN direction #17069 fixed one - // layer up, and the worse door for it: the fast inner-loop check is - // what an author runs BEFORE shipping, so its clean bill of health is - // the strongest false assurance the three commands can give. + // `packageBodyAsStack`. Every finding this pass yields is therefore + // one `os build` reported and this command structurally could not. + // Same FALSE-CLEAN direction #17069 fixed one layer up, and the worse + // door for it: the fast inner-loop check is what an author runs + // BEFORE shipping, so its clean bill of health is the strongest false + // assurance the three commands can give. + // + // ⚠️ [#18779] This step used to size that gap by quoting `compile.ts` + // step 3b-ii — "exactly the set the union could not see" — and that + // sentence was FALSE when it was copied here: the de-duplication key + // carried the POSITIONAL `path`, so a package-local finding and its + // flattened twin got two keys and the ECHO survived. Part of every + // survivor set was therefore something THIS door's own union run + // already reported. The key was corrected in + // `utils/artifact-packages.ts`; the gap this step closed is real and + // its direction is unchanged, but ⛔ do not re-derive its size from + // that sentence — it was quoted, never measured. // // ⛔ Not a second copy of the loop — `runPerPackageAuthoringRules` is // the one the build door calls, so the de-duplication key, the diff --git a/packages/cli/src/utils/artifact-packages.ts b/packages/cli/src/utils/artifact-packages.ts index af0f3a0554..826e9b8c01 100644 --- a/packages/cli/src/utils/artifact-packages.ts +++ b/packages/cli/src/utils/artifact-packages.ts @@ -46,15 +46,62 @@ import { type AuthoringFinding, } from '@objectstack/lint'; +/** + * The leading `collection[N]` of a finding path — the ONE coordinate that + * differs between the two views of a single finding. `objects[3].fields.x` + * matches `objects[3]`; `manifest.namespace` matches nothing. + */ +const TOP_LEVEL_COLLECTION_INDEX = /^([A-Za-z_][A-Za-z0-9_]*)\[\d+\]/; + /** * Identity of one finding, for the per-package de-duplication below. * - * Moved here from `compile.ts` unchanged (#18677): the two doors must - * de-duplicate identically, or "the set the union could not see" means two - * different things depending on which command the author happened to run. + * Moved here from `compile.ts` unchanged (#18677); its POSITIONAL half was + * corrected here (#18779). All three doors must de-duplicate identically, or + * what survives the filter means a different thing depending on which command + * the author happened to run. + * + * ## Why the top-level collection index is neutralised (#18779) + * + * `rule`, `where` and `message` say WHICH finding this is; `path` says where + * it sits. The inherited key used `path` raw — and `path` is positional, so + * one finding judged twice got two keys and the `Set` below never matched + * them. A package body re-bases every collection from 0, while the flattened + * union numbers that same entry wherever `authoringRuleUnionStack` placed it: + * `objects[0].fields.industry` (package-local) and `objects[1].fields.industry` + * (union) are ONE finding under two spellings. Measured on + * `examples/app-multi-package` before this landed — 1 survivor, 1 echo, 0 + * genuinely new, and `os build` printed "4 author-time warning(s)" for 3 + * distinct ones. The de-duplication exists precisely so that "the author + * cannot tell a real per-package finding from an echo" would stop being true, + * and the positional key is why it stayed true. + * + * ⛔ The rewrite touches the KEY only — a finding's own `path` is never + * modified, so every door still prints the positional location it always + * printed. And only the TOP-LEVEL index: nested positions (`.indexes[1]`, + * `.columns[0]`) address the author's own document and read identically in + * both views, so they stay in the key and keep discriminating. + * + * ⛔ Not `nameKeyFindingPath` (`@objectstack/lint`'s runtime-gate rewrite of + * this same coordinate), for the reason that function's own docblock records: + * it is "Applied AFTER the differential, not before it … two stored items that + * (illegitimately) share a name must not have their distinct findings merged + * or cancelled by the rewrite". A de-duplication key IS that differential, so + * name-keying is the one place it rules itself out. Two further readings from + * the same docblock: its key set is DERIVED and holds `objects`, `permissions` + * and `books` today, so it would leave every other collection's echo standing, + * and it is "Exported for the pin, not for callers" — it sits on neither of + * that package's entries. + * + * ⚠️ What this does NOT buy, written down so the next reader does not + * re-inflate it: the key becomes position-insensitive, ⛔ not collision-proof. + * Two entries that render the same `where` — an illegitimate duplicate name — + * still share a key, exactly as they already did whenever their indices + * matched too. The claim the pass below is entitled to make is stated there, + * and it is narrower than "exactly the set the union could not see". */ const findingKey = (f: { rule: string; where: string; path: string; message: string }): string => - [f.rule, f.where, f.path, f.message].join('\u0000'); + [f.rule, f.where, f.path.replace(TOP_LEVEL_COLLECTION_INDEX, '$1[]'), f.message].join('\u0000'); /** * The artifact's package entries, as `{ index, id, body }` (ADR-0130 D4). @@ -161,30 +208,45 @@ export function packageBodyAsStack( * ## What the asymmetry was, measured * * `os build` ran this pass; `os validate` ran the union fold and stopped, - * importing neither seam above. `compile.ts`' own comment says what survives - * the de-duplication is "exactly the set the union could not see" ⇒ that whole - * set was findings `os build` reported and `os validate` structurally could - * not. The direction is FALSE-CLEAN, and on the command an author runs BEFORE - * shipping — the same direction and the same door #17069 fixed one layer up, - * which is why `authoringRuleUnionStack` being in both commands did not settle - * it. `packages/cli/test/build-json-advisory-parity.e2e.test.ts` already - * asserted "nothing rides in build's `warnings` that validate does not also - * report"; it stayed green because its fixture declares no `packages[]` at all, - * so the pass it would have caught never ran there. - * - * ## The de-duplication key is the caller's, and it is not perfect - * - * `findingKey` below is `compile.ts`' key, moved unchanged: `rule`, `where`, - * `path`, `message`. ⚠️ `path` is POSITIONAL, and a collection index in one - * package's own body is not the index the flattened top level gives the same - * item — so a finding on any package whose local index differs from its - * flattened one survives the filter as an ECHO of a union finding rather than - * as something the union could not see. Measured on `examples/app-multi-package` - * (2 packages, `crm_account.industry`): 1 survivor, 0 of them new. ⛔ Not fixed - * here — changing the key changes what `os build` reports, which is a separate - * decision from making the two doors agree, and agreeing IMPERFECTLY at one - * seam is strictly better than disagreeing at two. When it is fixed it is - * fixed once, for both commands, which is the property this module buys. + * importing neither seam above. Every finding this pass yields was therefore + * one `os build` reported and `os validate` structurally could not — the + * direction is FALSE-CLEAN, and on the command an author runs BEFORE shipping. + * Same direction and same door #17069 fixed one layer up, which is why + * `authoringRuleUnionStack` being in both commands did not settle it. + * `packages/cli/test/build-json-advisory-parity.e2e.test.ts` already asserted + * "nothing rides in build's `warnings` that validate does not also report"; it + * stayed green because its fixture declares no `packages[]` at all, so the pass + * it would have caught never ran there. + * + * ⚠️ #18779 corrected the SIZE that sentence used to be given, ⛔ not its + * direction. #18677 and #18778 both sized this blind spot by quoting + * `compile.ts`' claim that the survivors are "exactly the set the union could + * not see" — but the key was positional, so part of every survivor set was + * ECHO: findings the union run ALSO reported, which means `os validate` was + * reporting them all along through its own union run. On + * `examples/app-multi-package` the whole of it was — 1 survivor, 1 echo, 0 + * genuinely new — so `os validate`'s true blind spot on that fixture was ZERO + * findings, not one. ⛔ Neither card measured that; both quoted it. The + * asymmetry was real and worth closing on every door; its magnitude was + * inherited from a sentence nobody had read the definition behind. + * + * ## What the de-duplication key can and cannot promise + * + * `findingKey` above neutralises the top-level collection index (#18779), so + * the two views of one finding now produce one key and an echo is filtered. + * What reaches the lists below is therefore the set of per-package findings + * whose `rule`, `where`, `message` and NON-top-level position no union finding + * already carried. + * + * ⚠️ That is the whole claim, and it is deliberately narrower than "exactly the + * set the union could not see" — ⛔ do not restate it as that sentence. Two + * entries rendering the same `where` still collapse (see `findingKey`), and a + * rule that reports the same `rule`/`where`/`message` for genuinely different + * items distinguished ONLY by their top-level index would collapse too. That + * combination is not reachable through any rule in the registry today — every + * rule names its entity in `where`, which `packages/lint/src/ + * data-model-rule-where-slot.test.ts` holds for the whole registry — so the + * bound is structural, ⛔ not a measured count of survivors on some corpus. */ export function runPerPackageAuthoringRules(run: { /** Which door is asking — the same string its union run passed. */ diff --git a/packages/cli/test/lint-per-package-authoring-parity.test.ts b/packages/cli/test/lint-per-package-authoring-parity.test.ts index 40c9cfdb93..e874f594f6 100644 --- a/packages/cli/test/lint-per-package-authoring-parity.test.ts +++ b/packages/cli/test/lint-per-package-authoring-parity.test.ts @@ -7,9 +7,16 @@ * * `os build` has run the rule table a second time, once per * `artifactPackages(…)` entry, since #16611; `os validate` joined it in #18677. - * `os lint` ran the union fold and stopped, so the survivors of that pass — - * "exactly the set the union could not see", in `compile.ts`' own words — were - * findings `os build` reported and `os lint` structurally could not. + * `os lint` ran the union fold and stopped, so every survivor of that pass was + * a finding `os build` reported and `os lint` structurally could not. + * + * ⚠️ [#18779] This header used to size that set by quoting `compile.ts` — + * "exactly the set the union could not see" — and the sentence was false when + * it was quoted: the de-duplication key carried the POSITIONAL `path`, so + * echoes of union findings survived it and part of every survivor set was + * already being reported by this door's own union run. The key was corrected + * there. The PARITY this file pins is unaffected in either direction, because + * both doors always ran the one pass with the one key. * * ## Why this file exists next to the in-process seam pin * diff --git a/packages/cli/test/lint-per-package-authoring-seam.test.ts b/packages/cli/test/lint-per-package-authoring-seam.test.ts index 684b9ad1b3..a22658631b 100644 --- a/packages/cli/test/lint-per-package-authoring-seam.test.ts +++ b/packages/cli/test/lint-per-package-authoring-seam.test.ts @@ -6,11 +6,17 @@ * * `os build` has run the table a second time, once per `artifactPackages(…)` * entry with `packageBodyAsStack(…)` as resolution context, since #16611; - * `os validate` joined it in #18677. `os lint` did not. `compile.ts` step 3b-ii - * says what survives that de-duplication is "exactly the set the union could - * not see" ⇒ that whole set was findings `os build` reported and `os lint` + * `os validate` joined it in #18677. `os lint` did not, so every survivor of + * that de-duplication was a finding `os build` reported and `os lint` * structurally could not — FALSE-CLEAN, on the fastest of the three doors. * + * ⚠️ [#18779] This header used to size that set by quoting `compile.ts` step + * 3b-ii — "exactly the set the union could not see" — and the sentence was + * false when it was quoted: the de-duplication key carried the POSITIONAL + * `path`, so echoes of union findings survived it. The key was corrected + * there. The fixture below is unaffected and is the reason why: it raises a + * finding the union genuinely cannot produce, ⛔ not an echo. + * * ## ⭐ Why symbol presence scored this door as covered * * `lint.ts` DOES import `artifactPackages` and `packageBodyAsStack`. Opening @@ -58,10 +64,16 @@ const PER_PACKAGE = /^package '[^']+' — /; * The `core` package owns `pp_account`; the `orders` package owns the view that * displays `pp_account.industry`. Judged as one flattened union the field has a * consumer and `field-no-consumers` says nothing; judged per package, `core` - * declares a field nothing IN CORE reads. That is not an echo of a union - * finding — it is a member of "the set the union could not see", which is the - * only shape that can falsify this card, and the reason this fixture is not the + * declares a field nothing IN CORE reads. That is ⛔ not an echo of a union + * finding — the union produced no such finding at all — which is the only + * shape that can falsify this card, and the reason this fixture is not the * sibling file's. + * + * [#18779] It is also the positive control for the corrected de-duplication + * key: a key that stopped discriminating would swallow this finding too, and + * this file goes red. The echo the corrected key DOES remove is pinned in + * `per-package-dedup-positional-echo.test.ts`, on a fixture built the other + * way round. */ function twoPackageArtifact(): Record { const core = defineStack({ diff --git a/packages/cli/test/per-package-dedup-positional-echo.test.ts b/packages/cli/test/per-package-dedup-positional-echo.test.ts new file mode 100644 index 0000000000..a6c9606621 --- /dev/null +++ b/packages/cli/test/per-package-dedup-positional-echo.test.ts @@ -0,0 +1,267 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #18779 — the per-package de-duplication key was POSITIONAL, so an ECHO of a + * union finding survived the filter that exists to remove it. + * + * `runPerPackageAuthoringRules` judges each `packages[]` entry as its own stack + * and drops anything the union run already reported. A package body re-bases + * every collection from 0, while the flattened union numbers that same entry + * wherever `authoringRuleUnionStack` put it — so one finding arrived under two + * paths, `findingKey` produced two keys, and the `Set` never matched them. + * + * ## What that cost, measured on `origin/main` 18cc3b1dfc + * + * On `examples/app-multi-package`, the repo's own two-package fixture: + * + * os build --json warnings 4 <- 3 union + 1 per-package survivor + * the survivor field-no-consumers on crm_account.industry + * union path objects[1].fields.industry + * survivor path objects[0].fields.industry + * => 1 of 1 an echo, 0 genuinely new + * + * So the pass's entire output on that fixture was a duplicate of a warning the + * same command had already printed, and `compile.ts`' claim that the survivors + * are "exactly the set the union could not see" was false in the one place + * anyone could check it. ⛔ Two cards (#18677, #18778) and two review seats + * quoted that sentence as authority; none opened `findingKey`. + * + * ## What this file pins + * + * The echo case ⇒ de-duplicated, and THREE controls that must still survive, + * because a key that stopped discriminating would satisfy the echo case + * trivially. The controls are what make a green here mean something: + * + * - a union twin differing in its NESTED position — the coordinate the + * rewrite deliberately leaves alone; + * - a union twin differing in `where` — a different entity; + * - a union twin differing in `rule`. + * + * `REAL_UNION` closes it end to end on a fixture shaped like the example: the + * union run really does raise the twin at a different index, so the case is + * not an artefact of hand-built `unionFindings`. + * + * ## Tier + * + * Spawns nothing and boots no kernel ⇒ UNIT tier by + * `packages/cli/vitest-tiers.ts`' predicate, asserted in BOTH directions by the + * last case rather than assumed from the filename. + */ + +import { describe, it, expect } from 'vitest'; +import { dirname, relative, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { normalizeStackInput, ObjectStackDefinitionSchema } from '@objectstack/spec'; +import { runAuthoringRules, type AuthoringFinding } from '@objectstack/lint'; +import { authoringRuleUnionStack } from '../src/utils/stack-collections.js'; +import { runPerPackageAuthoringRules } from '../src/utils/artifact-packages.js'; +import { firedSignals, integrationTestFiles, isIntegration, tierOfFile } from '../vitest-tiers.js'; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const PKG = resolve(HERE, '..'); +const THIS_FILE = relative(PKG, fileURLToPath(import.meta.url)); + +const CORE_MANIFEST = { + id: 'com.example.echo.core', + name: 'echo core', + namespace: 'pp', + version: '1.0.0', + type: 'app', + engines: { protocol: '^17' }, +}; + +const CORE_OBJECTS = [ + { + name: 'pp_account', + label: 'Account', + pluralLabel: 'Accounts', + sharingModel: 'private', + fields: { + name: { name: 'name', type: 'text', label: 'Account Name', required: true }, + industry: { name: 'industry', type: 'text', label: 'Industry' }, + }, + }, +]; + +const CORE_APPS = [ + { + name: 'pp_crm', + label: 'PP CRM', + navigation: [ + { + id: 'sales_group', + type: 'group', + label: 'Sales', + children: [{ id: 'nav_accounts', type: 'object', objectName: 'pp_account', label: 'Accounts' }], + }, + ], + }, +]; + +const ORDERS_MANIFEST = { + id: 'com.example.echo.orders', + name: 'echo orders', + namespace: 'pp', + version: '1.0.0', + type: 'module', + engines: { protocol: '^17' }, + dependencies: { 'com.example.echo.core': '^1.0.0' }, +}; + +const ORDERS_OBJECTS = [ + { + name: 'pp_order', + label: 'Order', + pluralLabel: 'Orders', + sharingModel: 'private', + fields: { + name: { name: 'name', type: 'text', label: 'Order Number', required: true }, + account: { name: 'account', type: 'lookup', label: 'Account', reference: 'pp_account' }, + }, + }, +]; + +/** + * The example's shape: `orders` is listed FIRST, so `pp_account` lands at + * `objects[1]` in the flattened union and at `objects[0]` in `core`'s own body. + * That index disagreement IS the defect — a fixture with one package, or with + * the App package first, cannot exhibit it. + */ +function artifact(): Record { + return { + manifest: CORE_MANIFEST, + objects: [...ORDERS_OBJECTS, ...CORE_OBJECTS], + apps: [...CORE_APPS], + packages: [ + { manifest: { ...ORDERS_MANIFEST, objects: ORDERS_OBJECTS } }, + { manifest: { ...CORE_MANIFEST, objects: CORE_OBJECTS, apps: CORE_APPS } }, + ], + } as Record; +} + +function parsedArtifact(): Record { + const normalized = normalizeStackInput(artifact(), {}); + const result = ObjectStackDefinitionSchema.safeParse(normalized); + if (!result.success) { + throw new Error(`fixture is not schema-valid: ${JSON.stringify(result.error.issues.slice(0, 3))}`); + } + return result.data as unknown as Record; +} + +/** The pass's own output with NO union run to de-duplicate against. */ +function rawSurvivors(parsed: Record): AuthoringFinding[] { + return runPerPackageAuthoringRules({ command: 'build', parsed, unionFindings: [] }).findings; +} + +/** `where` as the de-duplication sees it — the pass prefixes it on the way out. */ +const unprefixed = (f: AuthoringFinding): AuthoringFinding => ({ + ...f, + where: f.where.replace(/^package '[^']*' — /, ''), +}); + +const survivorsAgainst = ( + parsed: Record, + unionFindings: readonly AuthoringFinding[], +): AuthoringFinding[] => runPerPackageAuthoringRules({ command: 'build', parsed, unionFindings }).findings; + +describe('#18779 — the per-package de-duplication key is position-insensitive', () => { + it('the fixture reaches the pass and raises the finding the rest of this file is about', () => { + // Asserted first: every case below is vacuous if the pass produces nothing, + // which is how this defect survived two cards' worth of parity pins. + const raw = rawSurvivors(parsedArtifact()); + expect(raw.length).toBeGreaterThan(0); + expect(raw.some((f) => f.rule === 'field-no-consumers' && /industry/.test(f.path))).toBe(true); + expect(raw.every((f) => /^package '/.test(f.where))).toBe(true); + }); + + it('ECHO: a union twin at a different TOP-LEVEL index de-duplicates the survivor', () => { + const parsed = parsedArtifact(); + const raw = rawSurvivors(parsed); + const target = raw.find((f) => f.rule === 'field-no-consumers' && /industry/.test(f.path)); + expect(target, 'fixture no longer raises the finding this case is built on').toBeTruthy(); + + const local = unprefixed(target!); + // The SAME finding as the flattened union numbers it: `pp_account` sits at + // `objects[1]` there and at `objects[0]` in `core`'s body. Only that one + // coordinate differs — which is exactly what an echo is. + expect(local.path).toMatch(/^objects\[0\]\./); + const twin: AuthoringFinding = { ...local, path: local.path.replace(/^objects\[0\]/, 'objects[1]') }; + expect(twin.path).not.toBe(local.path); + + const survived = survivorsAgainst(parsed, [twin]); + expect( + survived.filter((f) => f.rule === target!.rule && f.where === target!.where), + 'an echo of a union finding must not survive the de-duplication that exists to remove it', + ).toEqual([]); + }); + + it('CONTROL: a union twin differing in its NESTED position still lets the survivor through', () => { + // The rewrite is the top-level index ONLY. Nested positions address the + // author's own document and read the same in both views, so widening the + // rewrite to every index would swallow genuinely different findings — this + // case is what goes red if someone does. + const parsed = parsedArtifact(); + const target = rawSurvivors(parsed).find((f) => f.rule === 'field-no-consumers')!; + const local = unprefixed(target); + const twin: AuthoringFinding = { + ...local, + path: `objects[1].fields.some_other_field`, + }; + expect(survivorsAgainst(parsed, [twin]).map((f) => f.where)).toContain(target.where); + }); + + it('CONTROL: a union twin differing in `where` or `rule` still lets the survivor through', () => { + const parsed = parsedArtifact(); + const target = rawSurvivors(parsed).find((f) => f.rule === 'field-no-consumers')!; + const local = unprefixed(target); + const otherWhere: AuthoringFinding = { + ...local, + where: 'object "pp_order" · field "industry"', + path: local.path.replace(/^objects\[0\]/, 'objects[1]'), + }; + const otherRule: AuthoringFinding = { + ...local, + rule: 'some-other-rule', + path: local.path.replace(/^objects\[0\]/, 'objects[1]'), + }; + expect(survivorsAgainst(parsed, [otherWhere]).map((f) => f.where)).toContain(target.where); + expect(survivorsAgainst(parsed, [otherRule]).map((f) => f.where)).toContain(target.where); + }); + + it('REAL_UNION: the union run really does raise the twin, at a different index, and it is filtered', () => { + // End to end, with no hand-built finding anywhere: the union run over the + // folded stack produces the twin itself. Before #18779 this fixture + // reported the same warning twice. + const parsed = parsedArtifact(); + const normalized = normalizeStackInput(artifact(), {}); + const unionFindings = runAuthoringRules('build', { + normalized: authoringRuleUnionStack(normalized as Record), + parsed: authoringRuleUnionStack(parsed), + }); + + const raw = rawSurvivors(parsed); + const target = unprefixed(raw.find((f) => f.rule === 'field-no-consumers' && /industry/.test(f.path))!); + const twin = unionFindings.find( + (u) => u.rule === target.rule && u.where === target.where && u.message === target.message, + ); + // Non-vacuity: the union MUST carry the twin, or "it was filtered" says + // nothing. And it must carry it at a DIFFERENT path, or the old key would + // have matched it too and this fixture would not exhibit the defect. + expect(twin, 'the union run no longer raises the twin — this fixture no longer exhibits the defect').toBeTruthy(); + expect(twin!.path).not.toBe(target.path); + + const survived = survivorsAgainst(parsed, unionFindings); + expect(survived.filter((f) => f.rule === target.rule && f.where.endsWith(target.where))).toEqual([]); + }); + + it('is a UNIT-tier file, asserted in BOTH directions from the predicate itself', () => { + // A pin in the wrong tier runs where the merge queue does not look, and a + // pin that MOVES its file between tiers takes its neighbours with it. Both + // directions, read from the predicate rather than from the filename: no + // integration signal fires, and the derived integration population does + // not contain this file. + const signals = tierOfFile(PKG, THIS_FILE); + expect(isIntegration(signals), `fired: ${firedSignals(signals)}`).toBe(false); + expect(integrationTestFiles(PKG)).not.toContain(THIS_FILE); + }); +}); diff --git a/packages/cli/test/validate-per-package-authoring-parity.test.ts b/packages/cli/test/validate-per-package-authoring-parity.test.ts index e4a24cdf2c..31d3a67a4b 100644 --- a/packages/cli/test/validate-per-package-authoring-parity.test.ts +++ b/packages/cli/test/validate-per-package-authoring-parity.test.ts @@ -5,11 +5,17 @@ * author-time advisory set for a MULTI-PACKAGE project. * * `os build` ran the rule table a second time, once per `artifactPackages(…)` - * entry; `os validate` ran the union fold and stopped. By `compile.ts`' own - * description the survivors of that pass are "exactly the set the union could - * not see" ⇒ that whole set was findings `os build` reported and `os validate` - * structurally could not. FALSE-CLEAN, on the fast pre-flight an author runs - * before shipping. + * entry; `os validate` ran the union fold and stopped, so every survivor of + * that pass was a finding `os build` reported and `os validate` structurally + * could not. FALSE-CLEAN, on the fast pre-flight an author runs before + * shipping. + * + * ⚠️ [#18779] This header used to size that set by quoting `compile.ts` — + * "exactly the set the union could not see" — and the sentence was false when + * it was quoted: the de-duplication key carried the POSITIONAL `path`, so + * echoes survived it. The key was corrected there; the PARITY pinned here is + * unaffected in either direction, because both doors run the one pass with the + * one key. * * ## Why a NEW fixture and not an assertion on the existing parity file * diff --git a/packages/cli/test/validate-per-package-authoring-seam.test.ts b/packages/cli/test/validate-per-package-authoring-seam.test.ts index cb45450cf9..d2df9753ef 100644 --- a/packages/cli/test/validate-per-package-authoring-seam.test.ts +++ b/packages/cli/test/validate-per-package-authoring-seam.test.ts @@ -3,12 +3,16 @@ /** * #18677 — `os build` ran the author-time rule table a SECOND time, once per * `artifactPackages(…)` entry with `packageBodyAsStack(…)` as resolution - * context; `os validate` ran the union fold and stopped. `compile.ts`' own - * comment says what survives that de-duplication is "exactly the set the union - * could not see" ⇒ that whole set was findings `os build` reported and - * `os validate` structurally could not. FALSE-CLEAN, on the command an author - * runs BEFORE shipping — the same direction #17069 fixed one layer up, which is - * why `authoringRuleUnionStack` landing in both commands did not settle it. + * context; `os validate` ran the union fold and stopped, so every survivor of + * that de-duplication was a finding `os build` reported and `os validate` + * structurally could not. FALSE-CLEAN, on the command an author runs BEFORE + * shipping — the same direction #17069 fixed one layer up, which is why + * `authoringRuleUnionStack` landing in both commands did not settle it. + * + * ⚠️ [#18779] This header used to size that set by quoting `compile.ts` — + * "exactly the set the union could not see" — and the sentence was false when + * it was quoted: the key carried the POSITIONAL `path`, so echoes survived it. + * The key was corrected there; the seam this file pins is unaffected. * * ## What this file pins, and what its sibling pins * @@ -186,8 +190,13 @@ describe('#18677 — the per-package author-time pass is ONE seam both doors rea it('de-duplicates against the union run it is handed', () => { // The filter `compile.ts` described and this pass now owns. Handing it its - // OWN output as the union run must empty it — the property that makes - // "exactly the set the union could not see" mean anything at all. + // OWN output as the union run must empty it — the property that makes the + // survivor list mean anything at all. ⚠️ This case is NOT sensitive to + // #18779's defect and never was: both runs judge the same stack, so the + // two paths are identical and the positional key matched them anyway. The + // echo it missed needs the two views to DISAGREE about the index, which + // only a real package-body-vs-union comparison produces — pinned in + // `per-package-dedup-positional-echo.test.ts`. const parsed = twoPackageArtifact(); const first = runPerPackageAuthoringRules({ command: 'validate', parsed, unionFindings: [] }); const raw = [...first.errors, ...first.advisories]; From 7acabb910421c649abdcff115187052edb915bfa Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 03:08:48 +0000 Subject: [PATCH 2/6] test(cli): the NESTED control is built on a finding that actually carries a nested index, and the key's bound is measured rather than quoted (#18779) Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 Co-authored-by: Claude --- .../per-package-dedup-positional-echo.test.ts | 54 +++++++++++++++---- 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/packages/cli/test/per-package-dedup-positional-echo.test.ts b/packages/cli/test/per-package-dedup-positional-echo.test.ts index a6c9606621..9451fdf4e1 100644 --- a/packages/cli/test/per-package-dedup-positional-echo.test.ts +++ b/packages/cli/test/per-package-dedup-positional-echo.test.ts @@ -32,11 +32,20 @@ * because a key that stopped discriminating would satisfy the echo case * trivially. The controls are what make a green here mean something: * - * - a union twin differing in its NESTED position — the coordinate the + * - a union twin differing only in its NESTED INDEX — the coordinate the * rewrite deliberately leaves alone; * - a union twin differing in `where` — a different entity; * - a union twin differing in `rule`. * + * ⚠️ The first control is built on `unique/unscoped-declared-index`, ⛔ not on + * the `field-no-consumers` finding the echo cases use, and the fixture carries + * a bare `unique: true` index for no other reason. Measured, ⛔ not reasoned: + * an ablation that widens the rewrite from the top-level index to EVERY index + * left all six cases green while that control was written against a + * `field-no-consumers` twin, because such a path ends in a field NAME and the + * twin therefore differed in a name rather than in a position. A control that + * cannot fail is decoration; re-run that ablation if this file is edited. + * * `REAL_UNION` closes it end to end on a fixture shaped like the example: the * union run really does raise the twin at a different index, so the case is * not an artefact of hand-built `unionFindings`. @@ -118,6 +127,16 @@ const ORDERS_OBJECTS = [ name: { name: 'name', type: 'text', label: 'Order Number', required: true }, account: { name: 'account', type: 'lookup', label: 'Account', reference: 'pp_account' }, }, + // ⛔ Not decoration. A bare `unique: true` trips + // `unique/unscoped-declared-index`, whose path carries a NESTED index + // (`objects[0].indexes[0]`) — and a finding with a nested index is the ONLY + // thing the NESTED control below can discriminate on. Every other rule this + // fixture raises produces `objects[N].fields.`, where the sole index + // is the top-level one, so a "nested position" control written against one + // of those cannot fail and is not a control. Measured: without this, an + // ablation that rewrites EVERY index instead of the top-level one keeps all + // six cases green. + indexes: [{ name: 'pp_order_name_uq', fields: ['name'], unique: true }], }, ]; @@ -171,6 +190,10 @@ describe('#18779 — the per-package de-duplication key is position-insensitive' const raw = rawSurvivors(parsedArtifact()); expect(raw.length).toBeGreaterThan(0); expect(raw.some((f) => f.rule === 'field-no-consumers' && /industry/.test(f.path))).toBe(true); + // The carrier the NESTED control needs — asserted here so its absence reads + // as "the fixture stopped raising it" rather than as a silently weakened + // control further down. + expect(raw.some((f) => /^objects\[\d+\]\.indexes\[\d+\]$/.test(f.path))).toBe(true); expect(raw.every((f) => /^package '/.test(f.where))).toBe(true); }); @@ -195,19 +218,32 @@ describe('#18779 — the per-package de-duplication key is position-insensitive' ).toEqual([]); }); - it('CONTROL: a union twin differing in its NESTED position still lets the survivor through', () => { + it('CONTROL: a union twin differing only in its NESTED index still lets the survivor through', () => { // The rewrite is the top-level index ONLY. Nested positions address the // author's own document and read the same in both views, so widening the // rewrite to every index would swallow genuinely different findings — this // case is what goes red if someone does. + // + // ⚠️ It has to be built on a finding whose path ACTUALLY carries a nested + // index. `unique/unscoped-declared-index` is that finding here + // (`objects[0].indexes[0]`); `field-no-consumers` is not — its path ends in + // a field NAME, so a twin built from it differs in a name rather than in a + // position and stays distinct under any index rewrite at all. Measured: the + // over-wide ablation keeps a `field-no-consumers` twin green and turns this + // one red, which is the whole difference between a control and a decoration. const parsed = parsedArtifact(); - const target = rawSurvivors(parsed).find((f) => f.rule === 'field-no-consumers')!; - const local = unprefixed(target); - const twin: AuthoringFinding = { - ...local, - path: `objects[1].fields.some_other_field`, - }; - expect(survivorsAgainst(parsed, [twin]).map((f) => f.where)).toContain(target.where); + const target = rawSurvivors(parsed).find((f) => f.rule === 'unique/unscoped-declared-index'); + expect(target, 'fixture no longer raises a finding with a NESTED index in its path').toBeTruthy(); + const local = unprefixed(target!); + expect(local.path).toMatch(/^objects\[\d+\]\.indexes\[\d+\]$/); + + // Same top-level index, DIFFERENT nested one — the coordinate the rewrite + // must leave alone. + const twin: AuthoringFinding = { ...local, path: local.path.replace(/\.indexes\[0\]$/, '.indexes[1]') }; + expect(twin.path).not.toBe(local.path); + expect(twin.path.replace(/^objects\[\d+\]/, '')).not.toBe(local.path.replace(/^objects\[\d+\]/, '')); + + expect(survivorsAgainst(parsed, [twin]).map((f) => f.where)).toContain(target!.where); }); it('CONTROL: a union twin differing in `where` or `rule` still lets the survivor through', () => { From 9a9e9b62a7a1bb8871dd5ca911f75d4dce4c6c0e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 03:09:15 +0000 Subject: [PATCH 3/6] docs(cli): the de-duplication key's residue is measured over the example corpus, not sized by quoting a neighbouring pin (#18779) Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 Co-authored-by: Claude --- packages/cli/src/utils/artifact-packages.ts | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/utils/artifact-packages.ts b/packages/cli/src/utils/artifact-packages.ts index 826e9b8c01..0b93d661b8 100644 --- a/packages/cli/src/utils/artifact-packages.ts +++ b/packages/cli/src/utils/artifact-packages.ts @@ -242,11 +242,21 @@ export function packageBodyAsStack( * set the union could not see" — ⛔ do not restate it as that sentence. Two * entries rendering the same `where` still collapse (see `findingKey`), and a * rule that reports the same `rule`/`where`/`message` for genuinely different - * items distinguished ONLY by their top-level index would collapse too. That - * combination is not reachable through any rule in the registry today — every - * rule names its entity in `where`, which `packages/lint/src/ - * data-model-rule-where-slot.test.ts` holds for the whole registry — so the - * bound is structural, ⛔ not a measured count of survivors on some corpus. + * items distinguished ONLY by their top-level index would collapse with them. + * + * ⛔ And do not size that residue by quoting the pin next door — that move is + * exactly what this card exists to correct. `packages/lint/src/ + * data-model-rule-where-slot.test.ts` holds something NARROWER than "every + * rule names its entity in `where`": it runs the whole registry and fails any + * rule that puts a BARE CONFIG PATH in `where`. That forbids the one spelling + * which would make the collapse systematic; it does ⛔ not promise that two + * entries always render different `where` strings. So the residue is MEASURED + * instead — over every example stack in this repo that parses today + * (`app-multi-package`'s built artifact, `app-crm`, `app-showcase`, + * `app-todo`), 45 registry rules produced 103 findings and 103 distinct + * neutralised keys: ZERO groups held two different raw paths. ⛔ Re-measure + * rather than re-quote that number — a corpus reading is a count plus the tree + * it was taken against, and this one was taken on a43b9d0654. */ export function runPerPackageAuthoringRules(run: { /** Which door is asking — the same string its union run passed. */ From 10af34af930ddfa51cdbeed27e0bc718093ff6ab Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 03:13:00 +0000 Subject: [PATCH 4/6] chore(changeset): patch for the per-package de-duplication key correction (#18779) Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 Co-authored-by: Claude --- .../18779-per-package-dedup-positional-key.md | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 .changeset/18779-per-package-dedup-positional-key.md diff --git a/.changeset/18779-per-package-dedup-positional-key.md b/.changeset/18779-per-package-dedup-positional-key.md new file mode 100644 index 0000000000..e79004b387 --- /dev/null +++ b/.changeset/18779-per-package-dedup-positional-key.md @@ -0,0 +1,58 @@ +--- +'@objectstack/cli': patch +--- + +The per-package author-time de-duplication key ignores the top-level collection index, so a package-local finding no longer survives as an echo of the union finding it duplicates + +`runPerPackageAuthoringRules` runs the author-time rule table once per +`packages[]` entry and drops anything the union run already reported. Its key +was `rule` + `where` + `path` + `message`, and `path` is **positional**: a +package body re-bases every collection from 0, while the flattened union numbers +that same entry wherever `authoringRuleUnionStack` placed it. +`objects[0].fields.industry` and `objects[1].fields.industry` are ONE finding +under two spellings, so the `Set` never matched them and the echo survived the +filter that exists to remove it. + +Measured on `origin/main` a43b9d0654 over the repo's own two-package fixture +`examples/app-multi-package`, at every door, before and after: + +| | before | after | +|---|---|---| +| `os build --json` | warnings 4, exit 0 | warnings 3, exit 0 | +| `os validate --json` | warnings 4, exit 0 | warnings 3, exit 0 | +| `os lint --json` | total 4, failing 0, exit 0 | total 3, failing 0, exit 0 | +| `os lint --json --strict` | total 4, failing 4, exit 1 | total 3, failing 3, exit 1 | + +The one warning that stops being reported is `field-no-consumers` on +`crm_account.industry` re-reported at the package-local index — the union run's +own finding, printed a second time. Its twin is still reported, which is why no +verdict moves. + +**No input's verdict changes, and that is structural rather than a property of +this fixture.** Every finding the de-duplication drops has, by construction, a +finding carrying the same key already in the reported set: the seed is the union +run's findings, which every door reports, and it grows only with per-package +findings that themselves survived. So a door's refusal cannot flip — `os build` +already exits 1 on a union error before this pass runs, and `os lint --strict` +fails on `errors + warnings`, a count that could only reach zero if the twin +went unreported too. + +Only the **top-level** index is neutralised. Nested positions (`.indexes[1]`, +`.columns[0]`) address the author's own document and read identically in both +views, so they stay in the key and keep discriminating. A finding's own `path` +is never modified — every door still prints the location it always printed. + +What this does **not** buy: the key becomes position-insensitive, not +collision-proof. Two entries that render the same `where` still share a key, +exactly as they already did whenever their indices happened to match. Measured +over every example stack in this repo that parses today (`app-multi-package`'s +built artifact, `app-crm`, `app-showcase`, `app-todo`), 45 registry rules +produced 103 findings and 103 distinct neutralised keys — zero collisions. + +Also corrected: the sentence "what survives the filter is exactly the set the +union could not see", which was false for as long as the key was positional and +had been copied from `compile.ts` into the `os validate` and `os lint` doors as +each was wired. It is now stated at the bound the pass can actually hold, in +every file that carried it. + +Clause-②: no From 15cc0db8d4c1ae3c4d86cbae31f7de6320434b86 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 03:46:12 +0000 Subject: [PATCH 5/6] test(cli): the validate parity fixture carries a survivor the union genuinely cannot see, not an echo (#18779) The per-package survivor this pin relied on was an echo of a union finding. Once the de-duplication key stopped comparing the top-level index it was correctly filtered, the fixture's per-package set went empty and the non-vacuity case went red. The fixture now uses the falsifier shape its lint sibling already used, and the non-vacuity case asserts the survivor's pedigree rather than only its count. Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 Co-authored-by: Claude --- ...idate-per-package-authoring-parity.test.ts | 74 +++++++++++++++---- 1 file changed, 60 insertions(+), 14 deletions(-) diff --git a/packages/cli/test/validate-per-package-authoring-parity.test.ts b/packages/cli/test/validate-per-package-authoring-parity.test.ts index 31d3a67a4b..893dc2856e 100644 --- a/packages/cli/test/validate-per-package-authoring-parity.test.ts +++ b/packages/cli/test/validate-per-package-authoring-parity.test.ts @@ -33,19 +33,31 @@ * os build --json warnings: 4 <- 3 union + 1 per-package survivor * os validate --json warnings: 3 <- the survivor is the defect * - * ## The survivor on that fixture is an ECHO, and this file says so + * ## This file's fixture used to be carried by an ECHO — [#18779] it is not now * - * ⚠️ The de-duplication key includes the POSITIONAL `path`, and a collection - * index inside one package's own body is not the index the flattened top level - * gives the same item — so on `examples/app-multi-package` what survives is the - * union's own `crm_account.industry` finding re-reported at the package-local - * index, not something the union could not see. That is a separate defect in - * the key (filed, not fixed here — fixing it changes what `os build` reports, - * which is a different decision from making the two doors agree). It does not - * weaken these pins: whatever the pass produces, the assertion is that BOTH - * doors produce it, so the pins measure parity rather than the survivor's - * pedigree. The `where` prefix (`package '' — …`) is what identifies a - * per-package finding here, and it is the pass's own, not this file's guess. + * ⚠️ The version of `CONFIG_MULTI` this file shipped with had NO consumer for + * `pp_account.industry` anywhere, so the union run raised that finding too and + * the per-package survivor was the same finding re-reported at the + * package-local index. Once the de-duplication key stopped comparing the + * top-level index (#18779) that survivor was correctly filtered, the fixture's + * per-package set went EMPTY, and the non-vacuity case below went red — which + * is how the echo was discovered to be the only thing holding this pin up. + * + * ⇒ the fixture now carries the FALSIFIER shape instead, the same one + * `lint-per-package-authoring-parity.test.ts` uses: `core` owns `pp_account` + * and `orders` owns the view that displays `pp_account.industry`. Folded into + * one union the field HAS a consumer and nothing is raised; judged per package, + * `core` declares a field nothing in `core` reads. So the union run is clean + * and the per-package run is not — a survivor the union genuinely could not + * see, ⛔ not a duplicate of something it already reported. Measured on the new + * fixture: `os build --json` warnings 2, of which exactly 1 carries the + * per-package `where` prefix and no union finding names `industry` at all. + * + * ⭐ The pins themselves are unchanged and still measure PARITY rather than the + * survivor's pedigree — but a parity pin whose per-package set is empty + * measures nothing, so the pedigree is what decides whether the pin is alive. + * The `where` prefix (`package '' — …`) is what identifies a per-package + * finding here, and it is the pass's own, not this file's guess. * * ## Tier * @@ -108,6 +120,14 @@ const perPackageWarnings = (warnings: unknown[]): string[] => * owning `pp_account` and publishing the navigation container, plus a module * owning `pp_order`, whose `account` lookup points at the sibling's object — * legal under ADR-0130 §1.5, and the point of the shape. + * + * ⚠️ [#18779] `orders` also owns the VIEW that displays `pp_account.industry`, + * and that is load-bearing rather than decoration: it is what makes the union + * run clean while the per-package run is not, so the survivor these pins + * compare is one the union could not see. Without it the only survivor was an + * echo of a union finding, and it stopped surviving the moment the + * de-duplication key was corrected — leaving the pins below comparing two empty + * sets. ⛔ Do not remove the view to "simplify" the fixture. */ const CONFIG_MULTI = ` const coreManifest = { @@ -141,6 +161,16 @@ const ordersObjects = [{ account: { name: 'account', type: 'lookup', label: 'Account', reference: 'pp_account' }, }, }]; +const ordersViews = [ + { + name: 'pp_account_list', label: 'Account List', object: 'pp_account', + list: { label: 'Account List', columns: ['name', 'industry'] }, + }, + { + name: 'pp_order_list', label: 'Order List', object: 'pp_order', + list: { label: 'Order List', columns: ['name', 'account'] }, + }, +]; export default { // The ARTIFACT's own identity: \`preserve\` is additive, so the singular @@ -150,12 +180,13 @@ export default { manifest: coreManifest, objects: [...ordersObjects, ...coreObjects], apps: [...coreApps], + views: [...ordersViews], // …and the per-package view the runtime registers from (ADR-0130 D4/D5). Each // entry's \`manifest\` is that package ASSEMBLED — its manifest fields with the // collections it owns written over them — which is the superset // \`packageBodyAsStack\` reads back as one package's stack. packages: [ - { manifest: { ...ordersManifest, objects: ordersObjects } }, + { manifest: { ...ordersManifest, objects: ordersObjects, views: ordersViews } }, { manifest: { ...coreManifest, objects: coreObjects, apps: coreApps } }, ], }; @@ -212,7 +243,7 @@ describe('#18677 — `os validate` and `os build` report the same per-package ad for (const dir of Object.values(dirs)) if (dir) rmSync(dir, { recursive: true, force: true }); }); - it('the multi-package fixture reaches the pass at all — `os build` raises a per-package finding', async () => { + it('the multi-package fixture reaches the pass at all — and its survivor is NOT an echo', async () => { // Asserted BEFORE any claim about parity: a fixture that never reaches the // per-package pass makes every comparison below vacuous, which is the exact // way this defect stayed invisible. @@ -220,6 +251,21 @@ describe('#18677 — `os validate` and `os build` report the same per-package ad expect(build.code, `os build --json failed:\n${build.stdout}${build.stderr}`).toBe(0); const warnings = payloadOf(build, 'os build --json').warnings as unknown[]; expect(perPackageWarnings(warnings).length).toBeGreaterThan(0); + + // ⭐ [#18779] And the second half, which is what this file was missing: a + // per-package survivor that merely ECHOES a union finding keeps the count + // above zero while measuring nothing. `industry` must be raised ONLY behind + // the per-package prefix — if a union finding names it too, the fixture has + // drifted back to the shape whose survivor the de-duplication now (rightly) + // removes, and every parity case below is comparing two empty sets. + const named = (w: unknown): string => + typeof (w as { where?: unknown })?.where === 'string' ? (w as { where: string }).where : ''; + const industryWarnings = warnings.filter((w) => /industry/.test(named(w))); + expect(industryWarnings.length, 'the fixture no longer raises the per-package finding').toBe(1); + expect( + PER_PACKAGE_WHERE.test(named(industryWarnings[0])), + 'the `industry` finding is being raised by the UNION run — the survivor would be an echo', + ).toBe(true); }, 180_000); it('`os validate` reports every per-package advisory `os build` does', async () => { From 6a0a4df1b4e9e2f83578c2278f5985fc0053e74c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 04:35:55 +0000 Subject: [PATCH 6/6] test(cli): #18780's lit control is lit by a real survivor, not by the echo (#18779) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The multi-package fixture gave bc_account.industry no consumer anywhere, so the union run raised it too and the per-package 'survivor' was that same finding re-reported at the package-local index. Once the de-duplication key stopped comparing the top-level collection index the survivor count went to 0 and the precondition failed — correctly. `orders` now owns the view that displays the field, the same falsifier shape the two parity pins carry, and the precondition additionally asserts the survivor's pedigree so the control cannot be re-lit by a duplicate. Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 Co-authored-by: Claude --- .../build-text-face-advisory-count.test.ts | 55 ++++++++++++++++++- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/packages/cli/test/build-text-face-advisory-count.test.ts b/packages/cli/test/build-text-face-advisory-count.test.ts index 04074622ff..e6d1765c04 100644 --- a/packages/cli/test/build-text-face-advisory-count.test.ts +++ b/packages/cli/test/build-text-face-advisory-count.test.ts @@ -44,6 +44,30 @@ * per-package prefix is not something the command emits unconditionally, and * the equality holds there on a shape that never had the defect. * + * ⚠️ [#18779] That control was itself being lit by an ECHO, and it went red + * when the echo stopped surviving. `CONFIG_MULTI` gave `bc_account.industry` no + * consumer anywhere, so the union run raised that finding too and the + * per-package "survivor" was the same finding re-reported at the package-local + * index. The de-duplication key used to compare the POSITIONAL `path`, which is + * why a duplicate reached the list at all; once the key stopped comparing the + * top-level collection index the survivor count went to 0 and + * `toBeGreaterThan(0)` failed — correctly. + * + * ⇒ `orders` now owns the VIEW that displays `bc_account.industry`, the same + * falsifier shape `lint-per-package-authoring-parity.test.ts` and + * `validate-per-package-authoring-parity.test.ts` carry. Folded into one union + * the field HAS a consumer and nothing is raised; judged per package, `core` + * declares a field nothing in `core` reads. So the control is now lit by a + * survivor the union genuinely could not see, ⛔ not by a duplicate of + * something it already printed. ⛔ Do not remove the view to "simplify" the + * fixture, and ⛔ do not relax `toBeGreaterThan(0)` — that assertion is what + * stops every equality below it from going vacuous. + * + * ⚠️ The `warnings: 4` in the measurement block above is a reading of the + * DEFECT on `examples/app-multi-package` at 17.4.0 and is kept as the record of + * it. That same fixture reports 3 since #18779 removed the echo; the equality + * these pins assert is unaffected, because it was never a number. + * * ## Tier * * SPAWNS the CLI ⇒ INTEGRATION tier by `packages/cli/vitest-tiers.ts`' @@ -161,13 +185,24 @@ const ordersObjects = [{ account: { name: 'account', type: 'lookup', label: 'Account', reference: 'bc_account' }, }, }]; +const ordersViews = [ + { + name: 'bc_account_list', label: 'Account List', object: 'bc_account', + list: { label: 'Account List', columns: ['name', 'industry'] }, + }, + { + name: 'bc_order_list', label: 'Order List', object: 'bc_order', + list: { label: 'Order List', columns: ['name', 'account'] }, + }, +]; export default { manifest: coreManifest, objects: [...ordersObjects, ...coreObjects], apps: [...coreApps], + views: [...ordersViews], packages: [ - { manifest: { ...ordersManifest, objects: ordersObjects } }, + { manifest: { ...ordersManifest, objects: ordersObjects, views: ordersViews } }, { manifest: { ...coreManifest, objects: coreObjects, apps: coreApps } }, ], }; @@ -235,8 +270,24 @@ describe("#18780 — `os build`'s text face prints every advisory its summary li expect(multiText.code, `${multiText.stdout}\n${multiText.stderr}`).toBe(0); expect(multiText.stdout).toContain('Running author-time rules per package (2)'); expect(multiJson.code, `${multiJson.stdout}\n${multiJson.stderr}`).toBe(0); - const perPackage = perPackageWarnings(payloadOf(multiJson, 'os build --json').warnings as unknown[]); + const warnings = payloadOf(multiJson, 'os build --json').warnings as unknown[]; + const perPackage = perPackageWarnings(warnings); expect(perPackage.length).toBeGreaterThan(0); + + // ⭐ [#18779] And the survivor's PEDIGREE, because the count alone is what + // this control had before: it was lit by an ECHO of a union finding, and a + // duplicate keeps the count above zero while proving nothing about the pass. + // `industry` must be named ONLY behind the per-package prefix — a union + // finding naming it too means the fixture has drifted back to the shape + // whose survivor the de-duplication now correctly removes. + const named = (w: unknown): string => + typeof (w as { where?: unknown })?.where === 'string' ? (w as { where: string }).where : ''; + const industry = warnings.filter((w) => /industry/.test(named(w))); + expect(industry.length, 'the fixture no longer raises the per-package finding').toBe(1); + expect( + PER_PACKAGE_WHERE.test(named(industry[0])), + 'the `industry` finding is being raised by the UNION run — this control would be lit by an echo', + ).toBe(true); }); it('the summary line counts exactly what the list above it renders', () => {