hooks: add simplify-ignore hook for code block protection

z23cc · z23cc · commit d5c154989555 · 2026-04-07T05:16:12.000+08:00
- Annotation-based protection: simplify-ignore-start/end markers
- Replaces blocks with BLOCK_&lt;hash&gt; placeholders before model reads
- Expands placeholders back on edit/write, restores on stop
- Supports //, /* */, #, &lt;!-- --&gt; comment styles
- Opt-in only via .claude/settings.json (not hooks.json)
- Documentation in hooks/SIMPLIFY-IGNORE.md

Task: fn-136-borrow-agent-skills-patterns-anti.8
diff --git a/agents/cross-model-reviewer.md b/agents/cross-model-reviewer.md
@@ -63,11 +63,50 @@ The `flowctl_review` MCP tool exposes cross-model review:
 ## Review Types
 
 ### ReviewFinding
-Individual issue with severity (critical/warning/info), category, description, and optional file/line.
+Individual issue with severity, category, dimension, description, and optional file/line.
+
+**Severity classification** (aligned with quality-auditor dimensions):
+
+| Severity | Meaning | Ship impact |
+|----------|---------|-------------|
+| `Critical` | Correctness failure, security vulnerability, data loss risk | Blocks ship |
+| `Important` | Architecture violation, readability problem, missing test coverage | Must fix before/shortly after ship |
+| `Suggestion` | Performance improvement, naming nit, minor simplification | Optional |
+
+**Dimension tags** — each finding maps to one of five review dimensions:
+- **Correctness** — edge cases, race conditions, state inconsistencies, off-by-one
+- **Readability** — naming consistency, control flow clarity, module organization
+- **Architecture** — pattern adherence, module boundaries, abstraction levels, dependency direction
+- **Security** — injection, auth, data exposure, dependencies
+- **Performance** — N+1, unbounded loops, blocking operations
+
+### Structured Output Format
+
+Each model's review MUST produce findings in this structure:
+
+```json
+{
+  "verdict": "SHIP | NEEDS_WORK | ABSTAIN",
+  "confidence": 0.0-1.0,
+  "findings": [
+    {
+      "severity": "Critical | Important | Suggestion",
+      "dimension": "Correctness | Readability | Architecture | Security | Performance",
+      "file": "path/to/file.rs",
+      "line": 42,
+      "description": "What is wrong",
+      "suggestion": "How to fix it"
+    }
+  ],
+  "positives": ["At least one positive observation"]
+}
+```
+
+The consensus algorithm uses severity to weight disagreements: a `Critical` finding from any model blocks SHIP regardless of the other model's verdict. `Suggestion`-only findings do not block.
 
 ### ReviewVerdict
 - **SHIP**: Code is ready
-- **NEEDS_WORK**: Code needs fixes
+- **NEEDS_WORK**: Code needs fixes (at least one Critical or multiple Important findings)
 - **ABSTAIN**: Model cannot determine (excluded from consensus)
 
 ### ConsensusResult
diff --git a/agents/quality-auditor.md b/agents/quality-auditor.md
@@ -17,7 +17,7 @@ You're invoked after implementation, before shipping. Review the changes and fla
 
 ## Audit Strategy
 
-### 1. Get the Diff
+### Step 0. Get the Diff + Quick Scan
 ```bash
 # What changed?
 git diff main --stat
@@ -27,72 +27,91 @@ git diff main --name-only
 git diff main
 ```
 
-### 2. Quick Scan (find obvious issues fast)
+Scan the diff for obvious issues before deep review:
 - **Secrets**: API keys, passwords, tokens in code
 - **Debug code**: console.log, debugger, TODO/FIXME
 - **Commented code**: Dead code that should be deleted
 - **Large files**: Accidentally committed binaries, logs
 
-### 3. Correctness Review
-- Does the code match the stated intent?
-- Are there off-by-one errors, wrong operators, inverted conditions?
-- Do error paths actually handle errors?
-- Are promises/async properly awaited?
+### Five-Dimension Review
+
+Evaluate every change across all five dimensions. Each dimension produces findings tagged with severity (see Output Format).
 
-### 4. Security Scan
+### 1. Correctness
+- Does the code match the stated intent?
+- Edge cases: off-by-one errors, wrong operators, inverted conditions
+- Race conditions and state inconsistencies in concurrent code
+- Error paths: do they actually handle errors or silently swallow?
+- Async correctness: are promises/async properly awaited?
+- Are new code paths tested? Do tests assert behavior (not just run)?
+- Are error paths and edge cases from gap analysis covered?
+
+### 2. Readability
+- Naming consistency: do new names follow existing conventions?
+- Control flow clarity: can you follow the logic without mental gymnastics?
+- Module organization: are things in the right files/directories?
+- Could this be simpler? Is there duplicated code that should be extracted?
+- Are there unnecessary abstractions or over-engineering for hypothetical futures?
+
+### 3. Architecture
+- Pattern adherence: does the change follow established project patterns?
+- Module boundaries: are concerns properly separated?
+- Abstraction levels: is the right level of abstraction used?
+- Dependency direction: do dependencies flow in the correct direction?
+
+### 4. Security
 - **Injection**: SQL, XSS, command injection vectors
 - **Auth/AuthZ**: Are permissions checked? Can they be bypassed?
 - **Data exposure**: Is sensitive data logged, leaked, or over-exposed?
 - **Dependencies**: Any known vulnerable packages added?
 
-### 5. Simplicity Check
-- Could this be simpler?
-- Is there duplicated code that should be extracted?
-- Are there unnecessary abstractions?
-- Over-engineering for hypothetical future needs?
-
-### 6. Test Coverage
-- Are new code paths tested?
-- Do tests actually assert behavior (not just run)?
-- Are edge cases from gap analysis covered?
-- Are error paths tested?
-
-### 7. Performance Red Flags
-- N+1 queries or O(n²) loops
-- Unbounded data fetching
-- Missing pagination/limits
+### 5. Performance
+- N+1 queries or O(n^2) loops
+- Unbounded data fetching, missing pagination/limits
 - Blocking operations on hot paths
+- Resource leaks (unclosed handles, missing cleanup)
 
 ## Output Format
 
+Every finding MUST carry a severity prefix. Use exactly these four levels:
+
+- **`Critical:`** — Blocks ship. Could cause outage, data loss, security breach, or correctness failure.
+- **`Important:`** — Must fix before or shortly after ship. Significant quality, readability, or architecture issue.
+- **`Nit:`** — Optional improvement. Style, naming, minor simplification.
+- **`FYI`** — Informational. Context for the author, no action required.
+
+Every review MUST include a "What's Good" section with at least one positive observation. Acknowledge patterns followed, good design decisions, thorough error handling, or clean naming.
+
 ```markdown
 ## Quality Audit: [Branch/Feature]
 
 ### Summary
 - Files changed: N
+- Dimensions reviewed: Correctness, Readability, Architecture, Security, Performance
 - Risk level: Low / Medium / High
 - Ship recommendation: ✅ Ship / ⚠️ Fix first / ❌ Major rework
 
-### Critical (MUST fix before shipping)
-- **[File:line]**: [Issue]
+### Critical (blocks ship)
+- **Critical:** [File:line] — [Issue] (Dimension: Correctness/Security/etc.)
   - Risk: [What could go wrong]
   - Fix: [Specific suggestion]
 
-### Should Fix (High priority)
-- **[File:line]**: [Issue]
-  - [Brief fix suggestion]
+### Important (must fix)
+- **Important:** [File:line] — [Issue] (Dimension: ...)
+  - Fix: [Brief fix suggestion]
 
-### Consider (Nice to have)
-- [Minor improvement suggestion]
+### Nit (optional)
+- **Nit:** [File:line] — [Improvement suggestion]
+
+### FYI
+- **FYI** [Informational observation]
 
 ### Test Gaps
 - [ ] [Untested scenario]
 
-### Security Notes
-- [Any security observations]
-
 ### What's Good
-- [Positive observations - patterns followed, good decisions]
+- [At least one positive observation — patterns followed, good decisions, clean code]
+- [Additional positives as warranted]
 ```
 
 ## Rules
diff --git a/hooks/SIMPLIFY-IGNORE.md b/hooks/SIMPLIFY-IGNORE.md
@@ -0,0 +1,89 @@
+# simplify-ignore hook
+
+Annotation-based code protection for Claude Code. Mark code blocks with `simplify-ignore-start` / `simplify-ignore-end` annotations and the hook replaces them with `BLOCK_<hash>` placeholders before the model sees them. On edit or write, placeholders are expanded back to the original code. On session stop, all files are fully restored.
+
+## Setup
+
+Add to your project's `.claude/settings.json` (NOT `hooks/hooks.json` -- this is opt-in per project):
+
+```json
+{
+  "hooks": {
+    "PreToolUse": [
+      {
+        "matcher": "Read",
+        "hooks": [{ "type": "command", "command": "bash ${CLAUDE_PROJECT_DIR}/hooks/simplify-ignore.sh" }]
+      }
+    ],
+    "PostToolUse": [
+      {
+        "matcher": "Edit|Write",
+        "hooks": [{ "type": "command", "command": "bash ${CLAUDE_PROJECT_DIR}/hooks/simplify-ignore.sh" }]
+      }
+    ],
+    "Stop": [
+      {
+        "hooks": [{ "type": "command", "command": "bash ${CLAUDE_PROJECT_DIR}/hooks/simplify-ignore.sh" }]
+      }
+    ]
+  }
+}
+```
+
+Add `.claude/.simplify-ignore-cache/` to your `.gitignore`.
+
+## Annotation syntax
+
+```js
+/* simplify-ignore-start: perf-critical */
+// manually unrolled XOR -- 3x faster than a loop
+result[0] = buf[0] ^ key[0];
+result[1] = buf[1] ^ key[1];
+/* simplify-ignore-end */
+```
+
+The reason after the colon is optional. It appears in the placeholder (`BLOCK_a1b2c3d4: perf-critical`) so the model knows why the block is hidden.
+
+## Supported comment styles
+
+| Style | Example |
+|-------|---------|
+| `/* */` | `/* simplify-ignore-start: reason */` |
+| `//` | `// simplify-ignore-start: reason` |
+| `#` | `# simplify-ignore-start: reason` |
+| `<!-- -->` | `<!-- simplify-ignore-start: reason -->` |
+
+Multiple blocks per file and single-line blocks (start + end on same line) are supported. Placeholders preserve the original comment prefix/suffix.
+
+## How it works
+
+| Hook event | Action |
+|------------|--------|
+| PreToolUse Read | Backs up file, replaces annotated blocks with `BLOCK_<hash>` placeholders |
+| PostToolUse Edit/Write | Expands placeholders back, saves model changes, re-filters |
+| Stop | Restores all files from backup |
+
+Block hashes are 8 hex chars from `shasum`/`sha1sum` of the block content, making round-trips unambiguous.
+
+## Crash recovery
+
+If Claude Code exits without triggering the Stop hook, files may still have `BLOCK_<hash>` placeholders. Restore manually:
+
+```bash
+echo '{}' | bash hooks/simplify-ignore.sh
+```
+
+Backups are in `.claude/.simplify-ignore-cache/` within your project directory.
+
+## Known limitations
+
+- **Single-line blocks hide the entire line.** Use dedicated lines for annotations.
+- **Suffix detection covers `*/` and `-->` only.** Non-standard template comment closers (ERB, Blade) may not work; use `#` or `//` instead.
+- **Fuzzy fallback on altered placeholders.** If the model changes a placeholder's reason text, the hook tries a hash-only match which may leave cosmetic debris.
+- **File renaming leaves placeholders.** Renamed files retain placeholders; originals are saved as `.recovered` on stop.
+
+## Requirements
+
+- `jq`
+- `shasum` or `sha1sum` (auto-detected)
+- Bash 3.2+
diff --git a/hooks/simplify-ignore.sh b/hooks/simplify-ignore.sh
