Test/57 add mock GitHub api server

JOURNAL.md

@@ -0,0 +1,112 @@
+## Week 7 — Issue selection
+
+**Issue link:** https://github.com/ascherj/pathreview/issues/57
+
+**Issue title:** Add a mock GitHub API server for integration tests
+
+**Tier:** [ ] Tier 1  [X] Tier 2  [ ] Tier 3
+
+I can explain what the issue is in my own words as I have done below. I can see the files this code affects and I have read through these as well as what it is testing. I have also located all of the relevant files in the repo after I opened it on Visual Studio. I understand that it looks done when the GitHub tool tests can occur. 
+
+This is my first open source contribution, but I would want to test myself and be able to give myself a challenge. I have looked through this issue in depth and I feel like I am willing to commit time and take this on. I am willing to look through multiple modules to see how everything interacts, so I am choosing a Tier 2 issue. 
+
+I've found and read the specific code the issue references. I've read enough surrounding context that I can write a rough plan for the fix. I admittedly will likely have to look some things up, but I am confident I will be able to understand quickly and figure out what is needed. I have read the test file and know what to contribute. 
+
+I've checked the issue comments and the ledger's Claims count, and I'm fine with how many others are on this issue. I've estimated the time this will take and I'm confident I can complete it before the Week 9 deadline. This issue has no open blockers or dependencies on other unresolved issues. 
+
+**Problem summary:**
+The `GitHubTool` in [agent/tools/github_tool.py](agent/tools/github_tool.py) calls the live GitHub REST API (`https://api.github.com`) to fetch repository metadata, so any
+integration test that exercises it needs real network access and is subject to GitHub's rate limits and auth. As a result, those tests are skipped in CI, there is currently no
+`tests/integration/test_github_tool.py` and no `tests/fixtures/github_responses/` directory, leaving the tool's request handling and error paths (404, 403/rate limit) untested by automation. The fix is to stand up a lightweight local mock HTTP server (`pytest-httpserver`) that serves canned JSON fixtures for GitHub endpoints and to point the tool's `base_url` at it, so the GitHub tool tests can run deterministically and offline in CI without hitting the real API.
+
+**Branch name:** test/57-add-mock-github-api-server
+
+**Setup confirmation:** [X] App runs locally at localhost:5173
+
+**Cohort ledger:** [X] Issue added to cohort ledger
+
+
+## Week 8 — Reproduction & solution planning
+
+**Reproduction commit link:** https://github.com/TanishaD111/pathreview/commit/31b67ccc409eeeb4ae852756ba168b68e99e02ed
+
+**Reproduction summary:**
+Since this issue is to add in a functionality, there is nothing broken because of it. To reproduce it, there is nothing broken to show, but what I do observe is that the two files the review mentions (tests/integration/test_github_tool.py, tests/fixtures/github_responses/) just don't exist yet. Since these tests do not exist and the tool cannot be tested offline since its URL is hardcoded, the two files just arent there. I can see in the agent/tools/github_tool.py file, there is no way to point anything to a url link since the base url is https://api.github.com with no override in the constructor, so its 200/404/403 error paths can only be exercised against the real API, which is why they're skipped in CI. 
+
+**PLAN.md link:** [X] This file is completed
+
+**Blockers or open questions:**
+n/a
+
+
+## Week 9 — Solution building & PR submission
+
+### Check-in 1 (mid-week)
+
+**Current progress:**
+All tasks from PLAN.md are done.
+
+**Next steps:**
+If I get any feedback on what I have done, I will revisit the issue and make the appropriate changes.
+
+**Blockers:**
+n/a
+
+---
+
+### Check-in 2 (end of week)
+
+**PR link:** https://github.com/ascherj/pathreview/pull/1021
+
+**Branch:** test/57-add-mock-github-api-server
+
+**What you built:**
+This fix was to set up a lightweight mock server that returns fixture responses, enabling GitHub tool tests in CI. Currently, these tests did not exist as there were no GitHub tool tests. I wrote the tests from scratch, which included making the base_url injectable as the hardcoded current URL made the tool untestable. 
+
+**Tests added or updated:**
+The test was added in tests/integration.test_github_tool.py. The four fixtures were added in tests/fixtures/github_responses. 
+
+**Self-review confirmation:** [X] make check passes  [X] make test-unit passes
+
+**Draft PR feedback received from:** none
+
+
+## Week 10 — Iteration & reflection
+
+### Reviewer feedback
+
+**Feedback received:** [ ] Yes  [X] No — still awaiting review
+
+**Summary of feedback:**
+I have received no feedback yet. 
+
+**How you responded:**
+I have received no feedback yet. 
+
+---
+
+### Reflection
+
+**What was harder than you expected?**
+[Be specific — what part of the process, codebase, or workflow
+surprised you?]
+It was just a new experience working on such a big repository. I am very used to building my own projects and understanding exactly what is going on in a repo. It was a new experience to fix a bug and write tests in someone else's repo. I have contributed to large scale repositories at work, but usually I have more of an idea as to what it is about, so it was definitely surprising to me to be able to do something with many other people fixing other issues within the same codebase. 
+
+**What did you learn about working in a large codebase?**
+[What's different about contributing to someone else's production code
+vs. building your own project?]
+With building my own project, I usually kno weverything that imy codebase. I know what all the functions are and what all the files do. It was different to work in someone else's codebase because I had to take time to look through files and folders exploring where everythig was. When I ran the verification checks, I also saw many errors not related to what I was working on. 
+
+**How did AI tools help — and where did they fall short?**
+[Where was AI assistance most useful this module? Where did you need
+to go beyond what AI could give you?]
+AI tools really helped me understand the code and what I had to work on. I was able to ask Claude to help me while I was creating the planning document. This falls short if I don't understand what the issue was intended to fix. There were times I was recommended to add something to the plan when it was unnecessary. 
+
+**What would you do differently if you started over?**
+[Issue selection, planning, implementation, or process — anything
+you'd change?]
+I would change working on the actual fix earlier. I was not able to due to certain family emergencies, but I would have loved to work on the PR earlier next time and being able to get some actual feedback on it before submitting. 
+
+**What are you most proud of from this module?**
+[One thing — it doesn't have to be the PR itself.]
+One thing I am most proud of is learning how to make a contribution in an open source codebase. I have always been intimidated of this personally and have never tried, but I am happy to say I have learned the proper way to go about making the fix using AI tools to help as well. Next time if I am working on something like this individually, I know what steps to take in order to create my PR and make my contribution. 

PLAN.md

@@ -0,0 +1,43 @@
+## Solution plan
+
+**Issue:** [#57 — Add a mock GitHub API server for integration tests](https://github.com/ascherj/pathreview/issues/57)
+
+### Understand
+The `GitHubTool` in [agent/tools/github_tool.py](agent/tools/github_tool.py) fetches repository
+metadata by calling the live GitHub REST API. Its `base_url` is hardcoded to `https://api.github.com` ([github_tool.py:23](agent/tools/github_tool.py#L23)), so any integration test that exercises the tool needs real network access, authentication, and is subject to GitHub's
+rate limits. Because of that, the GitHub tool tests are skipped in CI — in fact `tests/integration/test_github_tool.py` and `tests/fixtures/github_responses/` do not exist yet, so the tool's request handling and error paths (200 success, 404 not found, 403 rate limit) are currently untested by automation.
+
+- **Expected:** GitHub tool tests run deterministically and offline in CI against a local mock server.
+- **Actual:** No tests exist; testing the tool would hit the real API, so it is left uncovered.
+
+### Map
+Files/functions involved:
+- [agent/tools/github_tool.py](agent/tools/github_tool.py) — `GitHubTool.__init__` (make `base_url` injectable), `execute`, `_fetch_repo_metadata`, `_has_readme`. **Touch (small change).**
+- `tests/integration/test_github_tool.py` — **create.** The test module.
+- `tests/fixtures/github_responses/` — **create.** Canned JSON fixtures for GitHub endpoints.
+- [tests/conftest.py](tests/conftest.py) — optionally add a fixture for the mock server / fixture loader.
+
+No change needed to dependencies: `pytest-httpserver>=1.0.8` is already in the dev extras ([pyproject.toml:45](pyproject.toml#L45)), and CI installs `.[dev]` and runs `pytest tests/integration` ([.github/workflows/ci yml:77-79](.github/workflows/ci.yml#L77-L79)), so
+a new test file is picked up automatically.
+
+### Plan
+1. **Make `base_url` injectable.** Add an optional `base_url` parameter to `GitHubTool.__init__` (defaulting to `https://api.github.com`) so tests can point the tool at the mock server.
+2. **Add fixtures.** Create `tests/fixtures/github_responses/` with JSON files for a successful repo response and (if useful) a 404 body, mirroring the fields the tool reads (`name`, `description`, `language`, `stargazers_count`, `forks_count`, `open_issues_count`, `pushed_at`, `topics`, `homepage`).
+3. **Write the test module.** In `tests/integration/test_github_tool.py`, use the `pytest-httpserver` `httpserver` fixture to register handlers for `/repos/{user}/{repo}` and `/repos/{user}/{repo}/readme`, construct the tool with `base_url=httpserver.url_for("")`, and assert the returned `ToolResult`. Mark tests with `@pytest.mark.integration` so `make test-integration` (`-m integration`) collects them.
+4. **Cover the three paths:** success (200 → metadata mapped correctly, `has_readme` reflects the HEAD result), 404 → `success=False`, error `"Repository not found"`, and 403 → error `"Rate limited or access denied"`.
+5. **Verify locally and in CI.** Run `make test-integration` (or `pytest tests/integration/test_github_tool.py -v`) and confirm it passes offline, then `make check` for lint/format/typecheck.
+
+### Inputs & outputs
+- **Input:** the tool's `execute()` takes `{"github_username": ..., "repo_name": ...}`; tests supply these plus a mock `base_url`.
+- **Output/changes:** a new injectable `base_url` on `GitHubTool`, a new test module, and new JSON fixtures — enabling the GitHub tool tests to run in CI without live API access. No behavior change for production callers (default `base_url` is unchanged).
+
+### Risks & unknowns
+- `_has_readme` issues a separate `HEAD /repos/{user}/{repo}/readme` request; the mock server must register that route too or the success test will misreport `has_readme`.
+- `pytest-httpserver` uses HTTP (not HTTPS); confirm `httpx` calls resolve to the mock URL correctly and no code re-prepends `https://`.
+- Need to confirm whether the existing integration CI job (which spins up Postgres/Redis) imposes any extra setup on this test — the GitHub tool test itself needs neither.
+
+### Edge cases
+- Missing `github_username` or `repo_name` → tool returns `"Missing github_username or repo_name"` (no HTTP call). Worth a test.
+- Null/absent JSON fields (e.g. `description`, `language`, `homepage`) → tool coalesces to `""` / `"Unknown"`; fixture should include at least one null to exercise this.
+- README HEAD request failing/raising → `_has_readme` swallows the exception and returns `False`.
+- Generic non-404/403 status (e.g. 500) → error string `"GitHub API error: 500"`.

agent/tools/github_tool.py

@@ -2,6 +2,7 @@
 
 import httpx
 import structlog
+
 from .base import BaseTool, ToolResult
 
 logger = structlog.get_logger()
@@ -13,14 +14,17 @@ class GitHubTool(BaseTool):
     name = "github_tool"
     description = "Fetch repository metadata from GitHub"
 
-    def __init__(self, api_token: str | None = None):
+    DEFAULT_BASE_URL = "https://api.github.com"
+
+    def __init__(self, api_token: str | None = None, base_url: str | None = None):
         """Initialize GitHub tool.
 
         Args:
             api_token: GitHub API token (optional for unauthenticated requests)
+            base_url: GitHub API base URL (optional, for testing against a mock server)
         """
         self.api_token = api_token
-        self.base_url = "https://api.github.com"
+        self.base_url = (base_url or self.DEFAULT_BASE_URL).rstrip("/")
 
     def execute(self, input_data: dict) -> ToolResult:
         """Fetch GitHub repository metadata.
@@ -35,44 +39,30 @@ def execute(self, input_data: dict) -> ToolResult:
         repo_name = input_data.get("repo_name")
 
         if not username or not repo_name:
-            return ToolResult(
-                success=False,
-                data={},
-                error="Missing github_username or repo_name"
-            )
+            return ToolResult(success=False, data={}, error="Missing github_username or repo_name")
 
         try:
             repo_data = self._fetch_repo_metadata(username, repo_name)
             return ToolResult(success=True, data=repo_data)
 
         except httpx.HTTPStatusError as e:
-            logger.error("github_request_failed", status=e.response.status_code,
-                        username=username, repo=repo_name)
+            logger.error(
+                "github_request_failed",
+                status=e.response.status_code,
+                username=username,
+                repo=repo_name,
+            )
             if e.response.status_code == 404:
-                return ToolResult(
-                    success=False,
-                    data={},
-                    error="Repository not found"
-                )
+                return ToolResult(success=False, data={}, error="Repository not found")
             elif e.response.status_code == 403:
-                return ToolResult(
-                    success=False,
-                    data={},
-                    error="Rate limited or access denied"
-                )
+                return ToolResult(success=False, data={}, error="Rate limited or access denied")
             return ToolResult(
-                success=False,
-                data={},
-                error=f"GitHub API error: {e.response.status_code}"
+                success=False, data={}, error=f"GitHub API error: {e.response.status_code}"
             )
 
         except Exception as e:
             logger.error("github_tool_error", error=str(e))
-            return ToolResult(
-                success=False,
-                data={},
-                error=str(e)
-            )
+            return ToolResult(success=False, data={}, error=str(e))
 
     def _fetch_repo_metadata(self, username: str, repo_name: str) -> dict:
         """Fetch repository metadata from GitHub API.
@@ -109,8 +99,13 @@ def _fetch_repo_metadata(self, username: str, repo_name: str) -> dict:
             "homepage": repo_json.get("homepage") or "",
         }
 
-        logger.info("github_repo_fetched", username=username, repo=repo_name,
-                   language=metadata["primary_language"], stars=metadata["star_count"])
+        logger.info(
+            "github_repo_fetched",
+            username=username,
+            repo=repo_name,
+            language=metadata["primary_language"],
+            stars=metadata["star_count"],
+        )
 
         return metadata
 
@@ -132,6 +127,7 @@ def _has_readme(self, username: str, repo_name: str) -> bool:
 
         try:
             response = httpx.head(url, headers=headers, timeout=5.0)
-            return response.status_code == 200
+            status_code: int = response.status_code
+            return status_code == 200
         except Exception:
             return False

tests/fixtures/github_responses/rate_limited.json

@@ -0,0 +1,5 @@
+{
+  "message": "API rate limit exceeded for 203.0.113.42. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)",
+  "documentation_url": "https://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting",
+  "status": "403"
+}

tests/fixtures/github_responses/repo_not_found.json

@@ -0,0 +1,5 @@
+{
+  "message": "Not Found",
+  "documentation_url": "https://docs.github.com/rest/repos/repos#get-a-repository",
+  "status": "404"
+}

tests/fixtures/github_responses/repo_null_fields.json

@@ -0,0 +1,24 @@
+{
+  "id": 987654321,
+  "name": "scratch-repo",
+  "full_name": "janedoe/scratch-repo",
+  "private": false,
+  "html_url": "https://github.com/janedoe/scratch-repo",
+  "description": null,
+  "fork": false,
+  "created_at": "2024-01-05T12:00:00Z",
+  "updated_at": "2024-01-05T12:00:00Z",
+  "pushed_at": "2024-01-05T12:00:00Z",
+  "homepage": null,
+  "size": 0,
+  "stargazers_count": 0,
+  "watchers_count": 0,
+  "language": null,
+  "has_issues": true,
+  "forks_count": 0,
+  "open_issues_count": 0,
+  "license": null,
+  "topics": [],
+  "visibility": "public",
+  "default_branch": "main"
+}

tests/fixtures/github_responses/repo_success.json

@@ -0,0 +1,33 @@
+{
+  "id": 123456789,
+  "name": "weather-app",
+  "full_name": "janedoe/weather-app",
+  "private": false,
+  "html_url": "https://github.com/janedoe/weather-app",
+  "description": "A weather forecasting application built with React and OpenWeatherMap API.",
+  "fork": false,
+  "created_at": "2023-04-11T18:22:07Z",
+  "updated_at": "2024-05-02T09:14:51Z",
+  "pushed_at": "2024-05-01T21:03:44Z",
+  "homepage": "https://weather.janedoe.dev",
+  "size": 2841,
+  "stargazers_count": 42,
+  "watchers_count": 42,
+  "language": "TypeScript",
+  "has_issues": true,
+  "forks_count": 7,
+  "open_issues_count": 3,
+  "license": {
+    "key": "mit",
+    "name": "MIT License",
+    "spdx_id": "MIT"
+  },
+  "topics": [
+    "react",
+    "typescript",
+    "weather",
+    "tailwindcss"
+  ],
+  "visibility": "public",
+  "default_branch": "main"
+}