diff --git a/pyproject.toml b/pyproject.toml index ef838ba4..38c03f38 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -63,7 +63,7 @@ dev = [ "pytest-mock>=3.15.1", "pytest-cov>=7.1.0", "pytest-xdist[psutil]>=3.0.0", - "mcp>=1.26.0", + "mcp>=1.28.1", # >=1.28.1 clears CVE-2026-52869/52870 (1.27.2) + CVE-2026-59950 (1.28.1) "ruff>=0.15.7", "pyright>=1.1.408", "pip-audit>=2.10.0", diff --git a/src/coder_eval/cli/run_command.py b/src/coder_eval/cli/run_command.py index de12513e..b35366ca 100644 --- a/src/coder_eval/cli/run_command.py +++ b/src/coder_eval/cli/run_command.py @@ -583,9 +583,13 @@ async def _run_with_experiment( default_experiment = experiment # fall back to custom as its own baseline # Resolve tasks through experiment layer (applies all 5 config layers). - # Layer-5 override failures (invalid -D value/path, sdk_options on a - # non-claude agent, the agent.type guard) and duplicate-task-id checks raise - # ValueError here; surface them as a clean CLI error instead of a traceback. + # Global failures raise ValueError here — duplicate task IDs, early-stop + # arming, or an invocation error that trips every task identically (bad + # --type / -D value, repeats over the cap) — and we surface them as a clean + # CLI error instead of a traceback. Per-task config-resolution failures + # (e.g. sdk_options on a non-claude agent) among otherwise-resolvable tasks + # are NOT raised: resolve_all_tasks isolates them into `skipped` so one + # incompatible task can't abort the whole suite. try: resolved, skipped = resolve_all_tasks( task_files=all_task_files, diff --git a/src/coder_eval/orchestration/experiment.py b/src/coder_eval/orchestration/experiment.py index 0962c362..f84a4a59 100644 --- a/src/coder_eval/orchestration/experiment.py +++ b/src/coder_eval/orchestration/experiment.py @@ -552,10 +552,18 @@ def resolve_all_tasks( Also handles tag filtering and unique task ID validation. Task YAMLs that fail to load (YAML parse error, Pydantic validation, - dataset expansion error) are recorded in the returned ``skipped`` list - and excluded from the resolved set rather than aborting the suite. The - caller surfaces ``skipped`` in the run summary so the failure is loud - but recoverable. + dataset expansion error) are recorded in the returned ``skipped`` list and + excluded from the resolved set rather than aborting the suite. + + Per-task config-resolution failures (a task whose own YAML is incompatible + with the resolved run — e.g. Claude-only ``sdk_options`` surviving a + ``--type codex`` override, which ``CodexAgentConfig`` forbids) are likewise + demoted to ``skipped`` — but only when other tasks resolve. If EVERY task + that reaches resolution fails, the cause is a global invocation error (a bad + ``--type`` / ``-D`` value, repeats over the cap) rather than a per-task + incompatibility, so it is re-raised and aborts the run. Early-stop arming + errors always propagate. The caller surfaces ``skipped`` in the run summary + so per-task failures are loud but recoverable. Args: task_files: Paths to task YAML files. @@ -572,10 +580,18 @@ def resolve_all_tasks( Raises: ValueError: If duplicate task IDs are found after resolution. """ - from .early_stop import validate_early_stop + from .early_stop import EarlyStopConfigError, validate_early_stop resolved: list[ResolvedTask] = [] skipped: list[SkippedTask] = [] + # Per-task config-resolution failures are collected here rather than raised + # inline. After the loop they are demoted to ``skipped`` — UNLESS every task + # that reached resolution failed, which signals a global invocation error + # (bad --type, an invalid -D value, repeats over the cap) that trips every + # task identically rather than a per-task incompatibility; that case is + # re-raised so the CLI aborts cleanly instead of producing an empty run. + resolution_errors: list[tuple[Path, Exception]] = [] + attempted = 0 # Resolve variant-level initial_prompt_file paths before the main loop exp_dir = experiment_file.parent if experiment_file is not None else None @@ -623,50 +639,97 @@ def resolve_all_tasks( skipped.append(SkippedTask(path=str(task_file), reason=reason)) continue - for expanded_task in expanded_tasks: - for variant in experiment.variants: - # Apply layers 1-4 (default → experiment-defaults → task → variant) + resolve repeats - resolved_task, lineage, effective_repeats = resolve_task_for_variant( - default_experiment, expanded_task, experiment, variant, config - ) + # Isolate layer 1-5 config resolution per task file. A task whose own + # YAML config is incompatible with the resolved run — e.g. Claude-only + # agent fields (`sdk_options`) surviving a `--type codex` override, which + # `CodexAgentConfig` forbids (`extra="forbid"`) — raises here. Without + # isolation that single task aborts the entire coder-eval run. Buffer the + # file's resolved tasks and commit them only once the whole file + # resolves, so a mid-file failure discards this file's fan-out as a unit + # (mirroring the load/expand isolation above) rather than leaving a + # partial, lopsided fan-out behind. + attempted += 1 + file_resolved: list[ResolvedTask] = [] + try: + for expanded_task in expanded_tasks: + for variant in experiment.variants: + # Apply layers 1-4 (default → experiment-defaults → task → variant) + resolve repeats + resolved_task, lineage, effective_repeats = resolve_task_for_variant( + default_experiment, expanded_task, experiment, variant, config + ) - # Resolve file paths injected by variant overrides - resolve_task_files(resolved_task, task_file, experiment_file) - - # Apply prompt mutations or overrides (between file resolution and CLI overrides) - _apply_prompt_overrides(resolved_task, experiment, variant, lineage) - - # Apply layer 5 (CLI overrides) - _apply_cli_overrides(resolved_task, config, lineage) - - # Early-stop guardrails: run once the task is fully resolved (all 5 - # layers merged, incl. -D run_limits.stop_early). No-op unless armed; - # a bad arming raises EarlyStopConfigError (a ValueError) which the - # run path converts to a clean CLI error. - validate_early_stop(resolved_task) - - # Fan-out: simulation n_trials takes precedence over experiment repeats - # when simulation is active; otherwise use experiment-level repeats. - sim = resolved_task.simulation - n_trials = sim.n_trials if (sim is not None and sim.enabled) else 1 - fan_count = n_trials if n_trials > 1 else effective_repeats - for rep in range(fan_count): - resolved.append( - ResolvedTask( - task=resolved_task, - task_file=task_file, - run_dir=build_task_run_dir( - config.run_dir, - variant.variant_id, - resolved_task.task_id, + # Resolve file paths injected by variant overrides + resolve_task_files(resolved_task, task_file, experiment_file) + + # Apply prompt mutations or overrides (between file resolution and CLI overrides) + _apply_prompt_overrides(resolved_task, experiment, variant, lineage) + + # Apply layer 5 (CLI overrides) + _apply_cli_overrides(resolved_task, config, lineage) + + # Early-stop guardrails: run once the task is fully resolved (all 5 + # layers merged, incl. -D run_limits.stop_early). No-op unless armed; + # a bad arming raises EarlyStopConfigError (a ValueError). + validate_early_stop(resolved_task) + + # Fan-out: simulation n_trials takes precedence over experiment repeats + # when simulation is active; otherwise use experiment-level repeats. + sim = resolved_task.simulation + n_trials = sim.n_trials if (sim is not None and sim.enabled) else 1 + fan_count = n_trials if n_trials > 1 else effective_repeats + for rep in range(fan_count): + file_resolved.append( + ResolvedTask( + task=resolved_task, + task_file=task_file, + run_dir=build_task_run_dir( + config.run_dir, + variant.variant_id, + resolved_task.task_id, + replicate_index=rep, + ), + variant_id=variant.variant_id, replicate_index=rep, - ), - variant_id=variant.variant_id, - replicate_index=rep, - source_yaml=source_yaml, - config_lineage=dict(lineage), + source_yaml=source_yaml, + config_lineage=dict(lineage), + ) ) - ) + # Early-stop arming errors are a deliberate hard stop: they always + # propagate (never demoted to skipped) so a misarmed run fails loudly. + except EarlyStopConfigError: + raise + # Narrow set, matching the load/expand block above: config-resolution + # and IO failures are collected (decided after the loop, below); + # AttributeError / TypeError / ImportError still crash loudly as + # regressions. Pydantic ValidationError is a ValueError subclass in v2, + # so it's covered. + except (FileNotFoundError, OSError, ValueError, yaml.YAMLError) as exc: + resolution_errors.append((task_file, exc)) + continue + + resolved.extend(file_resolved) + + # Decide the fate of collected per-task resolution failures. If EVERY task + # that reached resolution failed, refusing to proceed (rather than producing + # an empty run) is the right call. We surface the first task's own error — + # not a synthesized "global misconfig" message — because we can't actually + # tell a genuine global cause (a bad --type / -D value that trips every task + # identically) from N tasks each independently incompatible for the same + # per-task reason. A ValueError (incl. Pydantic ValidationError and the "no + # agent registered" guard) is re-raised verbatim so its message stays clean; + # only a non-ValueError (FileNotFoundError/OSError/yaml.YAMLError — e.g. a + # missing system_prompt_file) is normalized to ValueError so it still lands + # in the caller's `except ValueError` (clean typer.BadParameter) instead of + # escaping as a raw traceback. + if resolution_errors and len(resolution_errors) == attempted: + first_exc = resolution_errors[0][1] + if isinstance(first_exc, ValueError): + raise first_exc + raise ValueError(str(first_exc)) from first_exc + for err_file, exc in resolution_errors: + reason = f"{type(exc).__name__}: {exc}"[:500] + logger.warning("Skipping task file %s — config resolution failed: %s", err_file, reason) + skipped.append(SkippedTask(path=str(err_file), reason=reason)) # Filter by tags if config.include_tags or config.exclude_tags: diff --git a/tests/test_dataset_expansion.py b/tests/test_dataset_expansion.py index fb741d63..08693cfb 100644 --- a/tests/test_dataset_expansion.py +++ b/tests/test_dataset_expansion.py @@ -726,3 +726,173 @@ def test_duplicate_detection_still_catches_true_dupes(self, tmp_path: Path) -> N with pytest.raises(ValueError, match="Duplicate task IDs"): resolve_all_tasks([task_file, task_file2], experiment, experiment, config) + + def test_config_resolution_failure_isolated_to_skipped(self, tmp_path: Path) -> None: + """A per-task layer-5 resolution failure skips just that task, not the suite. + + `--type codex` (config.agent_type) rewrites a task whose YAML carries the + Claude-only `sdk_options` into a CodexAgentConfig, which forbids that field + (`extra="forbid"`). The incompatibility raises during layer-5 resolution; + the task must land in `skipped` while a sibling still resolves — rather than + aborting the whole coder-eval run (as it did before this isolation). + """ + from coder_eval.orchestration.config import BatchRunConfig + from coder_eval.orchestration.experiment import resolve_all_tasks + + good = { + "task_id": "plain-task", + "description": "d", + "initial_prompt": "p", + "sandbox": {"driver": "tempdir"}, + "success_criteria": [{"type": "file_exists", "path": "f.py", "description": "d"}], + } + # Valid as claude-code (sdk_options is Claude-only); becomes invalid only + # once --type codex rewrites the agent kind at layer 5. + claude_only = { + **good, + "task_id": "claude-only-task", + "agent": {"type": "claude-code", "sdk_options": {"effort": "high"}}, + } + good_file = tmp_path / "good.yaml" + good_file.write_text(yaml.dump(good)) + bad_file = tmp_path / "claude_only.yaml" + bad_file.write_text(yaml.dump(claude_only)) + + experiment = ExperimentDefinition( + experiment_id="default", + defaults=ExperimentDefaults(agent={"type": "claude-code"}), + variants=[ExperimentVariant(variant_id="v1")], + ) + # --type codex applied to every task (layer 5, highest precedence). + config = BatchRunConfig(run_dir=tmp_path / "runs", agent_type="codex") + + resolved, skipped = resolve_all_tasks([good_file, bad_file], experiment, experiment, config) + + # Sibling resolved; the incompatible task is isolated (no raise). + assert [rt.task.task_id for rt in resolved] == ["plain-task"] + assert len(skipped) == 1 + assert skipped[0].path == str(bad_file) + assert "sdk_options" in skipped[0].reason + + def test_config_resolution_failure_skips_whole_file_no_partial_fanout(self, tmp_path: Path) -> None: + """A variant that fails resolution rolls back its whole file's fan-out. + + `bad.yaml` carries Claude-only `sdk_options`: it resolves cleanly under + the claude-code variant but fails under the codex variant (which forbids + the field). Because a file's resolved tasks are buffered and committed as + a unit, the clean claude-code entry is rolled back with the failing codex + one — the file is skipped whole, never left as a lopsided partial fan-out + that would skew an A/B comparison. Sibling `good.yaml` (no sdk_options) + resolves under both variants, so the failure is isolated, not a global + abort. + """ + from coder_eval.orchestration.config import BatchRunConfig + from coder_eval.orchestration.experiment import resolve_all_tasks + + def _task(task_id: str, agent: dict[str, Any] | None = None) -> dict[str, Any]: + data: dict[str, Any] = { + "task_id": task_id, + "description": "d", + "initial_prompt": "p", + "sandbox": {"driver": "tempdir"}, + "success_criteria": [{"type": "file_exists", "path": "f.py", "description": "d"}], + } + if agent is not None: + data["agent"] = agent + return data + + good_file = tmp_path / "good.yaml" + good_file.write_text(yaml.dump(_task("good-task"))) + bad_file = tmp_path / "bad.yaml" + # Claude-only sdk_options: valid under the claude-code variant, forbidden + # once the codex variant rewrites the agent kind. + bad_file.write_text( + yaml.dump(_task("bad-task", agent={"type": "claude-code", "sdk_options": {"effort": "high"}})) + ) + + experiment = ExperimentDefinition( + experiment_id="exp", + defaults=ExperimentDefaults(agent={"type": "claude-code"}), + variants=[ + ExperimentVariant(variant_id="as-claude"), + ExperimentVariant(variant_id="as-codex", agent={"type": "codex"}), + ], + ) + config = BatchRunConfig(run_dir=tmp_path / "runs") + + resolved, skipped = resolve_all_tasks([good_file, bad_file], experiment, experiment, config) + + # good.yaml resolves for BOTH variants; bad.yaml is skipped whole — its + # clean claude-code entry is rolled back with the failing codex one. + assert {rt.task.task_id for rt in resolved} == {"good-task"} + assert sorted(rt.variant_id for rt in resolved) == ["as-claude", "as-codex"] + assert len(skipped) == 1 + assert skipped[0].path == str(bad_file) + assert "sdk_options" in skipped[0].reason + + def test_every_task_failing_resolution_aborts_as_value_error(self, tmp_path: Path) -> None: + """When EVERY attempted task fails resolution, abort rather than return empty. + + Two task files that BOTH carry Claude-only `sdk_options` under `--type + codex`: with no sibling resolving, `len(resolution_errors) == attempted`, + so the suite must not silently produce an empty run. It aborts with the + first task's own error (a Pydantic ValidationError — a ValueError — so it + surfaces verbatim through the caller's `except ValueError`), never a + silent empty resolved list. + """ + + def _claude_only(task_id: str) -> dict[str, Any]: + return { + "task_id": task_id, + "description": "d", + "initial_prompt": "p", + "sandbox": {"driver": "tempdir"}, + "success_criteria": [{"type": "file_exists", "path": "f.py", "description": "d"}], + "agent": {"type": "claude-code", "sdk_options": {"effort": "high"}}, + } + + file_a = tmp_path / "a.yaml" + file_a.write_text(yaml.dump(_claude_only("task-a"))) + file_b = tmp_path / "b.yaml" + file_b.write_text(yaml.dump(_claude_only("task-b"))) + + experiment = ExperimentDefinition( + experiment_id="default", + defaults=ExperimentDefaults(agent={"type": "claude-code"}), + variants=[ExperimentVariant(variant_id="v1")], + ) + config = BatchRunConfig(run_dir=tmp_path / "runs", agent_type="codex") + + with pytest.raises(ValueError, match="sdk_options"): + resolve_all_tasks([file_a, file_b], experiment, experiment, config) + + def test_single_file_non_value_error_surfaces_as_value_error(self, tmp_path: Path) -> None: + """The all-fail abort normalizes a non-ValueError reason to ValueError. + + A lone task file whose variant injects a missing `system_prompt_file` + raises FileNotFoundError (an OSError, not a ValueError) during layer-5 + resolution. With a single file, `len(resolution_errors) == attempted == 1` + fires the all-fail branch. It must re-raise as a ``ValueError`` so the + caller's `except ValueError` catches it (clean CLI error) instead of a + FileNotFoundError escaping as a raw traceback — while preserving the + original message ("system_prompt_file not found"). + """ + task = { + "task_id": "missing-prompt-task", + "description": "d", + "initial_prompt": "p", + "sandbox": {"driver": "tempdir"}, + "success_criteria": [{"type": "file_exists", "path": "f.py", "description": "d"}], + } + task_file = tmp_path / "task.yaml" + task_file.write_text(yaml.dump(task)) + + experiment = ExperimentDefinition( + experiment_id="exp", + defaults=ExperimentDefaults(agent={"type": "claude-code"}), + variants=[ExperimentVariant(variant_id="v1", agent={"system_prompt_file": "does-not-exist.txt"})], + ) + config = BatchRunConfig(run_dir=tmp_path / "runs") + + with pytest.raises(ValueError, match="system_prompt_file not found"): + resolve_all_tasks([task_file], experiment, experiment, config, experiment_file=tmp_path / "exp.yaml") diff --git a/uv.lock b/uv.lock index 75aa8854..ff5dfb10 100644 --- a/uv.lock +++ b/uv.lock @@ -1,6 +1,12 @@ version = 1 revision = 3 requires-python = ">=3.13" +resolution-markers = [ + "python_full_version >= '3.14' and sys_platform == 'win32'", + "python_full_version >= '3.14' and sys_platform != 'win32'", + "python_full_version < '3.14' and sys_platform == 'win32'", + "python_full_version < '3.14' and sys_platform != 'win32'", +] [manifest] constraints = [ @@ -500,7 +506,7 @@ requires-dist = [ { name = "google-antigravity", marker = "extra == 'antigravity'", specifier = "==0.1.5" }, { name = "jmespath", specifier = ">=1.1.0" }, { name = "jsonschema", specifier = ">=4.26.0" }, - { name = "mcp", marker = "extra == 'dev'", specifier = ">=1.26.0" }, + { name = "mcp", marker = "extra == 'dev'", specifier = ">=1.28.1" }, { name = "openai-codex", marker = "extra == 'codex'", specifier = ">=0.1.0b3" }, { name = "opentelemetry-sdk", specifier = ">=1.30.0,<2.0.0" }, { name = "pip-audit", marker = "extra == 'dev'", specifier = ">=2.10.0" }, @@ -1083,7 +1089,7 @@ wheels = [ [[package]] name = "mcp" -version = "1.26.0" +version = "1.28.1" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "anyio" }, @@ -1101,9 +1107,9 @@ dependencies = [ { name = "typing-inspection" }, { name = "uvicorn", marker = "sys_platform != 'emscripten'" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/fc/6d/62e76bbb8144d6ed86e202b5edd8a4cb631e7c8130f3f4893c3f90262b10/mcp-1.26.0.tar.gz", hash = "sha256:db6e2ef491eecc1a0d93711a76f28dec2e05999f93afd48795da1c1137142c66", size = 608005, upload-time = "2026-01-24T19:40:32.468Z" } +sdist = { url = "https://files.pythonhosted.org/packages/6e/77/9450b8f251a13affb6281997d0523c4615f8a8b35d0b21ff30db3a5aac9d/mcp-1.28.1.tar.gz", hash = "sha256:d51e36a5f5644faea4f85ea649bfffa6bc6c26770d42798ad6a3de3d2ba69683", size = 638501, upload-time = "2026-06-26T12:57:29.093Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/fd/d9/eaa1f80170d2b7c5ba23f3b59f766f3a0bb41155fbc32a69adfa1adaaef9/mcp-1.26.0-py3-none-any.whl", hash = "sha256:904a21c33c25aa98ddbeb47273033c435e595bbacfdb177f4bd87f6dceebe1ca", size = 233615, upload-time = "2026-01-24T19:40:30.652Z" }, + { url = "https://files.pythonhosted.org/packages/e2/5e/d118fce19f87a2e7d8101c35c8ae0ec289098a4df0ff244cec23e415aca0/mcp-1.28.1-py3-none-any.whl", hash = "sha256:2726bca5e7193f61c5dde8b12500a6de2d9acf6d1a1c0be9e8c2e706437991df", size = 222620, upload-time = "2026-06-26T12:57:27.218Z" }, ] [[package]]