fix(cli): don't crash fern upgrade when the source version is a range pin - #17340
fix(cli): don't crash fern upgrade when the source version is a range pin#17340devin-ai-integration[bot] wants to merge 1 commit into
fern upgrade when the source version is a range pin#17340Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
AI Review Summary
Sensible fix: resolveSourceVersion now validates its output and returns undefined to mean "skip migrations", with the migration block and rerun args/env guarded accordingly. Tests use the real isValidVersion, which is a good call. Two minor concerns: the env: {} fallback doesn't actually unset a poisoned FERN_PRE_UPGRADE_VERSION inherited from the parent process, and the "Skipping migrations" warning is emitted even on the rerun path where the child will still run migrations.
- 🟡 1 warning(s)
- 🔵 1 suggestion(s)
| env: { | ||
| [PREVIOUS_VERSION_ENV_VAR]: resolvedFromVersion | ||
| }, | ||
| env: resolvedFromVersion != null ? { [PREVIOUS_VERSION_ENV_VAR]: resolvedFromVersion } : {}, |
There was a problem hiding this comment.
🟡 warning
env: {} doesn't remove an inherited FERN_PRE_UPGRADE_VERSION — execa merges env with process.env by default, so if the parent itself was invoked with a poisoned value (e.g. FERN_PRE_UPGRADE_VERSION=*), the child still sees it and re-resolves the same unparseable version. It won't crash today (the child falls back to its own CLI version), but the stated intent "omitted from the rerun instead of forwarding a poisoned value" isn't met. Consider explicitly clearing it, e.g. { [PREVIOUS_VERSION_ENV_VAR]: resolvedFromVersion ?? undefined } (if rerunFernCliAtVersion's env type allows undefined), or pass an empty string that the child's validation already rejects.
| return cliVersion; | ||
| } | ||
|
|
||
| cliContext.logger.warn(`Skipping migrations because ${resolvedFromVersion} is not a version that can be migrated.`); |
There was a problem hiding this comment.
🔵 suggestion
This warning fires from both call sites, including the rerun path — but there the child CLI will re-resolve a source version and likely do run migrations, so "Skipping migrations" is misleading to the user. Consider wording it in terms of the resolution failure (e.g. "${resolvedFromVersion}" is not a version that can be migrated from; migrations will be skipped for this step.) or only warning when migrations are actually being skipped in-process.
Also: the CLI-version fallback above is logged at debug, which hides a decision that can silently change which migrations run. info would be more honest.
Description
fern upgradedies withFailed to parse version: */Failed to rerun CLI at version <target>when the resolved migration source version isn't somethingparseVersionunderstands.Repro: a
fern.config.jsonwhose committed version is"*"(alsolatest,1.x).resolveSourceVersionfalls back togit show HEAD:fern.config.jsonwhen neither--fromnorFERN_PRE_UPGRADE_VERSIONis set, gets*, passes--from *to the newly installed CLI, andrunMigrations→getMigrationsToRun→isVersionAhead→parseVersion("*")throws. The existingversion === "latest" || version === "*"guard inupgrade()only covers the no-upgrade-available branch, so it never fires here. Note the working copy can hold a perfectly valid version — git HEAD wins.resolveSourceVersionnow returnsstring | undefinedand validates its result:undefinedmeans skip migrations: the migration block is guarded, and--from/FERN_PRE_UPGRADE_VERSIONare omitted from the rerun instead of forwarding a poisoned value.fern.config.jsonis still rewritten to the target version.Changes Made
resolveSourceVersionvalidates its result viaisValidVersion, falling back to the running CLI version, then to skipping migrationsTesting
Unit tests added/updated —
upgrade.test.tsnow uses the realisValidVersion(onlyisVersionAheadis mocked) and covers"*"in the rerun path, the already-at-target path, and the local-dev skip path. 50 tests pass.Manual testing completed. Clean repo, committed
fern.config.json="*", working copy =5.57.0.Before, with the published CLI:
After, with this branch built (
pnpm fern-dev:build):Link to Devin session: https://app.devin.ai/sessions/5ed9ce4d0f064626b2f66a2456abfb9a