From 64d2cdadc670c049d1f0c9769f274e0c84a7bcf5 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 16 Jul 2026 01:13:36 -0400 Subject: [PATCH 1/3] fix(code-tidying): route tidy's PR creation through the canonical gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tidy`'s Phase H told the agent to call `gh pr create` directly, bypassing `source-control:pull-request`'s canonical create flow — issue-linkage resolution with an issue-existence check, injection-safe PR-body assembly (quoted heredoc + parameter-expansion concat), and a pre-create gate that requires a valid closing keyword or explicit opt-out before the PR ships. Phase H now applies the same optional-plugin graceful-degrade pattern `tidy` already uses elsewhere (Phase C for `discovery`, Phase D for `work-items`): invoke `/pull-request create` when `source-control` is installed — its own stage-and-commit step is a no-op since Phase E already committed the tidyings — falling back to the same invariants applied inline when it isn't. This keeps `code-tidying` and `source-control` decoupled (no cross-plugin runtime dependency; either plugin remains independently installable) while closing the actual gap: an agent following Phase H literally never bypasses the gate again, regardless of which plugins happen to be present. No related issue: closes decisions #3/#42 in the melodic-software issue/PR consistency initiative's Decisions Log (an internal tracking artifact, not a GitHub issue in this repo). --- plugins/code-tidying/skills/tidy/SKILL.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/plugins/code-tidying/skills/tidy/SKILL.md b/plugins/code-tidying/skills/tidy/SKILL.md index 4250ae99b..dda3fe899 100644 --- a/plugins/code-tidying/skills/tidy/SKILL.md +++ b/plugins/code-tidying/skills/tidy/SKILL.md @@ -132,7 +132,11 @@ Self-review by the producing context is enough here — a fresh-context verifier ### Phase H — Ship -Create the PR (`gh pr create`) with title: +Never call `git commit` or `gh pr create` directly — Phase E already committed the tidyings, so what's left is PR creation, and that has a canonical gate (issue-linkage resolution, injection-safe body assembly, a pre-create check for a valid closing keyword or explicit opt-out) that a bare `gh pr create` skips entirely. + +If the `source-control` plugin is installed, invoke `/pull-request create`. Its stage-and-commit step is a no-op here (tree is already clean from Phase E), so it goes straight to rebase-check, issue-linkage resolution, and gated PR creation. Supply it this PR's title and body content: + +Title: ```text (): @@ -144,13 +148,15 @@ Examples: - `docs(skills): repair stale cross-references` - `chore(tools): apply shellcheck/shfmt drift across tools/*.sh` -PR body sections: +Body sections: - **Summary** — 1-3 bullets: which lane, which tidyings, anchor commit. - **Tidyings applied** — table: tidying type → file → line range → LOC delta. - **Deferred items** — if the scope budget capped the run, link filed issue numbers. - **Test plan** — verification commands run + results. +If `source-control` isn't installed, apply the same invariants inline: resolve issue-linkage before writing a closing keyword (`Closes #N` only after confirming issue #N exists in this repo — e.g. `gh issue view N`; otherwise state `No related issue: `), assemble the body via a quoted heredoc (`<<'EOF'`) plus parameter-expansion concat rather than an unquoted `< --watch`) until green. Address review-bot findings: verify each against the current code — fix the correct ones, rebut the incorrect ones with evidence. **Manual merge by a human** — this skill does NOT auto-merge. ## Global HARD/SOFT EXCLUSIONS From c4c95a889dfc84b6e6e1a7c21031610919fa4ba5 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 16 Jul 2026 10:22:25 -0400 Subject: [PATCH 2/3] fix(code-tidying): move audit trail to a follow-up comment, not the PR body The canonical /pull-request create body template is fixed to Summary + Test plan with no slot for tidy-specific content; extending it would reopen the bypass surface this PR exists to close. Post the Tidyings applied / Deferred items sections as a follow-up PR comment instead when source-control is installed, so the canonical gate stays the sole body-owner while the audit trail still lands in the PR timeline. --- plugins/code-tidying/skills/tidy/SKILL.md | 24 +++++++++++++++---- .../skills/tidy/reference/scope-budget.md | 4 ++-- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/plugins/code-tidying/skills/tidy/SKILL.md b/plugins/code-tidying/skills/tidy/SKILL.md index dda3fe899..eb5e6d4e4 100644 --- a/plugins/code-tidying/skills/tidy/SKILL.md +++ b/plugins/code-tidying/skills/tidy/SKILL.md @@ -134,7 +134,7 @@ Self-review by the producing context is enough here — a fresh-context verifier Never call `git commit` or `gh pr create` directly — Phase E already committed the tidyings, so what's left is PR creation, and that has a canonical gate (issue-linkage resolution, injection-safe body assembly, a pre-create check for a valid closing keyword or explicit opt-out) that a bare `gh pr create` skips entirely. -If the `source-control` plugin is installed, invoke `/pull-request create`. Its stage-and-commit step is a no-op here (tree is already clean from Phase E), so it goes straight to rebase-check, issue-linkage resolution, and gated PR creation. Supply it this PR's title and body content: +If the `source-control` plugin is installed, invoke `/pull-request create`. Its stage-and-commit step is a no-op here (tree is already clean from Phase E), so it goes straight to rebase-check, issue-linkage resolution, and gated PR creation. Supply it this PR's title and body content — the canonical flow's body template is fixed to Summary + Test plan (`plugins/source-control/skills/pull-request/reference/create.md` §2.4.1), so give it only those two sections; tidy's own audit-trail content goes in a follow-up comment (below), not the PR body: Title: @@ -151,11 +151,25 @@ Examples: Body sections: - **Summary** — 1-3 bullets: which lane, which tidyings, anchor commit. -- **Tidyings applied** — table: tidying type → file → line range → LOC delta. -- **Deferred items** — if the scope budget capped the run, link filed issue numbers. - **Test plan** — verification commands run + results. -If `source-control` isn't installed, apply the same invariants inline: resolve issue-linkage before writing a closing keyword (`Closes #N` only after confirming issue #N exists in this repo — e.g. `gh issue view N`; otherwise state `No related issue: `), assemble the body via a quoted heredoc (`<<'EOF'`) plus parameter-expansion concat rather than an unquoted `<` back on completion (its own §2.6 "Report and stop"). Immediately post one follow-up comment on that PR with `tidy`'s own audit trail — content the canonical body template has no slot for: + +```bash +gh pr comment --body-file - <<'EOF' +## Tidyings applied + + + +## Deferred items + + +EOF +``` + +Omit the "Deferred items" section entirely when nothing was deferred — don't post an empty table either; skip the whole comment if Phase D found nothing to report beyond what's already in the PR body. + +If `source-control` isn't installed, apply the same invariants inline: resolve issue-linkage before writing a closing keyword (`Closes #N` only after confirming issue #N exists in this repo — e.g. `gh issue view N`; otherwise state `No related issue: `), assemble the body via a quoted heredoc (`<<'EOF'`) plus parameter-expansion concat rather than an unquoted `< --watch`) until green. Address review-bot findings: verify each against the current code — fix the correct ones, rebut the incorrect ones with evidence. **Manual merge by a human** — this skill does NOT auto-merge. @@ -191,7 +205,7 @@ Full template: [reference/scope-budget.md](reference/scope-budget.md). Summary: - Every item the scope budget cuts becomes one filed work item. - Title format: `(): `. - Body must include: rationale, file list, scope estimate (LOC + files), and a link to the parent tidy PR. -- The PR body's "Deferred items" section links every filed item by number. +- Phase H's "Deferred items" follow-up comment (or, when `source-control` isn't installed, the PR body's own "Deferred items" section) links every filed item by number. ## Gotchas diff --git a/plugins/code-tidying/skills/tidy/reference/scope-budget.md b/plugins/code-tidying/skills/tidy/reference/scope-budget.md index 58dc61713..39a911403 100644 --- a/plugins/code-tidying/skills/tidy/reference/scope-budget.md +++ b/plugins/code-tidying/skills/tidy/reference/scope-budget.md @@ -51,7 +51,7 @@ When the hunt phase produces more candidates than fit in the budget: 3. **Defer the rest.** For each unselected candidate above a "would-be-worth-doing" threshold (i.e., not trivial micro-tidyings — those just go away), file a work item using the deferred-items template below: via `/work-items:track add` when that plugin is installed, else `gh issue create`, else present the list to the user. -4. **Record the deferred issue numbers in the PR body** under a `## Deferred items` section. This makes the PR's review obvious-by-default: "here's what I did, here's what I parked for next time, here are the issue numbers to hold me accountable." +4. **Record the deferred issue numbers** under a `## Deferred items` section — in Phase H's follow-up PR comment when `source-control` is installed, otherwise directly in the PR body. This makes the PR's review obvious-by-default: "here's what I did, here's what I parked for next time, here are the issue numbers to hold me accountable." ### Greedy vs. optimal selection @@ -128,6 +128,6 @@ No upper bound on deferred issues per run. If a single run defers >10 items, tha 1. **Phase D (Hunt + prioritize + scope-budget enforce)** — after building the prioritized findings table, sum the LOC deltas. Apply the greedy selection. 2. **Phase E (Implement)** — periodically check actual LOC delta against the running estimate (`git diff --stat origin/...HEAD`). This measures the full branch diff — all commits since the branch point, not just uncommitted changes relative to HEAD. If actual exceeds estimated by >25%, stop the current tidying mid-flight and re-budget. -3. **Phase H (Ship)** — the PR body's `## Deferred items` section comes directly from this protocol's filed-issue list. +3. **Phase H (Ship)** — the `## Deferred items` section (follow-up comment, or PR body when `source-control` isn't installed) comes directly from this protocol's filed-issue list. If the cap numbers themselves need to change, that's a research-driven update — not a tidy. See the SELF-UPDATE EXTRA HARD list in `reference/exclusions.md`. From 13390787b4c77ae50d747ecb0f62831d202127d5 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 16 Jul 2026 10:28:12 -0400 Subject: [PATCH 3/3] fix(code-tidying): make the follow-up audit comment unconditional The prior wording let an agent judge the Tidyings-applied table as "nothing beyond what's already in the PR body" and skip the whole comment -- but that table is never in the body anymore, so skipping silently reproduces the exact bug this PR fixes. Only Deferred items stays conditional. --- plugins/code-tidying/skills/tidy/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/code-tidying/skills/tidy/SKILL.md b/plugins/code-tidying/skills/tidy/SKILL.md index eb5e6d4e4..4b6110c39 100644 --- a/plugins/code-tidying/skills/tidy/SKILL.md +++ b/plugins/code-tidying/skills/tidy/SKILL.md @@ -167,7 +167,7 @@ gh pr comment --body-file - <<'EOF' EOF ``` -Omit the "Deferred items" section entirely when nothing was deferred — don't post an empty table either; skip the whole comment if Phase D found nothing to report beyond what's already in the PR body. +The comment itself is never optional when a PR was created — "Tidyings applied" is never empty at that point (Phase D's empty-PR-avoidance rule means no PR gets created when there's nothing to tidy), and it's the only place this content appears now that the canonical body template has no slot for it. Only the "Deferred items" subsection is conditional: omit it when nothing was deferred, and never post it as an empty table. If `source-control` isn't installed, apply the same invariants inline: resolve issue-linkage before writing a closing keyword (`Closes #N` only after confirming issue #N exists in this repo — e.g. `gh issue view N`; otherwise state `No related issue: `), assemble the body via a quoted heredoc (`<<'EOF'`) plus parameter-expansion concat rather than an unquoted `<