Uh oh!
There was an error while loading. Please reload this page.
fix(cli): read the upgrade advisory off manifest.engines.protocol, retiring specVersion from the stack config's CLI vocabulary - #14260
Conversation
…3860) Retire `specVersion` from the stack config's CLI vocabulary. The three commands that print the migration-guide advisory (validate / doctor / compile) read `manifest.specVersion`, a key `ManifestSchema` does not declare and silently drops, so the advisory has been dead for stack configs its whole life. It now reads `manifest.engines.protocol` — the declared, parsed and boot-enforced axis — and delegates the range verdict to `checkProtocolCompat`, the platform's single reader of that axis, rather than opening a second opinion on the grammar. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YPiiDdw96RGS25WLctCQP
…13860) check:test-source-alias flagged the new advisory test as importing the protocol handshake through `exports` -> dist. The advisory is a thin direction check over `checkProtocolCompat`, so without the alias the range grammar it pins would be whatever was last compiled — green against a stale artifact with nothing in the output saying so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YPiiDdw96RGS25WLctCQP
📓 Docs Drift CheckThis PR changes 1 package(s): 21 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin b6527c22033bc751029e863081eee6abfd1c9948 && git checkout b6527c22033bc751029e863081eee6abfd1c9948
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 035951faf99c42c258d470102da7346f309f7346 9e3745deda9c5b4bc3155be641cfa5b28d19b7dd && git checkout -B drift-repro 035951faf99c42c258d470102da7346f309f7346 && git merge --no-ff 9e3745deda9c5b4bc3155be641cfa5b28d19b7dd
node scripts/docs-audit/affected-docs.mjs --json 035951faf99c42c258d470102da7346f309f7346
|
…#13860) Renaming doctor's row from `Platform spec` to `Platform protocol` left two consumers behind, and only a repo-wide grep for the old string found them — the suites the rename touched were the wrong population to search. - `doctor-config-env-overlay.test.ts` pinned the old label as a LIVENESS proxy ("the config checks did not merely stop failing — they RAN"), not as a label check. Re-aimed at the new label, intent preserved: the row prints on either branch, so its presence still proves the check executed whatever it concluded. A comment now records that these strings are row labels that must track `doctor.ts` through a rename rather than be relaxed. - `configLoadFailureCheck`'s operator-visible sentence enumerated the skipped config-aware checks as "spec version", naming a row that no longer exists. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YPiiDdw96RGS25WLctCQP
Uh oh!
There was an error while loading. Please reload this page.
Fixes#13860
Retires
specVersionfrom the stack config's CLI vocabulary and points the upgrade advisory atmanifest.engines.protocol— the axis that is declared, parsed and enforced at boot. Implements the project-director's ruling (maintainer verbatim on the batch: 「同意」), quoted untranslated because rewriting a ruling is re-ruling it:Option A is closed and untouched: zero edits to
packages/spec(verified against the merge base, not asserted), andcloud/template-manifest.zod.tskeeps its ownspecVersiondeliberately.What moved
packages/cli/src/utils/spec-version.tsbecomesprotocol-version-gap.ts;checkSpecVersionGapbecomescheckProtocolVersionGap. All three call sites move in the same stroke —validate.ts,doctor.ts,compile.ts— anddoctor's printed rows now nameengines.protocolinstead of a key nobody can author. None of these symbols is on the package's export surface (exportspublishes.=src/index.ts, which re-exports command classes only, plus./console), so the rename is internal.Clause 4 is the load-bearing half, and it is pinned with evidence
The advisory has been dead for stack configs its whole life:
ManifestSchemadeclares nospecVersionand is not.strict(), so an authored one was accepted and dropped, and the check could only ever fire for a manifest carrying a key the schema does not offer. Relocating the read without demonstrating that it now fires would not discharge this card, so the positive example is pinned throughManifestSchema.parserather than on a hand-built literal — a literal would only prove the function works on input the platform never produces, which is the state being repaired:engines: { protocol: '^16' }survives the parse and FIRES against installed17.2.0, pointing at the v17 guide;engines: { protocol: '^17' }, is silent against17.2.0;specVersionwritten alongside does not survive the parse, and the retired key raises nothing.No pin was deleted to get green. The old suite's
checkSpecVersionGap({ specVersion: '^12.0.0' }, '14.7.0')pin was re-aimed into an assertion that the retired axis is now inert — a deleted pin and a passing one are not the same evidence.Both directions were then proven non-vacuous by ablation. Each mutation was confirmed on disk before the run (hash moved, injected-text count 1, removed-text count 0) and each restore was proven by
git diff HEADempty plus a byte-identical blob hash, not by an exit code:specVersion!==to===)The renamed label owed a repo-wide sweep, and it had two consumers
Renaming
doctor's row fromPlatform spectoPlatform protocolbroke a consumer that lived outside every suite this change touched — CI caught it, and the lesson is that a renamed user-visible string owes a grep for the old string across the whole tree, not a grep of the suites you expect to care. Swept with a positive control (Platform protocol, 2 hits) so the zero elsewhere is a reading rather than a broken probe. Two consumers found, two fixed:doctor-config-env-overlay.test.tspinned the old label as a liveness proxy — its own comment reads "The config checks did not merely stop failing — they RAN." Re-aimed at the new label, intent preserved: the row prints on either branch, so its presence still proves the check executed whatever it concluded. Not deleted, not weakened to something that would pass if the config checks never ran. A comment now records that these strings are row labels that must trackdoctor.tsthrough a rename.configLoadFailureCheck's operator-visible sentence enumerated the skipped config-aware checks as "spec version" — naming a row that no longer exists under that name. Renamed in the same stroke and re-wrapped to the surrounding gutter width.content/docs/**was checked for the same strings and has none, so no documentation describes a row that no longer exists.Why the verdict is delegated, not re-implemented
The range is judged by
checkProtocolCompatfrom@objectstack/metadata-core(added as a direct dependency; it was already in the install graph). That module is the platform's single reader of this axis and owns both the source priority (engines.protocolthenengines.platformthen legacyengine.objectstack) and the range grammar. Its own header records whyresolveDeclaredRangewas exported: two readers with two priority orders would be the "two opinions" defect. A private leading-integer parse would have been the third, and it would disagree exactly where it matters —>=15 <18targets 15 but admits 17, so a naive reading advises an upgrade against a range that already covers the installed platform. Both that case and its excluding twin (>=15 <17) are pinned. The advisory names the key it actually read, so an author is never told to bump a key they did not write.Comparing a protocol range against the
@objectstack/specresolved from the app'snode_modulesis sound becausePROTOCOL_VERSIONis held in lockstep with that package's major —protocol-version.test.tsfails on drift — so the two are the same integer and thedocs/releases/vMAJORlink stays correct. That lockstep is what makes this relocation safe at all; the mismatch it prevents is what #9264 was about.One narrowing guard is deliberate and is not a tolerant fallback:
doctorhands over an unvalidatednormalizeStackInputresult, soengines.protocolcan be any JSON value at that point, andresolveDeclaredRangecalls.trim()on it. The manifest slice is narrowed tostring | undefinedbefore the handshake sees it, so the failure mode stays "no advisory" rather than a throw. Types only — the range itself is still interpreted exclusively by the handshake.No command's rejection behaviour changes (clause ② stays
no)Verified in code rather than assumed, because a rejection change would be a clause-② flip:
validate.tsprints the advisory after the--strictexit and keeps it out of both faces'warningslist — it is explicitly never gated by--strict, unchanged;doctor.ts'shasWarningsnever exits non-zero; onlyhasErrorsdoes;compile.tsprints it after the artifact is already written.Accept sets and exit codes are identical. What changes is that the advisory now fires, and
os doctorwill summarise such a run as "functional but has some warnings" rather than "healthy". The--jsonpayload keeps the published keyspecVersionGapwith its value shape unchanged: the axis behind it moved, but that is a machine face with pinned consumers in three e2e suites, and renaming it is a break nobody asked for and no ruling covers. That rename went to the decision box as #14261; A stands for this card. The three suites that pin the key are green.Verification
Everything below ran at
9e3745deda, the final commit — the union was re-derived after the last edit and again after mergingorigin/main, withnode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackon a non-stale tree, and reconciled against the earlier derivations withcomm -23/comm -13: no additions, no removals, across all three. Exit codes captured before any pipe; verdicts quoted from each gate's own line.24 gates green:
changeset-gate-self-tests,cross-package-test-inputs,doc-authoring,docs-image-tag,dual-build-cjs-loads,engine-double-contract,i18n,i18n-coverage,logger-receiver-detach,objectql-double-limit,objectui-changeset,override-consistency,page-declaration-shape,pm-half-states,published-files,query-options-erasure,service-providers,slot-lookup,test-source-alias,turbo-task-graph,type-check-coverage,type-check-debt,type-source-resolution,where-matcher. The ratchet's own line: 27 ledger entries re-measured, none above its recorded number.Gates that read built output (
i18n,i18n-coverage,dual-build-cjs-loads,type-check-debt) first returned PREREQUISITE NOT MET — each states outright that nothing was measured. Those were read as NOT MEASURED, never as passes; their prerequisite closures were built (dual-buildnames its missing dists one round at a time, so it took three) and all four then ran green.check:test-source-aliaswas a genuine red earlier in the branch — the new test imported the handshake throughexportstodist, which would have made it a verdict about build state, green against a stale artifact with nothing saying so. Fixed with an anchoredresolve.aliasentry, not by widening the shrink-only registry.Tests:
tsc --noEmitclean over@objectstack/cli, and--listFilesconfirms both the new source and the new test file are inside the program (this package'stsconfigincludessrc, so the typecheck really does cover them — counted, not assumed). The wholedoctorfamily plus the advisory unit suite: 12 files, 173 tests, all passing, including the exact test CI failed on. The threespecVersionGapparity e2e suites are green too.Repo-wide
pnpm lintis left to CI; the local run is a declared narrowing, with all three pieces of evidence rather than a claim:File ignored because no matching configuration was suppliedand lints the rest;--format json: 7 files linted, 0 errors, 0 warnings;eslint.config.mjswhich never enables type-aware linting for any file (noparserOptions.project, no typed rules) — stated and positive-control-measured in that config's own header — so this diff cannot move the verdict on a file it does not touch.Findings
The residual hazard — an authored
manifest.specVersionis still accepted and dropped in silence — is already covered by open #14192 ("ManifestSchemastrips unknown keys silently"), which is the general form of it (anamespacetypo insidemanifest:parses green). No duplicate filed. WhetherspecVersionspecifically warrants aretiredKey()tombstone is a rejection change and therefore not this card's to make.