Skip to content

Commit 898cf79

Browse files
committed
fix(gui): keep seed revalidate across StrictMode and harden revisit UX
Clear seedNeedsRevalidate only after a settled fetch so aborted mounts still revalidate, stabilize the Claude Desktop port callback, and validate/announce Models combo cache failures.
1 parent 8d4122a commit 898cf79

5 files changed

Lines changed: 101 additions & 19 deletions

File tree

gui/src/client-resource.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,8 @@ async function runFetch<T>(
230230
try {
231231
const data = await fetcher(controller.signal);
232232
if (gen !== store.generation || controller.signal.aborted) return;
233+
// Cleared on settle (not at subscribe) so StrictMode's aborted first mount still revalidates.
234+
store.seedNeedsRevalidate = false;
233235
store.snapshot = {
234236
data,
235237
error: undefined,
@@ -240,6 +242,7 @@ async function runFetch<T>(
240242
};
241243
} catch (error) {
242244
if (gen !== store.generation || controller.signal.aborted) return;
245+
store.seedNeedsRevalidate = false;
243246
store.snapshot = {
244247
...store.snapshot,
245248
// Normalize so a loader that rejects with `undefined` still reads as a failure.
@@ -309,7 +312,6 @@ function subscribeResource<T>(
309312
// cached data across transient 0→1 resubscribe gaps when neither applies.
310313
if (store.subscriberCount === 1) {
311314
if (store.snapshot.data === undefined || store.seedNeedsRevalidate) {
312-
store.seedNeedsRevalidate = false;
313315
void runFetch(store, fetcher, { replaceInflight: true, owner: onStoreChange });
314316
}
315317
}

gui/src/pages/Claude.tsx

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useRef, useState, type KeyboardEvent } from "react";
1+
import { useCallback, useRef, useState, type KeyboardEvent } from "react";
22
import ClaudeCode from "./ClaudeCode";
33
import ClaudeDesktop from "./ClaudeDesktop";
44
import { useT } from "../i18n/shared";
@@ -22,7 +22,14 @@ export default function Claude({ apiBase }: { apiBase: string }) {
2222
const seededDesktopPort = readCachedDesktopPort(apiBase);
2323
const [liveDesktopPort, setLiveDesktopPort] = useState<{ base: string; port: number | null } | null>(null);
2424
const desktopPort = liveDesktopPort?.base === apiBase ? liveDesktopPort.port : seededDesktopPort;
25-
const setDesktopPort = (port: number | null) => setLiveDesktopPort({ base: apiBase, port });
25+
const desktopSettled = liveDesktopPort?.base === apiBase;
26+
// Skip no-op writes: an unstable callback + always-new object would loop with Desktop's effect.
27+
const setDesktopPort = useCallback((port: number | null) => {
28+
setLiveDesktopPort((prev) => {
29+
if (prev?.base === apiBase && prev.port === port) return prev;
30+
return { base: apiBase, port };
31+
});
32+
}, [apiBase]);
2633

2734
const selectTab = (next: ClaudeTab) => {
2835
setTab(next);
@@ -59,7 +66,9 @@ export default function Claude({ apiBase }: { apiBase: string }) {
5966
<p className="page-sub">
6067
{desktopPort != null
6168
? t("claudeDesktop.subtitle", { port: desktopPort })
62-
: t("claudeDesktop.loading")}
69+
: desktopSettled
70+
? t("claudeDesktop.loadFail")
71+
: t("claudeDesktop.loading")}
6372
</p>
6473
)}
6574
</div>

gui/src/pages/ClaudeDesktop.tsx

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,8 @@ export default function ClaudeDesktop({
150150
}: {
151151
apiBase: string;
152152
active?: boolean;
153-
/** Keeps the Claude page intro subtitle in sync once /api/claude-desktop resolves a port. */
154-
onPortChange?: (port: number) => void;
153+
/** Keeps the Claude page intro subtitle in sync once /api/claude-desktop settles (port or failure). */
154+
onPortChange?: (port: number | null) => void;
155155
}) {
156156
const { t, locale } = useI18n();
157157
const localeTag = LOCALES.find(l => l.code === locale)?.htmlLang;
@@ -225,8 +225,14 @@ export default function ClaudeDesktop({
225225
const destinations = Object.keys(draftDestinations).length > 0 ? draftDestinations : resourceDestinations;
226226

227227
useEffect(() => {
228-
if (typeof data?.port === "number") onPortChange?.(data.port);
229-
}, [data?.port, onPortChange]);
228+
if (!onPortChange) return;
229+
if (typeof data?.port === "number") {
230+
onPortChange(data.port);
231+
return;
232+
}
233+
// Cold failure with no port: stop the parent subtitle from claiming "Loading…" forever.
234+
if (loadState.kind === "failed-cold") onPortChange(null);
235+
}, [data?.port, loadState.kind, onPortChange]);
230236

231237
const dirty = useMemo(
232238
() => profile !== null && savedProfile !== null && JSON.stringify(profile) !== JSON.stringify(savedProfile),

gui/src/pages/Models.tsx

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@ type CachedModelsPage = {
5656
contextCapValue: number;
5757
};
5858

59+
/** Session JSON is untrusted — only seed rows that survive parseComboList (targets always arrays). */
60+
function readCachedCombos(value: unknown): ComboItem[] | null {
61+
if (!Array.isArray(value)) return null;
62+
return parseComboList({ combos: value });
63+
}
64+
5965
export default function Models({ apiBase }: { apiBase: string }) {
6066
const t: TFn = useT();
6167
const cacheKey = `ocx.models.catalog.v1:${apiBase}`;
@@ -106,11 +112,11 @@ export default function Models({ apiBase }: { apiBase: string }) {
106112
// null + combosError so an API error never masquerades as "no combos configured".
107113
const combosCacheKey = `ocx.models.combos.v1:${apiBase}`;
108114
const seededCombos = useMemo(() => {
109-
const own = readSessionListCache<ComboItem[]>(combosCacheKey);
110-
if (own) return own;
115+
const own = readCachedCombos(readSessionListCache<unknown>(combosCacheKey));
116+
if (own !== null) return own;
111117
// Reuse the Combos workspace session snapshot when Models opens first in the session.
112-
const workspace = readSessionListCache<{ combos?: ComboItem[] }>(`ocx.combos.workspace.v1:${apiBase}`);
113-
return Array.isArray(workspace?.combos) ? workspace.combos : null;
118+
const workspace = readSessionListCache<{ combos?: unknown }>(`ocx.combos.workspace.v1:${apiBase}`);
119+
return readCachedCombos(workspace?.combos);
114120
}, [apiBase, combosCacheKey]);
115121
const combosResource = useDataSurface<ComboItem[]>(
116122
`models-combos:${apiBase}`,
@@ -127,7 +133,8 @@ export default function Models({ apiBase }: { apiBase: string }) {
127133
const combosState = combosResource.state;
128134
// Keep a previously painted card on a later failure so the catalog does not yank down.
129135
const combos = combosState.data ?? seededCombos;
130-
const combosError = combosState.showError && combos === null;
136+
// Announce failures even when stale/seeded rows remain (layout kept; freshness not faked).
137+
const combosError = combosState.showError;
131138
const [combosOpen, setCombosOpen] = useState(readCombosOpen);
132139

133140
// App owns the in-session view mode; fallback to persisted mode for isolated renders/tests.
@@ -1053,23 +1060,35 @@ export default function Models({ apiBase }: { apiBase: string }) {
10531060
<strong>{t("nav.combos")}</strong>
10541061
<span className="muted text-label" role="alert">{t("models.loadFail")}</span>
10551062
</div>
1056-
<a className="btn btn-sm" href="#combos" style={{ flexShrink: 0 }}>{t("models.combosSetup")}</a>
1063+
<button type="button" className="btn btn-sm" style={{ flexShrink: 0 }} onClick={() => combosResource.refresh()}>
1064+
{t("common.retry")}
1065+
</button>
10571066
</div>
10581067
</div>
10591068
)}
1060-
{combos !== null && !combosError && combos.length === 0 && (
1069+
{combos !== null && combos.length === 0 && (
10611070
<div className="card models-combos-card">
10621071
<div className="row models-combos-empty-head">
10631072
<div className="row models-field-row" style={{ minWidth: 0 }}>
10641073
<IconShuffle width={14} height={14} aria-hidden="true" style={{ flexShrink: 0 }} />
10651074
<strong>{t("nav.combos")}</strong>
1066-
<span className="muted text-label">{t("models.combosEmpty")}</span>
1075+
{combosError ? (
1076+
<span className="muted text-label" role="alert">{t("models.loadFail")}</span>
1077+
) : (
1078+
<span className="muted text-label">{t("models.combosEmpty")}</span>
1079+
)}
10671080
</div>
1068-
<a className="btn btn-sm" href="#combos" style={{ flexShrink: 0 }}>{t("models.combosSetup")}</a>
1081+
{combosError ? (
1082+
<button type="button" className="btn btn-sm" style={{ flexShrink: 0 }} onClick={() => combosResource.refresh()}>
1083+
{t("common.retry")}
1084+
</button>
1085+
) : (
1086+
<a className="btn btn-sm" href="#combos" style={{ flexShrink: 0 }}>{t("models.combosSetup")}</a>
1087+
)}
10691088
</div>
10701089
</div>
10711090
)}
1072-
{combos !== null && !combosError && combos.length > 0 && (
1091+
{combos !== null && combos.length > 0 && (
10731092
<div className="card models-combos-card">
10741093
<div className={`row group-head models-field-row${combosOpen ? " open" : ""}`}>
10751094
<button
@@ -1083,8 +1102,17 @@ export default function Models({ apiBase }: { apiBase: string }) {
10831102
<IconShuffle width={14} height={14} aria-hidden="true" style={{ flexShrink: 0 }} />
10841103
<strong>{t("nav.combos")}</strong>
10851104
<span className="muted mono text-label">{t("models.combosActive", { count: combos.length })}</span>
1105+
{combosError && (
1106+
<span className="muted text-label" role="alert">{t("models.loadFail")}</span>
1107+
)}
10861108
</button>
1087-
<a className="btn btn-sm btn-ghost" href="#combos" style={{ flexShrink: 0 }}>{t("models.combosSetup")}</a>
1109+
{combosError ? (
1110+
<button type="button" className="btn btn-sm btn-ghost" style={{ flexShrink: 0 }} onClick={() => combosResource.refresh()}>
1111+
{t("common.retry")}
1112+
</button>
1113+
) : (
1114+
<a className="btn btn-sm btn-ghost" href="#combos" style={{ flexShrink: 0 }}>{t("models.combosSetup")}</a>
1115+
)}
10881116
</div>
10891117
{combosOpen && (
10901118
<div>

gui/tests/client-resource-poll.test.tsx

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,3 +591,40 @@ test("pre-subscribe seed still quiet-revalidates on first subscribe", async () =
591591
});
592592
container.remove();
593593
});
594+
595+
test("StrictMode remount still quiet-revalidates a pre-subscribe seed", async () => {
596+
const { createRoot } = await import("react-dom/client");
597+
const { StrictMode } = await import("react");
598+
const container = document.createElement("div");
599+
document.body.append(container);
600+
601+
const KEY = `seed-strictmode-${Date.now()}`;
602+
let fetches = 0;
603+
setClientResourceData(KEY, "seeded");
604+
605+
function Page() {
606+
const resource = useClientResource(KEY, async () => {
607+
fetches += 1;
608+
return "fresh";
609+
});
610+
return <span>{resource.data ?? ""}</span>;
611+
}
612+
613+
let root!: Root;
614+
await act(async () => {
615+
root = createRoot(container);
616+
root.render(
617+
<StrictMode>
618+
<Page />
619+
</StrictMode>,
620+
);
621+
});
622+
// Dev StrictMode aborts the first subscribe's fetch; the remount must still revalidate.
623+
await waitFor(() => container.textContent === "fresh");
624+
expect(fetches).toBeGreaterThanOrEqual(1);
625+
626+
await act(async () => {
627+
root.unmount();
628+
});
629+
container.remove();
630+
});

0 commit comments

Comments
 (0)