modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

Merge branch 'v2/main' into v2/feat/2082-file-secret-store-lock

Browse source
This commit is contained in:
Cliff Hall 2026-08-23 17:22:17 -04:00 • committed by GitHub
commit 9bc6ce1a68
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 1190 additions and 43 deletions
Download patch file Download diff file Expand all files Collapse all files

540
clients/web/src/App.test.tsx
View file

@ -1628,6 +1628,9 @@ describe("App task wiring", () => {
type RootsFakeClient = EventTarget & {
setRoots: ReturnType<typeof vi.fn>;
getRoots: ReturnType<typeof vi.fn>;
// Close also pushes the settings it decided to apply, which is what the
// failed-save case asserts on (#2089).
setServerSettings: ReturnType<typeof vi.fn>;
};
const settingsWithRoots = (
@ -1718,6 +1721,99 @@ describe("App roots live-apply on settings-dialog close", () => {
);
expect(client.setRoots).not.toHaveBeenCalled();
});
it("applies the last persisted settings on close after a save failed (#2089)", async () => {
// `useSettingsDraft` keeps the draft when a save rejects, so closing the
// modal would push a value that never reached disk into the live client —
// undoing the rollback that reported the failure. Close has to apply what
// landed instead.
const user = userEvent.setup();
updateServerSettingsSpy.mockClear();
const failedDraft: InspectorServerSettings = {
...settingsWithRoots([{ uri: "file:///rejected" }]),
paginatedLists: true,
};
const client = await openSettingsForConnectedServer(failedDraft);
client.getRoots.mockReturnValue([]);
const draftOptions = vi.mocked(useSettingsDraft).mock.calls.at(-1)?.[0];
if (!draftOptions) throw new Error("useSettingsDraft was never called");
// One save lands, so the tracker has an account of disk…
const persistedSettings: InspectorServerSettings = {
...settingsWithRoots([]),
paginatedLists: false,
};
await act(async () => {
await draftOptions.onPersist("A", persistedSettings);
});
// …then the next one rejects, leaving the draft holding an unsaved edit.
updateServerSettingsSpy.mockRejectedValueOnce(new Error("disk full"));
await act(async () => {
await draftOptions.onPersist("A", failedDraft).catch(() => undefined);
});
client.setServerSettings.mockClear();
await closeModal(user);
await waitFor(() => expect(client.setServerSettings).toHaveBeenCalled());
expect(client.setServerSettings).toHaveBeenLastCalledWith(
persistedSettings,
);
// …and the rejected roots edit is not advertised either.
expect(client.setRoots).not.toHaveBeenCalled();
});
it("restores the roots and log size too when a flushed save rejects (#2089)", async () => {
// Close flushes a pending save and live-applies the draft immediately —
// roots and the Network log size, not just the settings object. If that
// save then rejects, reconciling only `setServerSettings` would leave the
// server advertising roots that were never persisted.
const user = userEvent.setup();
updateServerSettingsSpy.mockClear();
const rejectedDraft: InspectorServerSettings = {
...settingsWithRoots([{ uri: "file:///rejected" }]),
maxFetchRequests: 10,
};
const client = await openSettingsForConnectedServer(rejectedDraft);
client.getRoots.mockReturnValue([]);
const draftOptions = vi.mocked(useSettingsDraft).mock.calls.at(-1)?.[0];
if (!draftOptions) throw new Error("useSettingsDraft was never called");
const persistedSettings = settingsWithRoots([]);
await act(async () => {
await draftOptions.onPersist("A", persistedSettings);
});
// Close applies the draft live — the state the flush leaves behind.
const fetchLog = fetchLogInstances.at(-1) as EventTarget & {
setMaxFetchRequests: ReturnType<typeof vi.fn>;
};
await closeModal(user);
await waitFor(() =>
expect(client.setRoots).toHaveBeenCalledWith([
{ uri: "file:///rejected" },
]),
);
expect(fetchLog.setMaxFetchRequests).toHaveBeenLastCalledWith(10);
client.getRoots.mockReturnValue([{ uri: "file:///rejected" }]);
client.setRoots.mockClear();
// …and only now does the flushed save reject.
updateServerSettingsSpy.mockRejectedValueOnce(new Error("disk full"));
await act(async () => {
await draftOptions.onPersist("A", rejectedDraft).catch(() => undefined);
});
// The whole live surface goes back to what landed — settings, roots, and
// the log size, each of which close had moved to the rejected draft.
await waitFor(() => expect(client.setRoots).toHaveBeenCalledWith([]));
expect(client.setServerSettings).toHaveBeenLastCalledWith(
persistedSettings,
);
expect(fetchLog.setMaxFetchRequests).toHaveBeenLastCalledWith(
persistedSettings.maxFetchRequests,
);
});
});
describe("App history pin/replay", () => {
@ -2423,6 +2519,450 @@ describe("App paginated list pagination toggle (#1721)", () => {
};
expect(lastPush?.paginatedLists).toBeFalsy();
});
it("rolls back to the last write that landed, not to a stale list entry (#2089)", async () => {
// Two toggles. The first PUT lands but the list reload behind it fails, so
// the `servers` entry keeps describing disk as it was *before* it — which
// this suite models exactly, because `useServers` is mocked to return the
// same entry forever. The second PUT fails outright and rolls back.
//
// Reverting to the `servers` entry there lands on `false`, the value from
// before the first toggle, contradicting what is on disk. The baseline has
// to come from the write that landed instead.
//
// One failed toggle cannot distinguish the two baselines — they only differ
// once a write has landed that the list never caught up with — so the test
// needs the full sequence.
const user = userEvent.setup();
renderWithMantine(<App />);
await user.click(screen.getByText("connect"));
await waitFor(() => expect(clientInstances).toHaveLength(1));
await user.click(screen.getByText("paginated-on"));
await waitFor(() =>
expect(updateServerSettingsSpy).toHaveBeenCalledWith(
"A",
expect.objectContaining({ paginatedLists: true }),
),
);
updateServerSettingsSpy.mockRejectedValueOnce(new Error("disk full"));
await user.click(screen.getByText("paginated-off"));
// Back to the value the first toggle put on disk, not to the pre-toggle one.
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("rolls back to a write that landed while this one was in flight (#2089)", async () => {
// Overlapping toggles. The baseline cannot be captured when a write is
// issued: toggle 1 is still in flight when toggle 2 reads it, so toggle 2
// would close over `false` — and by the time toggle 2 fails, toggle 1 has
// landed `true` on disk. Rolling back to the captured value contradicts
// disk exactly as the stale-list case does; the baseline has to be resolved
// at failure time. The two writes are driven by deferred promises so the
// order — issue 1, issue 2, land 1, fail 2 — is exact rather than timed.
const user = userEvent.setup();
let landFirst: (() => void) | undefined;
let failSecond: ((err: Error) => void) | undefined;
const first = new Promise<void>((resolve) => {
landFirst = resolve;
});
const second = new Promise<void>((_resolve, reject) => {
failSecond = (err) => {
reject(err);
};
});
updateServerSettingsSpy
.mockImplementationOnce(() => first)
.mockImplementationOnce(() => second);
renderWithMantine(<App />);
await user.click(screen.getByText("connect"));
await waitFor(() => expect(clientInstances).toHaveLength(1));
await user.click(screen.getByText("paginated-on"));
await user.click(screen.getByText("paginated-off"));
await waitFor(() =>
expect(updateServerSettingsSpy).toHaveBeenCalledTimes(2),
);
await act(async () => {
landFirst?.();
await first;
});
await act(async () => {
failSecond?.(new Error("disk full"));
await second.catch(() => undefined);
});
// `true` is what the landed write put on disk — not the `false` the second
// toggle read before it got there.
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("re-applies a write that lands after an overlapping one failed (#2089)", async () => {
// The mirror of the test above, in the other settlement order: toggle 1
// (`true`) is still in flight when toggle 2 (`false`) fails, so the rollback
// resolves to the stale entry's `false` — correct at that instant, since
// nothing had landed. Toggle 1 then lands `true`. If its list reload failed
// too, no render will ever correct the UI, so the write that settles has to
// re-apply itself.
const user = userEvent.setup();
let landFirst: (() => void) | undefined;
let failSecond: ((err: Error) => void) | undefined;
const first = new Promise<void>((resolve) => {
landFirst = resolve;
});
const second = new Promise<void>((_resolve, reject) => {
failSecond = (err) => {
reject(err);
};
});
updateServerSettingsSpy
.mockImplementationOnce(() => first)
.mockImplementationOnce(() => second);
renderWithMantine(<App />);
await user.click(screen.getByText("connect"));
await waitFor(() => expect(clientInstances).toHaveLength(1));
await user.click(screen.getByText("paginated-on"));
await user.click(screen.getByText("paginated-off"));
await waitFor(() =>
expect(updateServerSettingsSpy).toHaveBeenCalledTimes(2),
);
// Fail the second write first, then land the first.
await act(async () => {
failSecond?.(new Error("disk full"));
await second.catch(() => undefined);
});
expect(screen.getByTestId("tools-paginated")).toHaveTextContent("false");
await act(async () => {
landFirst?.();
await first;
});
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("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("applies a settled write to the current client, not the one it started on (#2089)", async () => {
// A write can outlive a disconnect/reconnect to the *same* server: the id
// guard still passes, but the client captured when the write was issued has
// been destroyed and replaced by one built from the stale list entry. The
// settled value has to reach the live instance.
const user = userEvent.setup();
let landWrite: (() => void) | undefined;
const write = new Promise<void>((resolve) => {
landWrite = resolve;
});
updateServerSettingsSpy.mockImplementationOnce(() => write);
renderWithMantine(<App />);
await user.click(screen.getByText("connect"));
await waitFor(() => expect(clientInstances).toHaveLength(1));
await user.click(screen.getByText("paginated-on"));
// Reconnect to the same server while that write is in flight. The transport
// drops, then the user reconnects — and `onToggleConnection` always
// rebuilds the client so the latest saved settings are picked up.
act(() => {
clientInstances[0].dispatchEvent(new Event("disconnect"));
});
await user.click(screen.getByText("connect"));
await waitFor(() => expect(clientInstances).toHaveLength(2));
const replacement = clientInstances[1] as EventTarget & {
setServerSettings: ReturnType<typeof vi.fn>;
};
replacement.setServerSettings.mockClear();
await act(async () => {
landWrite?.();
await write;
});
await waitFor(() =>
expect(replacement.setServerSettings).toHaveBeenCalledWith(
expect.objectContaining({ paginatedLists: true }),
),
);
});
it("reconciles after a modal save fails onto a write that landed first (#2089)", async () => {
// Reverse of the mixed-writer case above: the toggle lands while the modal
// save is still pending, so it is told it is not settled and applies
// nothing. The modal save then fails, which makes the toggle's value the
// account of disk — and nothing else is left to apply it.
const user = userEvent.setup();
let failModal: ((err: Error) => void) | undefined;
const modalWrite = new Promise<void>((_resolve, reject) => {
failModal = (err) => {
reject(err);
};
});
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: false })
.catch(() => undefined);
// The toggle lands `true` while the modal save is still in flight.
await user.click(screen.getByText("paginated-on"));
await waitFor(() =>
expect(updateServerSettingsSpy).toHaveBeenCalledTimes(2),
);
const client = clientInstances[0] as EventTarget & {
setServerSettings: ReturnType<typeof vi.fn>;
};
client.setServerSettings.mockClear();
await act(async () => {
failModal?.(new Error("disk full"));
await persisted;
});
// The failure is what reconciles: a push carrying the landed value arrives
// only after it, since the landed write itself was not settled at the time.
await waitFor(() =>
expect(client.setServerSettings).toHaveBeenCalledWith(
expect.objectContaining({ paginatedLists: true }),
),
);
});
it("seeds the settings modal from the last write, not the stale entry (#2089)", async () => {
// The modal draft is the input to the *next* write, so seeding it from a
// `servers` entry that a failed list reload froze would send the superseded
// value back to disk the moment the user saves any unrelated field.
const user = userEvent.setup();
renderWithMantine(<App />);
await user.click(screen.getByText("connect"));
await waitFor(() => expect(clientInstances).toHaveLength(1));
await user.click(screen.getByText("paginated-on"));
await waitFor(() =>
expect(updateServerSettingsSpy).toHaveBeenCalledWith(
"A",
expect.objectContaining({ paginatedLists: true }),
),
);
// `servers` is mocked to a fixed entry here, so it still reports the
// pre-toggle value — exactly what a failed reload leaves behind.
const draftOptions = vi.mocked(useSettingsDraft).mock.calls.at(-1)?.[0];
if (!draftOptions) throw new Error("useSettingsDraft was never called");
expect(draftOptions.resolveInitial("A")).toEqual(
expect.objectContaining({ paginatedLists: true }),
);
});
it("builds the next client from the last write, not the stale entry (#2089)", async () => {
// Connecting is where the whole connection is configured, and it read the
// `servers` entry directly — so a save that landed while list reads were
// failing (including one made from the modal for a server that was not
// connected at the time) was undone by the next connect, which rebuilt the
// client from the frozen entry.
const user = userEvent.setup();
renderWithMantine(<App />);
const draftOptions = vi.mocked(useSettingsDraft).mock.calls.at(-1)?.[0];
if (!draftOptions) throw new Error("useSettingsDraft was never called");
const saved: InspectorServerSettings = {
...settingsWithRoots([{ uri: "file:///saved" }]),
maxFetchRequests: 42,
};
await act(async () => {
await draftOptions.onPersist("A", saved);
});
// `servers` still reports the entry with no settings at all — what a failed
// reload leaves. The client must be built from the write that landed.
await user.click(screen.getByText("connect"));
await waitFor(() => expect(clientInstances).toHaveLength(1));
const clientOptions = vi
.mocked(McpIndex.InspectorClient)
.mock.calls.at(-1)?.[1] as { roots?: { uri: string }[] } | undefined;
expect(clientOptions?.roots).toEqual([{ uri: "file:///saved" }]);
const fetchLogOptions = vi
.mocked(FetchLogModule.FetchRequestLogState)
.mock.calls.at(-1)?.[1] as { maxFetchRequests?: number } | undefined;
expect(fetchLogOptions?.maxFetchRequests).toBe(42);
});
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
// later read reports the value the override replaced — an edit made outside
// the Inspector overtaking the write — neither that boolean nor the server
// id changes, and the UI would stay stuck on the override forever. A read
// rebuilds the list, so the entry object is the signal that supersedes it.
const user = userEvent.setup();
const previousUseServers = vi.mocked(useServers).getMockImplementation();
try {
const { rerender } = renderWithMantine(<App />);
await user.click(screen.getByText("connect"));
await waitFor(() => expect(clientInstances).toHaveLength(1));
// A write lands `true` while list reads are stale, then a toggle fails —
// leaving the override on `true` with the entry still reporting `false`.
const draftOptions = vi.mocked(useSettingsDraft).mock.calls.at(-1)?.[0];
if (!draftOptions) throw new Error("useSettingsDraft was never called");
await act(async () => {
await 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("true"),
);
// Now a list read succeeds and still says `false` — same boolean, same
// server id, new entry object. It is authoritative and must win.
vi.mocked(useServers).mockReturnValue({
servers: [{ ...SERVER_A } as ServerEntry],
loading: false,
error: undefined,
refresh: vi.fn().mockResolvedValue(undefined),
addServer: vi.fn(),
updateServer: vi.fn(),
updateServerSettings: updateServerSettingsSpy,
removeServer: vi.fn(),
reorderServers: vi.fn(),
importSource: vi.fn().mockResolvedValue({ servers: {} }),
});
rerender(<App />);
await waitFor(() =>
expect(screen.getByTestId("tools-paginated")).toHaveTextContent(
"false",
),
);
} finally {
if (previousUseServers) {
vi.mocked(useServers).mockImplementation(previousUseServers);
}
}
});
it("rolls back to a settings-modal save that landed, not just a toggle (#2089)", async () => {
// The two settings writers feed one record, and this is the half the
// toggle-driven tests cannot reach: `useSettingsDraft` is mocked for this
// file, so its `onPersist` never runs on its own and a regression that
// dropped `begin`/`landed` from the modal path would go unnoticed. The
// callback the App handed the hook is invoked directly instead.
const user = userEvent.setup();
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");
const saved: InspectorServerSettings = {
...settingsWithRoots([]),
paginatedLists: true,
};
// The save lands; the list read behind it is what stays stale, so `servers`
// (mocked to a fixed entry here) never reports it.
await act(async () => {
await draftOptions.onPersist("A", saved);
});
expect(updateServerSettingsSpy).toHaveBeenCalledWith("A", saved);
updateServerSettingsSpy.mockRejectedValueOnce(new Error("disk full"));
await user.click(screen.getByText("paginated-off"));
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);
});
});
// A background command runs through `runCommandInBackground`, which owns the

252
clients/web/src/App.tsx
View file

@ -110,6 +110,8 @@ import { usePagedTools } from "@inspector/core/react/usePagedTools.js";
import { usePagedPrompts } from "@inspector/core/react/usePagedPrompts.js";
import { usePagedResources } from "@inspector/core/react/usePagedResources.js";
import { usePaginatedList } from "./hooks/usePaginatedList";
import { useLastPersistedSettings } from "./hooks/useLastPersistedSettings";
import { useValueChange } from "./hooks/useValueChange";
import type { ListPaginationControlsProps } from "./components/elements/ListPaginationControls/ListPaginationControls";
import { useManagedResourceTemplates } from "@inspector/core/react/useManagedResourceTemplates.js";
import { useManagedRequestorTasks } from "@inspector/core/react/useManagedRequestorTasks.js";
@ -1159,19 +1161,34 @@ function App() {
error: pagedResourcesLoadError,
loadPage: loadResourcesPage,
} = usePagedResources(inspectorClient, pagedResourcesState);
// What every settings write that landed on disk actually wrote, so a later
// failed write can be rolled back to that rather than to a `servers` entry
// frozen at the last successful list read (#2089).
const lastPersistedSettings = useLastPersistedSettings(servers);
// The active server's persisted paginated setting drives the display mode.
// The sidebar toggle edits it (optimistically, below) and persists it.
const activeServerEntry = servers.find((s) => s.id === activeServerId);
const persistedPaginatedLists =
servers.find((s) => s.id === activeServerId)?.settings?.paginatedLists ??
false;
activeServerEntry?.settings?.paginatedLists ?? false;
const [paginatedListsOverride, setPaginatedListsOverride] = useState<
boolean | null
>(null);
// Drop the optimistic override once the persisted value catches up (or the
// active server changes), so the persisted setting is the resting source.
useEffect(() => {
setPaginatedListsOverride(null);
}, [persistedPaginatedLists, activeServerId]);
//
// Keyed on the entry object rather than on the boolean it carries: the
// override can hold a *concrete* value (a rollback sets one instead of
// clearing), and if a later successful read reports the value that override
// replaced — an outside edit overtaking the write — the boolean and the
// server id are both unchanged and the UI stays stuck on it. Every successful
// read rebuilds the list, so the entry is the signal that supersedes it, and
// it subsumes the other two: the value is read off this entry, and switching
// servers selects a different one (#2089).
//
// Adjusted during render rather than in an effect, per this repo's rule
// against resetting state from render data in `useEffect` — the entry is
// referentially stable between reads, which is what `useValueChange` needs.
useValueChange(activeServerEntry, () => setPaginatedListsOverride(null));
const paginatedLists = paginatedListsOverride ?? persistedPaginatedLists;
// The malformed-entry report is written by the aggregate walk's salvage. In
// paginated mode the tools/prompts/resources panels render the paged stores
@ -1715,11 +1732,18 @@ function App() {
const activeServerNameRef = useRef<string | undefined>(undefined);
const activeServerIdRef = useRef<string | undefined>(activeServerId);
const serversRef = useRef(servers);
// The live client, mirrored for the same reason: a settings write that
// outlives a disconnect/reconnect to the *same* server would otherwise push
// its value into the instance captured when it was issued — which has since
// been destroyed — while the replacement client, built from the stale list
// entry, keeps the value the write replaced (#2089).
const inspectorClientRef = useRef<InspectorClient | null>(inspectorClient);
useEffect(() => {
if (activeServer) activeServerNameRef.current = activeServer.name;
activeServerIdRef.current = activeServerId;
serversRef.current = servers;
}, [activeServer, activeServerId, servers]);
inspectorClientRef.current = inspectorClient;
}, [activeServer, activeServerId, servers, inspectorClient]);
// Last connection-level error message, surfaced as `data-error-message` on
// the InspectorView header so an automated driver can read *why* a connect
@ -1906,6 +1930,43 @@ function App() {
// rebound every time the active server (and its log state) changes.
const fetchLogRef = useRef<FetchRequestLogState | null>(null);
// Make the live client reflect one settings value, in full. Every path that
// decides what the active server's settings *are* goes through this: the
// settings modal's close (#1444), and each write's settled / rolled-back
// reconciliation (#2089). It is one function because the live surface is more
// than `setServerSettings` — a partial re-application would leave the Network
// log sized for, and the server advertised roots from, a value that is not on
// disk. Reads the client through its ref so a write outliving a reconnect
// applies to the instance that exists now.
const applyLiveServerSettings = useCallback(
(settings: InspectorServerSettings) => {
const client = inspectorClientRef.current;
if (!client) return;
// Settings the managed state reads at notification time
// (auto-refresh-on-list-changed) take effect without a reconnect (#1444).
// Connection-time inputs (transport, OAuth, timeouts) still only apply on
// the next connect.
client.setServerSettings(settings);
// Resize the Network log buffer live so a maxFetchRequests edit takes
// effect without a reconnect (shrinking trims immediately).
fetchLogRef.current?.setMaxFetchRequests(settings.maxFetchRequests);
// Root edits are diffed against what the client currently advertises
// (both cleaned) and notified only when they differ: `setRoots` fires
// `notifications/roots/list_changed`, which makes the server re-request
// `roots/list`.
const nextRoots = cleanRoots(settings.roots);
const currentRoots = cleanRoots(client.getRoots());
if (JSON.stringify(nextRoots) !== JSON.stringify(currentRoots)) {
void client.setRoots(nextRoots).catch(() => {
// setRoots swallows notification failures internally; a throw here
// only means the client is mid-teardown — the persisted roots will
// re-advertise on the next connect, so nothing to surface.
});
}
},
[],
);
// Flush the pre-redirect Network log to backend storage, keyed by the OAuth
// authId carried in the authorization URL's `state`. Runs synchronously from
// `BrowserNavigation` right before `window.location.href`, so the keepalive
@ -2461,7 +2522,16 @@ function App() {
);
// The settings node persisted in mcp.json for this server — distinct
// from the InspectorClient options we're about to derive from it.
const savedSettings = server.settings;
//
// Through the tracker, not `server.settings` directly: the entry only
// advances on a successful list read, so a write that landed while reads
// were failing — including one made from the settings modal for a server
// that was not connected at the time — would otherwise be undone at the
// next connect, which builds the client from that frozen entry. This is
// the one place the whole connection is configured, so every construction
// path (connect, reconnect, OAuth resume) goes through it (#2089).
const savedSettings =
lastPersistedSettings.resolve(server.id) ?? server.settings;
const activeIdp = getActiveEnterpriseManagedAuthIdp(clientConfig);
const activeCimdUrl = getActiveCimdClientMetadataUrl(clientConfig);
// Flatten the persisted settings into the InspectorClient options shape.
@ -2633,6 +2703,7 @@ function App() {
onBeforeOAuthRedirect,
clientConfig,
newAppElicitationSession,
lastPersistedSettings,
],
);
@ -3730,9 +3801,12 @@ function App() {
setPaginatedListsOverride(value);
const current = servers.find((s) => s.id === activeServerId);
if (!current || activeServerId === undefined) return;
const prevSettings = current.settings ?? EMPTY_SETTINGS;
// Not `current.settings` directly: that entry only advances on a
// successful list read, so once one has failed it describes disk as it
// was *before* the writes made since. Build on the last write known to
// have landed while that is the fresher account (#2089).
const next: InspectorServerSettings = {
...prevSettings,
...(lastPersistedSettings.resolve(activeServerId) ?? EMPTY_SETTINGS),
paginatedLists: value,
};
inspectorClient?.setServerSettings(next);
@ -3757,26 +3831,75 @@ function App() {
// server's rehydrated secrets, so it can trigger the pending
// plaintext-to-encrypted upgrade even though the user only toggled
// pagination (#1950 review r22).
// Announced before the request goes out, so two toggles in flight at once
// are ordered by when they were issued rather than by which one's list
// reload finished first (#2089).
const write = lastPersistedSettings.begin(activeServerId);
void refreshingPersist(updateServerSettings, refreshInitialConfig)(
activeServerId,
next,
).catch((err: unknown) => {
// Persist failed: revert the optimistic override (the effect only
// clears it when the persisted value changes, which won't happen here)
// and roll the live client setting back, so the UI and client reflect
// the value that's actually on disk rather than the failed edit (#1721).
setPaginatedListsOverride(null);
inspectorClient?.setServerSettings(prevSettings);
notifications.show({
title: "Failed to save pagination setting",
message: err instanceof Error ? err.message : String(err),
color: "red",
)
.then(() => {
// This value is on disk now. Remember it as the rollback baseline for
// whatever is written next, since the `servers` entry it was derived
// from will keep describing the old value if the reload behind this
// write — or any later one — fails (#2089).
//
// Re-apply it when this write is the settled one: an overlapping
// toggle that failed *first* rolled the UI and the live client back
// to a baseline this write has since replaced, and if the list read
// behind this write failed too, nothing else would ever correct them.
// Through the *current* client, not this continuation's closure: a
// reconnect to the same server passes the id check while the captured
// instance is already destroyed.
const settled = write.landed(next);
if (settled && activeServerIdRef.current === activeServerId) {
setPaginatedListsOverride(next.paginatedLists ?? false);
applyLiveServerSettings(next);
}
})
.catch((err: unknown) => {
// This write is over and never reached disk, so it stops counting as
// in flight: an earlier write still running is the settled state once
// it lands, and is what re-applies the UI this rollback is about to
// set (#2089).
write.failed();
// Persist failed: revert the optimistic override and roll the live
// client setting back, so the UI and client reflect the value that's
// actually on disk rather than the failed edit (#1721).
//
// The baseline is resolved *here*, not captured when this write was
// issued: another toggle can land in between, and its value is what
// disk holds by the time this one fails. The override is set to that
// baseline rather than cleared, because clearing it falls back to
// `persistedPaginatedLists` — read from a `servers` entry that may be
// stale, showing the same wrong value from the other side (#2089).
//
// Applied only while this write's server is still the active one: the
// override is a single app-wide boolean and the live client belongs to
// whichever server is connected now, so a rejection arriving after the
// user switched would render this server's value on another one.
// The client is taken from the ref for the same reason the success
// path does: a reconnect to the same server passes the id check while
// this continuation's captured instance is already destroyed.
const baseline =
lastPersistedSettings.resolve(activeServerId) ?? EMPTY_SETTINGS;
if (activeServerIdRef.current === activeServerId) {
setPaginatedListsOverride(baseline.paginatedLists ?? false);
applyLiveServerSettings(baseline);
}
notifications.show({
title: "Failed to save pagination setting",
message: err instanceof Error ? err.message : String(err),
color: "red",
});
});
});
},
[
servers,
activeServerId,
lastPersistedSettings,
applyLiveServerSettings,
inspectorClient,
updateServerSettings,
refreshInitialConfig,
@ -4134,8 +4257,12 @@ function App() {
flush: flushSettingsDraft,
} = useSettingsDraft<InspectorServerSettings>({
targetId: settingsModalTargetId,
resolveInitial: (id) =>
servers.find((s) => s.id === id)?.settings ?? EMPTY_SETTINGS,
// Through the tracker rather than off `servers` directly: after a write
// lands whose list reload failed, that entry still describes the previous
// value, so seeding from it and saving an unrelated field would write the
// superseded value straight back to disk (#2089). `resolve` falls back to
// the entry when nothing has landed, so the ordinary case is unchanged.
resolveInitial: (id) => lastPersistedSettings.resolve(id) ?? EMPTY_SETTINGS,
// The saved settings may include an OAuth client secret or a stdio `env:`
// value, and writing one can change what the secrets file *is* — the first
// save under a newly-set passphrase re-encrypts a pre-existing plaintext
@ -4143,7 +4270,49 @@ function App() {
// unit rather than an inline `await …; refresh()` because this file is
// outside the coverage gate, so wiring written here is tested by nothing
// (#1950 review r17).
onPersist: refreshingPersist(updateServerSettings, refreshInitialConfig),
onPersist: async (id: string, value: InspectorServerSettings) => {
// Announced before the request, like the toggle's own write: the debounced
// flush can put two saves for one server in flight, and the later-issued
// one describes disk however the two happen to finish (#2089).
const write = lastPersistedSettings.begin(id);
try {
await refreshingPersist(updateServerSettings, refreshInitialConfig)(
id,
value,
);
} catch (err) {
// Stop counting as in flight before the rejection goes on to
// `onError` — an earlier write still running settles the state once it
// lands, and cannot know that while this one looks pending (#2089).
write.failed();
// A write that landed while this one was pending was told it was not
// settled and so applied nothing; this save never reached disk, so that
// write is the account of it and nothing else will re-apply it. Same
// reconciliation the toggle's own failure path does.
const baseline = lastPersistedSettings.resolve(id);
if (baseline && activeServerIdRef.current === id) {
setPaginatedListsOverride(baseline.paginatedLists ?? false);
applyLiveServerSettings(baseline);
}
throw err;
}
// Recorded for the same reason the pagination toggle records its own
// write: this is what disk holds now, and the `servers` entry it was
// edited from will keep describing the previous value for as long as list
// 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*.
//
// 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);
applyLiveServerSettings(value);
}
},
// Surface failures via toast — the modal usually closes
// immediately on user dismiss, so a silent fail-on-flush would
// leave the user thinking their last edits saved when they
@ -4276,25 +4445,20 @@ function App() {
settingsModalTargetId === activeServerId &&
settingsDraft
) {
// Push the edited settings onto the live client so settings the managed
// state reads at notification time (auto-refresh-on-list-changed) take
// effect without a reconnect (#1444). Connection-time inputs (transport,
// OAuth, timeouts) still only apply on the next connect.
inspectorClient.setServerSettings(settingsDraft);
// Resize the Network log buffer live so a maxFetchRequests edit takes
// effect without a reconnect (shrinking trims immediately). Connect-time
// construction also reads this, so a reconnect would apply it anyway —
// this just makes the toast→adjust flow responsive.
fetchLogRef.current?.setMaxFetchRequests(settingsDraft.maxFetchRequests);
const nextRoots = cleanRoots(settingsDraft.roots);
const currentRoots = cleanRoots(inspectorClient.getRoots());
if (JSON.stringify(nextRoots) !== JSON.stringify(currentRoots)) {
void inspectorClient.setRoots(nextRoots).catch(() => {
// setRoots swallows notification failures internally; a throw here
// only means the client is mid-teardown — the persisted roots will
// re-advertise on the next connect, so nothing to surface.
});
}
// Normally the draft is what the user just saved, so it is also what is
// on disk. When this server's last write *rejected* it is not: the draft
// still holds the failed edit, and applying it here would contradict disk
// and undo the rollback that reported the failure — so apply what landed
// instead. `EMPTY_SETTINGS`, not the draft, when nothing has ever landed
// for this server: falling back to the draft would re-apply the very
// values that failed (#2089).
const applied = lastPersistedSettings.lastWriteFailed(
settingsModalTargetId,
)
? (lastPersistedSettings.resolve(settingsModalTargetId) ??
EMPTY_SETTINGS)
: settingsDraft;
applyLiveServerSettings(applied);
}
setSettingsModalTargetId(undefined);
}, [
@ -4303,6 +4467,8 @@ function App() {
settingsModalTargetId,
activeServerId,
settingsDraft,
lastPersistedSettings,
applyLiveServerSettings,
]);
// The Resources screen needs `isSubscribed` to flip the Subscribe button

225
clients/web/src/hooks/useLastPersistedSettings.test.tsx Normal file
View file

@ -0,0 +1,225 @@
import { describe, it, expect } from "vitest";
import type {
InspectorServerSettings,
ServerEntry,
} from "@inspector/core/mcp/types.js";
import { renderWithMantine } from "../test/renderWithMantine";
import {
useLastPersistedSettings,
type LastPersistedSettings,
} from "./useLastPersistedSettings";
const settings = (
overrides: Partial<InspectorServerSettings> = {},
): InspectorServerSettings => ({
headers: [],
env: [],
metadata: [],
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 0,
autoRefreshOnListChanged: false,
paginatedLists: false,
maxFetchRequests: 0,
roots: [],
...overrides,
});
/**
* One `servers` entry, a fresh object per call — most of these tests turn on
* object identity, so a shared fixture would defeat them. A list rebuilt from
* fresh calls is what a successful `GET /api/servers` produces.
*/
const entry = (
id: string,
settingsNode?: InspectorServerSettings,
): ServerEntry => ({
id,
name: id,
config: { type: "stdio", command: "node" },
connection: { status: "disconnected" },
settings: settingsNode,
});
interface Harness {
api: LastPersistedSettings;
/** Deliver a new server list, as a successful list read would. */
setServers: (next: ServerEntry[]) => void;
}
/**
* Hands the hook's API back to the test. The hook holds refs, so it has to be
* read from inside a rendered component; `setServers` re-renders it with a new
* list the way `useServers` does after a read.
*/
function harness(initial: ServerEntry[]): Harness {
let api: LastPersistedSettings | undefined;
function Probe({ servers }: { servers: ServerEntry[] }) {
api = useLastPersistedSettings(servers);
return null;
}
const { rerender } = renderWithMantine(<Probe servers={initial} />);
if (!api) throw new Error("hook did not render");
return {
api,
setServers: (next) => rerender(<Probe servers={next} />),
};
}
describe("useLastPersistedSettings", () => {
it("reads the list entry when no write has landed for that server", () => {
const onDisk = settings({ paginatedLists: true });
const { api } = harness([entry("A", onDisk)]);
expect(api.resolve("A")).toBe(onDisk);
});
it("returns nothing when the list has no settings and nothing landed", () => {
const { api } = harness([entry("A")]);
expect(api.resolve("A")).toBeUndefined();
});
it("returns the recorded write while the entry has not been re-read", () => {
const { api } = harness([entry("A", settings())]);
const written = settings({ paginatedLists: true });
api.begin("A").landed(written);
expect(api.resolve("A")).toBe(written);
});
it("ignores a record made for a different server", () => {
const onDiskB = settings();
const { api } = harness([entry("A"), entry("B", onDiskB)]);
api.begin("A").landed(settings({ paginatedLists: true }));
expect(api.resolve("B")).toBe(onDiskB);
});
it("keeps each server's record, so a save on B doesn't discard A's", () => {
// The settings modal can be opened for any server, connected or not, so
// this sequence is reachable: A's write lands while list reads are failing,
// B is saved from the modal, and only then does a write on A fail. A single
// slot would have lost A's baseline and reproduced #2089.
const { api } = harness([entry("A", settings()), entry("B", settings())]);
const writtenA = settings({ paginatedLists: true });
api.begin("A").landed(writtenA);
api.begin("B").landed(settings({ autoRefreshOnListChanged: true }));
expect(api.resolve("A")).toBe(writtenA);
});
it("stops trusting the record once a fresh list read replaces the entry", () => {
const { api, setServers } = harness([entry("A", settings())]);
api.begin("A").landed(settings({ paginatedLists: true }));
// A successful `GET /api/servers` rebuilds the list, so the entry is a new
// object even when its values are unchanged. That is the signal the list
// has caught up, and it is authoritative again from here.
const fresh = settings();
setServers([entry("A", fresh)]);
expect(api.resolve("A")).toBe(fresh);
});
it("pairs the record with the entry as of completion, not of issue", () => {
// A list read that lands while the write is in flight must not be mistaken
// for one that happened after it. The pairing is taken in `landed`, so the
// read below — which the write outlives — does not invalidate the record.
const { api, setServers } = harness([entry("A", settings())]);
const write = api.begin("A");
setServers([entry("A", settings())]);
const written = settings({ paginatedLists: true });
write.landed(written);
expect(api.resolve("A")).toBe(written);
});
it("keeps only the most recent write for a server", () => {
const { api } = harness([entry("A", settings())]);
api.begin("A").landed(settings({ paginatedLists: true }));
const later = settings({ autoRefreshOnListChanged: true });
api.begin("A").landed(later);
expect(api.resolve("A")).toBe(later);
});
it("lets the later-issued write win however the two finish", () => {
// `updateServerSettings` waits for a list read after its PUT, so an older
// write with a slow read can complete last. Ordering by completion would
// then record a value that a newer write has already replaced on disk, and
// the next failed write would roll back to it.
const { api } = harness([entry("A", settings())]);
const first = api.begin("A");
const second = api.begin("A");
const newest = settings({ paginatedLists: true });
second.landed(newest);
first.landed(settings({ paginatedLists: false }));
expect(api.resolve("A")).toBe(newest);
});
it("reports settled only for the last write issued", () => {
// The flag is what tells a caller it is safe to re-apply the value to the
// UI: while a later write is still in flight, that write will describe disk
// when it reports, and re-applying now would fight it.
const { api } = harness([entry("A", settings())]);
const first = api.begin("A");
expect(first.landed(settings({ paginatedLists: true }))).toBe(true);
const second = api.begin("A");
const third = api.begin("A");
expect(second.landed(settings({ paginatedLists: false }))).toBe(false);
expect(third.landed(settings({ paginatedLists: true }))).toBe(true);
// A superseded straggler is neither recorded nor settled.
expect(second.landed(settings({ paginatedLists: false }))).toBe(false);
});
it("treats an earlier write as settled once the later one has failed", () => {
// The order that leaves the UI wrong: the later write fails first, rolling
// the UI back to a baseline the earlier write then replaces on disk. Once
// the failed write stops counting as in flight, the earlier one settles and
// its caller can re-apply the value.
const { api } = harness([entry("A", settings())]);
const first = api.begin("A");
const second = api.begin("A");
second.failed();
const written = settings({ paginatedLists: true });
expect(first.landed(written)).toBe(true);
expect(api.resolve("A")).toBe(written);
});
it("does not let a straggler resurrect a record a fresh read dropped", () => {
// The high-water mark outlives the record, so an earlier write reporting
// after `resolve` invalidated things cannot reinstate a superseded value.
const { api, setServers } = harness([entry("A", settings())]);
const first = api.begin("A");
api.begin("A").landed(settings({ paginatedLists: true }));
const fresh = settings();
setServers([entry("A", fresh)]);
expect(api.resolve("A")).toBe(fresh);
first.landed(settings({ paginatedLists: false }));
expect(api.resolve("A")).toBe(fresh);
});
it("reports a failed last write per server, ordered by issue", () => {
// The settings modal's draft survives a rejected save, so its owner has to
// know whether what the user last tried to save is on disk. A failure on B
// says nothing about A, and a straggling failure from an older write does
// not overrule a newer write that succeeded.
const { api } = harness([entry("A", settings()), entry("B", settings())]);
expect(api.lastWriteFailed("A")).toBe(false);
const firstA = api.begin("A");
const secondA = api.begin("A");
secondA.landed(settings({ paginatedLists: true }));
firstA.failed();
expect(api.lastWriteFailed("A")).toBe(false);
api.begin("B").failed();
expect(api.lastWriteFailed("B")).toBe(true);
expect(api.lastWriteFailed("A")).toBe(false);
// …and a newer failure on A does count, until a newer write lands.
api.begin("A").failed();
expect(api.lastWriteFailed("A")).toBe(true);
api.begin("A").landed(settings());
expect(api.lastWriteFailed("A")).toBe(false);
});
it("returns nothing when the server has left the list entirely", () => {
const { api, setServers } = harness([entry("A", settings())]);
api.begin("A").landed(settings({ paginatedLists: true }));
setServers([]);
expect(api.resolve("A")).toBeUndefined();
});
});

216
clients/web/src/hooks/useLastPersistedSettings.ts Normal file
View file

@ -0,0 +1,216 @@
import { useCallback, useEffect, useRef } from "react";
import type {
InspectorServerSettings,
ServerEntry,
} from "@inspector/core/mcp/types.js";
/**
* Remembers, per server, the last settings write known to have reached disk, so
* a *later* failed write can be rolled back to what is actually persisted
* rather than to a `servers` entry that has gone stale (#2089).
*
* The `servers` list only advances when a `GET /api/servers` succeeds. Once a
* list read fails, every entry in it keeps describing disk *as of the last
* successful read* — while writes issued since then have changed disk. A
* rollback that reads its baseline from such an entry reverts to a value
* nothing on disk holds, and the UI contradicts the file until the next
* successful read. The reproduction is two toggles: the first write lands but
* its list reload fails, the second write fails outright and reverts to the
* pre-first-toggle value.
*
* Records are kept **per server id**, not in a single slot. The settings modal
* can be opened for any server, independently of which one is connected, so a
* single slot would let a save on B discard the baseline a later failed write
* on A still needs — reproducing the very bug this hook exists to prevent.
*
* ## Which of two overlapping writes wins
*
* A write is announced with `begin` when it is *issued* and confirmed with
* `landed` when it completes, and a confirmation is ignored if a later-issued
* write for that server has already confirmed. Completion order cannot be used
* for this: `updateServerSettings` waits for a list read after its PUT, so an
* older write with a slow read can finish last and would otherwise overwrite
* the record with a value that is no longer on disk.
*
* Issue order is a proxy for disk order, not a proof of it — HTTP does not
* promise that two in-flight PUTs arrive in the order they were sent. It is the
* best signal available on this side: the backend serializes writes to the
* catalog, but neither the route nor `useServers` returns a revision the client
* could order by. If one is added later, this is where it replaces the counter.
*
* ## When a record stops being believed
*
* A record is paired with the `servers` entry for that server **as of when the
* write completed**, and is dropped once the list carries a different entry
* object: a successful read rebuilds the list, so a fresh object is the signal
* that the entry has caught up with — or been overtaken by (an edit made
* outside the Inspector) — our write, and from there the entry is authoritative
* again. The pairing is taken at completion rather than at issue because a read
* that lands *while the write is in flight* would otherwise be mistaken for one
* that happened after it.
*
* One case the client cannot decide is left: a background read whose response
* was generated before the write landed but which arrives after it still looks
* newer here, and the record is dropped in its favour. That needs the same
* missing revision as above. What the fallback yields in that case is the
* freshest successful read — the same value the rest of the UI renders from —
* rather than an arbitrary one.
*
* The list and the records are held in refs rather than state deliberately:
* nothing renders from either, they are read only inside callbacks at the
* moment a write is issued, completes, or fails, and holding them in state
* would re-render the whole composition root on every settings save.
*/
export interface LastPersistedSettings {
/**
* Announce a settings write for `serverId` at the moment it is **issued**.
* Confirm it with the returned handle once it completes.
*/
begin: (serverId: string) => SettingsWrite;
/**
* The best available answer to "what is on disk for this server", **as of the
* moment it is called**. Returns the recorded write while the list still
* carries the entry that write was paired with, otherwise that server's
* current entry settings, and `undefined` when the list has no such entry (or
* one carrying no settings) and no write has landed for it this session.
*
* Both the record and the list are read live, so calling this in a failure
* handler answers for the state at failure time. Do not hoist the result into
* a variable at the point the write is issued and reuse it on rejection: an
* overlapping write that landed in between would make it stale, which is the
* same class of bug as reading the baseline from a stale `servers` entry.
*/
resolve: (serverId: string) => InspectorServerSettings | undefined;
/**
* Whether the most recently **issued** write that has reported for this
* server ended in failure — i.e. whatever the user last tried to save is not
* what disk holds. A caller holding an edit buffer for that server (the
* settings modal's draft) uses it to know its buffer is unpersisted, and to
* apply `resolve` instead of the buffer.
*
* Ordered by issue, not by arrival, for the same reason the record is: a
* straggling failure from an older write does not overwrite the outcome of a
* newer one, and outcomes are kept per server so a failure on B says nothing
* about A.
*/
lastWriteFailed: (serverId: string) => boolean;
}
export interface SettingsWrite {
/**
* Record that this write reached disk. A no-op if a write issued after this
* one has already been confirmed for the same server.
*
* Returns whether this write is now the **settled** state for that server —
* recorded, with no later write still in flight to describe disk after it.
* Callers that own UI derived from the setting use it to re-apply the value:
* an overlapping write that failed *first* will have rolled that UI back to a
* baseline this write has since replaced, and if the list read behind this
* one failed too, nothing else would ever correct it.
*/
landed: (written: InspectorServerSettings) => boolean;
/**
* Report that this write is over without reaching disk, so it stops counting
* as in flight. An earlier write that lands afterwards is then the settled
* state — which is exactly the order that leaves the UI on a rolled-back
* value nothing on disk holds.
*/
failed: () => void;
}
interface WriteRecord {
written: InspectorServerSettings;
entry: ServerEntry | undefined;
}
/**
* @param servers the current server list, as rendered
*/
export function useLastPersistedSettings(
servers: ServerEntry[],
): LastPersistedSettings {
const recordsRef = useRef<Map<string, WriteRecord>>(new Map());
const nextSequenceRef = useRef(0);
// Highest issue order confirmed per server. Kept apart from the records so a
// record dropped by `resolve` can't be resurrected by a straggler that was
// issued earlier: the high-water mark outlives the record it produced.
const confirmedRef = useRef<Map<string, number>>(new Map());
// Issue orders still in flight per server, so a write that lands can tell
// whether it is the last word or whether something later will still report.
// A write leaves this set through `landed` or `failed` alike.
const pendingRef = useRef<Map<string, Set<number>>>(new Map());
// Outcome of the latest-issued write that has reported, per server. Kept
// apart from the record because a *failed* write leaves no record but is
// still the answer to "is the user's last attempt on disk?".
const outcomesRef = useRef<Map<string, { sequence: number; ok: boolean }>>(
new Map(),
);
// Read the list through a ref so a write pairs with the entry as it stands
// when it finishes, not the one captured when it was issued. Mirrored in an
// effect rather than assigned during render: writing a ref mid-render is an
// error under `react-hooks/refs`, and every reader runs from a settled
// promise or an event handler, long after the commit.
const serversRef = useRef(servers);
useEffect(() => {
serversRef.current = servers;
}, [servers]);
const begin = useCallback((serverId: string): SettingsWrite => {
const sequence = ++nextSequenceRef.current;
const pending = pendingRef.current.get(serverId) ?? new Set<number>();
pending.add(sequence);
pendingRef.current.set(serverId, pending);
const settle = (ok: boolean) => {
pending.delete(sequence);
// Keep the outcome of the latest-issued write that has reported, so an
// older straggler cannot overwrite a newer one's verdict.
const previous = outcomesRef.current.get(serverId);
if (!previous || previous.sequence <= sequence) {
outcomesRef.current.set(serverId, { sequence, ok });
}
};
return {
landed: (written: InspectorServerSettings) => {
settle(true);
const confirmed = confirmedRef.current.get(serverId) ?? 0;
// A write issued after this one already reported; it describes disk
// more recently, whichever of the two happened to finish first.
if (confirmed > sequence) return false;
confirmedRef.current.set(serverId, sequence);
const entry = serversRef.current.find((s) => s.id === serverId);
recordsRef.current.set(serverId, { written, entry });
// Settled only while nothing later is still in flight — such a write
// will describe disk once it reports, so re-applying this value in the
// meantime would fight it.
for (const other of pending) if (other > sequence) return false;
return true;
},
failed: () => settle(false),
};
}, []);
const lastWriteFailed = useCallback(
(serverId: string) => outcomesRef.current.get(serverId)?.ok === false,
[],
);
const resolve = useCallback(
(serverId: string): InspectorServerSettings | undefined => {
const entry = serversRef.current.find((s) => s.id === serverId);
const current = recordsRef.current.get(serverId);
if (!current) return entry?.settings;
// Identity, not deep equality: the question is whether the list has been
// re-read since the write, not whether the values happen to match.
if (current.entry !== entry) {
// Superseded — drop it rather than re-checking a dead record on every
// later write for this server.
recordsRef.current.delete(serverId);
return entry?.settings;
}
return current.written;
},
[],
);
return { begin, resolve, lastWriteFailed };
}