From 41fe558b1476b269c1b2c7a16f25f5231f40b19e Mon Sep 17 00:00:00 2001 From: Kevin Simback <16635828+ksimback@users.noreply.github.com> Date: Tue, 7 Jul 2026 13:46:28 -0700 Subject: [PATCH 1/2] Add `looper lint`: the design rubrics as a static checker New subcommand checks any compilable loop.yaml for the rubric anti-patterns without running the wizard. Two severities: errors for specs that will not behave as written (judge criteria on a fixed_passes gate are never evaluated by the runner; a cross-vendor council member with no privacy.egress declaration; an egress entry naming an unknown member) and warnings for design coaching (all-vibe verification, same-family judge+host, judgment-only delivery gate, undeclared non-local sends, pre-granted consent, no checkpoint before cross-vendor sends, missing revision caps, null wall-clock budget, empty stop conditions, shell-string checks, unresolved placeholders). Exit contract: 0 clean or warnings-only, 1 on errors (or any finding with --strict), 2 when the spec does not compile. --json for CI. The wizard now runs lint after compile (errors block, warnings are relayed as coaching). 10 new tests (37 total) including a sweep asserting the five shipped templates and the example lint error-free. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 22 ++++ README.md | 17 ++++ SKILL.md | 7 ++ scripts/looper.py | 236 +++++++++++++++++++++++++++++++++++++++++++ tests/test_looper.py | 154 ++++++++++++++++++++++++++++ 5 files changed, 436 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea7fa7f..23c1996 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,28 @@ All notable changes to Looper are documented here. Versions follow [Semantic Versioning](https://semver.org/); the loop spec format is versioned separately via `version:` in `loop.yaml` (currently `1`). +## Unreleased + +### Added — `looper lint` +- `looper.py lint ` — the design rubrics as a static checker, no + wizard required. Compiles the spec first (compile rejections exit 2), then + reports findings in two severities: **errors** for specs that will not + behave the way they read at runtime (`judge-criterion-unreachable` — judge + criteria on a `fixed_passes` gate are never evaluated; `unscoped-egress` — + a cross-vendor member with no `privacy.egress` declaration; + `egress-unknown-member` — an egress entry naming nobody) and **warnings** + for rubric coaching (`all-vibe-verification`, `same-family-judge`, + `delivery-gate-no-programmatic`, `non-local-member-without-egress`, + `egress-consent-pregranted`, `cross-vendor-send-without-checkpoint`, + `missing-max-revisions`, `no-wall-clock-cap`, `no-stop-conditions`, + `shell-string-check`, `unresolved-placeholders`). Exit 1 on errors, or on + any finding with `--strict`; `--json` emits machine-readable findings for + CI. +- The wizard now runs `lint` after every compile and treats errors as + blockers, warnings as coaching to relay (SKILL.md step 10). +- 10 new tests (37 total), including a sweep asserting all five shipped + templates and the example lint with zero errors. + ## 0.3.0 — 2026-07-05 ### Added — loop pattern library diff --git a/README.md b/README.md index 875cdf1..18509c8 100644 --- a/README.md +++ b/README.md @@ -201,6 +201,23 @@ what's installed, and still runs its full critique, privacy, and preview flow before emitting. See [`templates/loops/`](templates/loops/) for the catalog and per-template docs — including how to add your own. +### Lint any loop.yaml + +The design rubrics also exist as a static checker — no wizard, no interview: + +```bash +python scripts/looper.py lint path/to/loop.yaml +``` + +`lint` compiles the spec, then checks it for the anti-patterns the rubrics +coach against: all-vibe verification (no programmatic checks), judge criteria +a runner would never evaluate, undeclared cross-vendor egress, a judge that +shares the host's model family, missing caps, and unresolved `{{PLACEHOLDER}}` +tokens. **Errors** mean the spec won't behave the way it reads (exit 1); +**warnings** are design coaching (exit 0, or exit 1 with `--strict`). Add +`--json` for tooling, and wire `lint --strict` into CI to gate loop specs in +PRs the same way you lint code. + ### Easy: run in the same session The default path is to let Looper continue in the same conversation. It follows diff --git a/SKILL.md b/SKILL.md index f2ae295..4219e60 100644 --- a/SKILL.md +++ b/SKILL.md @@ -66,6 +66,11 @@ advanced external runner. 10. After writing `loop.yaml`, resolve the helper Python (see Helper Python below) and run: `"$LOOPER_PYTHON" ${CLAUDE_SKILL_DIR}/scripts/looper.py compile /loop.yaml --out /loop.resolved.json --render /LOOP.md --session-prompt /RUN_IN_SESSION.md` + Then run + `"$LOOPER_PYTHON" ${CLAUDE_SKILL_DIR}/scripts/looper.py lint /loop.yaml` + and relay the findings: fix any `error[...]` before continuing (the spec + would not behave as written), and surface `warning[...]` lines to the user + as design coaching they may accept or address. 11. Ask whether the user wants to run the loop now in this session. If yes, follow `RUN_IN_SESSION.md` directly as the active task. If no, explain that the same file is the easy restart path and `run-loop.py` is available for @@ -148,6 +153,8 @@ same shell invocation: `"$LOOPER_PYTHON" ${CLAUDE_SKILL_DIR}/scripts/looper.py register-model --invoke " [args...]"` - Compile and render: `"$LOOPER_PYTHON" ${CLAUDE_SKILL_DIR}/scripts/looper.py compile /loop.yaml --out /loop.resolved.json --render /LOOP.md --session-prompt /RUN_IN_SESSION.md` +- Lint against the design-rubric anti-patterns (add `--strict` to fail on warnings, `--json` for tooling): + `"$LOOPER_PYTHON" ${CLAUDE_SKILL_DIR}/scripts/looper.py lint /loop.yaml` - Render only the in-session handoff: `"$LOOPER_PYTHON" ${CLAUDE_SKILL_DIR}/scripts/looper.py session-prompt /loop.resolved.json --out /RUN_IN_SESSION.md` diff --git a/scripts/looper.py b/scripts/looper.py index fea2edb..acff008 100644 --- a/scripts/looper.py +++ b/scripts/looper.py @@ -9,6 +9,7 @@ from __future__ import annotations import argparse +import copy import datetime as _dt import json import os @@ -517,6 +518,193 @@ def normalize_spec(spec: dict[str, Any], source_path: Path) -> dict[str, Any]: return to_jsonable(resolved) +# --------------------------------------------------------------------------- +# Lint: the design rubrics as a static checker. +# +# Severity contract: +# error - the spec will not behave the way it reads at runtime. +# warning - rubric coaching; the loop runs, but the design has a known trap. +# Compile validation always runs first, so anything normalize_spec rejects +# (reviewer verdict_source, missing caps enforced there, bad paths) surfaces +# as `looper: error:` before lint findings are evaluated. + +LINT_ERROR = "error" +LINT_WARNING = "warning" + + +def lint_spec(raw_spec: dict[str, Any], resolved: dict[str, Any]) -> list[dict[str, str]]: + findings: list[dict[str, str]] = [] + + def add(check: str, severity: str, message: str) -> None: + findings.append({"check": check, "severity": severity, "message": message}) + + criteria = resolved.get("criteria_by_id", {}) + members = resolved.get("council_by_id", {}) + host_cli = resolved.get("host", {}).get("cli", "") + gates = resolved.get("gates", {}) + control = resolved.get("loop_control", {}) + egress = resolved.get("privacy", {}).get("egress", []) + egress_targets = {entry.get("to") for entry in egress if isinstance(entry, dict)} + + programmatic_ids = { + cid for cid, item in criteria.items() if item.get("type") == "programmatic" + } + if not programmatic_ids: + add( + "all-vibe-verification", + LINT_WARNING, + "every verification criterion is judge or human ('all-vibe'); add at least " + "one programmatic check so something objective can block the loop " + "(verification rubric)", + ) + + raw_gates = raw_spec.get("gates") or {} + for gate_name in ("plan_gate", "delivery_gate"): + gate = gates.get(gate_name, {}) + policy = gate.get("verdict_policy") + gate_criteria = gate.get("criteria", []) or [] + + if policy == "fixed_passes": + dead = [ + cid for cid in gate_criteria if criteria.get(cid, {}).get("type") == "judge" + ] + if dead: + add( + "judge-criterion-unreachable", + LINT_ERROR, + f"{gate_name} uses fixed_passes but lists judge criteria " + f"({', '.join(dead)}); runners only consult a judge under " + "revise_until_clean, so these rubrics are never evaluated", + ) + + source = gate.get("verdict_source") + if policy == "revise_until_clean" and source and source != "human": + source_cli = members.get(source, {}).get("cli", "") + if source_cli and source_cli == host_cli: + add( + "same-family-judge", + LINT_WARNING, + f"{gate_name} verdict_source {source!r} runs on the same CLI family " + f"as the host ({host_cli}); a different model family closes " + "self-grading blind spots (council rubric)", + ) + + raw_gate = raw_gates.get(gate_name) or {} + if isinstance(raw_gate, dict) and "max_revisions" not in raw_gate: + add( + "missing-max-revisions", + LINT_WARNING, + f"{gate_name} does not set max_revisions; make the revision cap " + "explicit (control rubric requires one per gate)", + ) + + if programmatic_ids: + delivery_criteria = gates.get("delivery_gate", {}).get("criteria", []) or [] + if not any(cid in programmatic_ids for cid in delivery_criteria): + add( + "delivery-gate-no-programmatic", + LINT_WARNING, + "delivery_gate applies no programmatic criteria even though some are " + "defined; the shipped artifact is gated by judgment alone", + ) + + for member_id, member in members.items(): + if member.get("local"): + continue + if member_id in egress_targets: + continue + if member.get("cli") != host_cli: + add( + "unscoped-egress", + LINT_ERROR, + f"council member {member_id!r} is non-local and cross-vendor " + f"({member.get('cli')}) but has no privacy.egress entry; declare what " + "it receives, the redaction globs, and consent (privacy contract)", + ) + else: + add( + "non-local-member-without-egress", + LINT_WARNING, + f"council member {member_id!r} is non-local but has no privacy.egress " + "entry; project content leaves this machine undeclared - name the " + "sends and redactions even for a same-vendor CLI", + ) + + for entry in egress: + if not isinstance(entry, dict): + continue + target = entry.get("to") + if target not in members: + add( + "egress-unknown-member", + LINT_ERROR, + f"privacy.egress entry targets unknown council member {target!r}; its " + "redactions and consent will never apply to anyone", + ) + if entry.get("consent") == "granted": + add( + "egress-consent-pregranted", + LINT_WARNING, + f"privacy.egress to {target!r} pre-grants consent; the runner will " + "skip the first-send prompt for it", + ) + + has_cross_vendor = any( + not member.get("local") and member.get("cli") != host_cli + for member in members.values() + ) + if has_cross_vendor and not (control.get("human_checkpoints") or []): + add( + "cross-vendor-send-without-checkpoint", + LINT_WARNING, + "a cross-vendor council member is configured but " + "loop_control.human_checkpoints is empty; add a checkpoint before the " + "first cross-vendor send (control rubric)", + ) + + budget = control.get("budget", {}) or {} + if budget.get("wall_clock_min") is None: + add( + "no-wall-clock-cap", + LINT_WARNING, + "loop_control.budget.wall_clock_min is null; the runner will not enforce " + "any wall-clock cap", + ) + + if not (control.get("stop_conditions") or []): + add( + "no-stop-conditions", + LINT_WARNING, + "loop_control.stop_conditions is empty; state the success stop and the " + "failure/no-progress stops explicitly (control rubric)", + ) + + for item in (raw_spec.get("goal") or {}).get("verification") or []: + if ( + isinstance(item, dict) + and item.get("type") == "programmatic" + and isinstance(item.get("check"), str) + ): + add( + "shell-string-check", + LINT_WARNING, + f"criterion {item.get('id')!r} writes its check as a shell string; " + "prefer an argv array (verification rubric) unless a placeholder must " + "be shlex-split after substitution", + ) + + placeholders = sorted(find_placeholders(resolved)) + if placeholders: + add( + "unresolved-placeholders", + LINT_WARNING, + f"unresolved template placeholders remain ({', '.join(placeholders)}); " + "fill them in before running this loop", + ) + + return findings + + def clip(text: Any, width: int) -> str: value = str(text or "") return value if len(value) <= width else value[: width - 1] + "~" @@ -872,6 +1060,37 @@ def cmd_compile(args: argparse.Namespace) -> int: return 0 +def cmd_lint(args: argparse.Namespace) -> int: + source = args.loop_yaml.resolve() + raw = load_yaml(source) + raw_copy = copy.deepcopy(raw) + resolved = normalize_spec(raw, source) + findings = lint_spec(raw_copy, resolved) + errors = sum(1 for item in findings if item["severity"] == LINT_ERROR) + warnings = len(findings) - errors + + if args.json: + print( + json.dumps( + {"findings": findings, "errors": errors, "warnings": warnings}, + indent=2, + sort_keys=True, + ) + ) + else: + name = args.loop_yaml + for item in findings: + print(f"{name}: {item['severity']}[{item['check']}]: {item['message']}") + if findings: + print(f"looper lint: {errors} error(s), {warnings} warning(s)") + else: + print("looper lint: no findings") + + if errors or (args.strict and findings): + return 1 + return 0 + + def cmd_session_prompt(args: argparse.Namespace) -> int: resolved = load_json(args.resolved_json) prompt = render_session_prompt(resolved) @@ -909,6 +1128,23 @@ def build_parser() -> argparse.ArgumentParser: compile_cmd.add_argument("--session-prompt", type=Path) compile_cmd.set_defaults(func=cmd_compile) + lint = sub.add_parser( + "lint", + help="Check a loop.yaml against the design-rubric anti-patterns", + description=( + "Static anti-pattern checks over a compilable loop.yaml. " + "Errors mean the spec will not behave the way it reads at runtime; " + "warnings are rubric coaching. Exit 1 on errors (or on any finding " + "with --strict), 2 if the spec does not compile." + ), + ) + lint.add_argument("loop_yaml", type=Path) + lint.add_argument("--json", action="store_true", help="Emit findings as JSON") + lint.add_argument( + "--strict", action="store_true", help="Treat warnings as failures (exit 1)" + ) + lint.set_defaults(func=cmd_lint) + session_prompt = sub.add_parser( "session-prompt", help="Render the in-session execution prompt from loop.resolved.json" ) diff --git a/tests/test_looper.py b/tests/test_looper.py index c524d70..72e7698 100644 --- a/tests/test_looper.py +++ b/tests/test_looper.py @@ -671,6 +671,160 @@ def test_compile_placeholder_warning_absent_after_substitution(self) -> None: self.assertNotIn("unresolved template placeholders", filled.stderr) +class LintTests(unittest.TestCase): + def _setup_work(self, tmp: str) -> Path: + work = Path(tmp) + (work / "inputs").mkdir() + (work / "inputs" / "process-notes.md").write_text("Need a useful loop.\n", encoding="utf-8") + write_loop_yaml(work / "loop.yaml") + return work + + def _lint(self, work: Path, *flags: str) -> subprocess.CompletedProcess[str]: + return run_cmd([sys.executable, str(LOOPER), "lint", "loop.yaml", *flags], work) + + def _mutate(self, work: Path, old: str, new: str, count: int = -1) -> None: + text = (work / "loop.yaml").read_text(encoding="utf-8") + mutated = text.replace(old, new) if count < 0 else text.replace(old, new, count) + self.assertNotEqual(mutated, text, f"mutation did not apply: {old!r}") + (work / "loop.yaml").write_text(mutated, encoding="utf-8") + + def test_clean_fixture_loop_has_no_findings(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + work = self._setup_work(tmp) + result = self._lint(work) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("no findings", result.stdout) + + def test_all_vibe_verification_warns_and_fails_strict(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + work = self._setup_work(tmp) + lines = [] + for line in (work / "loop.yaml").read_text(encoding="utf-8").splitlines(): + if "check: [" in line or "expect: exit_zero" in line: + continue + if line.strip() == "type: programmatic": + lines.append(line.replace("programmatic", "judge")) + lines.append(line.replace("type: programmatic", "rubric: The artifact names an owner.")) + continue + lines.append(line) + (work / "loop.yaml").write_text("\n".join(lines) + "\n", encoding="utf-8") + + result = self._lint(work) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("warning[all-vibe-verification]", result.stdout) + + strict = self._lint(work, "--strict") + self.assertEqual(strict.returncode, 1, strict.stdout + strict.stderr) + + def test_judge_criterion_under_fixed_passes_is_error(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + work = self._setup_work(tmp) + self._mutate(work, "verdict_policy: revise_until_clean", "verdict_policy: fixed_passes") + self._mutate(work, "verdict_source: reviewer-1\n", "") + self._mutate(work, " criteria:", " criteria:") + result = self._lint(work) + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("error[judge-criterion-unreachable]", result.stdout) + + def test_cross_vendor_member_without_egress_is_error_and_scoped_egress_clears_it(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + work = Path(tmp) + (work / "inputs").mkdir() + (work / "inputs" / "process-notes.md").write_text("Notes.\n", encoding="utf-8") + write_loop_yaml(work / "loop.yaml", judge_local=False) + + result = self._lint(work) + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("error[unscoped-egress]", result.stdout) + + self._mutate( + work, + "privacy:\n egress: []", + "privacy:\n egress:\n - to: reviewer-1\n sends: [plan, deliveries]\n" + " redact: [\"inputs/**\"]\n consent: required", + ) + scoped = self._lint(work) + self.assertEqual(scoped.returncode, 0, scoped.stdout + scoped.stderr) + self.assertNotIn("unscoped-egress", scoped.stdout) + # Cross-vendor member with no human checkpoint is still coached. + self.assertIn("warning[cross-vendor-send-without-checkpoint]", scoped.stdout) + + self._mutate(work, "consent: required", "consent: granted") + granted = self._lint(work) + self.assertEqual(granted.returncode, 0, granted.stdout + granted.stderr) + self.assertIn("warning[egress-consent-pregranted]", granted.stdout) + + def test_non_local_same_family_member_without_egress_warns(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + work = Path(tmp) + (work / "inputs").mkdir() + (work / "inputs" / "process-notes.md").write_text("Notes.\n", encoding="utf-8") + write_loop_yaml(work / "loop.yaml", judge_local=False) + self._mutate(work, "cli: fake-judge", "cli: fake-host") + result = self._lint(work) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("warning[non-local-member-without-egress]", result.stdout) + self.assertIn("warning[same-family-judge]", result.stdout) + + def test_egress_to_unknown_member_is_error(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + work = self._setup_work(tmp) + self._mutate( + work, + "privacy:\n egress: []", + "privacy:\n egress:\n - to: ghost\n sends: [plan]\n consent: required", + ) + result = self._lint(work) + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("error[egress-unknown-member]", result.stdout) + + def test_coaching_warnings_for_caps_and_gating(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + work = self._setup_work(tmp) + self._mutate(work, "wall_clock_min: 5", "wall_clock_min: null") + self._mutate(work, " max_revisions: 2\n", "", 1) + self._mutate(work, "criteria: [has-owner, covers-goal]", "criteria: [covers-goal]") + result = self._lint(work) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("warning[no-wall-clock-cap]", result.stdout) + self.assertIn("warning[missing-max-revisions]", result.stdout) + self.assertIn("warning[delivery-gate-no-programmatic]", result.stdout) + + def test_json_output_matches_counts(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + work = Path(tmp) + (work / "inputs").mkdir() + (work / "inputs" / "process-notes.md").write_text("Notes.\n", encoding="utf-8") + write_loop_yaml(work / "loop.yaml", judge_local=False) + result = self._lint(work, "--json") + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + payload = json.loads(result.stdout) + self.assertEqual(payload["errors"], 1) + self.assertEqual( + payload["errors"] + payload["warnings"], len(payload["findings"]) + ) + checks = {item["check"] for item in payload["findings"]} + self.assertIn("unscoped-egress", checks) + + def test_lint_rejects_uncompilable_spec_with_exit_2(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + work = self._setup_work(tmp) + self._mutate(work, "dir: ./loop-workspace", "dir: ../escape") + result = self._lint(work) + self.assertEqual(result.returncode, 2, result.stdout + result.stderr) + self.assertIn("looper: error:", result.stderr) + + def test_shipped_templates_and_example_lint_without_errors(self) -> None: + specs = sorted((ROOT / "templates" / "loops").glob("*/loop.yaml")) + specs.append(ROOT / "examples" / "ai-workflow-mapping" / "loop.yaml") + self.assertGreaterEqual(len(specs), 6) + for spec in specs: + with self.subTest(spec.parent.name): + result = run_cmd([sys.executable, str(LOOPER), "lint", str(spec)], ROOT) + self.assertEqual(result.returncode, 0, f"{spec}: {result.stdout}{result.stderr}") + self.assertNotIn("error[", result.stdout, spec) + + LOOPS = ROOT / "templates" / "loops" From 2e336bef62c641a9d16e0b3f54f4e6190b3b063f Mon Sep 17 00:00:00 2001 From: Kevin Simback <16635828+ksimback@users.noreply.github.com> Date: Tue, 7 Jul 2026 14:05:03 -0700 Subject: [PATCH 2/2] Fix lint defects found in high-effort review Correctness: - judge-criterion-unreachable now also fires for revise_until_clean gates with verdict_source: human (the runner asks for a bare pass/revise and never surfaces judge rubrics there either). - unscoped-egress only applies to members actually referenced by a gate''s members or verdict_source; unreferenced members get a new unreferenced-council-member warning instead of a false-positive error. - egress-consent-pregranted groups entries per member and fires only when a non-local member''s entries are ALL consent: granted, matching the runner''s all() check; local members never prompt so never warn. - egress-unknown-member message no longer claims the entry''s redact globs are inert (all_redaction_globs folds them into context redaction regardless of target). - Removed cross-vendor-send-without-checkpoint: it was satisfied by checkpoint names the runner never fires, and consent is already the fail-closed before-first-send human gate. Replaced with unhonored-human-checkpoint, which warns when human_checkpoints entries other than after_plan will only bind in-session. Cleanup: - Empty goal.verification gets a dedicated no-verification-criteria message instead of a factually wrong all-vibe diagnostic. - missing-max-revisions reads the resolved gate (normalize_spec never writes the default back), dropping the redundant raw-gates plumbing. - Shared placeholder_warning_text() so compile and lint cannot drift. - Load-bearing deepcopy in cmd_lint is now explained. - README documents exit 2 and that --json emits nothing on it. 4 new tests (41 total). Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 11 ++-- README.md | 8 ++- scripts/looper.py | 135 ++++++++++++++++++++++++++++++------------- tests/test_looper.py | 77 +++++++++++++++++++++++- 4 files changed, 182 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23c1996..148f66b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,15 +12,16 @@ separately via `version:` in `loop.yaml` (currently `1`). reports findings in two severities: **errors** for specs that will not behave the way they read at runtime (`judge-criterion-unreachable` — judge criteria on a `fixed_passes` gate are never evaluated; `unscoped-egress` — - a cross-vendor member with no `privacy.egress` declaration; + a gate-referenced cross-vendor member with no `privacy.egress` declaration; `egress-unknown-member` — an egress entry naming nobody) and **warnings** - for rubric coaching (`all-vibe-verification`, `same-family-judge`, - `delivery-gate-no-programmatic`, `non-local-member-without-egress`, - `egress-consent-pregranted`, `cross-vendor-send-without-checkpoint`, + for rubric coaching (`all-vibe-verification`, `no-verification-criteria`, + `same-family-judge`, `delivery-gate-no-programmatic`, + `non-local-member-without-egress`, `egress-consent-pregranted`, + `unreferenced-council-member`, `unhonored-human-checkpoint`, `missing-max-revisions`, `no-wall-clock-cap`, `no-stop-conditions`, `shell-string-check`, `unresolved-placeholders`). Exit 1 on errors, or on any finding with `--strict`; `--json` emits machine-readable findings for - CI. + CI (exit 2 compile failures print to stderr, no JSON). - The wizard now runs `lint` after every compile and treats errors as blockers, warnings as coaching to relay (SKILL.md step 10). - 10 new tests (37 total), including a sweep asserting all five shipped diff --git a/README.md b/README.md index 18509c8..9499915 100644 --- a/README.md +++ b/README.md @@ -214,9 +214,11 @@ coach against: all-vibe verification (no programmatic checks), judge criteria a runner would never evaluate, undeclared cross-vendor egress, a judge that shares the host's model family, missing caps, and unresolved `{{PLACEHOLDER}}` tokens. **Errors** mean the spec won't behave the way it reads (exit 1); -**warnings** are design coaching (exit 0, or exit 1 with `--strict`). Add -`--json` for tooling, and wire `lint --strict` into CI to gate loop specs in -PRs the same way you lint code. +**warnings** are design coaching (exit 0, or exit 1 with `--strict`). A spec +that doesn't compile exits 2 with the compile error on stderr — in that case +`--json` emits nothing, so CI wrappers should check the exit code before +parsing. Add `--json` for tooling, and wire `lint --strict` into CI to gate +loop specs in PRs the same way you lint code. ### Easy: run in the same session diff --git a/scripts/looper.py b/scripts/looper.py index acff008..d7a253c 100644 --- a/scripts/looper.py +++ b/scripts/looper.py @@ -42,6 +42,14 @@ PLACEHOLDER_PATTERN = re.compile(r"\{\{[A-Za-z0-9_]+\}\}") +def placeholder_warning_text(placeholders: list[str]) -> str: + # Shared by cmd_compile and lint so the two surfaces never drift. + return ( + "unresolved template placeholders remain " + f"({', '.join(placeholders)}); fill them in before running this loop" + ) + + def find_placeholders(value: Any) -> set[str]: found: set[str] = set() if isinstance(value, str): @@ -549,7 +557,14 @@ def add(check: str, severity: str, message: str) -> None: programmatic_ids = { cid for cid, item in criteria.items() if item.get("type") == "programmatic" } - if not programmatic_ids: + if not criteria: + add( + "no-verification-criteria", + LINT_WARNING, + "goal.verification is empty; nothing checkable defines done - add " + "programmatic checks first, then judge rubrics (verification rubric)", + ) + elif not programmatic_ids: add( "all-vibe-verification", LINT_WARNING, @@ -558,26 +573,36 @@ def add(check: str, severity: str, message: str) -> None: "(verification rubric)", ) - raw_gates = raw_spec.get("gates") or {} for gate_name in ("plan_gate", "delivery_gate"): gate = gates.get(gate_name, {}) policy = gate.get("verdict_policy") + source = gate.get("verdict_source") gate_criteria = gate.get("criteria", []) or [] - if policy == "fixed_passes": - dead = [ - cid for cid in gate_criteria if criteria.get(cid, {}).get("type") == "judge" - ] - if dead: - add( - "judge-criterion-unreachable", - LINT_ERROR, - f"{gate_name} uses fixed_passes but lists judge criteria " - f"({', '.join(dead)}); runners only consult a judge under " - "revise_until_clean, so these rubrics are never evaluated", - ) + # Judge criteria are only ever evaluated when a judge member is the + # verdict source: under fixed_passes no verdict runs at all, and under + # revise_until_clean with a human source the runner asks for a bare + # pass/revise without ever surfacing the rubric. + dead = [ + cid for cid in gate_criteria if criteria.get(cid, {}).get("type") == "judge" + ] + if dead and policy == "fixed_passes": + add( + "judge-criterion-unreachable", + LINT_ERROR, + f"{gate_name} uses fixed_passes but lists judge criteria " + f"({', '.join(dead)}); runners only consult a judge under " + "revise_until_clean, so these rubrics are never evaluated", + ) + elif dead and policy == "revise_until_clean" and source == "human": + add( + "judge-criterion-unreachable", + LINT_ERROR, + f"{gate_name} lists judge criteria ({', '.join(dead)}) but its " + "verdict_source is human; the runner asks the human for a bare " + "pass/revise and never shows or evaluates these rubrics", + ) - source = gate.get("verdict_source") if policy == "revise_until_clean" and source and source != "human": source_cli = members.get(source, {}).get("cli", "") if source_cli and source_cli == host_cli: @@ -589,8 +614,9 @@ def add(check: str, severity: str, message: str) -> None: "self-grading blind spots (council rubric)", ) - raw_gate = raw_gates.get(gate_name) or {} - if isinstance(raw_gate, dict) and "max_revisions" not in raw_gate: + # normalize_spec validates max_revisions but never writes a default + # back, so its absence is visible in the resolved gate too. + if "max_revisions" not in gate: add( "missing-max-revisions", LINT_WARNING, @@ -608,7 +634,25 @@ def add(check: str, severity: str, message: str) -> None: "defined; the shipped artifact is gated by judgment alone", ) + # Only members a gate can actually invoke ever receive project content: + # run_reviewers iterates gate.members, run_judge only the verdict_source. + referenced: set = set() + for gate_name in ("plan_gate", "delivery_gate"): + gate = gates.get(gate_name, {}) + referenced.update(gate.get("members", []) or []) + source = gate.get("verdict_source") + if source and source != "human": + referenced.add(source) + for member_id, member in members.items(): + if member_id not in referenced: + add( + "unreferenced-council-member", + LINT_WARNING, + f"council member {member_id!r} is not listed in any gate's members " + "or verdict_source; the runner will never invoke it (dead config)", + ) + continue if member.get("local"): continue if member_id in egress_targets: @@ -630,36 +674,51 @@ def add(check: str, severity: str, message: str) -> None: "sends and redactions even for a same-vendor CLI", ) + entries_by_target: dict[str, list[dict[str, Any]]] = {} for entry in egress: if not isinstance(entry, dict): continue target = entry.get("to") + entries_by_target.setdefault(str(target), []).append(entry) if target not in members: add( "egress-unknown-member", LINT_ERROR, - f"privacy.egress entry targets unknown council member {target!r}; its " - "redactions and consent will never apply to anyone", + f"privacy.egress entry targets unknown council member {target!r} " + "(renamed or mistyped id?); its sends/consent scoping binds to no " + "member, though its redact globs still extend context redaction", ) - if entry.get("consent") == "granted": + + # The runner skips the first-send consent prompt only when every egress + # entry for a non-local member pre-grants consent; local members never + # prompt at all. + for member_id, entries in entries_by_target.items(): + member = members.get(member_id) + if member is None or member.get("local"): + continue + if all(entry.get("consent") == "granted" for entry in entries): add( "egress-consent-pregranted", LINT_WARNING, - f"privacy.egress to {target!r} pre-grants consent; the runner will " - "skip the first-send prompt for it", + f"every privacy.egress entry for {member_id!r} pre-grants consent; " + "the runner will skip the first-send prompt for it", ) - has_cross_vendor = any( - not member.get("local") and member.get("cli") != host_cli - for member in members.values() - ) - if has_cross_vendor and not (control.get("human_checkpoints") or []): + # run-loop.py recognizes only the literal checkpoint name "after_plan"; + # anything else in human_checkpoints is honored solely by an in-session + # operator reading the prompt. + unhonored = [ + name + for name in (control.get("human_checkpoints") or []) + if isinstance(name, str) and name != "after_plan" + ] + if unhonored: add( - "cross-vendor-send-without-checkpoint", + "unhonored-human-checkpoint", LINT_WARNING, - "a cross-vendor council member is configured but " - "loop_control.human_checkpoints is empty; add a checkpoint before the " - "first cross-vendor send (control rubric)", + f"human_checkpoints entries {unhonored!r} are not recognized by the " + "external runner (run-loop.py fires only 'after_plan'); they bind only " + "when an in-session operator runs the loop", ) budget = control.get("budget", {}) or {} @@ -695,12 +754,7 @@ def add(check: str, severity: str, message: str) -> None: placeholders = sorted(find_placeholders(resolved)) if placeholders: - add( - "unresolved-placeholders", - LINT_WARNING, - f"unresolved template placeholders remain ({', '.join(placeholders)}); " - "fill them in before running this loop", - ) + add("unresolved-placeholders", LINT_WARNING, placeholder_warning_text(placeholders)) return findings @@ -1042,8 +1096,7 @@ def cmd_compile(args: argparse.Namespace) -> int: placeholders = sorted(find_placeholders(resolved)) if placeholders: print( - "looper: warning: unresolved template placeholders remain " - f"({', '.join(placeholders)}); fill them in before running this loop", + f"looper: warning: {placeholder_warning_text(placeholders)}", file=sys.stderr, ) out = args.out or source.with_name("loop.resolved.json") @@ -1063,6 +1116,10 @@ def cmd_compile(args: argparse.Namespace) -> int: def cmd_lint(args: argparse.Namespace) -> int: source = args.loop_yaml.resolve() raw = load_yaml(source) + # normalize_spec mutates its input in place (e.g. normalize_argv rewrites + # string `check:` values into argv lists), so lint checks that need the + # pre-normalization shape (shell-string-check) must see a deep copy taken + # before compiling. raw_copy = copy.deepcopy(raw) resolved = normalize_spec(raw, source) findings = lint_spec(raw_copy, resolved) diff --git a/tests/test_looper.py b/tests/test_looper.py index 72e7698..2c2f651 100644 --- a/tests/test_looper.py +++ b/tests/test_looper.py @@ -746,14 +746,24 @@ def test_cross_vendor_member_without_egress_is_error_and_scoped_egress_clears_it scoped = self._lint(work) self.assertEqual(scoped.returncode, 0, scoped.stdout + scoped.stderr) self.assertNotIn("unscoped-egress", scoped.stdout) - # Cross-vendor member with no human checkpoint is still coached. - self.assertIn("warning[cross-vendor-send-without-checkpoint]", scoped.stdout) self._mutate(work, "consent: required", "consent: granted") granted = self._lint(work) self.assertEqual(granted.returncode, 0, granted.stdout + granted.stderr) self.assertIn("warning[egress-consent-pregranted]", granted.stdout) + # A second entry for the same member that still requires consent + # means the runner will prompt after all; no pre-grant warning. + self._mutate( + work, + " consent: granted", + " consent: granted\n - to: reviewer-1\n sends: [reviews]\n" + " redact: [\"inputs/**\"]\n consent: required", + ) + mixed = self._lint(work) + self.assertEqual(mixed.returncode, 0, mixed.stdout + mixed.stderr) + self.assertNotIn("egress-consent-pregranted", mixed.stdout) + def test_non_local_same_family_member_without_egress_warns(self) -> None: with tempfile.TemporaryDirectory() as tmp: work = Path(tmp) @@ -766,6 +776,69 @@ def test_non_local_same_family_member_without_egress_warns(self) -> None: self.assertIn("warning[non-local-member-without-egress]", result.stdout) self.assertIn("warning[same-family-judge]", result.stdout) + def test_judge_criterion_under_human_verdict_source_is_error(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + work = self._setup_work(tmp) + self._mutate(work, "verdict_source: reviewer-1", "verdict_source: human") + result = self._lint(work) + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("error[judge-criterion-unreachable]", result.stdout) + self.assertIn("verdict_source is human", result.stdout) + + def test_unreferenced_cross_vendor_member_warns_instead_of_erroring(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + work = Path(tmp) + (work / "inputs").mkdir() + (work / "inputs" / "process-notes.md").write_text("Notes.\n", encoding="utf-8") + write_loop_yaml(work / "loop.yaml", judge_local=False) + # Detach the member from both gates: human verdicts, no members, + # and no judge criteria left anywhere. + self._mutate(work, "verdict_source: reviewer-1", "verdict_source: human") + self._mutate(work, "members: [reviewer-1]", "members: []") + self._mutate(work, "criteria: [covers-goal]", "criteria: []") + self._mutate(work, "criteria: [has-owner, covers-goal]", "criteria: [has-owner]") + result = self._lint(work) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertNotIn("unscoped-egress", result.stdout) + self.assertIn("warning[unreferenced-council-member]", result.stdout) + + def test_unhonored_human_checkpoint_warns(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + work = self._setup_work(tmp) + self._mutate( + work, + "human_checkpoints: []", + "human_checkpoints: [after_plan, before cross-vendor send]", + ) + result = self._lint(work) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("warning[unhonored-human-checkpoint]", result.stdout) + # Only the unrecognized name is listed; 'after_plan' is honored. + self.assertIn("['before cross-vendor send']", result.stdout) + + def test_empty_verification_gets_dedicated_warning(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + work = self._setup_work(tmp) + lines = [] + skipping = False + for line in (work / "loop.yaml").read_text(encoding="utf-8").splitlines(): + if line.strip() == "verification:": + lines.append(line.replace("verification:", "verification: []")) + skipping = True + continue + if skipping and (line.startswith(" ") or not line.strip()): + continue + skipping = False + lines.append(line) + text = "\n".join(lines) + "\n" + text = text.replace("criteria: [covers-goal]", "criteria: []") + text = text.replace("criteria: [has-owner, covers-goal]", "criteria: []") + (work / "loop.yaml").write_text(text, encoding="utf-8") + result = self._lint(work) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("warning[no-verification-criteria]", result.stdout) + self.assertNotIn("all-vibe-verification", result.stdout) + def test_egress_to_unknown_member_is_error(self) -> None: with tempfile.TemporaryDirectory() as tmp: work = self._setup_work(tmp)