Uh oh!
There was an error while loading. Please reload this page.
fix(service-settings): refuse a settings write issued before the engine is bound - #10251
Conversation
…ne is bound `upsertRow` picks its store on `if (this.engine)`, and the engine is bound in exactly one place: `SettingsServicePlugin` registers a `kernel:ready` hook from its `start()` and calls `bindEngine` inside it. Hooks fire in registration order and every plugin's `init()` runs before any plugin's `start()`, so every `kernel:ready` hook registered from an `init()` runs inside that window — an ordinarily occupied position (`assembleMetadataProtocol` registers the three platform migrations' hook from `ObjectQLPlugin.init()`). A `set()` from there landed in the in-process memory fallback, re-resolved off that same array, and handed the caller a fully resolved value while `sys_setting` received nothing and both audit ledgers stayed silent. No log line at any level: the write did not fail, it succeeded against the wrong store. A write in the window now raises `SettingsEngineNotBoundError` (`SETTINGS_ENGINE_NOT_BOUND`, 503) naming `kernel:bootstrapped` as the earliest safe phase. The refusal is armed only by the new opt-in `SettingsServiceOptions.engineBindPending`, set by the plugin in `init()` and cleared on BOTH branches of its `kernel:ready` hook — `bindEngine` when `objectql` is present, the new `settleWithoutEngine()` when it is not. Every other engine-less reading of the memory fallback, and every read in any state, is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
📓 Docs Drift CheckThis PR changes 2 package(s): 7 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 124 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 bffafa173c08a0603eecb300b6ac11a0dcd9310b && git checkout bffafa173c08a0603eecb300b6ac11a0dcd9310b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 359f5956d7910aed7ae9f8fccc9fbb988b3e4882 8b2c585286a2402f5cf25f618e5c2681dfe70e06 && git checkout -B drift-repro 359f5956d7910aed7ae9f8fccc9fbb988b3e4882 && git merge --no-ff 8b2c585286a2402f5cf25f618e5c2681dfe70e06
node scripts/docs-audit/affected-docs.mjs --json 359f5956d7910aed7ae9f8fccc9fbb988b3e4882
|
…ENGINE_NOT_BOUND `content/docs/references/api/` is generated from `packages/spec` and was never regenerated after this branch registered `SETTINGS_ENGINE_NOT_BOUND` in `packages/spec/src/api/error-code-ledger.zod.ts`, so the `Type Check - source gates` job failed with both pages reported out of date. Produced by exactly the two commands the gate names, in that order: pnpm --filter @objectstack/spec gen:schema && pnpm --filter @objectstack/spec gen:docs The whole diff is the one new ledger member and its consequences: `error-code-ledger.mdx` gains the `SETTINGS_ENGINE_NOT_BOUND` row, and `contract.mdx` gains the same row plus the enum-summary count it carries (`+283 more` to `+284 more`). No source file, no changeset, no behaviour change. `gen:schema` rewrote no tracked file, and of the 229 files `gen:docs` renders only these two moved. `pnpm --filter @objectstack/spec check:docs` now exits 0: `OK 229 generated files in sync with packages/spec`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
Fixes#10159
SettingsService.upsertRowpicks its store onif (this.engine), and the engine is bound in exactly one place:SettingsServicePluginregisters akernel:readyhook from itsstart()and callsbindEngineinside it. Hooks fire in registration order (hooks.get(name).push(...)inpackages/core/src/kernel-base.ts, dispatched in array order) and every plugin'sinit()runs before any plugin'sstart()— so everykernel:readyhook registered from aninit()runs inside that window. Aset()from there landed in the in-process memory fallback, re-resolved off that same array, and handed the caller a fully resolved value whilesys_settingreceived nothing and both audit ledgers stayed silent. Nothing was logged at any level, because the write did not fail: it succeeded against the wrong store.Premise re-established on this branch before anything changed
A real
LiteKernel, a realObjectQLover the realSysSetting/SysSecretschemas, the realSettingsServicePlugin, and a probe plugin registering itskernel:readyhook frominit():One column is sourced differently and is flagged rather than blended in:
readAtReadywas not a field on the original pre-fix probe (it was added when the probe became the shipped test), so its BEFORE value is the ablation reading — the guard neutered on the fixed tree, which restores this code path toorigin/main's behaviour. Every other BEFORE cell was measured directly onorigin/mainbefore anything changed.The control write moments later lands a real
sys_settingrow and a realsys_setting_auditrow on the same connection, so the store existed throughout — the timing is the whole cause. The named population is real and static:assembleMetadataProtocolregisters the three platform migrations'kernel:readyhook fromObjectQLPlugin.init()(packages/objectql/src/plugin.ts,init = asyncat 303, well beforestartat 385 →packages/metadata-protocol/src/plugin.ts:306).Loud refusal, not buffered replay — argued from measured consequences
Who is in the window today. Swept the repo for settings writers: zero
.set(/.setMany(call sites against the settings service outsidepackages/services/service-settingsitself, and the two inside it are the HTTP PUT handler (settings-routes.ts:137, which cannot run beforekernel:listening) andsetMany's own internal delegate. Counter-check on the same corpus with the same regex family:.get(/.getNamespace(returns ten shipped call sites, so the sweep works and the zero is a measurement rather than a broken pattern. No shipped startup sequence becomes an error.Why a buffer cannot keep the promise the resolved value makes:
setMany's env-lock and upper-scope-lock checks read throughloadRows, which in the window readsmemory— an in-window write is validated against a store that does not contain the persisted locks. Replaying it would commit a write a real pre-flight would have refused withSETTINGS_LOCKED. That is a correctness failure, not a bookkeeping one.cryptoProviderandsecretStorearrive on the samebindEnginecall, so a buffer would have to hold plaintext in process memory until bind. In-window encrypted writes already refuse today (the plugin's defaultcryptoisNoopCryptoAdapter,providesConfidentialityreturns false,assertEncryptionAvailablethrows) — buffering would make the write door less uniform, in the directionSETTINGS_CRYPTO_UNAVAILABLEexists to prevent.What the refused caller does instead is a shipped, documented phase, named in the error message:
kernel:bootstrapped, whichplugin-lifecycle-events.tsalready describes as the "all synchronous bootstrap has settled" anchor for exactly this class of work.Clause-② determination: no — non-window callers observe nothing new
Decided before the fix was written, and pinned by tests rather than asserted. The refusal is armed only by the new opt-in
SettingsServiceOptions.engineBindPending, which onlySettingsServicePluginsets (ininit()) and which both branches of itskernel:readyhook clear —bindEnginewhenobjectqlresolves, the newSettingsService.settleWithoutEngine()when it does not. So the guard covers a declared, pending bind and nothing else:SETTINGS_ENGINE_NOT_BOUND/ 503sys_setting+ auditSettingsService(unit test / bootstrap / control-plane mock)objectqlkernel:readyhook settles the question (now with awarnthat those values are lost on restart)One surface addition is unavoidable for any refusal-shaped fix and is called out rather than buried:
SETTINGS_ENGINE_NOT_BOUNDis registered inERROR_CODE_LEDGER(packages/spec/src/api/error-code-ledger.zod.ts) per ADR-0112. That widens theErrorCodeunion by one member; it changes no existing envelope and no existing caller's accept/reject behaviour. The status (503, notSETTINGS_CRYPTO_UNAVAILABLE's 500 — this one is temporal, the identical write succeeds one phase later) is declared on the error class rather than at asendErrorsite because no HTTP door can reach it: the window closes atkernel:readyand sockets open atkernel:listening, strictly after.Tests
packages/services/service-settings/src/settings-engine-bind-window.test.ts, six cases, driving a real kernel boot. Cases 4–6 are the Clause-② pins above.The silent-loss direction is pinned explicitly. On the behaviour this replaces,
sys_settingwas also empty after the in-window write — so a case asserting only "no row landed" would have passed against the defect and tested nothing. The load-bearing assertion is the refusal: the probe records the write's outcome as a string,resolved:…on the old behaviour andthrew:SETTINGS_ENGINE_NOT_BOUND:503on the new one.Ablation. Predicted signature stated first: neutering
assertEngineBoundto an unconditionalreturnrestores the silent accept, so the two in-window write cases and the standalonebindEnginecase go red. Observed 4 red, not the predicted 3 — the extra one is real and is reported rather than smoothed over: case 2 (reads in the window are NOT gated) also reddened, withexpected 'resolved:"written-at-kernel-ready"' to be 'resolved:"never"', because under the ablation the in-window write lands in memory and the in-window read then reads the phantom value straight back out. That is a second silent-loss witness, and case 2 now documents the coupling deliberately instead of carrying it by accident.git hash-object packages/services/service-settings/src/settings-service.ts:ff35b55c7d25959350d185ea4db5ccffb8c953b2before →52ce662edcda238953c83bb543a5e6139a1bfc4eneutered →ff35b55c7d25959350d185ea4db5ccffb8c953b2restored, byte-identical, with the ablation marker absent from the tree andgit statusclean.Rebuild statement, argued from the files rather than recited. No rebuild was required for this ablation, and the reason is that
distis nowhere in the resolution path of the mutated file. The tests import the subject through relative specifiers (./settings-service.js), which vitest resolves tosrc/*.tsin the same directory — no packageexportsinvolved. Every cross-package specifier they do use is aliased to source by this package's ownvitest.config.ts(@objectstack/core,@objectstack/objectql,@objectstack/platform-objects[/system],@objectstack/spec[/*],@objectstack/types), which is why this package carries noKNOWN_UNALIASED_TEST_IMPORTSentry. The decisive check rather than the argument:packages/services/service-settings/dist/does not exist in this worktree, and the new cases pass asserting new behaviour — there is no build artifact they could have been reading.Gates
node scripts/pm/dispatch-gates.mjs(no paths passed — the script derives the change set from the merge base itself) run after the final commit, on a clean worktree. Exit codes captured by redirecting each gate to a file first and reading$?before any pipe, never throughtail.All at
cd161cb2d(git rev-parse --short HEAD), which is also the sha the package suite and the ablation above were measured on.check:nul-bytescheck:changeset-gate-self-testscheck:cross-package-test-inputs(both spellings)check:dispatcher-error-vocabularylint check:doc-formula-expressionsspec check:empty-statecheck:error-code-casingspec check:livenesscheck:merge-drivercheck:objectui-changesetcheck:slot-lookupcheck:spec-parsed-aliasspec check:strictness-ledgercheck:test-source-aliascheck:type-source-resolutionspec check:variant-docscheck-adr-0087-registrationcheck-changeset-no-majorcheck-empty-changesetdocs-audit/check-affected-docscheck:query-options-erasurecheck:type-check-coveragecheck-type-check-coverage: OK — 64/77 workspace packages type-checked (plus the root), 13 in the DEBT ledgercheck:type-check-debt(--re-measure)check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured in 238.5s, 1924 raw tsc error(s) total, none above its recorded number.check:engine-double-contractcheck:where-matchercheck-dev-prereqs✗ The workspace is not built — 1 unmet precondition, not a list of problems.An unbuilt-worktree precondition, not this diff — afterturbo run build(exit 0) it re-ran green:✓ 67 package build artifacts present.Package suite, at the same sha:
pnpm --filter @objectstack/service-settings test→Test Files 24 passed (24) · Tests 462 passed (462)(456 before, +6 new);pnpm --filter @objectstack/service-settings typecheck→ exit 0.check:type-check-coverageledger entries were not raised — the--re-measureverdict above says none is above its recorded number.Deliberately not done
get()resolves from manifest defaults / the memory fallback rather than from persistedsys_settingrows, so a boot-time reader can legitimately see a stale value. That is a different defect from this card's silent write loss, and closing it would change what a real population observes. Filed as [finding] A settings READ in the pre-bind window silently resolves to manifest defaults instead of the persistedsys_settingrow #10250 (unassigned,finding— the population is not yet measured).content/docs/releases/edit, nocheck-type-check-coverageledger movement, and no attempt to make the window smaller — a narrower silent-loss window would still be a silent-loss window.Generated by Claude Code