Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ Each task gets its own directory, its own worker, its own log, and its own verdi
| `model` | Which model a harness engine runs for this task — fills the engine's `{model}` placeholder (e.g. `"openrouter/moonshotai/kimi-k2.7"`); empty uses the engine's `model_default` |
| `task_type` | Optional free-form string naming the kind of work this task is, so the model-performance log can slice pass rates by task shape rather than only by model. Suggested vocabulary: `code-feature`, `code-fix`, `code-review`, `test-hardening`, `docs`, `research`, `persona-review`, `copywriting`, `site-build`, `motion-design`, `image-gen`, `data-pipeline`, `format-conversion`, `probe`, `bakeoff`. Empty is allowed; the log just reports it under `(none)`. |
| `timeout_s` | Per-task kill timer (default 900) |
| `check_timeout_s` | Optional per-task override of the default check timeout for checks that boot infrastructure. Match the timeout to the task applies to checks too. |
| `engine_args` | Extra CLI flags for this task's worker, spliced in at the engine's `{engine_args}` placeholder — e.g. `["-c", "model_reasoning_effort=low"]` so the orchestrator picks reasoning depth per task |
| `verified` | One plain-English sentence saying what the check proves — shown on the results page next to "finished & checked" |
| `full_access` | Worker runs unsandboxed — required for workers that spawn their own sub-workers; must also be enabled in config |
Expand Down Expand Up @@ -357,6 +358,7 @@ Every community PR that lands in main is credited here — that's a project rule
- [@davekopecek](https://github.com/davekopecek) (Dave Kopecek) — committed the design-reference fixture so the design-token guard runs on every machine (#30)
- [@snapsynapse](https://github.com/snapsynapse) (Sam Rogers) — graceful shutdown on SIGINT/SIGTERM with worker-tree cleanup and finished state, plus the 14-test end-to-end CLI regression suite (#4)
- [@mlava](https://github.com/mlava) (Mark Lavercombe) — named setup failures across every diagnostic surface (#37) and `run --baseline`, the no-workers check preflight (#38)
- [@davidfoy](https://github.com/davidfoy) (David Foy) — per-task `check_timeout_s` so infrastructure-booting checks don't time out under the fixed default (OA-140)

Contributions are welcome — see [CONTRIBUTING.md](CONTRIBUTING.md) for the philosophy and what gets a PR merged fast. The short version: small and scoped, rebased on current main, every claim backed by an executed test. Authorship is always preserved — where a maintainer pushes a mechanical fix to your branch, you remain the commit author.

Expand Down
39 changes: 35 additions & 4 deletions ringer.py
Original file line number Diff line number Diff line change
Expand Up @@ -1010,6 +1010,7 @@ class TaskSpec:
# engine's {model} placeholder); empty means the engine's model_default.
model: str = ""
task_type: str = ""
check_timeout_s: int | None = None

@classmethod
def from_obj(cls, obj: dict[str, Any]) -> "TaskSpec":
Expand Down Expand Up @@ -1050,6 +1051,11 @@ def from_obj(cls, obj: dict[str, Any]) -> "TaskSpec":
task_type = obj.get("task_type", "")
if not isinstance(task_type, str):
raise ValueError(f"task {key}: task_type must be a string")
check_timeout_s = None
if "check_timeout_s" in obj:
check_timeout_s = int(obj.get("check_timeout_s"))
if check_timeout_s <= 0:
raise ValueError(f"task {key}: check_timeout_s must be positive")
return cls(
key=key,
spec=spec,
Expand All @@ -1062,6 +1068,7 @@ def from_obj(cls, obj: dict[str, Any]) -> "TaskSpec":
verified=verified.strip(),
model=model.strip(),
task_type=task_type.strip(),
check_timeout_s=check_timeout_s,
)


Expand Down Expand Up @@ -1202,6 +1209,10 @@ def lint_manifest(
f"{task.key}: no 'verified' description; a reader of the results page sees "
"'checked' but not what the check proves — add one plain-English sentence."
)
if task.check_timeout_s is None and check_boots_infrastructure(task.check):
findings.append(
f"{task.key}: check appears to boot infrastructure; set check_timeout_s to match the task."
)
if include_model_log_nudges and not task.task_type:
findings.append(
f"{task.key}: no task_type; the model log buckets this as (untyped) — "
Expand Down Expand Up @@ -1267,6 +1278,13 @@ def check_cannot_fail(check: str) -> bool:
return consists_only_of_echo_commands(stripped)


INFRA_BOOT_CHECK_RE = re.compile(r"(?:\bdocker\b|\bcompose\s+up\b)", re.IGNORECASE)


def check_boots_infrastructure(check: str) -> bool:
return bool(INFRA_BOOT_CHECK_RE.search(strip_shell_comments(check)))


def strip_shell_comments(command: str) -> str:
result: list[str] = []
in_single = False
Expand Down Expand Up @@ -7951,7 +7969,11 @@ def run_models_command(config: AppConfig, args: argparse.Namespace) -> int:

class Verifier:
async def verify(self, task: TaskSpec, taskdir: Path) -> VerifyResult:
check_returncode, check_timed_out, output = await self._run_check(task.check, taskdir)
check_returncode, check_timed_out, output = await self._run_check(
task.check,
taskdir,
timeout_s=task.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))
)
Expand Down Expand Up @@ -7989,7 +8011,13 @@ 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 | None = None,
) -> tuple[int | None, bool, str]:
effective_timeout_s = timeout_s if timeout_s is not None else CHECK_TIMEOUT_S
timeout_source = "task check_timeout_s" if timeout_s is not None else "default"
proc = await asyncio.create_subprocess_shell(
command,
cwd=str(cwd),
Expand All @@ -8000,7 +8028,10 @@ 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=effective_timeout_s,
)
except asyncio.TimeoutError:
timed_out = True
terminate_process_group(proc)
Expand All @@ -8011,7 +8042,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 {effective_timeout_s}s ({timeout_source})\n"
return proc.returncode, timed_out, output


Expand Down
204 changes: 204 additions & 0 deletions tests/test_check_timeout.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
#!/usr/bin/env python3
"""Oracle for per-task check timeouts (OA-140).

A fixed CHECK_TIMEOUT_S = 60 fails any check that boots real infrastructure
(a docker-backed test stack takes ~90s), marking good workers failed. These
tests pin the contract for the optional per-task `check_timeout_s` field:

- absent -> behavior identical to today (module constant CHECK_TIMEOUT_S,
consulted at call time so tests can shrink it)
- present -> positive int, threaded into the check's asyncio.wait_for
- the timeout message names the limit AND where it came from, so retry
prompts and post-mortems show whether the cap was chosen or inherited
- lint warns when a check looks like it boots infrastructure (generic
patterns only: docker / compose up) while check_timeout_s is unset

Set RINGER_SLOW_ORACLE=1 to also run the ticket-verbatim slow cases
(a real 90s check against the real 60s default).
"""
from __future__ import annotations

import asyncio
import os
import sys
import tempfile
import time
import unittest
from pathlib import Path

ROOT = Path(__file__).resolve().parents[1]
sys.path.insert(0, str(ROOT))

import ringer as ringer_module # noqa: E402
from ringer import Manifest, TaskSpec, Verifier, lint_manifest # noqa: E402

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."
)

PLAIN_CHECK = (
"test -s output.txt && grep -q 'ready' output.txt || "
"{ echo 'FAIL: output.txt missing or does not contain ready'; exit 1; }"
)

STACK_BOOT_CHECK = (
"docker compose up -d && ./scripts/wait-healthy.sh && pytest tests/ || "
"{ echo 'FAIL: stack tests failed'; exit 1; }"
)


def task_obj(key: str = "one", **overrides: object) -> dict[str, object]:
obj: dict[str, object] = {
"key": key,
"spec": LONG_SPEC,
"check": PLAIN_CHECK,
"expect_files": ["output.txt"],
"verified": "output.txt exists and contains ready",
"task_type": "probe",
}
obj.update(overrides)
return obj


class TaskSpecCheckTimeoutTests(unittest.TestCase):
def test_absent_field_parses_as_none(self) -> None:
task = TaskSpec.from_obj(task_obj())
self.assertIsNone(task.check_timeout_s)

def test_present_field_parses_as_int(self) -> None:
task = TaskSpec.from_obj(task_obj(check_timeout_s=120))
self.assertEqual(task.check_timeout_s, 120)

def test_nonpositive_field_rejected(self) -> None:
for bad in (0, -5):
with self.assertRaises(ValueError):
TaskSpec.from_obj(task_obj(check_timeout_s=bad))


class VerifierCheckTimeoutTests(unittest.TestCase):
def verify(self, task: TaskSpec) -> "ringer_module.VerifyResult":
with tempfile.TemporaryDirectory() as tmp:
return asyncio.run(Verifier().verify(task, Path(tmp)))

def test_task_check_timeout_is_honored(self) -> None:
# sleep 3 passes under the 60s default; with a 1s per-task cap the
# check must time out, and quickly (proof the cap was threaded, not
# the constant).
task = TaskSpec.from_obj(
task_obj(check="sleep 3 && echo ok", check_timeout_s=1, expect_files=[])
)
start = time.monotonic()
result = self.verify(task)
elapsed = time.monotonic() - start
self.assertTrue(result.check_timed_out)
self.assertFalse(result.ok)
self.assertLess(elapsed, 3.0)

def test_timeout_message_names_task_source(self) -> None:
task = TaskSpec.from_obj(
task_obj(check="sleep 3 && echo ok", check_timeout_s=1, expect_files=[])
)
result = self.verify(task)
self.assertIn("check timed out after 1s", result.raw_output_excerpt)
self.assertIn("task check_timeout_s", result.raw_output_excerpt)

def test_timeout_message_names_default_source(self) -> None:
# The default path must consult the module constant at call time;
# shrink it so the default-timeout branch is provable in seconds.
original = ringer_module.CHECK_TIMEOUT_S
ringer_module.CHECK_TIMEOUT_S = 1
try:
task = TaskSpec.from_obj(
task_obj(check="sleep 3 && echo ok", expect_files=[])
)
result = self.verify(task)
finally:
ringer_module.CHECK_TIMEOUT_S = original
self.assertTrue(result.check_timed_out)
self.assertIn("check timed out after 1s", result.raw_output_excerpt)
self.assertIn("default", result.raw_output_excerpt)
self.assertNotIn("task check_timeout_s", result.raw_output_excerpt)

def test_generous_task_timeout_lets_slow_check_pass(self) -> None:
original = ringer_module.CHECK_TIMEOUT_S
ringer_module.CHECK_TIMEOUT_S = 1
try:
task = TaskSpec.from_obj(
task_obj(check="sleep 3 && echo ok", check_timeout_s=30, expect_files=[])
)
result = self.verify(task)
finally:
ringer_module.CHECK_TIMEOUT_S = original
self.assertFalse(result.check_timed_out)
self.assertEqual(result.check_returncode, 0)
self.assertTrue(result.ok)


class LintStackBootTests(unittest.TestCase):
def manifest(self, tasks: list[dict[str, object]]) -> Manifest:
temp_dir = tempfile.TemporaryDirectory()
self.addCleanup(temp_dir.cleanup)
return Manifest.from_obj(
{
"run_name": "check-timeout-lint-test",
"workdir": str(Path(temp_dir.name) / "work"),
"max_parallel": 1,
"worktrees": False,
"tasks": tasks,
}
)

def findings_mentioning_check_timeout(self, tasks: list[dict[str, object]]) -> list[str]:
findings = lint_manifest(self.manifest(tasks), allow_noncanonical_route=True)
return [f for f in findings if "check_timeout_s" in f]

def test_warns_on_stack_boot_check_without_field(self) -> None:
findings = self.findings_mentioning_check_timeout(
[task_obj(check=STACK_BOOT_CHECK)]
)
self.assertEqual(len(findings), 1)
self.assertIn("one:", findings[0])

def test_no_warning_when_field_is_set(self) -> None:
findings = self.findings_mentioning_check_timeout(
[task_obj(check=STACK_BOOT_CHECK, check_timeout_s=300)]
)
self.assertEqual(findings, [])

def test_no_warning_on_plain_check(self) -> None:
findings = self.findings_mentioning_check_timeout([task_obj()])
self.assertEqual(findings, [])


@unittest.skipUnless(
os.environ.get("RINGER_SLOW_ORACLE") == "1",
"ticket-verbatim slow oracle; set RINGER_SLOW_ORACLE=1 (runs ~2.5 min)",
)
class SlowTicketOracleTests(unittest.TestCase):
"""OA-140 oracle, verbatim: sleep 90 && echo ok against the REAL default."""

def verify(self, task: TaskSpec) -> "ringer_module.VerifyResult":
with tempfile.TemporaryDirectory() as tmp:
return asyncio.run(Verifier().verify(task, Path(tmp)))

def test_90s_check_fails_with_default(self) -> None:
task = TaskSpec.from_obj(
task_obj(check="sleep 90 && echo ok", expect_files=[])
)
result = self.verify(task)
self.assertTrue(result.check_timed_out)
self.assertFalse(result.ok)

def test_90s_check_passes_with_check_timeout_120(self) -> None:
task = TaskSpec.from_obj(
task_obj(check="sleep 90 && echo ok", check_timeout_s=120, expect_files=[])
)
result = self.verify(task)
self.assertFalse(result.check_timed_out)
self.assertEqual(result.check_returncode, 0)
self.assertTrue(result.ok)


if __name__ == "__main__":
unittest.main()