test(triggers): guard the provider granularity the path route depends on (#6437)

* test(triggers): guard the provider granularity the path route depends on

The public trigger route classifies delivery by provider, not by trigger
id, so a provider owning both a polling and an HTTP trigger would have its
HTTP deliveries rejected. Sim already models dual-delivery services as two
providers (slack / slack_app), but nothing asserted it: the existing
POLLING_PROVIDERS sync assertions all still pass for a mixed provider.

* test(triggers): exempt provider-gated triggers from the granularity check

Classifying every non-polling trigger as path-delivered was too broad:
internal and app-level-ingress providers never serve the public path
route either, so mixing polling with those would have failed the
invariant despite there being no routing conflict to fix.
This commit is contained in:
Waleed
2026-08-08 13:42:29 -07:00
committed by GitHub
parent 0b016c26dd
commit 77bc8badf0
+36 -1
View File
@@ -2,7 +2,8 @@
* @vitest-environment node
*/
import { describe, expect, it } from 'vitest'
import { POLLING_PROVIDERS } from '@/triggers/constants'
import { getProviderHandler } from '@/lib/webhooks/providers'
import { isInternalTriggerProvider, POLLING_PROVIDERS } from '@/triggers/constants'
import { TRIGGER_REGISTRY } from '@/triggers/registry'
describe('POLLING_PROVIDERS sync with TriggerConfig.polling', () => {
@@ -26,6 +27,40 @@ describe('POLLING_PROVIDERS sync with TriggerConfig.polling', () => {
expect(missing, `Triggers with polling: true missing from POLLING_PROVIDERS`).toEqual([])
})
/**
* `acceptsPathWebhookDelivery` gates the whole PROVIDER, not the trigger id, so a provider that
* serves the public path route must not also own a polling trigger - membership in
* `POLLING_PROVIDERS` would 404 its real deliveries. Providers gated wholesale for a
* provider-level reason (internal, or an app-level ingress route) never serve that route, so
* mixing is harmless there and they are exempt. Split dual-delivery services into two providers
* instead, as Slack does with `slack` and `slack_app`.
*/
it('no path-delivered provider also owns a polling trigger', () => {
const byProvider = new Map<string, { polling: string[]; path: string[] }>()
for (const trigger of Object.values(TRIGGER_REGISTRY)) {
const gatedByProvider =
isInternalTriggerProvider(trigger.provider) ||
getProviderHandler(trigger.provider).ingressMode === 'provider'
if (gatedByProvider) continue
const entry = byProvider.get(trigger.provider) ?? { polling: [], path: [] }
entry[trigger.polling === true ? 'polling' : 'path'].push(trigger.id)
byProvider.set(trigger.provider, entry)
}
const mixed = [...byProvider]
.filter(([, entry]) => entry.polling.length > 0 && entry.path.length > 0)
.map(
([provider, entry]) =>
`${provider}: polling=[${entry.polling.join(', ')}] path=[${entry.path.join(', ')}]`
)
expect(
mixed,
'Split the path-delivered triggers onto their own provider - the public trigger route rejects the whole provider'
).toEqual([])
})
it('no POLLING_PROVIDERS entry lacks a polling: true trigger in the registry', () => {
const extra: string[] = []
for (const provider of POLLING_PROVIDERS) {