diff --git a/src/llm/llama-server-health.test.ts b/src/llm/llama-server-health.test.ts new file mode 100644 index 0000000..d7edd76 --- /dev/null +++ b/src/llm/llama-server-health.test.ts @@ -0,0 +1,197 @@ +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("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.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 311d4dd..50a7525 100644 --- a/src/llm/llama-server-health.ts +++ b/src/llm/llama-server-health.ts @@ -3,6 +3,27 @@ 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. + * - `"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 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" | "llama-loading" | "openai-compat" | "unknown"; error: string | null; latencyMs: number; } @@ -35,10 +56,37 @@ 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, + error: `http ${response.status}`, + kind: "unknown", + latencyMs: Date.now() - start, + }; + } + const isLlama = bodyLooksLikeLlamaHealth(text); 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 +95,7 @@ async function pingOnce( reachable: false, status: null, error: message, + kind: "unknown", latencyMs: Date.now() - start, }; } finally { @@ -54,6 +103,79 @@ 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. + */ +function bodyLooksLikeLlamaHealth(text: string): boolean { + try { + const parsed: unknown = JSON.parse(text); + return ( + typeof parsed === "object" && + parsed !== null && + typeof (parsed as { status?: unknown }).status === "string" + ); + } catch { + return false; + } +} + +/** + * 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` + * 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)); } @@ -77,16 +199,35 @@ 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)); } } - 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, + }; + // 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 23dc01c..5f08558 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,33 @@ 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; + } + // 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"}`,