From d17a0de654d8f78d7c1877162e7c678241bde9d1 Mon Sep 17 00:00:00 2001 From: jimmy-holdout <346065422+jimmy-holdout@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:08:42 +0000 Subject: [PATCH 1/2] fix(samples): grade 3d-scan-calc against fixed expected values The verifier computed the expected mass and material ID from the scan_data.stl in the container at grading time. The agent can write that file, so replacing it with a zero-volume triangle and reporting mass 0 scored 1.0. Store the expected values as constants, derived once from the shipped environment/scan_data.stl with the existing parse, and grade against them. Grading no longer reads the working STL. A new test checks that the constants still match the shipped file. The skillsbench README now notes that this verifier is patched locally. Fixes #233 --- .../3d-scan-calc/3d-scan-calc.yaml | 2 +- .../3d-scan-calc/verifier/test_outputs.py | 14 +++- tasks/samples/skillsbench/README.md | 15 ++-- .../test_skillsbench_3d_scan_calc_verifier.py | 69 +++++++++++++++++++ 4 files changed, 91 insertions(+), 9 deletions(-) create mode 100644 tests/test_skillsbench_3d_scan_calc_verifier.py diff --git a/tasks/samples/skillsbench/3d-scan-calc/3d-scan-calc.yaml b/tasks/samples/skillsbench/3d-scan-calc/3d-scan-calc.yaml index 46cfef5b5..db435f4d6 100644 --- a/tasks/samples/skillsbench/3d-scan-calc/3d-scan-calc.yaml +++ b/tasks/samples/skillsbench/3d-scan-calc/3d-scan-calc.yaml @@ -58,7 +58,7 @@ run_limits: turn_timeout: 300 success_criteria: - # Run the skillsbench verifier verbatim: copy it from the task dir into a + # Run the skillsbench verifier (patched locally to grade against fixed expected values): copy it from the task dir into a # writable /verifier, run its test.sh (writes /logs/verifier/reward.txt), and # emit that reward (0.0-1.0) as the criterion score. - type: run_command diff --git a/tasks/samples/skillsbench/3d-scan-calc/verifier/test_outputs.py b/tasks/samples/skillsbench/3d-scan-calc/verifier/test_outputs.py index d12da4c2e..e3da5e7f7 100644 --- a/tasks/samples/skillsbench/3d-scan-calc/verifier/test_outputs.py +++ b/tasks/samples/skillsbench/3d-scan-calc/verifier/test_outputs.py @@ -4,14 +4,22 @@ import os import struct +# Expected values for the shipped environment/scan_data.stl, computed once with +# _get_ground_truth() below. They are fixed here because the agent can write to its +# working copy of the STL, so grading must not read it. +# tests/test_skillsbench_3d_scan_calc_verifier.py checks they match the shipped file. +EXPECTED_MASS = 34648.04123490179 # coordinates read as cm +EXPECTED_VOL = 6242.890312594918 +EXPECTED_ID = 42 + class TestOutputs: def test_file_exists(self): """Check output file was created.""" assert os.path.exists("/root/mass_report.json"), "Output file not found" - def _get_ground_truth(self): - filepath = "/root/scan_data.stl" + def _get_ground_truth(self, filepath): + """Derive (mass, vol, id) from an STL. Not used for grading; see EXPECTED_*.""" triangles = [] # Binary Parse with Attribute @@ -85,7 +93,7 @@ def quantize(v): def test_values_correct(self): """Validate submission against ground truth.""" - expected_mass, expected_vol, expected_id = self._get_ground_truth() + expected_mass, expected_vol, expected_id = EXPECTED_MASS, EXPECTED_VOL, EXPECTED_ID with open("/root/mass_report.json") as f: submission = json.load(f) diff --git a/tasks/samples/skillsbench/README.md b/tasks/samples/skillsbench/README.md index 1d819b934..91b28bd2f 100644 --- a/tasks/samples/skillsbench/README.md +++ b/tasks/samples/skillsbench/README.md @@ -8,7 +8,10 @@ They are vendored from [**benchflow-ai/skillsbench**](https://github.com/benchfl (Apache-2.0), pinned at commit [`95b3e60`](https://github.com/benchflow-ai/skillsbench/tree/95b3e60de7a075090e2f9137f32e0d24da1a289b). Each sample keeps the upstream prompt, inputs, and verifier; only the container -base and the coder_eval task wiring were added. +base and the coder_eval task wiring were added. The one exception is the +`3d-scan-calc` verifier, which is patched locally to grade against fixed +expected values instead of re-deriving them from the agent-writable +`scan_data.stl`. ## What they do @@ -44,8 +47,9 @@ coder-eval run tasks/samples/skillsbench/*/*.yaml ``` Each task builds its container, runs the agent (Claude Sonnet 4.6 by default) in -an isolated sandbox, then scores the result with the upstream verifier. Budget -roughly 1–3 minutes per task. +an isolated sandbox, then scores the result with the upstream verifier (for +`3d-scan-calc`, the locally patched one described above). Budget roughly 1–3 +minutes per task. ## Interpreting the results @@ -89,5 +93,6 @@ is non-deterministic, so exact numbers vary from run to run: These tasks are derived from benchflow-ai/skillsbench, licensed under Apache-2.0. The prompts, input data, and verifiers are the work of their original authors -(credited in each upstream task's `task.md`). See the pinned commit above for the -source of truth. +(credited in each upstream task's `task.md`). The `3d-scan-calc` verifier is +patched locally to grade against fixed expected values. See the pinned commit +above for the upstream source of truth. diff --git a/tests/test_skillsbench_3d_scan_calc_verifier.py b/tests/test_skillsbench_3d_scan_calc_verifier.py new file mode 100644 index 000000000..d812d1674 --- /dev/null +++ b/tests/test_skillsbench_3d_scan_calc_verifier.py @@ -0,0 +1,69 @@ +"""Regression tests for the vendored ``skillsbench/3d-scan-calc`` sample verifier.""" + +from __future__ import annotations + +import importlib.util +import json +import struct +from pathlib import Path +from types import ModuleType +from typing import IO, Any + +import pytest + + +_TASK_DIR = Path(__file__).resolve().parents[1] / "tasks" / "samples" / "skillsbench" / "3d-scan-calc" +_SHIPPED_STL_PATH = _TASK_DIR / "environment" / "scan_data.stl" +_SHIPPED_STL = _SHIPPED_STL_PATH.read_bytes() +# One all-zero triangle whose attribute (material ID) is 42: volume 0. +_ZERO_STL = b"\0" * 80 + struct.pack(" ModuleType: + spec = importlib.util.spec_from_file_location("scan_calc_verifier", _TASK_DIR / "verifier" / "test_outputs.py") + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def _grade(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, stl: bytes, report: dict[str, float]) -> None: + """Run ``test_values_correct`` with the verifier's ``/root/...`` paths redirected to ``tmp_path``. + + ``tmp_path`` stands in for the container's ``/root``: it holds the agent's working + ``scan_data.stl`` (*stl*) and its ``mass_report.json`` (*report*). + """ + module = _load_verifier() + (tmp_path / "scan_data.stl").write_bytes(stl) + (tmp_path / "mass_report.json").write_text(json.dumps(report)) + + def _open(path: str, *args: Any, **kwargs: Any) -> IO[Any]: + return open(str(path).replace("/root/", f"{tmp_path}/"), *args, **kwargs) + + monkeypatch.setattr(module, "open", _open, raising=False) + module.TestOutputs().test_values_correct() + + +@pytest.mark.parametrize("mass", [_CORRECT_MASS, _CORRECT_MASS / 1000.0], ids=["cm", "mm"]) +def test_correct_answer_passes(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, mass: float) -> None: + _grade(tmp_path, monkeypatch, _SHIPPED_STL, {"main_part_mass": mass, "material_id": 42}) + + +def test_correct_answer_passes_when_the_stl_header_differs(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + # The 80-byte header is not used by the computation, so changing it must not matter. + _grade(tmp_path, monkeypatch, b"x" * 80 + _SHIPPED_STL[80:], _CORRECT) + + +def test_overwritten_stl_does_not_change_the_expected_values(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + with pytest.raises(AssertionError, match="Mass mismatch"): + _grade(tmp_path, monkeypatch, _ZERO_STL, _MASS_ZERO) + _grade(tmp_path, monkeypatch, _ZERO_STL, _CORRECT) + + +def test_expected_values_match_the_shipped_stl() -> None: + module = _load_verifier() + derived = module.TestOutputs()._get_ground_truth(str(_SHIPPED_STL_PATH)) + assert derived == (module.EXPECTED_MASS, module.EXPECTED_VOL, module.EXPECTED_ID) From a21d1eeee025ca44e3c4b588ad8b00cae85318df Mon Sep 17 00:00:00 2001 From: jimmy-holdout <346065422+jimmy-holdout@users.noreply.github.com> Date: Mon, 26 Oct 2026 00:40:54 +0000 Subject: [PATCH 2/2] fix(samples): address review on 3d-scan-calc verifier State in the skillsbench README and the task YAML that the patch closes only the agent-writable scan_data.stl vector. Grading still runs in the agent's container, so the pytest on PATH, auto-loaded pytest11 plugins, site-packages and the $TASK_DIR verifier copy remain tamperable. The README no longer implies that the other verifiers avoid agent-writable inputs: dialogue-parser reads part of its ground truth from /app/script.txt. Mark the verifier as modified from upstream 95b3e60, and say why the expected values are re-aliased to locals. Tests: add negative cases for a wrong material ID and for a mass just outside the 0.1% tolerance. Drop the header-variation test, which the patched grader made vacuous. Write an STL only in the overwrite test, and take the expected mass and ID from the loaded verifier. Correct .claude/notes/isolation.md: withholding o+w on the task-dir copy does not stop root from rewriting it (CAP_FOWNER is kept), and no digest check covers that copy. Rewrap the YAML comment block. --- .claude/notes/isolation.md | 7 +-- .../3d-scan-calc/3d-scan-calc.yaml | 11 +++-- .../3d-scan-calc/verifier/test_outputs.py | 2 + tasks/samples/skillsbench/README.md | 16 +++++-- .../test_skillsbench_3d_scan_calc_verifier.py | 46 ++++++++++++------- 5 files changed, 56 insertions(+), 26 deletions(-) diff --git a/.claude/notes/isolation.md b/.claude/notes/isolation.md index 03dc8bc8e..5e507ef73 100644 --- a/.claude/notes/isolation.md +++ b/.claude/notes/isolation.md @@ -516,9 +516,10 @@ the `~/.claude` copy the target can be an arbitrary path outside the staging tre `writable=False` is not cosmetic. The container only ever *reads* and `chmod`s the reference copy, so withholding the write bit keeps `_verify_reference_integrity` from being the sole guard against tampering during the gaps between windows. The task-dir copy is -read-only for the same reason: criteria read fixtures there, nothing legitimately writes -them, and withholding `o+w` keeps an agent from rewriting the expectations it is graded -against. The `~/.claude` copy IS writable — the CLI rewrites settings and state in place, +read-only too: criteria read fixtures there and nothing legitimately writes them. That +does not stop an agent from rewriting the expectations it is graded against: root keeps +`CAP_FOWNER` and can chmod the copy back, and `_verify_reference_integrity` does not cover +it. The `~/.claude` copy IS writable — the CLI rewrites settings and state in place, `copytree` preserves host modes, and `~/.claude` is routinely 0700 with 0600 files, which without DAC_OVERRIDE is unreadable to the container, so the agent cannot authenticate. diff --git a/tasks/samples/skillsbench/3d-scan-calc/3d-scan-calc.yaml b/tasks/samples/skillsbench/3d-scan-calc/3d-scan-calc.yaml index db435f4d6..2433241dc 100644 --- a/tasks/samples/skillsbench/3d-scan-calc/3d-scan-calc.yaml +++ b/tasks/samples/skillsbench/3d-scan-calc/3d-scan-calc.yaml @@ -58,9 +58,14 @@ run_limits: turn_timeout: 300 success_criteria: - # Run the skillsbench verifier (patched locally to grade against fixed expected values): copy it from the task dir into a - # writable /verifier, run its test.sh (writes /logs/verifier/reward.txt), and - # emit that reward (0.0-1.0) as the criterion score. + # Run the skillsbench verifier (patched locally to grade against fixed + # expected values): copy it from the task dir into a writable /verifier, run + # its test.sh (writes /logs/verifier/reward.txt), and emit that reward + # (0.0-1.0) as the criterion score. + # The patch closes only the agent-writable scan_data.stl vector (#233). + # Grading still runs in the agent's container, so the pytest on PATH, + # auto-loaded pytest11 plugins, site-packages and the $TASK_DIR verifier + # copy remain tamperable; see the follow-up issue #238. - type: run_command command: | set -e diff --git a/tasks/samples/skillsbench/3d-scan-calc/verifier/test_outputs.py b/tasks/samples/skillsbench/3d-scan-calc/verifier/test_outputs.py index e3da5e7f7..5f1ff6f40 100644 --- a/tasks/samples/skillsbench/3d-scan-calc/verifier/test_outputs.py +++ b/tasks/samples/skillsbench/3d-scan-calc/verifier/test_outputs.py @@ -1,3 +1,4 @@ +# Modified from benchflow-ai/skillsbench@95b3e60: grade against fixed EXPECTED_* (issue #233). import collections import json import math @@ -93,6 +94,7 @@ def quantize(v): def test_values_correct(self): """Validate submission against ground truth.""" + # Local aliases so the upstream lines below stay unchanged (keeps the vendored diff small). expected_mass, expected_vol, expected_id = EXPECTED_MASS, EXPECTED_VOL, EXPECTED_ID with open("/root/mass_report.json") as f: diff --git a/tasks/samples/skillsbench/README.md b/tasks/samples/skillsbench/README.md index 91b28bd2f..0584d5d7b 100644 --- a/tasks/samples/skillsbench/README.md +++ b/tasks/samples/skillsbench/README.md @@ -8,10 +8,18 @@ They are vendored from [**benchflow-ai/skillsbench**](https://github.com/benchfl (Apache-2.0), pinned at commit [`95b3e60`](https://github.com/benchflow-ai/skillsbench/tree/95b3e60de7a075090e2f9137f32e0d24da1a289b). Each sample keeps the upstream prompt, inputs, and verifier; only the container -base and the coder_eval task wiring were added. The one exception is the -`3d-scan-calc` verifier, which is patched locally to grade against fixed -expected values instead of re-deriving them from the agent-writable -`scan_data.stl`. +base and the coder_eval task wiring were added. The one local change is to the +`3d-scan-calc` verifier, which is patched to grade against fixed expected +values instead of re-deriving them from the agent-writable `scan_data.stl` +(issue #233). The other verifiers are unchanged and can still read +agent-writable inputs: `dialogue-parser` takes part of its ground truth from +`/app/script.txt`. + +That patch closes only the `scan_data.stl` vector. Grading still runs in the +agent's container, so the `pytest` on `PATH`, auto-loaded `pytest11` plugins, +site-packages, and the `$TASK_DIR` verifier copy remain tamperable by the +agent. See the follow-up issue +[#238](https://github.com/UiPath/coder_eval/issues/238). ## What they do diff --git a/tests/test_skillsbench_3d_scan_calc_verifier.py b/tests/test_skillsbench_3d_scan_calc_verifier.py index d812d1674..9af437440 100644 --- a/tests/test_skillsbench_3d_scan_calc_verifier.py +++ b/tests/test_skillsbench_3d_scan_calc_verifier.py @@ -14,12 +14,6 @@ _TASK_DIR = Path(__file__).resolve().parents[1] / "tasks" / "samples" / "skillsbench" / "3d-scan-calc" _SHIPPED_STL_PATH = _TASK_DIR / "environment" / "scan_data.stl" -_SHIPPED_STL = _SHIPPED_STL_PATH.read_bytes() -# One all-zero triangle whose attribute (material ID) is 42: volume 0. -_ZERO_STL = b"\0" * 80 + struct.pack(" ModuleType: @@ -30,14 +24,24 @@ def _load_verifier() -> ModuleType: return module -def _grade(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, stl: bytes, report: dict[str, float]) -> None: +_VERIFIER = _load_verifier() +_CORRECT_MASS: float = _VERIFIER.EXPECTED_MASS +_CORRECT_ID: int = _VERIFIER.EXPECTED_ID +_CORRECT = {"main_part_mass": _CORRECT_MASS, "material_id": _CORRECT_ID} +_MASS_ZERO = {"main_part_mass": 0, "material_id": _CORRECT_ID} +# One all-zero triangle whose attribute (material ID) is the expected one: volume 0. +_ZERO_STL = b"\0" * 80 + struct.pack(" None: """Run ``test_values_correct`` with the verifier's ``/root/...`` paths redirected to ``tmp_path``. - ``tmp_path`` stands in for the container's ``/root``: it holds the agent's working - ``scan_data.stl`` (*stl*) and its ``mass_report.json`` (*report*). + ``tmp_path`` stands in for the container's ``/root``: it holds the agent's + ``mass_report.json`` (*report*) and, if *stl* is given, an agent-written ``scan_data.stl``. """ module = _load_verifier() - (tmp_path / "scan_data.stl").write_bytes(stl) + if stl is not None: + (tmp_path / "scan_data.stl").write_bytes(stl) (tmp_path / "mass_report.json").write_text(json.dumps(report)) def _open(path: str, *args: Any, **kwargs: Any) -> IO[Any]: @@ -49,18 +53,28 @@ def _open(path: str, *args: Any, **kwargs: Any) -> IO[Any]: @pytest.mark.parametrize("mass", [_CORRECT_MASS, _CORRECT_MASS / 1000.0], ids=["cm", "mm"]) def test_correct_answer_passes(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, mass: float) -> None: - _grade(tmp_path, monkeypatch, _SHIPPED_STL, {"main_part_mass": mass, "material_id": 42}) + _grade(tmp_path, monkeypatch, {"main_part_mass": mass, "material_id": _CORRECT_ID}) -def test_correct_answer_passes_when_the_stl_header_differs(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: - # The 80-byte header is not used by the computation, so changing it must not matter. - _grade(tmp_path, monkeypatch, b"x" * 80 + _SHIPPED_STL[80:], _CORRECT) +@pytest.mark.parametrize( + ("report", "message"), + [ + ({"main_part_mass": _CORRECT_MASS, "material_id": _CORRECT_ID + 1}, "Expected Material ID"), + ({"main_part_mass": _CORRECT_MASS * 1.002, "material_id": _CORRECT_ID}, "Mass mismatch"), + ], + ids=["wrong-material-id", "mass-just-outside-tolerance"], +) +def test_wrong_answer_fails( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, report: dict[str, float], message: str +) -> None: + with pytest.raises(AssertionError, match=message): + _grade(tmp_path, monkeypatch, report) def test_overwritten_stl_does_not_change_the_expected_values(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: with pytest.raises(AssertionError, match="Mass mismatch"): - _grade(tmp_path, monkeypatch, _ZERO_STL, _MASS_ZERO) - _grade(tmp_path, monkeypatch, _ZERO_STL, _CORRECT) + _grade(tmp_path, monkeypatch, _MASS_ZERO, stl=_ZERO_STL) + _grade(tmp_path, monkeypatch, _CORRECT, stl=_ZERO_STL) def test_expected_values_match_the_shipped_stl() -> None: