From 2fd6280cdba1e12582b6d0963dc97b0404f7207d Mon Sep 17 00:00:00 2001
From: cliffhall <cliff@futurescale.com>
Date: Sun, 23 Aug 2026 12:57:00 -0400
Subject: [PATCH] fix(web): reconcile a settled modal save, not just a settled
 toggle
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Review round 6 on #2091. The modal writer recorded its write but threw
away the settled result, so the mixed-writer overlap was still broken:
the modal's flush is in flight with `paginatedLists: true` when the
sidebar toggle fails and rolls back to the stale entry's `false`, then
the save lands. It is the settled state, but nothing re-applied it — so
with the list reload also failing, disk held `true` while the override
and the live client sat on `false`.

Both writers now share the reconciliation as well as the record: a
settled write for the still-active server re-applies its value to the
override and the live client.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UL9retfvAXRgvi6EWk4SY4
Signed-off-by: cliffhall <cliff@futurescale.com>
---
 clients/web/src/App.test.tsx | 47 ++++++++++++++++++++++++++++++++++++
 clients/web/src/App.tsx      | 11 ++++++++-
 2 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx
index ed1a83c8..c4bc832a 100644
--- a/clients/web/src/App.test.tsx
+++ b/clients/web/src/App.test.tsx
@@ -2577,6 +2577,53 @@ describe("App paginated list pagination toggle (#1721)", () => {
     expect(lastPush?.paginatedLists).toBe(true);
   });
 
+  it("re-applies a settled modal save after a toggle failed first (#2089)", async () => {
+    // The mixed-writer version of the order above: the modal's flush is in
+    // flight with `paginatedLists: true` when the sidebar toggle fails and
+    // rolls back to the stale entry's `false`. The modal save then lands, so it
+    // is the settled state — and it has to re-apply itself for the same reason
+    // the toggle does, or disk holds `true` while the UI and the live client
+    // sit on `false`.
+    const user = userEvent.setup();
+    let landModal: (() => void) | undefined;
+    const modalWrite = new Promise<void>((resolve) => {
+      landModal = resolve;
+    });
+    renderWithMantine(<App />);
+    await user.click(screen.getByText("connect"));
+    await waitFor(() => expect(clientInstances).toHaveLength(1));
+
+    const draftOptions = vi.mocked(useSettingsDraft).mock.calls.at(-1)?.[0];
+    if (!draftOptions) throw new Error("useSettingsDraft was never called");
+    updateServerSettingsSpy.mockImplementationOnce(() => modalWrite);
+    const persisted = draftOptions.onPersist("A", {
+      ...settingsWithRoots([]),
+      paginatedLists: true,
+    });
+
+    updateServerSettingsSpy.mockRejectedValueOnce(new Error("disk full"));
+    await user.click(screen.getByText("paginated-off"));
+    await waitFor(() =>
+      expect(screen.getByTestId("tools-paginated")).toHaveTextContent("false"),
+    );
+
+    await act(async () => {
+      landModal?.();
+      await persisted;
+    });
+
+    await waitFor(() =>
+      expect(screen.getByTestId("tools-paginated")).toHaveTextContent("true"),
+    );
+    const client = clientInstances[0] as EventTarget & {
+      setServerSettings: ReturnType<typeof vi.fn>;
+    };
+    const lastPush = client.setServerSettings.mock.calls.at(-1)?.[0] as {
+      paginatedLists?: boolean;
+    };
+    expect(lastPush?.paginatedLists).toBe(true);
+  });
+
   it("lets a successful list read supersede a concrete rollback override (#2089)", async () => {
     // A rollback sets the override to a value rather than clearing it, so the
     // effect that drops it cannot key on the persisted boolean alone: if a
diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx
index e41a71f0..80973768 100644
--- a/clients/web/src/App.tsx
+++ b/clients/web/src/App.tsx
@@ -4228,7 +4228,16 @@ function App() {
       // reads keep failing. Both settings writers feed the same per-server
       // record, so a rollback in either takes the most recent write for that
       // server, not just the most recent write *of its own kind*.
-      write.landed(value);
+      //
+      // And the reconciliation is shared too, for the mixed-writer overlap: a
+      // toggle failing while this save is in flight rolls the UI and the live
+      // client back to a baseline this save then replaces on disk, so a settled
+      // save has to re-apply itself exactly as a settled toggle does (#2089).
+      const settled = write.landed(value);
+      if (settled && activeServerIdRef.current === id) {
+        setPaginatedListsOverride(value.paginatedLists ?? false);
+        inspectorClient?.setServerSettings(value);
+      }
     },
     // Surface failures via toast — the modal usually closes
     // immediately on user dismiss, so a silent fail-on-flush would