diff --git a/.github/CODE_QUALITY.md b/.github/CODE_QUALITY.md index 903611d06d..465c8292aa 100644 --- a/.github/CODE_QUALITY.md +++ b/.github/CODE_QUALITY.md @@ -109,7 +109,7 @@ Branch protection to require these checks before merge is future work. ### 6. LLM Review -[`llm-review.yml`](workflows/llm-review.yml) runs after successful Lean Action CI on a PR head, or manually through `/review` and `workflow_dispatch`. It fetches the PR diff with `gh`, classifies the PR, and posts a sticky PR comment containing the reviewed head SHA, structured assessment, verdict, findings, token counts, tier, and estimated cost. +[`llm-review.yml`](workflows/llm-review.yml) runs after successful Lean Action CI on a PR head, or manually through `/review` and `workflow_dispatch`. It fetches the PR diff with `gh`, classifies the PR, and posts a sticky PR comment containing the reviewed head SHA, structured assessment, verdict, findings, token counts, and billing source. Reviews use GPT-6-Astra through the Azure VM's Codex account pool, without an OpenAI API key or paid API fallback. See [Azure review operations](../python/azure-review.md). The rules applied depend on what the PR does: diff --git a/.github/workflows/llm-review.yml b/.github/workflows/llm-review.yml index 6125338049..a2367da0b2 100644 --- a/.github/workflows/llm-review.yml +++ b/.github/workflows/llm-review.yml @@ -182,11 +182,35 @@ jobs: working-directory: python run: uv sync --group review + - name: Configure restricted Azure review connection + if: steps.ci.outputs.conclusion == 'success' + env: + REVIEW_SSH_KEY: ${{ secrets.REVIEW_AZURE_SSH_KEY }} + REVIEW_KNOWN_HOSTS: ${{ secrets.REVIEW_AZURE_KNOWN_HOSTS }} + run: | + set -euo pipefail + test -n "$REVIEW_SSH_KEY" + test -n "$REVIEW_KNOWN_HOSTS" + install -d -m 700 ~/.ssh + umask 077 + printf '%s\n' "$REVIEW_SSH_KEY" > ~/.ssh/lean_pool_review + printf '%s\n' "$REVIEW_KNOWN_HOSTS" > ~/.ssh/lean_pool_review_known_hosts + cat >> ~/.ssh/config <<'CONFIG' + Host lean-pool-review-azure + HostName leanvm-vilin97.westus2.cloudapp.azure.com + User vasil + IdentityFile ~/.ssh/lean_pool_review + IdentitiesOnly yes + UserKnownHostsFile ~/.ssh/lean_pool_review_known_hosts + StrictHostKeyChecking yes + CONFIG + - name: Run LLM review if: steps.ci.outputs.conclusion == 'success' working-directory: python env: - OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + REVIEW_BACKEND: codex-azure + REVIEW_SSH_HOST: lean-pool-review-azure # Mathlib prior-art search. Absent, the review still runs and # is told the search did not happen. LEANEXPLORE_API_KEY: ${{ secrets.LEANEXPLORE_API_KEY }} @@ -194,17 +218,15 @@ jobs: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} GITHUB_REPOSITORY: ${{ github.repository }} REVIEW_HEAD_SHA: ${{ steps.pr.outputs.head_sha }} - # Override here if the model or effort needs to change. The - # gpt-5.6 alias tracks the newest gpt-5.6-sol snapshot; there - # is no cross-family rolling alias, so a new family (gpt-5.7) - # needs a bump here. Per-tier prices live in PRICING_PER_M - # inside review.py — update there when bumping the model. - REVIEW_MODEL: gpt-5.6 - # Reasoning depth (main cost dial): none/low/medium/high/xhigh/max, - # or "default" to let the model decide. + # Uses the VM's Codex account quota; no API key or paid fallback. + REVIEW_MODEL: gpt-6-astra REVIEW_EFFORT: xhigh run: uv run python -m lean_pool.review + - name: Remove Azure review credentials + if: always() + run: rm -f ~/.ssh/lean_pool_review ~/.ssh/lean_pool_review_known_hosts + # Request Copilot's PR review. Idempotent — re-requesting the same # reviewer is a no-op. `if: always()` ensures Copilot still gets # asked even when LLM review was skipped or failed. diff --git a/AGENTS.md b/AGENTS.md index 148465b152..346b87ac46 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,6 +22,8 @@ Lean Pool sits between [`mathlib`](https://github.com/leanprover-community/mathl So far, projects have been added by hand: each is a suitable, permissively licensed (Apache-2.0 or MIT) Lean repository, bumped to the latest Lean and Mathlib, made to pass [CI](.github/workflows/lean_action_ci.yml) — it builds warning-free and clears Mathlib's linters, the style checker, and the repository quality gates (no `sorry`/`admit`, no axioms beyond `Classical.choice`/`propext`/`Quot.sound`, no `unsafe`/`partial`, file headers, size limits) — and an [LLM review](.github/REVIEW_RULES.md) of fit and significance, then merged. +LLM reviews use GPT-6-Astra at `xhigh` reasoning effort through the Azure VM's Codex account pool, without paid OpenAI API requests or an API fallback. See [review operations](python/azure-review.md) for deployment and diagnostics. + Project PRs also receive an advisory Greptile review, configured in [`.greptile/`](.greptile/), for cross-file integration, reusable abstractions, completeness, maintainability, and measured cost. It supplements rather than replaces the independent LLM verdict. ### Getting started diff --git a/README.md b/README.md index 57220d3b36..45e749d6eb 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,8 @@ Semantic search is also available via the [API](https://search.octo.axiomatic-ai So far, projects have been added by hand: each is a suitable, permissively licensed (Apache-2.0 or MIT) Lean repository, bumped to the latest Lean and Mathlib, made to pass [CI](.github/workflows/lean_action_ci.yml) — it builds warning-free and clears Mathlib's linters, the style checker, and the repository quality gates (no `sorry`/`admit`, no axioms beyond `Classical.choice`/`propext`/`Quot.sound`, no `unsafe`/`partial`, file headers, size limits) — and an [LLM review](.github/REVIEW_RULES.md) of fit and significance, then merged. +LLM reviews use GPT-6-Astra at `xhigh` reasoning effort through the Azure VM's Codex account pool, without paid OpenAI API requests or an API fallback. See [review operations](python/azure-review.md) for deployment and diagnostics. + Project PRs also receive an advisory Greptile review, configured in [`.greptile/`](.greptile/), for cross-file integration, reusable abstractions, completeness, maintainability, and measured cost. It supplements rather than replaces the independent LLM verdict. ### Getting started diff --git a/python/README.md b/python/README.md index 2018eafa75..031ee46518 100644 --- a/python/README.md +++ b/python/README.md @@ -5,6 +5,9 @@ and decide which Lean projects belong in `lean-pool`. Managed with [uv](https://docs.astral.sh/uv/). +LLM pull request reviews run through GPT-6-Astra on the Azure VM's Codex account +pool. See [Azure review operations](azure-review.md) for deployment and diagnostics. + ## Setup ```bash diff --git a/python/azure-review.md b/python/azure-review.md new file mode 100644 index 0000000000..7ac5d95cb0 --- /dev/null +++ b/python/azure-review.md @@ -0,0 +1,74 @@ +# Azure Codex reviews + +The LLM review workflow uses `gpt-6-astra` at `xhigh` reasoning effort through +the authenticated Codex account pool on the `lean` Azure VM. It consumes that +pool's Codex quota, with no OpenAI API key and no fallback to paid API requests. +The existing review rubrics, CI prerequisite, verdict aggregation, and sticky +comments still apply. Comments identify the model, token counts, and quota billing. + +GitHub Actions continues to fetch the PR and post the comment. Only the review +instructions and contributor text cross SSH. The workflow checks out its trusted +base code and never executes PR-head code on the VM. The worker runs Codex with +ChatGPT authentication required, user configuration ignored, a read-only sandbox, +and shell, apps, plugins, browser, image, and agent tools disabled. Each request +uses a temporary directory on `/data`, removed when it finishes. The VM's existing +account dispatcher handles account selection and quota waits. + +The initial text budget is 180,000 estimated input tokens, leaving room in the +VM's advertised 272,000-token model context for instructions and reasoning. +The existing partial-review banner and restriction against approving an elided +project diff apply. Each worker call has a 100-minute timeout, including quota +waits; a timeout kills its dispatcher process group and fails the review. + +## Deployment + +The standalone worker is `lean_pool/codex_review.py`, installed as +`/data/lean-pool-review/worker.py` on `lean`. It uses only the Python standard +library and invokes `/home/vasil/.local/bin/codex-auto`. The worker file is owned +by root; `/data/lean-pool-review/jobs` is private to `vasil` for temporary requests. +To update it from the repository root: + +```bash +ssh lean 'sudo tee /data/lean-pool-review/worker.py >/dev/null' \ + < python/lean_pool/codex_review.py +``` + +The dedicated public key in `vasil`'s `authorized_keys` has these options: + +```text +restrict,command="/usr/bin/python3 /data/lean-pool-review/worker.py --worker" +``` + +It cannot open a shell, forward ports, or select another command. Model and +effort values are validated, and contributor text is passed through stdin. + +Repository secrets: + +- `REVIEW_AZURE_SSH_KEY`: the dedicated private SSH key, used only by this workflow. +- `REVIEW_AZURE_KNOWN_HOSTS`: the VM's pinned Ed25519 host key, obtained through + an already trusted connection. Never replace this with an unverified keyscan. + +Codex credentials stay on the VM. Do not copy or print its `auth.json` files. +Use `cx status` and `cx doctor` on the VM to diagnose authentication, quota, +and disk issues. Rotate the dedicated SSH key by adding a new restricted public +key, updating the repository secret, verifying a review, then removing the old +dedicated key. Do not remove other operator keys. + +## Local use and failures + +Configure an SSH alias using the restricted key and pinned host key, then run: + +```bash +cd python +REVIEW_SSH_HOST=lean-pool-review-azure PR_NUMBER=123 \ + uv run --group review python -m lean_pool.review +``` + +`REVIEW_SSH_CONFIG` optionally selects a separate SSH configuration file. +`REVIEW_BACKEND` defaults to `codex-azure`. Connection, authentication, timeout, +and malformed-output errors fail the job, without contacting the paid API. +After fixing the VM or its credentials, rerun the workflow or request `/review`. + +The legacy API backend remains available only when explicitly selected with +`REVIEW_BACKEND=openai`, `REVIEW_MODEL`, and `OPENAI_API_KEY`. The production +workflow neither selects it nor supplies that key. diff --git a/python/lean_pool/codex_review.py b/python/lean_pool/codex_review.py new file mode 100644 index 0000000000..849dd3c830 --- /dev/null +++ b/python/lean_pool/codex_review.py @@ -0,0 +1,266 @@ +"""Review transport to the Azure VM's authenticated Codex account pool. + +The SSH key is restricted to ``python3 worker.py --worker`` on the VM. +Only review text crosses SSH; neither GitHub nor OpenAI credentials do. +This module is also the standalone, standard-library-only remote worker. +""" + +from __future__ import annotations + +import json +import os +import signal +import subprocess +import sys +import tempfile +from pathlib import Path +from types import SimpleNamespace +from typing import Any + +MODEL = "gpt-6-astra" +TIER = "codex-azure" +TIMEOUT_SECONDS = 6000 +MAX_REQUEST_BYTES = 4_000_000 +WORKER_ROOT = Path("/data/lean-pool-review/jobs") +CODEX = "/home/vasil/.local/bin/codex-auto" + + +def request_completion( + model: str, messages: list[dict[str, str]], effort: str | None +) -> tuple[Any, str, str | None]: + """Call the forced-command SSH worker, with no API fallback.""" + host = os.environ.get("REVIEW_SSH_HOST") + if not host or host.startswith("-"): + raise RuntimeError("REVIEW_SSH_HOST must name the configured Azure SSH host") + request = json.dumps({"model": model, "messages": messages, "effort": effort}) + configuration = os.environ.get("REVIEW_SSH_CONFIG") + options = ["-F", configuration] if configuration else [] + process = subprocess.run( + [ + "ssh", + *options, + "-T", + "-o", + "BatchMode=yes", + "-o", + "StrictHostKeyChecking=yes", + "-o", + "ConnectTimeout=15", + "-o", + "ServerAliveInterval=30", + "-o", + "ServerAliveCountMax=3", + host, + "lean-pool-review", + ], + input=request, + text=True, + capture_output=True, + timeout=TIMEOUT_SECONDS + 60, + check=False, + ) + if process.returncode: + raise RuntimeError(f"Azure Codex review failed: {process.stderr[-2000:]}") + result = json.loads(process.stdout) + if result.get("model") != model or not isinstance(result.get("payload"), dict): + raise RuntimeError("Azure Codex worker returned an invalid review response") + usage = result.get("usage") + response = SimpleNamespace( + choices=[ + SimpleNamespace( + message=SimpleNamespace(content=json.dumps(result["payload"])) + ) + ], + model=result["model"], + usage=SimpleNamespace(**usage) if usage else None, + ) + return response, TIER, result["effort"] + + +def validate_request(request: dict) -> None: + """Accept only the pinned model and two text messages, never commands.""" + if request.get("model") != MODEL: + raise ValueError(f"The Azure reviewer only serves {MODEL}") + if request.get("effort") not in (None, "low", "medium", "high", "xhigh", "max"): + raise ValueError("Unsupported reasoning effort") + messages = request.get("messages") + if not isinstance(messages, list) or len(messages) != 2: + raise ValueError("Expected a system message and a user message") + for message, role in zip(messages, ("system", "user"), strict=True): + if message.get("role") != role or not isinstance(message.get("content"), str): + raise ValueError("Expected a system message and a user message") + + +def codex_command(request: dict, directory: Path) -> list[str]: + """Build a fixed Codex invocation with tools and API-key auth disabled.""" + command = [ + CODEX, + "exec", + "--ignore-user-config", + "--ephemeral", + "--skip-git-repo-check", + "--cd", + str(directory), + "--sandbox", + "read-only", + "--model", + MODEL, + "--json", + "--output-last-message", + str(directory / "answer.json"), + "--output-schema", + str(directory / "schema.json"), + "-c", + 'forced_login_method="chatgpt"', + "-c", + 'web_search="disabled"', + "-c", + "project_doc_max_bytes=0", + "-c", + "features.skip_host_skill_discovery=true", + "-c", + "base_instructions=" + + json.dumps( + request["messages"][0]["content"] + + "\nYou are a text-only reviewer. Do not use tools. Return the complete " + "requested review JSON encoded as the review_json string in the output." + ), + ] + for feature in ( + "shell_tool", + "unified_exec", + "view_image", + "apps", + "plugins", + "hooks", + "multi_agent", + "multi_agent_v2", + "skill_search", + "image_generation", + "computer_use", + "browser_use", + "browser_use_external", + "sleep_tool", + ): + command.extend(["--disable", feature]) + if request.get("effort"): + command.extend( + ["-c", "model_reasoning_effort=" + json.dumps(request["effort"])] + ) + return [*command, "-"] + + +def run_codex(request: dict, directory: Path) -> dict: + """Run one bounded review and kill its whole process group on timeout.""" + schema = { + "type": "object", + "properties": {"review_json": {"type": "string"}}, + "required": ["review_json"], + "additionalProperties": False, + } + (directory / "schema.json").write_text(json.dumps(schema)) + environment = { + key: value + for key, value in os.environ.items() + if key + not in ( + "OPENAI_API_KEY", + "OPENAI_BASE_URL", + "OPENAI_ORG_ID", + "OPENAI_PROJECT_ID", + "GH_TOKEN", + "GITHUB_TOKEN", + "CODEX_HOME", + ) + } + process = subprocess.Popen( + codex_command(request, directory), + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + env=environment, + start_new_session=True, + ) + try: + stdout, stderr = process.communicate( + request["messages"][1]["content"], timeout=TIMEOUT_SECONDS + ) + except subprocess.TimeoutExpired: + stop_codex(process) + raise RuntimeError("Azure Codex review timed out") from None + except BaseException: + stop_codex(process) + raise + if process.returncode: + raise RuntimeError(f"Codex exited {process.returncode}: {stderr[-2000:]}") + envelope = json.loads((directory / "answer.json").read_text()) + payload = json.loads(envelope["review_json"]) + if not isinstance(payload, dict) or not payload: + raise ValueError("Codex did not return a review object") + usage = extract_usage(stdout) + return { + "payload": payload, + "model": MODEL, + "effort": request["effort"], + "usage": usage, + } + + +def stop_codex(process: subprocess.Popen) -> None: + """Let the account dispatcher forward termination to its separate child group.""" + try: + os.killpg(process.pid, signal.SIGTERM) + except ProcessLookupError: + pass + try: + process.communicate(timeout=15) + except subprocess.TimeoutExpired: + os.killpg(process.pid, signal.SIGKILL) + process.communicate() + + +def interrupted(signum: int, frame: Any) -> None: + """Unwind the active worker when SSH disconnects or the job is cancelled.""" + raise InterruptedError(f"Review interrupted by signal {signum}") + + +def extract_usage(stdout: str) -> dict | None: + """Read token counts from the final successful Codex turn event.""" + usage = None + for line in stdout.splitlines(): + try: + event = json.loads(line) + except json.JSONDecodeError: + continue # The account dispatcher also emits plain-text status. + if event.get("type") == "turn.completed" and event.get("usage"): + usage = { + "prompt_tokens": event["usage"].get("input_tokens", 0), + "completion_tokens": event["usage"].get("output_tokens", 0), + } + return usage + + +def main() -> int: + """Serve one SSH request using an isolated temporary directory on /data.""" + signal.signal(signal.SIGTERM, interrupted) + signal.signal(signal.SIGHUP, interrupted) + try: + raw = sys.stdin.buffer.read(MAX_REQUEST_BYTES + 1) + if len(raw) > MAX_REQUEST_BYTES: + raise ValueError("Review request exceeds the worker size limit") + request = json.loads(raw) + validate_request(request) + with tempfile.TemporaryDirectory(prefix="review-", dir=WORKER_ROOT) as name: + response = run_codex(request, Path(name)) + print(json.dumps(response)) + return 0 + except Exception as error: + print(f"Azure Codex review failed: {error}", file=sys.stderr) + return 1 + + +if __name__ == "__main__": + if sys.argv[1:] != ["--worker"]: + sys.exit("This entry point requires --worker") + sys.exit(main()) diff --git a/python/lean_pool/review.py b/python/lean_pool/review.py index b5825e90c8..18931bc4be 100644 --- a/python/lean_pool/review.py +++ b/python/lean_pool/review.py @@ -34,11 +34,10 @@ contributor, so it is framed to the model as evidence to verify rather than instruction to follow (:data:`UNTRUSTED_INPUT_RULE`). -The reviewer prefers OpenAI's ``flex`` tier (cheaper, slower, occasionally -unavailable). When flex returns 429 Resource Unavailable, the request is -retried with ``service_tier="auto"`` (standard pricing). The rendered -comment shows which tier was actually used and prices the request at that -tier's rate. +Reviews default to GPT-6-Astra through the Azure VM's authenticated Codex +account pool over restricted SSH. Failures never fall back to API credits. +The optional, explicitly selected ``openai`` backend retains flex/standard +API billing for local use; the production workflow does not provide an API key. Very large PRs (machine-generated certificates or case data) can exceed the model's input-token limit — PR #278's 3.1M-character diff was @@ -56,7 +55,9 @@ ``DEFAULT_MODEL`` or when OpenAI changes pricing. Environment variables: - OPENAI_API_KEY: OpenAI credentials (required). + REVIEW_BACKEND: ``codex-azure`` (default) or explicit ``openai``. + REVIEW_SSH_HOST: SSH host alias for the restricted Azure worker. + OPENAI_API_KEY: Required only for the explicit ``openai`` backend. PR_NUMBER: Pull request number to review (required). GH_TOKEN: Token for the GitHub CLI (required in CI). GITHUB_REPOSITORY: @@ -87,22 +88,16 @@ from openai import APIStatusError, BadRequestError, OpenAI, RateLimitError -from lean_pool import challenge, prior_art +from lean_pool import challenge, codex_review, prior_art REPO_ROOT = Path(__file__).resolve().parents[2] PROJECT_RULES_PATH = REPO_ROOT / ".github" / "REVIEW_RULES.md" REFACTOR_RULES_PATH = REPO_ROOT / ".github" / "REFACTOR_REVIEW_RULES.md" CHALLENGE_RULES_PATH = REPO_ROOT / ".github" / "CHALLENGE_REVIEW_RULES.md" SOLUTION_RULES_PATH = REPO_ROOT / ".github" / "SOLUTION_REVIEW_RULES.md" -# The `gpt-5.6` alias routes to the newest gpt-5.6-sol snapshot (OpenAI's -# flagship reasoning model), so snapshot upgrades arrive automatically. -# OpenAI publishes no cross-family rolling alias — when a new family ships -# (gpt-5.7, gpt-6), this line and PRICING_PER_M still need a bump. -DEFAULT_MODEL = "gpt-5.6" -# Reasoning effort sent with every review request. `xhigh` is the deep -# end of gpt-5.6's range (none/low/medium/high/xhigh/max); reasoning -# tokens are billed as output tokens, so this is the main cost dial — -# lower it via the REVIEW_EFFORT env var if review costs run hot. +# Reviews default to the authenticated Codex account pool on the Azure VM. +DEFAULT_MODEL = "gpt-6-astra" +# Preserve the existing review depth. DEFAULT_REASONING_EFFORT = "xhigh" # Flex tier requests can take longer than the default 10-minute timeout, # and xhigh reasoning stretches generation further. @@ -1068,7 +1063,7 @@ class ReviewResult: Attributes: payload: Parsed JSON review from the model. usage: OpenAI ``CompletionUsage`` object, or ``None``. - tier: ``"flex"`` or ``"standard"`` — the tier that served it. + tier: ``"codex-azure"``, ``"flex"``, or ``"standard"``. truncation: Diff elision applied, ``None`` for a full-diff review. model: Model that actually served the request — the resolved snapshot when the request went through an alias. @@ -1099,8 +1094,8 @@ def request_review( tokens (:func:`fit_diff_to_budget`). Token estimation is approximate, so if the API still rejects the input as too large, the diff budget is halved and the request refitted and retried instead of failing the - review. Tries the ``flex`` service tier first; if OpenAI returns 429 - Resource Unavailable, retries with ``service_tier="auto"`` (standard). + review. The default Azure Codex backend fails closed. Only the explicit + OpenAI backend uses the flex-to-standard API tier fallback. Args: model: Model name or alias to request. @@ -1115,7 +1110,10 @@ def request_review( Returns: The :class:`ReviewResult` for the request that succeeded. """ - client = OpenAI(timeout=REQUEST_TIMEOUT_SECONDS) + backend = os.environ.get("REVIEW_BACKEND", "codex-azure") + if backend not in ("codex-azure", "openai"): + raise ValueError(f"Unknown REVIEW_BACKEND: {backend}") + client = OpenAI(timeout=REQUEST_TIMEOUT_SECONDS) if backend == "openai" else None fence = secrets.token_hex(8) system_prompt = "\n".join( [ @@ -1133,7 +1131,11 @@ def request_review( + estimate_tokens(system_prompt) + estimate_tokens(prior_art_section or "") ) - diff_budget_tokens = MAX_INPUT_TOKENS - scaffold_tokens + # Leave room for output and Codex instructions in Astra's context window. + input_budget = ( + min(MAX_INPUT_TOKENS, 180_000) if backend == "codex-azure" else MAX_INPUT_TOKENS + ) + diff_budget_tokens = input_budget - scaffold_tokens for attempt in range(REVIEW_FIT_ATTEMPTS): fitted_diff, truncation = fit_diff_to_budget(diff, diff_budget_tokens) if truncation is not None: @@ -1152,9 +1154,14 @@ def request_review( {"role": "user", "content": user_content}, ] try: - response, tier, effort_used = _completion_with_tier_fallback( - client, model, messages, effort - ) + if backend == "codex-azure": + response, tier, effort_used = codex_review.request_completion( + model, messages, effort + ) + else: + response, tier, effort_used = _completion_with_tier_fallback( + client, model, messages, effort + ) except BadRequestError as error: if not _is_token_overflow(error) or attempt == REVIEW_FIT_ATTEMPTS - 1: raise @@ -1269,13 +1276,20 @@ def render_usage(usage: Any, model: str, tier: str, effort: str | None = None) - pair is not listed there. """ if usage is None: - return "" + return ( + "**Billing:** Codex account quota on Azure (no API credits)" + if tier == codex_review.TIER + else "" + ) in_tok = getattr(usage, "prompt_tokens", 0) or 0 out_tok = getattr(usage, "completion_tokens", 0) or 0 parts = [f"**Tokens:** {in_tok:,} in / {out_tok:,} out", f"**Tier:** `{tier}`"] if effort: parts.append(f"**Effort:** `{effort}`") + if tier == codex_review.TIER: + parts.append("**Billing:** Codex account quota on Azure (no API credits)") + return " · ".join(parts) rates = pricing_rates(model, tier, in_tok) if rates is not None: in_price, out_price = rates @@ -1495,7 +1509,9 @@ def _render_rubric_usage(outcomes: list[RubricOutcome], effort: str | None) -> s ] if effort: parts.append(f"**Effort:** `{effort}`") - if cost > 0 or not unpriced: + if all(o.result.tier == codex_review.TIER for o in outcomes): + parts.append("**Billing:** Codex account quota on Azure (no API credits)") + elif cost > 0 or not unpriced: cost_cell = f"**Cost:** ${cost:.4f}" if unpriced: cost_cell += " (partial — some calls unpriced)" @@ -1865,9 +1881,6 @@ def main() -> int: if not pr_number: print("PR_NUMBER not set", file=sys.stderr) return 2 - if not os.environ.get("OPENAI_API_KEY"): - print("OPENAI_API_KEY not set", file=sys.stderr) - return 2 model = os.environ.get("REVIEW_MODEL", DEFAULT_MODEL) effort: str | None = os.environ.get("REVIEW_EFFORT", DEFAULT_REASONING_EFFORT) diff --git a/python/tests/test_codex_review.py b/python/tests/test_codex_review.py new file mode 100644 index 0000000000..ea0a516d44 --- /dev/null +++ b/python/tests/test_codex_review.py @@ -0,0 +1,198 @@ +"""Azure review routing, credential isolation, failures, and quota reporting.""" + +import json +import subprocess +from types import SimpleNamespace + +import pytest + +from lean_pool import codex_review, review + + +def _request(): + return { + "model": "gpt-6-astra", + "effort": "xhigh", + "messages": [ + {"role": "system", "content": "Review the contribution."}, + {"role": "user", "content": "Contributor text; $(do-not-execute)"}, + ], + } + + +def test_default_routes_over_ssh_without_instantiating_openai(monkeypatch): + """Even an ambient personal API key cannot divert the default backend.""" + monkeypatch.delenv("REVIEW_BACKEND", raising=False) + monkeypatch.setenv("OPENAI_API_KEY", "must-not-be-used") + monkeypatch.setenv("REVIEW_SSH_HOST", "review-azure") + + def no_openai(**kwargs): + pytest.fail("The default backend must never instantiate the paid API client") + + def ssh(command, **kwargs): + assert command[0] == "ssh" + assert "StrictHostKeyChecking=yes" in command + assert command[-2:] == ["review-azure", "lean-pool-review"] + request = json.loads(kwargs["input"]) + assert request["model"] == "gpt-6-astra" + assert request["effort"] == "xhigh" + assert "Trust boundary" in request["messages"][0]["content"] + assert "$(do-not-execute)" in request["messages"][1]["content"] + assert "must-not-be-used" not in kwargs["input"] + return SimpleNamespace( + returncode=0, + stdout=json.dumps( + { + "model": "gpt-6-astra", + "effort": "xhigh", + "payload": {"summary": "ok", "verdict": "approve"}, + "usage": {"prompt_tokens": 20, "completion_tokens": 10}, + } + ), + ) + + monkeypatch.setattr(review, "OpenAI", no_openai) + monkeypatch.setattr(codex_review.subprocess, "run", ssh) + result = review.request_review( + review.DEFAULT_MODEL, "rules", "$(do-not-execute)", "Review.", "xhigh" + ) + assert result.payload["verdict"] == "approve" + assert result.tier == "codex-azure" + assert result.usage.prompt_tokens == 20 + footer = review.render_usage(result.usage, result.model, result.tier, result.effort) + assert "no API credits" in footer + assert "$" not in footer + + +@pytest.mark.parametrize( + "failure", ["disconnect", "timeout", "bad-json", "wrong-model"] +) +def test_azure_failure_never_falls_back(monkeypatch, failure): + """Transport and model failures stay failures even with API credentials.""" + monkeypatch.delenv("REVIEW_BACKEND", raising=False) + monkeypatch.setenv("REVIEW_SSH_HOST", "review-azure") + monkeypatch.setenv("OPENAI_API_KEY", "must-not-be-used") + + def no_openai(**kwargs): + pytest.fail("API fallback is forbidden") + + def fail(*args, **kwargs): + if failure == "timeout": + raise subprocess.TimeoutExpired("ssh", 1) + if failure == "disconnect": + return SimpleNamespace(returncode=255, stderr="unreachable") + stdout = "invalid" if failure == "bad-json" else '{"model": "other"}' + return SimpleNamespace(returncode=0, stdout=stdout) + + monkeypatch.setattr(review, "OpenAI", no_openai) + monkeypatch.setattr(codex_review.subprocess, "run", fail) + with pytest.raises((RuntimeError, ValueError, subprocess.TimeoutExpired)): + review.request_review(review.DEFAULT_MODEL, "rules", "diff", "Review.") + + +def test_unknown_backend_is_rejected(monkeypatch): + """A configuration typo must not choose a billable backend.""" + monkeypatch.setenv("REVIEW_BACKEND", "codex-azur") + with pytest.raises(ValueError, match="Unknown REVIEW_BACKEND"): + review.request_review(review.DEFAULT_MODEL, "rules", "diff", "Review.") + + +@pytest.mark.parametrize( + "field,value", + [ + ("model", "gpt-5.6"), + ("effort", "xhigh; echo unsafe"), + ("messages", [{"role": "user", "content": "only one"}]), + ], +) +def test_worker_rejects_invalid_requests(field, value): + """The SSH endpoint cannot select a different model or execute commands.""" + request = _request() + request[field] = value + with pytest.raises(ValueError): + codex_review.validate_request(request) + + +def test_worker_isolates_credentials_and_returns_structured_review( + monkeypatch, tmp_path +): + """Only ChatGPT authentication and text input reach the isolated process.""" + monkeypatch.setenv("OPENAI_API_KEY", "must-not-be-used") + monkeypatch.setenv("GH_TOKEN", "must-not-be-forwarded") + + class Process: + returncode = 0 + + def communicate(self, text, timeout): + assert text == _request()["messages"][1]["content"] + (tmp_path / "answer.json").write_text( + json.dumps( + {"review_json": json.dumps({"summary": "ok", "verdict": "approve"})} + ) + ) + return ( + 'dispatcher status\n{"type":"turn.completed",' + '"usage":{"input_tokens":30,"output_tokens":15}}', + "", + ) + + def spawn(command, **kwargs): + assert command[0] == codex_review.CODEX + assert 'forced_login_method="chatgpt"' in command + assert "--ignore-user-config" in command + assert "--ephemeral" in command + assert command[command.index("--sandbox") + 1] == "read-only" + assert "OPENAI_API_KEY" not in kwargs["env"] + assert "GH_TOKEN" not in kwargs["env"] + assert kwargs["start_new_session"] + assert command[-1] == "-" + assert _request()["messages"][1]["content"] not in command + disabled = { + command[i + 1] for i, flag in enumerate(command) if flag == "--disable" + } + assert {"shell_tool", "apps", "plugins", "multi_agent"} <= disabled + return Process() + + monkeypatch.setattr(codex_review.subprocess, "Popen", spawn) + response = codex_review.run_codex(_request(), tmp_path) + assert response["payload"]["verdict"] == "approve" + assert response["model"] == "gpt-6-astra" + assert response["usage"] == {"prompt_tokens": 30, "completion_tokens": 15} + + +def test_worker_timeout_kills_process_group(monkeypatch, tmp_path): + """Timeouts also stop the dispatcher and its model subprocesses.""" + killed = [] + + class Process: + pid = 1234 + calls = 0 + + def communicate(self, *args, **kwargs): + self.calls += 1 + if self.calls == 1: + raise subprocess.TimeoutExpired("codex", 1) + return "", "" + + monkeypatch.setattr(codex_review.subprocess, "Popen", lambda *a, **k: Process()) + monkeypatch.setattr(codex_review.os, "killpg", lambda *args: killed.append(args)) + with pytest.raises(RuntimeError, match="timed out"): + codex_review.run_codex(_request(), tmp_path) + assert killed == [(1234, codex_review.signal.SIGTERM)] + + +def test_azure_rubric_footer_reports_quota(): + """Five-rubric reviews report quota billing without inventing a dollar cost.""" + result = review.ReviewResult( + {}, + SimpleNamespace(prompt_tokens=20, completion_tokens=10), + codex_review.TIER, + None, + codex_review.MODEL, + "xhigh", + ) + outcomes = [SimpleNamespace(result=result)] * 5 + footer = review._render_rubric_usage(outcomes, "xhigh") + assert "100 in / 50 out" in footer + assert "no API credits" in footer + assert "$" not in footer diff --git a/python/tests/test_review.py b/python/tests/test_review.py index fabc6510d9..cb5910bb09 100644 --- a/python/tests/test_review.py +++ b/python/tests/test_review.py @@ -8,6 +8,13 @@ # The stub registered by tests/conftest.py; its exception classes carry # the status_code attributes the review module's error handling inspects. import openai +import pytest + + +@pytest.fixture(autouse=True) +def explicit_api_backend(monkeypatch): + """Keep legacy API tests explicit; Azure routing has its own test module.""" + monkeypatch.setenv("REVIEW_BACKEND", "openai") def _file_patch(path: str, body_lines: list[str]) -> str: