diff --git a/frontend/src/views/SettingsView.vue b/frontend/src/views/SettingsView.vue index f749eee..817b895 100644 --- a/frontend/src/views/SettingsView.vue +++ b/frontend/src/views/SettingsView.vue @@ -2160,9 +2160,10 @@ function formatUserDate(iso: string): string {

Git Forge

- Optional read-only connection to your git forge (Gitea) so snippet - code can be fetched and drift-checked server-side. A read-scope API - token is enough. Leave the kind unset to keep the integration off. + Optional read-only connection to your git forge (Gitea or GitHub) so + snippet code can be fetched, drift-checked, and coverage-measured + server-side. A read-scope token is enough. Leave the kind unset to + keep the integration off.

@@ -2179,6 +2180,13 @@ function formatUserDate(iso: string): string {
+

+ Gitea: an access token with read scope on repositories. GitHub: + a fine-grained PAT with Contents: Read-only (or a classic token + with repo read). For GitHub, use + https://github.com as the base URL — or your + GitHub Enterprise instance's URL. +

diff --git a/src/scribe/routes/webhooks.py b/src/scribe/routes/webhooks.py index 817bac8..12ca7de 100644 --- a/src/scribe/routes/webhooks.py +++ b/src/scribe/routes/webhooks.py @@ -43,14 +43,24 @@ FORGE_WEBHOOK_SECRET_KEY = "forge_webhook_secret" def signature_ok(secret: str, body: bytes, signature: str) -> bool: - """Validate Gitea's push signature: X-Gitea-Signature is the hex HMAC-SHA256 - of the raw body under the webhook secret. Constant-time compare.""" + """Validate a push signature: the hex HMAC-SHA256 of the raw body under + the webhook secret (Gitea's X-Gitea-Signature verbatim; GitHub's + X-Hub-Signature-256 minus its "sha256=" prefix). Constant-time compare.""" if not secret or not signature: return False expected = hmac.new(secret.encode("utf-8"), body, hashlib.sha256).hexdigest() return hmac.compare_digest(expected, signature.strip().lower()) +def delivered_signature(headers) -> str: + """The HMAC hex a delivery carries, whichever forge sent it: Gitea's + X-Gitea-Signature verbatim, or GitHub's X-Hub-Signature-256 minus its + "sha256=" scheme prefix (#2693). Empty when neither header is present.""" + return headers.get("X-Gitea-Signature", "") or headers.get( + "X-Hub-Signature-256", "" + ).removeprefix("sha256=") + + def push_facts(payload: dict) -> tuple[str, list[str], list[str], str]: """(repo identity, changed paths, removed paths, head commit) from a Gitea push payload. Tolerant: absent fields read as empty, never raise.""" @@ -76,7 +86,10 @@ async def forge_push(): return jsonify({"error": "Not found"}), 404 body = await request.get_data() - if not signature_ok(secret, body, request.headers.get("X-Gitea-Signature", "")): + # The payload shape push_facts reads (repository.clone_url, + # commits[].added/modified/removed, after) is common to both forges, so + # the signature header is the whole GitHub mapping. + if not signature_ok(secret, body, delivered_signature(request.headers)): return jsonify({"error": "Invalid signature"}), 401 try: diff --git a/src/scribe/services/coverage.py b/src/scribe/services/coverage.py index e518b40..524e13a 100644 --- a/src/scribe/services/coverage.py +++ b/src/scribe/services/coverage.py @@ -33,7 +33,7 @@ import re import tarfile from datetime import datetime, timezone -from scribe.services.forge import GiteaForge, get_forge +from scribe.services.forge import ForgeAdapter, get_forge from scribe.services.repo_bindings import keys_for_project from scribe.services.settings import get_setting, set_setting @@ -252,7 +252,7 @@ async def _recorded_locations(user_id: int, project_id: int) -> list[tuple[str, async def compute_coverage( - user_id: int, project_id: int, *, forge: GiteaForge | None = None + user_id: int, project_id: int, *, forge: ForgeAdapter | None = None ) -> dict | None: """Measure a project's pattern-library coverage against its bound repos. @@ -299,7 +299,7 @@ async def compute_coverage( async def refresh_coverage( - user_id: int, project_id: int, *, forge: GiteaForge | None = None + user_id: int, project_id: int, *, forge: ForgeAdapter | None = None ) -> dict | None: """Compute and cache. The only writer of the cache key.""" coverage = await compute_coverage(user_id, project_id, forge=forge) diff --git a/src/scribe/services/forge.py b/src/scribe/services/forge.py index 2a4af73..2936144 100644 --- a/src/scribe/services/forge.py +++ b/src/scribe/services/forge.py @@ -16,8 +16,9 @@ Design constraints, in force everywhere below: method to misuse. The token an operator mints for it only ever needs read scope, and the docs say so. - The contract stays as small as its consumers (steps 5-7): read_file / - default_branch / resolve_repo / check. GitHub later implements this same - contract (step 8); resist widening it speculatively. + latest_commit / archive / default_branch / resolve_repo / check. Two + implementations (Gitea, GitHub — step 8) keep it honest; resist widening + it speculatively. - Repo identity is the repo-binding key — `normalize_repo_key`'s host/owner/repo — so the join between a snippet's recorded repo and the forge needs no new identity scheme. The host segment selects whether THIS @@ -50,8 +51,8 @@ FORGE_KIND_KEY = "forge_kind" FORGE_BASE_URL_KEY = "forge_base_url" FORGE_TOKEN_KEY = "forge_token" -# Kinds an instance can configure. GitHub joins in step 8 of milestone 288. -FORGE_KINDS = ("gitea",) +# Kinds an instance can configure. Matches _FORGE_CLASSES below. +FORGE_KINDS = ("gitea", "github") # Total budget per forge call. Consumers either have a cache to fall back to # (step 5) or a user watching a button (the test probe) — neither tolerates a @@ -89,15 +90,19 @@ def _host_of(url: str) -> str: return (urlsplit(url).hostname or "").lower() -class GiteaForge: - """The Gitea implementation of the forge contract, over its REST API. +class ForgeAdapter: + """The shared plumbing of the forge contract; adapters supply the API + base, auth headers, and any endpoint that differs. `transport` exists for tests: httpx.MockTransport makes the contract testable without a live server or a new dependency. Production callers never pass it. """ - kind = "gitea" + kind = "" + # One "newest commit for this path" page — the endpoint is shared but the + # page-size parameter is not, so each adapter names its own. + _commit_page_params: dict = {} def __init__(self, base_url: str, token: str, *, transport=None) -> None: self.base_url = (base_url or "").rstrip("/") @@ -125,11 +130,22 @@ class GiteaForge: return None return rest + def _api_base(self) -> str: + raise NotImplementedError + + def _headers(self) -> dict: + raise NotImplementedError + def _client(self) -> httpx.AsyncClient: kwargs: dict = { - "base_url": f"{self.base_url}/api/v1", - "headers": {"Authorization": f"token {self._token}"}, + "base_url": self._api_base(), + "headers": self._headers(), "timeout": _TIMEOUT, + # GitHub serves tarballs via a 302 to codeload. httpx drops the + # Authorization header on the cross-host hop, and GitHub's + # redirect target carries its own short-lived token in the URL — + # so following is both necessary there and harmless on Gitea. + "follow_redirects": True, } if self._transport is not None: kwargs["transport"] = self._transport @@ -150,6 +166,87 @@ class GiteaForge: raise ForgeError(f"forge returned HTTP {resp.status_code} for {url}") return resp + def _decode_contents(self, payload, path: str) -> str: + """Both forges speak the same contents-API dialect: a base64 file + object, a list for a directory.""" + if isinstance(payload, list): + raise ForgeNotFound(f"{path} is a directory on the forge, not a file") + if payload.get("type") != "file": + raise ForgeNotFound( + f"{path} is a {payload.get('type', 'non-file')} on the forge" + ) + if payload.get("encoding") != "base64" or payload.get("content") is None: + raise ForgeError(f"forge returned no readable content for {path}") + try: + return base64.b64decode(payload["content"]).decode("utf-8") + except (binascii.Error, UnicodeDecodeError) as exc: + raise ForgeError(f"forge content for {path} is not utf-8 text") from exc + + async def _newest_commit( + self, client: httpx.AsyncClient, repo: str, path: str, ref: str + ) -> str: + params: dict = {**self._commit_page_params, "path": path} + if ref: + params["sha"] = ref + resp = await self._get(client, f"/repos/{repo}/commits", params=params) + payload = resp.json() + # Tolerant parse on purpose: the caller uses this as an optimization + # and falls back to read_file, so a surprising payload must read as + # "don't know", never break a pull. + if isinstance(payload, list) and payload and isinstance(payload[0], dict): + return str(payload[0].get("sha") or "") + return "" + + async def latest_commit(self, repo: str, path: str, ref: str = "") -> str: + """The newest commit touching ``path`` — "" when it can't be told. + + The cached-SHA short-circuit (#2693): when a snippet's provenance + already names a commit, this one small call can prove the file + hasn't moved since — no content transfer, which is what keeps + pull-time freshness inside GitHub's rate limits. + """ + async with self._client() as client: + return await self._newest_commit(client, repo, path, ref) + + def _archive_url(self, repo: str, ref: str) -> str: + raise NotImplementedError + + async def archive(self, repo: str, ref: str) -> bytes: + """The repo's content at ``ref`` as a gzipped tarball, in one request. + + Coverage measurement (step 7) needs every source file's text; per-file + reads would mean one API call per file, so the archive endpoint is the + only shape that scales past toy repos. Callers must never run this in + a request path — it moves the whole repo. + """ + async with self._client() as client: + resp = await self._get( + client, self._archive_url(repo, ref), timeout=_ARCHIVE_TIMEOUT + ) + return resp.content + + async def default_branch(self, repo: str) -> str: + async with self._client() as client: + resp = await self._get(client, f"/repos/{repo}") + branch = (resp.json() or {}).get("default_branch") or "" + if not branch: + raise ForgeError(f"forge reported no default branch for {repo}") + return branch + + +class GiteaForge(ForgeAdapter): + """The Gitea implementation of the forge contract, over its REST API.""" + + kind = "gitea" + # stat/verification/files add per-commit work Gitea skips when told to. + _commit_page_params = {"limit": 1, "stat": "false"} + + def _api_base(self) -> str: + return f"{self.base_url}/api/v1" + + def _headers(self) -> dict: + return {"Authorization": f"token {self._token}"} + async def read_file(self, repo: str, path: str, ref: str = "") -> ForgeFile: """Read one file's current content, with the commit it was served at. @@ -164,18 +261,7 @@ class GiteaForge: params=params, ) payload = resp.json() - if isinstance(payload, list): - raise ForgeNotFound(f"{path} is a directory on the forge, not a file") - if payload.get("type") != "file": - raise ForgeNotFound( - f"{path} is a {payload.get('type', 'non-file')} on the forge" - ) - if payload.get("encoding") != "base64" or payload.get("content") is None: - raise ForgeError(f"forge returned no readable content for {path}") - try: - content = base64.b64decode(payload["content"]).decode("utf-8") - except (binascii.Error, UnicodeDecodeError) as exc: - raise ForgeError(f"forge content for {path} is not utf-8 text") from exc + content = self._decode_contents(payload, path) return ForgeFile( content=content, # last_commit_sha is the commit that last touched the file — the @@ -185,29 +271,8 @@ class GiteaForge: path=payload.get("path") or path, ) - async def archive(self, repo: str, ref: str) -> bytes: - """The repo's content at ``ref`` as a gzipped tarball, in one request. - - Coverage measurement (step 7) needs every source file's text; per-file - reads would mean one API call per file, so the archive endpoint is the - only shape that scales past toy repos. Callers must never run this in - a request path — it moves the whole repo. - """ - async with self._client() as client: - resp = await self._get( - client, - f"/repos/{repo}/archive/{quote(ref, safe='')}.tar.gz", - timeout=_ARCHIVE_TIMEOUT, - ) - return resp.content - - async def default_branch(self, repo: str) -> str: - async with self._client() as client: - resp = await self._get(client, f"/repos/{repo}") - branch = (resp.json() or {}).get("default_branch") or "" - if not branch: - raise ForgeError(f"forge reported no default branch for {repo}") - return branch + def _archive_url(self, repo: str, ref: str) -> str: + return f"/repos/{repo}/archive/{quote(ref, safe='')}.tar.gz" async def check(self) -> dict: """Health probe for the settings test button: reach the forge AND @@ -222,6 +287,78 @@ class GiteaForge: } +class GitHubForge(ForgeAdapter): + """The GitHub implementation — the second one, which is the point (#2693): + it proves the seam is a contract rather than a Gitea-shaped hole. Works + against github.com and GitHub Enterprise; the token is a fine-grained PAT + with Contents: Read-only (or a classic token with `repo` read).""" + + kind = "github" + _commit_page_params = {"per_page": 1} + + _API_VERSION = "2022-11-28" + + def _api_base(self) -> str: + # github.com's API lives on its own host; GitHub Enterprise serves + # the same API under the instance at /api/v3. + if self.host == "github.com": + return "https://api.github.com" + return f"{self.base_url}/api/v3" + + def _headers(self) -> dict: + return { + "Authorization": f"Bearer {self._token}", + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": self._API_VERSION, + } + + async def read_file(self, repo: str, path: str, ref: str = "") -> ForgeFile: + """Same contents-API dialect as Gitea, minus one field: GitHub's + payload carries only the blob sha — a content address, not a point in + history — so the provenance stamp costs one extra commits call. "" + when even that can't be told; consumers already treat an empty stamp + as "don't restamp".""" + params = {"ref": ref} if ref else None + async with self._client() as client: + resp = await self._get( + client, + f"/repos/{repo}/contents/{quote(path, safe='/')}", + params=params, + ) + payload = resp.json() + content = self._decode_contents(payload, path) + try: + commit_sha = await self._newest_commit(client, repo, path, ref) + except ForgeError: + commit_sha = "" + return ForgeFile( + content=content, + commit_sha=commit_sha, + path=payload.get("path") or path, + ) + + def _archive_url(self, repo: str, ref: str) -> str: + return f"/repos/{repo}/tarball/{quote(ref, safe='')}" + + async def check(self) -> dict: + """GitHub has no /version endpoint; proving the token against /user + is the whole probe, and the pinned API version stands in as the + version string.""" + async with self._client() as client: + user = (await self._get(client, "/user")).json() or {} + return { + "ok": True, + "version": f"GitHub API {self._API_VERSION}", + "username": user.get("login") or "", + } + + +_FORGE_CLASSES: dict[str, type[ForgeAdapter]] = { + "gitea": GiteaForge, + "github": GitHubForge, +} + + async def forge_config() -> dict: """The instance's forge configuration, DB-first with env fallback. @@ -241,11 +378,12 @@ async def forge_config() -> dict: } -async def get_forge(*, transport=None) -> GiteaForge | None: +async def get_forge(*, transport=None) -> ForgeAdapter | None: """The configured forge adapter, or None — and None means "behave exactly as if this module did not exist", which every consumer must honor.""" cfg = await forge_config() - if cfg["kind"] not in FORGE_KINDS: + cls = _FORGE_CLASSES.get(cfg["kind"]) + if cls is None: if cfg["kind"]: # A kind we don't implement is a misconfiguration, not "off" — # say so once per lookup rather than silently reading as absent. @@ -256,4 +394,4 @@ async def get_forge(*, transport=None) -> GiteaForge | None: if not cfg["base_url"].startswith(("http://", "https://")): logger.warning("forge base URL %r has no http(s) scheme — forge disabled", cfg["base_url"]) return None - return GiteaForge(cfg["base_url"], cfg["token"], transport=transport) + return cls(cfg["base_url"], cfg["token"], transport=transport) diff --git a/src/scribe/services/snippets.py b/src/scribe/services/snippets.py index baefdf5..44d92ee 100644 --- a/src/scribe/services/snippets.py +++ b/src/scribe/services/snippets.py @@ -1046,10 +1046,27 @@ async def attach_live_body(note, data: dict) -> None: data["body_freshness"] = "repo-not-on-this-forge" return + stored_prov_sha = (fields.get("provenance") or {}).get("commit_sha") or "" + + async def _probe(): + # Cached-SHA short-circuit (#2693): provenance names the commit the + # cached code was last confirmed at, so one cheap "newest commit + # touching this path" call can prove the file hasn't moved since — + # no content transfer. That economy is what fits pull-time freshness + # inside GitHub's rate limits; it's merely nice on a self-hosted + # Gitea. Any surprise (error, empty, mismatch) falls through to the + # full fetch, which stays the authoritative path. + if stored_prov_sha: + try: + head = await forge.latest_commit(repo, loc["path"]) + except ForgeError: + head = "" + if head and head == stored_prov_sha: + return None + return await forge.read_file(repo, loc["path"]) + try: - fetched = await asyncio.wait_for( - forge.read_file(repo, loc["path"]), timeout=PULL_FETCH_BUDGET_S - ) + fetched = await asyncio.wait_for(_probe(), timeout=PULL_FETCH_BUDGET_S) except ForgeNotFound: data["body_source"] = "cache" data["body_freshness"] = "missing" @@ -1064,6 +1081,14 @@ async def attach_live_body(note, data: dict) -> None: data["body_freshness"] = "unreachable" return + if fetched is None: + # Unchanged since the provenance commit — confirmed against the + # source without moving the file. Same stamp, so nothing to persist + # (the same-sha rule); the body already reflects that commit. + data["body_source"] = "forge" + data["body_freshness"] = "current" + return + cached = _normalized_code(fields.get("code") or "") if cached and cached in _normalized_code(fetched.content): data["body_source"] = "forge" diff --git a/tests/test_forge_webhook.py b/tests/test_forge_webhook.py index eab71aa..0a86048 100644 --- a/tests/test_forge_webhook.py +++ b/tests/test_forge_webhook.py @@ -19,7 +19,7 @@ import hmac import pytest import pytest_asyncio -from scribe.routes.webhooks import push_facts, signature_ok +from scribe.routes.webhooks import delivered_signature, push_facts, signature_ok from scribe.services.snippets import _path_touches SECRET = "wh-secret" @@ -41,6 +41,46 @@ def test_signature_gate(): assert signature_ok("", body, _sign(body)) is False +def test_delivered_signature_reads_both_forges_headers(): + """#2693: GitHub signs the same HMAC but ships it as + X-Hub-Signature-256: sha256= — the whole webhook payload mapping is + this header, so pin it.""" + hexsig = _sign(b"{}") + assert delivered_signature({"X-Gitea-Signature": hexsig}) == hexsig + assert delivered_signature({"X-Hub-Signature-256": f"sha256={hexsig}"}) == hexsig + # Gitea's header wins when both appear; absence reads as empty (→ 401). + assert delivered_signature({}) == "" + # The stripped GitHub form still passes the gate end to end. + assert signature_ok( + SECRET, b'{"x": 1}', + delivered_signature({"X-Hub-Signature-256": "sha256=" + _sign(b'{"x": 1}')}), + ) + + +def test_push_facts_reads_a_github_shaped_payload(): + """GitHub's push payload carries the same fields push_facts consumes — + asserted against a real-shaped sample so a rename on either side of the + mapping breaks a test instead of silently flagging nothing.""" + payload = { + "ref": "refs/heads/main", + "after": HEAD, + "repository": { + "full_name": "alice/widget", + "clone_url": "https://github.com/alice/widget.git", + "html_url": "https://github.com/alice/widget", + }, + "commits": [ + {"id": "a" * 40, "added": [], "modified": ["src/x.py"], "removed": []}, + ], + "head_commit": {"id": HEAD}, + } + raw, changed, removed, head = push_facts(payload) + assert raw == "https://github.com/alice/widget.git" + assert changed == ["src/x.py"] + assert removed == [] + assert head == HEAD + + # --- unit: payload parsing --------------------------------------------------- def test_push_facts_collects_and_dedups_paths(): diff --git a/tests/test_services_forge.py b/tests/test_services_forge.py index 473111f..e1d4544 100644 --- a/tests/test_services_forge.py +++ b/tests/test_services_forge.py @@ -205,10 +205,19 @@ def test_forge_error_taxonomy_is_catchable_as_one_family(): def test_adapter_contract_surface(): - """Step 8's GitHub adapter implements exactly this surface — pin it.""" - for method in ("read_file", "default_branch", "resolve_repo", "check"): - assert callable(getattr(GiteaForge, method)) + """Both adapters implement exactly this surface — the second + implementation is what proves it's a contract (#2693).""" + from scribe.services.forge import FORGE_KINDS, GitHubForge + + for cls in (GiteaForge, GitHubForge): + for method in ( + "read_file", "latest_commit", "archive", + "default_branch", "resolve_repo", "check", + ): + assert callable(getattr(cls, method)) assert GiteaForge.kind == "gitea" + assert GitHubForge.kind == "github" + assert set(FORGE_KINDS) == {"gitea", "github"} def test_admin_routes_registered(): @@ -241,3 +250,122 @@ def test_config_has_the_docker_secret_channel(): from scribe.config import Config for attr in ("FORGE_KIND", "FORGE_BASE_URL", "FORGE_TOKEN"): assert hasattr(Config, attr) + + +# --- the GitHub adapter (#2693) ---------------------------------------------- +# Same contract, second implementation. Where behavior below differs from the +# Gitea tests above, that difference IS the adapter's job: API host mapping, +# Bearer auth, the missing last_commit_sha, the codeload redirect. + +def _github(handler, base: str = "https://github.com"): + from scribe.services.forge import GitHubForge + + return GitHubForge(base, "gh-tok", transport=httpx.MockTransport(handler)) + + +def test_github_resolve_repo_is_the_same_host_join(): + from scribe.services.forge import GitHubForge + + forge = GitHubForge("https://github.com", "t") + assert forge.resolve_repo("git@github.com:alice/Widget.git") == "alice/widget" + # A Gitea-hosted repo is a NORMAL miss for a GitHub forge, and vice versa. + assert forge.resolve_repo("https://git.example.com/alice/widget") is None + + +async def test_github_api_base_maps_dot_com_and_enterprise(): + seen = [] + + def handler(request): + seen.append(str(request.url)) + return _json(200, {"default_branch": "main"}) + + await _github(handler).default_branch("alice/widget") + await _github(handler, base="https://ghe.example.com").default_branch("alice/widget") + assert seen[0] == "https://api.github.com/repos/alice/widget" + assert seen[1] == "https://ghe.example.com/api/v3/repos/alice/widget" + + +async def test_github_read_file_decodes_and_stamps_from_the_commits_call(): + content = "def canonical():\n return 1\n" + + def handler(request): + assert request.headers["Authorization"] == "Bearer gh-tok" + assert request.headers["X-GitHub-Api-Version"] + if request.url.path.endswith("/commits"): + assert request.url.params["path"] == "src/x.py" + assert request.url.params["per_page"] == "1" + return _json(200, [{"sha": "c" * 40}]) + return _json(200, { + "type": "file", "encoding": "base64", + "content": base64.b64encode(content.encode()).decode(), + "path": "src/x.py", "sha": "blob-sha-not-a-point-in-history", + }) + + f = await _github(handler).read_file("alice/widget", "src/x.py") + assert f.content == content + # From /commits — GitHub's contents payload only carries the blob sha, + # which is a content address, not the provenance stamp. + assert f.commit_sha == "c" * 40 + + +async def test_github_read_file_serves_content_even_when_the_stamp_fails(): + content = "x = 1\n" + + def handler(request): + if request.url.path.endswith("/commits"): + return httpx.Response(500) + return _json(200, {"type": "file", "encoding": "base64", + "content": base64.b64encode(content.encode()).decode()}) + + f = await _github(handler).read_file("alice/widget", "x.py") + assert f.content == content + assert f.commit_sha == "" # unknown stamp, not a failed read + + +async def test_github_archive_follows_the_codeload_redirect(): + def handler(request): + if request.url.host == "api.github.com": + return httpx.Response(302, headers={ + "Location": "https://codeload.github.com/alice/widget/tar.gz/main", + }) + assert request.url.host == "codeload.github.com" + # httpx drops Authorization on the cross-host hop — codeload's URL + # carries its own grant, and leaking the PAT there would be a bug. + assert "Authorization" not in request.headers + return httpx.Response(200, content=b"tarball-bytes") + + assert await _github(handler).archive("alice/widget", "main") == b"tarball-bytes" + + +async def test_github_check_probes_the_token_with_user(): + result = await _github(lambda r: _json(200, {"login": "octo"})).check() + assert result["ok"] is True + assert result["username"] == "octo" + + +async def test_latest_commit_parses_tolerantly_on_both_adapters(): + """The one caller treats latest_commit as an optimization with a fallback, + so a surprising payload must read as "don't know", never raise.""" + assert await _github( + lambda r: _json(200, [{"sha": "d" * 40}]) + ).latest_commit("a/w", "x.py") == "d" * 40 + assert await _github( + lambda r: _json(200, {"weird": True}) + ).latest_commit("a/w", "x.py") == "" + assert await _forge( + lambda r: _json(200, [{"sha": "e" * 40}]) + ).latest_commit("a/w", "x.py") == "e" * 40 + assert await _forge(lambda r: _json(200, [])).latest_commit("a/w", "x.py") == "" + + +async def test_full_config_builds_a_github_adapter(): + from scribe.services.forge import GitHubForge + + with _settings({ + "forge_kind": "github", + "forge_base_url": "https://github.com", + "forge_token": "tok", + }), patch("scribe.services.forge.Config") as cfg: + cfg.FORGE_KIND = cfg.FORGE_BASE_URL = cfg.FORGE_TOKEN = "" + forge = await get_forge() + assert isinstance(forge, GitHubForge) diff --git a/tests/test_snippet_live_body.py b/tests/test_snippet_live_body.py index 29ab898..ee11af8 100644 --- a/tests/test_snippet_live_body.py +++ b/tests/test_snippet_live_body.py @@ -104,6 +104,73 @@ async def test_current_with_same_stored_sha_skips_the_write(): update.assert_not_called() +async def test_stored_sha_short_circuit_skips_the_content_fetch(): + """#2693: when provenance already names a commit and the forge reports no + newer commit touching the path, the pull is confirmed current WITHOUT a + content transfer — the economy that fits pull-time freshness inside + GitHub's rate limits.""" + calls = [] + + def handler(request): + calls.append(request.url.path) + if request.url.path.endswith("/commits"): + return httpx.Response(200, json=[{"sha": SHA}]) + raise AssertionError("the content fetch should have been skipped") + + update = AsyncMock() + data = _data(provenance={"commit_sha": SHA, "fetched_at": "t"}) + with _patched(_forge_with(handler)), patch.object(svc.notes_svc, "update_note", update): + await svc.attach_live_body(_note(), data) + await background.drain() + assert data["body_source"] == "forge" + assert data["body_freshness"] == "current" + assert len(calls) == 1 + update.assert_not_called() # same stamp — nothing to persist + + +async def test_moved_file_falls_through_to_the_full_fetch(): + new_sha = "0" * 40 + + def handler(request): + if request.url.path.endswith("/commits"): + return httpx.Response(200, json=[{"sha": new_sha}]) + return _file_response("prefix\n" + CODE, commit_sha=new_sha) + + saved = {} + + async def fake_update(uid, nid, **fields): + saved.update(fields) + + data = _data(provenance={"commit_sha": SHA, "fetched_at": "t"}) + with _patched(_forge_with(handler)), patch.object( + svc.notes_svc, "update_note", fake_update + ): + await svc.attach_live_body(_note(), data) + await background.drain() + # The file moved but still contains the code — current, with the stamp + # advanced by the authoritative full fetch. + assert data["body_freshness"] == "current" + assert saved["data"]["provenance"]["commit_sha"] == new_sha + + +async def test_short_circuit_failure_degrades_to_the_full_fetch(): + """A forge whose commits endpoint errors must cost nothing: the full + fetch stays the authoritative path and the pull behaves as before.""" + + def handler(request): + if request.url.path.endswith("/commits"): + return httpx.Response(500) + return _file_response("prefix\n" + CODE) + + update = AsyncMock() + data = _data(provenance={"commit_sha": SHA, "fetched_at": "t"}) + with _patched(_forge_with(handler)), patch.object(svc.notes_svc, "update_note", update): + await svc.attach_live_body(_note(), data) + await background.drain() + assert data["body_freshness"] == "current" + update.assert_not_called() # same sha via the full fetch → same-sha skip + + async def test_diverged_reports_without_clobbering(): forge = _forge_with(lambda r: _file_response("def helper(x):\n return x - 1\n")) update = AsyncMock()