fix(cli): handle catalog: protocol in dependency update check (#3905) - #4965
kaiizer777 wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 8499022 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Hi @kaiizer777, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
✨ 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. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (version.startsWith("workspace") || version.startsWith("catalog:")) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🟡 Catalog dependencies bypass version enforcement
With installed catalog dependencies, getTriggerDependencies drops them before resolving their concrete versions. dev, deploy, and update then accept incompatible Trigger.dev package versions without warning or updating them.
Learn more
Catalog specifiers such as catalog: name a version stored in workspace configuration. They are not evidence that the installed package matches the CLI. The existing resolver can recover that package's concrete installed version, and previously did so whenever resolution succeeded. Returning early now removes the dependency from both mismatch detection and the required update gate.
Example: A workspace catalog pins @trigger.dev/sdk to 4.5.0 while the CLI is 4.6.3. Even with 4.5.0 installed and resolvable, the dependency list is empty, so trigger deploy passes the required update check.
Recommended fix: Resolve catalog dependencies and retain their protocol metadata separately from the concrete installed version. Use the concrete version for mismatch enforcement. When applying an update, modify the relevant catalog definition or emit actionable guidance instead of replacing the package manifest's catalog reference.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const isDowngrade = mismatches.some((dep) => { | ||
| if (!semver.validRange(dep.version)) { | ||
| return false; | ||
| } | ||
|
|
||
| try { | ||
| const depMinVersion = semver.minVersion(dep.version); | ||
|
|
||
| if (!depMinVersion) { | ||
| return false; | ||
| } | ||
|
|
||
| return semver.gt(depMinVersion, targetVersion); | ||
| } catch { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🔍 Unknown protocols remain mismatches
getVersionMismatches avoids the crash but retains every unknown protocol in mismatches. Required checks still abort, while interactive updates replace those specifiers with the CLI version.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Thanks @devin-ai-integration for the thorough review! Both points have been addressed in commit
All 7 Vitest tests are passing, formatted with |
1 participant
Overview
Fixes #3905
In bun and pnpm monorepos utilizing workspace catalogs, dependencies in
package.jsonare specified with catalog protocols (e.g."@trigger.dev/sdk": "catalog:"or"catalog:<catalog-name>").When running CLI commands such as
trigger dev,trigger deploy, ortrigger update,updateTriggerPackages()checks for version mismatches:getTriggerDependencies()previously only skipped dependencies starting with"workspace", but did not filter out"catalog:"specifiers. If local resolution failed to resolve the local installed package path,"catalog:"was retained as the dependency version.getVersionMismatches(),semver.minVersion(dep.version)was called directly on"catalog:", which threw an unhandledTypeError: Invalid comparator: catalog:, crashing the CLI process immediately.Changes
packages/cli-v3/src/commands/update.ts:getTriggerDependencies()to skipcatalog:dependencies in addition toworkspacedependencies.getVersionMismatches()withsemver.validRange(dep.version)and wrappedsemver.minVersion(dep.version)in atry...catchblock to gracefully skip non-standard or unresolvable version specifiers without throwing.Dependency,getVersionMismatches, andgetTriggerDependenciesfor direct unit testing.packages/cli-v3/src/commands/update.test.tsverifying thatcatalog:andworkspace:protocols are handled cleanly without throwing, and thatgetVersionMismatches()never crashes on non-semver strings.trigger.dev.Note
A vouch request is already open at #4963.