From 6207d689484387fdf24060975c5db50202d9d30a Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Fri, 10 Jul 2026 02:29:58 -0500 Subject: [PATCH] fix(desktop): prevent bootstrap stale commit repin on existing checkouts Old packaged Desktop apps re-entering bootstrap against an existing ~/.hermes/hermes-agent were still passing the baked-in --commit pin, which detached the managed checkout back to the app stamp (e.g. 0.15.1) after hermes update had already moved it forward. Skip the packaged commit pin when activeRoot already has git metadata; keep branch args and fresh-install commit pinning unchanged. Port of #59902 onto the post-ts-ify bootstrap-runner.ts. Co-authored-by: helix4u <4317663+helix4u@users.noreply.github.com> --- .../desktop/electron/bootstrap-runner.test.ts | 62 ++++++++++++++- apps/desktop/electron/bootstrap-runner.ts | 78 +++++++++++++++---- 2 files changed, 124 insertions(+), 16 deletions(-) diff --git a/apps/desktop/electron/bootstrap-runner.test.ts b/apps/desktop/electron/bootstrap-runner.test.ts index a61cec77f59..7ba50c85e74 100644 --- a/apps/desktop/electron/bootstrap-runner.test.ts +++ b/apps/desktop/electron/bootstrap-runner.test.ts @@ -4,7 +4,15 @@ import os from 'node:os' import path from 'node:path' import test from 'node:test' -import { cachedScriptPath, installedAgentInstallScript, resolveInstallScript, runBootstrap } from './bootstrap-runner' +import { + buildPinArgs, + buildPosixPinArgs, + cachedScriptPath, + hasExistingGitCheckout, + installedAgentInstallScript, + resolveInstallScript, + runBootstrap +} from './bootstrap-runner' const SCRIPT_NAME = process.platform === 'win32' ? 'install.ps1' : 'install.sh' @@ -54,6 +62,58 @@ test('installedAgentInstallScript resolves the installer in the agent checkout', } }) +test('existing checkout detection requires git metadata', () => { + const home = mkTmpHome() + + try { + const activeRoot = path.join(home, 'hermes-agent') + assert.equal(hasExistingGitCheckout(activeRoot), false) + + fs.mkdirSync(path.join(activeRoot, '.git'), { recursive: true }) + assert.equal(hasExistingGitCheckout(activeRoot), true) + } finally { + fs.rmSync(home, { recursive: true, force: true }) + } +}) + +test('fresh bootstrap args include the packaged commit pin', () => { + const installStamp = { commit: 'a'.repeat(40), branch: 'main' } + + assert.deepEqual(buildPinArgs(installStamp), ['-Commit', installStamp.commit, '-Branch', 'main']) + assert.deepEqual( + buildPosixPinArgs({ + installStamp, + activeRoot: '/tmp/hermes-agent', + hermesHome: '/tmp/hermes' + }), + [ + '--dir', + '/tmp/hermes-agent', + '--hermes-home', + '/tmp/hermes', + '--branch', + 'main', + '--commit', + installStamp.commit + ] + ) +}) + +test('existing-checkout bootstrap args keep branch but skip the packaged commit pin', () => { + const installStamp = { commit: 'a'.repeat(40), branch: 'main' } + + assert.deepEqual(buildPinArgs(installStamp, { pinCommit: false }), ['-Branch', 'main']) + assert.deepEqual( + buildPosixPinArgs({ + installStamp, + activeRoot: '/tmp/hermes-agent', + hermesHome: '/tmp/hermes', + pinCommit: false + }), + ['--dir', '/tmp/hermes-agent', '--hermes-home', '/tmp/hermes', '--branch', 'main'] + ) +}) + test('resolveInstallScript prefers a cached script without touching the network', async () => { const home = mkTmpHome() diff --git a/apps/desktop/electron/bootstrap-runner.ts b/apps/desktop/electron/bootstrap-runner.ts index 21a8cb4cd5d..41ddd597a37 100644 --- a/apps/desktop/electron/bootstrap-runner.ts +++ b/apps/desktop/electron/bootstrap-runner.ts @@ -107,6 +107,18 @@ function installedAgentInstallScript(hermesHome) { } } +function hasExistingGitCheckout(activeRoot) { + if (!activeRoot) { + return false + } + + try { + return fs.existsSync(path.join(activeRoot, '.git')) + } catch { + return false + } +} + function cachedScriptPath(hermesHome, commit) { return path.join(bootstrapCacheDir(hermesHome), `install-${commit}.${process.platform === 'win32' ? 'ps1' : 'sh'}`) } @@ -529,13 +541,14 @@ function spawnBash(scriptPath, args, { emit, stageName, abortSignal, hermesHome // Manifest + stage dispatch // --------------------------------------------------------------------------- -// Build the install.ps1 pin args (-Commit / -Branch) from the install-stamp -// so the repository stage clones the exact SHA the .exe was tested with -// instead of falling back to install.ps1's default ($Branch = "main"). -function buildPinArgs(installStamp) { +// Build the installer branch/pin args from the install stamp. The commit pin +// is fresh-install only: once a managed checkout already exists, bootstrap is +// a repair/update path and must not let an old packaged app detach the checkout +// back to the commit baked into that app. +function buildPinArgs(installStamp, { pinCommit = true } = {}) { const args = [] - if (installStamp && installStamp.commit) { + if (pinCommit && installStamp && installStamp.commit) { args.push('-Commit', installStamp.commit) } @@ -546,26 +559,34 @@ function buildPinArgs(installStamp) { return args } -function buildPosixPinArgs({ installStamp, activeRoot, hermesHome }) { +function buildPosixPinArgs({ installStamp, activeRoot, hermesHome, pinCommit = true }) { const args = ['--dir', activeRoot, '--hermes-home', hermesHome] if (installStamp && installStamp.branch) { args.push('--branch', installStamp.branch) } - if (installStamp && installStamp.commit) { + if (pinCommit && installStamp && installStamp.commit) { args.push('--commit', installStamp.commit) } return args } -async function fetchManifest({ scriptPath, installerKind, emit, hermesHome, activeRoot, installStamp }) { +async function fetchManifest({ + scriptPath, + installerKind, + emit, + hermesHome, + activeRoot, + installStamp, + pinCommit +}) { const isPosix = installerKind === 'posix' const args = isPosix - ? ['--manifest', ...buildPosixPinArgs({ installStamp, activeRoot, hermesHome })] - : ['-Manifest', ...buildPinArgs(installStamp)] + ? ['--manifest', ...buildPosixPinArgs({ installStamp, activeRoot, hermesHome, pinCommit })] + : ['-Manifest', ...buildPinArgs(installStamp, { pinCommit })] const result = await (isPosix ? spawnBash : spawnPowerShell)(scriptPath, args, { emit, @@ -622,7 +643,17 @@ function parseStageResult(stdout) { return null } -async function runStage({ scriptPath, installerKind, stage, emit, hermesHome, activeRoot, abortSignal, installStamp }) { +async function runStage({ + scriptPath, + installerKind, + stage, + emit, + hermesHome, + activeRoot, + abortSignal, + installStamp, + pinCommit +}) { const startedAt = Date.now() emit({ type: 'stage', name: stage.name, state: 'running' }) @@ -634,9 +665,9 @@ async function runStage({ scriptPath, installerKind, stage, emit, hermesHome, ac stage.name, '--non-interactive', '--json', - ...buildPosixPinArgs({ installStamp, activeRoot, hermesHome }) + ...buildPosixPinArgs({ installStamp, activeRoot, hermesHome, pinCommit }) ] - : ['-Stage', stage.name, '-NonInteractive', '-Json', ...buildPinArgs(installStamp)] + : ['-Stage', stage.name, '-NonInteractive', '-Json', ...buildPinArgs(installStamp, { pinCommit })] const result = await (isPosix ? spawnBash : spawnPowerShell)(scriptPath, args, { emit, @@ -774,6 +805,18 @@ async function runBootstrap(opts) { }) try { + const existingCheckout = hasExistingGitCheckout(activeRoot) + const pinCommit = !existingCheckout + + if (existingCheckout && installStamp && installStamp.commit) { + emit({ + type: 'log', + line: + `[bootstrap] existing checkout detected at ${activeRoot}; ` + + `not pinning to packaged install stamp ${installStamp.commit.slice(0, 12)}` + }) + } + // 1. Resolve the platform installer. const scriptInfo = await resolveInstallScript({ installStamp, sourceRepoRoot, hermesHome, emit }) const installerKind = scriptInfo.kind || 'powershell' @@ -785,7 +828,8 @@ async function runBootstrap(opts) { emit, hermesHome, activeRoot, - installStamp + installStamp, + pinCommit }) emit({ @@ -813,7 +857,8 @@ async function runBootstrap(opts) { hermesHome, activeRoot, abortSignal, - installStamp + installStamp, + pinCommit }) if (ev.state === 'failed') { @@ -847,7 +892,10 @@ async function runBootstrap(opts) { } export { + buildPinArgs, + buildPosixPinArgs, cachedScriptPath, + hasExistingGitCheckout, installedAgentInstallScript, // Exposed for testability parseStageResult,