diff --git a/agentic/harness_optimizer/core.py b/agentic/harness_optimizer/core.py index 8c9857602..b0a415cbc 100644 --- a/agentic/harness_optimizer/core.py +++ b/agentic/harness_optimizer/core.py @@ -7,6 +7,7 @@ from __future__ import annotations +import math from dataclasses import asdict, dataclass, field from enum import StrEnum @@ -152,6 +153,12 @@ def __post_init__(self) -> None: raise AgenticError("run_report pass fields must be booleans", details={"variant_id": self.variant_id}) if not isinstance(self.score, int | float) or isinstance(self.score, bool): raise AgenticError("run_report.score must be numeric", details={"variant_id": self.variant_id}) + # decide_candidate's "score_not_improved" gate is a <= comparison, which + # is False for NaN on either side: a NaN score would fail that gate open. + # Floats only: an int is always finite, and math.isfinite(10**1000) raises + # OverflowError converting it. + if isinstance(self.score, float) and not math.isfinite(self.score): + raise AgenticError("run_report.score must be finite", details={"variant_id": self.variant_id}) @property def has_critical_governance_finding(self) -> bool: diff --git a/agentic/harness_optimizer/patching.py b/agentic/harness_optimizer/patching.py index 4d42c8c05..2ce03e6aa 100644 --- a/agentic/harness_optimizer/patching.py +++ b/agentic/harness_optimizer/patching.py @@ -185,15 +185,24 @@ def apply_candidate_artifact( with _write_lock: # serialize threads within this process _acquire_artifact_lock(lock_dir) # serialize other processes too try: - previous = {} + version = 1 if artifact_path.exists(): + # Valid JSON of the wrong shape ([], null, {"version": "x"}) or a + # non-UTF-8 file is just as malformed as invalid JSON; without this + # it escaped as a raw AttributeError/ValueError, outside the typed + # AgenticError contract the CLI maps to its exit codes. try: previous = json.loads(artifact_path.read_text(encoding="utf-8")) - except json.JSONDecodeError as exc: + if not isinstance(previous, dict): + raise TypeError("artifact is not a JSON object") + prior_version = previous.get("version", 0) + if isinstance(prior_version, bool) or not isinstance(prior_version, int) or prior_version < 0: + raise TypeError("artifact version is not a non-negative integer") + except (ValueError, TypeError, UnicodeDecodeError) as exc: raise AgenticError( "existing harness artifact is malformed", details={"path": str(artifact_path)} ) from exc - version = int(previous.get("version", 0)) + 1 + version = prior_version + 1 record = { "version": version, "variant_id": proposal.variant_id, diff --git a/agentic/harness_optimizer/scoring.py b/agentic/harness_optimizer/scoring.py index 6798890c0..f0d16d89c 100644 --- a/agentic/harness_optimizer/scoring.py +++ b/agentic/harness_optimizer/scoring.py @@ -2,6 +2,7 @@ from __future__ import annotations +import math from dataclasses import dataclass from agentic.harness_optimizer.core import CandidateDecision, RunReport @@ -24,7 +25,10 @@ def __post_init__(self) -> None: raise AgenticError("case result passed must be a boolean", details={"case_id": self.case_id}) if not isinstance(self.score, int | float) or isinstance(self.score, bool): raise AgenticError("case result score must be numeric", details={"case_id": self.case_id}) - if self.score < 0.0 or self.score > 1.0: + # NaN compares False against everything, so it would slip past the + # range check below and poison score_cases()' average. Floats only: + # math.isfinite overflows on a huge int, which the range check rejects. + if (isinstance(self.score, float) and not math.isfinite(self.score)) or self.score < 0.0 or self.score > 1.0: raise AgenticError("case result score must be between 0 and 1", details={"case_id": self.case_id}) diff --git a/tests/test_agentic_harness_optimizer.py b/tests/test_agentic_harness_optimizer.py index b0504a2bf..50029d861 100644 --- a/tests/test_agentic_harness_optimizer.py +++ b/tests/test_agentic_harness_optimizer.py @@ -19,6 +19,7 @@ build_proposer_workspace, decide_candidate, ) +from agentic.harness_optimizer.scoring import CaseResult from agentic.harness_optimizer.patching import ( _acquire_artifact_lock, _release_artifact_lock, @@ -79,6 +80,27 @@ def test_candidate_rejects_no_improvement() -> None: assert "score_not_improved" in decision.rejected_gates +@pytest.mark.parametrize("bad", [float("nan"), float("inf"), float("-inf")]) +def test_run_report_rejects_non_finite_score(bad: float) -> None: + # decide_candidate's score gate is `candidate <= baseline`, which is False + # whenever either side is NaN: a NaN report must never be constructible, + # or that gate fails open and the candidate is accepted. + with pytest.raises(AgenticError, match="finite"): + RunReport("candidate", train_passed=True, holdout_passed=True, score=bad) + + +def test_run_report_accepts_a_huge_int_score_without_overflow() -> None: + # math.isfinite(10**1000) raises OverflowError; ints are always finite, so + # the finiteness check must not run on them (Codex review on this PR). + assert RunReport("candidate", train_passed=True, holdout_passed=True, score=10**1000).score == 10**1000 + + +@pytest.mark.parametrize("bad", [float("nan"), float("inf"), 10**1000]) +def test_case_result_rejects_non_finite_score(bad: float) -> None: + with pytest.raises(AgenticError, match="between 0 and 1"): + CaseResult("case-1", passed=True, score=bad) + + def test_candidate_rejects_unallowed_surface_and_visible_case_hardcoding() -> None: baseline = RunReport("baseline", train_passed=True, holdout_passed=True, score=0.40) candidate = RunReport( diff --git a/tests/test_agentic_harness_phase679.py b/tests/test_agentic_harness_phase679.py index cd5a520dc..fad0357c4 100644 --- a/tests/test_agentic_harness_phase679.py +++ b/tests/test_agentic_harness_phase679.py @@ -315,6 +315,39 @@ def test_apply_candidate_artifact_requires_all_human_gates(tmp_path: Path) -> No assert record["proposal_sha256"] == proposal.proposal_sha256 +@pytest.mark.parametrize("prior", ["[]", "null", '{"version": "x"}', '{"version": -1}', '{"version": true}']) +def test_apply_candidate_artifact_maps_malformed_prior_artifact_to_agentic_error(tmp_path: Path, prior: str) -> None: + # Valid JSON of the wrong shape used to escape as AttributeError/ValueError, + # outside the typed-error contract the agentic CLI maps to exit codes. + workspace, cfg = _workspace(tmp_path) + workspace.proposal_path.write_text("# Proposal\n\nGeneral fix.", encoding="utf-8") + decision = decide_candidate( + baseline=RunReport("baseline", train_passed=True, holdout_passed=True, score=0.1), + candidate=RunReport( + "candidate", train_passed=True, holdout_passed=True, score=0.9, changed_surfaces=("planner",) + ), + allowed_surface_ids={"planner"}, + proposal_present=True, + ) + proposal = propose_candidate_application( + decision, Variant("candidate", ("planner",), "proposal.md", str(workspace.root)), workspace, cfg=cfg + ) + config = _config(harness={"enabled": True}) + config.mode = "write" + config.writes_enabled = True + config.harness_optimizer.output_dir = str(tmp_path / "output") + config.harness_optimizer.memory_dir = str(tmp_path / "memory") + artifact = tmp_path / "output" / "accepted" / f"{proposal.variant_id}.json" + artifact.parent.mkdir(parents=True) + artifact.write_text(prior, encoding="utf-8") + + with pytest.raises(AgenticError, match="malformed"): + apply_candidate_artifact(proposal, config, reason="record fixture candidate", confirm=True, cfg=cfg) + # The malformed artifact is left as found, and the lock is released. + assert artifact.read_text(encoding="utf-8") == prior + assert not artifact.with_suffix(".json.lock.d").exists() + + def test_apply_candidate_artifact_blocked_when_lock_held(tmp_path: Path) -> None: # A held lock (another accept in progress) makes a concurrent apply refuse # rather than race the read-modify-write of the version counter.