Skip to content

fix(cli,core,webapp): flag legacy Node deployments for runtime updates - #10

Open
anurag6569201 wants to merge 1 commit into
qa/agent-triggerdotdev-trigger-dev/pr-17-4865/basefrom
qa/agent-triggerdotdev-trigger-dev/pr-17-4865/head
Open

fix(cli,core,webapp): flag legacy Node deployments for runtime updates#10
anurag6569201 wants to merge 1 commit into
qa/agent-triggerdotdev-trigger-dev/pr-17-4865/basefrom
qa/agent-triggerdotdev-trigger-dev/pr-17-4865/head

Conversation

@anurag6569201

Copy link
Copy Markdown

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-update consistent 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.

Source merge-base: 43ecf15f80277c0eb931c08ddd89de55179cc795
Source head: 77d1cb0bb5272ff9a65d4d9a96a806b026e325e9

@shipwright-agent

Copy link
Copy Markdown

⛔ Shipwright · Blocked

Recommendation: do not merge PR #10 · Tier T1
Checks: 0 total · 0 needing attention

Next step: resolve the blocking findings before merge.

Findings (6)

  • CRITICAL The JS helper and SQL query disagree on which projects need a runtime update. · packages/core/src/v3/schemas/api.ts:88
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • CRITICAL The SQL second branch explicitly matches runtime: 'node-21' with runtimeVersion: null, but the JS helper's configuredMajorMatch only accepts runtime strings of the form 'node-<digi · apps/webapp/app/services/projectRuntimeUpdates.server.ts:111
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The new needsNodeRuntimeUpdate function treats a null runtimeVersion as needing an update for runtime 'node' or null, but the SQL query in organizationHasProjectRuntimeUpdate only · packages/core/src/v3/schemas/api.ts:88
    • Fix: Fix the review finding before release.
  • HIGH The SQL query's second branch matches runtimeVersion: null with runtime: 'node-21', but the JS helper's configuredMajorMatch only matches runtime strings of the form 'node-<digits> · apps/webapp/app/services/projectRuntimeUpdates.server.ts:111
    • Fix: Fix the review finding before release.
  • HIGH needsNodeRuntimeUpdate has no return type annotation and its branching logic is non-obvious: a null runtimeVersion with runtime 'node' returns true, but a null runtimeVersion with · packages/core/src/v3/schemas/api.ts:82
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The removed doc comment previously documented that the SQL filter mirrors the JS logic and is scoped to caller membership. · apps/webapp/app/services/projectRuntimeUpdates.server.ts:85
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

Fireworks usage: 8,643 input · 928 output · 9,571 total tokens · $0.0025 · 15s · 0 fix iteration(s)

Open the Shipwright check for full evidence and the audit bundle. Use /shipwright rerun to verify again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant