Fix update check for catalog protocol deps#3982
Conversation
|
|
Hi @RitwijParmar, 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. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe ✨ 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) { | ||
| deps.push({ type, name, version: $version }); | ||
| continue; | ||
| } | ||
|
|
||
| if (packageManagerProtocolFilter.test(version)) { | ||
| logger.debug("Skipping unresolved package-manager dependency specifier", { | ||
| type, | ||
| name, | ||
| version, | ||
| }); | ||
| continue; | ||
| } | ||
|
|
||
| deps.push({ type, name, version }); |
There was a problem hiding this comment.
🚩 Behavioral change: workspace: deps are no longer unconditionally skipped
The old code at the removed lines unconditionally skipped any dependency whose version started with "workspace". The new code at update.ts:350-364 only skips protocol-prefixed versions when tryResolveTriggerPackageVersion returns undefined. In a real monorepo where workspace-linked @trigger.dev/* packages are installed in node_modules, nodeResolve.sync will resolve them successfully, so the resolved version will be used for mismatch comparison. If a mismatch is detected and the user confirms an update, mutatePackageJsonWithUpdatedPackages at update.ts:421 would overwrite the workspace:* specifier with a pinned version (e.g., "4.5.0"), silently breaking the monorepo's workspace linking. This is mitigated by the fact that (a) it requires explicit user confirmation, (b) end users rarely have workspace: deps for trigger.dev packages, and (c) in CI (!hasTTY) the command exits before any mutation. Still, monorepo users who do use workspace protocol for trigger.dev packages could be surprised by this change.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Fixes #3905
Checks
I tried bunx vitest run packages/cli-v3/src/commands/update.test.ts, but the local runner failed before tests started because the temporary Rolldown native binding was rejected by macOS code signing. I could not use pnpm here because the shell has no pnpm binary and the bundled pnpm entrypoint hung on --version.