pal-e-docs

Review: Validation-gate hook — block done without validation proof

review-519-2026-03-28 Review

review ready
Project

Verdict: READY

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- standalone, discovered from sop-board-workflow and template-validation
  • [x] Repo -- forgejo_admin/claude-custom
  • [x] User Story -- clear "As Betty Sue I want... So that..."
  • [x] Context -- thorough, includes enforcement signal research with 4 options evaluated and decision documented
  • [x] File Targets -- 3 create/modify targets with explicit DO NOT TOUCH list
  • [x] Acceptance Criteria -- 6 testable conditions
  • [x] Test Expectations -- manual test commands with realistic JSON payloads
  • [x] Constraints -- fail-open pattern, API contract, column enum dependency, skill independence
  • [x] Checklist -- PR opened, tests pass, no unrelated changes
  • [x] Related -- 7 related items with context
All required sections for a Feature issue are present and substantive.

Traceability

  • [x] story:pm-scope -- PM scoping enforcement story, present on board item
  • [x] arch:hooks -- hooks architecture component, present on board item
  • [x] arch:board-api -- board API architecture component, present on board item
  • [x] Forgejo issue -- forgejo_admin/claude-custom#210, open
Full traceability. All three legs verified.

File Targets

  • [x] hooks/gate-validation-done.sh (NEW) -- does not exist yet, confirmed. Pattern reference hooks/check-board-item.sh exists at expected path.
  • [x] settings.json -- exists. See recommendation below regarding matcher placement.
  • [x] hooks/board-item-on-merge.sh -- exists. Issue says "line 133" but '{"column": "done"}' is on line 132. Off-by-one; agent will find via grep regardless.
  • [x] hooks/check-board-item.sh (DO NOT TOUCH) -- exists, verified separate concern (create vs update).
  • [x] hooks/boards-config.sh (DO NOT TOUCH) -- exists.
  • [x] hooks/forgejo-helper.sh (DO NOT TOUCH) -- exists.
All file targets verified. Line reference is off by one (132 not 133) but functionally harmless.

Repo Placement

OK. Issue filed on forgejo_admin/claude-custom, all file targets are in claude-custom. Single repo, no cross-repo concerns.

Dependencies

  • [x] forgejo_admin/pal-e-api#223 (adds validation BoardColumn) -- satisfied. Issue is closed, board item #480 is in done. The constraint about gating the board-item-on-merge.sh change is now moot; the validation column exists in production.
  • [x] /validate-ticket skill -- correctly identified as independent discovered scope. Hook works without it (agents can create validation notes manually).
  • [x] check-board-advance.sh -- existing PreToolUse hook on the same matcher. Both hooks run independently via the hooks array. No conflict: check-board-advance gates backlog-to-todo and todo-to-next_up; the new hook gates any-to-done. No overlapping transitions.

Acceptance Criteria

6 AC, all verifiable via shell test scripts. The test command example is realistic: echo '...' | bash hooks/gate-validation-done.sh. Each criterion has clear before/after conditions. AC #4 (fail-open for items without forgejo_issue_url) correctly handles note-type board items. AC #5 (board-item-on-merge.sh column change) is a one-line edit with clear verification. No ambiguous "works correctly" language found.

Blast Radius

  • check-board-advance.sh Test 5 currently asserts "move to done -- no gate." This test stays correct because check-board-advance itself still allows done moves. The new hook is a separate gate. No existing test modification needed.
  • bulk_move_board_items -- the issue's proposed settings.json matcher (mcp__pal-e-docs__update_board_item) does not cover bulk moves. If an agent uses bulk_move_board_items to move items to done, the validation gate would be bypassed. Minor gap but not a blocker -- bulk moves to done are uncommon.
  • commands/update-docs.md references update_board_item(column="done") at line 119. The /update-docs skill would be blocked by this new gate unless a validation note exists first, which is correct behavior.
  • Rollback is straightforward: remove hook from settings.json, delete the script file, revert the one-line change in board-item-on-merge.sh.

Decomposition Assessment

3 file targets, 1 repo, 6 AC. All tightly coupled: one new hook script, its registration in settings.json, and one related one-line fix in board-item-on-merge.sh. Estimated agent time: 3-4 minutes. No independent subtasks that would benefit from parallelization. No decomposition needed.

Recommendation

  • [BODY] Line reference: "line 133" should be "line 132" for board-item-on-merge.sh. Cosmetic only -- agent will find via grep.
  • [BODY] Settings.json guidance: instead of "Add a new PreToolUse entry with matcher mcp__pal-e-docs__update_board_item," recommend adding the hook to the existing matcher's hooks array (lines 194-201, alongside check-board-advance.sh). This is consistent with how the Bash matcher aggregates multiple hooks and avoids a duplicate matcher pattern. Alternatively, if a separate entry is preferred, the matcher should also include bulk_move_board_items to prevent bypass via bulk moves.
Neither recommendation is a blocker. The issue is well-scoped and actionable as-is.