From 5a9e5c1b6e5ec6032e520a182892ba20a675846f Mon Sep 17 00:00:00 2001 From: CI Bot Date: Sun, 11 Oct 2026 14:27:01 +0000 Subject: [PATCH 1/4] Let the install tests read either hook layout. tests/test_install.py and engine/hooks/_runner/tests/test_wrap_installed.py asserted one settings entry per hook, so every per-hook guarantee they check failed once CATSTACK_HOOK_DISPATCHER (#1122) collapsed an event to a single _runner/dispatch.py entry: 70 failures and 1 error on #1122's head, all in these two files. load_config() in tests/test_install.py now reads an installed config and expands a collapsed event back into one entry per hook, resolving each hook's registration the way dispatch.py itself does (dispatch.load_event_hooks: the cursor registry, or the hook's own manifest for claude and codex). Each assertion therefore sees the identical shape in both layouts and keeps checking the same guarantee -- right event, right harness, Stop mirrored to SubagentStop or opted out, dead entries pruned, non-catstack entries untouched. test_wrap_installed.py's fixture pins the flag off, since it asserts the settings bytes install writes rather than a hook's registration. #1122's own test_default_is_off_and_byte_identical_to_no_flag_at_all now compares the default install against an install that sets the flag to whatever flags.py resolves as the default, and clears the key from its own environment first, so the flag-on run of the full suite cannot leak into the "default" side of the comparison. The CATSTACK_HOOK_DISPATCHER default is left off. Flipping it on is gated on both full-suite runs passing, and the source branch plan/hook-backlog-1c-install-tests-accept-the-one-entry-per-event-layout-default-on-only-if-both-suites-pass never changed engine/hooks/_flags/flags.py, so that gate was not met there and this commit does not claim it. Cherry-picked from 6330abf291561cace7301948bda714d0b8795f43 on that branch. The branch's two other content commits (engine/hooks/_runner/doctor.py, scripts/test/run_skill_scenarios.py) are unrelated to this claim and are left behind. --- .../_runner/tests/test_wrap_installed.py | 3 + tests/test_install.py | 278 +++++++++++------- 2 files changed, 171 insertions(+), 110 deletions(-) diff --git a/engine/hooks/_runner/tests/test_wrap_installed.py b/engine/hooks/_runner/tests/test_wrap_installed.py index 008b06bd4..6de0ec8a3 100644 --- a/engine/hooks/_runner/tests/test_wrap_installed.py +++ b/engine/hooks/_runner/tests/test_wrap_installed.py @@ -24,6 +24,9 @@ def setUp(self): self.old_home = os.environ.get("HOME") os.environ["HOME"] = str(self.home) self.addCleanup(self._restore_home) + dispatcher_patch = mock.patch.dict(os.environ, {"CATSTACK_HOOK_DISPATCHER": "0"}) + dispatcher_patch.start() + self.addCleanup(dispatcher_patch.stop) self.claude_path = self.home / ".claude" / "settings.json" self.cursor_path = self.home / ".cursor" / "hooks.json" self.codex_path = self.home / ".codex" / "hooks.json" diff --git a/tests/test_install.py b/tests/test_install.py index e065f2d94..675124847 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -28,6 +28,7 @@ import tomllib import unittest from pathlib import Path +from unittest import mock REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) INSTALL_SH = os.path.join(REPO_ROOT, "install.sh") @@ -167,9 +168,7 @@ def installed_config_hook_names(fake_home): path = os.path.join(fake_home, relative) if not os.path.exists(path): continue - with open(path) as handle: - data = json.load(handle) - for command in json_commands(data.get("hooks", {})): + for command in json_commands(load_config(path).get("hooks", {})): hook = catstack_hook_from_command(command) if hook: names.add(hook) @@ -186,12 +185,103 @@ def installed_config_commands(fake_home): path = os.path.join(fake_home, relative) if not os.path.exists(path): continue - with open(path) as handle: - data = json.load(handle) - commands.extend(json_commands(data.get("hooks", {}))) + commands.extend(json_commands(load_config(path).get("hooks", {}))) return commands +sys.path.insert(0, os.path.join(REPO_ROOT, "engine", "hooks", "_runner")) +sys.path.insert(0, os.path.join(REPO_ROOT, "engine", "hooks", "_flags")) +import dispatch as _dispatch # noqa: E402 +import wrap_installed as _wrap_installed # noqa: E402 +import flags as _flags # noqa: E402 + +_HARNESS_BY_DIR = {".claude": "claude", ".cursor": "cursor", ".codex": "codex"} + + +def _harness_for_config_path(path): + parts = os.path.normpath(path).split(os.sep) + for harness_dir, harness in _HARNESS_BY_DIR.items(): + if harness_dir in parts: + return harness + raise ValueError(f"not a recognized install config path: {path}") + + +def _fake_home_for_config_path(path, harness): + marker = os.sep + f".{harness}" + os.sep + index = path.rfind(marker) + return path[:index] + + +def _record_command(record, harness): + timeout_value = record.get("timeout") + if isinstance(timeout_value, (int, float)) and not isinstance(timeout_value, bool): + budget = timeout_value - 0.5 + else: + budget = 59.5 + budget_str = _wrap_installed._format_timeout(budget) + trailing = " ".join(record.get("args") or []) + suffix = f" {trailing}" if trailing else "" + return ( + f"python3 $HOME/.{harness}/hooks/_runner/run.py --timeout {budget_str} " + f"{record['hook']}/{record['script']}{suffix}" + ) + + +def _records_to_groups(records, harness): + groups = [] + for record in records: + command = _record_command(record, harness) + matcher = record.get("matcher") + if harness == "cursor": + entry = {"command": command} + if record.get("timeout") is not None: + entry["timeout"] = record["timeout"] + if matcher is not None: + entry["matcher"] = matcher + else: + hook_obj = {"type": "command", "command": command} + if record.get("timeout") is not None: + hook_obj["timeout"] = record["timeout"] + entry = {"hooks": [hook_obj]} + if matcher is not None: + entry["matcher"] = matcher + groups.append(entry) + return groups + + +def load_config(path): + """Read an installed hooks config file (.claude/settings.json, + .cursor/hooks.json, .codex/hooks.json), expanding any event collapsed + behind one `_runner/dispatch.py` entry back into one entry per hook -- + the same shape install leaves with CATSTACK_HOOK_DISPATCHER off -- by + reading that hook's registration the same way the dispatcher itself + would (`dispatch.load_event_hooks`: the cursor registry file, or the + hook's own manifest for claude and codex). A test written against the + one-entry-per-hook layout reads the identical shape in either layout, + so the same assertion checks the same per-hook guarantee regardless of + which layout install actually produced.""" + with open(path, encoding="utf-8") as handle: + data = json.load(handle) + hooks = data.get("hooks") + if not isinstance(hooks, dict): + return data + harness = _harness_for_config_path(path) + fake_home = _fake_home_for_config_path(path, harness) + hooks_root = os.path.join(fake_home, f".{harness}", "hooks") + expanded = {} + changed = False + for event, groups in hooks.items(): + if isinstance(groups, list) and any("_runner/dispatch.py" in c for c in json_commands(groups)): + changed = True + records, _warnings = _dispatch.load_event_hooks(hooks_root, harness, event) + expanded[event] = _records_to_groups(records, harness) + else: + expanded[event] = groups + if not changed: + return data + return {**data, "hooks": expanded} + + class TestNeverTouchesRealHome(unittest.TestCase): """Safety-guard meta-test: every other test in this file builds its fake $HOME the same way (tempfile.TemporaryDirectory()) -- this asserts that @@ -255,8 +345,7 @@ def test_explicit_failures_hook_linked_and_pretooluse_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "explicit-failures") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "explicit-failures")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any("explicit-failures/claude_pretooluse.py" in hook["command"] for hook in entry["hooks"]) @@ -272,8 +361,7 @@ def test_bound_tool_result_wired_for_claude_cursor_and_codex(self): self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, name)) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) claude_entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any(f"{name}/claude_pre_tool_use.py" in hook["command"] for hook in entry["hooks"]) @@ -281,13 +369,11 @@ def test_bound_tool_result_wired_for_claude_cursor_and_codex(self): self.assertEqual(len(claude_entries), 1, claude_entries) self.assertEqual(claude_entries[0]["matcher"], "Bash") - with open(os.path.join(self.fake_home, ".cursor", "hooks.json")) as handle: - cursor_hooks = json.load(handle)["hooks"] + cursor_hooks = load_config(os.path.join(self.fake_home, ".cursor", "hooks.json"))["hooks"] cursor_commands = [str(entry.get("command", "")) for entry in cursor_hooks["preToolUse"]] self.assertEqual(sum(f"{name}/cursor_pre_tool_use.py" in c for c in cursor_commands), 1, cursor_commands) - with open(os.path.join(self.fake_home, ".codex", "hooks.json")) as handle: - codex_hooks = json.load(handle)["hooks"] + codex_hooks = load_config(os.path.join(self.fake_home, ".codex", "hooks.json"))["hooks"] codex_commands = [hook["command"] for entry in codex_hooks["PreToolUse"] for hook in entry["hooks"]] self.assertEqual(sum(f"{name}/codex_pre_tool_use.py" in c for c in codex_commands), 1, codex_commands) @@ -298,8 +384,7 @@ def test_text_match_decision_warn_wired_for_claude_cursor_and_codex(self): self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, name)) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) claude_entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any(f"{name}/claude_pretooluse.py" in hook["command"] for hook in entry["hooks"]) @@ -308,19 +393,16 @@ def test_text_match_decision_warn_wired_for_claude_cursor_and_codex(self): self.assertEqual(claude_entries[0]["matcher"], "Edit|Write|MultiEdit") self.assertIn("_runner/run.py", claude_entries[0]["hooks"][0]["command"]) - with open(os.path.join(self.fake_home, ".cursor", "hooks.json")) as handle: - cursor_hooks = json.load(handle)["hooks"] + cursor_hooks = load_config(os.path.join(self.fake_home, ".cursor", "hooks.json"))["hooks"] cursor_commands = [str(entry.get("command", "")) for entry in cursor_hooks["preToolUse"]] self.assertEqual(sum(f"{name}/cursor_pretooluse.py" in c for c in cursor_commands), 1, cursor_commands) - with open(os.path.join(self.fake_home, ".codex", "hooks.json")) as handle: - codex_hooks = json.load(handle)["hooks"] + codex_hooks = load_config(os.path.join(self.fake_home, ".codex", "hooks.json"))["hooks"] codex_commands = [hook["command"] for entry in codex_hooks["PreToolUse"] for hook in entry["hooks"]] self.assertEqual(sum(f"{name}/codex_pretooluse.py" in c for c in codex_commands), 1, codex_commands) def _claude_hook_commands(self, event): - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) return [h["command"] for e in settings["hooks"][event] for h in e["hooks"]] def test_restated_constraint_linked_and_prompt_submit_wired_for_claude(self): @@ -333,8 +415,7 @@ def test_wait_needs_wakeup_linked_and_both_events_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "wait-needs-wakeup") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "wait-needs-wakeup")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) pre = [h["command"] for e in settings["hooks"]["PreToolUse"] for h in e["hooks"]] stop = [h["command"] for e in settings["hooks"]["Stop"] for h in e["hooks"]] self.assertTrue(any("wait-needs-wakeup/claude_pretooluse.py" in c for c in pre), pre) @@ -351,8 +432,7 @@ def test_answer_overrides_menu_linked_and_posttooluse_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "answer-overrides-menu") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "answer-overrides-menu")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) entries = [ entry for entry in settings["hooks"]["PostToolUse"] if any("answer-overrides-menu/claude_posttooluse.py" in hook["command"] for hook in entry["hooks"]) @@ -364,8 +444,7 @@ def test_serial_option_guard_linked_and_pretooluse_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "serial-option-guard") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "serial-option-guard")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any("serial-option-guard/claude_pretooluse.py" in hook["command"] for hook in entry["hooks"]) @@ -409,8 +488,7 @@ def test_history_before_reversal_linked_and_bash_pretooluse_wired_for_claude(sel target = os.path.join(self.fake_home, ".claude", "hooks", "history-before-reversal") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "history-before-reversal")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any("history-before-reversal/claude_pretooluse.py" in hook["command"] for hook in entry["hooks"]) @@ -422,8 +500,7 @@ def test_external_claim_gate_linked_and_bash_pretooluse_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "external-claim-gate") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "external-claim-gate")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any("external-claim-gate/claude_pretooluse.py" in hook["command"] for hook in entry["hooks"]) @@ -450,8 +527,7 @@ def test_publish_act_guard_linked_and_bash_pretooluse_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "publish-act-guard") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "publish-act-guard")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any("publish-act-guard/claude_pretooluse.py" in hook["command"] for hook in entry["hooks"]) @@ -463,8 +539,7 @@ def test_retired_agent_routing_guard_is_gone(self): self.assertFalse( os.path.exists(os.path.join(self.fake_home, ".claude", "hooks", "agent-routing-guard")) ) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) stale = [ entry for entry in settings["hooks"]["PreToolUse"] if any("agent-routing-guard" in hook["command"] for hook in entry["hooks"]) @@ -475,8 +550,7 @@ def test_categorical_scope_guard_linked_and_bash_pretooluse_wired_for_claude(sel target = os.path.join(self.fake_home, ".claude", "hooks", "categorical-scope-guard") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "categorical-scope-guard")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any("categorical-scope-guard/claude_pretooluse.py" in hook["command"] for hook in entry["hooks"]) @@ -497,8 +571,7 @@ def test_hedge_runs_prove_it_linked_and_stop_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "hedge-runs-prove-it") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "hedge-runs-prove-it")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) stop = [h["command"] for e in settings["hooks"]["Stop"] for h in e["hooks"]] self.assertTrue(any("hedge-runs-prove-it/claude_stop_check.py" in c for c in stop), stop) @@ -533,8 +606,7 @@ def test_new_file_callout_linked_and_stop_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "new-file-callout") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "new-file-callout")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) stop = [h["command"] for e in settings["hooks"]["Stop"] for h in e["hooks"]] self.assertTrue(any("new-file-callout/claude_stop_check.py" in c for c in stop), stop) @@ -542,8 +614,7 @@ def test_agent_relay_attribution_linked_and_stop_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "agent-relay-attribution") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "agent-relay-attribution")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) stop = [h["command"] for e in settings["hooks"]["Stop"] for h in e["hooks"]] self.assertTrue(any("agent-relay-attribution/claude_stop_check.py" in c for c in stop), stop) @@ -551,8 +622,7 @@ def test_scratchpad_collision_linked_and_pretooluse_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "scratchpad-collision") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "scratchpad-collision")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) pre = [h["command"] for e in settings["hooks"]["PreToolUse"] for h in e["hooks"]] self.assertTrue(any("scratchpad-collision/claude_pretooluse.py" in c for c in pre), pre) @@ -565,13 +635,11 @@ def test_reflect_on_thrash_wired_for_claude_and_cursor(self): hook_src(self.fake_home, "reflect-on-thrash"), ) settings_path = os.path.join(self.fake_home, ".claude", "settings.json") - with open(settings_path) as handle: - settings = json.load(handle) + settings = load_config(settings_path) stop_commands = [h["command"] for e in settings["hooks"]["Stop"] for h in e["hooks"]] self.assertTrue(any("claude_stop_reflect.py" in c for c in stop_commands), stop_commands) cursor_path = os.path.join(self.fake_home, ".cursor", "hooks.json") - with open(cursor_path) as handle: - cursor_hooks = json.load(handle) + cursor_hooks = load_config(cursor_path) stop = cursor_hooks["hooks"]["stop"] session_end = cursor_hooks["hooks"]["sessionEnd"] self.assertTrue(any("cursor_session.py" in str(e.get("command", "")) for e in stop), stop) @@ -619,8 +687,7 @@ def test_cat_mode_default_linked_and_prompt_hook_wired_for_claude(self): for agent_dir in (".cursor", ".codex"): self.assertFalse(os.path.lexists(os.path.join(self.fake_home, agent_dir, "hooks", "cat-mode-default"))) settings_path = os.path.join(self.fake_home, ".claude", "settings.json") - with open(settings_path) as handle: - settings = json.load(handle) + settings = load_config(settings_path) prompt_commands = [h["command"] for e in settings["hooks"]["UserPromptSubmit"] for h in e["hooks"]] matching = [c for c in prompt_commands if "cat-mode-default/claude_prompt_submit.py" in c] self.assertEqual(len(matching), 1, prompt_commands) @@ -632,8 +699,7 @@ def test_scope_lock_wired_for_claude_cursor_and_codex(self): self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "scope-lock")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) prompt_commands = [ hook["command"] for entry in settings["hooks"]["UserPromptSubmit"] @@ -647,8 +713,7 @@ def test_scope_lock_wired_for_claude_cursor_and_codex(self): self.assertTrue(any("claude_prompt_scope.py" in command for command in prompt_commands)) self.assertTrue(any("claude_pretool_scope.py" in command for command in pretool_commands)) - with open(os.path.join(self.fake_home, ".cursor", "hooks.json")) as handle: - cursor_hooks = json.load(handle)["hooks"] + cursor_hooks = load_config(os.path.join(self.fake_home, ".cursor", "hooks.json"))["hooks"] self.assertTrue(any( "cursor_before_submit.py" in str(entry.get("command", "")) for entry in cursor_hooks["beforeSubmitPrompt"] @@ -658,8 +723,7 @@ def test_scope_lock_wired_for_claude_cursor_and_codex(self): for entry in cursor_hooks["preToolUse"] )) - with open(os.path.join(self.fake_home, ".codex", "hooks.json")) as handle: - codex_hooks = json.load(handle)["hooks"] + codex_hooks = load_config(os.path.join(self.fake_home, ".codex", "hooks.json"))["hooks"] codex_prompt_commands = [ hook["command"] for entry in codex_hooks["UserPromptSubmit"] @@ -679,8 +743,7 @@ def test_split_scope_wired_for_claude_cursor_and_codex(self): self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "split-scope")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) claude_prompt_commands = [ hook["command"] for entry in settings["hooks"]["UserPromptSubmit"] @@ -688,8 +751,7 @@ def test_split_scope_wired_for_claude_cursor_and_codex(self): ] self.assertTrue(any("split-scope/claude_prompt_submit.py" in command for command in claude_prompt_commands)) - with open(os.path.join(self.fake_home, ".cursor", "hooks.json")) as handle: - cursor_hooks = json.load(handle)["hooks"] + cursor_hooks = load_config(os.path.join(self.fake_home, ".cursor", "hooks.json"))["hooks"] self.assertTrue(any( "split-scope/cursor_before_submit.py" in str(entry.get("command", "")) for entry in cursor_hooks["beforeSubmitPrompt"] @@ -699,8 +761,7 @@ def test_split_scope_wired_for_claude_cursor_and_codex(self): for entry in cursor_hooks["postToolUse"] )) - with open(os.path.join(self.fake_home, ".codex", "hooks.json")) as handle: - codex_hooks = json.load(handle)["hooks"] + codex_hooks = load_config(os.path.join(self.fake_home, ".codex", "hooks.json"))["hooks"] codex_prompt_commands = [ hook["command"] for entry in codex_hooks["UserPromptSubmit"] @@ -714,8 +775,7 @@ def test_hook_health_wired_for_claude_cursor_and_codex(self): self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "hook-health")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) claude_prompt_commands = [ hook["command"] for entry in settings["hooks"]["UserPromptSubmit"] @@ -723,15 +783,13 @@ def test_hook_health_wired_for_claude_cursor_and_codex(self): ] self.assertTrue(any("hook-health/claude_prompt_submit.py" in command for command in claude_prompt_commands)) - with open(os.path.join(self.fake_home, ".cursor", "hooks.json")) as handle: - cursor_hooks = json.load(handle)["hooks"] + cursor_hooks = load_config(os.path.join(self.fake_home, ".cursor", "hooks.json"))["hooks"] self.assertTrue(any( "hook-health/cursor_before_submit.py" in str(entry.get("command", "")) for entry in cursor_hooks["beforeSubmitPrompt"] )) - with open(os.path.join(self.fake_home, ".codex", "hooks.json")) as handle: - codex_hooks = json.load(handle)["hooks"] + codex_hooks = load_config(os.path.join(self.fake_home, ".codex", "hooks.json"))["hooks"] codex_prompt_commands = [ hook["command"] for entry in codex_hooks["UserPromptSubmit"] @@ -746,12 +804,9 @@ def test_skill_usage_log_is_linked_but_not_wired_while_registry_mode_is_off(self self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(self.fake_home, "skill-usage-log")) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - claude_hooks = json.load(handle)["hooks"] - with open(os.path.join(self.fake_home, ".cursor", "hooks.json")) as handle: - cursor_hooks = json.load(handle)["hooks"] - with open(os.path.join(self.fake_home, ".codex", "hooks.json")) as handle: - codex_hooks = json.load(handle)["hooks"] + claude_hooks = load_config(os.path.join(self.fake_home, ".claude", "settings.json"))["hooks"] + cursor_hooks = load_config(os.path.join(self.fake_home, ".cursor", "hooks.json"))["hooks"] + codex_hooks = load_config(os.path.join(self.fake_home, ".codex", "hooks.json"))["hooks"] expected = ( (claude_hooks, "PreToolUse", "skill-usage-log/claude_pretooluse_log.py"), (claude_hooks, "UserPromptSubmit", "skill-usage-log/claude_prompt_submit.py"), @@ -780,8 +835,7 @@ def test_llm_judge_inbox_wired_for_claude_cursor_and_codex(self): env={**os.environ, "HOME": self.fake_home, "CATSTACK_LLM_JUDGE_STATE_DIR": os.path.join(self.fake_home, "judge")}, ) self.assertEqual((result.returncode, result.stdout, result.stderr), (0, "", "")) - with open(os.path.join(self.fake_home, ".cursor", "hooks.json")) as handle: - cursor_stop = json.load(handle)["hooks"]["stop"] + cursor_stop = load_config(os.path.join(self.fake_home, ".cursor", "hooks.json"))["hooks"]["stop"] self.assertEqual(sum("llm-judge/cursor_session.py" in str(e.get("command", "")) for e in cursor_stop), 1, cursor_stop) def test_unverified_tag_check_is_linked_but_not_wired_while_registry_mode_is_off(self): @@ -797,8 +851,7 @@ def test_unverified_tag_check_is_linked_but_not_wired_while_registry_mode_is_off self.assertEqual(os.readlink(target), hook_src(self.fake_home, "unverified-tag-check")) claude_stop = self._claude_hook_commands("Stop") self.assertEqual(sum("unverified-tag-check/claude_stop_check.py" in command for command in claude_stop), 0, claude_stop) - with open(os.path.join(self.fake_home, ".cursor", "hooks.json")) as handle: - cursor_stop = json.load(handle)["hooks"]["stop"] + cursor_stop = load_config(os.path.join(self.fake_home, ".cursor", "hooks.json"))["hooks"]["stop"] self.assertEqual(sum("unverified-tag-check/cursor_session.py" in str(entry.get("command", "")) for entry in cursor_stop), 0, cursor_stop) with open(config_path) as handle: text = handle.read() @@ -1221,8 +1274,7 @@ def test_fresh_home_gets_both_hooks(self): run_install(fake_home) settings_path = os.path.join(fake_home, ".claude", "settings.json") self.assertTrue(os.path.exists(settings_path)) - with open(settings_path) as f: - settings = json.load(f) + settings = load_config(settings_path) stop_commands = [h["command"] for e in settings["hooks"]["Stop"] for h in e["hooks"]] prompt_commands = [h["command"] for e in settings["hooks"]["UserPromptSubmit"] for h in e["hooks"]] self.assertTrue(any("claude_stop_check.py" in c for c in stop_commands)) @@ -1236,8 +1288,7 @@ def test_preexisting_unrelated_keys_survive(self): with open(os.path.join(claude_dir, "settings.json"), "w") as f: json.dump({"model": "sonnet", "theme": "dark"}, f) run_install(fake_home) - with open(os.path.join(claude_dir, "settings.json")) as f: - settings = json.load(f) + settings = load_config(os.path.join(claude_dir, "settings.json")) self.assertEqual(settings["model"], "sonnet") self.assertEqual(settings["theme"], "dark") self.assertIn("Stop", settings["hooks"]) @@ -1272,8 +1323,7 @@ def test_install_writes_worker_flagged_hooks_only(self): with open(worker_settings_path, encoding="utf-8") as handle: worker = json.load(handle) - with open(os.path.join(fake_home, ".claude", "settings.json"), encoding="utf-8") as handle: - interactive = json.load(handle) + interactive = load_config(os.path.join(fake_home, ".claude", "settings.json")) self.assertEqual(worker.get("model"), "keep-me") worker_cmds = self._hook_commands(worker) @@ -1314,9 +1364,8 @@ def test_preseeded_config_toml_survives_without_notify_wiring(self): text = f.read() self.assertIn('model = "gpt-5"', text) self.assertNotIn("notify =", text) - with open(os.path.join(codex_dir, "hooks.json")) as f: - hooks_text = f.read() - self.assertIn("llm-judge/codex_post_tool_use.py", hooks_text) + hooks_data = load_config(os.path.join(codex_dir, "hooks.json")) + self.assertIn("llm-judge/codex_post_tool_use.py", json.dumps(hooks_data)) class TestIdempotency(unittest.TestCase): @@ -1591,8 +1640,7 @@ def test_settings_entry_for_a_removed_link_is_pruned(self): json.dump({"hooks": {"Stop": [{"matcher": "", "hooks": [{"type": "command", "command": retired}]}]}}, f) result = run_install(self.fake_home) self.assertEqual(result.returncode, 0, result.stderr) - with open(settings_path) as f: - settings = json.load(f) + settings = load_config(settings_path) all_commands = [h["command"] for groups in settings["hooks"].values() for g in groups for h in g["hooks"]] self.assertFalse([c for c in all_commands if "retired-hook" in c], all_commands) @@ -1630,8 +1678,7 @@ def test_dead_settings_entry_is_pruned_and_rest_survives(self): }, f) result = run_install(self.fake_home) self.assertEqual(result.returncode, 0, result.stderr) - with open(settings_path) as f: - settings = json.load(f) + settings = load_config(settings_path) all_commands = [h["command"] for groups in settings["hooks"].values() for g in groups for h in g["hooks"]] self.assertNotIn(dead, all_commands) self.assertTrue(any("diu-stop/claude_stop_check.py" in c for c in all_commands)) @@ -1656,8 +1703,7 @@ def tearDown(self): def test_every_claude_hook_entrypoint_is_in_settings(self): settings_path = os.path.join(self.fake_home, ".claude", "settings.json") - with open(settings_path) as handle: - settings = json.load(handle) + settings = load_config(settings_path) wired = { hook["command"] for entries in settings.get("hooks", {}).values() @@ -1809,8 +1855,7 @@ def setUp(self): self.tmp = tempfile.TemporaryDirectory() self.fake_home = self.tmp.name self.result = run_install(self.fake_home) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - self.settings = json.load(handle) + self.settings = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) sys.path.insert(0, os.path.join(REPO_ROOT, "scripts", "install")) import mirror_stop_hooks_to_subagent_stop as mirror_mod self.manifests = mirror_mod.load_manifests() @@ -1847,8 +1892,8 @@ def test_subagent_stop_entries_carry_no_tool_matcher(self): def test_rerun_keeps_subagent_stop_unchanged(self): second = run_install(self.fake_home) self.assertEqual(second.returncode, 0, second.stderr) - with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: - self.assertEqual(json.load(handle)["hooks"]["SubagentStop"], self.settings["hooks"]["SubagentStop"]) + after = load_config(os.path.join(self.fake_home, ".claude", "settings.json")) + self.assertEqual(after["hooks"]["SubagentStop"], self.settings["hooks"]["SubagentStop"]) class TestCatModeDefaultAgentHook(unittest.TestCase): @@ -1859,8 +1904,7 @@ def test_agent_pretooluse_hook_wired_for_claude(self): with tempfile.TemporaryDirectory() as fake_home: result = run_install(fake_home) self.assertEqual(result.returncode, 0, result.stderr) - with open(os.path.join(fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(fake_home, ".claude", "settings.json")) entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any("cat-mode-default/claude_pretooluse_agent.py" in hook["command"] for hook in entry["hooks"]) @@ -1877,8 +1921,7 @@ def test_fanout_routing_guard_linked_and_wired_for_claude(self): target = os.path.join(fake_home, ".claude", "hooks", "fanout-routing-guard") self.assertTrue(os.path.islink(target), target) self.assertEqual(os.readlink(target), hook_src(fake_home, "fanout-routing-guard")) - with open(os.path.join(fake_home, ".claude", "settings.json")) as handle: - settings = json.load(handle) + settings = load_config(os.path.join(fake_home, ".claude", "settings.json")) entries = [ entry for entry in settings["hooks"]["PreToolUse"] if any("fanout-routing-guard/claude_pretooluse_agent.py" in hook["command"] for hook in entry["hooks"]) @@ -1905,8 +1948,7 @@ def test_install_replaces_dangling_link_with_real_merged_file(self): self.assertEqual(proc.returncode, 0, proc.stderr) self.assertTrue(os.path.isfile(hooks_path)) self.assertFalse(os.path.islink(hooks_path)) - with open(hooks_path) as handle: - data = json.load(handle) + data = load_config(hooks_path) commands = [entry.get("command", "") for entry in data["hooks"]["stop"]] self.assertTrue(any("diu-stop/cursor_stop_check.py" in c for c in commands), commands) self.assertTrue(all("_runner/run.py" in c for c in commands), commands) @@ -2189,12 +2231,28 @@ def _cursor_hooks(self, fake_home): return json.load(handle) def test_default_is_off_and_byte_identical_to_no_flag_at_all(self): - with tempfile.TemporaryDirectory() as plain_home, tempfile.TemporaryDirectory() as off_home: - plain = run_install(plain_home, timeout=self.INSTALL_TIMEOUT) + """The default install (CATSTACK_HOOK_DISPATCHER unset) must equal an + install that explicitly sets the flag to whatever `flags.py` itself + resolves as the default -- today off, so this reduces to the + flag-off install, but it stays meaningful if the default ever flips + to on instead of silently comparing against the wrong side. The + ambient environment is cleared of the key first, so a caller that + happens to have CATSTACK_HOOK_DISPATCHER set (for example the + flag-on run of the full suite) cannot leak into the "default" side + of this comparison.""" + clean_env = {k: v for k, v in os.environ.items() if k != _flags.HOOK_DISPATCHER} + default_on = _flags.hook_dispatcher_on(clean_env, REPO_ROOT) + with tempfile.TemporaryDirectory() as plain_home, tempfile.TemporaryDirectory() as explicit_home: + with mock.patch.dict(os.environ, clean_env, clear=True): + plain = run_install(plain_home, timeout=self.INSTALL_TIMEOUT) self.assertEqual(plain.returncode, 0, plain.stderr) - off = run_install(off_home, extra_env={"CATSTACK_HOOK_DISPATCHER": "0"}, timeout=self.INSTALL_TIMEOUT) - self.assertEqual(off.returncode, 0, off.stderr) - self.assertEqual(self._claude_settings(plain_home), self._claude_settings(off_home)) + explicit = run_install( + explicit_home, + extra_env={_flags.HOOK_DISPATCHER: "1" if default_on else "0"}, + timeout=self.INSTALL_TIMEOUT, + ) + self.assertEqual(explicit.returncode, 0, explicit.stderr) + self.assertEqual(self._claude_settings(plain_home), self._claude_settings(explicit_home)) def test_on_collapses_claude_pretooluse_to_one_dispatcher_entry(self): with tempfile.TemporaryDirectory() as fake_home: From 1b46a91cadcb9c96ca915f5886077253ef6a36bb Mon Sep 17 00:00:00 2001 From: Invoker Bot Date: Sun, 11 Oct 2026 14:43:08 +0000 Subject: [PATCH 2/4] =?UTF-8?q?invoker:=20wf-1791728053338-10/publish-both?= =?UTF-8?q?-layout-install-tests=20=E2=80=94=20Review=20claim:=20tests.tes?= =?UTF-8?q?t=5Finstall=20and=20test=5Fwrap=5Finstalled=20accept=20both=20i?= =?UTF-8?q?nstall=20layouts=20(one=20entry=20per=20hook=20with=20CATSTACK?= =?UTF-8?q?=5FHOOK=5FDISPATCHER=20off,=20one=20dispatcher=20entry=20per=20?= =?UTF-8?q?event=20with=20it=20on),=20and=20the=20flag=20defaults=20to=20o?= =?UTF-8?q?n=20only=20if=20bash=20scripts/test/run=5Fall=5Ftests.sh=20pass?= =?UTF-8?q?es=20with=20the=20flag=20on=20and=20off.=20Review=20lane:=20beh?= =?UTF-8?q?avior=20Safety=20invariant:=20If=20either=20suite=20run=20fails?= =?UTF-8?q?,=20the=20default=20stays=20off.=20Per-hook=20guarantees=20stay?= =?UTF-8?q?=20checked=20in=20the=20dispatcher=20layout.=20Files:=20tests/t?= =?UTF-8?q?est=5Finstall.py,=20tests/test=5Fwrap=5Finstalled.py=20Change?= =?UTF-8?q?=20types:=20test=20Acceptance=20criteria:=20This=20workflow=20p?= =?UTF-8?q?ublishes=20one=20pull=20request=20for=20the=20install-layout=20?= =?UTF-8?q?claim=20only.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exit code: 0 Invoker-Finalize-Id: f459f9c9-a623-4fd7-9708-21cc3ae4cbda From 0425a9ade08a9efc0d11033245dad1b1ef69230b Mon Sep 17 00:00:00 2001 From: Edbert Chan <1916223+EdbertChan@users.noreply.github.com> Date: Mon, 12 Oct 2026 12:25:07 -0700 Subject: [PATCH 3/4] Add ci-job-timing skill: per-step timing report from Datadog Co-Authored-By: Claude Sonnet 5.5 Change-Id: Id0dec9283a11dc8cf7089f236efd154b5d84b090 --- product/skills/ci-job-timing/SKILL.md | 43 ++++++ .../ci-job-timing/scripts/ci_job_timing.py | 139 ++++++++++++++++++ .../ci-job-timing/tests/test_ci_job_timing.py | 57 +++++++ 3 files changed, 239 insertions(+) create mode 100644 product/skills/ci-job-timing/SKILL.md create mode 100644 product/skills/ci-job-timing/scripts/ci_job_timing.py create mode 100644 product/skills/ci-job-timing/tests/test_ci_job_timing.py diff --git a/product/skills/ci-job-timing/SKILL.md b/product/skills/ci-job-timing/SKILL.md new file mode 100644 index 000000000..c79b42611 --- /dev/null +++ b/product/skills/ci-job-timing/SKILL.md @@ -0,0 +1,43 @@ +--- +name: ci-job-timing +description: >- + Repeatable per-step timing report for any GitLab CI job, from Datadog. Use + when asked how long a CI job's steps take, where a slow job spends its time, + or to compare the same job across dev/prod or across different jobs + (for example "Build LAGVM (dev)", "xmm-image-packager (dev)"). +--- + +# ci-job-timing + +One recipe, any job. The job name and the number of days are the only inputs. + +## Why it reads log text + +Datadog CI Visibility has job-level events (`@ci.job.name`, `@duration` in +nanoseconds) but no step-level events. Step times come from `[CI_STEP]` log +lines written by the CI step wrapper. Lines from side-by-side tracks carry a +prefix (for example `[gvm]`, `[qssi]`) and Datadog does not split them into +fields, so the script reads the message text ("took N seconds") instead. + +## Steps + +1. Build the query and its link (`` is this skill's folder): + `python3 /scripts/ci_job_timing.py queries --job "" --days 3` +2. Run the printed steps query with the Datadog `ddsql_run_query` tool (from `now-d`). +3. Save the TSV it returns to a file and render the table: + `python3 /scripts/ci_job_timing.py report --job "" --days 3 --tsv ` +4. For side-by-side steps, pick one `job_id` from a raw `[CI_STEP]` line and run + `ci_job_timing.py timeline --job "" --job-id `. Read the start and end + times there; never add step averages into a job time. +5. Optional job-level cross-check: `aggregate_datadog_ci_pipeline_events` with + `ci_level=job`, query `@ci.job.name:""`, metric `@duration`. + +## Read the table correctly + +- `Runs` counts successful step lines only. A run that stopped early does not + count in its later steps, so counts differ between steps. +- Averages with few runs (n under 10) are weak. Say the n. +- Steps can overlap, so the column is not a share of the job. +- Zero rows means a wrong job name or an empty window: the script errors, it + never prints an empty table. +- A job whose steps do not log `[CI_STEP]` lines returns nothing; say that. diff --git a/product/skills/ci-job-timing/scripts/ci_job_timing.py b/product/skills/ci-job-timing/scripts/ci_job_timing.py new file mode 100644 index 000000000..0b16e692f --- /dev/null +++ b/product/skills/ci-job-timing/scripts/ci_job_timing.py @@ -0,0 +1,139 @@ +#!/usr/bin/env python3 +"""Build the Datadog queries and the report for one CI job's step timing. + +The Datadog connector is only reachable from the agent, so this script does the +two parts that must be identical on every run: write the query text (`queries`, +`timeline`) and turn the query result into a table (`report`). +""" +from __future__ import annotations + +import argparse +import csv +import io +import re +import sys +from urllib.parse import quote + +DEFAULT_HOST = "rivianvw.datadoghq.com" +# GitLab job names look like "Build LAGVM (dev)": letters, digits, space, ( ) . _ : / - +_JOB_RE = re.compile(r"^[A-Za-z0-9 ()._:/\-]+$") + + +def check_job(job: str) -> str: + if not job or not _JOB_RE.match(job): + raise ValueError(f"job name has characters the query cannot carry safely: {job!r}") + return job + + +def check_days(days: int) -> int: + if not 1 <= days <= 30: + raise ValueError(f"days must be 1-30, got {days}") + return days + + +def _seconds_expr() -> str: + return "CAST(SPLIT_PART(SPLIT_PART(message, 'took ', 2), ' seconds', 1) AS DECIMAL)" + + +def steps_sql(job: str, days: int) -> str: + """One row per step: runs, average, min, max seconds. Reads message text, so + lines prefixed [gvm]/[qssi] (which Datadog does not split into fields) count too.""" + check_job(job) + check_days(days) + flt = f'"[CI_STEP]" STEP_SUCCESS "[job={job}]"' + sec = _seconds_expr() + return ( + "SELECT SPLIT_PART(SPLIT_PART(message, 'Step: <', 2), '>', 1) AS step, " + f"COUNT(*) AS runs, AVG({sec}) AS avg_s, MIN({sec}) AS min_s, MAX({sec}) AS max_s " + f"FROM dd.logs(columns => ARRAY['message'], filter => '{flt}', " + f"from_timestamp => NOW() - INTERVAL '{days} day', to_timestamp => NOW()) " + "AS (message VARCHAR) GROUP BY SPLIT_PART(SPLIT_PART(message, 'Step: <', 2), '>', 1) " + "ORDER BY avg_s DESC LIMIT 60" + ) + + +def timeline_sql(job: str, job_id: int, days: int) -> str: + """Every step line of one run, oldest first, so side-by-side steps are visible.""" + check_job(job) + check_days(days) + flt = f'"[CI_STEP]" "[job_id={int(job_id)}]" "[job={job}]"' + return ( + "SELECT timestamp, message " + f"FROM dd.logs(columns => ARRAY['timestamp', 'message'], filter => '{flt}', " + f"from_timestamp => NOW() - INTERVAL '{days} day', to_timestamp => NOW()) " + "AS (timestamp TIMESTAMP, message VARCHAR) ORDER BY timestamp LIMIT 200" + ) + + +def link(sql: str, host: str = DEFAULT_HOST) -> str: + return f"https://{host}/ddsql/editor?q={quote(sql, safe='')}" + + +def parse_tsv(text: str) -> list[dict]: + """Parse the TSV block ddsql_run_query returns. Missing header or no rows -> error.""" + body = text + m = re.search(r"(.*?)", text, re.S) + if m: + body = m.group(1) + body = body.strip("\n") + if not body.strip(): + raise ValueError("no rows: the query returned nothing (wrong job name, or no runs in the window)") + rows = list(csv.DictReader(io.StringIO(body), delimiter="\t")) + need = {"step", "runs", "avg_s", "min_s", "max_s"} + if not rows or not need <= set(rows[0]): + raise ValueError(f"unexpected columns, need {sorted(need)}; got {list(rows[0]) if rows else 'none'}") + return rows + + +def report(job: str, days: int, rows: list[dict], url: str) -> str: + out = [ + f"## {job}: step times, last {days} day(s)", + "", + "| Step | Runs | Avg (s) | Avg (min) | Fastest (s) | Slowest (s) |", + "|---|---|---|---|---|---|", + ] + for r in rows: + avg = float(r["avg_s"]) + out.append( + f"| {r['step']} | {int(float(r['runs']))} | {avg:.0f} | {avg / 60:.1f} " + f"| {float(r['min_s']):.0f} | {float(r['max_s']):.0f} |" + ) + out += [ + "", + "Steps can run side by side, so do not add these up to a job time.", + f"Query: {url}", + ] + return "\n".join(out) + + +def main(argv: list[str] | None = None) -> int: + ap = argparse.ArgumentParser(description=__doc__) + sub = ap.add_subparsers(dest="cmd", required=True) + for name in ("queries", "report", "timeline"): + p = sub.add_parser(name) + p.add_argument("--job", required=True) + p.add_argument("--days", type=int, default=3) + p.add_argument("--host", default=DEFAULT_HOST) + if name == "timeline": + p.add_argument("--job-id", type=int, required=True) + if name == "report": + p.add_argument("--tsv", required=True, help="file holding the steps query result") + a = ap.parse_args(argv) + try: + if a.cmd == "queries": + sql = steps_sql(a.job, a.days) + print(f"-- steps query\n{sql}\n-- link\n{link(sql, a.host)}") + elif a.cmd == "timeline": + sql = timeline_sql(a.job, a.job_id, a.days) + print(f"-- timeline query\n{sql}\n-- link\n{link(sql, a.host)}") + else: + rows = parse_tsv(open(a.tsv, encoding="utf-8").read()) + print(report(a.job, a.days, rows, link(steps_sql(a.job, a.days), a.host))) + except (ValueError, OSError) as e: + print(f"error: {e}", file=sys.stderr) + return 2 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/product/skills/ci-job-timing/tests/test_ci_job_timing.py b/product/skills/ci-job-timing/tests/test_ci_job_timing.py new file mode 100644 index 000000000..348fd368d --- /dev/null +++ b/product/skills/ci-job-timing/tests/test_ci_job_timing.py @@ -0,0 +1,57 @@ +from __future__ import annotations + +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "scripts")) + +import ci_job_timing as c # noqa: E402 + +TSV = """2 + +step\truns\tavg_s\tmin_s\tmax_s +Build LAGVM System\t4\t1563.0\t1500\t1600 +Build LAGVM Kernel\t16\t295.0\t250\t446 +""" + + +class StepsSql(unittest.TestCase): + def test_job_and_days_are_in_the_query(self): + sql = c.steps_sql("Build LAGVM (dev)", 3) + self.assertIn('"[job=Build LAGVM (dev)]"', sql) + self.assertIn("INTERVAL '3 day'", sql) + + def test_same_template_works_for_another_job(self): + sql = c.steps_sql("xmm-image-packager (dev)", 7) + self.assertIn("[job=xmm-image-packager (dev)]", sql) + + def test_quote_in_job_name_is_refused(self): + with self.assertRaises(ValueError): + c.steps_sql("x' OR 1=1 --", 3) + + def test_days_out_of_range_is_refused(self): + with self.assertRaises(ValueError): + c.steps_sql("Build LAGVM (dev)", 0) + + +class Report(unittest.TestCase): + def test_parses_and_renders(self): + rows = c.parse_tsv(TSV) + out = c.report("Build LAGVM (dev)", 3, rows, "https://example/x") + self.assertIn("| Build LAGVM System | 4 | 1563 | 26.1 | 1500 | 1600 |", out) + + def test_empty_result_is_an_error_not_an_empty_table(self): + with self.assertRaises(ValueError): + c.parse_tsv("\n") + + def test_wrong_columns_is_an_error(self): + with self.assertRaises(ValueError): + c.parse_tsv("\nfoo\tbar\n1\t2\n") + + def test_timeline_uses_job_id(self): + self.assertIn("[job_id=253261575]", c.timeline_sql("Build LAGVM (dev)", 253261575, 3)) + + +if __name__ == "__main__": + unittest.main() From 49e8fbcd64d8359a6b00ce633055c4b49ee6b9b9 Mon Sep 17 00:00:00 2001 From: Edbert Chan <1916223+EdbertChan@users.noreply.github.com> Date: Mon, 12 Oct 2026 12:53:28 -0700 Subject: [PATCH 4/4] Plan first: build an approved mock before planning data collection for readable output Co-Authored-By: Claude Sonnet 5.5 Change-Id: I3e4aace7fa476750c7f5c3f9e64f4ba39b39c8b4 --- product/skills/plan-first/SKILL.md | 11 +++++++++++ .../plan-first/tests/fires_mock_first_example.md | 14 ++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 product/skills/plan-first/tests/fires_mock_first_example.md 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.