Skip to content

feat(frontend): add serverless check when editing the runner config#4961

Merged
jog1t merged 1 commit into
mainfrom
04-29-feat_frontend_add_serverless_check_when_editing_the_runner_config
May 15, 2026
Merged

feat(frontend): add serverless check when editing the runner config#4961
jog1t merged 1 commit into
mainfrom
04-29-feat_frontend_add_serverless_check_when_editing_the_runner_config

Conversation

@jog1t

@jog1t jog1t commented May 4, 2026

Copy link
Copy Markdown
Contributor

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@railway-app

railway-app Bot commented May 4, 2026

Copy link
Copy Markdown

🚅 Deployed to the rivet-pr-4961 environment in rivet-frontend

Service Status Web Updated (UTC)
kitchen-sink ❌ Build Failed (View Logs) Web May 15, 2026 at 7:30 pm
website 😴 Sleeping (View Logs) Web May 7, 2026 at 3:56 am
frontend-inspector ❌ Build Failed (View Logs) Web May 5, 2026 at 6:20 pm
ladle ❌ Build Failed (View Logs) Web May 5, 2026 at 6:20 pm
frontend-cloud ❌ Build Failed (View Logs) Web May 5, 2026 at 6:19 pm
mcp-hub ✅ Success (View Logs) Web May 4, 2026 at 4:48 pm

@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-4961 May 4, 2026 16:44 Destroyed

jog1t commented May 4, 2026

Copy link
Copy Markdown
Contributor Author

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

@claude

claude Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

PR #4961 Review — feat(frontend): add serverless check when editing the runner config

Overview

This PR adds serverless/serverfull mode detection and switching to the runner config editor. Key additions:

  • Mode toggle between "serverless" and "serverfull" per-datacenter or globally
  • Live endpoint health checks via polling (serverless-endpoint-health.tsx) with debounce + visual indicators
  • Mode-switch confirmation flow in ConfirmableSaveButton that warns before overwriting existing configs
  • Form field reset via ResetOnModeChange when the mode toggle changes
  • Schema refactor: formSchema split into baseFormSchema + superRefine for conditional URL validation

The feature is well-designed and the UX patterns (confirmation dialog, inline health indicator, graceful reset) are solid. A few issues worth addressing:


Issues

1. SERVERLESS_FIELDS constant placed between import groups

// edit-runner-config.tsx
import { useFormContext, useFormState, useWatch } from "react-hook-form";

const SERVERLESS_FIELDS = [...] as const;   // ← sits between two import blocks

import * as EditRunnerConfigForm from "@/app/forms/edit-shared-runner-config-form";

CLAUDE.md: "Always add imports at the top of the file instead of inline within a function." The constant should move after all imports are grouped.

2. fallbackMetadata return type is unknown | undefined

function fallbackMetadata(...): unknown | undefined { ... }

unknown already includes undefined; the union is redundant. Return type should be unknown.

3. Polling overhead from EndpointHealthIndicator

Each Url field in the datacenter accordion spawns its own 5-second polling query via useQuery({ refetchInterval: 5_000 }). With many datacenters expanded simultaneously this multiplies server load linearly. Consider either:

  • Sharing a single polling slot per unique (endpoint, headers) pair, or
  • Only polling visible fields (the VisibilitySensor component already exists in the codebase for this pattern).

4. Hidden submit button pattern

<button type="submit" ref={hiddenSubmitRef} className="hidden" tabIndex={-1} aria-hidden />
// ...
hiddenSubmitRef.current?.click();

This works but is an unusual indirection. The more idiomatic react-hook-form approach is to call form.handleSubmit(onSubmit)() directly, which avoids the hidden DOM element and makes the control flow clearer.

5. useWatch() without args in ConfirmableSaveButton

const allValues = useWatch();
useEffect(() => { setPending(null); }, [allValues]);

useWatch() with no arguments subscribes to every field and returns a new object reference on each change. This is intentional here, but worth a brief comment since the pattern looks like an accidental "empty deps" bug to future readers.

6. www.rivet.dev subdomain in ServerfullModeNotice

href="https://www.rivet.dev/docs/general/runtime-modes/"

CLAUDE.md specifies using rivet.dev (no www). Use https://rivet.dev/docs/general/runtime-modes/ for consistency.

7. Mixed-mode detection picks only the first datacenter

const detectedMode: RuntimeMode = useMemo(() => {
    const modes = Object.values(data.datacenters).map(dcMode).filter(...);
    return modes[0] ?? "serverless";
}, [data.datacenters]);

When isSharedSettings is true all DCs should have the same mode signature, so modes[0] is correct. But this assumption is implicit — a short comment explaining that isSharedSettings already gates mixed-mode cases to the per-DC form would help future readers.

8. No test coverage

The PR checklist item for tests is unchecked. The health check polling logic and mode-switch confirmation flow have meaningful branches worth testing. At minimum, the pure functions extractErrorMessage, describeSwitches, dcMode, and dcSignatureWithoutMetadata are straightforward unit test targets.


Minor / Nits

  • Awkward as cast formatting in dcSignatureWithoutMetadata: the dc as wraps to the next line mid-expression; keep it on one line.
  • validateRuntimeModeFields allocates a Zod schema per call: z.string().url() is re-created on every validation. Hoist it to a module-level constant.
  • isLoading vs isPending: TanStack Query v5 deprecated isLoading in favour of isPending; confirm which TQ version is in use and align accordingly.

Summary

Good feature overall. The UX design is thoughtful and the schema split is clean. The main actionable items are the import ordering issue, polling deduplication for multi-datacenter scenarios, replacing the hidden-submit pattern, and adding at least a few unit tests for the pure helper functions.

@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

Preview packages published to npm

Install with:

npm install rivetkit@pr-4961

All packages published as 0.0.0-pr.4961.d447d31 with tag pr-4961.

Engine binary is shipped via @rivetkit/engine-cli on linux-x64-musl, linux-arm64-musl, darwin-x64, and darwin-arm64. Windows users should use the release installer or set RIVET_ENGINE_BINARY.

Docker images:

docker pull rivetdev/engine:slim-d447d31
docker pull rivetdev/engine:full-d447d31
Individual packages
npm install rivetkit@pr-4961
npm install @rivetkit/react@pr-4961
npm install @rivetkit/rivetkit-napi@pr-4961
npm install @rivetkit/workflow-engine@pr-4961

@jog1t jog1t force-pushed the 04-29-feat_frontend_add_serverless_check_when_editing_the_runner_config branch from f54e255 to 4c5456e Compare May 4, 2026 21:14
@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-4961 May 4, 2026 21:14 Destroyed
@jog1t jog1t mentioned this pull request May 4, 2026
11 tasks
@jog1t jog1t marked this pull request as ready for review May 4, 2026 21:14
@jog1t jog1t force-pushed the 04-29-feat_frontend_add_serverless_check_when_editing_the_runner_config branch from 4c5456e to c92292f Compare May 5, 2026 18:18
@jog1t jog1t force-pushed the 04-29-feat_frontend_add_serverless_check_when_editing_the_runner_config branch from c92292f to 1fd52f8 Compare May 11, 2026 18:49
@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-4961 May 11, 2026 18:49 Destroyed
@jog1t jog1t force-pushed the 04-29-feat_frontend_add_serverless_check_when_editing_the_runner_config branch from 1fd52f8 to 15d24cd Compare May 15, 2026 19:30
@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-4961 May 15, 2026 19:30 Destroyed
@jog1t jog1t mentioned this pull request May 15, 2026
11 tasks

jog1t commented May 15, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

  • May 15, 7:36 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • May 15, 7:36 PM UTC: @jog1t merged this pull request with Graphite.

@jog1t jog1t merged commit 9befa9a into main May 15, 2026
13 of 17 checks passed
@jog1t jog1t deleted the 04-29-feat_frontend_add_serverless_check_when_editing_the_runner_config branch May 15, 2026 19:36
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