feat(sentry): add worker-side Sentry webhook job dispatch#1024
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
The dispatch plumbing (SentryJobData, dispatchJob case) is correct and follows existing patterns. However, processSentryWebhook silently discards the pre-computed triggerResult from the router, always re-dispatching through the registry instead. This breaks the router→worker contract that every other handler (Trello, JIRA, GitHub) follows.
Architecture & Design
-
[BLOCKING]
triggerResultis accepted but never used (src/triggers/sentry/webhook-handler.ts): The router computes aTriggerResultviaSentryRouterAdapter.dispatchWithCredentials(), serializes it into the BullMQ job, and the worker passes it toprocessSentryWebhook. But the handler only logstriggerResult— it always callsregistry.dispatch(ctx)unconditionally, discarding the pre-computed result. Every other webhook handler usestriggerResultas a short-circuit:- Trello/JIRA:
processPMWebhook→resolveTriggerResult()returnspreResolvedResultdirectly when non-null (seesrc/pm/webhook-handler.ts:72-77) - GitHub:
processGitHubWebhookchecksif (triggerResult)and assignsresult = triggerResult(seesrc/triggers/github/webhook-handler.ts:254-258)
This means Sentry webhooks always re-evaluate triggers on the worker, which is wasteful and could produce different results if registry state diverges between router and worker. The handler should follow the same pattern: use
triggerResultwhen provided, fall back toregistry.dispatch(ctx)otherwise.Additionally, the handler dispatches triggers but never executes the matched agent — it calls
registry.dispatch(ctx)which returns aTriggerResult, but that return value is discarded. In contrast, Trello/JIRA handlers callrunAgentWithCredentialsand GitHub callsrunGitHubAgentwith the result. This means Sentry webhooks match triggers but never actually run agents. - Trello/JIRA:
Code Issues
Should Fix
- src/worker-entry.ts:75 — The
eventTypeJSDoc comment says'event_alert' | 'metric_alert'but the router-sideSentryJobtype insrc/router/queue.ts:55lists'event_alert' | 'metric_alert' | 'issue'. The worker-side comment should match the router-side type to avoid confusion.
Test Issues
- tests/unit/triggers/sentry-webhook-handler.test.ts — The test "dispatches even when triggerResult is provided" asserts that
registry.dispatchis always called regardless oftriggerResult. This test is passing, but it validates the buggy behavior. Once the handler is fixed to usetriggerResultwhen provided (skippingregistry.dispatch), this test should assert the opposite: thatregistry.dispatchis NOT called whentriggerResultis provided.
🕵️ claude-code · claude-opus-4-6 · run details
…entryWebhook - Short-circuit registry.dispatch when triggerResult is already provided, matching the router→worker contract followed by Trello, JIRA, and GitHub handlers - After resolving the trigger result, call runAgentExecutionPipeline to actually execute the matched agent (fixes silent discard of matched triggers) - Fix SentryJobData JSDoc comment: add 'issue' to eventType union to match router-side SentryJob type in src/router/queue.ts - Update tests: assert registry.dispatch is NOT called when triggerResult is provided; add coverage for agent execution pipeline call Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
LGTM — The implementation correctly fixes the dropped Sentry webhook jobs by adding the missing dispatchJob case and a well-structured handler. Tests are comprehensive and CI is green.
Minor Observations
- Doc comment not updated:
src/worker-entry.tslines 6 and 11 still list job types as "Trello, GitHub, or JIRA" — Sentry should be added to both the description andJOB_TYPEenv var docs. - No
withGitHubTokenscope: Unlike the Trello/JIRA/GitHub handlers that go throughrunAgentWithCredentials(which sets up a scoped GitHub Octokit client), the Sentry handler callsrunAgentExecutionPipelinedirectly. This is fine for the current alerting agent (which doesn't require SCM capabilities), but if a future Sentry-triggered agent needs GitHub access (e.g., auto-creating PRs), the missing GitHub client scope would cause failures inlinkPRPostExecution. Worth a code comment noting this intentional limitation. - Not re-exported from
triggers/index.ts: Minor inconsistency —processGitHubWebhookandprocessJiraWebhookare barrel-exported fromtriggers/index.ts, whileprocessSentryWebhookis imported directly. This matches the existingprocessTrelloWebhookpattern though, so not a real issue.
🕵️ claude-code · claude-opus-4-6 · run details
Summary
SentryJobDatainterface tosrc/worker-entry.tsmatching the router'sSentryJobtype structure'sentry'case todispatchJob()switch statement that callsprocessSentryWebhook()src/triggers/sentry/webhook-handler.tswithprocessSentryWebhook()following the thin-wrapper pattern from Trello/JIRA handlersSentryJobDatain theJobDataunion typeThis fixes a bug where Sentry webhook jobs enqueued by the router were being dropped with an "Unknown job type: sentry" error because
dispatchJob()had no'sentry'case.Card: https://trello.com/c/69c1b02b331142dd91419576
Test plan
tests/unit/worker-entry.test.ts— added test:routes sentry job to processSentryWebhook with payload, projectId, registry, and triggerResulttests/unit/triggers/sentry-webhook-handler.test.ts— new file testingprocessSentryWebhook():sentryand given payload🤖 Generated with Claude Code
🕵️ claude-code · claude-sonnet-4-6 · run details