fix: follow-up correctness fixes for #230 and #234

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

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:":

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)

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