diff --git a/CHANGELOG.md b/CHANGELOG.md index ca424f9d..e433efbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,16 @@ per-file diff commands. ### Fixed +- **Fork-skip guards no longer run on a fork during a local test run** (`tests/_upstream.py`, + `tests/test_placeholder_integrity.py`, `tests/test_setup_command.py`, + `tests/test_upstream_checkout.py`) - the fork skip read + `os.environ.get("GITHUB_REPOSITORY", UPSTREAM)`, so with the variable unset a local + run fell back to upstream and the placeholder guards fired on every personalized fork, + leaving its suite permanently red after `/setup`. The three skips now share one helper: + `GITHUB_REPOSITORY` decides when set, otherwise the checkout's `origin` remote is matched + against the upstream repo (https and ssh forms, optional `.git` suffix), and an + undeterminable checkout assumes upstream. Tests only, no `framework_version` bump. + - **`linkedin-search` and `freehire-search` reject invalid `--format` flags with `BAD_ARG`** (#555, `.agents/skills/linkedin-search/cli/src/cli.ts`, `.agents/skills/freehire-search/cli/src/cli.ts`) - unrecognized `--format` flags previously fell back silently to `"json"` with exit code 0, masking diff --git a/tests/_upstream.py b/tests/_upstream.py new file mode 100644 index 00000000..7fc2351e --- /dev/null +++ b/tests/_upstream.py @@ -0,0 +1,63 @@ +"""Shared fork-detection for tests that only apply to the upstream template. + +Several guards (placeholder integrity, template placeholders) assert on the +pristine upstream files, so they must skip on personalized forks. In CI the +``GITHUB_REPOSITORY`` variable is authoritative; in a local run it is unset, +so fall back to the checkout's ``origin`` remote instead of assuming +upstream. +""" +import os +import subprocess +from pathlib import Path + +UPSTREAM = "MadsLorentzen/ai-job-search" + +REPO = Path(__file__).resolve().parent.parent + +_HTTPS_PREFIX = "https://github.com/" +_SSH_PREFIX = "git@github.com:" +_SSH_URL_PREFIX = "ssh://git@github.com/" + + +def _is_upstream_url(url: str) -> bool: + """Whether a git remote URL points at the upstream template repo. + + Matches the https, scp-style ssh and ``ssh://`` forms with an optional + ``.git`` suffix; + anything else (a fork URL, some other host) is not upstream. + """ + lowered = url.strip().lower() + slug = UPSTREAM.lower() + for prefix in (_HTTPS_PREFIX, _SSH_PREFIX, _SSH_URL_PREFIX): + if lowered.startswith(prefix): + rest = lowered[len(prefix):].rstrip("/") + if rest.endswith(".git"): + rest = rest[: -len(".git")] + return rest == slug + return False + + +def is_upstream_checkout() -> bool: + """Whether this checkout is the upstream template rather than a fork. + + ``GITHUB_REPOSITORY`` decides when set (CI). Otherwise the ``origin`` + remote is inspected. When nothing can be determined (git missing, + timeout, non-zero exit), upstream is assumed: the guards then run + exactly as they do today. + """ + env_repo = os.environ.get("GITHUB_REPOSITORY") + if env_repo: + return env_repo == UPSTREAM + try: + origin = subprocess.run( + ["git", "remote", "get-url", "origin"], + cwd=REPO, + capture_output=True, + text=True, + timeout=10, + ) + except (OSError, subprocess.SubprocessError): + return True + if origin.returncode != 0: + return True + return _is_upstream_url(origin.stdout) diff --git a/tests/test_placeholder_integrity.py b/tests/test_placeholder_integrity.py index 2a2b91c7..6d3b38b5 100644 --- a/tests/test_placeholder_integrity.py +++ b/tests/test_placeholder_integrity.py @@ -15,11 +15,13 @@ the sentinels exist in the pristine files, and (c) that simulating the /setup edit destroys at least one checked sentinel per file - i.e. the guard actually fires on the failure it exists to catch. """ -import os import unittest from pathlib import Path -UPSTREAM = "MadsLorentzen/ai-job-search" +try: + from tests._upstream import is_upstream_checkout +except ImportError: # `python -m unittest discover -s tests` imports test modules top-level + from _upstream import is_upstream_checkout REPO = Path(__file__).resolve().parent.parent CI = REPO / ".github" / "workflows" / "ci.yml" @@ -45,7 +47,7 @@ def personalize_cv(text: str) -> str: @unittest.skipIf( - os.environ.get("GITHUB_REPOSITORY", UPSTREAM) != UPSTREAM, + not is_upstream_checkout(), "placeholder-integrity guards the pristine upstream template; forks personalize these files via /setup", ) class TestCvSentinelsAreDataLocated(unittest.TestCase): @@ -82,7 +84,7 @@ class TestCvSentinelsAreDataLocated(unittest.TestCase): @unittest.skipIf( - os.environ.get("GITHUB_REPOSITORY", UPSTREAM) != UPSTREAM, + not is_upstream_checkout(), "placeholder-integrity guards the pristine upstream template; forks personalize these files via /setup", ) class TestProfileSentinelIsDataLocated(unittest.TestCase): diff --git a/tests/test_setup_command.py b/tests/test_setup_command.py index 2f5deefe..1533a2ac 100644 --- a/tests/test_setup_command.py +++ b/tests/test_setup_command.py @@ -9,12 +9,14 @@ were not, so a full Path B/C run left `[YOUR_NAME]`, `[YOUR_EMAIL]` and on the drafter noticing. A real user (#420) ran `/setup` and then hand-edited both files to close the gap. """ -import os import re import unittest from pathlib import Path -UPSTREAM = "MadsLorentzen/ai-job-search" +try: + from tests._upstream import is_upstream_checkout +except ImportError: # `python -m unittest discover -s tests` imports test modules top-level + from _upstream import is_upstream_checkout REPO = Path(__file__).resolve().parent.parent COMMAND = REPO / ".claude" / "commands" / "setup.md" @@ -72,7 +74,7 @@ class SetupStep3ContactBlocks(unittest.TestCase): @unittest.skipIf( - os.environ.get("GITHUB_REPOSITORY", UPSTREAM) != UPSTREAM, + not is_upstream_checkout(), "template-placeholder guard targets the pristine upstream template; forks personalize 05-cv-templates.md and 06-cover-letter-templates.md via /setup", ) class TemplatesStillCarryThePlaceholders(unittest.TestCase): diff --git a/tests/test_upstream_checkout.py b/tests/test_upstream_checkout.py new file mode 100644 index 00000000..58a777ee --- /dev/null +++ b/tests/test_upstream_checkout.py @@ -0,0 +1,105 @@ +"""Unit tests for the shared fork-detection helper (issue #496). + +The placeholder guards must skip on personalized forks even in a local run +where GITHUB_REPOSITORY is unset. All branches are exercised with the env +and subprocess.run patched, so no git remote or CI variable is needed. +""" +import os +import subprocess +import unittest +from unittest import mock + +try: + from tests import _upstream + from tests._upstream import is_upstream_checkout +except ImportError: # `python -m unittest discover -s tests` imports test modules top-level + import _upstream + from _upstream import is_upstream_checkout + +UPSTREAM = "MadsLorentzen/ai-job-search" + + +def _no_github_env(testcase): + """Clear GITHUB_REPOSITORY so the origin-remote branch is exercised.""" + patcher = mock.patch.dict(os.environ) + env = patcher.start() + testcase.addCleanup(patcher.stop) + env.pop("GITHUB_REPOSITORY", None) + return env + + +def _completed(stdout="", returncode=0): + return subprocess.CompletedProcess( + args=["git", "remote", "get-url", "origin"], + returncode=returncode, + stdout=stdout, + ) + + +class UpstreamCheckoutTests(unittest.TestCase): + def test_env_says_upstream(self): + with mock.patch.dict(os.environ, {"GITHUB_REPOSITORY": UPSTREAM}): + with mock.patch.object(_upstream.subprocess, "run") as run: + self.assertTrue(is_upstream_checkout()) + run.assert_not_called() + + def test_env_says_fork(self): + with mock.patch.dict(os.environ, {"GITHUB_REPOSITORY": "someone-else/ai-job-search"}): + with mock.patch.object(_upstream.subprocess, "run") as run: + self.assertFalse(is_upstream_checkout()) + run.assert_not_called() + + def test_no_env_and_origin_is_fork(self): + _no_github_env(self) + with mock.patch.object( + _upstream.subprocess, "run", return_value=_completed("git@github.com:someone-else/ai-job-search.git\n") + ): + self.assertFalse(is_upstream_checkout()) + + def test_no_env_and_git_is_unavailable(self): + _no_github_env(self) + with mock.patch.object( + _upstream.subprocess, "run", side_effect=OSError("no git on PATH") + ): + self.assertTrue(is_upstream_checkout()) + + def test_no_env_and_git_times_out(self): + _no_github_env(self) + with mock.patch.object( + _upstream.subprocess, + "run", + side_effect=subprocess.TimeoutExpired(cmd="git", timeout=10), + ): + self.assertTrue(is_upstream_checkout()) + + def test_no_env_and_git_exits_nonzero(self): + _no_github_env(self) + with mock.patch.object( + _upstream.subprocess, "run", return_value=_completed("", returncode=128) + ): + self.assertTrue(is_upstream_checkout()) + + def test_origin_url_forms(self): + cases = [ + ("https://github.com/MadsLorentzen/ai-job-search", True), + ("https://github.com/MadsLorentzen/ai-job-search.git", True), + ("git@github.com:MadsLorentzen/ai-job-search", True), + ("git@github.com:MadsLorentzen/ai-job-search.git", True), + ("ssh://git@github.com/MadsLorentzen/ai-job-search.git", True), + ("ssh://git@github.com/MadsLorentzen/ai-job-search", True), + ("ssh://git@github.com/someone-else/ai-job-search.git", False), + ("https://github.com/someone-else/ai-job-search.git", False), + ("git@github.com:someone-else/ai-job-search", False), + ("https://gitlab.com/MadsLorentzen/ai-job-search.git", False), + ] + for url, expected in cases: + with self.subTest(url=url): + _no_github_env(self) + with mock.patch.object( + _upstream.subprocess, "run", return_value=_completed(url + "\n") + ): + self.assertEqual(is_upstream_checkout(), expected) + + +if __name__ == "__main__": + unittest.main()