fix: refresh_context usa HttpClient.post invece di requests.post (P2 audit) (#55)

* fix: refresh_context usa HttpClient.post invece di requests.post

Sostituisce requests.post() con HttpClient.post() per il dispatch
del workflow GitHub Actions. headers personalizzati (Authorization,
Accept) passati via kwargs. Gestione errori via HttpResult.is_ok
invece di try/except requests.RequestException.

import requests mantenuto per isinstance(e, requests.HTTPError)
nei tool di fetch esistenti.

* fix: mypy zero errors

- github.py: _raise_on_bad_status typed, restituisce Response.
  Tutti i 6 caller usano la response restituita (response.json/response.text).
- discussions.py: guard is_ok || response is None + err check.
- mcp_server.py _fetch: guard is_ok || response is None + response locale.

Author: Gabrymi93

src/agent_context_builder/discussions.py

@@ -87,11 +87,10 @@ def _get_repo_discussions(self, repo: str, first: int) -> list[Discussion]:
             headers={"Authorization": f"bearer {self.token}"},
             retries=0,
         )
-        if result.is_error:
-            raise result.err
+        if not result.is_ok or result.response is None:
+            raise result.err if result.err else RuntimeError("GraphQL request failed")
 
-        response = result.response
-        payload = response.json()
+        payload = result.response.json()
         if "errors" in payload:
             raise ValueError(f"GraphQL errors: {payload['errors']}")
 

src/agent_context_builder/github.py

@@ -5,7 +5,8 @@
 from dataclasses import dataclass
 from typing import Any, Optional
 
-from lab_connectors.http import HttpClient
+import requests
+from lab_connectors.http import HttpClient, HttpResult
 
 
 @dataclass
@@ -111,13 +112,14 @@ 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:
+    def _raise_on_bad_status(self, result: HttpResult, url_desc: str) -> requests.Response:
+        """Raise RuntimeError if result is error or response status >= 400.
+        Returns the response if OK."""
+        if not result.is_ok or result.response is None:
             raise RuntimeError(f"{url_desc}: {result.err}")
-        status = result.response.status_code
-        if status >= 400:
-            raise RuntimeError(f"{url_desc}: HTTP {status}")
+        if result.response.status_code >= 400:
+            raise RuntimeError(f"{url_desc}: HTTP {result.response.status_code}")
+        return result.response
 
     def _headers(self) -> dict[str, str]:
         """Build Authorization headers if token is set."""
@@ -130,10 +132,10 @@ def _get_repo_prs(self, repo: str, state: str = "open") -> list[PR]:
         url = f"{self.base_url}/repos/{self.org}/{repo}/pulls"
         params = {"state": state, "per_page": 50}
         result = self._http.get(url, params=params, headers=self._headers())
-        self._raise_on_bad_status(result, url)
+        response = self._raise_on_bad_status(result, url)
 
         prs = []
-        for item in result.response.json():
+        for item in response.json():
             prs.append(
                 PR(
                     number=item["number"],
@@ -170,8 +172,8 @@ def list_directory(self, repo: str, path: str, ref: str = "main") -> list[str] |
         params: dict[str, str] = {"ref": ref}
         try:
             result = self._http.get(url, params=params, headers=self._headers())
-            self._raise_on_bad_status(result, url)
-            items = result.response.json()
+            response = self._raise_on_bad_status(result, url)
+            items = response.json()
             if not isinstance(items, list):
                 # GitHub returns a single object if path is a file, not a directory
                 raise RuntimeError(f"{url}: path is not a directory")
@@ -203,8 +205,8 @@ def get_raw_file(self, repo: str, path: str, ref: str = "main") -> str | None:
         url = f"https://raw.githubusercontent.com/{self.org}/{repo}/{ref}/{path}"
         result = self._http.get(url, headers=self._headers())
         try:
-            self._raise_on_bad_status(result, url)
-            return result.response.text
+            response = self._raise_on_bad_status(result, url)
+            return response.text
         except Exception as exc:
             self.fetch_errors[f"{repo}:{path}"] = str(exc)
             return None
@@ -219,8 +221,8 @@ def get_repos_info(self, repos: list[str]) -> dict[str, RepoInfo]:
             try:
                 url = f"{self.base_url}/repos/{self.org}/{repo}"
                 result = self._http.get(url, headers=self._headers())
-                self._raise_on_bad_status(result, url)
-                data = result.response.json()
+                response = self._raise_on_bad_status(result, url)
+                data = response.json()
                 result_map[repo] = RepoInfo(
                     name=repo,
                     description=data.get("description") or "",
@@ -265,8 +267,8 @@ def get_latest_workflow_run(
         }
         try:
             result = self._http.get(url, params=params, headers=self._headers())
-            self._raise_on_bad_status(result, url)
-            data = result.response.json()
+            response = self._raise_on_bad_status(result, url)
+            data = response.json()
             runs = data.get("workflow_runs", [])
             if not runs:
                 return None
@@ -293,10 +295,10 @@ 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}
         result = self._http.get(url, params=params, headers=self._headers())
-        self._raise_on_bad_status(result, url)
+        response = self._raise_on_bad_status(result, url)
 
         issues = []
-        for item in result.response.json():
+        for item in response.json():
             # Skip pull requests: they have a pull_request field
             if "pull_request" in item:
                 continue

src/agent_context_builder/mcp_server.py

@@ -156,13 +156,14 @@ def _fetch(path: str, retries: int = 1, backoff: float = 1.0) -> str:
     client = HttpClient(max_retries=retries, retry_backoff=backoff, timeout=10)
     result = client.get(url, headers=headers)
 
-    if result.is_error:
+    if not result.is_ok or result.response is None:
         _log.error("fetch", "failed", path=path, error=str(result.err))
-        raise result.err
+        raise result.err if result.err else RuntimeError(f"Failed to fetch {path}")
 
-    result.response.raise_for_status()
-    _log.info("fetch", "success", path=path, status=result.response.status_code)
-    return result.response.text
+    response = result.response
+    response.raise_for_status()
+    _log.info("fetch", "success", path=path, status=response.status_code)
+    return response.text
 
 
 @mcp.tool()
@@ -361,60 +362,62 @@ def refresh_context() -> str:
             )
 
     _last_refresh_attempt = now
-    try:
-        response = requests.post(
-            f"{_API_BASE}/actions/workflows/build-context.yml/dispatches",
-            json={"ref": "main"},
-            headers={
-                "Authorization": f"token {token}",
-                "Accept": "application/vnd.github+json",
-            },
-            timeout=10,
+    client = HttpClient(timeout=10)
+    result = client.post(
+        f"{_API_BASE}/actions/workflows/build-context.yml/dispatches",
+        json={"ref": "main"},
+        headers={
+            "Authorization": f"token {token}",
+            "Accept": "application/vnd.github+json",
+        },
+    )
+
+    if not result.is_ok or result.response is None:
+        _log.error("refresh_context", "network_error", error=str(result.err))
+        return json.dumps(
+            {
+                "ok": False,
+                "tool": "refresh_context",
+                "error": f"Errore di rete: {result.err}",
+                "ts": datetime.now(timezone.utc).isoformat(),
+            }
         )
-        if response.status_code == 204:
-            _log.info("refresh_context", "triggered", ref="main")
-            return json.dumps(
-                {
-                    "ok": True,
-                    "tool": "refresh_context",
-                    "message": "Build triggerato. Artifact aggiornati entro ~1 minuto.",
-                    "ts": datetime.now(timezone.utc).isoformat(),
-                }
-            )
-        elif response.status_code == 422:
-            _log.error(
-                "refresh_context",
-                "rejected",
-                status=response.status_code,
-                body=response.text,
-            )
-            return json.dumps(
-                {
-                    "ok": False,
-                    "tool": "refresh_context",
-                    "error": "Build rifiutato (422). Verifica che il workflow sia su main.",
-                    "status_code": 422,
-                    "ts": datetime.now(timezone.utc).isoformat(),
-                }
-            )
-        else:
-            _log.error("refresh_context", "failed", status=response.status_code, body=response.text)
-            return json.dumps(
-                {
-                    "ok": False,
-                    "tool": "refresh_context",
-                    "error": f"Errore {response.status_code}: {response.text}",
-                    "status_code": response.status_code,
-                    "ts": datetime.now(timezone.utc).isoformat(),
-                }
-            )
-    except requests.RequestException as e:
-        _log.error("refresh_context", "network_error", error=str(e))
+
+    response = result.response
+    if response.status_code == 204:
+        _log.info("refresh_context", "triggered", ref="main")
+        return json.dumps(
+            {
+                "ok": True,
+                "tool": "refresh_context",
+                "message": "Build triggerato. Artifact aggiornati entro ~1 minuto.",
+                "ts": datetime.now(timezone.utc).isoformat(),
+            }
+        )
+    elif response.status_code == 422:
+        _log.error(
+            "refresh_context",
+            "rejected",
+            status=response.status_code,
+            body=response.text,
+        )
+        return json.dumps(
+            {
+                "ok": False,
+                "tool": "refresh_context",
+                "error": "Build rifiutato (422). Verifica che il workflow sia su main.",
+                "status_code": 422,
+                "ts": datetime.now(timezone.utc).isoformat(),
+            }
+        )
+    else:
+        _log.error("refresh_context", "failed", status=response.status_code, body=response.text)
         return json.dumps(
             {
                 "ok": False,
                 "tool": "refresh_context",
-                "error": f"Errore di rete: {e}",
+                "error": f"Errore {response.status_code}: {response.text}",
+                "status_code": response.status_code,
                 "ts": datetime.now(timezone.utc).isoformat(),
             }
         )

tests/test_mcp_server.py

@@ -242,7 +242,7 @@ def test_topic_index_resolve_not_found():
 
 
 # ---------------------------------------------------------------------------
-# refresh_context — still uses raw requests.post (separate concern)
+# refresh_context — via HttpClient.post
 # ---------------------------------------------------------------------------
 
 
@@ -256,10 +256,14 @@ def _reset_refresh_state(monkeypatch) -> None:
     monkeypatch.setattr(mcp_server, "_ENV_LOADED", False)
 
 
-def _mock_post_response(status: int = 204):
+def _mock_http_result(status: int = 204):
+    """Build an HttpResult-like object for mocking HttpClient.post."""
     resp = MagicMock()
     resp.status_code = status
-    return resp
+    resp.text = "mock response"
+    from lab_connectors.http import HttpResult
+
+    return HttpResult(response=resp, err=None, ssl_fallback_used=None)
 
 
 @pytest.mark.adapter
@@ -283,8 +287,8 @@ def test_refresh_context_loads_token_from_env_file(monkeypatch, tmp_path):
     monkeypatch.setenv("ACB_ENV_FILE", str(env_file))
     _reset_refresh_state(monkeypatch)
 
-    with patch("agent_context_builder.mcp_server.requests.post") as mock_post:
-        mock_post.return_value = _mock_post_response(204)
+    with patch("agent_context_builder.mcp_server.HttpClient.post") as mock_post:
+        mock_post.return_value = _mock_http_result(204)
         result = mcp_server.refresh_context()
 
     data = json.loads(result)
@@ -302,8 +306,8 @@ def test_refresh_context_loads_token_when_env_is_empty(monkeypatch, tmp_path):
     monkeypatch.setenv("ACB_ENV_FILE", str(env_file))
     _reset_refresh_state(monkeypatch)
 
-    with patch("agent_context_builder.mcp_server.requests.post") as mock_post:
-        mock_post.return_value = _mock_post_response(204)
+    with patch("agent_context_builder.mcp_server.HttpClient.post") as mock_post:
+        mock_post.return_value = _mock_http_result(204)
         result = mcp_server.refresh_context()
 
     data = json.loads(result)
@@ -324,8 +328,8 @@ def test_refresh_context_continues_after_partial_env(monkeypatch, tmp_path):
     monkeypatch.setenv("ACB_ENV_FILE", str(explicit_env))
     _reset_refresh_state(monkeypatch)
 
-    with patch("agent_context_builder.mcp_server.requests.post") as mock_post:
-        mock_post.return_value = _mock_post_response(204)
+    with patch("agent_context_builder.mcp_server.HttpClient.post") as mock_post:
+        mock_post.return_value = _mock_http_result(204)
         result = mcp_server.refresh_context()
 
     data = json.loads(result)
@@ -339,8 +343,8 @@ def test_refresh_context_success(monkeypatch):
     monkeypatch.setenv("GITHUB_TOKEN", "fake-token")
     _reset_refresh_state(monkeypatch)
 
-    with patch("agent_context_builder.mcp_server.requests.post") as mock_post:
-        mock_post.return_value = _mock_post_response(204)
+    with patch("agent_context_builder.mcp_server.HttpClient.post") as mock_post:
+        mock_post.return_value = _mock_http_result(204)
         result = mcp_server.refresh_context()
 
     data = json.loads(result)
@@ -353,8 +357,8 @@ def test_refresh_context_api_error(monkeypatch):
     monkeypatch.setenv("GITHUB_TOKEN", "fake-token")
     _reset_refresh_state(monkeypatch)
 
-    with patch("agent_context_builder.mcp_server.requests.post") as mock_post:
-        mock_post.return_value = _mock_post_response(403)
+    with patch("agent_context_builder.mcp_server.HttpClient.post") as mock_post:
+        mock_post.return_value = _mock_http_result(403)
         result = mcp_server.refresh_context()
 
     data = json.loads(result)