feat(source-control): setup skill supports pr_body_required_sections (#1032)

[kyle-sexton]

Closes #1032

Summary

Completes #975's adoption path: /source-control:setup was the last mechanism-owning surface that
didn't know about pr_body_required_sections (added in #1029), so a team using the sanctioned
guided setup flow couldn't configure the required-section scaffold or see which layer supplies it —
the only path was hand-editing .claude/source-control.md directly.

• check reports a pr_body_required_sections row on the effective-configuration table,
  resolving to the plugin's portable default (Summary, Test plan) with won by: plugin default
  when no layer sets it — a named, reportable value rather than a blank row, since "unset" is itself
  a meaningful state for this key.
• apply's interview offers setting it, deliberately recommending only the plugin's own portable
  default and never proposing a Related/linked-issue section (or any other org-specific list) as a
  universal default — it asks what the repo's actual convention requires (a PR template, a CI gate
  like pr-issue-linkage, team practice) rather than inventing one, per the plugin's Two-lane
  convention posture.
• The written-config template gains the matching ## pr_body_required_sections section, at
  parity with every other per-key surface (subject_pattern, pr_title_pattern, trailer_policy,
  pr_body_attribution).
• New evals 13-14 cover the check-report default and the agnostic-interview behavior.
• Plugin version bumped 0.18.0 → 0.19.0 with a matching CHANGELOG.md entry.

This branch was rebased onto feat/975-required-body-sections (then onto main after #1029 merged)
so it includes #1029's pr_body_required_sections key and #1055's setup-template preamble together —
verified both coexist correctly in the write template (preamble header, then the per-key section
list ending with the new ## pr_body_required_sections section).

Test plan

• npx markdownlint-cli2 over the changed skill doc and CHANGELOG: 0 errors.
• node -e "JSON.parse(...)" over the updated evals/evals.json: valid JSON.
• bash scripts/check-changelog-parity.sh --check-bump origin/main: PASS.
• bash scripts/validate-plugins.sh: every plugin manifest and the marketplace catalog validate.
• Manually verified the merged write template renders correctly end to end (preamble +
  pr_body_required_sections section both present, no duplication) after rebasing onto main.

Related

• pull-request skill: pre-create gate must verify non-empty '## Related' section #975 / feat(source-control): configurable pr_body_required_sections scaffold + gate (#975) #1029 — the mechanism and portable-default key this PR completes the adoption path for.
• feat(source-control): setup writes a self-describing team convention file #1055 — the setup-template self-describing-preamble change this branch was rebased across; both
  changes touch the same "Write the config" template step and coexist without conflict.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4ed782aded

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 2m 50s —— View job

---

PR Review

• Gather context and understand the request
• Examine changed files
• Review for correctness, security, and CLAUDE.md alignment
• Post findings

---

Overall: This is a clean, well-scoped PR. It completes the pr_body_required_sections adoption path by adding the key to all three surfaces of the setup skill (check table, apply interview, write template) and adds matching evals. The changes are consistent with the existing patterns for other optional keys, and the version bump + CHANGELOG entry follow conventions.

No blocking issues. One pre-existing bug (from #1029, not introduced here) worth tracking.

---

Findings

**🟣 Pre-existing — AWK inline-comment stripping drops visible text before /) in_comment = 0 next # skips the ENTIRE line, including any visible text before would have "Ran smoke tests" stripped before the non-empty check, causing a false "missing or empty" block. The section is valid to readers and to GitHub's rendering. The fix is to strip just the comment span (e.g. gsub(//, "", $0) for the inline case) rather than next-ing the whole line. This was introduced in #1029 — a follow-up PR or a fixup commit there would be the right home for it.

---

🟡 Nit — raw.githubusercontent.com URL hardcodes to main

SKILL.md:236

[`docs/conventions/pr-body-convention/README.md`](https://raw.githubusercontent.com/melodic-software/claude-code-plugins/main/docs/conventions/pr-body-convention/README.md)

Hardcoding main means a rename or move of that file silently breaks the link in already-distributed plugin installations. A relative path isn't an option here given the plugin's no-../-reach-out constraint, so this is acceptable as-is — just worth noting that if the target file moves, installed copies won't auto-update. Pinning to a commit SHA would be more stable but harder to maintain. Low priority.

---

Changes reviewed

File	Assessment
plugin.json	Version bump 0.18.0 → 0.19.0 — correct for a new-feature minor
CHANGELOG.md	Entry is accurate and follows Keep a Changelog format
skills/setup/SKILL.md	Check table, apply interview step, and write template all updated consistently; list-valued-key note is correct and well-placed
skills/setup/evals/evals.json	Evals 13–14 cover the two new behaviors (check default resolution and agnostic-interview posture); expectations are precise

No secrets, no injection surfaces, no multi-tenancy concerns — all blocking checks from REVIEW.md pass.

[claude]

Claude finished @kyle-sexton's task in 1m 49s —— View job

---

Security Review — PR #1059

• Gather context
• Read changed files
• Analyze for security issues
• Post findings

---

Scope: plugin.json, CHANGELOG.md, skills/setup/SKILL.md, skills/setup/evals/evals.json

Summary: No CRITICAL or IMPORTANT security issues found. One low-risk observation below.

---

Bash snippets in SKILL.md — no injection surface introduced

The two bash blocks added/retained in the apply verification steps use "$REPO_ROOT" and "$OVERLAY" with proper quoting throughout, and all path segments appended to them are hardcoded string literals (.claude/source-control.md, .claude/source-control.local.md). No user-controlled or external input flows into any command substitution or eval. The $IGNORE_MATCH capture from git check-ignore -v is echoed to stderr only — not passed to eval or used in a subsequent git invocation. No injection surface is introduced.

REPO_ROOT is resolved from the Claude Code runtime variable ${CLAUDE_PROJECT_DIR} (operator-controlled, not end-user-supplied) and falls back to git rev-parse --show-toplevel. Both sources are resolved into a scalar before any git call. No taint issue.

---

SUGGESTION — raw.githubusercontent.com URL pinned to main (mutable ref) _{Confidence: HIGH (the ref is mutable; exploitability is LOW within the same org)}

Location: SKILL.md line 236

[`docs/conventions/pr-body-convention/README.md`](https://raw.githubusercontent.com/melodic-software/claude-code-plugins/main/docs/conventions/pr-body-convention/README.md)

This URL resolves to the live main branch rather than a pinned commit or tag. If Claude follows this link during skill execution (e.g., to gather more context on allowed heading values), it reads content from whatever main holds at that moment — not the version the plugin was installed from. In the scenario where main is compromised or the file is edited to contain adversarial instruction text, a running skill instance could be influenced.

Within the same org/repo the risk is low, and the URL is currently in prose instruction text (not an auto-fetched resource in the skill's hot path). Pinning to a release tag or commit SHA would eliminate the mutable-ref window entirely.

Fix this →

---

Out-of-scope note on Codex finding

The Codex review flags a logic flaw in create.md line 390 (HTML comment stripping drops visible text on the same line before the non-empty check). That file is not changed in this PR. The finding is real and worth tracking, but it predates this diff and is not introduced here.

---

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: abb9607576

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[kyle-sexton]

🔒 babysit-prs lane claiming this PR for on-branch fix work this cycle (amendment-round: 16, safe tier). Will fix clear branch-owned findings and push; will not resolve threads or merge (safe tier).

[claude]

Claude encountered an error after 1s —— View job

---

I'll analyze this and get back to you.

[kyle-sexton]

Preserve visible text around inline comments — real bug, fixed (commit ada30f3). Rewrote the comment-aware scan to strip only the comment SPAN (single- or multi-line, via a strip_comment_span helper), preserving visible text before/between/after a span on the same line — a genuinely comment-only line, or a fully-hidden middle line of a multi-line span, still contributes nothing. While in there, did the requested defensive pass over the fence/comment interaction and found (and fixed) two more latent bugs: an already-open fence or comment now takes absolute priority over a NEW fence/comment marker inside it (each construct's content is parsed literally by GFM, never nested — a <!-- inside a fence stays literal code, a `` inside a comment stays literal comment text), and heading/exit-boundary detection now runs before comment-open detection so a heading line carrying a trailing inline comment (## Related <!-- draft -->) is read as a real exit boundary instead of being misrouted into comment handling. Verified locally against 7 scenarios (the 4 you named plus the 3 defensive-pass cases) before writing the change into the doc. New eval 18 covers the regression.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[kyle-sexton]

Write explicit default when overriding section lists — real bug, fixed (commit ada30f3). The interview guidance now states explicitly: because pr_body_required_sections falls through per key, omitting the H2 in an overlay/team write does NOT reset to the portable default when an earlier layer already sets the key — it keeps inheriting that layer's list. When the intent is to reset back to the portable default OVER a lower layer that sets something else, the explicit default bullet list must be written (not an omitted section), with a one-line reason stated ("written explicitly to override the team layer's list, not merely to restate the default"). New eval 15 covers the regression.

[github-actions]

Warning

Automated review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

• Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/29957086187
• Last SDK result: {"subtype":"success","is_error":true,"num_turns":1,"duration_ms":527,"total_cost_usd":0}

Re-running the job, or pushing a new commit, will retry the review.

[github-actions]

Warning

Automated security review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

• Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/29957084953
• Last SDK result: {"subtype":"success","is_error":true,"num_turns":1,"duration_ms":404,"total_cost_usd":0}

Re-running the job, or pushing a new commit, will retry the review.