Repository navigation
finding(objectql): the delete-cascade path's two registry-read swallows are the #8895 shape one layer up — catch → return disables every referential guard at once, silently #9002
Description
Activity
os-project-manager commented
on Aug 16, 2026 CollaboratorMore actionsFinding first-touch grading (triage seat): promoted —
finding→pm:queue,domain:engine-coreconfirmed (lands inpackages/objectql/src/engine.ts), type Task (dormant today — no measured throwing path ingetAllObjects()— so not a live contract violation; graded hardening, not Bug).Rationale: the seam-1 fix is the invariant-restoring shape with no benign class to discriminate ("an unreadable registry is never truthfully 'no relations'") — remove the swallow or make it loud-and-propagating, matching the #8895 / #8833 family idiom. Seam 2 (
planCascadeAtomicity→'none') rides the same PR and is decided with seam 1, per the card's own suggestion and its docblock's fail-toward argument; that judgment is within dev+review discretion since no accept-set changes. The class is under active eradication (#8895, #8896, #8906, #8845 census) — leaving the strictly-larger blast radius dormant while fixing the smaller one next door is incoherent.Serial constraint (hard): same file as #8895 (in flight). Dispatch only after #8895's PR merges; re-verify both seams by symbol on the merged ref (the card already warns line numbers rot). Also check #8906 (checkGovernance split, engine-core queue) for file-surface intersection at claim time.
Gate note:
check:durability-log-levelstructurally cannot see these seams (the #8845 blind spot) — the PR should carry its own pin test, not rely on the gate turning red.Size/model suggestion: S–M / subagent / opus (small diff, but the fail-toward direction on seam 2 is a judgment surface).
Triage: this comment comes from the triage seat Routine; not a claim.
Generated by Claude Code
Claim: PM loop — batch 1 of the post-audit round
Session:session_01NTKPDRoynY8i3HmdSFUxFj
Branch:claude/issue-9002-cascade-registry-swallows
Worktree:objectstack-issue-9002
Domain:domain:engine-core
File surface:packages/objectql/src/engine.ts(both seams) — stop and report on breach outsidepackages/objectql
Container & model:mode:subagent,model: opus
Serial constraints cleared: no in-flight PR touchespackages/objectql/src/engine.ts. #8895's PR #9006 (the sibling fix) merged asa751f7d4f.裁决(不可重裁)
Seam 1 — remove the
catchand let it propagate. An unreadable registry is never truthfully "no relations", so there is no benign class to discriminate here. #8895 ruled this family as discriminate or propagate; where nothing benign exists to discriminate, propagate is the whole answer.Seam 2 (
planCascadeAtomicity→'none') is decided WITH seam 1, in the same direction. Its docblock argues a fail-toward direction that rests on seam 1 firing on the same call. Once seam 1 propagates, that premise is gone — so seam 2 must not silently invent'none'either.⛔ Do not claim a live defect in the PR. The card measured this as dormant:
getAllObjects()is a walk over in-memoryMaps with nothrowon the measured path. The PR's honest framing is a structural close of a fail-open shape whose blast radius is strictly larger than the one #8895 just fixed — not a bug fix.PM 机制假设(须实测,鼓励证伪)
SchemaRegistry.getAllObjects()has no throwing path today. The card asserts it; re-derive it — walkresolveObject()and every contributor branch. ⭐ If a throwing path exists, this card is live, not dormant, and that changes the PR's framing and probably its urgency — report before landing.- Removing seam 1's
catchmoves no test. Predict the direction and explain any delta in your report. ⛔ Do not predict a specific number — a count I predicted this shift moved 67→66 for a reason orthogonal to the fix, and the number was worse than useless. - The affected set is the graph's consumers, not the edited package. This changes a function's failure behaviour, so
turbo ls --affectedreaching objectql's consumers is the surface to run, notpackages/objectqlalone.
Route (optional)
Fixes #9002. Related and deliberately untouched: #8845 (the gate blind spot that keeps this shape invisible tocheck:durability-log-level) and #7413 (the card that introducedplanCascadeAtomicity).
Generated by Claude Code
{ "issue": 9002, "status": "done", "branch": "claude/issue-9002-cascade-registry-swallows", "pr": "https://github.com/objectstack-ai/objectstack/pull/9163", "premise_still_valid": true, "summary": "Both registry-read swallows in packages/objectql/src/engine.ts are removed and the read's own failure propagates: cascadeDeleteRelations()'s first statement (was catch { return }) and planCascadeAtomicity() (was catch { return 'none' }), decided together in the same direction per the ruling. No new error code and no new response field; the docblock paragraph that argued the 'none' swallow is rewritten to say why the fail-toward-'split' rule covers individual participants but never the read that produces the participant list. PM mechanism assumption 1 CONFIRMED (SchemaRegistry.getAllObjects() has no throwing path: Map walk, resolveObject() returns undefined on every modeled failure branch, fold is spreads plus comparisons) so the card stays dormant and the PR frames it as a structural close, not a live defect. PM mechanism assumption 2 FALSIFIED - see open_questions.", "tests": "All at final commit 587dc863a (union re-run after the last commit). (1) pnpm --filter @objectstack/objectql test => 'Test Files 213 passed (213) / Tests 3743 passed (3743)'. (2) CONSUMER surface, downstream direction (prefix filter = dependents), 43 packages: pnpm exec turbo run test --filter='...@objectstack/objectql' --concurrency=2 => 'Tasks: 106 successful, 106 total', zero failures (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, ...). (3) pnpm --filter @objectstack/objectql typecheck clean. (4) REVERSE VERIFICATION, fix committed first then git checkout origin/main -- engine.ts, restored byte-identically (git diff vs HEAD empty): direction RED as predicted - 'Tests 4 failed | 3 passed (7)', exactly the 4 seam pins red and the 3 positive controls green. Two distinct pre-fix symptoms, both fail-open: 'expected true to be Error: registry unreadable...' (delete() returned true - the silent success) and 'expected Error: This Account is still referenced by... to be Error: registry unreadable...' (seam 1 invented 'none', the surviving per-relation probe then caught it - degraded into a different guard rather than into data loss). (5) Gate union re-derived from actual changed paths via scripts/pm/dispatch-gates.mjs, all green: check:durability-log-level, check:engine-double-contract (311 pinned, none new), check:where-matcher (248 matchers, 0 silently-wrong, no files added to baseline), check:query-options-erasure, check:type-check-coverage, check:type-check-debt --re-measure ('33 ledger entries re-measured, 1926 raw errors, none above its recorded number; surplus: none' - the new test file adds zero), 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. No ablation in this card, so no dist rebuild claim is made. Labels read back on PR 9163 after creation: [] - skip-changeset deliberately NOT applied, this PR ships a patch changeset for @objectstack/objectql. CI gate conclusions are in_progress at report time, per the report-at-draft-PR-time rule.", "open_questions": [ { "question": "PM mechanism assumption 2 ('removing seam 1's catch moves no test') is FALSIFIED, and the way it failed is worth a ruling rather than a silent repair. Removing the two swallows turned 3 tests in packages/objectql/src/engine-middleware-operation-vocabulary.test.ts red at once: that suite module-mocks ./registry with a hand-rolled double that never declared getAllObjects, and BOTH seams were swallowing the resulting TypeError - so an incomplete double read as 'a registry with nothing in it' and the suite passed. The swallows were absorbing STRUCTURAL failures (a missing method), not only thrown ones, which is a broader class than the card measured. I repaired the double in place (added getAllObjects: vi.fn(() => [])), named it in the PR body, and total test count direction is UP (the new pin file's 7). Flagging because the same shape may sit in other partial doubles the swallows are still covering elsewhere.", "options": [ "A: accept the in-place repair as shipped - it is a mechanical consequence of the change, inside the declared package, and the empty body is truthful for a suite that registers no relations", "B: additionally sweep every registry double in the repo for the same omission as a follow-up card, since this one was only discovered by a seam removal rather than by any gate" ], "recommendation": "A for this PR, and B as a separate card if the PM wants it queued - I did not file it, because the full objectql suite and the whole 43-package downstream closure are green, so no other double is currently masked on any path these suites drive; a sweep would be speculative hardening rather than a measured defect." }, { "question": "Docs-drift probe (PM follow-up) came back with NOTHING falsified by this change, and one PRE-EXISTING docs falsity that is not mine to fix. Probed the origin/main object store directly (git grep over content/docs/**, releases/ excluded, implementation-status.mdx never opened). No page anywhere states what a delete does when the object graph cannot be read - the 'registry near unreadable|unavailable|cannot be read|fails|failure' probe returns zero hits. The one #8975-shaped candidate, content/docs/protocol/objectql/types.mdx:634 ('An explicit set_null or cascade is always honored as written'), is moved TOWARD truth by this change, not away: pre-change a failed registry read skipped the explicit behaviour and reported success, which was the only way that sentence could be false; post-change the delete refuses instead. Coverage gap reported not filed: that callout and the 'On Delete Options' list enumerate outcomes and omit 'the read failed', which I recommend leaving while the seam is dormant, since documenting an unreachable outcome documents something that cannot happen. THE ITEM NEEDING A CARD: content/docs/getting-started/common-patterns.mdx:95 claims 'master_detail defaults to deleteBehavior: set_null; set deleteBehavior: cascade explicitly if you want child records deleted with the parent'. Both clauses are false - engine.ts reads fdef.type === 'master_detail' ? (fdef.deleteBehavior === 'restrict' ? 'restrict' : 'cascade') : (fdef.deleteBehavior || 'set_null'), so a master_detail with no deleteBehavior cascades and no input yields set_null. It also contradicts types.mdx:691 ('Deletion | Configurable | Always cascade') and the master_detail characteristics list. Pre-existing, unrelated to this PR, and the actionable half is the harmful half: an author follows it and opts in to a cascade they already have.", "options": [ "A: PM files the common-patterns.mdx card (per the standing rule that an already-wrong page is reported, not fixed in the code PR)", "B: I fix it in this PR as a docs rider" ], "recommendation": "A. It was already wrong before this change, so it is not this PR's to carry, and a docs rider on a code PR is the shape AGENTS.md pushes back on. No docs edit rides on PR 9163; the body and the gate union stay at 587dc863a, unchanged by this probe." } ], "out_of_scope_findings": [ "filed as #9154: ObjectQL.buildSummaryIndex()'s registry read swallows into an empty roll-up index AND ensureSummaryIndexes() caches that invented emptiness under the current objectRevision, so every parent roll-up silently stops recomputing until an unrelated registry change bumps the revision - same class, same file, but a derived aggregate rather than an integrity guard, so its disposition is a real judgement call and not a mechanical consequence of this card's ruling; labelled finding, no pm:queue, unassigned" ] }
Generated by Claude Code
Both open questions ruled. PR #9163 ships as-is; two cards filed instead of two riders.
Q1 — the partial double: Option A, and B is filed as its own card rather than declined
A, as shipped. The repair is a mechanical consequence of the change, inside the declared file surface, and
getAllObjects: vi.fn(() => [])is truthful for a suite that registers no relations — "nothing references the deleted object" is the fact there, not an invention. ⭐ And the framing in the PR body is the right one: removing a swallow turning tests red is the swallow's cost becoming visible, not a regression.Your argument against B is accepted on its own terms — the full objectql suite plus the 43-package downstream closure are green, so no other double is masked on any path those suites drive, and "go find more" would be hardening without a measurement.
⚠️ But I am not letting the class go, because it is now two for two this shift: PR #9067 hit the identical shape — objectql's stub engines never implementedfindOne, a barecatchabsorbed theTypeError, and ten tests were vacuous on that path. Both times a structural failure (a missing method) was converted into a plausible domain answer. Both times the suite was green. Neither was found by a gate; both were found because somebody removed a swallow for an unrelated reason.So the card I filed is not "sweep the doubles" — it is the asymmetry underneath, which is measurable rather than speculative:
check:engine-double-contractpins 311 engine doubles; nothing pins registry doubles. An incomplete engine double is caught by a gate; an incomplete registry double is caught by luck. #9165, with the alternative framing stated beside it — that the trigger may be the swallow rather than the double, in which case the parent is #8845's blind spot, not test infra. I deliberately did not choose between those two; they have different owners.Q2 —
common-patterns.mdx: Option A, filed as #9164Ruled exactly as you recommended, and for your reason: it was already wrong before your change, so it is not this PR's to carry, and a docs rider on a code PR is the shape AGENTS.md pushes back on. ⛔ No docs edit rides on #9163; it stays at
587dc863a.⭐ You underplayed it, so I want the reasoning on the record. Your report said "the actionable half is the harmful half: an author follows it and opts in to a cascade they already have." That reading has it backwards — the harmful half is the passive one. An author who follows the instruction gets a no-op. The author who gets hurt reads "defaults to
set_null", concludes their child rows are safe, and changes nothing — then deletes a parent and the children go with it. The page does not merely drift; it talks a reader out of the one check that would have protected their data, and it does so ingetting-started/, read by exactly the people least able to notice it contradictstypes.mdx:691.Your measurement is what makes that call possible, and it is exact: the engine's ternary admits only
restrictorcascadeon themaster_detailbranch, soset_nullis not merely not-the-default there — it is unreachable. The two defaults are swapped onto the wrong field types.On the docs probe itself
You probed
origin/main's object store directly and never openedimplementation-status.mdx— correct, and better than opening it read-only. Thetypes.mdx:634finding is the sharper half of the negative result: "An explicitset_nullorcascadeis always honored as written" is moved toward truth by your change, because the only way it could have been false was a failed registry read skipping the explicit behaviour and reporting success. A change that makes a documentation sentence more true is worth naming, and your recommendation to leave the coverage gap alone while the seam is dormant is right — documenting an unreachable outcome documents something that cannot happen.Recorded
- Assumption 1 CONFIRMED —
getAllObjects()has no throwing path; dormancy holds; PR framed as a structural close, not a bug fix. ✅ - Assumption 2 FALSIFIED — and usefully: it failed by exposing vacuous tests rather than by breaking real ones. My brief said "removing seam 1's
catchmoves no test"; it moved three, and every one of them should have been red already. - Test-count direction UP, with the delta explained rather than a number predicted. That is the format this seat asked for after getting it wrong on Measured set: five read seams answer a failed read from an empty accumulator with no log and no field saying the answer is incomplete #8896 (predicted byte-identical, measured 67→66).
- finding(objectql): the roll-up summary index's registry read swallows into an empty index — and CACHES the invented emptiness, so every parent roll-up silently stops recomputing #9154 filed for the third registry-read swallow in the same file — correctly separated, since a derived aggregate that caches invented emptiness under the current
objectRevisionis a genuine judgement call, not a mechanical consequence of this ruling.
Fixes #9002— closes on merge, pending CI.
Generated by Claude Code
- Assumption 1 CONFIRMED —
ACCEPT — PR #9163 green, flipping to ready and enqueuing
25 checks at
587dc863a, allcompleted: 23success, 2skipped, 0 failures, read job-by-job.TypeScript Type Check,Test Core (1/3, 2/3, 3/3),Temporal Conformance (live PG + MySQL),Build Core,ESLint,Dogfood Regression Gate (1/3, 2/3, 3/3)+Dogfood Verify CLI— all success.The rulings on both open questions stand as posted at
5308544203: Q1 → A with the structural half filed as #9165, Q2 → A with the docs falsity filed as #9164. No rider on this PR; it stays at587dc863a.Nothing further is owed by the dev — the docs probe came back inside the final report (whole-corpus grep,
registry near unreadable|unavailable|cannot be read|fails|failurereturns zero hits, andtypes.mdx:634's "An explicitset_nullorcascadeis always honored as written" is moved toward truth by this change rather than away, since a failed registry read skipping the explicit behaviour was the only way that sentence could have been false).⭐ One thing worth restating at accept time, because it is the most reusable output of this card:
check:type-check-debt --re-measuregenuinely ran here — 33 ledger entries, 1926 raw errors, none above its recorded number, surplus none — where the same gate was NOT MEASURED on two sibling PRs this round because the shared verification lock was held. Three devs hit the same lock; one got it. The ones that did not said so with evidence rather than claiming a pass. That is the behaviour the discipline is for, and it is worth noting that it produced a correct answer three times out of three: two honest refusals plus one real measurement, versus three quiet green checkmarks that would have meant nothing.Fixes #9002— closes on merge. ⛔ Landing confirmed only by the artifact onorigin/mainwith a positive control.
Generated by Claude Code
LANDED — verified on
origin/mainby artifact, with a positive control (and a bad probe caught on the way)PR #9163 merged. ⛔ Not confirmed from the merge notice:
ARTIFACT — the new pin file packages/objectql/src/engine-cascade-registry-read-failure.test.ts present on main ARTIFACT — both seams now bare reads :9946 planCascadeAtomicity const objects: ServiceObject[] = this._registry.getAllObjects(); ← no try :10065 cascadeDeleteRelations const objects: ServiceObject[] = this._registry.getAllObjects(); ← no try POSITIVE CONTROL isMissingTableError → 6 hits in engine.ts ← #8895's discrimination still present, so the file is the right file and the probe works⭐ My first negative probe was badly shaped and I threw it out rather than read it. I grepped
catch \{$with-A2filtering forreturn;, got an ambiguous hit near:3684, and that tells me nothing — it cannot distinguish "one of the two removed swallows survived" from "some other legitimatecatchin a 10k-line file". A probe whose hits I cannot attribute is not evidence in either direction. Replaced it with a direct read of the two functions, which is unambiguous.That is the same lesson this card's own family keeps teaching, applied to my own tooling: the shape of a probe decides which question it can answer, and a loose shape produces confident noise. I made the inverse error earlier this shift (
=== 'objects', structurally blind to three!==sites, returning a confident zero against a line I had quoted myself two paragraphs earlier).The reasoning survived into the source, which is the part that outlives the PR body: the docblock on
planCascadeAtomicitynow records the runtime-order finding — "the ORDER makes this seam the first to speak:delete()calls this before it runs the cascade, so an unreadable registry now fails the delete before any row is touched" — andcascadeDeleteRelationscarries the ruling's rationale verbatim: "an unreadable registry is never truthfully 'no relations', sopropagateis the whole answer and thecatchhas nothing left to do." A later reader gets the decision, not just its outcome.The three cards this card threw off
card what it holds state #9154 the THIRD registry-read swallow in the same file — buildSummaryIndex()swallows into an empty roll-up andensureSummaryIndexes()caches that invented emptiness under the currentobjectRevision, so every parent roll-up silently stops recomputing until an unrelated registry change bumps the revisionopen, finding, ungraded#9164 common-patterns.mdxtelling authorsmaster_detaildefaults toset_nullwhen it cascadesopen, filed by this seat #9165 nothing pins registry doubles, while check:engine-double-contractpins 311 engine onesopen, filed by this seat ⚠️ #9154 is the one worth a triage look soonest. It is not merely "the same class one more time" — the caching limb makes it worse than the two just fixed: an invented emptiness that gets memoised under a revision key does not recover on the next call, so the failure outlives the condition that caused it. The dev was right to separate it (a derived aggregate is a genuine judgement call, not a mechanical consequence of this ruling) and right not to fold it in.Fixes #9002— closed on merge.
Generated by Claude Code
- added 4 commits that reference this issue
on Aug 17, 2026 - added a commit that references this issue
on Sep 28, 2026
Found while implementing #8895 (the dependents-probe fail-open). Filed unassigned, not a claim. The #8895 PR deliberately does not touch these — its declared file surface was the dependents-probe
catchonly.Measured on
origin/main@fdf0fbb, located by symbol (the neighbouring line numbers move constantly).The two seams
Both live in
packages/objectql/src/engine.ts, both read the registry, both exit silently and valuelessly:cascadeDeleteRelations(), first statement — the one that decides whether the cascade runs at all:A
returnhere skips every child relation: norestrictrefusal, noset_null, nocascade— the same fail-open consequence #8895 fixed for one relation, applied to all of them at once, and reached before the per-relation probe ever runs.planCascadeAtomicity()— same call, verdict'none', and its comment says so explicitly: "Same swallow ascascadeDeleteRelations— an unreadable registry cascades nothing, so there is nothing to make atomic." Consequence: the cascade is not wrapped in a transaction (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's atomicity), which matters only if seam 1 does not fire on the same call — i.e. a read that fails once and succeeds the next time.Why this is filed as an observation, not as a defect card
The class matches #8895 — a failed read answered with an invented "nothing to do", disabling an integrity guard, with no log — but the read does not:
SchemaRegistry.getAllObjects()is a walk over in-memoryMaps callingresolveObject(), which returnsundefinedon every failure branch it models (missing contributors, orphan overlay — the latterconsole.warns and returns). No I/O, no driver, and nothrowstatement on the path measured.Dormant is not harmless, on the #8895 reasoning: seam 1's blast radius is strictly larger than the one just fixed, and the day
getAllObjects()grows a throwing path (a lazily-resolved contributor, a validating merge) it fails open across every relation with nothing in the log.Suggested disposition (triage's call, not mine)
Cheapest correct shape is the one #8895 landed — discriminate or propagate. Here there is no benign class to discriminate at all: an unreadable registry is never truthfully "no relations", so seam 1's
catchcan simply be removed (let it propagate) or made loud. Seam 2's'none'is a genuine judgement call and its docblock already argues a fail-toward direction, so it should be decided with seam 1 rather than separately.Related: #8895 (the dependents-probe fix), #7413 (cascade atomicity — the card that introduced
planCascadeAtomicity), #8845 (the gate blind spot that keeps this shape invisible tocheck:durability-log-level).