merge: resolve conflict with main (keep fence markers + _find_shell)

tests/test_runtime_provider_resolution.py

@@ -89,6 +89,38 @@ def test_resolve_runtime_provider_auto_uses_custom_config_base_url(monkeypatch):
     assert resolved["base_url"] == "https://custom.example/v1"
 
 
+def test_openrouter_key_takes_priority_over_openai_key(monkeypatch):
+    """OPENROUTER_API_KEY should be used over OPENAI_API_KEY when both are set.
+
+    Regression test for #289: users with OPENAI_API_KEY in .bashrc had it
+    sent to OpenRouter instead of their OPENROUTER_API_KEY.
+    """
+    monkeypatch.setattr(rp, "resolve_provider", lambda *a, **k: "openrouter")
+    monkeypatch.setattr(rp, "_get_model_config", lambda: {})
+    monkeypatch.delenv("OPENAI_BASE_URL", raising=False)
+    monkeypatch.delenv("OPENROUTER_BASE_URL", raising=False)
+    monkeypatch.setenv("OPENAI_API_KEY", "sk-openai-should-lose")
+    monkeypatch.setenv("OPENROUTER_API_KEY", "sk-or-should-win")
+
+    resolved = rp.resolve_runtime_provider(requested="openrouter")
+
+    assert resolved["api_key"] == "sk-or-should-win"
+
+
+def test_openai_key_used_when_no_openrouter_key(monkeypatch):
+    """OPENAI_API_KEY is used as fallback when OPENROUTER_API_KEY is not set."""
+    monkeypatch.setattr(rp, "resolve_provider", lambda *a, **k: "openrouter")
+    monkeypatch.setattr(rp, "_get_model_config", lambda: {})
+    monkeypatch.delenv("OPENAI_BASE_URL", raising=False)
+    monkeypatch.delenv("OPENROUTER_BASE_URL", raising=False)
+    monkeypatch.setenv("OPENAI_API_KEY", "sk-openai-fallback")
+    monkeypatch.delenv("OPENROUTER_API_KEY", raising=False)
+
+    resolved = rp.resolve_runtime_provider(requested="openrouter")
+
+    assert resolved["api_key"] == "sk-openai-fallback"
+
+
 def test_resolve_requested_provider_precedence(monkeypatch):
     monkeypatch.setenv("HERMES_INFERENCE_PROVIDER", "nous")
     monkeypatch.setattr(rp, "_get_model_config", lambda: {"provider": "openai-codex"})

tests/tools/test_skills_hub_clawhub.py

@@ -0,0 +1,126 @@
+#!/usr/bin/env python3
+
+import unittest
+from unittest.mock import patch
+
+from tools.skills_hub import ClawHubSource
+
+
+class _MockResponse:
+    def __init__(self, status_code=200, json_data=None, text=""):
+        self.status_code = status_code
+        self._json_data = json_data
+        self.text = text
+
+    def json(self):
+        return self._json_data
+
+
+class TestClawHubSource(unittest.TestCase):
+    def setUp(self):
+        self.src = ClawHubSource()
+
+    @patch("tools.skills_hub._write_index_cache")
+    @patch("tools.skills_hub._read_index_cache", return_value=None)
+    @patch("tools.skills_hub.httpx.get")
+    def test_search_uses_new_endpoint_and_parses_items(self, mock_get, _mock_read_cache, _mock_write_cache):
+        mock_get.return_value = _MockResponse(
+            status_code=200,
+            json_data={
+                "items": [
+                    {
+                        "slug": "caldav-calendar",
+                        "displayName": "CalDAV Calendar",
+                        "summary": "Calendar integration",
+                        "tags": ["calendar", "productivity"],
+                    }
+                ]
+            },
+        )
+
+        results = self.src.search("caldav", limit=5)
+
+        self.assertEqual(len(results), 1)
+        self.assertEqual(results[0].identifier, "caldav-calendar")
+        self.assertEqual(results[0].name, "CalDAV Calendar")
+        self.assertEqual(results[0].description, "Calendar integration")
+
+        mock_get.assert_called_once()
+        args, kwargs = mock_get.call_args
+        self.assertTrue(args[0].endswith("/skills"))
+        self.assertEqual(kwargs["params"], {"search": "caldav", "limit": 5})
+
+    @patch("tools.skills_hub.httpx.get")
+    def test_inspect_maps_display_name_and_summary(self, mock_get):
+        mock_get.return_value = _MockResponse(
+            status_code=200,
+            json_data={
+                "slug": "caldav-calendar",
+                "displayName": "CalDAV Calendar",
+                "summary": "Calendar integration",
+                "tags": ["calendar"],
+            },
+        )
+
+        meta = self.src.inspect("caldav-calendar")
+
+        self.assertIsNotNone(meta)
+        self.assertEqual(meta.name, "CalDAV Calendar")
+        self.assertEqual(meta.description, "Calendar integration")
+        self.assertEqual(meta.identifier, "caldav-calendar")
+
+    @patch("tools.skills_hub.httpx.get")
+    def test_fetch_resolves_latest_version_and_downloads_raw_files(self, mock_get):
+        def side_effect(url, *args, **kwargs):
+            if url.endswith("/skills/caldav-calendar"):
+                return _MockResponse(
+                    status_code=200,
+                    json_data={
+                        "slug": "caldav-calendar",
+                        "latestVersion": {"version": "1.0.1"},
+                    },
+                )
+            if url.endswith("/skills/caldav-calendar/versions/1.0.1"):
+                return _MockResponse(
+                    status_code=200,
+                    json_data={
+                        "files": [
+                            {"path": "SKILL.md", "rawUrl": "https://files.example/skill-md"},
+                            {"path": "README.md", "content": "hello"},
+                        ]
+                    },
+                )
+            if url == "https://files.example/skill-md":
+                return _MockResponse(status_code=200, text="# Skill")
+            return _MockResponse(status_code=404, json_data={})
+
+        mock_get.side_effect = side_effect
+
+        bundle = self.src.fetch("caldav-calendar")
+
+        self.assertIsNotNone(bundle)
+        self.assertEqual(bundle.name, "caldav-calendar")
+        self.assertIn("SKILL.md", bundle.files)
+        self.assertEqual(bundle.files["SKILL.md"], "# Skill")
+        self.assertEqual(bundle.files["README.md"], "hello")
+
+    @patch("tools.skills_hub.httpx.get")
+    def test_fetch_falls_back_to_versions_list(self, mock_get):
+        def side_effect(url, *args, **kwargs):
+            if url.endswith("/skills/caldav-calendar"):
+                return _MockResponse(status_code=200, json_data={"slug": "caldav-calendar"})
+            if url.endswith("/skills/caldav-calendar/versions"):
+                return _MockResponse(status_code=200, json_data=[{"version": "2.0.0"}])
+            if url.endswith("/skills/caldav-calendar/versions/2.0.0"):
+                return _MockResponse(status_code=200, json_data={"files": {"SKILL.md": "# Skill"}})
+            return _MockResponse(status_code=404, json_data={})
+
+        mock_get.side_effect = side_effect
+
+        bundle = self.src.fetch("caldav-calendar")
+        self.assertIsNotNone(bundle)
+        self.assertEqual(bundle.files["SKILL.md"], "# Skill")
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/tools/test_windows_compat.py

@@ -0,0 +1,80 @@
+"""Tests for Windows compatibility of process management code.
+
+Verifies that os.setsid and os.killpg are never called unconditionally,
+and that each module uses a platform guard before invoking POSIX-only functions.
+"""
+
+import ast
+import pytest
+from pathlib import Path
+
+# Files that must have Windows-safe process management
+GUARDED_FILES = [
+    "tools/environments/local.py",
+    "tools/process_registry.py",
+    "tools/code_execution_tool.py",
+    "gateway/platforms/whatsapp.py",
+]
+
+PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent
+
+
+def _get_preexec_fn_values(filepath: Path) -> list:
+    """Find all preexec_fn= keyword arguments in Popen calls."""
+    source = filepath.read_text(encoding="utf-8")
+    tree = ast.parse(source, filename=str(filepath))
+    values = []
+    for node in ast.walk(tree):
+        if isinstance(node, ast.keyword) and node.arg == "preexec_fn":
+            values.append(ast.dump(node.value))
+    return values
+
+
+class TestNoUnconditionalSetsid:
+    """preexec_fn must never be a bare os.setsid reference."""
+
+    @pytest.mark.parametrize("relpath", GUARDED_FILES)
+    def test_preexec_fn_is_guarded(self, relpath):
+        filepath = PROJECT_ROOT / relpath
+        if not filepath.exists():
+            pytest.skip(f"{relpath} not found")
+        values = _get_preexec_fn_values(filepath)
+        for val in values:
+            # A bare os.setsid would be: Attribute(value=Name(id='os'), attr='setsid')
+            assert "attr='setsid'" not in val or "IfExp" in val or "None" in val, (
+                f"{relpath} has unconditional preexec_fn=os.setsid"
+            )
+
+
+class TestIsWindowsConstant:
+    """Each guarded file must define _IS_WINDOWS."""
+
+    @pytest.mark.parametrize("relpath", GUARDED_FILES)
+    def test_has_is_windows(self, relpath):
+        filepath = PROJECT_ROOT / relpath
+        if not filepath.exists():
+            pytest.skip(f"{relpath} not found")
+        source = filepath.read_text(encoding="utf-8")
+        assert "_IS_WINDOWS" in source, (
+            f"{relpath} missing _IS_WINDOWS platform guard"
+        )
+
+
+class TestKillpgGuarded:
+    """os.killpg must always be behind a platform check."""
+
+    @pytest.mark.parametrize("relpath", GUARDED_FILES)
+    def test_no_unguarded_killpg(self, relpath):
+        filepath = PROJECT_ROOT / relpath
+        if not filepath.exists():
+            pytest.skip(f"{relpath} not found")
+        source = filepath.read_text(encoding="utf-8")
+        lines = source.splitlines()
+        for i, line in enumerate(lines):
+            stripped = line.strip()
+            if "os.killpg" in stripped or "os.getpgid" in stripped:
+                # Check that there's an _IS_WINDOWS guard in the surrounding context
+                context = "\n".join(lines[max(0, i - 15):i + 1])
+                assert "_IS_WINDOWS" in context or "else:" in context, (
+                    f"{relpath}:{i + 1} has unguarded os.killpg/os.getpgid call"
+                )