Uh oh!
There was an error while loading. Please reload this page.
fix(spec): gate (a)'s remedy names every retirement route instead of claiming tombstoning is the only one - #12630
Merged
Merged
Conversation
…claiming tombstoning is the only one (#12574) Gate (a) in build-schemas.ts told a retiring dev to "tombstone it instead of deleting it" and that an aged-out tombstone was "the ONE legitimate reason to delete a line here". That is a claim of exhaustiveness, and the gate's own check (c) already contradicts it: a baseline-line deletion is legitimate on any of three computed proofs -- aged-out tombstone, a def unreachable from the metadata-type roots, or a whole def leaving the build (#4650). The cost was measured on a named dev: #12428 started on the unreachable-def route its sibling #12425 had just landed green in this same file, hit gate (a), read the remedy, and switched to tombstoning. Tombstoning was right for #12428 -- the key was leaving a def that survives -- but the reasoning came from the message rather than from that distinction, which the message did not draw. Message text only; the gate's accept/reject behavior is unchanged. The remedy now asks the precondition question ("does anything still PARSE this def?") and gives both answers, carries the 2026-08-02 ruling's limit on the unreachable-def waiver, and keeps the aged-out tombstone as a route rather than as the reason. Pinned by a new check-mode test asserting the fork, both routes, and the absence of the exhaustiveness claim. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xGvxcwPRTJfA7RfjXEYA4
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
os-zhuang
marked this pull request as ready for review
August 27, 2026 02:16
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#12574
Gate (a) of
packages/spec/scripts/build-schemas.ts— "N authorable key(s) disappeared from the contract" — told a retiring dev to "tombstone it instead of deleting it" and closed with:That is a claim of exhaustiveness, and it is false as stated. This PR replaces the remedy text. Message text only — the gate's accept/reject behavior is unchanged.
Clause-②
Clause-②: no (gate message text only; accept/reject behavior unchanged, measured).
The defect, measured on this tree
The gate's own check (c) already admits three proofs for a baseline-line deletion, each computed in-gate (#4650):
RETIRED_KEYS_BY_MAJOR);So the gate's logic has admitted routes 2 and 3 all along; only gate (a)'s text claimed tombstoning was the only route. Re-measured on today's
mainbefore writing anything:tombstone it instead of deleting itinbuild-schemas.tsthe ONE legitimate reason to deletekernel/HotReloadConfig:distributedConfigin the committed baselinekernel/HotReloadConfig:stateStrategyauthorable-surface/kernel.json:227) — control survivesThe route-3 evidence in the card still reads as stated, and the dispatch ruling's ground (iii) holds: the logic admits route 3, the text does not. Not falsified.
The cost was paid by a named dev in one shift: the #12428 dev started on the route its sibling #12425 had just landed green in this same file, hit gate (a), read the remedy, and switched to tombstoning. Tombstoning was the right answer there — the key was leaving a def that survives — but the reasoning came from the message rather than from that distinction, which the message did not draw.
What the new text does
It stops decreeing and asks the precondition question instead:
retiredKey()+ D2/D3 + amajorchangeset). This is [finding] HotReloadManager.startWatching watches nothing and logs "File watching started" at info; watchPatterns has no reader and watchHandles is never populated #12428's case, and its three steps are carried over verbatim.gen:schema). This is fix(spec,core): HotReloadConfig.stateStrategy refuses the two values it never implemented; distributedConfig retired (#12340) #12425's case. The text is explicit that the dev is not taken at their word — check (c) recomputes the answer from the build's own Zod graph and the manifest deletion gate and refuses the deletion if it does not hold.The wording was written against
.claude/skills/spec-property-retirement/SKILL.mdsection 2 (the route-choice table) so the gate and the playbook agree on the preconditions.Behaviour neutrality — measured, not asserted
build-schemas.tsis a template-literal fragment inside theconsole.error(...)argument of gate (a). Filtering the diff to lines that are not a backtick-prefixed string fragment yields nothing — no condition, no control flow, no exit code, noprocess.exitmoved.+23 / -3lines, all inside one string.build-schemas-check-mode.test.ts, which exercise (a0), (a), (b), (b2) and all three of (c)'s proofs including the trip conditions — is green unchanged against the new text. Same trips, same passes. The suite now runs 63 (the 62 plus the new message pin).Verification
All through
scripts/pm/os-verify-lock.sh, exit codes captured before any pipe, and re-run after mergingorigin/main. Everything below is from the final commit1c0b3e302:pnpm --filter @objectstack/spec build(runs the gen:schema gates)VERDICT command-exit 0;authorable-defaults/ verified — 1255 default(s) unchangedpnpm --filter @objectstack/spec check:generatedAll 14 generated artifacts are up to date.Test Files 1 passed (1)·Tests 63 passed (63)pnpm --filter @objectstack/spec testTest Files 435 passed (435)·Tests 11526 passed (11526)pnpm --filter @objectstack/spec typechecktsc --noEmit+check:scripts-typecheck+check:test-typecheckpnpm check:nul-bytesOK (scanned 6979 text file(s) … no raw ASCII control bytes)pnpm check:cross-package-test-inputsOK: 20 package(s) read outside themselves, all declaredpnpm check:engine-double-contractOK — 689 pinned, 134 in the DEBT ledger, 3 exemptpnpm check:where-matcher303 matcher(s) discovered … none newpnpm check:query-options-erasureratchet holds … none newpnpm check:test-source-aliasOK — 72 packages with tests scannedpnpm --filter @objectstack/spec check:authorable-surface1595 schemasnode scripts/pm/bare-root-worklist.mjs --self-testOK self-test: … none stale, none missing, none contradictedpnpm check:pm-dispatch-gatesdispatch-gates self-test: 719 cases passThe last two are the obligations
dispatch-gates.mjsnames for a gate-script edit; both are green with no new ledger row, as a message-text change should be.Typecheck really covers the diff (the NOT-MEASURED trap):
tsc -p tsconfig.scripts.json --listFileslists both edited files, socheck:scripts-typecheck's green is a reading about them, not around them.Gate list derived with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(2 paths, three-dot vs merge base). Left to CI:check:type-check-coverage/check:type-check-debt— the debt ratchet refuses an unbuilt workspace closure and wants the whole monorepo built, which is CI's run; the new test file sits inpackages/spec/scripts, already insidetsconfig.scripts.json, so it is not a new unaccounted-for test surface. Also left to CI: the repo-widepnpm lintsweep and the remaining path-derived families in the table above that CI runs on every PR regardless.Reverse verification
With the text change reverted (
git checkoutof the base revision'sbuild-schemas.ts) and the updated test kept, the new pin turns RED — the predicted direction:Restore proven on disk rather than by exit code:
git checkout HEAD -- path(namingHEAD, since the mutation leg also wrote the index), thengit hash-objectequal to theHEADblob andgit diff HEADempty. The script carried atrap ... EXIT INT TERMrestore with absolute paths throughout. No rebuild is involved in this ablation and none is owed: the check-mode test copiesscripts/into a temp sandbox and runs the script withtsx, so it resolves from source, never fromdist/.Changeset
None — following the precedent for build-script message-text changes. The nearest precedent is
82bfb6ec1(#6309, "锚点漂移提示按实测方向措辞"), which reworded abuild-schemas.tsgate message and updated the same check-mode test — exactly this PR's two-file shape — and carried no changeset. By contrast25982166e(#5898) did carry one, because it changed the tombstone aging logic. The distinction is behaviour, and this PR has none:packages/spec/scripts/is not published, and nothing user-visible ships.skip-changesetapplied accordingly.Scope
Two files, both named in the dispatch. Horn 1 only. The horn-2 question (whether route 3 is legitimate at all, and the disposition of the already-deleted
distributedConfig) is untouched here — the fork condition did not fire, so it was never in scope. No out-of-scope findings.Generated by Claude Code