Skip to content

fix(cli): os migrate meta names the protocol, not a package version - #16058

Queued
os-litant wants to merge 1 commit into
mainfrom
claude/issue-15585-migrate-meta-protocol-label
Queued

fix(cli): os migrate meta names the protocol, not a package version#16058
os-litant wants to merge 1 commit into
mainfrom
claude/issue-15585-migrate-meta-protocol-label

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Part of #15585 — this lands option A only. The card's --json half (option C) is deliberately left open; the section "What is NOT in this PR" says exactly what remains and why, so the card must stay open after this merges.

The defect, driven

PROTOCOL_VERSION is the protocol major padded to a semver ('17.0.0', packages/spec/src/kernel/protocol-version.ts:18). It is not, and never tracks, the installed package version. It was printed under the word runtime, as a bare semver, beside the real package versions of the same upgrade session.

This repo is itself at @objectstack/spec 17.3.0 / @objectstack/cli 17.3.0, which is the reporter's exact scenario. Driving the real CLI (bin/run-dev.js, NO_COLOR=1) over a minimal stack config:

Before

◆ Migrate · meta
────────────────────────────────────────
  → Loading configuration…
  → Replaying chain: protocol 17 → 17…
  ℹ Config: /…/objectstack.config.ts
  ℹ Chain:  protocol 17 → 17 (runtime 17.0.0)

  ✓ Nothing to migrate — the metadata is already canonical for this range.

After

◆ Migrate · meta
────────────────────────────────────────
  → Loading configuration…
  → Replaying chain: protocol 17 → 17…
  ℹ Config: /…/objectstack.config.ts
  ℹ Chain:  protocol 17 → 17 (this runtime implements protocol 17)

  ✓ Nothing to migrate — the metadata is already canonical for this range.

Why relabelled and not dropped

The card offers dropping the parenthetical, since the line already says protocol 17 → 17. Driving the other case shows the parenthetical carries a fact nothing else on screen does — when --to stops below this build's major it is the only place the operator is told where the runtime actually stands:

before:  ℹ Chain:  protocol 16 → 16 (runtime 17.0.0)
after:   ℹ Chain:  protocol 16 → 16 (this runtime implements protocol 17)

So the value stays; the label and the semver form are what changed. Both halves mattered: a line that merely stopped saying "runtime" would still print a padded semver in a version position.

The sweep — every other site that prints PROTOCOL_VERSION

A one-site fix that leaves siblings lying would be worse than reporting the set, so the whole set was enumerated before touching one (grep over the tree, excluding node_modules and dist). Every other site already labels the value correctly:

Site Spelling Verdict
packages/cli/src/commands/migrate/meta.ts:353 (runtime ${PROTOCOL_VERSION}) the defect — fixed here
packages/cli/src/commands/migrate/meta.ts:330 runtime: PROTOCOL_VERSION (--json) same ambiguity, machine face — deliberately not moved, see below
packages/cli/src/commands/migrate/meta.ts:201 Target protocol major (defaults to this runtime's, ${PROTOCOL_MAJOR}) correct — an integer major in a "protocol major" sentence, not a semver in a version position
packages/metadata-core/src/protocol-handshake.ts:225,285 message …but this runtime is protocol ${runtimeVersion} correct — the prose says protocol. The machine field is named runtimeVersion, which is reported as a finding rather than moved: it is a different shipped surface (the OS_PROTOCOL_INCOMPATIBLE diagnostic) and its own contract move
packages/metadata-protocol/src/protocol.ts:15610 protocol: PROTOCOL_VERSION correct
packages/spec/scripts/build-spec-changes.ts:108 protocolVersion: PROTOCOL_VERSION correct
packages/spec/scripts/build-upgrade-guide.ts:40 Current protocol: **17.0.0** correct

What is NOT in this PR

The --json runtime key is untouched, on purpose. It is a machine-readable key on a published payload; renaming it is a contract change owing a reader census and a deprecation window, and the card's triage ruled A and C independent with an explicit instruction not to bundle them. The payload is byte-identical across this change — same key set, same values (duration excluded, being a timer):

keys before: from,to,runtime,applied,todos,specChanges,schemaValid,dataMigrations,duration
keys after:  from,to,runtime,applied,todos,specChanges,schemaValid,dataMigrations,duration
payload identical (duration excluded): true

A reader census for that key found no consumer in this tree: nothing reads .runtime off this payload, and the published skills/objectstack-upgrade/SKILL.md documents --json without naming the field. That is a measurement offered to whoever decides C, not a decision.

To keep the deferral honest rather than silent, a comment now sits on the emit site, and one of the new e2e cases pins the key's current value — so a future rename fails a test instead of shipping quietly.

Option B (printing the real package version too) stays out: it depends on @objectstack/cli/package.json being resolvable, which is another card's work. Out of scope: #15325.

Tests

packages/cli/test/migrate-meta.e2e.test.ts — four new cases in a new describe, driving the real CLI process, not a string in a fixture:

  1. the chain line names this build's protocol major, in majors;
  2. the human output contains no padded protocol semver under any label (PROTOCOL_VERSION absent, and no runtime N.N.N anywhere);
  3. the fact survives when --to stops below this build's major;
  4. the --json runtime key is exactly as published.

Every case derives its expectations from PROTOCOL_MAJOR / PROTOCOL_VERSION rather than hard-coding 17, so they follow the next major bump.

Suite: pnpm --filter @objectstack/cli exec vitest run --maxWorkers=2 test/migrate-meta.e2e.test.tsTest Files 1 passed (1) · Tests 18 passed (18).

Ablation. The CLI is driven from src/ (bin/run-dev.js imports ../src/…), so no rebuild sits in this loop — stated because an ablation over a dist/-resolved subject that skipped the rebuild would stay green and prove nothing. With the fix committed, the defective line was put back on disk and the mutation confirmed there before measuring (injected-line count 1, removed-line count 0, blob hash moved 058a5bb7…43064a9a…), not by trusting the editor's exit code:

× names this build's protocol major, in majors
× prints no padded protocol semver in the human output, under any label
× still says where the runtime stands when --to stops below this build's major
Tests  3 failed | 1 passed | 14 skipped (18)

The one that stayed green is case 4 — the --json pin, which the mutation did not touch. That is the discrimination working in both directions rather than a blanket red. Restore was proven by observed state, not by an exit code: git diff HEAD empty, worktree blob back to 058a5bb7…, fix line present, defect line absent.

Gates

Union derived mechanically at the delivered commit — node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands — and asserted against its own reconciliation line: 56 families claimed, 56 command lines harvested, 56 green. Exit codes captured before any pipe (cmd > log 2>&1; EXIT=$?).

Four came back NOT MEASURED on the first pass for want of built output — check:dual-build-cjs-loads and check:i18n / check:i18n-coverage (exit 3, PREREQUISITE NOT MET) and check:i18n-walk-parity (its own verdict line reads PREREQUISITE NOT MET). All four were then built for and re-run green; none was reported as a pass while unmeasured.

The Artifact rosters block (37 families, outside that total by the tool's design) was run separately: 35 green, 2 not measured —

  • node scripts/check-partof-closing-keyword.mjs and node scripts/check-single-claim-paths.mjs returned exit 2, NOT WIRED (no PR_BODY / PR_NUMBER outside CI). Their pnpm spellings are green above but grade only the checkers' own fixtures — measured, not assumed: both package.json entries resolve to --self-test alone. The body-judging form of the first was then run locally against this exact body with PR_BODY set, and passes.
  • pnpm --filter @objectstack/spec run check:react-declaration-parity exits 1 with MANIFEST is not set … This gate did NOT run. It needs an objectui-derived sdui.manifest.json and a browser, is an on-demand gate rather than a CI job, and this diff does not move the .objectui-sha pin.

Clause ② — declared per limb, from the delivered diff

Public-surface limb: NO. The diff adds no key to any published payload (the --json payload is byte-identical, shown above), touches nothing under packages/spec/src/**, adds and changes no export, and leaves the command's flags, args and exit codes exactly as they were; the only shipped bytes that move are one human-readable diagnostic line on stdout, which no consumer in the tree parses.

Conformance limb: NO. No input class is re-selected between two already-published verdicts: the same configs load, the schema verdict is computed by untouched code, the support-floor refusal and every exit code are unchanged, and the whole change is the rendering of one informational line after the verdict has already been reached.

Out of scope, filed rather than acted on

  • docs(releases): the v17 page's upgrade bullet asserts os migrate meta "also prints runtime 17.0.0 on a 17.3.0 install" — a sentence the CLI repair invalidates, in a file a code PR may not touch #16056content/docs/releases/v17.mdx:5363 asserts this command "also prints runtime 17.0.0 on a 17.3.0 install", a sentence this repair invalidates. content/docs/releases/ may not be edited by a code PR (AGENTS.md, Documentation Guardrails); a code PR's input to release notes is its changeset, and this PR's changeset carries the corrected line. Whether that sentence is stale guidance or a historical measurement is a judgement for the release process, which is why it is a card and not an edit.
  • runtimeVersion on the OS_PROTOCOL_INCOMPATIBLE diagnostic (packages/metadata-core/src/protocol-handshake.ts) carries the same padded protocol semver under a version-ish machine name. Its prose says protocol, so the human channel is unambiguous; the machine field is a separate shipped surface and a separate contract move. Reported here, not touched.

Changeset

.changeset/migrate-meta-chain-line-protocol-label.md@objectstack/cli: patch.

🤖 Generated with Claude Code

https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N


Generated by Claude Code

The chain line ended `(runtime 17.0.0)`. That value is `PROTOCOL_VERSION` --
the protocol major padded to a semver -- and it never tracks the installed
package version. Printed as a bare semver under the word "runtime", beside the
real package versions of the same upgrade session, it read as "your runtime is
17.0.0": an apparent downgrade on a 17.3.0 install.

The value was never wrong; the label and the semver form were. The line now
states the fact in the protocol's own units -- `(this runtime implements
protocol 17)` -- relabelled rather than dropped, because with `--to` stopping
below this build's major it is the only place the operator learns where the
runtime actually stands.

The `--json` `runtime` key is deliberately left as published: a machine-readable
key on a shipped payload owes a reader census and a deprecation window before it
moves. A new e2e pin drives the real CLI over both halves and asserts that key's
current value, so the contract move cannot happen silently.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 2 documentable anchor(s).

11 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/flows.mdx (via os migrate meta (command, read off packages/cli/src/commands/migrate/meta.ts))
  • content/docs/automation/hook-bodies.mdx (via os migrate meta (command, read off packages/cli/src/commands/migrate/meta.ts))
  • content/docs/data-modeling/fields.mdx (via os migrate meta (command, read off packages/cli/src/commands/migrate/meta.ts))
  • content/docs/data-modeling/objects.mdx (via os migrate meta (command, read off packages/cli/src/commands/migrate/meta.ts))
  • content/docs/data-modeling/queries.mdx (via os migrate meta (command, read off packages/cli/src/commands/migrate/meta.ts))
  • content/docs/deployment/cli.mdx (via os migrate meta (command, read off packages/cli/src/commands/migrate/meta.ts))
  • content/docs/deployment/index.mdx (via os migrate meta (command, read off packages/cli/src/commands/migrate/meta.ts))
  • content/docs/protocol/objectql/query-syntax.mdx (via os migrate meta (command, read off packages/cli/src/commands/migrate/meta.ts))
  • content/docs/protocol/objectui/actions.mdx (via os migrate meta (command, read off packages/cli/src/commands/migrate/meta.ts))
  • content/docs/ui/apps.mdx (via os migrate meta (command, read off packages/cli/src/commands/migrate/meta.ts))
  • content/docs/upgrading.mdx (via os migrate meta (command, read off packages/cli/src/commands/migrate/meta.ts))

2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v12.mdx (via os migrate meta (command, read off packages/cli/src/commands/migrate/meta.ts))
  • content/docs/releases/v17.mdx (via os migrate meta (command, read off packages/cli/src/commands/migrate/meta.ts))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 53cbad9f75572600ef43bb2a18071633fd6c0f68packageMentionDocs.

Which tree this was computed on

This run read content/docs from bab908eaa4d010d83d169ba87defa5296237ede0 — the merge of head e702dfad69a846be9774dcafa192cdbe6bef2ed4 into base 53cbad9f75572600ef43bb2a18071633fd6c0f68, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin bab908eaa4d010d83d169ba87defa5296237ede0 && git checkout bab908eaa4d010d83d169ba87defa5296237ede0
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 53cbad9f75572600ef43bb2a18071633fd6c0f68 e702dfad69a846be9774dcafa192cdbe6bef2ed4 && git checkout -B drift-repro 53cbad9f75572600ef43bb2a18071633fd6c0f68 && git merge --no-ff e702dfad69a846be9774dcafa192cdbe6bef2ed4

node scripts/docs-audit/affected-docs.mjs --json 53cbad9f75572600ef43bb2a18071633fd6c0f68

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 53cbad9f75572600ef43bb2a18071633fd6c0f68 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator Author

Seat ruling — clause ② is NO on both limbs, and one correction about how that grade was reached

Recording this so a later reader can tell a contract review that was not owed from one that was skipped, and so the reasoning is auditable.

Mechanical floor: NO

The --json payload is byte-identical across the change — same key set, same order, same values, duration excluded as a timer — measured by driving the real command on both sides rather than by reading the diff. That forecloses the "new key on a published payload" trigger in the strongest available form: not merely no key added but no byte moved. None of the three delivered paths is under packages/spec/src/**; packages/spec/src/kernel/protocol-version.ts was read and quoted, never modified.

Conformance limb: NO, on the narrow reading, and here is why the narrow reading is the right one

The changed segment interpolates build constants only, so its bytes are identical for every input the command can be given. The input-dependent prefix of the same line — Chain: protocol <from> → <to> — is byte-identical before and after. A string that is the same for every input cannot partition inputs and therefore carries no verdict about any of them; there is no pair of published verdicts for an input class to be re-selected between.

The wider reading — that the exact output bytes are themselves the verdict — grades every diagnostic-wording change in the tree as a contract move. A limb that answers YES to everything does not discriminate, and a rule that cannot fail to fire tells a reviewer nothing. That is the same principle this seat applies to controls, and it settles the reading here.

One support the dev's own argument did not name: this repository already governs console-output drift through a different mechanism — the CLI transcript and examples gates ("CLI transcripts in content/docs match the registry they quote", "CLI examples match the documented block"). Console prose has its own enforcement path; it is not clause ②'s job, and treating it as clause ②'s job would duplicate one gate inside another.

⚠️ Residue, stated rather than buried: the dev's consumer sweep (grep for Chain: and the old spelling across skills/, content/, docs/, READMEs and the source tree, finding only a hand-filled report template) can see this tree only — the same limitation it correctly flagged for the .runtime census on #15585 but did not carry into this limb. An out-of-tree operator script parsing runtime \d+\.\d+\.\d+ off stdout would break. The seat still grades NO, because --json exists precisely so that machine consumers do not have to parse prose, and that face did not move by a single byte.

The correction: do not optimise a diff to keep a limb NO

From the dev's own declaration, on what would have made the limb YES:

A conditional parenthetical. I considered printing it only when toMajor !== PROTOCOL_MAJOR, which is arguably better UX. I rejected it, and this limb is why: it would have split inputs that today share one output shape into two, which is re-selecting an input class on a shipped face. The unconditional form was chosen partly to keep this limb NO.

⛔ That is the wrong direction of optimisation, and naming it is partly this seat's job because the dispatch is what made clause ② feel like a cost. Clause ② is a review trigger, not a prohibition. Grading YES buys one review round; it does not block, penalise, or reflect badly on a diff. Trading away the better output to avoid that round optimises the wrong quantity, and if that instinct generalises it will quietly bias every dispatch on this lane toward the shape that dodges review rather than the shape that is right.

The unconditional form is kept — but on its merits, not on that reasoning. It is defensible on the measurement the dev itself took: driving --from 16 --to 16 shows this parenthetical is the only place an operator is told where the runtime stands when --to stops below this build's major, so always showing it is the more informative choice. Had the conditional form been better, the correct move would have been to write it and accept the review round.

The rest, verified

Both halves of the defect are repaired: the word runtime no longer labels a version and the padded semver is gone from the version position — a line that merely stopped saying runtime would still be showing a semver where a reader expects a package version. All five other PROTOCOL_VERSION print sites were swept and are correctly labelled, so no sibling was left lying. The ablation discriminates rather than reddening in bulk: three of four new cases red on exactly the human-line assertions, and the fourth — the --json pin — correctly stayed green, because the mutation did not touch the JSON path.

needs:contract-review is therefore not applied to this PR or to card #15585, and none is being stripped. The card stays open behind the maintainer's decision on the --json runtime key.


Generated by Claude Code

@os-litant
os-litant marked this pull request as ready for review September 5, 2026 22:56
@os-litant
os-litant enabled auto-merge September 5, 2026 22:56
@os-litant
os-litant added this pull request to the merge queue Sep 5, 2026
Any commits made after this event will not be merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants