pal-e-docs

Review: Validate: claude-custom (9 PRs, session restart)

review-511-2026-03-28 Doc

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

Checked against template-issue (Task type — Scope replaces File Targets):
  • [x] Type — Task
  • [x] Scope — present, replaces File Targets per Task convention
  • [x] Acceptance Criteria — 6 items
  • [x] Related — references project-pal-e-platform
  • [x] Lineage — present (embedded in Scope as "Lineage:" line)
  • [x] User Story — present (embedded in Scope as "User Story:" line)
  • [x] Test Expectations — present (embedded in Scope as "Test Expectations:" line)
  • [x] Constraints — present (embedded in Scope as "Constraints:" line)
  • [ ] Repo — MISSING. Should be forgejo_admin/claude-custom
  • [ ] Context — MISSING as standalone section. Background is folded into Scope.
  • [ ] Checklist — MISSING
Note: Lineage, User Story, Test Expectations, and Constraints are all embedded within the Scope block as bold inline labels rather than as standalone ### heading sections. The content is present but the structure does not match template headers.

Traceability

  • [x] story:pm-scope — platform operator validation story, present on board item
  • [x] arch:hooks — hooks architecture component, present on board item
  • [x] Forgejo issue — forgejo_admin/claude-custom#208, open
All three traceability legs are present and correct.

File Targets

N/A — Task type uses Scope instead of File Targets. No file targets required.
For reference, the 9 PRs touched these files across claude-custom:
  • hooks/cleanup-worktrees.sh (PR #196)
  • hooks/pre-spawn-freshness.sh, settings.json (PR #197)
  • agents/betty-sue.md (PR #198)
  • hooks/forgejo-helper.sh, hooks/post-mcp-merge-rebase.sh, hooks/post-merge-rebase.sh (PR #199)
  • hooks/board-item-on-merge.sh, hooks/forgejo-helper.sh, hooks/post-mcp-merge-rebase.sh, hooks/remind-update-docs.sh, tests/test_parse_merged_status.sh (PR #201)
  • hooks/check-note-template.sh, tests/test_check_note_template.sh (PR #200)
  • agents/penny.md, spikes/133-penny-mcp-inventory.md (PR #204)
  • hooks/check-branch-freshness.sh, settings.json (PR #206)
  • commands/update-docs.md (PR #207)
All 9 modified hooks pass bash -n syntax check. All 5 test suites pass (115/115 tests).

Repo Placement

OK. Issue is filed on forgejo_admin/claude-custom and all 9 PRs are on the same repo. No cross-repo concerns.

Dependencies

  • [x] All 9 PRs merged to main — satisfied
  • [x] ~/claude-custom checkout is on main with all PRs present — satisfied
  • [ ] Session restart — pending (requires human action, cannot be verified by the reviewing agent)
Board context:
  • Item #478 (in_progress): "Spike: Note type system audit" — same repo, independent work, not a blocker.
  • Item #523 (done): "Enforce backlog-first column" (PR #212) — merged around the same session but NOT in scope of this ticket's 9 PRs. Separate validation scope.
  • Items #518, #519 (backlog): downstream validation pipeline features, not blockers.

Acceptance Criteria

6 ACs listed. Assessment:
  • AC 1: "Hooks load on session restart without errors" — verifiable but requires a NEW session. Cannot be tested from within the current session. Mark as manual step.
  • AC 2: "Test suites pass (run any hook test commands)" — verifiable. 5 test suites exist but the issue does not list the specific commands. Commands are: bash tests/test_parse_merged_status.sh, bash tests/test_check_note_template.sh, bash tests/test_validate_branch_name.sh, bash tests/test_check_board_advance.sh, bash tests/test_block_groupme_send.sh. All 115 tests currently pass.
  • AC 3: "No regressions in agent behavior after merges" — NOT objectively verifiable. Too vague. What behaviors? Which agents? Which regressions?
  • AC 4: "Pipeline verified (N/A — no CI)" — correctly marked N/A.
  • AC 5: "Deployment confirmed (session restart)" — duplicates AC 1. For claude-custom, deployment IS session restart.
  • AC 6: "Features validated (hooks load, tests pass)" — duplicates AC 1 + AC 2.
Effective unique ACs: 2 (hooks load on restart + tests pass). ACs 3, 5, 6 are either vague or duplicative of 1 and 2.

Blast Radius

  • Hooks are hardlinked from ~/claude-custom/hooks/ to ~/.claude/hooks/. Changes are live immediately on git pull — no deployment step beyond session restart.
  • 9 PRs touched 17 files total: 9 hooks, 2 settings.json changes, 2 agent docs, 1 command doc, 1 spike doc, 2 test files.
  • The modified hooks fire on every PreToolUse, PostToolUse, and SessionStart event across ALL projects — blast radius is system-wide.
  • No CI pipeline exists for this repo, so these changes were merged without automated gate. The test suites are run manually.
  • Rollback: git revert on individual PRs is straightforward since all are merge commits on main.

Decomposition Assessment

Apply the three-thing limit and five-minute rule:
  • Does the ticket have >3 discrete changes? No — this is a validation task, not a code change task. Zero changes required.
  • Would an agent need >5 minutes? No — running all tests takes <30 seconds. Syntax checking takes <5 seconds.
  • Are there independent subtasks that could be parallelized? No.
No decomposition needed.
However: AC 1 (session restart verification) is a human action, not an agent action. The agent can run tests and verify syntax but cannot restart its own session. The ticket should explicitly mark this as a manual validation step.

Recommendation

  • [BODY] Add missing ### Repo section: forgejo_admin/claude-custom
  • [BODY] Add missing ### Checklist section (standard items: tests pass, no unrelated changes; PR items N/A for validation)
  • [BODY] Restructure embedded content into proper template sections — move Lineage, User Story, Context, Test Expectations, Constraints out of Scope into their own ### headers
  • [BODY] Remove duplicate ACs: collapse AC 5 and AC 6 into AC 1 + AC 2. Either sharpen AC 3 ("No regressions") into something testable (e.g., "SessionStart hooks complete without error output") or remove it.
  • [BODY] Add explicit test commands to Test Expectations: bash tests/test_parse_merged_status.sh, bash tests/test_check_note_template.sh, bash tests/test_validate_branch_name.sh, bash tests/test_check_board_advance.sh, bash tests/test_block_groupme_send.sh
  • [BODY] Mark AC 1 (session restart) as a manual validation step — agent cannot restart its own session.