From 72e8520fa3bf1392d6f4a8810fed3ed3427b0fc5 Mon Sep 17 00:00:00 2001 From: red person Date: Sat, 31 Oct 2026 16:51:59 +0300 Subject: [PATCH] fix(models): honor visible pinned IDs in route selection (#6922) --- routes/chat_routes.py | 5 +- routes/research/research_routes.py | 19 +--- routes/task/task_routes.py | 16 +-- tests/test_pinned_model_route_selection.py | 108 +++++++++++++++++++++ 4 files changed, 125 insertions(+), 23 deletions(-) create mode 100644 tests/test_pinned_model_route_selection.py diff --git a/routes/chat_routes.py b/routes/chat_routes.py index c6f306e96..d6fea66ff 100644 --- a/routes/chat_routes.py +++ b/routes/chat_routes.py @@ -1901,7 +1901,7 @@ def _ts_or_zero(value) -> float: def _recover_empty_session_model(sess, session_id: str, owner: str | None = None) -> bool: - """Re-populate sess.model from the matching endpoint's cached models. + """Recover sess.model from the endpoint's visible cached and pinned models. Covers the window between endpoint setup and the first chat send: the picker showed a model in the dropdown but the session record never got @@ -1965,6 +1965,9 @@ def _recover_empty_session_model(sess, session_id: str, owner: str | None = None visible = _visible_models(cached, getattr(ep, "hidden_models", None)) except Exception: visible = cached + if not is_chatgpt_subscription: + from src.endpoint_resolver import _endpoint_enabled_models + visible = _endpoint_enabled_models(ep) if current_model and current_model in {str(item).strip() for item in visible}: return False if is_chatgpt_subscription: diff --git a/routes/research/research_routes.py b/routes/research/research_routes.py index 89f426715..b0574e33d 100644 --- a/routes/research/research_routes.py +++ b/routes/research/research_routes.py @@ -183,6 +183,7 @@ def _resolve_endpoint_runtime(ep, owner=None, model: Optional[str] = None): OAuth tokens in ProviderAuthSession, so ep.api_key is intentionally empty. """ from src.endpoint_resolver import ( + _endpoint_enabled_models, build_chat_url, build_headers, resolve_endpoint_runtime as resolve_model_endpoint_runtime, @@ -197,7 +198,7 @@ def _resolve_endpoint_runtime(ep, owner=None, model: Optional[str] = None): ep_model = (model or "").strip() if not ep_model: try: - models = json.loads(ep.cached_models) if ep.cached_models else [] + models = _endpoint_enabled_models(ep) if models: ep_model = _first_chat_model(models) except Exception: @@ -824,23 +825,13 @@ def setup_research_routes(research_handler, session_manager=None) -> APIRouter: if not ep_url or not ep_model: # Last resort: this user's enabled endpoint, plus legacy shared rows. from src.database import SessionLocal - from src.endpoint_resolver import normalize_base, build_chat_url, build_headers db = SessionLocal() try: ep = _owned_enabled_endpoint(db, user) if ep: - base = normalize_base(ep.base_url) - fallback_url = build_chat_url(base) - fallback_headers = build_headers(ep.api_key, base) - fallback_model = "" - if ep.cached_models: - try: - models = json.loads(ep.cached_models) - if models: - fallback_model = _first_chat_model(models) - except Exception: - pass - _merge(fallback_url, fallback_model, fallback_headers) + resolved = _resolve_endpoint_runtime(ep, owner=user) + if resolved: + _merge(*resolved) finally: db.close() diff --git a/routes/task/task_routes.py b/routes/task/task_routes.py index 0b7093d15..6d152ae75 100644 --- a/routes/task/task_routes.py +++ b/routes/task/task_routes.py @@ -289,15 +289,15 @@ def _resolve_run_endpoint(db, task: ScheduledTask, run: TaskRun) -> str: try: from core.database import ModelEndpoint - eps = db.query(ModelEndpoint).filter(ModelEndpoint.is_enabled == True).all() + from src.auth_helpers import owner_filter + from src.endpoint_resolver import _endpoint_enabled_models + + eps = owner_filter( + db.query(ModelEndpoint).filter(ModelEndpoint.is_enabled == True), + ModelEndpoint, getattr(task, "owner", None), + ).all() for ep in eps: - cached = [] - if ep.cached_models: - try: - cached = json.loads(ep.cached_models) or [] - except Exception: - cached = [] - if model in cached: + if model in _endpoint_enabled_models(ep): return ep.base_url or "" except Exception: pass diff --git a/tests/test_pinned_model_route_selection.py b/tests/test_pinned_model_route_selection.py new file mode 100644 index 000000000..b901f313e --- /dev/null +++ b/tests/test_pinned_model_route_selection.py @@ -0,0 +1,108 @@ +"""Route defaults must use manually pinned IDs without widening ownership.""" + +import json +from types import SimpleNamespace + +import pytest +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker + +import core.database as cdb +import routes.chat_routes as chat +from routes.task.task_routes import _resolve_run_endpoint +from routes.research.research_routes import _resolve_endpoint_runtime + + +@pytest.fixture +def database(monkeypatch): + engine = create_engine("sqlite:///:memory:") + cdb.Base.metadata.create_all(engine) + factory = sessionmaker(bind=engine) + monkeypatch.setattr(chat, "SessionLocal", factory) + with factory() as db: + yield db + engine.dispose() + + +def endpoint(db, eid="owned", *, owner="alice", cached=(), pinned=("pinned-chat",), hidden=()): + row = cdb.ModelEndpoint( + id=eid, name=eid, owner=owner, is_enabled=True, + base_url=f"https://{eid}.example.test/v1", api_key=None, + cached_models=json.dumps(cached), pinned_models=json.dumps(pinned), + hidden_models=json.dumps(hidden), + ) + db.add(row) + db.commit() + return row + + +@pytest.mark.parametrize("cached", [(), ("hidden-chat",)]) +def test_empty_session_recovers_a_visible_pinned_model(database, cached): + ep = endpoint(database, cached=cached, hidden=("hidden-chat",)) + database.add(cdb.Session(id="chat", name="Chat", owner="alice", model="", endpoint_url=ep.base_url, endpoint_id=ep.id)) + database.commit() + session = SimpleNamespace(model="", endpoint_url=ep.base_url, endpoint_id=ep.id) + + assert chat._recover_empty_session_model(session, "chat", "alice") is True + assert session.model == "pinned-chat" + database.expire_all() + assert database.get(cdb.Session, "chat").model == "pinned-chat" + + +def test_empty_session_does_not_recover_a_hidden_pin(database): + ep = endpoint(database, hidden=("pinned-chat",)) + session = SimpleNamespace(model="", endpoint_url=ep.base_url, endpoint_id=ep.id) + assert chat._recover_empty_session_model(session, "missing", "alice") is False + assert session.model == "" + + +def test_subscription_recovery_still_requires_its_live_catalog(database): + ep = endpoint(database) + ep.base_url = "https://chatgpt.com/backend-api/codex" + database.commit() + session = SimpleNamespace(model="", endpoint_url=ep.base_url, endpoint_id=ep.id) + assert chat._recover_empty_session_model(session, "missing", "alice") is False + assert session.model == "" + + +def test_task_run_reopens_against_its_owners_pinned_endpoint(database): + endpoint(database, "foreign", owner="bob", cached=("pinned-chat",)) + own = endpoint(database) + task = SimpleNamespace(owner="alice", endpoint_url="", session_id=None, model="pinned-chat") + assert _resolve_run_endpoint(database, task, SimpleNamespace(model="pinned-chat")) == own.base_url + + +@pytest.mark.parametrize("owner", ["alice", None]) +def test_task_run_can_find_visible_pins_on_owned_or_shared_endpoints(database, owner): + ep = endpoint(database, owner=owner) + task = SimpleNamespace(owner="alice", endpoint_url="", session_id=None, model="pinned-chat") + assert _resolve_run_endpoint(database, task, SimpleNamespace(model=None)) == ep.base_url + + +def test_task_run_does_not_autoselect_a_hidden_pin(database): + endpoint(database, hidden=("pinned-chat",)) + task = SimpleNamespace(owner="alice", endpoint_url="", session_id=None, model="pinned-chat") + assert _resolve_run_endpoint(database, task, SimpleNamespace(model="pinned-chat")) == "" + + +@pytest.mark.parametrize("cached", [(), ("hidden-chat",)]) +def test_research_default_uses_the_visible_pinned_id(database, cached): + ep = endpoint(database, cached=cached, hidden=("hidden-chat",)) + assert _resolve_endpoint_runtime(ep, owner="alice") == ( + ep.base_url + "/chat/completions", "pinned-chat", {}, + ) + + +def test_research_default_does_not_autoselect_a_hidden_pin(database): + ep = endpoint(database, hidden=("pinned-chat",)) + assert _resolve_endpoint_runtime(ep, owner="alice") is None + + +def test_research_explicit_model_selection_is_preserved(database): + ep = endpoint(database, cached=("cached-chat",)) + assert _resolve_endpoint_runtime(ep, owner="alice", model="explicit-chat")[1] == "explicit-chat" + + +def test_research_cached_default_keeps_priority_over_pins(database): + ep = endpoint(database, cached=("cached-chat",)) + assert _resolve_endpoint_runtime(ep, owner="alice")[1] == "cached-chat"