diff --git a/config.sample.toml b/config.sample.toml index b7476bd6..81ea616a 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/docs/MODEL-NOTES.md b/docs/MODEL-NOTES.md index fbc20034..5532d8ec 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 b9262cd3..30c6f2ec 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 diff --git a/ringer.py b/ringer.py index aa75f845..0537b255 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 6837aa72..8f77c8df 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 7b184fe3..8e37e8bc 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 00000000..f266cf1f --- /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()