Skip to content

Conversation

@jog1t
Copy link
Contributor

@jog1t jog1t commented Jan 7, 2026

TL;DR

Refactored runner configuration UI to support both shared and per-datacenter settings, improving the management of serverless providers across multiple regions.

What changed?

  • Created a new RunnerConfigToggleGroup component for switching between serverless and dedicated modes
  • Split the runner config form into shared settings and per-datacenter settings
  • Added ability to copy settings between datacenters
  • Improved the runner config table to group configurations by provider and endpoint
  • Added status indicators showing runner pool errors with detailed error messages
  • Enhanced region selection with multi-select capability
  • Reorganized the edit dialog to use an accordion for datacenter-specific settings
  • Fixed namespace handling in the Vercel connection flow

How to test?

  1. Navigate to the runner configuration page
  2. Create a new provider or edit an existing one
  3. Test switching between "Shared Settings" and "Per Datacenter Settings" modes
  4. Verify that settings can be applied globally or per region
  5. Check that error states are properly displayed in the runner config table
  6. Confirm that settings can be copied between datacenters in the per-datacenter view

Why make this change?

This change provides more flexibility in managing runner configurations across multiple regions. Users can now choose between applying the same configuration to all datacenters or customizing settings for specific regions. The improved UI makes it easier to identify and troubleshoot configuration issues with better error reporting and visualization of provider status across regions.

@vercel
Copy link

vercel bot commented Jan 7, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
rivet-cloud Ready Ready Preview, Comment Jan 9, 2026 9:41pm
rivet-inspector Ready Ready Preview, Comment Jan 9, 2026 9:41pm
2 Skipped Deployments
Project Deployment Review Updated (UTC)
rivet-site Ignored Ignored Preview Jan 9, 2026 9:41pm
rivetkit-serverless Skipped Skipped Jan 9, 2026 9:41pm

@vercel vercel bot temporarily deployed to Preview – rivetkit-serverless January 7, 2026 01:23 Inactive
Copy link
Contributor Author

jog1t commented Jan 7, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 7, 2026

Graphite Automations

"Test" took an action on this PR • (01/07/26)

1 assignee was added to this PR based on Kacper Wojciechowski's automation.

@claude
Copy link

claude bot commented Jan 7, 2026

PR Review: Editable Serverless Configs

This PR adds significant functionality for managing runner configurations with both shared and per-datacenter settings. Overall, the implementation is well-structured, but there are several issues that should be addressed.


🔴 Critical Issues

1. Type Safety Issue in edit-single-runner-config-form.tsx:28

The Enable component has a type mismatch:

export const Enable = <TValues extends Record<string, any> = FormValues>({name}: {name: FieldPath<TValues>;}) => {
	const { control } = useFormContext<FormValues>(); // ❌ Should be TValues
	return <FormField
		control={control}
		name={name as FieldPath<FormValues>} // ❌ Type cast masking the issue

Fix: Use the generic type consistently:

const { control } = useFormContext<TValues>();
return <FormField
	control={control}
	name={name}

2. Inconsistent Error Handling in edit-runner-config.tsx:141

The form submission in SharedSettingsForm does not handle the case where no regions are selected. The validation happens at schema level, but the mutation can theoretically receive an empty config object:

const selectedRegions = regions || {};
for (const [regionId, isSelected] of Object.entries(selectedRegions)) {
	if (isSelected) {
		providerConfig[regionId] = config;
	}
}
await mutateAsync({ name, config: providerConfig }); // Could be empty

3. Missing Null Check in runner-config-table.tsx:291

if (providers.length === 1) {
	return <Provider metadata={datacenters[0][1].metadata} />; //  datacenters[0] might not exist
}

Fix: Add proper bounds checking:

if (providers.length === 1 && datacenters.length > 0) {
	return <Provider metadata={datacenters[0][1].metadata} />;
}

🟡 Moderate Issues

4. Spacing/Formatting in edit-shared-runner-config-form.tsx:272-273

Awkward line break in class names - should be on one line.

5. Unnecessary Type Assertions in Headers Component

Multiple instances of type assertions that could be avoided with better typing:

e.target.value as PathValue<TValues, Path<TValues>>

Consider refactoring to use proper type inference instead of casting.

6. Duplicate Code in Form Components

The SharedSettingsForm and DatacenterSettingsForm components have significant duplication in mutation handling and query invalidation. Consider extracting into a shared hook.

7. Missing Error State Handling

The FetchError class in engine-data-provider.tsx:680 has a description field that is not used anywhere. Either use it or remove it.

8. Inconsistent Component Naming

edit-runner-config-form.tsx was renamed to edit-shared-runner-config-form.tsx, but internal exports still reference "RunnerConfigForm". Consider updating for consistency.


🟢 Minor Issues / Suggestions

9. Code Organization

The edit-runner-config.tsx file is getting quite large (323 lines). Consider splitting into separate files:

  • edit-runner-config/index.tsx
  • edit-runner-config/shared-settings-form.tsx
  • edit-runner-config/datacenter-settings-form.tsx
  • edit-runner-config/datacenter-accordion.tsx

10. Missing JSDoc Comments

Complex form components like Headers would benefit from JSDoc comments explaining the nested array structure.

11. Accessibility: Missing ARIA Labels

The accordion items in DatacenterAccordion should have better accessibility labels for screen readers when displaying datacenter-specific configurations.

12. Performance: Unnecessary Re-renders

The SelectDatacenterSettingsSource component fetches all datacenters and runner config on every render. Consider memoizing these queries or moving them higher in the component tree.


🔒 Security Considerations

13. Header Injection Concerns

The custom headers feature allows arbitrary header values. While likely intentional, consider:

  • Adding validation for header names (valid HTTP header format)
  • Warning users about sensitive data in headers
  • Sanitizing header values to prevent injection attacks

✅ Test Coverage

14. No Tests Added

This PR adds significant new functionality but includes no tests. Consider adding:

  • Unit tests for form validation schemas
  • Integration tests for the toggle between shared/datacenter settings
  • Tests for the datacenter settings copy functionality
  • Tests for error state handling

📊 Performance Considerations

15. Infinite Query Usage

The use of useSuspenseInfiniteQuery with maxPages: Infinity could be problematic if there are many regions. Consider pagination or limiting the fetch.


🎨 Code Quality Highlights

What is Good:

  • ✅ Good use of TypeScript generics for form field reusability
  • ✅ Proper error boundary integration with react-error-boundary
  • ✅ Good separation of concerns between shared and per-datacenter forms
  • ✅ Nice UX with the copy settings between datacenters feature
  • ✅ Consistent use of React Hook Form patterns
  • ✅ Good use of Zod for schema validation

📝 Documentation

16. Missing Comments for Complex Logic

The isSharedSettings logic in edit-runner-config.tsx:51-54 determines if settings are shared by comparing JSON strings. This clever approach deserves a comment explaining the reasoning.


🎯 Summary

Priority Fixes:

  1. Fix type safety issue in Enable component (Critical)
  2. Add proper bounds checking in Providers component (Critical)
  3. Handle empty providerConfig in form submission (Critical)
  4. Add test coverage for new functionality

Recommended Improvements:

  • Extract duplicate mutation logic into shared hooks
  • Split large files for better maintainability
  • Add JSDoc comments for complex components
  • Consider pagination for regions list

Overall Assessment: The functionality is well-implemented with good UX, but needs attention to type safety issues and test coverage before merging.


Review generated by Claude Code. Last updated: 2026-01-09

@jog1t jog1t force-pushed the 01-07-feat_dashboard_editable_serverless_configs branch from 851656c to f23273a Compare January 7, 2026 22:17
@jog1t jog1t force-pushed the 01-06-feat_dashboard_prefetch_daa_for_context_switcher branch from 06d1b43 to 69729dc Compare January 7, 2026 22:17
Base automatically changed from 01-06-feat_dashboard_prefetch_daa_for_context_switcher to main January 7, 2026 22:19
@jog1t jog1t force-pushed the 01-07-feat_dashboard_editable_serverless_configs branch from f23273a to fbe8ce8 Compare January 8, 2026 23:55
@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 8, 2026

More templates

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@3765

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/db@3765

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@3765

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@3765

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@3765

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@3765

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@3765

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@3765

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@3765

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@3765

commit: b188c5d

Copy link
Member

@NathanFlurry NathanFlurry left a comment

Choose a reason for hiding this comment

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

  • remove collapsible completely
  • endpoints for different dc -> "multiple endpoints"
  • "shared settings" -> "global settings"

@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 9, 2026

Merge activity

  • Jan 9, 9:41 PM UTC: jog1t added this pull request to the Graphite merge queue.
  • Jan 9, 9:42 PM UTC: CI is running for this pull request on a draft pull request (#3805) due to your merge queue CI optimization settings.
  • Jan 9, 9:42 PM UTC: Merged by the Graphite merge queue via draft PR: #3805.

graphite-app bot pushed a commit that referenced this pull request Jan 9, 2026
### TL;DR

Refactored runner configuration UI to support both shared and per-datacenter settings, improving the management of serverless providers across multiple regions.

### What changed?

- Created a new `RunnerConfigToggleGroup` component for switching between serverless and dedicated modes
- Split the runner config form into shared settings and per-datacenter settings
- Added ability to copy settings between datacenters
- Improved the runner config table to group configurations by provider and endpoint
- Added status indicators showing runner pool errors with detailed error messages
- Enhanced region selection with multi-select capability
- Reorganized the edit dialog to use an accordion for datacenter-specific settings
- Fixed namespace handling in the Vercel connection flow

### How to test?

1. Navigate to the runner configuration page
2. Create a new provider or edit an existing one
3. Test switching between "Shared Settings" and "Per Datacenter Settings" modes
4. Verify that settings can be applied globally or per region
5. Check that error states are properly displayed in the runner config table
6. Confirm that settings can be copied between datacenters in the per-datacenter view

### Why make this change?

This change provides more flexibility in managing runner configurations across multiple regions. Users can now choose between applying the same configuration to all datacenters or customizing settings for specific regions. The improved UI makes it easier to identify and troubleshoot configuration issues with better error reporting and visualization of provider status across regions.
@graphite-app graphite-app bot closed this Jan 9, 2026
@graphite-app graphite-app bot deleted the 01-07-feat_dashboard_editable_serverless_configs branch January 9, 2026 21:42
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.

3 participants