Skip to content

Conversation

@jog1t
Copy link
Contributor

@jog1t jog1t commented Nov 21, 2025

No description provided.

@vercel
Copy link

vercel bot commented Nov 21, 2025

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

Project Deployment Preview Comments Updated (UTC)
rivet-inspector Error Error Dec 4, 2025 8:49pm
rivetkit-serverless Error Error Dec 4, 2025 8:49pm
2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
rivet-cloud Ignored Ignored Dec 4, 2025 8:49pm
rivet-site Ignored Ignored Preview Dec 4, 2025 8:49pm

Copy link
Contributor Author

jog1t commented Nov 21, 2025


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.

@claude
Copy link

claude bot commented Nov 21, 2025

PR Review: feat(devtools): add makeshift devtools

Summary

This PR introduces a new browser-based inspector package (@rivetkit/inspector) that can be embedded in web applications to debug Rivet Actors. The inspector uses Shadow DOM for isolation and reuses frontend components via symlinks.

Critical Issues 🚨

1. Broken JSX Merge Conflict (BLOCKING)

Location: frontend/src/components/actors/actors-actor-details.tsx:211-238

The file contains a malformed JSX merge conflict that will cause build failures:

{actorId ? (
  <Flex gap="2" justify="between" items="center" className="h-full pb-3 pt-2 pr-4 sticky right-0 bg-card">
    State  // ❌ This should be children of Flex, not a closing TabsTrigger
  </TabsTrigger>  // ❌ This closing tag doesn't match the opening Flex
) : null}
{supportsConnections ? (  // ❌ Duplicate tabs follow...
  <TabsTrigger disabled={disabled} value="connections" className="text-xs px-3 py-1 pb-2">
    Connections
  </TabsTrigger>
) : null}
// ... more duplicate tabs

Impact: Build will fail, TypeScript errors, broken UI.

Fix Required: Review lines 211-238 and remove duplicated tab declarations that appear after the TabsList closing tag.


2. Duplicate Function Declaration (BLOCKING)

Location: frontend/src/components/actors/worker/actor-worker-context.tsx:23-35, 61-74

The useConnectionDetails function is declared twice with identical implementations:

  • Lines 23-35 (first declaration)
  • Lines 61-74 (duplicate declaration)

Impact: Build failure due to duplicate declarations.

Fix Required: Remove one of the duplicate function declarations.


3. Hardcoded Credentials (SECURITY)

Locations:

  • rivetkit-typescript/packages/inspect/src/routes/_context.tsx:9-10
  • rivetkit-typescript/packages/inspect/src/routes/_context/index.tsx:22-23

Hardcoded credentials appear in multiple files:

url: "http://127.0.0.1:6420",
token: "lNtOy9TZxmt2yGukMJREcJcEp78WDzuj"

Impact: Credentials committed to version control, potential security risk if token is valid.

Recommendations:

  1. Use environment variables or allow runtime configuration
  2. Remove or rotate the hardcoded token if it's real
  3. Consider adding a configuration UI for users to input credentials
  4. Add this file pattern to .gitignore or use a .env.example pattern

4. Undefined Variable Reference (BLOCKING)

Location: rivetkit-typescript/packages/inspect/src/routes/_context/index.tsx:38

onClick={() => setIsOpen((prev) => !prev)}

The component uses setIsOpen but only declares credentials and setCredentials state (line 21-24). The isOpen state is managed in App.tsx, not this component.

Impact: Runtime error, broken close button functionality.

Fix Required: Either:

  • Lift isOpen state to a shared context, or
  • Remove the close button from this component (it's already in App.tsx)

Major Issues ⚠️

5. Missing ScrollArea Import

Location: frontend/src/components/actors/actors-actor-details.tsx:139

The component uses <ScrollArea> but the import isn't shown in the diff. Verify this is imported properly.


6. Build Configuration - Missing Format

Location: rivetkit-typescript/packages/inspect/vite.config.ts:19-26

The lib build configuration is missing a format specification:

build: {
  lib: {
    entry: path.resolve(__dirname, "src/main.tsx"),
    name: "Inspector",
    fileName: "index",
    // Missing: format: 'es' or 'iife'
  },
}

Recommendation: Add formats: ['es'] or formats: ['iife'] depending on usage requirements.


7. Symlinks May Break on Windows

Locations: Multiple symlinks in rivetkit-typescript/packages/inspect/src/

  • app -> ../../../../frontend/src/app
  • components -> ../../../../frontend/src/components
  • lib -> ../../../../frontend/src/lib
  • etc.

Impact: Development and build issues on Windows, potential CI/CD problems.

Recommendations:

  • Document the symlink requirement clearly in README
  • Consider using build-time copy scripts or workspace references instead
  • Test on Windows CI if supported
  • Add symlink verification to package scripts

Code Quality Issues

8. Inconsistent Padding Classes

Location: frontend/src/components/actors/actors-actor-details.tsx:151 vs 225

Tab triggers use inconsistent padding:

  • New tabs: pb-3 (line 151)
  • Duplicated/old tabs: pb-2 (line 225)

Recommendation: Use consistent styling throughout.


9. Dependency Management

Location: rivetkit-typescript/packages/inspect/package.json

The package lists runtime dependencies that should be dev dependencies:

"dependencies": {
  "autoprefixer": "^10.4.21",  // Should be devDependencies
  "postcss": "^8.5.6",           // Should be devDependencies
  "tailwindcss": "^3.4.17",      // Should be devDependencies
  "vite": "^7.2.2"                // Should be devDependencies
}

Impact: Larger bundle size for consumers, incorrect dependency tree.

Recommendation: Move build tools to devDependencies.


10. Magic Numbers

Locations:

  • rivetkit-typescript/packages/inspect/src/App.tsx:52: 24rem hardcoded height
  • rivetkit-typescript/packages/inspect/src/App.tsx:47: 2147483647 magic z-index

Recommendations:

  • Extract height to a named constant (e.g., INSPECTOR_HEIGHT)
  • Document why maximum z-index is needed or use a named constant

11. CSS Specificity Concerns

Location: rivetkit-typescript/packages/inspect/src/App.tsx:74

className="w-full h-96 bg-background-main border-t [&_*]:border-border [&_*]:text-foreground border-border"

The selector [&_*]:text-foreground and [&_*]:border-border will apply to ALL descendants, which may:

  • Override intentional color choices
  • Break third-party components
  • Cause performance issues with specificity wars

Recommendation: Use more targeted selectors or rely on CSS custom properties inheritance.


12. Theme Handling

Location: frontend/src/components/theme.css:1-3

:root[class~="dark"], :root, :host {
  /* Only dark theme variables defined */
}

Light theme CSS was removed entirely. If this is intentional for the inspector (dark-only), it should be documented. Otherwise, this breaks light theme support.


13. Missing Error Boundaries

The inspector embeds into arbitrary pages but has minimal error isolation:

  • No fallback UI if router fails
  • No handling of missing/invalid credentials
  • Connection failures could crash host app

Recommendation: Add error boundaries around critical sections, especially in App.tsx.


14. Accessibility Issues

Locations:

  • rivetkit-typescript/packages/inspect/src/App.tsx:68: Missing alt text on image
  • rivetkit-typescript/packages/inspect/src/routes/_context/index.tsx:30, 40: Missing alt text, missing ARIA labels on buttons
<img src={rivetLogo} className="w-full h-full" /> // ❌ Missing alt
<button type="button" className="size-4..."> // ❌ Missing aria-label

Recommendations:

  • Add alt="Rivet logo"
  • Add aria-label="Toggle inspector" to buttons

Minor Issues & Suggestions

15. Template README

Location: rivetkit-typescript/packages/inspect/README.md

The README is the default Vite + React template. Should be replaced with:

  • Purpose of the inspector package
  • Installation instructions
  • Usage examples
  • Configuration options
  • Symlink setup requirements

16. Console Logging

Location: rivetkit-typescript/packages/inspect/vite.config.ts:37

Error caught by router logs to console. Consider using a proper logging service for production builds.


17. Type Safety

Location: rivetkit-typescript/packages/inspect/src/routes/_context/index.tsx:57

(data?.metadata?.inspectorToken as string) || ""

Type assertion suggests weak typing. Use type guards or proper type definitions instead.


18. Biome Exclusion

Location: biome.json:8

The package is excluded from linting/formatting:

"rivetkit-typescript/packages/inspect/**/*",

If this is temporary for development, add a TODO comment. Otherwise, the code should follow project standards.


Testing Considerations

No tests included for the new inspector package.

Recommended tests:

  1. Shadow DOM isolation works correctly
  2. Credentials can be configured
  3. Inspector can open/close without affecting host page
  4. Symlinked components load correctly
  5. Connection error handling
  6. Router navigation works in memory mode

Documentation Gaps

  1. No documentation on how to integrate the inspector into apps
  2. No mention of browser compatibility (Shadow DOM support)
  3. No explanation of the symlink architecture
  4. Missing changelog entry

Positive Aspects ✅

  1. Good architectural choice using Shadow DOM for isolation
  2. Smart reuse of frontend components via symlinks reduces duplication
  3. Collapsible UI provides good UX
  4. Connection status display helps with debugging
  5. Type safety with TypeScript throughout
  6. Proper use of React 19 and modern patterns

Action Items

Must Fix Before Merge:

  1. ✅ Fix JSX merge conflict in actors-actor-details.tsx:211-238
  2. ✅ Remove duplicate useConnectionDetails function
  3. ✅ Remove hardcoded credentials or make configurable
  4. ✅ Fix undefined setIsOpen in _context/index.tsx

Should Fix:
5. Move build tools to devDependencies
6. Add format to Vite lib config
7. Verify ScrollArea import
8. Add error boundaries
9. Add accessibility attributes
10. Document symlink requirements

Nice to Have:
11. Replace template README
12. Add tests
13. Reduce CSS specificity
14. Extract magic numbers to constants


Recommendation

❌ Request Changes - Critical issues must be resolved before merging.

The concept and architecture are solid, but there are several blocking technical issues (merge conflict, duplicate declarations, undefined variables) that will prevent the code from building. Once these are fixed, this will be a valuable addition to the devtools ecosystem.

@jog1t
Copy link
Contributor Author

jog1t commented Dec 4, 2025

Closing in favor of #3589

@jog1t jog1t closed this Dec 4, 2025
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.

2 participants