From 6ef6cf70fc8fdf54dc11b71dc8d34e5f7302b3ec Mon Sep 17 00:00:00 2001 From: suraj Date: Fri, 18 Sep 2026 10:23:54 +0400 Subject: [PATCH 1/3] pr_diff: ship oracle + verifier via tests/, not baked in image MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The environment/Dockerfile base64-baked the oracle patch, instruction, and verifier into /verifier/ in the agent's own image. Since the agent has a root shell there, it could read /verifier/oracle.patch and git apply it for a perfect reward without solving anything. Ship them as tests/{oracle.patch,verifier.py,instruction.md} aux files instead. Harbor uploads a task's tests/ only at verify time, so the oracle never enters the agent's image. This is the plain-tests pattern pr_runtime has used since #45; pr_diff predated it (baked in #40). Also clear /logs/verifier/reward.txt before the verifier runs, so an agent that breaks python3 and pre-writes it can't pin the reward — the verifier is the sole authority. And teach validate --deep to require the tests/ verifier assets on a runnable pr_diff task. --- docs/pipelines/README.md | 9 ++-- docs/pipelines/pr_diff.md | 6 +-- docs/rfcs/0001-pr-diff.md | 2 +- src/repo2rlenv/pipelines/pr_diff.py | 84 ++++++++++++++++++----------- src/repo2rlenv/validation.py | 26 +++++++++ tests/test_pipeline_pr_diff.py | 78 ++++++++++++--------------- tests/test_pr_diff_clone.py | 2 - tests/test_validation.py | 4 +- 8 files changed, 125 insertions(+), 86 deletions(-) diff --git a/docs/pipelines/README.md b/docs/pipelines/README.md index b483e748..5aa4471b 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 b493681a..5d5bc861 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 b99aadea..ebc272de 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 a7ed1822..f9021274 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 4ff365e9..76389a79 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 d74d390d..92ad39d9 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 be365843..ab94341a 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 659756a5..480363b3 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) From 116c59df09a80148d8098144e5c9f0b36122d178 Mon Sep 17 00:00:00 2001 From: adithya-s-k Date: Tue, 22 Sep 2026 16:15:57 +0530 Subject: [PATCH 2/3] fix(pr-diff): discard stale JSON rewards before verification --- src/repo2rlenv/pipelines/pr_diff.py | 3 +- tests/test_pipeline_pr_diff.py | 69 ++++++++++++++++++++++++++++- 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/repo2rlenv/pipelines/pr_diff.py b/src/repo2rlenv/pipelines/pr_diff.py index f9021274..83ae0f81 100644 --- a/src/repo2rlenv/pipelines/pr_diff.py +++ b/src/repo2rlenv/pipelines/pr_diff.py @@ -289,7 +289,8 @@ def build_pr_diff_eval_script(*, base_commit: str) -> str: # 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" + "rm -f /logs/verifier/reward.txt /logs/verifier/reward.json " + "/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 diff --git a/tests/test_pipeline_pr_diff.py b/tests/test_pipeline_pr_diff.py index 92ad39d9..ccbdbff9 100644 --- a/tests/test_pipeline_pr_diff.py +++ b/tests/test_pipeline_pr_diff.py @@ -14,6 +14,13 @@ from __future__ import annotations +import os +import shlex +import subprocess +from pathlib import Path + +import pytest + from repo2rlenv.github import PullRequestSummary from repo2rlenv.pipelines.pr_diff import ( _build_instruction, @@ -314,7 +321,67 @@ 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 + assert ( + "rm -f /logs/verifier/reward.txt /logs/verifier/reward.json " + "/logs/verifier/reward-details.json" + ) in es + + +@pytest.mark.skipif(os.name == "nt", reason="The emitted verifier runs in a Linux sandbox") +@pytest.mark.parametrize("apply_oracle", [False, True], ids=["no-op", "oracle"]) +def test_verify_time_assets_grade_edits_and_remove_forged_rewards(tmp_path: Path, apply_oracle): + """Execute the emitted shell and real verifier with only paths relocated.""" + workspace = tmp_path / "repo" + workspace.mkdir() + + def git(*args: str) -> str: + return subprocess.run( + ["git", *args], cwd=workspace, capture_output=True, text=True, check=True + ).stdout + + git("init", "-q") + git("config", "user.name", "Test") + git("config", "user.email", "test@example.invalid") + source = workspace / "answer.py" + source.write_text("answer = 1\n", encoding="utf-8") + git("add", "answer.py") + git("commit", "-qm", "base") + base = git("rev-parse", "HEAD").strip() + source.write_text("answer = 2\n", encoding="utf-8") + oracle = git("diff", "HEAD") + if not apply_oracle: + git("checkout", "--", "answer.py") + + rewards = tmp_path / "rewards" + rewards.mkdir() + for name, content in (("reward.txt", "1"), ("reward.json", '{"reward": 1}')): + (rewards / name).write_text(content, encoding="utf-8") + for name, content in _pr_diff_aux_files(oracle_diff=oracle, instruction="Fix answer").items(): + target = tmp_path / name + target.parent.mkdir(exist_ok=True) + target.write_text(content.replace("/logs/verifier", str(rewards)), encoding="utf-8") + script = build_pr_diff_eval_script(base_commit=base) + for original, relocated in ( + ("/workspace", workspace), + ("/logs/verifier", rewards), + ("/tmp/predicted.patch", tmp_path / "predicted.patch"), + ): + script = script.replace(original, shlex.quote(str(relocated))) + script_path = tmp_path / "tests/test.sh" + script_path.write_text(script, encoding="utf-8") + env = {key: value for key, value in os.environ.items() if not key.startswith("R2E_")} + env.update(ANTHROPIC_API_KEY="", GIT_CONFIG_GLOBAL=str(tmp_path / "gitconfig")) + result = subprocess.run( + ["bash", str(script_path)], env=env, capture_output=True, text=True, timeout=30 + ) + assert result.returncode == 0, result.stdout + result.stderr + # Harbor reads reward.json before reward.txt; a forged JSON must not survive. + assert not (rewards / "reward.json").exists() + reward = float((rewards / "reward.txt").read_text(encoding="utf-8")) + if apply_oracle: + assert reward == 1.0 + else: + assert reward < 1.0 def test_eval_script_exits_zero() -> None: From 9176fcbed1f19ed260fb3304d197f536d8febfa8 Mon Sep 17 00:00:00 2001 From: adithya-s-k Date: Tue, 22 Sep 2026 16:23:04 +0530 Subject: [PATCH 3/3] docs(pr-diff): explain migration for published tasks --- docs/pipelines/pr_diff.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/pipelines/pr_diff.md b/docs/pipelines/pr_diff.md index 5d5bc861..f2a61ad0 100644 --- a/docs/pipelines/pr_diff.md +++ b/docs/pipelines/pr_diff.md @@ -14,6 +14,8 @@ | Options model | [`PRDiffOptions`](https://github.com/huggingface/Repo2RLEnv/blob/main/src/repo2rlenv/spec/options.py) | | Reference dataset | [`AdithyaSK/repo2rlenv-pr-diff`](https://huggingface.co/datasets/AdithyaSK/repo2rlenv-pr-diff) on HF Hub | +**Existing exports:** tasks generated before [#145](https://github.com/huggingface/Repo2RLEnv/pull/145) can expose the oracle patch inside the agent image. Updating the package does not repair those tasks or cached images. Regenerate affected exports and rebuild their images before using them for training or evaluation. Migration of the published reference dataset is tracked in [#155](https://github.com/huggingface/Repo2RLEnv/issues/155). + ## What it does ```mermaid @@ -56,7 +58,7 @@ The verifier captures the agent's edits as a unified diff against `base_commit`, Final reward is clipped to `[0, 1]`. A **catastrophic-size hard cap** clamps the final to ≤ 0.40 when `size_sanity < 0.10` — stops a charitable judge from inflating scores on patches that are wildly the wrong size. -The verifier writes both `/logs/verifier/reward.txt` (single float, Harbor reads this) and `/logs/verifier/reward.json` (full breakdown for downstream inspection / re-weighting). +The verifier writes `/logs/verifier/reward.txt` (the reward) and `/logs/verifier/reward-details.json` (the component breakdown). Before grading, the script removes stale text and JSON reward files, since Harbor gives `reward.json` priority when both formats exist. Weights are overridable per-task via `task.toml.metadata` or per-run via `R2E_W_{FORMAT,SIZE,FILE,REGION,SIM,JUDGE}` env vars passed to `harbor run --ve`.