Skip to content

Conversation

@christian-byrne
Copy link
Contributor

@christian-byrne christian-byrne commented Sep 3, 2025

Summary

  • Fix nodepack installation failure caused by using invalid 'stable' channel value
  • Replace 'stable' with 'default' which is a valid ManagerChannel enum value
  • Update ManagerChannelValues constant to use correct enum mapping

Problem

Nodepack installations were failing validation at the /v2/manager/queue/task endpoint because the frontend was sending channel: 'stable' but the backend ManagerChannel enum only accepts: 'default' | 'recent' | 'legacy' | 'forked' | 'dev' | 'tutorial'.

After, switching versions works again from the version selection popover

Selection_2081

┆Issue is synchronized with this Notion page by Unito

Fix nodepack installation failure caused by using 'stable' channel value
which is not defined in the ManagerChannel enum. Changed from 'stable'
to 'default' which is a valid enum value according to the backend schema.

Fixes nodepack installation requests that were failing validation at
/v2/manager/queue/task endpoint.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@christian-byrne christian-byrne requested a review from a team as a code owner September 3, 2025 03:43
@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Sep 3, 2025
@github-actions
Copy link

github-actions bot commented Sep 3, 2025

🎭 Playwright Test Results

All tests passed across all browsers!

⏰ Completed at: 09/03/2025, 04:30:52 AM UTC

📊 Test Reports by Browser


🎉 Your tests are passing across all browsers!

viva-jinyi
viva-jinyi previously approved these changes Sep 3, 2025
Copy link
Member

@viva-jinyi viva-jinyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Comment on lines 112 to 115
const ManagerChannelValues = {
STABLE: 'stable' as ManagerChannel,
DEFAULT: 'default' as ManagerChannel,
DEV: 'dev' as ManagerChannel
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We switched to API-driven design where there is a single source of truth (the openapi spec) which then generates TS types and Pydantic models and both client and server use those types. We should try to actually enforce the string enum type from those types here instead of defining our own enum inside the component (because it can allow for the exact type of error this PR is trying to resolve).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed to

const ManagerChannelValues: Record<string, ManagerChannel> = {
  DEFAULT: 'default',  // ✅ Compile-time validation
  DEV: 'dev'           // ✅ TypeScript catches invalid values
}

This ensures that future developers can't accidentally introduce invalid enum values - TypeScript will catch them during development rather than causing runtime API validation failures.

Remove type assertions (as ManagerChannel) and use explicit Record typing
to ensure compile-time validation of enum values. This prevents invalid
enum values from being used by catching them during TypeScript compilation
rather than runtime validation failures.

- Replace type assertions with Record<string, ManagerChannel> typing
- Remove manual casting that bypassed TypeScript's type checking
- Ensure invalid enum values cause compilation errors
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Sep 3, 2025
@christian-byrne christian-byrne merged commit 35f5773 into main Sep 3, 2025
14 checks passed
@christian-byrne christian-byrne deleted the fix/nodepack-installation-channel-enum branch September 3, 2025 05:55
@christian-byrne
Copy link
Contributor Author

Oops forgot to remove comments, will do in followup.

@benceruleanlu benceruleanlu mentioned this pull request Sep 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:manager size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants