From 0136ce20c80ddbb6f3eecb91b7f16112909897d8 Mon Sep 17 00:00:00 2001 From: Dudka Date: Fri, 7 Aug 2026 15:11:56 +0300 Subject: [PATCH 1/2] fix(llm): validate the /health body and identify OpenAI-compatible runners Closes #65, closes #66. KoboldCpp answers HTTP 200 with its web UI on every path, including /health, so the external-URL probe passed falsely, the chat route switched onto a server the llama.cpp client cannot drive, and the session hung with the keyboard eaten by the busy flag. With /v1 appended the downstream call failed fast instead, which is why the user saw a short freeze in one case and a permanent one in the other. - a 200 on /health now counts only when the body carries llama.cpp's JSON shape (a status string); anything else is not a llama-server - when /health says no, a secondary probe asks {base}/v1/models; a JSON answer with a data array identifies an OpenAI-compatible runner (KoboldCpp, LM Studio, vLLM) and the result carries kind: "openai-compat" - the URL-save path now emits "probing " immediately so the 8s probe window no longer reads as a freeze, and on an openai-compat answer it tells the operator exactly how to add the server as a cloud provider instead of failing with a bare error Co-Authored-By: Claude Fable 5 --- src/llm/llama-server-health.test.ts | 131 ++++++++++++++++++++++++++++ src/llm/llama-server-health.ts | 110 ++++++++++++++++++++--- src/tui/tui-command.ts | 21 +++++ 3 files changed, 252 insertions(+), 10 deletions(-) create mode 100644 src/llm/llama-server-health.test.ts diff --git a/src/llm/llama-server-health.test.ts b/src/llm/llama-server-health.test.ts new file mode 100644 index 0000000..489b8cd --- /dev/null +++ b/src/llm/llama-server-health.test.ts @@ -0,0 +1,131 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { resetConfigCache } from "../config/index.js"; +import { checkLlamaServer } from "./llama-server-health.js"; + +function jsonResponse(body: unknown, ok = true, status = 200) { + return { + ok, + status, + text: async () => JSON.stringify(body), + }; +} + +function htmlResponse() { + return { + ok: true, + status: 200, + text: async () => "KoboldCpp", + }; +} + +describe("checkLlamaServer", () => { + let stateDir: string; + + beforeEach(() => { + stateDir = mkdtempSync(join(tmpdir(), "llama-health-")); + process.env.ATOMIC_AGENT_STATE_DIR = stateDir; + resetConfigCache(); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + delete process.env.ATOMIC_AGENT_STATE_DIR; + resetConfigCache(); + rmSync(stateDir, { recursive: true, force: true }); + }); + + it("accepts a real llama.cpp /health answer", async () => { + vi.stubGlobal("fetch", vi.fn(async () => jsonResponse({ status: "ok" }))); + const result = await checkLlamaServer({ + url: "http://127.0.0.1:8080", + retries: 0, + }); + expect(result.reachable).toBe(true); + expect(result.kind).toBe("llama-server"); + }); + + it("rejects a 200 that is not llama.cpp's health shape (KoboldCpp web UI)", async () => { + // First call: /health returns HTML. Second call: /v1/models also HTML, + // so this is not even an OpenAI-compatible endpoint. + vi.stubGlobal("fetch", vi.fn(async () => htmlResponse())); + const result = await checkLlamaServer({ + url: "http://127.0.0.1:5001", + retries: 0, + }); + expect(result.reachable).toBe(false); + expect(result.kind).toBe("unknown"); + expect(result.error).toContain("not with llama.cpp"); + }); + + it("identifies an OpenAI-compatible runner via the /v1/models fallback", async () => { + const fetchMock = vi.fn(async (url: string | URL) => { + const u = String(url); + if (u.endsWith("/health")) return htmlResponse(); + if (u.endsWith("/v1/models")) { + return jsonResponse({ data: [{ id: "koboldcpp/model" }] }); + } + throw new Error(`unexpected url ${u}`); + }); + vi.stubGlobal("fetch", fetchMock); + const result = await checkLlamaServer({ + url: "http://127.0.0.1:5001", + retries: 0, + }); + expect(result.reachable).toBe(false); + expect(result.kind).toBe("openai-compat"); + }); + + it("reports unknown when nothing answers", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => { + throw new Error("fetch failed"); + }), + ); + const result = await checkLlamaServer({ + url: "http://127.0.0.1:9999", + retries: 0, + }); + expect(result.reachable).toBe(false); + expect(result.kind).toBe("unknown"); + expect(result.error).toContain("fetch failed"); + }); + + it("returns after the first successful attempt when retrying", async () => { + let calls = 0; + vi.stubGlobal( + "fetch", + vi.fn(async (url: string | URL) => { + if (String(url).endsWith("/health")) { + calls += 1; + if (calls === 1) throw new Error("cold start"); + return jsonResponse({ status: "ok" }); + } + throw new Error("unexpected"); + }), + ); + const result = await checkLlamaServer({ + url: "http://127.0.0.1:8080", + retries: 2, + backoffMs: 1, + }); + expect(result.reachable).toBe(true); + expect(calls).toBe(2); + }); + + it("still accepts 'loading model' as a llama-server answer", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => jsonResponse({ status: "loading model" })), + ); + const result = await checkLlamaServer({ + url: "http://127.0.0.1:8080", + retries: 0, + }); + expect(result.kind).toBe("llama-server"); + }); +}); diff --git a/src/llm/llama-server-health.ts b/src/llm/llama-server-health.ts index 311d4dd..0d8a3b3 100644 --- a/src/llm/llama-server-health.ts +++ b/src/llm/llama-server-health.ts @@ -3,6 +3,21 @@ import { getConfig } from "../config/index.js"; export interface HealthResult { reachable: boolean; status: number | null; + /** + * What actually answered. + * - `"llama-server"`: `/health` returned llama.cpp's JSON shape. + * - `"openai-compat"`: `/health` did not, but `{base}/v1/models` + * answered like an OpenAI-compatible server (KoboldCpp, LM Studio, + * vLLM). The external llama.cpp route cannot drive these; callers + * should steer the operator to the openai-compatible provider. + * - `"unknown"`: nothing recognisable answered. + * + * A bare HTTP 200 is deliberately NOT enough for `"llama-server"`: + * KoboldCpp answers 200 with HTML on every path, which used to make + * the probe pass falsely and let the chat route switch onto a server + * the llama.cpp client then hangs against (#65, #66). + */ + kind: "llama-server" | "openai-compat" | "unknown"; error: string | null; latencyMs: number; } @@ -35,10 +50,23 @@ async function pingOnce( headers: buildHeaders(apiKey), signal: controller.signal, }); + if (!response.ok) { + return { + reachable: false, + status: response.status, + error: `http ${response.status}`, + kind: "unknown", + latencyMs: Date.now() - start, + }; + } + const isLlama = await bodyLooksLikeLlamaHealth(response); return { - reachable: response.ok, + reachable: isLlama, status: response.status, - error: response.ok ? null : `http ${response.status}`, + error: isLlama + ? null + : "answered 200 but not with llama.cpp's /health shape", + kind: isLlama ? "llama-server" : "unknown", latencyMs: Date.now() - start, }; } catch (err) { @@ -47,6 +75,7 @@ async function pingOnce( reachable: false, status: null, error: message, + kind: "unknown", latencyMs: Date.now() - start, }; } finally { @@ -54,6 +83,61 @@ async function pingOnce( } } +/** + * llama.cpp's `/health` answers with a small JSON object carrying a + * `status` string (`ok`, `loading model`, `error`). Anything else that + * happens to return 200 on that path (KoboldCpp serves its web UI there) + * is not a llama-server and must not pass the probe. + */ +async function bodyLooksLikeLlamaHealth(response: Response): Promise { + try { + const text = await response.text(); + const parsed: unknown = JSON.parse(text); + return ( + typeof parsed === "object" && + parsed !== null && + typeof (parsed as { status?: unknown }).status === "string" + ); + } catch { + return false; + } +} + +/** + * Secondary probe for #66: when `/health` says this is not a + * llama-server, ask `{base}/v1/models`. A JSON answer with a `data` + * array is the OpenAI-compatible signature shared by KoboldCpp, + * LM Studio, vLLM and friends. Best-effort with its own timeout; + * network errors simply report `"unknown"`. + */ +async function probeOpenAiCompat( + base: string, + timeoutMs: number, + apiKey: string | null | undefined, +): Promise { + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), timeoutMs); + try { + const url = new URL("/v1/models", base).toString(); + const response = await fetch(url, { + method: "GET", + headers: buildHeaders(apiKey), + signal: controller.signal, + }); + if (!response.ok) return false; + const parsed: unknown = JSON.parse(await response.text()); + return ( + typeof parsed === "object" && + parsed !== null && + Array.isArray((parsed as { data?: unknown }).data) + ); + } catch { + return false; + } finally { + clearTimeout(timer); + } +} + function wait(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } @@ -81,12 +165,18 @@ export async function checkLlamaServer( await wait(backoffMs * Math.pow(2, attempt)); } } - return ( - last ?? { - reachable: false, - status: null, - error: "no attempts made", - latencyMs: 0, - } - ); + const failed: HealthResult = last ?? { + reachable: false, + status: null, + error: "no attempts made", + kind: "unknown", + latencyMs: 0, + }; + // The server is not a llama-server; find out whether it is an + // OpenAI-compatible runner so the caller can say something useful + // instead of a bare failure (#66). + if (await probeOpenAiCompat(base, timeoutMs, apiKey)) { + return { ...failed, kind: "openai-compat" }; + } + return failed; } diff --git a/src/tui/tui-command.ts b/src/tui/tui-command.ts index 23dc01c..e0d0a3d 100644 --- a/src/tui/tui-command.ts +++ b/src/tui/tui-command.ts @@ -428,6 +428,12 @@ function persistLlamaUrl( ): void { void (async () => { try { + // Immediate feedback: the probe can take up to 8s against a dead + // host, and a silent gap reads as a freeze (#65). + bus.emit({ + type: "runtime_info", + line: `probing ${nextUrl}…`, + }); const health = await checkLlamaServer({ url: nextUrl, retries: 0, @@ -435,6 +441,21 @@ function persistLlamaUrl( timeoutMs: 8000, }); if (!health.reachable) { + // An OpenAI-compatible runner (KoboldCpp, LM Studio, vLLM) is a + // real server, just not one the external llama.cpp route can + // drive. Say so and point at the path that works, instead of + // letting a false pass switch the route onto it and hang (#65, + // #66). + if (health.kind === "openai-compat") { + bus.emit({ + type: "runtime_info", + line: + `${nextUrl} answers like an OpenAI-compatible server, not ` + + `llama.cpp. Add it as a cloud provider instead: LLM tab -> ` + + `Cloud -> n (add provider) -> openai-compatible, base URL ${nextUrl}.`, + }); + return; + } bus.emit({ type: "runtime_info", line: `local-llm /health failed at ${nextUrl}: ${health.error ?? "unknown"}`, From e37d245ad07f2be20116494758f3b86480c144d9 Mon Sep 17 00:00:00 2001 From: Dudka Date: Fri, 7 Aug 2026 19:04:08 +0300 Subject: [PATCH 2/2] fix(llm): recognize llama.cpp's 503 loading answer and gate the OpenAI-compat probe Review follow-up for the /health body validation. Real llama.cpp builds answer /health with 503 while the model loads: new builds send {"error":{"code":503,"message":"Loading model..."}} without a status field, old builds {"status":"loading model"}. The probe treated any non-2xx as "unknown", fell through to the /v1/models probe (which fresh llama.cpp already serves during load), and told a genuine llama.cpp operator to re-add their server as a cloud provider. - a 503 whose body matches either loading shape now reports the new kind "llama-loading": still unreachable, but recognized as ours, and the OpenAI-compat probe is skipped for it - the URL-save path tells the operator the server is still loading the model and to save again in a minute, instead of misrouting them - the OpenAI-compat probe now runs only when the answer suggests a different server kind: a 200 with a non-llama body or a 404; nothing answering at all (connection refused, timeout) no longer burns 8-16 extra seconds on /v1/models - a deterministic 200 with a non-llama body (KoboldCpp web UI) exits the retry loop on the first attempt instead of spending the whole backoff budget on an answer that cannot change - tests now mock the real 503 loading answers instead of a 200 that llama.cpp never sends, and cover the no-retry and no-probe paths Co-Authored-By: Claude Fable 5 --- src/llm/llama-server-health.test.ts | 76 +++++++++++++++++++++++++++-- src/llm/llama-server-health.ts | 69 ++++++++++++++++++++++---- src/tui/tui-command.ts | 12 +++++ 3 files changed, 143 insertions(+), 14 deletions(-) diff --git a/src/llm/llama-server-health.test.ts b/src/llm/llama-server-health.test.ts index 489b8cd..d7edd76 100644 --- a/src/llm/llama-server-health.test.ts +++ b/src/llm/llama-server-health.test.ts @@ -117,15 +117,81 @@ describe("checkLlamaServer", () => { expect(calls).toBe(2); }); - it("still accepts 'loading model' as a llama-server answer", async () => { - vi.stubGlobal( - "fetch", - vi.fn(async () => jsonResponse({ status: "loading model" })), + it("recognizes a new-build llama.cpp 503 while the model loads", async () => { + // Fresh llama.cpp builds answer /health with 503 and an error body + // (no `status` field) until the model finishes loading. + const fetchMock = vi.fn(async () => + jsonResponse( + { error: { code: 503, message: "Loading model..." } }, + false, + 503, + ), ); + vi.stubGlobal("fetch", fetchMock); const result = await checkLlamaServer({ url: "http://127.0.0.1:8080", retries: 0, }); - expect(result.kind).toBe("llama-server"); + expect(result.reachable).toBe(false); + expect(result.kind).toBe("llama-loading"); + expect(result.error).toContain("loading"); + // This IS a llama-server; the OpenAI-compat probe must not run and + // misidentify it as a different runner. + const urls = fetchMock.mock.calls.map((c) => String(c[0])); + expect(urls.some((u) => u.endsWith("/v1/models"))).toBe(false); + }); + + it("recognizes an old-build llama.cpp 503 with a status body", async () => { + const fetchMock = vi.fn(async () => + jsonResponse({ status: "loading model" }, false, 503), + ); + vi.stubGlobal("fetch", fetchMock); + const result = await checkLlamaServer({ + url: "http://127.0.0.1:8080", + retries: 0, + }); + expect(result.reachable).toBe(false); + expect(result.kind).toBe("llama-loading"); + const urls = fetchMock.mock.calls.map((c) => String(c[0])); + expect(urls.some((u) => u.endsWith("/v1/models"))).toBe(false); + }); + + it("does not retry a deterministic 200 with a non-llama body", async () => { + // KoboldCpp's web UI answers 200 with HTML on every path; the same + // answer will come back on every retry, so the loop must bail early + // instead of burning the whole backoff budget. + let healthCalls = 0; + const fetchMock = vi.fn(async (url: string | URL) => { + if (String(url).endsWith("/health")) { + healthCalls += 1; + return htmlResponse(); + } + return htmlResponse(); + }); + vi.stubGlobal("fetch", fetchMock); + const result = await checkLlamaServer({ + url: "http://127.0.0.1:5001", + retries: 3, + backoffMs: 1, + }); + expect(result.kind).toBe("unknown"); + expect(healthCalls).toBe(1); + }); + + it("skips the OpenAI-compat probe when nothing answered at all", async () => { + // Connection refused / timeout means no server spoke; asking + // /v1/models afterwards only adds dead seconds. + const fetchMock = vi.fn(async () => { + throw new Error("connect ECONNREFUSED"); + }); + vi.stubGlobal("fetch", fetchMock); + const result = await checkLlamaServer({ + url: "http://127.0.0.1:9999", + retries: 0, + }); + expect(result.reachable).toBe(false); + expect(result.kind).toBe("unknown"); + const urls = fetchMock.mock.calls.map((c) => String(c[0])); + expect(urls.some((u) => u.endsWith("/v1/models"))).toBe(false); }); }); diff --git a/src/llm/llama-server-health.ts b/src/llm/llama-server-health.ts index 0d8a3b3..50a7525 100644 --- a/src/llm/llama-server-health.ts +++ b/src/llm/llama-server-health.ts @@ -6,18 +6,24 @@ export interface HealthResult { /** * What actually answered. * - `"llama-server"`: `/health` returned llama.cpp's JSON shape. + * - `"llama-loading"`: `/health` returned llama.cpp's 503 while the + * model is still loading (new builds answer + * `{"error":{"code":503,"message":"Loading model..."}}`, old builds + * `{"status":"loading model"}`). The server IS a llama-server; it + * just cannot serve yet. Callers should say "wait", not "wrong + * server kind". * - `"openai-compat"`: `/health` did not, but `{base}/v1/models` * answered like an OpenAI-compatible server (KoboldCpp, LM Studio, * vLLM). The external llama.cpp route cannot drive these; callers * should steer the operator to the openai-compatible provider. - * - `"unknown"`: nothing recognisable answered. + * - `"unknown"`: nothing recognizable answered. * * A bare HTTP 200 is deliberately NOT enough for `"llama-server"`: * KoboldCpp answers 200 with HTML on every path, which used to make * the probe pass falsely and let the chat route switch onto a server * the llama.cpp client then hangs against (#65, #66). */ - kind: "llama-server" | "openai-compat" | "unknown"; + kind: "llama-server" | "llama-loading" | "openai-compat" | "unknown"; error: string | null; latencyMs: number; } @@ -50,7 +56,21 @@ async function pingOnce( headers: buildHeaders(apiKey), signal: controller.signal, }); + const text = await response.text().catch(() => ""); if (!response.ok) { + // llama.cpp answers /health with 503 while the model is loading: + // new builds send {"error":{"code":503,"message":"Loading model..."}}, + // old builds {"status":"loading model"}. Both mean "this IS a + // llama-server, come back in a bit", not "wrong server kind". + if (response.status === 503 && bodyLooksLikeLlamaLoading(text)) { + return { + reachable: false, + status: response.status, + error: "llama.cpp is still loading the model", + kind: "llama-loading", + latencyMs: Date.now() - start, + }; + } return { reachable: false, status: response.status, @@ -59,7 +79,7 @@ async function pingOnce( latencyMs: Date.now() - start, }; } - const isLlama = await bodyLooksLikeLlamaHealth(response); + const isLlama = bodyLooksLikeLlamaHealth(text); return { reachable: isLlama, status: response.status, @@ -89,9 +109,8 @@ async function pingOnce( * happens to return 200 on that path (KoboldCpp serves its web UI there) * is not a llama-server and must not pass the probe. */ -async function bodyLooksLikeLlamaHealth(response: Response): Promise { +function bodyLooksLikeLlamaHealth(text: string): boolean { try { - const text = await response.text(); const parsed: unknown = JSON.parse(text); return ( typeof parsed === "object" && @@ -103,6 +122,25 @@ async function bodyLooksLikeLlamaHealth(response: Response): Promise { } } +/** + * Recognizes llama.cpp's 503 "still loading" body. New builds answer + * `{"error":{"code":503,"message":"Loading model..."}}` (no `status` + * field); old builds answer `{"status":"loading model"}`. + */ +function bodyLooksLikeLlamaLoading(text: string): boolean { + if (bodyLooksLikeLlamaHealth(text)) return true; + try { + const parsed: unknown = JSON.parse(text); + if (typeof parsed !== "object" || parsed === null) return false; + const error = (parsed as { error?: unknown }).error; + if (typeof error !== "object" || error === null) return false; + const message = (error as { message?: unknown }).message; + return typeof message === "string" && message.toLowerCase().includes("loading"); + } catch { + return false; + } +} + /** * Secondary probe for #66: when `/health` says this is not a * llama-server, ask `{base}/v1/models`. A JSON answer with a `data` @@ -161,6 +199,12 @@ export async function checkLlamaServer( for (let attempt = 0; attempt <= retries; attempt += 1) { last = await pingOnce(url, timeoutMs, apiKey); if (last.reachable) return last; + // A 200 with a non-llama body is deterministic: the same wrong + // server (KoboldCpp web UI) will answer the same way on every + // retry, so burning the whole backoff budget changes nothing. + // Transient failures (connection refused, timeouts, 503 while + // loading) still get the retries. + if (last.status === 200 && last.kind === "unknown") break; if (attempt < retries) { await wait(backoffMs * Math.pow(2, attempt)); } @@ -172,10 +216,17 @@ export async function checkLlamaServer( kind: "unknown", latencyMs: 0, }; - // The server is not a llama-server; find out whether it is an - // OpenAI-compatible runner so the caller can say something useful - // instead of a bare failure (#66). - if (await probeOpenAiCompat(base, timeoutMs, apiKey)) { + // Find out whether this is an OpenAI-compatible runner so the caller + // can say something useful instead of a bare failure (#66). Only worth + // asking when something HTTP actually answered in a way that suggests + // a different server kind: a 200 with a non-llama body, or a 404 + // (LM Studio and friends do not serve /health). Skip it when nothing + // answered at all (connection refused, timeout) and when the server + // already identified itself as llama.cpp loading a model. + const suggestsDifferentServer = + failed.kind === "unknown" && + (failed.status === 200 || failed.status === 404); + if (suggestsDifferentServer && (await probeOpenAiCompat(base, timeoutMs, apiKey))) { return { ...failed, kind: "openai-compat" }; } return failed; diff --git a/src/tui/tui-command.ts b/src/tui/tui-command.ts index e0d0a3d..5f08558 100644 --- a/src/tui/tui-command.ts +++ b/src/tui/tui-command.ts @@ -456,6 +456,18 @@ function persistLlamaUrl( }); return; } + // A 503 with llama.cpp's loading body is the right server at a + // wrong moment; telling the operator to reconfigure would be a + // lie. Just say to come back once the model is up. + if (health.kind === "llama-loading") { + bus.emit({ + type: "runtime_info", + line: + `${nextUrl} is a llama.cpp server that is still loading its ` + + `model. Give it a minute and save the URL again.`, + }); + return; + } bus.emit({ type: "runtime_info", line: `local-llm /health failed at ${nextUrl}: ${health.error ?? "unknown"}`,