anthropics / anthropics/claude-plugins-official

hookify: not_contains operator does literal substring match, not regex — breaks the shipped require-tests-stop example

Open
#5,602 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
36.3k
Forks
4.1k
Avg merge
2d 14h
Merged PRs (30d)
539

Description

### Summary

The `require-tests-stop.local.md` example shipped in `plugins/hookify/examples/` (reproduced in the README under "Example 3: Require Tests Before Stopping") can never pass. The Stop hook it defines blocks unconditionally, regardless of whether tests were actually run.

### Repro

The example rule:

```yaml
operator: not_contains
pattern: npm test|pytest|cargo test
```

Enable it, then trigger a Stop event with a transcript that *does* contain a passing test run (e.g. `"npm test ran and passed"`). Expected: hook allows exit. Actual: hook still blocks.

### Root cause

`core/rule_engine.py`, in the operator dispatch (~line 168-173):

```python
elif operator == 'contains':
return pattern in field_value
elif operator == 'equals':
return pattern == field_value
elif operator == 'not_contains':
return pattern not in field_value
```

Only `regex_match` compiles `pattern` as a regex. `contains`/`not_contains`/`equals` treat `pattern` as a literal string. The example's `pattern: npm test|pytest|cargo test` is written expecting regex alternation (match any of the three), but under `not_contains` it's checked as one literal 26-character string — including the pipe characters — which will essentially never appear verbatim in a real transcript. So `not_contains` evaluates `True` unconditionally, and the rule blocks every Stop event.

### Impact

Anyone who enables this documented example gets a gate that looks like enforcement but isn't — arguably worse than no gate, since it gives false confidence that tests are being required.

### Verified

Reproduced by executing `hooks/stop.py` directly against synthetic transcripts (not just read from source): blocks with no test evidence (correct), still incorrectly blocks with `"npm test ran and passed"` or `"pytest"` present in the transcript.

### Fix

Since `_rule_matches` requires ALL conditions in a rule to match (AND semantics, `rule_engine.py:120-125`), and the README already documents this exact multi-condition pattern elsewhere ("Multiple Conditions" section), the example can be fixed with no code change — replace the single condition with three `not_contains` conditions ANDed together (De Morgan's law: `NOT(A or B or C)` = `NOT A AND NOT B AND NOT C`):

```yaml
conditions:
- field: transcript
operator: not_contains
pattern: npm test
- field: transcript
operator: not_contains
pattern: pytest
- field: transcript
operator: not_contains
pattern: cargo test
```

I have this fix verified and staged (branch `fix/hookify-require-tests-not-contains-alternation` on my fork), but this repo's automation closes PRs from non-Anthropic-team-members on sight, so filing as an issue instead in case a maintainer wants to pick it up or point me at the right contribution path.

### Minor, unrelated

`plugins/hookify/.claude-plugin/plugin.json` has no `version` field, unlike sibling plugins in the marketplace.

Contributor guide

No contributing guide indexed for this repository

Research direction

Compare plugins/hookify/examples/require-tests-stop.local.md with the README's “Example 3: Require Tests Before Stopping,” then inspect hooks/stop.py and core/rule_engine.py. Run hooks/stop.py with synthetic transcripts containing no test evidence and each listed test command. Done means the example blocks without evidence, allows a transcript containing any one of the listed commands, and the example and README agree.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, yaml
Domain
documentation
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.