mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): treat failed process reconciliation as hard recovery
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>
This commit is contained in:
co-authored by
Tyler Longwell
parent
7722639f8e
commit
2983539742
@@ -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 () => {
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user