Skip to content

Preserve Clerk session between login window opens#642

Merged
highperformancecoder merged 3 commits intocopilot/create-angular-login-componentfrom
copilot/preserve-clerk-session-login-window
Apr 9, 2026
Merged

Preserve Clerk session between login window opens#642
highperformancecoder merged 3 commits intocopilot/create-angular-login-componentfrom
copilot/preserve-clerk-session-login-window

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 9, 2026

  • Add setSession(token) method to ClerkService - now actually reinstates the session by calling clerk.setActive() with the first available session from clerk.client.sessions if no session is currently active
  • Update LoginComponent to accept authToken query param via ActivatedRoute, use take(1), delegate to private initializeSession() method, add ngOnDestroy with destroy$ Subject
  • Update WindowManager.openLoginWindow() to retrieve stored authToken from StoreManager and pass it as URL query parameter to the login component
Original prompt

Preserve Clerk Session Between Login Window Opens

Problem

Currently, when "Manage Clerk session" is selected, it asks for the user to log in again, even if previously logged in. The Clerk session is not preserved between calls to open the login window.

Solution

Modify the existing LoginComponent to accept an authToken query parameter and use it to restore the Clerk session. Add a WindowManager.openLoginWindow() method that retrieves the stored token and passes it to the login component.

Changes Required

1. Update gui-js/libs/ui-components/src/lib/login/login.component.ts

Modify the component to:

  • Accept authToken as a query parameter via ActivatedRoute
  • Use the token to restore the Clerk session in ngOnInit()
  • Handle both authenticated and unauthenticated states properly

Changes needed:

import { ActivatedRoute } from '@angular/router';
import { takeUntil } from 'rxjs';

// In constructor:
constructor(
  private clerkService: ClerkService, 
  private electronService: ElectronService,
  private route: ActivatedRoute
) {}

// In ngOnInit:
async ngOnInit() {
  this.route.queryParams.pipe(takeUntil(this.destroy$)).subscribe(async (params) => {
    const authToken = params['authToken'];
    
    try {
      await this.clerkService.initialize();
      
      if (authToken) {
        // Try to restore session with the token
        await this.clerkService.setSession(authToken);
      }
      
      this.isAuthenticated = await this.clerkService.isSignedIn();
    } catch (err) {
      // Token expired or invalid, user needs to login
      this.errorMessage = 'Session expired. Please sign in again.';
      this.isAuthenticated = false;
    }
  });
}

Add a destroy$ Subject for cleanup:

private destroy$ = new Subject<void>();

ngOnDestroy() {
  this.destroy$.next();
  this.destroy$.complete();
}

2. Add openLoginWindow() to gui-js/apps/minsky-electron/src/app/managers/WindowManager.ts

Add a static method that:

  • Retrieves the stored authToken from StoreManager
  • Opens the login window with the token as a query parameter
  • Returns a Promise that resolves with the new token (or null)
static async openLoginWindow(): Promise<string | null> {
  const existingToken = StoreManager.store.get('authToken') || '';
  
  return new Promise<string | null>((resolve) => {
    WindowManager._resolveAuthToken = resolve;
    
    WindowManager.createPopupWindowWithRouting({
      width: 500,
      height: 650,
      title: 'Clerk Login',
      url: `#/headless/login?authToken=${encodeURIComponent(existingToken)}`,
      modal: true,
    });
  });
}

// Add this property to WindowManager class
static _resolveAuthToken: ((token: string | null) => void) | null = null;

3. Update menu item handler in gui-js/apps/minsky-electron/src/app/managers/ApplicationMenuManager.ts

Find the "Manage Clerk session" or similar menu item and update it to use WindowManager.openLoginWindow():

{
  label: 'Manage Clerk session',
  async click() {
    const token = await WindowManager.openLoginWindow();
    if (token) {
      console.log('User authenticated successfully');
    } else {
      console.log('Login cancelled or failed');
    }
  },
}

Testing

After these changes:

  1. First login should work normally
  2. Closing and reopening the login window should show "You are signed in" if the token is still valid
  3. If the token expires, the user should see an error message and the login form
  4. Signing out should clear the stored token

Notes

  • The existing IPC handler for SET_AUTH_TOKEN already handles storing/clearing tokens and resolving the Promise
  • The token is already stored in StoreManager via the IPC handler
  • This implementation follows the existing pattern used in other components like AuthorComponent and LockHandlesComponent which accept query parameters

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Currently, when "Manage Clerk session" is selected, it asks for the user to log in again, even if previously logged in, ie the Clerk session is not preserved between calls to WindowManager.openLoginWindow.

Assistant: I'll help you investigate why the Clerk session is not being preserved between calls to WindowManager.openLoginWindow. Let me start by examining the current implementation.

Assistant: Let me search more specifically for Clerk-related code in the PR.

Assistant: Based on my investigation of PR #635 and the codebase, I can see that the issue is that the Clerk session is not being preserved between calls to WindowManager.openLoginWindow. While I couldn't find the exact openLoginWindow method in the current codebase (it may have been added in PR #635), the problem is likely that:

  1. No session persistence mechanism: The Clerk token/session needs to be stored (likely via IPC to electron-store or similar) when the user log...

This pull request was created from Copilot chat.


This change is Reviewable

Agent-Logs-Url: https://github.com/highperformancecoder/minsky/sessions/fee97c92-7978-4c1d-8195-ace7a1267507

Co-authored-by: highperformancecoder <3075825+highperformancecoder@users.noreply.github.com>
Copilot AI changed the title [WIP] Update LoginComponent to preserve Clerk session between logins Preserve Clerk session between login window opens Apr 9, 2026
await this.electronService.invoke(events.SET_AUTH_TOKEN, token);
}

async setSession(_token: string): Promise<void> {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method discards the passed token. As a result, it never reinstates the session.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7d51337. The stored authToken is a short-lived JWT which Clerk doesn't accept directly for session restoration. Instead, setSession now calls clerk.setActive({ session: sessions[0].id }) when no session is currently active but clerk.client.sessions has available sessions (populated by clerk.load()). If neither path yields an active session, it throws so the login form is shown.

Agent-Logs-Url: https://github.com/highperformancecoder/minsky/sessions/198fce2f-6377-4ea0-a84d-1c4fe6545b1b

Co-authored-by: highperformancecoder <3075825+highperformancecoder@users.noreply.github.com>
@highperformancecoder highperformancecoder marked this pull request as ready for review April 9, 2026 01:42
Copilot AI review requested due to automatic review settings April 9, 2026 01:42
@highperformancecoder highperformancecoder merged commit b1d222b into copilot/create-angular-login-component Apr 9, 2026
@highperformancecoder highperformancecoder deleted the copilot/preserve-clerk-session-login-window branch April 9, 2026 01:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Preserves (or attempts to preserve) an existing Clerk authentication session when reopening the Electron login window, reducing unnecessary re-authentication prompts.

Changes:

  • Adds session-restore logic to ClerkService via a new setSession(token) method.
  • Updates LoginComponent to read an authToken query param and attempt to restore the session on init.
  • Updates WindowManager.openLoginWindow() to append the stored authToken as a URL query parameter when opening the login route.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File Description
gui-js/libs/ui-components/src/lib/login/login.component.ts Reads authToken from route query params and initializes/restores Clerk session on component init.
gui-js/libs/core/src/lib/services/clerk/clerk.service.ts Adds a session-restore helper that attempts to activate an existing Clerk client session.
gui-js/apps/minsky-electron/src/app/managers/WindowManager.ts Includes the stored auth token in the login popup URL to enable renderer-side session restoration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +37 to +43
private destroy$ = new Subject<void>();

constructor(
private clerkService: ClerkService,
private electronService: ElectronService,
private route: ActivatedRoute
) {}
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

destroy$/OnDestroy are currently unused for subscription cleanup (the queryParams subscription uses take(1) and completes automatically). Either remove destroy$ + ngOnDestroy, or switch to takeUntil(this.destroy$) if you intend to keep a long-lived subscription.

Copilot uses AI. Check for mistakes.
Comment on lines 45 to 59
async ngOnInit() {
this.route.queryParams.pipe(take(1)).subscribe((params) => {
this.initializeSession(params['authToken']);
});
}

private async initializeSession(authToken: string | undefined) {
try {
await this.clerkService.initialize();

if (authToken) {
await this.clerkService.setSession(authToken);
}

this.isAuthenticated = await this.clerkService.isSignedIn();
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initializeSession() is async but is invoked from the queryParams subscription without awaiting and without setting an initialization loading state. This allows the user to submit the form before clerkService.initialize() completes, which can cause signInWithEmailPassword() to fail with “Clerk is not initialized”. Consider awaiting initialization in ngOnInit (e.g., via firstValueFrom/await) and/or disabling the form until initialization finishes, and/or calling clerkService.initialize() inside onSubmit() as a guard.

Copilot uses AI. Check for mistakes.
Comment on lines 60 to 63
} catch (err) {
this.errorMessage = 'Failed to initialize authentication.';
this.errorMessage = 'Session expired. Please sign in again.';
this.isAuthenticated = false;
}
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The catch block always shows “Session expired. Please sign in again.” even when the failure is unrelated to session expiry (e.g., Clerk initialization/config/network errors). Consider using a more general message when initialize() fails, and only showing the “session expired” message when a session restore attempt actually fails.

Copilot uses AI. Check for mistakes.
Comment on lines +63 to +74
async setSession(token: string): Promise<void> {
if (!this.clerk) throw new Error('Clerk not initialized');
// clerk.load() in initialize() restores the session from browser storage if still valid.
// If no session is active but sessions exist on the client, activate the first available one.
// The token parameter is a hint that the user previously authenticated.
if (!token) return;
if (!this.clerk.session && this.clerk.client?.sessions?.length > 0) {
await this.clerk.setActive({ session: this.clerk.client.sessions[0].id });
}
if (!this.clerk.session) {
throw new Error('Session expired or invalid');
}
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setSession(token) does not actually use the provided token to reinstate a session; it only checks truthiness and then activates the first session from clerk.client.sessions. This is misleading for callers and makes it hard to reason about correctness/security. Consider renaming to reflect what it does (e.g., restoreSessionFromStorage()), or using the token meaningfully (or dropping the parameter entirely).

Suggested change
async setSession(token: string): Promise<void> {
if (!this.clerk) throw new Error('Clerk not initialized');
// clerk.load() in initialize() restores the session from browser storage if still valid.
// If no session is active but sessions exist on the client, activate the first available one.
// The token parameter is a hint that the user previously authenticated.
if (!token) return;
if (!this.clerk.session && this.clerk.client?.sessions?.length > 0) {
await this.clerk.setActive({ session: this.clerk.client.sessions[0].id });
}
if (!this.clerk.session) {
throw new Error('Session expired or invalid');
}
private getSessionIdFromToken(token: string): string | null {
const parts = token.split('.');
if (parts.length < 2) return null;
try {
const payload = parts[1]
.replace(/-/g, '+')
.replace(/_/g, '/')
.padEnd(Math.ceil(parts[1].length / 4) * 4, '=');
const decoded = JSON.parse(atob(payload));
return typeof decoded?.sid === 'string' ? decoded.sid : null;
} catch {
return null;
}
}
async setSession(token: string): Promise<void> {
if (!this.clerk) throw new Error('Clerk not initialized');
if (!token) throw new Error('Session token is required');
const sessionId = this.getSessionIdFromToken(token);
if (!sessionId) {
throw new Error('Session token is invalid');
}
// clerk.load() in initialize() may already have restored the matching session from browser storage.
if (this.clerk.session?.id === sessionId) {
return;
}
const matchingSession = this.clerk.client?.sessions?.find((session: { id: string }) => session.id === sessionId);
if (!matchingSession) {
throw new Error('Session expired or invalid');
}
await this.clerk.setActive({ session: matchingSession.id });
if (this.clerk.session?.id !== sessionId) {
throw new Error('Session expired or invalid');
}

Copilot uses AI. Check for mistakes.
Comment on lines +63 to +70
async setSession(token: string): Promise<void> {
if (!this.clerk) throw new Error('Clerk not initialized');
// clerk.load() in initialize() restores the session from browser storage if still valid.
// If no session is active but sessions exist on the client, activate the first available one.
// The token parameter is a hint that the user previously authenticated.
if (!token) return;
if (!this.clerk.session && this.clerk.client?.sessions?.length > 0) {
await this.clerk.setActive({ session: this.clerk.client.sessions[0].id });
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Activating this.clerk.client.sessions[0] is an arbitrary choice if multiple sessions exist; it may activate a different session than the one the user expects. If multiple sessions are possible here, prefer selecting the most recently active session or otherwise deterministically choosing the intended session (and document the selection criteria).

Suggested change
async setSession(token: string): Promise<void> {
if (!this.clerk) throw new Error('Clerk not initialized');
// clerk.load() in initialize() restores the session from browser storage if still valid.
// If no session is active but sessions exist on the client, activate the first available one.
// The token parameter is a hint that the user previously authenticated.
if (!token) return;
if (!this.clerk.session && this.clerk.client?.sessions?.length > 0) {
await this.clerk.setActive({ session: this.clerk.client.sessions[0].id });
private getSessionRecencyValue(session: any): number {
const recencyCandidate = session?.lastActiveAt ?? session?.updatedAt ?? session?.createdAt;
if (typeof recencyCandidate === 'number') {
return recencyCandidate;
}
if (recencyCandidate instanceof Date) {
return recencyCandidate.getTime();
}
if (typeof recencyCandidate === 'string') {
const parsedValue = Date.parse(recencyCandidate);
return Number.isNaN(parsedValue) ? Number.NEGATIVE_INFINITY : parsedValue;
}
return Number.NEGATIVE_INFINITY;
}
private selectSessionToActivate(): any | null {
const sessions = this.clerk?.client?.sessions;
if (!Array.isArray(sessions) || sessions.length === 0) return null;
return sessions.reduce((selectedSession: any, currentSession: any) => {
if (!selectedSession) return currentSession;
const selectedRecency = this.getSessionRecencyValue(selectedSession);
const currentRecency = this.getSessionRecencyValue(currentSession);
if (currentRecency > selectedRecency) return currentSession;
if (currentRecency < selectedRecency) return selectedSession;
const selectedId = String(selectedSession?.id ?? '');
const currentId = String(currentSession?.id ?? '');
return currentId.localeCompare(selectedId) < 0 ? currentSession : selectedSession;
}, null);
}
async setSession(token: string): Promise<void> {
if (!this.clerk) throw new Error('Clerk not initialized');
// clerk.load() in initialize() restores the session from browser storage if still valid.
// If no session is active but sessions exist on the client, activate one deterministically.
// Prefer the most recently active session (`lastActiveAt`), then `updatedAt`, then `createdAt`;
// if timestamps are unavailable, fall back to the lexicographically smallest session id.
// The token parameter is a hint that the user previously authenticated.
if (!token) return;
if (!this.clerk.session && this.clerk.client?.sessions?.length > 0) {
const sessionToActivate = this.selectSessionToActivate();
if (sessionToActivate?.id) {
await this.clerk.setActive({ session: sessionToActivate.id });
}

Copilot uses AI. Check for mistakes.
Comment on lines +63 to +74
async setSession(token: string): Promise<void> {
if (!this.clerk) throw new Error('Clerk not initialized');
// clerk.load() in initialize() restores the session from browser storage if still valid.
// If no session is active but sessions exist on the client, activate the first available one.
// The token parameter is a hint that the user previously authenticated.
if (!token) return;
if (!this.clerk.session && this.clerk.client?.sessions?.length > 0) {
await this.clerk.setActive({ session: this.clerk.client.sessions[0].id });
}
if (!this.clerk.session) {
throw new Error('Session expired or invalid');
}
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throwing Session expired or invalid when no session becomes active after attempting restore forces the caller to treat “no restorable session” as an exceptional condition. For this workflow it’s usually better to return a boolean (restored vs not) and let the UI fall back to the login form without showing an error unless the user action fails.

Suggested change
async setSession(token: string): Promise<void> {
if (!this.clerk) throw new Error('Clerk not initialized');
// clerk.load() in initialize() restores the session from browser storage if still valid.
// If no session is active but sessions exist on the client, activate the first available one.
// The token parameter is a hint that the user previously authenticated.
if (!token) return;
if (!this.clerk.session && this.clerk.client?.sessions?.length > 0) {
await this.clerk.setActive({ session: this.clerk.client.sessions[0].id });
}
if (!this.clerk.session) {
throw new Error('Session expired or invalid');
}
async setSession(token: string): Promise<boolean> {
if (!this.clerk) throw new Error('Clerk not initialized');
// clerk.load() in initialize() restores the session from browser storage if still valid.
// If no session is active but sessions exist on the client, activate the first available one.
// The token parameter is a hint that the user previously authenticated.
if (!token) return false;
if (!this.clerk.session && this.clerk.client?.sessions?.length > 0) {
await this.clerk.setActive({ session: this.clerk.client.sessions[0].id });
}
return !!this.clerk.session;

Copilot uses AI. Check for mistakes.
Comment on lines +392 to 400
const existingToken = StoreManager.store.get('authToken') || '';

const loginWindow = WindowManager.createPopupWindowWithRouting({
width: 420,
height: 500,
title: 'Login',
modal: false,
url: '#/headless/login',
url: `#/headless/login?authToken=${encodeURIComponent(existingToken)}`,
});
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing the (potentially long-lived) authToken in the login window URL query string risks accidental disclosure via logs, crash reports, screenshots, devtools, or navigation history. Prefer keeping tokens out of URLs: fetch the stored token from the main process via IPC after the window loads, or rely on Clerk’s own storage restoration without transmitting the token to the renderer.

Copilot uses AI. Check for mistakes.
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.

3 participants