fix(cloudflare/workflow): harden Workflow reconcile + add lifecycle convergence tests#234
Open
sam-goodwin wants to merge 1 commit intomainfrom
Open
fix(cloudflare/workflow): harden Workflow reconcile + add lifecycle convergence tests#234sam-goodwin wants to merge 1 commit intomainfrom
sam-goodwin wants to merge 1 commit intomainfrom
Conversation
…onvergence tests Cloudflare Workflow's reconcile previously did a blind PUT-as-upsert on every deploy and trusted that the API would converge. That's correct for the happy path but skips observation entirely — which means an idempotent redeploy still re-PUTs, and there's no signal when desired state already matches live state. - Observe live state via getWorkflow first; swallow only WorkflowNotFound (auth/throttling/5xx surface for engine retry, not as "no workflow live") - Skip the put when observed className+scriptName already match desired — no API churn on no-op redeploys - Adoption (output defined, olds undefined) now converges through the same observe path - Add lifecycle convergence tests: idempotent redeploy preserves workflow id, scriptName drift via raw putWorkflow is overwritten, out-of-band deleteWorkflow triggers re-create, double-destroy is a no-op - Pre-existing output?.stableId TS error from #167/#179 in test.resources.ts is fixed the same way the prior hardening PRs did No distilled patch needed — Cloudflare's client/api.ts already maps HTTP status to categorized errors (TooManyRequests → throttling+retryable, 5xx → server+retryable), and WorkflowNotFound covers the only NotFound-like code the reconciler routes on. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
|
Install the packages built from this commit: alchemy bun add alchemy@https://pkg.ing/alchemy/06f9d84@alchemy.run/better-auth bun add @alchemy.run/better-auth@https://pkg.ing/@alchemy.run/better-auth/06f9d84@alchemy.run/pr-package bun add @alchemy.run/pr-package@https://pkg.ing/@alchemy.run/pr-package/06f9d84 |
Contributor
Website Preview DeployedURL: https://alchemyeffectwebsite-worker-pr-234-eua2x4z3w3sbk3v2.testing-2b2.workers.dev Built from commit This comment updates automatically with each push. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hardens the Cloudflare Workflow reconciler. Previously every reconcile did a blind PUT regardless of live state, so every redeploy re-PUT and there was no signal when desired state already matched.
Reconciler changes
Observe live state first via
getWorkflow. OnlyWorkflowNotFoundis swallowed — auth, throttling, and 5xx errors must surface so the engine can retry or report cleanly. Skip the PUT when observed state already matches desired (no API churn on no-op redeploys).New lifecycle tests
Each test runs
destroy → deploy → … → destroyagainst a real Cloudflare account.scriptNamere-targeted out-of-band via rawputWorkflowWorkflowNotFoundin observe)deleteWorkflowcatchesWorkflowNotFound)Workflow has no
readlifecycle soadopt(true)and "owned without--adopt" cases don't apply here. Physical-name (workflowName) replace is also outside scope:workflowNamederives from the user-facing class name inCloudflare.Workflow("X", ...)and the engine's Resource diff handles class-rename as a logical-id change.The pre-existing
output?.stableIdTS error from #167/#179 intest.resources.tsis fixed the same way the prior hardening PRs did.Distilled patch
No patch needed. Cloudflare's
client/api.tsalready maps HTTP status to categorized errors (TooManyRequests→ throttling+retryable, 5xx → server+retryable), andWorkflowNotFoundcovers the only NotFound-like code the reconciler routes on.