From 27766c142b86f0f4ce0b00d16a2fb3ebb61576aa Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Sat, 9 May 2026 22:37:32 -0700 Subject: [PATCH] feat(system): Test: credential error handling Co-Authored-By: @devpulse --- .../drone/apps/handlers/git/pr_handler.py | 84 ++++++++++--------- .../apps/plugins/devpulse_ops/pr_plugin.py | 9 +- src/aipass/drone/tests/test_git_module.py | 82 +++++++++++++++++- 3 files changed, 127 insertions(+), 48 deletions(-) diff --git a/src/aipass/drone/apps/handlers/git/pr_handler.py b/src/aipass/drone/apps/handlers/git/pr_handler.py index 118fb0ab..0ffd7f29 100644 --- a/src/aipass/drone/apps/handlers/git/pr_handler.py +++ b/src/aipass/drone/apps/handlers/git/pr_handler.py @@ -30,48 +30,56 @@ from aipass.drone.apps.handlers.git.lock_handler import ( ) -def _is_permission_error(stderr: str) -> bool: - """Check if a push failure is a permission/auth error (fork scenario).""" - indicators = ["403", "permission", "denied", "could not read Username"] +def _has_credential_helper() -> bool: + """Check whether git has a credential.helper configured at any level.""" + try: + r = subprocess.run( + ["git", "config", "--get-all", "credential.helper"], + capture_output=True, + text=True, + ) + return r.returncode == 0 and bool(r.stdout.strip()) + except OSError as exc: + logger.warning("credential helper check failed: %s", exc) + return False + + +def _diagnose_push_failure(stderr: str, branch: str) -> str: + """Produce an actionable error message for a failed git push. + + Distinguishes between: + - No credential helper (most common on fresh setups) + - Expired or invalid token + - Actual permission / fork issues + """ lower = stderr.lower() - return any(ind.lower() in lower for ind in indicators) + auth_indicators = ["403", "could not read username", "terminal prompts disabled"] + permission_indicators = ["permission", "denied"] - -def _fork_recovery_message(branch: str, repo_root: str = ".") -> str: - """Build a dynamic fork recovery message with actual repo/user info.""" - origin = "AIOSAI/AIPass" - try: - r = subprocess.run( - ["gh", "repo", "view", "--json", "nameWithOwner", "-q", ".nameWithOwner"], - capture_output=True, - text=True, - cwd=repo_root, + if not _has_credential_helper(): + return ( + "Push failed: no git credential helper configured.\n" + " git cannot obtain auth tokens for push.\n" + " Fix: gh auth setup-git\n" + ' Then retry: drone @git pr ""' ) - if r.returncode == 0 and r.stdout.strip(): - origin = r.stdout.strip() - except OSError as exc: - logger.info("Could not detect origin repo: %s", exc) - gh_user = "" - try: - r = subprocess.run( - ["gh", "api", "user", "-q", ".login"], - capture_output=True, - text=True, + if any(ind in lower for ind in auth_indicators): + return ( + "Push failed: authentication error (token may be expired or invalid).\n" + f" stderr: {stderr}\n" + " Check: gh auth status\n" + " Fix: gh auth login" ) - if r.returncode == 0 and r.stdout.strip(): - gh_user = r.stdout.strip() - except OSError as exc: - logger.info("Could not detect gh user: %s", exc) - return f"""Push failed due to insufficient permissions. You may be working on a fork. + if any(ind in lower for ind in permission_indicators): + return ( + f"Push failed: permission denied for branch '{branch}'.\n" + f" stderr: {stderr}\n" + " Check repo write access: gh api repos/:owner/:repo --jq '.permissions'" + ) -To contribute from a fork: - 1. Create a fork: gh repo fork {origin} --remote=false --clone=false - 2. Add fork as remote: git remote add fork https://github.com/{gh_user}/{origin.split("/")[-1]}.git - 3. Push to your fork: git push -u fork {branch} - 4. Open cross-repo PR: gh pr create -R {origin} -H {gh_user}:{branch} -B main -""" + return f"Push failed: {stderr}" def _slugify(description: str) -> str: @@ -227,11 +235,7 @@ def create_pr(branch_name: str, description: str, branch_dir: Path) -> dict: cwd=str(repo_root), ) if push.returncode != 0: - stderr = push.stderr.strip() - if _is_permission_error(stderr): - result["message"] = _fork_recovery_message(feature_branch, str(repo_root)) - else: - result["message"] = f"Push failed: {stderr}" + result["message"] = _diagnose_push_failure(push.stderr.strip(), feature_branch) logger.error(result["message"]) return result diff --git a/src/aipass/drone/apps/plugins/devpulse_ops/pr_plugin.py b/src/aipass/drone/apps/plugins/devpulse_ops/pr_plugin.py index 0b2f0b6e..c0b06f1f 100644 --- a/src/aipass/drone/apps/plugins/devpulse_ops/pr_plugin.py +++ b/src/aipass/drone/apps/plugins/devpulse_ops/pr_plugin.py @@ -33,8 +33,7 @@ from aipass.drone.apps.handlers.git.lock_handler import ( release_lock, ) from aipass.drone.apps.handlers.git.pr_handler import ( - _is_permission_error, - _fork_recovery_message, + _diagnose_push_failure, ) @@ -216,11 +215,7 @@ def create_system_pr(description: str, caller: str) -> dict: cwd=str(repo_root), ) if push.returncode != 0: - stderr = push.stderr.strip() - if _is_permission_error(stderr): - result["message"] = _fork_recovery_message(feature_branch, str(repo_root)) - else: - result["message"] = f"Push failed: {stderr}" + result["message"] = _diagnose_push_failure(push.stderr.strip(), feature_branch) logger.error(result["message"]) return result diff --git a/src/aipass/drone/tests/test_git_module.py b/src/aipass/drone/tests/test_git_module.py index 633722b1..9d2f53ca 100644 --- a/src/aipass/drone/tests/test_git_module.py +++ b/src/aipass/drone/tests/test_git_module.py @@ -26,7 +26,11 @@ from aipass.drone.apps.handlers.git.lock_handler import ( ) from aipass.drone.apps.handlers.git.status_handler import get_branch_status from aipass.drone.apps.handlers.git.sync_handler import sync_main -from aipass.drone.apps.handlers.git.pr_handler import create_pr +from aipass.drone.apps.handlers.git.pr_handler import ( + _diagnose_push_failure, + _has_credential_helper, + create_pr, +) from aipass.drone.apps.modules.git_module import ( DRONE_MODULE, _detect_branch_dir, @@ -511,6 +515,82 @@ class TestPRHandler: assert "src/aipass/api" in pathspec, f"pathspec should target branch_dir, got: {pathspec}" +class TestDiagnosePushFailure: + """Tests for _has_credential_helper and _diagnose_push_failure.""" + + def test_has_credential_helper_true(self) -> None: + """Returns True when git credential.helper is configured.""" + mock_result = MagicMock() + mock_result.returncode = 0 + mock_result.stdout = "store\n" + with patch("aipass.drone.apps.handlers.git.pr_handler.subprocess.run", return_value=mock_result): + assert _has_credential_helper() is True + + def test_has_credential_helper_false_no_config(self) -> None: + """Returns False when git config returns non-zero (no helper set).""" + mock_result = MagicMock() + mock_result.returncode = 1 + mock_result.stdout = "" + with patch("aipass.drone.apps.handlers.git.pr_handler.subprocess.run", return_value=mock_result): + assert _has_credential_helper() is False + + def test_has_credential_helper_false_empty_stdout(self) -> None: + """Returns False when git config returns 0 but stdout is whitespace-only.""" + mock_result = MagicMock() + mock_result.returncode = 0 + mock_result.stdout = " \n" + with patch("aipass.drone.apps.handlers.git.pr_handler.subprocess.run", return_value=mock_result): + assert _has_credential_helper() is False + + def test_has_credential_helper_oserror(self) -> None: + """Returns False when subprocess raises OSError.""" + with patch( + "aipass.drone.apps.handlers.git.pr_handler.subprocess.run", + side_effect=OSError("git not found"), + ): + assert _has_credential_helper() is False + + def test_diagnose_no_credential_helper(self) -> None: + """Suggests gh auth setup-git when no credential helper is configured.""" + with patch("aipass.drone.apps.handlers.git.pr_handler._has_credential_helper", return_value=False): + msg = _diagnose_push_failure("fatal: could not read Username", "feat/test") + assert "no git credential helper configured" in msg.lower() + assert "gh auth setup-git" in msg + + def test_diagnose_auth_error_403(self) -> None: + """Identifies 403 as an authentication/token error.""" + with patch("aipass.drone.apps.handlers.git.pr_handler._has_credential_helper", return_value=True): + msg = _diagnose_push_failure("The requested URL returned error: 403", "feat/test") + assert "authentication error" in msg.lower() + assert "gh auth login" in msg + + def test_diagnose_auth_error_terminal_prompts(self) -> None: + """Identifies terminal prompts disabled as an auth error.""" + with patch("aipass.drone.apps.handlers.git.pr_handler._has_credential_helper", return_value=True): + msg = _diagnose_push_failure("terminal prompts disabled", "feat/test") + assert "authentication error" in msg.lower() + + def test_diagnose_permission_denied(self) -> None: + """Identifies permission denied as a repo access issue.""" + with patch("aipass.drone.apps.handlers.git.pr_handler._has_credential_helper", return_value=True): + msg = _diagnose_push_failure("Permission denied to AIOSAI/repo", "feat/test") + assert "permission denied" in msg.lower() + assert "feat/test" in msg + + def test_diagnose_unknown_error_passthrough(self) -> None: + """Passes through unrecognized errors with stderr content.""" + with patch("aipass.drone.apps.handlers.git.pr_handler._has_credential_helper", return_value=True): + msg = _diagnose_push_failure("some unknown git error", "feat/test") + assert msg == "Push failed: some unknown git error" + + def test_diagnose_credential_check_takes_priority(self) -> None: + """No credential helper is checked first, even if stderr also matches auth indicators.""" + with patch("aipass.drone.apps.handlers.git.pr_handler._has_credential_helper", return_value=False): + msg = _diagnose_push_failure("403 forbidden", "feat/test") + assert "credential helper" in msg.lower() + assert "403" not in msg + + # =========================================================================== # 5. git_module — command routing, unknown commands, help/introspection # ===========================================================================