From afd6dbd1b0e2f7e64f4230720b62bb9d911c8cb9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 00:35:48 +0000 Subject: [PATCH 1/2] fix(spec): registration remedies name the entry file and gen:migration-registry, never the generated table Gate (b), the def-removal gate, the manifest disappearance check and the (b2)/(b3) mirrors printed lines to paste into, or delete from, src/migrations/registry.ts, whose tables are generated from src/migrations/entries/. Each remedy now names the entry file (from the generator's own shardNameFor, made importable behind an entry guard) and the generator command. The guidance-slot docblocks say what the slot is for now: keys the shape does not declare. Claude-Session: https://claude.ai/code/session_017VaLJnYwhPsanVCe9dMCJU Co-authored-by: Claude --- .../build-migration-registry-entry.test.ts | 74 +++++++++ .../spec/scripts/build-migration-registry.ts | 117 ++++++++------ .../scripts/build-schemas-check-mode.test.ts | 139 +++++++++++++++-- packages/spec/scripts/build-schemas.ts | 147 ++++++++++++++---- packages/spec/src/ai/tool.zod.ts | 7 +- packages/spec/src/shared/strict-object.ts | 11 +- 6 files changed, 398 insertions(+), 97 deletions(-) create mode 100644 packages/spec/scripts/build-migration-registry-entry.test.ts diff --git a/packages/spec/scripts/build-migration-registry-entry.test.ts b/packages/spec/scripts/build-migration-registry-entry.test.ts new file mode 100644 index 0000000000..0882cf49c2 --- /dev/null +++ b/packages/spec/scripts/build-migration-registry-entry.test.ts @@ -0,0 +1,74 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// `build-migration-registry.ts` has TWO kinds of caller now, and its entry guard +// is the only thing telling them apart: +// +// - pnpm RUNS it (`gen:migration-registry`, `check:migration-registry`), and +// write mode rewrites `src/migrations/registry.ts`; +// - `build-schemas.ts` IMPORTS it, for `shardNameFor` — so the entry file each +// registration remedy names is the one name the generator accepts, read from +// the rule `entries/README.md` documents instead of restated beside it. +// +// Both directions are pinned, because each one fails silently on its own. A +// guard that answered "imported" for a real run turns the CI gate into exit 0 +// with no output; one that answered "run" for an import executes the generator +// inside every build of the schemas. Each probe is a real `tsx` process, the +// runner both callers use, so neither half is a model of the other. + +import { describe, it, expect, afterAll } from 'vitest'; +import { spawnSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +const PKG = path.resolve(HERE, '..'); +const TSX = path.join(PKG, 'node_modules', '.bin', 'tsx'); +const GENERATOR = path.join(HERE, 'build-migration-registry.ts'); +const SPAWN_TIMEOUT_MS = 60_000; + +const scratch = fs.mkdtempSync(path.join(os.tmpdir(), 'build-migration-registry-entry-')); +afterAll(() => fs.rmSync(scratch, { recursive: true, force: true })); + +function tsx(args: string[]): { status: number; stdout: string; stderr: string } { + const r = spawnSync(TSX, args, { + cwd: PKG, + encoding: 'utf8', + timeout: SPAWN_TIMEOUT_MS, + stdio: ['ignore', 'pipe', 'pipe'], + }); + return { status: r.status ?? -1, stdout: r.stdout ?? '', stderr: r.stderr ?? '' }; +} + +describe('build-migration-registry.ts — run, it speaks; import, it runs nothing', () => { + it('a direct run still reaches the generator (its self-test answers)', { timeout: SPAWN_TIMEOUT_MS }, () => { + const r = tsx([GENERATOR, '--self-test']); + expect(r.status, r.stderr).toBe(0); + expect(r.stdout).toContain('build-migration-registry --self-test: ok'); + }); + + it('a run through a symlink still reaches it — node keeps argv[1] as typed', { timeout: SPAWN_TIMEOUT_MS }, () => { + const link = path.join(scratch, 'linked-generator.ts'); + fs.symlinkSync(GENERATOR, link); + const r = tsx([link, '--self-test']); + expect(r.status, r.stderr).toBe(0); + expect(r.stdout).toContain('build-migration-registry --self-test: ok'); + }); + + it('an import runs nothing: the importer prints exactly what it asked for', { timeout: SPAWN_TIMEOUT_MS }, () => { + // The README's own example, so a reader can check the expectation by eye. + const probe = path.join(scratch, 'import-probe.ts'); + fs.writeFileSync( + probe, + `import { shardNameFor } from ${JSON.stringify(GENERATOR)};\n` + + `console.log(shardNameFor(17, 'data/AggregationNode:distinct'));\n`, + ); + const r = tsx([probe]); + expect(r.status, r.stderr).toBe(0); + // Write mode would have printed `✓ wrote src/migrations/registry.ts (…)`, and + // check mode `✓ … is current` — any line beyond the probe's own is the defect. + expect(r.stdout).toBe('17.data__AggregationNode__distinct.ts\n'); + expect(r.stderr).toBe(''); + }); +}); diff --git a/packages/spec/scripts/build-migration-registry.ts b/packages/spec/scripts/build-migration-registry.ts index 6356d8d004..685a093859 100644 --- a/packages/spec/scripts/build-migration-registry.ts +++ b/packages/spec/scripts/build-migration-registry.ts @@ -90,7 +90,7 @@ * pnpm --filter @objectstack/spec check:migration-registry # --self-test && --check */ -import { readFileSync, readdirSync, writeFileSync } from 'node:fs'; +import { readFileSync, readdirSync, realpathSync, writeFileSync } from 'node:fs'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -144,6 +144,10 @@ export interface Entry { * FUNCTION (one id → one name), never reversible — the id is written inside the * file, and two ids that collided on a name would collide as an add/add * conflict, which is the behaviour duplicates should get anyway. + * + * `build-schemas.ts` names the file each of its registration remedies asks for + * through this same function, so a gate never prints a name this generator + * would refuse. */ export function shardNameFor(major: number, id: string): string { return `${major}.${id.replaceAll('/', '__').replaceAll(':', '__')}.ts`; @@ -423,58 +427,79 @@ function selfTest(): string[] { } // --------------------------------------------------------------------------- -// main +// main — only when node RAN this file, never when something imported it // --------------------------------------------------------------------------- +// +// `build-schemas.ts` imports `shardNameFor`, `KINDS` and `ENTRIES_DIR`, so every +// remedy it prints names the entry file THIS rule accepts — the one naming rule +// `entries/README.md` documents, read rather than restated beside it. An import +// must therefore run nothing: write mode rewrites `registry.ts`. Spelled as +// `check-generated.ts` spells it, realpath on BOTH sides — node resolves +// symlinks for the module graph but leaves `argv[1]` as typed, and a guard that +// compared the two raw would turn `check:migration-registry` into exit 0 with +// no output. `build-migration-registry-entry.test.ts` pins both directions. + +function main(argv: readonly string[]): void { + if (argv.includes('--self-test')) { + const failures = selfTest(); + if (failures.length) { + console.error('build-migration-registry --self-test FAILED:\n' + failures.map((f) => ` - ${f}`).join('\n')); + process.exit(1); + } + console.log('build-migration-registry --self-test: ok'); + if (argv.length === 1) process.exit(0); + } -const argv = process.argv.slice(2); + const check = argv.includes('--check'); + const registryPath = join(pkgRoot, REGISTRY_PATH); + const source = readFileSync(registryPath, 'utf8'); -if (argv.includes('--self-test')) { - const failures = selfTest(); - if (failures.length) { - console.error('build-migration-registry --self-test FAILED:\n' + failures.map((f) => ` - ${f}`).join('\n')); - process.exit(1); + const byKind = {} as Record; + const errors: string[] = []; + for (const { kind } of KINDS) { + const loaded = loadEntries(kind); + byKind[kind] = loaded.entries; + errors.push(...loaded.errors); } - console.log('build-migration-registry --self-test: ok'); - if (argv.length === 1) process.exit(0); -} -const check = argv.includes('--check'); -const registryPath = join(pkgRoot, REGISTRY_PATH); -const source = readFileSync(registryPath, 'utf8'); + const { text, errors: spliceErrors } = renderRegistry(source, byKind); + errors.push(...spliceErrors); -const byKind = {} as Record; -const errors: string[] = []; -for (const { kind } of KINDS) { - const loaded = loadEntries(kind); - byKind[kind] = loaded.entries; - errors.push(...loaded.errors); -} - -const { text, errors: spliceErrors } = renderRegistry(source, byKind); -errors.push(...spliceErrors); + if (errors.length) { + console.error('build-migration-registry: the entry directories are not readable as a registry.\n'); + for (const e of errors) console.error(` ✗ ${e}`); + process.exit(1); + } -if (errors.length) { - console.error('build-migration-registry: the entry directories are not readable as a registry.\n'); - for (const e of errors) console.error(` ✗ ${e}`); - process.exit(1); + const counts = KINDS.map(({ kind }) => `${byKind[kind].length} ${kind}`).join(', '); + + if (check) { + if (text !== source) { + console.error( + `✗ ${REGISTRY_PATH} is stale — its generated regions do not match ${ENTRIES_DIR}/.\n\n` + + ' Run: pnpm --filter @objectstack/spec gen:migration-registry\n\n' + + ' If you reached this after resolving a merge conflict INSIDE a marked region: do not\n' + + ' hand-merge it. The entry files are the source and git merged them as a set; the only\n' + + ' correct resolution is to regenerate. This gate exists because a resolution that drops\n' + + " one side's entry produces no other error anywhere (#6957).", + ); + process.exit(1); + } + console.log(`✓ ${REGISTRY_PATH} is current (${counts})`); + } else { + if (text !== source) writeFileSync(registryPath, text); + console.log(`✓ wrote ${REGISTRY_PATH} (${counts})`); + } } -const counts = KINDS.map(({ kind }) => `${byKind[kind].length} ${kind}`).join(', '); - -if (check) { - if (text !== source) { - console.error( - `✗ ${REGISTRY_PATH} is stale — its generated regions do not match ${ENTRIES_DIR}/.\n\n` + - ' Run: pnpm --filter @objectstack/spec gen:migration-registry\n\n' + - ' If you reached this after resolving a merge conflict INSIDE a marked region: do not\n' + - ' hand-merge it. The entry files are the source and git merged them as a set; the only\n' + - ' correct resolution is to regenerate. This gate exists because a resolution that drops\n' + - " one side's entry produces no other error anywhere (#6957).", - ); - process.exit(1); +const invokedDirectly = (() => { + const entry = process.argv[1]; + if (!entry) return false; + try { + return realpathSync(entry) === realpathSync(fileURLToPath(import.meta.url)); + } catch { + return false; } - console.log(`✓ ${REGISTRY_PATH} is current (${counts})`); -} else { - if (text !== source) writeFileSync(registryPath, text); - console.log(`✓ wrote ${REGISTRY_PATH} (${counts})`); -} +})(); + +if (invokedDirectly) main(process.argv.slice(2)); diff --git a/packages/spec/scripts/build-schemas-check-mode.test.ts b/packages/spec/scripts/build-schemas-check-mode.test.ts index 93a41ecae5..e559ec85b7 100644 --- a/packages/spec/scripts/build-schemas-check-mode.test.ts +++ b/packages/spec/scripts/build-schemas-check-mode.test.ts @@ -64,6 +64,9 @@ import { RETIRED_DEFS_BY_MAJOR, RETIRED_KEYS_BY_MAJOR, } from '../src/migrations/registry'; +// The generator's own naming rule: the file name every registration remedy must +// print, so an author who follows a gate writes a file the generator accepts. +import { shardNameFor } from './build-migration-registry'; // Read ONLY to keep the #17356 fixture honest about which SET its root lives in // — never to assert gate behaviour, which is read off the spawned run's output. import { @@ -632,6 +635,12 @@ describe('build-schemas.ts --check — a check reports, it does not write (#4711 expect(status).toBe(1); expect(output).toMatch(/1 previously published schema\(s\) disappeared from this build/); expect(output).toContain(`- json-schema/${PHANTOM_KEY}.json`); + // The deliberate-removal route registers through the generated table's + // entry FILE and the generator — never by typing into the table. + expect(output).toContain(entryFile('retired-defs', PHANTOM_KEY)); + expect(output).toContain(`export const entry = '${PHANTOM_KEY}';`); + expect(output).toContain(REGEN_COMMAND); + expect(output).not.toContain('migrations/registry.ts'); expect(readManifest()).toBe(withPhantom); }, ); @@ -989,6 +998,16 @@ const CURRENT_MAJOR = Number.parseInt( 10, ); +/** + * The repo-relative entry file a registration remedy names for `id`: the two + * RETIRED_*_BY_MAJOR tables are GENERATED from these files, so a remedy that + * routes an author into `src/migrations/registry.ts` instead is the defect. + */ +const entryFile = (dir: 'retired-keys' | 'retired-defs', id: string, major = CURRENT_MAJOR): string => + `packages/spec/src/migrations/entries/${dir}/${shardNameFor(major, id)}`; +/** The one route into those tables — the generator, never a hand edit. */ +const REGEN_COMMAND = 'pnpm --filter @objectstack/spec gen:migration-registry'; + /** Reachable def (BUILTIN root `object`), never tombstoned — the #4643 shape. */ const DELETED_LIVE = 'data/Object:zzNeverRetired4650'; /** Reachable def, tombstoned at base but never registered in the ADR-0087 registries. */ @@ -3503,13 +3522,23 @@ describe('build-schemas.ts — check (b) matches the exact retired key, not its expect(output).toContain(`2 ${CHECK_B}`); expect(output).toContain(` - ${LEAF_COLLIDER_A}`); expect(output).toContain(` - ${LEAF_COLLIDER_B}`); - // The prescription IS the contract: the exact line to paste, and where. - expect(output).toContain(` '${LEAF_COLLIDER_A}',`); - expect(output).toContain(` '${LEAF_COLLIDER_B}',`); - expect(output).toContain('RETIRED_KEYS_BY_MAJOR'); - expect(output).toContain(`under \`${CURRENT_MAJOR}: [ … ]\``); + // The prescription IS the contract: the entry FILE to add per key, named + // by the generator's own rule, and the generator command. RETIRED_KEYS_BY_MAJOR + // is generated from those files, so a remedy that pastes lines into + // src/migrations/registry.ts sends the author to an edit the next run + // reverts and check:migration-registry fails. + const remedy = output.slice(output.indexOf(`2 ${CHECK_B}`)); + for (const key of [LEAF_COLLIDER_A, LEAF_COLLIDER_B]) { + expect(remedy).toContain(entryFile('retired-keys', key)); + expect(remedy).toContain(`export const entry = '${key}';`); + expect(remedy).not.toContain(` '${key}',`); + } + expect(remedy).toContain(REGEN_COMMAND); + expect(remedy).toContain('RETIRED_KEYS_BY_MAJOR'); + expect(remedy).not.toContain('migrations/registry.ts'); + expect(remedy).not.toContain('copy these lines'); // The D2 conversion is still asked for — the table did not replace it. - expect(output).toContain('src/conversions/registry.ts'); + expect(remedy).toContain('src/conversions/registry.ts'); }, ); @@ -3553,6 +3582,64 @@ describe('build-schemas.ts — check (b) matches the exact retired key, not its }, ); + it( + 'the printed route registers: write exactly the files check (b) names, run the generator, and (b) is silent', + { timeout: SPAWN_TIMEOUT_MS * 3 }, + () => { + // The case above registers by substituting the table. This one takes the + // route the remedy PRINTS, literally: every file/declaration pair is read + // off the red run's own output, written into the sandbox's entry + // directory, and concatenated by the sandbox's own copy of the generator — + // which refuses a file whose name does not match its id. So a remedy + // naming a file the generator would reject, or a route that does not + // register, goes red here rather than in an author's lap. + fs.writeFileSync(boxRegistry, pristineRegistry); // the real generated table, markers intact + untombstone(LEAF_COLLIDER_A, LEAF_COLLIDER_B); + const written: string[] = []; + try { + const red = runBox(['--check']); + expect(red.status).toBe(1); + expect(red.output).toContain(`2 ${CHECK_B}`); + + const pairs = [ + ...red.output.matchAll( + /^ +(packages\/spec\/src\/migrations\/entries\/retired-keys\/\S+\.ts)\n +(export const entry = '[^']+';)$/gm, + ), + ]; + expect(pairs.map((m) => m[2]).sort()).toEqual( + [LEAF_COLLIDER_A, LEAF_COLLIDER_B].map((k) => `export const entry = '${k}';`).sort(), + ); + for (const [, rel, declaration] of pairs) { + const file = path.join(box, rel!.slice('packages/spec/'.length)); + expect(fs.existsSync(file), `${rel} already exists in the sandbox`).toBe(false); + fs.writeFileSync(file, `// Fixture: registered by following check (b)'s remedy.\n${declaration}\n`); + written.push(file); + } + + const gen = spawnSync(TSX, [path.join(box, 'scripts', 'build-migration-registry.ts')], { + cwd: box, + encoding: 'utf8', + timeout: SPAWN_TIMEOUT_MS, + stdio: ['ignore', 'pipe', 'pipe'], + env: HERMETIC_ENV, + }); + expect(gen.status, `${gen.stdout ?? ''}${gen.stderr ?? ''}`).toBe(0); + expect(gen.stdout).toContain('✓ wrote src/migrations/registry.ts'); + const regenerated = fs.readFileSync(boxRegistry, 'utf8'); + for (const k of [LEAF_COLLIDER_A, LEAF_COLLIDER_B]) expect(regenerated).toContain(`'${k}',`); + + const green = runBox(['--check']); + expect(green.output).not.toContain(CHECK_B); + expect(green.output).not.toContain('still LIVE'); + // Same residue as the case above: the baseline still owes `gen:schema`. + expect(green.output).toContain('authorable-surface/ is out of date (2 key(s) not recorded)'); + } finally { + for (const file of written) fs.rmSync(file, { force: true }); + fs.writeFileSync(boxRegistry, pristineRegistry); + } + }, + ); + it( 'an entry naming a key that is still LIVE fails: a registration nothing consumed (b2)', { timeout: SPAWN_TIMEOUT_MS }, @@ -3568,8 +3655,14 @@ describe('build-schemas.ts — check (b) matches the exact retired key, not its expect(status).toBe(1); expect(output).toContain('RETIRED_KEYS_BY_MAJOR entr(ies) name a key that is still LIVE'); - expect(output).toContain(` - ${STILL_LIVE_KEY} (registered at major ${CURRENT_MAJOR})`); + expect(output).toContain( + ` - ${STILL_LIVE_KEY} (registered at major ${CURRENT_MAJOR})\n` + + ` ${entryFile('retired-keys', STILL_LIVE_KEY)}\n`, + ); expect(output).toContain('retiredKey('); + // Undoing a registration is an entry-file deletion plus the generator too. + expect(output).toContain(REGEN_COMMAND); + expect(output).not.toContain('migrations/registry.ts'); expect(output).not.toContain(CHECK_B); }, ); @@ -3831,11 +3924,19 @@ describe('build-schemas.ts — a deleted manifest key must prove itself (#4725)' expect(output).toContain(`2 ${NO_REGISTERED_REMOVAL}`); expect(output).toContain(` - json-schema/${REMOVED_DEF}.json`); expect(output).toContain(` - json-schema/${REMOVED_DEF_2}.json`); - // The prescription IS the contract: the exact lines to paste, and where. - expect(output).toContain(` '${REMOVED_DEF}',`); - expect(output).toContain(` '${REMOVED_DEF_2}',`); - expect(output).toContain('RETIRED_DEFS_BY_MAJOR'); - expect(output).toContain(`under \`${CURRENT_MAJOR}: [ … ]\``); + // The prescription IS the contract: the entry FILE to add per def, named + // by the generator's own rule, and the generator command — never lines to + // paste into the generated table in src/migrations/registry.ts. + const remedy = output.slice(output.indexOf(`2 ${NO_REGISTERED_REMOVAL}`)); + for (const def of [REMOVED_DEF, REMOVED_DEF_2]) { + expect(remedy).toContain(entryFile('retired-defs', def)); + expect(remedy).toContain(`export const entry = '${def}';`); + expect(remedy).not.toContain(` '${def}',`); + } + expect(remedy).toContain(REGEN_COMMAND); + expect(remedy).toContain('RETIRED_DEFS_BY_MAJOR'); + expect(remedy).not.toContain('migrations/registry.ts'); + expect(remedy).not.toContain('copy these lines'); // …and the two other routes a reader might actually need are named. expect(output).toContain('RENAMED_DEFS'); expect(output).toContain('src/conversions/registry.ts'); @@ -3914,7 +4015,12 @@ describe('build-schemas.ts — a deleted manifest key must prove itself (#4725)' expect(status).toBe(1); expect(output).toContain('RETIRED_DEFS_BY_MAJOR entr(ies) name a schema this build still publishes'); - expect(output).toContain(` - ${STILL_PUBLISHED_DEF} (registered at major ${CURRENT_MAJOR})`); + expect(output).toContain( + ` - ${STILL_PUBLISHED_DEF} (registered at major ${CURRENT_MAJOR})\n` + + ` ${entryFile('retired-defs', STILL_PUBLISHED_DEF)}\n`, + ); + expect(output).toContain(REGEN_COMMAND); + expect(output).not.toContain('migrations/registry.ts'); expect(output).not.toContain(NO_REGISTERED_REMOVAL); }, ); @@ -4420,7 +4526,12 @@ describe('build-schemas.ts — a nested retirement row is judged, not ignored (# expect(status).toBe(1); expect(output).toContain(`1 ${CHECK_B3}`); - expect(output).toContain(` - ${NESTED_TYPO_KEY} (registered at major ${CURRENT_MAJOR})`); + expect(output).toContain( + ` - ${NESTED_TYPO_KEY} (registered at major ${CURRENT_MAJOR})\n` + + ` ${entryFile('retired-keys', NESTED_TYPO_KEY)}\n`, + ); + expect(output).toContain(REGEN_COMMAND); + expect(output).not.toContain('migrations/registry.ts'); // Judged as absent, not as live: the two verdicts have different remedies. expect(output).not.toContain(CHECK_B2); // The refusal carries the remedy, including the one legitimate shape it diff --git a/packages/spec/scripts/build-schemas.ts b/packages/spec/scripts/build-schemas.ts index bc2fee7f43..1fbb6bbcaf 100644 --- a/packages/spec/scripts/build-schemas.ts +++ b/packages/spec/scripts/build-schemas.ts @@ -114,6 +114,10 @@ import { type NestedAuthorableKeyState, } from './lib/nested-authorable-keys'; import { RETIRED_DEFS_BY_MAJOR, RETIRED_KEYS_BY_MAJOR } from '../src/migrations/registry'; +// Where a registration is WRITTEN, as distinct from where it is read above: the +// two tables are generated from one file per entry, and the generator's own +// naming rule is what every remedy below prints (see `entryFileOf`). +import { ENTRIES_DIR, KINDS, shardNameFor, type Kind } from './build-migration-registry'; import { getMetadataTypeSchema, listMetadataTypeSchemaTypes, @@ -759,6 +763,63 @@ try { const generatedKeys = new Set(generatedSchemas.keys()); +/** The protocol major this build is: the major any retirement it makes belongs to. */ +const CURRENT_MAJOR = Number.parseInt(SPEC_VERSION, 10); + +// ─── Where a registration is WRITTEN ────────────────────────────────── +// RETIRED_KEYS_BY_MAJOR, RETIRED_DEFS_BY_MAJOR and every step's `semantic` +// list are GENERATED into src/migrations/registry.ts from one file per entry +// under src/migrations/entries/. So every remedy in this file that asks for a +// registration — or for one to be undone — names the entry FILE and the +// generator command, never the table: an entry typed between the generated +// markers is reverted by the next `gen:migration-registry` run and failed by +// `check:migration-registry` before that. The file name comes from the +// generator's own `shardNameFor`, the one naming rule entries/README.md +// documents, so a gate never prints a name the generator would refuse. + +const MIGRATION_ENTRIES_README = `packages/spec/${ENTRIES_DIR}/README.md`; +const MIGRATION_REGISTRY_COMMAND = 'pnpm --filter @objectstack/spec gen:migration-registry'; + +/** The repo-relative directory one kind of entry file lives in, with its trailing slash. */ +function entryDirOf(kind: Kind): string { + const { dir } = KINDS.find((k) => k.kind === kind)!; + return `packages/spec/${ENTRIES_DIR}/${dir}/`; +} + +/** The repo-relative file that registers `id` under `major`, named as the generator names it. */ +function entryFileOf(kind: Kind, major: number, id: string): string { + return entryDirOf(kind) + shardNameFor(major, id); +} + +/** + * The entry files to ADD for ids this build retires: each file under this + * build's major, and the one declaration it carries. + */ +function renderNewEntryFiles(kind: 'retired-key' | 'retired-def', ids: readonly string[]): string { + return ids + .map((id) => ` ${entryFileOf(kind, CURRENT_MAJOR, id)}\n export const entry = '${id}';\n`) + .join(''); +} + +/** + * The entry files that register `key` in RETIRED_KEYS_BY_MAJOR — named from each + * entry's OWN id and major, so a key carried through a declared def rename + * (`carryAuthorableKey`) still names the file that is actually on disk, and a + * key registered under two majors names both. + */ +function retiredKeyEntryFiles(key: string): string[] { + return Object.entries(RETIRED_KEYS_BY_MAJOR).flatMap(([major, ids]) => + ids.filter((id) => carryAuthorableKey(id) === key).map((id) => entryFileOf('retired-key', Number(major), id)), + ); +} + +/** The entry files that register `def` in RETIRED_DEFS_BY_MAJOR (no rename carry, as there). */ +function retiredDefEntryFiles(def: string): string[] { + return Object.entries(RETIRED_DEFS_BY_MAJOR).flatMap(([major, ids]) => + ids.filter((id) => id === def).map((id) => entryFileOf('retired-def', Number(major), id)), + ); +} + // ─── Declared def renames must describe THIS build ──────────────────── // Both ratchets below consult RENAMED_DEFS, so an entry that no longer matches // reality (target never emitted, or source still emitted alongside it) would @@ -786,10 +847,14 @@ if (missing.length > 0) { ` Zod change made it unrepresentable (e.g. an added .transform in "output" AND "input"\n` + ` io modes) or an export was renamed/removed. Fix the schema, or — if the removal is\n` + ` deliberate — delete the key(s) from packages/spec/${SCHEMA_MANIFEST_DIR_NAME}/.json\n` + - ` in the same PR AND declare each one in RETIRED_DEFS_BY_MAJOR (src/migrations/registry.ts),\n` + - ` which the manifest deletion gate below requires (#4725). Deleting the line alone\n` + - ` used to be the whole procedure, and nothing checked it. Silently unpublishing a\n` + - ` schema deletes its reference docs on the next gen:docs run (see #2978).`, + ` in the same PR AND register each removal in RETIRED_DEFS_BY_MAJOR, which the manifest\n` + + ` deletion gate below requires (#4725). That table is generated: add one entry file per\n` + + ` def (named per ${MIGRATION_ENTRIES_README}; these names are the generator's own) —\n\n` + + renderNewEntryFiles('retired-def', missing) + + `\n — then run \`${MIGRATION_REGISTRY_COMMAND}\`. ⛔ Never type an entry into the\n` + + ` generated table by hand. Deleting the line alone used to be the whole procedure, and\n` + + ` nothing checked it. Silently unpublishing a schema deletes its reference docs on the\n` + + ` next gen:docs run (see #2978).`, ); process.exit(1); } @@ -951,9 +1016,6 @@ function registeredRetiredKeys(): Map { return out; } -/** The protocol major this build is: the major any retirement it makes belongs to. */ -const CURRENT_MAJOR = Number.parseInt(SPEC_VERSION, 10); - const currentKeys = new Map(); // key -> isRetired for (const [defKey, schema] of generatedSchemas) { const props = (schema as { properties?: Record }).properties; @@ -1124,12 +1186,17 @@ if (surfaceDoc) { ` (ADR-0087 D4) is a projection of the conversion + migration registries, and the\n` + ` generated upgrade guide and the \`spec_changes\` MCP tool are projections of that.\n` + ` Without an entry a consumer only learns of this by failing.\n\n` + - ` 1. Declare each retirement by its EXACT key in RETIRED_KEYS_BY_MAJOR\n` + - ` (packages/spec/src/migrations/registry.ts) — copy these lines in:\n\n` + - unregistered.map((k) => ` '${k}',\n`).join('') + - `\n under \`${CURRENT_MAJOR}: [ … ]\` (create the major's array if it is the first).\n` + - ` Nothing is inferred here: a leaf name matched against unrelated conversion\n` + - ` surfaces is what let tombstones register themselves by coincidence (#4659).\n\n` + + ` 1. Register each retirement by its EXACT key in RETIRED_KEYS_BY_MAJOR. That table\n` + + ` is generated: add one entry file per key (named per\n` + + ` ${MIGRATION_ENTRIES_README}; these names are the generator's own) —\n\n` + + renderNewEntryFiles('retired-key', unregistered) + + `\n — with a \`//\` comment above the \`export\` saying why the key was retired (it\n` + + ` lands above the entry in the table), then run\n` + + ` \`${MIGRATION_REGISTRY_COMMAND}\`. ⛔ Never type an entry into the\n` + + ` generated table by hand: the next run reverts it, and check:migration-registry\n` + + ` fails it first. Nothing is inferred here: a leaf name matched against unrelated\n` + + ` conversion surfaces is what let tombstones register themselves by coincidence\n` + + ` (#4659).\n\n` + ` 2. Add a D2 conversion in src/conversions/registry.ts naming the surface (and a D3\n` + ` chain step referencing it) so \`os migrate meta\` rewrites their source.`, ); @@ -1174,15 +1241,19 @@ if (surfaceDoc) { console.error( `\n❌ ${liveButRegistered.length} RETIRED_KEYS_BY_MAJOR entr(ies) name a key that is still LIVE:`, ); - for (const [k, major] of liveButRegistered) console.error(` - ${k} (registered at major ${major})`); + for (const [k, major] of liveButRegistered) { + console.error(` - ${k} (registered at major ${major})`); + for (const file of retiredKeyEntryFiles(k)) console.error(` ${file}`); + } console.error( `\n RETIRED_KEYS_BY_MAJOR records keys that ARE tombstoned — \`retiredKey()\`, which Zod\n` + ` emits as \`{ "not": {} }\`. These are still writable, so the entry registers a\n` + ` retirement nobody performed, and check (b) above would accept the real tombstone\n` + ` later without it ever being declared.\n\n` + ` Either tombstone the key in its schema (\`retiredKey(' was removed in … — use\n` + - ` . …')\`), or delete the entry from\n` + - ` packages/spec/src/migrations/registry.ts.`, + ` . …')\`), or delete the entry file named under it above and run\n` + + ` \`${MIGRATION_REGISTRY_COMMAND}\` — the table is generated from\n` + + ` those files, so ⛔ never delete its line by hand.`, ); process.exit(1); } @@ -1213,6 +1284,7 @@ if (surfaceDoc) { ); for (const [k, major] of unresolvableNested) { console.error(` - ${k} (registered at major ${major})`); + for (const file of retiredKeyEntryFiles(k)) console.error(` ${file}`); } console.error( `\n The def IS emitted; the dotted path is not in it. Unlike a top-level key, a\n` + @@ -1221,7 +1293,10 @@ if (surfaceDoc) { ` aged-out steady state: the row names a path this build has no property for.\n\n` + ` Fix the spelling against the emitted schema (json-schema//.json —\n` + ` an array member is spelled without its \`[]\`, as \`steps.estimatedMinutes\`), or\n` + - ` delete the entry from packages/spec/src/migrations/registry.ts.\n\n` + + ` delete the entry. Either is an edit to the entry file named under the row above —\n` + + ` a respelled id renames that file, and the generator names the name it expects —\n` + + ` then run \`${MIGRATION_REGISTRY_COMMAND}\`; the table is generated\n` + + ` from those files, so ⛔ never edit its line by hand.\n\n` + ` Two legitimate shapes this check cannot yet tell apart from a wrong row, both\n` + ` unreached on this tree — if yours is the first, teach this check the shape,\n` + ` ⛔ do not delete a row that is telling the truth:\n` + @@ -2569,6 +2644,7 @@ function checkManifestRemovals(git: GitRun, baseRev: string | null): void { ); for (const [def, major] of stillPublished) { console.error(` - ${def} (registered at major ${major})`); + for (const file of retiredDefEntryFiles(def)) console.error(` ${file}`); } console.error( `\n RETIRED_DEFS_BY_MAJOR records defs that HAVE left the published set — no\n` + @@ -2576,7 +2652,9 @@ function checkManifestRemovals(git: GitRun, baseRev: string | null): void { ` so the entry registers a removal nobody performed, and the gate below would accept\n` + ` the real one later without it ever being declared.\n\n` + ` Either remove the export (and its ${MANIFEST_FILE_NAME} line) in this PR, or delete\n` + - ` the entry from packages/spec/src/migrations/registry.ts.`, + ` the entry file named under it above and run\n` + + ` \`${MIGRATION_REGISTRY_COMMAND}\` — the table is generated from\n` + + ` those files, so ⛔ never delete its line by hand.`, ); process.exit(1); } @@ -2633,10 +2711,13 @@ function checkManifestRemovals(git: GitRun, baseRev: string | null): void { ` waived every baseline line under the vanished def on the grounds that this file would\n` + ` adjudicate it. Removals are therefore compared against ${MANIFEST_FILE_NAME} at the\n` + ` merge base ${short} with origin/main, which this commit cannot rewrite.\n\n` + - ` 1. Declare each removal by its EXACT def key in RETIRED_DEFS_BY_MAJOR\n` + - ` (packages/spec/src/migrations/registry.ts) — copy these lines in:\n\n` + - unregistered.map((def) => ` '${def}',\n`).join('') + - `\n under \`${CURRENT_MAJOR}: [ … ]\` (create the major's array if it is the first).\n\n` + + ` 1. Register each removal by its EXACT def key in RETIRED_DEFS_BY_MAJOR. That table\n` + + ` is generated: add one entry file per def (named per\n` + + ` ${MIGRATION_ENTRIES_README}; these names are the generator's own) —\n\n` + + renderNewEntryFiles('retired-def', unregistered) + + `\n — then run \`${MIGRATION_REGISTRY_COMMAND}\`. ⛔ Never type an\n` + + ` entry into the generated table by hand: the next run reverts it, and\n` + + ` check:migration-registry fails it first.\n\n` + ` 2. Add a D2 conversion in src/conversions/registry.ts naming the surface (and a D3\n` + ` chain step referencing it) plus a \`major\` changeset, so the removal reaches\n` + ` spec-changes.json, the upgrade guide and \`os migrate meta\` — the table is the\n` + @@ -2822,10 +2903,13 @@ let gitResolvedAnchor: { rev: string; keys: string[] } | null = null; ` A line may only leave this file when:\n` + ` 1. its key was tombstoned (\`retiredKey()\` → "[RETIRED]") AND that key is\n` + ` declared — EXACTLY, as '\${defKey}:\${name}' — in RETIRED_KEYS_BY_MAJOR\n` + - ` (src/migrations/registry.ts) under a major ≥ ${TOMBSTONE_AGE_MAJORS} behind this one\n` + - ` (≤ v${CURRENT_MAJOR - TOMBSTONE_AGE_MAJORS}). Tombstones that predate that table are deliberately\n` + - ` undeclared: nothing could date them honestly, so they are NOT deletable\n` + - ` until someone establishes the true major and writes it down (#5898). The\n` + + ` under a major ≥ ${TOMBSTONE_AGE_MAJORS} behind this one (≤ v${CURRENT_MAJOR - TOMBSTONE_AGE_MAJORS}). Tombstones that\n` + + ` predate that table are deliberately undeclared: nothing could date them\n` + + ` honestly, so they are NOT deletable until someone establishes the true\n` + + ` major and writes it down (#5898) — as one entry file under\n` + + ` ${entryDirOf('retired-key')}, named for that major per\n` + + ` ${MIGRATION_ENTRIES_README}, then \`${MIGRATION_REGISTRY_COMMAND}\`;\n` + + ` the table is generated, ⛔ never typed. The\n` + ` D2 conversion (src/conversions/registry.ts) naming the surface stays\n` + ` required — it is the prescription consumers follow — but it is no longer\n` + ` what dates the clock; or\n` + @@ -2833,7 +2917,8 @@ let gitResolvedAnchor: { rev: string; keys: string[] } | null = null; ` that itself (it would have said so above); or\n` + ` 3. its whole def stopped being emitted — adjudicated by the manifest deletion\n` + ` gate above (#4725), which demands the removal be declared in\n` + - ` RETIRED_DEFS_BY_MAJOR (src/migrations/registry.ts); or\n` + + ` RETIRED_DEFS_BY_MAJOR — an entry file under ${entryDirOf('retired-def')},\n` + + ` whose name that gate prints; or\n` + ` 4. its baseline entry was NOT \`[RETIRED]\`, and writing the key on its def is\n` + ` REFUSED as an unrecognized key carrying the prescription its\n` + ` \`strictObject\` \`guidance\` table declares for it — an enumerated\n` + @@ -3258,9 +3343,11 @@ if (!defaultsBaseline) { ` against sources you do not control — \`from\` against the baseline, \`to\` against what\n` + ` the build emits — so the declaration dies the moment it stops being true.\n\n` + ` If the default was NOT meant to move, restore it in the schema. And if the value\n` + - ` genuinely has to change for existing documents too, add a \`semantic\` entry to this\n` + - ` major's step in src/migrations/registry.ts so it reaches spec-changes.json, the\n` + - ` upgrade guide and \`os migrate meta\`.`, + ` genuinely has to change for existing documents too, add a \`semantic\` entry for major\n` + + ` ${CURRENT_MAJOR} — one entry file under ${entryDirOf('semantic')}, named per\n` + + ` ${MIGRATION_ENTRIES_README}, then \`${MIGRATION_REGISTRY_COMMAND}\`\n` + + ` (the step's list is generated, ⛔ never typed) — so it reaches spec-changes.json,\n` + + ` the upgrade guide and \`os migrate meta\`.`, ); process.exit(1); } diff --git a/packages/spec/src/ai/tool.zod.ts b/packages/spec/src/ai/tool.zod.ts index 66110127be..5e2f387b1f 100644 --- a/packages/spec/src/ai/tool.zod.ts +++ b/packages/spec/src/ai/tool.zod.ts @@ -28,10 +28,11 @@ import { strictObject } from '../shared/strict-object'; // ========================================== /** - * Retired `ToolSchema` keys — the rejection carries the upgrade prescription, - * because the parse error is the one channel every consumer bumping + * Keys REMOVED from `ToolSchema`'s shape — the rejection carries the upgrade + * prescription, because the parse error is the one channel every consumer bumping * `@objectstack/spec` is guaranteed to hit (pattern of `object.zod.ts`'s - * `UNKNOWN_KEY_GUIDANCE`, ADR-0049 enforce-or-remove). + * `UNKNOWN_KEY_GUIDANCE`, ADR-0049 enforce-or-remove). A key retired in place is + * a `retiredKey()` tombstone in the shape instead, never a row here. */ const TOOL_RETIRED_KEY_GUIDANCE: Record = { permissions: diff --git a/packages/spec/src/shared/strict-object.ts b/packages/spec/src/shared/strict-object.ts index 96bc167d2e..8dfd41fd28 100644 --- a/packages/spec/src/shared/strict-object.ts +++ b/packages/spec/src/shared/strict-object.ts @@ -53,8 +53,9 @@ * tells an adopter holding a confident wrong suggestion — the case the * option is most needed for — that `aliases` is not their tool, which is * this campaign's own finding-7 shape (see {@link acceptsNothing}). - * - `guidance` — tombstones for retired keys (the rejection must carry the - * upgrade) and wrong-layer pointers. + * - `guidance` — prescriptions for keys the shape does not declare: wrong-layer + * pointers, and the upgrade for a spelling removed from it (a key retired in + * place is a `retiredKey()` tombstone in the shape, never a row here). * * Both are optional. A schema with neither still gets a named surface, the * offending key echoed back, and a distance-based suggestion — which is the @@ -149,8 +150,10 @@ export interface StrictObjectOptions { */ aliases?: Readonly>; /** - * Exact-key prescriptions appended as bullet lines: tombstones for retired - * keys, wrong-layer pointers. An entry here suppresses the rename suggestion. + * Exact-key prescriptions appended as bullet lines, for keys the shape does + * not declare: wrong-layer pointers, a removed spelling's upgrade (a retired + * key's tombstone is `retiredKey()` in the shape). An entry here suppresses + * the rename suggestion. */ guidance?: Readonly>; /** From c2e5b3b8146a6917038bb6647b658e29fc7ce1ee Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 00:48:12 +0000 Subject: [PATCH 2/2] test(spec): the end-to-end unpublish repro pins the entry file, not a paste line; changeset for the shipped docblocks Claude-Session: https://claude.ai/code/session_017VaLJnYwhPsanVCe9dMCJU Co-authored-by: Claude --- .changeset/20809-guidance-slot-docblocks.md | 16 ++++++++++++++++ .../scripts/build-schemas-check-mode.test.ts | 4 +++- 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 .changeset/20809-guidance-slot-docblocks.md diff --git a/.changeset/20809-guidance-slot-docblocks.md b/.changeset/20809-guidance-slot-docblocks.md new file mode 100644 index 0000000000..9886ddce22 --- /dev/null +++ b/.changeset/20809-guidance-slot-docblocks.md @@ -0,0 +1,16 @@ +--- +'@objectstack/spec': patch +--- + +docs(spec): the `strictObject` `guidance` option and the `ToolSchema` guidance table no longer describe `guidance` rows as tombstones for retired keys + +Clause-②: no + +`StrictObjectOptions.guidance` (in the published declarations) and the docblock above +`TOOL_RETIRED_KEY_GUIDANCE` in `src/ai/tool.zod.ts` (shipped as source) called the slot a +place for "tombstones for retired keys". A tombstone is `retiredKey()` in the shape, which +keeps the key declared, and a `guidance` row for a declared key never fires. The docblocks +now say what the slot is for: prescriptions for keys the shape does not declare — +wrong-layer pointers, and the upgrade for a spelling removed from the shape. + +Text only: no schema, no guidance entry, no prescription and no parse behaviour changes. diff --git a/packages/spec/scripts/build-schemas-check-mode.test.ts b/packages/spec/scripts/build-schemas-check-mode.test.ts index e559ec85b7..76e2f9c856 100644 --- a/packages/spec/scripts/build-schemas-check-mode.test.ts +++ b/packages/spec/scripts/build-schemas-check-mode.test.ts @@ -4103,7 +4103,9 @@ describe('build-schemas.ts — a deleted manifest key must prove itself (#4725)' expect(output).toContain(`${UNPUBLISHED_FAMILY.length} ${NO_REGISTERED_REMOVAL}`); for (const def of UNPUBLISHED_FAMILY) { expect(output).toContain(` - json-schema/${def}.json`); - expect(output).toContain(` '${def}',`); + // …and the entry file that registers it, never a line for the table. + expect(output).toContain(entryFile('retired-defs', def)); + expect(output).toContain(`export const entry = '${def}';`); } // The old verdict, in full: check (c) waived all 116 lines as "def no // longer emitted", the manifest ratchet said nothing, and the run exited