diff --git a/__tests__/integration/app.test.js b/__tests__/integration/app.test.js index da1cb5a..58333a1 100644 --- a/__tests__/integration/app.test.js +++ b/__tests__/integration/app.test.js @@ -695,6 +695,54 @@ describe('app', () => { expect(getLogger().error).toHaveBeenCalledWith({ err: error }, 'Probot error'); }); + + // Bug #13: pull_request.opened must dedupe on deliveryId. Without + // this, Probot retries replay enablePullRequestAutoMerge, the AI + // review (only the AI side has its own 5-min throttle), and the + // dependabot enqueue. + it('pull_request.opened skips duplicate webhook deliveries', async () => { + isProcessed.mockReturnValue(true); + _setConfigForTesting({ ai_review: { enabled: true } }); + reviewPullRequest.mockClear(); + const { handlers } = setupApp(); + + const context = { + id: 'delivery-pr-opened-dup', + log: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, + octokit: createMockOctokit(), + payload: { + pull_request: { number: 7, node_id: 'PR_7' }, + repository: { name: 'rivet', owner: { login: 'pulseengine' }, default_branch: 'main' }, + sender: { login: 'human' } + } + }; + + await handlers['pull_request.opened'](context); + + expect(reviewPullRequest).not.toHaveBeenCalled(); + expect(markProcessed).not.toHaveBeenCalled(); + }); + + it('pull_request.opened marks delivery processed on completion', async () => { + _setConfigForTesting({ ai_review: { enabled: true } }); + reviewPullRequest.mockResolvedValue({ success: true }); + const { handlers } = setupApp(); + + const context = { + id: 'delivery-pr-opened-1', + log: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, + octokit: createMockOctokit(), + payload: { + pull_request: { number: 8, node_id: 'PR_8' }, + repository: { name: 'rivet', owner: { login: 'pulseengine' }, default_branch: 'main' }, + sender: { login: 'human' } + } + }; + + await handlers['pull_request.opened'](context); + + expect(markProcessed).toHaveBeenCalledWith('delivery-pr-opened-1'); + }); }); // ========================================================================= diff --git a/src/app.js b/src/app.js index db1446d..770bdae 100644 --- a/src/app.js +++ b/src/app.js @@ -839,6 +839,18 @@ function registerApp(app, options = {}) { // PR opened: auto-merge for bots, AI review, dependabot generation check app.on('pull_request.opened', async (context) => { if (context.log) setLogger(context.log); + // Bug #13: dedupe at handler entry. Without this guard, a Probot + // webhook redelivery (~5 retries on 5xx/timeout) re-runs auto-merge + // enable, the AI review, and the dependabot enqueue. The 5-min + // _reviewTimestamps throttle in reviewPullRequest only catches the + // AI review path; it doesn't protect enablePullRequestAutoMerge or + // the task-store enqueue. + const deliveryId = context.id; + if (deliveryId && isProcessed(deliveryId)) { + getLogger().info({ deliveryId }, 'Skipping duplicate pull_request.opened delivery'); + return; + } + const config = getConfig(); const pr = context.payload.pull_request; const { repository } = context.payload; @@ -926,6 +938,8 @@ function registerApp(app, options = {}) { } } } + + if (deliveryId) markProcessed(deliveryId); }); app.on('pull_request.closed', async (context) => {