Support calling a workflow directly#244
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository: get-convex/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR significantly refactors the workflow startup API across the Convex workflow framework. It introduces a standalone Sequence DiagramsequenceDiagram
participant Client as Client Code
participant StartHelper as start() Helper
participant Mutation as Workflow Mutation<br/>(workflowMutation)
participant Metadata as Server Metadata
participant WorkflowExec as Workflow Execution
Client->>StartHelper: start(ctx, workflow, args, {onComplete?, context?, startAsync?})
StartHelper->>Mutation: ctx.runMutation({args, onComplete?, context?, startAsync?})
Mutation->>Metadata: getFunctionMetadata()
Metadata-->>Mutation: {name, componentPath}
alt startAsync = true
Mutation-->>StartHelper: return WorkflowId (immediately)
StartHelper-->>Client: WorkflowId
else startAsync = false/undefined
Mutation->>WorkflowExec: ctx.runMutation(workflow, {args})
WorkflowExec-->>Mutation: result
Mutation->>Mutation: if onComplete, schedule callback with result & context
Mutation-->>StartHelper: return WorkflowId
StartHelper-->>Client: WorkflowId
end
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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. Review rate limit: 0/1 reviews remaining, refill in 13 minutes and 17 seconds.Comment |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/types.ts (1)
110-125:⚠️ Potential issue | 🟠 Major
contextoptionality is inconsistent betweenOnCompleteandOnCompleteArgstypes, conflicting with schema validation.At line 112,
OnComplete.contextis optional (context?: Context), but at line 124,OnCompleteArgs.contextis required (context: Context). Additionally, schema validation insrc/component/schema.tsline 9 definescontext: v.optional(v.any()), which treats context as optional. This mismatch means the TypeScript types don't align with either each other or the runtime validation, creating type safety gaps where handlers may receiveundefinedcontext despite the required type annotation.Make
OnCompleteArgs.contextoptional to match bothOnCompleteand the schema validation:Proposed fix
export type OnCompleteArgs<Context = unknown> = { - context: Context; + context?: Context; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types.ts` around lines 110 - 125, The OnComplete types are inconsistent: OnComplete.context is optional but OnCompleteArgs.context is required, conflicting with the runtime schema (v.optional). Update the OnCompleteArgs type by changing the context property to optional (make context?: Context) in the OnCompleteArgs declaration so it matches OnComplete and the schema; then run TypeScript checks and adjust any callers of OnCompleteArgs or handlers that assume a non-null context to handle undefined.README.md (1)
228-255:⚠️ Potential issue | 🟡 MinorThe
onCompleteexample reads a field that the validator never defines.
contextis validated as{ intent, for }, but Line 255 usesargs.context.name, so copied code will either fail type-checking or logundefined.Proposed doc fix
- const name = args.context.name; + const name = args.context.for;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 228 - 255, The onComplete example is reading args.context.name in handleOnComplete but the context validator in start only defines { intent, for }, causing type errors or undefined; update either the start call's context validator (v.object({ intent: v.string(), for: v.string(), name: v.string() })) or change handleOnComplete to read an existing field (e.g., args.context.for or args.context.intent) so the validated shape and the usage in handleOnComplete (function name: handleOnComplete, symbol: args.context.name) match.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/convex-performance-audit/references/hot-path-rules.md:
- Around line 276-306: Update the table of contents entry at the top of
.agents/skills/convex-performance-audit/references/hot-path-rules.md so the
label for this section matches the renamed header "Isolate Frequently-Updated
Fields" (replace the stale "Skip No-Op Writes" entry); search for the
Contents/listing that references that old title and change it to "Isolate
Frequently-Updated Fields" so navigation and the section header align.
In `@src/client/index.ts`:
- Around line 150-188: Make StartOptions generic and propagate the Context type
through both entrypoints: change the StartOptions alias to accept a generic
parameter (e.g., Context = unknown) and extend CallbackOptions<Context> instead
of CallbackOptions; then add a generic parameter Context = unknown to the
standalone start function signature (export async function start< Context =
unknown, F extends FunctionReference<"mutation","internal"> >(...)) and use
options?: StartOptions<Context>; do the same for the WorkflowManager.start
method signature so its options parameter is StartOptions<Context> and the
Context generic flows to CallbackOptions used by onComplete handling.
In `@src/client/workflowMutation.ts`:
- Around line 29-49: WorkflowArgs currently allows onComplete and context
independently which can pass undefined to a handler expecting a required
Context; update the types so they are paired: change WorkflowArgs to a
discriminated union such that either { onComplete?: undefined; context?: Context
} or { onComplete: FunctionHandle<"mutation", OnCompleteArgs<Context>>; context:
Context } (i.e., require context when onComplete is present), and ensure
references to OnCompleteArgs and the usage at workflow.onComplete.context align
with this new union; alternatively, if you prefer the handler accept missing
context, make OnCompleteArgs<Context> accept context?: Context instead—pick one
of these two fixes and apply consistently to WorkflowArgs, OnCompleteArgs, and
the code path that reads workflow.onComplete.context.
---
Outside diff comments:
In `@README.md`:
- Around line 228-255: The onComplete example is reading args.context.name in
handleOnComplete but the context validator in start only defines { intent, for
}, causing type errors or undefined; update either the start call's context
validator (v.object({ intent: v.string(), for: v.string(), name: v.string() }))
or change handleOnComplete to read an existing field (e.g., args.context.for or
args.context.intent) so the validated shape and the usage in handleOnComplete
(function name: handleOnComplete, symbol: args.context.name) match.
In `@src/types.ts`:
- Around line 110-125: The OnComplete types are inconsistent: OnComplete.context
is optional but OnCompleteArgs.context is required, conflicting with the runtime
schema (v.optional). Update the OnCompleteArgs type by changing the context
property to optional (make context?: Context) in the OnCompleteArgs declaration
so it matches OnComplete and the schema; then run TypeScript checks and adjust
any callers of OnCompleteArgs or handlers that assume a non-null context to
handle undefined.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7d8b3863-74fe-4639-b9fb-50c9bfd952a3
⛔ Files ignored due to path filters (3)
example/convex/_generated/ai/ai-files.state.jsonis excluded by!**/_generated/**package-lock.jsonis excluded by!**/package-lock.jsonsrc/component/_generated/component.tsis excluded by!**/_generated/**
📒 Files selected for processing (15)
.agents/skills/convex-performance-audit/SKILL.md.agents/skills/convex-performance-audit/references/hot-path-rules.md.agents/skills/convex-performance-audit/references/occ-conflicts.mdREADME.mdexample/convex/example.tsexample/convex/test/start.test.tsexample/convex/transcription.tsexample/convex/userConfirmation.tspackage.jsonskills-lock.jsonsrc/client/index.tssrc/client/workflowMutation.tssrc/component/schema.tssrc/component/workflow.tssrc/types.ts
ianmacartney
force-pushed
the
ian/direct-workflow-call
branch
from
ca2085e to
5d9dd18
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.agents/skills/convex-performance-audit/references/hot-path-rules.md (1)
14-14:⚠️ Potential issue | 🟡 MinorUpdate the Contents entry to match the renamed section.
Line 14 still lists
4. Skip No-Op Writes, but the actual section at Line 276 is now4. Isolate Frequently-Updated Fields. Please sync these labels so in-doc navigation is accurate.Also applies to: 276-276
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/convex-performance-audit/references/hot-path-rules.md at line 14, Update the Contents entry that currently reads "4. Skip No-Op Writes" so it matches the renamed section title "4. Isolate Frequently-Updated Fields" (the section header text "Isolate Frequently-Updated Fields"); change the table-of-contents label so in-doc navigation points to the correct section.
🧹 Nitpick comments (2)
src/client/workflowMutation.ts (1)
81-89: Nit: the trailingassert(...)aftervalidate(..., { throw: true })is dead code.When
validate(vWorkflowArgs, args, { throw: true })fails it throws, so theassertwrapper is never meaningful — and when it succeedsvalidatealready returnstrue, making the assert a no-op. Consider simplifying.Proposed simplification
if (!validate(vWorkflowArgs, args)) { if (!("workflowId" in args) && !("args" in args)) { const console = createLogger(workpoolOptions?.logLevel); console.error( `Invalid arguments for workflow: When calling it directly, use '{ args: { ...your workflow args } }'`, ); } - assert(validate(vWorkflowArgs, args, { throw: true })); + validate(vWorkflowArgs, args, { throw: true }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/workflowMutation.ts` around lines 81 - 89, The assert after calling validate(vWorkflowArgs, args, { throw: true }) is redundant because validate will either throw on failure or return true on success; remove the trailing assert(...) and simply call validate(vWorkflowArgs, args, { throw: true }) (keeping the earlier conditional console error branch intact), ensuring no duplicate validation/assertion remains in the function containing this logic.example/convex/userConfirmation.ts (1)
15-26: Nit: JSDoc*prefix inconsistent inside the code fence.Lines 18-19 drop the leading
*that every other line in the block uses. It renders OK in most tools, but some JSDoc renderers (and editor folding) treat lines without the prefix as outside the comment, which can garble the output. If the goal is clean copy-paste of the shell command, consider keeping the prefix and relying on the fenced block (the*is conventionally stripped when rendering code inside JSDoc code fences).Proposed tweak
/** * Test this from the CLI: * ```sh - npx convex run userConfirmation:confirmationWorkflow \ - '{ "args": { "prompt": "Generate a recipe for me" } }' + * npx convex run userConfirmation:confirmationWorkflow \ + * '{ "args": { "prompt": "Generate a recipe for me" } }' * ``` * Copy the ID it returns, then run:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@example/convex/userConfirmation.ts` around lines 15 - 26, The JSDoc code-fence in the top-of-file comment in userConfirmation.ts has two lines (the CLI commands for running confirmationWorkflow and chooseProposal) that are missing the leading " * " prefix; update those lines to start with " * " so every line in the JSDoc block uses the same prefix (e.g., change the two lines showing "npx convex run userConfirmation:confirmationWorkflow ..." and the following indented JSON line to begin with " * "), preserving the fenced triple backticks and spacing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/client/workflowMutation.ts`:
- Around line 281-290: Update the incorrect TODO in the getTransactionMetrics
function: change the reference from ctx.meta.getFunctionMetadata() to the
correct future API (e.g., ctx.meta.getTransactionMetrics()) so the comment
accurately points to the replacement API for getTransactionMetrics; locate the
TODO inside the getTransactionMetrics function and update the comment text
accordingly.
- Around line 111-114: The code currently drops args.context when
args.onComplete is not a string; add a validation before computing onComplete to
detect when args.context is provided but args.onComplete is missing or not a
string and fail fast (throw a clear Error referencing OnCompleteArgs<Context>)
or at minimum log a warning. Update the logic around onComplete /
args.onComplete / args.context in the workflowMutation code so it either throws
a descriptive error (e.g., "context provided without string onComplete") or
emits a warning, ensuring callers cannot silently lose the context.
---
Duplicate comments:
In @.agents/skills/convex-performance-audit/references/hot-path-rules.md:
- Line 14: Update the Contents entry that currently reads "4. Skip No-Op Writes"
so it matches the renamed section title "4. Isolate Frequently-Updated Fields"
(the section header text "Isolate Frequently-Updated Fields"); change the
table-of-contents label so in-doc navigation points to the correct section.
---
Nitpick comments:
In `@example/convex/userConfirmation.ts`:
- Around line 15-26: The JSDoc code-fence in the top-of-file comment in
userConfirmation.ts has two lines (the CLI commands for running
confirmationWorkflow and chooseProposal) that are missing the leading " * "
prefix; update those lines to start with " * " so every line in the JSDoc block
uses the same prefix (e.g., change the two lines showing "npx convex run
userConfirmation:confirmationWorkflow ..." and the following indented JSON line
to begin with " * "), preserving the fenced triple backticks and spacing.
In `@src/client/workflowMutation.ts`:
- Around line 81-89: The assert after calling validate(vWorkflowArgs, args, {
throw: true }) is redundant because validate will either throw on failure or
return true on success; remove the trailing assert(...) and simply call
validate(vWorkflowArgs, args, { throw: true }) (keeping the earlier conditional
console error branch intact), ensuring no duplicate validation/assertion remains
in the function containing this logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 36d56409-69cb-402c-aae8-682a4ce98275
⛔ Files ignored due to path filters (3)
example/convex/_generated/ai/ai-files.state.jsonis excluded by!**/_generated/**package-lock.jsonis excluded by!**/package-lock.jsonsrc/component/_generated/component.tsis excluded by!**/_generated/**
📒 Files selected for processing (15)
.agents/skills/convex-performance-audit/SKILL.md.agents/skills/convex-performance-audit/references/hot-path-rules.md.agents/skills/convex-performance-audit/references/occ-conflicts.mdREADME.mdexample/convex/example.tsexample/convex/test/start.test.tsexample/convex/transcription.tsexample/convex/userConfirmation.tspackage.jsonskills-lock.jsonsrc/client/index.tssrc/client/workflowMutation.tssrc/component/schema.tssrc/component/workflow.tssrc/types.ts
✅ Files skipped from review due to trivial changes (3)
- package.json
- skills-lock.json
- .agents/skills/convex-performance-audit/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (6)
- example/convex/transcription.ts
- src/component/workflow.ts
- src/types.ts
- .agents/skills/convex-performance-audit/references/occ-conflicts.md
- README.md
- src/client/index.ts
| const onComplete = | ||
| typeof args.onComplete === "string" | ||
| ? { fnHandle: args.onComplete, context: args.context } | ||
| : undefined; |
There was a problem hiding this comment.
context is silently dropped when onComplete is not a string.
If a caller accidentally passes context without onComplete (e.g., from a malformed direct-call payload), the context is silently discarded instead of either being surfaced or rejected. Since OnCompleteArgs<Context> requires context and the past review already flagged the pairing, consider logging a warning or failing fast so misuse is observable rather than silent.
Suggested check
const onComplete =
typeof args.onComplete === "string"
? { fnHandle: args.onComplete, context: args.context }
: undefined;
+ if (!onComplete && args.context !== undefined) {
+ const console = createLogger(workpoolOptions?.logLevel);
+ console.warn(
+ `[workflow] "context" was provided without "onComplete"; it will be ignored.`,
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const onComplete = | |
| typeof args.onComplete === "string" | |
| ? { fnHandle: args.onComplete, context: args.context } | |
| : undefined; | |
| const onComplete = | |
| typeof args.onComplete === "string" | |
| ? { fnHandle: args.onComplete, context: args.context } | |
| : undefined; | |
| if (!onComplete && args.context !== undefined) { | |
| const console = createLogger(workpoolOptions?.logLevel); | |
| console.warn( | |
| `[workflow] "context" was provided without "onComplete"; it will be ignored.`, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/client/workflowMutation.ts` around lines 111 - 114, The code currently
drops args.context when args.onComplete is not a string; add a validation before
computing onComplete to detect when args.context is provided but args.onComplete
is missing or not a string and fail fast (throw a clear Error referencing
OnCompleteArgs<Context>) or at minimum log a warning. Update the logic around
onComplete / args.onComplete / args.context in the workflowMutation code so it
either throws a descriptive error (e.g., "context provided without string
onComplete") or emits a warning, ensuring callers cannot silently lose the
context.
5daa761 to
53ef3fb
Compare
ianmacartney
force-pushed
the
ian/direct-workflow-call
branch
from
53ef3fb to
76117fd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/client/workflowMutation.ts (1)
285-286:⚠️ Potential issue | 🟡 MinorCorrect the TODO API reference in
getTransactionMetrics.Line 285 points to
ctx.meta.getFunctionMetadata()but this helper is for transaction metrics. The TODO should referencectx.meta.getTransactionMetrics().#!/bin/bash # Verify mismatched TODO references in this file. rg -n -C1 'TODO: replace with ctx\.meta\.getFunctionMetadata\(\) in 1\.36\+' src/client/workflowMutation.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/workflowMutation.ts` around lines 285 - 286, Update the TODO in getTransactionMetrics to reference the correct helper: replace the incorrect mention of ctx.meta.getFunctionMetadata() with ctx.meta.getTransactionMetrics(); specifically change the comment above the exported async function getTransactionMetrics() so it advises using ctx.meta.getTransactionMetrics() in 1.36+ to accurately reflect the transaction metrics API.src/types.ts (1)
110-125:⚠️ Potential issue | 🟠 MajorAlign
contextoptionality betweenOnCompleteandOnCompleteArgs.Line 112 allows omitted
context, but Line 124 requires it. This weakens type safety and can passundefinedinto completion handlers expecting required context.Suggested minimal fix
export type OnCompleteArgs<Context = unknown> = { @@ - context: Context; + context?: Context; @@ };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types.ts` around lines 110 - 125, The optionality of the context field is inconsistent: OnComplete type allows context to be omitted (context?: Context) but OnCompleteArgs requires context, risking undefined passed into handlers; update the OnCompleteArgs type definition so its context property is also optional (change context: Context to context?: Context) in src/types.ts to match OnComplete, keeping the JSDoc/comments intact and ensuring types OnComplete and OnCompleteArgs align.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/client/index.ts`:
- Around line 55-57: The documentation example in src/client/index.ts uses a
stale argument name "workId"; update the example to use "workflowId" so it
matches the OnCompleteArgs type and other code paths—locate the example block
showing workId, result, context and replace workId with workflowId in the
callback example and any related JSDoc/inline docs for the
OnCompleteArgs/OnComplete callback to keep names consistent.
---
Duplicate comments:
In `@src/client/workflowMutation.ts`:
- Around line 285-286: Update the TODO in getTransactionMetrics to reference the
correct helper: replace the incorrect mention of ctx.meta.getFunctionMetadata()
with ctx.meta.getTransactionMetrics(); specifically change the comment above the
exported async function getTransactionMetrics() so it advises using
ctx.meta.getTransactionMetrics() in 1.36+ to accurately reflect the transaction
metrics API.
In `@src/types.ts`:
- Around line 110-125: The optionality of the context field is inconsistent:
OnComplete type allows context to be omitted (context?: Context) but
OnCompleteArgs requires context, risking undefined passed into handlers; update
the OnCompleteArgs type definition so its context property is also optional
(change context: Context to context?: Context) in src/types.ts to match
OnComplete, keeping the JSDoc/comments intact and ensuring types OnComplete and
OnCompleteArgs align.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: get-convex/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d322019d-d746-4f30-ac6c-3190e620f8af
⛔ Files ignored due to path filters (1)
src/component/_generated/component.tsis excluded by!**/_generated/**
📒 Files selected for processing (10)
README.mdexample/convex/example.tsexample/convex/test/start.test.tsexample/convex/transcription.tsexample/convex/userConfirmation.tssrc/client/index.tssrc/client/workflowMutation.tssrc/component/schema.tssrc/component/workflow.tssrc/types.ts
✅ Files skipped from review due to trivial changes (1)
- example/convex/test/start.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/component/schema.ts
- src/component/workflow.ts
- README.md
ianmacartney
force-pushed
the
ian/direct-workflow-call
branch
from
d82b6bb to
c912222
Compare
2 participants
Allow calling workflows directly
Workflows can now be invoked directly using
ctx.runMutation(internal.myWorkflow, { args: {...} })or from CLI/dashboard without requiring the workflow manager'sstart()method. Added new standalonestart()helper function for type safety & convenience in handling onComplete function handle creation.This is done by using ctx.meta.getFunctionMetadata() (Convex 1.36+)
Also:
getFunctionMetadata()andgetTransactionMetrics()syscalls to support workflow introspection and monitoring.