diff --git a/backend/app/services/git_providers/base.py b/backend/app/services/git_providers/base.py index 020802ad1..2aea4371b 100644 --- a/backend/app/services/git_providers/base.py +++ b/backend/app/services/git_providers/base.py @@ -12,9 +12,7 @@ class GitProviderBackend(ABC): @staticmethod def _blob_sha(content_bytes: bytes) -> str: """Compute the git blob SHA for content_bytes (sha1("blob {len}\\0" + data)).""" - return hashlib.sha1( - f"blob {len(content_bytes)}\0".encode() + content_bytes, usedforsecurity=False - ).hexdigest() + return hashlib.sha1(f"blob {len(content_bytes)}\0".encode() + content_bytes, usedforsecurity=False).hexdigest() @staticmethod def _truncated_response_text(response: httpx.Response, max_length: int = 200) -> str: @@ -24,6 +22,30 @@ class GitProviderBackend(ABC): return text return f"{text[: max_length - 3]}..." + @staticmethod + def _read_sha(response: httpx.Response, *path: str) -> tuple[str | None, str | None]: + """Walk a JSON path to a string SHA value. + + Returns ``(sha, None)`` on success, ``(None, reason)`` if the body is + not JSON, the path is missing, or the leaf is not a string. Callers + use the reason to build a clear failure message instead of letting + ``KeyError``/``JSONDecodeError`` bubble to the outer catch-all (which + surfaces cryptic one-word strings like ``"'object'"`` to operators). + """ + try: + data = response.json() + except ValueError: + return None, "non-JSON response body" + for key in path: + if not isinstance(data, dict): + return None, f"unexpected shape at key {key!r}" + if key not in data: + return None, f"missing key {key!r}" + data = data[key] + if not isinstance(data, str): + return None, f"value at {'.'.join(path)} is not a string" + return data, None + def get_headers(self, token: str) -> dict: """Return HTTP headers for authenticated API requests.""" return { diff --git a/backend/app/services/git_providers/forgejo.py b/backend/app/services/git_providers/forgejo.py index e9bd8cbbb..7e025ffab 100644 --- a/backend/app/services/git_providers/forgejo.py +++ b/backend/app/services/git_providers/forgejo.py @@ -1,11 +1,101 @@ -"""Forgejo backend — currently API-compatible with Gitea (/api/v1).""" +"""Forgejo backend — diverges from Gitea on token-scope validation (v15+).""" + +import logging + +import httpx from backend.app.services.git_providers.gitea import GiteaBackend +logger = logging.getLogger(__name__) + class ForgejoBackend(GiteaBackend): """Backend for Forgejo instances. - Currently API-compatible with Gitea (/api/v1). Override methods here - as the two projects' APIs diverge. + Forgejo v15+ returns 404 (not 403) for private repositories when the token + lacks repository scope, requiring a /user pre-check to distinguish bad tokens + from inaccessible repos. test_connection is overridden to handle this. + Other methods are inherited from GiteaBackend unchanged. """ + + async def test_connection(self, repo_url: str, token: str, client: httpx.AsyncClient) -> dict: + try: + owner, repo = self.parse_repo_url(repo_url) + api_base = self.get_api_base(repo_url) + headers = self.get_headers(token) + + # Verify token validity before hitting the repo. On Forgejo v15+, + # private repos return 404 (not 403) when the token lacks repo scope, + # so we must distinguish "bad token" from "token OK but repo not visible". + user_resp = await client.get(f"{api_base}/user", headers=headers) + if user_resp.status_code == 401: + return {"success": False, "message": "Invalid access token", "repo_name": None, "permissions": None} + if user_resp.status_code == 403: + return { + "success": False, + "message": "Token has no read:user scope; cannot validate identity", + "repo_name": None, + "permissions": None, + } + if user_resp.status_code != 200: + return { + "success": False, + "message": f"Forgejo API error on /user: {user_resp.status_code}", + "repo_name": None, + "permissions": None, + } + + repo_resp = await client.get(f"{api_base}/repos/{owner}/{repo}", headers=headers) + + if repo_resp.status_code == 404: + return { + "success": False, + "message": ( + "Repository not found or token cannot access it. " + "On Forgejo v15+, private repositories return 404 (not 403) " + "when the token lacks repository scope." + ), + "repo_name": None, + "permissions": None, + } + + if repo_resp.status_code != 200: + return { + "success": False, + "message": f"API error: {repo_resp.status_code}", + "repo_name": None, + "permissions": None, + } + + data = repo_resp.json() + permissions = data.get("permissions", {}) + + if not permissions.get("push", False): + return { + "success": False, + "message": "Token does not have push permission to this repository", + "repo_name": data.get("full_name"), + "permissions": permissions, + } + + return { + "success": True, + "message": "Connection successful", + "repo_name": data.get("full_name"), + "permissions": permissions, + } + + except Exception as e: + logger.exception("Forgejo connection test failed") + detail = str(e)[:200] + message = ( + f"Connection failed: {type(e).__name__}: {detail}" + if detail + else f"Connection failed: {type(e).__name__}" + ) + return { + "success": False, + "message": message, + "repo_name": None, + "permissions": None, + } diff --git a/backend/app/services/git_providers/gitea.py b/backend/app/services/git_providers/gitea.py index 45682dc4d..06e9dfda6 100644 --- a/backend/app/services/git_providers/gitea.py +++ b/backend/app/services/git_providers/gitea.py @@ -17,8 +17,8 @@ class GiteaBackend(GitHubBackend): """Backend for Gitea instances. Gitea's Git Data API (/api/v1/repos/{owner}/{repo}/git/...) is *mostly* - compatible with GitHub's, but diverges on two points that broke real-world - backups (#1224, #1225): + compatible with GitHub's, but diverges on three points that broke real-world + backups (#1224, #1225, #1239): 1. ``GET /git/refs/heads/{branch}`` returns a *list* of matching refs even when only one matches; GitHub returns a single object. The push paths @@ -29,6 +29,12 @@ class GiteaBackend(GitHubBackend): empty repository — every blob POST returns 404 until the repo has at least one commit. ``_create_initial_commit()`` is overridden to use the Contents API, which seeds the branch + initial commit in a single call. + + 3. The Git Data API does not support atomic multi-file commits — each file + requires a separate blob POST followed by a tree/commit/ref sequence. + ``push_files()`` is overridden to use the Contents API + (``POST /repos/.../contents`` with a ``files`` array), which commits all + changed files in a single round-trip and avoids partial-commit failures. """ @staticmethod @@ -45,10 +51,10 @@ class GiteaBackend(GitHubBackend): """Extract the tree SHA from a commit response. GitHub's ``GET /git/commits/{sha}`` returns the GitCommit schema with - ``tree`` at the top level. Gitea's same-named endpoint returns the + ``tree`` at the top level. Gitea's same-named endpoint may return the wrapped Commit schema where ``tree`` lives under ``commit``. Try the - flat shape first (GitHub-compatible deployments / Gitea ≤ 1.23) then - fall back to the wrapped shape (Gitea 1.24+, Forgejo). + flat shape first (GitHub-compatible deployments and some Gitea/Forgejo + versions) then fall back to the wrapped shape. """ tree_node = commit_data.get("tree") if not isinstance(tree_node, dict): @@ -94,6 +100,7 @@ class GiteaBackend(GitHubBackend): branch: str, files: dict, client: httpx.AsyncClient, + _allow_branch_create: bool = True, ) -> dict: """Push files via the Git Data API, normalising Gitea's list-shaped ref response.""" try: @@ -104,6 +111,14 @@ class GiteaBackend(GitHubBackend): ref_response = await client.get(f"{api_base}/repos/{owner}/{repo}/git/refs/heads/{branch}", headers=headers) if ref_response.status_code == 404: + if not _allow_branch_create: + return { + "status": "failed", + "message": ( + f"Branch '{branch}' not found after creation — possible replication lag. " + "The next scheduled backup will retry." + ), + } return await self._create_branch_and_push( client, headers, api_base, owner, repo, branch, files, repo_url, token ) @@ -121,93 +136,110 @@ class GiteaBackend(GitHubBackend): f"{api_base}/repos/{owner}/{repo}/git/commits/{current_commit_sha}", headers=headers ) if commit_response.status_code != 200: - return {"status": "failed", "message": "Failed to get current commit"} + msg = f"Failed to get current commit (HTTP {commit_response.status_code}): {self._truncated_response_text(commit_response)}" + logger.warning("push_files %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} current_tree_sha = self._commit_tree_sha(commit_response.json()) if not current_tree_sha: - return {"status": "failed", "message": "Failed to extract tree SHA from commit response"} + msg = ( + f"Failed to extract tree SHA from commit response: {self._truncated_response_text(commit_response)}" + ) + logger.warning("push_files %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} tree_response = await client.get( f"{api_base}/repos/{owner}/{repo}/git/trees/{current_tree_sha}?recursive=1", headers=headers ) + if tree_response.status_code != 200: + msg = f"Failed to list existing tree (HTTP {tree_response.status_code}): {self._truncated_response_text(tree_response)}" + logger.warning("push_files %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg, "error": self._truncated_response_text(tree_response)} + tree_data = tree_response.json() + # Gitea's tree API can report ``truncated: true`` for large + # listings; if we honour the partial map, the dedup check misses + # and every file gets re-uploaded each run. + if tree_data.get("truncated"): + msg = ( + "Repository tree exceeds the Gitea API listing limit (truncated=true). " + "Rotate the backup repository to avoid silent file-by-file churn on every backup." + ) + logger.warning("push_files %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} existing_files: dict[str, str] = {} - if tree_response.status_code == 200: - for item in tree_response.json().get("tree", []): - if item["type"] == "blob": - existing_files[item["path"]] = item["sha"] + for item in tree_data.get("tree", []): + if item.get("type") != "blob": + continue + path, sha = item.get("path"), item.get("sha") + if not path or not sha: + logger.warning("push_files: skipping malformed tree entry: %s", item) + continue + existing_files[path] = sha - tree_items = [] + api_files = [] files_changed = 0 for path, content in files.items(): content_str = json.dumps(content, indent=2, default=str) content_bytes = content_str.encode("utf-8") + content_b64 = base64.b64encode(content_bytes).decode() content_sha = self._blob_sha(content_bytes) - if path in existing_files and existing_files[path] == content_sha: - continue - - blob_response = await client.post( - f"{api_base}/repos/{owner}/{repo}/git/blobs", - headers=headers, - json={"content": base64.b64encode(content_bytes).decode(), "encoding": "base64"}, - ) - if blob_response.status_code != 201: - logger.error("Failed to create blob for %s: %s", path, self._truncated_response_text(blob_response)) - continue - - tree_items.append({"path": path, "mode": "100644", "type": "blob", "sha": blob_response.json()["sha"]}) + if path in existing_files: + if existing_files[path] == content_sha: + continue + api_files.append( + {"operation": "update", "path": path, "content": content_b64, "sha": existing_files[path]} + ) + else: + api_files.append({"operation": "create", "path": path, "content": content_b64}) files_changed += 1 - if not tree_items: + if not api_files: return {"status": "skipped", "message": "No changes to commit", "commit_sha": None, "files_changed": 0} - tree_response = await client.post( - f"{api_base}/repos/{owner}/{repo}/git/trees", - headers=headers, - json={"base_tree": current_tree_sha, "tree": tree_items}, - ) - if tree_response.status_code != 201: - return { - "status": "failed", - "message": f"Failed to create tree: {self._truncated_response_text(tree_response)}", - } - - new_tree_sha = tree_response.json()["sha"] commit_message = f"Bambuddy backup - {datetime.now(timezone.utc).strftime('%Y-%m-%d %H:%M:%S UTC')}" - commit_response = await client.post( - f"{api_base}/repos/{owner}/{repo}/git/commits", + response = await client.post( + f"{api_base}/repos/{owner}/{repo}/contents", headers=headers, - json={"message": commit_message, "tree": new_tree_sha, "parents": [current_commit_sha]}, + json={"branch": branch, "message": commit_message, "files": api_files}, ) - if commit_response.status_code != 201: + + if response.status_code == 404: return { "status": "failed", - "message": f"Failed to create commit: {self._truncated_response_text(commit_response)}", + "message": "Contents API endpoint not found — your Gitea instance may be older than v1.18 or the API may be disabled by an administrator (POST /contents returned 404)", } - - new_commit_sha = commit_response.json()["sha"] - - ref_update = await client.patch( - f"{api_base}/repos/{owner}/{repo}/git/refs/heads/{branch}", - headers=headers, - json={"sha": new_commit_sha}, - ) - if ref_update.status_code != 200: + if response.status_code == 409: return { "status": "failed", - "message": f"Failed to update branch: {self._truncated_response_text(ref_update)}", + "message": ( + "Conflict committing files — the branch likely advanced concurrently " + "(web-UI edit, another backup run, or path-vs-tree collision). " + "The next scheduled backup will re-read the current tree and resolve this." + ), + } + if response.status_code not in (200, 201): + return { + "status": "failed", + "message": f"Backup commit failed: {self._truncated_response_text(response)}", } + commit_sha = (response.json().get("commit") or {}).get("sha") + message = ( + f"Backup successful - {files_changed} files updated" + if commit_sha + else f"Backup successful - {files_changed} files updated (commit SHA not reported by server)" + ) return { "status": "success", - "message": f"Backup successful - {files_changed} files updated", - "commit_sha": new_commit_sha, + "message": message, + "commit_sha": commit_sha, "files_changed": files_changed, } except Exception as e: - logger.error("Push to Git failed: %s", e) + logger.exception("push_files failed for %s branch=%s", repo_url, branch) return {"status": "failed", "message": str(e), "error": str(e)} async def _create_branch_and_push( @@ -226,33 +258,44 @@ class GiteaBackend(GitHubBackend): try: repo_response = await client.get(f"{api_base}/repos/{owner}/{repo}", headers=headers) if repo_response.status_code != 200: - return {"status": "failed", "message": "Failed to get repo info"} + msg = f"Failed to get repo info (HTTP {repo_response.status_code}): {self._truncated_response_text(repo_response)}" + logger.warning("_create_branch_and_push %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} default_branch = repo_response.json().get("default_branch", "main") + # GET the default branch to confirm the repo is non-empty; SHA is intentionally unused — + # POST /branches takes a branch name, not a SHA. ref_response = await client.get( f"{api_base}/repos/{owner}/{repo}/git/refs/heads/{default_branch}", headers=headers ) if ref_response.status_code != 200: return await self._create_initial_commit(client, headers, api_base, owner, repo, branch, files) - base_sha = self._ref_sha(ref_response.json()) - create_ref = await client.post( - f"{api_base}/repos/{owner}/{repo}/git/refs", + f"{api_base}/repos/{owner}/{repo}/branches", headers=headers, - json={"ref": f"refs/heads/{branch}", "sha": base_sha}, + json={"new_branch_name": branch, "old_ref_name": default_branch}, ) + if create_ref.status_code == 403: + msg = f"Permission denied creating branch '{branch}' — token may lack write access to this repository" + logger.warning("_create_branch_and_push %s/%s: 403 %s", owner, repo, msg) + return {"status": "failed", "message": msg} + if create_ref.status_code == 409: + msg = f"Branch '{branch}' already exists (possible race condition)" + logger.warning("_create_branch_and_push %s/%s: 409 %s", owner, repo, msg) + return {"status": "failed", "message": msg} if create_ref.status_code != 201: - return { - "status": "failed", - "message": f"Failed to create branch: {self._truncated_response_text(create_ref)}", - } + msg = f"Failed to create branch '{branch}' (HTTP {create_ref.status_code}): {self._truncated_response_text(create_ref)}" + logger.warning("_create_branch_and_push %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} - return await self.push_files(repo_url, token, branch, files, client) + logger.info("Re-entering push_files after branch create %s/%s -> %s", owner, repo, branch) + return await self.push_files(repo_url, token, branch, files, client, _allow_branch_create=False) except Exception as e: - return {"status": "failed", "message": str(e)} + logger.exception("_create_branch_and_push failed for %s/%s branch=%s", owner, repo, branch) + return {"status": "failed", "message": str(e), "error": str(e)} async def _create_initial_commit( self, @@ -305,13 +348,18 @@ class GiteaBackend(GitHubBackend): data = response.json() commit_sha = (data.get("commit") or {}).get("sha") + message = ( + f"Initial backup created - {len(files)} files" + if commit_sha + else f"Initial backup created - {len(files)} files (commit SHA not reported by server)" + ) return { "status": "success", - "message": f"Initial backup created - {len(files)} files", + "message": message, "commit_sha": commit_sha, "files_changed": len(files), } except Exception as e: - logger.error("Gitea initial commit failed: %s", e) + logger.exception("_create_initial_commit failed for %s/%s branch=%s", owner, repo, branch) return {"status": "failed", "message": str(e), "error": str(e)} diff --git a/backend/app/services/git_providers/github.py b/backend/app/services/git_providers/github.py index a9b9ff5a1..302b5afc4 100644 --- a/backend/app/services/git_providers/github.py +++ b/backend/app/services/git_providers/github.py @@ -97,10 +97,16 @@ class GitHubBackend(GitProviderBackend): } except Exception as e: - logger.error("Git connection test failed: %s", e) + logger.exception("Git connection test failed") + detail = str(e)[:200] + message = ( + f"Connection failed: {type(e).__name__}: {detail}" + if detail + else f"Connection failed: {type(e).__name__}" + ) return { "success": False, - "message": f"Connection failed: {type(e).__name__}", + "message": message, "repo_name": None, "permissions": None, } @@ -112,6 +118,7 @@ class GitHubBackend(GitProviderBackend): branch: str, files: dict, client: httpx.AsyncClient, + _allow_branch_create: bool = True, ) -> dict: """Push files to the repository using the Git Data API.""" try: @@ -122,35 +129,72 @@ class GitHubBackend(GitProviderBackend): ref_response = await client.get(f"{api_base}/repos/{owner}/{repo}/git/refs/heads/{branch}", headers=headers) if ref_response.status_code == 404: + if not _allow_branch_create: + return { + "status": "failed", + "message": ( + f"Branch '{branch}' not found after creation — possible replication lag. " + "The next scheduled backup will retry." + ), + } return await self._create_branch_and_push( client, headers, api_base, owner, repo, branch, files, repo_url, token ) if ref_response.status_code != 200: - return { - "status": "failed", - "message": f"Failed to get branch ref: {ref_response.status_code}", - "error": self._truncated_response_text(ref_response), - } + msg = f"Failed to get branch ref (HTTP {ref_response.status_code}): {self._truncated_response_text(ref_response)}" + logger.warning("push_files %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg, "error": self._truncated_response_text(ref_response)} - current_commit_sha = ref_response.json()["object"]["sha"] + current_commit_sha, err = self._read_sha(ref_response, "object", "sha") + if err: + msg = f"Malformed ref response ({err}): {self._truncated_response_text(ref_response)}" + logger.warning("push_files %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} commit_response = await client.get( f"{api_base}/repos/{owner}/{repo}/git/commits/{current_commit_sha}", headers=headers ) if commit_response.status_code != 200: - return {"status": "failed", "message": "Failed to get current commit"} + msg = f"Failed to get current commit (HTTP {commit_response.status_code}): {self._truncated_response_text(commit_response)}" + logger.warning("push_files %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} - current_tree_sha = commit_response.json()["tree"]["sha"] + current_tree_sha, err = self._read_sha(commit_response, "tree", "sha") + if err: + msg = f"Malformed commit response ({err}): {self._truncated_response_text(commit_response)}" + logger.warning("push_files %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} tree_response = await client.get( f"{api_base}/repos/{owner}/{repo}/git/trees/{current_tree_sha}?recursive=1", headers=headers ) + if tree_response.status_code != 200: + msg = f"Failed to list existing tree (HTTP {tree_response.status_code}): {self._truncated_response_text(tree_response)}" + logger.warning("push_files %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg, "error": self._truncated_response_text(tree_response)} + tree_data = tree_response.json() + # GitHub's tree API truncates >7MB / >100k entries. A truncated tree + # listing makes the SHA-equality dedup miss and every file gets + # re-uploaded as a new blob each run — silent churn until someone + # notices the bloated history. Fail loudly so the user rotates the + # backup repo. + if tree_data.get("truncated"): + msg = ( + "Repository tree exceeds the GitHub API listing limit (truncated=true). " + "Rotate the backup repository to avoid silent file-by-file churn on every backup." + ) + logger.warning("push_files %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} existing_files: dict[str, str] = {} - if tree_response.status_code == 200: - for item in tree_response.json().get("tree", []): - if item["type"] == "blob": - existing_files[item["path"]] = item["sha"] + for item in tree_data.get("tree", []): + if item.get("type") != "blob": + continue + path, sha = item.get("path"), item.get("sha") + if not path or not sha: + logger.warning("push_files: skipping malformed tree entry: %s", item) + continue + existing_files[path] = sha tree_items = [] files_changed = 0 @@ -168,11 +212,21 @@ class GitHubBackend(GitProviderBackend): headers=headers, json={"content": base64.b64encode(content_bytes).decode(), "encoding": "base64"}, ) + if blob_response.status_code == 404: + msg = "GitHub API returned 404 for POST /git/blobs — check repository visibility and token scope" + logger.warning("push_files %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} if blob_response.status_code != 201: - logger.error("Failed to create blob for %s: %s", path, self._truncated_response_text(blob_response)) - continue + msg = f"Failed to create blob for {path} (HTTP {blob_response.status_code}): {self._truncated_response_text(blob_response)}" + logger.warning("push_files %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} - tree_items.append({"path": path, "mode": "100644", "type": "blob", "sha": blob_response.json()["sha"]}) + blob_sha, err = self._read_sha(blob_response, "sha") + if err: + msg = f"Malformed blob response for {path} ({err}): {self._truncated_response_text(blob_response)}" + logger.warning("push_files %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} + tree_items.append({"path": path, "mode": "100644", "type": "blob", "sha": blob_sha}) files_changed += 1 if not tree_items: @@ -184,12 +238,15 @@ class GitHubBackend(GitProviderBackend): json={"base_tree": current_tree_sha, "tree": tree_items}, ) if tree_response.status_code != 201: - return { - "status": "failed", - "message": f"Failed to create tree: {self._truncated_response_text(tree_response)}", - } + msg = f"Failed to create tree (HTTP {tree_response.status_code}): {self._truncated_response_text(tree_response)}" + logger.warning("push_files %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} - new_tree_sha = tree_response.json()["sha"] + new_tree_sha, err = self._read_sha(tree_response, "sha") + if err: + msg = f"Malformed tree-create response ({err}): {self._truncated_response_text(tree_response)}" + logger.warning("push_files %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} commit_message = f"Bambuddy backup - {datetime.now(timezone.utc).strftime('%Y-%m-%d %H:%M:%S UTC')}" commit_response = await client.post( f"{api_base}/repos/{owner}/{repo}/git/commits", @@ -197,12 +254,15 @@ class GitHubBackend(GitProviderBackend): json={"message": commit_message, "tree": new_tree_sha, "parents": [current_commit_sha]}, ) if commit_response.status_code != 201: - return { - "status": "failed", - "message": f"Failed to create commit: {self._truncated_response_text(commit_response)}", - } + msg = f"Failed to create commit (HTTP {commit_response.status_code}): {self._truncated_response_text(commit_response)}" + logger.warning("push_files %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} - new_commit_sha = commit_response.json()["sha"] + new_commit_sha, err = self._read_sha(commit_response, "sha") + if err: + msg = f"Malformed commit-create response ({err}): {self._truncated_response_text(commit_response)}" + logger.warning("push_files %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} ref_update = await client.patch( f"{api_base}/repos/{owner}/{repo}/git/refs/heads/{branch}", @@ -210,10 +270,9 @@ class GitHubBackend(GitProviderBackend): json={"sha": new_commit_sha}, ) if ref_update.status_code != 200: - return { - "status": "failed", - "message": f"Failed to update branch: {self._truncated_response_text(ref_update)}", - } + msg = f"Failed to update branch (HTTP {ref_update.status_code}): {self._truncated_response_text(ref_update)}" + logger.warning("push_files %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} return { "status": "success", @@ -223,7 +282,7 @@ class GitHubBackend(GitProviderBackend): } except Exception as e: - logger.error("Push to Git failed: %s", e) + logger.exception("push_files failed for %s branch=%s", repo_url, branch) return {"status": "failed", "message": str(e), "error": str(e)} async def _create_branch_and_push( @@ -242,9 +301,16 @@ class GitHubBackend(GitProviderBackend): try: repo_response = await client.get(f"{api_base}/repos/{owner}/{repo}", headers=headers) if repo_response.status_code != 200: - return {"status": "failed", "message": "Failed to get repo info"} + msg = f"Failed to get repo info (HTTP {repo_response.status_code}): {self._truncated_response_text(repo_response)}" + logger.warning("_create_branch_and_push %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} - default_branch = repo_response.json().get("default_branch", "main") + try: + default_branch = repo_response.json().get("default_branch", "main") + except ValueError: + msg = f"Malformed repo-info response (non-JSON body): {self._truncated_response_text(repo_response)}" + logger.warning("_create_branch_and_push %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} ref_response = await client.get( f"{api_base}/repos/{owner}/{repo}/git/refs/heads/{default_branch}", headers=headers @@ -252,7 +318,11 @@ class GitHubBackend(GitProviderBackend): if ref_response.status_code != 200: return await self._create_initial_commit(client, headers, api_base, owner, repo, branch, files) - base_sha = ref_response.json()["object"]["sha"] + base_sha, err = self._read_sha(ref_response, "object", "sha") + if err: + msg = f"Malformed default-branch ref response ({err}): {self._truncated_response_text(ref_response)}" + logger.warning("_create_branch_and_push %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} create_ref = await client.post( f"{api_base}/repos/{owner}/{repo}/git/refs", @@ -260,15 +330,16 @@ class GitHubBackend(GitProviderBackend): json={"ref": f"refs/heads/{branch}", "sha": base_sha}, ) if create_ref.status_code != 201: - return { - "status": "failed", - "message": f"Failed to create branch: {self._truncated_response_text(create_ref)}", - } + msg = f"Failed to create branch '{branch}' (HTTP {create_ref.status_code}): {self._truncated_response_text(create_ref)}" + logger.warning("_create_branch_and_push %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} - return await self.push_files(repo_url, token, branch, files, client) + logger.info("Re-entering push_files after branch create %s/%s -> %s", owner, repo, branch) + return await self.push_files(repo_url, token, branch, files, client, _allow_branch_create=False) except Exception as e: - return {"status": "failed", "message": str(e)} + logger.exception("_create_branch_and_push failed for %s/%s branch=%s", owner, repo, branch) + return {"status": "failed", "message": str(e), "error": str(e)} async def _create_initial_commit( self, @@ -290,10 +361,20 @@ class GitHubBackend(GitProviderBackend): headers=headers, json={"content": base64.b64encode(content_str.encode()).decode(), "encoding": "base64"}, ) - if blob_response.status_code == 201: - tree_items.append( - {"path": path, "mode": "100644", "type": "blob", "sha": blob_response.json()["sha"]} - ) + if blob_response.status_code == 404: + msg = "GitHub API returned 404 for POST /git/blobs — check repository visibility and token scope" + logger.warning("_create_initial_commit %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} + if blob_response.status_code != 201: + msg = f"Failed to create blob for {path} (HTTP {blob_response.status_code}): {self._truncated_response_text(blob_response)}" + logger.warning("_create_initial_commit %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} + blob_sha, err = self._read_sha(blob_response, "sha") + if err: + msg = f"Malformed blob response for {path} ({err}): {self._truncated_response_text(blob_response)}" + logger.warning("_create_initial_commit %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} + tree_items.append({"path": path, "mode": "100644", "type": "blob", "sha": blob_sha}) tree_response = await client.post( f"{api_base}/repos/{owner}/{repo}/git/trees", @@ -301,9 +382,15 @@ class GitHubBackend(GitProviderBackend): json={"tree": tree_items}, ) if tree_response.status_code != 201: - return {"status": "failed", "message": "Failed to create tree"} + msg = f"Failed to create tree (HTTP {tree_response.status_code}): {self._truncated_response_text(tree_response)}" + logger.warning("_create_initial_commit %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} - tree_sha = tree_response.json()["sha"] + tree_sha, err = self._read_sha(tree_response, "sha") + if err: + msg = f"Malformed tree-create response ({err}): {self._truncated_response_text(tree_response)}" + logger.warning("_create_initial_commit %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} commit_response = await client.post( f"{api_base}/repos/{owner}/{repo}/git/commits", headers=headers, @@ -313,16 +400,24 @@ class GitHubBackend(GitProviderBackend): }, ) if commit_response.status_code != 201: - return {"status": "failed", "message": "Failed to create commit"} + msg = f"Failed to create commit (HTTP {commit_response.status_code}): {self._truncated_response_text(commit_response)}" + logger.warning("_create_initial_commit %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} - commit_sha = commit_response.json()["sha"] + commit_sha, err = self._read_sha(commit_response, "sha") + if err: + msg = f"Malformed commit-create response ({err}): {self._truncated_response_text(commit_response)}" + logger.warning("_create_initial_commit %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} ref_response = await client.post( f"{api_base}/repos/{owner}/{repo}/git/refs", headers=headers, json={"ref": f"refs/heads/{branch}", "sha": commit_sha}, ) if ref_response.status_code != 201: - return {"status": "failed", "message": "Failed to create branch ref"} + msg = f"Failed to create branch ref (HTTP {ref_response.status_code}): {self._truncated_response_text(ref_response)}" + logger.warning("_create_initial_commit %s/%s: %s", owner, repo, msg) + return {"status": "failed", "message": msg} return { "status": "success", @@ -332,4 +427,5 @@ class GitHubBackend(GitProviderBackend): } except Exception as e: - return {"status": "failed", "message": str(e)} + logger.exception("_create_initial_commit failed for %s/%s branch=%s", owner, repo, branch) + return {"status": "failed", "message": str(e), "error": str(e)} diff --git a/backend/app/services/github_backup.py b/backend/app/services/github_backup.py index aed3383d9..d6e0b92a0 100644 --- a/backend/app/services/github_backup.py +++ b/backend/app/services/github_backup.py @@ -76,8 +76,8 @@ class GitHubBackupService: await self._check_scheduled_backups() except asyncio.CancelledError: break - except Exception as e: - logger.error("Error in GitHub backup scheduler: %s", e) + except Exception: + logger.exception("Error in GitHub backup scheduler") await asyncio.sleep(60) async def _check_scheduled_backups(self): @@ -202,7 +202,7 @@ class GitHubBackupService: } except Exception as e: - logger.error("Backup failed: %s", e) + logger.exception("Backup failed") log.status = "failed" log.completed_at = datetime.now(timezone.utc) log.error_message = str(e) @@ -381,12 +381,14 @@ class GitHubBackupService: } logger.info( - f"Collected cloud profiles: {len(filament_settings)} filament, " - f"{len(printer_settings)} printer, {len(process_settings)} process" + "Collected cloud profiles: %d filament, %d printer, %d process", + len(filament_settings), + len(printer_settings), + len(process_settings), ) - except Exception as e: - logger.warning("Failed to collect cloud profiles: %s", e) + except Exception: + logger.warning("Failed to collect cloud profiles", exc_info=True) finally: await cloud.close() diff --git a/backend/tests/integration/test_github_backup_api.py b/backend/tests/integration/test_github_backup_api.py index ba0672374..5ec9e96ed 100644 --- a/backend/tests/integration/test_github_backup_api.py +++ b/backend/tests/integration/test_github_backup_api.py @@ -164,9 +164,7 @@ class TestGitHubBackupConfigAPI: @pytest.mark.asyncio @pytest.mark.integration - async def test_update_config_rejects_disabling_insecure_http_for_stored_http_url( - self, async_client: AsyncClient - ): + async def test_update_config_rejects_disabling_insecure_http_for_stored_http_url(self, async_client: AsyncClient): """Verify PATCH rejects leaving a stored HTTP URL without explicit insecure-HTTP allowance.""" create_data = { "repository_url": "http://git.example.com/test/httprepo", diff --git a/backend/tests/unit/test_git_providers.py b/backend/tests/unit/test_git_providers.py index fa3ac48f8..bc50bae5a 100644 --- a/backend/tests/unit/test_git_providers.py +++ b/backend/tests/unit/test_git_providers.py @@ -82,6 +82,366 @@ class TestGitHubBackendApiBase: assert self.backend.get_api_base("git@github.example.com:owner/repo.git") == "https://github.example.com/api/v3" +class TestGitHubBackendPushFiles: + def setup_method(self): + self.backend = GitHubBackend() + self.repo_url = "https://github.com/owner/repo" + self.token = "ghp_token" + self.branch = "bambuddy-backup" + + @pytest.mark.asyncio + async def test_successful_push(self): + """Happy path: changed file goes through blob→tree→commit→ref-update.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, {"object": {"sha": "c1"}}), + _make_mock_response(200, {"tree": {"sha": "t1"}}), + _make_mock_response(200, {"tree": []}), + ] + ) + client.post = AsyncMock( + side_effect=[ + _make_mock_response(201, {"sha": "blob1"}), + _make_mock_response(201, {"sha": "new-tree"}), + _make_mock_response(201, {"sha": "new-commit"}), + ] + ) + client.patch = AsyncMock(return_value=_make_mock_response(200, {})) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) + + assert result["status"] == "success" + assert result["files_changed"] == 1 + + @pytest.mark.asyncio + async def test_skips_unchanged_files(self): + """File whose blob SHA matches the existing tree entry is excluded from the commit.""" + content = {"name": "my-printer"} + sha = _blob_sha(content) + + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, {"object": {"sha": "c1"}}), + _make_mock_response(200, {"tree": {"sha": "t1"}}), + _make_mock_response(200, {"tree": [{"type": "blob", "path": "config.json", "sha": sha}]}), + ] + ) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"config.json": content}, client) + + assert result["status"] == "skipped" + client.post.assert_not_called() + + @pytest.mark.asyncio + async def test_blob_failure_returns_failed_not_skipped(self): + """A non-201 blob response must return 'failed', not silently fall through to 'skipped'.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, {"object": {"sha": "c1"}}), + _make_mock_response(200, {"tree": {"sha": "t1"}}), + _make_mock_response(200, {"tree": []}), + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(500, {}, text="Internal Server Error")) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) + + assert result["status"] == "failed" + assert "failed" in result["message"].lower() + + @pytest.mark.asyncio + async def test_blob_404_surfaces_token_scope_hint(self): + """A 404 on POST /git/blobs surfaces a token scope/visibility hint, not 'skipped'.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, {"object": {"sha": "c1"}}), + _make_mock_response(200, {"tree": {"sha": "t1"}}), + _make_mock_response(200, {"tree": []}), + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(404, {})) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) + + assert result["status"] == "failed" + assert "404" in result["message"] + assert "token scope" in result["message"].lower() + + @pytest.mark.asyncio + async def test_initial_commit_blob_404_surfaces_token_scope_hint(self): + """Empty repo path: 404 on POST /git/blobs surfaces token scope hint.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(404, {}), # backup branch missing + _make_mock_response(200, {"default_branch": "main"}), # repo info + _make_mock_response(404, {}), # default branch missing -> empty repo + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(404, {})) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) + + assert result["status"] == "failed" + assert "404" in result["message"] + assert "token scope" in result["message"].lower() + + @pytest.mark.asyncio + async def test_initial_commit_blob_non_201_returns_path_in_message(self): + """Empty repo path: non-201 on POST /git/blobs includes the file path in the failure message.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(404, {}), # backup branch missing + _make_mock_response(200, {"default_branch": "main"}), # repo info + _make_mock_response(404, {}), # default branch missing -> empty repo + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(500, {}, text="Internal Server Error")) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) + + assert result["status"] == "failed" + assert "a.json" in result["message"] + + +class TestGitHubBackendRobustness: + """Coverage for the B18-B26 PR feedback round: GitHub backend. + + Targets failure paths that previously silent-failed or surfaced cryptic + one-word strings to operators (KeyError on missing JSON keys, etc.). + """ + + def setup_method(self): + self.backend = GitHubBackend() + self.repo_url = "https://github.com/owner/repo" + self.token = "ghp_token" + self.branch = "bambuddy-backup" + + @pytest.mark.asyncio + async def test_tree_fetch_failure_returns_failed_not_silent_skip(self): + """B18: A non-200 tree GET must surface a clear failure with status code, not let + the downstream blob POSTs fire with an empty existing_files map.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, {"object": {"sha": "c1"}}), + _make_mock_response(200, {"tree": {"sha": "t1"}}), + _make_mock_response(500, {}, text="Internal Server Error"), + ] + ) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) + + assert result["status"] == "failed" + assert "existing tree" in result["message"] + assert "500" in result["message"] + client.post.assert_not_called() + + @pytest.mark.asyncio + async def test_truncated_tree_response_returns_failed(self): + """B24: GitHub's tree API truncates >7MB / >100k entries. A truncated map would + miss SHAs and re-upload every file as new on each backup — fail loudly instead.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, {"object": {"sha": "c1"}}), + _make_mock_response(200, {"tree": {"sha": "t1"}}), + _make_mock_response( + 200, {"tree": [{"type": "blob", "path": "a.json", "sha": "old"}], "truncated": True} + ), + ] + ) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) + + assert result["status"] == "failed" + assert "truncated" in result["message"].lower() + assert "rotate the backup repository" in result["message"].lower() + client.post.assert_not_called() + + @pytest.mark.asyncio + async def test_malformed_ref_response_returns_clear_message(self): + """B20: An unexpected ref body (no object.sha) surfaces a clear shape-error, + not 'object' as the user-facing message via the catch-all.""" + client = AsyncMock() + client.get = AsyncMock(return_value=_make_mock_response(200, {"unexpected": "shape"})) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) + + assert result["status"] == "failed" + assert "ref response" in result["message"].lower() + assert "missing key 'object'" in result["message"] + + @pytest.mark.asyncio + async def test_malformed_commit_response_returns_clear_message(self): + """B20: An unexpected commit body (no tree.sha) surfaces a clear shape-error, + not 'tree' as the user-facing message via the catch-all.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, {"object": {"sha": "c1"}}), + _make_mock_response(200, {"sha": "c1"}), # no top-level tree + ] + ) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) + + assert result["status"] == "failed" + assert "commit response" in result["message"].lower() + assert "missing key 'tree'" in result["message"] + + @pytest.mark.asyncio + async def test_malformed_blob_response_returns_clear_message(self): + """B20: A 201 blob response with no sha field surfaces shape-error, not KeyError.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, {"object": {"sha": "c1"}}), + _make_mock_response(200, {"tree": {"sha": "t1"}}), + _make_mock_response(200, {"tree": []}), + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(201, {"unexpected": "shape"})) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) + + assert result["status"] == "failed" + assert "blob response" in result["message"].lower() + assert "a.json" in result["message"] + + @pytest.mark.asyncio + async def test_create_branch_403_includes_status_code(self): + """B19: A 403 on POST /git/refs surfaces the HTTP status code so the operator can + tell 'no write scope' apart from generic upstream errors.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(404, {}), # backup branch missing + _make_mock_response(200, {"default_branch": "main"}), # repo info + _make_mock_response(200, {"object": {"sha": "main-sha"}}), # default branch ref + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(403, {"message": "Forbidden"})) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {}}, client) + + assert result["status"] == "failed" + assert "403" in result["message"] + assert "branch" in result["message"].lower() + + @pytest.mark.asyncio + async def test_create_branch_422_includes_status_code(self): + """B19: 422 with empty body must still produce a diagnostic message — the previous + assertion accepted either the status code OR a body substring, masking this gap.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(404, {}), # backup branch missing + _make_mock_response(200, {"default_branch": "main"}), # repo info + _make_mock_response(200, {"object": {"sha": "main-sha"}}), # default branch ref + ] + ) + # 422 with empty body — the assertion must rely on the status code, not the body + client.post = AsyncMock(return_value=_make_mock_response(422, {}, text="")) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {}}, client) + + assert result["status"] == "failed" + assert "422" in result["message"] + + @pytest.mark.asyncio + async def test_test_connection_failure_includes_exception_message(self): + """B23: A network exception during test_connection surfaces both the class name and + the message (truncated), so the user clicking Test Connection sees actionable text + like 'certificate verify failed', not just 'ConnectError'.""" + import httpx + + client = AsyncMock() + client.get = AsyncMock( + side_effect=httpx.ConnectError("certificate verify failed: unable to get local issuer certificate") + ) + + result = await self.backend.test_connection(self.repo_url, self.token, client) + + assert result["success"] is False + assert "ConnectError" in result["message"] + assert "certificate verify failed" in result["message"] + + @pytest.mark.asyncio + async def test_test_connection_truncates_long_exception_message(self): + """B23: The user-facing exception detail is bounded to 200 chars.""" + import httpx + + long_message = "x" * 500 + client = AsyncMock() + client.get = AsyncMock(side_effect=httpx.ConnectError(long_message)) + + result = await self.backend.test_connection(self.repo_url, self.token, client) + + assert result["success"] is False + # Total message ≈ "Connection failed: ConnectError: " (33) + 200 char detail + assert len(result["message"]) < 300 + + @pytest.mark.asyncio + async def test_initial_commit_malformed_blob_response(self): + """B20: _create_initial_commit: 201 blob response with no sha surfaces shape-error.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(404, {}), # backup branch missing + _make_mock_response(200, {"default_branch": "main"}), + _make_mock_response(404, {}), # default branch missing -> empty repo + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(201, {"unexpected": "shape"})) + + result = await self.backend.push_files( + self.repo_url, self.token, self.branch, {"seed.json": {"k": "v"}}, client + ) + + assert result["status"] == "failed" + assert "blob response" in result["message"].lower() + assert "seed.json" in result["message"] + + @pytest.mark.asyncio + async def test_recursive_push_files_log_marker_on_branch_create(self, caplog): + """B26: After a successful branch create, the re-entry into push_files emits an + info-level marker so operators can correlate the second pass with the first.""" + import logging + + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(404, {}), # first push: branch missing + _make_mock_response(200, {"default_branch": "main"}), # repo info + _make_mock_response(200, {"object": {"sha": "main-sha"}}), # default branch ref + _make_mock_response(200, {"object": {"sha": "c1"}}), # second push: branch ref + _make_mock_response(200, {"tree": {"sha": "t1"}}), # commit + _make_mock_response(200, {"tree": []}), # tree listing + ] + ) + client.post = AsyncMock( + side_effect=[ + _make_mock_response(201, {}), # POST /git/refs (create branch) + _make_mock_response(201, {"sha": "blob1"}), + _make_mock_response(201, {"sha": "new-tree"}), + _make_mock_response(201, {"sha": "new-commit"}), + ] + ) + client.patch = AsyncMock(return_value=_make_mock_response(200, {})) + + with caplog.at_level(logging.INFO, logger="backend.app.services.git_providers.github"): + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {}}, client) + + assert result["status"] == "success" + assert any("Re-entering push_files" in r.message for r in caplog.records) + + class TestGiteaBackendApiBase: def setup_method(self): self.backend = GiteaBackend() @@ -113,7 +473,7 @@ class TestGiteaBackendPushFiles: @pytest.mark.asyncio async def test_n_files_produce_single_commit(self): - """All changed files are bundled into one commit via the Git Data API.""" + """All changed files are bundled into one Contents API call.""" files = {"a.json": {"k": "v1"}, "b.json": {"k": "v2"}} client = AsyncMock() client.get = AsyncMock( @@ -123,26 +483,18 @@ class TestGiteaBackendPushFiles: _make_mock_response(200, {"tree": []}), ] ) - client.post = AsyncMock( - side_effect=[ - _make_mock_response(201, {"sha": "blob1"}), - _make_mock_response(201, {"sha": "blob2"}), - _make_mock_response(201, {"sha": "new-tree"}), - _make_mock_response(201, {"sha": "new-commit"}), - ] - ) - client.patch = AsyncMock(return_value=_make_mock_response(200, {})) + client.post = AsyncMock(return_value=_make_mock_response(201, {"commit": {"sha": "new-commit"}})) result = await self.backend.push_files(self.repo_url, self.token, self.branch, files, client) assert result["status"] == "success" assert result["files_changed"] == 2 - commit_calls = [c for c in client.post.call_args_list if "/git/commits" in c.args[0]] - assert len(commit_calls) == 1 + contents_calls = [c for c in client.post.call_args_list if "/contents" in c.args[0]] + assert len(contents_calls) == 1 @pytest.mark.asyncio async def test_uses_gitea_api_v1_base_not_github(self): - """Git Data API calls target the instance's /api/v1, not api.github.com.""" + """Contents API calls target the instance's /api/v1, not api.github.com.""" client = AsyncMock() client.get = AsyncMock( side_effect=[ @@ -151,14 +503,7 @@ class TestGiteaBackendPushFiles: _make_mock_response(200, {"tree": []}), ] ) - client.post = AsyncMock( - side_effect=[ - _make_mock_response(201, {"sha": "blob1"}), - _make_mock_response(201, {"sha": "new-tree"}), - _make_mock_response(201, {"sha": "new-commit"}), - ] - ) - client.patch = AsyncMock(return_value=_make_mock_response(200, {})) + client.post = AsyncMock(return_value=_make_mock_response(201, {"commit": {"sha": "new-commit"}})) await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) @@ -189,8 +534,371 @@ class TestGiteaBackendPushFiles: client.post.assert_not_called() @pytest.mark.asyncio - async def test_creates_missing_branch_via_git_refs_api(self): - """A missing backup branch is created via the Git Data API refs endpoint.""" + async def test_changed_file_sent_as_update_with_sha(self): + """A file whose content changed is sent with operation='update' and the current blob SHA.""" + old_content = {"version": "1.0", "archives": []} + new_content = {"version": "1.0", "archives": [{"id": 1}]} + old_sha = _blob_sha(old_content) + + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, [{"object": {"sha": "c1"}}]), + _make_mock_response(200, {"tree": {"sha": "t1"}}), + _make_mock_response( + 200, {"tree": [{"type": "blob", "path": "archives/print_history.json", "sha": old_sha}]} + ), + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(201, {"commit": {"sha": "new-sha"}})) + + result = await self.backend.push_files( + self.repo_url, + self.token, + self.branch, + {"archives/print_history.json": new_content}, + client, + ) + + assert result["status"] == "success" + assert result["files_changed"] == 1 + body = client.post.call_args.kwargs["json"] + assert body["files"][0]["operation"] == "update" + assert body["files"][0]["sha"] == old_sha + + @pytest.mark.asyncio + async def test_new_file_sent_as_create_without_sha(self): + """A file not yet in the repo is sent with operation='create' and no sha field.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, [{"object": {"sha": "c1"}}]), + _make_mock_response(200, {"tree": {"sha": "t1"}}), + _make_mock_response(200, {"tree": []}), + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(201, {"commit": {"sha": "new-sha"}})) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"new.json": {"k": "v"}}, client) + + assert result["status"] == "success" + body = client.post.call_args.kwargs["json"] + assert body["files"][0]["operation"] == "create" + assert "sha" not in body["files"][0] + + @pytest.mark.asyncio + async def test_unchanged_file_excluded_from_contents_call(self): + """A file whose blob SHA matches the existing tree entry is not included in the Contents API call.""" + content = {"name": "printer-1"} + sha = _blob_sha(content) + + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, [{"object": {"sha": "c1"}}]), + _make_mock_response(200, {"tree": {"sha": "t1"}}), + _make_mock_response(200, {"tree": [{"type": "blob", "path": "config.json", "sha": sha}]}), + ] + ) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"config.json": content}, client) + + assert result["status"] == "skipped" + client.post.assert_not_called() + + @pytest.mark.asyncio + async def test_mixed_batch_create_update_unchanged_in_single_call(self): + """A batch with one new file, one changed file, and one unchanged file produces + exactly one create + one update in the Contents API payload, files_changed=2.""" + unchanged_content = {"version": 1} + unchanged_sha = _blob_sha(unchanged_content) + old_content = {"version": 1} + old_sha = _blob_sha(old_content) + + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, [{"object": {"sha": "c1"}}]), + _make_mock_response(200, {"tree": {"sha": "t1"}}), + _make_mock_response( + 200, + { + "tree": [ + {"type": "blob", "path": "unchanged.json", "sha": unchanged_sha}, + {"type": "blob", "path": "updated.json", "sha": old_sha}, + ] + }, + ), + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(201, {"commit": {"sha": "new-sha"}})) + + result = await self.backend.push_files( + self.repo_url, + self.token, + self.branch, + { + "unchanged.json": unchanged_content, # same SHA — should be skipped + "updated.json": {"version": 2}, # changed — should be update + "new.json": {"created": True}, # new — should be create + }, + client, + ) + + assert result["status"] == "success" + assert result["files_changed"] == 2 + body = client.post.call_args.kwargs["json"] + ops = {f["path"]: f for f in body["files"]} + assert set(ops.keys()) == {"updated.json", "new.json"} + assert ops["updated.json"]["operation"] == "update" + assert ops["updated.json"]["sha"] == old_sha + assert ops["new.json"]["operation"] == "create" + assert "sha" not in ops["new.json"] + + @pytest.mark.asyncio + async def test_tree_fetch_failure_returns_failed(self): + """A non-200 tree GET surfaces a clear failure, not a downstream 422 from the Contents API.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, [{"object": {"sha": "c1"}}]), + _make_mock_response(200, {"tree": {"sha": "t1"}}), + _make_mock_response(500, {}, text="Internal Server Error"), + ] + ) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) + + assert result["status"] == "failed" + assert "existing tree" in result["message"] + assert "500" in result["message"] + client.post.assert_not_called() + + @pytest.mark.asyncio + async def test_contents_api_failure_returns_failed(self): + """A non-2xx response from the Contents API returns status='failed', not 'skipped'.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, [{"object": {"sha": "c1"}}]), + _make_mock_response(200, {"tree": {"sha": "t1"}}), + _make_mock_response(200, {"tree": []}), + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(403, {}, text="Forbidden")) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) + + assert result["status"] == "failed" + assert "failed" in result["message"].lower() + + @pytest.mark.asyncio + async def test_contents_api_404_surfaces_version_hint(self): + """A 404 from POST /contents surfaces a Gitea version hint, not a generic 'Not Found'.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, [{"object": {"sha": "c1"}}]), + _make_mock_response(200, {"tree": {"sha": "t1"}}), + _make_mock_response(200, {"tree": []}), + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(404, {})) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) + + assert result["status"] == "failed" + assert "v1.18" in result["message"] + assert "404" in result["message"] + + @pytest.mark.asyncio + async def test_contents_api_409_surfaces_conflict_hint(self): + """409 on POST /contents surfaces a conflict hint (covers web-UI edit, concurrent backup, path collision).""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, [{"object": {"sha": "c1"}}]), + _make_mock_response(200, {"tree": {"sha": "t1"}}), + _make_mock_response(200, {"tree": []}), + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(409, {})) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) + + assert result["status"] == "failed" + assert "conflict" in result["message"].lower() + assert "advanced concurrently" in result["message"] + assert "next scheduled backup" in result["message"] + + @pytest.mark.asyncio + async def test_replication_lag_guard_fires_on_second_404(self): + """Branch 404 after successful creation (replication lag) returns a clear message, not infinite recursion.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(404, {}), # first push_files: branch missing + _make_mock_response(200, {"default_branch": "main"}), # repo info + _make_mock_response(200, [{"object": {"sha": "main-sha"}}]), # default branch ref + _make_mock_response(404, {}), # second push_files: branch still missing (lag) + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(201, {})) # POST /branches succeeds + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {}}, client) + + assert result["status"] == "failed" + assert "replication lag" in result["message"] + assert "next scheduled backup" in result["message"] + + @pytest.mark.asyncio + async def test_malformed_tree_entry_skipped_valid_entry_retained(self): + """A tree entry missing 'sha' is skipped; the valid entry is still compared for deduplication.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, [{"object": {"sha": "abc"}}]), # branch ref + _make_mock_response(200, {"tree": {"sha": "tree-sha"}}), # commit + _make_mock_response( + 200, + { + "tree": [ # tree listing + {"type": "blob", "path": "valid.json", "sha": "old-sha"}, + {"type": "blob", "path": "broken.json"}, # missing sha + ] + }, + ), + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(201, {"commit": {"sha": "new-commit"}})) + + result = await self.backend.push_files( + self.repo_url, + self.token, + self.branch, + {"valid.json": {"changed": True}, "broken.json": {"x": 1}}, + client, + ) + + assert result["status"] == "success" + assert result["files_changed"] == 2 # both files pushed (broken.json treated as new) + + @pytest.mark.asyncio + async def test_truncated_tree_response_returns_failed(self): + """B24: A truncated tree listing makes SHA-equality dedup miss; surface a failure + asking the user to rotate the repo rather than silently re-uploading every file.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, [{"object": {"sha": "c1"}}]), + _make_mock_response(200, {"tree": {"sha": "t1"}}), + _make_mock_response( + 200, {"tree": [{"type": "blob", "path": "a.json", "sha": "old"}], "truncated": True} + ), + ] + ) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) + + assert result["status"] == "failed" + assert "truncated" in result["message"].lower() + assert "rotate the backup repository" in result["message"].lower() + client.post.assert_not_called() + + @pytest.mark.asyncio + async def test_get_current_commit_failure_includes_status_and_body(self): + """B22: A 5xx on GET /git/commits surfaces both the status code and the body, + not the bare 'Failed to get current commit' string.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, [{"object": {"sha": "c1"}}]), + _make_mock_response(503, {}, text="Service Unavailable"), + ] + ) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) + + assert result["status"] == "failed" + assert "503" in result["message"] + assert "current commit" in result["message"].lower() + + @pytest.mark.asyncio + async def test_missing_tree_sha_surfaces_body(self): + """B22: 'Failed to extract tree SHA' now includes the (truncated) response body so + a future Gitea shape-shift is debuggable from the failure message alone.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, [{"object": {"sha": "c1"}}]), + # Neither flat .tree nor wrapped .commit.tree present + _make_mock_response( + 200, + {"sha": "c1", "url": "https://gitea.example.com/api/v1/.../c1"}, + text='{"sha":"c1","url":"https://gitea.example.com/api/v1/.../c1"}', + ), + ] + ) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) + + assert result["status"] == "failed" + assert "tree SHA" in result["message"] + assert "gitea.example.com" in result["message"] # body context included + + @pytest.mark.asyncio + async def test_repo_info_failure_includes_status_and_body(self): + """B22: 'Failed to get repo info' inside _create_branch_and_push now includes + the status code and response body.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(404, {}), # backup branch missing -> branch-and-push path + _make_mock_response(500, {}, text="Internal Server Error"), # repo info + ] + ) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {}}, client) + + assert result["status"] == "failed" + assert "repo info" in result["message"].lower() + assert "500" in result["message"] + assert "Internal Server Error" in result["message"] + + @pytest.mark.asyncio + async def test_recursive_push_files_log_marker_on_branch_create(self, caplog): + """B26: After POST /branches succeeds, the re-entry into push_files emits an + info-level marker so operators can debug second-pass failures (e.g. replication lag).""" + import logging + + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(404, {}), # first push: branch missing + _make_mock_response(200, {"default_branch": "main"}), # repo info + _make_mock_response(200, [{"object": {"sha": "main-sha"}}]), # default branch ref + # second push pass: + _make_mock_response(200, [{"object": {"sha": "c1"}}]), + _make_mock_response(200, {"tree": {"sha": "t1"}}), + _make_mock_response(200, {"tree": []}), + ] + ) + client.post = AsyncMock( + side_effect=[ + _make_mock_response(201, {}), # POST /branches + _make_mock_response(201, {"commit": {"sha": "new-commit"}}), # POST /contents + ] + ) + + with caplog.at_level(logging.INFO, logger="backend.app.services.git_providers.gitea"): + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {}}, client) + + assert result["status"] == "success" + assert any("Re-entering push_files" in r.message for r in caplog.records) + + @pytest.mark.asyncio + async def test_creates_missing_branch_via_branches_api(self): + """A missing backup branch is created via POST /branches, not /git/refs.""" client = AsyncMock() client.get = AsyncMock( side_effect=[ @@ -208,20 +916,18 @@ class TestGiteaBackendPushFiles: ) client.post = AsyncMock( side_effect=[ - _make_mock_response(201, {}), # create ref - _make_mock_response(201, {"sha": "blob1"}), - _make_mock_response(201, {"sha": "new-tree"}), - _make_mock_response(201, {"sha": "new-commit"}), + _make_mock_response(201, {}), # POST /branches + _make_mock_response(201, {"commit": {"sha": "new-commit"}}), # POST /contents ] ) - client.patch = AsyncMock(return_value=_make_mock_response(200, {})) result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) assert result["status"] == "success" - ref_create_call = client.post.call_args_list[0] - assert "/git/refs" in ref_create_call.args[0] - assert ref_create_call.kwargs["json"]["ref"] == f"refs/heads/{self.branch}" + branch_call = client.post.call_args_list[0] + assert "/branches" in branch_call.args[0] + assert "/git/refs" not in branch_call.args[0] + assert branch_call.kwargs["json"]["new_branch_name"] == self.branch @pytest.mark.asyncio async def test_truncates_upstream_error_body_in_failure_message(self): @@ -233,17 +939,12 @@ class TestGiteaBackendPushFiles: _make_mock_response(200, {"tree": []}), ] ) - client.post = AsyncMock( - side_effect=[ - _make_mock_response(201, {"sha": "blob1"}), - _make_mock_response(500, {}, text="x" * 500), - ] - ) + client.post = AsyncMock(return_value=_make_mock_response(500, {}, text="x" * 500)) result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) assert result["status"] == "failed" - assert result["message"] == f"Failed to create tree: {'x' * 197}..." + assert result["message"] == f"Backup commit failed: {'x' * 197}..." class TestGiteaBackendListShapeRefResponse: @@ -285,14 +986,7 @@ class TestGiteaBackendListShapeRefResponse: _make_mock_response(200, {"tree": []}), ] ) - client.post = AsyncMock( - side_effect=[ - _make_mock_response(201, {"sha": "blob1"}), - _make_mock_response(201, {"sha": "new-tree"}), - _make_mock_response(201, {"sha": "new-commit"}), - ] - ) - client.patch = AsyncMock(return_value=_make_mock_response(200, {})) + client.post = AsyncMock(return_value=_make_mock_response(201, {"commit": {"sha": "new-commit"}})) result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) @@ -316,18 +1010,68 @@ class TestGiteaBackendListShapeRefResponse: ) client.post = AsyncMock( side_effect=[ - _make_mock_response(201, {}), # create branch ref - _make_mock_response(201, {"sha": "blob1"}), - _make_mock_response(201, {"sha": "new-tree"}), - _make_mock_response(201, {"sha": "new-commit"}), + _make_mock_response(201, {}), # POST /branches + _make_mock_response(201, {"commit": {"sha": "new-commit"}}), # POST /contents ] ) - client.patch = AsyncMock(return_value=_make_mock_response(200, {})) result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) assert result["status"] == "success" + @pytest.mark.asyncio + async def test_create_branch_403_returns_permission_message(self): + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(404, {}), # missing backup branch + _make_mock_response(200, {"default_branch": "main"}), # repo info + _make_mock_response(200, [{"object": {"sha": "main-sha"}}]), # default branch ref + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(403, {"message": "Forbidden"})) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {}}, client) + + assert result["status"] == "failed" + assert "Permission denied" in result["message"] + assert "write access" in result["message"] + + @pytest.mark.asyncio + async def test_create_branch_409_returns_race_condition_message(self): + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(404, {}), # missing backup branch + _make_mock_response(200, {"default_branch": "main"}), # repo info + _make_mock_response(200, [{"object": {"sha": "main-sha"}}]), # default branch ref + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(409, {"message": "Conflict"})) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {}}, client) + + assert result["status"] == "failed" + assert "already exists" in result["message"] + assert "race" in result["message"] + + @pytest.mark.asyncio + async def test_create_branch_unexpected_status_includes_code_in_message(self): + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(404, {}), # missing backup branch + _make_mock_response(200, {"default_branch": "main"}), # repo info + _make_mock_response(200, [{"object": {"sha": "main-sha"}}]), # default branch ref + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(422, {"message": "Unprocessable"})) + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {}}, client) + + assert result["status"] == "failed" + assert "422" in result["message"] + class TestGiteaBackendWrappedCommitResponse: """#1224 regression: Gitea wraps the GitCommit fields under ``commit``. @@ -371,20 +1115,32 @@ class TestGiteaBackendWrappedCommitResponse: _make_mock_response(200, {"tree": []}), ] ) - client.post = AsyncMock( - side_effect=[ - _make_mock_response(201, {"sha": "blob1"}), - _make_mock_response(201, {"sha": "new-tree"}), - _make_mock_response(201, {"sha": "new-commit"}), - ] - ) - client.patch = AsyncMock(return_value=_make_mock_response(200, {})) + client.post = AsyncMock(return_value=_make_mock_response(201, {"commit": {"sha": "new-commit"}})) result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) assert result["status"] == "success" assert result["commit_sha"] == "new-commit" + @pytest.mark.asyncio + async def test_missing_commit_sha_in_push_response_surfaces_warning(self): + """200/201 with no commit.sha -> success with a human-readable note, not silent None.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, [{"object": {"sha": "base-commit"}}]), + _make_mock_response(200, {"sha": "base-commit", "commit": {"tree": {"sha": "base-tree"}}}), + _make_mock_response(200, {"tree": []}), + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(201, {})) # no commit key + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {"k": "v"}}, client) + + assert result["status"] == "success" + assert result["commit_sha"] is None + assert "not reported" in result["message"] + @pytest.mark.asyncio async def test_push_files_fails_cleanly_when_tree_sha_missing(self): """Defensive: malformed/unexpected commit response surfaces a clear error, not KeyError.""" @@ -509,6 +1265,25 @@ class TestGiteaBackendEmptyRepoInitialCommit: assert result["files_changed"] == 0 client.post.assert_not_called() + @pytest.mark.asyncio + async def test_missing_commit_sha_in_initial_commit_response_surfaces_warning(self): + """200/201 with no commit.sha -> success with a human-readable note, not silent None.""" + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(404, {}), + _make_mock_response(200, {"default_branch": "main"}), + _make_mock_response(404, {}), + ] + ) + client.post = AsyncMock(return_value=_make_mock_response(201, {})) # no commit key + + result = await self.backend.push_files(self.repo_url, self.token, self.branch, {"a.json": {}}, client) + + assert result["status"] == "success" + assert result["commit_sha"] is None + assert "not reported" in result["message"] + class TestForgejoInheritsGiteaFixes: """ForgejoBackend extends GiteaBackend with no overrides — must inherit both fixes.""" @@ -524,14 +1299,7 @@ class TestForgejoInheritsGiteaFixes: _make_mock_response(200, {"tree": []}), ] ) - client.post = AsyncMock( - side_effect=[ - _make_mock_response(201, {"sha": "blob1"}), - _make_mock_response(201, {"sha": "new-tree"}), - _make_mock_response(201, {"sha": "new-commit"}), - ] - ) - client.patch = AsyncMock(return_value=_make_mock_response(200, {})) + client.post = AsyncMock(return_value=_make_mock_response(201, {"commit": {"sha": "new-commit"}})) result = await backend.push_files( "https://forgejo.example.com/owner/repo", @@ -591,6 +1359,129 @@ class TestForgejoBackendApiBase: assert repo == "repo" +class TestForgejoTestConnection: + """ForgejoBackend overrides test_connection to handle Forgejo v15+ 404-not-403 behaviour.""" + + def setup_method(self): + self.backend = ForgejoBackend() + self.repo_url = "https://forgejo.example.com/owner/repo" + self.token = "fj-token" + + @pytest.mark.asyncio + async def test_valid_token_and_push_permission_returns_success(self): + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, {"login": "user"}), + _make_mock_response(200, {"full_name": "owner/repo", "permissions": {"push": True, "pull": True}}), + ] + ) + + result = await self.backend.test_connection(self.repo_url, self.token, client) + + assert result["success"] is True + assert result["repo_name"] == "owner/repo" + + @pytest.mark.asyncio + async def test_invalid_token_returns_clear_message_without_repo_call(self): + client = AsyncMock() + client.get = AsyncMock(return_value=_make_mock_response(401, {})) + + result = await self.backend.test_connection(self.repo_url, self.token, client) + + assert result["success"] is False + assert result["message"] == "Invalid access token" + assert client.get.call_count == 1 # only /user was called + + @pytest.mark.asyncio + async def test_zero_scope_token_403_on_user_returns_scope_hint(self): + """A 403 from /user (v15+ zero-scope token) returns a clear message without hitting the repo.""" + client = AsyncMock() + client.get = AsyncMock(return_value=_make_mock_response(403, {})) + + result = await self.backend.test_connection(self.repo_url, self.token, client) + + assert result["success"] is False + assert "read:user scope" in result["message"] + assert client.get.call_count == 1 + + @pytest.mark.asyncio + async def test_unexpected_user_status_returns_status_code(self): + """A non-200/401/403 response from /user (e.g. 429, 5xx) surfaces the status code.""" + client = AsyncMock() + client.get = AsyncMock(return_value=_make_mock_response(429, {})) + + result = await self.backend.test_connection(self.repo_url, self.token, client) + + assert result["success"] is False + assert "429" in result["message"] + assert client.get.call_count == 1 + + @pytest.mark.asyncio + async def test_repo_404_after_valid_token_surfaces_v15_scope_hint(self): + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, {"login": "user"}), + _make_mock_response(404, {}), + ] + ) + + result = await self.backend.test_connection(self.repo_url, self.token, client) + + assert result["success"] is False + assert "v15" in result["message"] + assert "scope" in result["message"] + + @pytest.mark.asyncio + async def test_token_lacks_push_permission_returns_failed(self): + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, {"login": "user"}), + _make_mock_response(200, {"full_name": "owner/repo", "permissions": {"push": False, "pull": True}}), + ] + ) + + result = await self.backend.test_connection(self.repo_url, self.token, client) + + assert result["success"] is False + assert "push permission" in result["message"] + assert result["repo_name"] == "owner/repo" + + @pytest.mark.asyncio + async def test_non_404_api_error_returns_status_code(self): + client = AsyncMock() + client.get = AsyncMock( + side_effect=[ + _make_mock_response(200, {"login": "user"}), + _make_mock_response(500, {}), + ] + ) + + result = await self.backend.test_connection(self.repo_url, self.token, client) + + assert result["success"] is False + assert "API error: 500" in result["message"] + + @pytest.mark.asyncio + async def test_connection_exception_includes_detail_not_just_classname(self): + """B23: A connection exception surfaces both the exception class and its message, + so 'Test Connection' in the UI shows actionable detail (e.g. cert verify failure).""" + import httpx + + client = AsyncMock() + client.get = AsyncMock( + side_effect=httpx.ConnectError("certificate verify failed: hostname mismatch"), + ) + + result = await self.backend.test_connection(self.repo_url, self.token, client) + + assert result["success"] is False + assert "ConnectError" in result["message"] + assert "certificate verify failed" in result["message"] + + class TestGitLabBackend: def setup_method(self): self.backend = GitLabBackend()