Skip to content

refactor(gui): react-doctor cleanup for App, Logs, Subagents #471

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Wibias merged 5 commits into from
Jul 26, 2026
Merged

refactor(gui): react-doctor cleanup for App, Logs, Subagents #471

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
43d3255
refactor(gui): react-doctor cleanup for App, Logs, and Subagents
Wibias Jul 25, 2026
e664956
fix(gui): harden Subagents empty-body load handling after rebase
Wibias Jul 26, 2026
7dad03c
fix(gui): address CodeRabbit findings on Claude toggle, Subagents bus…
Wibias Jul 26, 2026
3c75294
test(gui): cover Claude toggle and Subagents busy races
Wibias Jul 26, 2026
c9ba752
fix(gui): serialize Subagents save with ref guard and readJsonOrThrow
Wibias Jul 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 44 additions & 35 deletions gui/src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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";

Expand Down Expand Up @@ -71,7 +73,6 @@ function readStoredTheme(): Theme {
export default function App() {
const { page, navigateToPage } = useAppRouteState();
const [theme, setTheme] = useState<Theme>(readStoredTheme);
const [runtimeVersion, setRuntimeVersion] = useState<string | null>(null);
const { locale, setLocale } = useI18n();
const t = useT();

Expand All @@ -98,30 +99,37 @@ 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<boolean | null>(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;
const claudeToggleInFlight = useRef(false);
const [claudeTogglePending, setClaudeTogglePending] = useState(false);

useEffect(() => {
if (!navOpen) return;
Expand Down Expand Up @@ -151,28 +159,24 @@ 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;
if (claudeEnabled === null || claudeToggleInFlight.current) return;
claudeToggleInFlight.current = true;
setClaudeTogglePending(true);
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);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
} finally {
claudeToggleInFlight.current = false;
setClaudeTogglePending(false);
}
};
const handleStop = async () => {
Expand Down Expand Up @@ -222,7 +226,7 @@ export default function App() {
*/}
{NAV.map(({ id, tkey, Icon }) => (
<div key={id} className={`nav-entry${id === "claude" ? ` nav-entry-claude${page === id ? " active" : ""}` : ""}`}>
<button className={`nav-item${page === id ? " active" : ""}`} data-page={id}
<button type="button" className={`nav-item${page === id ? " active" : ""}`} data-page={id}
onClick={() => {
// Deliberate sidebar navigation — push a history entry.
navigateToPage(id);
Expand All @@ -232,7 +236,12 @@ export default function App() {
<Icon /> {t(tkey)}
</button>
{id === "claude" && claudeEnabled !== null && (
<Switch on={claudeEnabled} onClick={() => void toggleClaude()} label={t("claude.toggleAria")} />
<Switch
on={claudeEnabled}
onClick={() => void toggleClaude()}
disabled={claudeTogglePending}
label={t("claude.toggleAria")}
/>
)}
</div>
))}
Expand Down
20 changes: 5 additions & 15 deletions gui/src/pages/Logs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,8 @@ import { modelLabel } from "../model-display";
import { EmptyState, Notice } from "../ui";
import Debug from "./Debug";

type LogsTab = "logs" | "debug";

function readTabFromHash(): LogsTab {
return window.location.hash.replace(/^#\/?/, "") === "logs/debug" ? "debug" : "logs";
}
import type { LogsTab } from "./logs-tab-keydown";
import { logsTabKeyDown, readTabFromHash, selectLogsTab } from "./logs-tab-keydown";

interface UsageBreakdown {
inputTokens: number;
Expand Down Expand Up @@ -268,14 +265,7 @@ export default function Logs({ apiBase }: { apiBase: string }) {
return () => window.removeEventListener("hashchange", onHash);
}, []);

const selectTab = (next: LogsTab) => {
window.location.hash = next === "debug" ? "logs/debug" : "logs";
};

const onTabKeyDown = (e: React.KeyboardEvent) => {
if (e.key === "ArrowLeft" || e.key === "Home") { e.preventDefault(); selectTab("logs"); document.getElementById("logs-tab-logs")?.focus(); }
else if (e.key === "ArrowRight" || e.key === "End") { e.preventDefault(); selectTab("debug"); document.getElementById("logs-tab-debug")?.focus(); }
};
const selectTab = selectLogsTab;

const fetchLogs = useCallback(async (opts?: { silent?: boolean }) => {
const silent = opts?.silent === true;
Expand Down Expand Up @@ -345,7 +335,7 @@ export default function Logs({ apiBase }: { apiBase: string }) {
tabIndex={tab === "logs" ? 0 : -1}
className={`page-tab${tab === "logs" ? " page-tab--active" : ""}`}
onClick={() => selectTab("logs")}
onKeyDown={onTabKeyDown}
onKeyDown={logsTabKeyDown}
>
{t("logs.tabLogs")}
</button>
Expand All @@ -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")}
</button>
Expand Down
43 changes: 28 additions & 15 deletions gui/src/pages/Subagents.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
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";
import { useT } from "../i18n/shared";
Expand All @@ -13,12 +14,17 @@ 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);
/** 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]);

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"));
if (!r) throw new Error(t("sub.loadFail"));
const avail: string[] = r.available ?? [];
const availSet = new Set(avail);
setAvailable(avail);
Expand All @@ -38,10 +44,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;
Expand All @@ -52,21 +60,26 @@ export default function Subagents({ apiBase }: { apiBase: string }) {
};

const save = async () => {
if (busy || saveInFlight.current) return;
saveInFlight.current = true;
setBusy(true);
setStatus("");
try {
const r = await fetch(`${apiBase}/api/subagent-models`, {
method: "PUT",
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")));
} catch {
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 (error) {
setOk(false);
setStatus(t("sub.networkError"));
setStatus(error instanceof Error && error.message ? error.message : t("sub.networkError"));
} finally {
saveInFlight.current = false;
setBusy(false);
}
};

Expand Down Expand Up @@ -99,13 +112,13 @@ export default function Subagents({ apiBase }: { apiBase: string }) {
<div key={m} className="card panel-accent row" style={{ padding: "8px 12px", gap: 10 }}>
<span className="mono font-bold" style={{ width: 18, color: "var(--accent)" }}>{i + 1}</span>
<code className="mono" style={{ flex: 1, color: "var(--text)" }}>{modelLabel(m)}</code>
<button type="button" className="btn btn-ghost btn-icon btn-sm" onClick={() => move(i, -1)} disabled={i === 0} aria-label={t("sub.moveUp", { m })}>
<button type="button" className="btn btn-ghost btn-icon btn-sm" onClick={() => move(i, -1)} disabled={busy || i === 0} aria-label={t("sub.moveUp", { m })}>
<IconArrowUp />
</button>
<button type="button" className="btn btn-ghost btn-icon btn-sm" onClick={() => move(i, 1)} disabled={i === chosen.length - 1} aria-label={t("sub.moveDown", { m })}>
<button type="button" className="btn btn-ghost btn-icon btn-sm" onClick={() => move(i, 1)} disabled={busy || i === chosen.length - 1} aria-label={t("sub.moveDown", { m })}>
<IconArrowDown />
</button>
<button type="button" className="btn btn-ghost btn-icon btn-sm" onClick={() => toggle(m)} aria-label={t("sub.removeAria", { m })} style={{ color: "var(--red)" }}>
<button type="button" className="btn btn-ghost btn-icon btn-sm" onClick={() => toggle(m)} disabled={busy} aria-label={t("sub.removeAria", { m })} style={{ color: "var(--red)" }}>
<IconX />
</button>
</div>
Expand All @@ -114,7 +127,7 @@ export default function Subagents({ apiBase }: { apiBase: string }) {
)}

<div style={{ marginTop: 14 }}>
<button type="button" className="btn btn-primary" onClick={save}>{t("common.save")}</button>
<button type="button" className="btn btn-primary" onClick={() => void save()} disabled={busy}>{t("common.save")}</button>
</div>

<div className="h-section">{t("sub.models")} <span className="count">{filtered.length}</span></div>
Expand All @@ -139,9 +152,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" }}
>
<span style={{ width: 16, height: 16, flexShrink: 0, color: "var(--accent)", display: "inline-flex" }}>
{sel && <IconCheck style={{ width: 16, height: 16 }} />}
Expand Down
23 changes: 23 additions & 0 deletions gui/src/pages/logs-tab-keydown.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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();
}
}
Comment on lines +13 to +23

@coderabbitai coderabbitai Bot Jul 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Cover the extracted keyboard contract with tests.

Add focused coverage for ArrowLeft/ArrowRight/Home/End, unrelated keys, hash parsing, and focus targets. The PR notes that Logs smoke tests are still pending, and this helper now owns the tab-navigation behavior.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gui/src/pages/logs-tab-keydown.ts` around lines 13 - 23, Add focused tests
for logsTabKeyDown covering ArrowLeft, ArrowRight, Home, End, and unrelated
keys, including preventDefault behavior, selectLogsTab calls, and the
corresponding logs tab focus targets. Also cover hash parsing behavior using the
existing logs tab-navigation utilities or contract, ensuring the extracted
helper’s keyboard behavior is fully exercised.

Loading
Loading