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

forgejo_admin commented

2026-03-28 23:20:51 +00:00

Contributor

Type

Bug

Lineage

Board: board-pal-e-agency
Story: story:scope-review
Arch: arch:hooks
Discovered during session 2026-03-28 (/review-ticket for #585)

Repo

forgejo_admin/claude-custom

What Broke

check-board-advance.sh greps for "APPROVED" in review notes to allow todo→next_up advancement. But the skill-review-ticket note instructs review agents to use "READY" as the passing verdict. The hook rejects valid reviews because the keyword doesn't match.

Workaround used: manually updated the review note heading from "Verdict: READY" to "Verdict: APPROVED" via update_block.

Repro Steps

Run /review-ticket board-pal-e-agency#585
Agent creates review note with "Verdict: READY"
Attempt update_board_item to move todo→next_up
Hook rejects: "Cannot advance item #585... Run /review-ticket first"
The review note exists but uses "READY" not "APPROVED"

Expected Behavior

Hook and skill agree on vocabulary. Either:

Hook accepts both "READY" and "APPROVED", OR
Skill instructs agents to write "APPROVED"

Environment

Hook: hooks/check-board-advance.sh, check_review_approved() function
Skill: skill-review-ticket note in pal-e-docs
Grep pattern: grep -qi 'APPROVED'

File Targets

hooks/check-board-advance.sh — check_review_approved() function grep pattern
pal-e-docs note skill-review-ticket — verdict vocabulary section

Acceptance Criteria

Hook and skill use the same passing verdict keyword
Existing review notes with "READY" are recognized by the hook
No manual update_block workaround needed

Test Expectations

Create a review note with "Verdict: READY" — hook allows advancement
Create a review note with "Verdict: APPROVED" — hook allows advancement
"NOT APPROVED" still blocks (existing behavior preserved)

Constraints

Pick ONE canonical keyword and update both hook + skill, or make the hook accept both
Don't break existing review notes that already use "APPROVED"

Checklist

Decide canonical keyword (recommend: accept both in hook)
Update hook grep pattern
Update skill-review-ticket if needed
Add test case

claude-custom#214 — backlog→todo gate (extends same hook)
review-585-2026-03-28-v2 — the review note that exposed this bug

### Type Bug ### Lineage - Board: board-pal-e-agency - Story: story:scope-review - Arch: arch:hooks - Discovered during session 2026-03-28 (/review-ticket for #585) ### Repo `forgejo_admin/claude-custom` ### What Broke `check-board-advance.sh` greps for "APPROVED" in review notes to allow todo→next_up advancement. But the `skill-review-ticket` note instructs review agents to use "READY" as the passing verdict. The hook rejects valid reviews because the keyword doesn't match. Workaround used: manually updated the review note heading from "Verdict: READY" to "Verdict: APPROVED" via `update_block`. ### Repro Steps 1. Run `/review-ticket board-pal-e-agency#585` 2. Agent creates review note with "Verdict: READY" 3. Attempt `update_board_item` to move todo→next_up 4. Hook rejects: "Cannot advance item #585... Run /review-ticket first" 5. The review note exists but uses "READY" not "APPROVED" ### Expected Behavior Hook and skill agree on vocabulary. Either: - Hook accepts both "READY" and "APPROVED", OR - Skill instructs agents to write "APPROVED" ### Environment - Hook: `hooks/check-board-advance.sh`, `check_review_approved()` function - Skill: `skill-review-ticket` note in pal-e-docs - Grep pattern: `grep -qi 'APPROVED'` ### File Targets - `hooks/check-board-advance.sh` — `check_review_approved()` function grep pattern - pal-e-docs note `skill-review-ticket` — verdict vocabulary section ### Acceptance Criteria - [ ] Hook and skill use the same passing verdict keyword - [ ] Existing review notes with "READY" are recognized by the hook - [ ] No manual `update_block` workaround needed ### Test Expectations - [ ] Create a review note with "Verdict: READY" — hook allows advancement - [ ] Create a review note with "Verdict: APPROVED" — hook allows advancement - [ ] "NOT APPROVED" still blocks (existing behavior preserved) ### Constraints - Pick ONE canonical keyword and update both hook + skill, or make the hook accept both - Don't break existing review notes that already use "APPROVED" ### Checklist - [ ] Decide canonical keyword (recommend: accept both in hook) - [ ] Update hook grep pattern - [ ] Update skill-review-ticket if needed - [ ] Add test case ### Related - claude-custom#214 — backlog→todo gate (extends same hook) - review-585-2026-03-28-v2 — the review note that exposed this bug

forgejo_admin commented

2026-03-28 23:28:53 +00:00

Author

Contributor

Scope Review: READY

Review note: review-638-2026-03-28
Ticket is fully scoped, all file targets verified, traceability complete, fits in a single agent pass (<5 min). No refinements needed.

Key findings:

Hook grep at lines 62/71 confirmed: only accepts "APPROVED", rejects "READY"
pal-e-docs skill-review-ticket confirmed: instructs agents to write "READY"
Router SKILL.md already says "APPROVED" -- mismatch is isolated to the pal-e-docs note vs hook
label-on-verdict.sh (QA PR reviews) NOT affected -- separate pipeline
Existing test suite (tests 12-19) covers APPROVED/NOT APPROVED but not READY
Recommended fix: grep -qiE 'APPROVED|READY' at lines 62 and 71, plus new test case

## Scope Review: READY Review note: `review-638-2026-03-28` Ticket is fully scoped, all file targets verified, traceability complete, fits in a single agent pass (<5 min). No refinements needed. Key findings: - Hook grep at lines 62/71 confirmed: only accepts "APPROVED", rejects "READY" - pal-e-docs `skill-review-ticket` confirmed: instructs agents to write "READY" - Router SKILL.md already says "APPROVED" -- mismatch is isolated to the pal-e-docs note vs hook - `label-on-verdict.sh` (QA PR reviews) NOT affected -- separate pipeline - Existing test suite (tests 12-19) covers APPROVED/NOT APPROVED but not READY - Recommended fix: `grep -qiE 'APPROVED|READY'` at lines 62 and 71, plus new test case