clean up nested workflows and events by ianmacartney · Pull Request #204 · get-convex/workflow

ianmacartney · 2026-02-19T03:07:35Z

TL;DR

Enhanced workflow cleanup to properly delete associated events and nested workflows.

Fixes #186

What changed?

Added logic to the cleanup mutation to handle additional resource deletion:

When a journal entry's step is of kind "event", the associated event is now deleted
When a journal entry's step is of kind "workflow" with a workflowId, a cleanup mutation is enqueued to recursively clean up the nested workflow

How to test?

Create a workflow that contains event steps and nested workflow steps
Execute the workflow
Trigger the cleanup mutation for the parent workflow
Verify that all associated events are deleted from the database
Verify that cleanup is properly propagated to nested workflows

Why make this change?

This change prevents resource leaks by ensuring that when a workflow is cleaned up, all of its associated resources (events and nested workflows) are properly deleted as well. Without this change, orphaned events and nested workflows would remain in the database after the parent workflow is cleaned up.

Summary by CodeRabbit

Bug Fixes
- Improved workflow cleanup to properly handle cascading deletions of associated events and nested workflows.
Tests
- Added comprehensive test coverage for workflow cleanup scenarios, including nested workflows, event handling, and cascade deletion behavior.

coderabbitai · 2026-02-19T03:07:48Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

✅ Review completed - (🔄 Check again to review again)

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch ian/cleanup-nested-workflow-and-events

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

ianmacartney · 2026-02-19T03:07:49Z

allow resuming #205 : 2 dependent PRs (#206 , #215 )
clean up nested workflows and events #204 👈 (View in Graphite)
use getConvexSize and improve journal mismatch logging #203
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

ianmacartney · 2026-02-19T03:08:36Z

@cursor can you write tests for this?

pkg-pr-new · 2026-02-19T03:08:37Z

Open in StackBlitz

npm i https://pkg.pr.new/get-convex/workflow/@convex-dev/workflow@204

commit: 32f4d45

cursor · 2026-02-19T03:08:42Z

I've written tests for the cleanup functionality and also fixed a bug I discovered. Here's what was done:

Tests Added (5 new tests in src/component/workflow.test.ts):

cleanup deletes associated events - Verifies that when a workflow with event steps is cleaned up, the associated events are also deleted
cleanup enqueues cleanup for nested workflows - Verifies that when a parent workflow is cleaned up, cleanup is properly enqueued for nested workflows via workpool
cleanup handles workflow step without workflowId - Verifies cleanup works for workflow steps that haven't started their nested workflow yet
cleanup handles event step without eventId - Verifies cleanup works for event steps that don't have an eventId set
cleanup with mixed event and workflow steps - Integration test verifying both event and workflow step cleanup works together

Bug Fix in src/component/workflow.ts:
Changed the event deletion condition from:

if (journalEntry.step.kind === "event") {
  await ctx.db.delete(journalEntry.step.eventId!);

to:

if (journalEntry.step.kind === "event" && journalEntry.step.eventId) {
  await ctx.db.delete(journalEntry.step.eventId);

This prevents a crash when eventId is undefined on event steps.

- Test cleanup deletes associated events when cleaning up workflow with event steps - Test cleanup enqueues cleanup for nested workflows - Test cleanup handles workflow step without workflowId - Test cleanup handles event step without eventId - Test cleanup with mixed event and workflow steps Also fix bug where cleanup would crash when eventId is undefined on event steps. Co-authored-by: Ian Macartney <ianmacartney@users.noreply.github.com>

…journal.startSteps startSteps is a regular mutation, not an internal function. Co-authored-by: Ian Macartney <ianmacartney@users.noreply.github.com>

ianmacartney · 2026-02-19T03:40:04Z

+    if (!nestedWorkflowCompleted) {
+      await t.mutation(api.workflow.cancel, { workflowId: nestedWorkflowId! });
+    }


@cursor this should happen automatically as part of canceling the parent

Updated the tests to rely on the parent workflow's cancel automatically canceling nested workflows. The tests now:

Cancel only the parent workflow

Verify that the nested workflow was automatically canceled (by checking runResult is defined)

No longer manually cancel nested workflows separately

This better reflects the expected behavior where canceling a parent propagates cancellation to in-progress nested workflows.

…workflows Canceling the parent workflow automatically cancels in-progress nested workflows, so there's no need to manually cancel them separately. Co-authored-by: Ian Macartney <ianmacartney@users.noreply.github.com>

coderabbitai

🧹 Nitpick comments (1)

src/component/workflow.ts (1)

362-364: Reuse a single workpool instance during cleanup.

At Line 362, getWorkpool(ctx, {}) runs inside the per-entry loop, which repeats config reads and client construction. Collect nested workflow IDs first, then enqueue with one workpool instance (and dedupe IDs).

♻️ Suggested refactor

-    for (const journalEntry of journalEntries) {
+    const nestedWorkflowIds = new Set<Id<"workflows">>();
+    for (const journalEntry of journalEntries) {
       logger.debug("Deleting journal entry", journalEntry);
       await ctx.db.delete(journalEntry._id);
       if (journalEntry.step.kind === "event" && journalEntry.step.eventId) {
         await ctx.db.delete(journalEntry.step.eventId);
       } else if (
         journalEntry.step.kind === "workflow" &&
         journalEntry.step.workflowId
       ) {
-        const workpool = await getWorkpool(ctx, {});
-        await workpool.enqueueMutation(ctx, api.workflow.cleanup, {
-          workflowId: journalEntry.step.workflowId,
-        });
+        nestedWorkflowIds.add(journalEntry.step.workflowId);
       }
     }
+    if (nestedWorkflowIds.size > 0) {
+      const workpool = await getWorkpool(ctx, {});
+      for (const nestedWorkflowId of nestedWorkflowIds) {
+        await workpool.enqueueMutation(ctx, api.workflow.cleanup, {
+          workflowId: nestedWorkflowId,
+        });
+      }
+    }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/component/workflow.ts` around lines 362 - 364, The current code calls
getWorkpool(ctx, {}) inside the per-entry loop which rebuilds clients/config
repeatedly; instead, first collect all nested workflow IDs from
journalEntry.step.workflowId across entries (dedupe them), then call
getWorkpool(ctx, {}) once to obtain a single workpool and call
workpool.enqueueMutation(ctx, api.workflow.cleanup, { workflowId: id }) for each
unique id; update the loop to remove repeated getWorkpool calls and use the
single workpool instance for all enqueueMutation calls.

🤖 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/component/workflow.ts`:
- Around line 362-364: The current code calls getWorkpool(ctx, {}) inside the
per-entry loop which rebuilds clients/config repeatedly; instead, first collect
all nested workflow IDs from journalEntry.step.workflowId across entries (dedupe
them), then call getWorkpool(ctx, {}) once to obtain a single workpool and call
workpool.enqueueMutation(ctx, api.workflow.cleanup, { workflowId: id }) for each
unique id; update the loop to remove repeated getWorkpool calls and use the
single workpool instance for all enqueueMutation calls.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2629473 and 32f4d45.

⛔ Files ignored due to path filters (1)

package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (2)

src/component/workflow.test.ts
src/component/workflow.ts

sethconvex

If you're not worried about deleting lots of potential steps in one mutation, LGTM.

sethconvex · 2026-03-03T06:17:35Z

      logger.debug("Deleting journal entry", journalEntry);
      await ctx.db.delete(journalEntry._id);
+      if (journalEntry.step.kind === "event" && journalEntry.step.eventId) {
+        await ctx.db.delete(journalEntry.step.eventId);


Is this going to be a problem when there are lots of steps with big args?

We currently limit the journal & size of all steps to be max 8MiB, including events I believe, so deletion should only consume <= 16MiB if we read everything and delete them individually. If they hit the limit on the nose we might be in trouble for the extra workflow metadata, but deleting events take less data b/c we only read as part of the delete, not fetched in addition.

ianmacartney · 2026-03-03T23:44:44Z

Merged into main directly

clean up nested workflows and events

57676e9

ianmacartney mentioned this pull request Feb 19, 2026

use getConvexSize and improve journal mismatch logging #203

Merged

cursoragent and others added 2 commits February 19, 2026 03:16

Fix TypeScript error: use api.journal.startSteps instead of internal.…

cfab568

…journal.startSteps startSteps is a regular mutation, not an internal function. Co-authored-by: Ian Macartney <ianmacartney@users.noreply.github.com>

ianmacartney commented Feb 19, 2026

View reviewed changes

ianmacartney mentioned this pull request Feb 19, 2026

allow resuming #205

Merged

ianmacartney marked this pull request as ready for review February 19, 2026 04:30

ianmacartney mentioned this pull request Feb 19, 2026

surface ctx.getHistory() -> { size, stepCount } #206

Open

ianmacartney requested a review from sethconvex February 25, 2026 23:25

ianmacartney mentioned this pull request Mar 2, 2026

support newest workpool #215

Closed

coderabbitai Bot reviewed Mar 3, 2026

View reviewed changes

sethconvex reviewed Mar 3, 2026

View reviewed changes

ianmacartney closed this Mar 3, 2026

ianmacartney reopened this Mar 3, 2026

ianmacartney merged commit 065ed19 into ian/use-convex-size Mar 3, 2026
5 of 6 checks passed

ianmacartney deleted the ian/cleanup-nested-workflow-and-events branch March 3, 2026 23:45

Conversation

ianmacartney commented Feb 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TL;DR

What changed?

How to test?

Why make this change?

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

ianmacartney commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ianmacartney commented Feb 19, 2026

Uh oh!

pkg-pr-new Bot commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cursor Bot commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ianmacartney Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

sethconvex left a comment

Choose a reason for hiding this comment

Uh oh!

sethconvex Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ianmacartney Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ianmacartney commented Mar 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ianmacartney commented Feb 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Feb 19, 2026 •

edited

Loading

ianmacartney commented Feb 19, 2026 •

edited

Loading

pkg-pr-new Bot commented Feb 19, 2026 •

edited

Loading

cursor Bot commented Feb 19, 2026 •

edited

Loading

cursor Bot Feb 19, 2026 •

edited

Loading