Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/cli/dashboard/webhooklogs/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ export default class WebhookLogsList extends DashboardCommand {
{ key: 'eventType', header: 'Event' },
{ key: 'statusCode', header: 'Status' },
{ key: 'processed', header: 'Processed', format: (v) => (v ? 'yes' : 'no') },
{
key: 'decisionReason',
header: 'Reason',
format: (v) => (v ? String(v).slice(0, 50) : '-'),
},
{ key: 'receivedAt', header: 'Time', format: formatDate },
]);
} catch (err) {
Expand Down
1 change: 1 addition & 0 deletions src/cli/dashboard/webhooklogs/show.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export default class WebhookLogsShow extends DashboardCommand {
statusCode: { label: 'Status Code' },
processed: { label: 'Processed', format: (v) => (v ? 'yes' : 'no') },
projectId: { label: 'Project ID' },
decisionReason: { label: 'Decision Reason' },
receivedAt: { label: 'Received At', format: formatDate },
});

Expand Down
1 change: 1 addition & 0 deletions src/db/migrations/0027_webhook_decision_reason.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE webhook_logs ADD COLUMN decision_reason TEXT;
7 changes: 7 additions & 0 deletions src/db/migrations/meta/_journal.json
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,13 @@
"when": 1761000000000,
"tag": "0026_agent_type_concurrency",
"breakpoints": false
},
{
"idx": 27,
"version": "7",
"when": 1762000000000,
"tag": "0027_webhook_decision_reason",
"breakpoints": false
}
]
}
2 changes: 2 additions & 0 deletions src/db/repositories/webhookLogsRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export interface InsertWebhookLogInput {
projectId?: string;
eventType?: string;
processed?: boolean;
decisionReason?: string;
}

export interface ListWebhookLogsInput {
Expand Down Expand Up @@ -47,6 +48,7 @@ export async function insertWebhookLog(input: InsertWebhookLogInput): Promise<st
projectId: input.projectId,
eventType: input.eventType,
processed: input.processed ?? false,
decisionReason: input.decisionReason,
})
.returning({ id: webhookLogs.id });
return row.id;
Expand Down
1 change: 1 addition & 0 deletions src/db/schema/webhookLogs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export const webhookLogs = pgTable(
projectId: text('project_id'),
eventType: text('event_type'),
processed: boolean('processed').default(false),
decisionReason: text('decision_reason'),
},
(table) => [
index('idx_webhook_logs_received_at').on(table.receivedAt),
Expand Down
8 changes: 7 additions & 1 deletion src/router/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ app.post(
return {
processed: result.shouldProcess,
projectId: result.projectId,
decisionReason: result.decisionReason,
};
},
}),
Expand All @@ -94,7 +95,11 @@ app.post(
const adapter = new GitHubRouterAdapter();
const augmented = injectEventType(payload, eventType ?? 'unknown');
const result = await processRouterWebhook(adapter, augmented, triggerRegistry);
return { processed: result.shouldProcess };
return {
processed: result.shouldProcess,
projectId: result.projectId,
decisionReason: result.decisionReason,
};
},
}),
);
Expand All @@ -116,6 +121,7 @@ app.post(
return {
processed: result.shouldProcess,
projectId: result.projectId,
decisionReason: result.decisionReason,
};
},
}),
Expand Down
52 changes: 42 additions & 10 deletions src/router/webhook-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ export interface ProcessRouterWebhookResult {
shouldProcess: boolean;
/** The resolved project identifier, if any. */
projectId?: string;
/** Human-readable explanation of why the event was processed or skipped. */
decisionReason?: string;
}

/**
Expand Down Expand Up @@ -53,13 +55,16 @@ export async function processRouterWebhook(
const event = await adapter.parseWebhook(payload);
if (!event) {
logger.debug(`Ignoring ${adapter.type} event (unparseable or not processable)`);
return { shouldProcess: false };
return { shouldProcess: false, decisionReason: 'Event unparseable or not processable' };
}

// Step 2: Filter
if (!adapter.isProcessableEvent(event)) {
logger.debug(`Ignoring ${adapter.type} event`, { eventType: event.eventType });
return { shouldProcess: false };
return {
shouldProcess: false,
decisionReason: `Event type not processable: ${event.eventType}`,
};
}

// Step 3: Self-authored check
Expand All @@ -68,7 +73,7 @@ export async function processRouterWebhook(
eventType: event.eventType,
projectIdentifier: event.projectIdentifier,
});
return { shouldProcess: true };
return { shouldProcess: true, decisionReason: 'Self-authored event (loop prevention)' };
}

// Step 4: Fire acknowledgment reaction (fire-and-forget)
Expand All @@ -80,7 +85,10 @@ export async function processRouterWebhook(
logger.info(`No project config found for ${adapter.type} event`, {
projectIdentifier: event.projectIdentifier,
});
return { shouldProcess: true };
return {
shouldProcess: true,
decisionReason: `No project config for identifier ${event.projectIdentifier ?? '(unknown)'}`,
};
}

// Step 6: Dispatch triggers with credential scope
Expand All @@ -99,7 +107,11 @@ export async function processRouterWebhook(
eventType: event.eventType,
workItemId: event.workItemId,
});
return { shouldProcess: true, projectId: project.id };
return {
shouldProcess: true,
projectId: project.id,
decisionReason: 'No trigger matched for event',
};
}

logger.info(`${adapter.type} trigger matched`, {
Expand All @@ -112,7 +124,11 @@ export async function processRouterWebhook(
// dispatch already performed PM operations — no job queuing needed
if (!result.agentType) {
logger.info('Trigger completed without agent (PM operation done)');
return { shouldProcess: true, projectId: project.id };
return {
shouldProcess: true,
projectId: project.id,
decisionReason: 'Trigger completed without agent (PM operation)',
};
}

// Step 7: Work-item concurrency lock
Expand All @@ -125,7 +141,11 @@ export async function processRouterWebhook(
agentType: result.agentType,
reason: lockStatus.reason,
});
return { shouldProcess: true, projectId: project.id };
return {
shouldProcess: true,
projectId: project.id,
decisionReason: `Work item locked: ${lockStatus.reason ?? 'active run exists'}`,
};
}
}

Expand All @@ -139,7 +159,11 @@ export async function processRouterWebhook(
);
agentTypeMaxConcurrency = concurrencyCheck.maxConcurrency;
if (concurrencyCheck.blocked) {
return { shouldProcess: true, projectId: project.id };
return {
shouldProcess: true,
projectId: project.id,
decisionReason: 'Agent type concurrency limit reached',
};
}
}

Expand Down Expand Up @@ -170,7 +194,11 @@ export async function processRouterWebhook(
eventType: event.eventType,
workItemId: event.workItemId,
});
return { shouldProcess: true, projectId: project.id };
return {
shouldProcess: true,
projectId: project.id,
decisionReason: 'Failed to enqueue job to Redis',
};
}

// Step 11: Post acknowledgment comment and patch ack info onto the enqueued job.
Expand Down Expand Up @@ -199,5 +227,9 @@ export async function processRouterWebhook(
}
}

return { shouldProcess: true, projectId: project.id };
return {
shouldProcess: true,
projectId: project.id,
decisionReason: `Job queued: ${result.agentType} agent for work item ${event.workItemId ?? '(unknown)'}`,
};
}
2 changes: 2 additions & 0 deletions src/utils/webhookLogger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export interface WebhookLogInput {
projectId?: string;
eventType?: string;
processed: boolean;
decisionReason?: string;
}

/**
Expand All @@ -42,6 +43,7 @@ async function _logWebhookCallAsync(input: WebhookLogInput): Promise<void> {
projectId: input.projectId,
eventType: input.eventType,
processed: input.processed,
decisionReason: input.decisionReason,
});

insertCount += 1;
Expand Down
1 change: 1 addition & 0 deletions src/webhook/webhookHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export function createWebhookHandler(config: WebhookHandlerConfig): Handler {
statusCode: 400,
eventType: parseResult.eventType,
processed: false,
decisionReason: `Parse failed: ${parseResult.error}`,
});
return c.text('Bad Request', 400);
}
Expand Down
1 change: 1 addition & 0 deletions src/webhook/webhookLogging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export function logSuccessfulWebhook(
eventType,
processed: logOverrides?.processed ?? true,
projectId: logOverrides?.projectId,
decisionReason: logOverrides?.decisionReason,
});
}

Expand Down
1 change: 1 addition & 0 deletions src/webhook/webhookTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export type ParseResult =
export interface WebhookLogOverrides {
processed?: boolean;
projectId?: string;
decisionReason?: string;
}

/**
Expand Down
10 changes: 10 additions & 0 deletions tests/unit/router/webhook-processor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ describe('processRouterWebhook', () => {
});
const result = await processRouterWebhook(adapter, {}, mockTriggerRegistry);
expect(result.shouldProcess).toBe(false);
expect(result.decisionReason).toBe('Event unparseable or not processable');
expect(addJob).not.toHaveBeenCalled();
});

Expand All @@ -91,6 +92,7 @@ describe('processRouterWebhook', () => {
});
const result = await processRouterWebhook(adapter, {}, mockTriggerRegistry);
expect(result.shouldProcess).toBe(false);
expect(result.decisionReason).toBe('Event type not processable: commentCard');
expect(addJob).not.toHaveBeenCalled();
});

Expand All @@ -100,6 +102,7 @@ describe('processRouterWebhook', () => {
});
const result = await processRouterWebhook(adapter, {}, mockTriggerRegistry);
expect(result.shouldProcess).toBe(true);
expect(result.decisionReason).toBe('Self-authored event (loop prevention)');
expect(addJob).not.toHaveBeenCalled();
});

Expand All @@ -115,6 +118,7 @@ describe('processRouterWebhook', () => {
});
const result = await processRouterWebhook(adapter, {}, mockTriggerRegistry);
expect(result.shouldProcess).toBe(true);
expect(result.decisionReason).toMatch(/No project config for identifier/);
expect(addJob).not.toHaveBeenCalled();
});

Expand All @@ -124,6 +128,7 @@ describe('processRouterWebhook', () => {
});
const result = await processRouterWebhook(adapter, {}, mockTriggerRegistry);
expect(result.shouldProcess).toBe(true);
expect(result.decisionReason).toBe('No trigger matched for event');
expect(addJob).not.toHaveBeenCalled();
});

Expand All @@ -138,6 +143,7 @@ describe('processRouterWebhook', () => {
const result = await processRouterWebhook(adapter, {}, mockTriggerRegistry);
expect(result.shouldProcess).toBe(true);
expect(result.projectId).toBe('p1');
expect(result.decisionReason).toMatch(/Job queued: implementation agent for work item/);
// buildJob is called without ack params (ack is patched after enqueue)
expect(adapter.buildJob).toHaveBeenCalledWith(
expect.objectContaining({ eventType: 'commentCard' }),
Expand Down Expand Up @@ -217,6 +223,7 @@ describe('processRouterWebhook', () => {

const result = await processRouterWebhook(adapter, {}, mockTriggerRegistry);
expect(result.shouldProcess).toBe(true);
expect(result.decisionReason).toBe('Trigger completed without agent (PM operation)');
expect(addJob).not.toHaveBeenCalled();
});

Expand All @@ -240,6 +247,7 @@ describe('processRouterWebhook', () => {
// Should not throw
const result = await processRouterWebhook(adapter, {}, mockTriggerRegistry);
expect(result.shouldProcess).toBe(true);
expect(result.decisionReason).toBe('Failed to enqueue job to Redis');
});

it('works with adapters that do not implement firePreActions', async () => {
Expand Down Expand Up @@ -273,6 +281,7 @@ describe('processRouterWebhook', () => {
const result = await processRouterWebhook(adapter, {}, mockTriggerRegistry);
expect(result.shouldProcess).toBe(true);
expect(result.projectId).toBe('p1');
expect(result.decisionReason).toBe('Work item locked: db: active run exists');
expect(addJob).not.toHaveBeenCalled();
expect(adapter.postAck).not.toHaveBeenCalled();
});
Expand Down Expand Up @@ -309,6 +318,7 @@ describe('processRouterWebhook', () => {

const result = await processRouterWebhook(adapter, {}, mockTriggerRegistry);
expect(result.shouldProcess).toBe(true);
expect(result.decisionReason).toBe('Agent type concurrency limit reached');
expect(addJob).not.toHaveBeenCalled();
});

Expand Down
16 changes: 16 additions & 0 deletions tests/unit/utils/webhookLogger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ describe('logWebhookCall', () => {
projectId: undefined,
eventType: undefined,
processed: true,
decisionReason: undefined,
});
});

Expand All @@ -92,6 +93,21 @@ describe('logWebhookCall', () => {
);
});

it('passes decisionReason when provided', async () => {
logWebhookCall({
...sampleInput,
decisionReason: 'No trigger matched for event',
});

await vi.runAllTimersAsync();

expect(mockInsertWebhookLog).toHaveBeenCalledWith(
expect.objectContaining({
decisionReason: 'No trigger matched for event',
}),
);
});

it('handles github source', async () => {
logWebhookCall({ ...sampleInput, source: 'github' });

Expand Down
21 changes: 21 additions & 0 deletions tests/unit/webhook/webhookHandlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ describe('createWebhookHandler', () => {
statusCode: 400,
processed: false,
bodyRaw: 'bad json',
decisionReason: 'Parse failed: bad json',
}),
);
});
Expand Down Expand Up @@ -210,6 +211,26 @@ describe('createWebhookHandler', () => {
);
});

it('threads decisionReason from processWebhook return value to log', async () => {
const handler = createWebhookHandler({
source: 'github',
parsePayload: async () => ({ ok: true, payload: {}, eventType: 'push' }),
processWebhook: vi.fn().mockResolvedValue({
processed: false,
decisionReason: 'No trigger matched for event',
}),
});

const app = buildApp(handler);
await postJson(app, {});

expect(mockLogWebhookCall).toHaveBeenCalledWith(
expect.objectContaining({
decisionReason: 'No trigger matched for event',
}),
);
});

it('propagates processWebhook errors to Hono error handler', async () => {
const handler = createWebhookHandler({
source: 'jira',
Expand Down
Loading