Skip to content

Commit 1a4b93d

Browse files
kai392RealDiligentcursoragent
authored
fix(ui): drop stale repo-picker panel responses on keystroke races (#7784) (#7904)
Co-authored-by: RealDiligent <brave.challenge007@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent b3e1bc3 commit 1a4b93d

5 files changed

Lines changed: 196 additions & 112 deletions

File tree

‎apps/loopover-ui/src/components/site/app-panels/activation-preview.tsx‎

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -65,31 +65,41 @@ export function ActivationPreview({ reviewability }: { reviewability: Array<{ pr
6565
const base = repoApiBase(repoFullName);
6666
const hasRepos = repoOptions.length > 0;
6767

68-
const load = useCallback(async () => {
69-
const apiBase = repoApiBase(repoFullName);
70-
if (!apiBase) {
71-
setPreview(null);
68+
const load = useCallback(
69+
async (opts?: { cancelled?: () => boolean }) => {
70+
const isCancelled = opts?.cancelled ?? (() => false);
71+
const apiBase = repoApiBase(repoFullName);
72+
if (!apiBase) {
73+
setPreview(null);
74+
setLoadError(null);
75+
return;
76+
}
7277
setLoadError(null);
73-
return;
74-
}
75-
setLoadError(null);
76-
setLoading(true);
77-
const result = await apiFetch<ActivationPreviewResponse>(`${apiBase}/activation-preview`, {
78-
label: "Activation preview",
79-
credentials: "include",
80-
silentStatus: true,
81-
});
82-
if (result.ok) {
83-
setPreview(result.data);
84-
} else {
85-
setPreview(null);
86-
setLoadError(result.message);
87-
}
88-
setLoading(false);
89-
}, [repoFullName]);
78+
setLoading(true);
79+
const result = await apiFetch<ActivationPreviewResponse>(`${apiBase}/activation-preview`, {
80+
label: "Activation preview",
81+
credentials: "include",
82+
silentStatus: true,
83+
});
84+
// Ignore responses after a newer repoFullName keyed a fresh load (#7784).
85+
if (isCancelled()) return;
86+
if (result.ok) {
87+
setPreview(result.data);
88+
} else {
89+
setPreview(null);
90+
setLoadError(result.message);
91+
}
92+
setLoading(false);
93+
},
94+
[repoFullName],
95+
);
9096

9197
useEffect(() => {
92-
void load();
98+
let cancelled = false;
99+
void load({ cancelled: () => cancelled });
100+
return () => {
101+
cancelled = true;
102+
};
93103
}, [load]);
94104

95105
return (
@@ -148,8 +158,8 @@ export function ActivationPreview({ reviewability }: { reviewability: Array<{ pr
148158
isLoading={Boolean(base) && loading}
149159
isError={Boolean(base) && !loading && loadError !== null}
150160
isEmpty={Boolean(base) && !loading && preview !== null && preview.evaluatedCount === 0}
151-
onRetry={load}
152-
onRefresh={load}
161+
onRetry={() => void load()}
162+
onRefresh={() => void load()}
153163
loadingTitle="Building activation preview…"
154164
errorTitle="Couldn't load the activation preview"
155165
errorDescription={loadError ?? undefined}

‎apps/loopover-ui/src/components/site/app-panels/ai-review-settings.tsx‎

Lines changed: 37 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -61,35 +61,45 @@ export function AiReviewSettings({ reviewability }: { reviewability: Array<{ pr:
6161
const base = repoApiBase(repoFullName);
6262
const hasRepos = repoOptions.length > 0;
6363

64-
const load = useCallback(async () => {
65-
const apiBase = repoApiBase(repoFullName);
66-
if (!apiBase) return;
67-
setMessage(null);
68-
setLoading(true);
69-
const [settings, key] = await Promise.all([
70-
apiFetch<RepoSettingsResponse>(`${apiBase}/settings`, {
71-
label: "AI review settings",
72-
credentials: "include",
73-
silentStatus: true,
74-
}),
75-
apiFetch<AiKeyStatus>(`${apiBase}/ai-key`, {
76-
label: "AI key status",
77-
credentials: "include",
78-
silentStatus: true,
79-
}),
80-
]);
81-
if (settings.ok) {
82-
setMode(settings.data.aiReviewMode ?? "off");
83-
setByok(settings.data.aiReviewByok ?? false);
84-
setProvider(settings.data.aiReviewProvider ?? "anthropic");
85-
setModel(settings.data.aiReviewModel ?? "");
86-
}
87-
setKeyStatus(key.ok ? key.data : null);
88-
setLoading(false);
89-
}, [repoFullName]);
64+
const load = useCallback(
65+
async (opts?: { cancelled?: () => boolean }) => {
66+
const isCancelled = opts?.cancelled ?? (() => false);
67+
const apiBase = repoApiBase(repoFullName);
68+
if (!apiBase) return;
69+
setMessage(null);
70+
setLoading(true);
71+
const [settings, key] = await Promise.all([
72+
apiFetch<RepoSettingsResponse>(`${apiBase}/settings`, {
73+
label: "AI review settings",
74+
credentials: "include",
75+
silentStatus: true,
76+
}),
77+
apiFetch<AiKeyStatus>(`${apiBase}/ai-key`, {
78+
label: "AI key status",
79+
credentials: "include",
80+
silentStatus: true,
81+
}),
82+
]);
83+
// Ignore responses after a newer repoFullName keyed a fresh load (#7784).
84+
if (isCancelled()) return;
85+
if (settings.ok) {
86+
setMode(settings.data.aiReviewMode ?? "off");
87+
setByok(settings.data.aiReviewByok ?? false);
88+
setProvider(settings.data.aiReviewProvider ?? "anthropic");
89+
setModel(settings.data.aiReviewModel ?? "");
90+
}
91+
setKeyStatus(key.ok ? key.data : null);
92+
setLoading(false);
93+
},
94+
[repoFullName],
95+
);
9096

9197
useEffect(() => {
92-
void load();
98+
let cancelled = false;
99+
void load({ cancelled: () => cancelled });
100+
return () => {
101+
cancelled = true;
102+
};
93103
}, [load]);
94104

95105
async function saveKey() {

‎apps/loopover-ui/src/components/site/app-panels/ams-miner-cohort-card.test.tsx‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,4 +139,38 @@ describe("AmsMinerCohortCard", () => {
139139
});
140140
expect(screen.getByText(/This view is unavailable for this repository\./i)).toBeTruthy();
141141
});
142+
143+
it("drops a superseded response when repoFullName changes before the first fetch resolves (#7784)", async () => {
144+
let resolveStale!: (value: unknown) => void;
145+
let resolveFresh!: (value: unknown) => void;
146+
apiFetch
147+
.mockImplementationOnce(() => new Promise((resolve) => (resolveStale = resolve)))
148+
.mockImplementationOnce(() => new Promise((resolve) => (resolveFresh = resolve)));
149+
150+
render(<AmsMinerCohortCard reviewability={REVIEWABILITY} />);
151+
expect(screen.getByText(/Loading AMS contributor mix/i)).toBeTruthy();
152+
153+
// Keystroke races a second request while the first is still in flight.
154+
fireEvent.change(screen.getByPlaceholderText("owner/repo"), {
155+
target: { value: "acme/other" },
156+
});
157+
await waitFor(() => expect(apiFetch).toHaveBeenCalledTimes(2));
158+
159+
const freshComparison = {
160+
...POPULATED_COMPARISON,
161+
windowDays: 30,
162+
totalSubmitterCount: 2,
163+
checkedSubmitterCount: 2,
164+
};
165+
// Newer request resolves first and must win.
166+
resolveFresh({ ok: true, data: freshComparison });
167+
await waitFor(() => expect(screen.getByText(/Window: 30 days · checked 2 of/i)).toBeTruthy());
168+
169+
// Stale request resolves last — must not overwrite the fresh window.
170+
resolveStale({ ok: true, data: POPULATED_COMPARISON });
171+
await Promise.resolve();
172+
await Promise.resolve();
173+
expect(screen.getByText(/Window: 30 days · checked 2 of/i)).toBeTruthy();
174+
expect(screen.queryByText(/Window: 90 days · checked 5 of/i)).toBeNull();
175+
});
142176
});

‎apps/loopover-ui/src/components/site/app-panels/ams-miner-cohort-card.tsx‎

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -98,31 +98,41 @@ export function AmsMinerCohortCard({ reviewability }: { reviewability: Array<{ p
9898
const base = repoApiBase(repoFullName);
9999
const hasRepos = repoOptions.length > 0;
100100

101-
const load = useCallback(async () => {
102-
const apiBase = repoApiBase(repoFullName);
103-
if (!apiBase) {
104-
setComparison(null);
101+
const load = useCallback(
102+
async (opts?: { cancelled?: () => boolean }) => {
103+
const isCancelled = opts?.cancelled ?? (() => false);
104+
const apiBase = repoApiBase(repoFullName);
105+
if (!apiBase) {
106+
setComparison(null);
107+
setLoadError(null);
108+
return;
109+
}
105110
setLoadError(null);
106-
return;
107-
}
108-
setLoadError(null);
109-
setLoading(true);
110-
const result = await apiFetch<AmsMinerCohortComparison>(`${apiBase}/ams-miner-cohort`, {
111-
label: "AMS miner cohort comparison",
112-
credentials: "include",
113-
silentStatus: true,
114-
});
115-
if (result.ok) {
116-
setComparison(result.data);
117-
} else {
118-
setComparison(null);
119-
setLoadError(result.message);
120-
}
121-
setLoading(false);
122-
}, [repoFullName]);
111+
setLoading(true);
112+
const result = await apiFetch<AmsMinerCohortComparison>(`${apiBase}/ams-miner-cohort`, {
113+
label: "AMS miner cohort comparison",
114+
credentials: "include",
115+
silentStatus: true,
116+
});
117+
// Ignore responses after a newer repoFullName keyed a fresh load (#7784).
118+
if (isCancelled()) return;
119+
if (result.ok) {
120+
setComparison(result.data);
121+
} else {
122+
setComparison(null);
123+
setLoadError(result.message);
124+
}
125+
setLoading(false);
126+
},
127+
[repoFullName],
128+
);
123129

124130
useEffect(() => {
125-
void load();
131+
let cancelled = false;
132+
void load({ cancelled: () => cancelled });
133+
return () => {
134+
cancelled = true;
135+
};
126136
}, [load]);
127137

128138
return (
@@ -171,8 +181,8 @@ export function AmsMinerCohortCard({ reviewability }: { reviewability: Array<{ p
171181
isLoading={Boolean(base) && loading}
172182
isError={Boolean(base) && !loading && loadError !== null}
173183
isEmpty={Boolean(base) && !loading && comparison !== null && !comparison.present}
174-
onRetry={load}
175-
onRefresh={load}
184+
onRetry={() => void load()}
185+
onRefresh={() => void load()}
176186
loadingTitle="Loading AMS contributor mix…"
177187
errorTitle="Couldn't load the AMS contributor mix"
178188
errorDescription={loadError ?? undefined}

‎apps/loopover-ui/src/components/site/app-panels/maintainer-settings.tsx‎

Lines changed: 57 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -145,32 +145,42 @@ export function MaintainerSettings({ reviewability }: { reviewability: Array<{ p
145145
const base = repoApiBase(repoFullName);
146146
const hasRepos = repoOptions.length > 0;
147147

148-
const load = useCallback(async () => {
149-
const apiBase = repoApiBase(repoFullName);
150-
if (!apiBase) return;
151-
setMessage(null);
152-
setLoading(true);
153-
const result = await apiFetch<MaintainerSettings>(`${apiBase}/settings`, {
154-
label: "Repository settings",
155-
credentials: "include",
156-
silentStatus: true,
157-
});
158-
// Default the agent-layer fields defensively so the editor renders even against an older response shape.
159-
setSettings(
160-
result.ok
161-
? {
162-
...result.data,
163-
autonomy: result.data.autonomy ?? {},
164-
agentPaused: result.data.agentPaused ?? false,
165-
agentDryRun: result.data.agentDryRun ?? false,
166-
}
167-
: null,
168-
);
169-
setLoading(false);
170-
}, [repoFullName]);
148+
const load = useCallback(
149+
async (opts?: { cancelled?: () => boolean }) => {
150+
const isCancelled = opts?.cancelled ?? (() => false);
151+
const apiBase = repoApiBase(repoFullName);
152+
if (!apiBase) return;
153+
setMessage(null);
154+
setLoading(true);
155+
const result = await apiFetch<MaintainerSettings>(`${apiBase}/settings`, {
156+
label: "Repository settings",
157+
credentials: "include",
158+
silentStatus: true,
159+
});
160+
// Ignore responses after a newer repoFullName keyed a fresh load (#7784).
161+
if (isCancelled()) return;
162+
// Default the agent-layer fields defensively so the editor renders even against an older response shape.
163+
setSettings(
164+
result.ok
165+
? {
166+
...result.data,
167+
autonomy: result.data.autonomy ?? {},
168+
agentPaused: result.data.agentPaused ?? false,
169+
agentDryRun: result.data.agentDryRun ?? false,
170+
}
171+
: null,
172+
);
173+
setLoading(false);
174+
},
175+
[repoFullName],
176+
);
171177

172178
useEffect(() => {
173-
void load();
179+
let cancelled = false;
180+
void load({ cancelled: () => cancelled });
181+
return () => {
182+
cancelled = true;
183+
};
174184
}, [load]);
175185

176186
function setField<K extends keyof MaintainerSettings>(key: K, value: MaintainerSettings[K]) {
@@ -520,21 +530,31 @@ function FocusManifestEditor({ base }: { base: string | null }) {
520530
const [busy, setBusy] = useState(false);
521531
const [message, setMessage] = useState<Message | null>(null);
522532

523-
const load = useCallback(async () => {
524-
if (!base) return;
525-
setLoading(true);
526-
setMessage(null);
527-
const result = await apiFetch<FocusManifestResponse>(`${base}/focus-manifest`, {
528-
label: "Focus manifest",
529-
credentials: "include",
530-
silentStatus: true,
531-
});
532-
setText(result.ok ? JSON.stringify(result.data.manifest, null, 2) : "");
533-
setLoading(false);
534-
}, [base]);
533+
const load = useCallback(
534+
async (opts?: { cancelled?: () => boolean }) => {
535+
const isCancelled = opts?.cancelled ?? (() => false);
536+
if (!base) return;
537+
setLoading(true);
538+
setMessage(null);
539+
const result = await apiFetch<FocusManifestResponse>(`${base}/focus-manifest`, {
540+
label: "Focus manifest",
541+
credentials: "include",
542+
silentStatus: true,
543+
});
544+
// Ignore responses after a newer base keyed a fresh load (#7784).
545+
if (isCancelled()) return;
546+
setText(result.ok ? JSON.stringify(result.data.manifest, null, 2) : "");
547+
setLoading(false);
548+
},
549+
[base],
550+
);
535551

536552
useEffect(() => {
537-
void load();
553+
let cancelled = false;
554+
void load({ cancelled: () => cancelled });
555+
return () => {
556+
cancelled = true;
557+
};
538558
}, [load]);
539559

540560
async function save() {

0 commit comments

Comments
 (0)