feat(github): add github_get_content and github_resolve_ref read-by-ref tools (#193)

[MementoRC]

Summary

Closes #193. Adds two lightweight, read-only GitHub tools so two very common lookups no longer require cloning an entire repository.

New tools

• github_get_content — wraps GET /repos/{owner}/{repo}/contents/{path}?ref={ref}. Returns the base64-decoded file text plus blob sha and size. Lists entries for a directory path; reports too-large files (>1MB, which the contents API omits) with a pointer to the blobs API. ref is optional (defaults to the repo's default branch).
• github_resolve_ref — wraps GET /repos/{owner}/{repo}/commits/{ref} to resolve a branch, tag (including annotated tags), or short SHA to its full commit SHA (plus author/date/subject).

Both reuse github_client_context and work on private repos with the server's existing GITHUB_TOKEN.

Motivation (from the issue)

Resolving tag v2.9.6 → commit SHA and reading one workflow file from a private repo previously forced a full git_clone of a 71,000-file repo into scratchpad, then git_show + local Read — a heavyweight workaround for two one-line lookups. These tools also make the MCP a complete substitute for the gh api contents/commits calls the routing policy intentionally blocks.

Wiring

• New module github/contents.py; models GitHubGetContent / GitHubResolveRef; registered as core tools in the lean interface and re-exported via api.py.
• README tool counts updated to 64 (27 git, 34 GitHub, 3 Azure DevOps), which also reconciles a pre-existing 63-vs-62 doc inconsistency.

Tests

Adds 7 tests (decoded file, 404, directory listing, too-large; ref resolution success, 404, 422). Full github unit suite passes.

Closes #193

🤖 Generated with Claude Code

[github-actions]

Claude finished @MementoRC's task —— View job

---

PR Review

• Read and analyze changed files
• Review code quality and best practices
• Check for bugs and potential issues
• Assess security concerns
• Evaluate test coverage
• Post final review

---

Overall this is a clean, well-scoped addition. The implementation is idiomatic for this codebase. A few things worth addressing:

Bugs / Correctness

ref URL injection risk (contents.py:46)

endpoint = f"/repos/{repo_owner}/{repo_name}/contents/{path.lstrip('/')}"
if ref:
    endpoint += f"?ref={ref}"

ref is appended raw to the URL. A value like main&other=injected would silently corrupt the query string. Use aiohttp.ClientSession.get(params=...) or urllib.parse.urlencode:

params = {"ref": ref} if ref else {}
response = await client.get(endpoint, params=params)

(The client.get method already passes **kwargs through to aiohttp, so this would work without changes to client.py.)

Same issue in github_resolve_ref (contents.py:140)

endpoint = f"/repos/{repo_owner}/{repo_name}/commits/{ref}"

Here ref is a path segment, not a query param — but a ref containing / (e.g. refs/heads/main) or .. could still produce an unintended endpoint. URL-encoding the ref segment (urllib.parse.quote(ref, safe="")) would prevent this.

repo_owner / repo_name / path path segments not sanitised
Same concern applies to the owner/name/path — if an LLM or user passes a value with /../, the constructed path could resolve outside the intended repo. urllib.parse.quote on each segment is the standard fix.

---

Missing test coverage

No mock_context.return_value.__aexit__ setup (all tests)
All tests do:

mock_context.return_value.__aenter__.return_value = mock_client

but never configure __aexit__. This works because MagicMock auto-creates attributes and the default return is a coroutine that resolves False, but it's an implicit behaviour. Explicitly setting it makes intent clear and avoids surprises if the mock library changes behaviour:

mock_context.return_value.__aexit__ = AsyncMock(return_value=False)

No test for non-200/non-404 error path (github_get_content)
contents.py:56-58 handles arbitrary non-200 statuses by calling response.text(), but there's no unit test covering this branch. Same for the equivalent branch in github_resolve_ref (contents.py:146-147).

No test for unexpected encoding (contents.py:101-102)
The else branch returning raw as-is has no test.

No test for ValueError / ConnectionError exception paths (contents.py:111-119, 168-176)
These are meaningful error states (missing token, network failure) with no test coverage.

---

Minor / style

contents.py has no __all__
Every other module in github/ that is star-imported via api.py defines __all__. Without it, from .contents import * re-exports base64, logging, logger, and github_client_context into the api namespace. Add:

__all__ = ["github_get_content", "github_resolve_ref"]

registry_github.py:580-596 — new tools placed between GHAS and Release tools
Minor organisation nit: a # Content / read-by-ref Tools (Issue #193) section comment would make the grouping clearer, consistent with the other sections.

GitHubGetContent / GitHubResolveRef models have no validation
path could be validated (e.g. reject empty string) and ref could reject obviously malformed values. This is not blocking but the other models (e.g. GitHubGetRelease, branch models) set a precedent for validation.

---

Summary

The core logic is correct and the happy-path tests are solid. The two issues most worth fixing before merge are:

1. URL injection via unescaped ref/path segments — a real correctness/security concern for any input that reaches the API.
2. Missing __all__ — prevents namespace pollution from the star-import in api.py.

[github-actions]

Claude finished @MementoRC's task —— View job

---

PR Review

• Read and analyze changed files
• Review code quality and best practices
• Check for bugs and potential issues
• Assess security concerns
• Evaluate test coverage
• Post final review

---

Overall this is a clean, well-scoped addition that follows the codebase's conventions. The previous automated review already captured the main issues — here's a focused assessment of what still matters and what's fine.

Bugs / Security Issues

1. URL injection via unencoded ref query parameter (contents.py:46)

endpoint += f"?ref={ref}"

A ref value like main&other=injected silently corrupts the query string. The client.get method passes **kwargs through to aiohttp.ClientSession.get, so the fix is straightforward:

params = {"ref": ref} if ref else {}
response = await client.get(endpoint, params=params)

2. ref path segment in github_resolve_ref not URL-encoded (contents.py:140)

endpoint = f"/repos/{repo_owner}/{repo_name}/commits/{ref}"

A ref containing / (e.g. refs/heads/main) is valid and common. Without encoding, this produces /repos/.../commits/refs/heads/main, which maps to a different endpoint structure and will return 404 or silently hit the wrong resource. Fix:

import urllib.parse
endpoint = f"/repos/{repo_owner}/{repo_name}/commits/{urllib.parse.quote(ref, safe='')}"

3. Missing __all__ in contents.py

api.py uses from .contents import *. Without __all__, this re-exports base64, logging, logger, and github_client_context into the api namespace — polluting it and potentially shadowing names. Every other module with star-imports (e.g. scanning.py) defines __all__. Add:

__all__ = ["github_get_content", "github_resolve_ref"]

---

Test Coverage Gaps

Non-200/non-404 error path not tested — contents.py:56-58 and contents.py:145-147 handle arbitrary HTTP errors (e.g. 401, 500) but have no corresponding tests.

ValueError / ConnectionError exception paths not tested — contents.py:111-116 and contents.py:168-173 are meaningful branches with no coverage.

No test for unexpected encoding (else branch, contents.py:101-102) — minor, but it's a real code path.

---

Minor / Style

mock_context.return_value.__aexit__ not explicitly configured in tests — works due to MagicMock auto-creating attributes, but is implicit. Consider adding mock_context.return_value.__aexit__ = AsyncMock(return_value=False) in each test for clarity.

Models have no validation (models.py:786-808) — path could reject empty strings, ref could use the existing validate_branch_name helper or a similar pattern. Not blocking, but other models in this file (e.g. branch models) set a precedent for validation.

---

What's Good

• The implementation is idiomatic for this codebase and reuses github_client_context correctly.
• Directory listing, too-large file detection, and annotated tag resolution are all handled.
• Happy-path tests are well-structured and cover the core behaviors.
• Error types (ValueError, ConnectionError, broad Exception) are appropriately differentiated.
• The client.get passthrough via **kwargs means the params= fix requires no changes to client.py.

Summary

Two issues are worth addressing before merge:

1. ref URL injection (both functions) — correctness/security concern for any untrusted input.
2. Missing __all__ — namespace pollution from the star-import in api.py.

The test coverage gaps are secondary but would improve confidence in error handling.