Skip to content
Merged
20 changes: 10 additions & 10 deletions .github/workflows/react-doctor.yml
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
# React Doctor — finds security, performance, correctness, accessibility,
# bundle-size, and architecture issues in React codebases.
#
# Advisory-only and least-privilege: findings appear in the step log and the
# Actions run summary. All write-scoped outputs (sticky PR comments, inline
# review comments, commit statuses) are explicitly disabled so the workflow
# needs no write permissions. Do not re-add write scopes without revisiting
# tests/ci-workflows.test.ts, which pins this contract.
# Gating and least-privilege: findings fail the job (`blocking: warning`).
# Write-scoped outputs (sticky PR comments, inline review comments, commit
# statuses) stay disabled so the workflow needs no write permissions. Do not
# re-add write scopes without revisiting tests/ci-workflows.test.ts, which
# pins this contract.
#
# Docs: https://www.react.doctor/ci
# Docs: https://www.react.doctor/docs/ci-and-prs/github-actions-setup
# Source: https://github.com/millionco/react-doctor

name: React Doctor
Expand All @@ -24,7 +24,7 @@ permissions:
contents: read
# Needed so the action can list PR files for --changed-files-from.
# Without this, listFiles fails, the changed-files file is never written,
# and the CLI exits 1 on ENOENT even with blocking: none (fork PRs).
# and the CLI exits 1 on ENOENT even for fork PRs.
pull-requests: read

# Cancels any in-flight scan for the same PR (or branch, on push) the moment a
Expand All @@ -50,9 +50,9 @@ jobs:
directory: gui
# Pin the npm engine — the action wrapper would otherwise fetch
# react-doctor@latest, silently skewing CI from the local pinned runs.
version: "0.9.1"
# Advisory contract: report to the step log only; never gate, never write.
blocking: none
version: "0.9.2"
# Fail the job on any finding (errors or warnings).
blocking: warning
comment: false
review-comments: false
commit-status: false
6 changes: 3 additions & 3 deletions gui/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ the package layout used by `ocx gui`.
```bash
cd gui
bun run lint # ESLint — hard local/CI gate (`GUI lint` in CI)
bun run doctor # React Doctor vs origin/main (changed-scope, advisory)
bun run doctor:full # Full-project React Doctor scan
bun run doctor # React Doctor vs origin/main (changed-scope, gates on findings)
bun run doctor:full # Full-tree React Doctor (gates on findings)
```

From the repo root:
Expand All @@ -49,6 +49,6 @@ bun run setup:hooks # pre-push runs doctor when gui/ changed
| Tool | Role |
|------|------|
| **ESLint** (`bun run lint`) | Hard gate in CI and expected before merge |
| **React Doctor** (`bun run doctor`) | Advisory React health check pinned to react-doctor 0.9.1. Pre-push runs it only if `gui/` changed and never blocks the push. The CI workflow reports to the step log only |
| **React Doctor** (`bun run doctor`) | Gating React health check pinned to react-doctor 0.9.2 (`blocking: warning`). Pre-push runs it only if `gui/` changed and fails the push on findings. The CI workflow fails the job on any finding |

Fix ESLint errors first. Use `doctor` / `doctor:full` for deeper React triage.
2 changes: 1 addition & 1 deletion gui/doctor.config.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"$schema": "https://react.doctor/schema/config.json",
"scope": "full",
"blocking": "none",
"blocking": "warning",
"ignore": {
"files": ["dist/**", "node_modules/**"]
},
Expand Down
4 changes: 2 additions & 2 deletions gui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
"lint": "eslint .",
"test": "bun test tests",
"lint:i18n": "eslint src/pages src/components src/App.tsx src/ui.tsx",
"doctor": "npx --yes react-doctor@0.9.1 --verbose --scope changed --base origin/main --no-telemetry",
"doctor:full": "npx --yes react-doctor@0.9.1 --verbose --scope full --no-telemetry",
"doctor": "npx --yes react-doctor@0.9.2 --verbose --scope changed --base origin/main --no-telemetry",
"doctor:full": "npx --yes react-doctor@0.9.2 --verbose --scope full --no-telemetry",
"preview": "vite preview"
},
"dependencies": {
Expand Down
31 changes: 31 additions & 0 deletions gui/src/bounded-fetch.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/**
* Bound a fetch with AbortSignal.timeout when available; otherwise a manual
* timer that must be cleared after settlement / unmount.
*/

export type BoundedFetch = {
controller: AbortController;
signal: AbortSignal;
clear: () => void;
};

export function createBoundedFetch(ms: number): BoundedFetch {
const controller = new AbortController();
if (
typeof AbortSignal !== "undefined"
&& typeof AbortSignal.any === "function"
&& typeof AbortSignal.timeout === "function"
) {
return {
controller,
signal: AbortSignal.any([controller.signal, AbortSignal.timeout(ms)]),
clear: () => undefined,
};
}
const timeoutId = setTimeout(() => controller.abort(), ms);
return {
controller,
signal: controller.signal,
clear: () => clearTimeout(timeoutId),
};
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
15 changes: 10 additions & 5 deletions gui/src/combo-workspace-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export function intersectComboEfforts(
): ComboEffort[] {
const complete = targets.filter((t) => t.provider.trim() && t.model.trim());
if (complete.length === 0) return [...COMBO_EFFORTS];
const effortSet = new Set<string>(COMBO_EFFORTS);
let common: string[] | null = null;
for (const target of complete) {
const key = `${target.provider.trim()}/${target.model.trim()}`;
Expand All @@ -23,12 +24,16 @@ export function intersectComboEfforts(
// supportedLadderFor is undefined (#488 / Codex review).
const member: string[] = listed === undefined
? []
: listed.filter((effort) => (COMBO_EFFORTS as readonly string[]).includes(effort));
common = common === null
? member
: common.filter((effort) => member.includes(effort));
: listed.filter((effort) => effortSet.has(effort));
if (common === null) {
common = member;
} else {
const memberSet = new Set(member);
common = common.filter((effort) => memberSet.has(effort));
}
}
return COMBO_EFFORTS.filter((effort) => common?.includes(effort) === true);
const commonSet = new Set(common ?? []);
return COMBO_EFFORTS.filter((effort) => commonSet.has(effort));
}

export interface ComboTarget {
Expand Down
128 changes: 68 additions & 60 deletions gui/src/components/MemoryObservabilityCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { useEffect, useState } from "react";
import { formatUptime } from "../formatUptime";
import { IconActivity } from "../icons";
import { useI18n, type Locale } from "../i18n/shared";
import { createBoundedFetch, type BoundedFetch } from "../bounded-fetch";

/**
* Memory observability card. Polls GET /api/system/memory (#314 WP3) every 5s
Expand Down Expand Up @@ -163,67 +164,57 @@ export default function MemoryObservabilityCard({ apiBase }: { apiBase: string }
useEffect(() => {
let cancelled = false;
let inFlight = false;
let activeController: AbortController | null = null;
let active: BoundedFetch | null = null;
const fetchMemory = async () => {
// Serialize polls: a stalled request must not stack up or let an older
// payload land after a newer one.
if (inFlight) return;
inFlight = true;
// Bound each poll so a hung request cannot pin inFlight forever and
// starve the unavailable fallback. Prefer AbortSignal.timeout; fall back
// to a manual timer when the browser lacks AbortSignal.any/timeout.
const controller = new AbortController();
activeController = controller;
let timeoutId: ReturnType<typeof setTimeout> | undefined;
const signal = typeof AbortSignal !== "undefined" && "any" in AbortSignal && "timeout" in AbortSignal
? AbortSignal.any([controller.signal, AbortSignal.timeout(10_000)])
: (() => {
timeoutId = setTimeout(() => controller.abort(), 10_000);
return controller.signal;
})();
// Bound each poll so a hung request cannot pin inFlight forever.
const bounded = createBoundedFetch(10_000);
active = bounded;
try {
const res = await fetch(`${apiBase}/api/system/memory`, { signal });
const res = await fetch(`${apiBase}/api/system/memory`, { signal: bounded.signal });
if (!res.ok) throw new Error("memory unavailable");
const json = await res.json() as SystemMemory;
if (!cancelled) {
setData(json);
setUnavailable(false);
setSupportsRestart(typeof json.activeTurnCount === "number");
if (json.isDraining && restartPhase === "idle") setRestartPhase("draining");
// Fast recycle can finish between polls with no observed outage — detect pid change.
if (
(restartPhase === "draining" || restartPhase === "reconnecting")
&& restartFromPid != null
&& typeof json.pid === "number"
&& json.pid !== restartFromPid
&& !json.isDraining
) {
setRestartPhase("idle");
setRestartFromPid(null);
setRestartError(null);
}
if (cancelled) return;
setData(json);
setUnavailable(false);
setSupportsRestart(typeof json.activeTurnCount === "number");
if (json.isDraining && restartPhase === "idle") setRestartPhase("draining");
// Fast recycle can finish between polls with no observed outage — detect pid change.
if (
(restartPhase === "draining" || restartPhase === "reconnecting")
&& restartFromPid != null
&& typeof json.pid === "number"
&& json.pid !== restartFromPid
&& !json.isDraining
) {
setRestartPhase("idle");
setRestartFromPid(null);
setRestartError(null);
}
} catch {
// Old servers (pre-#314) 404 this route; degrade to a quiet unavailable note.
// During drain/restart the proxy goes away — switch to reconnect polling.
if (!cancelled) {
if (restartPhase === "draining" || restartPhase === "reconnecting") {
setRestartPhase("reconnecting");
} else {
setUnavailable(true);
}
if (cancelled) return;
if (restartPhase === "draining" || restartPhase === "reconnecting") {
setRestartPhase("reconnecting");
} else {
setUnavailable(true);
}
} finally {
if (timeoutId !== undefined) clearTimeout(timeoutId);
if (activeController === controller) activeController = null;
bounded.clear();
if (active === bounded) active = null;
inFlight = false;
}
};
void fetchMemory();
const interval = setInterval(() => void fetchMemory(), 5000);
return () => {
cancelled = true;
activeController?.abort();
active?.controller.abort();
active?.clear();
clearInterval(interval);
};
}, [apiBase, restartPhase, restartFromPid]);
Expand All @@ -232,15 +223,23 @@ export default function MemoryObservabilityCard({ apiBase }: { apiBase: string }
if (restartPhase !== "reconnecting") return;
let cancelled = false;
let inFlight = false;
let active: BoundedFetch | null = null;
const started = Date.now();
const tick = async () => {
if (inFlight) return;
const tick = () => {
if (inFlight || cancelled) return;
inFlight = true;
const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), 5_000);
try {
const res = await fetch(`${apiBase}/healthz`, { cache: "no-store", signal: controller.signal });
if (res.ok && !cancelled) {
const bounded = createBoundedFetch(5_000);
active = bounded;
void fetch(`${apiBase}/healthz`, { cache: "no-store", signal: bounded.signal })
.then(async (res) => {
if (cancelled) return;
if (!res.ok) {
if (Date.now() - started >= RECONNECT_GIVE_UP_MS) {
setRestartPhase("error");
setRestartError(t("dash.mem.restartFailed"));
}
return;
}
let replaced = restartFromPid == null;
if (restartFromPid != null) {
try {
Expand All @@ -250,28 +249,37 @@ export default function MemoryObservabilityCard({ apiBase }: { apiBase: string }
replaced = true;
}
}
if (cancelled) return;
if (replaced) {
setRestartPhase("idle");
setRestartFromPid(null);
setRestartError(null);
return;
}
}
} catch {
/* still down / aborted */
} finally {
clearTimeout(timeoutId);
inFlight = false;
}
if (!cancelled && Date.now() - started >= RECONNECT_GIVE_UP_MS) {
setRestartPhase("error");
setRestartError(t("dash.mem.restartFailed"));
}
if (Date.now() - started >= RECONNECT_GIVE_UP_MS) {
setRestartPhase("error");
setRestartError(t("dash.mem.restartFailed"));
}
})
.catch(() => {
if (cancelled) return;
if (Date.now() - started >= RECONNECT_GIVE_UP_MS) {
setRestartPhase("error");
setRestartError(t("dash.mem.restartFailed"));
}
})
.finally(() => {
bounded.clear();
if (active === bounded) active = null;
inFlight = false;
});
};
void tick();
const interval = setInterval(() => void tick(), RECONNECT_POLL_MS);
tick();
const interval = setInterval(tick, RECONNECT_POLL_MS);
return () => {
cancelled = true;
active?.controller.abort();
active?.clear();
clearInterval(interval);
};
}, [apiBase, restartPhase, restartFromPid, t]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export default function AnthropicAccountPoolSettings({
useEffect(() => {
let cancelled = false;
const ac = new AbortController();
void (async () => {
const load = async () => {
try {
const res = await fetch(`${apiBase}/api/oauth/accounts/pool?provider=anthropic`, {
signal: ac.signal,
Expand All @@ -44,10 +44,12 @@ export default function AnthropicAccountPoolSettings({
if (cancelled || ac.signal.aborted) return;
setLoadError(true);
}
})();
};
const timer = window.setTimeout(() => { void load(); }, 0);
return () => {
cancelled = true;
ac.abort();
window.clearTimeout(timer);
};
}, [apiBase]);

Expand Down
23 changes: 18 additions & 5 deletions gui/src/fetch-json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,22 @@
/** Parse an OK response body; 204 / empty bodies yield `undefined`. */
async function readJsonBody<T>(res: Response): Promise<T | undefined> {
if (res.status === 204) return undefined;
const text = await res.text();
if (!text.trim()) return undefined;
return JSON.parse(text) as T;
// Prefer text() so empty OK bodies are detectable. Fall back to json() for
// Response-like test doubles that only implement json().
if (typeof res.text === "function") {
const text = await res.text();
if (!text.trim()) return undefined;
return JSON.parse(text) as T;
}
return await res.json() as T;
}

function errorMessageFromBody(errBody: { error?: unknown; message?: unknown }, fallback: string): string {
if (typeof errBody.error === "string" && errBody.error) return errBody.error;
// Some management routes (e.g. Grok apply orphan repair) put the actionable
// copy in `message` rather than `error`.
if (typeof errBody.message === "string" && errBody.message) return errBody.message;
return fallback;
}

export async function readJsonOrThrow<T>(
Expand All @@ -19,8 +32,8 @@ export async function readJsonOrThrow<T>(
if (!res.ok) {
let message = fallbackMessage;
try {
const errBody = await res.json() as { error?: unknown };
if (typeof errBody?.error === "string" && errBody.error) message = errBody.error;
const errBody = await res.json() as { error?: unknown; message?: unknown };
message = errorMessageFromBody(errBody, fallbackMessage);
} catch {
// non-JSON error bodies keep the fallback message
}
Expand Down
2 changes: 1 addition & 1 deletion gui/src/pages/Claude.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useRef, useState, type KeyboardEvent } from "react";
import ClaudeCode from "./ClaudeCode";
import ClaudeDesktop from "./ClaudeDesktop";
import { useT } from "../i18n";
import { useT } from "../i18n/shared";

type ClaudeTab = "code" | "desktop";

Expand Down
Loading
Loading