From 2983539742ae8d5749db92572da9e68617705117 Mon Sep 17 00:00:00 2001
From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d
 <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co>
Date: Sat, 18 Jul 2026 20:07:06 -0400
Subject: [PATCH] fix(desktop): treat failed process reconciliation as hard
 recovery
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Mari's re-review: after a rollback/reconciliation pass, a failed
stopAgent leaves a process running under the wrong scope and a failed
startAgent leaves it stopped — committing a normal UI scope in either
case claims a convergence that does not exist. All process rollbacks
are still attempted, but any failure now surfaces onUnrecoverable
(card disables the toggle) instead of setUi.

Tests: the one-rollback-restart-fails case now asserts hard recovery
and no UI claim; new rollback-stop-failure case asserts the same.

Co-authored-by: Tyler Longwell <tlongwell@block.xyz>
Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
---
 .../ui/acpSessionScopeSetting.test.mjs        | 30 +++++++++++++++++--
 .../settings/ui/acpSessionScopeSetting.ts     | 16 ++++++++--
 2 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/desktop/src/features/settings/ui/acpSessionScopeSetting.test.mjs b/desktop/src/features/settings/ui/acpSessionScopeSetting.test.mjs
index 992b3d1ea..376852c95 100644
--- a/desktop/src/features/settings/ui/acpSessionScopeSetting.test.mjs
+++ b/desktop/src/features/settings/ui/acpSessionScopeSetting.test.mjs
@@ -89,7 +89,7 @@ describe("ACP session scope setting", () => {
     assert.equal(calls.at(-1)[1], false);
   });
 
-  it("attempts every process rollback even when one rollback restart fails", async () => {
+  it("attempts every process rollback and surfaces hard recovery when one rollback restart fails", async () => {
     const first = {
       pubkey: "first",
       status: "running",
@@ -120,6 +120,8 @@ describe("ACP session scope setting", () => {
       applyAcpSessionScopeSetting(false, true, deps),
       /apply failed/,
     );
+    // Every process rollback is still attempted, but a failed reconciliation
+    // must surface hard recovery instead of claiming a normal scope.
     assert.deepEqual(calls, [
       ["backend", "thread"],
       ["stop", "first"],
@@ -131,9 +133,33 @@ describe("ACP session scope setting", () => {
       ["start", "first"],
       ["stop", "second"],
       ["start", "second"],
-      ["ui", false],
+      ["unrecoverable"],
     ]);
     assert.equal(secondStarts, 2);
+    assert.ok(!calls.some((c) => c[0] === "ui"));
+  });
+
+  it("surfaces hard recovery when a rollback stop fails", async () => {
+    let stops = 0;
+    const { calls, deps } = harness({
+      stopAgent: async (pubkey) => {
+        calls.push(["stop", pubkey]);
+        stops += 1;
+        if (stops === 2) throw new Error("rollback stop failed");
+      },
+      startAgent: async (pubkey) => {
+        calls.push(["start", pubkey]);
+        if (calls.filter((c) => c[0] === "start").length === 1)
+          throw new Error("apply failed");
+      },
+    });
+    await assert.rejects(
+      applyAcpSessionScopeSetting(false, true, deps),
+      /apply failed/,
+    );
+    // The process may still be running under the wrong scope: no UI claim.
+    assert.ok(calls.some((c) => c[0] === "unrecoverable"));
+    assert.ok(!calls.some((c) => c[0] === "ui"));
   });
 
   it("reconciles UI and processes to the re-read authoritative scope when rollback persistence fails", async () => {
diff --git a/desktop/src/features/settings/ui/acpSessionScopeSetting.ts b/desktop/src/features/settings/ui/acpSessionScopeSetting.ts
index 0e91e8ab9..201596159 100644
--- a/desktop/src/features/settings/ui/acpSessionScopeSetting.ts
+++ b/desktop/src/features/settings/ui/acpSessionScopeSetting.ts
@@ -35,7 +35,8 @@ async function restartRunningLocalAgents(
  * processes must converge on one authoritative scope. Rollback is only claimed
  * after the rollback write is confirmed; if that write fails, the authoritative
  * value is re-read and UI/processes reconcile to it. If the authority cannot be
- * read either, `onUnrecoverable` fires and nothing pretends to know the scope.
+ * read, or any process fails to reconcile under it, `onUnrecoverable` fires and
+ * nothing pretends to know the scope.
  */
 export async function applyAcpSessionScopeSetting(
   previous: boolean,
@@ -72,7 +73,11 @@ export async function applyAcpSessionScopeSetting(
       }
     }
     // Reconcile every affected process under the confirmed authoritative
-    // scope — never under an assumed one.
+    // scope — never under an assumed one. Any failure here means a process
+    // may still be running under the wrong scope (or not running at all),
+    // so convergence must not be claimed: finish every attempt, then
+    // surface hard recovery instead of committing a normal UI scope.
+    let reconciliationFailed = false;
     for (const agent of agents) {
       if (agent.status !== "running" || agent.backend.type !== "local")
         continue;
@@ -80,13 +85,18 @@ export async function applyAcpSessionScopeSetting(
         await deps.stopAgent(agent.pubkey);
         await deps.startAgent(agent.pubkey);
       } catch (rollbackError) {
+        reconciliationFailed = true;
         console.error(
           `Failed to roll back ACP session-scope process ${agent.pubkey}`,
           rollbackError,
         );
       }
     }
-    deps.setUi(authoritative);
+    if (reconciliationFailed) {
+      deps.onUnrecoverable();
+    } else {
+      deps.setUi(authoritative);
+    }
     throw error;
   }
 }