Skip to content

fix(objectql): the delete-cascade path's two registry reads propagate instead of inventing 'no relations' (#9002) - #9163

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-9002-cascade-registry-swallows
Aug 16, 2026
Merged

fix(objectql): the delete-cascade path's two registry reads propagate instead of inventing 'no relations' (#9002)#9163
os-zhuang merged 2 commits into
mainfrom
claude/issue-9002-cascade-registry-swallows

Conversation

@claude

@claudeclaudeBot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Fixes#9002

What this closes

ObjectQL.delete()'s by-id branch reads registry.getAllObjects()twice, and both reads sat behind a swallow that invented an answer for a read that never happened. Both are in packages/objectql/src/engine.ts, and both are now unguarded:

seamoldconsequence of the old shape
planCascadeAtomicity()catch { return 'none' }'none' is the verdict that asserts nothing references this object, so #7413's cascade atomicity was silently switched off — no transaction, no degrade warning — over a schema nobody could read.
cascadeDeleteRelations(), first statementcatch { return }the cascade does not run at all: no restrict refusal, no set_null, no cascade, nothing logged, and the caller told the delete succeeded.

This is the #8895 shape one layer up, with a strictly larger blast radius: #8895's catch invented "no dependents" for one relation whose probe could not run; seam 2 here invented "no relations" for every relation at once, and reached that answer before the per-relation probe was ever run.

#8895 ruled the family discriminate or propagate. Discrimination needs a benign failure class — there it was the unprovisioned child table, which genuinely cannot hold a referencing row. Here there is none: an unreadable registry is never truthfully "no relations". So propagate is the whole answer, and both catches are removed rather than discriminated.

Seam 2 is decided with seam 1 and in the same direction, not separately: its own comment argued the 'none' direction from cascadeDeleteRelations returning on the same failure, so that nothing was left un-cascaded to make atomic. Once seam 1 propagates that premise is gone. Note the runtime order makes seam 1 the first to speak — delete() computes the plan before running the cascade — so an unreadable registry now fails the delete before any row is touched.

No new error code and no new response field: the registry read's own failure reaches the caller with its envelope intact, exactly as #8895's probe failure does.

This is a structural close, not a live defect

⚠️ Deliberately not claimed as a bug fix. SchemaRegistry.getAllObjects() is a walk over in-memory Maps; resolveObject() returns undefined on every failure branch it models (no contributors; an owner-less contributor set, where the orphan-overlay case console.warns and returns), and the fold beneath it — foldExtendersOntoDefinition to mergeObjectDefinitions, plus scalarOverridesPackagedBase from @objectstack/spec — is spreads and comparisons. No I/O, no driver, no throw statement on the path. Re-derived on the merged ref for this PR rather than taken from the card: the card's dormancy measurement holds.

So nothing in a running deployment changes behaviour. What changes is that the day getAllObjects() grows a throwing path — a lazily-resolved contributor, a validating merge — it fails loudly instead of disabling every referential guard at once, silently.

One thing the swallows were actively hiding

The two catches absorbed structural failures as well as thrown ones. engine-middleware-operation-vocabulary.test.ts module-mocks ./registry with a hand-rolled double that never declared getAllObjects; the resulting TypeError was swallowed by both seams, so an incomplete double read as a registry with nothing in it and the suite passed. Removing the swallows turned that into three hard failures at once, which is the correct outcome — the double now declares the method the engine actually calls, with an empty body (this suite pins the middleware operation vocabulary and registers no relations, so "nothing references the deleted object" is the truthful answer there rather than an invented one).

That repair is the only test change outside the new pin file, and it is a consequence of this change rather than a drive-by.

Tests

New pin: packages/objectql/src/engine-cascade-registry-read-failure.test.ts (7 tests).

The two seams are told apart by which read fails, not by mocking a function: the injector fails the registry's Nth getAllObjects() call and each test asserts the observed call count, so nth: 1 pins seam 1 and nth: 2 — a read that fails once after succeeding, the flaky shape the card names — pins seam 2 with the atomicity plan already computed. Every expectation is a literal (the injected error's object identity, its literal code / status / message, literal row counts read straight out of the stub store), and three positive controls in the same describe (restrict refusal, cascade removal, dependent-free delete) keep a harness that had stopped cascading at all from passing vacuously.

Because the seam is unreachable from real data, the failure is injected at the registry method itself — the injection is the statement that this is dormant.

Reverse verification (fix committed first, then git checkout origin/main -- packages/objectql/src/engine.ts, run, restore byte-identically): the 4 seam pins go red, the 3 positive controls stay green. Two distinct pre-fix symptoms show up in the failure text and both are the fail-open shape:

  • expected true to be Error: registry unreadable…delete()returned true, i.e. the silent success this PR removes.
  • expected Error: This Account is still referenced by… to be Error: registry unreadable… — seam 1's 'none' was invented, and the delete then happened to be caught by the surviving per-relation probe in seam 2. That case degraded into a different guard rather than into data loss, which is exactly why this class is easy to miss.

Test-count direction: up, by the new pin file's 7 tests, plus 3 tests moved (fail then repaired) in the vocabulary suite. No test was deleted or weakened.

Verification, all at branch head 587dc863a

This changes a function's failure behaviour, so the surface run is objectql's downstream consumers, not the edited package alone — --filter '...@objectstack/objectql', the prefix form (dependents), 43 packages:

  • pnpm --filter @objectstack/objectql test — 213 files, 3743 tests, all passing.
  • pnpm exec turbo run test --filter='...@objectstack/objectql' --concurrency=2 — 106 tasks successful, 106 total; zero failures across the closure (runtime 165 files, rest 120, cli 122, dogfood 110 + 1 skipped, driver-turso 39, client 23, plugin-approvals 23, cloud-connection 20, example-showcase 20, and the rest).
  • pnpm --filter @objectstack/objectql typecheck — clean.
  • pnpm check:type-check-debt --re-measure — 33 ledger entries re-measured, 1926 raw errors, none above its recorded number; "every entry sits exactly at its measurement", i.e. the new test file adds zero.
  • Gate union re-derived from the actual changed paths via scripts/pm/dispatch-gates.mjs and run green: check:durability-log-level, check:engine-double-contract, check:where-matcher, check:query-options-erasure, check:type-check-coverage, check:stack-collection-maps, check:nul-bytes, check:cross-package-test-inputs, check:changeset-gate-self-tests, check:objectui-changeset, check-empty-changeset, check-changeset-no-major, check-adr-0087-registration, check-engine-split-ratio.

One measurement worth recording: check:durability-log-level reports the same 66 read seams before and after this change. Neither seam removed here was ever in its population — that is the #8845 blind spot, observed rather than assumed, and the reason this PR carries its own pin.

Deliberately untouched


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql.

13 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx(via @objectstack/objectql)
  • content/docs/data-modeling/formulas.mdx(via packages/objectql)
  • content/docs/kernel/contracts/data-engine.mdx(via @objectstack/objectql)
  • content/docs/kernel/runtime-services/examples.mdx(via packages/objectql)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/objectql)
  • content/docs/kernel/services.mdx(via @objectstack/objectql)
  • content/docs/permissions/authentication.mdx(via @objectstack/objectql)
  • content/docs/permissions/system-context.mdx(via packages/objectql)
  • content/docs/plugins/index.mdx(via @objectstack/objectql)
  • content/docs/plugins/packages.mdx(via @objectstack/objectql)
  • content/docs/protocol/kernel/index.mdx(via @objectstack/objectql)
  • content/docs/protocol/objectql/query-syntax.mdx(via packages/objectql)
  • content/docs/protocol/objectql/state-machine.mdx(via @objectstack/objectql)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/objectql)

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.

Advisory only. 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.

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

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-zhuang@claude