From 2c777ab6cd3bd814b4b30ffe4eb9f7601838aad5 Mon Sep 17 00:00:00 2001 From: vaibhavdabas16 Date: Thu, 20 Aug 2026 00:07:26 +0530 Subject: [PATCH 1/3] fix(judge): a bad --judge model no longer discards a completed run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit load_model_config() called sys.exit(1) on an unknown or malformed model name. run.py loads the *judge* model inside the judge stage, which only executes after the agent has finished. SystemExit derives from BaseException, so it escaped both the judge stage's `except Exception` and run()'s top-level `except Exception`, and the process died before write_run_meta() ever ran. For a first-time user whose models.yaml lacks deepseek-v4-pro — the default judge — that meant burning a full agent run of up to 30 minutes, quite possibly a successful interception, and ending with no run-meta.json at all: invisible to batch stats, to clawbench-rescore (which globs run-meta.json), and to HF upload. Both asks in the issue are implemented: - load_model_config() raises ModelConfigError, a plain Exception, instead of calling sys.exit(). load_models_yaml() does the same, since load_model_config() calls it and a missing models.yaml would otherwise still exit from inside the judge stage. Every failure branch on that path — missing file, unknown model, illegal characters, missing required field, absent api key — is now catchable. The top-level call site that loads the agent's own model catches it and exits as before, since no output directory exists yet and there is nothing to record. The judge stage's existing except-Exception handler now catches it too, records judge_setup_failed in judge.json, and the run proceeds to write run-meta.json as normal. - The judge model is resolved at startup, before the agent runs, so a --judge typo costs seconds rather than half an hour. The judge stage reuses that validated config; the lazy load remains for --human runs, which skip startup validation. This holds the invariant the issue asks for: a completed agent run always writes its metadata, whatever happens at scoring time. Fixes #302. --- src/clawbench/runner/run.py | 27 +- src/clawbench/runner/run_support/config.py | 44 ++-- tests/test_run_judge_stage.py | 288 +++++++++++++++++++++ 3 files changed, 341 insertions(+), 18 deletions(-) create mode 100644 tests/test_run_judge_stage.py diff --git a/src/clawbench/runner/run.py b/src/clawbench/runner/run.py index 3f92276c..fd01f9ce 100644 --- a/src/clawbench/runner/run.py +++ b/src/clawbench/runner/run.py @@ -21,6 +21,7 @@ HARNESSES, IMAGE, WORKSPACE_ROOT, + ModelConfigError, harness_image, load_model_config, load_runtime_env, @@ -208,10 +209,28 @@ def main(): safe_model = "human" harness_tag = "human" else: - model_cfg = load_model_config(args.model) + try: + model_cfg = load_model_config(args.model) + except ModelConfigError as e: + # No output_dir exists yet at this point, so there is nothing + # for a run-meta.json to record — exit immediately like other + # pre-flight validation above (e.g. missing PURELY_MAIL_* env). + print(f"ERROR: {e}") + sys.exit(1) safe_model = re.sub(r"[/:]+", "--", args.model) harness_tag = args.harness + # Resolve the judge model now rather than after the agent run. A bad + # --judge is a typo in the command line, and finding out about it 30 + # minutes later — once the agent has already finished — helps nobody. + startup_judge_cfg: dict | None = None + if not args.human and args.judge and not args.no_judge: + try: + startup_judge_cfg = load_model_config(args.judge) + except ModelConfigError as e: + print(f"ERROR: --judge {args.judge!r}: {e}") + sys.exit(1) + container = f"clawbench-{harness_tag}-{case_name}-{safe_model}-{int(time.time())}" run_dir_name = f"{harness_tag}-{case_name}-{safe_model}-{ts}" @@ -227,7 +246,7 @@ def main(): extra_info_warnings: list[str] = [] intercepted = False host_port: int | None = None - judge_cfg: dict | None = None + judge_cfg: dict | None = startup_judge_cfg personal_info_metadata: dict[str, Any] | None = None browser_session: BrowserSession | None = None browser_runtime_cleaned = False @@ -575,7 +594,9 @@ def handle_sigint(sig, frame): try: from clawbench.runner.judge import judge_request - judge_cfg = load_model_config(args.judge) + # Validated at startup; reload only if that was skipped. + if judge_cfg is None: + judge_cfg = load_model_config(args.judge) instruction_text = ( task.get("instruction") if isinstance(task, dict) else "" ) or "" diff --git a/src/clawbench/runner/run_support/config.py b/src/clawbench/runner/run_support/config.py index b093ea59..37dc6a99 100644 --- a/src/clawbench/runner/run_support/config.py +++ b/src/clawbench/runner/run_support/config.py @@ -32,6 +32,7 @@ "MODELS_YAML", "WORKSPACE_ROOT", "HarnessRegistry", + "ModelConfigError", "harness_image", "load_dotenv", "load_harness_registry", @@ -44,6 +45,16 @@ ] +class ModelConfigError(Exception): + """Raised when a model config in models/models.yaml is missing or invalid. + + A plain Exception (not SystemExit) so callers that load a model mid-run + (e.g. the judge stage, after the agent has already produced results) can + catch it and continue instead of the process dying before run-meta.json + is written. + """ + + HARNESSES = HARNESS_REGISTRY.harnesses DEFAULT_HARNESS = HARNESS_REGISTRY.default BASE_IMAGE = HARNESS_REGISTRY.base_image @@ -107,12 +118,16 @@ def load_dotenv(path: Path) -> dict[str, str]: def load_models_yaml() -> dict: - """Load all model definitions from models/models.yaml.""" + """Load all model definitions from models/models.yaml. + + Raises ModelConfigError rather than exiting, for the same reason + load_model_config does: this runs inside the judge stage too, where a + SystemExit would escape the handlers and lose the run's metadata. + """ if not MODELS_YAML.exists(): - print( - f"ERROR: {MODELS_YAML} not found (copy models.example.yaml and fill in your keys)" + raise ModelConfigError( + f"{MODELS_YAML} not found (copy models.example.yaml and fill in your keys)" ) - sys.exit(1) return yaml.safe_load(MODELS_YAML.read_text()) or {} @@ -161,9 +176,10 @@ def load_model_config(model: str) -> dict: """ all_models = load_models_yaml() if model not in all_models: - print(f"ERROR: model '{model}' not found in {MODELS_YAML}") - print(f"Available models: {', '.join(sorted(all_models))}") - sys.exit(1) + raise ModelConfigError( + f"model '{model}' not found in {MODELS_YAML}. " + f"Available models: {', '.join(sorted(all_models))}" + ) # Validate model name characters. Note: '/' and ':' are valid in # vendor-prefixed ids like 'anthropic/claude-sonnet-4-6' or @@ -173,11 +189,10 @@ def load_model_config(model: str) -> dict: # that sanitization. bad = [c for c in ' \\*?"<>|' if c in model] if bad: - print( - f"ERROR: model name '{model}' contains illegal character(s): " + raise ModelConfigError( + f"model name '{model}' contains illegal character(s): " f"{' '.join(repr(c) for c in bad)}" ) - sys.exit(1) config = dict(all_models[model]) config["model"] = model # the YAML key IS the model name @@ -185,9 +200,9 @@ def load_model_config(model: str) -> dict: required = ["base_url", "api_type"] missing = [k for k in required if not config.get(k)] if missing: - for k in missing: - print(f"ERROR: Required field '{k}' missing for model '{model}'") - sys.exit(1) + raise ModelConfigError( + f"required field(s) missing for model '{model}': {', '.join(missing)}" + ) # Normalize API keys: api_keys list wins, else wrap api_key into list. if config.get("api_keys"): @@ -196,7 +211,6 @@ def load_model_config(model: str) -> dict: config["api_keys"] = [config["api_key"]] if not config.get("api_keys"): - print(f"ERROR: no api_key or api_keys for model '{model}'") - sys.exit(1) + raise ModelConfigError(f"no api_key or api_keys for model '{model}'") return config diff --git a/tests/test_run_judge_stage.py b/tests/test_run_judge_stage.py new file mode 100644 index 00000000..fac864f6 --- /dev/null +++ b/tests/test_run_judge_stage.py @@ -0,0 +1,288 @@ +"""Regression test for a missing/invalid --judge model (issue #302). + +Before the fix, load_model_config() called sys.exit() on a bad model name. +When that happened for the *judge* model — after the agent had already run +and produced results — the resulting SystemExit escaped both the judge +stage's `except Exception` and run()'s top-level `except Exception`, since +SystemExit is a BaseException, not an Exception. The process died without +ever calling write_run_meta(), silently discarding the run's results. + +The fix has two halves, one test each: + + * A bad --judge is now rejected at startup, before the agent runs at all, + so a command-line typo costs seconds instead of half an hour. + * A judge failure that only surfaces mid-run is caught and degraded to a + judge_setup_failed outcome, and the run still reaches write_run_meta(). + +These drive run_mod.main() through a fully mocked agent run, skipping the +docker/network/email side effects. +""" + +from __future__ import annotations + +import importlib +import json +import shutil +import sys +from pathlib import Path +from types import ModuleType +from typing import Any + +import pytest + + +def _import_run_module(monkeypatch: pytest.MonkeyPatch) -> ModuleType: + for module_name in ( + "clawbench.runner.run", + "clawbench.runner.run_support.metadata", + "clawbench.runner.run_support.docker", + "clawbench.runner.run_support.config", + ): + sys.modules.pop(module_name, None) + monkeypatch.delenv("CONTAINER_ENGINE", raising=False) + monkeypatch.setattr( + shutil, + "which", + lambda cmd: str(Path("mock-bin") / cmd) if cmd == "docker" else None, + ) + return importlib.import_module("clawbench.runner.run") + + +def _prepare_run( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + judge_model: str, +) -> tuple[ModuleType, list[tuple[Path, dict[str, Any]]], list[str]]: + """Mock out every side effect of a run; return the module, the + run-meta writes it performed, and a log of docker calls.""" + run_mod = _import_run_module(monkeypatch) + + task_dir = tmp_path / "case" + task_dir.mkdir() + (task_dir / "task.json").write_text( + json.dumps( + { + "instruction": "Do the task", + "eval_schema": {"url_pattern": "example", "method": "POST"}, + "time_limit": 1, + } + ) + ) + personal_info_dir = tmp_path / "personal-info" + personal_info_dir.mkdir() + + monkeypatch.setattr( + run_mod.sys, + "argv", + [ + "clawbench-run", + str(task_dir), + "model-a", + "--judge", + judge_model, + "--output-dir", + str(tmp_path / "out"), + "--no-build", + "--no-upload", + ], + ) + monkeypatch.setattr(run_mod, "ensure_workspace_templates", lambda: None) + monkeypatch.setattr( + run_mod, + "load_runtime_env", + lambda: { + "PURELY_MAIL_API_KEY": "pm-key", + "PURELY_MAIL_DOMAIN": "example.test", + }, + ) + + class FakeProvider: + name = "local" + + def start(self, task: dict, time_limit_s: int): + from clawbench.runner.run_support.browser_runtime.providers import ( + BrowserSession, + ) + + return BrowserSession( + provider="local", + cdp_url="http://127.0.0.1:9222", + mode="local", + recording_mode="x11", + local_viewer_port=6080, + ) + + def cleanup(self, session): + pass + + monkeypatch.setattr( + run_mod, "make_browser_runtime_provider", lambda args, env: FakeProvider() + ) + monkeypatch.setattr(run_mod, "preflight_model_api", lambda model_cfg: None) + + def fake_load_model_config(model: str) -> dict: + if model == "missing-judge-model": + raise run_mod.ModelConfigError( + f"model '{model}' not found in models.yaml. Available models: model-a" + ) + return { + "model": model, + "base_url": "https://api.example.test/v1", + "api_type": "openai-completions", + "api_key": "secret-key", + "api_keys": ["secret-key"], + } + + monkeypatch.setattr(run_mod, "load_model_config", fake_load_model_config) + monkeypatch.setattr( + run_mod, + "create_email", + lambda pm_key, pm_domain: ("agent@example.test", "email-pw"), + ) + monkeypatch.setattr(run_mod, "delete_email", lambda pm_key, email: None) + monkeypatch.setattr( + run_mod, + "prepare_personal_info", + lambda shared_root, email, email_pw, output_dir: (personal_info_dir, {}), + ) + monkeypatch.setattr(run_mod, "copy_extra_info", lambda task, task_dir, dest: []) + monkeypatch.setattr(run_mod, "build_instruction", lambda task: "Do the task") + docker_calls: list[str] = [] + monkeypatch.setattr( + run_mod, "docker_run", lambda *args, **kwargs: docker_calls.append("run") + ) + monkeypatch.setattr(run_mod, "docker_wait", lambda *args, **kwargs: None) + monkeypatch.setattr(run_mod, "docker_logs", lambda *args, **kwargs: None) + monkeypatch.setattr(run_mod, "docker_copy", lambda *args, **kwargs: None) + monkeypatch.setattr(run_mod, "docker_rm", lambda *args, **kwargs: None) + monkeypatch.setattr(run_mod, "_fix_data_ownership", lambda *args, **kwargs: None) + monkeypatch.setattr(run_mod, "ensure_interception", lambda *args, **kwargs: None) + monkeypatch.setattr( + run_mod, "print_results", lambda *args, **kwargs: True + ) # stage 1 (intercepted) passed + monkeypatch.setattr( + run_mod, "remove_transient_usage_artifact", lambda *args, **kwargs: None + ) + monkeypatch.setattr( + run_mod, + "classify_run", + lambda *args, **kwargs: { + "result_category": "pass", + "failure_category": None, + "infra_failure": False, + "adjusted_eligible": True, + "infra_flags": [], + "metrics": {}, + }, + ) + + written: list[tuple[Path, dict[str, Any]]] = [] + monkeypatch.setattr( + run_mod, + "make_run_meta", + lambda **kwargs: {"failure_reason": kwargs.get("failure_reason")}, + ) + monkeypatch.setattr( + run_mod, + "write_run_meta", + lambda output_dir, meta: written.append((output_dir, meta)), + ) + + return run_mod, written, docker_calls + + +def test_bad_judge_model_is_rejected_before_the_agent_runs( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + """Ask 1 of the issue: catch a --judge typo at startup, not 30 minutes in.""" + run_mod, written, docker_calls = _prepare_run( + monkeypatch, tmp_path, "missing-judge-model" + ) + + with pytest.raises(SystemExit) as excinfo: + run_mod.main() + + assert excinfo.value.code == 1 + assert docker_calls == [] # the agent never ran + assert written == [] # and there is no run to record + out = capsys.readouterr().out + assert "--judge" in out + assert "missing-judge-model" in out + + +def test_judge_failure_after_the_run_still_writes_run_meta( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """Ask 2, and the invariant: a completed agent run always writes metadata. + + Before the fix, a ModelConfigError surfacing here was a SystemExit, which + escaped both the judge stage's `except Exception` and run()'s top-level + handler, killing the process before write_run_meta(). + """ + run_mod, written, docker_calls = _prepare_run(monkeypatch, tmp_path, "model-a") + + import clawbench.runner.judge as judge_mod + + def explode(*args: Any, **kwargs: Any) -> dict: + raise run_mod.ModelConfigError("judge backend went away mid-run") + + monkeypatch.setattr(judge_mod, "judge_request", explode) + + with pytest.raises(SystemExit) as excinfo: + run_mod.main() + + # Inconclusive judge -> normal exit 1, not an uncaught crash. + assert excinfo.value.code == 1 + assert docker_calls == ["run"] # the agent did run + + # The core regression: its results must not be silently discarded. + assert len(written) == 1 + meta = written[0][1] + assert meta["judge_match"] is None + assert "judge_setup_failed" in meta["judge"]["reason"] + assert "went away mid-run" in meta["judge"]["reason"] + assert meta["pass"] is False + + +def test_model_config_error_is_catchable_as_a_plain_exception( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The whole point: SystemExit is a BaseException and slips past + `except Exception`. ModelConfigError must not.""" + ModelConfigError = _import_run_module(monkeypatch).ModelConfigError + + assert issubclass(ModelConfigError, Exception) + assert not issubclass(ModelConfigError, SystemExit) + + try: + raise ModelConfigError("boom") + except Exception as e: + assert "boom" in str(e) + else: # pragma: no cover + pytest.fail("ModelConfigError was not caught by `except Exception`") + + +def test_missing_models_yaml_is_also_catchable( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """load_model_config() calls load_models_yaml(). If that still exited, + a SystemExit would escape the judge stage exactly as before.""" + config = _import_run_module(monkeypatch) + cfg_mod = sys.modules["clawbench.runner.run_support.config"] + monkeypatch.setattr(cfg_mod, "MODELS_YAML", tmp_path / "absent.yaml") + + with pytest.raises(config.ModelConfigError) as excinfo: + cfg_mod.load_models_yaml() + + assert "absent.yaml" in str(excinfo.value) + + # And the same call reached through load_model_config stays catchable. + try: + cfg_mod.load_model_config("anything") + except Exception as e: + assert isinstance(e, config.ModelConfigError) + else: # pragma: no cover + pytest.fail("expected ModelConfigError") From 39c8915889404b5f1646e0cc3bbdd5c6019206d0 Mon Sep 17 00:00:00 2001 From: "Perry Z." Date: Sun, 30 Aug 2026 14:37:04 -0700 Subject: [PATCH 2/3] fix: add a dummy finalize() method to match the changed browser runtime signature --- tests/test_run_judge_stage.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_run_judge_stage.py b/tests/test_run_judge_stage.py index fac864f6..d911492f 100644 --- a/tests/test_run_judge_stage.py +++ b/tests/test_run_judge_stage.py @@ -112,6 +112,9 @@ def start(self, task: dict, time_limit_s: int): local_viewer_port=6080, ) + def finalize(self, session, output_dir): + pass + def cleanup(self, session): pass From e3e7e59182b67d097f75081662b29cb26ba600bc Mon Sep 17 00:00:00 2001 From: "Perry Z." Date: Sun, 30 Aug 2026 14:38:12 -0700 Subject: [PATCH 3/3] docs: update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07fb54e1..aa0accea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ### Fixed - Fixed an issue where malformed per-run metadata could prevent `batch-summary.json` from being written and, when configured, uploaded. +- Fixed the issue that an invalid judge model would lose the `run-meta.json` file. ## [0.9.2] - 2026-08-18 ### Added