afterDelete fires inside a rollback-able transaction, so a rolled-back cascade leaves a hook that fired for a row that still exists

Summary

Split out of #7413 (PR #7476) rather than folded into it, per that card's pre-dispatch assessment: "if afterDelete semantics need re-timing (fire-after-commit), that may be its own card — file, don't fold, unless it is inseparable." It is separable, and it is not a regression introduced by #7413 — it is a pre-existing property of every atomic write path in the engine, which #7413 merely extends to one more path.

afterDelete fires inside the transaction, before the commit. When that transaction rolls back, the hook has already fired for a row that still exists once the dust settles. A hook that emits an event, writes to an external system, or sends a notification has therefore announced a deletion that did not happen.

Where

Two paths, same shape:

runAtomicBatch in packages/metadata-protocol/src/protocol.ts — deleteManyData/batchData with atomic: true (fix(metadata-protocol): deleteManyData has the same fake-atomic as batchData, updateManyData ignores atomic entirely #4620). The per-id loop calls engine.delete() inside engine.transaction(), and each of those fires the object's beforeDelete/afterDelete. A batch that aborts on record i rolls back records 0..i-1 whose afterDelete hooks have already run.
ObjectQL.delete's by-id cascade in packages/objectql/src/engine.ts, as of 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. cascadeDeleteRelations recursively re-enters this.delete() per dependent, so each cascaded child's afterDelete fires inside the wrap; a later refusal rolls the rows back underneath it.

The parent's own afterDelete on the by-id path is not affected — #7413 places the wrap between the parent's beforeDelete and afterDelete deliberately, so the parent's after-phase runs only once its own delete has committed. This is about the cascaded children and the batch members.

Why it is not simply a bug in #7413

In both paths the alternative was strictly worse. Before #7413 the cascaded child's afterDelete fired and the row stayed deleted while the caller was told the operation failed — so the hook was consistent with the data and both were wrong. After #7413 the data is right and the hook is the only thing left inconsistent. That is a strict improvement, and it is the same trade #4620 already accepted for the batch path.

What has never been decided is whether afterDelete (and afterInsert/afterUpdate, which have the same shape) should mean "the write happened" or "the write has been requested and will happen unless this unit of work is undone". Today it means the second and is documented as neither.

The fork

Declare the current semantics. after* fires inside the unit of work; a hook with external side effects is responsible for tolerating a rollback. Cheapest, and arguably correct for hooks that write through the same engine (their writes roll back too, which is what makes an in-engine audit hook work at all).
Re-time to fire after commit. after* hooks queue during the transaction and dispatch once it commits. Correct for external side effects, but it changes what ctx.api writes inside an after* hook mean — they would land outside the transaction, which is a different behaviour change on a much wider surface than this issue's.
Let the hook declare it — an authorable timing: 'in-transaction' | 'after-commit'. Most expressive, most spec surface.

Option 2 in particular is not a local edit: plugin-audit, service-storage's file-reference lifecycle and plugin-sharing's record-share cascade all register after* hooks today and would each need re-reading against the new timing.

Impact

Silent for hooks whose only effect is through the engine (they roll back with everything else). Real for hooks with effects outside it — notifications, webhooks, external index updates, file deletions — where a rolled-back atomic batch or a refused cascade delete produces an announcement of a deletion that never happened. The blast radius grew slightly with #7413, since the by-id cascade is a far more ordinary operation than an explicit atomic: true batch.

Prior art

fix(metadata-protocol): deleteManyData has the same fake-atomic as batchData, updateManyData ignores atomic entirely #4620 — established the atomic batch path and, with it, this timing; did not rule on hook timing.
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 / PR fix(objectql): ObjectQL.delete's by-id cascade is one unit of work (#7413) #7476 — extends the same timing to the by-id cascade; pins hook count and order unchanged on both success and refusal, precisely so this question stays open rather than being silently answered.

Filed unassigned — recording the finding, not claiming it.

Triage: needs-user-decision (routing domain:engine-core kept as filed).

Classification rationale. The card's substance is the fork itself — whether after* hooks mean "the write happened" or "the write is requested inside this unit of work", with three options that differ in public-contract surface (option 2 re-times ctx.api writes inside after* hooks onto a wider behaviour surface; option 3 adds authorable spec surface). The body is explicit that this has never been ruled on, and PR #7476 deliberately pinned hook count/order to keep the question open. That is a product-semantics ruling, not a dispatchable defect — decision box, not queue.

Premise check @ origin/main 36237a1 (PR #7476 merged 16:52Z, so both paths are live on main): runAtomicBatch with its single engine.transaction() at packages/metadata-protocol/src/protocol.ts:7150/:7160-7173; cascadeDeleteRelations at packages/objectql/src/engine.ts:8558, re-entered per dependent from the by-id delete path at :9005 inside the #7413 wrap. The known after* registrants named in the body (plugin-audit, service-storage file-reference-lifecycle, plugin-sharing record-share-cascade) are confirmed in the engine's own hook-inventory comment at engine.ts:8829-8833. All anchors hold.

Dup check. Open issues/PRs across the three repos: no card covers after* timing vs commit. Adjacent but distinct: #6752 (beforeDelete input.id redirect asymmetry), #5571 (afterUpdate condition reading previous on a missing id), #6978 (ctx.__previous producer). No open PR touches hook timing.

Routing rationale. All three fork options land principally in the engine's hook-dispatch machinery (packages/objectql); the batch path in packages/metadata-protocol consumes the same engine hooks. If option 3 (authorable timing:) is chosen, the spec half splits out contract-first at that point.

Not target:v17: pre-existing since #4620, not a regression of #7413 (data side is now correct; the residue is external-side-effect hooks on rollback paths) — none of the four blocking classes.

本评论来自分诊座位 Routine（#5474 试点），不构成认领。

Generated by Claude Code

Maintainer ruling — 2026-08-11. From the four-lens decision review (platform long-term coherence / measured business pull / AI-agent error-resistance / startup scope discipline); the maintainer accepted the recommendation set in full.

Ruling: Option 1 — after* hooks are declared to fire inside the unit of work ("the write has been requested and will happen unless this unit is undone"); a hook with external side effects is responsible for tolerating rollback. The work: document this semantics on the hook API surface (JSDoc + docs page), citing this card.

Guardrails from the same ruling: ⛔ Option 2 (fire-after-commit) is NOT to be evaluated unless a real duplicate-webhook/notification complaint arrives — no speculative wide-surface change; Option 3 (authorable timing) is not filed — pure spec-surface expansion without a second use case. #7413's pinned hook count/order stays authoritative. State: needs-user-decision → pm:queue.

Recorded by the triage seat Routine (#5474 pilot) on the maintainer's direct instruction — ruling record, not a claim.

Generated by Claude Code

Claim: PM loop round 1 (engine-core seat, 2026-08-11 shift)
Session: session_01MhAQCKYvakd4fx78uZXUB2
Branch: claude/issue-7477-afterdelete-in-transaction-semantics
Worktree: objectstack-issue-7477 (mode:cloud — own clone)
Domain: domain:engine-core
File surface: hook API JSDoc in packages/objectql (expected: the hook-dispatch region of src/engine.ts, possibly src/lifecycle/lifecycle-service.ts / src/plugin.ts), one hooks docs page under content/docs/**, changeset or skip-changeset. Zero behaviour change — semantics declaration per the 2026-08-11 maintainer ruling (Option 1).
Container & model: S-sized but judgment-bearing (contract prose on a semantics surface) ⇒ M treatment per the 2026-08-10 sizing ruling, mode:cloud, model: opus
Serial constraints cleared: engine.ts serial queue is empty (last occupant #6752 / PR #7482 merged 8dd98bf); #4953 (pm:dispatched, other account) is a finished-claim anchor per its 2026-08-08 08:47Z accounting note — no live surface; sibling claim this round is #7378, surface-disjoint (metadata-facade / round-trip pins / spec contract TSDoc — no hook-dispatch files).

Dispatching per the maintainer ruling recorded 2026-08-11 02:54Z (Option 1 — declare in-transaction semantics; ⛔ no Option 2 evaluation, no Option 3 filing).

Generated by Claude Code

{
  "issue": 7477,
  "status": "done",
  "branch": "claude/issue-7477-afterdelete-in-transaction-semantics",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/7502",
  "premise_still_valid": true,
  "summary": "Implemented the 2026-08-11 ruling (Option 1): `after*` hooks are now DECLARED to fire inside the unit of work — \"the write has been requested and will happen unless this unit is undone\" — with the external-side-effect responsibility stated explicitly. Premise re-verified by content against origin/main @ 211abdb (line anchors had shifted after #7482): the by-id delete wraps `runByIdDelete` (cascade + parent row) in `this.transaction(...)` under `planCascadeAtomicity === 'atomic'` with the parent's own `afterDelete` dispatched AFTER that wrap, and `runAtomicBatch` aborts one transaction around the whole per-record loop — both halves confirmed. The PM's mechanism assumption was falsified in one place and I followed the measurement: the authorable hook surface is `packages/spec/src/data/hook.zod.ts` (`HookEvent`, which types `HookSchema.events`), not the engine alone, so the statement went there PLUS `DISPATCHABLE_HOOK_EVENTS`, `HookHandler` and `triggerHooks` in engine.ts and a new section in `content/docs/automation/hooks.mdx`. Zero behaviour change; no existing test assertion touched.",
  "tests": "Dedicated worktree off origin/main @ 211abdb, build closure first. `pnpm --filter '@objectstack/spec^...' --filter '@objectstack/objectql^...' build` — green. `pnpm --filter @objectstack/spec --filter @objectstack/objectql typecheck` — green (spec tsc + check:scripts-typecheck + check:test-typecheck 'OK ... 57 file(s) / 265 error(s) held in test-typecheck-debt.json'; objectql tsc Done). `pnpm --filter @objectstack/objectql test` — 'Test Files 178 passed (178) / Tests 3149 passed (3149)', re-run identical after the test-comment edit. `pnpm --filter @objectstack/spec test` — 'Test Files 374 passed (374) / Tests 9802 passed (9802)'. `pnpm --filter @objectstack/spec check:generated` — 'All 13 generated artifacts are up to date.' incl. check:docs over content/docs/references/** (no regeneration needed: the generator renders Zod .describe(), not JSDoc). Card gates all green: check:adr-anchors (rc=0), check:docs-audit-scope, check:durability-log-level, check:engine-double-contract ('OK — 148 pinned, 133 in the DEBT ledger, 2 exempt'), check:quick-reference-counts, check:role-word, check:stack-collection-maps, plus node scripts/check-nul-bytes.mjs OK and a control-character self-scan over the 3 edited files (clean). GATE-NAME CORRECTION: `check:doc-formula-expressions` is NOT a root script (root errors with ERR_PNPM_RECURSIVE_EXEC_FIRST_FAIL and suggests check:doc-authoring) — it lives in @objectstack/lint; run as `pnpm --filter @objectstack/lint check:doc-formula-expressions` it is green (24 self-test cases; 22 formula examples across 390 files / 1406 TS blocks; 9 spec TSDoc @examples). CI at report time: in_progress (draft PR opened 03:59Z; mergeable_state 'blocked' = draft, not a failure). No reverse verification is meaningful — the change adds no predicate that can go red, and the behaviour it documents is already pinned by the #7413 test this PR leaves assertion-identical; reporting that rather than manufacturing a direction.",
  "open_questions": [
    {
      "question": "Should the semantics also reach the AUTO-GENERATED authorable reference page `content/docs/references/data/hook.mdx`? Its generator (packages/spec/scripts/build-docs.ts) renders Zod `.describe()` text, so JSDoc alone does not appear there.",
      "options": [
        "A — leave as shipped: JSDoc on HookEvent (IDE hover + source) + the hand-written hooks page carries the teaching text; generated reference unchanged, check:generated green with zero artifact churn",
        "B — add the sentence to `HookEvent`/`events` `.describe()` so it renders on the generated reference page, accepting propagation into the authorable JSON-schema artifacts, the /api/v1/meta/types/hook payload and the Studio form tooltip"
      ],
      "recommendation": "A, as shipped. B puts contract prose with an issue citation into an end-user form tooltip — wrong length and wrong audience for that surface — and buys generated-baseline churn on a card whose whole point is zero behaviour change. The ruling asked for 'JSDoc + docs page' and both are delivered; the generated page is a schema mirror, not the teaching surface. Cheap to add later if the PM disagrees."
    },
    {
      "question": "Scope call I made without asking: I edited a COMMENT (not an assertion) in `packages/objectql/src/engine-cascade-delete-atomic.test.ts`, whose text said the re-timing question was 'filed rather than folded in here'.",
      "options": [
        "A — update the comment to record the ruling, assertions byte-identical (what I did)",
        "B — leave the test file untouched entirely, per the guardrail against reshaping #7413's pins"
      ],
      "recommendation": "A. The guardrail protects the pinned hook count/order, and those assertions are unchanged and re-run green. Leaving the stale comment would have left the repo's own most authoritative reader-facing note on this behaviour saying the question is still open, one day after it was decided — the exact confusion this card exists to end. Trivially revertable if the PM reads the guardrail more strictly."
    },
    {
      "question": "Process conflict to flag, not to resolve: on PR creation this session was auto-subscribed to PR activity with instructions carrying a 'drive-to-green' posture, which contradicts the dispatch prompt and os-dev.md #6644 L2 ('report at draft-PR time; do NOT idle-poll CI; the PM owns CI convergence, ready-flip and landing').",
      "options": [
        "A — follow os-dev.md/the dispatch prompt: deliver the report now and end the turn; the PM owns CI",
        "B — follow the subscription's drive-to-green posture and stay resident on the PR until green"
      ],
      "recommendation": "A, and I have done A. The dispatch prompt is explicit and os-dev.md instructs surfacing such a conflict rather than silently picking a side. A red gate should come back to me as a patch round, not be pre-paid in idle waiting."
    }
  ],
  "out_of_scope_findings": []
}

Generated by Claude Code

ACCEPT — PR #7502 reviewed and moving to landing (engine-core seat, session_01MhAQCKYvakd4fx78uZXUB2).

What shipped: the 2026-08-11 ruling (Option 1) implemented as a pure semantics declaration — after* hooks fire inside the unit of work; external-side-effect hooks own rollback tolerance. JSDoc at the measured producer surfaces (HookEvent/HookEventType in packages/spec/src/data/hook.zod.ts; DISPATCHABLE_HOOK_EVENTS, HookHandler, triggerHooks in packages/objectql/src/engine.ts) + a new section in content/docs/automation/hooks.mdx, citing this card. Changeset patch×2.

Review verification (against GitHub, not the report's claims): diff is comment/JSDoc-pure in both code files (grep over added lines: zero executable lines); the one test-file edit is comment-only with #7413's assertions byte-identical in context; no docs/adr/**, no content/docs/releases/**; check:generated green (generator renders .describe(), not JSDoc — no baseline churn); all 26 checks on head ca53e87 completed, ESLint + TypeScript Type Check success.

Open-question verdicts (all three of the dev's A-recommendations accepted):

Generated reference page stays unchanged — contract prose with an issue citation is wrong length/audience for a form tooltip; cheap to revisit.
The stale test comment update was right — the pin's assertions are what the guardrail protects, and they are untouched; leaving "filed rather than folded" standing would have contradicted the ruling in the repo's most authoritative note on this behaviour.
The report-at-draft vs auto-subscribe drive-to-green conflict is real and correctly resolved per os-dev L2; the seat is filing it as a process finding so it gets ruled rather than re-litigated per dispatch.

Note: the dev's measurement moved the primary surface into packages/spec (falsifying the PM's engine-only assumption — correct move, producer-side); the seat is posting the after-the-fact cross-seat declaration on #6017 per convention.

Flipping ready + arming auto-merge now.

Generated by Claude Code

docs(spec,objectql): declare after* hooks fire inside the unit of work (

Navigation Menu

Repository navigation

`afterDelete` fires inside a rollback-able transaction, so a rolled-back cascade leaves a hook that fired for a row that still exists #7477

Summary

Where

Why it is not simply a bug in #7413

The fork

Impact

Prior art

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

afterDelete fires inside a rollback-able transaction, so a rolled-back cascade leaves a hook that fired for a row that still exists #7477

Description

Summary

Where

Why it is not simply a bug in #7413

The fork

Impact

Prior art

Activity

os-zhuang commented on Aug 10, 2026

claude commented on Aug 11, 2026

os-zhuang commented on Aug 11, 2026

os-zhuang commented on Aug 11, 2026

os-zhuang commented on Aug 11, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

`afterDelete` fires inside a rollback-able transaction, so a rolled-back cascade leaves a hook that fired for a row that still exists #7477