ldraney/claude-custom
1
0
Fork 0
Code Issues 32 Pull requests Projects Releases Packages Wiki Activity Actions

Phase 4-1: Hook hardening (sed bug, portability, dynamic fields) #65

New issue
Closed
opened 2026-03-07 23:14:08 +00:00 by forgejo_admin · 0 comments
forgejo_admin commented 2026-03-07 23:14:08 +00:00
Contributor
Copy link

Lineage

plan-2026-03-07-note-hierarchy-conventions → Phase 4 (Enforcement Hook) → Phase 4-1 (Hook Hardening)

Repo

forgejo_admin/claude-custom

User Story

As a platform operator
I want hooks to be correct and portable
So that HTML entity decoding doesn't corrupt data and hooks work on non-GNU systems

Context

QA review of PR #64 (check-phase-template.sh) found three issues. Two are cross-cutting bugs affecting all hooks that decode HTML entities, one is specific to the new phase hook.

Bug 1 — sed ampersand backreference (CROSS-CUTTING):
All hooks using sed 's/&lt;/</g' style HTML entity decoding have a latent bug. In sed replacement strings, & is a backreference to the matched pattern. Example: if content contains &amp; literally, the sed chain s/&amp;/\&/g replaces &amp; with &amp; (the matched text) rather than &. The fix is to escape & as \& in replacements, or better yet, avoid the ambiguity entirely.

Affected files:

  • hooks/check-issue-template.sh
  • hooks/check-note-template.sh
  • hooks/check-phase-template.sh

Bug 2 — GNU grep \| syntax (CROSS-CUTTING):
Hooks use grep 'patternA\|patternB' which is GNU grep BRE syntax. POSIX-portable equivalent is grep -E 'patternA|patternB'. Works on Arch (GNU grep) but fails silently on Alpine/busybox.

Affected files: any hook using \| in grep patterns. Check all hooks.

Improvement 3 — Hardcoded field list in check-phase-template.sh:
The hook hardcodes Goal/Owner/Repo/Depends on checks. Unlike check-issue-template.sh (which extracts ### headings from the markdown template dynamically), the phase template uses HTML <strong> tags which are harder to parse generically. Options: (a) extract from template's Hook Enforcement section, (b) add a comment documenting why it's hardcoded. Option (b) is simpler and honest.

File Targets

Files the agent should modify:

  • hooks/check-issue-template.sh — fix sed ampersand bug, fix grep portability
  • hooks/check-note-template.sh — fix sed ampersand bug, fix grep portability
  • hooks/check-phase-template.sh — fix sed ampersand bug, fix grep portability, add comment about hardcoded fields

Files the agent should check (fix if affected):

  • All other hooks/*.sh files — scan for \| in grep patterns and & in sed replacements

Files the agent should NOT touch:

  • settings.json — no registration changes needed
  • Agent profile .md files

Acceptance Criteria

  • All sed HTML entity decoding uses properly escaped replacements (no bare & in replacement strings)
  • All grep patterns use -E flag with | instead of BRE \|
  • check-phase-template.sh has a comment explaining why fields are hardcoded (HTML <strong> tags vs markdown ### headings)
  • All three affected hooks still work correctly after fixes (issue creation, note creation, phase creation all still validated)
  • No other hooks are affected (or if they are, they're fixed too)

Test Expectations

  • Manual test: create a Forgejo issue → still validated by check-issue-template.sh
  • Manual test: create a plan note → still validated by check-note-template.sh
  • Manual test: create a phase note → still validated by check-phase-template.sh
  • Run command: manual testing via Claude Code session

Constraints

  • Keep fail-open pattern (trap 'exit 0' ERR) in all hooks
  • Minimal changes — fix the bugs, don't refactor the hooks
  • For the sed fix, prefer explicit escaping over rewriting the decode approach
  • For grep, just add -E flag and remove backslash before |

Checklist

  • PR opened
  • Hooks work (manual test)
  • No unrelated changes

Related

  • phase-hierarchy-4-1-hook-hardening — phase note
  • template-phase — the template check-phase-template.sh enforces
  • PR #64 (claude-custom) — the PR where these nits were found
  • ai-agency — project
### Lineage `plan-2026-03-07-note-hierarchy-conventions` → Phase 4 (Enforcement Hook) → Phase 4-1 (Hook Hardening) ### Repo `forgejo_admin/claude-custom` ### User Story As a platform operator I want hooks to be correct and portable So that HTML entity decoding doesn't corrupt data and hooks work on non-GNU systems ### Context QA review of PR #64 (check-phase-template.sh) found three issues. Two are cross-cutting bugs affecting all hooks that decode HTML entities, one is specific to the new phase hook. **Bug 1 — sed ampersand backreference (CROSS-CUTTING):** All hooks using `sed 's/&lt;/</g'` style HTML entity decoding have a latent bug. In sed replacement strings, `&` is a backreference to the matched pattern. Example: if content contains `&amp;` literally, the sed chain `s/&amp;/\&/g` replaces `&amp;` with `&amp;` (the matched text) rather than `&`. The fix is to escape `&` as `\&` in replacements, or better yet, avoid the ambiguity entirely. Affected files: - `hooks/check-issue-template.sh` - `hooks/check-note-template.sh` - `hooks/check-phase-template.sh` **Bug 2 — GNU grep `\|` syntax (CROSS-CUTTING):** Hooks use `grep 'patternA\|patternB'` which is GNU grep BRE syntax. POSIX-portable equivalent is `grep -E 'patternA|patternB'`. Works on Arch (GNU grep) but fails silently on Alpine/busybox. Affected files: any hook using `\|` in grep patterns. Check all hooks. **Improvement 3 — Hardcoded field list in check-phase-template.sh:** The hook hardcodes Goal/Owner/Repo/Depends on checks. Unlike `check-issue-template.sh` (which extracts `### headings` from the markdown template dynamically), the phase template uses HTML `<strong>` tags which are harder to parse generically. Options: (a) extract from template's Hook Enforcement section, (b) add a comment documenting why it's hardcoded. Option (b) is simpler and honest. ### File Targets Files the agent should modify: - `hooks/check-issue-template.sh` — fix sed ampersand bug, fix grep portability - `hooks/check-note-template.sh` — fix sed ampersand bug, fix grep portability - `hooks/check-phase-template.sh` — fix sed ampersand bug, fix grep portability, add comment about hardcoded fields Files the agent should check (fix if affected): - All other `hooks/*.sh` files — scan for `\|` in grep patterns and `&` in sed replacements Files the agent should NOT touch: - `settings.json` — no registration changes needed - Agent profile `.md` files ### Acceptance Criteria - [ ] All `sed` HTML entity decoding uses properly escaped replacements (no bare `&` in replacement strings) - [ ] All `grep` patterns use `-E` flag with `|` instead of BRE `\|` - [ ] `check-phase-template.sh` has a comment explaining why fields are hardcoded (HTML `<strong>` tags vs markdown `###` headings) - [ ] All three affected hooks still work correctly after fixes (issue creation, note creation, phase creation all still validated) - [ ] No other hooks are affected (or if they are, they're fixed too) ### Test Expectations - [ ] Manual test: create a Forgejo issue → still validated by check-issue-template.sh - [ ] Manual test: create a plan note → still validated by check-note-template.sh - [ ] Manual test: create a phase note → still validated by check-phase-template.sh - Run command: manual testing via Claude Code session ### Constraints - Keep fail-open pattern (`trap 'exit 0' ERR`) in all hooks - Minimal changes — fix the bugs, don't refactor the hooks - For the sed fix, prefer explicit escaping over rewriting the decode approach - For grep, just add `-E` flag and remove backslash before `|` ### Checklist - [ ] PR opened - [ ] Hooks work (manual test) - [ ] No unrelated changes ### Related - `phase-hierarchy-4-1-hook-hardening` — phase note - `template-phase` — the template check-phase-template.sh enforces - PR #64 (claude-custom) — the PR where these nits were found - `ai-agency` — project
forgejo_admin 2026-03-07 23:19:29 +00:00
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
217-enhance-skill-review-ticket-verify-arch
86-epilogue-nits-hook-updates-dottie-config
76-add-sessionstart-hook-to-detect-missing
69-fix-block-docs-writes-sh-security-gap-ad
51-phase-3-agent-label-behavior-posttooluse
No results found.
Labels
Clear labels   
domain:backend

   
domain:devops

   
domain:frontend

   
status:approved

QA passed, awaiting merge approval

  
status:in-progress

Dev agent is actively working

  
status:needs-fix

QA found issues, back to dev

  
status:qa

PR submitted, awaiting QA review

  
type:bug

Bug fix

  
type:devops

Infrastructure/CI/config work

  
type:feature

New functionality

No labels
domain:backend
domain:devops
domain:frontend
status:approved
status:in-progress
status:needs-fix
status:qa
type:bug
type:devops
type:feature
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
ldraney/claude-custom#65
No description provided.