From 43d32553905c3d232a09a342ff1656081858bd97 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sun, 26 Jul 2026 01:36:36 +0200 Subject: [PATCH 1/5] refactor(gui): react-doctor cleanup for App, Logs, and Subagents Harden fetch status checks, loading/busy guards, and extract Logs tab keydown helper while keeping viewMode wiring. --- gui/src/App.tsx | 63 +++++++++++++++---------------- gui/src/pages/Logs.tsx | 20 +++------- gui/src/pages/Subagents.tsx | 25 ++++++++---- gui/src/pages/logs-tab-keydown.ts | 23 +++++++++++ 4 files changed, 76 insertions(+), 55 deletions(-) create mode 100644 gui/src/pages/logs-tab-keydown.ts diff --git a/gui/src/App.tsx b/gui/src/App.tsx index cb0ff4195..f792ccdf2 100644 --- a/gui/src/App.tsx +++ b/gui/src/App.tsx @@ -1,4 +1,5 @@ -import { useEffect, useRef, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; +import { setClientResourceData, useKeyedClientResource } from "./client-resource"; import Dashboard from "./pages/Dashboard"; import Providers from "./pages/Providers"; import Models from "./pages/Models"; @@ -16,6 +17,7 @@ import { IconGrid, IconServer, IconBoxes, IconBot, IconList, IconActivity, IconH import { useI18n, useT, LOCALES, type Locale, type TKey } from "./i18n"; import { Select, Switch } from "./ui"; import { installApiAuthFetch } from "./api"; +import { readJsonIfOk } from "./fetch-json"; import { type Page } from "./app-routing"; import { useAppRouteState } from "./use-app-route-state"; @@ -71,7 +73,6 @@ function readStoredTheme(): Theme { export default function App() { const { page, navigateToPage } = useAppRouteState(); const [theme, setTheme] = useState(readStoredTheme); - const [runtimeVersion, setRuntimeVersion] = useState(null); const { locale, setLocale } = useI18n(); const t = useT(); @@ -98,30 +99,35 @@ export default function App() { else { el.setAttribute("data-theme", theme); localStorage.setItem(THEME_KEY, theme); } }, [theme]); - useEffect(() => { - let cancelled = false; - const fetchRuntimeVersion = async () => { - try { - const res = await fetch(`${API_BASE}/healthz`); - if (!res.ok) return; - const version = readRuntimeVersion(await res.json()); - if (!cancelled && version) setRuntimeVersion(version); - } catch { - // Keep the build-time fallback when the proxy is unavailable. - } - }; - fetchRuntimeVersion(); - const interval = setInterval(fetchRuntimeVersion, 30000); - return () => { cancelled = true; clearInterval(interval); }; - }, []); + const healthPoll = useKeyedClientResource( + `app-healthz:${API_BASE}`, + [], + async (signal) => { + const res = await fetch(`${API_BASE}/healthz`, { signal }); + if (!res.ok) return null; + return readRuntimeVersion(await res.json()); + }, + { pollMs: 30_000 }, + ); const cycleTheme = () => setTheme(t => (t === "light" ? "dark" : t === "dark" ? "system" : "light")); const ThemeIcon = THEME_ICON[theme]; - const displayedVersion = runtimeVersion ?? __APP_VERSION__; + const displayedVersion: string = healthPoll.data ?? __APP_VERSION__; const [stopping, setStopping] = useState(false); // Claude navigation row also owns the connection toggle. - const [claudeEnabled, setClaudeEnabled] = useState(null); + const fetchClaudeEnabled = useCallback(async (signal: AbortSignal) => { + const res = await fetch(`${API_BASE}/api/claude-code`, { signal }); + const d = await readJsonIfOk<{ enabled?: unknown }>(res); + return d && typeof d.enabled === "boolean" ? d.enabled : null; + }, []); + + const claudePoll = useKeyedClientResource( + `app-claude-code:${API_BASE}`, + [], + fetchClaudeEnabled, + ); + const claudeEnabled = claudePoll.data ?? null; useEffect(() => { if (!navOpen) return; @@ -151,28 +157,19 @@ export default function App() { return () => mq.removeEventListener("change", onChange); }, []); - useEffect(() => { - let cancelled = false; - fetch(`${API_BASE}/api/claude-code`) - .then(res => res.json()) - .then(d => { if (!cancelled && typeof d.enabled === "boolean") setClaudeEnabled(d.enabled); }) - .catch(() => { /* toggle stays hidden until the API answers */ }); - return () => { cancelled = true; }; - }, []); - const toggleClaude = async () => { if (claudeEnabled === null) return; const next = !claudeEnabled; - setClaudeEnabled(next); // optimistic + setClientResourceData(`app-claude-code:${API_BASE}`, next); try { const res = await fetch(`${API_BASE}/api/claude-code`, { method: "PUT", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ enabled: next }), }); - if (!res.ok) setClaudeEnabled(!next); + if (!res.ok) setClientResourceData(`app-claude-code:${API_BASE}`, !next); } catch { - setClaudeEnabled(!next); + setClientResourceData(`app-claude-code:${API_BASE}`, !next); } }; const handleStop = async () => { @@ -222,7 +219,7 @@ export default function App() { */} {NAV.map(({ id, tkey, Icon }) => (
- @@ -358,7 +348,7 @@ export default function Logs({ apiBase }: { apiBase: string }) { tabIndex={tab === "debug" ? 0 : -1} className={`page-tab${tab === "debug" ? " page-tab--active" : ""}`} onClick={() => selectTab("debug")} - onKeyDown={onTabKeyDown} + onKeyDown={logsTabKeyDown} > {t("logs.tabDebug")} diff --git a/gui/src/pages/Subagents.tsx b/gui/src/pages/Subagents.tsx index dc9262652..a3cad19ff 100644 --- a/gui/src/pages/Subagents.tsx +++ b/gui/src/pages/Subagents.tsx @@ -1,4 +1,5 @@ import { useCallback, useEffect, useMemo, useState } from "react"; +import { readJsonOrThrow } from "../fetch-json"; import { Notice, EmptyState } from "../ui"; import { IconArrowUp, IconArrowDown, IconX, IconCheck, IconSearch, IconBot, IconInfo } from "../icons"; import { useT } from "../i18n/shared"; @@ -13,12 +14,14 @@ export default function Subagents({ apiBase }: { apiBase: string }) { const [status, setStatus] = useState(""); const [ok, setOk] = useState(false); const [loading, setLoading] = useState(true); + const [busy, setBusy] = useState(false); const chosenSet = useMemo(() => new Set(chosen), [chosen]); const load = useCallback(async () => { try { - const r = await fetch(`${apiBase}/api/subagent-models`).then(res => res.json()); + const res = await fetch(`${apiBase}/api/subagent-models`); + const r = await readJsonOrThrow<{ available?: string[]; chosen?: string[] }>(res, t("sub.loadFail")); const avail: string[] = r.available ?? []; const availSet = new Set(avail); setAvailable(avail); @@ -52,6 +55,8 @@ export default function Subagents({ apiBase }: { apiBase: string }) { }; const save = async () => { + if (busy) return; + setBusy(true); setStatus(""); try { const r = await fetch(`${apiBase}/api/subagent-models`, { @@ -59,14 +64,20 @@ export default function Subagents({ apiBase }: { apiBase: string }) { headers: { "Content-Type": "application/json" }, body: JSON.stringify({ models: chosen }), }); - const d = await r.json(); - setOk(r.ok); - setStatus(r.ok - ? t("sub.saved", { n: d.applied?.length ?? 0, cmd: "ocx sync" }) - : (d.error || t("sub.saveFailed"))); + if (!r.ok) { + const d = await r.json().catch(() => ({})) as { error?: string; applied?: string[] }; + setOk(false); + setStatus(d.error || t("sub.saveFailed")); + return; + } + const d = await r.json() as { applied?: string[] }; + setOk(true); + setStatus(t("sub.saved", { n: d.applied?.length ?? 0, cmd: "ocx sync" })); } catch { setOk(false); setStatus(t("sub.networkError")); + } finally { + setBusy(false); } }; @@ -114,7 +125,7 @@ export default function Subagents({ apiBase }: { apiBase: string }) { )}
- +
{t("sub.models")} {filtered.length}
diff --git a/gui/src/pages/logs-tab-keydown.ts b/gui/src/pages/logs-tab-keydown.ts new file mode 100644 index 000000000..c39d22eb2 --- /dev/null +++ b/gui/src/pages/logs-tab-keydown.ts @@ -0,0 +1,23 @@ +import type { KeyboardEvent } from "react"; + +export type LogsTab = "logs" | "debug"; + +export function readTabFromHash(): LogsTab { + return window.location.hash.replace(/^#\/?/, "") === "logs/debug" ? "debug" : "logs"; +} + +export function selectLogsTab(next: LogsTab) { + window.location.hash = next === "debug" ? "logs/debug" : "logs"; +} + +export function logsTabKeyDown(e: KeyboardEvent) { + if (e.key === "ArrowLeft" || e.key === "Home") { + e.preventDefault(); + selectLogsTab("logs"); + document.getElementById("logs-tab-logs")?.focus(); + } else if (e.key === "ArrowRight" || e.key === "End") { + e.preventDefault(); + selectLogsTab("debug"); + document.getElementById("logs-tab-debug")?.focus(); + } +} From e6649563df84717f6be56a5809ec9bcef07e5ba1 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sun, 26 Jul 2026 03:50:34 +0200 Subject: [PATCH 2/5] fix(gui): harden Subagents empty-body load handling after rebase Guard readJsonOrThrow undefined bodies and align the classic Subagents fetch mock with text()-based status checks. --- gui/src/pages/Subagents.tsx | 1 + gui/tests/subagents-classic.test.tsx | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/gui/src/pages/Subagents.tsx b/gui/src/pages/Subagents.tsx index a3cad19ff..79fd602bc 100644 --- a/gui/src/pages/Subagents.tsx +++ b/gui/src/pages/Subagents.tsx @@ -22,6 +22,7 @@ export default function Subagents({ apiBase }: { apiBase: string }) { try { const res = await fetch(`${apiBase}/api/subagent-models`); const r = await readJsonOrThrow<{ available?: string[]; chosen?: string[] }>(res, t("sub.loadFail")); + if (!r) throw new Error(t("sub.loadFail")); const avail: string[] = r.available ?? []; const availSet = new Set(avail); setAvailable(avail); diff --git a/gui/tests/subagents-classic.test.tsx b/gui/tests/subagents-classic.test.tsx index b1cc0025d..795a3467a 100644 --- a/gui/tests/subagents-classic.test.tsx +++ b/gui/tests/subagents-classic.test.tsx @@ -40,7 +40,13 @@ beforeEach(() => { configurable: true, value: async (url: string, init?: RequestInit) => { requests.push({ url: String(url), init }); - return { ok: true, json: async () => ({ available, chosen }) } as unknown as Response; + const body = JSON.stringify({ available, chosen }); + return { + ok: true, + status: 200, + text: async () => body, + json: async () => ({ available, chosen }), + } as unknown as Response; }, }); From 7dad03ca02b02663802d356c1b8305f267ea32eb Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sun, 26 Jul 2026 04:28:13 +0200 Subject: [PATCH 3/5] fix(gui): address CodeRabbit findings on Claude toggle, Subagents busy, logs keydown tests Serialize Claude toggle PUTs with an in-flight guard, disable Subagents mutations while saving, and cover logs tab keyboard helpers. --- gui/src/App.tsx | 16 ++++- gui/src/pages/Subagents.tsx | 13 ++-- gui/tests/logs-tab-keydown.test.ts | 105 +++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 7 deletions(-) create mode 100644 gui/tests/logs-tab-keydown.test.ts diff --git a/gui/src/App.tsx b/gui/src/App.tsx index f792ccdf2..9ee347031 100644 --- a/gui/src/App.tsx +++ b/gui/src/App.tsx @@ -128,6 +128,8 @@ export default function App() { fetchClaudeEnabled, ); const claudeEnabled = claudePoll.data ?? null; + const claudeToggleInFlight = useRef(false); + const [claudeTogglePending, setClaudeTogglePending] = useState(false); useEffect(() => { if (!navOpen) return; @@ -158,7 +160,9 @@ export default function App() { }, []); const toggleClaude = async () => { - if (claudeEnabled === null) return; + if (claudeEnabled === null || claudeToggleInFlight.current) return; + claudeToggleInFlight.current = true; + setClaudeTogglePending(true); const next = !claudeEnabled; setClientResourceData(`app-claude-code:${API_BASE}`, next); try { @@ -170,6 +174,9 @@ export default function App() { if (!res.ok) setClientResourceData(`app-claude-code:${API_BASE}`, !next); } catch { setClientResourceData(`app-claude-code:${API_BASE}`, !next); + } finally { + claudeToggleInFlight.current = false; + setClaudeTogglePending(false); } }; const handleStop = async () => { @@ -229,7 +236,12 @@ export default function App() { {t(tkey)} {id === "claude" && claudeEnabled !== null && ( - void toggleClaude()} label={t("claude.toggleAria")} /> + void toggleClaude()} + disabled={claudeTogglePending} + label={t("claude.toggleAria")} + /> )}
))} diff --git a/gui/src/pages/Subagents.tsx b/gui/src/pages/Subagents.tsx index 79fd602bc..288068bf3 100644 --- a/gui/src/pages/Subagents.tsx +++ b/gui/src/pages/Subagents.tsx @@ -42,10 +42,12 @@ export default function Subagents({ apiBase }: { apiBase: string }) { }, [load]); const toggle = (m: string) => { + if (busy) return; setStatus(""); setChosen(prev => prev.includes(m) ? prev.filter(x => x !== m) : (prev.length >= 5 ? prev : [...prev, m])); }; const move = (i: number, dir: -1 | 1) => { + if (busy) return; setChosen(prev => { const next = [...prev]; const j = i + dir; @@ -72,6 +74,7 @@ export default function Subagents({ apiBase }: { apiBase: string }) { return; } const d = await r.json() as { applied?: string[] }; + if (d.applied) setChosen(d.applied); setOk(true); setStatus(t("sub.saved", { n: d.applied?.length ?? 0, cmd: "ocx sync" })); } catch { @@ -111,13 +114,13 @@ export default function Subagents({ apiBase }: { apiBase: string }) {
{i + 1} {modelLabel(m)} - - -
@@ -151,9 +154,9 @@ export default function Subagents({ apiBase }: { apiBase: string }) { type="button" className={`card row${sel ? " panel-accent" : ""}`} onClick={() => toggle(m)} - disabled={full} + disabled={busy || full} aria-pressed={sel} - style={{ width: "100%", opacity: full ? 0.45 : 1, cursor: full ? "not-allowed" : "pointer" }} + style={{ width: "100%", opacity: full || busy ? 0.45 : 1, cursor: full || busy ? "not-allowed" : "pointer" }} > {sel && } diff --git a/gui/tests/logs-tab-keydown.test.ts b/gui/tests/logs-tab-keydown.test.ts new file mode 100644 index 000000000..848af2f51 --- /dev/null +++ b/gui/tests/logs-tab-keydown.test.ts @@ -0,0 +1,105 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { Window } from "happy-dom"; +import type { KeyboardEvent as ReactKeyboardEvent } from "react"; +import { logsTabKeyDown, readTabFromHash, selectLogsTab } from "../src/pages/logs-tab-keydown"; + +const globals = ["document", "window", "navigator"] as const; +let previousGlobals: Record<(typeof globals)[number], unknown>; +let testWindow: Window; + +beforeEach(() => { + previousGlobals = Object.fromEntries(globals.map((k) => [k, Reflect.get(globalThis, k)])) as typeof previousGlobals; + testWindow = new Window({ url: "http://localhost/#logs" }); + Object.defineProperties(globalThis, { + document: { configurable: true, value: testWindow.document }, + window: { configurable: true, value: testWindow }, + navigator: { configurable: true, value: testWindow.navigator }, + }); +}); + +afterEach(() => { + testWindow.close(); + for (const key of globals) { + Object.defineProperty(globalThis, key, { configurable: true, value: previousGlobals[key] }); + } +}); + +function keyEvent(key: string): ReactKeyboardEvent & { prevented: boolean } { + let prevented = false; + const e = { + key, + preventDefault() { prevented = true; }, + get prevented() { return prevented; }, + }; + return e as unknown as ReactKeyboardEvent & { prevented: boolean }; +} + +function mountTabs() { + const logs = testWindow.document.createElement("button"); + logs.id = "logs-tab-logs"; + const debug = testWindow.document.createElement("button"); + debug.id = "logs-tab-debug"; + testWindow.document.body.append(logs, debug); + return { logs, debug }; +} + +test("readTabFromHash maps logs and logs/debug hashes", () => { + testWindow.location.hash = "#logs"; + expect(readTabFromHash()).toBe("logs"); + + testWindow.location.hash = "#logs/debug"; + expect(readTabFromHash()).toBe("debug"); + + testWindow.location.hash = "#/logs/debug"; + expect(readTabFromHash()).toBe("debug"); + + testWindow.location.hash = "#logs/other"; + expect(readTabFromHash()).toBe("logs"); +}); + +test("selectLogsTab writes the matching hash", () => { + selectLogsTab("debug"); + expect(testWindow.location.hash).toBe("#logs/debug"); + + selectLogsTab("logs"); + expect(testWindow.location.hash).toBe("#logs"); +}); + +test("ArrowLeft and Home select logs and focus the logs tab", () => { + const { logs, debug } = mountTabs(); + debug.focus(); + + for (const key of ["ArrowLeft", "Home"] as const) { + testWindow.location.hash = "#logs/debug"; + debug.focus(); + const e = keyEvent(key); + logsTabKeyDown(e); + expect(e.prevented).toBe(true); + expect(testWindow.location.hash).toBe("#logs"); + expect(testWindow.document.activeElement).toBe(logs); + } +}); + +test("ArrowRight and End select debug and focus the debug tab", () => { + const { logs, debug } = mountTabs(); + logs.focus(); + + for (const key of ["ArrowRight", "End"] as const) { + testWindow.location.hash = "#logs"; + logs.focus(); + const e = keyEvent(key); + logsTabKeyDown(e); + expect(e.prevented).toBe(true); + expect(testWindow.location.hash).toBe("#logs/debug"); + expect(testWindow.document.activeElement).toBe(debug); + } +}); + +test("unrelated keys do not navigate or preventDefault", () => { + mountTabs(); + testWindow.location.hash = "#logs"; + const e = keyEvent("ArrowDown"); + logsTabKeyDown(e); + expect(e.prevented).toBe(false); + expect(testWindow.location.hash).toBe("#logs"); +}); From 3c75294bb2b845c67147cb4722e89cad98acd450 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sun, 26 Jul 2026 04:49:13 +0200 Subject: [PATCH 4/5] test(gui): cover Claude toggle and Subagents busy races Add focused regressions for serialized Claude toggle PUTs and Subagents edit-blocking plus applied reconciliation while a save is in flight. --- gui/tests/claude-toggle-race.test.tsx | 179 +++++++++++++++++++++++++ gui/tests/subagents-busy-race.test.tsx | 175 ++++++++++++++++++++++++ 2 files changed, 354 insertions(+) create mode 100644 gui/tests/claude-toggle-race.test.tsx create mode 100644 gui/tests/subagents-busy-race.test.tsx diff --git a/gui/tests/claude-toggle-race.test.tsx b/gui/tests/claude-toggle-race.test.tsx new file mode 100644 index 000000000..ea4f89675 --- /dev/null +++ b/gui/tests/claude-toggle-race.test.tsx @@ -0,0 +1,179 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { Window } from "happy-dom"; +import { act } from "react"; +import type { Root } from "react-dom/client"; + +/** + * Rapid clicks on the sidebar Claude switch must serialize to a single in-flight PUT + * (App.tsx claudeToggleInFlight + disabled while pending). + */ + +const globals = [ + "document", + "window", + "navigator", + "localStorage", + "sessionStorage", + "fetch", + "IS_REACT_ACT_ENVIRONMENT", +] as const; + +let previousGlobals: Record<(typeof globals)[number], unknown>; +let testWindow: Window; +let container: HTMLElement; +let root: Root | null = null; +let putBodies: unknown[] = []; +let releasePut: (() => void) | null = null; +let putGate: Promise | null = null; + +function jsonResponse(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { "content-type": "application/json" }, + }); +} + +beforeEach(() => { + previousGlobals = Object.fromEntries(globals.map((k) => [k, Reflect.get(globalThis, k)])) as typeof previousGlobals; + testWindow = new Window({ url: "http://localhost/#api" }); + Object.defineProperty(testWindow.navigator, "language", { configurable: true, value: "en-US" }); + Object.defineProperties(globalThis, { + document: { configurable: true, value: testWindow.document }, + window: { configurable: true, value: testWindow }, + navigator: { configurable: true, value: testWindow.navigator }, + localStorage: { configurable: true, value: testWindow.localStorage }, + sessionStorage: { configurable: true, value: testWindow.sessionStorage }, + }); + (globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + (globalThis as Record).__APP_VERSION__ = "0.0.0-test"; + + putBodies = []; + putGate = new Promise((resolve) => { + releasePut = resolve; + }); + + const mockFetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const url = String(input instanceof Request ? input.url : input); + const method = (init?.method ?? (input instanceof Request ? input.method : "GET")).toUpperCase(); + + if (url.includes("/api/claude-code") && method === "PUT") { + putBodies.push(JSON.parse(String(init?.body ?? "{}"))); + await putGate; + return jsonResponse({ enabled: true }); + } + if (url.includes("/api/claude-code")) { + return jsonResponse({ enabled: false }); + } + if (url.includes("/healthz")) { + return jsonResponse({ status: "ok", version: "0.0.0-test", uptime: 1 }); + } + return jsonResponse({}); + }) as typeof fetch; + + Object.defineProperty(globalThis, "fetch", { configurable: true, value: mockFetch }); + Object.defineProperty(testWindow, "fetch", { configurable: true, value: mockFetch }); + + container = testWindow.document.createElement("div") as unknown as HTMLElement; + testWindow.document.body.appendChild(container as never); +}); + +afterEach(async () => { + if (root) { + const current = root; + await act(async () => { + current.unmount(); + }); + root = null; + } + releasePut?.(); + releasePut = null; + putGate = null; + testWindow.close(); + for (const key of globals) { + Object.defineProperty(globalThis, key, { configurable: true, value: previousGlobals[key] }); + } +}); + +async function waitFor(predicate: () => boolean, timeoutMs = 2000): Promise { + const start = Date.now(); + while (!predicate()) { + if (Date.now() - start > timeoutMs) throw new Error("waitFor timed out"); + await act(async () => { + await new Promise((resolve) => testWindow.setTimeout(resolve, 10)); + }); + } +} + +function claudeSwitch(): HTMLButtonElement { + const btn = Array.from(container.querySelectorAll("button")).find( + (b) => b.getAttribute("aria-label") === "Toggle Claude connection", + ); + if (!btn) throw new Error("Claude toggle switch not found"); + return btn as unknown as HTMLButtonElement; +} + +test("rapid Claude toggle clicks issue only one PUT until the first settles", async () => { + const { resetApiAuthFetchForTests, installApiAuthFetch } = await import("../src/api"); + resetApiAuthFetchForTests(); + installApiAuthFetch(); + Object.defineProperty(globalThis, "fetch", { configurable: true, value: window.fetch }); + + const [{ createRoot }, { LanguageProvider }, { default: App }] = await Promise.all([ + import("react-dom/client"), + import("../src/i18n/provider"), + import("../src/App"), + ]); + + await act(async () => { + root = createRoot(container); + root.render( + + + , + ); + }); + + await waitFor(() => { + try { + return !!claudeSwitch(); + } catch { + return false; + } + }); + + const sw = claudeSwitch(); + expect(sw.disabled).toBe(false); + expect(sw.getAttribute("aria-pressed")).toBe("false"); + + await act(async () => { + sw.click(); + sw.click(); + sw.click(); + }); + + expect(putBodies).toEqual([{ enabled: true }]); + expect(claudeSwitch().disabled).toBe(true); + + await act(async () => { + releasePut?.(); + releasePut = null; + await Promise.resolve(); + }); + + await waitFor(() => !claudeSwitch().disabled); + + // Re-arm a gated PUT and confirm a later click can fire again. + putGate = new Promise((resolve) => { + releasePut = resolve; + }); + await act(async () => { + claudeSwitch().click(); + }); + expect(putBodies).toEqual([{ enabled: true }, { enabled: false }]); + + await act(async () => { + releasePut?.(); + releasePut = null; + }); + await waitFor(() => !claudeSwitch().disabled); +}); diff --git a/gui/tests/subagents-busy-race.test.tsx b/gui/tests/subagents-busy-race.test.tsx new file mode 100644 index 000000000..170d48978 --- /dev/null +++ b/gui/tests/subagents-busy-race.test.tsx @@ -0,0 +1,175 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { Window } from "happy-dom"; +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import Subagents from "../src/pages/Subagents"; +import { LanguageProvider } from "../src/i18n/provider"; + +/** + * While a Subagents save is in flight, toggle/remove/reorder must be blocked; + * after success, `chosen` reconciles from `d.applied`. + */ + +const globals = ["document", "window", "navigator", "localStorage", "fetch", "IS_REACT_ACT_ENVIRONMENT"] as const; +let previousGlobals: Record<(typeof globals)[number], unknown>; +let testWindow: Window; +let container: HTMLElement; +let root: Root | null = null; +let available: string[] = []; +let chosen: string[] = []; +let putCount = 0; +let releasePut: (() => void) | null = null; +let putGate: Promise | null = null; +let appliedOnSave: string[] = []; + +beforeEach(() => { + previousGlobals = Object.fromEntries(globals.map((k) => [k, Reflect.get(globalThis, k)])) as typeof previousGlobals; + testWindow = new Window({ url: "http://localhost/" }); + Object.defineProperty(testWindow.navigator, "language", { configurable: true, value: "en-US" }); + Object.defineProperties(globalThis, { + document: { configurable: true, value: testWindow.document }, + window: { configurable: true, value: testWindow }, + navigator: { configurable: true, value: testWindow.navigator }, + localStorage: { configurable: true, value: testWindow.localStorage }, + }); + (globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + + available = ["a-1", "a-2", "a-3", "a-4"]; + chosen = ["a-1", "a-2"]; + putCount = 0; + appliedOnSave = ["a-1"]; + putGate = new Promise((resolve) => { + releasePut = resolve; + }); + + Object.defineProperty(globalThis, "fetch", { + configurable: true, + value: async (_url: string, init?: RequestInit) => { + if (init?.method === "PUT") { + putCount += 1; + await putGate; + return { + ok: true, + status: 200, + json: async () => ({ applied: appliedOnSave }), + text: async () => JSON.stringify({ applied: appliedOnSave }), + } as unknown as Response; + } + const body = JSON.stringify({ available, chosen }); + return { + ok: true, + status: 200, + text: async () => body, + json: async () => ({ available, chosen }), + } as unknown as Response; + }, + }); + + container = testWindow.document.createElement("div") as unknown as HTMLElement; + testWindow.document.body.appendChild(container as never); +}); + +afterEach(async () => { + if (root) { + const current = root; + await act(async () => { + current.unmount(); + }); + root = null; + } + releasePut?.(); + releasePut = null; + putGate = null; + for (const key of globals) { + Object.defineProperty(globalThis, key, { configurable: true, value: previousGlobals[key] }); + } +}); + +async function mount() { + await act(async () => { + root = createRoot(container); + root.render( + + + , + ); + }); + await act(async () => { + await new Promise((r) => setTimeout(r, 50)); + }); +} + +function modelRow(id: string): HTMLButtonElement { + const row = Array.from(container.querySelectorAll("button")).find( + (b) => b.hasAttribute("aria-pressed") && (b.textContent ?? "").includes(id), + ); + if (!row) throw new Error(`model row not found: ${id}`); + return row as unknown as HTMLButtonElement; +} + +function removeButtons(): HTMLButtonElement[] { + return Array.from(container.querySelectorAll("button")).filter((b) => + /^Remove /.test(b.getAttribute("aria-label") ?? ""), + ) as unknown as HTMLButtonElement[]; +} + +function moveUp(id: string): HTMLButtonElement { + const btn = Array.from(container.querySelectorAll("button")).find( + (b) => b.getAttribute("aria-label") === `Move ${id} up`, + ); + if (!btn) throw new Error(`move-up not found: ${id}`); + return btn as unknown as HTMLButtonElement; +} + +function saveButton(): HTMLButtonElement { + const btn = Array.from(container.querySelectorAll("button")).find((b) => b.textContent?.trim() === "Save"); + if (!btn) throw new Error("Save button not found"); + return btn as unknown as HTMLButtonElement; +} + +test("blocks edits while save is busy and reconciles chosen from applied", async () => { + await mount(); + + expect(removeButtons().length).toBe(2); + expect(modelRow("a-2").getAttribute("aria-pressed")).toBe("true"); + + await act(async () => { + saveButton().click(); + }); + + expect(putCount).toBe(1); + expect(saveButton().disabled).toBe(true); + expect(removeButtons().every((b) => b.disabled)).toBe(true); + expect(modelRow("a-3").disabled).toBe(true); + expect(moveUp("a-2").disabled).toBe(true); + + // Force clicks past disabled attributes — state guards must still no-op. + await act(async () => { + modelRow("a-3").dispatchEvent(new (globalThis as { window: Window }).window.MouseEvent("click", { bubbles: true })); + removeButtons()[1]!.dispatchEvent(new (globalThis as { window: Window }).window.MouseEvent("click", { bubbles: true })); + moveUp("a-2").dispatchEvent(new (globalThis as { window: Window }).window.MouseEvent("click", { bubbles: true })); + }); + + // Featured list unchanged while the gated PUT is outstanding. + expect(removeButtons().length).toBe(2); + expect(modelRow("a-2").getAttribute("aria-pressed")).toBe("true"); + expect(modelRow("a-3").getAttribute("aria-pressed")).toBe("false"); + expect(putCount).toBe(1); + + await act(async () => { + releasePut?.(); + releasePut = null; + await Promise.resolve(); + }); + + await act(async () => { + await new Promise((r) => setTimeout(r, 20)); + }); + + // Server applied only a-1 — local chosen must reconcile. + expect(removeButtons().length).toBe(1); + expect(removeButtons()[0]!.getAttribute("aria-label")).toBe("Remove a-1"); + expect(modelRow("a-2").getAttribute("aria-pressed")).toBe("false"); + expect(saveButton().disabled).toBe(false); + expect(container.textContent).toContain("1/5"); +}); From c9ba752645fb9b27e7cdadabdad14d07f5ea9821 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sun, 26 Jul 2026 05:02:14 +0200 Subject: [PATCH 5/5] fix(gui): serialize Subagents save with ref guard and readJsonOrThrow Mirror the Claude toggle in-flight pattern so rapid Save clicks cannot race before busy re-renders, and reuse readJsonOrThrow for PUT error/success parsing. --- gui/src/pages/Subagents.tsx | 24 +++++++------- gui/tests/subagents-busy-race.test.tsx | 43 ++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 13 deletions(-) diff --git a/gui/src/pages/Subagents.tsx b/gui/src/pages/Subagents.tsx index 288068bf3..6adcd5e30 100644 --- a/gui/src/pages/Subagents.tsx +++ b/gui/src/pages/Subagents.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { readJsonOrThrow } from "../fetch-json"; import { Notice, EmptyState } from "../ui"; import { IconArrowUp, IconArrowDown, IconX, IconCheck, IconSearch, IconBot, IconInfo } from "../icons"; @@ -15,6 +15,8 @@ export default function Subagents({ apiBase }: { apiBase: string }) { const [ok, setOk] = useState(false); const [loading, setLoading] = useState(true); const [busy, setBusy] = useState(false); + /** Sync guard: state-only `busy` can miss clicks before the disabled re-render commits. */ + const saveInFlight = useRef(false); const chosenSet = useMemo(() => new Set(chosen), [chosen]); @@ -58,7 +60,8 @@ export default function Subagents({ apiBase }: { apiBase: string }) { }; const save = async () => { - if (busy) return; + if (busy || saveInFlight.current) return; + saveInFlight.current = true; setBusy(true); setStatus(""); try { @@ -67,20 +70,15 @@ export default function Subagents({ apiBase }: { apiBase: string }) { headers: { "Content-Type": "application/json" }, body: JSON.stringify({ models: chosen }), }); - if (!r.ok) { - const d = await r.json().catch(() => ({})) as { error?: string; applied?: string[] }; - setOk(false); - setStatus(d.error || t("sub.saveFailed")); - return; - } - const d = await r.json() as { applied?: string[] }; - if (d.applied) setChosen(d.applied); + const d = await readJsonOrThrow<{ applied?: string[] }>(r, t("sub.saveFailed")); + if (d?.applied) setChosen(d.applied); setOk(true); - setStatus(t("sub.saved", { n: d.applied?.length ?? 0, cmd: "ocx sync" })); - } catch { + setStatus(t("sub.saved", { n: d?.applied?.length ?? 0, cmd: "ocx sync" })); + } catch (error) { setOk(false); - setStatus(t("sub.networkError")); + setStatus(error instanceof Error && error.message ? error.message : t("sub.networkError")); } finally { + saveInFlight.current = false; setBusy(false); } }; diff --git a/gui/tests/subagents-busy-race.test.tsx b/gui/tests/subagents-busy-race.test.tsx index 170d48978..9e153f350 100644 --- a/gui/tests/subagents-busy-race.test.tsx +++ b/gui/tests/subagents-busy-race.test.tsx @@ -173,3 +173,46 @@ test("blocks edits while save is busy and reconciles chosen from applied", async expect(saveButton().disabled).toBe(false); expect(container.textContent).toContain("1/5"); }); + +test("rapid Save clicks issue only one PUT until the first settles", async () => { + await mount(); + + await act(async () => { + const btn = saveButton(); + btn.click(); + btn.click(); + btn.click(); + }); + + expect(putCount).toBe(1); + expect(saveButton().disabled).toBe(true); + + await act(async () => { + releasePut?.(); + releasePut = null; + await Promise.resolve(); + }); + await act(async () => { + await new Promise((r) => setTimeout(r, 20)); + }); + expect(saveButton().disabled).toBe(false); + + // Re-arm a gated PUT and confirm a later click can fire again. + putGate = new Promise((resolve) => { + releasePut = resolve; + }); + await act(async () => { + saveButton().click(); + }); + expect(putCount).toBe(2); + + await act(async () => { + releasePut?.(); + releasePut = null; + await Promise.resolve(); + }); + await act(async () => { + await new Promise((r) => setTimeout(r, 20)); + }); + expect(saveButton().disabled).toBe(false); +});