diff --git a/docs/pipelines/README.md b/docs/pipelines/README.md index b483e74..5aa4471 100644 --- a/docs/pipelines/README.md +++ b/docs/pipelines/README.md @@ -132,10 +132,13 @@ default/__/ │ ├── patch.diff # the merged PR's diff = oracle │ └── solve.sh # `git apply patch.diff` (used by harbor's oracle agent) ├── environment/ -│ └── Dockerfile # python:3.12-slim + repo @ base_commit + base64-baked -│ # oracle.patch, instruction.md, verifier.py +│ └── Dockerfile # python:3.12-slim + repo @ base_commit +│ # (the oracle is NOT baked here — see tests/) └── tests/ - └── test.sh # extract verifier from base64; run on the agent's diff + ├── test.sh # capture the agent's diff, run the verifier + ├── verifier.py # the 6-component scorer + ├── oracle.patch # the reference diff — Harbor delivers tests/ only + └── instruction.md # at verify time, so the agent never sees it ``` ### The 6-component reward diff --git a/docs/pipelines/pr_diff.md b/docs/pipelines/pr_diff.md index b493681..5d5bc86 100644 --- a/docs/pipelines/pr_diff.md +++ b/docs/pipelines/pr_diff.md @@ -26,7 +26,7 @@ flowchart TD E --> F[Strip info-leak from
instruction title + body] F --> G[Compute baseline reward
+ difficulty bucket] G --> H[Build Harbor task] - H --> I[task.toml
+ instruction.md
+ solution/patch.diff
+ solution/solve.sh
+ environment/Dockerfile
+ tests/test.sh] + H --> I[task.toml
+ instruction.md
+ solution/patch.diff
+ solution/solve.sh
+ environment/Dockerfile
+ tests/test.sh
+ tests/verifier.py
+ tests/oracle.patch] ``` For each merged PR within scope: @@ -35,9 +35,9 @@ For each merged PR within scope: 2. Fetch the unified diff via `gh pr diff`. 3. Strip leakage patterns from the PR title + body (eight pattern families — see [Instruction info-leak strip](#instruction-info-leak-strip) below). 4. Compute the **calibration baseline** (the score an empty patch would get against this oracle) and the **difficulty bucket** by LOC changed. -5. Emit a Harbor-spec task: `instruction.md`, `solution/{patch.diff, solve.sh}`, `environment/Dockerfile`, `tests/test.sh`, `task.toml`. +5. Emit a Harbor-spec task: `instruction.md`, `solution/{patch.diff, solve.sh}`, `environment/Dockerfile`, `tests/{test.sh, verifier.py, oracle.patch, instruction.md}`, `task.toml`. The oracle and verifier ship under `tests/`, which Harbor uploads only at verify time, so they never enter the agent's image. -The environment is a thin, **agent-agnostic** `python:3.12-slim` image with git + the repo checked out at `base_commit` — no agent CLI is pre-installed. Harbor's agent adapter (`-a claude-code`, `-a openhands`, `-a codex`, `-a aider`, …) drops in the runtime its agent needs when the container starts. The verifier (`tests/test.sh`) runs after the agent and computes the [multi-component reward](#multi-component-reward). +The environment is a thin, **agent-agnostic** `python:3.12-slim` image with git + the repo checked out at `base_commit` — no agent CLI is pre-installed, and **no oracle or verifier is baked in**. Harbor's agent adapter (`-a claude-code`, `-a openhands`, `-a codex`, `-a aider`, …) drops in the runtime its agent needs when the container starts. After the agent, Harbor uploads `tests/` (which carries `verifier.py`, `oracle.patch`, and `instruction.md`) and runs `tests/test.sh`, which computes the [multi-component reward](#multi-component-reward). Keeping the oracle in `tests/` rather than the image is what stops an agent from reading and re-applying it for a free score. **Source host and authentication:** the Dockerfile clones the original GitHub or GitLab repository over HTTPS, preserving its full path. Public repos need no token. The optional clone build arg is `GITHUB_TOKEN` for GitHub or `GITLAB_TOKEN` for GitLab; the consumer supplies it at build time, and the remote URL is scrubbed afterward. Private GitLab MR diff fetching during generation remains a separate unsupported case ([#65](https://github.com/huggingface/Repo2RLEnv/issues/65)); clone authentication alone does not enable end-to-end private GitLab mining. See [`reference/AUTH.md`](../reference/AUTH.md#private-repos-at-task-build-time). diff --git a/docs/rfcs/0001-pr-diff.md b/docs/rfcs/0001-pr-diff.md index b99aade..ebc272d 100644 --- a/docs/rfcs/0001-pr-diff.md +++ b/docs/rfcs/0001-pr-diff.md @@ -25,7 +25,7 @@ The starting point for the project. Datasets of merged PR diffs are the closest 1. `gh pr list --state merged --json ...` — filter mergeAts and skip drafts client-side. 2. Fetch `base.sha` per PR via `github.fetch_pr` (patched in #73 — `gh pr list --json baseRefOid` doesn't populate). 3. Per PR: split into `(source_patch, test_patch)`, apply structural filters, drop drafts and CI-only changes. -4. Emit a Harbor task with the thin env: `python:3.12-slim` + repo clone at `base_commit` + base64-baked `/verifier/oracle.patch`, `/verifier/instruction.md`, `/verifier/verifier.py`. +4. Emit a Harbor task with the thin env: `python:3.12-slim` + repo clone at `base_commit`. The oracle, instruction, and verifier ship as `tests/` aux files (`tests/{oracle.patch, instruction.md, verifier.py}`), which Harbor delivers only at verify time — not baked into the agent's image. (Early versions baked them into `/verifier/`; that let the agent read the oracle, fixed by moving them to `tests/`.) 5. **No sandbox bootstrap.** The Dockerfile is self-contained; consumers rebuild it in ~30 s. ### Output diff --git a/src/repo2rlenv/pipelines/pr_diff.py b/src/repo2rlenv/pipelines/pr_diff.py index a7ed182..f902127 100644 --- a/src/repo2rlenv/pipelines/pr_diff.py +++ b/src/repo2rlenv/pipelines/pr_diff.py @@ -38,7 +38,6 @@ from __future__ import annotations -import base64 import logging import re import shlex @@ -178,16 +177,17 @@ def _verifier_source() -> str: return verifier_path.read_text(encoding="utf-8") -def build_pr_diff_environment_dockerfile( - *, repo_url: str, base_commit: str, oracle_diff: str, instruction: str -) -> str: +def build_pr_diff_environment_dockerfile(*, repo_url: str, base_commit: str) -> str: """Build the minimal Harbor environment/Dockerfile for a pr_diff task. No bootstrap LLM agent — just python:3.12-slim + git + the repo checked - out at ``base_commit``. The oracle diff, the instruction, AND the - verifier source are all base64-baked into the image so the verifier - runs offline with only the LLM-judge call (Anthropic by default, or - the server named by ``R2E_JUDGE_ENDPOINT``) as its outbound dep. + out at ``base_commit``. The oracle diff, the instruction, and the + verifier source are NOT baked into this image: they ship as ``tests/`` + aux files (see ``_pr_diff_aux_files``), which Harbor uploads only at + verification time. Baking the oracle into the agent's own image let the + agent read ``/verifier/oracle.patch`` and ``git apply`` it for a perfect + score; keeping the oracle out of the image closes that. This mirrors the + plain-artifact pattern ``pr_runtime`` has used since #45. The clone uses an optional ``GITHUB_TOKEN`` or ``GITLAB_TOKEN`` build arg, selected by host. Public repos need no arg. The authenticated @@ -200,9 +200,6 @@ def build_pr_diff_environment_dockerfile( reward function is text-similarity + LLM-as-judge. A bare python+git image keeps the build under ~30 s per cell. """ - encoded_oracle = base64.b64encode(oracle_diff.encode("utf-8")).decode("ascii") - encoded_instruction = base64.b64encode(instruction.encode("utf-8")).decode("ascii") - encoded_verifier = base64.b64encode(_verifier_source().encode("utf-8")).decode("ascii") # The image uses HTTPS, including for accepted scp-style git@ inputs. # Preserve the entire path (GitLab projects may have nested namespaces). repo_url = re.sub(r"^git@(github\.com|gitlab\.com):", r"https://\1/", repo_url) @@ -222,7 +219,8 @@ def build_pr_diff_environment_dockerfile( "# Auto-generated by Repo2RLEnv pr_diff — 6-component reward env.\n" "# Agent-agnostic: the agent (claude-code / openhands / codex / etc.)\n" "# installs itself at run time via its harbor adapter. We only ship\n" - "# the source repo + verifier files needed to score the agent's edits.\n" + "# the source repo — the oracle + verifier arrive at verify time via\n" + "# tests/, never baked into the agent's image.\n" "FROM python:3.12-slim\n" # Optional build-time token for private repos. Empty by default → # public clone. The remote is scrubbed post-clone. @@ -250,29 +248,27 @@ def build_pr_diff_environment_dockerfile( f"RUN git fetch --depth 1 origin {base_commit} 2>/dev/null \\\n" " || git fetch --unshallow origin 2>/dev/null || true\n" f"RUN git reset --hard {base_commit} \\\n" - " && git clean -fdx -e .venv -e venv -e __pycache__\n" + " && git clean -fdx -e .venv -e venv -e __pycache__\n" + git_history_scrub(base_commit) # ANTI-CHEAT: strip .git down to base_commit so an agent cannot # `git fetch`/`git show` the merged PR (the oracle) from origin. - + git_history_scrub(base_commit) - + "RUN mkdir -p /verifier\n" - # Bake the oracle diff, instruction, and verifier source so the - # container is fully self-contained (only the LLM-judge step - # requires outbound network). - f'RUN echo "{encoded_oracle}" | base64 -d > /verifier/oracle.patch\n' - f'RUN echo "{encoded_instruction}" | base64 -d > /verifier/instruction.md\n' - f'RUN echo "{encoded_verifier}" | base64 -d > /verifier/verifier.py\n' + # ANTI-CHEAT: the oracle patch, instruction, and verifier are NOT + # baked here — they ship as tests/ aux files Harbor delivers only at + # verification time, so the agent's own image never contains the + # answer. See `_pr_diff_aux_files`. ) def build_pr_diff_eval_script(*, base_commit: str) -> str: """Build the tests/test.sh that Harbor runs after the agent's edits. - Thin shim — the 6-component reward logic lives in - ``/verifier/verifier.py`` (baked into the image by the Dockerfile). - This script just: + Thin shim — the 6-component reward logic lives in ``verifier.py``, + shipped next to this script under ``tests/`` (``$SCRIPT_DIR``) and + delivered by Harbor only at verify time. This script just: - 1. Captures the agent's edits via ``git diff `` - 2. Invokes the verifier, which writes ``/logs/verifier/reward.txt`` + 1. Clears any pre-existing reward file (a tampering agent must not be + able to pre-write ``/logs/verifier/reward.txt`` and have it stand). + 2. Captures the agent's edits via ``git diff ``. + 3. Invokes the verifier, which writes ``/logs/verifier/reward.txt`` (single float) and ``/logs/verifier/reward-details.json`` (component breakdown) for Harbor + downstream inspection. @@ -284,9 +280,16 @@ def build_pr_diff_eval_script(*, base_commit: str) -> str: return ( "#!/bin/bash\n" "set -uxo pipefail\n" + # tests/ is delivered by Harbor at verify time; the verifier and the + # oracle sit next to this script, never in the agent's image. + 'SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"\n' "cd /workspace\n" "git config --global --add safe.directory /workspace\n" "mkdir -p /logs/verifier\n" + # The verifier is the sole authority on the reward: drop any reward + # file the agent may have pre-written. Mirrors Harbor's own + # separate-verifier `empty_dirs([verifier_dir])`. + "rm -f /logs/verifier/reward.txt /logs/verifier/reward-details.json\n" # Capture the agent's edits as a unified diff against base_commit. # IMPORTANT: `git add -A` stages new (untracked) files. Without this # step, `git diff` would skip any file the agent created from scratch @@ -295,16 +298,34 @@ def build_pr_diff_eval_script(*, base_commit: str) -> str: "git add -A\n" f"git diff --cached {base_commit} > /tmp/predicted.patch\n" ": 'START_VERIFY_OUTPUT'\n" - "python3 /verifier/verifier.py \\\n" - " /verifier/oracle.patch \\\n" + 'python3 "$SCRIPT_DIR/verifier.py" \\\n' + ' "$SCRIPT_DIR/oracle.patch" \\\n' " /tmp/predicted.patch \\\n" - " /verifier/instruction.md\n" + ' "$SCRIPT_DIR/instruction.md"\n' ": 'END_VERIFY_OUTPUT'\n" # Always exit 0 — verifier writes reward.txt; bash exit code is moot "exit 0\n" ) +def _pr_diff_aux_files(*, oracle_diff: str, instruction: str) -> dict[str, str]: + """The plain ``tests/`` artifacts the eval script reads at verify time. + + Harbor mounts a task's ``tests/`` into the container only during + verification, so shipping the oracle here (instead of baking it into the + environment/Dockerfile) keeps it out of the image the agent works in. + ``tests/instruction.md`` is the verifier's own copy (it feeds the LLM + judge in-container, and ``tests/`` is the only directory delivered at + verify time). It equals the root ``instruction.md`` the agent sees at + emit time; the two are not re-derived from each other at run time. + """ + return { + "tests/verifier.py": _verifier_source(), + "tests/oracle.patch": oracle_diff, + "tests/instruction.md": instruction, + } + + # --------------------------------------------------------------------------- # Gen-time helpers: quality filter, baseline calibration, difficulty bucket # --------------------------------------------------------------------------- @@ -584,14 +605,14 @@ def _build_task(self, pr: PullRequestSummary, diff: str) -> HarborTask: repo_url = self.input.repo.url dockerfile: str | None = None eval_script: str | None = None + aux_files: dict[str, str] = {} if self.options.emit_harbor_env: dockerfile = build_pr_diff_environment_dockerfile( repo_url=repo_url, base_commit=pr.base_sha, - oracle_diff=diff, - instruction=instruction_text, ) eval_script = build_pr_diff_eval_script(base_commit=pr.base_sha) + aux_files = _pr_diff_aux_files(oracle_diff=diff, instruction=instruction_text) return HarborTask( name=task_id, @@ -605,4 +626,5 @@ def _build_task(self, pr: PullRequestSummary, diff: str) -> HarborTask: keywords=[name, "pr_diff"], environment_dockerfile=dockerfile, test_script=eval_script, + aux_files=aux_files, ) diff --git a/src/repo2rlenv/validation.py b/src/repo2rlenv/validation.py index 4ff365e..76389a7 100644 --- a/src/repo2rlenv/validation.py +++ b/src/repo2rlenv/validation.py @@ -58,6 +58,11 @@ # Pipelines that ship an LLM-authored test at tests/. TEST_FILE_PIPELINES = frozenset({"code_instruct", "equivalence_tests"}) +# Pipelines whose runnable test.sh reads tests/{verifier.py,oracle.patch, +# instruction.md} — shipped as plain aux files (by `pipelines.pr_diff. +# _pr_diff_aux_files`) so the oracle stays out of the agent's image. +DIFF_VERIFIER_PIPELINES = frozenset({"pr_diff"}) + _SHA256_RE = re.compile(r"^sha256:[0-9a-f]{64}$") _FROM_LINE_RE = re.compile(r"^\s*FROM\s+(\S+)", re.IGNORECASE | re.MULTILINE) _DIFF_FILE_HEADER_RE = re.compile(r"^(diff --git |\+\+\+ )", re.MULTILINE) @@ -174,6 +179,8 @@ def _check_layout( _check_graded_verifier(task_dir, sub["fail_to_pass"], report) if pipeline in TEST_FILE_PIPELINES and "test_filename" in sub: _check_test_file(task_dir, sub["test_filename"], report) + if pipeline in DIFF_VERIFIER_PIPELINES and has_env_definition: + _check_diff_verifier(task_dir, report) def _reward_kinds(r2e: dict[str, Any], report: _Report) -> list[str]: @@ -208,6 +215,25 @@ def _check_graded_verifier(task_dir: Path, meta_f2p: Any, report: _Report) -> No _load_test_id_list(task_dir, "tests/p2p.json", report) +def _check_diff_verifier(task_dir: Path, report: _Report) -> None: + """A runnable pr_diff task must ship its verifier + oracle under tests/. + + The oracle is deliberately NOT in the environment image (an agent could + read and apply it), so it rides in tests/, which Harbor delivers only at + verify time. If these are missing the task builds but scores nothing. + """ + verifier = _check_nonempty_file(task_dir, "tests/verifier.py", report) + if verifier is not None: + try: + ast.parse(verifier, filename="tests/verifier.py") + except SyntaxError as exc: + report.error("tests/verifier.py", f"not valid Python: {exc.msg} (line {exc.lineno})") + oracle = _check_nonempty_file(task_dir, "tests/oracle.patch", report) + if oracle is not None and not _DIFF_FILE_HEADER_RE.search(oracle): + report.error("tests/oracle.patch", "does not look like a unified diff") + _check_nonempty_file(task_dir, "tests/instruction.md", report) + + def _load_test_id_list(task_dir: Path, rel: str, report: _Report) -> list[str] | None: path = task_dir / rel if not path.is_file(): diff --git a/tests/test_pipeline_pr_diff.py b/tests/test_pipeline_pr_diff.py index d74d390..92ad39d 100644 --- a/tests/test_pipeline_pr_diff.py +++ b/tests/test_pipeline_pr_diff.py @@ -17,6 +17,7 @@ from repo2rlenv.github import PullRequestSummary from repo2rlenv.pipelines.pr_diff import ( _build_instruction, + _pr_diff_aux_files, _strip_info_leak, build_pr_diff_environment_dockerfile, build_pr_diff_eval_script, @@ -257,8 +258,6 @@ def test_dockerfile_starts_from_python_slim() -> None: df = build_pr_diff_environment_dockerfile( repo_url="https://github.com/pallets/click.git", base_commit="abc1234567890", - oracle_diff="diff --git a/x.py b/x.py\n", - instruction="# Issue\n\nfix the thing", ) assert "FROM python:3.12-slim" in df assert "apt-get install" in df and "git" in df @@ -266,49 +265,31 @@ def test_dockerfile_starts_from_python_slim() -> None: assert "git reset --hard abc1234567890" in df -def test_dockerfile_bakes_oracle_diff_as_base64() -> None: +def test_dockerfile_does_not_contain_the_oracle() -> None: + """The oracle must NOT be baked into the agent's image — it ships as a + tests/ aux file Harbor delivers only at verify time. Baking it let the + agent read /verifier/oracle.patch and `git apply` it for a free 1.0.""" import base64 - oracle = "diff --git a/foo.py b/foo.py\n@@ -1 +1 @@\n-old\n+new\n" + oracle = "diff --git a/foo.py b/foo.py\n@@ -1 +1 @@\n-old\n+secret_fix\n" df = build_pr_diff_environment_dockerfile( repo_url="https://github.com/x/y.git", base_commit="deadbeef", - oracle_diff=oracle, - instruction="anything", ) - encoded = base64.b64encode(oracle.encode("utf-8")).decode("ascii") - assert encoded in df - assert "base64 -d > /verifier/oracle.patch" in df + # Neither the oracle text nor its base64 encoding appears in the image. + assert "secret_fix" not in df + assert base64.b64encode(oracle.encode()).decode() not in df + # No /verifier bakes at all. + assert "/verifier/" not in df + assert "base64 -d" not in df -def test_dockerfile_bakes_instruction_and_verifier_source() -> None: - import base64 - - instr = "# Issue\nTitle: Fix the thing" - df = build_pr_diff_environment_dockerfile( - repo_url="https://github.com/x/y.git", - base_commit="deadbeef", - oracle_diff="diff --git a/x b/x\n", - instruction=instr, - ) - encoded_instr = base64.b64encode(instr.encode("utf-8")).decode("ascii") - assert encoded_instr in df - assert "base64 -d > /verifier/instruction.md" in df - assert "base64 -d > /verifier/verifier.py" in df - - -def test_dockerfile_handles_oracle_with_special_chars() -> None: - """A patch containing quotes / $ / backticks must base64-encode cleanly.""" - oracle = 'diff --git a/q.py b/q.py\n+x = "$y `cmd` $(other)"\n' - df = build_pr_diff_environment_dockerfile( - repo_url="https://github.com/x/y.git", - base_commit="cafe", - oracle_diff=oracle, - instruction="anything", - ) - # No raw special chars from the patch should appear in the Dockerfile — - # they're only in the base64 blob. - assert "$y `cmd`" not in df +def test_aux_files_ship_verifier_oracle_instruction() -> None: + """The verifier, oracle, and instruction ride in tests/ (verify-time only).""" + aux = _pr_diff_aux_files(oracle_diff="diff --git a/x b/x\n+fix\n", instruction="# Issue\ndo it") + assert set(aux) == {"tests/verifier.py", "tests/oracle.patch", "tests/instruction.md"} + assert aux["tests/oracle.patch"] == "diff --git a/x b/x\n+fix\n" + assert "def main" in aux["tests/verifier.py"] def test_eval_script_shebang_and_paths() -> None: @@ -319,10 +300,21 @@ def test_eval_script_shebang_and_paths() -> None: # without this, PRs that add files would silently downscore. assert "git add -A" in es assert "git diff --cached abc1234567890 > /tmp/predicted.patch" in es - # The thin shim just invokes the baked-in verifier - assert "/verifier/verifier.py" in es - assert "/verifier/oracle.patch" in es - assert "/verifier/instruction.md" in es + # The verifier + oracle come from tests/ ($SCRIPT_DIR), not the image. + assert 'SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"' in es + assert '"$SCRIPT_DIR/verifier.py"' in es + assert '"$SCRIPT_DIR/oracle.patch"' in es + assert '"$SCRIPT_DIR/instruction.md"' in es + # The old baked location must be gone (but /logs/verifier/ stays). + assert "/verifier/verifier.py" not in es + assert "/verifier/oracle.patch" not in es + + +def test_eval_script_clears_stale_reward_files() -> None: + """A tampering agent must not be able to pre-write reward.txt and have it + stand — the script drops any reward file before the verifier runs.""" + es = build_pr_diff_eval_script(base_commit="abc") + assert "rm -f /logs/verifier/reward.txt /logs/verifier/reward-details.json" in es def test_eval_script_exits_zero() -> None: @@ -338,8 +330,6 @@ def test_dockerfile_supports_private_repo_build_arg() -> None: df = build_pr_diff_environment_dockerfile( repo_url="https://github.com/myorg/private-repo.git", base_commit="abc123", - oracle_diff="diff --git a/x b/x\n+1\n", - instruction="do it", ) # Build arg declared, empty default (public repos need no arg). assert "ARG GITHUB_TOKEN=" in df @@ -354,7 +344,7 @@ def test_dockerfile_supports_private_repo_build_arg() -> None: def test_verifier_source_is_stdlib_only() -> None: - """The verifier is baked into a bare python:3.12-slim image — nothing but stdlib may be imported.""" + """The verifier ships to a bare python:3.12-slim container — nothing but stdlib may be imported.""" import ast import sys diff --git a/tests/test_pr_diff_clone.py b/tests/test_pr_diff_clone.py index be36584..ab94341 100644 --- a/tests/test_pr_diff_clone.py +++ b/tests/test_pr_diff_clone.py @@ -88,8 +88,6 @@ def test_clone_shell_uses_correct_token_and_scrubs_origin( dockerfile = build_pr_diff_environment_dockerfile( repo_url=repo_url, base_commit="0" * 40, - oracle_diff="", - instruction="Fix the result", ) assert f"ARG {token_arg}=\n" in dockerfile assert other_arg not in dockerfile diff --git a/tests/test_validation.py b/tests/test_validation.py index 659756a..480363b 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -21,6 +21,7 @@ from repo2rlenv.pipelines._env_guard import egress_guard_compose from repo2rlenv.pipelines.code_instruct import build_code_instruct_dockerfile from repo2rlenv.pipelines.pr_diff import ( + _pr_diff_aux_files, build_pr_diff_environment_dockerfile, build_pr_diff_eval_script, ) @@ -61,10 +62,9 @@ def _pr_diff_env(tmp_path: Path) -> Path: environment_dockerfile=build_pr_diff_environment_dockerfile( repo_url="https://github.com/demo/repo.git", base_commit=BASE, - oracle_diff=PATCH, - instruction="fix the bug", ), test_script=build_pr_diff_eval_script(base_commit=BASE), + aux_files=_pr_diff_aux_files(oracle_diff=PATCH, instruction="fix the bug"), ) return write_harbor_task(task, tmp_path)