From c3d19d497a9ca4138476c4d7a58d272d024b64ea Mon Sep 17 00:00:00 2001
From: Teknium <127238744+teknium1@users.noreply.github.com>
Date: Mon, 5 Oct 2026 22:39:40 -0700
Subject: [PATCH] fix: a failing withdraw helper is asked once, not three times

withdrawBridge re-asked every unsettled answer, so on the never-started
path a helper that errors (a hung or blocked interpreter) cost 3 x the 20 s
MARKER_HELPER_TIMEOUT_MS with the user waiting. Only `busy` (the sidecar
lock, usually brief) is re-asked now; every other answer stands at once
and the existing settled / quit decision is unchanged. The existing test's
"asked three times" for `error` encoded that cost and now expects one ask
(review K132354 P3 withdraw-retries-error).
---
 apps/desktop/electron/updater/checkout-handoff.test.ts |  5 +++--
 apps/desktop/electron/updater/checkout.ts              | 10 +++++++---
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/apps/desktop/electron/updater/checkout-handoff.test.ts b/apps/desktop/electron/updater/checkout-handoff.test.ts
index ff00bb35a3..c736bacc1b 100644
--- a/apps/desktop/electron/updater/checkout-handoff.test.ts
+++ b/apps/desktop/electron/updater/checkout-handoff.test.ts
@@ -390,8 +390,9 @@ describe.skipIf(IS_WINDOWS)('protocol 2 hand-off', () => {
         expect(deps.startHermes).not.toHaveBeenCalled()
         expect(kill).not.toHaveBeenCalled()
         expect(deps.markQuittingForHandoff).toHaveBeenCalled()
-        // Asked again while unsettled, and Electron left the bytes alone.
-        expect(helperCalls(deps.hermesHome)).toHaveLength(3)
+        // Re-asked only while the lock is busy (an error re-asked could cost
+        // 3 x 20 s, review K132354 P3), and Electron left the bytes alone.
+        expect(helperCalls(deps.hermesHome)).toHaveLength(answer === 'busy' ? 3 : 1)
         expect(fs.readFileSync(markerPath(deps.hermesHome), 'utf8')).toBe(runs[0]!.bridgeAtSpawn)
       } finally {
         fs.rmSync(root, { recursive: true, force: true })
diff --git a/apps/desktop/electron/updater/checkout.ts b/apps/desktop/electron/updater/checkout.ts
index ea315a3644..5029f436e5 100644
--- a/apps/desktop/electron/updater/checkout.ts
+++ b/apps/desktop/electron/updater/checkout.ts
@@ -89,7 +89,11 @@ const STILL_RUNNING_VERDICTS = new Set(['held', 'busy', 'live'])
  */
 const WITHDRAW_SETTLED_VERDICTS = new Set(['withdrawn', 'absent', 'foreign'])
 
-/** Withdraw attempts before an unsettled answer stands (a busy lock is usually brief). */
+/**
+ * Withdraw attempts while the sidecar lock is `busy` (usually brief). Any
+ * other unsettled answer stands at once: an `error` re-asked would cost up to
+ * MARKER_HELPER_TIMEOUT_MS per try with the user waiting.
+ */
 const WITHDRAW_ATTEMPTS = 3
 const WITHDRAW_RETRY_MS = 250
 
@@ -137,7 +141,7 @@ export function createCheckoutStrategy(deps: CheckoutStrategyDeps): UpdaterStrat
     })
   }
 
-  /** `withdraw`, re-asked while the answer leaves the bridge in place; `taken` ends it at once. */
+  /** `withdraw`, re-asked only while the lock is `busy`; every other answer ends it at once. */
   async function withdrawBridge(runId: string): Promise<MarkerHelperVerdict> {
     let verdict: MarkerHelperVerdict = { kind: 'error' }
 
@@ -148,7 +152,7 @@ export function createCheckoutStrategy(deps: CheckoutStrategyDeps): UpdaterStrat
 
       verdict = await markerHelper('withdraw', runId)
 
-      if (verdict.kind === 'taken' || WITHDRAW_SETTLED_VERDICTS.has(verdict.kind)) {
+      if (verdict.kind !== 'busy') {
         break
       }
     }