Uh oh!
There was an error while loading. Please reload this page.
fix(cli): make a declaration boot write nothing at the driver seam, not by suppressing start() alone - #14053
Conversation
📓 Docs Drift CheckThis PR changes 1 package(s): 4 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 — 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 24230ff0a3160c5da4bcde41cafb84978573ccd3 && git checkout 24230ff0a3160c5da4bcde41cafb84978573ccd3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c75962ac3e0af69ae20f658618c4d3ab58192cc8 840f058ecb3034f65d137ccd0c62c735325cced4 && git checkout -B drift-repro c75962ac3e0af69ae20f658618c4d3ab58192cc8 && git merge --no-ff 840f058ecb3034f65d137ccd0c62c735325cced4
node scripts/docs-audit/affected-docs.mjs --json c75962ac3e0af69ae20f658618c4d3ab58192cc8
|
os-steve
commented
Sep 1, 2026
REQUEST CHANGES At-tier contract review under The ruling this PR routed here (both limbs, judged separately)1. Accept/reject change — NO, this is conformance, and the PM's 2. Public-surface widening — NO, measured with a control. Built So on the contract question: clause ② does not apply — the guarantee was already the contract. The REQUEST CHANGES is not about the ruling; it is about claims the PR rests on that did not survive verification. Verified TRUE (independently)
What did not survive verification — the required changesR1. The refusal-surface claim is false as stated, and I measured the hole. The PR: "Refused members are read off the contract, not off a survey… a driver's raw escape hatches — The same armed guard, the same driver instance, the same hook: R2. "DDL — already held back by R3. One unstated residue worth naming in the census: the guard's scan covers "every R4. Text accuracy in the same pass: Changeset grade
SummaryDesign (b) is the right design, the seam is the right seam and verifiably the only one that intercepts, the tests are real and their controls are the right controls, and the contract question resolves to conformance on both limbs — no accept/reject change, no measured surface widening. What must change before this leaves draft: R1 (cover or report Generated by Claude Code |
…execute() escape hatch, correct the census
R1: execute() is a REQUIRED member of IDataDriver (packages/spec/src/
contracts/data-driver.ts:108, 'Raw Execution (Escape Hatch)'), not a
driver-sql extension, and the guard did not touch it: in one guarded boot
create() was refused and reported while execute("INSERT ...") landed a
row silently under a printed 'a plan writes nothing'. Chosen: REPORT, not
cover — a raw command is unknown by contract ('SQL string, shell command,
or API payload') and SQL text cannot be classified read-vs-write reliably
(a SELECT can quote INSERT in a literal; a CTE can write), while the
framework's own boot work (ensureOverlayIndex's index DDL) runs through
this seam, so a refusal would either break legitimate calls or rest on a
guess. A boot-window execute() is now forwarded, counted per driver,
warned once per driver on stderr, and named in the composition notes —
and no note claims the plan wrote nothing on such a run: the flat claim
is an OUTCOME, printed by disarm() only when it held.
R2: 'DDL already held back by deferSchemaDdl' was false for dropTable /
rotateShards — they run assertSchemaMutable (a schemaMode/dialect gate,
not a deferral check) and execute immediately. The census now states the
split and names them a genuinely open boundary.
R3: drivers the engine holds for a non-default datasource are never
published as driver.* (the only registration is
default-datasource-plugin.ts) and are invisible to the guard's scan —
now stated in the census, module header and changeset.
R4: ObjectQLPlugin.start (plugin.ts:469/:628), not init(), walks the
driver.* services; the interface is IDataDriver, not IDataSourceDriver
(no such symbol exists in spec). Corrected in docblocks, tests and the
changeset.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UngCYXF98BVpYA9hfz6NYk…claration-boot-write-suppression
os-steve
commented
Sep 1, 2026
Review response — R1–R4 addressed at Re-derivations (before any edit):
R1 — chose REPORT, not cover, for the reason the review anticipated: the command is R2 — the census and changeset no longer claim R3 — the engine-held non-default-datasource residue is named in the module header census, PR body and changeset: only the default datasource is ever published as R4 — Gates at PR remains draft; labels untouched — Generated by Claude Code Generated by Claude Code |
os-steve
commented
Sep 1, 2026
APPROVE Follow-up at-tier contract review under 1. Report over cover — the right call, on premises that heldAll three stated premises re-derived, plus one the fix did not claim:
I asked for cover-or-report in R1 and offered report as an acceptable shape; report is not just acceptable — for this member it is the only shape that neither guesses nor breaks. No bounded cover exists without one of those two defects. 2. The disarm()-owned outcome — closes it, does not relocate it (with one stated residue)Measured behaviourally, both directions, at the new head:
So the sentence is no longer an assertion printed over an unverified run; it is a measurement over everything the guard can observe. The honest residue, for the record: the claim can still print on a run where a write escaped through a census-stated open boundary — 3. R3 residue — stated accurately, and deferral is acceptableThe census/changeset wording matches what I measured, item for item: only the default datasource is ever published as 4. R4 zero — re-derived, holds, with controls
Also re-verified rather than assumed after the merge: the seam facts stand (engine RulingThe clause-② ruling from the first review stands unchanged at this head: conformance, not a contract accept/reject change; no public-surface widening. All four required items are addressed and verified. Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#13332
Design decision (b) — suppress writes at the driver seam for the declaration boot, so read/log hooks still run — as decided by the
domain:cliPM seat in the claim comment. It was implementable as stated; the named fallback (report a seam census showing no single driver seam exists) was not taken.The defect
composeForDeclarationsdocumented the plan path's guarantee in its own words — "(init runs, start does not — a plan writes nothing)" — and implemented it as a Proxy whose only override isstart.packages/core/src/kernel.tsthen fires three phases unconditionally after the suppressed start pass:A writing hook registered from
init()survives the suppression on all three. The guarantee was therefore a property of plugins that happen to seed fromstart()— the shape of the one plugin that had been measured — and not a property of the plan path.packages/core/src/kernel.tsis untouched by this PR: the unconditional firing is the condition, not the fix site.The seam, and why this one
The driver instance, guarded for the length of the kernel bootstrap.
driver.*service entry.ObjectQLPlugin.start()walks the kernel'sdriver.*services (packages/objectql/src/plugin.ts— the discovery loop lives instart, notinit) and hands each to the engine, which keys its registry bydriver.nameand discards a second instance under a name it already holds (packages/objectql/src/engine.ts,registerDriver). A wrapper registered in place of the service would be refused by the engine, and everyobjectql-mediated write would go straight to the raw driver. The instance is shared and the engine's write path is a call-time property lookup on it, so guarding the object itself covers both the plugin that resolvesdriver.*directly and the engine that writes through it.IDataDriver(packages/spec/src/contracts/data-driver.ts):create,update,upsert,delete,bulkCreate,bulkUpdate,bulkDelete,updateMany,deleteMany. Adding a write to that interface is a spec diff every driver has to implement, so the list goes stale loudly rather than silently.execute()is a required member ofIDataDriver(data-driver.ts:108, under the contract's own "Raw Execution (Escape Hatch)" heading) — on every driver, not a driver-sql extension. It is not refused, for a stated reason: the command isunknownby contract ("SQL string, shell command, or API payload"), SQL text cannot be classified read-vs-write reliably in either direction (aSELECTcan quote the wordINSERTin a literal; a CTE can write), and the framework's own boot-legitimate work runs through this seam (ensureOverlayIndex's index DDL). What must never happen is the silent half: a boot-windowexecute()is forwarded, counted per driver, warned once per driver on stderr, and named in the composition notes — and on such a run no note claims the plan wrote nothing. The "a plan writes nothing" sentence is an outcome, printed bydisarm()only on runs where it held.context.trigger()dispatches boot hooks propagating (packages/core/src/hook-dispatch.ts), so a throwing refusal would abort the bootstrap — turning "your plugin wrote during a dry run" into "you cannot get a plan at all", on the command whose whole job is to be read before a production apply. A refused call returns a contract-shaped value (create/upsertecho the caller's own payload rather than inventing an id;deletereturns the contract's not-foundfalse;updateMany/deleteManyreturn0), and the run says so out loud: one warning on stderr per driver/method/object triple, plus a line in the composition notes the plan prints and--jsoncarries.The guard is armed by a plugin composed first in
buildSchemaMigrationPlugins, so itsinit()is ordered ahead of every host plugin's (resolvePluginOrderis a DFS in registration order), and it re-scans in Phase 2 to pick up a driver registered by a laterinit().bootSchemaStackdisarms it the moment the bootstrap returns — everything after that is work the command was asked for, soos migrate apply's confirmed DDL flush and the coverage pass are untouched.Seam census — what is NOT covered, stated rather than hidden
Every one of these is named in the module header too, so a future reader can tell a deliberate boundary from an oversight:
execute()— reported, never silent (see above): forwarded during the boot window, counted per driver, warned on stderr, named in the notes, and the "writes nothing" claim dropped for that run.getKnex()(a driver-sql extension, genuinely off-contract) is not intercepted; it was not the path any measured instance of this defect took.deferSchemaDdlholds back theinitObjects/syncSchemapath, whichapplyflushes on purpose once the operator confirms — guarding that would refuse the one write these commands exist to make.dropTable(and driver-sql'srotateShards) are NOT held back by that deferral: they runassertSchemaMutable— a schemaMode/dialect gate, not a deferral check — and execute immediately, so a hook callingdriver.dropTable(...)on a managed datasource during a declaration boot executes, today as before this guard. A genuinely open boundary, stated.driver.*services the kernel publishes, and the only such registration repo-wide is the default datasource's (packages/runtime/src/default-datasource-plugin.ts).DatasourceConnectionService.connect()hands every other datasource's driver straight toengine.registerDriver, never throughdriver.*— so a host stack that connects a second datasource during a composed boot holds an engine-side driver this guard cannot see, and objectql-mediated writes to objects bound to it would land. Likely coverable by also arming instances at theengine.registerDriverseam while the guard is armed; proposed as a follow-up rather than widened into this PR — scope growth on a p1 is the PM's call.Verification
The decisive test, with its positive control —
packages/cli/src/utils/schema-migration-plugins.declaration-boot-write-guard.test.ts, booting a realObjectKernelwith a recording driver whose write methods live on the prototype, exactly like a real driver's:create:sys_permission_setfromstart(), thencreate:sys_ai_modelon ready, bootstrapped and listening);composeForDeclarationsalone removes only thestart()seed — all threeinit()-registered hooks still write. Pinned, not described;log-only:kernel:ready, write:kernel:ready, …, i.e. every handler executed and only the write was refused;create()(refused — the in-run control) and a rawexecute("INSERT …"): the raw command is forwarded (the real driver ran it and its return value came back), counted (rawExecutions= 1 via that driver), and reported — and the disarm note drops the flat claim:expect(note).not.toContain('a plan writes nothing'). The refusal-only case pins the converse: with no raw command forwarded, the claim held and is printed;create()), a refused call hands back a contract-shaped value instead of throwing, anddisarm()removes the shadowing own properties (theexecuteforwarder included) rather than overwriting them.End to end against a real SQL driver —
packages/cli/src/utils/schema-migrate.host-composition.integration.test.ts, on a sqlite database whose tables already exist:bootSchemaStackwith the writer coming fromobjectstack.config.tsand one handed straight to the kernel adds 0 rows (measured as a delta against the control's 3), every log-only hook ran, andcomposition.notescarriesRefused 6 write(s) during the declaration boot — a plan writes nothing(no raw command that run — the claim held);create()refused (0 rows) whileexecute("INSERT …")landed 1 row — forwarded on purpose, and no longer silent: the notes carryRefused 4 write(s) during the declaration boot:(the colon marks the dropped claim),Raw execute() was called 1 time(s) during the declaration boot, and no note in the run claims the plan wrote nothing.Ablations — direction stated in writing before each run.
Original guard ablation (measured at
6a8a182b77): drop the guard from the composed plugin list ⇒ predicted RED on the integration fix case and the two composition-ordering assertions, guard unit file GREEN; observed exactly that (expected 9 to be 3— six rows the operator never asked for). Mutate/restore/measure in one process undertrap … EXIT INT TERM, blob-hash-verified.R1 reporting ablation (measured at
840f058ecb): remove theexecutearm (DRIVER_RAW_EXECUTION_METHODS.slice(0, 0); anchor occurs exactly once) ⇒ predicted RED on the unit escape-hatch case, the disarm restore case (it asserts theexecuteshadow exists), and the integration R1 case (the unqualified claim returns); predicted GREEN elsewhere, the raw row still landing. Observed exactly that:Gates — union derived on the merged tree at
840f058ecbwithnode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, reconciled withcomm -23/comm -13in both directions, exact string comparison: named 35, ran 35, unreconciled 0. Pluspnpm lint(repo-wide,eslint . --no-inline-config, exit 0 — no narrowing) andpnpm check:nul-bytes(exit 0), neither of which the derivation names.34 of the 35 are green.
node scripts/check-test-completeness.mjsis NOT MEASURED (exit 3, PREREQUISITE NOT MET) — it grades a savedturbo run testlog, does not run tests, and its own text instructs a local family run to record it as NOT MEASURED. Three others first reported PREREQUISITE NOT MET for missing build output and are green after building the closures they named:check:dual-build-cjs-loads,check:i18n-coverage,check:type-check-debt(exit codes captured before any pipe, per gate log).Tests at
840f058ecb: the guard unit + composition files 36/36; the#13332integration block 3/3; the whole@objectstack/clipackage 224 files / 2551 passed + 15 skipped, with two unrelated files (test/init-created-files-summary.e2e.test.ts,src/commands/datasource/envelope-unwrap.test.ts) timing out theirbeforeAllhooks under load during the full parallel run and passing 15/15 when re-run in isolation;pnpm --filter @objectstack/cli typecheckexit 0.Contract review requested
Changing what a declaration boot permits is likely a contract change, and this PR does not decide that. The at-tier review (2026-09-01) ruled clause ② no on both limbs — conformance, not a contract change; no measured public-surface widening — and its four required items are addressed above.
What changed, and for whom. For
os migrate planandos migrate apply: during the kernel bootstrap, a row write issued through anydriver.*instance the kernel publishes is now refused and reported instead of executed, and a rawexecute()call through such an instance is forwarded but reported — with the run's notes declining to claim the plan wrote nothing. A host whose plugins wrote during that window will see contract writes stop landing and raw commands named; a host that did neither sees no change whatsoever: no disarm note is emitted when nothing was refused and no raw command went through, and the artifact-less, config-less run is untouched (that path returns before the guard is composed). Nothing outside these two commands is affected —os serve,os devandos startdo not boot throughbootSchemaStack.The changeset is
minoron@objectstack/cli.Generated by Claude Code