From 468193602c70ce58fad02baff273df2eb4653e3a Mon Sep 17 00:00:00 2001 From: KarthikNambiar04 Date: Tue, 22 Sep 2026 11:20:13 +0000 Subject: [PATCH] migrate: repair pr_diff tasks baked before the #145 oracle-isolation fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #145 stopped baking the oracle/verifier/instruction into pr_diff's agent image, but already-published tasks (e.g. AdithyaSK/repo2rlenv-pr-diff, 181 tasks) keep the pre-fix Dockerfile until re-emitted — #155. repo2rlenv migrate pr-diff [--apply] detects the baked-oracle marker and rewrites environment/Dockerfile + tests/{test.sh,verifier.py, oracle.patch,instruction.md} via the SAME builder functions fresh generate calls today, so a migrated task is byte-identical to one emitted now, not a hand-maintained parallel implementation. instruction.md and solution/patch.diff (the oracle itself) are never touched, so a migrated task keeps its original content_hash. repo_url/base_commit are recovered from the existing Dockerfile's remote set-url line and task.toml's metadata.repo2env.ref, not re-derived or guessed. Refuses to touch a task whose instruction.md/solution/patch.diff don't hash to the content_hash task.toml already claims, rather than silently rewriting an inconsistent bundle. Defaults to a dry-run report; --apply writes. --- src/repo2rlenv/cli.py | 45 ++++++ src/repo2rlenv/pipelines/pr_diff.py | 132 +++++++++++++++++ tests/test_pipeline_pr_diff.py | 219 ++++++++++++++++++++++++++++ 3 files changed, 396 insertions(+) diff --git a/src/repo2rlenv/cli.py b/src/repo2rlenv/cli.py index f3f8f76a..306b846a 100644 --- a/src/repo2rlenv/cli.py +++ b/src/repo2rlenv/cli.py @@ -498,6 +498,35 @@ def cmd_validate(args: argparse.Namespace) -> int: return 0 if failures == 0 else 1 +def cmd_migrate(args: argparse.Namespace) -> int: + from repo2rlenv.pipelines.pr_diff import migrate_pr_diff_task + + dataset_dir = Path(args.path).expanduser().resolve() + task_dirs = sorted({tf.parent for tf in dataset_dir.rglob("task.toml")}) + if not task_dirs: + console.error(f"no task.toml files found under {dataset_dir}") + return 1 + + apply = getattr(args, "apply", False) + label = "Migrating" if apply else "Auditing (dry run — pass --apply to write)" + counts: dict[str, int] = {} + errors: list[dict[str, str]] = [] + with console.section(f"{label} pr_diff tasks under {dataset_dir}"): + for task_dir in task_dirs: + result = migrate_pr_diff_task(task_dir, dry_run=not apply) + action = result["action"] + counts[action] = counts.get(action, 0) + 1 + if action == "error": + errors.append(result) + elif action in ("migrated", "would_migrate"): + console.success(f"{result['task']}: {action}") + for err in errors: + console.error(f"{err['task']}: {err['detail']}") + + console.kv(counts, title="pr_diff migration" if apply else "pr_diff migration (dry run)") + return 0 if not errors else 1 + + class _Backend: """Which source/destination a URI points at.""" @@ -1101,6 +1130,22 @@ def _dispatch(argv: list[str]) -> int: ) v.set_defaults(func=cmd_validate) + # migrate + mg = sub.add_parser("migrate", help="Repair already-emitted tasks after a generator fix") + mg_sub = mg.add_subparsers(dest="migration", required=True) + mg_pr_diff = mg_sub.add_parser( + "pr-diff", + help=( + "repair pr_diff tasks emitted before #145's oracle-isolation fix " + "(oracle baked into the agent's own image)" + ), + ) + mg_pr_diff.add_argument("path", help="dataset or task directory") + mg_pr_diff.add_argument( + "--apply", action="store_true", help="write the fix (default: dry-run report only)" + ) + mg_pr_diff.set_defaults(func=cmd_migrate) + # push p = sub.add_parser("push", help="Push a local dataset directory to HF Hub") p.add_argument( diff --git a/src/repo2rlenv/pipelines/pr_diff.py b/src/repo2rlenv/pipelines/pr_diff.py index 83ae0f81..ba459a97 100644 --- a/src/repo2rlenv/pipelines/pr_diff.py +++ b/src/repo2rlenv/pipelines/pr_diff.py @@ -327,6 +327,138 @@ def _pr_diff_aux_files(*, oracle_diff: str, instruction: str) -> dict[str, str]: } +# --------------------------------------------------------------------------- +# Migration: repair tasks emitted before #145's oracle-isolation fix +# --------------------------------------------------------------------------- + +_BAKED_ORACLE_MARKER = "base64 -d > /verifier/oracle.patch" + + +def _harbor_content_hash(instruction: str, oracle_diff: str) -> str: + """Mirror ``emitter/harbor.py:_content_hash`` without needing a full HarborTask. + + ``content_hash`` only ever covers ``instruction.md`` + ``solution/patch.diff`` + (see that function) — the migration below never touches either file, so this + is purely a sanity check that we're reading the same pair the task was + originally hashed against, not a value the migration recomputes and writes. + """ + import hashlib + + h = hashlib.sha256() + h.update(instruction.encode("utf-8")) + h.update(b"\0") + h.update(oracle_diff.encode("utf-8")) + return f"sha256:{h.hexdigest()}" + + +def migrate_pr_diff_task(task_dir: Path, *, dry_run: bool = False) -> dict[str, str]: + """Repair a single ``pr_diff`` task emitted before #145's oracle-isolation fix. + + Pre-#145, ``environment/Dockerfile`` baked the oracle patch, instruction, and + verifier into the agent's own image — readable and ``git apply``-able for a + perfect score. This detects that shape and rewrites ``environment/Dockerfile`` + + ``tests/{test.sh,verifier.py,oracle.patch,instruction.md}`` using the exact + builder functions fresh ``generate`` calls today, so a migrated task is + byte-identical to one emitted now — not a hand-maintained parallel + implementation that could drift from the real fix. + + ``instruction.md`` and ``solution/patch.diff`` (the oracle diff itself) are + never modified, so ``content_hash`` — which only covers those two files + (``emitter/harbor.py:_content_hash``) — is unaffected; a migrated task keeps + its original identity. ``repo_url`` and ``base_commit`` are recovered from + the existing Dockerfile's ``remote set-url origin`` line and ``task.toml``'s + ``metadata.repo2env.ref`` respectively — both fields the pre-#145 builder + already wrote unchanged, so nothing about them needs to be guessed or + re-derived from an external source. + + With ``dry_run=True``, every check runs identically (including the + ``content_hash`` sanity check) but nothing is written — ``action`` reports + what *would* happen (``"would_migrate"`` in place of ``"migrated"``). + + Returns ``{"task": , "action": ..., "detail": ...}`` where + ``action`` is one of: + + - ``"migrated"`` / ``"would_migrate"`` — rewritten in place / would be + - ``"already_safe"`` — no baked oracle found; nothing to do + - ``"skipped"`` — not a pr_diff task (different pipeline, or malformed) + - ``"error"`` — needs manual review; nothing was written + """ + import tomllib + + name = task_dir.name + toml_path = task_dir / "task.toml" + if not toml_path.exists(): + return {"task": name, "action": "error", "detail": "no task.toml"} + try: + config = tomllib.loads(toml_path.read_text(encoding="utf-8")) + except tomllib.TOMLDecodeError as exc: + return {"task": name, "action": "error", "detail": f"cannot parse task.toml: {exc}"} + repo2env = config.get("metadata", {}).get("repo2env", {}) + if repo2env.get("pipeline") != "pr_diff": + return {"task": name, "action": "skipped", "detail": "not a pr_diff task"} + + dockerfile_path = task_dir / "environment" / "Dockerfile" + if not dockerfile_path.exists(): + return {"task": name, "action": "skipped", "detail": "text-only task, no environment/"} + dockerfile = dockerfile_path.read_text(encoding="utf-8") + if _BAKED_ORACLE_MARKER not in dockerfile: + return {"task": name, "action": "already_safe", "detail": ""} + + base_commit = repo2env.get("ref") + origin_match = re.search(r"remote set-url origin (\S+)\n", dockerfile) + if not base_commit or not origin_match: + return { + "task": name, + "action": "error", + "detail": "cannot recover repo_url/base_commit from task.toml/Dockerfile", + } + repo_url = origin_match.group(1).strip("'\"") + + instruction_path = task_dir / "instruction.md" + oracle_path = task_dir / "solution" / "patch.diff" + if not instruction_path.exists() or not oracle_path.exists(): + return { + "task": name, + "action": "error", + "detail": "missing instruction.md or solution/patch.diff", + } + instruction = instruction_path.read_text(encoding="utf-8") + oracle_diff = oracle_path.read_text(encoding="utf-8") + + claimed_hash = repo2env.get("content_hash") + recomputed_hash = _harbor_content_hash(instruction, oracle_diff) + if claimed_hash and claimed_hash != recomputed_hash: + return { + "task": name, + "action": "error", + "detail": ( + f"content_hash mismatch: task.toml claims {claimed_hash}, " + f"instruction.md + solution/patch.diff hash to {recomputed_hash} — " + "needs manual review, not auto-migrated" + ), + } + + if dry_run: + return {"task": name, "action": "would_migrate", "detail": ""} + + dockerfile_path.write_text( + build_pr_diff_environment_dockerfile(repo_url=repo_url, base_commit=base_commit), + encoding="utf-8", + ) + tests_dir = task_dir / "tests" + tests_dir.mkdir(exist_ok=True) + (tests_dir / "test.sh").write_text( + build_pr_diff_eval_script(base_commit=base_commit), encoding="utf-8" + ) + (tests_dir / "test.sh").chmod(0o755) + for relative, content in _pr_diff_aux_files( + oracle_diff=oracle_diff, instruction=instruction + ).items(): + (task_dir / relative).write_text(content, encoding="utf-8") + + return {"task": name, "action": "migrated", "detail": ""} + + # --------------------------------------------------------------------------- # Gen-time helpers: quality filter, baseline calibration, difficulty bucket # --------------------------------------------------------------------------- diff --git a/tests/test_pipeline_pr_diff.py b/tests/test_pipeline_pr_diff.py index ccbdbff9..1a057cf2 100644 --- a/tests/test_pipeline_pr_diff.py +++ b/tests/test_pipeline_pr_diff.py @@ -24,10 +24,12 @@ from repo2rlenv.github import PullRequestSummary from repo2rlenv.pipelines.pr_diff import ( _build_instruction, + _harbor_content_hash, _pr_diff_aux_files, _strip_info_leak, build_pr_diff_environment_dockerfile, build_pr_diff_eval_script, + migrate_pr_diff_task, ) @@ -426,3 +428,220 @@ def test_verifier_source_is_stdlib_only() -> None: imported.add(node.module.split(".")[0]) non_stdlib = sorted(imported - sys.stdlib_module_names) assert non_stdlib == [], f"verifier imports non-stdlib modules: {non_stdlib}" + + +# --------------------------------------------------------------------------- +# migrate_pr_diff_task — repair for tasks emitted before #145 +# --------------------------------------------------------------------------- + + +def _write_old_style_pr_diff_task( + task_dir: Path, + *, + repo_url: str = "https://github.com/x/y.git", + base_commit: str = "deadbeef1234", + oracle_diff: str = "diff --git a/x b/x\n+fix\n", + instruction: str = "# Issue\ndo it", + content_hash: str | None = None, +) -> None: + """Reconstruct a pre-#145 emitted task: the pattern from #144/#155, not the + real old builder (removed) — just enough surface for migrate_pr_diff_task + to detect and repair: the baked-oracle Dockerfile marker, the old test.sh + shim, and the task.toml fields the migration reads.""" + import base64 + + import tomli_w + + task_dir.mkdir(parents=True, exist_ok=True) + (task_dir / "instruction.md").write_text(instruction, encoding="utf-8") + (task_dir / "solution").mkdir(exist_ok=True) + (task_dir / "solution" / "patch.diff").write_text(oracle_diff, encoding="utf-8") + + encoded_oracle = base64.b64encode(oracle_diff.encode("utf-8")).decode("ascii") + env_dir = task_dir / "environment" + env_dir.mkdir(exist_ok=True) + (env_dir / "Dockerfile").write_text( + "FROM python:3.12-slim\n" + f"RUN git clone --filter=blob:none {repo_url} /workspace \\\n" + f" && git -C /workspace remote set-url origin {repo_url}\n" + f"RUN git reset --hard {base_commit}\n" + "RUN mkdir -p /verifier\n" + f'RUN echo "{encoded_oracle}" | base64 -d > /verifier/oracle.patch\n', + encoding="utf-8", + ) + tests_dir = task_dir / "tests" + tests_dir.mkdir(exist_ok=True) + (tests_dir / "test.sh").write_text( + "#!/bin/bash\npython3 /verifier/verifier.py /verifier/oracle.patch " + "/tmp/predicted.patch /verifier/instruction.md\n", + encoding="utf-8", + ) + + resolved_hash = content_hash or _harbor_content_hash(instruction, oracle_diff) + payload = { + "version": "1.0", + "task": {"name": "test/task", "description": "x"}, + "metadata": { + "difficulty": "medium", + "category": "bugfix", + "keywords": [], + "repo2env": { + "pipeline": "pr_diff", + "ref": base_commit, + "content_hash": resolved_hash, + "reward_kinds": ["test_execution", "diff_similarity"], + }, + }, + "agent": {"timeout_sec": 1800.0}, + "verifier": {"timeout_sec": 300.0}, + } + (task_dir / "task.toml").write_bytes(tomli_w.dumps(payload).encode("utf-8")) + + +def test_migrate_detects_and_repairs_baked_oracle(tmp_path: Path) -> None: + task_dir = tmp_path / "x__y-1" + _write_old_style_pr_diff_task( + task_dir, + repo_url="https://github.com/x/y.git", + base_commit="deadbeef1234", + oracle_diff="diff --git a/x b/x\n+fix\n", + instruction="# Issue\ndo it", + ) + + result = migrate_pr_diff_task(task_dir) + + assert result == {"task": "x__y-1", "action": "migrated", "detail": ""} + dockerfile = (task_dir / "environment" / "Dockerfile").read_text(encoding="utf-8") + assert "base64 -d" not in dockerfile + assert "/verifier/" not in dockerfile + assert "git clone --filter=blob:none https://github.com/x/y.git /workspace" in dockerfile + assert "git reset --hard deadbeef1234" in dockerfile + + test_sh = (task_dir / "tests" / "test.sh").read_text(encoding="utf-8") + assert '"$SCRIPT_DIR/oracle.patch"' in test_sh + assert "/verifier/verifier.py" not in test_sh + + aux = _pr_diff_aux_files(oracle_diff="diff --git a/x b/x\n+fix\n", instruction="# Issue\ndo it") + for relative, content in aux.items(): + assert (task_dir / relative).read_text(encoding="utf-8") == content + + # The oracle itself and the instruction are untouched by the migration. + assert (task_dir / "instruction.md").read_text(encoding="utf-8") == "# Issue\ndo it" + assert (task_dir / "solution" / "patch.diff").read_text( + encoding="utf-8" + ) == "diff --git a/x b/x\n+fix\n" + + +def test_migrate_preserves_content_hash(tmp_path: Path) -> None: + """content_hash only ever covers instruction.md + solution/patch.diff — since + the migration never touches either, a migrated task keeps its identity.""" + task_dir = tmp_path / "x__y-1" + oracle = "diff --git a/x b/x\n+fix\n" + instruction = "# Issue\ndo it" + _write_old_style_pr_diff_task(task_dir, oracle_diff=oracle, instruction=instruction) + before = _harbor_content_hash(instruction, oracle) + + migrate_pr_diff_task(task_dir) + + after = _harbor_content_hash( + (task_dir / "instruction.md").read_text(encoding="utf-8"), + (task_dir / "solution" / "patch.diff").read_text(encoding="utf-8"), + ) + assert before == after + + +def test_migrate_is_idempotent(tmp_path: Path) -> None: + task_dir = tmp_path / "x__y-1" + _write_old_style_pr_diff_task(task_dir) + migrate_pr_diff_task(task_dir) + + second = migrate_pr_diff_task(task_dir) + + assert second == {"task": "x__y-1", "action": "already_safe", "detail": ""} + + +def test_migrate_dry_run_does_not_write(tmp_path: Path) -> None: + task_dir = tmp_path / "x__y-1" + _write_old_style_pr_diff_task(task_dir) + original_dockerfile = (task_dir / "environment" / "Dockerfile").read_text(encoding="utf-8") + + result = migrate_pr_diff_task(task_dir, dry_run=True) + + assert result == {"task": "x__y-1", "action": "would_migrate", "detail": ""} + assert (task_dir / "environment" / "Dockerfile").read_text(encoding="utf-8") == ( + original_dockerfile + ) + assert not (task_dir / "tests" / "oracle.patch").exists() + + +def test_migrate_skips_non_pr_diff_task(tmp_path: Path) -> None: + import tomli_w + + task_dir = tmp_path / "other-task" + task_dir.mkdir() + payload = { + "version": "1.0", + "task": {"name": "test/other", "description": "x"}, + "metadata": {"repo2env": {"pipeline": "pr_runtime"}}, + } + (task_dir / "task.toml").write_bytes(tomli_w.dumps(payload).encode("utf-8")) + + result = migrate_pr_diff_task(task_dir) + + assert result == {"task": "other-task", "action": "skipped", "detail": "not a pr_diff task"} + + +def test_migrate_already_safe_task_is_a_noop(tmp_path: Path) -> None: + """A task already emitted by the fixed generator has no baked-oracle marker + in its Dockerfile — migrate must recognize that and touch nothing.""" + task_dir = tmp_path / "x__y-1" + task_dir.mkdir() + env_dir = task_dir / "environment" + env_dir.mkdir() + (env_dir / "Dockerfile").write_text( + build_pr_diff_environment_dockerfile( + repo_url="https://github.com/x/y.git", base_commit="deadbeef" + ), + encoding="utf-8", + ) + + import tomli_w + + payload = { + "version": "1.0", + "task": {"name": "test/task", "description": "x"}, + "metadata": {"repo2env": {"pipeline": "pr_diff", "ref": "deadbeef"}}, + } + (task_dir / "task.toml").write_bytes(tomli_w.dumps(payload).encode("utf-8")) + + result = migrate_pr_diff_task(task_dir) + + assert result == {"task": "x__y-1", "action": "already_safe", "detail": ""} + + +def test_migrate_flags_content_hash_mismatch_instead_of_silently_rewriting( + tmp_path: Path, +) -> None: + """If instruction.md/solution/patch.diff on disk don't hash to the + content_hash task.toml claims, something is inconsistent — refuse to + auto-migrate rather than paper over it.""" + task_dir = tmp_path / "x__y-1" + _write_old_style_pr_diff_task(task_dir, content_hash="sha256:" + "0" * 64) + original_dockerfile = (task_dir / "environment" / "Dockerfile").read_text(encoding="utf-8") + + result = migrate_pr_diff_task(task_dir) + + assert result["action"] == "error" + assert "content_hash mismatch" in result["detail"] + assert (task_dir / "environment" / "Dockerfile").read_text(encoding="utf-8") == ( + original_dockerfile + ) + + +def test_migrate_reports_error_for_missing_task_toml(tmp_path: Path) -> None: + task_dir = tmp_path / "empty" + task_dir.mkdir() + + result = migrate_pr_diff_task(task_dir) + + assert result == {"task": "empty", "action": "error", "detail": "no task.toml"}