fix(updater): preserve holder age across handoffs

apps/bootstrap-installer/src-tauri/src/update.rs

@@ -133,17 +133,13 @@ struct MarkerOwner {
     age_secs: u64,
 }
 
-/// Read the marker and report a live *foreign* owner, if any. `None` for every
-/// "no live update" case — absent, unreadable, malformed, dead pid, past the
-/// ceiling, or a marker whose pid is **this** process — matching
-/// `readLiveUpdateMarker` in the Electron gate. Never panics.
+/// Read the marker and report a live owner, if any. `None` for every "no live
+/// update" case — absent, unreadable, malformed, dead pid, or past the ceiling
+/// — matching `readLiveUpdateMarker` in the Electron gate. Never panics.
 ///
-/// Self-PID is treated as non-ownership on purpose (#74761): since #50238 the
-/// desktop pre-writes this marker with the spawned updater's pid before the
-/// updater reaches `acquire`. Without the exclusion, `acquire` sees a live
-/// owner that is itself and aborts ("Another Hermes update is already
-/// running"), then the desktop relaunches and retries forever. A foreign live
-/// pid (e.g. a dashboard-spawned `hermes update`) still blocks.
+/// Self-PID is returned so `acquire` can adopt the desktop's pre-written claim
+/// without refreshing its acquisition time (#74761). A foreign live pid (e.g.
+/// a dashboard-spawned `hermes update`) still blocks.
 fn live_marker_owner(path: &Path) -> Option<MarkerOwner> {
     let raw = std::fs::read_to_string(path).ok()?;
     let mut lines = raw.lines();
@@ -157,11 +153,6 @@ fn live_marker_owner(path: &Path) -> Option<MarkerOwner> {
     if age_secs > UPDATE_MARKER_MAX_AGE_SECS || !pid_is_alive(pid) {
         return None;
     }
-    // Desktop `writeUpdateMarker(hermesHome, child.pid)` races ahead of us;
-    // adopt that pre-claim rather than refusing our own marker.
-    if pid == std::process::id() {
-        return None;
-    }
     Some(MarkerOwner { pid, age_secs })
 }
 
@@ -207,10 +198,16 @@ impl UpdateMarkerGuard {
     /// behavior), so we log and carry on with a guard that still attempts
     /// cleanup of whatever may exist at the path.
     fn acquire(path: PathBuf) -> Result<Self, MarkerOwner> {
+        let pid = std::process::id();
         if let Some(owner) = live_marker_owner(&path) {
+            if owner.pid == pid {
+                // The desktop races ahead and pre-writes our pid. Adopt that
+                // claim verbatim: rewriting started_at here lets retries reset
+                // a wedged updater's age before the stale ceiling can clear it.
+                return Ok(Self { path, owned: true });
+            }
             return Err(owner);
         }
-        let pid = std::process::id();
         let started_at = std::time::SystemTime::now()
             .duration_since(std::time::UNIX_EPOCH)
             .map(|d| d.as_secs())
@@ -1321,8 +1318,8 @@ mod tests {
 
     /// Spawn a short-lived sibling process whose pid stands in for a foreign
     /// updater. Same-process double-acquire no longer models contention: since
-    /// #74761 `live_marker_owner` treats our own pid as adoptable (desktop
-    /// pre-writes it), so a second acquire in *this* process would succeed.
+    /// #74761 `acquire` treats our own pid as adoptable (desktop pre-writes it),
+    /// so a second acquire in *this* process would succeed.
     fn spawn_foreign_holder() -> std::process::Child {
         #[cfg(windows)]
         {
@@ -1379,7 +1376,8 @@ mod tests {
     fn acquire_adopts_a_marker_prewritten_with_our_own_pid() {
         // #74761: desktop writeUpdateMarker(hermesHome, child.pid) races ahead
         // of UpdateMarkerGuard::acquire. The marker names US; refusing it made
-        // every in-app desktop update loop forever. Adopt and rewrite.
+        // every in-app desktop update loop forever. Adopt it without resetting
+        // the holder age, so a wedged updater still reaches the stale ceiling.
         let dir = unique_tmp_dir("marker-own-pid");
         std::fs::create_dir_all(&dir).unwrap();
         let marker = dir.join(".hermes-update-in-progress");
@@ -1402,7 +1400,12 @@ mod tests {
         assert_eq!(
             body.lines().next().unwrap().trim().parse::<u32>().unwrap(),
             std::process::id(),
-            "acquire rewrites the marker with our pid + fresh started_at"
+            "acquire keeps the adopted marker owner"
+        );
+        assert_eq!(
+            body.lines().nth(1).unwrap().trim().parse::<u64>().unwrap(),
+            started_at,
+            "adopting an own-pid marker must preserve its original holder age"
         );
         drop(guard);
         assert!(

apps/desktop/electron/update-marker.test.ts

@@ -115,6 +115,19 @@ test('writeUpdateMarker writes a marker that readLiveUpdateMarker accepts', () =
   assert.ok(fs.existsSync(markerPath(home)), 'marker file should exist after write')
 })
 
+test('writeUpdateMarker preserves a live holder age across pid hand-off', () => {
+  const home = tmpHome('write-handoff-age')
+  const now = 1_000_000_000_000
+  const startedAt = Math.floor(now / 1000) - 300
+
+  writeMarker(home, 1010, startedAt)
+  writeUpdateMarker(home, 2020, { kill: ALIVE, now: () => now })
+
+  const [pidLine, startedLine] = fs.readFileSync(markerPath(home), 'utf8').split('\n')
+  assert.equal(Number.parseInt(pidLine, 10), 2020, 'the hand-off records the new owner')
+  assert.equal(Number.parseInt(startedLine, 10), startedAt, 'the holder age must not restart during hand-off')
+})
+
 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())

apps/desktop/electron/update-marker.ts

@@ -119,15 +119,30 @@ export function readLiveUpdateMarker(
  *
  * 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.
+ * later adopt it or another hand-off stage may replace the PID. A live
+ * holder's original timestamp is preserved across those transfers so retries
+ * cannot keep resetting the 20-minute stale ceiling. 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.
  */
-export function writeUpdateMarker(hermesHome, pid, { now = Date.now } = {}) {
+export function writeUpdateMarker(
+  hermesHome,
+  pid,
+  {
+    kill,
+    now = Date.now,
+    maxAgeMs = UPDATE_MARKER_MAX_AGE_MS
+  }: {
+    now?: () => number
+    maxAgeMs?: number
+    kill?: typeof process.kill
+  } = {}
+) {
   const file = markerPath(hermesHome)
-  const startedAt = Math.floor(now() / 1000)
+  const nowMs = now()
+  const owner = readLiveUpdateMarker(hermesHome, { kill, maxAgeMs, now: () => nowMs })
+  const startedAt = owner ? Math.floor((nowMs - owner.ageMs) / 1000) : Math.floor(nowMs / 1000)
 
   try {
     fs.writeFileSync(file, `${pid}\n${startedAt}\n`, 'utf8')