ldraney/claude-custom

Fork 0

feat: upgrade story: label to hard-block and add Task type routing #171

Merged

forgejo_admin merged 1 commit from 167-upgrade-story-label-hard-block into main

2026-03-26 22:59:57 +00:00

forgejo_admin commented

2026-03-26 22:24:37 +00:00

Contributor

Summary

Upgrades the story: label from a warning to a hard-block in check-board-item.sh, completing the traceability triangle (type + arch + story). Adds Task type routing in check-issue-template.sh so Task-type issues validate against template-issue-task.

Changes

hooks/check-board-item.sh -- story: label enforcement upgraded from WARNINGS to ERRORS (deny on missing)
hooks/check-board-item.sh -- line 8 comment header updated from "recommended -- warn, don't block" to "required"
hooks/check-board-item.sh -- removed dead WARNINGS infrastructure (variable, advisory output block)
hooks/check-issue-template.sh -- added Task|task) case routing to template-issue-task
hooks/check-issue-template.sh -- updated comment header to list Task type

Test Plan

create_board_item without story: label is DENIED with actionable message pointing to template-ticket
create_board_item with story: label still succeeds (exit 0)
type: and arch: enforcement unchanged
Task-type issues route to template-issue-task template
Feature/Bug/Spike/Nit-Bundle routing unchanged
Fail-open pattern preserved (trap 'exit 0' ERR)

Review Checklist

story: label deny tested (board item without story: returns deny JSON)
story: label allow tested (board item with story: exits 0)
Task type routing verified (Task -> template-issue-task)
All other type routing unchanged (Bug, Feature, Spike, Nit-Bundle)
No WARNINGS references remain in check-board-item.sh
Comment headers updated in both files

Closes #167
Discovered scope (tracked separately per review): session-start-context.sh Task type gap

## Summary Upgrades the `story:` label from a warning to a hard-block in `check-board-item.sh`, completing the traceability triangle (type + arch + story). Adds `Task` type routing in `check-issue-template.sh` so Task-type issues validate against `template-issue-task`. ## Changes - `hooks/check-board-item.sh` -- story: label enforcement upgraded from WARNINGS to ERRORS (deny on missing) - `hooks/check-board-item.sh` -- line 8 comment header updated from "recommended -- warn, don't block" to "required" - `hooks/check-board-item.sh` -- removed dead WARNINGS infrastructure (variable, advisory output block) - `hooks/check-issue-template.sh` -- added `Task|task)` case routing to `template-issue-task` - `hooks/check-issue-template.sh` -- updated comment header to list Task type ## Test Plan - `create_board_item` without `story:` label is DENIED with actionable message pointing to template-ticket - `create_board_item` with `story:` label still succeeds (exit 0) - `type:` and `arch:` enforcement unchanged - Task-type issues route to `template-issue-task` template - Feature/Bug/Spike/Nit-Bundle routing unchanged - Fail-open pattern preserved (trap 'exit 0' ERR) ## Review Checklist - [x] story: label deny tested (board item without story: returns deny JSON) - [x] story: label allow tested (board item with story: exits 0) - [x] Task type routing verified (Task -> template-issue-task) - [x] All other type routing unchanged (Bug, Feature, Spike, Nit-Bundle) - [x] No WARNINGS references remain in check-board-item.sh - [x] Comment headers updated in both files ## Related - Closes #167 - Discovered scope (tracked separately per review): `session-start-context.sh` Task type gap

forgejo_admin added 1 commit

2026-03-26 22:24:37 +00:00

feat: upgrade story: label to hard-block and add Task type routing (#167 ) a18dfa4d87

Enforce story: label as a hard requirement in check-board-item.sh
(was warn-only), completing the traceability triangle. Add Task type
routing in check-issue-template.sh so Task-type issues validate
against template-issue-task. Clean up dead WARNINGS infrastructure.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-26 22:25:15 +00:00

Author

Contributor

QA Review: PR #171

Scope Verification (issue #167 + comments)

Acceptance Criteria	Status
`create_board_item` without `story:` label is DENIED	PASS -- line 48-50 now appends to ERRORS with deny message
Deny message includes explanation + template-ticket pointer	PASS -- message includes "Every ticket must trace to a user story..." and "See template-ticket"
`create_board_item` WITH `story:` label still succeeds	PASS -- grep guard unchanged, no story: match skips the block
Existing board items unaffected (hook only fires on creation)	PASS -- PreToolUse matcher is create_board_item only
`check-issue-template.sh` recognizes `### Type: Task`	PASS -- `Task\|task)` case added routing to `template-issue-task`
Task-type issues validate against template-issue-task	PASS -- TEMPLATE_SLUG set correctly
Feature/Bug/Spike validation unchanged	PASS -- existing cases untouched

Review Nits from Issue Comment 4

Nit	Status
Line 8 comment header updated to "required"	PASS
Line 48 inline comment updated to "require"	PASS
Dead WARNINGS infrastructure cleaned up	PASS -- variable removed, advisory output block removed

Domain Review

Fail-open pattern preserved (trap 'exit 0' ERR at line 17)
type: and arch: enforcement logic untouched
Case statement alignment in check-issue-template.sh is cosmetic-only, correct
No unrelated changes in the diff (+9/-19, 2 files)

VERDICT: APPROVE

All 7 acceptance criteria met. All 3 review nits addressed. No issues found.

## QA Review: PR #171 ### Scope Verification (issue #167 + comments) | Acceptance Criteria | Status | |---|---| | `create_board_item` without `story:` label is DENIED | PASS -- line 48-50 now appends to ERRORS with deny message | | Deny message includes explanation + template-ticket pointer | PASS -- message includes "Every ticket must trace to a user story..." and "See template-ticket" | | `create_board_item` WITH `story:` label still succeeds | PASS -- grep guard unchanged, no story: match skips the block | | Existing board items unaffected (hook only fires on creation) | PASS -- PreToolUse matcher is create_board_item only | | `check-issue-template.sh` recognizes `### Type: Task` | PASS -- `Task\|task)` case added routing to `template-issue-task` | | Task-type issues validate against template-issue-task | PASS -- TEMPLATE_SLUG set correctly | | Feature/Bug/Spike validation unchanged | PASS -- existing cases untouched | ### Review Nits from Issue Comment 4 | Nit | Status | |---|---| | Line 8 comment header updated to "required" | PASS | | Line 48 inline comment updated to "require" | PASS | | Dead WARNINGS infrastructure cleaned up | PASS -- variable removed, advisory output block removed | ### Domain Review - Fail-open pattern preserved (`trap 'exit 0' ERR` at line 17) - `type:` and `arch:` enforcement logic untouched - Case statement alignment in check-issue-template.sh is cosmetic-only, correct - No unrelated changes in the diff (+9/-19, 2 files) ### VERDICT: APPROVE All 7 acceptance criteria met. All 3 review nits addressed. No issues found.

forgejo_admin commented

2026-03-26 22:27:11 +00:00

Author

Contributor

PR #171 Review

DOMAIN REVIEW

Domain: Shell/Bash hooks (Claude Code PreToolUse enforcement layer)

Two files changed, both shell scripts that enforce board item and issue template conventions.

hooks/check-board-item.sh (story: label upgrade)

The change is clean and surgical. The WARNINGS variable and its entire advisory output block are removed. The story: label check is moved from a warning path to the existing error path. The error message is actionable: it tells the user what label is needed, gives an example, explains the "create a user story first" requirement, and points to template-ticket. The fail-open pattern (trap 'exit 0' ERR) is preserved. No dead code remains -- the WARNINGS variable, the advisory JSON output block, and the comment referencing "no warnings" are all cleaned up.

hooks/check-issue-template.sh (Task type routing)

The Task|task) case is added, routing to template-issue-task. I verified that the template-issue-task note exists in pal-e-docs (note ID 721, project: pal-e-agency, tags: active+template) with proper required headings (Type, Scope, Acceptance Criteria, Related). The routing target is valid and the template is well-structured. The case statement alignment changes are cosmetic whitespace normalization -- correct and non-breaking.

Correctness check: The default *) case still maps to template-issue-feature, so unknown types continue to fall through to feature. The existing Bug, Spike, and Nit-Bundle routes are unchanged.

BLOCKERS

None.

No new functionality requiring test coverage (this is hook enforcement logic -- the PR body documents manual test verification, which is appropriate for shell hooks that gate MCP tool calls).
No user input validation concerns (input is JSON from Claude Code's hook framework, not external user input).
No secrets or credentials in code.
No DRY violations in auth/security paths.

NITS

Discovered scope confirmed: The PR body notes session-start-context.sh has a Task type gap. Verified at lines 558-562 of that file -- the "Bug/TODO Tracking" section lists Bug, Feature, Spike but not Task. This is tracked separately per the PR body, which is correct process.
Alignment inconsistency in check-issue-template.sh case statement: The Bug|bug) case uses extra spaces for alignment but the pattern width differs from Spike|spike) and Task|task). After the PR, the alignment looks like:
```
Bug|bug)               TEMPLATE_SLUG=...
Spike|spike)           TEMPLATE_SLUG=...
Task|task)             TEMPLATE_SLUG=...
Nit-Bundle|nit-bundle) TEMPLATE_SLUG=...
*)                     TEMPLATE_SLUG=...
```
The Bug|bug) line has more padding than the others (15 spaces vs 11). Extremely minor -- not blocking.

SOP COMPLIANCE

Branch named after issue: 167-upgrade-story-label-hard-block references issue #167
PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related sections all present
Related references parent issue: "Closes #167" present
No secrets committed
No unnecessary file changes (scope is tight -- only the two files relevant to the issue)
Commit messages: PR title is descriptive and follows conventional commit format
Discovered scope tracked separately (session-start-context.sh Task gap noted in Related)

PROCESS OBSERVATIONS

Change failure risk: Low. The change is a policy tightening (warning to error) with dead code removal. The fail-open wrapper ensures any unexpected failure still allows the tool call.
Deployment frequency impact: Positive. Completing the traceability triangle (type + arch + story) as a hard requirement means all future board items will have full traceability labels, improving kanban observability.
Net code reduction: +9/-19 is a good sign -- removing the WARNINGS infrastructure simplifies the hook and eliminates a code path that was a half-measure.

VERDICT: APPROVED

## PR #171 Review ### DOMAIN REVIEW **Domain:** Shell/Bash hooks (Claude Code PreToolUse enforcement layer) Two files changed, both shell scripts that enforce board item and issue template conventions. **`hooks/check-board-item.sh` (story: label upgrade)** The change is clean and surgical. The `WARNINGS` variable and its entire advisory output block are removed. The story: label check is moved from a warning path to the existing error path. The error message is actionable: it tells the user what label is needed, gives an example, explains the "create a user story first" requirement, and points to `template-ticket`. The fail-open pattern (`trap 'exit 0' ERR`) is preserved. No dead code remains -- the `WARNINGS` variable, the advisory JSON output block, and the comment referencing "no warnings" are all cleaned up. **`hooks/check-issue-template.sh` (Task type routing)** The `Task|task)` case is added, routing to `template-issue-task`. I verified that the `template-issue-task` note exists in pal-e-docs (note ID 721, project: pal-e-agency, tags: active+template) with proper required headings (Type, Scope, Acceptance Criteria, Related). The routing target is valid and the template is well-structured. The case statement alignment changes are cosmetic whitespace normalization -- correct and non-breaking. **Correctness check:** The default `*)` case still maps to `template-issue-feature`, so unknown types continue to fall through to feature. The existing Bug, Spike, and Nit-Bundle routes are unchanged. ### BLOCKERS None. - No new functionality requiring test coverage (this is hook enforcement logic -- the PR body documents manual test verification, which is appropriate for shell hooks that gate MCP tool calls). - No user input validation concerns (input is JSON from Claude Code's hook framework, not external user input). - No secrets or credentials in code. - No DRY violations in auth/security paths. ### NITS 1. **Discovered scope confirmed:** The PR body notes `session-start-context.sh` has a Task type gap. Verified at lines 558-562 of that file -- the "Bug/TODO Tracking" section lists Bug, Feature, Spike but not Task. This is tracked separately per the PR body, which is correct process. 2. **Alignment inconsistency in check-issue-template.sh case statement:** The `Bug|bug)` case uses extra spaces for alignment but the pattern width differs from `Spike|spike)` and `Task|task)`. After the PR, the alignment looks like: ``` Bug|bug) TEMPLATE_SLUG=... Spike|spike) TEMPLATE_SLUG=... Task|task) TEMPLATE_SLUG=... Nit-Bundle|nit-bundle) TEMPLATE_SLUG=... *) TEMPLATE_SLUG=... ``` The `Bug|bug)` line has more padding than the others (15 spaces vs 11). Extremely minor -- not blocking. ### SOP COMPLIANCE - [x] Branch named after issue: `167-upgrade-story-label-hard-block` references issue #167 - [x] PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related sections all present - [x] Related references parent issue: "Closes #167" present - [x] No secrets committed - [x] No unnecessary file changes (scope is tight -- only the two files relevant to the issue) - [x] Commit messages: PR title is descriptive and follows conventional commit format - [x] Discovered scope tracked separately (session-start-context.sh Task gap noted in Related) ### PROCESS OBSERVATIONS - **Change failure risk:** Low. The change is a policy tightening (warning to error) with dead code removal. The fail-open wrapper ensures any unexpected failure still allows the tool call. - **Deployment frequency impact:** Positive. Completing the traceability triangle (type + arch + story) as a hard requirement means all future board items will have full traceability labels, improving kanban observability. - **Net code reduction:** +9/-19 is a good sign -- removing the WARNINGS infrastructure simplifies the hook and eliminates a code path that was a half-measure. ### VERDICT: APPROVED