From c0e363cc6995441dc0aeae313d7fe20c166d76e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 14 Sep 2026 22:03:10 +0000 Subject: [PATCH] refactor(plugin-detail): rename the spec-named record:alert props interface (objectui#7265 plugin-detail slice) `record:alert`'s module-local `interface RecordAlertProps` is renamed to `RecordAlertRendererProps`, the spelling every other renderer in that directory already uses. It was the last entry in `check:spec-symbols`' rule-1 DEBT ledger, which the `--ledger` regeneration in this commit therefore leaves EMPTY. Route by measurement, not preference. At the resolved `@objectstack/spec` pin, `@objectstack/spec/ui` exports `RecordAlertProps` for the block's AUTHORED properties -- the bag this interface nests under `schema.properties` -- and owns a `RecordProps` for every sibling block the directory renders. The local symbol was the React props envelope one level up (`schema` + `className` + an open tail), so BIND had nothing to bind to; and an `interface` is recorded as derived only through `extends`, so the third route a TYPE usually has was not available to a wrapper around the type it wraps. What settled it is that the siblings already spell the distinction: the `Renderer` infix is exactly why none of them was ever in this ledger. Type-only, and no published face moves -- the declaration was never exported and the package entry imports the COMPONENT. Also in the same file, because the empty ledger makes both live: - The stale-entry message no longer spells a GitHub closing keyword in front of the ledger anchor's own number. Taking the INTERMEDIATE reading and quoting it is standing practice on this card, so that text reaches pull-request bodies by design and the keyword put a card-ending trigger in every slice's merge path. The constraint is now written at the ratchet that emits it and pinned by a test with a lit control in both directions. The CLAIM_DEBT twin is deliberately untouched: its anchor is already a closed issue. - The near-neighbour comment in the gate's own test file called `isContextToken` and `normalizeFilterOperator` "the live instances ... both DEBT entries today". This card burned both down -- the first BOUND, the second RENAMED -- so the examples are re-anchored to the commit that measured them, the way the `rendersJsx` docblock already does. The reasoning is kept; only the tense moved. Ratchets: the site, the block, the empty-ledger path and the message's wording in scripts/__tests__/spec-symbol-ledger-plugin-detail-7265.test.ts; the rename and the directory's convention, re-derived from the directory rather than listed, in packages/plugin-detail/src/__tests__/spec-symbol-batch7.test.ts. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011QreXiyMEqKLN4U5daMPVa --- ...ugin-detail-record-alert-renderer-props.md | 18 + .../src/__tests__/spec-symbol-batch7.test.ts | 83 ++++ .../renderers/__tests__/record-alert.test.tsx | 15 +- .../src/renderers/record-alert.readProps.ts | 4 +- .../src/renderers/record-alert.tsx | 19 +- .../check-spec-symbol-derivation.test.ts | 30 +- ...c-symbol-ledger-plugin-detail-7265.test.ts | 409 ++++++++++++++++++ scripts/check-spec-symbol-derivation.mjs | 71 ++- 8 files changed, 622 insertions(+), 27 deletions(-) create mode 100644 .changeset/7265-plugin-detail-record-alert-renderer-props.md create mode 100644 scripts/__tests__/spec-symbol-ledger-plugin-detail-7265.test.ts diff --git a/.changeset/7265-plugin-detail-record-alert-renderer-props.md b/.changeset/7265-plugin-detail-record-alert-renderer-props.md new file mode 100644 index 0000000000..ab567ad834 --- /dev/null +++ b/.changeset/7265-plugin-detail-record-alert-renderer-props.md @@ -0,0 +1,18 @@ +--- +--- + +Internal, type-only: `@object-ui/plugin-detail`'s `record:alert` renderer renames +its module-local props interface from `RecordAlertProps` to +`RecordAlertRendererProps`, the spelling every other renderer in that directory +already uses. `@objectstack/spec/ui` exports `RecordAlertProps` for the block's +AUTHORED properties — the bag this interface nests under `schema.properties` — +so the two names described different concepts, which is what +`pnpm check:spec-symbols` had been flagging. Nothing is published: the interface +was never exported, the renderer's own name and runtime behaviour are unchanged, +and the declaration's member list is byte-for-byte what it was. + +This is the last entry in that gate's rule-1 `DEBT` ledger (objectui#7265). The +same change rewords the gate's stale-entry message so it no longer spells a +GitHub closing keyword in front of the ledger anchor's own number — that message +is quoted into pull-request bodies by design on this card, and the keyword put a +card-ending trigger in the merge path of every slice that quoted its own reading. diff --git a/packages/plugin-detail/src/__tests__/spec-symbol-batch7.test.ts b/packages/plugin-detail/src/__tests__/spec-symbol-batch7.test.ts index 80e2c2ce47..6965d759a1 100644 --- a/packages/plugin-detail/src/__tests__/spec-symbol-batch7.test.ts +++ b/packages/plugin-detail/src/__tests__/spec-symbol-batch7.test.ts @@ -25,13 +25,19 @@ */ import { describe, it, expect } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import type * as React from 'react'; import { FeedFilterMode as SpecFeedFilterModeEnum } from '@objectstack/spec/data'; import type { FeedFilterMode as SpecFeedFilterMode } from '@objectstack/spec/data'; import type { ObjectFieldLike as SpecObjectFieldLike } from '@objectstack/spec/system'; +import type { RecordAlertProps as SpecRecordAlertProps } from '@objectstack/spec/ui'; import type { FeedFilterMode } from '../RecordActivityTimeline'; import type { ObjectDefFieldLike } from '../synth/buildDefaultPageSchema'; import { normalizeFilterMode } from '../renderers/recordActivityFeed'; +import type { RecordAlertRenderer } from '../renderers/record-alert'; describe('FeedFilterMode is the spec enum, at runtime as well as in types', () => { it('accepts every member the spec declares — read from the spec, not restated', () => { @@ -85,3 +91,80 @@ describe('the two verdicts are pinned at compile time', () => { expect(true).toBe(true); }); }); + +/* -------------------------------------------------------------------------- */ +/* objectui#7265, this package's slice — `RecordAlertProps`. */ +/* */ +/* Appended here rather than given a file of its own: this IS the package's */ +/* spec-symbol parity file, and the shape it already holds (a RENAME, pinned */ +/* by what the spec means rather than by what the copy said) is exactly the */ +/* shape this one needs. The block, the site and the empty-ledger path are */ +/* pinned one level up, in scripts/__tests__ beside the gate they are about. */ +/* -------------------------------------------------------------------------- */ + +describe('the renderer props convention this directory already kept', () => { + // ⛔ Rooted at THIS FILE, never at `process.cwd()` — a package test runs under + // two different cwds depending on the invocation (`check:test-path-roots`). + const renderersDir = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../renderers'); + + /** Every `React.FC` annotation in the renderers directory, file by file. */ + const annotations = (): Array<{ file: string; propsType: string }> => { + const out: Array<{ file: string; propsType: string }> = []; + for (const name of fs.readdirSync(renderersDir).sort()) { + if (!name.endsWith('.tsx')) continue; + const text = fs.readFileSync(path.join(renderersDir, name), 'utf8'); + for (const hit of text.matchAll(/React\.FC<\s*([A-Za-z0-9_]+)\s*>/g)) { + out.push({ file: name, propsType: hit[1] }); + } + } + return out; + }; + + it('the scan finds annotations at all — the control', () => { + // A convention derived from an empty population is a green that means + // nothing. This leg fails if the directory moves, is renamed, or stops + // spelling its components `React.FC<…>`. + expect(annotations().length).toBeGreaterThan(1); + }); + + it('every renderer names its props type `…RendererProps`, never the block name', () => { + // This is the reason `RecordAlertProps` was the LAST entry in the gate's + // DEBT ledger while its siblings were never in it: `@objectstack/spec/ui` + // owns a `RecordProps` for each of the blocks rendered here, so the + // `Renderer` infix is what keeps a renderer's props type out of the spec's + // namespace. Derived from the directory rather than listed, so a renderer + // added later is judged too. + const offenders = annotations().filter((a) => !a.propsType.endsWith('RendererProps')); + expect( + offenders, + 'a renderer in this directory types its component with a props type that is not ' + + 'spelled `…RendererProps`. If the name is one @objectstack/spec exports, ' + + '`pnpm check:spec-symbols` will fail on it as a hand-written mirror of the ' + + "block's authored properties — which is what objectui#7265's last slice " + + 'repaired. Rename the props type, or delete this assertion deliberately.', + ).toEqual([]); + }); +}); + +describe('RecordAlertRendererProps is NOT the spec bag it wraps', () => { + it('is pinned at compile time', () => { + // The measurement that refused BIND, as assertions. The spec's + // `RecordAlertProps` is the block's AUTHORED property bag; the renderer's + // props are the React envelope that carries it. `severity` belongs to the + // first and `schema` to the second, and the spec's declaration has no index + // signature, so both questions have real answers on that side. + type _SpecIsReal = Assert, false>>; + type _SpecCarriesSeverity = Assert>; + type _SpecIsNotTheEnvelope = Assert, false>>; + + // ⛔ Deliberately NOT asserted with `HasKey` on the renderer's side: its + // props carry `[k: string]: any`, so `K extends keyof T` is true for every + // K there and the question would answer itself. Identity is the probe that + // still discriminates. + type _RendererIsNotTheBag = Assert< + Equal, SpecRecordAlertProps>, false> + >; + + expect(true).toBe(true); + }); +}); diff --git a/packages/plugin-detail/src/renderers/__tests__/record-alert.test.tsx b/packages/plugin-detail/src/renderers/__tests__/record-alert.test.tsx index 620c7017f6..8cfdb19d40 100644 --- a/packages/plugin-detail/src/renderers/__tests__/record-alert.test.tsx +++ b/packages/plugin-detail/src/renderers/__tests__/record-alert.test.tsx @@ -201,13 +201,14 @@ describe('RecordAlertRenderer', () => { // objectui#4998 — `action.label` accepts the inline locale map, not just a // plain string. The renderer already resolved one before this pin existed // (`const ctaLabel = pickLocalized(props.action?.label, language)`), so a - // plain-string CTA label is green whether or not the local `RecordAlertProps` - // type admits a map — that's the `label: 'Send again'` case just above, and - // it cannot tell the widened declaration apart from the narrow one it - // replaced. This case can: a bare `label?: string` refused a locale-map - // value at the TYPE level even though `pickLocalized` (an `unknown` input) - // already resolved it correctly at runtime, exactly the `title` / `body` - // contradiction objectui#4970 fixed one level up in the same interface. + // plain-string CTA label is green whether or not the local + // `RecordAlertRendererProps` type admits a map — that's the + // `label: 'Send again'` case just above, and it cannot tell the widened + // declaration apart from the narrow one it replaced. This case can: a bare + // `label?: string` refused a locale-map value at the TYPE level even though + // `pickLocalized` (an `unknown` input) already resolved it correctly at + // runtime, exactly the `title` / `body` contradiction objectui#4970 fixed + // one level up in the same interface. it('resolves an inline locale-map action.label to the active-language string (objectui#4998)', () => { render( diff --git a/packages/plugin-detail/src/renderers/record-alert.readProps.ts b/packages/plugin-detail/src/renderers/record-alert.readProps.ts index 3103c9ad3d..8794117399 100644 --- a/packages/plugin-detail/src/renderers/record-alert.readProps.ts +++ b/packages/plugin-detail/src/renderers/record-alert.readProps.ts @@ -10,8 +10,8 @@ import { isConfigBag } from '@object-ui/react'; /** * The config bag `record:alert` reads: the node's own keys UNDERNEATH (the - * legacy flat spelling `RecordAlertProps` tolerates) and `properties` on top. - * It is the sixth member of the `readProps()` family that + * legacy flat spelling `RecordAlertRendererProps` tolerates) and `properties` + * on top. It is the sixth member of the `readProps()` family that * `packages/components/src/__tests__/alias-precedence-cross-channel.test.tsx` * names, spelled differently — `{ ...schema, ...schema.properties }`, no * `props` alias leg — and since objectui#6790 it asks the same question the diff --git a/packages/plugin-detail/src/renderers/record-alert.tsx b/packages/plugin-detail/src/renderers/record-alert.tsx index a11b10f9c9..235000e4d9 100644 --- a/packages/plugin-detail/src/renderers/record-alert.tsx +++ b/packages/plugin-detail/src/renderers/record-alert.tsx @@ -102,6 +102,21 @@ type Severity = 'info' | 'warning' | 'error' | 'success'; /** * Local (unexported) prop shape for the renderer below. * + * ⚠️ Spelled `RecordAlertRendererProps`, NOT `RecordAlertProps`. At the RESOLVED + * `@objectstack/spec` pin, `@objectstack/spec/ui` exports `RecordAlertProps` — + * and a `RecordProps` for every sibling block this directory renders — + * for the block's AUTHORED properties — the key set nested under + * `schema.properties` below, and nothing else. What this declaration holds is a + * different thing one level up: the React props this renderer is called with, a + * `schema` node plus `className` plus an open tail. Every other renderer beside + * this one already spells that distinction `…RendererProps`; this file did not, + * which is why its name was rule 1's last entry in the `check:spec-symbols` DEBT + * ledger (objectui#7265) while its structurally identical siblings were never + * in it. Both halves are pinned — that the spec still owns the plain name, and + * that it does not own this one — in this package's spec-symbol file, together + * with the sibling convention itself, re-derived from the directory rather than + * restated here. + * * `title` / `body` accept the inline locale map as well as a plain string * (objectui#4970): both are read through `pickLocalized` further down, and the * block's published authoring surface declares the two arms @@ -120,7 +135,7 @@ type Severity = 'info' | 'warning' | 'error' | 'success'; * (`plugin-detail/src/index.tsx`), so there is no manifest arm to align — that * half stays parked on the `ComponentInput` member-shape question (PR #3795). */ -interface RecordAlertProps { +interface RecordAlertRendererProps { schema?: { properties?: { severity?: Severity; @@ -169,7 +184,7 @@ const SEVERITY_STYLES: Record = { }, }; -export const RecordAlertRenderer: React.FC = ({ schema = {}, className }) => { +export const RecordAlertRenderer: React.FC = ({ schema = {}, className }) => { const props = readProps(schema); const recordCtx = useRecordContext(); const record = recordCtx?.data; diff --git a/scripts/__tests__/check-spec-symbol-derivation.test.ts b/scripts/__tests__/check-spec-symbol-derivation.test.ts index acec682455..472e5f3bc3 100644 --- a/scripts/__tests__/check-spec-symbol-derivation.test.ts +++ b/scripts/__tests__/check-spec-symbol-derivation.test.ts @@ -741,9 +741,10 @@ describe('the guard file itself', () => { * now judged structurally and none of them has an entry. * * The near neighbours are the point: `rendersJsx` must not become "functions are - * exempt" (that would silence `isContextToken` and `normalizeFilterOperator`, - * both real mirrors), and `isPureAlias` must not become "type aliases are - * exempt" (that would silence every hand-written union under a spec name). + * exempt" (that would have silenced `isContextToken` and + * `normalizeFilterOperator`, the two module-local functions the census above + * classified as real mirrors), and `isPureAlias` must not become "type aliases + * are exempt" (that would silence every hand-written union under a spec name). */ describe("rule 1 sees module-local declarations, and the narrowings say which it may not", () => { const RULE1_SPEC_NAMES = new Map>([ @@ -790,11 +791,24 @@ const NavigationConfig = () => ; it('…but a module-local FUNCTION that renders nothing is still a fork', () => { // The near neighbour that keeps `rendersJsx` from decaying into - // `isRendererLike`. `isContextToken` (@object-ui/core) and - // `normalizeFilterOperator` (@object-ui/data-objectstack) are the live - // instances: non-exported functions under spec export names, both real - // mirrors, both DEBT entries today. A blanket "functions are renderers" - // would have made them invisible instead — silently, and for good. + // `isRendererLike`. On the objectui#6291 commit — the widening this block + // proves — the module-local functions the census classified as real mirrors + // were `isContextToken` (@object-ui/core) and `normalizeFilterOperator` + // (@object-ui/data-objectstack): non-exported functions under spec export + // names, and both went into the DEBT ledger. A blanket "functions are + // renderers" would have made them invisible instead — silently, and for + // good. + // + // ⚠️ Read that as the measurement it was, ⛔ not as a census of today's tree. + // objectui#7265 burned both names down — `isContextToken` BOUND to the + // spec's own export, so no local declaration of it survives anywhere under + // `packages/**/src/**`, and `normalizeFilterOperator` RENAMED to + // `toAstFilterOperator` because the two folds have different codomains. The + // fixture below is therefore the shape the narrowing was designed against, + // not a name you will find in the ledger; what has to keep holding is the + // narrowing, which is why the fixture is written out here instead of read + // off the tree. Whether any name is in the ledger at all is a question + // `--ledger` answers and this comment deliberately does not. withFixture( { 'predicate.ts': ` diff --git a/scripts/__tests__/spec-symbol-ledger-plugin-detail-7265.test.ts b/scripts/__tests__/spec-symbol-ledger-plugin-detail-7265.test.ts new file mode 100644 index 0000000000..643236df28 --- /dev/null +++ b/scripts/__tests__/spec-symbol-ledger-plugin-detail-7265.test.ts @@ -0,0 +1,409 @@ +import { describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +// Plain-JS CI helper. Its types are INFERRED from the .mjs source by +// `tsconfig.scripts.json` (`allowJs`), so no `@ts-expect-error` here. +import { scanFile, specExportNames } from '../check-spec-symbol-derivation.mjs'; + +/** + * objectui#7265, the `@object-ui/plugin-detail` slice -- the LAST group, and the + * one whose landing leaves rule 1's `DEBT` block empty for the first time since + * objectui#6291 re-seeded it. + * + * Sibling of `spec-symbol-ledger-core-7265.test.ts`, + * `spec-symbol-ledger-app-shell-7265.test.ts`, + * `spec-symbol-ledger-types-7265.test.ts`, + * `spec-symbol-ledger-components-7265.test.ts` and + * `spec-symbol-ledger-data-objectstack-7265.test.ts`, same two-part shape, + * because a ledger needs both halves: + * + * 1. THE SITE. The real scanner, run over the real file. This is the half that + * reds if a local copy comes back -- deleting a name from a ledger is not a + * burn-down unless the declaration went with it. + * 2. THE BLOCK. Shrink-only is the card's own invariant, so it is asserted, + * not just respected. This slice is the one that can finally state it as an + * equality, because the floor and the ceiling have met. + * + * ...plus two halves that belong to nobody else, because nothing after this + * slice will be in a position to add them: + * + * 3. THE EMPTY-LEDGER PATH, live for the first time. `DEBT`, `DEBT_ISSUE` and + * the ratchet that reads them are the guard against RE-SEEDING, so an empty + * ledger is the end state rather than the cue to delete the machinery. + * 4. THE STALE-ENTRY MESSAGE's wording. That message is quoted into pull + * requests by design on this card; the reason its wording is constrained is + * written at the ratchet that emits it, and this is what makes the + * constraint fail loudly instead of being remembered. + * + * WHY THE ROUTE WAS RENAME, in one line: at the resolved pin the spec's + * `RecordAlertProps` is the AUTHORED property bag of the `record:alert` block, + * while the local declaration was the React props its RENDERER is called with -- + * the spec's bag nested one level down inside it. Different concepts under one + * name, and the directory's other renderers already spell the difference. + * + * ⚠️ The scanner is only evidence if it can fail, so every site reading here is + * paired with a fixture of the same kind that it MUST flag. A green scan with an + * empty `specNames` map, or over a path that does not exist, looks exactly like + * a green scan over a burned-down site. + */ + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const gateSource = path.join(repoRoot, 'scripts/check-spec-symbol-derivation.mjs'); +const gateTestSource = path.join(repoRoot, 'scripts/__tests__/check-spec-symbol-derivation.test.ts'); + +/** The name that was BURNED DOWN by renaming off it. */ +const FORMERLY = 'RecordAlertProps'; +/** What it is called now -- the `…RendererProps` spelling its siblings already use. */ +const RENAMED = 'RecordAlertRendererProps'; +/** The subpath the spec owns the old name on. */ +const SUBPATH = '@objectstack/spec/ui'; +/** The single file it lived in. */ +const SITE = 'packages/plugin-detail/src/renderers/record-alert.tsx'; +/** The ledger key a waiver would have been written under. */ +const ALLOW_KEY = `@object-ui/plugin-detail:${FORMERLY}`; + +/** + * The gate's OWN view of what `@objectstack/spec` exports, per subpath -- read + * from the gate rather than rebuilt here. A hand-written map would make every + * assertion below a statement about this file instead of about the spec, and it + * would keep passing after the spec stopped exporting the name that is the whole + * reason for the rename. + */ +const specNames = specExportNames().names as Map>; + +/** Reads one `const NAME = { … };` block out of the gate's source text. */ +function ledgerBlock(name: string): string { + const text = fs.readFileSync(gateSource, 'utf8'); + const start = text.indexOf(`const ${name} = {\n`); + expect(start, `${name} block not found in the gate source`).toBeGreaterThan(-1); + const end = text.indexOf('\n};\n', start); + expect(end, `${name} block is not terminated`).toBeGreaterThan(start); + return text.slice(start, end + 3); +} + +const ledgerNames = (block: string) => [...block.matchAll(/^ {4}"([^"]+)",$/gm)].map((m) => m[1]); + +/** A throwaway file, so a control cannot be satisfied by anything in the tree. */ +function withFixture(prefix: string, name: string, body: string, check: (file: string) => void) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + try { + const file = path.join(dir, name); + fs.writeFileSync(file, body); + check(file); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +} + +describe('the spec export-name probe this file judges against', () => { + it('reads a non-trivial number of names', () => { + // Guards every assertion below: an empty or collapsed map turns the site + // reading and the tripwire into the same confident green. + expect(specNames.size).toBeGreaterThan(1000); + }); + + it('sees TYPE-only exports, not just runtime values', () => { + // `RecordAlertPropsParsed` is a `z.infer<…>` alias — invisible to a runtime + // `import()`, so a probe that missed it could not answer the tripwire below + // for a name the spec publishes as a type. + expect(specNames.has('RecordAlertPropsParsed')).toBe(true); + }); +}); + +describe('the reason the route was RENAME', () => { + it(`the spec still owns \`${FORMERLY}\`, on ${SUBPATH}`, () => { + // If it ever stops, the rename's reason is spent and the plain name can be + // taken back. Asserted with the SUBPATH, because that is the half the + // failure message has to carry for the next reader to re-triage. + expect(specNames.has(FORMERLY)).toBe(true); + expect([...(specNames.get(FORMERLY) ?? [])]).toContain(SUBPATH); + }); + + it(`…and it does NOT own \`${RENAMED}\` — the tripwire the rename owes`, () => { + expect( + specNames.has(RENAMED), + `@objectstack/spec now exports \`${RENAMED}\`, which this package declares. ` + + 'The rename has re-created the collision under the new name. Rename again — and ' + + 'check the new name against this probe FIRST: objectui#3074 landed a rename ' + + 'straight onto another spec export.', + ).toBe(false); + }); + + it('the two names are different CONCEPTS, which is what BIND could not bridge', () => { + // The measurement, not the preference. The spec's symbol is the block's + // authored property bag; the local one was the renderer's React props, with + // that bag nested inside it. Read off the spec's own declared members rather + // than restated: `schema` is the wrapper's key and the spec's props bag has + // no business carrying it, while `severity` is the bag's and the wrapper + // never declared it at the top level. + const specSubpaths = [...(specNames.get(FORMERLY) ?? [])]; + expect(specSubpaths.length).toBeGreaterThan(0); + + const site = fs.readFileSync(path.join(repoRoot, SITE), 'utf8'); + const decl = site.slice(site.indexOf(`interface ${RENAMED} {`)); + expect(decl).toContain('schema?:'); + expect(decl).toContain('className?:'); + // The spec's bag, nested — not the wrapper's own member list. + expect(decl).toContain('properties?:'); + }); +}); + +describe('the site: the module-local mirror is gone', () => { + const found = () => + scanFile(path.join(repoRoot, SITE), specNames).map((f: { name: string }) => f.name); + + it(`rule 1 sees no spec-named declaration in ${SITE} at all`, () => { + // Judged against the REAL name map, so this single reading answers two + // questions at once: the old name is gone, and the new one is not itself a + // spec export. A map built by hand here could answer neither. + expect(found()).toEqual([]); + }); + + it('the scanner can still see the shape it used to flag — the control', () => { + // Same kind as the subject: the exact declaration this slice renamed. + withFixture( + 'spec-symbol-plugin-detail-7265-', + 'relapse.tsx', + [ + `interface ${FORMERLY} {`, + ' schema?: { properties?: { severity?: string } };', + ' className?: string;', + '}', + '', + `export const Renderer = (props: ${FORMERLY}) => props.className ?? '';`, + '', + ].join('\n'), + (file) => + expect( + scanFile(file, specNames).map((f: { name: string; kind: string }) => ({ + name: f.name, + kind: f.kind, + })), + ).toEqual([{ name: FORMERLY, kind: 'interface' }]), + ); + }); + + it(`…and the SAME fixture under \`${RENAMED}\` is clean — the other direction`, () => { + // Both legs of the tripwire through one instrument. Without this one, the + // reading above could be green because the scanner stopped seeing + // interfaces rather than because the name stopped colliding. + withFixture( + 'spec-symbol-plugin-detail-7265-renamed-', + 'renamed.tsx', + [ + `interface ${RENAMED} {`, + ' schema?: { properties?: { severity?: string } };', + ' className?: string;', + '}', + '', + `export const Renderer = (props: ${RENAMED}) => props.className ?? '';`, + '', + ].join('\n'), + (file) => expect(scanFile(file, specNames).map((f: { name: string }) => f.name)).toEqual([]), + ); + }); + + it('…and derive-in-place was never on the table for this shape', () => { + // The route note, asserted rather than narrated. An `interface` is recorded + // as derived only through `extends` (the gate's header says so, and + // `referencesSpec` is called with the heritage clauses for exactly that), so + // importing the spec's type and USING it in the member block leaves the + // collision standing. That is why a TYPE having a third route did not give + // THIS type one. + withFixture( + 'spec-symbol-plugin-detail-7265-derive-', + 'derive.tsx', + [ + `import type { ${FORMERLY} as SpecProps } from '${SUBPATH}';`, + '', + `interface ${FORMERLY} {`, + ' schema?: { properties?: SpecProps };', + ' className?: string;', + '}', + '', + `export const Renderer = (props: ${FORMERLY}) => props.className ?? '';`, + '', + ].join('\n'), + (file) => + expect(scanFile(file, specNames).map((f: { name: string }) => f.name)).toEqual([FORMERLY]), + ); + }); +}); + +describe('the block: shrink-only, and this is the slice where it reaches the floor', () => { + it(`DEBT no longer lists \`${FORMERLY}\``, () => { + expect(ledgerBlock('DEBT')).not.toContain(`"${FORMERLY}"`); + }); + + it('the whole `@object-ui/plugin-detail` group is gone', () => { + expect(ledgerBlock('DEBT')).not.toContain('"@object-ui/plugin-detail"'); + }); + + it('DEBT is EMPTY — 0 is both the ceiling and the floor now', () => { + // Every earlier slice could only state a ceiling, because it did not know + // what the next one would take. This one does. Stated as an equality on + // purpose: a later name appearing here is the SHRINK-ONLY invariant being + // broken, and the block's own note says the single sanctioned reason it may + // grow (rule 1's jurisdiction widening) — which is a deliberate act that + // should have to delete this line, not slip past it. + expect(ledgerNames(ledgerBlock('DEBT'))).toEqual([]); + }); + + it('CLAIM_DEBT did not grow either — this slice removed no rule 2 claim', () => { + // Measured, not assumed: `--claim-ledger` regenerates that block + // byte-identically across this change, because the declaration that moved + // carried no spec-alignment claim. + expect(ledgerNames(ledgerBlock('CLAIM_DEBT')).length).toBeLessThanOrEqual(18); + }); + + it('ALLOW did NOT gain a waiver — the route was RENAME, and that is worth pinning', () => { + // A sibling slice put one of its names in ALLOW instead, so "a name left + // DEBT" does not say which route was taken. This one took none: the + // collision is gone rather than excused. + expect(fs.readFileSync(gateSource, 'utf8')).not.toContain(`"${ALLOW_KEY}"`); + }); +}); + +describe('the empty-ledger path — the end state, not the cue to delete the machinery', () => { + const gateText = () => fs.readFileSync(gateSource, 'utf8'); + + it('`DEBT` still exists as a block, and it parses as empty', () => { + const block = ledgerBlock('DEBT'); + expect(block.startsWith('const DEBT = {')).toBe(true); + expect(ledgerNames(block)).toEqual([]); + }); + + it('`DEBT_ISSUE` is still declared — an empty ledger keeps its anchor', () => { + // Deleting it would silently turn the stale-entry message's tail off (it is + // written behind a `DEBT_ISSUE ?` guard), which is the instruction a + // re-seeded ledger would need most. + expect(gateText()).toMatch(/^const DEBT_ISSUE = \d+;$/m); + }); + + it('the ratchet that reads `DEBT` is still wired — it is the re-seeding guard', () => { + // An empty shrink-only ledger does its work through the OTHER half: rule 1's + // "not in the ledger ⇒ NEW" arm fails a fresh fork by name, and the ratchet + // here fails a name that outlives its collision. Neither may be deleted on + // the grounds that there is currently nothing to iterate. + expect(gateText()).toContain('for (const [pkg, names] of Object.entries(DEBT)) {'); + expect(gateText()).toContain('const declared = new Set(DEBT[pkg] ?? []);'); + }); +}); + +describe('the stale-entry message may not carry a card-closing keyword', () => { + /** The rule-2 ratchet, from its banner comment to the next one. */ + function staleEntryRatchet(): string { + const text = fs.readFileSync(gateSource, 'utf8'); + const start = text.indexOf('// 2. Ratchet — a ledger entry whose symbol is fixed'); + expect(start, "rule 2's ratchet banner is gone").toBeGreaterThan(-1); + const end = text.indexOf('// 3. Ratchet —', start); + expect(end, "rule 3's ratchet banner is gone").toBeGreaterThan(start); + return text.slice(start, end); + } + + /** + * GitHub's closing-keyword grammar, as the mitigation in root `AGENTS.md` + * states it: one of the keywords immediately before an issue reference, in any + * tense, with no sentence parsing of any kind. `#${…}` is included because + * that is how this message spells the number it interpolates. + */ + const CLOSING_TRIGGER = + /\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+(?:[A-Za-z0-9-]+\/[A-Za-z0-9-]+|objectui|objectstack)?#(?:\$\{|\d)/i; + + it('the regex can fire — the control, on the wording this card replaced', () => { + // ⛔ Without this leg the assertion below is unfalsifiable: a regex that + // matches nothing renders exactly like a message that carries nothing. + // + // Two legs, and neither writes a live trigger into the tree. The first is + // the SOURCE form the tail carried until this slice, byte for byte — a + // template interpolation, which can never resolve to an issue reference in a + // rendered body. The second proves the regex also catches the RENDERED form, + // spelled with issue number ZERO because GitHub resolves no card for it: + // writing a real number here would plant in this file the exact shape the + // assertion below exists to keep out of it. + expect(CLOSING_TRIGGER.test('(and close #${DEBT_ISSUE} once the ledger is empty)')).toBe(true); + expect( + CLOSING_TRIGGER.test('so the names cannot be re-forked silently (and close #0 once the ledger is empty).'), + ).toBe(true); + }); + + it('…and the live ratchet carries no trigger', () => { + expect( + CLOSING_TRIGGER.test(staleEntryRatchet()), + "rule 2's stale-entry message has a GitHub closing keyword in front of an issue " + + 'reference again. Taking the INTERMEDIATE reading and quoting it is standing ' + + 'practice on objectui#7265, so this text reaches pull-request bodies by design ' + + 'and a keyword here ends the anchor card on merge. The reason is written at the ' + + 'ratchet itself.', + ).toBe(false); + }); + + it('…while still telling the reader what to do with an emptied ledger', () => { + // The positive half, so "no trigger" cannot be satisfied by deleting the + // instruction outright. + expect(staleEntryRatchet()).toContain('once the ledger is empty'); + expect(staleEntryRatchet()).toContain('objectui#${DEBT_ISSUE}'); + }); + + it('the CLAIM_DEBT twin is deliberately left alone', () => { + // Scope, asserted. Its anchor is a CLOSED issue, so the keyword there fires + // nothing, and the note beside `DEBT_ISSUE` records the exclusion as + // deliberate. A later hand "finishing the job" makes that note false. + const text = fs.readFileSync(gateSource, 'utf8'); + expect(text).toContain('(and close #${CLAIM_DEBT_ISSUE} once the ledger is empty)'); + }); +}); + +describe("the narrowing fixture's examples describe a measurement, not today's tree", () => { + /** The comment attached to the `rendersJsx` near-neighbour fixture. */ + function narrowingNote(): string { + const text = fs.readFileSync(gateTestSource, 'utf8'); + const end = text.indexOf("it('…but a module-local FUNCTION that renders nothing is still a fork'"); + expect(end, 'the near-neighbour case is gone').toBeGreaterThan(-1); + const start = text.indexOf('withFixture', end); + expect(start, 'the fixture is gone').toBeGreaterThan(end); + return text.slice(end, start); + } + + /** The claim shape the repair removed — kept as a literal so the probe below has a control. */ + const PRESENT_TENSE_CLAIM = /\bDEBT entries today\b|\bare the live\b/i; + + it('the probe can fire — the control, on the sentence this card repaired', () => { + // Same discipline as the trigger regex above: a pattern that matches nothing + // renders exactly like a comment that claims nothing. + expect( + PRESENT_TENSE_CLAIM.test( + '`isContextToken` and `normalizeFilterOperator` are the live instances: ' + + 'non-exported functions under spec export names, both real mirrors, both DEBT ' + + 'entries today.', + ), + ).toBe(true); + }); + + it('carries no present-tense ledger-membership claim', () => { + // The repair objectui#7265's last slice owed. Both names it cites were + // burned down by this very card — one BOUND, one RENAMED — so a sentence + // calling them "the live instances … both DEBT entries today" sent the next + // reader looking for declarations that no longer exist. Same family as the + // stale COUNT the previous slice deleted: a fact derived once and re-derived + // never (root `AGENTS.md` #9). + const note = narrowingNote(); + expect( + PRESENT_TENSE_CLAIM.test(note), + 'the near-neighbour comment claims present-tense ledger membership again. Anchor the ' + + 'examples to the commit that measured them, the way the `rendersJsx` docblock in ' + + 'the gate itself does, or re-derive them from the tree.', + ).toBe(false); + }); + + it('…and anchors its examples to the commit that measured them', () => { + // The positive half: "no present-tense claim" must not be satisfiable by + // deleting the reasoning the fixture exists to carry. + const note = narrowingNote(); + expect(note).toContain('objectui#6291'); + expect(note).toContain('rendersJsx'); + }); +}); diff --git a/scripts/check-spec-symbol-derivation.mjs b/scripts/check-spec-symbol-derivation.mjs index efa43d3f38..eb03a2f45b 100644 --- a/scripts/check-spec-symbol-derivation.mjs +++ b/scripts/check-spec-symbol-derivation.mjs @@ -860,9 +860,12 @@ const ALLOW = { // Re-anchored at objectui#6291. It was `4115` (objectstack#4115) while the block // was EMPTY — burned down in objectui#3162, and objectstack#4115 itself closed by // objectstack#6883. A ledger whose anchor is CLOSED makes the stale-entry message -// below ("…and close #N once the ledger is empty") a dead instruction, and #6291 -// could not serve either, being the card its own PR closes. objectui#7265 is the -// open burn-down card for the population seeded here. +// below — the one that tells you the anchor can be ended once the ledger is empty +// — a dead instruction, and #6291 could not serve either, being the card its own +// PR closes. objectui#7265 is the open burn-down card for the population seeded +// here. (That message used to spell its own instruction with a GitHub closing +// keyword in front of the number; the reason it no longer does is written at the +// ratchet that emits it, not restated here.) // ⚠️ `CLAIM_DEBT_ISSUE` a few screens down has the same defect — objectui#4592 is // closed while its block is still live — and is deliberately NOT changed here, // because rule 2's ledger is not what objectui#6291 widened. (Still deliberate at @@ -993,10 +996,36 @@ const DEBT_ISSUE = 7265; // RENAMES table, not duplicated), and the site, the block and the measured // behaviour in // scripts/__tests__/spec-symbol-ledger-data-objectstack-7265.test.ts. +// +// Then the `@object-ui/plugin-detail` slice at objectui#7265, the LAST group, and +// the one where the route was settled by the tree rather than by the shape. The +// site was a non-exported `interface RecordAlertProps` used at exactly one place, +// the `React.FC<…>` type argument of `RecordAlertRenderer`. Being a TYPE, it had +// the derive-in-place route a function does not -- and reading it took that route +// off the table rather than onto it. At the RESOLVED pin the spec's +// `RecordAlertProps` is the AUTHORED property bag of the `record:alert` block, +// while the local one is the React props the renderer is called with: a `schema` +// node, a `className`, an open tail, with the spec's bag NESTED inside it under +// `schema.properties` and mirrored FLAT beside it for legacy nodes. Not the same +// concept, so BIND was refused; and an `interface` derives only through +// `extends`, which a wrapper around a type cannot use on the type it wraps. +// +// What decided it is that the collision was a SPELLING SLIP against a convention +// this repo already keeps. `@objectstack/spec/ui` owns a `RecordProps` +// for every block that directory renders, and every sibling renderer beside this +// one already spells its own props type `RecordRendererProps` -- which is +// exactly why none of them was ever in this ledger and this one was. RENAMED to +// `RecordAlertRendererProps`, therefore: not a dialect minted for the occasion +// but the name the file should have carried, and no published face moves with it +// (the declaration was never exported, and `index.tsx` imports the COMPONENT). +// Pinned in both directions -- the spec still owns the plain name, the spec does +// not own the new one, and the sibling convention is re-derived from the +// directory rather than restated -- in this package's own spec-symbol file, +// packages/plugin-detail/src/__tests__/spec-symbol-batch7.test.ts (appended to, +// not duplicated), with the site, the block and the empty-ledger path in +// scripts/__tests__/spec-symbol-ledger-plugin-detail-7265.test.ts. const DEBT = { - "@object-ui/plugin-detail": [ - "RecordAlertProps", - ], + }; // Files under these paths are not objectui's own authored surface. @@ -1161,7 +1190,12 @@ const CLAIM_DEBT = { // Types AND values: the drifted symbols in the table above are mostly types, and // a runtime `import()` only sees values. The compiler's own view of each // subpath's `.d.ts` is the only source that covers both. -function specExportNames() { +// +// Exported for the pins in `scripts/__tests__/`, for the same reason `scanFile` +// is: a ledger test that builds its own name map is asserting against a COPY of +// the spec's export set, which is the exact failure this whole guard exists to +// catch, one level up. A pin that reads THIS function reads what the gate reads. +export function specExportNames() { const require = createRequire(import.meta.url); let pkgPath; try { @@ -2074,6 +2108,27 @@ for (const [pkg, found] of byPackage) { // 2. Ratchet — a ledger entry whose symbol is fixed (or gone) must be deleted. // Left in, it reserves the name: the next fork under it would land silently. +// +// ⚠️ The tail's WORDING is constrained, and not by taste (objectui#7265). This +// message names the ledger's anchor card, and taking the INTERMEDIATE reading — +// the site burned down, the block not yet regenerated — is standing practice on +// that card, because it is the only proof the edit reached the symbol rather than +// the block being rewritten around it. So this text is pasted into pull-request +// bodies and commit messages BY DESIGN. GitHub's closing-keyword parser reads one +// of its keywords (the close / fix / resolve families, in every tense) sitting +// immediately before an issue reference, and it does not parse sentences: the +// tail this line replaced put such a keyword directly in front of the anchor's +// own number, so every slice that quoted its own reading carried a trigger that +// would have ended the card on merge — silently, from a body whose author was +// being careful. ⛔ Never write one of those keywords in front of the reference +// here, and keep the `objectui#` prefix rather than a bare `#`. Pinned by +// scripts/__tests__/spec-symbol-ledger-plugin-detail-7265.test.ts, which reads +// this module's emitted text rather than trusting the comment. +// +// ⛔ The `CLAIM_DEBT` twin further down is deliberately NOT given the same +// treatment. Its anchor is a CLOSED issue — the note beside `DEBT_ISSUE` above +// records that exclusion and why it stands — so the keyword there triggers +// nothing, and changing it would quietly make that note false. for (const [pkg, names] of Object.entries(DEBT)) { const live = new Set((byPackage.get(pkg) ?? []).map((v) => v.name)); const stale = names.filter((n) => !live.has(n)); @@ -2083,7 +2138,7 @@ for (const [pkg, names] of Object.entries(DEBT)) { ` — \`${stale.join("`, `")}\`.\n` + ` Delete them from scripts/check-spec-symbol-derivation.mjs (\`--ledger\` regenerates the\n` + ` block) so the names cannot be re-forked silently` + - `${DEBT_ISSUE ? ` (and close #${DEBT_ISSUE} once the ledger is empty)` : ""}.` + `${DEBT_ISSUE ? `, and objectui#${DEBT_ISSUE} can be ended once the ledger is empty` : ""}.` ); }