fix(objectql)!: registerHook refuses an empty object target and a self-cancelling scope (#6573) - #7210
Merged
Merged
Conversation
…elf-cancelling scope (#6573) #4281 ("an empty target is not *no* target") was closed at the two metadata doors — `HookSchema.object`'s refine and `hook-binder.ts`'s `normalizeObjects`. `engine.registerHook`, the code door, goes through neither, so all three spellings still walked in: `object: ''` falsy => allow half skipped => registers GLOBAL (#4281's headline failure mode, on the path it never covered) `object: []` truthy, admits nothing => can never fire (ADR-0078) `object: ['']` same, via a name nothing is called (ADR-0078) #5928's exclusion face added a fourth, reached by arithmetic rather than by one bad name: an allow face fully cancelled by the exclusion face (`{ object: 'account', excludeObjects: 'account' }`). #5928 named only `''` / `['']` / `'*'`-in-excludes, so this fell outside its letter and was deliberately left registering a dead entry. All four are refused at REGISTRATION, reusing #4281's wording. The matching read of `object: ''` is deliberately UNCHANGED: teaching `hookMatchesObject` that `''` is an unmatchable name would silently turn a hook firing on every object into one firing on none — the same defect pointing the other way, which is why #5928 declined to do it in passing. Only a finite allow face can be decided for cancellation. `'*'` and an absent `object` admit an open universe (objects register into a running engine), so no enumerated exclusion list can prove them inert; the one exclusion that would — `'*'` — is already refused by #5928. `hook-exclude-objects.test.ts`'s pin of `object: ''` as preserved-not-endorsed is updated in the same change, as is the property matrix's now-unregistrable fully-cancelled scope. Refs #6573, #4281, #4001, #5928, ADR-0078 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011rfAjYFr66Mjnz89gD5CL5
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 109 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
github-actions
Bot
added
documentation
…isterhook-refusal
This was referenced Aug 10, 2026
…ep-17 ledger The #6573 changeset declares a breaking change, so ADR-0087 requires a written disposition. Three of the four are mechanically false here: `@objectstack/objectql` is published (not `unpublished`), no prior id covers this surface (not `already-registered`), and the changeset ships rewrite instructions, so `no-migration-prescription` would be a self-contradiction the gate refuses. So it is registered. The entry is `semantic`, not a conversion, for the reason `hook-context-session-roles-retired` is at the same step: this is a RUNTIME registration API, so no `sys_metadata` row exists for the D2 chain to rewrite and the ledger is the notification channel. It is also genuinely non-lossless — `object: ''` could have meant `'*'` (what it did) or a name the author left blank, and those are opposite registrations. One metadata surface reaches the break indirectly and the entry names it: a record-change flow start node forwards `config.objectName` into `registerHook` verbatim, so a flow authored with a blank objectName used to bind a trigger to every object and now fails to bind loudly instead. Artifacts regenerated (`gen:spec-changes`, `gen:upgrade-guide`). Refs #6573, #6148, ADR-0087 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011rfAjYFr66Mjnz89gD5CL5
os-zhuang
marked this pull request as ready for review
August 10, 2026 04:18
This was referenced Aug 10, 2026
This was referenced Aug 10, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #6573.
The defect
#4281 ("an empty target is not no target") was closed at the two metadata doors —
HookSchema.object's refine inpackages/spec, andnormalizeObjectsinhook-binder.ts.engine.registerHook— the code door — goes through neither, so all three spellings still walked in, and the matching read turns each into a defect:object: ''''is falsy, so the allow half is skipped entirely — #4281's headline failure mode (blank intent becoming the broadest possible blast radius), reproduced verbatim on the path that ruling never reachedobject: []object: ['']#5928's exclusion face added a fourth, reached by arithmetic rather than by one bad name:
{ object: 'account', excludeObjects: 'account' }''/['']/'*'-in-excludes, so this fell outside its letter and was deliberately leftThe fix, and the route deliberately not taken
All four are refused at registration, reusing #4281's wording. The matching read of
object: ''is left unchanged on purpose: teachinghookMatchesObjectthat''is an unmatchable name would silently turn a hook firing on every object into one firing on none — the same class of defect pointing the other way, which is exactly why #5928 declined to do it in passing. A throw at the door changes no dispatch and leaves nothing to misread.Only a finite allow face can be decided for cancellation.
object: '*'and an absentobjectadmit an open universe —applyObjectRegistryMutationregisters objects into a running engine — so no enumerated exclusion list can prove them inert, and the one exclusion that would ('*') is already refused by #5928. Partial cancellation ({ object: ['a','b'], excludeObjects: ['b'] }) is untouched — that is what the exclusion face is for.Note the deliberate asymmetry with
assertValidHookExcludeObjects, which accepts[]: on the subtract face an empty list is the honest spelling of "subtract nothing"; on the allow face it says "admit nothing", i.e. a hook that can never fire.[]is also named in #4281's own message, so accepting it here would contradict the ruling this reuses.The card grades this observation-level on the measured claim that every
registerHookcaller passes literal constants, so a refusal has zero impact today. That claim is wrong. A full sweep of all 60+ non-test call sites found three that forward a non-literalobject:packages/triggers/trigger-record-change/src/record-change-trigger.ts:253—object: binding.object, whichservice-automation'sresolveFlowTriggerfills from a flow start node'sconfig.objectName, forwarded verbatim (typeof config.objectName === 'string' ? config.objectName : undefined). A flow authored withobjectName: ''therefore reachesregisterHookwith''— and today binds a record-change trigger to every object in the tenant. This is authored metadata, not a code constant.packages/objectql/src/engine.ts:8909—ObjectQL.create({ hooks })forwards eachhook.objectthrough theon()alias, unvalidated.packages/objectql/src/hook-binder.ts:226— computed, but provably safe:normalizeObjectsfilters blank members and the caller skips a hook whose target list is empty.So the correct grade is reachable via authored flow metadata, which strengthens rather than weakens the disposition: there is a live blast-radius bug behind this door. The consequence of the refusal on that path is good and contained —
activateFlowTriggeralready wrapstrigger.start()in a per-flow try/catch that warns and leaves the flow unbound, and thekernel:bootstrappedbinding audit re-reports every unbound triggered flow. A flow with a blankobjectNamegoes from silently global to loudly unbound. Flagged for the maintainer since it makes the change a behaviour change for one authored shape rather than the zero-impact one the card assumed.ADR-0087 disposition —
registered, and why the bump staysmajorThe changeset registers a semantic entry,
hook-register-empty-object-target-refused, in the protocol-17 step of the ADR-0087 ledger.Why
registeredand not an exemption. Three of the four dispositions are mechanically false here:@objectstack/objectqlis published, sounpublishedfails outright; no prior id covers this surface, soalready-registeredfails; and the changeset ships rewrite instructions, sono-migration-prescriptionwould be the self-contradiction that category exists to refuse. Auditing the whole.changesetstock (--list) confirms the convention — every disposed changeset withprescription=yesusesregistered.Why
semanticand not a conversion.registerHookis a runtime registration API: nosys_metadatarow can carry the shape, so there is nothing for the D2 chain to rewrite. That is precisely the reasoninghook-context-session-roles-retiredcarries at this same step — a runtime hook surface with no stored form, registered as semantic. The break is also genuinely non-lossless:object: ''could have meant'*'(what it actually did) or a name the author left blank, and those are opposite registrations, so no transform can choose between them. The closest shape precedent isjob-retry-policy-constraints-tightened— a tightened constraint that turns previously-accepted values into parse-time failures.Why the bump stays
major. Repo convention is unambiguous: the #4001 campaign shipped "reject what used to be accepted" as**BREAKING**/feat(spec)!:throughout. Two concrete, observable consequences here — an externalregisterHook({ object: '' })caller now throws at boot, and a blank-objectNameflow stops binding — and@objectstack/objectqlis a published package, so the contract is with external consumers, not in-tree callers. The suggestion to considerminorrested on "no in-tree caller changes behaviour", which is the premise disproved above; even were it true, in-tree callers do not bound a published package's semver. (#5928 shipped its three sibling refusals asminor, but its stated reason was thatexcludeObjectswas brand new with no callers — that reason does not transfer to a pre-existing option.)Tests
packages/objectql/src/hook-exclude-objects.test.ts— 31 pass. Two existing pins updated in the same change, as required:object: ''pin that hook 注册契约只能表达「命中这些对象」,无法表达「全局但排除这些对象」—— #5860 因此在 plugin-audit 内无法落地 #5928 left as preserved-not-endorsed now records [观察] registerHook 的代码注册面没有 #4281 那道校验:object: '' 静默变全局;allow 与 exclude 完全抵消时静默永不触发 #6573's ruling: the matcher still reads a hand-built{ object: '' }as global, and registration refuses it, so no live entry can carry it;{ object: ['account','contact'], excludeObjects: ['account','contact'] }— a scope that can no longer be registered cannot violate the property. Its refusal is pinned in the new block instead.Reverse-verification (pre-fix
engine.tsrestored, new tests kept): predicted 9 RED / 22 GREEN, naming each case. Actual 9 failed / 22 passed, the same nine — the 4 allow-face refusals, the 4 cancel-out refusals, and the rewrittenobject: ''pin. Zero deviations.Full suites:
@objectstack/objectql2918/2918,@objectstack/spec9397/9397 (including the 91 migration-chain replays),plugin-audit143,plugin-sharing418,plugin-auth996,service-automation885,trigger-record-change56 — all pass.Gates
All 64
lint.ymlgates pass, plus all 15@objectstack/specgates re-run after the ledger entry, plus the full Check Changeset job locally (check-adr-0087-registration,check-changeset-no-major,check-empty-changeset,check-changeset-fixed— each--self-testand--base <merge-base>). Generated artifacts regenerated withgen:spec-changes/gen:upgrade-guide, outside any merge state.packages/spec, so the #6017 cross-seat declaration is a hard ACCEPT prerequisite (#6532 / #5586). Left in draft for that.Refs #6573, #4281, #4001, #5928, #6148, ADR-0078, ADR-0087.