fix-auth-not-using-refresh-token

[Makisuo]

No description provided.

[vercel]

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

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
hazel-docs	Skipped		Feb 16, 2026 6:40pm

[claude]

Claude finished @Makisuo's task —— View job

---

PR Review: Authentication Refactoring

I've completed a comprehensive review of this authentication refactoring PR. This is a well-architected consolidation that addresses the original issue of not properly using refresh tokens.

Review Tasks

• Analyze the new auth-token.ts implementation
• Review changes to auth atoms and middleware
• Check test coverage and quality
• Identify security considerations
• Assess performance implications
• Provide constructive feedback

Key Changes Overview

The PR consolidates authentication logic into a new unified auth-token.ts module (+296 lines) that handles token operations for both web and desktop platforms. Major refactoring includes:

• New centralized auth module: apps/web/src/lib/auth-token.ts
• Simplified auth atoms: Reduced from ~397 to ~41 lines in desktop-auth.ts, ~366 to ~11 lines in web-auth.ts
• Unified auth-fetch: Streamlined from ~78 to ~35 lines using the new auth-token module
• Updated tests: Focused test coverage for the new architecture

---

✅ Strengths

1. Excellent Architecture & Consolidation

• Single source of truth: All token operations now centralized in auth-token.ts:77-297
• Platform abstraction: Seamlessly handles both Tauri and web storage via helper functions auth-token.ts:72-109
• Proper separation of concerns: Auth atoms now focus on UI state, while core logic is in the unified module

2. Robust Error Handling

• Smart error classification: isFatalRefreshError() and isTransientError() properly categorize errors auth-token.ts:44-66
• Exponential backoff: Implements proper retry logic with 1s, 2s, 4s intervals auth-token.ts:222-229
• Graceful failure modes: Clear token cleanup and event dispatching on auth failure

3. Strong Concurrency Management

• Race condition prevention: Uses Ref and Deferred to coordinate concurrent refresh attempts auth-token.ts:29-31
• Single refresh guarantee: Multiple callers wait for in-progress refresh rather than triggering concurrent refreshes auth-token.ts:147-156

4. Effect-TS Best Practices

• Proper service dependencies: Uses dependencies array in services (following CLAUDE.md guidelines)
• Effect-first design: Core operations are Effects with Promise wrappers for React integration
• Comprehensive error types: Uses Schema.TaggedError for structured error handling

---

⚠️ Areas for Improvement

1. Security Considerations

Token Logging Exposure

// auth-token.ts:200, 211, etc.
yield* Effect.log(`[auth-token:${tag}] forceRefresh: tokens refreshed successfully`)
console.error(`[auth-token:${tag}] Fatal token refresh error:`, error)

Issue: Tokens might be included in error objects or debug logs, potentially exposing sensitive data.
Recommendation: Sanitize error logging to exclude token values.

Window Event Security

// auth-token.ts:217, 242
window.dispatchEvent(new CustomEvent("auth:session-expired"))

Issue: Global event dispatching could be intercepted by malicious scripts.
Recommendation: Consider using a more secure internal event system or validate event handlers.

2. Performance & UX Concerns

Redundant Storage Calls

// auth-fetch.ts:82-83
const newToken = await getAccessToken()
if (newToken) {

Issue: After successful refresh, immediately calls getAccessToken() again rather than using the returned token.
Recommendation: forceRefresh() should return the new token to avoid unnecessary storage reads.

Missing Request Queuing

When a refresh is in progress, new requests wait but don't queue - they make the request immediately after refresh completes. High-frequency API calls during refresh could create a thundering herd.
Recommendation: Consider implementing request queuing during refresh periods.

3. Test Coverage Gaps

Limited Desktop Testing

// auth-fetch.test.ts:234-250
describe("authenticatedFetch desktop mode documentation", () => {
  it("documents the expected behavior for desktop mode", () => {
    // This behavior is tested through integration testing...
    expect(true).toBe(true)
  })
})

Issue: Desktop mode has documentation-only tests rather than actual test coverage.
Recommendation: Add proper desktop mode tests with mocked Tauri storage.

Missing Edge Cases

• No tests for concurrent refresh scenarios
• Missing timeout/network error simulation
• No tests for malformed JWT handling in logout flow web-auth.ts:47-70

4. Code Quality

Platform Detection Pattern

// Repeated pattern throughout
if (isTauri()) {
  // desktop logic
} else {
  // web logic
}

Issue: Platform checks scattered throughout code could become maintenance burden.
Recommendation: Consider a strategy pattern or platform-specific implementations.

Magic Numbers

// auth-token.ts:22-23, web-auth.ts:76
const MAX_REFRESH_RETRIES = 3
const REFRESH_BUFFER_MS = 5 * 60 * 1000

Issue: Configuration values hardcoded rather than configurable.
Recommendation: Move to config file or environment variables.

---

🚀 Performance Assessment

Positive Impact:

• Reduced bundle size: ~1400 lines of code removed through consolidation
• Eliminated duplicate logic: Single refresh implementation vs. platform-specific variants
• Better caching: Centralized token access reduces storage calls

Potential Concerns:

• Import chaining: New module creates longer import chains for some operations
• Effect overhead: Promise wrappers add minimal overhead for React consumers

---

🔒 Security Assessment

Strong Security Practices:

• ✅ Uses secure storage (Tauri store for desktop, localStorage for web)
• ✅ Proper Bearer token implementation
• ✅ Automatic token cleanup on auth failure
• ✅ WorkOS logout integration clears server-side sessions

Security Recommendations:

• 🔧 Sanitize error logs to prevent token exposure
• 🔧 Add token validation before storage
• 🔧 Consider implementing token fingerprinting for additional security

---

📝 Final Verdict

This is a high-quality refactoring that successfully addresses the core issue of refresh token usage while significantly improving code organization. The new architecture is more maintainable, follows Effect-TS best practices, and provides better error handling.

Recommendation: ✅ This refactoring is well-executed

The identified issues are primarily minor security hardening and test coverage improvements rather than fundamental architectural problems. The consolidation achieves its goals effectively.

Priority follow-ups:

1. Add desktop mode test coverage
2. Sanitize token-related logging
3. Consider implementing request queuing during refresh

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	36.78%	3177 / 8637
🔵	Statements	36.77%	3313 / 9009
🔵	Functions	23.66%	687 / 2903
🔵	Branches	27.61%	1015 / 3675

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
apps/web/src/atoms/desktop-auth.ts	13.93%	0%	0%	14.65%	75, 86-130, 139-158, 167-168, 185-232, 241-279, 287-319, 327-328
apps/web/src/atoms/web-auth.ts	11.65%	0%	0%	12.12%	48-62, 66-69, 96, 108-155, 170-171, 180-219, 227-259
apps/web/src/atoms/web-callback-atoms.ts	9.75%	0%	0%	9.75%	81-102, 120-254, 267-277, 289-290, 303
apps/web/src/lib/auth-fetch.ts	72.09%	56.25%	100%	72.09%	82-98, 107
apps/web/src/lib/auth-token.ts	37.96%	31.37%	37.5%	35.92%	72-73, 78-81, 87, 98-107, 120, 131-132, 136, 150-155, 168-266, 290
apps/web/src/lib/registry.ts	100%	100%	100%	100%
apps/web/src/lib/rpc-auth-middleware.ts	12.5%	0%	0%	12.5%	14-24

Generated in workflow #1575 for commit 539d34a by the Vitest Coverage Report Action