fix(doctor): add skipped discriminator to distinguish probe skip from gateway timeout

Previously both a planned probe skip (probe:false path) and a transport timeout
returned checked:false, so the renderer's !checked early return would silently
suppress diagnostics for key-optional providers even when the gateway had timed out.

- Add `skipped?: boolean` to GatewayMemoryProbe: true for gateway-confirmed skip,
  false for timeout/unavailable paths
- Renderer now guards on `probe.skipped` instead of `!probe.checked`, so timeouts
  fall through to the existing warning path
- Update doctor-memory-search inline type and buildGatewayProbeWarning signature
- Update skipped-probe tests to pass { skipped: true }; add regression test for
  key-optional timeout (lmstudio gateway timeout now warns)

Addresses clawsweeper P2: src/commands/doctor-memory-search.ts:416
This commit is contained in:
HCL
2026-04-30 07:22:11 +08:00
committed by Peter Steinberger
parent 34d62b0650
commit 549624ffb2
4 changed files with 62 additions and 13 deletions

View File

@@ -74,6 +74,7 @@ describe("probeGatewayMemoryStatus", () => {
checked: true,
ready: true,
error: undefined,
skipped: false,
});
expect(callGateway).toHaveBeenCalledWith({
@@ -84,7 +85,9 @@ describe("probeGatewayMemoryStatus", () => {
});
});
it("treats outer gateway timeouts as inconclusive", async () => {
it("treats outer gateway timeouts as inconclusive (skipped: false)", async () => {
// A transport timeout must NOT be treated as a skipped probe. It is a real
// diagnostic signal and the renderer should warn for key-optional providers.
callGateway.mockRejectedValue(
new Error("gateway timeout after 8000ms\nGateway target: ws://127.0.0.1:18789"),
);
@@ -93,12 +96,15 @@ describe("probeGatewayMemoryStatus", () => {
checked: false,
ready: false,
error: expect.stringContaining("gateway memory probe timed out"),
skipped: false,
});
});
it("propagates checked: false when gateway skipped the embedding probe", async () => {
it("propagates checked: false and skipped: true when gateway skipped the embedding probe", async () => {
// Gateway returns checked: false when called with probe: false and no cached
// availability data (SKIPPED_MEMORY_EMBEDDING_PROBE shape).
// availability data (SKIPPED_MEMORY_EMBEDDING_PROBE shape). The adapter must
// also set skipped: true so renderers can distinguish this from a transport
// timeout (which also returns checked: false but skipped: false).
callGateway.mockResolvedValue({
embedding: {
ok: false,
@@ -112,6 +118,7 @@ describe("probeGatewayMemoryStatus", () => {
checked: false,
ready: false,
error: expect.stringContaining("not checked"),
skipped: true,
});
});

View File

@@ -11,6 +11,13 @@ export type GatewayMemoryProbe = {
checked: boolean;
ready: boolean;
error?: string;
/**
* True when the probe was intentionally skipped by the gateway (probe: false
* path). Distinct from checked: false caused by a network timeout or
* unavailable gateway. Renderers should suppress warnings only for skipped
* probes, not for transport failures.
*/
skipped?: boolean;
};
function isGatewayCallTimeout(message: string): boolean {
@@ -91,11 +98,14 @@ export async function probeGatewayMemoryStatus(params: {
// readiness determination was made. Mapping that to checked: true here would
// cause the renderer to treat a skipped probe as a checked-but-not-ready
// failure and emit a false-positive warning for key-optional providers.
// We also carry skipped: true so renderers can distinguish an intentional
// non-deep skip from a transport timeout (which also returns checked: false).
const gatewayChecked = payload.embedding.checked !== false;
return {
checked: gatewayChecked,
ready: payload.embedding.ok,
error: payload.embedding.error,
skipped: !gatewayChecked,
};
} catch (err) {
const message = formatErrorMessage(err);
@@ -104,12 +114,14 @@ export async function probeGatewayMemoryStatus(params: {
checked: false,
ready: false,
error: `gateway memory probe timed out: ${message}`,
skipped: false,
};
}
return {
checked: true,
ready: false,
error: `gateway memory probe unavailable: ${message}`,
skipped: false,
};
}
}

View File

@@ -459,10 +459,11 @@ describe("noteMemorySearchHealth", () => {
expect(message).toContain("embeddings are not ready");
});
it("does not warn when key-optional provider (lmstudio) probe was skipped (checked: false)", async () => {
it("does not warn when key-optional provider (lmstudio) probe was skipped (skipped: true)", async () => {
// When `openclaw doctor` runs without --deep, the probe is skipped and returns
// { checked: false, ready: false }. This must NOT produce a false-positive warning —
// it means readiness was never checked, not that embeddings are unavailable.
// { checked: false, ready: false, skipped: true }. This must NOT produce a
// false-positive warning — it means readiness was never checked, not that
// embeddings are unavailable.
// Regression test for: https://github.com/openclaw/openclaw/issues/74608
resolveMemorySearchConfig.mockReturnValue({
provider: "lmstudio",
@@ -471,13 +472,13 @@ describe("noteMemorySearchHealth", () => {
});
await noteMemorySearchHealth(cfg, {
gatewayMemoryProbe: { checked: false, ready: false },
gatewayMemoryProbe: { checked: false, ready: false, skipped: true },
});
expect(note).not.toHaveBeenCalled();
});
it("does not warn when key-optional provider (ollama) probe was skipped (checked: false)", async () => {
it("does not warn when key-optional provider (ollama) probe was skipped (skipped: true)", async () => {
// Same guard for ollama — the most commonly reported false-positive case.
resolveMemorySearchConfig.mockReturnValue({
provider: "ollama",
@@ -486,12 +487,35 @@ describe("noteMemorySearchHealth", () => {
});
await noteMemorySearchHealth(cfg, {
gatewayMemoryProbe: { checked: false, ready: false },
gatewayMemoryProbe: { checked: false, ready: false, skipped: true },
});
expect(note).not.toHaveBeenCalled();
});
it("warns for key-optional provider (lmstudio) when gateway probe timed out", async () => {
// A gateway timeout sets checked: false but skipped: false/absent. This is a
// real diagnostic signal — embeddings may be unavailable — so we should warn.
// Regression guard: https://github.com/openclaw/openclaw/issues/74608
resolveMemorySearchConfig.mockReturnValue({
provider: "lmstudio",
local: {},
remote: {},
});
await noteMemorySearchHealth(cfg, {
gatewayMemoryProbe: {
checked: false,
ready: false,
error: "gateway memory probe timed out: gateway timeout after 8000ms",
skipped: false,
},
});
const message = String(note.mock.calls[0]?.[0] ?? "");
expect(message).toContain('provider "lmstudio" is configured');
});
it("notes when gateway probe reports embeddings ready and CLI API key is missing", async () => {
resolveMemorySearchConfig.mockReturnValue({
provider: "gemini",

View File

@@ -316,6 +316,7 @@ export async function noteMemorySearchHealth(
checked: boolean;
ready: boolean;
error?: string;
skipped?: boolean;
};
},
): Promise<void> {
@@ -410,10 +411,14 @@ export async function noteMemorySearchHealth(
if (opts?.gatewayMemoryProbe?.checked && opts.gatewayMemoryProbe.ready) {
return;
}
// When the probe was skipped (checked: false), we have no embedding status
// information — do not warn. A skipped probe means the user ran `openclaw doctor`
// without --deep; it does not mean embeddings are unavailable.
if (opts?.gatewayMemoryProbe && !opts.gatewayMemoryProbe.checked) {
// When the probe was intentionally skipped (skipped: true / checked: false
// due to probe:false path), we have no embedding status information — do
// not warn. A skipped probe means the user ran `openclaw doctor` without
// --deep; it does not mean embeddings are unavailable.
// NOTE: a transport timeout also sets checked: false, but skipped stays
// false/absent — a timeout is a real diagnostic signal and should fall
// through to the warning below.
if (opts?.gatewayMemoryProbe?.skipped) {
return;
}
const gatewayProbeWarning = buildGatewayProbeWarning(opts?.gatewayMemoryProbe);
@@ -585,6 +590,7 @@ function buildGatewayProbeWarning(
checked: boolean;
ready: boolean;
error?: string;
skipped?: boolean;
}
| undefined,
): string | null {