Skip to content

feat(frontend): allow creating serverfull instances during onboarding#5035

Merged
jog1t merged 1 commit into
mainfrom
05-11-feat_frontend_allow_creating_serverfull_instances_during_onboarding
May 15, 2026
Merged

feat(frontend): allow creating serverfull instances during onboarding#5035
jog1t merged 1 commit into
mainfrom
05-11-feat_frontend_allow_creating_serverfull_instances_during_onboarding

Conversation

@jog1t

@jog1t jog1t commented May 11, 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

@jog1t jog1t marked this pull request as ready for review May 11, 2026 18:42
@claude

claude Bot commented May 11, 2026

Copy link
Copy Markdown
Contributor

PR Review: feat(frontend): allow creating serverfull instances during onboarding

Overview

This PR adds a serverless/runner toggle to the onboarding flow so users can configure a self-hosted runner (serverfull) instance instead of only serverless backends. It also ports the engine-mode namespace route to show the onboarding screen inline, and adds a "Skip onboarding" link for non-platform deployments.


Bugs

1. Leftover console.log in production code (critical)
dev-toolbar.tsx:15

console.log(ls.get("__I_SOLELY_SWORE_TO_ONLY_ENABLE_DEV_TOOLBAR_FOR_DEBUGGING_PURPOSES..."));

This fires on every render, including in production. Remove it before merging.

2. DevToolbar now renders twice in cloud routes
__root.tsx already had <DevToolbar /> in RootRoute (line 20). The PR adds a second one inside CloudRoute (line 32). Because CloudRoute is a child of the root, cloud pages mount two toolbar instances — both trigger the stale console.log above on every render. Keep it in one place only.

3. d.skipOnboarding in the loader is always undefined — dead code
ns.$namespace.tsx:66

const isSkipped =
    ls.onboarding.getSkipWelcomeEngine(params.namespace) ||
    d.skipOnboarding;  // always undefined

beforeLoad intercepts ?skipOnboarding=true, writes to localStorage, then redirects to { search: {} } before the loader runs. So d.skipOnboarding is always stripped from deps by the time it reaches the loader. The branch is dead code. Either remove the redirect from beforeLoad and move the localStorage write into the loader, or remove d.skipOnboarding from the loader and clean up the matching loaderDeps entries.

4. Unguarded datacenter cast
getting-started.tsx (serverfull submission block)

[v.datacenter as string]: {
    normal: {},
    metadata: { ... }
}

datacenter is typed string | undefined. The as string cast bypasses the type narrowing. If the serverfull schema is not selected correctly at validation time, an empty string or undefined key can silently be written into the config object. Validate v.datacenter is non-empty before using it as a key, or add an explicit guard.


Code Quality

5. Pervasive as unknown as type casts
Several places in getting-started.tsx use:

const v = values as unknown as { runnerName: string; provider: string; mode?: ...; datacenter?: string; ... };

This signals the form values aren't properly typed end-to-end. Consider defining a discriminated union for the two form shapes so the compiler can verify correctness rather than casting.

6. Mode-based schema selection is fragile
getting-started.tsx:104

if ((values.mode as string) === "serverfull") {
    return z.object({ mode: z.literal("serverfull"), ... });
}
return z.object({
    mode: z.union([...]).optional(),
    ...ConnectServerlessForm.configurationSchema.shape,
    ...ConnectServerlessForm.deploymentSchema.shape,
});

The fallback (serverless) schema makes mode optional and merges both shapes, so Zod won't reject a malformed submission when mode isn't set. z.discriminatedUnion("mode", [...]) would give exhaustive validation and eliminate the conditional branching.

7. Actors count fetched sequentially after runner queries
ns.$namespace.tsx:95

const [runnerNames, runnerConfigs] = await Promise.all([...]);
// ...
const actors = await context.queryClient.fetchQuery(
    context.dataProvider.actorsCountQueryOptions(),
);

All three are independent queries. Fetching the actors count in the same Promise.all removes a full round-trip from the onboarding page load.

8. Only first page checked for runner configs
ns.$namespace.tsx:100-101

const hasRunnerConfigs =
    Object.entries(runnerConfigs.pages[0].runnerConfigs).length > 0;

If the API paginates runner configs, a user with runners on page 2+ would be incorrectly shown the onboarding screen. Either flatten all pages (as runnerProvider already does), or add a comment explaining that a single page is sufficient here.

9. Duplicate isCustom logic
Both BackendSetupServerless and BackendSetupServerfull compute:

const isCustom = provider === "custom" || provider === "custom-platform";

Extract to a shared helper.

10. Unnecessary type cast on default form value
getting-started.tsx:213

mode: "serverless" as "serverless" | "serverfull",

TypeScript narrows string literals in object literals automatically. The cast is noise.


Architecture / Design

11. Duck-typing data provider capabilities

"upsertCurrentNamespaceManagedPoolMutationOptions" in dataProvider
"publishableTokenQueryOptions" in dataProvider

This works but accumulates fragile "key" in dataProvider checks across the codebase. If there isn't already a discriminant on the provider type, adding one (e.g. dataProvider.kind === "cloud" | "engine") and narrowing through it would be cleaner and safer.

12. Untyped search params
ns.$namespace.tsx:31

const s = search as unknown as { skipOnboarding?: boolean; onboardingSuccess?: boolean };

TanStack Router supports typed, runtime-validated search params via validateSearch. Using as unknown as silently swallows malformed query strings.


Minor

  • PR description checklist is fully unchecked, including self-review and style guidelines.
  • useServerfullEndpoint falls back to "auto" when datacenter is empty — worth a brief comment explaining what "auto" means to the API so future readers don't treat it as a magic string.

@jog1t jog1t force-pushed the 05-11-feat_frontend_add_provider_icons branch from 97c2f75 to 577a189 Compare May 11, 2026 18:49
@jog1t jog1t force-pushed the 05-11-feat_frontend_allow_creating_serverfull_instances_during_onboarding branch from e600754 to 81994b8 Compare May 11, 2026 18:49
@jog1t jog1t force-pushed the 05-11-feat_frontend_add_provider_icons branch from 577a189 to a23259e Compare May 15, 2026 19:30
@jog1t jog1t force-pushed the 05-11-feat_frontend_allow_creating_serverfull_instances_during_onboarding branch from 81994b8 to 4b2680f Compare May 15, 2026 19:30
@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, 8:02 PM UTC: Graphite rebased this pull request as part of a merge.
  • May 15, 8:03 PM UTC: @jog1t merged this pull request with Graphite.

@jog1t jog1t changed the base branch from 05-11-feat_frontend_add_provider_icons to graphite-base/5035 May 15, 2026 19:59
@jog1t jog1t changed the base branch from graphite-base/5035 to main May 15, 2026 20:00
@jog1t jog1t force-pushed the 05-11-feat_frontend_allow_creating_serverfull_instances_during_onboarding branch from 4b2680f to 865a907 Compare May 15, 2026 20:01
@jog1t jog1t merged commit f9d7f74 into main May 15, 2026
8 of 11 checks passed
@jog1t jog1t deleted the 05-11-feat_frontend_allow_creating_serverfull_instances_during_onboarding branch May 15, 2026 20:03
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