diff --git a/.changeset/18095-retire-reference-carrier-shape-gate.md b/.changeset/18095-retire-reference-carrier-shape-gate.md new file mode 100644 index 00000000000..4c88331622b --- /dev/null +++ b/.changeset/18095-retire-reference-carrier-shape-gate.md @@ -0,0 +1,26 @@ +--- +"@objectstack/spec": minor +"@objectstack/lint": minor +--- + +A `reference` carrier that no reader can read is now **REFUSED** where it is read, instead of coming back as `undefined`. The source-level gate that guarded the same shape (`check:reference-carrier-shape`) is retired in the same change (#18095, executing a maintainer ruling). + +`FieldSchema.reference` is `z.string().optional()`, so `ObjectSchema.safeParse` already refuses an object- or array-valued carrier at the contract door with a located `invalid_type` issue. Measured on the pre-change tree: + +``` +ObjectSchema.safeParse({ fields: { invoice: { type: 'lookup', + reference: { object: 'shop_invoice' } } } }) + -> success = false, issue invalid_type at path ["fields","invoice","reference"] +control: the same object with reference: 'shop_invoice' + -> success = true (so the refusal is about the carrier's SHAPE) +``` + +What was missing was the other door — the one a value reaches only when it never went through parse at all. #13053's fixture spelled `reference: { object: … }` inside `fields:`, and the rule reading it answered `undefined`: refused where it was written, read as absent where it was consumed, reported nowhere. The fixture passed, and would have kept passing. + +**New export — `referenceCarrierOf(def, reader?)` in `@objectstack/spec/data`.** It answers the carrier as the string the contract declares, and throws a `TypeError` naming the shape and the fix when the key is present in any other shape. `null`, `undefined` and `''` are ABSENCE, not a wrong shape, and still answer `undefined` — a field is allowed to name no target. + +**`referenceTargetOf` reads through it**, so the single arbiter of "what does this field expand into" refuses rather than answering "no target". Every consumer that already asks the arbiter — `$expand`, the record-title deriver, the dangling-reference audit, the analytics dimension labeller — inherits the refusal with no edit. + +**`@objectstack/lint`** routes its own target readers through the same accessor: `refOf` in `validate-security-posture.ts` (the reader in the #13053 incident) and in `data-model-rules.ts`, plus the object-graph slice every other rule downstream reads. + +Upgrading: nothing conformant changes. A non-string `reference` could not be authored, stored or parsed before this release either; what changes is that a hand-built fixture or a raw registry entry carrying one now fails loudly at the read instead of being silently treated as targetless. If a test asserted the old silence, assert the refusal instead — `packages/cli/test/data-model-rules.test.ts` is the worked example. diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 185392dfd30..c509fd78614 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -4630,38 +4630,6 @@ jobs: - name: workspace manifest dependency graph has no cycle run: pnpm check:workspace-manifest-cycles - # A relationship carrier is spelled as the target object's NAME (#13103). - # `FieldSchema.reference` is `z.string()`, and #13053 measured what a - # non-string one costs: a fixture spelling `reference: { object: '...' }` - # inside `fields:` is refused by `ObjectSchema.safeParse` AND read as - # `undefined` by the rule resolving it, so the suite was blind in both - # directions and the test still passed. - # - # ⛔ Deliberately NOT the broad guard that first suggests itself. Running - # every object fixture through `ObjectSchema.safeParse` was measured at - # 838 fixtures across 303 files going red, and those fixtures are invalid - # ON PURPOSE — the same carve-out `check:query-options-erasure` prints for - # itself: a rejection test must be able to build off-contract input. So - # the population judged here is the CARRIER, never the fixture. - # - # Lands as a ZERO-BASELINE ratchet: re-measured before landing at 443 - # string carriers and 0 object/array ones at a field-def position, so - # there is no grandfathering file and nowhere to park the next defect. - # Two words keep it narrow, and the self-test is where both are observed: - # only a LITERAL is judged (identifiers, pass-throughs, Zod declarations - # and `reference: null` stay unjudged), and only at a field-def carrier - # POSITION (the generated i18n bundles write `reference:` as a field NAME, - # and are excluded BY SHAPE, never by a path ignore that would rot). - # A position it cannot resolve REFUSES with exit 3 rather than counting as - # "not a field def", and so does a run that measures zero carriers — - # a green whose success condition equals its total-failure condition. - # - # Reads source with scripts/ts-parse.mjs; no build, no spawns, ~5s. - - name: Relationship carriers are spelled as the string the spec declares - run: | - node packages/lint/scripts/check-reference-carrier-shape.mjs --self-test - node packages/lint/scripts/check-reference-carrier-shape.mjs - # #15149. Grep-level guard for the defect this very repo just shipped: an # unquoted `- name:` step name containing ` #` is silently truncated by # YAML at that point (a space + hash starts a comment inside a plain diff --git a/packages/cli/test/data-model-rules.test.ts b/packages/cli/test/data-model-rules.test.ts index c6019a3610c..d8ecfcd2615 100644 --- a/packages/cli/test/data-model-rules.test.ts +++ b/packages/cli/test/data-model-rules.test.ts @@ -780,32 +780,51 @@ describe('lintDataModel — `reference_to` is a rejected alias, not a tolerated ).toBe(false); }); - it('a non-string `reference` is not a target either', () => { - // `refOf` is declared `string | undefined`; the old `||` chain returned - // whatever truthy value was there, so this shape used to be reported as a - // resolved target named `[object Object]`. + it('a non-string `reference` is REFUSED, loudly, rather than read as no target', () => { + // #13053's whole defect, and the assertion this case exists for, INVERTED on + // purpose. `refOf` used to answer `undefined` for a non-string carrier, so a + // fixture spelling `reference: { object: … }` was invisible in both + // directions at once: refused by `ObjectSchema.safeParse` where it was + // written, read as absent where it was consumed, and therefore reported + // nowhere. This case used to pin the SECOND half of that silence — it + // asserted the shape resolved to nothing and produced `missing-reference`, + // which is a finding about the wrong thing: the target is not missing, it is + // unreadable, and the two want different fixes from the author. // - // ⚠️ The non-string value is BOUND THROUGH A VARIABLE rather than written - // inline, and the binding is a legibility device — not a way past a gate. - // `packages/lint/scripts/check-reference-carrier-shape.mjs` refuses a - // non-string LITERAL at a `reference` carrier position, and it is right to: - // an AUTHORED site of that shape is invisible in both directions at once — - // refused by `ObjectSchema.safeParse` and read as `undefined` by every rule - // that resolves it (#13053), so it reports nothing either way. + // The reader now throws (`referenceCarrierOf`, `@objectstack/spec/data`), so + // a fixture that cannot be read fails its test instead of passing it. The + // gate that used to guard this shape at the source level + // (`check:reference-carrier-shape`) is retired: it caught zero in its + // lifetime, its coverage was partial by its own header, and the protocol + // already refuses the shape at the contract door. // - // That shape is exactly what this case must keep driving, because the whole - // assertion is that such a value resolves to NOTHING: it is a deliberate - // counter-example, not an authored carrier. The gate judges literals and - // leaves a non-literal unjudged, so the binding keeps its authored-site - // sweep honest while the assertion goes on testing the identical shape. - // ⛔ Do not inline this value again; ⛔ do not weaken the gate or add a path - // ignore there — it has no baseline and wants none, by design. + // ⛔ Not a bare `toThrow()`: an unrepaired reader that threw some other + // `Error` on some other input would satisfy that. The class and the sentence + // the author actually reads are both asserted. const nonStringReference = { object: 'project' }; + const lintWithObjectCarrier = () => + lintDataModel([ + { name: 'task', fields: { project: { type: 'lookup', reference: nonStringReference } } }, + ]); + expect(lintWithObjectCarrier).toThrow(TypeError); + expect(lintWithObjectCarrier).toThrow(/`reference` is an object/); + expect(lintWithObjectCarrier).toThrow(/FieldSchema declares it as an optional STRING/); + + // CONTROL — the identical object with a STRING carrier reads fine and reaches + // the ordinary rules, so the throw above is about the carrier's SHAPE and not + // about this fixture, this field type, or `lintDataModel` refusing to run. + const stringCarrier = lintDataModel([ + { name: 'project', fields: { name: { type: 'text', label: 'Name' } } }, + { name: 'task', fields: { project: { type: 'lookup', reference: 'project' } } }, + ]); + expect(has(stringCarrier, 'relationship/missing-reference')).toBe(false); + + // CONTROL — an ABSENT carrier is still the ordinary `missing-reference` + // finding, not a throw. Absence and unreadability are different answers and + // the reader must keep telling them apart. expect( has( - lintDataModel([ - { name: 'task', fields: { project: { type: 'lookup', reference: nonStringReference } } }, - ]), + lintDataModel([{ name: 'task', fields: { project: { type: 'lookup' } } }]), 'relationship/missing-reference', ), ).toBe(true); diff --git a/packages/lint/scripts/check-reference-carrier-shape.mjs b/packages/lint/scripts/check-reference-carrier-shape.mjs deleted file mode 100644 index 37270da841d..00000000000 --- a/packages/lint/scripts/check-reference-carrier-shape.mjs +++ /dev/null @@ -1,584 +0,0 @@ -#!/usr/bin/env node -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * check-reference-carrier-shape -- the relationship-CARRIER ratchet (#13103). - * - * node packages/lint/scripts/check-reference-carrier-shape.mjs # scan - * node packages/lint/scripts/check-reference-carrier-shape.mjs --census # site table - * node packages/lint/scripts/check-reference-carrier-shape.mjs --self-test # both directions - * - * Run from the repo root, the GATE INVOCATION IDIOM lint.yml states once at its - * top. It lives under `packages/lint/` because that package owns the authoring - * lane this judges, beside the two docs gates that already follow the same - * shape; it needs no build and no package import, so it runs in the pre-build - * `Lint & Repo Gates` job rather than in its siblings' post-build lane. - * - * ## The predicate, and the two words that keep it honest - * - * a `reference` whose value is a LITERAL must be a STRING literal, - * at a field-def carrier position. - * - * LITERAL is the first word. This is NOT "a `reference` must be a string" -- - * that wider rule would judge the spellings a source scan cannot evaluate - * (identifiers holding string constants, runtime pass-throughs, the Zod - * declarations that DEFINE the key) and would be wrong about every one of - * them. Those stay UNJUDGED here, and they are counted rather than dropped so - * the census says how much of the population the gate declined to judge. - * - * POSITION is the second. `reference` is an ordinary English word and this - * tree writes it as a map key, a test's expected-result slot, and a - * translation entry. Judging by the key name alone reds four generated i18n - * bundles and three driver assertions on the day it lands. - * - * ## The defect class, and why the BROAD version of this gate was refused - * - * #13053: a runtime-gate fixture spelled `reference: { object: 'shop_invoice' }` - * inside `fields:`. `FieldSchema.reference` is `z.string().optional()`, so - * `ObjectSchema.safeParse` refuses that carrier -- and the rule the fixture - * exercises does not read it either (`validate-security-posture.ts` `refOf()` - * returns `undefined` for a non-string), so the suite was blind in both - * directions at once. The fixture passed, and would have kept passing if a - * later assertion had come to depend on the parent it silently could not see. - * - * The obvious guard -- run every object fixture through `ObjectSchema.safeParse` - * -- was measured and REFUSED (#13103): **838 fixtures across 303 files** would - * red, and they are invalid ON PURPOSE. A fixture that parses cleanly cannot - * drive a rule that fires on malformed input. This repo has already adjudicated - * exactly that carve-out, in `check:query-options-erasure`'s own printed text: - * a rejection test must be able to build off-contract input. - * - * So the population this gate judges is the CARRIER, not the fixture. A - * deliberately-invalid fixture stays free to be invalid in the ways its rule - * exists to judge; what it may not do is carry a relationship target in a shape - * no reader in the tree can read. - * - * ## Zero-baseline ratchet -- no grandfathering file, and none is wanted - * - * Re-measured on this tree before landing: 0 object-valued and 0 array-valued - * `reference` carriers at a field-def position. That is the whole warrant for - * landing a new gate without a maintainer decision, and it is why there is no - * baseline JSON next to this file. A baseline here would be a place to put the - * next defect. - * - * ## How a field-def carrier position is identified (and how it REFUSES) - * - * Two POSITIVE rules say a holder object literal IS a carrier position: - * - * R1 the holder is a value in a `fields` map, or an element of a `fields` - * array. Structural, and the exact shape of #13053. - * R2 the holder declares `type` as a string literal that is a member of - * `FieldType`. That enum is the field definition's own discriminator, and - * it is READ from `packages/spec/src/data/field.zod.ts` at run time, never - * copied here -- a value added to the enum is picked up with no edit. - * - * Three POSITIVE rules say a holder provably is NOT one, each derived from the - * spec rather than from a path ignore: - * - * N1 `reference` is itself a key of a `fields` map -- so it names a FIELD, - * and its value is that field's definition or its translation entry, not - * a carrier. This is the whole of the generated-i18n exclusion. - * N2 the holder declares a key `data/Field` does not. `FieldSchema` is a - * `strictObject`, so such a literal cannot be a field definition. The key - * set is READ from `packages/spec/authorable-surface/data.json`, the - * ratcheted authorable surface, never copied here. - * N3 the holder declares `type` as a string literal that is NOT a - * `FieldType`. The enum would refuse it, so the holder is a map keyed by - * field-property NAME (`FIELD_KEY_STORAGE_CLASS` in driver-sql is the - * live example) rather than a field definition. - * - * When neither side fires -- or when both do -- the position is UNRESOLVED, and - * this is where the gate refuses instead of guessing. An unresolved position - * silently counted as "not a field def" is a hole shaped exactly like the - * defect. But the refusal is scoped to the ambiguity that CHANGES THE VERDICT: - * - * - value is a string literal -> PASS under either reading. Immaterial. - * - value is not a literal at all -> UNJUDGED under either reading. Immaterial. - * - value is a non-string literal -> the readings disagree. **REFUSE (exit 3).** - * - * Measured on the tree at landing: 27 unresolved and 5 conflicting positions, - * every one of them holding a string literal, so the gate is green while the - * refusal branch is live. It is the branch, not the count, that closes the hole. - * - * ## `null` is not a wrong carrier -- it is an absent one - * - * Four `reference: null` sites in `packages/spec/src/ai/solution-blueprint.test.ts` - * sit at a field-def position and are spec-LEGAL there: `StrictField` (the shape - * behind `SolutionBlueprintStrict`) declares `reference: z.string().nullable()`. - * `null` and `undefined` express ABSENCE of a target, not a target spelled in the - * wrong shape, and deciding whether absence is legal would require knowing WHICH - * schema a literal is an instance of -- precisely what the position recogniser - * deliberately does not attempt. They are counted in their own census bucket, so - * the exemption is visible rather than folded into "unjudged". - * - * ## Exclusions by SHAPE, not by path -- and the path spellings were already stale - * - * The #13103 census excluded three paths because `reference` there is a map KEY: - * `packages/spec/json-schema/**`, `packages/spec/liveness/**`, and the generated - * i18n translation maps. Re-verified here rather than inherited, and none of the - * three survives as a path rule: - * - * - `packages/spec/json-schema/**` does not exist under that name; the sharded - * `packages/spec/json-schema.manifest/` that replaced it contains the string - * `reference` zero times. - * - `packages/spec/liveness/**` is JSON, outside a TS/JS source scan entirely. - * (Its `reference` really is a props-map key -- `liveness/field.json` has it - * under `props` and `props > inlineColumns > children`, never under `fields`.) - * - the i18n maps are real and really do write `reference: { label, helpText }` - * -- four of them, and they are the only object-valued `reference` in the - * tree. N1 excludes them by their SHAPE, which also covers the map nobody has - * generated yet, and an object that legitimately declares a field NAMED - * `reference`. - * - * An exclusion inherited without its reason is how a gate quietly stops watching - * a real population; a path list is the form that rots first. - * - * ## Refusal when nothing was measured - * - * A guard whose success condition equals its total-failure condition must - * refuse: "0 bad carriers" and "the scan found no carriers at all" print the - * same green. A moved directory, a renamed key or a parse failure would produce - * the second while reading as the first, so a run that finds no field-def - * carrier -- or no files -- exits 3 saying it is NOT a pass. Same shape as - * `check:dual-build-cjs-loads`. - * - * Exit codes: 0 clean · 1 findings · 3 could not measure (unresolved material - * ambiguity, an empty population, or a missing spec input). Both non-zero, and a - * reader never has to guess which verdict they got. - * - * ## What it does NOT claim - * - * - It reads SOURCE, so a carrier assembled from a variable, a helper or a - * spread is invisible to it. #13103 measured that second population at 511 - * not-statically-evaluable fixtures repo-wide: covering those needs a runtime - * hook, not a scan, and is a different card. - * - R2 also admits the field-def-ADJACENT literals that carry a `type` and a - * `reference`: `data/InlineGridColumn`, `ui/ActionParam`, `ui/FormField` and - * `ai/BlueprintField`. That is deliberate and costs nothing -- all five - * schemas that declare a `reference` key declare it as a string, so the - * verdict is identical whichever one a literal turns out to be. - */ - -import { readdirSync, readFileSync, mkdtempSync, writeFileSync, rmSync } from 'node:fs'; -import { join, relative, resolve, dirname, extname } from 'node:path'; -import { fileURLToPath } from 'node:url'; -import { tmpdir } from 'node:os'; - -import { requireDefaultExport } from '../../../scripts/import-prerequisite.mjs'; -import { parseSourceFile, EXIT_UNPARSEABLE } from '../../../scripts/ts-parse.mjs'; - -const ts = await requireDefaultExport('typescript', () => import('typescript'), import.meta.url); - -const HERE = dirname(fileURLToPath(import.meta.url)); -const REPO_ROOT = resolve(HERE, '../../..'); - -/** - * The SCAN SURFACE, in the syntax `scripts/pm/dispatch-gates.mjs` reads as this - * gate's declared population, so a card touching one of these trees derives it. - * - * ⚠️ Provenance, never a lookup key. `walk(REPO_ROOT)` does the walking and it - * walks the WHOLE root minus `SKIP_DIRS` -- deliberately wider than this list, - * because a directory that can hide from the scan is the failure this gate's - * empty-measurement refusal exists to catch. The four trees named here are the - * ones that actually hold source today (4,985 / 205 / 191 / 35 files; the - * remainder is three root-level config files), measured rather than assumed. - */ -const ROOT_DIR_WATCH_HINTS = ['packages/**', 'examples/**', 'scripts/**', 'apps/**']; -void ROOT_DIR_WATCH_HINTS; - -/** Where the two spec inputs live. Missing or empty -> refuse, never degrade. */ -const FIELD_ZOD = 'packages/spec/src/data/field.zod.ts'; -const AUTHORABLE_SURFACE = 'packages/spec/authorable-surface/data.json'; -const FIELD_ENTRY_PREFIX = 'data/Field:'; - -const SKIP_DIRS = new Set(['node_modules', 'dist', '.git', '.turbo', '.next', 'coverage', 'build', '.cache']); -const SOURCE_EXTS = new Set(['.ts', '.tsx', '.mts', '.cts', '.js', '.mjs', '.cjs', '.jsx']); - -const KEY = 'reference'; - -// ── spec inputs ───────────────────────────────────────────────────────────── - -/** Every `FieldType` value, read from the Zod enum that declares them. */ -function readFieldTypes(root = REPO_ROOT) { - const file = join(root, FIELD_ZOD); - const sf = parseSourceFile(file, readFileSync(file, 'utf8')); - const values = new Set(); - (function find(node) { - if ( - ts.isVariableDeclaration(node) - && ts.isIdentifier(node.name) - && node.name.text === 'FieldType' - && node.initializer - && ts.isCallExpression(node.initializer) - && node.initializer.expression.getText(sf) === 'z.enum' - && node.initializer.arguments[0] - && ts.isArrayLiteralExpression(node.initializer.arguments[0]) - ) { - for (const el of node.initializer.arguments[0].elements) { - if (ts.isStringLiteralLike(el)) values.add(el.text); - } - } - ts.forEachChild(node, find); - })(sf); - return values; -} - -/** Every authorable key on `data/Field`, read from the ratcheted surface. */ -function readFieldKeys(root = REPO_ROOT) { - const raw = JSON.parse(readFileSync(join(root, AUTHORABLE_SURFACE), 'utf8')); - const keys = new Set(); - for (const entry of raw.keys ?? []) { - if (!entry.startsWith(FIELD_ENTRY_PREFIX)) continue; - // A retired key is still a key a fixture may spell, and treating it as - // unknown would make N2 fire on a real field def -- the hole direction. - keys.add(entry.slice(FIELD_ENTRY_PREFIX.length).replace(/\s*\[RETIRED\]$/, '')); - } - return keys; -} - -// ── AST helpers ───────────────────────────────────────────────────────────── - -const propertyName = (name) => - (ts.isIdentifier(name) || ts.isStringLiteralLike(name) || ts.isNumericLiteral(name) ? name.text : null); - -/** Peel the wrappers that do not change a value's literal-ness. */ -function unwrap(expr) { - let v = expr; - for (;;) { - if (!v) return v; - if (ts.isParenthesizedExpression(v) || ts.isAsExpression(v) || ts.isNonNullExpression(v)) { v = v.expression; continue; } - if (ts.isSatisfiesExpression?.(v)) { v = v.expression; continue; } - return v; - } -} - -/** - * What KIND of carrier a value expression is. - * - * `string` and `non-string-literal` are the only two the predicate judges; - * `absent` and `non-literal` are counted and left unjudged. The literal test is - * TypeScript's own (`isLiteralExpression`), so a literal kind this file has - * never heard of is classified as a literal rather than slipping into - * "non-literal" -- the direction that would hide a carrier. - */ -function classifyValue(expr) { - if (expr === null) return 'non-literal'; // shorthand `{ reference }` - const v = unwrap(expr); - if (ts.isStringLiteralLike(v)) return 'string'; - if (v.kind === ts.SyntaxKind.NullKeyword) return 'absent'; - if (ts.isIdentifier(v) && v.text === 'undefined') return 'absent'; - if (ts.isObjectLiteralExpression(v) || ts.isArrayLiteralExpression(v)) return 'non-string-literal'; - if (v.kind === ts.SyntaxKind.TrueKeyword || v.kind === ts.SyntaxKind.FalseKeyword) return 'non-string-literal'; - if (ts.isLiteralExpression(v)) return 'non-string-literal'; - return 'non-literal'; -} - -/** - * Which rules fire on the object literal holding this `reference` property. - * Returns the rule names, never a verdict -- `positionOf` composes them so the - * both-sides-fire case stays visible instead of being resolved by precedence. - */ -function rulesFor(prop, vocab) { - const fired = []; - const holder = prop.parent; - if (!holder || !ts.isObjectLiteralExpression(holder)) return ['N0']; - const up = holder.parent; - - // N1 -- `reference` is a key of a `fields` map, so it NAMES a field. - if (up && ts.isPropertyAssignment(up) && propertyName(up.name) === 'fields') fired.push('N1'); - - // R1 -- holder is a value in a `fields` map ... - if (up && ts.isPropertyAssignment(up)) { - const grand = up.parent; - if ( - grand && ts.isObjectLiteralExpression(grand) && grand.parent - && ts.isPropertyAssignment(grand.parent) && propertyName(grand.parent.name) === 'fields' - ) fired.push('R1'); - } - // ... or an element of a `fields` array. - if ( - up && ts.isArrayLiteralExpression(up) && up.parent - && ts.isPropertyAssignment(up.parent) && propertyName(up.parent.name) === 'fields' - ) fired.push('R1'); - - // R2 / N3 -- the `type` discriminator, when it is statically a string. - for (const member of holder.properties) { - if (!ts.isPropertyAssignment(member) || propertyName(member.name) !== 'type') continue; - const v = unwrap(member.initializer); - if (!ts.isStringLiteralLike(v)) continue; - fired.push(vocab.fieldTypes.has(v.text) ? 'R2' : 'N3'); - } - - // N2 -- a key `FieldSchema` (a strictObject) does not declare. - for (const member of holder.properties) { - if (ts.isSpreadAssignment(member)) continue; - const k = member.name ? propertyName(member.name) : null; - if (k === null || !vocab.fieldKeys.has(k)) { fired.push('N2'); break; } - } - - return fired; -} - -function positionOf(fired) { - const yes = fired.some((r) => r[0] === 'R'); - const no = fired.some((r) => r[0] === 'N'); - if (yes && no) return 'conflicting'; - if (yes) return 'field-def'; - if (no) return 'not-a-carrier'; - return 'unresolved'; -} - -// ── the scan ──────────────────────────────────────────────────────────────── - -/** Every `reference` property site in one source text, classified. */ -function scanText(fileName, text, vocab) { - const sites = []; - const sf = parseSourceFile(fileName, text); - (function visit(node) { - let expr; - if (ts.isPropertyAssignment(node) && propertyName(node.name) === KEY) expr = node.initializer; - else if (ts.isShorthandPropertyAssignment(node) && propertyName(node.name) === KEY) expr = null; - else expr = undefined; - if (expr !== undefined) { - const fired = rulesFor(node, vocab); - sites.push({ - line: sf.getLineAndCharacterOfPosition(node.getStart(sf)).line + 1, - position: positionOf(fired), - rules: fired.join('+') || '(none)', - value: classifyValue(expr), - text: expr === null ? '{ reference }' : unwrap(expr).getText(sf).replace(/\s+/g, ' ').slice(0, 72), - }); - } - ts.forEachChild(node, visit); - })(sf); - return sites; -} - -function walk(dir, out = []) { - for (const e of readdirSync(dir, { withFileTypes: true })) { - if (e.isDirectory()) { if (!SKIP_DIRS.has(e.name)) walk(join(dir, e.name), out); continue; } - if (e.isFile() && SOURCE_EXTS.has(extname(e.name))) out.push(join(dir, e.name)); - } - return out; -} - -/** Scan a whole tree. Returns the census; decides nothing. */ -function scanTree(root, vocab) { - const files = walk(root); - const sites = []; - for (const file of files) { - const text = readFileSync(file, 'utf8'); - if (!text.includes(KEY)) continue; // over-inclusive on purpose - for (const s of scanText(file, text, vocab)) { - sites.push({ ...s, file: relative(root, file).split('\\').join('/') }); - } - } - return { filesScanned: files.length, sites }; -} - -const tally = (sites, position) => { - const row = { string: 0, 'non-string-literal': 0, absent: 0, 'non-literal': 0 }; - for (const s of sites) if (s.position === position) row[s.value] += 1; - return row; -}; - -// ── verdict ───────────────────────────────────────────────────────────────── - -function main() { - const vocab = { fieldTypes: readFieldTypes(), fieldKeys: readFieldKeys() }; - const refusals = []; - if (vocab.fieldTypes.size === 0) refusals.push(`${FIELD_ZOD} yielded no FieldType values — the enum moved or was renamed.`); - if (vocab.fieldKeys.size === 0) refusals.push(`${AUTHORABLE_SURFACE} yielded no \`${FIELD_ENTRY_PREFIX}*\` keys — the shard moved or was renamed.`); - if (refusals.length > 0) return refuse(refusals); - - const { filesScanned, sites } = scanTree(REPO_ROOT, vocab); - if (filesScanned === 0) return refuse(['walked the tree and found no source files at all.']); - - const fieldDef = tally(sites, 'field-def'); - const notCarrier = tally(sites, 'not-a-carrier'); - const unresolved = tally(sites, 'unresolved'); - const conflicting = tally(sites, 'conflicting'); - - if (process.argv.includes('--census')) { - for (const s of sites) console.log(`${s.position}\t${s.value}\t${s.rules}\t${s.file}:${s.line}\t${s.text}`); - } - - const carriersMeasured = fieldDef.string + fieldDef['non-string-literal']; - if (carriersMeasured === 0) { - return refuse([ - `scanned ${filesScanned} file(s) and found ZERO \`${KEY}\` carriers at a field-def position.`, - 'This is NOT a pass: nothing was measured. A moved directory, a renamed key or a', - 'position recogniser that stopped matching produces this reading, and it is', - 'indistinguishable from a clean tree by the exit code alone.', - ]); - } - - const material = sites.filter( - (s) => (s.position === 'unresolved' || s.position === 'conflicting') && s.value === 'non-string-literal', - ); - if (material.length > 0) { - return refuse([ - `${material.length} \`${KEY}\` site(s) carry a non-string LITERAL at a position this gate could not resolve.`, - 'The two readings disagree here — "field def" makes it a finding, "not a carrier"', - 'makes it invisible — so the gate refuses rather than picking one.', - 'Resolve it by making the holder legible: put the field definition under a `fields:`', - 'map/array, or give it a `type` that is a FieldType string literal. If the holder is', - 'genuinely not a field definition, say so in its shape (a key FieldSchema does not', - 'declare already proves it) — never by adding a path ignore here.', - '', - ...material.map((s) => ` ${s.file}:${s.line} ${s.text} [rules: ${s.rules}]`), - ]); - } - - const findings = sites.filter((s) => s.position === 'field-def' && s.value === 'non-string-literal'); - if (findings.length > 0) { - console.error(`\ncheck-reference-carrier-shape: ${findings.length} problem(s).\n`); - for (const s of findings) { - console.error(` ${s.file}:${s.line}`); - console.error(` \`${KEY}\` carries a literal that is not a string: ${s.text}`); - } - console.error( - `\n\`FieldSchema.${KEY}\` is \`z.string()\` — a relationship target is the target object's` - + '\n NAME. A non-string carrier is refused by `ObjectSchema.safeParse` AND read as' - + '\n `undefined` by every rule that resolves it, so the site is invisible in both' - + '\n directions at once (#13053). Spell it as the object name string.\n', - ); - process.exit(1); - } - - const unjudged = fieldDef['non-literal'] + fieldDef.absent; - console.log( - `check-reference-carrier-shape: OK — ${filesScanned} file(s) scanned, ${sites.length} \`${KEY}\` site(s).\n` - + ` field-def carrier position: ${fieldDef.string} string literal(s), ` - + `${fieldDef['non-string-literal']} non-string literal(s) — ${unjudged} unjudged ` - + `(${fieldDef['non-literal']} non-literal, ${fieldDef.absent} null/undefined).\n` - + ` provably not a carrier: ${notCarrier.string + notCarrier['non-string-literal'] + notCarrier.absent + notCarrier['non-literal']} site(s), ` - + `${notCarrier['non-string-literal']} of them non-string literals (excluded by SHAPE, not by path).\n` - + ` position unresolved: ${unresolved.string + unresolved['non-literal'] + unresolved.absent} site(s) + ` - + `${conflicting.string + conflicting['non-literal'] + conflicting.absent} conflicting — 0 material ` - + '(a refusal here is scoped to a non-string literal, where the readings disagree).', - ); -} - -function refuse(lines) { - console.error('\ncheck-reference-carrier-shape: REFUSED — could not measure.\n'); - for (const l of lines) console.error(l ? ` ${l}` : ''); - console.error(''); - process.exit(EXIT_UNPARSEABLE); -} - -// ── self-test ─────────────────────────────────────────────────────────────── - -function selfTest() { - const failures = []; - const check = (ok, msg) => { if (!ok) failures.push(msg); }; - - const vocab = { fieldTypes: readFieldTypes(), fieldKeys: readFieldKeys() }; - check(vocab.fieldTypes.has('lookup') && vocab.fieldTypes.has('master_detail'), - 'FieldType vocabulary does not contain lookup/master_detail — the enum read is broken'); - check(vocab.fieldKeys.has('reference') && vocab.fieldKeys.has('type'), - 'data/Field authorable keys do not contain reference/type — the surface read is broken'); - - const one = (src, label) => { - const sites = scanText(join(REPO_ROOT, `selftest-${label}.ts`), src, vocab); - check(sites.length === 1, `${label}: expected exactly 1 \`${KEY}\` site, got ${sites.length}`); - return sites[0] ?? { position: '(none)', value: '(none)' }; - }; - const expect = (label, src, position, value) => { - const s = one(src, label); - check(s.position === position && s.value === value, - `${label}: expected ${position}/${value}, got ${s.position}/${s.value} [rules: ${s.rules}]`); - return s; - }; - - // ── the DEFECT, in both container spellings. Without these the whole gate - // could be a function that returns "clean". ────────────────────────────── - expect('defect-map', `const o = { name: 'a', fields: { invoice: { type: 'master_detail', reference: { object: 'shop_invoice' } } } };`, - 'field-def', 'non-string-literal'); - expect('defect-array', `const o = { fields: [{ name: 'invoice', type: 'lookup', reference: ['shop_invoice'] }] };`, - 'field-def', 'non-string-literal'); - expect('defect-no-container', `const f = { name: 'invoice', type: 'lookup', reference: { object: 'x' } };`, - 'field-def', 'non-string-literal'); - expect('defect-number', `const o = { fields: { a: { type: 'lookup', reference: 42 } } };`, - 'field-def', 'non-string-literal'); - - // ── the string carrier, which is the whole 451-site population. ──────────── - expect('ok-map', `const o = { fields: { a: { type: 'lookup', reference: 'crm_account' } } };`, 'field-def', 'string'); - expect('ok-array', `const o = { fields: [{ name: 'a', type: 'lookup', reference: 'crm_account' }] };`, 'field-def', 'string'); - expect('ok-template', `const o = { fields: { a: { type: 'lookup', reference: \`crm_account\` } } };`, 'field-def', 'string'); - expect('ok-as-const', `const o = { fields: { a: { type: 'lookup', reference: 'crm_account' as const } } };`, 'field-def', 'string'); - - // ── the 17 non-literal spellings the census left UNJUDGED, one per family. - // Every one of these staying green IS the narrowness of the predicate. ─── - expect('unjudged-identifier', `const o = { fields: { a: { type: 'lookup', reference: ACCOUNT } } };`, 'field-def', 'non-literal'); - expect('unjudged-template-sub', `const o = { fields: { a: { type: 'lookup', reference: \`\${p}_1\` } } };`, 'field-def', 'non-literal'); - expect('unjudged-member', `const o = { fields: { a: { type: 'lookup', reference: cfg.target } } };`, 'field-def', 'non-literal'); - expect('unjudged-call', `const o = { fields: { a: { type: 'lookup', reference: String(f.reference) } } };`, 'field-def', 'non-literal'); - expect('unjudged-conditional', `const o = { fields: { a: { type: 'lookup', reference: ok ? a : undefined } } };`, 'field-def', 'non-literal'); - expect('unjudged-shorthand', `const mk = (reference) => ({ type: 'lookup', reference });`, 'field-def', 'non-literal'); - expect('unjudged-zod', `const S = strictObject({ name: z.string(), type: FieldType, reference: z.string().optional() });`, - 'unresolved', 'non-literal'); - - // ── `reference: null` — spec-legal under StrictField's z.string().nullable(). - expect('absent-null', `const o = { fields: [{ name: 'f', type: 'text', reference: null }] };`, 'field-def', 'absent'); - expect('absent-undefined', `const o = { fields: [{ name: 'f', type: 'text', reference: undefined }] };`, 'field-def', 'absent'); - - // ── the shapes that are provably NOT carriers. Each of these reds a - // key-name-only gate on the day it lands. ─────────────────────────────── - expect('not-i18n-map', `export const t = { field: { fields: { type: { label: 'T' }, reference: { label: 'Reference', helpText: 'x' } } } };`, - 'not-a-carrier', 'non-string-literal'); - expect('not-expect-object', `it('x', () => { expect({ live, reference: ['1', '2'] }).toEqual(y); });`, - 'not-a-carrier', 'non-string-literal'); - expect('not-field-key-class', `const C = { type: 'storage', maxLength: 'storage', reference: 'storage' };`, - 'not-a-carrier', 'string'); - - // ── the REFUSAL branch: a non-string literal at a position neither side - // claims. This must never be silently read as "not a field def". ──────── - const amb = one(`const x = { label: 'Account', reference: { object: 'crm_account' } };`, 'ambiguous'); - check(amb.position === 'unresolved' && amb.value === 'non-string-literal', - `ambiguous: expected unresolved/non-string-literal, got ${amb.position}/${amb.value}`); - - // ── a field NAMED `fields` makes N1 and R1 both fire: a real conflict, and - // it must surface as one rather than being resolved by rule order. ────── - const conflict = one(`const o = { fields: { fields: { type: 'lookup', reference: { object: 'x' } } } };`, 'conflict'); - check(conflict.position === 'conflicting', - `conflict: expected conflicting, got ${conflict.position} [rules: ${conflict.rules}]`); - - // ── the empty-measurement refusal, over a real tree on disk. This is the - // control that makes the green line mean something: the same code path - // that prints OK must exit non-zero when it reads nothing. ───────────── - { - const dir = mkdtempSync(join(tmpdir(), 'ref-carrier-')); - try { - writeFileSync(join(dir, 'a.ts'), `export const x = { note: 'no carriers here' };\n`); - const empty = scanTree(dir, vocab); - check(empty.filesScanned === 1, `empty-tree: expected 1 file, got ${empty.filesScanned}`); - check(tally(empty.sites, 'field-def').string === 0, - 'empty-tree: a tree with no carriers reported carriers — the instrument is fabricating'); - - writeFileSync(join(dir, 'b.ts'), `export const o = { fields: { a: { type: 'lookup', reference: 'crm_account' } } };\n`); - const seeded = scanTree(dir, vocab); - check(tally(seeded.sites, 'field-def').string === 1, - 'seeded-tree: the same scan over one real carrier did not count it — a zero here would be unfalsifiable'); - } finally { - rmSync(dir, { recursive: true, force: true }); - } - } - - if (failures.length > 0) { - for (const f of failures) console.error(`✗ self-test: ${f}`); - console.error(`\ncheck-reference-carrier-shape --self-test: ${failures.length} failure(s).\n`); - process.exit(1); - } - console.log( - '✅ self-test: flags an object / array / numeric carrier at a field-def position in all\n' - + ' three container spellings; stays silent on string, template and `as const` carriers;\n' - + ' leaves every non-literal spelling and `reference: null` unjudged; excludes the i18n\n' - + ' field-name map, the { live, reference } assertion and the field-key class map BY SHAPE;\n' - + ' surfaces an unresolved position and a rule conflict instead of guessing; and proves\n' - + ' over a real tree that the same scan returns 0 on an empty one and 1 on a seeded one.', - ); -} - -if (process.argv.includes('--self-test')) selfTest(); -else main(); diff --git a/packages/lint/src/data-model-rules.ts b/packages/lint/src/data-model-rules.ts index 35f2f864978..0edf745a0aa 100644 --- a/packages/lint/src/data-model-rules.ts +++ b/packages/lint/src/data-model-rules.ts @@ -16,7 +16,7 @@ * schema-valid AND lint-clean here. */ -import { BOOLEAN_VALUE_TYPES, NUMERIC_VALUE_TYPES } from '@objectstack/spec/data'; +import { BOOLEAN_VALUE_TYPES, NUMERIC_VALUE_TYPES, referenceCarrierOf } from '@objectstack/spec/data'; export type Severity = 'error' | 'warning' | 'suggestion'; @@ -238,10 +238,16 @@ function fieldEntries(fields: any): FieldEntry[] { * `relationship/missing-reference` report a valid target for a field that has * none: the one component whose job is to catch the misspelling was the one * accepting it. + * + * The narrowing is now a REFUSAL rather than a quiet `undefined` (#13053): a + * `reference` present in a shape no reader can read throws, because a rule whose + * job is to tell an author their metadata is wrong must not be the component + * that reads the wrong metadata as absent. The predicate is the spec's single + * carrier accessor, so this file, `validate-security-posture.ts` and the runtime + * cannot drift into three answers. */ function refOf(def: any): string | undefined { - const r = def?.reference as unknown; - return typeof r === 'string' && r ? r : undefined; + return referenceCarrierOf({ reference: def?.reference }, 'data-model-rules refOf'); } // ─── Uniqueness declarations (ADR-0120) ───────────────────────────── @@ -624,7 +630,7 @@ export function lintDataModel(objects: any[]): LintIssue[] { if (OPTION_FIELD_TYPES.has(type)) { const hasOptions = (Array.isArray(def.options) && def.options.length > 0) || - !!def.optionsFrom || !!def.dataSource || !!def.reference; + !!def.optionsFrom || !!def.dataSource || !!refOf(def); if (!hasOptions) { issues.push({ severity: 'warning', @@ -775,7 +781,7 @@ export function lintDataModel(objects: any[]): LintIssue[] { const summaryChildObjects = new Set( fields .filter((f) => f.def?.type === 'summary') - .map((f) => f.def?.summaryOperations?.object || f.def?.reference) + .map((f) => f.def?.summaryOperations?.object || refOf(f.def)) .filter(Boolean), ); const seenSuggestedChild = new Set(); diff --git a/packages/lint/src/object-graph.ts b/packages/lint/src/object-graph.ts index 9bc1b05baf9..70975d983c7 100644 --- a/packages/lint/src/object-graph.ts +++ b/packages/lint/src/object-graph.ts @@ -68,6 +68,8 @@ * second question about it is still unanswered — truthfully, and only there. */ +import { referenceCarrierOf } from '@objectstack/spec/data'; + import { injectedColumnDefsFor, injectedColumnsFor } from './system-fields.js'; /** Any plain metadata record. */ @@ -234,7 +236,10 @@ function strName(v: unknown): string | undefined { function graphFieldOf(def: AnyRec): GraphField { return { type: typeof def.type === 'string' ? def.type : undefined, - reference: strName(def.reference), + // ⛔ NOT `strName` here. A carrier in a shape no reader can read is refused + // rather than narrowed to `undefined` (#13053): every rule downstream reads + // this slice, so a silent narrowing here is that blindness wholesaled. + reference: referenceCarrierOf({ reference: def.reference }, 'object-graph graphFieldOf'), multiple: def.multiple === true ? true : undefined, }; } diff --git a/packages/lint/src/validate-security-posture.ts b/packages/lint/src/validate-security-posture.ts index a95310e35a8..9e89bb5f80c 100644 --- a/packages/lint/src/validate-security-posture.ts +++ b/packages/lint/src/validate-security-posture.ts @@ -118,6 +118,7 @@ * credit belongs to the schema's closed enum. */ +import { referenceCarrierOf } from '@objectstack/spec/data'; import { describeAnchorForbiddenBits } from '@objectstack/spec/security'; import { indexObjectGraph, recordsOf, type ObjectGraph } from './object-graph.js'; @@ -276,10 +277,19 @@ function labelHasRoleWord(label: unknown): boolean { * `reference` is the only spelling `FieldSchema` declares; `reference_to` (like * `referenceTo` / `relatedTo` / `target`) is a rejected alias the strict error * map renames for the author, so a field carrying it does not parse (#5017). + * + * A NON-STRING carrier now THROWS rather than reading as "no target" (#13053). + * This function was the reader in that incident: a fixture spelled + * `reference: { object: … }`, `ObjectSchema.safeParse` refused it where it was + * written, and this returned `undefined` where it was consumed — so the suite + * was blind in both directions at once and green. The refusal is the spec's + * single carrier accessor, so this rule and the runtime give one answer. */ function refOf(def: AnyRec): string | undefined { - const r = def.reference as unknown; - return typeof r === 'string' && r ? r : undefined; + // The read stays HERE, on `def.reference`, so the #5017 receiver meta-test + // below keeps its subject: this rule reads `reference` and never the alias. + // Only the SHAPE judgment moves out, to the spec's one carrier accessor. + return referenceCarrierOf({ reference: def.reference }, 'validate-security-posture refOf'); } /** diff --git a/packages/services/service-automation/src/builtin/screen-nodes.ts b/packages/services/service-automation/src/builtin/screen-nodes.ts index c9f88d74df3..409b2419316 100644 --- a/packages/services/service-automation/src/builtin/screen-nodes.ts +++ b/packages/services/service-automation/src/builtin/screen-nodes.ts @@ -50,20 +50,19 @@ import { judgeHeadlessScreen } from '../screen-input-contract.js'; * object PICKER (that is what `xRef` marks) whose collected value is the target * object's NAME — the same string `FieldSchema.reference` carries. * - * Named rather than written inline at its `reference:` key, and the naming is - * load-bearing. `check:reference-carrier-shape` classifies the object HOLDING a - * `reference` key; here that holder is a JSON-Schema `properties` map, keyed by - * property NAME — the same class the gate excludes by shape for a `fields` map - * (N1) and for a field-key class map (N3), but a shape its position rules have - * no case for. Written inline, the holder resolved under neither reading and - * the gate refused to guess (exit 3 — correctly). None of its three site - * remedies is spellable here: the holder is not under `fields:`, its own `type` - * key holds a sub-schema rather than a string literal, and all twelve of its - * keys are `data/Field` authorable keys, so no key can prove it is not a field - * definition. A value reached through a name is UNJUDGED by the gate's stated - * predicate, which judges LITERALS only. That narrows the gate nowhere else in - * the tree; the missing `properties`-map rule is reported to the maintainer - * rather than patched from inside this PR. + * Named rather than written inline at its `reference:` key. The naming was + * originally forced by `check:reference-carrier-shape`, a source scan that + * classified the object HOLDING a `reference` key and had no position rule for a + * JSON-Schema `properties` map, so an inline holder made it refuse (exit 3) with + * none of its three site remedies spellable here. That gate has been RETIRED by + * maintainer ruling — the contract door refuses a non-string carrier on its own + * (`FieldSchema.reference` is `z.string().optional()`), and the defect class it + * guarded now lives in the reader, which throws rather than reading a non-string + * as "no target". + * + * The name stays. It is a designer-form column that two call sites below read, + * and this is product surface; the retirement removed the reason it was + * MANDATORY, not the reason it is right. */ const LOOKUP_TARGET_COLUMN = { type: 'string', diff --git a/packages/spec/api-surface/data.json b/packages/spec/api-surface/data.json index b38bd783eba..4a15d6d2f4d 100644 --- a/packages/spec/api-surface/data.json +++ b/packages/spec/api-surface/data.json @@ -802,6 +802,7 @@ "redactableConfigKeys (function)", "reduceFilterKeyVerdict (function)", "reduceFilterVerdict (function)", + "referenceCarrierOf (function)", "referenceTargetOf (function)", "referencedFields (function)", "refusedCredentialKeys (function)", diff --git a/packages/spec/export-origins/data.json b/packages/spec/export-origins/data.json index b6552ee80e3..a07c5b8b169 100644 --- a/packages/spec/export-origins/data.json +++ b/packages/spec/export-origins/data.json @@ -789,6 +789,7 @@ "redactableConfigKeys": "src/data/datasource-credential-redaction.ts#redactableConfigKeys (function)", "reduceFilterKeyVerdict": "src/data/filter-verdict.ts#reduceFilterKeyVerdict (function)", "reduceFilterVerdict": "src/data/filter-verdict.ts#reduceFilterVerdict (function)", + "referenceCarrierOf": "src/data/field-value.zod.ts#referenceCarrierOf (function)", "referenceTargetOf": "src/data/field-value.zod.ts#referenceTargetOf (function)", "referencedFields": "src/data/autonumber-format.ts#referencedFields (function)", "refusedCredentialKeys": "src/data/datasource-credential-redaction.ts#refusedCredentialKeys (function)", diff --git a/packages/spec/src/automation/builtin-node-config.test.ts b/packages/spec/src/automation/builtin-node-config.test.ts index 5d690ac7ea4..78512a2b26d 100644 --- a/packages/spec/src/automation/builtin-node-config.test.ts +++ b/packages/spec/src/automation/builtin-node-config.test.ts @@ -346,17 +346,13 @@ describe('ScreenFieldConfigSchema — the bound pair, help text and lookup targe // A lookup target is the target object's NAME, so every non-string SHAPE has to // be refused, not just the array this once spelled inline. Tabled for two // reasons. It widens the pin — `{ object: 'x' }` is the exact carrier shape - // #13053 was filed for, and it was untested here. And it is the only spelling - // available: `check:reference-carrier-shape` judges a `reference` whose value - // is a LITERAL, and it could place THIS holder under neither of its readings - // (`{ ...BASE, reference: … }` is a spread plus one key that `data/Field` - // does declare), so it refused rather than guess. Its three site remedies all - // make the refusal WORSE here: giving the holder a FieldType `type` or a - // `fields:` parent turns a rejection fixture into a reported finding, and - // `ScreenFieldConfig`'s twelve keys are every one of them `data/Field` keys, - // so none can prove the holder is not a field definition. Reaching the value - // through a name puts it in the population the gate documents as unjudged — - // it judges literals — while the assertion below gets STRICTER, not weaker. + // #13053 was filed for, and it was untested here. And the table was, at the + // time, the only spelling available: `check:reference-carrier-shape` judged a + // `reference` whose value was a LITERAL and could place THIS holder under + // neither of its readings, so it refused rather than guess. That gate has since + // been RETIRED by maintainer ruling, so the table is no longer forced — it is + // kept because it is the stricter assertion, which is why it was written this + // way in the first place. const NON_STRING_LOOKUP_TARGETS: readonly unknown[] = [['a'], { object: 'crm_account' }, 42, true]; it('refuses help text and a lookup target that are not strings', () => { diff --git a/packages/spec/src/data/field-value.test.ts b/packages/spec/src/data/field-value.test.ts index 5be6b3df148..8c556a0a373 100644 --- a/packages/spec/src/data/field-value.test.ts +++ b/packages/spec/src/data/field-value.test.ts @@ -29,6 +29,7 @@ import { isMultiValueField, valueSchemaFor, referenceTargetOf, + referenceCarrierOf, } from './field-value.zod'; const ok = (def: Parameters[0], v: unknown, form?: 'stored' | 'expanded') => @@ -74,6 +75,60 @@ describe('semantic type classes', () => { expect(referenceTargetOf('user')).toBeUndefined(); }); + it('`referenceCarrierOf` REFUSES an unreadable carrier instead of answering "no target" (#13053)', () => { + // The defect class, stated as a test. A `reference` in a shape no reader can + // read used to come back as `undefined` — indistinguishable from a field that + // names no target at all — so the carrier was refused by + // `ObjectSchema.safeParse` where it was WRITTEN and read as absent where it + // was CONSUMED, and nothing anywhere reported it. + // + // #13053's exact shape first. + expect(() => referenceCarrierOf({ type: 'lookup', reference: { object: 'shop_invoice' } })).toThrow(TypeError); + expect(() => referenceCarrierOf({ type: 'lookup', reference: { object: 'shop_invoice' } })) + .toThrow(/`reference` is an object/); + // Every other non-string shape, named in the message so the author can see + // which one they wrote. + expect(() => referenceCarrierOf({ reference: ['a', 'b'] })).toThrow(/`reference` is an array \(length 2\)/); + expect(() => referenceCarrierOf({ reference: 42 })).toThrow(/`reference` is a number/); + expect(() => referenceCarrierOf({ reference: true })).toThrow(/`reference` is a boolean/); + // The refusal carries the fix, not just the complaint. + expect(() => referenceCarrierOf({ reference: { object: 'x' } })) + .toThrow(/FieldSchema declares it as an optional STRING/); + // The caller label is the reader's, so the message says WHO could not read it. + expect(() => referenceCarrierOf({ reference: { object: 'x' } }, 'some-rule refOf')) + .toThrow(/^some-rule refOf: /); + + // CONTROLS — everything that is not an unreadable carrier still answers. + expect(referenceCarrierOf({ reference: 'shop_invoice' })).toBe('shop_invoice'); + // ABSENCE is not a wrong shape: `undefined` is what `.optional()` admits and + // `null` is what the blueprint's `StrictField` admits. Neither throws. + expect(referenceCarrierOf({ type: 'lookup' })).toBeUndefined(); + expect(referenceCarrierOf({ type: 'lookup', reference: undefined })).toBeUndefined(); + expect(referenceCarrierOf({ type: 'lookup', reference: null })).toBeUndefined(); + // An empty string names no object — absence too, and the answer every caller + // already read for it. + expect(referenceCarrierOf({ type: 'lookup', reference: '' })).toBeUndefined(); + // Not a field-def at all: still `undefined`, never a throw. + expect(referenceCarrierOf(undefined)).toBeUndefined(); + expect(referenceCarrierOf('user')).toBeUndefined(); + }); + + it('`referenceTargetOf` inherits the refusal — one carrier accessor, one answer', () => { + // The single arbiter reads the carrier through `referenceCarrierOf`, so the + // engine, the expand gate and every lint rule that asks it get the refusal + // rather than three different silences. + expect(() => referenceTargetOf({ type: 'lookup', reference: { object: 'shop_invoice' } })) + .toThrow(/referenceTargetOf: `reference` is an object/); + // ⛔ Not gated on the field being a reference TYPE: the carrier is refused by + // `FieldSchema` for every type, so reading it as absent on a `text` field is + // the same silence one type over. + expect(() => referenceTargetOf({ type: 'text', reference: { object: 'x' } })).toThrow(TypeError); + // CONTROL — the answers the arbiter already gave are unmoved. + expect(referenceTargetOf({ type: 'lookup', reference: 'accounts' })).toBe('accounts'); + expect(referenceTargetOf({ type: 'user' })).toBe('sys_user'); + expect(referenceTargetOf({ type: 'lookup', reference: null })).toBeUndefined(); + }); + it('every reference type either implies a target or admits one — no third state', () => { // Guards the set from drifting: adding a reference type without deciding // which half it belongs to would leave `referenceTargetOf` silently diff --git a/packages/spec/src/data/field-value.zod.ts b/packages/spec/src/data/field-value.zod.ts index f160c6ad238..6045e078701 100644 --- a/packages/spec/src/data/field-value.zod.ts +++ b/packages/spec/src/data/field-value.zod.ts @@ -192,12 +192,72 @@ const IMPLICIT_REFERENCE_TARGETS: ReadonlyMap = new Map([ */ export function referenceTargetOf(def: unknown): string | undefined { if (!def || typeof def !== 'object') return undefined; - const { type, reference } = def as { type?: unknown; reference?: unknown }; + const { type } = def as { type?: unknown }; + const reference = referenceCarrierOf(def, 'referenceTargetOf'); if (typeof type !== 'string' || !REFERENCE_VALUE_TYPES.has(type)) return undefined; - if (typeof reference === 'string' && reference) return reference; + if (reference) return reference; return IMPLICIT_REFERENCE_TARGETS.get(type); } +/** + * The `reference` carrier read as the string the contract declares it to be — + * or a THROW, when the key is present in a shape no reader can read. + * + * `FieldSchema.reference` is `z.string().optional()`, so `ObjectSchema.safeParse` + * already refuses an object- or array-valued carrier at the contract door, with a + * located `invalid_type` issue. This accessor is the OTHER door: the one a value + * reaches only when it never went through parse at all — a hand-built test + * fixture, a driver's raw registry entry, a metadata row rehydrated past its + * schema. + * + * Returning `undefined` there is the defect this function exists to end. A reader + * that answers "no target" for `{ object: 'shop_invoice' }` is blind in BOTH + * directions at once: the carrier is refused where it was written and read as + * absent where it is consumed, so nothing anywhere reports it. A fixture in that + * shape passes, and goes on passing until an assertion comes to depend on the + * parent it silently could not see. + * + * ## `null` and `undefined` are ABSENCE, not a wrong shape — and do not throw + * + * They express that the field names no target, which is a legal thing for a field + * to say: `undefined` is what `.optional()` admits, and `null` is what the + * blueprint's `StrictField` admits. Deciding whether an ABSENT target is legal + * needs to know which schema a literal is an instance of, and that is the parse + * step's question, not this accessor's. An empty string is absence too — it names + * no object — and is likewise returned as `undefined` rather than thrown on, so + * this function's answer stays exactly what every caller already read. + * + * ## Why a throw rather than a finding + * + * A finding needs a reader that can still read the record to report it. This one + * cannot: the carrier is the very thing it was asked for. `TypeError` follows the + * class this package's other total accessors already throw — + * `getDriverConfigJsonSchemaById` in `data/driver/config-registry.zod.ts` is the + * reference text — so a caller that catches one catches this. + * + * @param def The field definition (or field-def-adjacent literal) to read. + * @param reader A label naming the caller, so the message says who could not read it. + */ +export function referenceCarrierOf(def: unknown, reader = 'referenceCarrierOf'): string | undefined { + if (!def || typeof def !== 'object') return undefined; + const { reference } = def as { reference?: unknown }; + if (reference === undefined || reference === null) return undefined; + if (typeof reference === 'string') return reference === '' ? undefined : reference; + throw new TypeError( + `${reader}: \`reference\` is ${describeCarrierValue(reference)}, and FieldSchema declares it as an ` + + 'optional STRING (the target object\'s name). A non-string carrier is refused by ' + + 'ObjectSchema.safeParse, so this value never went through parse — spell the target as the object ' + + 'name (reference: \'shop_invoice\'), or omit the key when the field names no target.', + ); +} + +/** How a value that is not a readable `reference` is named in the refusal above. */ +function describeCarrierValue(value: unknown): string { + if (Array.isArray(value)) return `an array (length ${value.length})`; + if (typeof value === 'object') return 'an object'; + return `a ${typeof value}`; +} + /** * Media/attachment types. The STORED value of every member is an opaque * `sys_file` id ({@link FileReferenceIdValueSchema}); the inline metadata diff --git a/scripts/check-self-test-wired.mjs b/scripts/check-self-test-wired.mjs index c1ae8e30a10..461096f8536 100644 --- a/scripts/check-self-test-wired.mjs +++ b/scripts/check-self-test-wired.mjs @@ -161,9 +161,13 @@ const ROOT_DIR_WATCH_HINTS = ['scripts/**/*.mjs', 'scripts/**/*.mts', 'scripts/* * * ## The LEFT boundary, and why the prefix is read rather than cut off (#15342) * - * This repo has a package-local gate lane, and `lint.yml` really does run one - * of its gates by path: `node packages/lint/scripts/check-reference-carrier- - * shape.mjs --self-test`. The pattern used to open on the bare literal + * This repo has a package-local gate lane, and `lint.yml` ran one of its gates + * by path: `node packages/lint/scripts/check-reference-carrier-shape.mjs + * --self-test`. That gate has since been retired by maintainer ruling and the + * lane currently has NO live member, so the boundary below is held by the + * synthetic cases rather than by a specimen; the grammar stays because the next + * such invocation must be keyed WHOLE on the day it lands, not one release + * later. The pattern used to open on the bare literal * `scripts/` with nothing to its left, so it matched that path as a SUBSTRING * and filed the gate under `scripts/check-reference-carrier-shape.mjs` — a key * with no file behind it. Both directions of that were silent: the real file @@ -1006,12 +1010,23 @@ function selfTest() { 'a path this gate keys an invocation to has NO file behind it. Every audit downstream then runs ' + 'against a script that does not exist, and passes for the wrong reason, in both directions (#15342)', ); - // Named BY NAME on purpose: it is this tree's only package-local gate - // invocation, so it is the whole specimen set for the widening above. + // The live specimen this pin used to name BY NAME was + // `packages/lint/scripts/check-reference-carrier-shape.mjs`, and it was the + // tree's ONLY package-local gate invocation. That gate was retired by + // maintainer ruling, so the lane did not move — it EMPTIED. Measured at the + // retirement: `packageLocal` goes 1 → 0 while `population` goes 214 → 213. + // + // A live pin on an empty lane can only be a pin on zero, so what is asserted + // here instead is that the widening still ADMITS the shape — driven by the + // synthetic `battery('left boundary')` above, which keys a package-local path + // WHOLE and refuses the phantom root key beside it, with no live member + // needed. ⛔ Do not re-add a live-specimen pin on a hopeful path: the first + // author to invoke a package-local gate by path from a workflow makes this + // lane live again, and THAT is the moment to name a specimen here. ok( - keys.includes('packages/lint/scripts/check-reference-carrier-shape.mjs'), - "lint.yml's package-local gate is not in the live population. Either the lane moved — re-point this " - + 'pin at the new specimen — or the anchor regressed to a root-only one and the widening is untested', + keys.length > 0 && keys.every((p) => !p.startsWith('..')), + 'the anchor minted a key that climbs out of the root — a path this ROOT cannot resolve is exactly ' + + 'the phantom identity #15342 was about, and every audit downstream then passes for the wrong reason', ); } @@ -1137,10 +1152,24 @@ function selfTest() { live.refusal === null && live.population.length > 0, `the live population could not be read (${live.refusal ?? 'empty'}), so the cases below prove nothing (#4690)`, ); + // #15414's subject is the EXPORT: a consumer that gets back a population with + // no package-local half is in the root-walk-only world this card exists to + // end. That used to be read off a live member; the tree's only one retired + // with `check-reference-carrier-shape`, so the live lane now measures ZERO. + // + // The DERIVATION is pinned instead, and it holds at zero exactly as it holds + // at one: `packageLocal` is the part of `population` the root walk did not + // produce, so dropping the field, hard-coding it empty, or re-deriving it + // from a `startsWith` on a re-spelling of the root all red here. The control + // for the zero is the `population.length > 0` pin immediately above — an + // empty `packageLocal` beside an empty `population` is a broken reader, and + // that case is already refused. ok( - live.packageLocal.length > 0 - && live.population.includes('packages/lint/scripts/check-reference-carrier-shape.mjs'), - 'the EXPORT dropped the package-local half. A consumer of it is then back in the root-walk-only ' + Array.isArray(live.packageLocal) + && live.packageLocal.length === live.population.filter((s) => !live.walked.has(s)).length + && live.packageLocal.every((s) => live.population.includes(s) && !live.walked.has(s)), + 'the EXPORT dropped the package-local half, or derives it as something other than "the part of the ' + + 'population the root walk did not produce". A consumer of it is then back in the root-walk-only ' + 'population this card exists to end, and nothing on either side would redden (#15414)', ); ok( diff --git a/scripts/check-self-test-workflow-commands.mjs b/scripts/check-self-test-workflow-commands.mjs index 7aa9fa55df2..fab01010eb1 100644 --- a/scripts/check-self-test-workflow-commands.mjs +++ b/scripts/check-self-test-workflow-commands.mjs @@ -71,7 +71,9 @@ * check-self-test-workflow-commands 168 script(s) CI runs ship a `--self-test` * * The missing member was `packages/lint/scripts/check-reference-carrier-shape - * .mjs`, which `lint.yml` runs with `--self-test` on every pull request. Its + * .mjs`, which `lint.yml` then ran with `--self-test` on every pull request (that + * gate has since been retired by maintainer ruling, and the package-local lane + * has no live member today — the mechanism below is unchanged by that). Its * output was in no sweep — and NOTHING said so, because every `#4690` refusal * below fires on an EMPTY population or an empty candidate set. A population * that is complete-minus-one refuses nothing and prints a confident scope line. @@ -537,21 +539,34 @@ function selfTest() { // here and stayed. battery('the population is imported, never re-walked'); { - const SPECIMEN = 'packages/lint/scripts/check-reference-carrier-shape.mjs'; const live = collectPopulation(); ok( live.refusal === null && live.population.length > 0, `the imported population could not be read (${live.refusal ?? 'empty'}), so the cases below prove nothing (#4690)`, ); + // The specimen these two pins named was + // `packages/lint/scripts/check-reference-carrier-shape.mjs` — the tree's only + // package-local gate invocation, and the file whose absence from this gate's + // private walk was the 169/168 split. It was retired by maintainer ruling, so + // the package-local lane is now EMPTY (measured at the retirement: + // `packageLocal` 1 → 0, `population` 214 → 213). + // + // What the pins were really holding is that this gate takes the IMPORTED + // population whole rather than re-deriving one, and that survives the lane + // emptying: every package-local member the shared reader hands over is scanned + // here on the same terms as a root script. Both halves are asserted over the + // whole population, so they hold at zero package-local members and start + // judging the day one returns — ⛔ no re-pointing at a hopeful path. ok( - live.population.includes(SPECIMEN) && live.packageLocal.length > 0, - `${SPECIMEN} is not in the population this gate scans. CI runs its --self-test on every pull request; ` - + 'out of the population, its output is in no sweep and nothing says so (#15414)', + live.packageLocal.every((s) => live.population.includes(s) && typeof isCandidate(s, live.sources.get(s) ?? '') === 'boolean'), + 'a package-local member of the imported population is not scanned on the same terms as a root script. ' + + "CI runs such a gate's --self-test on every pull request; out of this sweep its output is in no " + + 'sweep at all and nothing says so (#15414)', ); ok( - typeof isCandidate(SPECIMEN, live.sources.get(SPECIMEN) ?? '') === 'boolean', - 'the prefilter could not be applied to the package-local member — it is filtered on exactly the same ' - + 'terms as a root script, and its VALUE is a measurement of that file, deliberately not pinned here', + live.population.every((s) => typeof isCandidate(s, live.sources.get(s) ?? '') === 'boolean'), + 'the prefilter could not be applied to a population member — it is filtered on exactly the same ' + + 'terms for every member, and its VALUE is a measurement of that file, deliberately not pinned here', ); ok( live.workflowDir === WORKFLOW_DIR, diff --git a/scripts/pm/dispatch-gates.mjs b/scripts/pm/dispatch-gates.mjs index b6fcb3cf7d4..8014e2b2caf 100644 --- a/scripts/pm/dispatch-gates.mjs +++ b/scripts/pm/dispatch-gates.mjs @@ -14451,8 +14451,10 @@ function selfTest() { // ── The package-local gate lane: a path is keyed WHOLE (#15342) ─────────── // - // `lint.yml` invokes `packages/lint/scripts/check-reference-carrier-shape.mjs` - // by path, twice. Before the directory prefix documented beside the patterns, + // `lint.yml` invoked `packages/lint/scripts/check-reference-carrier-shape.mjs` + // by path, twice (that gate is retired; the lane has no live member today, so + // these cases are synthetic on purpose and are what holds the grammar). + // Before the directory prefix documented beside the patterns, // `node ` had to be followed IMMEDIATELY by `scripts/`, so neither matcher saw // that step at all: no family, no hints, and nothing for `--residue` to place. // @@ -14981,13 +14983,25 @@ function selfTest() { const direct = liveInvs.filter((i) => i.direct); const valueBearing = direct.filter((i) => (i.argvVariables ?? []).length > 0); // The live half of the package-local lane (#15342). The fixtures above prove - // the pattern; these two read the tree CI actually runs, so the day the lane - // moves this reds here instead of going quiet — and the second one holds the - // phantom class over EVERY direct invocation, not only over the specimen. + // the pattern; these read the tree CI actually runs, so the phantom class is + // held over EVERY direct invocation, not only over a specimen. + // + // The specimen the first of these used to name — + // `packages/lint/scripts/check-reference-carrier-shape.mjs` — was the tree's + // ONLY package-local by-path invocation, and it was retired by maintainer + // ruling. So the lane did not move, it emptied, and a live pin on it could + // only ever be a pin on zero from here. The reading is recorded as a zero WITH + // ITS CONTROL: no `packages/…` direct invocation, while the same extraction + // over the same corpus yields 143 root ones — so the zero is an empty lane and + // not a reader that stopped matching. The grammar itself stays under the + // synthetic fixtures above. ⛔ Do not widen the matchers to manufacture a + // subject; the day CI invokes a package-local gate by path, the fixtures + // already key it and a specimen can be named here again. t( - '⭐ the live corpus really carries the package-local lane the directory prefix exists for, so the ' - + 'fixtures above judge a live class. If this reds, the lane moved — re-point it, never widen further', - direct.some((i) => i.script === 'packages/lint/scripts/check-reference-carrier-shape.mjs'), + `⭐ the package-local lane reads as EMPTY against a corpus that yields ${direct.length} direct root ` + + 'invocation(s) — the control that makes the zero a reading. If the control collapses to 0 the ' + + 'extraction broke; if a `packages/…` direct invocation appears, name it as the specimen again', + direct.length > 0 && !direct.some((i) => i.script.startsWith('packages/')), ); t( `every one of the ${direct.length} direct invocation(s) in the live tree resolves to a file that EXISTS ` @@ -23528,9 +23542,16 @@ function selfTest() { ); const liveCommands = liveTail.rows.flatMap((r) => r.commands); t('the live tail is not empty — an empty one would mean the walk broke, not that CI runs nothing', liveTail.rows.length > 0); - // #13333's first live instance was a package-local gate invoked by path. It is - // no longer here, and that is #15342 landing rather than this pin rotting — - // both halves are asserted for the reason the fixture case above states. + // #13333's first live instance was a package-local gate invoked by path. It + // left the unmeasured tail when #15342 gave the derivation a family for it, and + // it has since left the TREE: the gate was retired by maintainer ruling, so + // there is no package-local by-path invocation left to be accounted for. + // + // The pin keeps the half that is still about this file's walk — the tail names + // no such step — and states the second half as a zero with a control, because + // "the derivation names a family for it" has no `it` any more. ⛔ The control is + // not decoration: `liveDirectScripts` empty would satisfy the zero for the wrong + // reason, which is this whole file's failure mode. const liveDirectScripts = new Set( readdirSync(nodePath.join(ROOT, '.github/workflows')) .filter((f) => /\.ya?ml$/.test(f)) @@ -23539,10 +23560,11 @@ function selfTest() { .map((i) => i.script), ); t( - 'the INSTANCE the card was filed about has LEFT the live tail (#15342), and the derivation now names a ' - + 'family for it — the tail shrank because the step became accounted for, not because the walk broke', + 'the INSTANCE the card was filed about has LEFT the live tail (#15342) and then the tree — the tail ' + + 'names no package-local step, and the derivation still reaches the direct invocations that remain', !liveCommands.some((c) => /^node\s+packages\/\S+\/check-[\w.-]+\.mjs/.test(c)) - && liveDirectScripts.has('packages/lint/scripts/check-reference-carrier-shape.mjs'), + && liveDirectScripts.size > 0 + && ![...liveDirectScripts].some((s) => s.startsWith('packages/')), ); // The class assertion. The instance above is invisible because its path is // not under `scripts/`; this one is invisible because its INTERPRETER is not