Add phase note template enforcement hook (#63) #64

Merged

forgejo_admin merged 1 commit from 63-check-phase-template-hook into main

2026-03-07 23:12:55 +00:00

forgejo_admin commented

2026-03-07 23:08:59 +00:00

Contributor

Summary

PreToolUse hook that validates mcp__pal-e-docs__create_note calls where note_type=phase. Checks HTML content against required fields from template-phase: bold header fields (Goal, Owner, Repo, Depends on), structure (Scope OR Problem+Fix), and Related section. Fail-open on all errors.

Changes

hooks/check-phase-template.sh (new) -- PreToolUse hook that validates phase note content against template-phase. Checks tool_input.note_type == "phase" first, bails early for non-phase notes. Validates required bold fields, structure shape (full phase vs subphase), and Related section.
settings.json -- Registered the new hook under the existing mcp__pal-e-docs__create_note matcher alongside check-note-template.sh.

Test Plan

Manual: create a phase note missing Goal -- blocked with helpful error
Manual: create a phase note with all required fields (full phase shape) -- allowed
Manual: create a phase note with Problem+Fix (no Scope, subphase shape) -- allowed
Manual: create a phase note with Problem but no Fix -- blocked
Manual: create a non-phase note -- allowed (not affected)
Manual: empty content -- denied with pointer to template-phase
Manual: invalid JSON input -- fail-open (exit 0)

Review Checklist

Hook follows fail-open pattern (set -eo pipefail + trap exit 0 ERR)
Non-phase notes pass through unaffected
Both phase shapes validated (full phase with Scope, subphase with Problem+Fix)
settings.json is valid JSON
No unrelated changes

Plan: plan-2026-03-07-note-hierarchy-conventions (Phase 4)
Forgejo issue: #63

## Summary PreToolUse hook that validates `mcp__pal-e-docs__create_note` calls where `note_type=phase`. Checks HTML content against required fields from `template-phase`: bold header fields (Goal, Owner, Repo, Depends on), structure (Scope OR Problem+Fix), and Related section. Fail-open on all errors. ## Changes - **`hooks/check-phase-template.sh`** (new) -- PreToolUse hook that validates phase note content against template-phase. Checks `tool_input.note_type == "phase"` first, bails early for non-phase notes. Validates required bold fields, structure shape (full phase vs subphase), and Related section. - **`settings.json`** -- Registered the new hook under the existing `mcp__pal-e-docs__create_note` matcher alongside `check-note-template.sh`. ## Test Plan - Manual: create a phase note missing Goal -- blocked with helpful error - Manual: create a phase note with all required fields (full phase shape) -- allowed - Manual: create a phase note with Problem+Fix (no Scope, subphase shape) -- allowed - Manual: create a phase note with Problem but no Fix -- blocked - Manual: create a non-phase note -- allowed (not affected) - Manual: empty content -- denied with pointer to template-phase - Manual: invalid JSON input -- fail-open (exit 0) ## Review Checklist - [ ] Hook follows fail-open pattern (set -eo pipefail + trap exit 0 ERR) - [ ] Non-phase notes pass through unaffected - [ ] Both phase shapes validated (full phase with Scope, subphase with Problem+Fix) - [ ] settings.json is valid JSON - [ ] No unrelated changes ## Related - Plan: `plan-2026-03-07-note-hierarchy-conventions` (Phase 4) - Forgejo issue: #63

forgejo_admin added 1 commit

2026-03-07 23:08:59 +00:00

Add phase note template enforcement hook (#63 ) e589ea15e1

PreToolUse hook that validates phase notes against template-phase.
Checks note_type=phase, required bold fields (Goal, Owner, Repo,
Depends on), structure (Scope or Problem+Fix), and Related section.
Fail-open on all errors.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

forgejo_admin commented

2026-03-07 23:10:48 +00:00

Author

Contributor

PR #64 Review

BLOCKERS

None.

NITS

Hardcoded vs. dynamic template validation -- check-note-template.sh and check-issue-template.sh fetch the template from pal-e-docs at runtime and extract required headings dynamically. This hook hardcodes the required fields (Goal:, Owner:, Repo:, Depends on:, Scope, etc.) directly in the script. This means if template-phase changes, the hook must be updated manually. This is a reasonable tradeoff (phase templates are stable, and HTML structure parsing for bold fields would be fragile), but worth documenting in the script header that the field list is hardcoded and must be kept in sync with template-phase.
sed ampersand replacement (pre-existing) -- Line 43: s/&/\&/g -- the \& in sed replacement is a backreference to the matched text, so this replaces & with itself (&). The correct replacement would be s/&/\&/g with a literal & or s/&/\x26/g. However, this is the exact same pattern used in check-issue-template.sh (line 53), check-note-template.sh (lines 84, 91), so it is a pre-existing issue across all hooks, not introduced here. Non-blocking.
Minor: grep -qi with \| alternation -- Lines 57-59 use \| for alternation in basic regex mode. This works on GNU grep (which is what Arch ships) but is not POSIX-standard. Since the platform is Arch Linux only, this is fine, but a comment noting GNU grep dependency would be defensive.

SOP COMPLIANCE

Branch named after issue (63-check-phase-template-hook references issue #63)
PR body has: Summary, Changes, Test Plan, Related
Related references plan slug (plan-2026-03-07-note-hierarchy-conventions)
No secrets, .env files, or credentials committed
No unnecessary file changes (2 files: new hook + settings.json registration)
Commit title is descriptive and references issue number
settings.json is valid JSON and hook is registered on the correct matcher (mcp__pal-e-docs__create_note)

Additional Observations

note_type field confirmed valid -- NoteType enum in pal-e-docs/schemas.py includes "phase". The tool_input.note_type gate is correct.
Non-phase notes pass through -- Line 24 exits 0 if note_type != "phase". Correct.
Both phase shapes validated -- Full phase (Scope) and subphase (Problem + Fix) correctly handled with clear error messages for partial subphase patterns (Problem without Fix, Fix without Problem).
No command injection -- $CONTENT and $DECODED are properly double-quoted in all uses. jq -n --arg safely escapes the $MISSING variable in JSON output.
Fail-open pattern correct -- set -eo pipefail + trap 'exit 0' ERR matches all existing hooks.
settings.json registration -- Hook added to existing mcp__pal-e-docs__create_note matcher array alongside check-note-template.sh. Both hooks will run: check-note-template.sh validates tag-based templates (plan, bug, etc.), check-phase-template.sh validates phase-specific structure. No conflict since they check orthogonal things.

VERDICT: APPROVED

## PR #64 Review ### BLOCKERS None. ### NITS 1. **Hardcoded vs. dynamic template validation** -- `check-note-template.sh` and `check-issue-template.sh` fetch the template from pal-e-docs at runtime and extract required headings dynamically. This hook hardcodes the required fields (`Goal:`, `Owner:`, `Repo:`, `Depends on:`, `Scope`, etc.) directly in the script. This means if `template-phase` changes, the hook must be updated manually. This is a reasonable tradeoff (phase templates are stable, and HTML structure parsing for bold fields would be fragile), but worth documenting in the script header that the field list is hardcoded and must be kept in sync with `template-phase`. 2. **`sed` ampersand replacement (pre-existing)** -- Line 43: `s/&/\&/g` -- the `\&` in sed replacement is a backreference to the matched text, so this replaces `&` with itself (`&`). The correct replacement would be `s/&/\&/g` with a literal `&` or `s/&/\x26/g`. However, this is the exact same pattern used in `check-issue-template.sh` (line 53), `check-note-template.sh` (lines 84, 91), so it is a pre-existing issue across all hooks, not introduced here. Non-blocking. 3. **Minor: `grep -qi` with `\|` alternation** -- Lines 57-59 use `\|` for alternation in basic regex mode. This works on GNU grep (which is what Arch ships) but is not POSIX-standard. Since the platform is Arch Linux only, this is fine, but a comment noting GNU grep dependency would be defensive. ### SOP COMPLIANCE - [x] Branch named after issue (`63-check-phase-template-hook` references issue #63) - [x] PR body has: Summary, Changes, Test Plan, Related - [x] Related references plan slug (`plan-2026-03-07-note-hierarchy-conventions`) - [x] No secrets, .env files, or credentials committed - [x] No unnecessary file changes (2 files: new hook + settings.json registration) - [x] Commit title is descriptive and references issue number - [x] settings.json is valid JSON and hook is registered on the correct matcher (`mcp__pal-e-docs__create_note`) ### Additional Observations - **`note_type` field confirmed valid** -- `NoteType` enum in `pal-e-docs/schemas.py` includes `"phase"`. The `tool_input.note_type` gate is correct. - **Non-phase notes pass through** -- Line 24 exits 0 if `note_type != "phase"`. Correct. - **Both phase shapes validated** -- Full phase (Scope) and subphase (Problem + Fix) correctly handled with clear error messages for partial subphase patterns (Problem without Fix, Fix without Problem). - **No command injection** -- `$CONTENT` and `$DECODED` are properly double-quoted in all uses. `jq -n --arg` safely escapes the `$MISSING` variable in JSON output. - **Fail-open pattern correct** -- `set -eo pipefail` + `trap 'exit 0' ERR` matches all existing hooks. - **settings.json registration** -- Hook added to existing `mcp__pal-e-docs__create_note` matcher array alongside `check-note-template.sh`. Both hooks will run: `check-note-template.sh` validates tag-based templates (plan, bug, etc.), `check-phase-template.sh` validates phase-specific structure. No conflict since they check orthogonal things. ### VERDICT: APPROVED