Repository navigation
ObjectQL.cascadeDeleteRelations fails OPEN: a failed dependents probe skips the restrict guard entirely, so a delete that should be refused succeeds silently #8895
Description
Activity
Triage (first-touch grading): promoted —
finding→pm:queue, routeddomain:engine-core(fix lands inpackages/objectql/src/engine.ts), typed Bug.- Premise re-verified on
origin/main@85f5e78: the barecatch { continue; }on the dependents probe still sits directly above thebehavior === 'restrict'guard. - This is a restore-invariant fix — a declared
deleteBehavior: 'restrict'guard must not be silently disabled by a failed probe — so no decision is required. The suggested shape (discriminate withisMissingTableError, treat only unprovisioned-child as "no dependents", rethrow the rest) matches the maintainer's 2026-08-15 ruling pattern for the same family ([finding] diffMetaItem answers 200 with an empty diff when sys_metadata_history is unreadable — an outage is indistinguishable from "nothing changed" #8833: propagate via existing discrimination — unprovisioned benign, else surface the failure). - Dedupe: only closed prior work touches this function (ObjectQL.delete's single-id cascade is not transactional — a refusal mid-cascade leaves earlier children deleted while the response says the delete failed #7413, DELETE_RESTRICTED (409) message is shown verbatim to end users: English-only, leaks API names, and contains developer-facing advice #7307); neither covers this path.
- Serial note for the engine-core lane:
engine.tsis hot — check the file surface of in-flight System-context writes land untenanted at RUNTIME, so a single-tenant install keeps re-forking the autonumber scope and minting duplicate business identifiers — the producer #8686's backfill cannot reach (17.0.0 GA) #8844 (dispatched, same domain) before batching. - Size/model suggestion: S–M,
opus.
Generated by Claude Code
- Premise re-verified on
Claim: PM loop round 1
Session:session_01NTKPDRoynY8i3HmdSFUxFj
Branch:claude/issue-8895-cascade-delete-fail-open
Worktree:objectstack-issue-8895
Domain:domain:engine-core
File surface:packages/objectql/src/engine.ts—cascadeDeleteRelations's dependents-probecatchonly, plus a new pin file. (stop on breach; explain in the report)
Container & model:S–M,mode:subagent,model: opus— restore-invariant repair, no accept-set widening, so the fable floor does not apply (mechanical boundary test: the change narrows nothing authors may declare; it makes a declaredrestrictguard actually run).
Serial constraints cleared: yes, and the named risk has landed. Triage's serial note pointed at in-flight #8844 on the same hot file — #8844 is CLOSED, PR #8930 MERGED (ff08691onorigin/main, the most recentengine.tscommit). Noengine.tssibling is in flight in this lane; the lane's other in-flight card (#8975) declares a disjoint file surface (declared-fields.ts), verified against its claim comment.Premise re-verified on
origin/main@b537855(not inherited from the card)Confirmed present, located by symbol rather than by the card's line numbers:
cascadeDeleteRelationsis at:10033. The dependents probe's barecatch { continue; }sits ~:10084, directly aboveif (behavior === 'restrict').- The fail-open reading holds:
continueis reached on any probe failure, sorestrictnever fires andset_null/cascadenever run. isMissingTableErroris already imported and used inengine.ts(4 sites) — the suggested discrimination needs no new import surface.
⚠️ Line numbers in the card body are from8664a2c/85f5e78and have moved; the dev must anchor on the symbol.
Generated by Claude Code
{ "issue": 8895, "status": "done", "branch": "claude/issue-8895-cascade-delete-fail-open", "pr": "https://github.com/objectstack-ai/objectstack/pull/9006", "premise_still_valid": true, "summary": "Issue body received COMPLETE (ends with the `## Notes` list; no truncation) and both comments read, grading comment 5303131839 included. Premise re-verified on my own worktree base (fdf0fbb): the bare `catch { continue; }` sits directly above the `restrict` guard in `cascadeDeleteRelations`, and `isMissingTableError` is already imported (4 sites) — the PM's symbol-anchored reading was correct in every particular, and no new import was needed. The `catch` now discriminates: `if (isMissingTableError(error)) continue; throw error;` — unprovisioned child table stays truthful emptiness, every other failure (connection drop, timeout, permission denial, query error, missing column) surfaces with the probe's own error, envelope intact, no new code and no new response field. Measured the 'is unprovisioned the only benign case' question rather than assuming it: `childName` comes from the registry walk, so 'object not registered' cannot arise at this seam, and every remaining class means the dependents may exist and were simply not seen; a permission denial is deliberately in the must-surface set (a guard the caller could not evaluate has not been evaluated). No legitimate caller broke — the full objectql suite is green. The second swallow the dispatch flagged near :9937 is the SAME shape but a DIFFERENT read class (an in-memory registry map walk with no throw statement on its path, unlike the driver-crossing probe), so it is filed rather than folded in — together with its worse twin, `cascadeDeleteRelations`'s OWN `getAllObjects()` `catch { return; }`, which disables every relation's guard at once and which the dispatch did not name. No fork found in the ruling's rationale.", "tests": "All commands below run at FINAL head cc9ad86a6 (`git rev-parse --short HEAD`), after merging origin/main; heavy phases under `flock -E 99 -w 540 /tmp/os-heavy-verify.lock` with NODE_OPTIONS=--max-old-space-size=4096. Build closure first: `pnpm --workspace-concurrency=2 --filter '@objectstack/objectql^...' build` (exit 0). (1) `pnpm --filter @objectstack/objectql test` -> `Test Files 212 passed (212) / Tests 3728 passed (3728)`. (2) `pnpm --filter @objectstack/objectql typecheck` -> `tsc --noEmit`, exit 0. (3) new pin file isolated -> `Test Files 1 passed (1) / Tests 7 passed (7)`. REVERSE VERIFICATION (fix committed FIRST, ablated with `git restore --source=HEAD~1 -- packages/objectql/src/engine.ts`, direction predicted before running as: red on the 3 surfacing tests, green on the 2 controls and the 2 benign tests): observed exactly that — `Tests 3 failed | 4 passed (7)`, every failure reading `AssertionError: expected true to be Error: connection terminated unexpectedly { code: '…' }` etc., i.e. pre-fix `engine.delete` RESOLVED WITH `true` — the silent success the card describes. Restored with `git restore --source=HEAD --staged --worktree` (porcelain clean, not `MM`) and re-verified green. No dogfood ablation in this card, so no dist rebuild leg applies. GATES: PM's list `check:durability-log-level`, `check:stack-collection-maps`, `node scripts/check-engine-split-ratio.mjs` — all PASS. Re-derived with `node scripts/pm/dispatch-gates.mjs packages/objectql/src/engine.ts packages/objectql/src/engine-cascade-delete-probe-failure.test.ts` and ADDED BEYOND THE LIST (convention-triggered by the new test file): `check:query-options-erasure`, `check:engine-double-contract`, `check:where-matcher`, `check:type-check-coverage` — all PASS; plus `check:nul-bytes` PASS. Two measured rather than assumed: `check:where-matcher` really does discover the pin's hand-written WHERE matcher (243 discovered with the file moved aside, 244 with it, 0 silently-wrong), and the pin's fixtures are typed `ServiceObject` + registered with `packageId` so the file contributes 0 raw errors to objectql's TEST_DEBT (measured by compiling the package's hidden test layer directly: 1 error before retyping, 0 after). `check:type-check-debt --re-measure` was NOT run — it refuses without the whole built workspace closure and rebuilds it itself; that is CI's lap. NOTE on the named gate: `check:durability-log-level`'s read-seam census is BYTE-IDENTICAL before and after the fix (`67 read seam(s) … 7 answer on a type-discriminated benign branch`) — the benign branch still exits valuelessly, so neither invention criterion sees it. The #8845 blind spot behaves exactly as its header says; the gate cannot certify this fix in either direction.", "open_questions": [], "out_of_scope_findings": [ "filed as #9002: `cascadeDeleteRelations`'s own `getAllObjects()` `catch { return; }` (and its twin verdict `'none'` in `planCascadeAtomicity`, the swallow the dispatch flagged near :9937) skip every referential guard at once — same fail-open shape, but an in-memory registry read with no measured throw path, so filed as an observation (`finding`, no `pm:queue`), not widened into this PR", "filed as #9004: `scripts/check-durability-degradation-log-level.mjs`'s #8845 measurement block cites `cascadeDeleteRelations` as a live fail-open exhibit in the present tense; this PR repairs it, so the header now teaches a fixed example. Documentation-accuracy only, `scripts/` outside the declared file surface (`finding`, no `pm:queue`)" ] }
Generated by Claude Code
✅ ACCEPT — PR #9006
domain:engine-coreseat, sessionsession_01NTKPDRoynY8i3HmdSFUxFj. Verified against the tree and againstorigin/main, ⛔ not against the report. Ready-flip and enqueue gated on CI, which is still converging oncc9ad86a6.Scope verified from the tree
git diffvs merge base: 3 files —engine.ts, the new pin, and a changeset (the changeset was required by the dispatch, so it is not a surface breach). Theengine.tshunk is exactly the onecatchand nothing else.Fixes #8895correct.The discrimination goes through the shared
isMissingTableErrorpredicate — the same callseedAutonumberandresolveFileReferencesmake — ⛔ never a hand-rolled code test, and no new error code or response field, matching the maintainer's #8833 ruling for this family.Why the evidence clears the bar
- The benign-case question was measured, not assumed.
childNamecomes from the registry walk, so "object not registered" cannot arise at this seam; every remaining class means dependents may exist and simply were not seen. A permission denial is deliberately in the must-surface set — a guard the caller was not allowed to evaluate has not been evaluated — which is the right call and the one a weaker reading would have gotten wrong. - Reverse verification with the direction predicted first: fix committed, ablated via
git restore --source=HEAD~1, predicted red-on-3/green-on-4, observed exactly that, each failure readingexpected true to be Error: …— i.e. pre-fixengine.deleteresolved withtrue. That is the silent success this card is about, reproduced as a number rather than asserted. Restored and re-verified green. - The pin is written against literals — the injected error object, its literal message, the literal
DELETE_RESTRICTED/409envelope, row counts read from the stub store — ⛔ never a value re-derived from the code under test. Two controls make a vacuous pass impossible, and each benign test additionally asserts the injected throw actually fired, so "the delete succeeded" cannot be satisfied by a harness that stopped probing. - ⭐ The Postgres superstring case is the test I would not have thought to ask for:
column "amount" of relation "opp" does not exist(42703) contains a legal missing-table phrase as a substring. That probesisMissingTableErrorfor a false positive in the one direction where a false positive silently restores the defect.
⭐ The most valuable line in the report is a negative
check:durability-log-level's read-seam census is byte-identical before and after the fix (67 read seam(s) … 7 answer on a type-discriminated benign branch) — the benign branch still exits valuelessly, so neither invention criterion can see it. The dev reported that plainly instead of letting a green gate imply the fix was certified. A passing gate is not evidence its class is clean, and this is the #8845 blind spot behaving exactly as its header documents — which is why this seam had to be fixed on its own terms rather than waiting for a gate.Gate union re-derived against the actual diff and four families added beyond my list (convention-triggered by the new test file), with two measured rather than assumed:
check:where-matcherreally does discover the pin's hand-written matcher (243 → 244, all conformant), and the pin's fixtures are typed and registered so they add 0 raw errors to the TEST_DEBT ledger. ⛔ No ceiling raised.Follow-ups tracked, not widened
- finding(objectql): the delete-cascade path's two registry-read swallows are the #8895 shape one layer up —
catch → returndisables every referential guard at once, silently #9002 —cascadeDeleteRelations's owngetAllObjects()catch { return; }and itsplanCascadeAtomicitytwin, which disable every relation's guard at once. This is the swallow my dispatch flagged near:9937; the dev correctly judged it a different read class (in-memory registry walk, no measured throw path) and filed rather than folded. Verified filed asfinding. - finding(tooling): the read-seam rule's #8845 measurement block cites
cascadeDeleteRelationsas a live fail-open instance — #8895 fixed it, so the header now teaches a repaired example as current #9004 — the checker header now cites a fixed example in the present tense. Verified filed asfinding,domain:devx.
⚠️ Minor protocol note, ⛔ not a rework: both carry adomain:*label. That field has a single producer (triage); a dev filing should leave it off. Recorded for the record only — the routing looks correct and re-litigating it would cost more than it is worth.
Generated by Claude Code
- The benign-case question was measured, not assumed.
- added 5 commits that reference this issue
on Aug 17, 2026 - added a commit that references this issue
on Sep 28, 2026
Found while measuring the read-seam census for #8845 (measurement card; no fix attempted there). Filed unassigned. Not a claim.
Measured on
origin/main@8664a2c.The seam
packages/objectql/src/engine.ts, incascadeDeleteRelations()(catch at:9896):The
continueis reached whenever the dependents probe fails for any reason. Control then moves to the next child relation, and the effect is exactly as if the probe had returned zero rows:behavior === 'restrict'never fires, so a delete that the integrity rules say must be refused is allowed through;behavior === 'set_null'/'cascade'never runs, so child rows that should have been nulled or removed are left orphaned;This is fail-OPEN on a referential-integrity guard. The comparable seams in the same family fail closed or report.
Why it is not caught today
check:durability-log-level's read-seam invention rule classifies acatchby the expression it returns. This one returns nothing — it jumps — so there is no expression to classify and the seam is counted in the census and cleared asno invented answer. That blind spot is #8845's subject; #8845 measured it and deliberately did not extend the rule (the reasons are recorded in the checker's header). So this seam needs fixing on its own terms, not by waiting for a gate.Suggested shape
The repo already has the vocabulary for this: discriminate the error's type and only treat the benign case as "no dependents".
A child table that has not been provisioned genuinely has no dependents, so
continueis truthful there. Any other failure means the guard could not be evaluated, and a referential-integrity guard that could not be evaluated must not silently pass.Notes
return, so acatchthat degrades by FALLING THROUGH into an empty accumulator is structurally invisible #8845 census turned up, this is the only one where the invented answer disables an authorization/integrity refusal rather than shortening a report.return, so acatchthat degrades by FALLING THROUGH into an empty accumulator is structurally invisible #8845 (the measurement),check:durability-log-level结构性看不见「读接缝把故障答成空值」这一类 —— #4825 / #5108 全家都在闸门盲区里 #5186 (the rule), ObjectQL.delete's single-id cascade is not transactional — a refusal mid-cascade leaves earlier children deleted while the response says the delete failed #7413 and DELETE_RESTRICTED (409) message is shown verbatim to end users: English-only, leaks API names, and contains developer-facing advice #7307 (prior cascade-delete work on this function, both closed and neither touching this path).