fix(memory): add workflow state parity for SQL adapters#1082
Conversation
🦋 Changeset detectedLatest commit: 30d008a The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
This comment has been minimized.
This comment has been minimized.
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughThis PR adds persistent parity for workflow state across SQL adapters by adding Changes
Sequence Diagram(s)mermaid Note: colored rectangles not used because sequence flow is simple and component boxes suffice. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/postgres/src/memory-adapter.spec.ts (1)
844-943:⚠️ Potential issue | 🟡 Minor
getSuspendedWorkflowStatestest is missing assertions forinputand entirely skipscontext.
inputis present in the fixture objects (lines 851, 876) and mock DB rows (lines 906, 923) but is never asserted in the finalexpectblock (lines 940–943).contextis absent from the suspended-state fixtures and mock rows altogether, leaving thecontextround-trip completely untested for this retrieval path — unlikegetWorkflowState(lines 730/777) andqueryWorkflowRuns(lines 957/984) which cover it fully.🛠️ Suggested additions
expect(suspended[0]?.events).toEqual(states[0].events); expect(suspended[0]?.output).toEqual(states[0].output); expect(suspended[0]?.cancellation).toEqual(states[0].cancellation); expect(suspended[0]?.workflowState).toEqual(states[0].workflowState); + expect(suspended[0]?.input).toEqual(states[0].input);And add
contextto the first suspended-state fixture + mock row:{ id: "wf-susp-1", ... input: { task: 1 }, + context: [["tenantId", "acme"]] as Array<[string, unknown]>, workflowState: { phase: "s1" },{ id: "wf-susp-1", ... input: states[0].input, + context: states[0].context, workflow_state: states[0].workflowState,+ expect(suspended[0]?.context).toEqual(states[0].context);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/postgres/src/memory-adapter.spec.ts` around lines 844 - 943, The test for getSuspendedWorkflowStates is missing assertions for the state's input and omits testing context entirely; update the suspended-state fixtures (the entries in the states array used in this test) to include a context value for at least the first suspended fixture, add the same context to the corresponding mockPoolQueryResult row, then add assertions after calling adapter.getSuspendedWorkflowStates to expect suspended[0].input toEqual the fixture input and suspended[0].context toEqual the fixture context (and add similar checks for suspended[1] if you add context there) so the round-trip for input and context is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/postgres/src/memory-adapter.spec.ts`:
- Around line 844-943: The test for getSuspendedWorkflowStates is missing
assertions for the state's input and omits testing context entirely; update the
suspended-state fixtures (the entries in the states array used in this test) to
include a context value for at least the first suspended fixture, add the same
context to the corresponding mockPoolQueryResult row, then add assertions after
calling adapter.getSuspendedWorkflowStates to expect suspended[0].input toEqual
the fixture input and suspended[0].context toEqual the fixture context (and add
similar checks for suspended[1] if you add context there) so the round-trip for
input and context is covered.
Deploying voltagent with
|
| Latest commit: |
30d008a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3684eeec.voltagent.pages.dev |
| Branch Preview URL: | https://fix-workflow-state-parity-10.voltagent.pages.dev |
1 participant
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
Workflow state persistence is inconsistent across SQL adapters.
getWorkflowStateandqueryWorkflowRunscan return only partial execution state, andgetSuspendedWorkflowStatesmay omit fields likeevents,output, andcancellation. Top-levelinput,context, andworkflowStateare not fully persisted/read in these adapters.What is the new behavior?
This PR brings workflow state parity for SQL adapters:
input,context, and top-levelworkflowStategetSuspendedWorkflowStatesincludesevents,output, andcancellationfixes #1080
Notes for reviewers
Validated with targeted checks:
pnpm -C packages/postgres test -- src/memory-adapter.spec.tspnpm -C packages/supabase test -- src/memory-adapter.spec.tspnpm -C packages/libsql test -- src/memory-v2-adapter.spec.tspnpm -C packages/cloudflare-d1 buildSummary by cubic
Ensures full workflow state persistence and parity across SQL adapters (Postgres, Supabase, LibSQL, Cloudflare D1) so reads and queries return complete data. Fixes Linear #1080.
Bug Fixes
Migration
Written for commit 30d008a. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Chores