fix(cli,core,webapp): flag legacy Node deployments for runtime updates - #4865
Conversation
|
WalkthroughThe change adds Merge Risk: 🟡 Moderate · up to The PR expands legacy Node.js update reporting, but malformed recorded versions can still make the organization indicator disagree with the settings page and CLI. This bounded reporting inconsistency should be fixed or explicitly accepted before merge; no security or deployed-runtime risk is indicated. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/core/src/v3/schemas/api.ts (1)
82-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd required crumb markers to the changed behavioral blocks.
packages/core/src/v3/schemas/api.ts#L82-L98: add a//@Crumbsmarker or `#region `@crumbsblock for the classifier changes.packages/core/src/v3/schemas/api-type.test.ts#L25-L39: add a crumb marker for the new classifier test block.apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.projects/route.tsx#L57-L57: add a crumb marker for the updated project classification branch.packages/cli-v3/src/commands/projects/list.ts#L73-L77: add a crumb marker for the updated CLI filter.apps/webapp/app/services/projectRuntimeUpdates.server.ts#L111-L124: add a crumb marker for the database filter change.As per coding guidelines, “Add crumbs as you write code — not just when debugging.”
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Team
Run ID: 2fb6b962-c8d7-4c6b-ae1e-3c91899832f6
📒 Files selected for processing (5)
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.projects/route.tsxapps/webapp/app/services/projectRuntimeUpdates.server.tspackages/cli-v3/src/commands/projects/list.tspackages/core/src/v3/schemas/api-type.test.tspackages/core/src/v3/schemas/api.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (48)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (22, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
- GitHub Check: sdk-compat / Node.js 24.18 (warp-ubuntu-latest-x64-4x)
- GitHub Check: sdk-compat / Node.js 20.20 (warp-ubuntu-latest-x64-4x)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-ubuntu-latest-x64-4x - pnpm)
- GitHub Check: obsmap / 🧪 Unit Tests: Observability Map
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - npm)
- GitHub Check: sdk-compat / Node.js 22.23 (warp-ubuntu-latest-x64-4x)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: sdk-compat / Node.js 26.4 (warp-ubuntu-latest-x64-4x)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-ubuntu-latest-x64-4x - npm)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
- GitHub Check: runops-guard / runops-guard
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
- GitHub Check: fk-cascade-guard / fk-cascade-guard
- GitHub Check: packages / 🧪 Unit Tests: Packages (2, 3)
- GitHub Check: code-quality / code-quality
- GitHub Check: Build and publish previews
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (14)
Never use `request.signal` to detect client disconnects. Use `getRequestAbortSignal()` from `app/services/httpAsyncStorage.server.ts`, which is wired to Express response close events.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/services/projectRuntimeUpdates.server.ts
We use vitest exclusively. **Never mock anything** - use testcontainers instead.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/core/src/v3/schemas/api-type.test.ts
For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/services/projectRuntimeUpdates.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.projects/route.tsx
**Import subpaths only** (never root).
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/core/src/v3/schemas/api-type.test.tspackages/core/src/v3/schemas/api.ts
**Prefer static imports over dynamic imports.** Only use dynamic `import()` when:
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/core/src/v3/schemas/api-type.test.tsapps/webapp/app/services/projectRuntimeUpdates.server.tspackages/cli-v3/src/commands/projects/list.tspackages/core/src/v3/schemas/api.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.projects/route.tsx
Add crumbs as you write code — not just when debugging. Mark lines with
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/core/src/v3/schemas/api-type.test.tsapps/webapp/app/services/projectRuntimeUpdates.server.tspackages/cli-v3/src/commands/projects/list.tspackages/core/src/v3/schemas/api.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.projects/route.tsx
Use zod for validation in packages/core and apps/webapp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/core/src/v3/schemas/api-type.test.tsapps/webapp/app/services/projectRuntimeUpdates.server.tspackages/core/src/v3/schemas/api.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.projects/route.tsx
CLI command definitions should be located in `src/commands/`
📄 CodeRabbit inference engine (packages/cli-v3/CLAUDE.md)
Files:
packages/cli-v3/src/commands/projects/list.ts
Never import the root package (`@trigger.dev/core`). Always use subpath imports such as `@trigger.dev/core/v3`, `@trigger.dev/core/v3/utils`, `@trigger.dev/core/logger`, or `@trigger.dev/core/schemas`
📄 CodeRabbit inference engine (packages/core/CLAUDE.md)
Files:
packages/core/src/v3/schemas/api-type.test.tspackages/core/src/v3/schemas/api.ts
Access environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env`
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/app/services/projectRuntimeUpdates.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.projects/route.tsx
Use vitest for all tests in the Trigger.dev repository
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/core/src/v3/schemas/api-type.test.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/core/src/v3/schemas/api-type.test.tsapps/webapp/app/services/projectRuntimeUpdates.server.tspackages/cli-v3/src/commands/projects/list.tspackages/core/src/v3/schemas/api.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.projects/route.tsx
Use types over interfaces for TypeScript
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/core/src/v3/schemas/api-type.test.tsapps/webapp/app/services/projectRuntimeUpdates.server.tspackages/cli-v3/src/commands/projects/list.tspackages/core/src/v3/schemas/api.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.projects/route.tsx
When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
packages/core/src/v3/schemas/api-type.test.tsapps/webapp/app/services/projectRuntimeUpdates.server.tspackages/cli-v3/src/commands/projects/list.tspackages/core/src/v3/schemas/api.ts
2 participants
Summary
Include legacy Node.js deployments in runtime update reports when their stored runtime name or version is missing. This keeps the dashboard and
projects list --needs-updateconsistent with how those deployments are displayed.Root cause
The runtime display treated a missing runtime name as Node.js, while update detection required both a Node runtime name and a parseable version. A shared classifier now treats missing legacy Node.js metadata as requiring an update, while explicit non-Node runtimes and supported Node.js majors remain excluded.
The organization settings indicator uses the same rules. Verified with the runtime classification unit tests and typechecks for core, CLI, and webapp.