Uh oh!
There was an error while loading. Please reload this page.
docs(objectql): audit the three unverified delete-phase hook bullets — all correct, and the audit recorded (#7933) - #8085
Conversation
…auth, plugin-sharing and service-storage (#7933) The three entries #7707 left unverified in engine.ts's `wantsPreImage` bullet list have been read against the functions that bind them (`registerIdentityWriteGuard`, `bindRecordShareCascade`, `installFileReferenceHooks`). All three match what the list claims: same events, same object-less registration, same in-handler filter. No bullet's substance changed — the audit is recorded because "never checked" and "checked and correct" read identically in prose, which is what produced #7933 in the first place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014C8pAprWdmtecFsEprZax4
…image-bullet-audit # Conflicts: # packages/objectql/src/engine.ts
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 15 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also reference the affected code. These are read-only:
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#7933
Audits the three never-verified entries of the delete-phase hook bullet list in
packages/objectql/src/engine.ts—plugin-auth,plugin-sharing,service-storage— against each package's actual current hook registrations.All three are correct as written, on both halves (hook name and which gate term they hold). No bullet's substance changed. The diff is 13 comment lines recording the audit.
Verdict per bullet
1.
plugin-authidentity-write-guardbeforeDelete— CORRECT (term 1 only).packages/plugins/plugin-auth/src/identity-write-guard.ts:199, bound byregisterIdentityWriteGuard:engine.registerHook('beforeDelete', rejectWrite('delete'), { priority: 10, packageId })— noobjectkey, so global. The handler opens by returning unlessisManaged(ctx.object), as the bullet claims. The package registers noafterDeleteat all, so "term 1 only" is right.2.
plugin-sharingrecord-share-cascadebefore+afterDelete— CORRECT (terms 1 and 2).packages/plugins/plugin-sharing/src/record-share-cascade.ts:320and:331, bound bybindRecordShareCascade, both registered withopts = { packageId: RECORD_SHARE_CASCADE_PACKAGE, priority: 190 }— noobjectkey. Both handlers open withconst t = targets(objectName); if (!t.shares && !t.links) return;, matching the bullet's named filter.3.
service-storagefile-reference-lifecyclebefore+afterDelete— CORRECT (terms 1 and 2).packages/services/service-storage/src/file-reference-lifecycle.ts:782and:801, bound byinstallFileReferenceHooks, both{ packageId: PACKAGE_ID }— noobjectkey. Both handlers open withif (activeFileFields(engine, object).length === 0) return;, matching the bullet's named filter.None of the three modules has moved or been retired.
The "global" premise, checked rather than assumed
The paragraph above the list asserts every listed hook registers with no
object. Verified from both ends:hookMatchesObject(engine.ts:1026) treats an absententry.objectas global, so an object-less registration makeshasHooksFortrue for every object.packages/pluginsandpackages/servicesis object-scoped:last-admin-guard(four, onsys_user/sys_member/sys_user_permission_set/sys_permission_set),rule-hooks(object: objectName),primary-bu-projectionandbu-tree-recompute,attachment-lifecycleandattachment-access-hooks(sys_attachment),comment-access-hooks(sys_comment), and metadata-driven record-change triggers (object: binding.object).So the list's population — the global delete-phase registrants — is complete, not merely accurate.
plugin-sharingsays the same thing independently atsharing-plugin.ts:513: "Not bound per object: the posture is judged per delete from live metadata".The gate itself was retired mid-flight
PR #7989 (issue #7867) landed during this work and retired the three-term
wantsPreImagegate: theconst wantsPreImage =expression is gone frommain, and only the prose mention survives inside its own RETIRED banner. That PR deliberately kept this bullet list, because its retirement rationale leans on it:That makes the audit worth more rather than less: the sentence the retirement stands on is exactly the claim these three bullets encode, and it had never been checked.
origin/mainis merged in here, its RETIRED banner preserved verbatim, and the audit note reframed to sit beneath it and say what it now supports.Verification
check:adr-anchors,check:durability-log-level,check:engine-double-contract,check:stack-collection-maps,check:nul-bytes— all passnode scripts/check-engine-split-ratio.mjs— report-only metric, ran cleanpnpm --filter @objectstack/objectql typecheck— cleanpnpm --filter @objectstack/objectql test— 190 files / 3376 tests passedengine.ts— cleanA first typecheck run reported
TS7006inregistry.ts— a file this PR does not touch. It was a stale-distfalse red: the merge brought incheckManagedApiMethodAffordances(from #7851) while the built closure predated it. Rebuilding withpnpm --filter '@objectstack/objectql^...' buildcleared it.mainis not broken.Comments-only diff, no test file touched, so neither repo-wide ratchet moves — this carries the
skip-changesetlabel rather than a changeset, since an empty-frontmatter one would be rejected bycheck-empty-changeset.mjs.Generated by Claude Code