mirror of
https://github.com/hanzoai/datastore.git
synced 2026-10-30 14:48:37 +00:00
406 lines
18 KiB
Python
406 lines
18 KiB
Python
"""
|
|
AI-based automated PR code review job.
|
|
|
|
Two backends are supported, selected by flag:
|
|
|
|
--codex OpenAI Codex CLI (auth: OPENAI_API_KEY from `/ci/llm/openai_api_key`)
|
|
--copilot GitHub Copilot CLI (auth: gh robot token from `/ci/robot-ch-test-poll-copilot`)
|
|
|
|
Both agents shell out to `gh` to post inline review comments, so the gh CLI is
|
|
always authenticated against a temporary `GH_CONFIG_DIR` with the robot token
|
|
regardless of which backend runs the review itself.
|
|
|
|
The agent writes a free-form Markdown summary to `REVIEW_FILE`. The job script
|
|
then posts that summary via `python3 -m ci.praktika.gh post-or-update --tag review`
|
|
so the top-level comment is always authored by the pre-authenticated app, not
|
|
the agent's robot account.
|
|
|
|
Each agent occasionally hits transient GitHub authorization errors mid-run
|
|
("Authorization error, you may need to run /login"). The whole `gh auth` +
|
|
agent sequence is wrapped in a retry loop with exponential backoff so a single
|
|
transient API failure does not fail the Code Review job. Authorization-style
|
|
failures do not always surface as a Python exception — they show up as a
|
|
non-zero exit code and a missing/empty review file — so both are checked here.
|
|
"""
|
|
|
|
import os
|
|
import random
|
|
import shlex
|
|
import subprocess
|
|
import sys
|
|
import tempfile
|
|
import time
|
|
import traceback
|
|
import urllib.parse
|
|
|
|
from ci.praktika import Secret
|
|
from ci.praktika.info import Info
|
|
from ci.praktika.result import Result
|
|
|
|
REVIEW_FILE = "./ci/tmp/copilot_review.md"
|
|
GH_PREFIX = "env -u GH_CONFIG_DIR"
|
|
|
|
# Number of attempts at the full gh-auth + agent run. The agents fetch
|
|
# auth state and make GitHub / model-provider API calls during execution;
|
|
# either layer can hit a transient 5xx / authorization error that no
|
|
# single inner subprocess controls. Re-authing and retrying the whole
|
|
# sequence is the only reliable way to recover.
|
|
MAX_ATTEMPTS = 3
|
|
|
|
# Robot gh tokens. Used by both backends — Copilot authenticates against
|
|
# GitHub with one of these directly; Codex needs gh authed so the agent's
|
|
# shelled-out `gh` calls (for posting inline comments) succeed. Each
|
|
# attempt picks one in a randomised rotation so a single robot's rate
|
|
# limit or token issue does not fail every attempt.
|
|
ROBOT_NAMES = [
|
|
"/ci/robot-ch-test-poll-copilot",
|
|
"/ci/robot-ch-test-poll-1-copilot",
|
|
]
|
|
|
|
# OpenAI API key for the Codex CLI, written into `$CODEX_HOME/auth.json`
|
|
# via `codex login --with-api-key`.
|
|
OPENAI_KEY_SECRET = "/ci/llm/openai_api_key"
|
|
|
|
|
|
def _join_prompt(*sections):
|
|
return "\n\n".join(section.rstrip() for section in sections if section).rstrip() + "\n"
|
|
|
|
|
|
def _repo_from_pr_url(pr_url):
|
|
path_parts = urllib.parse.urlparse(pr_url).path.strip("/").split("/")
|
|
if len(path_parts) >= 4 and path_parts[2] == "pull":
|
|
return f"{path_parts[0]}/{path_parts[1]}"
|
|
return ""
|
|
|
|
|
|
def _pr_repository(info):
|
|
return _repo_from_pr_url(info.pr_url) or info.repo_name
|
|
|
|
|
|
def _pre_review_instructions():
|
|
return """\
|
|
Review instructions:
|
|
- Follow the Review Instructions in .claude/skills/review/SKILL.md.
|
|
- Repo is checked out at PR head.
|
|
- Post findings as individual inline review comments on specific lines.
|
|
"""
|
|
|
|
|
|
def _review_target(info):
|
|
repo_name = _pr_repository(info)
|
|
return f"""\
|
|
Review target:
|
|
- PR URL: {info.pr_url}
|
|
- PR repository: `{repo_name}`
|
|
- Always derive the PR repository from the PR URL or the CI event. For ClickHouse reviews this will be either
|
|
`ClickHouse/ClickHouse` or `ClickHouse/ClickHouse-private`. Do not infer the review repository from
|
|
the local checkout remote, because local checkouts may point to a fork.
|
|
"""
|
|
|
|
|
|
def _pre_review_tools(pr_number, repo_name):
|
|
return f"""\
|
|
Tools:
|
|
- Prefix every `gh` call with `{GH_PREFIX}`.
|
|
- Pass `--repo {repo_name}` exactly as shown in each command below. For `gh` subcommands not shown
|
|
here, only add `--repo` if the command documents it. Do NOT add `--repo` to
|
|
`resolve-pr-review-thread` / `unresolve-pr-review-thread`: they take only `--thread-id` (a globally
|
|
unique GraphQL node id that already identifies the repo) and reject `--repo` with
|
|
`error: unrecognized arguments`.
|
|
- Post ALL new inline findings as ONE batched review, not as separate comments. Write each finding's
|
|
body to its own Markdown file, then list the findings in a single JSON file: an array of objects
|
|
`{{"path": "<file>", "line": <N>, "side": "RIGHT", "body_file": "<body.md>"}}` (for a multi-line
|
|
range add `"start_line": <N>` and `"start_side": "RIGHT"`; use `"side": "LEFT"` for a deleted line).
|
|
Submit the whole batch with a single call:
|
|
`{GH_PREFIX} python3 -m ci.praktika.gh post-pr-review --comments-file <comments.json> --commit <sha> --repo {repo_name}`.
|
|
The wrapper reads each `body_file` and assembles the review payload, so multi-line Markdown is
|
|
uploaded correctly. Call it exactly once per run, and only if there is at least one new inline finding.
|
|
- Do NOT post new findings as individual comments (`post-pr-line-comment` without `--reply-to`); that
|
|
is reserved for thread replies (see below). Do NOT call `gh api .../pulls/.../comments`,
|
|
`gh api .../pulls/.../reviews`, or `gh pr review` directly: the wrappers exist to avoid the
|
|
`-f`/`-F` and JSON-escaping footguns that have posted literal `@<file>` / `\\n` bodies before.
|
|
- Fetch inline review threads with:
|
|
`{GH_PREFIX} python3 -m ci.praktika.gh list-pr-review-threads --pr {pr_number} --repo {repo_name}`.
|
|
The command returns JSON; each thread carries its node `id`, `isResolved`, `resolvedBy.login`
|
|
(the user who most recently resolved it, or `null`), `path`, `line`, and `comments.nodes` with
|
|
author, body, `databaseId`, and `createdAt`.
|
|
- Fetch top-level conversation with:
|
|
`{GH_PREFIX} gh api '/repos/{repo_name}/issues/{pr_number}/comments' --paginate`.
|
|
- Reply on an existing thread with:
|
|
`{GH_PREFIX} python3 -m ci.praktika.gh post-pr-line-comment --file <body.md> --reply-to <parent_databaseId> --repo {repo_name}`.
|
|
Use the `databaseId` of the first comment on the thread as `<parent_databaseId>`, and omit
|
|
`--commit`, `--path`, and `--line`.
|
|
- Resolve or unresolve bot-authored review threads with:
|
|
`{GH_PREFIX} python3 -m ci.praktika.gh resolve-pr-review-thread --thread-id <thread_node_id>`
|
|
`{GH_PREFIX} python3 -m ci.praktika.gh unresolve-pr-review-thread --thread-id <thread_node_id>`.
|
|
"""
|
|
|
|
|
|
def _pre_review_procedure(pr_url):
|
|
return f"""\
|
|
Procedure:
|
|
1. In GitHub discussions, "you" are the `clickhouse-gh` GitHub App. Its identity appears in two
|
|
forms depending on which GraphQL field returns it: `clickhouse-gh` in `author.login` (comments,
|
|
reviews) and `clickhouse-gh[bot]` in `resolvedBy.login` (review threads). Treat both as you.
|
|
2. Fetch all prior discussion on this PR before reviewing.
|
|
3. Provide a thorough review of {pr_url}. Read the current code and PR diff, not only the discussion.
|
|
4. Read every reply on every thread. Treat each reply as a deliberate engineering decision by the
|
|
author. An explanation that holds up, a pointer to a fixing commit, or a tradeoff you agree with
|
|
means drop the point. A dismissal ("no", "won't fix", "by design", "pathological", "wontfix",
|
|
"agree to disagree", a silent thread resolution) is also a deliberate decision -- accept it. If
|
|
you still believe the issue is real after the author's reply, see step 5 for what to do with it.
|
|
5. Apply the same judgment to your own prior summary: drop findings that have been addressed, keep
|
|
or sharpen the rest. Verify by reading the current code, not by trusting the author's reply.
|
|
Findings the author dismissed but you still consider real STAY in the summary's `Findings`
|
|
section, marked `[dismissed by author -- <thread URL>]` with a one-line note on why you still
|
|
consider it real. Do NOT migrate them back to the inline thread.
|
|
6. On existing threads, post a new comment only in these two cases:
|
|
(a) The author asked you a direct, answerable question (e.g. "what would the fix look like?",
|
|
"do you have a repro?"). Answer it once and stop -- do not restate the original finding.
|
|
(b) The author explicitly claimed the issue is fixed ("fixed in <commit>", "this is fixed now")
|
|
but the current code shows the original issue is still present. Reply once pointing to the
|
|
`file:line` that disproves the claim. Distinguish this from a dismissal: "won't fix",
|
|
"by design", "pathological", "no", a silent thread resolution are NOT fixed claims.
|
|
In every other case, do not reply on the thread.
|
|
7. Resolve and re-open only threads you created yourself: that means threads whose first entry in
|
|
`comments.nodes` has `author.login == "clickhouse-gh"`. Resolve such a thread when the issue no
|
|
longer holds in the current code. Re-open such a thread only when it is resolved AND the issue
|
|
is still present AND either:
|
|
(a) the thread's `resolvedBy.login` is `"clickhouse-gh[bot]"` (whether you resolved it
|
|
prematurely or a later commit reintroduced the issue). Re-open silently, no reply needed.
|
|
(b) case 6(b) fires -- re-open AND post the 6(b) reply in the same run.
|
|
Never resolve or unresolve threads whose first comment was authored by anyone else.
|
|
8. For genuinely new issues that do not already have a thread, collect them all into the single
|
|
batched review described under Tools (one inline comment per issue, on the relevant changed line).
|
|
For architectural issues that do not map cleanly to one line, attach the comment to the most
|
|
relevant change in the diff. Submit the whole batch with one `post-pr-review` call; never post new
|
|
findings one at a time.
|
|
9. Do NOT post inline comments for issues that dedicated CI jobs already catch and report: build /
|
|
compilation failures (missing headers, undeclared symbols, type errors, link errors) and style
|
|
check failures (formatting, linters, `check_cpp.sh` / `check_style.sh` output). These are not
|
|
blockers in this review context: the build and Style Check jobs surface them with full toolchain
|
|
output, so a review comment is pure noise. If you want to mention them, include them only as
|
|
`💡 Nits` in the summary file, never as inline comments or as `❌ Blockers` / `⚠️ Majors`.
|
|
"""
|
|
|
|
|
|
def _pre_review_output():
|
|
return f"""\
|
|
Output:
|
|
Write a self-contained summary of ALL findings, regardless of previous summaries, as plain Markdown
|
|
to {REVIEW_FILE} using the REQUESTED OUTPUT FORMAT from .claude/skills/review/SKILL.md:
|
|
start with `---
|
|
#### AI Review`, then use ##### for section headers.
|
|
Do NOT post the summary yourself: the job script will post it after you finish.
|
|
"""
|
|
|
|
|
|
def _pre_review_prompt(info):
|
|
repo_name = _pr_repository(info)
|
|
return _join_prompt(
|
|
_pre_review_instructions(),
|
|
_review_target(info),
|
|
_pre_review_tools(info.pr_number, repo_name),
|
|
_pre_review_procedure(info.pr_url),
|
|
_pre_review_output(),
|
|
)
|
|
|
|
|
|
def _reauth_gh():
|
|
"""Force re-auth of the main gh context (outside any GH_CONFIG_DIR override).
|
|
|
|
Called at job start regardless of current auth status, so the token is
|
|
always fresh when the agent invokes `env -u GH_CONFIG_DIR`.
|
|
"""
|
|
from ci.praktika.gh_auth import GHAuth
|
|
|
|
GHAuth.auth_from_settings()
|
|
|
|
|
|
def _post_review():
|
|
"""Post REVIEW_FILE as a PR comment. Raises on failure, failing the job."""
|
|
subprocess.run(
|
|
[
|
|
sys.executable, "-m", "ci.praktika.gh",
|
|
"post-or-update", "--tag", "review", "--file", REVIEW_FILE,
|
|
],
|
|
check=True,
|
|
)
|
|
|
|
|
|
def _drop_stale_review_file():
|
|
"""Remove any leftover REVIEW_FILE so a prior attempt's artifact cannot
|
|
be mistaken for the result of a later failed attempt."""
|
|
if os.path.exists(REVIEW_FILE):
|
|
try:
|
|
os.unlink(REVIEW_FILE)
|
|
except OSError as e:
|
|
print(f"WARNING: Failed to remove stale {REVIEW_FILE}: {e}")
|
|
|
|
|
|
def _gh_auth_with_robot_token(gh_config_dir, robot_name):
|
|
"""Authenticate gh CLI in a scoped GH_CONFIG_DIR using the given robot token."""
|
|
print(f"Using robot: {robot_name}")
|
|
token = Secret.Config(
|
|
name=robot_name, type=Secret.Type.AWS_SSM_PARAMETER
|
|
).get_value()
|
|
subprocess.run(
|
|
["gh", "auth", "login", "--with-token"],
|
|
input=token, text=True, check=True,
|
|
env={**os.environ, "GH_CONFIG_DIR": gh_config_dir},
|
|
)
|
|
|
|
|
|
def _run_copilot_once(prompt, robot_name):
|
|
"""Run a single attempt of `gh auth login` + `copilot` for one robot."""
|
|
_drop_stale_review_file()
|
|
with tempfile.TemporaryDirectory() as gh_config_dir:
|
|
_gh_auth_with_robot_token(gh_config_dir, robot_name)
|
|
return Result.from_commands_run(
|
|
name="copilot review",
|
|
# --allow-all: enable all permissions; --allow-all-tools alone hits
|
|
# a CLI bug where compound shell commands are denied and the gate
|
|
# then tries to escalate to a human (github/copilot-cli#176, #2971)
|
|
# --no-ask-user: disable ask_user so the agent cannot try to prompt
|
|
# for permission in a non-interactive session
|
|
# --add-dir .: restrict file access to repo root (default, but explicit)
|
|
# </dev/null: ensure stdin is definitively non-interactive
|
|
command=f"GH_CONFIG_DIR={shlex.quote(gh_config_dir)} "
|
|
f"copilot -p {shlex.quote(prompt)} --allow-all --no-ask-user "
|
|
f"--add-dir . --model gpt-5.4 --effort xhigh < /dev/null",
|
|
with_info=True,
|
|
)
|
|
|
|
|
|
def _run_codex_once(prompt, robot_name):
|
|
"""Run a single attempt of `gh auth login` + `codex login` + `codex exec`.
|
|
|
|
Codex stores credentials in `$CODEX_HOME/auth.json` and does NOT consult
|
|
`OPENAI_API_KEY` directly when invoked — you have to run
|
|
`codex login --with-api-key` first, which reads the key from stdin and
|
|
writes it into `auth.json`. `CODEX_HOME` is scoped to a per-attempt
|
|
temporary directory under `./ci/tmp` (not `/tmp`, which codex refuses
|
|
to use for helper binaries) so the API key never lands on global runner
|
|
state.
|
|
"""
|
|
_drop_stale_review_file()
|
|
with tempfile.TemporaryDirectory() as gh_config_dir, \
|
|
tempfile.TemporaryDirectory(dir="./ci/tmp") as codex_home:
|
|
_gh_auth_with_robot_token(gh_config_dir, robot_name)
|
|
|
|
openai_key = Secret.Config(
|
|
name=OPENAI_KEY_SECRET, type=Secret.Type.AWS_SSM_PARAMETER
|
|
).get_value()
|
|
subprocess.run(
|
|
["codex", "login", "--with-api-key"],
|
|
input=openai_key, text=True, check=True,
|
|
env={**os.environ, "CODEX_HOME": codex_home},
|
|
)
|
|
|
|
return Result.from_commands_run(
|
|
name="codex review",
|
|
# -m gpt-5.4: same model the Copilot CLI uses, so
|
|
# review quality stays comparable across backends.
|
|
# -s workspace-write: writable workspace + /tmp + CODEX_HOME,
|
|
# read-only elsewhere; sufficient for review output and
|
|
# the gh CLI's /tmp config dir.
|
|
# sandbox_workspace_write.network_access=true: the agent
|
|
# shells out to `gh` to post inline comments, which needs
|
|
# network.
|
|
# approval_policy=never: codex `exec` is non-interactive,
|
|
# but the approval policy still applies; "never" lets the
|
|
# agent execute without blocking on an approval request.
|
|
# --color never: no ANSI codes in the job log.
|
|
command=f"CODEX_HOME={shlex.quote(codex_home)} "
|
|
f"GH_CONFIG_DIR={shlex.quote(gh_config_dir)} "
|
|
f"codex exec "
|
|
f"-m gpt-5.4 -c 'model_reasoning_effort=xhigh' "
|
|
f"-s workspace-write "
|
|
f"-c sandbox_workspace_write.network_access=true "
|
|
f"-c approval_policy=never "
|
|
f"--color never "
|
|
f"{shlex.quote(prompt)}",
|
|
with_info=True,
|
|
)
|
|
|
|
|
|
def _run(prompt, run_once, agent_name):
|
|
"""Run the chosen agent with retries, then post the review comment.
|
|
|
|
Each attempt re-fetches secrets, re-auths the temporary `GH_CONFIG_DIR`,
|
|
and re-runs the agent. The attempt counts as success only if the
|
|
subprocess exits 0 AND `REVIEW_FILE` was written with non-empty content.
|
|
"""
|
|
last_error = None
|
|
robots = ROBOT_NAMES.copy()
|
|
random.shuffle(robots)
|
|
for attempt in range(1, MAX_ATTEMPTS + 1):
|
|
robot_name = robots[(attempt - 1) % len(robots)]
|
|
try:
|
|
result = run_once(prompt, robot_name)
|
|
if not result.is_ok():
|
|
last_error = (
|
|
f"{agent_name} subprocess exited with non-OK status [{result.status}]"
|
|
)
|
|
print(f"WARNING: {agent_name} attempt {attempt}/{MAX_ATTEMPTS} failed: {last_error}")
|
|
elif not os.path.exists(REVIEW_FILE):
|
|
last_error = f"{agent_name} did not write {REVIEW_FILE}"
|
|
print(f"WARNING: {agent_name} attempt {attempt}/{MAX_ATTEMPTS} failed: {last_error}")
|
|
elif os.path.getsize(REVIEW_FILE) == 0:
|
|
last_error = f"{REVIEW_FILE} is empty"
|
|
print(f"WARNING: {agent_name} attempt {attempt}/{MAX_ATTEMPTS} failed: {last_error}")
|
|
else:
|
|
last_error = None
|
|
break
|
|
except Exception as e: # noqa: BLE001 — broad catch: any exception is retryable here
|
|
last_error = f"{type(e).__name__}: {e}"
|
|
print(f"WARNING: {agent_name} attempt {attempt}/{MAX_ATTEMPTS} raised: {last_error}")
|
|
traceback.print_exc()
|
|
|
|
if attempt < MAX_ATTEMPTS:
|
|
delay = min(2 ** attempt, 60)
|
|
print(f"Retrying {agent_name} in {delay}s ...")
|
|
time.sleep(delay)
|
|
|
|
if last_error is not None:
|
|
raise RuntimeError(
|
|
f"{agent_name} review failed after {MAX_ATTEMPTS} attempts: {last_error}"
|
|
)
|
|
|
|
# Post the summary from the job script so the job fails loudly if anything is broken.
|
|
_post_review()
|
|
|
|
|
|
def review(run_once, agent_name):
|
|
info = Info()
|
|
if not info.pr_number:
|
|
print("Not a PR, skipping")
|
|
return
|
|
|
|
_reauth_gh()
|
|
os.makedirs("./ci/tmp", exist_ok=True)
|
|
prompt = _pre_review_prompt(info)
|
|
_run(prompt, run_once, agent_name)
|
|
|
|
|
|
if __name__ == "__main__":
|
|
if "--codex" in sys.argv:
|
|
run_once, agent_name = _run_codex_once, "Codex"
|
|
elif "--copilot" in sys.argv:
|
|
run_once, agent_name = _run_copilot_once, "Copilot"
|
|
else:
|
|
print("Usage: copilot_review_job.py --codex | --copilot")
|
|
sys.exit(1)
|
|
|
|
status = Result.Status.OK
|
|
info = ""
|
|
try:
|
|
review(run_once, agent_name)
|
|
except Exception as e:
|
|
info = f"ERROR: {e}"
|
|
print(info)
|
|
traceback.print_exc()
|
|
status = Result.Status.FAIL
|
|
|
|
Result.create_from(status=status, info=info).complete_job()
|