diff --git a/ais_core/repo_resolve.py b/ais_core/repo_resolve.py index fc004e3..ea9133d 100644 --- a/ais_core/repo_resolve.py +++ b/ais_core/repo_resolve.py @@ -1,21 +1,52 @@ """ais_core.repo_resolve — resolve a repo hint to a fully-qualified GitHub repository reference plus optional local checkout path. -This module is intentionally side-effect-free: it does not import any -GitHub client or perform I/O at import time. The functions below are -placeholders for the v0.10.0 implementation (Issue #1b); they will -be filled in by subsequent work. - -Public API: - resolve_repo_hint(hint) — accept 'owner/repo', 'repo', - 'repo_hint', or '/local/path' - resolve_from_owner_repo(owner, repo) - resolve_from_git_remote(path) - ResolvedRepo — typed result +This module is intentionally side-effect-free for ``resolve_from_owner_repo`` +(no network, no GitHub API call) and only invokes local ``git`` for +``resolve_from_git_remote`` (no network). It is the entry point for +extracting repo-resolution logic out of ``scripts/solve_issues.py`` +under Issue #1b (Wave 1b). + +Resolution surface (public API): + +- :func:`resolve_from_owner_repo` — normalize an explicit + ``(owner, repo)`` pair into a :class:`ResolvedRepo`. No API call. +- :func:`resolve_from_git_remote` — read ``remote.origin.url`` from a + local git checkout and parse out the (owner, repo) pair. No + network; uses local ``git config``. +- :func:`resolve_repo_hint` — dispatch a free-form hint string to the + appropriate resolver based on its shape: + + * ``"/local/path"`` or any existing directory → :func:`resolve_from_git_remote` + * ``"owner/repo"`` → :func:`resolve_from_owner_repo` with the split pair + * bare ``"repo"`` → :func:`resolve_from_owner_repo` with the + default owner from the ``GITHUB_USER`` environment variable + +Resolution rules (encoded in :func:`_parse_repo_components`): + +- ``owner`` and ``repo`` must be non-empty, must not contain ``/`` (after + splitting for the hint case), and must match the GitHub naming + character set (alnum, ``-``, ``_``, ``.``). +- The canonical remote URL is ``https://github.com/{owner}/{repo}``. + (We do not honor per-host overrides yet; that's deferred to a + follow-up issue.) + +Limitations (intentional for #1b): + +- No GitHub API call: existence of the repo on github.com is NOT + verified. ``resolve_from_owner_repo`` will happily produce a + ``ResolvedRepo`` for a non-existent repo. +- No SSH-vs-HTTPS preference: the canonical remote is always HTTPS. +- No sub-group / sub-path support: the remote is always + ``github.com/{owner}/{repo}``, not e.g. a GitHub Enterprise URL. """ from __future__ import annotations +import os +import re +import subprocess +from pathlib import Path from typing import NamedTuple @@ -25,7 +56,8 @@ class ResolvedRepo(NamedTuple): Attributes: owner: GitHub owner (user or org). repo: GitHub repository name. - remote: Canonical remote URL (e.g. 'https://github.com/owner/repo'). + remote: Canonical remote URL (currently always + ``https://github.com/{owner}/{repo}``). local_path: Optional local checkout path (None if not on disk). """ @@ -37,29 +69,263 @@ class ResolvedRepo(NamedTuple): __all__ = [ "ResolvedRepo", - "resolve_repo_hint", "resolve_from_owner_repo", "resolve_from_git_remote", + "resolve_repo_hint", ] -def resolve_repo_hint(hint: str) -> ResolvedRepo: - """Resolve an arbitrary repo hint to a ResolvedRepo. +# --- helpers --------------------------------------------------------------- + +# GitHub naming rules differ between owner and repo: +# - owner: alnum + '-', 1-39 chars. Underscores and dots are NOT +# allowed in GitHub owner names. Must START and END with alnum +# (no leading/trailing dashes). +# - repo: alnum + '-', '_', '.', 1-100 chars. Underscores and dots +# ARE allowed in GitHub repo names. Must START and END with alnum +# (no leading/trailing separators). +_OWNER_NAME_RE = re.compile(r"^[A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?$") +_REPO_NAME_RE = re.compile(r"^[A-Za-z0-9](?:[A-Za-z0-9._-]*[A-Za-z0-9])?$") +_OWNER_MAX = 39 +_REPO_MAX = 100 + +# Hosts we accept for git remote URLs. GitHub supports both +# `github.com` and `www.github.com` as canonical hosts; anything else +# (GitLab, Bitbucket, Gitea, self-hosted) is rejected to keep the +# library's contract honest about its scope. +_ALLOWED_GIT_HOSTS = frozenset({"github.com", "www.github.com"}) + +# GitHub remote paths are EXACTLY /owner/repo[.git]. We reject +# anything with more or fewer path segments (e.g. org subgroups, +# deep paths, or empty owner/repo). +_MIN_PATH_SEGMENTS = 2 +_MAX_PATH_SEGMENTS = 2 + - Accepts any of: - - explicit 'owner/repo' - - bare 'repo' (assumes configured default owner) - - 'repo_hint' (looked up against a hint registry) - - '/local/path' to a git checkout +def _validate_component(value: str, *, kind: str) -> str: + """Validate a single owner/repo name component. Returns the cleaned + string on success, raises :class:`ValueError` otherwise. """ - raise NotImplementedError("ais_core.repo_resolve.resolve_repo_hint (Issue #1b)") + if not isinstance(value, str): + raise TypeError( + f"{kind} must be a str, got {type(value).__name__}" + ) + cleaned = value.strip() + if not cleaned: + raise ValueError(f"{kind} must not be empty") + if kind == "owner": + max_len = _OWNER_MAX + pattern = _OWNER_NAME_RE + allowed = "alnum and '-'" + else: + max_len = _REPO_MAX + pattern = _REPO_NAME_RE + allowed = "alnum, '.', '-', '_'" + if len(cleaned) > max_len: + raise ValueError( + f"{kind} too long: {len(cleaned)} chars (max {max_len})" + ) + if not pattern.match(cleaned): + raise ValueError( + f"{kind} contains invalid characters: {value!r} " + f"(allowed: {allowed})" + ) + return cleaned + + +def _canonical_remote(owner: str, repo: str) -> str: + return f"https://github.com/{owner}/{repo}" + + +def _read_git_remote_url(path: Path) -> str: + """Read ``remote.origin.url`` from a local git checkout. + + Uses ``git config --get`` (local invocation; no network). + Raises :class:`ValueError` if the URL is missing or ``git`` is not + available. + """ + try: + result = subprocess.run( + ["git", "-C", str(path), "config", "--get", "remote.origin.url"], + check=True, + capture_output=True, + text=True, + ) + except FileNotFoundError as exc: + raise ValueError( + "git executable not found; cannot resolve from local checkout" + ) from exc + except subprocess.CalledProcessError as exc: + raise ValueError( + f"no remote.origin.url configured for {path}" + ) from exc + url = result.stdout.strip() + if not url: + raise ValueError(f"empty remote.origin.url for {path}") + return url + + +def _parse_git_remote_url(url: str) -> tuple[str, str]: + """Parse a git remote URL into ``(owner, repo)``. + + Supports: + - HTTPS: ``https://github.com/owner/repo.git`` (also without ``.git``) + - SSH: ``git@github.com:owner/repo.git`` + - ssh:// URL form + """ + if not isinstance(url, str): + raise TypeError(f"url must be a str, got {type(url).__name__}") + url = url.strip() + if not url: + raise ValueError("empty git remote URL") + + if url.startswith("git@"): + # git@github.com:owner/repo.git + # Convert to ssh:// form for uniform parsing. + host_and_path = url[4:] # strip "git@" + if ":" not in host_and_path: + raise ValueError(f"cannot parse SSH git URL: {url!r}") + host, path = host_and_path.split(":", 1) + ssh_url = f"ssh://{host}/{path}" + return _parse_http_or_ssh(ssh_url, original=url) + + if url.startswith(("http://", "https://", "ssh://")): + return _parse_http_or_ssh(url, original=url) + + raise ValueError(f"unrecognized git remote URL format: {url!r}") + + +def _parse_http_or_ssh(url: str, *, original: str) -> tuple[str, str]: + """Parse an http(s) or ssh:// URL into ``(owner, repo)``. + + Also enforces that the URL's host is in :data:`_ALLOWED_GIT_HOSTS` + (currently ``github.com`` and ``www.github.com``). Other hosts + (GitLab, Bitbucket, Gitea, self-hosted) raise ``ValueError``. + """ + from urllib.parse import urlparse + parsed = urlparse(url) + host = (parsed.hostname or "").lower() + if host not in _ALLOWED_GIT_HOSTS: + raise ValueError( + f"unsupported git host: {host!r} " + f"(allowed: {sorted(_ALLOWED_GIT_HOSTS)})" + ) + path = parsed.path.strip("/") + if not path: + raise ValueError(f"cannot parse URL (no path): {original!r}") + parts = path.split("/") + if not (_MIN_PATH_SEGMENTS <= len(parts) <= _MAX_PATH_SEGMENTS): + raise ValueError( + f"unexpected path depth in {original!r}: " + f"expected exactly {_MIN_PATH_SEGMENTS} segments " + f"(/owner/repo[.git]), got {len(parts)}" + ) + owner = parts[-2] + repo = parts[-1] + if repo.endswith(".git"): + repo = repo[:-4] + return owner, repo + + +# --- public API ------------------------------------------------------------ def resolve_from_owner_repo(owner: str, repo: str) -> ResolvedRepo: - """Resolve from explicit (owner, repo) GitHub coordinates.""" - raise NotImplementedError("ais_core.repo_resolve.resolve_from_owner_repo (Issue #1b)") + """Resolve an explicit ``(owner, repo)`` pair into a :class:`ResolvedRepo`. + + Does NOT call the GitHub API. Existence of the repo on github.com + is not verified. + + Args: + owner: GitHub owner (user or org). Validated for non-empty + and GitHub naming rules. + repo: GitHub repository name. Validated likewise. + + Returns: + A :class:`ResolvedRepo` with ``remote`` set to + ``https://github.com/{owner}/{repo}`` and ``local_path=None``. + """ + owner_clean = _validate_component(owner, kind="owner") + repo_clean = _validate_component(repo, kind="repo") + return ResolvedRepo( + owner=owner_clean, + repo=repo_clean, + remote=_canonical_remote(owner_clean, repo_clean), + local_path=None, + ) def resolve_from_git_remote(path: str) -> ResolvedRepo: - """Resolve by inspecting the `origin` remote of a local git checkout.""" - raise NotImplementedError("ais_core.repo_resolve.resolve_from_git_remote (Issue #1b)") + """Resolve by reading ``remote.origin.url`` of a local git checkout. + + Args: + path: Filesystem path to a local git checkout (must contain + ``.git/config`` with a ``remote.origin.url``). + + Returns: + A :class:`ResolvedRepo` with ``local_path`` set to the given + path and ``remote`` set to the canonical HTTPS URL. + + Raises: + ValueError: if the path is not a directory, the remote URL + is missing, or the URL cannot be parsed. + """ + p = Path(path) + if not p.is_dir(): + raise ValueError(f"path is not a directory: {path!r}") + url = _read_git_remote_url(p) + owner, repo = _parse_git_remote_url(url) + return ResolvedRepo( + owner=owner, + repo=repo, + remote=_canonical_remote(owner, repo), + local_path=str(p), + ) + + +def resolve_repo_hint(hint: str) -> ResolvedRepo: + """Dispatch a free-form hint to the appropriate resolver. + + Recognized shapes (in order): + + 1. Existing directory (absolute or relative) → :func:`resolve_from_git_remote` + 2. ``"owner/repo"`` (contains a ``/``) → :func:`resolve_from_owner_repo` + with the split pair + 3. Bare ``"repo"`` (no ``/``) → :func:`resolve_from_owner_repo` with + the default owner from the ``GITHUB_USER`` environment variable + + Args: + hint: Free-form repo hint. + + Returns: + A :class:`ResolvedRepo`. + + Raises: + ValueError: if the hint is empty, the bare-repo form is used + without a ``GITHUB_USER`` env var, the path doesn't + resolve to a git checkout, or the input is otherwise + invalid. + """ + if not isinstance(hint, str): + raise TypeError(f"hint must be a str, got {type(hint).__name__}") + h = hint.strip() + if not h: + raise ValueError("hint must not be empty") + + # 1. Directory → resolve from local git remote + if Path(h).is_dir(): + return resolve_from_git_remote(h) + + # 2. owner/repo → split + if "/" in h: + owner, repo = h.split("/", 1) + return resolve_from_owner_repo(owner, repo) + + # 3. Bare repo → use GITHUB_USER default owner + default_owner = os.environ.get("GITHUB_USER", "").strip() + if not default_owner: + raise ValueError( + f"bare repo hint {h!r} requires GITHUB_USER env var " + f"(or use 'owner/repo' form or an existing local path)" + ) + return resolve_from_owner_repo(default_owner, h) diff --git a/scripts/solve_issues.py b/scripts/solve_issues.py index 6c39b4b..8d15f37 100644 --- a/scripts/solve_issues.py +++ b/scripts/solve_issues.py @@ -4318,6 +4318,15 @@ def main(): else: token, user = require_github_config(cfg, require_user=True) + # Smoke-Use of ais_core.repo_resolve (Issue #1b): normalize the + # (owner, args.repo) pair through the new library. The result is + # currently unused (no broad refactor under #1b); this single call + # exercises the library code path and validates the integration. + if args.repo: + from ais_core.repo_resolve import resolve_from_owner_repo + _resolved_repo = resolve_from_owner_repo(user, args.repo) + del _resolved_repo # suppress unused-variable lint + # KI-Worker prüfen if args.model == "codex" and not find_codex_executable() and not args.dry_run: print_err("Codex CLI wurde nicht gefunden!") diff --git a/tests/test_ais_core/test_repo_resolve.py b/tests/test_ais_core/test_repo_resolve.py index 980cdcf..36bfee0 100644 --- a/tests/test_ais_core/test_repo_resolve.py +++ b/tests/test_ais_core/test_repo_resolve.py @@ -1,31 +1,471 @@ -"""Smoke tests for ais_core.repo_resolve stub (Issue #1a). +"""Tests for ais_core.repo_resolve (Issue #1b). -These tests verify only that the stub can be imported and exposes the -expected public surface. Behaviour tests are intentionally deferred to -Issue #1b, where the real repo-resolution logic lands. +Coverage: +- Unit tests for resolve_from_owner_repo, resolve_from_git_remote, + resolve_repo_hint (no network access, all tests offline). +- Characterization tests for the legacy behaviour: owner comes from + config/auth, ``args.repo`` is the repo-name string. Captured as a + test helper ``_legacy_resolve()`` — NOT exposed as a production + function in ais_core. +- Comparison tests: library and legacy agree on the simple + ``(owner, repo)`` case. +- No-network test: monkey-patch subprocess to ensure resolve_from_git_remote + does not call any URL beyond the local git binary. """ +import os +import subprocess +import tempfile import unittest +from pathlib import Path +from unittest import mock +from ais_core.repo_resolve import ( + ResolvedRepo, + resolve_from_owner_repo, + resolve_from_git_remote, + resolve_repo_hint, +) -class TestRepoResolveStub(unittest.TestCase): - def test_module_importable(self) -> None: - import ais_core.repo_resolve - self.assertTrue(hasattr(ais_core.repo_resolve, "__all__")) +# --- Test-Helper: characterizes pre-#470 behaviour ----------------------------- +# This is NOT a production function. It exists only to pin down what +# the old solve_issues.py code path *would* do, so that we can verify +# the new library function produces an equivalent result. - def test_all_exports_resolve(self) -> None: - from ais_core.repo_resolve import ( - ResolvedRepo, - resolve_from_git_remote, - resolve_from_owner_repo, - resolve_repo_hint, +def _legacy_resolve(owner: str, repo: str) -> ResolvedRepo: + """Characterization of the pre-#470 behaviour. + + Pre-#470, scripts/solve_issues.py accepted ``--repo `` plus + an ``owner`` derived from preflight_checks / require_github_config + (i.e. from the GitHub auth response). There was no unified + ``ResolvedRepo`` shape; the script just used the two strings + independently. + + This helper pins the equivalent of that code path: take + ``(owner, repo)`` strings, return a ResolvedRepo with the + canonical remote URL and no local path. This is the behaviour we + want to preserve across the refactor. + """ + return ResolvedRepo( + owner=owner, + repo=repo, + remote=f"https://github.com/{owner}/{repo}", + local_path=None, + ) + + +# --- Unit tests ------------------------------------------------------------ + + +class TestResolveFromOwnerRepo(unittest.TestCase): + def test_basic(self) -> None: + r = resolve_from_owner_repo("alice", "demo") + self.assertEqual(r.owner, "alice") + self.assertEqual(r.repo, "demo") + self.assertEqual(r.remote, "https://github.com/alice/demo") + self.assertIsNone(r.local_path) + + def test_preserves_case(self) -> None: + # GitHub preserves case in the URL. + r = resolve_from_owner_repo("Alice", "Demo") + self.assertEqual(r.owner, "Alice") + self.assertEqual(r.repo, "Demo") + self.assertEqual(r.remote, "https://github.com/Alice/Demo") + + def test_strips_whitespace(self) -> None: + r = resolve_from_owner_repo(" alice ", " demo ") + self.assertEqual(r.owner, "alice") + self.assertEqual(r.repo, "demo") + + def test_repo_with_dash_dot_underscore(self) -> None: + r = resolve_from_owner_repo("alice", "ai.issue_solver-v2") + self.assertEqual(r.repo, "ai.issue_solver-v2") + + def test_owner_with_dash(self) -> None: + # Owner names allow '-' (orgs often have dashes). + r = resolve_from_owner_repo("my-org", "demo") + self.assertEqual(r.owner, "my-org") + + def test_owner_with_underscore_rejected(self) -> None: + # Per #470 review: owner names must NOT contain '_'. + with self.assertRaises(ValueError): + resolve_from_owner_repo("my_owner", "demo") + + def test_owner_with_dot_rejected(self) -> None: + # Per #470 review: owner names must NOT contain '.'. + with self.assertRaises(ValueError): + resolve_from_owner_repo("my.owner", "demo") + + def test_owner_with_dash_ok(self) -> None: + # Sanity: '-' IS still allowed in owner names. + r = resolve_from_owner_repo("my-org-2024", "demo") + self.assertEqual(r.owner, "my-org-2024") + + def test_owner_leading_dash_rejected(self) -> None: + # Per #470 review: owner must START with alnum, not '-'. + with self.assertRaises(ValueError): + resolve_from_owner_repo("-owner", "demo") + + def test_owner_trailing_dash_rejected(self) -> None: + # Per #470 review: owner must END with alnum, not '-'. + with self.assertRaises(ValueError): + resolve_from_owner_repo("owner-", "demo") + + def test_owner_only_dashes_rejected(self) -> None: + # '-' alone is not a valid owner. + with self.assertRaises(ValueError): + resolve_from_owner_repo("---", "demo") + + def test_repo_leading_dash_rejected(self) -> None: + # Same rule applies to repo: start with alnum. + with self.assertRaises(ValueError): + resolve_from_owner_repo("alice", "-repo") + + def test_repo_trailing_dash_rejected(self) -> None: + with self.assertRaises(ValueError): + resolve_from_owner_repo("alice", "repo-") + + def test_repo_only_dots_rejected(self) -> None: + # Repo allows '.' but must start/end with alnum. + with self.assertRaises(ValueError): + resolve_from_owner_repo("alice", "...") + + def test_empty_owner_raises(self) -> None: + with self.assertRaises(ValueError): + resolve_from_owner_repo("", "demo") + + def test_empty_repo_raises(self) -> None: + with self.assertRaises(ValueError): + resolve_from_owner_repo("alice", "") + + def test_slash_in_owner_raises(self) -> None: + with self.assertRaises(ValueError): + resolve_from_owner_repo("alice/bob", "demo") + + def test_slash_in_repo_raises(self) -> None: + with self.assertRaises(ValueError): + resolve_from_owner_repo("alice", "demo/sub") + + def test_owner_too_long_raises(self) -> None: + # GitHub owner names are max 39 chars. + with self.assertRaises(ValueError): + resolve_from_owner_repo("a" * 40, "demo") + + def test_repo_too_long_raises(self) -> None: + with self.assertRaises(ValueError): + resolve_from_owner_repo("alice", "a" * 101) + + def test_no_api_call_made(self) -> None: + # Smoke: the function must complete without network. + # We assert by patching subprocess and socket; if either was + # invoked we'd notice via the mock. + with mock.patch("subprocess.run") as sr, mock.patch( + "urllib.request.urlopen" + ) as urlopen: + resolve_from_owner_repo("alice", "demo") + sr.assert_not_called() + urlopen.assert_not_called() + + +class TestParseGitRemoteUrl(unittest.TestCase): + """Direct tests of the URL parser, since it's the trickiest piece.""" + + def test_https_with_git_suffix(self) -> None: + from ais_core.repo_resolve import _parse_git_remote_url + self.assertEqual( + _parse_git_remote_url("https://github.com/alice/demo.git"), + ("alice", "demo"), + ) + + def test_https_without_git_suffix(self) -> None: + from ais_core.repo_resolve import _parse_git_remote_url + self.assertEqual( + _parse_git_remote_url("https://github.com/alice/demo"), + ("alice", "demo"), + ) + + def test_https_with_trailing_slash(self) -> None: + from ais_core.repo_resolve import _parse_git_remote_url + self.assertEqual( + _parse_git_remote_url("https://github.com/alice/demo/"), + ("alice", "demo"), + ) + + def test_https_with_www_prefix(self) -> None: + # GitHub accepts www.github.com as a canonical alias. + from ais_core.repo_resolve import _parse_git_remote_url + self.assertEqual( + _parse_git_remote_url("https://www.github.com/alice/demo.git"), + ("alice", "demo"), + ) + + def test_ssh_short_form(self) -> None: + from ais_core.repo_resolve import _parse_git_remote_url + self.assertEqual( + _parse_git_remote_url("git@github.com:alice/demo.git"), + ("alice", "demo"), + ) + + def test_ssh_url_form(self) -> None: + from ais_core.repo_resolve import _parse_git_remote_url + self.assertEqual( + _parse_git_remote_url("ssh://git@github.com/alice/demo.git"), + ("alice", "demo"), + ) + + def test_ssh_url_with_www_prefix(self) -> None: + from ais_core.repo_resolve import _parse_git_remote_url + self.assertEqual( + _parse_git_remote_url("ssh://git@www.github.com/alice/demo.git"), + ("alice", "demo"), + ) + + # --- exact /owner/repo path enforcement --- + + def test_extra_path_segments_rejected(self) -> None: + # Per #470 review: GitHub URLs are EXACTLY /owner/repo[.git]. + # Subgroups like /org/sub/repo must be rejected, not silently + # truncated to the last two segments. + from ais_core.repo_resolve import _parse_git_remote_url + with self.assertRaises(ValueError): + _parse_git_remote_url("https://github.com/org/sub/repo.git") + + def test_single_segment_path_rejected(self) -> None: + from ais_core.repo_resolve import _parse_git_remote_url + with self.assertRaises(ValueError): + _parse_git_remote_url("https://github.com/justarepo.git") + + def test_empty_path_rejected(self) -> None: + from ais_core.repo_resolve import _parse_git_remote_url + with self.assertRaises(ValueError): + _parse_git_remote_url("https://github.com/") + + def test_three_segment_ssh_rejected(self) -> None: + from ais_core.repo_resolve import _parse_git_remote_url + with self.assertRaises(ValueError): + _parse_git_remote_url("git@github.com:org/sub/repo.git") + + # --- non-GitHub host rejection --- + + def test_gitlab_https_rejected(self) -> None: + from ais_core.repo_resolve import _parse_git_remote_url + with self.assertRaises(ValueError): + _parse_git_remote_url("https://gitlab.com/alice/demo.git") + + def test_gitlab_ssh_rejected(self) -> None: + from ais_core.repo_resolve import _parse_git_remote_url + with self.assertRaises(ValueError): + _parse_git_remote_url("git@gitlab.com:alice/demo.git") + + def test_bitbucket_rejected(self) -> None: + from ais_core.repo_resolve import _parse_git_remote_url + with self.assertRaises(ValueError): + _parse_git_remote_url("https://bitbucket.org/alice/demo.git") + + def test_self_hosted_rejected(self) -> None: + from ais_core.repo_resolve import _parse_git_remote_url + with self.assertRaises(ValueError): + _parse_git_remote_url("https://git.example.com/alice/demo.git") + + def test_invalid_url_raises(self) -> None: + from ais_core.repo_resolve import _parse_git_remote_url + with self.assertRaises(ValueError): + _parse_git_remote_url("not a url") + + def test_empty_url_raises(self) -> None: + from ais_core.repo_resolve import _parse_git_remote_url + with self.assertRaises(ValueError): + _parse_git_remote_url("") + + +class TestResolveFromGitRemote(unittest.TestCase): + def setUp(self) -> None: + # Create a tiny local git repo with a remote.origin.url set. + self.tmpdir = Path(tempfile.mkdtemp(prefix="ais-repo-resolve-")) + self._run_git("init") + self._run_git("config", "user.email", "test@example.com") + self._run_git("config", "user.name", "Test") + self._run_git("config", "remote.origin.url", "https://github.com/alice/demo.git") + + def tearDown(self) -> None: + import shutil + shutil.rmtree(self.tmpdir, ignore_errors=True) + + def _run_git(self, *args: str) -> str: + result = subprocess.run( + ["git", "-C", str(self.tmpdir), *args], + check=True, + capture_output=True, + text=True, + ) + return result.stdout.strip() + + def test_basic(self) -> None: + r = resolve_from_git_remote(str(self.tmpdir)) + self.assertEqual(r.owner, "alice") + self.assertEqual(r.repo, "demo") + self.assertEqual(r.remote, "https://github.com/alice/demo") + self.assertEqual(r.local_path, str(self.tmpdir)) + + def test_ssh_remote(self) -> None: + self._run_git("config", "remote.origin.url", "git@github.com:alice/demo.git") + r = resolve_from_git_remote(str(self.tmpdir)) + self.assertEqual(r.owner, "alice") + self.assertEqual(r.repo, "demo") + + def test_path_is_not_dir(self) -> None: + with self.assertRaises(ValueError): + resolve_from_git_remote("/nonexistent/path/that/is/not/here") + + def test_missing_remote_url(self) -> None: + # Init a fresh repo WITHOUT setting remote.origin.url. + empty = Path(tempfile.mkdtemp(prefix="ais-repo-empty-")) + try: + subprocess.run( + ["git", "-C", str(empty), "init"], + check=True, + capture_output=True, + ) + with self.assertRaises(ValueError): + resolve_from_git_remote(str(empty)) + finally: + import shutil + shutil.rmtree(empty, ignore_errors=True) + + +class TestResolveRepoHint(unittest.TestCase): + def setUp(self) -> None: + self.tmpdir = Path(tempfile.mkdtemp(prefix="ais-hint-")) + subprocess.run( + ["git", "-C", str(self.tmpdir), "init"], + check=True, capture_output=True, ) + subprocess.run( + ["git", "-C", str(self.tmpdir), "config", "remote.origin.url", + "https://github.com/bob/local-checkout.git"], + check=True, capture_output=True, + ) + # Stash + restore GITHUB_USER around each test. + self._saved_github_user = os.environ.get("GITHUB_USER") + self._saved_cwd = os.getcwd() + os.chdir(self.tmpdir) # so Path(hint).is_dir() works for relative paths + + def tearDown(self) -> None: + import shutil + os.chdir(self._saved_cwd) + if self._saved_github_user is None: + os.environ.pop("GITHUB_USER", None) + else: + os.environ["GITHUB_USER"] = self._saved_github_user + shutil.rmtree(self.tmpdir, ignore_errors=True) + + def test_owner_repo_format(self) -> None: + r = resolve_repo_hint("alice/other-repo") + self.assertEqual(r.owner, "alice") + self.assertEqual(r.repo, "other-repo") + self.assertIsNone(r.local_path) + + def test_directory_format_absolute(self) -> None: + r = resolve_repo_hint(str(self.tmpdir)) + self.assertEqual(r.owner, "bob") + self.assertEqual(r.repo, "local-checkout") + self.assertEqual(r.local_path, str(self.tmpdir)) + + def test_bare_repo_uses_github_user_env(self) -> None: + os.environ["GITHUB_USER"] = "default-owner" + r = resolve_repo_hint("just-a-repo") + self.assertEqual(r.owner, "default-owner") + self.assertEqual(r.repo, "just-a-repo") + + def test_bare_repo_without_github_user_raises(self) -> None: + os.environ.pop("GITHUB_USER", None) + with self.assertRaises(ValueError): + resolve_repo_hint("just-a-repo") + + def test_empty_hint_raises(self) -> None: + with self.assertRaises(ValueError): + resolve_repo_hint("") + + def test_whitespace_hint_raises(self) -> None: + with self.assertRaises(ValueError): + resolve_repo_hint(" ") + + +# --- Characterization + Comparison tests ---------------------------------- + + +class TestCharacterizationLegacy(unittest.TestCase): + """Pin the pre-#470 behaviour so we can detect drift during refactor.""" + + def test_legacy_resolve_basic(self) -> None: + # Owner from auth/config, repo from --repo CLI arg. + r = _legacy_resolve("SaJaToGu", "ai-issue-solver") + self.assertEqual(r.owner, "SaJaToGu") + self.assertEqual(r.repo, "ai-issue-solver") + self.assertEqual(r.remote, "https://github.com/SaJaToGu/ai-issue-solver") + self.assertIsNone(r.local_path) + + def test_legacy_resolve_strips_nothing(self) -> None: + # The legacy code did NOT validate input. This test pins that + # the new library function tightens input (separate test). + r = _legacy_resolve("SaJaToGu", "ai-issue-solver ") + # Legacy passed through with whitespace; we DO NOT preserve that. + # This documents the behavior the library function CHANGES. + self.assertEqual(r.repo, "ai-issue-solver ") + + +class TestComparisonLegacyVsLibrary(unittest.TestCase): + """For the simple (owner, repo) case, library and legacy must agree.""" + + def test_agrees_on_basic(self) -> None: + lib = resolve_from_owner_repo("SaJaToGu", "ai-issue-solver") + leg = _legacy_resolve("SaJaToGu", "ai-issue-solver") + self.assertEqual(lib, leg) + + def test_agrees_on_realistic_owners(self) -> None: + for owner, repo in [ + ("SaJaToGu", "ai-issue-solver"), + ("python", "cpython"), + ("microsoft", "TypeScript"), + ]: + with self.subTest(owner=owner, repo=repo): + self.assertEqual( + resolve_from_owner_repo(owner, repo), + _legacy_resolve(owner, repo), + ) + + +class TestNoNetworkAccess(unittest.TestCase): + """All repo_resolve functions must work fully offline.""" + + def test_resolve_from_owner_repo_no_network(self) -> None: + # Block all network at the socket layer; the function should + # still succeed. + import socket + original_socket = socket.socket + with mock.patch("socket.socket", side_effect=AssertionError("network call")): + r = resolve_from_owner_repo("alice", "demo") + self.assertEqual(r.owner, "alice") - self.assertTrue(callable(resolve_repo_hint)) - self.assertTrue(callable(resolve_from_owner_repo)) - self.assertTrue(callable(resolve_from_git_remote)) - self.assertTrue(callable(ResolvedRepo)) + def test_resolve_from_git_remote_uses_local_git_only(self) -> None: + # Create a local repo and verify resolve_from_git_remote + # only invokes the local git binary (no URL fetches). + with tempfile.TemporaryDirectory() as tmp: + tmpdir = Path(tmp) + subprocess.run( + ["git", "-C", str(tmpdir), "init"], + check=True, capture_output=True, + ) + subprocess.run( + ["git", "-C", str(tmpdir), "config", "remote.origin.url", + "https://github.com/alice/demo.git"], + check=True, capture_output=True, + ) + with mock.patch("urllib.request.urlopen") as urlopen: + r = resolve_from_git_remote(str(tmpdir)) + urlopen.assert_not_called() + self.assertEqual(r.owner, "alice") if __name__ == "__main__":