diff --git a/apps/desktop/electron/update-marker.cjs b/apps/desktop/electron/update-marker.cjs index 5c4a14a1690..da3df6a8d4a 100644 --- a/apps/desktop/electron/update-marker.cjs +++ b/apps/desktop/electron/update-marker.cjs @@ -1,127 +1,127 @@ -/** - * In-app update mutual-exclusion marker (#50238). - * - * The Tauri updater writes HERMES_HOME/.hermes-update-in-progress for the whole - * duration of an `--update` run (see apps/bootstrap-installer/src-tauri/src/ - * update.rs `UpdateMarkerGuard`). The marker body is two lines: the updater's - * pid and the unix-seconds it started. - * - * Why: if the user relaunches the desktop mid-update — the window vanished with - * no progress and looks crashed — a fresh instance must NOT spawn its own local - * backend. That backend re-locks the venv shim, the updater's straggler cleanup - * (`force_kill_other_hermes`, taskkill /IM hermes.exe) kills it, the launch - * fails with the 45s "backend didn't come up" timeout, and the user relaunches - * into the same trap — an infinite respawn/kill loop. The desktop gates local - * backend startup on this marker and parks until the update finishes. - * - * This module holds the PURE, side-effect-light logic (path, pid liveness, - * parse + staleness) so it is unit-testable without booting Electron. The - * polling/boot-progress wrapper lives in main.cjs where the boot-progress and - * log sinks are. - */ - -const fs = require('fs') -const path = require('path') - -// Even with a live-looking PID, never treat a marker older than this as a live -// update. A full update (git pull + pip + desktop rebuild) is minutes, not tens -// of minutes; past this the marker is almost certainly stale (e.g. the OS -// recycled the pid onto an unrelated process), so the gate self-heals. -const UPDATE_MARKER_MAX_AGE_MS = 20 * 60 * 1000 - -function markerPath(hermesHome) { - return path.join(hermesHome, '.hermes-update-in-progress') -} - -// True only if a host process with this pid is currently alive. Signal 0 does -// not deliver a signal — it just probes existence/permission. ESRCH => dead; -// EPERM => alive but owned by another user (still "alive" for our purposes). -// Injectable `kill` keeps it unit-testable. -function isPidAlive(pid, kill = process.kill.bind(process)) { - if (!Number.isInteger(pid) || pid <= 0) return false - try { - kill(pid, 0) - return true - } catch (err) { - return Boolean(err && err.code === 'EPERM') - } -} - -/** - * Read + interpret the marker. - * - * Returns `{ pid, ageMs }` only when an update is GENUINELY still running - * (parseable pid that is alive, within the age ceiling). Returns `null` for - * every "no live update" case — absent, unreadable, malformed, dead pid, or - * past the ceiling — and, when a stale marker file exists, deletes it so it - * cannot strand future launches. - * - * Pure-ish: file I/O against the given path, plus an injectable pid probe and - * clock for tests. - */ -function readLiveUpdateMarker(hermesHome, { kill, now = Date.now, maxAgeMs = UPDATE_MARKER_MAX_AGE_MS } = {}) { - const file = markerPath(hermesHome) - let raw - try { - raw = fs.readFileSync(file, 'utf8') - } catch { - return null // absent or unreadable => no live update - } - - const [pidLine, startedLine] = String(raw).split('\n') - const pid = Number.parseInt((pidLine || '').trim(), 10) - const startedAt = Number.parseInt((startedLine || '').trim(), 10) - const ageMs = Number.isFinite(startedAt) ? now() - startedAt * 1000 : Infinity - const alive = Number.isInteger(pid) && isPidAlive(pid, kill) - - if (!alive || ageMs > maxAgeMs) { - try { - fs.unlinkSync(file) - } catch { - void 0 - } - return null - } - return { pid, ageMs } -} - -/** - * Write the update-in-progress marker *from the desktop* before handing off - * to the detached updater. - * - * The Tauri-based hermes-setup.exe takes several seconds to initialise its - * window and reach the Rust `run_update` entry point where it writes the - * marker itself. During that gap the desktop's `app.quit()` teardown kills - * the backend child, the renderer's WebSocket drops, and the renderer - * immediately calls `ensureBackend()` → `waitForUpdateToFinish()`. Because - * the updater hasn't written the marker yet, the gate sees no live update - * and spawns a *new* backend — which re-locks `.pyd` files in the venv. - * When the updater finally reaches the venv-rebuild stage it finds those - * files locked and the update bricks. - * - * Fix: the desktop writes the marker itself, using the spawned updater's - * PID, immediately after `spawn()`. The updater's `UpdateMarkerGuard` will - * later overwrite it with its own PID — that's fine, the marker body is - * the same format and `readLiveUpdateMarker` only cares that *some* live - * pid owns it. When the updater finishes it deletes the marker as before. - * If the updater never starts (spawn failure) the marker still contains a - * real PID, so `readLiveUpdateMarker` will self-heal once that PID exits. - */ -function writeUpdateMarker(hermesHome, pid, { now = Date.now } = {}) { - const file = markerPath(hermesHome) - const startedAt = Math.floor(now() / 1000) - try { - fs.writeFileSync(file, `${pid}\n${startedAt}\n`, 'utf8') - } catch { - // Best-effort: if we can't write the marker, proceed anyway. The - // updater will write its own when it reaches run_update. - } -} - -module.exports = { - UPDATE_MARKER_MAX_AGE_MS, - markerPath, - isPidAlive, - readLiveUpdateMarker, - writeUpdateMarker -} +/** + * In-app update mutual-exclusion marker (#50238). + * + * The Tauri updater writes HERMES_HOME/.hermes-update-in-progress for the whole + * duration of an `--update` run (see apps/bootstrap-installer/src-tauri/src/ + * update.rs `UpdateMarkerGuard`). The marker body is two lines: the updater's + * pid and the unix-seconds it started. + * + * Why: if the user relaunches the desktop mid-update — the window vanished with + * no progress and looks crashed — a fresh instance must NOT spawn its own local + * backend. That backend re-locks the venv shim, the updater's straggler cleanup + * (`force_kill_other_hermes`, taskkill /IM hermes.exe) kills it, the launch + * fails with the 45s "backend didn't come up" timeout, and the user relaunches + * into the same trap — an infinite respawn/kill loop. The desktop gates local + * backend startup on this marker and parks until the update finishes. + * + * This module holds the PURE, side-effect-light logic (path, pid liveness, + * parse + staleness) so it is unit-testable without booting Electron. The + * polling/boot-progress wrapper lives in main.cjs where the boot-progress and + * log sinks are. + */ + +const fs = require('fs') +const path = require('path') + +// Even with a live-looking PID, never treat a marker older than this as a live +// update. A full update (git pull + pip + desktop rebuild) is minutes, not tens +// of minutes; past this the marker is almost certainly stale (e.g. the OS +// recycled the pid onto an unrelated process), so the gate self-heals. +const UPDATE_MARKER_MAX_AGE_MS = 20 * 60 * 1000 + +function markerPath(hermesHome) { + return path.join(hermesHome, '.hermes-update-in-progress') +} + +// True only if a host process with this pid is currently alive. Signal 0 does +// not deliver a signal — it just probes existence/permission. ESRCH => dead; +// EPERM => alive but owned by another user (still "alive" for our purposes). +// Injectable `kill` keeps it unit-testable. +function isPidAlive(pid, kill = process.kill.bind(process)) { + if (!Number.isInteger(pid) || pid <= 0) return false + try { + kill(pid, 0) + return true + } catch (err) { + return Boolean(err && err.code === 'EPERM') + } +} + +/** + * Read + interpret the marker. + * + * Returns `{ pid, ageMs }` only when an update is GENUINELY still running + * (parseable pid that is alive, within the age ceiling). Returns `null` for + * every "no live update" case — absent, unreadable, malformed, dead pid, or + * past the ceiling — and, when a stale marker file exists, deletes it so it + * cannot strand future launches. + * + * Pure-ish: file I/O against the given path, plus an injectable pid probe and + * clock for tests. + */ +function readLiveUpdateMarker(hermesHome, { kill, now = Date.now, maxAgeMs = UPDATE_MARKER_MAX_AGE_MS } = {}) { + const file = markerPath(hermesHome) + let raw + try { + raw = fs.readFileSync(file, 'utf8') + } catch { + return null // absent or unreadable => no live update + } + + const [pidLine, startedLine] = String(raw).split('\n') + const pid = Number.parseInt((pidLine || '').trim(), 10) + const startedAt = Number.parseInt((startedLine || '').trim(), 10) + const ageMs = Number.isFinite(startedAt) ? now() - startedAt * 1000 : Infinity + const alive = Number.isInteger(pid) && isPidAlive(pid, kill) + + if (!alive || ageMs > maxAgeMs) { + try { + fs.unlinkSync(file) + } catch { + void 0 + } + return null + } + return { pid, ageMs } +} + +/** + * Write the update-in-progress marker *from the desktop* before handing off + * to the detached updater. + * + * The Tauri-based hermes-setup.exe takes several seconds to initialise its + * window and reach the Rust `run_update` entry point where it writes the + * marker itself. During that gap the desktop's `app.quit()` teardown kills + * the backend child, the renderer's WebSocket drops, and the renderer + * immediately calls `ensureBackend()` → `waitForUpdateToFinish()`. Because + * the updater hasn't written the marker yet, the gate sees no live update + * and spawns a *new* backend — which re-locks `.pyd` files in the venv. + * When the updater finally reaches the venv-rebuild stage it finds those + * files locked and the update bricks. + * + * Fix: the desktop writes the marker itself, using the spawned updater's + * PID, immediately after `spawn()`. The updater's `UpdateMarkerGuard` will + * later overwrite it with its own PID — that's fine, the marker body is + * the same format and `readLiveUpdateMarker` only cares that *some* live + * pid owns it. When the updater finishes it deletes the marker as before. + * If the updater never starts (spawn failure) the marker still contains a + * real PID, so `readLiveUpdateMarker` will self-heal once that PID exits. + */ +function writeUpdateMarker(hermesHome, pid, { now = Date.now } = {}) { + const file = markerPath(hermesHome) + const startedAt = Math.floor(now() / 1000) + try { + fs.writeFileSync(file, `${pid}\n${startedAt}\n`, 'utf8') + } catch { + // Best-effort: if we can't write the marker, proceed anyway. The + // updater will write its own when it reaches run_update. + } +} + +module.exports = { + UPDATE_MARKER_MAX_AGE_MS, + markerPath, + isPidAlive, + readLiveUpdateMarker, + writeUpdateMarker +} diff --git a/apps/desktop/electron/update-marker.test.cjs b/apps/desktop/electron/update-marker.test.cjs index 61d70959d4c..d84483714c6 100644 --- a/apps/desktop/electron/update-marker.test.cjs +++ b/apps/desktop/electron/update-marker.test.cjs @@ -1,118 +1,118 @@ -/** - * Tests for electron/update-marker.cjs — the in-app update mutual-exclusion - * marker that prevents a desktop relaunched mid-update from spawning a backend - * the updater then kills in a loop (#50238). - * - * Run with: node --test electron/update-marker.test.cjs - * (Wired into npm test:desktop:platforms in package.json.) - * - * Why this matters: the gate must (a) report a live update only when the - * updater pid is alive AND the marker is fresh, (b) treat absent/malformed/ - * dead-pid/expired markers as "no live update" so a crashed updater can't - * strand future launches, and (c) self-heal by deleting a stale marker file. - */ - -const test = require('node:test') -const assert = require('node:assert/strict') -const fs = require('fs') -const os = require('os') -const path = require('path') - -const { markerPath, isPidAlive, readLiveUpdateMarker, writeUpdateMarker, UPDATE_MARKER_MAX_AGE_MS } = require('./update-marker.cjs') - -function tmpHome(tag) { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), `hermes-marker-${tag}-`)) - return dir -} - -function writeMarker(home, pid, startedAtSec) { - fs.writeFileSync(markerPath(home), `${pid}\n${startedAtSec}`) -} - -const ALIVE = () => true // injected kill that "succeeds" => pid alive -const DEAD = () => { - const err = new Error('no such process') - err.code = 'ESRCH' - throw err -} - -test('absent marker => no live update', () => { - const home = tmpHome('absent') - assert.equal(readLiveUpdateMarker(home, { kill: ALIVE }), null) -}) - -test('live pid within age ceiling => live update reported', () => { - const home = tmpHome('live') - const now = 1_000_000_000_000 - writeMarker(home, 4242, Math.floor(now / 1000) - 5) // 5s old - const res = readLiveUpdateMarker(home, { kill: ALIVE, now: () => now }) - assert.ok(res, 'a fresh, alive marker is a live update') - assert.equal(res.pid, 4242) - assert.ok(res.ageMs >= 0 && res.ageMs < 10_000) - assert.ok(fs.existsSync(markerPath(home)), 'a live marker is NOT deleted') -}) - -test('dead pid => no live update and marker is pruned', () => { - const home = tmpHome('dead') - writeMarker(home, 999999, Math.floor(Date.now() / 1000)) - assert.equal(readLiveUpdateMarker(home, { kill: DEAD }), null) - assert.ok(!fs.existsSync(markerPath(home)), 'a dead-pid marker self-heals (deleted)') -}) - -test('expired marker (past age ceiling) => no live update and pruned', () => { - const home = tmpHome('expired') - const now = 1_000_000_000_000 - writeMarker(home, 4242, Math.floor((now - UPDATE_MARKER_MAX_AGE_MS - 60_000) / 1000)) - // Even though the pid is "alive", the marker is too old to trust. - assert.equal(readLiveUpdateMarker(home, { kill: ALIVE, now: () => now }), null) - assert.ok(!fs.existsSync(markerPath(home)), 'an expired marker self-heals (deleted)') -}) - -test('malformed marker => no live update and pruned', () => { - const home = tmpHome('malformed') - fs.writeFileSync(markerPath(home), 'not-a-pid\nnonsense') - assert.equal(readLiveUpdateMarker(home, { kill: ALIVE }), null) - assert.ok(!fs.existsSync(markerPath(home))) -}) - -test('isPidAlive: own pid is alive, impossible pid is dead', () => { - assert.equal(isPidAlive(process.pid), true) - assert.equal(isPidAlive(-1), false) - assert.equal(isPidAlive(0), false) - assert.equal(isPidAlive(NaN), false) -}) - -test('isPidAlive: EPERM counts as alive (process owned by another user)', () => { - const eperm = () => { - const err = new Error('operation not permitted') - err.code = 'EPERM' - throw err - } - assert.equal(isPidAlive(4242, eperm), true) -}) - -test('writeUpdateMarker writes a marker that readLiveUpdateMarker accepts', () => { - const home = tmpHome('write') - const now = 1_000_000_000_000 - writeUpdateMarker(home, 4242, { now: () => now }) - // The marker should be readable and report the same pid. - const res = readLiveUpdateMarker(home, { kill: ALIVE, now: () => now }) - assert.ok(res, 'marker written by writeUpdateMarker should be detected as live') - assert.equal(res.pid, 4242) - assert.ok(fs.existsSync(markerPath(home)), 'marker file should exist after write') -}) - -test('writeUpdateMarker is best-effort (no throw on bad path)', () => { - // A non-existent directory should not throw. - const badHome = path.join(os.tmpdir(), 'hermes-marker-nonexistent-' + Date.now()) - assert.doesNotThrow(() => writeUpdateMarker(badHome, 4242)) -}) - -test('writeUpdateMarker + dead pid => self-heals on read', () => { - const home = tmpHome('write-dead') - writeUpdateMarker(home, 999999, { now: () => Date.now() }) - // PID 999999 is almost certainly not alive. - const res = readLiveUpdateMarker(home, { kill: DEAD }) - assert.equal(res, null, 'a dead-pid marker from writeUpdateMarker self-heals') - assert.ok(!fs.existsSync(markerPath(home)), 'marker file is pruned') -}) +/** + * Tests for electron/update-marker.cjs — the in-app update mutual-exclusion + * marker that prevents a desktop relaunched mid-update from spawning a backend + * the updater then kills in a loop (#50238). + * + * Run with: node --test electron/update-marker.test.cjs + * (Wired into npm test:desktop:platforms in package.json.) + * + * Why this matters: the gate must (a) report a live update only when the + * updater pid is alive AND the marker is fresh, (b) treat absent/malformed/ + * dead-pid/expired markers as "no live update" so a crashed updater can't + * strand future launches, and (c) self-heal by deleting a stale marker file. + */ + +const test = require('node:test') +const assert = require('node:assert/strict') +const fs = require('fs') +const os = require('os') +const path = require('path') + +const { markerPath, isPidAlive, readLiveUpdateMarker, writeUpdateMarker, UPDATE_MARKER_MAX_AGE_MS } = require('./update-marker.cjs') + +function tmpHome(tag) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), `hermes-marker-${tag}-`)) + return dir +} + +function writeMarker(home, pid, startedAtSec) { + fs.writeFileSync(markerPath(home), `${pid}\n${startedAtSec}`) +} + +const ALIVE = () => true // injected kill that "succeeds" => pid alive +const DEAD = () => { + const err = new Error('no such process') + err.code = 'ESRCH' + throw err +} + +test('absent marker => no live update', () => { + const home = tmpHome('absent') + assert.equal(readLiveUpdateMarker(home, { kill: ALIVE }), null) +}) + +test('live pid within age ceiling => live update reported', () => { + const home = tmpHome('live') + const now = 1_000_000_000_000 + writeMarker(home, 4242, Math.floor(now / 1000) - 5) // 5s old + const res = readLiveUpdateMarker(home, { kill: ALIVE, now: () => now }) + assert.ok(res, 'a fresh, alive marker is a live update') + assert.equal(res.pid, 4242) + assert.ok(res.ageMs >= 0 && res.ageMs < 10_000) + assert.ok(fs.existsSync(markerPath(home)), 'a live marker is NOT deleted') +}) + +test('dead pid => no live update and marker is pruned', () => { + const home = tmpHome('dead') + writeMarker(home, 999999, Math.floor(Date.now() / 1000)) + assert.equal(readLiveUpdateMarker(home, { kill: DEAD }), null) + assert.ok(!fs.existsSync(markerPath(home)), 'a dead-pid marker self-heals (deleted)') +}) + +test('expired marker (past age ceiling) => no live update and pruned', () => { + const home = tmpHome('expired') + const now = 1_000_000_000_000 + writeMarker(home, 4242, Math.floor((now - UPDATE_MARKER_MAX_AGE_MS - 60_000) / 1000)) + // Even though the pid is "alive", the marker is too old to trust. + assert.equal(readLiveUpdateMarker(home, { kill: ALIVE, now: () => now }), null) + assert.ok(!fs.existsSync(markerPath(home)), 'an expired marker self-heals (deleted)') +}) + +test('malformed marker => no live update and pruned', () => { + const home = tmpHome('malformed') + fs.writeFileSync(markerPath(home), 'not-a-pid\nnonsense') + assert.equal(readLiveUpdateMarker(home, { kill: ALIVE }), null) + assert.ok(!fs.existsSync(markerPath(home))) +}) + +test('isPidAlive: own pid is alive, impossible pid is dead', () => { + assert.equal(isPidAlive(process.pid), true) + assert.equal(isPidAlive(-1), false) + assert.equal(isPidAlive(0), false) + assert.equal(isPidAlive(NaN), false) +}) + +test('isPidAlive: EPERM counts as alive (process owned by another user)', () => { + const eperm = () => { + const err = new Error('operation not permitted') + err.code = 'EPERM' + throw err + } + assert.equal(isPidAlive(4242, eperm), true) +}) + +test('writeUpdateMarker writes a marker that readLiveUpdateMarker accepts', () => { + const home = tmpHome('write') + const now = 1_000_000_000_000 + writeUpdateMarker(home, 4242, { now: () => now }) + // The marker should be readable and report the same pid. + const res = readLiveUpdateMarker(home, { kill: ALIVE, now: () => now }) + assert.ok(res, 'marker written by writeUpdateMarker should be detected as live') + assert.equal(res.pid, 4242) + assert.ok(fs.existsSync(markerPath(home)), 'marker file should exist after write') +}) + +test('writeUpdateMarker is best-effort (no throw on bad path)', () => { + // A non-existent directory should not throw. + const badHome = path.join(os.tmpdir(), 'hermes-marker-nonexistent-' + Date.now()) + assert.doesNotThrow(() => writeUpdateMarker(badHome, 4242)) +}) + +test('writeUpdateMarker + dead pid => self-heals on read', () => { + const home = tmpHome('write-dead') + writeUpdateMarker(home, 999999, { now: () => Date.now() }) + // PID 999999 is almost certainly not alive. + const res = readLiveUpdateMarker(home, { kill: DEAD }) + assert.equal(res, null, 'a dead-pid marker from writeUpdateMarker self-heals') + assert.ok(!fs.existsSync(markerPath(home)), 'marker file is pruned') +})