diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 4db0427..de6536e 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -70,7 +70,7 @@ each one: rejections, not silent success. 7. **No secret leakage.** Adapters receive `ServerCredential` but must never log/serialize `apiKey`. -`CONTRACT_VERSION = 2`. Any breaking change bumps it; registry asserts adapters match. +`CONTRACT_VERSION = 4`. Any breaking change bumps it; registry asserts adapters match. ## 3. Provider-registration shim (`shim/provider-shim.ts`) diff --git a/CAPABILITY-MATRIX.md b/CAPABILITY-MATRIX.md index 025a388..e75fe5e 100644 --- a/CAPABILITY-MATRIX.md +++ b/CAPABILITY-MATRIX.md @@ -4,21 +4,22 @@ Backend endpoints are **[WEB]** — confirm live per adapter. `pi api` = which built-in Pi API type the adapter registers under (`oai` = `openai-completions`, `ant` = `anthropic-messages`). -| Backend | port | pi api | listModels | introspectLoaded | switchModel | loadUnload | auth | health | perModelCaps | streaming | discovery fingerprint | -|---|---|---|---|---|---|---|---|---|---|---|---| -| **Ollama** | 11434 | oai | ✅ `/api/tags`,`/v1/models` | ✅ `/api/ps` | ✅ implicit (request id) | ✅ `keep_alive:0` | ◐ none local | ✅ `GET /` text | ✅ `/api/show` caps + ctx | ✅ | `GET /` → `Ollama is running` | -| **LM Studio** | 1234 | oai | ✅ `/api/v1/models` (v0 fallback) | ✅ `state` field | ✅ JIT + `/api/v1/models/load` | ✅ load/unload + `lms` | ◐ Bearer, none default | ◐ infer 200 | ✅ type+`max_context_length` | ✅ | `/api/v1/models` (v0 fallback) w/ `state`,`compatibility_type` | -| **llama-server** | 8080 | oai | ✅ `/v1/models` | ◐ `/props`,`/slots` (single) | ❌ (1/instance) | ❌ classic | ◐ none / `--api-key` | ✅ `/health` | ◐ ctx via `meta.n_ctx`, router args, `/props` | ✅ | `/props` w/ `default_generation_settings`+`build_info` | -| **llama-swap** | 8080 | oai/ant | ✅ `/v1/models` (all config) | ✅ `/running` | ✅ via `model` → restart upstream | ✅ `/api/models/unload`, ttl | ◐ optional multi-scheme | ✅ `/health`→OK | ◐ ctx via `context_length`; output unknown | ✅ | `/` → `/ui/`; `/running`,`/upstream/{model}` | -| **vLLM** | 8000 | oai | ✅ `/v1/models` | ◐ `/is_sleeping` (dev) | ❌ base · ◐ LoRA | ◐ sleep/wake + LoRA | ◐ none / `--api-key` | ✅ `/health` | ◐ `max_model_len` only | ✅ | `/version` + `/metrics` `vllm:` + `owned_by:"vllm"` | -| **OpenAI** | cloud | oai | ✅ `/v1/models` | ❌ | ✅ (pick id) | ❌ managed | ✅ Bearer | ❌ (status page) | ❌ (static table needed) | ✅ | n/a (configured, not probed) | -| **Anthropic** | cloud | ant | ✅ `/v1/models` | ❌ | ✅ (pick id) | ❌ managed | ✅ x-api-key+version | ❌ | ✅ caps + `max_input_tokens` | ✅ | n/a | -| **TabbyAPI** | 5000 | oai | ✅ `/v1/model/list` | ✅ `/v1/model` | ✅ load | ✅ `/v1/model/{load,unload}` | ✅ x-api-key/x-admin-key | ◐ | ◐ | ✅ | `/v1/model/*` + `x-admin-key` | -| **KoboldCpp** | 5001 | oai | ✅ `/v1/models` | ✅ `/api/v1/model` | ❌ (1 GGUF) | ❌ | ◐ `--password` | ✅ `/api/extra/version` | ◐ | ✅ | `/api/extra/version`→`{"result":"KoboldCpp"}` | -| **oobabooga** | 5000 | oai | ✅ `/v1/models` | ✅ `/v1/internal/model/info` | ✅ load | ✅ `/v1/internal/model/{load,unload}` | ◐ `--api-key` | ◐ | ◐ | ✅ | `/v1/internal/*` namespace | -| **Jan** | 1337 | oai | ✅ `/v1/models` | ◐ | ◐ engine | ◐ engine | ◐ Bearer | ❌ | ◐ | ✅ | weak (log line) | -| **llamafile** | 8080 | oai | ✅ `/v1/models` | ◐ `/props` | ❌ | ❌ | ◐ `--api-key` | ✅ `/health` | ◐ via `/props` | ✅ | `/props` w/ non-`bNNNN` build_info | -| **generic OpenAI-compat** | varies | oai | ✅ `/v1/models` | ❌ | ❌ | ❌ | ◐ optional Bearer | ◐ | ◐ | ✅ | anything serving `/v1/models` (fallback) | +| Backend | port | pi api | listModels | introspectLoaded | switchModel | loadUnload | autoLoadStatus | auth | health | perModelCaps | streaming | discovery fingerprint | +|---|---|---|---|---|---|---|---|---|---|---|---|---| +| **Ollama** | 11434 | oai | ✅ `/api/tags`,`/v1/models` | ✅ `/api/ps` | ✅ implicit (request id) | ✅ `keep_alive:0` | ❌ (always on) | ◐ none local | ✅ `GET /` text | ✅ `/api/show` caps + ctx | ✅ | `GET /` → `Ollama is running` | +| **LM Studio** | 1234 | oai | ✅ `/api/v1/models` (v0 fallback) | ✅ `state` field | ✅ JIT + `/api/v1/models/load` | ✅ load/unload + `lms` | ❌ (always on) | ◐ Bearer, none default | ◐ infer 200 | ✅ type+`max_context_length` | ✅ | `/api/v1/models` (v0 fallback) w/ `state`,`compatibility_type` | +| **llama-server** | 8080 | oai | ✅ `/v1/models` | ◐ `/props`,`/slots` (single) | ❌ (1/instance) | ❌ classic | ❌ (always on) | ◐ none / `--api-key` | ✅ `/health` | ◐ ctx via `meta.n_ctx`, router args, `/props` | ✅ | `/props` w/ `default_generation_settings`+`build_info` | +| **llama-swap** | 8080 | oai/ant | ✅ `/v1/models` (all config) | ✅ `/running` | ✅ via `model` → restart upstream | ✅ `/api/models/unload`, ttl | ❌ (always on) | ◐ optional multi-scheme | ✅ `/health`→OK | ◐ ctx via `context_length`; output unknown | ✅ | `/` → `/ui/`; `/running`,`/upstream/{model}` | +| **vLLM** | 8000 | oai | ✅ `/v1/models` | ◐ `/is_sleeping` (dev) | ❌ base · ◐ LoRA | ◐ sleep/wake + LoRA | ❌ | ◐ none / `--api-key` | ✅ `/health` | ◐ `max_model_len` only | ✅ | `/version` + `/metrics` `vllm:` + `owned_by:"vllm"` | +| **OpenAI** | cloud | oai | ✅ `/v1/models` | ❌ | ✅ (pick id) | ❌ managed | ❌ | ✅ Bearer | ❌ (status page) | ❌ (static table needed) | ✅ | n/a (configured, not probed) | +| **Anthropic** | cloud | ant | ✅ `/v1/models` | ❌ | ✅ (pick id) | ❌ managed | ❌ | ✅ x-api-key+version | ❌ | ✅ caps + `max_input_tokens` | ✅ | n/a | +| **TabbyAPI** | 5000 | oai | ✅ `/v1/model/list` | ✅ `/v1/model` | ✅ load | ✅ `/v1/model/{load,unload}` | ❌ | ✅ x-api-key/x-admin-key | ◐ | ◐ | ✅ | `/v1/model/*` + `x-admin-key` | +| **KoboldCpp** | 5001 | oai | ✅ `/v1/models` | ✅ `/api/v1/model` | ❌ (1 GGUF) | ❌ | ❌ | ◐ `--password` | ✅ `/api/extra/version` | ◐ | ✅ | `/api/extra/version`→`{"result":"KoboldCpp"}` | +| **oobabooga** | 5000 | oai | ✅ `/v1/models` | ✅ `/v1/internal/model/info` | ✅ load | ✅ `/v1/internal/model/{load,unload}` | ❌ | ◐ `--api-key` | ◐ | ◐ | ✅ | `/v1/internal/*` namespace | +| **Jan** | 1337 | oai | ✅ `/v1/models` | ◐ | ◐ engine | ◐ engine | ❌ | ◐ Bearer | ❌ | ◐ | ✅ | weak (log line) | +| **llamafile** | 8080 | oai | ✅ `/v1/models` | ◐ `/props` | ❌ | ❌ | ❌ (always on) | ◐ `--api-key` | ✅ `/health` | ◐ via `/props` | ✅ | `/props` w/ non-`bNNNN` build_info | +| **Unsloth Studio** | 8888 | oai | ✅ `/v1/models` (keyed) | ✅ per-model `loaded` field | ❌ (UI-only) | ❌ (UI-only) | ✅ `/api/settings/openai-auto-switch` | ✅ Bearer `sk-unsloth-…` (required) | ❌ (poll via listModels) | ◐ ctx fields only while loaded | ✅ | `Server: unsloth-studio` header (every response) | +| **generic OpenAI-compat** | varies | oai | ✅ `/v1/models` | ❌ | ❌ | ❌ | ❌ | ◐ optional Bearer | ◐ | ◐ | ✅ | anything serving `/v1/models` (fallback) | ## Capability-driven UX rules (derived) @@ -33,6 +34,12 @@ adapter registers under (`oai` = `openai-completions`, `ant` = `anthropic-messag key for inference (a `401` on `/v1/chat/completions` but `200` on `/v1/models` ⇒ "running but keyed"). - **loadUnload ✅** (Ollama, LM Studio, TabbyAPI, oobabooga, llama-swap) → expose explicit load/unload; elsewhere degrade to implicit-on-use (Ollama) or nothing. +- **autoLoadStatus ✅** (Unsloth Studio) → when the backend answers `false`, mark not-loaded models + with ○ + "no auto-load" in the picker, add a footnote pointing at the backend's own load UI, + and make them non-confirmable (Enter is swallowed with an explanation); if *no* model is loaded + the picker doesn't open at all and the notification says so. `undefined` (unknown/older versions) + never marks or blocks. Backends that always auto-load (Ollama, LM Studio, llama-server, + llama-swap, llamafile) need no query — they are never marked. ## Discovery probe order (cheapest/most-specific first) diff --git a/CHANGES.md b/CHANGES.md new file mode 100644 index 0000000..aa3081c --- /dev/null +++ b/CHANGES.md @@ -0,0 +1,79 @@ +## Fix: contextWindow and maxTokens not read from OpenAI-compatible servers + +### maxTokens Issue + +**Problem**: The generic OpenAI-compatible adapter always used a hardcoded default of 4096 for `maxTokens`, even when servers reported `max_completion_tokens` in the `/v1/models` response. This was way too small for modern models with large context windows. + +**Fix**: +1. First determine the `contextWindow` (checking `context_window`, `max_model_len`, `context_length`, `max_context_length`) +2. Try to read `max_completion_tokens` from the server response +3. If the server provides it, use that value +4. Otherwise, default to **half the context window** (reasonable balance between input and output) + +### contextWindow Issue + +### Problem + +The generic OpenAI-compatible adapter in `src/adapters/generic.ts` only checks for these context-window fields in the `/v1/models` response: + +- `context_window` +- `context_length` +- `max_context_length` + +Many servers (e.g. **omlx**) report the value under **`max_model_len`** instead. When this field is encountered, `contextWindow` stays `undefined` and a default fallback is used, giving wrong values to the user. + +### Fix + +In `src/adapters/generic.ts`: + +1. **Add `max_model_len` to the response type** so TypeScript recognizes the field. +2. **Insert `max_model_len` into the context-window detection chain** as the second check (after `context_window`, before `context_length` and `max_context_length`). + +### Files changed + +- `src/adapters/generic.ts` + +### Diff + +```diff +--- a/src/adapters/generic.ts ++++ b/src/adapters/generic.ts +@@ -122,7 +122,7 @@ + headers["Authorization"] = `Bearer ${cred.apiKey}`; + } + +- const body = r.json as { data?: Array<{ id?: unknown; max_completion_tokens?: number; context_length?: number; max_context_length?: number; context_window?: number }> } | undefined; ++ const body = r.json as { data?: Array<{ id?: unknown; max_completion_tokens?: number; max_model_len?: number; context_length?: number; max_context_length?: number; context_window?: number }> } | undefined; + if (!Array.isArray(body?.data)) return []; + + return body.data +@@ -135,13 +135,18 @@ + const contextWindow = + typeof item.context_window === "number" && item.context_window > 0 + ? item.context_window ++ : typeof item.max_model_len === "number" && item.max_model_len > 0 ++ ? item.max_model_len + : typeof item.context_length === "number" && item.context_length > 0 + ? item.context_length + : typeof item.max_context_length === "number" && item.max_context_length > 0 + ? item.max_context_length + : undefined; + ++ const finalContextWindow = contextWindow ?? DEFAULT_CONTEXT_WINDOW; ++ + const maxTokens = + typeof item.max_completion_tokens === "number" && item.max_completion_tokens > 0 + ? item.max_completion_tokens ++ : Math.floor(finalContextWindow / 2); + + return { + id: item.id, + name: item.id, +- contextWindow: contextWindow ?? DEFAULT_CONTEXT_WINDOW, +- maxTokens: DEFAULT_MAX_TOKENS, ++ contextWindow: finalContextWindow, ++ maxTokens, + input: ["text"], + reasoning: false, + embeddings: isEmbedding, +``` diff --git a/package-lock.json b/package-lock.json index 45fe4d3..2a79e74 100644 --- a/package-lock.json +++ b/package-lock.json @@ -678,9 +678,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -698,9 +695,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -718,9 +712,6 @@ "riscv64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -738,9 +729,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -758,9 +746,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -2172,9 +2157,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -2192,9 +2174,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -2212,9 +2191,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -2232,9 +2208,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -2252,9 +2225,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -2272,9 +2242,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -2780,9 +2747,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -2804,9 +2768,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -2828,9 +2789,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -2852,9 +2810,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ diff --git a/src/adapters/generic.ts b/src/adapters/generic.ts index a2a85fc..552572c 100644 --- a/src/adapters/generic.ts +++ b/src/adapters/generic.ts @@ -91,7 +91,7 @@ class GenericAdapter implements BackendAdapter { /** * GET /v1/models → map data[].id to ModelDescriptor. - * Conservative defaults are applied (contextWindow 8192, maxTokens 4096, input ["text"]). + * Conservative defaults are applied (contextWindow 8192, maxTokens = half of contextWindow, input ["text"]). * Common embedding/reranking model families are excluded from chat registration. * Throws on non-ok / 401 / status:0. */ @@ -112,21 +112,52 @@ class GenericAdapter implements BackendAdapter { if (r.status === 0) throw new Error("listModels failed: server unreachable (status 0)"); if (!r.ok) throw new Error(`listModels failed: HTTP ${r.status}`); - const body = r.json as { data?: Array<{ id?: unknown }> } | undefined; + interface GenericModelEntry { + id?: unknown; + max_completion_tokens?: number; + max_model_len?: number; + context_length?: number; + max_context_length?: number; + context_window?: number; + } + interface GenericModelEntryWithId extends GenericModelEntry { + id: string; + } + + const body = r.json as { data?: GenericModelEntry[] } | undefined; if (!Array.isArray(body?.data)) return []; return body.data - .filter((item): item is { id: string } => typeof item?.id === "string") + .filter((item): item is GenericModelEntryWithId => typeof item?.id === "string") .map((item): ModelDescriptor => { const normalizedId = item.id.toLowerCase(); const isEmbedding = /(^|[/:._-])(embed|embedding|bge|gte|e5|reranker)([/:._-]|$)/.test(normalizedId) || normalizedId.includes("nomic-embed"); + + const contextWindow = + typeof item.context_window === "number" && item.context_window > 0 + ? item.context_window + : typeof item.max_model_len === "number" && item.max_model_len > 0 + ? item.max_model_len + : typeof item.context_length === "number" && item.context_length > 0 + ? item.context_length + : typeof item.max_context_length === "number" && item.max_context_length > 0 + ? item.max_context_length + : undefined; + + const finalContextWindow = contextWindow ?? DEFAULT_CONTEXT_WINDOW; + + const maxTokens = + typeof item.max_completion_tokens === "number" && item.max_completion_tokens > 0 + ? item.max_completion_tokens + : Math.floor(finalContextWindow / 2); + return { id: item.id, name: item.id, - contextWindow: DEFAULT_CONTEXT_WINDOW, - maxTokens: DEFAULT_MAX_TOKENS, + contextWindow: finalContextWindow, + maxTokens, input: ["text"], reasoning: false, embeddings: isEmbedding, diff --git a/src/adapters/index.ts b/src/adapters/index.ts index a5b0269..56475e6 100644 --- a/src/adapters/index.ts +++ b/src/adapters/index.ts @@ -19,6 +19,7 @@ import { vllmAdapter } from "./vllm.ts"; import { omlxAdapter } from "./omlx.ts"; import { openaiAdapter } from "./openai.ts"; import { anthropicAdapter } from "./anthropic.ts"; +import { unslothAdapter } from "./unsloth.ts"; import { genericAdapter } from "./generic.ts"; /** Every adapter Crossbar ships. */ @@ -31,6 +32,7 @@ export const ADAPTERS: readonly BackendAdapter[] = [ omlxAdapter, openaiAdapter, anthropicAdapter, + unslothAdapter, genericAdapter, ]; @@ -63,5 +65,6 @@ export { omlxAdapter, openaiAdapter, anthropicAdapter, + unslothAdapter, genericAdapter, }; diff --git a/src/adapters/unsloth.ts b/src/adapters/unsloth.ts new file mode 100644 index 0000000..cf4e5a3 --- /dev/null +++ b/src/adapters/unsloth.ts @@ -0,0 +1,311 @@ +/** + * Unsloth Studio backend adapter for Crossbar. + * + * Unsloth Studio (https://unsloth.ai) serves an OpenAI/Anthropic-compatible surface + * (`/v1/models`, `/v1/chat/completions`, `/v1/messages`, `/v1/responses`, `/v1/completions`, + * `/v1/embeddings`). Unlike most local backends, it has NO unauthenticated mode: every + * request — including `GET /v1/models` — requires `Authorization: Bearer sk-unsloth-…`. + * + * `authRequired: true` tells the onboarding flow this backend can never be added with + * `auth: "none"` — see ARCHITECTURE.md and the BackendAdapter contract. + * + * # Fingerprint discriminator — verified against a live instance (2026-08-19) + * + * Unsloth Studio sets `Server: unsloth-studio` on EVERY response — 200, 401 with no + * Authorization header, and 401 with a wrong/expired key alike. Confirmed via curl against a + * running server: + * + * $ curl -sD- https://:8888/v1/models # no header + * HTTP/2 401 + * server: unsloth-studio + * www-authenticate: Bearer + * {"error":{"message":"Not authenticated","type":"authentication_error","param":null,"code":null}} + * + * $ curl -sD- https://:8888/v1/models -H "Authorization: Bearer sk-unsloth-…" + * HTTP/2 200 + * server: unsloth-studio + * {"object":"list","data":[{"id":"unsloth/Qwen3.8-27B-GGUF","owned_by":"unsloth-studio", + * "quant":"UD-Q4_K_XL","context_length":49152,"max_context_length":229888, + * "native_context_length":262144,"loaded":true}, ...]} + * + * This is a real, explicit, always-present product header — a MUCH stronger discriminator + * than guessing at error-message wording (an earlier version of this adapter tried to match + * a hypothetical FastAPI `{"detail": "Missing authentication token"}` 401 body, which turned + * out not to match the actual server at all: the real 401 body is an OpenAI-style + * `{"error": {"type": "authentication_error", ...}}` envelope instead). The `Server` header + * lets `fingerprint()` positively identify Unsloth Studio — AND flag that it needs a key — + * from a single unauthenticated probe, before the user has entered a working key at all, + * instead of falling through every adapter to the generic "could not identify the server" + * dead end. Each model entry in the authenticated `data[]` also self-reports + * `owned_by: "unsloth-studio"` and a `loaded: boolean` residency flag, used below for + * IntrospectLoaded. + * + * Uses ONLY the injected Probe — never calls fetch directly. + */ + +import { Capability } from "../core/capability.ts"; +import type { BackendAdapter, PiApiType } from "../core/backend-adapter.ts"; +import type { + DiscoveredServer, + LoadedState, + ModelDescriptor, + PiModelEntry, + Probe, + ServerCredential, +} from "../core/types.ts"; + +// --------------------------------------------------------------------------- +// API response shapes +// --------------------------------------------------------------------------- + +interface UnslothModelEntry { + id: string; + owned_by?: string; + quant?: string; + display_name?: string; + /** Currently-configured context for a loaded model. Absent when not loaded. */ + context_length?: number; + /** Usable context ceiling (may be less than native due to available VRAM/RAM). */ + max_context_length?: number; + /** The model's absolute trained/architectural context length. */ + native_context_length?: number; + loaded?: boolean; +} + +interface UnslothModelsResponse { + data?: UnslothModelEntry[]; +} + +/** + * Shape of `GET /api/settings/openai-auto-switch` — the API surface of the + * "Switch model by request" toggle (Settings ▸ API in the Studio UI). Only + * `enabled` matters here; the rest of the body is ignored. + */ +interface UnslothAutoSwitchSettings { + enabled?: unknown; +} + +/** The literal header value Unsloth Studio sets on every response. */ +const SERVER_HEADER_VALUE = "unsloth-studio"; + +// --------------------------------------------------------------------------- +// Constants +// --------------------------------------------------------------------------- + +/** Unsloth Studio's documented default port (`UNSLOTH_STUDIO_URL` default). */ +const DEFAULT_PORT = 8888; + +/** + * Settings endpoint backing the "Switch model by request" toggle. Discovered in the + * Studio frontend bundle and verified against a live instance (2026-08-19): + * `GET /api/settings/openai-auto-switch` → 200 `{ "enabled": false, ... }` with a + * bearer key; the toggle's PUT writes the same object back. + */ +const AUTO_SWITCH_SETTINGS_PATH = "/api/settings/openai-auto-switch"; + +/** + * Fallback context used ONLY at the Pi-mapping boundary, where the field is mandatory and a + * model with no known context would otherwise be unusable. Matches the llama.cpp/llama-swap + * adapters. `maxTokens: 0` means "no client-side cap — let the server decide". + */ +const FALLBACK_CONTEXT_WINDOW = 128_000; +const FALLBACK_MAX_TOKENS = 0; + +function isUnslothStudioResponse(headers: Record): boolean { + // Probe lowercases header names AND we compare the value case-insensitively — cheap + // insurance against a future casing change upstream, no behavioural cost today. + const server = headers["server"]; + return typeof server === "string" && server.toLowerCase() === SERVER_HEADER_VALUE; +} + +function isEmbeddingId(id: string): boolean { + const normalized = id.toLowerCase(); + return ( + /(^|[/:._-])(embed|embedding|bge|gte|e5|reranker)([/:._-]|$)/.test(normalized) || + normalized.includes("nomic-embed") + ); +} + +function positiveSafeInteger(value: unknown): number | undefined { + return typeof value === "number" && Number.isSafeInteger(value) && value > 0 ? value : undefined; +} + +/** + * Prefer the model's currently-configured context (only present while loaded), then the + * usable ceiling, then the architectural native max. + * + * Returns `undefined` — NOT a fabricated default — when the server reports none of them. + * Verified against a live instance: Unsloth Studio emits the three `*context_length` fields + * ONLY for models that are currently loaded; every unloaded entry carries no context + * information at all. Inventing a number here (this used to return 8192) baked a bogus + * "8k ctx" into both the model picker and the `lastKnownModels` cache in crossbar.json for + * every unloaded model — including 262144-context ones. Same rule as the llama.cpp and + * llama-swap adapters: report only what the backend actually said, and let `toPiModel` + * apply the single, clearly-marked fallback. + */ +function contextWindowFor(entry: UnslothModelEntry): number | undefined { + return ( + positiveSafeInteger(entry.context_length) ?? + positiveSafeInteger(entry.max_context_length) ?? + positiveSafeInteger(entry.native_context_length) + ); +} + +// --------------------------------------------------------------------------- +// UnslothAdapter +// --------------------------------------------------------------------------- + +class UnslothAdapter implements BackendAdapter { + readonly kind = "unsloth" as const; + readonly displayName = "Unsloth Studio"; + readonly defaultPorts: readonly number[] = [DEFAULT_PORT]; + readonly piApi: PiApiType = "openai-completions"; + readonly capabilities: ReadonlySet = new Set([ + Capability.ListModels, + Capability.IntrospectLoaded, + Capability.Streaming, + Capability.AutoLoadStatus, + ]); + /** Unsloth Studio rejects every request — including GET /v1/models — without a valid key. */ + readonly authRequired = true; + + // --- fingerprint ------------------------------------------------------------------------ + + async fingerprint(baseUrl: string, probe: Probe): Promise { + const r = await probe("/v1/models"); + if (r.status === 0) return null; + + // The `Server: unsloth-studio` header is present on every response this backend gives, + // authenticated or not — the one thing that's actually unique to this product (see the + // header comment above for a verified capture of both branches). + if (!isUnslothStudioResponse(r.headers)) return null; + + return { + kind: "unsloth", + baseUrl, + // The backend requires a key unconditionally, regardless of whether THIS particular + // probe happened to carry a working one. + auth: "apiKey", + label: `Unsloth Studio (${baseUrl.replace(/^https?:\/\//, "")})`, + confidence: 0.95, + }; + } + + // --- listModels --------------------------------------------------------------------------- + + async listModels( + _server: DiscoveredServer, + cred: ServerCredential, + probe: Probe, + ): Promise { + const headers: Record = {}; + if (cred.mode === "apiKey" && cred.apiKey) { + headers["Authorization"] = `Bearer ${cred.apiKey}`; + } + + const r = await probe("/v1/models", { headers }); + + if (r.status === 401) throw new Error("401 Unauthorized: invalid or missing Unsloth API key"); + if (r.status === 0) throw new Error("listModels failed: server unreachable (status 0)"); + if (!r.ok) throw new Error(`listModels failed: HTTP ${r.status}`); + + const body = r.json as UnslothModelsResponse | undefined; + if (!Array.isArray(body?.data)) return []; + + return body.data + .filter((entry): entry is UnslothModelEntry => typeof entry?.id === "string") + .map((entry): ModelDescriptor => { + const contextWindow = contextWindowFor(entry); + const descriptor: ModelDescriptor = { + id: entry.id, + name: entry.display_name ?? entry.id, + input: ["text"], + reasoning: false, + embeddings: isEmbeddingId(entry.id), + loaded: entry.loaded === true, + raw: entry, + }; + // Omitted entirely when unknown, so the cached descriptor never asserts a context + // the server did not report — and picks up the real value once the model is loaded. + if (contextWindow !== undefined) descriptor.contextWindow = contextWindow; + return descriptor; + }); + } + + // --- introspectLoaded ---------------------------------------------------------------------- + + /** + * Each `/v1/models` entry self-reports `loaded: boolean` — no separate endpoint needed. + * Reuses `listModels`'s parsing so the two never drift on field handling. + */ + async introspectLoaded( + server: DiscoveredServer, + cred: ServerCredential, + probe: Probe, + ): Promise { + const models = await this.listModels(server, cred, probe); + return { + loadedModelIds: models.filter((m) => m.loaded === true).map((m) => m.id), + source: "introspection", + }; + } + + // --- autoLoadsOnDemand ------------------------------------------------------------------------ + + /** + * Reads the "Switch model by request" setting (Settings ▸ API). When it is OFF, a request + * naming an unloaded model 404s with a `model_not_found` error that points at exactly this + * setting — so the picker must mark those models instead of letting the user pick one and + * watch the first turn fail. When ON, unloaded models are loaded on demand and need no mark. + * + * Defensive by contract: a non-200 (old server versions without the endpoint 404), a + * missing/malformed body, or a non-boolean `enabled` all yield `undefined` (unknown) — + * the caller then shows the picker unmarked, i.e. today's behaviour. + */ + async autoLoadsOnDemand( + _server: DiscoveredServer, + cred: ServerCredential, + probe: Probe, + ): Promise { + const headers: Record = {}; + if (cred.mode === "apiKey" && cred.apiKey) { + headers["Authorization"] = `Bearer ${cred.apiKey}`; + } + + const r = await probe(AUTO_SWITCH_SETTINGS_PATH, { headers }); + if (!r.ok || r.status !== 200) return undefined; + + const body = r.json as UnslothAutoSwitchSettings | undefined; + return typeof body?.enabled === "boolean" ? body.enabled : undefined; + } + + // --- toPiModel ------------------------------------------------------------------------------ + + toPiModel(_server: DiscoveredServer, model: ModelDescriptor): PiModelEntry { + return { + id: model.id, + name: model.name, + reasoning: model.reasoning ?? false, + input: model.input.length > 0 ? model.input : ["text"], + // Local inference is free — cost is zero — but cache-hit token COUNTS still matter, so + // streaming usage stays enabled (never fabricated) in case llama-server reports them. + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: positiveSafeInteger(model.contextWindow) ?? FALLBACK_CONTEXT_WINDOW, + maxTokens: positiveSafeInteger(model.maxTokens) ?? FALLBACK_MAX_TOKENS, + compat: { supportsUsageInStreaming: true }, + }; + } + + // --- inferenceBaseUrl ------------------------------------------------------------------------ + + inferenceBaseUrl(server: DiscoveredServer): string { + const stripped = server.baseUrl.endsWith("/") ? server.baseUrl.slice(0, -1) : server.baseUrl; + return stripped.endsWith("/v1") ? stripped : `${stripped}/v1`; + } +} + +// --------------------------------------------------------------------------- +// Singleton export +// --------------------------------------------------------------------------- + +export const unslothAdapter: BackendAdapter = new UnslothAdapter(); diff --git a/src/core/backend-adapter.ts b/src/core/backend-adapter.ts index c419b2e..2906dfb 100644 --- a/src/core/backend-adapter.ts +++ b/src/core/backend-adapter.ts @@ -28,8 +28,15 @@ import type { ServerCredential, } from "./types.ts"; -/** Bumped on any breaking change to this interface. Adapters and the registry assert on it. */ -export const CONTRACT_VERSION = 2 as const; +/** + * Bumped on any change to this interface. Adapters and the registry assert on it. + * v3: added optional `authRequired` (non-breaking — existing adapters are unaffected; only + * backends that can never work without a key, e.g. Unsloth Studio, need to set it). + * v4: added optional `autoLoadsOnDemand` + `Capability.AutoLoadStatus` (non-breaking — + * existing adapters are unaffected; only backends that can answer authoritatively, + * e.g. Unsloth Studio's "Switch model by request" setting, implement it). + */ +export const CONTRACT_VERSION = 4 as const; /** Which built-in Pi API type the adapter registers its models under. */ export type PiApiType = "openai-completions" | "anthropic-messages"; @@ -45,6 +52,14 @@ export interface BackendAdapter { readonly piApi: PiApiType; /** The capabilities this backend exposes. Drives UX and which optional methods are present. */ readonly capabilities: ReadonlySet; + /** + * True when this backend rejects EVERY request without a valid API key — there is no + * unauthenticated mode at all (e.g. Unsloth Studio). Defaults to false (most local backends + * are keyless by default). Onboarding uses this to skip/short-circuit the "No authentication" + * choice for adapters that can never work without a key, instead of silently failing + * fingerprint and surfacing a generic "could not identify the server" error. + */ + readonly authRequired?: boolean; /** * Decide whether `baseUrl` is *this* backend. MUST use only unauthenticated metadata endpoints @@ -79,6 +94,20 @@ export interface BackendAdapter { probe: Probe, ): Promise; + /** + * Will this server auto-load (or switch to) an UNLOADED model when a request names it? + * Present iff {@link Capability.AutoLoadStatus}. Informational — the UI uses it to warn + * that a model must be loaded via the backend's own interface before it can serve + * requests. `true` = unloaded models are served on demand; `false` = requests for + * unloaded models fail until the user loads them elsewhere; `undefined` = the server + * did not answer authoritatively (treat as unknown, never guess). + */ + autoLoadsOnDemand?( + server: DiscoveredServer, + cred: ServerCredential, + probe: Probe, + ): Promise; + /** Explicit load/unload. Present iff {@link Capability.LoadUnload}. */ loadUnload?( server: DiscoveredServer, @@ -121,3 +150,9 @@ export function canLoadUnload( ): a is BackendAdapter & Required> { return typeof a.loadUnload === "function"; } + +export function canAutoLoadStatus( + a: BackendAdapter, +): a is BackendAdapter & Required> { + return typeof a.autoLoadsOnDemand === "function"; +} diff --git a/src/core/capability.ts b/src/core/capability.ts index f939473..08c2561 100644 --- a/src/core/capability.ts +++ b/src/core/capability.ts @@ -25,6 +25,12 @@ export enum Capability { PerModelCaps = "perModelCaps", /** Supports streaming responses. Effectively universal for the chat path. */ Streaming = "streaming", + /** + * Can report whether unloaded models are auto-loaded when a request names them + * (e.g. Unsloth Studio's "Switch model by request" setting). Informational — + * drives picker warnings, not actions. + */ + AutoLoadStatus = "autoLoadStatus", } /** Authentication scheme a server requires. Crossbar only ever sends a bearer/api key or nothing. */ @@ -48,6 +54,7 @@ export type BackendKind = | "oobabooga" | "jan" | "llamafile" + | "unsloth" | "openai-generic"; /** Backends that are remote cloud services (configured, never port-probed). */ diff --git a/src/core/index.ts b/src/core/index.ts index cfcc80a..b689880 100644 --- a/src/core/index.ts +++ b/src/core/index.ts @@ -9,6 +9,7 @@ export { export { CONTRACT_VERSION, + canAutoLoadStatus, canIntrospect, canLoadUnload, canSwitch, diff --git a/src/core/types.ts b/src/core/types.ts index b6cffac..c8fb829 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -175,8 +175,13 @@ export interface CrossbarSettings { export interface CrossbarConfigFile { version: 1; - /** Version of the cached model-capability data. Missing or unknown values are migrated on load. */ - modelCacheVersion?: 1; + /** + * Version of the cached model-capability data. Missing or unknown values are migrated on + * load. Bumped to 2 when the Unsloth Studio adapter stopped fabricating an 8192/4096 + * context for models the server reports no context for, so already-written caches carrying + * those invented values get re-sanitized once. + */ + modelCacheVersion?: 1 | 2; servers: ServerRecord[]; settings?: CrossbarSettings; } diff --git a/src/index.ts b/src/index.ts index 6ebd90d..7e0889f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -25,7 +25,7 @@ import { createProbe } from "./discovery/probe.ts"; import { catalogueChanged, pollAll } from "./poll.ts"; import { preloadCachedProviders } from "./preload.ts"; import { loadConfig, saveConfig } from "./registry/persistence.ts"; -import { createPiCredentialStore } from "./registry/pi-credential-store.ts"; +import { createAuthJsonCredentialStore } from "./registry/auth-json-credential-store.ts"; import { serverId } from "./registry/ids.ts"; import { ServerRegistry } from "./registry/registry.ts"; import { registerServer, unregisterServer } from "./shim/provider-shim.ts"; @@ -173,7 +173,7 @@ export default async function crossbar(pi: ExtensionAPI): Promise { pollTimer = undefined; } - const store = createPiCredentialStore(ctx.modelRegistry.authStorage); + const store = createAuthJsonCredentialStore(); const reg = new ServerRegistry({ store, persist: (cfg) => saveConfig(cfg) }); const cfg = await loadConfig(); reg.load(cfg); // registry now owns discovery settings (cfg.settings) diff --git a/src/preload.ts b/src/preload.ts index 43341e0..dd55152 100644 --- a/src/preload.ts +++ b/src/preload.ts @@ -6,12 +6,23 @@ * pi.registerProvider for each enabled server that has a cached model catalogue. * It NEVER performs network requests, discovery, UI work, timers, or credential * writes, and it NEVER throws — a failure here must not prevent Pi from starting. + * + * It DOES perform one credential READ: for `auth: "apiKey"` records, it looks up the + * already-persisted key (auth.json, via the same store `registerServer()` uses at + * session_start) and bridges it into `process.env[envVarFor(record.id)]` — the exact + * variable the `$ENV` sentinel in `buildProviderConfig()` references. Without this, a keyed + * server's preloaded models would be unusable (`pi.setModel()` returns false) for the brief + * window between factory load and `session_start`'s authoritative `refreshAndRegister()` + * pass, which performs the same bridge. See `shim/provider-shim.ts`'s header for why Pi's own + * config-value resolver needs this rather than resolving auth.json by provider id itself. */ import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; import type { PersistenceOpts } from "./registry/persistence.ts"; import { loadConfig } from "./registry/persistence.ts"; import { registerCachedServer } from "./shim/provider-shim.ts"; +import { createAuthJsonCredentialStore } from "./registry/auth-json-credential-store.ts"; +import { envVarFor } from "./registry/ids.ts"; /** * Read crossbar.json and register each enabled server with a non-empty chat @@ -32,10 +43,23 @@ export async function preloadCachedProviders( return; } + const credentialStore = createAuthJsonCredentialStore( + opts?.dir ? { authPath: `${opts.dir}/auth.json` } : undefined, + ); + for (const record of cfg.servers) { try { if (!record.enabled) continue; if (!record.lastKnownModels || record.lastKnownModels.length === 0) continue; + if (record.auth === "apiKey") { + const key = await credentialStore.get(record.id); + // Bridge the key into process.env when we already have one cached; when we don't + // (first-ever load before any key was entered, or a store read failure), still + // register with the unresolved `$ENV` sentinel exactly as before this bridge existed + // — session_start's refreshAndRegister() re-registers every enabled record shortly + // after with an authoritative credential resolution regardless. + if (key !== undefined) process.env[envVarFor(record.id)] = key; + } registerCachedServer(pi, record, record.lastKnownModels); } catch { // One malformed record must not block others. diff --git a/src/registry/auth-json-credential-store.ts b/src/registry/auth-json-credential-store.ts new file mode 100644 index 0000000..0c04542 --- /dev/null +++ b/src/registry/auth-json-credential-store.ts @@ -0,0 +1,142 @@ +/** + * Direct auth.json–backed CredentialStore. + * + * Background: pi-coding-agent 0.80+ removed the extension-facing `AuthStorage` + * class from its public SDK surface (see CHANGELOG "Replaced the SDK's + * `CreateAgentSessionOptions.authStorage` and `modelRegistry` options with the + * async `modelRuntime` option. `AuthStorage` and its storage backends are no + * longer exported"). `ExtensionContext.modelRegistry` no longer exposes + * `authStorage` at all, so `createPiCredentialStore()` (which read + * `ctx.modelRegistry.authStorage`) crashes with "Cannot read properties of + * undefined (reading 'set')" the moment Crossbar tries to persist a key. + * + * Only `readStoredCredential()` (one-off, read-only) is still exported by the + * SDK. There is currently no supported write path for extensions. + * + * This module restores Crossbar's original guarantee — "secrets live only in + * Pi's auth.json, never crossbar.json" — by reading/writing that file directly, + * in the exact flat-map shape Pi itself uses: + * + * { "": { "type": "api_key", "key": "<...>" }, ... } + * + * Writes are read-modify-write + atomic rename (temp file on the same + * filesystem), so a concurrent Pi-side write (e.g. an OAuth login finishing + * around the same time) can only ever lose one side's *own* key entry in the + * unlikely event both writes race — never corrupt the file. Every entry + * belonging to other providers (anthropic, openrouter, github-copilot, ...) + * is preserved verbatim; Crossbar only ever touches its own provider ids. + * + * File mode is forced to 0600, matching Pi's own auth.json permissions. + * + * # Why built-in providers (anthropic, openai, github-copilot, ...) still "just work" + * + * This might look contradictory at first: Crossbar needed this whole module because + * `AuthStorage` was removed, yet `/login` for Anthropic/OpenAI/GitHub Copilot and typing an + * API key for one of them still transparently reads and writes `auth.json` with no issue. + * The resolution is that pi-coding-agent 0.80.8 removed `AuthStorage` from the EXTENSION-FACING + * SDK surface only — not from pi-coding-agent itself. Built-in providers are driven by Pi's + * internal `ModelRuntime` (`setRuntimeApiKey()`, `login()`, etc. — see + * `dist/core/model-runtime.d.ts`), which keeps full, privileged, in-process access to the same + * `auth.json`. The CHANGELOG line is precise about the scope: + * + * "AuthStorage and its storage backends are no longer exported" + * + * — exported meaning exported *to extensions*, not removed from the core. What actually + * disappeared is only the bridge extensions used to reach in from outside + * (`CreateAgentSessionOptions.authStorage`, `ExtensionContext.modelRegistry.authStorage`). + * + * Crossbar doesn't register its backends (Unsloth Studio, llama.cpp, vLLM, ...) as built-in + * providers — it registers them through the EXTENSION provider API, `pi.registerProvider(id, + * config)`, which is exactly the API that lost its privileged path into `auth.json` when + * `AuthStorage` stopped being exported. This module doesn't bypass any security boundary Pi + * introduced; it replicates, for Crossbar's own provider ids, the same read/write/0600 + * behaviour Pi's internal `ModelRuntime` already performs for its built-in providers — and, + * per the read-modify-write contract above, never touches an id it doesn't own. + */ + +import { readFileSync, writeFileSync, renameSync, chmodSync, existsSync } from "node:fs"; +import { join } from "node:path"; +import { getAgentDir } from "@earendil-works/pi-coding-agent"; +import type { CredentialStore } from "./persistence.ts"; + +interface ApiKeyCredential { + type: "api_key"; + key: string; +} + +/** Other credential shapes (oauth, etc.) are opaque to us — preserved as-is. */ +type AuthJsonEntry = ApiKeyCredential | Record; +type AuthJsonData = Record; + +export interface AuthJsonStoreOpts { + /** Override the auth.json path (tests only). Default: getAgentDir()/auth.json. */ + authPath?: string; +} + +function resolvePath(opts?: AuthJsonStoreOpts): string { + return opts?.authPath ?? join(getAgentDir(), "auth.json"); +} + +function readAll(path: string): AuthJsonData { + try { + if (!existsSync(path)) return {}; + const text = readFileSync(path, "utf-8"); + if (text.trim().length === 0) return {}; + const parsed = JSON.parse(text) as unknown; + if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { + return parsed as AuthJsonData; + } + return {}; + } catch { + // Missing, unreadable, or corrupt — treat as empty rather than throwing. + // A subsequent write will recreate the file; other providers' credentials + // may already be unrecoverable at that point, but we never make it worse. + return {}; + } +} + +function writeAll(path: string, data: AuthJsonData): void { + const json = JSON.stringify(data, null, 2); + const tmp = `${path}.crossbar-${Date.now()}-${Math.random().toString(36).slice(2)}.tmp`; + writeFileSync(tmp, json, { encoding: "utf-8", mode: 0o600 }); + renameSync(tmp, path); + try { + chmodSync(path, 0o600); + } catch { + // Best effort — some filesystems (e.g. certain network mounts) reject chmod. + } +} + +function isApiKeyCredential(entry: AuthJsonEntry | undefined): entry is ApiKeyCredential { + return !!entry && (entry as Record)["type"] === "api_key" && typeof (entry as Record)["key"] === "string"; +} + +/** + * Build a {@link CredentialStore} that reads/writes auth.json directly. + * Every operation re-reads the file first so concurrent external changes + * (Pi logging a provider in/out in the same process) are never clobbered + * for keys other than the one being touched. + */ +export function createAuthJsonCredentialStore(opts?: AuthJsonStoreOpts): CredentialStore { + const path = resolvePath(opts); + + return { + get(id: string): string | undefined { + const data = readAll(path); + const entry = data[id]; + return isApiKeyCredential(entry) ? entry.key : undefined; + }, + set(id: string, key: string): void { + const data = readAll(path); + data[id] = { type: "api_key", key }; + writeAll(path, data); + }, + remove(id: string): void { + const data = readAll(path); + if (id in data) { + delete data[id]; + writeAll(path, data); + } + }, + }; +} diff --git a/src/registry/persistence.ts b/src/registry/persistence.ts index c227bbc..ac13c94 100644 --- a/src/registry/persistence.ts +++ b/src/registry/persistence.ts @@ -49,18 +49,26 @@ function stripSecrets(record: ServerRecord): ServerRecord { return safe; } -const MODEL_CACHE_VERSION = 1 as const; +const MODEL_CACHE_VERSION = 2 as const; + +/** Backends whose cached model entries may contain a fabricated 8192/4096 context pair. */ +const FABRICATED_CONTEXT_KINDS = new Set([ + "llamaswap", + "llamacpp", + "unsloth", +]); function sanitizeLegacyModel(model: ModelDescriptor, kind: ServerRecord["kind"]): ModelDescriptor { - // llama-swap's 8192 was fabricated; positive llama.cpp contexts may be authoritative. + // llama-swap's and Unsloth Studio's 8192 was fabricated (Unsloth reports context fields only + // for LOADED models, so every unloaded entry got the invented default baked into the cache); + // positive llama.cpp contexts may be authoritative. const removeContextWindow = - kind === "llamaswap" + kind === "llamaswap" || kind === "unsloth" ? model.contextWindow === 8192 : kind === "llamacpp" && typeof model.contextWindow === "number" && model.contextWindow <= 0; - const removeMaxTokens = - (kind === "llamaswap" || kind === "llamacpp") && model.maxTokens === 4096; + const removeMaxTokens = FABRICATED_CONTEXT_KINDS.has(kind) && model.maxTokens === 4096; if (!removeContextWindow && !removeMaxTokens) return model; @@ -71,10 +79,7 @@ function sanitizeLegacyModel(model: ModelDescriptor, kind: ServerRecord["kind"]) } function sanitizeLegacyModelCache(record: ServerRecord): ServerRecord { - if ( - (record.kind !== "llamaswap" && record.kind !== "llamacpp") || - !Array.isArray(record.lastKnownModels) - ) { + if (!FABRICATED_CONTEXT_KINDS.has(record.kind) || !Array.isArray(record.lastKnownModels)) { return record; } diff --git a/src/registry/pi-credential-store.ts b/src/registry/pi-credential-store.ts deleted file mode 100644 index 56a7c56..0000000 --- a/src/registry/pi-credential-store.ts +++ /dev/null @@ -1,27 +0,0 @@ -/** - * Bridges Pi's `AuthStorage` (reached at runtime via `ctx.modelRegistry.authStorage`) to Crossbar's - * framework-free {@link CredentialStore} boundary. This is the ONLY place keys cross into Pi's store — - * they land in `auth.json` (mode 0600) keyed by the Crossbar provider id, exactly like Pi's own creds. - * - * Keeping the adapter here (not in persistence.ts) preserves the rule that the persistence/registry - * core never imports Pi runtime, so they stay unit-testable with a fake store. - */ - -import type { AuthStorage } from "@earendil-works/pi-coding-agent"; -import type { CredentialStore } from "./persistence.ts"; - -/** Wrap a Pi `AuthStorage` as a Crossbar `CredentialStore` (api-key credentials only). */ -export function createPiCredentialStore(authStorage: AuthStorage): CredentialStore { - return { - get(id: string): string | undefined { - const cred = authStorage.get(id); - return cred?.type === "api_key" ? cred.key : undefined; - }, - set(id: string, key: string): void { - authStorage.set(id, { type: "api_key", key }); - }, - remove(id: string): void { - authStorage.remove(id); - }, - }; -} diff --git a/src/registry/registry.ts b/src/registry/registry.ts index 7217635..85cbcc4 100644 --- a/src/registry/registry.ts +++ b/src/registry/registry.ts @@ -239,7 +239,7 @@ export class ServerRegistry { private async flush(): Promise { const config: CrossbarConfigFile = { version: 1, - modelCacheVersion: 1, + modelCacheVersion: 2, servers: this.list(), }; // Preserve discovery settings across every mutation — otherwise any server diff --git a/src/shim/provider-shim.ts b/src/shim/provider-shim.ts index aa70306..6a7c372 100644 --- a/src/shim/provider-shim.ts +++ b/src/shim/provider-shim.ts @@ -12,8 +12,22 @@ * Pi documents this explicitly for local models: the API key is required by Pi, * but local OpenAI-compatible servers ignore it, so any value works. * - * - Keyed server: use the `$ENV` sentinel. Pi reads the real key from auth.json - * first, so no plaintext secret is stored in ProviderConfig. + * - Keyed server: use the `$ENV` sentinel, where the env var name is + * `envVarFor(record.id)`. `registerServer()` below is responsible for setting + * that exact `process.env` entry to the real key (resolved via the registry's + * CredentialStore, ultimately backed by auth.json) immediately before calling + * `pi.registerProvider` — Pi's own config-value resolver + * (`resolveConfigValue`) reads live `process.env`, with no separate lookup of + * auth.json BY PROVIDER ID for extension-registered providers. An earlier + * version of this comment assumed Pi did that auth.json lookup itself for any + * provider id, built-in or extension; that turned out to be true only for + * Pi's own BUILT-IN providers (driven by its internal `ModelRuntime`, see + * `auth-json-credential-store.ts` for the full writeup) — `pi.setModel()` + * silently returned `false` for an extension-registered model until this env + * var bridge was added, with no diagnostic beyond "Pi could not select + * ``" client-side. The real key itself is never embedded in + * `ProviderConfig` (only the `$ENV` reference is), so it still never reaches + * crossbar.json or Pi's in-memory provider config objects as plaintext. * - No-auth server: use a fixed, non-secret placeholder. This passes both Pi's * registration and request-time auth checks. */ @@ -127,6 +141,10 @@ export async function registerServer( if (credential.apiKey === undefined) { throw new Error(`API key missing for ${record.label}; add it again through /crossbar`); } + // Bridge the resolved secret into the exact process.env entry the `$ENV` sentinel in + // buildProviderConfig() references, so Pi's own config-value resolver can actually find + // it at request time (see the module header for why this is necessary, not optional). + process.env[envVarFor(record.id)] = credential.apiKey; } // Retrieve the DiscoveredServer shape from the record. diff --git a/src/ui/onboarding.ts b/src/ui/onboarding.ts index 027ba8b..ef20c60 100644 --- a/src/ui/onboarding.ts +++ b/src/ui/onboarding.ts @@ -5,6 +5,9 @@ * - Pure, unit-testable helpers: * buildDiscoveredItems — SelectItem[] from discovered servers + existing registry * buildModelItems — SelectItem[] from ModelDescriptor[] + * hasManualLoadModels — whether the picker needs the ○ manual-load footnote + * manualLoadDisabledValues — ○ model ids that the picker must not confirm + * manualLoadFootnote / manualLoadReason — the picker's ○ explanations * capabilityActions — capability-filtered action list * normalizeManualUrl — coerce bare host:port / missing-scheme inputs to a valid origin * @@ -23,8 +26,8 @@ import { DynamicBorder, getSelectListTheme } from "@earendil-works/pi-coding-age import { Container, type SelectItem, SelectList, Text, matchesKey, CancellableLoader } from "@earendil-works/pi-tui"; import type { BackendAdapter } from "../core/backend-adapter.ts"; -import { canIntrospect, canLoadUnload, canSwitch } from "../core/backend-adapter.ts"; -import type { CrossbarSettings, DiscoveredServer, HealthState, LoadedState, ModelDescriptor, ServerRecord } from "../core/types.ts"; +import { canAutoLoadStatus, canIntrospect, canLoadUnload, canSwitch } from "../core/backend-adapter.ts"; +import type { CrossbarSettings, DiscoveredServer, HealthState, LoadedState, ModelDescriptor, ServerCredential, ServerRecord } from "../core/types.ts"; import type { ServerRegistry } from "../registry/registry.ts"; import { serverId } from "../registry/ids.ts"; import { adapterFor } from "../adapters/index.ts"; @@ -277,8 +280,18 @@ export function buildSettingsItems(settings: CrossbarSettings): SelectItem[] { * The description line surfaces the context window (when known) and any * capability badges (vision, tools, reasoning, embeddings). A currently-loaded model * (where the backend reports it) is marked with a leading ● and a "loaded" badge. + * + * When `options.autoLoadsOnDemand === false` (the backend answered authoritatively + * that it will NOT auto-load unloaded models on request — e.g. Unsloth Studio with + * "Switch model by request" off), every not-loaded model is additionally marked with + * a leading ○ and a "no auto-load" badge: picking it would only fail on the first + * request, so the picker says so up front. `undefined` (unknown) leaves the list + * unmarked — same as before this option existed. */ -export function buildModelItems(models: ModelDescriptor[]): SelectItem[] { +export function buildModelItems( + models: ModelDescriptor[], + options?: { autoLoadsOnDemand?: boolean | undefined }, +): SelectItem[] { return models.map((m): SelectItem => { const parts: string[] = []; @@ -301,11 +314,15 @@ export function buildModelItems(models: ModelDescriptor[]): SelectItem[] { // (LM Studio's `state:"loaded"`). The ● matches the loaded-model widget; the // "loaded" badge leads the description so it reads even without colour. const name = m.name || m.id; + // ○ is the hollow counterpart of ●: not resident, and the server will not load + // it on demand either — it must be loaded in the backend's own UI first. + const needsManualLoad = options?.autoLoadsOnDemand === false && m.loaded !== true; const item: SelectItem = { value: m.id, - label: m.loaded ? `● ${name}` : name, + label: m.loaded ? `● ${name}` : needsManualLoad ? `○ ${name}` : name, }; if (m.loaded) parts.unshift("loaded"); + else if (needsManualLoad) parts.unshift("no auto-load"); if (parts.length > 0) { item.description = parts.join(" "); } @@ -313,6 +330,44 @@ export function buildModelItems(models: ModelDescriptor[]): SelectItem[] { }); } +/** + * True when at least one of these models is not currently loaded and the server + * will not auto-load it on demand — i.e. the picker shows ○ marks and must carry + * the explanatory footnote. `undefined` (unknown) is never true: we only warn when + * the backend told us explicitly. + */ +export function hasManualLoadModels( + models: ModelDescriptor[], + autoLoadsOnDemand: boolean | undefined, +): boolean { + return autoLoadsOnDemand === false && models.some((m) => m.loaded !== true); +} + +/** + * The ids of models that must NOT be confirmable in the picker: not currently + * loaded AND the server answered authoritatively that it will not auto-load them + * on demand. Selecting such a model would only fail on the first request, so the + * picker keeps it visible (marked ○) but swallows Enter on it and explains why. + * `undefined` (unknown) yields an empty set — never block a selection on a guess. + */ +export function manualLoadDisabledValues( + models: ModelDescriptor[], + autoLoadsOnDemand: boolean | undefined, +): Set { + if (autoLoadsOnDemand !== false) return new Set(); + return new Set(models.filter((m) => m.loaded !== true).map((m) => m.id)); +} + +/** Footnote line for pickers that show ○ marks. */ +export function manualLoadFootnote(adapter: BackendAdapter): string { + return `○ = not loaded — ${adapter.displayName} won't auto-load it; load it in ${adapter.displayName} first`; +} + +/** Per-model explanation shown when the user tries to select a ○ model. */ +export function manualLoadReason(adapter: BackendAdapter, modelId: string): string { + return `Crossbar: ${modelId} is not loaded and ${adapter.displayName} won't auto-load it — load it in ${adapter.displayName} first.`; +} + /** * Return only the actions that `adapter` actually supports — capability-driven * hiding in its simplest form. @@ -434,14 +489,28 @@ function serverFromRecord(record: ServerRecord): DiscoveredServer { /** * Render a single-select overlay (titled SelectList in an accent border) and resolve - * to the chosen item value, or `null` on Esc/cancel. Shared by the model picker and - * the manage menus so they stay visually consistent. +/** + * Items the user may navigate to but NOT confirm (Enter is swallowed and + * `reason` is shown as a notification). Used for models the server cannot serve + * until loaded elsewhere — visible for orientation, not selectable. */ -function selectOverlay( +interface DisabledItems { + values: ReadonlySet; + reason: (value: string) => string; +} + +/** + * Shared list overlay: renders a bordered SelectList, resolves to the chosen item + * value, or `null` on Esc/cancel. Shared by the model picker and the manage menus + * so they stay visually consistent. Exported for tests. + */ +export function selectOverlay( ctx: ExtensionCommandContext, title: string, items: SelectItem[], hint: string, + footnote?: string, + disabled?: DisabledItems, ): Promise { return ctx.ui.custom( (_tui, theme, _kb, done) => { @@ -454,6 +523,9 @@ function selectOverlay( list.onCancel = () => done(null); container.addChild(list); + if (footnote) { + container.addChild(new Text(theme.fg("dim", footnote))); + } container.addChild(new Text(theme.fg("dim", hint))); container.addChild(new DynamicBorder((s) => theme.fg("accent", s))); @@ -465,6 +537,16 @@ function selectOverlay( done(null); return; } + // Disabled items are visible but not confirmable: swallow Enter and + // explain why, instead of making a selection that would fail on the + // server's first request. + if (disabled && matchesKey(data, "return")) { + const sel = list.getSelectedItem(); + if (sel && disabled.values.has(sel.value)) { + ctx.ui.notify(disabled.reason(sel.value), "warning"); + return; + } + } list.handleInput(data); _tui.requestRender(); }, @@ -555,6 +637,26 @@ async function fetchModels( } } +/** + * Ask a backend whether unloaded models auto-load on demand — only when it can + * answer authoritatively (Capability.AutoLoadStatus, e.g. Unsloth Studio's + * "Switch model by request" setting). Never throws; `undefined` means unknown, + * in which case the picker stays unmarked and fully selectable. + */ +async function resolveAutoLoadsOnDemand( + server: DiscoveredServer, + cred: ServerCredential, +): Promise { + const adapter = adapterFor(server.kind); + if (!canAutoLoadStatus(adapter)) return undefined; + try { + const probe = createProbe(server.baseUrl, { auth: cred }); + return await adapter.autoLoadsOnDemand(server, cred, probe); + } catch { + return undefined; + } +} + /** * Pick one of the server's registered models and make it the model Pi uses — a * pure Pi-side selection (no server-side switch/load). Works for every backend, @@ -574,11 +676,32 @@ async function performUseModel( return; } + // Ask before showing the picker: if the server will NOT auto-load unloaded + // models, mark them (○) and make them non-confirmable — picking one would + // only fail on the first request. + const adapter = adapterFor(record.kind); + const cred = await registry.resolveCredential(record); + const autoLoadsOnDemand = await resolveAutoLoadsOnDemand(serverFromRecord(record), cred); + const disabledValues = manualLoadDisabledValues(selectableModels, autoLoadsOnDemand); + + // Nothing selectable at all — don't open a picker the user could only Esc out of. + if (disabledValues.size === selectableModels.length) { + ctx.ui.notify( + `Crossbar: no loaded models on ${record.label} — ${adapter.displayName} won't auto-load them. Load one in ${adapter.displayName} first.`, + "warning", + ); + return; + } + const modelId = await selectOverlay( ctx, `Use a model in Pi — ${record.label}`, - buildModelItems(selectableModels), + buildModelItems(selectableModels, { autoLoadsOnDemand }), "↑↓ navigate · Enter select · Esc cancel", + hasManualLoadModels(selectableModels, autoLoadsOnDemand) ? manualLoadFootnote(adapter) : undefined, + disabledValues.size > 0 + ? { values: disabledValues, reason: (id) => manualLoadReason(adapter, id) } + : undefined, ); if (!modelId) return; @@ -611,15 +734,21 @@ async function performModelAction( const title = action === "switch" ? `Switch model — ${record.label}` : `Load model — ${record.label}`; + const cred = await registry.resolveCredential(record); + const autoLoadsOnDemand = await resolveAutoLoadsOnDemand(serverFromRecord(record), cred); + const disabledValues = manualLoadDisabledValues(selectableModels, autoLoadsOnDemand); const modelId = await selectOverlay( ctx, title, - buildModelItems(selectableModels), + buildModelItems(selectableModels, { autoLoadsOnDemand }), "↑↓ navigate · Enter select · Esc cancel", + hasManualLoadModels(selectableModels, autoLoadsOnDemand) ? manualLoadFootnote(adapter) : undefined, + disabledValues.size > 0 + ? { values: disabledValues, reason: (id) => manualLoadReason(adapter, id) } + : undefined, ); if (!modelId) return; - const cred = await registry.resolveCredential(record); // Loads can be slow (cold model into VRAM) — give them a generous budget. const probe = createProbe(record.baseUrl, { auth: cred, defaultTimeoutMs: 60_000 }); @@ -1279,21 +1408,69 @@ export async function openOnboarding( continue; } - const authChoice = await ctx.ui.select( - "Authentication", - ["No authentication (open server)", "Enter API key"], - ); - if (authChoice === undefined) continue; + // Pre-probe unauthenticated, once, before asking the auth question. Some backends + // (e.g. Unsloth Studio) declare `authRequired: true` because they reject EVERY + // request — including their own metadata endpoints — without a key. For those, the + // adapter can still identify itself from public response shape/headers even on a + // bare 401 (see e.g. unsloth.ts). Detecting that up front lets Crossbar skip the + // "No authentication" choice entirely for a backend that can never work with it, + // instead of letting the user pick it and land on the generic "could not identify + // the server" dead end once every adapter's fingerprint 401s. + let preProbeAdapter: BackendAdapter | undefined; + let preProbeServer: DiscoveredServer | undefined; + try { + const bareProbe = createProbe(targetBaseUrl, { + auth: { mode: "none" }, + defaultTimeoutMs: 3000, + }); + const { DISCOVERY_ADAPTERS } = await import("../adapters/index.ts"); + for (const adapter of DISCOVERY_ADAPTERS) { + try { + const result = await adapter.fingerprint(targetBaseUrl, bareProbe); + if (result) { + preProbeAdapter = adapter; + preProbeServer = result; + break; + } + } catch { + // Try the next adapter. + } + } + } catch { + // Pre-probe is best-effort only — network trouble here just falls through to the + // normal auth question below, exactly like before this pre-probe existed. + } - selectedAuth = authChoice === "Enter API key" ? "apiKey" : "none"; - if (selectedAuth === "apiKey") { - const key = await ctx.ui.input("API key", "Paste your key (hidden after this dialog)"); + if (preProbeAdapter?.authRequired && preProbeServer) { + ctx.ui.notify( + `Crossbar: ${preProbeServer.label} requires an API key.`, + "info", + ); + selectedAuth = "apiKey"; + const key = await ctx.ui.input("API key", `Required by ${preProbeAdapter.displayName}`); if (key === undefined) continue; if (key.length === 0) { ctx.ui.notify("Crossbar: API key cannot be empty.", "warning"); continue; } manualApiKey = key; + } else { + const authChoice = await ctx.ui.select( + "Authentication", + ["No authentication (open server)", "Enter API key"], + ); + if (authChoice === undefined) continue; + + selectedAuth = authChoice === "Enter API key" ? "apiKey" : "none"; + if (selectedAuth === "apiKey") { + const key = await ctx.ui.input("API key", "Paste your key (hidden after this dialog)"); + if (key === undefined) continue; + if (key.length === 0) { + ctx.ui.notify("Crossbar: API key cannot be empty.", "warning"); + continue; + } + manualApiKey = key; + } } } else { const existingRecord = registry.list().find((r) => r.baseUrl === chosenBaseUrl); @@ -1409,11 +1586,20 @@ export async function openOnboarding( continue; } + // Same ○ treatment as the manage flow. No dead-end guard here: Esc still + // adds the server (just without a model pre-selected), so a fully disabled + // list is a valid — if unhelpful — outcome. + const autoLoadsOnDemand = await resolveAutoLoadsOnDemand(discoveredServer, cred); + const disabledValues = manualLoadDisabledValues(selectableModels, autoLoadsOnDemand); const chosenModelId = await selectOverlay( ctx, `Select model to use in Pi — ${discoveredServer.label}`, - buildModelItems(selectableModels), + buildModelItems(selectableModels, { autoLoadsOnDemand }), "↑↓ navigate · Enter select · Esc skip", + hasManualLoadModels(selectableModels, autoLoadsOnDemand) ? manualLoadFootnote(adapter) : undefined, + disabledValues.size > 0 + ? { values: disabledValues, reason: (id) => manualLoadReason(adapter, id) } + : undefined, ); const id = serverId(discoveredServer.kind, targetBaseUrl); diff --git a/tests/adapters/unsloth.fixture.ts b/tests/adapters/unsloth.fixture.ts new file mode 100644 index 0000000..93246b7 --- /dev/null +++ b/tests/adapters/unsloth.fixture.ts @@ -0,0 +1,173 @@ +/** + * Conformance fixture for the Unsloth Studio adapter. + * + * Routes below are captured verbatim from a live Unsloth Studio instance (2026-08-19, curl + * against `/v1/models` with and without a valid `Authorization` header) — see unsloth.ts for + * the full capture and rationale. + * + * Unsloth Studio characteristics exercised: + * - Fingerprint via the `Server: unsloth-studio` header, present on both the unauthenticated + * 401 and the authenticated 200 — the only signal actually unique to this product (see + * unsloth.test.ts for the discrimination edge cases the shared harness doesn't reach). + * - listModels via the same endpoint, authenticated; `owned_by`, `context_length` / + * `max_context_length` / `native_context_length`, and `loaded` are real response fields. + * - IntrospectLoaded, sourced from the same `loaded: boolean` field per model — no separate + * endpoint. + * - `authRequired: true`. + * - No SwitchModel / LoadUnload / Health (capability honesty) — not exposed by this backend. + */ + +import type { AdapterFixture } from "../conformance/fixtures.ts"; +import type { ProbeInit, ProbeResult } from "../../src/core/types.ts"; +import { unslothAdapter } from "../../src/adapters/unsloth.ts"; + +const LOADED_MODEL_ID = "unsloth/Qwen3.8-27B-GGUF"; +const UNLOADED_MODEL_ID = "Qwen3.6-35B-A3B-UD-Q4_K_XL"; +const EMBED_ID = "nomic-embed-text-v1.5"; + +/** Captured verbatim: `GET /v1/models` with no (or an invalid) Authorization header. */ +const UNAUTHENTICATED_RESPONSE: ProbeResult = { + status: 401, + ok: false, + headers: { + server: "unsloth-studio", + "www-authenticate": "Bearer", + "content-type": "application/json", + }, + json: { error: { message: "Not authenticated", type: "authentication_error", param: null, code: null } }, +}; + +/** Captured verbatim (trimmed to the fields this adapter reads) from the same live instance. */ +const AUTHENTICATED_RESPONSE: ProbeResult = { + status: 200, + ok: true, + headers: { + server: "unsloth-studio", + "content-type": "application/json", + }, + json: { + object: "list", + data: [ + { + id: LOADED_MODEL_ID, + object: "model", + owned_by: "unsloth-studio", + quant: "UD-Q4_K_XL", + context_length: 49152, + max_context_length: 229888, + native_context_length: 262144, + loaded: true, + }, + { + id: UNLOADED_MODEL_ID, + object: "model", + owned_by: "unsloth-studio", + loaded: false, + display_name: UNLOADED_MODEL_ID, + }, + { + id: EMBED_ID, + object: "model", + owned_by: "unsloth-studio", + loaded: false, + display_name: EMBED_ID, + }, + ], + }, +}; + +/** + * Real Unsloth Studio behaviour: `GET /v1/models` 401s without a bearer token and 200s with + * one, but sets the SAME `Server: unsloth-studio` header either way. `fingerprint()` calls the + * bare probe with no headers of its own (in production it never sees the raw credential — the + * orchestrator's bound `Probe` closure does that); `listModels()` attaches the `Authorization` + * header itself from `cred`. This one factory route reproduces both paths so the harness's + * fingerprint-positive test (no headers) and listModels happy-path test (cred's header + * attached) — which share this same `routes` map — each see the real response shape. + */ +const MODELS_ROUTE = (init?: ProbeInit): ProbeResult => + init?.headers?.["Authorization"] ? AUTHENTICATED_RESPONSE : UNAUTHENTICATED_RESPONSE; + +/** + * Captured verbatim from the live instance (2026-08-19): `GET /api/settings/openai-auto-switch` + * with a bearer token — the API surface of the "Switch model by request" toggle (Settings ▸ API). + * `enabled: false` is the state that makes requests for unloaded models 404 with + * `model_not_found`, which is exactly the case the adapter's `autoLoadsOnDemand` must surface. + */ +const AUTO_SWITCH_OFF_RESPONSE: ProbeResult = { + status: 200, + ok: true, + headers: { + server: "unsloth-studio", + "content-type": "application/json", + }, + json: { + enabled: false, + auto_unload_idle_seconds: 0, + default_enabled: false, + idle_unload_active: false, + auto_unload_keep_kv: true, + auto_download_model: false, + auto_unload_api_only: false, + media_auto_unload_idle_seconds: 0, + media_idle_unload_active: false, + }, +}; + +/** + * Another backend's response shape: no `Server: unsloth-studio` header at all. This is the + * only signal this adapter's fingerprint claims a kind from, so anything lacking it — even a + * plausible-looking 200 + `data[]` — must yield null. + */ +const NEGATIVE_ROUTES: Record = { + "/v1/models": { + status: 200, + ok: true, + headers: { server: "some-other-backend" }, + json: { object: "list", data: [{ id: "some-other-model", owned_by: "someone-else" }] }, + }, +}; + +/** Auth failure for the shared harness's listModels 401 test — no header needed here. */ +const AUTH_FAILURE_ROUTES: Record = { + "/v1/models": UNAUTHENTICATED_RESPONSE, +}; + +export const unslothFixture: AdapterFixture = { + name: "Unsloth Studio", + adapter: unslothAdapter, + cred: { mode: "apiKey", apiKey: "sk-unsloth-test-key" }, + + routes: { + "/v1/models": MODELS_ROUTE, + "/api/settings/openai-auto-switch": AUTO_SWITCH_OFF_RESPONSE, + }, + + negativeRoutes: NEGATIVE_ROUTES, + authFailureRoutes: AUTH_FAILURE_ROUTES, + + expect: { + fingerprint: { + kind: "unsloth", + confidenceMin: 0.9, + confidenceMax: 1.0, + }, + models: { + includedIds: [LOADED_MODEL_ID, UNLOADED_MODEL_ID], + excludedIds: [EMBED_ID], + minCount: 2, + }, + loadedState: { + anyOf: [LOADED_MODEL_ID], + source: "introspection", + }, + inferenceBaseUrlPrefix: "http://", + // Unsloth Studio reports no context/token limits for unloaded models, so the adapter + // deliberately emits maxTokens: 0 ("unbounded — let the server decide") rather than + // inventing one. Same contract as llama.cpp and llama-swap. + maxTokensMayBeUnbounded: true, + // The fixture's settings endpoint reports the toggle OFF — the case that matters for + // the picker warning (unloaded models 404 until loaded in the Studio UI). + autoLoadStatus: { expected: false }, + }, +}; diff --git a/tests/adapters/unsloth.test.ts b/tests/adapters/unsloth.test.ts new file mode 100644 index 0000000..9c30f29 --- /dev/null +++ b/tests/adapters/unsloth.test.ts @@ -0,0 +1,299 @@ +/** + * Conformance tests for the Unsloth Studio backend adapter. + * + * Delegates the standard contract checks to the shared conformance harness, then adds + * adapter-specific coverage for the discriminator that actually motivated this adapter: + * identifying Unsloth Studio from its `Server: unsloth-studio` response header even when no + * working API key is in hand yet, and declaring `authRequired` so onboarding can't offer a + * "No authentication" option for it. + */ + +import { describe, it, expect } from "vitest"; + +import { runConformance } from "../conformance/run-conformance.ts"; +import { createFakeProbe } from "../conformance/fake-probe.ts"; +import { unslothAdapter } from "../../src/adapters/unsloth.ts"; +import { unslothFixture } from "./unsloth.fixture.ts"; +import { Capability } from "../../src/core/capability.ts"; +import type { Probe } from "../../src/core/types.ts"; + +runConformance([unslothFixture]); + +describe("[unsloth] adapter-specific", () => { + it("declares authRequired — this backend has no unauthenticated mode", () => { + expect(unslothAdapter.authRequired).toBe(true); + }); + + it("identifies the server from the Server header alone, even on an unauthenticated 401", async () => { + const probe = createFakeProbe({ + "/v1/models": { + status: 401, + ok: false, + headers: { server: "unsloth-studio", "www-authenticate": "Bearer" }, + json: { error: { message: "Not authenticated", type: "authentication_error" } }, + }, + }); + const result = await unslothAdapter.fingerprint("http://127.0.0.1:8888", probe); + expect(result).not.toBeNull(); + expect(result?.kind).toBe("unsloth"); + expect(result?.auth).toBe("apiKey"); + expect(result?.confidence).toBeGreaterThan(0.9); + }); + + it("matches the Server header value case-insensitively", async () => { + const probe = createFakeProbe({ + "/v1/models": { + status: 401, + ok: false, + headers: { server: "Unsloth-Studio" }, + }, + }); + const result = await unslothAdapter.fingerprint("http://127.0.0.1:8888", probe); + expect(result?.kind).toBe("unsloth"); + }); + + it("does NOT claim a 401 from a different backend lacking the Server header", async () => { + const probe = createFakeProbe({ + "/v1/models": { + status: 401, + ok: false, + headers: { "content-type": "application/json" }, + json: { error: "Unauthorized" }, + }, + }); + const result = await unslothAdapter.fingerprint("http://127.0.0.1:8888", probe); + expect(result).toBeNull(); + }); + + it("does NOT claim a 200 + data[] response lacking the Server header, no matter how plausible", async () => { + const probe = createFakeProbe({ + "/v1/models": { + status: 200, + ok: true, + headers: { "content-type": "application/json" }, + json: { object: "list", data: [{ id: "totally-plausible-model", owned_by: "unsloth-studio" }] }, + }, + }); + const result = await unslothAdapter.fingerprint("http://127.0.0.1:8888", probe); + expect(result).toBeNull(); + }); + + it("returns null on connection refused (status 0)", async () => { + const probe = createFakeProbe({}); + const result = await unslothAdapter.fingerprint("http://127.0.0.1:8888", probe); + expect(result).toBeNull(); + }); + + it("introspectLoaded reports only models with loaded: true", async () => { + const probe = createFakeProbe({ + "/v1/models": { + status: 200, + ok: true, + headers: { server: "unsloth-studio" }, + json: { + data: [ + { id: "a", owned_by: "unsloth-studio", loaded: true }, + { id: "b", owned_by: "unsloth-studio", loaded: false }, + ], + }, + }, + }); + const server = { + kind: "unsloth" as const, + baseUrl: "http://127.0.0.1:8888", + auth: "apiKey" as const, + label: "Unsloth Studio", + confidence: 0.95, + }; + const result = await unslothAdapter.introspectLoaded?.(server, { mode: "apiKey", apiKey: "k" }, probe); + expect(result?.loadedModelIds).toEqual(["a"]); + expect(result?.source).toBe("introspection"); + }); + + // Regression: the adapter used to fabricate contextWindow 8192 / maxTokens 4096 for any + // model without context fields. Captured verbatim from a live instance: Unsloth Studio + // emits the *context_length fields ONLY for the currently-loaded model, so EVERY unloaded + // entry got a bogus "8k ctx" — shown in the picker and frozen into crossbar.json's + // lastKnownModels — even for models with a real 262144-token context. + describe("context window reporting", () => { + const server = { + kind: "unsloth" as const, + baseUrl: "http://127.0.0.1:8888", + auth: "apiKey" as const, + label: "Unsloth Studio", + confidence: 0.95, + }; + const cred = { mode: "apiKey" as const, apiKey: "k" }; + + const probeFor = (entries: unknown[]) => + createFakeProbe({ + "/v1/models": { + status: 200, + ok: true, + headers: { server: "unsloth-studio" }, + json: { data: entries }, + }, + }); + + it("omits contextWindow entirely when the server reports no context fields", async () => { + const models = await unslothAdapter.listModels( + server, + cred, + probeFor([{ id: "unloaded", owned_by: "unsloth-studio", loaded: false }]), + ); + expect(models).toHaveLength(1); + expect(models[0]).not.toHaveProperty("contextWindow"); + expect(models[0]).not.toHaveProperty("maxTokens"); + }); + + it("reports the real context of a loaded model verbatim", async () => { + const models = await unslothAdapter.listModels( + server, + cred, + probeFor([ + { + id: "Qwen3.6-35B-A3B-UD-Q4_K_M", + owned_by: "unsloth-studio", + loaded: true, + context_length: 262144, + max_context_length: 262144, + native_context_length: 262144, + }, + ]), + ); + expect(models[0]?.contextWindow).toBe(262144); + }); + + it("falls back to max/native context when the configured one is absent or zero", async () => { + const models = await unslothAdapter.listModels( + server, + cred, + probeFor([ + { id: "max-only", owned_by: "unsloth-studio", max_context_length: 229888 }, + { id: "native-only", owned_by: "unsloth-studio", context_length: 0, native_context_length: 262144 }, + ]), + ); + expect(models[0]?.contextWindow).toBe(229888); + expect(models[1]?.contextWindow).toBe(262144); + }); + + it("maps an unknown context to Pi's 128k fallback and unbounded maxTokens, never 8192", () => { + const entry = unslothAdapter.toPiModel(server, { + id: "unloaded", + name: "unloaded", + input: ["text"], + }); + expect(entry.contextWindow).toBe(128_000); + expect(entry.maxTokens).toBe(0); + }); + + it("passes a known context through to the Pi entry untouched", () => { + const entry = unslothAdapter.toPiModel(server, { + id: "loaded", + name: "loaded", + input: ["text"], + contextWindow: 262144, + }); + expect(entry.contextWindow).toBe(262144); + }); + }); + + it("inferenceBaseUrl appends /v1 exactly once", () => { + const server = { + kind: "unsloth" as const, + baseUrl: "http://127.0.0.1:8888", + auth: "apiKey" as const, + label: "Unsloth Studio", + confidence: 0.95, + }; + expect(unslothAdapter.inferenceBaseUrl(server)).toBe("http://127.0.0.1:8888/v1"); + expect(unslothAdapter.inferenceBaseUrl({ ...server, baseUrl: "http://127.0.0.1:8888/v1" })).toBe( + "http://127.0.0.1:8888/v1", + ); + }); + + describe("autoLoadsOnDemand (\"Switch model by request\")", () => { + const server = { + kind: "unsloth" as const, + baseUrl: "http://127.0.0.1:8888", + auth: "apiKey" as const, + label: "Unsloth Studio", + confidence: 0.95, + }; + const cred = { mode: "apiKey" as const, apiKey: "sk-unsloth-test-key" }; + + const settingsRoute = (enabled: unknown) => ({ + "/api/settings/openai-auto-switch": { + status: 200, + ok: true, + headers: { server: "unsloth-studio", "content-type": "application/json" }, + json: { enabled, auto_unload_idle_seconds: 0 }, + }, + }); + + it("declares the AutoLoadStatus capability", () => { + expect(unslothAdapter.capabilities.has(Capability.AutoLoadStatus)).toBe(true); + }); + + it("returns false when the toggle is off (unloaded models 404 until loaded in the UI)", async () => { + const probe = createFakeProbe(settingsRoute(false)); + const result = await unslothAdapter.autoLoadsOnDemand!(server, cred, probe); + expect(result).toBe(false); + }); + + it("returns true when the toggle is on (unloaded models are served on demand)", async () => { + const probe = createFakeProbe(settingsRoute(true)); + const result = await unslothAdapter.autoLoadsOnDemand!(server, cred, probe); + expect(result).toBe(true); + }); + + it("sends the bearer key with the settings request", async () => { + let seenHeaders: Record | undefined; + const probe: Probe = async (_path, init) => { + seenHeaders = init?.headers; + return { + status: 200, + ok: true, + headers: { server: "unsloth-studio" }, + json: { enabled: false }, + }; + }; + await unslothAdapter.autoLoadsOnDemand!(server, cred, probe); + expect(seenHeaders?.["Authorization"]).toBe("Bearer sk-unsloth-test-key"); + }); + + it("returns undefined (not a throw) when the endpoint is missing — old server versions", async () => { + const probe = createFakeProbe({}); // no fixture → status 0 + const result = await unslothAdapter.autoLoadsOnDemand!(server, cred, probe); + expect(result).toBeUndefined(); + }); + + it("returns undefined on 401 (invalid key)", async () => { + const probe = createFakeProbe({ + "/api/settings/openai-auto-switch": { + status: 401, + ok: false, + headers: { server: "unsloth-studio" }, + json: { error: { message: "Not authenticated", type: "authentication_error" } }, + }, + }); + const result = await unslothAdapter.autoLoadsOnDemand!(server, cred, probe); + expect(result).toBeUndefined(); + }); + + it("returns undefined when the body is malformed or `enabled` is not a boolean", async () => { + for (const body of [undefined, { enabled: "yes" }, { enabled: 0 }, { other: true }]) { + const probe = createFakeProbe({ + "/api/settings/openai-auto-switch": { + status: 200, + ok: true, + headers: { server: "unsloth-studio" }, + json: body, + }, + }); + const result = await unslothAdapter.autoLoadsOnDemand!(server, cred, probe); + expect(result).toBeUndefined(); + } + }); + }); +}); diff --git a/tests/conformance/fixtures.ts b/tests/conformance/fixtures.ts index b0a67ff..94c8441 100644 --- a/tests/conformance/fixtures.ts +++ b/tests/conformance/fixtures.ts @@ -196,5 +196,11 @@ export interface AdapterFixture { * Must start with "http://" or "https://". */ inferenceBaseUrlPrefix: string; + + /** + * What `autoLoadsOnDemand` must return on the success path. + * Only validated when `AutoLoadStatus` is in adapter.capabilities. + */ + autoLoadStatus?: { expected: boolean | undefined }; }; } diff --git a/tests/conformance/run-conformance.ts b/tests/conformance/run-conformance.ts index 92b81b4..5284a7f 100644 --- a/tests/conformance/run-conformance.ts +++ b/tests/conformance/run-conformance.ts @@ -272,6 +272,28 @@ function describeAdapter(fixture: AdapterFixture): void { }); }); } + + // 10. autoLoadsOnDemand (when capability present) + if (supports(adapter, Capability.AutoLoadStatus)) { + describe("autoLoadsOnDemand", () => { + it("returns the fixture's expected answer on the success path", async () => { + const result = await adapter.autoLoadsOnDemand!( + resolveServer(fixture), + cred, + createFakeProbe(routes as RouteMap), + ); + expect(result).toBe(exp.autoLoadStatus?.expected); + }); + it("returns undefined (not a throw) when the endpoint is unreachable", async () => { + const result = await adapter.autoLoadsOnDemand!( + resolveServer(fixture), + cred, + createFakeProbe({}), + ); + expect(result).toBeUndefined(); + }); + }); + } }); } diff --git a/tests/integration/cli-preload.test.ts b/tests/integration/cli-preload.test.ts index 7c73550..7922db5 100644 --- a/tests/integration/cli-preload.test.ts +++ b/tests/integration/cli-preload.test.ts @@ -179,4 +179,25 @@ describe("[integration] programmatic preload via CLI", () => { // The specific model should not be treated as available (no key) expect(output).not.toMatch(/test-model|TestKeyed/i); }); + + // End-to-end regression test for the real bug: a key was written to auth.json (via + // createAuthJsonCredentialStore), but `pi.setModel()` still failed because nothing ever + // bridged it into the process.env variable the registered ProviderConfig's `$ENV` sentinel + // references. This spawns the REAL local `pi` binary (no mocks, no fakes) against an + // isolated agent dir with a genuine (throwaway, never-real) auth.json entry, and asserts the + // cached model shows up as an AVAILABLE model — the exact opposite assertion from the + // "keyed-without-key" test above, using the same record shape plus one auth.json entry. + it("keyed-WITH-key (in auth.json) is registered AND available via --list-models", () => { + writeCrossbarJson(agentDir, [keyedRecord]); + writeAuthJson(agentDir, { + [keyedRecord.id]: { type: "api_key", key: "sk-test-dummy-throwaway-key" }, + }); + + const { status, output } = runPi(["--extension", EXT_PATH, "--list-models", "--no-session"]); + + expect(output).not.toMatch(/Error|exception|crash|API key missing/i); + // Unlike the no-key case, the model must now be listed as available. + expect(output).toMatch(/test-model|TestKeyed/i); + expect(status).toBe(0); + }); }); diff --git a/tests/preload.test.ts b/tests/preload.test.ts index e4f7a5f..be16ce3 100644 --- a/tests/preload.test.ts +++ b/tests/preload.test.ts @@ -38,6 +38,9 @@ beforeEach(() => { afterEach(() => { rmSync(dir, { recursive: true, force: true }); + // Defensive: the process.env bridge (for auth.json-backed keys, see below) must never leak + // into other test files sharing this worker process. + delete process.env[envVarFor(keyedRecord.id)]; }); // --------------------------------------------------------------------------- @@ -252,6 +255,30 @@ describe("preloadCachedProviders", () => { expect(config.apiKey).toBe("$CROSSBAR_OPENAI"); }); + it("bridges an already-persisted key from auth.json into process.env for the $ENV sentinel", async () => { + await writeCfg([keyedRecord]); + writeFileSync( + join(dir, "auth.json"), + JSON.stringify({ [keyedRecord.id]: { type: "api_key", key: "sk-preloaded-key" } }, null, 2), + ); + const { pi } = makeFakePi(); + + await preloadCachedProviders(pi, { dir }); + + expect(process.env[envVarFor(keyedRecord.id)]).toBe("sk-preloaded-key"); + }); + + it("registers with the unresolved $ENV sentinel (no throw) when auth.json has no entry yet", async () => { + await writeCfg([keyedRecord]); + delete process.env[envVarFor(keyedRecord.id)]; + const { pi, registerProvider } = makeFakePi(); + + await preloadCachedProviders(pi, { dir }); + + expect(registerProvider).toHaveBeenCalledOnce(); + expect(process.env[envVarFor(keyedRecord.id)]).toBeUndefined(); + }); + it("does not throw and registers zero when the config file is missing", async () => { // dir exists but crossbar.json was never written const { pi, registerProvider } = makeFakePi(); diff --git a/tests/registry/auth-json-credential-store.test.ts b/tests/registry/auth-json-credential-store.test.ts new file mode 100644 index 0000000..2731987 --- /dev/null +++ b/tests/registry/auth-json-credential-store.test.ts @@ -0,0 +1,135 @@ +/** + * Unit tests for the auth.json–backed CredentialStore. + * + * This module exists because pi-coding-agent ≥0.80.8 removed the extension-facing + * `AuthStorage` class (see CHANGELOG "AuthStorage and its storage backends are no longer + * exported") — `ctx.modelRegistry.authStorage` used to be the write path and is now + * `undefined`, which crashed Crossbar's original credential store with "Cannot read + * properties of undefined (reading 'set')". These tests lock in the replacement's contract: + * read/write auth.json directly, in Pi's own flat shape, without ever touching entries that + * belong to other providers. + */ + +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync, statSync, existsSync, readdirSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { createAuthJsonCredentialStore } from "../../src/registry/auth-json-credential-store.ts"; + +let dir: string; +let authPath: string; + +beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "crossbar-auth-test-")); + authPath = join(dir, "auth.json"); +}); + +afterEach(() => { + rmSync(dir, { recursive: true, force: true }); +}); + +describe("createAuthJsonCredentialStore", () => { + it("get() returns undefined when auth.json does not exist yet", async () => { + const store = createAuthJsonCredentialStore({ authPath }); + expect(await store.get("crossbar-unsloth-example")).toBeUndefined(); + }); + + it("round-trips a key through set() then get()", async () => { + const store = createAuthJsonCredentialStore({ authPath }); + await store.set("crossbar-unsloth-example", "sk-unsloth-abc123"); + expect(await store.get("crossbar-unsloth-example")).toBe("sk-unsloth-abc123"); + }); + + it("persists across store instances (survives a process restart)", async () => { + const first = createAuthJsonCredentialStore({ authPath }); + await first.set("crossbar-unsloth-example", "sk-unsloth-abc123"); + + const second = createAuthJsonCredentialStore({ authPath }); + expect(await second.get("crossbar-unsloth-example")).toBe("sk-unsloth-abc123"); + }); + + it("writes the exact flat shape Pi's own auth.json uses", async () => { + const store = createAuthJsonCredentialStore({ authPath }); + await store.set("crossbar-unsloth-example", "sk-unsloth-abc123"); + + const raw = JSON.parse(readFileSync(authPath, "utf-8")); + expect(raw).toEqual({ + "crossbar-unsloth-example": { type: "api_key", key: "sk-unsloth-abc123" }, + }); + }); + + it("never touches other providers' entries — oauth, api_key, or otherwise", async () => { + writeFileSync( + authPath, + JSON.stringify( + { + "github-copilot": { type: "oauth", refresh: "ghu_x", access: "y", expires: 123 }, + anthropic: { type: "api_key", key: "sk-ant-existing" }, + }, + null, + 2, + ), + ); + + const store = createAuthJsonCredentialStore({ authPath }); + await store.set("crossbar-unsloth-example", "sk-unsloth-abc123"); + + const raw = JSON.parse(readFileSync(authPath, "utf-8")); + expect(raw["github-copilot"]).toEqual({ type: "oauth", refresh: "ghu_x", access: "y", expires: 123 }); + expect(raw["anthropic"]).toEqual({ type: "api_key", key: "sk-ant-existing" }); + expect(raw["crossbar-unsloth-example"]).toEqual({ type: "api_key", key: "sk-unsloth-abc123" }); + }); + + it("remove() deletes only the targeted id", async () => { + const store = createAuthJsonCredentialStore({ authPath }); + await store.set("crossbar-unsloth-a", "key-a"); + await store.set("crossbar-unsloth-b", "key-b"); + + await store.remove("crossbar-unsloth-a"); + + expect(await store.get("crossbar-unsloth-a")).toBeUndefined(); + expect(await store.get("crossbar-unsloth-b")).toBe("key-b"); + }); + + it("remove() on an unknown id is a no-op, not a throw", async () => { + const store = createAuthJsonCredentialStore({ authPath }); + await store.remove("never-existed"); + // No throw is the assertion; also confirm the file wasn't created for nothing. + expect(existsSync(authPath)).toBe(false); + }); + + it("get() on a non-api_key entry (e.g. oauth) returns undefined rather than the wrong shape", async () => { + writeFileSync( + authPath, + JSON.stringify({ "github-copilot": { type: "oauth", refresh: "ghu_x" } }, null, 2), + ); + const store = createAuthJsonCredentialStore({ authPath }); + expect(await store.get("github-copilot")).toBeUndefined(); + }); + + it("treats a corrupt auth.json as empty rather than throwing, and recovers on the next write", async () => { + writeFileSync(authPath, "not valid json{{{"); + const store = createAuthJsonCredentialStore({ authPath }); + + expect(await store.get("crossbar-unsloth-example")).toBeUndefined(); + await store.set("crossbar-unsloth-example", "sk-unsloth-abc123"); + expect(await store.get("crossbar-unsloth-example")).toBe("sk-unsloth-abc123"); + }); + + it("writes auth.json with 0600 permissions", async () => { + const store = createAuthJsonCredentialStore({ authPath }); + await store.set("crossbar-unsloth-example", "sk-unsloth-abc123"); + + const mode = statSync(authPath).mode & 0o777; + expect(mode).toBe(0o600); + }); + + it("leaves no stray temp files behind after a write", async () => { + const store = createAuthJsonCredentialStore({ authPath }); + await store.set("crossbar-unsloth-example", "sk-unsloth-abc123"); + + const leftover = readdirSync(dir).filter((f) => f !== "auth.json"); + expect(leftover).toEqual([]); + expect(existsSync(authPath)).toBe(true); + }); +}); diff --git a/tests/registry/persistence.test.ts b/tests/registry/persistence.test.ts index 6b261f9..9ea5b21 100644 --- a/tests/registry/persistence.test.ts +++ b/tests/registry/persistence.test.ts @@ -36,25 +36,25 @@ const minimalRecord: ServerRecord = { describe("loadConfig", () => { it("returns empty config when file does not exist", async () => { const config = await loadConfig({ dir }); - expect(config).toEqual({ version: 1, modelCacheVersion: 1, servers: [] }); + expect(config).toEqual({ version: 1, modelCacheVersion: 2, servers: [] }); }); it("returns empty config when file is not valid JSON", async () => { writeFileSync(join(dir, "crossbar.json"), "not json"); const config = await loadConfig({ dir }); - expect(config).toEqual({ version: 1, modelCacheVersion: 1, servers: [] }); + expect(config).toEqual({ version: 1, modelCacheVersion: 2, servers: [] }); }); it("returns empty config when version is wrong", async () => { writeFileSync(join(dir, "crossbar.json"), JSON.stringify({ version: 2, servers: [] })); const config = await loadConfig({ dir }); - expect(config).toEqual({ version: 1, modelCacheVersion: 1, servers: [] }); + expect(config).toEqual({ version: 1, modelCacheVersion: 2, servers: [] }); }); it("round-trips a valid config", async () => { const original: CrossbarConfigFile = { version: 1, - modelCacheVersion: 1, + modelCacheVersion: 2, servers: [minimalRecord], }; await saveConfig(original, { dir }); @@ -64,7 +64,7 @@ describe("loadConfig", () => { it.each([ ["missing", undefined], - ["invalid", 2], + ["invalid", 1], ])("migrates legacy llama-swap fallbacks without stripping positive llama.cpp context with a %s marker", async (_label, marker) => { const settings = { lanDiscovery: true, probePorts: [8080, 8081] }; const legacy = { @@ -124,6 +124,31 @@ describe("loadConfig", () => { }, ], }, + { + ...minimalRecord, + id: "legacy-unsloth", + kind: "unsloth", + lastKnownModels: [ + { + // Unsloth reports context fields only for LOADED models; unloaded ones got the + // adapter's invented 8192/4096 frozen into the cache. + id: "unsloth-fabricated", + name: "Unsloth fabricated", + input: ["text"], + contextWindow: 8192, + maxTokens: 4096, + loaded: false, + }, + { + id: "unsloth-authoritative", + name: "Unsloth authoritative", + input: ["text"], + contextWindow: 262144, + maxTokens: 131072, + loaded: true, + }, + ], + }, { ...minimalRecord, id: "unaffected-ollama", @@ -145,7 +170,7 @@ describe("loadConfig", () => { expect(loaded).toEqual({ version: 1, - modelCacheVersion: 1, + modelCacheVersion: 2, settings, servers: [ { @@ -196,6 +221,27 @@ describe("loadConfig", () => { }, ], }, + { + ...minimalRecord, + id: "legacy-unsloth", + kind: "unsloth", + lastKnownModels: [ + { + id: "unsloth-fabricated", + name: "Unsloth fabricated", + input: ["text"], + loaded: false, + }, + { + id: "unsloth-authoritative", + name: "Unsloth authoritative", + input: ["text"], + contextWindow: 262144, + maxTokens: 131072, + loaded: true, + }, + ], + }, { ...minimalRecord, id: "unaffected-ollama", @@ -213,10 +259,10 @@ describe("loadConfig", () => { }); }); - it("trusts marker-1 model caches without re-migrating legitimate fallback-shaped values", async () => { + it("trusts current-marker model caches without re-migrating legitimate fallback-shaped values", async () => { const marked: CrossbarConfigFile = { version: 1, - modelCacheVersion: 1, + modelCacheVersion: 2, servers: [ { ...minimalRecord, @@ -264,7 +310,7 @@ describe("saveConfig", () => { const text = readFileSync(join(dir, "crossbar.json"), "utf-8"); // Pretty JSON has newlines expect(text).toContain("\n"); - expect(JSON.parse(text)).toMatchObject({ version: 1, modelCacheVersion: 1 }); + expect(JSON.parse(text)).toMatchObject({ version: 1, modelCacheVersion: 2 }); }); it("strips apiKey fields from server records", async () => { @@ -295,7 +341,7 @@ describe("saveConfig", () => { const nested = join(dir, "nested", "deep"); await saveConfig({ version: 1, servers: [] }, { dir: nested }); const loaded = await loadConfig({ dir: nested }); - expect(loaded).toEqual({ version: 1, modelCacheVersion: 1, servers: [] }); + expect(loaded).toEqual({ version: 1, modelCacheVersion: 2, servers: [] }); }); }); diff --git a/tests/shim/provider-shim.test.ts b/tests/shim/provider-shim.test.ts index f4a5fee..19604f7 100644 --- a/tests/shim/provider-shim.test.ts +++ b/tests/shim/provider-shim.test.ts @@ -12,7 +12,7 @@ * - No-auth providers use a resolved, non-secret placeholder key. */ -import { describe, it, expect, vi, beforeEach, type Mock } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach, type Mock } from "vitest"; import { buildProviderConfig, registerCachedServer, registerServer, unregisterServer, reRegisterServer } from "../../src/shim/provider-shim.ts"; import { ollamaAdapter } from "../../src/adapters/ollama.ts"; import { openaiAdapter } from "../../src/adapters/openai.ts"; @@ -324,6 +324,16 @@ function makeRegistry(resolvedKey?: string): ServerRegistry { } describe("registerServer", () => { + // registerServer() bridges the resolved key into process.env[envVarFor(record.id)] so Pi's + // own $ENV config-value resolution can find it (see shim/provider-shim.ts's header). That's + // a real, intentional side effect on the shared, process-global process.env — clean it up + // after every test in this file so it can't leak into the (deliberately isolated, real- + // ModelRegistry) "keyed availability" tests below, which reuse the same provider id to + // exercise the OPPOSITE case (no credential at all). + afterEach(() => { + delete process.env[envVarFor(openaiRecord.id)]; + }); + it("calls pi.registerProvider with the record id", async () => { const { pi, registerProvider } = makePi(); const registry = makeRegistry(); @@ -357,6 +367,20 @@ describe("registerServer", () => { expect(JSON.stringify(config)).not.toContain(plaintextKey); }); + it("bridges the resolved key into process.env for the $ENV sentinel to resolve", async () => { + const plaintextKey = "sk-realkey-for-env-bridge"; + const { pi, registry } = { ...makePi(), registry: makeRegistry(plaintextKey) }; + await registerServer(pi, registry, openaiRecord, [gpt4oModel]); + expect(process.env[envVarFor(openaiRecord.id)]).toBe(plaintextKey); + }); + + it("does not touch process.env for a no-auth server", async () => { + const { pi, registry } = { ...makePi(), registry: makeRegistry() }; + delete process.env[envVarFor(ollamaRecord.id)]; + await registerServer(pi, registry, ollamaRecord, [chatModel]); + expect(process.env[envVarFor(ollamaRecord.id)]).toBeUndefined(); + }); + it("rejects a keyed server when its stored credential is missing", async () => { const { pi, registerProvider } = makePi(); const registry = makeRegistry(); diff --git a/tests/ui/onboarding-flow.test.ts b/tests/ui/onboarding-flow.test.ts index 00b46a6..7836f3b 100644 --- a/tests/ui/onboarding-flow.test.ts +++ b/tests/ui/onboarding-flow.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const adapterMocks = vi.hoisted(() => ({ listModels: vi.fn(), @@ -41,6 +41,7 @@ import type { DiscoveredServer, ModelDescriptor, ServerRecord } from "../../src/ import { ServerRegistry } from "../../src/registry/registry.ts"; import { registerServer } from "../../src/shim/provider-shim.ts"; import { openOnboarding } from "../../src/ui/onboarding.ts"; +import { DISCOVERY_ADAPTERS } from "../../src/adapters/index.ts"; const model: ModelDescriptor = { id: "local-model", @@ -84,6 +85,26 @@ function makeRegistry(records: ServerRecord[] = []): ServerRegistry { return registry; } +/** Like {@link makeRegistry}, but the credential store actually round-trips set()/get() — needed + * for scenarios that add an apiKey-auth server and then rely on registerServer() resolving it. */ +function makeRegistryWithRealCredentialStore(records: ServerRecord[] = []): ServerRegistry { + const keys = new Map(); + const registry = new ServerRegistry({ + store: { + get: vi.fn(async (id: string) => keys.get(id)), + set: vi.fn(async (id: string, key: string) => { + keys.set(id, key); + }), + remove: vi.fn(async (id: string) => { + keys.delete(id); + }), + }, + persist: vi.fn(async () => undefined), + }); + registry.load({ version: 1, servers: records }); + return registry; +} + function makeHarness(customResults: unknown[]) { const registered = new Map(); const registerProvider = vi.fn((id: string, config: ProviderConfig) => { @@ -354,4 +375,76 @@ describe("openOnboarding navigation and registration", () => { // customs: server, settings, ports1, ports2, settings2, server2 expect(custom).toHaveBeenCalledTimes(6); }); + + describe("manual add — authRequired backends skip the auth question", () => { + const mockAdapter = DISCOVERY_ADAPTERS[0] as unknown as { + authRequired?: boolean; + fingerprint: ReturnType; + }; + const authRequiredServer: DiscoveredServer = { + kind: "llamacpp", + baseUrl: "http://mock-auth-required:8888", + auth: "apiKey", + label: "Mock Auth-Required Backend (mock-auth-required:8888)", + confidence: 0.9, + }; + + afterEach(() => { + delete mockAdapter.authRequired; + mockAdapter.fingerprint.mockReset(); + }); + + it("skips the auth question and goes straight to the API-key prompt", async () => { + mockAdapter.authRequired = true; + mockAdapter.fingerprint.mockResolvedValue(authRequiredServer); + + const registry = makeRegistryWithRealCredentialStore(); + const { pi, ctx, registerProvider } = makeHarness([ + "__manual__", // server selector → manual add + model.id, // model picker + null, // server selector closes + ]); + (ctx.ui.input as ReturnType) + .mockResolvedValueOnce("mock-auth-required:8888") // Server URL + .mockResolvedValueOnce("the-real-key"); // API key (asked directly, no auth menu) + + await openOnboarding(pi, ctx, { registry, discover: async () => [] }); + + // The auth choice menu (["No authentication", "Enter API key"]) must never appear — + // authRequired short-circuits straight to the key prompt. + expect(ctx.ui.select).not.toHaveBeenCalledWith( + "Authentication", + expect.arrayContaining(["No authentication (open server)"]), + ); + expect(ctx.ui.notify).toHaveBeenCalledWith( + expect.stringContaining("requires an API key"), + "info", + ); + expect(registerProvider).toHaveBeenCalledOnce(); + const record = registry.list()[0]!; + expect(record.auth).toBe("apiKey"); + }); + + it("still asks the normal auth question when no adapter declares authRequired", async () => { + delete mockAdapter.authRequired; + mockAdapter.fingerprint.mockResolvedValue(authRequiredServer); + + const registry = makeRegistry(); + const { pi, ctx, registerProvider } = makeHarness([ + "__manual__", + model.id, + null, + ]); + (ctx.ui.input as ReturnType).mockResolvedValueOnce("mock-auth-required:8888"); + (ctx.ui.select as ReturnType).mockResolvedValueOnce("No authentication (open server)"); + + await openOnboarding(pi, ctx, { registry, discover: async () => [] }); + + expect(ctx.ui.select).toHaveBeenCalledWith( + "Authentication", + ["No authentication (open server)", "Enter API key"], + ); + expect(registerProvider).toHaveBeenCalledOnce(); + }); + }); }); diff --git a/tests/ui/onboarding.test.ts b/tests/ui/onboarding.test.ts index 5a99daf..3b34f66 100644 --- a/tests/ui/onboarding.test.ts +++ b/tests/ui/onboarding.test.ts @@ -1,11 +1,10 @@ /** - * Unit tests for the pure helpers in src/ui/onboarding.ts. + * Unit tests for the helpers in src/ui/onboarding.ts. * - * NO overlay rendering attempted — these tests cover only the four pure helpers: - * - buildDiscoveredItems - * - buildModelItems - * - capabilityActions - * - normalizeManualUrl + * Mostly pure helpers (buildDiscoveredItems, buildModelItems, capabilityActions, + * normalizeManualUrl, …). The selectOverlay tests drive the REAL pi-tui + * SelectList headlessly (stub theme/tui, raw key sequences) to verify the + * disabled-item Enter interception. * * Capability-driven hiding is verified with REAL adapter singletons imported from * src/adapters/, so the test is a live check that the adapters honestly declare @@ -25,12 +24,19 @@ import { buildModelItems, buildSettingsItems, capabilityActions, + hasManualLoadModels, + manualLoadDisabledValues, + manualLoadFootnote, + manualLoadReason, normalizeManualUrl, parseHosts, parsePorts, probePortDescription, + selectOverlay, } from "../../src/ui/onboarding.ts"; +import type { ExtensionCommandContext } from "@earendil-works/pi-coding-agent"; + import type { DiscoveredServer, HealthState, ModelDescriptor, ServerRecord } from "../../src/core/types.ts"; // Real adapter singletons for capability-driven tests @@ -452,6 +458,224 @@ describe("buildModelItems", () => { it("handles an empty model list", () => { expect(buildModelItems([])).toHaveLength(0); }); + + describe("manual-load marking (autoLoadsOnDemand === false)", () => { + it("marks unloaded models with a ○ label prefix and a 'no auto-load' badge", () => { + const items = buildModelItems( + [makeModel({ id: "big-model", name: "Big Model", loaded: false })], + { autoLoadsOnDemand: false }, + ); + expect(items[0]!.label).toBe("○ Big Model"); + expect(items[0]!.description?.startsWith("no auto-load")).toBe(true); + }); + + it("does not mark loaded models even when auto-loading is off", () => { + const items = buildModelItems( + [makeModel({ id: "resident", name: "Resident", loaded: true })], + { autoLoadsOnDemand: false }, + ); + expect(items[0]!.label).toBe("● Resident"); + expect(items[0]!.description ?? "").not.toContain("no auto-load"); + }); + + it("does not mark anything when auto-loading is on", () => { + const items = buildModelItems( + [makeModel({ id: "a", name: "A", loaded: false })], + { autoLoadsOnDemand: true }, + ); + expect(items[0]!.label).toBe("A"); + expect(items[0]!.description ?? "").not.toContain("no auto-load"); + }); + + it("does not mark anything when the answer is unknown (undefined)", () => { + const items = buildModelItems( + [makeModel({ id: "a", name: "A", loaded: false })], + { autoLoadsOnDemand: undefined }, + ); + expect(items[0]!.label).toBe("A"); + expect(items[0]!.description ?? "").not.toContain("no auto-load"); + }); + + it("keeps the context window and caps after the badge", () => { + const items = buildModelItems( + [makeModel({ id: "a", name: "A", loaded: false, contextWindow: 32768, reasoning: true })], + { autoLoadsOnDemand: false }, + ); + expect(items[0]!.description).toBe("no auto-load 33k ctx reasoning"); + }); + }); +}); + +// ─── hasManualLoadModels ───────────────────────────────────────────────────── + +describe("hasManualLoadModels", () => { + it("is true only when the answer is explicitly false and some model is not loaded", () => { + const loaded = makeModel({ id: "l", loaded: true }); + const unloaded = makeModel({ id: "u", loaded: false }); + expect(hasManualLoadModels([unloaded], false)).toBe(true); + expect(hasManualLoadModels([loaded, unloaded], false)).toBe(true); + expect(hasManualLoadModels([loaded], false)).toBe(false); + expect(hasManualLoadModels([unloaded], true)).toBe(false); + expect(hasManualLoadModels([unloaded], undefined)).toBe(false); + expect(hasManualLoadModels([], false)).toBe(false); + }); +}); + +// ─── manualLoadDisabledValues / footnote / reason ──────────────────────────── + +describe("manualLoadDisabledValues", () => { + it("is empty unless the answer is explicitly false", () => { + const m = makeModel({ id: "u", loaded: false }); + expect(manualLoadDisabledValues([m], true).size).toBe(0); + expect(manualLoadDisabledValues([m], undefined).size).toBe(0); + expect(manualLoadDisabledValues([m], false).size).toBe(1); + }); + + it("disables exactly the models that are not reported loaded", () => { + const loaded = makeModel({ id: "l", loaded: true }); + const unloaded = makeModel({ id: "u", loaded: false }); + const unknown = makeModel({ id: "k" }); // no `loaded` field at all + expect([...manualLoadDisabledValues([loaded, unloaded, unknown], false)].sort()).toEqual([ + "k", + "u", + ]); + }); +}); + +describe("manualLoadFootnote / manualLoadReason", () => { + const fakeAdapter = { kind: "unsloth", displayName: "Unsloth Studio" } as unknown as Parameters< + typeof manualLoadFootnote + >[0]; + + it("footnote explains the ○ mark and names the backend", () => { + expect(manualLoadFootnote(fakeAdapter)).toBe( + "○ = not loaded — Unsloth Studio won't auto-load it; load it in Unsloth Studio first", + ); + }); + + it("reason names the model and the backend", () => { + expect(manualLoadReason(fakeAdapter, "big-model")).toBe( + "Crossbar: big-model is not loaded and Unsloth Studio won't auto-load it — load it in Unsloth Studio first.", + ); + }); +}); + +// ─── selectOverlay — disabled items ────────────────────────────────────────── +// +// Drives the REAL pi-tui SelectList headlessly: the ctx.ui.custom mock invokes +// the overlay factory with a stub theme/tui, then the test feeds raw key +// sequences into the returned component's handleInput. + +describe("selectOverlay — disabled items", () => { + const ENTER = "\r"; + const DOWN = "\x1b[B"; + const ESC = "\x1b"; + + function openOverlay( + items: Array<{ value: string; label: string }>, + disabled?: { values: ReadonlySet; reason: (value: string) => string }, + ) { + let component: { handleInput: (data: string) => void } | undefined; + let result: string | null | undefined; + const notifications: Array<{ text: string; level: string }> = []; + + const ctx = { + ui: { + custom: ( + factory: ( + tui: unknown, + theme: unknown, + kb: unknown, + done: (value: string | null) => void, + ) => { handleInput: (data: string) => void }, + ) => + new Promise((resolve) => { + const theme = { + fg: (_color: string, text?: string) => text ?? "", + bold: (t: string) => t, + }; + const tui = { requestRender: () => undefined }; + component = factory(tui, theme, {} as never, (value: string | null) => { + result = value; + resolve(value); + }); + }), + notify: (text: string, level: string) => { + notifications.push({ text, level }); + }, + }, + } as unknown as ExtensionCommandContext; + + void selectOverlay(ctx, "Test picker", items, "hint", undefined, disabled); + expect(component).toBeDefined(); + + return { + press: (data: string) => component!.handleInput(data), + result: () => result, + notifications, + }; + } + + const items = [ + { value: "loaded", label: "● loaded-model" }, + { value: "unloaded", label: "○ unloaded-model" }, + ]; + const disabled = { + values: new Set(["unloaded"]), + reason: (id: string) => `Crossbar: ${id} is not loadable.`, + }; + + it("swallows Enter on a disabled item and explains why", () => { + const h = openOverlay(items, disabled); + h.press(DOWN); // move onto the ○ item + h.press(ENTER); + expect(h.result()).toBeUndefined(); // overlay still open + expect(h.notifications).toEqual([ + { text: "Crossbar: unloaded is not loadable.", level: "warning" }, + ]); + }); + + it("keeps the overlay open after a blocked Enter so the user can recover", () => { + const h = openOverlay(items, disabled); + h.press(DOWN); + h.press(ENTER); // blocked + h.press(ENTER); // blocked again + expect(h.notifications).toHaveLength(2); + h.press(ESC); + expect(h.result()).toBeNull(); + }); + + it("confirms an enabled item normally", () => { + const h = openOverlay(items, disabled); + h.press(ENTER); // index 0 = loaded + expect(h.result()).toBe("loaded"); + expect(h.notifications).toHaveLength(0); + }); + + it("allows navigating onto a disabled item — it stays visible", () => { + const h = openOverlay(items, disabled); + h.press(DOWN); // now on the ○ item + h.press(ENTER); // blocked + h.press(DOWN); // wrap back to the ● item + h.press(ENTER); + expect(h.result()).toBe("loaded"); + }); + + it("without a disabled set, Enter confirms any item (back-compat)", () => { + const h = openOverlay(items); + h.press(DOWN); + h.press(ENTER); + expect(h.result()).toBe("unloaded"); + expect(h.notifications).toHaveLength(0); + }); + + it("Esc always cancels, even when sitting on a disabled item", () => { + const h = openOverlay(items, disabled); + h.press(DOWN); + h.press(ESC); + expect(h.result()).toBeNull(); + expect(h.notifications).toHaveLength(0); + }); }); // ─── capabilityActions ────────────────────────────────────────────────────────