refactor: migra GitHubCollector da requests a lab_connectors.http.HttpClient (#34)

* refactor: migra GitHubCollector da requests a lab_connectors.http.HttpClient

* refactor: migra GitHubCollector a HttpClient + core dep lab-connectors

Allineato al pattern toolkit e SO: lab-connectors come dipendenza core,
import top-level senza try/except. Rimossa dipendenza da [mcp] extra.

* fix: _raise_on_bad_status per 4xx + 6 regression test

Author: Gabrymi93

.github/workflows/build-context.yml

@@ -31,7 +31,7 @@ jobs:
           python-version: '3.12'
 
       - name: Install
-        run: pip install -e ".[dev,mcp]"
+        run: pip install -e ".[dev]"
 
       - name: Test
         run: pytest --cov=src/agent_context_builder --cov-report=xml

pyproject.toml

@@ -27,12 +27,12 @@ dependencies = [
     "pydantic>=2.0",
     "pyyaml>=6.0",
     "requests>=2.28",
+    "lab-connectors @ git+https://github.com/dataciviclab/lab-connectors.git@v0.2.0",
 ]
 
 [project.optional-dependencies]
 mcp = [
     "mcp>=1.0",
-    "lab-connectors @ git+https://github.com/dataciviclab/lab-connectors.git@v0.2.0",
 ]
 dev = [
     "pytest>=7.0",

src/agent_context_builder/github.py

@@ -1,9 +1,11 @@
 """GitHub API interactions for context collection."""
 
+from __future__ import annotations
+
 from dataclasses import dataclass
 from typing import Optional
 
-import requests
+from lab_connectors.http import HttpClient
 
 
 @dataclass
@@ -51,11 +53,12 @@ def __init__(self, org: str, token: Optional[str] = None):
         self.org = org
         self.token = token
         self.base_url = "https://api.github.com"
+        self._http = HttpClient(timeout=10)
         # Populated by get_prs/get_issues — maps "<repo>:prs" or "<repo>:issues" to error message
         self.fetch_errors: dict[str, str] = {}
 
     def collector_warning(self) -> str | None:
-        """Return a human-readable warning if fetch errors suggest rate-limit or auth degradation."""
+        """Return a warning if fetch errors suggest rate-limit or auth degradation."""
         if not self.fetch_errors:
             return None
         msgs = " ".join(self.fetch_errors.values()).lower()
@@ -105,19 +108,29 @@ def get_issues(self, repos: list[str], state: str = "open") -> list[Issue]:
                 self.fetch_errors[f"{repo}:issues"] = str(e)
         return issues
 
+    def _raise_on_bad_status(self, result, url_desc: str) -> None:
+        """Raise RuntimeError if result is error or response status >= 400."""
+        if not result.is_ok:
+            raise RuntimeError(f"{url_desc}: {result.err}")
+        status = result.response.status_code  # type: ignore[union-attr]
+        if status >= 400:
+            raise RuntimeError(f"{url_desc}: HTTP {status}")
+
+    def _headers(self) -> dict[str, str]:
+        """Build Authorization headers if token is set."""
+        if self.token:
+            return {"Authorization": f"token {self.token}"}
+        return {}
+
     def _get_repo_prs(self, repo: str, state: str = "open") -> list[PR]:
         """Get PRs for a specific repo."""
         url = f"{self.base_url}/repos/{self.org}/{repo}/pulls"
         params = {"state": state, "per_page": 50}
-        headers = {}
-        if self.token:
-            headers["Authorization"] = f"token {self.token}"
-
-        response = requests.get(url, params=params, headers=headers, timeout=10)
-        response.raise_for_status()
+        result = self._http.get(url, params=params, headers=self._headers())
+        self._raise_on_bad_status(result, url)
 
         prs = []
-        for item in response.json():
+        for item in result.response.json():  # type: ignore[union-attr]
             prs.append(
                 PR(
                     number=item["number"],
@@ -145,13 +158,10 @@ def get_raw_file(self, repo: str, path: str, ref: str = "main") -> str | None:
             Raw file content as string, or None on failure.
         """
         url = f"https://raw.githubusercontent.com/{self.org}/{repo}/{ref}/{path}"
-        headers = {}
-        if self.token:
-            headers["Authorization"] = f"token {self.token}"
+        result = self._http.get(url, headers=self._headers())
         try:
-            response = requests.get(url, headers=headers, timeout=10)
-            response.raise_for_status()
-            return response.text
+            self._raise_on_bad_status(result, url)
+            return result.response.text  # type: ignore[union-attr]
         except Exception as exc:
             self.fetch_errors[f"{repo}:{path}"] = str(exc)
             return None
@@ -161,24 +171,21 @@ def get_repos_info(self, repos: list[str]) -> dict[str, RepoInfo]:
 
         Returns a dict keyed by repo name. Missing/failed repos are skipped silently.
         """
-        result: dict[str, RepoInfo] = {}
-        headers = {}
-        if self.token:
-            headers["Authorization"] = f"token {self.token}"
+        result_map: dict[str, RepoInfo] = {}
         for repo in repos:
             try:
                 url = f"{self.base_url}/repos/{self.org}/{repo}"
-                response = requests.get(url, headers=headers, timeout=10)
-                response.raise_for_status()
-                data = response.json()
-                result[repo] = RepoInfo(
+                result = self._http.get(url, headers=self._headers())
+                self._raise_on_bad_status(result, url)
+                data = result.response.json()  # type: ignore[union-attr]
+                result_map[repo] = RepoInfo(
                     name=repo,
                     description=data.get("description") or "",
                     url=data.get("html_url", ""),
                 )
             except Exception as exc:
                 self.fetch_errors[f"{repo}:info"] = str(exc)
-        return result
+        return result_map
 
     def _get_repo_issues(self, repo: str, state: str = "open") -> list[Issue]:
         """Get issues for a specific repo (excluding pull requests).
@@ -188,15 +195,11 @@ def _get_repo_issues(self, repo: str, state: str = "open") -> list[Issue]:
         """
         url = f"{self.base_url}/repos/{self.org}/{repo}/issues"
         params = {"state": state, "per_page": 50}
-        headers = {}
-        if self.token:
-            headers["Authorization"] = f"token {self.token}"
-
-        response = requests.get(url, params=params, headers=headers, timeout=10)
-        response.raise_for_status()
+        result = self._http.get(url, params=params, headers=self._headers())
+        self._raise_on_bad_status(result, url)
 
         issues = []
-        for item in response.json():
+        for item in result.response.json():  # type: ignore[union-attr]
             # Skip pull requests: they have a pull_request field
             if "pull_request" in item:
                 continue

tests/test_github.py

@@ -0,0 +1,85 @@
+"""Tests for github module HTTP boundary."""
+
+from unittest.mock import MagicMock, patch
+
+from agent_context_builder.github import GitHubCollector
+
+
+def _make_http_result(status_code: int, body: str = ""):
+    """Build a mock HttpResult with the given status code."""
+    mock_resp = MagicMock()
+    mock_resp.status_code = status_code
+    mock_resp.text = body
+    mock_resp.json.return_value = {}
+    result = MagicMock()
+    result.is_ok = status_code < 500  # come HttpClient reale
+    result.response = mock_resp
+    result.err = None
+    return result
+
+
+class TestGetRawFile:
+    """get_raw_file deve gestire 4xx come errori, non restituire body di errore."""
+
+    def test_404_returns_none(self):
+        """404 su get_raw_file → None + fetch_errors popolato."""
+        collector = GitHubCollector("test-org", token="fake")
+        with patch.object(collector._http, "get", return_value=_make_http_result(404)):
+            result = collector.get_raw_file("some-repo", "data/file.json")
+        assert result is None
+        assert "some-repo:data/file.json" in collector.fetch_errors
+        assert "HTTP 404" in collector.fetch_errors["some-repo:data/file.json"]
+
+    def test_200_returns_text(self):
+        """200 su get_raw_file → body restituito."""
+        collector = GitHubCollector("test-org", token="fake")
+        with patch.object(collector._http, "get", return_value=_make_http_result(200, "ok")):
+            result = collector.get_raw_file("some-repo", "data/file.json")
+        assert result == "ok"
+
+    def test_403_records_error(self):
+        """403 su get_raw_file → None + errore tracciato."""
+        collector = GitHubCollector("test-org", token="fake")
+        with patch.object(collector._http, "get", return_value=_make_http_result(403)):
+            result = collector.get_raw_file("some-repo", "data/file.json")
+        assert result is None
+        assert "HTTP 403" in collector.fetch_errors["some-repo:data/file.json"]
+
+
+class TestGetReposInfo:
+    """get_repos_info deve segnalare 4xx come errori, non dati vuoti."""
+
+    def test_403_skips_and_records_error(self):
+        """403 su get_repos_info → repo skippato + errore tracciato."""
+        collector = GitHubCollector("test-org", token="fake")
+        with patch.object(collector._http, "get", return_value=_make_http_result(403)):
+            result = collector.get_repos_info(["some-repo"])
+        assert "some-repo" not in result
+        assert "some-repo:info" in collector.fetch_errors
+        assert "HTTP 403" in collector.fetch_errors["some-repo:info"]
+
+
+class TestGetRepoPrs:
+    """get_prs deve propagare 4xx come errori."""
+
+    def test_403_records_error(self):
+        """403 su get_prs → lista vuota + errore tracciato."""
+        collector = GitHubCollector("test-org", token="fake")
+        with patch.object(collector._http, "get", return_value=_make_http_result(403)):
+            result = collector.get_prs(["some-repo"])
+        assert result == []
+        assert "some-repo:prs" in collector.fetch_errors
+        assert "HTTP 403" in collector.fetch_errors["some-repo:prs"]
+
+
+class TestGetIssues:
+    """get_issues deve propagare 4xx come errori."""
+
+    def test_403_records_error(self):
+        """403 su get_issues → lista vuota + errore tracciato."""
+        collector = GitHubCollector("test-org", token="fake")
+        with patch.object(collector._http, "get", return_value=_make_http_result(403)):
+            result = collector.get_issues(["some-repo"])
+        assert result == []
+        assert "some-repo:issues" in collector.fetch_errors
+        assert "HTTP 403" in collector.fetch_errors["some-repo:issues"]