diff --git a/corpus/CLAUDE.learned.md b/corpus/CLAUDE.learned.md index 2c777432f..fb09cdd3a 100644 --- a/corpus/CLAUDE.learned.md +++ b/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 ` / `! python3 ` 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). diff --git a/engine/hooks/_runner/dispatch.py b/engine/hooks/_runner/dispatch.py index 35743450d..090acbb09 100644 --- a/engine/hooks/_runner/dispatch.py +++ b/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: diff --git a/engine/hooks/_runner/tests/chaos/exit_hook.py b/engine/hooks/_runner/tests/chaos/exit_hook.py new file mode 100644 index 000000000..d1b944041 --- /dev/null +++ b/engine/hooks/_runner/tests/chaos/exit_hook.py @@ -0,0 +1,4 @@ +from __future__ import annotations + +def detect(event): + raise SystemExit(3) diff --git a/engine/hooks/_runner/tests/chaos/hang_hook.py b/engine/hooks/_runner/tests/chaos/hang_hook.py new file mode 100644 index 000000000..8643e7979 --- /dev/null +++ b/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 [] diff --git a/engine/hooks/_runner/tests/chaos/ok_hook.py b/engine/hooks/_runner/tests/chaos/ok_hook.py new file mode 100644 index 000000000..c56eb8665 --- /dev/null +++ b/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", + ) + ] diff --git a/engine/hooks/_runner/tests/chaos/raise_hook.py b/engine/hooks/_runner/tests/chaos/raise_hook.py new file mode 100644 index 000000000..39a4c6d0c --- /dev/null +++ b/engine/hooks/_runner/tests/chaos/raise_hook.py @@ -0,0 +1,5 @@ +from __future__ import annotations + + +def detect(event): + raise RuntimeError("chaos fixture boom") diff --git a/engine/hooks/_runner/tests/test_dispatch.py b/engine/hooks/_runner/tests/test_dispatch.py index 20dc6e02c..f96ab1ecc 100644 --- a/engine/hooks/_runner/tests/test_dispatch.py +++ b/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" diff --git a/product/skills/plan-first/SKILL.md b/product/skills/plan-first/SKILL.md index 981b56967..020a8d92f 100644 --- a/product/skills/plan-first/SKILL.md +++ b/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 diff --git a/product/skills/plan-first/tests/fires_mock_first_example.md b/product/skills/plan-first/tests/fires_mock_first_example.md new file mode 100644 index 000000000..6353f0963 --- /dev/null +++ b/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.