Skip to content

feat(rules): add 5 anti-foot-shoot block rules (Tier 1)#17

Merged
lapc506 merged 2 commits into
mainfrom
feat/hooks-tier-1-foot-shoot-prevention
May 10, 2026
Merged

feat(rules): add 5 anti-foot-shoot block rules (Tier 1)#17
lapc506 merged 2 commits into
mainfrom
feat/hooks-tier-1-foot-shoot-prevention

Conversation

@lapc506
Copy link
Copy Markdown
Collaborator

@lapc506 lapc506 commented May 10, 2026

Summary

Adds a new Anti foot-shoot (Tier 1) rule family — five block rules
covering the high-cost mistakes the team has already paid for. Each rule
maps 1:1 to a feedback memory whose intent is "never do this", not "be
careful".

The 5 new rules

Rule Action Tool Memory
block-playwright-headless block Bash feedback_no_headless.md
block-git-force-push-no-lease block Bash feedback_resolve_merge_conflicts.md
block-git-rebase-skip block Bash feedback_resolve_merge_conflicts.md
block-standup-not-in-escritorio block Edit/Write/MultiEdit feedback_standup_desktop.md
block-goodnight-not-in-escritorio block Edit/Write/MultiEdit feedback_goodnight_desktop.md

Each rule ships a kebab-case bypass_marker for the rare intentional
case (e.g. an explicit CI run for the playwright rule, an acknowledged
data-loss for the rebase-skip rule, etc.).

Coverage notes

  • block-playwright-headless — matches playwright test after npx,
    pnpm, yarn, or bare. Requires --headed / --ui / --debug on
    the same line, otherwise blocks. `\b` boundaries on playwright
    exclude unrelated tools like playwright-extra-cli.

  • block-git-force-push-no-lease — positive match on -f or
    --force (with proper word boundaries that already exclude
    --force-with-lease), plus a defense-in-depth not_pattern on
    --force-with-lease to abort the fire even in pathological orderings
    like `git push -f --force-with-lease`.

  • block-git-rebase-skip — exact match on git rebase --skip;
    --continue and --abort are unaffected.

  • block-{standup,goodnight}-not-in-escritoriopattern matches
    the filename and not_pattern excludes ~/Escritorio (Spanish
    locale) and ~/Desktop (English / aliasing) on the same file_path
    condition.

Test count

```
Before: 99 / 99 passed (19 rules)
After: 128 / 128 passed (24 rules, +29 new tests)
```

Each rule covers positive matches across realistic invocations,
negative matches that should NOT fire, and bypass-marker behavior.

Version bump

`1.8.0 -> 1.9.0`

Per semver, adding new enforcement rules is a feature add -> minor bump.
Bumped in:

  • `package.json`
  • `.claude-plugin/plugin.json`
  • `.claude-plugin/marketplace.json` (top-level + nested plugin entry)

Test plan

  • `npm run build-rules` regenerates `rules.json` (19 -> 24 rules)
  • `npm run test-hooks` 128 / 128 pass
  • CI (Blacksmith runner) — `npm run test-hooks` + `git diff hooks/rules/rules.json` clean
  • Greptile review -> 5 / 5

Notes for reviewers

  • This is the second of three sequential PRs that close the audit of
    feedback memories vs. existing rules. The first (feat(rules): add warn-curl-mutating-supabase-rest (close feedback_scripts_not_db.md) #16) added the
    curl-supabase-rest warn rule; this PR adds the 5 hard-stop rules; the
    next will add 6 discipline warns.
  • The 5 rules use existing fields (command, file_path) and existing
    matcher semantics — no schema or dispatcher changes.
  • The git rules deliberately require word boundaries / explicit flags so
    `-fu`, `-force` (single dash), and similar mistypes do NOT
    accidentally fire.

OpenSpec: `openspec/changes/2026-05-hooks-tier-1-foot-shoot-prevention/`
(proposal, design, tasks).

Created by Claude Code on behalf of @lapc506

Claude Code

Adds a new rule family - Anti foot-shoot (Tier 1) - covering the high-cost
mistakes the team has already paid for. Each rule maps 1:1 to a feedback
memory whose intent is "never do this", and ships a bypass marker for
the rare cases where the action is intentional.

The 5 new rules:

| Rule | Action | Tool | Memory |
|---|---|---|---|
| block-playwright-headless | block | Bash | feedback_no_headless.md |
| block-git-force-push-no-lease | block | Bash | feedback_resolve_merge_conflicts.md |
| block-git-rebase-skip | block | Bash | feedback_resolve_merge_conflicts.md |
| block-standup-not-in-escritorio | block | Edit/Write/MultiEdit | feedback_standup_desktop.md |
| block-goodnight-not-in-escritorio | block | Edit/Write/MultiEdit | feedback_goodnight_desktop.md |

Coverage notes:

- block-playwright-headless: matches `playwright test` (npx, pnpm, yarn,
  bare) and requires --headed / --ui / --debug on the same line.
- block-git-force-push-no-lease: positive match on `-f` or `--force` plus
  defense-in-depth negative match on `--force-with-lease` for
  pathological orderings like `git push -f --force-with-lease`.
- block-git-rebase-skip: matches `git rebase --skip` exactly; --continue
  and --abort are unaffected.
- block-{standup,goodnight}-not-in-escritorio: pattern + not_pattern on
  the same file_path field. Both `~/Escritorio` (Spanish locale) and
  `~/Desktop` (English / aliasing) are accepted.

Tests: 99 -> 128 passing (+29 new). Each rule covers positive matches,
negative matches, and bypass behavior.

Version bump: 1.8.0 -> 1.9.0 in package.json, plugin.json, and
marketplace.json (top-level + nested plugin entry).
@lapc506
Copy link
Copy Markdown
Collaborator Author

lapc506 commented May 10, 2026

@greptile review

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 10, 2026

Greptile Summary

This PR adds five block-action hook rules (Tier 1 "anti-foot-shoot") covering four feedback memories: headless Playwright runs, bare git push --force, git rebase --skip, and standup/goodnight files written outside ~/Escritorio. All three concerns raised in the previous round of reviews are resolved in this revision.

  • block-playwright-headless — positive \\bplaywright test\\b + negative (--headed|--ui|--debug); pnpm dlx / yarn dlx cases now explicitly tested.
  • block-git-force-push-no-lease — primary pattern already excludes --force-with-lease via ([[:space:]]|$) anchor; defense-in-depth not_pattern handles pathological orderings like git push -f ... --force-with-lease, with that exact test now present.
  • block-{standup,goodnight}-not-in-escritorio — bypass marker reachability for Edit and MultiEdit (via new_string and edits[].new_string) is now covered by explicit tests, and the YAML comment documents that the dispatcher scans the entire raw tool_input JSON.

Confidence Score: 5/5

All five new block rules are well-scoped, have comprehensive tests covering positive matches, negative matches, and bypass-marker behavior, and raise no correctness concerns. Safe to merge.

Every concern raised in the prior review round — bypass marker reachability for Edit/MultiEdit, the missing defense-in-depth test for git push -f ... --force-with-lease, and pnpm dlx / yarn dlx coverage — has been addressed with explicit test cases and explanatory comments. The regex patterns are logically sound: word boundaries correctly exclude false matches, and the --force([[:space:]]|$) anchor cleanly distinguishes --force from --force-with-lease and --force-if-includes. No regressions are expected in the 99 pre-existing tests given the purely additive nature of the change.

No files require special attention.

Important Files Changed

Filename Overview
hooks/rules/rules.yaml Five new block rules added: playwright headless, git force-push, git rebase --skip, standup/goodnight desktop placement. All previous review concerns addressed — bypass marker reachability for Edit/MultiEdit now has explicit tests, defense-in-depth test added, pnpm/yarn dlx cases covered.
hooks/rules/rules.json Regenerated artifact from rules.yaml via npm run build-rules; structurally mirrors the YAML additions with no discrepancies found.
hooks/rules/README.md Documentation updated to list the new "Anti foot-shoot (Tier 1)" family with accurate descriptions of all five rules.
.claude-plugin/marketplace.json Both top-level and nested plugin version bumped from 1.8.0 to 1.9.0 consistently.
package.json Version bumped from 1.8.0 to 1.9.0, consistent with all other manifests.
openspec/changes/2026-05-hooks-tier-1-foot-shoot-prevention/design.md Design document accurately describes the regex logic for all five rules; no discrepancies with the actual implementation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Tool Call Intercepted] --> B{Check applies_to}
    B -->|Bash| C1[block-playwright-headless]
    B -->|Bash| C2[block-git-force-push-no-lease]
    B -->|Bash| C3[block-git-rebase-skip]
    B -->|Edit / Write / MultiEdit| C4[block-standup-not-in-escritorio]
    B -->|Edit / Write / MultiEdit| C5[block-goodnight-not-in-escritorio]
    C1 --> D1{command matches playwright test?}
    D1 -->|No| PASS1[Allow]
    D1 -->|Yes| E1{--headed / --ui / --debug present?}
    E1 -->|Yes| PASS1
    E1 -->|No| BLOCK1[Block exit 2]
    C2 --> D2{git push -f / --force?}
    D2 -->|No| PASS2[Allow]
    D2 -->|Yes| E2{--force-with-lease present?}
    E2 -->|Yes| PASS2
    E2 -->|No| BLOCK2[Block exit 2]
    C3 --> D3{git rebase --skip?}
    D3 -->|No| PASS3[Allow]
    D3 -->|Yes| BLOCK3[Block exit 2]
    C4 --> D4{file_path matches daily-standup*.md?}
    D4 -->|No| PASS4[Allow]
    D4 -->|Yes| E4{under /Escritorio/ or /Desktop/?}
    E4 -->|Yes| PASS4
    E4 -->|No| BLOCK4[Block exit 2]
    C5 --> D5{file_path matches next-day-* / goodnight-*?}
    D5 -->|No| PASS5[Allow]
    D5 -->|Yes| E5{under /Escritorio/ or /Desktop/?}
    E5 -->|Yes| PASS5
    E5 -->|No| BLOCK5[Block exit 2]
Loading

Reviews (2): Last reviewed commit: "fix(rules): close 3 Greptile findings on..." | Re-trigger Greptile

Comment thread hooks/rules/rules.yaml
Comment thread hooks/rules/rules.yaml
1. **Edit/MultiEdit bypass reachability documented and tested**
   (block-standup-not-in-escritorio + block-goodnight-not-in-escritorio).
   The dispatcher's bypass scan already runs against the full raw
   tool_input JSON via `parse-input.sh INPUT_RAW`, so a marker in any
   string-valued field (`new_string`, `old_string`, `edits[].new_string`)
   short-circuits the rule. Add explicit `allows-bypass-marker-edit-tool`
   and `allows-bypass-marker-multiedit-tool` tests for both rules and a
   clarifying comment so a future maintainer doesn't reintroduce the
   reachability concern. Renames `allows-bypass-marker` ->
   `allows-bypass-marker-write-tool` to keep the tool-shape symmetry.

2. **Combined --force + --force-with-lease defense-in-depth tested**
   (block-git-force-push-no-lease). Add
   `allows-force-with-lease-and-force-combined` with the exact
   pathological-ordering command the design doc calls out, locking in
   the not_pattern safety net against future regex refactors.

3. **pnpm dlx / yarn dlx playwright invocations tested**
   (block-playwright-headless). Add `blocks-pnpm-dlx-playwright-headless`,
   `blocks-yarn-dlx-playwright-headless`, and
   `allows-pnpm-dlx-playwright-headed`. The pattern already covers these
   forms; the tests pin that behavior.

Tests: 128 -> 136 passing (+8).
@lapc506
Copy link
Copy Markdown
Collaborator Author

lapc506 commented May 10, 2026

Round 2: addressed all 3 Greptile findings. See commit b0d1d9c.

  1. Edit/MultiEdit bypass reachability — added allows-bypass-marker-edit-tool and allows-bypass-marker-multiedit-tool for both standup and goodnight rules with a clarifying comment that the dispatcher scans INPUT_RAW (full JSON) so any string-valued field is reachable.
  2. Force-with-lease combined defense-in-depth tested — added allows-force-with-lease-and-force-combined for git push -f origin main --force-with-lease.
  3. pnpm dlx / yarn dlx playwright tests — added blocks-pnpm-dlx-playwright-headless, blocks-yarn-dlx-playwright-headless, allows-pnpm-dlx-playwright-headed.

Tests: 128 -> 136 passing.

@greptile review

@lapc506 lapc506 merged commit e5666c8 into main May 10, 2026
2 checks passed
@lapc506 lapc506 deleted the feat/hooks-tier-1-foot-shoot-prevention branch May 10, 2026 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant