feat(github): show every target's plan, not just the reviewed one

[aparajon]

Targets under a targets: list are planned each and converge on their own, so
they are free to hold different schemas and usually do. The plan comment already
named that divergence, but it still rendered only the reviewed plan's DDL. An
operator was being asked to authorize work the comment never showed them.

Each distinct plan now renders under the members that would run it. The reviewed
plan stays open, the rest collapse behind a line saying the apply runs them too,
and a group already at the desired schema is named rather than hidden. The
summary line counts the rollout rather than the reviewed plan alone.

A primary already at the desired schema also no longer short-circuits the whole
comment to "No schema changes detected" while its siblings still have work.

In the example below primary/testapp_1 is the reviewed target and alters
users, primary/testapp_2 plans the same change, and primary/testapp_3 plans
that alter plus an index.

Before                                          After

+------------------------------------------+    +------------------------------------------+
| Planned separately for 3 targets         |    | Planned separately for 3 targets         |
| 2 distinct plans. Each applies its own   |    | 2 distinct plans. Each applies its own   |
+------------------------------------------+    +------------------------------------------+
| ALTER users ADD email                    |    | [open] testapp_1 (primary), testapp_2    |
|                                          |    |        - 1 DDL statement                 |
|   x testapp_3's second statement is      |    |        ALTER users ADD email             |
|     nowhere on the comment               |    +------------------------------------------+
|                                          |    | [+] testapp_3 - 2 DDL statements         |
|                                          |    +------------------------------------------+
|                                          |    | ! Applying runs each target's own plan,  |
+------------------------------------------+    |   including the collapsed one            |
| Plan: 1 table to alter                   |    +------------------------------------------+
|   x counts the primary alone             |    | Plan: 2 distinct plans on 3 targets      |
+------------------------------------------+    +------------------------------------------+

This upholds RV-3: the consent an operator gives is specific to what will
run, and what will run is every target's plan, not one member's.

Targets converging: before

Schema Change Plan — Production

Database: testapp | Type: MySQL | Schema Name: testapp

Requested by @jackjackbits at 2026-01-01 00:00:00 UTC · planned from abcdef1

✅ Planned separately for all 3 targets (primary/testapp_1, primary/testapp_2, primary/testapp_3) — 2 need this change, 1 is already at this schema.

ALTER TABLE `users` ADD COLUMN `email` varchar(255);

📋 Plan: 1 table to alter

---

▶️ To apply all schema changes from this PR, comment:

schemabot apply -e production

Targets converging: after

Schema Change Plan — Production

Database: testapp | Type: MySQL | Schema Name: testapp

Requested by @jackjackbits at 2026-01-01 00:00:00 UTC · planned from abcdef1

✅ Planned separately for all 3 targets (primary/testapp_1, primary/testapp_2, primary/testapp_3) — 2 need this change, 1 is already at this schema.

primary/testapp_1 (primary), primary/testapp_3 — 1 DDL statement

ALTER TABLE `users` ADD COLUMN `email` varchar(255);

primary/testapp_2 — already at this schema, nothing to apply.

📋 Plan: 1 DDL statement on 2 of 3 targets

---

▶️ To apply all schema changes from this PR, comment:

schemabot apply -e production

Targets diverging: before

Schema Change Plan — Production

Database: testapp | Type: MySQL | Schema Name: testapp

Requested by @jackjackbits at 2026-01-01 00:00:00 UTC · planned from abcdef1

✅ Planned separately for all 3 targets (primary/testapp_1, primary/testapp_2, primary/testapp_3) — 2 distinct plans. Each target applies its own.

ALTER TABLE `users` ADD COLUMN `email` varchar(255);

📋 Plan: 1 table to alter

---

▶️ To apply all schema changes from this PR, comment:

schemabot apply -e production

Targets diverging: after

Schema Change Plan — Production

Database: testapp | Type: MySQL | Schema Name: testapp

Requested by @jackjackbits at 2026-01-01 00:00:00 UTC · planned from abcdef1

✅ Planned separately for all 3 targets (primary/testapp_1, primary/testapp_2, primary/testapp_3) — 2 distinct plans. Each target applies its own.

`primary/testapp_1` (primary), `primary/testapp_2` — 1 DDL statement

ALTER TABLE `users` ADD COLUMN `email` varchar(255);

`primary/testapp_3` — 2 DDL statements

ALTER TABLE `users` ADD COLUMN `email` varchar(255);

ALTER TABLE `users` ADD INDEX `idx_email`(`email`);

⚠️ Applying runs each target's own plan, including the ones collapsed above.

📋 Plan: 2 distinct plans on 3 targets

---

▶️ To apply all schema changes from this PR, comment:

schemabot apply -e production

Opened by Claude (Claude Opus 5).

[Kiran01bm]

🤖 Review findings - created by Kiran's code review agent - for schemabot/pull/1425, f2ca09b.

Verdict: 4 findings — 2 blocking (unswept multi-env renderer, per-group DDL budget), 2 non-blocking.

Blocking

The multi-environment plan comment was not swept, so the misreport this PR fixes survives there. writeEnvironmentPlanSection still short-circuits on if totalChanges == 0 (the reviewed primary's count) and never calls writePlanGroups, while plan.go:466 hands it the same group-bearing rollup. Running head with staging changed and production's primary converged prints "2 need this change, 1 is already at this schema" immediately followed by "✅ No schema changes detected", dropping the two changing targets' DDL — and this is the auto-plan surface for any database with 2+ environments. Precedent is to sweep both renderers (writeDeploymentDrift was wired into both when drift landed).

The per-comment DDL budget is re-opened for every plan group, so the comment can exceed GitHub's hard 65536-char cap. writePlanGroups now calls writeKeyspaceChanges once per group and each call re-runs newDDLBlockBudget with remaining: maxCommentDDLLen, contradicting fence.go's "a budget shared across the blocks the comment renders"; vschemaDiffBudget resets the same way. Three ~25KB plans render ~75KB with no truncation marker, CreateIssueComment 422s, and plan_comment_retire.go:63 only logs the failure — the PR gets no plan comment while the check still records success and the stale comment is never retired.

Non-blocking

When the primary's group is empty, no group gets open, so every plan renders collapsed. Line 1271's docstring promises "A single plan is never collapsed", but the primary takes the g.Empty() continue before reaching the only place open is set, so the comment's sole DDL sits inside a closed <details> under "including the ones collapsed above". The operator authorizing the apply sees zero SQL without expanding; TestRenderPlanComment_ConvergedPrimaryStillShowsSiblingWork passes from inside the collapsed block.

Sibling groups' DDL is rendered on the consent surface with none of the disclosures that belong beside it. Line 1289 copies a member's own plan verbatim, but unsafe/blocked/lint/execution-mode data is derived from the primary response alone (plan.go:981), and a member's blocked verdict never errors the rollup. A sibling's DROP COLUMN therefore renders with no unsafe warning, no blocked section and no lint violation attached.

The one thing that could have broken, verified

The new && !groupsCarryWork(groups) on the no-changes short-circuit (plan.go:367) sits on the path every single-environment plan comment takes, so a wrong answer there would either drop real DDL or spam empty plan blocks on every PR. It cannot regress the ungrouped path: groups is nil there and slices.ContainsFunc over nil is false, so totalChanges == 0 still short-circuits exactly as before.

Verified correct

• planGroups() gates on len(Plans) < 2, and Plans is only populated for a Clean+Independent rollup, so mirrored/blocked/unverified rollups keep pre-PR rendering.
• planGroupWorkLabel(countChanges(g.Changes)) is a legal multi-value pass-through — (statements, vschemaNamespaces int) match.
• The label's statements == 0 branch cannot print "0 vschema updates": Empty() is statements+vschema == 0 and empty groups continue earlier.
• planGroupHeading's primary-is-first assumption holds: deploymentPlanGroups walks entries in rollout order with Primary at i == 0.
• Member names pass through inlineCodeList before the " (primary)" suffix and <summary>, so hostile target names stay inside a code span.
• writePlanGroupSummary re-renders writeIgnoredNamespaces and writeExemptTables, so those disclosures survive the group-summary substitution.
• The converging preview's collapse=false is right — its only non-empty group is the primary, so no <details> is emitted and the TEMPLATES.md golden matches.

This review was generated by Claude Code (claude-opus-5).

[morgo]

🤖 Reviewed on Morgan's behalf, at f2ca09b1. Approving — the rendering rules are all pinned (seven of seven injections caught) and writeKeyspaceChanges really is reused rather than reimplemented, which is the thing I was watching for after #1424. Two findings, both about shapes the new branch structure opens up rather than about the main path.

1. When the primary's group is converged, nothing is expanded — every plan on the comment is collapsed

collapse is any(!Empty && !Primary) and open is set only on g.Primary. So in the exact shape this PR adds support for — a primary already at the desired schema while its siblings still have work — collapse is true, the primary's group takes the Empty() branch and never reaches the <details> code at all, and every group that does carry work renders with open = "".

Rendered, three targets, primary converged:

✅ **Planned separately for all 3 targets** — 2 distinct plans across the 2 targets that change; 1 is already at this schema.
**`primary/a` (primary)** — already at this schema, nothing to apply.
<details>
<details>
⚠️ Applying runs each target's own plan, including the ones collapsed above.

Two <details>, zero <details open>. The operator opens the PR and sees no DDL anywhere.

TestRenderPlanComment_ConvergedPrimaryStillShowsSiblingWork passes on this because it asserts Contains(out, "ALTER TABLE ..."), which is satisfied by text inside a collapsed block. So the case is tested for presence but not for visibility.

The doc comment states the rule as "the primary's group is open and the rest are collapsed… A single plan is never collapsed, since there is nothing to collapse it against" — which silently assumes the primary has work. Falling back to the first non-empty group when no group is both non-empty and primary is a one-line change and preserves the intent (the reader always has one plan in front of them).

2. The per-comment DDL and vschema budgets are now per-group, and the comment can exceed GitHub's limit

writeKeyspaceChanges constructs both budgets itself — newDDLBlockBudget(...) starts at maxCommentDDLLen (32768) and vschemaDiffBudget divides maxCommentVSchemaDiffLen (16384) — and its own comment says "The VSchema diff budget is per comment, not per keyspace". writePlanGroups now calls it once per group, so both caps are multiplied by the number of distinct plans.

Measured, with maxCommentDDLLen = 32768 and GitHubIssueCommentMaxChars = 65536:

2 groups, one large plan   ->  33,569 chars   (budget working)
4 groups, all large plans  -> 132,313 chars   (over GitHub's cap by 2x)

Nothing on the plan-comment path bounds the whole body today — GitHubIssueCommentMaxChars is consulted only in failure_logs.go and control_rejection_notice.go — so at this commit a four-plan rollout with substantial DDL renders a comment GitHub will reject.

I then read ahead: #1427 fixes this, with a whole-comment planCommentBudget and a withhold-and-re-render loop, so by the top of the stack the comment cannot overflow. Two things still worth saying. First, stacked PRs can land individually, and this one is the commit where the regression exists. Second, #1427's fix is a backstop on total size rather than a fix to the multiplication: each group still renders against a budget sized for a whole comment, so a rollout will withhold more group bodies than it would if the DDL budget were divided across the groups first. Passing a shared *ddlBlockBudget into writeKeyspaceChanges instead of letting it build its own would make the withholding a last resort rather than the normal outcome for a large multi-plan rollout.

Notes

• writePlanGroupSummary has an unreachable-but-ungraceful plans == 0 arm. If every group were Empty() it would print "📋 Plan: 0 distinct plans on 3 targets". It cannot be reached: all-empty groups means groupsCarryWork is false, and the only way past the short-circuit is totalChanges != 0, which needs data.Changes to carry a statement the groups do not — and the paths that could produce that mismatch (an empty-DDL table change) fail canonicalDDLForDrift upstream and clear Clean before Plans is ever populated. Worth a line saying so, since the arm reads like a live default.
• singleKeyspace heading suppression is now decided per group. writeKeyspaceChanges suppresses the #### Schema Name: heading when a plan has exactly one keyspace matching the database name; with scoped changes, one group can suppress it while a neighbour with two keyspaces shows it. Cosmetic, and arguably right per block, but the blocks are read side by side.
• data.UnsafeChanges and data.BlockedChanges remain the primary's, rendered once below the groups. Correct as far as writeKeyspaceChanges goes — I checked, it reads only DatabaseType, IsMySQL, Database and Changes, all of which are rollout-wide except the one that is scoped. But the unsafe/blocked sections now sit under a set of blocks where only one of them is the plan those warnings were computed from. Not something to change in this PR; noting it because #1426 and beyond keep building on this layout.

What I checked rather than took on trust

• Fault injection — seven for seven. Lowering the < 2 group threshold → OnePlanRendersAsTheReviewedPlan; dropping !groupsCarryWork from the short-circuit → ConvergedPrimaryStillShowsSiblingWork; forcing collapse false → DistinctPlansRenderUnderTheirMembers; never opening the primary → same; dropping the (primary) suffix → three tests; rendering the reviewed plan instead of the group's → two tests; disabling the plans == 1 summary arm → three tests.
• scoped := data; scoped.Changes = g.Changes is safe. Read writeKeyspaceChanges end to end: the only per-plan field it touches is Changes. So the "same code that renders the reviewed plan" claim holds here, unlike the parallel builder in #1424.
• planGroupHeading does not mutate the group. inlineCodeList allocates a fresh slice (make([]string, len(values))), so names[0] += " (primary)" cannot write back into g.Members.
• Only one group can be open. Primary is set at group creation from i == 0 in #1424, so at most one group carries it, and writePlanGroups reads it directly.
• planGroupWorkLabel(0, 0) would say "0 vschema updates", and is unreachable — the Empty() branch returns before the label is computed, and Empty() is exactly statements+vschema == 0.
• The short-circuit change cannot regress the single-plan case. planGroups() returns nil below two groups, so groupsCarryWork(nil) is false and totalChanges == 0 short-circuits exactly as before for every non-multi-target comment.
• Merge-base against pr1424 is an ordinary commit; +457/−4, the four deletions being the two call sites this branches. No test deletions. CI: 41 checks, no genuine failures.