From 98dbc59a028fa365a84c9e85f73d6ee440a658b8 Mon Sep 17 00:00:00 2001 From: NoiceHax Date: Thu, 13 Aug 2026 22:46:40 +0530 Subject: [PATCH] fix(github-webhook): apply preview limit only to new previews The preview deployment limit was checked before looking up whether the pull request already had a preview, so once an application reached the limit every `synchronize` event was dropped and existing previews went stale while GitHub still saw a 200. The check now runs inside the branch that creates a preview, uses the schema default of 3 when no limit is configured (instead of 0, which blocked every app without an explicit value) and compares with `>=` so the configured limit is inclusive. Closes #4074 --- .../deploy/github-webhook-handler.test.ts | 161 +++++++++++++++++- apps/dokploy/pages/api/deploy/github.ts | 13 +- 2 files changed, 168 insertions(+), 6 deletions(-) diff --git a/apps/dokploy/__test__/deploy/github-webhook-handler.test.ts b/apps/dokploy/__test__/deploy/github-webhook-handler.test.ts index e25bd425e..6b61c97d7 100644 --- a/apps/dokploy/__test__/deploy/github-webhook-handler.test.ts +++ b/apps/dokploy/__test__/deploy/github-webhook-handler.test.ts @@ -12,6 +12,8 @@ const mocks = vi.hoisted(() => ({ queueAdd: vi.fn(), verify: vi.fn(), shouldDeploy: vi.fn(), + createPreviewDeployment: vi.fn(), + findPreviewDeploymentByApplicationId: vi.fn(), })); vi.mock("drizzle-orm", () => ({ @@ -64,10 +66,11 @@ vi.mock("@dokploy/server", () => ({ IS_CLOUD: false, shouldDeploy: mocks.shouldDeploy, checkUserRepositoryPermissions: vi.fn(), - createPreviewDeployment: vi.fn(), + createPreviewDeployment: mocks.createPreviewDeployment, createSecurityBlockedComment: vi.fn(), findGithubById: vi.fn(), - findPreviewDeploymentByApplicationId: vi.fn(), + findPreviewDeploymentByApplicationId: + mocks.findPreviewDeploymentByApplicationId, findPreviewDeploymentsByPullRequestId: vi.fn(), getBitbucketHeaders: vi.fn(() => ({})), removePreviewDeployment: vi.fn(), @@ -321,3 +324,157 @@ describe("GitHub app webhook auto-deploy", () => { expect(res.json).toHaveBeenCalledWith({ message: "No apps to deploy" }); }); }); + +describe("GitHub app webhook preview deployments", () => { + const createApplication = ( + overrides: Record = {}, + ): Record => ({ + applicationId: "application-id", + name: "my-app", + serverId: null, + previewLabels: [], + previewLimit: 3, + previewDeployments: [], + previewRequireCollaboratorPermissions: false, + ...overrides, + }); + + const createPreviewDeployments = (total: number) => + Array.from({ length: total }, (_, index) => ({ + previewDeploymentId: `existing-preview-${index}`, + })); + + const createPullRequestRequest = (action: string) => + ({ + headers: { + "x-hub-signature-256": "sha256=test-signature", + "x-github-event": "pull_request", + }, + body: { + installation: { + id: 12345, + }, + action, + pull_request: { + id: 987, + number: 42, + title: "feat: add preview", + html_url: "https://github.com/agentHits/dokploy/pull/42", + labels: [], + user: { + login: "agentHits", + }, + head: { + ref: "feature", + sha: "abc123", + }, + base: { + ref: "main", + }, + }, + repository: { + name: "dokploy", + owner: { + login: "agentHits", + }, + }, + }, + }) as unknown as NextApiRequest; + + beforeEach(() => { + vi.clearAllMocks(); + mocks.githubFindFirst.mockResolvedValue({ + githubId: "github-provider-id", + githubInstallationId: 12345, + githubWebhookSecret: "webhook-secret", + }); + mocks.verify.mockResolvedValue(true); + mocks.queueAdd.mockResolvedValue({ id: "job-id" }); + mocks.createPreviewDeployment.mockResolvedValue({ + previewDeploymentId: "new-preview-id", + }); + mocks.findPreviewDeploymentByApplicationId.mockResolvedValue(undefined); + }); + + it("redeploys an existing preview even when the limit is reached", async () => { + mocks.applicationsFindMany.mockResolvedValue([ + createApplication({ + previewLimit: 2, + previewDeployments: createPreviewDeployments(3), + }), + ]); + mocks.findPreviewDeploymentByApplicationId.mockResolvedValue({ + previewDeploymentId: "existing-preview-0", + }); + const res = createResponse(); + + await handler(createPullRequestRequest("synchronize"), res); + + expect(mocks.createPreviewDeployment).not.toHaveBeenCalled(); + expect(mocks.queueAdd).toHaveBeenCalledWith( + "deployments", + expect.objectContaining({ + applicationId: "application-id", + applicationType: "application-preview", + previewDeploymentId: "existing-preview-0", + type: "deploy", + }), + expect.objectContaining({ + removeOnComplete: true, + removeOnFail: true, + }), + ); + expect(res.status).toHaveBeenCalledWith(200); + }); + + it("does not create a new preview once the limit is reached", async () => { + mocks.applicationsFindMany.mockResolvedValue([ + createApplication({ + previewLimit: 2, + previewDeployments: createPreviewDeployments(2), + }), + ]); + const res = createResponse(); + + await handler(createPullRequestRequest("opened"), res); + + expect(mocks.createPreviewDeployment).not.toHaveBeenCalled(); + expect(mocks.queueAdd).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(200); + }); + + it("falls back to the default limit when none is configured", async () => { + mocks.applicationsFindMany.mockResolvedValue([ + createApplication({ + previewLimit: null, + previewDeployments: createPreviewDeployments(2), + }), + ]); + const res = createResponse(); + + await handler(createPullRequestRequest("opened"), res); + + expect(mocks.createPreviewDeployment).toHaveBeenCalledWith( + expect.objectContaining({ + applicationId: "application-id", + branch: "feature", + pullRequestId: 987, + pullRequestNumber: 42, + }), + ); + expect(mocks.queueAdd).toHaveBeenCalledWith( + "deployments", + expect.objectContaining({ + applicationId: "application-id", + applicationType: "application-preview", + previewDeploymentId: "new-preview-id", + type: "deploy", + }), + expect.objectContaining({ + removeOnComplete: true, + removeOnFail: true, + }), + ); + expect(res.status).toHaveBeenCalledWith(200); + }); +}); diff --git a/apps/dokploy/pages/api/deploy/github.ts b/apps/dokploy/pages/api/deploy/github.ts index 4fc75fc50..2273ac3bc 100644 --- a/apps/dokploy/pages/api/deploy/github.ts +++ b/apps/dokploy/pages/api/deploy/github.ts @@ -484,10 +484,6 @@ export default async function handler( if (!hasLabel) continue; } - const previewLimit = app?.previewLimit || 0; - if (app?.previewDeployments?.length > previewLimit) { - continue; - } const previewDeploymentResult = await findPreviewDeploymentByApplicationId(app.applicationId, prId); @@ -495,6 +491,15 @@ export default async function handler( previewDeploymentResult?.previewDeploymentId || ""; if (!previewDeploymentResult && shouldCreateDeployment) { + // The limit only applies to new previews, existing ones must + // still be redeployed when the pull request is updated. + const previewLimit = app?.previewLimit ?? 3; + if ((app?.previewDeployments?.length ?? 0) >= previewLimit) { + console.warn( + `⚠️ Preview deployment limit (${previewLimit}) reached for ${app.name}, skipping preview for pull request #${prNumber}`, + ); + continue; + } const previewDeployment = await createPreviewDeployment({ applicationId: app.applicationId as string, branch: prBranch,