fix(fe): empty token it is a valid token

[jog1t]

No description provided.

[vercel]

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

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-inspector	Ready	Preview	Comment	Oct 27, 2025 11:27pm

2 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Oct 27, 2025 11:27pm
rivet-site	Ignored	Preview		Oct 27, 2025 11:27pm

[jog1t]

• fix(fe): redirect engine users to connect page when there's no actors #3269 : 2 dependent PRs (#3270 , #3271 )
• fix(fe): empty token it is a valid token #3268 👈 (View in Graphite)
• main

---

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]

PR Review: fix(fe): empty token it is a valid token

Summary

This PR removes the conditional check that prevented creating an actorContext when engineToken is falsy (including empty string). The change allows the actor inspector to function even when the engine token is an empty string.

Analysis

What Changed

Before:

• actorContext was only created if engineToken was truthy
• An empty string ("") would result in actorContext being null
• This triggered the error UI at line 274-290 about missing engine token

After:

• actorContext is always created, regardless of engineToken value
• The engineToken parameter (which can be undefined or "") is still passed to createInspectorActorContext

Why This Works

Looking at createInspectorActorContext in frontend/src/queries/actor-inspector.ts:30:

• engineToken is optional (engineToken?: string)
• It's conditionally added to headers: ...(engineToken ? { "x-rivet-token": engineToken } : {})
• An empty string is falsy, so the header is omitted when engineToken is ""
• This means passing an empty string has the same effect as passing undefined

Code Quality Assessment

✅ Positive:

1. Logic Simplification: Removes unnecessary conditional logic
2. Correct Behavior: Empty string is semantically valid when the token is optional
3. Proper Fallback: The downstream function handles missing tokens correctly
4. User Experience: Removes incorrect error message for valid scenarios

⚠️ Potential Concerns:

1. Unreachable Code: The null check at line 274 becomes unreachable after this change:

if (!actorContext) {
  // This will never execute now since actorContext is always created
  return (
    <InspectorGuardContext.Provider value={...}>
      ...
    </InspectorGuardContext.Provider>
  );
}

Recommendation: This dead code should be removed in a follow-up PR or this PR to keep the codebase clean.

2. Type Clarity: Consider making the return type explicit to indicate it never returns null:

const actorContext = useMemo((): ActorContext => {
  return createInspectorActorContext({...});
}, [...]);

Testing Considerations

• ✅ Verify actor inspector works with empty engine token
• ✅ Verify actor inspector still works with valid engine token
• ✅ Test both cloud and engine app types

Security Assessment

✅ No security concerns. The change properly handles optional authentication tokens.

Performance

✅ No performance impact. Minor improvement by removing unnecessary conditional.

Verdict

APPROVE with suggestion: The change is correct and improves the code. However, the dead code at lines 274-290 should be cleaned up since actorContext can never be null after this change.

---

Review generated with Claude Code

[pkg-pr-new]

More templates

• example-ai-agent
• example-background-jobs
• example-better-auth-external-db
• example-bots
• chat-room
• example-cloudflare-workers
• example-cloudflare-workers-hono
• example-counter
• example-counter-serverless
• example-crdt
• example-cursors
• example-cursors-raw-websocket
• example-database
• example-deno
• example-sqlite
• example-elysia
• example-express
• example-freestyle
• example-game
• example-hono
• example-hono-bun
• example-hono-react
• example-kitchen-sink
• example-next-js
• example-rate
• example-raw-fetch-handler
• example-raw-websocket-handler
• example-raw-websocket-handler-proxy
• example-react
• example-smoke-test
• starter
• example-stream
• example-sync
• example-tenant
• example-trpc
• example-workflows

@rivetkit/actor

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

@rivetkit/cloudflare-workers

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

@rivetkit/core

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: 03bc5d3

[graphite-app]

Graphite Automations

"Test" took an action on this PR • (10/26/25)

1 assignee was added to this PR based on Kacper Wojciechowski's automation.

[jog1t]

Merge activity

• Oct 27, 10:35 PM UTC: The merge label 'merge-queue' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.

[claude]

Code Review

Summary

This PR fixes a logic issue where the actor context was being set to null when engineToken was falsy (empty string, undefined, etc.). The change ensures createInspectorActorContext() is always called, which is the correct behavior since engineToken is an optional parameter.

Analysis

The Problem:
The previous code had a conditional that would return null for actorContext when engineToken was falsy:

return engineToken
  ? createInspectorActorContext({ ... })
  : null;

This caused issues because:

1. engineToken is marked as optional in createInspectorActorContext (line 14 of actor-inspector.ts: engineToken?: string)
2. An empty string ("") is a valid value - it's used as the default in local storage (line 222)
3. The downstream code at lines 274-290 has error handling specifically for when actorContext is null, showing an error message about missing engine token

The Fix:
The PR correctly removes the conditional check and always creates the context, allowing engineToken to be undefined or an empty string, which aligns with the function signature.

Review Findings

✅ Positive Aspects

1. Correct fix - The change properly handles the optional engineToken parameter
2. Maintains type safety - The engineToken parameter correctly uses the optional ?: syntax
3. No breaking changes - The function signature already supported this use case
4. Preserves error handling - The existing null check at line 274 remains for other error scenarios

⚠️ Potential Issues

1. Dead Code Detection
The null check at lines 274-290 in ActorEngineProvider may now be unreachable:

if (!actorContext) {
  return (
    <InspectorGuardContext.Provider
      value={
        <Info>
          <p>Unable to connect to the Actor's Inspector.</p>
          <p>
            Your namespace is missing an engine token. Please
            report this to Rivet support.
          </p>
        </Info>
      }
    >
      {children}
    </InspectorGuardContext.Provider>
  );
}

Since actorContext is now always assigned a value, this condition will never be true. Consider:

• Removing this dead code block (lines 274-290)
• Or updating the return type of useActorEngineContext to reflect that actorContext is no longer nullable

2. Type Consistency
The function return type should be updated to reflect that actorContext is now always defined:

// Current (implicit):
return { actorContext, actor, runner, isLoading };
// actorContext type: ActorContext | null

// Should be:
return { actorContext, actor, runner, isLoading };
// actorContext type: ActorContext

3. Missing Dependency in useMemo
At line 248, the useMemo dependency array should include actorId or the dependencies are incomplete:

}, [actor?.runner, provider.runnerByNameQueryOptions, engineToken]);

The queryClient.fetchQuery call inside the memoized function uses actor?.runner, but actor itself is not in the dependency array. This could lead to stale closures. Consider adding actorId to ensure proper re-computation.

Recommendations

1. Remove dead code - Clean up the now-unreachable null check at frontend/src/components/actors/guard-connectable-inspector.tsx:274-290
2. Update types - Make the return type explicit to show actorContext is non-nullable
3. Fix dependencies - Add missing dependencies to the useMemo hook or use actor?.runner directly in the dependency array

Test Coverage

Consider adding tests for:

• Actor inspector connection with an empty string engineToken
• Actor inspector connection with undefined engineToken
• Verify the token header is correctly omitted when engineToken is falsy (per line 30 of actor-inspector.ts)

Verdict

✅ APPROVE with suggestions

The core fix is correct and addresses a legitimate bug. The suggestions above are for cleanup and consistency rather than blocking issues. The change improves the system's robustness by properly handling optional engine tokens.

---

Review generated by Claude Code

[claude]

Final Code Review

Summary

This PR correctly fixes a bug where the actor inspector failed to work when engineToken was an empty string. The fix aligns with the function signature where engineToken is optional.

Key Findings

✅ The Fix is Correct

• engineToken is marked as optional in createInspectorActorContext (line 14 of actor-inspector.ts)
• Empty string is a valid value (used as default in local storage at line 222)
• The conditional removal allows proper handling of optional authentication

⚠️ Follow-up Cleanup Needed

1. Dead Code at lines 274-290 - The null check for actorContext is now unreachable and should be removed:

   if (!actorContext) {
     // This block will never execute now
     return ( ... );
   }

2. Dependency Array Issue (line 248) - The useMemo hook is missing actorId in its dependency array. The current fix adds it, which is correct, but actor might also be needed since actor?.runner is used inside the memoized function.

Recommendations

1. Remove the unreachable code block at lines 274-290 in this PR or a follow-up
2. Verify the useMemo dependencies are complete - consider if actor should be included

Verdict

LGTM ✅ - The core fix is solid and addresses a real bug. The suggestions above are for code cleanliness and can be addressed in follow-up PRs if preferred.

---

Reviewed with Claude Code