fix: make inspector working again

[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 21, 2025 6:54pm

2 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Oct 21, 2025 6:54pm
rivet-site	Ignored	Preview		Oct 21, 2025 6:54pm

[jog1t]

• fix: make inspector working again #3229 👈 (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.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/rivet-dev/engine/@rivetkit/engine-runner@3229

npm i https://pkg.pr.new/rivet-dev/engine/@rivetkit/engine-runner-protocol@3229

commit: ce7ecee

[claude]

PR Review: Fix Inspector Working Again

Overview

This PR fixes the actor inspector functionality by properly handling authentication tokens and improving the REPL worker implementation. The changes span across the frontend data providers, actor worker logic, and dependency management.

Code Quality & Best Practices ✅

Strengths:

• Good use of TypeScript patterns with ts-pattern for type-safe matching
• Proper async/await handling throughout
• Clean separation of concerns between worker and container logic
• Good use of structuredClone for deep cloning init data (actor-repl.worker.ts:117)
• Proper error handling with try-catch blocks

Minor Improvements:

1. actor-worker-context.tsx:42 - Type assertion as string could be improved with proper type narrowing:

   return (data?.metadata?.inspectorToken as string) || "";
   // Consider: return typeof data?.metadata?.inspectorToken === 'string' ? data.metadata.inspectorToken : '';

2. actor-editable-state.tsx:87 - Good use of mutateAsync instead of mutate for proper awaiting, though you could add error handling:

   onClick={async () => {
     try {
       await mutateAsync(JSON.parse(value || ""));
       setIsEditing(false);
       setValue(null);
     } catch (error) {
       // Consider showing error feedback to user
     }
   }}

Potential Bugs & Issues ⚠️

1. Token Function Resolution (actor-worker-container.ts:144-147)

   • The code properly handles function tokens by awaiting them, but there's no error handling if the token function throws:

   engineToken:
     typeof this.#meta?.engineToken === "function"
       ? await this.#meta?.engineToken()
       : this.#meta?.engineToken,

   • Recommendation: Wrap in try-catch to handle token retrieval failures gracefully.

2. Empty Runner Name (actor-worker-context.tsx:68)

   • useInspectorToken(runner || "") passes an empty string when runner is undefined/null
   • This could lead to unnecessary query execution with invalid parameters
   • Recommendation: Consider early return or conditional execution when runner is falsy.

3. Error Response Handling (actor-repl.worker.ts:210-216)

   • The error handling tries JSON first, then falls back to text, which is good
   • However, the successful response assumes .result exists without validation
   • Recommendation: Add validation or optional chaining: return (await response.json())?.result;

4. Plain SDK Guard (waitForClerk.ts:38)

   • Good addition of typeof Plain !== "undefined" check
   • However, the optional chaining Plain?.setCustomerDetails is redundant after the typeof check
   • Recommendation: Either use typeof check OR optional chaining, not both.

Performance Considerations 🚀

Positive Changes:

1. Token caching by extracting to a function reduces redundant code (cloud-data-provider.tsx:342-348)
2. Proper dependency array in useEffect prevents unnecessary worker re-initialization
3. Use of useSyncExternalStore for efficient subscription management

Concerns:

1. actor-worker-context.tsx:108-118 - Large dependency array could cause frequent re-renders

   • Consider using useRef for stable values or memoizing token functions
   • The engineToken function recreation on every render could be optimized

2. words.ts - Massive reduction from ~1300 lines to ~150 lines (great!)

   • Simplification is excellent for bundle size and performance

Security Concerns 🔒

1. Token Handling - Generally good practices:

   • Tokens are passed through headers correctly
   • Authorization header properly formatted with Bearer scheme
   • Token functions allow for dynamic token retrieval (good for expiration handling)

2. X-Rivet-Token Header (actor-repl.worker.ts:199)

   • Custom header for engine tokens
   • Ensure this is properly sanitized on the backend

3. REPL Code Execution (actor-repl.worker.ts:73-81)

   • Creating functions from user input using new Function()
   • This is properly isolated in a Web Worker ✅
   • Window object is stubbed out with empty object {} ✅
   • "use strict" mode enabled ✅
   • Good security posture, but ensure backend validates action calls

4. Actor Inspector Token - New token type introduced

   • Metadata field inspectorToken retrieved from runner (actor-worker-context.tsx:42)
   • Should verify this field is properly secured on the backend

Test Coverage 🧪

Observations:

• No test files modified in this PR
• Changes involve complex async flows and worker communication
• Recommendation: Consider adding tests for:
  • Token function resolution (both sync and async)
  • Worker initialization and message passing
  • Error handling paths (network failures, invalid tokens)
  • Empty/undefined runner name handling

Additional Notes 📝

1. Breaking Change in Response Format (actor.ts:61-67)

   • Response now returns { state, enabled } instead of just the state
   • Good that it's handled consistently in onSuccess
   • Ensure all consumers expect this format

2. Dependency Changes

   • Removed vendor packages (rivetkit-actor.tgz, rivetkit-core.tgz)
   • Switched to npm registry packages (good for maintainability)
   • pnpm-lock.yaml shows clean dependency resolution

3. Unused Import Removed (cloud-data-provider.tsx:3)

   • FetchFunction import removed but not used - good cleanup

Summary

Overall Assessment: Good PR with minor improvements needed ✅

The changes successfully fix the inspector functionality with solid architecture. Main concerns are around:

1. Error handling for token resolution
2. Type safety improvements
3. Adding test coverage for critical paths

Recommended Actions:

1. Add error handling around token function calls
2. Consider adding unit tests for worker communication
3. Validate backend security for inspector tokens
4. Minor refactoring for type safety (avoid as string assertions)

The PR is in good shape and demonstrates solid understanding of async patterns, security considerations, and React best practices. The reduction in the words.ts file is particularly nice for bundle size!

[claude]

PR Review: Fix Inspector Functionality

Overview

This PR fixes the inspector functionality by properly handling authentication tokens and inspector credentials across different app types (engine vs inspector). The changes reduce code bloat (1403 deletions vs 299 additions) while fixing critical functionality.

---

✅ Positive Aspects

1. Token Management Improvements

   • Properly handles async token resolution in actor-worker-container.ts:144-147
   • Tokens are now awaited when they're functions, preventing potential race conditions
   • Adds inspectorToken to the worker initialization flow

2. Better Type Safety

   • Added proper return type for state patch mutation in actor.ts:61-64
   • The mutation now correctly types the response with { state: unknown; enabled: boolean }

3. Code Simplification

   • Massive reduction in word list (words.ts) - from heroes to tools/adjectives, much more maintainable
   • Cleaner token handling by extracting the token function in cloud-data-provider.tsx:342-348

4. Proper Data Flow

   • Inspector token properly flows through: context → worker container → worker → API calls
   • Pattern matching with ts-pattern ensures proper token headers based on __APP_TYPE__

---

⚠️ Issues & Concerns

🔴 Critical Issues

1. Missing Runner Null Check (actor-worker-context.tsx:68)

   const inspectorToken = useInspectorToken(runner || "");

   Problem: If runner is undefined/null, this passes an empty string to useInspectorToken, which will:

   • In engine mode: make a query with runnerName: ""
   • Potentially fail silently or return incorrect data

   Recommendation: Add proper null handling:

   const inspectorToken = runner ? useInspectorToken(runner) : undefined;

2. Unsafe Type Assertion (actor-worker-context.tsx:42)

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

   Problem: Using as string when the actual type is unknown. If inspectorToken is not a string, this could cause runtime errors.

   Recommendation: Add runtime validation:

   const token = data?.metadata?.inspectorToken;
   return typeof token === 'string' ? token : "";

3. Query Made Before Data Availability (actor-worker-context.tsx:36-40)

   const { data } = useQuery(
     provider.runnerByNameQueryOptions({
       runnerName,
     }),
   );

   Problem: This query runs even when runnerName is empty string. Should be conditionally enabled.

   Recommendation:

   const { data } = useQuery({
     ...provider.runnerByNameQueryOptions({ runnerName }),
     enabled: !!runnerName,
   });

🟡 Medium Priority Issues

4. Error Handling in Worker (actor-repl.worker.ts:210-216)

   if (!response.ok) {
     try {
       return await response.json();
     } catch {
       return await response.text();
     }
   }

   Problem: On error, you're returning the error body but not throwing. This means the caller receives error data as if it were a successful result.

   Recommendation: Throw the error:

   if (!response.ok) {
     let errorBody;
     try {
       errorBody = await response.json();
     } catch {
       errorBody = await response.text();
     }
     throw new Error(\`API Error: \${errorBody}\`);
   }

5. Mutation Not Awaited (actor-editable-state.tsx:87)

   onClick={async () => {
     await mutateAsync(JSON.parse(value || ""));
     setIsEditing(false);
     setValue(null);
   }}

   Good: The mutation is now awaited! But there's no error handling.

   Recommendation: Add try-catch:

   onClick={async () => {
     try {
       await mutateAsync(JSON.parse(value || ""));
       setIsEditing(false);
       setValue(null);
     } catch (error) {
       // Handle error - show toast, keep editing mode, etc.
       console.error('Failed to update actor state:', error);
     }
   }}

6. Missing Runner Field (default-data-provider.tsx:311)

   runner: data.runner ?? undefined,

   Issue: This is added to the data transformation, but we need to verify the source data.runner actually exists in the API response. If it doesn't, this will always be undefined.

🔵 Minor Issues

7. Console.error Ordering (actor-worker-container.ts:243)

   • Moving the console.error before the if-check is good for debugging, but the message format could be improved
   • Consider: console.error("Actor Worker Error:", msg.type, msg.data);

8. Plain SDK Check (waitForClerk.ts:38)

   if (typeof Plain !== "undefined") {
     Plain?.setCustomerDetails({...});
   }

   Good defensive coding, but the ?. is redundant after the typeof check.

   Minor improvement:

   if (typeof Plain !== "undefined") {
     Plain.setCustomerDetails({...});
   }

9. Word List Changes (words.ts)

   • The new word list is much shorter and more focused
   • However, consider if the reduction from ~1300 to ~150 entries increases collision risk for generated keys
   • Assuming collision risk is acceptable, this is a huge improvement

---

🔒 Security Considerations

1. Token Exposure in Headers (actor-repl.worker.ts:186-203)

   • Tokens are properly sent via headers (good)
   • Using Authorization: Bearer pattern correctly
   • Inspector token conditionally included based on presence
   • Engine token uses custom header X-Rivet-Token
   • ✅ No obvious security issues, but ensure tokens are not logged

2. Token Storage

   • Tokens are passed through context and not stored in localStorage within this PR
   • ✅ Good practice

3. Worker Scope

   • Web Worker has limited access to DOM/APIs
   • Token is passed into worker for API calls
   • ✅ Reasonable isolation

---

🚀 Performance Considerations

1. Token Resolution - Now properly async, won't block rendering

2. useEffect Dependencies (actor-worker-context.tsx:108-118)

   • All necessary dependencies included
   • Worker will reinitialize when tokens change - this could be expensive
   • Consider: If tokens change frequently, this might cause excessive worker restarts
   • Recommendation: Add stability check if token reference changes but value doesn't

3. Query Optimizations

   • The useInspectorToken hook makes a query that could be optimized with proper enabled flag (see issue [SVC-2504] Fix 5 GB upload limit for local development from Cloudflare #3)

---

📝 Best Practices & Style

1. ✅ Imports: Properly organized at top of files
2. ✅ Logging: Uses structured logging with tracing patterns in most places
3. ✅ Error Messages: Clear and descriptive
4. ⚠️ Comments: The comment in actor-repl.worker.ts:182-184 is helpful
5. ✅ Naming: Variable names are clear and consistent

---

🧪 Test Coverage

Concern: No test files are included in this PR. Given the complexity of the token flow and worker communication, consider adding tests for:

1. Token resolution (sync vs async vs function)
2. Worker initialization with different app types
3. Inspector token retrieval with missing runner
4. Error cases in API calls

---

📋 Summary

Recommendation: 🟡 Request Changes

The PR successfully fixes the inspector functionality and improves code organization. However, there are 3 critical issues that should be addressed before merge:

1. Handle null/undefined runner properly
2. Add runtime type validation for inspectorToken
3. Enable query only when runnerName is available

The medium-priority issues (error handling in worker, mutation error handling) should also be addressed for production robustness.

Estimated Risk: Medium - Core functionality works but edge cases could cause issues

---

Great work on the token flow refactoring! The architecture is much cleaner. Once the critical issues are addressed, this will be a solid improvement. 🎉

---

Reviewed by Claude Code

[graphite-app]

Graphite Automations

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

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

[claude]

Pull Request Review: Fix Inspector Functionality

Summary

This PR fixes the actor inspector functionality by resolving authentication and data flow issues. The changes primarily focus on properly passing tokens and metadata through the data provider chain to enable inspector API calls from web workers.

Code Quality & Best Practices

✅ Strengths

1. Proper async/await handling: The mutation in actor-editable-state.tsx:87-90 now correctly uses await mutateAsync() instead of mutate(), ensuring the UI state is only updated after the mutation completes.

2. Token abstraction: Extracting the token function in cloud-data-provider.tsx:342-348 improves code reusability and reduces duplication.

3. Type safety improvements: The mutation response in actor.ts:61-64 now has explicit typing, making the contract clearer.

4. Separation of concerns: The PR properly separates inspector token handling from engine tokens using pattern matching.

⚠️ Areas for Improvement

1. Missing error handling in async setupWorker (actor-worker-container.ts:122)

   async #setupWorker(worker: Worker) {
       // ... 
       engineToken:
           typeof this.#meta?.engineToken === "function"
               ? await this.#meta?.engineToken()
               : this.#meta?.engineToken,

   Issue: If engineToken() throws, this could crash the worker setup without proper error handling.

   Recommendation: Add try-catch or handle the promise rejection:

   engineToken:
       typeof this.#meta?.engineToken === "function"
           ? await this.#meta?.engineToken().catch((e) => {
               console.error("Failed to fetch engine token:", e);
               return "";
           })
           : this.#meta?.engineToken ?? "",

2. Type inconsistency (engine-data-provider.tsx:59)

   engineToken: (() => string) | string | (() => Promise<string>);

   Issue: Having three different types for the same field makes the code harder to reason about and could lead to runtime errors.

   Recommendation: Normalize to always use () => Promise<string> for consistency, or use a discriminated union.

3. Unsafe type assertion (actor-worker-context.tsx:42)

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

   Issue: Type assertion bypasses type safety. If inspectorToken is not a string, this could cause issues.

   Recommendation: Use type guard or validate the type:

   const token = data?.metadata?.inspectorToken;
   return typeof token === "string" ? token : "";

Potential Bugs

1. Race condition in useInspectorToken (actor-worker-context.tsx:36-40)

   const { data } = useQuery(
       provider.runnerByNameQueryOptions({
           runnerName,
       }),
   );

   Issue: If runnerName is empty string, this will still execute the query. The hook is called with runner || "" on line 68.

   Recommendation: Add enabled flag:

   const { data } = useQuery({
       ...provider.runnerByNameQueryOptions({ runnerName }),
       enabled: !!runnerName,
   });

2. Missing null checks in worker (actor-repl.worker.ts:177-218)
   The callAction function doesn't validate init.endpoint exists before using it in new URL().

   Recommendation: Add validation:

   if (!init?.endpoint) throw new Error("Endpoint not configured");

Performance Considerations

1. Word list optimization (words.ts) - ✅ Excellent improvement!

   • Reduced from 1,403 lines to 299 lines (~79% reduction)
   • Changed from superhero names to simpler adjective-tool combinations
   • This will reduce bundle size and improve load times

2. Unnecessary re-renders (actor-worker-context.tsx:108-118)
   The dependency array is quite large. Consider using useMemo for compound dependencies or reducing the number of reactive values.

Security Concerns

1. Token exposure in headers (actor-repl.worker.ts:186-203)

   headers: {
       Authorization: init.inspectorToken ? `Bearer ${init.inspectorToken}` : "",
       "X-Rivet-Token": init.engineToken ? init.engineToken : "",

   ✅ Good: Tokens are properly set in Authorization headers
   ⚠️ Concern: Ensure these tokens have appropriate scopes and are not over-privileged

   Recommendation: Document the required token scopes/permissions in code comments

2. Error message leakage (actor-repl.worker.ts:210-215)

   if (!response.ok) {
       try {
           return await response.json();
       } catch {
           return await response.text();
       }
   }

   Issue: Returning raw error responses to the client could leak sensitive information

   Recommendation: Sanitize error messages before returning to the client

Test Coverage

❌ Missing: This PR doesn't include any test changes or additions.

Recommendations:

1. Add unit tests for the new token handling logic
2. Add integration tests for the inspector worker flow
3. Test the async token resolution paths
4. Test error cases (missing tokens, invalid tokens, network failures)

Additional Observations

1. Plain SDK optional check (waitForClerk.ts:38-42) - ✅ Good defensive programming

   if (typeof Plain !== "undefined") {
       Plain?.setCustomerDetails({...});
   }

2. Deleted vendor files - Files frontend/vendor/rivetkit-actor.tgz and rivetkit-core.tgz were removed. Ensure these are now properly sourced from npm/package registry.

3. Railway env vars reordering (connect-railway-frame.tsx:179-182) - Minor change to reorder environment variables. Ensure documentation is updated if this order matters.

Summary Recommendations

Critical

• Add error handling for async engineToken() resolution
• Add validation for init.endpoint before creating URL
• Add enabled flag to useInspectorToken query

Important

• Fix type assertion for inspectorToken
• Add test coverage for authentication flow
• Document token scope requirements

Nice to Have

• Normalize engineToken type to single async function type
• Optimize dependency array in worker context
• Add error message sanitization

Verdict

This is a solid fix that addresses the core inspector issues. The code quality is generally good with proper async handling and token management. However, there are some error handling gaps and potential race conditions that should be addressed before merging to production.

Recommendation: Approve with requested changes for error handling and null checks.

---

Review generated with Claude Code

[claude]

Code Review - PR #3229: fix: make inspector working again

Overview

This PR fixes the inspector functionality by updating token handling, authentication flow, and improving the actor worker/REPL integration. The changes primarily focus on frontend TypeScript code related to actor inspection and data providers.

---

✅ Strengths

1. Improved Token Management: The refactoring to pass engineToken as a function (supporting both sync and async) is more flexible and allows for dynamic token fetching.

2. Better Type Safety: Added proper type definitions for response structures (e.g., actor.ts:61-64).

3. Cleaner Code Organization: Extracting the token function in cloud-data-provider.tsx:342-348 improves readability and reusability.

4. Bug Fix for Plain Integration: The defensive check for Plain in waitForClerk.ts:38 prevents runtime errors when Plain is not defined.

---

🔍 Issues & Recommendations

1. Security Concern: Token Handling in Worker (HIGH PRIORITY)

Location: actor-repl.worker.ts:177-219 and actor-worker-container.ts:122-152

The worker receives and stores the raw token value after resolving it:

engineToken:
    typeof this.#meta?.engineToken === "function"
        ? await this.#meta?.engineToken()
        : this.#meta?.engineToken,

Issues:

• The token is serialized and sent to the web worker via postMessage, which stores it in the worker context
• If the token expires or rotates, the worker will continue using the stale token
• No refresh mechanism exists for tokens in the worker

Recommendation:

• Consider passing a message-based token fetching mechanism where the worker requests a fresh token from the main thread when needed, rather than storing it
• Alternatively, implement token refresh logic with expiration checking

---

2. Error Handling Issues

Location: actor-repl.worker.ts:210-216

if (!response.ok) {
    try {
        return await response.json();
    } catch {
        return await response.text();
    }
}

Issues:

• Returns error response data directly without throwing or proper error signaling
• The calling code may not distinguish between successful and error responses
• Silent catch block with no logging

Recommendation:

if (!response.ok) {
    let errorMsg: string;
    try {
        const errorData = await response.json();
        errorMsg = errorData.message || JSON.stringify(errorData);
    } catch {
        errorMsg = await response.text();
    }
    throw new Error(`Action failed: ${errorMsg}`);
}

---

3. Race Condition in Actor Initialization

Location: actor-worker-container.ts:80-82

if (this.#meta.actorId !== meta.actorId) {
    // if not, we don't need to do anything
    return null;
}

Issue: This check happens after async operations, but this.#meta could be modified by another concurrent init() call.

Recommendation:

• Use a unique initialization ID or AbortController-based cancellation
• Consider making init() reject concurrent calls explicitly

---

4. Missing Null/Undefined Checks

Location: actor-worker-context.tsx:68 and default-data-provider.tsx:310

In actor-worker-context.tsx:68, the code uses:

const inspectorToken = useInspectorToken(runner || "");

Issue: If runner is undefined, it passes empty string which triggers the query unnecessarily.

Recommendation:

const inspectorToken = useInspectorToken(runner);
// And update useInspectorToken to handle undefined:
const useInspectorToken = (runnerName: string | undefined) => {
    return match(__APP_TYPE__)
        .with("inspector", () => {
            return useInspectorCredentials().credentials?.token;
        })
        .otherwise(() => {
            const provider = useEngineCompatDataProvider();
            const { data } = useQuery({
                ...provider.runnerByNameQueryOptions({ runnerName }),
                enabled: !!runnerName, // Add this
            });
            return (data?.metadata?.inspectorToken as string) || "";
        });
};

---

5. Type Safety Issue

Location: actor-worker-context.tsx:42

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

Issue: Using type assertion without validation. If inspectorToken is not a string, this could cause runtime issues.

Recommendation:

return typeof data?.metadata?.inspectorToken === 'string' 
    ? data.metadata.inspectorToken 
    : "";

---

6. Async Function Not Awaited

Location: actor-worker-container.ts:122

async #setupWorker(worker: Worker) {

This function is async but the await happens inside for engineToken. The function is called without await in actor-worker-container.ts:88:

this.#setupWorker(worker);

Issue: Potential race condition where initialization may not complete before worker is used.

Recommendation: Either await the #setupWorker call or make it synchronous and handle async token fetching differently.

---

7. Missing Error Logging

Location: actor-worker-container.ts:92

} catch (e) {
    console.log(e);  // Should be console.error

Recommendation: Use console.error for errors, or better yet, use structured logging if available.

---

8. Console Error Moved But Information Lost

Location: actor-worker-container.ts:243

The error logging was moved but the context message was removed:

console.error("Actor Worker Error", msg);
if (!msg.id) {
    this.#state.status = { type: "error", error: msg.data };
    // Old code had: console.error("Actor Worker Error", msg.data);

Recommendation: Keep detailed error logging for debugging purposes.

---

📝 Minor Issues

1. Inconsistent Error Message Casing (actor.ts:34): Changed from async call pattern but mutation is now awaited in component. Consider documenting this pattern change.

2. Unused Import: actor-queries.ts:3 removes applyPatch import but compare is still used - good cleanup.

3. Words File Simplification: The massive reduction in words.ts from hero names to tool names is a good simplification, though it's unclear if this is intentional or related to the inspector fix.

---

🧪 Test Coverage Concerns

No test files are included in this PR. Consider adding tests for:

• Token refresh/expiration scenarios
• Worker initialization race conditions
• Error handling paths in the REPL worker
• Inspector token fetching for different app types

---

✨ Suggestions for Future Improvements

1. Token Refresh Strategy: Implement automatic token refresh before expiration
2. Worker Lifecycle Management: Add explicit state machine for worker states
3. Structured Logging: Replace console.log/error with proper logging framework
4. Error Boundaries: Add React error boundaries around actor worker components

---

Summary

The PR successfully fixes the inspector functionality with good architectural improvements in token handling. However, there are several security, error handling, and race condition concerns that should be addressed before merging.

Recommendation: Request changes to address the high-priority security concern around token handling in workers and the error handling issues. The other issues can be addressed as follow-up improvements.

Priority Fixes Needed:

1. ⚠️ Token refresh mechanism in workers
2. ⚠️ Proper error handling in callAction
3. ⚠️ Race condition protection in init()
4. Missing null checks for runner-based token fetching

🤖 Generated with Claude Code

[claude]

PR Review: fix: make inspector working again

Summary

This PR fixes the inspector functionality with 18 files changed (+299/-1403 lines). The changes primarily focus on fixing authentication and token handling for the actor inspector, along with a significant simplification of the word list.

Positives ✅

1. Good separation of concerns: The token extraction in cloud-data-provider.tsx:342-350 properly separates the async token function for reusability
2. Proper error handling: The worker error handling in actor-worker-container.ts:242-248 correctly logs errors before state updates
3. Type safety improvements: Added proper return type for useActorStatePatchMutation in actor.ts:61-64
4. Defensive programming: Added typeof Plain !== "undefined" check in waitForClerk.ts:38 to prevent runtime errors
5. Code cleanup: Massive simplification of word lists (1292 → 151 entries) makes the code more maintainable

Issues & Concerns 🔴

Critical Issues

1. Async/await inconsistency in actor-worker-context.tsx:122

   async #setupWorker(worker: Worker) {
       // ... async operations
       this.#worker.postMessage({
           engineToken: typeof this.#meta?.engineToken === "function"
               ? await this.#meta?.engineToken()
               : this.#meta?.engineToken,

   The method is async and uses await, but the caller at line 88 doesn't await it: this.#setupWorker(worker);. This could cause race conditions where the worker isn't fully initialized before being used.

2. Potential race condition in useInspectorToken (actor-worker-context.tsx:29-44)

   const useInspectorToken = (runnerName: string) => {
       return match(__APP_TYPE__)
           .otherwise(() => {
               const { data } = useQuery(
                   provider.runnerByNameQueryOptions({ runnerName }),
               );
               return (data?.metadata?.inspectorToken as string) || "";
           });
   };

   The query might not have completed when this value is used. No loading state is checked before accessing data.

3. Type casting without validation (actor-worker-context.tsx:42)

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

   Type assertion without runtime validation. If inspectorToken is not a string, this could cause issues downstream.

Medium Priority Issues

4. Missing error boundary in actor-repl.worker.ts:210-216

   if (!response.ok) {
       try {
           return await response.json();
       } catch {
           return await response.text();
       }
   }

   The error return doesn't distinguish between JSON parse failures and actual API errors. Consider logging or handling these differently.

5. Hardcoded authentication header logic (actor-repl.worker.ts:186-202)
   The authentication logic with match(__APP_TYPE__) pattern is duplicated across multiple files. Consider extracting to a shared utility function.

6. Incomplete useEffect dependencies
   In actor-worker-context.tsx:87-118, the effect depends on engineToken which could be a function. If the function reference changes, it will trigger re-initialization unnecessarily.

Minor Issues

7. console.log in production code (actor-worker-container.ts:92)

   } catch (e) {
       console.log(e);

   Should use proper logging framework or at least console.error.

8. Mutation response handling (actor-editable-state.tsx:87)

   await mutateAsync(JSON.parse(value || ""));

   The mutation result is not checked. If it fails, setIsEditing(false) still executes.

9. Missing null/empty string validation
   In actor-worker-context.tsx:68: const inspectorToken = useInspectorToken(runner || "");
   Passing empty string to useInspectorToken when runner is falsy could trigger unnecessary queries.

Security Considerations 🔒

1. Token handling: Tokens are now properly passed through headers rather than query strings ✅
2. Authorization header construction: Properly uses Bearer token format ✅
3. Consider: Add token expiration/refresh handling if not already present elsewhere

Performance Considerations ⚡

1. Positive: Word list reduction significantly reduces bundle size
2. Concern: The useInspectorToken hook could trigger queries even when not needed (when runner is empty)
3. Concern: structuredClone(data) in actor-repl.worker.ts:117 creates deep copies - ensure this is necessary for the data size

Testing & Coverage 🧪

⚠️ No test files modified or added

Given the complexity of authentication and worker initialization changes, I recommend adding:

• Unit tests for token resolution logic
• Integration tests for worker initialization flow
• Tests for error scenarios (invalid tokens, failed queries, etc.)

Recommendations 📋

1. High Priority: Fix the async/await issue in #setupWorker ([SVC-2555] Set up issue templates #1)
2. High Priority: Add loading/error state checks for useInspectorToken ([SVC-2479] Send cluster events to PostHog #2)
3. Medium Priority: Extract authentication header logic to shared utility
4. Medium Priority: Add proper error handling for mutation failures
5. Low Priority: Replace console.log with proper logging
6. Add tests: Cover the authentication flow and worker initialization

Additional Notes

• The PR title says "make inspector working again" - consider documenting what broke it initially in the PR description for future reference
• The vendor package updates (rivetkit-actor.tgz, rivetkit-core.tgz removal) suggest dependency management changes - ensure these are intentional

---

Overall, this PR addresses important functionality issues but has some async/await and error handling gaps that should be addressed before merging.

🤖 Generated with Claude Code

[graphite-app]

Merge activity

• Oct 22, 6:58 AM UTC: This pull request can not be added to the Graphite merge queue. Please try rebasing and resubmitting to merge when ready.
• Oct 22, 6:58 AM UTC: Graphite disabled "merge when ready" on this PR due to: a merge conflict with the target branch; resolve the conflict and try again..