fix(updater): avoid deleting shared ShipIt state

src/main/mac-update-install-marker.test.ts

@@ -1,12 +1,4 @@
-import {
-  existsSync,
-  mkdirSync,
-  mkdtempSync,
-  readFileSync,
-  readdirSync,
-  rmSync,
-  writeFileSync
-} from 'node:fs'
+import { mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
@@ -16,7 +8,6 @@ const {
   getVersionMock,
   recordUpdaterLifecycleMock,
   markerPathRef,
-  isShipItProvenExitedMock,
   getShipItLivenessMock,
   getProcessStartTimesMock,
   isProcessAliveMock
@@ -24,7 +15,6 @@ const {
   getVersionMock: vi.fn(),
   recordUpdaterLifecycleMock: vi.fn(),
   markerPathRef: { dir: '', bundle: '/Applications/Orca.app' },
-  isShipItProvenExitedMock: vi.fn(),
   getShipItLivenessMock: vi.fn(),
   getProcessStartTimesMock: vi.fn(),
   isProcessAliveMock: vi.fn()
@@ -35,7 +25,6 @@ vi.mock('./updater-lifecycle-diagnostics', () => ({
   recordUpdaterLifecycle: recordUpdaterLifecycleMock
 }))
 vi.mock('../shared/shipit-liveness', () => ({
-  isShipItProvenExited: isShipItProvenExitedMock,
   getShipItLivenessForBundle: getShipItLivenessMock,
   getProcessStartTimes: getProcessStartTimesMock,
   isRecordedProcessAlive: isProcessAliveMock
@@ -73,8 +62,6 @@ vi.mock('../shared/mac-update-install-marker', async (importOriginal) => {
 
 import type * as MarkerModule from '../shared/mac-update-install-marker'
 import {
-  _setShipItStatePathForTests,
-  canDeleteShipItState,
   isMacUpdateInstallInFlight,
   markMacUpdateInstallInFlight,
   reconcileMacUpdateInstallMarker,
@@ -116,21 +103,16 @@ beforeEach(() => {
   markerPathRef.bundle = '/Applications/Orca.app'
   recordUpdaterLifecycleMock.mockReset()
   getVersionMock.mockReset()
-  isShipItProvenExitedMock.mockReset()
-  isShipItProvenExitedMock.mockReturnValue(false)
   getShipItLivenessMock.mockReset()
   getShipItLivenessMock.mockReturnValue('live')
   getProcessStartTimesMock.mockReset()
   getProcessStartTimesMock.mockReturnValue(new Map())
   isProcessAliveMock.mockReset()
   isProcessAliveMock.mockReturnValue(false)
-  // Never let a test resolve this to the real Squirrel cache.
-  _setShipItStatePathForTests(join(dir, 'ShipItState.plist'))
 })
 
 afterEach(() => {
   Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true })
-  _setShipItStatePathForTests(null)
   rmSync(dir, { recursive: true, force: true })
 })
 
@@ -234,38 +216,6 @@ describe('shouldExitForInFlightMacUpdateInstall', () => {
   })
 })
 
-describe('reconcile safety', () => {
-  it('leaves the installer state alone while a swap is still running', () => {
-    // A Dock or alternate-profile launch can reconcile while ShipIt is legitimately mid-swap;
-    // deleting its state there would cancel a working install.
-    const shipItState = join(dir, 'ShipItState.plist')
-    writeFileSync(shipItState, 'live installer state', 'utf8')
-    writeMarker()
-    getVersionMock.mockReturnValue('1.4.194')
-    isShipItProvenExitedMock.mockReturnValue(false)
-
-    reconcileMacUpdateInstallMarker()
-
-    expect(lifecycleNames()).toContain('install_did_not_apply')
-    expect(existsSync(shipItState)).toBe(true)
-  })
-
-  it('clears state left by an installer that is gone, which otherwise blocks updates forever', () => {
-    // #14732 needed this deleted by hand; a resumed stale state pins the machine to an update
-    // that can never complete.
-    const shipItState = join(dir, 'ShipItState.plist')
-    writeFileSync(shipItState, 'stale installer state', 'utf8')
-    writeMarker()
-    getVersionMock.mockReturnValue('1.4.194')
-    isShipItProvenExitedMock.mockReturnValue(true)
-
-    reconcileMacUpdateInstallMarker()
-
-    expect(existsSync(shipItState)).toBe(false)
-    expect(lifecycleNames()).toContain('shipit_state_cleared')
-  })
-})
-
 describe('startup gate liveness', () => {
   it('exits during the pre-spawn window, before the installer exists', () => {
     // Pins the writerAlive wiring in the startup gate. Without it, reverting to a bare liveness
@@ -320,25 +270,14 @@ describe('startup gate liveness', () => {
     expect(shouldExitForInFlightMacUpdateInstall()).toBe(false)
   })
 
-  it('refuses to resolve a real home path under test, and allows it in production', () => {
-    // Asserting the decision directly. An absence check cannot tell "the guard worked" from "the
-    // real file happened not to exist" — which is how this test passed while still being able to
-    // delete a developer's own Squirrel state.
-    expect(canDeleteShipItState({ hasPathOverride: false, isUnderTest: true })).toBe(false)
-    expect(canDeleteShipItState({ hasPathOverride: true, isUnderTest: true })).toBe(true)
-    expect(canDeleteShipItState({ hasPathOverride: false, isUnderTest: false })).toBe(true)
-  })
-
-  it('keeps a concurrent install whose owner is still running', () => {
-    // Reconcile reads then clears; an install committed in between must survive, or the next
-    // launch is free to cancel it.
+  it('keeps an install committed while an older attempt is being settled', () => {
+    // Reconcile reads then renames one exact attempt; a concurrently written one must survive.
     writeMarker({ targetVersion: '1.4.195', createdAtMs: Date.now() - 30_000 })
     getVersionMock.mockReturnValue('1.4.194')
     const original = reconcileMacUpdateInstallMarker
-    // Simulate the race by rewriting the marker between read and clear.
+    // Simulate the race by writing another marker between read and settle.
     recordUpdaterLifecycleMock.mockImplementation(() => {
-      // A live owner (this process) means the attempt is still going; only a dead owner's marker
-      // is spent. A wall-clock cutoff would have wrongly cleared this.
+      // A live owner (this process) means the new attempt is still going.
       writeMarker({
         targetVersion: '1.4.196',
         createdAtMs: Date.now(),

src/main/mac-update-install-marker.ts

@@ -26,11 +26,8 @@ import {
   getShipItLivenessForBundle,
   isRecordedProcessAlive
 } from '../shared/shipit-liveness'
-import { clearWedgedShipItState } from './mac-update-shipit-state'
 import { recordUpdaterLifecycle } from './updater-lifecycle-diagnostics'
 
-export { _setShipItStatePathForTests, canDeleteShipItState } from './mac-update-shipit-state'
-
 /**
  * Publish "an install is in flight for this bundle" where a launching process can see it.
  *
@@ -271,12 +268,8 @@ export function reconcileMacUpdateInstallMarker(): void {
   //
   // Only the reported attempt is settled. An unexpired older sibling can therefore report its own
   // outcome on a later startup — telemetry noise, bounded by expiry, and preferable to settling
-  // attempts this pass never examined. The ShipIt cleanup below stays guarded by proven-exited, so
-  // a duplicate report cannot act on a swap that has since become real.
+  // attempts this pass never examined.
   settleMacUpdateInstallMarker(marker)
-  if (!applied) {
-    clearWedgedShipItState(bundlePath)
-  }
 }
 
 /**

src/main/mac-update-shipit-state.ts

@@ -1,56 +0,0 @@
-import { unlinkSync } from 'node:fs'
-import { homedir } from 'node:os'
-import { join } from 'node:path'
-import { isShipItProvenExited } from '../shared/shipit-liveness'
-import { recordUpdaterLifecycle } from './updater-lifecycle-diagnostics'
-
-// Why injectable: this is destructive, and tests must never resolve it to the developer's cache.
-let shipItStatePathOverride: string | null = null
-
-function getShipItStatePath(): string {
-  return (
-    shipItStatePathOverride ??
-    join(homedir(), 'Library', 'Caches', 'com.stablyai.orca.ShipIt', 'ShipItState.plist')
-  )
-}
-
-export function _setShipItStatePathForTests(path: string | null): void {
-  shipItStatePathOverride = path
-}
-
-/** May this process delete the installer's state file? */
-export function canDeleteShipItState(input: {
-  hasPathOverride: boolean
-  isUnderTest: boolean
-}): boolean {
-  return input.hasPathOverride || !input.isUnderTest
-}
-
-/** Delete Squirrel's staged state only after its installer is proven gone. */
-export function clearWedgedShipItState(bundlePath: string): void {
-  if (!isShipItProvenExited(bundlePath)) {
-    return
-  }
-  // A forgotten override must not let a test delete the developer's real Squirrel state.
-  if (
-    !canDeleteShipItState({
-      hasPathOverride: shipItStatePathOverride !== null,
-      isUnderTest: Boolean(process.env.VITEST)
-    })
-  ) {
-    return
-  }
-  try {
-    unlinkSync(getShipItStatePath())
-    recordUpdaterLifecycle(
-      'shipit_state_cleared',
-      {},
-      {
-        level: 'warn',
-        message: 'Cleared a stale Squirrel install state left behind by a failed update'
-      }
-    )
-  } catch {
-    // Absent is normal; stale state exists only after an aborted install.
-  }
-}

src/shared/shipit-liveness.ts

@@ -75,16 +75,6 @@ export function getShipItLivenessForBundle(bundlePath: string): ShipItLiveness {
   }
 }
 
-/** Destructive cleanup requires positive evidence of absence; uncertainty never qualifies. */
-export function isShipItProvenExited(bundlePath: string): boolean {
-  return getShipItLivenessForBundle(bundlePath) === 'exited'
-}
-
-/** True while the installer may still be working — live, or we could not tell. */
-export function mayShipItBeRunning(bundlePath: string): boolean {
-  return getShipItLivenessForBundle(bundlePath) !== 'exited'
-}
-
 /** Read process birth times in one bounded probe so marker count cannot multiply subprocesses. */
 export function getProcessStartTimes(pids: readonly number[]): ReadonlyMap<number, number> | null {
   const uniquePids = [...new Set(pids.filter((pid) => Number.isInteger(pid) && pid > 0))]