Skip to content

fix(objectql): a failed cascade-delete dependents probe surfaces instead of silently skipping the referential guard (#8895) - #9006

Merged
os-zhuang merged 3 commits into
mainfrom
claude/issue-8895-cascade-delete-fail-open
Aug 16, 2026
Merged

fix(objectql): a failed cascade-delete dependents probe surfaces instead of silently skipping the referential guard (#8895)#9006
os-zhuang merged 3 commits into
mainfrom
claude/issue-8895-cascade-delete-fail-open

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#8895

ObjectQL.cascadeDeleteRelations() probes each child relation to decide what the parent's delete must do. That probe is the referential-integrity guard, and it sat behind a bare catch { continue; }, so any failure of it was indistinguishable from "this child has no rows": a deleteBehavior: 'restrict' relation never refused, set_null / cascade never ran, nothing was logged, and the caller was told the delete succeeded. Fail-OPEN on an integrity guard.

The change — one catch, discriminated

packages/objectql/src/engine.ts, in cascadeDeleteRelations (located by symbol; the card's line numbers had rotted):

}catch(error){if(isMissingTableError(error))continue;throwerror;}

The catch is not deleted. It asks by error type, through the shared isMissingTableError predicate (@objectstack/metadata/errors) already imported and used at 4 sites in this file — the same call seedAutonumber and resolveFileReferences make, never a hand-rolled code test:

  • benign, behaviour unchanged — the child object is registered but its table was never provisioned. It cannot hold a referencing row, so zero dependents is the truth and the relation is skipped exactly as before.
  • everything else surfaces — connection drop, timeout, permission denial, query error, missing column. The delete fails with the probe's own error, envelope intact, and nothing is written.

No new error code and no new response field, per the maintainer's 2026-08-15 ruling for this family (propagate via existing discrimination; unprovisioned is truthful emptiness, everything else must surface).

The premise held. Verified on origin/main before implementing: the seam, the fail-open reading and the already-present import are all as described.

Is unprovisioned-table the only benign case? Measured, yes for this seam. The probe is this.find(childName, …) where childName comes from the registry walk, so "object not registered" cannot occur here; every remaining failure class means the dependents may exist and simply were not seen. A permission denial on the child is deliberately in the "must surface" set: a guard the caller was not allowed to evaluate has not been evaluated. No legitimate caller regressed — the full @objectstack/objectql suite (212 files / 3728 tests) is green with the fix in.

Pin — packages/objectql/src/engine-cascade-delete-probe-failure.test.ts

Real ObjectQL engine plus a stub driver with a per-object read-failure injector. Every expectation is written against literals (the injected error object itself, its literal message, the literal DELETE_RESTRICTED / 409 envelope, literal row counts read straight out of the stub store), never against a value re-derived from the code under test. 7 tests:

controlprobe RUNS, finds a dependent: code === 'DELETE_RESTRICTED', status === 409, both rows survive
controlprobe RUNS, finds nothing: the delete goes through
fixprobe fails ECONNRESET: the delete rejects with that exact error object, parent and child both still present
fixprobe fails with the Postgres superstring shape column "amount" of relation "opp" does not exist (42703) — a legal missing-table phrase as a substring — and still surfaces
fixprobe fails on a cascade relation: surfaces; no parent deleted over unread children
benignno such table: opp (sqlite phrasing): the delete proceeds
benignrelation "opp" does not exist (42P01): the delete proceeds

The two controls are what make a vacuous pass impossible on the refusal assertions, and each benign test additionally asserts the injected throw actually fired (readCalls contains the child) — otherwise "the delete succeeded" would also be consistent with a harness that stopped probing.

Reverse verification (fix committed first, ablated with git restore --source=HEAD~1, direction predicted before running: red on the three surfacing tests, green on controls and benign). Observed exactly that — Tests 3 failed | 4 passed, each failure reading expected true to be Error: …, i.e. pre-fix the delete resolved with true: the silent success this card is about. Fix restored via git restore --source=HEAD --staged --worktree and re-verified green.

Verification — all at final head cc9ad86a (post-merge of origin/main)

pnpm --filter @objectstack/objectql test 212 files / 3728 tests passed
pnpm --filter @objectstack/objectql typecheck tsc --noEmit, exit 0
new pin file, isolated 7 passed (7)

Gate union re-derived with node scripts/pm/dispatch-gates.mjs against the actual changed paths and run at the final head — all PASS:

check:durability-log-level · check:stack-collection-maps · scripts/check-engine-split-ratio.mjs · check:nul-bytes · added beyond the dispatch list (convention-triggered by the new test file): check:query-options-erasure · check:engine-double-contract · check:where-matcher · check:type-check-coverage.

Two of those were worth the run rather than the assumption: check:where-matcher discovers the pin's hand-written WHERE matcher (243 matchers without the file, 244 with it, all judged conformant), and the pin's fixtures are typed ServiceObject and registered with their packageId so the file contributes 0 raw errors to @objectstack/objectql's TEST_DEBT ledger — measured directly, since check:type-check-debt --re-measure needs the whole built workspace closure and is CI's to run.

Note on the gate that cannot see this fix.check:durability-log-level's read-seam census reports byte-identical numbers before and after (67 read seam(s) … 7 answer on a type-discriminated benign branch): the benign branch still exits valuelessly (continue), so neither invention criterion sees it. That is the documented #8845 blind spot behaving as its header says, and the reason this seam had to be fixed on its own terms.

Out of scope, filed instead of widened


Generated by Claude Code

@vercel

vercelBot commented Aug 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 16, 2026 5:33am

Request Review

@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.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 16, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 16, 2026 05:57
@os-zhuang
os-zhuang added this pull request to the merge queueAug 16, 2026
Merged via the queue into main with commit a751f7dAug 16, 2026
27 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-8895-cascade-delete-fail-open branch August 16, 2026 06:15
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