Repository navigation
fix(cli): os migrate plan / apply run no app onEnable and no post-declaration host hooks - #21138
Conversation
…ration hooks os migrate plan / apply compose host code for what it declares. The config's onEnable is withheld by the AppPlugin the composition builds (skipOnEnable), and a host plugin's init() gets a context that does not register kernel:bootstrapped / kernel:listening hooks. The plan's notes name what was withheld. Claude-Session: https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB Co-authored-by: Claude <noreply@anthropic.com>
…hook Runtime: AppPlugin skipOnEnable withholds onEnable wherever it resolves. CLI unit: composeForDeclarations' init context declines the two post-declaration phases; the composed app carries skipOnEnable. CLI integration: the write-guard pins move host hooks to kernel:ready and keep the guard's phase-agnostic property on an unwrapped writer; an app-crm-shaped fixture prints zero DATABASE_ERROR on a migrated and on an absent file, with a served-composition positive control and the apply flush/coverage control. Claude-Session: https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB Co-authored-by: Claude <noreply@anthropic.com>
…hooks Claude-Session: https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB Co-authored-by: Claude <noreply@anthropic.com>
…an-no-app-hooks Claude-Session: https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB Co-authored-by: Claude <noreply@anthropic.com>
…e 21054 pin check:test-source-alias: a dynamic import of an unaliased dependency inside a test body pays its first transform inside a clocked window. Claude-Session: https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 2 package(s): 9 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 42 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 fb98d75bc4ecc8e33b9ff36c48912ec239472f8a && git checkout fb98d75bc4ecc8e33b9ff36c48912ec239472f8a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 665cab338f3f8c78c74e773a10434ed4ea2d12c6 afc44ba5bf59bd98f4a78050e48b751cb5f03835 && git checkout -B drift-repro 665cab338f3f8c78c74e773a10434ed4ea2d12c6 && git merge --no-ff afc44ba5bf59bd98f4a78050e48b751cb5f03835
node scripts/docs-audit/affected-docs.mjs --json 665cab338f3f8c78c74e773a10434ed4ea2d12c6
|
Contract reviewServed-tier: This is the record of record for PR #21138 at Inputs, and nothing else: card #21054's body and its three comments; #13332 (its body, triage Check-runs on Mergeability: ① Derived judgments(a) The two doors are closed where each enters, for every app, by phase and not by plugin name; the write guard is unchanged in force; the declined set is exactly the claim's; the stop clause's evidence is true at the head — RIGHT. Door 1, the config's
Door 2, a host plugin's
(b) The #13332 reversal: every inversion is compelled by the grade, the three writer moves keep their pins non-vacuous, and the guard's phase-agnostic property is pinned by a writer the composition does not wrap — RIGHT, with the overridden constraint named.
(c) The pins red without the fix as reported, the positive controls discriminate, and the app-crm-shaped fixture is an acceptable stand-in — RIGHT.
(d) The plan's output: one more entry in an existing open-ended notes array; not a contract change of the machine output — RIGHT.
Surface inventory: no route, flag, exit code or JSON key changes; one new optional constructor option and one new public getter on ② Semver levelThe PR body's line 2 and the changeset read
③ Boundary flags
Implemented-by: VERDICT: FAIL |
…ening) AppPlugin, exported from @objectstack/runtime's root, gains the optional skipOnEnable constructor option and the onEnableWithheld getter: an additive widening of a published surface, which takes at least minor. Contract review record 5928867906. Claude-Session: https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: This is the record of record for PR #21138 at Inputs: the FAIL record What moved, from the diff. Check-runs on Mergeability: ① Derived judgmentsUnchanged, and carried from ② Semver levelThe PR body's line 2 reads
③ Boundary flags
Implemented-by: VERDICT: PASS |
Fixes #21054
Clause-②: yes (widening)
What was wrong
os migrate planandos migrate applyboot the host's stack to read what it declares. That boot also ran host code that has nothing to do with declarations:onEnable, which runs from theAppPluginthe migrate composition builds out ofobjectstack.config.ts. ThatAppPluginis not wrapped bycomposeForDeclarations.kernel:bootstrapped/kernel:listeninghook that a host plugin registers frominit().composeForDeclarationssuppressedstart()and nothing else.examples/app-crm'sonEnablehookskernel:bootstrappedand readssys_position/sys_permission_set. The plan's composition never declares those tables. The write guard from the earlier write-suppression work refuses writes and lets hooks run, so it cannot stop a read. Every plan therefore printed 6DATABASE_ERRORlines on stderr and 6position binding lookup failedwarnings, on a migrated file and on an absent one.The fix: one point per door, for every app
Host code enters a declaration boot through exactly two doors, and each is closed where it enters:
onEnable.AppPlugingets askipOnEnableoption, a sibling of the existingskipSeedData, which serves the same commands. With it set,start()does not runonEnable. It logsruntime.onEnable NOT executed …and reports the skip throughonEnableWithheld. The migrate composition sets the option on the app it builds.packages/runtime/src/app-plugin.ts) and not in a stripped copy of the bundle. The executor is what resolves which object carries the hook (bundle.defaultbefore the bundle itself). A copy would re-state that rule at the CLI call site. The boot would then also log "No runtime.onEnable function found" about an app that has one.onEnableat all: it is JSON, and its runtime module contributesfunctionsonly. A host plugin that is itself anAppPluginis already wrapped, so its wholestart()is suppressed.init().composeForDeclarationsnow forwardsinitwith a context whosehook()does not registerkernel:bootstrappedorkernel:listening. A host that keeps that context and registers later is declined too.IPluginLifecycleEvents).kernel:bootstrappedis for "reconcile/backfill work that consumes" data.kernel:listeningcomes after every plugin "has had a chance to register routes / services / middleware duringkernel:ready". Both say that registration is over.kernel:readyis deliberately kept. The contract puts late registration there, and a host that provisions its tables from akernel:readyhook is a measured shape whose tables the plan must see. The write guard still refuses row writes onkernel:ready.kernel:shutdownhooks, data hooks and custom events register as before.This repo's own plugins (the data stack,
PlatformObjectsPlugin, the guard, andextraPlugins) are not host code and are untouched. The plan still prints the value-shape gate announcement that the engine makes from its ownkernel:bootstrappedhook. No in-repo plugin registers a post-declaration hook frominit(): all seven sites are instart().The plan's notes, and
composition.notesin--json, carry one line naming what was not run. A host with nothing withheld gets no line.Stop clause (does the plan need an app hook for its declarations?) No. On app-crm, the table list, the pending DDL, the drift and
--json(all butnotes) are identical before and after; see below.The earlier design, and how this changes it
The write-suppression card chose to refuse writes at the driver over neutralising
init()-registered hooks. One reason was that a log-only hook should keep running on the plan path. Triage's direction on this card (comment 5924795251) sets the boundary more narrowly: the declaration boot does not fire apponEnable/kernel:bootstrappedhooks. The guard stays the write guarantee on every phase. Only the two post-declaration phases are now withheld for host code. The existing pins that asserted host log-only hooks run on those two phases were inverted in place, and their writers moved tokernel:readywhere the case was about the guard rather than the phase. A new pin keeps the guard's phase-agnostic property: a writer the composition does not wrap is refused on all three phases and instart().Release grading
@objectstack/runtimetakesminor, and this PR declaresClause-②: yes (widening).AppPlugin, exported from the package root, gains the optional constructor optionskipOnEnable(defaultfalse) and the read-only getteronEnableWithheld. That is an additive widening of a published surface, which takes at leastminor.@objectstack/clistayspatch. This was re-graded frompatch/Clause-②: noafter the contract review record 5928867906, in the changeset-only commitafc44ba5bf.Measured on
examples/app-crmnode packages/cli/bin/run.js migrate …fromexamples/app-crm, with nodist/artifact. Base isorigin/main9c8b65aa23, built. Fix isaf9ac5ded3, with runtime and cli rebuilt.DATABASE_ERROR(stderr)position binding lookup failedDATABASE_ERRORplanon an absent fileplan --json, absent fileapply --yesplanon the migrated fileplan --json, migrated filediffshows 1 line added and 0 removed, for plan absent, plan migrated and apply).--jsonis identical exceptcomposition.notes(2 to 3 entries):pending15/15 (absent) and 0/0 (migrated),total0,managedTables15.Examined 15 managed table(s)holds on both. The absent file is not created.Tests
@objectstack/runtimesrc/app-plugin.test.ts: theskipOnEnablepins. The hook is withheld, logged and reported, including when it sits onbundle.default. A bundle with noonEnablereports nothing withheld.@objectstack/cliunit,schema-migration-plugins.test.ts:init()context declines the two phases and forwardskernel:ready,kernel:shutdown, data hooks and every other member;skipOnEnable, itsonEnabledoes not run, and the lifecycle names it.@objectstack/cliintegration,schema-migration-plugins.declaration-boot-write-guard.test.ts, using a realObjectKernel:kernel:readyfor host code, keeps the teardown, and leaves an unwrapped platform plugin on all three phases in the same boot;kernel:readyis declined;@objectstack/cliintegration,schema-migrate.host-composition.integration.test.ts, new block for this card. It uses an app-crm-shaped fixture: a stack with one object, a namedonEnablethat hookskernel:bootstrappedand reads the two undeclared tables, and a host plugin with a readinginit()-registeredkernel:bootstrappedhook.servecomposes it prints the lines.apply's confirmed DDL flush still creates the app's table, and the coverage pass still examines it.DATABASE_ERROR, runs neither hook, and still runs the host'skernel:readyhook.DATABASE_ERRORand leaves no file behind.Runs:
3781713631:vitest run --project local: 297 files, 4252 passed, 5 skipped.--project unit: 240 files, 3422 passed.--project integrationover the 11 migrate-related files: 59 passed.origin/main:5bd79b1b3c: runtimeapp-plugin.test.ts35/35; cli unit file 32/32; write-guard, host-composition andplan.deferred-reads36/36 (integration);typecheck(includingcheck:test-typecheck) green for runtime and cli;6d4ef7c9aa: host-composition 14/14 and clitypecheckgreen, after the test-only fix thatcheck:test-source-aliasasked for;dc1c40ec39differs from6d4ef7c9aaby one comment line.Gates, at
dc1c40ec39.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandsderived 64 commands. All 64 ran with their exit codes recorded before any pipe, and all 64 exited 0.--ranreports "64 derived, 64 run, 0 NOT-MEASURED, 0 UNRUN".check:dual-build-cjs-loadsandcheck:i18n-coveragemeasured, after the nine packages they read were built.check:test-source-aliasturned red on the first pass: a dynamicimport('@objectstack/runtime')sat inside a test body. Moving it to module top made it green.Gates, at
afc44ba5bf(changeset-only commit). The diff fromdc1c40ec39is the one changeset file, so the code families keep theirdc1c40ec39results.check-changeset-no-majorin event mode with this body: the level axis is green ("@objectstack/runtime: minor… the declared widening is accounted for"). The control, the same body againstdc1c40ec39where runtime waspatch, exits 1.--ranover the fresh 19 plus the 45 carried fromdc1c40ec39: "64 derived, 64 run, 0 NOT-MEASURED, 0 UNRUN".Lint, a proven narrowing.
eslint --no-inline-config --format jsonover the 7 touched TypeScript files.--print-configresolves a config for all 7, with 5 to 6 rules and noparserOptions.project.eslint.config.mjsenables no type-aware linting, so this diff cannot move the verdict on any untouched file. The fullpnpm lintis left to CI.Ablations. The fix was committed first. Each run went through
scripts/ablation-replace.mjs: the anchor moved 1 to 0, the blob changed, and the restore brought the blob back equal to HEAD with an emptygit diff HEAD.POST_DECLARATION_PHASESemptied in the CLI source.os migrate planon examples/app-crm runs the app'sonEnablehook, which readssys_position/sys_permission_setthe plan never declares: 6 DATABASE_ERROR + 6 WARN lines on every plan #21054 kernel cases, thecomposeForDeclarationssuppresses onlystart(), so aninit()-registeredkernel:readyhook still writes duringos migrate plan— the guarantee holds only for hosts following an unwritten convention #13332 FIX and R1 cases, and both [finding]os migrate planon examples/app-crm runs the app'sonEnablehook, which readssys_position/sys_permission_setthe plan never declares: 6 DATABASE_ERROR + 6 WARN lines on every plan #21054 plan cases. The plan cases fail onDATABASE_ERROR … 'sys_position'from the host hook.skipOnEnable: truechanged tofalseat the composition.os migrate planon examples/app-crm runs the app'sonEnablehook, which readssys_position/sys_permission_setthe plan never declares: 6 DATABASE_ERROR + 6 WARN lines on every plan #21054 plan cases (DATABASE_ERRORonsys_positionandsys_permission_setfromonEnable).app-plugin.tsneutralised with a planted marker. The runtime was rebuilt, andablation-dist-preflightfound the marker in 2 built files.os migrate planon examples/app-crm runs the app'sonEnablehook, which readssys_position/sys_permission_setthe plan never declares: 6 DATABASE_ERROR + 6 WARN lines on every plan #21054 plan cases.--absentfound the marker in none of the 6 built files and the tree clean. Everything was green again (35, 32, 34).Acceptance notes
examples/app-crmitself. A cli test that reads another package's tree is a cross-package test input. Declaring it would mean editingscripts/cross-package-test-inputs.mjsandturbo.json, which are outside this card's file surface. The real app-crm is measured by the CLI runs in the table above.init()received is outside the composition's reach: throughgetKernel(), or from a service factory, which the kernel calls with its own context. Its writes still meet the guard.kernel:bootstrapped/kernel:listeninghook would lose them from the plan. The contract says registration is over by then, and no in-repo plugin does it.examples/**and driver-sql are untouched.#20821is not reopened here; its deferred-DDL demotion is unchanged.Generated by Claude Code