Skip to content

Commit af46917

Browse files
fix(deploy): release a settled idempotency key so redeploys aren't permanently blocked
Redeploying a workflow could fail forever with "Idempotency key was already used for a different deployment request", with no way to clear it from the workflow. The deploy orchestrator scopes one idempotency key to a whole request (`idempotencyKey: params.requestId`) while the request hash is derived from workflow state. That is fine when one request performs one deployment, but the agent path issues several deployments per chat request: deploy, edit, redeploy. The second deployment presents the same key with a new hash, so prepareWorkflowDeployment took the conflict branch — and kept taking it, because the key was never released. Two problems, both in the same block: - The request-hash comparison ran BEFORE the terminal-status check, so the release path was unreachable whenever the hash differed. Its own comment ("a terminally failed or superseded attempt releases its idempotency key") therefore never applied to the case that needed it. - A completed (`active`) operation was not treated as settled at all, so a successful deployment reserved its key permanently. Reordered so the key is released whenever the prior attempt is settled (`active`, `failed`, or `superseded`) and the request genuinely differs. A conflict is now raised only when a *different* deployment is still in flight (`pending`/`preparing`/`activating`), which is the ambiguous case idempotency is meant to guard. True duplicate submissions (same key, same hash) still reuse the existing operation and never redeploy. Observed on staging: one chat request (58a47006) deployed three workflows, then its redeploy of the first failed five times in a row with this error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018asmKsWQ5Vi7T7wD9uHofz
1 parent 79c57bf commit af46917

2 files changed

Lines changed: 151 additions & 9 deletions

File tree

apps/sim/lib/workflows/persistence/deployment-operations.test.ts

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,126 @@ describe('deployment operation persistence', () => {
308308
expect(dbChainMockFns.insert).toHaveBeenCalled()
309309
})
310310

311+
/**
312+
* Callers scope one idempotency key to a whole request (the deploy orchestrator
313+
* passes `requestId`), so a single request that deploys the same workflow twice —
314+
* e.g. the agent redeploying after edits within one chat request — presents the
315+
* same key with a new request hash. Once the first deployment settled, the key
316+
* must be reassignable or that caller is locked out permanently.
317+
*/
318+
it('releases the key of a completed (active) operation when the request differs', async () => {
319+
const completed = operationRow({
320+
id: 'operation-active',
321+
idempotencyKey: 'deploy-1',
322+
requestHash: 'original-hash',
323+
status: 'active',
324+
generation: 2,
325+
})
326+
const fresh = operationRow({ id: 'operation-fresh', generation: 3 })
327+
mockGenerateId.mockReturnValueOnce('version-fresh').mockReturnValueOnce('operation-fresh')
328+
dbChainMockFns.for.mockResolvedValueOnce([{ id: WORKFLOW_ID, archivedAt: null }])
329+
dbChainMockFns.limit
330+
.mockResolvedValueOnce([completed])
331+
.mockResolvedValueOnce([])
332+
.mockResolvedValueOnce([{ maxVersion: 2 }])
333+
.mockResolvedValueOnce([{ maxGeneration: 2 }])
334+
dbChainMockFns.returning.mockResolvedValueOnce([fresh])
335+
336+
const result = await prepareWorkflowDeployment({
337+
workflowId: WORKFLOW_ID,
338+
actorId: 'user-1',
339+
requestHash: 'different-hash',
340+
idempotencyKey: 'deploy-1',
341+
workflowState: workflowState(),
342+
readinessComponents: ['webhooks'],
343+
})
344+
345+
expect(result).toEqual({ success: true, operation: fresh, reused: false })
346+
expect(dbChainMockFns.set).toHaveBeenCalledWith(
347+
expect.objectContaining({ idempotencyKey: null })
348+
)
349+
expect(dbChainMockFns.insert).toHaveBeenCalled()
350+
})
351+
352+
it('releases the key of a failed operation even when the request differs', async () => {
353+
const failed = operationRow({
354+
id: 'operation-failed',
355+
idempotencyKey: 'deploy-1',
356+
requestHash: 'original-hash',
357+
status: 'failed',
358+
generation: 2,
359+
})
360+
const fresh = operationRow({ id: 'operation-fresh', generation: 3 })
361+
mockGenerateId.mockReturnValueOnce('version-fresh').mockReturnValueOnce('operation-fresh')
362+
dbChainMockFns.for.mockResolvedValueOnce([{ id: WORKFLOW_ID, archivedAt: null }])
363+
dbChainMockFns.limit
364+
.mockResolvedValueOnce([failed])
365+
.mockResolvedValueOnce([])
366+
.mockResolvedValueOnce([{ maxVersion: 2 }])
367+
.mockResolvedValueOnce([{ maxGeneration: 2 }])
368+
dbChainMockFns.returning.mockResolvedValueOnce([fresh])
369+
370+
const result = await prepareWorkflowDeployment({
371+
workflowId: WORKFLOW_ID,
372+
actorId: 'user-1',
373+
requestHash: 'different-hash',
374+
idempotencyKey: 'deploy-1',
375+
workflowState: workflowState(),
376+
readinessComponents: ['webhooks'],
377+
})
378+
379+
expect(result).toEqual({ success: true, operation: fresh, reused: false })
380+
expect(dbChainMockFns.insert).toHaveBeenCalled()
381+
})
382+
383+
it('still rejects a different request while an operation is mid-activation', async () => {
384+
dbChainMockFns.for.mockResolvedValueOnce([{ id: WORKFLOW_ID, archivedAt: null }])
385+
dbChainMockFns.limit.mockResolvedValueOnce([
386+
operationRow({
387+
idempotencyKey: 'deploy-1',
388+
requestHash: 'original-hash',
389+
status: 'activating',
390+
}),
391+
])
392+
393+
const result = await prepareWorkflowDeployment({
394+
workflowId: WORKFLOW_ID,
395+
actorId: 'user-1',
396+
requestHash: 'different-hash',
397+
idempotencyKey: 'deploy-1',
398+
workflowState: workflowState(),
399+
})
400+
401+
expect(result).toEqual({
402+
success: false,
403+
reason: 'idempotency_conflict',
404+
error: 'Idempotency key was already used for a different deployment request',
405+
})
406+
expect(dbChainMockFns.insert).not.toHaveBeenCalled()
407+
expect(dbChainMockFns.update).not.toHaveBeenCalled()
408+
})
409+
410+
it('reuses a completed (active) operation for a true duplicate request', async () => {
411+
const completed = operationRow({
412+
idempotencyKey: 'deploy-1',
413+
requestHash: 'hash-1',
414+
status: 'active',
415+
})
416+
dbChainMockFns.for.mockResolvedValueOnce([{ id: WORKFLOW_ID, archivedAt: null }])
417+
dbChainMockFns.limit.mockResolvedValueOnce([completed])
418+
419+
const result = await prepareWorkflowDeployment({
420+
workflowId: WORKFLOW_ID,
421+
actorId: 'user-1',
422+
requestHash: 'hash-1',
423+
idempotencyKey: 'deploy-1',
424+
workflowState: workflowState(),
425+
})
426+
427+
expect(result).toEqual({ success: true, operation: completed, reused: true })
428+
expect(dbChainMockFns.insert).not.toHaveBeenCalled()
429+
})
430+
311431
it('rejects stale generation callbacks before mutating legacy deployment state', async () => {
312432
dbChainMockFns.for
313433
.mockResolvedValueOnce([{ id: WORKFLOW_ID }])

apps/sim/lib/workflows/persistence/deployment-operations.ts

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -751,22 +751,44 @@ async function prepareOperation(
751751
)
752752
.limit(1)
753753
if (existing) {
754-
if (existing.requestHash !== requestHash) {
754+
/**
755+
* `spent` — the attempt ended without deploying, so it can never be
756+
* handed back as a successful duplicate.
757+
* `settled` — the attempt reached any terminal state (including a
758+
* completed `active` deploy), so no work is still in flight under this
759+
* key.
760+
*/
761+
const spent = existing.status === 'failed' || existing.status === 'superseded'
762+
const settled = spent || existing.status === 'active'
763+
764+
if (existing.requestHash === requestHash) {
765+
// A genuine duplicate submission: hand back the operation that is
766+
// already doing (or has already done) the work.
767+
if (!spent) {
768+
return { success: true, operation: existing, reused: true }
769+
}
770+
} else if (!settled) {
771+
/**
772+
* A *different* deployment is still in flight under this key. Which
773+
* one the caller meant is genuinely ambiguous, so refuse rather than
774+
* race two payloads through the same key.
775+
*/
755776
return {
756777
success: false,
757778
reason: 'idempotency_conflict',
758779
error: 'Idempotency key was already used for a different deployment request',
759780
}
760781
}
761-
if (existing.status !== 'failed' && existing.status !== 'superseded') {
762-
return { success: true, operation: existing, reused: true }
763-
}
764782
/**
765-
* A terminally failed or superseded attempt releases its idempotency
766-
* key: duplicate-submission protection must never pin a retry to a
767-
* spent attempt — the caller would get success with no live work
768-
* behind it. The key moves to the fresh operation created below so
769-
* later duplicates of the same request reuse that one instead.
783+
* The key belongs to a settled attempt, so it is free to reassign:
784+
* duplicate-submission protection must never pin a retry to a spent
785+
* attempt (the caller would get success with no live work behind it),
786+
* nor permanently reserve a key that a later, genuinely different
787+
* deployment needs. Callers that scope one key to a whole request —
788+
* e.g. the agent redeploying an edited workflow within a single chat
789+
* request — would otherwise be locked out for good once the first
790+
* deployment settled. The key moves to the fresh operation created
791+
* below so later duplicates of the same request reuse that one instead.
770792
*/
771793
await tx
772794
.update(workflowDeploymentOperation)

0 commit comments

Comments
 (0)