Skip to content

fix(deployments): stop superseded activations from dead-lettering - #6522

Open
icecrasher321 wants to merge 1 commit into
stagingfrom
staging-v33
Open

fix(deployments): stop superseded activations from dead-lettering#6522
icecrasher321 wants to merge 1 commit into
stagingfrom
staging-v33

Conversation

@icecrasher321

@icecrasher321 icecrasher321 commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

What

29 workflow.deployment.prepare.v2 outbox events dead-lettered with Webhook registration operation is stale, spanning 2026-07-21 → 2026-08-09 across 13 workflows. Every one sat at attempts = max_attempts.

That full retry budget is the tell: the failure is deterministic, which rules out the preparation path. An attempt superseded while preparing is marked superseded, so its next attempt short-circuits at the top of the handler and completes — it would die at attempts = 1, never 4.

The branch a retry re-enters is the other one. isTerminalNonActiveOperation covers failed and superseded but not active, so an attempt that activated and was then superseded by the next deploy keeps its own active status, re-enters post-activation work on every retry, and re-fails the same generation fence until the event dies.

The fence it fails is correct — it takes the same workflow row lock the generation bump takes, and compares generations exactly. Nothing about the detection is racy; only the reaction to it was wrong. Reaching it at all needs a handler timeout, which parks the row for the 10-minute reaper instead of the 2s/4s/8s backoff, opening a window wide enough for a redeploy to land. That is why the same workflow dominates both this class and the Outbox handler timed out after 90000ms class.

Changes

Gate the resume branch on still owning the current generation, matching the sibling cleanup that already does this, and complete the event as a no-op when it does not. The newer generation adopts the leftover work anyway — it collects every retired registration below its own fence, so nothing leaks.

Reverse the post-activation order. The audit entry, analytics event, socket notification, and workspace event describe a cutover that is already durable, and each is separately checkpointed — but they ran behind retiring the previous generation's external subscriptions, one provider call per retired row and by far the most failure-prone step there. A single flaky provider silently cost the deploy its audit trail and left clients on the old version until something else refreshed them. Both call sites now share one helper so the order cannot drift apart again.

Notes

  • No new guards, try/catch, or fallbacks — the gate completes an existing terminal-state short-circuit that was missing a case, and the reorder moves existing work.
  • Zero pre-existing tests changed; all test diffs are additive. The one deletion hoists a shared runInTx helper to module scope for a new describe block.
  • Three new tests, each verified to fail against the unfixed source: superseded-resume no-op, still-current resume, and notify-before-cleanup ordering.
  • Not covered: whatever makes that workflow's prepare step slow enough to hit 90s. This stops it manufacturing dead letters; it does not make it faster.

🤖 Generated with Claude Code

@vercel

vercel Bot commented Aug 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 11, 2026 3:31am

Request Review

@cursor

cursor Bot commented Aug 11, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches deployment outbox orchestration and post-activation ordering on a critical path, but changes are narrow guards and reordering of existing steps with additive tests rather than new failure modes.

Overview
Fixes workflow.deployment.prepare.v2 events that kept retrying after cutover when a newer deploy superseded the generation. An operation that already reached active was not treated like superseded, so each resume re-entered post-activation work, hit stale webhook-generation fences, and could exhaust retries with Webhook registration operation is stale.

On resume with active, the handler now checks isDeploymentOperationCurrent and no-ops when a newer generation owns the workflow (logged with deployment_operation_superseded). Post-activation steps for both fresh activation and resume go through runPostActivationWork, which runs audit, analytics, socket, and workspace notifications before retiring old webhook subscriptions and inactive-version cleanup—so flaky or slow provider cleanup no longer blocks deploy visibility.

Adds regression tests for superseded-resume no-op, still-current resume, redeploy race in webhook registration store, and notify-before-cleanup ordering.

Reviewed by Cursor Bugbot for commit c6c5f56. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR makes resumed deployment preparation stop when a newer generation owns the workflow and moves durable post-activation notifications ahead of fallible external cleanup.

  • Adds a named superseded-operation outcome for deployment lifecycle logging.
  • Reuses a common post-activation sequence for initial and resumed processing.
  • Adds coverage for racing redeploys, superseded resumes, current resumes, and notification ordering.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/lib/workflows/deployment-outbox.ts Adds the resumed-operation generation guard and centralizes notification-first post-activation processing.
apps/sim/lib/workflows/deployment-lifecycle.ts Adds a stable logging identity for benign operation supersession without classifying it as a deployment failure.
apps/sim/lib/workflows/deployment-outbox.test.ts Covers superseded and current active-operation resumes plus post-activation ordering when provider cleanup fails.
apps/sim/lib/webhooks/registration-store.test.ts Adds a race-oriented harness and verifies that a superseded registration attempt cannot write while the newer generation can activate.

Sequence Diagram

sequenceDiagram
  participant O as Deployment outbox
  participant G as Generation store
  participant S as Post-activation effects
  participant W as Retired webhook cleanup
  participant D as Inactive deployment cleanup
  O->>G: Check operation still owns generation
  alt Superseded
    G-->>O: Not current
    O-->>O: Log benign hand-off and stop
  else Current
    G-->>O: Current
    O->>S: Emit/checkpoint audit and notifications
    O->>W: Retire old external subscriptions
    O->>D: Clean inactive deployment versions
  end
Loading

Reviews (3): Last reviewed commit: "fix(deployments): stop superseded activa..." | Re-trigger Greptile

Comment thread apps/sim/lib/billing/entitlement-drift.ts Outdated
Comment thread apps/sim/lib/billing/entitlement-drift.ts Outdated
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8fc4c4c. Configure here.

29 workflow.deployment.prepare.v2 events dead-lettered with "Webhook
registration operation is stale", every one at attempts = max_attempts. A full
retry budget means the failure is deterministic, which rules out the
preparation path: an attempt superseded while preparing is marked superseded,
so its next attempt short-circuits at the top of the handler and completes.

The branch a retry re-enters is the other one. isTerminalNonActiveOperation
covers failed and superseded but not active, so an attempt that activated and
was then superseded by the next deploy keeps its own active status, re-enters
post-activation work on every retry, and re-fails the same generation fence
until the event dies. The fence it fails is correct — it takes the same
workflow row lock the generation bump takes, and compares generations exactly
— so nothing about the detection is racy; only the reaction to it was wrong.
Reaching it needs a handler timeout, which parks the row for the 10-minute
reaper instead of the 2s/4s/8s backoff, opening a window wide enough for a
redeploy to land.

Gate the resume branch on the operation still owning the current generation,
matching the sibling cleanup that already does this, and complete the event as
a no-op when it does not. The newer generation adopts the leftover work
anyway: it collects every retired registration below its own fence.

Also reverse the post-activation order. The audit entry, analytics event,
socket notification, and workspace event describe a cutover that is already
durable, and each is separately checkpointed, but they ran behind retiring the
previous generation's external subscriptions — one provider call per retired
row, and by far the most failure-prone step there. A single flaky provider
silently cost the deploy its audit trail and left clients on the old version
until something else refreshed them. Both call sites now share one helper so
the order cannot drift apart again.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@icecrasher321 icecrasher321 changed the title fix(billing,deployments): recover silently dropped post-event writes fix(deployments): stop superseded activations from dead-lettering Aug 11, 2026
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

Dropped the billing reconciler from this PR — the root cause it backstopped is already fixed by #6510, and the three affected users need a one-time backfill rather than a permanent sweep. This PR is now deployment-only: 4 files, +212/-33.

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit c6c5f56. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant