feat(rules): add 6 Tier 2 discipline rules + extend schema with old_string

[lapc506]

Summary

Adds the Tier 2 discipline rule family — five warn rules and one
block rule that nudge authors toward documented patterns from feedback
memories without blocking the operation. The one block rule
(block-sibling-suffix-storage) is intentional because the storage-layout
error is hard to undo once committed.

This PR also extends the rule schema with a new old_string field so
the new warn-deletes-console-log rule can inspect the deleted side of
an Edit.

The 6 new rules

Rule	Action	Tool	Memory
warn-deletes-console-log	warn	Edit/MultiEdit	feedback_never_delete_debug_logs.md
warn-time-estimates-in-plans	warn	Edit/Write/MultiEdit	feedback_no_time_estimates.md
warn-pr-create-many-files	warn	Bash	feedback_split_large_prs.md
warn-posthog-in-alerts	warn	Slack	feedback_no_posthog_nag.md
block-sibling-suffix-storage	block	Bash	feedback_explicit_storage_prefixes.md
warn-localhost-in-pr-body	warn	Bash	feedback_test_on_staging.md

Schema extension: old_string field

warn-deletes-console-log needs to inspect the old_string of an Edit
to detect deletion of debug-log lines. Previously parse-input.sh
exposed only tool_input.content and tool_input.new_string (under
the same INPUT_CONTENT alias). This PR adds:

• INPUT_OLD_STRING extraction in parse-input.sh resolving from
  tool_input.old_string first, falling back to a newline-joined
  flatten of every tool_input.edits[].old_string (MultiEdit case).
• old_string) case in the field-resolver switch in eval-rule.sh.
• README schema list updated (field: ... | old_string).

Both edits are additive. No existing rule references old_string, and
the 136 / 136 baseline tests still pass after the schema extension is
applied alone (verified before adding the new rules).

Coverage notes

• warn-deletes-console-log — covers console.log(, console.debug(,
  console.error(, debug(, logger.debug/info/trace(. Bypass marker
  documented for the legitimate flows: env-gating conversion,
  post-prod-verification cleanup, documented debug-temp.

• warn-time-estimates-in-plans — bilingual EN/ES pattern covering
  days/sprints/weeks/cycles + dias/semanas/ciclos with
  optional accent. Scoped to plans/, specs/, design/, openspec/,
  docs/superpowers/ paths.

• warn-pr-create-many-files — heuristic on gh pr create (cannot
  count diff files itself); the message instructs the caller to run
  git diff --stat <base>..HEAD | tail -1 before bypassing.

• warn-posthog-in-alerts — applies_to: [Slack] using the existing
  post-slack.sh dispatcher (warn-only by construction). Catches
  posthog, dashboard, instrumentation keywords in messages.

• block-sibling-suffix-storage — Bash CLI surface only (supabase storage upload, gcloud storage cp, gsutil cp). Application-layer
  storage-uploader paths are covered in code review.

• warn-localhost-in-pr-body — two AND-chained command-field
  conditions (gh pr create/edit + inline --body + localhost
  /127.0.0.1). The --body-file form is naturally excluded.

Test count

```
Before: 136 / 136 passed (24 rules)
After: 171 / 171 passed (30 rules, +35 new tests)
```

Each rule covers positive matches across realistic invocations,
negative matches that should NOT fire, and bypass-marker behavior. The
schema extension was verified independently before adding rules: all
136 baseline tests pass after the parse-input.sh + eval-rule.sh
edits alone.

Version bump

`1.9.0 -> 1.10.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` (24 -> 30 rules)
• `npm run test-hooks` 171 / 171 pass
• CI (Blacksmith runner) — `npm run test-hooks` + `git diff hooks/rules/rules.json` clean
• Greptile review -> 5 / 5

Notes for reviewers

• Final PR of three sequential rollouts (feat(rules): add warn-curl-mutating-supabase-rest (close feedback_scripts_not_db.md) #16 warn-curl, feat(rules): add 5 anti-foot-shoot block rules (Tier 1) #17 Tier 1
  blocks, this PR Tier 2 warns).
• The schema extension is the first time parse-input.sh /
  eval-rule.sh change since the manifest landed; the change is
  strictly additive.
• The warn-time-estimates-in-plans test fixture intentionally uses
  inline content (no `\n` literals) — YAML single-quoted strings keep
  `\n` as a literal backslash-n which prevents the `\b` regex
  boundary from firing. Reviewers verifying patterns should keep this
  in mind.

OpenSpec: `openspec/changes/2026-05-hooks-tier-2-discipline-warnings/`
(proposal, design, tasks).

Created by Claude Code on behalf of @lapc506

Claude Code

[lapc506]

@greptile review

[greptile-apps]

Greptile Summary

This PR adds six Tier 2 discipline rules to the manifest-driven hook system and extends the rule schema with a new old_string field — the first time parse-input.sh / eval-rule.sh have changed since the manifest landed.

• Schema extension: INPUT_OLD_STRING is now extracted in parse-input.sh using | strings to filter nulls before join, and wired through eval-rule.sh as a first-class field; both edits are strictly additive.
• Six new rules covering debug-log deletion, time estimates in plan files, PR size, PostHog Slack nags, sibling-suffix storage uploads, and localhost in PR bodies — five warn and one block; each ships ≥8 tests including MultiEdit shape coverage that was missing in prior iterations.
• Version bump 1.9.0 → 1.10.0 applied consistently across package.json, plugin.json, and marketplace.json.

Confidence Score: 5/5

Safe to merge — all previously flagged issues are resolved and the additive schema extension introduces no regressions.

The two concrete defects identified in prior reviews — missing | strings null-guard in the MultiEdit jq path and the absent \b word-boundary anchor on the bare debug\( alternative — are both fixed in this iteration. MultiEdit-shape test coverage has also been added. The six new rules are purely additive, and the 136 baseline tests are unaffected by the schema extension. No new correctness problems were found.

No files require special attention.

Important Files Changed

Filename	Overview
hooks/lib/parse-input.sh	Adds INPUT_OLD_STRING extraction with `
hooks/lib/eval-rule.sh	Adds old_string) case to the field-resolver switch; no existing branches are touched.
hooks/rules/rules.yaml	Six new rules with comprehensive tests; previously flagged \bdebug\( word-boundary and MultiEdit shape tests are present; `
hooks/rules/rules.json	Regenerated artifact matching rules.yaml; all six new rule objects and their test fixtures are present.
hooks/rules/README.md	Schema field list and Tier 2 rule family description updated; new old_string field documented.
package.json	Version bumped from 1.9.0 to 1.10.0; consistent with plugin.json and marketplace.json.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Tool Call arrives] --> B{jq available?}
    B -- No --> C[Skip extraction]
    B -- Yes --> D[Extract INPUT_COMMAND, INPUT_FILE_PATH, INPUT_CONTENT, INPUT_TEXT]
    D --> E[Extract INPUT_OLD_STRING]
    E --> F{tool_input.old_string exists?}
    F -- Yes --> G[Use tool_input.old_string directly]
    F -- No --> H[Flatten edits via strings filter and join on newline]
    G --> I[Export all INPUT vars]
    H --> I
    I --> J[eval-rule.sh iterates conditions]
    J --> K{field type?}
    K -- command --> L[VALUE = INPUT_COMMAND]
    K -- file_path --> M[VALUE = INPUT_FILE_PATH]
    K -- content --> N[VALUE = INPUT_CONTENT]
    K -- text --> O[VALUE = INPUT_TEXT]
    K -- old_string --> P[VALUE = INPUT_OLD_STRING NEW]
    K -- unknown --> Q[exit 0 rule skipped]
    L & M & N & O & P --> R{pattern matches?}
    R -- warn --> S[exit 0 and stderr message]
    R -- block --> T[exit 2 and stderr message]
    R -- No match --> U[exit 0 silent]

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
hooks/rules/rules.yaml:703-717
**Path to 5/5 Confidence**

All previously flagged issues (`| strings` null-guard, `\bdebug\(` word-boundary, MultiEdit-shape tests) are resolved in this iteration. The following concrete additions would make the test suite exhaustive and eliminate the last heuristic gaps:

1. **`hooks/rules/rules.yaml` — add a MultiEdit test with a missing `old_string` key** (`warns-on-multiedit-with-null-old-string`): supply `edits: [{new_string: "x"}, {old_string: "console.log(y)", new_string: ""}]` (first edit has no `old_string` key at all). This directly exercises the `| strings` null-guard in `parse-input.sh` and confirms the rule still fires when one edit object is missing the key entirely.

2. **`hooks/rules/rules.yaml` — add a negative `block-sibling-suffix-storage` test for correct-destination-bad-source-name**: `gsutil cp ./data.processed.zip gs://bucket/processed/data.zip`. Currently the regex matches any position in the command, so this correct upload (bad source name, good destination) would be blocked. If the intent is to match only the destination path, narrowing the regex would eliminate that false positive; if the intent is to also catch bad source naming, a test and a note in the message would document the deliberate choice.

3. **`hooks/rules/rules.yaml` — add a `warn-time-estimates-in-plans` negative test for numbers-without-units** (e.g., `content: "Phase 1 and Phase 2 cover the implementation"`) to lock in that bare ordinal numbers don't trigger the rule when no time-unit word follows.

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

[greptile-apps]

Path to 5/5 Confidence

Three concrete changes would close every open gap:

1. hooks/lib/parse-input.sh line 48 — fix null filtering in the MultiEdit jq path
   Change [.tool_input.edits[]?.old_string] | join("\n") → [.tool_input.edits[]?.old_string | strings] | join("\n"). The | strings filter drops null entries before join is called, preventing silent jq failure when any edit object lacks an old_string key.

2. hooks/rules/rules.yaml (and regenerated rules.json) — add word boundary to debug\(
   Change debug\( → \bdebug\( so the rule does not fire on function names that merely end with debug (e.g. setDebug(, embedDebug(). The \b anchor is already used in the time-estimates pattern in this same PR.

3. hooks/rules/rules.yaml — add a MultiEdit-shape test for warn-deletes-console-log
   Add two test cases that supply tool_input.edits instead of tool_input.old_string:

   - name: warns-on-multiedit-deleting-console-log
     input:
       tool_input:
         edits:
           - old_string: "console.log('debug:', x)"
             new_string: ''
     expected_exit: 0
     expected_stderr_contains: 'warn-deletes-console-log'
   - name: allows-multiedit-not-touching-logs
     input:
       tool_input:
         edits:
           - old_string: 'const x = 1'
             new_string: 'const x = 2'
     expected_exit: 0

Prompt To Fix With AI

This is a comment left during a code review.
Path: hooks/rules/rules.yaml
Line: 1467

Comment:
**Path to 5/5 Confidence**

Three concrete changes would close every open gap:

1. **`hooks/lib/parse-input.sh` line 48 — fix null filtering in the MultiEdit jq path**
   Change `[.tool_input.edits[]?.old_string] | join("\n")` → `[.tool_input.edits[]?.old_string | strings] | join("\n")`. The `| strings` filter drops null entries before `join` is called, preventing silent jq failure when any edit object lacks an `old_string` key.

2. **`hooks/rules/rules.yaml` (and regenerated `rules.json`) — add word boundary to `debug\(`**
   Change `debug\(` → `\bdebug\(` so the rule does not fire on function names that merely end with `debug` (e.g. `setDebug(`, `embedDebug(`). The `\b` anchor is already used in the time-estimates pattern in this same PR.

3. **`hooks/rules/rules.yaml` — add a MultiEdit-shape test for `warn-deletes-console-log`**
   Add two test cases that supply `tool_input.edits` instead of `tool_input.old_string`:
   ```yaml
   - name: warns-on-multiedit-deleting-console-log
     input:
       tool_input:
         edits:
           - old_string: "console.log('debug:', x)"
             new_string: ''
     expected_exit: 0
     expected_stderr_contains: 'warn-deletes-console-log'
   - name: allows-multiedit-not-touching-logs
     input:
       tool_input:
         edits:
           - old_string: 'const x = 1'
             new_string: 'const x = 2'
     expected_exit: 0
   ```

How can I resolve this? If you propose a fix, please make it concise.

[lapc506]

Round 2: addressed all 3 Greptile findings. See commit 8f21122.

1. jq null-handling for MultiEdit (parse-input.sh) — added | strings filter before join so null old_string entries no longer blow up the extraction.
2. \bdebug\( word boundary in warn-deletes-console-log — now consistent with the \b anchor used in warn-time-estimates-in-plans.
3. MultiEdit-shape tests for warn-deletes-console-log — three new tests exercise the edits[] code path (single-edit positive, multi-edit second-element positive, multi-edit negative).

Tests: 171 -> 174 passing.

@greptile review