From 32d5f7b8dd9a6ecd770a8158c44abb41d585605c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 11:13:11 +0000 Subject: [PATCH] =?UTF-8?q?fix(spec):=20=E9=80=80=E4=BC=91=E7=99=BB?= =?UTF-8?q?=E8=AE=B0=E6=8C=89=E7=A1=AE=E5=88=87=20key=20=E5=88=A4=E5=AE=9A?= =?UTF-8?q?,=E5=8F=B6=E5=90=8D=E5=8C=B9=E9=85=8D=E4=B8=8D=E5=86=8D?= =?UTF-8?q?=E8=AE=A9=E6=97=A0=E5=85=B3=E7=B0=87=E6=9B=BF=20tombstone=20?= =?UTF-8?q?=E8=83=8C=E4=B9=A6=20(#4659)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `build-schemas.ts` 检查 (b)(`check:authorable-surface`)此前判定「这次退休 已登记」的方式是:取 key 的叶名,和全部 major 的所有 conversion / migration `surface` 子句做 `endsWith('.' + name)`,完全不看 key 属于哪个 def。任何无关 登记只要 surface 以同名叶子结尾,就替这个墓碑背书。#4658 实测: `automation/Event:type` 零 conversion 静默通过,命中的是 protocol 11 的 `flow.node.type`;#5509 之后 `.description` 也进了这个免检名单。 - 新增导出 `RETIRED_KEYS_BY_MAJOR`(`src/migrations/registry.ts` 末尾追加, 未改动该文件任何既有行),值是确切的 `${defKey}:${name}`。 - 检查 (b) 改为对该表精确集合判定;失败信息直接打印要粘贴的那一行和 major。 - 新增检查 (b2):登记了一个仍然 live 的 key 直接失败;登记了一个本次构建已不 再产出的 key 不是错误(墓碑老化后的预期稳态)。 - conversion 的 `surface` 散文一字未动;检查 (c) 的叶名匹配保留,原因与后续 处置记在 #5898。 - 退休 playbook(`.claude/skills/spec-property-retirement/SKILL.md`)同步: 原先那条「surface 必须以裸 key 结尾」正是本单的缺陷,已改写。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01559M8FVm6W6vDLABL3jvdW --- .changeset/retired-keys-exact-registry.md | 44 +++ .../skills/spec-property-retirement/SKILL.md | 43 ++- packages/spec/api-surface.json | 1 + .../scripts/build-schemas-check-mode.test.ts | 290 +++++++++++++++++- packages/spec/scripts/build-schemas.ts | 103 +++++-- packages/spec/src/migrations/index.ts | 1 + packages/spec/src/migrations/registry.ts | 68 ++++ 7 files changed, 514 insertions(+), 36 deletions(-) create mode 100644 .changeset/retired-keys-exact-registry.md diff --git a/.changeset/retired-keys-exact-registry.md b/.changeset/retired-keys-exact-registry.md new file mode 100644 index 0000000000..f8dffe00a6 --- /dev/null +++ b/.changeset/retired-keys-exact-registry.md @@ -0,0 +1,44 @@ +--- +"@objectstack/spec": patch +--- + +fix(spec): 退休登记按**确切 key** 判定 —— 无关簇的同名叶子不再让 tombstone 冒充「已登记」(#4659) + +`scripts/build-schemas.ts` 的检查 (b)(`check:authorable-surface`)保证一件事: +一个 authorable key 从 live 翻成 retired(`retiredKey()` 墓碑)时,这次退休必须 +被登记下来,否则 `spec-changes.json`(ADR-0087 D4)、生成的升级指南、`spec_changes` +MCP 工具全是空的,消费者只能靠失败才知道。 + +它此前是这样判定「已登记」的:取 key 的**叶名**,拿去和**全部 major** 的所有 +conversion / migration `surface` 子句做 `endsWith('.' + name)` —— 完全不看 key +属于哪个 def。于是任何一条无关登记,只要 surface 以同名叶子结尾,就把这个墓碑判为 +「已登记」: + +- #4658 实测:`automation/Event:type` 被 tombstone、零 conversion,门禁全绿 —— + 命中的是 protocol 11 的 `flow-node-http-callout-rename`(`flow.node.type`), + 一个 flow 节点的 `type`,和状态机事件毫无关系。 +- #5509(ADR-0087 D2 `page-header-subtitle-alias`)登记 + `page.component.page-header.description` 之后,任何叶名为 `description` 的 key + 也进了同一个免检名单。`type` / `name` / `config` / `filter` / `schema` / + `description` 都是 authorable 形状上最常见的叶子,这条保证对它们整体失效。 + +现在登记有了自己的表: + +- **新增导出 `RETIRED_KEYS_BY_MAJOR`**(`@objectstack/spec`, + `src/migrations/registry.ts`),值就是确切的 `` `${defKey}:${name}` `` 字符串 —— + `authorable-surface.json` 怎么写它就怎么写,去掉 `[RETIRED]` 标记。 +- 检查 (b) 改为对这张表做**精确集合判定**:不再有 `endsWith`,不再取叶名,不再从 + 相邻的 key 辐射过来。门禁失败时直接打印要粘贴的那一行和它该进哪个 major。 +- 新增检查 (b2):表里登记了一个**当前仍然 live** 的 key —— 一次没有任何东西消费的 + 登记 —— 直接失败;它会替一次尚未发生的退休提前放行。登记了一个本次构建**已不再 + 产出**的 key 则不是错误:墓碑满 ~2 个 major 之后由检查 (c) 放行其基线行,登记条目 + 留下,这是预期稳态。 + +conversion 的 `surface` 保持散文形态、一个字没动:它面向作者、按作者书写元数据的 +形状表达(`flow.nodes[].outputSchema`),本来就无法可判定地映射回 def key。所以 +搬走的是机器事实,不是散文。一次退休仍然两样都要写:登记条目是**声明的凭据**, +conversion 是**消费者照着做的处方**。 + +不回填历史:检查 (b) 只在相对已提交基线的 live → retired **新**跃迁上触发,而更早的 +墓碑在基线里已经是 `[RETIRED]`,不会再触发它。所以这张表读作「在确切-key 门禁下登记的 +退休」,不是「历史上的全部退休」—— 空表在 `main` 上实测全绿。 diff --git a/.claude/skills/spec-property-retirement/SKILL.md b/.claude/skills/spec-property-retirement/SKILL.md index 1e49d5a148..db4c16e269 100644 --- a/.claude/skills/spec-property-retirement/SKILL.md +++ b/.claude/skills/spec-property-retirement/SKILL.md @@ -171,10 +171,15 @@ jumping several majors at once, whom the load-path conversion no longer covers. ## 3. Register the surface (ADR-0087 D2/D3) — or the gate stops you -`scripts/build-schemas.ts` gate (b) fails any newly-tombstoned key with no -registered migration surface: the tombstone is audible only to whoever *hits* -it, while `spec-changes.json`, the generated upgrade guide and the -`spec_changes` MCP tool are the primary channel and would stay empty. +`scripts/build-schemas.ts` gate (b) fails any newly-tombstoned key that is not +registered by its **exact** `${defKey}:${name}` in `RETIRED_KEYS_BY_MAJOR`: the +tombstone is audible only to whoever *hits* it, while `spec-changes.json`, the +generated upgrade guide and the `spec_changes` MCP tool are the primary channel +and would stay empty. + +⚠ **This is two separate obligations, and only one of them is a string match.** +The registry entry is what the gate reads; the conversion is what a consumer +follows. Write both. - [ ] **A `MetadataConversion`** in `packages/spec/src/conversions/registry.ts`: kebab-case `id` ending `-removed`, `toMajor`, one @@ -183,13 +188,27 @@ it, while `spec-changes.json`, the generated upgrade guide and the not the item count. Walkers (`mapCollection`, `mapFlowNodes`, `renameKey`) live in `conversions/walk.ts` and are copy-on-write — return the input reference untouched when nothing matched. -- [ ] **`surface` must end with the bare key.** The matcher is - `surfaces.some((s) => s.endsWith('.' + key))` after - `.flatMap((s) => s.split(' / '))`. Multi-key conversions join clauses with - exactly `' / '` (house style since the tool sweep) and **each clause must - end with its own key**. Caveat: only the last dotted segment is compared, - so the schema name is never checked — `dashboard.aria` would satisfy - `ui/FormView:aria`. Don't lean on the gate for attribution. +- [ ] **A `RETIRED_KEYS_BY_MAJOR` entry** in + `packages/spec/src/migrations/registry.ts` — the literal + `':'` as `authorable-surface.json` spells it, minus the + `[RETIRED]` mark, under this major. This is the string gate (b) reads, by + exact set membership; nothing is inferred and nothing radiates from a + neighbouring key. The gate's failure prints the line to paste. ⚠ Do **not** + add the entry before the tombstone lands: an entry naming a key that is + still live fails gate (b2) as a registration nothing consumed. + *Why a second table:* until #4659 gate (b) matched the key's **leaf** against + every registered `surface` (`endsWith('.' + name)`, all majors, def + ignored), so `dashboard.aria` registered `ui/FormView:aria` and protocol + 11's `flow.node.type` registered any `.type` at all (#4658). The guarantee + had lapsed for every common leaf. +- [ ] **`surface` stays prose — it is no longer matched.** Write it the way an + author writes metadata (`flow.nodes[].outputSchema`), which is what the + upgrade guide prints. Multi-key conversions still join clauses with exactly + `' / '` (house style since the tool sweep). Nothing downstream parses it + for attribution any more — that job moved to the entry above. ⚠ One + consumer still does read the clauses by leaf: gate (c)'s *aged-out + tombstone* proof, which adjudicates retirements older than + `RETIRED_KEYS_BY_MAJOR` and could not be moved with it (#5898). - [ ] **`retiredFromLoadPath: true`** — for a retirement, always. Two distinct justifications, and they are not interchangeable: for a *rename* it means "no alias window, deliberately" (the tombstone owns the refusal; the entry @@ -243,7 +262,7 @@ Work top to bottom; each line has a gate behind it. keys from `defineAction`'s input and the snapshot did not change). Its gate also lives in a different workflow (`TypeScript Type Check`, not `Check Generated Artifacts`) and reads the built `dist/*.d.ts`. -- [ ] **Conversion + chain step** (§3). +- [ ] **Conversion + chain step + the exact-key `RETIRED_KEYS_BY_MAJOR` entry** (§3). - [ ] **Liveness ledger** — per §2's route table, with `verifiedAt`. Update the README's per-type row **and its counts** (that table has drifted badly once; regenerate the counts with the python snippet in the README rather diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 3ea70c5c92..ed3d3dc05c 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -91,6 +91,7 @@ "PredicateInput (type)", "PredicateInputSchema (const)", "PredicateSchema (const)", + "RETIRED_KEYS_BY_MAJOR (const)", "STACK_KEY_GUIDANCE (const)", "STACK_RUNTIME_MEMBERS (const)", "SemanticMigration (interface)", diff --git a/packages/spec/scripts/build-schemas-check-mode.test.ts b/packages/spec/scripts/build-schemas-check-mode.test.ts index a67597f6db..3e438911a0 100644 --- a/packages/spec/scripts/build-schemas-check-mode.test.ts +++ b/packages/spec/scripts/build-schemas-check-mode.test.ts @@ -56,7 +56,7 @@ import { fileURLToPath } from 'node:url'; import { RENAMED_DEFS } from './lib/renamed-defs'; import { CONVERSIONS_BY_MAJOR } from '../src/conversions/registry'; -import { MIGRATIONS_BY_MAJOR } from '../src/migrations/registry'; +import { MIGRATIONS_BY_MAJOR, RETIRED_KEYS_BY_MAJOR } from '../src/migrations/registry'; const HERE = path.dirname(fileURLToPath(import.meta.url)); const PKG = path.resolve(HERE, '..'); @@ -71,6 +71,8 @@ const REAL_MANIFEST = path.join(PKG, 'json-schema.manifest.json'); */ const SPAWN_TIMEOUT_MS = 180_000; +/** The mark `authorable-surface.json` puts on a tombstoned key (build-schemas.ts). */ +const RETIRED_MARK = ' [RETIRED]'; /** A schema key the committed manifest carries; dropping it fakes "one addition pending". */ const KNOWN_KEY = 'ui/View'; /** A key no build can emit — the `missing` (disappearance) ratchet's input. */ @@ -1218,3 +1220,289 @@ describe('build-schemas.ts — --update-base moves the anchor forward or not at }, ); }); + +// ───────────────────────────────────────────────────────────────────────────── +// #4659 — check (b) registers a tombstone by its EXACT key, not by its leaf. +// +// Until this change, "live → retired must be registered" was satisfied by +// matching the key's LEAF against every ' / '-separated conversion/migration +// `surface` in the ADR-0087 registries — `endsWith('.' + name)`, across all +// majors, ignoring which def the key belonged to. So any unrelated registration +// ending in the same leaf registered a tombstone for free. #4658 measured it: +// tombstoning `automation/Event:type` passed silently because protocol 11's +// `flow-node-http-callout-rename` had registered `flow.node.type`. #5509 widened +// the leaf vocabulary to `.description`, one of the most common keys on +// authorable shapes, and the exposure grew with every conversion. +// +// The registration now lives in its own table — RETIRED_KEYS_BY_MAJOR in +// src/migrations/registry.ts, values the literal `${defKey}:${name}` — and +// check (b) is exact set membership over it. Conversion `surface` prose is +// untouched: it addresses authors in the shape they write metadata +// (`flow.nodes[].outputSchema`) and no rule can map that back onto a def key. +// +// ── Why a SECOND sandbox ────────────────────────────────────────────────── +// These tests need the gate to read a registry that differs per case, and the +// sandbox above symlinks `src/`, so writing one there would write the repo's +// tracked registry. This one COPIES `src/` instead (10 MB, ~40 ms) so its +// `src/migrations/registry.ts` is a fixture. Nothing else changes: the gate +// under test is the same copied `scripts/build-schemas.ts`, reading a byte-equal +// spec source, and no test-only seam is added to it — the substituted table is +// fixture data exactly as the fabricated `refs/remotes/origin/main` is. +// +// ── Why the green cases still exit 1 ────────────────────────────────────── +// A live → retired transition exists only when the committed baseline disagrees +// with the emitted contract, so every fixture here is, by construction, a +// baseline the generated-form reporter at the end of the script will call stale. +// That reporter is a different failure with a different remedy, and the +// assertions below discriminate on the message, never on the exit code alone. + +/** Retired keys whose ONLY registered clause names a DIFFERENT def — the defect. */ +const LEAF_COLLIDER_A = 'api/HttpFindQueryParams:distinct'; +/** …matched by this clause, which registers `data/Query`'s key, not this one. */ +const LEAF_COLLIDER_A_CLAUSE = 'data.query.distinct'; +/** The issue's own `.type` shape: an index type dated by a flow node rename. */ +const LEAF_COLLIDER_B = 'data/Index:type'; +/** …registered at protocol 11 by `flow-node-http-callout-rename`. Unrelated. */ +const LEAF_COLLIDER_B_CLAUSE = 'flow.node.type'; +/** A key no author has ever been able to lose — the (b2) "still LIVE" fixture. */ +const STILL_LIVE_KEY = 'data/Object:name'; +/** A key no build emits: the aged-out steady state a stale entry decays into. */ +const AGED_OUT_KEY = 'data/Object:zzAgedOutTombstone4659'; + +/** + * The pre-#4659 matcher, reproduced ONLY to prove the fixtures still model the + * defect: every registered clause whose leaf would have registered `key`. If + * this ever returns nothing for a fixture, the fixture stopped being a + * collision and the test below asserts something weaker than it claims. + */ +function clausesMatchingLeafOf(key: string): string[] { + const leaf = key.slice(key.indexOf(':') + 1); + const out: string[] = []; + const consider = (surface: string): void => { + for (const clause of surface.split(' / ')) if (clause.endsWith('.' + leaf)) out.push(clause); + }; + for (const list of Object.values(CONVERSIONS_BY_MAJOR)) for (const c of list) consider(c.surface); + for (const step of Object.values(MIGRATIONS_BY_MAJOR)) { + for (const sem of step.semantic ?? []) consider(sem.surface); + } + return out; +} + +describe('build-schemas.ts — check (b) matches the exact retired key, not its leaf (#4659)', () => { + let box: string; + let boxScript: string; + let boxSurface: string; + let boxRegistry: string; + let pristineRegistry: string; + + const boxGit = (...args: string[]): string => { + const r = spawnSync( + 'git', + ['-c', 'user.name=build-schemas-test', '-c', 'user.email=test@example.invalid', ...args], + { cwd: box, encoding: 'utf8' }, + ); + if (r.status !== 0) throw new Error(`git ${args.join(' ')} failed (${r.status}): ${r.stderr}`); + return (r.stdout ?? '').trim(); + }; + + const runBox = (args: string[] = []): { status: number; output: string } => { + const r = spawnSync(TSX, [boxScript, ...args], { + cwd: box, + encoding: 'utf8', + timeout: SPAWN_TIMEOUT_MS, + stdio: ['ignore', 'pipe', 'pipe'], + }); + return { status: r.status ?? -1, output: `${r.stdout ?? ''}${r.stderr ?? ''}` }; + }; + + /** Untombstone keys in the sandbox baseline: the gate then sees live → retired. */ + const untombstone = (...keys: string[]): void => { + const doc = JSON.parse(pristineSurface) as { description: string; keys: string[] }; + doc.keys = doc.keys.map((e) => (keys.includes(e.replace(RETIRED_MARK, '')) ? e.replace(RETIRED_MARK, '') : e)); + fs.writeFileSync(boxSurface, JSON.stringify(doc, null, 2) + '\n'); + }; + + /** Substitute RETIRED_KEYS_BY_MAJOR in the sandbox's own copy of the registry. */ + const seedRetiredKeys = (table: Record): void => { + const rendered = + `export const RETIRED_KEYS_BY_MAJOR: Readonly> = {\n` + + Object.keys(table) + .map(Number) + .sort((a, b) => a - b) + .map((m) => ` ${m}: [\n${table[m]!.map((k) => ` '${k}',\n`).join('')} ],\n`) + .join('') + + `};\n`; + const anchor = /export const RETIRED_KEYS_BY_MAJOR[\s\S]*?\n\};\n/; + expect( + anchor.test(pristineRegistry), + 'RETIRED_KEYS_BY_MAJOR is no longer a single object literal in src/migrations/registry.ts — ' + + 'this fixture substitutes it textually and can no longer find it', + ).toBe(true); + fs.writeFileSync(boxRegistry, pristineRegistry.replace(anchor, rendered)); + }; + + const CHECK_B = 'key(s) were tombstoned with no registered retirement'; + + beforeAll(() => { + // Fixture validity, loud: both keys must still be leaf-collisions (they are + // the defect), and neither may be registered in the real table (they are + // NOT retirements this repo made under the new gate). + expect(clausesMatchingLeafOf(LEAF_COLLIDER_A)).toContain(LEAF_COLLIDER_A_CLAUSE); + expect(clausesMatchingLeafOf(LEAF_COLLIDER_B)).toContain(LEAF_COLLIDER_B_CLAUSE); + const declared = Object.values(RETIRED_KEYS_BY_MAJOR).flat(); + for (const k of [LEAF_COLLIDER_A, LEAF_COLLIDER_B]) { + expect(declared, `${k} is now registered for real — pick an unregistered fixture`).not.toContain(k); + } + const baselineKeys = (JSON.parse(pristineSurface) as { keys: string[] }).keys; + for (const k of [LEAF_COLLIDER_A, LEAF_COLLIDER_B]) { + expect(baselineKeys, `${k} is no longer tombstoned — re-pick`).toContain(k + RETIRED_MARK); + } + expect(baselineKeys, `${STILL_LIVE_KEY} is no longer a live authorable key`).toContain(STILL_LIVE_KEY); + expect(baselineKeys.some((k) => k.startsWith(AGED_OUT_KEY))).toBe(false); + + box = fs.mkdtempSync(path.join(os.tmpdir(), 'build-schemas-retired-keys-')); + fs.cpSync(path.join(PKG, 'scripts'), path.join(box, 'scripts'), { recursive: true }); + fs.cpSync(path.join(PKG, 'src'), path.join(box, 'src'), { recursive: true }); + for (const entry of ['node_modules', 'package.json']) { + fs.symlinkSync(path.join(PKG, entry), path.join(box, entry)); + } + fs.writeFileSync(path.join(box, 'json-schema.manifest.json'), pristine); + fs.writeFileSync(path.join(box, 'authorable-surface.json'), pristineSurface); + boxScript = path.join(box, 'scripts', 'build-schemas.ts'); + boxSurface = path.join(box, 'authorable-surface.json'); + boxRegistry = path.join(box, 'src', 'migrations', 'registry.ts'); + pristineRegistry = fs.readFileSync(boxRegistry, 'utf8'); + + boxGit('init', '-q', '-b', 'main', '.'); + boxGit('add', 'authorable-surface.json'); + boxGit('commit', '-q', '-m', 'baseline: committed authorable-surface.json'); + fs.writeFileSync( + path.join(box, 'authorable-surface.base.json'), + JSON.stringify( + { + description: surfaceBaseDescription, + baseRev: boxGit('rev-parse', 'HEAD'), + keys: (JSON.parse(pristineSurface) as { keys: string[] }).keys, + }, + null, + 2, + ) + '\n', + ); + boxGit('add', 'authorable-surface.base.json'); + boxGit('commit', '-q', '-m', 'baseline anchor'); + boxGit('update-ref', 'refs/remotes/origin/main', 'HEAD'); + }); + + afterAll(() => { + if (box) fs.rmSync(box, { recursive: true, force: true }); + }); + + it( + 'a tombstone whose leaf collides with an unrelated registered surface is NOT registered (#4658)', + { timeout: SPAWN_TIMEOUT_MS }, + () => { + // The repro, in the shape #4658 recorded it: two keys tombstoned, no entry + // written for either, and both leaves already spoken for by conversions + // belonging to other defs. Before this change the gate said nothing at all + // about them and the run fell through to the staleness reporter. + seedRetiredKeys({}); + untombstone(LEAF_COLLIDER_A, LEAF_COLLIDER_B); + + const { status, output } = runBox(['--check']); + + expect(status).toBe(1); + 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 D2 conversion is still asked for — the table did not replace it. + expect(output).toContain('src/conversions/registry.ts'); + }, + ); + + it( + 'registering one key registers exactly that key — its neighbour with the same leaf still fails', + { timeout: SPAWN_TIMEOUT_MS }, + () => { + // `data.query.distinct` covered A only by leaf; now even a REAL entry for + // B cannot cover A. This is the whole change in one run: membership is per + // key, and nothing radiates from one registration to another. + seedRetiredKeys({ [CURRENT_MAJOR]: [LEAF_COLLIDER_B] }); + untombstone(LEAF_COLLIDER_A, LEAF_COLLIDER_B); + + const { status, output } = runBox(['--check']); + + expect(status).toBe(1); + expect(output).toContain(`1 ${CHECK_B}`); + expect(output).toContain(` - ${LEAF_COLLIDER_A}`); + expect(output).not.toContain(` - ${LEAF_COLLIDER_B}`); + }, + ); + + it( + 'both keys registered by exact name: check (b) is silent (the run then only owes the regenerated baseline)', + { timeout: SPAWN_TIMEOUT_MS }, + () => { + seedRetiredKeys({ [CURRENT_MAJOR]: [LEAF_COLLIDER_A, LEAF_COLLIDER_B] }); + untombstone(LEAF_COLLIDER_A, LEAF_COLLIDER_B); + + const { status, output } = runBox(['--check']); + + expect(output).not.toContain(CHECK_B); + expect(output).not.toContain('still LIVE'); + expect(output).not.toContain('deleted without proof'); + // Intrinsic to the fixture, not a residue of the gate: a live → retired + // transition IS a baseline that has not been regenerated yet, so the run + // still owes `gen:schema`. Asserted rather than papered over. + expect(status).toBe(1); + expect(output).toContain('authorable-surface.json is out of date (2 key(s) not recorded)'); + expect(output).toContain(`+ ${LEAF_COLLIDER_A}${RETIRED_MARK}`); + }, + ); + + it( + 'an entry naming a key that is still LIVE fails: a registration nothing consumed (b2)', + { timeout: SPAWN_TIMEOUT_MS }, + () => { + // The mirror defect. Without this, an author could pre-register the key + // they intend to retire, and the tombstone would then land — months later, + // in someone else's PR — with check (b) already satisfied and nobody + // writing anything down. + seedRetiredKeys({ [CURRENT_MAJOR]: [STILL_LIVE_KEY] }); + untombstone(); // baseline pristine: nothing is newly retired + + const { status, output } = runBox(['--check']); + + 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('retiredKey('); + expect(output).not.toContain(CHECK_B); + }, + ); + + it( + 'an entry naming a key this build no longer emits is the aged-out steady state, not an error', + { timeout: SPAWN_TIMEOUT_MS }, + () => { + // A tombstone ages out after TOMBSTONE_AGE_MAJORS and check (c) lets its + // baseline line go; the entry here stays and then names nothing. Pinned + // because the opposite ruling — "an entry must always resolve" — would + // force every aged-out retirement to be un-declared to keep the gate + // green, which is the record deleting itself. + seedRetiredKeys({ 11: [AGED_OUT_KEY] }); + untombstone(); + + const { status, output } = runBox(['--check']); + + expect(output).not.toContain('still LIVE'); + expect(output).not.toContain(CHECK_B); + expect(output).not.toContain('deleted without proof'); + expect(status).toBe(0); + }, + ); +}); diff --git a/packages/spec/scripts/build-schemas.ts b/packages/spec/scripts/build-schemas.ts index 5db0ae92ba..8f1347c72f 100644 --- a/packages/spec/scripts/build-schemas.ts +++ b/packages/spec/scripts/build-schemas.ts @@ -12,7 +12,7 @@ import { z } from 'zod'; import { schemaNameFromExportKey } from './lib/schema-name'; import { RENAMED_DEFS, carryAuthorableKey, checkRenameTable } from './lib/renamed-defs'; import { CONVERSIONS_BY_MAJOR } from '../src/conversions/registry'; -import { MIGRATIONS_BY_MAJOR } from '../src/migrations/registry'; +import { MIGRATIONS_BY_MAJOR, RETIRED_KEYS_BY_MAJOR } from '../src/migrations/registry'; import { getMetadataTypeSchema, listMetadataTypeSchemaTypes, @@ -566,18 +566,27 @@ function isRetired(prop: unknown): boolean { return !!not && typeof not === 'object' && Object.keys(not).length === 0; } -/** Every conversion/migration surface registered across the ADR-0087 registries. */ -function registeredRetirementSurfaces(): string[] { - const out: string[] = []; - for (const list of Object.values(CONVERSIONS_BY_MAJOR)) { - for (const c of list) out.push(c.surface); - } - for (const step of Object.values(MIGRATIONS_BY_MAJOR)) { - for (const sem of step.semantic ?? []) out.push(sem.surface); +/** + * Every authorable key the ADR-0087 registries declare as tombstoned, by exact + * `${defKey}:${name}` — carried through declared def renames so an entry + * written under the old def name still resolves (same discipline as the + * baseline snapshot below). + */ +function registeredRetiredKeys(): Map { + const out = new Map(); // key -> earliest major that registered it + for (const [major, keys] of Object.entries(RETIRED_KEYS_BY_MAJOR)) { + for (const key of keys) { + const carried = carryAuthorableKey(key); + const prev = out.get(carried); + if (prev === undefined || Number(major) < prev) out.set(carried, Number(major)); + } } 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; @@ -662,20 +671,28 @@ if (surfaceDoc) { // (b) live → retired: a removal. It must be registered, or the change never // reaches the upgrade guide / `spec_changes` / `migrate meta`. + // + // Registration means EXACT set membership in RETIRED_KEYS_BY_MAJOR + // (src/migrations/registry.ts) — the literal `${defKey}:${name}`. Until + // #4659 this check matched the key's LEAF against every conversion + // `surface` in the ADR-0087 registries (`endsWith('.' + name)`, all + // majors, def ignored), so any unrelated registration ending in the same + // leaf registered a tombstone for free: #4658 measured + // `automation/Event:type` passing silently on protocol 11's + // `flow.node.type`, and #5509 widened the vocabulary to `.description`. + // A conversion `surface` is prose addressed to authors + // (`flow.nodes[].outputSchema`) and cannot be mapped back onto a def key, + // so the machine fact moved to its own table instead of being encoded + // into the prose. The conversion is still the PRESCRIPTION consumers + // follow — the message below asks for both. + const registeredRetired = registeredRetiredKeys(); const newlyRetired = [...currentKeys.entries()] .filter(([k, retired]) => retired && prev.get(k) === false) .map(([k]) => k); if (newlyRetired.length > 0) { - // A multi-key conversion names its keys as ' / '-separated clauses - // (`'flow.active / flow.template / …'` — the house style since the tool - // sweep), so split before matching: every clause must still END with the - // key, which keeps the check exactly as strict per key as before. - const surfaces = registeredRetirementSurfaces().flatMap((s) => s.split(' / ')); - const unregistered = newlyRetired.filter( - (k) => !surfaces.some((s) => s.endsWith('.' + k.split(':')[1])), - ); + const unregistered = newlyRetired.filter((k) => !registeredRetired.has(k)); if (unregistered.length > 0) { - console.error(`\n❌ ${unregistered.length} key(s) were tombstoned with no registered migration:`); + console.error(`\n❌ ${unregistered.length} key(s) were tombstoned with no registered retirement:`); for (const k of unregistered) console.error(` - ${k}`); console.error( `\n The tombstone makes the removal audible to whoever hits it, but the change\n` + @@ -683,12 +700,45 @@ 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` + - ` 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.`, + ` 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` + + ` 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.`, ); process.exit(1); } } + + // (b2) The other direction: an entry that registers a key this build still + // emits as LIVE. Nothing consumed that registration — it pre-approves a + // retirement that has not happened, and check (b) would then wave the + // real one through without anyone writing it down. An entry naming a key + // the build no longer emits at all is NOT an error: that is the expected + // steady state once a tombstone ages out and check (c) lets its baseline + // line go (see RETIRED_KEYS_BY_MAJOR's "Lifecycle"). + const liveButRegistered = [...registeredRetired.entries()].filter( + ([k]) => currentKeys.get(k) === false, + ); + if (liveButRegistered.length > 0) { + 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})`); + 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.`, + ); + process.exit(1); + } } // ─── (c) A deleted baseline line must prove itself (#4650) ───────────── @@ -722,14 +772,21 @@ if (surfaceDoc) { /** A tombstone may be deleted once its registration is this many majors old. */ const TOMBSTONE_AGE_MAJORS = 2; -const CURRENT_MAJOR = Number.parseInt(SPEC_VERSION, 10); /** * Every ' / '-separated surface clause registered across the ADR-0087 * registries, mapped to the EARLIEST major that registered it — the moment the * retirement became visible to consumers, which is when its aging clock - * started. Same clause vocabulary as check (b) above (#4659 tracks the shared - * leaf-name matching). + * started. + * + * This is now the LAST leaf-name matcher in this file: check (b) above moved to + * exact `${defKey}:${name}` membership in RETIRED_KEYS_BY_MAJOR (#4659). Check + * (c) cannot follow it there, because it adjudicates tombstones that predate + * that table and were deliberately not backfilled into it — so the aging proof + * still reads the conversion clauses, leaf and all, and inherits the same + * coincidence (an unrelated cluster's `.type` can date a tombstone's clock). + * Narrowing it needs the historical mapping #4659 would have had to invent — + * all 97 current tombstones predate that table — so it is tracked in #5898. */ function registeredClauseMajors(): Map { const out = new Map(); diff --git a/packages/spec/src/migrations/index.ts b/packages/spec/src/migrations/index.ts index 39f12b0613..0ddf24a264 100644 --- a/packages/spec/src/migrations/index.ts +++ b/packages/spec/src/migrations/index.ts @@ -21,6 +21,7 @@ export { MIGRATIONS_BY_MAJOR, MIGRATION_MAJORS, MIGRATION_SUPPORT_FLOOR, + RETIRED_KEYS_BY_MAJOR, } from './registry.js'; export { applyMetaMigrations, diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index dc8563f7ac..2484edeeba 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -1973,3 +1973,71 @@ export const MIGRATIONS_BY_MAJOR: Readonly> = { export const MIGRATION_MAJORS: readonly number[] = Object.keys(MIGRATIONS_BY_MAJOR) .map(Number) .sort((a, b) => a - b); + +/** + * Every authorable key TOMBSTONED at each protocol major, named EXACTLY. + * + * One entry per key, spelled `${defKey}:${name}` — the same string + * `packages/spec/authorable-surface.json` records, minus its `[RETIRED]` mark + * (`'automation/Event:type'`, `'ui/App:sharing'`). Not a surface path, not a + * prose clause, not a prefix: the literal key. + * + * ## What reads it + * + * Check (b) of `scripts/build-schemas.ts` (`check:authorable-surface`): a key + * that flips live → retired must appear here, by exact set membership, or the + * build fails. Nothing else consumes the table, and nothing infers an entry — + * the author writes the key down or the gate stays red. + * + * ## Why exact keys, and not the conversion `surface` + * + * Check (b) used to satisfy itself by matching the key's LEAF against every + * `surface` registered in {@link CONVERSIONS_BY_MAJOR} / {@link + * MIGRATIONS_BY_MAJOR} — `endsWith('.' + name)`, across all majors, ignoring + * which def the key belonged to. Any unrelated registration ending in the same + * leaf therefore registered a tombstone for free: measured in #4658, tombstoning + * `automation/Event:type` passed silently because protocol 11's + * `flow-node-http-callout-rename` had registered `flow.node.type`. The exposure + * grew with the vocabulary — `type`, `name`, `config`, `filter`, `schema`, + * `description` (#5509) are ordinary leaves on hundreds of authorable shapes — + * so the gate's whole guarantee had lapsed for the most common keys (#4659). + * + * A `surface` is deliberately PROSE: it addresses authors in the shape they + * write metadata (`flow.nodes[].outputSchema`), which no rule can map back onto + * a def key. So the registration moved here instead of being encoded into the + * surface — the conversion keeps its prose, and this table carries the machine + * fact. + * + * ## What it does NOT replace + * + * The D2 conversion (`src/conversions/registry.ts`) and D3 semantic entry stay + * the *documentation* channel: `spec-changes.json` (ADR-0087 D4), the generated + * upgrade guide and `os migrate meta` are projections of those, not of this + * table. An entry here is the *proof the retirement was declared*; the + * conversion is the *prescription a consumer follows*. A retirement needs both. + * + * ## Not a backfill of history + * + * Check (b) fires only on a NEW live → retired transition — measured against the + * committed `authorable-surface.json` baseline, which already records every + * older tombstone as `[RETIRED]`. Retirements that landed before this table + * existed therefore never re-trigger it and are deliberately absent: this reads + * "retirements registered under the exact-key gate", not "every retirement + * ever". Do not reconstruct the missing history by leaf-matching the conversion + * registry — that is precisely the inference #4659 removed. + * + * ## Lifecycle + * + * Entries are permanent. A tombstone ages out after ~two majors and its line + * leaves `authorable-surface.json` (check (c)); its entry here stays, and then + * names a key the build no longer emits — the expected steady state, not an + * error. The one state the gate rejects is an entry naming a key that is still + * LIVE: a registration nothing consumed, pre-approving a retirement that has not + * happened. + * + * @see scripts/build-schemas.ts — checks (b)/(b2), the only consumers + */ +export const RETIRED_KEYS_BY_MAJOR: Readonly> = { + // Empty by design at protocol 17: see "Not a backfill of history" above. The + // first entry arrives with the first retirement tombstoned after #4659. +};