ciphera-net/pulse
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 15 Wiki Activity

Settings page overhaul, auth resilience, and automated testing #38

Merged
uz1mani merged 21 commits from staging into main 2026-03-01 13:05:56 +00:00
Conversation 7 Commits 21 Files Changed 25 +3981 -28
uz1mani commented 2026-02-28 23:33:53 +00:00 (Migrated from github.com)
Copy Link

Work Item

Summary

  • Redesigned settings page with expandable sidebar navigation, account management, security activity, trusted devices, and email notification preferences
  • Added refresh token support and improved auth context for session resilience
  • Introduced unit testing with Vitest and CI workflow for automated test runs

Changes

  • Settings page: New SettingsPageClient.tsx with sidebar navigation covering profile, security activity (SecurityActivityCard.tsx), trusted devices (TrustedDevicesCard.tsx), and notification preferences with granular security alerts
  • Auth resilience: lib/auth/context.tsx now handles refresh token flow; lib/api/client.ts propagates Request ID headers; local storage management updated
  • API layer: Added lib/api/activity.ts and lib/api/devices.ts for security activity and trusted device endpoints
  • Error handling: New lib/utils/errorHandler.ts and lib/utils/requestId.ts utilities for structured error handling and request tracing
  • Testing: Added Vitest config (vitest.config.ts, vitest.setup.ts), middleware tests, and unit tests for useOnlineStatus, useVisibilityPolling, errorHandler, logger, and requestId
  • CI: .github/workflows/test.yml runs tests on push with PKG_READ_TOKEN for private dependency access
  • Deps: Bumped @ciphera-net/ui to 0.0.74, added Vitest and testing-library dev dependencies
  • Version: Bumped to 0.12.0-alpha; CHANGELOG updated with all new entries

Test Plan

  • Navigate to Settings and verify sidebar sections (Profile, Security Activity, Trusted Devices, Notifications) load correctly
  • Trigger a session expiry and confirm refresh token silently renews the access token
  • Verify notification preference toggles persist across page reloads
  • Confirm CI workflow passes on push (check GitHub Actions)
  • Run npm test locally and verify all unit tests pass
## Work Item ## Summary - Redesigned settings page with expandable sidebar navigation, account management, security activity, trusted devices, and email notification preferences - Added refresh token support and improved auth context for session resilience - Introduced unit testing with Vitest and CI workflow for automated test runs ## Changes - **Settings page:** New `SettingsPageClient.tsx` with sidebar navigation covering profile, security activity (`SecurityActivityCard.tsx`), trusted devices (`TrustedDevicesCard.tsx`), and notification preferences with granular security alerts - **Auth resilience:** `lib/auth/context.tsx` now handles refresh token flow; `lib/api/client.ts` propagates Request ID headers; local storage management updated - **API layer:** Added `lib/api/activity.ts` and `lib/api/devices.ts` for security activity and trusted device endpoints - **Error handling:** New `lib/utils/errorHandler.ts` and `lib/utils/requestId.ts` utilities for structured error handling and request tracing - **Testing:** Added Vitest config (`vitest.config.ts`, `vitest.setup.ts`), middleware tests, and unit tests for `useOnlineStatus`, `useVisibilityPolling`, `errorHandler`, `logger`, and `requestId` - **CI:** `.github/workflows/test.yml` runs tests on push with `PKG_READ_TOKEN` for private dependency access - **Deps:** Bumped `@ciphera-net/ui` to 0.0.74, added Vitest and testing-library dev dependencies - **Version:** Bumped to 0.12.0-alpha; CHANGELOG updated with all new entries ## Test Plan - [x] Navigate to Settings and verify sidebar sections (Profile, Security Activity, Trusted Devices, Notifications) load correctly - [x] Trigger a session expiry and confirm refresh token silently renews the access token - [x] Verify notification preference toggles persist across page reloads - [x] Confirm CI workflow passes on push (check GitHub Actions) - [x] Run `npm test` locally and verify all unit tests pass
👍 1
greptile-apps[bot] commented 2026-02-28 23:45:28 +00:00 (Migrated from github.com)
Copy Link

Greptile Summary

This PR introduces a comprehensive settings page redesign, auth resilience improvements, and automated testing infrastructure. The settings page features an expandable sidebar with profile management, security activity logs, trusted devices, and granular notification preferences. Auth improvements include automatic refresh token handling, session expiry warnings, and cross-tab session synchronization using BroadcastChannel.

Key Changes:

  • Settings UI: New SettingsPageClient.tsx with expandable sidebar navigation, SecurityActivityCard.tsx for audit logs, TrustedDevicesCard.tsx for device management, and granular security alert toggles (login, password, 2FA alerts)
  • Auth resilience: lib/auth/context.tsx now handles automatic token refresh on 401, displays session expiry warnings, and syncs logout/login events across browser tabs
  • API improvements: lib/api/client.ts adds request ID propagation for debugging, automatic retry after token refresh, and GET request deduplication/caching (2s TTL)
  • Testing: Added Vitest with jsdom, comprehensive middleware tests, hook tests, and CI workflow with PKG_READ_TOKEN for private package access
  • Dependencies: Bumped @ciphera-net/ui from 0.0.69 to 0.0.78 (adds SessionExpiryWarning and useSessionSync)

Issues Found:

  • lib/api/user.ts contains dead code attempting to read refreshToken from localStorage, but refresh tokens are stored exclusively in httpOnly cookies. The X-Current-Session-Hash header will never be sent.

Confidence Score: 4/5

  • Safe to merge with one minor fix needed for session identification
  • The PR is well-implemented with comprehensive testing, proper error handling, and good architectural decisions. The auth flow improvements are sound and the UI components are clean. One logical issue exists in getUserSessions where dead code tries to read a refresh token from localStorage that doesn't exist (tokens are in httpOnly cookies). This should be cleaned up but won't cause runtime errors since the code handles null gracefully.
  • Pay attention to lib/api/user.ts - the getUserSessions function contains dead code that should be removed or fixed

Important Files Changed
Filename Overview
lib/auth/context.tsx Added refresh token flow, session expiry warning, and cross-tab session sync using BroadcastChannel. Improved auth resilience and user experience.
lib/api/client.ts Added request ID propagation, automatic token refresh on 401, and request deduplication/caching for GET requests. Improved error handling and debugging.
lib/api/user.ts Updated email notification preferences schema and added display name update. Contains dead code in getUserSessions (refresh token localStorage read).
app/settings/SettingsPageClient.tsx New expandable sidebar navigation with profile, security activity, trusted devices, and notification preferences. Clean UI implementation with proper state management.
components/settings/SecurityActivityCard.tsx Displays paginated security audit log with event type icons, browser/OS detection, and failure reasons. Clean implementation with proper error handling.
components/settings/TrustedDevicesCard.tsx Manages trusted devices with remove functionality. Properly prevents removal of current device and shows appropriate feedback.

Sequence Diagram

sequenceDiagram
    participant User
    participant Browser
    participant AuthContext
    participant APIClient
    participant Backend
    participant RefreshEndpoint

    Note over User,RefreshEndpoint: Normal API Request Flow
    User->>Browser: Interact with app
    Browser->>APIClient: API request (with cookies)
    APIClient->>Backend: GET/POST with access_token cookie
    Backend-->>APIClient: 401 Unauthorized
    
    Note over APIClient,RefreshEndpoint: Automatic Token Refresh
    APIClient->>APIClient: Check if refresh in progress
    APIClient->>RefreshEndpoint: POST /api/auth/refresh
    RefreshEndpoint-->>APIClient: 200 OK (new cookies set)
    APIClient->>Backend: Retry original request
    Backend-->>APIClient: 200 OK (data)
    APIClient-->>Browser: Return data
    
    Note over AuthContext,User: Session Expiry Warning
    AuthContext->>AuthContext: Monitor token age
    AuthContext->>User: Show expiry warning
    User->>AuthContext: Click "Stay signed in"
    AuthContext->>RefreshEndpoint: POST /api/auth/refresh
    RefreshEndpoint-->>AuthContext: 200 OK (session extended)
    
    Note over AuthContext,Browser: Cross-Tab Session Sync
    Browser->>AuthContext: Login in Tab 1
    AuthContext->>Browser: BroadcastChannel message
    Browser->>AuthContext: Tab 2 receives login event
    AuthContext->>Browser: Update UI in Tab 2

Last reviewed commit: 29e84e3

<h3>Greptile Summary</h3> This PR introduces a comprehensive settings page redesign, auth resilience improvements, and automated testing infrastructure. The settings page features an expandable sidebar with profile management, security activity logs, trusted devices, and granular notification preferences. Auth improvements include automatic refresh token handling, session expiry warnings, and cross-tab session synchronization using BroadcastChannel. **Key Changes:** - **Settings UI**: New `SettingsPageClient.tsx` with expandable sidebar navigation, `SecurityActivityCard.tsx` for audit logs, `TrustedDevicesCard.tsx` for device management, and granular security alert toggles (login, password, 2FA alerts) - **Auth resilience**: `lib/auth/context.tsx` now handles automatic token refresh on 401, displays session expiry warnings, and syncs logout/login events across browser tabs - **API improvements**: `lib/api/client.ts` adds request ID propagation for debugging, automatic retry after token refresh, and GET request deduplication/caching (2s TTL) - **Testing**: Added Vitest with jsdom, comprehensive middleware tests, hook tests, and CI workflow with `PKG_READ_TOKEN` for private package access - **Dependencies**: Bumped `@ciphera-net/ui` from 0.0.69 to 0.0.78 (adds `SessionExpiryWarning` and `useSessionSync`) **Issues Found:** - `lib/api/user.ts` contains dead code attempting to read `refreshToken` from localStorage, but refresh tokens are stored exclusively in httpOnly cookies. The `X-Current-Session-Hash` header will never be sent. <h3>Confidence Score: 4/5</h3> - Safe to merge with one minor fix needed for session identification - The PR is well-implemented with comprehensive testing, proper error handling, and good architectural decisions. The auth flow improvements are sound and the UI components are clean. One logical issue exists in `getUserSessions` where dead code tries to read a refresh token from localStorage that doesn't exist (tokens are in httpOnly cookies). This should be cleaned up but won't cause runtime errors since the code handles null gracefully. - Pay attention to `lib/api/user.ts` - the `getUserSessions` function contains dead code that should be removed or fixed <details><summary><h3>Important Files Changed</h3></summary> | Filename | Overview | |----------|----------| | lib/auth/context.tsx | Added refresh token flow, session expiry warning, and cross-tab session sync using BroadcastChannel. Improved auth resilience and user experience. | | lib/api/client.ts | Added request ID propagation, automatic token refresh on 401, and request deduplication/caching for GET requests. Improved error handling and debugging. | | lib/api/user.ts | Updated email notification preferences schema and added display name update. Contains dead code in getUserSessions (refresh token localStorage read). | | app/settings/SettingsPageClient.tsx | New expandable sidebar navigation with profile, security activity, trusted devices, and notification preferences. Clean UI implementation with proper state management. | | components/settings/SecurityActivityCard.tsx | Displays paginated security audit log with event type icons, browser/OS detection, and failure reasons. Clean implementation with proper error handling. | | components/settings/TrustedDevicesCard.tsx | Manages trusted devices with remove functionality. Properly prevents removal of current device and shows appropriate feedback. | </details> </details> <h3>Sequence Diagram</h3> ```mermaid sequenceDiagram participant User participant Browser participant AuthContext participant APIClient participant Backend participant RefreshEndpoint Note over User,RefreshEndpoint: Normal API Request Flow User->>Browser: Interact with app Browser->>APIClient: API request (with cookies) APIClient->>Backend: GET/POST with access_token cookie Backend-->>APIClient: 401 Unauthorized Note over APIClient,RefreshEndpoint: Automatic Token Refresh APIClient->>APIClient: Check if refresh in progress APIClient->>RefreshEndpoint: POST /api/auth/refresh RefreshEndpoint-->>APIClient: 200 OK (new cookies set) APIClient->>Backend: Retry original request Backend-->>APIClient: 200 OK (data) APIClient-->>Browser: Return data Note over AuthContext,User: Session Expiry Warning AuthContext->>AuthContext: Monitor token age AuthContext->>User: Show expiry warning User->>AuthContext: Click "Stay signed in" AuthContext->>RefreshEndpoint: POST /api/auth/refresh RefreshEndpoint-->>AuthContext: 200 OK (session extended) Note over AuthContext,Browser: Cross-Tab Session Sync Browser->>AuthContext: Login in Tab 1 AuthContext->>Browser: BroadcastChannel message Browser->>AuthContext: Tab 2 receives login event AuthContext->>Browser: Update UI in Tab 2 ``` <sub>Last reviewed commit: 29e84e3</sub>
greptile-apps[bot] (Migrated from github.com) reviewed 2026-02-28 23:45:38 +00:00
greptile-apps[bot] (Migrated from github.com) left a comment
Copy Link

25 files reviewed, 5 comments

Edit Code Review Agent Settings | Greptile

<sub>25 files reviewed, 5 comments</sub> <sub>[Edit Code Review Agent Settings](https://app.greptile.com/review/github) | [Greptile](https://greptile.com?utm_source=greptile_expert&utm_medium=github&utm_campaign=code_reviews)</sub>
components/settings/TrustedDevicesCard.tsx
@@ -0,0 +1,130 @@
'use client'
greptile-apps[bot] (Migrated from github.com) commented 2026-02-28 23:45:37 +00:00
Copy Link

Duplicated formatRelativeTime / formatFullDate helpers

formatRelativeTime and formatFullDate are defined identically in both TrustedDevicesCard.tsx and SecurityActivityCard.tsx. Consider extracting them to a shared utility (e.g., lib/utils/formatDate.ts) to avoid the two implementations drifting out of sync.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: components/settings/TrustedDevicesCard.tsx
Line: 1-30

Comment:
**Duplicated `formatRelativeTime` / `formatFullDate` helpers**

`formatRelativeTime` and `formatFullDate` are defined identically in both `TrustedDevicesCard.tsx` and `SecurityActivityCard.tsx`. Consider extracting them to a shared utility (e.g., `lib/utils/formatDate.ts`) to avoid the two implementations drifting out of sync.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

**Duplicated `formatRelativeTime` / `formatFullDate` helpers** `formatRelativeTime` and `formatFullDate` are defined identically in both `TrustedDevicesCard.tsx` and `SecurityActivityCard.tsx`. Consider extracting them to a shared utility (e.g., `lib/utils/formatDate.ts`) to avoid the two implementations drifting out of sync. <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: components/settings/TrustedDevicesCard.tsx Line: 1-30 Comment: **Duplicated `formatRelativeTime` / `formatFullDate` helpers** `formatRelativeTime` and `formatFullDate` are defined identically in both `TrustedDevicesCard.tsx` and `SecurityActivityCard.tsx`. Consider extracting them to a shared utility (e.g., `lib/utils/formatDate.ts`) to avoid the two implementations drifting out of sync. How can I resolve this? If you propose a fix, please make it concise. ````` </details> <sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
lib/auth/context.tsx
greptile-apps[bot] (Migrated from github.com) commented 2026-02-28 23:45:32 +00:00
Copy Link

Incomplete localStorage cleanup on cross-tab logout

The onLogout callback in useSessionSync only removes user from localStorage, whereas the main logout() function also clears ciphera_token_refreshed_at and ciphera_last_activity. While the page redirect (window.location.href = '/') prevents these stale entries from causing authentication issues on reload, it's inconsistent and leaves orphaned keys for any logic that might read them before the redirect completes.

  useSessionSync({
    onLogout: () => {
      localStorage.removeItem('user')
      localStorage.removeItem('ciphera_token_refreshed_at')
      localStorage.removeItem('ciphera_last_activity')
      window.location.href = '/'
    },
    onLogin: (userData) => {
      setUser(userData as User)
      router.refresh()
    },
    onRefresh: () => {
      refresh()
    },
  })
Prompt To Fix With AI 
This is a comment left during a code review.
Path: lib/auth/context.tsx
Line: 174-186

Comment:
**Incomplete localStorage cleanup on cross-tab logout**

The `onLogout` callback in `useSessionSync` only removes `user` from localStorage, whereas the main `logout()` function also clears `ciphera_token_refreshed_at` and `ciphera_last_activity`. While the page redirect (`window.location.href = '/'`) prevents these stale entries from causing authentication issues on reload, it's inconsistent and leaves orphaned keys for any logic that might read them before the redirect completes.

```suggestion
  useSessionSync({
    onLogout: () => {
      localStorage.removeItem('user')
      localStorage.removeItem('ciphera_token_refreshed_at')
      localStorage.removeItem('ciphera_last_activity')
      window.location.href = '/'
    },
    onLogin: (userData) => {
      setUser(userData as User)
      router.refresh()
    },
    onRefresh: () => {
      refresh()
    },
  })
```

How can I resolve this? If you propose a fix, please make it concise.
**Incomplete localStorage cleanup on cross-tab logout** The `onLogout` callback in `useSessionSync` only removes `user` from localStorage, whereas the main `logout()` function also clears `ciphera_token_refreshed_at` and `ciphera_last_activity`. While the page redirect (`window.location.href = '/'`) prevents these stale entries from causing authentication issues on reload, it's inconsistent and leaves orphaned keys for any logic that might read them before the redirect completes. ```suggestion useSessionSync({ onLogout: () => { localStorage.removeItem('user') localStorage.removeItem('ciphera_token_refreshed_at') localStorage.removeItem('ciphera_last_activity') window.location.href = '/' }, onLogin: (userData) => { setUser(userData as User) router.refresh() }, onRefresh: () => { refresh() }, }) ``` <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: lib/auth/context.tsx Line: 174-186 Comment: **Incomplete localStorage cleanup on cross-tab logout** The `onLogout` callback in `useSessionSync` only removes `user` from localStorage, whereas the main `logout()` function also clears `ciphera_token_refreshed_at` and `ciphera_last_activity`. While the page redirect (`window.location.href = '/'`) prevents these stale entries from causing authentication issues on reload, it's inconsistent and leaves orphaned keys for any logic that might read them before the redirect completes. ```suggestion useSessionSync({ onLogout: () => { localStorage.removeItem('user') localStorage.removeItem('ciphera_token_refreshed_at') localStorage.removeItem('ciphera_last_activity') window.location.href = '/' }, onLogin: (userData) => { setUser(userData as User) router.refresh() }, onRefresh: () => { refresh() }, }) ``` How can I resolve this? If you propose a fix, please make it concise. ````` </details>
lib/utils/requestId.ts
@@ -0,0 +39,4 @@
export function getLastRequestId(): string | null {
return lastRequestId
}
greptile-apps[bot] (Migrated from github.com) commented 2026-02-28 23:45:36 +00:00
Copy Link

Module-level singleton is unsafe for concurrent server-side use

lastRequestId is a module-level mutable variable. In a Node.js server (SSR/server actions), module state is shared across all concurrent requests. If apiRequest is called simultaneously from two server-side contexts, request A sets lastRequestId = "REQ_A", then request B sets it to "REQ_B", and request A's error handler ends up reporting "REQ_B" — attributing the wrong request ID to the error.

Today apiRequest is primarily called from 'use client' components (single-threaded browser), so the practical risk is low. However, to be safe, consider scoping the request ID to each call rather than storing it globally, e.g. by returning it from generateRequestId and passing it explicitly through error paths, or by using an AsyncLocalStorage context for server-side scenarios.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: lib/utils/requestId.ts
Line: 28-36

Comment:
**Module-level singleton is unsafe for concurrent server-side use**

`lastRequestId` is a module-level mutable variable. In a Node.js server (SSR/server actions), module state is shared across all concurrent requests. If `apiRequest` is called simultaneously from two server-side contexts, request A sets `lastRequestId = "REQ_A"`, then request B sets it to `"REQ_B"`, and request A's error handler ends up reporting `"REQ_B"` — attributing the wrong request ID to the error.

Today `apiRequest` is primarily called from `'use client'` components (single-threaded browser), so the practical risk is low. However, to be safe, consider scoping the request ID to each call rather than storing it globally, e.g. by returning it from `generateRequestId` and passing it explicitly through error paths, or by using an `AsyncLocalStorage` context for server-side scenarios.

How can I resolve this? If you propose a fix, please make it concise.
**Module-level singleton is unsafe for concurrent server-side use** `lastRequestId` is a module-level mutable variable. In a Node.js server (SSR/server actions), module state is shared across all concurrent requests. If `apiRequest` is called simultaneously from two server-side contexts, request A sets `lastRequestId = "REQ_A"`, then request B sets it to `"REQ_B"`, and request A's error handler ends up reporting `"REQ_B"` — attributing the wrong request ID to the error. Today `apiRequest` is primarily called from `'use client'` components (single-threaded browser), so the practical risk is low. However, to be safe, consider scoping the request ID to each call rather than storing it globally, e.g. by returning it from `generateRequestId` and passing it explicitly through error paths, or by using an `AsyncLocalStorage` context for server-side scenarios. <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: lib/utils/requestId.ts Line: 28-36 Comment: **Module-level singleton is unsafe for concurrent server-side use** `lastRequestId` is a module-level mutable variable. In a Node.js server (SSR/server actions), module state is shared across all concurrent requests. If `apiRequest` is called simultaneously from two server-side contexts, request A sets `lastRequestId = "REQ_A"`, then request B sets it to `"REQ_B"`, and request A's error handler ends up reporting `"REQ_B"` — attributing the wrong request ID to the error. Today `apiRequest` is primarily called from `'use client'` components (single-threaded browser), so the practical risk is low. However, to be safe, consider scoping the request ID to each call rather than storing it globally, e.g. by returning it from `generateRequestId` and passing it explicitly through error paths, or by using an `AsyncLocalStorage` context for server-side scenarios. How can I resolve this? If you propose a fix, please make it concise. ````` </details>
greptile-apps[bot] commented 2026-02-28 23:45:39 +00:00 (Migrated from github.com)
Copy Link
Additional Comments (1)

app/settings/SettingsPageClient.tsx
Parent section header not highlighted for child selections

When the user navigates to "Trusted Devices" (active.section === 'devices') or "Security Activity" (active.section === 'activity'), both of which live under the "Manage Account" parent in the sidebar, the SectionHeader for "Manage Account" shows active={false} because neither 'devices' nor 'activity' equals 'account'. The child SubItem is correctly highlighted, but the parent heading is not — leaving no visual breadcrumb to indicate which top-level group the user is in.

              active={active.section === 'account' || active.section === 'devices' || active.section === 'activity'}
Prompt To Fix With AI 
This is a comment left during a code review.
Path: app/settings/SettingsPageClient.tsx
Line: 636

Comment:
**Parent section header not highlighted for child selections**

When the user navigates to "Trusted Devices" (`active.section === 'devices'`) or "Security Activity" (`active.section === 'activity'`), both of which live under the "Manage Account" parent in the sidebar, the `SectionHeader` for "Manage Account" shows `active={false}` because neither `'devices'` nor `'activity'` equals `'account'`. The child `SubItem` is correctly highlighted, but the parent heading is not — leaving no visual breadcrumb to indicate which top-level group the user is in.

```suggestion
              active={active.section === 'account' || active.section === 'devices' || active.section === 'activity'}
```

How can I resolve this? If you propose a fix, please make it concise.
<details> <summary>Additional Comments (1)</summary> **`app/settings/SettingsPageClient.tsx`** **Parent section header not highlighted for child selections** When the user navigates to "Trusted Devices" (`active.section === 'devices'`) or "Security Activity" (`active.section === 'activity'`), both of which live under the "Manage Account" parent in the sidebar, the `SectionHeader` for "Manage Account" shows `active={false}` because neither `'devices'` nor `'activity'` equals `'account'`. The child `SubItem` is correctly highlighted, but the parent heading is not — leaving no visual breadcrumb to indicate which top-level group the user is in. ```suggestion active={active.section === 'account' || active.section === 'devices' || active.section === 'activity'} ``` <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: app/settings/SettingsPageClient.tsx Line: 636 Comment: **Parent section header not highlighted for child selections** When the user navigates to "Trusted Devices" (`active.section === 'devices'`) or "Security Activity" (`active.section === 'activity'`), both of which live under the "Manage Account" parent in the sidebar, the `SectionHeader` for "Manage Account" shows `active={false}` because neither `'devices'` nor `'activity'` equals `'account'`. The child `SubItem` is correctly highlighted, but the parent heading is not — leaving no visual breadcrumb to indicate which top-level group the user is in. ```suggestion active={active.section === 'account' || active.section === 'devices' || active.section === 'activity'} ``` How can I resolve this? If you propose a fix, please make it concise. ````` </details> </details>
uz1mani (Migrated from github.com) reviewed 2026-03-01 12:52:40 +00:00
lib/auth/context.tsx
uz1mani (Migrated from github.com) commented 2026-03-01 12:52:39 +00:00
Copy Link

Issue: The onLogout callback in useSessionSync only removed user from localStorage, while the main logout() function also clears ciphera_token_refreshed_at and ciphera_last_activity.
Fix: Added the two missing localStorage.removeItem calls to onLogout so it matches the main logout() cleanup.
Why: Orphaned keys could be read by session-related logic in the window between the removeItem call and the redirect completing.

Issue: The `onLogout` callback in `useSessionSync` only removed `user` from localStorage, while the main `logout()` function also clears `ciphera_token_refreshed_at` and `ciphera_last_activity`. Fix: Added the two missing `localStorage.removeItem` calls to `onLogout` so it matches the main `logout()` cleanup. Why: Orphaned keys could be read by session-related logic in the window between the removeItem call and the redirect completing.
uz1mani (Migrated from github.com) reviewed 2026-03-01 12:52:51 +00:00
lib/utils/requestId.ts
@@ -0,0 +39,4 @@
export function getLastRequestId(): string | null {
return lastRequestId
}
uz1mani (Migrated from github.com) commented 2026-03-01 12:52:51 +00:00
Copy Link

Issue: lastRequestId is a module-level mutable variable. In a Node.js SSR context, concurrent requests could overwrite each other's request IDs.
Fix: Added documentation explicitly scoping this module to client-side use only. apiRequest (the sole caller of setLastRequestId) runs exclusively in the browser where JS is single-threaded, so no concurrent mutation can occur.
Why: The practical risk is zero today, but the intent constraint should be documented so future server-side callers know to use a different pattern (e.g. AsyncLocalStorage).

Issue: `lastRequestId` is a module-level mutable variable. In a Node.js SSR context, concurrent requests could overwrite each other's request IDs. Fix: Added documentation explicitly scoping this module to client-side use only. `apiRequest` (the sole caller of `setLastRequestId`) runs exclusively in the browser where JS is single-threaded, so no concurrent mutation can occur. Why: The practical risk is zero today, but the intent constraint should be documented so future server-side callers know to use a different pattern (e.g. `AsyncLocalStorage`).
uz1mani (Migrated from github.com) reviewed 2026-03-01 12:52:58 +00:00
components/settings/TrustedDevicesCard.tsx
@@ -0,0 +1,130 @@
'use client'
uz1mani (Migrated from github.com) commented 2026-03-01 12:52:58 +00:00
Copy Link

Issue: formatRelativeTime and formatFullDate were defined identically in both TrustedDevicesCard.tsx and SecurityActivityCard.tsx.
Fix: Extracted both functions to a shared lib/utils/formatDate.ts module and replaced the inline definitions with imports.
Why: Two copies can drift out of sync silently, and the functions are generic enough to be reused elsewhere.

Issue: `formatRelativeTime` and `formatFullDate` were defined identically in both `TrustedDevicesCard.tsx` and `SecurityActivityCard.tsx`. Fix: Extracted both functions to a shared `lib/utils/formatDate.ts` module and replaced the inline definitions with imports. Why: Two copies can drift out of sync silently, and the functions are generic enough to be reused elsewhere.
greptile-apps[bot] commented 2026-03-01 13:15:14 +00:00 (Migrated from github.com)
Copy Link
Additional Comments (1)

lib/api/user.ts, line 32
dead code - refresh tokens are stored in httpOnly cookies, not localStorage

This code tries to read refreshToken from localStorage and hash it, but the auth implementation (see lib/auth/context.tsx and middleware.ts) stores refresh tokens exclusively in httpOnly cookies. localStorage.getItem('refreshToken') will always return null, so currentTokenHash will always be empty and the X-Current-Session-Hash header will never be sent.

Either remove this dead code if the backend doesn't need this header anymore, or find an alternative way to identify the current session (backend could use the refresh_token cookie directly).

<details> <summary>Additional Comments (1)</summary> **`lib/api/user.ts`, line 32** dead code - refresh tokens are stored in httpOnly cookies, not localStorage This code tries to read `refreshToken` from localStorage and hash it, but the auth implementation (see `lib/auth/context.tsx` and `middleware.ts`) stores refresh tokens exclusively in httpOnly cookies. `localStorage.getItem('refreshToken')` will always return null, so `currentTokenHash` will always be empty and the `X-Current-Session-Hash` header will never be sent. Either remove this dead code if the backend doesn't need this header anymore, or find an alternative way to identify the current session (backend could use the `refresh_token` cookie directly). </details>
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something isn't working
documentation
Improvements or additions to documentation
duplicate
This issue or pull request already exists
enhancement
New feature or request
good first issue
Good for newcomers
help wanted
Extra attention is needed
invalid
This doesn't seem right
question
Further information is requested
wontfix
This will not be worked on
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
usman
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: ciphera-net/pulse#38
No description provided.
Delete Branch "staging"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?