From dcea1ae425465e9804e691e1db8f5235ae4604f4 Mon Sep 17 00:00:00 2001 From: openhands Date: Wed, 23 Sep 2026 05:41:30 +0000 Subject: [PATCH] fix(review): bound the unrequested scan to a rotating KV-backed window Restacked onto the current #659 head (openhands/issue-656). This keeps the continuous unrequested green-PR discovery and the corrected base behavior it builds on: scheduled candidates gate on GitHub-required checks (falling back to every current-head check and workflow run when the required signal is unavailable), and an explicit all-hands-bot request bypasses the CI gate. The scheduled scan examined every open, non-draft PR. Classifying each unrequested head costs a review read plus the exact-head check and workflow reads, so one run read one list per PR across the largest repository and posted a managed gate comment for every red or pending head, which the live canary of - The unrequested part of a scheduled scan is now a bounded, rotating window: at most SCAN_WINDOW (10) unrequested PRs per repository, starting where the previous scan stopped. The position is retained in the existing Automation KV store under a per-repository review-scan:{owner}__{repo} key, so successive scans rotate through the whole backlog instead of reading one pull request per open PR. With no KV store the position is kept in memory. - Explicit all-hands-bot review requests and trigger labels are never subject to the window: every explicit candidate is examined on every scan, and an explicit request still bypasses the CI gate. - A gate stop on an unrequested head posts no managed comment, so a scan over a large backlog cannot storm the PRs with comments. The managed comment stays for explicit requests and labeled heads, which still get their blocked or waiting explanation. - Remove the one-off .pr canary script and bump the catalog entry and bundle 1.7.0 -> 1.8.0. Tests cover rotation and KV persistence, explicit priority over the window, the absence of unrequested gate comments, and the bounded pull-request reads, alongside the base required-check and explicit-request-bypass tests. --- automations/bundle-index.js | 2 +- .../catalog/github-pr-reviewer/manifest.json | 12 +- skills/github-pr-reviewer/README.md | 11 + skills/github-pr-reviewer/SKILL.md | 56 +- .../references/state-schema.md | 15 + skills/github-pr-reviewer/scripts/worker.py | 324 ++++++++-- skills/index.js | 2 +- .../automations/github-pr-reviewer.json | 18 +- tests/test_github_reviewer_delivery.py | 568 +++++++++++++++++- 9 files changed, 919 insertions(+), 89 deletions(-) diff --git a/automations/bundle-index.js b/automations/bundle-index.js index 84bd26a9..6741bdc3 100644 --- a/automations/bundle-index.js +++ b/automations/bundle-index.js @@ -8,7 +8,7 @@ export const AUTOMATION_BUNDLE_FILES = { "github_client.py": "\"\"\"Shared GitHub transport and repository operations for GitHub automations.\"\"\"\n\nimport argparse\nimport json\nimport os\nimport re\nimport subprocess\nfrom functools import cached_property\nfrom pathlib import Path\nfrom urllib.error import HTTPError\nfrom urllib.parse import parse_qsl, urlencode, urlsplit\nfrom urllib.request import Request, urlopen\n\n\ndef _load_secret(name: str) -> str:\n \"\"\"Read one named secret from the environment or configured Agent Server.\"\"\"\n value = os.environ.get(name)\n if value:\n return value\n\n from openhands.sdk.workspace import RemoteWorkspace\n\n workspace = RemoteWorkspace(\n host=os.environ[\"AGENT_SERVER_URL\"],\n api_key=os.environ[\"SESSION_API_KEY\"],\n working_dir=os.environ.get(\"WORKSPACE_BASE\", \"/workspace\"),\n )\n try:\n secret = workspace.get_secrets([name]).get(name)\n value = secret.get_value() if secret else None\n finally:\n workspace.reset_client()\n if not value:\n raise ValueError(f\"The GitHub credential {name} is unavailable\")\n return value\n\n\ndef github_request(\n token: str,\n method: str,\n path: str,\n params: dict | None = None,\n body: dict | None = None,\n accept: str = \"application/vnd.github+json\",\n) -> tuple:\n url = f\"https://api.github.com{path}\"\n if params:\n url = f\"{url}?{urlencode(params)}\"\n headers = {\n \"Authorization\": f\"Bearer {token}\",\n \"Accept\": accept,\n \"X-GitHub-Api-Version\": \"2022-11-28\",\n \"Content-Type\": \"application/json\",\n }\n data = json.dumps(body).encode() if body is not None else None\n req = Request(url, data=data, headers=headers, method=method)\n with urlopen(req, timeout=90) as r:\n raw = r.read()\n return (json.loads(raw) if raw.strip() else {}), dict(r.headers)\n\n\ndef github_paginate(token: str, path: str, params: dict | None = None) -> list:\n results = []\n base_params = dict(params or {})\n base_params.setdefault(\"per_page\", 100)\n for page in range(1, 101):\n base_params[\"page\"] = page\n data, _ = github_request(token, \"GET\", path, params=base_params)\n if not isinstance(data, list):\n raise TypeError(\"Expected a paginated GitHub list\")\n results.extend(data)\n if len(data) < int(base_params[\"per_page\"]):\n return results\n raise RuntimeError(\"GitHub pagination exceeded limit\")\n\n\nclass GitHubRepository:\n name = \"GitHub automation\"\n\n def __init__(\n self,\n config_path=Path(\"config.json\"),\n *,\n github_token_secret,\n repository=None,\n conversation=None,\n dispatcher=None,\n ):\n self.config = json.loads(Path(config_path).read_text())\n self.repository = repository or self.config[\"repository\"]\n if not re.fullmatch(r\"[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+\", self.repository):\n raise ValueError(\"repository must be owner/repo\")\n if not re.fullmatch(r\"[A-Z_][A-Z0-9_]*\", github_token_secret):\n raise ValueError(\n \"Expected the environment variable containing the GitHub token\"\n )\n self.token_name = github_token_secret\n self.token = _load_secret(github_token_secret)\n self.conversation = conversation\n self.conversation_id = str(conversation.id) if conversation else None\n self.dispatcher = dispatcher\n self.workspace = Path(os.environ[\"WORKSPACE_BASE\"])\n self.project = self.workspace\n self.evidence = self.workspace / \"evidence\"\n self.evidence.mkdir(exist_ok=True)\n self._completed_dependencies = {}\n\n @cached_property\n def base_branch(self):\n return self.config.get(\"base_branch\") or self.gh(\"GET\", \"\")[\"default_branch\"]\n\n @property\n def github_instructions(self):\n return (\n f\"Use `GH_TOKEN=${self.token_name} gh api` for GitHub requests. \"\n \"Never print the credential value. \"\n f\"Only {self.repository} is in scope. Work in {self.project}. \"\n \"Do not modify the automation bundle or its configuration.\"\n )\n\n def gh(self, method, path, body=None):\n return github_request(\n self.token, method, f\"/repos/{self.repository}\" + path, body=body\n )[0]\n\n def api(self, method, path, params=None, body=None):\n \"\"\"Call a GitHub endpoint that is not scoped to one repository.\"\"\"\n return github_request(self.token, method, path, params=params, body=body)[0]\n\n def shell(self, args, cwd=None, timeout=300):\n result = subprocess.run(\n args,\n cwd=cwd or self.project,\n text=True,\n stdout=subprocess.PIPE,\n stderr=subprocess.STDOUT,\n timeout=timeout,\n check=False,\n )\n if result.returncode:\n raise RuntimeError(\n f\"{args[0]} failed: {result.stdout[-4000:].replace(self.token, '[REDACTED]')}\"\n )\n return result.stdout.strip()\n\n def comment(self, number, text):\n return self.gh(\n \"POST\",\n f\"/issues/{number}/comments\",\n {\n \"body\": text\n + f\"\\n\\nFactory role: `{self.name}`; conversation: `{self.conversation_id}`.\"\n + \"\\n\\n_This comment was posted by an AI agent (OpenHands)._\"\n },\n )\n\n def open_issues(self):\n return [\n i for i in self.gh_pages(\"/issues?state=open\") if \"pull_request\" not in i\n ]\n\n def statuses(self, sha):\n result = {}\n for item in self.gh_pages(f\"/commits/{sha}/statuses\"):\n result.setdefault(item[\"context\"], item[\"state\"])\n return result\n\n def check_runs(self, sha):\n \"\"\"Return every check run GitHub reported for one commit SHA.\n\n Reading check runs needs no branch-protection or ruleset access, so this\n is the head-eligibility signal the automation's own token can always\n see. The endpoint answers with an object rather than a list, so it\n paginates through ``gh`` instead of ``gh_pages``.\n \"\"\"\n runs = []\n for page in range(1, 101):\n data = self.gh(\n \"GET\", f\"/commits/{sha}/check-runs?per_page=100&page={page}\"\n )\n batch = data.get(\"check_runs\") or []\n runs.extend(batch)\n if len(runs) >= int(data.get(\"total_count\") or 0) or not batch:\n return runs\n raise RuntimeError(\"GitHub check-run pagination exceeded limit\")\n\n def required_check_contexts(self, number):\n \"\"\"Return the status contexts GitHub marks required on one pull request.\n\n `isRequired` is the merge-policy source of truth and is PR-scoped, so it\n is correct for a stacked PR whose symbolic base branch carries no branch\n rules of its own. It is a field on each context in the head's status\n check rollup and takes the pull request number, so a single GraphQL query\n answers with the required check runs and any required commit statuses,\n each already attributed to the exact head. Only contexts GitHub itself\n reports as required are returned, so an optional workflow that fails\n before creating any check run is not present.\n \"\"\"\n owner, name = self.repository.split(\"/\", 1)\n query = (\n \"query($owner:String!,$name:String!,$number:Int!){\"\n \"repository(owner:$owner,name:$name){\"\n \"pullRequest(number:$number){\"\n \"commits(last:1){\"\n \"nodes{\"\n \"commit{\"\n \"statusCheckRollup{\"\n \"contexts(first:100){\"\n \"pageInfo{hasNextPage}\"\n \"nodes{\"\n \"__typename\"\n \" ... on CheckRun{name isRequired(pullRequestNumber:$number)}\"\n \" ... on StatusContext{context isRequired(pullRequestNumber:$number)}\"\n \"}\"\n \"}\"\n \"}\"\n \"}\"\n \"}\"\n \"}\"\n \"}\"\n \"}\"\n \"}\"\n )\n data = self.api(\n \"POST\",\n \"/graphql\",\n body={\n \"query\": query,\n \"variables\": {\"owner\": owner, \"name\": name, \"number\": number},\n },\n )\n if data.get(\"errors\"):\n raise RuntimeError(\n f\"GitHub required-check query failed: {data['errors'][0].get('message')}\"\n )\n nodes = (\n (((data.get(\"data\") or {}).get(\"repository\") or {}).get(\"pullRequest\") or {})\n .get(\"commits\", {})\n .get(\"nodes\")\n ) or []\n contexts = (\n ((nodes[0].get(\"commit\") or {}).get(\"statusCheckRollup\") or {}).get(\n \"contexts\", {}\n )\n if nodes\n else {}\n )\n if (contexts.get(\"pageInfo\") or {}).get(\"hasNextPage\"):\n raise RuntimeError(\n \"GitHub required-check rollup exceeds one page; the required \"\n \"set may be incomplete\"\n )\n return [\n {\n \"name\": node.get(\"name\") or node.get(\"context\"),\n \"kind\": node.get(\"__typename\"),\n }\n for node in (contexts.get(\"nodes\") or [])\n if node.get(\"isRequired\") and (node.get(\"name\") or node.get(\"context\"))\n ]\n\n def workflow_runs(self, sha):\n \"\"\"Return every Actions workflow run GitHub reported for one commit SHA.\n\n A workflow that fails before any job starts - a workflow-level error, or\n a `pull_request` run whose jobs never materialize - still records a\n failed check suite, but it contributes no check runs, so the commit's\n check-run rollup and `gh pr checks` both report success. Reading the\n workflow runs directly is the only way the gate can see that failure.\n Like the check-run endpoint this answers with an object, so it paginates\n manually.\n \"\"\"\n runs = []\n for page in range(1, 101):\n data = self.gh(\n \"GET\", f\"/actions/runs?head_sha={sha}&per_page=100&page={page}\"\n )\n batch = data.get(\"workflow_runs\") or []\n runs.extend(batch)\n if len(runs) >= int(data.get(\"total_count\") or 0) or not batch:\n return runs\n raise RuntimeError(\"GitHub workflow-run pagination exceeded limit\")\n\n def completed_dependency(self, number):\n if number in self._completed_dependencies:\n return self._completed_dependencies[number]\n try:\n dependency = self.gh(\"GET\", f\"/issues/{number}\")\n except HTTPError as exc:\n if exc.code == 404:\n return False\n raise\n completed = (\n dependency[\"state\"] == \"closed\"\n and dependency.get(\"state_reason\") == \"completed\"\n )\n self._completed_dependencies[number] = completed\n return completed\n\n def dependencies_complete(self, issue):\n \"\"\"Honor explicit Depends on lines; unknown/incomplete issues remain blocked.\"\"\"\n for line in re.findall(\n \"^Depends on:\\\\s*(.+)$\",\n issue.get(\"body\") or \"\",\n re.MULTILINE | re.IGNORECASE,\n ):\n for number in re.findall(\"#(\\\\d+)\", line):\n if not self.completed_dependency(number):\n return False\n return True\n\n def gh_pages(self, endpoint):\n split = urlsplit(endpoint)\n return github_paginate(\n self.token,\n f\"/repos/{self.repository}\" + split.path,\n params=dict(parse_qsl(split.query)),\n )\n\n\ndef run_repositories(automation_type, conversation=None, dispatcher=None):\n parser = argparse.ArgumentParser(description=automation_type.__doc__)\n parser.add_argument(\"--github-token-secret\")\n args = parser.parse_args()\n config = json.loads(Path(\"config.json\").read_text())\n token_name = args.github_token_secret or config.get(\n \"github_token_secret\", \"GITHUB_PERSONAL_ACCESS_TOKEN\"\n )\n repositories = config.get(\"repos\") or [config[\"repository\"]]\n failures = []\n for repository in repositories:\n options = dict(\n github_token_secret=token_name,\n repository=repository,\n conversation=conversation,\n )\n if dispatcher is not None:\n options[\"dispatcher\"] = dispatcher\n automation = automation_type(**options)\n try:\n automation.run()\n except Exception as exc: # noqa: BLE001 - one repository must not block others\n failures.append(repository)\n print(\n json.dumps({\"repository\": repository, \"error\": type(exc).__name__}),\n flush=True,\n )\n if failures:\n raise RuntimeError(\"Automation failed for: \" + \", \".join(failures))\n return str(conversation.id) if conversation else None\n", "main.py": "\"\"\"\nGitHub PR Reviewer - OpenHands Automation Script\n\nCron-polls one or more GitHub repositories for open pull requests carrying the\nconfigured trigger label. A review is queued only when the latest matching\nGitHub `labeled` event has not already been processed by this automation.\n\nEach repository is polled independently and keeps its own state document, so\npull-request numbers never collide across repositories.\n\nThis standalone script owns the repository checkout: it downloads the pull\nrequest's head commit as a tarball, hands the agent that directory as its\nworkspace, and removes it once the review has finished. Catalog workers may\ninstead reuse its prompt builder with their own workspace instructions.\n\"\"\"\n\nimport io\nimport json\nimport os\nimport re\nimport shutil\nimport sys\nimport tarfile\nimport time\nimport urllib.error\nimport urllib.request\nfrom collections.abc import Callable\nfrom pathlib import Path, PurePosixPath\n\nfrom github_client import github_request as _github_request\nfrom github_client import github_paginate as _github_paginate\n\n# Configuration. Two setup paths write it, and both end up here:\n#\n# - the agent-driven path (SKILL.md) substitutes these constants directly\n# into a copy of this file before packaging it;\n# - the catalog path packs an unmodified copy and ships a rendered\n# config.json beside it, which is loaded over these defaults below.\n#\n# A declarative host cannot rewrite Python - the catalog schema admits data,\n# not code - so the constants stay as the defaults and config.json is the\n# override, rather than one path being expressed in terms of the other.\nREPOS = [\"owner/repo\"]\nTRIGGER_LABEL = \"openhands-review\"\nREVIEW_TONE = \"thorough\"\nREVIEW_STYLE_INSTRUCTIONS = \"\"\n# Path within the checked-out repository to a repo-specific review guide\n# (e.g. the repo's own code-review skill). When the file exists at this path\n# relative to the repo root, its contents are read and injected verbatim into\n# the review prompt so the guide is always applied deterministically, rather\n# than relying on the spawned agent's skill activation. Set to \"\" to disable.\nREPO_REVIEW_GUIDE_PATH = \".agents/skills/custom-codereview-guide.md\"\nDEFAULT_OPENHANDS_URL = \"http://localhost:8000\"\n# The most new review conversations one scheduled scan may start, counted across\n# every configured repository rather than per repository. The scheduled scan\n# drains outstanding reviewer requests in oldest-request order, so a small bound\n# keeps a first scan over a large backlog from starting an agent for every\n# eligible pull request at once.\nMAX_NEW_PER_RUN = 2\n\n# A review that ends with this marker is a scope stop: the reviewer found the\n# change out of scope, or needing a product/architecture decision, before the\n# technical review. The completion handler hands it to a maintainer without\n# approving or merging the PR.\nMAINTAINER_DECISION_VERDICT = \"šŸ›‘ MAINTAINER DECISION REQUIRED\"\n\nCONFIG_FILENAME = \"config.json\"\n\n# Config keys, paired with the type each must have. A wrong type is a hard\n# error at import: the alternative is polling the string \"owner/repo\" one\n# character at a time, or matching a label that is silently a list.\n_CONFIG_TYPES: dict[str, type] = {\n \"repos\": list,\n \"trigger_label\": str,\n \"review_tone\": str,\n \"review_style_instructions\": str,\n \"repo_review_guide_path\": str,\n \"max_new_per_run\": int,\n \"openhands_url\": str,\n}\n\n\ndef load_config(directory: Path | None = None) -> dict:\n \"\"\"Return the rendered config shipped beside this script, or {} if absent.\n\n Only the keys above are read; anything else in the file is ignored, so a\n host may ship provenance there without this script caring.\n \"\"\"\n path = (directory or Path(__file__).resolve().parent) / CONFIG_FILENAME\n if not path.is_file():\n return {}\n\n try:\n raw = json.loads(path.read_text())\n except json.JSONDecodeError as e:\n raise SystemExit(f\"{CONFIG_FILENAME} is not valid JSON: {e}\") from e\n if not isinstance(raw, dict):\n raise SystemExit(f\"{CONFIG_FILENAME} must contain a JSON object\")\n\n config = {}\n for key, expected in _CONFIG_TYPES.items():\n if key not in raw:\n continue\n value = raw[key]\n # bool is an int in Python, so an unguarded int check would accept\n # `\"max_new_per_run\": true` and then start `True` conversations.\n if not isinstance(value, expected) or (\n expected is int and isinstance(value, bool)\n ):\n raise SystemExit(\n f\"{CONFIG_FILENAME}: {key} must be {expected.__name__}, \"\n f\"got {type(value).__name__}\"\n )\n if key == \"repos\" and not (\n value and all(isinstance(item, str) and item for item in value)\n ):\n raise SystemExit(\n f'{CONFIG_FILENAME}: repos must be a non-empty list of \"owner/repo\" strings'\n )\n if key == \"max_new_per_run\" and value < 1:\n raise SystemExit(\n f\"{CONFIG_FILENAME}: max_new_per_run must be at least 1\"\n )\n config[key] = value\n return config\n\n\n# owner/repo, which is what every GitHub API path in this script is built from.\n_REPO_NAME_RE = re.compile(r\"^[A-Za-z0-9._-]+/[A-Za-z0-9._-]+$\")\n\n\ndef normalize_repo(value: str) -> str:\n \"\"\"Return ``owner/repo`` for the ways a repository gets written down.\n\n A clone URL is what a repository page offers to copy, so it is what ends up\n pasted into a setup form. Left alone it becomes\n ``/repos/https://github.com/owner/repo``, which GitHub answers with a 404 -\n indistinguishable, from here, from a repository the token cannot see.\n\n Raises ValueError for anything that is not a repository name, so the run\n says which value it could not read instead of blaming the token.\n \"\"\"\n repo = value.strip()\n if repo.startswith(\"git@\"):\n # git@github.com:owner/repo.git\n repo = repo.partition(\":\")[2]\n elif \"://\" in repo:\n # https://github.com/owner/repo, and anything else with a host\n repo = repo.split(\"://\", 1)[1].partition(\"/\")[2]\n repo = repo.strip(\"/\")\n if repo.endswith(\".git\"):\n repo = repo[: -len(\".git\")]\n\n if not _REPO_NAME_RE.match(repo):\n raise ValueError(\n f\"{value!r} is not a repository. Use owner/repo, for example \"\n \"OpenHands/automation.\"\n )\n return repo\n\n\n_CONFIG = load_config()\nREPOS = _CONFIG.get(\"repos\", REPOS)\nTRIGGER_LABEL = _CONFIG.get(\"trigger_label\", TRIGGER_LABEL)\nREVIEW_TONE = _CONFIG.get(\"review_tone\", REVIEW_TONE)\nREVIEW_STYLE_INSTRUCTIONS = _CONFIG.get(\"review_style_instructions\", REVIEW_STYLE_INSTRUCTIONS)\nREPO_REVIEW_GUIDE_PATH = _CONFIG.get(\"repo_review_guide_path\", REPO_REVIEW_GUIDE_PATH)\nMAX_NEW_PER_RUN = _CONFIG.get(\"max_new_per_run\", MAX_NEW_PER_RUN)\nDEFAULT_OPENHANDS_URL = _CONFIG.get(\"openhands_url\", DEFAULT_OPENHANDS_URL)\n\nDONE_DEBOUNCE = 15\nTERMINAL_STATUSES = {\"idle\", \"finished\", \"error\", \"stuck\"}\n# A conversation that never reaches a terminal status would hold its checkout\n# forever. After this long the review is abandoned so the disk can be reclaimed.\nMAX_ACTIVE_AGE = 2 * 60 * 60\n# A label event is claimed in the state document before its review starts, so an\n# overlapping poll skips it. If the claiming poll dies before the conversation\n# exists, the claim is released after this long - comfortably longer than\n# fetching an archive and opening a conversation, short enough that a crash does\n# not park the review until someone notices.\nSTALLED_CLAIM_SECONDS = 15 * 60\n\n# Login of the token owner, filled in by _verify_token. Reviews are matched\n# against it to answer \"did we already publish a review for this commit\", which\n# is checked on GitHub rather than trusted from the agent.\n_AUTH_LOGIN = \"\"\n\n\ndef _get_env_key() -> str:\n return os.environ.get(\"SESSION_API_KEY\") or os.environ.get(\"OH_SESSION_API_KEYS_0\") or \"\"\n\n\ndef get_secret(name: str) -> str:\n url = os.environ.get(\"AGENT_SERVER_URL\", \"\").rstrip(\"/\")\n key = _get_env_key()\n req = urllib.request.Request(\n f\"{url}/api/settings/secrets/{name}\",\n headers={\"X-Session-API-Key\": key},\n )\n with urllib.request.urlopen(req) as r:\n return r.read().decode().strip()\n\n\ndef fire_callback(\n status: str = \"COMPLETED\",\n error: str | None = None,\n conversation_id: str | None = None,\n) -> None:\n url = os.environ.get(\"AUTOMATION_CALLBACK_URL\", \"\")\n if not url:\n return\n body: dict = {\"status\": status, \"run_id\": os.environ.get(\"AUTOMATION_RUN_ID\", \"\")}\n if error:\n body[\"error\"] = error\n if conversation_id:\n body[\"conversation_id\"] = conversation_id\n req = urllib.request.Request(\n url,\n data=json.dumps(body).encode(),\n headers={\n \"Content-Type\": \"application/json\",\n \"Authorization\": f\"Bearer {os.environ.get('AUTOMATION_CALLBACK_API_KEY', '')}\",\n },\n )\n try:\n urllib.request.urlopen(req)\n except Exception as exc:\n print(f\"Callback error (non-fatal): {exc}\")\n\n\n# ā”€ā”€ State persistence (KV store with local-file fallback) ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€\n\n_KV_TOKEN = os.environ.get(\"AUTOMATION_KV_TOKEN\", \"\")\n_KV_BASE = os.environ.get(\"AUTOMATION_API_URL\", \"\").rstrip(\"/\")\n# Single-repository deployments of this script kept their state under a bare\n# \"state\" key. It is adopted once, on first poll after an upgrade, so the\n# switch to per-repository keys does not re-review every open labelled PR.\n_LEGACY_STATE_KEY = \"state\"\n\n\ndef _repo_slug(repo: str) -> str:\n return repo.replace(\"/\", \"__\")\n\n\ndef _state_key(repo: str) -> str:\n return f\"state:{_repo_slug(repo)}\"\n\n\ndef _kv_available() -> bool:\n return bool(_KV_TOKEN and _KV_BASE)\n\n\ndef _kv_get(key: str) -> dict | None:\n req = urllib.request.Request(\n f\"{_KV_BASE}/v1/kv/{key}\",\n headers={\"Authorization\": f\"Bearer {_KV_TOKEN}\"},\n )\n try:\n with urllib.request.urlopen(req) as r:\n return json.loads(r.read())[\"value\"]\n except urllib.error.HTTPError as exc:\n if exc.code == 404:\n return None\n raise\n\n\ndef _kv_set(key: str, value: dict) -> None:\n req = urllib.request.Request(\n f\"{_KV_BASE}/v1/kv/{key}\",\n data=json.dumps(value).encode(),\n headers={\n \"Authorization\": f\"Bearer {_KV_TOKEN}\",\n \"Content-Type\": \"application/json\",\n },\n method=\"PUT\",\n )\n with urllib.request.urlopen(req) as r:\n r.read()\n\n\ndef _state_dir() -> Path:\n workspace_base = os.environ.get(\"WORKSPACE_BASE\", \"\")\n if workspace_base:\n root = Path(workspace_base).resolve().parent.parent\n else:\n root = Path.home() / \".openhands\" / \"workspaces\"\n state_dir = root / \"automation-state\"\n state_dir.mkdir(parents=True, exist_ok=True)\n return state_dir\n\n\ndef _automation_id() -> str:\n event_payload = json.loads(os.environ.get(\"AUTOMATION_EVENT_PAYLOAD\", \"{}\"))\n return event_payload.get(\"automation_id\", \"default\")\n\n\ndef _state_file_path(repo: str) -> str:\n name = f\"github_pr_reviewer_label_event_{_automation_id()}_{_repo_slug(repo)}.json\"\n return str(_state_dir() / name)\n\n\ndef _legacy_state_file_path() -> str:\n return str(_state_dir() / f\"github_pr_reviewer_label_event_{_automation_id()}.json\")\n\n\ndef _read_state_file(path: str) -> dict | None:\n if not os.path.exists(path):\n return None\n try:\n with open(path) as f:\n return json.load(f)\n except (json.JSONDecodeError, OSError) as exc:\n print(f\" Warning: state file {path} unreadable ({exc}); starting fresh\")\n return None\n\n\ndef _default_state(repo: str) -> dict:\n return {\n \"version\": 3,\n \"repo\": repo,\n \"trigger_label\": TRIGGER_LABEL,\n \"reviews\": {},\n \"prs\": {},\n }\n\n\ndef load_state(repo: str) -> dict:\n \"\"\"Load this repository's state, adopting a pre-multi-repo document once.\"\"\"\n if _kv_available():\n data = _kv_get(_state_key(repo))\n if data is not None:\n print(f\" State loaded from KV store ({_state_key(repo)})\")\n return data\n legacy = _kv_get(_LEGACY_STATE_KEY)\n if legacy is not None and legacy.get(\"repo\") == repo:\n print(f\" Adopted legacy KV state for {repo}\")\n return legacy\n return _default_state(repo)\n\n data = _read_state_file(_state_file_path(repo))\n if data is not None:\n return data\n legacy = _read_state_file(_legacy_state_file_path())\n if legacy is not None and legacy.get(\"repo\") == repo:\n print(f\" Adopted legacy state file for {repo}\")\n return legacy\n return _default_state(repo)\n\n\ndef save_state(repo: str, state: dict) -> None:\n if _kv_available():\n _kv_set(_state_key(repo), state)\n print(f\" State saved to KV store ({_state_key(repo)})\")\n return\n path = _state_file_path(repo)\n tmp_path = f\"{path}.tmp\"\n with open(tmp_path, \"w\") as f:\n json.dump(state, f, indent=2, sort_keys=True)\n os.replace(tmp_path, path)\n print(f\" State saved to {path}\")\n\n\n\ndef _resolve_github_token() -> str:\n try:\n token = get_secret(\"GITHUB_PERSONAL_ACCESS_TOKEN\")\n if token:\n return token\n except Exception:\n pass\n raise RuntimeError(\n \"GITHUB_PERSONAL_ACCESS_TOKEN secret is not set. \"\n \"Go to OpenHands Settings ā†’ Secrets and add your GitHub Personal Access Token.\"\n )\n\n\ndef _verify_token(token: str) -> None:\n \"\"\"Check the token once per run and remember who it belongs to.\"\"\"\n global _AUTH_LOGIN\n try:\n user_data, _ = _github_request(token, \"GET\", \"/user\")\n except urllib.error.HTTPError as exc:\n if exc.code == 401:\n raise RuntimeError(\"GITHUB_PERSONAL_ACCESS_TOKEN is invalid or expired.\") from exc\n raise RuntimeError(f\"GitHub /user check failed: {exc.code}\") from exc\n\n _AUTH_LOGIN = user_data.get(\"login\", \"\")\n print(f\"Authenticated as GitHub user: {_AUTH_LOGIN or '?'}\")\n\n\ndef _verify_repo(token: str, repo: str) -> None:\n try:\n _github_request(token, \"GET\", f\"/repos/{repo}\")\n except urllib.error.HTTPError as exc:\n if exc.code == 404:\n raise RuntimeError(f\"Repository '{repo}' is not accessible with the current token.\") from exc\n raise RuntimeError(f\"GitHub /repos/{repo} check failed: {exc.code}\") from exc\n\n\ndef _list_open_prs(token: str, repo: str) -> list[dict]:\n return _github_paginate(\n token,\n f\"/repos/{repo}/pulls\",\n {\"state\": \"open\", \"sort\": \"updated\", \"direction\": \"desc\"},\n )\n\n\ndef _get_pr(token: str, repo: str, pr_number: int) -> dict:\n pr, _ = _github_request(token, \"GET\", f\"/repos/{repo}/pulls/{pr_number}\")\n return pr\n\n\ndef _get_issue_events(token: str, repo: str, pr_number: int) -> list[dict]:\n return _github_paginate(token, f\"/repos/{repo}/issues/{pr_number}/events\")\n\n\ndef _latest_trigger_label_event(token: str, repo: str, pr_number: int) -> dict | None:\n events = _get_issue_events(token, repo, pr_number)\n matching = [\n event for event in events\n if event.get(\"event\") == \"labeled\"\n and (event.get(\"label\") or {}).get(\"name\", \"\").lower() == TRIGGER_LABEL.lower()\n and event.get(\"id\") is not None\n ]\n if not matching:\n return None\n return max(matching, key=lambda event: (event.get(\"created_at\") or \"\", int(event.get(\"id\") or 0)))\n\n\ndef _post_github_comment(token: str, repo: str, pr_number: int, body: str) -> None:\n try:\n _github_request(\n token,\n \"POST\",\n f\"/repos/{repo}/issues/{pr_number}/comments\",\n body={\"body\": body},\n )\n except Exception as exc:\n print(f\" Warning: failed to post comment on PR #{pr_number}: {exc}\")\n\n\ndef _matching_review_exists(token: str, repo: str, pr_number: int, head_sha: str) -> bool:\n \"\"\"Has this token's user already published a review for this exact commit?\n\n The agent is asked to report success, but a report is not evidence: reviews\n have been reported as posted when none existed. GitHub is the source of\n truth for whether the review landed.\n \"\"\"\n if not head_sha or not _AUTH_LOGIN:\n return False\n try:\n reviews = _github_paginate(token, f\"/repos/{repo}/pulls/{pr_number}/reviews\")\n except Exception as exc:\n print(f\" Warning: could not list reviews for PR #{pr_number}: {exc}\")\n return False\n for review in reviews:\n if (review.get(\"user\") or {}).get(\"login\", \"\").lower() != _AUTH_LOGIN.lower():\n continue\n if review.get(\"commit_id\") == head_sha:\n return True\n return False\n\n\n# ā”€ā”€ Repository checkout ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€\n\n\ndef _checkouts_root() -> Path:\n return Path(os.environ.get(\"WORKSPACE_BASE\", \"/workspace\")).resolve() / \"repositories\"\n\n\ndef _checkout_path(repo: str, pr_number: int, head_sha: str) -> Path:\n return _checkouts_root() / _repo_slug(repo) / f\"pr-{pr_number}-{head_sha[:12]}\"\n\n\ndef _prepare_repository(token: str, repo: str, pr_number: int, head_sha: str) -> Path:\n \"\"\"Materialise the pull request's head commit as the agent's workspace.\n\n The commit is fetched as a tarball rather than cloned, so the directory\n holds exactly the reviewed tree with no history and no git remote for the\n agent to push to.\n \"\"\"\n checkout = _checkout_path(repo, pr_number, head_sha)\n if checkout.exists():\n shutil.rmtree(checkout)\n checkout.mkdir(parents=True)\n\n req = urllib.request.Request(\n f\"https://api.github.com/repos/{repo}/tarball/{head_sha}\",\n headers={\n \"Authorization\": f\"Bearer {token}\",\n \"Accept\": \"application/vnd.github+json\",\n \"X-GitHub-Api-Version\": \"2022-11-28\",\n },\n )\n skipped_links = 0\n try:\n with urllib.request.urlopen(req) as response:\n archive = tarfile.open(fileobj=io.BytesIO(response.read()), mode=\"r:gz\")\n with archive:\n members = archive.getmembers()\n roots = {\n PurePosixPath(member.name).parts[0]\n for member in members\n if PurePosixPath(member.name).parts\n }\n if len(roots) != 1:\n raise RuntimeError(\"Repository archive has an unexpected layout\")\n root = next(iter(roots))\n for member in members:\n path = PurePosixPath(member.name)\n if not path.parts or path.parts[0] != root:\n raise RuntimeError(\"Repository archive contains an invalid path\")\n relative = PurePosixPath(*path.parts[1:])\n if not relative.parts:\n continue\n if relative.is_absolute() or \"..\" in relative.parts:\n raise RuntimeError(\"Repository archive contains path traversal\")\n if member.issym() or member.islnk() or member.isdev():\n # Repositories legitimately contain symlinks. Reviewing does\n # not need them, and materialising them risks escaping the\n # checkout, so skip rather than reject the whole archive.\n skipped_links += 1\n continue\n destination = checkout.joinpath(*relative.parts)\n if member.isdir():\n destination.mkdir(parents=True, exist_ok=True)\n continue\n if not member.isfile():\n continue\n destination.parent.mkdir(parents=True, exist_ok=True)\n source = archive.extractfile(member)\n if source is None:\n raise RuntimeError(f\"Could not read archive member {member.name}\")\n with source, destination.open(\"wb\") as target:\n shutil.copyfileobj(source, target)\n destination.chmod(member.mode & 0o777)\n except Exception:\n shutil.rmtree(checkout, ignore_errors=True)\n raise\n\n if skipped_links:\n print(f\" Skipped {skipped_links} link/device entries while extracting\")\n return checkout\n\n\ndef _release_checkout(rec: dict, agent_url: str, api_key: str) -> bool:\n \"\"\"Remove a finished review's checkout. Returns True when nothing is left.\n\n The checkout is the conversation's working directory, so it is only removed\n once the conversation has stopped - deleting it under a running agent would\n pull the ground out from under it. When the status cannot be confirmed the\n directory is left alone and the next poll tries again.\n \"\"\"\n workspace_dir = rec.get(\"workspace_dir\")\n if not workspace_dir:\n return True\n\n conversation_id = rec.get(\"conversation_id\")\n if conversation_id:\n try:\n status = conversation_status(agent_url, api_key, conversation_id)\n except urllib.error.HTTPError as exc:\n status = \"finished\" if exc.code == 404 else None\n except Exception:\n status = None\n if status is None:\n print(f\" Could not confirm conversation {conversation_id} has stopped; keeping {workspace_dir}\")\n return False\n if status not in TERMINAL_STATUSES:\n print(f\" Conversation {conversation_id} is still '{status}'; keeping its checkout\")\n return False\n\n path = Path(workspace_dir)\n root = _checkouts_root()\n try:\n resolved = path.resolve()\n except OSError:\n resolved = path\n if resolved == root or not resolved.is_relative_to(root):\n # Never delete anything the script did not create under the checkout\n # root, whatever ended up recorded in state.\n print(f\" Refusing to remove {resolved}: outside {root}\")\n rec.pop(\"workspace_dir\", None)\n return True\n\n shutil.rmtree(resolved, ignore_errors=True)\n rec.pop(\"workspace_dir\", None)\n print(f\" Removed checkout {resolved}\")\n return True\n\n\ndef _oh_request(agent_url: str, api_key: str, method: str, path: str, body: dict | None = None) -> dict:\n url = f\"{agent_url}{path}\"\n headers = {\"X-Session-API-Key\": api_key, \"Content-Type\": \"application/json\"}\n data = json.dumps(body).encode() if body is not None else None\n req = urllib.request.Request(url, data=data, headers=headers, method=method)\n try:\n with urllib.request.urlopen(req) as r:\n raw = r.read()\n return json.loads(raw) if raw.strip() else {}\n except urllib.error.HTTPError as exc:\n body_text = exc.read().decode()\n raise RuntimeError(f\"Agent API {method} {path} ā†’ {exc.code}: {body_text}\") from exc\n\n\ndef _fetch_settings(agent_url: str, api_key: str) -> dict:\n req = urllib.request.Request(\n f\"{agent_url}/api/settings\",\n headers={\"X-Session-API-Key\": api_key, \"X-Expose-Secrets\": \"plaintext\"},\n )\n with urllib.request.urlopen(req) as r:\n return json.loads(r.read())\n\n\ndef _get_agent_dict(agent_url: str, api_key: str) -> dict:\n data = _fetch_settings(agent_url, api_key)\n llm = data.get(\"agent_settings\", {}).get(\"llm\", {})\n return {\n \"kind\": \"Agent\",\n \"llm\": llm,\n \"tools\": [{\"name\": \"terminal\"}, {\"name\": \"file_editor\"}],\n }\n\n\ndef _get_mcp_config(agent_url: str, api_key: str) -> dict | None:\n try:\n data = _fetch_settings(agent_url, api_key)\n mcp_config = data.get(\"agent_settings\", {}).get(\"mcp_config\")\n if isinstance(mcp_config, dict) and mcp_config.get(\"mcpServers\"):\n return mcp_config\n except Exception as exc:\n print(f\"Warning: could not fetch MCP config: {exc}\")\n return None\n\n\ndef _list_secret_names(agent_url: str, api_key: str) -> list[dict]:\n try:\n result = _oh_request(agent_url, api_key, \"GET\", \"/api/settings/secrets\")\n return result.get(\"secrets\", [])\n except Exception as exc:\n print(f\"Warning: could not list secrets: {exc}\")\n return []\n\n\ndef _build_secrets_payload(agent_url: str, api_key: str) -> dict:\n secrets = {}\n for secret in _list_secret_names(agent_url, api_key):\n name = secret.get(\"name\", \"\")\n if not name:\n continue\n lookup: dict = {\n \"kind\": \"LookupSecret\",\n \"url\": f\"/api/settings/secrets/{name}\",\n }\n if api_key:\n lookup[\"headers\"] = {\"X-Session-API-Key\": api_key}\n desc = secret.get(\"description\")\n if desc:\n lookup[\"description\"] = desc\n secrets[name] = lookup\n return secrets\n\n\ndef create_conversation(\n agent_url: str,\n api_key: str,\n initial_message: str,\n workspace_dir: Path,\n) -> str:\n payload: dict = {\n \"workspace\": {\"working_dir\": str(workspace_dir)},\n \"agent\": _get_agent_dict(agent_url, api_key),\n \"initial_message\": {\"content\": [{\"text\": initial_message}]},\n }\n secrets = _build_secrets_payload(agent_url, api_key)\n if secrets:\n payload[\"secrets\"] = secrets\n mcp_config = _get_mcp_config(agent_url, api_key)\n if mcp_config:\n payload[\"mcp_config\"] = mcp_config\n result = _oh_request(agent_url, api_key, \"POST\", \"/api/conversations\", payload)\n return result[\"id\"]\n\n\ndef conversation_status(agent_url: str, api_key: str, conv_id: str) -> str:\n result = _oh_request(agent_url, api_key, \"GET\", f\"/api/conversations/{conv_id}\")\n return result.get(\"execution_status\", \"unknown\")\n\n\ndef conversation_final_response(agent_url: str, api_key: str, conv_id: str) -> str:\n result = _oh_request(agent_url, api_key, \"GET\", f\"/api/conversations/{conv_id}/agent_final_response\")\n return result.get(\"response\", \"\")\n\n\n_TONE_INSTRUCTIONS = {\n \"thorough\": (\n \"Provide a comprehensive review. Cover correctness, security vulnerabilities, \"\n \"missing or inadequate tests, code style, maintainability, and potential edge cases. \"\n \"Reference specific files and line numbers where relevant.\"\n ),\n \"concise\": (\n \"Provide a brief, high-signal review. Focus only on important bugs, security problems, \"\n \"or significant design flaws. Omit minor style feedback.\"\n ),\n \"friendly\": (\n \"Provide a constructive, encouraging review. Acknowledge what is done well before \"\n \"raising concerns while still noting real issues.\"\n ),\n}\n\n\ndef _labels(pr: dict) -> list[str]:\n return [label.get(\"name\", \"\") for label in pr.get(\"labels\", [])]\n\n\ndef _has_trigger_label(pr: dict) -> bool:\n return any(label.lower() == TRIGGER_LABEL.lower() for label in _labels(pr))\n\n\ndef _head_sha(pr: dict) -> str:\n return ((pr.get(\"head\") or {}).get(\"sha\") or \"\").strip()\n\n\ndef _review_key(pr_number: int, label_event_id: int | str) -> str:\n return f\"{pr_number}:label:{label_event_id}\"\n\n\ndef _with_ai_disclosure(body: str) -> str:\n disclosure = \"_This comment was posted by an AI agent (OpenHands)._\"\n body = (body or \"\").strip()\n if disclosure.lower() in body.lower():\n return body\n return f\"{body}\\n\\n{disclosure}\" if body else disclosure\n\n\ndef _load_repo_review_guide(workspace_dir: Path) -> str | None:\n \"\"\"Read the repo-specific review guide from the checked-out repository.\n\n The path is taken from ``REPO_REVIEW_GUIDE_PATH``. An empty path disables\n the feature. Returns the file contents, or None if the file is absent or\n unreadable ā€” a missing guide is never fatal, the review simply proceeds\n without it.\n \"\"\"\n if not REPO_REVIEW_GUIDE_PATH:\n return None\n candidate = workspace_dir / REPO_REVIEW_GUIDE_PATH\n try:\n if candidate.is_file():\n text = candidate.read_text(encoding=\"utf-8\", errors=\"replace\").strip()\n if text:\n return text\n except Exception as exc:\n print(f\" Warning: could not read repo review guide {candidate}: {exc}\")\n return None\n\n\ndef _build_review_prompt(\n repo: str,\n pr: dict,\n head_sha: str,\n label_event: dict,\n repo_review_guide: str | None = None,\n *,\n workspace_instructions: str | None = None,\n github_token_secret: str = \"GITHUB_PERSONAL_ACCESS_TOKEN\",\n trigger_description: str | None = None,\n) -> str:\n number = pr.get(\"number\", \"?\")\n title = pr.get(\"title\", \"(no title)\")\n body = (pr.get(\"body\") or \"\").strip() or \"(no description)\"\n html_url = pr.get(\"html_url\", \"\")\n author = (pr.get(\"user\") or {}).get(\"login\", \"?\")\n base_branch = (pr.get(\"base\") or {}).get(\"ref\", \"?\")\n head_branch = (pr.get(\"head\") or {}).get(\"ref\", \"?\")\n label_str = \", \".join(_labels(pr)) or \"(none)\"\n label_event_id = label_event.get(\"id\", \"?\")\n label_event_created_at = label_event.get(\"created_at\", \"?\")\n trigger = trigger_description or (\n f\"latest `{TRIGGER_LABEL}` labeled event {label_event_id} \"\n f\"at {label_event_created_at}\"\n )\n changed_files = pr.get(\"changed_files\", \"?\")\n additions = pr.get(\"additions\", \"?\")\n deletions = pr.get(\"deletions\", \"?\")\n tone = _TONE_INSTRUCTIONS.get(REVIEW_TONE, _TONE_INSTRUCTIONS[\"thorough\"])\n extra = f\"\\n\\nAdditional style instructions:\\n{REVIEW_STYLE_INSTRUCTIONS}\" if REVIEW_STYLE_INSTRUCTIONS.strip() else \"\"\n guide_section = (\n f\"\\n\\nRepo-specific review guide (from {REPO_REVIEW_GUIDE_PATH}):\\n---\\n{repo_review_guide}\\n---\\n\"\n if repo_review_guide else \"\"\n )\n workspace = workspace_instructions or (\n \"The workspace is already the repository root at the exact Head SHA above. \"\n \"Do not clone, fetch, check out, or delete the repository.\"\n )\n\n return (\n \"You are an AI code reviewer. Review the GitHub pull request below and publish \"\n \"the review directly to GitHub. Do not modify files, push commits, or merge \"\n \"the pull request.\\n\\n\"\n f\"Repository : {repo}\\n\"\n f\"PR #{number}: \\\"{title}\\\"\\n\"\n f\"Author : @{author}\\n\"\n f\"Base ā†’ Head: {base_branch} ā† {head_branch}\\n\"\n f\"Head SHA : {head_sha}\\n\"\n f\"Trigger : {trigger}\\n\"\n f\"Labels : {label_str}\\n\"\n f\"Changes : +{additions} -{deletions} across {changed_files} file(s)\\n\"\n f\"URL : {html_url}\\n\"\n f\"\\nPR Description:\\n---\\n{body}\\n---\\n\\n\"\n \"Required workflow:\\n\"\n f\"1. {workspace}\\n\"\n \"2. CURRENT STATE - treat this request as a fresh review, never a continuation of \"\n \"earlier observations. Before the scope gate and before deciding any verdict, re-fetch \"\n \"the current mutable GitHub state for this pull request and act only on what you read \"\n \"now: whether the exact head still matches the Head SHA above, the current PR title and \"\n \"body, the current review comments and threads, the current review requests, the current \"\n \"GitHub Actions check results for that head, and the current body and labels of every \"\n \"linked issue the PR references - a linked issue may have gained or lost a readiness \"\n \"label (for example `ready-for-dev`) or changed priority since an earlier turn. If an \"\n \"earlier turn in this conversation reviewed this PR or its linked issues, that analysis \"\n \"and the repository guidance you read remain useful background, but every mutable fact \"\n \"above must be re-established now; never repeat an earlier finding, verdict, or \"\n \"label/priority claim that the state you just read does not support.\\n\"\n f\" Use `gh` or GitHub REST API calls with `{github_token_secret}`; never print secret values.\\n\"\n \"3. Before reviewing, you MUST read the repository's own guidance to understand the repo first.\\n\"\n \" Read `AGENTS.md` at the repository root (and any nested `AGENTS.md` covering the \"\n \"changed files), plus other relevant docs when present - e.g. `CONTRIBUTING.md`, \"\n \"`CLAUDE.md`, `.cursorrules`, and any review or coding-guideline docs. Apply that \"\n \"guidance to your review.\\n\"\n \"4. SCOPE GATE - before inspecting changed files, reading the diff, or running any test, \"\n \"use the repository guidance above (its scope categories and ownership boundaries) to decide \"\n \"whether this change belongs in this repository and has the product/architecture direction \"\n \"it needs. If it does, continue the technical review unchanged. If it does not, or needs a \"\n \"product/architecture decision, stop here and publish exactly one review with \"\n f\"`POST /repos/{repo}/pulls/{number}/reviews`, using `commit_id` equal to the Head SHA above \"\n \"and `event: COMMENT`. Briefly say whether the change should move repositories, close, or \"\n \"receive a maintainer decision; when it belongs elsewhere, name the likely owning repository \"\n \"only if the evidence supports it. Do not run tests or report implementation findings. This \"\n \"outcome is not an approval, and its body ends with the verdict on its own line: \"\n f\"`{MAINTAINER_DECISION_VERDICT}`.\\n\"\n \"5. Otherwise continue the technical review. Inspect the PR discussion, existing review \"\n \"comments, changed files, and the diff, together with the surrounding code in the workspace.\\n\"\n \"6. Ground every finding in the workspace code. Before using an inline location, verify that \"\n \"the path and line are part of this pull request's diff. Compare every changed branch with \"\n \"the base behavior, including side effects outside the reported bug; a revision, event, or \"\n \"delivery identifier proves only the inputs it actually includes, not that unrelated profile, \"\n \"credential, configuration, or external state stayed unchanged. Do not add speculative or \"\n \"out-of-scope notes: every blocking or non-blocking observation must identify demonstrated \"\n \"behavior on the current head and explain why it matters to the merge decision.\\n\"\n f\"7. Publish one review with `POST /repos/{repo}/pulls/{number}/reviews`, using \"\n \"`commit_id` equal to the Head SHA above. Use `event: APPROVE` when there are \"\n \"no material findings; otherwise use `event: COMMENT`. Never use \"\n \"`REQUEST_CHANGES`.\\n\"\n \" The native GitHub review is the only result channel. Do not create commit \"\n \"statuses or Checks, post a separate issue comment, change labels, request \"\n \"reviewers, or merge. The deterministic automation owns trigger completion \"\n \"and any human-review handoff.\\n\"\n \" Put the overall assessment in `body`, and each line-specific finding in the `comments` \"\n \"array with `path`, `line`, `side: RIGHT`, and `body`.\\n\"\n \" Only create inline comments for actionable findings; do not open praise or nitpick threads.\\n\"\n \"8. If a finding cannot be attached to a changed line, put it in the review body instead. \"\n \"If the API rejects the inline positions, retry with every finding in the body and no `comments` array. \"\n \"If GitHub forbids the configured bot from approving its own PR, retry the clean review with \"\n \"`event: COMMENT` and keep the approved verdict.\\n\"\n \"9. Begin the review body with this disclosure: \"\n \"`_This review was posted by an AI agent (OpenHands)._`\\n\"\n \"10. End the review body with a verdict on its own line: either `āœ… APPROVED` \"\n \"or `šŸ”„ CHANGES REQUESTED`.\\n\"\n \"11. If there are no material issues, still publish a review saying so, with the \"\n \"disclosure and the verdict.\\n\"\n f\"\\nReview instructions:\\n{tone}{extra}{guide_section}\\n\\n\"\n \"After GitHub accepts the review, output exactly `GITHUB_REVIEW_POSTED`. \"\n \"If publishing still fails after the fallback in step 8, output the complete review text \"\n \"so it can be posted as a comment instead.\"\n )\n\n\ndef _process_review_request(\n github_token: str,\n agent_url: str,\n api_key: str,\n openhands_url: str,\n repo: str,\n pr: dict,\n label_event: dict,\n reviews: dict,\n persist: Callable[[], None],\n) -> str | None:\n number = pr[\"number\"]\n head_sha = _head_sha(pr)\n label_event_id = label_event[\"id\"]\n key = _review_key(number, label_event_id)\n title = pr.get(\"title\", \"(no title)\")\n html_url = pr.get(\"html_url\", \"\")\n\n print(f\" Queuing review for PR #{number} from `{TRIGGER_LABEL}` event {label_event_id} at {head_sha[:12]}: {title}\")\n\n # Claim the label event and persist it *before* the slow work below. State\n # is otherwise only written when the repository finishes polling, so a poll\n # starting while this one downloads an archive or spins up a conversation\n # would read no record for this event and review the same commit a second\n # time - two conversations, two \"reviewing\" comments, two reviews.\n reviews[key] = {\n \"pr_number\": number,\n \"head_sha\": head_sha,\n \"trigger_label_event_id\": label_event_id,\n \"trigger_label_event_created_at\": label_event.get(\"created_at\"),\n \"html_url\": html_url,\n \"status\": \"starting\",\n \"conversation_id\": None,\n \"workspace_dir\": None,\n \"last_activity\": time.time(),\n }\n persist()\n\n workspace_dir = None\n try:\n workspace_dir = _prepare_repository(github_token, repo, number, head_sha)\n repo_review_guide = _load_repo_review_guide(workspace_dir)\n if repo_review_guide:\n print(f\" Injected repo review guide for PR #{number}\")\n prompt = _build_review_prompt(repo, pr, head_sha, label_event, repo_review_guide)\n conv_id = create_conversation(agent_url, api_key, prompt, workspace_dir)\n except Exception as exc:\n # The claim is dropped so the next poll retries this label event. The\n # checkout goes with it rather than being left behind.\n if workspace_dir:\n shutil.rmtree(workspace_dir, ignore_errors=True)\n reviews.pop(key, None)\n persist()\n print(f\" Error starting review for PR #{number}: {exc}\")\n return None\n\n reviews[key].update(\n {\n \"status\": \"active\",\n \"conversation_id\": conv_id,\n \"workspace_dir\": str(workspace_dir),\n \"last_activity\": time.time(),\n }\n )\n persist()\n print(f\" Created review conversation {conv_id}\")\n\n conv_url = f\"{openhands_url}/conversations/{conv_id}\"\n _post_github_comment(\n github_token,\n repo,\n number,\n _with_ai_disclosure(\n \"šŸ¤– **OpenHands is reviewing this PR.**\\n\\n\"\n f\"Trigger label: `{TRIGGER_LABEL}`\\n\"\n f\"Label event: `{label_event_id}` at `{label_event.get('created_at', '?')}`\\n\"\n f\"Head commit: `{head_sha}`\\n\"\n f\"View the conversation: {conv_url}\"\n ),\n )\n return conv_id\n\n\ndef _check_conversation_completion(\n rec: dict,\n latest_open_prs: dict[int, dict],\n github_token: str,\n agent_url: str,\n api_key: str,\n repo: str,\n) -> None:\n age = time.time() - rec.get(\"last_activity\", 0.0)\n if age < DONE_DEBOUNCE:\n return\n\n conv_id = rec[\"conversation_id\"]\n pr_number = rec[\"pr_number\"]\n reviewed_sha = rec.get(\"head_sha\", \"\")\n current_pr = latest_open_prs.get(pr_number)\n\n if not current_pr:\n rec[\"status\"] = \"closed\"\n print(f\" PR #{pr_number} closed/merged ā€” skipping result post\")\n _release_checkout(rec, agent_url, api_key)\n return\n\n current_sha = _head_sha(current_pr)\n if current_sha and reviewed_sha and current_sha != reviewed_sha:\n rec[\"status\"] = \"stale\"\n rec[\"stale_reason\"] = f\"head changed from {reviewed_sha} to {current_sha}\"\n print(f\" PR #{pr_number} advanced to {current_sha[:12]} ā€” suppressing stale review {conv_id}\")\n _release_checkout(rec, agent_url, api_key)\n return\n\n try:\n status = conversation_status(agent_url, api_key, conv_id)\n except Exception as exc:\n print(f\" Warning: could not get status for {conv_id}: {exc}\")\n return\n\n print(f\" PR #{pr_number} conversation {conv_id} ā†’ status={status}\")\n if status not in TERMINAL_STATUSES:\n if age > MAX_ACTIVE_AGE:\n rec[\"status\"] = \"expired\"\n rec[\"expired_after\"] = age\n print(f\" Review for PR #{pr_number} still '{status}' after {int(age)}s; abandoning it\")\n _release_checkout(rec, agent_url, api_key)\n return\n\n try:\n final = conversation_final_response(agent_url, api_key, conv_id)\n except Exception:\n final = \"\"\n\n if status in {\"error\", \"stuck\"}:\n _post_github_comment(\n github_token,\n repo,\n pr_number,\n _with_ai_disclosure(\n f\"āš ļø **OpenHands PR Reviewer encountered a problem** at commit `{reviewed_sha[:12]}` \"\n f\"(status: `{status}`).\\n\\n{final}\".strip()\n ),\n )\n elif _matching_review_exists(github_token, repo, pr_number, reviewed_sha):\n print(f\" PR #{pr_number}: review confirmed on GitHub at {reviewed_sha[:12]}\")\n else:\n # The agent was asked to publish the review itself; it did not, so the\n # work is not lost - post whatever it produced as a comment.\n _post_github_comment(\n github_token,\n repo,\n pr_number,\n _with_ai_disclosure(\n final\n or f\"āœ… **OpenHands completed the review for commit `{reviewed_sha[:12]}`.** No review text was produced.\"\n ),\n )\n print(f\" PR #{pr_number}: no review found on GitHub; posted the result as a comment\")\n\n rec[\"status\"] = \"closed\"\n rec[\"completed_at\"] = time.time()\n _release_checkout(rec, agent_url, api_key)\n\n\ndef _process_repo(\n repo: str,\n github_token: str,\n agent_url: str,\n api_key: str,\n openhands_url: str,\n) -> str | None:\n \"\"\"Poll one repository end to end. Its state is loaded and saved here, so a\n failure in another repository cannot discard this one's progress.\"\"\"\n print(f\"\\n=== {repo} ===\")\n _verify_repo(github_token, repo)\n\n state = load_state(repo)\n reviews: dict = state.setdefault(\"reviews\", {})\n prs_state: dict = state.setdefault(\"prs\", {})\n\n def persist() -> None:\n state[\"version\"] = 3\n state[\"repo\"] = repo\n state[\"trigger_label\"] = TRIGGER_LABEL\n state[\"updated_at\"] = time.time()\n save_state(repo, state)\n\n open_prs = _list_open_prs(github_token, repo)\n latest_open_prs = {pr[\"number\"]: pr for pr in open_prs}\n print(f\" Found {len(open_prs)} open PR(s)\")\n\n last_conversation_id = None\n\n for pr in open_prs:\n number = pr[\"number\"]\n head_sha = _head_sha(pr)\n label_present = _has_trigger_label(pr)\n prs_state[str(number)] = {\n \"head_sha\": head_sha,\n \"label_present\": label_present,\n \"labels\": _labels(pr),\n \"last_seen\": time.time(),\n }\n\n if not label_present:\n continue\n if not head_sha:\n print(f\" PR #{number} has no head SHA; skipping\")\n continue\n\n fresh_pr = _get_pr(github_token, repo, number)\n fresh_head_sha = _head_sha(fresh_pr)\n if fresh_head_sha != head_sha:\n print(f\" PR #{number} head changed during poll ({head_sha[:12]} ā†’ {fresh_head_sha[:12]}); using latest PR metadata\")\n if not _has_trigger_label(fresh_pr):\n print(f\" PR #{number} lost `{TRIGGER_LABEL}` during poll; skipping\")\n continue\n\n label_event = _latest_trigger_label_event(github_token, repo, number)\n if not label_event:\n print(f\" PR #{number} has `{TRIGGER_LABEL}` but no matching labeled event; skipping\")\n continue\n\n key = _review_key(number, label_event[\"id\"])\n if key in reviews:\n print(f\" PR #{number} label event {label_event['id']} already tracked ({reviews[key].get('status')})\")\n continue\n\n conv_id = _process_review_request(\n github_token, agent_url, api_key, openhands_url, repo, fresh_pr, label_event, reviews, persist\n )\n if conv_id:\n last_conversation_id = conv_id\n\n for rev_key, rec in list(reviews.items()):\n if rec.get(\"status\") == \"starting\":\n # A claim this poll made has already moved to \"active\" or been\n # dropped, so one still sitting here belongs to a poll that died\n # between claiming and creating its conversation. Release it once it\n # is old enough that no live poll could still be working on it,\n # otherwise the label event would never be reviewed.\n age = time.time() - float(rec.get(\"last_activity\") or 0)\n if age > STALLED_CLAIM_SECONDS:\n print(f\" Releasing a claim stalled for {int(age)}s: {rev_key}\")\n reviews.pop(rev_key, None)\n continue\n if rec.get(\"status\") == \"active\":\n _check_conversation_completion(rec, latest_open_prs, github_token, agent_url, api_key, repo)\n elif rec.get(\"workspace_dir\"):\n # A checkout whose removal could not be confirmed on an earlier\n # poll, e.g. the agent was still running when its PR was closed.\n _release_checkout(rec, agent_url, api_key)\n\n persist()\n return last_conversation_id\n\n\ndef main() -> str | None:\n agent_url = os.environ.get(\"AGENT_SERVER_URL\", \"\").rstrip(\"/\")\n api_key = _get_env_key()\n\n github_token = _resolve_github_token()\n _verify_token(github_token)\n\n try:\n openhands_url = get_secret(\"OPENHANDS_URL\").rstrip(\"/\") or DEFAULT_OPENHANDS_URL\n except Exception:\n openhands_url = DEFAULT_OPENHANDS_URL\n\n last_conversation_id = None\n failures = []\n for configured in REPOS:\n # One repository failing must not stop the others from being polled.\n try:\n repo = normalize_repo(configured)\n conv_id = _process_repo(repo, github_token, agent_url, api_key, openhands_url)\n if conv_id:\n last_conversation_id = conv_id\n except Exception as exc:\n print(f\"Error processing {configured}: {exc}\")\n failures.append(f\"{configured}: {exc}\")\n\n if failures and len(failures) == len(REPOS):\n # Every repository failed, so the run achieved nothing - report it as a\n # failed run rather than a successful no-op.\n raise RuntimeError(\"; \".join(failures))\n return last_conversation_id\n\n\nif __name__ == \"__main__\":\n try:\n conversation_id = main()\n fire_callback(\"COMPLETED\", conversation_id=conversation_id)\n except Exception as exc:\n import traceback\n\n traceback.print_exc()\n fire_callback(\"FAILED\", str(exc))\n sys.exit(1)\n", "maintainer_handoff.py": "\"\"\"Choose one code-aware, currently available maintainer for a reviewed PR.\"\"\"\n\nfrom urllib.error import HTTPError\nfrom urllib.parse import urlencode\n\n_DECISIVE_REVIEW_STATES = {\"APPROVED\", \"CHANGES_REQUESTED\"}\n_MAX_PATHS = 8\n_COMMITS_PER_PATH = 10\n\n\nclass HandoffConfigurationError(ValueError):\n \"\"\"The configured roster cannot produce a GitHub review request.\"\"\"\n\n\ndef parse_maintainers(value):\n \"\"\"Normalize a comma-separated catalog value or a JSON-style list.\"\"\"\n if isinstance(value, str):\n value = value.split(\",\")\n result = []\n seen = set()\n for item in value or []:\n login = str(item).strip()\n key = login.lower()\n if login and key not in seen:\n seen.add(key)\n result.append(login)\n return result\n\n\ndef _reviewer_states(reviews, head_sha=None):\n standing = {}\n for review in reviews:\n if head_sha is not None and review.get(\"commit_id\") != head_sha:\n continue\n state = (review.get(\"state\") or \"\").upper()\n login = (review.get(\"user\") or {}).get(\"login\", \"\").lower()\n if not login:\n continue\n if state == \"DISMISSED\":\n standing.pop(login, None)\n elif state in _DECISIVE_REVIEW_STATES:\n standing[login] = state\n return standing\n\n\ndef _path_scores(repository, pr_number, base_ref, candidates):\n scores = dict.fromkeys(candidates, 0)\n files = repository.gh_pages(f\"/pulls/{pr_number}/files\")[:_MAX_PATHS]\n for changed_file in files:\n path = changed_file.get(\"filename\")\n if not path:\n continue\n query = urlencode(\n {\"path\": path, \"sha\": base_ref, \"per_page\": _COMMITS_PER_PATH}\n )\n commits = repository.gh(\"GET\", f\"/commits?{query}\")\n for rank, commit in enumerate(commits[:_COMMITS_PER_PATH]):\n login = ((commit.get(\"author\") or {}).get(\"login\") or \"\").lower()\n if login in scores:\n scores[login] += _COMMITS_PER_PATH - rank\n return scores\n\n\ndef _open_review_load(repository, owner, owner_type, login):\n owner_qualifier = \"org\" if owner_type.lower() == \"organization\" else \"user\"\n response = repository.api(\n \"GET\",\n \"/search/issues\",\n params={\n \"q\": (\n f\"is:pr is:open {owner_qualifier}:{owner} \"\n f\"review-requested:{login}\"\n ),\n \"per_page\": 1,\n },\n )\n return int(response.get(\"total_count\", 0))\n\n\ndef request_maintainer_review(repository, pr, maintainers):\n \"\"\"Ensure one configured maintainer is reviewing *pr*.\n\n API failures propagate so the scanner can retry without clearing its trigger\n label. The return value is the existing or newly requested login.\n \"\"\"\n roster = parse_maintainers(maintainers)\n if not roster:\n return None\n\n by_key = {login.lower(): login for login in roster}\n requested = {\n (item.get(\"login\") or \"\").lower() for item in pr.get(\"requested_reviewers\", [])\n }\n for login in roster:\n if login.lower() in requested:\n return login\n\n number = pr[\"number\"]\n head_sha = pr[\"head\"][\"sha\"]\n reviews = repository.gh_pages(f\"/pulls/{number}/reviews\")\n # A human approval remains useful after a later push unless GitHub dismisses\n # it or the reviewer subsequently requests changes. Do not ask that person\n # to review the same PR again merely because the automated review targets a\n # newer commit.\n review_states = _reviewer_states(reviews)\n for login in roster:\n if review_states.get(login.lower()) == \"APPROVED\":\n return login\n\n standing = _reviewer_states(reviews, head_sha)\n for login in roster:\n if login.lower() in standing:\n return login\n\n author = ((pr.get(\"user\") or {}).get(\"login\") or \"\").lower()\n candidates = [key for key in by_key if key != author]\n if not candidates:\n raise HandoffConfigurationError(\n \"No eligible maintainer remains after excluding the PR author\"\n )\n\n base_ref = (pr.get(\"base\") or {}).get(\"ref\") or \"main\"\n scores = _path_scores(repository, number, base_ref, candidates)\n owner = repository.repository.split(\"/\", 1)[0]\n owner_type = (\n (((pr.get(\"base\") or {}).get(\"repo\") or {}).get(\"owner\") or {}).get(\"type\")\n or \"Organization\"\n )\n loads = {\n login: _open_review_load(repository, owner, owner_type, by_key[login])\n for login in candidates\n }\n order = {login.lower(): index for index, login in enumerate(roster)}\n selected_key = min(\n candidates,\n key=lambda login: (-scores[login], loads[login], order[login]),\n )\n selected = by_key[selected_key]\n try:\n repository.gh(\n \"POST\",\n f\"/pulls/{number}/requested_reviewers\",\n {\"reviewers\": [selected]},\n )\n except HTTPError as exc:\n if exc.code == 422:\n raise HandoffConfigurationError(\n f\"GitHub cannot assign configured maintainer @{selected}\"\n ) from exc\n raise\n return selected\n", - "worker.py": "\"\"\"Select requested PR heads and delegate each review to a sandboxed agent.\"\"\"\n\nimport json\nimport os\nimport sys\nfrom functools import cached_property\nfrom urllib.parse import quote\n\nimport main as workflow\nfrom agent_conversation import AgentConversationDispatcher\nfrom github_client import GitHubRepository, run_repositories\nfrom maintainer_handoff import (\n HandoffConfigurationError,\n parse_maintainers,\n request_maintainer_review,\n)\n\n# The head-eligibility gate. Scheduled discovery classifies only the checks\n# GitHub reports as required for the pull request, read through the GraphQL\n# `isRequired` signal, so an optional workflow that fails before creating any\n# check run cannot block a head whose required checks pass. An explicit\n# `all-hands-bot` review request is the intake-policy exception and bypasses the\n# gate entirely. A completed required run blocks unless its conclusion is\n# explicitly non-blocking, so an unrecognized conclusion fails closed rather than\n# approving silently, and a required run that has not completed means waiting,\n# never approval. When the required set cannot be read, the gate falls back to\n# every current-head check and workflow run, so a red head still blocks.\nCHECK_GATE_MARKER = \"\\n\\n\"\n f\"_This is an automated check - {WORKFLOW_DISCLOSURE}._\"\n )\n\n def _gate_head(self, pr, requested=False, scheduled=False):\n \"\"\"Return the head's eligibility, explaining any stop on the PR.\n\n An explicit `all-hands-bot` review request is the intake-policy\n exception: the caller asked for this head by name, so the CI gate does\n not apply and no gate comment is left. Scheduled discovery still gates on\n the required checks. When it stops a scheduled run, the explanation names\n the retry that deployment actually has, which is why `scheduled` is\n threaded into the body.\n \"\"\"\n sha = pr[\"head\"][\"sha\"]\n if requested:\n return \"green\", sha\n state, names = self._classify_check_runs(pr)\n if state in (\"blocked\", \"waiting\"):\n marker = f\"{CHECK_GATE_MARKER}{state}:{sha} -->\"\n self._gate_comment(\n pr[\"number\"],\n marker,\n self._gate_body(sha, state, names, scheduled),\n names,\n )\n return state, sha\n\n def _outstanding_review_request(self, pr):\n \"\"\"Whether an open, non-draft PR still holds a request for the reviewer.\n\n The list endpoint already answers this: `requested_reviewers` is the live\n set, so a review that was submitted, or a request that was withdrawn, is\n simply absent. Drafts are excluded because a draft is not reviewable.\n \"\"\"\n if pr.get(\"draft\"):\n return False\n return any(\n (item.get(\"login\") or \"\").lower() == self.trigger_reviewer\n for item in pr.get(\"requested_reviewers\") or []\n )\n\n def run(self):\n repository_id = self.gh(\"GET\", \"\")[\"id\"]\n label = self.config.get(\"trigger_label\", workflow.TRIGGER_LABEL)\n payload = self._event_payload()\n if payload is None:\n prs = self.gh_pages(\"/pulls?state=open&sort=updated&direction=asc\")\n else:\n candidate = self._event_candidate(payload)\n if candidate and self.github_login.lower() != self.trigger_reviewer:\n raise RuntimeError(\n \"The configured GitHub credential must authenticate as \"\n f\"{self.trigger_reviewer} for reviewer-request mode\"\n )\n prs = [candidate] if candidate else []\n failures = []\n for candidate in prs:\n event_mode = payload is not None\n if not event_mode and label not in {\n item[\"name\"] for item in candidate.get(\"labels\", [])\n } and not self._outstanding_review_request(candidate):\n # A scheduled scan covers the trigger label and any PR that still\n # holds a review request the CI gate deferred. Everything else is\n # not ours to review.\n continue\n try:\n pr = self.gh(\"GET\", f\"/pulls/{candidate['number']}\")\n has_label = label in {\n item[\"name\"] for item in pr.get(\"labels\", [])\n }\n if event_mode or has_label:\n trigger = (\n self._latest_reviewer_request(pr[\"number\"])\n if event_mode\n else workflow._latest_trigger_label_event(\n self.token, self.repository, pr[\"number\"]\n )\n )\n else:\n # The outstanding request is the trigger, so its own event\n # keys the delivery and dedupes repeated scans.\n trigger = self._latest_reviewer_request(pr[\"number\"])\n if trigger is None:\n continue\n trigger_label = label if (not event_mode and has_label) else None\n if self._finish_completed_review(pr, trigger, trigger_label):\n continue\n if event_mode and payload.get(\"action\") == \"submitted\":\n # A submitted review is a completion signal, never a fresh\n # trigger. A non-decisive review, or one superseded by a new\n # head, must wait for another reviewer request instead of\n # dispatching a review the caller never asked for.\n continue\n sha = pr[\"head\"][\"sha\"]\n gate_state, sha = self._gate_head(\n pr, requested=event_mode, scheduled=not event_mode\n )\n if gate_state != \"green\":\n # A deterministic blocker stops the run without spending a\n # worker slot on an agent. The trigger is not consumed: the\n # next scheduled scan or explicit request re-evaluates the\n # head once its checks are non-blocking.\n print(\n json.dumps(\n {\n \"repository\": self.repository,\n \"pr\": pr[\"number\"],\n \"head_sha\": sha,\n \"disposition\": f\"review-{gate_state}\",\n }\n ),\n flush=True,\n )\n continue\n record = {\n \"repository\": self.repository,\n \"number\": pr[\"number\"],\n # The oldest outstanding request drains first; repository and\n # number break a tie so the order is deterministic.\n \"created_at\": trigger.get(\"created_at\") or \"\",\n \"config\": self.config,\n \"start\": lambda pr=pr, trigger=trigger, sha=sha,\n trigger_label=trigger_label: self._start_review(\n repository_id, pr, trigger, sha, trigger_label\n ),\n }\n if event_mode:\n # An explicit request is a caller's decision to spend a\n # conversation now, not a backlog item, so the event path\n # dispatches immediately and is never bounded by the\n # scheduled scan's per-run maximum.\n self._start_review(\n repository_id, pr, trigger, sha, trigger_label\n )\n else:\n self.intake.register(record)\n except Exception as exc: # noqa: BLE001 - one PR must not block the scan\n failures.append(candidate.get(\"number\", \"?\"))\n print(\n f\"Failed to submit {self.repository} PR \"\n f\"#{candidate.get('number', '?')}: \"\n f\"{type(exc).__name__}: {exc}\",\n file=sys.stderr,\n flush=True,\n )\n if self.scan_intake is None:\n # A run() with no shared scan intake owns its whole scan, so it\n # drains the candidates it collected, bounded by the per-run maximum.\n self.intake.drain()\n if failures:\n raise RuntimeError(\n \"Reviewer scan failed for PRs: \"\n + \", \".join(f\"#{number}\" for number in failures)\n )\n\n def _start_review(self, repository_id, pr, trigger, sha, trigger_label):\n result = self.dispatcher.deliver(\n subject=f\"{repository_id}:pr:{pr['number']}\",\n delivery=f\"{trigger['id']}:{sha}\",\n prompt=self._prompt(pr, trigger, trigger_label),\n )\n print(\n json.dumps(\n {\n \"repository\": self.repository,\n \"pr\": pr[\"number\"],\n \"head_sha\": sha,\n \"disposition\": result[\"disposition\"],\n \"conversation_id\": result[\"conversation_id\"],\n }\n ),\n flush=True,\n )\n return result\n\n\ndef run_scan(dispatcher):\n \"\"\"Run one scheduled scan over every configured repository, then drain.\n\n One shared intake spans the whole scan, so the per-run maximum is global\n rather than reset per repository, and the drain happens after every\n repository has been scanned so the oldest outstanding request across all of\n them starts first. One repository failing must not discard another\n repository's drained candidates, so the drain still runs and the first\n failure is what the scan reports.\n \"\"\"\n PullRequestReviewer.scan_intake = ReviewIntake()\n failure = None\n try:\n run_repositories(PullRequestReviewer, dispatcher=dispatcher)\n except Exception as exc: # noqa: BLE001 - reported after the drain\n failure = exc\n try:\n PullRequestReviewer.scan_intake.drain()\n except Exception as exc: # noqa: BLE001 - reported after the scan failure\n failure = failure or exc\n if failure is not None:\n raise failure\n\n\nif __name__ == \"__main__\":\n with AgentConversationDispatcher() as dispatcher:\n run_scan(dispatcher)\n" + "worker.py": "\"\"\"Select requested PR heads and delegate each review to a sandboxed agent.\"\"\"\n\nimport json\nimport os\nimport sys\nfrom functools import cached_property\nfrom urllib.parse import quote\n\nimport main as workflow\nfrom agent_conversation import AgentConversationDispatcher\nfrom github_client import GitHubRepository, run_repositories\nfrom maintainer_handoff import (\n HandoffConfigurationError,\n parse_maintainers,\n request_maintainer_review,\n)\n\n# The head-eligibility gate. Scheduled discovery classifies only the checks\n# GitHub reports as required for the pull request, read through the GraphQL\n# `isRequired` signal, so an optional workflow that fails before creating any\n# check run cannot block a head whose required checks pass. An explicit\n# `all-hands-bot` review request is the intake-policy exception and bypasses the\n# gate entirely. A completed required run blocks unless its conclusion is\n# explicitly non-blocking, so an unrecognized conclusion fails closed rather than\n# approving silently, and a required run that has not completed means waiting,\n# never approval. When the required set cannot be read, the gate falls back to\n# every current-head check and workflow run, so a red head still blocks.\nCHECK_GATE_MARKER = \"\\n\\n\"\n f\"_This is an automated check - {WORKFLOW_DISCLOSURE}._\"\n )\n\n def _gate_head(self, pr, requested=False, scheduled=False, explain=True):\n \"\"\"Return the head's eligibility, explaining any stop on the PR.\n\n An explicit `all-hands-bot` review request is the intake-policy\n exception: the caller asked for this head by name, so the CI gate does\n not apply and no gate comment is left. Scheduled discovery still gates on\n the required checks. When it stops a scheduled run, the explanation names\n the retry that deployment actually has, which is why `scheduled` is\n threaded into the body.\n\n `explain` is False for an unrequested candidate: a PR nobody asked about\n that is merely red or pending gets no managed comment. Announcing a\n blocked or waiting head for every open PR is what produced the comment\n storm the rotating window is bounded against, and the managed comment is\n the answer to an explicit request or trigger label. The gate still\n classifies the head, so a red or pending unrequested head is skipped\n without starting an agent.\n \"\"\"\n sha = pr[\"head\"][\"sha\"]\n if requested:\n return \"green\", sha\n state, names = self._classify_check_runs(pr)\n if explain and state in (\"blocked\", \"waiting\"):\n marker = f\"{CHECK_GATE_MARKER}{state}:{sha} -->\"\n self._gate_comment(\n pr[\"number\"],\n marker,\n self._gate_body(sha, state, names, scheduled),\n names,\n )\n return state, sha\n\n def _outstanding_review_request(self, pr):\n \"\"\"Whether an open, non-draft PR still holds a request for the reviewer.\n\n The list endpoint already answers this: `requested_reviewers` is the live\n set, so a review that was submitted, or a request that was withdrawn, is\n simply absent. Drafts are excluded because a draft is not reviewable.\n \"\"\"\n if pr.get(\"draft\"):\n return False\n return any(\n (item.get(\"login\") or \"\").lower() == self.trigger_reviewer\n for item in pr.get(\"requested_reviewers\") or []\n )\n\n def _has_current_head_review(self, number, sha):\n \"\"\"Whether the reviewer account already published a review on this head.\n\n This is the candidate filter's negative: an open, non-draft PR without a\n current-head review by the configured reviewer is eligible for a\n scheduled scan even when nobody requested the bot. The same predicate is\n what the completion handler reconciles, so a review this scan starts and\n a review it finds already present are the same set.\n \"\"\"\n return any(\n review.get(\"commit_id\") == sha\n and ((review.get(\"user\") or {}).get(\"login\") or \"\").lower()\n == self.github_login.lower()\n for review in self.gh_pages(f\"/pulls/{number}/reviews\")\n )\n\n def _unrequested_head(self, pr):\n \"\"\"The current-head delivery key for an unrequested PR, or None.\n\n The key is the repository/PR identity plus the head SHA, so repeated\n scheduled scans over one head reuse one conversation and one native\n review, while a changed head becomes eligible again under its new SHA.\n \"\"\"\n sha = pr[\"head\"][\"sha\"]\n return f\"scan:{self.repository}:{pr['number']}:{sha}\"\n\n @property\n def _scan_cursor(self):\n \"\"\"This repository's scan position, created lazily like the intake.\"\"\"\n cursor = self.__dict__.get(\"_cursor\")\n if cursor is None:\n cursor = self.__dict__[\"_cursor\"] = ScanCursor(self.repository)\n return cursor\n\n def _explicit_candidate(self, pr, label):\n \"\"\"Whether a PR was explicitly requested by a caller.\n\n An explicit `all-hands-bot` review request or a trigger label is a\n caller's decision, so it is never subject to the rotating window: every\n explicit candidate is examined on every scan, whatever the stored scan\n position is.\n \"\"\"\n if label in {item[\"name\"] for item in pr.get(\"labels\", [])}:\n return True\n return self._outstanding_review_request(pr)\n\n def _rotating_window(self, prs, label):\n \"\"\"The explicit candidates plus a bounded slice of the unrequested ones.\n\n Classifying an unrequested head costs a review read and the exact-head\n check/workflow reads, so examining every open PR in one scan is what let\n a single run exhaust the API budget and post a managed gate comment for\n every red or pending head. The unrequested backlog is therefore examined\n a bounded `SCAN_WINDOW` at a time, starting where the previous scan\n stopped (the per-repository position in the Automation KV store), so\n successive scans rotate through the whole backlog. Explicit candidates\n are always included, so a request is never delayed behind the window.\n \"\"\"\n explicit, unrequested = [], []\n for pr in prs:\n if pr.get(\"draft\") and label not in {\n item[\"name\"] for item in pr.get(\"labels\", [])\n } and not self._outstanding_review_request(pr):\n # A draft that is neither labeled nor requested is not reviewable\n # by the unrequested path either, so drop it before the full read.\n continue\n if self._explicit_candidate(pr, label):\n explicit.append(pr)\n else:\n unrequested.append(pr)\n if not unrequested:\n return explicit\n start = self._scan_cursor.position(len(unrequested))\n window = unrequested[start : start + SCAN_WINDOW]\n self._scan_cursor.advance(start + len(window))\n return explicit + window\n\n def run(self):\n repository_id = self.gh(\"GET\", \"\")[\"id\"]\n label = self.config.get(\"trigger_label\", workflow.TRIGGER_LABEL)\n payload = self._event_payload()\n event_mode = payload is not None\n if not event_mode:\n prs = self._rotating_window(\n self.gh_pages(\"/pulls?state=open&sort=updated&direction=asc\"), label\n )\n else:\n candidate = self._event_candidate(payload)\n if candidate and self.github_login.lower() != self.trigger_reviewer:\n raise RuntimeError(\n \"The configured GitHub credential must authenticate as \"\n f\"{self.trigger_reviewer} for reviewer-request mode\"\n )\n prs = [candidate] if candidate else []\n failures = []\n for candidate in prs:\n try:\n pr = self.gh(\"GET\", f\"/pulls/{candidate['number']}\")\n has_label = label in {\n item[\"name\"] for item in pr.get(\"labels\", [])\n }\n requested = self._outstanding_review_request(pr)\n trigger_label = label if (not event_mode and has_label) else None\n unrequested_candidate = False\n if event_mode or has_label:\n trigger = (\n self._latest_reviewer_request(pr[\"number\"])\n if event_mode\n else workflow._latest_trigger_label_event(\n self.token, self.repository, pr[\"number\"]\n )\n )\n delivery_key = None\n elif requested:\n # The outstanding request is the trigger, so its own event\n # keys the delivery and dedupes repeated scans.\n trigger = self._latest_reviewer_request(pr[\"number\"])\n delivery_key = None\n else:\n # No label and no outstanding request: an unrequested PR the\n # scheduled scan reviews on its own, keyed by repository/PR/\n # head. A draft is not reviewable, and a head this account\n # already reviewed is done - reconcile that review's verdict\n # and maintainer handoff, then skip it so no second\n # conversation or review is created.\n if pr.get(\"draft\"):\n continue\n if self._has_current_head_review(pr[\"number\"], pr[\"head\"][\"sha\"]):\n self._finish_completed_review(pr, None)\n continue\n trigger = None\n delivery_key = self._unrequested_head(pr)\n unrequested_candidate = True\n if trigger is None and delivery_key is None:\n continue\n if delivery_key is None and self._finish_completed_review(\n pr, trigger, trigger_label\n ):\n continue\n if event_mode and payload.get(\"action\") == \"submitted\":\n # A submitted review is a completion signal, never a fresh\n # trigger. A non-decisive review, or one superseded by a new\n # head, must wait for another reviewer request instead of\n # dispatching a review the caller never asked for.\n continue\n sha = pr[\"head\"][\"sha\"]\n gate_state, sha = self._gate_head(\n pr,\n requested=event_mode,\n scheduled=not event_mode,\n explain=not unrequested_candidate,\n )\n if gate_state != \"green\":\n # A deterministic blocker stops the run without spending a\n # worker slot on an agent. The trigger is not consumed: the\n # next scheduled scan or explicit request re-evaluates the\n # head once its checks are non-blocking. An unrequested head\n # that is merely red or pending gets no managed comment, so a\n # scan over a large backlog cannot storm the PRs with gate\n # comments; the managed comment answers an explicit request.\n print(\n json.dumps(\n {\n \"repository\": self.repository,\n \"pr\": pr[\"number\"],\n \"head_sha\": sha,\n \"disposition\": f\"review-{gate_state}\",\n }\n ),\n flush=True,\n )\n continue\n record = {\n \"repository\": self.repository,\n \"number\": pr[\"number\"],\n # An explicit request is ordered before an unrequested\n # candidate; within a priority the oldest candidate drains\n # first - the request time for a requested PR, the PR's own\n # creation time (oldest first) for an unrequested one - and\n # repository and number break a tie so the order is\n # deterministic.\n \"priority\": 0 if (has_label or requested) else 1,\n \"created_at\": (\n trigger.get(\"created_at\") or \"\"\n if trigger\n else pr.get(\"created_at\") or \"\"\n ),\n \"config\": self.config,\n \"start\": lambda pr=pr, trigger=trigger, sha=sha,\n trigger_label=trigger_label,\n delivery_key=delivery_key: self._start_review(\n repository_id, pr, trigger, sha, trigger_label, delivery_key\n ),\n }\n if event_mode:\n # An explicit request is a caller's decision to spend a\n # conversation now, not a backlog item, so the event path\n # dispatches immediately and is never bounded by the\n # scheduled scan's per-run maximum.\n self._start_review(\n repository_id, pr, trigger, sha, trigger_label, delivery_key\n )\n else:\n self.intake.register(record)\n except Exception as exc: # noqa: BLE001 - one PR must not block the scan\n failures.append(candidate.get(\"number\", \"?\"))\n print(\n f\"Failed to submit {self.repository} PR \"\n f\"#{candidate.get('number', '?')}: \"\n f\"{type(exc).__name__}: {exc}\",\n file=sys.stderr,\n flush=True,\n )\n if self.scan_intake is None:\n # A run() with no shared scan intake owns its whole scan, so it\n # drains the candidates it collected, bounded by the per-run maximum.\n self.intake.drain()\n if failures:\n raise RuntimeError(\n \"Reviewer scan failed for PRs: \"\n + \", \".join(f\"#{number}\" for number in failures)\n )\n\n def _start_review(\n self, repository_id, pr, trigger, sha, trigger_label, delivery_key=None\n ):\n result = self.dispatcher.deliver(\n subject=f\"{repository_id}:pr:{pr['number']}\",\n delivery=delivery_key or f\"{trigger['id']}:{sha}\",\n prompt=self._prompt(pr, trigger, trigger_label, delivery_key),\n )\n print(\n json.dumps(\n {\n \"repository\": self.repository,\n \"pr\": pr[\"number\"],\n \"head_sha\": sha,\n \"disposition\": result[\"disposition\"],\n \"conversation_id\": result[\"conversation_id\"],\n }\n ),\n flush=True,\n )\n return result\n\n\ndef run_scan(dispatcher):\n \"\"\"Run one scheduled scan over every configured repository, then drain.\n\n One shared intake spans the whole scan, so the per-run maximum is global\n rather than reset per repository, and the drain happens after every\n repository has been scanned so the oldest outstanding request across all of\n them starts first. One repository failing must not discard another\n repository's drained candidates, so the drain still runs and the first\n failure is what the scan reports.\n \"\"\"\n PullRequestReviewer.scan_intake = ReviewIntake()\n failure = None\n try:\n run_repositories(PullRequestReviewer, dispatcher=dispatcher)\n except Exception as exc: # noqa: BLE001 - reported after the drain\n failure = exc\n try:\n PullRequestReviewer.scan_intake.drain()\n except Exception as exc: # noqa: BLE001 - reported after the scan failure\n failure = failure or exc\n if failure is not None:\n raise failure\n\n\nif __name__ == \"__main__\":\n with AgentConversationDispatcher() as dispatcher:\n run_scan(dispatcher)\n" }, "github-issue-to-pr": { "agent_conversation.py": "\"\"\"Idempotently deliver automation work to profile-backed conversations.\"\"\"\n\nimport json\nimport os\nfrom urllib.error import HTTPError\nfrom urllib.request import Request, urlopen\nfrom uuid import NAMESPACE_URL, UUID, uuid5\n\nimport httpx\nfrom openhands.sdk import RemoteConversation\nfrom openhands.sdk.conversation.request import (\n SendMessageRequest,\n StartConversationRequest,\n)\nfrom openhands.sdk.conversation.state import ConversationExecutionStatus\nfrom openhands.sdk.llm.message import TextContent\nfrom openhands.sdk.workspace import LocalWorkspace, RemoteWorkspace\n\n_CONVERSATION_KEY_PREFIX = \"agent-conversation-\"\n\n\ndef _register_tools() -> None:\n \"\"\"Register tool models needed to deserialize an attached agent.\"\"\"\n from openhands.tools import register_default_tools\n\n register_default_tools()\n\n\ndef _kv_request(key: str, method: str, value: dict | None = None) -> dict | None:\n base_url = os.environ.get(\"AUTOMATION_API_URL\", \"\").rstrip(\"/\")\n token = os.environ.get(\"AUTOMATION_KV_TOKEN\", \"\")\n if not base_url or not token:\n raise RuntimeError(\"Automation KV is required for agent conversation dispatch\")\n request = Request(\n f\"{base_url}/v1/kv/{key}\",\n data=json.dumps(value).encode() if value is not None else None,\n headers={\n \"Authorization\": f\"Bearer {token}\",\n \"Content-Type\": \"application/json\",\n },\n method=method,\n )\n try:\n with urlopen(request, timeout=90) as response:\n body = json.load(response)\n except HTTPError as exc:\n if method == \"GET\" and exc.code == 404:\n return None\n raise\n return body.get(\"value\") if method == \"GET\" else body\n\n\nclass AgentConversationDispatcher:\n \"\"\"Deliver one revision at a time to a stable conversation for each subject.\"\"\"\n\n def __init__(self) -> None:\n self.agent_url = os.environ[\"AGENT_SERVER_URL\"]\n self.api_key = os.environ[\"SESSION_API_KEY\"]\n self.profile_id = UUID(os.environ[\"AUTOMATION_AGENT_PROFILE_ID\"])\n payload = json.loads(os.environ[\"AUTOMATION_EVENT_PAYLOAD\"])\n self.automation_id = str(payload[\"automation_id\"])\n self._workspace: RemoteWorkspace | None = None\n self._secrets = {}\n\n def __enter__(self):\n _register_tools()\n self._workspace = RemoteWorkspace(\n host=self.agent_url,\n api_key=self.api_key,\n working_dir=os.environ.get(\"WORKSPACE_BASE\", \"/workspace\"),\n )\n self._workspace.__enter__()\n self._secrets = self._workspace.get_secrets(\n agent_profile_id=str(self.profile_id)\n )\n return self\n\n def __exit__(self, *args):\n assert self._workspace is not None\n return self._workspace.__exit__(*args)\n\n def deliver(self, subject: str, delivery: str, prompt: str) -> dict[str, str]:\n conversation_id = uuid5(NAMESPACE_URL, f\"{self.automation_id}:{subject}\")\n state_key = f\"{_CONVERSATION_KEY_PREFIX}{conversation_id}\"\n record = _kv_request(state_key, \"GET\") or {}\n if self._workspace is None:\n raise RuntimeError(\"AgentConversationDispatcher must be used as a context\")\n\n same_delivery = record.get(\"delivery\") == delivery\n\n try:\n conversation = RemoteConversation.attach(\n self._workspace, conversation_id, visualizer=None\n )\n disposition = \"resumed\"\n except httpx.HTTPStatusError as exc:\n if exc.response.status_code != 404:\n raise\n conversation = RemoteConversation.create(\n self._workspace,\n StartConversationRequest(\n workspace=LocalWorkspace(working_dir=\"/workspace\"),\n conversation_id=conversation_id,\n agent_profile_id=self.profile_id,\n secrets=self._secrets,\n initial_message=SendMessageRequest(\n content=[TextContent(text=prompt)], run=True\n ),\n ),\n visualizer=None,\n )\n disposition = \"created\"\n try:\n if disposition == \"resumed\":\n if same_delivery:\n if (\n conversation.state.execution_status\n == ConversationExecutionStatus.RUNNING\n ):\n conversation.update_secrets(self._secrets)\n disposition = \"in_progress\"\n elif conversation.state.execution_status in (\n ConversationExecutionStatus.IDLE,\n ConversationExecutionStatus.PAUSED,\n ):\n conversation.update_secrets(self._secrets)\n conversation.run(blocking=False)\n else:\n disposition = \"deduplicated\"\n else:\n conversation.update_secrets(self._secrets)\n conversation.send_message(prompt)\n conversation.run(blocking=False)\n finally:\n conversation.close()\n\n if disposition in (\"deduplicated\", \"in_progress\"):\n return {\n \"disposition\": disposition,\n \"conversation_id\": str(conversation_id),\n }\n\n _kv_request(\n state_key,\n \"PUT\",\n {\n \"subject\": subject,\n \"conversation_id\": str(conversation_id),\n \"delivery\": delivery,\n },\n )\n return {\n \"disposition\": disposition,\n \"conversation_id\": str(conversation_id),\n }\n", diff --git a/automations/catalog/github-pr-reviewer/manifest.json b/automations/catalog/github-pr-reviewer/manifest.json index 16483270..b794141b 100644 --- a/automations/catalog/github-pr-reviewer/manifest.json +++ b/automations/catalog/github-pr-reviewer/manifest.json @@ -1,9 +1,9 @@ { "id": "github-pr-reviewer", - "version": "1.7.0", + "version": "1.8.0", "name": "GitHub code review", "category": "Code review", - "description": "Review requested pull-request heads, resuming an outstanding reviewer request on a schedule once its checks are green, draining the backlog oldest-request first under a small per-scan new-conversation bound, record exact-head decisions, stop early on out-of-scope changes, and optionally request a code-aware human maintainer after approval or a maintainer-decision scope stop.", + "description": "Review requested pull-request heads, resume an outstanding reviewer request on a schedule once its checks are green, review a bounded rotating window of open non-draft pull requests that nobody requested once their current head is green and unreviewed, drain the backlog oldest-first under a small per-scan new-conversation bound, record exact-head decisions, stop early on out-of-scope changes, and optionally request a code-aware human maintainer after approval or a maintainer-decision scope stop.", "requires": { "integrations": {}, "features": [ @@ -13,7 +13,7 @@ }, "popularityRank": 100, "estimatedSetupMinutes": 4, - "exampleImplementation": "A scheduled scan of the trigger label and of open non-draft PRs holding an outstanding reviewer request, or a GitHub reviewer-request event, selects a PR head and submits an idempotent subject turn. The scheduled scan orders eligible heads by the oldest outstanding request and starts at most a small configurable number of new conversations per scan across every repository. The profile-backed agent clones that exact head in its runtime, applies an early repository-ownership and product/architecture scope gate, then reuses the established review workflow, runs appropriate tests, and publishes a readable native review. A native approval, or a maintainer-decision scope stop, can then be handed to a code-aware human maintainer.", + "exampleImplementation": "A scheduled scan of the trigger label, of open non-draft PRs holding an outstanding reviewer request, and of a bounded rotating window of open non-draft PRs nobody requested whose current head is green and unreviewed, or a GitHub reviewer-request event, selects a PR head and submits an idempotent subject turn. The unrequested window resumes from a per-repository position kept in the Automation KV store, so each scan reads a bounded number of pull requests while still covering the backlog over time, and explicit requests are always examined. The scheduled scan orders eligible heads with explicit requests first and then oldest-first, and starts at most a small configurable number of new conversations per scan across every repository. The profile-backed agent clones that exact head in its runtime, applies an early repository-ownership and product/architecture scope gate, then reuses the established review workflow, runs appropriate tests, and publishes a readable native review. A native approval, or a maintainer-decision scope stop, can then be handed to a code-aware human maintainer.", "impact": { "basis": "completed-runs", "one": "1 PR review sweep completed", @@ -28,7 +28,7 @@ "schedule": { "type": "cron", "label": "Check frequency", - "help": "How often to look for newly labelled pull requests and for outstanding reviewer requests whose checks have not yet finished.", + "help": "How often to look for newly labelled pull requests, outstanding reviewer requests whose checks have not yet finished, and open non-draft pull requests nobody requested that still need a review.", "default": "*/5 * * * *", "required": true }, @@ -68,7 +68,7 @@ "triggerLabel": { "type": "text", "label": "Trigger label", - "help": "Only pull requests carrying this label are reviewed.", + "help": "Pull requests carrying this label are reviewed. An open, non-draft pull request that nobody requested is also reviewed once its current head is green and carries no review yet, examined a bounded rotating window at a time so one scan reads only a slice of the backlog.", "default": "openhands-review", "required": true, "constraints": { @@ -135,7 +135,7 @@ } }, "bundle": { - "version": "1.7.0", + "version": "1.8.0", "entrypoint": "python3 worker.py", "timeout": 300, "files": { diff --git a/skills/github-pr-reviewer/README.md b/skills/github-pr-reviewer/README.md index a0b0b96a..3be7e8bb 100644 --- a/skills/github-pr-reviewer/README.md +++ b/skills/github-pr-reviewer/README.md @@ -27,6 +27,17 @@ This skill is activated by: - Resumes an outstanding reviewer request on the next scheduled scan once the requested head's checks are green, so a request that arrives during CI is not lost; the explicit-request event path is kept for event-only deployments +- Reviews open, non-draft PRs that nobody requested on a scheduled scan once + their current head is green and carries no review yet, keyed by + repository/PR/head so a repeat scan never duplicates a conversation or review + and a changed head becomes eligible again +- Examines a bounded, rotating window of that unrequested backlog per scan + (10 PRs per repository), remembering its position in the Automation KV store so + the next scan resumes past it; explicit requests and trigger labels are always + examined, never skipped behind the window +- Posts no managed gate comment for an unrequested PR that is merely red or + pending - the comment answers an explicit request, so a scan over a large + backlog cannot storm the PRs with comments - Names the retry the deployment actually has in the waiting comment: a scheduled scan where a cron trigger exists, and removing and re-requesting the bot where only the event trigger does diff --git a/skills/github-pr-reviewer/SKILL.md b/skills/github-pr-reviewer/SKILL.md index 2bf93a24..2f1646ea 100644 --- a/skills/github-pr-reviewer/SKILL.md +++ b/skills/github-pr-reviewer/SKILL.md @@ -64,6 +64,41 @@ symbolic base branch carries no branch rules of its own: `review_requested` event, so repeated scans reuse one conversation and one review instead of creating duplicates, and a PR whose request was answered or withdrawn simply drops out of `requested_reviewers`. +- The scheduled scan also reviews open, non-draft PRs that **nobody requested** + and that carry no trigger label, once their current head has no completed + review by this account. That is what keeps reviewing PRs after the outstanding + requests are exhausted. The delivery key is the stable + `scan:{repository}:{number}:{head}`, so repeated scans reuse one conversation + and one native review, and a changed head becomes eligible again under its new + SHA. +- The unrequested part of a scan is a **bounded, rotating window**, not the whole + backlog. Classifying one unrequested head costs a review read plus the + exact-head check and workflow reads, so examining every open PR in one scan + spent the run's API budget in the largest repository and announced every red or + pending head. A scan examines at most `SCAN_WINDOW` (10) unrequested PRs per + repository and stores where the window ended in the automation service's own + KV store, under a per-repository `review-scan:{owner}__{repo}` key, so the next + scan resumes past that point and the backlog is covered fairly over several + scans. The window wraps at the end of the backlog. When the KV store is not + available (a local run) the position is kept in memory, so the scan still + rotates within the run. +- An explicit `all-hands-bot` review request or a trigger label is **never** + subject to the window: every explicit candidate is examined on every scan, + whatever the stored position is, so a request is not delayed behind the + rotation. +- A gate stop on an **unrequested** head posts no managed comment. A PR nobody + asked about that is merely red or pending is simply skipped without starting an + agent, which is what keeps a scan over a large backlog from posting a gate + comment on every PR. The managed comment is the answer to an explicit request, + so a requested or labeled head that is red or pending still gets its + explanation. +- Workflow runs are read as well as check runs, because a workflow can fail + before creating any check run - a workflow-level error, or a `pull_request` + run whose jobs never start. Such a run leaves a failed check suite with no + check runs under it, so the commit's check-run rollup and `gh pr checks` both + report success and only the workflow run reveals the red CI. A workflow run + whose check suite already reported check runs is left to those runs, so a + workflow is never counted twice. - Runs attributed to any other (obsolete) head SHA are ignored, so a stale failure cannot block the push that fixed it. This applies to workflow runs too. @@ -128,18 +163,23 @@ is `2`, and the rendered `config.json` overrides it through the `max_new_per_run` key that `github-issue-to-pr` and `gitlab-issue-to-mr` already use. -- Eligible candidates are ordered by the oldest outstanding `all-hands-bot` - `review_requested` event, then by repository, then by pull-request number, so - the oldest requests drain first and a later scan reaches the remainder. +- Eligible candidates are ordered deterministically: an explicit `all-hands-bot` + request (or a trigger label) is ordered before an unrequested PR, then by the + oldest `all-hands-bot` `review_requested` event for a requested PR or the PR's + own creation time (oldest first) for an unrequested one, then by repository, + then by pull-request number, so the oldest work drains first and a later scan + reaches the remainder. - The bound counts the conversations a scan **starts**. A delivery that only deduplicates or reports an already-running conversation reuses a runtime and consumes no slot, so repeated scans make progress on the backlog instead of re-spending the bound on work already in flight. - Reaching the bound never cuts the scan short. The scan still evaluates the - exact-head checks of the remaining candidates, posts its waiting or blocked - gate comments, reconciles completed reviews, and runs the maintainer handoff. - A candidate whose checks are pending or failing starts no conversation and - consumes no slot, so it cannot block a later green candidate. + exact-head checks of the remaining candidates, reconciles completed reviews, + and runs the maintainer handoff. A candidate whose checks are pending or + failing starts no conversation and consumes no slot, so it cannot block a later + green candidate. Only an explicit request or a trigger label gets a waiting or + blocked gate comment; an unrequested head that is merely red or pending is + skipped silently. - A dispatch that raises is reported and does not consume a slot or abort the scan, so the candidates behind it are still considered. - The explicit `review_requested` event path and the trigger-label scan are @@ -459,7 +499,7 @@ The completion callback fires once for the whole run. | Review paused with a failing-check comment | A current-head required check reported `failure`, `cancelled`, or `timed_out` | Fix the named checks and push; the review starts on the new head, or request `all-hands-bot` to review immediately | | Review reported waiting on checks | A current-head required check is `queued` or `in_progress`, or has not reported yet | No action; a later scan or a new review request retries | | Optional workflow failed but no review was paused | The failed workflow is not required, so the required-only scheduled gate ignored it | No action; only GitHub-required checks gate scheduled discovery | -| Only a few reviews start on a large backlog | The per-scan `max_new_per_run` bound (default 2) reached | No action; later scans drain the remaining oldest requests, or raise `max_new_per_run` if the deployment can hold more agents | +| Only a few reviews start on a large backlog | The per-scan `max_new_per_run` bound (default 2) reached | No action; later scans drain the remaining oldest requests and the bounded rotating window of unrequested PRs, or raise `max_new_per_run` if the deployment can hold more agents | | Review result never posts | Conversation still running or stuck | Open the conversation link from the acknowledgement comment | | Stale review suppressed | PR head SHA changed while the agent was reviewing | Re-apply the trigger label after the latest commit | | Review arrives as a plain comment, not a review | Publishing failed, so the script posted the text as a fallback | Check that the token has Pull requests: Read and Write | diff --git a/skills/github-pr-reviewer/references/state-schema.md b/skills/github-pr-reviewer/references/state-schema.md index 4f6c877b..1ab4d6f9 100644 --- a/skills/github-pr-reviewer/references/state-schema.md +++ b/skills/github-pr-reviewer/references/state-schema.md @@ -18,6 +18,21 @@ store under the key `state:{owner}__{repo}` ā€” for example `AUTOMATION_KV_TOKEN` is injected into the run environment. Each automation has its own isolated namespace. +The scheduled worker (`worker.py`) uses the same store for one more document, the +**unrequested-scan cursor**, under the key `review-scan:{owner}__{repo}`: + +```json +{ "cursor": 20 } +``` + +`cursor` is the position in the repository's unrequested-PR backlog at which the +next scheduled scan's bounded window starts. Each scan examines at most +`SCAN_WINDOW` (10) unrequested PRs from that position, then writes the position +after the window, so successive scans rotate through the whole backlog instead of +reading one pull request per open PR. A cursor past the end wraps to the start. +This document is separate from `main.py`'s per-repository review state; a missing +or unreadable cursor simply starts the window at the beginning. + **Fallback (local/dev):** When the KV store is not available, the state is written to a local JSON file at: diff --git a/skills/github-pr-reviewer/scripts/worker.py b/skills/github-pr-reviewer/scripts/worker.py index dd0b8f36..632c60b3 100644 --- a/skills/github-pr-reviewer/scripts/worker.py +++ b/skills/github-pr-reviewer/scripts/worker.py @@ -27,6 +27,11 @@ # every current-head check and workflow run, so a red head still blocks. CHECK_GATE_MARKER = "" self._gate_comment( pr["number"], @@ -673,12 +778,92 @@ def _outstanding_review_request(self, pr): for item in pr.get("requested_reviewers") or [] ) + def _has_current_head_review(self, number, sha): + """Whether the reviewer account already published a review on this head. + + This is the candidate filter's negative: an open, non-draft PR without a + current-head review by the configured reviewer is eligible for a + scheduled scan even when nobody requested the bot. The same predicate is + what the completion handler reconciles, so a review this scan starts and + a review it finds already present are the same set. + """ + return any( + review.get("commit_id") == sha + and ((review.get("user") or {}).get("login") or "").lower() + == self.github_login.lower() + for review in self.gh_pages(f"/pulls/{number}/reviews") + ) + + def _unrequested_head(self, pr): + """The current-head delivery key for an unrequested PR, or None. + + The key is the repository/PR identity plus the head SHA, so repeated + scheduled scans over one head reuse one conversation and one native + review, while a changed head becomes eligible again under its new SHA. + """ + sha = pr["head"]["sha"] + return f"scan:{self.repository}:{pr['number']}:{sha}" + + @property + def _scan_cursor(self): + """This repository's scan position, created lazily like the intake.""" + cursor = self.__dict__.get("_cursor") + if cursor is None: + cursor = self.__dict__["_cursor"] = ScanCursor(self.repository) + return cursor + + def _explicit_candidate(self, pr, label): + """Whether a PR was explicitly requested by a caller. + + An explicit `all-hands-bot` review request or a trigger label is a + caller's decision, so it is never subject to the rotating window: every + explicit candidate is examined on every scan, whatever the stored scan + position is. + """ + if label in {item["name"] for item in pr.get("labels", [])}: + return True + return self._outstanding_review_request(pr) + + def _rotating_window(self, prs, label): + """The explicit candidates plus a bounded slice of the unrequested ones. + + Classifying an unrequested head costs a review read and the exact-head + check/workflow reads, so examining every open PR in one scan is what let + a single run exhaust the API budget and post a managed gate comment for + every red or pending head. The unrequested backlog is therefore examined + a bounded `SCAN_WINDOW` at a time, starting where the previous scan + stopped (the per-repository position in the Automation KV store), so + successive scans rotate through the whole backlog. Explicit candidates + are always included, so a request is never delayed behind the window. + """ + explicit, unrequested = [], [] + for pr in prs: + if pr.get("draft") and label not in { + item["name"] for item in pr.get("labels", []) + } and not self._outstanding_review_request(pr): + # A draft that is neither labeled nor requested is not reviewable + # by the unrequested path either, so drop it before the full read. + continue + if self._explicit_candidate(pr, label): + explicit.append(pr) + else: + unrequested.append(pr) + if not unrequested: + return explicit + start = self._scan_cursor.position(len(unrequested)) + window = unrequested[start : start + SCAN_WINDOW] + self._scan_cursor.advance(start + len(window)) + return explicit + window + def run(self): repository_id = self.gh("GET", "")["id"] label = self.config.get("trigger_label", workflow.TRIGGER_LABEL) payload = self._event_payload() - if payload is None: - prs = self.gh_pages("/pulls?state=open&sort=updated&direction=asc") + event_mode = payload is not None + if not event_mode: + prs = self._rotating_window( + self.gh_pages("/pulls?state=open&sort=updated&direction=asc"), label + ) else: candidate = self._event_candidate(payload) if candidate and self.github_login.lower() != self.trigger_reviewer: @@ -689,19 +874,14 @@ def run(self): prs = [candidate] if candidate else [] failures = [] for candidate in prs: - event_mode = payload is not None - if not event_mode and label not in { - item["name"] for item in candidate.get("labels", []) - } and not self._outstanding_review_request(candidate): - # A scheduled scan covers the trigger label and any PR that still - # holds a review request the CI gate deferred. Everything else is - # not ours to review. - continue try: pr = self.gh("GET", f"/pulls/{candidate['number']}") has_label = label in { item["name"] for item in pr.get("labels", []) } + requested = self._outstanding_review_request(pr) + trigger_label = label if (not event_mode and has_label) else None + unrequested_candidate = False if event_mode or has_label: trigger = ( self._latest_reviewer_request(pr["number"]) @@ -710,14 +890,32 @@ def run(self): self.token, self.repository, pr["number"] ) ) - else: + delivery_key = None + elif requested: # The outstanding request is the trigger, so its own event # keys the delivery and dedupes repeated scans. trigger = self._latest_reviewer_request(pr["number"]) - if trigger is None: + delivery_key = None + else: + # No label and no outstanding request: an unrequested PR the + # scheduled scan reviews on its own, keyed by repository/PR/ + # head. A draft is not reviewable, and a head this account + # already reviewed is done - reconcile that review's verdict + # and maintainer handoff, then skip it so no second + # conversation or review is created. + if pr.get("draft"): + continue + if self._has_current_head_review(pr["number"], pr["head"]["sha"]): + self._finish_completed_review(pr, None) + continue + trigger = None + delivery_key = self._unrequested_head(pr) + unrequested_candidate = True + if trigger is None and delivery_key is None: continue - trigger_label = label if (not event_mode and has_label) else None - if self._finish_completed_review(pr, trigger, trigger_label): + if delivery_key is None and self._finish_completed_review( + pr, trigger, trigger_label + ): continue if event_mode and payload.get("action") == "submitted": # A submitted review is a completion signal, never a fresh @@ -727,13 +925,19 @@ def run(self): continue sha = pr["head"]["sha"] gate_state, sha = self._gate_head( - pr, requested=event_mode, scheduled=not event_mode + pr, + requested=event_mode, + scheduled=not event_mode, + explain=not unrequested_candidate, ) if gate_state != "green": # A deterministic blocker stops the run without spending a # worker slot on an agent. The trigger is not consumed: the # next scheduled scan or explicit request re-evaluates the - # head once its checks are non-blocking. + # head once its checks are non-blocking. An unrequested head + # that is merely red or pending gets no managed comment, so a + # scan over a large backlog cannot storm the PRs with gate + # comments; the managed comment answers an explicit request. print( json.dumps( { @@ -749,13 +953,23 @@ def run(self): record = { "repository": self.repository, "number": pr["number"], - # The oldest outstanding request drains first; repository and - # number break a tie so the order is deterministic. - "created_at": trigger.get("created_at") or "", + # An explicit request is ordered before an unrequested + # candidate; within a priority the oldest candidate drains + # first - the request time for a requested PR, the PR's own + # creation time (oldest first) for an unrequested one - and + # repository and number break a tie so the order is + # deterministic. + "priority": 0 if (has_label or requested) else 1, + "created_at": ( + trigger.get("created_at") or "" + if trigger + else pr.get("created_at") or "" + ), "config": self.config, "start": lambda pr=pr, trigger=trigger, sha=sha, - trigger_label=trigger_label: self._start_review( - repository_id, pr, trigger, sha, trigger_label + trigger_label=trigger_label, + delivery_key=delivery_key: self._start_review( + repository_id, pr, trigger, sha, trigger_label, delivery_key ), } if event_mode: @@ -764,7 +978,7 @@ def run(self): # dispatches immediately and is never bounded by the # scheduled scan's per-run maximum. self._start_review( - repository_id, pr, trigger, sha, trigger_label + repository_id, pr, trigger, sha, trigger_label, delivery_key ) else: self.intake.register(record) @@ -787,11 +1001,13 @@ def run(self): + ", ".join(f"#{number}" for number in failures) ) - def _start_review(self, repository_id, pr, trigger, sha, trigger_label): + def _start_review( + self, repository_id, pr, trigger, sha, trigger_label, delivery_key=None + ): result = self.dispatcher.deliver( subject=f"{repository_id}:pr:{pr['number']}", - delivery=f"{trigger['id']}:{sha}", - prompt=self._prompt(pr, trigger, trigger_label), + delivery=delivery_key or f"{trigger['id']}:{sha}", + prompt=self._prompt(pr, trigger, trigger_label, delivery_key), ) print( json.dumps( diff --git a/skills/index.js b/skills/index.js index 8a9ed66b..25015f09 100644 --- a/skills/index.js +++ b/skills/index.js @@ -283,7 +283,7 @@ export const SKILLS_CATALOG = [ "triggers": [ "/pr-reviewer:setup" ], - "content": "# GitHub PR Reviewer Automation\n\n## Agent Canvas catalog\n\nFor new Agent Canvas installations, use the **GitHub code review** catalog\nentry. Its deterministic `worker.py` delegates each requested exact head to a\nstable conversation using the selected agent profile. It supports GitHub\nreviewer-request events and scheduled label scans. The manual upload flow below\nremains for existing deployments and is deprecated for new installations.\n\nCreate a cron automation that watches one or more GitHub repositories for pull\nrequests with a review trigger label, starts an OpenHands review conversation\nonce per label event, and publishes the AI review to GitHub.\nWindows PowerShell equivalents for the setup, packaging, upload, and API-check shell snippets are in `references/windows.md`.\n\nThe automation script is deterministic: PR discovery, label-event tracking,\nhead eligibility, state persistence, stale-result suppression, the repository\ncheckout, and its removal are all handled in Python. The LLM is invoked only for\nthe review itself.\n\nBefore any review conversation is created, a scheduled scan evaluates the\ncurrent head's GitHub-required checks, read through the GraphQL `isRequired`\nsignal for the pull request. That signal is the merge policy's own source of\ntruth and is pull-request-scoped, so it stays correct for a stacked PR whose\nsymbolic base branch carries no branch rules of its own:\n\n- Only checks GitHub reports as required decide the scheduled gate. An optional\n workflow that fails - including one that fails before creating any check run -\n does not block a head whose required checks pass.\n- A completed required run on the exact head whose conclusion is `failure`,\n `cancelled`, or `timed_out` blocks the review. Any other unrecognized\n conclusion fails closed as a block rather than silently approving.\n- A completed required run whose conclusion is `success`, `neutral`, or\n `skipped` does not block. A required `commit status` context is classified\n alongside required check runs.\n- A `queued` or `in_progress` required run on the exact head, and a required\n context that has not reported a current-head check run at all, make the run\n exit with a **waiting on checks** outcome. Unfulfilled is never approval. The\n worker does not hold a slot polling; the next scheduled scan or explicit\n review request retries.\n- When the required-check signal cannot be read or is empty, the gate falls back\n to every current-head check run and workflow run, so a red head still blocks.\n In that fallback, workflow runs are read as well as check runs, because a\n workflow can fail before creating any check run - a workflow-level error, or a\n `pull_request` run whose jobs never start. Such a run leaves a failed check\n suite with no check runs under it, so the commit's check-run rollup and\n `gh pr checks` both report success and only the workflow run reveals the red\n CI. A workflow run whose check suite already reported check runs is left to\n those runs, so a workflow is never counted twice.\n- A scheduled scan considers the trigger label **and** every open, non-draft PR\n that still holds an outstanding `all-hands-bot` review request. That is what\n resumes a request made while CI was running: the request is keyed by its own\n `review_requested` event, so repeated scans reuse one conversation and one\n review instead of creating duplicates, and a PR whose request was answered or\n withdrawn simply drops out of `requested_reviewers`.\n- Runs attributed to any other (obsolete) head SHA are ignored, so a stale\n failure cannot block the push that fixed it. This applies to workflow runs\n too.\n- Only the latest run of each logical check or workflow counts. A logical check\n is its name plus the reporting app identity, and a logical workflow is its\n name plus workflow ID. The latest is chosen by the run ID (the reliable\n creation sequence) with the start time as a tie-break, so a re-run that fixed\n a check supersedes its earlier failure on the same SHA while a newer queued or\n in-progress re-run supersedes an earlier success and makes the head wait.\n Ordering by the run ID keeps a run whose start time is still absent in its\n true creation position in both directions.\n\nAn explicit `all-hands-bot` review request is the intake-policy exception: the\ncaller asked for that head by name, so the request dispatches even when required\nCI is red or pending, and the gate leaves no explanatory comment. Draft, scope,\nexact-head, and delivery-deduplication safeguards still apply. A blocked or\nwaiting scheduled stop consumes no trigger: once the head's required checks are\nnon-blocking, the scheduled scan, or a new `all-hands-bot` review request,\nstarts the normal review.\n\nThe gate needs no configured list of check names. When it stops a review it\nleaves one concise explanation on the PR, identified by a hidden marker carrying\nthe head SHA and gate category, so a later run for a different head updates that\ncomment instead of posting another. Only a marker this reviewer account authored\ncounts as its own comment; every other PR comment is untrusted and can neither\nsuppress the explanation nor be edited. If the repository's own workflow already\nposted a deterministic remediation comment that names the same current-head\nchecks, the gate adds nothing; a disclosure about some other check does not\nsuppress it. The gate applies to scheduled label scans; an explicit\n`all-hands-bot` review request bypasses it by design.\n\nEach explicit review request - a re-applied trigger label, or a new reviewer\nrequest in event mode - is a fresh review. The conversation for a PR is reused,\nbut its earlier turns must not be trusted as current: before deciding a verdict\nthe reviewer re-fetches the mutable GitHub state (the exact head, the PR body,\nreview comments and threads, review requests, the linked issues' bodies and\nlabels, and the current-head Actions results) and ignores any earlier finding,\nverdict, or label/priority claim that state no longer supports. Repository\nanalysis the conversation already did, such as reading `AGENTS.md`, stays\nuseful and is not repeated.\n\nThe waiting and blocked explanations name the retry the deployment actually has.\nA scheduled run proves a scan is configured, so it says the next scan will\nretry. An event-only run does not, so it tells the reader to remove the\noutstanding `all-hands-bot` request and request `all-hands-bot` again instead of\npromising a scan that does not exist: GitHub will not accept a second request\nwhile the first is still outstanding.\n\nThe gate leaves only one explanation per head. When the same head keeps the same\nhidden marker but the retry wording changes -- an event-only comment on a head\nwhose automation is later switched to a cron scan -- the scheduled run rewrites\nthat managed comment in place, so the comment always names the retry that is\nactually deployed, and an unchanged body is left untouched.\n\n## Bounded intake per scheduled scan\n\nA scheduled scan drains outstanding reviewer requests fairly, but starting an\nagent for every eligible pull request at once would overload the deployment, so\nthe scan starts at most `MAX_NEW_PER_RUN` new review conversations, counted\n**across every configured repository** rather than per repository. The default\nis `2`, and the rendered `config.json` overrides it through the\n`max_new_per_run` key that `github-issue-to-pr` and `gitlab-issue-to-mr` already\nuse.\n\n- Eligible candidates are ordered by the oldest outstanding `all-hands-bot`\n `review_requested` event, then by repository, then by pull-request number, so\n the oldest requests drain first and a later scan reaches the remainder.\n- The bound counts the conversations a scan **starts**. A delivery that only\n deduplicates or reports an already-running conversation reuses a runtime and\n consumes no slot, so repeated scans make progress on the backlog instead of\n re-spending the bound on work already in flight.\n- Reaching the bound never cuts the scan short. The scan still evaluates the\n exact-head checks of the remaining candidates, posts its waiting or blocked\n gate comments, reconciles completed reviews, and runs the maintainer handoff.\n A candidate whose checks are pending or failing starts no conversation and\n consumes no slot, so it cannot block a later green candidate.\n- A dispatch that raises is reported and does not consume a slot or abort the\n scan, so the candidates behind it are still considered.\n- The explicit `review_requested` event path and the trigger-label scan are\n unchanged: an explicit request still starts its conversation immediately, and\n only the scheduled scan's new conversations are bounded.\n\nThe review prompt starts with a scope gate: using the repository's own guidance\n(its scope categories and ownership boundaries, not a list of individual PR\nnumbers), the reviewer decides whether the change belongs in this repository\nand has the product/architecture direction it needs. When it does not, the review\nstops with a single `event: COMMENT` review that says whether the change should\nmove repositories, close, or receive a maintainer decision, and ends with the\n`šŸ›‘ MAINTAINER DECISION REQUIRED` verdict. That outcome is **neither an approval\nnor a change request**: it does not approve or merge the PR. The completion\nhandler recognizes the verdict and requests one configured maintainer through the\nsame handoff used after an approval. An in-scope change continues the existing\nreview unchanged.\n\nThe script prepares each review's workspace before the agent starts: the pull\nrequest's head commit is downloaded as a tarball and extracted to a directory of\nits own, which becomes the conversation's working directory. The agent is told\nnot to clone, fetch, check out, or delete anything, and the script removes the\ncheckout once the conversation has stopped. Nothing accumulates between runs.\n\n---\n\nThe script imports shared GitHub transport from\n`scripts/github_client.py`, installed with this skill. Include it beside\n`main.py` when packaging manually, as shown below; catalog bundles include it\nautomatically.\n\n## Prerequisites\n\n### Required secret\n\nVerify that the following secret is set in **OpenHands Settings -> Secrets**:\n\n| Secret name | Token type | Minimum permissions |\n|---|---|---|\n| `GITHUB_PERSONAL_ACCESS_TOKEN` | Classic PAT | `repo` for private repos or `public_repo` for public repos |\n| `GITHUB_PERSONAL_ACCESS_TOKEN` | Fine-grained PAT | Contents: Read, Metadata: Read, Pull requests: **Read and Write**, Issues: Read and Write |\n\nPull-request **write** access is required because the agent publishes a pull\nrequest review, not just an issue comment. The Agent Canvas catalog worker may\nalso request a configured human reviewer after approval. A token with only Pull\nrequests: Read will poll happily and then fail at the point of publishing or\nrequesting the handoff.\n\nWhen several repositories are monitored, the token must cover all of them.\n\nCheck with:\n```bash\ncurl -s https://api.github.com/user \\\n -H \"Authorization: Bearer $GITHUB_PERSONAL_ACCESS_TOKEN\" \\\n | python3 -c \"import json,sys; d=json.load(sys.stdin); print(d.get('login') or d.get('message'))\"\n```\n\nIf the token is missing or invalid, inform the user and stop.\n\n---\n\n## Setup Workflow\n\nFollow these steps in order.\n\n### Step 1 - Verify `GITHUB_PERSONAL_ACCESS_TOKEN`\n\nRun the `curl` check above.\n\n- If absent: *\"GITHUB_PERSONAL_ACCESS_TOKEN is not set. Please add it in\n OpenHands Settings -> Secrets.\"* Stop.\n- If the API returns `{\"message\": \"Bad credentials\"}`: tell the user the\n token is invalid and ask them to update it. Stop.\n\n### Step 2 - Collect repositories\n\nAsk: *\"Which GitHub repositories should be monitored?\n(Format: `owner/repo`, e.g. `myorg/backend`. List several separated by commas to\nreview them all from one automation.)\"*\n\nValidate access to **each** repository:\n```bash\ncurl -s \"https://api.github.com/repos/{owner}/{repo}\" \\\n -H \"Authorization: Bearer $GITHUB_PERSONAL_ACCESS_TOKEN\" \\\n | python3 -c \"\nimport json, sys\nd = json.load(sys.stdin)\nif 'message' in d:\n print('ERROR:', d['message'])\nelse:\n print(f\\\"Accessible. Private: {d.get('private')}. Permissions: {d.get('permissions')}\\\")\n\"\n```\n\nRecord every accepted repository into `REPOS = [\"{owner}/{repo}\", ...]`. If one\nrepository fails the check, say which and ask whether to continue without it.\n\nEach repository is polled independently and keeps its own state, so pull-request\nnumbers never collide between them. The trigger label, tone, and schedule are\nshared by all of them; a repository needing different settings wants its own\nautomation.\n\n### Step 3 - Collect trigger label\n\nAsk: *\"Which PR label should trigger a review?\n(Press Enter for the default: `openhands-review`.)\"*\n\nRecord the answer as `TRIGGER_LABEL`. If the label does not exist yet, tell the\nuser that GitHub will still record the event once the label is created and\napplied to a PR.\n\nThe automation reviews a PR when it sees the latest matching `labeled` event for\nthat label. To request another review later, remove and re-apply the label.\n\n### Step 4 - Collect review tone\n\nAsk: *\"What review tone should the reviewer use?\n 1. Thorough (default) - comprehensive coverage of correctness, security, tests, style\n 2. Concise - high-signal only, skips minor style feedback\n 3. Friendly - constructive and encouraging\n(Press Enter for Thorough, or type your choice or any custom style description)\"*\n\nMap the choice to `REVIEW_TONE`:\n\n| Answer | `REVIEW_TONE` | `REVIEW_STYLE_INSTRUCTIONS` |\n|---|---|---|\n| 1 / Enter | `\"thorough\"` | `\"\"` |\n| 2 | `\"concise\"` | `\"\"` |\n| 3 | `\"friendly\"` | `\"\"` |\n| Custom text, e.g. `strict but kind` | `\"thorough\"` | the custom text verbatim |\n\n### Step 5 - Collect cron schedule\n\nAsk: *\"How often should the automation poll for labeled PRs?\n(Press Enter for the default: every 5 minutes.\nUse a cron expression for a different interval, e.g. `0 * * * *` = hourly)\"*\n\nDefault: `*/5 * * * *`.\n\nRecord as `CRON_SCHEDULE`.\n\n### Step 6 - Generate the automation script\n\nRead `scripts/main.py` from this skill's directory. Apply exactly seven constant\nsubstitutions near the top of the file:\n\n> The script also reads a `config.json` shipped beside it, if there is one, over\n> these constants. That is how the catalog entry\n> (`automations/catalog/github-pr-reviewer/`) configures an unmodified copy,\n> since a declarative host cannot rewrite Python. This setup path substitutes the\n> constants and ships no `config.json`, so the two never collide.\n\n| Placeholder | Replace with |\n|---|---|\n| `REPOS = [\"owner/repo\"]` | `REPOS = [\"{owner_repo}\", ...]` - one entry per repository collected in Step 2 |\n| `TRIGGER_LABEL = \"openhands-review\"` | `TRIGGER_LABEL = \"{trigger_label}\"` |\n| `REVIEW_TONE = \"thorough\"` | `REVIEW_TONE = \"{review_tone}\"` |\n| `REVIEW_STYLE_INSTRUCTIONS = \"\"` | `REVIEW_STYLE_INSTRUCTIONS = \"{style_instructions}\"` |\n| `REPO_REVIEW_GUIDE_PATH = \".agents/skills/custom-codereview-guide.md\"` | leave unchanged to auto-load a repo review guide at this path, or set to `\"\"` to disable |\n| `MAX_NEW_PER_RUN = 2` | leave unchanged to bound a scheduled scan to two new review conversations across all repositories, or raise it if the deployment can hold more agents at once |\n| `DEFAULT_OPENHANDS_URL = \"http://localhost:8000\"` | leave unchanged unless the user has a preference |\n\nUse a safe string writer such as `json.dumps(value)` when inserting user-provided\nrepository names, labels, or style instructions into Python string literals.\n`json.dumps(list_of_repos)` produces the whole `REPOS` list safely in one step.\n\nRun these commands from this skill's directory and write the customized script\nto a temporary build directory:\n```bash\nmkdir -p /tmp/pr-reviewer-build\ncp -L scripts/github_client.py /tmp/pr-reviewer-build/github_client.py\n# write the customized main.py to /tmp/pr-reviewer-build/main.py\n```\n\nValidate syntax before packaging:\n```bash\npython3 -m py_compile /tmp/pr-reviewer-build/main.py && echo \"Syntax OK\"\n```\n\nFix any syntax errors before proceeding.\n\n### Step 7 - Package and upload\n\nDetermine the Automation backend URL and auth from the ``\nblock in your system context:\n- **OPENHANDS_HOST**: the Automation backend `url_from_agent`\n- **Auth**: `X-Session-API-Key: $OPENHANDS_AUTOMATION_API_KEY`\n\n```bash\ntar -czf /tmp/pr-reviewer.tar.gz -C /tmp/pr-reviewer-build .\n\nTARBALL_PATH=$(curl -s -X POST \\\n \"${OPENHANDS_HOST}/api/automation/v1/uploads?name=github-pr-reviewer\" \\\n -H \"X-Session-API-Key: $OPENHANDS_AUTOMATION_API_KEY\" \\\n -H \"Content-Type: application/gzip\" \\\n --data-binary @/tmp/pr-reviewer.tar.gz \\\n | python3 -c \"import json,sys; print(json.load(sys.stdin)['tarball_path'])\")\n\necho \"Uploaded: $TARBALL_PATH\"\n```\n\n### Step 8 - Register the automation\n\n```bash\ncurl -s -X POST \"${OPENHANDS_HOST}/api/automation/v1\" \\\n -H \"X-Session-API-Key: $OPENHANDS_AUTOMATION_API_KEY\" \\\n -H \"Content-Type: application/json\" \\\n -d \"{\n \\\"name\\\": \\\"GitHub PR Reviewer: {repo_summary} label {trigger_label}\\\",\n \\\"trigger\\\": {\\\"type\\\": \\\"cron\\\", \\\"schedule\\\": \\\"{cron_schedule}\\\"},\n \\\"tarball_path\\\": \\\"$TARBALL_PATH\\\",\n \\\"entrypoint\\\": \\\"python3 main.py\\\",\n \\\"timeout\\\": 600\n }\" | python3 -m json.tool\n```\n\nUse the single repository as `{repo_summary}` when there is one, and something\nlike `3 repos` when there are several. A poll now downloads a tarball per queued\nreview, so the timeout allows for that; a run never waits for a review to\nfinish, only for it to be started.\n\nRecord the returned `id`.\n\n### Step 9 - Confirm\n\nTell the user:\n\n> āœ… **GitHub PR Reviewer** is running!\n>\n> - Automation ID: `{id}`\n> - Repositories: `{owner}/{repo}`, ... (one line each)\n> - Trigger label: `{trigger_label}`\n> - Review tone: `{tone}`\n> - Polling schedule: `{cron_schedule}`\n> - State file per repository:\n> `~/.openhands/workspaces/automation-state/github_pr_reviewer_label_event_{id}_{owner}__{repo}.json`\n>\n> Apply the `{trigger_label}` label to a pull request to queue a review. Each\n> label event is processed once. To request another review, remove and re-apply\n> the label.\n>\n> The review is published as a pull request review on the head commit, with\n> inline comments where a finding maps to a changed line.\n\n---\n\n## Runtime Behaviour (per poll)\n\nEach cron run executes `main.py`, which resolves and validates\n`GITHUB_PERSONAL_ACCESS_TOKEN` once, then processes every repository in `REPOS`\nindependently. One repository failing does not stop the others; the run fails\nonly if every repository fails.\n\nFor each repository:\n\n1. Loads that repository's state (see `references/state-schema.md`).\n2. Verifies repository access.\n3. Lists open PRs, newest-updated first.\n4. For each open PR carrying `TRIGGER_LABEL`:\n - Refetches current PR metadata to avoid acting on stale list data.\n - Finds the latest matching GitHub `labeled` issue event.\n - Skips the event if it has already been tracked.\n - Downloads the PR's head commit as a tarball and extracts it to\n `{WORKSPACE_BASE}/repositories/{owner}__{repo}/pr-{number}-{sha12}`. The\n archive is checked as it is unpacked: a single root, no absolute or `..`\n paths, and symlinks skipped rather than materialised.\n - Starts an OpenHands conversation **whose working directory is that\n checkout**, with a review prompt carrying PR metadata, the exact head SHA,\n label event details, and the requirement to re-fetch the current mutable\n GitHub state before deciding a verdict.\n - Posts an acknowledgement comment with the label event, head SHA, and\n conversation link.\n - Records the review in state with `status: \"active\"` and the checkout path.\n - If the checkout or the conversation cannot be created, the checkout is\n removed and nothing is recorded, so the next poll retries the label event.\n5. For each active review conversation:\n - Marks it closed without posting if the PR has closed or merged.\n - Suppresses stale results if the PR head SHA changed after the review was\n queued.\n - When the conversation reaches `idle`, `finished`, `error`, or `stuck`,\n asks GitHub whether a review by the token's own user exists for that head\n SHA. If it does, the review is complete. If it does not, the agent's final\n response is posted as a comment so the work is not lost.\n - Abandons a conversation that has not reached a terminal status within two\n hours, so its checkout can be reclaimed.\n6. Removes the checkout of every finished review, but only after confirming the\n conversation has stopped - deleting it under a running agent would remove its\n working directory. When that cannot be confirmed the directory is left alone\n and the next poll tries again.\n7. Saves that repository's state atomically.\n\nThe completion callback fires once for the whole run.\n\n---\n\n## Additional Resources\n\n- **`references/state-schema.md`** - State JSON schema, field definitions, and\n review lifecycle diagram.\n- **`scripts/main.py`** - The complete automation script. Customize the five\n constants at the top before packaging.\n- **`tests/test_main.py`** - Unit tests for the checkout, its removal, and state\n handling. Run them from the skill root with `python -m pytest tests/` after\n editing the script.\n\n---\n\n## Troubleshooting\n\n| Symptom | Likely cause | Fix |\n|---|---|---|\n| Bot never queues reviews | Trigger label not present or no matching `labeled` event | Apply the configured label to the PR |\n| \"Bad credentials\" in run logs | Token expired | Rotate and update `GITHUB_PERSONAL_ACCESS_TOKEN` |\n| 404 on repo access | Repo name wrong or no access | Re-check the entry in `REPOS` and the token's permissions |\n| One repository is skipped, others work | That repository failed its access check | Read the `=== owner/repo ===` block in the run log |\n| Same PR not reviewed after new commits | Label event was already processed | Remove and re-apply the trigger label |\n| Review paused with a failing-check comment | A current-head required check reported `failure`, `cancelled`, or `timed_out` | Fix the named checks and push; the review starts on the new head, or request `all-hands-bot` to review immediately |\n| Review reported waiting on checks | A current-head required check is `queued` or `in_progress`, or has not reported yet | No action; a later scan or a new review request retries |\n| Optional workflow failed but no review was paused | The failed workflow is not required, so the required-only scheduled gate ignored it | No action; only GitHub-required checks gate scheduled discovery |\n| Only a few reviews start on a large backlog | The per-scan `max_new_per_run` bound (default 2) reached | No action; later scans drain the remaining oldest requests, or raise `max_new_per_run` if the deployment can hold more agents |\n| Review result never posts | Conversation still running or stuck | Open the conversation link from the acknowledgement comment |\n| Stale review suppressed | PR head SHA changed while the agent was reviewing | Re-apply the trigger label after the latest commit |\n| Review arrives as a plain comment, not a review | Publishing failed, so the script posted the text as a fallback | Check that the token has Pull requests: Read and Write |\n| Agent reports it cannot clone the repo | Prompt asked it not to; the workspace is already the checkout | No action - the code is at the head SHA in its working directory |\n| Checkouts remain under `repositories/` | Their conversations had not stopped yet | They are removed by a later poll once the conversation is terminal |", + "content": "# GitHub PR Reviewer Automation\n\n## Agent Canvas catalog\n\nFor new Agent Canvas installations, use the **GitHub code review** catalog\nentry. Its deterministic `worker.py` delegates each requested exact head to a\nstable conversation using the selected agent profile. It supports GitHub\nreviewer-request events and scheduled label scans. The manual upload flow below\nremains for existing deployments and is deprecated for new installations.\n\nCreate a cron automation that watches one or more GitHub repositories for pull\nrequests with a review trigger label, starts an OpenHands review conversation\nonce per label event, and publishes the AI review to GitHub.\nWindows PowerShell equivalents for the setup, packaging, upload, and API-check shell snippets are in `references/windows.md`.\n\nThe automation script is deterministic: PR discovery, label-event tracking,\nhead eligibility, state persistence, stale-result suppression, the repository\ncheckout, and its removal are all handled in Python. The LLM is invoked only for\nthe review itself.\n\nBefore any review conversation is created, a scheduled scan evaluates the\ncurrent head's GitHub-required checks, read through the GraphQL `isRequired`\nsignal for the pull request. That signal is the merge policy's own source of\ntruth and is pull-request-scoped, so it stays correct for a stacked PR whose\nsymbolic base branch carries no branch rules of its own:\n\n- Only checks GitHub reports as required decide the scheduled gate. An optional\n workflow that fails - including one that fails before creating any check run -\n does not block a head whose required checks pass.\n- A completed required run on the exact head whose conclusion is `failure`,\n `cancelled`, or `timed_out` blocks the review. Any other unrecognized\n conclusion fails closed as a block rather than silently approving.\n- A completed required run whose conclusion is `success`, `neutral`, or\n `skipped` does not block. A required `commit status` context is classified\n alongside required check runs.\n- A `queued` or `in_progress` required run on the exact head, and a required\n context that has not reported a current-head check run at all, make the run\n exit with a **waiting on checks** outcome. Unfulfilled is never approval. The\n worker does not hold a slot polling; the next scheduled scan or explicit\n review request retries.\n- When the required-check signal cannot be read or is empty, the gate falls back\n to every current-head check run and workflow run, so a red head still blocks.\n In that fallback, workflow runs are read as well as check runs, because a\n workflow can fail before creating any check run - a workflow-level error, or a\n `pull_request` run whose jobs never start. Such a run leaves a failed check\n suite with no check runs under it, so the commit's check-run rollup and\n `gh pr checks` both report success and only the workflow run reveals the red\n CI. A workflow run whose check suite already reported check runs is left to\n those runs, so a workflow is never counted twice.\n- A scheduled scan considers the trigger label **and** every open, non-draft PR\n that still holds an outstanding `all-hands-bot` review request. That is what\n resumes a request made while CI was running: the request is keyed by its own\n `review_requested` event, so repeated scans reuse one conversation and one\n review instead of creating duplicates, and a PR whose request was answered or\n withdrawn simply drops out of `requested_reviewers`.\n- The scheduled scan also reviews open, non-draft PRs that **nobody requested**\n and that carry no trigger label, once their current head has no completed\n review by this account. That is what keeps reviewing PRs after the outstanding\n requests are exhausted. The delivery key is the stable\n `scan:{repository}:{number}:{head}`, so repeated scans reuse one conversation\n and one native review, and a changed head becomes eligible again under its new\n SHA.\n- The unrequested part of a scan is a **bounded, rotating window**, not the whole\n backlog. Classifying one unrequested head costs a review read plus the\n exact-head check and workflow reads, so examining every open PR in one scan\n spent the run's API budget in the largest repository and announced every red or\n pending head. A scan examines at most `SCAN_WINDOW` (10) unrequested PRs per\n repository and stores where the window ended in the automation service's own\n KV store, under a per-repository `review-scan:{owner}__{repo}` key, so the next\n scan resumes past that point and the backlog is covered fairly over several\n scans. The window wraps at the end of the backlog. When the KV store is not\n available (a local run) the position is kept in memory, so the scan still\n rotates within the run.\n- An explicit `all-hands-bot` review request or a trigger label is **never**\n subject to the window: every explicit candidate is examined on every scan,\n whatever the stored position is, so a request is not delayed behind the\n rotation.\n- A gate stop on an **unrequested** head posts no managed comment. A PR nobody\n asked about that is merely red or pending is simply skipped without starting an\n agent, which is what keeps a scan over a large backlog from posting a gate\n comment on every PR. The managed comment is the answer to an explicit request,\n so a requested or labeled head that is red or pending still gets its\n explanation.\n- Workflow runs are read as well as check runs, because a workflow can fail\n before creating any check run - a workflow-level error, or a `pull_request`\n run whose jobs never start. Such a run leaves a failed check suite with no\n check runs under it, so the commit's check-run rollup and `gh pr checks` both\n report success and only the workflow run reveals the red CI. A workflow run\n whose check suite already reported check runs is left to those runs, so a\n workflow is never counted twice.\n- Runs attributed to any other (obsolete) head SHA are ignored, so a stale\n failure cannot block the push that fixed it. This applies to workflow runs\n too.\n- Only the latest run of each logical check or workflow counts. A logical check\n is its name plus the reporting app identity, and a logical workflow is its\n name plus workflow ID. The latest is chosen by the run ID (the reliable\n creation sequence) with the start time as a tie-break, so a re-run that fixed\n a check supersedes its earlier failure on the same SHA while a newer queued or\n in-progress re-run supersedes an earlier success and makes the head wait.\n Ordering by the run ID keeps a run whose start time is still absent in its\n true creation position in both directions.\n\nAn explicit `all-hands-bot` review request is the intake-policy exception: the\ncaller asked for that head by name, so the request dispatches even when required\nCI is red or pending, and the gate leaves no explanatory comment. Draft, scope,\nexact-head, and delivery-deduplication safeguards still apply. A blocked or\nwaiting scheduled stop consumes no trigger: once the head's required checks are\nnon-blocking, the scheduled scan, or a new `all-hands-bot` review request,\nstarts the normal review.\n\nThe gate needs no configured list of check names. When it stops a review it\nleaves one concise explanation on the PR, identified by a hidden marker carrying\nthe head SHA and gate category, so a later run for a different head updates that\ncomment instead of posting another. Only a marker this reviewer account authored\ncounts as its own comment; every other PR comment is untrusted and can neither\nsuppress the explanation nor be edited. If the repository's own workflow already\nposted a deterministic remediation comment that names the same current-head\nchecks, the gate adds nothing; a disclosure about some other check does not\nsuppress it. The gate applies to scheduled label scans; an explicit\n`all-hands-bot` review request bypasses it by design.\n\nEach explicit review request - a re-applied trigger label, or a new reviewer\nrequest in event mode - is a fresh review. The conversation for a PR is reused,\nbut its earlier turns must not be trusted as current: before deciding a verdict\nthe reviewer re-fetches the mutable GitHub state (the exact head, the PR body,\nreview comments and threads, review requests, the linked issues' bodies and\nlabels, and the current-head Actions results) and ignores any earlier finding,\nverdict, or label/priority claim that state no longer supports. Repository\nanalysis the conversation already did, such as reading `AGENTS.md`, stays\nuseful and is not repeated.\n\nThe waiting and blocked explanations name the retry the deployment actually has.\nA scheduled run proves a scan is configured, so it says the next scan will\nretry. An event-only run does not, so it tells the reader to remove the\noutstanding `all-hands-bot` request and request `all-hands-bot` again instead of\npromising a scan that does not exist: GitHub will not accept a second request\nwhile the first is still outstanding.\n\nThe gate leaves only one explanation per head. When the same head keeps the same\nhidden marker but the retry wording changes -- an event-only comment on a head\nwhose automation is later switched to a cron scan -- the scheduled run rewrites\nthat managed comment in place, so the comment always names the retry that is\nactually deployed, and an unchanged body is left untouched.\n\n## Bounded intake per scheduled scan\n\nA scheduled scan drains outstanding reviewer requests fairly, but starting an\nagent for every eligible pull request at once would overload the deployment, so\nthe scan starts at most `MAX_NEW_PER_RUN` new review conversations, counted\n**across every configured repository** rather than per repository. The default\nis `2`, and the rendered `config.json` overrides it through the\n`max_new_per_run` key that `github-issue-to-pr` and `gitlab-issue-to-mr` already\nuse.\n\n- Eligible candidates are ordered deterministically: an explicit `all-hands-bot`\n request (or a trigger label) is ordered before an unrequested PR, then by the\n oldest `all-hands-bot` `review_requested` event for a requested PR or the PR's\n own creation time (oldest first) for an unrequested one, then by repository,\n then by pull-request number, so the oldest work drains first and a later scan\n reaches the remainder.\n- The bound counts the conversations a scan **starts**. A delivery that only\n deduplicates or reports an already-running conversation reuses a runtime and\n consumes no slot, so repeated scans make progress on the backlog instead of\n re-spending the bound on work already in flight.\n- Reaching the bound never cuts the scan short. The scan still evaluates the\n exact-head checks of the remaining candidates, reconciles completed reviews,\n and runs the maintainer handoff. A candidate whose checks are pending or\n failing starts no conversation and consumes no slot, so it cannot block a later\n green candidate. Only an explicit request or a trigger label gets a waiting or\n blocked gate comment; an unrequested head that is merely red or pending is\n skipped silently.\n- A dispatch that raises is reported and does not consume a slot or abort the\n scan, so the candidates behind it are still considered.\n- The explicit `review_requested` event path and the trigger-label scan are\n unchanged: an explicit request still starts its conversation immediately, and\n only the scheduled scan's new conversations are bounded.\n\nThe review prompt starts with a scope gate: using the repository's own guidance\n(its scope categories and ownership boundaries, not a list of individual PR\nnumbers), the reviewer decides whether the change belongs in this repository\nand has the product/architecture direction it needs. When it does not, the review\nstops with a single `event: COMMENT` review that says whether the change should\nmove repositories, close, or receive a maintainer decision, and ends with the\n`šŸ›‘ MAINTAINER DECISION REQUIRED` verdict. That outcome is **neither an approval\nnor a change request**: it does not approve or merge the PR. The completion\nhandler recognizes the verdict and requests one configured maintainer through the\nsame handoff used after an approval. An in-scope change continues the existing\nreview unchanged.\n\nThe script prepares each review's workspace before the agent starts: the pull\nrequest's head commit is downloaded as a tarball and extracted to a directory of\nits own, which becomes the conversation's working directory. The agent is told\nnot to clone, fetch, check out, or delete anything, and the script removes the\ncheckout once the conversation has stopped. Nothing accumulates between runs.\n\n---\n\nThe script imports shared GitHub transport from\n`scripts/github_client.py`, installed with this skill. Include it beside\n`main.py` when packaging manually, as shown below; catalog bundles include it\nautomatically.\n\n## Prerequisites\n\n### Required secret\n\nVerify that the following secret is set in **OpenHands Settings -> Secrets**:\n\n| Secret name | Token type | Minimum permissions |\n|---|---|---|\n| `GITHUB_PERSONAL_ACCESS_TOKEN` | Classic PAT | `repo` for private repos or `public_repo` for public repos |\n| `GITHUB_PERSONAL_ACCESS_TOKEN` | Fine-grained PAT | Contents: Read, Metadata: Read, Pull requests: **Read and Write**, Issues: Read and Write |\n\nPull-request **write** access is required because the agent publishes a pull\nrequest review, not just an issue comment. The Agent Canvas catalog worker may\nalso request a configured human reviewer after approval. A token with only Pull\nrequests: Read will poll happily and then fail at the point of publishing or\nrequesting the handoff.\n\nWhen several repositories are monitored, the token must cover all of them.\n\nCheck with:\n```bash\ncurl -s https://api.github.com/user \\\n -H \"Authorization: Bearer $GITHUB_PERSONAL_ACCESS_TOKEN\" \\\n | python3 -c \"import json,sys; d=json.load(sys.stdin); print(d.get('login') or d.get('message'))\"\n```\n\nIf the token is missing or invalid, inform the user and stop.\n\n---\n\n## Setup Workflow\n\nFollow these steps in order.\n\n### Step 1 - Verify `GITHUB_PERSONAL_ACCESS_TOKEN`\n\nRun the `curl` check above.\n\n- If absent: *\"GITHUB_PERSONAL_ACCESS_TOKEN is not set. Please add it in\n OpenHands Settings -> Secrets.\"* Stop.\n- If the API returns `{\"message\": \"Bad credentials\"}`: tell the user the\n token is invalid and ask them to update it. Stop.\n\n### Step 2 - Collect repositories\n\nAsk: *\"Which GitHub repositories should be monitored?\n(Format: `owner/repo`, e.g. `myorg/backend`. List several separated by commas to\nreview them all from one automation.)\"*\n\nValidate access to **each** repository:\n```bash\ncurl -s \"https://api.github.com/repos/{owner}/{repo}\" \\\n -H \"Authorization: Bearer $GITHUB_PERSONAL_ACCESS_TOKEN\" \\\n | python3 -c \"\nimport json, sys\nd = json.load(sys.stdin)\nif 'message' in d:\n print('ERROR:', d['message'])\nelse:\n print(f\\\"Accessible. Private: {d.get('private')}. Permissions: {d.get('permissions')}\\\")\n\"\n```\n\nRecord every accepted repository into `REPOS = [\"{owner}/{repo}\", ...]`. If one\nrepository fails the check, say which and ask whether to continue without it.\n\nEach repository is polled independently and keeps its own state, so pull-request\nnumbers never collide between them. The trigger label, tone, and schedule are\nshared by all of them; a repository needing different settings wants its own\nautomation.\n\n### Step 3 - Collect trigger label\n\nAsk: *\"Which PR label should trigger a review?\n(Press Enter for the default: `openhands-review`.)\"*\n\nRecord the answer as `TRIGGER_LABEL`. If the label does not exist yet, tell the\nuser that GitHub will still record the event once the label is created and\napplied to a PR.\n\nThe automation reviews a PR when it sees the latest matching `labeled` event for\nthat label. To request another review later, remove and re-apply the label.\n\n### Step 4 - Collect review tone\n\nAsk: *\"What review tone should the reviewer use?\n 1. Thorough (default) - comprehensive coverage of correctness, security, tests, style\n 2. Concise - high-signal only, skips minor style feedback\n 3. Friendly - constructive and encouraging\n(Press Enter for Thorough, or type your choice or any custom style description)\"*\n\nMap the choice to `REVIEW_TONE`:\n\n| Answer | `REVIEW_TONE` | `REVIEW_STYLE_INSTRUCTIONS` |\n|---|---|---|\n| 1 / Enter | `\"thorough\"` | `\"\"` |\n| 2 | `\"concise\"` | `\"\"` |\n| 3 | `\"friendly\"` | `\"\"` |\n| Custom text, e.g. `strict but kind` | `\"thorough\"` | the custom text verbatim |\n\n### Step 5 - Collect cron schedule\n\nAsk: *\"How often should the automation poll for labeled PRs?\n(Press Enter for the default: every 5 minutes.\nUse a cron expression for a different interval, e.g. `0 * * * *` = hourly)\"*\n\nDefault: `*/5 * * * *`.\n\nRecord as `CRON_SCHEDULE`.\n\n### Step 6 - Generate the automation script\n\nRead `scripts/main.py` from this skill's directory. Apply exactly seven constant\nsubstitutions near the top of the file:\n\n> The script also reads a `config.json` shipped beside it, if there is one, over\n> these constants. That is how the catalog entry\n> (`automations/catalog/github-pr-reviewer/`) configures an unmodified copy,\n> since a declarative host cannot rewrite Python. This setup path substitutes the\n> constants and ships no `config.json`, so the two never collide.\n\n| Placeholder | Replace with |\n|---|---|\n| `REPOS = [\"owner/repo\"]` | `REPOS = [\"{owner_repo}\", ...]` - one entry per repository collected in Step 2 |\n| `TRIGGER_LABEL = \"openhands-review\"` | `TRIGGER_LABEL = \"{trigger_label}\"` |\n| `REVIEW_TONE = \"thorough\"` | `REVIEW_TONE = \"{review_tone}\"` |\n| `REVIEW_STYLE_INSTRUCTIONS = \"\"` | `REVIEW_STYLE_INSTRUCTIONS = \"{style_instructions}\"` |\n| `REPO_REVIEW_GUIDE_PATH = \".agents/skills/custom-codereview-guide.md\"` | leave unchanged to auto-load a repo review guide at this path, or set to `\"\"` to disable |\n| `MAX_NEW_PER_RUN = 2` | leave unchanged to bound a scheduled scan to two new review conversations across all repositories, or raise it if the deployment can hold more agents at once |\n| `DEFAULT_OPENHANDS_URL = \"http://localhost:8000\"` | leave unchanged unless the user has a preference |\n\nUse a safe string writer such as `json.dumps(value)` when inserting user-provided\nrepository names, labels, or style instructions into Python string literals.\n`json.dumps(list_of_repos)` produces the whole `REPOS` list safely in one step.\n\nRun these commands from this skill's directory and write the customized script\nto a temporary build directory:\n```bash\nmkdir -p /tmp/pr-reviewer-build\ncp -L scripts/github_client.py /tmp/pr-reviewer-build/github_client.py\n# write the customized main.py to /tmp/pr-reviewer-build/main.py\n```\n\nValidate syntax before packaging:\n```bash\npython3 -m py_compile /tmp/pr-reviewer-build/main.py && echo \"Syntax OK\"\n```\n\nFix any syntax errors before proceeding.\n\n### Step 7 - Package and upload\n\nDetermine the Automation backend URL and auth from the ``\nblock in your system context:\n- **OPENHANDS_HOST**: the Automation backend `url_from_agent`\n- **Auth**: `X-Session-API-Key: $OPENHANDS_AUTOMATION_API_KEY`\n\n```bash\ntar -czf /tmp/pr-reviewer.tar.gz -C /tmp/pr-reviewer-build .\n\nTARBALL_PATH=$(curl -s -X POST \\\n \"${OPENHANDS_HOST}/api/automation/v1/uploads?name=github-pr-reviewer\" \\\n -H \"X-Session-API-Key: $OPENHANDS_AUTOMATION_API_KEY\" \\\n -H \"Content-Type: application/gzip\" \\\n --data-binary @/tmp/pr-reviewer.tar.gz \\\n | python3 -c \"import json,sys; print(json.load(sys.stdin)['tarball_path'])\")\n\necho \"Uploaded: $TARBALL_PATH\"\n```\n\n### Step 8 - Register the automation\n\n```bash\ncurl -s -X POST \"${OPENHANDS_HOST}/api/automation/v1\" \\\n -H \"X-Session-API-Key: $OPENHANDS_AUTOMATION_API_KEY\" \\\n -H \"Content-Type: application/json\" \\\n -d \"{\n \\\"name\\\": \\\"GitHub PR Reviewer: {repo_summary} label {trigger_label}\\\",\n \\\"trigger\\\": {\\\"type\\\": \\\"cron\\\", \\\"schedule\\\": \\\"{cron_schedule}\\\"},\n \\\"tarball_path\\\": \\\"$TARBALL_PATH\\\",\n \\\"entrypoint\\\": \\\"python3 main.py\\\",\n \\\"timeout\\\": 600\n }\" | python3 -m json.tool\n```\n\nUse the single repository as `{repo_summary}` when there is one, and something\nlike `3 repos` when there are several. A poll now downloads a tarball per queued\nreview, so the timeout allows for that; a run never waits for a review to\nfinish, only for it to be started.\n\nRecord the returned `id`.\n\n### Step 9 - Confirm\n\nTell the user:\n\n> āœ… **GitHub PR Reviewer** is running!\n>\n> - Automation ID: `{id}`\n> - Repositories: `{owner}/{repo}`, ... (one line each)\n> - Trigger label: `{trigger_label}`\n> - Review tone: `{tone}`\n> - Polling schedule: `{cron_schedule}`\n> - State file per repository:\n> `~/.openhands/workspaces/automation-state/github_pr_reviewer_label_event_{id}_{owner}__{repo}.json`\n>\n> Apply the `{trigger_label}` label to a pull request to queue a review. Each\n> label event is processed once. To request another review, remove and re-apply\n> the label.\n>\n> The review is published as a pull request review on the head commit, with\n> inline comments where a finding maps to a changed line.\n\n---\n\n## Runtime Behaviour (per poll)\n\nEach cron run executes `main.py`, which resolves and validates\n`GITHUB_PERSONAL_ACCESS_TOKEN` once, then processes every repository in `REPOS`\nindependently. One repository failing does not stop the others; the run fails\nonly if every repository fails.\n\nFor each repository:\n\n1. Loads that repository's state (see `references/state-schema.md`).\n2. Verifies repository access.\n3. Lists open PRs, newest-updated first.\n4. For each open PR carrying `TRIGGER_LABEL`:\n - Refetches current PR metadata to avoid acting on stale list data.\n - Finds the latest matching GitHub `labeled` issue event.\n - Skips the event if it has already been tracked.\n - Downloads the PR's head commit as a tarball and extracts it to\n `{WORKSPACE_BASE}/repositories/{owner}__{repo}/pr-{number}-{sha12}`. The\n archive is checked as it is unpacked: a single root, no absolute or `..`\n paths, and symlinks skipped rather than materialised.\n - Starts an OpenHands conversation **whose working directory is that\n checkout**, with a review prompt carrying PR metadata, the exact head SHA,\n label event details, and the requirement to re-fetch the current mutable\n GitHub state before deciding a verdict.\n - Posts an acknowledgement comment with the label event, head SHA, and\n conversation link.\n - Records the review in state with `status: \"active\"` and the checkout path.\n - If the checkout or the conversation cannot be created, the checkout is\n removed and nothing is recorded, so the next poll retries the label event.\n5. For each active review conversation:\n - Marks it closed without posting if the PR has closed or merged.\n - Suppresses stale results if the PR head SHA changed after the review was\n queued.\n - When the conversation reaches `idle`, `finished`, `error`, or `stuck`,\n asks GitHub whether a review by the token's own user exists for that head\n SHA. If it does, the review is complete. If it does not, the agent's final\n response is posted as a comment so the work is not lost.\n - Abandons a conversation that has not reached a terminal status within two\n hours, so its checkout can be reclaimed.\n6. Removes the checkout of every finished review, but only after confirming the\n conversation has stopped - deleting it under a running agent would remove its\n working directory. When that cannot be confirmed the directory is left alone\n and the next poll tries again.\n7. Saves that repository's state atomically.\n\nThe completion callback fires once for the whole run.\n\n---\n\n## Additional Resources\n\n- **`references/state-schema.md`** - State JSON schema, field definitions, and\n review lifecycle diagram.\n- **`scripts/main.py`** - The complete automation script. Customize the five\n constants at the top before packaging.\n- **`tests/test_main.py`** - Unit tests for the checkout, its removal, and state\n handling. Run them from the skill root with `python -m pytest tests/` after\n editing the script.\n\n---\n\n## Troubleshooting\n\n| Symptom | Likely cause | Fix |\n|---|---|---|\n| Bot never queues reviews | Trigger label not present or no matching `labeled` event | Apply the configured label to the PR |\n| \"Bad credentials\" in run logs | Token expired | Rotate and update `GITHUB_PERSONAL_ACCESS_TOKEN` |\n| 404 on repo access | Repo name wrong or no access | Re-check the entry in `REPOS` and the token's permissions |\n| One repository is skipped, others work | That repository failed its access check | Read the `=== owner/repo ===` block in the run log |\n| Same PR not reviewed after new commits | Label event was already processed | Remove and re-apply the trigger label |\n| Review paused with a failing-check comment | A current-head required check reported `failure`, `cancelled`, or `timed_out` | Fix the named checks and push; the review starts on the new head, or request `all-hands-bot` to review immediately |\n| Review reported waiting on checks | A current-head required check is `queued` or `in_progress`, or has not reported yet | No action; a later scan or a new review request retries |\n| Optional workflow failed but no review was paused | The failed workflow is not required, so the required-only scheduled gate ignored it | No action; only GitHub-required checks gate scheduled discovery |\n| Only a few reviews start on a large backlog | The per-scan `max_new_per_run` bound (default 2) reached | No action; later scans drain the remaining oldest requests and the bounded rotating window of unrequested PRs, or raise `max_new_per_run` if the deployment can hold more agents |\n| Review result never posts | Conversation still running or stuck | Open the conversation link from the acknowledgement comment |\n| Stale review suppressed | PR head SHA changed while the agent was reviewing | Re-apply the trigger label after the latest commit |\n| Review arrives as a plain comment, not a review | Publishing failed, so the script posted the text as a fallback | Check that the token has Pull requests: Read and Write |\n| Agent reports it cannot clone the repo | Prompt asked it not to; the workspace is already the checkout | No action - the code is at the head SHA in its working directory |\n| Checkouts remain under `repositories/` | Their conversations had not stopped yet | They are removed by a later poll once the conversation is terminal |", "category": "automations" }, { diff --git a/tests/fixtures/automations/github-pr-reviewer.json b/tests/fixtures/automations/github-pr-reviewer.json index 2408083a..0e0abfc9 100644 --- a/tests/fixtures/automations/github-pr-reviewer.json +++ b/tests/fixtures/automations/github-pr-reviewer.json @@ -68,7 +68,7 @@ "timeout": 300, "template": { "id": "github-pr-reviewer", - "version": "1.7.0", + "version": "1.8.0", "config": { "repos": [ "OpenHands/agent-server-gui" @@ -108,7 +108,7 @@ "timeout": 300, "template": { "id": "github-pr-reviewer", - "version": "1.7.0", + "version": "1.8.0", "config": { "repos": [ "OpenHands/agent-server-gui" @@ -213,7 +213,7 @@ "timeout": 300, "template": { "id": "github-pr-reviewer", - "version": "1.7.0", + "version": "1.8.0", "config": { "repos": [ "OpenHands/agent-server-gui", @@ -254,7 +254,7 @@ "timeout": 300, "template": { "id": "github-pr-reviewer", - "version": "1.7.0", + "version": "1.8.0", "config": { "repos": [ "OpenHands/agent-server-gui", @@ -296,7 +296,7 @@ "preset_metadata": { "template": { "id": "github-pr-reviewer", - "version": "1.7.0", + "version": "1.8.0", "config": { "repos": [ "OpenHands/agent-server-gui", @@ -351,7 +351,7 @@ "timeout": 300, "template": { "id": "github-pr-reviewer", - "version": "1.7.0", + "version": "1.8.0", "config": { "repos": [ "OpenHands/agent-server-gui" @@ -444,7 +444,7 @@ "timeout": 300, "template": { "id": "github-pr-reviewer", - "version": "1.7.0", + "version": "1.8.0", "config": { "repos": [ "OpenHands/agent-server-gui" @@ -540,7 +540,7 @@ "timeout": 300, "template": { "id": "github-pr-reviewer", - "version": "1.7.0", + "version": "1.8.0", "config": { "repos": [ "OpenHands/agent-server-gui" @@ -616,7 +616,7 @@ "timeout": 300, "template": { "id": "github-pr-reviewer", - "version": "1.7.0", + "version": "1.8.0", "config": { "repos": [ "OpenHands/automation" diff --git a/tests/test_github_reviewer_delivery.py b/tests/test_github_reviewer_delivery.py index 2b33a757..39d0d8c2 100644 --- a/tests/test_github_reviewer_delivery.py +++ b/tests/test_github_reviewer_delivery.py @@ -138,7 +138,9 @@ def test_reviewer_submits_each_labeled_exact_head(tmp_path, monkeypatch): "head": {"sha": "head-2"}, "labels": [{"name": "openhands-review"}], }, - {"number": 1, "head": {"sha": "head-1"}, "labels": []}, + # A draft is not eligible for the unrequested scan, so it is skipped + # without a full read and only the labeled head is submitted. + {"number": 1, "head": {"sha": "head-1"}, "labels": [], "draft": True}, ] run.gh_pages = lambda path: prs run.gh = Mock(side_effect=[{"id": 99}, prs[0]]) @@ -417,9 +419,12 @@ def test_reviewer_does_not_trust_another_reviewers_verdict(tmp_path, monkeypatch run.dispatcher.deliver.assert_called_once() -def test_reviewer_ignores_unlabeled_prs(tmp_path, monkeypatch): +def test_reviewer_ignores_a_draft_unlabeled_unrequested_pr(tmp_path, monkeypatch): + """The unrequested scan skips a draft: it is not reviewable yet.""" _module, run = _reviewer(tmp_path, monkeypatch) - run.gh_pages = lambda path: [{"number": 1, "labels": [], "head": {"sha": "head"}}] + run.gh_pages = lambda path: [ + {"number": 1, "labels": [], "head": {"sha": "head"}, "draft": True} + ] run.gh = Mock(return_value={"id": 99}) submit = Mock() run.dispatcher.deliver = submit @@ -427,6 +432,8 @@ def test_reviewer_ignores_unlabeled_prs(tmp_path, monkeypatch): run.run() submit.assert_not_called() + # Skipped before the full PR read, so only the repository-id lookup ran. + assert run.gh.call_count == 1 def test_reviewer_continues_after_one_submission_fails_then_reports_run_failure( @@ -1627,18 +1634,48 @@ def test_reviewer_scheduled_scan_ignores_a_draft_with_a_request( run.dispatcher.deliver.assert_not_called() -def test_reviewer_scheduled_scan_ignores_an_unlabeled_unrequested_pr( +def test_reviewer_scheduled_scan_reviews_an_unrequested_green_pr( tmp_path, monkeypatch ): + """A green PR with no request and no label is still reviewed on a scan.""" _module, run = _reviewer(tmp_path, monkeypatch) pr = {"number": 3, "head": {"sha": "head-3"}, "labels": [], "draft": False} - run.gh_pages = lambda path: [pr] - run.gh = Mock(return_value={"id": 99}) + run.gh_pages = lambda path: ( + [] if path.endswith("/reviews") else [pr] + ) + run.gh = Mock(side_effect=[{"id": 99}, pr]) + run.check_runs = lambda sha: _checks(("ci", "completed", "success"), sha=sha) + run.dispatcher.deliver.return_value = { + "disposition": "created", + "conversation_id": "conversation", + } + + run.run() + + run.dispatcher.deliver.assert_called_once() + call = run.dispatcher.deliver.call_args.kwargs + assert call["subject"] == "99:pr:3" + assert call["delivery"] == "scan:owner/repo:3:head-3" + assert "scheduled scan of open, non-draft pull requests" in call["prompt"] + + +def test_reviewer_scheduled_scan_skips_a_pr_with_a_current_head_review( + tmp_path, monkeypatch +): + """An already-reviewed head is not re-reviewed by the unrequested scan.""" + _module, run = _reviewer(tmp_path, monkeypatch) + pr = {"number": 3, "head": {"sha": "head-3"}, "labels": [], "draft": False} + run.gh_pages = lambda path: ( + _reviews(sha="head-3") if path.endswith("/reviews") else [pr] + ) + # The repository-id lookup, the full PR read, and the head re-read that the + # completion path performs to confirm the reviewed head has not moved. + run.gh = Mock(side_effect=[{"id": 99}, pr, pr]) + run.check_runs = lambda sha: _checks(("ci", "completed", "success"), sha=sha) run.run() run.dispatcher.deliver.assert_not_called() - assert run.gh.call_count == 1 def test_reviewer_scheduled_scan_keeps_the_label_path_unchanged( @@ -1758,9 +1795,11 @@ class _DedupeDispatcher: def __init__(self): self.seen = {} self.calls = [] + self.prompts = [] def deliver(self, *, subject, delivery, prompt): self.calls.append((subject, delivery)) + self.prompts.append(prompt) if self.seen.get(subject) == delivery: return {"disposition": "deduplicated", "conversation_id": subject} self.seen[subject] = delivery @@ -1838,8 +1877,33 @@ def _eligible_pr(number, sha, request_id, created_at, *, checks=()): } +def _unrequested_pr( + number, + sha, + *, + created_at="2026-01-01T00:00:00Z", + checks=(), + workflows=(), + reviews=(), + author="someone", +): + """An open non-draft PR nobody requested, as the unrequested scan sees it.""" + return { + "number": number, + "head": {"sha": sha}, + "labels": [], + "draft": False, + "created_at": created_at, + "user": {"login": author}, + "requested_reviewers": [], + "_checks": list(checks), + "_workflows": list(workflows), + "_reviews": list(reviews), + } + + def _wire_scan(run, prs, repository_id): - """Point one reviewer at its PRs, request events, and check runs.""" + """Point one reviewer at its PRs, requests, reviews, and runs.""" by_number = {pr["number"]: pr for pr in prs} def gh(method, path, body=None): @@ -1854,8 +1918,11 @@ def gh(method, path, body=None): def gh_pages(path): if path.startswith("/pulls?"): return list(prs) + if path.endswith("/reviews"): + return list(by_number[int(path.split("/")[2])].get("_reviews", [])) if path.endswith("/events"): - return [by_number[int(path.split("/")[2])]["_request"]] + request = by_number[int(path.split("/")[2])].get("_request") + return [request] if request else [] return [] def check_runs(sha): @@ -1868,13 +1935,35 @@ def check_runs(sha): "conclusion": conclusion, "head_sha": sha, } - for name, status, conclusion in pr["_checks"] + for name, status, conclusion in pr.get("_checks", []) + ] + return [] + + def workflow_runs(sha): + for pr in by_number.values(): + if pr["head"]["sha"] == sha: + return [ + { + "name": name, + "status": status, + "conclusion": conclusion, + "head_sha": sha, + "id": run_id, + "check_suite_id": suite_id, + "workflow_id": workflow_id, + "run_started_at": "", + "created_at": "", + } + for name, status, conclusion, run_id, suite_id, workflow_id in pr.get( + "_workflows", [] + ) ] return [] run.gh = gh run.gh_pages = gh_pages run.check_runs = check_runs + run.workflow_runs = workflow_runs def test_one_scan_starts_at_most_the_maximum_and_a_later_scan_reaches_the_rest( @@ -2070,6 +2159,7 @@ def test_the_explicit_request_event_path_is_not_bounded(tmp_path, monkeypatch): if path.endswith("/events") else [] ) + one.workflow_runs = lambda sha: [] one.run() @@ -2108,3 +2198,461 @@ def test_the_rendered_config_rejects_a_misbehaving_max_new_per_run( with pytest.raises(SystemExit): module.workflow.load_config(tmp_path / "github-pr-reviewer") + +# --------------------------------------------------------------------------- # +# Unrequested scheduled scan: an open, non-draft PR with a green current head and +# no all-hands-bot review is reviewed without any request or trigger label. +# --------------------------------------------------------------------------- # + + +def test_unrequested_scan_reviews_a_green_pr_with_no_request_or_label( + tmp_path, monkeypatch +): + """The core new behavior: a green PR nobody requested starts a review.""" + module, (one,) = _scan_reviewers(tmp_path, monkeypatch, ["owner/one"]) + _wire_scan(one, [_unrequested_pr(5, "head-5")], 101) + + _run_scan(module, [one]) + + assert one.dispatcher.seen == {"101:pr:5": "scan:owner/one:5:head-5"} + + +def test_unrequested_scan_blocks_a_failed_zero_job_workflow(tmp_path, monkeypatch): + """A green check-run rollup with a failed zero-job workflow still blocks. + + The workflow failed before creating any check run, so the commit's rollup is + green while the Actions run is `completed`/`failure`. The gate must read the + workflow run and start nothing - and, because nobody requested this PR, it + must do so without posting a managed gate comment. + """ + module, (one,) = _scan_reviewers(tmp_path, monkeypatch, ["owner/one"]) + _wire_scan( + one, + [ + _unrequested_pr( + 5, + "head-5", + checks=(("pr-title", "completed", "success"),), + workflows=(("Tests", "completed", "failure", 3, 3003, 236324519),), + ) + ], + 101, + ) + one.gh = Mock(side_effect=one.gh) + + _run_scan(module, [one]) + + assert one.dispatcher.seen == {} + assert [call for call in one.gh.call_args_list if call.args[0] == "POST"] == [] + + +def test_unrequested_scan_waits_on_a_pending_workflow(tmp_path, monkeypatch): + """A current-head workflow still in progress makes the unrequested head wait. + + The unrequested head starts no conversation and, being unrequested, gets no + managed gate comment either. + """ + module, (one,) = _scan_reviewers(tmp_path, monkeypatch, ["owner/one"]) + _wire_scan( + one, + [ + _unrequested_pr( + 5, + "head-5", + workflows=(("Tests", "in_progress", None, 4, 4004, 236324519),), + ) + ], + 101, + ) + one.gh = Mock(side_effect=one.gh) + + _run_scan(module, [one]) + + assert one.dispatcher.seen == {} + assert [call for call in one.gh.call_args_list if call.args[0] == "POST"] == [] + + +def test_unrequested_scan_does_not_duplicate_across_repeated_scans( + tmp_path, monkeypatch +): + """The stable repository/PR/head key makes a repeat scan a no-op.""" + module, (one,) = _scan_reviewers(tmp_path, monkeypatch, ["owner/one"]) + _wire_scan(one, [_unrequested_pr(5, "head-5")], 101) + + _run_scan(module, [one]) + _run_scan(module, [one]) + + assert one.dispatcher.calls == [ + ("101:pr:5", "scan:owner/one:5:head-5"), + ("101:pr:5", "scan:owner/one:5:head-5"), + ] + assert one.dispatcher.seen == {"101:pr:5": "scan:owner/one:5:head-5"} + + +def test_unrequested_scan_reviews_a_changed_head_again_under_a_new_key( + tmp_path, monkeypatch +): + """A new head is eligible again and reviewed once under its own key.""" + module, (one,) = _scan_reviewers(tmp_path, monkeypatch, ["owner/one"]) + _wire_scan(one, [_unrequested_pr(5, "head-5")], 101) + _run_scan(module, [one]) + + # The PR is pushed to a new head, so the old key no longer matches. + _wire_scan(one, [_unrequested_pr(5, "head-6")], 101) + _run_scan(module, [one]) + + assert one.dispatcher.seen == {"101:pr:5": "scan:owner/one:5:head-6"} + assert one.dispatcher.calls[-1] == ("101:pr:5", "scan:owner/one:5:head-6") + + +def test_unrequested_scan_bound_spans_repositories_with_explicit_priority( + tmp_path, monkeypatch +): + """The cap is global; an explicit request outranks unrequested candidates. + + Repository `owner/one` holds an unrequested PR older than `owner/two`'s + explicit request. The request must win the single slot even though it is + younger, because explicit requests are ordered first. + """ + module, (one, two) = _scan_reviewers( + tmp_path, monkeypatch, ["owner/one", "owner/two"], max_new=1 + ) + _wire_scan( + one, [_unrequested_pr(5, "head-5", created_at="2026-01-01T00:00:00Z")], 101 + ) + _wire_scan(two, [_eligible_pr(2, "head-2", 200, "2026-01-05T00:00:00Z")], 102) + + _run_scan(module, [one, two]) + + assert one.dispatcher.seen == {} + assert two.dispatcher.seen == {"102:pr:2": "200:head-2"} + + +def test_unrequested_scan_reviews_the_oldest_eligible_prs_under_the_cap( + tmp_path, monkeypatch +): + """With no requests, the cap starts the oldest unrequested PRs first.""" + module, (one,) = _scan_reviewers(tmp_path, monkeypatch, ["owner/one"]) + _wire_scan( + one, + [ + _unrequested_pr(9, "head-9", created_at="2026-01-03T00:00:00Z"), + _unrequested_pr(5, "head-5", created_at="2026-01-01T00:00:00Z"), + _unrequested_pr(7, "head-7", created_at="2026-01-02T00:00:00Z"), + ], + 101, + ) + + _run_scan(module, [one]) + + assert one.dispatcher.seen == { + "101:pr:5": "scan:owner/one:5:head-5", + "101:pr:7": "scan:owner/one:7:head-7", + } + + +def test_unrequested_scan_reconciles_a_completed_review_and_hands_off( + tmp_path, monkeypatch +): + """A head that already carries a completed review is not re-dispatched. + + The review is found on the current head, so the scan reconciles it and runs + the existing maintainer handoff instead of starting a second conversation. + """ + module, (one,) = _scan_reviewers(tmp_path, monkeypatch, ["owner/one"]) + _wire_scan( + one, + [ + _unrequested_pr( + 5, + "head-5", + reviews=[ + { + "body": "Review body\n\nāœ… APPROVED", + "commit_id": "head-5", + "submitted_at": "2026-01-02T00:00:00Z", + "user": {"login": "all-hands-bot"}, + } + ], + ) + ], + 101, + ) + one.config["maintainers"] = "neubig" + handoff = Mock(return_value="neubig") + monkeypatch.setattr(module, "request_maintainer_review", handoff) + + _run_scan(module, [one]) + + assert one.dispatcher.seen == {} + handoff.assert_called_once() + + +def test_unrequested_scan_still_gates_a_blocked_pr_past_the_cap( + tmp_path, monkeypatch +): + """An ineligible unrequested PR past the cap consumes no slot and stays silent. + + Two green unrequested PRs fill the cap, and a third blocked one - older than + them - is skipped without consuming a slot, without aborting the scan, and + without a managed gate comment, because nobody requested it. + """ + module, (one,) = _scan_reviewers(tmp_path, monkeypatch, ["owner/one"]) + _wire_scan( + one, + [ + _unrequested_pr( + 5, + "head-5", + created_at="2026-01-01T00:00:00Z", + checks=(("ci", "completed", "failure"),), + ), + _unrequested_pr(6, "head-6", created_at="2026-01-02T00:00:00Z"), + _unrequested_pr(7, "head-7", created_at="2026-01-03T00:00:00Z"), + ], + 101, + ) + one.gh = Mock(side_effect=one.gh) + + _run_scan(module, [one]) + + assert set(one.dispatcher.seen) == {"101:pr:6", "101:pr:7"} + assert [call for call in one.gh.call_args_list if call.args[0] == "POST"] == [] + + +def test_an_explicit_request_still_gets_its_managed_gate_comment( + tmp_path, monkeypatch +): + """The managed comment answers an explicit request, red or pending. + + Suppressing the unrequested comment must not silence the explanation for the + request a caller actually made: an outstanding request whose head is failing + still gets its blocked gate comment. + """ + module, (one,) = _scan_reviewers(tmp_path, monkeypatch, ["owner/one"]) + _wire_scan( + one, + [ + _eligible_pr( + 5, + "head-5", + 500, + "2026-01-01T00:00:00Z", + checks=(("ci", "completed", "failure"),), + ) + ], + 101, + ) + one.gh = Mock(side_effect=one.gh) + + _run_scan(module, [one]) + + assert one.dispatcher.seen == {} + posted = [call for call in one.gh.call_args_list if call.args[0] == "POST"] + assert len(posted) == 1 + assert "" in posted[0].args[2]["body"] + + +def test_unrequested_scan_marks_a_self_authored_pr_for_the_comment_verdict( + tmp_path, monkeypatch +): + """A bot-authored PR is told to use the non-approval verdict path. + + GitHub ignores a self-review request, so the reviewer must publish the clean + review as a `COMMENT` event that keeps the approved verdict instead of being + skipped. + """ + module, (one,) = _scan_reviewers(tmp_path, monkeypatch, ["owner/one"]) + _wire_scan(one, [_unrequested_pr(5, "head-5", author="all-hands-bot")], 101) + + _run_scan(module, [one]) + + assert one.dispatcher.seen == {"101:pr:5": "scan:owner/one:5:head-5"} + prompt = one.dispatcher.prompts[-1] + assert "authored by the configured reviewer account" in prompt + assert "keep the approved verdict" in prompt + + +# --------------------------------------------------------------------------- # +# Bounded rotating scan window: each scheduled scan examines a bounded slice of +# the unrequested backlog, remembers its position in the Automation KV store, and +# resumes there on the next scan. Explicit requests are never subject to it. +# --------------------------------------------------------------------------- # + + +def _kv_store(module, monkeypatch): + """Back the scan cursor with an in-memory stand-in for the Automation KV store. + + The real store is an HTTP service; this reproduces the contract the cursor + relies on - a per-key `GET` that misses with None and a `PUT` that replaces + the value - so the shipped `ScanCursor` persists and reloads through it. + """ + values = {} + monkeypatch.setattr(module.workflow, "_kv_available", lambda: True) + monkeypatch.setattr( + module.workflow, "_kv_get", lambda key: values.get(key) + ) + + def put(key, value): + values[key] = value + + monkeypatch.setattr(module.workflow, "_kv_set", put) + return values + + +def _many_unrequested(count, *, checks=()): + """`count` open, non-draft PRs nobody requested, numbered 1..count.""" + return [ + _unrequested_pr( + number, + f"head-{number}", + created_at=f"2026-01-{number:02d}T00:00:00Z", + checks=checks, + ) + for number in range(1, count + 1) + ] + + +def _examined(run): + """The PR numbers a scan actually dispatched, in call order.""" + return [int(subject.split(":")[-1]) for subject, _ in run.dispatcher.calls] + + +def test_scan_examines_a_bounded_rotating_window_of_unrequested_prs( + tmp_path, monkeypatch +): + """Each scan covers the next slice, so the whole backlog is reached in turn.""" + module, (one,) = _scan_reviewers(tmp_path, monkeypatch, ["owner/one"], max_new=5) + monkeypatch.setattr(module, "SCAN_WINDOW", 2) + _kv_store(module, monkeypatch) + _wire_scan(one, _many_unrequested(5), 101) + + _run_scan(module, [one]) + assert _examined(one) == [1, 2] + + _run_scan(module, [one]) + assert _examined(one)[-2:] == [3, 4] + + _run_scan(module, [one]) + assert _examined(one)[-1:] == [5] + + # Past the end the window wraps, so the backlog keeps rotating. + _run_scan(module, [one]) + assert _examined(one)[-2:] == [1, 2] + + +def test_the_scan_position_is_persisted_per_repository_in_the_kv_store( + tmp_path, monkeypatch +): + """The cursor is written under a per-repository key, so a later run resumes. + + A cron run is a fresh process, so the only way the window survives is the + Automation KV store; this pins the key and the value the next scan reads. + """ + module, (one,) = _scan_reviewers(tmp_path, monkeypatch, ["owner/one"], max_new=5) + monkeypatch.setattr(module, "SCAN_WINDOW", 2) + values = _kv_store(module, monkeypatch) + _wire_scan(one, _many_unrequested(5), 101) + + _run_scan(module, [one]) + + assert values == {"review-scan:owner__one": {"cursor": 2}} + + +def test_explicit_requests_are_examined_regardless_of_the_rotation_window( + tmp_path, monkeypatch +): + """An explicit request is never skipped because the window sits elsewhere. + + The cursor points at the last unrequested PR, but the explicit request on + PR #1 is still examined and dispatched, because a caller asked for it. + """ + module, (one,) = _scan_reviewers(tmp_path, monkeypatch, ["owner/one"], max_new=5) + monkeypatch.setattr(module, "SCAN_WINDOW", 1) + values = _kv_store(module, monkeypatch) + # Only PR #1 holds a request; the rest are unrequested and the window sits on + # the last of them, so the request must still be examined. + _wire_scan( + one, + [_eligible_pr(1, "head-1", 100, "2026-01-01T00:00:00Z")] + + _many_unrequested(4)[1:], + 101, + ) + values["review-scan:owner__one"] = {"cursor": 2} + + _run_scan(module, [one]) + + assert one.dispatcher.seen == { + "101:pr:1": "100:head-1", + "101:pr:4": "scan:owner/one:4:head-4", + } + + +def test_unrequested_red_and_pending_prs_post_no_gate_comments( + tmp_path, monkeypatch +): + """A scan over a large unrequested backlog posts no managed gate comments. + + This is the storm the canary exposed: every red or pending head used to get a + comment. Unrequested heads now stay silent; only an explicit request gets the + managed explanation. + """ + module, (one,) = _scan_reviewers(tmp_path, monkeypatch, ["owner/one"], max_new=5) + monkeypatch.setattr(module, "SCAN_WINDOW", 20) + _wire_scan( + one, + [ + _unrequested_pr( + 5, "head-5", checks=(("ci", "completed", "failure"),) + ), + _unrequested_pr( + 6, "head-6", checks=(("slow-e2e", "in_progress", None),) + ), + _unrequested_pr( + 7, "head-7", checks=(("ci", "completed", "success"),) + ), + ], + 101, + ) + one.gh = Mock(side_effect=one.gh) + + _run_scan(module, [one]) + + assert [call for call in one.gh.call_args_list if call.args[0] == "POST"] == [] + # The green unrequested head still reaches the bounded dispatch queue. + assert one.dispatcher.seen == {"101:pr:7": "scan:owner/one:7:head-7"} + + +def test_scan_reads_a_bounded_number_of_pull_requests_per_repository( + tmp_path, monkeypatch +): + """A scan reads one list page plus at most one head per examined candidate. + + A full scan would issue one full pull read per open PR - the API cost that + made the canary time out. With a window of 10 over 40 PRs, only the 10 in the + window are read. + """ + module, (one,) = _scan_reviewers(tmp_path, monkeypatch, ["owner/one"], max_new=5) + monkeypatch.setattr(module, "SCAN_WINDOW", 10) + _kv_store(module, monkeypatch) + _wire_scan(one, _many_unrequested(40), 101) + + list_reads = [] + original_pages = one.gh_pages + + def counting_pages(path): + if path.startswith("/pulls?"): + list_reads.append(path) + return original_pages(path) + + one.gh_pages = counting_pages + one.gh = Mock(side_effect=one.gh) + + _run_scan(module, [one]) + + full_reads = [ + call for call in one.gh.call_args_list + if call.args[0] == "GET" and str(call.args[1]).startswith("/pulls/") + ] + assert len(list_reads) == 1 + assert len(full_reads) == 10