diff --git a/src/router/webhook-processor.ts b/src/router/webhook-processor.ts index 125b4b94..57a88b5b 100644 --- a/src/router/webhook-processor.ts +++ b/src/router/webhook-processor.ts @@ -13,6 +13,7 @@ import { getCoalesceWindowMs } from '../pm/coalesce-config.js'; import { captureException } from '../sentry.js'; import type { TriggerRegistry } from '../triggers/registry.js'; +import type { TriggerResult } from '../types/index.js'; import { logger } from '../utils/logging.js'; import { isDuplicateAction, markActionProcessed } from './action-dedup.js'; import { @@ -23,10 +24,30 @@ import { markRecentlyDispatched, } from './agent-type-lock.js'; import { classifyLockState } from './lock-state-classifier.js'; -import type { RouterPlatformAdapter } from './platform-adapter.js'; +import type { ParsedWebhookEvent, RouterPlatformAdapter } from './platform-adapter.js'; import { addJob, scheduleCoalescedJob } from './queue.js'; import { clearWorkItemEnqueued, isWorkItemLocked, markWorkItemEnqueued } from './work-item-lock.js'; +/** + * Pick the most specific work-item label for a webhook log decisionReason. + * + * `event.workItemId` is set at parse time (`adapter.parseWebhook`) — for + * GitHub `pull_request`-shaped events the parser populates it from + * `payload.pull_request.number`, but for `check_suite` webhooks the PR + * number lives under `payload.check_suite.pull_requests[0].number` and the + * parser leaves the field undefined. The trigger handler resolves it + * internally and returns `result.workItemId` / `result.prNumber`; both are + * better diagnostic labels than `(unknown)` and the dashboard webhook log + * should prefer them. + * + * Order: result.workItemId > `PR #` > event.workItemId > `(unknown)`. + */ +function resolveWorkItemLabel(result: TriggerResult, event: ParsedWebhookEvent): string { + if (result.workItemId) return result.workItemId; + if (typeof result.prNumber === 'number') return `PR #${result.prNumber}`; + return event.workItemId ?? '(unknown)'; +} + export interface ProcessRouterWebhookResult { /** Whether the event was of a processable type for this platform. */ shouldProcess: boolean; @@ -269,7 +290,7 @@ export async function processRouterWebhook( return { shouldProcess: true, projectId: project.id, - decisionReason: `Coalesced dispatch scheduled: ${result.agentType} agent for work item ${result.workItemId ?? '(unknown)'}`, + decisionReason: `Coalesced dispatch scheduled: ${result.agentType} agent for work item ${resolveWorkItemLabel(result, event)}`, }; } } @@ -414,6 +435,6 @@ export async function processRouterWebhook( return { shouldProcess: true, projectId: project.id, - decisionReason: `Job queued: ${result.agentType} agent for work item ${event.workItemId ?? '(unknown)'}`, + decisionReason: `Job queued: ${result.agentType} agent for work item ${resolveWorkItemLabel(result, event)}`, }; } diff --git a/tests/unit/router/webhook-processor.test.ts b/tests/unit/router/webhook-processor.test.ts index 199a485c..f405c934 100644 --- a/tests/unit/router/webhook-processor.test.ts +++ b/tests/unit/router/webhook-processor.test.ts @@ -261,6 +261,54 @@ describe('processRouterWebhook', () => { expect(addJob).toHaveBeenCalled(); }); + // 2026-04-29: prod check_suite-triggered respond-to-ci dispatch logged + // `Job queued: respond-to-ci agent for work item (unknown)` because the + // GitHub adapter's parseWebhook only extracts `event.workItemId` from + // `payload.pull_request.number` — `check_suite` payloads have it under + // `check_suite.pull_requests[0].number`. The trigger handler resolves it + // internally and returns `result.workItemId` / `result.prNumber`; the + // decisionReason should prefer those over the parse-time event field. + it('decisionReason prefers result.workItemId over event.workItemId when both differ', async () => { + const triggerResult = { + agentType: 'respond-to-ci', + agentInput: {}, + workItemId: 'PROJ-42', + }; + vi.mocked(addJob).mockResolvedValue('job-1'); + const adapter = makeMockAdapter({ + // parseWebhook resolved workItemId to undefined (e.g. check_suite payload) + parseWebhook: vi.fn().mockResolvedValue({ + eventType: 'check_suite', + workItemId: undefined, + }), + dispatchWithCredentials: vi.fn().mockResolvedValue(triggerResult), + postAck: vi.fn().mockResolvedValue({ commentId: 'c', message: 'm' }), + }); + + const result = await processRouterWebhook(adapter, {}, mockTriggerRegistry); + expect(result.decisionReason).toBe('Job queued: respond-to-ci agent for work item PROJ-42'); + }); + + it('decisionReason falls back to result.prNumber when workItemId is unavailable', async () => { + const triggerResult = { + agentType: 'respond-to-ci', + agentInput: {}, + prNumber: 155, + }; + vi.mocked(addJob).mockResolvedValue('job-1'); + const adapter = makeMockAdapter({ + parseWebhook: vi.fn().mockResolvedValue({ + eventType: 'check_suite', + workItemId: undefined, + }), + dispatchWithCredentials: vi.fn().mockResolvedValue(triggerResult), + postAck: vi.fn().mockResolvedValue({ commentId: 'c', message: 'm' }), + }); + + const result = await processRouterWebhook(adapter, {}, mockTriggerRegistry); + expect(result.decisionReason).toBe('Job queued: respond-to-ci agent for work item PR #155'); + }); + it('posts ack comment before enqueuing job', async () => { const callOrder: string[] = []; const triggerResult = { agentType: 'implementation', agentInput: {} };