Fix sessionCreatedAt overwritten on token refresh#483
Conversation
Co-authored-by: mblomdahl <786326+mblomdahl@users.noreply.github.com>
- Add optional isRefresh parameter to parseAuthSessionData() - When isRefresh=true, omit sessionCreatedAt to preserve original login time - Update axiosInstance.ts to pass isRefresh=true during token refresh - Add comprehensive unit tests for the fix Co-authored-by: mblomdahl <786326+mblomdahl@users.noreply.github.com>
mblomdahl
left a comment
There was a problem hiding this comment.
@claude Please review if the implementation here aligns with your latest comment on #451 here: #451 (comment)
|
Claude finished @Copilot's task —— View job Code Review: Session Timestamp FixI've reviewed the implementation in this PR. While I cannot access my specific comment from issue #451 due to tool restrictions, I can thoroughly analyze the implementation against the problem statement and best practices. Summary✅ The implementation correctly solves the problem - Detailed Review✅ OwnTube.tv/utils/auth.ts (Lines 4-32)What was changed:
Review: ...(!isRefresh && { sessionCreatedAt: new Date().toISOString() }),This is the correct approach:
Potential concern: The implementation relies on ✅ OwnTube.tv/api/axiosInstance.ts (Line 132)What was changed: const parsed = parseAuthSessionData(refreshed, backend, true);Review:
✅ OwnTube.tv/utils/auth.test.ts (New file, 76 lines)What was tested:
Review:
Architecture VerificationThe fix relies on this flow:
Critical assumption: The Zustand Recommendations1. ✅ No issues found - implementation is correctThe code correctly solves the problem as described. The approach is:
2. 📝 Optional enhancement: Verify store behaviorIf you want to be extra defensive, you could add an integration test that verifies the full flow:
However, this may be overkill if the store implementation is already tested elsewhere. ConclusionApproval recommendation: ✅ APPROVE The implementation correctly fixes the bug where
The PR is ready to merge from a code quality perspective. | View PR |
sessionCreatedAt overwritten on token refresh
Token refresh was overwriting
sessionCreatedAtwith the refresh timestamp instead of preserving the original login time. This caused the UI to show incorrect "session started" times after tokens were refreshed.Changes
utils/auth.tsisRefresh?: booleanparameter toparseAuthSessionData()sessionCreatedAtfrom returned object whenisRefresh=trueapi/axiosInstance.tsisRefresh: truewhen callingparseAuthSessionData()during token refresh (line 132)utils/auth.test.ts(new)sessionCreatedAtbehavior for both login and refresh scenariosImplementation
When
isRefresh=true,sessionCreatedAtis omitted from the partial object. TheupdateSession()merge preserves the existing value while updating other fields (sessionUpdatedAt, token timestamps).Existing call sites in SignIn/Otp screens continue to work without modification (default
isRefresh=undefinedsets timestamp on initial login).Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
eu-assets.i.posthog.com/usr/local/bin/node /home/REDACTED/work/web-client/web-client/OwnTube.tv/node_modules/jest-worker/build/workers/processChild.js(dns block)eu.i.posthog.com/usr/local/bin/node /home/REDACTED/work/web-client/web-client/OwnTube.tv/node_modules/jest-worker/build/workers/processChild.js(dns block)peertube2.cpy.re/usr/local/bin/node /home/REDACTED/work/web-client/web-client/OwnTube.tv/node_modules/jest-worker/build/workers/processChild.js(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.