skill(reflect): do not apply findings from a fan-out that never returned

engine/skills/reflect/SKILL.md

@@ -105,6 +105,8 @@ One more `Agent` call, given all reviewers' output, merges overlapping findings,
 
 Present the full Accepted / Backlog / Route-to-automate-me / Rejected list to the user in the same turn.
 
+**Precondition: apply nothing on a fan-out that did not return.** Step 5 starts only when step 3's reviewers reported and step 4's synthesis, including the grounding gate, actually ran. A fan-out that died -- rate limits, a crashed lens, a killed agent -- means the pass is incomplete: re-run the missing lenses, staggered or on a second model, or say plainly that the pass is partial and name which lenses are missing. Unreturned reviewers are not reviewers that passed, and the parent doing the investigation itself is a fallback for a failed worktree spawn, not for a failed review. `scripts/fanout_complete.py --expected <lenses> --returned <lenses>` decides it: exit 0 complete, 1 incomplete with the missing lenses named, 2 unchecked when no expected set was recorded. Only exit 0 opens step 5.
+
 **When Accepted is non-empty, do not wait for a second “apply those” / “make a PR for Accepted” turn.** In that same turn, fire a **dedicated git worktree keyed to the owning repo of each Accepted item's named skill** (per Write roots above) that applies only the Accepted items (fix hierarchy: categorical / lint-test / hook before skill prose):
 
 0. Group Accepted items by owning repo. Catstack-owned skills and any working-style/personal-mode item always group under catstack — never split those into an external worktree.

engine/skills/reflect/scripts/fanout_complete.py

@@ -0,0 +1,99 @@
+#!/usr/bin/env python3
+"""Decide whether a reflect fan-out returned enough to apply its findings.
+
+Step 5 of reflect applies Accepted findings. It may start only when every
+reviewer lens from step 3 reported. A lens that died -- rate limit, crashed
+worktree, killed agent -- is not a lens that passed, so a pass missing one is
+partial and says so rather than applying what the survivors happened to find.
+
+Three outcomes, because a check that could not run is not a pass:
+
+    complete    every expected lens returned; apply is allowed
+    incomplete  at least one expected lens did not return; apply is refused
+    unchecked   the expected set is unknown, so completeness cannot be judged
+
+`unchecked` is not `complete`. It exits non-zero like `incomplete`, because a
+fan-out nobody can account for is exactly the case this exists to catch.
+
+    python3 fanout_complete.py --expected a b c --returned a b
+    python3 fanout_complete.py --manifest run.json
+    python3 fanout_complete.py --expected a b --returned a b --json
+"""
+from __future__ import annotations
+
+import argparse
+import json
+import sys
+
+COMPLETE = "complete"
+INCOMPLETE = "incomplete"
+UNCHECKED = "unchecked"
+
+EXIT = {COMPLETE: 0, INCOMPLETE: 1, UNCHECKED: 2}
+
+
+def verdict(expected, returned):
+    """Outcome plus the lenses missing from this fan-out.
+
+    An empty or absent expected set is `unchecked`: nothing pins what the pass
+    was supposed to cover, so a returned set cannot be called complete. A
+    returned lens outside the expected set does not make the pass complete and
+    is reported so a mislabelled lens is visible rather than silently counted.
+    """
+    if not expected:
+        return UNCHECKED, [], []
+    missing = sorted(set(expected) - set(returned))
+    unexpected = sorted(set(returned) - set(expected))
+    return (COMPLETE if not missing else INCOMPLETE), missing, unexpected
+
+
+def _from_manifest(path):
+    with open(path, encoding="utf-8") as handle:
+        data = json.load(handle)
+    if not isinstance(data, dict):
+        raise ValueError("manifest must be a JSON object")
+    return data.get("expected") or [], data.get("returned") or []
+
+
+def render(outcome, missing, unexpected):
+    lines = []
+    if outcome == COMPLETE:
+        lines.append("complete  every expected lens returned; apply may proceed")
+    elif outcome == INCOMPLETE:
+        lines.append(f"incomplete  {len(missing)} lens(es) did not return: {', '.join(missing)}")
+        lines.append("            re-run them, or report the pass as partial and name them.")
+    else:
+        lines.append("unchecked  no expected lens set given; completeness cannot be judged")
+        lines.append("            name the lenses step 3 launched, then re-run this.")
+    if unexpected:
+        lines.append(f"            returned but not expected: {', '.join(unexpected)}")
+    return "\n".join(lines)
+
+
+def main(argv=None):
+    ap = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter)
+    ap.add_argument("--expected", nargs="*", default=None, help="lens names step 3 launched")
+    ap.add_argument("--returned", nargs="*", default=None, help="lens names that reported")
+    ap.add_argument("--manifest", help="JSON object with 'expected' and 'returned' lists")
+    ap.add_argument("--json", action="store_true", help="machine-readable verdict")
+    args = ap.parse_args(argv)
+
+    if args.manifest:
+        expected, returned = _from_manifest(args.manifest)
+    else:
+        expected, returned = args.expected or [], args.returned or []
+
+    outcome, missing, unexpected = verdict(expected, returned)
+    if args.json:
+        print(json.dumps({
+            "outcome": outcome, "missing": missing,
+            "unexpected": unexpected, "expected": sorted(set(expected)),
+            "returned": sorted(set(returned)),
+        }, indent=2))
+    else:
+        print(render(outcome, missing, unexpected))
+    return EXIT[outcome]
+
+
+if __name__ == "__main__":
+    sys.exit(main())

engine/skills/reflect/scripts/tests/test_fanout_complete.py

@@ -0,0 +1,98 @@
+#!/usr/bin/env python3
+"""A reflect fan-out is complete only when every launched lens reported.
+
+The failure this exists to stop: a lens dies, the survivors return findings,
+and step 5 applies them as though the pass were whole. Unreturned reviewers are
+not reviewers that passed.
+
+The `unchecked` outcome is pinned as a non-pass on purpose. A fan-out whose
+expected set nobody recorded cannot be called complete, and a check that could
+not run must not read as clean.
+"""
+from __future__ import annotations
+
+import json
+import os
+import subprocess
+import sys
+import tempfile
+import unittest
+
+SCRIPTS = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+if SCRIPTS not in sys.path:
+    sys.path.insert(0, SCRIPTS)
+import fanout_complete as fc  # noqa: E402
+
+SCRIPT = os.path.join(SCRIPTS, "fanout_complete.py")
+
+
+def run(*args):
+    return subprocess.run([sys.executable, SCRIPT, *args], capture_output=True, text=True)
+
+
+class TestVerdict(unittest.TestCase):
+    def test_every_lens_returned_is_complete(self):
+        self.assertEqual(fc.verdict(["a", "b"], ["b", "a"])[0], fc.COMPLETE)
+
+    def test_a_missing_lens_is_incomplete_and_named(self):
+        outcome, missing, _ = fc.verdict(["a", "b", "c"], ["a"])
+        self.assertEqual(outcome, fc.INCOMPLETE)
+        self.assertEqual(missing, ["b", "c"])
+
+    def test_no_expected_set_is_unchecked_not_complete(self):
+        """The whole point: nothing recorded cannot mean nothing missing."""
+        self.assertEqual(fc.verdict([], ["a", "b"])[0], fc.UNCHECKED)
+        self.assertNotEqual(fc.verdict([], ["a", "b"])[0], fc.COMPLETE)
+
+    def test_a_lens_nobody_expected_does_not_fill_a_gap(self):
+        outcome, missing, unexpected = fc.verdict(["a", "b"], ["a", "zzz"])
+        self.assertEqual(outcome, fc.INCOMPLETE)
+        self.assertEqual(missing, ["b"])
+        self.assertEqual(unexpected, ["zzz"])
+
+    def test_duplicate_returns_do_not_count_twice(self):
+        self.assertEqual(fc.verdict(["a", "b"], ["a", "a"])[0], fc.INCOMPLETE)
+
+
+class TestExitCodes(unittest.TestCase):
+    """Exit codes are the contract a caller gates on, so they are pinned."""
+
+    def test_complete_exits_zero(self):
+        self.assertEqual(run("--expected", "a", "--returned", "a").returncode, 0)
+
+    def test_incomplete_exits_nonzero_and_names_the_lens(self):
+        res = run("--expected", "a", "b", "--returned", "a")
+        self.assertEqual(res.returncode, 1)
+        self.assertIn("b", res.stdout)
+
+    def test_unchecked_exits_nonzero(self):
+        """`unchecked` must not be usable as a pass by a caller reading $?."""
+        self.assertEqual(run("--returned", "a").returncode, 2)
+
+    def test_json_carries_the_outcome_and_the_missing_lenses(self):
+        res = run("--expected", "a", "b", "--returned", "a", "--json")
+        payload = json.loads(res.stdout)
+        self.assertEqual(payload["outcome"], "incomplete")
+        self.assertEqual(payload["missing"], ["b"])
+
+
+class TestManifest(unittest.TestCase):
+    def test_manifest_drives_the_same_verdict(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            path = os.path.join(tmp, "run.json")
+            with open(path, "w", encoding="utf-8") as handle:
+                json.dump({"expected": ["a", "b"], "returned": ["a"]}, handle)
+            res = run("--manifest", path)
+            self.assertEqual(res.returncode, 1)
+            self.assertIn("b", res.stdout)
+
+    def test_manifest_without_an_expected_list_is_unchecked(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            path = os.path.join(tmp, "run.json")
+            with open(path, "w", encoding="utf-8") as handle:
+                json.dump({"returned": ["a"]}, handle)
+            self.assertEqual(run("--manifest", path).returncode, 2)
+
+
+if __name__ == "__main__":
+    unittest.main()