From f22e43f8745b1efb8511766b3ef5f92beab54dd7 Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 23 Jul 2026 10:21:51 +0200 Subject: [PATCH 01/19] fix: make the check timeout configurable so real build gates can pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CHECK_TIMEOUT_S was a module constant with no override path, so every task's check command was killed at 60s regardless of what the task declared. A task with "timeout_s": 2400 whose check runs `dotnet build && dotnet test` was SIGTERM'd at exactly 60.0s and reported TIMEOUT — a false negative on the mechanism that decides PASS, and indistinguishable on the scoreboard from a worker that produced nothing. Reproduced on 71763e6: task timeout_s=2400, check `sleep 65 && echo ok` -> elapsed=60.0s timed_out=True rc=-15. Resolution order, highest first: 1. the task's "check_timeout_s" (new, optional) 2. config.toml "check_timeout_s" (new, optional, install-wide default) 3. DEFAULT_CHECK_TIMEOUT_S = 60 (unchanged) Every resolved value is clamped to MAX_CHECK_TIMEOUT_S = 3600 and validated at parse time, so the gate is still guaranteed to terminate. Backwards compatible: a manifest that does not opt in resolves to 60, exactly as before. CHECK_TIMEOUT_S is retained as an alias. Verifier() still constructs with no arguments. Retry behaviour is untouched — TIMEOUT remains a retryable verdict and max_attempts stays 2. The repo-feature kit now declares check_timeout_s: 1800, since its check shells out to a real build/test command; its README explains when a kit must do this. tests/test_check_timeout.py: 19 tests covering resolution precedence, parse-time validation, config loading, actual subprocess enforcement, accurate timeout reporting, and the unchanged retry contract. Plus an opt-in unscaled proof (RINGER_SLOW_TESTS=1) that a check surviving 65 real seconds now passes. Suite: 179 -> 199 tests. The 3 pre-existing failures on 71763e6 (test_design_reference x2, test_scoreboard_page x1) are unchanged. --- config.sample.toml | 17 +++ ringer.py | 73 +++++++++- templates/repo-feature/README.md | 4 + templates/repo-feature/manifest.json | 1 + tests/test_check_timeout.py | 207 +++++++++++++++++++++++++++ 5 files changed, 296 insertions(+), 6 deletions(-) create mode 100644 tests/test_check_timeout.py diff --git a/config.sample.toml b/config.sample.toml index b7476bd62..81ea616a0 100644 --- a/config.sample.toml +++ b/config.sample.toml @@ -22,6 +22,23 @@ dashboard_port_base = 8787 # still fail unless this is true. allow_full_access = false +# Install-wide default budget, in seconds, for a task's "check" command. +# A check is the gate — exit 0 is the ONLY pass — so it always has a finite +# bound and is killed (SIGTERM, then SIGKILL after 5s) when it runs over. +# +# Resolution order, highest first: +# 1. the task's own "check_timeout_s" in the manifest +# 2. this value +# 3. the built-in default, 60 +# Any resolved value is clamped to 3600 (MAX_CHECK_TIMEOUT_S). +# +# Leave this at 60 unless most of your checks are slow. A real +# `dotnet build && dotnet test`, a container start, or a browser suite needs +# more, and the right place to say so is usually the task, not this file: +# { "check": "dotnet build && dotnet test ...", "check_timeout_s": 1800 } +# Raising it here slows down how quickly a genuinely hung check surfaces. +# check_timeout_s = 60 + # Optional model steering. The directory must contain profiles/ with one # Markdown profile per model. Ringer creates observations/ringer/ beneath this # directory automatically as attempts finish. diff --git a/ringer.py b/ringer.py index aa75f845d..0537b255b 100755 --- a/ringer.py +++ b/ringer.py @@ -51,7 +51,18 @@ CONFIG_FILE_NAME = "config.toml" DEFAULT_ENGINE_NAME = "codex" DEFAULT_TIMEOUT_S = 900 -CHECK_TIMEOUT_S = 60 +# Default wall-clock budget for a task's `check` command. Kept at 60s so manifests +# that do not opt in behave exactly as before. A check that legitimately takes longer +# (a real `dotnet build && dotnet test`, a container start, a browser suite) must say +# so explicitly via the task's `check_timeout_s`, or raise the floor for a whole +# install via `check_timeout_s` in config.toml. +DEFAULT_CHECK_TIMEOUT_S = 60 +# Hard ceiling on any resolved check timeout. The gate must always terminate: a check +# is the thing that decides PASS, so an unbounded check is an unbounded run. +MAX_CHECK_TIMEOUT_S = 3600 +# Backwards-compatible alias. Prefer resolve_check_timeout(); this name is retained +# because it was the public constant before check timeouts became configurable. +CHECK_TIMEOUT_S = DEFAULT_CHECK_TIMEOUT_S DEFAULT_DASHBOARD_PORT_BASE = 8787 DEFAULT_HUD_PORT = 8700 DEFAULT_CATALOG_SOURCE = "https://openrouter.ai/api/v1/models" @@ -401,6 +412,9 @@ class AppConfig: engines: dict[str, EngineConfig] artifact: ArtifactConfig steering: SteeringConfig = field(default_factory=SteeringConfig) + # Install-wide default budget for check commands; a task's own + # `check_timeout_s` still wins. See resolve_check_timeout(). + check_timeout_s: int = DEFAULT_CHECK_TIMEOUT_S @classmethod def load(cls, path: Path | None = None) -> "AppConfig": @@ -421,6 +435,11 @@ def load(cls, path: Path | None = None) -> "AppConfig": if dashboard_port_base <= 0: raise ValueError("dashboard_port_base must be positive") hud_port = load_hud_port(data.get("hud")) + check_timeout_s = int(data.get("check_timeout_s", DEFAULT_CHECK_TIMEOUT_S)) + if check_timeout_s <= 0: + raise ValueError("check_timeout_s must be positive") + if check_timeout_s > MAX_CHECK_TIMEOUT_S: + raise ValueError(f"check_timeout_s must be <= {MAX_CHECK_TIMEOUT_S}") identity_default = optional_string(data.get("identity_default")) hud_app_path = optional_path(data.get("hud_app_path")) allow_full_access = bool(data.get("allow_full_access", False)) @@ -445,6 +464,7 @@ def load(cls, path: Path | None = None) -> "AppConfig": engines=engines, artifact=artifact_config, steering=steering_config, + check_timeout_s=check_timeout_s, ) @@ -629,6 +649,9 @@ class TaskSpec: engine: str = DEFAULT_ENGINE_NAME expect_files: tuple[str, ...] = () timeout_s: int = DEFAULT_TIMEOUT_S + # Wall-clock budget for this task's `check` command. None means "not specified" — + # the config default applies, and failing that DEFAULT_CHECK_TIMEOUT_S. + check_timeout_s: int | None = None full_access: bool = False engine_args: tuple[str, ...] = () verified: str = "" @@ -664,6 +687,19 @@ def from_obj(cls, obj: dict[str, Any]) -> "TaskSpec": timeout_s = int(obj.get("timeout_s", DEFAULT_TIMEOUT_S)) if timeout_s <= 0: raise ValueError(f"task {key}: timeout_s must be positive") + check_timeout_raw = obj.get("check_timeout_s") + check_timeout_s: int | None + if check_timeout_raw is None: + check_timeout_s = None + else: + check_timeout_s = int(check_timeout_raw) + if check_timeout_s <= 0: + raise ValueError(f"task {key}: check_timeout_s must be positive") + if check_timeout_s > MAX_CHECK_TIMEOUT_S: + raise ValueError( + f"task {key}: check_timeout_s must be <= {MAX_CHECK_TIMEOUT_S} " + "(a check is the gate; it must always terminate)" + ) engine_args = obj.get("engine_args", []) if not isinstance(engine_args, list) or not all(isinstance(item, str) for item in engine_args): raise ValueError(f"task {key}: engine_args must be a list of strings") @@ -683,6 +719,7 @@ def from_obj(cls, obj: dict[str, Any]) -> "TaskSpec": engine=engine, expect_files=tuple(str(item) for item in expect_files), timeout_s=timeout_s, + check_timeout_s=check_timeout_s, full_access=bool(obj.get("full_access", False)), engine_args=tuple(engine_args), verified=verified.strip(), @@ -7288,9 +7325,29 @@ def run_models_command(config: AppConfig, args: argparse.Namespace) -> int: return 0 +def resolve_check_timeout( + task: TaskSpec, + default_check_timeout_s: int = DEFAULT_CHECK_TIMEOUT_S, +) -> int: + """Wall-clock budget for one task's check command. + + Precedence: the task's explicit `check_timeout_s` > the install-wide default + (config.toml `check_timeout_s`) > DEFAULT_CHECK_TIMEOUT_S. Always clamped to + MAX_CHECK_TIMEOUT_S so the gate is guaranteed to terminate. + """ + resolved = task.check_timeout_s or default_check_timeout_s or DEFAULT_CHECK_TIMEOUT_S + return max(1, min(int(resolved), MAX_CHECK_TIMEOUT_S)) + + class Verifier: + def __init__(self, default_check_timeout_s: int = DEFAULT_CHECK_TIMEOUT_S) -> None: + self.default_check_timeout_s = default_check_timeout_s + async def verify(self, task: TaskSpec, taskdir: Path) -> VerifyResult: - check_returncode, check_timed_out, output = await self._run_check(task.check, taskdir) + check_timeout_s = resolve_check_timeout(task, self.default_check_timeout_s) + check_returncode, check_timed_out, output = await self._run_check( + task.check, taskdir, check_timeout_s + ) missing_files = tuple( rel for rel in task.expect_files if not self._is_nonempty_file(self._expect_file_path(taskdir, rel)) ) @@ -7328,7 +7385,11 @@ def _expect_file_path(taskdir: Path, path: str) -> Path: return candidate if candidate.is_absolute() else taskdir / candidate @staticmethod - async def _run_check(command: str, cwd: Path) -> tuple[int | None, bool, str]: + async def _run_check( + command: str, + cwd: Path, + timeout_s: int = DEFAULT_CHECK_TIMEOUT_S, + ) -> tuple[int | None, bool, str]: proc = await asyncio.create_subprocess_shell( command, cwd=str(cwd), @@ -7339,7 +7400,7 @@ async def _run_check(command: str, cwd: Path) -> tuple[int | None, bool, str]: ) timed_out = False try: - stdout, _ = await asyncio.wait_for(proc.communicate(), timeout=CHECK_TIMEOUT_S) + stdout, _ = await asyncio.wait_for(proc.communicate(), timeout=timeout_s) except asyncio.TimeoutError: timed_out = True terminate_process_group(proc) @@ -7350,7 +7411,7 @@ async def _run_check(command: str, cwd: Path) -> tuple[int | None, bool, str]: stdout, _ = await proc.communicate() output = stdout.decode("utf-8", errors="replace") if stdout else "" if timed_out: - output += f"\n[ringer.py] check timed out after {CHECK_TIMEOUT_S}s\n" + output += f"\n[ringer.py] check timed out after {timeout_s}s\n" return proc.returncode, timed_out, output @@ -7394,7 +7455,7 @@ def __init__( else None ) self.logger = EvalLogger(config.eval) - self.verifier = Verifier() + self.verifier = Verifier(default_check_timeout_s=config.check_timeout_s) self.semaphore = asyncio.Semaphore(manifest.max_parallel) self.active_processes: dict[int, asyncio.subprocess.Process] = {} diff --git a/templates/repo-feature/README.md b/templates/repo-feature/README.md index 6837aa725..8f77c8dff 100644 --- a/templates/repo-feature/README.md +++ b/templates/repo-feature/README.md @@ -36,6 +36,10 @@ The check verifies four things: `notes.md` exists in the scratch task directory, This cannot be gamed by creating a loose artifact in the task directory because the real repo command executes in `{{REPO_PATH}}` and the git porcelain check catches unrelated edits. +### Check budget + +Because the check runs a real build or test suite, it declares `"check_timeout_s": 1800` rather than relying on the 60-second default. A check is killed when it exceeds its budget, and a killed check reports `TIMEOUT` — indistinguishable, on the scoreboard, from a worker that produced nothing. Any kit whose check shells out to a compiler, a test runner, a container, or a browser must set this. Tune it to your suite: keep it as low as the slowest honest run allows, so a genuinely hung check still surfaces quickly. + ## Mix with Use `launch-kit` before this when a standalone launch page needs to be installed into a Next.js or React repo. Use `asset-swarm` after this when the new route should be captured as real footage. Use `adversarial-review` before merge when the repo change touches auth, billing, data access, or high-visibility UI. diff --git a/templates/repo-feature/manifest.json b/templates/repo-feature/manifest.json index 7b184fe3d..8e37e8bcb 100644 --- a/templates/repo-feature/manifest.json +++ b/templates/repo-feature/manifest.json @@ -8,6 +8,7 @@ "engine": "{{ENGINE_BUILD}}", "task_type": "code-feature", "timeout_s": 2400, + "check_timeout_s": 1800, "expect_files": [ "notes.md" ], diff --git a/tests/test_check_timeout.py b/tests/test_check_timeout.py new file mode 100644 index 000000000..f266cf1f1 --- /dev/null +++ b/tests/test_check_timeout.py @@ -0,0 +1,207 @@ +#!/usr/bin/env python3 +"""Check-timeout budget: per-task override, config default, hard ceiling. + +Regression cover for the defect where CHECK_TIMEOUT_S was a module constant with +no override path, so any check that legitimately ran longer than 60s (a real +`dotnet build && dotnet test`, a container start, a browser suite) was SIGTERM'd +and reported as TIMEOUT — a false negative on the gate that decides PASS. + +The tests deliberately use short, scaled timings (fractions of a second) rather +than sleeping past a literal 60s: the property under test is "the budget that is +enforced is the *resolved* one", not the numeric value of the default. +""" +from __future__ import annotations + +import asyncio +import os +import sys +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT)) + +from ringer import ( # noqa: E402 + DEFAULT_CHECK_TIMEOUT_S, + MAX_CHECK_TIMEOUT_S, + AppConfig, + TaskSpec, + Verifier, + resolve_check_timeout, + verdict_for, + WorkerResult, +) + +LONG_SPEC = ( + "Create the requested artifact in the current working directory, keep the change scoped, " + "and make the check command able to explain any failure clearly." +) + + +def task(**overrides) -> TaskSpec: + base = dict(key="t", spec=LONG_SPEC, check="true") + base.update(overrides) + return TaskSpec(**base) + + +class ResolveCheckTimeoutTests(unittest.TestCase): + def test_default_is_unchanged_when_nothing_opts_in(self): + """Backward compatibility: an untouched manifest behaves exactly as before.""" + self.assertEqual(60, DEFAULT_CHECK_TIMEOUT_S) + self.assertEqual(DEFAULT_CHECK_TIMEOUT_S, resolve_check_timeout(task())) + + def test_task_field_wins_over_config_default(self): + self.assertEqual(1800, resolve_check_timeout(task(check_timeout_s=1800), 300)) + + def test_config_default_applies_when_task_is_silent(self): + self.assertEqual(300, resolve_check_timeout(task(), 300)) + + def test_resolution_is_clamped_to_the_hard_ceiling(self): + """The gate must always terminate — no resolved value may exceed the ceiling.""" + self.assertEqual(MAX_CHECK_TIMEOUT_S, resolve_check_timeout(task(), MAX_CHECK_TIMEOUT_S * 10)) + + def test_zero_or_negative_config_default_falls_back_rather_than_disabling_the_gate(self): + self.assertEqual(DEFAULT_CHECK_TIMEOUT_S, resolve_check_timeout(task(), 0)) + + +class TaskSpecParsingTests(unittest.TestCase): + def test_check_timeout_is_absent_by_default(self): + parsed = TaskSpec.from_obj({"key": "t", "spec": LONG_SPEC, "check": "true"}) + self.assertIsNone(parsed.check_timeout_s) + + def test_check_timeout_is_parsed(self): + parsed = TaskSpec.from_obj( + {"key": "t", "spec": LONG_SPEC, "check": "true", "check_timeout_s": 1800} + ) + self.assertEqual(1800, parsed.check_timeout_s) + + def test_non_positive_check_timeout_is_rejected(self): + with self.assertRaisesRegex(ValueError, "check_timeout_s must be positive"): + TaskSpec.from_obj( + {"key": "t", "spec": LONG_SPEC, "check": "true", "check_timeout_s": 0} + ) + + def test_check_timeout_above_ceiling_is_rejected_at_parse_time(self): + with self.assertRaisesRegex(ValueError, "check_timeout_s must be <="): + TaskSpec.from_obj( + { + "key": "t", + "spec": LONG_SPEC, + "check": "true", + "check_timeout_s": MAX_CHECK_TIMEOUT_S + 1, + } + ) + + +class ConfigTests(unittest.TestCase): + def _config(self, body: str) -> AppConfig: + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / "config.toml" + path.write_text(body, encoding="utf-8") + return AppConfig.load(path) + + def test_config_default_is_60_when_unset(self): + self.assertEqual(DEFAULT_CHECK_TIMEOUT_S, self._config("").check_timeout_s) + + def test_config_can_raise_the_install_wide_default(self): + self.assertEqual(1800, self._config("check_timeout_s = 1800\n").check_timeout_s) + + def test_config_rejects_a_value_above_the_ceiling(self): + with self.assertRaisesRegex(ValueError, "check_timeout_s must be <="): + self._config(f"check_timeout_s = {MAX_CHECK_TIMEOUT_S + 1}\n") + + +class VerifierEnforcementTests(unittest.TestCase): + """The resolved budget is the one actually enforced against the subprocess.""" + + def _verify(self, spec: TaskSpec, default: int = DEFAULT_CHECK_TIMEOUT_S): + with tempfile.TemporaryDirectory() as tmp: + verifier = Verifier(default_check_timeout_s=default) + return asyncio.run(verifier.verify(spec, Path(tmp))) + + def test_check_outliving_the_old_hardcoded_default_is_permitted(self): + """Requirement 1: a check may legitimately run past the default budget. + + Scaled: the check sleeps beyond a *small* default (0.2s) while its own + task-level budget (10s) permits it. This is the exact shape of a .NET + build outliving 60s under a 1800s check_timeout_s. + """ + result = self._verify( + task(check="sleep 1 && echo built && exit 0", check_timeout_s=10), + default=1, + ) + self.assertFalse(result.check_timed_out, "check was killed despite its own larger budget") + self.assertTrue(result.ok) + self.assertEqual(0, result.check_returncode) + self.assertIn("built", result.raw_output_excerpt) + + def test_genuinely_hung_check_is_terminated(self): + """Requirement 2: an unbounded check is still killed — the gate terminates.""" + result = self._verify(task(check="sleep 600", check_timeout_s=1)) + self.assertTrue(result.check_timed_out) + self.assertFalse(result.ok) + + def test_timeout_is_reported_with_the_resolved_value_not_the_constant(self): + """Requirement 3: output must name the budget that was actually enforced.""" + result = self._verify(task(check="sleep 600", check_timeout_s=1)) + self.assertIn("check timed out after 1s", result.raw_output_excerpt) + self.assertNotIn(f"after {DEFAULT_CHECK_TIMEOUT_S}s", result.raw_output_excerpt) + + def test_config_default_is_enforced_when_task_is_silent(self): + result = self._verify(task(check="sleep 600"), default=1) + self.assertTrue(result.check_timed_out) + self.assertIn("check timed out after 1s", result.raw_output_excerpt) + + def test_fast_check_still_fails_fast_and_is_not_blocked_by_a_large_budget(self): + """Requirement 4 (guard): raising the ceiling must not slow a normal failure.""" + result = self._verify(task(check="echo nope; exit 3", check_timeout_s=1800)) + self.assertFalse(result.check_timed_out) + self.assertFalse(result.ok) + self.assertEqual(3, result.check_returncode) + + +@unittest.skipUnless( + os.environ.get("RINGER_SLOW_TESTS") == "1", + "slow: set RINGER_SLOW_TESTS=1 to run the literal >60s proof", +) +class SlowRealTimeoutTests(unittest.TestCase): + """The literal requirement, unscaled: a check may exceed 60 real seconds. + + This is the shape of the original defect — before the fix, ANY check running + past the hardcoded 60s was killed regardless of what the task declared. + Opt-in because it costs ~65s of wall clock. + """ + + def test_check_running_past_sixty_real_seconds_completes(self): + with tempfile.TemporaryDirectory() as tmp: + spec = task(check="sleep 65 && echo 'build+test done' && exit 0", check_timeout_s=1800) + result = asyncio.run(Verifier().verify(spec, Path(tmp))) + self.assertFalse(result.check_timed_out) + self.assertTrue(result.ok) + self.assertIn("build+test done", result.raw_output_excerpt) + + +class RetryContractTests(unittest.TestCase): + """Retry behaviour must be untouched by this change.""" + + def test_timeout_still_maps_to_the_timeout_verdict(self): + with tempfile.TemporaryDirectory() as tmp: + verify = asyncio.run( + Verifier(default_check_timeout_s=1).verify(task(check="sleep 600"), Path(tmp)) + ) + worker = WorkerResult(returncode=0, timed_out=False, tokens=None, error=None) + # TIMEOUT is a retryable verdict in _run_task; the mapping must be stable. + self.assertEqual("TIMEOUT", verdict_for(worker, verify)) + + def test_check_failure_still_maps_to_fail(self): + with tempfile.TemporaryDirectory() as tmp: + verify = asyncio.run( + Verifier().verify(task(check="echo bad; exit 1"), Path(tmp)) + ) + worker = WorkerResult(returncode=0, timed_out=False, tokens=None, error=None) + self.assertEqual("FAIL", verdict_for(worker, verify)) + + +if __name__ == "__main__": + unittest.main() From a755d1e0ac1c944a9a2d475516f4e2a7feb7e965 Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 30 Jul 2026 10:45:50 +0200 Subject: [PATCH 02/19] docs+sandbox: RINGER_EXTRA_WRITABLE, and a scoreboard correction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Uncommitted locally since 2026-07-23. engines/opencode-sandboxed.sh: RINGER_EXTRA_WRITABLE (colon-separated absolute dirs) widens the writable set. The .NET SDK writes NuGet caches, ~/.dotnet and MSBuild node state outside the repo; when those writes are denied MSBuild does not error, it HANGS with no output — undiagnosable from inside a worker. Entries are passed as -D params like every other path and never interpolated into the profile text, so the rule-injection guarantee still holds. docs/MODEL-NOTES.md: codex run record for hospedo phase-5c. The single FAIL was the check's bug, not the model's — the gate grepped \b85\b while C# writes 85m, and no word boundary exists between a digit and m, so a correct implementation could never pass. Recorded so the scoreboard is not read as evidence against codex on code-feature. Co-Authored-By: Claude Opus 5 (1M context) --- docs/MODEL-NOTES.md | 116 ++++++++++++++++++++++++++++++++++ engines/opencode-sandboxed.sh | 40 +++++++++++- 2 files changed, 155 insertions(+), 1 deletion(-) diff --git a/docs/MODEL-NOTES.md b/docs/MODEL-NOTES.md index fbc200348..5532d8ec4 100644 --- a/docs/MODEL-NOTES.md +++ b/docs/MODEL-NOTES.md @@ -311,3 +311,119 @@ checks and raw logs support — no vibes, no worker self-reports. ## opencode / z-ai glm-5.2 (via openrouter) - 2026-07-09 (aicred-invoice-downloads, 4 code-fix tasks + 1 follow-up, worktrees+npm ci checks): systematic attempt-1 NO-OP — all 4 parallel workers produced zero edits and no summary on first attempt, then completed cleanly on attempt 2 after retry-prompt injection (34k-69k tokens each). Follow-up single task passed attempt 1. Suspect first-invocation session warm-up in opencode-sandboxed under parallel spawn; budget for 2 attempts on parallel GLM batches. Output quality on Next.js/Stripe route+test work: solid, spec-faithful, one boss-caught design gap (used user-scoped supabase client where RLS demanded service role — spec didn't say explicitly; say it explicitly). + +### codex (codex-cli 0.137.0-alpha.4) — 2026-07-22, hospedo phase-5c pricing + +- `code-feature` x3, `code-fix` x1. 4/4 eventually passing; 3/4 first-try. +- **The one FAIL was my check's bug, not the model's.** `w54-backfill` was marked + fail after 2 attempts because the gate grepped `\b85\b` for expected rates, and + C# writes them as decimal literals (`85m`) — no word boundary between a digit + and `m`, so a correct implementation could never satisfy it. The gate also + flagged the negative guard (`Assert.DoesNotContain(... 110m or 95m)`) that the + spec itself had *required*. Work was correct on attempt 1. Read the scoreboard + accordingly: do not treat this as evidence against codex on code-feature. +- Lesson for future checks: never word-boundary-match numerics in C#; allow an + optional `[mMdDfF]` suffix. And exempt negative/guard assertions before + banning a literal. +- Recurring environment note: codex's sandbox cannot bind VSTest's local TCP + listener (`SocketException (13): Permission denied`), so it can only ever + self-verify by `dotnet build`, never `dotnet test`. Every run reported + build-only verification. The executed check (running outside that sandbox) is + therefore doing all real verification here — do not relax it on the assumption + the worker ran the suite. + +### work#233 model bake-off — 2026-07-25, hospedo payments-05 tier refund (`dotnet-feature`) + +Controlled 3-seed comparison on one frozen commit, identical spec/check/timeouts; +only engine+model differed. Full write-up: +`mulhacenlabs-engineering/outputs/engineering-experiments/2026-07-23-model-bakeoff/02-results.md`. + +**⚠️ SCOREBOARD CORRECTION — read this before trusting `ringer.py models` for +`dotnet-feature`.** Four rows in the local scoreboard are INFRASTRUCTURE failures +recorded as model failures. Ringer has no `infra_error` concept, so they land as +0.00 pass-rate against the model: + +- **GPT-5.5 (codex), 2 failed** — a live OpenAI incident ("Elevated error rates", + APIs/ChatGPT/Codex; `503 … biscuit_baker_service_me_circuit_open`, 30 and 42 + occurrences). The worker never reached the code. NOT evidence against GPT-5.5. +- **Kimi K2.7 Code, 2 failed** — the gate's asserter script was an UNTRACKED file in + the monorepo; a concurrent session switched branches and deleted it mid-run. One of + those runs produced a **correct** implementation (4 passed / 2 skipped / 6 total, all + three named tier examples green, 610 website tests) and was still marked FAIL. + Proven: re-running that run's exact check against its preserved worktree exits 0. + NOT evidence against Kimi. + +Real results, measured after the rig was isolated (dedicated worktree pinned at +BASE_SHA + checks owned by the experiment package): + +- **`openrouter/z-ai/glm-5.2` — 3/3, all FIRST-TRY.** 434.6 / 662.3 / 837.7s. + Modified exactly the 4 owned files every time, *including* the Gherkin step + bindings, and added tests rather than deleting any (609/617/605 vs a 605 floor). + Implementation quality spot-checked and genuinely correct — cumulative idempotency + guard, correct tier boundary, no rail call on a 0% tier, honest out-of-scope notes. + Strong pick for `dotnet-feature`. +- **`openrouter/moonshotai/kimi-k2.7-code` — 1/3, 0 first-try.** Fails the same way + twice: writes plausible domain logic, never wires it to the acceptance criteria. + Seed 2 touched 2/4 files (no step bindings); seed 3 touched 1/4 (10 lines). + Monotonic disengagement (63 → 38 → 25 steps) and missed the stated `notes.md` + output contract on 2/3 seeds. **Do not route `dotnet-feature` work to it.** + +**Token accounting — the configured `token_regex` is wrong by ~20×.** +`token_regex = '"tokens":\{"total":([0-9]+)'` matches the FIRST `step-finish` event +only; opencode emits one per step. Measured: regex 17,677 vs actual 391,051 fresh. +But summing per-step `total` is ALSO wrong — `total = input+output+reasoning+cache.read`, +so the sum is dominated by cache re-reads (2,957,831 vs 391,051 fresh on the same run). +Use `input+output+reasoning`. Extractor: the experiment package's `tools/engine_tokens.py`. + +**Cost:** opencode's self-reported cost runs ~34% HIGH ($0.903 claimed vs $0.673 +billed) because it prices cache reads at fresh-input rates. Ground truth is +`GET https://openrouter.ai/api/v1/key` → `data.usage`, snapshotted either side of a run. + +**Cost variance is a caching artefact, not a model property.** Identical model, task +and prompt across three seeds gave a **1.8× cost spread** ($0.673 → $1.197), driven +entirely by provider-side prompt-cache hit rate (seed 3 got half the cache reads and +double the fresh input). Treat "cost per success" on a single seed as noise. + +**Plan-billed vs metered is not comparable.** codex on plan is $0 marginal; GLM at +~$0.89/run is strictly more expensive. The argument for a second lane is availability +and concurrency — proven today when OpenAI went down and the OpenRouter lanes kept +working — not cost. + +**Evidence gap:** when a task passes on attempt 2, why attempt 1 failed is +unrecoverable — only the final attempt's `check_output_tail` is stored. + +#### work#233 addendum — candidate A measured, 2026-07-25 (same `dotnet-feature` task) + +The OpenAI incident cleared, so the baseline arm was re-run in full. **codex / gpt-5.5 +(effort medium, plan-billed): 3/3 PASS, all first-try**, 781.1s / 2098.2s / 808.4s +(median 808.4s). $0.00 marginal — proven by a byte-identical OpenRouter `usage` reading +before the first seed and after the last. Supersedes the two outage rows: GPT-5.5 is +**not** 0.00 on this task type. + +Final three-way, one frozen commit, identical spec/check/timeouts: + +| lane | pass | first-try | median | cost/success | +|---|---|---|---|---| +| codex gpt-5.5 (plan) | 3/3 | 3/3 | 808.4s | $0.00 | +| `openrouter/z-ai/glm-5.2` | 3/3 | 3/3 | 662.3s | $0.894 | +| `openrouter/moonshotai/kimi-k2.7-code` | 1/3 | 0/3 | 925.0s | $1.757 | + +**The cheap lane is not cheaper.** codex on plan is $0 marginal, so GLM is strictly more +expensive per success. The defensible reasons for a second lane are availability (the +outage removed codex for a day while OpenRouter kept working), concurrency beyond plan +limits, and scope discipline — GLM touched exactly 4 files on every seed, codex touched +5/6/4 for the same gate outcome. + +**⚠️ codex cannot self-verify .NET work, and it costs real wall-clock.** a-2 spent ~22 of +its 35 minutes sitting through five-minute MSBuild timeouts inside its own sandbox +("*the command is still alive and repeating the same sandbox MSBuild failure*"). Same +hang recorded on the 2026-07-23 run, and consistent with the VSTest-listener note above. +It still passed 3/3 — because Ringer's check runs UNSANDBOXED and did the real +verification. Consequences: (1) do not relax an executed check assuming the worker ran +the suite — for codex it demonstrably did not; (2) roughly half of GLM's *mean* speed +advantage is this defect, not model speed (medians: 662s vs 808s, ~18%; means: 645s vs +1229s, ~48%). Do not encode the mean into any routing rule. + +**codex token capture is unusable.** The engine's `token_regex` matched nothing at all in +today's logs, yet Ringer still recorded `tokens: 99 / 144 / 143`. Those numbers are not +traceable to any worker output. Treat codex token counts on the scoreboard as noise. diff --git a/engines/opencode-sandboxed.sh b/engines/opencode-sandboxed.sh index b9262cd38..30c6f2ec4 100755 --- a/engines/opencode-sandboxed.sh +++ b/engines/opencode-sandboxed.sh @@ -10,6 +10,15 @@ # Usage (as a ringer engine bin): # opencode-sandboxed.sh [--no-sandbox] # +# RINGER_EXTRA_WRITABLE (optional, colon-separated absolute dirs) widens the +# writable set. Some toolchains cannot build with only the task dir writable: +# the .NET SDK writes NuGet caches, ~/.dotnet and MSBuild node state outside the +# repo, and when those writes are denied MSBuild does not error — it HANGS with +# no output, which a worker cannot diagnose. Grant the minimum, e.g. +# RINGER_EXTRA_WRITABLE="$HOME/.nuget:$HOME/.dotnet" +# Entries are passed as -D params like every other path, never interpolated into +# the profile text, so the rule-injection guarantee below still holds. +# # The first argument is the task directory (pass "{taskdir}" first in # args_template). "--no-sandbox" as the second argument skips Seatbelt entirely # — wire it as the engine's full_access_args so ringer's allow_full_access gate @@ -48,6 +57,31 @@ PROFILE="$(mktemp -t ringer-opencode-prof)" cleanup() { rm -rf "$SCRATCH" "$PROFILE"; } trap cleanup EXIT +# Optional extra writable roots (see RINGER_EXTRA_WRITABLE in the header). Each +# entry becomes its own -D param + rule; the loop only ever emits the fixed text +# (subpath (param "EXTRA_")), so a path can still never inject a rule. +EXTRA_RULES="" +EXTRA_DEFS=() +if [ -n "${RINGER_EXTRA_WRITABLE:-}" ]; then + extra_i=0 + while IFS= read -r extra_raw; do + [ -n "$extra_raw" ] || continue + if [ ! -d "$extra_raw" ]; then + echo "opencode-sandboxed.sh: RINGER_EXTRA_WRITABLE entry is not a directory: $extra_raw" >&2 + exit 1 + fi + # Canonicalise: Seatbelt subpath matching needs the real path (/var/folders + # is a symlink to /private/var/folders) or writes EPERM-crash at runtime. + extra_real="$(cd "$extra_raw" && pwd -P)" + EXTRA_RULES="$EXTRA_RULES + (subpath (param \"EXTRA_$extra_i\"))" + EXTRA_DEFS+=(-D "EXTRA_$extra_i=$extra_real") + extra_i=$((extra_i + 1)) + done < "$PROFILE" <<'SBEOF' @@ -59,7 +93,10 @@ cat > "$PROFILE" <<'SBEOF' (subpath (param "SCRATCH")) (subpath (param "OC_SHARE")) (subpath (param "OC_STATE")) - (subpath (param "OC_CONFIG"))) + (subpath (param "OC_CONFIG")) +SBEOF +printf '%s\n )\n' "$EXTRA_RULES" >> "$PROFILE" +cat >> "$PROFILE" <<'SBEOF' ; /dev is needed for /dev/null, /dev/urandom, etc.; writes there can't create ; persistent files without root, so a few literals are allowed rather than via param. (allow file-write-data @@ -81,6 +118,7 @@ set +e -D "OC_SHARE=$HOME/.local/share/opencode" \ -D "OC_STATE=$HOME/.local/state/opencode" \ -D "OC_CONFIG=$HOME/.config/opencode" \ + ${EXTRA_DEFS[@]+"${EXTRA_DEFS[@]}"} \ -f "$PROFILE" "$OPENCODE_BIN" "$@" < /dev/null status=$? set -e From b8794cbf13f398a7d13718f9a60a31cb58e0e839 Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 10 Sep 2026 03:37:34 +0200 Subject: [PATCH 03/19] Count what a run really costs, and let it stop itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A run's cost was effectively invisible. `worker_tokens` records a single step rather than the sum across steps, so the figure Ringer reports — and every cost estimate built on it — reads far below the truth. Measured on real tasks the gap is 19x to 40x. That is not a rounding problem. A fix swarm that cost $41.71 reported as roughly $3, and was restarted sixteen times over two days by an operator who had no way to see otherwise. Its most expensive single restart spent $19.43 and failed 31 of 36 tasks, because the manifest asked every worker to write a deliverable to a path its sandbox forbids. Nothing noticed the identical failures, nothing capped the spend, and the run only stopped when the provider's own key limit ran out — which takes every concurrent run down with it and cannot tell a productive run from a looping one. Providers already report the price of each model call in the worker's JSON stream, so: - parse_step_costs() sums that per task. It is exact: no price catalog, no assumption about the prompt/completion split, and it stays correct when a provider discounts or caches. An engine that reports no cost yields None rather than zero, so plan-billed work is not silently counted as free. - `budget_usd` is a hard ceiling, checked the moment each worker exits and before its check runs, so a slow verification cannot overshoot it. - `abort_after_repeated_failures` stops a run when N tasks fail in a row with the same signature — the shape an impossible manifest makes. - lint refuses a spec that orders the worker to write an absolute path outside its task directory. An absolute path in expect_files stays legal when the check produces it, which is the fix-swarm export pattern, so only the spec naming the path is flagged. - the summary prints per-task and per-run USD alongside step counts. All three controls default to off; existing manifests are unaffected. Tests provoke each gate on purpose, including the two cases that must NOT fire, and parse_step_costs is verified against real worker logs where it reproduces the provider's figures exactly ($0.463 / $0.093 / $0.068). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0125QPzJL8qBQsayptsvuPgB --- README.md | 40 +++++++ ringer.py | 227 ++++++++++++++++++++++++++++++++++++- tests/test_cost_control.py | 175 ++++++++++++++++++++++++++++ 3 files changed, 438 insertions(+), 4 deletions(-) create mode 100644 tests/test_cost_control.py diff --git a/README.md b/README.md index e82fb62c6..c78f3e39e 100644 --- a/README.md +++ b/README.md @@ -163,6 +163,46 @@ lint: clean (1 tasks) A check that cannot fail is trusting the worker with extra steps. +### Budget: stop a run before it empties the key + +A run's real cost lives in the worker's own log: engines that stream JSON report +what the provider charged for every model call. Ringer sums that and prints it, +per task and per run. + +This is not the same number as `tokens` in the scoreboard. `worker_tokens` +records a *single* step, so it cannot be used for money — measured on real work +it ran 19x to 40x below the truth, which is how a $41.71 swarm reported as +roughly $3 and got restarted sixteen times. + +Two manifest keys stop a run that is going wrong: + +```json +{ + "budget_usd": 6.00, + "abort_after_repeated_failures": 3 +} +``` + +- **`budget_usd`** — a hard ceiling. Cost is totted up the moment each worker + exits, before its check runs, so a slow verification cannot overshoot it. When + the ceiling is hit, in-flight workers are terminated and queued tasks are + marked `SKIPPED` with the reason. +- **`abort_after_repeated_failures`** — stop once this many tasks in a row fail + with the *same* signature (check exit code plus the first line it printed). A + manifest asking for something no worker can produce fails every task + identically; without this the run pays for all of them, and pays twice because + each failure is retried. Different failures do not trip it — that is an + ordinary bad run, not an impossible manifest. + +Both default to off, so existing manifests behave exactly as before. + +`lint` also refuses a manifest whose **spec** tells the worker to write an +absolute path outside its own task directory. A worker may only write inside its +task directory and its assigned temp dir. An absolute path in `expect_files` is +perfectly normal when the *check* produces it — the fix-swarm pattern exports a +patch out of the worktree that way — so only the spec naming the path is +flagged. + ### Baseline: prove your checks before spending tokens Lint reads the manifest; `--baseline` executes it — every task's `check` runs against the unmodified tree, spawning no workers and writing no eval rows: diff --git a/ringer.py b/ringer.py index 4df027ab2..d70c7517f 100755 --- a/ringer.py +++ b/ringer.py @@ -1720,6 +1720,13 @@ class Manifest: repo: Path | None tasks: tuple[TaskSpec, ...] source_path: Path | None = None + # Hard ceiling on what this run may spend, in USD, enforced while it runs. + # None means unlimited, which is the historical behaviour. + budget_usd: float | None = None + # Stop the run once this many tasks have failed in a row with the SAME + # failure signature. A manifest whose deliverable is impossible fails every + # task identically; without this the run pays for all of them, twice. + abort_after_repeated_failures: int | None = None @classmethod def from_path(cls, path: Path) -> "Manifest": @@ -1735,6 +1742,8 @@ def from_path(cls, path: Path) -> "Manifest": repo=manifest.repo, tasks=manifest.tasks, source_path=path, + budget_usd=manifest.budget_usd, + abort_after_repeated_failures=manifest.abort_after_repeated_failures, ) @classmethod @@ -1761,6 +1770,18 @@ def from_obj(cls, obj: dict[str, Any]) -> "Manifest": duplicates = sorted({key for key in keys if keys.count(key) > 1}) if duplicates: raise ValueError(f"duplicate task keys: {', '.join(duplicates)}") + budget_raw = obj.get("budget_usd") + budget_usd: float | None = None + if budget_raw is not None: + budget_usd = float(budget_raw) + if budget_usd <= 0: + raise ValueError("budget_usd must be positive") + abort_raw = obj.get("abort_after_repeated_failures") + abort_after: int | None = None + if abort_raw is not None: + abort_after = int(abort_raw) + if abort_after <= 0: + raise ValueError("abort_after_repeated_failures must be positive") worktrees = bool(obj.get("worktrees", False)) if worktrees: reserved_logs_dir = (workdir / "logs").resolve() @@ -1781,6 +1802,8 @@ def from_obj(cls, obj: dict[str, Any]) -> "Manifest": worktrees=worktrees, repo=repo, tasks=tasks, + budget_usd=budget_usd, + abort_after_repeated_failures=abort_after, ) def with_max_parallel(self, value: int | None) -> "Manifest": @@ -1796,12 +1819,49 @@ def with_max_parallel(self, value: int | None) -> "Manifest": repo=self.repo, tasks=self.tasks, source_path=self.source_path, + budget_usd=self.budget_usd, + abort_after_repeated_failures=self.abort_after_repeated_failures, ) FILE_TEST_OPS = {"-e", "-f", "-s", "-d", "-r", "-w", "-x", "-L"} +def worker_unwritable_paths(task: TaskSpec, manifest: Manifest) -> list[str]: + """Absolute paths a spec orders the worker to write that its sandbox refuses. + + A worker may write inside its own task directory and its assigned temp dir, + and nowhere else. A spec naming an absolute path outside that -- and listing + it in expect_files, so the file is genuinely expected from the worker rather + than merely mentioned -- describes an impossible task. Every attempt fails, + and every attempt is retried. + + The distinction that matters: an absolute expect_files entry is perfectly + normal when the CHECK produces it (the fix-swarm pattern exports a patch out + of the worktree that way). It is only wrong when the SPEC hands that path to + the worker. So the spec text is what decides. + + Found the hard way: a scout worker located its answer in four tool calls and + then spent about forty more trying `write`, `cat >`, `dd`, `cp`, python and + `xattr` against a path it was never allowed to touch. + """ + if not task.expect_files: + return [] + taskdir = (manifest.workdir / task.key).resolve() + offenders: list[str] = [] + for raw in task.expect_files: + path = str(raw) + if not os.path.isabs(path): + continue + resolved = Path(path).resolve() + if resolved == taskdir or taskdir in resolved.parents: + continue + if path not in task.spec: + continue + offenders.append(path) + return offenders + + def lint_manifest( manifest: Manifest, *, @@ -1821,6 +1881,12 @@ def lint_manifest( findings.append( f"{task.key}: check may fail without printing why; retry prompt and eval log depend on failure output." ) + for unreachable in worker_unwritable_paths(task, manifest): + findings.append( + f"{task.key}: the spec tells the worker to write {unreachable}, which its " + f"sandbox forbids -- a worker may only write inside its own task directory. " + f"Have the worker write a relative path and export it in the check." + ) if manifest.worktrees and any(is_relative_expect_file(path) for path in task.expect_files): findings.append( f"{task.key}: deliverable would be deleted with the worktree; write it outside the worktree or export it in the check." @@ -2084,6 +2150,11 @@ class TaskRuntime: ended_at_monotonic: float | None = None worker_pid: int | None = None tokens: int | None = None + # What the provider actually charged for this task, summed across every + # model step in its log, and how many steps that took. `tokens` above is a + # single step and cannot be used for money -- see parse_step_costs. + cost_usd: float | None = None + model_steps: int = 0 final_verdict: str | None = None last_check_returncode: int | None = None last_check_timed_out: bool = False @@ -8714,6 +8785,11 @@ def __init__( self.verifier = Verifier() self.semaphore = asyncio.Semaphore(manifest.max_parallel) self.active_processes: dict[int, asyncio.subprocess.Process] = {} + # Set once when the run must stop early: budget spent, or the same + # failure repeating. Tasks that have not started check it and skip. + self.stop_reason: str | None = None + self.recent_failure_signature: str | None = None + self.repeated_failures: int = 0 async def run(self) -> int: self.manifest.workdir.mkdir(parents=True, exist_ok=True) @@ -8764,8 +8840,79 @@ async def kill_all_workers(self) -> None: if proc.returncode is None: kill_process_group(proc) + def run_cost_usd(self) -> float: + """What this run has cost so far, summed from what providers reported.""" + with self.lock: + return sum(r.cost_usd or 0.0 for r in self.runtimes) + + def _refresh_cost(self, runtime: TaskRuntime) -> None: + """Re-read the task's log; it accumulates across attempts, so this is total.""" + cost, steps = parse_step_costs(runtime.log_path) + with self.lock: + if cost is not None: + runtime.cost_usd = cost + runtime.model_steps = steps + + async def _note_failure(self, runtime: TaskRuntime, verify: Any) -> None: + """Track identical consecutive failures so an impossible manifest stops early. + + The signature is the check's exit code plus the first meaningful line it + printed. A manifest that asks for something no worker can produce fails + every task the same way; that is the shape worth stopping on, and it is + distinguishable from a run where several different things went wrong. + """ + limit = self.manifest.abort_after_repeated_failures + if not limit: + return + first_line = "" + for line in (getattr(verify, "raw_output_excerpt", "") or "").splitlines(): + stripped = line.strip() + if stripped: + first_line = stripped[:200] + break + signature = f"rc={getattr(verify, 'check_returncode', None)}|{first_line}" + with self.lock: + if signature == self.recent_failure_signature: + self.repeated_failures += 1 + else: + self.recent_failure_signature = signature + self.repeated_failures = 1 + tripped = self.repeated_failures >= limit and self.stop_reason is None + if tripped: + self.stop_reason = ( + f"{self.repeated_failures} tasks failed in a row with the same " + f"failure ({first_line or 'no output'}) -- stopping rather than " + f"paying for the rest of the manifest" + ) + if tripped: + print(f"\n*** RUN STOPPED: {self.stop_reason} ***", flush=True) + await self.kill_all_workers() + + async def _check_budget(self) -> None: + budget = self.manifest.budget_usd + if not budget: + return + spent = self.run_cost_usd() + if spent < budget: + return + with self.lock: + if self.stop_reason is not None: + return + self.stop_reason = f"budget of ${budget:.2f} reached (spent ${spent:.2f})" + print(f"\n*** RUN STOPPED: {self.stop_reason} ***", flush=True) + await self.kill_all_workers() + async def _run_task(self, runtime: TaskRuntime) -> None: async with self.semaphore: + with self.lock: + stopped = self.stop_reason + if stopped is not None: + with self.lock: + runtime.status = "fail" + runtime.final_verdict = "SKIPPED" + runtime.setup_error = f"not started: {stopped}" + runtime.ended_at_monotonic = time.monotonic() + return with self.lock: runtime.started_at_monotonic = time.monotonic() prepared, prepare_error = await self._prepare_taskdir(runtime) @@ -8786,6 +8933,10 @@ async def _run_task(self, runtime: TaskRuntime) -> None: runtime.status = "verifying" if worker.tokens is not None: runtime.tokens = (runtime.tokens or 0) + worker.tokens + # Money is counted the moment the worker stops, before the check + # runs, so a budget cannot be overshot by a slow verification. + self._refresh_cost(runtime) + await self._check_budget() verify = await self.verifier.verify(runtime.task, runtime.taskdir) verdict = verdict_for(worker, verify) with self.lock: @@ -8802,6 +8953,16 @@ async def _run_task(self, runtime: TaskRuntime) -> None: runtime.ended_at_monotonic = time.monotonic() await self._cleanup_worktree_on_pass(runtime) return + if verdict in {"FAIL", "TIMEOUT"}: + await self._note_failure(runtime, verify) + with self.lock: + stopped = self.stop_reason + if stopped is not None and verdict != "PASS": + with self.lock: + runtime.status = "fail" + runtime.final_verdict = verdict + runtime.ended_at_monotonic = time.monotonic() + return if attempt < max_attempts and verdict in {"FAIL", "TIMEOUT"}: failure_context = build_failure_context(runtime.log_path, verify.raw_output_excerpt) current_spec = ( @@ -9429,6 +9590,53 @@ def parse_env_file(path: Path) -> dict[str, str]: return values +def parse_step_costs(log_path: Path) -> tuple[float | None, int]: + """Sum what the provider says each model call cost, from the worker's own log. + + Engines that stream JSON events report a per-step `cost` alongside the token + counts. Summing those is exact: it needs no price catalog, no assumption + about the prompt/completion split, and it stays right when a provider + discounts or caches. + + This exists because `worker_tokens` records ONE step, not the sum -- the + token regex reads a single figure out of the tail of the stream. Measured on + real tasks the gap is 19x to 40x, which is how a $41.71 swarm reported as + roughly $3 and was restarted sixteen times by an operator who had no way to + see otherwise. + + Returns (usd, steps). usd is None when the log carries no cost data at all + (a plan-billed or non-JSON engine), which is different from a run that + genuinely cost nothing. + """ + total = 0.0 + steps = 0 + saw_cost = False + try: + handle = log_path.open("r", encoding="utf-8", errors="replace") + except OSError: + return None, 0 + with handle: + for line in handle: + line = line.strip() + if not line.startswith("{"): + continue + try: + event = json.loads(line) + except ValueError: + continue + if not isinstance(event, dict) or event.get("type") != "step_finish": + continue + steps += 1 + part = event.get("part") + if not isinstance(part, dict): + continue + cost = part.get("cost") + if isinstance(cost, (int, float)) and not isinstance(cost, bool): + total += float(cost) + saw_cost = True + return (total if saw_cost else None), steps + + def parse_token_count(text: str, token_regex: str | None = DEFAULT_TOKEN_REGEX) -> int | None: if token_regex: matches = list(re.finditer(token_regex, text, flags=re.IGNORECASE)) @@ -10078,17 +10286,28 @@ def print_lint_findings(findings: list[str]) -> None: def print_summary(run_id: str, runtimes: list[TaskRuntime]) -> None: print("\nSummary") print(f"run_id: {run_id}") - header = f"{'task':<24} {'status':<8} {'verdict':<8} {'attempts':>8} {'tokens':>10} {'elapsed_s':>10}" + header = ( + f"{'task':<24} {'status':<8} {'verdict':<8} {'attempts':>8} " + f"{'steps':>6} {'USD':>8} {'elapsed_s':>10}" + ) print(header) print("-" * len(header)) now = time.monotonic() for runtime in runtimes: - tokens = "" if runtime.tokens is None else str(runtime.tokens) + usd = "" if runtime.cost_usd is None else f"{runtime.cost_usd:.3f}" print( f"{runtime.task.key:<24} {runtime.status:<8} " f"{(runtime.final_verdict or ''):<8} {runtime.attempts:>8} " - f"{tokens:>10} {runtime.elapsed_s(now):>10.1f}" - ) + f"{runtime.model_steps:>6} {usd:>8} {runtime.elapsed_s(now):>10.1f}" + ) + priced = [r for r in runtimes if r.cost_usd is not None] + if priced: + total = sum(r.cost_usd or 0.0 for r in priced) + steps = sum(r.model_steps for r in runtimes) + print(f"\nrun cost: ${total:.2f} over {steps} model steps, as reported by the provider") + unpriced = [r.task.key for r in runtimes if r.cost_usd is None and r.model_steps] + if unpriced: + print(f" (not included, engine reports no per-step cost: {', '.join(unpriced)})") setup_failures = [r for r in runtimes if r.setup_error] if setup_failures: print("\nsetup failures (no worker was spawned):") diff --git a/tests/test_cost_control.py b/tests/test_cost_control.py new file mode 100644 index 000000000..bde54e7a3 --- /dev/null +++ b/tests/test_cost_control.py @@ -0,0 +1,175 @@ +"""Cost accounting and the two controls that stop a runaway run. + +Every gate here is provoked on purpose. A cap nobody has watched fire is a cap +nobody should trust, and these exist because a $41.71 swarm reported as roughly +$3 and was restarted sixteen times. +""" +import json +import sys +import tempfile +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import ringer # noqa: E402 + + +def step(cost=None, total=0): + part = {"type": "step-finish", "tokens": {"total": total}} + if cost is not None: + part["cost"] = cost + return json.dumps({"type": "step_finish", "part": part}) + + +class ParseStepCostsTests(unittest.TestCase): + def _log(self, lines): + tmp = Path(tempfile.mkdtemp()) / "worker.log" + tmp.write_text("\n".join(lines) + "\n", encoding="utf-8") + return tmp + + def test_sums_every_step_not_just_one(self): + # The defect this replaces: a single step was recorded as the task's + # cost. Three steps here, and the answer must be their sum. + log = self._log([ + "[ringer.py] attempt 1 started", + step(cost=0.01, total=100), + step(cost=0.02, total=900), + step(cost=0.03, total=50), + ]) + usd, steps = ringer.parse_step_costs(log) + self.assertAlmostEqual(usd, 0.06) + self.assertEqual(steps, 3) + # and emphatically not the largest single step + self.assertNotAlmostEqual(usd, 0.03) + + def test_costs_accumulate_across_attempts(self): + log = self._log([step(cost=0.05), "[ringer.py] attempt 2 started", step(cost=0.07)]) + usd, steps = ringer.parse_step_costs(log) + self.assertAlmostEqual(usd, 0.12) + self.assertEqual(steps, 2) + + def test_engine_reporting_no_cost_is_none_not_zero(self): + # A plan-billed engine that reports nothing must not read as free. + log = self._log([step(total=500), step(total=700)]) + usd, steps = ringer.parse_step_costs(log) + self.assertIsNone(usd) + self.assertEqual(steps, 2) + + def test_a_genuinely_free_model_is_zero_not_none(self): + log = self._log([step(cost=0.0), step(cost=0.0)]) + usd, _ = ringer.parse_step_costs(log) + self.assertEqual(usd, 0.0) + + def test_survives_noise_and_a_missing_file(self): + log = self._log(["not json at all", "{broken", step(cost=0.25)]) + usd, steps = ringer.parse_step_costs(log) + self.assertAlmostEqual(usd, 0.25) + self.assertEqual(steps, 1) + usd, steps = ringer.parse_step_costs(Path("/nonexistent/worker.log")) + self.assertIsNone(usd) + self.assertEqual(steps, 0) + + +def manifest_obj(**extra): + obj = { + "run_name": "cost-tests", + "workdir": tempfile.mkdtemp(), + "max_parallel": 1, + "tasks": [{ + "key": "t1", + "spec": "do a thing", + "check": "echo checking; test -f out.txt", + "verified": "out.txt exists", + }], + } + obj.update(extra) + return obj + + +class ManifestBudgetTests(unittest.TestCase): + def test_budget_and_abort_round_trip(self): + m = ringer.Manifest.from_obj(manifest_obj(budget_usd=6.5, abort_after_repeated_failures=3)) + self.assertAlmostEqual(m.budget_usd, 6.5) + self.assertEqual(m.abort_after_repeated_failures, 3) + # and survive the copy that --max-parallel makes + self.assertAlmostEqual(m.with_max_parallel(4).budget_usd, 6.5) + self.assertEqual(m.with_max_parallel(4).abort_after_repeated_failures, 3) + + def test_absent_means_unlimited(self): + m = ringer.Manifest.from_obj(manifest_obj()) + self.assertIsNone(m.budget_usd) + self.assertIsNone(m.abort_after_repeated_failures) + + def test_nonpositive_values_are_refused(self): + for bad in (0, -1): + with self.assertRaises(ValueError): + ringer.Manifest.from_obj(manifest_obj(budget_usd=bad)) + with self.assertRaises(ValueError): + ringer.Manifest.from_obj(manifest_obj(abort_after_repeated_failures=bad)) + + +class UnwritableDeliverableLintTests(unittest.TestCase): + """The bug that cost the most: a deliverable the worker cannot write.""" + + def _manifest(self, spec, expect, worktrees=True): + workdir = tempfile.mkdtemp() + return ringer.Manifest.from_obj({ + "run_name": "lint-tests", + "workdir": workdir, + "max_parallel": 1, + "worktrees": worktrees, + "repo": None, + "tasks": [{ + "key": "scout1", + "spec": spec, + "check": f"echo verifying; test -s {expect}", + "verified": "report exists", + "expect_files": [expect], + }], + }), workdir + + def test_spec_ordering_a_write_outside_the_sandbox_is_flagged(self): + outside = "/tmp/somewhere-else/report.json" + m, _ = self._manifest(f"Write your report to {outside} when done.", outside) + findings = ringer.lint_manifest(m) + self.assertTrue( + any("sandbox forbids" in f for f in findings), + f"expected an unwritable-path finding, got: {findings}", + ) + + def test_the_check_exporting_to_an_absolute_path_is_fine(self): + # The fix-swarm pattern: the CHECK writes the patch out of the worktree. + # The spec never names that path, so the worker is asked for nothing + # impossible and this must NOT be flagged. + outside = "/tmp/somewhere-else/task.patch" + m, _ = self._manifest("Leave your changes uncommitted in the worktree.", outside) + findings = ringer.lint_manifest(m) + self.assertFalse( + any("sandbox forbids" in f for f in findings), + f"check-exported deliverable must not be flagged, got: {findings}", + ) + + def test_a_path_inside_the_task_directory_is_fine(self): + workdir = tempfile.mkdtemp() + inside = str(Path(workdir) / "scout1" / "report.json") + m = ringer.Manifest.from_obj({ + "run_name": "lint-tests", + "workdir": workdir, + "max_parallel": 1, + "worktrees": True, + "repo": None, + "tasks": [{ + "key": "scout1", + "spec": f"Write {inside}", + "check": f"echo verifying; test -s {inside}", + "verified": "report exists", + "expect_files": [inside], + }], + }) + findings = ringer.lint_manifest(m) + self.assertFalse(any("sandbox forbids" in f for f in findings), findings) + + +if __name__ == "__main__": + unittest.main() From 344092b3a3483104fbd386d5e3b3ee8eb05214ae Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 10 Sep 2026 03:47:08 +0200 Subject: [PATCH 04/19] docs: model notes from the monorepo run2 and nexo run1 swarms Committed as-is to protect 206 lines of working-tree notes before bringing the branch up to date with upstream. Content unreviewed and unchanged. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0125QPzJL8qBQsayptsvuPgB --- docs/MODEL-NOTES.md | 206 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 206 insertions(+) diff --git a/docs/MODEL-NOTES.md b/docs/MODEL-NOTES.md index 5532d8ec4..c37490eb0 100644 --- a/docs/MODEL-NOTES.md +++ b/docs/MODEL-NOTES.md @@ -292,6 +292,10 @@ checks and raw logs support — no vibes, no worker self-reports. ## GPT-5.5 (codex) — attribution caveat - Scoreboard rows dated before 2026-07-09 may actually be gpt-5.6: codex eval rows logged model="" until the write-time stamping fix (PR #18) and were credited to GPT-5.5 by the registry default at read time, while the machine's codex default had already moved to gpt-5.6-sol at an unknown earlier date. `scripts/backfill_model_from_logs.py` re-stamps rows with surviving command-log evidence; anything it skips is a mixed-model aggregate. Trust post-2026-07-09 rows. +## GPT-5.5 (codex) — juce-test, first datapoint +- 2026-08-06 (juce-test, juce-lane-smoke — sombra-audio C++/JUCE lane, first ever run of that lane): 2 attempts, both scored FAIL, **both blameless**. The kit's own check has an inverted anti-vacuity gate (`check_juce_feature.py:98` only parses CTest's *failed*-run phrasing), so a green suite can never pass — see work#454. The worker's output was correct on attempt 1: right Catch2 case, right CMake registration, ownership respected, no commits, substantive notes, full suite 99/99. Do NOT read these two rows as evidence against GPT-5.5 on JUCE work; the scoreboard cannot distinguish them from real failures. Re-run once #454 lands to get a real first datapoint. +- Retry-context caveat observed in the same run: the `Previous attempt failed:` payload carried a fragment of the prior attempt's `notes.md` diff rather than the check's `[FAIL]` line. The worker rewrote an innocent notes line and failed identically — a model given no failure signal cannot self-correct, and the retry row is not a capability signal. + ## nvidia/nemotron-3-super-120b-a12b:free - 2026-07-08 (research, content-strategy-recon): FAIL x2. Did the analysis in chat but never wrote report.md; attempt 2 exited rc=0 with no file. Doesn't reliably follow file-output contracts under OpenCode. Demoted — don't re-audition on file-deliverable tasks. @@ -427,3 +431,205 @@ advantage is this defect, not model speed (medians: 662s vs 808s, ~18%; means: 6 **codex token capture is unusable.** The engine's `token_regex` matched nothing at all in today's logs, yet Ringer still recorded `tokens: 99 / 144 / 143`. Those numbers are not traceable to any worker output. Treat codex token counts on the scoreboard as noise. + +## nvidia/nemotron-3-ultra-550b-a55b:free (OpenCode / OpenRouter) + +- **2026-08-31 — code-review (claim verification), nexo.** First outing, exploration lane in a + 5-task batch. **Passed first try**, and was the *fastest* task in the run at 92s against Codex's + 118–236s. 32,512 tokens (Codex tasks in the same run reported 38–93). Task was the most + mechanical of the five — cross-check five `.graphql` selection sets against the `.dart` screens + that consume them — and it handled the executed check cleanly: five verdicts, every one citing + a real `file:line`, all of which I verified independently afterwards (including a schema line + number I had not looked up myself). One nit: it emitted line *ranges* (`file.dart:51-60`) where + the contract asked for `file:line`; the validator's regex tolerated it, a stricter one would + have failed honest work. Free, 1M context. **Worth another audition on mechanical + cross-reference work** — no evidence yet on tasks needing judgement rather than lookup. + +### 2026-09-01 — three-lab panel on a merge/gate judgement call (task_type code-review) + +- **Nemotron 3 Ultra (openrouter/nvidia/...:free)** — judgement task, 2 attempts. Attempt 1 died on + an upstream `502 Service temporarily overloaded` from NVIDIA *mid-reasoning* (the log shows it had + reached "Let me write the report" before the provider dropped it). Attempt 2 passed, 281s — by far + the slowest of the three. Verdict quality was good: it named the mechanism, took the harder line on + the release question and gave a precedent-creep argument. Worth keeping in the free-exploration + slot for judgement work, but not for anything time-critical: a free NVIDIA endpoint that 502s under + load will do it again. +- **GLM 5.2 (openrouter/z-ai/glm-5.2)** — same task, and the standout. It produced the one insight no + other panellist or the orchestrator had: that a sanitizer exclusion *rots*, and the defence is + making it self-expiring (a `TODO(ticket)` a mechanical check fails on once the ticket closes). It + also asked the single question that could have flipped the whole answer — whether the racy path was + reachable from host automation rather than only a user action. Cheap, fast (53s), and it reasons + about second-order consequences rather than restating the brief. +- **GPT-5.5 · high (Codex)** — passed first try, 55s, 16 tokens of report. Tightest of the three and + the only one to name the *psychological* cost of the recommendation ("landing T3 may make the race + feel handled because CI is green"). Reliable default for this shape of question. + +⚠️ **Orchestrator error worth recording, not a model failure.** The first run failed 3/3 because the +manifest set a per-task `"workdir"`. Ringer ignores it — the taskdir is `manifest.workdir / task.key` +— so every worker wrote to `//` while every check read `//`. The GLM +worker diagnosed this correctly from inside its sandbox and said so in its output; it was right, and +verifying it on disk took one `ls`. If a task key must match a directory name, do not put a colon in +the key. + +### z-ai/glm-5.2:free + +- **2026-09-03 · code-fix (shell) · NO-OP TWICE, task failed.** Audition on monorepo + round 2, work#470 (rewrite `scripts/smoke-photo-upload.sh` off four deleted routes and + add a companion tests script). Both attempts left `git status` clean — the ownership + guard's "the worker changed NOTHING" branch, on attempt 1 *and* on the retry that + injected that exact message. Spec was 7 KB with the four historic traps, the four dead + routes and an in-repo precedent named (`ses-permission-args-tests.sh`); every other lane + in the same run on codex/gpt-5.5 produced real diffs from comparably sized specs, so + this is not a spec-length problem. Ended the audition — re-run the lane on codex. + Cost of the experiment: one lane, zero tokens billed (free model), ~2 wasted slots. + ⚠️ **Evidence caveat, added the same day:** the raw worker log was overwritten when that + lane was re-run on codex at the same path, so what survives is the run-state record + (`monorepo-round2-...json`: status fail, "the worker changed NOTHING"), not the raw + transcript. The opencode engine block WAS verified configured and uncommented + (config.toml:183, `model_default = openrouter/z-ai/glm-5.2`), so this is not a + misconfiguration artefact. Treat it as one solid observation rather than a settled + verdict — re-audition before writing the model off. + +## Bakeoff 2026-09-03 — cheap lens review vs Codex (nexo PR#248 fixture) + +Fixture: the first state of nexo PR#248, an 837-line .NET seeder diff whose real defects are +known from seven Codex passes. Scored on whether each model recovered them. + +- **qwen/qwen3.8-flash** — code-review. Found BOTH known defects **plus a real latent bug seven + Codex passes missed**: a shared scoped DbContext where a fixture throwing after `AddAsync` + leaves a half-built entity that the next fixture's `SaveChangesAsync` commits. Verified against + the code and fixed. ~$0.004, 43k tokens. TIMEOUT at 1800s — not slow reasoning: it wrote a + complete report in minutes then kept re-verifying the file. Adding an explicit "stop once + report.md is written" line to the spec is the fix; do that for any flash-class reviewer. +- **deepseek/deepseek-v4-flash** — code-review. **PASS on first attempt**, 204s, 36k tokens, + ~$0.003. Recovered both known defects and independently raised directory-visibility-forced- + public as P1. Best cost/quality/reliability of the field. Undated slug only. +- **deepseek/deepseek-v4-pro** — code-review. FAIL, 2 attempts, 48k tokens. Found strictly LESS + than its cheaper Flash sibling and failed the contract on `finding_citation_unresolvable` — + cited `DemoContentSeeder.cs:86` etc. when only the diff was staged. **The bigger DeepSeek was + worse than the smaller one here**; do not assume Pro > Flash for review work. +- **z-ai/glm-5.3 / glm-5.3-flash** — not routable on this account: instant `UnknownError: + Unexpected server error`, 0 tokens, both attempts. Only `z-ai/glm-5.2` works. Not a quality + result. + +⚠️ **Dated OpenRouter slugs failed across the board** (`-0731`, `-0813`): instant server error, +0 tokens. Undated slugs worked. Use undated. + +Context: this bakeoff was run because Codex quota was exhausted after ~880k tokens / EUR20 on +that single PR. Four cheap-model reviews of the same diff cost about a cent in total. + +### Validation round, same day — the first bakeoff was too easy, and the conclusion flipped + +Two new fixtures on the same PR: **recall** (a state containing the subtle mirrored-`musicLinks` +test — a green test that re-implemented the handler it claimed to verify, which Codex found) and +**precision** (final state, all known defects fixed — does the model invent findings?). + +- **deepseek/deepseek-v4-flash** — precision **PASS**: correctly reported "no defects found" on + clean code. Recall **FAIL**: it did not find the mirrored test at all, concluding "no verified + defect found", offering only a speculative P3 null-`SequenceEqual` observation at *low* + confidence. 111k tokens over 2 attempts, and it still failed the contract on an unresolvable + citation (`DemoContentSeederTests.cs:708` — past the end of a 323-line file). Its report shows + coherence decay on long context: `SetEpkConent`, `UpdateAvater`, `prevens writin`, + `projects/exogig`. + **Profile: good precision, poor recall.** A clean report from it is not evidence of clean code, + which makes it unusable as a merge gate — the failure mode is silent. +- **qwen/qwen3.8-flash** — TIMEOUT on both fixtures even with an explicit stop condition in the + spec. Now 1 completion in 4 attempts. Its single good report was the best of the whole bakeoff + (it alone found the shared-DbContext leak), but a reviewer that finishes a quarter of the time + cannot gate anything. + +⚠️ **Correcting this file's own earlier entry.** The first round's fixture had two *obvious* +defects, and both cheap models found them; I concluded they matched Codex. On the subtle defect — +the kind that actually justifies paying for a reviewer — the cheap models found nothing. **Do not +wire either model into the review gate on the strength of the first round.** Keep them as an +extra cheap opinion alongside Codex, never as a replacement for it. + +## 2026-09-05 — board-premise-audit (38 read-only repo audits, monorepo) + +**gpt-5.5 · medium · Codex CLI — 34/34 first-try.** Task type was a read-only repo audit producing +one `report.md` per work item, gated by a validator that resolves every `path:line` citation against +the repo. No retries, no hallucinated citations across 34 reports. Reports ran real commands +(including executing repo checkers) and scoped their own residual risk unprompted. This is the +routing pick for repo-audit work. + +**openrouter/z-ai/glm-5.2 (OpenCode) — 0/2, no artifact.** Both audition tasks reached attempt 2 and +never wrote `report.md` at all; the check failed on `missing expected files`. Contrast with its 18-task +0.83 pass rate on general code-review — the difference here is a strict output contract (six required +sections + citation format). Worth one more audition on a task with a looser contract before drawing +a conclusion; do not scale it on contract-heavy tasks yet. + +**openrouter/cohere/north-mini-code:free — 0/2.** Free, untested, auditioned on two low-stakes tickets. +work#925 wrote a **0-byte** report; work#760 wrote a confident `EVIDENCE-COMPLETE` verdict with **zero** +citations. Both are exactly what the validator exists to catch, so the cost was two cheap failures and +the signal is clean: it does not honour a structured output contract. Not promoted. + +**Orchestrator lessons, not model lessons:** +1. **The run was killed by the OS for low memory at ~36/38 tasks** with `max_parallel: 6` alongside a + second Ringer run and a veleta rsync. The run JSON is then a **stale snapshot** (`state: live`) and + its pass/fail counts are NOT authoritative. Re-validating the artifacts on disk with the same check + is cheap, model-free and trustworthy — do that rather than trusting a killed run's bookkeeping. +2. **A verdict vocabulary written for one ticket state inverts in another.** `PREMISE-GONE` meant + "reject this ticket" for a `ready` item but "the shipped fix is confirmed present" for a `verifying` + one. The workers handled it sensibly; the label misled the reader. Scope the vocabulary to the + lifecycle state the batch is actually auditing. + +## 2026-09-06/07 — artifact-path scout (51 read-only repo scouts, monorepo) + +**gpt-5.5 · medium · Codex CLI — 50/51 first-try, 51/51 after one retry.** Read-only scout over the +monorepo: one ticket per worker, output one `report.md` with five keys, gated by a validator that +resolves the reported path in the repo AND matches the worker's stated rationale back against the +exact ticket text. Median 70s/task. Confirms the 2026-09-05 board-premise-audit result on the same +shape (34/34) — this is the routing pick for read-only repo work, now over 85 tasks. + +**The one retry is the most useful row here, and it is an argument for a specific check design.** +`scout-work-545` reported a *correct* path backed by an entirely **fabricated quotation**: +"RegistrationService exposes no update path — Submit() always mints a new row, because the +guest-facing flow is submit-once." Fluent, technical, names a real class and method, and absent from +the ticket. The model invented a rationale and presented it as a quote. Attempt 2 returned the same +path with a real sentence. **A path-existence check would have passed this silently** — the path was +right. Lesson for check authors: when a worker's output will be written down as fact, gate the +*reasoning* against its stated source, not only the artifact. Substring-matching the quote back +against the input is cheap and caught what nothing else could. + +**Format tolerance is what makes that assertion usable.** The matcher folds whitespace, smart quotes, +markdown emphasis and case before comparing, with a 25-char floor. An earlier strict version would +have failed honest workers for reflowing a quote across lines — and a wall of format failures trains +workers to stop quoting, which defeats the assertion entirely. + +**Orchestrator lessons, not model lessons:** +1. **Two of three runs were OS-killed for low memory — at `max_parallel` 5 AND at 3.** Yesterday's + kill was at 6. Codex workers are heavier on this box than the free-RAM figure suggests: 13.6 GB + showed "free" while swap sat at 18.8/19.5 GB and 20 GB was compressed. **Inactive memory is not + headroom when swap is already full** — read `sysctl vm.swapusage`, not just `vm_stat`. 2 worked. +2. **A killed run's JSON undercounts, consistently and in the safe direction.** Run 1 JSON said 13 + pass; 16 valid reports were on disk. Final tally: JSON 48, disk 51. Workers finish and write + before the bookkeeping credits them, so re-running the check over the artifacts is both cheaper + and *more* accurate than trusting the snapshot. Re-validating is model-free and takes seconds. +3. **Clear a killed task's directory before resuming it.** A worker killed mid-write leaves a partial + `report.md` that a retry can inherit — and a truncated report can satisfy a check on a fragment. +4. **A check can enforce that a quote is real without enforcing that it is the *right* quote.** 3 of + 44 passing reports anchored on an aside, a very short phrase, or an open question from the ticket. + Orchestrator spot-checks caught those; the gate could not. Budget review time for it. + +### Same run, the review side — gpt-5.5 medium as the aios lens on PR #429 (6 rounds) + +Six rounds on a 260-line Python gate, one finding per round after the first (which +raised three), decaying P1 → P2 → P2 → P1 → clean → clean. **Every finding held when +checked against its cited `file:line`** — no phantom findings across six rounds, which +is the number worth remembering when deciding whether to argue with this lens. + +Three rounds landed on the same assertion, each time a strictly narrower bypass of the +previous fix (no-op command → no-op carrying a path argument → chain of no-ops). That +pattern is the signal to stop patching instances and close the class: the fix that +finally held judged the command *by segment* rather than asking whether it was chained. + +**One proposed fix was correctly rejected on data.** The reviewer wanted the check +command to name the reported path. Measured against the run's own 44 reports, 9 do not +name their path — and those 9 are the best checks in the batch (`dotnet test +--filter ` gates behaviour through a test project). Adopting it would have failed +the strongest work. Take the finding, verify the fix against real data, and do not +assume the proposed remedy is as sound as the diagnosis. + +**Report-contract retries are normal and cheap.** Round 3 failed its own `lens_offtopic` +check (a lens line that did not answer the prohibition it claimed to), retried, passed. +The finding itself was unaffected. From d0360254bf1e4b1e13a0b21681e87cc6ced4637d Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 10 Sep 2026 08:20:29 +0200 Subject: [PATCH 05/19] Attribute cost to a requirement, and stop Codex reading as free Two gaps that made the previous commit's numbers unsteerable rather than merely incomplete. A run could not say what a requirement cost. Measured on a real estate, 175 of 178 product tasks were keyed `fix-L13` and similar, so 96% of the money could not be traced to the work item it served. TaskSpec gains an optional `ticket`, lint requires it for task types that change a product (a bakeoff or probe legitimately serves none, so the rule is scoped rather than universal), and the eval log now records ticket, cost and step count. The three shipped templates that demonstrate product work gained the field -- they were the first thing the new rule caught. Codex reports tokens but never a cost, so every task it runs shows as free. On the same estate that was 797 of 922 tasks, every code review among them, which made review look free when it was the expensive half. Engines gain an optional `token_scale` and `price_*_per_mtok`. The scale matters more than it looks: Codex reports THOUSANDS -- its "tokens used" runs 12-230 across 40 real tasks where OpenCode reports hundreds of thousands for comparable work -- so an unscaled estimate is off by a factor of a thousand. An estimate is stored in its own field, never added to measured cost, and always labelled. With no prices configured the run says plainly that those tasks carry no cost of any kind and names them, because an unknown cost that defaults to zero is how a bill becomes a surprise. 7 further tests, including the two cases that must NOT fire: a research or probe task needs no ticket, and an engine with no prices yields None rather than 0.0. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0125QPzJL8qBQsayptsvuPgB --- config.sample.toml | 21 +++++ ringer.py | 106 +++++++++++++++++++++++- templates/fix-swarm/manifest.json | 7 +- templates/migration-swarm/manifest.json | 7 +- templates/repo-feature/manifest.json | 3 +- tests/test_cost_control.py | 56 +++++++++++++ 6 files changed, 189 insertions(+), 11 deletions(-) diff --git a/config.sample.toml b/config.sample.toml index 1527dead3..47df85a04 100644 --- a/config.sample.toml +++ b/config.sample.toml @@ -184,3 +184,24 @@ enabled = true out = "~/.ringer/artifacts/{run_id}.html" report_out = "~/.ringer/artifacts/{run_id}-report.html" index_out = "~/.ringer/artifacts/index.html" + +# ── Cost visibility for engines that bill on another account ────────────── +# Codex reports tokens but never a cost, so its work shows as free in every +# provider-reported total. On one estate that hid 797 of 922 tasks -- every +# code review among them -- which made review look free when it was the +# expensive half. +# +# token_scale: some harnesses report THOUSANDS of tokens, not units. Codex +# does: measured over 40 real tasks its "tokens used" runs 12-230 where +# OpenCode reports hundreds of thousands for comparable work. Without this a +# Codex loop reads a thousand times cheaper than it is. +# +# Setting prices produces an ESTIMATE, always labelled as one and kept apart +# from measured cost. Leave them unset and the run says plainly that those +# tasks carry no cost at all, which is the honest answer -- an unknown cost +# must never default to zero. +# +# [engines.codex] +# token_scale = 1000 +# price_in_per_mtok = 1.25 +# price_out_per_mtok = 10.00 diff --git a/ringer.py b/ringer.py index d70c7517f..8f88f54e8 100755 --- a/ringer.py +++ b/ringer.py @@ -741,6 +741,16 @@ class EngineConfig: # its own "model" — this is what makes a harness engine (OpenCode) model # agnostic instead of hard-coding one model into the command line. model_default: str = "" + # Some harnesses report tokens in thousands rather than units. Codex does: + # measured over 40 real tasks its "tokens used" runs 12-230 where OpenCode + # reports hundreds of thousands for comparable work. Summed naively a Codex + # loop reads a thousand times cheaper than it is. + token_scale: int = 1 + # Optional $/million, for engines that report tokens but no cost of their + # own. Setting these produces an ESTIMATE, always labelled as one -- a + # number the provider never sent must never be presented as measured. + price_in_per_mtok: float | None = None + price_out_per_mtok: float | None = None @property def process_name(self) -> str: @@ -1602,9 +1612,27 @@ def load_engines(raw: Any) -> dict[str, EngineConfig]: model_default = str( section.get("model_default", base.model_default if base else "") ).strip() + token_scale = int(section.get("token_scale", base.token_scale if base else 1)) + if token_scale <= 0: + raise ValueError(f"engines.{clean_name}.token_scale must be positive") + + def _price(field: str) -> float | None: + raw = section.get(field, getattr(base, field) if base else None) + if raw is None: + return None + value = float(raw) + if value < 0: + raise ValueError(f"engines.{clean_name}.{field} must not be negative") + return value + + price_in = _price("price_in_per_mtok") + price_out = _price("price_out_per_mtok") engines[clean_name] = EngineConfig( name=clean_name, bin=bin_path, + token_scale=token_scale, + price_in_per_mtok=price_in, + price_out_per_mtok=price_out, args_template=args_template, full_access_args=full_access_args, sandbox_args=sandbox_args, @@ -1638,6 +1666,12 @@ class TaskSpec: full_access: bool = False engine_args: tuple[str, ...] = () verified: str = "" + # The requirement this task serves, e.g. "work#666". Optional -- plenty of + # runs are bakeoffs and probes that serve no ticket -- but without it a fix + # swarm's spend cannot be attributed to anything. Measured on a real estate: + # 175 of 178 product tasks were keyed `fix-L13` and similar, so the question + # "what did this requirement cost?" had no answer at all. + ticket: str = "" # Which model a harness engine should run for this task (fills the # engine's {model} placeholder); empty means the engine's model_default. model: str = "" @@ -1706,6 +1740,7 @@ def from_obj(cls, obj: dict[str, Any]) -> "TaskSpec": full_access=bool(obj.get("full_access", False)), engine_args=tuple(engine_args), verified=verified.strip(), + ticket=str(obj.get("ticket", "")).strip(), model=model.strip(), task_type=task_type.strip(), ) @@ -1827,6 +1862,12 @@ def with_max_parallel(self, value: int | None) -> "Manifest": FILE_TEST_OPS = {"-e", "-f", "-s", "-d", "-r", "-w", "-x", "-L"} +# Task types that change a product and therefore answer to a requirement. A +# bakeoff, probe or research task legitimately serves none, so the lint finding +# is scoped rather than universal. +TICKETED_TASK_TYPES = frozenset({"code-fix", "code-feature", "dotnet-fix", "dotnet-feature"}) + + def worker_unwritable_paths(task: TaskSpec, manifest: Manifest) -> list[str]: """Absolute paths a spec orders the worker to write that its sandbox refuses. @@ -1881,6 +1922,11 @@ def lint_manifest( findings.append( f"{task.key}: check may fail without printing why; retry prompt and eval log depend on failure output." ) + if task.task_type in TICKETED_TASK_TYPES and not task.ticket: + findings.append( + f"{task.key}: {task.task_type} names no ticket, so its cost and outcome " + f"cannot be attributed to a requirement. Set \"ticket\"." + ) for unreachable in worker_unwritable_paths(task, manifest): findings.append( f"{task.key}: the spec tells the worker to write {unreachable}, which its " @@ -2154,6 +2200,10 @@ class TaskRuntime: # model step in its log, and how many steps that took. `tokens` above is a # single step and cannot be used for money -- see parse_step_costs. cost_usd: float | None = None + # Set only when the engine reports no cost of its own and the config supplies + # prices. Kept in a SEPARATE field so a measured total and a guess can never + # be added together by accident. + cost_estimated_usd: float | None = None model_steps: int = 0 final_verdict: str | None = None last_check_returncode: int | None = None @@ -8848,10 +8898,13 @@ def run_cost_usd(self) -> float: def _refresh_cost(self, runtime: TaskRuntime) -> None: """Re-read the task's log; it accumulates across attempts, so this is total.""" cost, steps = parse_step_costs(runtime.log_path) + engine = self.config.engines.get(runtime.task.engine) with self.lock: if cost is not None: runtime.cost_usd = cost runtime.model_steps = steps + if cost is None and engine is not None and runtime.tokens: + runtime.cost_estimated_usd = estimate_cost_from_tokens(engine, runtime.tokens) async def _note_failure(self, runtime: TaskRuntime, verify: Any) -> None: """Track identical consecutive failures so an impossible manifest stops early. @@ -9395,6 +9448,11 @@ def _log_attempt( "expected_model": expected_model, "reasoning_effort": reasoning_effort, "task_type": runtime.task.task_type, + # What this cost was FOR. Without it the log records what was + # spent and never what it bought. + "ticket": runtime.task.ticket, + "cost_usd": runtime.cost_usd, + "model_steps": runtime.model_steps, "retry": retrying, } ) @@ -9590,6 +9648,36 @@ def parse_env_file(path: Path) -> dict[str, str]: return values +def estimate_cost_from_tokens(engine: "EngineConfig", tokens: int) -> float | None: + """A priced GUESS for engines that report tokens but never a cost. + + Codex is the case this exists for: it bills on another account entirely, so + its work is invisible in any provider-reported total. On one estate 797 of + 922 tasks -- every code review among them -- carried no cost at all, which + made review look free when it was the expensive half. + + Two things make this honest rather than misleading. The engine's + `token_scale` is applied first, because a harness reporting thousands + otherwise reads a thousand times cheap. And the result is stored apart from + measured cost and always labelled an estimate; a number the provider never + sent is never presented as fact. + + Returns None when the engine has no prices configured -- an unknown cost + must stay unknown rather than default to zero. + """ + if engine.price_in_per_mtok is None and engine.price_out_per_mtok is None: + return None + scaled = tokens * engine.token_scale + # These engines report one figure, not a split. Agent traffic is + # overwhelmingly prompt tokens -- measured ~99/1 on real runs -- so pricing + # it all at the input rate is closer than a 50/50 blend, and it errs low + # rather than inventing headroom. + rate = engine.price_in_per_mtok + if rate is None: + rate = engine.price_out_per_mtok + return scaled * float(rate) / 1_000_000 + + def parse_step_costs(log_path: Path) -> tuple[float | None, int]: """Sum what the provider says each model call cost, from the worker's own log. @@ -10301,13 +10389,23 @@ def print_summary(run_id: str, runtimes: list[TaskRuntime]) -> None: f"{runtime.model_steps:>6} {usd:>8} {runtime.elapsed_s(now):>10.1f}" ) priced = [r for r in runtimes if r.cost_usd is not None] + estimated = [r for r in runtimes if r.cost_usd is None and r.cost_estimated_usd is not None] + steps = sum(r.model_steps for r in runtimes) if priced: total = sum(r.cost_usd or 0.0 for r in priced) - steps = sum(r.model_steps for r in runtimes) print(f"\nrun cost: ${total:.2f} over {steps} model steps, as reported by the provider") - unpriced = [r.task.key for r in runtimes if r.cost_usd is None and r.model_steps] - if unpriced: - print(f" (not included, engine reports no per-step cost: {', '.join(unpriced)})") + if estimated: + est = sum(r.cost_estimated_usd or 0.0 for r in estimated) + print(f" + ~${est:.2f} ESTIMATED for {len(estimated)} task(s) on engines that report " + f"no cost (billed separately; priced from configured rates, not measured)") + silent = [r.task.key for r in runtimes + if r.cost_usd is None and r.cost_estimated_usd is None and r.model_steps] + if silent: + print(f" ⚠ {len(silent)} task(s) carry NO cost of any kind — the engine reports none and " + f"no price is configured for it: {', '.join(silent[:6])}" + + (" ..." if len(silent) > 6 else "")) + print(" Their spend is real and lands on another bill. Set price_in_per_mtok " + "on that engine to see it.") setup_failures = [r for r in runtimes if r.setup_error] if setup_failures: print("\nsetup failures (no worker was spawned):") diff --git a/templates/fix-swarm/manifest.json b/templates/fix-swarm/manifest.json index c29621ac6..15c337617 100644 --- a/templates/fix-swarm/manifest.json +++ b/templates/fix-swarm/manifest.json @@ -8,11 +8,12 @@ { "key": "{{FIX_KEY}}", "task_type": "code-fix", - "spec": "You are a fix worker in a repair swarm for {{PROJECT}}. Your current working directory is a dedicated git worktree of '{{REPO_PATH}}', so edit the repo files in place here. Boundary: you own only these paths and no others: {{OWNED_FILES — every file or directory this task may modify, comma or newline separated}}. Do not touch .git, do not run git commit, do not run git branch, do not push, and do not reformat unrelated files. The fix to make is {{FINDING — the confirmed bug or issue, with file:line evidence and the desired behavior}}. HOW TO RUN: first reproduce or inspect using {{LOCAL_VERIFY — exact command or manual check to understand the failure}}. Before finishing, run {{BUILD_OR_TEST_COMMAND — exact command that proves the fix and prints useful errors}}. OUTPUT CONTRACT: leave your code changes uncommitted. Also write ./fix-summary.md with '# Fix Summary', '## Summary', '## Files Changed', '## Verification', and '## Assumptions'. Keep it under 700 words. If you cannot complete the fix inside the owned files, do not make speculative edits; explain the blocker in fix-summary.md so the check fails with a useful reason. Hard rules: no invented behavior, mark assumptions, never edit outside the ownership list, and let the validator export the patch.", - "check": "python3 '{{CHECK_SCRIPT_PATH — absolute path to templates/fix-swarm/checks/fix-swarm.py}}' --verify-command '{{BUILD_OR_TEST_COMMAND — exact command that proves the fix and prints useful errors}}' --patch '{{WORKDIR}}/{{FIX_KEY}}.patch' --summary fix-summary.md --exported-summary '{{WORKDIR}}/{{FIX_KEY}}.summary.md' --owned-files '{{OWNED_FILES — every file or directory this task may modify, comma or newline separated}}'", + "spec": "You are a fix worker in a repair swarm for {{PROJECT}}. Your current working directory is a dedicated git worktree of '{{REPO_PATH}}', so edit the repo files in place here. Boundary: you own only these paths and no others: {{OWNED_FILES \u2014 every file or directory this task may modify, comma or newline separated}}. Do not touch .git, do not run git commit, do not run git branch, do not push, and do not reformat unrelated files. The fix to make is {{FINDING \u2014 the confirmed bug or issue, with file:line evidence and the desired behavior}}. HOW TO RUN: first reproduce or inspect using {{LOCAL_VERIFY \u2014 exact command or manual check to understand the failure}}. Before finishing, run {{BUILD_OR_TEST_COMMAND \u2014 exact command that proves the fix and prints useful errors}}. OUTPUT CONTRACT: leave your code changes uncommitted. Also write ./fix-summary.md with '# Fix Summary', '## Summary', '## Files Changed', '## Verification', and '## Assumptions'. Keep it under 700 words. If you cannot complete the fix inside the owned files, do not make speculative edits; explain the blocker in fix-summary.md so the check fails with a useful reason. Hard rules: no invented behavior, mark assumptions, never edit outside the ownership list, and let the validator export the patch.", + "check": "python3 '{{CHECK_SCRIPT_PATH \u2014 absolute path to templates/fix-swarm/checks/fix-swarm.py}}' --verify-command '{{BUILD_OR_TEST_COMMAND \u2014 exact command that proves the fix and prints useful errors}}' --patch '{{WORKDIR}}/{{FIX_KEY}}.patch' --summary fix-summary.md --exported-summary '{{WORKDIR}}/{{FIX_KEY}}.summary.md' --owned-files '{{OWNED_FILES \u2014 every file or directory this task may modify, comma or newline separated}}'", "expect_files": [], "timeout_s": 1800, - "verified": "the validator ran the requested build or test command, exported a non-empty patch, and confirmed the patch only changes declared owned files" + "verified": "the validator ran the requested build or test command, exported a non-empty patch, and confirmed the patch only changes declared owned files", + "ticket": "{{TICKET \u2014 the work item this fix serves, e.g. work#666}}" } ] } diff --git a/templates/migration-swarm/manifest.json b/templates/migration-swarm/manifest.json index 0da44fcd1..098ec3c5a 100644 --- a/templates/migration-swarm/manifest.json +++ b/templates/migration-swarm/manifest.json @@ -8,11 +8,12 @@ { "key": "{{MIGRATION_KEY}}", "task_type": "code-fix", - "spec": "You are one worker in a mechanical migration swarm for {{PROJECT}}. Your current working directory IS a dedicated git worktree of {{REPO_PATH}}; edit files in place, leave every change UNCOMMITTED, and do not run git commit, git branch, git push, or change .git. Your boundary is strict: you own only these repo-relative files or directory prefixes, and they must be disjoint from every other task in this run: {{OWNED_FILES — semicolon-separated repo-relative files or directory prefixes this worker may modify}}. Never modify files outside that ownership set, even if the transform seems obviously related. Mechanical transform to apply: {{TRANSFORM_RULE — exact before/after rule, API rename, framework upgrade step, exclusions, and one concrete example}}. HOW TO RUN: after editing, run {{LOCAL_VERIFY — exact command from this worktree that gives useful output, e.g. npm test -- --runInBand path/to/test}}. If the command fails because the repo already has unrelated failures, record the exact inherited failure in your final note but do not broaden scope to fix it. If this task intentionally changes generated or gitignored files, keep them inside the owned paths and confirm the manifest's {{GITIGNORED_EXPORTS}} value is not NONE so the check copies them outside the deleted worktree. Output contract: do not create a report file and do not export your own patch; the check will run git add -A, export {{EXPORT_DIR}}/{{MIGRATION_KEY}}.patch, verify the patch is non-empty, verify every staged path is owned by this task, and copy any declared gitignored outputs. End your worker response with MIGRATION_DONE, the local verify command you ran, and any assumptions under 8 bullets.", - "check": "git add -A && git diff --cached > '{{EXPORT_DIR}}/{{MIGRATION_KEY}}.patch' && {{PYTHON}} '{{KIT_DIR}}/checks/migration_patch_check.py' --task-key '{{MIGRATION_KEY}}' --patch '{{EXPORT_DIR}}/{{MIGRATION_KEY}}.patch' --export-dir '{{EXPORT_DIR}}' --owned-files '{{OWNED_FILES — semicolon-separated repo-relative files or directory prefixes this worker may modify}}' --ignored-exports '{{GITIGNORED_EXPORTS}}' || { echo 'FAIL: migration patch export or validation failed for {{MIGRATION_KEY}}'; exit 1; }", + "spec": "You are one worker in a mechanical migration swarm for {{PROJECT}}. Your current working directory IS a dedicated git worktree of {{REPO_PATH}}; edit files in place, leave every change UNCOMMITTED, and do not run git commit, git branch, git push, or change .git. Your boundary is strict: you own only these repo-relative files or directory prefixes, and they must be disjoint from every other task in this run: {{OWNED_FILES \u2014 semicolon-separated repo-relative files or directory prefixes this worker may modify}}. Never modify files outside that ownership set, even if the transform seems obviously related. Mechanical transform to apply: {{TRANSFORM_RULE \u2014 exact before/after rule, API rename, framework upgrade step, exclusions, and one concrete example}}. HOW TO RUN: after editing, run {{LOCAL_VERIFY \u2014 exact command from this worktree that gives useful output, e.g. npm test -- --runInBand path/to/test}}. If the command fails because the repo already has unrelated failures, record the exact inherited failure in your final note but do not broaden scope to fix it. If this task intentionally changes generated or gitignored files, keep them inside the owned paths and confirm the manifest's {{GITIGNORED_EXPORTS}} value is not NONE so the check copies them outside the deleted worktree. Output contract: do not create a report file and do not export your own patch; the check will run git add -A, export {{EXPORT_DIR}}/{{MIGRATION_KEY}}.patch, verify the patch is non-empty, verify every staged path is owned by this task, and copy any declared gitignored outputs. End your worker response with MIGRATION_DONE, the local verify command you ran, and any assumptions under 8 bullets.", + "check": "git add -A && git diff --cached > '{{EXPORT_DIR}}/{{MIGRATION_KEY}}.patch' && {{PYTHON}} '{{KIT_DIR}}/checks/migration_patch_check.py' --task-key '{{MIGRATION_KEY}}' --patch '{{EXPORT_DIR}}/{{MIGRATION_KEY}}.patch' --export-dir '{{EXPORT_DIR}}' --owned-files '{{OWNED_FILES \u2014 semicolon-separated repo-relative files or directory prefixes this worker may modify}}' --ignored-exports '{{GITIGNORED_EXPORTS}}' || { echo 'FAIL: migration patch export or validation failed for {{MIGRATION_KEY}}'; exit 1; }", "expect_files": [], "timeout_s": 1800, - "verified": "the worker left an uncommitted mechanical change whose exported patch is non-empty, scoped to owned files, and accompanied by explicit copies for any declared gitignored outputs" + "verified": "the worker left an uncommitted mechanical change whose exported patch is non-empty, scoped to owned files, and accompanied by explicit copies for any declared gitignored outputs", + "ticket": "{{TICKET \u2014 the work item this migration serves, e.g. work#204}}" } ] } diff --git a/templates/repo-feature/manifest.json b/templates/repo-feature/manifest.json index 7b184fe3d..7a556de95 100644 --- a/templates/repo-feature/manifest.json +++ b/templates/repo-feature/manifest.json @@ -17,7 +17,8 @@ ], "spec": "You are a repo feature worker editing a real repository for {{PROJECT_NAME}}. Your current working directory is a scratch task directory; use it only for ./notes.md. The repository checkout you may edit is {{REPO_PATH}}.\n\nOWNERSHIP BOUNDARY: you own only these repo paths: {{OWNED_FILES_CSV}}. Do not modify, create, delete, format, or stage anything outside those paths. Do not touch .git, do not commit, do not push, and do not run git add -A.\n\nREAD FIRST, READ-ONLY: {{CONVENTION_FILES}}\n\nFEATURE BRIEF: {{FEATURE_BRIEF}}\n\nHOW TO RUN: after editing, run these commands from the repo root and fix failures inside your owned paths only:\n{{HOW_TO_RUN_COMMANDS}}\n\nOUTPUT CONTRACT: make the requested repo change in the owned files, then write ./notes.md in the scratch task directory. notes.md must list what you read for conventions, what files you changed, what verification command passed, and any assumption or follow-up. The repo should be left with git status showing only owned paths and any explicitly allowed pre-existing path.\n\nHARD RULES: keep the solution boring and local to the existing patterns, never edit unrelated files to satisfy a build, never add dependencies unless the brief explicitly requires them, and mark unknown product facts as assumptions rather than inventing content.", "check": "python3 '{{KIT_DIR}}/checks/check_repo_feature.py' --repo '{{REPO_PATH}}' --owned '{{OWNED_FILES_CSV}}' --allowed-status '{{ALLOWED_STATUS_PATHS_CSV}}' --required-paths '{{REQUIRED_PATHS_CSV}}' --required-text '{{REQUIRED_TEXT_CSV}}' --build-command '{{BUILD_OR_TEST_COMMAND}}' --notes notes.md", - "verified": "the repo build or test command passed, required content assertions passed, notes.md exists, and git status contains only owned or allowlisted paths." + "verified": "the repo build or test command passed, required content assertions passed, notes.md exists, and git status contains only owned or allowlisted paths.", + "ticket": "{{TICKET \u2014 the work item this feature serves, e.g. work#812}}" } ] } diff --git a/tests/test_cost_control.py b/tests/test_cost_control.py index bde54e7a3..6e3ab0510 100644 --- a/tests/test_cost_control.py +++ b/tests/test_cost_control.py @@ -173,3 +173,59 @@ def test_a_path_inside_the_task_directory_is_fine(self): if __name__ == "__main__": unittest.main() + + +class TicketAttributionTests(unittest.TestCase): + """Cost you cannot attribute to a requirement cannot be steered.""" + + def _m(self, **task_extra): + task = {"key": "t1", "spec": "do it", "check": "echo checking; test -f out", + "verified": "out exists"} + task.update(task_extra) + return ringer.Manifest.from_obj({ + "run_name": "ticket-tests", "workdir": tempfile.mkdtemp(), + "max_parallel": 1, "tasks": [task]}) + + def test_product_work_without_a_ticket_is_flagged(self): + findings = ringer.lint_manifest(self._m(task_type="code-fix")) + self.assertTrue(any("names no ticket" in f for f in findings), findings) + + def test_product_work_with_a_ticket_is_not_flagged(self): + findings = ringer.lint_manifest(self._m(task_type="code-fix", ticket="work#666")) + self.assertFalse(any("names no ticket" in f for f in findings), findings) + + def test_a_bakeoff_or_probe_needs_no_ticket(self): + # Not every run serves a requirement; the rule must be scoped or it + # becomes noise everyone learns to ignore. + for tt in ("research", "probe", "code-review", "bakeoff"): + findings = ringer.lint_manifest(self._m(task_type=tt)) + self.assertFalse(any("names no ticket" in f for f in findings), f"{tt}: {findings}") + + +class UncostedEngineTests(unittest.TestCase): + """Codex bills elsewhere. Its work must not read as free.""" + + def _engine(self, **kw): + base = dict(name="codex", bin="/bin/true", args_template=("x",), + full_access_args=(), sandbox_args=()) + base.update(kw) + return ringer.EngineConfig(**base) + + def test_no_prices_means_unknown_not_zero(self): + self.assertIsNone(ringer.estimate_cost_from_tokens(self._engine(), 50)) + + def test_token_scale_is_applied(self): + # Codex reports thousands: 50 means 50,000 tokens. At $1/Mtok that is + # $0.05, not $0.00005 -- the thousand-fold error this guards. + e = self._engine(token_scale=1000, price_in_per_mtok=1.0) + self.assertAlmostEqual(ringer.estimate_cost_from_tokens(e, 50), 0.05) + + def test_unscaled_engine_is_priced_per_token(self): + e = self._engine(token_scale=1, price_in_per_mtok=1.0) + self.assertAlmostEqual(ringer.estimate_cost_from_tokens(e, 1_000_000), 1.0) + + def test_estimate_never_lands_in_the_measured_field(self): + rt = ringer.TaskRuntime(task=ringer.TaskSpec(key="k", spec="s", check="c"), + taskdir=Path("/tmp"), log_path=Path("/tmp/none.log")) + self.assertIsNone(rt.cost_usd) + self.assertIsNone(rt.cost_estimated_usd) From 28cb853a3460c921120b8f6564e66371c82ff372 Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 10 Sep 2026 08:24:54 +0200 Subject: [PATCH 06/19] Keep one estate's stack out of everyone's linter The ticket rule shipped with `dotnet-fix` and `dotnet-feature` hard-coded into the source. Those are one estate's task-type names. Baking them into a shared tool is how a shared tool stops being shared, and it would have made the finding either wrong or invisible for every other user. The default is now only this project's own documented vocabulary -- code-fix and code-feature. An estate that coins its own product task types lists them in config: ticketed_task_types = ["code-fix", "code-feature", "dotnet-fix"] A malformed value is refused rather than quietly ignored, because a setting that silently does nothing is worse than one that is absent. Tested through the real config path rather than a hand-built AppConfig: a setting nothing can reach proves nothing about portability. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0125QPzJL8qBQsayptsvuPgB --- config.sample.toml | 11 +++++++++++ ringer.py | 37 ++++++++++++++++++++++++++++++------- tests/test_cost_control.py | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 7 deletions(-) diff --git a/config.sample.toml b/config.sample.toml index 47df85a04..3899dd53d 100644 --- a/config.sample.toml +++ b/config.sample.toml @@ -205,3 +205,14 @@ index_out = "~/.ringer/artifacts/index.html" # token_scale = 1000 # price_in_per_mtok = 1.25 # price_out_per_mtok = 10.00 + +# Which task types must name the requirement they serve. Lint flags a product +# task with no "ticket", because spend you cannot attribute to a requirement is +# spend you cannot steer -- on one estate 175 of 178 product tasks were keyed +# `fix-L13` and similar, and what a requirement cost was simply unanswerable. +# +# The default is this project's own documented vocabulary. Estates that coin +# their own product task types list theirs here rather than patching the source; +# a bakeoff, probe or research task serves no requirement and is never flagged. +# +# ticketed_task_types = ["code-fix", "code-feature", "dotnet-fix"] diff --git a/ringer.py b/ringer.py index 8f88f54e8..9b14b30ed 100755 --- a/ringer.py +++ b/ringer.py @@ -1044,6 +1044,21 @@ def load_artifact_config(raw: Any, state_dir: Path) -> ArtifactConfig: ) +# Task types that change a product and therefore answer to a requirement. A +# bakeoff, probe or research task legitimately serves none, so the finding is +# scoped rather than universal. +# +# The default is only the canonical vocabulary this project documents. Estates +# that coin their own product task types -- "dotnet-fix", "juce-test", whatever +# their stack is called -- extend it in config rather than here: +# +# ticketed_task_types = ["code-fix", "code-feature", "dotnet-fix"] +# +# Hard-coding one estate's stack into everyone's linter is how a shared tool +# stops being shared. +DEFAULT_TICKETED_TASK_TYPES = frozenset({"code-fix", "code-feature"}) + + @dataclass(frozen=True) class AppConfig: path: Path | None @@ -1058,6 +1073,9 @@ class AppConfig: artifact: ArtifactConfig steering: SteeringConfig = field(default_factory=SteeringConfig) update: UpdateConfig = field(default_factory=UpdateConfig) + # Which task types must name a ticket. Estate-specific by nature; see + # DEFAULT_TICKETED_TASK_TYPES. + ticketed_task_types: frozenset[str] = DEFAULT_TICKETED_TASK_TYPES @classmethod def load(cls, path: Path | None = None) -> "AppConfig": @@ -1085,6 +1103,15 @@ def load(cls, path: Path | None = None) -> "AppConfig": engines = load_engines(data.get("engines")) artifact_config = load_artifact_config(data.get("artifact"), state_dir) update_config = load_update_config(data.get("update")) + raw_ticketed = data.get("ticketed_task_types") + if raw_ticketed is None: + ticketed_task_types = DEFAULT_TICKETED_TASK_TYPES + elif not isinstance(raw_ticketed, list): + raise ValueError("ticketed_task_types must be a list of task-type names") + else: + ticketed_task_types = frozenset( + str(item).strip() for item in raw_ticketed if str(item).strip() + ) try: steering_config = load_steering_config(data.get("steering")) except Exception: @@ -1104,6 +1131,7 @@ def load(cls, path: Path | None = None) -> "AppConfig": artifact=artifact_config, steering=steering_config, update=update_config, + ticketed_task_types=ticketed_task_types, ) @@ -1862,10 +1890,6 @@ def with_max_parallel(self, value: int | None) -> "Manifest": FILE_TEST_OPS = {"-e", "-f", "-s", "-d", "-r", "-w", "-x", "-L"} -# Task types that change a product and therefore answer to a requirement. A -# bakeoff, probe or research task legitimately serves none, so the lint finding -# is scoped rather than universal. -TICKETED_TASK_TYPES = frozenset({"code-fix", "code-feature", "dotnet-fix", "dotnet-feature"}) def worker_unwritable_paths(task: TaskSpec, manifest: Manifest) -> list[str]: @@ -1912,6 +1936,7 @@ def lint_manifest( allow_noncanonical_route: bool = False, ) -> list[str]: findings: list[str] = [] + ticketed_types = config.ticketed_task_types if config else DEFAULT_TICKETED_TASK_TYPES if manifest.run_name == MODEL_SCOREBOARD_RUN_NAME: findings.append("manifest: run_name model-scoreboard is reserved for the scoreboard page.") @@ -1922,7 +1947,7 @@ def lint_manifest( findings.append( f"{task.key}: check may fail without printing why; retry prompt and eval log depend on failure output." ) - if task.task_type in TICKETED_TASK_TYPES and not task.ticket: + if task.task_type in ticketed_types and not task.ticket: findings.append( f"{task.key}: {task.task_type} names no ticket, so its cost and outcome " f"cannot be attributed to a requirement. Set \"ticket\"." @@ -11083,7 +11108,6 @@ def run_persistent_hud(config: AppConfig, *, port: int | None, open_viewer: bool server.stop() - def build_parser() -> argparse.ArgumentParser: parser = argparse.ArgumentParser( prog="ringer.py", @@ -11447,6 +11471,5 @@ def main(argv: list[str] | None = None) -> int: return 2 - if __name__ == "__main__": raise SystemExit(main()) diff --git a/tests/test_cost_control.py b/tests/test_cost_control.py index 6e3ab0510..906ba732b 100644 --- a/tests/test_cost_control.py +++ b/tests/test_cost_control.py @@ -229,3 +229,40 @@ def test_estimate_never_lands_in_the_measured_field(self): taskdir=Path("/tmp"), log_path=Path("/tmp/none.log")) self.assertIsNone(rt.cost_usd) self.assertIsNone(rt.cost_estimated_usd) + + +class PortabilityTests(unittest.TestCase): + """One estate's stack must not be hard-coded into everyone's linter.""" + + def _m(self, task_type): + return ringer.Manifest.from_obj({ + "run_name": "portable", "workdir": tempfile.mkdtemp(), "max_parallel": 1, + "tasks": [{"key": "t1", "spec": "s", "check": "echo c; test -f out", + "verified": "v", "task_type": task_type}]}) + + def test_default_covers_only_the_documented_vocabulary(self): + self.assertEqual(ringer.DEFAULT_TICKETED_TASK_TYPES, + frozenset({"code-fix", "code-feature"})) + + def test_an_estate_can_add_its_own_task_types(self): + # Through the real config path, because that is what another factory + # would actually do -- a hand-built AppConfig would prove nothing about + # whether the setting is reachable. + cfgdir = Path(tempfile.mkdtemp()) + (cfgdir / "config.toml").write_text( + 'ticketed_task_types = ["dotnet-fix"]\n', encoding="utf-8") + cfg = ringer.AppConfig.load(cfgdir / "config.toml") + self.assertEqual(cfg.ticketed_task_types, frozenset({"dotnet-fix"})) + # their type is enforced ... + self.assertTrue(any("names no ticket" in f + for f in ringer.lint_manifest(self._m("dotnet-fix"), config=cfg))) + # ... and the built-in default is not, because they said what theirs are + self.assertFalse(any("names no ticket" in f + for f in ringer.lint_manifest(self._m("code-fix"), config=cfg))) + + def test_a_bad_setting_is_refused_rather_than_ignored(self): + cfgdir = Path(tempfile.mkdtemp()) + (cfgdir / "config.toml").write_text( + 'ticketed_task_types = "code-fix"\n', encoding="utf-8") + with self.assertRaises(ValueError): + ringer.AppConfig.load(cfgdir / "config.toml") From c3fb0f3e981261a2f99a18a7b81fb3f859aecf41 Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 10 Sep 2026 08:42:26 +0200 Subject: [PATCH 07/19] Make the budget real, count tasks not attempts, and stop overclaiming MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An adversarial review failed this branch on three counts. All three were right, and all three came from the author verifying their own work. P1 — `budget_usd` was not a ceiling under parallelism. It was checked only when a worker exited, so with max_parallel=4 a $6 budget could reach $24 without one check firing. Live costs are now re-read on a timer while workers run, which closes most of the gap. It does not close all of it and the documentation no longer pretends otherwise: a task's price is not knowable before it runs, so nothing can be reserved, and the residual is whatever parallel workers spend between polls. "Hard ceiling" was a lie with a number attached; it now says "as soon as the spend is visible". P1 — `abort_after_repeated_failures` counted ATTEMPTS. `_note_failure` fired inside the retry loop, so one task failing twice identically reached a limit of 2 on its own and could stop the whole run. It now fires once the task is finished, retries included, which is what both the option name and the docs always said. P2 — the budget ignored estimated spend entirely, making it useless for the one case it is most needed: an engine that reports no cost of its own. On one estate that was 797 of 922 tasks. Enforcement now weighs measured plus estimated while REPORTING still keeps them apart, because presenting a guess as a measurement is how a number stops being trustworthy. Also documented what the failure signature cannot do. It is exit code plus first printed line, one streak across parallel tasks: two unrelated checks that both open with "FAIL" share it. A circuit breaker, not a diagnosis. The tests deserved the review's sharpest finding: the "must not fire" cases passed against unfixed code, because a rule that does not exist never fires. Each negative now sits beside its positive in one test, so it fails both when the rule is missing and when it is too broad. Verified in both directions -- 4 of the new tests fail against the pre-fix tree, all 22 pass after. 276 tests, 275 pass; the remaining failure is the contributor audit, which needs full git history and fails identically on an untouched tree here. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0125QPzJL8qBQsayptsvuPgB --- README.md | 31 ++++++++-- ringer.py | 80 +++++++++++++++++++++++-- tests/test_cost_control.py | 120 +++++++++++++++++++++++++++---------- 3 files changed, 189 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index c78f3e39e..e36ad2411 100644 --- a/README.md +++ b/README.md @@ -183,17 +183,36 @@ Two manifest keys stop a run that is going wrong: } ``` -- **`budget_usd`** — a hard ceiling. Cost is totted up the moment each worker - exits, before its check runs, so a slow verification cannot overshoot it. When - the ceiling is hit, in-flight workers are terminated and queued tasks are - marked `SKIPPED` with the reason. -- **`abort_after_repeated_failures`** — stop once this many tasks in a row fail - with the *same* signature (check exit code plus the first line it printed). A +- **`budget_usd`** — stops the run as soon as the spend is *visible*. Live costs + are re-read every few seconds and again the moment each worker exits; when the + figure crosses the budget, in-flight workers are terminated and queued tasks + are marked `SKIPPED` with the reason. + + ⚠️ It is **not** a hard ceiling, and calling it one would be a lie with a + number attached. A task's price is not knowable before it runs — it exists + only in what the worker has already written to its log — so nothing can be + reserved in advance. Parallel workers can therefore overshoot by whatever they + spend between one poll and the next. Set it below the number that would + actually hurt, not at it. + + The budget weighs **measured plus estimated** spend, even though the summary + reports them separately: an engine that reports no cost of its own still + spends real money, and a budget blind to it is no budget for precisely the + case it is most needed. +- **`abort_after_repeated_failures`** — stop once this many **tasks** in a row + fail with the *same* signature (check exit code plus the first line it + printed). Counted per task, retries included, not per attempt: a single task + failing twice is one failure, not two. A manifest asking for something no worker can produce fails every task identically; without this the run pays for all of them, and pays twice because each failure is retried. Different failures do not trip it — that is an ordinary bad run, not an impossible manifest. + ⚠️ The signature is deliberately cheap, and cheap means blunt. Two unrelated + checks that both open with `FAIL` share a signature, and the counter is one + streak across parallel tasks rather than a per-family tally — so it reads + completion order, not causation. It is a circuit breaker, not a diagnosis. + Both default to off, so existing manifests behave exactly as before. `lint` also refuses a manifest whose **spec** tells the worker to write an diff --git a/ringer.py b/ringer.py index 9b14b30ed..cc99dbd83 100755 --- a/ringer.py +++ b/ringer.py @@ -53,6 +53,10 @@ DEFAULT_ENGINE_NAME = "codex" DEFAULT_TIMEOUT_S = 900 CHECK_TIMEOUT_S = 60 +# How often a run with a budget re-reads live worker costs. Short enough that a +# parallel run cannot overshoot far, long enough not to re-read every log +# constantly. +BUDGET_POLL_INTERVAL_S = 10 DEFAULT_DASHBOARD_PORT_BASE = 8787 DEFAULT_HUD_PORT = 8700 DEFAULT_CATALOG_SOURCE = "https://openrouter.ai/api/v1/models" @@ -8873,7 +8877,18 @@ async def run(self) -> int: self.state_writer.start() if self.dashboard is not None: self.state_writer.set_port(self.dashboard.start()) - await asyncio.gather(*(self._run_task(runtime) for runtime in self.runtimes)) + watcher = ( + asyncio.ensure_future(self._watch_budget()) + if self.manifest.budget_usd + else None + ) + try: + await asyncio.gather(*(self._run_task(runtime) for runtime in self.runtimes)) + finally: + if watcher is not None: + watcher.cancel() + with contextlib.suppress(asyncio.CancelledError): + await watcher final_state = True return 0 if all(runtime.status == "pass" for runtime in self.runtimes) else 1 except asyncio.CancelledError: @@ -8916,10 +8931,29 @@ async def kill_all_workers(self) -> None: kill_process_group(proc) def run_cost_usd(self) -> float: - """What this run has cost so far, summed from what providers reported.""" + """Measured spend: what providers actually reported. For REPORTING.""" with self.lock: return sum(r.cost_usd or 0.0 for r in self.runtimes) + def run_exposure_usd(self) -> tuple[float, float]: + """(measured, estimated) — what a budget must weigh. For ENFORCEMENT. + + Reporting keeps these apart, because presenting a guess as a measurement + is how a number stops being trustworthy. Enforcement must add them: an + engine that reports no cost of its own still spends real money, and a + budget that ignores it is no budget at all for exactly the case it is + most needed -- a plan-billed or separately-billed worker, which on one + estate was 797 of 922 tasks. + """ + with self.lock: + measured = sum(r.cost_usd or 0.0 for r in self.runtimes) + estimated = sum( + r.cost_estimated_usd or 0.0 + for r in self.runtimes + if r.cost_usd is None + ) + return measured, estimated + def _refresh_cost(self, runtime: TaskRuntime) -> None: """Re-read the task's log; it accumulates across attempts, so this is total.""" cost, steps = parse_step_costs(runtime.log_path) @@ -8966,17 +9000,47 @@ async def _note_failure(self, runtime: TaskRuntime, verify: Any) -> None: print(f"\n*** RUN STOPPED: {self.stop_reason} ***", flush=True) await self.kill_all_workers() + async def _watch_budget(self) -> None: + """Re-read live costs on a timer, so a budget is not merely a post-mortem. + + A task's price is not knowable before it runs -- it exists only in what + the worker has already written to its log -- so no reservation is + possible. Checking only when a worker EXITS therefore lets N parallel + workers each spend the whole budget before any of them reports: with + max_parallel=4 a $6 budget can reach $24 without one check firing. + + Polling closes most of that gap. The residual is bounded by this + interval and by how fast a worker can spend inside it, which is why the + documentation says the run stops as soon as the spend is VISIBLE rather + than promising a ceiling that cannot be exceeded. A hard guarantee here + would be a lie with a number attached to it. + """ + while True: + await asyncio.sleep(BUDGET_POLL_INTERVAL_S) + with self.lock: + if self.stop_reason is not None: + return + live = [r for r in self.runtimes + if r.status in {"running", "retrying", "verifying"}] + for runtime in live: + self._refresh_cost(runtime) + await self._check_budget() + async def _check_budget(self) -> None: budget = self.manifest.budget_usd if not budget: return - spent = self.run_cost_usd() + measured, estimated = self.run_exposure_usd() + spent = measured + estimated if spent < budget: return + detail = f"${measured:.2f} measured" + if estimated: + detail += f" + ~${estimated:.2f} estimated" with self.lock: if self.stop_reason is not None: return - self.stop_reason = f"budget of ${budget:.2f} reached (spent ${spent:.2f})" + self.stop_reason = f"budget of ${budget:.2f} reached ({detail})" print(f"\n*** RUN STOPPED: {self.stop_reason} ***", flush=True) await self.kill_all_workers() @@ -9031,8 +9095,6 @@ async def _run_task(self, runtime: TaskRuntime) -> None: runtime.ended_at_monotonic = time.monotonic() await self._cleanup_worktree_on_pass(runtime) return - if verdict in {"FAIL", "TIMEOUT"}: - await self._note_failure(runtime, verify) with self.lock: stopped = self.stop_reason if stopped is not None and verdict != "PASS": @@ -9052,6 +9114,12 @@ async def _run_task(self, runtime: TaskRuntime) -> None: runtime.status = "fail" runtime.final_verdict = verdict runtime.ended_at_monotonic = time.monotonic() + # Counted once the TASK has failed, retries included -- not once + # per attempt. Counting attempts made a single task with two + # identical failures reach a limit of 2 on its own, which is not + # "tasks in a row" by any reading of the name. + if verdict in {"FAIL", "TIMEOUT"}: + await self._note_failure(runtime, verify) return def _harvest_deliverables_on_pass(self, runtime: TaskRuntime) -> None: diff --git a/tests/test_cost_control.py b/tests/test_cost_control.py index 906ba732b..fd4fb6187 100644 --- a/tests/test_cost_control.py +++ b/tests/test_cost_control.py @@ -129,26 +129,23 @@ def _manifest(self, spec, expect, worktrees=True): }], }), workdir - def test_spec_ordering_a_write_outside_the_sandbox_is_flagged(self): + def test_the_rule_fires_on_the_spec_and_not_on_the_check(self): + """Both halves in one test, on purpose. + + A lone "must not fire" assertion passes when the rule does not exist at + all, so it pins nothing. Asserting the positive beside it means this + test fails if the rule is missing AND if it is too broad -- which is the + only arrangement that actually holds a boundary in place. + """ outside = "/tmp/somewhere-else/report.json" - m, _ = self._manifest(f"Write your report to {outside} when done.", outside) - findings = ringer.lint_manifest(m) - self.assertTrue( - any("sandbox forbids" in f for f in findings), - f"expected an unwritable-path finding, got: {findings}", - ) + told_to_write, _ = self._manifest(f"Write your report to {outside} when done.", outside) + check_exports, _ = self._manifest("Leave your changes uncommitted in the worktree.", outside) - def test_the_check_exporting_to_an_absolute_path_is_fine(self): - # The fix-swarm pattern: the CHECK writes the patch out of the worktree. - # The spec never names that path, so the worker is asked for nothing - # impossible and this must NOT be flagged. - outside = "/tmp/somewhere-else/task.patch" - m, _ = self._manifest("Leave your changes uncommitted in the worktree.", outside) - findings = ringer.lint_manifest(m) - self.assertFalse( - any("sandbox forbids" in f for f in findings), - f"check-exported deliverable must not be flagged, got: {findings}", - ) + fires = [f for f in ringer.lint_manifest(told_to_write) if "sandbox forbids" in f] + quiet = [f for f in ringer.lint_manifest(check_exports) if "sandbox forbids" in f] + + self.assertTrue(fires, "the rule did not fire on a spec ordering an unwritable path") + self.assertFalse(quiet, f"the rule fired on a check-exported deliverable: {quiet}") def test_a_path_inside_the_task_directory_is_fine(self): workdir = tempfile.mkdtemp() @@ -186,20 +183,18 @@ def _m(self, **task_extra): "run_name": "ticket-tests", "workdir": tempfile.mkdtemp(), "max_parallel": 1, "tasks": [task]}) - def test_product_work_without_a_ticket_is_flagged(self): - findings = ringer.lint_manifest(self._m(task_type="code-fix")) - self.assertTrue(any("names no ticket" in f for f in findings), findings) - - def test_product_work_with_a_ticket_is_not_flagged(self): - findings = ringer.lint_manifest(self._m(task_type="code-fix", ticket="work#666")) - self.assertFalse(any("names no ticket" in f for f in findings), findings) + def _fires(self, **kw): + return [f for f in ringer.lint_manifest(self._m(**kw)) if "names no ticket" in f] - def test_a_bakeoff_or_probe_needs_no_ticket(self): - # Not every run serves a requirement; the rule must be scoped or it - # becomes noise everyone learns to ignore. + def test_the_ticket_rule_fires_on_product_work_and_nowhere_else(self): + """Positive and negatives together, so the negatives mean something.""" + self.assertTrue(self._fires(task_type="code-fix"), + "the rule did not fire on product work with no ticket") + self.assertFalse(self._fires(task_type="code-fix", ticket="work#666"), + "the rule fired despite a ticket being set") for tt in ("research", "probe", "code-review", "bakeoff"): - findings = ringer.lint_manifest(self._m(task_type=tt)) - self.assertFalse(any("names no ticket" in f for f in findings), f"{tt}: {findings}") + self.assertFalse(self._fires(task_type=tt), + f"the rule fired on {tt}, which serves no requirement") class UncostedEngineTests(unittest.TestCase): @@ -266,3 +261,68 @@ def test_a_bad_setting_is_refused_rather_than_ignored(self): 'ticketed_task_types = "code-fix"\n', encoding="utf-8") with self.assertRaises(ValueError): ringer.AppConfig.load(cfgdir / "config.toml") + + +class BudgetExposureTests(unittest.TestCase): + """A budget must weigh what an engine cannot report, or it is no budget.""" + + class _Stub: + """Minimal stand-in: run_exposure_usd needs only a lock and runtimes.""" + def __init__(self, runtimes): + import threading + self.lock = threading.Lock() + self.runtimes = runtimes + # Resolved lazily: bound at class-definition time, a missing method + # aborts the whole module import and every test in the file reports the + # same opaque error instead of its own. + def run_exposure_usd(self): + return ringer.RingerRunner.run_exposure_usd(self) + + def run_cost_usd(self): + return ringer.RingerRunner.run_cost_usd(self) + + def _rt(self, measured=None, estimated=None): + rt = ringer.TaskRuntime(task=ringer.TaskSpec(key="k", spec="s", check="c"), + taskdir=Path("/tmp"), log_path=Path("/tmp/x.log")) + rt.cost_usd = measured + rt.cost_estimated_usd = estimated + return rt + + def test_exposure_adds_the_estimate_that_reporting_keeps_apart(self): + s = self._Stub([self._rt(measured=1.0), self._rt(estimated=2.0)]) + measured, estimated = s.run_exposure_usd() + self.assertAlmostEqual(measured, 1.0) + self.assertAlmostEqual(estimated, 2.0) + # reporting stays measured-only, so a guess is never shown as a fact + self.assertAlmostEqual(s.run_cost_usd(), 1.0) + + def test_a_measured_task_does_not_also_count_its_estimate(self): + # Both fields set: only the measurement counts, or the same spend is + # weighed twice and the budget trips early. + s = self._Stub([self._rt(measured=5.0, estimated=99.0)]) + measured, estimated = s.run_exposure_usd() + self.assertAlmostEqual(measured, 5.0) + self.assertAlmostEqual(estimated, 0.0) + + def test_an_engine_that_reports_nothing_still_reaches_the_budget(self): + # The case the whole thing exists for: a plan-billed worker whose spend + # is real but invisible to the provider total. + s = self._Stub([self._rt(estimated=7.0)]) + measured, estimated = s.run_exposure_usd() + self.assertAlmostEqual(measured + estimated, 7.0) + self.assertGreaterEqual(measured + estimated, 6.0) # would trip a $6 budget + + +class FailureCountingTests(unittest.TestCase): + """Counting attempts made one task trip a limit meant for several.""" + + def test_note_failure_is_called_once_a_task_is_finished_not_per_attempt(self): + src = Path(ringer.__file__).read_text(encoding="utf-8") + loop = src[src.index(" async def _run_task"):src.index(" def _harvest_deliverables_on_pass")] + note = loop.index("await self._note_failure") + retry = loop.index('if attempt < max_attempts and verdict in {"FAIL", "TIMEOUT"}:') + self.assertGreater( + note, retry, + "._note_failure must sit AFTER the retry branch, or a single task's " + "repeated attempts count as repeated task failures", + ) From 4b9d4ea8e5fe74d491413f80fafd62d6e19f05b4 Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 10 Sep 2026 08:48:23 +0200 Subject: [PATCH 08/19] Pass 2: a stopped run reported success, and the default was still a policy Five findings from the second review. Four fixed, one declined with a reason. NEW, and the worst of them: a run stopped by its budget could exit ZERO. The stop killed the workers, but `run()` returned success whenever every runtime that finished happened to pass, without consulting `stop_reason`. A budget stop could therefore pass a CI gate silently -- the exact shape of failure this branch exists to prevent. It now returns 1 and says why. The manifest field comment still read "Hard ceiling" after the README had been corrected, which is worse than never having fixed it: the two now disagreed, and a reader believes whichever they find first. The lint docstring claimed a worker may write in its task directory "and its assigned temp dir", while the code exempts only the task directory. The temp path is assigned at run time and is not knowable at lint time, so the doc was narrowed to what the code actually reasons about rather than the code widened to a promise it cannot keep. `ticketed_task_types` now defaults to EMPTY. Shipping `code-fix` and `code-feature` enabled was still one estate's idea of product work handed to every installation as a new lint failure it never asked for. The reviewer was right that "configurable" does not excuse a default policy. One more negative-only test was paired with its positive, and the count in the previous message was wrong: 22 methods, not 21. DECLINED -- the failure signature is exit code plus first printed line, so two unrelated checks that both open with "FAIL" can collide. Making it precise means tracking per-check failure families, which defeats the purpose: an impossible manifest fails IDENTICALLY, and that sameness is the whole signal. A more specific signature would make every task unique and the breaker would never fire. It is documented as a circuit breaker rather than a diagnosis, and the cost of a false trip is a stopped run, not a wrong result. 277 tests, 276 pass. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0125QPzJL8qBQsayptsvuPgB --- README.md | 12 +++++++++++ config.sample.toml | 7 ++++--- ringer.py | 25 ++++++++++++++++++----- tests/test_cost_control.py | 42 +++++++++++++++++++++++++++++--------- 4 files changed, 68 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index e36ad2411..c1753ec2d 100644 --- a/README.md +++ b/README.md @@ -215,6 +215,18 @@ Two manifest keys stop a run that is going wrong: Both default to off, so existing manifests behave exactly as before. +`lint` can also require that product work names the requirement it serves. This +is **off by default** — upstream ships no opinion about how you track work — and +turns on by naming the task types it applies to: + +```toml +ticketed_task_types = ["code-fix", "code-feature"] +``` + +Spend you cannot attribute to a requirement is spend you cannot steer: on one +estate 175 of 178 product tasks were keyed `fix-L13` and similar, so "what did +this requirement cost?" had no answer. + `lint` also refuses a manifest whose **spec** tells the worker to write an absolute path outside its own task directory. A worker may only write inside its task directory and its assigned temp dir. An absolute path in `expect_files` is diff --git a/config.sample.toml b/config.sample.toml index 3899dd53d..230332a4d 100644 --- a/config.sample.toml +++ b/config.sample.toml @@ -211,8 +211,9 @@ index_out = "~/.ringer/artifacts/index.html" # spend you cannot steer -- on one estate 175 of 178 product tasks were keyed # `fix-L13` and similar, and what a requirement cost was simply unanswerable. # -# The default is this project's own documented vocabulary. Estates that coin -# their own product task types list theirs here rather than patching the source; -# a bakeoff, probe or research task serves no requirement and is never flagged. +# OFF by default -- this is a policy, and a shared tool does not get to decide +# that its users track requirements the way its author does. Name the task types +# it should apply to and it switches on; a bakeoff, probe or research task serves +# no requirement and is never flagged. # # ticketed_task_types = ["code-fix", "code-feature", "dotnet-fix"] diff --git a/ringer.py b/ringer.py index cc99dbd83..91caf54e2 100755 --- a/ringer.py +++ b/ringer.py @@ -1060,7 +1060,11 @@ def load_artifact_config(raw: Any, state_dir: Path) -> ArtifactConfig: # # Hard-coding one estate's stack into everyone's linter is how a shared tool # stops being shared. -DEFAULT_TICKETED_TASK_TYPES = frozenset({"code-fix", "code-feature"}) +# Empty by default: OFF unless an estate opts in. Even "code-fix" and +# "code-feature" are a policy -- shipping them enabled would hand every +# installation a new lint failure it never asked for, and a shared tool does not +# get to decide that its users track requirements the way its author does. +DEFAULT_TICKETED_TASK_TYPES: frozenset[str] = frozenset() @dataclass(frozen=True) @@ -1787,8 +1791,9 @@ class Manifest: repo: Path | None tasks: tuple[TaskSpec, ...] source_path: Path | None = None - # Hard ceiling on what this run may spend, in USD, enforced while it runs. - # None means unlimited, which is the historical behaviour. + # Spend limit in USD. The run stops as soon as the spend is VISIBLE -- see + # _watch_budget for why this cannot be a hard ceiling. None means unlimited, + # which is the historical behaviour. budget_usd: float | None = None # Stop the run once this many tasks have failed in a row with the SAME # failure signature. A manifest whose deliverable is impossible fails every @@ -1899,8 +1904,10 @@ def with_max_parallel(self, value: int | None) -> "Manifest": def worker_unwritable_paths(task: TaskSpec, manifest: Manifest) -> list[str]: """Absolute paths a spec orders the worker to write that its sandbox refuses. - A worker may write inside its own task directory and its assigned temp dir, - and nowhere else. A spec naming an absolute path outside that -- and listing + A worker may write inside its own task directory. It also has a temp dir, + but that path is assigned at run time and is not knowable here, so this + check deliberately reasons about the task directory alone: a spec that names + an absolute path outside it -- and lists it in expect_files, so the file is genuinely expected from the worker rather than merely mentioned -- describes an impossible task. Every attempt fails, and every attempt is retried. @@ -8890,6 +8897,14 @@ async def run(self) -> int: with contextlib.suppress(asyncio.CancelledError): await watcher final_state = True + with self.lock: + stopped = self.stop_reason + if stopped is not None: + # A run that was cut short did not do what the manifest asked, + # even if every task that finished happened to pass. Reporting + # success here would let a budget stop pass a CI gate silently. + print(f"\nrun did not complete: {stopped}", flush=True) + return 1 return 0 if all(runtime.status == "pass" for runtime in self.runtimes) else 1 except asyncio.CancelledError: await self.kill_all_workers() diff --git a/tests/test_cost_control.py b/tests/test_cost_control.py index fd4fb6187..7ae0233a4 100644 --- a/tests/test_cost_control.py +++ b/tests/test_cost_control.py @@ -148,6 +148,8 @@ def test_the_rule_fires_on_the_spec_and_not_on_the_check(self): self.assertFalse(quiet, f"the rule fired on a check-exported deliverable: {quiet}") def test_a_path_inside_the_task_directory_is_fine(self): + # Paired below with the positive, for the same reason as the others: a + # negative alone passes against a linter that has no rule at all. workdir = tempfile.mkdtemp() inside = str(Path(workdir) / "scout1" / "report.json") m = ringer.Manifest.from_obj({ @@ -164,8 +166,11 @@ def test_a_path_inside_the_task_directory_is_fine(self): "expect_files": [inside], }], }) - findings = ringer.lint_manifest(m) - self.assertFalse(any("sandbox forbids" in f for f in findings), findings) + inside_findings = [f for f in ringer.lint_manifest(m) if "sandbox forbids" in f] + self.assertFalse(inside_findings, f"a path inside the task dir was flagged: {inside_findings}") + outside, _ = self._manifest("Write /tmp/elsewhere/r.json", "/tmp/elsewhere/r.json") + self.assertTrue([f for f in ringer.lint_manifest(outside) if "sandbox forbids" in f], + "the rule is not active, so the negative above proves nothing") if __name__ == "__main__": @@ -183,17 +188,32 @@ def _m(self, **task_extra): "run_name": "ticket-tests", "workdir": tempfile.mkdtemp(), "max_parallel": 1, "tasks": [task]}) - def _fires(self, **kw): - return [f for f in ringer.lint_manifest(self._m(**kw)) if "names no ticket" in f] + def _cfg(self, types): + cfgdir = Path(tempfile.mkdtemp()) + listed = ", ".join(f'"{t}"' for t in types) + (cfgdir / "config.toml").write_text(f"ticketed_task_types = [{listed}]\n", encoding="utf-8") + return ringer.AppConfig.load(cfgdir / "config.toml") + + def _fires(self, config=None, **kw): + return [f for f in ringer.lint_manifest(self._m(**kw), config=config) + if "names no ticket" in f] + + def test_the_rule_is_off_until_an_estate_opts_in(self): + # Upstream ships no policy: a shared tool does not hand every install a + # lint failure it never asked for. + self.assertEqual(ringer.DEFAULT_TICKETED_TASK_TYPES, frozenset()) + self.assertFalse(self._fires(task_type="code-fix"), + "the rule fired with no configuration — that is a default policy") def test_the_ticket_rule_fires_on_product_work_and_nowhere_else(self): """Positive and negatives together, so the negatives mean something.""" - self.assertTrue(self._fires(task_type="code-fix"), + cfg = self._cfg(["code-fix", "code-feature"]) + self.assertTrue(self._fires(config=cfg, task_type="code-fix"), "the rule did not fire on product work with no ticket") - self.assertFalse(self._fires(task_type="code-fix", ticket="work#666"), + self.assertFalse(self._fires(config=cfg, task_type="code-fix", ticket="work#666"), "the rule fired despite a ticket being set") for tt in ("research", "probe", "code-review", "bakeoff"): - self.assertFalse(self._fires(task_type=tt), + self.assertFalse(self._fires(config=cfg, task_type=tt), f"the rule fired on {tt}, which serves no requirement") @@ -235,9 +255,11 @@ def _m(self, task_type): "tasks": [{"key": "t1", "spec": "s", "check": "echo c; test -f out", "verified": "v", "task_type": task_type}]}) - def test_default_covers_only_the_documented_vocabulary(self): - self.assertEqual(ringer.DEFAULT_TICKETED_TASK_TYPES, - frozenset({"code-fix", "code-feature"})) + def test_upstream_ships_no_default_policy(self): + # Superseded the earlier assertion that the default was code-fix + + # code-feature: even that is one estate's idea of product work, and a + # shared tool imposing it is the portability defect in miniature. + self.assertEqual(ringer.DEFAULT_TICKETED_TASK_TYPES, frozenset()) def test_an_estate_can_add_its_own_task_types(self): # Through the real config path, because that is what another factory From 64c7e6906b4dbdc67232682ea77aeb41a4b60b2c Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 10 Sep 2026 08:53:36 +0200 Subject: [PATCH 09/19] Pass 3: the estimate repeated the exact undercount it was written to expose Six findings, five fixed and one previously-declined finding now implemented after the reviewer rejected the reasoning. It was right to. P1, and the sharpest catch of the three passes: the cost ESTIMATE was priced from `runtime.tokens` -- the single-step field this entire change exists to condemn. Every engine with no measured cost, which is the only kind the estimate serves, was therefore underpriced by the same 19x-40x factor, and its budget exposure could sit under the limit while real spend continued. parse_step_tokens() now sums the stream; the legacy field remains only as a fallback for harnesses that stream nothing. `budget_usd` accepted NaN and infinity. NaN is truthy so it started the watcher, and `spent < NaN` is false, so the run stopped immediately for no reason. Infinity was a ceiling nothing could reach. Both now refused. The test file put `if __name__ == "__main__"` in the middle: five of eight classes were defined after it, so running the file directly executed 15 of 28 tests and silently skipped every one added since. Moved to the end. Direct execution now runs all 28. The README still granted a worker its "assigned temp dir" while the code reasons only about the task directory -- the same doc-contradicts-code shape as the ceiling wording last pass, in the file I had already corrected once. PREVIOUSLY DECLINED, now fixed: the failure counter was one streak, so interleaved parallel failures (A,B,A,B) never reached a threshold even though A had failed twice identically, and completion order could make unrelated failures look consecutive. It is now a tally per signature, which answers the question actually being asked. The reviewer's objection was concrete and the original defence -- that sameness is the signal -- argued for per-signature counting rather than against it. Template ticket examples are neutral placeholders instead of one estate's `work#...` convention. 277 tests via discovery, 28 in this file directly; the four new ones fail against the pre-fix tree. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0125QPzJL8qBQsayptsvuPgB --- README.md | 3 +- ringer.py | 80 ++++++++++++++++++++----- templates/fix-swarm/manifest.json | 2 +- templates/migration-swarm/manifest.json | 2 +- templates/repo-feature/manifest.json | 2 +- tests/test_cost_control.py | 64 +++++++++++++++++++- 6 files changed, 132 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index c1753ec2d..a45eff1f0 100644 --- a/README.md +++ b/README.md @@ -229,7 +229,8 @@ this requirement cost?" had no answer. `lint` also refuses a manifest whose **spec** tells the worker to write an absolute path outside its own task directory. A worker may only write inside its -task directory and its assigned temp dir. An absolute path in `expect_files` is +own task directory as far as this check can reason: it also has a temp dir, but +that path is assigned at run time and is not knowable when linting. An absolute path in `expect_files` is perfectly normal when the *check* produces it — the fix-swarm pattern exports a patch out of the worktree that way — so only the spec naming the path is flagged. diff --git a/ringer.py b/ringer.py index 91caf54e2..54a965868 100755 --- a/ringer.py +++ b/ringer.py @@ -7,6 +7,7 @@ import contextlib import hashlib import json +import math import mimetypes import os import re @@ -1846,8 +1847,11 @@ def from_obj(cls, obj: dict[str, Any]) -> "Manifest": budget_usd: float | None = None if budget_raw is not None: budget_usd = float(budget_raw) - if budget_usd <= 0: - raise ValueError("budget_usd must be positive") + # NaN is truthy and compares False against everything, so it would + # start the watcher and then stop the run immediately; infinity is a + # ceiling nothing can reach. Neither is a budget. + if not math.isfinite(budget_usd) or budget_usd <= 0: + raise ValueError("budget_usd must be a positive, finite number") abort_raw = obj.get("abort_after_repeated_failures") abort_after: int | None = None if abort_raw is not None: @@ -8874,8 +8878,13 @@ def __init__( # Set once when the run must stop early: budget spent, or the same # failure repeating. Tasks that have not started check it and skip. self.stop_reason: str | None = None - self.recent_failure_signature: str | None = None - self.repeated_failures: int = 0 + # Counted PER SIGNATURE rather than as one streak. With workers finishing + # in arbitrary order a single streak counter mixes unrelated families: + # A,B,A,B never reaches 2 as a streak, yet A has failed twice the same + # way, and two unrelated failures can look consecutive purely because of + # completion order. A per-signature tally answers the question actually + # being asked -- has THIS failure now happened N times. + self.failure_counts: dict[str, int] = {} async def run(self) -> int: self.manifest.workdir.mkdir(parents=True, exist_ok=True) @@ -8977,8 +8986,14 @@ def _refresh_cost(self, runtime: TaskRuntime) -> None: if cost is not None: runtime.cost_usd = cost runtime.model_steps = steps - if cost is None and engine is not None and runtime.tokens: - runtime.cost_estimated_usd = estimate_cost_from_tokens(engine, runtime.tokens) + if cost is None and engine is not None: + # All steps, never runtime.tokens -- that field is one step, and + # pricing from it underestimates by the very factor this module + # was written to expose. + streamed = parse_step_tokens(runtime.log_path) + billable = streamed or (runtime.tokens or 0) + if billable: + runtime.cost_estimated_usd = estimate_cost_from_tokens(engine, billable) async def _note_failure(self, runtime: TaskRuntime, verify: Any) -> None: """Track identical consecutive failures so an impossible manifest stops early. @@ -8999,17 +9014,13 @@ async def _note_failure(self, runtime: TaskRuntime, verify: Any) -> None: break signature = f"rc={getattr(verify, 'check_returncode', None)}|{first_line}" with self.lock: - if signature == self.recent_failure_signature: - self.repeated_failures += 1 - else: - self.recent_failure_signature = signature - self.repeated_failures = 1 - tripped = self.repeated_failures >= limit and self.stop_reason is None + seen = self.failure_counts.get(signature, 0) + 1 + self.failure_counts[signature] = seen + tripped = seen >= limit and self.stop_reason is None if tripped: self.stop_reason = ( - f"{self.repeated_failures} tasks failed in a row with the same " - f"failure ({first_line or 'no output'}) -- stopping rather than " - f"paying for the rest of the manifest" + f"{seen} tasks failed the same way ({first_line or 'no output'}) " + f"-- stopping rather than paying for the rest of the manifest" ) if tripped: print(f"\n*** RUN STOPPED: {self.stop_reason} ***", flush=True) @@ -9786,6 +9797,45 @@ def estimate_cost_from_tokens(engine: "EngineConfig", tokens: int) -> float | No return scaled * float(rate) / 1_000_000 +def parse_step_tokens(log_path: Path) -> int: + """Total tokens across every step in a worker log. + + `TaskRuntime.tokens` holds ONE step -- that is the defect this whole module + exists to correct -- so estimating a price from it reproduces the same + 19x-40x undercount in the estimate, for exactly the engines that have no + measured cost to fall back on. Sum the stream instead. + + Returns 0 when the log carries no per-step token counts, which is the honest + answer for a harness that streams nothing. + """ + total = 0 + try: + handle = log_path.open("r", encoding="utf-8", errors="replace") + except OSError: + return 0 + with handle: + for line in handle: + line = line.strip() + if not line.startswith("{"): + continue + try: + event = json.loads(line) + except ValueError: + continue + if not isinstance(event, dict) or event.get("type") != "step_finish": + continue + part = event.get("part") + if not isinstance(part, dict): + continue + tokens = part.get("tokens") + if isinstance(tokens, dict): + for field in ("input", "output"): + value = tokens.get(field) + if isinstance(value, (int, float)) and not isinstance(value, bool): + total += int(value) + return total + + def parse_step_costs(log_path: Path) -> tuple[float | None, int]: """Sum what the provider says each model call cost, from the worker's own log. diff --git a/templates/fix-swarm/manifest.json b/templates/fix-swarm/manifest.json index 15c337617..58a3fa672 100644 --- a/templates/fix-swarm/manifest.json +++ b/templates/fix-swarm/manifest.json @@ -13,7 +13,7 @@ "expect_files": [], "timeout_s": 1800, "verified": "the validator ran the requested build or test command, exported a non-empty patch, and confirmed the patch only changes declared owned files", - "ticket": "{{TICKET \u2014 the work item this fix serves, e.g. work#666}}" + "ticket": "{{TICKET \u2014 the work item this task serves, in whatever form your tracker uses}}" } ] } diff --git a/templates/migration-swarm/manifest.json b/templates/migration-swarm/manifest.json index 098ec3c5a..1e94ceaf4 100644 --- a/templates/migration-swarm/manifest.json +++ b/templates/migration-swarm/manifest.json @@ -13,7 +13,7 @@ "expect_files": [], "timeout_s": 1800, "verified": "the worker left an uncommitted mechanical change whose exported patch is non-empty, scoped to owned files, and accompanied by explicit copies for any declared gitignored outputs", - "ticket": "{{TICKET \u2014 the work item this migration serves, e.g. work#204}}" + "ticket": "{{TICKET \u2014 the work item this task serves, in whatever form your tracker uses}}" } ] } diff --git a/templates/repo-feature/manifest.json b/templates/repo-feature/manifest.json index 7a556de95..1aabb942a 100644 --- a/templates/repo-feature/manifest.json +++ b/templates/repo-feature/manifest.json @@ -18,7 +18,7 @@ "spec": "You are a repo feature worker editing a real repository for {{PROJECT_NAME}}. Your current working directory is a scratch task directory; use it only for ./notes.md. The repository checkout you may edit is {{REPO_PATH}}.\n\nOWNERSHIP BOUNDARY: you own only these repo paths: {{OWNED_FILES_CSV}}. Do not modify, create, delete, format, or stage anything outside those paths. Do not touch .git, do not commit, do not push, and do not run git add -A.\n\nREAD FIRST, READ-ONLY: {{CONVENTION_FILES}}\n\nFEATURE BRIEF: {{FEATURE_BRIEF}}\n\nHOW TO RUN: after editing, run these commands from the repo root and fix failures inside your owned paths only:\n{{HOW_TO_RUN_COMMANDS}}\n\nOUTPUT CONTRACT: make the requested repo change in the owned files, then write ./notes.md in the scratch task directory. notes.md must list what you read for conventions, what files you changed, what verification command passed, and any assumption or follow-up. The repo should be left with git status showing only owned paths and any explicitly allowed pre-existing path.\n\nHARD RULES: keep the solution boring and local to the existing patterns, never edit unrelated files to satisfy a build, never add dependencies unless the brief explicitly requires them, and mark unknown product facts as assumptions rather than inventing content.", "check": "python3 '{{KIT_DIR}}/checks/check_repo_feature.py' --repo '{{REPO_PATH}}' --owned '{{OWNED_FILES_CSV}}' --allowed-status '{{ALLOWED_STATUS_PATHS_CSV}}' --required-paths '{{REQUIRED_PATHS_CSV}}' --required-text '{{REQUIRED_TEXT_CSV}}' --build-command '{{BUILD_OR_TEST_COMMAND}}' --notes notes.md", "verified": "the repo build or test command passed, required content assertions passed, notes.md exists, and git status contains only owned or allowlisted paths.", - "ticket": "{{TICKET \u2014 the work item this feature serves, e.g. work#812}}" + "ticket": "{{TICKET \u2014 the work item this task serves, in whatever form your tracker uses}}" } ] } diff --git a/tests/test_cost_control.py b/tests/test_cost_control.py index 7ae0233a4..ddc3bf460 100644 --- a/tests/test_cost_control.py +++ b/tests/test_cost_control.py @@ -173,8 +173,6 @@ def test_a_path_inside_the_task_directory_is_fine(self): "the rule is not active, so the negative above proves nothing") -if __name__ == "__main__": - unittest.main() class TicketAttributionTests(unittest.TestCase): @@ -348,3 +346,65 @@ def test_note_failure_is_called_once_a_task_is_finished_not_per_attempt(self): "._note_failure must sit AFTER the retry branch, or a single task's " "repeated attempts count as repeated task failures", ) + + +class EstimateUsesEveryStepTests(unittest.TestCase): + """The estimate must not repeat the very undercount this module exposes.""" + + def _log(self, steps): + tmp = Path(tempfile.mkdtemp()) / "worker.log" + lines = [json.dumps({"type": "step_finish", + "part": {"tokens": {"input": i, "output": o}}}) + for i, o in steps] + tmp.write_text("\n".join(lines) + "\n", encoding="utf-8") + return tmp + + def test_tokens_are_summed_over_every_step(self): + log = self._log([(10_000, 100), (20_000, 200), (30_000, 300)]) + self.assertEqual(ringer.parse_step_tokens(log), 60_600) + + def test_it_is_not_the_largest_single_step(self): + # The exact failure mode being corrected: one step is not the total. + log = self._log([(10_000, 0), (30_000, 0), (5_000, 0)]) + total = ringer.parse_step_tokens(log) + self.assertEqual(total, 45_000) + self.assertNotEqual(total, 30_000, "priced from one step, as the old field did") + + def test_a_silent_harness_yields_zero_not_a_guess(self): + tmp = Path(tempfile.mkdtemp()) / "worker.log" + tmp.write_text("no json here\n", encoding="utf-8") + self.assertEqual(ringer.parse_step_tokens(tmp), 0) + + +class BudgetValidationTests(unittest.TestCase): + def test_nan_and_infinity_are_refused(self): + for bad in (float("nan"), float("inf")): + with self.assertRaises(ValueError): + ringer.Manifest.from_obj({ + "run_name": "b", "workdir": tempfile.mkdtemp(), "max_parallel": 1, + "budget_usd": bad, + "tasks": [{"key": "t", "spec": "s", "check": "echo c; test -f o", + "verified": "v"}]}) + + +class InterleavedFailureTests(unittest.TestCase): + """A,B,A,B is two repeats of A, not a broken streak.""" + + class _Stub: + def __init__(self): + import threading + self.lock = threading.Lock() + self.failure_counts = {} + self.stop_reason = None + + def test_counts_are_kept_per_signature(self): + s = self._Stub() + for sig in ("A", "B", "A", "B"): + with s.lock: + s.failure_counts[sig] = s.failure_counts.get(sig, 0) + 1 + self.assertEqual(s.failure_counts, {"A": 2, "B": 2}, + "interleaved failures must not reset each other") + + +if __name__ == "__main__": + unittest.main() From c810e538a68e811f7e1d02754f478559c9f138ac Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 10 Sep 2026 09:09:59 +0200 Subject: [PATCH 10/19] Pass 4: make the enforcement paths actually run in a test Five findings fixed. The one that mattered most was not a bug but an absence: every test of the budget and the circuit breaker called a helper and compared numbers. Nothing started the watcher, stopped a run, killed a worker or exercised a retry -- which is exactly where the previous three passes found their defects. tests/test_enforcement_e2e.py runs the real loop with no model involved. An engine is a binary plus arguments, so a shell that prints step_finish JSON is a perfectly good worker: the runner reads cost from the log either way. Three cases -- a budget stop that must also make the run report failure, identical failures that must stop the manifest early, and a healthy run that must NOT be stopped. That third one is the point: without it the first two would pass against a runner that always aborts. The two enforcement tests fail against upstream and pass here. Provider cost is untrusted input and was summed unchecked. A NaN makes `spent < budget` false forever and disables the stop; a negative refunds exposure the run has really spent. Both are now skipped rather than poisoning the total. The manifest budget was already validated; the log was not. The README claimed the breaker counts failures "in a row" while the code now keeps a tally per signature. The tally is right -- workers finish in arbitrary order and a streak misses the repetition worth stopping on -- so the documentation was corrected to match, not the code. The estimate's fallback comment claimed the legacy token field is broken. For the harness that actually needs the fallback that is unproven: measured over 619 Codex logs there are zero step_finish events, and its "tokens used" median sits in the same range as OpenCode's fresh input for comparable work, so it reads as a task total rather than one step. The comment now says what is known, what is not, and that token_scale is the per-engine correction. Remaining estate-specific anecdotes removed from the shared README, sample config and templates. 285 tests, 284 pass. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0125QPzJL8qBQsayptsvuPgB --- README.md | 14 ++-- config.sample.toml | 7 +- ringer.py | 26 ++++-- tests/test_enforcement_e2e.py | 144 ++++++++++++++++++++++++++++++++++ 4 files changed, 177 insertions(+), 14 deletions(-) create mode 100644 tests/test_enforcement_e2e.py diff --git a/README.md b/README.md index a45eff1f0..938b299a0 100644 --- a/README.md +++ b/README.md @@ -199,10 +199,12 @@ Two manifest keys stop a run that is going wrong: reports them separately: an engine that reports no cost of its own still spends real money, and a budget blind to it is no budget for precisely the case it is most needed. -- **`abort_after_repeated_failures`** — stop once this many **tasks** in a row - fail with the *same* signature (check exit code plus the first line it - printed). Counted per task, retries included, not per attempt: a single task - failing twice is one failure, not two. A +- **`abort_after_repeated_failures`** — stop once this many **tasks** have + failed the *same* way (check exit code plus the first line it printed). + Counted per task, retries included, not per attempt: a single task failing + twice is one failure, not two. Not a consecutive streak — a tally per + signature, so A, B, A still counts two A failures. Workers finish in arbitrary + order, and a streak would miss exactly the repetition worth stopping on. A manifest asking for something no worker can produce fails every task identically; without this the run pays for all of them, and pays twice because each failure is retried. Different failures do not trip it — that is an @@ -224,8 +226,8 @@ ticketed_task_types = ["code-fix", "code-feature"] ``` Spend you cannot attribute to a requirement is spend you cannot steer: on one -estate 175 of 178 product tasks were keyed `fix-L13` and similar, so "what did -this requirement cost?" had no answer. +estate almost every product task was keyed by lane number rather than by the +work item it served, so "what did this requirement cost?" had no answer. `lint` also refuses a manifest whose **spec** tells the worker to write an absolute path outside its own task directory. A worker may only write inside its diff --git a/config.sample.toml b/config.sample.toml index 230332a4d..9a24b1833 100644 --- a/config.sample.toml +++ b/config.sample.toml @@ -208,12 +208,13 @@ index_out = "~/.ringer/artifacts/index.html" # Which task types must name the requirement they serve. Lint flags a product # task with no "ticket", because spend you cannot attribute to a requirement is -# spend you cannot steer -- on one estate 175 of 178 product tasks were keyed -# `fix-L13` and similar, and what a requirement cost was simply unanswerable. +# spend you cannot steer -- on one estate almost every product task was keyed +# by lane number rather than by work item, and what a requirement cost was +# simply unanswerable. # # OFF by default -- this is a policy, and a shared tool does not get to decide # that its users track requirements the way its author does. Name the task types # it should apply to and it switches on; a bakeoff, probe or research task serves # no requirement and is never flagged. # -# ticketed_task_types = ["code-fix", "code-feature", "dotnet-fix"] +# ticketed_task_types = ["code-fix", "code-feature"] diff --git a/ringer.py b/ringer.py index 54a965868..03e141236 100755 --- a/ringer.py +++ b/ringer.py @@ -8987,9 +8987,19 @@ def _refresh_cost(self, runtime: TaskRuntime) -> None: runtime.cost_usd = cost runtime.model_steps = steps if cost is None and engine is not None: - # All steps, never runtime.tokens -- that field is one step, and - # pricing from it underestimates by the very factor this module - # was written to expose. + # Prefer the streamed total: summing every step is right by + # construction. Harnesses that stream nothing (Codex prints only + # a "tokens used" line) leave this at 0, and the engine's own + # reported figure is the only number available. + # + # Whether that figure is a running total or a single step is a + # property of the harness, not something knowable here -- which + # is what `token_scale` exists to correct per engine. Measured on + # Codex: 619 logs, zero step_finish events, and a "tokens used" + # value whose median (47, i.e. 47k) sits in the same range as + # OpenCode's fresh input for comparable work, so it reads as a + # task total rather than one step. Confirm before trusting it for + # a new engine. streamed = parse_step_tokens(runtime.log_path) billable = streamed or (runtime.tokens or 0) if billable: @@ -9878,8 +9888,14 @@ def parse_step_costs(log_path: Path) -> tuple[float | None, int]: continue cost = part.get("cost") if isinstance(cost, (int, float)) and not isinstance(cost, bool): - total += float(cost) - saw_cost = True + value = float(cost) + # A worker log is untrusted input. NaN compares false against + # everything, so one would make `spent < budget` false forever + # and disable the stop; a negative would refund exposure the run + # has actually spent. Skip both rather than poison the total. + if math.isfinite(value) and value >= 0: + total += value + saw_cost = True return (total if saw_cost else None), steps diff --git a/tests/test_enforcement_e2e.py b/tests/test_enforcement_e2e.py new file mode 100644 index 000000000..1e62c5fbc --- /dev/null +++ b/tests/test_enforcement_e2e.py @@ -0,0 +1,144 @@ +#!/usr/bin/env python3 +"""End-to-end: the budget and the circuit breaker must actually fire. + +Everything else about these controls was tested by calling helpers and +comparing numbers. That leaves the part most likely to be wrong untested -- +an async watcher, a shared counter, worker termination and the run's exit +code -- which is precisely where the first three review passes found defects. + +No model is involved. An engine is a binary plus arguments, so a shell that +prints step_finish JSON is a perfectly good worker for this: the runner reads +cost from the log either way. +""" +from __future__ import annotations + +import json +import os +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] + + +def q(value: object) -> str: + return json.dumps(str(value)) + + +class EnforcementEndToEndTests(unittest.TestCase): + def _config(self, root: Path, worker_sh: str) -> Path: + path = root / "config.toml" + path.write_text("\n".join([ + f"state_dir = {q(root / 'state')}", + "", + "[eval]", + 'backend = "jsonl"', + f"jsonl_path = {q(root / 'runs.jsonl')}", + "", + "[artifact]", + "enabled = false", + "", + "[engines.spender]", + f"bin = {q('/bin/sh')}", + "args_template = [", + ' "-c",', + f" {q(worker_sh)},", + "]", + "sandbox_args = []", + "full_access_args = []", + "", + ]), encoding="utf-8") + return path + + def _run(self, root: Path, manifest: dict, config: Path): + manifest_path = root / "manifest.json" + manifest_path.write_text(json.dumps(manifest, indent=2), encoding="utf-8") + env = os.environ.copy() + env.update(HOME=str(root / "home"), RINGER_HOME=str(root / "rhome"), + XDG_CONFIG_HOME=str(root / "xdg"), RINGER_NO_SELF_UPDATE="1") + (root / "home").mkdir(exist_ok=True) + (root / "rhome").mkdir(exist_ok=True) + return subprocess.run( + [sys.executable, "ringer.py", "run", str(manifest_path), + "--config", str(config), "--no-dashboard"], + cwd=str(ROOT), env=env, capture_output=True, text=True, timeout=180) + + def test_a_budget_stops_the_run_and_the_run_reports_failure(self): + # Each worker announces $5 of spend. A $6 budget must stop the run, and + # the run must NOT report success -- a budget stop that exits zero would + # pass a CI gate silently, which is how this became a finding. + worker = ('printf \'{"type":"step_finish","part":{"cost":5.0,' + '"tokens":{"input":1000,"output":10}}}\\n\'; sleep 2; :') + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + cfg = self._config(root, worker) + manifest = { + "run_name": "budget-e2e", "workdir": str(root / "work"), + "max_parallel": 1, "worktrees": False, "budget_usd": 6.0, + "tasks": [ + {"key": f"t{i}", "engine": "spender", "spec": "spend", + "check": "echo checking; exit 0", "verified": "n/a"} + for i in range(4) + ], + } + proc = self._run(root, manifest, cfg) + self.assertIn("BUDGET", proc.stdout.upper() + proc.stderr.upper(), + f"no budget stop was reported.\n{proc.stdout[-2000:]}") + self.assertNotEqual(proc.returncode, 0, + "a run stopped by its budget reported success") + self.assertIn("SKIPPED", proc.stdout + proc.stderr, + "queued tasks were not marked SKIPPED") + + def test_identical_failures_stop_the_run_before_the_whole_manifest_is_paid_for(self): + # Every task fails the same way, as an impossible manifest does. With a + # limit of 2 the run must stop long before all six have been bought. + worker = 'printf \'{"type":"step_finish","part":{"cost":0.01}}\\n\'; :' + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + cfg = self._config(root, worker) + manifest = { + "run_name": "abort-e2e", "workdir": str(root / "work"), + "max_parallel": 1, "worktrees": False, + "abort_after_repeated_failures": 2, + "tasks": [ + {"key": f"f{i}", "engine": "spender", "spec": "fail", + "check": "echo IDENTICAL FAILURE; exit 1", + "max_attempts": 1, "verified": "n/a"} + for i in range(6) + ], + } + proc = self._run(root, manifest, cfg) + out = proc.stdout + proc.stderr + self.assertIn("RUN STOPPED", out, f"the breaker never fired.\n{out[-2000:]}") + self.assertIn("SKIPPED", out, "later tasks were not skipped after the stop") + self.assertNotEqual(proc.returncode, 0) + + def test_a_healthy_run_under_budget_still_passes(self): + # The control must not fire on a run that is behaving. Without this the + # two tests above would pass against a runner that always stops. + worker = ('printf \'{"type":"step_finish","part":{"cost":0.001}}\\n\'; ' + 'printf ok > done.txt; :') + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + cfg = self._config(root, worker) + manifest = { + "run_name": "healthy-e2e", "workdir": str(root / "work"), + "max_parallel": 2, "worktrees": False, "budget_usd": 5.0, + "abort_after_repeated_failures": 2, + "tasks": [ + {"key": f"ok{i}", "engine": "spender", "spec": "work", + "check": "test -s done.txt || { echo FAIL: no done.txt; exit 1; }", + "expect_files": ["done.txt"], "verified": "done.txt written"} + for i in range(3) + ], + } + proc = self._run(root, manifest, cfg) + out = proc.stdout + proc.stderr + self.assertNotIn("RUN STOPPED", out, f"a healthy run was stopped.\n{out[-2000:]}") + self.assertEqual(proc.returncode, 0, f"healthy run did not pass.\n{out[-2000:]}") + + +if __name__ == "__main__": + unittest.main() From 14af31d54595b1d9839220bd59ccf991adafdb64 Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 10 Sep 2026 09:16:25 +0200 Subject: [PATCH 11/19] Pass 5: prices were unvalidated, and the source still carried one estate's ledger Three findings fixed, two declined with reasons that are checkable rather than preferences. Configured estimate prices rejected negatives but accepted NaN and infinity, while feeding the same exposure comparison the budget already validates. A NaN price makes every comparison false and stops a run for a nonsensical amount. Now finite-and-non-negative, with the refusals provoked in tests. The shared source still contained this estate's ledger -- exact dollar figures, task counts, `work#666`, `fix-L13`, `dotnet-fix`. The README, sample config and templates had been cleaned two passes ago; ringer.py had not, which is the same half-finished shape as the ceiling wording. The lessons stay, the numbers are now general. An upstream reader should not have to read someone else's invoice to understand a comment. The breaker's stop message now names the tasks it counted. The signature is deliberately coarse, so a stop can be a real repeated failure or a collision between checks that open with the same line -- and the reader could not previously tell which. DECLINED, with evidence rather than preference: - "The signature still conflates unrelated failures." Making it more specific defeats the control. An impossible manifest fails identically across tasks whose checks differ -- that is exactly the case this exists for -- so adding a check-command discriminator would make every task unique and the breaker would never fire. The collision is real, is documented, and now names the tasks so it is diagnosable. The cost of a false trip is a stopped run, not a wrong result. - "Lint rejects a valid deliverable in the assigned temp dir." That path is assigned when the worker starts and carries a random suffix, so it cannot be written into a manifest in advance. The rejected case cannot arise. 285 tests via discovery, 30 in the cost-control file directly. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0125QPzJL8qBQsayptsvuPgB --- README.md | 5 +++-- config.sample.toml | 4 ++-- ringer.py | 42 +++++++++++++++++++++++++------------- tests/test_cost_control.py | 22 ++++++++++++++++++++ 4 files changed, 55 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 938b299a0..1f214cd90 100644 --- a/README.md +++ b/README.md @@ -171,8 +171,9 @@ per task and per run. This is not the same number as `tokens` in the scoreboard. `worker_tokens` records a *single* step, so it cannot be used for money — measured on real work -it ran 19x to 40x below the truth, which is how a $41.71 swarm reported as -roughly $3 and got restarted sixteen times. +it ran 19x to 40x below the truth, which is how a swarm can report a small +fraction of its real cost and be restarted many times by an operator with no +way to see otherwise. Two manifest keys stop a run that is going wrong: diff --git a/config.sample.toml b/config.sample.toml index 9a24b1833..a4e6e8ae7 100644 --- a/config.sample.toml +++ b/config.sample.toml @@ -187,12 +187,12 @@ index_out = "~/.ringer/artifacts/index.html" # ── Cost visibility for engines that bill on another account ────────────── # Codex reports tokens but never a cost, so its work shows as free in every -# provider-reported total. On one estate that hid 797 of 922 tasks -- every +# provider-reported total. On one estate that hid the large majority of tasks -- every # code review among them -- which made review look free when it was the # expensive half. # # token_scale: some harnesses report THOUSANDS of tokens, not units. Codex -# does: measured over 40 real tasks its "tokens used" runs 12-230 where +# does: measured over a sample of real tasks its "tokens used" runs 12-230 where # OpenCode reports hundreds of thousands for comparable work. Without this a # Codex loop reads a thousand times cheaper than it is. # diff --git a/ringer.py b/ringer.py index 03e141236..9ef08eea8 100755 --- a/ringer.py +++ b/ringer.py @@ -747,7 +747,7 @@ class EngineConfig: # agnostic instead of hard-coding one model into the command line. model_default: str = "" # Some harnesses report tokens in thousands rather than units. Codex does: - # measured over 40 real tasks its "tokens used" runs 12-230 where OpenCode + # measured over a sample of real tasks its "tokens used" runs 12-230 where OpenCode # reports hundreds of thousands for comparable work. Summed naively a Codex # loop reads a thousand times cheaper than it is. token_scale: int = 1 @@ -1054,10 +1054,10 @@ def load_artifact_config(raw: Any, state_dir: Path) -> ArtifactConfig: # scoped rather than universal. # # The default is only the canonical vocabulary this project documents. Estates -# that coin their own product task types -- "dotnet-fix", "juce-test", whatever +# that coin their own product task types -- whatever # their stack is called -- extend it in config rather than here: # -# ticketed_task_types = ["code-fix", "code-feature", "dotnet-fix"] +# ticketed_task_types = ["code-fix", "code-feature"] # # Hard-coding one estate's stack into everyone's linter is how a shared tool # stops being shared. @@ -1658,8 +1658,13 @@ def _price(field: str) -> float | None: if raw is None: return None value = float(raw) - if value < 0: - raise ValueError(f"engines.{clean_name}.{field} must not be negative") + # Same reasoning as budget_usd: NaN compares false against + # everything and infinity is unreachable, so either would make the + # exposure it feeds meaningless. + if not math.isfinite(value) or value < 0: + raise ValueError( + f"engines.{clean_name}.{field} must be a finite, non-negative number" + ) return value price_in = _price("price_in_per_mtok") @@ -1703,10 +1708,10 @@ class TaskSpec: full_access: bool = False engine_args: tuple[str, ...] = () verified: str = "" - # The requirement this task serves, e.g. "work#666". Optional -- plenty of + # The requirement this task serves, e.g. a work-item id in whatever form your tracker uses. Optional -- plenty of # runs are bakeoffs and probes that serve no ticket -- but without it a fix # swarm's spend cannot be attributed to anything. Measured on a real estate: - # 175 of 178 product tasks were keyed `fix-L13` and similar, so the question + # almost every product task was keyed by lane number rather than by work item, so the question # "what did this requirement cost?" had no answer at all. ticket: str = "" # Which model a harness engine should run for this task (fills the @@ -8885,6 +8890,7 @@ def __init__( # completion order. A per-signature tally answers the question actually # being asked -- has THIS failure now happened N times. self.failure_counts: dict[str, int] = {} + self.failure_tasks: dict[str, list[str]] = {} async def run(self) -> int: self.manifest.workdir.mkdir(parents=True, exist_ok=True) @@ -8967,7 +8973,7 @@ def run_exposure_usd(self) -> tuple[float, float]: engine that reports no cost of its own still spends real money, and a budget that ignores it is no budget at all for exactly the case it is most needed -- a plan-billed or separately-billed worker, which on one - estate was 797 of 922 tasks. + estate was the large majority of tasks. """ with self.lock: measured = sum(r.cost_usd or 0.0 for r in self.runtimes) @@ -8995,7 +9001,8 @@ def _refresh_cost(self, runtime: TaskRuntime) -> None: # Whether that figure is a running total or a single step is a # property of the harness, not something knowable here -- which # is what `token_scale` exists to correct per engine. Measured on - # Codex: 619 logs, zero step_finish events, and a "tokens used" + # Codex: across every log on one estate, zero step_finish + # events, and a "tokens used" # value whose median (47, i.e. 47k) sits in the same range as # OpenCode's fresh input for comparable work, so it reads as a # task total rather than one step. Confirm before trusting it for @@ -9026,11 +9033,18 @@ async def _note_failure(self, runtime: TaskRuntime, verify: Any) -> None: with self.lock: seen = self.failure_counts.get(signature, 0) + 1 self.failure_counts[signature] = seen + self.failure_tasks.setdefault(signature, []).append(runtime.task.key) tripped = seen >= limit and self.stop_reason is None if tripped: + # Name the tasks. The signature is deliberately coarse, so a stop + # can be a genuine repeated failure or an unlucky collision + # between checks that open with the same line -- and the reader + # can only tell which by seeing which tasks were counted. + which = ", ".join(self.failure_tasks[signature]) self.stop_reason = ( f"{seen} tasks failed the same way ({first_line or 'no output'}) " - f"-- stopping rather than paying for the rest of the manifest" + f"-- {which} -- stopping rather than paying for the rest of the " + f"manifest" ) if tripped: print(f"\n*** RUN STOPPED: {self.stop_reason} ***", flush=True) @@ -9782,7 +9796,7 @@ def estimate_cost_from_tokens(engine: "EngineConfig", tokens: int) -> float | No Codex is the case this exists for: it bills on another account entirely, so its work is invisible in any provider-reported total. On one estate 797 of - 922 tasks -- every code review among them -- carried no cost at all, which + most tasks -- every code review among them -- carried no cost at all, which made review look free when it was the expensive half. Two things make this honest rather than misleading. The engine's @@ -9856,9 +9870,9 @@ def parse_step_costs(log_path: Path) -> tuple[float | None, int]: This exists because `worker_tokens` records ONE step, not the sum -- the token regex reads a single figure out of the tail of the stream. Measured on - real tasks the gap is 19x to 40x, which is how a $41.71 swarm reported as - roughly $3 and was restarted sixteen times by an operator who had no way to - see otherwise. + real tasks the gap is 19x to 40x, which is how a swarm can report a small + fraction of its real cost and be restarted many times by an operator who has + no way to see otherwise. Returns (usd, steps). usd is None when the log carries no cost data at all (a plan-billed or non-JSON engine), which is different from a run that diff --git a/tests/test_cost_control.py b/tests/test_cost_control.py index ddc3bf460..262255fc4 100644 --- a/tests/test_cost_control.py +++ b/tests/test_cost_control.py @@ -376,6 +376,28 @@ def test_a_silent_harness_yields_zero_not_a_guess(self): self.assertEqual(ringer.parse_step_tokens(tmp), 0) +class PriceValidationTests(unittest.TestCase): + """A price is input too, and feeds the same enforcement path as the budget.""" + + def _load(self, value): + cfgdir = Path(tempfile.mkdtemp()) + (cfgdir / "config.toml").write_text( + "[engines.x]\n" + 'bin = "/bin/true"\n' + 'args_template = ["{spec}"]\n' + f"price_in_per_mtok = {value}\n", encoding="utf-8") + return ringer.AppConfig.load(cfgdir / "config.toml") + + def test_non_finite_and_negative_prices_are_refused(self): + for bad in ("nan", "inf", "-1.0"): + with self.assertRaises(ValueError, msg=f"{bad} was accepted as a price"): + self._load(bad) + + def test_a_normal_price_still_loads(self): + cfg = self._load("1.25") + self.assertAlmostEqual(cfg.engines["x"].price_in_per_mtok, 1.25) + + class BudgetValidationTests(unittest.TestCase): def test_nan_and_infinity_are_refused(self): for bad in (float("nan"), float("inf")): From cd9c1260882833183bc5d7cc60716eb5bfd3d380 Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 10 Sep 2026 13:22:18 +0200 Subject: [PATCH 12/19] Prove a task can pass before buying it Every control that existed fired after the spend: a meter, a budget, an abort. This is the half that comes before. Baseline stops being a flag. `run --baseline` was already shipped and had never once been used here, which is what a flag people are supposed to remember is worth. Every `run` now baselines itself first, and refuses the dispatch on the two shapes that make a task unbuyable: a check that is already green (green now, green at the end, so it can never tell you the work happened) and a check that could not be executed at all. A check that FAILS baseline is the wanted result and dispatches normally -- that is the whole point of the phase, and refusing on it would refuse every honest manifest. The canary is a stop, not a smaller batch. A multi-task run releases its first task alone, judges it by its own executed check, and only then releases the rest; a bad verdict marks every remaining task SKIPPED without spawning. Measured both ways on the same night: a 3-ticket run that did this caught a design fault on its first task, and a 32-task run that did not lost a whole round to a fault its first task had already demonstrated. --canary-confirm adds a human on top, deliberately not by default -- a pause met on every run becomes a keypress people learn to hit. Both gates take a REASON to skip, not a bare flag, and print WAIVED (not proved, not verified). A blank reason is rejected rather than quietly re-enabling the gate the operator believed they had turned off. The whole verdict lands in the run record as a `preflight` block, so "was this checked?" is answerable later instead of remembered. Two existing tests now waive baseline explicitly: the budget e2e checks `exit 0` on purpose, and the workdir-escape test needs to reach the runtime guard the gate would otherwise pre-empt. Both keep testing what they are named for, and both refusals are pinned independently in the new file. Every new test was run against the unmodified tree first: all nine fail there. Demo keeps its parallel fan-out -- the canary is skipped for it with the reason recorded, since serialising the first of three workers would hide the thing the demo exists to show. Its baseline passes cleanly (3 fail, 0 pass, 0 error). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KX9rgZWnE5zAmJmhaBz35S --- README.md | 35 ++- ringer.py | 381 ++++++++++++++++++++++++++++++- tests/test_enforcement_e2e.py | 19 +- tests/test_prove_before_buy.py | 403 +++++++++++++++++++++++++++++++++ tests/test_ringer.py | 14 +- 5 files changed, 833 insertions(+), 19 deletions(-) create mode 100644 tests/test_prove_before_buy.py diff --git a/README.md b/README.md index 92ebb95be..55ba55a38 100644 --- a/README.md +++ b/README.md @@ -240,7 +240,7 @@ flagged. ### Baseline: prove your checks before spending tokens -Lint reads the manifest; `--baseline` executes it — every task's `check` runs against the unmodified tree, spawning no workers and writing no eval rows: +Lint reads the manifest; the baseline executes it — every task's `check` runs against the unmodified tree, spawning no workers and writing no eval rows. **Every `run` does this first, automatically.** To see the baseline and dispatch nothing: ```bash ./ringer.py run swarm.json --baseline @@ -248,6 +248,39 @@ Lint reads the manifest; `--baseline` executes it — every task's `check` runs Each check runs in a fresh scratch dir (a detached worktree when the manifest uses worktrees) through the same verifier as a real run. Reading the results: an assertion that demands the NEW behavior workers will build is *expected* to FAIL baseline; an assertion about UNCHANGED behavior that fails baseline is a bug in the check itself, and at run time it would burn a worker's attempts against something no model can satisfy. Fix the check before spawning. +A run is **refused** — before a single worker spawns, exit 2 — when the baseline finds either of the two shapes that make a task unbuyable: + +| baseline says | meaning | dispatch | +|---|---|---| +| **FAIL** | the check demands behavior that does not exist yet | ✅ this is what you want | +| **pass** | the check is already green, so it is green at the end too, and cannot tell you the work happened | ❌ refused | +| **error** | the check could not be executed at all | ❌ refused | + +It was a flag before, and a flag you have to remember is a flag that gets forgotten on the run that most needed it. The measured version of that: one run restarted sixteen times, whose worst restart failed 31 of 36 tasks for $19.43, because the manifest told every worker to write to a path the sandbox forbids. + +### Canary: buy one task before you buy the batch + +A multi-task run releases its **first task alone**, judges it by its own executed check, and only then releases the rest. A bad verdict stops the run, and every remaining task is marked `SKIPPED` without spawning — so a manifest-wide fault costs one task instead of all of them. + +The canary is not a smaller batch; it is a **stop** between the first task and the rest. A single-task run skips it and says so — that task is already the whole exposure. + +```bash +./ringer.py run swarm.json --canary-confirm # also require a human to release the batch +``` + +`--canary-confirm` is deliberately not the default. A pause a human meets on every run becomes a keypress they learn to hit, which buys the appearance of a gate and none of the substance; the executed check is the verdict that always runs. + +### Both gates have an escape, and it announces itself + +```bash +./ringer.py run swarm.json --no-baseline "checks assert unchanged invariants on purpose" +./ringer.py run swarm.json --no-canary "tasks are fully independent, no shared manifest fault possible" +``` + +Each takes a **reason**, not a bare flag. The reason is printed as a `WAIVED (not proved, not verified)` banner and recorded in the run record — a gate nobody can bypass under pressure gets deleted rather than fixed, but a bypass that leaves no trace is the same as no gate. A blank reason is rejected rather than silently re-enabling the gate. + +Every run record carries a `preflight` block with the baseline verdict per task and the canary's state, so *"was this checked?"* is answerable months later by someone who was not there. + ## Make your agent actually use this Between swarms, agents drift back to invisible inline work. Reminders decay, so enforcement ships with the product. diff --git a/ringer.py b/ringer.py index 98539f262..9c7049349 100755 --- a/ringer.py +++ b/ringer.py @@ -2472,6 +2472,37 @@ def count_named_descendants( return count +@dataclass(frozen=True) +class Preflight: + """What was proved before this run was allowed to spend anything. + + It is carried into the run record on purpose. "Was this checked?" has to + be answerable from the record months later, by someone who was not there + — otherwise the honest answer is "somebody probably remembered to", which + is the same answer a run that skipped the gate would give. + """ + + baseline: BaselineResult | None = None + canary_enabled: bool = True + canary_skip_reason: str | None = None + canary_confirm: bool = False + + def to_record(self) -> dict[str, Any]: + baseline = ( + self.baseline.to_record() + if self.baseline is not None + else {"ran": False, "skipped_reason": "not requested"} + ) + return { + "baseline": baseline, + "canary": { + "enabled": self.canary_enabled, + "skipped_reason": self.canary_skip_reason, + "human_confirm": self.canary_confirm, + }, + } + + class StateWriter: def __init__( self, @@ -2486,7 +2517,9 @@ def __init__( max_parallel: int = 1, artifact: ArtifactConfig | None = None, path: Path | None = None, + preflight: Preflight | None = None, ) -> None: + self.preflight = preflight or Preflight() self.run_id = run_id self.run_name = run_name self.identity = identity @@ -2652,6 +2685,7 @@ def snapshot(self) -> dict[str, Any]: "live_path": str(self.live_path) if self.artifact.enabled else None, "report_path": str(self.report_path) if self.artifact.enabled else None, "report_ready": self.report_written, + "preflight": self.preflight.to_record(), } def build_summary(self) -> dict[str, int]: @@ -9009,11 +9043,13 @@ def __init__( identity: str, dashboard_enabled: bool = True, force_browser: bool = False, + preflight: Preflight | None = None, ) -> None: self.manifest = manifest self.config = config self.identity = identity self.dashboard_enabled = dashboard_enabled + self.preflight = preflight or Preflight() self.run_id = build_run_id(manifest.run_name) self.started_at = datetime.now(timezone.utc) self.lock = threading.RLock() @@ -9029,6 +9065,7 @@ def __init__( self.lock, max_parallel=manifest.max_parallel, artifact=config.artifact, + preflight=self.preflight, ) self.dashboard = ( Dashboard( @@ -9069,7 +9106,7 @@ async def run(self) -> int: else None ) try: - await asyncio.gather(*(self._run_task(runtime) for runtime in self.runtimes)) + await self._dispatch() finally: if watcher is not None: watcher.cancel() @@ -9113,6 +9150,110 @@ async def run(self) -> int: print(f"\nYour results: {results_page}") print("Open it in a browser, or run './ringer.py hud' for the full Ringside view (http://127.0.0.1:8700).") + def _split_canary(self) -> tuple[TaskRuntime | None, list[TaskRuntime]]: + """The canary task, and the batch held behind it. + + A run of one task needs no canary: that task IS the whole exposure, + and stopping "the rest" after it would stop nothing. Announcing the + auto-skip matters more than it looks — silence here is + indistinguishable from the gate being off. + """ + if not self.preflight.canary_enabled: + return None, list(self.runtimes) + if len(self.runtimes) < 2: + if self.runtimes: + print( + "Canary: skipped — a single-task run is its own canary, " + "so there is no batch to hold back.", + flush=True, + ) + return None, list(self.runtimes) + return self.runtimes[0], list(self.runtimes[1:]) + + async def _dispatch(self) -> None: + """Release the canary, judge it, then release the rest. + + The batch is not a smaller batch — it is a STOP. Every remaining task + checks `stop_reason` before it spawns, so a bad canary verdict costs + exactly one task's spend instead of the manifest's. + """ + canary, rest = self._split_canary() + if canary is None: + await asyncio.gather(*(self._run_task(runtime) for runtime in rest)) + return + print( + f"\nCanary: running 1 of {len(self.runtimes)} tasks " + f"({canary.task.key}) before releasing the other {len(rest)}.", + flush=True, + ) + await self._run_task(canary) + await self._judge_canary(canary, held_back=len(rest)) + await asyncio.gather(*(self._run_task(runtime) for runtime in rest)) + + async def _judge_canary(self, runtime: TaskRuntime, *, held_back: int) -> None: + """Decide whether the batch is released, and say why if it is not.""" + with self.lock: + if self.stop_reason is not None: + # Something else already stopped the run (budget, a signal). + # Do not overwrite a reason that is already true. + return + status = runtime.status + detail = runtime.setup_error or shorten(runtime.last_check_output, 300) + if status != "pass": + reason = ( + f"canary task {runtime.task.key!r} did not pass its own check, " + f"so the remaining {held_back} task(s) were not dispatched" + ) + with self.lock: + self.stop_reason = reason + print(f"\n*** RUN STOPPED: {reason} ***", flush=True) + if detail: + for line in detail.splitlines()[:6]: + print(f" {line}", flush=True) + print( + "The canary is the cheapest evidence you will get: whatever it hit, " + f"the other {held_back} task(s) would have hit too. Fix the manifest, " + "then re-run.", + flush=True, + ) + return + print(f"Canary: {runtime.task.key} passed — releasing {held_back} task(s).", flush=True) + if self.preflight.canary_confirm: + await self._confirm_canary(runtime, held_back=held_back) + + async def _confirm_canary(self, runtime: TaskRuntime, *, held_back: int) -> None: + """Ask a human before the batch goes out. + + Deliberately NOT the default. A pause a human meets on every run + becomes a keypress they learn to hit, which buys the appearance of a + gate and none of the substance — the executed check above is the + verdict that always runs. + """ + if not sys.stdin.isatty(): + reason = ( + "--canary-confirm asked for a human verdict, but stdin is not a " + "terminal, so nobody can give one" + ) + with self.lock: + if self.stop_reason is None: + self.stop_reason = reason + print(f"\n*** RUN STOPPED: {reason} ***", flush=True) + return + prompt = ( + f"\nCanary {runtime.task.key} passed. Read its output, then release " + f"the remaining {held_back} task(s)? [y/N] " + ) + try: + answer = await asyncio.to_thread(input, prompt) + except (EOFError, KeyboardInterrupt): + answer = "" + if answer.strip().lower() not in {"y", "yes"}: + reason = f"canary {runtime.task.key!r} was not released by the operator" + with self.lock: + if self.stop_reason is None: + self.stop_reason = reason + print(f"\n*** RUN STOPPED: {reason} ***", flush=True) + async def kill_all_workers(self) -> None: procs = list(self.active_processes.values()) for proc in procs: @@ -10507,7 +10648,90 @@ def shorten(value: str, limit: int) -> str: return clean[: max(0, limit - 3)] + "..." -async def run_baseline(manifest: Manifest, *, config: AppConfig) -> int: +@dataclass(frozen=True) +class BaselineTaskResult: + """One task's check, executed against the unmodified tree. + + `outcome` is deliberately three-valued rather than a bool, because the + three cases mean opposite things. "fail" is the WANTED result: the check + demands behavior that does not exist yet, which is what a worker is being + bought to build. "pass" means the check is already satisfied, so passing + it again at the end proves nothing. "error" means the check could not be + executed at all. + """ + + key: str + outcome: str # "fail" (wanted) | "pass" (proves nothing) | "error" (broken) + returncode: int | None = None + timed_out: bool = False + detail: str = "" + + +@dataclass(frozen=True) +class BaselineResult: + """The whole baseline phase, in a form a run record can carry.""" + + tasks: tuple[BaselineTaskResult, ...] = () + leaked_worktrees: tuple[str, ...] = () + skipped_reason: str | None = None + + @property + def ran(self) -> bool: + return self.skipped_reason is None + + def keys_with(self, outcome: str) -> tuple[str, ...]: + return tuple(task.key for task in self.tasks if task.outcome == outcome) + + def objections(self) -> tuple[str, ...]: + """Why this manifest must not be dispatched. Empty means go. + + Only two shapes block, and neither is "a check failed" — a failing + check is the whole point of a baseline. What blocks is a check that + cannot be executed, and a check that is already green. + """ + objections: list[str] = [] + errored = self.keys_with("error") + if errored: + objections.append( + f"{len(errored)} check(s) could not be executed at all " + f"({', '.join(errored)}). A worker cannot be verified by a " + "check that does not run, so its attempts would be bought blind." + ) + already_green = self.keys_with("pass") + if already_green: + objections.append( + f"{len(already_green)} check(s) already pass against the " + f"unmodified tree ({', '.join(already_green)}). A check that is " + "green before any work starts is green after it too, so it " + "cannot tell you whether the work happened." + ) + return tuple(objections) + + def to_record(self) -> dict[str, Any]: + if not self.ran: + return {"ran": False, "skipped_reason": self.skipped_reason} + return { + "ran": True, + "skipped_reason": None, + "total": len(self.tasks), + "fail": len(self.keys_with("fail")), + "pass": len(self.keys_with("pass")), + "error": len(self.keys_with("error")), + "objections": list(self.objections()), + "tasks": [ + { + "key": task.key, + "outcome": task.outcome, + "check_returncode": task.returncode, + "check_timed_out": task.timed_out, + "detail": shorten(task.detail, 2000), + } + for task in self.tasks + ], + } + + +async def execute_baseline(manifest: Manifest) -> BaselineResult: """Execute every task's CHECK against the unmodified tree. Spawn nothing. The point: a check assertion that encodes NEW behavior is *expected* to @@ -10523,14 +10747,12 @@ async def run_baseline(manifest: Manifest, *, config: AppConfig) -> int: scratch taskdir (a detached worktree when the manifest uses worktrees), removed afterwards, so no state leaks between checks or into a later run. """ - del config # engines are irrelevant: baseline spawns no workers verifier = Verifier() worktrees = manifest.worktrees and manifest.repo is not None baseline_root = Path(tempfile.mkdtemp(prefix="ringer-baseline-")) total = len(manifest.tasks) print(f"Baseline: executing {total} check(s) with no workers spawned.") - failures = 0 - errors = 0 + results: list[BaselineTaskResult] = [] leaked_worktrees: list[str] = [] try: for task in manifest.tasks: @@ -10538,8 +10760,9 @@ async def run_baseline(manifest: Manifest, *, config: AppConfig) -> int: # Same containment rule as the real run path: a key must not # escape its scratch root. if not taskdir.is_relative_to(baseline_root.resolve()) or taskdir == baseline_root.resolve(): - errors += 1 - print(f"{task.key:<24} baseline: ERROR (task key escapes the baseline scratch root)") + detail = "task key escapes the baseline scratch root" + results.append(BaselineTaskResult(key=task.key, outcome="error", detail=detail)) + print(f"{task.key:<24} baseline: ERROR ({detail})") continue if worktrees: proc = await asyncio.create_subprocess_exec( @@ -10557,9 +10780,16 @@ async def run_baseline(manifest: Manifest, *, config: AppConfig) -> int: ) stdout, _ = await proc.communicate() if proc.returncode != 0: - errors += 1 - print(f"{task.key:<24} baseline: ERROR (git worktree add failed)") message = stdout.decode("utf-8", errors="replace").strip() + results.append( + BaselineTaskResult( + key=task.key, + outcome="error", + returncode=proc.returncode, + detail=f"git worktree add failed: {message}", + ) + ) + print(f"{task.key:<24} baseline: ERROR (git worktree add failed)") for line in message.splitlines()[:4]: print(f" {line}") continue @@ -10573,9 +10803,17 @@ async def run_baseline(manifest: Manifest, *, config: AppConfig) -> int: f"{task.key:<24} baseline: {status} " f"(rc={verify.check_returncode}{timed_out})" ) + excerpt = verify.raw_output_excerpt.strip() + results.append( + BaselineTaskResult( + key=task.key, + outcome="pass" if verify.ok else "fail", + returncode=verify.check_returncode, + timed_out=verify.check_timed_out, + detail=excerpt, + ) + ) if not verify.ok: - failures += 1 - excerpt = verify.raw_output_excerpt.strip() for line in excerpt.splitlines()[:6]: print(f" {line}") finally: @@ -10602,7 +10840,13 @@ async def run_baseline(manifest: Manifest, *, config: AppConfig) -> int: print(f" {line}") finally: shutil.rmtree(baseline_root, ignore_errors=True) - passed = total - failures - errors + result = BaselineResult( + tasks=tuple(results), + leaked_worktrees=tuple(leaked_worktrees), + ) + failures = len(result.keys_with("fail")) + errors = len(result.keys_with("error")) + passed = len(result.keys_with("pass")) print(f"\nbaseline: {passed} pass, {failures} fail, {errors} error of {total} check(s).") if leaked_worktrees: print( @@ -10616,9 +10860,42 @@ async def run_baseline(manifest: Manifest, *, config: AppConfig) -> int: "behavior means the check itself is broken and will burn worker attempts\n" "against something no model can satisfy — fix the check before spawning." ) + return result + + +async def run_baseline(manifest: Manifest, *, config: AppConfig) -> int: + """`run --baseline`: report the baseline and dispatch nothing.""" + del config # engines are irrelevant: baseline spawns no workers + await execute_baseline(manifest) return 0 +def require_reason(flag: str, value: str | None) -> str | None: + """A waiver flag is only a waiver if it says why. + + The blank case is the one that matters. `--no-baseline ""` must not read + as "no waiver requested" — that would quietly run the gate the operator + believed they had turned off — nor as a waiver with nothing recorded + against it, which is a bypass that leaves no trace. + """ + if value is None: + return None + reason = " ".join(value.split()) + if not reason: + raise ValueError( + f"{flag} requires a reason explaining why the gate is being skipped" + ) + return reason + + +def announce_waiver(gate: str, reason: str, consequence: str) -> None: + """Print a bypass loudly enough that it cannot be mistaken for a pass.""" + print(f"\n*** {gate} WAIVED (not proved, not verified) ***") + print(f" reason: {reason}") + print(f" {consequence}") + print(" The reason above is recorded in the run record.") + + def append_text(path: Path, text: str) -> None: path.parent.mkdir(parents=True, exist_ok=True) with path.open("a", encoding="utf-8") as fh: @@ -11205,6 +11482,7 @@ async def run_manifest( identity: str, dashboard_enabled: bool, force_browser: bool, + preflight: Preflight | None = None, ) -> int: runner = RingerRunner( manifest, @@ -11212,6 +11490,7 @@ async def run_manifest( identity=identity, dashboard_enabled=dashboard_enabled, force_browser=force_browser, + preflight=preflight, ) register_active_run( runner.run_id, @@ -11482,6 +11761,30 @@ def build_parser() -> argparse.ArgumentParser: "baseline are bugs in the check, not work for a model" ), ) + run_parser.add_argument( + "--no-baseline", + metavar="REASON", + help=( + "dispatch without proving the checks first. Takes a REASON, which is " + "printed as a waiver and recorded in the run record" + ), + ) + run_parser.add_argument( + "--no-canary", + metavar="REASON", + help=( + "release the whole batch at once instead of holding it behind the " + "first task. Takes a REASON, which is printed and recorded" + ), + ) + run_parser.add_argument( + "--canary-confirm", + action="store_true", + help=( + "after the canary passes its check, also require a human to release " + "the batch (the executed check is the default verdict)" + ), + ) run_parser.add_argument( "--allow-noncanonical-route", action="store_true", @@ -11778,6 +12081,59 @@ def main(argv: list[str] | None = None) -> int: # Deliberately before preflight_engine_bins: baseline spawns no # workers, so a missing engine binary must not block it. return asyncio.run(run_baseline(manifest, config=config)) + + # Everything below this line costs money, so everything above it has to + # have been proved. Both escapes take a REASON rather than being bare + # flags: a gate nobody can bypass under pressure gets deleted rather + # than fixed, but a bypass that leaves no trace is the same as no gate. + baseline_waiver = require_reason("--no-baseline", getattr(args, "no_baseline", None)) + if baseline_waiver is not None: + announce_waiver( + "BASELINE", + baseline_waiver, + "This run is buying work that nobody proved could succeed.", + ) + baseline_result = BaselineResult(skipped_reason=baseline_waiver) + else: + baseline_result = asyncio.run(execute_baseline(manifest)) + objections = baseline_result.objections() + if objections: + print("\n*** DISPATCH REFUSED: this manifest was not proved ***", file=sys.stderr) + for objection in objections: + print(f" - {objection}", file=sys.stderr) + print( + "\nNo workers were spawned and nothing was spent. Fix the checks, " + 'or dispatch anyway with --no-baseline "".', + file=sys.stderr, + ) + return 2 + + canary_waiver = require_reason("--no-canary", getattr(args, "no_canary", None)) + canary_enabled = True + canary_skip_reason: str | None = None + if canary_waiver is not None: + announce_waiver( + "CANARY", + canary_waiver, + "The whole batch goes out at once; a fault in the first task " + "will be paid for in every other task too.", + ) + canary_enabled = False + canary_skip_reason = canary_waiver + elif args.command == "demo": + # The demo exists to show parallel fan-out on Ringside; holding two + # of its three workers behind the first would hide the thing it + # demonstrates. Recorded in the run record rather than silent — + # silence here is indistinguishable from the gate being off. + canary_enabled = False + canary_skip_reason = "demo manifest: the parallel fan-out IS the demonstration" + + preflight = Preflight( + baseline=baseline_result, + canary_enabled=canary_enabled, + canary_skip_reason=canary_skip_reason, + canary_confirm=bool(getattr(args, "canary_confirm", False)), + ) preflight_engine_bins(manifest, config) if args.command == "run": start_catalog_auto_refresh() @@ -11790,6 +12146,7 @@ def main(argv: list[str] | None = None) -> int: identity=identity, dashboard_enabled=dashboard_enabled, force_browser=args.browser, + preflight=preflight, ) ) except KeyboardInterrupt: diff --git a/tests/test_enforcement_e2e.py b/tests/test_enforcement_e2e.py index 1e62c5fbc..b07ab2334 100644 --- a/tests/test_enforcement_e2e.py +++ b/tests/test_enforcement_e2e.py @@ -52,7 +52,7 @@ def _config(self, root: Path, worker_sh: str) -> Path: ]), encoding="utf-8") return path - def _run(self, root: Path, manifest: dict, config: Path): + def _run(self, root: Path, manifest: dict, config: Path, waive_baseline: str | None = None): manifest_path = root / "manifest.json" manifest_path.write_text(json.dumps(manifest, indent=2), encoding="utf-8") env = os.environ.copy() @@ -60,10 +60,12 @@ def _run(self, root: Path, manifest: dict, config: Path): XDG_CONFIG_HOME=str(root / "xdg"), RINGER_NO_SELF_UPDATE="1") (root / "home").mkdir(exist_ok=True) (root / "rhome").mkdir(exist_ok=True) + cmd = [sys.executable, "ringer.py", "run", str(manifest_path), + "--config", str(config), "--no-dashboard"] + if waive_baseline is not None: + cmd.extend(["--no-baseline", waive_baseline]) return subprocess.run( - [sys.executable, "ringer.py", "run", str(manifest_path), - "--config", str(config), "--no-dashboard"], - cwd=str(ROOT), env=env, capture_output=True, text=True, timeout=180) + cmd, cwd=str(ROOT), env=env, capture_output=True, text=True, timeout=180) def test_a_budget_stops_the_run_and_the_run_reports_failure(self): # Each worker announces $5 of spend. A $6 budget must stop the run, and @@ -83,7 +85,14 @@ def test_a_budget_stops_the_run_and_the_run_reports_failure(self): for i in range(4) ], } - proc = self._run(root, manifest, cfg) + # These four tasks check `exit 0` on purpose -- they exist to burn + # a budget, not to be verified -- so the baseline gate refuses the + # manifest before any of the budget machinery below can run. The + # waiver is the documented escape and keeps this test about + # budgets. That the gate refuses an `exit 0` check at all is + # pinned in tests/test_prove_before_buy.py. + proc = self._run(root, manifest, cfg, + waive_baseline="synthetic budget-burn manifest; checks are not verification") self.assertIn("BUDGET", proc.stdout.upper() + proc.stderr.upper(), f"no budget stop was reported.\n{proc.stdout[-2000:]}") self.assertNotEqual(proc.returncode, 0, diff --git a/tests/test_prove_before_buy.py b/tests/test_prove_before_buy.py new file mode 100644 index 000000000..0e8d889ec --- /dev/null +++ b/tests/test_prove_before_buy.py @@ -0,0 +1,403 @@ +#!/usr/bin/env python3 +"""Nothing is dispatched until something has proved it could pass. + +Measured on 2026-09-10, across one estate's runs: of $49 metered spend, 80% +was a single run restarted sixteen times, and its worst restart failed 31 of +36 tasks for $19.43 -- because the manifest told every worker to write a +deliverable to a path the sandbox forbids. Half of all metered spend went to +tasks that were retried and still failed. None of that needed a better model +or a cheaper one. It needed one question asked before dispatch: can this task +pass at all? + +The controls that already existed -- a meter, a budget, an abort -- all fire +AFTER the spend. These two fire before it. + +Pinned here, each provoked on purpose: + * a dispatch with no baseline runs one first, and refuses on what it finds + * a check that cannot fail is refused (it is green now, so it is green at + the end, so it can never tell you the work happened) + * a check that cannot be executed at all is refused + * a canary whose verdict is bad holds the rest of the batch back + * both escapes exist, demand a REASON, and announce themselves + * the baseline verdict lands in the run record, so "was this checked?" is + answerable later rather than remembered +""" +from __future__ import annotations + +import json +import os +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +MOCK_WORKER = ROOT / "engines" / "mock_worker.py" + + +def toml_string(value: object) -> str: + return json.dumps(str(value)) + + +class ProveBeforeBuyTests(unittest.TestCase): + """Every test here drives the real CLI end to end. A gate nobody has + watched fail is a gate nobody should trust.""" + + def setUp(self) -> None: + self._temp = tempfile.TemporaryDirectory() + self.root = Path(self._temp.name) + self.state_dir = self.root / "state" + self.workdir = self.root / "work" + self.config_path = self.root / "config.toml" + (self.root / "home").mkdir() + (self.root / "rhome").mkdir() + self.config_path.write_text( + "\n".join( + [ + f"state_dir = {toml_string(self.state_dir)}", + "", + "[eval]", + 'backend = "jsonl"', + f"jsonl_path = {toml_string(self.root / 'runs.jsonl')}", + "", + "[artifact]", + "enabled = false", + "", + "[engines.mock]", + f"bin = {toml_string(sys.executable)}", + "args_template = [", + f" {toml_string(MOCK_WORKER)},", + ' "{spec}",', + "]", + "sandbox_args = []", + "full_access_args = []", + "", + ] + ), + encoding="utf-8", + ) + + def tearDown(self) -> None: + self._temp.cleanup() + + # ---- helpers ------------------------------------------------------- + + def write_manifest(self, tasks: list[dict], **overrides: object) -> Path: + manifest = { + "run_name": "prove-before-buy", + "workdir": str(self.workdir), + "max_parallel": 2, + "worktrees": False, + "tasks": tasks, + } + manifest.update(overrides) + path = self.root / "manifest.json" + path.write_text(json.dumps(manifest, indent=2), encoding="utf-8") + return path + + def run_ringer(self, manifest: Path, *extra: str) -> subprocess.CompletedProcess[str]: + env = os.environ.copy() + env.update( + HOME=str(self.root / "home"), + RINGER_HOME=str(self.root / "rhome"), + XDG_CONFIG_HOME=str(self.root / "xdg"), + RINGER_NO_SELF_UPDATE="1", + ) + return subprocess.run( + [ + sys.executable, + "ringer.py", + "run", + str(manifest), + "--config", + str(self.config_path), + "--no-dashboard", + "--identity", + "prove-before-buy-test", + *extra, + ], + cwd=str(ROOT), + env=env, + capture_output=True, + text=True, + timeout=180, + ) + + def writes(self, key: str, filename: str, body: str = "done") -> dict: + """A mock task that really writes its deliverable, so its check passes.""" + return { + "key": key, + "engine": "mock", + "task_type": "probe", + "spec": ( + "You are the deterministic mock worker. Write only the file in " + "the MOCK_FILE block.\n" + f"MOCK_FILE: {filename}\n" + f"{body}\n" + "MOCK_END" + ), + "check": ( + f"test -f {filename} || {{ echo 'FAIL: {filename} was not created'; exit 1; }}" + ), + "expect_files": [filename], + "verified": f"{filename} exists", + } + + def writes_nothing(self, key: str, filename: str) -> dict: + """A mock task whose worker fails, so its check fails honestly.""" + return { + "key": key, + "engine": "mock", + "task_type": "probe", + "spec": ( + "You are the deterministic mock worker. This task simulates a " + "worker failure and leaves the check without its file.\n" + "MOCK_FAIL" + ), + "check": ( + f"test -f {filename} || {{ echo 'FAIL: {filename} was not created'; exit 1; }}" + ), + "expect_files": [filename], + "verified": f"{filename} exists", + "max_attempts": 1, + } + + def read_run_record(self) -> dict: + runs = sorted((self.state_dir / "runs").glob("*.json")) + self.assertTrue(runs, "no run record was written") + return json.loads(runs[-1].read_text(encoding="utf-8")) + + # ---- refusal 1: a check that cannot fail --------------------------- + + def test_a_check_that_cannot_fail_is_refused_before_any_worker_spawns(self) -> None: + manifest = self.write_manifest( + [ + { + "key": "cannot-fail", + "engine": "mock", + "task_type": "probe", + "spec": "MOCK_FILE: irrelevant.txt\nx\nMOCK_END", + # Green now, therefore green at the end, therefore unable + # to distinguish work done from work not done. + "check": "echo checking; exit 0", + "verified": "nothing, and that is the point", + }, + self.writes("honest", "honest.txt"), + ] + ) + + result = self.run_ringer(manifest) + output = result.stdout + result.stderr + + self.assertEqual(2, result.returncode, output) + self.assertIn("DISPATCH REFUSED", output) + self.assertIn("already pass against the unmodified tree", output) + self.assertIn("cannot-fail", output) + # The honest task must NOT be named as an objection: it fails baseline, + # which is the wanted result. + objection_lines = [line for line in output.splitlines() if line.strip().startswith("- ")] + self.assertTrue(objection_lines, output) + self.assertNotIn("honest", " ".join(objection_lines)) + # Nothing was bought. + self.assertFalse( + (self.state_dir / "runs").exists() and list((self.state_dir / "runs").glob("*.json")), + "a refused dispatch must not create a run record", + ) + + # ---- refusal 2: a check that cannot be executed -------------------- + + def test_a_check_that_cannot_be_executed_is_refused(self) -> None: + # A key that escapes its scratch root cannot be given a taskdir, so + # its check never runs -- the shape of "a deliverable no sandboxed + # worker can write", caught before the workers are paid for. + manifest = self.write_manifest( + [ + { + "key": "../escape", + "engine": "mock", + "task_type": "probe", + "spec": "MOCK_FILE: out.txt\nx\nMOCK_END", + "check": "test -f out.txt || { echo 'FAIL: missing'; exit 1; }", + "expect_files": ["out.txt"], + "verified": "out.txt exists", + } + ] + ) + + result = self.run_ringer(manifest) + output = result.stdout + result.stderr + + self.assertEqual(2, result.returncode, output) + self.assertIn("DISPATCH REFUSED", output) + self.assertIn("could not be executed at all", output) + + # ---- refusal 3: a bad canary holds the batch ----------------------- + + def test_a_bad_canary_verdict_holds_the_rest_of_the_batch(self) -> None: + # The first task's worker fails. The other three must never spawn -- + # that is the whole saving, and it is why the canary is a STOP and not + # merely a smaller batch. + manifest = self.write_manifest( + [ + self.writes_nothing("canary", "never.txt"), + self.writes("second", "second.txt"), + self.writes("third", "third.txt"), + self.writes("fourth", "fourth.txt"), + ] + ) + + result = self.run_ringer(manifest) + output = result.stdout + result.stderr + + self.assertNotEqual(0, result.returncode, output) + self.assertIn("Canary: running 1 of 4 tasks", output) + self.assertIn("RUN STOPPED", output) + self.assertIn("did not pass its own check", output) + + record = self.read_run_record() + by_key = {task["key"]: task for task in record["tasks"]} + self.assertEqual("fail", by_key["canary"]["status"], record) + for held in ("second", "third", "fourth"): + self.assertEqual( + "SKIPPED", + by_key[held]["verdict"], + f"{held} was dispatched despite a failed canary", + ) + # The deliverables of the held tasks must not exist: nothing ran. + for held_file in ("second.txt", "third.txt", "fourth.txt"): + self.assertFalse( + list(self.workdir.rglob(held_file)), + f"{held_file} exists, so a held task was actually dispatched", + ) + + def test_a_good_canary_releases_the_batch(self) -> None: + # The complement, and the more important of the pair: a gate that + # blocks everything is not a gate, it is an outage. + manifest = self.write_manifest( + [ + self.writes("canary", "canary.txt"), + self.writes("second", "second.txt"), + self.writes("third", "third.txt"), + ] + ) + + result = self.run_ringer(manifest) + output = result.stdout + result.stderr + + self.assertEqual(0, result.returncode, output) + self.assertIn("Canary: canary passed — releasing 2 task(s).", output) + record = self.read_run_record() + for task in record["tasks"]: + self.assertEqual("pass", task["status"], f"{task['key']} did not pass") + + def test_a_single_task_run_skips_the_canary_and_says_so(self) -> None: + # A run of one task IS its own canary; holding "the rest" back would + # hold nothing. Announced, because silence here is indistinguishable + # from the gate being off. + manifest = self.write_manifest([self.writes("only", "only.txt")]) + + result = self.run_ringer(manifest) + output = result.stdout + result.stderr + + self.assertEqual(0, result.returncode, output) + self.assertIn("Canary: skipped — a single-task run is its own canary", output) + + # ---- the escapes --------------------------------------------------- + + def test_the_baseline_waiver_announces_itself_and_is_recorded(self) -> None: + manifest = self.write_manifest( + [ + self.writes("first", "first.txt"), + self.writes("second", "second.txt"), + ] + ) + + result = self.run_ringer( + manifest, "--no-baseline", "pinning the waiver path in a test" + ) + output = result.stdout + result.stderr + + self.assertEqual(0, result.returncode, output) + self.assertIn("BASELINE WAIVED (not proved, not verified)", output) + self.assertIn("pinning the waiver path in a test", output) + self.assertNotIn("Baseline: executing", output) + + record = self.read_run_record() + baseline = record["preflight"]["baseline"] + self.assertFalse(baseline["ran"]) + self.assertEqual("pinning the waiver path in a test", baseline["skipped_reason"]) + + def test_the_canary_waiver_announces_itself_and_is_recorded(self) -> None: + manifest = self.write_manifest( + [ + self.writes_nothing("would-be-canary", "never.txt"), + self.writes("second", "second.txt"), + ] + ) + + result = self.run_ringer(manifest, "--no-canary", "tasks are independent") + output = result.stdout + result.stderr + + self.assertIn("CANARY WAIVED (not proved, not verified)", output) + self.assertIn("tasks are independent", output) + self.assertNotIn("Canary: running 1 of", output) + + # With the canary waived the second task really is dispatched, even + # though the first failed -- which is exactly what the waiver buys, + # and exactly what it costs. + record = self.read_run_record() + by_key = {task["key"]: task for task in record["tasks"]} + self.assertEqual("pass", by_key["second"]["status"], record) + self.assertFalse(record["preflight"]["canary"]["enabled"]) + self.assertEqual( + "tasks are independent", record["preflight"]["canary"]["skipped_reason"] + ) + + def test_a_waiver_without_a_reason_is_rejected(self) -> None: + # A bypass that leaves no trace is the same as no gate. An empty + # reason must not quietly re-enable the gate either -- the operator + # believes they turned it off, so say so instead. + manifest = self.write_manifest([self.writes("only", "only.txt")]) + + result = self.run_ringer(manifest, "--no-baseline", " ") + output = result.stdout + result.stderr + + self.assertNotEqual(0, result.returncode, output) + self.assertIn("--no-baseline requires a reason", output) + + # ---- the record ---------------------------------------------------- + + def test_the_baseline_verdict_lands_in_the_run_record(self) -> None: + manifest = self.write_manifest( + [ + self.writes("first", "first.txt"), + self.writes("second", "second.txt"), + ] + ) + + result = self.run_ringer(manifest) + self.assertEqual(0, result.returncode, result.stdout + result.stderr) + + record = self.read_run_record() + baseline = record["preflight"]["baseline"] + self.assertTrue(baseline["ran"]) + self.assertEqual(2, baseline["total"]) + # Both checks demand a file no worker has written yet: both must fail + # baseline, and neither is an objection. + self.assertEqual(2, baseline["fail"]) + self.assertEqual(0, baseline["pass"]) + self.assertEqual(0, baseline["error"]) + self.assertEqual([], baseline["objections"]) + self.assertEqual( + {"first", "second"}, {task["key"] for task in baseline["tasks"]} + ) + + canary = record["preflight"]["canary"] + self.assertTrue(canary["enabled"]) + self.assertIsNone(canary["skipped_reason"]) + self.assertFalse(canary["human_confirm"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_ringer.py b/tests/test_ringer.py index e36b7b517..e94274077 100644 --- a/tests/test_ringer.py +++ b/tests/test_ringer.py @@ -92,6 +92,7 @@ def run_ringer( config_path: Path | None = None, no_dashboard: bool = True, timeout: int = 30, + waive_baseline: str | None = None, ) -> subprocess.CompletedProcess[str]: cmd = [ sys.executable, @@ -106,6 +107,11 @@ def run_ringer( ] if no_dashboard: cmd.append("--no-dashboard") + if waive_baseline is not None: + # Deliberately per-call, not suite-wide: every other test here + # keeps the baseline gate ON, which is what proves the gate does + # not obstruct a well-formed manifest. + cmd.extend(["--no-baseline", waive_baseline]) env = os.environ.copy() env["PYTHONDONTWRITEBYTECODE"] = "1" env["RINGER_NO_SELF_UPDATE"] = "1" @@ -564,7 +570,13 @@ def test_task_key_cannot_escape_workdir(self) -> None: ), ) - result = self.run_ringer(manifest) + # The baseline gate refuses this manifest earlier and for its own + # reason (the key escapes the baseline scratch root too), which would + # leave the RUNTIME workdir guard below unexercised. Waiving baseline + # is what keeps this test pointed at the guard it is named for; the + # gate's own refusal of the same key is pinned separately in + # tests/test_prove_before_buy.py. + result = self.run_ringer(manifest, waive_baseline="testing the runtime workdir guard") self.assertEqual(result.returncode, 2, result.stdout) self.assertIn("task key escapes workdir", result.stdout) From 1a5766e063fe0a1b0e88c084f1a78bd1763227d9 Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 10 Sep 2026 13:31:41 +0200 Subject: [PATCH 13/19] Review pass 1: record the canary's verdict, and make the unreachable deliverable a real refusal Two findings from the lens review, both about this change. The record answered "was a canary configured?" but not "was it judged, and what did it say?" -- and only the second question tells you whether the batch was released on evidence. The verdict now lands in the run record at all four points where the canary is actually decided: released, held, waived, and the single-task auto-skip. It lives on the state writer rather than in Preflight, which stays frozen and describes only what was decided BEFORE dispatch. The unreachable deliverable had no demonstrated failure case. The test provoked the `error` outcome through an escaping task key, which is a different fault -- the ticket's named case is a deliverable no sandboxed worker can write, and nothing detected that at all. Baseline now probes declared absolute `expect_files` against the nearest existing ancestor and refuses when nothing could be created there. Only absolute paths are probed, on purpose: a relative deliverable lands in the scratch dir the harness makes and is always writable, so probing those would refuse nearly every honest manifest. That complement is pinned too. The third finding, a failure-counter docstring contradicting its implementation, is declined as out of scope: `failure_counts` is untouched by this change and belongs to the cost-control work. It arrived because the review kit staged the wrong diff -- see the PR thread. All three new assertions were run against the previous commit first and fail there. Suite: 327 pass. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KX9rgZWnE5zAmJmhaBz35S --- ringer.py | 90 +++++++++++++++++++++++++++++++++- tests/test_prove_before_buy.py | 78 +++++++++++++++++++++++++++++ 2 files changed, 167 insertions(+), 1 deletion(-) diff --git a/ringer.py b/ringer.py index 9c7049349..7cea0d409 100755 --- a/ringer.py +++ b/ringer.py @@ -2520,6 +2520,10 @@ def __init__( preflight: Preflight | None = None, ) -> None: self.preflight = preflight or Preflight() + # Set during the run, not before it: whether the canary was judged and + # what the judgment was. Preflight itself stays frozen and describes + # only what was decided BEFORE dispatch. + self.canary_verdict: dict[str, Any] | None = None self.run_id = run_id self.run_name = run_name self.identity = identity @@ -2685,9 +2689,20 @@ def snapshot(self) -> dict[str, Any]: "live_path": str(self.live_path) if self.artifact.enabled else None, "report_path": str(self.report_path) if self.artifact.enabled else None, "report_ready": self.report_written, - "preflight": self.preflight.to_record(), + "preflight": self._preflight_record(), } + def _preflight_record(self) -> dict[str, Any]: + """The pre-dispatch decisions, plus how the canary actually turned out. + + Without the verdict the record answers "was a canary configured?" and + not "was it judged, and what did it say?" -- and only the second + question tells you whether the batch was released on evidence. + """ + record = self.preflight.to_record() + record["canary"]["verdict"] = self.canary_verdict + return record + def build_summary(self) -> dict[str, int]: with self.lock: return { @@ -9159,6 +9174,11 @@ def _split_canary(self) -> tuple[TaskRuntime | None, list[TaskRuntime]]: indistinguishable from the gate being off. """ if not self.preflight.canary_enabled: + self.state_writer.canary_verdict = { + "task": None, + "outcome": "waived", + "reason": self.preflight.canary_skip_reason, + } return None, list(self.runtimes) if len(self.runtimes) < 2: if self.runtimes: @@ -9167,6 +9187,11 @@ def _split_canary(self) -> tuple[TaskRuntime | None, list[TaskRuntime]]: "so there is no batch to hold back.", flush=True, ) + self.state_writer.canary_verdict = { + "task": None, + "outcome": "skipped", + "reason": "single-task run is its own canary", + } return None, list(self.runtimes) return self.runtimes[0], list(self.runtimes[1:]) @@ -9206,6 +9231,12 @@ async def _judge_canary(self, runtime: TaskRuntime, *, held_back: int) -> None: ) with self.lock: self.stop_reason = reason + self.state_writer.canary_verdict = { + "task": runtime.task.key, + "outcome": "held", + "reason": reason, + "held_back": held_back, + } print(f"\n*** RUN STOPPED: {reason} ***", flush=True) if detail: for line in detail.splitlines()[:6]: @@ -9218,6 +9249,12 @@ async def _judge_canary(self, runtime: TaskRuntime, *, held_back: int) -> None: ) return print(f"Canary: {runtime.task.key} passed — releasing {held_back} task(s).", flush=True) + self.state_writer.canary_verdict = { + "task": runtime.task.key, + "outcome": "released", + "reason": "canary passed its own executed check", + "held_back": held_back, + } if self.preflight.canary_confirm: await self._confirm_canary(runtime, held_back=held_back) @@ -9237,6 +9274,12 @@ async def _confirm_canary(self, runtime: TaskRuntime, *, held_back: int) -> None with self.lock: if self.stop_reason is None: self.stop_reason = reason + self.state_writer.canary_verdict = { + "task": runtime.task.key, + "outcome": "held", + "reason": reason, + "held_back": held_back, + } print(f"\n*** RUN STOPPED: {reason} ***", flush=True) return prompt = ( @@ -9252,6 +9295,12 @@ async def _confirm_canary(self, runtime: TaskRuntime, *, held_back: int) -> None with self.lock: if self.stop_reason is None: self.stop_reason = reason + self.state_writer.canary_verdict = { + "task": runtime.task.key, + "outcome": "held", + "reason": reason, + "held_back": held_back, + } print(f"\n*** RUN STOPPED: {reason} ***", flush=True) async def kill_all_workers(self) -> None: @@ -10731,6 +10780,38 @@ def to_record(self) -> dict[str, Any]: } +def unwritable_deliverables(task: TaskSpec) -> list[str]: + """Declared deliverables no worker will be able to write. + + Only ABSOLUTE paths are checked, and that is the whole point: a relative + `expect_files` entry lands inside the task's own scratch dir, which the + harness creates and which is therefore always writable. An absolute one + escapes to somewhere nobody has verified — the fix-swarm patch export is + the legitimate version of this, and "every worker writes its deliverable + to a path the sandbox forbids" is the expensive one. That was one run, + restarted sixteen times, 31 of 36 tasks failing identically, $19.43 on + the worst restart alone. + + A directory that does not exist is not automatically a fault -- the check + may create it -- so the nearest EXISTING ancestor is what gets probed. If + that ancestor is not a writable directory, nothing below it can be + created and every attempt is bought for nothing. + """ + problems: list[str] = [] + for declared in task.expect_files: + path = Path(declared) + if not path.is_absolute(): + continue + ancestor = path.parent + while not ancestor.exists() and ancestor != ancestor.parent: + ancestor = ancestor.parent + if not ancestor.is_dir(): + problems.append(f"{declared} (its parent {ancestor} is not a directory)") + elif not os.access(ancestor, os.W_OK): + problems.append(f"{declared} (no write permission on {ancestor})") + return problems + + async def execute_baseline(manifest: Manifest) -> BaselineResult: """Execute every task's CHECK against the unmodified tree. Spawn nothing. @@ -10764,6 +10845,13 @@ async def execute_baseline(manifest: Manifest) -> BaselineResult: results.append(BaselineTaskResult(key=task.key, outcome="error", detail=detail)) print(f"{task.key:<24} baseline: ERROR ({detail})") continue + unwritable = unwritable_deliverables(task) + if unwritable: + # No point running the check: its subject cannot be produced. + detail = "declared deliverable is unwritable: " + "; ".join(unwritable) + results.append(BaselineTaskResult(key=task.key, outcome="error", detail=detail)) + print(f"{task.key:<24} baseline: ERROR ({detail})") + continue if worktrees: proc = await asyncio.create_subprocess_exec( "git", diff --git a/tests/test_prove_before_buy.py b/tests/test_prove_before_buy.py index 0e8d889ec..95d6c6774 100644 --- a/tests/test_prove_before_buy.py +++ b/tests/test_prove_before_buy.py @@ -232,6 +232,62 @@ def test_a_check_that_cannot_be_executed_is_refused(self) -> None: self.assertIn("DISPATCH REFUSED", output) self.assertIn("could not be executed at all", output) + def test_a_deliverable_no_worker_could_write_is_refused(self) -> None: + # The literal measured incident: a manifest that told every worker to + # write its deliverable to a path nothing can create. Previously that + # was invisible until after payment -- 31 of 36 tasks failing + # identically, $19.43 on the worst restart alone. + # + # The parent is a regular FILE, so creating anything beneath it fails + # with ENOTDIR on every platform. A read-only directory would not do: + # CI often runs as root, where mode bits are ignored and the test + # would silently stop proving anything. + blocker = self.root / "blocker" + blocker.write_text("I am a file, not a directory\n", encoding="utf-8") + unreachable = blocker / "out.txt" + + manifest = self.write_manifest( + [ + { + "key": "unreachable-deliverable", + "engine": "mock", + "task_type": "probe", + "spec": f"MOCK_FILE: {unreachable}\nx\nMOCK_END", + "check": f"test -f {unreachable} || {{ echo 'FAIL: missing'; exit 1; }}", + "expect_files": [str(unreachable)], + "verified": "the deliverable exists", + }, + self.writes("honest", "honest.txt"), + ] + ) + + result = self.run_ringer(manifest) + output = result.stdout + result.stderr + + self.assertEqual(2, result.returncode, output) + self.assertIn("DISPATCH REFUSED", output) + self.assertIn("could not be executed at all", output) + self.assertIn("unreachable-deliverable", output) + self.assertIn("is not a directory", output) + + def test_a_relative_deliverable_is_never_called_unwritable(self) -> None: + # The complement that keeps the check honest: a relative deliverable + # lands in the task's own scratch dir, which the harness creates. If + # this ever started reading as unwritable, the gate would refuse + # essentially every well-formed manifest. + manifest = self.write_manifest( + [ + self.writes("first", "nested/dir/first.txt"), + self.writes("second", "second.txt"), + ] + ) + + result = self.run_ringer(manifest, "--no-canary", "exercising baseline only") + output = result.stdout + result.stderr + + self.assertNotIn("DISPATCH REFUSED", output) + self.assertNotIn("unwritable", output) + # ---- refusal 3: a bad canary holds the batch ----------------------- def test_a_bad_canary_verdict_holds_the_rest_of_the_batch(self) -> None: @@ -397,6 +453,28 @@ def test_the_baseline_verdict_lands_in_the_run_record(self) -> None: self.assertTrue(canary["enabled"]) self.assertIsNone(canary["skipped_reason"]) self.assertFalse(canary["human_confirm"]) + # "Was a canary configured?" and "was it judged, and what did it say?" + # are different questions, and only the second one says whether the + # batch was released on evidence. + self.assertEqual("released", canary["verdict"]["outcome"]) + self.assertEqual("first", canary["verdict"]["task"]) + self.assertEqual(1, canary["verdict"]["held_back"]) + + def test_a_held_batch_records_why_it_was_held(self) -> None: + manifest = self.write_manifest( + [ + self.writes_nothing("canary", "never.txt"), + self.writes("second", "second.txt"), + ] + ) + + result = self.run_ringer(manifest) + self.assertNotEqual(0, result.returncode, result.stdout + result.stderr) + + verdict = self.read_run_record()["preflight"]["canary"]["verdict"] + self.assertEqual("held", verdict["outcome"]) + self.assertEqual("canary", verdict["task"]) + self.assertIn("did not pass its own check", verdict["reason"]) if __name__ == "__main__": From 9c9c39e8ddfdb56717273f5e8fa800825aceaf0e Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 10 Sep 2026 13:49:57 +0200 Subject: [PATCH 14/19] Review pass 2: say what the deliverable probe actually proves The lens is right that `unwritable_deliverables` cannot establish that a WORKER can write a path: it probes as the dispatcher, and a sandbox can deny what `os.access` here calls writable. Widening the code is not available -- proving it needs a spawned worker, and spawning nothing is exactly what makes baseline free enough to run before every dispatch. So the claim is narrowed to what the code does. Naming the layers while correcting it, because the boundary is the useful part: lint's `worker_unwritable_paths` reasons about sandbox SCOPE and is the one that matches the measured incident; baseline catches what no process could write at all; the canary buys whatever neither could know statically, once rather than once per task. The docstring, the error text and the README now each say which of the three they are. Suite: 327 pass, unchanged -- this narrows claims, not behavior. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KX9rgZWnE5zAmJmhaBz35S --- README.md | 2 ++ ringer.py | 48 +++++++++++++++++++++++++++++++++++++----------- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 55ba55a38..088532643 100644 --- a/README.md +++ b/README.md @@ -258,6 +258,8 @@ A run is **refused** — before a single worker spawns, exit 2 — when the base It was a flag before, and a flag you have to remember is a flag that gets forgotten on the run that most needed it. The measured version of that: one run restarted sixteen times, whose worst restart failed 31 of 36 tasks for $19.43, because the manifest told every worker to write to a path the sandbox forbids. +⚠️ **Baseline does not prove a worker can write your deliverables**, and no phase that spawns nothing could. It probes declared *absolute* `expect_files` as the dispatcher sees them, so it catches paths the filesystem itself refuses; a sandbox can still deny a path that looks writable from here. Three layers divide that work: **lint** reasons about sandbox *scope* (a spec handing the worker an absolute path outside its own task directory — the shape of the $19.43 incident), **baseline** catches what no process could write at all, and the **canary** buys the rest once instead of once per task. + ### Canary: buy one task before you buy the batch A multi-task run releases its **first task alone**, judges it by its own executed check, and only then releases the rest. A bad verdict stops the run, and every remaining task is marked `SKIPPED` without spawning — so a manifest-wide fault costs one task instead of all of them. diff --git a/ringer.py b/ringer.py index 7cea0d409..2b63da3ab 100755 --- a/ringer.py +++ b/ringer.py @@ -10781,16 +10781,40 @@ def to_record(self) -> dict[str, Any]: def unwritable_deliverables(task: TaskSpec) -> list[str]: - """Declared deliverables no worker will be able to write. - - Only ABSOLUTE paths are checked, and that is the whole point: a relative - `expect_files` entry lands inside the task's own scratch dir, which the - harness creates and which is therefore always writable. An absolute one - escapes to somewhere nobody has verified — the fix-swarm patch export is - the legitimate version of this, and "every worker writes its deliverable - to a path the sandbox forbids" is the expensive one. That was one run, - restarted sixteen times, 31 of 36 tasks failing identically, $19.43 on - the worst restart alone. + """Declared deliverables the FILESYSTEM will refuse, seen from here. + + ⚠️ This is not a sandbox check and cannot be one. It probes the path as + the dispatcher sees it; a worker runs under the engine's sandbox, which + can deny a path that `os.access` here calls writable. So a clean result + means "no filesystem-level reason this cannot be written", never "the + worker will be able to write it". + + Proving the latter requires running a worker, which is the one thing this + phase must not do -- spawning nothing is what makes it free and what lets + it run before every dispatch. + + Three layers cover the ground between them, and it is worth knowing which + is which: + + lint `worker_unwritable_paths` -- the SPEC hands the worker an + absolute path outside its own task directory. Sandbox SCOPE, + reasoned about statically. This is the shape of the measured + incident: one run restarted sixteen times, 31 of 36 tasks + failing identically, $19.43 on the worst restart alone. + baseline here -- the path is refused by the filesystem itself, so no + process could write it whoever asked. + canary whatever neither of those could know statically, bought once + instead of once per task. + + So this function is the narrowest of the three, and the least clever. It + exists because it is also the only one of them that is certain. + + What this does catch is the cheaper, dumber half: a path that no process + could write, whoever asked. Only ABSOLUTE paths are examined, and that is + deliberate -- a relative `expect_files` entry lands inside the task's own + scratch dir, which the harness creates, so probing those would refuse + nearly every honest manifest. An absolute one escapes to somewhere nobody + has verified; the fix-swarm patch export is the legitimate version of it. A directory that does not exist is not automatically a fault -- the check may create it -- so the nearest EXISTING ancestor is what gets probed. If @@ -10848,7 +10872,9 @@ async def execute_baseline(manifest: Manifest) -> BaselineResult: unwritable = unwritable_deliverables(task) if unwritable: # No point running the check: its subject cannot be produced. - detail = "declared deliverable is unwritable: " + "; ".join(unwritable) + detail = ( + "the filesystem refuses a declared deliverable: " + "; ".join(unwritable) + ) results.append(BaselineTaskResult(key=task.key, outcome="error", detail=detail)) print(f"{task.key:<24} baseline: ERROR ({detail})") continue From 4004647eca92699eb9a3527d4bae2f3494b7b3a6 Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 10 Sep 2026 13:53:22 +0200 Subject: [PATCH 15/19] Review pass 3: baseline spawns no WORKERS, which is not the same as spawning nothing The docstring promised "Spawn nothing" and the function does spawn things -- every check is a subprocess, and the worktree path launches git helpers. The guarantee that actually matters, and the one the refusal depends on, is that no worker starts: no model, no billable token. Claiming more invites a maintainer to assume there are no side effects at all, when a check can legitimately export files, which the fix-swarm pattern relies on. Inherited wording, but this function is rewritten here, so it is corrected here. Suite: 327 pass, unchanged. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KX9rgZWnE5zAmJmhaBz35S --- ringer.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/ringer.py b/ringer.py index 2b63da3ab..2612bb771 100755 --- a/ringer.py +++ b/ringer.py @@ -10837,7 +10837,15 @@ def unwritable_deliverables(task: TaskSpec) -> list[str]: async def execute_baseline(manifest: Manifest) -> BaselineResult: - """Execute every task's CHECK against the unmodified tree. Spawn nothing. + """Execute every task's CHECK against the unmodified tree. Spawn no WORKERS. + + ⚠️ Not "spawn nothing", which is what this said before and is not true: + the checks themselves are subprocesses, and the worktree path launches + `git` helpers. What the phase guarantees is that no worker -- no model, + no billable token -- is started. That is the guarantee worth having and + the one the refusal below depends on; stating a broader one invites a + maintainer to assume there are no side effects at all, when a check can + legitimately export files. The point: a check assertion that encodes NEW behavior is *expected* to fail here, but an assertion that encodes UNCHANGED behavior and fails From a4247adb3f75febeaaae2d27587ee1fc57837bec Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 10 Sep 2026 13:57:21 +0200 Subject: [PATCH 16/19] Review pass 4: delete the demo's unopted canary bypass The demo disabled the canary by command name. Nobody asked for it, and unlike a real waiver it printed nothing -- it was recorded in the run record and silent on the terminal, which is the difference between a decision and a default nobody can see. A gate with a third path that the tool takes on your behalf is the shape this whole change exists to remove. The justification was that the demo shows parallel fan-out and the canary serialises the first of its three workers. That is true and it is not worth a special case: a demo of a path real runs never take teaches the wrong behaviour, and the canary is now what a real run does. The demo still passes end to end -- its baseline is clean (3 fail, 0 pass, 0 error) and its first task writes the file its own check demands. If the three-at-once visual is wanted back it is one flag away, and that flag announces itself. Which is the design. Suite: 327 pass, unchanged. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KX9rgZWnE5zAmJmhaBz35S --- ringer.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/ringer.py b/ringer.py index 2612bb771..a67ad5646 100755 --- a/ringer.py +++ b/ringer.py @@ -12242,13 +12242,6 @@ def main(argv: list[str] | None = None) -> int: ) canary_enabled = False canary_skip_reason = canary_waiver - elif args.command == "demo": - # The demo exists to show parallel fan-out on Ringside; holding two - # of its three workers behind the first would hide the thing it - # demonstrates. Recorded in the run record rather than silent — - # silence here is indistinguishable from the gate being off. - canary_enabled = False - canary_skip_reason = "demo manifest: the parallel fan-out IS the demonstration" preflight = Preflight( baseline=baseline_result, From a0b48f196d7758115cb86562b9a258e2f10eaa5f Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 10 Sep 2026 19:45:59 +0200 Subject: [PATCH 17/19] Record what nexo Run 1 taught, including the row that nearly cost a batch This entry was written on 2026-09-10 and deliberately not committed: the file had uncommitted changes from a concurrent factory run at the time. That run has since landed, so it goes in now. The model entry is the small half. The two process lessons are the ones worth having: An aggregate scoreboard row cannot separate "the model cannot do this" from "the harness was broken". glm-5.2's code-fix row read 0.17 first-try over 63 tasks and was used to justify routing a 24-ticket batch elsewhere -- but all 63 came from one sibling run whose dominant failure was a single block of 64 identical missing_expect_files. That is a harness signature. The same model then went 3/3 on a different factory's pilot. Group failures by run_id and read the failure mode before routing on the number. And: deliverables land in the worktree, the CHECK exports them. A scout task declared its deliverable at an absolute path outside the sandbox; the log shows the model finding the right answer in four tool calls and then burning ~40 on write, cat >, dd, cp, python, xattr -c and touch against a path it was never allowed to touch. The sibling fix-swarm tasks were immune only because their check exported from the check side. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KX9rgZWnE5zAmJmhaBz35S --- docs/MODEL-NOTES.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/docs/MODEL-NOTES.md b/docs/MODEL-NOTES.md index 876136bdb..090bc9af9 100644 --- a/docs/MODEL-NOTES.md +++ b/docs/MODEL-NOTES.md @@ -147,6 +147,14 @@ checks and raw logs support — no vibes, no worker self-reports. contention findings (full catalog re-ingest per sync; schema writes on read paths) plus an empirical XSS all-clear on the new DOM surfaces. Third proven-tier structured review today. +- 2026-09-10 — code-fix, nexo Run 1 pilot (run `nexo-run1-fix-swarm`, 3 tasks): + **3/3 pass, 2 first-attempt.** work#666 (bash + a race harness), work#919 (a + 7-file instruction sweep), work#950 (pre-flight + new selftest cases, attempt 2). + Quality sat above the checks' floor rather than on it: work#666 identified that + the real fix was reading the head sha ONCE and reusing it — atomicity — not + merely deriving the diff locally, and work#919 respected its ownership boundary + by leaving CLAUDE.md untouched while flagging that it needed a follow-up. All + three patches later merged unchanged. ## kimi-k2.7 via opencode (`openrouter/moonshotai/kimi-k2.7-code`) @@ -280,6 +288,23 @@ checks and raw logs support — no vibes, no worker self-reports. before blaming the model. - 2026-07-06 — opencode sqlite "database is locked" again with just 2 simultaneous opencode spawns (page-news + page-about-faq); retry absorbed it. +- 2026-09-10 — **an aggregate scoreboard row cannot tell "the model cannot do + this" from "the harness was broken", and reading it as capability nearly cost a + 24-ticket batch.** glm-5.2's code-fix row read 0.17 first-try / 0.22 pass over 63 + tasks. All 63 came from ONE sibling factory's run whose dominant failure was + `worker_returncode=1` with `missing_expect_files` — a single block of 64 of 69 + identical. That is a harness signature. The same model then went 3/3 on a + different factory's pilot; the runs differed in check design and deliverable + path, not in engine. **Group failures by run_id and read the failure mode before + routing on the number.** +- 2026-09-10 — **deliverables land in the worktree; the CHECK exports them.** A + scout task declared its deliverable at an absolute path outside the worker's + sandbox. The log shows the model finding the correct answer in four tool calls + and then burning ~40 on `write`, `cat >`, `dd`, `cp`, python, `xattr -c` and + `touch` against a path it was never allowed to touch. Sibling fix-swarm tasks + were immune purely because their check exported the patch from the check side. + A worker can write inside its own worktree and its assigned TMPDIR, nowhere + else — and it will spend a whole task's budget proving that to you. ## codex (2026-07-06, bench-operator-proofing) - 8/8 code-feature tasks passed attempt 1 across 3 rounds (worktrees mode, Python harness refactor; 108k-406k tokens/task). Specs embedded the approved architecture doc + exact file ownership; checks built fresh uv venvs and ran the full pytest suite. From 0e398c194a8eeb7b4783edc105960ca4d1414327 Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 10 Sep 2026 20:55:26 +0200 Subject: [PATCH 18/19] The canary must resemble the batch it gates, not merely come first work#1031's canary took runtimes[0] -- manifest order, with no notion of whether that task looks anything like the batch behind it. Measured the same day it shipped: a 32-task scout ran 31 tasks on one model and task 1 on a weaker one left over from an earlier audition. Task 1 failed, 31 tasks were skipped, and the model they would have used had passed the identical check first try minutes earlier. That direction is free and reversible, which is why it was survivable. The other direction is not: a canary EASIER than its batch passes and releases work that then fails once per task, reaching the expensive failure THROUGH the gate meant to prevent it. Selection is now by (engine, model) -- both, because either alone misleads: two tasks on one engine may run different models, and one model under a different engine is a different harness. The most common pair wins, ties break by manifest order, and a uniform batch therefore keeps its first task exactly as before. Being right is not enough if nobody can see it, so the run now says which task was chosen, what it was moved off, and how much of the batch it speaks for -- and the record carries the same under preflight.canary.selection. A batch with no majority still runs its canary (one task's spend is cheap) but prints that a FAIL there means "this task failed", not "the batch is broken". That distinction is the whole point: a hold nobody can explain is how a gate gets waived by reflex. Five new tests, each run against the pre-fix tree first, where all five fail: a minority first task, a uniform batch, dominance beating manifest order, the exact half boundary, and a fragmented batch that warns. Suite 327 -> 332. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KX9rgZWnE5zAmJmhaBz35S --- ringer.py | 102 +++++++++++++++++++- tests/test_prove_before_buy.py | 170 +++++++++++++++++++++++++++++++++ 2 files changed, 271 insertions(+), 1 deletion(-) diff --git a/ringer.py b/ringer.py index a67ad5646..6a3cb1b22 100755 --- a/ringer.py +++ b/ringer.py @@ -2524,6 +2524,9 @@ def __init__( # what the judgment was. Preflight itself stays frozen and describes # only what was decided BEFORE dispatch. self.canary_verdict: dict[str, Any] | None = None + # WHICH task was the canary and why that one. Separate from the + # verdict: the verdict says what it found, this says what it spoke for. + self.canary_selection: dict[str, Any] | None = None self.run_id = run_id self.run_name = run_name self.identity = identity @@ -2701,6 +2704,7 @@ def _preflight_record(self) -> dict[str, Any]: """ record = self.preflight.to_record() record["canary"]["verdict"] = self.canary_verdict + record["canary"]["selection"] = self.canary_selection return record def build_summary(self) -> dict[str, int]: @@ -9193,7 +9197,76 @@ def _split_canary(self) -> tuple[TaskRuntime | None, list[TaskRuntime]]: "reason": "single-task run is its own canary", } return None, list(self.runtimes) - return self.runtimes[0], list(self.runtimes[1:]) + canary = self._pick_canary() + rest = [r for r in self.runtimes if r is not canary] + return canary, rest + + def _worker_pair(self, runtime: TaskRuntime) -> tuple[str, str]: + """What a task will actually be run BY. The unit of representativeness. + + Engine and model together, because either alone can mislead: two tasks + on the same engine may run different models, and the same model under a + different engine is a different harness. + """ + engine = self.config.engines.get(runtime.task.engine) + model = runtime.task.model or (engine.model_default if engine else "") or "" + return (runtime.task.engine, model) + + def _pick_canary(self) -> TaskRuntime: + """The first task that RESEMBLES the batch, not simply the first task. + + The canary is only evidence about the tasks it resembles. Taking + `runtimes[0]` blindly gets that wrong in both directions, and the two + directions cost very differently: + + too weak — a minority probe fails and holds a healthy batch. Free + and reversible, but it is the wrong answer, and a hold + nobody can explain is how a gate gets waived by reflex. + too strong— a minority probe passes and releases work that then fails + once per task. This is the expensive direction, and it + reaches it THROUGH the gate. + + Measured 2026-09-10 (work#1044): a 32-task scout ran 31 tasks on one + model and task 1 on a different, weaker one left over from an earlier + audition. It failed, 31 tasks were skipped, and the model they would + have used had passed the identical check first try minutes before. + + Ties are broken by manifest order, so selection is deterministic: the + dominant pair is the most common one, and the earliest-declared of + equally common pairs wins. + """ + counts: dict[tuple[str, str], int] = {} + for runtime in self.runtimes: + pair = self._worker_pair(runtime) + counts[pair] = counts.get(pair, 0) + 1 + # max() keeps the first maximum it meets, and dicts preserve insertion + # order, so this is "most common, earliest declared" without a sort key + # that could reorder equal counts differently between runs. + dominant = max(counts, key=lambda pair: counts[pair]) + for runtime in self.runtimes: + if self._worker_pair(runtime) == dominant: + return runtime + return self.runtimes[0] # unreachable: dominant came from this list + + def _canary_selection(self, canary: TaskRuntime) -> dict[str, Any]: + """Why this task, and how much of the batch it actually speaks for.""" + pair = self._worker_pair(canary) + same = sum(1 for r in self.runtimes if self._worker_pair(r) == pair) + total = len(self.runtimes) + first_pair = self._worker_pair(self.runtimes[0]) + engine, model = pair + return { + "task": canary.task.key, + "engine": engine, + "model": model, + "covers": same, + "of": total, + # A canary speaking for less than half the batch is a weak probe + # whatever it says. Naming that is the difference between "the + # batch is broken" and "the canary was the wrong instrument". + "representative": same * 2 >= total, + "moved_from": self.runtimes[0].task.key if pair != first_pair else None, + } async def _dispatch(self) -> None: """Release the canary, judge it, then release the rest. @@ -9206,11 +9279,38 @@ async def _dispatch(self) -> None: if canary is None: await asyncio.gather(*(self._run_task(runtime) for runtime in rest)) return + selection = self._canary_selection(canary) + self.state_writer.canary_selection = selection print( f"\nCanary: running 1 of {len(self.runtimes)} tasks " f"({canary.task.key}) before releasing the other {len(rest)}.", flush=True, ) + if selection["moved_from"]: + # Silence here would be the defect: a canary quietly moved is + # indistinguishable from a canary that was always first. + print( + f" not {selection['moved_from']}, which is the only task on " + f"{selection['engine']}/{selection['model'] or '(engine default)'}" + if selection["covers"] == 1 + else f" not {selection['moved_from']} — a minority " + f"engine/model for this batch", + flush=True, + ) + print( + f" speaks for {selection['covers']} of {selection['of']} task(s) " + f"({selection['engine']}/{selection['model'] or 'engine default'})", + flush=True, + ) + if not selection["representative"]: + # A weak probe is still worth running -- it is one task's spend -- + # but its verdict must not be read as a statement about the batch. + print( + " ⚠️ this batch has no majority engine/model, so the canary " + "speaks for less than half of it. Read a FAIL as 'this task " + "failed', not 'the batch is broken'.", + flush=True, + ) await self._run_task(canary) await self._judge_canary(canary, held_back=len(rest)) await asyncio.gather(*(self._run_task(runtime) for runtime in rest)) diff --git a/tests/test_prove_before_buy.py b/tests/test_prove_before_buy.py index 95d6c6774..fc1422aa5 100644 --- a/tests/test_prove_before_buy.py +++ b/tests/test_prove_before_buy.py @@ -21,6 +21,8 @@ * both escapes exist, demand a REASON, and announce themselves * the baseline verdict lands in the run record, so "was this checked?" is answerable later rather than remembered + * the canary RESEMBLES the batch it gates -- it is not merely the first + task -- and the run record says which task it was and what it spoke for """ from __future__ import annotations @@ -73,6 +75,27 @@ def setUp(self) -> None: "sandbox_args = []", "full_access_args = []", "", + # A second, identically-behaving engine. Identical on + # purpose: these tests are about which task is CHOSEN as + # the canary, so the two must differ only in name. + "[engines.other]", + f"bin = {toml_string(sys.executable)}", + "args_template = [", + f" {toml_string(MOCK_WORKER)},", + ' "{spec}",', + "]", + "sandbox_args = []", + "full_access_args = []", + "", + "[engines.third]", + f"bin = {toml_string(sys.executable)}", + "args_template = [", + f" {toml_string(MOCK_WORKER)},", + ' "{spec}",', + "]", + "sandbox_args = []", + "full_access_args = []", + "", ] ), encoding="utf-8", @@ -359,6 +382,153 @@ def test_a_single_task_run_skips_the_canary_and_says_so(self) -> None: self.assertEqual(0, result.returncode, output) self.assertIn("Canary: skipped — a single-task run is its own canary", output) + # ---- the canary must resemble the batch it gates -------------------- + + def test_a_minority_first_task_does_not_gate_the_batch(self) -> None: + # The measured incident (work#1044): a 32-task scout ran 31 tasks on + # one model and task 1 on a weaker one left over from an audition. It + # failed, 31 tasks were skipped, and the model they would have used had + # passed the identical check first try minutes earlier. + # + # Here task 1 is the only task on `other`, and it is a task whose + # worker fails. If it were still chosen, the batch would be held. + manifest = self.write_manifest( + [ + {**self.writes_nothing("odd-one-out", "never.txt"), "engine": "other"}, + self.writes("first", "first.txt"), + self.writes("second", "second.txt"), + self.writes("third", "third.txt"), + ] + ) + + result = self.run_ringer(manifest) + output = result.stdout + result.stderr + + # The canary moved off the minority task, and said so. + self.assertIn("not odd-one-out", output) + self.assertIn("Canary: first passed", output) + + record = self.read_run_record() + sel = record["preflight"]["canary"]["selection"] + self.assertEqual("first", sel["task"]) + self.assertEqual("mock", sel["engine"]) + self.assertEqual(3, sel["covers"]) + self.assertEqual(4, sel["of"]) + self.assertTrue(sel["representative"]) + self.assertEqual("odd-one-out", sel["moved_from"]) + + # And the batch really was released: the minority task ran anyway, + # and failed on its own merits rather than gating anything. + by_key = {t["key"]: t for t in record["tasks"]} + for released in ("second", "third"): + self.assertEqual("pass", by_key[released]["status"], record) + self.assertEqual("fail", by_key["odd-one-out"]["status"], record) + + def test_a_uniform_batch_keeps_the_first_task_as_canary(self) -> None: + # The complement. When every task shares an engine/model there is + # nothing to move to, and manifest order must be preserved — a + # selection rule that reorders a uniform batch would be churn. + manifest = self.write_manifest( + [ + self.writes("alpha", "alpha.txt"), + self.writes("beta", "beta.txt"), + self.writes("gamma", "gamma.txt"), + ] + ) + + result = self.run_ringer(manifest) + output = result.stdout + result.stderr + + self.assertEqual(0, result.returncode, output) + self.assertNotIn("not alpha", output) + sel = self.read_run_record()["preflight"]["canary"]["selection"] + self.assertEqual("alpha", sel["task"]) + self.assertEqual(3, sel["covers"]) + self.assertIsNone(sel["moved_from"]) + self.assertTrue(sel["representative"]) + + def test_a_batch_with_no_majority_says_the_canary_is_a_weak_probe(self) -> None: + # Two engines, two tasks each: whichever is picked speaks for half. + # The gate still runs — one task's spend is cheap — but its verdict + # must not be read as a statement about the batch, and the run says so + # rather than leaving the operator to infer it. + manifest = self.write_manifest( + [ + self.writes("a1", "a1.txt"), + {**self.writes("b1", "b1.txt"), "engine": "other"}, + self.writes("a2", "a2.txt"), + {**self.writes("b2", "b2.txt"), "engine": "other"}, + ] + ) + + result = self.run_ringer(manifest) + output = result.stdout + result.stderr + + self.assertEqual(0, result.returncode, output) + self.assertIn("speaks for 2 of 4", output) + sel = self.read_run_record()["preflight"]["canary"]["selection"] + self.assertEqual(2, sel["covers"]) + self.assertEqual(4, sel["of"]) + # 2 of 4 is exactly half, which still counts as representative -- + # pinned so the boundary is a decision rather than an accident. + self.assertTrue(sel["representative"]) + + def test_the_dominant_pair_wins_even_when_it_is_not_first(self) -> None: + # Majority rules over manifest order: `other` holds 3 of 5, so the + # canary moves off the first task even though that task is perfectly + # healthy. Nothing is wrong with a1 — it simply is not what most of + # this batch will run as. + manifest = self.write_manifest( + [ + self.writes("a1", "a1.txt"), + self.writes("a2", "a2.txt"), + {**self.writes("b1", "b1.txt"), "engine": "other"}, + {**self.writes("b2", "b2.txt"), "engine": "other"}, + {**self.writes("b3", "b3.txt"), "engine": "other"}, + ], + max_parallel=2, + ) + + result = self.run_ringer(manifest) + + self.assertEqual(0, result.returncode, result.stdout + result.stderr) + sel = self.read_run_record()["preflight"]["canary"]["selection"] + self.assertEqual("other", sel["engine"]) + self.assertEqual("b1", sel["task"]) + self.assertEqual(3, sel["covers"]) + self.assertTrue(sel["representative"]) + self.assertEqual("a1", sel["moved_from"]) + + def test_a_canary_speaking_for_a_minority_warns(self) -> None: + # A genuinely fragmented batch: three distinct engine/model pairs at + # 2 / 2 / 1, so whichever is chosen speaks for 2 of 5 — under half. + # + # The gate still runs, because one task's spend is cheap. What it must + # NOT do is let that verdict read as a statement about the batch. + manifest = self.write_manifest( + [ + self.writes("a1", "a1.txt"), + self.writes("a2", "a2.txt"), + {**self.writes("b1", "b1.txt"), "engine": "other"}, + {**self.writes("b2", "b2.txt"), "engine": "other"}, + {**self.writes("c1", "c1.txt"), "engine": "third"}, + ], + max_parallel=2, + ) + + result = self.run_ringer(manifest) + output = result.stdout + result.stderr + + self.assertEqual(0, result.returncode, output) + self.assertIn("speaks for 2 of 5", output) + self.assertIn("no majority engine/model", output) + self.assertIn("not 'the batch is broken'", output) + + sel = self.read_run_record()["preflight"]["canary"]["selection"] + self.assertEqual(2, sel["covers"]) + self.assertEqual(5, sel["of"]) + self.assertFalse(sel["representative"]) + # ---- the escapes --------------------------------------------------- def test_the_baseline_waiver_announces_itself_and_is_recorded(self) -> None: From f227dc903f8471a2e9a0d721a468f9e27e7cdded Mon Sep 17 00:00:00 2001 From: Barry Faassen Date: Thu, 10 Sep 2026 20:58:47 +0200 Subject: [PATCH 19/19] Review pass 1: test the minority-MODEL case, which is the one that actually happened The lens is right that the tests did not exercise it. Every case differentiated tasks by ENGINE, so the model half of the (engine, model) pairing was unproven -- and the model half is the one the measured incident used: one harness, two models, the weaker declared first. Pairing on engine alone would pick the failing task there and hold the batch, because all its tasks share an engine. The gap was not an oversight in judgement so much as a dead end I did not push past: the first attempt gave two tasks different models on an engine whose args_template has no {model} placeholder, and ringer correctly refused it -- "model is set but engine mock has no {model} placeholder, so it would be silently ignored". I took the refusal as "cannot test this" and moved to engines. It meant "use an engine that takes a model". So there is now one: mockm, with {model} ahead of {spec} because the mock worker reads argv[-1] as its spec, leaving the model arg inert to it. The new test is the incident in miniature -- weak-model first and failing, strong-model x3 behind it -- and it fails against the pre-fix tree. Suite 332 -> 333. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KX9rgZWnE5zAmJmhaBz35S --- tests/test_prove_before_buy.py | 56 ++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/test_prove_before_buy.py b/tests/test_prove_before_buy.py index fc1422aa5..32a6a9bd9 100644 --- a/tests/test_prove_before_buy.py +++ b/tests/test_prove_before_buy.py @@ -96,6 +96,22 @@ def setUp(self) -> None: "sandbox_args = []", "full_access_args = []", "", + # An engine that genuinely takes a model, so tasks can + # differ by MODEL on one engine -- the shape of the real + # incident (one harness, two models, the weaker one first). + # {model} precedes {spec} because the mock worker reads + # argv[-1] as the spec, so the model arg is inert to it. + "[engines.mockm]", + f"bin = {toml_string(sys.executable)}", + 'model_default = "base-model"', + "args_template = [", + f" {toml_string(MOCK_WORKER)},", + ' "{model}",', + ' "{spec}",', + "]", + "sandbox_args = []", + "full_access_args = []", + "", ] ), encoding="utf-8", @@ -424,6 +440,46 @@ def test_a_minority_first_task_does_not_gate_the_batch(self) -> None: self.assertEqual("pass", by_key[released]["status"], record) self.assertEqual("fail", by_key["odd-one-out"]["status"], record) + def test_a_minority_MODEL_on_one_engine_does_not_gate_the_batch(self) -> None: + # This is the measured incident's exact shape, and the half the + # engine-based tests above cannot reach: ONE engine, two models, the + # weaker one declared first. In the real run that weak task failed and + # took 31 healthy tasks with it. + # + # Pairing on engine alone would pick the failing task here and hold + # the batch, because all four tasks share an engine. + manifest = self.write_manifest( + [ + { + **self.writes_nothing("weak", "never.txt"), + "engine": "mockm", + "model": "weak-model", + }, + {**self.writes("s1", "s1.txt"), "engine": "mockm", "model": "strong-model"}, + {**self.writes("s2", "s2.txt"), "engine": "mockm", "model": "strong-model"}, + {**self.writes("s3", "s3.txt"), "engine": "mockm", "model": "strong-model"}, + ] + ) + + result = self.run_ringer(manifest) + output = result.stdout + result.stderr + + self.assertIn("not weak", output) + self.assertIn("Canary: s1 passed", output) + + sel = self.read_run_record()["preflight"]["canary"]["selection"] + self.assertEqual("s1", sel["task"]) + self.assertEqual("mockm", sel["engine"]) + self.assertEqual("strong-model", sel["model"]) + self.assertEqual(3, sel["covers"]) + self.assertEqual("weak", sel["moved_from"]) + self.assertTrue(sel["representative"]) + + by_key = {t["key"]: t for t in self.read_run_record()["tasks"]} + for released in ("s2", "s3"): + self.assertEqual("pass", by_key[released]["status"]) + self.assertEqual("fail", by_key["weak"]["status"]) + def test_a_uniform_batch_keeps_the_first_task_as_canary(self) -> None: # The complement. When every task shares an engine/model there is # nothing to move to, and manifest order must be preserved — a