diff --git a/templates/hookify-rules/README.md b/templates/hookify-rules/README.md index 0fa8b8c..f7beb38 100644 --- a/templates/hookify-rules/README.md +++ b/templates/hookify-rules/README.md @@ -44,3 +44,8 @@ Your message when this rule triggers. | `fact-check-template` | block | Template for catching specific misattributions or wrong facts | | `public-repo-firewall` | block | Personal names/data leaking into public repos | | `dangerous-rm` | block | `rm -rf` commands without confirmation | + +## Authoring guide and regression harness + +- **Authoring cheatsheet:** `templates/rules/hookify-authoring.md` covers operators, supported fields, the negative-lookahead pattern (no `regex_not_match` operator exists), YAML quoting gotchas (`[` and `\` patterns must be single-quoted), companion PreToolUse hooks for logic that goes beyond regex, and how to test a rule manually. +- **Regression harness:** `templates/scripts/hookify-rule-tests.py` is a copy-then-edit script for running a list of `(rule_name, tool_type, path, content, expect_fire, label)` tuples through the hookify subprocess. Catches three classes of bug: (1) non-existent operators that silently never fire, (2) YAML parse errors that drop a rule at load time, (3) pattern logic regressions after a rule edit. Run it after any rule change OR after upgrading the hookify plugin. Exit 0 means all pass. diff --git a/templates/rules/hookify-authoring.md b/templates/rules/hookify-authoring.md index 036fea7..cbebdc8 100644 --- a/templates/rules/hookify-authoring.md +++ b/templates/rules/hookify-authoring.md @@ -23,21 +23,21 @@ All conditions are ANDed. To OR conditions, write two rules with the same messag ### Supported operators (core/rule_engine.py _check_condition) -- `regex_match` — Python `re.search` -- `contains` — literal substring (no regex) -- `equals` — exact string match -- `not_contains` — literal substring, negated +- `regex_match`: Python `re.search` +- `contains`: literal substring (no regex) +- `equals`: exact string match +- `not_contains`: literal substring, negated - `starts_with`, `ends_with` -**There is NO `regex_not_match`, `regex_not`, or negated-regex operator.** Using one makes the condition always return False → rule never fires. To express "regex should NOT match", invert the pattern with a negative lookahead (tempered greedy token) inside a single `regex_match`. Example: +**There is NO `regex_not_match`, `regex_not`, or negated-regex operator.** Using one makes the condition always return False, so the rule never fires. To express "regex should NOT match", invert the pattern with a negative lookahead (tempered greedy token) inside a single `regex_match`. Example: ```yaml -# WRONG — rule silently never fires +# WRONG: rule silently never fires - field: content operator: regex_not_match pattern: '- \[ \][^\n]*\[\[[^\]]+\]\]' -# RIGHT — matches tasks WITHOUT a wikilink +# RIGHT: matches tasks WITHOUT a wikilink - field: content operator: regex_match pattern: '- \[ \](?:(?!\[\[|\n).){0,300}$' @@ -45,28 +45,59 @@ All conditions are ANDed. To OR conditions, write two rules with the same messag The `(?:(?!X).)*` pattern is a tempered greedy token: "any char as long as the next chars aren't X". Here X is `\[\[` (wikilink start) or `\n` (keep match on one line). +### Negative lookahead with `re.search`: anchor with `^` + +`re.search()` tries every position in the string. A pattern like `(?!.*/Archive/).*foo` does NOT actually exclude `/Archive/` paths, because the engine can skip past the negative lookahead and match later. Anchor with `^` so the lookahead evaluates from position 0: + +```yaml +# WRONG: Archive paths still match +pattern: '(?!.*/Archive/).*Sensitive/.+\.md$' + +# RIGHT: anchored, Archive correctly excluded +pattern: '^(?!.*/Archive/).*Sensitive/.+\.md$' +``` + ### Supported fields by event/tool - `file` event (Write, Edit, MultiEdit): `file_path`, `content`, `new_text`/`new_string`, `old_text`/`old_string` - - `content` on Write returns `tool_input.content`; on Edit returns `tool_input.new_string`. Use `content` if you want both. - - `new_text` on Write returns empty unless content also matches. Prefer `content`. + - `content` on Write returns `tool_input.content`; on Edit returns `tool_input.new_string`. Use `content` if you want both in one field. + - `new_text` is now equivalent to `content` on Write (falls back to `tool_input.content` when `new_string` is absent). Older hookify versions returned empty for `new_text` on Write. If your rule was authored against that bug, swap to `content` for clarity. - `bash` event: `command` - `prompt` event (UserPromptSubmit): `user_prompt` - `stop` event: `reason`, `transcript` (full transcript file contents) ### YAML gotchas -- **Double-quoted strings double-escape regex backslashes.** `"- \\[ \\]"` in YAML gets loaded as `'- \\[ \\]'` (four backslashes in the regex engine's view = literal `\[`, not `[`). **Always single-quote regex patterns** or use unquoted scalars. Single-quoted: `'- \[ \]'` loads as `- \[ \]` verbatim. -- Unquoted patterns with `:` or `#` break YAML. If in doubt, single-quote. -- `$` inside single-quoted strings is literal (good for regex end-of-line). +- **Always single-quote regex patterns.** Single-quoted YAML strings store characters verbatim: backslashes, brackets, dollar signs, all literal. This is the only quoting style that's safe for regex. +- **Unquoted patterns starting with `[` break YAML.** `pattern: [A-Z].*` makes YAML read `[A-Z]` as a flow sequence (a list with one item), not a string. Result: parser error, rule skipped at load time. Always single-quote when the pattern contains `[`. +- **Unquoted patterns starting with `\` break YAML.** `pattern: \[owner\]` produces a YAML error on the bare backslash. Single-quote it. +- **Unquoted patterns with `:` or `#` break YAML.** Both are reserved characters. When in doubt, single-quote. +- **Double-quoted strings process escape sequences.** `"\d{8}"` becomes `\d{8}` (correct for regex), but it's confusing because the source file shows `\d` while the loader stores `\d`. Single quotes are easier to reason about. +- `$` inside single-quoted strings is literal (correct for regex end-of-line). + +If your rule isn't firing and the test harness says it's missing from the loaded rule list, check stderr for a YAML parse warning. That's almost always an unquoted pattern. ### Loading behavior - `load_rules()` in `core/config_loader.py` uses relative glob `.claude/hookify.*.local.md`. **Rules only load when CWD = vault root.** Worktrees under `.claude/worktrees/*/` have CWD = worktree root, which has its own `.claude/`. Rules in the vault-root `.claude/` do NOT apply to worktrees unless duplicated or symlinked. -- `event` filter is strict — a rule with `event: file` never fires on `bash` events and vice-versa. +- `event` filter is strict. A rule with `event: file` never fires on `bash` events and vice-versa. + +### Companion PreToolUse hooks + +Sometimes a rule needs more than regex matching. For example, "warn when writing to a sensitive folder, but only if the file isn't already protected by frontmatter." A hookify rule can detect the path; the protection check needs to read the file or another config. + +For these cases, write a companion PreToolUse Python hook in `~/.claude/hooks/` and register it in `settings.json`. The hook reads `tool_input` from stdin, runs whatever logic it needs (file reads, config lookups, regex matching), and outputs either: + +- `{}`: silent passthrough +- `{"systemMessage": "..."}`: warn (no block) +- `{"hookSpecificOutput": {"hookEventName": "PreToolUse", "permissionDecision": "deny", "permissionDecisionReason": "..."}}`: block + +**MCP tools cannot be called from a hook subprocess.** MCP servers run in the parent Claude process, not in hook children. If you need MCP-equivalent logic, replicate it by reading the underlying config files directly. Pattern: a hook that reads `agents/some-mcp/config.yaml` and emits a warning is more reliable than one that tries to invoke `mcp__some-mcp__some_tool` from the subprocess (which will silently fail). ### Testing a rule before committing +Single-rule probe: + ```bash cd "${VAULT_ROOT}" echo '{"tool_name":"Write","tool_input":{"file_path":"","content":""}}' \ @@ -80,6 +111,8 @@ Expected output: Test at least: positive case, negative case, wrong-file-path case, wrong-tool case. If you added a negative lookahead, also test a mixed-content input (one line matches, one line doesn't) to confirm per-line behavior. +**Full regression harness:** `python3 templates/scripts/hookify-rule-tests.py` (after copying it to a location where it can find your rules). The harness defines a list of `(rule_name, tool_type, path, content, expect_fire, label)` tuples and pipes each into the hookify subprocess, asserting that fire-expectation matches reality. Run after any rule change OR any hookify engine update. See the script header for usage. + --- Codified after a rule was written with a non-existent `regex_not_match` operator and silently never fired. Fix: single `regex_match` with negative lookahead. This cheatsheet exists so the same class of bug is caught at authoring time, not at runtime when content slips past the gate. diff --git a/templates/scripts/hookify-rule-tests.py b/templates/scripts/hookify-rule-tests.py new file mode 100644 index 0000000..3650115 --- /dev/null +++ b/templates/scripts/hookify-rule-tests.py @@ -0,0 +1,214 @@ +#!/usr/bin/env python3 +"""Hookify rule regression harness (template). + +Pipes test payloads into the hookify PreToolUse hook and asserts each rule +either fires or stays silent as expected. Catches three classes of bug: + + 1. Rule references a non-existent operator (e.g. regex_not_match) → + condition silently always False → rule never fires. + 2. YAML parse errors from unquoted patterns → rule dropped at load time. + 3. Pattern logic regressions after a rule edit. + +USAGE + 1. Copy this file to your vault under scripts/ or ~/.claude/scripts/. + 2. Set VAULT_ROOT below to your vault's absolute path. + 3. Set HOOKIFY_PRETOOLUSE_PATH to the path of the hookify pretooluse.py + hook (the one your settings.json invokes). + 4. Edit TESTS to cover your rules. Each entry is: + (rule_name, tool_type, file_path, content, expect_fire, label) + `rule_name` is the `name:` field from the rule's frontmatter. The harness + greps the hook's stdout for that string to decide whether the rule fired. + 5. Run: `python3 hookify-rule-tests.py` from the vault root. + Exit 0 = all pass. Exit 1 = at least one failure. + +DESIGN NOTES +- Tests run in a subprocess so the hook sees a clean environment, including + CWD. The harness sets cwd=VAULT_ROOT because hookify's load_rules() uses a + relative glob `.claude/hookify.*.local.md`. +- Triggering strings inside content can self-trigger the rule under test. + When that happens, write the content via Path.write_text() to a temp file + and reference it indirectly, OR base64-encode the trigger and decode at + send time. Both approaches are fine; the included example uses plain text + for clarity. +- The harness does NOT modify the rule files. It only sends synthetic + tool_input payloads to the hook. + +EXIT CODES + 0: all tests pass + 1: one or more failures (details printed) + 2: configuration error (wrong VAULT_ROOT or missing hookify hook) +""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +from pathlib import Path + +# ── Configuration ───────────────────────────────────────────────────────────── +# Edit these two paths for your environment. + +VAULT_ROOT = Path(os.environ.get("VAULT_ROOT", Path.home() / "vault")) +HOOKIFY_PRETOOLUSE_PATH = Path( + os.environ.get( + "HOOKIFY_PRETOOLUSE_PATH", + Path.home() + / ".claude" + / "plugins" + / "local" + / "hookify-plugin" + / "hooks" + / "pretooluse.py", + ) +) + + +# ── Test definitions ────────────────────────────────────────────────────────── +# Each test: (rule_name, tool_type, file_path, content, expect_fire, label) +# +# rule_name: the `name:` field from your hookify rule's frontmatter +# tool_type: "Write" | "Edit" | "MultiEdit" | "Bash" +# file_path: absolute path the synthetic write would target +# content: for Write/Edit, the content the tool would write +# expect_fire: True if the rule should fire on this input, False if it should stay silent +# label: short description shown in test output + +# Example test list. Replace with tests for your own rules. +TESTS: list[tuple[str, str, str, str, bool, str]] = [ + # voice-no-exclamation should fire on exclamation mark in prose + ( + "warn-exclamation-marks", + "Write", + str(VAULT_ROOT / "Notes" / "draft.md"), + "This is great content! Ready to ship.", + True, + "exclamation in prose: fires", + ), + # voice-no-exclamation should NOT fire when content has no exclamation + ( + "warn-exclamation-marks", + "Write", + str(VAULT_ROOT / "Notes" / "draft.md"), + "This is great content. Ready to ship.", + False, + "no exclamation: silent", + ), + # no-duplicate-h1 should fire on `# Title` at start of content + # (the rule's pattern `^# .+` is anchored to string start, not line start) + ( + "no-duplicate-h1", + "Write", + str(VAULT_ROOT / "Notes" / "draft.md"), + "# Duplicate Title\n\nBody.", + True, + "H1 at content start: fires", + ), + # no-duplicate-h1 should NOT fire when content starts with H2 + ( + "no-duplicate-h1", + "Write", + str(VAULT_ROOT / "Notes" / "draft.md"), + "## Section\n\nBody.", + False, + "H2 only: silent", + ), +] + + +# ── Harness ──────────────────────────────────────────────────────────────────── +def run_test( + rule_name: str, + tool_type: str, + file_path: str, + content: str, + expect_fire: bool, + label: str, +) -> tuple[bool, str]: + """Run one test. Returns (passed, message).""" + if tool_type in ("Write",): + tool_input = {"file_path": file_path, "content": content} + elif tool_type in ("Edit",): + # Edit uses old_string/new_string; we put content in new_string + tool_input = {"file_path": file_path, "old_string": "", "new_string": content} + elif tool_type in ("MultiEdit",): + tool_input = { + "file_path": file_path, + "edits": [{"old_string": "", "new_string": content}], + } + elif tool_type == "Bash": + tool_input = {"command": content} + else: + return False, f"unknown tool_type: {tool_type}" + + payload = {"tool_name": tool_type, "tool_input": tool_input} + + try: + proc = subprocess.run( + ["python3", str(HOOKIFY_PRETOOLUSE_PATH)], + input=json.dumps(payload), + capture_output=True, + text=True, + cwd=str(VAULT_ROOT), + timeout=10, + ) + except subprocess.TimeoutExpired: + return False, f"hook timed out after 10s" + except FileNotFoundError: + return False, f"hookify hook not found at {HOOKIFY_PRETOOLUSE_PATH}" + + fired = rule_name in proc.stdout + passed = fired == expect_fire + + if passed: + return True, "" + if expect_fire: + return False, ( + f"expected fire, got silence. stdout: {proc.stdout[:200]!r} " + f"stderr: {proc.stderr[:200]!r}" + ) + return False, f"expected silence, got fire. stdout: {proc.stdout[:200]!r}" + + +def main() -> int: + # Sanity checks + if not VAULT_ROOT.exists(): + print(f"❌ VAULT_ROOT does not exist: {VAULT_ROOT}", file=sys.stderr) + print( + " Set VAULT_ROOT env var or edit the constant at the top of this file.", + file=sys.stderr, + ) + return 2 + if not HOOKIFY_PRETOOLUSE_PATH.exists(): + print( + f"❌ Hookify pretooluse hook not found: {HOOKIFY_PRETOOLUSE_PATH}", + file=sys.stderr, + ) + print( + " Set HOOKIFY_PRETOOLUSE_PATH env var or edit the constant.", + file=sys.stderr, + ) + return 2 + + print(f"Testing {len(TESTS)} hookify rule cases against {HOOKIFY_PRETOOLUSE_PATH}") + print(f"CWD: {VAULT_ROOT}\n") + + passed = 0 + failed = 0 + for rule_name, tool_type, file_path, content, expect_fire, label in TESTS: + ok, msg = run_test(rule_name, tool_type, file_path, content, expect_fire, label) + marker = "✓" if ok else "✗" + print(f" {marker} [{rule_name}] {label}") + if not ok: + print(f" {msg}") + failed += 1 + else: + passed += 1 + + print(f"\n{passed}/{len(TESTS)} passed, {failed} failed") + return 0 if failed == 0 else 1 + + +if __name__ == "__main__": + sys.exit(main())