diff --git a/.github/workflows/desktop-bundled-release.yml b/.github/workflows/desktop-bundled-release.yml index ba76570d79..9f5bacfc02 100644 --- a/.github/workflows/desktop-bundled-release.yml +++ b/.github/workflows/desktop-bundled-release.yml @@ -7,20 +7,19 @@ name: Desktop Bundled Release # files + receipts to R2 (releases/tag//) → gated publish jobs write # feeds → builds table. Stable candidates stop before feed publication. # -# Per-OS topology (MSIX work is gated on the WINDOWS build only — mac/linux -# legs never block the win32 feed or Store submission): +# Per-OS topology (canary feeds follow their own platform builds; stable +# publication and Store submission require the accepted cross-platform candidate): # # build-win32 (win32-x64 + win32-arm64) → stage packages + receipts to R2 # build-darwin (darwin-arm64 + darwin-x64) → sign + notarize + stage # dmg/zip/blockmap + per-arch feed inputs to the R2 tag archive # build-linux → DISABLED for now (dummy skips) -# publish-win32-updater → App Installer feed -# (needs build-win32): releases/win32//*.appinstaller + -# *.msixbundle (stage-msixbundle.mjs --variant bundled) -# publish-win32-store → Windows Store submission -# (needs build-win32, PARALLEL with the updater feed): bundle the two -# Store-*.msix into one universal Store .msixbundle and submit via the -# MSStore CLI. Only stable is eligible, gated on MS_STORE_PRODUCT_ID. +# publish-win32-updater → native universal bundles +# (needs build-win32): canary also publishes its App Installer feed; +# stable candidates stage sideload + Store bundles without touching feeds. +# stable-store → verified Store submission +# (needs stable-publish): materialize the immutable accepted Store bundle +# and submit via the MSStore CLI without rebuilding. # publish-darwin-updater → macOS electron-updater feed # (needs BOTH darwin legs): scripts.releases.r2 finalize merges the per-arch # ymls into releases/darwin//-mac.yml — the feed @@ -29,7 +28,7 @@ name: Desktop Bundled Release # # Feed layout (matches apps/desktop/electron/app-updater.ts's arms): # releases/win32//.appinstaller App Installer feed -# releases/win32//*.msixbundle (publish-win32-updater) +# canary bundles live beside the feed; stable points into releases/tag// # releases/darwin//-mac.yml electron-updater feed # (dmg/zip live once in releases/tag//; the merged feed points at # them with absolute object keys) @@ -59,13 +58,13 @@ name: Desktop Bundled Release # non-secret vars. scripts/releases/r2.py derives the S3 endpoint from # the account id. Upload/list operations need only Python; feed operations use ruamel.yaml. # -# Windows Store submission (publish-win32-store): MSStore CLI via +# Windows Store submission (stable-store): MSStore CLI via # microsoft/microsoft-store-apppublisher. Credentials live in the # release-signing environment: # secrets: MS_STORE_TENANT_ID, MS_STORE_SELLER_ID, MS_STORE_CLIENT_ID, # MS_STORE_CLIENT_SECRET # vars: MS_STORE_PRODUCT_ID (the Partner Center product ID) -# msstore reconfigure → (delete pending) → msstore publish +# msstore reconfigure → msstore publish # -id # The Store bundle is UNSIGNED on purpose — Partner Center re-signs on # ingestion (see sign-msix.mjs). @@ -1037,148 +1036,6 @@ jobs: --name windows-universal --root apps/desktop/release --include '*.msixbundle' fi - # ── Windows Store submission (REAL — PARALLEL with publish-win32-updater) ─ - # Bundles the two Store-*.msix into one universal Store .msixbundle and - # submits it to the Windows Store via the MSStore CLI. - # - # Only stable releases can claim the official Partner Center identity. - # Canary and commit builds are sideload-only, even when a flight exists. - publish-win32-store: - name: Publish the stable Windows Store submission - needs: [validate, build-win32] - if: | - inputs.build_commit == '' - && inputs.upload_release == true - && vars.MS_STORE_PRODUCT_ID != '' - && contains(inputs.tag, '-canary.') == false - runs-on: windows-2025 - environment: release-signing - timeout-minutes: 60 - env: - HERMES_PAYLOAD_TAG: ${{ inputs.tag }} - ELECTRON_BUILDER_CACHE: ${{ github.workspace }}/.cache/electron-builder - CLOUDFLARE_R2_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_R2_ACCOUNT_ID }} - CLOUDFLARE_R2_ACCESS_KEY_ID: ${{ secrets.CLOUDFLARE_R2_ACCESS_KEY_ID }} - CLOUDFLARE_R2_SECRET_ACCESS_KEY: ${{ secrets.CLOUDFLARE_R2_SECRET_ACCESS_KEY }} - CLOUDFLARE_R2_BUCKET: ${{ vars.CLOUDFLARE_R2_BUCKET }} - CLOUDFLARE_R2_PUBLIC_URL: ${{ vars.CLOUDFLARE_R2_PUBLIC_URL }} - MS_STORE_TENANT_ID: ${{ secrets.MS_STORE_TENANT_ID }} - MS_STORE_SELLER_ID: ${{ secrets.MS_STORE_SELLER_ID }} - MS_STORE_CLIENT_ID: ${{ secrets.MS_STORE_CLIENT_ID }} - MS_STORE_CLIENT_SECRET: ${{ secrets.MS_STORE_CLIENT_SECRET }} - MS_STORE_PRODUCT_ID: ${{ vars.MS_STORE_PRODUCT_ID }} - steps: - - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - # Privileged job: pin to the SHA validate admitted, not the tag. - ref: ${{ needs.validate.outputs.sha }} - fetch-tags: true - - - name: Set up the locked build toolchain - uses: ./.github/actions/setup-pm - with: - toolchain: all - archive-inputs: true - cache-python: false - - - name: Install the locked Windows bundle tooling - uses: ./.github/actions/retry - with: - # No Electron/native postinstalls: only the builder's SDK downloader. - command: npm ci --workspace apps/desktop --include-workspace-root --include=dev --ignore-scripts --no-audit --no-fund - - - name: Resolve toolchain cache key - id: toolchain - shell: bash - run: | - node -e ' - const l = require("./package-lock.json") - const el = l.packages["apps/desktop/node_modules/electron"].version - const eb = l.packages["node_modules/electron-builder"].version - if (!el || !eb) process.exit(1) - console.log(`electron=${el}`) - console.log(`builder=${eb}`) - ' >> "$GITHUB_OUTPUT" - - # Reuse the build cache when available. The bundle script also works - # cold by provisioning the same SDK through the pinned builder. - - name: Resolve electron's default download cache path - shell: bash - run: | - case "$RUNNER_OS" in - Windows) echo "ELECTRON_DEFAULT_CACHE=$LOCALAPPDATA/electron/Cache" >> "$GITHUB_ENV" ;; - macOS) echo "ELECTRON_DEFAULT_CACHE=$HOME/Library/Caches/electron" >> "$GITHUB_ENV" ;; - *) echo "ELECTRON_DEFAULT_CACHE=$HOME/.cache/electron" >> "$GITHUB_ENV" ;; - esac - - - name: Cache electron + electron-builder toolchain - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 - with: - path: | - ${{ github.workspace }}/.cache/electron-builder - ${{ github.workspace }}/.cache/electron - ${{ env.ELECTRON_DEFAULT_CACHE }} - key: eb2-${{ runner.os }}-${{ runner.arch }}-electron-${{ steps.toolchain.outputs.electron }}-builder-${{ steps.toolchain.outputs.builder }} - restore-keys: | - eb2-${{ runner.os }}-${{ runner.arch }}- - - - name: Retrieve Store packages from R2 - shell: bash - env: - RELEASE_COMMIT: ${{ needs.validate.outputs.sha }} - run: | - python -m scripts.releases.handoff fetch --tag "$HERMES_PAYLOAD_TAG" --commit "$RELEASE_COMMIT" \ - --name win32-x64 --name win32-arm64 --root apps/desktop/release --include 'Store-*.msix' - - - name: Bundle the Store submission MSIX - id: storebundle - shell: bash - run: | - # The SDK downloader logs to stdout; the path has its own output. - result="$RUNNER_TEMP/store-bundle-path" - node scripts/bundle-store-msixbundle.mjs --tag "$HERMES_PAYLOAD_TAG" --output-file "$result" - bundle="$(< "$result")" - test -f "$bundle" - echo "bundle=$bundle" >> "$GITHUB_OUTPUT" - echo "Store bundle: $bundle" - - - name: Archive the Store bundle to the tag dir - shell: bash - run: | - # The per-arch Store-*.msix are already in the immutable archive - # (uploaded by the build legs); keep the assembled universal bundle - # there too as the record of exactly what was submitted. - python -m scripts.releases.r2 put \ - --tag "$HERMES_PAYLOAD_TAG" \ - --key "$(basename "${{ steps.storebundle.outputs.bundle }}")" \ - --file "${{ steps.storebundle.outputs.bundle }}" - - - name: Setup MSStore CLI - uses: microsoft/microsoft-store-apppublisher@cc9910a8d59f2eb55cbb83df0a3800cf3b5300e0 # v1.4 - - - name: Configure Store credentials - shell: bash - run: | - msstore reconfigure \ - --tenantId "$MS_STORE_TENANT_ID" \ - --sellerId "$MS_STORE_SELLER_ID" \ - --clientId "$MS_STORE_CLIENT_ID" \ - --clientSecret "$MS_STORE_CLIENT_SECRET" - - - name: Publish the Store submission - shell: bash - run: | - # The universal Store .msixbundle is accepted directly by msstore - # publish (PackageFilesExtensionInclude: .msix/.msixbundle/.msixupload). - # Partner Center signs on ingestion — no signing here. - # - # Partner Center allows one pending submission. Clear that draft - # before publishing. Only the publish result gates this step. - bundle="${{ steps.storebundle.outputs.bundle }}" - msstore submission delete "$MS_STORE_PRODUCT_ID" --no-confirm \ - || echo "no pending submission to clear" - msstore publish "$bundle" -id "$MS_STORE_PRODUCT_ID" - # ── macOS updater channel (REAL) ─────────────────────────────────────────── # Merges the per-arch feed ymls (arm64-stable-mac.yml / x64-stable-mac.yml # …) into releases/darwin//-mac.yml via @@ -1681,6 +1538,7 @@ jobs: ref: ${{ needs.validate.outputs.sha }} - name: Render the full expected-binary matrix env: + HERMES_BUNDLE_ENV_JSON: ${{ inputs.bundle_env }} CLOUDFLARE_R2_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_R2_ACCOUNT_ID }} CLOUDFLARE_R2_ACCESS_KEY_ID: ${{ secrets.CLOUDFLARE_R2_ACCESS_KEY_ID }} CLOUDFLARE_R2_SECRET_ACCESS_KEY: ${{ secrets.CLOUDFLARE_R2_SECRET_ACCESS_KEY }} @@ -1702,7 +1560,7 @@ jobs: PY ) python3 scripts/render-builds-table.py \ - --summary-commit "$RELEASE_COMMIT" \ + --summary-commit "$RELEASE_COMMIT" --repo "$GITHUB_REPOSITORY" \ --summary-out "$GITHUB_STEP_SUMMARY" \ --summary-failed-legs "$failed" --run-url "$RUN_URL" diff --git a/apps/desktop/electron/backend-child.ts b/apps/desktop/electron/backend-child.ts index 46c901ddf1..34eb0cc0ea 100644 --- a/apps/desktop/electron/backend-child.ts +++ b/apps/desktop/electron/backend-child.ts @@ -33,13 +33,6 @@ export interface StopBackendChildDeps { killGroup?: (pgid: number, signal: string) => void } -export interface StopBackendTreesForUpdateDeps { - /** Synchronous Windows taskkill /T /F implementation. */ - forceKillProcessTree: (pid: number) => void - /** Clears and stops the desktop's pooled backends. */ - stopAllPoolBackends: () => void -} - export interface BackendProcessRoot { pid?: number | null } @@ -122,13 +115,13 @@ export async function waitForBackendExit( * throws (the process may already be gone) -- mirrors the original inline * best-effort semantics in main.ts. */ -export function stopBackendChild(child: KillableChild | null | undefined, deps: StopBackendChildDeps) { +export function stopBackendChild(child: KillableChild | null | undefined, deps: StopBackendChildDeps): void { if (!child || child.killed) { return } const isWindows = deps.isWindows ?? process.platform === 'win32' - const killGroup = deps.killGroup ?? ((pgid: number, signal: string) => process.kill(pgid, signal)) + const killGroup = deps.killGroup ?? ((pgid: number, signal: string): boolean => process.kill(pgid, signal)) try { if (isWindows && Number.isInteger(child.pid)) { @@ -148,23 +141,3 @@ export function stopBackendChild(child: KillableChild | null | undefined, deps: // Already gone. } } - -/** - * Stop every backend tree owned by a Windows Desktop update hand-off. - * - * Tree-kill the primary root while its PID is still live, then delegate pool - * teardown to the existing routine that tree-kills each pooled root exactly - * once before mutating its registry. In particular, do not signal the primary - * first: if that root exits before taskkill /T runs, Windows can no longer - * enumerate its MCP grandchildren and they survive with the venv locked. - */ -export function stopBackendTreesForUpdate( - primary: BackendProcessRoot | null | undefined, - deps: StopBackendTreesForUpdateDeps -): void { - if (primary && Number.isInteger(primary.pid)) { - deps.forceKillProcessTree(primary.pid as number) - } - - deps.stopAllPoolBackends() -} diff --git a/apps/desktop/electron/backend-stop-overlap.test.ts b/apps/desktop/electron/backend-stop-overlap.test.ts new file mode 100644 index 0000000000..2cd57774bc --- /dev/null +++ b/apps/desktop/electron/backend-stop-overlap.test.ts @@ -0,0 +1,183 @@ +import assert from 'node:assert/strict' +import { type ChildProcess, spawn } from 'node:child_process' +import { once } from 'node:events' + +import { test } from 'vitest' + +import { stopBackendChild, waitForBackendExit } from './backend-child' +import { createLocalBackendLifecycle } from './local-backend-lifecycle' +import { + assertPoolEntryStillOwned, + type LocalBackendSlotEntry, + LocalBackendSpawnCoordinator, + releaseLocalBackendSlot, + releaseLocalBackendSlotAfterExit +} from './pool-spawn-coordinator' +import { createPoolStopper } from './pool-stop' + +test.skipIf(process.platform === 'win32')( + 'eviction, update and quit share physical exit before releasing a slot', + async (): Promise => { + let signals = 0 + let released = false + + const physical = { + forceKillProcessTree: (): never => { + throw new Error('POSIX test') + } + } + + const lifecycle = createLocalBackendLifecycle({ + cancelSetup: (): void => {}, + stopChild: (child: ChildProcess): void => { + signals++ + stopBackendChild(child, physical) + }, + waitForExit: (child: ChildProcess): Promise => waitForBackendExit(child, physical) + }) + + const child = lifecycle.spawn((): ChildProcess => + spawn( + process.execPath, + [ + '-e', + ` + process.on('SIGTERM', () => process.send('stopping')); + process.on('message', () => process.exit(0)); + setInterval(() => {}, 1000); + process.send('ready'); + ` + ], + { detached: true, stdio: ['ignore', 'ignore', 'ignore', 'ipc'] } + ) + ) + + child.once('exit', (): boolean => lifecycle.release(child)) + + const deps = { + pool: new Map([['profile', { process: child }]]), + stopChild: lifecycle.stop + } + + const pool = createPoolStopper(deps) + + try { + await once(child, 'message') + const signalled = once(child, 'message') + + const eviction = releaseLocalBackendSlotAfterExit( + (): void => { + released = true + }, + (): Promise => pool.stop('profile') + ) + + const update = lifecycle.stop(child) + const quit = lifecycle.shutdown() + await signalled + assert.equal(signals, 1) + assert.equal(released, false, 'a pool slot must remain occupied while its process is alive') + assert.ok(pool.inFlight('profile')) + assert.throws((): ChildProcess => lifecycle.spawn((): ChildProcess => child)) + child.send('exit') + await Promise.all([eviction, update, quit]) + assert.equal(child.exitCode, 0) + assert.equal(released, true) + assert.equal(pool.inFlight('profile'), undefined) + assert.equal(lifecycle.hasPending(), false) + } finally { + if (child.exitCode === null && child.signalCode === null) { + child.kill('SIGKILL') + await once(child, 'exit') + } + } + } +) + +test.skipIf(process.platform === 'win32')( + 'a claim completed after eviction retains its live child capacity', + async (): Promise => { + const slots = new LocalBackendSpawnCoordinator(1) + const signal = new AbortController().signal + const unspawned: LocalBackendSlotEntry = { releaseLocalBackendSlot: await slots.acquire('not-spawned') } + assert.throws((): void => assertPoolEntryStillOwned('not-spawned', unspawned, new Map(), signal)) + assert.equal(slots.activeCount, 0, 'pre-spawn cancellation releases its reservation') + + const physical = { + forceKillProcessTree: (): never => { + throw new Error('POSIX test') + } + } + + const lifecycle = createLocalBackendLifecycle({ + cancelSetup: (): void => {}, + stopChild: (child: ChildProcess): void => stopBackendChild(child, physical), + waitForExit: (child: ChildProcess): Promise => waitForBackendExit(child, physical) + }) + + const release = await slots.acquire('claiming') + + const child = lifecycle.spawn((): ChildProcess => + spawn( + process.execPath, + [ + '-e', + ` + process.on('SIGTERM', () => process.send('stopping')); + process.on('message', () => process.exit(0)); + setInterval(() => {}, 1000); + process.send('ready'); + ` + ], + { detached: true, stdio: ['ignore', 'ignore', 'ignore', 'ipc'] } + ) + ) + + child.once('exit', (): boolean => lifecycle.release(child)) + const entry = { process: child, releaseLocalBackendSlot: release } + const entries = new Map([['claiming', entry]]) + const pool = createPoolStopper({ pool: entries, stopChild: lifecycle.stop }) + let finishClaim!: () => void + + const claim = new Promise((resolve: () => void): void => { + finishClaim = resolve + }) + + const starting = claim.then((): void => assertPoolEntryStillOwned('claiming', entry, entries, signal)) + const rejected = assert.rejects(starting, /cancelled/) + + try { + await once(child, 'message') + const signalled = once(child, 'message') + + const eviction = releaseLocalBackendSlotAfterExit( + (): void => releaseLocalBackendSlot(entry), + (): Promise => pool.stop('claiming') + ) + + await signalled + finishClaim() + await rejected + assert.equal(child.exitCode, null) + assert.equal(child.signalCode, null) + assert.ok(pool.inFlight('claiming')) + assert.equal(slots.activeCount, 1, 'the post-claim guard must not release a live child slot') + const replacement = slots.request('different-profile') + assert.equal(replacement.queued, true) + child.send('exit') + await eviction + const releaseReplacement = await replacement.acquired + assert.equal(child.exitCode, 0) + releaseReplacement() + assert.equal(slots.activeCount, 0) + } finally { + finishClaim() + await rejected + + if (child.exitCode === null && child.signalCode === null) { + child.kill('SIGKILL') + await once(child, 'exit') + } + } + } +) diff --git a/apps/desktop/electron/local-backend-lifecycle.ts b/apps/desktop/electron/local-backend-lifecycle.ts index 49f52d5476..fecacb3707 100644 --- a/apps/desktop/electron/local-backend-lifecycle.ts +++ b/apps/desktop/electron/local-backend-lifecycle.ts @@ -7,7 +7,7 @@ export async function waitForTeardown(tasks: readonly Promise[], timeou try { await Promise.race([ Promise.allSettled(tasks), - new Promise(resolve => { + new Promise((resolve: () => void): void => { timer = setTimeout(resolve, timeoutMs) }) ]) @@ -29,7 +29,20 @@ interface LocalBackendLifecycleDeps { * shutdown signal and synchronous spawn fence make bounded waiting safe: a late * resolver can finish, but can never create another owned backend. */ -export function createLocalBackendLifecycle(deps: LocalBackendLifecycleDeps) { +export interface LocalBackendLifecycle { + signal: AbortSignal + assertCanStart: () => void + hasPending: () => boolean + start: (run: () => Promise) => Promise + spawn: (create: () => Child) => Child + release: (child: Child) => boolean + stop: (child: Child | null | undefined) => Promise + shutdown: () => Promise +} + +export function createLocalBackendLifecycle( + deps: LocalBackendLifecycleDeps +): LocalBackendLifecycle { const controller = new AbortController() const starts = new Set>() const children = new Set() @@ -46,21 +59,21 @@ export function createLocalBackendLifecycle(deps: LocalBackendLifecycleDe return existing } - const stopping = (async () => { + const stopping = (async (): Promise => { deps.stopChild(child) await deps.waitForExit(child) })() stops.set(child, stopping) void stopping.then( - () => stops.delete(child), - () => stops.delete(child) + (): boolean => stops.delete(child), + (): boolean => stops.delete(child) ) return stopping } - const shutdown = createBackendShutdownCoordinator(() => { + const shutdown = createBackendShutdownCoordinator((): Promise => { controller.abort(new Error('Hermes Desktop is quitting.')) deps.cancelSetup() @@ -69,15 +82,15 @@ export function createLocalBackendLifecycle(deps: LocalBackendLifecycleDe return { signal: controller.signal, - assertCanStart: () => controller.signal.throwIfAborted(), - hasPending: () => starts.size > 0 || children.size > 0 || stops.size > 0 || shutdown.isPending(), + assertCanStart: (): void => controller.signal.throwIfAborted(), + hasPending: (): boolean => starts.size > 0 || children.size > 0 || stops.size > 0 || shutdown.isPending(), start(run: () => Promise): Promise { if (controller.signal.aborted) { return Promise.reject(controller.signal.reason) } // Defer invocation one microtask so the inventory precedes all work. - const promise = Promise.resolve().then(() => { + const promise = Promise.resolve().then((): Promise => { controller.signal.throwIfAborted() return run() @@ -85,8 +98,8 @@ export function createLocalBackendLifecycle(deps: LocalBackendLifecycleDe starts.add(promise) void promise.then( - () => starts.delete(promise), - () => starts.delete(promise) + (): boolean => starts.delete(promise), + (): boolean => starts.delete(promise) ) return promise @@ -98,7 +111,7 @@ export function createLocalBackendLifecycle(deps: LocalBackendLifecycleDe return child }, - release: (child: Child) => children.delete(child), + release: (child: Child): boolean => children.delete(child), stop, shutdown: shutdown.run } diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 97ad5f60d0..e630e95176 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -37,7 +37,7 @@ import { appIconCandidates, resolveAppIcon } from './app-icon' import { stageAppInstallerFile } from './app-installer-file' import { appVersionInfo, type AppVersionInfo, assertSourceUpdateChannel, packagedReleaseChannel } from './app-version' import { runAppInstallerChecker } from './appinstaller-checker' -import { stopBackendChild as stopBackendChildImpl, stopBackendTreesForUpdate, waitForBackendExit as waitForBackendExitImpl } from './backend-child' +import { stopBackendChild as stopBackendChildImpl, waitForBackendExit } from './backend-child' import { type BackendOutputTail, claimDecision, @@ -59,7 +59,7 @@ import { makeUnsignedOauthError, waitForHermesReady } from './backend-health' -import { backendCommandMatches, createBackendOwnership, createBackendShutdownCoordinator } from './backend-ownership' +import { backendCommandMatches, type BackendOwnershipEntry, createBackendOwnership, createBackendShutdownCoordinator } from './backend-ownership' import { canImportHermesCli, execProbeSync, @@ -310,10 +310,11 @@ import { import { selectPoolEvictions } from './pool-eviction' import { clampPoolLimits, parsePoolLimits, POOL_LIMITS_DEFAULTS } from './pool-limits' import { + assertPoolEntryStillOwned, isBackgroundSlotWaitTimeout, LocalBackendSpawnCoordinator, type LocalBackendSpawnPriority, - type LocalBackendSpawnRequest, + releaseLocalBackendSlot, releaseLocalBackendSlotAfterExit } from './pool-spawn-coordinator' import { createPoolStopper } from './pool-stop' @@ -412,7 +413,6 @@ import { } from './translucency' import { waitForUpdateClearance } from './update-gate' import { readLiveUpdateMarker, updateHandoffConflict, writeUpdateMarker } from './update-marker' -import { isOfficialSshRemote, OFFICIAL_REPO_HTTPS_URL } from './update-remote' import { resolveUpdaterMechanism, type UpdaterApplyResultWire, @@ -434,6 +434,7 @@ import { ExternalStrategy } from './updater/external' import { createMacStrategy } from './updater/mac-client' import { type ConsumedRelaunch, consumePendingRelaunch, registerUpdateRelaunch, type RelaunchRegistration } from './updater/relaunch' import { startRelaunchWaiter } from './updater/relaunch-waiter' +import { preflightStateDb } from './updater/state-db-preflight' import { createStoreStrategy } from './updater/store-client' import { isHermesOwnedVenvDaemon } from './venv-holder-select' import { fetchMarketplaceThemes, searchMarketplaceThemes } from './vscode-marketplace' @@ -1318,7 +1319,7 @@ const localBackendLifecycle = createLocalBackendLifecycle({ stopBackendChildImpl(child, { forceKillProcessTree, isWindows: IS_WINDOWS }) } }, - waitForExit: (child: ChildProcess): Promise => waitForBackendExit(child), + waitForExit: (child: ChildProcess): Promise => waitForBackendExit(child, { forceKillProcessTree, isWindows: IS_WINDOWS }), cancelSetup: (): void => { firstRunSetupGate?.resetForRetry() bootstrapAbortController?.abort() @@ -2947,12 +2948,6 @@ function runGit(args, options: any = {}): Promise<{ code: number; stdout: string const firstLine = text => (text || '').split('\n').find(Boolean) || '' -async function getOriginUrl(updateRoot) { - const origin = await runGit(['remote', 'get-url', 'origin'], { cwd: updateRoot }) - - return origin.code === 0 ? origin.stdout.trim() : '' -} - function emitUpdateProgress(payload) { const merged = { stage: 'idle', message: '', percent: null, error: null, ...payload, at: Date.now() } rememberLog(`[updates] ${merged.stage}: ${merged.message || merged.error || ''}`) @@ -2962,35 +2957,6 @@ function emitUpdateProgress(payload) { } } -// Self-heal the tracked update branch: if origin no longer publishes it (e.g. -// bb/gui was merged into main and deleted), fall back to main and persist so -// every later check/apply follows main — no manual flip, even for already- -// installed clients. Read-only ls-remote probe; only flips on a definitive -// "ref absent" (exit 2), never on a transient network error, so a flaky -// connection can't strand a user on the wrong branch. -async function resolveHealedBranch(updateRoot, branch) { - if (!branch || branch === 'main') { - return branch || 'main' - } - - const originUrl = await getOriginUrl(updateRoot) - const remote = isOfficialSshRemote(originUrl) ? OFFICIAL_REPO_HTTPS_URL : 'origin' - const probe = await runGit(['ls-remote', '--exit-code', '--heads', remote, branch], { cwd: updateRoot }) - - if (probe.code !== 2) { - return branch - } - - rememberLog(`[updates] origin/${branch} is gone (merged?); falling back to main`) - const config = readDesktopUpdateConfig() - - if (config.branch !== 'main') { - writeDesktopUpdateConfig({ ...config, branch: 'main' }) - } - - return 'main' -} - async function checkUpdates(opts: { force?: boolean } = {}): Promise { // A packaged install delegates to the update owner named by its stamp. let strategy: UpdaterStrategy | null = null @@ -3015,19 +2981,6 @@ async function checkUpdates(opts: { force?: boolean } = {}): Promise { - const response = await fetch(url, { - headers: { Accept: accept, 'User-Agent': 'hermes-desktop-update-check' }, - signal: AbortSignal.timeout(10_000) - }) - - if (!response.ok) { - throw new Error(`HTTP ${response.status}`) - } - - return accept === 'application/vnd.github.sha' ? response.text() : response.json() -} - let updateInFlight = false // ── bundled / App Installer helpers ───────────────────────────────────────── @@ -3162,31 +3115,27 @@ function resolveCheckoutUpdateStrategy(): UpdaterStrategy { defaultUpdateBranch: DEFAULT_UPDATE_BRANCH, updateHandoffDwellMs: UPDATE_HANDOFF_DWELL_MS, directoryExists, - readCanonicalInstallStamp, - readDesktopUpdateConfig, - readSourceUpdate: (updateRoot: string): Promise => readSourceUpdate({ + readSourceUpdate: (updateRoot: string, opts: { force?: boolean }): Promise => readSourceUpdate({ python: findPythonForRoot(updateRoot), git: resolveGitBinary(), updateRoot, - hermesHome: HERMES_HOME + hermesHome: HERMES_HOME, + branchConfigPath: DESKTOP_UPDATE_CONFIG_PATH, + force: opts.force }), resolveUpdateRoot, resolveUpdaterBinary, - resolveHealedBranch, - getOriginUrl, - runGit, firstLine, - fetchGitHubApi, - isGitCheckout, - updateCheckCachePath: path.join(app.getPath('userData'), 'update-check-cache.json'), - writeFileAtomic, emitUpdateProgress, rememberLog, startHermes, stopBackendsForUpdate, repairMacUpdaterHelper, - preflightStateDb, + preflightStateDb: (home: string, log: (message: string) => void): void => { + const root: string = resolveUpdateRoot() + preflightStateDb({ python: findPythonForRoot(root), script: path.join(root, 'hermes_cli', 'backup_sqlite.py'), home, log }) + }, runningAppBundle, markQuittingForHandoff: () => { isQuittingForHandoff = true @@ -3618,7 +3567,13 @@ const desktopParentStartMarker = createParentStartMarkerResolver({ } }) -async function claimBackendChild(child, command, profile, nonce, outputTail: BackendOutputTail | null = null) { +async function claimBackendChild( + child: ChildProcess & { hermesBackendIdentity?: BackendOwnershipEntry }, + command: string, + profile: string, + nonce: string, + outputTail: BackendOutputTail | null = null +): Promise { // Probe/claim policy lives in backend-claim.ts (#93608): a marker probe // that fails against a LIVE child degrades to PID-only identity — matching // createParentStartMarkerResolver — instead of killing a healthy backend @@ -3628,8 +3583,7 @@ async function claimBackendChild(child, command, profile, nonce, outputTail: Bac const decision = claimDecision(child.exitCode === null && !child.killed, probe) if (decision.action === 'fail') { - stopBackendChild(child) - await waitForBackendExit(child) + await localBackendLifecycle.stop(child) throw new Error( `Hermes backend (PID ${child.pid}) died before its identity could be recorded: ${decision.reason}${outputTail?.describe() ?? ''}` ) @@ -3665,8 +3619,7 @@ async function claimBackendChild(child, command, profile, nonce, outputTail: Bac return identity } catch (error) { - stopBackendChild(child) - await waitForBackendExit(child) + await localBackendLifecycle.stop(child) throw new Error( `Could not persist ownership for the Hermes backend: ${error.message}${outputTail?.describe() ?? ''}` ) @@ -3710,16 +3663,13 @@ function reapOrphanedBackendsOnce() { // `hermes update`; neither venv scans nor a second fleet stop belong here. async function stopBackendsForUpdate(): Promise { if (IS_WINDOWS) { - stopBackendTreesForUpdate(backendConnectionState.getProcess(), { - forceKillProcessTree, - stopAllPoolBackends - }) + await Promise.all([teardownPrimaryBackendAndWait(), stopAllPoolBackends()]) } } // Uninstall still deletes the installation and its historical venv. Unlike // generation updates, deletion must wait for those old files to be released. -async function releaseBackendLock(updateRoot, tag) { +async function releaseBackendLock(updateRoot: string, tag: string): Promise<{ unlocked: boolean }> { if (!IS_WINDOWS) { return { unlocked: true } } @@ -3744,10 +3694,7 @@ async function releaseBackendLock(updateRoot, tag) { } } - stopBackendTreesForUpdate(hermesProcess, { - forceKillProcessTree, - stopAllPoolBackends - }) + await Promise.all([teardownPrimaryBackendAndWait(), stopAllPoolBackends()]) // Uninstall deletes the whole runtime. Drain separately-running gateways // through the CLI, rather than targeting a gateway worker by PID. @@ -3882,9 +3829,8 @@ async function handOffWindowsBootstrapRecovery(reason) { const updateRoot = resolveUpdateRoot() const { branch: configuredBranch } = readDesktopUpdateConfig() - const branch = isGitCheckout(updateRoot) - ? await resolveHealedBranch(updateRoot, configuredBranch || DEFAULT_UPDATE_BRANCH) - : configuredBranch || DEFAULT_UPDATE_BRANCH + // Recovery can run without Python. Keep the chosen branch; do not guess a replacement. + const branch: string = configuredBranch || DEFAULT_UPDATE_BRANCH const updaterArgs: string[] = chooseUpdaterArgs( { runtimeUsable: isSourceRuntimeUsable(updateRoot) }, @@ -3962,92 +3908,6 @@ function runningAppBundle() { return dir.endsWith('.app') ? dir : null } -// ── Pre-flight state.db integrity guard (#68474) ───────────────────── -// Take an emergency snapshot of state.db and verify the live copy is -// intact before any update process mutates the install. Runs in the -// desktop Electron process itself, before the backend is killed and -// before the updater is spawned — a separate safety net from the -// Python-level pre-update snapshot inside `hermes update`. -function preflightStateDb(hermesHome, rememberLog) { - const stateDbPath = path.join(hermesHome, 'state.db') - - if (!fileExists(stateDbPath)) { - rememberLog('[updates] state.db pre-flight: not found (fresh install?)') - - return - } - - try { - const stat = fs.statSync(stateDbPath) - - if (stat.size > 100) { - const fd = fs.openSync(stateDbPath, 'r') - const header = Buffer.alloc(16) - - fs.readSync(fd, header, 0, 16, 0) - fs.closeSync(fd) - - const expectedHeader = Buffer.from('SQLite format 3\0') - const headerOk = header.equals(expectedHeader) - - rememberLog( - `[updates] state.db pre-flight: size=${stat.size}, ` + - `headerOk=${headerOk}, headerHex=${header.toString('hex')}` - ) - - if (!headerOk) { - rememberLog( - '[updates] state.db header is INVALID before update — ' + - 'this indicates pre-existing corruption or a concurrent write issue' - ) - } - - // Emergency timestamped backup, separate from the Python-level snapshot. - const ts = new Date().toISOString().replace(/[:.]/g, '-') - - const emergencyPath = path.join(hermesHome, `state.db.pre-update-emergency-${ts}.bak`) - - try { - fs.copyFileSync(stateDbPath, emergencyPath) - const emergStat = fs.statSync(emergencyPath) - - rememberLog(`[updates] emergency state.db backup: ${emergencyPath} ` + `(${emergStat.size} bytes)`) - - // Prune to the 2 most recent emergency backups. - try { - const homeDir = fs.readdirSync(hermesHome) - - const backups = homeDir - .filter( - f => - f.startsWith('state.db.pre-update-emergency-') && - f.endsWith('.bak') && - f !== path.basename(emergencyPath) - ) - .sort() - .reverse() - - for (const old of backups.slice(2)) { - try { - fs.unlinkSync(path.join(hermesHome, old)) - } catch { - void 0 - } - } - } catch { - void 0 - } - } catch (copyErr) { - rememberLog(`[updates] emergency state.db backup failed: ${copyErr.message}`) - } - } else { - rememberLog(`[updates] state.db too small (${stat.size} bytes) for a valid SQLite database`) - } - } catch (statErr) { - rememberLog(`[updates] could not stat state.db before update: ${statErr.message}`) - } -} - // macOS/Linux update hand-off: spawn the repo-owned posix orchestrator // (scripts/desktop-update/posix.sh) detached and QUIT. The script waits us // out, runs `hermes update`, swaps/relaunches the app bundle, and writes @@ -10692,20 +10552,13 @@ function resetBootProgressForReconnect() { ) } -function stopBackendChild(child: ChildProcess | null | undefined): void { - void localBackendLifecycle.stop(child).catch((error: unknown): void => rememberLog(`Backend teardown failed: ${error instanceof Error ? error.message : String(error)}`)) -} - -// Soft gateway-mode apply: tear down the primary without resetting boot UI or -// reloading the renderer. The shell stays up; the renderer wipes session lists -// (so skeletons retrigger) and re-dials. Distinct from hard re-home (profile -// switch / crash recovery), which still resets boot progress + reloads. -function resetHermesConnection({ soft = false } = {}) { +// Reset routing and UI state only. Local callers must await physical teardown. +// Remote revalidation has no local child and can reset this state directly. +function resetHermesConnectionState({ soft = false }: { soft?: boolean } = {}): void { backendStartFailure = null remoteReauthFailure = null remoteLiveness.clear() - const hermesProcess = backendConnectionState.invalidate() - stopBackendChild(hermesProcess) + backendConnectionState.invalidate() if (!soft) { resetBootProgressForReconnect() @@ -10724,7 +10577,7 @@ async function teardownPrimaryBackendAndWait({ soft = false }: { soft?: boolean } try { - resetHermesConnection({ soft }) + resetHermesConnectionState({ soft }) await stopping } finally { if (soft) { @@ -10762,29 +10615,6 @@ function broadcastConnectionsChanged(payload: { connectionId: string; reason: 'r } } -const backendExitWaits = new Map>() - -function waitForBackendExit(child: ChildProcess | null | undefined, timeoutMs: number = 5000): Promise { - if (!child) { - return Promise.resolve() - } - - const existing = backendExitWaits.get(child) - - if (existing) { - return existing - } - - const waiting = waitForBackendExitImpl(child, { forceKillProcessTree, isWindows: IS_WINDOWS }, timeoutMs) - backendExitWaits.set(child, waiting) - void waiting.then( - (): boolean => backendExitWaits.delete(child), - (): boolean => backendExitWaits.delete(child) - ) - - return waiting -} - // The profile the primary (window) backend runs as. readActiveDesktopProfile() // returns the desktop's stored preference, or null when unset (legacy launch // that defers to active_profile / default). @@ -11796,31 +11626,6 @@ function startPoolIdleReaper() { } } -function releaseLocalBackendSlot(entry: any) { - if (!entry) { - return - } - - const release = entry.releaseLocalBackendSlot - const request = entry.localBackendSpawnRequest as LocalBackendSpawnRequest | null - entry.releaseLocalBackendSlot = null - entry.localBackendSlotKey = null - entry.localBackendSpawnRequest = null - - if (release) { - release() - } else { - request?.cancel() - } -} - -function assertPoolEntryStillOwned(poolKey: string, entry: any): void { - if (localBackendLifecycle.signal.aborted || backendPool.get(poolKey) !== entry) { - releaseLocalBackendSlot(entry) - throw new Error(`Profile backend start for "${poolKey}" was cancelled before spawn.`) - } -} - const failedLocalBackendTeardowns = new WeakMap>() function teardownFailedLocalBackend(poolKey: string, entry: any): Promise { @@ -11837,14 +11642,9 @@ function teardownFailedLocalBackend(poolKey: string, entry: any): Promise const child = entry.process const teardown = releaseLocalBackendSlotAfterExit( - () => releaseLocalBackendSlot(entry), - async () => { - stopBackendChild(child) - await waitForBackendExit(child) - - if (child && child.exitCode === null && child.signalCode === null) { - throw new Error(`Profile backend for "${poolKey}" did not exit; keeping the local slot occupied.`) - } + (): void => releaseLocalBackendSlot(entry), + async (): Promise => { + await localBackendLifecycle.stop(child) releaseBackendChild(child) } @@ -11864,7 +11664,11 @@ function teardownFailedLocalBackend(poolKey: string, entry: any): Promise // entry means THIS machine regardless of the v1 routing table); `opts.poolKey` // is the backendPool key when it differs from the profile name (composite // registry scopes) so the exit/error cleanup evicts the right entry. -async function spawnPoolBackend(profile, entry, opts: { forceLocal?: boolean; poolKey?: string } = {}) { +async function spawnPoolBackend( + profile: string, + entry: any, + opts: { forceLocal?: boolean; poolKey?: string } = {} +): Promise>> { const poolKey = opts.poolKey || profile await reapOrphanedBackendsOnce() @@ -11907,7 +11711,7 @@ async function spawnPoolBackend(profile, entry, opts: { forceLocal?: boolean; po const spawnPriority: LocalBackendSpawnPriority = spawnPriorityFrom(entry.spawnPriority) - assertPoolEntryStillOwned(poolKey, entry) + assertPoolEntryStillOwned(poolKey, entry, backendPool, localBackendLifecycle.signal) const spawnRequest = localBackendSpawnCoordinator.request(poolKey, { timeoutMs: POOL_SLOT_WAIT_MS, @@ -11936,7 +11740,7 @@ async function spawnPoolBackend(profile, entry, opts: { forceLocal?: boolean; po entry.localBackendSpawnRequest = null } - assertPoolEntryStillOwned(poolKey, entry) + assertPoolEntryStillOwned(poolKey, entry, backendPool, localBackendLifecycle.signal) const token = crypto.randomBytes(32).toString('base64url') @@ -11988,7 +11792,7 @@ async function spawnPoolBackend(profile, entry, opts: { forceLocal?: boolean; po const parentStartMarker = await desktopParentStartMarker() const backendNonce = crypto.randomBytes(16).toString('hex') const parentIdentityEnv = parentWatchdogEnv(process.pid, parentStartMarker, backendNonce) - assertPoolEntryStillOwned(poolKey, entry) + assertPoolEntryStillOwned(poolKey, entry, backendPool, localBackendLifecycle.signal) const child = spawnOwnedBackend( backend.command, @@ -12043,7 +11847,7 @@ async function spawnPoolBackend(profile, entry, opts: { forceLocal?: boolean; po // surface as an unhandled rejection before the Promise.race below attaches. portAnnouncement.catch(() => {}) await claimBackendChild(child, `${backend.command} ${backend.args.join(' ')}`, profile, backendNonce, outputTail) - assertPoolEntryStillOwned(poolKey, entry) + assertPoolEntryStillOwned(poolKey, entry, backendPool, localBackendLifecycle.signal) child.stdout.on('data', rememberLog) child.stderr.on('data', rememberLog) @@ -12130,13 +11934,12 @@ async function spawnPoolBackend(profile, entry, opts: { forceLocal?: boolean; po // Bounded, deduplicated pool teardown (see pool-stop.ts): every stop path — // idle reaper, LRU eviction, profile delete/rename, quit — shares one // in-flight stop per key and retains the process handle until the bounded -// SIGTERM -> SIGKILL escalation in waitForBackendExit() resolves. Previously +// physical shutdown promise resolves. Previously // SIGTERM + immediate entry delete dropped the handle and a slow child // survived detached under PID 1. const poolStopper = createPoolStopper({ pool: backendPool, - stopChild: child => stopBackendChild(child), - waitForExit: child => waitForBackendExit(child) + stopChild: localBackendLifecycle.stop }) async function stopPoolBackend(profile: string) { @@ -12273,7 +12076,7 @@ async function prepareProfileRenameRequest(request) { }) } -async function startHermes() { +async function startHermes(): Promise>> { // Only the single-instance lock holder may reap/spawn/claim the desktop // backend. A lock-losing instance must stay inert even if some path reaches // here (e.g. the deferred-quit window before `ready`): its reapOrphans() @@ -12528,8 +12331,7 @@ async function startHermes() { )) if (!processOwner) { - stopBackendChild(hermesProcess) - await waitForBackendExit(hermesProcess) + await localBackendLifecycle.stop(hermesProcess) releaseBackendChild(hermesProcess) throw new Error('Hermes backend start was superseded by a newer connection attempt.') } @@ -14361,7 +14163,7 @@ ipcMain.handle('hermes:connection:revalidate', async () => { currentConnectionPromise: () => backendConnectionState.getPromise(), log: rememberLog, probe: (connection, path, options) => fetchJsonForBackend(connection, path, options), - resetConnection: () => resetHermesConnection({ soft: true }), + resetConnection: () => resetHermesConnectionState({ soft: true }), tracker: remoteLiveness }), revalidatePool() @@ -14626,7 +14428,7 @@ ipcMain.handle('hermes:bootstrap:reset', async () => { return { ok: true } }) -ipcMain.handle('hermes:bootstrap:repair', async () => { +ipcMain.handle('hermes:bootstrap:repair', async (): Promise<{ ok: boolean; bundled?: boolean; error?: string }> => { // A bundled install's payload is immutable and sealed at build time — // "repair" would re-run the installer against a separate // %LOCALAPPDATA%\hermes tree the app doesn't own. The only repair for a @@ -14685,7 +14487,7 @@ ipcMain.handle('hermes:bootstrap:repair', async () => { backendStartFailure = null remoteReauthFailure = null getFirstRunSetupGate().resetForRepair() - resetHermesConnection() + await teardownPrimaryBackendAndWait() return { ok: true } }) diff --git a/apps/desktop/electron/pool-spawn-coordinator.ts b/apps/desktop/electron/pool-spawn-coordinator.ts index 5f80af332d..11c9c8a69f 100644 --- a/apps/desktop/electron/pool-spawn-coordinator.ts +++ b/apps/desktop/electron/pool-spawn-coordinator.ts @@ -42,6 +42,47 @@ export function isBackgroundSlotWaitTimeout(error: unknown): boolean { return error instanceof LocalBackendSlotWaitTimeoutError && error.silent } +export interface LocalBackendSlotEntry { + process?: unknown + releaseLocalBackendSlot?: ReleaseLocalBackendSlot | null + localBackendSlotKey?: string | null + localBackendSpawnRequest?: LocalBackendSpawnRequest | null +} + +export function releaseLocalBackendSlot(entry: LocalBackendSlotEntry | undefined): void { + if (!entry) { + return + } + + const release = entry.releaseLocalBackendSlot + const request = entry.localBackendSpawnRequest + entry.releaseLocalBackendSlot = null + entry.localBackendSlotKey = null + entry.localBackendSpawnRequest = null + + if (release) { + release() + } else { + request?.cancel() + } +} + +export function assertPoolEntryStillOwned( + poolKey: string, + entry: LocalBackendSlotEntry, + pool: ReadonlyMap, + signal: AbortSignal +): void { + if (signal.aborted || pool.get(poolKey) !== entry) { + // A post-claim cancellation still owns a child. Its exit releases capacity. + if (!entry.process) { + releaseLocalBackendSlot(entry) + } + + throw new Error(`Profile backend start for "${poolKey}" was cancelled during start.`) + } +} + export async function releaseLocalBackendSlotAfterExit( release: ReleaseLocalBackendSlot, waitForExit: () => Promise @@ -54,7 +95,8 @@ export async function releaseLocalBackendSlotAfterExit( * Bounds the number of local profile backends that are starting or running. * * A lease is acquired immediately before local start work and is held until - * the child exits or the start fails. Remote descriptors never call request(). + * the child exits, or a start is cancelled before spawn. Remote descriptors + * never call request(). * * When the cap is at least 2, one slot is reserved for foreground (user-open) * requests so background roster hydration cannot occupy the whole pool. diff --git a/apps/desktop/electron/pool-stop.test.ts b/apps/desktop/electron/pool-stop.test.ts index ce497cbad0..c6cc06c63c 100644 --- a/apps/desktop/electron/pool-stop.test.ts +++ b/apps/desktop/electron/pool-stop.test.ts @@ -13,25 +13,31 @@ interface Child { killed: boolean } -function harness() { - const pool = new Map() +function harness(): { + addChild: (key: string) => Child + events: string[] + exitResolvers: Map void> + pool: Map> + stopper: ReturnType> +} { + const pool = new Map>() const events: string[] = [] const exitResolvers = new Map void>() const stopper = createPoolStopper({ pool, - stopChild: child => { + stopChild: (child: Child | undefined): Promise => { ;(child as Child).killed = true events.push('stop') - }, - waitForExit: child => - new Promise(resolve => { - exitResolvers.set(child as Child, () => { + + return new Promise((resolve: () => void): void => { + exitResolvers.set(child as Child, (): void => { ;(child as Child).exited = true events.push('exit') resolve() }) }) + } }) function addChild(key: string): Child { @@ -181,9 +187,13 @@ test('failed stops block respawn and retain the child for a later stop retry', a const stopper = createPoolStopper({ pool, - stopChild: current => { attempts.push(current!) }, - waitForExit: async current => { - if (refuses) { throw failure } + stopChild: async (current: Child | undefined): Promise => { + attempts.push(current!) + + if (refuses) { + throw failure + } + current!.exited = true } }) diff --git a/apps/desktop/electron/pool-stop.ts b/apps/desktop/electron/pool-stop.ts index beca9466c5..551c67f7b2 100644 --- a/apps/desktop/electron/pool-stop.ts +++ b/apps/desktop/electron/pool-stop.ts @@ -29,10 +29,8 @@ export interface PoolStopEntry { export interface PoolStopperDeps { /** The live backend pool. Entries are evicted synchronously on stop. */ pool: Map> - /** Signal the child (tree/group kill per platform). Synchronous. */ - stopChild: (child: Process | undefined) => void - /** Bounded wait: resolves when the child exits, escalating to SIGKILL. */ - waitForExit: (child: Process | undefined) => Promise + /** The physical lifecycle owns signalling, escalation and confirmed exit. */ + stopChild: (child: Process | undefined) => Promise } export interface PoolStopper { @@ -73,8 +71,7 @@ export function createPoolStopper(deps: PoolStopperDeps): Pool deps.pool.delete(key) const stopping = (async (): Promise => { - deps.stopChild(entry.process) - await deps.waitForExit(entry.process) + await deps.stopChild(entry.process) })().then( (): void => { stops.delete(key) diff --git a/apps/desktop/electron/update-api-check.test.ts b/apps/desktop/electron/update-api-check.test.ts deleted file mode 100644 index 9da6e47eba..0000000000 --- a/apps/desktop/electron/update-api-check.test.ts +++ /dev/null @@ -1,81 +0,0 @@ -/** - * Tests for electron/update-api-check.ts — the API-first passive update check. - * - * Why this exists: every desktop client used to `git fetch` twice every 30 - * minutes. GitHub measured tens of millions of fetch/clone requests per day - * from the install base and asked us to poll via the API instead. These pin - * the two load-bearing contracts: the cache answers passive checks for a full - * day but invalidates the moment HEAD moves, and the compare payload maps to - * an honest behind count (never a fabricated one). - */ - -import assert from 'node:assert/strict' - -import { test } from 'vitest' - -import { - branchTipApiUrl, - cacheIsFresh, - githubRepoSlug, - parseCompare, - UPDATE_CHECK_FAILURE_TTL_MS, - UPDATE_CHECK_TTL_MS -} from './update-api-check' - -const SHA_A = 'a'.repeat(40) -const SHA_B = 'b'.repeat(40) -const HOUR = 60 * 60 * 1000 - -test('cache serves a passive check for 24h, but not once HEAD or the branch changes', () => { - const cached = { fetchedAt: 0, currentSha: SHA_A, branch: 'main', status: { behind: 0 } } - - assert.equal(cacheIsFresh(cached, { branch: 'main', currentSha: SHA_A, now: UPDATE_CHECK_TTL_MS - 1 }), true) - assert.equal(cacheIsFresh(cached, { branch: 'main', currentSha: SHA_A, now: UPDATE_CHECK_TTL_MS }), false) - // Applying an update moves HEAD: a stale "update available" must never survive it. - assert.equal(cacheIsFresh(cached, { branch: 'main', currentSha: SHA_B, now: 1 }), false) - assert.equal(cacheIsFresh(cached, { branch: 'bb/gui', currentSha: SHA_A, now: 1 }), false) - - // Failures retry sooner than successes, but still not on every tick. - const failed = { ...cached, status: { error: 'fetch-failed' } } - assert.equal(cacheIsFresh(failed, { branch: 'main', currentSha: SHA_A, now: UPDATE_CHECK_FAILURE_TTL_MS - 1 }), true) - assert.equal(cacheIsFresh(failed, { branch: 'main', currentSha: SHA_A, now: 2 * HOUR }), false) -}) - -test('compare payload maps to the behind count and a newest-first commit list; malformed = null', () => { - const payload = { - ahead_by: 2, - status: 'ahead', - commits: [ - { - sha: SHA_A, - commit: { message: 'fix: older\n\nbody', author: { name: 'A' }, committer: { date: '2026-09-10T00:00:00Z' } } - }, - { - sha: SHA_B, - commit: { message: 'feat: newer', author: { name: 'B' }, committer: { date: '2026-09-10T01:00:00Z' } } - } - ] - } - - const parsed = parseCompare(payload) - assert.equal(parsed?.behind, 2) - assert.deepEqual( - parsed?.commits.map(c => [c.sha, c.summary, c.author]), - [ - [SHA_B, 'feat: newer', 'B'], - [SHA_A, 'fix: older', 'A'] - ] - ) - - assert.equal(parseCompare({ ahead_by: -1 }), null) - assert.equal(parseCompare({ status: 'ahead' }), null) - assert.equal(parseCompare('nope'), null) - - // Forks and SSH forms hit the API for their own repo; non-GitHub origins don't. - assert.equal(githubRepoSlug('git@github.com:Someone/hermes-agent.git'), 'someone/hermes-agent') - assert.equal(githubRepoSlug('https://gitlab.example/x/y.git'), null) - assert.equal( - branchTipApiUrl('nousresearch/hermes-agent', 'bb/gui'), - 'https://api.github.com/repos/nousresearch/hermes-agent/commits/bb%2Fgui' - ) -}) diff --git a/apps/desktop/electron/update-api-check.ts b/apps/desktop/electron/update-api-check.ts deleted file mode 100644 index 10aa82b948..0000000000 --- a/apps/desktop/electron/update-api-check.ts +++ /dev/null @@ -1,111 +0,0 @@ -/** - * Passive update checks against the GitHub REST API instead of git. - * - * Every desktop client used to run `git fetch origin ` (or `ls-remote`) - * twice every 30 minutes, plus on each window focus. Multiplied across the - * install base that is tens of millions of pack negotiations a day against one - * repo — GitHub flagged it. A passive check only needs two facts the API gives - * for free: the remote tip SHA (`GET /repos/{repo}/commits/{branch}` with the - * `application/vnd.github.sha` media type — a 40-byte body) and, when the tips - * differ, the compare endpoint's `ahead_by` + `commits[]`. `git fetch` now - * runs only when the user actually applies an update. - * - * Pure helpers here (URL builders, cache policy, payload mapping) so they are - * unit-testable without booting Electron; the bounded network call is injected. - */ - -import { canonicalGitHubRemote } from './update-remote' - -export const UPDATE_CHECK_TTL_MS = 24 * 60 * 60 * 1000 -// A failed check (offline, 403 rate-limit) is retried sooner than a good one, -// but never on every poller tick. -export const UPDATE_CHECK_FAILURE_TTL_MS = 60 * 60 * 1000 - -export interface CachedUpdateCheck { - fetchedAt: number - currentSha: string - branch: string - status: Record & { error?: string } -} - -/** `owner/repo` for any GitHub remote form; null for non-GitHub origins. */ -export function githubRepoSlug(originUrl: string): string | null { - const canonical = canonicalGitHubRemote(originUrl) - const match = /^github\.com\/([^/]+\/[^/]+)$/.exec(canonical) - - return match ? match[1] : null -} - -export function branchTipApiUrl(slug: string, branch: string): string { - return `https://api.github.com/repos/${slug}/commits/${encodeURIComponent(branch)}` -} - -export function compareApiUrl(slug: string, currentSha: string, targetSha: string): string { - return `https://api.github.com/repos/${slug}/compare/${currentSha}...${targetSha}` -} - -/** - * Whether a cached result still answers a passive check. The cache is keyed on - * the local HEAD and branch: applying an update or switching branches changes - * HEAD and invalidates it immediately, so a 24h TTL never shows a stale - * "update available" after the user just updated. - */ -export function cacheIsFresh( - cached: CachedUpdateCheck | null | undefined, - { branch, currentSha, now }: { branch: string; currentSha: string; now: number } -): boolean { - if (!cached || cached.branch !== branch || cached.currentSha !== currentSha) { - return false - } - - const ttl = cached.status.error ? UPDATE_CHECK_FAILURE_TTL_MS : UPDATE_CHECK_TTL_MS - - return now - cached.fetchedAt < ttl -} - -export interface CompareCommit { - sha: string - summary: string - author: string - at: number -} - -/** - * Map the compare payload to the shape the update overlay renders. `ahead_by` - * is how far the remote tip is ahead of local HEAD, i.e. the behind count; 0 - * with differing tips means local carries commits on top of origin (not - * behind). Any shape surprise returns null so callers keep the honest - * "update available, count unknown" state instead of trusting a partial answer. - */ -export function parseCompare(payload: unknown): { behind: number; commits: CompareCommit[] } | null { - if (!payload || typeof payload !== 'object') { - return null - } - - const ahead = (payload as { ahead_by?: unknown }).ahead_by - - if (typeof ahead !== 'number' || !Number.isInteger(ahead) || ahead < 0) { - return null - } - - const raw = (payload as { commits?: unknown }).commits - - const commits: CompareCommit[] = Array.isArray(raw) - ? raw - .map(entry => { - const sha = typeof entry?.sha === 'string' ? entry.sha : '' - const message = typeof entry?.commit?.message === 'string' ? entry.commit.message : '' - const author = typeof entry?.commit?.author?.name === 'string' ? entry.commit.author.name : '' - - const date = - typeof entry?.commit?.committer?.date === 'string' ? Date.parse(entry.commit.committer.date) : NaN - - return { sha, summary: message.split('\n')[0], author, at: Number.isFinite(date) ? date : 0 } - }) - .filter(commit => commit.sha) - // The overlay lists newest first; compare returns oldest first. - .reverse() - : [] - - return { behind: ahead, commits } -} diff --git a/apps/desktop/electron/update-count.test.ts b/apps/desktop/electron/update-count.test.ts deleted file mode 100644 index d3281f126c..0000000000 --- a/apps/desktop/electron/update-count.test.ts +++ /dev/null @@ -1,302 +0,0 @@ -import assert from 'node:assert/strict' -import { execFileSync } from 'node:child_process' -import fs from 'node:fs' -import os from 'node:os' -import path from 'node:path' - -import { test } from 'vitest' - -import { - compareApiUrl, - parseCompareBehindCount, - resolveBehindCount, - resolveCommitLogSelection, - shouldCountCommits -} from './update-count' - -function createTempGitRepo() { - const cwd = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-update-count-')) - const git = (...args: string[]) => execFileSync('git', args, { cwd, encoding: 'utf8', timeout: 10_000 }).trim() - - try { - git('init', '--quiet') - git('config', 'commit.gpgSign', 'false') - git('config', 'core.hooksPath', '.git/no-hooks') - git('config', 'user.name', 'Hermes Test') - git('config', 'user.email', 'hermes@example.invalid') - - return { cwd, git } - } catch (error) { - fs.rmSync(cwd, { recursive: true, force: true }) - throw error - } -} - -// FAIL-BEFORE: pre-fix the function did `Number.parseInt(countStr) || 0` -// unconditionally, so a shallow checkout with no merge-base surfaced the bogus -// rev-list count (e.g. 12104) — #51922. Later the branch returned the sentinel -// `1`, which the UI rendered as a literal "1 change included" even when the -// true count was far higher (e.g. 90, or the real-world 61 in #84591). An -// update IS available here, but its exact size is unknown — the only honest -// value is `null`. -test('shallow checkout with no merge-base reports null (unknown count), not a fake 1', () => { - assert.equal( - resolveBehindCount({ - countStr: '12104', - currentSha: 'aaa', - targetSha: 'bbb', - isShallow: true - }), - null - ) -}) - -test('shallow checkout with no merge-base but identical SHA reports up-to-date', () => { - assert.equal( - resolveBehindCount({ - countStr: '12104', - currentSha: 'abc', - targetSha: 'abc', - isShallow: true - }), - 0 - ) -}) - -test('shallow local-ahead checkout reports up-to-date when origin is a known ancestor', () => { - assert.equal( - resolveBehindCount({ - countStr: '', - currentSha: 'local-child', - targetSha: 'origin-parent', - isShallow: true, - targetIsAncestorOfHead: true - }), - 0 - ) -}) - -test('shallow Git graph proves the remote tip is an ancestor of a local commit', () => { - const { cwd, git } = createTempGitRepo() - - try { - git('commit', '--allow-empty', '-m', 'origin tip') - - const targetSha = git('rev-parse', 'HEAD') - - git('update-ref', 'refs/remotes/origin/main', targetSha) - fs.writeFileSync(path.join(cwd, '.git', 'shallow'), `${targetSha}\n`) - git('commit', '--allow-empty', '-m', 'local child') - - const currentSha = git('rev-parse', 'HEAD') - - git('merge-base', '--is-ancestor', 'origin/main', 'HEAD') - assert.notEqual(currentSha, targetSha) - assert.equal( - resolveBehindCount({ - countStr: '', - currentSha, - targetSha, - isShallow: true, - targetIsAncestorOfHead: true - }), - 0 - ) - } finally { - fs.rmSync(cwd, { recursive: true, force: true }) - } -}, 30_000) - -test('shallow checkout with a merge-base does not trust an inflated rev-list count', () => { - const { cwd, git } = createTempGitRepo() - - try { - git('commit', '--allow-empty', '-m', 'root') - git('commit', '--allow-empty', '-m', 'ancestor') - - const redundantParent = git('rev-parse', 'HEAD') - - git('commit', '--allow-empty', '-m', 'installed head') - - const currentSha = git('rev-parse', 'HEAD') - const tree = git('rev-parse', 'HEAD^{tree}') - - const targetSha = execFileSync('git', ['commit-tree', tree, '-p', currentSha, '-p', redundantParent], { - cwd, - encoding: 'utf8', - input: 'remote merge\n', - timeout: 10_000 - }).trim() - - git('update-ref', 'refs/remotes/origin/main', targetSha) - - const completeCount = git('rev-list', 'HEAD..origin/main', '--count') - - assert.equal(completeCount, '1') - - fs.writeFileSync(path.join(cwd, '.git', 'shallow'), `${currentSha}\n`) - - assert.equal(git('rev-parse', '--is-shallow-repository'), 'true') - assert.equal(git('merge-base', 'HEAD', 'origin/main'), currentSha) - - const shallowCount = git('rev-list', 'HEAD..origin/main', '--count') - - assert.ok(Number.parseInt(shallowCount, 10) > Number.parseInt(completeCount, 10)) - assert.equal( - resolveBehindCount({ - countStr: shallowCount, - currentSha, - targetSha, - isShallow: true - }), - null - ) - } finally { - fs.rmSync(cwd, { recursive: true, force: true }) - } -}, 30_000) - -test('shallow checkout with a merge-base still uses presence-only status', () => { - assert.equal( - resolveBehindCount({ - countStr: '3', - currentSha: 'aaa', - targetSha: 'bbb', - isShallow: true - }), - null - ) -}) - -test('full (non-shallow) clone keeps the exact count path unchanged', () => { - assert.equal( - resolveBehindCount({ - countStr: '7', - currentSha: 'aaa', - targetSha: 'bbb', - isShallow: false - }), - 7 - ) -}) - -test('up-to-date full clone reports 0', () => { - assert.equal( - resolveBehindCount({ - countStr: '0', - currentSha: 'x', - targetSha: 'x', - isShallow: false - }), - 0 - ) -}) - -test('non-numeric count falls back to 0 (defensive, unchanged behaviour)', () => { - assert.equal( - resolveBehindCount({ - countStr: '', - currentSha: 'aaa', - targetSha: 'bbb', - isShallow: false - }), - 0 - ) -}) - -// shouldCountCommits gates the expensive `rev-list --count` in checkUpdates(). -// Every shallow graph is incomplete, so a visible merge-base is not enough to -// prove that the count is exact. -test('shallow checkouts skip the rev-list count', () => { - assert.equal(shouldCountCommits({ isShallow: true }), false) -}) - -test('full (non-shallow) clones run the rev-list count', () => { - assert.equal(shouldCountCommits({ isShallow: false }), true) -}) - -test('shallow commit logs select only the fetched remote tip', () => { - assert.deepEqual(resolveCommitLogSelection({ branch: 'main', isShallow: true }), { - limit: 1, - revision: 'origin/main' - }) -}) - -test('full-clone commit logs keep the complete behind range', () => { - assert.deepEqual(resolveCommitLogSelection({ branch: 'release', isShallow: false }), { - limit: 40, - revision: 'HEAD..origin/release' - }) -}) - -// The skip path produces an empty countStr; resolveBehindCount must NOT trust -// it and must fall through to the SHA compare (mirrors the live call site). -test('skipped-count path resolves via SHA compare, never via empty countStr', () => { - assert.equal( - resolveBehindCount({ - countStr: '', - currentSha: 'aaa', - targetSha: 'bbb', - isShallow: true - }), - null - ) - assert.equal( - resolveBehindCount({ - countStr: '', - currentSha: 'same', - targetSha: 'same', - isShallow: true - }), - 0 - ) -}) - -// --- compare-API recovery: the accuracy half of the class fix (#84591) --- - -const SHA_A = 'a'.repeat(40) -const SHA_B = 'b'.repeat(40) - -test('compareApiUrl builds the GitHub compare URL for HTTPS origins', () => { - assert.equal( - compareApiUrl({ - currentSha: SHA_A, - originUrl: 'https://github.com/NousResearch/hermes-agent.git', - targetSha: SHA_B - }), - `https://api.github.com/repos/NousResearch/hermes-agent/compare/${SHA_A}...${SHA_B}` - ) -}) - -test('compareApiUrl handles SSH origin forms', () => { - for (const originUrl of [ - 'git@github.com:NousResearch/hermes-agent.git', - 'ssh://git@github.com/NousResearch/hermes-agent.git', - 'git@github.com:NousResearch/hermes-agent' - ]) { - assert.equal( - compareApiUrl({ currentSha: SHA_A, originUrl, targetSha: SHA_B }), - `https://api.github.com/repos/NousResearch/hermes-agent/compare/${SHA_A}...${SHA_B}` - ) - } -}) - -test('compareApiUrl refuses non-GitHub remotes and partial SHAs', () => { - assert.equal(compareApiUrl({ currentSha: SHA_A, originUrl: 'https://gitlab.com/x/y.git', targetSha: SHA_B }), null) - assert.equal(compareApiUrl({ currentSha: 'abc123', originUrl: 'https://github.com/x/y.git', targetSha: SHA_B }), null) - assert.equal(compareApiUrl({ currentSha: SHA_A, originUrl: '', targetSha: SHA_B }), null) -}) - -test('parseCompareBehindCount returns ahead_by (the behind count)', () => { - assert.equal(parseCompareBehindCount({ ahead_by: 61, status: 'ahead' }), 61) - assert.equal(parseCompareBehindCount({ ahead_by: 0, status: 'behind' }), 0) -}) - -test('parseCompareBehindCount rejects malformed payloads', () => { - assert.equal(parseCompareBehindCount(null), null) - assert.equal(parseCompareBehindCount({}), null) - assert.equal(parseCompareBehindCount({ ahead_by: -2 }), null) - assert.equal(parseCompareBehindCount({ ahead_by: '61' }), null) - assert.equal(parseCompareBehindCount({ ahead_by: 1.5 }), null) - assert.equal(parseCompareBehindCount([]), null) -}) diff --git a/apps/desktop/electron/update-count.ts b/apps/desktop/electron/update-count.ts deleted file mode 100644 index ebca95271a..0000000000 --- a/apps/desktop/electron/update-count.ts +++ /dev/null @@ -1,115 +0,0 @@ -// Whether `git rev-list HEAD..origin/ --count` produces a meaningful -// number worth computing. Installer checkouts are shallow (`--depth 1`), so -// their visible graph is incomplete even when `merge-base` happens to find a -// common commit. A merge can expose ancestry that the local shallow boundary -// hides from HEAD, inflating the count with old commits. Exact counts are only -// trustworthy in full clones; shallow checkouts use presence-only status plus -// any positively proven local-ahead ancestry. -function shouldCountCommits({ isShallow }: { isShallow: boolean }): boolean { - return !isShallow -} - -// Resolve how many commits the local checkout is behind origin for the desktop -// update indicator. Shallow checkouts use SHA equality plus any positively -// proven local-ahead ancestry; exact counts remain exclusive to full clones. -function resolveBehindCount({ - countStr, - currentSha, - targetSha, - isShallow, - targetIsAncestorOfHead = false -}: { - countStr: string - currentSha: string - targetSha: string - isShallow: boolean - targetIsAncestorOfHead?: boolean -}): number | null { - if (!shouldCountCommits({ isShallow })) { - if (currentSha && targetSha && (currentSha === targetSha || targetIsAncestorOfHead)) { - return 0 - } - - // An update IS available, but its size is unknowable without a merge-base. - // Return null — never a numeric sentinel: the UI used to render the old - // `1` as a literal "1 change included" even when the true distance was - // far larger. null lets every surface say "update available" honestly. - return null - } - - return Number.parseInt(countStr, 10) || 0 -} - -// Shallow history can also contaminate the changelog range. Trust the fetched -// remote tip itself, but do not walk its ancestry. Full clones retain the -// detailed range used by the existing update overlay. -function resolveCommitLogSelection({ branch, isShallow }: { branch: string; isShallow: boolean }): { - limit: number - revision: string -} { - const remote = `origin/${branch}` - - return isShallow ? { limit: 1, revision: remote } : { limit: 40, revision: `HEAD..${remote}` } -} - -// When the local graph can't count (behind === null), the GitHub compare API -// still can: `GET /repos///compare/...` returns -// `ahead_by` — how many commits the remote tip is ahead of the local HEAD, -// i.e. exactly the behind count the shallow clone lost. Unauthenticated, no -// clone depth required. Pure URL builder + response parser here; the network -// call lives with the caller. -function compareApiUrl({ - currentSha, - originUrl, - targetSha -}: { - currentSha: string - originUrl: string - targetSha: string -}): string | null { - const sha = /^[0-9a-f]{40}$/i - - if (!sha.test(currentSha || '') || !sha.test(targetSha || '')) { - return null - } - - // Only GitHub remotes have a compare API. Reuse the canonical form the - // official-remote check produces: `github.com//`. - const canonical = canonicalRemoteForCompare(originUrl) - - if (!canonical) { - return null - } - - return `https://api.github.com/repos/${canonical}/compare/${currentSha}...${targetSha}` -} - -function canonicalRemoteForCompare(originUrl: string): string | null { - const value = String(originUrl || '').trim() - - const match = - /^git@github\.com:([^/]+\/[^/]+?)(?:\.git)?\/?$/i.exec(value) || - /^(?:ssh:\/\/git@|https:\/\/|http:\/\/)github\.com\/([^/]+\/[^/]+?)(?:\.git)?\/?$/i.exec(value) - - return match ? match[1] : null -} - -// `ahead_by` counts target commits not reachable from current — the behind -// count. `status` is "ahead" / "behind" / "diverged" / "identical" relative to -// current...target; any shape surprise returns null so the caller keeps the -// honest "update available" fallback instead of trusting a partial answer. -function parseCompareBehindCount(payload: unknown): number | null { - if (!payload || typeof payload !== 'object') { - return null - } - - const ahead = (payload as { ahead_by?: unknown }).ahead_by - - if (typeof ahead !== 'number' || !Number.isInteger(ahead) || ahead < 0) { - return null - } - - return ahead -} - -export { compareApiUrl, parseCompareBehindCount, resolveBehindCount, resolveCommitLogSelection, shouldCountCommits } diff --git a/apps/desktop/electron/update-handoff-marker.test.ts b/apps/desktop/electron/update-handoff-marker.test.ts index a8f48e6a38..66934d5351 100644 --- a/apps/desktop/electron/update-handoff-marker.test.ts +++ b/apps/desktop/electron/update-handoff-marker.test.ts @@ -24,8 +24,8 @@ function markerStartedAt(home: string): number { return Number.parseInt(startedAt, 10) } -function runPosix(installRoot: string, startedAt?: string) { - const env = { ...process.env } +function runPosix(installRoot: string, startedAt?: string): ReturnType { + const env: NodeJS.ProcessEnv = { ...process.env, HERMES_HOME: path.dirname(installRoot) } if (startedAt === undefined) { delete env.HERMES_UPDATE_STARTED_AT @@ -33,14 +33,14 @@ function runPosix(installRoot: string, startedAt?: string) { env.HERMES_UPDATE_STARTED_AT = startedAt } - return spawnSync('/bin/bash', [POSIX_SCRIPT, '--daemonized', '--install-root', installRoot, '--self-test-marker'], { + return spawnSync('bash', [POSIX_SCRIPT, '--daemonized', '--install-root', installRoot, '--self-test-marker'], { env, encoding: 'utf8' }) } -function runWindows(installRoot: string, startedAt?: string) { - const env = { ...process.env } +function runWindows(installRoot: string, startedAt?: string): ReturnType { + const env: NodeJS.ProcessEnv = { ...process.env, HERMES_HOME: path.dirname(installRoot) } if (startedAt === undefined) { delete env.HERMES_UPDATE_STARTED_AT diff --git a/apps/desktop/electron/update-root-policy.test.ts b/apps/desktop/electron/update-root-policy.test.ts deleted file mode 100644 index 3078895bd3..0000000000 --- a/apps/desktop/electron/update-root-policy.test.ts +++ /dev/null @@ -1,57 +0,0 @@ -/** - * Tests for electron/update-root-policy.ts — the pure classifier that decides - * whether the desktop's git-based self-update may run against a resolved - * update root. A checkout the install contract does not manage (steward-owned - * `external`/`electron-updater` mechanisms) must be refused with a user-action - * pointer instead of the desktop pulling into it. - */ - -import assert from 'node:assert/strict' - -import { test } from 'vitest' - -import { classifyUpdateRoot } from './update-root-policy' - -test('a non-git root is not updatable and carries no user advice', () => { - const result = classifyUpdateRoot({ isGitTree: false, updateMechanism: null }) - - assert.equal(result.updatable, false) - assert.equal(result.verdict, 'not-a-checkout') - assert.equal(result.provenance, 'not-a-checkout') - assert.equal(result.advice, null) - assert.ok(result.message) -}) - -test('a self-managed checkout is updatable', () => { - const result = classifyUpdateRoot({ isGitTree: true, updateMechanism: 'self' }) - - assert.equal(result.updatable, true) - assert.equal(result.verdict, 'updatable') - assert.equal(result.provenance, 'managed-self') - assert.equal(result.advice, null) - assert.equal(result.message, null) -}) - -test.each(['external', 'electron-updater', 'app-installer'] as const)('a %s-owned checkout is never git-updated by the desktop', updateMechanism => { - const result = classifyUpdateRoot({ isGitTree: true, updateMechanism }) - - assert.equal(result.updatable, false) - assert.equal(result.verdict, 'steward-owned-git-tree') - assert.equal(result.provenance, 'steward-owned') - assert.equal(result.advice, 'git pull') - assert.ok(result.message?.includes(updateMechanism)) -}) - -test('an unstamped git tree (dev checkout) stays updatable with unknown provenance', () => { - const result = classifyUpdateRoot({ isGitTree: true, updateMechanism: null }) - - assert.equal(result.updatable, true) - assert.equal(result.verdict, 'updatable') - assert.equal(result.provenance, 'unknown') - assert.equal(result.advice, null) -}) - -test('the classification is a pure function of its inputs', () => { - const facts = { isGitTree: true, updateMechanism: 'external' as const } - assert.deepEqual(classifyUpdateRoot(facts), classifyUpdateRoot({ ...facts })) -}) diff --git a/apps/desktop/electron/update-root-policy.ts b/apps/desktop/electron/update-root-policy.ts deleted file mode 100644 index f7af06b0b4..0000000000 --- a/apps/desktop/electron/update-root-policy.ts +++ /dev/null @@ -1,94 +0,0 @@ -// update-root-policy.ts — pure classifier for the desktop self-update root. -// -// The desktop's git-based self-update arm runs `git fetch/merge` against a -// checkout it resolved (resolveUpdateRoot in main.ts). That root is only -// legitimate update territory when the install contract says so: -// -// - Not a `.git` tree → there is nothing to pull; the desktop cannot -// self-update here (bundled installs use the OS App Installer arm and -// never reach this classifier). -// - A `.git` tree whose install stamp says `updateMechanism: "self"` → the -// install is a managed self-updating checkout (e.g. the desktop -// bootstrap's clone, which writes exactly that stamp) — updatable. -// - A `.git` tree with any OTHER mechanism (`external`: the store/steward -// owns updates; `electron-updater`: the desktop package does) → the tree -// is managed by someone else. Pulling into it from the desktop would -// stash-and-move a user's checkout out from under its steward. -// - No stamp at all (null mechanism; a dev source checkout) → provenance -// unknown. The classifier reports `updatable` with `provenance: -// 'unknown'`: a developer's working tree keeps its existing update flow, -// and callers log the ambiguity rather than silently widening the refusal. -// -// Pure and dependency-injected (same shape as update-gate.ts) so the policy -// is unit-testable without booting Electron, and every caller gets the same -// answer from one authority. - -import type { InstallStamp } from './install-stamp' - -export type UpdateRootProvenance = 'managed-self' | 'steward-owned' | 'unknown' | 'not-a-checkout' - -export interface UpdateRootClassification { - /** Whether the desktop's git-based self-update may run against this root. */ - updatable: boolean - /** Machine-readable verdict for update-check results and logs. */ - verdict: 'updatable' | 'not-a-checkout' | 'steward-owned-git-tree' | 'unmanaged-git-tree' - /** Why the root is (or is not) updatable, for user-facing messages. */ - message: string | null - /** The command that fixes it, when the user (not the app) must act. */ - advice: 'git pull' | null - provenance: UpdateRootProvenance -} - -export interface UpdateRootFacts { - /** True when the resolved update root contains a `.git` entry. */ - isGitTree: boolean - /** The install stamp's updateMechanism, or null when there is no stamp. */ - updateMechanism: InstallStamp['updateMechanism'] | null -} - -/** - * Classify whether the desktop may self-update (git pull semantics) against - * the resolved update root. - */ -export function classifyUpdateRoot(facts: UpdateRootFacts): UpdateRootClassification { - if (!facts.isGitTree) { - return { - updatable: false, - verdict: 'not-a-checkout', - message: 'This install has no git checkout to update — the app or its steward owns the update loop.', - advice: null, - provenance: 'not-a-checkout' - } - } - - if (facts.updateMechanism === 'self') { - return { - updatable: true, - verdict: 'updatable', - message: null, - advice: null, - provenance: 'managed-self' - } - } - - if (facts.updateMechanism !== null) { - return { - updatable: false, - verdict: 'steward-owned-git-tree', - message: - `This checkout is managed by its install method (${facts.updateMechanism}); ` + - 'the desktop will not run git updates against it.', - advice: 'git pull', - provenance: 'steward-owned' - } - } - - // No stamp: unknown provenance (typically a developer source checkout). - return { - updatable: true, - verdict: 'updatable', - message: null, - advice: null, - provenance: 'unknown' - } -} diff --git a/apps/desktop/electron/updater/app-installer.ts b/apps/desktop/electron/updater/app-installer.ts index edaba628b8..21c86c0452 100644 --- a/apps/desktop/electron/updater/app-installer.ts +++ b/apps/desktop/electron/updater/app-installer.ts @@ -19,6 +19,7 @@ import { win32AppInstallerFeedPath } from '../app-updater' +import { applyPackagedHandoff } from './packaged-handoff' import type { RelaunchRegistration } from './relaunch' import type { UpdaterApplyResultWire, UpdaterStatusWire } from './index' @@ -106,57 +107,35 @@ export class AppInstallerStrategy { percent: 100 }) - let registration: RelaunchRegistration | undefined - let teardownStarted = false - - try { - await triggerAppInstallerUpdate( - feedBaseUrl, - this.deps.channel, - this.deps.light, - this.deps.installer, - async () => { - registration = await this.deps.registerPendingRelaunch(this.deps.appVersion) - - if (!registration.automatic) { + return applyPackagedHandoff( + { + teardown: this.deps.teardownBundledBackend, + restore: this.deps.restoreBundledBackend, + emitProgress: this.deps.emitUpdateProgress, + relaunch: { + register: (): Promise => this.deps.registerPendingRelaunch(this.deps.appVersion), + onManual: (): void => this.deps.emitUpdateProgress({ - stage: 'restart', percent: 100, + stage: 'restart', + percent: 100, message: 'Automatic relaunch could not be registered. Reopen Hermes after App Installer finishes.' }) - } - - teardownStarted = true - await this.deps.teardownBundledBackend() - }, - sourceUri - ) - - this.deps.quit() - } catch (error) { - const errors: unknown[] = [error] - - try { - await registration?.cancel() - } catch (cancelError) { - errors.push(cancelError) - } - - if (teardownStarted) { - try { - await this.deps.restoreBundledBackend() - } catch (restoreError) { - errors.push(restoreError) } + }, + async (stop: () => Promise): Promise => { + await triggerAppInstallerUpdate( + feedBaseUrl, + this.deps.channel, + this.deps.light, + this.deps.installer, + stop, + sourceUri + ) + this.deps.quit() + + return { ok: true, manual: false, bundled: true, handedOff: true, mechanism: this.mechanism } } - - const message = errors.map(item => item instanceof Error ? item.message : String(item)).join('; ') - this.deps.emitUpdateProgress({ stage: 'error', message, percent: null }) - - if (errors.length > 1) { throw new AggregateError(errors, message, { cause: error }) } - throw error - } - - return { ok: true, manual: false, bundled: true, handedOff: true, mechanism: this.mechanism } + ) } } diff --git a/apps/desktop/electron/updater/checkout-check-live.test.ts b/apps/desktop/electron/updater/checkout-check-live.test.ts deleted file mode 100644 index 40c93f465b..0000000000 --- a/apps/desktop/electron/updater/checkout-check-live.test.ts +++ /dev/null @@ -1,91 +0,0 @@ -import { execFileSync } from 'node:child_process' -import * as fs from 'node:fs' -import * as http from 'node:http' -import type { AddressInfo } from 'node:net' -import * as os from 'node:os' -import * as path from 'node:path' - -import { expect, it } from 'vitest' - -import { checkCheckoutUpdates, type CheckoutCheckDeps } from './checkout-check' - -it('checks a real linked worktree through HTTP and reuses the disk cache until forced or HEAD changes', async (): Promise => { - const root = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-update-worktree-')) - const checkout = path.join(root, 'source') - const worktree = path.join(root, 'worktree') - const requests: string[] = [] - - const server = http.createServer((request: http.IncomingMessage, response: http.ServerResponse): void => { - requests.push(request.url ?? '') - response.end(targetSha) - }) - - let targetSha = '' - - function git(args: string[], cwd: string = checkout): string { - return execFileSync('git', ['-c', 'user.name=Test', '-c', 'user.email=test@example.invalid', '-c', 'commit.gpgsign=false', ...args], { - cwd, - encoding: 'utf8', - stdio: ['ignore', 'pipe', 'pipe'] - }).trim() - } - - try { - fs.mkdirSync(checkout) - git(['init', '--initial-branch=main']) - git(['commit', '--allow-empty', '-m', 'initial']) - git(['remote', 'add', 'origin', 'https://github.com/example/test.git']) - git(['worktree', 'add', '--detach', worktree]) - targetSha = git(['rev-parse', 'HEAD']) - await new Promise((resolve: () => void): void => { server.listen(0, '127.0.0.1', resolve) }) - const address = server.address() as AddressInfo - - const deps: CheckoutCheckDeps = { - updateCheckCachePath: path.join(root, 'cache.json'), - writeFileAtomic: (filePath: string, contents: string): void => { - fs.writeFileSync(`${filePath}.tmp`, contents) - fs.renameSync(`${filePath}.tmp`, filePath) - }, - isGitCheckout: (directory: string): boolean => fs.existsSync(path.join(directory, '.git')), - readCanonicalInstallStamp: (): null => null, - readDesktopUpdateConfig: (): { branch: string } => ({ branch: 'main' }), - resolveUpdateRoot: (): string => worktree, - resolveHealedBranch: async (_directory: string, branch: string): Promise => branch, - getOriginUrl: async (directory: string): Promise => git(['remote', 'get-url', 'origin'], directory), - runGit: async (args: string[], options?: { cwd?: string }): Promise<{ code: number; stdout: string; stderr: string }> => { - // A passive check must not use git to contact the configured remote. - expect(['rev-parse', 'status']).toContain(args[0]) - - return { code: 0, stdout: git(args, options?.cwd), stderr: '' } - }, - readSourceUpdate: async (): Promise<{ channel: 'main' }> => ({ channel: 'main' }), - fetchGitHubApi: async (url: string, accept?: string): Promise => { - const parsed = new URL(url) - expect(parsed.hostname).toBe('api.github.com') - - const response = await fetch(`http://127.0.0.1:${address.port}${parsed.pathname}`, { - headers: { Accept: accept ?? 'application/json' } - }) - - return response.text() - }, - rememberLog: (message: unknown): void => { throw new Error(String(message)) } - } - - expect(fs.statSync(path.join(worktree, '.git')).isFile()).toBe(true) - expect(await checkCheckoutUpdates(deps)).toMatchObject({ supported: true, currentSha: targetSha, updateAvailable: false }) - expect(requests).toHaveLength(1) - await checkCheckoutUpdates({ ...deps }) - expect(requests).toHaveLength(1) - await checkCheckoutUpdates(deps, { force: true }) - expect(requests).toHaveLength(2) - git(['commit', '--allow-empty', '-m', 'advance'], worktree) - targetSha = git(['rev-parse', 'HEAD'], worktree) - expect(await checkCheckoutUpdates(deps)).toMatchObject({ currentSha: targetSha, updateAvailable: false }) - expect(requests).toHaveLength(3) - } finally { - server.closeAllConnections() - await new Promise((resolve: () => void): void => { server.close((): void => resolve()) }) - fs.rmSync(root, { recursive: true, force: true }) - } -}) diff --git a/apps/desktop/electron/updater/checkout-check.test.ts b/apps/desktop/electron/updater/checkout-check.test.ts deleted file mode 100644 index 2c69f14b3a..0000000000 --- a/apps/desktop/electron/updater/checkout-check.test.ts +++ /dev/null @@ -1,156 +0,0 @@ -import * as fs from 'node:fs' -import * as os from 'node:os' -import * as path from 'node:path' - -import { afterEach, expect, it, vi } from 'vitest' - -import { checkCheckoutUpdates, type CheckoutCheckDeps } from './checkout-check' - -const roots: string[] = [] -afterEach((): void => { - for (const root of roots.splice(0)) { - fs.rmSync(root, { recursive: true, force: true }) - } -}) - -function fixture(): CheckoutCheckDeps { - const root = fs.mkdtempSync(path.join(os.tmpdir(), 'checkout-check-')) - roots.push(root) - - return { - writeFileAtomic: (filePath: string, contents: string): void => fs.writeFileSync(filePath, contents), - updateCheckCachePath: path.join(root, 'cache.json'), - isGitCheckout: (): boolean => true, - readCanonicalInstallStamp: (): null => null, - readDesktopUpdateConfig: (): { branch: string } => ({ branch: 'main' }), - resolveUpdateRoot: (): string => root, - resolveHealedBranch: async (_root: string, branch: string): Promise => branch, - getOriginUrl: async (): Promise => 'git@github.com:NousResearch/hermes-agent.git', - runGit: vi.fn(async (args: string[]): Promise<{ code: number; stdout: string; stderr: string }> => { - const key = args.join(' ') - - if (key === 'rev-parse HEAD') { - return { code: 0, stdout: 'a'.repeat(40), stderr: '' } - } - - if (key === 'rev-parse --abbrev-ref HEAD') { - return { code: 0, stdout: 'main', stderr: '' } - } - - if (key === 'status --porcelain') { - return { code: 0, stdout: '', stderr: '' } - } - - throw new Error(`Unexpected git operation: ${key}`) - }), - readSourceUpdate: async (): Promise<{ channel: 'main' }> => ({ channel: 'main' }), - fetchGitHubApi: vi.fn(async (url: string): Promise => - url.includes('/commits/') ? 'b'.repeat(40) : { ahead_by: 3, commits: [] } - ), - rememberLog: vi.fn() - } -} - -it('uses API checks and a disk cache while a forced check bypasses the cache', async (): Promise => { - const deps = fixture() - const first = await checkCheckoutUpdates(deps) - expect(first.behind).toBe(3) - expect(first.updateAvailable).toBe(true) - expect(deps.fetchGitHubApi).toHaveBeenCalledTimes(2) - expect(await checkCheckoutUpdates(deps)).toEqual(first) - expect(deps.fetchGitHubApi).toHaveBeenCalledTimes(2) - await checkCheckoutUpdates(deps, { force: true }) - expect(deps.fetchGitHubApi).toHaveBeenCalledTimes(4) -}) - -it('follows the current branch unless config explicitly overrides it, including cached checks', async (): Promise => { - const deps: CheckoutCheckDeps = fixture() - const localGit: CheckoutCheckDeps['runGit'] = deps.runGit - let currentBranch: string = 'feature/foo' - let configuredBranch: string = 'main' - let branchExplicit: boolean = false - deps.readDesktopUpdateConfig = (): { branch: string; branchExplicit: boolean } => ({ - branch: configuredBranch, - branchExplicit - }) - deps.runGit = async ( - args: string[], - options?: { cwd?: string } - ): Promise<{ code: number; stdout: string; stderr: string }> => - args.join(' ') === 'rev-parse --abbrev-ref HEAD' - ? { code: 0, stdout: currentBranch, stderr: '' } - : localGit(args, options) - - for (const scenario of [ - { current: 'feature/foo', configured: 'main', explicit: false, target: 'feature/foo' }, - { current: 'feature/foo', configured: 'main', explicit: true, target: 'main' }, - { current: 'feature/foo', configured: 'release/next', explicit: true, target: 'release/next' }, - { current: 'feature/foo', configured: 'main', explicit: false, target: 'feature/foo' }, - { current: 'feature/bar', configured: 'main', explicit: false, target: 'feature/bar' }, - { current: 'HEAD', configured: 'main', explicit: false, target: 'main' }, - { current: '', configured: 'main', explicit: false, target: 'main' } - ]) { - currentBranch = scenario.current - configuredBranch = scenario.configured - branchExplicit = scenario.explicit - vi.mocked(deps.fetchGitHubApi).mockClear() - expect(await checkCheckoutUpdates(deps)).toMatchObject({ - branch: scenario.target, - currentBranch, - targetSha: 'b'.repeat(40) - }) - - if (currentBranch) { - expect(deps.fetchGitHubApi).toHaveBeenCalledWith( - expect.stringContaining(`/commits/${encodeURIComponent(scenario.target)}`), - 'application/vnd.github.sha' - ) - } - - vi.mocked(deps.fetchGitHubApi).mockClear() - expect(await checkCheckoutUpdates(deps)).toMatchObject({ branch: scenario.target, currentBranch }) - expect(deps.fetchGitHubApi).not.toHaveBeenCalled() - } -}) - -it('does not report locally ahead commits as an update', async (): Promise => { - const deps = fixture() - deps.fetchGitHubApi = async (url: string): Promise => - url.includes('/commits/') ? 'b'.repeat(40) : { ahead_by: 0 } - expect(await checkCheckoutUpdates(deps)).toMatchObject({ behind: 0, updateAvailable: false, commits: [] }) -}) - -it('keeps an unknown count when the compare API fails', async (): Promise => { - const deps = fixture() - - deps.fetchGitHubApi = async (url: string): Promise => { - if (url.includes('/commits/')) { - return 'b'.repeat(40) - } - - throw new Error('rate limited') - } - - expect(await checkCheckoutUpdates(deps)).toMatchObject({ behind: null, updateAvailable: true }) -}) - -it('uses only ls-remote for non-GitHub network checks', async (): Promise => { - const deps = fixture() - const localGit = deps.runGit - deps.getOriginUrl = async (): Promise => 'https://git.example/repo.git' - deps.runGit = vi.fn( - async (args: string[], options: { cwd?: string }): Promise<{ code: number; stdout: string; stderr: string }> => { - if (args[0] === 'ls-remote') { - return { code: 0, stdout: `${'b'.repeat(40)}\trefs/heads/main`, stderr: '' } - } - - if (args[0] === 'cat-file') { - return { code: 1, stdout: '', stderr: '' } - } - - return localGit(args, options) - } - ) - expect(await checkCheckoutUpdates(deps)).toMatchObject({ behind: null, updateAvailable: true }) - expect(deps.fetchGitHubApi).not.toHaveBeenCalled() -}) diff --git a/apps/desktop/electron/updater/checkout-check.ts b/apps/desktop/electron/updater/checkout-check.ts deleted file mode 100644 index fc9ef072d1..0000000000 --- a/apps/desktop/electron/updater/checkout-check.ts +++ /dev/null @@ -1,207 +0,0 @@ -import * as fs from 'node:fs' -import * as path from 'node:path' - -import type { InstallStamp } from '../install-stamp' -import { branchTipApiUrl, cacheIsFresh, compareApiUrl, githubRepoSlug, parseCompare } from '../update-api-check' -import { classifyUpdateRoot } from '../update-root-policy' - -import { SOURCE_PROBE_RECOVERY, type SourceUpdate } from './checkout-source' - -import type { UpdaterStatusWire } from './index' - -export interface CheckoutCheckDeps { - writeFileAtomic: (filePath: string, contents: string) => void - updateCheckCachePath: string - isGitCheckout: (root: string) => boolean - readCanonicalInstallStamp: () => { updateMechanism?: InstallStamp['updateMechanism'] } | null - readDesktopUpdateConfig: () => { branch: string; branchExplicit?: boolean } - readSourceUpdate: (root: string) => Promise - resolveUpdateRoot: () => string - resolveHealedBranch: (root: string, branch: string) => Promise - getOriginUrl: (root: string) => Promise - runGit: (args: string[], options?: { cwd?: string }) => Promise<{ code: number; stdout: string; stderr: string }> - fetchGitHubApi: (url: string, accept?: string) => Promise - rememberLog: (chunk: unknown) => void -} - -interface CachedCheckoutCheck { - fetchedAt: number - currentSha: string - branch: string - originUrl: string - updateRoot: string - status: UpdaterStatusWire & Record -} - -function readCache(filePath: string): CachedCheckoutCheck | null { - try { - const cached: CachedCheckoutCheck = JSON.parse(fs.readFileSync(filePath, 'utf8')) - - return cached?.status?.supported === true ? cached : null - } catch { - return null - } -} - -async function checkApi( - deps: CheckoutCheckDeps, - slug: string, - branch: string, - currentSha: string -): Promise> { - let targetSha: string - - try { - targetSha = String(await deps.fetchGitHubApi(branchTipApiUrl(slug, branch), 'application/vnd.github.sha')).trim() - } catch (error: unknown) { - return { error: 'fetch-failed', message: `GitHub API: ${error instanceof Error ? error.message : String(error)}` } - } - - if (!/^[0-9a-f]{40}$/i.test(targetSha)) { - return { error: 'fetch-failed', message: 'GitHub API returned no tip SHA.' } - } - - if (targetSha === currentSha) { - return { behind: 0, updateAvailable: false, targetSha, commits: [] } - } - - const compared = await deps - .fetchGitHubApi(compareApiUrl(slug, currentSha, targetSha)) - .then(parseCompare) - .catch((): null => null) - - return { - behind: compared?.behind ?? null, - updateAvailable: compared?.behind !== 0, - targetSha, - commits: compared?.behind === 0 ? [] : (compared?.commits ?? []) - } -} - -async function checkLsRemote( - deps: CheckoutCheckDeps, - updateRoot: string, - branch: string, - currentSha: string -): Promise> { - const target = await deps.runGit(['ls-remote', 'origin', `refs/heads/${branch}`], { cwd: updateRoot }) - const targetSha = target.stdout.trim().split(/\s+/)[0] || '' - - if (target.code !== 0 || !targetSha) { - return { error: 'fetch-failed', message: target.stderr.split('\n')[0] || 'git ls-remote failed.' } - } - - if (targetSha === currentSha) { - return { behind: 0, updateAvailable: false, targetSha, commits: [] } - } - - const known = (await deps.runGit(['cat-file', '-e', `${targetSha}^{commit}`], { cwd: updateRoot })).code === 0 - - const isAncestor = - known && (await deps.runGit(['merge-base', '--is-ancestor', targetSha, 'HEAD'], { cwd: updateRoot })).code === 0 - - return { behind: isAncestor ? 0 : null, updateAvailable: !isAncestor, targetSha, commits: [] } -} - -/** Passive checks must not fetch packs or mutate a steward-owned checkout. */ -export async function checkCheckoutUpdates( - deps: CheckoutCheckDeps, - { force = false }: { force?: boolean } = {} -): Promise { - const updateRoot: string = deps.resolveUpdateRoot() - const config: ReturnType = deps.readDesktopUpdateConfig() - let branch: string = config.branch - - const policy = classifyUpdateRoot({ - isGitTree: deps.isGitCheckout(updateRoot), - updateMechanism: deps.readCanonicalInstallStamp()?.updateMechanism ?? null - }) - - if (!policy.updatable) { - return { - supported: false, - reason: policy.verdict === 'not-a-checkout' ? 'not-a-git-checkout' : `update-root-${policy.verdict}`, - message: policy.message, - advice: policy.advice, - hermesRoot: updateRoot, - branch - } - } - - const git = async (args: string[]): Promise => (await deps.runGit(args, { cwd: updateRoot })).stdout.trim() - - const [currentSha, dirty, currentBranch, originUrl] = await Promise.all([ - git(['rev-parse', 'HEAD']), - git(['status', '--porcelain']), - git(['rev-parse', '--abbrev-ref', 'HEAD']), - deps.getOriginUrl(updateRoot) - ]) - - if (config.branchExplicit === false && currentBranch && currentBranch !== 'HEAD') { - branch = currentBranch - } - - const selection: SourceUpdate | null = await deps.readSourceUpdate(updateRoot) - - if (selection === null) { - return { supported: false, reason: 'source-probe-unavailable', message: SOURCE_PROBE_RECOVERY, hermesRoot: updateRoot } - } - - if (selection.channel !== 'main') { - return { - supported: true, - ...selection, - channel: selection.channel, - currentSha, - currentBranch, - dirty: dirty.length > 0, - hermesRoot: updateRoot, - fetchedAt: Date.now(), - updateAvailable: !selection.error && selection.targetSha !== currentSha, - behind: selection.targetSha === currentSha ? 0 : null, - commits: [] - } - } - - const cached = readCache(deps.updateCheckCachePath) - const now = Date.now() - - if ( - !force && - cached?.updateRoot === updateRoot && - cached.originUrl === originUrl && - cacheIsFresh(cached, { branch, currentSha, now }) - ) { - return { ...cached.status, dirty: dirty.length > 0, currentBranch } - } - - branch = await deps.resolveHealedBranch(updateRoot, branch) - const slug = githubRepoSlug(originUrl) - - const status = slug - ? await checkApi(deps, slug, branch, currentSha) - : await checkLsRemote(deps, updateRoot, branch, currentSha) - - const result: UpdaterStatusWire & Record = { - supported: true, - branch, - currentBranch, - currentSha, - dirty: dirty.length > 0, - hermesRoot: updateRoot, - fetchedAt: now, - ...status - } - - try { - fs.mkdirSync(path.dirname(deps.updateCheckCachePath), { recursive: true }) - const entry: CachedCheckoutCheck = { fetchedAt: now, currentSha, branch, originUrl, updateRoot, status: result } - deps.writeFileAtomic(deps.updateCheckCachePath, JSON.stringify(entry)) - } catch (error: unknown) { - deps.rememberLog( - `[updates] could not persist check cache: ${error instanceof Error ? error.message : String(error)}` - ) - } - - return result -} diff --git a/apps/desktop/electron/updater/checkout-legacy.test.ts b/apps/desktop/electron/updater/checkout-legacy.test.ts index 2c0977a6ef..1236682ad9 100644 --- a/apps/desktop/electron/updater/checkout-legacy.test.ts +++ b/apps/desktop/electron/updater/checkout-legacy.test.ts @@ -10,7 +10,7 @@ import { readSourceUpdate, type SourceUpdate } from './checkout-source' it('offers manual recovery only for a missing source probe, never for a broken probe', async (): Promise => { const root: string = fs.mkdtempSync(path.join(os.tmpdir(), 'legacy-channel-')) const home: string = path.join(root, 'profile') - const modulePath: string = path.join(root, 'hermes_cli', 'source_releases.py') + const modulePath: string = path.join(root, 'hermes_cli', 'source_check.py') fs.mkdirSync(path.dirname(modulePath)) fs.mkdirSync(home) fs.writeFileSync(path.join(root, 'hermes_cli', '__init__.py'), '') @@ -20,19 +20,11 @@ it('offers manual recovery only for a missing source probe, never for a broken p }) const deps: CheckoutStrategyDeps = { - isGitCheckout: (): boolean => true, - updateCheckCachePath: path.join(root, 'cache.json'), - writeFileAtomic: vi.fn(), readSourceUpdate: probe, fetchGitHubApi: vi.fn(), + readSourceUpdate: probe, hermesHome: home, isWindows: process.platform === 'win32', isMac: process.platform === 'darwin', defaultUpdateBranch: 'main', updateHandoffDwellMs: 0, - directoryExists: fs.existsSync, readCanonicalInstallStamp: (): null => null, - readDesktopUpdateConfig: (): { branch: string } => ({ branch: 'main' }), + directoryExists: fs.existsSync, resolveUpdateRoot: (): string => root, resolveUpdaterBinary: vi.fn((): string => 'frozen-updater'), - resolveHealedBranch: vi.fn(async (_root: string, branch: string): Promise => branch), - getOriginUrl: async (): Promise => 'https://github.com/fixture/repo', - runGit: async (args: string[]): Promise<{ code: number; stdout: string; stderr: string }> => ({ - code: 0, stdout: args.includes('--abbrev-ref') ? 'feature/work' : args.includes('HEAD') ? 'a'.repeat(40) : '', stderr: '' - }), firstLine: (text: string): string => text.split('\n')[0], emitUpdateProgress: vi.fn(), rememberLog: vi.fn(), startHermes: vi.fn(async (): Promise => {}), stopBackendsForUpdate: vi.fn(async (): Promise => {}), @@ -54,7 +46,6 @@ it('offers manual recovery only for a missing source probe, never for a broken p expect(result.command).not.toContain('--branch') expect(deps.stopBackendsForUpdate).not.toHaveBeenCalled() expect(deps.resolveUpdaterBinary).not.toHaveBeenCalled() - expect(deps.fetchGitHubApi).not.toHaveBeenCalled() expect(deps.quit).not.toHaveBeenCalled() } diff --git a/apps/desktop/electron/updater/checkout-ownership.test.ts b/apps/desktop/electron/updater/checkout-ownership.test.ts index 9328b14bc8..dbe124253c 100644 --- a/apps/desktop/electron/updater/checkout-ownership.test.ts +++ b/apps/desktop/electron/updater/checkout-ownership.test.ts @@ -1,62 +1,43 @@ -import { describe, expect, it, vi } from 'vitest' +import { expect, it, vi } from 'vitest' import { type CheckoutStrategyDeps, createCheckoutStrategy } from './checkout' +import type { SourceUpdate } from './checkout-source' -function dependencies(): CheckoutStrategyDeps { - return { - isGitCheckout: (): boolean => true, - updateCheckCachePath: 'unused-cache.json', - writeFileAtomic: vi.fn(), - readSourceUpdate: vi.fn(async (): Promise<{ channel: 'main' }> => ({ channel: 'main' })), - fetchGitHubApi: vi.fn(), - hermesHome: 'home', - isWindows: process.platform === 'win32', - isMac: process.platform === 'darwin', - defaultUpdateBranch: 'main', - updateHandoffDwellMs: 0, - directoryExists: () => true, - readCanonicalInstallStamp: () => ({ updateMechanism: 'external' }), - readDesktopUpdateConfig: () => ({ branch: 'main' }), - resolveUpdateRoot: () => 'repo', - resolveUpdaterBinary: () => null, - resolveHealedBranch: async (_, branch) => branch, - getOriginUrl: async () => '', - runGit: vi.fn(async () => { throw new Error('unexpected git invocation') }), - firstLine: text => text.split('\n')[0], - emitUpdateProgress: vi.fn(), - rememberLog: vi.fn(), - startHermes: vi.fn(async () => {}), - stopBackendsForUpdate: vi.fn(async (): Promise => {}), - repairMacUpdaterHelper: vi.fn(), - preflightStateDb: vi.fn(), - runningAppBundle: () => null, - markQuittingForHandoff: vi.fn(), - quit: vi.fn() - } -} +it.each(['not-a-git-checkout', 'update-root-steward-owned-git-tree', 'fetch-failed'])( + 'preserves the Python refusal or error before handoff: %s', + async (reason: string): Promise => { + const status: SourceUpdate = reason === 'fetch-failed' + ? { supported: true, error: reason } + : { supported: false, reason } -describe('checkout update admission', () => { - it.each(['external', 'app-installer', 'electron-updater'] as const)('refuses %s-owned code without fetching or stopping the backend', async updateMechanism => { - const deps = dependencies() - deps.readCanonicalInstallStamp = () => ({ updateMechanism }) - const strategy = createCheckoutStrategy(deps) - const result = await strategy.check() + const deps: CheckoutStrategyDeps = { + readSourceUpdate: vi.fn(async (): Promise => status), + hermesHome: 'home', + isWindows: process.platform === 'win32', + isMac: process.platform === 'darwin', + defaultUpdateBranch: 'main', + updateHandoffDwellMs: 0, + directoryExists: (): boolean => true, + resolveUpdateRoot: (): string => 'repo', + resolveUpdaterBinary: vi.fn((): null => null), + firstLine: (text: string): string => text.split('\n')[0], + emitUpdateProgress: vi.fn(), + rememberLog: vi.fn(), + startHermes: vi.fn(async (): Promise => {}), + stopBackendsForUpdate: vi.fn(async (): Promise => {}), + repairMacUpdaterHelper: vi.fn(), + preflightStateDb: vi.fn(), + runningAppBundle: (): null => null, + markQuittingForHandoff: vi.fn(), + quit: vi.fn() + } - expect(result.supported).toBe(false) - expect(await strategy.apply()).toMatchObject({ ok: false }) - expect(deps.readSourceUpdate).not.toHaveBeenCalled() - expect(result.mechanism).toBe(strategy.mechanism) - expect(deps.runGit).not.toHaveBeenCalled() + const strategy: ReturnType = createCheckoutStrategy(deps) + expect(await strategy.check()).toMatchObject({ ...status, mechanism: strategy.mechanism }) + expect(await strategy.apply()).toMatchObject({ ok: false, error: reason }) + expect(deps.readSourceUpdate).toHaveBeenLastCalledWith('repo', { force: true }) + expect(deps.resolveUpdaterBinary).not.toHaveBeenCalled() expect(deps.stopBackendsForUpdate).not.toHaveBeenCalled() expect(deps.quit).not.toHaveBeenCalled() - }) - - it('rejects a missing source checkout without attempting git', async () => { - const deps = dependencies() - deps.isGitCheckout = (): boolean => false - const result = await createCheckoutStrategy(deps).check() - - expect(result.reason).toBe('not-a-git-checkout') - expect(deps.runGit).not.toHaveBeenCalled() - }) -}) + } +) diff --git a/apps/desktop/electron/updater/checkout-source.test.ts b/apps/desktop/electron/updater/checkout-source.test.ts index b50cee461a..109a7e36e2 100644 --- a/apps/desktop/electron/updater/checkout-source.test.ts +++ b/apps/desktop/electron/updater/checkout-source.test.ts @@ -56,7 +56,7 @@ it('carries each install channel from Python publication checks into the source try { fs.mkdirSync(origin) fs.mkdirSync(home) - git(['init', '-b', 'feature/gui']) + git(['init', '-b', 'upstream-build']) const commits: string[] = [] for (const label of ['old', 'stable', 'canary', 'unpublished']) { @@ -80,7 +80,10 @@ it('carries each install channel from Python publication checks into the source responses.set('/releases/stable/release-candidates.json', { tag: tags.stable, commit: commits[1] }) git(['tag', 'v99.0.0']) - git(['clone', origin, root], temporary) + git(['worktree', 'add', '-b', 'feature/gui', root]) + git(['remote', 'add', 'origin', origin]) + fs.writeFileSync(path.join(origin, 'install-stamp.json'), JSON.stringify({ updateMechanism: 'external' })) + fs.writeFileSync(path.join(root, 'install-stamp.json'), JSON.stringify({ updateMechanism: 'self' })) await new Promise((resolve: () => void): void => { server.listen(0, '127.0.0.1', resolve) }) @@ -122,48 +125,22 @@ import urllib.request\nfrom urllib.parse import urlsplit\noriginal = urllib.requ ) } + const checkerPath: string = path.join(root, 'hermes_cli', 'source_check.py') + fs.writeFileSync(checkerPath, fs.readFileSync(checkerPath, 'utf8').replace( + 'from __future__ import annotations', + `from __future__ import annotations\nimport runpy; runpy.run_path(${JSON.stringify(path.join(root, 'transport.py'))})` + )) + const deps: CheckoutStrategyDeps = { hermesHome: home, isWindows: process.platform === 'win32', isMac: process.platform === 'darwin', defaultUpdateBranch: 'main', updateHandoffDwellMs: 0, - updateCheckCachePath: path.join(home, 'cache.json'), - writeFileAtomic: (file: string, contents: string): void => fs.writeFileSync(file, contents), - isGitCheckout: (): boolean => true, - readCanonicalInstallStamp: (): null => null, - readDesktopUpdateConfig: (): { branch: string; branchExplicit: boolean } => ({ - branch: 'main', - branchExplicit: false - }), resolveUpdateRoot: (): string => root, - readSourceUpdate: async (install: string): Promise => { - const result: { stdout: string } = await execute( - python, - [ - '-c', - "import runpy,sys; runpy.run_path(sys.argv.pop(1)); runpy.run_module('hermes_cli.source_releases', run_name='__main__')", - path.join(root, 'transport.py'), - '--install-root', - install, - '--git', - 'git' - ], - { cwd: root, env: environment } - ) - - return JSON.parse(result.stdout) as SourceUpdate - }, - resolveHealedBranch: async (_root: string, branch: string): Promise => branch, - getOriginUrl: async (): Promise => origin, - runGit: async (args: string[]): Promise<{ code: number; stdout: string; stderr: string }> => ({ - code: 0, - stdout: git(args, root), - stderr: '' + readSourceUpdate: (install: string, opts: { force?: boolean }): Promise => readSourceUpdate({ + python, git: 'git', updateRoot: install, hermesHome: home, force: opts.force }), - fetchGitHubApi: async (): Promise => { - throw new Error('branch API must not resolve releases') - }, directoryExists: fs.existsSync, resolveUpdaterBinary: (): null => null, firstLine: (text: string): string => text.split('\n')[0], @@ -199,7 +176,6 @@ import urllib.request\nfrom urllib.parse import urlsplit\noriginal = urllib.requ for (const channel of ['stable', 'canary'] as const) { await setChannel(channel) - await setChannel(channel === 'stable' ? 'canary' : 'stable', origin) const sha: string = commits[channel === 'stable' ? 1 : 2] const checked: unknown = await strategy.check() expect(checked, JSON.stringify({ checked, requests })).toMatchObject({ @@ -246,8 +222,8 @@ import urllib.request\nfrom urllib.parse import urlsplit\noriginal = urllib.requ expect(deps.stopBackendsForUpdate).not.toHaveBeenCalled() expect(spawned).toHaveLength(0) await setChannel('main') - expect(await readSourceUpdate({ python, git: 'git', updateRoot: root, hermesHome: home })).toEqual({ - channel: 'main' + expect(await readSourceUpdate({ python, git: 'git', updateRoot: root, hermesHome: home })).toMatchObject({ + supported: true, branch: 'feature/gui', targetSha: commits[3], updateAvailable: false }) const count: number = requests.length expect(await strategy.check()).toMatchObject({ diff --git a/apps/desktop/electron/updater/checkout-source.ts b/apps/desktop/electron/updater/checkout-source.ts index cabcf4c470..68d140fc6d 100644 --- a/apps/desktop/electron/updater/checkout-source.ts +++ b/apps/desktop/electron/updater/checkout-source.ts @@ -4,19 +4,20 @@ import { promisify } from 'node:util' import { buildDesktopBackendEnv } from '../backend-env' import { hiddenWindowsChildOptions } from '../windows-child-options' -export interface SourceUpdate { - channel: 'main' | 'stable' | 'canary' - latestTag?: string - targetSha?: string - error?: string - message?: string -} +import type { UpdaterStatusWire } from './index' + +export interface SourceUpdate extends UpdaterStatusWire {} export interface SourceUpdateProbe { python: string | null git: string updateRoot: string hermesHome: string + branch?: string + channel?: 'main' | 'stable' | 'canary' + force?: boolean + cachePath?: string + branchConfigPath?: string } const execute: typeof execFile.__promisify__ = promisify(execFile) @@ -48,8 +49,13 @@ export async function readSourceUpdate(probe: SourceUpdateProbe): Promise string + readSourceUpdate: (root: string, opts: { force?: boolean }) => Promise hermesHome: string isWindows: boolean isMac: boolean @@ -68,7 +69,12 @@ export function createCheckoutStrategy(deps: CheckoutStrategyDeps): UpdaterStrat const mechanism: UpdaterMechanism = deps.isWindows ? 'windows-handoff' : 'posix-handoff' async function check(opts: { force?: boolean } = {}): Promise { - const status = await checkCheckoutUpdates(deps, opts) + const root: string = deps.resolveUpdateRoot() + + const status: UpdaterStatusWire = await deps.readSourceUpdate(root, opts) ?? { + supported: false, reason: 'source-probe-unavailable', message: SOURCE_PROBE_RECOVERY, hermesRoot: root + } + status.mechanism = mechanism return status @@ -84,7 +90,7 @@ export function createCheckoutStrategy(deps: CheckoutStrategyDeps): UpdaterStrat return { mechanism, check, apply } async function applyBody(): Promise { - const status: UpdaterStatusWire = await checkCheckoutUpdates(deps, { force: true }) + const status: UpdaterStatusWire = await check({ force: true }) if (status.reason === 'source-probe-unavailable') { return { ok: true, manual: true, command: 'hermes update --help', message: status.message, hermesRoot: status.hermesRoot } diff --git a/apps/desktop/electron/updater/mac.test.ts b/apps/desktop/electron/updater/mac.test.ts index 19ec867cb1..c105d04252 100644 --- a/apps/desktop/electron/updater/mac.test.ts +++ b/apps/desktop/electron/updater/mac.test.ts @@ -16,18 +16,28 @@ function fixture() { return { isUpdateAvailable: true, updateInfo: info, versionInfo: info } }), - downloadUpdate: vi.fn(async () => { events.push('download'); + downloadUpdate: vi.fn(async () => { + events.push('download') - return [] }), - quitAndInstall: vi.fn(() => { events.push('install') }), + return [] + }), + quitAndInstall: vi.fn(() => { + events.push('install') + }), on: emitter.on.bind(emitter) as MacStrategyDeps['updater']['on'], removeListener: emitter.removeListener.bind(emitter) as MacStrategyDeps['updater']['removeListener'] }, channel: 'canary', appVersion: '0.28.0', - prepareInstall: vi.fn(async () => { events.push('verify') }), - beforeInstall: vi.fn(async () => { events.push('stop') }), - onInstallFailure: vi.fn(async () => { events.push('restore') }), + prepareInstall: vi.fn(async () => { + events.push('verify') + }), + beforeInstall: vi.fn(async () => { + events.push('stop') + }), + onInstallFailure: vi.fn(async () => { + events.push('restore') + }), emitProgress: vi.fn() } @@ -48,8 +58,9 @@ describe('macOS strategy', () => { it.each(['downloadUpdate', 'prepareInstall'] as const)('keeps backends alive on %s failure', async failure => { const { deps, strategy, events, emitter } = fixture() - vi.mocked(failure === 'downloadUpdate' ? deps.updater.downloadUpdate : deps.prepareInstall) - .mockRejectedValueOnce(new Error('invalid update')) + vi.mocked(failure === 'downloadUpdate' ? deps.updater.downloadUpdate : deps.prepareInstall).mockRejectedValueOnce( + new Error('invalid update') + ) await expect(strategy.apply()).rejects.toThrow('invalid update') expect(events).not.toContain('stop') expect(events).not.toContain('install') @@ -60,7 +71,9 @@ describe('macOS strategy', () => { const { deps, strategy, events } = fixture() const info = { version: '0.27.0', files: [], releaseDate: '', path: '', sha512: '' } vi.mocked(deps.updater.checkForUpdates).mockResolvedValue({ - isUpdateAvailable: false, updateInfo: info, versionInfo: info + isUpdateAvailable: false, + updateInfo: info, + versionInfo: info }) await strategy.apply() expect(events).toEqual([]) @@ -69,15 +82,39 @@ describe('macOS strategy', () => { it('restores the backend if install handoff throws', async () => { const { deps, strategy, events } = fixture() - vi.mocked(deps.updater.quitAndInstall).mockImplementation(() => { throw new Error('handoff failed') }) + vi.mocked(deps.updater.quitAndInstall).mockImplementation(() => { + throw new Error('handoff failed') + }) await expect(strategy.apply()).rejects.toThrow('handoff failed') expect(events.slice(-2)).toEqual(['stop', 'restore']) }) + it('preserves the handoff error when backend recovery also fails', async (): Promise => { + const { deps, strategy, emitter } = fixture() + const handoff = new Error('native handoff failed') + const recovery = new Error('backend recovery failed') + vi.mocked(deps.updater.quitAndInstall).mockImplementation((): never => { + throw handoff + }) + vi.mocked(deps.onInstallFailure).mockRejectedValue(recovery) + await expect(strategy.apply()).rejects.toMatchObject({ cause: handoff, errors: [handoff, recovery] }) + expect(deps.emitProgress).toHaveBeenLastCalledWith({ + stage: 'error', + message: 'native handoff failed; backend recovery failed', + percent: null + }) + expect(emitter.listenerCount('download-progress')).toBe(0) + }) + it('rejects simultaneous apply calls', async () => { const { deps, strategy } = fixture() let release!: () => void - vi.mocked(deps.prepareInstall).mockImplementation(() => new Promise(resolve => { release = resolve })) + vi.mocked(deps.prepareInstall).mockImplementation( + () => + new Promise(resolve => { + release = resolve + }) + ) const applying = strategy.apply() await vi.waitFor(() => expect(deps.prepareInstall).toHaveBeenCalledOnce()) await expect(strategy.apply()).rejects.toThrow('already in progress') @@ -91,7 +128,11 @@ describe('native signature verification', () => { it('waits for native readiness and removes both listeners', async () => { const native = Object.assign(new EventEmitter(), { checkForUpdates: vi.fn() }) let ready = false - const pending = prepareMacInstall(native).then(() => { ready = true }) + + const pending = prepareMacInstall(native).then(() => { + ready = true + }) + await Promise.resolve() expect(ready).toBe(false) native.emit('update-downloaded') diff --git a/apps/desktop/electron/updater/mac.ts b/apps/desktop/electron/updater/mac.ts index 12ddfc17f7..03a4df8f7b 100644 --- a/apps/desktop/electron/updater/mac.ts +++ b/apps/desktop/electron/updater/mac.ts @@ -1,5 +1,7 @@ import type { AppUpdater } from 'electron-updater' +import { applyPackagedHandoff } from './packaged-handoff' + import type { UpdaterApplyResultWire, UpdaterStatusWire, UpdaterStrategy } from './index' export interface MacStrategyDeps { @@ -20,7 +22,9 @@ export class MacStrategy implements UpdaterStrategy { constructor(private readonly deps: MacStrategyDeps) {} async check(): Promise { - if (this.applying) { throw new Error('An update is already in progress.') } + if (this.applying) { + throw new Error('An update is already in progress.') + } return this.checkRelease() } @@ -28,7 +32,9 @@ export class MacStrategy implements UpdaterStrategy { private async checkRelease(): Promise { const result = await this.deps.updater.checkForUpdates() - if (!result) { throw new Error('The macOS updater is not active for this app.') } + if (!result) { + throw new Error('The macOS updater is not active for this app.') + } return { supported: true, @@ -42,9 +48,11 @@ export class MacStrategy implements UpdaterStrategy { } async apply(): Promise { - if (this.applying) { throw new Error('An update is already in progress.') } + if (this.applying) { + throw new Error('An update is already in progress.') + } + this.applying = true - let stopped = false const progress = ({ percent }: { percent: number }): void => { this.deps.emitProgress({ stage: 'fetch', message: 'Downloading the Hermes update.', percent }) @@ -53,21 +61,33 @@ export class MacStrategy implements UpdaterStrategy { this.deps.updater.on('download-progress', progress) try { - const status = await this.checkRelease() + return await applyPackagedHandoff( + { + teardown: this.deps.beforeInstall, + restore: this.deps.onInstallFailure, + emitProgress: this.deps.emitProgress + }, + async (stop: () => Promise): Promise => { + const status = await this.checkRelease() - if (!status.updateAvailable) { return { ok: true, mechanism: this.mechanism } } - await this.deps.updater.downloadUpdate() - this.deps.emitProgress({ stage: 'prepare', message: 'Verifying the signed macOS update.', percent: null }) - await this.deps.prepareInstall() - stopped = true - await this.deps.beforeInstall() - this.deps.emitProgress({ stage: 'restart', message: 'Restarting Hermes to install the update.', percent: 100 }) - this.deps.updater.quitAndInstall() + if (!status.updateAvailable) { + return { ok: true, mechanism: this.mechanism } + } - return { ok: true, bundled: true, handedOff: true, mechanism: this.mechanism } - } catch (error) { - if (stopped) { await this.deps.onInstallFailure() } - throw error + await this.deps.updater.downloadUpdate() + this.deps.emitProgress({ stage: 'prepare', message: 'Verifying the signed macOS update.', percent: null }) + await this.deps.prepareInstall() + await stop() + this.deps.emitProgress({ + stage: 'restart', + message: 'Restarting Hermes to install the update.', + percent: 100 + }) + this.deps.updater.quitAndInstall() + + return { ok: true, bundled: true, handedOff: true, mechanism: this.mechanism } + } + ) } finally { this.deps.updater.removeListener('download-progress', progress) this.applying = false @@ -84,21 +104,32 @@ export interface NativeMacUpdater { } /** Download completion alone does not mean Squirrel accepted the signature. */ -export function prepareMacInstall(native: NativeMacUpdater, timeoutMs = 120_000): Promise { - return new Promise((resolve, reject) => { +export function prepareMacInstall(native: NativeMacUpdater, timeoutMs: number = 120_000): Promise { + return new Promise((resolve: () => void, reject: (error: Error) => void): void => { const cleanup = (): void => { clearTimeout(timer) native.removeListener('error', failed) native.removeListener('update-downloaded', ready) } - const failed = (error: Error): void => { cleanup(); reject(error) } + const failed = (error: Error): void => { + cleanup() + reject(error) + } - const ready = (): void => { cleanup(); resolve() } - const timer = setTimeout(() => failed(new Error('macOS update verification timed out.')), timeoutMs) + const ready = (): void => { + cleanup() + resolve() + } + + const timer = setTimeout((): void => failed(new Error('macOS update verification timed out.')), timeoutMs) native.once('error', failed) native.once('update-downloaded', ready) - try { native.checkForUpdates() } catch (error) { failed(error as Error) } + try { + native.checkForUpdates() + } catch (error) { + failed(error as Error) + } }) } diff --git a/apps/desktop/electron/updater/packaged-handoff.ts b/apps/desktop/electron/updater/packaged-handoff.ts new file mode 100644 index 0000000000..8e0fa62e3c --- /dev/null +++ b/apps/desktop/electron/updater/packaged-handoff.ts @@ -0,0 +1,62 @@ +import type { RelaunchRegistration } from './relaunch' + +import type { UpdaterApplyResultWire } from './index' + +interface PackagedHandoffDeps { + teardown: () => void | Promise + restore: () => Promise + emitProgress: (progress: { stage: string; message: string; percent: number | null }) => void + relaunch?: { + register: () => Promise + onManual: () => void + } +} + +/** Native preparation decides when it is safe to stop. Recovery has one owner. */ +export async function applyPackagedHandoff( + deps: PackagedHandoffDeps, + apply: (stop: () => Promise) => Promise +): Promise { + let registration: RelaunchRegistration | undefined + let teardownStarted = false + + const stop = async (): Promise => { + if (deps.relaunch) { + registration = await deps.relaunch.register() + + if (!registration.automatic) { + deps.relaunch.onManual() + } + } + + teardownStarted = true + await deps.teardown() + } + + try { + return await apply(stop) + } catch (error) { + const errors: unknown[] = [error] + + try { + await registration?.cancel() + } catch (cancelError) { + errors.push(cancelError) + } + + if (teardownStarted) { + try { + await deps.restore() + } catch (restoreError) { + errors.push(restoreError) + } + } + + const message = errors + .map((item: unknown): string => (item instanceof Error ? item.message : String(item))) + .join('; ') + + deps.emitProgress({ stage: 'error', message, percent: null }) + throw errors.length > 1 ? new AggregateError(errors, message, { cause: error }) : error + } +} diff --git a/apps/desktop/electron/updater/state-db-preflight.test.ts b/apps/desktop/electron/updater/state-db-preflight.test.ts new file mode 100644 index 0000000000..ab5267225d --- /dev/null +++ b/apps/desktop/electron/updater/state-db-preflight.test.ts @@ -0,0 +1,108 @@ +import assert from 'node:assert/strict' +import { spawn, spawnSync } from 'node:child_process' +import { once } from 'node:events' +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { fileURLToPath } from 'node:url' + +import { test } from 'vitest' + +import { preflightStateDb } from './state-db-preflight' + +test('the desktop preflight publishes committed WAL rows before its caller can stop the backend', async (): Promise => { + const home: string = fs.mkdtempSync(path.join(os.tmpdir(), 'desktop-db-')) + const python: string = process.env.HERMES_PYTHON || 'python3' + const script: string = fileURLToPath(new URL('../../../../hermes_cli/backup_sqlite.py', import.meta.url)) + + const child = spawn( + python, + [ + '-I', + '-S', + '-u', + '-c', + ` +import sqlite3, sys +c = sqlite3.connect(sys.argv[1]) +c.execute('PRAGMA journal_mode=WAL') +c.execute('PRAGMA wal_autocheckpoint=0') +c.execute('CREATE TABLE messages (body TEXT)') +c.commit() +c.execute('PRAGMA wal_checkpoint(TRUNCATE)') +c.execute("INSERT INTO messages VALUES ('pending in WAL')") +c.commit() +print('ready', flush=True) +sys.stdin.readline() +c.close() +`, + path.join(home, 'state.db') + ], + { stdio: ['pipe', 'pipe', 'pipe'] } + ) + + const logs: string[] = [] + + try { + await once(child.stdout!, 'data') + preflightStateDb({ + python, + script, + home, + log: (message: string): void => { + logs.push(message) + } + }) + assert.equal(child.exitCode, null) + const backups: string[] = fs.readdirSync(home).filter((name: string): boolean => name.endsWith('.bak')) + assert.equal(backups.length, 1, logs.join('\n')) + + const verify = spawnSync( + python, + [ + '-I', + '-S', + '-c', + ` +import sqlite3, sys +with sqlite3.connect(sys.argv[1]) as c: + assert c.execute('SELECT body FROM messages').fetchall() == [('pending in WAL',)] +`, + path.join(home, backups[0]!) + ], + { encoding: 'utf8' } + ) + + assert.equal(verify.status, 0, verify.stderr) + const exited = once(child, 'exit') + child.stdin!.end('\n') + await exited + } finally { + if (child.exitCode === null && child.signalCode === null) { + child.kill('SIGKILL') + await once(child, 'exit') + } + + fs.rmSync(home, { recursive: true, force: true }) + } +}) + +test('an older selected checkout without the snapshot helper refuses before backend stop', (): void => { + const oldRoot: string = fs.mkdtempSync(path.join(os.tmpdir(), 'old-preflight-')) + let stopped = false + + try { + assert.throws((): void => { + preflightStateDb({ + python: process.env.HERMES_PYTHON || 'python3', + script: path.join(oldRoot, 'hermes_cli', 'backup_sqlite.py'), + home: oldRoot, + log: (): void => {} + }) + stopped = true + }, /snapshot|pre-flight/) + assert.equal(stopped, false) + } finally { + fs.rmSync(oldRoot, { recursive: true, force: true }) + } +}) diff --git a/apps/desktop/electron/updater/state-db-preflight.ts b/apps/desktop/electron/updater/state-db-preflight.ts new file mode 100644 index 0000000000..bbc8ffdf61 --- /dev/null +++ b/apps/desktop/electron/updater/state-db-preflight.ts @@ -0,0 +1,34 @@ +import { execFileSync } from 'node:child_process' + +import { hiddenWindowsChildOptions } from '../windows-child-options' + +interface StateDbPreflight { + python: string | null + script: string + home: string + log: (message: string) => void +} + +// Synchronous by design: the caller must not stop the backend before the snapshot. +export function preflightStateDb({ python, script, home, log }: StateDbPreflight): void { + try { + if (!python) { + throw new Error('Python not found') + } + + const result: string = execFileSync( + python, + ['-I', '-S', script, home], + hiddenWindowsChildOptions({ encoding: 'utf8', timeout: 30_000, stdio: ['ignore', 'pipe', 'pipe'] }) + ) + + log(`[updates] state.db pre-flight: ${result.trim()}`) + } catch (error: unknown) { + const message = + `state.db pre-flight failed: ${error instanceof Error ? error.message : String(error)}. ` + + 'Update cancelled before backend shutdown. Update the selected installation with its hermes update command, then retry.' + + log(`[updates] ${message}`) + throw new Error(message, { cause: error }) + } +} diff --git a/apps/desktop/electron/updater/store.test.ts b/apps/desktop/electron/updater/store.test.ts index 3ed532176f..9098a85fce 100644 --- a/apps/desktop/electron/updater/store.test.ts +++ b/apps/desktop/electron/updater/store.test.ts @@ -38,6 +38,20 @@ function dependencies(failAt?: string): { deps: StoreStrategyDeps; calls: string } describe('Microsoft Store update lifecycle', () => { + it('requires automatic relaunch and cancels registration without stopping the backend', async (): Promise => { + const { deps, calls } = dependencies() + deps.registerPendingRelaunch = async (): Promise< + Awaited> + > => ({ + automatic: false, + cancel: async (): Promise => { + calls.push('cancel') + } + }) + await expect(new StoreStrategy(deps).apply()).rejects.toThrow('Could not register automatic relaunch') + expect(calls).toEqual(['download', 'cancel']) + }) + it('downloads before shutdown, owns relaunch before install, and keeps no-update non-destructive', async () => { const { deps, calls } = dependencies() const strategy = new StoreStrategy(deps) diff --git a/apps/desktop/electron/updater/store.ts b/apps/desktop/electron/updater/store.ts index 48a3b7297e..dd1412c1b4 100644 --- a/apps/desktop/electron/updater/store.ts +++ b/apps/desktop/electron/updater/store.ts @@ -1,3 +1,4 @@ +import { applyPackagedHandoff } from './packaged-handoff' import type { RelaunchRegistration } from './relaunch' import type { UpdaterApplyResultWire, UpdaterStatusWire, UpdaterStrategy } from './index' @@ -40,63 +41,50 @@ export class StoreStrategy implements UpdaterStrategy { } async apply(): Promise { - let registration: RelaunchRegistration | undefined - let stopped = false - - try { - this.deps.emitProgress({ stage: 'fetch', message: 'Downloading the update from Microsoft Store.', percent: null }) - const downloaded = await this.deps.run('download') - - if (!downloaded.ok || downloaded.available === null) { - throw new Error(downloaded.error || 'Microsoft Store download did not complete') - } - - if (!downloaded.available) { - return { ok: true, updateAvailable: false, mechanism: this.mechanism } - } - - registration = await this.deps.registerPendingRelaunch(this.deps.appVersion) - - if (!registration.automatic) { - throw new Error('Could not register automatic relaunch for the Store update') - } - - stopped = true - await this.deps.teardown() - this.deps.emitProgress({ - stage: 'restart', - message: 'Microsoft Store is installing the update. Hermes will reopen.', - percent: null - }) - const installed = await this.deps.run('install') - - if (!installed.ok || installed.available !== true) { - throw new Error(installed.error || 'Microsoft Store did not confirm installation') - } - - this.deps.quit() - - return { ok: true, bundled: true, handedOff: true, mechanism: this.mechanism } - } catch (error) { - const errors: unknown[] = [error] - - try { - await registration?.cancel() - } catch (cancelError) { - errors.push(cancelError) - } - - if (stopped) { - try { - await this.deps.restore() - } catch (restoreError) { - errors.push(restoreError) + return applyPackagedHandoff( + { + teardown: this.deps.teardown, + restore: this.deps.restore, + emitProgress: this.deps.emitProgress, + relaunch: { + register: (): Promise => this.deps.registerPendingRelaunch(this.deps.appVersion), + onManual: (): never => { + throw new Error('Could not register automatic relaunch for the Store update') + } } - } + }, + async (stop: () => Promise): Promise => { + this.deps.emitProgress({ + stage: 'fetch', + message: 'Downloading the update from Microsoft Store.', + percent: null + }) + const downloaded = await this.deps.run('download') - const message = errors.map(item => (item instanceof Error ? item.message : String(item))).join('; ') - this.deps.emitProgress({ stage: 'error', message, percent: null }) - throw errors.length > 1 ? new AggregateError(errors, message, { cause: error }) : error - } + if (!downloaded.ok || downloaded.available === null) { + throw new Error(downloaded.error || 'Microsoft Store download did not complete') + } + + if (!downloaded.available) { + return { ok: true, updateAvailable: false, mechanism: this.mechanism } + } + + await stop() + this.deps.emitProgress({ + stage: 'restart', + message: 'Microsoft Store is installing the update. Hermes will reopen.', + percent: null + }) + const installed = await this.deps.run('install') + + if (!installed.ok || installed.available !== true) { + throw new Error(installed.error || 'Microsoft Store did not confirm installation') + } + + this.deps.quit() + + return { ok: true, bundled: true, handedOff: true, mechanism: this.mechanism } + } + ) } } diff --git a/apps/desktop/electron/windows-child-options.test.ts b/apps/desktop/electron/windows-child-options.test.ts index a68b9dfc5e..14a6de355c 100644 --- a/apps/desktop/electron/windows-child-options.test.ts +++ b/apps/desktop/electron/windows-child-options.test.ts @@ -2,7 +2,8 @@ import assert from 'node:assert/strict' import { test } from 'vitest' -import { stopBackendChild, stopBackendTreesForUpdate } from './backend-child' +import { stopBackendChild } from './backend-child' +import { createLocalBackendLifecycle } from './local-backend-lifecycle' import { hiddenWindowsChildOptions } from './windows-child-options' test('hiddenWindowsChildOptions adds windowsHide:true on Windows when unset', () => { @@ -148,21 +149,33 @@ test('stopBackendChild swallows errors thrown by the kill strategy', () => { }) }) -test('Windows update tree-kills captured roots without pre-signalling the primary backend', () => { +test('Windows shutdown tree-kills before waiting, and joins an overlapping stop', async (): Promise => { const primary = makeChild({ pid: 101 }) - const pooled = makeChild({ pid: 202 }) const events: string[] = [] + let exit!: () => void - stopBackendTreesForUpdate(primary.child, { - forceKillProcessTree: pid => events.push(`tree:${pid}`), - stopAllPoolBackends: () => { - events.push('pool-stop') - // Production stopAllPoolBackends() already tree-kills every pool root. - events.push(`tree:${pooled.child.pid}`) - } + const lifecycle = createLocalBackendLifecycle({ + stopChild: (child: typeof primary.child): void => + stopBackendChild(child, { + forceKillProcessTree: (pid: number): void => { + events.push(`tree:${pid}`) + }, + isWindows: true + }), + waitForExit: (): Promise => + new Promise((resolve: () => void): void => { + events.push('wait') + exit = resolve + }), + cancelSetup: (): void => {} }) - assert.deepEqual(events, ['tree:101', 'pool-stop', 'tree:202']) - assert.deepEqual(primary.calls, [], 'the primary root must not be signalled before taskkill /T sees it') - assert.deepEqual(pooled.calls, []) + const child = lifecycle.spawn((): typeof primary.child => primary.child) + const stopped = lifecycle.stop(child) + assert.equal(lifecycle.stop(child), stopped) + const shutdown = lifecycle.shutdown() + assert.deepEqual(events, ['tree:101', 'wait']) + assert.deepEqual(primary.calls, [], 'taskkill must enumerate descendants before the root can exit') + exit() + await Promise.all([stopped, shutdown]) }) diff --git a/apps/desktop/scripts/gen-appinstaller.mjs b/apps/desktop/scripts/gen-appinstaller.mjs deleted file mode 100644 index 078d88fbc0..0000000000 --- a/apps/desktop/scripts/gen-appinstaller.mjs +++ /dev/null @@ -1,71 +0,0 @@ -#!/usr/bin/env node -// gen-appinstaller.mjs — generate the Windows App Installer (.appinstaller) -// file for an out-of-store MSIX channel feed. -// -// The out-of-store distribution is App Installer owned: each stable/canary -// channel dir under the feed host holds a universal .msixbundle plus an -// .appinstaller that (a) installs the bundle and (b) records the .appinstaller -// URI as the package's update source, so the OS can re-check it on launch. -// -// The identity comes from product-identity.cjs via scripts/msix-shared.mjs -// (the SAME single derivation as the package manifest), so the -// .appinstaller's MainBundle Name/Publisher always match the bundle's -// manifest. `store` has no appinstaller (the Store owns its distribution). -// -// Pure buildAppInstaller() lives in scripts/msix-shared.mjs and is -// unit-tested; this module is the CLI wrapper: -// node apps/desktop/scripts/gen-appinstaller.mjs --out --base-url -// Reads HERMES_DESKTOP_VARIANT (bundled|light), HERMES_PAYLOAD_TAG (channel) -// and package.json (version) like the rest of the build. -import fs from 'node:fs' -import path from 'node:path' -import { fileURLToPath, pathToFileURL } from 'node:url' - -import { appIdentity, buildAppInstaller } from '../../../scripts/msix-shared.mjs' - -const desktop = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..') - -const isCli = process.argv[1] && import.meta.url === pathToFileURL(path.resolve(process.argv[1])).href - -if (isCli) { - // node strips the first '--' (and an immediately-following option) for its - // own use; --out lands as a BARE arg. Parse space-separated flag pairs, - // not --flag=value, so the CLI survives that mangling. - const args = process.argv.slice(2) - const flagValue = (name) => { - for (let i = 0; i < args.length - 1; i += 1) { - if (args[i] === name) return args[i + 1] - } - return undefined - } - const out = flagValue('--out') - const baseUrl = flagValue('--base-url') || process.env.CLOUDFLARE_R2_PUBLIC_URL - - if (!out || !baseUrl) { - console.error('[gen-appinstaller] --out= and --base-url= (or CLOUDFLARE_R2_PUBLIC_URL) are required') - process.exit(1) - } - - const { identity, version, name } = appIdentity(desktop, process.env.HERMES_PAYLOAD_TAG) - if (!identity.channel) { - console.error('[gen-appinstaller] Store and commit builds have no App Installer feed') - process.exit(1) - } - - const canary = /-canary/.test(process.env.HERMES_PAYLOAD_TAG || '') - const variantDir = identity.light ? 'light/' : '' - const ch = canary ? 'canary' : 'stable' - const channelPath = `win32/${variantDir}${ch}` - - const xml = buildAppInstaller({ - baseUrl: String(baseUrl).replace(/\/+$/, ''), - variantChannelPath: channelPath, - identityName: identity.msixAppIdWithOrg, - version, - bundleFilename: `${name}-${version}-win.msixbundle` - }) - - fs.mkdirSync(path.dirname(out), { recursive: true }) - fs.writeFileSync(out, xml) - console.log(`[gen-appinstaller] wrote ${out} (${channelPath}/${name}-${version}-win.msixbundle)`) -} diff --git a/apps/desktop/scripts/gen-appinstaller.test.mjs b/apps/desktop/scripts/gen-appinstaller.test.mjs deleted file mode 100644 index 129e12607c..0000000000 --- a/apps/desktop/scripts/gen-appinstaller.test.mjs +++ /dev/null @@ -1,59 +0,0 @@ -// gen-appinstaller — the .appinstaller document for an out-of-store channel. -// The identity inside must match the package manifest (same derivation), and -// the bundle/package URLs must resolve under the feed host. -import assert from 'node:assert/strict' - -import { describe, test } from 'vitest' - -import { OUT_OF_STORE_PUBLISHER, buildAppInstaller } from '../../../scripts/msix-shared.mjs' - -describe('buildAppInstaller', () => { - const base = { - baseUrl: 'https://updates.example.com', - variantChannelPath: 'win32/stable', - identityName: 'NousResearch.HermesBundled', - version: '0.3.0.0', - bundleFilename: 'HermesBundled-0.3.0.0-win.msixbundle' - } - - test('pins the same publisher as the out-of-store manifest (ATS cert subject)', () => { - const xml = buildAppInstaller(base) - assert.match(xml, /Publisher="CN=Nous Research Inc\., O=Nous Research Inc\., L=Austin, S=Texas, C=US"/) - assert.equal(OUT_OF_STORE_PUBLISHER, 'CN=Nous Research Inc., O=Nous Research Inc., L=Austin, S=Texas, C=US') - }) - - test('MainBundle points at the package bytes and self URI stays on the published channel descriptor', () => { - const xml = buildAppInstaller(base) - assert.match(xml, / { - const xml = buildAppInstaller(base) - assert.match(xml, /Name="NousResearch\.HermesBundled"/) - const versionCount = (xml.match(/Version="0\.3\.0\.0"/g) || []).length - // AppInstaller Version + MainBundle Version = 2 occurrences. - assert.equal(versionCount, 2) - }) - - test('UpdateSettings keeps the OS prompt off (the in-app checker owns the prompt)', () => { - const xml = buildAppInstaller(base) - assert.match(xml, //) - assert.doesNotMatch(xml, /ShowPrompt=/) - }) - - test('a variant channel path with a trailing slash still resolves under the host', () => { - const xml = buildAppInstaller({ ...base, variantChannelPath: 'win32/canary/' }) - assert.match(xml, /https:\/\/updates\.example\.com\/win32\/canary\//) - }) - - test('reserved XML characters in identity values are escaped', () => { - const xml = buildAppInstaller({ ...base, identityName: 'A&B' }) - assert.match(xml, /Name="A&B<App>"/) - }) -}) diff --git a/apps/desktop/scripts/msix-shared.test.mjs b/apps/desktop/scripts/msix-shared.test.mjs index fceddc18cf..5c094cc423 100644 --- a/apps/desktop/scripts/msix-shared.test.mjs +++ b/apps/desktop/scripts/msix-shared.test.mjs @@ -89,16 +89,3 @@ test('legacy 8-digit canary stamp still computes minutes (midnight of that day)' const minutes = msix.canaryBuildMinutesFor('v0.27.2-canary.20260801', base) assert.equal(minutes, 0) }) - -test('buildAppInstaller pins the derived 4-part version everywhere', () => { - const xml = msix.buildAppInstaller({ - baseUrl: 'https://updates.example.com', - variantChannelPath: 'win32/canary', - identityName: 'NousResearch.HermesBundled', - version: '0.27.2.1234', - bundleFilename: 'HermesBundled-0.27.2.1234-win.msixbundle' - }) - assert.match(xml, /Uri="https:\/\/updates\.example\.com\/win32\/canary\/HermesBundled-0\.27\.2\.1234-win\.msixbundle"/) - const versionCount = (xml.match(/Version="0\.27\.2\.1234"/g) || []).length - assert.equal(versionCount, 2) -}) diff --git a/apps/desktop/scripts/side-by-side.windows.test.mjs b/apps/desktop/scripts/side-by-side.windows.test.mjs index c4c744c948..33c557ebd5 100644 --- a/apps/desktop/scripts/side-by-side.windows.test.mjs +++ b/apps/desktop/scripts/side-by-side.windows.test.mjs @@ -48,7 +48,7 @@ async function nativeProof() { 'apps/desktop/product-identity.cjs', 'apps/desktop/electron-builder.config.cjs', 'apps/desktop/package.json', 'apps/desktop/update-feed.cjs', 'apps/desktop/update-feed.json', 'apps/desktop/assets/msix-manifest.xml', - ...['before-build', 'gen-msix-manifest', 'gen-appinstaller', 'mac-sign', 'payload-digests', 'write-build-stamp', 'utils'] + ...['before-build', 'gen-msix-manifest', 'mac-sign', 'payload-digests', 'write-build-stamp', 'utils'] .map(name => `apps/desktop/scripts/${name}.mjs`), 'scripts/msix-shared.mjs', 'scripts/release-content-types.json', 'scripts/build/python.mjs', ] @@ -159,13 +159,24 @@ foreach ($asset in @(@('Square44x44Logo.png',44,44), @('Square150x150Logo.png',1 } } const descriptor = path.join(root, `${label}.appinstaller`) - const generated = run(process.execPath, [path.join(desktop, 'scripts/gen-appinstaller.mjs'), '--out', descriptor, '--base-url', 'https://example.invalid/fixture'], { cwd: work, env: childEnv }) - if (flavorEnv.HERMES_BUILD_COMMIT) { - check(generated.status !== 0 && !fs.existsSync(descriptor), `${label}: commit build emitted App Installer feed`) - check(facts.config.publish === null, `${label}: commit build config still publishes`) + if (facts.identity.channel) { + const publisher = attribute(roundtrip, 'Identity', 'Publisher') + const selfUri = `https://example.invalid/fixture/${facts.identity.channel}.appinstaller` + const artifactUri = `https://example.invalid/fixture/${facts.app.name}-${version}-win.msixbundle` + checked(process.env.HERMES_PYTHON || 'python', [ + '-m', 'scripts.bundles.release_artifacts', 'appinstaller', '--root', root, '--out', descriptor, + '--identity', name, '--publisher', publisher, '--version', version, + '--self-uri', selfUri, '--artifact-uri', artifactUri, + ], { cwd: repo, env: childEnv }) + const feed = fs.readFileSync(descriptor, 'utf8') + check(attribute(feed, 'MainBundle', 'Name') === name, `${label}: App Installer targets another family`) + check(attribute(feed, 'MainBundle', 'Publisher') === publisher, `${label}: App Installer changed publisher`) + check(attribute(feed, 'MainBundle', 'Version') === version, `${label}: App Installer changed version`) + check(attribute(feed, 'AppInstaller', 'Uri') === selfUri, `${label}: App Installer changed subscription`) + check(attribute(feed, 'MainBundle', 'Uri') === artifactUri, `${label}: App Installer changed artifact`) } else { - assert.equal(generated.status, 0, generated.output) - check(attribute(fs.readFileSync(descriptor, 'utf8'), 'MainBundle', 'Name') === name, `${label}: App Installer targets another family`) + check(!fs.existsSync(descriptor), `${label}: commit build emitted App Installer feed`) + check(facts.config.publish === null, `${label}: commit build config still publishes`) } const row = { label, name, version, aliases, packageDir, cliName: facts.identity.cliName, commit: flavorEnv.HERMES_BUILD_COMMIT || null } rows.push(row) diff --git a/apps/desktop/src/api/client.ts b/apps/desktop/src/api/client.ts index b61f0d33b4..256feb1064 100644 --- a/apps/desktop/src/api/client.ts +++ b/apps/desktop/src/api/client.ts @@ -1,4 +1,5 @@ import { JsonRpcGatewayClient } from '@hermes/shared' +import { map, type MapStore } from 'nanostores' import type { HermesApiRequest } from '@/global' @@ -44,14 +45,20 @@ export class HermesGateway extends JsonRpcGatewayClient { // REST handlers accept profile reuse the primary dashboard via ?profile=; // unscoped handlers retain a profile backend. Remote overrides still route to // their owning backend. Null → primary, so single-profile users are unaffected. -let _apiProfile: null | string = null +interface ApiRequestScope { + profile: string | null + connectionId: string | null +} + +// This is the request authority, not a second copy in a presentation store. +export const $apiRequestScope: MapStore = map({ profile: null, connectionId: null }) export function setApiRequestProfile(profile: null | string): void { - _apiProfile = profile || null + $apiRequestScope.setKey('profile', profile || null) } export function profileScoped(profile?: null | string): { profile?: string } { - const selected = profile === undefined ? _apiProfile : profile + const selected = profile === undefined ? $apiRequestScope.get().profile : profile return selected ? { profile: selected } : {} } @@ -60,7 +67,7 @@ export function profileScoped(profile?: null | string): { profile?: string } { * Read-only twin of setApiRequestProfile for modules (e.g. voice playback) * that build their own connection URLs and must stay on the same backend. */ export function getApiRequestProfile(): null | string { - return _apiProfile + return $apiRequestScope.get().profile } // Registry connection serving the active gateway (null → the local pool). @@ -69,11 +76,10 @@ export function getApiRequestProfile(): null | string { // that dial their own backend (pluginSocket) resolve it through the SAME // source of truth those paths maintain for $connection. That makes the plugin // socket follow registry-agent activations too, not just profile switches. -// Same no-store-import contract as _apiProfile (avoids a cycle). -let _apiConnectionId: null | string = null +// Same no-store-import contract as profile scope (avoids a cycle). export function setApiRequestConnection(connectionId: null | string): void { - _apiConnectionId = connectionId || null + $apiRequestScope.setKey('connectionId', connectionId || null) } // Registry connection scope for a REST request. A registered remote gateway @@ -83,11 +89,13 @@ export function setApiRequestConnection(connectionId: null | string): void { // resolves to no tag, keeping single-source users byte-identical; explicit // 'local' must remain tagged when the legacy primary points elsewhere. export function connectionScoped(): { connectionId?: string } { - return _apiConnectionId ? { connectionId: _apiConnectionId } : {} + const connectionId: string | null = $apiRequestScope.get().connectionId + + return connectionId ? { connectionId } : {} } // Whether the window's primary connection is the local pool. Pushed from -// store/session's setConnection (same no-store-import contract as _apiProfile) +// store/session's setConnection (same no-store-import contract as profile scope) // so api/ helpers can name the backend an UNTAGGED request lands on without // importing the heavy session store — which would close a module cycle // through @/hermes. @@ -102,7 +110,7 @@ export function setApiRequestLocalMode(local: boolean): void { * never send this as a request pin (an explicit `'local'` bypasses Electron's * legacy per-profile remote overrides). */ export function ambientOwnerConnectionId(): string | undefined { - return _apiConnectionId ?? (_apiLocalMode ? 'local' : undefined) + return $apiRequestScope.get().connectionId ?? (_apiLocalMode ? 'local' : undefined) } /** Send a REST request to the renderer's active registry source. Request-level @@ -175,5 +183,5 @@ export function profileScopeKey(scope?: ProfileScope): string { /** Registry connection id that connection-scoped WS calls should target * (null → the local pool). Read-only twin of setApiRequestConnection. */ export function getApiRequestConnection(): null | string { - return _apiConnectionId + return $apiRequestScope.get().connectionId } diff --git a/apps/desktop/src/api/local-models-owner.test.ts b/apps/desktop/src/api/local-models-owner.test.ts new file mode 100644 index 0000000000..3ee2149da5 --- /dev/null +++ b/apps/desktop/src/api/local-models-owner.test.ts @@ -0,0 +1,24 @@ +import { beforeEach, expect, it, vi } from 'vitest' + +import { setApiRequestConnection, setApiRequestProfile } from './client' +import { getLocalModelsJobs, getLocalModelsStatus, pauseLocalDownload } from './local-models' + +beforeEach((): void => { + Object.defineProperty(window, 'hermesDesktop', { + configurable: true, + value: { api: vi.fn().mockResolvedValue({ jobs: [] }) } + }) + setApiRequestConnection('foreground') + setApiRequestProfile('default') +}) + +it('pins delayed reads and controls to their captured connection and profile', async (): Promise => { + const owner = { connectionId: 'background', profile: 'work' } + await getLocalModelsStatus(owner) + await getLocalModelsJobs(owner) + await pauseLocalDownload('download', owner) + + for (const [request] of vi.mocked(window.hermesDesktop.api).mock.calls) { + expect(request).toMatchObject(owner) + } +}) diff --git a/apps/desktop/src/api/local-models.ts b/apps/desktop/src/api/local-models.ts index 7a31ccda0b..490199573e 100644 --- a/apps/desktop/src/api/local-models.ts +++ b/apps/desktop/src/api/local-models.ts @@ -2,33 +2,41 @@ import type { LocalCatalogModel, LocalHardware, LocalModelsStatus, LocalRuntimeJ import { hermesApi, profileScoped } from './client' +export interface LocalModelsScope { + connectionId: string | null + profile: string +} + // The desktop surface of the managed llama.cpp runtime: status/catalog // reads, download/install/activate jobs, and server control. -export function getLocalModelsStatus(): Promise { +export function getLocalModelsStatus(scope?: LocalModelsScope): Promise { return hermesApi({ - ...profileScoped(), + ...(scope ?? profileScoped()), path: '/api/local-models/status' }) } -export function getLocalHardware(): Promise { +export function getLocalHardware(scope?: LocalModelsScope): Promise { return hermesApi({ - ...profileScoped(), + ...(scope ?? profileScoped()), path: '/api/local-models/hardware' }) } -export function getLocalCatalog(): Promise<{ models: LocalCatalogModel[] }> { +export function getLocalCatalog(scope?: LocalModelsScope): Promise<{ models: LocalCatalogModel[] }> { return hermesApi<{ models: LocalCatalogModel[] }>({ - ...profileScoped(), + ...(scope ?? profileScoped()), path: '/api/local-models/catalog' }) } -export function installLocalRuntime(backend?: string): Promise<{ backend: string; job_id: string; tag: string }> { +export function installLocalRuntime( + backend?: string, + scope?: LocalModelsScope +): Promise<{ backend: string; job_id: string; tag: string }> { return hermesApi<{ backend: string; job_id: string; tag: string }>({ - ...profileScoped(), + ...(scope ?? profileScoped()), body: { backend: backend ?? null }, method: 'POST', path: '/api/local-models/runtime/install' @@ -44,42 +52,45 @@ export interface QuickstartResponse { needs_runtime: boolean } -export function quickstartLocalModels(modelId?: string): Promise { +export function quickstartLocalModels(modelId?: string, scope?: LocalModelsScope): Promise { return hermesApi({ - ...profileScoped(), + ...(scope ?? profileScoped()), body: { model_id: modelId ?? null }, method: 'POST', path: '/api/local-models/quickstart' }) } -export function downloadLocalModel(modelId: string): Promise<{ already_downloaded?: boolean; job_id: null | string }> { +export function downloadLocalModel( + modelId: string, + scope?: LocalModelsScope +): Promise<{ already_downloaded?: boolean; job_id: null | string }> { return hermesApi<{ already_downloaded?: boolean; job_id: null | string }>({ - ...profileScoped(), + ...(scope ?? profileScoped()), body: { model_id: modelId }, method: 'POST', path: '/api/local-models/download' }) } -export function deleteLocalModel(modelId: string): Promise<{ ok: boolean }> { +export function deleteLocalModel(modelId: string, scope?: LocalModelsScope): Promise<{ ok: boolean }> { return hermesApi<{ ok: boolean }>({ - ...profileScoped(), + ...(scope ?? profileScoped()), method: 'DELETE', path: `/api/local-models/models/${encodeURIComponent(modelId)}` }) } -export function getLocalRuntimeJob(jobId: string): Promise { +export function getLocalRuntimeJob(jobId: string, scope?: LocalModelsScope): Promise { return hermesApi({ - ...profileScoped(), + ...(scope ?? profileScoped()), path: `/api/local-models/jobs/${encodeURIComponent(jobId)}` }) } -export function getLocalModelsJobs(): Promise<{ jobs: LocalRuntimeJob[] }> { +export function getLocalModelsJobs(scope?: LocalModelsScope): Promise<{ jobs: LocalRuntimeJob[] }> { return hermesApi<{ jobs: LocalRuntimeJob[] }>({ - ...profileScoped(), + ...(scope ?? profileScoped()), path: '/api/local-models/jobs' }) } @@ -88,45 +99,48 @@ export function getLocalModelsJobs(): Promise<{ jobs: LocalRuntimeJob[] }> { // install/update, HF-browsed). The backend answers {ok, paused} / // {ok, resumed} — a false flag (no live download handle, e.g. a // quickstart engine leg) is reported to the caller, not treated as success. -export function pauseLocalDownload(jobId: string): Promise<{ ok: boolean; paused: boolean }> { +export function pauseLocalDownload(jobId: string, scope?: LocalModelsScope): Promise<{ ok: boolean; paused: boolean }> { return hermesApi<{ ok: boolean; paused: boolean }>({ - ...profileScoped(), + ...(scope ?? profileScoped()), body: { job_id: jobId }, method: 'POST', path: '/api/local-models/download/pause' }) } -export function resumeLocalDownload(jobId: string): Promise<{ ok: boolean; resumed: boolean }> { +export function resumeLocalDownload( + jobId: string, + scope?: LocalModelsScope +): Promise<{ ok: boolean; resumed: boolean }> { return hermesApi<{ ok: boolean; resumed: boolean }>({ - ...profileScoped(), + ...(scope ?? profileScoped()), body: { job_id: jobId }, method: 'POST', path: '/api/local-models/download/resume' }) } -export function activateLocalModel(modelId: string): Promise<{ job_id: string }> { +export function activateLocalModel(modelId: string, scope?: LocalModelsScope): Promise<{ job_id: string }> { return hermesApi<{ job_id: string }>({ - ...profileScoped(), + ...(scope ?? profileScoped()), body: { model_id: modelId }, method: 'POST', path: '/api/local-models/activate' }) } -export function ejectLocalModel(modelId: string): Promise<{ ok: boolean }> { +export function ejectLocalModel(modelId: string, scope?: LocalModelsScope): Promise<{ ok: boolean }> { return hermesApi<{ ok: boolean }>({ - ...profileScoped(), + ...(scope ?? profileScoped()), body: { model_id: modelId }, method: 'POST', path: '/api/local-models/eject' }) } -export function setLocalServer(action: 'start' | 'stop'): Promise<{ ok: boolean }> { +export function setLocalServer(action: 'start' | 'stop', scope?: LocalModelsScope): Promise<{ ok: boolean }> { return hermesApi<{ ok: boolean }>({ - ...profileScoped(), + ...(scope ?? profileScoped()), body: { action }, method: 'POST', path: '/api/local-models/server' @@ -150,26 +164,31 @@ export interface HFFileGroup { fit: 'fits-gpu' | 'needs-ram' | 'too-big' | 'unknown' } -export function searchHFModels(q: string, limit = 20): Promise<{ hits: HFSearchHit[] }> { +export function searchHFModels( + q: string, + limit: number = 20, + scope?: LocalModelsScope +): Promise<{ hits: HFSearchHit[] }> { return hermesApi<{ hits: HFSearchHit[] }>({ - ...profileScoped(), + ...(scope ?? profileScoped()), path: `/api/local-models/search?q=${encodeURIComponent(q)}&limit=${limit}` }) } -export function listHFRepoFiles(repo: string): Promise<{ files: HFFileGroup[] }> { +export function listHFRepoFiles(repo: string, scope?: LocalModelsScope): Promise<{ files: HFFileGroup[] }> { return hermesApi<{ files: HFFileGroup[] }>({ - ...profileScoped(), + ...(scope ?? profileScoped()), path: `/api/local-models/search/files?repo=${encodeURIComponent(repo)}` }) } export function downloadBrowsedModel( repo: string, - paths: string[] + paths: string[], + scope?: LocalModelsScope ): Promise<{ already_downloaded?: boolean; job_id: null | string; model_id: string }> { return hermesApi<{ already_downloaded?: boolean; job_id: null | string; model_id: string }>({ - ...profileScoped(), + ...(scope ?? profileScoped()), body: { paths, repo }, method: 'POST', path: '/api/local-models/download-browsed' @@ -177,10 +196,11 @@ export function downloadBrowsedModel( } export function sideloadLocalModel( - path: string + path: string, + scope?: LocalModelsScope ): Promise<{ already_present?: boolean; model_id: string; ok: boolean }> { return hermesApi<{ already_present?: boolean; model_id: string; ok: boolean }>({ - ...profileScoped(), + ...(scope ?? profileScoped()), body: { path }, method: 'POST', path: '/api/local-models/sideload' diff --git a/apps/desktop/src/app/settings/connections-registry.test.tsx b/apps/desktop/src/app/settings/connections-registry.test.tsx index 6a5b4007f7..2409436e4f 100644 --- a/apps/desktop/src/app/settings/connections-registry.test.tsx +++ b/apps/desktop/src/app/settings/connections-registry.test.tsx @@ -1,9 +1,10 @@ -import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { act, cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { DesktopConnectionsRegistry } from '@/global' import { _resetFleetRosterForTests, refreshFleetRoster } from '@/store/fleet-roster' import { $connection } from '@/store/session' +import { deferred } from '@/test/deferred' import { ConnectionsRegistrySection, @@ -69,6 +70,88 @@ afterEach(() => { }) describe('ConnectionsRegistrySection', () => { + it('preserves, replaces and deletes stored headers through plaintext consent without selecting a source', async () => { + const active = $connection.get() + + const withHeaders: DesktopConnectionsRegistry = { + ...registry, + secureTokenStorage: false, + connections: [registry.connections[0], { ...registry.connections[1], headerNames: ['Keep', 'Replace', 'Delete'] }] + } + + list.mockResolvedValueOnce(withHeaders) + save.mockRejectedValueOnce(new Error('plaintext consent required')) + const applyConnectionConfig = vi.fn() + const select = vi.fn() + Object.assign(window.hermesDesktop, { applyConnectionConfig }) + Object.assign(window.hermesDesktop.connections, { select }) + render() + fireEvent.click(await screen.findByRole('button', { name: 'Edit' })) + const values = screen.getAllByPlaceholderText('Saved — leave blank to keep') + fireEvent.change(values[1], { target: { value: 'new-header-secret' } }) + fireEvent.click(within(screen.getByDisplayValue('Delete').parentElement!).getByRole('button', { name: 'Remove' })) + fireEvent.change(screen.getByPlaceholderText('Existing token ...abc123'), { target: { value: 'new-token' } }) + fireEvent.click(screen.getByText('Save connection')) + await screen.findByText('Store the gateway token in plain text?') + + const expected = { + id: 'homelab', + kind: 'remote', + label: 'Homelab', + url: 'http://homelab.lan:9119', + authMode: 'token', + token: 'new-token', + headers: { Keep: null, Replace: 'new-header-secret' } + } + + expect(save).toHaveBeenCalledExactlyOnceWith(expected) + fireEvent.click(screen.getByRole('button', { name: 'Save as plain text' })) + await waitFor(() => expect(save).toHaveBeenLastCalledWith({ ...expected, allowPlainTextToken: true })) + expect(applyConnectionConfig).not.toHaveBeenCalled() + expect(select).not.toHaveBeenCalled() + expect($connection.get()).toBe(active) + }) + + it('keeps stale browser sign-in out of a storage-only edit for a different URL', async () => { + const active = $connection.get() + const applyConnectionConfig = vi.fn() + const saveConnectionConfig = vi.fn() + const probeConnectionConfig = vi.fn().mockResolvedValue({ reachable: true, authMode: 'oauth', providers: [] }) + const pendingLogin = deferred<{ connected: boolean }>() + const oauthLoginConnectionConfig = vi.fn().mockReturnValue(pendingLogin.promise) + + Object.assign(window.hermesDesktop, { + applyConnectionConfig, + saveConnectionConfig, + probeConnectionConfig, + oauthLoginConnectionConfig + }) + render() + fireEvent.click(await screen.findByText('Add connection')) + fireEvent.change(screen.getByPlaceholderText('Homelab'), { target: { value: 'New gateway' } }) + const url = screen.getByPlaceholderText('http://homelab.lan:9119') + fireEvent.change(url, { target: { value: 'https://a.example' } }) + fireEvent.click(screen.getByRole('button', { name: /^(OAuth|Sign in)$/ })) + fireEvent.click(await screen.findByRole('button', { name: /Sign in with/ })) + await waitFor(() => expect(oauthLoginConnectionConfig).toHaveBeenCalledWith('https://a.example')) + fireEvent.change(url, { target: { value: 'https://b.example' } }) + await act(async (): Promise => pendingLogin.resolve({ connected: true })) + expect(screen.queryByText('Signed in')).toBeNull() + fireEvent.click(screen.getByText('Save connection')) + await waitFor(() => + expect(save).toHaveBeenCalledWith({ + kind: 'remote', + label: 'New gateway', + url: 'https://b.example', + authMode: 'oauth', + headers: {} + }) + ) + expect(applyConnectionConfig).not.toHaveBeenCalled() + expect(saveConnectionConfig).not.toHaveBeenCalled() + expect($connection.get()).toBe(active) + }) + it('refreshes a cached roster immediately after a successful connection test', async () => { _resetFleetRosterForTests() const getAgentRoster = vi.fn().mockResolvedValue({ agents: [], sources: [] }) diff --git a/apps/desktop/src/app/settings/connections-registry.tsx b/apps/desktop/src/app/settings/connections-registry.tsx index a6af04357c..af2bf5a190 100644 --- a/apps/desktop/src/app/settings/connections-registry.tsx +++ b/apps/desktop/src/app/settings/connections-registry.tsx @@ -1,12 +1,13 @@ import { useStore } from '@nanostores/react' import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' +import { RemoteSetupFields } from '@/components/remote-setup/fields' +import { useRemoteSetup } from '@/components/remote-setup/use-remote-setup' import { Button } from '@/components/ui/button' import { ConfirmDialog } from '@/components/ui/confirm-dialog' import { Input } from '@/components/ui/input' import type { DesktopConnectionKind, - DesktopConnectionProbeResult, DesktopConnectionsRegistry, DesktopRegistryConnection, DesktopRegistryConnectionInput @@ -17,23 +18,8 @@ import { connectionMatchesQuery, sortConnectionsForDisplay } from '@/lib/connection-display' -import { deriveRemoteAuthProviderShape } from '@/lib/desktop-remote-auth' import { triggerHaptic } from '@/lib/haptics' -import { - Check, - Cloud, - Globe, - Loader2, - LogIn, - Monitor, - Pencil, - Plus, - RefreshCw, - SearchIcon, - Terminal, - Trash2 -} from '@/lib/icons' -import { coerceRemoteUrlScheme } from '@/lib/remote-url' +import { Cloud, Globe, Loader2, Monitor, Pencil, Plus, RefreshCw, SearchIcon, Terminal, Trash2 } from '@/lib/icons' import { $activeConnectionId, setConnectionsRegistry } from '@/store/connections' import { refreshFleetRoster } from '@/store/fleet-roster' import { notify, notifyError } from '@/store/notifications' @@ -52,9 +38,6 @@ interface EditorState { id: null | string kind: DesktopConnectionKind label: string - url: string - authMode: 'oauth' | 'token' - token: string host: string keyPath: string // ssh remote profile, hydrated on edit so the duplicate key matches the @@ -73,9 +56,6 @@ function editorFromConnection(conn: DesktopRegistryConnection): EditorState { id: conn.id, kind: conn.kind, label: conn.label, - url: conn.url || '', - authMode: conn.authMode || 'token', - token: '', // Reconstruct the composite the single ssh host field displays. The save // payload sends ONLY this string (never separate user/port), because // normalizeSshConfig gives explicit user/port fields precedence over the @@ -93,9 +73,6 @@ function emptyEditor(kind: DesktopConnectionKind): EditorState { id: null, kind, label: '', - url: '', - authMode: 'token', - token: '', host: '', keyPath: '', remoteProfile: '', @@ -143,7 +120,7 @@ export function sshCompositeKey(composite: string): string { * Returns the existing entry the candidate collides with, or null. */ export function findDuplicateConnection( - editor: Pick, + editor: Pick & { url: string }, connections: DesktopRegistryConnection[] ): DesktopRegistryConnection | null { if (editor.kind === 'local') { @@ -255,14 +232,12 @@ export function ConnectionsRegistrySection() { // Inline duplicate rejection from the save path (dedupe is also enforced in // the main process, so a crafted payload can't slip past the UI check). const [dupeError, setDupeError] = useState(null) - // A gated remote gateway (OAuth, or username/password) never accepts a - // session token: it authenticates with a browser sign-in and keeps the - // session itself. Probe the edited URL so this row can name the provider, - // and remember whether the login round-trip actually completed. - const [authProbe, setAuthProbe] = useState(null) - const [signingIn, setSigningIn] = useState(false) - const [oauthConnected, setOauthConnected] = useState(false) - const probeSeq = useRef(0) + + const remote = useRemoteSetup({ + host: 'registry', + enabled: editor?.kind === 'remote', + onNotice: notify + }) const bridge = window.hermesDesktop?.connections @@ -273,94 +248,6 @@ export function ConnectionsRegistrySection() { setConnectionsRegistry(next) }, []) - const editorUrl = editor?.kind === 'remote' ? coerceRemoteUrlScheme(editor.url) : '' - const editorWantsOauth = editor?.kind === 'remote' && editor.authMode === 'oauth' - const authProviderShape = deriveRemoteAuthProviderShape(authProbe?.providers, t.boot.failure.identityProvider) - - // Probe only while the sign-in row is on screen, and debounce it so typing a - // URL doesn't fire a request per keystroke. Best-effort: a failed probe just - // leaves the generic provider label, it never blocks signing in. - useEffect(() => { - if (!editorWantsOauth || !editorUrl || !window.hermesDesktop?.probeConnectionConfig) { - setAuthProbe(null) - - return - } - - const seq = ++probeSeq.current - // Staleness is covered by probeSeq, but not unmount: a probe resolving - // after the editor closes would still call setAuthProbe on an unmounted - // component. Harmless in React 18, still worth not doing. - let cancelled = false - - const timer = setTimeout(() => { - window.hermesDesktop - .probeConnectionConfig(editorUrl) - .then(result => { - if (!cancelled && seq === probeSeq.current) { - setAuthProbe(result) - } - }) - .catch(() => { - if (!cancelled && seq === probeSeq.current) { - setAuthProbe(null) - } - }) - }, 400) - - return () => { - cancelled = true - clearTimeout(timer) - } - }, [editorUrl, editorWantsOauth]) - - // The session is scoped to an origin, so pointing the editor at a different - // URL invalidates the "signed in" state this row is reporting. Flipping the - // auth mode invalidates it too: a saved row edited token -> oauth must not - // present a stale "Signed in" pill from an earlier oauth stint. - useEffect(() => { - setOauthConnected(false) - }, [editorUrl, editorWantsOauth]) - - // Open the gateway's own login window and let the main process keep whatever - // it mints (native PKCE bearer tokens, or the legacy session cookies). This - // is the same IPC the first-run form and the gateway panel use — the - // registry editor simply had no affordance to reach it. - const signInOauth = useCallback(async () => { - if (!editorUrl) { - notify({ kind: 'warning', title: t.settings.gateway.authTitle, message: t.settings.gateway.enterUrlFirst }) - - return - } - - setSigningIn(true) - - try { - const result = await window.hermesDesktop.oauthLoginConnectionConfig(editorUrl) - - setOauthConnected(Boolean(result.connected)) - - if (result.connected) { - notify({ - title: t.settings.gateway.signedIn, - message: t.settings.gateway.connectedTo(authProviderShape.providerLabel) - }) - } else { - notify({ - kind: 'warning', - title: t.boot.failure.signInIncompleteTitle, - message: result?.error - ? `${t.boot.failure.signInIncompleteMessage}: ${result.error}` - : t.boot.failure.signInIncompleteMessage - }) - } - } catch (err) { - notifyError(err, t.settings.gateway.signInFailed) - } finally { - setSigningIn(false) - } - }, [authProviderShape.providerLabel, editorUrl, t]) - const load = useCallback(async () => { if (!bridge) { setLoading(false) @@ -383,13 +270,19 @@ export function ConnectionsRegistrySection() { void load() }, [load]) - const openEditor = (next: EditorState | null) => { + const openEditor = (next: EditorState | null, saved?: DesktopRegistryConnection): void => { setDupeError(null) + remote.reset({ + url: saved?.url || '', + authMode: saved?.authMode || 'token', + tokenSet: saved?.tokenSet ?? false, + tokenPreview: saved?.tokenPreview ?? null + }) setEditor(next) } const save = useCallback( - async (allowPlainTextToken = false) => { + async (allowPlainTextToken: boolean = false): Promise => { if (!bridge || !editor) { return } @@ -397,7 +290,7 @@ export function ConnectionsRegistrySection() { // Duplicate prevention lives in the save path (not just a disabled // button): reject a candidate that collides with an existing entry with // an inline error before anything crosses the IPC boundary. - const dupe = findDuplicateConnection(editor, registry?.connections ?? []) + const dupe = findDuplicateConnection({ ...editor, url: remote.credentials.url }, registry?.connections ?? []) if (dupe) { setDupeError( @@ -425,11 +318,11 @@ export function ConnectionsRegistrySection() { } if (editor.kind === 'remote' || editor.kind === 'cloud') { - payload.url = editor.url - payload.authMode = editor.authMode + payload.url = remote.payload.remoteUrl + payload.authMode = remote.credentials.authMode - if (editor.token.trim()) { - payload.token = editor.token.trim() + if (remote.payload.remoteToken) { + payload.token = remote.payload.remoteToken } if (allowPlainTextToken) { @@ -467,8 +360,8 @@ export function ConnectionsRegistrySection() { !allowPlainTextToken && registry?.secureTokenStorage === false && editor.kind === 'remote' && - editor.authMode === 'token' && - editor.token.trim() + remote.credentials.authMode === 'token' && + remote.credentials.token.trim() ) { setPlainTextConfirm(true) @@ -480,7 +373,16 @@ export function ConnectionsRegistrySection() { setSaving(false) } }, - [bridge, editor, publishRegistry, registry?.connections, registry?.secureTokenStorage, s] + [ + bridge, + editor, + remote.credentials, + remote.payload, + publishRegistry, + registry?.connections, + registry?.secureTokenStorage, + s + ] ) const remove = useCallback(async () => { @@ -722,7 +624,7 @@ export function ConnectionsRegistrySection() { <> - ))} - - } - title={t.settings.gateway.authTitle} - /> - {editor.authMode === 'token' && ( - setEditor({ ...editor, token: e.target.value })} - placeholder={t.settings.gateway.pasteSessionToken} - type="password" - value={editor.token} - /> - } - description={t.settings.gateway.tokenDesc} - title={t.settings.gateway.tokenTitle} - /> - )} - {editor.authMode === 'oauth' && ( - - {t.settings.gateway.signedIn} - - ) : ( - - ) - } - description={ - oauthConnected - ? authProviderShape.isPassword - ? t.settings.gateway.authSignedInPassword - : t.settings.gateway.authSignedInOauth - : authProviderShape.isPassword - ? t.settings.gateway.authNeedsPassword - : t.settings.gateway.authNeedsOauth(authProviderShape.providerLabel) - } - title={t.settings.gateway.authTitle} - /> - )} - - )} - {(editor.kind === 'remote' || editor.kind === 'cloud') && (
diff --git a/apps/desktop/src/app/settings/gateway-settings.test.tsx b/apps/desktop/src/app/settings/gateway-settings.test.tsx index a42a6ed77f..3a4d0a10fd 100644 --- a/apps/desktop/src/app/settings/gateway-settings.test.tsx +++ b/apps/desktop/src/app/settings/gateway-settings.test.tsx @@ -1,6 +1,8 @@ -import { cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react' +import { act, cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { deferred } from '@/test/deferred' + // Collect the component graph before the behavioral test deadline starts. import { GatewaySettings } from './gateway-settings' @@ -57,6 +59,155 @@ afterEach(() => { }) describe('GatewaySettings', () => { + it('releases a pending save after a late probe invalidates its response', async () => { + const saved = { ...localConnection, mode: 'remote', remoteUrl: 'https://a.example', remoteTokenSet: true } + getConnectionConfig.mockResolvedValue(saved) + const pendingSave = deferred() + const pendingProbe = deferred<{ reachable: boolean; authMode: string; providers: never[] }>() + saveConnectionConfig.mockReturnValueOnce(pendingSave.promise) + const probeConnectionConfig = vi.fn().mockReturnValue(pendingProbe.promise) + + Object.assign(window.hermesDesktop, { probeConnectionConfig }) + render() + const saveButton = (await screen.findByRole('button', { name: 'Save for next restart' })) as HTMLButtonElement + await waitFor(() => expect(probeConnectionConfig).toHaveBeenCalledWith('https://a.example')) + fireEvent.click(saveButton) + expect(saveConnectionConfig).toHaveBeenCalledExactlyOnceWith({ + mode: 'remote', + remoteUrl: 'https://a.example', + remoteAuthMode: 'token', + remoteToken: undefined + }) + expect(saveButton.disabled).toBe(true) + await act(async (): Promise => pendingProbe.resolve({ reachable: true, authMode: 'oauth', providers: [] })) + await act(async (): Promise => pendingSave.resolve(saved)) + expect(saveButton.disabled).toBe(false) + expect(screen.getByRole('button', { name: /Sign in with/ })).toBeTruthy() + expect(screen.queryByPlaceholderText('Existing token saved')).toBeNull() + }) + + it('pre-saves OAuth before login and applies the resolved auth mode without requiring a test', async () => { + getConnectionConfig.mockResolvedValue({ ...localConnection, mode: 'remote', remoteUrl: 'https://login.example' }) + const pendingSave = deferred() + saveConnectionConfig.mockReturnValueOnce(pendingSave.promise) + const oauthLoginConnectionConfig = vi.fn().mockResolvedValue({ connected: true }) + const applyConnectionConfig = vi.fn().mockResolvedValue(localConnection) + const testConnectionConfig = vi.fn() + Object.assign(window.hermesDesktop, { + oauthLoginConnectionConfig, + applyConnectionConfig, + testConnectionConfig, + probeConnectionConfig: vi.fn().mockResolvedValue({ + reachable: true, + authMode: 'oauth', + providers: [{ name: 'password', displayName: 'Username & Password', supportsPassword: true }] + }) + }) + render() + fireEvent.click(await screen.findByRole('button', { name: 'Sign in' })) + expect(saveConnectionConfig).toHaveBeenCalledExactlyOnceWith({ + mode: 'remote', + remoteAuthMode: 'oauth', + remoteUrl: 'https://login.example' + }) + expect(oauthLoginConnectionConfig).not.toHaveBeenCalled() + await act(async (): Promise => pendingSave.resolve()) + await screen.findByText('Signed in') + expect(oauthLoginConnectionConfig).toHaveBeenCalledExactlyOnceWith('https://login.example') + fireEvent.click(screen.getByRole('button', { name: 'Save and reconnect' })) + await waitFor(() => + expect(applyConnectionConfig).toHaveBeenCalledExactlyOnceWith({ + mode: 'remote', + remoteAuthMode: 'oauth', + remoteUrl: 'https://login.example', + remoteToken: undefined + }) + ) + expect(testConnectionConfig).not.toHaveBeenCalled() + }) + + it('keeps a saved token when blank and requires consent before replacing it in plaintext', async () => { + const saved = { + ...localConnection, + mode: 'remote', + remoteUrl: 'https://a.example', + remoteTokenSet: true, + remoteTokenPreview: 'saved-preview', + secureTokenStorage: false, + remoteTokenPlainText: true + } + + getConnectionConfig.mockResolvedValue(saved) + saveConnectionConfig.mockResolvedValue(saved) + const pendingSave = deferred() + saveConnectionConfig.mockReturnValueOnce(pendingSave.promise) + Object.assign(window.hermesDesktop, { + probeConnectionConfig: vi.fn().mockResolvedValue({ reachable: true, authMode: 'token', providers: [] }) + }) + render() + await screen.findByPlaceholderText('Existing token saved-preview') + fireEvent.click(screen.getByRole('button', { name: 'Save for next restart' })) + await waitFor(() => + expect(saveConnectionConfig).toHaveBeenCalledExactlyOnceWith({ + mode: 'remote', + remoteUrl: 'https://a.example', + remoteAuthMode: 'token', + remoteToken: undefined + }) + ) + // Flush the save's reset and probe effects before acquiring the replacement field. + await act(async (): Promise => pendingSave.resolve(saved)) + const tokenInput = await screen.findByPlaceholderText('Existing token saved-preview') + expect(tokenInput.isConnected, 'saved credential control must survive the refresh probe').toBe(true) + fireEvent.change(tokenInput, { target: { value: 'replacement' } }) + fireEvent.click(screen.getByRole('button', { name: 'Save for next restart' })) + await screen.findByText('Store the gateway token in plain text?') + expect(saveConnectionConfig).toHaveBeenCalledTimes(1) + fireEvent.click(screen.getByRole('button', { name: 'Save as plain text' })) + await waitFor(() => + expect(saveConnectionConfig).toHaveBeenLastCalledWith({ + mode: 'remote', + remoteUrl: 'https://a.example', + remoteAuthMode: 'token', + remoteToken: 'replacement', + allowPlainTextToken: true + }) + ) + }) + + it('discards an old token test while saving the current credential-ready payload', async () => { + getConnectionConfig.mockResolvedValue({ ...localConnection, mode: 'remote', remoteUrl: 'https://a.example' }) + const probeConnectionConfig = vi.fn().mockResolvedValue({ reachable: true, authMode: 'token', providers: [] }) + const pendingTest = deferred<{ ok: boolean; baseUrl: string }>() + const testConnectionConfig = vi.fn().mockReturnValue(pendingTest.promise) + + Object.assign(window.hermesDesktop, { probeConnectionConfig, testConnectionConfig }) + render() + const token = await screen.findByPlaceholderText('Paste session token') + fireEvent.change(token, { target: { value: 'old-token' } }) + fireEvent.click(screen.getByRole('button', { name: 'Test remote' })) + expect(testConnectionConfig).toHaveBeenCalledWith({ + mode: 'remote', + remoteUrl: 'https://a.example', + remoteAuthMode: 'token', + remoteToken: 'old-token' + }) + fireEvent.change(token, { target: { value: 'new-token' } }) + await act(async (): Promise => pendingTest.resolve({ ok: true, baseUrl: 'https://a.example' })) + expect(screen.queryByText('Connected to https://a.example')).toBeNull() + fireEvent.click(screen.getByRole('button', { name: 'Save for next restart' })) + await waitFor(() => + expect(saveConnectionConfig).toHaveBeenCalledWith( + expect.objectContaining({ + mode: 'remote', + remoteUrl: 'https://a.example', + remoteAuthMode: 'token', + remoteToken: 'new-token' + }) + ) + ) + }) + it('keeps saved Cloud instances usable without discovery and marks the live source, not the default', async () => { getConnectionConfig.mockResolvedValue({ ...localConnection, mode: 'cloud', remoteUrl: 'https://a.example' }) registry.value = { diff --git a/apps/desktop/src/app/settings/gateway-settings.tsx b/apps/desktop/src/app/settings/gateway-settings.tsx index ac5a2b43d2..5409a87c41 100644 --- a/apps/desktop/src/app/settings/gateway-settings.tsx +++ b/apps/desktop/src/app/settings/gateway-settings.tsx @@ -1,12 +1,14 @@ import { useStore } from '@nanostores/react' -import { useEffect, useMemo, useRef, useState } from 'react' +import { useEffect, useRef, useState } from 'react' +import { RemoteSetupFields } from '@/components/remote-setup/fields' +import { useRemoteSetup } from '@/components/remote-setup/use-remote-setup' import { Button } from '@/components/ui/button' import { ConfirmDialog } from '@/components/ui/confirm-dialog' import { Input } from '@/components/ui/input' import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select' import { Tip } from '@/components/ui/tooltip' -import type { DesktopAuthProvider, DesktopCloudAgent, DesktopCloudOrg, DesktopConnectionProbeResult } from '@/global' +import type { DesktopCloudAgent, DesktopCloudOrg, DesktopConnectionConfigInput } from '@/global' import { useI18n } from '@/i18n' import { ExternalLink } from '@/lib/external-link' import { @@ -41,7 +43,6 @@ import { enrichSelectedSshHost, selectSshHost } from './ssh-host-selection' type Mode = 'local' | 'remote' | 'cloud' | 'ssh' type AuthMode = 'oauth' | 'token' -type ProbeStatus = 'idle' | 'probing' | 'done' | 'error' // Hermes Cloud discovery lifecycle for the cloud-mode panel. type CloudDiscoverStatus = 'idle' | 'loading' | 'done' | 'error' @@ -164,15 +165,24 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = { const [loading, setLoading] = useState(true) const [saving, setSaving] = useState(false) const [testing, setTesting] = useState(false) - const [signingIn, setSigningIn] = useState(false) const [state, setState] = useState(EMPTY_STATE) - const [remoteToken, setRemoteToken] = useState('') + + const remote = useRemoteSetup({ + host: 'settings', + enabled: !loading && state.mode === 'remote', + beforeOAuthLogin: async (payload: DesktopConnectionConfigInput): Promise => { + await window.hermesDesktop.saveConnectionConfig(payload) + }, + onNotice: notify + }) + const [lastTest, setLastTest] = useState(null) const [sshHostSuggestions, setSshHostSuggestions] = useState([]) const [sshCustomHost, setSshCustomHost] = useState(false) const sshResolveSeq = useRef(0) const sshTestSeq = useRef(0) const saveSeq = useRef(0) + const saveOwner = useRef(null) const signingSeq = useRef(0) const cloudConnectSeq = useRef(0) const contextSeq = useRef(0) @@ -224,10 +234,17 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = { } } - const acceptSavedConfig = (config: GatewaySettingsState) => { + const acceptSavedConfig = (config: GatewaySettingsState): void => { const normalized = normalizeGatewaySettingsState(config) setState(normalized) + remote.reset({ + url: normalized.remoteUrl, + authMode: normalized.remoteAuthMode, + oauthConnected: normalized.remoteOauthConnected, + tokenSet: normalized.remoteTokenSet, + tokenPreview: normalized.remoteTokenPreview + }) } // When set, the plain-text opt-in dialog is open; `apply` remembers whether @@ -261,13 +278,6 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = { setCloudOrgState(value) } - // Auth-mode probe: as the user types a remote URL we ask the gateway (via - // its public /api/status) whether it gates with OAuth or a static session - // token, so we can show the right control (login button vs token box). - const [probeStatus, setProbeStatus] = useState('idle') - const [probe, setProbe] = useState(null) - const probeSeq = useRef(0) - useEffect(() => { let cancelled = false const desktop = window.hermesDesktop @@ -300,12 +310,6 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = { // eslint-disable-next-line react-hooks/exhaustive-deps -- load once on mount; copy is stable }, []) - // Debounced probe of the entered remote URL. Only runs in remote mode with a - // syntactically plausible URL. The probe result drives whether we render the - // OAuth login button or the session-token entry box. The effective auth mode - // prefers a fresh probe result over the saved value. - const trimmedUrl = coerceRemoteUrlScheme(state.remoteUrl) - const savedAgent = (agent: DesktopCloudAgent) => registry?.connections.find( connection => @@ -330,105 +334,6 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = { } } - useEffect(() => { - if (state.mode !== 'remote' || !trimmedUrl || !/^https?:\/\//i.test(trimmedUrl)) { - setProbeStatus('idle') - setProbe(null) - - return - } - - const desktop = window.hermesDesktop - - if (!desktop?.probeConnectionConfig) { - return - } - - const seq = ++probeSeq.current - setProbeStatus('probing') - - const timer = setTimeout(() => { - desktop - .probeConnectionConfig(trimmedUrl) - .then(result => { - if (seq !== probeSeq.current) { - return - } - - setProbe(result) - setProbeStatus(result.reachable ? 'done' : 'error') - }) - .catch(() => { - if (seq !== probeSeq.current) { - return - } - - setProbe(null) - setProbeStatus('error') - }) - }, 500) - - return () => clearTimeout(timer) - }, [state.mode, trimmedUrl]) - - // Effective auth mode: a reachable probe wins; otherwise fall back to the - // saved config's mode so a re-open of settings doesn't flicker. - const authMode: AuthMode = useMemo(() => { - if (probeStatus === 'done' && probe && probe.authMode !== 'unknown') { - return probe.authMode - } - - return state.remoteAuthMode - }, [probe, probeStatus, state.remoteAuthMode]) - - // Whether we actually KNOW how this gateway authenticates yet. Until we do, - // neither the OAuth button nor the session-token box should render — - // `authMode` defaults to 'token', so without this gate the token box flashes - // for every gateway (including OAuth ones) during the idle/probing window - // before the first probe lands. The scheme is known when either: - // * the live probe finished (probeStatus 'done'), or - // * we're idle but showing a previously-saved remote config (re-opening - // settings for a gateway already signed-in or with a saved token), so - // its control appears immediately with no flicker. - // While probing (or after a probe error), the scheme is unknown and we show - // the probe status row instead of a control. - const hasSavedRemote = state.remoteTokenSet || state.remoteOauthConnected - - const authResolved = useMemo(() => { - if (probeStatus === 'done') { - return true - } - - return probeStatus === 'idle' && hasSavedRemote - }, [probeStatus, hasSavedRemote]) - - const providerLabel = useMemo(() => { - const providers: DesktopAuthProvider[] = probe?.providers ?? [] - - if (providers.length === 1) { - return providers[0].displayName || providers[0].name - } - - if (providers.length > 1) { - return providers.map(p => p.displayName || p.name).join(' / ') - } - - return t.boot.failure.identityProvider - }, [probe, t.boot.failure.identityProvider]) - - // A username/password gateway authenticates through a credential form on the - // gateway's /login page (POST /auth/password-login) rather than an OAuth - // redirect. Everything downstream — the session cookie, the ws-ticket mint, - // the persistent partition — is identical, so the desktop drives it through - // the same sign-in window; only the button copy changes. We treat the - // gateway as password-style only when EVERY advertised provider supports - // password, so a mixed deployment keeps the generic OAuth copy. - const isPasswordProvider = useMemo(() => { - const providers: DesktopAuthProvider[] = probe?.providers ?? [] - - return providers.length > 0 && providers.every(p => p.supportsPassword) - }, [probe]) - useEffect(() => { // One-directional: a saved host that isn't in the suggestions must render // the free-text input (rehydration). Never force custom OFF here — that @@ -472,6 +377,9 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = { setLastTest(null) }, [ state.mode, + remote.credentials.url, + remote.credentials.token, + remote.credentials.authMode, state.sshHost, state.sshUser, state.sshPort, @@ -480,33 +388,21 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = { state.sshRemoteProfile ]) - const oauthConnected = state.remoteOauthConnected - - const canUseRemote = useMemo(() => { - if (!trimmedUrl) { - return false - } - - if (authMode === 'oauth') { - return oauthConnected - } - - return Boolean(remoteToken.trim()) || state.remoteTokenSet - }, [authMode, oauthConnected, remoteToken, state.remoteTokenSet, trimmedUrl]) - - const payload = (allowPlainTextToken?: boolean) => ({ - mode: state.mode, - remoteAuthMode: authMode, - remoteToken: authMode === 'token' ? remoteToken.trim() || undefined : undefined, - remoteUrl: trimmedUrl, - sshHost: state.sshHost.trim(), - sshUser: state.sshUser.trim() || undefined, - sshPort: state.sshPort, - sshKeyPath: state.sshKeyPath.trim() || undefined, - sshRemoteHermesPath: state.sshRemoteHermesPath.trim(), - // Preserve an intentional blank so an existing remote-profile mapping can - // be cleared instead of being mistaken for an omitted field. - sshRemoteProfile: state.sshRemoteProfile.trim(), + const payload = (allowPlainTextToken?: boolean): DesktopConnectionConfigInput => ({ + ...(state.mode === 'remote' + ? remote.payload + : { + mode: state.mode, + remoteAuthMode: state.remoteAuthMode, + remoteUrl: coerceRemoteUrlScheme(state.remoteUrl), + sshHost: state.sshHost.trim(), + sshUser: state.sshUser.trim() || undefined, + sshPort: state.sshPort, + sshKeyPath: state.sshKeyPath.trim() || undefined, + sshRemoteHermesPath: state.sshRemoteHermesPath.trim(), + // A blank clears an existing remote-profile mapping. + sshRemoteProfile: state.sshRemoteProfile.trim() + }), ...(allowPlainTextToken ? { allowPlainTextToken: true } : {}) }) @@ -515,13 +411,14 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = { // and this machine has no OS keyring (safeStorage unavailable). In that case // we must get an explicit opt-in before persisting. const wouldPersistPlainTextToken = - (state.mode === 'remote' || state.mode === 'cloud') && - authMode !== 'oauth' && - Boolean(remoteToken.trim()) && + state.mode === 'remote' && + remote.credentials.authMode === 'token' && + Boolean(remote.credentials.token.trim()) && state.secureTokenStorage === false - const performSave = async (apply: boolean, allowPlainTextToken: boolean) => { + const performSave = async (apply: boolean, allowPlainTextToken: boolean): Promise => { const seq = ++saveSeq.current + saveOwner.current = seq setSaving(true) try { @@ -534,7 +431,6 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = { } acceptSavedConfig(next) - setRemoteToken('') notify({ kind: 'success', title: apply ? g.restartingTitle : g.savedTitle, @@ -574,18 +470,20 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = { notifyError(err, apply ? g.applyFailed : g.saveFailed) } } finally { - if (seq === saveSeq.current) { + // A stale response cannot replace the draft, but its request must release busy state. + if (seq === saveOwner.current) { + saveOwner.current = null setSaving(false) } } } - const save = async (apply: boolean) => { - if (state.mode === 'remote' && !canUseRemote) { + const save = async (apply: boolean): Promise => { + if (state.mode === 'remote' && !remote.canCommit) { notify({ kind: 'warning', title: g.incompleteTitle, - message: authMode === 'oauth' ? g.incompleteSignIn : g.incompleteToken + message: remote.credentials.authMode === 'oauth' ? g.incompleteSignIn : g.incompleteToken }) return @@ -601,94 +499,6 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = { await performSave(apply, false) } - // OAuth sign-in: persist the URL + oauth mode first (so the saved config has - // the URL the login window needs), then open the gateway login window and - // refresh the connection status from the saved config once it completes. - const signIn = async () => { - const seq = ++signingSeq.current - - if (!trimmedUrl) { - notify({ kind: 'warning', title: g.incompleteTitle, message: g.enterUrlFirst }) - - return - } - - setSigningIn(true) - - try { - // Save (don't apply/restart) so the login window has a URL to use and the - // oauth mode is persisted, without yet flipping the live connection. - const saved = await window.hermesDesktop.saveConnectionConfig({ - mode: state.mode, - remoteAuthMode: 'oauth', - remoteUrl: trimmedUrl - }) - - if (seq !== signingSeq.current) { - return - } - - acceptSavedConfig(saved) - - const result = await window.hermesDesktop.oauthLoginConnectionConfig(trimmedUrl) - - if (seq !== signingSeq.current) { - return - } - - if (result.connected) { - const refreshed = await window.hermesDesktop.getConnectionConfig(null) - acceptSavedConfig(refreshed) - notify({ kind: 'success', title: g.signedIn, message: g.connectedTo(providerLabel) }) - } else { - notify({ - kind: 'warning', - title: t.boot.failure.signInIncompleteTitle, - message: result?.error - ? `${t.boot.failure.signInIncompleteMessage}: ${result.error}` - : t.boot.failure.signInIncompleteMessage - }) - } - } catch (err) { - if (seq === signingSeq.current) { - notifyError(err, g.signInFailed) - } - } finally { - if (seq === signingSeq.current) { - setSigningIn(false) - } - } - } - - const signOut = async () => { - if (!trimmedUrl) { - return - } - - const seq = ++signingSeq.current - setSigningIn(true) - - try { - await window.hermesDesktop.oauthLogoutConnectionConfig(trimmedUrl) - const refreshed = await window.hermesDesktop.getConnectionConfig(null) - - if (seq !== signingSeq.current) { - return - } - - acceptSavedConfig(refreshed) - notify({ kind: 'success', title: g.signedOutTitle, message: g.signedOutMessage }) - } catch (err) { - if (seq === signingSeq.current) { - notifyError(err, g.signOutFailed) - } - } finally { - if (seq === signingSeq.current) { - setSigningIn(false) - } - } - } - // --- Hermes Cloud handlers --- // Pull the discovered agent list over the shared portal session. Tolerant of @@ -1054,48 +864,6 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = { } } - const testRemote = async () => { - const seq = ++sshTestSeq.current - - if (!canUseRemote) { - notify({ - kind: 'warning', - title: g.incompleteTitle, - message: authMode === 'oauth' ? g.incompleteSignInTest : g.incompleteTokenTest - }) - - return - } - - setTesting(true) - setLastTest(null) - - try { - const result = await window.hermesDesktop.testConnectionConfig({ - mode: 'remote', - remoteAuthMode: authMode, - remoteToken: authMode === 'token' ? remoteToken.trim() || undefined : undefined, - remoteUrl: trimmedUrl - }) - - if (seq !== sshTestSeq.current) { - return - } - - const message = g.connectedTo(result.baseUrl || trimmedUrl, result.version ?? undefined) - setLastTest(message) - notify({ kind: 'success', title: g.reachableTitle, message }) - } catch (err) { - if (seq === sshTestSeq.current) { - notifyError(err, g.testFailed) - } - } finally { - if (seq === sshTestSeq.current) { - setTesting(false) - } - } - } - if (loading) { return ( - setState(current => ({ ...current, remoteUrl: event.target.value }))} - placeholder="https://gateway.example.com/hermes" - value={state.remoteUrl} - /> - } - description={g.remoteUrlDesc} - title={g.remoteUrlTitle} - /> - - {state.mode === 'remote' && probeStatus === 'probing' ? ( -
- - {g.probing} +
+ + {remote.credentials.authMode === 'token' && state.remoteTokenPlainText ? ( +
+
{g.plainTextStoredTitle}
+
{g.plainTextStoredDesc}
) : null} - - {state.mode === 'remote' && probeStatus === 'error' ? ( -
- - {g.probeError} -
- ) : null} - - {/* OAuth / password gateways: present a sign-in button + connection status. */} - {state.mode === 'remote' && authResolved && authMode === 'oauth' ? ( - - - {g.signedIn} - - -
- ) : ( - - ) - } - description={ - oauthConnected - ? isPasswordProvider - ? g.authSignedInPassword - : g.authSignedInOauth - : isPasswordProvider - ? g.authNeedsPassword - : g.authNeedsOauth(providerLabel) - } - title={g.authTitle} - /> - ) : null} - - {/* Session-token gateways: keep the existing token entry box. */} - {state.mode === 'remote' && authResolved && authMode === 'token' ? ( - <> - setRemoteToken(event.target.value)} - placeholder={ - state.remoteTokenSet - ? g.existingToken(state.remoteTokenPreview ?? g.savedToken) - : g.pasteSessionToken - } - type="password" - value={remoteToken} - /> - } - description={g.tokenDesc} - title={g.tokenTitle} - /> - - {/* The saved token is on disk in plain text (no OS keyring). Same - banner idiom as envOverride so it reads as a real warning. */} - {state.remoteTokenPlainText ? ( -
- -
-
{g.plainTextStoredTitle}
-
{g.plainTextStoredDesc}
-
-
- ) : null} - - ) : null}
) : null} @@ -1575,12 +1252,12 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = { {state.mode === 'remote' ? ( ) : state.mode === 'ssh' ? ( diff --git a/apps/desktop/src/app/settings/local-model-download-progress.test.tsx b/apps/desktop/src/app/settings/local-model-download-progress.test.tsx index 08decada98..269d49c352 100644 --- a/apps/desktop/src/app/settings/local-model-download-progress.test.tsx +++ b/apps/desktop/src/app/settings/local-model-download-progress.test.tsx @@ -2,7 +2,6 @@ import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-libra import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { I18nProvider } from '@/i18n' -import type * as RuntimeJobs from '@/store/local-runtime-jobs' import type * as Notifications from '@/store/notifications' import type { LocalRuntimeJob } from '@/types/hermes' @@ -16,8 +15,7 @@ vi.mock('@/store/notifications', async importOriginal => ({ notifyError: vi.fn() })) -vi.mock('@/store/local-runtime-jobs', async importOriginal => ({ - ...(await importOriginal()), +vi.mock('@/store/local-runtime-jobs', (): object => ({ watchLocalRuntimeJobs: vi.fn() })) @@ -71,7 +69,7 @@ describe('LocalModelDownloadActions', () => { await waitFor(() => { expect(pauseLocalDownload).toHaveBeenCalledTimes(1) }) - expect(pauseLocalDownload).toHaveBeenCalledWith('j1') + expect(pauseLocalDownload).toHaveBeenCalledWith('j1', undefined) expect(resumeLocalDownload).not.toHaveBeenCalled() }) @@ -105,7 +103,7 @@ describe('LocalModelDownloadActions', () => { await waitFor(() => { expect(resumeLocalDownload).toHaveBeenCalledTimes(1) }) - expect(resumeLocalDownload).toHaveBeenCalledWith('j1') + expect(resumeLocalDownload).toHaveBeenCalledWith('j1', undefined) expect(pauseLocalDownload).not.toHaveBeenCalled() }) diff --git a/apps/desktop/src/app/settings/local-model-download-progress.tsx b/apps/desktop/src/app/settings/local-model-download-progress.tsx index 7d3e8d8418..777d9f9622 100644 --- a/apps/desktop/src/app/settings/local-model-download-progress.tsx +++ b/apps/desktop/src/app/settings/local-model-download-progress.tsx @@ -1,11 +1,16 @@ -import { useState } from 'react' +import { type ReactElement, useState } from 'react' import { Button } from '@/components/ui/button' import { pauseLocalDownload, resumeLocalDownload } from '@/hermes' import { useI18n } from '@/i18n' import { Loader2, Pause, Play } from '@/lib/icons' import { cn } from '@/lib/utils' -import { watchLocalRuntimeJobs } from '@/store/local-runtime-jobs' +import { + isCurrentLocalModelsOwner, + type LocalModelsOwner, + localModelsRequestScope, + watchLocalRuntimeJobs +} from '@/store/local-runtime-jobs' import { notifyError } from '@/store/notifications' import type { LocalRuntimeJob } from '@/types/hermes' @@ -91,7 +96,10 @@ export function LocalModelDownloadProgress({ job }: LocalModelDownloadProps) { ) } -export function LocalModelDownloadActions({ job }: LocalModelDownloadProps) { +export function LocalModelDownloadActions({ + job, + owner +}: LocalModelDownloadProps & { owner?: LocalModelsOwner }): ReactElement | null { const { t } = useI18n() const copy = t.settings.localModels const [busy, setBusy] = useState(false) @@ -105,14 +113,16 @@ export function LocalModelDownloadActions({ job }: LocalModelDownloadProps) { // below decides what the row shows; only a real transport failure // surfaces as an error. if (kind === 'pause') { - await pauseLocalDownload(job.job_id) + await pauseLocalDownload(job.job_id, owner ? localModelsRequestScope(owner) : undefined) } else { - await resumeLocalDownload(job.job_id) + await resumeLocalDownload(job.job_id, owner ? localModelsRequestScope(owner) : undefined) } - watchLocalRuntimeJobs() + watchLocalRuntimeJobs(owner) } catch (err) { - notifyError(err, copy.downloadFailed(job.target)) + if (!owner || isCurrentLocalModelsOwner(owner)) { + notifyError(err, copy.downloadFailed(job.target)) + } } finally { setBusy(false) } diff --git a/apps/desktop/src/app/settings/local-models-settings.test.tsx b/apps/desktop/src/app/settings/local-models-settings.test.tsx index bf84f1869a..1db6e8f480 100644 --- a/apps/desktop/src/app/settings/local-models-settings.test.tsx +++ b/apps/desktop/src/app/settings/local-models-settings.test.tsx @@ -1,9 +1,22 @@ +vi.mock('@/store/profile', async (): Promise => { + const { atom } = await import('nanostores') + + return { $activeGatewayProfile: atom('default') } +}) +vi.mock('@/store/session', async (): Promise => { + const { atom } = await import('nanostores') + + return { $connection: atom(null), $defaultReasoningEffort: atom('') } +}) + +import { QueryClientProvider } from '@tanstack/react-query' import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' import { MemoryRouter, useLocation } from 'react-router' import { afterEach, beforeEach, describe, expect, it, type Mock, vi } from 'vitest' import { I18nProvider } from '@/i18n' -import { $localRuntimeJobs, watchLocalRuntimeJobs } from '@/store/local-runtime-jobs' +import { queryClient } from '@/lib/query-client' +import { localModelsKey, localModelsOwner, watchLocalRuntimeJobs } from '@/store/local-runtime-jobs' import type { LocalCatalogModel, LocalHardware, LocalModelsStatus, LocalRuntimeJob } from '@/types/hermes' import { LocalModelsSettings } from './local-models-settings' @@ -105,11 +118,13 @@ const REFUSED_MODEL: LocalCatalogModel = { function renderPane() { return render( - - - - - + + + + + + + ) } @@ -124,7 +139,8 @@ async function renderFullPane(): Promise> { // straight to the full pane, the runtime section directly. await waitFor((): void => { expect( - Boolean(screen.queryByRole('button', { name: /let me choose/i })) || screen.queryAllByText(/this machine/i).length > 0 + Boolean(screen.queryByRole('button', { name: /let me choose/i })) || + screen.queryAllByText(/this machine/i).length > 0 ).toBe(true) }) @@ -137,18 +153,23 @@ async function renderFullPane(): Promise> { return result } -beforeEach(() => { +beforeEach((): void => { + queryClient.clear() + queryClient.setDefaultOptions({ queries: { ...queryClient.getDefaultOptions().queries, retry: false } }) mocked.getLocalModelsStatus.mockResolvedValue(BASE_STATUS) mocked.getLocalHardware.mockResolvedValue(BASE_HARDWARE) mocked.getLocalCatalog.mockResolvedValue({ models: [FITTING_MODEL, SPILLED_MODEL, REFUSED_MODEL] }) // The backend mock ECHOES the atom: the watcher's immediate poll reads // seeded jobs instead of wiping them with a default {jobs:[]}. - mocked.getLocalModelsJobs.mockImplementation(async () => ({ jobs: [...$localRuntimeJobs.get()] })) - $localRuntimeJobs.set([]) + mocked.getLocalModelsJobs.mockImplementation(async () => ({ + jobs: [...(queryClient.getQueryData(localModelsKey(localModelsOwner(), 'jobs')) ?? [])] + })) + queryClient.setQueryData(localModelsKey(localModelsOwner(), 'jobs'), []) }) afterEach(async () => { cleanup() + queryClient.clear() mocked.getLocalModelsJobs.mockResolvedValue({ jobs: [] }) await act(async () => { watchLocalRuntimeJobs() @@ -318,7 +339,7 @@ describe('LocalModelsSettings', () => { }) // A running job already in the app-level store — as after closing and // reopening the pane mid-download. - $localRuntimeJobs.set([ + queryClient.setQueryData(localModelsKey(localModelsOwner(), 'jobs'), [ { job_id: 'j9', kind: 'model-download', @@ -351,7 +372,7 @@ describe('LocalModelsSettings', () => { runtime_installed: true, runtime_backend: 'cuda' }) - $localRuntimeJobs.set([ + queryClient.setQueryData(localModelsKey(localModelsOwner(), 'jobs'), [ { job_id: 'j2', kind: 'model-download', @@ -397,7 +418,7 @@ describe('quickstart', () => { }) it('pins the quickstart progress view while the job runs', async () => { - $localRuntimeJobs.set([ + queryClient.setQueryData(localModelsKey(localModelsOwner(), 'jobs'), [ { job_id: 'q1', kind: 'quickstart', @@ -470,12 +491,15 @@ describe('BrowseSection', () => { }) fireEvent.click(screen.getByRole('button', { name: /download ·/i })) await waitFor((): void => { - expect(mocked.downloadLocalModel).toHaveBeenCalledWith(SPILLED_MODEL.id) + expect(mocked.downloadLocalModel).toHaveBeenCalledWith(SPILLED_MODEL.id, { + connectionId: null, + profile: 'default' + }) }) mocked.activateLocalModel.mockResolvedValue({ job_id: 'explicit-spill' }) fireEvent.click(await screen.findByRole('button', { name: /^use$/i })) await waitFor((): void => { - expect(mocked.activateLocalModel).toHaveBeenCalledWith(stagedId) + expect(mocked.activateLocalModel).toHaveBeenCalledWith(stagedId, { connectionId: null, profile: 'default' }) }) expect(mocked.quickstartLocalModels).not.toHaveBeenCalled() }) @@ -495,11 +519,13 @@ describe('BrowseSection', () => { }) render( - - - - - + + + + + + + ) await act(async () => { await vi.runOnlyPendingTimersAsync() @@ -514,7 +540,7 @@ describe('BrowseSection', () => { await act(async () => { await vi.advanceTimersByTimeAsync(400) }) - expect(hermes.searchHFModels).toHaveBeenCalledWith('qwen') + expect(hermes.searchHFModels).toHaveBeenCalledWith('qwen', 20, { connectionId: null, profile: 'default' }) expect(screen.getByText('unsloth/Qwen3.8-27B-GGUF')).toBeTruthy() fireEvent.click(screen.getByRole('button', { name: /show files/i })) @@ -534,7 +560,11 @@ describe('BrowseSection', () => { await act(async () => { await vi.runOnlyPendingTimersAsync() }) - expect(hermes.downloadBrowsedModel).toHaveBeenCalledWith('unsloth/Qwen3.8-27B-GGUF', ['Qwen3.8-27B-Q4_K_M.gguf']) + expect(hermes.downloadBrowsedModel).toHaveBeenCalledWith( + 'unsloth/Qwen3.8-27B-GGUF', + ['Qwen3.8-27B-Q4_K_M.gguf'], + { connectionId: null, profile: 'default' } + ) } finally { vi.useRealTimers() } @@ -597,33 +627,43 @@ describe('quickstart completion navigation', () => { // A finished quickstart already in history when the pane mounts — // must NOT trigger navigation. - $localRuntimeJobs.set([doneJob]) + queryClient.setQueryData(localModelsKey(localModelsOwner(), 'jobs'), [doneJob]) render( - - - - - - + + + + + + + + ) await act(async () => {}) expect(routeProbe).not.toHaveBeenCalledWith('/') // A quickstart the pane SAW running that then completes -> navigate. const running: LocalRuntimeJob = { ...doneJob, job_id: 'live-run', phase: 'downloading', status: 'running' } - await act(async () => { - $localRuntimeJobs.set([doneJob, running]) + await act(async (): Promise => { + queryClient.setQueryData(localModelsKey(localModelsOwner(), 'jobs'), [doneJob, running]) + await new Promise((resolve): void => { + setTimeout(resolve, 0) + }) }) await act(async () => { - $localRuntimeJobs.set([doneJob, { ...running, phase: 'done', status: 'done' }]) + queryClient.setQueryData(localModelsKey(localModelsOwner(), 'jobs'), [ + doneJob, + { ...running, phase: 'done', status: 'done' } + ]) }) - expect(routeProbe).toHaveBeenCalledWith('/') + await waitFor((): void => expect(routeProbe).toHaveBeenCalledWith('/')) }) }) describe('pause / resume integration', () => { - beforeEach(() => { + beforeEach((): void => { + queryClient.clear() + queryClient.setDefaultOptions({ queries: { ...queryClient.getDefaultOptions().queries, retry: false } }) vi.mocked(hermes.pauseLocalDownload).mockResolvedValue({ ok: true, paused: true }) vi.mocked(hermes.resumeLocalDownload).mockResolvedValue({ ok: true, resumed: true }) }) @@ -634,7 +674,7 @@ describe('pause / resume integration', () => { runtime_installed: true, runtime_backend: 'cuda' }) - $localRuntimeJobs.set([ + queryClient.setQueryData(localModelsKey(localModelsOwner(), 'jobs'), [ { job_id: 'j1', kind: 'model-download', @@ -659,7 +699,7 @@ describe('pause / resume integration', () => { await waitFor(() => { expect(hermes.pauseLocalDownload).toHaveBeenCalledTimes(1) }) - expect(hermes.pauseLocalDownload).toHaveBeenCalledWith('j1') + expect(hermes.pauseLocalDownload).toHaveBeenCalledWith('j1', { connectionId: null, profile: 'default' }) expect(hermes.resumeLocalDownload).not.toHaveBeenCalled() }) @@ -669,7 +709,7 @@ describe('pause / resume integration', () => { runtime_installed: true, runtime_backend: 'cuda' }) - $localRuntimeJobs.set([ + queryClient.setQueryData(localModelsKey(localModelsOwner(), 'jobs'), [ { job_id: 'j1', kind: 'model-download', @@ -695,7 +735,7 @@ describe('pause / resume integration', () => { fireEvent.click(screen.getByRole('button', { name: /resume/i })) await waitFor(() => { - expect(hermes.resumeLocalDownload).toHaveBeenCalledWith('j1') + expect(hermes.resumeLocalDownload).toHaveBeenCalledWith('j1', { connectionId: null, profile: 'default' }) }) // The watcher re-kicked: an authoritative re-read happens after resume. @@ -705,7 +745,7 @@ describe('pause / resume integration', () => { }) it('a paused quickstart stays pinned in the hero with a Resume control', async () => { - $localRuntimeJobs.set([ + queryClient.setQueryData(localModelsKey(localModelsOwner(), 'jobs'), [ { job_id: 'q1', kind: 'quickstart', @@ -731,12 +771,12 @@ describe('pause / resume integration', () => { fireEvent.click(screen.getByRole('button', { name: /resume/i })) await waitFor(() => { - expect(hermes.resumeLocalDownload).toHaveBeenCalledWith('q1') + expect(hermes.resumeLocalDownload).toHaveBeenCalledWith('q1', { connectionId: null, profile: 'default' }) }) }) it('a running quickstart hero shows Pause during the download stage', async () => { - $localRuntimeJobs.set([ + queryClient.setQueryData(localModelsKey(localModelsOwner(), 'jobs'), [ { job_id: 'q1', kind: 'quickstart', @@ -765,7 +805,7 @@ describe('pause / resume integration', () => { runtime_installed: false, update_available: false }) - $localRuntimeJobs.set([ + queryClient.setQueryData(localModelsKey(localModelsOwner(), 'jobs'), [ { job_id: 'r1', kind: 'runtime-install', @@ -789,12 +829,12 @@ describe('pause / resume integration', () => { fireEvent.click(pause) await waitFor(() => { - expect(hermes.pauseLocalDownload).toHaveBeenCalledWith('r1') + expect(hermes.pauseLocalDownload).toHaveBeenCalledWith('r1', { connectionId: null, profile: 'default' }) }) }) it('quickstart hero suppresses the byte counter outside download phases', async () => { - $localRuntimeJobs.set([ + queryClient.setQueryData(localModelsKey(localModelsOwner(), 'jobs'), [ { job_id: 'q1', kind: 'quickstart', diff --git a/apps/desktop/src/app/settings/local-models-settings.tsx b/apps/desktop/src/app/settings/local-models-settings.tsx index bda53c933e..75d4e51554 100644 --- a/apps/desktop/src/app/settings/local-models-settings.tsx +++ b/apps/desktop/src/app/settings/local-models-settings.tsx @@ -1,5 +1,5 @@ -import { useStore } from '@nanostores/react' -import { useCallback, useEffect, useRef, useState } from 'react' +import { type QueryClient, useQuery, useQueryClient } from '@tanstack/react-query' +import { type ReactElement, useCallback, useEffect, useRef, useState } from 'react' import { useNavigate } from 'react-router' import { NEW_CHAT_ROUTE } from '@/app/routes' @@ -11,9 +11,6 @@ import { downloadBrowsedModel, downloadLocalModel, ejectLocalModel, - getLocalCatalog, - getLocalHardware, - getLocalModelsStatus, type HFFileGroup, type HFSearchHit, installLocalRuntime, @@ -42,13 +39,23 @@ import { } from '@/lib/icons' import { cn } from '@/lib/utils' import { - $localRuntimeJobs, + isCurrentLocalModelsOwner, + localModelsCatalogOptions, + localModelsHardwareOptions, + localModelsKey, + localModelsNotificationTitle, + type LocalModelsOwner, + localModelsRequestScope, + refreshLocalModels, runningDownloadFor, runningRuntimeInstall, + useLocalModelsOwner, + useLocalModelsStatus, + useLocalRuntimeJobs, watchLocalRuntimeJobs } from '@/store/local-runtime-jobs' import { notify, notifyError } from '@/store/notifications' -import type { LocalCatalogModel, LocalHardware, LocalModelsStatus, LocalRuntimeJob } from '@/types/hermes' +import type { LocalCatalogModel, LocalRuntimeJob } from '@/types/hermes' import { gbLabel, @@ -80,105 +87,57 @@ function isActiveStatus(status: LocalRuntimeJob['status']): boolean { return status === 'paused' || status === 'running' } -export function LocalModelsSettings() { +export function LocalModelsSettings(): ReactElement { + const owner: LocalModelsOwner = useLocalModelsOwner() + + return +} + +function ScopedLocalModelsSettings({ owner }: { owner: LocalModelsOwner }): ReactElement { const { t } = useI18n() const copy = t.settings.localModels - const [status, setStatus] = useState(null) - const [hardware, setHardware] = useState(null) - const [catalog, setCatalog] = useState(null) + const client: QueryClient = useQueryClient() + const { data: status } = useLocalModelsStatus(owner) + const { data: hardware } = useQuery(localModelsHardwareOptions(owner)) + const { data: catalog } = useQuery(localModelsCatalogOptions(owner)) const [deleting, setDeleting] = useState(null) - const [serverBusy, setServerBusy] = useState(false) + const [serverBusy, setServerBusy] = useState(false) // Quickstart escape hatch: true once the user asks for the full pane // (model list, HF browser) instead of the one-button setup card. - const [configure, setConfigure] = useState(false) - // Jobs live in the app-level store (they must survive this pane - // unmounting); the pane just renders the slice it cares about. - const jobs = useStore($localRuntimeJobs) + const [configure, setConfigure] = useState(false) - const refresh = useCallback(() => { - void getLocalModelsStatus() - .then(setStatus) - .catch(() => setStatus(null)) - void getLocalCatalog() - .then(data => setCatalog(data.models)) - .catch(() => setCatalog([])) - }, []) + const jobs: readonly LocalRuntimeJob[] = useLocalRuntimeJobs( + owner, + (value: readonly LocalRuntimeJob[]): readonly LocalRuntimeJob[] => value + ) - // Snappy first paint: status + catalog immediately; hardware (may shell out - // to nvidia-smi) backfills and pops in-place. The job watcher also kicks - // here so reopening the pane rediscovers work started before. - useEffect(() => { - refresh() - watchLocalRuntimeJobs() - void getLocalHardware() - .then(setHardware) - .catch(() => setHardware(null)) - }, [refresh]) + const refresh = useCallback((): void => refreshLocalModels(owner, client), [owner, client]) - // The pane is LIVE while visible: residency changes without user action - // (boot warm finishing, idle sweep unloading, another surface ejecting), - // and a stale snapshot here reads as a broken feature — 'VRAM full but - // the pane says Not in memory'. The status route is built cheap for - // polling; setTimeout chain, never overlapping. - useEffect(() => { - let cancelled = false - let timer: number | undefined - - const tick = async () => { - try { - const next = await getLocalModelsStatus() - - if (!cancelled) { - setStatus(next) - } - } catch { - // Backend briefly unreachable — keep the last snapshot. - } - - if (!cancelled) { - timer = window.setTimeout(() => void tick(), 4_000) - } - } - - timer = window.setTimeout(() => void tick(), 4_000) - - return () => { - cancelled = true - - if (timer !== undefined) { - window.clearTimeout(timer) - } - } - }, []) - - // A job finishing (download done, install done) changes what status/catalog - // should show — refresh whenever the running set shrinks. - const runningCount = jobs.filter(j => j.status === 'running').length - useEffect(() => { - refresh() - }, [refresh, runningCount]) - - async function handleInstallRuntime() { + async function handleInstallRuntime(): Promise { try { - await installLocalRuntime() - watchLocalRuntimeJobs() + await installLocalRuntime(undefined, localModelsRequestScope(owner)) + watchLocalRuntimeJobs(owner, client) } catch (err) { - notifyError(err, copy.installFailed) + if (isCurrentLocalModelsOwner(owner)) { + notifyError(err, copy.installFailed) + } } } - async function handleQuickstart() { + async function handleQuickstart(): Promise { try { - await quickstartLocalModels() - watchLocalRuntimeJobs() + await quickstartLocalModels(undefined, localModelsRequestScope(owner)) + watchLocalRuntimeJobs(owner, client) } catch (err) { - notifyError(err, copy.quickstartFailed) + if (isCurrentLocalModelsOwner(owner)) { + notifyError(err, copy.quickstartFailed) + } } } - async function handleDownload(model: LocalCatalogModel) { + async function handleDownload(model: LocalCatalogModel): Promise { try { - const res = await downloadLocalModel(model.id) + const res = await downloadLocalModel(model.id, localModelsRequestScope(owner)) if (res.already_downloaded || !res.job_id) { refresh() @@ -186,55 +145,63 @@ export function LocalModelsSettings() { return } - watchLocalRuntimeJobs() + watchLocalRuntimeJobs(owner, client) } catch (err) { - notifyError(err, copy.downloadFailed(model.display_name)) + if (isCurrentLocalModelsOwner(owner)) { + notifyError(err, copy.downloadFailed(model.display_name)) + } } } - async function handleActivate(target: null | string, displayName: string) { + async function handleActivate(target: null | string, displayName: string): Promise { if (!target) { return } try { - await activateLocalModel(target) - watchLocalRuntimeJobs() + await activateLocalModel(target, localModelsRequestScope(owner)) + watchLocalRuntimeJobs(owner, client) } catch (err) { - notifyError(err, copy.activateFailed(displayName)) + if (isCurrentLocalModelsOwner(owner)) { + notifyError(err, copy.activateFailed(displayName)) + } } } - async function handleEject(modelId: string) { + async function handleEject(modelId: string): Promise { try { - await ejectLocalModel(modelId) - notify({ durationMs: 3_000, kind: 'success', message: copy.ejected, title: copy.title }) + await ejectLocalModel(modelId, localModelsRequestScope(owner)) + notify({ durationMs: 3_000, kind: 'success', message: copy.ejected, title: localModelsNotificationTitle(owner) }) refresh() } catch (err) { - notifyError(err, copy.ejectFailed) + if (isCurrentLocalModelsOwner(owner)) { + notifyError(err, copy.ejectFailed) + } } } - async function handleServer(action: 'start' | 'stop') { + async function handleServer(action: 'start' | 'stop'): Promise { setServerBusy(true) try { - await setLocalServer(action) + await setLocalServer(action, localModelsRequestScope(owner)) notify({ durationMs: 3_500, kind: 'success', message: action === 'stop' ? copy.serverStopped : copy.serverStarted, - title: copy.title + title: localModelsNotificationTitle(owner) }) refresh() } catch (err) { - notifyError(err, action === 'stop' ? copy.serverStopFailed : copy.serverStartFailed) + if (isCurrentLocalModelsOwner(owner)) { + notifyError(err, action === 'stop' ? copy.serverStopFailed : copy.serverStartFailed) + } } finally { setServerBusy(false) } } - async function handleDelete(target: string, rowId: string) { + async function handleDelete(target: string, rowId: string): Promise { if (!window.confirm(copy.deleteConfirm(target))) { return } @@ -242,11 +209,18 @@ export function LocalModelsSettings() { setDeleting(rowId) try { - await deleteLocalModel(target) - notify({ durationMs: 2_500, kind: 'success', message: copy.deleted(target), title: copy.title }) + await deleteLocalModel(target, localModelsRequestScope(owner)) + notify({ + durationMs: 2_500, + kind: 'success', + message: copy.deleted(target), + title: localModelsNotificationTitle(owner) + }) refresh() } catch (err) { - notifyError(err, copy.deleteFailed) + if (isCurrentLocalModelsOwner(owner)) { + notifyError(err, copy.deleteFailed) + } } finally { setDeleting(null) } @@ -282,7 +256,7 @@ export function LocalModelsSettings() { } }, [jobs, navigate]) - if (!status || catalog === null) { + if (!status || !catalog) { return } @@ -362,7 +336,7 @@ export function LocalModelsSettings() { when controls exist (engine legs, server start report false); the paused state always offers Resume. */}
- +
{/* Stage rail: engine -> model -> finish. */} @@ -473,7 +447,7 @@ export function LocalModelsSettings() { /> ) : rJob ? ( } + action={} below={} description={rJob.detail || copy.installing} title={ @@ -515,7 +489,7 @@ export function LocalModelsSettings() { {rJob && status.runtime_installed && ( } + action={} below={} description={rJob.detail || copy.updating} title={ @@ -682,7 +656,7 @@ export function LocalModelsSettings() { ) : dJob ? ( - + ) : (