Code Review: PR #484

Commit: 5c4be13498b6ad67e5035c208ed70c7939fa3a74

Summary

Adds two intent-named skills — /report-apexyard-bug and /request-apexyard-feature — that file structured bug reports / feature requests about the apexyard framework itself upstream to me2resh/apexyard, distinct from /bug and /feature which file into the adopter's own project tracker. Ships AgDR-0059, refreshes the CLAUDE.md skills table + counts (57→59), and updates site/* counts. No new hook/role/agent.

Checklist Results

• Architecture & Design: Pass — thin, well-scoped skills; reuse existing plumbing (issue-skill marker, leak backstop)
• Code Quality: Pass — docs-only; consistent with /bug conventions
• Testing: Pass — site-counts green; no executable tests applicable to skill docs
• Security: Pass — leak-protection is load-bearing and correctly wired (see below)
• Performance: N/A
• PR Description & Glossary: Pass — Glossary present; AgDR linked via <!-- agdr: --> marker
• Summary Bullet Narrative: Pass — bullets are narrative (what + why)
• Technical Decisions (AgDR): Pass — AgDR-0059 present at HEAD, referenced in body
• Adopter Handbooks: N/A — no handbooks loaded (diff is framework docs only)

Issues Found

None blocking.

Verification performed

• AgDR linkage — body opens with <!-- agdr: docs/agdr/AgDR-0059-upstream-feedback-skills.md -->; file confirmed present at HEAD SHA. AgDR follows the template (context / options / decision / consequences / artifacts) and records the three load-bearing calls: upstream target, named-not-/feedback, mandatory leak-scrubbing.
• Leak protection (the load-bearing safety property) — verified on both skills: a mandatory callout, explicit reference to .claude/rules/leak-protection.md, naming of the block-private-refs-in-public-repos.sh backstop (confirmed both exist on disk), authoring-time scrub instruction, a re-scan of the rendered body before the confirm step, and the <!-- private-refs: allow --> escape gated behind explicit user confirmation only. Correct and complete.
• Upstream resolution — git remote get-url upstream with fallback to the canonical me2resh/apexyard; confirms with the user (or defaults to canonical) if a fork owner crept into a mis-set upstream. Never defaults to the adopter's fork or a managed project. Sound.
• Issue-skill marker gate — confirmed by reading require-skill-for-issue-create.sh: lines 132–138 accept ANY non-empty marker value (no fixed allowlist), so report-apexyard-bug / request-apexyard-feature pass the gate without a hook change, exactly as the PR claims. Both skills write the marker at entry and rm -f it on every exit path.
• Skill conventions — frontmatter (name/description/argument-hint/allowed-tools) matches /bug; one-question-at-a-time; confirm-before-create (yes/edit/cancel); framework-version capture for actionable reports.
• Counts — actual skill count on disk at HEAD = 59. test_site_counts.sh PASSES (skills 59, hooks 37, roles 20; payload meta within 5%). CLAUDE.md table + counts and all site/* headline counts flip 57→59 consistently.
• Commit SHA — HEAD 5c4be134... unchanged during review.

Suggestions

• nit (non-blocking, pre-existing): site/index.md.gen narrative prose still reads "and 46 more" / "54 of these in total". Not introduced by this PR (the diff only touches headline count lines) and out of scope for the site-counts test — worth a follow-up sweep but no action needed here.

Verdict

APPROVED (posted as comment — GitHub blocks self-approval)

🤖 Reviewed by Rex (Code Reviewer Agent)
📌 Reviewed commit: 5c4be13498b6ad67e5035c208ed70c7939fa3a74

Code Review (re-review): PR #484

Commit: bc9f1c609b886f5ba6aa4649d2ce97a45275cd22 (new HEAD)
Prior approved commit: 5c4be13

Summary

Fast confirmation re-review after a mechanical rebase onto the updated dev (#483 merged first, bumping the hooks count). No fresh full review — verifying the rebase introduced nothing beyond the count-line conflict resolution.

Verification

• ✅ Skill files + AgDR byte-identical — .claude/skills/report-apexyard-bug/SKILL.md, .claude/skills/request-apexyard-feature/SKILL.md, and docs/agdr/AgDR-0059-upstream-feedback-skills.md are byte-for-byte identical to the approved 5c4be13 tree (shasum match on all three).
• ✅ Only count-line resolution in site files — site/llms.txt and site/llms-full.txt differ from the prior HEAD solely on 37 hooks → 38 hooks (hooks count carried forward from #483; skills=59 from this PR). No skill content, no logic, no AgDR change.
• ✅ No leftover conflict markers — clean across CLAUDE.md, all site/* files, and the full gh pr diff (no <<<<<<< / ======= / >>>>>>> in added lines).
• ✅ site-counts internally consistent — test_site_counts.sh PASS: skills=59, hooks=38, roles=20 match actuals across CLAUDE.md + all scanned site files; per-page LLM payload meta tags within 5%.

Note: a raw git diff 5c4be13 bc9f1c6 surfaces #483's reindex-hook content as well — that's a cross-rebase tree artifact (the dev base shifted), not a change in this PR. The authoritative gh pr diff shows exactly the expected PR contents (two skill files + AgDR-0059 + count bumps).

Verdict

APPROVED — carries forward the prior approval at the new HEAD. The rebase was clean and limited to the count-line resolution.

🤖 Reviewed by Rex (Code Reviewer Agent)
📌 Reviewed commit: bc9f1c609b886f5ba6aa4649d2ce97a45275cd22