merge queue: checking #1358 + #1359 + #1336 together on main (76f7a53)

corpus/CLAUDE.learned.md

@@ -39,6 +39,7 @@ Engine-only install drops this file; reflect Accepted global rules land here.
 
 - After an audit that names existing scorers, rankers, or weight formulas, the next plan picks among those named gaps. Bind "real classifier" / "these weights are arbitrary" onto a named scorer before proposing a new model or library. If I say I'm not comfortable with ML, teach the existing named scorers first and offer a no-library path before sklearn. The audit's named gaps are the backlog: a plan that jumps to a new classifier throws away findings already paid for, and the fix the audit points at (for example, scoring a signal by how many events it appears in rather than by its exact string) is usually a small change to a scorer that exists. This is Zinkevich's first rule of machine learning: do not be afraid to launch without machine learning — improve the heuristic you have before replacing it.
 - When a permission classifier or sandbox denies a tool call, treat the first denial as categorical for that target — don't retry the same blocked action through a different tool (Bash → Edit → a different Bash invocation). Switch strategy immediately, usually to a script handoff (next rule). The policy judges the target and the action, not the tool, so every route to the same write meets the same check — Saltzer and Schroeder's complete mediation — and hunting for an unguarded route is working around a security control.
+- Before handing me a step to run, run its first command yourself and read the result. Hand it back only when the result names a blocker that only I can clear: a password, a browser login, hardware, or an operating-system approval dialog. A missing tool, a wrong path, or a script you have not run is not a blocker: install the tool, fix the path, or run the script, then report. When you do hand a step back, quote the blocker in the same message. A handoff written before the first command was tried tends to fail on its own first line, and the person pays for it twice.
 - When handing off a blocked action for me to run myself, always write it to a small script file and hand back exactly one `! bash <path>` / `! python3 <path>` line — never paste inline multi-line, multi-flag, or `&&`/`&`-backgrounded shell text into chat for me to copy. This applies on the very first handoff and every one after it, not after I complain about copy-paste pain. Pasted shell breaks in transit — wrapped lines, lost quoting, an `&` that detaches a job — while one `!` line runs exactly the file that was written.
 - Never pass a quoted command string through `sudo -i`, `su -`, or `su -l`. Those start a login shell that reads the command a second time, after the first shell already used up the quotes, so the string splits on spaces and its first word runs alone (`sudo -u demo -H -i bash -lc 'set -u; echo one'` fails with `set: -c: invalid option`; the same line without `-i` prints `one`). Copy the script to the host and run it by path, or drop `-i`.
 - The shell behind the Bash tool may be zsh, which does not split an unquoted variable into words: `for x in $LIST` runs once with the whole list as one item, exits 0, and reads as "all done". Run any loop over a list under bash (`bash <<'EOF' ... EOF`), count lookups that failed as unchecked rather than done, and spot-check one item before acting on the loop's summary. This is zsh's `SH_WORD_SPLIT` option being off by default (zsh manual, Options, https://zsh.sourceforge.io/Doc/Release/Options.html#index-SH_005fWORD_005fSPLIT).

engine/hooks/_runner/dispatch.py

@@ -424,7 +424,7 @@ def _timeout(_signum, _frame):
         if isinstance(exc, SystemExit):
             exit_code = exc.code if isinstance(exc.code, int) else 1
         else:
-            exit_code = 0
+            exit_code = 1
         stdout = b""
         stderr = f"catstack-hook-error {hook}: {type(exc).__name__}: {exc}\n".encode()
     finally:

engine/hooks/_runner/tests/chaos/exit_hook.py

@@ -0,0 +1,4 @@
+from __future__ import annotations
+
+def detect(event):
+    raise SystemExit(3)

engine/hooks/_runner/tests/chaos/hang_hook.py

@@ -0,0 +1,8 @@
+from __future__ import annotations
+
+import time
+
+
+def detect(event):
+    time.sleep(30)
+    return []

engine/hooks/_runner/tests/chaos/ok_hook.py

@@ -0,0 +1,15 @@
+from __future__ import annotations
+
+from finding import Finding
+
+
+def detect(event):
+    hook_name = str(event.get("_catstack_current_hook") or "chaos-ok")
+    return [
+        Finding(
+            rule_id=f"{hook_name}.spoke",
+            subject=hook_name,
+            message=f"{hook_name} sibling spoke",
+            evidence="chaos fixture",
+        )
+    ]

engine/hooks/_runner/tests/chaos/raise_hook.py

@@ -0,0 +1,5 @@
+from __future__ import annotations
+
+
+def detect(event):
+    raise RuntimeError("chaos fixture boom")

engine/hooks/_runner/tests/test_dispatch.py

@@ -10,6 +10,7 @@
 from unittest import mock
 
 RUNNER_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+CHAOS_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "chaos")
 
 
 class DispatchCLI(unittest.TestCase):
@@ -55,6 +56,19 @@ def _write_hook(
         with open(os.path.join(hook_dir, "claude.hook.json"), "w", encoding="utf-8") as handle:
             json.dump(manifest, handle)
 
+    def _write_chaos_sdk_hook(self, name: str, module_file: str) -> None:
+        with open(os.path.join(CHAOS_DIR, module_file), encoding="utf-8") as handle:
+            module_body = handle.read()
+        module_name = os.path.splitext(module_file)[0]
+        self._write_hook(
+            name,
+            "fallback.py",
+            "raise SystemExit('chaos SDK fixture unexpectedly used subprocess fallback')\n",
+            event="Stop",
+            subagent_stop={"dispatch": {"entry": f"{module_name}:detect"}},
+            extra_files={module_file: module_body},
+        )
+
     def _env(self) -> dict[str, str]:
         env = os.environ.copy()
         env["HOME"] = self.home
@@ -261,6 +275,41 @@ def test_sdk_entrypoint_runs_in_process_and_emits_rule_ids(self):
         self.assertEqual(row["dispatch_path"], "sdk")
         self.assertEqual(row["rule_ids"], ["fixture.sdk"])
 
+    def test_sdk_chaos_failures_each_emit_rows_without_silencing_siblings(self):
+        self._install_sdk()
+        self._write_chaos_sdk_hook("chaos-a-ok", "ok_hook.py")
+        self._write_chaos_sdk_hook("chaos-b-raise", "raise_hook.py")
+        self._write_chaos_sdk_hook("chaos-c-hang", "hang_hook.py")
+        self._write_chaos_sdk_hook("chaos-d-exit", "exit_hook.py")
+        self._write_chaos_sdk_hook("chaos-e-ok", "ok_hook.py")
+        mode_env = {
+            "CATSTACK_HOOK_MODE_CHAOS_A_OK": "warn",
+            "CATSTACK_HOOK_MODE_CHAOS_E_OK": "warn",
+        }
+        env_patch = mock.patch.dict(os.environ, mode_env)
+        env_patch.start()
+        self.addCleanup(env_patch.stop)
+
+        result = self._run(event="SubagentStop", timeout="1")
+
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertIn(b"chaos-ok sibling spoke", result.stdout)
+        rows = {row["hook"]: row for row in self._rows()}
+        self.assertEqual(
+            set(rows),
+            {"chaos-a-ok", "chaos-b-raise", "chaos-c-hang", "chaos-d-exit", "chaos-e-ok"},
+        )
+        self.assertEqual(rows["chaos-a-ok"]["outcome"], "spoke")
+        self.assertEqual(rows["chaos-e-ok"]["outcome"], "spoke")
+        self.assertEqual(rows["chaos-b-raise"]["outcome"], "crashed")
+        self.assertEqual(rows["chaos-c-hang"]["outcome"], "timed_out")
+        self.assertEqual(rows["chaos-d-exit"]["outcome"], "crashed")
+        self.assertEqual(rows["chaos-b-raise"]["exit_code"], 1)
+        self.assertEqual(rows["chaos-c-hang"]["exit_code"], 1)
+        self.assertEqual(rows["chaos-d-exit"]["exit_code"], 3)
+        self.assertEqual(rows["chaos-b-raise"]["dispatch_path"], "sdk_error")
+        self.assertEqual(rows["chaos-c-hang"]["dispatch_path"], "sdk_error")
+        self.assertEqual(rows["chaos-d-exit"]["dispatch_path"], "sdk_error")
 
 FIXTURES_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "fixtures")
 FIXTURE_HOOK = "fixture-extra-manifest"

product/skills/plan-first/SKILL.md

@@ -69,6 +69,17 @@ rows on real data, or a file that doesn't shrink without a follow-up
 `VACUUM`, is the plan finding that out before the diff does, not after — the
 alternative is a second PR stack to fix what the first one didn't move.
 
+If the claim is something a person will read, such as a page, dashboard,
+report, chart, or table of metrics, the claim is not settled until the user has
+seen it. Build a mock of the output on non-production data first, and settle
+three things with the user before any collection or production code is
+planned: what is shown, what each number or column means, and how it is
+grouped. The done-gate then names the approved mock. Collecting real data
+before that agreement tends to produce a pipeline that answers a question the
+user did not ask, and the expensive part is rebuilding it. When the plan goes
+to Invoker, mark it `deliverable: analytics` so its completeness gate requires
+the approved mock as the first task.
+
 ## Step 2. Derive the slices before writing any of them
 
 Delegate to `split-scope`. Decide the boundaries from what the repo's own

product/skills/plan-first/tests/fires_mock_first_example.md

@@ -0,0 +1,14 @@
+A user asks for a page that shows how much faster merge requests would finish if
+a chosen CI job ran faster. The agent is about to write a collector that pulls
+job data from the CI system.
+
+This skill fires. The output is something a person will read, so the claim is
+not settled until the user has seen it. Before any collection code is planned,
+the agent builds a mock page on fake data and asks the user what is shown, what
+each number means, and how merge requests are grouped.
+
+The done-gate then names the approved mock, not "the page loads". A collector
+built first would answer whatever question the agent guessed, and the user would
+only find out after the slow collection run finished. When the plan goes to
+Invoker it carries `deliverable: analytics`, so the first task is the approved
+mock and every other task depends on it.