From cefe05e3b30ebc2086d1b605913d767963f31241 Mon Sep 17 00:00:00 2001 From: luckyPipewrench Date: Sun, 18 Oct 2026 15:06:56 -0400 Subject: [PATCH 1/2] ci: adopt shared pull request review Signed-off-by: luckyPipewrench --- .github/agent-review.toml | 7 + .github/workflows/agent-review.yaml | 33 ++ .github/workflows/pr-review.yaml | 35 -- .github/workflows/review-caller-contract.yaml | 109 ++--- scripts/check_agent_review_workflow.py | 78 ++++ scripts/pr-review.py | 361 --------------- scripts/test_pr_review_workflow.py | 419 ------------------ tests/test_pr_review_routing.py | 121 ----- 8 files changed, 161 insertions(+), 1002 deletions(-) create mode 100644 .github/agent-review.toml create mode 100644 .github/workflows/agent-review.yaml delete mode 100644 .github/workflows/pr-review.yaml create mode 100644 scripts/check_agent_review_workflow.py delete mode 100644 scripts/pr-review.py delete mode 100644 scripts/test_pr_review_workflow.py delete mode 100644 tests/test_pr_review_routing.py diff --git a/.github/agent-review.toml b/.github/agent-review.toml new file mode 100644 index 0000000..066290c --- /dev/null +++ b/.github/agent-review.toml @@ -0,0 +1,7 @@ +[review] +instructions = """ +Review receipt verification for strict canonicalization, signature and trust validation, chain integrity, malformed input handling, and compatibility with the published receipt specification. Never accept unverifiable evidence. +""" + +[ignore] +glob = ["**/go.sum"] diff --git a/.github/workflows/agent-review.yaml b/.github/workflows/agent-review.yaml new file mode 100644 index 0000000..a0b8136 --- /dev/null +++ b/.github/workflows/agent-review.yaml @@ -0,0 +1,33 @@ +name: PR-Agent review + +on: + # Default-branch code only; the identity gate and fork admission are tested. + issue_comment: # zizmor: ignore[dangerous-triggers] + types: [created] + +permissions: {} + +jobs: + review: + if: >- + github.event_name == 'issue_comment' && + github.event.action == 'created' && + github.actor == 'luckyPipewrench' && + github.actor_id == '142104046' && + github.run_attempt == '1' && + github.triggering_actor == 'luckyPipewrench' && + github.event.comment.user.login == 'luckyPipewrench' && + github.event.comment.user.id == 142104046 && + github.event.comment.user.type == 'User' && + github.event.comment.author_association == 'MEMBER' && + github.event.issue.pull_request && + contains(fromJSON('["/review","/review deep"]'), github.event.comment.body) + permissions: + contents: read + issues: write + pull-requests: write + uses: pipelab-org/agent-review/.github/workflows/agent-review-reusable.yaml@856e474c1697f3897ccaa156f083e0b9504196e0 # main + secrets: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + ANTHROPIC_API_KEY_2: ${{ secrets.ANTHROPIC_API_KEY_2 }} + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} diff --git a/.github/workflows/pr-review.yaml b/.github/workflows/pr-review.yaml deleted file mode 100644 index 41b2a14..0000000 --- a/.github/workflows/pr-review.yaml +++ /dev/null @@ -1,35 +0,0 @@ -name: AI PR Review - -on: - issue_comment: - types: [created] - -permissions: - contents: read - issues: write - pull-requests: write - -jobs: - review: - if: >- - github.actor == 'luckyPipewrench' && - github.actor_id == '142104046' && - github.run_attempt == '1' && - github.triggering_actor == 'luckyPipewrench' && - github.event.comment.user.login == 'luckyPipewrench' && - github.event.comment.user.id == 142104046 && - github.event.comment.author_association == 'MEMBER' && - github.event.issue.pull_request && - (github.event.comment.body == '/review' || - github.event.comment.body == '/review deep') - # Keep both pins equal until the nested source identity is verified live. - uses: pipelab-org/pipelock/.github/workflows/pr-review-reusable.yaml@4edcf46d02011c2aa1e305aff69d4d7100aae0ca - with: - reviewer_sha: 4edcf46d02011c2aa1e305aff69d4d7100aae0ca - pr_number: ${{ github.event.issue.number }} - review_mode: >- - ${{ github.event.comment.body == '/review deep' && 'deep' || - 'default' }} - secrets: - review_token: ${{ secrets.GITHUB_TOKEN }} - openai_api_key: ${{ secrets.OPENAI_API_KEY }} diff --git a/.github/workflows/review-caller-contract.yaml b/.github/workflows/review-caller-contract.yaml index b1b60f8..e2c3478 100644 --- a/.github/workflows/review-caller-contract.yaml +++ b/.github/workflows/review-caller-contract.yaml @@ -4,107 +4,84 @@ on: pull_request: types: [opened, synchronize, reopened] paths: - - '.github/workflows/pr-review.yaml' + - '.github/workflows/agent-review.yaml' + - '.github/agent-review.toml' - '.github/workflows/review-caller-contract.yaml' - - 'scripts/test_pr_review_workflow.py' + - 'scripts/check_agent_review_workflow.py' pull_request_target: types: [opened, synchronize, reopened] paths: - - '.github/workflows/pr-review.yaml' + - '.github/workflows/agent-review.yaml' + - '.github/agent-review.toml' - '.github/workflows/review-caller-contract.yaml' - - 'scripts/test_pr_review_workflow.py' + - 'scripts/check_agent_review_workflow.py' push: branches: [main] paths: - - '.github/workflows/pr-review.yaml' + - '.github/workflows/agent-review.yaml' + - '.github/agent-review.toml' - '.github/workflows/review-caller-contract.yaml' - - 'scripts/test_pr_review_workflow.py' + - 'scripts/check_agent_review_workflow.py' permissions: contents: read jobs: - bootstrap: - if: github.event_name == 'pull_request' - runs-on: ubuntu-latest - timeout-minutes: 5 - steps: - - name: Check out proposed contract for unprivileged installation tests - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - with: - persist-credentials: false - - name: Test proposed caller and contract without provider secrets - run: python3 -I scripts/test_pr_review_workflow.py contract: - if: github.event_name != 'pull_request' runs-on: ubuntu-latest timeout-minutes: 5 steps: - - name: Check out trusted default-branch caller validator + - name: Check out the caller validator uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 with: - repository: luckyPipewrench/pipelock-verify-python - ref: ${{ github.sha }} + # Bootstrap tests run proposed code without secrets. Target validation + # always runs the default-branch copy, independently of proposed code. + repository: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name || github.repository }} + ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }} + allow-unsafe-pr-checkout: true path: trusted-validator persist-credentials: false - sparse-checkout: scripts/test_pr_review_workflow.py + sparse-checkout: scripts/check_agent_review_workflow.py sparse-checkout-cone-mode: false - - name: Check out proposed caller as data + - name: Check out proposed policy as data uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 with: repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }} ref: ${{ github.event.pull_request.head.sha || github.sha }} - # Proposed workflows are bounded data; no proposed code is executed. allow-unsafe-pr-checkout: true path: proposed persist-credentials: false - filter: blob:none sparse-checkout: | - .github/workflows/pr-review.yaml + .github/workflows/agent-review.yaml + .github/agent-review.toml .github/workflows/review-caller-contract.yaml sparse-checkout-cone-mode: false - - name: Validate proposed caller with trusted test code - id: validate - run: | - python3 -I - <<'PY' - import importlib.util - import os - import pathlib - import unittest - - validator = pathlib.Path("trusted-validator/scripts/test_pr_review_workflow.py") - spec = importlib.util.spec_from_file_location("trusted_caller", validator) - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - module.WORKFLOW = pathlib.Path("proposed/.github/workflows/pr-review.yaml") - reviewer_sha = module.validate_caller(module.WORKFLOW) - module.validate_contract( - pathlib.Path("proposed/.github/workflows/review-caller-contract.yaml") - ) - suite = unittest.defaultTestLoader.loadTestsFromModule(module) - result = unittest.TextTestRunner(verbosity=2).run(suite) - if not result.wasSuccessful(): - raise SystemExit(1) - with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as output: - output.write(f"reviewer_sha={reviewer_sha}\n") - PY - - name: Check out immutable reviewer and its security tests + - name: Check approved source and checking workflow before loading shared code + id: policy + run: python3 -I trusted-validator/scripts/check_agent_review_workflow.py --root proposed --output "$GITHUB_OUTPUT" + - name: Check out the approved immutable shared validator uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 with: - repository: luckyPipewrench/pipelock - ref: ${{ steps.validate.outputs.reviewer_sha }} + repository: pipelab-org/agent-review + ref: ${{ steps.policy.outputs.reviewer_sha }} path: trusted-reviewer persist-credentials: false - - name: Install pinned reviewer test dependencies - working-directory: trusted-reviewer + sparse-checkout: scripts/agent_review_contract.py + sparse-checkout-cone-mode: false + - name: Validate caller and profile with approved shared code + env: + REVIEW_SOURCE_SHA: ${{ steps.policy.outputs.reviewer_sha }} run: | - python3 -m venv .review-test-venv - .review-test-venv/bin/python -m pip install --require-hashes \ - -r .github/actions/pr-review/requirements.txt \ - -r .github/requirements-pr-review-test.txt - - name: Verify shared reviewer workflow protections - working-directory: trusted-reviewer - run: >- - .review-test-venv/bin/python -m unittest - scripts.pr_review_test.WorkflowPackagingTest - scripts.pr_review_test.FailureDirectionTest.test_review_job_timeout_exceeds_the_wall_clock_with_finalization_margin + python3 -I - <<'PYCODE' + import os + import pathlib + import runpy + + contract = runpy.run_path("trusted-reviewer/scripts/agent_review_contract.py") + root = pathlib.Path("proposed") + contract["validate_caller"]( + root / ".github/workflows/agent-review.yaml", os.environ["REVIEW_SOURCE_SHA"], + ) + contract["load_profile"](root / ".github/agent-review.toml") + print("Review caller and profile contract passed") + PYCODE diff --git a/scripts/check_agent_review_workflow.py b/scripts/check_agent_review_workflow.py new file mode 100644 index 0000000..37fb984 --- /dev/null +++ b/scripts/check_agent_review_workflow.py @@ -0,0 +1,78 @@ +"""Validate proposed caller policy using the trusted default-branch definition.""" + +import argparse +import os +import pathlib +import re +import stat + +# Existing caller-policy contract: approve a source/shape on main, migrate, +# then remove the predecessor approval. Never execute proposed target code. +MAX_WORKFLOW_BYTES = 65536 +APPROVED_REVIEWER_SHAS = ("856e474c1697f3897ccaa156f083e0b9504196e0",) +EXPECTED_CONTRACTS = ( + "name: Review caller contract\n\non:\n pull_request:\n types: [opened, synchronize, reopened]\n paths:\n - '.github/workflows/agent-review.yaml'\n - '.github/agent-review.toml'\n - '.github/workflows/review-caller-contract.yaml'\n - 'scripts/check_agent_review_workflow.py'\n pull_request_target:\n types: [opened, synchronize, reopened]\n paths:\n - '.github/workflows/agent-review.yaml'\n - '.github/agent-review.toml'\n - '.github/workflows/review-caller-contract.yaml'\n - 'scripts/check_agent_review_workflow.py'\n push:\n branches: [main]\n paths:\n - '.github/workflows/agent-review.yaml'\n - '.github/agent-review.toml'\n - '.github/workflows/review-caller-contract.yaml'\n - 'scripts/check_agent_review_workflow.py'\n\npermissions:\n contents: read\n\njobs:\n contract:\n runs-on: ubuntu-latest\n timeout-minutes: 5\n steps:\n - name: Check out the caller validator\n uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7\n with:\n # Bootstrap tests run proposed code without secrets. Target validation\n # always runs the default-branch copy, independently of proposed code.\n repository: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name || github.repository }}\n ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }}\n allow-unsafe-pr-checkout: true\n path: trusted-validator\n persist-credentials: false\n sparse-checkout: scripts/check_agent_review_workflow.py\n sparse-checkout-cone-mode: false\n - name: Check out proposed policy as data\n uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7\n with:\n repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }}\n ref: ${{ github.event.pull_request.head.sha || github.sha }}\n allow-unsafe-pr-checkout: true\n path: proposed\n persist-credentials: false\n sparse-checkout: |\n .github/workflows/agent-review.yaml\n .github/agent-review.toml\n .github/workflows/review-caller-contract.yaml\n sparse-checkout-cone-mode: false\n - name: Check approved source and checking workflow before loading shared code\n id: policy\n run: python3 -I trusted-validator/scripts/check_agent_review_workflow.py --root proposed --output \"$GITHUB_OUTPUT\"\n - name: Check out the approved immutable shared validator\n uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7\n with:\n repository: pipelab-org/agent-review\n ref: ${{ steps.policy.outputs.reviewer_sha }}\n path: trusted-reviewer\n persist-credentials: false\n sparse-checkout: scripts/agent_review_contract.py\n sparse-checkout-cone-mode: false\n - name: Validate caller and profile with approved shared code\n env:\n REVIEW_SOURCE_SHA: ${{ steps.policy.outputs.reviewer_sha }}\n run: |\n python3 -I - <<'PYCODE'\n import os\n import pathlib\n import runpy\n\n contract = runpy.run_path(\"trusted-reviewer/scripts/agent_review_contract.py\")\n root = pathlib.Path(\"proposed\")\n contract[\"validate_caller\"](\n root / \".github/workflows/agent-review.yaml\", os.environ[\"REVIEW_SOURCE_SHA\"],\n )\n contract[\"load_profile\"](root / \".github/agent-review.toml\")\n print(\"Review caller and profile contract passed\")\n PYCODE\n", +) + + +def read_workflow(path): + """Read bounded UTF-8 data without following any path-component symlink.""" + path = pathlib.Path(path) + if ".." in path.parts: + raise ValueError("workflow path must not traverse parents") + parts = path.parts[1:] if path.is_absolute() else path.parts + directory_flags = os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW + directory = os.open(path.anchor if path.is_absolute() else ".", directory_flags) + try: + for part in parts[:-1]: + child = os.open(part, directory_flags, dir_fd=directory) + os.close(directory) + directory = child + fd = os.open(parts[-1], os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK, dir_fd=directory) + try: + info = os.fstat(fd) + if not stat.S_ISREG(info.st_mode) or info.st_size > MAX_WORKFLOW_BYTES: + raise ValueError("workflow must be a bounded regular file") + with os.fdopen(fd, "rb", closefd=False) as stream: + data = stream.read(MAX_WORKFLOW_BYTES + 1) + if len(data) > MAX_WORKFLOW_BYTES: + raise ValueError("workflow exceeds the data limit") + return data.decode("utf-8") + finally: + os.close(fd) + finally: + os.close(directory) + + +def validate_policy(root): + root = pathlib.Path(root) + caller = read_workflow(root / ".github/workflows/agent-review.yaml") + pins = re.findall( + r"^ uses: pipelab-org/agent-review/\.github/workflows/agent-review-reusable\.yaml@([0-9a-f]{40}) # main$", + caller, + re.MULTILINE, + ) + if len(pins) != 1 or pins[0] not in APPROVED_REVIEWER_SHAS: + raise ValueError("approve the immutable reviewer source on main before migrating") + contract = read_workflow(root / ".github/workflows/review-caller-contract.yaml") + if contract not in EXPECTED_CONTRACTS: + raise ValueError("proposed checking workflow differs from its trusted definition") + return pins[0] + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--root", type=pathlib.Path, required=True) + parser.add_argument("--output", type=pathlib.Path, required=True) + args = parser.parse_args() + try: + reviewer_sha = validate_policy(args.root) + except (OSError, ValueError): + parser.exit(1, "Invalid proposed reviewer source or checking workflow\n") + with args.output.open("a", encoding="utf-8") as output: + output.write(f"reviewer_sha={reviewer_sha}\n") + print("Trusted reviewer-source and checking-workflow contract passed") + + +if __name__ == "__main__": + main() diff --git a/scripts/pr-review.py b/scripts/pr-review.py deleted file mode 100644 index 55fba73..0000000 --- a/scripts/pr-review.py +++ /dev/null @@ -1,361 +0,0 @@ -#!/usr/bin/env python3 -# Copyright 2026 Pipelock contributors -# SPDX-License-Identifier: Apache-2.0 - -"""AI-powered PR review for the pipelock-verify Python package. - -Triggered by /review comments on PRs. Supports multiple review modes: - /review - Security and correctness review (default model) - /review deep - Deeper review (higher-capacity model) - /review tests - Test coverage and boundary analysis - /review docs - Documentation accuracy check - -Requires environment variables: - GITHUB_TOKEN - GitHub token (provided by Actions) - REPO - owner/repo - PR_NUMBER - PR number - REVIEW_MODE - "default", "deep", "tests", or "docs" - -LLM configuration (one of): - LITELLM_BASE_URL + LITELLM_API_KEY - LiteLLM proxy - OPENAI_API_KEY - Direct OpenAI API - -Model selection: - PR_REVIEW_MODEL_FAST - Optional model override for default/tests/docs - (default: gpt-5.6-luna) - PR_REVIEW_MODEL_DEEP - Optional model override for /review deep - (default: gpt-5.6-terra) - -The PR_REVIEW_MODEL_FAST env var keeps its name for backwards compatibility -with existing repository-variable overrides; the user-facing /review fast -alias was dropped 2026-04-23 because the default mode is sufficient. -""" - -import json -import os -import sys - -import requests - -# --- Constants --- - -MAX_DIFF_CHARS = 100_000 -# These are the sole default model definitions. GitHub Actions may supply -# optional repository-variable overrides, but an unset or empty override must -# fall back here so workflow configuration cannot drift from local behavior. -DEFAULT_MODEL_FAST = "gpt-5.6-luna" -DEFAULT_MODEL_DEEP = "gpt-5.6-terra" -DEFAULT_TEMPERATURE = 0.2 -DEFAULT_MAX_COMPLETION_TOKENS = 8192 -# max_completion_tokens is shared by reasoning and visible output. At xhigh -# effort, 25000 was consumed by reasoning alone and produced an empty review. -DEEP_MAX_COMPLETION_TOKENS = 64000 -DEFAULT_LLM_TIMEOUT_SECONDS = 120 -DEEP_LLM_TIMEOUT_SECONDS = 300 -FAST_REASONING_EFFORT = "low" -DEEP_REASONING_EFFORT = "xhigh" - - -class LLMReviewError(RuntimeError): - """Raised when the LLM call completed but did not produce a usable review.""" - - -PROMPT_SECURITY = """You are reviewing a pull request for pipelock-verify, the Python verifier for Pipelock action receipts: Ed25519-signed, hash-chained records that an independent party uses to check what an AI agent actually did. - -This library's defining property is BYTE-EXACT PARITY with the Go emitter. The signing input is the SHA-256 of a canonical JSON projection of the action record, reproduced to match Go's encoding/json byte for byte. Any divergence in field set, field order, omitempty semantics, escaping, or number spelling changes the hash and breaks verification. There is no slack. - -Two failure directions both matter, and they are not symmetric: -- FAIL-OPEN: accepting something Go rejects. A receipt that verifies here but not in Go means this tool blesses evidence the emitter considers invalid. Treat as high severity. -- FAIL-CLOSED: rejecting something Go accepts. This breaks honest operators and makes the tool untrustworthy in the other direction. Still a real defect, usually medium. - -Flag: -- any change to the canonical projection: added, removed, reordered, or re-typed fields, and whether omitempty matches Go -- signature or chain-hash construction changes, including what is and is not covered by each -- validation that accepts a shape Go's strict Unmarshal would reject, or rejects one it accepts -- parser-differential surfaces: duplicate keys, trailing tokens, unicode normalization, HTML escaping, integer bounds, float spelling, empty versus absent -- anywhere parsed-and-re-serialized data is used where the producer's original bytes are what actually get hashed -- guards that cannot be reached, or that no test would catch the removal of -- error paths that surface as tracebacks rather than a clean invalid result on attacker-supplied input - -Do not waste time on style nits. -For each finding, include: -1. severity: high, medium, or low -2. file and function -3. why it matters, and which direction it fails -4. a concrete fix - -If there are no material issues, say exactly: No material security or correctness issues found in this diff.""" - -PROMPT_TESTS = """You are reviewing the TEST COVERAGE of a pull request for pipelock-verify, a Python verifier for Ed25519-signed, hash-chained Pipelock receipts. - -The single most important question: WOULD THIS TEST FAIL IF THE THING IT GUARDS WERE DELETED? A test that passes whether or not the guard exists is worse than no test, because it reports safety it does not provide. Say so explicitly wherever you suspect it. - -For each code change, check: - -1. **Vacuity**: for every new guard or validation, is there a test that fails when that guard is removed? If the guard is only reachable through one entry point, does a test actually use that entry point? -2. **Parity, not just validity**: chain tests that assert only "valid" do not catch a divergence in what the chain commits to. Is the root hash or canonical output pinned to an independently computed value? -3. **Real emitter output**: is parity proven against a captured real chain, or only against hand-written data that can drift alongside the code it checks? -4. **Both directions**: for a new rejection, is there a positive test proving legitimate input still passes? Over-tightening is a real defect. -5. **Boundaries**: integer limits including int64 and uint64 edges, empty versus absent, null, empty object and array, non-sorted keys, unicode and HTML-escapable characters, very large values. -6. **Error paths**: are new error returns exercised? - -For each gap: -1. severity: high (untested guard or unproven parity), medium (untested boundary), low (nice-to-have) -2. file and line -3. the specific missing case -4. a concrete test to add, including input and expected result - -If coverage is adequate, say exactly: Test coverage is adequate for this diff.""" - -PROMPT_DOCS = """You are reviewing a pull request for pipelock-verify for DOCUMENTATION ACCURACY. - -Check every claim in the diff against the code: - -1. **Contract claims**: statements about which fields are signed, what the chain hash covers, or how canonicalization works must match the actual implementation and the Go source it mirrors. -2. **Source-of-truth pointers**: comments naming a Go file or struct as the contract must name the file that actually defines it. A pointer to the wrong file is how drift goes unnoticed. -3. **Capability claims**: if docs say something is verified, enforced, or guaranteed, confirm the code does it. Flag anything that overstates what an offline verifier can prove. -4. **Stated limitations**: known gaps should be documented plainly rather than implied or omitted. -5. **Examples**: sample code and CLI invocations must actually run as written. -6. **Stale references**: removed functions, renamed parameters, old behavior. - -For each issue: -1. severity: high (wrong claim about what is verified), medium (stale or misleading), low (unclear) -2. file and line -3. what it says versus what the code shows -4. the correct statement - -If documentation is accurate, say exactly: Documentation accurately reflects the codebase in this diff.""" - - -def get_pr_diff(repo: str, pr_number: str, token: str) -> str: - """Fetch the PR diff from GitHub.""" - url = f"https://api.github.com/repos/{repo}/pulls/{pr_number}" - headers = { - "Authorization": f"Bearer {token}", - "Accept": "application/vnd.github.v3.diff", - } - resp = requests.get(url, headers=headers, timeout=30) - resp.raise_for_status() - return resp.text - - -def truncate_diff(diff: str, max_chars: int = MAX_DIFF_CHARS) -> str: - """Truncate diff to stay within token limits.""" - if len(diff) <= max_chars: - return diff - truncated = diff[:max_chars] - return truncated + f"\n\n... (diff truncated at {max_chars} chars, {len(diff)} total)" - - -def model_supports_custom_temperature(model: str) -> bool: - """Return whether chat completions should send a non-default temperature.""" - normalized = model.strip().lower() - model_name = normalized.rsplit("/", 1)[-1] - return not model_name.startswith(("gpt-5", "o1", "o3", "o4")) - - -def model_supports_reasoning_effort(model: str) -> bool: - """Return whether chat completions should pin reasoning effort.""" - normalized = model.strip().lower() - model_name = normalized.rsplit("/", 1)[-1] - return model_name.startswith(("gpt-5", "o1", "o3", "o4")) - - -def build_llm_payload( - model: str, - system_prompt: str, - diff: str, - *, - max_completion_tokens: int = DEFAULT_MAX_COMPLETION_TOKENS, - reasoning_effort: str = FAST_REASONING_EFFORT, -) -> dict: - """Build the chat completions payload for the selected review model.""" - payload = { - "model": model, - "messages": [ - {"role": "system", "content": system_prompt}, - { - "role": "user", - "content": f"Review this pull request diff:\n\n```diff\n{diff}\n```", - }, - ], - "max_completion_tokens": max_completion_tokens, - } - if model_supports_custom_temperature(model): - payload["temperature"] = DEFAULT_TEMPERATURE - if model_supports_reasoning_effort(model): - payload["reasoning_effort"] = reasoning_effort - return payload - - -def model_for_mode(mode: str) -> str: - """Return the configured model for a review mode, with Python defaults.""" - if mode == "deep": - return os.environ.get("PR_REVIEW_MODEL_DEEP") or DEFAULT_MODEL_DEEP - return os.environ.get("PR_REVIEW_MODEL_FAST") or DEFAULT_MODEL_FAST - - -def summarize_usage(data: dict) -> str: - """Return compact token usage details for operator-visible errors.""" - usage = data.get("usage") - if not isinstance(usage, dict): - return "usage unavailable" - details = usage.get("completion_tokens_details") or {} - parts = [ - f"prompt={usage.get('prompt_tokens', 'unknown')}", - f"completion={usage.get('completion_tokens', 'unknown')}", - f"total={usage.get('total_tokens', 'unknown')}", - ] - if isinstance(details, dict) and "reasoning_tokens" in details: - parts.append(f"reasoning={details['reasoning_tokens']}") - return ", ".join(parts) - - -def extract_chat_content(data: dict) -> str: - """Extract visible text from a chat-completions response.""" - choices = data.get("choices", []) - if not isinstance(choices, list) or not choices: - raise LLMReviewError("LLM returned no choices.") - - choice = choices[0] if isinstance(choices[0], dict) else {} - message = choice.get("message") if isinstance(choice.get("message"), dict) else {} - content = message.get("content", "") - if isinstance(content, list): - content = "".join(part.get("text", "") for part in content if isinstance(part, dict)) - if isinstance(content, str) and content.strip(): - if choice.get("finish_reason") == "length": - content += ( - "\n\n> **Warning:** Review output was truncated by the model " - f"completion limit ({summarize_usage(data)}). Treat this as an " - "incomplete review and rerun with a narrower diff if needed." - ) - return content - - finish_reason = choice.get("finish_reason", "unknown") - raise LLMReviewError( - f"LLM returned empty content (finish_reason={finish_reason}; {summarize_usage(data)})." - ) - - -def call_llm(diff: str, mode: str, system_prompt: str) -> str: - """Send the diff to the LLM and return the review.""" - litellm_url = os.environ.get("LITELLM_BASE_URL", "") - litellm_key = os.environ.get("LITELLM_API_KEY", "") - openai_key = os.environ.get("OPENAI_API_KEY", "") - - model = model_for_mode(mode) - - if litellm_url and litellm_key: - api_url = litellm_url.rstrip("/") + "/chat/completions" - bearer = litellm_key - elif openai_key: - api_url = "https://api.openai.com/v1/chat/completions" - bearer = openai_key - else: - raise LLMReviewError( - "No LLM API configured. Set LITELLM_BASE_URL + LITELLM_API_KEY " - "or OPENAI_API_KEY in repo secrets." - ) - - headers = { - "Authorization": f"Bearer {bearer}", - "Content-Type": "application/json", - } - is_deep = mode == "deep" - max_completion_tokens = DEEP_MAX_COMPLETION_TOKENS if is_deep else DEFAULT_MAX_COMPLETION_TOKENS - reasoning_effort = DEEP_REASONING_EFFORT if is_deep else FAST_REASONING_EFFORT - payload = build_llm_payload( - model, - system_prompt, - diff, - max_completion_tokens=max_completion_tokens, - reasoning_effort=reasoning_effort, - ) - - timeout = DEEP_LLM_TIMEOUT_SECONDS if is_deep else DEFAULT_LLM_TIMEOUT_SECONDS - resp = requests.post(api_url, headers=headers, json=payload, timeout=timeout) - if resp.status_code != 200: - raise LLMReviewError(f"LLM API returned {resp.status_code} for model `{model}`.") - try: - data = resp.json() - except (json.JSONDecodeError, ValueError) as error: - raise LLMReviewError("LLM returned invalid JSON.") from error - if not isinstance(data, dict): - raise LLMReviewError("LLM returned a non-object JSON response.") - return extract_chat_content(data) - - -def post_comment(repo: str, pr_number: str, token: str, body: str) -> None: - """Post a comment on the PR.""" - url = f"https://api.github.com/repos/{repo}/issues/{pr_number}/comments" - headers = { - "Authorization": f"Bearer {token}", - "Accept": "application/vnd.github.v3+json", - } - resp = requests.post(url, headers=headers, json={"body": body}, timeout=30) - resp.raise_for_status() - - -def main() -> None: - gh_token = os.environ.get("GITHUB_TOKEN", "") - repo = os.environ.get("REPO", "") - pr_number = os.environ.get("PR_NUMBER", "") - mode = os.environ.get("REVIEW_MODE", "default") - - if not all([gh_token, repo, pr_number]): - print("Missing required environment variables", file=sys.stderr) - sys.exit(1) - - print(f"Reviewing PR #{pr_number} in {repo} (mode: {mode})") - - # All other modes need the diff. - try: - diff = get_pr_diff(repo, pr_number, gh_token) - except requests.RequestException as e: - post_comment( - repo, pr_number, gh_token, f"**AI Review Error:** Failed to fetch PR diff: {e}" - ) - sys.exit(1) - - if not diff.strip(): - post_comment(repo, pr_number, gh_token, "**AI Review:** No diff found for this PR.") - return - - diff = truncate_diff(diff) - print(f"Diff size: {len(diff)} chars") - - # Select prompt. - prompts = { - "default": PROMPT_SECURITY, - "deep": PROMPT_SECURITY, - "tests": PROMPT_TESTS, - "docs": PROMPT_DOCS, - } - system_prompt = prompts.get(mode, PROMPT_SECURITY) - - try: - review = call_llm(diff, mode, system_prompt) - except (requests.RequestException, LLMReviewError) as e: - post_comment(repo, pr_number, gh_token, f"**AI Review Error:** {e}") - sys.exit(1) - - model_name = model_for_mode(mode) - - mode_labels = { - "default": "security", - "deep": "security deep", - "tests": "test coverage", - "docs": "docs accuracy", - } - label = mode_labels.get(mode, mode) - # The default mode is invoked as bare `/review`, not `/review default`, - # so the header omits the suffix in that case to match what the user - # actually typed. - cmd = "/review" if mode == "default" else f"/review {mode}" - header = f"## AI Review: {label} (`{cmd}`)\n\n**Model:** `{model_name}`\n\n---\n\n" - post_comment(repo, pr_number, gh_token, header + review) - print("Review posted.") - - -if __name__ == "__main__": - main() diff --git a/scripts/test_pr_review_workflow.py b/scripts/test_pr_review_workflow.py deleted file mode 100644 index 80fb2f2..0000000 --- a/scripts/test_pr_review_workflow.py +++ /dev/null @@ -1,419 +0,0 @@ -#!/usr/bin/env python3 -"""Validate bounded proposed workflows using trusted default-branch definitions.""" - -import os -import pathlib -import re -import stat -import tempfile -import unittest -from unittest import mock - -WORKFLOW = pathlib.Path(__file__).resolve().parents[1] / ".github/workflows/pr-review.yaml" -MAX_WORKFLOW_BYTES = 65536 -# Upgrade trusted approvals in three stages: expand, migrate, then contract. -# Both bootstrap and trusted validation must pass each stage before merging. -APPROVED_REVIEWER_SHAS = ("4edcf46d02011c2aa1e305aff69d4d7100aae0ca",) -EXPECTED_CALLERS = ( - r"""name: AI PR Review - -on: - issue_comment: - types: [created] - -permissions: - contents: read - issues: write - pull-requests: write - -jobs: - review: - if: >- - github.actor == 'luckyPipewrench' && - github.actor_id == '142104046' && - github.run_attempt == '1' && - github.triggering_actor == 'luckyPipewrench' && - github.event.comment.user.login == 'luckyPipewrench' && - github.event.comment.user.id == 142104046 && - github.event.comment.author_association == 'MEMBER' && - github.event.issue.pull_request && - (github.event.comment.body == '/review' || - github.event.comment.body == '/review deep') - # Keep both pins equal until the nested source identity is verified live. - uses: pipelab-org/pipelock/.github/workflows/pr-review-reusable.yaml@REVIEWER_SHA - with: - reviewer_sha: REVIEWER_SHA - pr_number: ${{ github.event.issue.number }} - review_mode: >- - ${{ github.event.comment.body == '/review deep' && 'deep' || - 'default' }} - secrets: - review_token: ${{ secrets.GITHUB_TOKEN }} - openai_api_key: ${{ secrets.OPENAI_API_KEY }} -""", -) -EXPECTED_CONTRACTS = ( - r"""name: Review caller contract - -on: - pull_request: - types: [opened, synchronize, reopened] - paths: - - '.github/workflows/pr-review.yaml' - - '.github/workflows/review-caller-contract.yaml' - - 'scripts/test_pr_review_workflow.py' - pull_request_target: - types: [opened, synchronize, reopened] - paths: - - '.github/workflows/pr-review.yaml' - - '.github/workflows/review-caller-contract.yaml' - - 'scripts/test_pr_review_workflow.py' - push: - branches: [main] - paths: - - '.github/workflows/pr-review.yaml' - - '.github/workflows/review-caller-contract.yaml' - - 'scripts/test_pr_review_workflow.py' - -permissions: - contents: read - -jobs: - bootstrap: - if: github.event_name == 'pull_request' - runs-on: ubuntu-latest - timeout-minutes: 5 - steps: - - name: Check out proposed contract for unprivileged installation tests - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - with: - persist-credentials: false - - name: Test proposed caller and contract without provider secrets - run: python3 -I scripts/test_pr_review_workflow.py - contract: - if: github.event_name != 'pull_request' - runs-on: ubuntu-latest - timeout-minutes: 5 - steps: - - name: Check out trusted default-branch caller validator - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - with: - repository: luckyPipewrench/pipelock-verify-python - ref: ${{ github.sha }} - path: trusted-validator - persist-credentials: false - sparse-checkout: scripts/test_pr_review_workflow.py - sparse-checkout-cone-mode: false - - name: Check out proposed caller as data - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - with: - repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }} - ref: ${{ github.event.pull_request.head.sha || github.sha }} - # Proposed workflows are bounded data; no proposed code is executed. - allow-unsafe-pr-checkout: true - path: proposed - persist-credentials: false - filter: blob:none - sparse-checkout: | - .github/workflows/pr-review.yaml - .github/workflows/review-caller-contract.yaml - sparse-checkout-cone-mode: false - - name: Validate proposed caller with trusted test code - id: validate - run: | - python3 -I - <<'PY' - import importlib.util - import os - import pathlib - import unittest - - validator = pathlib.Path("trusted-validator/scripts/test_pr_review_workflow.py") - spec = importlib.util.spec_from_file_location("trusted_caller", validator) - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - module.WORKFLOW = pathlib.Path("proposed/.github/workflows/pr-review.yaml") - reviewer_sha = module.validate_caller(module.WORKFLOW) - module.validate_contract( - pathlib.Path("proposed/.github/workflows/review-caller-contract.yaml") - ) - suite = unittest.defaultTestLoader.loadTestsFromModule(module) - result = unittest.TextTestRunner(verbosity=2).run(suite) - if not result.wasSuccessful(): - raise SystemExit(1) - with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as output: - output.write(f"reviewer_sha={reviewer_sha}\n") - PY - - name: Check out immutable reviewer and its security tests - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - with: - repository: luckyPipewrench/pipelock - ref: ${{ steps.validate.outputs.reviewer_sha }} - path: trusted-reviewer - persist-credentials: false - - name: Install pinned reviewer test dependencies - working-directory: trusted-reviewer - run: | - python3 -m venv .review-test-venv - .review-test-venv/bin/python -m pip install --require-hashes \ - -r .github/actions/pr-review/requirements.txt \ - -r .github/requirements-pr-review-test.txt - - name: Verify shared reviewer workflow protections - working-directory: trusted-reviewer - run: >- - .review-test-venv/bin/python -m unittest - scripts.pr_review_test.WorkflowPackagingTest - scripts.pr_review_test.FailureDirectionTest.test_review_job_timeout_exceeds_the_wall_clock_with_finalization_margin -""", -) - - -def read_workflow(path): - """Read bounded UTF-8 data without following any path-component symlink.""" - path = pathlib.Path(path) - if ".." in path.parts: - raise ValueError("workflow path must not traverse parents") - parts = path.parts[1:] if path.is_absolute() else path.parts - directory_flags = os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW - directory = os.open(path.anchor if path.is_absolute() else ".", directory_flags) - try: - for part in parts[:-1]: - child = os.open(part, directory_flags, dir_fd=directory) - os.close(directory) - directory = child - fd = os.open(parts[-1], os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK, dir_fd=directory) - try: - info = os.fstat(fd) - if not stat.S_ISREG(info.st_mode) or info.st_size > MAX_WORKFLOW_BYTES: - raise ValueError("workflow must be a bounded regular file") - with os.fdopen(fd, "rb", closefd=False) as stream: - data = stream.read(MAX_WORKFLOW_BYTES + 1) - if len(data) > MAX_WORKFLOW_BYTES: - raise ValueError("workflow exceeds the data limit") - return data.decode("utf-8") - finally: - os.close(fd) - finally: - os.close(directory) - - -def validate_caller(path): - """Accept only an approved shape with matching approved immutable pins.""" - text = read_workflow(path) - pins = re.findall( - r"(?:pr-review-reusable\.yaml@|reviewer_sha: )([0-9a-f]{40})$", - text, - re.MULTILINE, - ) - if len(pins) != 2 or pins[0] != pins[1]: - raise ValueError("both reviewer references must be matching full SHAs") - if pins[0] not in APPROVED_REVIEWER_SHAS: - raise ValueError("land reviewer approval on main before migrating the caller") - if text.replace(pins[0], "REVIEWER_SHA") not in EXPECTED_CALLERS: - raise ValueError("proposed caller differs from its trusted definition") - return pins[0] - - -def validate_contract(path): - """Compare exact text, including executable block-scalar comments.""" - if read_workflow(path) not in EXPECTED_CONTRACTS: - raise ValueError("proposed contract differs from its trusted definition") - - -class ReviewCallerTest(unittest.TestCase): - def test_caller_contract_and_matching_immutable_pins(self): - validate_caller(WORKFLOW) - - def test_installed_contract(self): - validate_contract(WORKFLOW.with_name("review-caller-contract.yaml")) - - def test_only_the_organization_caller_and_reviewer_are_approved(self): - self.assertEqual(APPROVED_REVIEWER_SHAS, ("4edcf46d02011c2aa1e305aff69d4d7100aae0ca",)) - self.assertEqual(len(EXPECTED_CALLERS), 1) - self.assertEqual(len(EXPECTED_CONTRACTS), 1) - for required in [ - "uses: pipelab-org/pipelock/.github/workflows/pr-review-reusable.yaml@REVIEWER_SHA", - "github.actor_id == '142104046'", - "github.event.comment.user.id == 142104046", - "github.run_attempt == '1'", - "author_association == 'MEMBER'", - ]: - self.assertIn(required, EXPECTED_CALLERS[0]) - self.assertNotIn("luckyPipewrench/pipelock/", EXPECTED_CALLERS[0]) - - -class WorkflowInputTest(unittest.TestCase): - def test_bounded_regular_utf8_file(self): - with tempfile.TemporaryDirectory() as tmp: - path = pathlib.Path(tmp) / "caller.yaml" - path.write_text("name: caller\n", encoding="utf-8") - self.assertEqual(read_workflow(path), "name: caller\n") - path.write_bytes(b"x" * (MAX_WORKFLOW_BYTES + 1)) - with self.assertRaises(ValueError): - read_workflow(path) - path.write_bytes(b"\xff") - with self.assertRaises(UnicodeError): - read_workflow(path) - - def test_file_growth_after_stat_is_bounded(self): - with tempfile.TemporaryDirectory() as tmp: - path = pathlib.Path(tmp) / "caller.yaml" - path.write_text("name: caller\n", encoding="utf-8") - original_fstat = os.fstat - - def grow_after_stat(fd): - info = original_fstat(fd) - path.write_bytes(b"x" * (MAX_WORKFLOW_BYTES + 1)) - return info - - with mock.patch.object(os, "fstat", side_effect=grow_after_stat): - with self.assertRaisesRegex(ValueError, "exceeds the data limit"): - read_workflow(path) - - def test_symlink_and_symlink_parent_are_rejected(self): - with tempfile.TemporaryDirectory() as tmp: - root = pathlib.Path(tmp) - directory = root / "outside" - directory.mkdir() - path = directory / "caller.yaml" - path.write_text("outside sentinel", encoding="utf-8") - link = root / "caller.yaml" - link.symlink_to(path) - parent = root / "proposed" - parent.symlink_to(directory, target_is_directory=True) - for candidate in [link, parent / "caller.yaml"]: - with self.subTest(path=candidate), self.assertRaises(OSError): - read_workflow(candidate) - - def test_fifo_and_parent_traversal_are_rejected(self): - with tempfile.TemporaryDirectory() as tmp: - path = pathlib.Path(tmp) / "caller.yaml" - os.mkfifo(path) - with self.assertRaises(ValueError): - read_workflow(path) - with self.assertRaises(ValueError): - read_workflow(path.parent / ".." / "caller.yaml") - - def test_exact_contract_and_caller_changes_are_rejected(self): - caller = EXPECTED_CALLERS[0].replace("REVIEWER_SHA", APPROVED_REVIEWER_SHAS[0]) - contract = EXPECTED_CONTRACTS[0] - with tempfile.TemporaryDirectory() as tmp: - path = pathlib.Path(tmp) / "workflow.yaml" - for expected, check, mutations in [ - ( - caller, - validate_caller, - [ - caller.replace("contents: read", "contents: write"), - caller + "# unapproved caller comment\n", - caller.replace(" issue_comment:", " issue_comment:\n\n"), - caller.replace(APPROVED_REVIEWER_SHAS[0], "main"), - caller.replace(APPROVED_REVIEWER_SHAS[0], "a" * 40, 1), - caller.replace(APPROVED_REVIEWER_SHAS[0], "a" * 40), - caller.replace(APPROVED_REVIEWER_SHAS[0], "a" * 39), - caller.replace( - APPROVED_REVIEWER_SHAS[0], "3e8534e0dd621c62879238cc01d56f7e58e9d096" - ), - caller.replace("pipelab-org/pipelock/", "luckyPipewrench/pipelock/"), - caller.replace("github.actor_id == '142104046'", "github.actor_id == '0'"), - caller.replace( - "github.event.comment.user.id == 142104046", - "github.event.comment.user.id == 0", - ), - caller.replace("github.run_attempt == '1'", "github.run_attempt == '2'"), - caller.replace( - "author_association == 'MEMBER'", "author_association == 'OWNER'" - ), - ], - ), - ( - contract, - validate_contract, - [ - contract.replace("contents: read", "contents: write"), - contract.replace("module.validate_contract(", "module.skip_contract("), - contract.replace("--require-hashes", "--no-deps"), - contract.replace("WorkflowPackagingTest", "MissingProtectionTest"), - contract.replace( - "ref: ${{ github.sha }}", - "ref: ${{ github.event.pull_request.head.sha }}", - ), - contract.replace( - " scripts.pr_review_test.FailureDirectionTest", - " # retained\n scripts.pr_review_test.FailureDirectionTest", - ), - contract.replace( - "ref: ${{ steps.validate.outputs.reviewer_sha }}", "ref: main" - ), - contract + "\n", - ], - ), - ]: - path.write_text(expected, encoding="utf-8") - check(path) - for mutation in mutations: - with self.subTest(check=check.__name__, mutation=mutation[:60]): - self.assertNotEqual(mutation, expected) - path.write_text(mutation, encoding="utf-8") - with self.assertRaises(ValueError): - check(path) - - def test_reviewer_upgrade_expand_migrate_contract(self): - """Trusted code expands approval before migration and removes it after.""" - old = APPROVED_REVIEWER_SHAS[0] - new = "a" * 40 - with tempfile.TemporaryDirectory() as tmp: - caller = pathlib.Path(tmp) / "caller.yaml" - contract = pathlib.Path(tmp) / "contract.yaml" - contract.write_text(EXPECTED_CONTRACTS[0], encoding="utf-8") - for approved, proposed, accepted in [ - ((old,), old, True), - ((old,), new, False), - ((old, new), old, True), - ((old, new), new, True), - ((new,), new, True), - ((new,), old, False), - ]: - with self.subTest(approved=approved, proposed=proposed): - caller.write_text( - EXPECTED_CALLERS[0].replace("REVIEWER_SHA", proposed), encoding="utf-8" - ) - with mock.patch.dict(globals(), APPROVED_REVIEWER_SHAS=approved): - if accepted: - self.assertEqual(validate_caller(caller), proposed) - validate_contract(contract) - else: - with self.assertRaises(ValueError): - validate_caller(caller) - - def test_shape_upgrade_expand_migrate_contract(self): - """Future exact templates can migrate without weakening the comparator.""" - with tempfile.TemporaryDirectory() as tmp: - path = pathlib.Path(tmp) / "workflow.yaml" - for key, check, old in [ - ("EXPECTED_CALLERS", validate_caller, EXPECTED_CALLERS[0]), - ("EXPECTED_CONTRACTS", validate_contract, EXPECTED_CONTRACTS[0]), - ]: - new = old.replace("name:", "# approved shape revision\nname:", 1) - for approved, proposed, accepted in [ - ((old,), old, True), - ((old,), new, False), - ((old, new), old, True), - ((old, new), new, True), - ((new,), new, True), - ((new,), old, False), - ]: - with self.subTest(key=key, accepted=accepted): - path.write_text( - proposed.replace("REVIEWER_SHA", APPROVED_REVIEWER_SHAS[0]), - encoding="utf-8", - ) - with mock.patch.dict(globals(), {key: approved}): - if accepted: - check(path) - else: - with self.assertRaises(ValueError): - check(path) - - -if __name__ == "__main__": - unittest.main() diff --git a/tests/test_pr_review_routing.py b/tests/test_pr_review_routing.py deleted file mode 100644 index 14653f7..0000000 --- a/tests/test_pr_review_routing.py +++ /dev/null @@ -1,121 +0,0 @@ -"""Regression tests for the trusted GitHub PR-review runner's model routing.""" - -from __future__ import annotations - -import importlib.util -import unittest -from pathlib import Path -from types import ModuleType -from unittest.mock import Mock, patch - -ROOT = Path(__file__).resolve().parents[1] -SCRIPT_PATH = ROOT / "scripts" / "pr-review.py" - - -def load_pr_review_module() -> ModuleType: - """Load the workflow script without executing its CLI entry point.""" - spec = importlib.util.spec_from_file_location("pr_review_routing", SCRIPT_PATH) - assert spec is not None - assert spec.loader is not None - module = importlib.util.module_from_spec(spec) - with patch.dict("sys.modules", {"requests": Mock()}): - spec.loader.exec_module(module) - return module - - -class PRReviewRoutingTests(unittest.TestCase): - def test_model_defaults_and_mode_routing(self): - module = load_pr_review_module() - with patch.dict( - module.os.environ, - {"PR_REVIEW_MODEL_FAST": "", "PR_REVIEW_MODEL_DEEP": ""}, - clear=False, - ): - self.assertEqual(module.DEFAULT_MODEL_FAST, "gpt-5.6-luna") - self.assertEqual(module.DEFAULT_MODEL_DEEP, "gpt-5.6-terra") - self.assertEqual(module.model_for_mode("default"), "gpt-5.6-luna") - self.assertEqual(module.model_for_mode("tests"), "gpt-5.6-luna") - self.assertEqual(module.model_for_mode("docs"), "gpt-5.6-luna") - self.assertEqual(module.model_for_mode("deep"), "gpt-5.6-terra") - self.assertEqual(module.FAST_REASONING_EFFORT, "low") - self.assertEqual(module.DEEP_REASONING_EFFORT, "xhigh") - - def test_gpt_5_6_payloads_pin_the_requested_reasoning_effort(self): - module = load_pr_review_module() - - ordinary_payload = module.build_llm_payload( - module.DEFAULT_MODEL_FAST, - "system prompt", - "diff", - reasoning_effort=module.FAST_REASONING_EFFORT, - ) - deep_payload = module.build_llm_payload( - module.DEFAULT_MODEL_DEEP, - "system prompt", - "diff", - reasoning_effort=module.DEEP_REASONING_EFFORT, - max_completion_tokens=module.DEEP_MAX_COMPLETION_TOKENS, - ) - - self.assertEqual(ordinary_payload["model"], "gpt-5.6-luna") - self.assertEqual(ordinary_payload["reasoning_effort"], "low") - self.assertEqual(ordinary_payload["max_completion_tokens"], 8192) - self.assertEqual(deep_payload["model"], "gpt-5.6-terra") - self.assertEqual(deep_payload["reasoning_effort"], "xhigh") - self.assertEqual(deep_payload["max_completion_tokens"], 64000) - - def test_model_repository_variable_overrides(self): - module = load_pr_review_module() - with patch.dict( - module.os.environ, - { - "PR_REVIEW_MODEL_FAST": "provider/ordinary-override", - "PR_REVIEW_MODEL_DEEP": "provider/deep-override", - }, - clear=False, - ): - self.assertEqual(module.model_for_mode("default"), "provider/ordinary-override") - self.assertEqual(module.model_for_mode("tests"), "provider/ordinary-override") - self.assertEqual(module.model_for_mode("docs"), "provider/ordinary-override") - self.assertEqual(module.model_for_mode("deep"), "provider/deep-override") - - def test_empty_repository_variable_overrides_use_python_defaults(self): - module = load_pr_review_module() - with patch.dict( - module.os.environ, - {"PR_REVIEW_MODEL_FAST": "", "PR_REVIEW_MODEL_DEEP": ""}, - clear=False, - ): - self.assertEqual(module.model_for_mode("default"), module.DEFAULT_MODEL_FAST) - self.assertEqual(module.model_for_mode("deep"), module.DEFAULT_MODEL_DEEP) - - def test_response_shape_errors_are_generic_and_fail_closed(self): - module = load_pr_review_module() - - with self.assertRaises(module.LLMReviewError) as ctx: - module.extract_chat_content({"choices": [], "private": "provider detail"}) - self.assertIn("no choices", str(ctx.exception)) - self.assertNotIn("provider detail", str(ctx.exception)) - - with self.assertRaisesRegex(module.LLMReviewError, "empty content"): - module.extract_chat_content({"choices": [None]}) - - def test_call_rejects_invalid_or_non_object_json(self): - module = load_pr_review_module() - invalid = Mock(status_code=200) - invalid.json.side_effect = ValueError("invalid") - with ( - patch.dict(module.os.environ, {"OPENAI_API_KEY": "test"}, clear=True), - patch.object(module.requests, "post", return_value=invalid), - self.assertRaisesRegex(module.LLMReviewError, "invalid JSON"), - ): - module.call_llm("diff", "default", "system") - - non_object = Mock(status_code=200) - non_object.json.return_value = [] - with ( - patch.dict(module.os.environ, {"OPENAI_API_KEY": "test"}, clear=True), - patch.object(module.requests, "post", return_value=non_object), - self.assertRaisesRegex(module.LLMReviewError, "non-object JSON"), - ): - module.call_llm("diff", "default", "system") From 394fcd2966e86b8e18ffb2c920ba1907444feda7 Mon Sep 17 00:00:00 2001 From: luckyPipewrench Date: Sun, 18 Oct 2026 20:45:35 -0400 Subject: [PATCH 2/2] ci: remove obsolete review dependencies --- .github/requirements-pr-review.txt | 146 ----------------------------- 1 file changed, 146 deletions(-) delete mode 100644 .github/requirements-pr-review.txt diff --git a/.github/requirements-pr-review.txt b/.github/requirements-pr-review.txt deleted file mode 100644 index 0c5b24d..0000000 --- a/.github/requirements-pr-review.txt +++ /dev/null @@ -1,146 +0,0 @@ -# Pinned dependencies for pr-review.yaml workflow. -# All hashes from PyPI; includes py3-none-any and cp312 manylinux_x86_64 -# variants so the install works on GitHub Ubuntu runners. -# Regenerate: pip download --no-deps, then pip hash . -certifi==2026.4.22 \ - --hash=sha256:3cb2210c8f88ba2318d29b0388d1023c8492ff72ecdde4ebdaddbb13a31b1c4a \ - --hash=sha256:8d455352a37b71bf76a79caa83a3d6c25afee4a385d632127b6afb3963f1c580 -charset-normalizer==3.4.7 \ - --hash=sha256:007d05ec7321d12a40227aae9e2bc6dca73f3cb21058999a1df9e193555a9dcc \ - --hash=sha256:03853ed82eeebbce3c2abfdbc98c96dc205f32a79627688ac9a27370ea61a49c \ - --hash=sha256:07d9e39b01743c3717745f4c530a6349eadbfa043c7577eef86c502c15df2c67 \ - --hash=sha256:08e721811161356f97b4059a9ba7bafb23ea5ee2255402c42881c214e173c6b4 \ - --hash=sha256:0c96c3b819b5c3e9e165495db84d41914d6894d55181d2d108cc1a69bfc9cce0 \ - --hash=sha256:0ea948db76d31190bf08bd371623927ee1339d5f2a0b4b1b4a4439a65298703c \ - --hash=sha256:0f7eb884681e3938906ed0434f20c63046eacd0111c4ba96f27b76084cd679f5 \ - --hash=sha256:12a6fff75f6bc66711b73a2f0addfc4c8c15a20e805146a02d147a318962c444 \ - --hash=sha256:12d8baf840cc7889b37c7c770f478adea7adce3dcb3944d02ec87508e2dcf153 \ - --hash=sha256:14265bfe1f09498b9d8ec91e9ec9fa52775edf90fcbde092b25f4a33d444fea9 \ - --hash=sha256:16d971e29578a5e97d7117866d15889a4a07befe0e87e703ed63cd90cb348c01 \ - --hash=sha256:177a0ba5f0211d488e295aaf82707237e331c24788d8d76c96c5a41594723217 \ - --hash=sha256:1a87ca9d5df6fe460483d9a5bbf2b18f620cbed41b432e2bddb686228282d10b \ - --hash=sha256:1c2a768fdd44ee4a9339a9b0b130049139b8ce3c01d2ce09f67f5a68048d477c \ - --hash=sha256:1c2aed2e5e41f24ea8ef1590b8e848a79b56f3a5564a65ceec43c9d692dc7d8a \ - --hash=sha256:1dc8b0ea451d6e69735094606991f32867807881400f808a106ee1d963c46a83 \ - --hash=sha256:1efde3cae86c8c273f1eb3b287be7d8499420cf2fe7585c41d370d3e790054a5 \ - --hash=sha256:202389074300232baeb53ae2569a60901f7efadd4245cf3a3bf0617d60b439d7 \ - --hash=sha256:203104ed3e428044fd943bc4bf45fa73c0730391f9621e37fe39ecf477b128cb \ - --hash=sha256:2257141f39fe65a3fdf38aeccae4b953e5f3b3324f4ff0daf9f15b8518666a2c \ - --hash=sha256:298930cec56029e05497a76988377cbd7457ba864beeea92ad7e844fe74cd1f1 \ - --hash=sha256:2cd4a60d0e2fb04537162c62bbbb4182f53541fe0ede35cdf270a1c1e723cc42 \ - --hash=sha256:2d6eb928e13016cea4f1f21d1e10c1cebd5a421bc57ddf5b1142ae3f86824fab \ - --hash=sha256:2fe249cb4651fd12605b7288b24751d8bfd46d35f12a20b1ba33dea122e690df \ - --hash=sha256:30b8d1d8c52a48c2c5690e152c169b673487a2a58de1ec7393196753063fcd5e \ - --hash=sha256:320ade88cfb846b8cd6b4ddf5ee9e80ee0c1f52401f2456b84ae1ae6a1a5f207 \ - --hash=sha256:3534e7dcbdcf757da6b85a0bbf5b6868786d5982dd959b065e65481644817a18 \ - --hash=sha256:36836d6ff945a00b88ba1e4572d721e60b5b8c98c155d465f56ad19d68f23734 \ - --hash=sha256:38c0109396c4cfc574d502df99742a45c72c08eff0a36158b6f04000043dbf38 \ - --hash=sha256:3946fa46a0cf3e4c8cb1cc52f56bb536310d34f25f01ca9b6c16afa767dab110 \ - --hash=sha256:3bec022aec2c514d9cf199522a802bd007cd588ab17ab2525f20f9c34d067c18 \ - --hash=sha256:3c9a494bc5ec77d43cea229c4f6db1e4d8fe7e1bbffa8b6f0f0032430ff8ab44 \ - --hash=sha256:3dce51d0f5e7951f8bb4900c257dad282f49190fdbebecd4ba99bcc41fef404d \ - --hash=sha256:3dedcc22d73ec993f42055eff4fcfed9318d1eeb9a6606c55892a26964964e48 \ - --hash=sha256:4042d5c8f957e15221d423ba781e85d553722fc4113f523f2feb7b188cc34c5e \ - --hash=sha256:481551899c856c704d58119b5025793fa6730adda3571971af568f66d2424bb5 \ - --hash=sha256:4dc1e73c36828f982bfe79fadf5919923f8a6f4df2860804db9a98c48824ce8d \ - --hash=sha256:4e5163c14bffd570ef2affbfdd77bba66383890797df43dc8b4cc7d6f500bf53 \ - --hash=sha256:511ef87c8aec0783e08ac18565a16d435372bc1ac25a91e6ac7f5ef2b0bff790 \ - --hash=sha256:532bc9bf33a68613fd7d65e4b1c71a6a38d7d42604ecf239c77392e9b4e8998c \ - --hash=sha256:54523e136b8948060c0fa0bc7b1b50c32c186f2fceee897a495406bb6e311d2b \ - --hash=sha256:5649fd1c7bade02f320a462fdefd0b4bd3ce036065836d4f42e0de958038e116 \ - --hash=sha256:56be790f86bfb2c98fb742ce566dfb4816e5a83384616ab59c49e0604d49c51d \ - --hash=sha256:5b77459df20e08151cd6f8b9ef8ef1f961ef73d85c21a555c7eed5b79410ec10 \ - --hash=sha256:5ed6ab538499c8644b8a3e18debabcd7ce684f3fa91cf867521a7a0279cab2d6 \ - --hash=sha256:6178f72c5508bfc5fd446a5905e698c6212932f25bcdd4b47a757a50605a90e2 \ - --hash=sha256:6370e8686f662e6a3941ee48ed4742317cafbe5707e36406e9df792cdb535776 \ - --hash=sha256:64f02c6841d7d83f832cd97ccf8eb8a906d06eb95d5276069175c696b024b60a \ - --hash=sha256:65bcd23054beab4d166035cabbc868a09c1a49d1efe458fe8e4361215df40265 \ - --hash=sha256:66671f93accb62ed07da56613636f3641f1a12c13046ce91ffc923721f23c008 \ - --hash=sha256:6696b7688f54f5af4462118f0bfa7c1621eeb87154f77fa04b9295ce7a8f2943 \ - --hash=sha256:6785f414ae0f3c733c437e0f3929197934f526d19dfaa75e18fdb4f94c6fb374 \ - --hash=sha256:67f6279d125ca0046a7fd386d01b311c6363844deac3e5b069b514ba3e63c246 \ - --hash=sha256:6c114670c45346afedc0d947faf3c7f701051d2518b943679c8ff88befe14f8e \ - --hash=sha256:6e0d51f618228538a3e8f46bd246f87a6cd030565e015803691603f55e12afb5 \ - --hash=sha256:6ed74185b2db44f41ef35fd1617c5888e59792da9bbc9190d6c7300617182616 \ - --hash=sha256:708838739abf24b2ceb208d0e22403dd018faeef86ddac04319a62ae884c4f15 \ - --hash=sha256:715479b9a2802ecac752a3b0efa2b0b60285cf962ee38414211abdfccc233b41 \ - --hash=sha256:733784b6d6def852c814bce5f318d25da2ee65dd4839a0718641c696e09a2960 \ - --hash=sha256:750e02e074872a3fad7f233b47734166440af3cdea0add3e95163110816d6752 \ - --hash=sha256:752a45dc4a6934060b3b0dab47e04edc3326575f82be64bc4fc293914566503e \ - --hash=sha256:7579e913a5339fb8fa133f6bbcfd8e6749696206cf05acdbdca71a1b436d8e72 \ - --hash=sha256:7641bb8895e77f921102f72833904dcd9901df5d6d72a2ab8f31d04b7e51e4e7 \ - --hash=sha256:7804338df6fcc08105c7745f1502ba68d900f45fd770d5bdd5288ddccb8a42d8 \ - --hash=sha256:80d04837f55fc81da168b98de4f4b797ef007fc8a79ab71c6ec9bc4dd662b15b \ - --hash=sha256:813c0e0132266c08eb87469a642cb30aaff57c5f426255419572aaeceeaa7bf4 \ - --hash=sha256:82b271f5137d07749f7bf32f70b17ab6eaabedd297e75dce75081a24f76eb545 \ - --hash=sha256:84c018e49c3bf790f9c2771c45e9313a08c2c2a6342b162cd650258b57817706 \ - --hash=sha256:8751d2787c9131302398b11e6c8068053dcb55d5a8964e114b6e196cf16cb366 \ - --hash=sha256:8778f0c7a52e56f75d12dae53ae320fae900a8b9b4164b981b9c5ce059cd1fcb \ - --hash=sha256:87fad7d9ba98c86bcb41b2dc8dbb326619be2562af1f8ff50776a39e55721c5a \ - --hash=sha256:8d828b6667a32a728a1ad1d93957cdf37489c57b97ae6c4de2860fa749b8fc1e \ - --hash=sha256:8e385e4267ab76874ae30db04c627faaaf0b509e1ccc11a95b3fc3e83f855c00 \ - --hash=sha256:92a0a01ead5e668468e952e4238cccd7c537364eb7d851ab144ab6627dbbe12f \ - --hash=sha256:94e1885b270625a9a828c9793b4d52a64445299baa1fea5a173bf1d3dd9a1a5a \ - --hash=sha256:a180c5e59792af262bf263b21a3c49353f25945d8d9f70628e73de370d55e1e1 \ - --hash=sha256:a277ab8928b9f299723bc1a2dabb1265911b1a76341f90a510368ca44ad9ab66 \ - --hash=sha256:a5fe03b42827c13cdccd08e6c0247b6a6d4b5e3cdc53fd1749f5896adcdc2356 \ - --hash=sha256:a6c5863edfbe888d9eff9c8b8087354e27618d9da76425c119293f11712a6319 \ - --hash=sha256:a89c23ef8d2c6b27fd200a42aa4ac72786e7c60d40efdc76e6011260b6e949c4 \ - --hash=sha256:adb2597b428735679446b46c8badf467b4ca5f5056aae4d51a19f9570301b1ad \ - --hash=sha256:ae196f021b5e7c78e918242d217db021ed2a6ace2bc6ae94c0fc596221c7f58d \ - --hash=sha256:ae89db9e5f98a11a4bf50407d4363e7b09b31e55bc117b4f7d80aab97ba009e5 \ - --hash=sha256:aed52fea0513bac0ccde438c188c8a471c4e0f457c2dd20cdbf6ea7a450046c7 \ - --hash=sha256:aef65cd602a6d0e0ff6f9930fcb1c8fec60dd2cfcb6facaf4bdb0e5873042db0 \ - --hash=sha256:af21eb4409a119e365397b2adbaca4c9ccab56543a65d5dbd9f920d6ac29f686 \ - --hash=sha256:b14b2d9dac08e28bb8046a1a0434b1750eb221c8f5b87a68f4fa11a6f97b5e34 \ - --hash=sha256:bb6d88045545b26da47aa879dd4a89a71d1dce0f0e549b1abcb31dfe4a8eac49 \ - --hash=sha256:bb8cc7534f51d9a017b93e3e85b260924f909601c3df002bcdb58ddb4dc41a5c \ - --hash=sha256:bc17a677b21b3502a21f66a8cc64f5bfad4df8a0b8434d661666f8ce90ac3af1 \ - --hash=sha256:bd6c2a1c7573c64738d716488d2cdd3c00e340e4835707d8fdb8dc1a66ef164e \ - --hash=sha256:bd9b23791fe793e4968dba0c447e12f78e425c59fc0e3b97f6450f4781f3ee60 \ - --hash=sha256:c03a41a8784091e67a39648f70c5f97b5b6a37f216896d44d2cdcb82615339a0 \ - --hash=sha256:c0f081d69a6e58272819b70288d3221a6ee64b98df852631c80f293514d3b274 \ - --hash=sha256:c35abb8bfff0185efac5878da64c45dafd2b37fb0383add1be155a763c1f083d \ - --hash=sha256:c36c333c39be2dbca264d7803333c896ab8fa7d4d6f0ab7edb7dfd7aea6e98c0 \ - --hash=sha256:c45e9440fb78f8ddabcf714b68f936737a121355bf59f3907f4e17721b9d1aae \ - --hash=sha256:c593052c465475e64bbfe5dbd81680f64a67fdc752c56d7a0ae205dc8aeefe0f \ - --hash=sha256:cdd68a1fb318e290a2077696b7eb7a21a49163c455979c639bf5a5dcdc46617d \ - --hash=sha256:ce3412fbe1e31eb81ea42f4169ed94861c56e643189e1e75f0041f3fe7020abe \ - --hash=sha256:cf1493cd8607bec4d8a7b9b004e699fcf8f9103a9284cc94962cb73d20f9d4a3 \ - --hash=sha256:cf29836da5119f3c8a8a70667b0ef5fdca3bb12f80fd06487cfa575b3909b393 \ - --hash=sha256:d4a48e5b3c2a489fae013b7589308a40146ee081f6f509e047e0e096084ceca1 \ - --hash=sha256:d560742f3c0d62afaccf9f41fe485ed69bd7661a241f86a3ef0f0fb8b1a397af \ - --hash=sha256:d6038d37043bced98a66e68d3aa2b6a35505dc01328cd65217cefe82f25def44 \ - --hash=sha256:d61f00a0869d77422d9b2aba989e2d24afa6ffd552af442e0e58de4f35ea6d00 \ - --hash=sha256:d635aab80466bc95771bb78d5370e74d36d1fe31467b6b29b8b57b2a3cd7d22c \ - --hash=sha256:dca4bbc466a95ba9c0234ef56d7dd9509f63da22274589ebd4ed7f1f4d4c54e3 \ - --hash=sha256:dd915403e231e6b1809fe9b6d9fc55cf8fb5e02765ac625d9cd623342a7905d7 \ - --hash=sha256:e044c39e41b92c845bc815e5ae4230804e8e7bc29e399b0437d64222d92809dd \ - --hash=sha256:e060d01aec0a910bdccb8be71faf34e7799ce36950f8294c8bf612cba65a2c9e \ - --hash=sha256:e1421b502d83040e6d7fb2fb18dff63957f720da3d77b2fbd3187ceb63755d7b \ - --hash=sha256:e17b8d5d6a8c47c85e68ca8379def1303fd360c3e22093a807cd34a71cd082b8 \ - --hash=sha256:e5f4d355f0a2b1a31bc3edec6795b46324349c9cb25eed068049e4f472fb4259 \ - --hash=sha256:e712b419df8ba5e42b226c510472b37bd57b38e897d3eca5e8cfd410a29fa859 \ - --hash=sha256:e74327fb75de8986940def6e8dee4f127cc9752bee7355bb323cc5b2659b6d46 \ - --hash=sha256:e80c8378d8f3d83cd3164da1ad2df9e37a666cdde7b1cb2298ed0b558064be30 \ - --hash=sha256:e8ac484bf18ce6975760921bb6148041faa8fef0547200386ea0b52b5d27bf7b \ - --hash=sha256:eca9705049ad3c7345d574e3510665cb2cf844c2f2dcfe675332677f081cbd46 \ - --hash=sha256:ed065083d0898c9d5b4bbec7b026fd755ff7454e6e8b73a67f8c744b13986e24 \ - --hash=sha256:edac0f1ab77644605be2cbba52e6b7f630731fc42b34cb0f634be1a6eface56a \ - --hash=sha256:effc3f449787117233702311a1b7d8f59cba9ced946ba727bdc329ec69028e24 \ - --hash=sha256:f22dec1690b584cea26fade98b2435c132c1b5f68e39f5a0b7627cd7ae31f1dc \ - --hash=sha256:f495a1652cf3fbab2eb0639776dad966c2fb874d79d87ca07f9d5f059b8bd215 \ - --hash=sha256:f496c9c3cc02230093d8330875c4c3cdfc3b73612a5fd921c65d39cbcef08063 \ - --hash=sha256:f59099f9b66f0d7145115e6f80dd8b1d847176df89b234a5a6b3f00437aa0832 \ - --hash=sha256:f59ad4c0e8f6bba240a9bb85504faa1ab438237199d4cce5f622761507b8f6a6 \ - --hash=sha256:fbccdc05410c9ee21bbf16a35f4c1d16123dcdeb8a1d38f33654fa21d0234f79 \ - --hash=sha256:fea24543955a6a729c45a73fe90e08c743f0b3334bbf3201e6c4bc1b0c7fa464 -idna==3.15 \ - --hash=sha256:048adeaf8c2d788c40fee287673ccaa74c24ffd8dcf09ffa555a2fbb59f10ac8 \ - --hash=sha256:ca962446ea538f7092a95e057da437618e886f4d349216d2b1e294abfdb65fdc -requests==2.34.0 \ - --hash=sha256:7d62fe92f50eb82c529b0916bb445afa1531a566fc8f35ffdc64446e771b856a \ - --hash=sha256:917520a21b767485ce7c588f4ebb917c436b24a31231b44228715eaeb5a52c60 -urllib3==2.7.0 \ - --hash=sha256:231e0ec3b63ceb14667c67be60f2f2c40a518cb38b03af60abc813da26505f4c \ - --hash=sha256:9fb4c81ebbb1ce9531cce37674bbc6f1360472bc18ca9a553ede278ef7276897