feat(deploy): --external-id and --force for deploy idempotency - #4663
feat(deploy): --external-id and --force for deploy idempotency#46630ski wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 2e065ce The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
A deploy can carry an opaque external id (commit SHA, CI run id, release tag). Repeating an id that already deployed returns the existing version as a no-op instead of rebuilding; an id with a build in flight is rejected with 409 naming that version; a failed id rebuilds freely. --force is non-destructive to deployments that already succeeded - both persist and the higher version wins - but cancels a build still in flight, so one id never has two live builds racing to define it. Cancelling writes a terminal status and appends a finalized event, which aborts a build the platform drives; a build it does not drive keeps running but can never land, and the CLI says so. Ids are deliberately not unique - reuse is resolved in application code by highest version, never timestamps. The no-op path mints no build credentials and no event stream (TRI-12923). What that means for callers: a --force rebuild leaves two deployments holding one id, and runs triggered with it go to the higher version once the rebuild lands, so the takeover needs no separate promotion. Until a successful build exists for an id, runs triggered with it park and then expire rather than falling back to current - a failed build is therefore visible to the caller as expired runs, not as runs on the wrong release.
18ec657 to
2e065ce
Compare
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
| const candidates = await prisma.workerDeployment.findMany({ | ||
| where: { environmentId, externalId }, | ||
| select: { | ||
| id: true, | ||
| friendlyId: true, | ||
| shortCode: true, | ||
| version: true, | ||
| status: true, | ||
| contentHash: true, | ||
| imageReference: true, | ||
| imagePlatform: true, | ||
| externalId: true, | ||
| promotions: { | ||
| where: { label: CURRENT_DEPLOYMENT_LABEL }, | ||
| select: { id: true }, | ||
| }, | ||
| }, | ||
| orderBy: { id: "desc" }, | ||
| take: MAX_CANDIDATES, | ||
| }); |
There was a problem hiding this comment.
🟡 Reusing the same deploy id many times can silently rebuild or leave two builds racing
Only the 20 most recent deployments for a given id are examined (take: MAX_CANDIDATES at apps/webapp/app/v3/services/initializeDeployment/resolveExternalIdReuse.server.ts:68) when deciding whether to reuse or cancel, so older records for that id are invisible, meaning a repeat deploy can rebuild instead of doing nothing, and a forced deploy can leave an older build still running for the same id.
Impact: A user who deploys the same id more than twenty times may get an unexpected rebuild, or end up with two builds competing to define that id.
How the candidate cap interacts with reuse resolution
resolveExternalIdReuse fetches at most 20 rows for (environmentId, externalId) ordered by id desc, then partitions them into in-flight and DEPLOYED sets (apps/webapp/app/v3/services/initializeDeployment/resolveExternalIdReuse.server.ts:71-92). Since ids are deliberately non-unique (each --force adds another row, and failed attempts add rows too), a single external id can accumulate more than 20 rows over time. Once it does:
- A
DEPLOYEDrow that falls outside the newest 20 is not seen, so a plain repeat deploy returns{ action: "build" }and rebuilds instead of short-circuiting to the existing version. - An in-flight row outside the newest 20 is not passed to
cancelSupersededDeployments(apps/webapp/app/v3/services/initializeDeployment.server.ts:191-199), so--forceleaves it running, breaking the stated invariant that one id never has two live builds.
Consider filtering/ordering in SQL (e.g. query in-flight rows separately without a cap, and select the highest DEPLOYED version) rather than truncating the candidate set client-side.
Prompt for agents
In apps/webapp/app/v3/services/initializeDeployment/resolveExternalIdReuse.server.ts, the reuse decision is made from at most MAX_CANDIDATES (20) rows fetched with orderBy id desc for a given (environmentId, externalId). External ids are intentionally not unique — every --force rebuild and every failed attempt adds another row — so a frequently reused id can exceed 20 rows. When it does, older rows become invisible to the resolver: a DEPLOYED row outside the newest 20 causes an unnecessary rebuild instead of the intended no-op, and an in-flight row outside the newest 20 is never handed to cancelSupersededDeployments, so --force can leave a second live build defining the same id. Consider pushing the partitioning into the database: query in-flight rows (status not in FINAL_DEPLOYMENT_STATUSES) for the id without a cap (or with a much higher bound), and separately fetch the highest-version DEPLOYED row, instead of truncating a single unfiltered candidate list.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (reuse.action === "short-circuit") { | ||
| span.setAttribute("outcome", "existing"); | ||
|
|
||
| logger.debug("Reusing deployed external id, skipping build", { | ||
| environmentId: environment.id, | ||
| projectId: environment.projectId, | ||
| externalId: payload.externalId, | ||
| version: reuse.deployment.version, | ||
| }); | ||
|
|
||
| return { | ||
| outcome: "existing", | ||
| deployment: reuse.deployment, | ||
| imageRef: reuse.deployment.imageReference ?? "", | ||
| isPromoted: reuse.isPromoted, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🔍 Promotion intent is silently dropped on the no-op path
When an external id short-circuits to an already DEPLOYED version, nothing touches promotions: a version that was originally deployed with --skip-promotion stays unpromoted even if the repeat deploy omits --skip-promotion. The CLI only warns ("Promote it from the dashboard") and sets needsPromotion in the GitHub Actions output. Worth confirming this is the desired contract for CI pipelines that expect a plain deploy to make the version current — a deliberate promote on the short-circuit path may be a friendlier behavior.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
A deploy can carry an opaque external id (commit SHA, CI run id, release tag). Repeating an id that already deployed returns the existing version as a no-op instead of rebuilding; an id with a build in flight is rejected with 409 naming that version; a failed id rebuilds freely. --force is non-destructive to deployments that already succeeded - both persist and the higher version wins - but cancels a build still in flight, so one id never has two live builds racing to define it. Cancelling writes a terminal status and appends a finalized event, which aborts a build the platform drives; a build it does not drive keeps running but can never land, and the CLI says so. Ids are deliberately not unique - reuse is resolved in application code by highest version, never timestamps. The no-op path mints no build credentials and no event stream (TRI-12923).
What that means for callers: a --force rebuild leaves two deployments holding one id, and runs triggered with it go to the higher version once the rebuild lands, so the takeover needs no separate promotion. Until a successful build exists for an id, runs triggered with it park and then expire rather than falling back to current - a failed build is therefore visible to the caller as expired runs, not as runs on the wrong release.