fix(triggers): repair broken idempotency dedup across 4 webhook/polling triggers#5524
Merged
Conversation
Instantly webhook deliveries had no extractIdempotencyId, so retried deliveries were never deduped. Instantly doesn't send a delivery-id header, so key on email_id when present (unique per email/reply) and fall back to a content-based key (event_type + campaign_id + lead_email + event timestamp) otherwise. Added instantly.test.ts covering auth, event matching, formatInput/output alignment, idempotency, and the subscription lifecycle.
…lision risk extractIdempotencyId was missing entirely, so GitLab's automatic webhook retries (triggered after timeouts/5xx, up to 4 consecutive failures before temporary disable) would re-execute the workflow as a brand-new event. Added a content-derived fallback key (checkout_sha for push, object id + updated_at otherwise) since GitLab's X-Gitlab-Event-UUID delivery header isn't available to extractIdempotencyId (body-only). Also renamed the Issue Hook payload's object_attributes.type (GitLab 17.2+ work item type) to work_item_type in formatInput and exposed it in buildGitLabIssueOutputs, since 'type' collides with the TriggerOutput meta-key. Added detailed_merge_status to the merge request outputs to match the deprecation of merge_status. Added apps/sim/lib/webhooks/providers/gitlab.test.ts covering verifyAuth, matchEvent, formatInput, and extractIdempotencyId.
…gnment test The gmail_poller trigger declared its `labels` output as type: 'string' while the description and actual runtime value (email.labelIds) are a string array. Changed to type: 'json' to match the shape actually delivered, consistent with how other triggers type array-of-string fields. Added apps/sim/lib/webhooks/providers/gmail.test.ts covering gmailHandler.formatInput passthrough behavior and a regression check that every key formatInput can deliver on `email` matches a key declared in the trigger's output schema.
…cy fallback - Reuse the shared createHmacVerifier helper for Linear's signature check instead of hand-rolling header/secret validation. - Tighten the webhookTimestamp replay-protection skew from 5 minutes to 60 seconds, matching Linear's documented recommendation. - Add extractIdempotencyId as a content-based fallback (type:action:id: updatedAt) for when the Linear-Delivery header is unavailable. - Remove unused teamOutputs/stateOutputs dead code from triggers/linear/utils.ts.
…type formatInput previously spread the entire raw body, so object_attributes.type (GitLab 17.2+ work item type) was already reachable via a manual expression even though undocumented. The earlier fix renamed it to work_item_type in the delivered data too, which would silently break any pre-existing workflow referencing the raw path. Keep both keys in the delivered data — this is plain passthrough data, not schema-constrained, so there's no reserved-key collision risk in doing so; only the declared *schema* needs the work_item_type name.
…_id repeats
Instantly's is_first field ("whether this is the first event of this
type for the lead") only makes sense if the same event_type can fire
more than once for the same email_id: every open of the same email,
every click, and every reply in a thread all share one email_id
(Instantly's reply_to_uuid). Keying idempotency on email_id alone
collapsed all of those distinct, legitimate deliveries into a single
dedup slot, so every open/click/reply after the first was silently
dropped for the 7-day idempotency TTL. Append timestamp (fixed per
event occurrence, confirmed not to regenerate on retry) to keep the
key both retry-stable and unique per occurrence.
…ics unverified The prior pass tightened LINEAR_WEBHOOK_TIMESTAMP_SKEW_MS from 5 minutes to Linear's suggested 60 seconds, justified by the claim that webhookTimestamp is re-stamped fresh on every delivery/retry attempt. That claim is not stated anywhere in Linear's webhook docs (verified against the raw docs page directly, including the exact wording around retries and the webhookTimestamp field) — the docs only say it is "the time when the webhook was sent" with no mention of retry behavior. A third-party implementation guide explicitly recommends a 5-minute window instead of Linear's literal 60s suggestion, citing this same ambiguity. If webhookTimestamp is actually fixed to the original event time (not refreshed per attempt), a strict 60s window would silently and permanently reject every one of Linear's own 1hr/6hr retry deliveries following any transient outage on our side, since Linear gives up after 3 failed attempts. Idempotency dedup (Linear-Delivery header / extractIdempotencyId fallback) already prevents double-processing of replayed/retried deliveries within a wider window, so the extra replay-protection from matching the literal 60s suggestion is marginal next to the risk of dropping real business events. Reverting to the 5-minute window pending explicit confirmation from Linear on retry timestamp semantics.
…tion collisions GitLab sets checkout_sha to null on branch/tag deletion, so the extractIdempotencyId fallback (checkout_sha || after) degenerated to the all-zeros SHA for every deletion. Without ref in the key, two unrelated branch deletions in the same project produced the same idempotency key, silently dropping the second (and any subsequent) legitimate trigger execution as a false duplicate.
The prior pass changed the gmail_poller trigger's `labels` output from
'string' to 'json', which is directionally correct (labelIds is a
string array, not a string) but not the precise type. The codebase's
own PrimitiveValueType union has a dedicated 'array' variant, used by
sibling triggers with identical semantics (github issue/PR triggers,
Jira triggers all declare their `labels` field as type: 'array').
Concretely, 'json' vs 'array' diverges in generateMockValue
(apps/sim/lib/workflows/triggers/trigger-utils.ts), which backs the
auto-generated "Event Payload Example" sample payload shown in the
trigger config UI (apps/sim/triggers/index.ts, gated on
trigger.id.includes('poller') — gmail_poller qualifies). 'json' mocks
as a single generic object ({id, name, status}), actively misleading
for a field that is always an array. 'array' mocks as a list, matching
the true shape (Gmail API docs confirm labelIds is a string array on
the Message resource).
No behavioral difference in output-path validation/resolution
(isPathInSchema, generateOutputPaths) since the field declares no
items/properties either way — this is purely a type-declaration
correctness fix, verified against Gmail API docs and cross-checked
against every other output field in poller.ts (all match runtime
shape).
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview GitLab adds Instantly adds Linear adds Gmail corrects the polled Large Vitest suites cover auth, matching, formatting, idempotency edge cases, and Instantly subscription lifecycle. Reviewed by Cursor Bugbot for commit 96acfc4. Configure here. |
Contributor
Greptile SummaryThis PR fixes idempotency and schema issues across several webhook and polling triggers. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (3): Last reviewed commit: "fix(gitlab): distinguish pipeline lifecy..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…o TSDoc GitLab does not automatically retry failed webhook deliveries — a failed request only counts toward auto-disabling the webhook (4 consecutive failures = temporary disable, 40 = permanent), and re-delivery only happens via a manual "Resend Request" (UI or API), which carries the same webhook-id/Idempotency-Key/X-Gitlab-Event-UUID headers as the original delivery. Those headers are already in the shared idempotency service's allowlist and checked ahead of extractIdempotencyId, so the body-based fallback matters for the rare case those headers are stripped in transit — not for "automatic retries" as previously stated. Verified directly against docs.gitlab.com. Also replaced inline `//` comments across the gitlab/gmail/linear/ instantly provider and trigger files with TSDoc blocks on the relevant declaration where the reasoning was non-obvious, or removed them outright where they only restated adjacent code.
instantly: the timestamp-present branch was fixed for the email_id collision risk, but the no-timestamp fallback still keyed on bare email_id — same collision risk, unfixed. Return null instead so dedup is skipped rather than risking a false collision between repeat opens/ clicks/replies on the same email. linear: extractIdempotencyId cast `body` straight to a Record without checking it was actually an object first, so a JSON `null` body (no Linear-Delivery header available) would throw on `b.type` and turn a malformed payload into a webhook 500 instead of a clean skip.
Collaborator
Author
Collaborator
Author
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Found during a final backwards-compat pass, same class of bug Greptile
just caught in extractIdempotencyId: both cast body straight to a
Record without checking it was actually an object, so a genuinely
null/malformed body would throw instead of degrading gracefully.
gitlab.ts (asRecord's `|| {}`) and instantly.ts (isRecordLike checks)
already guard this everywhere; linear.ts was the one inconsistent
provider. Uses the same isRecordLike helper as instantly.ts.
…y key Pipeline Hook payloads have no updated_at field (confirmed against docs.gitlab.com — only Issue/Merge Request/Note hooks reliably include it), so every lifecycle transition of the same pipeline (pending -> running -> success/failed) collapsed onto the identical fallback key and later real transitions were skipped as duplicates. Falls back to status + finished_at/created_at when updated_at is absent.
Collaborator
Author
Collaborator
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 96acfc4. Configure here.
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
1 participant
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.
Summary
/validate-triggeraudits on 6 triggers (incidentio, notion, instantly, gitlab, gmail, linear) against each service's live API docs, then a second independent adversarial re-audit on every trigger that needed a fixextractIdempotencyIdwas entirely missing — every delivery, including retries, got a random dedup key. Fixed, then the re-audit caught thatemail_idalone isn't unique per-occurrence (Instantly can fire the same event type repeatedly against one email — repeat opens/clicks, thread replies), so a second legitimate event would've been dropped for 7 days. Fixed by appending the fixed-per-occurrence timestamp to the keyTriggerOutput.typemeta-key collision on Issue Hook payloads (GitLab 17.2+ addsobject_attributes.type). Fixed, then the re-audit found GitLab setscheckout_shato null/all-zeros on branch deletion, so the fallback key collapsed every branch deletion in a project onto one identical key — fixed by includingrefin the key. Also kept the rawtypefield alongside the newwork_item_typein the delivered data, since the old code did a full passthrough and any workflow already referencing the undocumented raw path shouldn't silently breaklabelsoutput was mistyped asstringwhen the real delivered value is always an array. Fixed to'json', then the re-audit refined it to'array'to match both the runtime shape and the codebase's own convention (github/jira triggers use'array'for the identical field)createHmacVerifierhelper. Also tightened the replay-window from 5min to 60s citing a claim about Linear'swebhookTimestampsemantics — the re-audit found that claim isn't actually verifiable from Linear's docs, and reverted it to the conservative 5-minute window given the asymmetric risk (a wrong guess could silently and permanently drop legitimate 1hr/6hr retries)buildFallbackDeliveryFingerprinthelper (used by the Ashby fix in a companion PR) — not duplicated here, already shared viaproviders/utils.tsType of Change
Testing
lib/webhooks/providers/suite (24 files)bunx turbo run type-check --forceclean (forced, non-cached — a cached run gave a false pass earlier in this workstream)bunx biome checkclean on all touched filesbun run check:api-validationpassedChecklist