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
42 changes: 42 additions & 0 deletions ringer.py
Original file line number Diff line number Diff line change
Expand Up @@ -1814,6 +1814,22 @@ def lint_manifest(
if manifest.run_name == MODEL_SCOREBOARD_RUN_NAME:
findings.append("manifest: run_name model-scoreboard is reserved for the scoreboard page.")

# An engine name that resolves to nothing is the one manifest error that
# costs a whole dispatch to discover: `run` fails at spawn time, once the
# run row, the worktree and the dashboard entry already exist. Lint used to
# call a manifest naming `no-such-engine-xyz` clean, because nothing here
# ever looked the name up: the only code that touched it,
# noncanonical_route_findings, does a `.get()` that returns None and then
# `continue`s, so an unknown engine took the quiet path out.
if config is not None:
known = ", ".join(sorted(config.engines))
for task in manifest.tasks:
if task.engine not in config.engines:
findings.append(
f"ERROR: {task.key}: engine {task.engine!r} is not configured; "
f"engines available here: {known}."
)

for task in manifest.tasks:
if check_cannot_fail(task.check):
findings.append(f"{task.key}: check cannot fail, so the task cannot be verified.")
Expand Down Expand Up @@ -10925,6 +10941,9 @@ def build_parser() -> argparse.ArgumentParser:

lint_parser = subparsers.add_parser("lint", help="lint a ringer manifest")
lint_parser.add_argument("manifest", type=Path, help="path to ringer.json")
# Lint reads the config now (engine names, model routes), so it takes the
# same suppressed --config every other config-consuming command takes.
lint_parser.add_argument("--config", type=Path, default=argparse.SUPPRESS, help=argparse.SUPPRESS)
lint_parser.add_argument(
"--allow-noncanonical-route",
action="store_true",
Expand Down Expand Up @@ -11039,10 +11058,33 @@ def main(argv: list[str] | None = None) -> int:

if args.command == "lint":
manifest = Manifest.from_path(args.manifest)
# Lint ran with config=None until 2026-08-15 - the shared
# `AppConfig.load` below sits AFTER this branch returns. That made
# two checks vacuous at once: engine names were never resolvable,
# and noncanonical_route_findings fell back to the identity
# registry's defaults instead of the engine's real model_default
# for every task that does not name a model. Load it here.
lint_config: AppConfig | None
config_error = ""
try:
lint_config = AppConfig.load(args.config)
except Exception as exc: # noqa: BLE001 - reported, not swallowed
lint_config = None
config_error = str(exc)
findings = lint_manifest(
manifest,
config=lint_config,
allow_noncanonical_route=args.allow_noncanonical_route,
)
if config_error:
# Say so rather than degrade quietly: without a config the
# engine and route checks did not run, and a bare "clean" would
# claim more than was actually checked.
findings.insert(
0,
f"ERROR: manifest: ringer config could not be loaded ({config_error}); "
"engine names and model routes were NOT checked.",
)
if findings:
print_lint_findings(findings)
return 1
Expand Down
126 changes: 125 additions & 1 deletion tests/test_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
from __future__ import annotations

import asyncio
import contextlib
import io
import json
import os
import sys
import tempfile
Expand All @@ -11,7 +14,7 @@
ROOT = Path(__file__).resolve().parents[1]
sys.path.insert(0, str(ROOT))

from ringer import Manifest, TaskSpec, Verifier, lint_manifest # noqa: E402
from ringer import AppConfig, Manifest, TaskSpec, Verifier, lint_manifest, main # noqa: E402


LONG_SPEC = (
Expand Down Expand Up @@ -311,5 +314,126 @@ def test_templates_are_clean(self) -> None:
self.assertEqual([], findings, f"{path} should lint clean, got: {findings}")


class UnknownEngineLintTests(unittest.TestCase):
"""`ringer lint` called a manifest naming a nonexistent engine clean.

Two separate holes, and the second is the one that made the first
invisible: `lint_manifest` never resolved the engine name at all, AND the
`lint` CLI branch returned before the shared `AppConfig.load`, so even a
correct check would have run with `config=None` and found nothing.
Both halves are pinned here, and each guard is exercised in BOTH
directions - a guard only proven to block is indistinguishable from one
welded shut.
"""

def config_with(self, *engines: str) -> AppConfig:
path = self.write_config(*engines)
return AppConfig.load(path)

def write_config(self, *engines: str) -> Path:
temp_dir = tempfile.TemporaryDirectory()
self.addCleanup(temp_dir.cleanup)
path = Path(temp_dir.name) / "config.toml"
body = "".join(
f'[engines.{name}]\nbin = "{name}"\nargs_template = ["{{spec}}"]\n\n' for name in engines
)
path.write_text(body, encoding="utf-8")
return path

def manifest_using(self, engine: str) -> Manifest:
temp_dir = tempfile.TemporaryDirectory()
self.addCleanup(temp_dir.cleanup)
return Manifest.from_obj(
{
"run_name": "engine-lint-test",
"workdir": str(Path(temp_dir.name) / "work"),
"max_parallel": 1,
"tasks": [
{
"key": "one",
"engine": engine,
"spec": LONG_SPEC,
"check": GOOD_CHECK,
"expect_files": ["output.txt"],
"verified": "the output file exists and contains the expected content",
}
],
}
)

def engine_findings(self, findings: list[str]) -> list[str]:
return [f for f in findings if "is not configured" in f]

def test_unknown_engine_is_reported(self) -> None:
findings = lint_manifest(self.manifest_using("no-such-engine-xyz"), config=self.config_with("cline"))
self.assertEqual(1, len(self.engine_findings(findings)), findings)
self.assertIn("no-such-engine-xyz", self.engine_findings(findings)[0])
# The message must name what IS available, so a config that failed to
# load is diagnosable from the finding alone.
self.assertIn("cline", self.engine_findings(findings)[0])

def test_configured_engine_is_not_reported(self) -> None:
# The half that proves the guard is not welded shut.
findings = lint_manifest(self.manifest_using("cline"), config=self.config_with("cline"))
self.assertEqual([], self.engine_findings(findings), findings)

def test_check_is_vacuous_without_config(self) -> None:
# Documents WHY the CLI must load the config: with config=None the
# check cannot fire at all, which is exactly how the bug survived.
findings = lint_manifest(self.manifest_using("no-such-engine-xyz"))
self.assertEqual([], self.engine_findings(findings), findings)

def run_lint_cli(self, engine: str, config_path: Path) -> tuple[int, str]:
manifest_dir = tempfile.TemporaryDirectory()
self.addCleanup(manifest_dir.cleanup)
manifest_path = Path(manifest_dir.name) / "manifest.json"
manifest_path.write_text(
json.dumps(
{
"run_name": "engine-lint-cli",
"workdir": str(Path(manifest_dir.name) / "work"),
"max_parallel": 1,
"tasks": [
{
"key": "one",
"engine": engine,
"spec": LONG_SPEC,
"check": GOOD_CHECK,
"expect_files": ["output.txt"],
"verified": "the output file exists and contains the expected content",
}
],
}
),
encoding="utf-8",
)
previous = os.environ.get("RINGER_NO_SELF_UPDATE")
os.environ["RINGER_NO_SELF_UPDATE"] = "1"
buffer = io.StringIO()
try:
with contextlib.redirect_stdout(buffer):
code = main(["lint", str(manifest_path), "--config", str(config_path)])
finally:
if previous is None:
os.environ.pop("RINGER_NO_SELF_UPDATE", None)
else:
os.environ["RINGER_NO_SELF_UPDATE"] = previous
return code, buffer.getvalue()

def test_cli_lint_loads_the_config(self) -> None:
# The wiring test. lint_manifest can be perfectly correct while the CLI
# keeps passing config=None, which is the state this fix found.
config_path = self.write_config("cline")
code, output = self.run_lint_cli("no-such-engine-xyz", config_path)
self.assertEqual(1, code, output)
self.assertIn("is not configured", output)

def test_cli_lint_stays_clean_on_a_configured_engine(self) -> None:
config_path = self.write_config("cline")
code, output = self.run_lint_cli("cline", config_path)
self.assertEqual(0, code, output)
self.assertIn("lint: clean", output)


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