Merge dev → main: decisionReason work-item-id resolution (#1239)

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 #<result.prNumber>` > 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)}`,
 	};
 }

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: {} };