feat: add workos migrations command#149
Conversation
📝 WalkthroughWalkthroughThis PR adds a new top-level ChangesMigrations Command Feature
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Programmatically imports the Commander program from workos-migrations and delegates all arg parsing/execution to it. Auth is bridged automatically by injecting the resolved API key as WORKOS_SECRET_KEY. - Register `migrations` top-level command with strict(false) passthrough - Strip --api-key, --insecure-storage, --json from forwarded args - Add all 12 migration subcommands to help-json registry - Link workos-migrations as local dependency - Bump Node engine to >=22.11 (required by workos-migrations)
Disable yargs help/version interception for the migrations command so Commander's native help output is shown instead.
Without this, Commander's help displays `Usage: workos-migrate` which would mislead users into thinking that's the command.
Replace local link with published @workos/migrations@^2.0.0 package. Update all import paths from workos-migrations/ to @workos/migrations/. Remove pnpm-workspace.yaml that was only needed for the local link.
3c12454 to
bdbcd16
Compare
Greptile SummaryThis PR adds a
Confidence Score: 5/5Safe to merge; the passthrough arg-filtering logic is correct, tests cover all significant edge cases, and the auth bridge is implemented consistently with the rest of the CLI. The flag-stripping logic in getMigrationsPassthroughArgs correctly handles inline-value flags (--mode=ci), space-separated flags (--mode agent), and ambiguous tokens (migrations as a flag value vs. command). Tests enumerate all these cases. The auth injection pattern and the yargs passthrough setup are consistent with the stated intent. Previously noted concerns around the deep dist/ import and silent --json stripping are known design trade-offs already discussed. No files require special attention beyond the previously-discussed deep-import coupling in src/commands/migrations.ts. Important Files Changed
Reviews (3): Last reviewed commit: "fix: handle migrations flag value edge c..." | Re-trigger Greptile |
| export async function runMigrations(args: string[], apiKey: string): Promise<void> { | ||
| process.env.WORKOS_SECRET_KEY = apiKey; | ||
|
|
||
| const { program } = (await import('@workos/migrations/dist/cli/index.js')) as { |
There was a problem hiding this comment.
Deep import into
@workos/migrations internals
The import path @workos/migrations/dist/cli/index.js reaches into the package's compiled dist directory rather than using a declared subpath export. If the migrations package reorganizes its build output (e.g., renames the entry file or switches to an exports map that restricts deep imports), this will throw a module-not-found error at runtime. Consider requesting the package expose a named subpath export like @workos/migrations/cli to avoid coupling to the build artifact layout.
| const { runMigrations } = await import('./commands/migrations.js'); | ||
| const migrationsIdx = rawArgs.indexOf('migrations'); | ||
| const passthrough: string[] = []; | ||
| const skip = new Set(['--insecure-storage', '--json', '--api-key']); |
There was a problem hiding this comment.
🟡 Global --mode flag leaks into migrations passthrough args
The passthrough filter skip set at src/bin.ts:2414 only includes ['--insecure-storage', '--json', '--api-key'] but omits the global --mode option (defined at src/bin.ts:207-211 as a string type taking values human, agent, or ci). When a user places --mode after the migrations keyword (e.g. workos migrations import --csv users.csv --mode agent), both --mode and its value (e.g. agent) are passed through to the Commander-based @workos/migrations CLI, which doesn't recognize them and will error. The --api-key skip correctly handles its value argument, but --mode isn't handled at all.
| const skip = new Set(['--insecure-storage', '--json', '--api-key']); | |
| const skip = new Set(['--insecure-storage', '--json', '--api-key', '--mode']); |
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (skip.has(key)) { | ||
| if (key === '--api-key' && !arg.includes('=')) i++; | ||
| continue; |
There was a problem hiding this comment.
🟡 Passthrough filter only skips value for --api-key, but --mode also takes a value argument
Even after adding --mode to the skip set, the value-skipping logic at src/bin.ts:2420 only handles --api-key specifically: if (key === '--api-key' && !arg.includes('=')) i++. For --mode agent (without =), only --mode would be skipped but agent would still be pushed to passthrough, corrupting the arguments sent to the migrations CLI. Any string-typed global flag added to the skip set needs its value skipped when passed in --flag value form (not just --flag=value).
| if (skip.has(key)) { | |
| if (key === '--api-key' && !arg.includes('=')) i++; | |
| continue; | |
| if ((key === '--api-key' || key === '--mode') && !arg.includes('=')) i++; |
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
workos migrationstop-level command that wraps the standalone@workos/migrationsCLI via programmatic importWORKOS_SECRET_KEY--mode agent/--mode=ci,--api-key,--insecure-storage, and--jsondo not leak into Commander argsmigrationswithout treating that value as the command boundary>=22.11to match@workos/migrationsrequirementReview & Testing Checklist for Human
workos migrations --helpshows Commander help output for the wrapped migrations CLIworkos migrations import --csv <file> --mode agent --api-key <key>delegates without Commander rejecting--modeor the key valueworkos --help --jsonincludesmigrationsin the commands arrayNotes
Validated locally with:
pnpm install --frozen-lockfilepnpm test -- src/commands/migrations.spec.ts(Vitest ran full suite: 1795/1795 passing)pnpm lintpnpm format:checkpnpm typecheckpnpm buildLink to Devin session: https://app.devin.ai/sessions/f300e72a5caa41b49efb4d4311c0895a
Requested by: @nicknisi