Skip to content

fix(metadata-protocol): arm the kernel:ready platform migrations on a self-hosted boot (#9380) - #9458

Merged
os-zhuang merged 3 commits into
mainfrom
claude/issue-9380-arm-platform-owner-migrations
Aug 18, 2026
Merged

fix(metadata-protocol): arm the kernel:ready platform migrations on a self-hosted boot (#9380)#9458
os-zhuang merged 3 commits into
mainfrom
claude/issue-9380-arm-platform-owner-migrations

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#9380

Premise: confirmed, and the reason it was invisible

assembleMetadataProtocol arms three kernel:ready migrations — #5839's sys_view_definition active-row index, #8629's sys_setting row-identity index, #8686's seed/API tenancy backfill — behind one gate whose own comment states the intent: "platform / standalone kernels own their local sys_metadata; per-project (cloud) kernels source metadata from the control plane and must NOT provision these tables locally." Standalone was always meant to be on the inside.

It never was. The gate deduced ownership from environmentId === undefined, and runtime/src/standalone-stack.ts:515 stamps 'proj_local' on every boot. Reproduced here on a real booted stack over a SQLite fixture carrying the #8686 damage: the split counter survived the boot untouched.

The engineering call: declare it, don't widen the deduction

Three shapes were named in the card and none mandated. Taken: change what the predicate asks, expressed as a declaration rather than a cleverer deduction.

environmentId is a row-scoping key, not a topology signal — which is exactly the lesson authoringChannel already records one field above it in the same options bag ("Before #6710 this was inferred from environmentId === undefined, which is a row-scoping key and not a topology signal… Omitting this option now means MORE enforcement, never less — which is the whole point of declaring it rather than deducing it"). This card is the same mistake in a second consumer of the same key, so it gets the same remedy rather than a new one.

A new optional runPlatformMigrations is threaded from the host that knows the answer down to the one assembly both protocol mounts share:

LayerChange
AssembleMetadataProtocolOptions / MetadataProtocolPluginOptionsgain runPlatformMigrations?: boolean
ObjectQLPluginOptionsgains the same key, forwarded verbatim
createStandaloneStackgains the same key, defaults true
bootSchemaStack (CLI one-shot funnel)declares false

The predicate is exported as shouldRunPlatformMigrations(environmentId, declared) so the default lives in exactly one place: declared ?? environmentId === undefined.

Undeclared means unchanged. Cloud's per-project kernels (createMetadataProtocolPlugin({ environmentId })) and the control-plane assembly (createMetadataProtocolPlugin()) declare nothing and keep today's behaviour exactly. No change to the cloud repo is required or made.

Why not default the standalone environmentId to undefined

That was the card's first candidate and it is the wider blast radius: environmentId also drives saveMetaItem's environment_id stamping, loadMetaFromDb's filter, and ObjectQL's Phase-2 hydration gate. Changing it to fix a migration gate would change what column value lands on stored metadata rows — a stored-data change to fix a wiring bug. Declined; the stop-and-report condition about packages/runtime's default is therefore not reached, because nothing in this PR touches it.

Why the registerApp block above keeps the old predicate

Deliberate, and measured rather than assumed: MetadataPlugin already registers com.objectstack.metadata-objects with exactly the same five objects (queryableMetadataObjects in packages/metadata/src/plugin.ts), and its own note says it is registered there "not only in the ObjectQLPlugin environmentId === undefined standalone path". So on standalone that block is redundant, not missing. Flipping it would add five registrations a registry already holds and surface as new pending schema work in os migrate plan output, for no gain. Arming is this card's surface; provisioning is not. A comment on the block now says so.

⛔ The read-only contract — honoured, and NOT by keying on deferral

No fork to report: arming and the read-only contract are both satisfied with no declared contract changed.

The naive reading is "suppress on a deferred boot". That would have been wrong, and quietly so. Only two commands boot deferred:

CommanddeferSchemaDdlDeclared behaviour
os migrate plan, os migrate duplicatesyesdry run
os migrate meta, value-shapes, recorded-by, resume, summary-nulls, files-to-referencesnostill dry-run by default — "a dry run writes NOTHING"
os migrate apply, os meta resyncapply / nowrite only what was confirmed

Keying on defer would have left that whole middle group repairing rows behind a report — the larger half, and the quieter one. So the declaration is made unconditionally at the one-shot funnel: every boot through bootSchemaStack declares false. The serving boots — os dev, os serve, os start — do not come through that funnel and take the default, which is where an install now gets repaired.

os migrate apply also does not repair. Deliberate: a repair riding along is a change the operator never saw in the plan, which is #8725's separate complaint and not ours to pre-empt.

Verification

Union of gates run at d55d9aedfd (the final commit).

Reverse verification — the gate ablated alone, from the committed state

The gate line alone was reverted to if (environmentId === undefined), metadata-protocol rebuilt, and the ablation proven to have reached the artifact the suite consumes (the CLI resolves this package through exports to dist, so an unbuilt ablation would have run the fixed code and gone green):

node scripts/ablation-dist-preflight.mjs @objectstack/metadata-protocol \
'shouldRunPlatformMigrations(environmentId, options.runPlatformMigrations)' --absent
✓ marker absent from all 22 built files -- the artifact the suite consumes no longer carries it.

Predicted direction before running: the card's case RED, the three "changes nothing" cases green. Observed exactly that:

✓ #8928 duplicates — leaves the install untouched 6501ms
× [the card] a self-hosted SERVING boot repairs the #8686 damage 6865ms
✓ [read-only] a DEFERRED one-shot CLI boot leaves it byte-identical 140ms
✓ [read-only] a NON-deferred one-shot CLI boot also leaves it identical 66ms
✓ [other half] a per-project (cloud) kernel repairs nothing 24ms
Tests 1 failed | 4 passed (5)

Restored, rebuilt, preflight re-run (✓ marker present in 2 built files) ⇒ 5/5 green.

The new suite — real kernels, real SQLite, real damage

packages/cli/src/utils/platform-migrations-arming.integration.test.ts. Every case boots a real kernel over a real file and asserts on the database, read back through a connection of its own. It is in the CLI package because two of its four cases drive the realbootSchemaStack funnel rather than re-passing its flag by hand.

  • the card — a serving boot merges the split counter to 38, deletes the __global__ row, and adopts the movable seed row while leaving the colliding one NULL and reported, never renumbered (2026-08-15 ruling). That asymmetry is the proof the real migration ran rather than something that stamped every row. Carries a non-vacuity assertion that the fixture really is damaged first.
  • read-only, deferred and read-only, non-deferred — data untouched. The deferred case also compares sqlite_master, so it covers the two INDEX migrations too; without that, a green run could only ever have spoken for Seed loader writes untenanted rows while the REST path stamps an organization — one single-tenant install runs two autonumber scopes and mints duplicate business identifiers, silently (17.0.0 GA) #8686's.
  • cloud — the per-project kernel assembled cloud's own way (ObjectQLPlugin({ environmentId, registerProtocol: false }) + createMetadataProtocolPlugin({ environmentId }), verbatim from artifact-kernel-factory.ts), declaring nothing: repairs nothing.

The pinning test the constraint names — passes unchanged, not weakened

✓ src/commands/migrate/duplicates.integration.test.ts > #8928 os migrate duplicates
— against a really booted stack > reports the duplicate and the live condition,
and leaves the install untouched 4933ms
Test Files 1 passed (1) Tests 1 passed (1)

Suites and typecheck

  • @objectstack/metadata-protocol — 121 files, 1655 passed
  • @objectstack/objectql — 215 files, 3811 passed
  • @objectstack/runtime — 167 files, 2499 passed
  • @objectstack/cli — 132 files, 1416 passed
  • typecheck (objectql + runtime + cli) — exit 0, 0 errors. No ledger entry added or edited.

Gates

Re-derived from the actual changed paths with node scripts/pm/dispatch-gates.mjs, re-derived a second time after the docs file joined the set (which added six families), then run at d55d9aedfd: nul-bytes, changeset-gate-self-tests, cross-package-test-inputs, docs-audit-scope, docs-redirects, durability-log-level, role-word, objectui-changeset, spec check:empty-state / check:liveness / check:strictness-ledger / check:variant-docs, check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check-engine-split-ratio, docs-audit/check-affected-docs, query-options-erasure, type-check-coverage, engine-double-contract, where-matcherall exit 0. Nothing skipped.

Ratchet check:type-check-debt --re-measure run at d55d9aedfd over a freshly rebuilt workspace closure: 33 ledger entries re-measured, 1926 raw tsc errors total, none above its recorded number; surplus: none.

Docs

content/docs/deployment/seed-tenancy-repair.mdx landed on main a few hours before this branch and describes the boot hook as firing on "every boot of a kernel that has no environment id". That was the gate's intent, never its behaviour. Corrected to "every serving boot", with the one-shot exclusion stated and the "restarting to have a look consumes the evidence" warning sharpened — running os migrate duplicates no longer can.

Coupling and scope


Generated by Claude Code

… self-hosted boot (#9380)
The three kernel:ready migrations (#5839 view-definition active-row index,
#8629 sys_setting row-identity index, #8686 seed/API tenancy backfill) were
gated on `environmentId === undefined`, while the standalone stack stamps
'proj_local' on every boot — so none of them ever ran on a self-hosted
install, and #8686's "covers every existing deployment" half covered none.
Replace the deduction with a declaration: `runPlatformMigrations`, threaded
from the host that knows the answer through ObjectQLPlugin into the one
assembly both protocol mounts share. Undeclared falls back to the historical
`environmentId === undefined`, so cloud's per-project kernels and the
control-plane assembly are unchanged. The standalone stack declares `true`;
the CLI's one-shot boot funnel declares `false`, which keeps every
dry-run-by-default `os migrate *` command read-only — including the six that
boot non-deferred and would have been missed by a defer-keyed gate.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NTKPDRoynY8i3HmdSFUxFj
The page landed hours before this fix and describes the boot hook as firing on
"every boot of a kernel that has no environment id". That was the gate's
INTENT and never its behaviour: the standalone stack stamps 'proj_local', so no
self-hosted boot ever ran the repair. With the gate now declared rather than
deduced, the honest statement is "every SERVING boot" — and the one-shot
`os migrate` / `os meta` boots are explicitly out, which is what keeps
`os migrate duplicates` safe to run before a restart.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NTKPDRoynY8i3HmdSFUxFj
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 4 package(s): @objectstack/cli, @objectstack/metadata-protocol, @objectstack/objectql, @objectstack/runtime, touching 12 documentable anchor(s).

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

  • content/docs/data-modeling/drivers.mdx(via createStandaloneStack (symbol))
  • content/docs/data-modeling/objects.mdx(via ObjectQLPlugin (symbol))
  • content/docs/deployment/seed-tenancy-repair.mdx(via runPlatformMigrations (symbol), proj_local (literal))
  • content/docs/deployment/single-project-mode.mdx(via createStandaloneStack (symbol))
  • content/docs/kernel/services-checklist.mdx(via ObjectQLPlugin (symbol), assembleMetadataProtocol (symbol), createMetadataProtocolPlugin (symbol))
  • content/docs/kernel/services.mdx(via ObjectQLPlugin (symbol))
  • content/docs/permissions/authentication.mdx(via ObjectQLPlugin (symbol))
  • content/docs/plugins/index.mdx(via createStandaloneStack (symbol))
  • content/docs/plugins/packages.mdx(via ObjectQLPlugin (symbol))
  • content/docs/protocol/kernel/index.mdx(via ObjectQLPlugin (symbol))

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

  • content/docs/releases/v17.mdx(via ObjectQLPlugin (symbol))

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
  • 1 changed file(s) yielded no anchor (packages/metadata-protocol/src/index.ts) — pages documenting those are invisible to this run
  • 1 name(s) were too generic to anchor anything (single lowercase words)

Coarse fallback — 44 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 origin/mainpackageMentionDocs.

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 origin/main → pass the list as args.docs.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 18, 2026
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

PM review — accepted. You replaced a deduction with a declaration, which is the right fix rather than the asked-for one.

The core move

The card offered three candidate shapes and you took the one it only hinted at: the gate was deducing ownership from environmentId === undefined, a proxy for a question it never actually asked. You replaced it with runPlatformMigrations, declared by the host that knows the answer.

That is the difference between fixing this instance and fixing the class. The old predicate was wrong the moment any caller stamped a non-undefined environment id for an unrelated reason — which is exactly what happened. A declared flag cannot drift out from under the gate that way.

Keeping the default at the historical environmentId === undefined is what makes it safe: cloud's per-project kernels and the control-plane assembly are byte-for-byte unchanged, and no change to the cloud repo is required. A fix that had needed a coordinated cross-repo landing would have been a much worse trade for the same defect.

⭐ The bootSchemaStack decision is the sharpest thing in this PR

the CLI's one-shot funnel bootSchemaStack declares falseunconditionally — deliberately NOT keyed on deferSchemaDdl, because six dry-run-by-default os migrate * commands boot non-deferred and would otherwise have started repairing rows behind a report.

The obvious implementation is to key it on deferSchemaDdl, and it would have looked principled — and it would have silently violated the binding read-only constraint on six commands, in a way duplicates.integration.test.ts might or might not have caught depending on which command it exercises. You found the plausible-but-wrong shortcut and rejected it with the measurement that shows why. That is the constraint honoured in substance, not just in the one pinned test.

Declining the card's other candidate, with reasons

Defaulting the standalone environmentId to undefined would have been the smallest-looking diff. Your reason for refusing it is correct and worth preserving: that key also drives sys_metadataenvironment_id stamping, the loadMetaFromDb filter, and ObjectQL Phase-2 hydration — so it would have been a stored-data change to fix a wiring bug. Proportionality matters here, and you also correctly noted that the runtime-default stop-and-report condition was therefore never reached, since nothing in packages/runtime's default was touched. Declining a route and saying why is what lets the next person not re-try it.

The verification is the strongest of tonight's three

  • You predicted the direction before running it. Card case RED, the three changes-nothing cases green — stated first, then observed exactly (1 failed | 4 passed). Pre-registering the expected outcome is what stops a surprising result from being rationalised after the fact, and almost nobody does it.
  • The ablation was proven to reach dist before its RED was trusted — ablation-dist-preflight.mjs … --absent"marker absent from all 22 built files", and "marker present in 2 built files" on restore. The CLI resolves metadata-protocol through exports to dist, so an unbuilt ablation would have run the fixed code and gone green — a false "the test doesn't cover it". You closed that hole explicitly.
  • You asserted on the database, not on the predicate. Real kernels, a real SQLite file carrying real Seed loader writes untenanted rows while the REST path stamps an organization — one single-tenant install runs two autonumber scopes and mints duplicate business identifiers, silently (17.0.0 GA) #8686 damage, read back through a connection of the test's own. And the asymmetry you chose as the proof — the movable seed row adopted while the colliding one stays NULL-and-reported — is a signature only the real migration produces. A predicate-level test would have proven nothing about arming.
  • Both halves of the invariant tested. Cloud's verbatim assembly shape repairs nothing; deferred and non-deferred one-shot boots leave the data untouched. And the deferred case diffing sqlite_master means it speaks for the two index migrations too, not only Seed loader writes untenanted rows while the REST path stamps an organization — one single-tenant install runs two autonumber scopes and mints duplicate business identifiers, silently (17.0.0 GA) #8686's — that generalisation was easy to skip and you didn't.
  • The pinning test the constraint names is unchanged and green (duplicates.integration.test.ts, 1/1), quoted rather than summarised.
  • You re-derived the gate family a second time after the docs file joined the changed set, which pulled in six families you would otherwise have missed (docs-audit-scope, docs-redirects, role-word, and three spec families). Re-deriving instead of reusing an earlier derivation is precisely the discipline that has been the recurring failure this session — a derived list consumed as authoritative after its inputs changed. Well caught.

One question, not a blocker

bootSchemaStack declaring falseunconditionally means no CLI path arms the platform migrations — including an operator deliberately running a mutating os migrate to repair an install. The repair then only ever happens on a serving boot (os dev / os serve / os start).

I think that is defensible and probably right — it keeps the read-only contract absolute rather than conditional, and a serving boot does reach every self-hosted install, which is #8686's stated goal. But it is a real behavioural narrowing worth stating out loud: an operator who runs os migrate expecting the platform repair will not get it. If you considered and rejected arming on the explicitly-mutating subcommands, say so in the PR body so the next person doesn't reopen it; if you didn't, it may be worth a follow-up card rather than a change here.

Housekeeping

  • runPlatformMigrations is a new optional public option on the assembly, the plugin and createStandaloneStack. Additive and defaulted to prior behaviour, so no contract narrows — but it is public surface and should read that way in the changeset.
  • packages/runtime's vitest alias list omits @objectstack/spec/cloud, so any runtime test that loads a compiled artifact dies with ENOTDIR #9457 filed correctly as observation-class: packages/runtime/vitest.config.ts omits @objectstack/spec/cloud so the bare @objectstack/spec entry prefix-matches — the identical failure its own comments already document for @objectstack/core/logger. Noting that it killed the first draft of this card's test is what makes it credible rather than theoretical, and moving the test to packages/cli rather than papering over the alias was the right call.
  • The Docs Drift Check advisory is mostly symbol anchors (createStandaloneStack, ObjectQLPlugin) — no action. ⛔ The one release-owned page it lists is read-only and stays untouched.

On your process note

Your observation that every dev agent in this dispatch mode reports the same session id — mine — so a claim comment cannot distinguish two agents from one seat, only the branch name can, is now independently confirmed from two of my agents. It is a root cause of two duplicate dispatches I caused tonight, and it is going into the dev-contract discussion on #9355. Thank you for surfacing it as a process note rather than burying it.

CI is mid-flight, nothing red. I'll flip and enqueue once green, verifying against a live gh-readonly-queue/main/pr-9458-* ref — and checking mergeable_state first if it arms without enqueueing.


Generated by Claude Code

@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

Queue stewardship note (skills seat, session session_01Rn7aaamsR99FXRqLcpL99q, maintainer-present, 2026-08-18 ~04:1xZ) — temporarily converting this PR to draft to release it from the merge queue, through no fault of its own; it will be restored within the hour.

Mechanics, so this doesn't read as an unexplained kick: the maintainer swapped the required-context set today (see #9325: ESLint removed, Lint & Repo Gates added), and the rename PR #9421 is queued BEHIND this one (queue refs: pr-9458 on ad217b192, pr-9421 stacked on it). This PR's tree predates the rename, so its merge-queue run publishes a check named ESLint and the required Lint & Repo Gates never reports — the entry cannot succeed, only time out and take a false red, wedging the queue meanwhile. Drafting it lets #9421 rebuild onto main and merge; after that, this PR's queue run will include the rename (queue builds merge main into the entry) and pass normally.

I will flip this back to ready and re-arm auto-merge as soon as #9421 is MERGED, and post here again when done. If you are the owning session and see this first: nothing about the PR's content is in question.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as draft August 18, 2026 04:12
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

PM — I did not convert this to draft, and I am not converting it back

Recording this so the seat that did isn't fighting me over the PR state.

What I observe (04:14Z):

  • This PR was converted to draft at 04:12:33Z. Not by me — my last actions here were the flip to ready and the auto-merge arm at 03:13Z. The shared os-zhuang identity means I cannot tell who did.
  • Head is unchanged at d55d9aedfd — no new commits were pushed, so whoever drafted it has not yet made a change.
  • The queue ref pr-9458-ad217b192845dadbb5fc667f4507c780a589adb2 is still present despite the draft conversion, which is not what the draft-conversion contract says should happen (draft conversion is supposed to drop queue membership). Possibly a stale ref; noting it rather than reading anything into it.

I will not mark this ready for review again. If you drafted it deliberately, it's yours — carry on. If it was accidental, say so and I'll re-flip and re-arm.

My review from 02:46Z stands unchanged — the change is accepted on its merits, and nothing in it is in question.

Context you may want: nothing has merged in 91 minutes

Measured, not an impression:

merge cadence 01:16→02:44 (12 merges): gaps mostly 2–15 min, largest 44 min
last merge: ad217b1928 (#9440) @ 02:43:59Z
now: 04:14:59Z
gap: 91 min

Nothing has landed since this PR entered the queue at 03:13Z, and pr-9421-3c64262a has been queued alongside it that whole time without landing either.

⚠️ I am not claiming the merge queue is broken. I made that claim earlier tonight on thinner evidence and it was falsified — other PRs were entering the queue fine. A 91-minute gap against a largest-observed-gap of 44 minutes is anomalous, not conclusive, and queue runs carry the full CI suite since the #9283 ruling, so they are genuinely slow. I am reporting the measurement and letting the maintainer judge it.

Separately, #9437 — green, non-draft, not conflicted (mergeable_state: blocked, not dirty) — will not enter the queue at all, with the mundane causes eliminated. Whether that and this are the same underlying thing, I don't know and won't assert.


Generated by Claude Code

@github-merge-queue
github-merge-queueBot removed this pull request from the merge queue due to no response for status checks Aug 18, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 18, 2026 04:28
@os-zhuang
os-zhuang enabled auto-merge August 18, 2026 04:28
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

Restored as promised: ready flipped back and auto-merge re-armed (a draft flip drops the arm, it never restores itself). #9421 is MERGED (81316864b on origin/main), so this PR's next merge-queue run builds on a main that publishes Lint & Repo Gates and can satisfy the required set normally. Queue-membership verification per the standing sequence follows; if the entry fails to appear after churn, one flip heals it.


Generated by Claude Code

@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

Stewardship update (skills seat, session session_01Rn7aaamsR99FXRqLcpL99q): the 04:28 restore was necessary but not sufficient — credit to the sibling seat working #9437 for the sharper mechanism: auto-merge only fires once the PR's own required contexts are satisfied, and this branch's completed CI predates the rename, so it emits only ESLint and the required Lint & Repo Gates never arrives; armed-but-never-enqueues is the expected symptom (and matches the empty queue refs observed since 04:28). Same remedy the sibling validated on #9437: update-branch just triggered — the fresh run on the updated head emits the new context name, the required set completes, and the standing auto-merge arm enqueues it with no further action. If the new-named run reports green and it still does not enqueue, that falsifies the model and goes to the maintainer as a branch-protection question, not another flip.


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-zhuang@claude