Skip to content

fix: missing context switcher for engine#4305

Open
jog1t wants to merge 1 commit into02-24-feat_allow_ips_in_runner_configsfrom
02-24-fix_missing_context_switcher_for_engine
Open

fix: missing context switcher for engine#4305
jog1t wants to merge 1 commit into02-24-feat_allow_ips_in_runner_configsfrom
02-24-fix_missing_context_switcher_for_engine

Conversation

@jog1t
Copy link
Contributor

@jog1t jog1t commented Feb 24, 2026

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

Copy link
Contributor Author

jog1t commented Feb 24, 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.

@jog1t jog1t mentioned this pull request Feb 24, 2026
11 tasks
@jog1t jog1t marked this pull request as ready for review February 24, 2026 20:46
@pkg-pr-new
Copy link

pkg-pr-new bot commented Feb 24, 2026

More templates

@rivetkit/cloudflare-workers

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/sqlite-vfs

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sqlite-vfs@4305

@rivetkit/traces

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

@rivetkit/workflow-engine

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/workflow-engine@4305

@rivetkit/virtual-websocket

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

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: 32a7836

@claude
Copy link

claude bot commented Feb 24, 2026

PR Review: fix: missing context switcher for engine

Summary

This PR adds context switcher support for the engine app type by adding an EngineNamespaceBreadcrumb component and routing the /ns/$namespace path through useContextSwitcherMatch. The other changes (builds list data provider swap, Array.from simplification, return navigate fix) are reasonable cleanup.


Issues

1. Unused import: useEngineDataProvider (context-switcher.tsx:33)

useEngineDataProvider is imported but never used in the file. This is dead code.

import {
    useCloudDataProvider,
    useEngineCompatDataProvider,
    useEngineDataProvider, // <-- never referenced in this file
} from "@/components/actors";

2. Inconsistent return type in namespaceQueryOptions (engine-data-provider.tsx:99–109)

The sibling method namespacesQueryOptions maps each API namespace to the local Namespace type ({ id, name, displayName, createdAt }). The new namespaceQueryOptions returns the raw API object (data.namespaces[0]) which has different field names (namespaceId instead of id, createTs instead of createdAt). The breadcrumb only accesses displayName so it works today, but the inconsistency is a type footgun.

// namespacesQueryOptions (consistent with Namespace type)
namespaces: data.namespaces.map((ns) => ({
    id: ns.namespaceId,     // mapped
    displayName: ns.displayName,
    name: ns.name,
    createdAt: new Date(ns.createTs).toISOString(),  // mapped
})),

// namespaceQueryOptions (raw API shape — inconsistent)
queryFn: async () => {
    const data = await client.namespaces.list({ name });
    return data.namespaces[0]; // returns namespaceId, createTs, etc.
},

Apply the same mapping used in namespacesQueryOptions, or at minimum type the return as Namespace | undefined so the mismatch is caught at compile time.

3. Missing query options in namespaceQueryOptions (engine-data-provider.tsx:99–109)

Compared to namespacesQueryOptions, the new method is missing:

  • retry: shouldRetryAllExpect403
  • throwOnError: noThrow
  • meta: { mightRequireAuth }

These aren't critical but the omission is inconsistent with every other query in this file and could cause different error behavior (e.g., 403s being retried unnecessarily).

4. Engine mode: context switcher dropdown is a no-op (context-switcher.tsx:273)

The Content component (the dropdown body) guards with if (!params.organization) return;. For the engine app type there is no organization, so the popover will open but render nothing. If switching namespaces is not yet intended in engine mode, the ContextSwitcherInner should not render a Popover at all for engine mode (or the button should be non-interactive). Currently the button opens an empty popover.

5. Imprecise biome-ignore placement (context-switcher.tsx:67–71)

The second biome-ignore lint/correctness/useHookAtTopLevel comment is on the useCloudDataProvider() call, not on usePrefetchInfiniteQuery. useCloudDataProvider is the one actually called conditionally here. The comment should be on the line that contains the hook being suppressed.

if (__APP_TYPE__ === "cloud") {
    // biome-ignore lint/correctness/useHookAtTopLevel: guaranteed by build condition
    usePrefetchInfiniteQuery({
        // biome-ignore lint/correctness/useHookAtTopLevel: guaranteed by build condition  ← this covers useCloudDataProvider, not usePrefetchInfiniteQuery (already covered above)
        ...useCloudDataProvider().projectsQueryOptions({

Consider restructuring as:

if (__APP_TYPE__ === "cloud") {
    // biome-ignore lint/correctness/useHookAtTopLevel: guaranteed by build condition
    const cloudProvider = useCloudDataProvider();
    // biome-ignore lint/correctness/useHookAtTopLevel: guaranteed by build condition
    usePrefetchInfiniteQuery(cloudProvider.projectsQueryOptions({ organization: organization! }));
}

Minor observations

  • return navigate(...) fix in actor-builds-list.tsx is correct and consistent with other onSelect handlers in the file.
  • Array.from() removal in default-data-provider.tsx is a valid simplification — Object.entries(...).map(...) already returns an array.
  • useEngineCompatDataProvider swap in ActorBuildsList makes sense since the component is app-type agnostic and should use the compat provider.

Verdict

The core feature (engine namespace breadcrumb) works for the happy path. The inconsistent raw API return type (#2) is the most likely source of a future bug if any consumer accesses fields other than displayName. The empty dropdown (#4) is a UX issue worth addressing before merge.

🤖 Generated with Claude Code

@jog1t jog1t mentioned this pull request Feb 24, 2026
11 tasks
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