pal-e-docs

Review: Remove plan/phase hooks from enforcement layer

review-396-2026-03-26 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Lineage
  • [x] Repo
  • [x] User Story
  • [x] Context
  • [x] File Targets
  • [x] Acceptance Criteria
  • [x] Test Expectations
  • [x] Constraints
  • [x] Checklist
  • [x] Related
All required sections from template-issue are present. Lineage uses "standalone" which is valid for unplanned work.

File Targets

  • [x] hooks/check-phase-template.sh — verified: exists, entire file is phase-specific (line 25: NOTE_TYPE = "phase"). Orphaned as described.
  • [x] hooks/check-note-template.sh lines 53-55 — verified: lines 53-55 route plan tag to template-plan. Also line 6 comment references plan routing.
  • [x] hooks/check-board-item.sh lines 60-63 — verified: phase) case validates note_slug. Also line 10 comment references phase items. Removal is structurally safe (case/esac remains valid).
  • [x] hooks/remind-update-docs.sh line 34 — verified: additionalContext string contains "plan notes, phase status" in the post-merge message.
  • [x] settings.json lines 177-180 — verified: check-phase-template.sh registered under mcp__pal-e-docs__create_note PreToolUse matcher.
  • [ ] hooks/session-start-context.sh — MISSING FROM TICKET: Lines 112-260+ contain ~150 lines of active plan-fetching logic. Queries notes?tags=plan,active, fetches plan details, injects plan TOCs into session context, and extracts in-progress phase titles for semantic search. This is the largest plan/phase code point in the entire hooks directory. Must be addressed.
  • [ ] hooks/stop-doc-checkin.sh — MISSING FROM TICKET: Lines 9-10 say "plans advanced" and "plan notes in pal-e-docs." This is a reminder-only hook but still references deprecated concepts.
  • [ ] hooks/inject-subagent-context.sh — MINOR: Line 2 comment says "plan/SOP context." Comment-only, low priority, but stale language.
The "files NOT to touch" list is correct — check-agent-spawn.sh, label-on-branch.sh, label-on-pr.sh, label-on-verdict.sh, and board-item-on-merge.sh all confirmed to have zero plan/phase references.

Repo Placement

OK. Issue filed on forgejo_admin/claude-custom, all file targets are in that repo. No cross-repo changes needed.

Dependencies

  • Board item #397 (claude-custom#169: "Update 11 SOPs/conventions for kanban-over-plans") is a sibling ticket in the same todo column with the same labels. These are independent — #396 is hooks code, #397 is documentation. No blocking dependency between them.
  • Board item #395 (claude-custom#167: "Upgrade story: label to hard-block + Task type routing") touches arch:enforcement and could have merge conflicts if both modify check-board-item.sh or settings.json. Worth noting but not a blocker.
  • convention-kanban-over-plans exists and is the authorizing convention. No dependency issue.

Acceptance Criteria

  • [x] "check-phase-template.sh removed or disabled" — testable via file existence check
  • [x] "Creating note with note_type=phase no longer triggers validation" — testable via manual hook test
  • [x] "Creating note tagged plan no longer routes to template-plan validation" — testable
  • [x] "Board items with item_type=issue still validated" — testable regression check
  • [x] "Post-merge reminder no longer references plans/phases" — testable via string check
  • [x] "settings.json no longer registers check-phase-template.sh" — testable via JSON parse
  • [ ] MISSING: No acceptance criterion for session-start-context.sh plan injection removal
  • [ ] MISSING: No acceptance criterion for stop-doc-checkin.sh language update
Test commands are manual (no automated test harness for hooks). This is acceptable given the hook architecture — bash hooks don't have a test framework. The JSON validation check for settings.json is practical.

Blast Radius

  • session-start-context.sh is the critical miss. This hook runs on every session start and actively fetches plan data from pal-e-docs. If plans are deprecated but this hook still queries for them, agents still see plan context injected every session — directly contradicting the convention. This is the highest-impact code point for the kanban-over-plans migration.
  • stop-doc-checkin.sh fires on every session stop. Lower impact (advisory only) but reinforces deprecated mental model.
  • No downstream consumer impact beyond agent behavior — all changes are in the hook layer which is self-contained in claude-custom.

Recommendation

Two issues must be resolved before this ticket is READY:
  • Add hooks/session-start-context.sh to File Targets. The plan-fetching block (lines 112-260+) must be removed or replaced. This includes the notes?tags=plan,active query, all plan TOC injection logic, and the phase-title semantic search extraction. The output context block referencing "Active Plan TOCs" must also be updated. This is a significant scope addition (~150 lines).
  • Add hooks/stop-doc-checkin.sh to File Targets. Lines 9-10 need language updated from "plans advanced" / "plan notes" to board-appropriate language. Small change.
  • Add acceptance criteria for session-start-context.sh (no plan data injected at session start) and stop-doc-checkin.sh (no plan references in stop message).
Optional: Update the comment on line 2 of inject-subagent-context.sh from "plan/SOP context" to just "SOP context." Low priority.