Skip to content

Add onCancel completion callbacks - #236

Closed
ianmacartney wants to merge 2 commits into
ian/on-failurefrom
ian/on-cancel
Closed

ianmacartney wants to merge 2 commits into
ian/on-failurefrom
ian/on-cancel

Conversation

@ianmacartney

@ianmacartney ianmacartney commented Sep 10, 2026 •

Copy link
Copy Markdown
Member

Add onCancel so callers can handle a canceled result without invoking their completion handler for successful or failed work. It accepts existing onComplete handlers and context, can be combined with onFailure, and cannot be combined with onComplete. The two outcome callbacks may use the same handler or different handlers.

Callbacks follow the final result: canceling pending work or preventing a retry invokes onCancel; canceling an in-progress attempt still allows that attempt to succeed or fail. Direct scheduler cancellation remains a failure. Cancellation callbacks retain the existing separate scheduling and transaction behavior.

Stacked on #231 (ian/on-failure). Extends the stored outcome map with an optional canceled handle, preserving the existing legacy and failure-only shapes. Enqueue mutations reject maps without a callback handle before writing any work, including batches. The client omits callback metadata when both handlers are disabled. Workers, retry logic, and cancellation processing are unchanged.

Validation: build, typecheck, lint, formatting, and all 199 tests pass. Added coverage includes pending cancellation, retry backoff, cancellation during a running attempt, cancelAll for batched actions/mutations/queries, large and omitted context, callback rollback, distinct failure/cancel handlers, component and client mutual exclusion, empty-map rejection without partial batch writes, disabled client callbacks, and scheduler recovery. Type checks remain in the regular index.test.ts.

Component API types were synchronized and checked against the actual function validators. CLI codegen is unavailable here because no deployment is configured.

@pkg-pr-new

pkg-pr-new Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/get-convex/workpool/@convex-dev/workpool@236

commit: ad31fa6

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 8 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/component/shared.ts">

<violation number="1" location="src/component/shared.ts:120">
P2: When a component caller supplies `onStatusHandle: {}`, this validator accepts it and stores a callback configuration with no handler, so terminal completion silently skips the callback. Require at least one of `failed` or `canceled` while still allowing failure-only, cancel-only, and combined maps.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread README.md Resolved
Comment thread src/component/shared.ts
Comment on lines +120 to +123
onStatusHandle: v.object({
failed: v.optional(v.string()), // mutation
canceled: v.optional(v.string()), // mutation
}),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When a component caller supplies onStatusHandle: {}, this validator accepts it and stores a callback configuration with no handler, so terminal completion silently skips the callback. Require at least one of failed or canceled while still allowing failure-only, cancel-only, and combined maps.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/component/shared.ts, line 120:

<comment>When a component caller supplies `onStatusHandle: {}`, this validator accepts it and stores a callback configuration with no handler, so terminal completion silently skips the callback. Require at least one of `failed` or `canceled` while still allowing failure-only, cancel-only, and combined maps.</comment>

<file context>
@@ -117,7 +117,10 @@ export const vOnCompleteFnContext = v.union(
   }),
   v.object({
-    onStatusHandle: v.object({ failed: v.string() }), // mutations
+    onStatusHandle: v.object({
+      failed: v.optional(v.string()), // mutation
+      canceled: v.optional(v.string()), // mutation
</file context>
Suggested change
onStatusHandle: v.object({
failed: v.optional(v.string()), // mutation
canceled: v.optional(v.string()), // mutation
}),
onStatusHandle: v.union(
v.object({
failed: v.string(), // mutation
canceled: v.optional(v.string()), // mutation
}),
v.object({
failed: v.optional(v.string()), // mutation
canceled: v.string(), // mutation
}),
),

Copy link
Copy Markdown
Member Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

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

Copy link
Copy Markdown
Member Author

superceded by #237

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant