fix(core): token race condition - #15320
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughPersisted authentication sessions are initialized through a new exported function. Electron protocol registration awaits this initialization before installing protocol hooks, and tests verify restored endpoint management and access tokens. ChangesAuth session startup
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant registerProtocol
participant initializeAuthSessions
participant readStore
participant AuthTokenBroker
participant ElectronHooks
registerProtocol->>initializeAuthSessions: initialize persisted sessions
initializeAuthSessions->>readStore: read stored endpoints
readStore-->>initializeAuthSessions: return session records
initializeAuthSessions->>AuthTokenBroker: getAuthSessionBroker(endpoint)
AuthTokenBroker-->>initializeAuthSessions: initialize endpoint broker
initializeAuthSessions-->>registerProtocol: complete initialization
registerProtocol->>ElectronHooks: register protocol and handlers
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/frontend/apps/electron/src/main/auth/auth-session.ts`:
- Around line 146-149: Update initializeAuthSessions to check
secureStorageAvailable() before reading persisted store keys or restoring
brokers, and skip initialization when secure storage is unavailable so endpoints
are not marked managed without usable tokens. Add a regression test covering
restart with unavailable secure storage and preserving the renderer
Authorization header.
In `@packages/frontend/apps/electron/test/main/auth-session.spec.ts`:
- Around line 103-105: Update the seeded session in the test using
accessExpiresAt to derive the expiry from the test clock, Date.now(), and
pair.expiresIn instead of a fixed 2030 timestamp; leave the refresh token and
refreshExpiresAt setup unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d8d59595-6654-439e-94ae-5e1188937698
📒 Files selected for processing (3)
packages/frontend/apps/electron/src/main/auth/auth-session.tspackages/frontend/apps/electron/src/main/protocol.tspackages/frontend/apps/electron/test/main/auth-session.spec.ts
fix #15318
fix #15310
PR Dependency Tree
This tree was auto-generated by Charcoal
Summary by CodeRabbit