fix(memory): re-check Qdrant health after saving settings

save() optimistically flipped enabled and started the PUT while the mount
effect could immediately GET /api/settings/qdrant/health against the OLD
persisted settings. If that GET won, it returned not_configured/failed and -
because health was non-null - the effect never retried after the PUT
succeeded, leaving a healthy Qdrant red until a manual Test connection.

Invalidate health (generation counter + setHealth(null)) at save start and
after a successful PUT, then explicitly schedule a fresh check: setting
health to null alone is not enough, React bails on the no-op when health is
already null (the exact GET-wins ordering). Stale responses are dropped via
the sequence guard so an in-flight pre-save check can never overwrite the
post-save result. Adds a regression test covering enable ordering.

Addresses PR #10489 review finding (issuecomment-5312271806).
This commit is contained in:
Rouzbeh
2026-08-17 16:18:16 +00:00
parent 3471666741
commit 3469234272
3 changed files with 116 additions and 20 deletions

View File

@@ -1 +1 @@
- **fix(memory):** auto-check Qdrant health on mount and stop the false-red status badge on `/dashboard/memory?tab=engine` — the badge treated "not yet checked" (`health === null`) as a failure, so a healthy Qdrant showed red after every page refresh until "Test connection" was clicked ([#10489](https://github.com/diegosouzapw/OmniRoute/pull/10489)) - **fix(memory):** auto-check Qdrant health on mount and stop the false-red status badge on `/dashboard/memory?tab=engine` — the badge treated "not yet checked" (`health === null`) as a failure, so a healthy Qdrant showed red after every page refresh until "Test connection" was clicked; settings changes now also invalidate the stale result and re-check after the save persists, so a health check racing the settings PUT can no longer keep the badge red until a manual re-test ([#10489](https://github.com/diegosouzapw/OmniRoute/pull/10489))

View File

@@ -1,6 +1,6 @@
"use client"; "use client";
import { useState, useEffect, useCallback } from "react"; import { useState, useEffect, useCallback, useRef } from "react";
import { useTranslations } from "next-intl"; import { useTranslations } from "next-intl";
import { Card } from "@/shared/components"; import { Card } from "@/shared/components";
@@ -47,6 +47,12 @@ export default function QdrantConfigCard() {
const [embeddingOptions, setEmbeddingOptions] = useState<EmbeddingModelOption[]>([]); const [embeddingOptions, setEmbeddingOptions] = useState<EmbeddingModelOption[]>([]);
const [loading, setLoading] = useState(true); const [loading, setLoading] = useState(true);
// Generation counter for health checks. Bumping it invalidates any in-flight
// or already-resolved check so a stale result (for example one that raced a
// settings save and read the pre-save configuration) can never be applied
// out of order.
const healthSeqRef = useRef(0);
useEffect(() => { useEffect(() => {
Promise.all([ Promise.all([
fetch("/api/settings/qdrant").then((r) => (r.ok ? r.json() : null)), fetch("/api/settings/qdrant").then((r) => (r.ok ? r.json() : null)),
@@ -65,10 +71,40 @@ export default function QdrantConfigCard() {
.finally(() => setLoading(false)); .finally(() => setLoading(false));
}, []); }, []);
const checkHealth = useCallback(async () => {
const seq = ++healthSeqRef.current;
setChecking(true);
try {
const res = await fetch("/api/settings/qdrant/health");
if (res.ok) {
const data = await res.json();
// Drop the result if a newer save/check invalidated this one.
if (healthSeqRef.current !== seq) return;
setHealth(data);
} else {
if (healthSeqRef.current !== seq) return;
setHealth({ ok: false, latencyMs: 0, error: "HTTP error" });
}
} catch (e) {
if (healthSeqRef.current !== seq) return;
setHealth({
ok: false,
latencyMs: 0,
error: e instanceof Error ? e.message : String(e),
});
} finally {
if (healthSeqRef.current === seq) setChecking(false);
}
}, []);
const save = useCallback( const save = useCallback(
async (updates: Partial<QdrantSettings> & { apiKey?: string }) => { async (updates: Partial<QdrantSettings> & { apiKey?: string }) => {
const prev = qdrant; const prev = qdrant;
const next = { ...qdrant, ...updates }; const next = { ...qdrant, ...updates };
// Settings are changing, so any prior health result is stale: drop it and
// invalidate in-flight checks so they cannot overwrite the new state.
healthSeqRef.current += 1;
setHealth(null);
setQdrant(next); setQdrant(next);
setSaving(true); setSaving(true);
setSaveStatus(""); setSaveStatus("");
@@ -91,6 +127,16 @@ export default function QdrantConfigCard() {
setQdrant(data); setQdrant(data);
setApiKeyInput(""); setApiKeyInput("");
setSaveStatus("saved"); setSaveStatus("saved");
// A health check started during the optimistic window (enabled just
// flipped and health was null) can race the PUT and read the OLD
// persisted settings -> not_configured/failed. Invalidate it and
// schedule a fresh check against the just-persisted settings. This
// must be explicit: if health was still null the mount effect bails
// on the setHealth(null) no-op, so a healthy Qdrant would stay red
// until a manual test.
healthSeqRef.current += 1;
setHealth(null);
void checkHealth();
setTimeout(() => setSaveStatus(""), 2000); setTimeout(() => setSaveStatus(""), 2000);
} else { } else {
setQdrant(prev); setQdrant(prev);
@@ -103,26 +149,9 @@ export default function QdrantConfigCard() {
setSaving(false); setSaving(false);
} }
}, },
[qdrant] [qdrant, checkHealth]
); );
const checkHealth = useCallback(async () => {
setChecking(true);
try {
const res = await fetch("/api/settings/qdrant/health");
if (res.ok) setHealth(await res.json());
else setHealth({ ok: false, latencyMs: 0, error: "HTTP error" });
} catch (e) {
setHealth({
ok: false,
latencyMs: 0,
error: e instanceof Error ? e.message : String(e),
});
} finally {
setChecking(false);
}
}, []);
// Auto-check on mount once settings load: without this the status badge // Auto-check on mount once settings load: without this the status badge
// renders red after a page refresh because `health` starts as null and the // renders red after a page refresh because `health` starts as null and the
// old code treated "not checked yet" the same as "failed". The Test // old code treated "not checked yet" the same as "failed". The Test

View File

@@ -376,4 +376,71 @@ describe("QdrantConfigCard", () => {
expect(container.textContent).toContain("qdrant.statusActive"); expect(container.textContent).toContain("qdrant.statusActive");
expect(container.textContent).not.toContain("qdrant.statusError"); expect(container.textContent).not.toContain("qdrant.statusError");
}); });
it("re-checks health after a successful save so a stale optimistic-window result cannot leave the badge red (enable ordering)", async () => {
let healthFetchCount = 0;
const fetchMock = vi.fn().mockImplementation((url: string, opts?: { method?: string }) => {
if (url === "/api/settings/qdrant" && opts?.method === "PUT") {
return Promise.resolve({
ok: true,
json: async () => ({ ...MOCK_QDRANT_SETTINGS, enabled: true }),
});
}
if (url === "/api/settings/qdrant") {
return Promise.resolve({ ok: true, json: async () => MOCK_QDRANT_SETTINGS });
}
if (url === "/api/settings/qdrant/embedding-models") {
return Promise.resolve({ ok: true, json: async () => ({ models: [] }) });
}
if (url === "/api/settings/qdrant/health") {
healthFetchCount += 1;
// The first GET races the settings PUT and sees the OLD persisted
// config (enabled=false) -> not_configured. Post-PUT checks see a
// healthy Qdrant.
if (healthFetchCount === 1) {
return Promise.resolve({
ok: true,
json: async () => ({ ok: false, latencyMs: 0, error: "not configured" }),
});
}
return Promise.resolve({ ok: true, json: async () => ({ ok: true, latencyMs: 2 }) });
}
return Promise.resolve({ ok: true, json: async () => ({}) });
});
globalThis.fetch = fetchMock;
const { default: QdrantConfigCard } =
await import("../../../src/app/(dashboard)/dashboard/memory/components/QdrantConfigCard");
const container = makeContainer();
const root = createRoot(container);
await act(async () => {
root.render(<QdrantConfigCard />);
});
await act(async () => {
await new Promise((r) => setTimeout(r, 50));
});
// Enable Qdrant: save() optimistically flips the switch and starts the
// PUT while the mount effect immediately GETs health against the OLD
// persisted settings (returned as not_configured above).
const toggleBtn = container.querySelector(
"[data-testid='qdrant-enabled-switch']"
) as HTMLButtonElement | null;
expect(toggleBtn).toBeTruthy();
await act(async () => {
toggleBtn?.click();
});
await act(async () => {
await new Promise((r) => setTimeout(r, 50));
});
// The stale first check must not be the last word: a fresh health GET must
// be scheduled after the PUT succeeds so the badge ends green.
const allHealthCalls = fetchMock.mock.calls.filter(
(c: [string]) => typeof c[0] === "string" && c[0] === "/api/settings/qdrant/health"
);
expect(allHealthCalls.length).toBeGreaterThanOrEqual(2);
expect(container.textContent).toContain("qdrant.statusActive");
expect(container.textContent).not.toContain("qdrant.statusError");
});
}); });