Skip to content

Commit 4c7a8ea

Browse files
committed
fix(gui): clear the two React Doctor findings blocking the push gate
Both live in commits that are local-only, which is why origin/dev passes the same gate and this tree did not. Logs kept a `pollFailing` boolean in sync with the settlement state via an effect, so every settle painted one frame with the stale banner and the deps needed `logsState.error` purely to make repeated failures count once. It is now derived: the streak is stored keyed by the error identity that produced it, so re-renders of the same failure do not inflate the count and a success clears it. (An intermediate attempt counted in a ref during render, which traded this rule for no-ref-current-in-render - the keyed-state form is the one React actually endorses.) The Anthropic pool loader was already safe - `cancelled` guard plus an AbortController - but its async/await shape put the setters after an await where static analysis cannot see the guard. Rewritten as a promise chain so each setter sits in a `.then` the checker can verify. Behaviour is unchanged. gui typecheck clean; gui doctor exit 0; gui test 427 pass 0 fail.
1 parent 4c8bc61 commit 4c7a8ea

2 files changed

Lines changed: 33 additions & 32 deletions

File tree

gui/src/components/provider-workspace/AnthropicAccountPoolSettings.tsx

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -39,41 +39,43 @@ export default function AnthropicAccountPoolSettings({
3939
useEffect(() => {
4040
let cancelled = false;
4141
const ac = new AbortController();
42-
const load = async () => {
43-
try {
44-
const res = await fetch(`${apiBase}/api/oauth/accounts/pool?provider=anthropic`, {
45-
signal: ac.signal,
46-
});
42+
// Promise chain rather than async/await: every setter then lives in a `.then`
43+
// callback guarded by the same `cancelled` flag, which is the shape static analysis
44+
// (react-doctor no-set-state-after-await-in-effect) can actually verify. The
45+
// behaviour is unchanged — the guard and the abort controller were already here.
46+
//
47+
// Deferred by a microtask, not a timer: a timer had to be cancelled in cleanup, so a
48+
// mount-then-unmount dropped the request entirely. The abort controller already covers
49+
// in-flight cancellation, which is the part that actually needs to be cancellable.
50+
void Promise.resolve()
51+
.then(() => fetch(`${apiBase}/api/oauth/accounts/pool?provider=anthropic`, { signal: ac.signal }))
52+
.then(res => {
4753
if (!res.ok) throw new Error("load");
48-
const json = await res.json() as {
54+
return res.json() as Promise<{
4955
enabled?: boolean;
5056
autoSwitchThreshold?: number;
5157
strategy?: unknown;
5258
stickyLimit?: unknown;
53-
};
59+
}>;
60+
})
61+
.then(json => {
5462
if (cancelled) return;
55-
const nextEnabled = json.enabled === true;
5663
const nextThreshold = typeof json.autoSwitchThreshold === "number" ? json.autoSwitchThreshold : 80;
57-
const nextStrategy = normalizeAccountPoolStrategy(json.strategy);
5864
const nextSticky = normalizeAccountPoolStickyLimit(json.stickyLimit);
5965
setState({
60-
enabled: nextEnabled,
66+
enabled: json.enabled === true,
6167
threshold: nextThreshold,
62-
strategy: nextStrategy,
68+
strategy: normalizeAccountPoolStrategy(json.strategy),
6369
stickyLimit: nextSticky,
6470
});
6571
setDraft(String(nextThreshold));
6672
setStickyDraft(String(nextSticky));
6773
setLoadError(false);
68-
} catch {
74+
})
75+
.catch(() => {
6976
if (cancelled || ac.signal.aborted) return;
7077
setLoadError(true);
71-
}
72-
};
73-
// Deferred by a microtask, not a timer: a timer had to be cancelled in cleanup, so a
74-
// mount-then-unmount dropped the request entirely. The abort controller below already covers
75-
// in-flight cancellation, which is the part that actually needs to be cancellable.
76-
void Promise.resolve().then(() => { void load(); });
78+
});
7779
return () => {
7880
cancelled = true;
7981
ac.abort();

gui/src/pages/Logs.tsx

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -391,22 +391,21 @@ export default function Logs({ apiBase }: { apiBase: string }) {
391391
// A single failed tick on a two-second poll is noise, but an outage that never recovers must not
392392
// leave the user reading stale rows as if they were current. Count consecutive failures and speak
393393
// up once it is clearly not transient.
394-
const [pollFailing, setPollFailing] = useState(false);
395-
const failureStreakRef = useRef(0);
396394
const settledFailure = !logsResource.refreshing && logsState.showError;
397395
const settledSuccess = !logsResource.refreshing && !logsState.showError && logsState.data !== undefined;
398-
useEffect(() => {
399-
if (settledSuccess) {
400-
failureStreakRef.current = 0;
401-
setPollFailing(false);
402-
return;
403-
}
404-
if (!settledFailure) return;
405-
failureStreakRef.current += 1;
406-
if (failureStreakRef.current >= STALE_POLL_FAILURE_LIMIT) setPollFailing(true);
407-
// `logsState.error` is in the deps so each new failed settlement counts once, rather than the
408-
// effect re-running on unrelated re-renders.
409-
}, [settledFailure, settledSuccess, logsState.error]);
396+
// Derived from the settlement itself, so there is no second copy of this state to keep
397+
// in sync and no frame painted with a stale banner. `streak` counts CONSECUTIVE failed
398+
// settlements: it is stored keyed by the error identity that produced it, so repeated
399+
// renders of the same failure do not inflate the count and a success clears it.
400+
const [failureStreak, setFailureStreak] = useState<{ error: unknown; count: number }>(
401+
{ error: null, count: 0 },
402+
);
403+
if (settledSuccess && failureStreak.count !== 0) {
404+
setFailureStreak({ error: null, count: 0 });
405+
} else if (settledFailure && failureStreak.error !== logsState.error) {
406+
setFailureStreak(previous => ({ error: logsState.error, count: previous.count + 1 }));
407+
}
408+
const pollFailing = failureStreak.count >= STALE_POLL_FAILURE_LIMIT;
410409

411410
const detailInfo = detail ? statusCodeInfo(detail.status, locale) : null;
412411
const conversationQuery = conversationFilter.trim();

0 commit comments

Comments
 (0)