diff --git a/.changeset/18778-lint-per-package-authoring-pass.md b/.changeset/18778-lint-per-package-authoring-pass.md new file mode 100644 index 00000000000..acd6d6207e6 --- /dev/null +++ b/.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) + + diff --git a/packages/cli/src/commands/lint.ts b/packages/cli/src/commands/lint.ts index 5c0e752dcbd..5b2eb1af023 100644 --- a/packages/cli/src/commands/lint.ts +++ b/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,7 +670,7 @@ 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); - for (const f of runAuthoringRules('lint', { + const unionFindings = runAuthoringRules('lint', { normalized: stack, parsed: lowered, sduiManifest: opts.sduiManifest, @@ -674,7 +678,65 @@ export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue // 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, diff --git a/packages/cli/src/utils/artifact-packages.ts b/packages/cli/src/utils/artifact-packages.ts index c7740598214..af0f3a0554c 100644 --- a/packages/cli/src/utils/artifact-packages.ts +++ b/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 }; } diff --git a/packages/cli/test/lint-per-package-authoring-parity.test.ts b/packages/cli/test/lint-per-package-authoring-parity.test.ts new file mode 100644 index 00000000000..40c9cfdb937 --- /dev/null +++ b/packages/cli/test/lint-per-package-authoring-parity.test.ts @@ -0,0 +1,289 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #18778, the BEHAVIOURAL half — `os lint` and `os build` report the same + * author-time finding set for a MULTI-PACKAGE project, and the `--strict` exit + * that only the real binary can be asked about. + * + * `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. + * + * ## Why this file exists next to the in-process seam pin + * + * `lint-per-package-authoring-seam.test.ts` reaches `lintConfig` directly, which + * is where the findings are produced — and that is exactly what it CANNOT + * answer about: `os lint`'s verdict is `failing = errors + (strict ? warnings : + * 0)`, computed in `run()`, above the two faces and below no test that does not + * spawn. The NARROWING this card lands is a `--strict` exit, so the pin for it + * has to be a process. + * + * ## The reading this file was written from + * + * On `origin/main` 7572329069, over `CONFIG_FLIP` below — a project whose union + * run raises NOTHING and whose per-package run raises one advisory: + * + * os build --json warnings 1 <- per-package only + * os lint --json total 0 ✓ exit 0 + * os lint --json --strict total 0 ✓ exit 0 <- the false clean + * + * and after: `os lint --json` total 1, `os lint --json --strict` exit 1. That + * exit is a NEWLY-REFUSED INPUT on this door, exhibited rather than reasoned + * about — ⛔ #18677's `Clause-②: no` was measured on `os validate`'s door and + * does not transfer. + * + * ## Tier + * + * SPAWNS the CLI ⇒ INTEGRATION tier by `packages/cli/vitest-tiers.ts`' predicate + * (`childProcess` + `helperCliOrTsx`). The filename carries no `.e2e` segment, + * so by the orthogonal NIGHTLY cut it is a QUEUE-tier file — the combination + * that module names as deliberate. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFile } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync, mkdirSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { CLI, TSX, childEnv } from './helpers/serve-process.js'; + +interface Run { + code: number; + stdout: string; + stderr: string; +} + +function runCli(args: string[], cwd: string): Promise { + return new Promise((resolvePromise) => { + execFile( + TSX, + [CLI, ...args], + { cwd, maxBuffer: 16 * 1024 * 1024, env: childEnv({ NO_COLOR: '1' }) }, + (err, stdout, stderr) => { + resolvePromise({ + code: err ? (typeof (err as { code?: unknown }).code === 'number' ? (err as unknown as { code: number }).code : 1) : 0, + stdout: String(stdout), + stderr: String(stderr), + }); + }, + ); + }); +} + +function payloadOf(run: Run, label: string): Record { + try { + return JSON.parse(run.stdout) as Record; + } catch { + throw new Error(`${label}: stdout was not one JSON document (exit ${run.code})\n${run.stdout}\n${run.stderr}`); + } +} + +/** The `where` prefix `runPerPackageAuthoringRules` puts on every finding it raises. */ +const PER_PACKAGE_WHERE = /^package '[^']+' — /; + +/** + * `os build` publishes `where` as its own key; `os lint` folds it into the head + * of `message` (`${where}: ${message}`) and always has. Two readers, ONE prefix + * — which is why the comparison below is on the prefix + the rule id and not on + * a whole rendered string the two faces were never required to share. + */ +const buildPerPackage = (warnings: unknown[]): string[] => + warnings + .filter((w): w is { where: string; rule: string } => typeof (w as { where?: unknown })?.where === 'string') + .filter((w) => PER_PACKAGE_WHERE.test(w.where)) + .map((w) => `${w.rule} @ ${w.where}`) + .sort(); + +const lintPerPackage = (issues: unknown[]): string[] => + issues + .filter((i): i is { message: string; rule: string } => typeof (i as { message?: unknown })?.message === 'string') + .filter((i) => PER_PACKAGE_WHERE.test(i.message)) + .map((i) => `${i.rule} @ ${i.message.slice(0, i.message.indexOf(': '))}`) + .sort(); + +/** + * The falsifier. `core` owns `pp_account`; `orders` owns the view that displays + * `pp_account.industry`. Judged as one flattened union the field has a consumer + * and nothing is raised; judged per package, `core` declares a field nothing in + * `core` reads. ⇒ the union run is CLEAN and the per-package run is not, which + * is the one shape that can tell "the doors agree" from "the doors agree because + * neither of them looked". + */ +const CONFIG_FLIP = ` +const coreManifest = { + id: 'com.example.ppflip.core', name: 'ppflip core', namespace: 'pp', + version: '1.0.0', type: 'app', engines: { protocol: '^17' }, +}; +const coreObjects = [{ + 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 coreApps = [{ + 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 ordersManifest = { + id: 'com.example.ppflip.orders', name: 'ppflip orders', namespace: 'pp', + version: '1.0.0', type: 'module', engines: { protocol: '^17' }, + dependencies: { 'com.example.ppflip.core': '^1.0.0' }, +}; +const ordersObjects = [{ + 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' }, + }, +}]; +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 { + manifest: coreManifest, + objects: [...ordersObjects, ...coreObjects], + apps: [...coreApps], + views: [...ordersViews], + packages: [ + { manifest: { ...ordersManifest, objects: ordersObjects, views: ordersViews } }, + { manifest: { ...coreManifest, objects: coreObjects, apps: coreApps } }, + ], +}; +`; + +/** + * The CONTROL: the same kind of project as ONE package, no `packages[]`. The + * pass is skipped on every door, so agreement here holds for a reason that has + * nothing to do with this change — which is precisely how the gap survived two + * cards' worth of parity files. + */ +const CONFIG_SINGLE = ` +export default { + manifest: { + id: 'com.example.ppsingle', name: 'ppsingle', namespace: 'ps', + version: '1.0.0', type: 'app', engines: { protocol: '^17' }, + }, + objects: [{ + name: 'ps_thing', label: 'Thing', pluralLabel: 'Things', sharingModel: 'private', + fields: { + name: { name: 'name', type: 'text', label: 'Name', required: true }, + unused: { name: 'unused', type: 'text', label: 'Unused' }, + }, + }], + apps: [{ + name: 'ps_app', label: 'PS App', + navigation: [{ id: 'nav_things', type: 'object', objectName: 'ps_thing', label: 'Things' }], + }], +}; +`; + +const dirs = { flip: '', single: '' }; + +function plant(config: string): string { + const dir = mkdtempSync(join(tmpdir(), 'os-lintpp-')); + mkdirSync(join(dir, 'src'), { recursive: true }); + writeFileSync(join(dir, 'objectstack.config.ts'), config, 'utf8'); + writeFileSync( + join(dir, 'package.json'), + JSON.stringify({ name: 'lintpp-fixture', private: true, type: 'module' }, null, 2), + 'utf8', + ); + return dir; +} + +describe('#18778 — `os lint` and `os build` report the same per-package finding set', () => { + beforeAll(() => { + dirs.flip = plant(CONFIG_FLIP); + dirs.single = plant(CONFIG_SINGLE); + }); + + afterAll(() => { + for (const dir of Object.values(dirs)) if (dir) rmSync(dir, { recursive: true, force: true }); + }); + + it('the fixture reaches the pass at all, and the UNION sees nothing — `os build` raises exactly the survivor', async () => { + // Asserted BEFORE any claim about parity: a fixture that never reaches the + // per-package pass makes every comparison below vacuous, and a fixture whose + // union run raises the same finding makes the comparison an echo count. + const build = await runCli(['build', '--json'], dirs.flip); + 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(buildPerPackage(warnings).length).toBeGreaterThan(0); + // Nothing else: every warning on this fixture is a per-package survivor. + expect(warnings.length).toBe(buildPerPackage(warnings).length); + }, 180_000); + + it('`os lint` reports every per-package finding `os build` does', async () => { + // The pin. On `origin/main` 7572329069 this read `build: 1, lint: 0`. + const build = await runCli(['build', '--json'], dirs.flip); + const lint = await runCli(['lint', '--json'], dirs.flip); + expect(build.code, `os build --json failed:\n${build.stdout}${build.stderr}`).toBe(0); + expect(lint.code, `os lint --json failed:\n${lint.stdout}${lint.stderr}`).toBe(0); + + const inLint = new Set(lintPerPackage(payloadOf(lint, 'os lint --json').issues as unknown[])); + const missing = buildPerPackage(payloadOf(build, 'os build --json').warnings as unknown[]) + .filter((w) => !inLint.has(w)); + expect( + missing, + 'these per-package findings ride `os build` and `os lint` cannot see them — the #18778 false-clean set', + ).toEqual([]); + }, 180_000); + + it('…and nothing per-package rides `os lint` that `os build` does not report either', async () => { + // Parity is an equality. A lint that over-reports is its own defect — an + // author fixing a finding the command that SHIPS never raises. + const build = await runCli(['build', '--json'], dirs.flip); + const lint = await runCli(['lint', '--json'], dirs.flip); + const inBuild = new Set(buildPerPackage(payloadOf(build, 'os build --json').warnings as unknown[])); + expect(lintPerPackage(payloadOf(lint, 'os lint --json').issues as unknown[]).filter((w) => !inBuild.has(w))).toEqual([]); + }, 180_000); + + it('⚠️ THE NARROWING — `os lint --strict` now EXITS 1 on a project it exited 0 for', async () => { + // The Clause-② evidence, pinned so it can neither regress silently nor widen + // silently. `--strict` is documented as "treat warnings as errors", and a + // per-package advisory is a warning this door could not see before, so the + // verdict moves with it. ⛔ The DEFAULT face is deliberately NOT asserted to + // move: no `error`-severity per-package-only finding was exhibited on this + // fixture, and a pin asserting one would be asserting something unmeasured. + const strict = await runCli(['lint', '--json', '--strict'], dirs.flip); + const payload = payloadOf(strict, 'os lint --json --strict'); + expect(payload.strict).toBe(true); + expect(payload.failing).toBe(1); + expect(payload.passed).toBe(false); + expect(strict.code, 'the --strict exit must follow `failing`').toBe(1); + + // …and the same project without `--strict` still exits 0, so what moved is + // the strict verdict and not the default one. + const plain = await runCli(['lint', '--json'], dirs.flip); + expect(plain.code).toBe(0); + expect(payloadOf(plain, 'os lint --json').passed).toBe(true); + }, 180_000); + + it('CONTROL — a single-package project raises NO per-package finding on either door', async () => { + // "Present" must be distinguishable from "always present": the prefix this + // file matches on is not something either command emits unconditionally, and + // `os lint --strict` still exits 1 there for the ORDINARY warning, which is + // how the case above is read as a change and not as a constant. + const build = await runCli(['build', '--json'], dirs.single); + const lint = await runCli(['lint', '--json'], dirs.single); + expect(build.code, `os build --json failed:\n${build.stdout}${build.stderr}`).toBe(0); + expect(lint.code, `os lint --json failed:\n${lint.stdout}${lint.stderr}`).toBe(0); + expect(buildPerPackage(payloadOf(build, 'os build --json').warnings as unknown[])).toEqual([]); + expect(lintPerPackage(payloadOf(lint, 'os lint --json').issues as unknown[])).toEqual([]); + }, 180_000); +}); diff --git a/packages/cli/test/lint-per-package-authoring-seam.test.ts b/packages/cli/test/lint-per-package-authoring-seam.test.ts new file mode 100644 index 00000000000..684b9ad1b3a --- /dev/null +++ b/packages/cli/test/lint-per-package-authoring-seam.test.ts @@ -0,0 +1,266 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #18778 — `os lint` is the THIRD door that ran the author-time rule table over + * the union fold and stopped, and #18677's table called the asymmetry 1 of 2. + * + * `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` + * structurally could not — FALSE-CLEAN, on the fastest of the three doors. + * + * ## ⭐ Why symbol presence scored this door as covered + * + * `lint.ts` DOES import `artifactPackages` and `packageBodyAsStack`. Opening + * the hits is what settles it: they feed `os lint`'s OWN intra-package + * duplicate-name advisory (#17821), never the shared rule table. ⛔ A count is + * not a reading, and the last case in this file is the ratchet that keeps the + * distinction mechanical at this door — the sibling doors get it for free from + * `validate-per-package-authoring-seam.test.ts`' "names it at all" assertion, + * which ⛔ cannot be transplanted here, because this door legitimately names the + * seam once. + * + * ## What this file pins, and what its sibling pins + * + * This one is the SEAM plus the in-process verdict: the pass exists once, all + * THREE doors reach it, it answers the same for each, and `lintConfig` now + * reports what the union could not see. It spawns nothing and boots no kernel, + * so it is UNIT tier (`packages/cli/vitest-tiers.ts`). + * `lint-per-package-authoring-parity.test.ts` is the behavioural half — the + * real binaries, and the `--strict` EXIT the in-process function cannot reach — + * and lands in the INTEGRATION tier because it spawns the CLI. + */ + +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { defineStack, composeStacks } from '@objectstack/spec'; +import { artifactPackages, runPerPackageAuthoringRules } from '../src/utils/artifact-packages.js'; +import { lintConfig } from '../src/commands/lint.js'; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const COMMANDS = resolve(HERE, '../src/commands'); +const sourceOf = (file: string) => readFileSync(join(COMMANDS, file), 'utf8'); + +/** All three doors of the one wall (`@objectstack/lint`'s `authoring-rules.ts`). */ +const DOORS = ['compile.ts', 'validate.ts', 'lint.ts'] as const; + +/** The `where` prefix `runPerPackageAuthoringRules` owns. */ +const PER_PACKAGE = /^package '[^']+' — /; + +/** + * A two-package artifact whose union run is CLEAN of the finding its + * per-package run raises. + * + * 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 + * sibling file's. + */ +function twoPackageArtifact(): Record { + const core = defineStack({ + manifest: { + id: 'com.example.lintseam.core', + name: 'Lint Seam Core', + namespace: 'pp', + version: '1.0.0', + type: 'app', + engines: { protocol: '^17' }, + }, + 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' }, + }, + }, + ], + 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 = defineStack({ + manifest: { + id: 'com.example.lintseam.orders', + name: 'Lint Seam Orders', + namespace: 'pp', + version: '1.0.0', + type: 'module', + engines: { protocol: '^17' }, + dependencies: { 'com.example.lintseam.core': '^1.0.0' }, + }, + 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' }, + }, + }, + ], + views: [ + { + 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'] }, + }, + ], + }); + return composeStacks([orders, core], { manifest: 'preserve' }) as unknown as Record; +} + +/** The same metadata as ONE package — no `packages[]`, so the pass is skipped. */ +function singlePackageStack(): Record { + return defineStack({ + manifest: { + id: 'com.example.lintseam.single', + name: 'Lint Seam Single', + namespace: 'ps', + version: '1.0.0', + type: 'app', + engines: { protocol: '^17' }, + }, + objects: [ + { + name: 'ps_thing', + label: 'Thing', + pluralLabel: 'Things', + sharingModel: 'private', + fields: { name: { name: 'name', type: 'text', label: 'Name', required: true } }, + }, + ], + }) as unknown as Record; +} + +describe('#18778 — the per-package author-time pass is ONE seam all THREE doors reach', () => { + it('all three authoring commands CALL the shared pass', () => { + for (const door of DOORS) { + expect( + sourceOf(door), + `${door} must run the per-package author-time pass — its absence is the #18677/#18778 ` + + `false-clean gap, and the door it is absent from is the bar the whole wall is held to`, + ).toMatch(/\brunPerPackageAuthoringRules\s*\(/); + } + }); + + it('the pass answers IDENTICALLY for all three doors — same survivors, same order', () => { + // Every gating entry in the registry is `commands: ALL`, so a door-dependent + // answer here would mean the three commands hold one artifact to three bars. + // #18677 pinned this for two doors; the third is the card. + const parsed = twoPackageArtifact(); + expect(artifactPackages(parsed).length, 'fixture must carry `packages[]`').toBe(2); + + const forDoor = (command: 'build' | 'validate' | 'lint') => + runPerPackageAuthoringRules({ command, parsed, unionFindings: [] }); + + const build = forDoor('build'); + for (const command of ['validate', 'lint'] as const) { + const other = forDoor(command); + expect(other.packageCount, `${command} walked a different package count`).toBe(build.packageCount); + expect(other.findings, `${command} produced a different finding list`).toEqual(build.findings); + expect(other.errors).toEqual(build.errors); + expect(other.advisories).toEqual(build.advisories); + } + }); + + it('`findings` is the SAME set as `errors` ∪ `advisories`, in walk order', () => { + // The member `os lint` reads. It must be the one loop's output and not a + // second computation: a `findings` that could differ from the split is a + // door-dependent verdict wearing one function's name. + const run = runPerPackageAuthoringRules({ + command: 'lint', + parsed: twoPackageArtifact(), + unionFindings: [], + }); + expect(run.findings.length, 'NON-VACUITY — the pass produced nothing on this fixture').toBeGreaterThan(0); + expect(run.findings.length).toBe(run.errors.length + run.advisories.length); + expect(run.findings.every((f) => PER_PACKAGE.test(f.where))).toBe(true); + // Every split member is a findings member, compared on the finding itself — + // `errors` carries the extra `package` key the two artifact doors render. + const flat = new Set(run.findings.map((f) => JSON.stringify(f))); + for (const a of run.advisories) expect(flat.has(JSON.stringify(a))).toBe(true); + for (const { package: _pkg, ...e } of run.errors) expect(flat.has(JSON.stringify(e))).toBe(true); + }); + + it('`os lint` now reports a finding the union run could NOT see', () => { + // The card, measured in process. Before this change `lintConfig` returned + // ZERO issues carrying the per-package prefix on this fixture, and the + // `industry` finding appeared on NEITHER face — the union cannot see it and + // `os lint` did not run the leg that can. + const issues = lintConfig(twoPackageArtifact()); + const perPackage = issues.filter((i) => PER_PACKAGE.test(i.message)); + expect( + perPackage.map((i) => i.rule), + 'os lint reported no per-package finding at all — the #18778 gap', + ).toContain('field-no-consumers'); + + // …and it is a survivor, not an echo: the union run raised nothing about + // this field, which is what makes the fixture a falsifier rather than a + // duplicate counter. + const industry = (i: { rule: string; message: string }) => + i.rule === 'field-no-consumers' && i.message.includes('"industry"'); + expect(issues.filter((i) => industry(i) && !PER_PACKAGE.test(i.message))).toEqual([]); + expect(issues.filter((i) => industry(i) && PER_PACKAGE.test(i.message)).length).toBe(1); + }); + + it('CONTROL — a single-package stack raises no per-package finding through `os lint`', () => { + // "Present" must be distinguishable from "always present". A stack with no + // `packages[]` is one package by definition: the union run judged it whole, + // the pass is skipped, and `packageCount` says so. + const single = singlePackageStack(); + const run = runPerPackageAuthoringRules({ command: 'lint', parsed: single, unionFindings: [] }); + expect(run.packageCount).toBe(0); + expect(run.findings).toEqual([]); + expect(lintConfig(single).filter((i) => PER_PACKAGE.test(i.message))).toEqual([]); + }); + + it('⛔ `lint.ts` names `packageBodyAsStack` exactly ONCE — a second call IS a second loop', () => { + // The sibling file asserts the two artifact doors name this seam NEVER. + // ⛔ That assertion cannot be transplanted: this door legitimately calls it, + // for the #17821 intra-package duplicate-name advisory, which is `os lint`'s + // OWN rubric. So the ratchet here is the COUNT — and the reason it is worth + // a case of its own is that this file is the one place in the tree where + // "the loop is already written here, write the second one beside it" is the + // cheap move. What drifts between two hand-written loops is the VERDICT (the + // de-duplication key, the severity split, the `where` prefix), never the + // package reading — `utils/artifact-packages.ts`' header forbids it by name. + const calls = sourceOf('lint.ts').replace(/`packageBodyAsStack`/g, '').match(/\bpackageBodyAsStack\s*\(/g) ?? []; + expect( + calls.length, + `lint.ts calls packageBodyAsStack ${calls.length} time(s). Exactly one is sanctioned — the ` + + `#17821 duplicate-name advisory. A second call means the shared rule table is being walked ` + + `by a loop this file wrote instead of by runPerPackageAuthoringRules().`, + ).toBe(1); + }); +}); diff --git a/packages/cli/test/validate-build-gate-parity.test.ts b/packages/cli/test/validate-build-gate-parity.test.ts index 840145e55f5..16050b4e64d 100644 --- a/packages/cli/test/validate-build-gate-parity.test.ts +++ b/packages/cli/test/validate-build-gate-parity.test.ts @@ -225,10 +225,26 @@ const NOT_A_GATE: Readonly> = { const NOT_A_GATE_NAMES: ReadonlySet = new Set(Object.values(NOT_A_GATE).flat()); /** - * The two doors this file holds equal. `lint.ts` is the third authoring - * command and is held to the registry by the checks further down, but it emits - * no artifact and runs no artifact-level gate, so it is not part of the parity - * question. + * The two doors this file holds equal. `lint.ts` is the third authoring command + * and is held to the registry by the checks further down; it emits no artifact, + * so it is not part of THIS file's parity question — which is the one its + * header states, `os validate` as the read-only superset of `os build`. + * + * ⚠️ [#18778] That clause used to read "and runs no artifact-level gate", and + * it was FALSE on the tree when it was written: `lint.ts` has called + * `collectAndLintDocs` — a name in {@link SHARED_NON_REGISTRY_GATES} above — + * since long before it, and since #18778 it calls + * `runPerPackageAuthoringRules` as well. The sentence is corrected rather than + * deleted because it is the one statement in this repository that could be read + * as "`os lint` is exempt from the artifact-level gates", and #18778's dispatch + * had to settle exactly that question before wiring the third door. What + * excludes `lint.ts` from PARITY_COMMANDS is the ARTIFACT, not the gates. + * + * ⛔ Do NOT answer that by adding `lint.ts` here. The rosters below are keyed to + * the two commands' call sites and the ledgers classify exactly those; widening + * the constant would re-open every classification against a third file in the + * same stroke, which is a card, not a line. `AUTHORING_COMMANDS` is the list + * that means all three, and the checks that belong to all three already use it. */ const PARITY_COMMANDS: readonly string[] = ['compile.ts', 'validate.ts'];