From 485ce7d1c2cfd1bc48257b363b2b5348f198e728 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 10:26:32 +0000 Subject: [PATCH] refactor(lint): derive the runtime gate's name-keyed collection set The set of collections the runtime publish gate carries in a per-write snapshot was written down in five places, and `NAME_KEYED_STACK_KEYS` was the one with no guard of any kind: `CONTEXT_STACK_KEYS` carries a `satisfies` clause (validity, not completeness) and the compiler held nothing else. That list carries a real invariant. A collection the CONTEXT fills AND that some write type maps into must be name-keyed, or a finding's `path` is a positional index into an in-memory snapshot the caller has never seen and cannot enumerate. Omitting a member did not fail to build, fail a test, or fail a gate; it emitted correct-LOOKING findings the receiver cannot resolve. Adding the `pages` collection had to touch all five spellings and only one of them announced itself. `NAME_KEYED_STACK_KEYS` and the `TOP_LEVEL_INDEX` pattern built from it are now derived from the two inputs that already state the answer: `CONTEXT_STACK_KEYS` intersected with the values of `TYPE_TO_STACK_KEY`. The intersection was measured against the list it replaces before anything changed: same four members in the same order, `datasets` excluded on its own because no write type maps into it. No member needed a hand-written exception and none is kept. Constructive preservation, not a tightening or a loosening: the derived pattern's `source` is byte-identical to the literal it replaces. The two hazards a derived alternation has and a literal did not - member escaping and prefix ordering - are decided in the builder's docblock and pinned on synthetic inputs, because the four real keys cannot exercise either. No published entry point changed: the new exports are module-level, for the pin, and are on neither `@objectstack/lint` nor `@objectstack/lint/runtime`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC --- .changeset/derive-name-keyed-stack-keys.md | 30 ++++ .../runtime-gate.derived-name-keys.test.ts | 148 ++++++++++++++++++ packages/lint/src/runtime-gate.ts | 145 ++++++++++++++--- 3 files changed, 301 insertions(+), 22 deletions(-) create mode 100644 .changeset/derive-name-keyed-stack-keys.md create mode 100644 packages/lint/src/runtime-gate.derived-name-keys.test.ts diff --git a/.changeset/derive-name-keyed-stack-keys.md b/.changeset/derive-name-keyed-stack-keys.md new file mode 100644 index 0000000000..391fcf06d9 --- /dev/null +++ b/.changeset/derive-name-keyed-stack-keys.md @@ -0,0 +1,30 @@ +--- +"@objectstack/lint": patch +--- + +refactor(lint): derive the runtime gate's name-keyed collection set instead of hand-listing it (#13390) + +The set of collections the runtime publish gate carries in a per-write snapshot was +written down in five places, and `NAME_KEYED_STACK_KEYS` was the one with no guard of +any kind — `CONTEXT_STACK_KEYS` carries a `satisfies` clause, which is validity rather +than completeness, and the compiler held nothing else. + +That list carries a real invariant: a collection the CONTEXT fills **and** that some +write type maps into must be name-keyed, or a finding's `path` is a positional index +into an in-memory snapshot the caller has never seen and cannot enumerate — the defect +#10064 fixed for `objects` / `permissions` / `books`. Omitting a member did not fail to +build, fail a test, or fail a gate; it produced correct-LOOKING findings with paths the +receiver cannot resolve. Adding the `pages` collection had to touch all five spellings +and only one of them announced itself. + +`NAME_KEYED_STACK_KEYS` and the `TOP_LEVEL_INDEX` pattern built from it are now derived +from the two inputs that already state the answer: `CONTEXT_STACK_KEYS` intersected with +the values of `TYPE_TO_STACK_KEY`. The intersection was measured against the list it +replaces before anything changed — same four members (`objects`, `permissions`, `books`, +`pages`) in the same order, and `datasets` excluded on its own because no write type maps +into it, so no member needed a hand-written exception and none is kept. + +Constructive preservation, not a tightening or a loosening: the derived pattern's `source` +is byte-identical to the literal it replaces, and the gate returns the same findings for +the same inputs. No published entry point changed — `@objectstack/lint` and +`@objectstack/lint/runtime` export exactly the names they did before. diff --git a/packages/lint/src/runtime-gate.derived-name-keys.test.ts b/packages/lint/src/runtime-gate.derived-name-keys.test.ts new file mode 100644 index 0000000000..03c201bf26 --- /dev/null +++ b/packages/lint/src/runtime-gate.derived-name-keys.test.ts @@ -0,0 +1,148 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#13390] The name-keyed collection set is DERIVED, and this is what makes the + * derivation load-bearing rather than decorative. + * + * ## What was wrong with the list + * + * The set of collections a per-write snapshot carries was written down in five + * places. `NAME_KEYED_STACK_KEYS` was the one with no guard of any kind, and it + * carries a real invariant: a collection the CONTEXT fills **and** that some + * write type maps into must be name-keyed, or a finding's `path` is a positional + * index into an in-memory snapshot the caller has never seen and cannot + * enumerate — the #10064 defect. Adding `pages` (#13216) had to touch all five + * and only one announced itself. Omitting this one produced correct-LOOKING + * findings and no test, type or gate went red. + * + * ## What this file pins, and why in this shape + * + * The four keys that exist today agree with the list they replaced. That shows + * the answer is right, and cannot show the derivation is the REASON — the whole + * value of this card is about the NEXT widening. So every claim here is made + * one of two ways: + * + * - against the module's own inputs (`WRITTEN_STACK_KEYS` and the context set + * read back out of a real snapshot), never against a restated list — a test + * that hand-listed the members would be a sixth spelling of the same set; + * - on SYNTHETIC inputs through the exported pure builders, which is the only + * way to exercise a widening that has not happened, and the only way to + * exercise the two hazards a derived regex has and a literal did not. + */ + +import { describe, expect, it } from 'vitest'; + +import { + WRITTEN_STACK_KEYS, + buildRuntimeWriteSnapshots, + buildTopLevelIndexPattern, + deriveNameKeyedStackKeys, + nameKeyFindingPath, +} from './runtime-gate.js'; + +/** + * The context set, read back from a REAL snapshot rather than imported as a + * constant: `buildRuntimeWriteSnapshots` gives the baseline one key per context + * collection, so this is the set the gate actually carries, not a claim about it. + */ +const contextStackKeys = Object.keys( + buildRuntimeWriteSnapshots({ type: 'object', item: { name: 'probe_object' } })!.baseline, +); + +describe('the derived name-keyed set (#13390)', () => { + it('reproduces exactly the membership the hand list carried — four, in the same order', () => { + expect(deriveNameKeyedStackKeys(contextStackKeys, WRITTEN_STACK_KEYS)).toEqual([ + 'objects', + 'permissions', + 'books', + 'pages', + ]); + }); + + it('excludes `datasets` by derivation, not by a written-down exception', () => { + // The old comment had to STATE this. Now it falls out: the context fills + // `datasets`, and no write type lands an item in it, so a `datasets[0]` + // path is not a position in anything the caller cannot enumerate. + expect(contextStackKeys).toContain('datasets'); + expect(WRITTEN_STACK_KEYS.has('datasets')).toBe(false); + }); + + it.each(contextStackKeys)( + '%s is name-keyed exactly when a write type maps into it', + (key) => { + // The invariant end to end, asked per context collection against the same + // table the gate consults. A hand-kept list that forgot a member — the + // #13216 near-miss — fails here; so does one that name-keys a + // context-only collection. + const candidate = { [key]: [{ name: 'acme_thing' }] }; + const rewritten = nameKeyFindingPath(`${key}[0].sharingModel`, candidate); + + expect(rewritten).toBe( + WRITTEN_STACK_KEYS.has(key) + ? `${key}.acme_thing.sharingModel` + : `${key}[0].sharingModel`, + ); + }, + ); + + it('takes the context order, so a write-table reordering cannot move it', () => { + expect(deriveNameKeyedStackKeys(['c', 'b', 'a'], ['a', 'b'])).toEqual(['b', 'a']); + }); + + it('a write type mapping onto a NON-context key contributes nothing', () => { + // `flow` -> `flows` is a real mapping onto a collection the context never + // fills; `flows[0]` IS the write, trivially stable, and must stay positional. + expect(deriveNameKeyedStackKeys(['objects'], ['objects', 'flows'])).toEqual(['objects']); + expect(nameKeyFindingPath('flows[0].name', { flows: [{ name: 'acme_flow' }] })).toBe( + 'flows[0].name', + ); + }); + + it('the next widening cannot half-land', () => { + // The acceptance criterion, stated synthetically because the real widening + // has not happened yet: adding a context collection that a write type maps + // into name-keys it and joins the pattern, with no second edit to forget. + const widenedContext = [...contextStackKeys, 'reports']; + const widenedWrites = new Set([...WRITTEN_STACK_KEYS, 'reports']); + + const derived = deriveNameKeyedStackKeys(widenedContext, widenedWrites); + expect(derived).toContain('reports'); + expect(buildTopLevelIndexPattern(derived).exec('reports[7].title')?.[1]).toBe('reports'); + }); +}); + +describe('the derived top-level index pattern (#13390)', () => { + it('rebuilds the literal it replaced, source for source', () => { + // Constructive preservation, byte for byte: same members, same order, so + // the derived pattern and the hand-written one are the same regex. + expect(buildTopLevelIndexPattern(['objects', 'permissions', 'books', 'pages']).source).toBe( + /^(objects|permissions|books|pages)\[(\d+)\](.*)$/.source, + ); + }); + + it('escapes members instead of trusting them to be `[a-z]+`', () => { + const re = buildTopLevelIndexPattern(['a.c']); + expect(re.test('a.c[0].x')).toBe(true); + // An unescaped `.` matches any character — the silent-widening direction. + expect(re.test('abc[0].x')).toBe(false); + }); + + it.each([ + ['short branch first', ['page', 'pages']], + ['long branch first', ['pages', 'page']], + ])('resolves a prefix pair regardless of order (%s)', (_label, keys) => { + // Alternation IS ordered, so `page|pages` reads as though it shadows + // `pages`. The `\[` anchor fails the short branch and forces a backtrack + // into the long one — measured here in both orders, which is why the + // builder does not carry a longest-first sort it would never exercise. + const re = buildTopLevelIndexPattern(keys); + expect(re.exec('pages[3].x')?.[1]).toBe('pages'); + expect(re.exec('page[3].x')?.[1]).toBe('page'); + }); + + it('an empty set matches nothing, rather than every top-level index', () => { + const re = buildTopLevelIndexPattern([]); + expect(re.test('objects[0].x')).toBe(false); + expect(re.test('[0].x')).toBe(false); + }); +}); diff --git a/packages/lint/src/runtime-gate.ts b/packages/lint/src/runtime-gate.ts index 2f6f621e9c..3f5d048601 100644 --- a/packages/lint/src/runtime-gate.ts +++ b/packages/lint/src/runtime-gate.ts @@ -65,6 +65,12 @@ type AnyRec = Record; * Only the types some rule declares in `runtimeTypes` need an entry; the guard * in `authoring-rule-wiring.test.ts` fails if a declared type is missing one, * so widening the gate cannot half-land. + * + * [#13390] The VALUES here are also one of the two inputs + * {@link NAME_KEYED_STACK_KEYS} is derived from — a stack key that some write + * type maps into is a key whose top-level index the caller cannot resolve. Adding + * a mapping onto a context collection therefore name-keys it by construction; it + * is no longer a second edit that nothing checks. */ const TYPE_TO_STACK_KEY: Readonly> = { flow: 'flows', @@ -413,30 +419,118 @@ export function buildRuntimeWriteSnapshots(args: { return { baseline, candidate }; } +/** + * The name-keyed stack keys implied by a context shape and a write-type table: + * the context collections that some write type ALSO lands an item inside. + * + * Exported (#13390) as a pure function of its two inputs so the derivation can + * be exercised on SYNTHETIC sets. The real inputs are four keys that agree with + * the list they replaced, which shows the answer is right today and cannot show + * that the DERIVATION is the reason — the property this card buys is about the + * next widening, so it has to be measured on inputs that widen. + * + * Order follows `contextStackKeys`, deliberately: it keeps the derived value + * comparable to the hand list it replaced position for position, and it keeps + * the pattern built from it byte-identical to the literal it replaced. + */ +export function deriveNameKeyedStackKeys( + contextStackKeys: readonly string[], + writtenStackKeys: Iterable, +): readonly string[] { + const written = new Set(writtenStackKeys); + return contextStackKeys.filter((key) => written.has(key)); +} + +/** + * `['objects', 'pages']` → `/^(objects|pages)\[(\d+)\](.*)$/` — the top-level + * index matcher, BUILT from the name-keyed set instead of restating it (#13390). + * + * A derived alternation has two hazards a hand-written literal did not, and both + * are decided here rather than left implicit: + * + * - **Escaping.** Every member today is `[a-z]+`, so nothing needs escaping and + * nothing would notice if it were skipped. But a stack key is a + * {@link RuntimeStackContext} property name, and a quoted one may hold a `.` + * or a `-`; an unescaped `.` matches ANY character, which is the silent-failure + * direction. Members are escaped rather than trusted — one `replace`. + * - **Prefix ordering.** Alternation is ordered, so `page|pages` reads as though + * the short branch shadows the long one. It does not in THIS pattern: the group + * is anchored by `\[`, which fails the short branch and forces the engine to + * backtrack into the long one. That is a property of the anchor, not of + * alternation — so it is pinned by test with a synthetic `page` / `pages` pair + * in BOTH orders, rather than papered over with a longest-first sort that would + * silently stop being exercised and would leave the claim untested either way. + * + * An empty set yields a pattern matching nothing. Interpolating it would produce + * `^()\[(\d+)\](.*)$`, which name-keys EVERY top-level index — the failure + * direction that widens the rewrite instead of narrowing it. + */ +export function buildTopLevelIndexPattern(stackKeys: readonly string[]): RegExp { + if (stackKeys.length === 0) return /(?!)/; + const alternation = stackKeys.map((key) => key.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')).join('|'); + return new RegExp(`^(${alternation})\\[(\\d+)\\](.*)$`); +} + +/** + * The stack keys some write type lands an item INSIDE — the VALUES of + * {@link TYPE_TO_STACK_KEY}, read off the table rather than restated, so a new + * `type → key` mapping cannot arrive without this set seeing it. + * + * Exported for the pin in `runtime-gate.derived-name-keys.test.ts` and for that + * only — it is not on either package entry. The pin asks, per context + * collection, whether a top-level index is name-keyed, and it must ask that + * against the SAME table the gate uses; a test that restated the answer would + * be a sixth hand-written spelling of the very set this card removed. + */ +export const WRITTEN_STACK_KEYS: ReadonlySet = new Set(Object.values(TYPE_TO_STACK_KEY)); + /** * The collection-resident stack keys whose TOP-LEVEL index the gate rewrites - * to a name key before findings leave it (#10064). + * to a name key before findings leave it (#10064) — DERIVED, not listed (#13390). * * These are the collections a written item lands INSIDE **and** that the - * context also fills (`TYPE_TO_STACK_KEY` routes `object` / `permission` / - * `book` / `page` writes into them) — so a finding's `objects[417]` is an - * offset into this gate's per-write snapshot, an in-memory array the caller has - * never seen and cannot enumerate. Every other write type is the sole member of - * its own collection (`flows[0]` IS this write, trivially stable), and - * `datasets` is context-only — no write type maps into it — so both keep their - * positional spelling. - * - * [#13216] `pages` JOINED this list in the same change that made `pages` a - * context collection, and the pairing is the rule rather than a coincidence: - * before that, a `page` write's snapshot held exactly one page, so `pages[0]` - * was this write, trivially stable, and name-keying it would have been - * pointless. The moment the live universe joins the snapshot, the index stops - * meaning anything to the caller — `validatePresetComparands` already runs on - * `page` writes and emits paths into this collection. So: adding a key to - * {@link CONTEXT_STACK_KEYS} that some write type ALSO maps into means adding - * it here too. + * context also fills — so a finding's `objects[417]` is an offset into this + * gate's per-write snapshot, an in-memory array the caller has never seen and + * cannot enumerate. Every other write type is the sole member of its own + * collection (`flows[0]` IS this write, trivially stable), and a context-only + * collection holds no write at all — so both keep their positional spelling. + * + * ## Why it is derived + * + * That paragraph is not a judgement call, it is two conditions intersected, and + * both are already written down: the context fills the collection + * ({@link CONTEXT_STACK_KEYS}) and some write type maps into it + * ({@link TYPE_TO_STACK_KEY}). Kept as a literal it was the one spelling of that + * set with NO guard — `CONTEXT_STACK_KEYS` carries a `satisfies` clause, which + * is validity, not completeness, and the compiler holds nothing else. Omitting a + * member here did not fail to build, fail a test, or fail a gate; it emitted + * findings that LOOK correct whose `path` the caller cannot resolve, which is + * the #10064 defect re-created silently. + * + * [#13216] `pages` is the measurement that made the case: adding it touched + * FIVE spellings of this one set and only the fifth announced itself — the one + * the compiler could see, and only after that accumulator was retyped as a + * mapped type. The pairing is the rule rather than a coincidence. Before the + * live page universe joined the snapshot, a `page` write's snapshot held exactly + * one page, so `pages[0]` WAS this write and name-keying it would have been + * pointless; the moment the universe joins, the index stops meaning anything to + * the caller (`validatePresetComparands` already runs on `page` writes and emits + * paths into this collection). Derived, the two move together by construction + * and the next widening is a one-key edit again. + * + * ## Measured against the list it replaces (#13390) + * + * Same four members in the same order — `objects`, `permissions`, `books`, + * `pages`. `datasets` falls out on its own, for exactly the reason the old + * comment had to state by hand: it is context-only, no write type maps into it. + * So **no member needed a hand-written exception** and none is kept. If a future + * member ever does need one, state it here WITH its reason — quietly + * re-introducing a literal is the thing this constant now exists to prevent. */ -const NAME_KEYED_STACK_KEYS = ['objects', 'permissions', 'books', 'pages'] as const; +const NAME_KEYED_STACK_KEYS: readonly string[] = deriveNameKeyedStackKeys( + CONTEXT_STACK_KEYS, + WRITTEN_STACK_KEYS, +); /** * Machine names safe to splice into a dotted path. Matches the spec's @@ -446,7 +540,7 @@ const NAME_KEYED_STACK_KEYS = ['objects', 'permissions', 'books', 'pages'] as co */ const PATH_SAFE_NAME = /^[A-Za-z_][A-Za-z0-9_]*$/; -const TOP_LEVEL_INDEX = /^(objects|permissions|books|pages)\[(\d+)\](.*)$/; +const TOP_LEVEL_INDEX = buildTopLevelIndexPattern(NAME_KEYED_STACK_KEYS); /** * `objects[417].sharingModel` → `objects.acme_invoice.sharingModel` (#10064). @@ -458,6 +552,13 @@ const TOP_LEVEL_INDEX = /^(objects|permissions|books|pages)\[(\d+)\](.*)$/; * purpose — within one named item they index the author's own document, which * the receiver holds and can resolve. * + * [#13390] Exported for the pin, not for callers (it is on neither package + * entry). It is the one place the derived set and the derived pattern MEET, so + * it is where the invariant is observable end to end: for each context + * collection, is the top-level index rewritten exactly when a write type maps + * into that collection? Asked here, the answer cannot be produced by a list + * that agrees with the derivation by luck. + * * Fallback is the positional spelling, never a hole: an entry that is missing, * unnamed, or whose name will not splice into a dotted path keeps the index. * @@ -466,11 +567,11 @@ const TOP_LEVEL_INDEX = /^(objects|permissions|books|pages)\[(\d+)\](.*)$/; * stored items that (illegitimately) share a name must not have their distinct * findings merged or cancelled by the rewrite. */ -function nameKeyFindingPath(path: string, candidate: AnyRec): string { +export function nameKeyFindingPath(path: string, candidate: AnyRec): string { const m = TOP_LEVEL_INDEX.exec(path); if (!m) return path; const [, stackKey, index, rest] = m; - if (!(NAME_KEYED_STACK_KEYS as readonly string[]).includes(stackKey!)) return path; + if (!NAME_KEYED_STACK_KEYS.includes(stackKey!)) return path; const collection = candidate[stackKey!] as readonly unknown[] | undefined; const entry = collection?.[Number(index)]; const name = entry && typeof entry === 'object' ? (entry as AnyRec).name : undefined;