Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
13a1d2d
Initial plan
Copilot Mar 31, 2026
6e0e17e
Add Clerk authentication integration with Angular login component and…
Copilot Mar 31, 2026
8a03f24
Merge branch 'master' into copilot/create-angular-login-component
highperformancecoder Mar 31, 2026
3ebc404
Added a "login to clerk" menu item, and added a clerk publishable key.
highperformancecoder Mar 31, 2026
9d1d261
Fix login window not rendering by using createPopupWindowWithRouting
Copilot Mar 31, 2026
62a9ead
Fix login window showing app shell (header/tabs) by excluding login r…
Copilot Mar 31, 2026
f9b417d
Move login route to headless/login for consistency with other popup w…
Copilot Mar 31, 2026
6f7259d
Add openLoginWindow() promise that resolves when auth token is stored
Copilot Mar 31, 2026
60f50ea
Rearrange promise from events to WindowManager
highperformancecoder Mar 31, 2026
8da2877
Close login dialog window once submitted.
highperformancecoder Mar 31, 2026
59e8663
Got the upgradeUsingClerk process working.
highperformancecoder Apr 1, 2026
6539e6e
Dead code removal
highperformancecoder Apr 1, 2026
8663d10
More dead code removal.
highperformancecoder Apr 1, 2026
c5fae66
Merge branch 'master' into copilot/create-angular-login-component
highperformancecoder Apr 8, 2026
5936fe1
Address review comments from CodeRabbit.
highperformancecoder Apr 9, 2026
32fbb2d
Initial plan
Copilot Apr 9, 2026
8852185
Preserve Clerk session between login window opens
Copilot Apr 9, 2026
7d51337
Fix setSession to actually reinstate the Clerk session
Copilot Apr 9, 2026
b1d222b
Merge pull request #642 from highperformancecoder/copilot/preserve-cl…
highperformancecoder Apr 9, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions gui-js/apps/minsky-electron/src/app/events/electron.events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -288,3 +288,16 @@ ipcMain.handle(events.OPEN_URL, (event,options)=> {
let window=WindowManager.createWindow(options);
window.loadURL(options.url);
});

ipcMain.handle(events.SET_AUTH_TOKEN, async (event, token: string | null) => {
if (token) {
StoreManager.store.set('authToken', token);
} else {
StoreManager.store.delete('authToken');
}
if (WindowManager._resolveAuthToken) {
WindowManager._resolveAuthToken(token);
WindowManager._resolveAuthToken = null;
}
return { success: true };
});
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,15 @@ export class ApplicationMenuManager {
window.loadURL('https://www.patreon.com/logout');
},
},
{
label: 'Upgrade via Clerk',
click() {CommandsManager.upgradeUsingClerk();},
},
{
label: 'Manage Clerk Session',
click() {WindowManager.openLoginWindow();},
},

{
label: 'New System',
accelerator: 'CmdOrCtrl + Shift + N',
Expand Down
218 changes: 184 additions & 34 deletions gui-js/apps/minsky-electron/src/app/managers/CommandsManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,67 @@
import {exec,spawn} from 'child_process';
import decompress from 'decompress';
import {promisify} from 'util';
import {net, safeStorage } from 'electron';

function semVer(version: string) {
const pattern=/(\d+)\.(\d+)\.(\d+)/;
let [,major,minor,patch]=pattern.exec(version);
return {major: +major,minor: +minor, patch: +patch};
}
function semVerLess(x: string, y: string): boolean {
let xver=semVer(x), yver=semVer(y);
return xver.major<yver.major ||
xver.major===yver.major && (
xver.minor<yver.minor ||
xver.minor===yver.minor && xver.patch<yver.patch
);
}

const backendAPI='https://minskybe-x7dj1.sevalla.app/api';
// perform a call on the backend API, returning the JSON encoded result
// options is passed to the constructor of a ClienRequest object https://www.electronjs.org/docs/latest/api/client-request#requestendchunk-encoding-callback
async function callBackendAPI(options: string|Object, token: string) {
return new Promise<string>((resolve, reject)=> {
let request=net.request(options);
request.setHeader('Authorization',`Bearer ${token}`);
request.on('response', (response)=>{
let chunks=[];
response.on('data', (chunk)=>{chunks.push(chunk);});
response.on('end', ()=>resolve(Buffer.concat(chunks).toString()));
response.on('error',()=>reject(response.statusMessage));
});
request.on('error',(err)=>reject(err.toString()));
request.end();
});
}

// to handle redirects
async function getFinalUrl(initialUrl, token) {
try {
const response = await fetch(initialUrl, {
method: 'GET',
headers: {
'Authorization': `Bearer ${token}`
},
redirect: 'manual' // This tells fetch NOT to follow the link automatically
});

// In 'manual' mode, a redirect returns an 'opaqueredirect' type or status 302
if (response.status >= 300 && response.status < 400) {
const redirectUrl = response.headers.get('location');
if (redirectUrl) return redirectUrl;
}

if (response.ok) return initialUrl;

throw new Error(`Server responded with ${response.status}`);
} catch (error) {
// If redirect: 'manual' is used, fetch might throw a 'TypeError'
// when it hits the redirect—this is actually what we want to catch.
console.error("Fetch encountered the redirect/error:", error);
throw error;
}
}

export class CommandsManager {
static activeGodleyWindowItems = new Map<string, CanvasItem>();
Expand Down Expand Up @@ -1137,6 +1198,7 @@

// handler for downloading Ravel and installing it
static downloadRavel(event,item,webContents) {

switch (process.platform) {
case 'win32':
const savePath=dirname(process.execPath)+'/libravel.dll';
Expand All @@ -1158,7 +1220,7 @@
// handler for when download completed
item.once('done', (event,state)=>{
progress.close();

if (state==='completed') {
dialog.showMessageBoxSync(WindowManager.getMainWindow(),{
message: 'Ravel plugin updated successfully - restart Ravel to use',
Expand Down Expand Up @@ -1308,6 +1370,44 @@
modal: false,
});
}

// return information about the current system
static async buildState(previous: boolean) {
// need to pass what platform we are
let state;
switch (process.platform) {
case 'win32':
state={system: 'windows', distro: '', version: '', arch:'', previous: ''};
break;
case 'darwin':
state={system: 'macos', distro: '', version: '', arch: `${process.arch}`, previous: ''};
break;
case 'linux': {
state={system: 'linux', distro: '', version: '',arch:'', previous: ''};
// figure out distro and version from /etc/os-release
let aexec=promisify(exec);
let osRelease='/etc/os-release';
if (existsSync(process.resourcesPath+'/os-release'))
osRelease=process.resourcesPath+'/os-release';
let distroInfo=await aexec(`grep ^ID= ${osRelease}`);
// value may or may not be quoted
let extractor=/.*=['"]?([^'"\n]*)['"]?/;
state.distro=extractor.exec(distroInfo.stdout)[1];
distroInfo=await aexec(`grep ^VERSION_ID= ${osRelease}`);
state.version=extractor.exec(distroInfo.stdout)[1];
break;
}
default:
dialog.showMessageBoxSync(WindowManager.getMainWindow(),{
message: `In app update is not available for your operating system yet, please check back later`,
type: 'error',
});
return null;
}
if (await minsky.ravelAvailable() && previous)
state.previous=/[^:]*/.exec(await minsky.ravelVersion())[0];
return state;
}

static async upgrade(installCase: InstallCase=InstallCase.theLot) {
const window=this.createDownloadWindow();
Expand Down Expand Up @@ -1344,7 +1444,7 @@
}
if (ravelFile) {
// currently on latest, so reinstall ravel
window.webContents.session.on('will-download',this.downloadRavel);
window.webContents.session.on('will-download',this.downloadRavel);
window.webContents.downloadURL(ravelFile);
return;
}
Expand All @@ -1357,44 +1457,94 @@
}
});

let clientId='-PiL7snNmZL_BlLJTPm62SHBcFTMG5d46m2336r118mfrp6sz4ty0g-thbKAs76c';
// need to pass what platform we are
let state;
switch (process.platform) {
case 'win32':
state={system: 'windows', distro: '', version: '', arch:'', previous: ''};
break;
case 'darwin':
state={system: 'macos', distro: '', version: '', arch: `${process.arch}`, previous: ''};
break;
case 'linux':
state={system: 'linux', distro: '', version: '',arch:'', previous: ''};
// figure out distro and version from /etc/os-release
let aexec=promisify(exec);
let osRelease='/etc/os-release';
if (existsSync(process.resourcesPath+'/os-release'))
osRelease=process.resourcesPath+'/os-release';
let distroInfo=await aexec(`grep ^ID= ${osRelease}`);
// value may or may not be quoted
let extractor=/.*=['"]?([^'"\n]*)['"]?/;
state.distro=extractor.exec(distroInfo.stdout)[1];
distroInfo=await aexec(`grep ^VERSION_ID= ${osRelease}`);
state.version=extractor.exec(distroInfo.stdout)[1];
break;
default:
dialog.showMessageBoxSync(WindowManager.getMainWindow(),{
message: `In app update is not available for your operating system yet, please check back later`,
type: 'error',
});
let state=await CommandsManager.buildState(installCase==InstallCase.previousRavel);
if (!state) {
window.close();
return;
break;
}
if (await minsky.ravelAvailable() && installCase===InstallCase.previousRavel)
state.previous=/[^:]*/.exec(await minsky.ravelVersion())[0];
let clientId='-PiL7snNmZL_BlLJTPm62SHBcFTMG5d46m2336r118mfrp6sz4ty0g-thbKAs76c';
let encodedState=encodeURI(JSON.stringify(state));
// load patreon's login page
window.loadURL(`https://www.patreon.com/oauth2/authorize?response_type=code&client_id=${clientId}&redirect_uri=https://ravelation.net/ravel-downloader.cgi&scope=identity%20identity%5Bemail%5D&state=${encodedState}`);
}


// gets release URL for current system from Ravelation.net backend
static async getRelease(product: string, previous: boolean, token: string) {
let state=await CommandsManager.buildState(previous);
if (!state) return '';
let query=`product=${product}&os=${state.system}&arch=${state.arch}&distro=${state.distro}&distro_version=${state.version}`;
if (previous) {
let releases=JSON.parse(await callBackendAPI(`${backendAPI}/releases?${query}`, token));
let prevRelease;
for (let release of releases)
if (semVerLess(release.version, state.previous))
prevRelease=release;
if (prevRelease) return prevRelease.download_url;
// if not, then treat the request as latest
}
let release=JSON.parse(await callBackendAPI(`${backendAPI}/releases/latest?${query}`, token));
return release?.release?.download_url;
}

static stashClerkToken(token: string) {
if (token) {
if (safeStorage.isEncryptionAvailable()) {
const encrypted = safeStorage.encryptString(token);
StoreManager.store.set('authToken', encrypted.toString('latin1'));
} else
// fallback: store plaintext
StoreManager.store.set('authToken', token);
} else {
StoreManager.store.delete('authToken');
}
}

static async upgradeUsingClerk(installCase: InstallCase=InstallCase.theLot) {
while (!StoreManager.store.get('authToken'))
if (!await WindowManager.openLoginWindow()) {
let response=await dialog.showMessageBox(WindowManager.getMainWindow(),{
message: 'Login failed',
type: 'error',
buttons: ['Cancel','Try again'],
title: 'Login failed',
});
if (response.response===0) break;
}
let token=StoreManager.store.get('authToken');
Comment on lines +1503 to +1514
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Let the user cancel login.

openLoginWindow() resolves null when the popup closes, but this loop ignores that value and immediately opens another window. As written, closing the dialog never cancels the upgrade.

↩️ Suggested fix
-    while (!StoreManager.store.get('authToken'))
-      await WindowManager.openLoginWindow();
-    let token=StoreManager.store.get('authToken');
+    let token=StoreManager.store.get('authToken');
+    while (!token) {
+      token=await WindowManager.openLoginWindow();
+      if (!token) return;
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gui-js/apps/minsky-electron/src/app/managers/CommandsManager.ts` around lines
1491 - 1494, The loop in CommandsManager.upgradeUsingClerk continuously calls
WindowManager.openLoginWindow() until StoreManager.store.get('authToken')
exists, but openLoginWindow() can resolve null when the user closes the popup;
update upgradeUsingClerk to capture the result of
WindowManager.openLoginWindow(), check for null, and break/return/throw to allow
cancelation instead of immediately reopening the dialog—use the returned value
from openLoginWindow() (or a boolean) before re-checking
StoreManager.store.get('authToken') so a closed popup stops the loop and cancels
the upgrade flow.

// decrypt token if encrypted
if (safeStorage.isEncryptionAvailable())
token=safeStorage.decryptString(Buffer.from(token, 'latin1'));

const window=WindowManager.getMainWindow();
let minskyAsset;
try {
if (installCase===InstallCase.theLot)
minskyAsset=await CommandsManager.getRelease('minsky', false, token);
let ravelAsset=await CommandsManager.getRelease('ravel', installCase===InstallCase.previousRavel, token);

if (minskyAsset) {
if (ravelAsset) { // stash ravel upgrade to be installed on next startup
StoreManager.store.set('ravelPlugin',await getFinalUrl(ravelAsset,token));
}
window.webContents.session.on('will-download',this.downloadMinsky);
window.webContents.downloadURL(await getFinalUrl(minskyAsset,token));
return;
} else if (ravelAsset) {
window.webContents.session.on('will-download',this.downloadRavel);
window.webContents.downloadURL(await getFinalUrl(ravelAsset,token));
return;
}
dialog.showMessageBoxSync(WindowManager.getMainWindow(),{
message: "Everything's up to date, nothing to do.\n"+
"If you're trying to download the Ravel plugin, please ensure you are logged into an account subscribed to Ravel Fan or Explorer tiers.",
type: 'info',
});
}
catch (error) {
dialog.showErrorBox('Error', error.toString());
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ interface MinskyStore {
defaultModelDirectory: string;
defaultDataDirectory: string;
ravelPlugin: string; // used for post installation installation of Ravel
authToken?: string;
}

class StoreManager {
Expand Down
29 changes: 29 additions & 0 deletions gui-js/apps/minsky-electron/src/app/managers/WindowManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ export class WindowManager {
static canvasWidth: number;
static scaleFactor: number;
static currentTab=minsky.canvas as RenderNativeWindow;
// Pending resolver for the auth-token promise created by openLoginWindow()
static _resolveAuthToken: ((token: string | null) => void) | null = null;
Comment on lines +31 to +32
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 2, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Reuse the in-flight login promise/window.

A second openLoginWindow() call replaces _resolveAuthToken before the first promise settles. The first caller then waits forever, and the earlier popup becomes orphaned. This is reachable because both the upgrade flow and the menu can open the login window. Return the existing pending promise instead of overwriting the resolver.

Also applies to: 387-398

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gui-js/apps/minsky-electron/src/app/managers/WindowManager.ts` around lines
31 - 32, openLoginWindow currently overwrites the shared resolver
_resolveAuthToken when called a second time, orphaning the first caller; fix by
detecting an in-flight auth promise/resolver and returning that existing promise
instead of creating a new one, and only create/assign a new _resolveAuthToken
and _authTokenPromise when none exists; ensure you also reuse any existing
_loginWindow instead of creating a second popup (see references to
openLoginWindow, _resolveAuthToken, _authTokenPromise and _loginWindow and the
similar logic around lines 387-398) so multiple callers share the same
promise/window rather than replacing the resolver.

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.

openLoginWindow does return the promise. _resolveToken stores the returned token. It shouldn't matter if this gets overwritten, as node does this atomically, IIUC.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@highperformancecoder, you're absolutely right that Node.js's single-threaded event loop makes the assignment itself atomic — no torn writes or data races there.

The concern is a logical one rather than a concurrency/atomicity one. Here's what happens with two sequential (but overlapping) calls:

  1. Call 1: promise1 is created; its internal resolve1 is stored in _resolveAuthToken. promise1 is returned to caller 1.
  2. Call 2: _resolveAuthToken is overwritten with resolve2. At this point, resolve1 is no longer reachable from anywhere — the only reference to it was _resolveAuthToken, and that's gone.
  3. When the IPC handler fires _resolveAuthToken(token), it calls resolve2promise2 settles. ✅
  4. promise1 never settles — caller 1 awaits it forever. 🔴

The window1.once('closed') handler also checks _resolveAuthToken, which by now either points to resolve2 or is already null, so it doesn't help promise1 either.

That said, if the application ensures only one login window can be opened at a time (e.g., it's purely user-triggered via a single menu item and the menu is disabled while the window is open), this is low-risk in practice. If you're confident that's the case, the current code is fine for practical purposes.


🧠 Learnings used
Learnt from: highperformancecoder
Repo: highperformancecoder/minsky PR: 630
File: RESTService/addon.cc:255-264
Timestamp: 2026-03-22T22:13:42.606Z
Learning: In Minsky (model/renderNativeWindow.cc, model/minsky.cc), `RenderNativeWindow::requestRedraw()` may only ever be called on the minsky thread (not from the RK/simulation thread). The simulation thread already explicitly avoids calling `requestRedraw()` via the guard `if (!RKThreadRunning) canvas.requestRedraw()` at model/minsky.cc:1276. If this invariant holds everywhere, the data race on `nativeWindowsToRedraw` reported in PR `#630` does not exist, and the swap-before-lock pattern introduced by that PR (releasing `nativeWindowsToRedrawMutex` before acquiring `minskyCmdMutex`) creates a new use-after-free hazard that is worse than the original code.

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.

_resolveAuthToken is only overwritten when the promise resolves. The promise is not stored there. Let's say promise1 resolves first, then _resolveAuthToken is updated to that value. promise 2 is still alive, and being awaited on. Meanwhile, login window 1 proceeds, as it has the token. Then promise 2 resolves, _resolveAuthToken is updated with the exact same token, and the second upgradeUsingClerk method continues, duplicating everything the first method has done. I don't see the problem here.


static activeWindows = new Map<number, ActiveWindow>();
private static uidToWindowMap = new Map<string, ActiveWindow>();
Expand Down Expand Up @@ -381,4 +383,31 @@ export class WindowManager {
catch (err) {} // absorb any exceptions due to windows disappearing
}
}

static async openLoginWindow() {
const promise = new Promise<string | null>((resolve) => {
WindowManager._resolveAuthToken = resolve;
});

const existingToken = StoreManager.store.get('authToken') || '';

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

// Resolve with null if the user closes the window before authenticating
loginWindow.once('closed', () => {
if (WindowManager._resolveAuthToken) {
WindowManager._resolveAuthToken(null);
WindowManager._resolveAuthToken = null;
}
});

return promise;
}

}
7 changes: 6 additions & 1 deletion gui-js/apps/minsky-web/src/app/app-routing.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ import {
EditHandleDescriptionComponent,
EditHandleDimensionComponent,
PickSlicesComponent,
LockHandlesComponent
LockHandlesComponent,
LoginComponent,
} from '@minsky/ui-components';

const routes: Routes = [
Expand Down Expand Up @@ -149,6 +150,10 @@ const routes: Routes = [
path: 'headless/variable-pane',
component: VariablePaneComponent,
},
{
path: 'headless/login',
component: LoginComponent,
},
{
path: '**',
component: PageNotFoundComponent,
Expand Down
2 changes: 1 addition & 1 deletion gui-js/apps/minsky-web/src/app/app.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
}

@if (!loading) {
@if (router.url.includes('headless');) {
@if (router.url.includes('headless')) {
<router-outlet></router-outlet>
}
@else {
Expand Down
1 change: 1 addition & 0 deletions gui-js/apps/minsky-web/src/environments/environment.dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@
export const AppConfig = {
production: false,
environment: 'DEV',
clerkPublishableKey: 'pk_test_cG9zaXRpdmUtcGhvZW5peC04NS5jbGVyay5hY2NvdW50cy5kZXYk',
};
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export const AppConfig = {
production: true,
environment: 'PROD',
clerkPublishableKey: 'pk_test_cG9zaXRpdmUtcGhvZW5peC04NS5jbGVyay5hY2NvdW50cy5kZXYk',
};
1 change: 1 addition & 0 deletions gui-js/apps/minsky-web/src/environments/environment.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export const AppConfig = {
production: false,
environment: 'LOCAL',
clerkPublishableKey: 'pk_test_cG9zaXRpdmUtcGhvZW5peC04NS5jbGVyay5hY2NvdW50cy5kZXYk',
};
Loading
Loading