diff --git a/.github/workflows/claim-notice.yml b/.github/workflows/claim-notice.yml index 107ff7245..78205e80c 100644 --- a/.github/workflows/claim-notice.yml +++ b/.github/workflows/claim-notice.yml @@ -17,11 +17,11 @@ on: type: string pr_title: description: "PR title" - required: true + required: false type: string pr_author: description: "PR author login" - required: true + required: false type: string permissions: @@ -51,6 +51,13 @@ jobs: gh label create registry-claim-pending --repo "$GITHUB_REPOSITORY" --color D4C5F9 --description "Claim notice awaits registry ingestion" --force fi + - name: Mark notice for retry + if: github.event_name != 'schedule' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ inputs.pr_number || github.event.pull_request.number }} + run: gh pr edit "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --add-label registry-claim-pending + - name: Post claim notice if: github.event_name != 'schedule' env: @@ -58,10 +65,17 @@ jobs: PR_NUMBER: ${{ github.event.inputs.pr_number || github.event.pull_request.number }} PR_TITLE: ${{ github.event.inputs.pr_title || github.event.pull_request.title }} PR_AUTHOR: ${{ github.event.inputs.pr_author || github.event.pull_request.user.login }} - PENDING_RETRY: ${{ github.event_name == 'workflow_dispatch' && '1' || '0' }} + VERIFY_MERGED_PR: ${{ github.event_name == 'workflow_dispatch' }} REGISTRY_API: https://hol.org/registry/api/v1 run: python3 scripts/post-claim-notice.py + - name: Clear delivered notice from retry queue + if: success() && github.event_name != 'schedule' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ inputs.pr_number || github.event.pull_request.number }} + run: gh pr edit "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --remove-label registry-claim-pending + - name: Retry notices for recently merged plugins if: github.event_name == 'schedule' env: diff --git a/.github/workflows/validate-contribution.yml b/.github/workflows/validate-contribution.yml index 38b77b0dc..1176a3a88 100644 --- a/.github/workflows/validate-contribution.yml +++ b/.github/workflows/validate-contribution.yml @@ -13,6 +13,11 @@ on: - "scripts/publish-open-pr-checks.py" - "scripts/publish-open-pr-checks-centralized.py" - "scripts/test_publish_open_pr_checks.py" + - "scripts/post-claim-notice.py" + - "scripts/backfill-claim-notices.py" + - "tests/test_post_claim_notice.py" + - "tests/test_backfill_claim_notices.py" + - ".github/workflows/claim-notice.yml" - ".github/workflows/sweep-open-prs.yml" - ".github/workflows/validate-contribution.yml" workflow_dispatch: @@ -45,6 +50,7 @@ jobs: python3 -m unittest scripts/test_validate_contribution_branch.py python3 -m unittest scripts/test_publish_open_pr_checks.py python3 -m unittest scripts/test_check_alphabetical.py + python3 -m unittest tests/test_post_claim_notice.py tests/test_backfill_claim_notices.py python3 scripts/check-alphabetical.py README.md --base-ref "$BASE_REF" python3 scripts/validate-contribution-centralized.py \ --base-ref "$BASE_REF" \ diff --git a/scripts/post-claim-notice.py b/scripts/post-claim-notice.py index 1530f03b7..214845358 100755 --- a/scripts/post-claim-notice.py +++ b/scripts/post-claim-notice.py @@ -1,13 +1,9 @@ #!/usr/bin/env python3 """ -Post a "claim your plugin" comment on merged PRs that add plugins present in the -HOL registry but not yet owner-verified. - -Triggers via GitHub Actions on pull_request closed (merged). -Checks the HOL registry catalog API for matching repos, then posts a comment -with claim instructions. If a newly merged repository is present only in the -catalog source and has not reached the live Registry yet, the notice says it is -still syncing and does not send the creator into a claim flow that cannot work. +Post a claim notice on merged PRs that add plugins to the HOL catalog source. + +Merged catalog listings are enough to issue a claim link. Registry lookups only +suppress redundant invitations for repositories whose ownership is verified. """ import json @@ -15,8 +11,14 @@ import re import sys import subprocess +import random +import time import urllib.request import urllib.error +from datetime import datetime, timezone +from email.utils import parsedate_to_datetime +from pathlib import Path +from urllib.parse import urlencode # --- Config --- REGISTRY_API = os.environ.get("REGISTRY_API", "https://hol.org/registry/api/v1") @@ -25,8 +27,13 @@ PR_TITLE = os.environ.get("PR_TITLE", "") PR_AUTHOR = os.environ.get("PR_AUTHOR", "") REPO_FULL = os.environ.get("GITHUB_REPOSITORY", "") -PENDING_RETRY = os.environ.get("PENDING_RETRY") == "1" -PENDING_LABEL = "registry-claim-pending" +MAX_CATALOG_PAGES = 5 +MAX_COMMENT_PAGES = 20 +CATALOG_REPO_CACHE = {} +REGISTRY_MAX_ATTEMPTS = 4 +REGISTRY_MAX_RETRY_DELAY = 60 +REGISTRY_RETRY_STATUSES = {429, 500, 502, 503, 504} +NOTICE_DEFERRED = 75 # Skip titles that aren't new plugin additions SKIP_PATTERNS = [ @@ -61,47 +68,79 @@ ] -def build_comment_body(author: str, repositories=(), registry_ready: bool = True) -> str: - """Invite the author to claim as soon as the catalog pull request merges.""" - del registry_ready - from urllib.parse import urlencode - - repos = sorted({repo for repo in repositories if repo}) - if repos: - claim_links = "\n".join( - "- [Verify ownership of `{repo}`](https://hol.org/guard/plugins?{query})".format( - repo=repo, - query=urlencode( - { - "claim": repo, - "utm_source": "github", - "utm_medium": "pr_comment", - "utm_campaign": "plugin_claim", - "utm_content": "merge_notice", - } - ), - ) - for repo in repos +class RegistryCatalogFetchError(RuntimeError): + """Raised when Registry catalog evidence is unavailable or incomplete.""" + + +class RegistryTemporarilyUnavailable(RegistryCatalogFetchError): + """Raised after bounded retries of a temporary Registry failure.""" + + +class GitHubCommentsFetchError(RuntimeError): + """Raised when existing claim comments cannot be checked completely.""" + + +DEFERRED_NOTICE_MARKERS = ( + "Still syncing", + "Registry sync in progress", + "not live in the HOL Registry", + "No action is needed yet", +) + + +def claim_link(repo: str) -> str: + return ( + "https://hol.org/guard/plugins?" + + urlencode( + { + "claim": repo, + "utm_source": "github", + "utm_medium": "pr_comment", + "utm_campaign": "plugin_claim", + "utm_content": "merge_notice", + } ) - else: - claim_links = "[Open the plugin dashboard](https://hol.org/guard/plugins)" + ) - return f""" -Hey @{author}, your plugin is merged into the HOL catalog and ready to claim. -## Claim your plugin +def build_comment_body(author: str, repositories=(), pending_repositories=()) -> str: + """Invite the author to verify ownership as soon as the catalog PR merges. -{claim_links} + Registry indexing is not a gate. A repository merged into the catalog + source can be claimed before its public listing finishes syncing. + """ + + claimable = sorted(set(repositories) | set(pending_repositories)) + if not claimable: + return f"{MARKER} 🎉 Congrats @{author}, your plugin has been merged and ownership is already verified! [Open your plugin dashboard](https://hol.org/guard/plugins) to view its trust score, installs, and engagement." -Open the link and choose **"Continue with GitHub"**. Use the GitHub account that maintains the repository. HOL requests only `read:user` and `user:email`. It does not request write access to the repository. + claim_links = "; ".join( + f"[Claim your plugin `{repo}`]({claim_link(repo)})" for repo in claimable + ) -After verification, the listing gets an owner-verified badge, and the plugin dashboard shows its trust score, installs, and engagement.""" + return f'{MARKER} 🎉 Congrats @{author}, your plugin has been merged into the catalog! {claim_links}. Choose **"Continue with GitHub"** using the account that maintains the repository to verify ownership, view its trust score, and track installs and engagement. If you have already claimed it, the same link opens your plugin dashboard.' MARKER = "" -def api_request(url, headers=None, method="GET", data=None): +def registry_retry_delay(attempt, retry_after=None): + """Use exponential backoff with jitter and respect Retry-After when valid.""" + delay = 2 ** (attempt + 1) + random.uniform(0, 1) + if retry_after is not None: + try: + seconds = int(retry_after) + except (TypeError, ValueError): + try: + retry_at = parsedate_to_datetime(retry_after) + seconds = (retry_at - datetime.now(timezone.utc)).total_seconds() + except (TypeError, ValueError, OverflowError): + seconds = 0 + delay = max(delay, seconds) + return delay + + +def api_request(url, headers=None, method="GET", data=None, retry_registry=False): """Make an HTTP request and return parsed JSON.""" req_headers = {"Accept": "application/json", "User-Agent": "hol-claim-notice/1.0"} if headers: @@ -110,56 +149,115 @@ def api_request(url, headers=None, method="GET", data=None): data = json.dumps(data).encode("utf-8") req_headers["Content-Type"] = "application/json" req = urllib.request.Request(url, headers=req_headers, method=method, data=data) - try: - with urllib.request.urlopen(req, timeout=30) as resp: - return json.loads(resp.read().decode("utf-8")) - except urllib.error.HTTPError as e: - body = e.read().decode("utf-8", errors="replace") - print(f" HTTP {e.code} from {url}: {body[:200]}", file=sys.stderr) + attempts = REGISTRY_MAX_ATTEMPTS if retry_registry and method == "GET" else 1 + for attempt in range(attempts): + retry_after = None + try: + with urllib.request.urlopen(req, timeout=30) as resp: + return json.loads(resp.read().decode("utf-8")) + except urllib.error.HTTPError as error: + print(f" HTTP {error.code} from {url}", file=sys.stderr) + retry_after = error.headers.get("Retry-After") if error.headers else None + error.close() + if attempts == 1 or error.code not in REGISTRY_RETRY_STATUSES: + return None + except (urllib.error.URLError, TimeoutError) as error: + print(f" Error fetching {url}: {error}", file=sys.stderr) + if attempts == 1: + return None + except Exception as error: + print(f" Error fetching {url}: {error}", file=sys.stderr) + return None + + if attempt + 1 == attempts: + raise RegistryTemporarilyUnavailable("Registry retries exhausted") + delay = registry_retry_delay(attempt, retry_after) + if delay > REGISTRY_MAX_RETRY_DELAY: + raise RegistryTemporarilyUnavailable("Registry Retry-After exceeds retry budget") + print(f" Retrying Registry request in {delay:.1f}s ({attempt + 2}/{attempts})", file=sys.stderr) + time.sleep(delay) + + +def fetch_merged_pr_metadata(): + """Load trusted metadata for a manually dispatched claim notice.""" + url = f"https://api.github.com/repos/{REPO_FULL}/pulls/{PR_NUMBER}" + headers = {"Authorization": f"token {GH_TOKEN}"} + data = api_request(url, headers=headers) + if not isinstance(data, dict): return None - except Exception as e: - print(f" Error fetching {url}: {e}", file=sys.stderr) + if not data.get("merged_at"): return None + title = data.get("title") + user = data.get("user") + author = user.get("login") if isinstance(user, dict) else None + if not isinstance(title, str) or not isinstance(author, str): + return None + return title, author def should_skip_title(title: str) -> bool: """Return True if the PR title matches a non-plugin pattern.""" + # `docs: add ` is a common legitimate contribution title. Allow it + # through the title gate, while retaining specific docs-only exclusions below. + allow_docs_add = bool(re.match(r"^docs?:\s+add\b", title.strip(), re.IGNORECASE)) for pattern in SKIP_PATTERNS: + if pattern == r"^docs?:" and allow_docs_add: + continue if re.search(pattern, title, re.IGNORECASE): return True return False -def fetch_catalog_repos(owner_verified: bool = False): - """Fetch repos from the registry catalog, optionally filtered by owner verification. +def fetch_catalog_repos(repository: str): + """Look up verified ownership for one repository, never the full catalog.""" + cache_key = normalize_repo_url(repository) + cached = CATALOG_REPO_CACHE.get(cache_key) + if cached is not None: + return cached - Returns a set of lowercase 'owner/repo' strings. - """ repos = set() cursor = None - base_url = f"{REGISTRY_API}/plugins/catalog?limit=50" - if owner_verified: - base_url += "&ownerVerified=true" seen_cursors = set() - while True: - url = f"{base_url}&cursor={cursor}" if cursor else base_url - data = api_request(url) - if not data or "items" not in data: - raise RuntimeError("Registry catalog is unavailable; claim notice will be retried") + base_url = f"{REGISTRY_API}/plugins/catalog?" + urlencode( + {"limit": 50, "q": cache_key, "ownerVerified": "true"} + ) + + for page_index in range(MAX_CATALOG_PAGES): + url = base_url if not cursor else f"{base_url}&cursor={cursor}" + data = api_request(url, retry_registry=True) + if not isinstance(data, dict) or not isinstance(data.get("items"), list): + raise RegistryCatalogFetchError( + f"registry catalog page {page_index + 1} unavailable" + ) + for plugin in data["items"]: - # Vendored marketplace plugins have a catalog sourceRepo of - # awesome-codex-plugins; ownership belongs to the author repo. + if not isinstance(plugin, dict): + continue repo = plugin.get("repository") or plugin.get("sourceRepo") or "" - repo = normalize_repo_url(repo) if repo else "" - if repo: - repos.add(repo.lower()) - cursor = data.get("nextCursor") - if not cursor: - break - if cursor in seen_cursors: - raise RuntimeError("Registry catalog cursor repeated; claim notice will be retried") - seen_cursors.add(cursor) - return repos + if not isinstance(repo, str): + continue + if (repo and normalize_repo_url(repo) == cache_key + and plugin.get("ownerVerified") is True): + repos.add(cache_key) + + next_cursor = data.get("nextCursor") + if not next_cursor: + CATALOG_REPO_CACHE[cache_key] = repos + return repos + if not isinstance(next_cursor, str): + raise RegistryCatalogFetchError( + f"registry catalog page {page_index + 1} returned an invalid cursor" + ) + if next_cursor in seen_cursors: + raise RegistryCatalogFetchError( + f"registry catalog repeated cursor {next_cursor!r}" + ) + seen_cursors.add(next_cursor) + cursor = next_cursor + + raise RegistryCatalogFetchError( + f"registry catalog exceeded {MAX_CATALOG_PAGES} pages" + ) def normalize_repo_url(raw: str) -> str: @@ -181,16 +279,6 @@ def normalize_repo_url(raw: str) -> str: return s.lower() -def readme_contains_repo(readme: str, repo: str) -> bool: - """Match owner/repo as its own token, not as a prefix of a longer name.""" - pattern = ( - r"(? bool: + """Replace a deferred sync notice with a claim link.""" + if not isinstance(comment_id, int): + return False + url = f"https://api.github.com/repos/{REPO_FULL}/issues/comments/{comment_id}" + headers = {"Authorization": f"token {GH_TOKEN}"} + body = build_comment_body(author, repositories) + result = api_request(url, headers=headers, method="PATCH", data={"body": body}) + return result is not None + + def main(): + global PR_AUTHOR, PR_TITLE + # Validate required environment variables missing = [] if not GH_TOKEN: @@ -316,26 +445,45 @@ def main(): print(f"Error: missing required environment variables: {', '.join(missing)}", file=sys.stderr) return 1 - print(f'PR #{PR_NUMBER}: "{PR_TITLE}" by @{PR_AUTHOR}') + if os.environ.get("VERIFY_MERGED_PR") == "true": + metadata = fetch_merged_pr_metadata() + if metadata is None: + print("Error: PR metadata is unavailable or the PR is not merged", file=sys.stderr) + return 1 + PR_TITLE, PR_AUTHOR = metadata - # 1. Skip non-plugin PRs - if not PENDING_RETRY and should_skip_title(PR_TITLE): - print(" Skipping: non-plugin PR title pattern") - return 0 + print(f'PR #{PR_NUMBER}: "{PR_TITLE}" by @{PR_AUTHOR}') if PR_AUTHOR in ("kantorcodes", "github-actions[bot]"): print(" Skipping: bot/owner PR") return 0 - # 2. Parse PR diff for GitHub repo URLs + # 2. Check for existing claim comment. A deferred "still syncing" + # notice is replaced once we know which repositories to claim. + existing_notice = False + try: + existing_notice = has_existing_claim_comment() + if existing_notice is True: + print(" Skipping: claim notice already posted") + return 0 + except GitHubCommentsFetchError as error: + print(f" Existing claim comment check failed: {error}", file=sys.stderr) + return 1 + + # Recover an existing deferred notice even if its PR title was later edited. + if not existing_notice and should_skip_title(PR_TITLE): + print(" Skipping: non-plugin PR title pattern") + return 0 + + # 3. Parse PR diff for GitHub repo URLs try: pr_repos = parse_pr_diff_for_repos() except subprocess.CalledProcessError as e: print(f" Failed to get PR diff (exit {e.returncode}): {e.stderr}", file=sys.stderr) - return 0 + return 1 except subprocess.TimeoutExpired: print(" Failed to get PR diff: timed out", file=sys.stderr) - return 0 + return 1 if not pr_repos: print(" Skipping: no GitHub repo URLs found in PR diff") @@ -343,76 +491,63 @@ def main(): print(f" Found repos in diff: {', '.join(pr_repos)}") - set_pending_label(True) - existing_notice = has_existing_claim_comment() - if existing_notice and existing_notice != "pending": - set_pending_label(False) - print(" Skipping: claim-ready notice already posted") - return 0 - - # 4. Fetch all registry plugins - print(" Fetching registry catalog...") - registry_repos = fetch_catalog_repos(owner_verified=False) - print(f" Registry has {len(registry_repos)} plugins") - - # 5. Check which PR repos are in the live registry. If they only appear in - # the merged README source, keep the notice informational until ingestion finishes. - matched = set(pr_repos & registry_repos) - missing = pr_repos - matched - if missing: - print(" Checking README for repositories the Registry has not indexed...") - readme_path = os.path.join(os.path.dirname(os.path.dirname(__file__)), "README.md") - if os.path.exists(readme_path): - readme_content = open(readme_path, encoding="utf-8").read().lower() - readme_matched = { - repo for repo in missing if readme_contains_repo(readme_content, repo) - } - if readme_matched: - print(f" Claimable from catalog source: {', '.join(sorted(readme_matched))}") - matched |= readme_matched - registry_ready = bool(pr_repos & registry_repos) + # Only invite repositories present in the trusted, merged catalog source. + try: + matched = pr_repos & catalog_source_repos() + except OSError as error: + print(f" Could not read merged catalog source: {error}", file=sys.stderr) + return 1 if not matched: - set_pending_label(False) - print(" Skipping: none of the PR repos are in the registry or README") + print(" Skipping: none of the PR repos are listed in the merged catalog source") return 0 - if registry_ready: - print(f" Matched in registry: {', '.join(matched)}") - else: - print(f" Pending registry sync: {', '.join(matched)}") - + # Ownership is enforced by the claim flow. This optional query only avoids + # redundant invitations; unavailable Registry evidence must not drop notices. already_verified = set() - # 6. Check if already owner-verified only when the repository is actually live. - if registry_ready: - print(" Checking owner verification status...") - verified_repos = fetch_catalog_repos(owner_verified=True) - already_verified = matched & verified_repos - if already_verified and len(already_verified) == len(matched): - set_pending_label(False) - print(" Skipping: all matched repos already owner-verified") + for repo in sorted(matched): + try: + already_verified |= fetch_catalog_repos(repo) + except RegistryCatalogFetchError as error: + print(f"::warning::Owner verification lookup unavailable for {repo}: {error}; including its claim/dashboard link.") + + claimable_repos = matched - already_verified + if already_verified: + print(f" Already verified: {', '.join(sorted(already_verified))}") + + if not claimable_repos: + if isinstance(existing_notice, dict) and existing_notice.get("deferred"): + print(" Clearing deferred notice; repositories are already verified") + if update_comment(existing_notice.get("id"), PR_AUTHOR, set()): + print(" ✅ Deferred notice cleared") + return 0 + print(" ❌ Failed to clear deferred notice") + return 1 + print(" Posting verified-owner dashboard notice...") + if post_comment(PR_AUTHOR, set()): + print(" ✅ Dashboard notice posted successfully") return 0 + print(" ❌ Failed to post dashboard notice") + return 1 - if already_verified: - print(f" Some already verified: {', '.join(already_verified)}") - - # A deferred sync notice is replaced by a claim link. Registry indexing - # is not required before the author can verify ownership. - claimable = matched - already_verified - if not claimable: - set_pending_label(False) - print(" Skipping: all matched repos already owner-verified") - return 0 + # 7. Post or replace the comment. Catalog-source repos are claimable + # before the public Registry listing finishes indexing. + if isinstance(existing_notice, dict) and existing_notice.get("deferred"): + print(" Replacing deferred claim notice...") + if update_comment(existing_notice.get("id"), PR_AUTHOR, claimable_repos): + print(" ✅ Comment updated successfully") + return 0 + print(" ❌ Failed to update comment") + return 1 print(" Posting claim notice comment...") - if post_comment(PR_AUTHOR, claimable, registry_ready=registry_ready): - if registry_ready: - set_pending_label(False) + if post_comment(PR_AUTHOR, claimable_repos): print(" ✅ Comment posted successfully") return 0 - else: - print(" ❌ Failed to post comment") - return 1 + + print(" ❌ Failed to post comment") + return 1 if __name__ == "__main__": - sys.exit(main()) + result = main() + sys.exit(0 if result == NOTICE_DEFERRED else result) diff --git a/tests/test_post_claim_notice.py b/tests/test_post_claim_notice.py index 6c03cc2cc..9834f740c 100644 --- a/tests/test_post_claim_notice.py +++ b/tests/test_post_claim_notice.py @@ -1,6 +1,15 @@ import importlib.util +import io +import json +import os +import re +import tempfile import unittest +from datetime import datetime, timezone from pathlib import Path +from subprocess import CompletedProcess +from urllib.error import HTTPError, URLError +from urllib.parse import parse_qs, urlparse from unittest.mock import mock_open, patch SPEC = importlib.util.spec_from_file_location( @@ -10,117 +19,267 @@ SPEC.loader.exec_module(MODULE) -class ClaimNoticeTests(unittest.TestCase): - def test_catalog_uses_author_repository_across_pages(self): - with patch.object(MODULE, "api_request", side_effect=[ - {"items": [{"repository": "https://github.com/author/plugin", "sourceRepo": "https://github.com/owner/catalog"}], "nextCursor": "next"}, - {"items": [{"repository": "https://github.com/another/plugin.git"}], "nextCursor": None}, - ]) as request: - self.assertEqual(MODULE.fetch_catalog_repos(), {"author/plugin", "another/plugin"}) - self.assertEqual(request.call_count, 2) - - def test_comment_lookup_finds_ready_notice_after_pending_on_later_page(self): - first_page = [{"body": " Registry sync in progress"}] - first_page += [{"body": "unrelated"}] * 99 - with patch.multiple(MODULE, GH_TOKEN="fixture", PR_NUMBER="1", REPO_FULL="owner/catalog"), patch.object( - MODULE, "api_request", side_effect=[first_page, [{"body": " Claim your plugin"}]] - ): - self.assertEqual(MODULE.has_existing_claim_comment(), "ready") - - def test_catalog_repository_is_claimable_before_registry_sync(self): - body = MODULE.build_comment_body("author", {"owner/pending"}, registry_ready=False) - self.assertIn("ready to claim", body) - self.assertIn("claim=owner%2Fpending", body) - self.assertNotIn("Registry sync in progress", body) - self.assertNotIn("read:org", body) - - def test_live_repository_notice_retains_claim_link(self): - body = MODULE.build_comment_body("author", {"owner/live"}, registry_ready=True) - self.assertIn("ready to claim", body) - self.assertIn("claim=owner%2Flive", body) - self.assertNotIn("read:org", body) - - def test_readme_only_repository_posts_pending_notice(self): +class DeliveryRegressionTests(unittest.TestCase): + def test_catalog_page_limit_cannot_block_a_merged_listing_notice(self): with patch.multiple( - MODULE, - GH_TOKEN="fixture", - PR_NUMBER="1", - REPO_FULL="owner/catalog", - PR_AUTHOR="author", - PR_TITLE="Add plugin", - ), patch.object( - MODULE, "has_existing_claim_comment", return_value=False + MODULE, GH_TOKEN="fixture", PR_NUMBER="993", REPO_FULL="owner/catalog", + PR_AUTHOR="author", PR_TITLE="Add plugin", + ), patch.object(MODULE, "has_existing_claim_comment", return_value=False), patch.object( + MODULE, "parse_pr_diff_for_repos", return_value={"owner/plugin"} ), patch.object( - MODULE, "parse_pr_diff_for_repos", return_value={"owner/pending"} - ), patch.object( - MODULE, "fetch_catalog_repos", return_value=set() + MODULE, "fetch_catalog_repos", + side_effect=MODULE.RegistryCatalogFetchError("registry catalog exceeded 100 pages"), ), patch( - "builtins.open", mock_open(read_data="https://github.com/owner/pending") - ), patch.object(MODULE, "set_pending_label") as label, patch.object(MODULE, "post_comment", return_value=True) as post: + "builtins.open", mock_open(read_data="- [Plugin](https://github.com/owner/plugin) - Plugin") + ), patch.object(MODULE, "post_comment", return_value=True) as post: self.assertEqual(MODULE.main(), 0) - post.assert_called_once_with("author", {"owner/pending"}, registry_ready=False) - label.assert_called_once_with(True) + post.assert_called_once_with("author", {"owner/plugin"}) - def test_live_repository_posts_claim_ready_notice(self): - with patch.multiple( - MODULE, - GH_TOKEN="fixture", - PR_NUMBER="1", - REPO_FULL="owner/catalog", - PR_AUTHOR="author", - PR_TITLE="Add plugin", - ), patch.object( - MODULE, "has_existing_claim_comment", return_value=False - ), patch.object( - MODULE, "parse_pr_diff_for_repos", return_value={"owner/live"} - ), patch.object( - MODULE, "fetch_catalog_repos", side_effect=[{"owner/live"}, set()] - ), patch.object(MODULE, "set_pending_label") as label, patch.object(MODULE, "post_comment", return_value=True) as post: - self.assertEqual(MODULE.main(), 0) - post.assert_called_once_with("author", {"owner/live"}, registry_ready=True) - self.assertEqual([call.args for call in label.call_args_list], [(True,), (False,)]) - def test_pending_notice_gets_claim_ready_followup_after_ingestion(self): - with patch.multiple( - MODULE, - GH_TOKEN="fixture", - PR_NUMBER="1", - REPO_FULL="owner/catalog", - PR_AUTHOR="author", - PR_TITLE="Add plugin", - ), patch.object( - MODULE, "has_existing_claim_comment", return_value="pending" - ), patch.object( - MODULE, "parse_pr_diff_for_repos", return_value={"owner/live"} - ), patch.object( - MODULE, "fetch_catalog_repos", side_effect=[{"owner/live"}, set()] - ), patch.object(MODULE, "set_pending_label") as label, patch.object( - MODULE, "post_comment", return_value=True - ) as post: - self.assertEqual(MODULE.main(), 0) - post.assert_called_once_with("author", {"owner/live"}, registry_ready=True) - self.assertEqual([call.args for call in label.call_args_list], [(True,), (False,)]) +class RegistryLookupTests(unittest.TestCase): + def setUp(self): + self.sleep = patch.object(MODULE.time, "sleep").start() + patch.object(MODULE.random, "uniform", return_value=0).start() + patch.dict(MODULE.CATALOG_REPO_CACHE, clear=True).start() + self.addCleanup(patch.stopall) - def test_retry_ignores_later_title_edit_to_skip_pattern(self): - with patch.multiple( - MODULE, - GH_TOKEN="fixture", - PR_NUMBER="1", - REPO_FULL="owner/catalog", - PR_AUTHOR="author", - PR_TITLE="fix: update title", - PENDING_RETRY=True, - ), patch.object( - MODULE, "has_existing_claim_comment", return_value="pending" - ), patch.object( - MODULE, "parse_pr_diff_for_repos", return_value={"owner/live"} - ), patch.object( - MODULE, "fetch_catalog_repos", side_effect=[{"owner/live"}, set()] - ), patch.object(MODULE, "set_pending_label"), patch.object( - MODULE, "post_comment", return_value=True - ) as post: + @staticmethod + def response(data): + return io.BytesIO(json.dumps(data).encode()) + + @staticmethod + def http_error(code, retry_after=None): + headers = {} if retry_after is None else {"Retry-After": retry_after} + return HTTPError("https://registry.example/catalog", code, "failure", headers, None) + + def test_lookup_targets_exact_verified_repository_and_caches_result(self): + data = {"items": [ + {"sourceRepo": "Owner/Plugin", "ownerVerified": True}, + {"sourceRepo": "owner/plugin-extra", "ownerVerified": True}, + ]} + with patch.object(MODULE, "api_request", return_value=data) as fetch: + self.assertEqual(MODULE.fetch_catalog_repos("Owner/Plugin"), {"owner/plugin"}) + self.assertEqual(MODULE.fetch_catalog_repos("owner/plugin"), {"owner/plugin"}) + fetch.assert_called_once() + self.assertEqual(parse_qs(urlparse(fetch.call_args.args[0]).query), + {"q": ["owner/plugin"], "ownerVerified": ["true"], "limit": ["50"]}) + + def test_vendored_repository_identity_takes_precedence(self): + with patch.object(MODULE, "api_request", return_value={"items": [{ + "sourceRepo": "owner/catalog", "repository": "https://github.com/Author/Plugin.git", + "ownerVerified": True, + }]}): + self.assertEqual(MODULE.fetch_catalog_repos("author/plugin"), {"author/plugin"}) + + def test_unverified_or_incomplete_evidence_cannot_suppress_an_invitation(self): + with patch.object(MODULE, "api_request", return_value={"items": [ + {"sourceRepo": "owner/plugin", "ownerVerified": False}, {"sourceRepo": "owner/plugin"}, + ]}): + self.assertEqual(MODULE.fetch_catalog_repos("owner/plugin"), set()) + + def test_filtered_pagination_and_rate_limit_retry_preserve_query(self): + replies = [self.response({"items": [], "nextCursor": "50"}), self.http_error(429, "7"), + self.response({"items": [{"sourceRepo": "owner/plugin", "ownerVerified": True}]})] + with patch.object(MODULE.urllib.request, "urlopen", side_effect=replies) as fetch: + self.assertEqual(MODULE.fetch_catalog_repos("owner/plugin"), {"owner/plugin"}) + self.assertEqual(fetch.call_count, 3) + self.assertEqual(fetch.call_args_list[-2].args[0].full_url, fetch.call_args.args[0].full_url) + query = parse_qs(urlparse(fetch.call_args.args[0].full_url).query) + self.assertEqual(query["q"], ["owner/plugin"]) + self.assertEqual(query["cursor"], ["50"]) + self.sleep.assert_called_once_with(7) + + def test_failed_or_incomplete_queries_are_not_cached(self): + for replies in ([None], [{"items": [], "nextCursor": "next"}, None], + [{"items": [], "nextCursor": "next"}] * 2): + with self.subTest(replies=replies), patch.object(MODULE, "api_request", side_effect=replies): + with self.assertRaises(MODULE.RegistryCatalogFetchError): + MODULE.fetch_catalog_repos("owner/plugin") + self.assertEqual(MODULE.CATALOG_REPO_CACHE, {}) + + def test_search_is_bounded_even_if_the_server_ignores_the_query(self): + pages = [{"items": [], "nextCursor": str(i)} for i in range(1, MODULE.MAX_CATALOG_PAGES + 1)] + with patch.object(MODULE, "api_request", side_effect=pages) as fetch: + with self.assertRaises(MODULE.RegistryCatalogFetchError): + MODULE.fetch_catalog_repos("owner/plugin") + self.assertEqual(fetch.call_count, MODULE.MAX_CATALOG_PAGES) + self.assertEqual(MODULE.CATALOG_REPO_CACHE, {}) + + def test_temporary_failures_exhaust_bounded_retries(self): + for failure in (429, 500, 502, 503, 504, "network", "timeout"): + with self.subTest(failure=failure): + self.sleep.reset_mock() + errors = [URLError("unavailable") if failure == "network" else + TimeoutError("timed out") if failure == "timeout" else + self.http_error(failure) for _ in range(MODULE.REGISTRY_MAX_ATTEMPTS)] + with patch.object(MODULE.urllib.request, "urlopen", side_effect=errors): + with self.assertRaises(MODULE.RegistryTemporarilyUnavailable): + MODULE.fetch_catalog_repos("owner/plugin") + self.assertEqual([c.args[0] for c in self.sleep.call_args_list], [2, 4, 8]) + + def test_retry_after_http_date_and_invalid_header(self): + with patch.object(MODULE, "datetime") as clock: + clock.now.return_value = datetime(2026, 10, 24, tzinfo=timezone.utc) + self.assertEqual(MODULE.registry_retry_delay(0, "Sat, 24 Oct 2026 00:00:12 GMT"), 12) + self.assertEqual(MODULE.registry_retry_delay(0, "invalid"), 2) + + def test_long_retry_after_does_not_exceed_budget(self): + with patch.object(MODULE.urllib.request, "urlopen", side_effect=self.http_error(429, "120")) as fetch: + with self.assertRaises(MODULE.RegistryTemporarilyUnavailable): + MODULE.fetch_catalog_repos("owner/plugin") + fetch.assert_called_once() + self.sleep.assert_not_called() + + def test_github_writes_are_not_retried_to_avoid_duplicate_comments(self): + for method in ("POST", "PATCH"): + with self.subTest(method=method), patch.object(MODULE.urllib.request, "urlopen", side_effect=self.http_error(429)) as fetch: + self.assertIsNone(MODULE.api_request("https://api.github.com/example", method=method, data={"body": "notice"})) + fetch.assert_called_once() + self.sleep.assert_not_called() + + +class ClaimNoticeTests(unittest.TestCase): + def setUp(self): + patch.dict(os.environ, {}, clear=True).start() + patch.multiple(MODULE, GH_TOKEN="fixture", PR_NUMBER="1", REPO_FULL="owner/catalog", + PR_AUTHOR="author", PR_TITLE="Add plugin").start() + self.existing = patch.object(MODULE, "has_existing_claim_comment", return_value=False).start() + patch.object(MODULE, "parse_pr_diff_for_repos", return_value={"owner/plugin"}).start() + self.source = patch.object(MODULE, "catalog_source_repos", return_value={"owner/plugin"}).start() + self.lookup = patch.object(MODULE, "fetch_catalog_repos", return_value=set()).start() + self.post = patch.object(MODULE, "post_comment", return_value=True).start() + self.update = patch.object(MODULE, "update_comment", return_value=True).start() + self.addCleanup(patch.stopall) + + def test_unindexed_listing_gets_a_claim_link(self): + self.assertEqual(MODULE.main(), 0) + self.lookup.assert_called_once_with("owner/plugin") + self.post.assert_called_once_with("author", {"owner/plugin"}) + + def test_registry_outage_does_not_block_delivery(self): + self.lookup.side_effect = MODULE.RegistryTemporarilyUnavailable("retry budget exhausted") + self.assertEqual(MODULE.main(), 0) + self.post.assert_called_once_with("author", {"owner/plugin"}) + + def test_verified_owner_receives_a_dashboard_confirmation(self): + self.lookup.return_value = {"owner/plugin"} + self.assertEqual(MODULE.main(), 0) + self.post.assert_called_once_with("author", set()) + + def test_verified_owner_deferred_notice_is_replaced_with_dashboard_link(self): + self.lookup.return_value = {"owner/plugin"} + self.existing.return_value = {"id": 123, "deferred": True} + self.assertEqual(MODULE.main(), 0) + self.update.assert_called_once_with(123, "author", set()) + self.post.assert_not_called() + + def test_existing_notice_prevents_duplicate_delivery(self): + self.existing.return_value = True + self.assertEqual(MODULE.main(), 0) + self.post.assert_not_called() + self.lookup.assert_not_called() + + def test_old_pending_notice_is_updated_in_place(self): + self.existing.return_value = {"id": 123, "deferred": True} + self.assertEqual(MODULE.main(), 0) + self.update.assert_called_once_with(123, "author", {"owner/plugin"}) + self.post.assert_not_called() + + def test_pending_notice_survives_a_later_title_change(self): + self.existing.return_value = {"id": 123, "deferred": True} + with patch.object(MODULE, "PR_TITLE", "fix: renamed contribution"): self.assertEqual(MODULE.main(), 0) - post.assert_called_once_with("author", {"owner/live"}, registry_ready=True) + self.update.assert_called_once_with(123, "author", {"owner/plugin"}) + + def test_unlisted_diff_links_cannot_receive_claim_notices(self): + self.source.return_value = {"owner/plugin-extra"} + self.assertEqual(MODULE.main(), 0) + self.post.assert_not_called() + self.lookup.assert_not_called() + + def test_unreadable_catalog_is_reported_for_retry(self): + self.source.side_effect = OSError("unavailable") + self.assertEqual(MODULE.main(), 1) + self.post.assert_not_called() + + def test_failed_duplicate_check_cannot_write_another_comment(self): + self.existing.side_effect = MODULE.GitHubCommentsFetchError("unavailable") + self.assertEqual(MODULE.main(), 1) + self.post.assert_not_called() + + def test_failed_comment_write_is_reported_for_retry(self): + self.post.return_value = False + self.assertEqual(MODULE.main(), 1) + + +class NoticeEvidenceTests(unittest.TestCase): + def test_source_accepts_exact_listing_links_only(self): + readme = "\n".join([ + "- [Plugin](https://github.com/Owner/Plugin.git/tree/main) - Plugin", + "Documentation mentions https://github.com/other/unlisted", + ]) + with patch("builtins.open", mock_open(read_data=readme)): + self.assertEqual(MODULE.catalog_source_repos(), {"owner/plugin"}) + + def test_local_mirror_uses_trusted_upstream_identity_instead_of_folder_name(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + (root / "README.md").write_text("- [Mirror](./plugins/author/alias) - Plugin") + manifest = root / "plugins" / "author" / "alias" / ".codex-plugin" / "plugin.json" + manifest.parent.mkdir(parents=True) + manifest.write_text(json.dumps({"repository": "https://github.com/author/upstream.git"})) + with patch.object(MODULE, "__file__", str(root / "scripts" / "post-claim-notice.py")): + self.assertEqual(MODULE.catalog_source_repos(), {"author/upstream"}) + + def test_local_listing_cannot_read_metadata_outside_the_catalog(self): + with patch("builtins.open", mock_open(read_data="- [Bad](./plugins/../../outside) - Plugin")): + with self.assertRaises(OSError): + MODULE.catalog_source_repos() + + def test_manual_dispatch_requires_trusted_merged_metadata(self): + for merged_at, expected in ((None, None), ("2026-10-26T00:00:00Z", ("Add plugin", "author"))): + with self.subTest(merged_at=merged_at), patch.object(MODULE, "api_request", return_value={ + "merged_at": merged_at, "title": "Add plugin", "user": {"login": "author"}, + }): + self.assertEqual(MODULE.fetch_merged_pr_metadata(), expected) + + def test_ready_notice_on_later_page_takes_precedence_over_old_pending_notice(self): + first = [{"id": 1, "body": MODULE.MARKER + " Still syncing"}] * 100 + second = [{"id": 2, "body": MODULE.build_comment_body("author", {"owner/plugin"})}] + with patch.object(MODULE, "api_request", side_effect=[first, second]): + self.assertIs(MODULE.has_existing_claim_comment(), True) + + def test_comment_lookup_errors_are_not_treated_as_absence(self): + with patch.object(MODULE, "api_request", return_value=None): + with self.assertRaises(MODULE.GitHubCommentsFetchError): + MODULE.has_existing_claim_comment() + + def test_notice_is_one_paragraph_with_attributed_repository_links(self): + body = MODULE.build_comment_body("author", ["owner/second", "owner/first"]) + self.assertNotIn("\n", body) + self.assertNotIn("\\n", body) + self.assertIn("@author", body) + self.assertIn("Claim your plugin", body) + self.assertIn("Continue with GitHub", body) + links = re.findall(r"https://hol.org/guard/plugins\?[^)]+", body) + queries = [parse_qs(urlparse(link).query) for link in links] + self.assertEqual([q["claim"][0] for q in queries], ["owner/first", "owner/second"]) + self.assertTrue(all(q["utm_campaign"] == ["plugin_claim"] for q in queries)) + self.assertNotIn("is now listed", body) + + def test_already_verified_notice_retains_dashboard_link(self): + body = MODULE.build_comment_body("author") + self.assertIn("[Open your plugin dashboard]", body) + self.assertIn("already verified", body) + self.assertNotIn("?claim=", body) + + def test_diff_ignores_moved_listing_lines(self): + diff = "\n".join(["+++ b/README.md", + "-- [Old](https://github.com/owner/existing) - Old", + "+- [Old](https://github.com/owner/existing) - Old", + "+- [New](https://github.com/owner/new) - New"]) + with patch.object(MODULE.subprocess, "run", return_value=CompletedProcess([], 0, stdout=diff)): + self.assertEqual(MODULE.parse_pr_diff_for_repos(), {"owner/new"}) if __name__ == "__main__":