Allow calling migration mutations directly#47
Merged
Conversation
📝 WalkthroughWalkthroughA new convex routing skill is introduced with documentation and lock entry; migration CLI invocation is simplified with direct command examples replacing function-based runners; example code simplified by removing prefix variants; client-side logic enhanced to auto-detect CLI/dashboard invocation and fetch function metadata; development dependency version updated. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Member
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
commit: |
ianmacartney
force-pushed
the
ian/call-directly
branch
from
a9a7273 to
490bd3c
Compare
This was referenced Apr 16, 2026
ianmacartney
changed the title
ianmacartney
changed the title
reeceyang
approved these changes
Apr 21, 2026
| const metadata = await getFunctionMetadata(); | ||
| if (args.fn && this.prefixedName(args.fn) !== metadata.name) { | ||
| throw new Error( | ||
| `Function name mismatch: expected ${metadata.name}, got ${args.fn}. Hint: you can `, |
There was a problem hiding this comment.
looks like this hint might be cut off?
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.claude/skills/convex (1)
1-1: Consider adding a trailing newline.POSIX convention recommends that text files end with a newline character. While not critical, adding one improves compatibility with certain tools and editors.
📝 Proposed fix
-../../.agents/skills/convex \ No newline at end of file +../../.agents/skills/convex🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/convex at line 1, The file named "convex" in the skills set is missing a trailing newline; open that file and append a single POSIX newline character (LF) at the end so the file ends with '\n' (ensure your editor uses Unix line endings), save and commit the change.src/client/index.ts (1)
753-754: Fix the copied TODO target.This helper is for transaction metrics, but the TODO says to replace it with
ctx.meta.getFunctionMetadata(). That will send maintainers to the wrong Convex API when the syscall shim is removed.Proposed wording fix
-// TODO: replace with ctx.meta.getFunctionMetadata() in 1.36+ +// TODO: replace with ctx.meta.getTransactionMetrics() in 1.36+ export async function getTransactionMetrics(): Promise<TransactionMetrics> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/index.ts` around lines 753 - 754, The TODO above the getTransactionMetrics function references the wrong Convex API (ctx.meta.getFunctionMetadata()); update the comment to point to the correct API for transaction metrics (e.g., ctx.meta.getTransactionMetrics() or the accurate future syscall name) so maintainers are directed to the proper replacement when the syscall shim is removed; change the TODO text near the getTransactionMetrics declaration to mention the correct ctx.meta.* method for transaction metrics.
🤖 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 294-330: Harden the direct-invocation detection in the block using
args (check for args.fn, args.next, args.cursor, args.dryRun, args.batchSize) so
null is treated as an invalid/ambiguous sentinel: update the condition that
decides direct vs batch invocation to consider args.cursor === null (and/or
args.dryRun === null) as triggering the direct-invocation branch or immediately
throw a clear error instructing callers to use reset:true or proper dryRun
values; specifically modify the logic around the existing check and the
subsequent cursor validation before batch processing (the code that calls
getFunctionMetadata(), this.prefixedName(args.fn) check, and the call to
this._runInteractive with makeFunctionReference) to either accept null as
direct-invocation or to reject ambiguous combos like {cursor:null,dryRun:false}
with a user-facing Error guiding them to use reset:true.
---
Nitpick comments:
In @.claude/skills/convex:
- Line 1: The file named "convex" in the skills set is missing a trailing
newline; open that file and append a single POSIX newline character (LF) at the
end so the file ends with '\n' (ensure your editor uses Unix line endings), save
and commit the change.
In `@src/client/index.ts`:
- Around line 753-754: The TODO above the getTransactionMetrics function
references the wrong Convex API (ctx.meta.getFunctionMetadata()); update the
comment to point to the correct API for transaction metrics (e.g.,
ctx.meta.getTransactionMetrics() or the accurate future syscall name) so
maintainers are directed to the proper replacement when the syscall shim is
removed; change the TODO text near the getTransactionMetrics declaration to
mention the correct ctx.meta.* method for transaction metrics.
🪄 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: e57a2296-8b2f-4d1b-a843-d7f9ad296c63
⛔ Files ignored due to path filters (3)
example/convex/_generated/ai/ai-files.state.jsonis excluded by!**/_generated/**example/convex/_generated/ai/guidelines.mdis excluded by!**/_generated/**package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
.agents/skills/convex/SKILL.md.claude/skills/convexREADME.mdexample/convex/example.tspackage.jsonskills-lock.jsonsrc/client/index.ts
💤 Files with no reviewable changes (1)
- example/convex/example.ts
Comment on lines
+294
to
+330
| // Auto-detect function name for direct CLI/dashboard invocation. | ||
| // The component always provides cursor and dryRun when scheduling | ||
| // batches, so if either is missing this is a direct invocation. | ||
| if ( | ||
| args.fn || | ||
| args.next || | ||
| args.cursor === undefined || | ||
| args.cursor === "" || | ||
| args.dryRun === undefined || | ||
| args.batchSize === 0 | ||
| ) { | ||
| console.warn( | ||
| "Running this from the CLI or dashboard? Here's some args to use:", | ||
| ); | ||
| console.warn({ | ||
| "Dry run": '{ "dryRun": true, "reset": true }', | ||
| "For real": '{ "fn": "path/to/migrations:yourFnName" }', | ||
| }); | ||
| if (args.cursor === "" || args.batchSize === 0) { | ||
| console.warn( | ||
| "Running this from the CLI or dashboard? Here's some args to use:", | ||
| ); | ||
| console.warn({ | ||
| "Dry run": '{ "dryRun": true, "reset": true }', | ||
| "For real": "{}", | ||
| }); | ||
| return; | ||
| } | ||
| const metadata = await getFunctionMetadata(); | ||
| if (args.fn && this.prefixedName(args.fn) !== metadata.name) { | ||
| const componentFlag = metadata.componentPath | ||
| ? ` --component ${metadata.componentPath}` | ||
| : ""; | ||
| throw new Error( | ||
| `Function name mismatch: expected ${metadata.name}, got ${args.fn}. Hint: you can call it directly with \`npx convex run ${metadata.name}${componentFlag}\`.`, | ||
| ); | ||
| } | ||
| return (await this._runInteractive( | ||
| ctx, | ||
| args, | ||
| makeFunctionReference( | ||
| metadata.name, | ||
| ) as unknown as MigrationFunctionReference, | ||
| )) as any; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect migration argument definitions and component scheduling calls
# to confirm whether scheduled worker invocations can be distinguished from direct CLI calls.
set -euo pipefail
echo "migrationArgs definition:"
rg -n -C4 'migrationArgs\s*='
echo
echo "Calls that pass cursor/dryRun/batchSize into migration function handles:"
rg -n -C5 '\b(cursor|dryRun|batchSize|fnHandle|oneBatchOnly)\b' --glob '!README.md'Repository: get-convex/migrations
Length of output: 50379
🏁 Script executed:
#!/bin/bash
# Verify: can the internal mutation be called directly, and does component validation gate the risk?
echo "=== Component fnHandle validation (defensive guard) ==="
sed -n '52,58p' src/component/lib.ts
echo
echo "=== Check if oneBatchOnly is always present in scheduled calls ==="
rg -n 'ctx.scheduler.runAfter.*api.lib.migrate' -A5 src/component/lib.ts
echo
echo "=== Verify _runInteractive always provides fnHandle for component call ==="
sed -n '193,201p' src/client/index.ts
echo
echo "=== Check if cursor and oneBatchOnly together discriminate scheduled from CLI ==="
rg -n 'oneBatchOnly' src/component/lib.ts | head -20Repository: get-convex/migrations
Length of output: 1448
🏁 Script executed:
#!/bin/bash
# Verify error handling for the specific scenario: {cursor: null, dryRun: false}
echo "=== Lines 333-350: Batch processing path validation ==="
sed -n '333,350p' src/client/index.tsRepository: get-convex/migrations
Length of output: 874
🏁 Script executed:
#!/bin/bash
# Final verification: are internal mutations called only via runner/runOne, or can they be called directly?
echo "=== How is the internal mutation typically invoked by users? ==="
rg -n 'migrations.define|migrations.runner|migrations.runOne|migrations.runSerially' -A2 example/ | head -30
echo
echo "=== Confirm that internal mutations are NOT exported/exposed to users directly ==="
rg -n 'export.*internal.*Migration' src/client/index.ts
echo
echo "=== Check if runner() wraps the internal mutation ==="
sed -n '73,125p' src/client/index.tsRepository: get-convex/migrations
Length of output: 3875
The direct-invocation detection logic is incomplete and should be hardened.
A call with {cursor: null, dryRun: false} does not trigger the direct-invocation branch (line 297) because cursor is neither undefined nor "", and dryRun is defined. This falls through to batch-processing (line 333), where cursor validation at line 334 also skips—because null is neither undefined nor "". The code then processes locally without component migration tracking.
While the component's fnHandle validation (lib.ts line 52) provides a second-line defense against direct calls, the incomplete detection logic is fragile. Either:
- Add an explicit scheduler-only sentinel argument (beyond the optional
oneBatchOnly), or - Harden the cursor/dryRun validation at lines 333–350 to explicitly error on ambiguous combinations like
{cursor: null, dryRun: false}with a message directing users to usereset: trueinstead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/client/index.ts` around lines 294 - 330, Harden the direct-invocation
detection in the block using args (check for args.fn, args.next, args.cursor,
args.dryRun, args.batchSize) so null is treated as an invalid/ambiguous
sentinel: update the condition that decides direct vs batch invocation to
consider args.cursor === null (and/or args.dryRun === null) as triggering the
direct-invocation branch or immediately throw a clear error instructing callers
to use reset:true or proper dryRun values; specifically modify the logic around
the existing check and the subsequent cursor validation before batch processing
(the code that calls getFunctionMetadata(), this.prefixedName(args.fn) check,
and the call to this._runInteractive with makeFunctionReference) to either
accept null as direct-invocation or to reject ambiguous combos like
{cursor:null,dryRun:false} with a user-facing Error guiding them to use
reset:true.
There was a problem hiding this comment.
@ianmacartney, understood — thanks for the follow-up pointer! I'll track the hardening of the direct-invocation detection logic (handling {cursor: null, dryRun: false} ambiguity) in #49.
Member
Author
Merge activity
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allow calling a migration directly
Enhanced migration system to support direct CLI invocation:
npx convex run migrations:myMigrationmigrations.runner()patternSummary by CodeRabbit
Release Notes
New Features
Documentation
Refactor
Chores