Skip to content

[fix][hooks] index endpoint rules by path, and let the older hook keep it - #7901

Open
ar2rsawseen wants to merge 1 commit into
masterfrom
fix/hooks-endpoint-path-uniqueness
Open

[fix][hooks] index endpoint rules by path, and let the older hook keep it#7901
ar2rsawseen wants to merge 1 commit into
masterfrom
fix/hooks-endpoint-path-uniqueness

Conversation

@ar2rsawseen

@ar2rsawseen ar2rsawseen commented Aug 5, 2026

Copy link
Copy Markdown
Member

An api endpoint hook's path is global while the hook belongs to apps, and nothing stopped a hook on one app claiming a path already held by a hook on another. Dispatch then matched on the path alone, assigning on every match with no break, so the winner was whichever the hooks query returned last — and that query is find({"enabled": true}).toArray() with no sort, so the order is undefined.

Where the colliding hooks belong to different apps, whichever wins receives the other's callback parameters, and the default email effect sends the whole parameter object, so a webhook's payload leaves the instance to an address chosen by the other app's hook. The losing hook stops firing. Between two hooks on the same app it is simply unpredictable, and natural order is not stable across updates or compaction, so a hook can stop working with nothing logged.

Dispatch is now a lookup, not a scan

The rules are indexed by path where the rule list is already rebuilt, every refreshRulesPeriod. So a collision is resolved once per refresh rather than re-decided on every callback, and dispatch is an O(1) get. At a million callbacks a minute that removes a linear scan per request, which makes this faster than before the report rather than slower. The only added cost is one reference per hook in memory, and one extra pass over an array the refresh had just materialised anyway — next to the database query it already performs, that is noise.

Collisions resolve to the older hook

By created_at, falling back to _id for hooks predating that field, since an ObjectID's leading bytes are the creation time. That gives three properties:

  • deterministic — no dependence on query order, so no flipping after an update or compaction
  • non-breaking — the hook that has been serving a path keeps serving it
  • closes the issue — a hook created later is by definition the later claimant, so it never fires and cannot take over a path in use

Refusing both matches, which an earlier revision of this branch did, punishes the app that did nothing wrong. Serving the newest is the defect itself.

Uniqueness at save, backed by an index

Saving already refuses a path another hook holds, on create and on update, with the update excluding the hook being edited. That is a read then a write, so two concurrent saves could still slip a duplicate through; a partial unique index closes it. Partial because only api endpoint hooks have a path and every other trigger type would otherwise collide on null.

An instance that already holds a duplicate cannot build the index. That is deliberately left alone rather than special-cased: the error is logged as a warning, everything keeps working, new duplicates are still refused on save, and the existing collision resolves to the older hook at dispatch.

Verification

Nine checks on the collision resolution: the older hook wins whether it appears first or last in the array, the fallback to _id when created_at is absent, the oldest of three, distinct paths both surviving, pathless rules skipped, empty and undefined input, and that a log is emitted on collision and not otherwise. node --check and npx eslint clean.

@ar2rsawseen ar2rsawseen changed the title [fix][hooks] keep api endpoint paths unique, and refuse to guess when they are not [fix][hooks] index endpoint rules by path, and let the older hook keep it Aug 5, 2026
…p it

An api endpoint hook's path is global while the hook belongs to apps, and nothing
stopped a hook on one app claiming a path already held by a hook on another.
Dispatch then matched on the path alone and assigned on every match without
breaking, so the winner was whichever the hooks query returned last, and that
query is unsorted.

Where the colliding hooks belong to different apps, whichever wins receives the
other's callback parameters, and the default email effect sends the whole
parameter object. The losing hook stops firing. Between two hooks on the same app
it is simply unpredictable, and natural order is not stable across updates or
compaction, so a hook can stop working with nothing logged.

Dispatch is now a lookup rather than a scan. The rules are indexed by path where
the rule list is already rebuilt, every refreshRulesPeriod, so a collision is
resolved once per refresh instead of being re-decided on each callback. That
removes a linear scan per request in favour of an O(1) get, which makes this
faster than it was rather than slower.

A collision resolves by age: the older hook keeps the path. That is deterministic,
where the previous behaviour depended on query order. It also means a hook created
later cannot take over a path already in use, so whichever hook has been serving a
path continues to, and nothing that works today stops working. created_at is
absent on hooks predating it, so the id is the fallback, its leading bytes being
the creation time anyway.

Saving refuses a path another hook holds, on create and on update, with the update
excluding the hook being edited. That is a read then a write, so a partial unique
index backs it. Partial because only api endpoint hooks have a path and every
other trigger type would collide on null. An instance already holding a duplicate
cannot build the index: that is logged as a warning and otherwise left alone, so
nothing breaks, new duplicates are still refused on save, and the existing
collision resolves to the older hook.
@ar2rsawseen
ar2rsawseen force-pushed the fix/hooks-endpoint-path-uniqueness branch from 82b35a6 to 8c4ed1a Compare August 5, 2026 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant