From 8061425689d8b1c61282a65e9f79b6b505bda775 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Tue, 28 Jul 2026 04:57:44 +0200 Subject: [PATCH 1/9] chore(gui): restore React Doctor 100 and gate CI on findings Clear the post-cleanup regressions, pin react-doctor 0.9.2, and fail CI/prepush on any doctor finding so the score cannot silently drift again. --- .github/workflows/react-doctor.yml | 20 ++-- gui/README.md | 6 +- gui/doctor.config.json | 2 +- gui/package.json | 4 +- gui/src/combo-workspace-data.ts | 15 ++- .../components/MemoryObservabilityCard.tsx | 106 +++++++++--------- .../AnthropicAccountPoolSettings.tsx | 6 +- gui/src/pages/Claude.tsx | 2 +- gui/src/pages/ClaudeDesktop.tsx | 26 +++-- gui/src/pages/Grok.tsx | 18 +-- gui/src/pages/Logs.tsx | 5 +- gui/src/pages/Storage.tsx | 85 +++++++++++--- scripts/doctor-gui-if-changed.ts | 20 ++-- tests/ci-workflows.test.ts | 30 +++-- 14 files changed, 222 insertions(+), 123 deletions(-) diff --git a/.github/workflows/react-doctor.yml b/.github/workflows/react-doctor.yml index f81b046e05..b9076bcb8a 100644 --- a/.github/workflows/react-doctor.yml +++ b/.github/workflows/react-doctor.yml @@ -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 @@ -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 @@ -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 diff --git a/gui/README.md b/gui/README.md index fdbd4f2be0..8981908b95 100644 --- a/gui/README.md +++ b/gui/README.md @@ -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: @@ -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. diff --git a/gui/doctor.config.json b/gui/doctor.config.json index f9bd272c20..46169c5bf0 100644 --- a/gui/doctor.config.json +++ b/gui/doctor.config.json @@ -1,7 +1,7 @@ { "$schema": "https://react.doctor/schema/config.json", "scope": "full", - "blocking": "none", + "blocking": "warning", "ignore": { "files": ["dist/**", "node_modules/**"] }, diff --git a/gui/package.json b/gui/package.json index b8e26c169d..ce7f602b0b 100644 --- a/gui/package.json +++ b/gui/package.json @@ -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": { diff --git a/gui/src/combo-workspace-data.ts b/gui/src/combo-workspace-data.ts index 9634c885c6..bc29bff703 100644 --- a/gui/src/combo-workspace-data.ts +++ b/gui/src/combo-workspace-data.ts @@ -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(COMBO_EFFORTS); let common: string[] | null = null; for (const target of complete) { const key = `${target.provider.trim()}/${target.model.trim()}`; @@ -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 { diff --git a/gui/src/components/MemoryObservabilityCard.tsx b/gui/src/components/MemoryObservabilityCard.tsx index fe968e7533..3276020318 100644 --- a/gui/src/components/MemoryObservabilityCard.tsx +++ b/gui/src/components/MemoryObservabilityCard.tsx @@ -169,52 +169,45 @@ export default function MemoryObservabilityCard({ apiBase }: { apiBase: string } // 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. + // Bound each poll so a hung request cannot pin inFlight forever. + // Prefer AbortSignal.timeout (no setTimeout to clean up); fall back to + // unmount-only abort when the browser lacks timeout/any. const controller = new AbortController(); activeController = controller; - let timeoutId: ReturnType | 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; - })(); + : controller.signal; try { const res = await fetch(`${apiBase}/api/system/memory`, { 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; inFlight = false; } @@ -232,15 +225,19 @@ export default function MemoryObservabilityCard({ apiBase }: { apiBase: string } if (restartPhase !== "reconnecting") return; let cancelled = false; let inFlight = false; + let activeController: AbortController | 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) { + activeController = controller; + const signal = typeof AbortSignal !== "undefined" && "any" in AbortSignal && "timeout" in AbortSignal + ? AbortSignal.any([controller.signal, AbortSignal.timeout(5_000)]) + : controller.signal; + void fetch(`${apiBase}/healthz`, { cache: "no-store", signal }) + .then(async (res) => { + if (!res.ok || cancelled) return; let replaced = restartFromPid == null; if (restartFromPid != null) { try { @@ -250,28 +247,35 @@ 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(() => { + if (activeController === controller) activeController = null; + inFlight = false; + }); }; - void tick(); - const interval = setInterval(() => void tick(), RECONNECT_POLL_MS); + tick(); + const interval = setInterval(tick, RECONNECT_POLL_MS); return () => { cancelled = true; + activeController?.abort(); clearInterval(interval); }; }, [apiBase, restartPhase, restartFromPid, t]); diff --git a/gui/src/components/provider-workspace/AnthropicAccountPoolSettings.tsx b/gui/src/components/provider-workspace/AnthropicAccountPoolSettings.tsx index 257e566f8d..2bd21d454d 100644 --- a/gui/src/components/provider-workspace/AnthropicAccountPoolSettings.tsx +++ b/gui/src/components/provider-workspace/AnthropicAccountPoolSettings.tsx @@ -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, @@ -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]); diff --git a/gui/src/pages/Claude.tsx b/gui/src/pages/Claude.tsx index f8b611b5b1..9dfd048682 100644 --- a/gui/src/pages/Claude.tsx +++ b/gui/src/pages/Claude.tsx @@ -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"; diff --git a/gui/src/pages/ClaudeDesktop.tsx b/gui/src/pages/ClaudeDesktop.tsx index 069fdf0c81..39426ca7d8 100644 --- a/gui/src/pages/ClaudeDesktop.tsx +++ b/gui/src/pages/ClaudeDesktop.tsx @@ -3,7 +3,8 @@ import { LANE_PAGE, defaultCollapsedFamilies, laneView, rowStartsOpen } from "./ import { makeCollapseStore, toggleInSet } from "./collapse-store"; import { IconChevron } from "../icons"; import { EmptyState, Notice } from "../ui"; -import { useT, type TFn, type TKey } from "../i18n"; +import { useT, type TFn, type TKey } from "../i18n/shared"; +import { readJsonIfOk, readJsonOrThrow } from "../fetch-json"; const FAMILIES = ["opus", "fable", "sonnet", "haiku"] as const; type Family = typeof FAMILIES[number]; @@ -150,8 +151,11 @@ export default function ClaudeDesktop({ apiBase }: { apiBase: string }) { setLoadError(""); try { const response = await fetch(`${apiBase}/api/claude-desktop`); - const payload = await response.json() as DesktopResponse | { error?: string }; - if (!response.ok || !("profile" in payload) || !("models" in payload)) { + const payload = await readJsonOrThrow( + response, + t("claudeDesktop.loadFail"), + ); + if (!payload || !("profile" in payload) || !("models" in payload)) { throw new Error(errorMessage(payload, t("claudeDesktop.loadFail"))); } const normalized = normalizeProfile(payload); @@ -204,7 +208,15 @@ export default function ClaudeDesktop({ apiBase }: { apiBase: string }) { // Poll Desktop status every 5s for applied-state + health. useEffect(() => { let cancelled = false; - const poll = () => fetch(`${apiBase}/api/claude-desktop/status`).then(r => r.json()).then(d => { if (!cancelled) setStatus(d as DesktopStatus); }).catch(() => {}); + const poll = () => { + void fetch(`${apiBase}/api/claude-desktop/status`) + .then((response) => readJsonIfOk(response)) + .then((data) => { + if (cancelled) return; + if (data) setStatus(data); + }) + .catch(() => { /* offline / older proxy */ }); + }; poll(); const timer = setInterval(poll, 5000); return () => { cancelled = true; clearInterval(timer); }; @@ -248,15 +260,13 @@ export default function ClaudeDesktop({ apiBase }: { apiBase: string }) { headers: { "Content-Type": "application/json" }, body: JSON.stringify({ profile }), }); - const payload = await response.json().catch(() => ({})) as { error?: string }; - if (!response.ok) throw new Error(errorMessage(payload, t("claudeDesktop.saveFailed"))); + await readJsonOrThrow<{ error?: string }>(response, t("claudeDesktop.saveFailed")); setSavedProfile(cloneProfile(profile)); if (applyAfter) { setPending("apply"); const applyResponse = await fetch(`${apiBase}/api/claude-desktop/apply`, { method: "POST" }); - const applyPayload = await applyResponse.json().catch(() => ({})) as { error?: string }; - if (!applyResponse.ok) throw new Error(errorMessage(applyPayload, t("claudeDesktop.applyFailed"))); + await readJsonOrThrow<{ error?: string }>(applyResponse, t("claudeDesktop.applyFailed")); setMessage({ tone: "ok", text: t("claudeDesktop.savedApplied") }); setAnnouncement(t("claudeDesktop.savedAppliedAnnounce")); } else { diff --git a/gui/src/pages/Grok.tsx b/gui/src/pages/Grok.tsx index 7dc5b5f2a8..559245f98d 100644 --- a/gui/src/pages/Grok.tsx +++ b/gui/src/pages/Grok.tsx @@ -1,7 +1,8 @@ import { useCallback, useEffect, useMemo, useState } from "react"; import { EmptyState, Notice, Switch } from "../ui"; import { IconChevron } from "../icons"; -import { useT, type TKey } from "../i18n"; +import { useT, type TKey } from "../i18n/shared"; +import { readJsonOrThrow } from "../fetch-json"; import { makeCollapseStore, toggleInSet } from "./collapse-store"; import { grokGroupView, type GrokCandidate } from "./grok-groups"; @@ -67,8 +68,8 @@ export default function Grok({ apiBase }: { apiBase: string }) { setError(""); try { const response = await fetch(`${apiBase}/api/grok`); - const payload = await response.json() as GrokStatus & { error?: string }; - if (!response.ok) throw new Error(payload.error || t("grok.loadFail")); + const payload = await readJsonOrThrow(response, t("grok.loadFail")); + if (!payload) throw new Error(t("grok.loadFail")); // Tolerate an older proxy that predates the selection routes: the page degrades // to the read-only fence view instead of crashing on a missing field. setStatus({ ...payload, candidates: payload.candidates ?? [], excluded: payload.excluded ?? [] }); @@ -124,18 +125,19 @@ export default function Grok({ apiBase }: { apiBase: string }) { headers: { "Content-Type": "application/json" }, body: JSON.stringify({ excluded: [...excluded] }), }); - const savePayload = await response.json().catch(() => ({})) as { error?: string }; - if (!response.ok) throw new Error(savePayload.error ?? t("grok.saveFailed")); + await readJsonOrThrow<{ error?: string }>(response, t("grok.saveFailed")); setSavedExcluded(new Set(excluded)); if (applyAfter) { setPending("apply"); const applied = await fetch(`${apiBase}/api/grok/apply`, { method: "POST" }); - const payload = await applied.json().catch(() => ({})) as { message?: string; skippedReason?: string }; - if (!applied.ok) throw new Error(payload.message ?? t("grok.applyFailed")); + const payload = await readJsonOrThrow<{ message?: string; skippedReason?: string }>( + applied, + t("grok.applyFailed"), + ); // A policy skip is not success theatre: the Grok config did NOT change // (non-loopback bind, or no ~/.grok), so say that instead of "applied". - if (payload.skippedReason) { + if (payload?.skippedReason) { setMessage({ tone: "err", text: payload.message ?? t("grok.applySkipped") }); setAnnouncement(payload.message ?? t("grok.applySkipped")); } else { diff --git a/gui/src/pages/Logs.tsx b/gui/src/pages/Logs.tsx index 92bfdb66d4..92464397df 100644 --- a/gui/src/pages/Logs.tsx +++ b/gui/src/pages/Logs.tsx @@ -295,8 +295,9 @@ function summarizeFilteredLogs(entries: LogEntry[]): { continue; } const cost = entry.displayMetrics?.cost; - if (cost?.kind === "value" && Number.isFinite(cost.estimate.cost.total) && cost.estimate.cost.total >= 0) { - estimatedCostUsd += cost.estimate.cost.total; + const total = cost?.kind === "value" ? cost.estimate.cost.total : undefined; + if (total !== undefined && Number.isFinite(total) && total >= 0) { + estimatedCostUsd += total; continue; } unpricedRequests += 1; diff --git a/gui/src/pages/Storage.tsx b/gui/src/pages/Storage.tsx index 171172666a..e3a05f4d7a 100644 --- a/gui/src/pages/Storage.tsx +++ b/gui/src/pages/Storage.tsx @@ -274,8 +274,11 @@ function ArchivedCleanupPanel({ headers: { "content-type": "application/json" }, body: JSON.stringify({ percent }), }); - const json = await res.json() as CleanupPreview & { error?: string }; - if (!res.ok) throw new Error(mapCleanupError(json.error, t("storage.cleanup.previewFailed"))); + if (!res.ok) { + const json = await res.json().catch(() => ({})) as { error?: string }; + throw new Error(mapCleanupError(json.error, t("storage.cleanup.previewFailed"))); + } + const json = await res.json() as CleanupPreview; setPreview(json); setConfirmOpen(true); } catch (e) { @@ -299,14 +302,21 @@ function ArchivedCleanupPanel({ digest: preview.digest, }), }); - const json = await res.json() as CleanupResult; - if (!res.ok || !json.ok) { + if (!res.ok) { + const json = await res.json().catch(() => ({})) as CleanupResult; if (json.error === "stale_preview") { // Digest can never succeed again — send the user back to Preview. closeConfirm(true); } throw new Error(mapCleanupError(json.error, json.message, json.trashDir)); } + const json = await res.json() as CleanupResult; + if (!json.ok) { + if (json.error === "stale_preview") { + closeConfirm(true); + } + throw new Error(mapCleanupError(json.error, json.message, json.trashDir)); + } closeConfirm(true); setStatus( permanent @@ -479,9 +489,13 @@ function QuarantineTrashPanel({ setLoading(true); try { const res = await fetch(`${apiBase}/api/storage/trash`, { signal }); - const json = await res.json() as TrashList & { error?: string }; + if (!res.ok) { + const json = await res.json().catch(() => ({})) as { error?: string }; + if (signal?.aborted || generation !== loadGenerationRef.current) return; + throw new Error(json.error ?? "list_failed"); + } + const json = await res.json() as TrashList; if (signal?.aborted || generation !== loadGenerationRef.current) return; - if (!res.ok) throw new Error(json.error ?? "list_failed"); const next = Array.isArray(json.entries) ? json.entries : []; setEntries(next); onEntriesChange?.(next); @@ -537,8 +551,12 @@ function QuarantineTrashPanel({ headers: { "content-type": "application/json" }, body: JSON.stringify({ id: confirmEntry.id }), }); + if (!res.ok) { + const json = await res.json().catch(() => ({})) as RestoreResult; + throw new Error(mapRestoreError(json.error, json.message)); + } const json = await res.json() as RestoreResult; - if (!res.ok || !json.ok) { + if (!json.ok) { throw new Error(mapRestoreError(json.error, json.message)); } closeConfirm(); @@ -788,8 +806,12 @@ function AutoCleanupPolicyPanel({ headers: { "content-type": "application/json" }, body: JSON.stringify(body), }); + if (!res.ok) { + setError(t("storage.policy.saveFailed")); + return; + } const json = await res.json() as { ok?: boolean; policy?: CleanupPolicy; error?: string }; - if (!res.ok || !json.policy) { + if (!json.policy) { setError(t("storage.policy.saveFailed")); return; } @@ -823,9 +845,14 @@ function AutoCleanupPolicyPanel({ body: JSON.stringify(base), signal, }); + if (signal.aborted) return; + if (!saveRes.ok) { + setError(t("storage.policy.saveFailed")); + return; + } const saved = await saveRes.json() as { policy?: CleanupPolicy; error?: string }; if (signal.aborted) return; - if (!saveRes.ok || !saved.policy) { + if (!saved.policy) { setError(t("storage.policy.saveFailed")); return; } @@ -835,6 +862,31 @@ function AutoCleanupPolicyPanel({ method: "POST", signal, }); + if (signal.aborted) return; + if (res.status === 409) { + const conflict = await res.json().catch(() => ({})) as { + error?: string; + policy?: CleanupPolicy; + }; + if (signal.aborted) return; + if (conflict.policy) setPolicy(policyFieldsFromResponse(conflict.policy)); + setError(t("storage.policy.alreadyRunning")); + return; + } + if (!res.ok) { + const failed = await res.json().catch(() => ({})) as { + error?: string; + policy?: CleanupPolicy; + }; + if (signal.aborted) return; + if (failed.policy) setPolicy(policyFieldsFromResponse(failed.policy)); + if (failed.error === "already_running") { + setError(t("storage.policy.alreadyRunning")); + return; + } + setError(t("storage.policy.runFailed")); + return; + } const startJson = await res.json() as { ok?: boolean; started?: boolean; @@ -844,14 +896,10 @@ function AutoCleanupPolicyPanel({ }; if (signal.aborted) return; if (startJson.policy) setPolicy(policyFieldsFromResponse(startJson.policy)); - if (startJson.error === "already_running" || res.status === 409) { + if (startJson.error === "already_running") { setError(t("storage.policy.alreadyRunning")); return; } - if (!res.ok) { - setError(t("storage.policy.runFailed")); - return; - } if (!startJson.started || !startJson.job?.startedAt) { setError(t("storage.policy.runFailed")); return; @@ -1000,7 +1048,13 @@ function AutoCleanupPolicyPanel({ max={100} value={percent} disabled={saving || running} - onChange={e => setPercent(Number(e.target.value))} + aria-label={t("storage.policy.targetPercent")} + onChange={e => { + const raw = e.target.value; + if (raw.trim() === "") return; + const next = Number(raw); + if (Number.isFinite(next)) setPercent(next); + }} onBlur={() => void savePolicy()} style={{ display: "block", marginTop: 4, width: "100%" }} /> @@ -1022,6 +1076,7 @@ function AutoCleanupPolicyPanel({ step={0.1} value={reduceGb} disabled={saving || running} + aria-label={t("storage.policy.targetReduce")} onChange={e => setReduceGb(e.target.value)} onBlur={() => void savePolicy()} style={{ display: "block", marginTop: 4, width: "100%" }} diff --git a/scripts/doctor-gui-if-changed.ts b/scripts/doctor-gui-if-changed.ts index 9990913056..43257d9303 100644 --- a/scripts/doctor-gui-if-changed.ts +++ b/scripts/doctor-gui-if-changed.ts @@ -2,9 +2,9 @@ * Run React Doctor in gui/ when this push includes gui/ changes. * Used by `bun run prepush`. Skip with: git push --no-verify * - * Advisory by contract (doctor.config.json blocking: "none"): findings never - * gate the push, so an unavailable engine (offline npx fetch, registry outage) - * degrades to a warning instead of blocking. + * Gating by contract (doctor.config.json blocking: "warning"): findings fail + * the push. An unavailable engine (offline npx fetch, missing binary) still + * degrades to a warning so infrastructure outages do not brick pushes. * * Test hooks: DOCTOR_DRY_RUN=1 prints the run/skip decision without spawning; * DOCTOR_FILES (newline-separated) overrides git-derived changed files; @@ -78,10 +78,16 @@ if (import.meta.main) { stdio: "inherit", env: { ...process.env, npm_config_yes: "true" }, }); - } catch { - // Findings are non-gating (blocking: none), so any failure here is - // infrastructure noise, not a content signal. Never block the push on it. - console.warn("doctor:gui: react-doctor unavailable (offline?) — skipping advisory scan"); + } catch (err) { + const status = typeof err === "object" && err !== null && "status" in err + ? (err as { status?: unknown }).status + : undefined; + // Doctor ran and returned a non-zero status (findings or engine error) — gate the push. + if (typeof status === "number") { + process.exit(status === 0 ? 1 : status); + } + // Spawn failed (missing binary / offline npx) — do not brick the push on infra. + console.warn("doctor:gui: react-doctor unavailable (offline?) — skipping scan"); process.exit(0); } } diff --git a/tests/ci-workflows.test.ts b/tests/ci-workflows.test.ts index d61363ebcc..6d3d144814 100644 --- a/tests/ci-workflows.test.ts +++ b/tests/ci-workflows.test.ts @@ -1912,7 +1912,7 @@ describe("GitHub Actions hardening", () => { expect(helperSrc).not.toContain(".ocx-translation-state"); }); - test("React Doctor workflow is SHA-pinned, engine-pinned, advisory, and read-only", async () => { + test("React Doctor workflow is SHA-pinned, engine-pinned, gating, and read-only", async () => { const workflow = await readText(".github/workflows/react-doctor.yml"); expect(workflow).toContain("actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8"); @@ -1920,17 +1920,17 @@ describe("GitHub Actions hardening", () => { expect(workflow).not.toMatch(/uses:\s+\S+@(?:v\d+|main|master)\b/); // Engine pin: the action wrapper would fetch react-doctor@latest without it. - expect(workflow).toContain('version: "0.9.1"'); + expect(workflow).toContain('version: "0.9.2"'); - // Action pin must accept CLI JSON schemaVersion 3 (baseline reports from 0.9.1). + // Action pin must accept CLI JSON schemaVersion 3 (baseline reports from 0.9.x). // v2.1.0's ensure-json-report only knew schemas 1–2 and failed every PR scan. - // Advisory + least privilege: read-only token, all write-scoped outputs off. + // Gating + least privilege: read-only token, all write-scoped outputs off. // pull-requests: read is required so the action can list PR files for // --changed-files-from; without it, fork PRs fail with ENOENT on that file. expect(workflow).toContain("contents: read"); expect(workflow).toContain("pull-requests: read"); expect(workflow).not.toContain(": write"); - expect(workflow).toContain("blocking: none"); + expect(workflow).toContain("blocking: warning"); expect(workflow).toContain("comment: false"); expect(workflow).toContain("review-comments: false"); expect(workflow).toContain("commit-status: false"); @@ -1940,13 +1940,15 @@ describe("GitHub Actions hardening", () => { test("React Doctor package scripts pin the exact engine version with no @latest anywhere", async () => { const guiPkg = await readText("gui/package.json"); const rootPkg = await readText("package.json"); + const doctorConfig = await readText("gui/doctor.config.json"); - expect(guiPkg).toContain("react-doctor@0.9.1"); + expect(guiPkg).toContain("react-doctor@0.9.2"); expect(guiPkg).not.toContain("react-doctor@latest"); expect(rootPkg).not.toContain("react-doctor@latest"); + expect(doctorConfig).toContain('"blocking": "warning"'); expect(rootPkg).toContain('"doctor:gui:if-changed": "bun scripts/doctor-gui-if-changed.ts"'); expect(rootPkg).toContain('"lint:gui": "cd gui && bun run lint"'); - // Gating steps (typecheck, eslint, tests, privacy) run before advisory React Doctor. + // Gating steps include React Doctor after privacy scan on gui/ pushes. expect(rootPkg).toContain("bun run typecheck && bun run lint:gui && bun run test"); expect(rootPkg).toContain("bun run privacy:scan && bun run doctor:gui:if-changed"); }); @@ -1987,6 +1989,18 @@ describe("doctor-gui-if-changed", () => { }, }); expect(run.exitCode).toBe(0); - expect(run.stderr.toString()).toContain("skipping advisory scan"); + expect(run.stderr.toString()).toContain("skipping scan"); + }); + + test("propagates a non-zero doctor exit so findings gate the push", () => { + const run = Bun.spawnSync(["bun", doctorGuiIfChangedScript], { + env: { + ...process.env, + DOCTOR_FILES: "gui/src/App.tsx", + // `bun` exits 1 when asked to run a missing script name. + DOCTOR_CMD: "bun run definitely-not-a-doctor-script", + }, + }); + expect(run.exitCode).not.toBe(0); }); }); From b30d8593aa8ff1ad1766d12989e52e87404d4af2 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Tue, 28 Jul 2026 05:13:09 +0200 Subject: [PATCH 2/9] fix(gui): address CI/Codex follow-ups for React Doctor gate Keep Response-like test doubles working, preserve Grok/message error text, honor reconnect deadlines on non-OK healthz, and soft-skip offline npx/registry failures without letting real findings through. --- .../components/MemoryObservabilityCard.tsx | 9 +++- gui/src/fetch-json.ts | 23 +++++++--- gui/tests/fetch-json.test.ts | 18 ++++++++ scripts/doctor-gui-if-changed.ts | 43 +++++++++++++++---- scripts/fixtures/doctor-findings-exit.ts | 2 + scripts/fixtures/doctor-offline-exit.ts | 2 + tests/ci-workflows.test.ts | 24 ++++++++++- 7 files changed, 105 insertions(+), 16 deletions(-) create mode 100644 scripts/fixtures/doctor-findings-exit.ts create mode 100644 scripts/fixtures/doctor-offline-exit.ts diff --git a/gui/src/components/MemoryObservabilityCard.tsx b/gui/src/components/MemoryObservabilityCard.tsx index 3276020318..35db338c30 100644 --- a/gui/src/components/MemoryObservabilityCard.tsx +++ b/gui/src/components/MemoryObservabilityCard.tsx @@ -237,7 +237,14 @@ export default function MemoryObservabilityCard({ apiBase }: { apiBase: string } : controller.signal; void fetch(`${apiBase}/healthz`, { cache: "no-store", signal }) .then(async (res) => { - if (!res.ok || cancelled) return; + 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 { diff --git a/gui/src/fetch-json.ts b/gui/src/fetch-json.ts index e788ba5fc7..7250292409 100644 --- a/gui/src/fetch-json.ts +++ b/gui/src/fetch-json.ts @@ -7,9 +7,22 @@ /** Parse an OK response body; 204 / empty bodies yield `undefined`. */ async function readJsonBody(res: Response): Promise { 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( @@ -19,8 +32,8 @@ export async function readJsonOrThrow( 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 } diff --git a/gui/tests/fetch-json.test.ts b/gui/tests/fetch-json.test.ts index cf93d3f864..465760e8a8 100644 --- a/gui/tests/fetch-json.test.ts +++ b/gui/tests/fetch-json.test.ts @@ -23,3 +23,21 @@ test("readJsonOrThrow surfaces server error messages", async () => { const res = Response.json({ error: "locked" }, { status: 503 }); await expect(readJsonOrThrow(res, "fallback")).rejects.toThrow("locked"); }); + +test("readJsonOrThrow prefers error, then message, then fallback", async () => { + await expect( + readJsonOrThrow(Response.json({ message: "repair marker" }, { status: 500 }), "fallback"), + ).rejects.toThrow("repair marker"); + await expect( + readJsonOrThrow(Response.json({ error: "err", message: "msg" }, { status: 500 }), "fallback"), + ).rejects.toThrow("err"); +}); + +test("readJsonOrThrow accepts Response-like mocks that only implement json()", async () => { + const mock = { + ok: true, + status: 200, + json: async () => ({ hello: "world" }), + } as unknown as Response; + expect(await readJsonOrThrow<{ hello: string }>(mock)).toEqual({ hello: "world" }); +}); diff --git a/scripts/doctor-gui-if-changed.ts b/scripts/doctor-gui-if-changed.ts index 43257d9303..c5b3c2a722 100644 --- a/scripts/doctor-gui-if-changed.ts +++ b/scripts/doctor-gui-if-changed.ts @@ -18,6 +18,12 @@ export function guiPathsChanged(files: string[]): boolean { return files.some(f => f === "gui" || f.startsWith("gui/")); } +/** True when stderr/stdout looks like a registry/network failure rather than findings. */ +export function looksLikeDoctorInfraFailure(output: string): boolean { + return /ENOTFOUND|ECONNREFUSED|ETIMEDOUT|ENETUNREACH|EAI_AGAIN|getaddrinfo|network|registry|npm ERR|fetch failed|ERR_INVALID_URL|certificate|SSL|TLS|offline|could not resolve|unable to resolve|socket hang up|connect ECONN|connect ENOTFOUND/i + .test(output); +} + if (import.meta.main) { const repoRoot = resolve(import.meta.dirname, ".."); const guiDir = join(repoRoot, "gui"); @@ -73,21 +79,42 @@ if (import.meta.main) { ? process.env.DOCTOR_CMD.split(" ") : ["bun", "run", "doctor"]; try { - execFileSync(cmd!, args, { + // Pipe stdout/stderr so we can distinguish findings from offline/registry + // failures, then replay them so the developer still sees the doctor output. + const out = execFileSync(cmd!, args, { cwd: guiDir, - stdio: "inherit", + encoding: "utf8", + stdio: ["inherit", "pipe", "pipe"], env: { ...process.env, npm_config_yes: "true" }, }); + if (out) process.stdout.write(out); } catch (err) { const status = typeof err === "object" && err !== null && "status" in err ? (err as { status?: unknown }).status : undefined; - // Doctor ran and returned a non-zero status (findings or engine error) — gate the push. - if (typeof status === "number") { - process.exit(status === 0 ? 1 : status); + const stdout = typeof err === "object" && err !== null && "stdout" in err + ? String((err as { stdout?: unknown }).stdout ?? "") + : ""; + const stderr = typeof err === "object" && err !== null && "stderr" in err + ? String((err as { stderr?: unknown }).stderr ?? "") + : ""; + const combined = `${stdout}\n${stderr}`; + if (stdout) process.stdout.write(stdout); + if (stderr) process.stderr.write(stderr); + + // Spawn failed (missing binary) — do not brick the push on infra. + if (typeof status !== "number") { + console.warn("doctor:gui: react-doctor unavailable (offline?) — skipping scan"); + process.exit(0); } - // Spawn failed (missing binary / offline npx) — do not brick the push on infra. - console.warn("doctor:gui: react-doctor unavailable (offline?) — skipping scan"); - process.exit(0); + + // Doctor started but npx/registry failed — same soft-skip contract. + if (looksLikeDoctorInfraFailure(combined)) { + console.warn("doctor:gui: react-doctor unavailable (offline?) — skipping scan"); + process.exit(0); + } + + // Real findings (or other doctor/engine errors) gate the push. + process.exit(status === 0 ? 1 : status); } } diff --git a/scripts/fixtures/doctor-findings-exit.ts b/scripts/fixtures/doctor-findings-exit.ts new file mode 100644 index 0000000000..c0af45362e --- /dev/null +++ b/scripts/fixtures/doctor-findings-exit.ts @@ -0,0 +1,2 @@ +process.stdout.write("All 2 issues\n"); +process.exit(1); diff --git a/scripts/fixtures/doctor-offline-exit.ts b/scripts/fixtures/doctor-offline-exit.ts new file mode 100644 index 0000000000..a072d75763 --- /dev/null +++ b/scripts/fixtures/doctor-offline-exit.ts @@ -0,0 +1,2 @@ +process.stderr.write("npm ERR! network getaddrinfo ENOTFOUND registry.npmjs.org\n"); +process.exit(1); diff --git a/tests/ci-workflows.test.ts b/tests/ci-workflows.test.ts index 6d3d144814..ae01fca118 100644 --- a/tests/ci-workflows.test.ts +++ b/tests/ci-workflows.test.ts @@ -1966,6 +1966,12 @@ describe("doctor-gui-if-changed", () => { expect(guiPathsChanged([])).toBe(false); }); + test("looksLikeDoctorInfraFailure detects registry/network outages", async () => { + const { looksLikeDoctorInfraFailure } = await import("../scripts/doctor-gui-if-changed"); + expect(looksLikeDoctorInfraFailure("npm ERR! network getaddrinfo ENOTFOUND registry.npmjs.org")).toBe(true); + expect(looksLikeDoctorInfraFailure("All 2 issues\nBugs > 1 errors")).toBe(false); + }); + test("DRY_RUN prints the run/skip decision without spawning the doctor", () => { const run = Bun.spawnSync(["bun", doctorGuiIfChangedScript], { env: { ...process.env, DOCTOR_DRY_RUN: "1", DOCTOR_FILES: "gui/src/App.tsx\nscripts/x.ts" }, @@ -1992,13 +1998,27 @@ describe("doctor-gui-if-changed", () => { expect(run.stderr.toString()).toContain("skipping scan"); }); + test("soft-skips when doctor exits nonzero due to a registry/network failure", () => { + // Simulate `bun run doctor` starting, then npx failing offline: numeric status + // plus registry noise in stderr — must not gate the push. + // cwd for DOCTOR_CMD is gui/, so reach fixtures via ../scripts/... + const run = Bun.spawnSync(["bun", doctorGuiIfChangedScript], { + env: { + ...process.env, + DOCTOR_FILES: "gui/src/App.tsx", + DOCTOR_CMD: "bun ../scripts/fixtures/doctor-offline-exit.ts", + }, + }); + expect(run.exitCode).toBe(0); + expect(run.stderr.toString()).toContain("skipping scan"); + }); + test("propagates a non-zero doctor exit so findings gate the push", () => { const run = Bun.spawnSync(["bun", doctorGuiIfChangedScript], { env: { ...process.env, DOCTOR_FILES: "gui/src/App.tsx", - // `bun` exits 1 when asked to run a missing script name. - DOCTOR_CMD: "bun run definitely-not-a-doctor-script", + DOCTOR_CMD: "bun ../scripts/fixtures/doctor-findings-exit.ts", }, }); expect(run.exitCode).not.toBe(0); From b6f493cb5c42605ca21812c6f63f5613aac38429 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Tue, 28 Jul 2026 05:16:02 +0200 Subject: [PATCH 3/9] fix(gui): address CodeRabbit follow-ups on doctor gate PR Extract boundedSignal, serialize Claude Desktop status polls, keep storage percent as a draft string, map trash list errors through i18n, and tighten React Doctor workflow assertions to active YAML. --- .../components/MemoryObservabilityCard.tsx | 15 ++++++++------ gui/src/pages/ClaudeDesktop.tsx | 20 ++++++++++++++++--- gui/src/pages/Storage.tsx | 15 +++++--------- tests/ci-workflows.test.ts | 12 ++++++----- 4 files changed, 38 insertions(+), 24 deletions(-) diff --git a/gui/src/components/MemoryObservabilityCard.tsx b/gui/src/components/MemoryObservabilityCard.tsx index 35db338c30..0c02d566a2 100644 --- a/gui/src/components/MemoryObservabilityCard.tsx +++ b/gui/src/components/MemoryObservabilityCard.tsx @@ -135,6 +135,13 @@ const DRAIN_TIMEOUT_S = 60; const RECONNECT_POLL_MS = 1500; const RECONNECT_GIVE_UP_MS = 120_000; +/** Bound a fetch with AbortSignal.timeout when available; else unmount-abort only. */ +function boundedSignal(controller: AbortController, ms: number): AbortSignal { + return typeof AbortSignal !== "undefined" && "any" in AbortSignal && "timeout" in AbortSignal + ? AbortSignal.any([controller.signal, AbortSignal.timeout(ms)]) + : controller.signal; +} + export default function MemoryObservabilityCard({ apiBase }: { apiBase: string }) { const { locale, t } = useI18n(); const [data, setData] = useState(null); @@ -174,9 +181,7 @@ export default function MemoryObservabilityCard({ apiBase }: { apiBase: string } // unmount-only abort when the browser lacks timeout/any. const controller = new AbortController(); activeController = controller; - const signal = typeof AbortSignal !== "undefined" && "any" in AbortSignal && "timeout" in AbortSignal - ? AbortSignal.any([controller.signal, AbortSignal.timeout(10_000)]) - : controller.signal; + const signal = boundedSignal(controller, 10_000); try { const res = await fetch(`${apiBase}/api/system/memory`, { signal }); if (!res.ok) throw new Error("memory unavailable"); @@ -232,9 +237,7 @@ export default function MemoryObservabilityCard({ apiBase }: { apiBase: string } inFlight = true; const controller = new AbortController(); activeController = controller; - const signal = typeof AbortSignal !== "undefined" && "any" in AbortSignal && "timeout" in AbortSignal - ? AbortSignal.any([controller.signal, AbortSignal.timeout(5_000)]) - : controller.signal; + const signal = boundedSignal(controller, 5_000); void fetch(`${apiBase}/healthz`, { cache: "no-store", signal }) .then(async (res) => { if (cancelled) return; diff --git a/gui/src/pages/ClaudeDesktop.tsx b/gui/src/pages/ClaudeDesktop.tsx index 39426ca7d8..3944e77ac7 100644 --- a/gui/src/pages/ClaudeDesktop.tsx +++ b/gui/src/pages/ClaudeDesktop.tsx @@ -208,18 +208,32 @@ export default function ClaudeDesktop({ apiBase }: { apiBase: string }) { // Poll Desktop status every 5s for applied-state + health. useEffect(() => { let cancelled = false; + let inFlight = false; + let controller: AbortController | null = null; const poll = () => { - void fetch(`${apiBase}/api/claude-desktop/status`) + if (inFlight) return; + inFlight = true; + controller = new AbortController(); + const active = controller; + void fetch(`${apiBase}/api/claude-desktop/status`, { signal: active.signal }) .then((response) => readJsonIfOk(response)) .then((data) => { if (cancelled) return; if (data) setStatus(data); }) - .catch(() => { /* offline / older proxy */ }); + .catch(() => { /* offline / older proxy / aborted */ }) + .finally(() => { + if (controller === active) controller = null; + inFlight = false; + }); }; poll(); const timer = setInterval(poll, 5000); - return () => { cancelled = true; clearInterval(timer); }; + return () => { + cancelled = true; + clearInterval(timer); + controller?.abort(); + }; }, [apiBase]); const moveModel = (route: string, family: Family) => { diff --git a/gui/src/pages/Storage.tsx b/gui/src/pages/Storage.tsx index e3a05f4d7a..0fc3e9e9be 100644 --- a/gui/src/pages/Storage.tsx +++ b/gui/src/pages/Storage.tsx @@ -490,9 +490,8 @@ function QuarantineTrashPanel({ try { const res = await fetch(`${apiBase}/api/storage/trash`, { signal }); if (!res.ok) { - const json = await res.json().catch(() => ({})) as { error?: string }; if (signal?.aborted || generation !== loadGenerationRef.current) return; - throw new Error(json.error ?? "list_failed"); + throw new Error(t("storage.trash.listFailed")); } const json = await res.json() as TrashList; if (signal?.aborted || generation !== loadGenerationRef.current) return; @@ -709,7 +708,8 @@ function AutoCleanupPolicyPanel({ const [status, setStatus] = useState(null); const [error, setError] = useState(null); const [targetMode, setTargetMode] = useState<"percent" | "reduce">("percent"); - const [percent, setPercent] = useState(25); + /** Draft string so blank/invalid percent targets are rejected instead of coerced. */ + const [percent, setPercent] = useState("25"); /** Draft string so blank/invalid reduce targets are rejected instead of coerced to 0. */ const [reduceGb, setReduceGb] = useState("4"); /** Draft string so a cleared threshold is rejected instead of coerced to 0. */ @@ -732,7 +732,7 @@ function AutoCleanupPolicyPanel({ setReduceGb(String(Math.max(0, Math.round((json.target.reduceToBytes / GB) * 100) / 100))); } else { setTargetMode("percent"); - setPercent(Math.min(100, Math.max(1, Math.floor(json.target.removeOldestPercent ?? 25)))); + setPercent(String(Math.min(100, Math.max(1, Math.floor(json.target.removeOldestPercent ?? 25))))); } } catch { if (signal?.aborted) return; @@ -1049,12 +1049,7 @@ function AutoCleanupPolicyPanel({ value={percent} disabled={saving || running} aria-label={t("storage.policy.targetPercent")} - onChange={e => { - const raw = e.target.value; - if (raw.trim() === "") return; - const next = Number(raw); - if (Number.isFinite(next)) setPercent(next); - }} + onChange={e => setPercent(e.target.value)} onBlur={() => void savePolicy()} style={{ display: "block", marginTop: 4, width: "100%" }} /> diff --git a/tests/ci-workflows.test.ts b/tests/ci-workflows.test.ts index ae01fca118..1057602f5c 100644 --- a/tests/ci-workflows.test.ts +++ b/tests/ci-workflows.test.ts @@ -1917,7 +1917,9 @@ describe("GitHub Actions hardening", () => { expect(workflow).toContain("actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8"); expect(workflow).toContain("millionco/react-doctor@938008119a288f2fb47c66a69cd9279a21f31784"); - expect(workflow).not.toMatch(/uses:\s+\S+@(?:v\d+|main|master)\b/); + expect(workflow).not.toMatch( + /^\s*-\s+uses:\s+\S+@(?![0-9a-f]{40}\b)\S+/m, + ); // Engine pin: the action wrapper would fetch react-doctor@latest without it. expect(workflow).toContain('version: "0.9.2"'); @@ -1930,10 +1932,10 @@ describe("GitHub Actions hardening", () => { expect(workflow).toContain("contents: read"); expect(workflow).toContain("pull-requests: read"); expect(workflow).not.toContain(": write"); - expect(workflow).toContain("blocking: warning"); - expect(workflow).toContain("comment: false"); - expect(workflow).toContain("review-comments: false"); - expect(workflow).toContain("commit-status: false"); + expect(workflow).toMatch(/^\s+blocking:\s+warning\s*$/m); + expect(workflow).toMatch(/^\s+comment:\s+false\s*$/m); + expect(workflow).toMatch(/^\s+review-comments:\s+false\s*$/m); + expect(workflow).toMatch(/^\s+commit-status:\s+false\s*$/m); expect(workflow).toContain("timeout-minutes: 10"); }); From 37b3cbcf3fbceee3d60c96b6e71a91e77ec8e192 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Tue, 28 Jul 2026 05:23:28 +0200 Subject: [PATCH 4/9] fix(gui): tighten doctor offline classifier and Grok apply errors Make apply failures keep server message text, cover the fetch-json fallback path, and only soft-skip prepush on concrete registry/network signatures so findings with words like network still gate. --- gui/src/pages/Grok.tsx | 16 +++++++++++----- gui/tests/fetch-json.test.ts | 3 +++ scripts/doctor-gui-if-changed.ts | 3 ++- tests/ci-workflows.test.ts | 2 ++ 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/gui/src/pages/Grok.tsx b/gui/src/pages/Grok.tsx index 559245f98d..e30a38ba16 100644 --- a/gui/src/pages/Grok.tsx +++ b/gui/src/pages/Grok.tsx @@ -131,13 +131,19 @@ export default function Grok({ apiBase }: { apiBase: string }) { if (applyAfter) { setPending("apply"); const applied = await fetch(`${apiBase}/api/grok/apply`, { method: "POST" }); - const payload = await readJsonOrThrow<{ message?: string; skippedReason?: string }>( - applied, - t("grok.applyFailed"), - ); + // Apply errors use `{ message, skippedReason }` (not always `error`); preserve that + // actionable copy for orphan-marker repair and policy skips. + if (!applied.ok) { + const failed = await applied.json().catch(() => ({})) as { message?: string; error?: string }; + throw new Error(failed.message ?? failed.error ?? t("grok.applyFailed")); + } + const payload = await applied.json().catch(() => ({})) as { + message?: string; + skippedReason?: string; + }; // A policy skip is not success theatre: the Grok config did NOT change // (non-loopback bind, or no ~/.grok), so say that instead of "applied". - if (payload?.skippedReason) { + if (payload.skippedReason) { setMessage({ tone: "err", text: payload.message ?? t("grok.applySkipped") }); setAnnouncement(payload.message ?? t("grok.applySkipped")); } else { diff --git a/gui/tests/fetch-json.test.ts b/gui/tests/fetch-json.test.ts index 465760e8a8..9489fb76c9 100644 --- a/gui/tests/fetch-json.test.ts +++ b/gui/tests/fetch-json.test.ts @@ -31,6 +31,9 @@ test("readJsonOrThrow prefers error, then message, then fallback", async () => { await expect( readJsonOrThrow(Response.json({ error: "err", message: "msg" }, { status: 500 }), "fallback"), ).rejects.toThrow("err"); + await expect( + readJsonOrThrow(Response.json({ detail: "other" }, { status: 500 }), "fallback"), + ).rejects.toThrow("fallback"); }); test("readJsonOrThrow accepts Response-like mocks that only implement json()", async () => { diff --git a/scripts/doctor-gui-if-changed.ts b/scripts/doctor-gui-if-changed.ts index c5b3c2a722..103351cbca 100644 --- a/scripts/doctor-gui-if-changed.ts +++ b/scripts/doctor-gui-if-changed.ts @@ -20,7 +20,8 @@ export function guiPathsChanged(files: string[]): boolean { /** True when stderr/stdout looks like a registry/network failure rather than findings. */ export function looksLikeDoctorInfraFailure(output: string): boolean { - return /ENOTFOUND|ECONNREFUSED|ETIMEDOUT|ENETUNREACH|EAI_AGAIN|getaddrinfo|network|registry|npm ERR|fetch failed|ERR_INVALID_URL|certificate|SSL|TLS|offline|could not resolve|unable to resolve|socket hang up|connect ECONN|connect ENOTFOUND/i + // Keep this narrow: bare words like "network"/"offline" also appear in findings text. + return /(?:^|\b)(?:npm ERR!|ENOTFOUND|ECONNREFUSED|ETIMEDOUT|ENETUNREACH|EAI_AGAIN|getaddrinfo ENOTFOUND|fetch failed|ERR_INVALID_URL|UNABLE_TO_GET_ISSUER_CERT|certificate has expired|unable to resolve|could not resolve host|socket hang up|connect ECONNREFUSED|connect ENOTFOUND)(?:\b|$)/im .test(output); } diff --git a/tests/ci-workflows.test.ts b/tests/ci-workflows.test.ts index 1057602f5c..a7700ef788 100644 --- a/tests/ci-workflows.test.ts +++ b/tests/ci-workflows.test.ts @@ -1972,6 +1972,8 @@ describe("doctor-gui-if-changed", () => { const { looksLikeDoctorInfraFailure } = await import("../scripts/doctor-gui-if-changed"); expect(looksLikeDoctorInfraFailure("npm ERR! network getaddrinfo ENOTFOUND registry.npmjs.org")).toBe(true); expect(looksLikeDoctorInfraFailure("All 2 issues\nBugs > 1 errors")).toBe(false); + // Findings copy can mention "network" without being an infra outage. + expect(looksLikeDoctorInfraFailure("Network requests > 1 errors")).toBe(false); }); test("DRY_RUN prints the run/skip decision without spawning the doctor", () => { From eb11a7f4e05ff75dc0d3cd41558905363c13c797 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Tue, 28 Jul 2026 05:36:44 +0200 Subject: [PATCH 5/9] fix(gui): bound polls and harden doctor prepush gate Restore manual timeout fallbacks via shared createBoundedFetch, raise doctor maxBuffer and fail on ENOBUFS, recognize ECONNRESET registry failures, and tighten action SHA pinning assertions. --- gui/src/bounded-fetch.ts | 27 +++++++++++++ .../components/MemoryObservabilityCard.tsx | 40 ++++++++----------- gui/src/pages/ClaudeDesktop.tsx | 15 ++++--- scripts/doctor-gui-if-changed.ts | 16 +++++++- tests/ci-workflows.test.ts | 3 +- 5 files changed, 70 insertions(+), 31 deletions(-) create mode 100644 gui/src/bounded-fetch.ts diff --git a/gui/src/bounded-fetch.ts b/gui/src/bounded-fetch.ts new file mode 100644 index 0000000000..428840dc72 --- /dev/null +++ b/gui/src/bounded-fetch.ts @@ -0,0 +1,27 @@ +/** + * 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" && "any" in AbortSignal && "timeout" in AbortSignal) { + 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), + }; +} diff --git a/gui/src/components/MemoryObservabilityCard.tsx b/gui/src/components/MemoryObservabilityCard.tsx index 0c02d566a2..d703f5262c 100644 --- a/gui/src/components/MemoryObservabilityCard.tsx +++ b/gui/src/components/MemoryObservabilityCard.tsx @@ -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 @@ -135,13 +136,6 @@ const DRAIN_TIMEOUT_S = 60; const RECONNECT_POLL_MS = 1500; const RECONNECT_GIVE_UP_MS = 120_000; -/** Bound a fetch with AbortSignal.timeout when available; else unmount-abort only. */ -function boundedSignal(controller: AbortController, ms: number): AbortSignal { - return typeof AbortSignal !== "undefined" && "any" in AbortSignal && "timeout" in AbortSignal - ? AbortSignal.any([controller.signal, AbortSignal.timeout(ms)]) - : controller.signal; -} - export default function MemoryObservabilityCard({ apiBase }: { apiBase: string }) { const { locale, t } = useI18n(); const [data, setData] = useState(null); @@ -170,20 +164,17 @@ 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. - // Prefer AbortSignal.timeout (no setTimeout to clean up); fall back to - // unmount-only abort when the browser lacks timeout/any. - const controller = new AbortController(); - activeController = controller; - const signal = boundedSignal(controller, 10_000); + 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) return; @@ -213,7 +204,8 @@ export default function MemoryObservabilityCard({ apiBase }: { apiBase: string } setUnavailable(true); } } finally { - if (activeController === controller) activeController = null; + bounded.clear(); + if (active === bounded) active = null; inFlight = false; } }; @@ -221,7 +213,8 @@ export default function MemoryObservabilityCard({ apiBase }: { apiBase: string } const interval = setInterval(() => void fetchMemory(), 5000); return () => { cancelled = true; - activeController?.abort(); + active?.controller.abort(); + active?.clear(); clearInterval(interval); }; }, [apiBase, restartPhase, restartFromPid]); @@ -230,15 +223,14 @@ export default function MemoryObservabilityCard({ apiBase }: { apiBase: string } if (restartPhase !== "reconnecting") return; let cancelled = false; let inFlight = false; - let activeController: AbortController | null = null; + let active: BoundedFetch | null = null; const started = Date.now(); const tick = () => { if (inFlight || cancelled) return; inFlight = true; - const controller = new AbortController(); - activeController = controller; - const signal = boundedSignal(controller, 5_000); - void fetch(`${apiBase}/healthz`, { cache: "no-store", signal }) + 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) { @@ -277,7 +269,8 @@ export default function MemoryObservabilityCard({ apiBase }: { apiBase: string } } }) .finally(() => { - if (activeController === controller) activeController = null; + bounded.clear(); + if (active === bounded) active = null; inFlight = false; }); }; @@ -285,7 +278,8 @@ export default function MemoryObservabilityCard({ apiBase }: { apiBase: string } const interval = setInterval(tick, RECONNECT_POLL_MS); return () => { cancelled = true; - activeController?.abort(); + active?.controller.abort(); + active?.clear(); clearInterval(interval); }; }, [apiBase, restartPhase, restartFromPid, t]); diff --git a/gui/src/pages/ClaudeDesktop.tsx b/gui/src/pages/ClaudeDesktop.tsx index 3944e77ac7..2eaae112c6 100644 --- a/gui/src/pages/ClaudeDesktop.tsx +++ b/gui/src/pages/ClaudeDesktop.tsx @@ -5,6 +5,7 @@ import { IconChevron } from "../icons"; import { EmptyState, Notice } from "../ui"; import { useT, type TFn, type TKey } from "../i18n/shared"; import { readJsonIfOk, readJsonOrThrow } from "../fetch-json"; +import { createBoundedFetch } from "../bounded-fetch"; const FAMILIES = ["opus", "fable", "sonnet", "haiku"] as const; type Family = typeof FAMILIES[number]; @@ -209,13 +210,13 @@ export default function ClaudeDesktop({ apiBase }: { apiBase: string }) { useEffect(() => { let cancelled = false; let inFlight = false; - let controller: AbortController | null = null; + let active: ReturnType | null = null; const poll = () => { if (inFlight) return; inFlight = true; - controller = new AbortController(); - const active = controller; - void fetch(`${apiBase}/api/claude-desktop/status`, { signal: active.signal }) + const bounded = createBoundedFetch(10_000); + active = bounded; + void fetch(`${apiBase}/api/claude-desktop/status`, { signal: bounded.signal }) .then((response) => readJsonIfOk(response)) .then((data) => { if (cancelled) return; @@ -223,7 +224,8 @@ export default function ClaudeDesktop({ apiBase }: { apiBase: string }) { }) .catch(() => { /* offline / older proxy / aborted */ }) .finally(() => { - if (controller === active) controller = null; + bounded.clear(); + if (active === bounded) active = null; inFlight = false; }); }; @@ -232,7 +234,8 @@ export default function ClaudeDesktop({ apiBase }: { apiBase: string }) { return () => { cancelled = true; clearInterval(timer); - controller?.abort(); + active?.controller.abort(); + active?.clear(); }; }, [apiBase]); diff --git a/scripts/doctor-gui-if-changed.ts b/scripts/doctor-gui-if-changed.ts index 103351cbca..2b32814c7d 100644 --- a/scripts/doctor-gui-if-changed.ts +++ b/scripts/doctor-gui-if-changed.ts @@ -21,10 +21,14 @@ export function guiPathsChanged(files: string[]): boolean { /** True when stderr/stdout looks like a registry/network failure rather than findings. */ export function looksLikeDoctorInfraFailure(output: string): boolean { // Keep this narrow: bare words like "network"/"offline" also appear in findings text. - return /(?:^|\b)(?:npm ERR!|ENOTFOUND|ECONNREFUSED|ETIMEDOUT|ENETUNREACH|EAI_AGAIN|getaddrinfo ENOTFOUND|fetch failed|ERR_INVALID_URL|UNABLE_TO_GET_ISSUER_CERT|certificate has expired|unable to resolve|could not resolve host|socket hang up|connect ECONNREFUSED|connect ENOTFOUND)(?:\b|$)/im + // `npm ERR!` deliberately has no trailing \b — `!` is non-word so `\b` after it never matches. + return /(?:^|\b)(?:npm ERR!|ENOTFOUND|ECONNRESET|ECONNREFUSED|ETIMEDOUT|ENETUNREACH|EAI_AGAIN|getaddrinfo ENOTFOUND|fetch failed|ERR_INVALID_URL|UNABLE_TO_GET_ISSUER_CERT|certificate has expired|unable to resolve|could not resolve host|socket hang up|connect ECONNREFUSED|connect ENOTFOUND|connect ECONNRESET)(?:\b|$)/im .test(output); } +/** Large enough for verbose doctor scans; overflows must not soft-skip the gate. */ +const DOCTOR_MAX_BUFFER = 20 * 1024 * 1024; + if (import.meta.main) { const repoRoot = resolve(import.meta.dirname, ".."); const guiDir = join(repoRoot, "gui"); @@ -86,6 +90,7 @@ if (import.meta.main) { cwd: guiDir, encoding: "utf8", stdio: ["inherit", "pipe", "pipe"], + maxBuffer: DOCTOR_MAX_BUFFER, env: { ...process.env, npm_config_yes: "true" }, }); if (out) process.stdout.write(out); @@ -93,6 +98,9 @@ if (import.meta.main) { const status = typeof err === "object" && err !== null && "status" in err ? (err as { status?: unknown }).status : undefined; + const code = typeof err === "object" && err !== null && "code" in err + ? String((err as { code?: unknown }).code ?? "") + : ""; const stdout = typeof err === "object" && err !== null && "stdout" in err ? String((err as { stdout?: unknown }).stdout ?? "") : ""; @@ -103,6 +111,12 @@ if (import.meta.main) { if (stdout) process.stdout.write(stdout); if (stderr) process.stderr.write(stderr); + // Buffer overflow means doctor ran and produced a large report — gate, do not soft-skip. + if (code === "ENOBUFS") { + console.error("doctor:gui: react-doctor output exceeded buffer — failing push"); + process.exit(1); + } + // Spawn failed (missing binary) — do not brick the push on infra. if (typeof status !== "number") { console.warn("doctor:gui: react-doctor unavailable (offline?) — skipping scan"); diff --git a/tests/ci-workflows.test.ts b/tests/ci-workflows.test.ts index a7700ef788..b41200243a 100644 --- a/tests/ci-workflows.test.ts +++ b/tests/ci-workflows.test.ts @@ -1918,7 +1918,7 @@ describe("GitHub Actions hardening", () => { expect(workflow).toContain("actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8"); expect(workflow).toContain("millionco/react-doctor@938008119a288f2fb47c66a69cd9279a21f31784"); expect(workflow).not.toMatch( - /^\s*-\s+uses:\s+\S+@(?![0-9a-f]{40}\b)\S+/m, + /^\s*-\s+uses:\s+\S+@(?![0-9a-f]{40}(?=[ \t]*(?:#.*)?$))\S+/m, ); // Engine pin: the action wrapper would fetch react-doctor@latest without it. @@ -1971,6 +1971,7 @@ describe("doctor-gui-if-changed", () => { test("looksLikeDoctorInfraFailure detects registry/network outages", async () => { const { looksLikeDoctorInfraFailure } = await import("../scripts/doctor-gui-if-changed"); expect(looksLikeDoctorInfraFailure("npm ERR! network getaddrinfo ENOTFOUND registry.npmjs.org")).toBe(true); + expect(looksLikeDoctorInfraFailure("npm ERR! code ECONNRESET")).toBe(true); expect(looksLikeDoctorInfraFailure("All 2 issues\nBugs > 1 errors")).toBe(false); // Findings copy can mention "network" without being an infra outage. expect(looksLikeDoctorInfraFailure("Network requests > 1 errors")).toBe(false); From d66b0cdc0d0769b215acf22c16632bb609d1d7b3 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Tue, 28 Jul 2026 05:46:10 +0200 Subject: [PATCH 6/9] fix(gui): match nonterminal npm ERR! soft-skip signatures Give npm ERR! its own alternative so trailing whitespace after the bang still counts as an offline/registry failure instead of gating the push. --- scripts/doctor-gui-if-changed.ts | 4 ++-- tests/ci-workflows.test.ts | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/doctor-gui-if-changed.ts b/scripts/doctor-gui-if-changed.ts index 2b32814c7d..2933e7224c 100644 --- a/scripts/doctor-gui-if-changed.ts +++ b/scripts/doctor-gui-if-changed.ts @@ -21,8 +21,8 @@ export function guiPathsChanged(files: string[]): boolean { /** True when stderr/stdout looks like a registry/network failure rather than findings. */ export function looksLikeDoctorInfraFailure(output: string): boolean { // Keep this narrow: bare words like "network"/"offline" also appear in findings text. - // `npm ERR!` deliberately has no trailing \b — `!` is non-word so `\b` after it never matches. - return /(?:^|\b)(?:npm ERR!|ENOTFOUND|ECONNRESET|ECONNREFUSED|ETIMEDOUT|ENETUNREACH|EAI_AGAIN|getaddrinfo ENOTFOUND|fetch failed|ERR_INVALID_URL|UNABLE_TO_GET_ISSUER_CERT|certificate has expired|unable to resolve|could not resolve host|socket hang up|connect ECONNREFUSED|connect ENOTFOUND|connect ECONNRESET)(?:\b|$)/im + // `npm ERR!` is matched on its own — `!` is non-word, so a shared trailing `\b` never fires after it. + return /npm ERR!|(?:^|\b)(?:ENOTFOUND|ECONNRESET|ECONNREFUSED|ETIMEDOUT|ENETUNREACH|EAI_AGAIN|getaddrinfo ENOTFOUND|fetch failed|ERR_INVALID_URL|UNABLE_TO_GET_ISSUER_CERT|certificate has expired|unable to resolve|could not resolve host|socket hang up|connect ECONNREFUSED|connect ENOTFOUND|connect ECONNRESET)(?:\b|$)/im .test(output); } diff --git a/tests/ci-workflows.test.ts b/tests/ci-workflows.test.ts index b41200243a..0e824c1352 100644 --- a/tests/ci-workflows.test.ts +++ b/tests/ci-workflows.test.ts @@ -1972,6 +1972,7 @@ describe("doctor-gui-if-changed", () => { const { looksLikeDoctorInfraFailure } = await import("../scripts/doctor-gui-if-changed"); expect(looksLikeDoctorInfraFailure("npm ERR! network getaddrinfo ENOTFOUND registry.npmjs.org")).toBe(true); expect(looksLikeDoctorInfraFailure("npm ERR! code ECONNRESET")).toBe(true); + expect(looksLikeDoctorInfraFailure("npm ERR! network timeout")).toBe(true); expect(looksLikeDoctorInfraFailure("All 2 issues\nBugs > 1 errors")).toBe(false); // Findings copy can mention "network" without being an infra outage. expect(looksLikeDoctorInfraFailure("Network requests > 1 errors")).toBe(false); From e80dbca33fd086c36532ba43efa4733a76c17524 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Tue, 28 Jul 2026 05:57:42 +0200 Subject: [PATCH 7/9] fix(gui): treat stdio maxBuffer overflows as doctor gate failures Also recognize Node's ERR_CHILD_PROCESS_STDIO_MAXBUFFER so oversized doctor output cannot soft-skip the prepush findings gate. --- scripts/doctor-gui-if-changed.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/doctor-gui-if-changed.ts b/scripts/doctor-gui-if-changed.ts index 2933e7224c..5f40b57fe5 100644 --- a/scripts/doctor-gui-if-changed.ts +++ b/scripts/doctor-gui-if-changed.ts @@ -112,7 +112,7 @@ if (import.meta.main) { if (stderr) process.stderr.write(stderr); // Buffer overflow means doctor ran and produced a large report — gate, do not soft-skip. - if (code === "ENOBUFS") { + if (code === "ENOBUFS" || code === "ERR_CHILD_PROCESS_STDIO_MAXBUFFER") { console.error("doctor:gui: react-doctor output exceeded buffer — failing push"); process.exit(1); } From b9123ca205bba16c35eb834962b50d3d090a130b Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Tue, 28 Jul 2026 06:15:14 +0200 Subject: [PATCH 8/9] fix(gui): run React Doctor via spawnSync with explicit maxBuffer Replace execFileSync so prepush classification uses spawnSync status/error channels and cannot soft-skip oversized doctor output. --- scripts/doctor-gui-if-changed.ts | 103 ++++++++++++++++--------------- 1 file changed, 53 insertions(+), 50 deletions(-) diff --git a/scripts/doctor-gui-if-changed.ts b/scripts/doctor-gui-if-changed.ts index 5f40b57fe5..8e488a1328 100644 --- a/scripts/doctor-gui-if-changed.ts +++ b/scripts/doctor-gui-if-changed.ts @@ -10,7 +10,7 @@ * DOCTOR_FILES (newline-separated) overrides git-derived changed files; * DOCTOR_CMD overrides the spawned command (offline-degradation testing). */ -import { execFileSync } from "node:child_process"; +import { spawnSync } from "node:child_process"; import { join, resolve } from "node:path"; /** True when any changed path is the gui directory or inside it (slash-guarded). */ @@ -27,7 +27,11 @@ export function looksLikeDoctorInfraFailure(output: string): boolean { } /** Large enough for verbose doctor scans; overflows must not soft-skip the gate. */ -const DOCTOR_MAX_BUFFER = 20 * 1024 * 1024; +export const DOCTOR_MAX_BUFFER = 20 * 1024 * 1024; + +function isBufferOverflow(code: string | undefined): boolean { + return code === "ENOBUFS" || code === "ERR_CHILD_PROCESS_STDIO_MAXBUFFER"; +} if (import.meta.main) { const repoRoot = resolve(import.meta.dirname, ".."); @@ -35,8 +39,11 @@ if (import.meta.main) { const hasRef = (ref: string): boolean => { try { - execFileSync("git", ["rev-parse", "--verify", ref], { cwd: repoRoot, stdio: "ignore" }); - return true; + const probe = spawnSync("git", ["rev-parse", "--verify", ref], { + cwd: repoRoot, + stdio: "ignore", + }); + return probe.status === 0; } catch { return false; } @@ -44,7 +51,12 @@ if (import.meta.main) { const diffNames = (range: string): string[] => { try { - return execFileSync("git", ["diff", "--name-only", range], { cwd: repoRoot, encoding: "utf8" }) + const diff = spawnSync("git", ["diff", "--name-only", range], { + cwd: repoRoot, + encoding: "utf8", + }); + if (diff.status !== 0) return []; + return (diff.stdout ?? "") .split(/\r?\n/) .map(line => line.trim()) .filter(Boolean); @@ -83,53 +95,44 @@ if (import.meta.main) { const [cmd, ...args] = process.env.DOCTOR_CMD ? process.env.DOCTOR_CMD.split(" ") : ["bun", "run", "doctor"]; - try { - // Pipe stdout/stderr so we can distinguish findings from offline/registry - // failures, then replay them so the developer still sees the doctor output. - const out = execFileSync(cmd!, args, { - cwd: guiDir, - encoding: "utf8", - stdio: ["inherit", "pipe", "pipe"], - maxBuffer: DOCTOR_MAX_BUFFER, - env: { ...process.env, npm_config_yes: "true" }, - }); - if (out) process.stdout.write(out); - } catch (err) { - const status = typeof err === "object" && err !== null && "status" in err - ? (err as { status?: unknown }).status - : undefined; - const code = typeof err === "object" && err !== null && "code" in err - ? String((err as { code?: unknown }).code ?? "") - : ""; - const stdout = typeof err === "object" && err !== null && "stdout" in err - ? String((err as { stdout?: unknown }).stdout ?? "") - : ""; - const stderr = typeof err === "object" && err !== null && "stderr" in err - ? String((err as { stderr?: unknown }).stderr ?? "") - : ""; - const combined = `${stdout}\n${stderr}`; - if (stdout) process.stdout.write(stdout); - if (stderr) process.stderr.write(stderr); - - // Buffer overflow means doctor ran and produced a large report — gate, do not soft-skip. - if (code === "ENOBUFS" || code === "ERR_CHILD_PROCESS_STDIO_MAXBUFFER") { - console.error("doctor:gui: react-doctor output exceeded buffer — failing push"); - process.exit(1); - } - // Spawn failed (missing binary) — do not brick the push on infra. - if (typeof status !== "number") { - console.warn("doctor:gui: react-doctor unavailable (offline?) — skipping scan"); - process.exit(0); - } + // spawnSync (not execFileSync): explicit maxBuffer + status/error channels so + // oversized doctor output cannot be mistaken for an offline soft-skip. + const result = spawnSync(cmd!, args, { + cwd: guiDir, + encoding: "utf8", + maxBuffer: DOCTOR_MAX_BUFFER, + env: { ...process.env, npm_config_yes: "true" }, + }); - // Doctor started but npx/registry failed — same soft-skip contract. - if (looksLikeDoctorInfraFailure(combined)) { - console.warn("doctor:gui: react-doctor unavailable (offline?) — skipping scan"); - process.exit(0); - } + const stdout = result.stdout ?? ""; + const stderr = result.stderr ?? ""; + if (stdout) process.stdout.write(stdout); + if (stderr) process.stderr.write(stderr); - // Real findings (or other doctor/engine errors) gate the push. - process.exit(status === 0 ? 1 : status); + const overflowCode = result.error && "code" in result.error + ? String((result.error as NodeJS.ErrnoException).code ?? "") + : ""; + if (isBufferOverflow(overflowCode)) { + console.error("doctor:gui: react-doctor output exceeded buffer — failing push"); + process.exit(1); } + + // Spawn failed (missing binary) — do not brick the push on infra. + if (result.error && typeof result.status !== "number") { + console.warn("doctor:gui: react-doctor unavailable (offline?) — skipping scan"); + process.exit(0); + } + + if (result.status === 0) process.exit(0); + + const combined = `${stdout}\n${stderr}`; + // Doctor started but npx/registry failed — same soft-skip contract. + if (looksLikeDoctorInfraFailure(combined)) { + console.warn("doctor:gui: react-doctor unavailable (offline?) — skipping scan"); + process.exit(0); + } + + // Real findings (or other doctor/engine errors) gate the push. + process.exit(result.status === null || result.status === 0 ? 1 : result.status); } From 2ee77cdfc379cc0f4951369823a57c4f4c50035c Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Tue, 28 Jul 2026 06:37:04 +0200 Subject: [PATCH 9/9] test(gui): cover bounded fetch and doctor maxBuffer hard-fail Add regression tests CodeRabbit asked for, and check AbortSignal helpers with typeof so the manual timeout fallback actually runs when any/timeout are missing. --- gui/src/bounded-fetch.ts | 6 +++- gui/tests/bounded-fetch.test.ts | 46 ++++++++++++++++++++++++++ scripts/doctor-gui-if-changed.ts | 17 +++++++--- scripts/fixtures/doctor-huge-output.ts | 3 ++ tests/ci-workflows.test.ts | 22 ++++++++++++ 5 files changed, 89 insertions(+), 5 deletions(-) create mode 100644 gui/tests/bounded-fetch.test.ts create mode 100644 scripts/fixtures/doctor-huge-output.ts diff --git a/gui/src/bounded-fetch.ts b/gui/src/bounded-fetch.ts index 428840dc72..bec2d83edf 100644 --- a/gui/src/bounded-fetch.ts +++ b/gui/src/bounded-fetch.ts @@ -11,7 +11,11 @@ export type BoundedFetch = { export function createBoundedFetch(ms: number): BoundedFetch { const controller = new AbortController(); - if (typeof AbortSignal !== "undefined" && "any" in AbortSignal && "timeout" in AbortSignal) { + if ( + typeof AbortSignal !== "undefined" + && typeof AbortSignal.any === "function" + && typeof AbortSignal.timeout === "function" + ) { return { controller, signal: AbortSignal.any([controller.signal, AbortSignal.timeout(ms)]), diff --git a/gui/tests/bounded-fetch.test.ts b/gui/tests/bounded-fetch.test.ts new file mode 100644 index 0000000000..0685897b28 --- /dev/null +++ b/gui/tests/bounded-fetch.test.ts @@ -0,0 +1,46 @@ +import { expect, test } from "bun:test"; +import { createBoundedFetch } from "../src/bounded-fetch"; + +test("createBoundedFetch aborts when the controller is aborted (native path)", () => { + const bounded = createBoundedFetch(60_000); + expect(bounded.signal.aborted).toBe(false); + bounded.controller.abort(); + expect(bounded.signal.aborted).toBe(true); + bounded.clear(); +}); + +test("createBoundedFetch aborts after the timeout on the compatibility path", async () => { + const originalAny = Object.getOwnPropertyDescriptor(AbortSignal, "any"); + const originalTimeout = Object.getOwnPropertyDescriptor(AbortSignal, "timeout"); + // Force the manual setTimeout fallback so we exercise the timer path. + Object.defineProperty(AbortSignal, "any", { value: undefined, configurable: true }); + Object.defineProperty(AbortSignal, "timeout", { value: undefined, configurable: true }); + + try { + const bounded = createBoundedFetch(20); + expect(bounded.signal.aborted).toBe(false); + await new Promise(resolve => setTimeout(resolve, 50)); + expect(bounded.signal.aborted).toBe(true); + bounded.clear(); + } finally { + if (originalAny) Object.defineProperty(AbortSignal, "any", originalAny); + if (originalTimeout) Object.defineProperty(AbortSignal, "timeout", originalTimeout); + } +}); + +test("createBoundedFetch clear cancels a pending manual timeout", async () => { + const originalAny = Object.getOwnPropertyDescriptor(AbortSignal, "any"); + const originalTimeout = Object.getOwnPropertyDescriptor(AbortSignal, "timeout"); + Object.defineProperty(AbortSignal, "any", { value: undefined, configurable: true }); + Object.defineProperty(AbortSignal, "timeout", { value: undefined, configurable: true }); + + try { + const bounded = createBoundedFetch(100); + bounded.clear(); + await new Promise(resolve => setTimeout(resolve, 150)); + expect(bounded.signal.aborted).toBe(false); + } finally { + if (originalAny) Object.defineProperty(AbortSignal, "any", originalAny); + if (originalTimeout) Object.defineProperty(AbortSignal, "timeout", originalTimeout); + } +}); diff --git a/scripts/doctor-gui-if-changed.ts b/scripts/doctor-gui-if-changed.ts index 8e488a1328..b341e0eab2 100644 --- a/scripts/doctor-gui-if-changed.ts +++ b/scripts/doctor-gui-if-changed.ts @@ -8,7 +8,8 @@ * * Test hooks: DOCTOR_DRY_RUN=1 prints the run/skip decision without spawning; * DOCTOR_FILES (newline-separated) overrides git-derived changed files; - * DOCTOR_CMD overrides the spawned command (offline-degradation testing). + * DOCTOR_CMD overrides the spawned command (offline-degradation testing); + * OCX_DOCTOR_MAX_BUFFER overrides spawnSync maxBuffer (overflow hard-fail testing). */ import { spawnSync } from "node:child_process"; import { join, resolve } from "node:path"; @@ -29,10 +30,18 @@ export function looksLikeDoctorInfraFailure(output: string): boolean { /** Large enough for verbose doctor scans; overflows must not soft-skip the gate. */ export const DOCTOR_MAX_BUFFER = 20 * 1024 * 1024; -function isBufferOverflow(code: string | undefined): boolean { +/** True when spawnSync failed because child stdout/stderr exceeded maxBuffer. */ +export function isDoctorBufferOverflow(code: string | undefined): boolean { return code === "ENOBUFS" || code === "ERR_CHILD_PROCESS_STDIO_MAXBUFFER"; } +function resolveMaxBuffer(): number { + const raw = process.env.OCX_DOCTOR_MAX_BUFFER; + if (raw === undefined || raw === "") return DOCTOR_MAX_BUFFER; + const n = Number(raw); + return Number.isFinite(n) && n > 0 ? Math.floor(n) : DOCTOR_MAX_BUFFER; +} + if (import.meta.main) { const repoRoot = resolve(import.meta.dirname, ".."); const guiDir = join(repoRoot, "gui"); @@ -101,7 +110,7 @@ if (import.meta.main) { const result = spawnSync(cmd!, args, { cwd: guiDir, encoding: "utf8", - maxBuffer: DOCTOR_MAX_BUFFER, + maxBuffer: resolveMaxBuffer(), env: { ...process.env, npm_config_yes: "true" }, }); @@ -113,7 +122,7 @@ if (import.meta.main) { const overflowCode = result.error && "code" in result.error ? String((result.error as NodeJS.ErrnoException).code ?? "") : ""; - if (isBufferOverflow(overflowCode)) { + if (isDoctorBufferOverflow(overflowCode)) { console.error("doctor:gui: react-doctor output exceeded buffer — failing push"); process.exit(1); } diff --git a/scripts/fixtures/doctor-huge-output.ts b/scripts/fixtures/doctor-huge-output.ts new file mode 100644 index 0000000000..6ab4c3cea8 --- /dev/null +++ b/scripts/fixtures/doctor-huge-output.ts @@ -0,0 +1,3 @@ +// Emit more than a tiny OCX_DOCTOR_MAX_BUFFER so the prepush wrapper hard-fails. +process.stdout.write("x".repeat(64 * 1024)); +process.exit(0); diff --git a/tests/ci-workflows.test.ts b/tests/ci-workflows.test.ts index 0e824c1352..ec95bc200e 100644 --- a/tests/ci-workflows.test.ts +++ b/tests/ci-workflows.test.ts @@ -2029,4 +2029,26 @@ describe("doctor-gui-if-changed", () => { }); expect(run.exitCode).not.toBe(0); }); + + test("isDoctorBufferOverflow recognizes ENOBUFS / maxBuffer errors", async () => { + const { isDoctorBufferOverflow } = await import("../scripts/doctor-gui-if-changed"); + expect(isDoctorBufferOverflow("ENOBUFS")).toBe(true); + expect(isDoctorBufferOverflow("ERR_CHILD_PROCESS_STDIO_MAXBUFFER")).toBe(true); + expect(isDoctorBufferOverflow("ENOENT")).toBe(false); + expect(isDoctorBufferOverflow(undefined)).toBe(false); + }); + + test("hard-fails when doctor output exceeds maxBuffer (does not soft-skip)", () => { + const run = Bun.spawnSync(["bun", doctorGuiIfChangedScript], { + env: { + ...process.env, + DOCTOR_FILES: "gui/src/App.tsx", + DOCTOR_CMD: "bun ../scripts/fixtures/doctor-huge-output.ts", + // Tiny buffer so the fixture's stdout trips the overflow branch. + OCX_DOCTOR_MAX_BUFFER: "256", + }, + }); + expect(run.exitCode).not.toBe(0); + expect(run.stderr.toString()).toContain("exceeded buffer"); + }); });