Skip to content

Conversation

@graphite-app
Copy link
Contributor

@graphite-app graphite-app bot commented Dec 3, 2025

This draft PR was created by the Graphite merge queue.
Trunk will be fast forwarded to the HEAD of this PR when CI passes, and the original PRs will be closed.

The following PRs are included in this draft PR:

@vercel
Copy link

vercel bot commented Dec 3, 2025

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

Project Deployment Preview Comments Updated (UTC)
rivet-inspector Ready Ready Preview Comment Dec 3, 2025 8:10am
3 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
rivet-cloud Ignored Ignored Dec 3, 2025 8:10am
rivet-site Ignored Ignored Preview Dec 3, 2025 8:10am
rivetkit-serverless Skipped Skipped Dec 3, 2025 8:10am

@graphite-app graphite-app bot closed this Dec 3, 2025
@vercel vercel bot temporarily deployed to Production – rivetkit-serverless December 3, 2025 08:08 Inactive
@graphite-app graphite-app bot deleted the gtmq_spec_a0e79b_1764749257337-11fd350f-a865-4890-b56b-03e9f9e628a7 branch December 3, 2025 08:08
@claude
Copy link

claude bot commented Dec 3, 2025

Pull Request Review

This PR consolidates three changes related to frontend improvements for API token management and environment configuration. Overall, the code quality is good, but I've identified several issues that should be addressed.


Critical Issues

1. Date Handling Bug in convertDurationToExpiresAt

Location: frontend/src/app/dialogs/create-api-token-frame.tsx:20-51

The function mutates the Date object directly, which can lead to incorrect calculations across time boundaries (DST, month boundaries, leap years). The current implementation using setMinutes, setHours, etc. is problematic.

Recommendation: Use milliseconds-based calculations or a date library like date-fns for more accurate date arithmetic, especially for year calculations.

2. Missing Error Handling in Token Creation

Location: frontend/src/app/dialogs/create-api-token-frame.tsx:99-105

The form submission doesn't handle API errors. If the token creation fails, users won't see an error message.

Recommendation: Add try-catch with user-facing error messages or toast notifications.

3. Sensitive Token Exposure Risk

Location: frontend/src/app/dialogs/create-api-token-frame.tsx:82

Ensure the token is cleared from memory/state when the dialog closes to minimize exposure.


Medium Priority Issues

4. Inconsistent Indentation

Location: frontend/src/app/dialogs/api-tokens-frame.tsx:37-38

There's an extra indentation level on line 38.

5. Missing Input Validation

Location: frontend/src/app/forms/create-api-token-form.tsx:23

The expiresIn field is optional but doesn't validate the duration format. Add regex validation.

6. Revoke Action Without Confirmation

Location: frontend/src/app/dialogs/api-tokens-frame.tsx:156

Revoking a token is a destructive action that should require confirmation dialog.

7. Hardcoded Clerk Key in .env

Location: frontend/.env:4

The Clerk publishable key is committed to the repository. Ensure production keys are never committed.


Low Priority Issues

8. Magic String cloud_api_

Location: frontend/src/app/dialogs/api-tokens-frame.tsx:145

Consider extracting this prefix to a constant for consistency.

9. Accessibility: Table Width Prop

Location: frontend/src/app/dialogs/api-tokens-frame.tsx:65

The prop w="min" is non-standard. Verify this is supported by your Table component.

10. Timezone Display Issue

Location: frontend/src/app/dialogs/api-tokens-frame.tsx:135-138

toLocaleDateString() displays in the user's timezone without showing which timezone. Consider adding time information too.


Positive Observations

✅ Good separation of concerns with dedicated form components
✅ Proper React Query usage with query invalidation on mutations
✅ Good UX showing token only once after creation
✅ Consistent styling with existing components
✅ Loading states properly handled with skeletons
✅ Type safety with proper TypeScript usage


Security Considerations

  1. Token Display: The one-time token display is implemented correctly
  2. Revoked Tokens: Good visual indication of revoked tokens
  3. API Tokens vs Engine Tokens: Clear separation and documentation

Performance Considerations

  1. Query Refetch: The refetchInterval: 5000 in publishable-token-code-group.tsx:21 seems aggressive for runner configs. Consider increasing the interval.

  2. Token List: For large token lists, consider pagination rather than loading all tokens at once.


Testing Recommendations

  1. Test date conversion edge cases (DST transitions, leap years, month boundaries)
  2. Test error scenarios (API failures, network errors)
  3. Test token revocation flow with confirmation
  4. Test form validation with invalid inputs
  5. Verify token is only shown once and cannot be retrieved again

Summary

The implementation is solid overall with good patterns and user experience. The main concerns are:

  1. Fix the date calculation bug (critical)
  2. Add error handling for token creation (critical)
  3. Add confirmation dialog for token revocation (medium)
  4. Improve validation for duration format (medium)

All other issues are minor and can be addressed in follow-up PRs if time is constrained.

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