feat(openworkflow, server, dashboard): add resume workflow run functionality and UI components#624
Open
vudc wants to merge 4 commits into
Open
feat(openworkflow, server, dashboard): add resume workflow run functionality and UI components#624vudc wants to merge 4 commits into
vudc wants to merge 4 commits into
Conversation
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Backend correctness: - Delete every non-successful step attempt on resume, not just failed ones. Sleep, signal-wait and child-workflow attempts sit in 'running' and could survive a resume, making replay treat the step as in-flight instead of re-executing it. - Clear started_at so a resumed run reports its new lifecycle, matching rescheduleWorkflowRunAfterFailedStepAttempt. - Restructure the SQLite transaction so the conflict lookup happens outside it, removing the double ROLLBACK on an already-closed transaction. - Read the conflicting run through the transaction in Postgres instead of the outer connection, and use the shared withTransaction helper. Tests: - Add a resumeWorkflowRun() block to the shared backend testsuite so both SQLite and Postgres are covered; the SQLite path had none. - Cover the non-existent-run branch of resolveResumeWorkflowRunConflict. - Assert started_at is cleared and only successful attempts survive. Remove the flaky-payment demo, which drove failures from module-scoped mutable state in a workflow handler.
Correctness: - Narrow the step-attempt cleanup to failed attempts plus inert running attempts (function, signal-send). Running sleep, signal-wait and workflow attempts are now preserved: deleting them orphaned live child runs via ON DELETE SET NULL, dropped already-delivered signals, and restarted durable timers from zero. - Reset the run-level attempts counter on resume so a resumed run gets the fresh retry budget the UI promises. - Gate resume on an unexpired deadline and reject deadline-expired runs with a clear error instead of destroying their failure history for a run the next tick would immediately re-fail. Dashboard: - Extract the shared RunActionDialog so resume and cancel stop tripping the zero-tolerance jscpd duplication gate. - Use .validator instead of the removed .inputValidator, matching the sibling server functions. Tests: - Rewrite the attempt-cleanup test to assert in-flight attempts survive, and add a child-workflow test proving the linked child is not orphaned (both fail against the previous broad DELETE). - Add deadline-rejection and attempts-reset coverage. - Give the end-to-end resume test a 20s wait budget so the flaky step's 1s retry backoff fits inside it.
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.
Closes #520
Supersedes #521, which I was unable to reopen (GitHub refuses the transition on both the GraphQL and REST endpoints, with no reason given). This PR is the same work, rebased onto current
mainand with the outstanding review items addressed.Summary
Adds
resumeWorkflowRun(workflowRunId)to the client and backend, flipping a failed workflow run back topendingso the worker picks it up on the next tick. Completed steps are served from the existing step-attempt cache and are not re-executed; the failing step starts with a fresh retry budget. A "Resume Run" button is wired up on the run-detail page of the dashboard.Changes
Backend
Backend.resumeWorkflowRun({ workflowRunId })added to the interface, implemented for Postgres and SQLite. In a single transaction it flipsstatusfromfailedtopending, clearserror/worker_id/started_at/finished_at, setsavailable_at = NOW(), and deletes the run's unsuccessfulstep_attempts. Non-failed runs throw a clear error.ow.resumeWorkflowRun(workflowRunId)client method.resolveResumeWorkflowRunConflictincore/workflow-run.ts.Dashboard
RunResumeActioncomponent with a confirmation dialog, following the existingRunCancelActionstructure.resumeWorkflowRunServerFninlib/api.ts, alongsidecancelWorkflowRunServerFn.isRunResumableStatushelper inlib/status.ts(currently"failed"only).Tests
resumeWorkflowRun()block in the sharedtesting/backend.testsuite.ts, so both SQLite and Postgres are covered. The previous revision tested only throughworker/execution.test.ts, which runs against Postgres, leaving the SQLite implementation untested.worker/execution.test.ts: a happy path asserting the completed step is not re-executed, and a negative path for a non-failed run.What changed since #521
Rebased onto current
main, plus the following review fixes.Only failed step attempts were deleted.
StepAttemptStatusincludesrunning, and sleep, signal-wait and child-workflow attempts stay in that state for their whole lifetime. A run can reachfailedwith one stillrunning, and sincestep-history.tsbuildsrunningByStepNamefrom whatever attempts it loads, a surviving row made replay treat that step as in-flight instead of re-executing it. Resume now deletes everything that is notcompletedorsucceeded. The new testsuite case fails against the old query, with the leftoverrunning-stepattempt surviving the resume.started_atwas not cleared.rescheduleWorkflowRunAfterFailedStepAttemptexplicitly nulls it when sending a run back topending; resume now matches.SQLite transaction control flow.
resolveResumeWorkflowRunConflictthrew from inside thetry, so thecatchissued a secondROLLBACKon an already-rolled-back transaction, which better-sqlite3 rejects and the innercatch {}swallowed.BEGINnow sits outside thetry(matching the other transactional methods in this file) and the conflict lookup happens after the transaction closes.Postgres read outside the transaction. The conflict lookup called
this.getWorkflowRun, which uses the outer connection. It now reads throughtx, and the method uses the sharedwithTransactionhelper rather thanpg.beginwith a cast.Removed the
flaky-paymentdemo. It drove failures from module-scoped mutable state in a workflow handler, which is not a pattern worth demonstrating and made the demo dependent on worker process lifetime.Review items not changed, with reasoning
RETURNING *returning snake_case columns.cancelWorkflowRunandmarkWorkflowRunFailedin the same file use the identicalthis.pg<WorkflowRun[]>plusRETURNING *pattern; the column transform is configured globally.AlertDialogTrigger.RunCancelActionuses the same controlledopenstate with a sibling<Button>. Changing only the resume component would leave the two inconsistent. Worth doing for both in a separate PR if you would like.RunCancelAction.cancelWorkflowRunServerFnand the rest of the dashboard's server functions.flakyPayment.specmay not exist. Moot now that the demo is removed; it was the same accesshello-world.run.tsuses.Testing
tsc --noEmitis clean. The SQLite and core suites pass locally (370 tests). I was not able to run the Postgres-backed suites in my environment, so the Postgres path relies on CI.