diff --git a/COMMANDS.md b/COMMANDS.md index c1cf306..bf83bae 100644 --- a/COMMANDS.md +++ b/COMMANDS.md @@ -446,13 +446,31 @@ Requires an active orchestrator — switch with `/agent neo` or `/agent kronus` | `AETHER_LOG_DIR` | `~/.aether-agent/logs` | Where session logs (and therefore `aether resume`) live. | | `AETHER_BACKEND` | `auto` | `local` \| `cloud` \| `auto` — overrides the config `backend`. | | `AETHER_LOCAL_BRAIN` | *(unset)* | `python` runs the separately-installed Unlimited-Context brain instead of the built-in Ollama one. | -| `OLLAMA_HOST` | `http://localhost:11434` | Where the offline brain looks for Ollama. | +| `OLLAMA_HOST` | `http://localhost:11434` | Where the offline brain looks for Ollama. Accepts Ollama's own scheme-less form (`127.0.0.1:11434`) as well as a full URL — see below. | | `AETHER_STREAM_TIMEOUT_MS` | `120000` | Stream open/idle timeout (ms). `0` disables it. | | `AETHER_NO_ANIM` | *(unset)* | `1` disables all animated status lines and the thinking pulse. | | `NO_COLOR` | *(unset)* | Any value disables ANSI colors (https://no-color.org). | See [`.env.example`](.env.example). +### `OLLAMA_HOST` accepted forms + +`ollama serve` prints and binds a **scheme-less** `host:port`, and that is what most +people paste into `OLLAMA_HOST`. Every accepted form below is normalized to a full +base URL before any request is built: + +| You set | Aether uses | Note | +|---|---|---| +| *(unset or empty)* | `http://localhost:11434` | The default. | +| `127.0.0.1:11434` | `http://127.0.0.1:11434` | Scheme-less — `http://` is added. | +| `localhost:11434` | `http://localhost:11434` | Scheme-less. | +| `0.0.0.0:11434` | `http://127.0.0.1:11434` | `0.0.0.0` is a *bind* address, not a *connect* address. | +| `http://localhost:11434/` | `http://localhost:11434` | Trailing slashes are stripped. | +| `https://ollama.example.com` | `https://ollama.example.com` | A remote/proxied Ollama. | + +Anything that still will not parse as an `http`/`https` URL is rejected up front with +an error naming the bad value, instead of failing later as "cannot reach Ollama". + --- ## Exit codes diff --git a/scripts/handoff-demo.ts b/scripts/handoff-demo.ts index a74ac10..c1038ad 100644 --- a/scripts/handoff-demo.ts +++ b/scripts/handoff-demo.ts @@ -24,6 +24,7 @@ import { cpSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } f import { tmpdir } from "node:os"; import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; +import { normalizeOllamaHost } from "../src/core/ollama.js"; const here = dirname(fileURLToPath(import.meta.url)); const repoRoot = resolve(here, "..", ".."); @@ -240,7 +241,9 @@ async function main(): Promise { const state: StubState = { turns: new Map(), firstPrompt: new Map(), failures: [] }; let server: Server | null = null; - let ollamaHost = process.env["OLLAMA_HOST"] ?? "http://localhost:11434"; + // Normalize so Ollama's own scheme-less OLLAMA_HOST convention ("127.0.0.1:11434") + // is a usable base URL for the child CLI instead of an unparseable string. + let ollamaHost = normalizeOllamaHost(process.env["OLLAMA_HOST"]); if (!REAL) { const stub = await startStub(state); server = stub.server; diff --git a/src/core/ollama.ts b/src/core/ollama.ts index 384e835..0e0954f 100644 --- a/src/core/ollama.ts +++ b/src/core/ollama.ts @@ -89,6 +89,60 @@ const PROFILES: readonly Profile[] = [ // Safe default for an unknown tag: deterministic, like Qwen coding. const DEFAULT_SAMPLING: Sampling = { temperature: 0.2, top_p: 0.9, top_k: 40 }; +// --- host normalization ------------------------------------------------------ +// Ollama's own convention for OLLAMA_HOST is a scheme-LESS host:port — that is +// exactly what `ollama serve` prints and binds ("127.0.0.1:11434", "0.0.0.0:11434"). +// Concatenating "/v1/chat/completions" onto that produces a string fetch() cannot +// parse, so every local turn died with "Failed to parse URL" and an error hint +// telling the user to set the very variable they had just set. Normalize once, +// here, and route every call site through it. + +/** Matches an explicit `scheme://` prefix (http, https, or anything else). */ +const SCHEME_RE = /^[a-z][a-z0-9+.-]*:\/\//i; + +/** + * Canonicalize an Ollama base URL. Accepts every form users actually type: + * `127.0.0.1:11434` · `localhost:11434` · `0.0.0.0:11434` · + * `http://localhost:11434` · `https://ollama.example.com` · + * `http://localhost:11434/` (trailing slashes stripped) · `` (→ default). + * + * A bare `0.0.0.0` is a *bind* address, not a *connect* address, so it maps to + * `127.0.0.1` for the client side. Throws an Error naming the offending value + * when the result still will not parse. + */ +export function normalizeOllamaHost(raw?: string | null): string { + const trimmed = (raw ?? "").trim(); + if (trimmed === "") return DEFAULT_OLLAMA_HOST; + + const candidate = SCHEME_RE.test(trimmed) ? trimmed : `http://${trimmed}`; + let url: URL; + try { + url = new URL(candidate); + } catch { + throw new Error( + `Invalid Ollama host ${JSON.stringify(trimmed)}. Use 'host:port' (e.g. 127.0.0.1:11434) ` + + `or a full URL (e.g. http://localhost:11434).`, + ); + } + if (url.protocol !== "http:" && url.protocol !== "https:") { + throw new Error( + `Invalid Ollama host ${JSON.stringify(trimmed)}: unsupported scheme '${url.protocol.replace(/:$/, "")}'. ` + + `Ollama speaks http (or https behind a proxy).`, + ); + } + if (url.hostname === "") { + throw new Error( + `Invalid Ollama host ${JSON.stringify(trimmed)}: no hostname. Use 'host:port' (e.g. 127.0.0.1:11434).`, + ); + } + // 0.0.0.0 / :: mean "bind every interface"; you cannot connect to them portably. + if (url.hostname === "0.0.0.0") url.hostname = "127.0.0.1"; + else if (url.hostname === "[::]") url.hostname = "[::1]"; + + const path = url.pathname.replace(/\/+$/, ""); + return `${url.protocol}//${url.host}${path}`; +} + /** The sampling knobs for a model tag (first substring match; safe default). */ export function samplingFor(model: string): Sampling { const low = (model || "").toLowerCase(); @@ -236,7 +290,7 @@ export async function ollamaChat( messages: readonly ChatMessage[], opts: ChatOptions = {}, ): Promise { - const host = (opts.host || process.env["OLLAMA_HOST"] || DEFAULT_OLLAMA_HOST).replace(/\/+$/, ""); + const host = normalizeOllamaHost(opts.host || process.env["OLLAMA_HOST"] || DEFAULT_OLLAMA_HOST); const model = opts.model || DEFAULT_OLLAMA_MODEL; const sampling = samplingFor(model); const temperature = opts.temperature ?? sampling.temperature; @@ -255,51 +309,72 @@ export async function ollamaChat( const controller = new AbortController(); const timeoutMs = opts.timeoutMs ?? DEFAULT_TIMEOUT_MS; const timer = setTimeout(() => controller.abort(), timeoutMs); - let res: Response; - try { - res = await fetch(url, { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify(body), - signal: controller.signal, - }); - } catch (err) { + const timedOut = (): Error => + new Error(`Ollama request timed out after ${Math.round(timeoutMs / 1000)}s (${url}).`); + const isAbort = (err: unknown): boolean => { const msg = err instanceof Error ? err.message : String(err); - if ((err instanceof Error && err.name === "AbortError") || msg.includes("aborted")) { - throw new Error(`Ollama request timed out after ${Math.round(timeoutMs / 1000)}s (${url}).`); - } - throw new Error( - `Cannot reach Ollama at ${host}. Is it running? Start it with 'ollama serve' ` + - `(default port 11434), or set OLLAMA_HOST. Underlying error: ${msg}`, - ); - } finally { - clearTimeout(timer); - } + return (err instanceof Error && err.name === "AbortError") || msg.includes("aborted"); + }; - if (!res.ok) { - const text = await res.text().catch(() => ""); - if (res.status === 404) { + // The timer must stay armed until the BODY is read, not just until headers + // arrive: stream:false means the whole completion is in the body, so a server + // that answers 200 and then stalls mid-body would otherwise hang forever. + try { + let res: Response; + try { + res = await fetch(url, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + signal: controller.signal, + }); + } catch (err) { + if (isAbort(err)) throw timedOut(); + const msg = err instanceof Error ? err.message : String(err); throw new Error( - `Ollama model '${model}' not found (404). Pull it first: 'ollama pull ${model}'. ` + - `Server said: ${text.slice(0, 300)}`, + `Cannot reach Ollama at ${host}. Is it running? Start it with 'ollama serve' ` + + `(default port 11434), or set OLLAMA_HOST. Underlying error: ${msg}`, ); } - throw new Error(`Ollama error ${res.status} at ${url}: ${text.slice(0, 300)}`); - } - const json = (await res.json().catch(() => ({}))) as Record; - const choices = json["choices"]; - const first = Array.isArray(choices) && isRecord(choices[0]) ? (choices[0] as Record) : {}; - const message = isRecord(first["message"]) ? (first["message"] as Record) : {}; - const content = typeof message["content"] === "string" ? message["content"] : ""; + if (!res.ok) { + let text = ""; + try { + text = await res.text(); + } catch (err) { + if (isAbort(err)) throw timedOut(); + } + if (res.status === 404) { + throw new Error( + `Ollama model '${model}' not found (404). Pull it first: 'ollama pull ${model}'. ` + + `Server said: ${text.slice(0, 300)}`, + ); + } + throw new Error(`Ollama error ${res.status} at ${url}: ${text.slice(0, 300)}`); + } + + let json: Record = {}; + try { + json = (await res.json()) as Record; + } catch (err) { + if (isAbort(err)) throw timedOut(); + json = {}; + } + const choices = json["choices"]; + const first = Array.isArray(choices) && isRecord(choices[0]) ? (choices[0] as Record) : {}; + const message = isRecord(first["message"]) ? (first["message"] as Record) : {}; + const content = typeof message["content"] === "string" ? message["content"] : ""; - const structured = normalizeToolCalls(message["tool_calls"]); - const toolCalls = structured.length > 0 ? structured : extractToolCalls(content); + const structured = normalizeToolCalls(message["tool_calls"]); + const toolCalls = structured.length > 0 ? structured : extractToolCalls(content); - const reply: ChatReply = { - role: "assistant", - content, - ...(toolCalls.length > 0 ? { tool_calls: toolCalls } : {}), - }; - return reply; + const reply: ChatReply = { + role: "assistant", + content, + ...(toolCalls.length > 0 ? { tool_calls: toolCalls } : {}), + }; + return reply; + } finally { + clearTimeout(timer); + } } diff --git a/src/core/smoke.ts b/src/core/smoke.ts index 3a06163..3739315 100644 --- a/src/core/smoke.ts +++ b/src/core/smoke.ts @@ -12,7 +12,7 @@ import { isSafeUrl, webFetch, webSearch } from "./web.js"; import { OllamaBrain } from "./brain_ollama.js"; import { CloudBrain } from "./brain_cloud.js"; import { ToolExecutor, type ToolResult } from "./tool_executor.js"; -import { DEFAULT_OLLAMA_HOST, DEFAULT_OLLAMA_MODEL } from "./ollama.js"; +import { DEFAULT_OLLAMA_HOST, DEFAULT_OLLAMA_MODEL, normalizeOllamaHost } from "./ollama.js"; import { defaultTokenStore, type TokenStore } from "./auth.js"; import { loadConfig } from "./config.js"; import { ApiClient } from "./transport.js"; @@ -40,7 +40,7 @@ export async function ollamaUp(host: string, timeoutMs = 4000): Promise const ctl = new AbortController(); const timer = setTimeout(() => ctl.abort(), timeoutMs); try { - const res = await fetch(host.replace(/\/$/, "") + "/api/tags", { signal: ctl.signal }); + const res = await fetch(normalizeOllamaHost(host) + "/api/tags", { signal: ctl.signal }); return res.ok; } catch { return false; @@ -210,7 +210,16 @@ export async function smokeMain(): Promise { const cfg = loadConfig(); const baseUrl = cfg.baseUrl || "https://api.aethersystems.net/cloud"; const model = cfg.defaultModel || DEFAULT_OLLAMA_MODEL; - const host = process.env["OLLAMA_HOST"] || DEFAULT_OLLAMA_HOST; + // Normalize here so a scheme-less OLLAMA_HOST (Ollama's own convention) is + // diagnosed as a bad value instead of being reported as "Ollama is down". + const rawHost = process.env["OLLAMA_HOST"] || DEFAULT_OLLAMA_HOST; + let host = DEFAULT_OLLAMA_HOST; + let hostError = ""; + try { + host = normalizeOllamaHost(rawHost); + } catch (err) { + hostError = err instanceof Error ? err.message : String(err); + } const safe = async (name: string, fn: () => Promise): Promise => { try { @@ -219,11 +228,12 @@ export async function smokeMain(): Promise { return { name, status: "FAIL", detail: `crashed: ${err instanceof Error ? err.message : String(err)}` }; } }; + const badHost = (name: string): Check => ({ name, status: "FAIL", detail: hostError }); const results: Check[] = [ checkSsrf(), - await safe("ollama", () => checkOllama(host)), - await safe("local turn", () => checkLocalTurn(host, model)), + hostError ? badHost("ollama") : await safe("ollama", () => checkOllama(host)), + hostError ? badHost("local turn") : await safe("local turn", () => checkLocalTurn(host, model)), await safe("web_search", () => checkWebSearch()), await safe("web_fetch", () => checkWebFetch()), await safe("auth", () => checkAuth(baseUrl)), diff --git a/test/ollama.test.ts b/test/ollama.test.ts index 00083a7..e9154d3 100644 --- a/test/ollama.test.ts +++ b/test/ollama.test.ts @@ -7,6 +7,8 @@ import { ollamaChat, extractToolCalls, samplingFor, + normalizeOllamaHost, + DEFAULT_OLLAMA_HOST, DEFAULT_OLLAMA_MODEL, type ChatMessage, type ToolSchema, @@ -213,3 +215,88 @@ test("ollamaChat throws a model-not-pulled hint on a 404", async () => { test("DEFAULT_OLLAMA_MODEL is the universal small coder default", () => { assert.equal(DEFAULT_OLLAMA_MODEL, "qwen2.5-coder:7b"); }); + +// --- normalizeOllamaHost: Ollama's own scheme-less OLLAMA_HOST convention ---- +test("normalizeOllamaHost accepts the scheme-less host:port ollama serve prints", () => { + assert.equal(normalizeOllamaHost("127.0.0.1:11434"), "http://127.0.0.1:11434"); + assert.equal(normalizeOllamaHost("localhost:11434"), "http://localhost:11434"); +}); + +test("normalizeOllamaHost maps the 0.0.0.0 bind address to a connectable one", () => { + assert.equal(normalizeOllamaHost("0.0.0.0:11434"), "http://127.0.0.1:11434"); + assert.equal(normalizeOllamaHost("http://0.0.0.0:11434"), "http://127.0.0.1:11434"); +}); + +test("normalizeOllamaHost keeps explicit schemes and strips trailing slashes", () => { + assert.equal(normalizeOllamaHost("http://localhost:11434/"), "http://localhost:11434"); + assert.equal(normalizeOllamaHost("http://localhost:11434///"), "http://localhost:11434"); + assert.equal(normalizeOllamaHost("https://ollama.example.com"), "https://ollama.example.com"); + assert.equal(normalizeOllamaHost("https://ollama.example.com/proxy/"), "https://ollama.example.com/proxy"); +}); + +test("normalizeOllamaHost falls back to the default for empty/whitespace/undefined", () => { + assert.equal(normalizeOllamaHost(undefined), DEFAULT_OLLAMA_HOST); + assert.equal(normalizeOllamaHost(""), DEFAULT_OLLAMA_HOST); + assert.equal(normalizeOllamaHost(" "), DEFAULT_OLLAMA_HOST); + assert.equal(normalizeOllamaHost(" 127.0.0.1:11434 "), "http://127.0.0.1:11434"); +}); + +test("normalizeOllamaHost rejects unusable values and names the bad value", () => { + assert.throws(() => normalizeOllamaHost("ftp://localhost:11434"), /ftp/); + assert.throws(() => normalizeOllamaHost("http://"), /"http:\/\/"|no hostname|Invalid Ollama host/); + assert.throws(() => normalizeOllamaHost(":::"), /Invalid Ollama host/); +}); + +// The bug this fixes: a scheme-less host was concatenated raw, so fetch() got +// "127.0.0.1:11434/v1/chat/completions" -> "Failed to parse URL", which the +// unreachable-handler then reported as "set OLLAMA_HOST" — the thing the user did. +test("ollamaChat completes a turn against a scheme-less host:port", async () => { + const { base, server } = await stub((_req, _body, res) => { + res.end(JSON.stringify({ choices: [{ message: { role: "assistant", content: "pong" } }] })); + }); + try { + const schemeless = base.replace(/^https?:\/\//, ""); + assert.ok(!schemeless.includes("://"), "test really is exercising the scheme-less form"); + const reply = await ollamaChat([{ role: "user", content: "ping" }], { host: schemeless }); + assert.equal(reply.content, "pong"); + } finally { + server.close(); + } +}); + +test("ollamaChat surfaces a bad OLLAMA_HOST as a host error, not 'cannot reach Ollama'", async () => { + await assert.rejects( + () => ollamaChat([{ role: "user", content: "x" }], { host: "ftp://localhost:11434" }), + /Invalid Ollama host/, + ); +}); + +// --- the timeout must cover the BODY, not just the headers ------------------ +// stream:false means the whole completion arrives in the body. A server that +// answers 200 and then stalls mid-body used to hang forever, because the timer +// was cleared as soon as headers arrived. +test("ollamaChat times out when the server stalls mid-body after 200 headers", async () => { + const server = createServer((req, res) => { + req.on("data", () => {}); + req.on("end", () => { + res.writeHead(200, { "content-type": "application/json" }); + res.write('{"choices":[{"message":{"role":"assistant","content":"'); + // ...and then never finishes. No res.end(). + }); + }); + await new Promise((r) => server.listen(0, "127.0.0.1", () => r())); + const port = (server.address() as AddressInfo).port; + try { + await assert.rejects( + () => + ollamaChat([{ role: "user", content: "x" }], { + host: `http://127.0.0.1:${port}`, + timeoutMs: 400, + }), + /timed out after/, + ); + } finally { + server.closeAllConnections(); + server.close(); + } +});