diff --git a/skillopt/model/minimax_backend.py b/skillopt/model/minimax_backend.py index f1e25b9f..596924c6 100644 --- a/skillopt/model/minimax_backend.py +++ b/skillopt/model/minimax_backend.py @@ -46,6 +46,10 @@ def base_url_for_region(region: str | None) -> str: REGION = normalize_region(os.environ.get("MINIMAX_REGION")) +# An explicit base URL (a proxy or a private gateway) must survive a later +# region selection, so remember whether the current value was chosen by the +# user or merely derived from the region default. +_BASE_URL_EXPLICIT = bool(os.environ.get("MINIMAX_BASE_URL", "").strip()) BASE_URL = os.environ.get("MINIMAX_BASE_URL", "").strip() or base_url_for_region(REGION) API_KEY = os.environ.get("MINIMAX_API_KEY", "") TIMEOUT_SECONDS = float(os.environ.get("MINIMAX_TIMEOUT_SECONDS", "300") or 300) @@ -215,15 +219,20 @@ def configure_minimax_chat( enable_thinking: bool | str | None = None, ) -> None: global BASE_URL, API_KEY, TEMPERATURE, TIMEOUT_SECONDS, MAX_TOKENS, ENABLE_THINKING, REGION + global _BASE_URL_EXPLICIT with _config_lock: + if base_url is not None and str(base_url).strip(): + BASE_URL = str(base_url).strip() + _BASE_URL_EXPLICIT = True + os.environ["MINIMAX_BASE_URL"] = BASE_URL if region is not None: REGION = normalize_region(region) os.environ["MINIMAX_REGION"] = REGION - BASE_URL = base_url_for_region(REGION) - os.environ["MINIMAX_BASE_URL"] = BASE_URL - if base_url is not None: - BASE_URL = str(base_url).strip() or BASE_URL - os.environ["MINIMAX_BASE_URL"] = BASE_URL + # Only fill in the region default when no explicit base URL is in + # play; otherwise a configured proxy would be silently discarded. + if not _BASE_URL_EXPLICIT: + BASE_URL = base_url_for_region(REGION) + os.environ["MINIMAX_BASE_URL"] = BASE_URL if api_key is not None: API_KEY = str(api_key).strip() os.environ["MINIMAX_API_KEY"] = API_KEY diff --git a/skillopt_sleep/harvest_opencode.py b/skillopt_sleep/harvest_opencode.py index 0fb7e565..f68b9b95 100644 --- a/skillopt_sleep/harvest_opencode.py +++ b/skillopt_sleep/harvest_opencode.py @@ -44,17 +44,33 @@ } -def default_opencode_db() -> str: - """Return the OpenCode database selected by its environment variables.""" +def _opencode_data_candidates() -> List[str]: + """Where OpenCode may keep its data directory, most specific first.""" data_home = os.environ.get("XDG_DATA_HOME", "") if data_home: - data_dir = os.path.abspath(os.path.expanduser(data_home)) - elif sys.platform == "win32" and (os.environ.get("LOCALAPPDATA") or os.environ.get("APPDATA")): - win_appdata = os.environ.get("LOCALAPPDATA") or os.environ.get("APPDATA") or "" - data_dir = os.path.abspath(os.path.expanduser(win_appdata)) - else: - data_dir = os.path.join(os.path.expanduser("~"), ".local", "share") - opencode_data = os.path.join(data_dir, "opencode") + return [os.path.join(os.path.abspath(os.path.expanduser(data_home)), "opencode")] + + candidates: List[str] = [] + if sys.platform == "win32": + for base in (os.environ.get("LOCALAPPDATA"), os.environ.get("APPDATA")): + if base: + candidates.append( + os.path.join(os.path.abspath(os.path.expanduser(base)), "opencode") + ) + candidates.append( + os.path.join(os.path.expanduser("~"), ".local", "share", "opencode") + ) + return list(dict.fromkeys(candidates)) + + +def default_opencode_db() -> str: + """Return the OpenCode database selected by its environment variables.""" + candidates = _opencode_data_candidates() + opencode_data = candidates[0] + for candidate in candidates: + if os.path.exists(os.path.join(candidate, "opencode.db")): + opencode_data = candidate + break configured = os.environ.get("OPENCODE_DB", "") if configured == ":memory:": diff --git a/tests/test_harvest_opencode.py b/tests/test_harvest_opencode.py index acaf6a27..d54f4f5b 100644 --- a/tests/test_harvest_opencode.py +++ b/tests/test_harvest_opencode.py @@ -919,12 +919,85 @@ def test_default_database_honors_windows_appdata(monkeypatch, tmp_path: Path) -> monkeypatch.delenv("XDG_DATA_HOME", raising=False) monkeypatch.delenv("OPENCODE_DB", raising=False) monkeypatch.setattr(sys, "platform", "win32") + monkeypatch.setenv("HOME", str(tmp_path / "home")) + monkeypatch.setenv("USERPROFILE", str(tmp_path / "home")) monkeypatch.setenv("LOCALAPPDATA", str(local_app_data)) monkeypatch.delenv("APPDATA", raising=False) assert default_opencode_db() == os.path.abspath(local_app_data / "opencode" / "opencode.db") +def test_default_database_honors_windows_roaming_appdata(monkeypatch, tmp_path: Path) -> None: + """A session with only APPDATA set still resolves below Roaming.""" + roaming = tmp_path / "Roaming" + monkeypatch.delenv("XDG_DATA_HOME", raising=False) + monkeypatch.delenv("OPENCODE_DB", raising=False) + monkeypatch.setattr(sys, "platform", "win32") + monkeypatch.setenv("HOME", str(tmp_path / "home")) + monkeypatch.setenv("USERPROFILE", str(tmp_path / "home")) + monkeypatch.delenv("LOCALAPPDATA", raising=False) + monkeypatch.setenv("APPDATA", str(roaming)) + + assert default_opencode_db() == os.path.abspath(roaming / "opencode" / "opencode.db") + + +def test_default_database_prefers_the_appdata_root_that_has_the_database( + monkeypatch, tmp_path: Path +) -> None: + """Both roots are set (the usual Windows session) but only Roaming has the db.""" + local_app_data = tmp_path / "LocalAppData" + roaming = tmp_path / "Roaming" + roaming_db = roaming / "opencode" / "opencode.db" + roaming_db.parent.mkdir(parents=True) + roaming_db.write_bytes(b"") + monkeypatch.delenv("XDG_DATA_HOME", raising=False) + monkeypatch.delenv("OPENCODE_DB", raising=False) + monkeypatch.setattr(sys, "platform", "win32") + monkeypatch.setenv("LOCALAPPDATA", str(local_app_data)) + monkeypatch.setenv("APPDATA", str(roaming)) + + assert default_opencode_db() == os.path.abspath(roaming_db) + + +def test_default_database_prefers_local_appdata_when_neither_exists( + monkeypatch, tmp_path: Path +) -> None: + """With no database on disk the Local root stays the reported default.""" + local_app_data = tmp_path / "LocalAppData" + roaming = tmp_path / "Roaming" + monkeypatch.delenv("XDG_DATA_HOME", raising=False) + monkeypatch.delenv("OPENCODE_DB", raising=False) + monkeypatch.setattr(sys, "platform", "win32") + monkeypatch.setenv("HOME", str(tmp_path / "home")) + monkeypatch.setenv("USERPROFILE", str(tmp_path / "home")) + monkeypatch.setenv("LOCALAPPDATA", str(local_app_data)) + monkeypatch.setenv("APPDATA", str(roaming)) + + assert default_opencode_db() == os.path.abspath( + local_app_data / "opencode" / "opencode.db" + ) + + +def test_relative_opencode_db_resolves_below_the_selected_windows_root( + monkeypatch, tmp_path: Path +) -> None: + """A relative OPENCODE_DB follows the root that actually holds the database.""" + local_app_data = tmp_path / "LocalAppData" + roaming = tmp_path / "Roaming" + roaming_db = roaming / "opencode" / "opencode.db" + roaming_db.parent.mkdir(parents=True) + roaming_db.write_bytes(b"") + monkeypatch.delenv("XDG_DATA_HOME", raising=False) + monkeypatch.setattr(sys, "platform", "win32") + monkeypatch.setenv("LOCALAPPDATA", str(local_app_data)) + monkeypatch.setenv("APPDATA", str(roaming)) + monkeypatch.setenv("OPENCODE_DB", "nightly.db") + + assert default_opencode_db() == os.path.abspath( + roaming / "opencode" / "nightly.db" + ) + + def test_default_database_falls_back_to_home_local_share(monkeypatch, tmp_path: Path) -> None: home = tmp_path / "home" monkeypatch.delenv("XDG_DATA_HOME", raising=False) diff --git a/tests/test_minimax_region.py b/tests/test_minimax_region.py index 3dcf09ef..c9567c3d 100644 --- a/tests/test_minimax_region.py +++ b/tests/test_minimax_region.py @@ -14,7 +14,7 @@ _GLOBAL_BASE_URL = "https://api.minimax.io/v1" _CN_BASE_URL = "https://api.minimaxi.com/v1" _ENV_KEYS = ("MINIMAX_REGION", "MINIMAX_BASE_URL") -_GLOBAL_KEYS = ("REGION", "BASE_URL") +_GLOBAL_KEYS = ("REGION", "BASE_URL", "_BASE_URL_EXPLICIT") @pytest.fixture(autouse=True) @@ -116,3 +116,41 @@ def test_configure_rejects_unsupported_region() -> None: with pytest.raises(ValueError, match="Unsupported MiniMax region"): minimax_backend.configure_minimax_chat(region="apac") assert minimax_backend.get_base_url() == before + + +def test_env_base_url_survives_a_later_region_selection( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A proxy set via MINIMAX_BASE_URL is not clobbered by model.minimax_region.""" + module = _reload_with_env( + monkeypatch, + MINIMAX_REGION=None, + MINIMAX_BASE_URL="https://proxy.internal/v1", + ) + try: + module.configure_minimax_chat(region="cn_zh", base_url=None) + assert module.get_region() == "cn_zh" + assert module.get_base_url() == "https://proxy.internal/v1" + assert os.environ["MINIMAX_BASE_URL"] == "https://proxy.internal/v1" + finally: + monkeypatch.undo() + importlib.reload(minimax_backend) + + +def test_explicit_base_url_survives_a_later_region_only_call() -> None: + """Once configured explicitly, the base URL outlives subsequent region switches.""" + minimax_backend.configure_minimax_chat(base_url="https://proxy.internal/v1") + minimax_backend.configure_minimax_chat(region="cn_zh") + assert minimax_backend.get_region() == "cn_zh" + assert minimax_backend.get_base_url() == "https://proxy.internal/v1" + + minimax_backend.configure_minimax_chat(region="global_en") + assert minimax_backend.get_base_url() == "https://proxy.internal/v1" + + +def test_region_still_applies_when_base_url_is_blank() -> None: + """A blank base_url (the usual `cfg.get(...) or None`) keeps region selection working.""" + minimax_backend.configure_minimax_chat(region="cn_zh", base_url=None) + assert minimax_backend.get_base_url() == _CN_BASE_URL + minimax_backend.configure_minimax_chat(region="global_en", base_url="") + assert minimax_backend.get_base_url() == _GLOBAL_BASE_URL