feat: self-updating local-model backends (backend, steps 1–3)

[mriechers]

Self-updating local-model backends — backend (steps 1–3)

Makes local LLMs first-class, self-onboarding agent options. North star:

Pull a model onto local hardware → the server serves it → a "Refresh models" lists it → assign it to a phase. No code, no llm-config.json hand-edits, no redeploy.

Full design: planning/2026-07-10-self-updating-local-model-backends.md (in this PR).

This PR delivers the entire backend; the Settings UI (step 4) follows as a separate PR.

What's here

1. Route on the (backend, model) pair (1e395d2) — chat() sends a phase's assigned model to its backend; the backend's own model (honoring LOCAL_LLM_MODEL) is the fallback. Dropped the force_model single-model lock so any served model is selectable per phase. Verified live against oMLX at $0.
2. Roster multi-source (174dbb2) — get_available_models() merges each discover-flagged local backend's /v1/models unfiltered (a new model is never dropped), tagged by serving software (provider from owned_by → oMLX), host, $0, and context length. A down endpoint is non-fatal.
3. Backend-CRUD API (cba9107) — GET/POST/PATCH/DELETE /config/backends, host-keyed (self-identifying, no local-llm-2), forces type: openai, defaults discover+enabled and $0; guards duplicate host (409), invalid URL (400), and refuses to delete a backend still routing traffic (409).

Follow-up (separate PR)

4. Settings panel — add-endpoint form + Discover button; local models appear in the per-phase dropdown grouped Cloud vs oMLX · host.

Verification

• Full suite 861 passed, 5 xfailed.
• Step 1 confirmed live against oMLX (studio.riechers.co:8000): an assigned model id reaches the server at $0. Roster URL-derivation/tagging + the CRUD are covered by unit + ASGI-integration tests. (A fresh live discovery run was blocked only by a 1Password CLI session re-lock — auth, not code.)

Relationship to #291

Branched off #291's oMLX-backend commit (40bf6c1), excluding its deterministic-pipeline docs. This PR is the de-scoped "generic local backend" feature; #291 is repurposed for the separate deterministic-pipeline epic (which keeps those docs).

🤖 Generated with Claude Code

https://claude.ai/code/session_01FSw8KPi4k2MKgZ1AKjb5jR

[mriechers]

@claude please review

[claude]

Claude finished @mriechers's task in 3m 36s —— View job

---

Code Review — PR #293

• Understand the request (PR review)
• Read key changed files (config.py, llm.py, model_roster.py)
• Review backend CRUD API
• Review routing changes in llm.py
• Review multi-source roster in model_roster.py
• Review tests
• Review config/env changes
• Write up findings

---

Overall this is a well-scoped PR with a clear design goal. The host-keyed backend identity, the (backend, model) routing pair, and the non-fatal discovery design are all solid choices. I found one bug that will cause a 500 in production as soon as a local backend is active, one edge-case crash, and a few lower-severity items.

---

Bug — AvailableModel.tier: int rejects None (will 500 in production)

api/routers/config.py:135

tier: int = Field(..., ge=0, le=2, description="Cost tier (0=economy, 1=standard, 2=premium)")

Local roster entries (from fetch_local_models) always set tier: None. With Pydantic v2 in use (pydantic>=2.13.4), AvailableModel(**m) will raise a ValidationError when m["tier"] is None — it cannot be coerced to int. This means the GET /config/models and PATCH /config/models endpoints will return HTTP 500 for any user who has a local backend registered and discover: true.

The test suite passes because test_get_available_models_merges_cloud_and_local mocks at the roster layer and never exercises the Pydantic coercion path.

Fix: change the field type in AvailableModel to Optional[int]:

tier: Optional[int] = Field(None, ge=0, le=2, description="Cost tier (0=economy, 1=standard, 2=premium)")

[Fix this →](https://claude.ai/code?q=In%20api%2Frouters%2Fconfig.py%20on%20branch%20feat%2Flocal-model-discovery%2C%20change%20the%20AvailableModel%20class%20field%20%60tier%3A%20int%20%3D%20Field(...%2C%20ge%3D0%2C%20le%3D2%2C%20...%29%60%20to%20%60tier%3A%20Optional%5Bint%5D%20%3D%20Field(None%2C%20ge%3D0%2C%20le%3D2%2C%20...%29%60.%20Local%20roster%20entries%20from%20fetch_local_models%20set%20tier%3DNone%20and%20Pydantic%20v2%20rejects%20None%20for%20int%20fields.&repo=mriechers%2Fcardigan)

---

Bug (edge case) — None model_id + non-empty allowlist → AttributeError

api/services/llm.py:457

If a backend has no configured model, no phase assignment, and no fallback_model, model_id remains None after the resolution chain. It then reaches:

self.check_model_allowed(model_id)  # model_id is None

Inside check_model_allowed, if self.model_allowlist is non-empty:

if model == allowed or model.startswith(allowed + ":"):

...calls None.startswith(...) → AttributeError → unhandled 500.

In practice this only fires if both a non-empty model_allowlist AND a misconfigured backend (no model anywhere in the chain). But the safeguard should be defensive. Fix: add an early return or guard:

if not model:
    return  # no model resolved; let the backend call fail with a descriptive error

Similarly, it's worth adding an explicit check after the resolution chain (before the API call) to surface a clean ValueError instead of a cryptic backend 400.

Fix this →

---

Design concern — fallback/local roster not cached (latency under OpenRouter outage)

api/services/model_roster.py:239–246

The double-checked lock only caches a result when _classify_models returns non-empty cloud models. On every fallback path (no model_families configured, OpenRouter unreachable, or zero pattern matches), the function returns _static_fallback(config) + local uncached. This means fetch_local_models — which opens N HTTP connections to local servers — runs on every call to get_available_models() when OpenRouter is down. Under API load this could be significant.

The comment "uncached, so a later config change is picked up" applies only to the no-families path and is intentional there. But the "OpenRouter fetch failed" path (line 245) has no such reason to skip caching. Consider caching the static+local result for a shorter TTL (e.g. 60s) on failure paths.

---

Low — Double reload_config() in PATCH endpoints

api/routers/config.py:68 and :120–121 (also :220–222)

_save_config already calls llm.reload_config() at the end (line 67–68). Then update_phase_backends and update_phase_models each call it again explicitly after _save_config. The second call is redundant. No functional impact, but worth cleaning up.

---

Low — POC scripts committed to scripts/

scripts/eval_pipeline.py and scripts/poc_house_style_normalizer.py are added to the main scripts/ directory. The poc_ prefix signals these are experiments. If they're not intended to be part of the production scripts directory, consider moving them to planning/ or another scratch location (or adding to .gitignore) to keep the scripts directory reserved for operational tooling.

---

What looks good

• Host-keyed backend identity is the right call — avoids invented names and makes the key self-describing. The 409 duplicate guard is correct.
• Non-fatal discovery (continue on error, logged) — a down local server never blocks the cloud roster. Good.
• discover-flag gate in fetch_local_models is clean: enabled + discover + openai-type only.
• force_model removal is clean — no dangling references found.
• strip_reasoning / Qwen3 <think> handling with both well-formed and dangling-close cases covered by regex.
• _resolve_endpoint and _resolve_model env-var override pattern is consistent with the existing deploy model.
• Delete guard (refuses to remove a backend in use by primary_backend, fallback_backend, or phase_backends) is the right safety rail.
• Test coverage for the CRUD happy paths and error paths is solid.

---

[claude]

Code review

Two confirmed issues found: one security vulnerability and one logic bug.

---

Issue 1 — SSRF + server-secret exfiltration (api/routers/config.py)

The only URL validation in create_backend is urlparse(body.endpoint).netloc being non-empty — no scheme restriction, no private-IP blocklist. api_key_env accepts any string with no allowlist. Chained exploit path:

1. An attacker registers a backend with endpoint: 'http://attacker.example/v1' and api_key_env: 'OPENROUTER_API_KEY'.
2. The next GET /config/models triggers fetch_local_models() (

   cardigan/api/services/model_roster.py

   Lines 174 to 184 in cba9107

   	if not (cfg.get("enabled") and cfg.get("discover") and cfg.get("type") == "openai"):
   	continue
   	endpoint = _resolve_backend_endpoint(cfg)
   	url = _models_url(endpoint)
   	host = urlparse(endpoint).netloc or endpoint
   	api_key = get_secret(cfg["api_key_env"]) if cfg.get("api_key_env") else None
   	headers = {"Authorization": f"Bearer {api_key}"} if api_key else {}
   	try:
   	async with httpx.AsyncClient(timeout=15.0) as client:
   	resp = await client.get(url, headers=headers)
   	resp.raise_for_status()

   ), which resolves the named secret via get_secret() and sends it as Authorization: Bearer to the attacker URL.
3. get_secret() (

   cardigan/api/services/secrets.py

   Lines 47 to 52 in cba9107

   	"""
   	# 1. Docker secret file
   	secret_file = DOCKER_SECRETS_DIR / key.lower()
   	if secret_file.is_file():
   	try:
   	return secret_file.read_text().strip()

   ) does DOCKER_SECRETS_DIR / key.lower() with no path sanitization — a name like ../../etc/passwd reads arbitrary files.
4. update_backend (

   cardigan/api/routers/config.py

   Lines 454 to 460 in cba9107

   	entry = backends[name]
   	for field in ("endpoint", "api_key_env", "model", "enabled", "discover"):
   	value = getattr(body, field)
   	if value is not None:
   	entry[field] = value

   ) has zero endpoint validation.

The homelab deployment is unauthenticated (tailnet-only), so no API key is needed.

Suggested fixes: restrict endpoint scheme to http/https and block private/link-local ranges (RFC 1918, 169.254.0.0/16, ::1) in both create_backend and update_backend; restrict api_key_env to an explicit allowlist and reject values containing / or ...

---

Issue 2 — _resolve_model empty-string env fallback bug (api/services/llm.py L342)

cardigan/api/services/llm.py

Lines 340 to 344 in cba9107

	env_var = config.get("model_env")
	if env_var:
	return os.getenv(env_var, config.get("model"))
	return config.get("model")

os.getenv(env_var, config.get('model')) returns '' (not the default) when the env var is set but empty. docker-compose.prod.yml sets LOCAL_LLM_MODEL=${LOCAL_LLM_MODEL:-}, injecting an empty string when the host env var is unset. _resolve_model() returns '', which or backend_config.get('fallback_model') discards (falsy) — since local-llm has no fallback_model, model_id = None is sent to the backend in the default deploy state.

Fix: change line 342 from os.getenv(env_var, config.get('model')) to os.getenv(env_var) or config.get('model').

[mriechers]

@claude please review

Merged origin/main in to resolve the conflict from #295's merge. Conflicts were only in house-style/eval artifacts inherited from the shared oMLX exploration base — resolved by taking main's authoritative versions (prompts/seo.md tokenized copy rules + 80/90/350 limits; scripts/eval_pipeline.py and scripts/poc_house_style_normalizer.py main versions). #293's core (backend CRUD, roster discovery, (backend,model) routing, secrets) was conflict-free. Full suite 1632 passing, ruff + black clean.

[claude]

Claude finished @mriechers's task in 3m 40s —— View job

---

Code Review — PR #293 (post-main-merge)

• Understand the request
• Read key changed files after merge
• Check if previous review issues were addressed
• Review conflict resolution (eval scripts, prompts)
• Write up findings

---

Merge resolution — clean

The merge is clean. Conflicts were only in the eval/house-style artifacts (scripts/eval_pipeline.py, scripts/poc_house_style_normalizer.py, prompts/seo.md) inherited from the shared oMLX exploration base; taking main's authoritative versions was the right call. The prior concern about POC scripts living in scripts/ no longer applies — they're in main now, not unique to this PR.

The core PR changes (backend CRUD, roster discovery, (backend, model) routing, secrets bootstrap) were conflict-free throughout.

---

What got fixed ✅

AvailableModel.tier: Optional[int] — confirmed fixed. config.py:136 now correctly uses Optional[int] = Field(None, ...). The GET /config/models 500-on-local-backend bug is resolved.

---

Remaining issues from prior reviews

Bug — _resolve_model returns "" in the default Docker deploy

api/services/llm.py:342

docker-compose.prod.yml:27/62 injects LOCAL_LLM_MODEL=${LOCAL_LLM_MODEL:-} — an empty string when the host env var is unset. os.getenv(env_var, config.get("model")) returns "" (not the default) when the var exists but is empty. The call chain then produces model_id = None:

_resolve_model → "" → "" or fallback_model → None

None is forwarded to _call_openai as payload["model"] = None, which gets a backend 400 instead of a readable local error. Still unfixed. One-line fix:

# line 342
return os.getenv(env_var) or config.get("model")

Fix this →

Bug (edge case) — check_model_allowed(None) AttributeError

api/services/llm.py:457

If model_id resolves to None (e.g. via the empty-string bug above or a fully unconfigured backend) and LLM_MODEL_ALLOWLIST is set, model.startswith(allowed + ":") raises AttributeError on None. The default deploy has an empty allowlist so this is latent, but it will surface if allowlist enforcement is ever turned on. Still unfixed. One-line guard:

def check_model_allowed(self, model: str) -> None:
    if not self.enforce_guards:
        return
    if not self.model_allowlist:
        return
+   if not model:
+       return  # no resolved model; the backend call will fail with a clearer error
    for allowed in self.model_allowlist:

Fix this →

Security — update_backend still has zero endpoint validation

api/routers/config.py:480-496

create_backend at least checks urlparse(body.endpoint).netloc is non-empty (though it still lacks scheme/RFC-1918 restrictions). update_backend has no validation at all on the incoming endpoint value — it writes it straight to config. An attacker with access to the tailnet could register a legitimate-looking backend then patch it to point at http://169.254.169.254/ or any internal host. The api_key_env path-traversal issue in get_secret (DOCKER_SECRETS_DIR / key.lower() has no sanitization against ../../ components) noted in the earlier security review also remains.

Given the tailnet-only threat model these are lower risk than they would be on a public service, but update_backend validation is a one-line fix to keep parity with create_backend:

# api/routers/config.py, update_backend()
if body.endpoint is not None:
    if not urlparse(body.endpoint).netloc:
        raise HTTPException(status_code=400, detail="endpoint must be an absolute URL")
    entry["endpoint"] = body.endpoint

Fix this →

Low — Double reload_config() in PATCH handlers

_save_config already calls llm.reload_config() (line 68). update_phase_backends (line 120) and update_phase_models (line 255) each call it again after _save_config returns. No functional impact, but both explicit calls are dead.

---

What looks good

• Merge resolution is correct — no behavioral regressions in the core PR from the conflict resolution.
• AvailableModel schema fix is confirmed in place.
• Settings.tsx handleAddEndpoint/handleToggleEndpoint correctly use encodeURIComponent(name) on the backend name path param.
• useMemo for groupAvailableModels is the right call (one computation per roster change, not per-agent-row render).
• refreshRosterAndLists combining POST /config/models/refresh + parallel fetchBackends + fetchConfig is the right sequencing.
• The core design choices from the prior review — host-keyed identity, non-fatal discovery, (backend, model) routing pair — remain solid.

---

Summary: Merge is clean. The tier bug is fixed. The two medium-severity bugs (_resolve_model empty string, check_model_allowed(None)) and the update_backend endpoint validation gap are still outstanding. All three are small fixes.