From 5401f727bc9b0a2473ce8a02fe25efaed31027de Mon Sep 17 00:00:00 2001 From: kai392 Date: Fri, 24 Jul 2026 20:22:33 +0800 Subject: [PATCH] fix: clear stale eval workspace so a failed re-run can't inherit the previous score MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `evaluate_submission` writes every run of a submission into a workspace keyed by submission id. Submission rows are reused rather than recreated — pushing a new commit to a submission PR updates the existing row — so re-evaluations land in the same directory, and nothing removed the previous `results.json`. A re-run that produced no results therefore did not trip either `rc != 0 and not results_path.exists()` guard, and `_prepare_results` parsed the stale payload: the run was marked `completed` and carried the earlier run's score (and the earlier run's cost ledger). That score propagates to `submission.latest_score`, `best_eval_id`, the leaderboard, `update_king_score` and the worker's merge/close decision, so a broken checkpoint could inherit a previously banked passing score. Unlink `results.json` and `cost_ledger.jsonl` before the run starts, and `rm -f` the remote `results.json` in the remote command prologue — `_remote_attempt` rsyncs the remote workspace back without `--delete`, so a stale remote copy would otherwise overwrite the freshly cleared local one. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/JOURNAL.md | 9 +++ .../src/eval_backend/services/eval_runner.py | 20 +++++++ validator/tests/test_eval_runner.py | 60 +++++++++++++++++++ 3 files changed, 89 insertions(+) diff --git a/docs/JOURNAL.md b/docs/JOURNAL.md index 13a8715..85bad10 100644 --- a/docs/JOURNAL.md +++ b/docs/JOURNAL.md @@ -18,6 +18,15 @@ protocol. **Newest entries at the top.** Tag each entry with one or more of: --- +## 2026-07-24 — eval_runner scored a failed re-run from the previous run's results.json #mistake #gotcha +**Context:** `evaluate_submission` writes every run of a submission into `data/workspaces/submissions//`. Submission rows are reused, not recreated — `create_pr_submission` updates the existing row when a miner pushes a new commit to the same PR, so every re-evaluation lands in the same workspace directory. +**Expected:** an evaluation that produces no `results.json` is reported as failed (that is what `_is_missing_results_payload` and the two `rc != 0 and not results_path.exists()` guards are for). +**Actual:** the second evaluation of a submission whose run crashed without writing anything was marked `completed` and carried the **first** run's score. The stale `cost_ledger.jsonl` was billed to it too. +**Root cause:** nothing cleared the workspace between runs. `results.json` from the previous evaluation was still on disk, so `not local_results_path.exists()` was False (no raise), and `_prepare_results` parsed the old payload. The remote path had the same hole one level up: `_remote_attempt` rsyncs the remote workspace back **without `--delete`**, so a stale remote `results.json` would be copied over a freshly cleared local one. +**Impact:** a wrong score propagates all the way through — `submission.latest_score`, `best_eval_id`, the public leaderboard, `update_king_score`, and the worker's `should_promote_submission` merge/close decision. A miner could bank one good score and have any later broken checkpoint inherit it. +**Fix / decision:** `_clear_previous_run_files()` unlinks `results.json` and `cost_ledger.jsonl` at the top of `evaluate_submission`, and the remote command prologue now `rm -f`s the remote `results.json` alongside the ledger truncation it already did. Both existing "missing results" guards then work as written. Regression test: evaluate once with a scoring stub, then again with a stub that writes nothing — the second run must fail with `score is None`. +**Follow-up:** provider-route evaluations are keyed by a unique `EvaluationRun.id`, so their workspace is never reused; left alone. + ## 2026-07-12 — code grader: add resource limits on top of the HOME/secrets fix #security #decision **Context:** issue #71 — the code grader (`run_pass_at_1`) runs untrusted miner/LLM candidate code. The core secret-leak fix (isolated throwaway HOME/cwd, scrubbed env, `python -I`) already landed on main. **Finding:** main's sandbox closes the HOME/secrets exfiltration vector but has **no resource limits** — an untrusted candidate can still exhaust host memory or fork-bomb the eval box within its wall-clock timeout (verified: a 4 GiB `bytearray` allocation runs to completion and "passes" on main). diff --git a/validator/src/eval_backend/services/eval_runner.py b/validator/src/eval_backend/services/eval_runner.py index f06cb0b..d0b2df0 100644 --- a/validator/src/eval_backend/services/eval_runner.py +++ b/validator/src/eval_backend/services/eval_runner.py @@ -460,6 +460,21 @@ def _provider_eval_workspace(settings: Settings, evaluation_id: int) -> Path: return settings.workspace_root.expanduser() / "provider_evaluations" / str(evaluation_id) +def _clear_previous_run_files(*paths: Path) -> None: + """Delete run outputs left behind by an earlier evaluation of the same submission. + + Submission workspaces are keyed by submission id and reused on every + re-evaluation (pushing a new commit to a submission PR updates the existing + row instead of creating a new one). Nothing truncated the previous + ``results.json``, so a re-run that produced no results at all still found the + old file: the ``rc != 0 and not results_path.exists()`` guards did not fire + and ``_prepare_results`` happily parsed the stale payload, reporting the + previous run's score for the new checkpoint. + """ + for path in paths: + path.unlink(missing_ok=True) + + def _prepare_results( results_path: Path, *, @@ -590,6 +605,10 @@ def _build_remote_command( ) return ( f"mkdir -p {shlex.quote(str(ledger_path.parent))} && : > {shlex.quote(str(ledger_path))} && " + # The remote workspace is reused across evaluations of the same submission + # and is rsynced back without --delete, so a stale results.json would be + # copied over the freshly cleared local one when this run produces none. + f"rm -f {shlex.quote(str(results_path))} && " f"export TRINITY_COST_LEDGER={shlex.quote(str(ledger_path))}; " f"export TRINITY_REMOTE_DIR={repo_dir}; " f"export TRINITY_GPU_INDEX={shlex.quote(str(getattr(settings, 'trinity_gpu_index', 5)))}; " @@ -876,6 +895,7 @@ def evaluate_submission( local_workspace.mkdir(parents=True, exist_ok=True) local_results_path = local_workspace / "results.json" local_cost_ledger_path = local_workspace / "cost_ledger.jsonl" + _clear_previous_run_files(local_results_path, local_cost_ledger_path) effective_allow_local_fallback = ( settings.eval_allow_local_fallback if allow_local_fallback is None else allow_local_fallback diff --git a/validator/tests/test_eval_runner.py b/validator/tests/test_eval_runner.py index fc31216..cf12705 100644 --- a/validator/tests/test_eval_runner.py +++ b/validator/tests/test_eval_runner.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +import shlex from pathlib import Path from eval_backend.core.config import Settings @@ -137,6 +138,65 @@ def _fake_local_attempt( assert result.score == 0.88 +def test_reevaluation_does_not_reuse_previous_results(validator_session, tmp_path, monkeypatch): + """A re-run that produces no results must fail, not inherit the old score.""" + session = validator_session + settings = _build_settings(tmp_path) + checkpoint_path = tmp_path / "theta.npy" + checkpoint_path.write_bytes(b"theta") + submission = _add_submission(session, checkpoint_path) + + def _scoring_local_attempt( + settings, + checkpoint_path, + local_results_path, + local_ledger_path, + submission_id, + env, + ): + local_results_path.write_text( + json.dumps({"results": {"TRINITY": {"accuracy": 0.91}}}), + encoding="utf-8", + ) + local_ledger_path.write_text( + json.dumps({"provider": "chutes", "m": "google/gemma-4-31B-turbo-TEE", "p": 100, "c": 50}) + + "\n", + encoding="utf-8", + ) + return ("fake-eval-command", 0, "stdout", "") + + monkeypatch.setattr(eval_runner, "_local_attempt", _scoring_local_attempt) + first = eval_runner.evaluate_submission(session, submission, settings) + assert first.score == 0.91 + + # The miner pushes a new commit: the same submission row (and therefore the + # same workspace) is evaluated again, but this run writes nothing at all. + def _crashing_local_attempt(*args, **kwargs): + return ("fake-eval-command", 1, "stdout", "boom") + + monkeypatch.setattr(eval_runner, "_local_attempt", _crashing_local_attempt) + second = eval_runner.evaluate_submission(session, submission, settings) + + assert second.run.status == "failed" + assert second.score is None + assert submission.status == "failed" + assert submission.latest_score is None + assert second.metrics["cost_usd"] == 0.0 + + +def test_remote_command_clears_stale_remote_results(tmp_path): + settings = _build_settings(tmp_path) + results_path = tmp_path / "workspace" / "results.json" + command = eval_runner._build_remote_command( + settings, + tmp_path / "theta.npy", + results_path, + tmp_path / "workspace" / "ledger.jsonl", + tmp_path / "workspace", + ) + assert f"rm -f {shlex.quote(str(results_path))}" in command + + def test_ledger_cost_report_prices_current_openrouter_models(tmp_path): ledger = tmp_path / "cost_ledger.jsonl" ledger.write_text(