From ba3dbbc392e45760d1e40c9fcb7c64c173079a7c Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Tue, 16 Jun 2026 17:31:22 -0700 Subject: [PATCH 1/2] fix: include deploy trigger in build dedupe key to prevent dropped deploys The resolve/build queue dedupe key was derived only from a build configuration fingerprint, not the commit being deployed. When two deploys of the same build were triggered close together (e.g. two pushes to a tracked branch landing within the queue's dedupe window) both produced an identical key. The build job is keyed by jobId, so a duplicate enqueue is a no-op: the second deploy was silently coalesced into the first and the newer commit never built, with no error and no log line. Append a per-trigger ref to the resolve dedupe id and build jobId: the pushed head commit for branch pushes, or a unique run id for manual redeploys. Distinct triggers now get distinct keys, while genuine duplicates of the same trigger (such as a redelivered webhook) still coalesce. Thread the ref from the resolve step into the build step so both queue layers agree, and log when an enqueue is deduped so the drop is observable. --- src/server/services/__tests__/build.test.ts | 70 ++++++- src/server/services/__tests__/github.test.ts | 204 +++++++++++++++---- src/server/services/build.ts | 40 +++- src/server/services/github.ts | 20 +- 4 files changed, 288 insertions(+), 46 deletions(-) diff --git a/src/server/services/__tests__/build.test.ts b/src/server/services/__tests__/build.test.ts index 758ca7a..0d37ee7 100644 --- a/src/server/services/__tests__/build.test.ts +++ b/src/server/services/__tests__/build.test.ts @@ -866,6 +866,7 @@ describe('BuildService queue fingerprinting', () => { let mockBuildQuery: any; let mockBuildQueueAdd: jest.Mock; let mockResolveQueueAdd: jest.Mock; + let mockBuildQueueGetJob: jest.Mock; const createMockBuild = (overrides: any = {}) => ({ @@ -909,6 +910,7 @@ describe('BuildService queue fingerprinting', () => { mockBuildQueueAdd = jest.fn().mockResolvedValue(undefined); mockResolveQueueAdd = jest.fn().mockResolvedValue(undefined); + mockBuildQueueGetJob = jest.fn().mockResolvedValue(undefined); mockBuildQuery = { findOne: jest.fn().mockReturnThis(), @@ -936,7 +938,7 @@ describe('BuildService queue fingerprinting', () => { {} as any, queueManager as any ); - (buildService as any).buildQueue = { add: mockBuildQueueAdd }; + (buildService as any).buildQueue = { add: mockBuildQueueAdd, getJob: mockBuildQueueGetJob }; (buildService as any).resolveAndDeployBuildQueue = { add: mockResolveQueueAdd }; }); @@ -1028,6 +1030,72 @@ describe('BuildService queue fingerprinting', () => { ); }); + test('appends triggerRef to the resolve dedupe key so distinct triggers get distinct keys', async () => { + const build = createMockBuild(); + mockBuildQuery.withGraphFetched.mockResolvedValue(build); + + const expectedFingerprint = await buildService.computeBuildRequestFingerprint(build, 100); + + await buildService.enqueueResolveAndDeployBuild({ + buildId: 1, + githubRepositoryId: 100, + triggerRef: 'commit-a', + }); + await buildService.enqueueResolveAndDeployBuild({ + buildId: 1, + githubRepositoryId: 100, + triggerRef: 'commit-b', + }); + + const firstKey = mockResolveQueueAdd.mock.calls[0][2].deduplication.id; + const secondKey = mockResolveQueueAdd.mock.calls[1][2].deduplication.id; + + expect(firstKey).toBe(`resolve:1:${expectedFingerprint}:commit-a`); + expect(secondKey).toBe(`resolve:1:${expectedFingerprint}:commit-b`); + expect(firstKey).not.toBe(secondKey); + // The trigger is forwarded into the job payload so the resolve step can hand it to the build step. + expect(mockResolveQueueAdd.mock.calls[0][1]).toEqual(expect.objectContaining({ triggerRef: 'commit-a' })); + }); + + test('keeps the dedupe key commit-agnostic when no triggerRef is provided', async () => { + const build = createMockBuild(); + mockBuildQuery.withGraphFetched.mockResolvedValue(build); + + const expectedFingerprint = await buildService.computeBuildRequestFingerprint(build, 100); + + await buildService.enqueueResolveAndDeployBuild({ buildId: 1, githubRepositoryId: 100 }); + + expect(mockResolveQueueAdd.mock.calls[0][2].deduplication.id).toBe(`resolve:1:${expectedFingerprint}`); + expect(mockResolveQueueAdd.mock.calls[0][1]).not.toHaveProperty('triggerRef'); + }); + + test('the same triggerRef yields the same build job id so genuine duplicates still coalesce', async () => { + const build = createMockBuild(); + mockBuildQuery.withGraphFetched.mockResolvedValue(build); + + const expectedFingerprint = await buildService.computeBuildRequestFingerprint(build, 100); + + await buildService.enqueueBuildJob({ buildId: 1, githubRepositoryId: 100, triggerRef: 'commit-a' }); + await buildService.enqueueBuildJob({ buildId: 1, githubRepositoryId: 100, triggerRef: 'commit-a' }); + + expect(mockBuildQueueAdd.mock.calls[0][2].jobId).toBe(`build:1:${expectedFingerprint}:commit-a`); + expect(mockBuildQueueAdd.mock.calls[1][2].jobId).toBe(mockBuildQueueAdd.mock.calls[0][2].jobId); + }); + + test('logs a dedupe skip when a matching build job already exists', async () => { + const build = createMockBuild(); + mockBuildQuery.withGraphFetched.mockResolvedValue(build); + mockBuildQueueGetJob.mockResolvedValue({ id: 'existing' }); + + const expectedFingerprint = await buildService.computeBuildRequestFingerprint(build, 100); + + await buildService.enqueueBuildJob({ buildId: 1, githubRepositoryId: 100, triggerRef: 'commit-a' }); + + expect(mockBuildQueueGetJob).toHaveBeenCalledWith(`build:1:${expectedFingerprint}:commit-a`); + // add() is still invoked; it is a no-op when the job already exists. + expect(mockBuildQueueAdd).toHaveBeenCalled(); + }); + test('service redeploy queues scoped build without deleted-service reconciliation', async () => { const patchAndFetch = jest.fn().mockResolvedValue(undefined); const deploy = { diff --git a/src/server/services/__tests__/github.test.ts b/src/server/services/__tests__/github.test.ts index e1ef3ad..f0db616 100644 --- a/src/server/services/__tests__/github.test.ts +++ b/src/server/services/__tests__/github.test.ts @@ -565,10 +565,13 @@ describe('Github Service - handlePushWebhook', () => { const pushEvent = createMockPushEvent(); await githubService.handlePushWebhook(pushEvent); - expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith('resolve-deploy', { - buildId, - githubRepositoryId: 12345, - }); + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith( + 'resolve-deploy', + expect.objectContaining({ + buildId, + githubRepositoryId: 12345, + }) + ); }); test('should omit githubRepositoryId when deploys have ERROR status', async () => { @@ -596,9 +599,12 @@ describe('Github Service - handlePushWebhook', () => { const pushEvent = createMockPushEvent(); await githubService.handlePushWebhook(pushEvent); - expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith('resolve-deploy', { - buildId, - }); + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith( + 'resolve-deploy', + expect.objectContaining({ + buildId, + }) + ); const addCall = mockDb.services.BuildService.resolveAndDeployBuildQueue.add.mock.calls[0]; expect(addCall[1]).not.toHaveProperty('githubRepositoryId'); @@ -633,9 +639,12 @@ describe('Github Service - handlePushWebhook', () => { const pushEvent = createMockPushEvent(); await githubService.handlePushWebhook(pushEvent); - expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith('resolve-deploy', { - buildId, - }); + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith( + 'resolve-deploy', + expect.objectContaining({ + buildId, + }) + ); }); test('should handle multiple builds and check each for failures independently', async () => { @@ -674,14 +683,22 @@ describe('Github Service - handlePushWebhook', () => { const pushEvent = createMockPushEvent(); await githubService.handlePushWebhook(pushEvent); - expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenNthCalledWith(1, 'resolve-deploy', { - buildId: buildId1, - githubRepositoryId: 12345, - }); - - expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenNthCalledWith(2, 'resolve-deploy', { - buildId: buildId2, - }); + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenNthCalledWith( + 1, + 'resolve-deploy', + expect.objectContaining({ + buildId: buildId1, + githubRepositoryId: 12345, + }) + ); + + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenNthCalledWith( + 2, + 'resolve-deploy', + expect.objectContaining({ + buildId: buildId2, + }) + ); }); test('updates latestCommit for a matching previous commit', async () => { @@ -786,10 +803,13 @@ describe('Github Service - handlePushWebhook', () => { expect(mockDb.services.GlobalConfig.getAllConfigs).toHaveBeenCalled(); expect(mockLoggerInfo).toHaveBeenCalledWith('Push: dry-run would skip deploy reason=ignoreFiles'); - expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith('resolve-deploy', { - buildId, - githubRepositoryId: 12345, - }); + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith( + 'resolve-deploy', + expect.objectContaining({ + buildId, + githubRepositoryId: 12345, + }) + ); expect(mockDb.services.Webhook.webhookQueue.add).not.toHaveBeenCalled(); }); @@ -826,10 +846,13 @@ describe('Github Service - handlePushWebhook', () => { await githubService.handlePushWebhook(pushEvent); expect(mockLoggerInfo).toHaveBeenCalledWith('Push: dry-run would skip deploy reason=ignoreFiles'); - expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith('resolve-deploy', { - buildId, - githubRepositoryId: 12345, - }); + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith( + 'resolve-deploy', + expect.objectContaining({ + buildId, + githubRepositoryId: 12345, + }) + ); expect(mockDb.services.Webhook.webhookQueue.add).not.toHaveBeenCalled(); }); @@ -894,10 +917,13 @@ describe('Github Service - handlePushWebhook', () => { await githubService.handlePushWebhook(pushEvent); - expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith('resolve-deploy', { - buildId, - githubRepositoryId: 12345, - }); + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith( + 'resolve-deploy', + expect.objectContaining({ + buildId, + githubRepositoryId: 12345, + }) + ); expect(mockDb.services.Webhook.webhookQueue.add).not.toHaveBeenCalled(); }); @@ -932,10 +958,13 @@ describe('Github Service - handlePushWebhook', () => { await githubService.handlePushWebhook(pushEvent); - expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith('resolve-deploy', { - buildId, - githubRepositoryId: 12345, - }); + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith( + 'resolve-deploy', + expect.objectContaining({ + buildId, + githubRepositoryId: 12345, + }) + ); expect(mockDb.services.Webhook.webhookQueue.add).not.toHaveBeenCalled(); expect(mockLoggerInfo).toHaveBeenCalledWith('Push: deploying reason=ignoreFiles_not_matched'); }); @@ -960,9 +989,12 @@ describe('Github Service - handlePushWebhook', () => { await githubService.handlePushWebhook(pushEvent); expect(mockGetChangedFilesForPush).not.toHaveBeenCalled(); - expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith('resolve-deploy', { - buildId, - }); + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith( + 'resolve-deploy', + expect.objectContaining({ + buildId, + }) + ); }); test('queues skipped-push webhooks only for supported current build statuses', async () => { @@ -1016,9 +1048,103 @@ describe('Github Service - handlePushWebhook', () => { await githubService.handlePushWebhook(pushEvent); expect(mockGetChangedFilesForPush).not.toHaveBeenCalled(); - expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith('resolve-deploy', { - buildId, + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith( + 'resolve-deploy', + expect.objectContaining({ + buildId, + }) + ); + }); + + test('forwards the pushed commit as triggerRef so distinct commits are not coalesced', async () => { + const buildId = 100; + const mockDeploy = createMockDeploy(buildId); + + let queryCount = 0; + mockDb.models.Deploy.query.mockImplementation(() => { + queryCount++; + return queryCount === 1 ? createAllDeploysQuery([mockDeploy]) : createFailedDeploysQuery([]); + }); + + // Keep the default void `before` so the deploy routes directly without changed-files setup; triggerRef + // derives only from the pushed head commit (`after`). + const pushEvent = { + ...createMockPushEvent(), + after: 'head-commit-sha', + } as PushEvent; + + await githubService.handlePushWebhook(pushEvent); + + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith( + 'resolve-deploy', + expect.objectContaining({ buildId, triggerRef: 'head-commit-sha' }) + ); + }); + + test('omits triggerRef when the pushed head commit is void', async () => { + const buildId = 100; + const mockDeploy = createMockDeploy(buildId); + + let queryCount = 0; + mockDb.models.Deploy.query.mockImplementation(() => { + queryCount++; + return queryCount === 1 ? createAllDeploysQuery([mockDeploy]) : createFailedDeploysQuery([]); }); + + const pushEvent = { + ...createMockPushEvent(), + before: 'previous-commit', + after: '0000000000000000000000000000000000000000', + } as PushEvent; + + await githubService.handlePushWebhook(pushEvent); + + const addCall = mockDb.services.BuildService.resolveAndDeployBuildQueue.add.mock.calls[0]; + expect(addCall[1]).not.toHaveProperty('triggerRef'); + }); + + test('forwards the pushed commit as triggerRef for static environments', async () => { + const buildId = 701; + const repoBuilder = { + from: jest.fn().mockReturnThis(), + select: jest.fn().mockReturnThis(), + where: jest.fn().mockReturnThis(), + }; + const prBuilder = { + from: jest.fn().mockReturnThis(), + select: jest.fn().mockReturnThis(), + where: jest.fn().mockReturnThis(), + whereIn: jest.fn((_column, callback) => { + callback(repoBuilder); + return prBuilder; + }), + }; + const buildQuery = { + whereIn: jest.fn((_column, callback) => { + callback(prBuilder); + return buildQuery; + }), + andWhere: jest.fn().mockReturnThis(), + first: jest.fn().mockResolvedValue({ id: buildId }), + }; + + mockDb.models.Build = { query: jest.fn().mockReturnValue(buildQuery) }; + mockDb.models.PullRequest.tableName = 'pull_requests'; + mockDb.models.Repository = { tableName: 'repositories' }; + mockDb.models.Deploy.query.mockReturnValue(createAllDeploysQuery([])); + + const pushEvent = { + ...createMockPushEvent(), + before: 'previous-commit', + after: 'static-head-sha', + } as PushEvent; + + await githubService.handlePushWebhook(pushEvent); + + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith( + 'resolve-deploy', + expect.objectContaining({ buildId, triggerRef: 'static-head-sha' }) + ); }); test('should not add to queue when PR is closed', async () => { diff --git a/src/server/services/build.ts b/src/server/services/build.ts index c985114..79e1567 100644 --- a/src/server/services/build.ts +++ b/src/server/services/build.ts @@ -165,14 +165,21 @@ export default class BuildService extends BaseService { async enqueueResolveAndDeployBuild({ buildId, githubRepositoryId, + triggerRef, ...jobData }: { buildId: number; githubRepositoryId?: number | null; + triggerRef?: string | null; [key: string]: any; }) { const fingerprint = await this.computeBuildRequestFingerprint(buildId, githubRepositoryId ?? undefined); - const dedupeId = `resolve:${buildId}:${fingerprint}`; + // The fingerprint only captures build configuration, not the commit being deployed. Without a per-trigger + // suffix, two deploys of the same build (e.g. two pushes to a tracked branch landing close together) collapse + // onto one dedupe key, and the later one is silently dropped. triggerRef (the pushed commit, or a redeploy id) + // makes each distinct trigger its own key while still coalescing genuine duplicates of the same trigger. + const suffix = triggerRef ? `:${triggerRef}` : ''; + const dedupeId = `resolve:${buildId}:${fingerprint}${suffix}`; getLogger({ stage: LogStage.BUILD_QUEUED }).info( `Build queue: name=resolve-deploy buildId=${buildId} scope=${githubRepositoryId || 'all'} dedupeKey=${dedupeId}` ); @@ -181,6 +188,7 @@ export default class BuildService extends BaseService { { buildId, ...(githubRepositoryId ? { githubRepositoryId } : {}), + ...(triggerRef ? { triggerRef } : {}), ...jobData, }, { @@ -195,22 +203,37 @@ export default class BuildService extends BaseService { async enqueueBuildJob({ buildId, githubRepositoryId, + triggerRef, ...jobData }: { buildId: number; githubRepositoryId?: number | null; + triggerRef?: string | null; [key: string]: any; }) { const fingerprint = await this.computeBuildRequestFingerprint(buildId, githubRepositoryId ?? undefined); - const jobId = `build:${buildId}:${fingerprint}`; + // Mirror the suffix used by the resolve step so both queue layers agree on identity. A build job is keyed by + // jobId, which makes add() idempotent: an enqueue whose jobId matches an existing job is a no-op rather than new + // work. Including triggerRef ensures a distinct trigger yields a distinct job instead of being dropped. + const suffix = triggerRef ? `:${triggerRef}` : ''; + const jobId = `build:${buildId}:${fingerprint}${suffix}`; getLogger({ stage: LogStage.BUILD_QUEUED }).info( `Build queue: name=build buildId=${buildId} scope=${githubRepositoryId || 'all'} jobId=${jobId}` ); + // Best-effort visibility: a matching job here means this enqueue will be coalesced into existing work rather + // than building. Without this log the drop is invisible, since the dedupe happens inside the queue. + const existing = await this.buildQueue.getJob(jobId); + if (existing) { + getLogger({ stage: LogStage.BUILD_QUEUED }).info( + `Build queue: skipped reason=deduped buildId=${buildId} jobId=${jobId}` + ); + } return this.buildQueue.add( 'build', { buildId, ...(githubRepositoryId ? { githubRepositoryId } : {}), + ...(triggerRef ? { triggerRef } : {}), ...jobData, }, { @@ -441,6 +464,8 @@ export default class BuildService extends BaseService { githubRepositoryId, skipDeletedServiceReconciliation: true, runUUID, + // Use the unique run id as the trigger so an explicit redeploy is never coalesced into a prior deploy. + triggerRef: runUUID, ...extractContextForQueue(), }); @@ -483,10 +508,13 @@ export default class BuildService extends BaseService { } const buildId = build.id; + const runUUID = nanoid(); await this.enqueueResolveAndDeployBuild({ buildId, - runUUID: nanoid(), + runUUID, + // Use the unique run id as the trigger so an explicit redeploy is never coalesced into a prior deploy. + triggerRef: runUUID, correlationId, }); @@ -1910,7 +1938,7 @@ export default class BuildService extends BaseService { * @param done the Bull callback to invoke when we're done */ processResolveAndDeployBuildQueue = async (job) => { - const { sender, correlationId, skipDeletedServiceReconciliation, _ddTraceContext } = job.data; + const { sender, correlationId, skipDeletedServiceReconciliation, triggerRef, _ddTraceContext } = job.data; return withLogContext({ correlationId, sender, _ddTraceContext }, async () => { let jobId; @@ -1938,11 +1966,13 @@ export default class BuildService extends BaseService { getLogger().info('Deploy: skipping reason=deployOnUpdateDisabled'); return; } - // Enqueue a standard resolve build + // Enqueue a standard resolve build. Forward triggerRef so the build job shares the resolve step's dedupe + // identity; otherwise the two layers would disagree and idempotent coalescing of genuine duplicates breaks. await this.enqueueBuildJob({ buildId, githubRepositoryId, skipDeletedServiceReconciliation, + triggerRef, ...extractContextForQueue(), }); } catch (error) { diff --git a/src/server/services/github.ts b/src/server/services/github.ts index c345f50..2de6de1 100644 --- a/src/server/services/github.ts +++ b/src/server/services/github.ts @@ -519,6 +519,10 @@ export default class GithubService extends Service { const branchName = ref.split('refs/heads/')[1]; if (!branchName) return; const hasVoidCommit = [previousCommit, latestCommit].some((commit) => this.isVoidCommit(commit)); + // The pushed head commit is the deploy trigger used to keep distinct commits from collapsing onto one dedupe + // key. Guard on the head commit only (the value that enters the key); a void head means there is nothing to + // deploy, so fall back to the commit-agnostic key. + const deployTriggerRef = latestCommit && !this.isVoidCommit(latestCommit) ? latestCommit : undefined; getLogger({}).debug(`Push event repo=${repoName} branch=${branchName}`); const models = this.db.models; let changedFilesForPush: github.ChangedFilesForPushResult | null = null; @@ -568,7 +572,11 @@ export default class GithubService extends Service { if (!allDeploys.length) { // additional check for static env branch - await this.handlePushForStaticEnv({ githubRepositoryId, branchName }); + await this.handlePushForStaticEnv({ + githubRepositoryId, + branchName, + headCommit: deployTriggerRef ?? null, + }); return; } const deploysToRebuild = allDeploys.filter((deploy) => { @@ -676,6 +684,10 @@ export default class GithubService extends Service { await this.db.services.BuildService.enqueueResolveAndDeployBuild({ buildId, ...(hasFailedDeploys ? {} : { githubRepositoryId }), + // The pushed commit is the deploy trigger. It keeps back-to-back pushes to the same branch from collapsing + // onto one dedupe key (which would silently drop the later commit), while a redelivered webhook for the + // same commit still coalesces. + ...(deployTriggerRef ? { triggerRef: deployTriggerRef } : {}), ...extractContextForQueue(), }); } @@ -693,9 +705,11 @@ export default class GithubService extends Service { handlePushForStaticEnv = async ({ githubRepositoryId, branchName, + headCommit, }: { githubRepositoryId: number; branchName: string; + headCommit?: string | null; }): Promise => { try { const build = await this.db.models.Build.query() @@ -720,6 +734,10 @@ export default class GithubService extends Service { getLogger().info(`Push: redeploying reason=staticEnv`); await this.db.services.BuildService.enqueueResolveAndDeployBuild({ buildId: build?.id, + // The pushed commit is the deploy trigger, so a later push to the tracked branch is not coalesced onto an + // in-flight or recent deploy of an earlier commit. Static envs rebuild on every tracked-branch push by + // design, so this only prevents wrongly dropping a distinct commit. + ...(headCommit ? { triggerRef: headCommit } : {}), ...extractContextForQueue(), }); } catch (error) { From 6486033851b1ab2a9ef7a85461d05ecf48441b7f Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Wed, 17 Jun 2026 09:27:37 -0700 Subject: [PATCH 2/2] fix: apply deploy trigger to comment and override redeploy paths The comment-triggered redeploy (#REDEPLOY) and the override-triggered redeploy already generate a unique run id but passed it only as runUUID, not as the dedupe trigger. As a result they kept the commit-agnostic dedupe key and two redeploys landing close together could still be silently coalesced into one. Pass the existing run id as triggerRef on both paths so each explicit redeploy gets a distinct dedupe key. Also add a test that locks in the resolve-step to build-step triggerRef forwarding, the invariant that keeps both queue layers on the same dedupe identity. --- src/server/services/__tests__/build.test.ts | 25 +++++++++++++++++++ .../services/__tests__/override.test.ts | 7 ++++++ src/server/services/activityStream.ts | 2 ++ src/server/services/override.ts | 2 ++ 4 files changed, 36 insertions(+) diff --git a/src/server/services/__tests__/build.test.ts b/src/server/services/__tests__/build.test.ts index 0d37ee7..5bdfd3e 100644 --- a/src/server/services/__tests__/build.test.ts +++ b/src/server/services/__tests__/build.test.ts @@ -1161,4 +1161,29 @@ describe('BuildService queue fingerprinting', () => { }) ); }); + + test('forwards triggerRef from the resolve job to the build job so both layers share dedupe identity', async () => { + const build = createMockBuild({ + id: 1449, + uuid: 'good-dev-0', + pullRequest: { + latestCommit: 'abcdef123456', + deployOnUpdate: true, + $fetchGraph: jest.fn().mockResolvedValue(undefined), + }, + $fetchGraph: jest.fn().mockResolvedValue(undefined), + }); + mockBuildQuery.findOne.mockResolvedValue(build); + const enqueueBuildJob = jest.spyOn(buildService, 'enqueueBuildJob').mockResolvedValue(undefined as any); + + await buildService.processResolveAndDeployBuildQueue({ + data: { + buildId: 1449, + githubRepositoryId: 425935548, + triggerRef: 'head-commit-sha', + }, + }); + + expect(enqueueBuildJob).toHaveBeenCalledWith(expect.objectContaining({ triggerRef: 'head-commit-sha' })); + }); }); diff --git a/src/server/services/__tests__/override.test.ts b/src/server/services/__tests__/override.test.ts index c4da190..0c62e8d 100644 --- a/src/server/services/__tests__/override.test.ts +++ b/src/server/services/__tests__/override.test.ts @@ -299,6 +299,7 @@ describe('OverrideService.applyBuildOverrides', () => { expect(enqueueResolveAndDeployBuild).toHaveBeenCalledWith({ buildId: 42, runUUID: 'run-uuid', + triggerRef: 'run-uuid', correlationId: 'test-correlation', }); }); @@ -444,6 +445,7 @@ describe('OverrideService.applyBuildOverrides', () => { expect(enqueueResolveAndDeployBuild).toHaveBeenCalledWith({ buildId: 42, runUUID: 'run-uuid', + triggerRef: 'run-uuid', correlationId: 'test-correlation', }); }); @@ -475,6 +477,7 @@ describe('OverrideService.applyBuildOverrides', () => { expect(enqueueResolveAndDeployBuild).toHaveBeenCalledWith({ buildId: 42, runUUID: 'run-uuid', + triggerRef: 'run-uuid', correlationId: 'test-correlation', }); expect(result).toEqual({ @@ -946,6 +949,7 @@ describe('OverrideService.applyBuildConfigPatch', () => { expect(enqueueResolveAndDeployBuild).toHaveBeenCalledWith({ buildId: 42, runUUID: 'run-uuid', + triggerRef: 'run-uuid', correlationId: 'test-correlation', }); expect(result).toMatchObject({ @@ -978,6 +982,7 @@ describe('OverrideService.applyBuildConfigPatch', () => { expect(enqueueResolveAndDeployBuild).toHaveBeenCalledWith({ buildId: 42, runUUID: 'run-uuid', + triggerRef: 'run-uuid', correlationId: 'test-correlation', }); expect(result).toMatchObject({ @@ -1081,6 +1086,7 @@ describe('OverrideService.applyBuildConfigPatch', () => { expect(enqueueResolveAndDeployBuild).toHaveBeenCalledWith({ buildId: 42, runUUID: 'run-uuid', + triggerRef: 'run-uuid', correlationId: 'test-correlation', }); expect(result).toMatchObject({ @@ -1134,6 +1140,7 @@ describe('OverrideService.applyBuildConfigPatch', () => { expect(mockFallbackEnqueueResolveAndDeployBuild).toHaveBeenCalledWith({ buildId: 42, runUUID: 'run-uuid', + triggerRef: 'run-uuid', correlationId: 'test-correlation', }); }); diff --git a/src/server/services/activityStream.ts b/src/server/services/activityStream.ts index 6430ebc..80c8a3e 100644 --- a/src/server/services/activityStream.ts +++ b/src/server/services/activityStream.ts @@ -149,6 +149,8 @@ export default class ActivityStream extends BaseService { await this.db.services.BuildService.enqueueResolveAndDeployBuild({ buildId, runUUID: runUuid, + // Use the unique run id as the trigger so an explicit redeploy is never coalesced into a prior deploy. + triggerRef: runUuid, ...extractContextForQueue(), }); return; diff --git a/src/server/services/override.ts b/src/server/services/override.ts index 9c03b17..2a4d2f5 100644 --- a/src/server/services/override.ts +++ b/src/server/services/override.ts @@ -558,6 +558,8 @@ export default class OverrideService extends BaseService { await buildService.enqueueResolveAndDeployBuild({ buildId: build.id, runUUID: runUuid, + // Use the unique run id as the trigger so an explicit redeploy is never coalesced into a prior deploy. + triggerRef: runUuid, ...extractContextForQueue(), }); return true;