allow resuming#205
Conversation
📝 WalkthroughWalkthroughThis pull request implements a workflow retry mechanism enabling workflows to resume from specific steps after failure. It includes client-facing API, server-side mutations, step-management utilities, validation updates, documentation, and comprehensive tests. Changes
Sequence DiagramsequenceDiagram
participant Client as Application Code
participant API as WorkflowManager API
participant Mutation as Retry Mutation
participant DB as Database
participant Engine as Workflow Engine
participant Pool as Workpool
Client->>API: retry(ctx, workflowId, {from, startAsync})
API->>Mutation: Invoke retry mutation with normalized args
Mutation->>DB: Get workflow & validate completion
activate DB
DB-->>Mutation: Workflow with runResult
deactivate DB
alt from specified
Mutation->>DB: Delete steps from specified point
activate DB
DB-->>Mutation: Cascade cleanup nested workflows
deactivate DB
end
Mutation->>DB: Increment generationNumber & clear runResult
activate DB
DB-->>Mutation: Updated workflow
deactivate DB
alt startAsync true
Mutation->>Pool: Enqueue via workpool
Pool-->>Engine: Async execution
else startAsync false
Mutation->>Engine: Direct runMutation
end
Engine-->>Client: Workflow resumed from retry point
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
commit: |
acee365 to
b7fdb37
Compare
065ed19 to
6c9bd72
Compare
…kflow when not checking generation number
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/component/workflow.ts (1)
479-492: Consider extractinggetWorkpooloutside the loop.
getWorkpoolis called inside the loop for each workflow step. If there are multiple nested workflow steps to clean up, this results in repeated workpool instantiation. Consider moving it outside the loop for efficiency.♻️ Suggested refactor
async function deleteSteps(ctx: MutationCtx, steps: Doc<"steps">[]) { + let workpool: Awaited<ReturnType<typeof getWorkpool>> | undefined; for (const entry of steps) { await ctx.db.delete(entry._id); if (entry.step.kind === "event" && entry.step.eventId) { await ctx.db.delete(entry.step.eventId); } else if (entry.step.kind === "workflow" && entry.step.workflowId) { - const workpool = await getWorkpool(ctx, {}); + workpool ??= await getWorkpool(ctx, {}); await workpool.enqueueMutation(ctx, api.workflow.cleanup, { workflowId: entry.step.workflowId, force: true, }); } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/component/workflow.ts` around lines 479 - 492, deleteSteps currently calls getWorkpool inside the for-loop causing repeated instantiation when multiple steps are workflow kind; refactor by calling getWorkpool once before the loop (e.g., const workpool = await getWorkpool(ctx, {})) and reuse that instance when handling entry.step.kind === "workflow" so workpool.enqueueMutation(ctx, api.workflow.cleanup, {...}) uses the pre-fetched workpool; ensure you still only call getWorkpool when at least one workflow step may be present (lazy-init or null-check) and keep deletion of entry._id and event handling unchanged.src/client/index.ts (1)
230-240: Consider validating thatsafeFunctionNamereturns a non-empty string.If
safeFunctionName(options.from)returns an empty string (possible edge case with malformed function references), the retry mutation would receivefrom: "", which passes validation but would fail with a confusing error message likeStep "" not found.💡 Optional defensive check
} else { - from = safeFunctionName(options.from); + from = safeFunctionName(options.from); + if (!from) { + throw new Error("Could not extract function name from reference"); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/index.ts` around lines 230 - 240, The code sets from = safeFunctionName(options.from) without ensuring the returned string is non-empty, which can pass validation but cause confusing runtime errors (e.g., Step "" not found); update the logic around safeFunctionName (used where options.from is handled and later passed into the retry mutation) to validate that the result is a non-empty string and handle the empty case (e.g., throw a clear error or treat as undefined) so the retry mutation never receives an empty "" value for from. Ensure references: options.from, safeFunctionName(...), the local variable from, and the retry mutation input are covered by this check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/client/index.ts`:
- Around line 230-240: The code sets from = safeFunctionName(options.from)
without ensuring the returned string is non-empty, which can pass validation but
cause confusing runtime errors (e.g., Step "" not found); update the logic
around safeFunctionName (used where options.from is handled and later passed
into the retry mutation) to validate that the result is a non-empty string and
handle the empty case (e.g., throw a clear error or treat as undefined) so the
retry mutation never receives an empty "" value for from. Ensure references:
options.from, safeFunctionName(...), the local variable from, and the retry
mutation input are covered by this check.
In `@src/component/workflow.ts`:
- Around line 479-492: deleteSteps currently calls getWorkpool inside the
for-loop causing repeated instantiation when multiple steps are workflow kind;
refactor by calling getWorkpool once before the loop (e.g., const workpool =
await getWorkpool(ctx, {})) and reuse that instance when handling
entry.step.kind === "workflow" so workpool.enqueueMutation(ctx,
api.workflow.cleanup, {...}) uses the pre-fetched workpool; ensure you still
only call getWorkpool when at least one workflow step may be present (lazy-init
or null-check) and keep deletion of entry._id and event handling unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0b6bcfa4-98ca-4358-a3f6-f2a65ac494e0
⛔ Files ignored due to path filters (1)
src/component/_generated/component.tsis excluded by!**/_generated/**
📒 Files selected for processing (7)
README.mdsrc/client/index.tssrc/component/journal.tssrc/component/model.tssrc/component/pool.tssrc/component/workflow.test.tssrc/component/workflow.ts

Add workflow retry functionality
Fixes #38
Fixes #191
This change introduces the ability to retry previously-failed workflows from a specific step or from the beginning.
Key Features
startAsyncoptionAPI Changes
workflow.retry()method with options for specifying retry point and execution modeforceparameterValidation
Summary by CodeRabbit
New Features
Documentation
Tests