pal-e-docs

Review: Bug: review hook expects APPROVED but skill-review-ticket agents write READY

review-638-2026-03-28 Doc

review ready
Project

Verdict: APPROVED

Template Completeness

  • [x] Type -- Bug
  • [x] Lineage -- board, story, arch, discovery context documented
  • [x] Repo -- forgejo_admin/claude-custom (correct)
  • [x] What Broke -- clear description of keyword mismatch between hook and skill note
  • [x] Repro Steps -- 5-step reproduction with exact behavior at each step
  • [x] Expected Behavior -- two clear resolution options proposed
  • [x] Environment -- hook path, skill slug, grep pattern all specified
  • [x] File Targets -- both targets listed with specific function and section references
  • [x] Acceptance Criteria -- 3 criteria, all verifiable by an agent
  • [x] Test Expectations -- 3 concrete test cases specified
  • [x] Constraints -- backward compatibility requirement stated
  • [x] Checklist -- 4 discrete execution steps
  • [x] Related -- links to #214 and the review note that exposed the bug

Traceability

  • [x] story:scope-review -- present on board item
  • [x] arch:hooks -- present on board item
  • [x] Forgejo issue -- forgejo_admin/claude-custom#220, open

File Targets

  • [x] hooks/check-board-advance.sh -- verified: check_review_approved() at line 46. Grep pattern at lines 62 and 71: grep -vi 'NOT APPROVED' | grep -qi 'APPROVED'. Only accepts "APPROVED", rejects everything else including "READY".
  • [x] pal-e-docs note skill-review-ticket -- verified: Step 11 defines verdicts as "READY", "NEEDS_REFINEMENT", "BLOCK". The spawned review agent reads this note and writes "READY" as the passing verdict.
  • [x] skills/review-ticket/SKILL.md -- verified (additional context): The router skill already says "APPROVED" throughout (lines 3, 9, 77-78, 97, 104-105). The mismatch is specifically between the pal-e-docs note (what the spawned agent reads) and the hook (what gates advancement).
  • [x] tests/test_check_board_advance.sh -- verified: Tests 12-19 cover APPROVED/NOT APPROVED verdicts. No test currently covers "READY" as a passing verdict.

Repo Placement

Issue correctly filed on claude-custom. The hook shell script lives in that repo. The pal-e-docs skill-review-ticket note is a data update via MCP tool (not a code change requiring a separate repo PR), so a single-repo PR is appropriate. The agent can update the pal-e-docs note content as part of the same work unit.

Dependencies

  • [x] Item #581 (claude-custom#214 -- backlog-to-todo gate) -- in done, satisfied
  • [x] Item #585 (claude-custom#216 -- the review that exposed this bug) -- in done, satisfied
  • No unresolved dependencies. No items currently blocked by this ticket.

Acceptance Criteria

All 3 acceptance criteria are testable and specific:
  • "Hook and skill use the same passing verdict keyword" -- verifiable by grepping hook and note after fix
  • "Existing review notes with READY are recognized by the hook" -- testable via new test case in test suite
  • "No manual update_block workaround needed" -- verifiable via end-to-end flow (create review with READY, advance item)
Test expectations are well-specified with 3 concrete cases: READY allows, APPROVED allows, NOT APPROVED blocks.

Blast Radius

  • label-on-verdict.sh -- NOT affected. Uses "### VERDICT: APPROVED" / "### VERDICT: NOT APPROVED" for QA PR reviews. Entirely separate pipeline (PR reviews vs scope reviews). Different vocabulary, different context.
  • skills/review-pr/SKILL.md -- NOT affected. PR review uses "APPROVED" / "NOT APPROVED" verdicts for code review, not scope review.
  • Existing review notes already using "APPROVED" (from manual workaround) will continue to work regardless of fix approach.
  • docs/superpowers/specs/2026-03-18-review-ticket-design.md -- uses "READY" vocabulary in the design spec. Informational only; not executable. Low priority alignment but noted.
  • Rollback is straightforward -- revert one grep pattern change.

Decomposition Assessment

2 file targets in 1 repo plus 1 pal-e-docs note update via MCP. 3 acceptance criteria. 1 new test case to add. Estimated agent time: under 5 minutes. Three-thing limit: hook grep (1), test case (2), optional note alignment (3). No decomposition needed.

Recommendation

No action needed. Scope is solid, all file targets verified, traceability complete, single agent pass.
Implementation note: The issue's recommended approach (accept both READY and APPROVED in the hook) is correct. Changing the grep from grep -qi 'APPROVED' to grep -qiE 'APPROVED|READY' at lines 62 and 71 (preserving NOT APPROVED rejection via the preceding grep -vi) is the minimal safe change. Add a test case for "Verdict: READY" alongside existing "Verdict: APPROVED" test.