feat: hosted webhooks, agent channels, and human-in-the-loop - #4344
feat: hosted webhooks, agent channels, and human-in-the-loop#4344ericallam wants to merge 11 commits into
Conversation
🦋 Changeset detectedLatest commit: f6b224b The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR introduces a hosted webhooks platform spanning core schemas/types, a new internal webhook-engine package (signature verification, filtering, signing, delivery routing, partitioning), a webhook-sources provider registry with sample payloads, database/ClickHouse/replication infrastructure, webapp persistence, services, presenters, public API routes, and dashboard UI for managing webhook endpoints and deliveries. It also adds a 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (23)
packages/cli-v3/src/dev/devSupervisor.ts-476-476 (1)
476-476: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFail dequeued runs whose worker is unavailable.
Removing
#failRunWithMissingWorkerleaves this attempt without a controller or terminal API update. It can remain stuck or be repeatedly redelivered. Restore the terminal failure call before continuing..changeset/hosted-webhook-ingress.md-8-14 (1)
8-14: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not publish this as generally available while the feature flag remains disabled.
The PR objective says hosted webhooks are unreleased and disabled by default, but these changes publish a release and public documentation that present it as available.
.changeset/hosted-webhook-ingress.md#L8-L14: defer the public release until rollout, or use the project’s preview-release mechanism.docs/docs.json#L164-L176: keep the navigation out of public docs until the feature is enabled.docs/webhooks/overview.mdx#L7-L9: clearly mark and gate the feature as preview if these docs must ship first.internal-packages/clickhouse/src/webhookDeliveries.ts-140-154 (1)
140-154: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winExclude tombstoned deliveries from grouped counts.
count(DISTINCT delivery_id)deduplicates status versions but still counts a delivery whose latest row has_is_deleted = 1. Unlike the list and total-count builders, this query skipsFINAL, so endpoint totals can be stale after deletes. UseFINALor a version-awareargMaxaggregation.Proposed fix
- "SELECT webhook_endpoint_id, count(DISTINCT delivery_id) AS count FROM trigger_dev.webhook_deliveries_v1", + "SELECT webhook_endpoint_id, count() AS count FROM trigger_dev.webhook_deliveries_v1 FINAL",internal-packages/replication/src/client.ts-450-462 (1)
450-462: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate existing publications against
publish_via_partition_root.
#validatePublicationConfiguration()checks the publication table and actions, but notpg_publication.pubviaroot. A partitioned source publication created withoutpubviaroot = truewill be reused, so replication will stream events using child-partition identity/schema instead of the intended root-table semantics. ComparepubviarootwhenpublishViaPartitionRootis enabled and fail with remediation guidance such asALTER PUBLICATION ... SET (publish_via_partition_root = true).internal-packages/webhook-sources/src/index.ts-22-27 (1)
22-27: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftMake individual samples uniquely addressable.
getSample()returns the first duplicate(provider, eventType). Selecting “Inbound image message” returns the text-message body; Zendesk’s laterticket.updatedexamples are similarly unreachable. Add a stable sample ID to manifest items and use it for lookup.Based on supplied context,
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.webhooks.samples.ts:53-57calls this function with only provider and event type.internal-packages/webhook-sources/catalog/providers.json-1-1 (1)
1-1: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winCatalog checklist contradicts the registry files' own "sample-only" doc comments for workos and zendesk.
Both
providers.jsonentries claimroundTrip/producerare done and tierfirst-class, but the corresponding registry files explicitly state, in the present tense, that the provider currently ships sample-only pending a custom verifier config — the opposite conclusion. Compare withanthropic/hubspot/twilioin the same file, which correctly downgraded tier and checklist when a similar caveat applied.
internal-packages/webhook-sources/catalog/providers.json#L903-923: either downgrade theworkosentry totier: "sample-only"with onlyregistryEntry/sampleschecked, or confirm a custom verifier + round-trip test genuinely exists and updateregistry/workos.ts's stale comment.internal-packages/webhook-sources/catalog/providers.json#L660-681: same reconciliation needed for thezendeskentry.internal-packages/webhook-sources/src/registry/workos.ts#L4-6: if the checklist is correct, update this comment — it currently reads as if verification is still pending.internal-packages/webhook-sources/src/registry/zendesk.ts#L4-8: same — update this comment if verification is in fact complete.internal-packages/webhook-sources/src/handAuthored/sentry.ts-58-60 (1)
58-60: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep
Sentry-Hook-Resourceresource-level.All four samples set the resource header to
<resource>.<action>, but Sentry delivers only the resource in the reservedSentry-Hook-Resourceheader (issueorerrorhere), while the action lives in the payload. Updateissue.created/resolved/assignedtoissueanderror.createdtoerror; also apply the same change tointernal-packages/webhook-sources/src/handAuthored/sentry.ts:114-116, 174-176, 253-255.Source: MCP tools
internal-packages/webhook-sources/src/registry/telegram.ts-17-18 (1)
17-18: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDo not use the Telegram message object as the event-type path.
Line 17 resolves to an object, not an event-type value, and is absent for most Telegram Update variants. Extend
EventTypeSourceand its consumer to support top-level-key discrimination, or provide a provider-specific extractor before enabling this entry.internal-packages/webhook-sources/src/registry/whatsapp.ts-18-18 (1)
18-18: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDo not use
fieldas WhatsApp’s event discriminant.The configured path is
"messages"for both inbound messages and status updates, so routing and filtering cannot distinguish them. Use an extractor that checksvalue.messagesversusvalue.statuses, or extend the source contract to support key-presence discrimination.apps/webapp/app/db.server.ts-270-278 (1)
270-278: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUnhandled promise rejection risk on
client.$connect().
client.$connect()at line 275 is fire-and-forget: noawait, no.catch(). This function is now the single construction point for 4 Prisma clients (main writer/reader plus the new webhook writer/reader). If any ofDATABASE_URL,DATABASE_READ_REPLICA_URL,WEBHOOK_DATABASE_URL, orWEBHOOK_DATABASE_READ_REPLICA_URLis misconfigured, the rejected connect promise goes unhandled — in modern Node this crashes the process instead of surfacing a clear, logged error. The"connected"console.log right after also prints unconditionally, independent of whether the connection actually succeeded.🛡️ Proposed fix: observe the connect promise
// connect eagerly - client.$connect(); - - console.log(`🔌 ${clientType} prisma client connected`); + client + .$connect() + .then(() => console.log(`🔌 ${clientType} prisma client connected`)) + .catch((error) => { + logger.error(`Failed to connect ${clientType} prisma client`, { + clientType, + error: error instanceof Error ? error.message : String(error), + }); + });apps/webapp/app/services/realtime/sessions.server.ts-118-125 (1)
118-125: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse
findFirstinstead offindUnique.
findSessionByExternalIdqueries withfindUnique. The composite key works equally withfindFirst, which is the required convention in this codebase.♻️ Proposed change
- return prisma.session.findUnique({ + return prisma.session.findFirst({ where: { runtimeEnvironmentId_externalId: { runtimeEnvironmentId: environment.id, externalId } }, });As per path instructions: "Always use Prisma
findFirstinstead offindUnique."Source: Path instructions
apps/webapp/app/env.server.ts-1523-1524 (1)
1523-1524: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDefault the webhook feature flags to opt-in.
entry.server.tsxtouches thewebhookEnginesingleton at startup, so the worker starts unless itsdisabledflag is"0". SinceWEBHOOK_WORKER_ENABLEDdefaults fromWORKER_ENABLED(?? "true"), existing deploys withWORKER_ENABLED=truewill start the webhook redis-worker on upgrade.WEBHOOK_INGRESS_ENABLEDalso defaults to"1". Hard-default both flags to"0"unless each feature is behind a separate rollout gate.Source: Learnings
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.webhooks.deliveries.live.ts-65-77 (1)
65-77: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAuth/scope check is skipped when no filters are supplied.
The empty-result short-circuit (line 73-75) runs before
loadProjectEnvironmentFromRequest(line 77), so a request to this route with nodeliveryIds/sinceparams never authenticates or validates org/project/env scope — it just returns200 { deliveries: [] }. Move the check after resolvingproject/environmentso the route consistently enforces auth regardless of which filters are present.🔒 Proposed fix: authenticate/scope before short-circuiting
const newDeliveriesSince = includeNewDeliveries && since !== undefined ? since : undefined; - if (deliveryIds.length === 0 && newDeliveriesSince === undefined) { - return typedjson({ deliveries: [] }); - } - const { project, environment } = await loadProjectEnvironmentFromRequest(request, params); + if (deliveryIds.length === 0 && newDeliveriesSince === undefined) { + return typedjson({ deliveries: [] }); + } + const clickhouse = await clickhouseFactory.getClickhouseForOrganization(apps/webapp/app/v3/services/createDeploymentBackgroundWorkerV4.server.ts-230-246 (1)
230-246: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winNon-
ServiceValidationErrorwebhook sync failures are silently swallowed.Unlike the schedules block just above (lines 199-228), a transient/unexpected
webhooksErrorthat isn't aServiceValidationErroris only logged — the deployment then proceeds toDEPLOYINGas if webhook sync succeeded, leavingWebhookEndpointrows stale/missing.createDeploymentBackgroundWorkerV3.server.tsdoesn't have this gap since its surrounding try/catch fails the deployment on any error.🐛 Proposed fix: fail the deployment on any webhook sync error
if (webhooksError) { logger.error("Error syncing declarative webhooks", { error: webhooksError }); if (webhooksError instanceof ServiceValidationError) { await this.#failBackgroundWorkerDeployment(deployment, webhooksError); throw webhooksError; } + + const serviceError = new ServiceValidationError("Error syncing declarative webhooks"); + await this.#failBackgroundWorkerDeployment(deployment, serviceError); + throw serviceError; }apps/webapp/app/routes/api.v1.webhooks.endpoints.$endpointId.disable.ts-26-33 (1)
26-33: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRead-after-write via replica right after a primary write can return stale status.
webhookPrisma.webhookEndpoint.updatewrites to the primary, butfindWebhookEndpointResourcereads throughApiWebhookEndpointPresenter, which querieswebhookReplica. If replication lags, the API response to this "disable" call can still show the endpoint's old status instead ofinactive.🛡️ Proposed fix
- await webhookPrisma.webhookEndpoint.update({ + const updated = await webhookPrisma.webhookEndpoint.update({ where: { id: endpoint.id }, data: { status: "INACTIVE" }, }); webhookEngine.invalidateEndpoint(endpoint.opaqueId); - return json(await findWebhookEndpointResource(authentication, params.endpointId)); + // Build the response from the primary-write result to avoid replica-lag staleness. + return json(toApiEndpointFromPrimary(updated));Alternatively, have the presenter accept an explicit Prisma client so this route can pass
webhookPrismafor a guaranteed-fresh read.apps/webapp/app/presenters/v3/ApiWebhookDeliveryPresenter.server.ts-92-105 (1)
92-105: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPagination cursor/direction disagree when both
page[after]andpage[before]are supplied.
cursorresolvespage[after]first, butdirectionresolvespage[before]first. If both are present, the request is sent asdirection: "backward"while using thepage[after]value as the cursor — an inconsistent pair. Per the established convention,page[before]should win for both the cursor value and the direction.🐛 Proposed fix
- cursor: searchParams["page[after]"] ?? searchParams["page[before]"], + cursor: searchParams["page[before]"] ?? searchParams["page[after]"], direction: searchParams["page[before]"] ? "backward" : "forward",Based on learnings, "it is an established shared convention to allow both cursor query params
page[after]andpage[before]... When both are present,page[before]must take precedence (i.e., it should be used/wins)."Source: Learnings
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.webhooks.deliveries.$deliveryParam/route.tsx-134-175 (1)
134-175: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFix the hook call order before the early return.
useStateis currently reached only afterdeliveryis truthy, so any render transition between the missing-delivery branch and the delivery branch changes the hook order/instance count within the same component instance. Move this hook above the!deliveryreturn.apps/webapp/app/routes/api.v1.webhooks.endpoints.$endpointId.enable.ts-26-33 (1)
26-33: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRoute the response through the primary webhook client.
endpoint.update()is performed withwebhookPrisma, but the returned resource is built fromfindWebhookEndpointResource, whoseApiWebhookEndpointPresenterreads viawebhookReplica. Replica lag can return the previousPAUSEDstatus immediately after enabling; build the response from the updatedendpointrow or re-fetch viawebookPrisma.apps/webapp/app/routes/api.v1.webhooks.endpoints.$endpointId.rotate-secret.ts-37-47 (1)
37-47: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftSecret rotation isn't fail-safe: a mid-request failure can permanently strand the endpoint.
secretKey(Line 38) is deterministic per endpoint, sosetSecret(Line 39) overwrites the endpoint's live signing secret immediately — before the caller has received the response at Line 47. Sinceprisma(secret store) andwebhookPrisma(endpoint row) are separate databases, these writes cannot be made atomic with a single$transaction. If the process fails between line 39 and line 47 (network drop, thewebhookPrisma.webhookEndpoint.updatethrowing, etc.), the new secret is already live, the caller never received it, and there is no way to retrieve it again — the endpoint is effectively bricked until another rotation, which carries the same risk.🔒 Suggested fix: version the key, swap the pointer, don't overwrite in place
- const secret = `whsec_${randomBytes(32).toString("hex")}`; - const secretKey = `webhook:signing-secret:${endpoint.id}`; - await getSecretStore("DATABASE", { prismaClient: prisma }).setSecret(secretKey, { secret }); - await webhookPrisma.webhookEndpoint.update({ - where: { id: endpoint.id }, - data: { signingSecretKey: secretKey }, - }); + const secret = `whsec_${randomBytes(32).toString("hex")}`; + const secretKey = `webhook:signing-secret:${endpoint.id}:${randomBytes(8).toString("hex")}`; + await getSecretStore("DATABASE", { prismaClient: prisma }).setSecret(secretKey, { secret }); + // Only swap the pointer after the new secret is durably stored; the old secret + // (and old key) remain valid verification material until this succeeds. + await webhookPrisma.webhookEndpoint.update({ + where: { id: endpoint.id }, + data: { signingSecretKey: secretKey }, + }); + // TODO: schedule deletion of the previous secretKey after a grace period.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.webhooks._index/route.tsx-116-133 (1)
116-133: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPresenter failures are silently swallowed as "no deliveries."
.catch(() => ({ deliveries: [], pagination: {} }))masks any ClickHouse/DB failure as an empty result with no logging. Users see "No deliveries for this webhook yet" during an actual outage, and there's no signal for on-call to notice the failure.🩹 Suggested fix
const list = await presenter .call({...}) - .catch(() => ({ deliveries: [], pagination: {} })); + .catch((error) => { + logger.error("Failed to load webhook deliveries", { error }); + return { deliveries: [], pagination: {}, error: true }; + });Then surface
errorin the UI as a distinct state from "no data yet."internal-packages/webhook-engine/src/engine/filter/parse.ts-173-180 (1)
173-180: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winField-to-field (
valueRef) clauses silently always fail for non-comparison operators.
parseOpaccepts word operators (in,startsWith,endsWith,contains, andninvianot in), and the branch below then builds avalueRefclause for any of them when the RHS is a namespace-prefixed word. ButapplyOpRefinevaluate.tsonly implementseq/neq/gt/lt/gte/lteand returnsfalsein itsdefaultcase — so a filter likeevent.tags in event.allowedorevent.title startsWith event.repository.nameparses cleanly yet evaluates tofalsefor every delivery. For a routing/loop-guard filter this is a silent no-match that drops all events.The comment in
evaluate.ts("Non-comparison ops are rejected by the parser/types") documents the intended contract, but nothing enforces it here. Reject non-comparison ops when building avalueRefclause so authors get aFilterParseErrorinstead of a silently-dead filter.🐛 Proposed guard
const operand = peek(); if (operand?.t === "word" && NAMESPACES.has(operand.v.split(".")[0])) { next(); + if (op !== "eq" && op !== "neq" && op !== "gt" && op !== "lt" && op !== "gte" && op !== "lte") { + throw new FilterParseError( + `field-to-field comparison only supports ==, !=, >, <, >=, <= (got "${op}")` + ); + } return { kind: "clause", path, op, valueRef: operand.v }; }internal-packages/webhook-engine/src/engine/verification/hmac.ts-33-37 (1)
33-37: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject parsing failures before returning
ok: true.parseEventBody()andtryParseJson()return{ error }for non-parseable bodies, buthmac.tsandsharedSecret.tsspread that result into the success object, producingok: truewith anerrorfield and noparsedEvent. Check.errorand return the verifier’sfail(...)path before building theok: trueresult; forsharedSecret.ts, this affects the success path onplacement: "header"/bearer/basicsecrets, andbodysecrets already fail to extract when parsing fails.internal-packages/webhook-engine/src/engine/verification/index.ts-21-41 (1)
21-41: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
verify()'s call graph can throw synchronously, breaking the fail-closed contract relied on by the caller. Theengine/index.tsingest() flow explicitlysafeParses the artifact and callsverify()with notry/catch, intending that bad input always becomes{ outcome: "verification_failed" }(a 400) rather than a 5xx. Two spots inside the verify() call graph currently violate that assumption by throwing rawErrors instead of returning{ ok: false }.
internal-packages/webhook-engine/src/engine/verification/index.ts#L21-L41: return{ ok: false, error }instead of throwing for the"bundle"kind and the unregistered-schemedefaultbranch.internal-packages/webhook-engine/src/engine/verification/urlSecret.ts#L9-L17: wrapnew URL(input.url)in a try/catch and return{ ok: false, error: "invalid url", ... }on failure instead of letting it throw.
🟡 Minor comments (24)
docs/webhooks/channels.mdx-11-19 (1)
11-19: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake the examples self-contained.
docs/webhooks/channels.mdx#L11-L19: importstreamTextfromaiandanthropicfrom@ai-sdk/anthropic.docs/webhooks/human-in-the-loop.mdx#L101-L105: importwebhooksfrom@trigger.dev/sdk.As per coding guidelines, “Code examples must be complete and runnable where possible.”
Source: Coding guidelines
docs/webhooks/connect.mdx-3-3 (1)
3-3: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDescribe this as a verification credential, not always a signing secret.
webhooks.discord()uses the provider’s public key rather than a shared secret, so these universal instructions lead Discord users to configure the wrong value. Distinguish shared-secret and public-key flows.Also applies to: 15-22
internal-packages/webhook-sources/catalog/mark.ts-33-45 (1)
33-45: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winKeep
preset,tier, and checklist state consistent.
--presetaccepts arbitrary values and does not recompute the tier;--tier sample-onlycan also retain a preset. Derive tier/checklist from a validated preset, or reject contradictory flag combinations.internal-packages/webhook-sources/catalog/build-brief.md-24-24 (1)
24-24: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language tag to the TypeScript fence.
This triggers MD040.
Proposed fix
-``` +```typescriptSource: Linters/SAST tools
internal-packages/webhook-sources/catalog/build-v1.ts-33-33 (1)
33-33: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep Jira out of the
githubpreset.Jira Cloud uses
X-Hub-Signaturewithsha256=, while the catalog’sgithubpreset modelsX-Hub-Signature-256; this makes the generated tier/checklist incorrectly first-class and can break round-trip verification. Set this row’s preset tonulluntil a Jira-specific verifier is added.internal-packages/webhook-sources/catalog/status.ts-7-18 (1)
7-18: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
sampleCountis never populated, so the CLI always reports "no samples yet".
Provider.sampleCountis read at Line 83, but no entry inproviders.jsonsets this field, sop.sampleCountis alwaysundefinedand${samples}always resolves to"no samples yet"regardless of actual sample coverage — the reported status is misleading for every provider.Do you want me to wire in an actual sample count (e.g. by importing/counting `src/samples.ts` entries per provider)?🐛 Proposed fix: derive sampleCount instead of trusting an unset JSON field
-const line = (p: Provider) => { +const line = (p: Provider, sampleCounts: Record<string, number>) => { const { done, total } = dodProgress(p); const owner = p.owner ? ` owner=${p.owner}` : ""; const drift = p.status === "complete" && !derivedComplete(p) ? " [!] checklist incomplete" : ""; - const samples = p.sampleCount > 0 ? `${p.sampleCount} samples` : "no samples yet"; + const count = sampleCounts[p.id] ?? 0; + const samples = count > 0 ? `${count} samples` : "no samples yet";Also applies to: 79-87
internal-packages/webhook-sources/src/registry/postmark.ts-3-17 (1)
3-17: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPoint
docsUrlat Postmark’s general webhook page.This registry entry models Postmark delivery-related webhooks with
RecordType, but thedocsUrlsends users to the separate inbound-webhook documentation. Use the webhook overview URL so the docs match the configured event type.Suggested fix
- docsUrl: "https://postmarkapp.com/developer/webhooks/inbound-webhook", + docsUrl: "https://postmarkapp.com/developer/webhooks/webhooks-overview",internal-packages/webhook-sources/src/handAuthored/vapi.ts-20-29 (1)
20-29: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAlign epoch timestamps with the ISO timestamps.
The
timestampand artifact message times resolve to July 8, 2026, while the same call’screatedAtandupdatedAtare July 14, 2026. Regenerate the epoch-millisecond values from the July 14 timestamps so the sample is internally coherent.Also applies to: 113-121, 127-147
internal-packages/webhook-sources/src/handAuthored/brex.ts-7-10 (1)
7-10: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the extrapolated
USER_UPDATEDsample shape.
USER_UPDATEDshould include the required Brex fields:event_type,user_id,company_id, andupdated_attributes. The current sample hides required payload structure and can mislead users building filters or payloads.internal-packages/webhook-sources/src/handAuthored/openai.ts-19-19 (1)
19-19: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a distinct OpenAI event ID per sample.
The
batch.completed,response.completed, andrealtime.call.incomingsamples all reuseevt_685343a1381c819085d44c354e1b330e. OpenAI event objects useidas the unique event identifier, so consumers using this as their dedupe key can treat distinct samples as the same event.internal-packages/webhook-sources/src/handAuthored/elevenlabs.ts-18-91 (1)
18-91: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude the audio availability fields in the transcription samples.
Both
post_call_transcriptionexamples omithas_audio,has_user_audio, andHas_response_audio, which are part of the ElevenLabs post-call webhook data object. Add realistic boolean values so the catalog samples match current payload shape.Also applies to: 103-184
internal-packages/webhook-sources/src/handAuthored/linear.ts-19-21 (1)
19-21: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the stale
webhookTimestampvalues.The millisecond timestamps resolve to July 2025 while the adjacent
createdAtvalues are July 2026. This gives consumers contradictory event times.Proposed fix
- webhookTimestamp: 1751380338084, + webhookTimestamp: 1782916338084, ... - webhookTimestamp: 1751388322391, + webhookTimestamp: 1782924322391, ... - webhookTimestamp: 1751389329514, + webhookTimestamp: 1782925329514, ... - webhookTimestamp: 1751361344201, + webhookTimestamp: 1782897344201, ... - webhookTimestamp: 1751389533802, + webhookTimestamp: 1782925533802,Also applies to: 63-65, 121-124, 138-140, 173-175
internal-packages/webhook-sources/src/roundtrip.test.ts-61-64 (1)
61-64: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFail when a selected sample has no verifier config.
A sample with
presetId: "custom"or a stale provider mapping entersverifiableSamples, then passes without signing or verification because Line 64 returns. Throw instead so the catalog cannot silently lose round-trip coverage.Proposed fix
const config = configForSample(sample); - if (!config) return; + if (!config) { + throw new Error(`No verifier config for ${sample.provider} / ${sample.eventType}`); + }internal-packages/webhook-sources/src/handAuthored/close-crm.ts-222-238 (1)
222-238: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the event and payload opportunity IDs consistent.
Line 223 and Line 238 describe different opportunities, so filters based on
event.object_iddisagree with task code readingevent.data.id.Proposed fix
- id: "oppo_8H4sjNso7FyBFaeR3RXi5PMJbilfo0c6UPCxsJtEhCO", + id: "oppo_7H4sjNso7FyBFaeR3RXi5PMJbilfo0c6UPCxsJtEhCO",internal-packages/webhook-sources/src/registry/index.ts-65-67 (1)
65-67: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard the registry lookup to own keys only.
registryis a normal object, sogetProvider("toString")orgetProvider("__proto__")returns an inherited value despite theProviderRegistryEntry | undefinedreturn type. Update the generator to use an own-key check or a null-prototype map.apps/webapp/app/services/webhookDeliveriesReplicationInstance.server.ts-15-16 (1)
15-16: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse
env.DATABASE_URLinstead of readingprocess.envdirectly.
DATABASE_URLis already exposed byapp/env.server.ts, and this function already usesenvfor every other variable. Replace the rawconst { DATABASE_URL } = process.envread withenv.DATABASE_URL.Source: Path instructions
apps/webapp/app/presenters/v3/WebhookDeliveryDetailPresenter.server.ts-87-90 (1)
87-90: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse
findFirstinstead offindUnique.- this.replica.sessionRun.findUnique({ + this.replica.sessionRun.findFirst({ where: { runId: delivery.runId }, select: { session: { select: { friendlyId: true, externalId: true } } }, }),As per path instructions: "Always use Prisma
findFirstinstead offindUnique."Source: Path instructions
apps/webapp/app/presenters/v3/ApiWebhookDeliveryPresenter.server.ts-62-67 (1)
62-67: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winError message omits a valid status value.
The "Allowed" list in the validation error hardcodes
pending, processing, succeeded, failedbut omitsfiltered, even thoughfilteredis a valid key inAPI_STATUS_TO_DBand will be accepted. This misleads API consumers debugging an invalidfilter[status]value.🐛 Proposed fix
- message: `Invalid status values: ${invalid.join( - ", " - )}. Allowed: pending, processing, succeeded, failed.`, + message: `Invalid status values: ${invalid.join( + ", " + )}. Allowed: ${Object.keys(API_STATUS_TO_DB).join(", ")}.`,apps/webapp/app/components/webhookDeliveries/v1/WebhookDeliveryFilters.tsx-39-44 (1)
39-44: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStatus filter is missing the
FILTEREDoption.
deliveryStatusesonly covers 4 of the 5WebhookDeliveryStatusvalues. The delivery timeline builder (and this PR's own test suite) treatsFILTEREDas a first-class outcome, and the server-side loader already accepts it viaObject.values(WebhookDeliveryStatus)— but this dropdown has no way to select it, so users can't filter deliveries down to filtered-out events through the UI.const deliveryStatuses: { value: WebhookDeliveryStatus; title: string; color: string }[] = [ { value: "PENDING", title: "Pending", color: "`#878C99`" }, { value: "PROCESSING", title: "Processing", color: "`#3B82F6`" }, { value: "SUCCEEDED", title: "Succeeded", color: "`#28BF5C`" }, { value: "FAILED", title: "Failed", color: "`#E11D48`" }, + { value: "FILTERED", title: "Filtered", color: "`#878C99`" }, ];Since the comment says this list intentionally "Match[es] DeliveriesTable's DELIVERY_STATUS_COLOR / DELIVERY_STATUS_LABEL," worth checking whether that file (not in this batch) also needs the same addition.
apps/webapp/app/components/webhookDeliveries/v1/DeliveriesTable.tsx-240-247 (1)
240-247: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win"View session" popover item uses the run color, not the session color.
Elsewhere in this file (Line 160-161) sessions use
text-sessions; this menu item usestext-runsfor "View session", inconsistent with the established color-coding convention distinguishing sessions from runs.🎨 Fix
{sessionPath ? ( <PopoverMenuItem to={sessionPath} icon={ArrowRightIcon} - leadingIconClassName="text-runs" + leadingIconClassName="text-sessions" title="View session" /> ) : null}apps/webapp/app/components/webhookConsole/SampleSourcePicker.tsx-56-62 (1)
56-62: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCategory search is case-sensitive; label search isn't.
queryis lowercased butp.categoryis compared as-is, so a category like"Payments"won't match a lowercase search term even though the label check does lowercase.🐛 Fix
- (p) => p.label.toLowerCase().includes(query) || (p.category ?? "").includes(query) + (p) => p.label.toLowerCase().includes(query) || (p.category ?? "").toLowerCase().includes(query)packages/core/src/v3/schemas/schemas.ts-2-8 (1)
2-8: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRestore type-only imports for
RequireKeys,AnyRunTypes, and the inferredRetrieveRunResponse.These symbols are
export typeutilities/inferences, not runtime exports; importing them withouttypecan break withisolatedModules/verbatimModuleSyntax-style builds that cannot drop unused imports.
packages/core/src/v3/schemas/schemas.ts#L2-L8packages/core/src/v3/types/index.ts#L1-L3packages/core/src/v3/schemas/webhookApi.ts-64-70 (1)
64-70: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
paginationis required here but documented as optional (Line 7). A deliveries response serialized as{ data }(nopaginationkey) would failListWebhookDeliveriesResponse.parse(...). Consider making it optional to match the documented{ data, pagination? }contract and avoid a parse failure.🛡️ Proposed change
export const ListWebhookDeliveriesResponse = z.object({ data: z.array(WebhookDeliveryListItem), pagination: z.object({ next: z.string().optional(), previous: z.string().optional(), - }), + }).optional(), });internal-packages/webhook-engine/src/engine/verification/urlSecret.ts-9-17 (1)
9-17: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUnguarded
new URL(input.url)can throw on malformed input.If
input.urlisn't a valid absolute URL,new URL()throws synchronously and isn't caught here or (per the cross-fileengine/index.tsingest() evidence) by the caller, risking an unhandled 5xx instead of a fail-closed{ ok: false }result — the same fail-closed contract violated inverification/index.ts.🛡️ Proposed fix
verify(config, input): VerifierResult { const cfg = config as Extract<UrlSecretConfig, { scheme: "url-secret" }>; - const u = new URL(input.url); + let u: URL; + try { + u = new URL(input.url); + } catch { + return { ok: false, error: "invalid url", idempotencyKey: derive0(cfg, input) }; + }
🧹 Nitpick comments (13)
internal-packages/replication/src/client.ts (1)
34-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a type alias for this configuration shape.
LogicalReplicationClientOptionsis a data shape, so extend atypealias rather than aninterface.Proposed change
-export interface LogicalReplicationClientOptions { +export type LogicalReplicationClientOptions = { // ... -} +};Source: Coding guidelines
internal-packages/webhook-sources/catalog/providers.json (1)
23-923: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCatalog omits 11 registry providers that exist in code.
registry/adyen.ts,bigcommerce.ts,bitbucket.ts,checkout.ts,commercelayer.ts,monday.ts,paddlebilling.ts,paddleclassic.ts,paypal.ts,pipedrive.ts, andwoocommerce.tsall exist per the cohort's file list, but none of them have a corresponding entry in thisprovidersarray.catalog/status.tsderives "remaining"/"claimable"/"complete" counts purely from this file, so these 11 providers are silently excluded from progress tracking (the tool will report the wave as fully done while 11 shipped providers are untracked).internal-packages/webhook-sources/src/handAuthored/slack.ts (1)
17-17: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueReplace the credential-shaped Slack token with an explicit fixture value.
The shared-secret callback token is repeated in several payloads; use an unmistakable placeholder such as
"fixture-token"across these samples.Sources: MCP tools, Linters/SAST tools
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test.tasks.$taskParam/route.tsx (1)
134-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
v3WebhookTaskPathinstead of a hand-built URL.This PR adds
v3WebhookTaskPathinpathBuilder.ts(which alsoencodeURIComponents the slug); this redirect duplicates that logic with raw string interpolation instead. See consolidated comment.apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.webhooks.endpoints.$endpointParam.send.ts (1)
175-176: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
v3WebhookDeliveryPathinstead of a hand-built URL.Duplicates the new
v3WebhookDeliveryPathpathBuilder helper with raw string interpolation. See consolidated comment.apps/webapp/app/components/webhookConsole/WebhookComposer.tsx (1)
117-119: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
signatureModeis not reconciled when the selected endpoint changes.
signatureModeis initialized once fromsignedAvailable. When the user switches endpoints (dropdown at Lines 267-287) to one that is asymmetric or has no signing secret, a previously-selected"signed"mode stays set even though itsSelectItembecomes disabled, so a subsequent send submitssignedfor an unsupported endpoint. Consider resetting to"simulate"in an effect whensignedAvailablebecomes false.apps/webapp/app/v3/services/createBackgroundWorker.server.ts (1)
225-232: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNon-
ServiceValidationErrorwebhook-sync failures are swallowed (deploy reports success).Unlike
syncDeclarativeSchedulesabove (which wraps unexpected errors into aServiceValidationErrorand rethrows), a transient failure here (e.g. awebhookPrisma/prismaerror) is only logged; the deploy then completes as successful with webhook endpoints left unsynced/partially updated. Consider mirroring the schedules path so an infra failure fails the deploy rather than silently diverging routing state.apps/webapp/app/presenters/v3/WebhookDetailPresenter.server.ts (1)
447-450: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer the structured
loggeroverconsole.error(also at Line 541).
console.errorbypassesLogger.onErrorforwarding (e.g. Sentry) and structured fields used elsewhere in the presenters. Consider importingloggerfrom~/services/logger.serverfor these ClickHouse query-failure paths.apps/webapp/app/presenters/v3/ApiWebhookDeliveryPresenter.server.ts (1)
32-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate delivery-mapping logic.
ApiWebhookDeliveryPresenter.callre-implements the same field mapping already encapsulated intoApiListItem, risking silent drift if one is updated without the other.♻️ Proposed refactor
return { - id: d.friendlyId, - webhook: d.webhook?.slug ?? null, - status: DB_STATUS_TO_API[d.status], - externalDeliveryId: d.externalDeliveryId, - runId: d.run?.friendlyId ?? null, - createdAt: d.createdAt, - processedAt: d.processedAt, + ...toApiListItem(d), idempotencyKey: d.idempotencyKey, event: d.parsedEvent ?? null, headers: (d.headers as Record<string, string> | null) ?? null, rawBodyHash: d.rawBodyHash, error: d.errorMessage, filterReason: d.filterReason, updatedAt: d.updatedAt, };Note:
toApiListItemtakesWebhookDeliveryListItem;dhere is aWebhookDeliveryDetail— confirm it's structurally compatible (a superset) before applying.Also applies to: 133-148
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.webhooks.deliveries.$deliveryParam/route.tsx (1)
109-115: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate duration-formatting logic.
This local
formatDurationre-implements whatformatDurationfrom@trigger.dev/core/v3/utils/durationsalready provides (used inapps/webapp/app/components/webhookDeliveries/v1/DeliveryTimeline.tsx). Consider reusing the shared utility for consistent formatting across the delivery UI.internal-packages/webhook-engine/src/engine/types.ts (1)
28-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer a
typealias for theWebhookEngineOptionsdata shape.
WebhookEngineOptionsis a plain configuration/object shape, so the repo convention of types-over-interfaces applies (the two callbackinterfaces are behavioral contracts and are fine as-is).♻️ Convert to a type alias
-export interface WebhookEngineOptions { +export type WebhookEngineOptions = { logger?: Logger; ... deliverToSession?: DeliverWebhookToSessionCallback; -} +};As per coding guidelines: "Use types over interfaces for TypeScript".
Sources: Coding guidelines, Learnings
internal-packages/webhook-engine/src/engine/index.ts (1)
70-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the redundant
/millisecondssuffix from the duration metric.The histogram is declared with
unit: "ms"and Prometheus-exported metric names already receive the_millisecondsunit suffix, so these records are exported aswebhook_delivery_execution_duration_milliseconds_milliseconds. Rename it towebhook_delivery_execution_durationand keep the"ms"unit; update dashboards/queries accordingly.Source: Coding guidelines
internal-packages/webhook-engine/src/engine/verification/sharedSecret.ts (1)
25-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLoosely-typed
cfg: anyinextractCandidate/failSS.Unlike
hmac.ts'sfail(error, cfg: HmacConfig, input), these helpers typecfgasany, losing compile-time checks oncfg.placement,cfg.fieldName, andcfg.idempotencyFieldin secret-extraction code.♻️ Proposed fix: use the narrowed config type
-function extractCandidate(cfg: any, input: VerifyInput): string | undefined { +function extractCandidate( + cfg: Extract<SharedSecretConfig, { scheme: "shared-secret" }>, + input: VerifyInput +): string | undefined {-function failSS(error: string, cfg: any, input: VerifyInput): VerifierResult { +function failSS( + error: string, + cfg: Extract<SharedSecretConfig, { scheme: "shared-secret" }>, + input: VerifyInput +): VerifierResult {Also applies to: 50-62
65e81fc to
37d5285
Compare
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/slack
@trigger.dev/sdk
commit: |
0375d33 to
8c3eeb6
Compare
A UI pass over the webhooks dashboard, on top of #4344. No behaviour changes beyond the fixes below. ## Deliveries list - Whole row is clickable. The external delivery ID, created, processed and error cells had no link, and the target cell only linked when the delivery had a run or session, so most of each row was dead. - Dimmed "None" and "Unknown" cells now brighten with the row on hover. - The new-deliveries button sits inline, left of the pager, instead of on its own row beneath it. - The table scrolls. It was passing `stickyHeader`, which switches the table container to `overflow-visible` and stops it being the scroll container; every other list in the app leaves it off. The header stays sticky either way. - 60 deliveries per page, up from 25. Test tag uses the shared `Badge`, the webhook icon matches the Tasks page, and the Status and More filters menus drop their redundant search fields. ## Delivery detail - Dropped the duplicate status badge from the title bar; the sidebar already has a Status row. - The "nothing was captured" tab messages are centred and a size larger. - Copyable sidebar values ellipsise instead of overflowing their column, so an unbreakable hash or opaque id no longer runs past the edge. `CopyableText` gains an opt-in `truncate` prop that reserves a gutter for the copy button. - The delivery timeline's thick bar is rounded at the top. The run timeline gets that corner from the `start-cap-thick` event above its thick line, but a delivery only has two timestamps, so the line itself starts the bar and had a square top on every succeeded and failed delivery. `RunTimelineLine` gains an opt-in `roundedTop`, so other callers are unaffected. ## Navigation Webhooks was a section containing a single item. It now sits as a top-level item below Sessions, and the page is titled "Webhook deliveries". Registering the page in the favourites registry also fixes its favourite name, which was saving as "Page: Deliveries". ## Also One fix outside the UI: the delivery seed script minted `id` and `friendlyId` as two independent ids, but the detail lookup derives the row id from the friendlyId, so every seeded delivery's page reported that it could not be found.
… checks Guard the webhook console path builder against prototype-pollution keys (__proto__, constructor, prototype) flagged by static analysis. Route webhook delivery run lookups through the run store instead of the control-plane replica, matching how run tables are read elsewhere. Update @trigger.dev/slack to a type-export checker compatible with the current TypeScript, keep compiled test files out of its build, and format two JSON fixtures.
…re flag The webhook handler detail page and the deliveries-live and samples resource loaders were reachable by direct URL without the hasWebhooksAccess flag. Add the same org-flag guard the other webhook routes use, so the whole dashboard surface stays hidden until the flag is enabled (admins and impersonators still pass).
…branch Clean up unused and duplicate imports across the webhook files that oxlint flagged (they were never linted before). Add the missing `webhooks` entry to core's typesVersions so the `@trigger.dev/core/webhooks` subpath passes the package type-export check. Move the webhook nav color into the Tailwind v4 CSS theme (--color-webhooks) and drop the stale tailwind.config.js the v4 migration removed.
…ation parsing The 034 migration had an inline comment containing a semicolon. The test ClickHouse migration runner splits statements on semicolons, so it cut the CREATE TABLE mid-statement and tried to parse the comment tail as SQL, which failed ClickHouse setup for every test suite that starts a container. Removing the comment leaves the CREATE as a single statement.
…l test shard
The webhook API action routes exported action/loader via an inline destructured
`export const { action, loader } = createActionApiRoute(...)`, which the Remix
production build cannot strip, so it pulled the server-only ~/db.server into the
client bundle and failed the build. Split the export into a plain const plus a
separate `export { action, loader }`, matching the other API routes, so the
server code is removed as expected.
Also drop the extra --run from the @internal/webhook-sources test script; CI
appends its own --run and the duplicate flag failed the shard.
The slack package pinned its own vitest ^2.1.9 while the rest of the repo uses 4.1.7, so its test blob reports could not be merged with the others (the packages Merge Reports step requires a single vitest version). Drop the pin so slack inherits the root vitest like every other package.
New functionality (hosted webhooks, agent channels, human-in-the-loop), so this ships as a minor bump, not a patch.
main has since merged 036_create_queue_metrics_v1 (plus 034/035), so the branch's webhook migration collided at 036 and must sort above the current max. Goose strict mode rejects a version at or below current, which would block the deploy. Renumbered 036 to 037; the table name is unchanged.
A UI pass over the webhooks dashboard, on top of #4344. No behaviour changes beyond the fixes below. ## Deliveries list - Whole row is clickable. The external delivery ID, created, processed and error cells had no link, and the target cell only linked when the delivery had a run or session, so most of each row was dead. - Dimmed "None" and "Unknown" cells now brighten with the row on hover. - The new-deliveries button sits inline, left of the pager, instead of on its own row beneath it. - The table scrolls. It was passing `stickyHeader`, which switches the table container to `overflow-visible` and stops it being the scroll container; every other list in the app leaves it off. The header stays sticky either way. - 60 deliveries per page, up from 25. Test tag uses the shared `Badge`, the webhook icon matches the Tasks page, and the Status and More filters menus drop their redundant search fields. ## Delivery detail - Dropped the duplicate status badge from the title bar; the sidebar already has a Status row. - The "nothing was captured" tab messages are centred and a size larger. - Copyable sidebar values ellipsise instead of overflowing their column, so an unbreakable hash or opaque id no longer runs past the edge. `CopyableText` gains an opt-in `truncate` prop that reserves a gutter for the copy button. - The delivery timeline's thick bar is rounded at the top. The run timeline gets that corner from the `start-cap-thick` event above its thick line, but a delivery only has two timestamps, so the line itself starts the bar and had a square top on every succeeded and failed delivery. `RunTimelineLine` gains an opt-in `roundedTop`, so other callers are unaffected. ## Navigation Webhooks was a section containing a single item. It now sits as a top-level item below Sessions, and the page is titled "Webhook deliveries". Registering the page in the favourites registry also fixes its favourite name, which was saving as "Page: Deliveries". ## Also One fix outside the UI: the delivery seed script minted `id` and `friendlyId` as two independent ids, but the detail lookup derives the row id from the friendlyId, so every seeded delivery's page reported that it could not be found.
a0dcd78 to
f6b224b
Compare
| function capParsedEvent(event: unknown): Prisma.InputJsonValue | undefined { | ||
| if (event === undefined || event === null) return undefined; | ||
| try { | ||
| const serialized = JSON.stringify(event); | ||
| const CAP_BYTES = 8 * 1024; | ||
| if (serialized.length <= CAP_BYTES) return event as Prisma.InputJsonValue; | ||
| return { truncated: true, bytes: serialized.length } as Prisma.InputJsonValue; | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 Larger webhook payloads reach the user's code as a placeholder instead of the real event
The received event is replaced with a small "truncated" placeholder (capParsedEvent at internal-packages/webhook-engine/src/engine/index.ts:274) before it is handed to the customer's code, so any event bigger than 8KB silently arrives as { truncated: true, bytes: N } instead of the real content.
Impact: Common provider events (GitHub pushes, Stripe invoices, Slack payloads with blocks) start a run whose payload contains none of the event data, so handlers see garbage rather than failing loudly.
Why the cap is not just a display cap: the stored snapshot is the only copy, and it is what gets routed
#recordAndRoute writes parsedEvent: capParsedEvent(parsedEvent) (internal-packages/webhook-engine/src/engine/index.ts:274), and capParsedEvent (internal-packages/webhook-engine/src/engine/index.ts:819-829) replaces anything whose JSON serialization exceeds 8KB with { truncated: true, bytes }.
The comment on that function says "the full event lives in ClickHouse", but the ClickHouse table added in this PR (internal-packages/clickhouse/schema/037_create_webhook_deliveries_v1.sql) has no event/body column at all — the Postgres parsedEvent is the only copy.
The routing job then uses that stored column as the run payload: payload: delivery.parsedEvent in #handleDeliverJob (internal-packages/webhook-engine/src/engine/index.ts:604), and event: delivery.parsedEvent in #deliverToSession (internal-packages/webhook-engine/src/engine/index.ts:654). Replay (internal-packages/webhook-engine/src/engine/index.ts:386) and the console's replay picker (apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.webhooks.endpoints.$endpointParam.replay-source.ts:117-118) copy the same truncated value.
Either the routed payload must come from an uncapped copy of the verified body, or only the dashboard-facing snapshot should be capped (routing keeping the full in-memory event).
Prompt for agents
The webhook engine caps the verified event at 8KB before persisting it (capParsedEvent in internal-packages/webhook-engine/src/engine/index.ts), replacing larger bodies with `{ truncated: true, bytes }`. That same stored column is later read back as the run payload in #handleDeliverJob (`payload: delivery.parsedEvent`), in #deliverToSession (`event: delivery.parsedEvent`), and in replayDelivery, so any event over 8KB is routed to the customer's task as the placeholder object rather than the real event. The code comment claims the full event lives in ClickHouse, but the new webhook_deliveries_v1 ClickHouse table has no event/body column, so no full copy exists anywhere. Decide on one of: (a) store the full parsed event (or the raw body) somewhere durable and read the routing payload from that, or (b) keep the cap only for the dashboard-facing snapshot and pass the full in-memory event through to the deliver job (e.g. carry it on the job payload or re-derive it), and make replay explicitly fail/warn when the full event is unavailable instead of silently sending the placeholder.
Was this helpful? React with 👍 or 👎 to provide feedback.
| await webhookPrisma.webhookEndpoint.update({ | ||
| where: { id: endpoint.id }, | ||
| data: { signingSecretKey: secretKey }, | ||
| }); | ||
|
|
||
| return { success: true as const }; |
There was a problem hiding this comment.
🟡 Deliveries are rejected for up to half a minute after rotating a signing secret from the dashboard
The stored signing secret is replaced (webhookEndpoint.update at apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.webhooks.endpoints.$endpointParam/route.tsx:192-195) without telling the running service to forget the old one, so incoming events keep being checked against the previous secret until the cached copy expires.
Impact: Right after rotating or regenerating a secret in the dashboard, genuine provider deliveries are rejected with an error for up to 30 seconds.
The engine caches the resolved secret per endpoint, and only the HTTP API invalidates it
WebhookEngine.#resolveEndpoint caches { endpoint, secret, filterAst } keyed by opaqueId for WEBHOOK_ENDPOINT_CACHE_TTL_MS (default 30s) — see internal-packages/webhook-engine/src/engine/index.ts:403-441. The engine exposes invalidateEndpoint(opaqueId) for exactly this case, and the API route apps/webapp/app/routes/api.v1.webhooks.endpoints.$endpointId.rotate-secret.ts:47 calls it after rotating (as do the enable/disable routes).
The dashboard action here writes the new secret to the secret store and repoints signingSecretKey, but never calls webhookEngine.invalidateEndpoint(endpoint.opaqueId) — for either the generate-secret branch (lines 175-183) or the set/rotate branch. findEndpoint already returns opaqueId, so the fix is a one-liner in both branches.
A visible symptom: the console's test send reads the secret store directly (...webhooks.endpoints.$endpointParam.send.ts:121-124), so it signs with the new secret while the engine still verifies with the cached old one, producing a spurious verification failure.
| await webhookPrisma.webhookEndpoint.update({ | |
| where: { id: endpoint.id }, | |
| data: { signingSecretKey: secretKey }, | |
| }); | |
| return { success: true as const }; | |
| await webhookPrisma.webhookEndpoint.update({ | |
| where: { id: endpoint.id }, | |
| data: { signingSecretKey: secretKey }, | |
| }); | |
| // Drop the cached secret so verification uses the new one immediately on this instance. | |
| webhookEngine.invalidateEndpoint(endpoint.opaqueId); | |
| return { success: true as const }; |
Was this helpful? React with 👍 or 👎 to provide feedback.
| ); | ||
| const [newDeliveriesCount, setNewDeliveriesCount] = useState(0); | ||
|
|
||
| const shouldPollForNewDeliveries = hasAnyDeliveries && !isLoading && newDeliveriesCount < 100; |
There was a problem hiding this comment.
🟡 The live delivery feed never picks up the very first delivery
Automatic refreshing is switched off whenever the list is currently empty (hasAnyDeliveries gate at apps/webapp/app/components/webhookDeliveries/v1/useDeliveriesLiveReload.ts:77), so a page showing no deliveries never notices when the first one arrives.
Impact: The console tells users to "send an event to watch it arrive here", but nothing appears until they manually reload the page.
Both poll triggers are false for an empty list
shouldPoll = !isLoading && (hasActiveDeliveries || shouldPollForNewDeliveries) (apps/webapp/app/components/webhookDeliveries/v1/useDeliveriesLiveReload.ts:219).
With an empty list, activeDeliveryIdsParam is empty so hasActiveDeliveries is false, and shouldPollForNewDeliveries requires hasAnyDeliveries, which callers pass as list.deliveries.length > 0 (e.g. ConsoleFeedList at apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.webhooks.$webhookParam/route.tsx:592-601, LiveDeliveriesTable at :491-500, and the cross-endpoint list at ..._app...webhooks._index/route.tsx:188-196). So useInterval is disabled entirely and the new-deliveries check never runs.
The hasAnyDeliveries gate looks intended to avoid polling before any data exists, but the new-delivery check is exactly what an empty list needs: it just asks "are there deliveries newer than knownNewestDeliveryMs", which is seeded to Date.now() when the list is empty.
Prompt for agents
In apps/webapp/app/components/webhookDeliveries/v1/useDeliveriesLiveReload.ts, `shouldPollForNewDeliveries` requires `hasAnyDeliveries`, and callers pass `list.deliveries.length > 0`. Combined with `shouldPoll = !isLoading && (hasActiveDeliveries || shouldPollForNewDeliveries)`, an empty list disables polling completely, so the webhook console's live feed (and the deliveries list) never surfaces the first delivery without a manual reload. Consider allowing the new-deliveries check to run when the list is empty (the `since` watermark is already seeded to Date.now() in that case), or drop the `hasAnyDeliveries` condition entirely and rely on the existing 100-new-deliveries cap and pauseWhenHidden.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const loadingDeliveryId = | ||
| payloadFetcher.state !== "idle" | ||
| ? (new URLSearchParams(payloadFetcher.formAction?.split("?")[1] ?? "").get("deliveryId") ?? | ||
| undefined) | ||
| : undefined; |
There was a problem hiding this comment.
🟡 Per-row loading spinner never appears while a sample or past delivery loads
The row currently loading is worked out from a form-submission field that is never populated for plain data loads (payloadFetcher.formAction at apps/webapp/app/components/webhookDeliveries/v1/../webhookConsole/ReplaySourcePicker.tsx:38), so the spinner is never rendered for the clicked row.
Impact: Clicking a sample or a past delivery gives no visual feedback while the payload is fetched; the rows just go dim.
fetcher.formAction is only set for submissions
Both pickers call fetcher.load(url) (ReplaySourcePicker.tsx:43, SampleSourcePicker.tsx:90) and then derive the in-flight id by parsing fetcher.formAction's query string. In Remix/React Router, formAction (like formMethod/formData) is only populated for fetcher.submit()/<fetcher.Form> submissions; a load() leaves it undefined. So loadingDeliveryId (ReplaySourcePicker.tsx:36-40) and loadingEventType (SampleSourcePicker.tsx:82-86) are always undefined and the <Spinner> branches at ReplaySourcePicker.tsx:82-86 / SampleSourcePicker.tsx:164-166 are dead code.
Track the selected id in local state when the click handler fires instead, and compare that against fetcher.state !== "idle".
Prompt for agents
ReplaySourcePicker.tsx and SampleSourcePicker.tsx both compute the currently-loading row from `fetcher.formAction`, but those fetchers are driven by `fetcher.load(...)`, and `formAction` is only set for submissions — it is undefined during a load, so the per-row Spinner never renders. Replace the derivation with local state: record the clicked delivery friendlyId / sample eventType in a useState when `selectDelivery`/`selectEvent` runs, and show the spinner when that state matches the row and `fetcher.state !== "idle"`.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const deliveryStatuses: { value: WebhookDeliveryStatus; title: string; color: string }[] = [ | ||
| { value: "PENDING", title: "Pending", color: "#878C99" }, | ||
| { value: "PROCESSING", title: "Processing", color: "#3B82F6" }, | ||
| { value: "SUCCEEDED", title: "Succeeded", color: "#28BF5C" }, | ||
| { value: "FAILED", title: "Failed", color: "#E11D48" }, | ||
| ]; |
There was a problem hiding this comment.
🟡 Filtered deliveries cannot be filtered for in the dashboard
The status picker only offers four of the five delivery states (deliveryStatuses at apps/webapp/app/components/webhookDeliveries/v1/WebhookDeliveryFilters.tsx:33-38), leaving out the "Filtered" state that the table and detail page both display.
Impact: Users can see filtered deliveries in the list but can't narrow the list to them, and an existing filtered-status URL renders with a raw uppercase label.
FILTERED exists everywhere else in the surface
WebhookDeliveryStatus includes FILTERED (added by internal-packages/database/prisma/migrations/20260630120000_add_webhook_delivery_filter/migration.sql), the badge palette covers it (apps/webapp/app/components/webhookDeliveries/v1/DeliveryStatus.tsx:11), the loader accepts it (parseStatuses in ..._app...webhooks._index/route.tsx:40-48 validates against the full enum), and the API status map exposes filtered.
Because the option is missing here, statusTitleByValue.get(...) in the applied-filter chip (WebhookDeliveryFilters.tsx:269) falls back to the raw FILTERED string for any URL that already carries it.
Related: DELIVERY_STATUSES used for the delivery activity chart legend (apps/webapp/app/presenters/v3/WebhookDetailPresenter.server.ts:171) also omits FILTERED, so filtered deliveries are invisible in that chart. The API's error message for an invalid filter[status] (ApiWebhookDeliveryPresenter.server.ts:64-67) likewise doesn't mention filtered even though it is accepted.
| const deliveryStatuses: { value: WebhookDeliveryStatus; title: string; color: string }[] = [ | |
| { value: "PENDING", title: "Pending", color: "#878C99" }, | |
| { value: "PROCESSING", title: "Processing", color: "#3B82F6" }, | |
| { value: "SUCCEEDED", title: "Succeeded", color: "#28BF5C" }, | |
| { value: "FAILED", title: "Failed", color: "#E11D48" }, | |
| ]; | |
| const deliveryStatuses: { value: WebhookDeliveryStatus; title: string; color: string }[] = [ | |
| { value: "PENDING", title: "Pending", color: "#878C99" }, | |
| { value: "PROCESSING", title: "Processing", color: "#3B82F6" }, | |
| { value: "SUCCEEDED", title: "Succeeded", color: "#28BF5C" }, | |
| { value: "FAILED", title: "Failed", color: "#E11D48" }, | |
| { value: "FILTERED", title: "Filtered", color: "#64748B" }, | |
| ]; |
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (filtered) { | ||
| this.deliveryFilteredCounter.add(1); | ||
| } else { | ||
| this.deliveryEnqueueCounter.add(1); | ||
| await this.worker.enqueueOnce({ | ||
| id: `webhook-delivery:${id}`, | ||
| job: "webhook.deliver", | ||
| payload: { deliveryId: id, createdAt }, | ||
| }); | ||
| } | ||
| } catch (error) { | ||
| await this.frontGate.del(gateKey).catch(() => {}); | ||
| return { outcome: "enqueue_failed", error: String(error) }; | ||
| } | ||
|
|
There was a problem hiding this comment.
🔍 A crash between the delivery row insert and the enqueue leaves a permanently PENDING delivery
#recordAndRoute creates the WebhookDelivery row and then enqueues webhook.deliver, both inside one try/catch. If the row is created but worker.enqueueOnce throws, the catch deletes the Redis front-gate key and returns enqueue_failed (a 5xx to the provider), so the provider's retry mints a new delivery — no event is lost. But the original row stays PENDING forever with nothing to process it, and the delivery list has no way to distinguish it from an in-flight one (the detail page will poll it indefinitely via the 3s useInterval). Worth either marking it FAILED in the catch, or reaping stale PENDING rows.
Was this helpful? React with 👍 or 👎 to provide feedback.
| await webhookPrisma.webhookEndpoint.update({ | ||
| where: { id: found.id }, | ||
| data: { | ||
| source: wh.source, | ||
| routingTarget: wh.routingTarget as unknown as Prisma.InputJsonValue, | ||
| verifierArtifact: wh.verifierArtifact as unknown as Prisma.InputJsonValue, | ||
| secretProvisioning: wh.secretProvisioning ?? "either", | ||
| metadata: (wh.metadata ?? {}) as unknown as Prisma.InputJsonValue, | ||
| status: "ACTIVE", | ||
| ...filterData, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🔍 Redeploying re-activates an endpoint that was explicitly disabled via the API
syncDeclarativeWebhooks unconditionally writes status: "ACTIVE" on the update path for every declared webhook. An endpoint paused with POST /api/v1/webhooks/endpoints/:id/disable (which sets INACTIVE) will therefore silently flip back to ACTIVE on the next deploy or trigger dev sync, resuming ingress without any user action. If disable is meant to be an operator override, the sync should preserve a manually-set INACTIVE (e.g. only force ACTIVE on create, or track who disabled it).
Was this helpful? React with 👍 or 👎 to provide feedback.
| const runList = new NextRunListPresenter($replica, clickhouse) | ||
| .call(project.organizationId, environment.id, { | ||
| userId, | ||
| projectId: project.id, | ||
| tasks: [webhook.slug], | ||
| period, | ||
| from, | ||
| to, | ||
| cursor, | ||
| direction, | ||
| }) | ||
| .catch(() => null); | ||
|
|
||
| const deliveriesList = presenter | ||
| .listDeliveries({ | ||
| organizationId: project.organizationId, | ||
| projectId: project.id, | ||
| environmentId: environment.id, | ||
| webhookEndpointId: webhook.endpoint.id, | ||
| period, | ||
| from, | ||
| to, | ||
| cursor, | ||
| direction, | ||
| }) | ||
| .catch(() => null); |
There was a problem hiding this comment.
🔍 The webhook handler page shares one cursor across the Deliveries and Runs tabs
The loader reads a single cursor/direction pair from the URL and passes it to both NextRunListPresenter.call and presenter.listDeliveries. Because the tab is client-side state (useState) and doesn't clear the query params, paging the Deliveries tab and then switching to Runs applies a delivery cursor (a whd-derived composite token) to the runs query, and vice versa. At best the other list lands on an arbitrary page; both ListPagination components also render from whichever list resolved. Worth scoping the cursor per tab (e.g. deliveriesCursor / runsCursor) or clearing it on tab change.
Was this helpful? React with 👍 or 👎 to provide feedback.
| FROM latest_workers | ||
| JOIN ${sqlDatabaseSchema}."BackgroundWorkerTask" bwt ON bwt."workerId" = latest_workers.id | ||
| WHERE bwt."triggerSource" != 'AGENT' | ||
| WHERE bwt."triggerSource" NOT IN ('AGENT') |
There was a problem hiding this comment.
🔍 TestPresenter change from != 'AGENT' to NOT IN ('AGENT') is a no-op
This rewrite is semantically identical, which suggests the intent was to also exclude 'WEBHOOK' now that webhook tasks exist. Webhook tasks therefore still appear in the Test task picker; selecting one loads test.tasks.$taskParam, which now throws a redirect to the webhook console. That works, but it's an extra round-trip and the picker advertises a Test page that doesn't exist for that kind. Either add 'WEBHOOK' to the exclusion list or drop the no-op edit.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Receive and verify provider webhooks as a first-class Trigger.dev task, with no ingress route, no signature code, and no raw-body plumbing of your own. Declare a
webhook(), point it at a source preset (Stripe, GitHub, Svix, Square, Discord, and more) or a custom config, and a verified, typed event lands in youronEventhandler:That is the whole integration. We host the URL, verify the signature, record every delivery, retry on failure, and show it in the dashboard. You never register a URL by hand or hand-roll an HMAC comparison.
One verified pipe, three frontends
The same ingress that feeds
onEventalso routes a delivery to a durable session or a chat channel. Pick the frontend by which primitive you declare.A durable session, keyed per entity.
chat.eventturns a webhook into an action on a long-lived session (one per customer, per repo, per whatever the key template resolves to), handled inonAction:A chat channel. Give a
chat.agenta channel (Slack ships in the new@trigger.dev/slackpackage) and inbound messages run as turns; the agent's reply posts back to the thread:Human-in-the-loop, built in
A tool with no
executepauses the turn for a human decision. The channel posts the controls (Slack renders Approve / Deny buttons), and a verified click resumes the run right where it stopped:Filter server-side, before anything runs
A
filterexpression gates which deliveries actually start a run, so you are not paying to spin up a task just toreturnearly:Sources and verification
Presets cover the common providers and a
custom()escape hatch covers the rest. Verification is entirely config-driven, stored as data on the endpoint, so there is no per-provider verification code: HMAC, asymmetric (ed25519 / ECDSA / RSA), shared-secret, and url-secret schemes, plus a synchronous handshake for providers like Slack and Discord. A built-in provider sample library, round-trip tested against roughly 30 providers, backs the presets and the dashboard's test console.In the dashboard
Every delivery is recorded and browsable, with the verified event body, the curated request headers, the delivery status, and a link to the run it triggered. Endpoints show their URL, signing-secret state, and provider setup, and you can send a test payload or replay a past delivery.
An env-scoped HTTP API mirrors the surface: list and detail for endpoints and deliveries, rotate-secret, enable/disable, and replay.
Rollout
Off by default. The dashboard is behind the
hasWebhooksAccessorg flag (admins and impersonators aside), so the whole surface stays hidden until the flag is on.Verification
Covered by unit tests (verifier schemes against real signatures), container tests (ingest to delivery, concurrency dedup, crash recovery, partition management), and the provider round-trip suite. Verified end-to-end against a live environment, including a real Slack approve/deny round-trip.