From 2a6d90f1daa07b79c27439a1fd340628c59f18af Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Mon, 26 Oct 2026 21:49:03 -0400 Subject: [PATCH] fix: isolate fresh run directories and exact resume labels --- devdex/scorer/suite.py | 21 +++++- devdex/tests/test_run_directory_isolation.py | 75 ++++++++++++++++++++ 2 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 devdex/tests/test_run_directory_isolation.py diff --git a/devdex/scorer/suite.py b/devdex/scorer/suite.py index 71d5a18..fe1b10c 100644 --- a/devdex/scorer/suite.py +++ b/devdex/scorer/suite.py @@ -347,6 +347,8 @@ def run_cell(track, arm, driver, n_runs, concurrency, out_root, label=None, limi # untouched default closes the same gap for the plain `run_eval.py` invocation the README # documents, so `report_metrics.py --pass p1` finds it with no extra flag. label = label or f"p1-{driver}-{track}-{arm}" + if label in (".", "..") or any(c in label for c in ("/", "\\", "\0")): + raise SystemExit("run label must be a single directory component (no slash, backslash or NUL)") # RESUME. Pointing at an existing run directory reuses its run/tasks/ files, and the # harness's own _resume() then skips every qid already finished. This is what makes a # rate-limited vendor runnable across days: stop when the daily cap is hit, continue @@ -356,7 +358,10 @@ def run_cell(track, arm, driver, n_runs, concurrency, out_root, label=None, limi # partial cell file. With it, a rerun of the same label lands in the SAME directory and # _resume() skips every qid already written (flushed every CHUNK=25 items). if auto_resume and not resume_dir: - prior = sorted(Path(out_root).glob(f"*-{label}")) + # The suffix after the timestamp is one complete label, not a glob or a suffix + # match: label 'example' must not inherit a run named 'team-example'. + prior = sorted(p for p in Path(out_root).glob("*") + if p.is_dir() and p.name.partition("-")[2] == label) if prior: resume_dir = str(prior[-1]) # newest wins @@ -373,7 +378,19 @@ def run_cell(track, arm, driver, n_runs, concurrency, out_root, label=None, limi print(f"resuming into {run_dir} ({done} cell file(s) already present)", flush=True) else: stamp = time.strftime("%Y%m%dT%H%M%S") - run_dir = (Path(out_root) / f"{stamp}-{label}").resolve() + root = Path(out_root).resolve() + root.mkdir(parents=True, exist_ok=True) + attempt = 0 + while True: + # Exclusive reservation also handles two launchers racing within the same second. + # Padded suffixes sort after the unsuffixed timestamp and in creation order. + suffix = f"_{attempt:06d}" if attempt else "" + run_dir = root / f"{stamp}{suffix}-{label}" + try: + run_dir.mkdir() + break + except FileExistsError: + attempt += 1 run_dir.mkdir(parents=True, exist_ok=True) ds = DATASET_ALIASES.get(dataset, dataset) if dataset else TRACKS[track] diff --git a/devdex/tests/test_run_directory_isolation.py b/devdex/tests/test_run_directory_isolation.py new file mode 100644 index 0000000..9fe0061 --- /dev/null +++ b/devdex/tests/test_run_directory_isolation.py @@ -0,0 +1,75 @@ +"""Fresh runs cannot inherit old cells; automatic resume identifies one exact label.""" +import json +from pathlib import Path + +import pytest + +from devdex.scorer import suite + + +@pytest.fixture +def runner(tmp_path, monkeypatch): + outputs = [] + monkeypatch.setattr(suite.time, "strftime", lambda _: "20261026T120000") + def fake_call(argv, cwd, env): + directory = Path(env["OUT_DIR"]) + outputs.append(directory.parent) + tasks = directory / "tasks" + tasks.mkdir(exist_ok=True) + (tasks / "p1_repo_no-tool.json").write_text(json.dumps([ + {"qid": f"q{len(outputs)}", "gold_accept": ["gold"], "citations": [], + "committed": False}])) + return 0 + monkeypatch.setattr(suite.subprocess, "call", fake_call) + def run(label="example", **options): + return suite.run_cell("repo", "no-tool", "opus", n_runs=1, concurrency=1, + out_root=str(tmp_path), label=label, **options) + return run, outputs + + +def test_fresh_same_second_runs_reserve_distinct_directories(tmp_path, runner): + run, outputs = runner + run() + first = outputs[0] / "run1/tasks/p1_repo_no-tool.json" + original = first.read_bytes() + run() + assert len(set(outputs)) == 2 + assert first.read_bytes() == original + assert len(list(tmp_path.glob("*/results.json"))) == 2 + + +def test_auto_resume_still_selects_newest_same_label_after_collision(runner): + run, outputs = runner + run() + run() + latest = outputs[-1] + run(auto_resume=True) + assert outputs[-1] == latest + assert outputs[0] != latest + + +def test_auto_resume_does_not_match_another_labels_suffix(tmp_path, runner): + unrelated = tmp_path / "20261025T120000-team-example" + unrelated.mkdir() + run, outputs = runner + run(auto_resume=True) + assert outputs[-1] != unrelated + + +def test_glob_characters_in_a_label_are_literal(tmp_path, runner): + unrelated = tmp_path / "20261025T120000-other" + unrelated.mkdir() + run, outputs = runner + run(label="*", auto_resume=True) + assert outputs[-1] != unrelated + literal = outputs[-1] + run(label="*", auto_resume=True) + assert outputs[-1] == literal + + +@pytest.mark.parametrize("label", ["../outside", "../../../outside", "one/two", "one\\two", ".", "..", "bad\0name"]) +def test_labels_are_single_directory_components_and_rejected_before_calls(label, runner): + run, outputs = runner + with pytest.raises(SystemExit, match="label"): + run(label=label) + assert outputs == []