fix(web): reconcile a settled modal save, not just a settled toggle
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>
This commit is contained in:
parent
a935de08dc
commit
2fd6280cdb
2 changed files with 57 additions and 1 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in a new issue