Skip to content

fix(objectql): plugin registry reads discriminate unreadable from empty — schema sync no longer skips every object silently at boot (#9285) - #9682

Merged
os-sam merged 3 commits into
mainfrom
claude/issue-9285-objectql-registry-read-seams
Aug 18, 2026
Merged

fix(objectql): plugin registry reads discriminate unreadable from empty — schema sync no longer skips every object silently at boot (#9285)#9682
os-sam merged 3 commits into
mainfrom
claude/issue-9285-objectql-registry-read-seams

Conversation

@os-sam

Copy link
Copy Markdown
Collaborator

Fixes#9285

ObjectQLPlugin read the registered object set in three places, all spelled
this.ql.registry?.getAllObjects?.() ?? []. That expression folds three different
facts into one value:

  1. the registry answered, and holds no objects;
  2. the engine exposes no registry at all;
  3. the registry exposes no getAllObjects — a structural omission that never
    throws, so it is invisible precisely when it is wrong.

Only (1) is truthfully "no objects". This applies the inherited adjudication, it
does not re-argue it: #8895 ruled the family discriminate or propagate, #9002
applied it to the two delete-cascade seams, #9154 to the roll-up summary index
(PR #9284). Triage promoted this card straight to pm:queue for that reason.

The ruling, per seam

All three seams now read through one shared private helper,
ObjectQLPlugin.readRegisteredObjects(seam), which throws rather than
inventing — naming the consequence — and adds no catch of its own, so a
registry that throws propagates its own error verbatim. What each caller does
with that failure is decided at the seam, and the three answers differ:

seammethoddispositionwhy
2syncRegisteredSchemaspropagateits next line is if (allObjects.length === 0) return;, so an invented empty answer meant no registered object's schema was synced to any driver — no table created, no column added — silently, at boot, with the plugin reporting a clean start. Failing the boot is more truthful than starting against a store whose DDL never ran.
1reconcileFederatedBindingsreport at error, then degradethe pass exists to name the federated objects it could not bind — its own docblock: "a boot with nothing to report says nothing" — so an unreadable registry making it report nothing is exactly the silence it was written to prevent. It stays exception-proof: it is a post-hoc reconciliation deliberately run after every start() so a late-connecting datasource is not a boot failure, and propagating would turn a diagnostic into the hard stop it was written not to be. error, not warn, matches the level its existing report already uses for the same consequence.
3runGovernanceInventoryreport at warn, then skipwarn-only and exception-proof by contract ("a diagnostic must never be the reason a kernel fails to boot"), so it must not propagate.

Seam 1's degradation is proportionate rather than merely convenient: on every boot
that does not set skipSchemaSync, seam 2 has already read the same registry
successfully before kernel:ready, so a failure at seam 1 is a registry that
became unreadable mid-boot, not the boot-wide condition seam 2 now catches.

Seam 3's double swallow — addressed explicitly

Seam 3 carried two independent inventions on one expression:
?.() for a registry that does not implement the method, and a wrapping
try { … } catch { return [] } for one that throws. A fix scoped to the ??
class alone would have left a throwing registry indistinguishable from an empty
one. Both are gone.

The measurement that decided the disposition: feeding the audit an invented empty
object set is worse than silence. collectEngineActionDeclarations derives
declarations from the objects, so with none, every handler declared on an
object reconciles as an "undeclared handler … REFUSED at dispatch" — an
unreadable registry produced a page of false accusations against a healthy
deployment. So the seam does not audit a substitute set at all: it reports once and
returns, leaving lastGovernanceFingerprint untouched so the next successful run
reports in full rather than being suppressed as "unchanged". warn and not
error per the AGENTS.md degradation-log-level rule — an audit that did not run is
a functional degradation; nothing here claims to have been persisted.

Deliberately untouched

The objectsRegistered: … ?.length || 0 count in the ObjectQL engine started
info log, per the card and the dispatch. Judged benign, not left unexamined: a
wrong 0 there costs one advisory line and no data. It is the one executable
occurrence of the old shape the probe below still reports.

① Boot-time reachability — what was MEASURED

Triage deliberately did not establish this, and the fix direction does not hinge on
it. Re-derived on this tree rather than inherited, and it still holds:

  • SchemaRegistry.getAllObjects() (registry.ts) is a walk over the in-memory
    objectContributorsMap, calling resolveObject(fqn). resolveObject returns
    undefined on every failure branch it models — no contributors, no owner (after
    a console.warn) — and never throws. Below it the fold is
    foldExtendersfoldExtendersOntoDefinitiontenantAuthoredScalars /
    mergeObjectDefinitions / scalarOverridesPackagedBase: spreads, Set adds and
    comparisons. No I/O, no driver, no throw on the measured path.
  • The ?. links are dead for a real engine too, which the card did not state:
    ObjectQL.registry is a plain getter returning this._registry, a
    field-initializedSchemaRegistry, and getAllObjects is a prototype method.
    Neither optional link can short-circuit for any engine constructed by
    ObjectQLPlugin.init() or handed in as opts.ql (typed ObjectQL).

So: the seams are dormant against real data, and this is a structural close, not
a live-defect fix.
The reach that is real is a duck-typed ql — an incomplete
test double, which is the #9154 blind spot below. The tests therefore inject the
failure at the registry itself, and the injection is the statement that nothing
shipped reaches these seams today.

This does not change the verdict — discriminate unreadable from empty is right
either way — but it does mean the urgency is "keep the fail-open shape from coming
back", not "a boot is failing today".

② The vi.mock blind spot — classified, not blanket-patched

#9154 measured 83 reds across 9 suites when ?.() came off buildSummaryIndex.
Measured here: zero reds, in both directions.

  • @objectstack/objectql full suite, before: 217 files / 3836 tests, all green.
    After: 218 / 3853, all green (the +1 file and +17 tests are this PR's pin).
    No suite went red — so there is nothing to classify as either "a double to
    complete" or "real coverage"
    , and nothing was blanket-added to any double.
  • Why the two cards differ, measured rather than assumed: the 12 suites carrying
    vi.mock('./registry') in this package drive engine.ts (which is where
    buildSummaryIndex lives), never ObjectQLPlugin's three seams.
  • The plugin's own reach was checked repo-wide: every construction of
    ObjectQLPlugin outside this package is new ObjectQLPlugin() or
    new ObjectQLPlugin({ …options }) with no ql, so init() builds a real
    ObjectQL with a real SchemaRegistry. Not one consumer injects a duck-typed
    engine. Verified across packages/runtime, packages/metadata,
    packages/metadata-protocol, packages/rest, packages/client,
    packages/services/*, packages/cli and packages/qa/*.
  • The consumer sweep ran anyway rather than resting on that reading — see below.

The one place a double now has to answer deliberately is this PR's own pin
file, and it answers by failing: two of its three injections install a registry
that cannot serve getAllObjects, which is the exact indistinguishability the card
is about, asserted rather than papered over.

Reverse verification — predicted vs observed

Predictions were written before running. Resolution note: the pin imports
./plugin.js, a same-package relative specifier vitest resolves to
src/plugin.ts — no exports-mediated hop into dist/, so no rebuild leg applies
to this ablation. Each leg reverted only its own seam, leaving the tests and the
shared helper in place; the tree was then restored and git diff HEAD proved
empty before the restoration leg was re-run.

legpredictedobservedmatch
A — seam 2 back to ?? []3 red2 red, 15 green⚠️partially falsified — see below
B — seam 1 back to ?? []4 red (with two different reasons by injection)4 red, 13 green, both reasons exactly as predicted
C — seam 3 back to the IIFE4 red4 red, 13 green
restoration17 green17 green

Observed failure reasons, all as predicted:

  • A: AssertionError: promise resolved "undefined" instead of rejecting (×2) —
    the read answers [], the next line early-returns, the call resolves.
  • B, no-method / no-registry: expected [] to have a length of 1 but got +0
    — the silent early return, zero error logs. B, throws:
    promise rejected "Error: registry read exploded" instead of resolving — a
    different reason, predicted in advance, because old seam 1 had no catch.
  • C: expected [] to have a length of 1 but got +0 (no SKIPPED warn) and
    expected 'u:acct:ping' to be 'previous-run' — the fingerprint overwritten with
    the false accusation against a declared action.

The falsified prediction, and what was done about it

Leg A predicted 3 reds and produced 2. The test
"hands a THROWING registry error to the caller, identity intact" stayed green
in both directions
— i.e. it pinned nothing about this change. The reason is a
real correction to how seam 2 was described: ?. short-circuits on absence,
never on a throw, and seam 2 had no catch (unlike seam 3), so a throwing
registry already propagated there before this PR. Seam 2's swallow was the
structural half alone.

Rather than delete or quietly keep it, the test is relabelled as
"a THROWING registry already propagated, and must keep propagating (preserved, not
fixed)"
, with the measurement recorded in the file header — because it still earns
its place: it fails the day someone "hardens" this read by wrapping it in a
try/catch, which is precisely how seam 3 acquired its second swallow.

Gates — union derived on the final commit

node scripts/pm/dispatch-gates.mjs (no path arguments) against the committed
diff, re-derived on the final commit and unchanged from the first derivation.
All values below are from runs on 5da142ba7, captured as cmd > log 2>&1; EXIT=$?
— never piped into tail, per #9552.

gateEXIT
pnpm check:changeset-gate-self-tests0
pnpm check:durability-log-level0
pnpm check:objectui-changeset0
node scripts/check-adr-0087-registration.mjs0
node scripts/check-changeset-no-major.mjs0
node scripts/check-empty-changeset.mjs0
node scripts/check-engine-split-ratio.mjs0
node scripts/docs-audit/check-affected-docs.mjs0
pnpm check:query-options-erasure0
pnpm check:type-check-coverage0
pnpm check:type-check-debt --re-measure0
pnpm check:engine-double-contract0
pnpm check:where-matcher0

check:durability-log-level stays green and its census is unchanged — neither
the read-invention baseline (scripts/durability-read-invention.baseline.json, no
plugin.ts entry before or after) nor the empty degradation baseline moved. That
is the #8845 blind spot re-confirmed across this fix, exactly as #9154 re-measured
it: this shape remains invisible to the gate, which is why the pin file exists.

check:type-check-debt --re-measure caught a real regression and it was fixed at
the source, not at the ledger.
The new test file first added +1 raw tsc error to
@objectstack/objectql's TEST_DEBT (355 → 356): an action's body is
{ language, source }, not a bare string. Re-measured after the fixture fix:
355, with 0 errors attributable to this file — the shrink-only ledger is
untouched, and raising it was never considered (maintainer-only).

Suites

packageresultEXIT
@objectstack/objectql (full, final commit)218 files / 3853 tests passed0
@objectstack/objectql (baseline, before any edit)217 files / 3836 tests passed0
pin file alone17 / 17 passed0
@objectstack/objectql typecheck (tsc --noEmit, script name echoed)pass0
@objectstack/runtime170 files passed0
@objectstack/metadata31 files / 603 tests passed0
@objectstack/metadata-protocol121 passed + 2 skipped / 1669 passed0
@objectstack/rest125 files / 2058 tests passed0
@objectstack/http-conformance4 files / 72 tests passed0
@objectstack/client23 files / 310 tests passed0

@objectstack/http-conformance failed once on
Failed to resolve entry for package "@objectstack/runtime" — that is the
unbuilt-dist/ prerequisite, not a finding. It passed after building runtime.
The consumer suites resolve @objectstack/objectql through package exports into
dist/, so dist/index.js was confirmed to carry the fix (readRegisteredObjects
present) before those runs were read.

Probe, with its positive control

A zero-hit grep is not a measurement, so the probe getAllObjects\?\.\(\) is shown
hitting the known sites first:

Scope

Clause ② was not reached and was not approached: no file under
packages/spec/src/** (only an import type { ServiceObject }, added so the seams
keep the exact element type they already inferred), no contract accept/reject
behaviour changed, no public surface widened. syncRegisteredSchemas propagating
instead of silently returning is a boot behaviour change, not a contract one.

Files changed: packages/objectql/src/plugin.ts,
packages/objectql/src/plugin-registry-read-failure.test.ts (new),
.changeset/objectql-plugin-registry-read-seams.md (new).


Generated by Claude Code

…ty (#9285)
`ObjectQLPlugin` read the registered object set in three places, all spelled
`this.ql.registry?.getAllObjects?.() ?? []` — an expression that folds "the
registry holds nothing", "the engine exposes no registry" and "the registry
does not implement getAllObjects" into one value. Only the first is truthfully
"no objects" (#8895: discriminate or propagate; applied by #9002 and #9154).
- syncRegisteredSchemas propagates: its early return meant NO object's schema
was synced to any driver, silently, at boot.
- reconcileFederatedBindings reports at `error` then degrades — it is a
post-hoc reconciliation, deliberately not a boot gate.
- runGovernanceInventory reports at `warn` then skips, closing BOTH swallows
(`?.()` and the wrapping try/catch); auditing an invented empty object set
accused every object-declared action of being undeclared.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017qYPmkKEsfbWY1yVg83p8F
…ed (#9285)
Reverse verification measured it green in both directions: seam 2's swallow was
the optional-chain half only (`?.` short-circuits on absence, never on a throw,
and that seam had no `catch`), so a throwing registry propagated there before
this change too. The test is kept — relabelled, with the measurement recorded in
the file header — because it fails the day someone wraps this read in a
try/catch, which is how seam 3 acquired its second swallow.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017qYPmkKEsfbWY1yVg83p8F
…ys flat
`check:type-check-debt --re-measure` caught the new file adding +1 raw tsc error
to @objectstack/objectql's TEST_DEBT (355 -> 356): `body` on an action is
`{ language, source }`, not a bare string. Fixed at the fixture, which is the
author's remedy — the ledger is shrink-only and raising it is maintainer-only.
Re-measured: 355, and 0 errors attributable to this file.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017qYPmkKEsfbWY1yVg83p8F
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/data-modeling/objects.mdx(via ObjectQLPlugin (symbol))
  • content/docs/kernel/services-checklist.mdx(via ObjectQLPlugin (symbol))
  • content/docs/kernel/services.mdx(via ObjectQLPlugin (symbol))
  • content/docs/permissions/authentication.mdx(via ObjectQLPlugin (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

Coarse fallback — 14 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 4b2de3cd1406315754eb5ea3d016dc867740325fpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 6db3ebe334e036398967909ce204ecccf05fcbba — the merge of head 5da142ba7d12507f18d01679bbe46d482120b2dd into base 4b2de3cd1406315754eb5ea3d016dc867740325f, 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 6db3ebe334e036398967909ce204ecccf05fcbba && git checkout 6db3ebe334e036398967909ce204ecccf05fcbba
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 4b2de3cd1406315754eb5ea3d016dc867740325f 5da142ba7d12507f18d01679bbe46d482120b2dd && git checkout -B drift-repro 4b2de3cd1406315754eb5ea3d016dc867740325f && git merge --no-ff 5da142ba7d12507f18d01679bbe46d482120b2dd
node scripts/docs-audit/affected-docs.mjs --json 4b2de3cd1406315754eb5ea3d016dc867740325f

⚠️ 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 4b2de3cd1406315754eb5ea3d016dc867740325f → pass the list as
args.docs, on the commit named under Which tree this was computed on.

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-sam@claude