fix(groom): bound finder and builder exploration

.github/groom/builder.md

@@ -7,6 +7,8 @@ Rules:
 2. **Keep it green.** Match the repo's conventions (read its AGENTS.md/CLAUDE.md/README). If the finding is a refactor, preserve behavior exactly. If the repo has tests for the touched area, update them; do NOT delete a test to make a change "pass".
 3. **NEVER touch security/auth-adjacent code.** Those findings are filed as investigations, never auto-built — you should not have received one, but if the finding turns out to touch auth, permissions, secrets, or a trust boundary, BAIL (see below) instead of guessing.
 4. **Patch-size bail-out.** If a faithful implementation balloons (many files, a large or risky diff, or it needs a design decision you can't make blindly), do NOT force a giant or speculative change. BAIL: it will be filed as an issue for a human instead.
+5. **Decide early.** After at most 20 inspection tool calls, make one feasibility decision before editing: implement the finding or BAIL. Only choose `patched` when the exact bounded diff is clear and confidently finishable in this run. A request to restructure a whole subsystem, combine several independent concerns, or invent a new contract belongs in an issue, not an automatic patch. Reserve enough time to write the control file (and PR body after a patch); do not spend the full run exploring.
+6. **Use the granted tools.** Prefer Read, Glob, and Grep for inspection. If a tool call is denied, do not retry that operation or seek a shell-command substitute; use a directly granted tool instead, or BAIL if the missing operation is necessary.
 
 When done, write a small control file to {{BUILDER_OUT}} — VALID JSON, EXACTLY this shape (JSON ONLY, no prose):
 {"status":"patched|bail","summary":"<one line: what you changed, or why you bailed>"}

.github/groom/finder.md

@@ -2,8 +2,10 @@ You are a one-shot agent-work GROOM FINDER on the Mac Studio — phase 1 of 2. Y
 
 Find genuine, high-value refactor opportunities a thoughtful senior engineer would actually greenlight — NOT an exhaustive lint. Dimensions: (1) genuine duplication (same non-trivial logic ~15+ lines or a clear repeated shape across >=2 sites); (2) inconsistent patterns (one concept done N ways where converging helps — list the variants); (3) missing abstractions; (4) complexity hotspots; (5) dead/vestigial code.
 
-HARD PRECISION BAR — this is the entire point: only things you'd stake your credibility on; ~6-12 findings MAX, ranked. EXPLICITLY AVOID premature abstraction (in Go especially, a little duplication beats the wrong abstraction; never DRY incidentally-similar-but-semantically-distinct code), bikeshedding, and anything linters/formatters already enforce. For EACH finding include a 'steelman-against' (the strongest reason NOT to do it) and DROP it if the steelman wins.
+Work to a bounded inspection plan. The run has an externally enforced $8 hard ceiling. After at most 60 inspection tool calls, stop exploring, write the best valid result supported by the evidence already gathered, and finish. Reserve enough time to write the result; broad repository coverage is NOT a completion requirement. If an inspection call is denied, do not retry that operation through another tool; move on using other evidence available from the allowed read-only tools.
 
-Write your result as VALID JSON to {{FINDER_OUT}}, EXACTLY this shape (JSON ONLY, no prose) — escape all string contents (quotes, backslashes, newlines):
-{"repo":"{{REPO}}","scope":"{{SCOPE_LABEL}}","findings":[{"title":"...","dimension":"...","sites":["file:line"],"evidence":"...","proposed":"...","value":"...","risk":"...","confidence":"high|med","steelman":"..."}]}
+HARD PRECISION BAR — this is the entire point: only things you'd stake your credibility on; ~6-12 findings MAX, ranked. Fewer than 6 findings is valid, including zero, when that is all the bounded inspection supports. EXPLICITLY AVOID premature abstraction (in Go especially, a little duplication beats the wrong abstraction; never DRY incidentally-similar-but-semantically-distinct code), bikeshedding, and anything linters/formatters already enforce. For EACH finding include a 'steelman-against' (the strongest reason NOT to do it) and DROP it if the steelman wins.
+
+Write your result as VALID JSON to {{FINDER_OUT}}, EXACTLY this shape (JSON ONLY, no prose) — escape all string contents (quotes, backslashes, newlines). Set `stop_reason` to `inspection-complete` when you have enough evidence to finish, or `inspection-call-limit` when the 60-call limit ended inspection:
+{"repo":"{{REPO}}","scope":"{{SCOPE_LABEL}}","stop_reason":"inspection-complete|inspection-call-limit","findings":[{"title":"...","dimension":"...","sites":["file:line"],"evidence":"...","proposed":"...","value":"...","risk":"...","confidence":"high|med","steelman":"..."}]}
 That file is the ONLY handoff to phase 2. When it's written, you may stop.

.github/groom/tests/test_builder_bounds.py

@@ -0,0 +1,45 @@
+import pathlib
+import re
+import unittest
+
+
+ROOT = pathlib.Path(__file__).resolve().parents[3]
+WORKFLOW = (ROOT / ".github/workflows/groom.yml").read_text(encoding="utf-8")
+BUILDER_BRIEF = (ROOT / ".github/groom/builder.md").read_text(encoding="utf-8")
+
+
+class TestBuilderBounds(unittest.TestCase):
+    @staticmethod
+    def builder_step():
+        match = re.search(
+            r"(?ms)^      - name: Run builder\n(?P<body>.*?)^      - name: Unlock the clone's \.git\n",
+            WORKFLOW,
+        )
+        if match is None:
+            raise AssertionError("groom.yml has no Run builder step followed by Unlock the clone's .git")
+        return match.group("body")
+
+    def test_brief_requires_a_bounded_feasibility_decision_before_edits(self):
+        self.assertIn("After at most 20 inspection tool calls", BUILDER_BRIEF)
+        self.assertIn("make one feasibility decision before editing", BUILDER_BRIEF)
+        self.assertIn("Only choose `patched` when the exact bounded diff is clear", BUILDER_BRIEF)
+        self.assertIn("Reserve enough time to write the control file", BUILDER_BRIEF)
+        self.assertIn(
+            "If a tool call is denied, do not retry that operation or seek a shell-command substitute",
+            BUILDER_BRIEF,
+        )
+
+    def test_complete_clean_bail_survives_a_late_cli_failure(self):
+        builder_step = self.builder_step()
+        self.assertIn('if [ -s "$BUILDER_OUT" ] && jq -e', builder_step)
+        self.assertIn('.status == "bail"', builder_step)
+        self.assertIn('test -z "$(git status --short)"', builder_step)
+        self.assertIn("preserving the clean bail-out for issue filing", builder_step)
+        self.assertRegex(
+            builder_step,
+            r'(?s)preserving the clean bail-out for issue filing\."\n\s+STATUS=0\n\s+else',
+        )
+
+
+if __name__ == "__main__":
+    unittest.main()

.github/groom/tests/test_claude_code_pin.py

@@ -32,6 +32,7 @@
 import re
 import shutil
 import subprocess
+import tempfile
 import time
 import unittest
 
@@ -662,6 +663,57 @@ def _fail_or_skip(self, spec, error, unavailable):
             self.fail(message)
         self.skipTest(message)
 
+    def test_finder_budget_flag_is_supported_by_pinned_cli(self):
+        """Install the exact pin and ask its real argument parser for the flag.
+
+        A textual workflow assertion cannot catch an option removed by a CLI
+        bump. This uses the same npm package, registry and install-script path as
+        groom.yml, then executes only `--help` (no auth and no API request).
+        """
+        npm = shutil.which("npm")
+        if npm is None:
+            self._fail_or_skip(self.spec, "no `npm` on PATH", True)
+        try:
+            with tempfile.TemporaryDirectory() as prefix:
+                install = subprocess.run(
+                    [
+                        npm,
+                        "install",
+                        "--prefix",
+                        prefix,
+                        "--no-audit",
+                        "--no-fund",
+                        self.spec,
+                        *_REGISTRY_FLAGS,
+                    ],
+                    capture_output=True,
+                    text=True,
+                    timeout=120,
+                    stdin=subprocess.DEVNULL,
+                )
+                if install.returncode != 0:
+                    self._fail_or_skip(
+                        self.spec,
+                        f"`npm install` exited {install.returncode}: {install.stderr[-500:]}",
+                        False,
+                    )
+                cli = os.path.join(prefix, "node_modules", ".bin", "claude")
+                help_result = subprocess.run(
+                    [cli, "--help"],
+                    capture_output=True,
+                    text=True,
+                    timeout=30,
+                    stdin=subprocess.DEVNULL,
+                )
+        except (OSError, subprocess.TimeoutExpired) as exc:
+            self._fail_or_skip(self.spec, f"could not run pinned CLI `--help`: {exc}", False)
+        self.assertEqual(0, help_result.returncode, help_result.stderr[-500:])
+        self.assertIn(
+            "--max-budget-usd <amount>",
+            help_result.stdout,
+            f"{self.spec} does not advertise the finder budget flag in `claude --help`",
+        )
+
     def _positive_control(self):
         """Prove this npm/registry pair answers about the pinned spec at all.
 

.github/groom/tests/test_finder_bounds.py

@@ -0,0 +1,57 @@
+import pathlib
+import re
+import unittest
+
+
+ROOT = pathlib.Path(__file__).resolve().parents[3]
+WORKFLOW = (ROOT / ".github/workflows/groom.yml").read_text(encoding="utf-8")
+FINDER_BRIEF = (ROOT / ".github/groom/finder.md").read_text(encoding="utf-8")
+
+
+class TestFinderBounds(unittest.TestCase):
+    @staticmethod
+    def finder_step():
+        match = re.search(
+            r"(?ms)^      - name: Run finder\n(?P<body>.*?)^      - name: Unlock the clone\n",
+            WORKFLOW,
+        )
+        if match is None:
+            raise AssertionError("groom.yml has no Run finder step followed by Unlock the clone")
+        return match.group("body")
+
+    def test_finder_cli_has_turn_and_dollar_caps(self):
+        finder_step = self.finder_step()
+        claude_command = finder_step.split('claude -p "$PROMPT"', 1)[1].split(
+            "--output-format json", 1
+        )[0]
+
+        self.assertRegex(claude_command, re.compile(r"--max-turns\s+150\s+\\"))
+        self.assertRegex(claude_command, re.compile(r"--max-budget-usd\s+8\s+\\"))
+
+    def test_brief_requires_an_early_result_and_reports_why_it_stopped(self):
+        self.assertNotIn("estimated inspection spend", FINDER_BRIEF)
+        self.assertIn("After at most 60 inspection tool calls", FINDER_BRIEF)
+        self.assertIn("Reserve enough time to write the result", FINDER_BRIEF)
+        self.assertIn("Fewer than 6 findings is valid", FINDER_BRIEF)
+        self.assertIn('"stop_reason":"inspection-complete|inspection-call-limit"', FINDER_BRIEF)
+        self.assertIn(
+            "If an inspection call is denied, do not retry that operation through another tool",
+            FINDER_BRIEF,
+        )
+
+    def test_complete_handoff_survives_a_late_cli_failure(self):
+        finder_step = self.finder_step()
+        self.assertIn('if [ -s "$FINDER_OUT" ] && jq -e', finder_step)
+        self.assertIn('(.stop_reason | IN("inspection-complete", "inspection-call-limit"))', finder_step)
+        self.assertIn("preserving it for independent verification", finder_step)
+        self.assertIn("STATUS=0", finder_step)
+
+    def test_zero_at_call_limit_is_not_reported_as_clean(self):
+        self.assertIn(
+            "reached its inspection-call limit with zero findings",
+            WORKFLOW,
+        )
+
+
+if __name__ == "__main__":
+    unittest.main()

.github/workflows/groom.yml

@@ -1380,8 +1380,12 @@ jobs:
           # auto-discovery (and hooks/plugins/keychain); auth stays on the
           # ANTHROPIC_API_KEY set above, the only credential this step has (--bare
           # never reads OAuth/keychain, so a missing key fails loudly, not silently).
-          # --max-turns 150: the whole-repo finder brief needs ~82 turns in a healthy
-          # environment (validated); the old 40 could not finish the brief at all.
+          # Two independent ceilings bound a runaway finder: 150 turns and $8.
+          # Healthy whole-repo finder runs have measured about 82 turns, so 150
+          # retains enough headroom for denied calls and synthesis while the brief's
+          # separately countable 60-inspection-call limit drives early completion.
+          # The dollar ceiling is intentionally separate: turn cost varies with
+          # context size, so a turn cap alone is not a reliable spend boundary.
           # `git grep` is deliberately NOT allowlisted: `--open-files-in-pager=<cmd>`
           # executes an arbitrary command. The Grep tool covers content search.
           # `Edit(//<abs path>)` instead of a bare `Write` is the STRUCTURAL close of
@@ -1424,6 +1428,7 @@ jobs:
           claude -p "$PROMPT" \
             --model "$MODEL" \
             --max-turns 150 \
+            --max-budget-usd 8 \
             --allowedTools "Read,Glob,Grep,Edit(//${FINDER_OUT#/}),Bash(git log:*),Bash(git show:*),Bash(grep:*),Bash(cat:*),Bash(ls:*),Bash(head:*),Bash(tail:*),Bash(wc:*)" \
             --bare \
             --setting-sources "" \
@@ -1441,12 +1446,22 @@ jobs:
             echo "::error::finder output contains ANTHROPIC_API_KEY — refusing to publish (possible prompt-injection exfil)."
             rm -f "$FINDER_OUT"; STATUS=1
           fi
-          # Do NOT swallow a non-zero exit: a rate-limit/network/turn-exhaustion
-          # failure must fail the job loudly. Swallowing it made the run green and
-          # silently discarded the finder's real candidates. The diagnostics upload
-          # below is `if: always()` so it still survives this failure.
+          # Preserve a complete handoff even if the CLI exits non-zero after writing
+          # it (for example, if a ceiling lands after the final Edit). The independent
+          # verifier still checks every candidate. A missing/incomplete handoff keeps
+          # the CLI failure loud, and the diagnostics upload below survives either
+          # path via `if: always()`.
           if [ "$STATUS" -ne 0 ]; then
-            echo "::error::claude exited $STATUS — see the groom-finder-exec artifact for subtype/num_turns."
+            if [ -s "$FINDER_OUT" ] && jq -e '
+              type == "object"
+              and (.findings | type == "array")
+              and (.stop_reason | IN("inspection-complete", "inspection-call-limit"))
+            ' "$FINDER_OUT" >/dev/null 2>&1; then
+              echo "::warning::claude exited $STATUS after producing a complete finder handoff; preserving it for independent verification."
+              STATUS=0
+            else
+              echo "::error::claude exited $STATUS without a complete finder handoff — see the groom-finder-exec artifact for subtype/num_turns."
+            fi
           fi
           exit "$STATUS"
 
@@ -1541,6 +1556,14 @@ jobs:
             echo "::error::Finder did not produce valid JSON at $FINDER_OUT."
             exit 1
           fi
+          if ! jq -e '
+            type == "object"
+            and (.findings | type == "array")
+            and (.stop_reason | IN("inspection-complete", "inspection-call-limit"))
+          ' "$FINDER_OUT" >/dev/null; then
+            echo "::error::Finder output is missing a valid findings array or stop_reason."
+            exit 1
+          fi
           # ENFORCE the path scope (BE-4757). The brief ASKS the finder to stay in
           # the directory; this is what makes it true. Runs on the finder output
           # because that is where the evidence `sites` live, and filtering here
@@ -1558,6 +1581,9 @@ jobs:
           fi
           COUNT=$(jq '.findings | length' "$FINDER_OUT")
           echo "Finder candidates: $COUNT"
+          if [ "$COUNT" -eq 0 ] && [ "$(jq -r '.stop_reason' "$FINDER_OUT")" = "inspection-call-limit" ]; then
+            echo "::warning::Finder reached its inspection-call limit with zero findings; this was a bounded run, not a clean-repository claim."
+          fi
           if [ "$COUNT" -gt 0 ]; then
             echo "have_candidates=true" >> "$GITHUB_OUTPUT"
           else
@@ -2660,7 +2686,21 @@ jobs:
           # bank a half-finished (or empty) patch as a legitimate result.
           # `fail-fast: false` keeps the other findings building.
           if [ "$STATUS" -ne 0 ]; then
-            echo "::error::claude exited $STATUS — see the groom-builder-exec-$IDX artifact."
+            # A ceiling can land just after the builder completed a clean bail-out.
+            # Preserve that bounded handoff so build_pr can file the verified finding
+            # as an issue. Never preserve `patched` here: a non-zero exit can leave a
+            # half-written patch, and the normal successful path must validate it.
+            if [ -s "$BUILDER_OUT" ] && jq -e '
+              type == "object"
+              and .status == "bail"
+              and (.summary | type == "string" and length > 0)
+            ' "$BUILDER_OUT" >/dev/null 2>&1 \
+              && test -z "$(git status --short)"; then
+              echo "::warning::claude exited $STATUS after producing a complete control file; preserving the clean bail-out for issue filing."
+              STATUS=0
+            else
+              echo "::error::claude exited $STATUS — see the groom-builder-exec-$IDX artifact."
+            fi
           fi
           exit "$STATUS"