Lint: reject a check no shell can parse

Burned a full worker run against a check command that no shell could parse.
The manifest linted clean, the worker ran to completion and was paid for, and
the only evidence was an opaque `/bin/sh: syntax error near unexpected token
`('` after the fact. Because a check that fails to parse can never exit 0, the
task can never pass -- every retry attempt is spent before the run fails.

The mistake is easy to make by hand and invisible on the page: single quotes
nested inside a single-quoted argument, which POSIX shells cannot express.

    --verify-command 'a && grep -qE '^## Heading (x|y)' out.md'

The inner quote closes the argument early and `(x|y)` is then reparsed as bare
syntax.

lint now runs `sh -n` on each task's check -- which parses without executing,
so it is safe on an arbitrary command -- and reports the shell's own diagnosis
rather than a bare "invalid", per the house rule that a check should print why
it failed.

shlex.split() is deliberately not used: it accepts this exact string happily,
so it cannot catch the mistake. Where no POSIX shell exists (Windows) the
helper returns None and lint stays silent rather than reporting a platform gap
as a manifest defect; the accompanying tests assert only shell-independent
behaviour on that path.

Proof: tests/test_lint.py gains two tests -- one asserting the finding appears
and carries the shell's diagnosis, one asserting four valid checks (including
correctly-escaped nested quotes) are not flagged. Both fail against unfixed
main and pass with this change. Full suite: 255 tests, the single failure being
test_deliverables.test_runner_harvests_when_task_passes, which fails
identically on pristine main with and without this change.

Author: robreyreynolds

ringer.py

@@ -1815,6 +1815,12 @@ def lint_manifest(
         findings.append("manifest: run_name model-scoreboard is reserved for the scoreboard page.")
 
     for task in manifest.tasks:
+        shell_parse_error = check_shell_parse_error(task.check)
+        if shell_parse_error:
+            findings.append(
+                f"{task.key}: check is not valid shell, so it can never exit 0 and the task "
+                f"can never pass — {shell_parse_error}"
+            )
         if check_cannot_fail(task.check):
             findings.append(f"{task.key}: check cannot fail, so the task cannot be verified.")
         if check_may_fail_silently(task.check):
@@ -1906,6 +1912,48 @@ def spec_is_file_pointer(spec: str) -> bool:
     return bool(FILE_POINTER_SPEC_RE.search(text))
 
 
+# `sh -n` parses without executing, so this is safe to run on an arbitrary check.
+SHELL_PARSE_TIMEOUT_S = 10
+
+
+def check_shell_parse_error(check: str) -> str | None:
+    """Return the shell's own complaint when `check` is not valid shell, else None.
+
+    A task's check is run through the shell, so a check the shell cannot parse can
+    never exit 0: the task can never pass, every retry attempt is spent, and the
+    only evidence is an opaque `/bin/sh: syntax error ...` in the run log after the
+    workers have already been paid for.
+
+    `shlex.split()` is deliberately not used here -- it accepts strings a real shell
+    rejects, notably single quotes nested inside a single-quoted argument, which is
+    the most common way to write an unparseable check by hand:
+
+        --verify-command 'a && grep -qE '^## Heading (x|y)' out.md'
+
+    Returns None when no POSIX shell is available (Windows), so lint degrades to
+    silence there rather than reporting a platform gap as a manifest defect.
+    """
+    shell = shutil.which("sh")
+    if not shell:
+        return None
+    try:
+        proc = subprocess.run(
+            [shell, "-n"],
+            input=check,
+            capture_output=True,
+            text=True,
+            timeout=SHELL_PARSE_TIMEOUT_S,
+        )
+    except (OSError, subprocess.SubprocessError):
+        return None
+    if proc.returncode == 0:
+        return None
+    for line in (proc.stderr or proc.stdout or "").splitlines():
+        if line.strip():
+            return line.strip()
+    return "the shell rejected it"
+
+
 def check_cannot_fail(check: str) -> bool:
     stripped = strip_shell_comments(check).strip()
     if stripped in {"true", ":", "exit 0"}:

tests/test_lint.py

@@ -77,6 +77,42 @@ def test_task_fields_must_be_strings(self) -> None:
         with self.assertRaisesRegex(ValueError, r"task key must be a string"):
             self.manifest([task])
 
+    def test_check_must_be_parseable_by_a_shell(self) -> None:
+        """A check no shell can parse can never exit 0, so the task can never pass.
+
+        Nesting single quotes inside a single-quoted argument is the common way to
+        write one by accident: the inner quote closes the argument early and the
+        rest is reparsed as bare syntax. Caught here, it costs seconds; uncaught,
+        the manifest lints clean, a worker runs to completion and is paid for, and
+        every retry attempt is burned before the run fails on an opaque
+        `/bin/sh: syntax error near unexpected token` from the check.
+        """
+        unparseable = (
+            "verify.py --pattern 'a && grep -qE '^## Heading (x|y)' out.md' --strict"
+        )
+        findings = lint_manifest(self.manifest([self.task(check=unparseable)]))
+        matching = [f for f in findings if f.startswith("one: check is not valid shell")]
+        self.assertTrue(
+            matching,
+            f"expected an unparseable-check finding\nfindings: {findings}",
+        )
+        # The finding must carry the shell's own diagnosis -- a bare "invalid"
+        # leaves the author hunting for which quote broke it.
+        self.assertRegex(matching[0], r"(?i)syntax error|unexpected")
+
+    def test_valid_checks_are_not_flagged_as_unparseable(self) -> None:
+        for check in (
+            GOOD_CHECK,
+            "test -s out.md && grep -qE '^## Heading' out.md",
+            'verify.py --pattern \'a && grep -qE "^## Heading (x|y)" out.md\' --strict',
+            "python3 check.py --arg \"nested 'quotes' fine\" || { echo 'FAIL'; exit 1; }",
+        ):
+            findings = lint_manifest(self.manifest([self.task(check=check)]))
+            self.assertFalse(
+                [f for f in findings if "not valid shell" in f],
+                f"valid check wrongly flagged as unparseable: {check}\nfindings: {findings}",
+            )
+
     def test_w1_unverifiable_check(self) -> None:
         manifest = self.manifest([self.task(check="echo ok && echo done")])
         self.assertHasFinding(