From 27971e18059e710ec24ca2cf56998a651a9670e4 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 16 Sep 2026 07:31:42 +0000 Subject: [PATCH 1/2] Fix Windows local-model register 500 INTERNAL_ERROR Ollama create used an unquoted Windows FROM path and decoded CLI output with the locale codec, while list_models assumed a dict. Those failures escaped as opaque INTERNAL_ERROR. Quote POSIX FROM lines, decode UTF-8, accept SDK list payloads, register already-pulled tags, and return LOCAL_MODEL_REGISTER_FAILED with the real reason. Co-authored-by: XYAI Labs --- dashboard/src/locales/en.json | 3 +- dashboard/src/locales/zh.json | 3 +- .../Models/components/LocalHardwarePanel.tsx | 11 +- src/octop/api/routers/local_models.py | 70 ++++++-- src/octop/i18n/en.json | 3 +- src/octop/i18n/zh.json | 3 +- .../infra/agents/providers/local_probe.py | 1 + .../infra/agents/providers/local_register.py | 128 ++++++++++++--- src/octop/infra/errors.py | 2 + src/octop/infra/utils/ollama_manager.py | 97 +++++++++-- tests/unit/api/test_local_models_register.py | 38 +++++ tests/unit/test_local_register.py | 152 +++++++++++++++++- tests/unit/test_ollama_create.py | 132 +++++++++++++++ 13 files changed, 582 insertions(+), 61 deletions(-) create mode 100644 tests/unit/api/test_local_models_register.py create mode 100644 tests/unit/test_ollama_create.py diff --git a/dashboard/src/locales/en.json b/dashboard/src/locales/en.json index f00da4e8..041bf92c 100644 --- a/dashboard/src/locales/en.json +++ b/dashboard/src/locales/en.json @@ -307,7 +307,8 @@ "INVITE_REVOKED": "This invite code has been revoked.", "INVITE_RATE_LIMITED": "Too many invite attempts. Try again later.", "WORKSPACE_ROOT_RESTRICTED": "The selected storage root is outside the allowed directory for this account.", - "TOKEN_QUOTA_EXCEEDED": "This account has reached its token quota. Please contact an administrator to raise the limit ({{used}}/{{quota}})." + "TOKEN_QUOTA_EXCEEDED": "This account has reached its token quota. Please contact an administrator to raise the limit ({{used}}/{{quota}}).", + "LOCAL_MODEL_REGISTER_FAILED": "Could not register the local model: {{reason}}" }, "nav": { "chat": "Conversation", diff --git a/dashboard/src/locales/zh.json b/dashboard/src/locales/zh.json index c8d589fc..1a4eeb90 100644 --- a/dashboard/src/locales/zh.json +++ b/dashboard/src/locales/zh.json @@ -307,7 +307,8 @@ "INVITE_REVOKED": "该邀请码已被撤销。", "INVITE_RATE_LIMITED": "邀请尝试次数过多，请稍后再试。", "WORKSPACE_ROOT_RESTRICTED": "所选存储根目录超出了该账号允许的范围。", - "TOKEN_QUOTA_EXCEEDED": "该账号已达到 token 配额上限，请联系管理员提升额度（{{used}}/{{quota}}）。" + "TOKEN_QUOTA_EXCEEDED": "该账号已达到 token 配额上限，请联系管理员提升额度（{{used}}/{{quota}}）。", + "LOCAL_MODEL_REGISTER_FAILED": "无法注册本地模型：{{reason}}" }, "nav": { "chat": "对话", diff --git a/dashboard/src/pages/Settings/Models/components/LocalHardwarePanel.tsx b/dashboard/src/pages/Settings/Models/components/LocalHardwarePanel.tsx index d7f02382..a430d65a 100644 --- a/dashboard/src/pages/Settings/Models/components/LocalHardwarePanel.tsx +++ b/dashboard/src/pages/Settings/Models/components/LocalHardwarePanel.tsx @@ -28,6 +28,7 @@ import { } from "../../../../utils/desktopFolder"; import { providerApi } from "../../../../api/modules/provider"; import { message } from "../../../../utils/antdMessage"; +import { apiErrorMessage } from "../../../../utils/apiError"; import { CONVERSATION_LIST_PATH } from "../../../../layouts/conversationHome"; import { isOllamaProviderRow } from "../presetUtils"; @@ -69,9 +70,11 @@ function mergeModels( } function canRegister(item: LocalInstalledModel): boolean { - if (item.source === "ollama" || item.registered) return false; + if (item.registered) return false; if (item.registerable === false) return false; - return item.source === "gguf" || item.source === "ggml"; + return ( + item.source === "gguf" || item.source === "ggml" || item.source === "ollama" + ); } export function LocalHardwarePanel() { @@ -338,9 +341,7 @@ export function LocalHardwarePanel() { message.success(t("models.localRegisterDone", { name: result.name })); await refresh(); } catch (err) { - message.error( - err instanceof Error ? err.message : t("models.localRegisterFailed"), - ); + message.error(apiErrorMessage(err, t("models.localRegisterFailed"), t)); } finally { setRegistering(null); } diff --git a/src/octop/api/routers/local_models.py b/src/octop/api/routers/local_models.py index d37565f6..761d9092 100644 --- a/src/octop/api/routers/local_models.py +++ b/src/octop/api/routers/local_models.py @@ -4,9 +4,11 @@ import asyncio import json +import logging +import subprocess from typing import Any -from fastapi import APIRouter, Depends, HTTPException +from fastapi import APIRouter, Depends from pydantic import BaseModel, Field from octop.api.deps import current_user, get_server, require_permission @@ -24,6 +26,20 @@ from octop.infra.server import OctopServer from octop.infra.utils.ollama_manager import OllamaModelManager, start_ollama_service_result +logger = logging.getLogger(__name__) + +_REGISTER_FAIL_TYPES = ( + OSError, + UnicodeError, + TimeoutError, + ValueError, + RuntimeError, + TypeError, + AttributeError, + ImportError, + subprocess.SubprocessError, +) + router = APIRouter() @@ -41,10 +57,16 @@ class LocalScanBody(BaseModel): class LocalRegisterBody(BaseModel): path: str = Field( - min_length=1, max_length=1024, description="Absolute path to a GGUF/GGML file" + default="", + max_length=1024, + description="Absolute path to a GGUF/GGML file; empty for an already-pulled Ollama tag", ) name: str | None = Field(default=None, max_length=80, description="Ollama model name to create") - source: str = Field(default="gguf", max_length=32, description="gguf, ggml, or safetensors") + source: str = Field( + default="gguf", + max_length=32, + description="gguf, ggml, safetensors, or ollama", + ) size: int = Field(default=0, ge=0, description="Size in bytes from the scan result") @@ -55,9 +77,27 @@ class LocalEnsureBody(BaseModel): ) +def register_failure(exc: BaseException) -> OctopError: + """Map a recoverable register/import failure to a 400 the UI can show.""" + reason = str(exc).strip() or exc.__class__.__name__ + return OctopError( + ErrorCode.LOCAL_MODEL_REGISTER_FAILED, + reason, + details={"reason": reason}, + ) + + def _merge_registered(probe: dict[str, Any], rows: list[dict[str, Any]]) -> dict[str, Any]: installed = list(probe.get("installed") or []) + registered_names = {str(row.get("name") or "") for row in rows if row.get("name")} + registered_paths = {str(row.get("path") or "") for row in rows if row.get("path")} seen = {str(item.get("path") or item.get("name") or "") for item in installed} + for item in installed: + name = str(item.get("name") or "") + path = str(item.get("path") or "") + if name in registered_names or (path and path in registered_paths): + item["registered"] = True + item["registerable"] = False for row in rows: key = str(row.get("path") or row.get("name") or "") if key and key in seen: @@ -146,8 +186,11 @@ async def local_models_install( try: info = await asyncio.to_thread(OllamaModelManager.pull_model, body.name) except Exception as exc: - raise HTTPException(status_code=400, detail=str(exc)) from exc - registered = _register_ollama_model(server, info.name or body.name) + raise register_failure(exc) from exc + try: + registered = _register_ollama_model(server, info.name or body.name) + except Exception as exc: + raise register_failure(exc) from exc return { "ok": True, "name": info.name, @@ -214,10 +257,17 @@ async def local_models_register( provider_repo=server.services.provider_repo, settings_repo=server.services.settings_repo, ) - except OSError as exc: - raise HTTPException(status_code=400, detail=str(exc)) from exc + except _REGISTER_FAIL_TYPES as exc: + raise register_failure(exc) from exc if server.app_runtime is not None and result.get("ok"): - await server.app_runtime.agent_registry.on_provider_changed( - provider_name=str(result.get("provider_name") or "Ollama (Local)") - ) + try: + await server.app_runtime.agent_registry.on_provider_changed( + provider_name=str(result.get("provider_name") or "Ollama (Local)") + ) + except Exception as exc: + logger.warning( + "Registered %s but provider reload failed: %s", + result.get("name"), + exc, + ) return result diff --git a/src/octop/i18n/en.json b/src/octop/i18n/en.json index e0fd2274..0212ef91 100644 --- a/src/octop/i18n/en.json +++ b/src/octop/i18n/en.json @@ -400,7 +400,8 @@ "INVITE_REVOKED": "This invite code has been revoked.", "INVITE_RATE_LIMITED": "Too many invite attempts. Try again later.", "WORKSPACE_ROOT_RESTRICTED": "The selected storage root is outside the allowed directory for this account.", - "TOKEN_QUOTA_EXCEEDED": "This account has reached its token quota. Please contact an administrator to raise the limit ({used}/{quota})." + "TOKEN_QUOTA_EXCEEDED": "This account has reached its token quota. Please contact an administrator to raise the limit ({used}/{quota}).", + "LOCAL_MODEL_REGISTER_FAILED": "Could not register the local model: {reason}" }, "desktop": { "install_log_deps": "Installing Python dependencies…", diff --git a/src/octop/i18n/zh.json b/src/octop/i18n/zh.json index 272da1ed..49304f8f 100644 --- a/src/octop/i18n/zh.json +++ b/src/octop/i18n/zh.json @@ -400,7 +400,8 @@ "INVITE_REVOKED": "该邀请码已被撤销。", "INVITE_RATE_LIMITED": "邀请尝试次数过多，请稍后再试。", "WORKSPACE_ROOT_RESTRICTED": "所选存储根目录超出了该账号允许的范围。", - "TOKEN_QUOTA_EXCEEDED": "该账号已达到 token 配额上限，请联系管理员提升额度（{used}/{quota}）。" + "TOKEN_QUOTA_EXCEEDED": "该账号已达到 token 配额上限，请联系管理员提升额度（{used}/{quota}）。", + "LOCAL_MODEL_REGISTER_FAILED": "无法注册本地模型：{reason}" }, "desktop": { "install_log_deps": "正在安装 Python 依赖…", diff --git a/src/octop/infra/agents/providers/local_probe.py b/src/octop/infra/agents/providers/local_probe.py index 6bcc512d..175d9037 100644 --- a/src/octop/infra/agents/providers/local_probe.py +++ b/src/octop/infra/agents/providers/local_probe.py @@ -85,6 +85,7 @@ def _ollama_models() -> tuple[bool, list[dict[str, Any]]]: "path": "", "size": int(getattr(item, "size", 0) or 0), "source": "ollama", + "registerable": True, } ) return True, models diff --git a/src/octop/infra/agents/providers/local_register.py b/src/octop/infra/agents/providers/local_register.py index 7f10d089..6cb6c9c3 100644 --- a/src/octop/infra/agents/providers/local_register.py +++ b/src/octop/infra/agents/providers/local_register.py @@ -3,14 +3,22 @@ from __future__ import annotations import json +import logging import re +from pathlib import Path from typing import Any from octop.infra.agents.providers.model_flags import is_ollama_local_provider from octop.infra.utils.ollama_manager import OllamaModelManager, create_from_weight +logger = logging.getLogger(__name__) + _SETTINGS_KEY = "local_registered_weights" -_NAME_SAFE = re.compile(r"[^a-z0-9._-]+") +# Keep Ollama tags (``llama3.2:1b``) while sanitizing Windows paths / file stems. +_NAME_SAFE = re.compile(r"[^a-z0-9._:-]+") +_OLLAMA_BASE_URL = "http://127.0.0.1:11434/v1" +_WEIGHT_SOURCES = frozenset({"gguf", "ggml"}) +_REGISTER_SOURCES = frozenset({*_WEIGHT_SOURCES, "ollama"}) def sanitize_model_name(raw: str) -> str: @@ -39,10 +47,14 @@ def save_registered(settings_repo: Any, rows: list[dict[str, Any]]) -> None: settings_repo.set(_SETTINGS_KEY, json.dumps(rows, ensure_ascii=False)) +def _entry_key(entry: dict[str, Any]) -> str: + return str(entry.get("path") or entry.get("name") or "") + + def remember_weight(settings_repo: Any, entry: dict[str, Any]) -> None: rows = load_registered(settings_repo) - path = str(entry.get("path") or "") - kept = [item for item in rows if str(item.get("path") or "") != path] + key = _entry_key(entry) + kept = [item for item in rows if _entry_key(item) != key] kept.append(entry) save_registered(settings_repo, kept) @@ -67,14 +79,20 @@ def _upsert_ollama_model(provider_repo: Any, model_id: str, display: str) -> str "input": ["text"], } if row is None: - provider_repo.create( - name="Ollama (Local)", - kind="ollama", - base_url="http://127.0.0.1:11434", - api_key="ollama", - models_json=json.dumps([model]), - ) - return "Ollama (Local)" + try: + provider_repo.create( + name="Ollama (Local)", + kind="ollama", + base_url=_OLLAMA_BASE_URL, + api_key="ollama", + models_json=json.dumps([model]), + ) + return "Ollama (Local)" + except Exception as exc: + existing = getattr(provider_repo, "get_by_name", None) + row = existing("Ollama (Local)") if callable(existing) else None + if row is None: + raise OSError(f"Could not save the Ollama provider: {exc}") from exc models = row.get_models() existing = next((item for item in models if str(item.get("id") or "") == model_id), None) if existing is None: @@ -86,11 +104,47 @@ def _upsert_ollama_model(provider_repo: Any, model_id: str, display: str) -> str row.id, models_json=json.dumps(models), enabled=True, - base_url=row.base_url or "http://127.0.0.1:11434", + base_url=row.base_url or _OLLAMA_BASE_URL, ) return str(row.name) +def _model_id_for(*, name: str | None, path: str, source: str) -> str: + if name and name.strip(): + raw = name.strip() + if source == "ollama": + return raw[:80] + if ":" in raw and "/" not in raw and "\\" not in raw: + return raw[:80] + return sanitize_model_name(raw) + if path: + return sanitize_model_name(Path(path).stem or path) + return sanitize_model_name("") + + +def _ollama_has_model(model_id: str) -> bool | None: + """True/False when the daemon answers; ``None`` when the list call fails.""" + try: + rows = OllamaModelManager.list_models() + except Exception as exc: + logger.warning("Could not list Ollama models while registering %s: %s", model_id, exc) + return None + for item in rows: + listed = getattr(item, "name", "") or "" + if listed == model_id or listed.startswith(f"{model_id}:"): + return True + return False + + +def _fail(message: str, *, action: str = "error") -> dict[str, Any]: + return { + "ok": False, + "action": action, + "next_step": message, + "error": message, + } + + def register_local_weight( *, path: str, @@ -100,8 +154,9 @@ def register_local_weight( provider_repo: Any, settings_repo: Any, ) -> dict[str, Any]: - """Import a GGUF/GGML file into Ollama and enable it on the local provider.""" - if source not in {"gguf", "ggml"}: + """Import a GGUF/GGML file into Ollama, or enable an already-pulled tag.""" + kind = (source or "gguf").strip().lower() + if kind not in _REGISTER_SOURCES: return { "ok": False, "action": "manual", @@ -110,29 +165,50 @@ def register_local_weight( "or use the vendor tool that created it, then search again." ), } - model_id = sanitize_model_name(name or path) - create_from_weight(model_id, path) + model_id = _model_id_for(name=name, path=path, source=kind) + if kind in _WEIGHT_SOURCES: + if not (path or "").strip(): + return _fail("Weight file path is required.") + already = _ollama_has_model(model_id) + if already is not True: + try: + create_from_weight(model_id, path) + except (OSError, ValueError, RuntimeError, UnicodeError, TimeoutError) as exc: + return _fail(str(exc).strip() or "ollama create failed") + elif kind == "ollama": + if not (name or path or "").strip(): + return _fail("Ollama model name is required.") info = None - for item in OllamaModelManager.list_models(): - if item.name == model_id or item.name.startswith(f"{model_id}:"): - info = item - break - provider_name = _upsert_ollama_model(provider_repo, model_id, model_id) + try: + for item in OllamaModelManager.list_models(): + listed = getattr(item, "name", "") or "" + if listed == model_id or listed.startswith(f"{model_id}:"): + info = item + break + except Exception as exc: + logger.warning("Ollama list after register failed for %s: %s", model_id, exc) + try: + provider_name = _upsert_ollama_model(provider_repo, model_id, model_id) + except Exception as exc: + return _fail(f"Could not save the model provider: {exc}") entry = { "name": model_id, - "path": path, - "size": int(info.size) if info is not None else size, + "path": path or "", + "size": int(getattr(info, "size", 0) or size), "source": "ollama", "registerable": False, "registered": True, - "imported_from": source, + "imported_from": kind, } - remember_weight(settings_repo, entry) + try: + remember_weight(settings_repo, entry) + except Exception as exc: + return _fail(f"Could not remember the registered model: {exc}") return { "ok": True, "action": "registered", "name": model_id, - "path": path, + "path": path or "", "provider_name": provider_name, "source": "ollama", } diff --git a/src/octop/infra/errors.py b/src/octop/infra/errors.py index 81837027..5f8f597b 100644 --- a/src/octop/infra/errors.py +++ b/src/octop/infra/errors.py @@ -105,6 +105,7 @@ class ErrorCode(StrEnum): INVITE_RATE_LIMITED = "INVITE_RATE_LIMITED" WORKSPACE_ROOT_RESTRICTED = "WORKSPACE_ROOT_RESTRICTED" TOKEN_QUOTA_EXCEEDED = "TOKEN_QUOTA_EXCEEDED" + LOCAL_MODEL_REGISTER_FAILED = "LOCAL_MODEL_REGISTER_FAILED" _DEFAULT_STATUS: dict[ErrorCode, int] = { @@ -201,6 +202,7 @@ class ErrorCode(StrEnum): ErrorCode.INVITE_RATE_LIMITED: 429, ErrorCode.WORKSPACE_ROOT_RESTRICTED: 400, ErrorCode.TOKEN_QUOTA_EXCEEDED: 403, + ErrorCode.LOCAL_MODEL_REGISTER_FAILED: 400, } diff --git a/src/octop/infra/utils/ollama_manager.py b/src/octop/infra/utils/ollama_manager.py index 9678aa59..e29d0c3a 100644 --- a/src/octop/infra/utils/ollama_manager.py +++ b/src/octop/infra/utils/ollama_manager.py @@ -5,6 +5,7 @@ import os import platform import subprocess +import tempfile import time from datetime import datetime from pathlib import Path @@ -236,6 +237,73 @@ def start_ollama_service() -> None: _ensure_ollama_server() +def resolve_weight_file(weight_path: str) -> Path: + """Return an existing weight file path, or raise ``OSError``.""" + raw = (weight_path or "").strip() + if not raw: + raise OSError("Weight file path is required.") + source = Path(raw).expanduser() + try: + source = source.resolve() + except OSError as exc: + raise OSError(f"Weight file not found: {weight_path}") from exc + try: + if not source.is_file(): + raise OSError(f"Weight file not found: {weight_path}") + except OSError as exc: + if "not found" in str(exc).lower(): + raise + raise OSError(f"Cannot read weight file {weight_path}: {exc}") from exc + return source + + +def modelfile_from_instruction(weight: Path) -> str: + """Build a Modelfile ``FROM`` line that works on Windows absolute paths. + + Unquoted ``FROM C:\\Users\\...\\file.gguf`` breaks Ollama's parser (backslashes + and spaces). Use a quoted POSIX path, or a same-directory relative name. + """ + posix = weight.as_posix() + return f'FROM "{posix}"\n' + + +def _iter_ollama_list_models(raw: Any) -> list[dict[str, Any]]: + """Normalize ``ollama.list()`` payloads (dict or pydantic ``ListResponse``).""" + if raw is None: + return [] + models = getattr(raw, "models", None) + if models is None and isinstance(raw, dict): + models = raw.get("models") + if not models: + return [] + out: list[dict[str, Any]] = [] + for item in models: + if isinstance(item, dict): + out.append(item) + continue + dumped: dict[str, Any] = {} + dump = getattr(item, "model_dump", None) + if callable(dump): + try: + maybe = dump() + except Exception: + maybe = None + if isinstance(maybe, dict): + dumped = maybe + name = dumped.get("model") or dumped.get("name") or getattr(item, "model", None) + if name is None: + name = getattr(item, "name", "") + out.append( + { + "model": name or "", + "size": dumped.get("size", getattr(item, "size", 0)) or 0, + "digest": dumped.get("digest", getattr(item, "digest", None)), + "modified_at": dumped.get("modified_at", getattr(item, "modified_at", None)), + } + ) + return out + + def create_from_weight(name: str, weight_path: str) -> None: """Import a local GGUF/GGML file into Ollama via ``ollama create``.""" binary = find_ollama_binary() @@ -245,10 +313,7 @@ def create_from_weight(name: str, weight_path: str) -> None: "before registering a local weight file." ) _ensure_ollama_server() - source = Path(weight_path) - if not source.is_file(): - raise OSError(f"Weight file not found: {weight_path}") - import tempfile + source = resolve_weight_file(weight_path) with tempfile.NamedTemporaryFile( mode="w", @@ -256,21 +321,27 @@ def create_from_weight(name: str, weight_path: str) -> None: suffix=".Modelfile", delete=False, ) as handle: - handle.write(f"FROM {source}\n") + handle.write(modelfile_from_instruction(source)) modelfile = handle.name run_kwargs: dict[str, Any] = { "check": False, "capture_output": True, "text": True, + "encoding": "utf-8", + "errors": "replace", "timeout": 600, + "cwd": str(source.parent), } if os.name == "nt": run_kwargs["creationflags"] = int(getattr(subprocess, "CREATE_NO_WINDOW", 0)) try: - completed = subprocess.run( - [binary, "create", name, "-f", modelfile], - **run_kwargs, - ) + try: + completed = subprocess.run( + [binary, "create", name, "-f", modelfile], + **run_kwargs, + ) + except (OSError, subprocess.SubprocessError, UnicodeError, TimeoutError) as exc: + raise OSError(str(exc).strip() or "ollama create failed") from exc finally: with contextlib.suppress(OSError): os.unlink(modelfile) @@ -319,12 +390,14 @@ class OllamaModelManager: def list_models() -> list[OllamaModelInfo]: """Return the current model list from ``ollama.list()``.""" ollama = _ensure_ollama() - raw = ollama.list() models: list[OllamaModelInfo] = [] - for m in raw.get("models", []): + for m in _iter_ollama_list_models(ollama.list()): + name = str(m.get("model") or m.get("name") or "") + if not name: + continue models.append( OllamaModelInfo( - name=m.get("model", ""), + name=name, size=m.get("size", 0) or 0, digest=m.get("digest"), modified_at=m.get("modified_at"), diff --git a/tests/unit/api/test_local_models_register.py b/tests/unit/api/test_local_models_register.py new file mode 100644 index 00000000..b249d3a8 --- /dev/null +++ b/tests/unit/api/test_local_models_register.py @@ -0,0 +1,38 @@ +from __future__ import annotations + +from octop.api.routers.local_models import _merge_registered, register_failure +from octop.infra.errors import ErrorCode + + +def test_register_failure_is_user_facing_400() -> None: + err = register_failure(AttributeError("'ListResponse' object has no attribute 'get'")) + assert err.code is ErrorCode.LOCAL_MODEL_REGISTER_FAILED + assert err.status == 400 + assert "ListResponse" in err.details["reason"] + envelope = err.to_envelope(locale="zh") + assert envelope["error"]["code"] == "LOCAL_MODEL_REGISTER_FAILED" + assert "无法注册本地模型" in envelope["error"]["message"] + assert "ListResponse" in envelope["error"]["message"] + + +def test_register_failure_keeps_cli_reason() -> None: + err = register_failure(OSError("Weight file not found: C:\\Models\\a.gguf")) + assert err.status == 400 + assert "Weight file not found" in err.details["reason"] + + +def test_merge_registered_marks_matching_ollama_tags() -> None: + probe = { + "installed": [ + {"name": "llama3.2:1b", "path": "", "source": "ollama", "registerable": True}, + {"name": "other", "path": "/tmp/other.gguf", "source": "gguf", "registerable": True}, + ] + } + merged = _merge_registered( + probe, + [{"name": "llama3.2:1b", "path": "", "registered": True}], + ) + by_name = {item["name"]: item for item in merged["installed"]} + assert by_name["llama3.2:1b"]["registered"] is True + assert by_name["llama3.2:1b"]["registerable"] is False + assert by_name["other"].get("registered") is not True diff --git a/tests/unit/test_local_register.py b/tests/unit/test_local_register.py index 583e3ad7..505efdb6 100644 --- a/tests/unit/test_local_register.py +++ b/tests/unit/test_local_register.py @@ -2,8 +2,11 @@ from types import SimpleNamespace +import pytest + from octop.infra.agents.providers.local_register import ( register_local_weight, + remember_weight, sanitize_model_name, ) @@ -26,15 +29,28 @@ def __init__(self) -> None: def list_all(self) -> list[SimpleNamespace]: return self.rows + def get_by_name(self, name: str) -> SimpleNamespace | None: + return next((row for row in self.rows if row.name == name), None) + def create(self, **kwargs: object) -> int: + models_json = kwargs.get("models_json") + + def _models() -> list[dict[str, object]]: + import json + + if not isinstance(models_json, str): + return [] + data = json.loads(models_json) + return data if isinstance(data, list) else [] + row = SimpleNamespace( id=1, name=kwargs["name"], kind=kwargs["kind"], base_url=kwargs.get("base_url"), api_key=kwargs.get("api_key"), - models_json=kwargs.get("models_json"), - get_models=lambda: [], + models_json=models_json, + get_models=_models, ) self.rows.append(row) return 1 @@ -46,6 +62,7 @@ def update(self, provider_id: int, **kwargs: object) -> None: def test_sanitize_model_name() -> None: assert sanitize_model_name("Qwen 2.5 / Chat.gguf") == "qwen-2.5-chat.gguf" assert sanitize_model_name("@@@") == "local-model" + assert sanitize_model_name("llama3.2:1b") == "llama3.2:1b" def test_register_refuses_safetensors() -> None: @@ -61,15 +78,23 @@ def test_register_refuses_safetensors() -> None: assert result["action"] == "manual" -def test_register_imports_gguf(monkeypatch) -> None: +def test_register_imports_gguf(monkeypatch: pytest.MonkeyPatch) -> None: created: list[tuple[str, str]] = [] monkeypatch.setattr( "octop.infra.agents.providers.local_register.create_from_weight", lambda name, path: created.append((name, path)), ) + listed = {"n": 0} + + def _list() -> list[SimpleNamespace]: + listed["n"] += 1 + if listed["n"] == 1: + return [] + return [SimpleNamespace(name="tiny", size=12)] + monkeypatch.setattr( "octop.infra.agents.providers.local_register.OllamaModelManager.list_models", - lambda: [SimpleNamespace(name="tiny", size=12)], + _list, ) providers = _Providers() settings = _Settings() @@ -84,4 +109,123 @@ def test_register_imports_gguf(monkeypatch) -> None: assert result["ok"] is True assert created == [("tiny", "/tmp/tiny.gguf")] assert providers.rows[0].name == "Ollama (Local)" + assert providers.rows[0].base_url == "http://127.0.0.1:11434/v1" assert "tiny.gguf" in settings.data.get("local_registered_weights", "") + + +def test_register_ollama_source_skips_create(monkeypatch: pytest.MonkeyPatch) -> None: + created: list[tuple[str, str]] = [] + monkeypatch.setattr( + "octop.infra.agents.providers.local_register.create_from_weight", + lambda name, path: created.append((name, path)), + ) + monkeypatch.setattr( + "octop.infra.agents.providers.local_register.OllamaModelManager.list_models", + lambda: [SimpleNamespace(name="llama3.2:1b", size=99)], + ) + result = register_local_weight( + path="", + name="llama3.2:1b", + source="ollama", + size=99, + provider_repo=_Providers(), + settings_repo=_Settings(), + ) + assert result["ok"] is True + assert result["name"] == "llama3.2:1b" + assert created == [] + + +def test_register_skips_create_when_ollama_already_has_tag( + monkeypatch: pytest.MonkeyPatch, +) -> None: + created: list[tuple[str, str]] = [] + monkeypatch.setattr( + "octop.infra.agents.providers.local_register.create_from_weight", + lambda name, path: created.append((name, path)), + ) + monkeypatch.setattr( + "octop.infra.agents.providers.local_register.OllamaModelManager.list_models", + lambda: [SimpleNamespace(name="tiny:latest", size=12)], + ) + result = register_local_weight( + path=r"C:\Users\x\Downloads\tiny.gguf", + name="tiny", + source="gguf", + size=12, + provider_repo=_Providers(), + settings_repo=_Settings(), + ) + assert result["ok"] is True + assert created == [] + + +def test_register_missing_weight_path_is_ok_false() -> None: + result = register_local_weight( + path="", + name="tiny", + source="gguf", + size=1, + provider_repo=_Providers(), + settings_repo=_Settings(), + ) + assert result["ok"] is False + assert "path" in str(result["next_step"]).lower() + + +def test_register_create_failure_is_ok_false(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + "octop.infra.agents.providers.local_register.create_from_weight", + lambda name, path: (_ for _ in ()).throw(OSError("FROM path is invalid")), + ) + monkeypatch.setattr( + "octop.infra.agents.providers.local_register.OllamaModelManager.list_models", + lambda: [], + ) + result = register_local_weight( + path=r"C:\Users\x\model.gguf", + name="model", + source="gguf", + size=1, + provider_repo=_Providers(), + settings_repo=_Settings(), + ) + assert result["ok"] is False + assert result["error"] == "FROM path is invalid" + assert "FROM path is invalid" in str(result["next_step"]) + + +def test_register_survives_list_models_attribute_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + created: list[tuple[str, str]] = [] + monkeypatch.setattr( + "octop.infra.agents.providers.local_register.create_from_weight", + lambda name, path: created.append((name, path)), + ) + + def _boom() -> list[object]: + raise AttributeError("'ListResponse' object has no attribute 'get'") + + monkeypatch.setattr( + "octop.infra.agents.providers.local_register.OllamaModelManager.list_models", + _boom, + ) + result = register_local_weight( + path="/tmp/tiny.gguf", + name="tiny", + source="gguf", + size=12, + provider_repo=_Providers(), + settings_repo=_Settings(), + ) + assert result["ok"] is True + assert created == [("tiny", "/tmp/tiny.gguf")] + + +def test_remember_weight_keys_ollama_rows_by_name() -> None: + settings = _Settings() + remember_weight(settings, {"name": "a", "path": "", "source": "ollama"}) + remember_weight(settings, {"name": "b", "path": "", "source": "ollama"}) + rows = __import__("json").loads(settings.data["local_registered_weights"]) + assert {row["name"] for row in rows} == {"a", "b"} diff --git a/tests/unit/test_ollama_create.py b/tests/unit/test_ollama_create.py new file mode 100644 index 00000000..2655438e --- /dev/null +++ b/tests/unit/test_ollama_create.py @@ -0,0 +1,132 @@ +from __future__ import annotations + +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from octop.infra.utils import ollama_manager + + +def test_modelfile_from_instruction_quotes_windows_path() -> None: + weight = Path(r"C:/Users/Alice/My Models/Qwen 2.5.gguf") + line = ollama_manager.modelfile_from_instruction(weight) + assert line.startswith("FROM ") + assert line.endswith("\n") + assert '"C:/Users/Alice/My Models/Qwen 2.5.gguf"' in line + assert "\\" not in line + + +def test_resolve_weight_file_missing(tmp_path: Path) -> None: + with pytest.raises(OSError, match="not found"): + ollama_manager.resolve_weight_file(str(tmp_path / "missing.gguf")) + + +def test_resolve_weight_file_empty() -> None: + with pytest.raises(OSError, match="required"): + ollama_manager.resolve_weight_file(" ") + + +def test_create_from_weight_writes_quoted_from_and_utf8( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + weight = tmp_path / "Qwen 2.5.gguf" + weight.write_bytes(b"gguf") + ran: dict[str, object] = {} + + monkeypatch.setattr(ollama_manager, "find_ollama_binary", lambda: "/usr/bin/ollama") + monkeypatch.setattr(ollama_manager, "_ensure_ollama_server", lambda: None) + + def _run(argv: list[str], **kwargs: object) -> SimpleNamespace: + ran["argv"] = argv + ran["kwargs"] = kwargs + ran["modelfile"] = Path(argv[-1]).read_text(encoding="utf-8") + return SimpleNamespace(returncode=0, stdout="", stderr="") + + monkeypatch.setattr(ollama_manager.subprocess, "run", _run) + ollama_manager.create_from_weight("qwen-2.5", str(weight)) + assert ran["modelfile"] == f'FROM "{weight.resolve().as_posix()}"\n' + argv = ran["argv"] + assert isinstance(argv, list) + assert argv[:3] == ["/usr/bin/ollama", "create", "qwen-2.5"] + kwargs = ran["kwargs"] + assert isinstance(kwargs, dict) + assert kwargs["encoding"] == "utf-8" + assert kwargs["errors"] == "replace" + assert Path(str(kwargs["cwd"])) == weight.parent + + +def test_create_from_weight_maps_unicode_decode_error( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + weight = tmp_path / "model.gguf" + weight.write_bytes(b"gguf") + monkeypatch.setattr(ollama_manager, "find_ollama_binary", lambda: "/usr/bin/ollama") + monkeypatch.setattr(ollama_manager, "_ensure_ollama_server", lambda: None) + + def _run(*args: object, **kwargs: object) -> SimpleNamespace: + del args, kwargs + raise UnicodeDecodeError("gbk", b"\x80", 0, 1, "invalid") + + monkeypatch.setattr(ollama_manager.subprocess, "run", _run) + with pytest.raises(OSError, match="codec"): + ollama_manager.create_from_weight("model", str(weight)) + + +def test_create_from_weight_surfaces_cli_stderr( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + weight = tmp_path / "model.gguf" + weight.write_bytes(b"gguf") + monkeypatch.setattr(ollama_manager, "find_ollama_binary", lambda: "/usr/bin/ollama") + monkeypatch.setattr(ollama_manager, "_ensure_ollama_server", lambda: None) + monkeypatch.setattr( + ollama_manager.subprocess, + "run", + lambda *a, **k: SimpleNamespace(returncode=1, stdout="", stderr="parser error"), + ) + with pytest.raises(OSError, match="parser error"): + ollama_manager.create_from_weight("model", str(weight)) + + +def test_iter_ollama_list_models_accepts_pydantic_payload() -> None: + model = SimpleNamespace( + model="llama3.2:1b", + size=11, + digest="abc", + modified_at="now", + model_dump=lambda: { + "model": "llama3.2:1b", + "size": 11, + "digest": "abc", + "modified_at": "now", + }, + ) + raw = SimpleNamespace(models=[model]) + rows = ollama_manager._iter_ollama_list_models(raw) + assert rows[0]["model"] == "llama3.2:1b" + assert rows[0]["size"] == 11 + + +def test_list_models_accepts_sdk_object(monkeypatch: pytest.MonkeyPatch) -> None: + model = SimpleNamespace(model="qwen2.5:7b", size=7, digest=None, modified_at=None) + payload = SimpleNamespace(models=[model]) + monkeypatch.setattr( + ollama_manager, + "_ensure_ollama", + lambda: SimpleNamespace(list=lambda: payload), + ) + rows = ollama_manager.OllamaModelManager.list_models() + assert rows[0].name == "qwen2.5:7b" + assert rows[0].size == 7 + + +def test_list_models_accepts_legacy_dict(monkeypatch: pytest.MonkeyPatch) -> None: + payload = {"models": [{"model": "tiny", "size": 3, "digest": None, "modified_at": None}]} + monkeypatch.setattr( + ollama_manager, + "_ensure_ollama", + lambda: SimpleNamespace(list=lambda: payload), + ) + rows = ollama_manager.OllamaModelManager.list_models() + assert rows[0].name == "tiny" From 1c48e116e36901f767402c62f152913d0b034bbf Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 16 Sep 2026 07:32:12 +0000 Subject: [PATCH 2/2] Note the local-model register fix in the changelog Co-authored-by: XYAI Labs --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aac685e8..c148e5f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ ## [Unreleased] +### 修复 + +- 模型页注册本机 GGUF / 已拉取的 Ollama 模型时，Windows 桌面不再返回笼统的 `INTERNAL_ERROR` 500；`ollama create` 使用带引号的 POSIX `FROM` 路径，并把真实失败原因（文件不存在、权限、create 失败）返回给界面。 + ### 变更 - 对话与工作区共用一份列表：侧栏「对话」进入工作区「对话」标签；点开一条才进入聊天画布。工作区侧栏默认打开「项目」标签。