[fix]. Restore PR-scoped review + remove fix cmd
This commit is contained in:
@@ -50,8 +50,5 @@ CONCURRENCY=1
|
|||||||
# Image used for ephemeral job containers (Node + npm + Codex CLI install).
|
# Image used for ephemeral job containers (Node + npm + Codex CLI install).
|
||||||
REVIEW_RUNNER_IMAGE=node:22-bookworm-slim
|
REVIEW_RUNNER_IMAGE=node:22-bookworm-slim
|
||||||
|
|
||||||
# Keep false for review-only mode.
|
|
||||||
ENABLE_FIX_COMMANDS=false
|
|
||||||
|
|
||||||
# Security: fork PRs are skipped unless explicitly enabled.
|
# Security: fork PRs are skipped unless explicitly enabled.
|
||||||
ALLOW_UNTRUSTED_FORKS=false
|
ALLOW_UNTRUSTED_FORKS=false
|
||||||
|
|||||||
@@ -78,7 +78,6 @@ Primary implementation lives under `src/gitea_codex_bot`.
|
|||||||
- `@codex review [security|performance|tests] [--full]`
|
- `@codex review [security|performance|tests] [--full]`
|
||||||
- `@codex rerun`
|
- `@codex rerun`
|
||||||
- `@codex explain`
|
- `@codex explain`
|
||||||
- `@codex fix [--branch ...]` (gated by `ENABLE_FIX_COMMANDS`)
|
|
||||||
- `@codex ignore`
|
- `@codex ignore`
|
||||||
|
|
||||||
## Local Development
|
## Local Development
|
||||||
@@ -124,7 +123,6 @@ Common optional:
|
|||||||
- `CODEX_AUTH_JSON_PATH` (custom path to `auth.json` for `chatgpt` mode)
|
- `CODEX_AUTH_JSON_PATH` (custom path to `auth.json` for `chatgpt` mode)
|
||||||
- `WORKDIR`, `MAX_DIFF_BYTES`, `MAX_REVIEW_MINUTES`, `CONCURRENCY`
|
- `WORKDIR`, `MAX_DIFF_BYTES`, `MAX_REVIEW_MINUTES`, `CONCURRENCY`
|
||||||
- `REVIEW_RUNNER_IMAGE`
|
- `REVIEW_RUNNER_IMAGE`
|
||||||
- `ENABLE_FIX_COMMANDS`
|
|
||||||
- `ALLOW_UNTRUSTED_FORKS`
|
- `ALLOW_UNTRUSTED_FORKS`
|
||||||
|
|
||||||
## Database and Migrations
|
## Database and Migrations
|
||||||
|
|||||||
4
TODO.md
4
TODO.md
@@ -6,7 +6,7 @@
|
|||||||
- [ ] `BUG`: True isolated runner flow: clone/fetch/checkout PR branch inside the ephemeral container itself, not on host before prompt generation.
|
- [ ] `BUG`: True isolated runner flow: clone/fetch/checkout PR branch inside the ephemeral container itself, not on host before prompt generation.
|
||||||
- [x] `BUG`: Remove host-side fallback path for review execution, or gate it behind explicit `ALLOW_HOST_FALLBACK=false` by default so isolation cannot be bypassed silently.
|
- [x] `BUG`: Remove host-side fallback path for review execution, or gate it behind explicit `ALLOW_HOST_FALLBACK=false` by default so isolation cannot be bypassed silently.
|
||||||
- [x] `BUG`: Enforce `.codex-review.yml` `enabled=false` at runtime (currently loaded but not enforced).
|
- [x] `BUG`: Enforce `.codex-review.yml` `enabled=false` at runtime (currently loaded but not enforced).
|
||||||
- [x] `BUG`: Remove `.codex-review.yml` fix policy (`commands.allow_fix`) and rely on global `ENABLE_FIX_COMMANDS`.
|
- [x] `BUG`: Remove fix command support from runtime and command parsing.
|
||||||
- [x] `BUG`: Add stuck-job recovery for `running` jobs (lease timeout + requeue/fail) so one crashed worker does not deadlock the queue.
|
- [x] `BUG`: Add stuck-job recovery for `running` jobs (lease timeout + requeue/fail) so one crashed worker does not deadlock the queue.
|
||||||
- [x] `BUG`: Validate required secrets/settings are non-empty at startup (`GITEA_WEBHOOK_SECRET`, `GITEA_TOKEN`, `ALLOWED_REPOS`) and fail fast if blank.
|
- [x] `BUG`: Validate required secrets/settings are non-empty at startup (`GITEA_WEBHOOK_SECRET`, `GITEA_TOKEN`, `ALLOWED_REPOS`) and fail fast if blank.
|
||||||
- [ ] `TEST`: Add integration test proving the runner executes the exact PR head SHA in isolated mode and does not rely on host checkout.
|
- [ ] `TEST`: Add integration test proving the runner executes the exact PR head SHA in isolated mode and does not rely on host checkout.
|
||||||
@@ -28,7 +28,7 @@
|
|||||||
- [x] `FEATURE`: Add a note line at the end of comments to show model tokens used and such.
|
- [x] `FEATURE`: Add a note line at the end of comments to show model tokens used and such.
|
||||||
- [x] `FEATURE`: Little static tailwind cdn styled page for any http endpoint that just shows what this is, incase this gets discovered by some random lad. Other routes than "/" should return a 404 with if a browser accessed it a again, tailwind cdn themed 404 page. Both should be nicely designed and minimalistic.
|
- [x] `FEATURE`: Little static tailwind cdn styled page for any http endpoint that just shows what this is, incase this gets discovered by some random lad. Other routes than "/" should return a 404 with if a browser accessed it a again, tailwind cdn themed 404 page. Both should be nicely designed and minimalistic.
|
||||||
- [x] `FEATURE`: Apply `.codex-review.yml` `review.default_mode` when `@codex review` is issued without explicit mode.
|
- [x] `FEATURE`: Apply `.codex-review.yml` `review.default_mode` when `@codex review` is issued without explicit mode.
|
||||||
- [ ] `FEATURE`: Add per-repo command policy in `.codex-review.yml` for enabling/disabling `review`, `fix`, `explain`, and `rerun` independently.
|
- [ ] `FEATURE`: Add per-repo command policy in `.codex-review.yml` for enabling/disabling `review`, `explain`, and `rerun` independently.
|
||||||
- [ ] `TEST`: Add structured log redaction tests to ensure PAT/keys never appear in logs/comments.
|
- [ ] `TEST`: Add structured log redaction tests to ensure PAT/keys never appear in logs/comments.
|
||||||
|
|
||||||
### P3 (Backlog)
|
### P3 (Backlog)
|
||||||
|
|||||||
@@ -40,7 +40,6 @@ class Settings(BaseSettings):
|
|||||||
concurrency: int = Field(default=1, alias="CONCURRENCY")
|
concurrency: int = Field(default=1, alias="CONCURRENCY")
|
||||||
|
|
||||||
review_runner_image: str = Field(default="node:22-bookworm-slim", alias="REVIEW_RUNNER_IMAGE")
|
review_runner_image: str = Field(default="node:22-bookworm-slim", alias="REVIEW_RUNNER_IMAGE")
|
||||||
enable_fix_commands: bool = Field(default=False, alias="ENABLE_FIX_COMMANDS")
|
|
||||||
allow_untrusted_forks: bool = Field(default=False, alias="ALLOW_UNTRUSTED_FORKS")
|
allow_untrusted_forks: bool = Field(default=False, alias="ALLOW_UNTRUSTED_FORKS")
|
||||||
|
|
||||||
@field_validator("gitea_base_url")
|
@field_validator("gitea_base_url")
|
||||||
|
|||||||
@@ -495,7 +495,7 @@ async def gitea_webhook(
|
|||||||
gitea.post_issue_comment(repo, pr_number, format_queue_ack(head_sha))
|
gitea.post_issue_comment(repo, pr_number, format_queue_ack(head_sha))
|
||||||
return {"accepted": True, "job_id": job.id, "status": "queued"}
|
return {"accepted": True, "job_id": job.id, "status": "queued"}
|
||||||
|
|
||||||
if parsed_command.name in {"fix", "explain", "ignore", "help"}:
|
if parsed_command.name in {"explain", "ignore", "help"}:
|
||||||
job = enqueue_job(
|
job = enqueue_job(
|
||||||
session,
|
session,
|
||||||
repo=repo,
|
repo=repo,
|
||||||
|
|||||||
@@ -7,7 +7,7 @@ from gitea_codex_bot.types import ParsedCommand
|
|||||||
|
|
||||||
PREFIX_RE = re.compile(r"^@([^\s]+)\s+(.+)$", re.IGNORECASE | re.DOTALL)
|
PREFIX_RE = re.compile(r"^@([^\s]+)\s+(.+)$", re.IGNORECASE | re.DOTALL)
|
||||||
HELP_ALIASES = {"-h", "--help", "help"}
|
HELP_ALIASES = {"-h", "--help", "help"}
|
||||||
SUPPORTED_COMMANDS = {"review", "explain", "fix", "ignore", "rerun"}
|
SUPPORTED_COMMANDS = {"review", "explain", "ignore", "rerun"}
|
||||||
|
|
||||||
|
|
||||||
def parse_command(body: str, aliases: Iterable[str] | None = None) -> ParsedCommand | None:
|
def parse_command(body: str, aliases: Iterable[str] | None = None) -> ParsedCommand | None:
|
||||||
@@ -44,6 +44,4 @@ def parse_command(body: str, aliases: Iterable[str] | None = None) -> ParsedComm
|
|||||||
parsed.mode = mode
|
parsed.mode = mode
|
||||||
parsed.mode_explicit = True
|
parsed.mode_explicit = True
|
||||||
break
|
break
|
||||||
elif name == "fix":
|
|
||||||
parsed.branch_fix = "--branch" in tokens
|
|
||||||
return parsed
|
return parsed
|
||||||
|
|||||||
@@ -1,32 +1,12 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import shlex
|
|
||||||
import subprocess
|
|
||||||
from pathlib import Path
|
|
||||||
from tempfile import TemporaryDirectory
|
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
from gitea_codex_bot.services.gitea import PullRequestContext
|
|
||||||
from gitea_codex_bot.types import ParsedCommand
|
|
||||||
|
|
||||||
|
|
||||||
class ReviewError(RuntimeError):
|
class ReviewError(RuntimeError):
|
||||||
pass
|
pass
|
||||||
|
|
||||||
|
|
||||||
def _run_git(args: list[str], cwd: Path | None = None) -> str:
|
|
||||||
completed = subprocess.run(["git", *args], cwd=cwd, check=True, capture_output=True, text=True)
|
|
||||||
return completed.stdout
|
|
||||||
|
|
||||||
|
|
||||||
def checkout_pr(tmpdir: Path, pr: PullRequestContext) -> Path:
|
|
||||||
repo_dir = tmpdir / "repo"
|
|
||||||
_run_git(["clone", "--no-tags", "--depth", "50", pr.clone_url, str(repo_dir)])
|
|
||||||
_run_git(["fetch", "origin", pr.base_ref, pr.head_ref], cwd=repo_dir)
|
|
||||||
_run_git(["checkout", pr.head_sha], cwd=repo_dir)
|
|
||||||
return repo_dir
|
|
||||||
|
|
||||||
|
|
||||||
def normalize_review_result(result: Any) -> dict[str, Any]:
|
def normalize_review_result(result: Any) -> dict[str, Any]:
|
||||||
if not isinstance(result, dict):
|
if not isinstance(result, dict):
|
||||||
raise ReviewError(f"Invalid review result type: {type(result)!r}")
|
raise ReviewError(f"Invalid review result type: {type(result)!r}")
|
||||||
@@ -39,44 +19,3 @@ def normalize_review_result(result: Any) -> dict[str, Any]:
|
|||||||
if "confidence" not in result:
|
if "confidence" not in result:
|
||||||
result["confidence"] = 0.5
|
result["confidence"] = 0.5
|
||||||
return result
|
return result
|
||||||
|
|
||||||
|
|
||||||
def summarize_command(command: ParsedCommand) -> str:
|
|
||||||
return " ".join(["@codex", command.name, *command.arguments]).strip()
|
|
||||||
|
|
||||||
|
|
||||||
def fix_branch_name(pr_number: int, arguments: list[str] | None = None) -> str:
|
|
||||||
suffix = "fix"
|
|
||||||
if arguments:
|
|
||||||
words = [token.lower().strip() for token in arguments if token.strip() and not token.startswith("--")]
|
|
||||||
if words:
|
|
||||||
clean = "-".join(words[:4])
|
|
||||||
cleaned = "".join(ch if ch.isalnum() or ch == "-" else "-" for ch in clean).strip("-")
|
|
||||||
if cleaned:
|
|
||||||
suffix = f"fix-{cleaned}"
|
|
||||||
return f"codex/pr-{pr_number}-{suffix}"
|
|
||||||
|
|
||||||
|
|
||||||
def create_fix_patch_note(command: ParsedCommand) -> str:
|
|
||||||
details = shlex.join(command.arguments) if command.arguments else "latest findings"
|
|
||||||
return f"Fix command requested for {details}."
|
|
||||||
|
|
||||||
|
|
||||||
def create_fix_branch(
|
|
||||||
pr: PullRequestContext,
|
|
||||||
*,
|
|
||||||
note: str,
|
|
||||||
arguments: list[str] | None = None,
|
|
||||||
) -> str:
|
|
||||||
branch = fix_branch_name(pr.pr_number, arguments=arguments)
|
|
||||||
with TemporaryDirectory(prefix="gitea-codex-fix-") as tmp:
|
|
||||||
tmpdir = Path(tmp)
|
|
||||||
repo_dir = checkout_pr(tmpdir, pr)
|
|
||||||
_run_git(["checkout", "-b", branch], cwd=repo_dir)
|
|
||||||
notes_dir = repo_dir / ".codex"
|
|
||||||
notes_dir.mkdir(parents=True, exist_ok=True)
|
|
||||||
(notes_dir / "fix-note.md").write_text(f"# Codex Fix Note\n\n{note}\n", encoding="utf-8")
|
|
||||||
_run_git(["add", ".codex/fix-note.md"], cwd=repo_dir)
|
|
||||||
_run_git(["-c", "user.name=codex-bot", "-c", "user.email=codex-bot@example.invalid", "commit", "-m", f"Codex fix note for PR {pr.pr_number}"], cwd=repo_dir)
|
|
||||||
_run_git(["push", "origin", f"{branch}:{branch}", "--force"], cwd=repo_dir)
|
|
||||||
return branch
|
|
||||||
|
|||||||
@@ -4,7 +4,7 @@ from dataclasses import dataclass, field
|
|||||||
from typing import Literal
|
from typing import Literal
|
||||||
|
|
||||||
|
|
||||||
CommandName = Literal["review", "explain", "fix", "ignore", "rerun", "help"]
|
CommandName = Literal["review", "explain", "ignore", "rerun", "help"]
|
||||||
|
|
||||||
|
|
||||||
@dataclass(slots=True)
|
@dataclass(slots=True)
|
||||||
@@ -14,5 +14,4 @@ class ParsedCommand:
|
|||||||
mode: str = "summary"
|
mode: str = "summary"
|
||||||
mode_explicit: bool = False
|
mode_explicit: bool = False
|
||||||
full: bool = False
|
full: bool = False
|
||||||
branch_fix: bool = False
|
|
||||||
arguments: list[str] = field(default_factory=list)
|
arguments: list[str] = field(default_factory=list)
|
||||||
|
|||||||
@@ -64,8 +64,12 @@ def run_review_ephemeral(
|
|||||||
gitea = GiteaClient(settings)
|
gitea = GiteaClient(settings)
|
||||||
pr = gitea.get_pull_request(repo, pr_number)
|
pr = gitea.get_pull_request(repo, pr_number)
|
||||||
repo_cfg = _load_repo_review_config_from_gitea(gitea, repo, pr.head_sha)
|
repo_cfg = _load_repo_review_config_from_gitea(gitea, repo, pr.head_sha)
|
||||||
review_prompt = _build_exec_review_prompt(command)
|
_apply_repo_default_review_mode(command, repo_cfg)
|
||||||
|
review_prompt = _build_exec_review_prompt(command, repo_cfg, pr)
|
||||||
container_name = f"codex-review-{uuid.uuid4().hex[:12]}"
|
container_name = f"codex-review-{uuid.uuid4().hex[:12]}"
|
||||||
|
marker_nonce = uuid.uuid4().hex
|
||||||
|
result_start_marker = f"{RESULT_START_MARKER}_{marker_nonce}"
|
||||||
|
result_end_marker = f"{RESULT_END_MARKER}_{marker_nonce}"
|
||||||
extra_env: dict[str, str] = {
|
extra_env: dict[str, str] = {
|
||||||
"GITEA_TOKEN": settings.gitea_token.get_secret_value(),
|
"GITEA_TOKEN": settings.gitea_token.get_secret_value(),
|
||||||
"GITEA_GIT_USERNAME": settings.gitea_bot_username,
|
"GITEA_GIT_USERNAME": settings.gitea_bot_username,
|
||||||
@@ -84,11 +88,17 @@ def run_review_ephemeral(
|
|||||||
pr=pr,
|
pr=pr,
|
||||||
container_name=container_name,
|
container_name=container_name,
|
||||||
review_prompt=review_prompt,
|
review_prompt=review_prompt,
|
||||||
|
result_start_marker=result_start_marker,
|
||||||
|
result_end_marker=result_end_marker,
|
||||||
extra_env=extra_env,
|
extra_env=extra_env,
|
||||||
)
|
)
|
||||||
if completed.returncode != 0:
|
if completed.returncode != 0:
|
||||||
raise RuntimeError(_format_runner_failure(completed))
|
raise RuntimeError(_format_runner_failure(completed))
|
||||||
parsed = _parse_review_result_from_stdout_artifact(completed.stdout)
|
parsed = _parse_review_result_from_stdout_artifact(
|
||||||
|
completed.stdout,
|
||||||
|
result_start_marker=result_start_marker,
|
||||||
|
result_end_marker=result_end_marker,
|
||||||
|
)
|
||||||
parsed["_meta"] = _extract_result_meta_from_codex_stdout(completed.stdout, settings)
|
parsed["_meta"] = _extract_result_meta_from_codex_stdout(completed.stdout, settings)
|
||||||
return normalize_review_result(parsed), repo_cfg
|
return normalize_review_result(parsed), repo_cfg
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
@@ -102,9 +112,17 @@ def _run_ephemeral_container(
|
|||||||
pr: PullRequestContext,
|
pr: PullRequestContext,
|
||||||
container_name: str,
|
container_name: str,
|
||||||
review_prompt: str,
|
review_prompt: str,
|
||||||
|
result_start_marker: str,
|
||||||
|
result_end_marker: str,
|
||||||
extra_env: dict[str, str],
|
extra_env: dict[str, str],
|
||||||
) -> subprocess.CompletedProcess[str]:
|
) -> subprocess.CompletedProcess[str]:
|
||||||
install_and_run = _build_install_and_run_command(settings, pr=pr, review_prompt=review_prompt)
|
install_and_run = _build_install_and_run_command(
|
||||||
|
settings,
|
||||||
|
pr=pr,
|
||||||
|
review_prompt=review_prompt,
|
||||||
|
result_start_marker=result_start_marker,
|
||||||
|
result_end_marker=result_end_marker,
|
||||||
|
)
|
||||||
cmd = _build_docker_command(settings, container_name=container_name, install_and_run=install_and_run)
|
cmd = _build_docker_command(settings, container_name=container_name, install_and_run=install_and_run)
|
||||||
return subprocess.run(
|
return subprocess.run(
|
||||||
cmd,
|
cmd,
|
||||||
@@ -121,6 +139,8 @@ def _build_install_and_run_command(
|
|||||||
*,
|
*,
|
||||||
pr: PullRequestContext,
|
pr: PullRequestContext,
|
||||||
review_prompt: str,
|
review_prompt: str,
|
||||||
|
result_start_marker: str,
|
||||||
|
result_end_marker: str,
|
||||||
) -> str:
|
) -> str:
|
||||||
steps = ["set -euo pipefail"]
|
steps = ["set -euo pipefail"]
|
||||||
if settings.codex_auth_mode != "chatgpt":
|
if settings.codex_auth_mode != "chatgpt":
|
||||||
@@ -190,23 +210,45 @@ def _build_install_and_run_command(
|
|||||||
steps.extend(
|
steps.extend(
|
||||||
[
|
[
|
||||||
" ".join(codex_exec_parts),
|
" ".join(codex_exec_parts),
|
||||||
f'echo "{RESULT_START_MARKER}"',
|
f'echo "{result_start_marker}"',
|
||||||
f"cat {shlex.quote(REVIEW_OUTPUT_FILE)}",
|
f"cat {shlex.quote(REVIEW_OUTPUT_FILE)}",
|
||||||
f'echo "{RESULT_END_MARKER}"',
|
f'echo "{result_end_marker}"',
|
||||||
]
|
]
|
||||||
)
|
)
|
||||||
return "\n".join(steps)
|
return "\n".join(steps)
|
||||||
|
|
||||||
|
|
||||||
def _build_exec_review_prompt(command: ParsedCommand) -> str:
|
def _apply_repo_default_review_mode(command: ParsedCommand, repo_cfg: RepoReviewConfig) -> None:
|
||||||
|
if command.name != "review" or command.mode_explicit:
|
||||||
|
return
|
||||||
|
configured_mode = repo_cfg.default_mode
|
||||||
|
command.mode = configured_mode if configured_mode in {"summary", "security", "performance", "tests", "full"} else "summary"
|
||||||
|
|
||||||
|
|
||||||
|
def _build_exec_review_prompt(command: ParsedCommand, repo_cfg: RepoReviewConfig, pr: PullRequestContext) -> str:
|
||||||
raw = (command.raw or "").strip()
|
raw = (command.raw or "").strip()
|
||||||
remainder = raw
|
remainder = raw
|
||||||
match = re.match(r"^@[^\s]+\s+\S+\s*(.*)$", raw, flags=re.IGNORECASE | re.DOTALL)
|
match = re.match(r"^@[^\s]+\s+\S+\s*(.*)$", raw, flags=re.IGNORECASE | re.DOTALL)
|
||||||
if match:
|
if match:
|
||||||
remainder = match.group(1).strip()
|
remainder = match.group(1).strip()
|
||||||
if not remainder:
|
intent = remainder or "review this pull request and report introduced issues."
|
||||||
return "review: review this pull request and report introduced issues."
|
focus = ", ".join(repo_cfg.focus) if repo_cfg.focus else "correctness, security, maintainability"
|
||||||
return f"review: {remainder}"
|
ignore = ", ".join(repo_cfg.ignore) if repo_cfg.ignore else "(none)"
|
||||||
|
mode = command.mode if command.name in {"review", "rerun"} else "summary"
|
||||||
|
return "\n".join(
|
||||||
|
[
|
||||||
|
f"review: {intent}",
|
||||||
|
"Review only issues introduced by this PR.",
|
||||||
|
f"Compare exactly these commits: base `{pr.base_sha}` ... head `{pr.head_sha}`.",
|
||||||
|
"Use local git data from this checkout; do not review unrelated history.",
|
||||||
|
f"Requested mode: {mode}.",
|
||||||
|
f"Focus areas: {focus}.",
|
||||||
|
f"Ignore patterns: {ignore}.",
|
||||||
|
f"Include tests setting: {repo_cfg.include_tests}.",
|
||||||
|
f"Full review requested: {command.full}.",
|
||||||
|
"Return strict JSON matching the provided output schema.",
|
||||||
|
]
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _build_docker_command(settings: Settings, *, container_name: str, install_and_run: str) -> list[str]:
|
def _build_docker_command(settings: Settings, *, container_name: str, install_and_run: str) -> list[str]:
|
||||||
@@ -323,12 +365,27 @@ def _load_repo_review_config_from_gitea(gitea: GiteaClient, repo: str, head_sha:
|
|||||||
return parse_repo_review_config_text(content, configured=True)
|
return parse_repo_review_config_text(content, configured=True)
|
||||||
|
|
||||||
|
|
||||||
def _parse_review_result_from_stdout_artifact(stdout: str) -> dict[str, Any]:
|
def _parse_review_result_from_stdout_artifact(
|
||||||
start = stdout.find(RESULT_START_MARKER)
|
stdout: str,
|
||||||
end = stdout.find(RESULT_END_MARKER)
|
*,
|
||||||
if start == -1 or end == -1 or end <= start:
|
result_start_marker: str,
|
||||||
|
result_end_marker: str,
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
lines = stdout.splitlines()
|
||||||
|
start_idx = -1
|
||||||
|
end_idx = -1
|
||||||
|
for idx, line in enumerate(lines):
|
||||||
|
if line.strip() == result_start_marker:
|
||||||
|
start_idx = idx
|
||||||
|
break
|
||||||
|
if start_idx != -1:
|
||||||
|
for idx in range(start_idx + 1, len(lines)):
|
||||||
|
if lines[idx].strip() == result_end_marker:
|
||||||
|
end_idx = idx
|
||||||
|
break
|
||||||
|
if start_idx == -1 or end_idx == -1 or end_idx <= start_idx:
|
||||||
raise RuntimeError("Runner output did not include final review artifact markers.")
|
raise RuntimeError("Runner output did not include final review artifact markers.")
|
||||||
artifact = stdout[start + len(RESULT_START_MARKER) : end].strip()
|
artifact = "\n".join(lines[start_idx + 1 : end_idx]).strip()
|
||||||
if not artifact:
|
if not artifact:
|
||||||
raise RuntimeError("Runner output contained empty final review artifact.")
|
raise RuntimeError("Runner output contained empty final review artifact.")
|
||||||
try:
|
try:
|
||||||
|
|||||||
@@ -14,7 +14,6 @@ from gitea_codex_bot.services.comments import upsert_persistent_review_comment_i
|
|||||||
from gitea_codex_bot.services.gitea import GiteaClient
|
from gitea_codex_bot.services.gitea import GiteaClient
|
||||||
from gitea_codex_bot.services.jobs import claim_next_job, finish_job
|
from gitea_codex_bot.services.jobs import claim_next_job, finish_job
|
||||||
from gitea_codex_bot.services.review_format import format_disabled_ack, format_result_comment
|
from gitea_codex_bot.services.review_format import format_disabled_ack, format_result_comment
|
||||||
from gitea_codex_bot.services.reviewer import create_fix_branch, create_fix_patch_note
|
|
||||||
from gitea_codex_bot.types import ParsedCommand
|
from gitea_codex_bot.types import ParsedCommand
|
||||||
from gitea_codex_bot.workers.container_runner import run_review_ephemeral
|
from gitea_codex_bot.workers.container_runner import run_review_ephemeral
|
||||||
|
|
||||||
@@ -24,7 +23,7 @@ logger = logging.getLogger(__name__)
|
|||||||
def _command_from_job(job: ReviewJob) -> ParsedCommand:
|
def _command_from_job(job: ReviewJob) -> ParsedCommand:
|
||||||
args = job.command_args.split() if job.command_args else []
|
args = job.command_args.split() if job.command_args else []
|
||||||
raw = (job.trigger_comment_body or "").strip() or f"@codex {job.command}"
|
raw = (job.trigger_comment_body or "").strip() or f"@codex {job.command}"
|
||||||
return ParsedCommand(name=job.command, raw=raw, arguments=args, full="--full" in args, branch_fix="--branch" in args)
|
return ParsedCommand(name=job.command, raw=raw, arguments=args, full="--full" in args)
|
||||||
|
|
||||||
|
|
||||||
def _handle_non_review_command(
|
def _handle_non_review_command(
|
||||||
@@ -61,23 +60,10 @@ def _handle_non_review_command(
|
|||||||
message = "## Codex Explain\n\nNo previous result found for this command."
|
message = "## Codex Explain\n\nNo previous result found for this command."
|
||||||
gitea.post_issue_comment(job.repo, job.pr_number, message)
|
gitea.post_issue_comment(job.repo, job.pr_number, message)
|
||||||
return True, True, {"summary": message}, None
|
return True, True, {"summary": message}, None
|
||||||
if command.name == "fix":
|
if str(command.name).lower() == "fix":
|
||||||
if not settings.enable_fix_commands:
|
message = "⚠️ `@codex fix` is no longer supported on this bot."
|
||||||
message = "⚠️ `@codex fix` is disabled on this bot instance."
|
|
||||||
gitea.post_issue_comment(job.repo, job.pr_number, message)
|
gitea.post_issue_comment(job.repo, job.pr_number, message)
|
||||||
return True, True, {"summary": message}, None
|
return True, True, {"summary": message}, None
|
||||||
note = create_fix_patch_note(command)
|
|
||||||
if command.branch_fix:
|
|
||||||
try:
|
|
||||||
pr = gitea.get_pull_request(job.repo, job.pr_number)
|
|
||||||
branch = create_fix_branch(pr, note=note, arguments=command.arguments)
|
|
||||||
message = f"## Codex Fix\n\n{note}\n\nCreated branch `{branch}`."
|
|
||||||
gitea.post_issue_comment(job.repo, job.pr_number, message)
|
|
||||||
return True, True, {"summary": note, "mode": "branch", "branch": branch}, None
|
|
||||||
except Exception as exc:
|
|
||||||
return True, False, None, f"Failed to create fix branch: {exc}"
|
|
||||||
gitea.post_issue_comment(job.repo, job.pr_number, f"## Codex Fix\n\n{note}\n\nPatch suggestion mode.")
|
|
||||||
return True, True, {"summary": note, "mode": "patch"}, None
|
|
||||||
return False, False, None, None
|
return False, False, None, None
|
||||||
|
|
||||||
|
|
||||||
@@ -120,13 +106,11 @@ def _build_help_comment(settings: Settings, session: Session, gitea: GiteaClient
|
|||||||
"- `@codex review [security|performance|tests] [--full]`",
|
"- `@codex review [security|performance|tests] [--full]`",
|
||||||
"- `@codex rerun`",
|
"- `@codex rerun`",
|
||||||
"- `@codex explain`",
|
"- `@codex explain`",
|
||||||
"- `@codex fix [--branch ...]`",
|
|
||||||
"- `@codex ignore`",
|
"- `@codex ignore`",
|
||||||
"- `@codex -h` / `@codex --help` / `@codex help`",
|
"- `@codex -h` / `@codex --help` / `@codex help`",
|
||||||
"",
|
"",
|
||||||
"Status note:",
|
"Status note:",
|
||||||
f"- Pending jobs on this PR: `{pending_count}`",
|
f"- Pending jobs on this PR: `{pending_count}`",
|
||||||
f"- Fix command enabled: `{str(settings.enable_fix_commands).lower()}`",
|
|
||||||
f"- {latest_status_line}",
|
f"- {latest_status_line}",
|
||||||
"",
|
"",
|
||||||
f"Discussion summary ({comment_summaries['total']} comments, human `{comment_summaries['human']}`, bot `{comment_summaries['bot']}`):",
|
f"Discussion summary ({comment_summaries['total']} comments, human `{comment_summaries['human']}`, bot `{comment_summaries['bot']}`):",
|
||||||
|
|||||||
@@ -17,7 +17,6 @@ def main() -> int:
|
|||||||
raw=f"@codex {command_payload['name']}",
|
raw=f"@codex {command_payload['name']}",
|
||||||
mode=command_payload.get("mode", "summary"),
|
mode=command_payload.get("mode", "summary"),
|
||||||
full=bool(command_payload.get("full", False)),
|
full=bool(command_payload.get("full", False)),
|
||||||
branch_fix=bool(command_payload.get("branch_fix", False)),
|
|
||||||
arguments=list(command_payload.get("arguments", [])),
|
arguments=list(command_payload.get("arguments", [])),
|
||||||
)
|
)
|
||||||
result, _repo_cfg = run_review_ephemeral(
|
result, _repo_cfg = run_review_ephemeral(
|
||||||
|
|||||||
@@ -17,11 +17,8 @@ def test_parse_review_command_defaults_to_non_explicit_summary_mode() -> None:
|
|||||||
assert cmd.mode_explicit is False
|
assert cmd.mode_explicit is False
|
||||||
|
|
||||||
|
|
||||||
def test_parse_fix_branch() -> None:
|
def test_parse_fix_command_returns_none() -> None:
|
||||||
cmd = parse_command("@codex fix --branch finding 2")
|
assert parse_command("@codex fix --branch finding 2") is None
|
||||||
assert cmd is not None
|
|
||||||
assert cmd.name == "fix"
|
|
||||||
assert cmd.branch_fix is True
|
|
||||||
|
|
||||||
|
|
||||||
def test_invalid_command_returns_none() -> None:
|
def test_invalid_command_returns_none() -> None:
|
||||||
|
|||||||
@@ -6,11 +6,13 @@ import pytest
|
|||||||
|
|
||||||
from gitea_codex_bot.config import get_settings
|
from gitea_codex_bot.config import get_settings
|
||||||
from gitea_codex_bot.services.gitea import PullRequestContext
|
from gitea_codex_bot.services.gitea import PullRequestContext
|
||||||
|
from gitea_codex_bot.services.repo_config import RepoReviewConfig
|
||||||
from gitea_codex_bot.types import ParsedCommand
|
from gitea_codex_bot.types import ParsedCommand
|
||||||
from gitea_codex_bot.workers.container_runner import (
|
from gitea_codex_bot.workers.container_runner import (
|
||||||
CONTAINER_CODEX_HOME,
|
CONTAINER_CODEX_HOME,
|
||||||
RESULT_END_MARKER,
|
RESULT_END_MARKER,
|
||||||
RESULT_START_MARKER,
|
RESULT_START_MARKER,
|
||||||
|
_apply_repo_default_review_mode,
|
||||||
_build_docker_command,
|
_build_docker_command,
|
||||||
_build_exec_review_prompt,
|
_build_exec_review_prompt,
|
||||||
_build_install_and_run_command,
|
_build_install_and_run_command,
|
||||||
@@ -96,7 +98,13 @@ def test_build_install_command_chatgpt_mode_sets_git_checkout_and_review(monkeyp
|
|||||||
settings = get_settings()
|
settings = get_settings()
|
||||||
pr = _sample_pr()
|
pr = _sample_pr()
|
||||||
|
|
||||||
command = _build_install_and_run_command(settings, pr=pr, review_prompt="review: security --full")
|
command = _build_install_and_run_command(
|
||||||
|
settings,
|
||||||
|
pr=pr,
|
||||||
|
review_prompt="review: security --full",
|
||||||
|
result_start_marker=f"{RESULT_START_MARKER}_x",
|
||||||
|
result_end_marker=f"{RESULT_END_MARKER}_x",
|
||||||
|
)
|
||||||
|
|
||||||
assert 'printf "%s" "$CODEX_AUTH_JSON_B64" | base64 -d > /root/.codex/auth.json' in command
|
assert 'printf "%s" "$CODEX_AUTH_JSON_B64" | base64 -d > /root/.codex/auth.json' in command
|
||||||
assert "git -c http.extraHeader=" in command
|
assert "git -c http.extraHeader=" in command
|
||||||
@@ -115,15 +123,21 @@ def test_build_install_command_chatgpt_mode_sets_git_checkout_and_review(monkeyp
|
|||||||
assert "npm install -g @openai/codex@latest" in command
|
assert "npm install -g @openai/codex@latest" in command
|
||||||
assert "codex --version >/tmp/codex-version.log" in command
|
assert "codex --version >/tmp/codex-version.log" in command
|
||||||
assert " - " not in command
|
assert " - " not in command
|
||||||
assert f'echo "{RESULT_START_MARKER}"' in command
|
assert f'echo "{RESULT_START_MARKER}_x"' in command
|
||||||
assert f'echo "{RESULT_END_MARKER}"' in command
|
assert f'echo "{RESULT_END_MARKER}_x"' in command
|
||||||
|
|
||||||
|
|
||||||
def test_build_install_command_does_not_include_reasoning_effort_flag() -> None:
|
def test_build_install_command_does_not_include_reasoning_effort_flag() -> None:
|
||||||
settings = get_settings()
|
settings = get_settings()
|
||||||
pr = _sample_pr()
|
pr = _sample_pr()
|
||||||
|
|
||||||
command = _build_install_and_run_command(settings, pr=pr, review_prompt="review: tests")
|
command = _build_install_and_run_command(
|
||||||
|
settings,
|
||||||
|
pr=pr,
|
||||||
|
review_prompt="review: tests",
|
||||||
|
result_start_marker=f"{RESULT_START_MARKER}_x",
|
||||||
|
result_end_marker=f"{RESULT_END_MARKER}_x",
|
||||||
|
)
|
||||||
|
|
||||||
assert "--reasoning-effort" not in command
|
assert "--reasoning-effort" not in command
|
||||||
|
|
||||||
@@ -132,7 +146,13 @@ def test_build_install_command_uses_upstream_remote_for_fork_pr_base_fetch() ->
|
|||||||
settings = get_settings()
|
settings = get_settings()
|
||||||
pr = _sample_fork_pr()
|
pr = _sample_fork_pr()
|
||||||
|
|
||||||
command = _build_install_and_run_command(settings, pr=pr, review_prompt="review: tests")
|
command = _build_install_and_run_command(
|
||||||
|
settings,
|
||||||
|
pr=pr,
|
||||||
|
review_prompt="review: tests",
|
||||||
|
result_start_marker=f"{RESULT_START_MARKER}_x",
|
||||||
|
result_end_marker=f"{RESULT_END_MARKER}_x",
|
||||||
|
)
|
||||||
|
|
||||||
assert "base_remote=upstream" in command
|
assert "base_remote=upstream" in command
|
||||||
assert f"git remote add upstream {pr.base_clone_url}" in command
|
assert f"git remote add upstream {pr.base_clone_url}" in command
|
||||||
@@ -271,6 +291,10 @@ def test_run_review_ephemeral_single_attempt_success(monkeypatch: pytest.MonkeyP
|
|||||||
return None
|
return None
|
||||||
|
|
||||||
monkeypatch.setattr("gitea_codex_bot.workers.container_runner.GiteaClient", _FakeGiteaClient)
|
monkeypatch.setattr("gitea_codex_bot.workers.container_runner.GiteaClient", _FakeGiteaClient)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"gitea_codex_bot.workers.container_runner.uuid.uuid4",
|
||||||
|
lambda: type("U", (), {"hex": "abc123def4567890abc123def4567890"})(),
|
||||||
|
)
|
||||||
calls: list[list[str]] = []
|
calls: list[list[str]] = []
|
||||||
|
|
||||||
def _fake_run(cmd, *args, **kwargs):
|
def _fake_run(cmd, *args, **kwargs):
|
||||||
@@ -282,9 +306,9 @@ def test_run_review_ephemeral_single_attempt_success(monkeypatch: pytest.MonkeyP
|
|||||||
"returncode": 0,
|
"returncode": 0,
|
||||||
"stdout": (
|
"stdout": (
|
||||||
'{"type":"response.started","model":"gpt-5.3-codex"}\n'
|
'{"type":"response.started","model":"gpt-5.3-codex"}\n'
|
||||||
f"{RESULT_START_MARKER}\n"
|
f"{RESULT_START_MARKER}_abc123def4567890abc123def4567890\n"
|
||||||
'{"verdict":"correct","confidence":0.9,"summary":"ok","findings":[],"markdown_comment":"ok"}\n'
|
'{"verdict":"correct","confidence":0.9,"summary":"ok","findings":[],"markdown_comment":"ok"}\n'
|
||||||
f"{RESULT_END_MARKER}\n"
|
f"{RESULT_END_MARKER}_abc123def4567890abc123def4567890\n"
|
||||||
),
|
),
|
||||||
"stderr": "",
|
"stderr": "",
|
||||||
},
|
},
|
||||||
@@ -308,30 +332,62 @@ def test_run_review_ephemeral_single_attempt_success(monkeypatch: pytest.MonkeyP
|
|||||||
def test_parse_review_result_from_stdout_artifact() -> None:
|
def test_parse_review_result_from_stdout_artifact() -> None:
|
||||||
stdout = (
|
stdout = (
|
||||||
"noise\n"
|
"noise\n"
|
||||||
f"{RESULT_START_MARKER}\n"
|
f"{RESULT_START_MARKER}_test\n"
|
||||||
'{"verdict":"correct","confidence":0.9,"summary":"ok","findings":[],"markdown_comment":"ok"}\n'
|
'{"verdict":"correct","confidence":0.9,"summary":"ok","findings":[],"markdown_comment":"ok"}\n'
|
||||||
f"{RESULT_END_MARKER}\n"
|
f"{RESULT_END_MARKER}_test\n"
|
||||||
|
)
|
||||||
|
parsed = _parse_review_result_from_stdout_artifact(
|
||||||
|
stdout,
|
||||||
|
result_start_marker=f"{RESULT_START_MARKER}_test",
|
||||||
|
result_end_marker=f"{RESULT_END_MARKER}_test",
|
||||||
)
|
)
|
||||||
parsed = _parse_review_result_from_stdout_artifact(stdout)
|
|
||||||
assert parsed["verdict"] == "correct"
|
assert parsed["verdict"] == "correct"
|
||||||
assert parsed["summary"] == "ok"
|
assert parsed["summary"] == "ok"
|
||||||
|
|
||||||
|
|
||||||
def test_parse_review_result_from_stdout_artifact_fails_without_markers() -> None:
|
def test_parse_review_result_from_stdout_artifact_fails_without_markers() -> None:
|
||||||
with pytest.raises(RuntimeError):
|
with pytest.raises(RuntimeError):
|
||||||
_parse_review_result_from_stdout_artifact("no markers here")
|
_parse_review_result_from_stdout_artifact(
|
||||||
|
"no markers here",
|
||||||
|
result_start_marker=f"{RESULT_START_MARKER}_x",
|
||||||
|
result_end_marker=f"{RESULT_END_MARKER}_x",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def test_build_exec_review_prompt_strips_mention_and_command() -> None:
|
def test_build_exec_review_prompt_strips_mention_and_command() -> None:
|
||||||
prompt = _build_exec_review_prompt(
|
prompt = _build_exec_review_prompt(
|
||||||
ParsedCommand(name="review", raw="@codex review security --full\nfocus session handling")
|
ParsedCommand(name="review", raw="@codex review security --full\nfocus session handling"),
|
||||||
|
RepoReviewConfig(),
|
||||||
|
_sample_pr(),
|
||||||
)
|
)
|
||||||
assert prompt == "review: security --full\nfocus session handling"
|
assert prompt.startswith("review: security --full\nfocus session handling")
|
||||||
|
assert "Compare exactly these commits:" in prompt
|
||||||
|
|
||||||
|
|
||||||
def test_build_exec_review_prompt_falls_back_when_no_extra_text() -> None:
|
def test_build_exec_review_prompt_falls_back_when_no_extra_text() -> None:
|
||||||
prompt = _build_exec_review_prompt(ParsedCommand(name="rerun", raw="@codex rerun"))
|
prompt = _build_exec_review_prompt(ParsedCommand(name="rerun", raw="@codex rerun"), RepoReviewConfig(), _sample_pr())
|
||||||
assert prompt == "review: review this pull request and report introduced issues."
|
assert prompt.startswith("review: review this pull request and report introduced issues.")
|
||||||
|
|
||||||
|
|
||||||
|
def test_apply_repo_default_review_mode_for_review_command() -> None:
|
||||||
|
command = ParsedCommand(name="review", raw="@codex review")
|
||||||
|
cfg = RepoReviewConfig(default_mode="tests")
|
||||||
|
_apply_repo_default_review_mode(command, cfg)
|
||||||
|
assert command.mode == "tests"
|
||||||
|
|
||||||
|
|
||||||
|
def test_parse_review_result_from_stdout_artifact_uses_end_marker_after_start() -> None:
|
||||||
|
stdout = (
|
||||||
|
f"{RESULT_START_MARKER}_a\n"
|
||||||
|
'{"verdict":"correct","confidence":0.9,"summary":"contains marker text __CODEX_REVIEW_RESULT_END___a","findings":[],"markdown_comment":"ok"}\n'
|
||||||
|
f"{RESULT_END_MARKER}_a\n"
|
||||||
|
)
|
||||||
|
parsed = _parse_review_result_from_stdout_artifact(
|
||||||
|
stdout,
|
||||||
|
result_start_marker=f"{RESULT_START_MARKER}_a",
|
||||||
|
result_end_marker=f"{RESULT_END_MARKER}_a",
|
||||||
|
)
|
||||||
|
assert parsed["verdict"] == "correct"
|
||||||
|
|
||||||
|
|
||||||
def test_extract_result_meta_from_codex_stdout_collects_model_and_usage() -> None:
|
def test_extract_result_meta_from_codex_stdout_collects_model_and_usage() -> None:
|
||||||
|
|||||||
@@ -184,5 +184,6 @@ def test_process_one_job_help_command_posts_summary(monkeypatch) -> None:
|
|||||||
body = posted_comments[0]
|
body = posted_comments[0]
|
||||||
assert "## Codex Help" in body
|
assert "## Codex Help" in body
|
||||||
assert "@codex -h" in body
|
assert "@codex -h" in body
|
||||||
|
assert "@codex fix" not in body
|
||||||
assert "Discussion summary" in body
|
assert "Discussion summary" in body
|
||||||
assert "@alice: Please check auth edge cases" in body
|
assert "@alice: Please check auth edge cases" in body
|
||||||
|
|||||||
Reference in New Issue
Block a user