From 09cf145ec8c0d3bae209ef922dfd5b7d9379228a Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Tue, 15 Sep 2026 00:03:38 +0530 Subject: [PATCH] refactor(desktop): tighten runtime-discovery async and attempt guards isActiveRuntimeUsable relied on async-return flattening for the trailing canImportHermesCli promise: correct today, but appending any further `&&` operand would have made the expression truthy regardless of the probe. Await the probe explicitly so the intent survives future edits, and mark unwrapWindowsVenvHermesCommand async like its siblings since it always returns a promise. connectRemote used two inline isCurrentAttempt/throw pairs with the same message that backendConnectionState.assertCurrentAttempt already emits, and the third guard in the same function already uses the helper. Use it in all three places so the superseded-attempt message has one source. The fast-path/probe/cache explanation for serve support lived above the resolver call site in main.ts while the logic lives in backend-serve-support.ts; move it next to the code and leave a pointer. --- .../desktop/electron/backend-serve-support.ts | 10 ++++++++ apps/desktop/electron/main.ts | 23 ++++++++----------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/apps/desktop/electron/backend-serve-support.ts b/apps/desktop/electron/backend-serve-support.ts index 175e6fa887..ca7567a7e0 100644 --- a/apps/desktop/electron/backend-serve-support.ts +++ b/apps/desktop/electron/backend-serve-support.ts @@ -13,6 +13,16 @@ interface ServeCandidate { label?: string } +// Does the resolved runtime understand the `serve` subcommand? The desktop +// spawns `hermes serve`; runtimes older than serve only have `dashboard`, so +// main.ts routes those through the legacy `dashboard --no-open` form instead +// of crashing on an unknown subcommand. +// +// Fast path: read the runtime's own dashboard.py (instant, covers managed +// installs, dev checkouts, and the Windows venv). Fallback: probe the CLI once +// (covers a bare `hermes` resolved from PATH with no known source root). Result +// is cached per resolved runtime so we probe at most once per backend. +// // One cache per desktop runtime context; source inspection precedes a CLI probe. export function createBackendServeSupportResolver(hermesHome: string, rememberLog: (message: string) => void) { const cache = new Map>() diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index f29812322d..a7306b0cbd 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -2558,7 +2558,7 @@ function isCommandScript(command) { return IS_WINDOWS && /\.(cmd|bat)$/i.test(command || '') } -function unwrapWindowsVenvHermesCommand(command, backendArgs) { +async function unwrapWindowsVenvHermesCommand(command, backendArgs) { return resolveVenvHermesCommand(command, backendArgs, { isWindows: IS_WINDOWS, isCommandScript, @@ -2581,11 +2581,7 @@ function unwrapWindowsVenvHermesCommand(command, backendArgs) { // detect support so getBackendArgsForRuntime() can route old runtimes through // the legacy `dashboard --no-open` form instead of crashing on an unknown // subcommand (would brick every user mid-upgrade — #54568 follow-up). -// -// Fast path: read the runtime's own dashboard.py (instant, covers managed -// installs, dev checkouts, and the Windows venv). Fallback: probe the CLI once -// (covers a bare `hermes` resolved from PATH with no known source root). Result -// is cached per resolved runtime so we probe at most once per backend. +// Fast-path / probe / cache strategy: see backend-serve-support.ts header. const backendSupportsServe = createBackendServeSupportResolver(HERMES_HOME, rememberLog) // Given a resolved backend whose args target `serve`, return the args the @@ -4674,11 +4670,14 @@ async function isActiveRuntimeUsable() { return ( isHermesSourceRoot(ACTIVE_HERMES_ROOT) && fileExists(venvPython) && - canImportHermesCli(venvPython, { + // Explicit await: a bare promise as the last `&&` operand only works via + // async-return flattening; any operand appended after it would make the + // expression truthy regardless of the probe result. + (await canImportHermesCli(venvPython, { env: { PYTHONPATH: [ACTIVE_HERMES_ROOT, process.env.PYTHONPATH].filter(Boolean).join(path.delimiter) } - }) + })) ) } @@ -12876,18 +12875,14 @@ async function runHermesStart() { // resolveRemote() may take arbitrarily long (settings resolve / ws-ticket // mint). If a newer attempt started meanwhile (e.g. the user switched // remotes and Apply invalidated this attempt), bail before probing. - if (!backendConnectionState.isCurrentAttempt(connectionAttempt)) { - throw new Error('Hermes backend start was superseded by a newer connection attempt.') - } + backendConnectionState.assertCurrentAttempt(connectionAttempt) await advanceBootProgress('backend.remote', `Connecting to remote Hermes backend at ${remote.baseUrl}`, 24) await waitForHermes(remote.baseUrl, remote.token, undefined, remote.authMode, remote.headers) // Second async boundary: the health probe itself can outlive the // attempt. A late success here must not publish a stale descriptor. - if (!backendConnectionState.isCurrentAttempt(connectionAttempt)) { - throw new Error('Hermes backend start was superseded by a newer connection attempt.') - } + backendConnectionState.assertCurrentAttempt(connectionAttempt) updateBootProgress({ phase: 'backend.ready',