Skip to content

feat(frontend): add enoys to the runner table#4675

Open
jog1t wants to merge 1 commit into04-14-refactor_frontend_support_envoysfrom
04-16-feat_frontend_add_enoys_to_the_runner_table
Open

feat(frontend): add enoys to the runner table#4675
jog1t wants to merge 1 commit into04-14-refactor_frontend_support_envoysfrom
04-16-feat_frontend_add_enoys_to_the_runner_table

Conversation

@jog1t
Copy link
Copy Markdown
Contributor

@jog1t jog1t commented Apr 16, 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

@railway-app
Copy link
Copy Markdown

railway-app bot commented Apr 16, 2026

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

Service Status Web Updated (UTC)
frontend-cloud 😴 Sleeping (View Logs) Web Apr 16, 2026 at 9:53 pm
frontend-inspector 😴 Sleeping (View Logs) Web Apr 16, 2026 at 9:50 pm
website 😴 Sleeping (View Logs) Web Apr 16, 2026 at 9:43 pm
kitchen-sink ❌ Build Failed (View Logs) Web Apr 16, 2026 at 9:42 pm
ladle ❌ Build Failed (View Logs) Web Apr 16, 2026 at 9:42 pm
mcp-hub ✅ Success (View Logs) Web Apr 16, 2026 at 9:33 pm

Copy link
Copy Markdown
Contributor Author

jog1t commented Apr 16, 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

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

@jog1t jog1t mentioned this pull request Apr 16, 2026
11 tasks
@claude
Copy link
Copy Markdown

claude bot commented Apr 16, 2026

PR Review: Add Envoys to the Runner Table

Summary

This PR adds Envoy entries alongside Runners in the settings table view, with supporting data fetching and type-safe rendering. Overall the approach is sound, but there are a few issues worth addressing.


Issues

Pagination logic is broken (Medium)

File: frontend/src/routes/_context/_cloud/orgs.$organization/projects.$project/ns.$namespace/settings.tsx

The fetchNextPage handler calls both paginations independently:

fetchNextPage={() => {
  if (hasNextEnvoysPage) fetchNextEnvoysPage();
  if (hasNextPage) fetchNextPage();
}}

This fires both fetches simultaneously on every click, rather than coordinating them. If envoys have 2 pages and runners have 1, the second click will attempt to fetch a runners page that doesn't exist. Consider tracking the two queries separately or using a combined cursor strategy.

Dead code (Low)

File: frontend/src/lib/data.ts

safeJsonParse() is defined but never exported, imported, or called anywhere. Remove it or export it if it's intended for future use.

Unrelated change needs context (Low)

File: frontend/src/app/dialogs/connect-manual-serverfull-frame.tsx

Step 3 ("Wait for the Runner to connect") is removed with no explanation. This appears unrelated to the envoy feature. If this is intentional (e.g., runners now auto-connect), a comment or PR note explaining why would help reviewers and future readers.

Missing memoization

File: frontend/src/routes/.../settings.tsx

allRunners = [...envoys, ...runners]

This array is recreated on every render. Wrapping in useMemo would avoid unnecessary re-renders of RunnersTable.


Positive Notes

  • Type-guard pattern (if ('runnerId' in runner)) is correct and type-safe for the discriminated union.
  • Narrowing RunnerStatusBadge to Pick<Rivet.Runner, 'version' | 'lastPingTs' | 'lastRtt'> reduces unnecessary coupling.
  • Switching from .parse() to .safeParse() in deriveRivetkitVersionFromMetadata() prevents runtime exceptions on malformed metadata — good defensive change.
  • Schema extraction (rivetkitSchema, metadataSchema) improves reusability.

Test Coverage

No tests cover the combined Runner+Envoy rendering path or the pagination coordination logic. Given the pagination issue above, a test for the load-more behavior with mixed data would be valuable.

@jog1t jog1t force-pushed the 04-16-feat_frontend_add_enoys_to_the_runner_table branch from 6a878b9 to d9492e3 Compare April 16, 2026 21:41
@jog1t jog1t force-pushed the 04-14-refactor_frontend_support_envoys branch from 597599a to b34ac38 Compare April 16, 2026 21:41
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 16, 2026

Preview packages published to npm

Install with:

npm install rivetkit@pr-4675

All packages published as 0.0.0-pr.4675.c03f935 with tag pr-4675.

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-c03f935
docker pull rivetdev/engine:full-c03f935
Individual packages
npm install rivetkit@pr-4675
npm install @rivetkit/react@pr-4675
npm install @rivetkit/rivetkit-native@pr-4675
npm install @rivetkit/sqlite-wasm@pr-4675
npm install @rivetkit/workflow-engine@pr-4675

@claude
Copy link
Copy Markdown

claude bot commented Apr 16, 2026

Code Review

This PR adds Envoy entries to the Runners table, extends metadata parsing, and removes a dialog step.

Issues:

  1. safeJsonParse in data.ts is declared but never called or exported - dead code, remove it.

  2. data.ts is missing a trailing newline at end of file.

  3. Commented-out code in connect-manual-serverfull-frame.tsx (step-3 line) should be removed since Step3 was deleted.

  4. latestVersion is computed from Math.max over both Runner and Envoy versions mixed together. If version spaces are independent counters this will produce spurious outdated badges on the RunnerStatusBadge. Confirm they share the same version space or split the calculation by type.

  5. envoysListQueryOptions uses 'as any' for queryKey - use 'as QueryKey' to match the runners query pattern.

  6. envoysListQueryOptions is missing retry: shouldRetryAllExpect403 and meta: mightRequireAuth that the runners query has. Without these, auth-error UX will not trigger correctly on 403 responses.

  7. The Runners container div has both pb-4 and pb-8 classes - pb-4 is dead since Tailwind applies pb-8.

Minor notes:

  • PR title says 'enoys' (missing v) - should be 'envoys'.
  • PR description body is still the default template, not filled in.
  • metadataSchema string-fallback union behavior could use a brief comment.

What looks good:

  • Rivet.Runner | Rivet.Envoy union with 'runnerId' in runner discriminant is the right approach.
  • infiniteQueryOptions with select to flatten pages is consistent with existing patterns.
  • safeParse instead of parse in deriveRivetkitVersionFromMetadata is a real improvement.
  • Removing the Wait for Runner step is a reasonable simplification.

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