feat: remove the volume control operation end to end in favor of autoscaling

[aparajon]

Why this matters

Volume was a 1–11 dial for "how hard should this schema change push." It predates autoscaling. Spirit now sets its own write-thread count from live throttler feedback, and that autoscaling is on by default — so on the copy path there is nothing left for a fixed operator level to tune. The dial still moved a number; it had stopped moving the behavior it claimed to control. A control an operator reaches for mid-incident and gets nothing from is worse than no control at all.

Autoscaling is the direction. A schema change should find its own safe throughput from the signal it already has — replication lag, throttler feedback, target load — not from a number someone guessed before the copy started. A fixed thread count is the classic failure mode on large targets: throughput that made sense on one instance class silently starves or overloads another. Removing the knob is the other half of committing to that; keeping it around as a no-op is how you get an operator tuning a dial instead of reading the throttle signal.

What it does

Removes the volume operation end to end, one surface per line:

• the Volume RPC and its HTTP routes
• the schemabot volume CLI command, and the TUI's volume mode (v and the volume bar)
• the schemabot volume PR comment command and its help-table row
• engine.Engine.Volume and the Spirit and PlanetScale/Vitess implementations, including the server-side throttle-ratio endpoint that volume was the only caller of
• the durable control-request path, ApplyOptions.Volume, and ProgressResponse.volume
• the progress-comment rotation that fired when volume changed

Spirit's throttler is untouched: the throttled progress signal, the write-throttle tooltip, and the autoscaling kill switch (enable_experimental_autoscaling: false, for when autoscaling misbehaves on a target fleet) all stay.

Two things worth a reviewer's attention

On Vitess, throttle the deploy request in the PlanetScale console. Volume never had thread counts to set there — it mapped an 11-point scale onto a server-side throttle ratio, an imperfect translation of a control the platform already owns and states plainly. Operators who want a Vitess schema change to back off should set the deploy request's throttle in the PlanetScale console, where the value they choose is the value that gets applied. That is the trade: each engine's real control, over one SchemaBot control whose name meant something different on each of them.

What a caller or a stored row from the previous release sees. A client pinned to the removed API gets an explicit error, not silence: the gRPC method is unknown and the HTTP routes are gone. An apply options map that still carries a volume key is ignored — options are read by name. Durable volume control request rows are handled through an explicit retired-operations concept (ControlOperation.Retired()) rather than left to fall wherever the generic paths drop them: the remote-progress mirror skips their settled reports at debug level instead of warning on every poll, the PR rejection notice stops rendering a pre-upgrade volume rejection whose "re-issue the command" remedy no longer exists, and the terminal-apply sweep settles a leftover pending row no driver services anymore. A progress comment a pre-removal binary froze with the volume headline stays recognizable to the freeze retry, so it is never folded twice. The removed ProgressResponse.volume field number and name are reserved, so a future field cannot misparse what an embedded pre-removal data plane still emits. So a stale row can neither wedge a drive nor hold a database, and an in-flight apply keeps its progress; the volume field simply stops being reported.

The apply_comments.posted_volume column stays this release. EnsureSchema applies whatever DDL its diff against the embedded schema files produces, so dropping the column in the same release that removes its writers would strip it out from under a pod still running the previous binary. The column is nullable and now unread; the DROP COLUMN lands in the next release.

Comment rotation now keys on posted_phase alone. The generic "progress comment superseded" fold became the zero-value reason, so a freeze retry that no longer knows which rotation superseded a comment renders that generic fold instead of an unearned headline.

Rendered result

Progress comment — no volume line

Schema Change Status — Staging

Database: testapp | Apply ID: apply-a1b2c3d4e5f6

Applied by @jackjackbits at 2026-01-01 00:00:00 UTC

Status: In Progress

users: 🟦🟦🟦🟦🟦🟦🟦🟦🟦⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜ 48%

ALTER TABLE `users` ADD INDEX `idx_email_created`(`email`, `created_at`);

• Rows: 3,500,000 / 7,200,000 · ETA: 5m 30s

---

To stop this schema change:

schemabot stop apply-a1b2c3d4e5f6 -e staging

Last updated: 2026-01-01 00:00:00 UTC

PR comment help — no volume row

📚 SchemaBot Help

Command	Description
schemabot plan [-e <env>]	Preview schema changes
schemabot apply -e <env>	Plan, lock, and apply after safety rechecks
schemabot apply-confirm -e <env>	Confirm a downgraded locked plan
schemabot unlock	Release lock and discard plan
schemabot stop <apply-id> -e <env>	Stop an in-progress deployment
schemabot cancel <apply-id> -e <env>	Permanently cancel an in-progress deployment
schemabot start <apply-id> -e <env>	Resume a stopped deployment
schemabot release <apply-id> -e <env>	Release a paused rollout to proceed
schemabot cutover <apply-id> -e <env>	Complete a deferred cutover
schemabot rollback <apply-id> -e <env> [-t <tenant>]	Generate a rollback plan
schemabot rollback-confirm -e <env> [-t <tenant>]	Execute a rollback

Options: -e <env> environment, -d <db> database, -t, --tenant <name> deployment routing, --defer-cutover, --allow-unsafe, --skip-revert (Vitess)

Quick start: plan → apply

CLI watch mode — footer and status block

     ~ orders: 🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩 ✓ Complete
       ALTER TABLE `orders` ADD INDEX `idx_user_id`(`user_id`);

ESC detach • s stop

┌──────────────────────────────────┐
│  Apply ID:  apply-a1b2c3d4e5f6   │
│  State:     Running              │
│  Started:   Jan 15 14:18:00 UTC  │
│  Duration:  12m                  │
└──────────────────────────────────┘

Opened by Claude (Opus 5).

[Copilot]

Pull request overview

This PR removes the “volume” control operation end-to-end across SchemaBot’s operator surfaces (PR comments, CLI/TUI, HTTP + gRPC APIs, Tern orchestration, engines, and storage metadata), aligning the system on autoscaling (Spirit) and removing the PlanetScale/Vitess throttle-ratio lever.

Changes:

• Removed the volume command/RPC/routes/types, including durable control-request plumbing and progress rendering/rotation that depended on volume.
• Simplified progress/comment rendering and stored comment tracking to key rotation on posted_phase only.
• Updated docs, previews, metrics enums, and tests to reflect the removal.

Reviewed changes

Copilot reviewed 127 out of 127 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
README.md	Removes volume from the high-level control list.
pkg/webhook/templates/preview.go	Removes volume-related preview helpers.
pkg/webhook/templates/issue_comment.go	Removes volume command usage/errors/ack templates and volume-specific superseded rendering.
pkg/webhook/templates/issue_comment_test.go	Removes tests for volume comment templates and volume supersede detection.
pkg/webhook/templates/help.go	Removes volume row from PR help table.
pkg/webhook/templates/help_test.go	Updates help-table assertions after removing volume.
pkg/webhook/templates/control_rejection.go	Updates control rejection docs to remove volume-specific wording.
pkg/webhook/templates/control_rejection_test.go	Adjusts rejection rendering tests to use a non-volume example.
pkg/webhook/templates/apply.go	Removes volume from apply status comment rendering and progress→comment mapping.
pkg/webhook/templates/apply_test.go	Removes tests asserting volume appears on status line.
pkg/webhook/issue_comment.go	Removes volume PR command handling and fan-out membership.
pkg/webhook/handler_test.go	Removes webhook tests covering volume parsing/usage errors.
pkg/webhook/control.go	Removes volume dispatcher + validation + durable request queueing path.
pkg/webhook/control_rejection_notice_test.go	Updates rejection-notice tests to non-volume operations.
pkg/webhook/control_integration_test.go	Removes end-to-end integration tests for volume PR command.
pkg/webhook/commands.go	Removes parsing support for -v/--volume and the volume command spec.
pkg/webhook/commands_test.go	Removes command-spec and parsing tests for volume.
pkg/webhook/apply.go	Stops passing apply option volume into webhook comment data.
pkg/webhook/aggregate_participant_fanout_test.go	Removes volume from unscoped fan-out coverage.
pkg/webhook/aggregate_fanout_silent_skip_test.go	Removes invalid-volume-level silent-skip behavior tests.
pkg/webhook/action/action.go	Removes Volume action constant.
pkg/tern/volume_convergence_test.go (deleted)	Removes volume convergence behavior tests.
pkg/tern/target_router.go	Removes Volume routing RPC.
pkg/tern/target_router_test.go	Removes Volume from router client recording.
pkg/tern/state_converters_test.go	Updates a test to avoid using the removed volume display key.
pkg/tern/server.go	Removes gRPC server method Volume.
pkg/tern/server_test.go	Removes apply-id requirement test case for Volume.
pkg/tern/routing_client.go	Removes routing client Volume.
pkg/tern/README.md	Removes Volume from the documented client interface and file map.
pkg/tern/local_control_test.go	Removes Postgres “pending volume” decline-resolution test.
pkg/tern/local_control_caller_integration_test.go	Updates caller-recording tests to use remaining controls instead of volume.
pkg/tern/local_client.go	Removes mention/round-trip of volume in apply options and progress response.
pkg/tern/local_client_test.go	Removes fake engine Volume implementation hooks.
pkg/tern/local_client_integration_test.go	Removes integration test for local Volume RPC.
pkg/tern/local_client_existing_copies_test.go	Removes assertion that plan path “must not set volume”.
pkg/tern/local_apply_sequential.go	Removes per-task volume convergence and pending-volume processing in poll loop.
pkg/tern/local_apply_grouped.go	Removes pending-volume processing in grouped/atomic tick loop.
pkg/tern/grpc_drive_logger_test.go	Removes remote-volume mirror logging test.
pkg/tern/grpc_client.go	Removes remote volume mirroring logic and Volume RPC; updates proxy/mirror docs.
pkg/tern/grpc_client_test.go	Removes server/client volume RPC tests and remote-volume mirroring tests.
pkg/tern/control_requests.go	Removes terminal-apply sweeping of pending volume control requests.
pkg/tern/client.go	Removes Volume from the client interface.
pkg/storage/types.go	Removes ApplyOptions.Volume, volume control operation constant/metadata helpers, posted volume tracking, and related log event.
pkg/storage/types_test.go	Removes apply-option roundtrip coverage for volume and volume-metadata encode/decode tests.
pkg/storage/storagetest/control_requests.go	Removes volume metadata lifecycle tests; updates remote-failure mirroring tests to use another operation.
pkg/storage/storagetest/apply_comments.go	Removes posted-volume persistence expectations; keeps posted-phase behavior.
pkg/storage/storage.go	Updates comment/docs about “pure proxy operations” after removing volume.
pkg/storage/README.md	Removes volume from storage apply options description.
pkg/storage/operator_facing_test.go	Updates operator-facing message rewrite test to a non-volume example.
pkg/storage/internal/sqlstore/dialect_test.go	Updates apply_comments upsert assignment expectations after removing posted_volume writes.
pkg/storage/internal/sqlstore/apply_comments.go	Stops selecting/inserting/upserting posted_volume.
pkg/storage/internal/sqlstore/applies_test.go	Removes volume from persisted apply options test coverage.
pkg/state/apply.go	Removes volume from docs about running-state control eligibility.
pkg/psclient/client.go	Removes PlanetScale throttle endpoint support used only by volume.
pkg/psclient/client_test.go	Updates raw-request failure test to cover a remaining baseURL endpoint.
pkg/proto/ternv1/tern.pb.gw.go	Removes volume HTTP gateway bindings.
pkg/proto/ternv1/tern_grpc.pb.go	Removes volume service method and related generated stubs/docs.
pkg/proto/tern.proto	Removes Volume RPC and associated request/response + progress field.
pkg/metrics/README.md	Removes volume from documented label value sets.
pkg/metrics/metrics.go	Removes volume from known command/operation enums and docs.
pkg/localscale/server.go	Removes throttle route registration (previously used by volume).
pkg/localscale/server_deploy_integration_test.go	Removes throttle boundary test that depended on volume-era throttle endpoint.
pkg/localscale/handlers_actions.go	Removes handler for throttle route (previously used by volume).
pkg/engine/spirit/spirit.go	Removes volume-related tuning/restart plumbing; clarifies threads as a starting point under autoscaling.
pkg/engine/spirit/spirit_test.go	Removes progress-state behavior around “volume restart”.
pkg/engine/spirit/README.md	Removes volume section and references.
pkg/engine/spirit/execution.go	Uses engine defaults directly (no per-apply copySettings).
pkg/engine/README.md	Removes Volume from engine interface docs and control op list.
pkg/engine/postgres/postgres.go	Removes unsupported Volume method.
pkg/engine/postgres/postgres_test.go	Removes unsupported-controls test case for volume.
pkg/engine/planetscale/volume.go (deleted)	Deletes PlanetScale/Vitess throttle-ratio volume implementation.
pkg/engine/planetscale/volume_test.go (deleted)	Deletes volume→throttle mapping unit test.
pkg/engine/planetscale/planetscale.go	Removes volume references from PlanetScale engine docs.
pkg/engine/planetscale/database_resolution_test.go	Removes volume from “must address resolved DB” parity test.
pkg/engine/engine.go	Removes Volume control operation and request/result types.
pkg/engine/engine_test.go	Updates unsupported-operation error test string away from “volume”.
pkg/cmd/main.go	Removes schemabot volume CLI command registration.
pkg/cmd/internal/templates/progress.go	Removes volume from CLI progress/detail box rendering + data models.
pkg/cmd/internal/templates/progress_render.go	Removes volume hint from watch footer output.
pkg/cmd/internal/templates/progress_parse.go	Stops parsing volume from progress response.
pkg/cmd/internal/templates/progress_parse_test.go	Removes volume parsing and volume box-row tests.
pkg/cmd/internal/templates/progress_multi.go	Removes volume row from multi-deployment header rendering.
pkg/cmd/internal/templates/preview.go	Removes volume-related preview types.
pkg/cmd/internal/templates/preview_status.go	Removes volume from preview status-list sample data.
pkg/cmd/internal/templates/preview_sequential.go	Removes volume from sequential preview sample data.
pkg/cmd/internal/templates/preview_progress.go	Removes volume bar/mode preview outputs.
pkg/cmd/internal/templates/preview_dispatch.go	Removes volume preview dispatch cases and comment previews.
pkg/cmd/internal/templates/preview_comment.go	Removes volume scenarios from comment preview sets.
pkg/cmd/commands/watch_tui.go	Removes volume mode/state from TUI watch model and progress parsing.
pkg/cmd/commands/watch_tui_view.go	Removes volume UI/footers and volume-changing status line.
pkg/cmd/commands/watch_tui_commands.go	Removes volume-mode key handling and API calls.
pkg/cmd/commands/volume.go (deleted)	Deletes the schemabot volume command implementation.
pkg/cmd/commands/status.go	Removes volume from status list data mapping.
pkg/cmd/commands/preview.go	Removes volume scenarios from preview command routing and help text.
pkg/cmd/commands/preview_tui.go	Removes volume field wiring in static TUI preview.
pkg/cmd/commands/common.go	Removes volume response wrapper for accepted-response handling.
pkg/cmd/commands/apply.go	Updates apply command docs to remove volume from interactive controls list.
pkg/cmd/client/client.go	Removes client helper for /api/volume.
pkg/apitypes/apitypes.go	Removes volume request/response types and progress/status volume fields.
pkg/api/service.go	Removes /api/volume route and handler wiring.
pkg/api/route_authorization_sweep_test.go	Removes /api/volume from write-route authorization sweeps.
pkg/api/README.md	Removes /api/volume endpoint docs and handler listing.
pkg/api/progress_handlers.go	Stops mapping/storing volume on progress and active-apply responses.
pkg/api/handlers_test.go	Removes volume handler tests and mock tern volume plumbing.
pkg/api/control_handlers.go	Removes volume request decoding, validation, and execution paths.
integration/locks_test.go	Removes CLI lock interaction test coverage for volume.
integration/cli_test.go	Removes CLI tests for volume command validation.
e2e/local/local_test.go	Removes local E2E scenarios for volume.
e2e/grpc/helpers_test.go	Removes helper response struct for volume.
e2e/grpc/grpc_test.go	Removes gRPC E2E volume scenario.
e2e/grpc/cli_test.go	Removes gRPC CLI E2E volume scenario.
docs/strata-engine.md	Updates Strata engine docs to remove volume from supported/roadmap controls.
docs/spirit_progress.md	Removes volume references in control gating + footer docs (but needs a footer fix; see comment).
docs/configuration.md	Removes volume as an operator control, leaning on autoscaling.
docs/architecture.md	Removes schemabot volume and proto Volume references from architecture docs.
docs/apply-lifecycle.md	Updates data model diagram to remove volume mention (but needs broader control list; see comment).
AGENTS.md	Removes volume from the list of required control-operation properties and rotation signals.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[morgo]

🤖 Reviewed on Morgan's behalf by his AI agent. Not approving — at +390/-5985 across 127 files, removing an RPC, HTTP routes, a CLI command and an engine.Engine method, this is squarely a human-review change under Morgan's standing bar. Flagging one concrete collision I found rather than attempting a full review.

This will break #1211 on merge, in whichever order they land.

#1211 (already approved, green, targeting main) adds a reflection ratchet over the engine contract suite. Its completeness test asserts that every documented exclusion still corresponds to a real method:

for method := range engineMethodExclusions {
    if !engineMethods[method] {
        staleExclusions = append(staleExclusions, method)
    }
}
assert.Empty(t, staleExclusions, "excluded methods missing from engine.Engine")

and engineMethodExclusions contains:

"Volume": "throttle semantics are engine-specific; each engine's own tests pin them",

This PR removes Volume from engine.Engine (pkg/engine/engine.go, -35). So once both are on main, Volume is an exclusion for a method that no longer exists and that assertion fails. Both PRs are green right now only because each is tested against a main that doesn't contain the other.

The fix is one deleted line in engineMethodExclusions — the ratchet is doing exactly what it was built to do, which is arguably a small vote of confidence in #1211. But someone has to actually do it:

• If #1211 merges first, this PR needs to drop the "Volume" exclusion in the same change.
• If this merges first, #1211 needs a rebase dropping it before it lands.

Worth deciding the order deliberately rather than discovering it in CI on main.

For what it's worth, the motivation reads well — a control that still moves a number after it stopped moving the behavior is worse than no control, and that's a fair reason to delete rather than deprecate-in-place. The thing I'd want a human to weigh is the compatibility surface: removing an RPC and its HTTP routes breaks any caller pinned to the old API, and the PR description would benefit from stating explicitly what happens to an in-flight or stored apply that still carries a volume value, and whether any client outside this repo calls the Volume RPC today.

[Kiran01bm]

🤖 Review findings - created by Kiran's code review agent - for schemabot/pull/1225, 7a4440a.

Verdict: 6 findings — 0 blocking, 5 non-blocking (all durable pre-upgrade "volume" state during the rollout window), 1 suggestion.

CI is fully green on this head (lint — golangci-lint across all four matrix variants plus golangci-lint config verify and make check-webhookheaders; test — unit, storage conformance against real MySQL and Postgres, LocalScale/Vitess and E2E suites; docker; release). For a feature-removal PR that establishes exactly one thing: nothing remaining in the tree references the removed volume machinery, and the surviving code compiles and behaves as before without it. It cannot speak to (a) durable state written by the previous binary — apply_control_requests rows, apply_comments.pending_freeze_github_comment_id, frozen GitHub comment bodies; (b) wire/version skew between a new control plane and an old data plane, since no job runs a skewed pair; or (c) behavior whose only coverage this PR deleted. Every finding below lives in one of those three blind spots, which is why they are worth stating even though the build is clean. Separately, buf breaking is never invoked in CI (see the suggestion), so green CI is not evidence of proto compatibility either.

The removal itself is unusually thorough. The residual-reference sweep across pkg/, docs/, charts/, deploy/, e2e/, TEMPLATES.md and the generated proto stubs is clean — every remaining case-insensitive volume hit is a Kubernetes/Helm/EBS/"webhook delivery volume" false positive. TEMPLATES.md was verified to be a genuine regeneration (rebuilding the head binary and re-running scripts/update-templates.sh into a scratch tree reproduces the committed file byte-for-byte), and the committed pkg/proto/ternv1 descriptor matches the .proto exactly.

Non-blocking

1. Removing volume from ControlOperation.Valid() turns every durable settled volume row into a WARN on each 500 ms remote progress poll.
pkg/tern/grpc_client.go:1933:

operation := storage.ControlOperation(entry.Operation)
if !operation.Valid() {
	logger.Warn("data plane reported a settled control request for an unrecognized operation; it will not reach the operator", ...)
	continue
}

pkg/storage/types.go:1043 no longer lists ControlOperationVolume, so ControlOperation("volume").Valid() is now false. The data plane still ships the row: ListSettled filters on WHERE apply_id = ? AND status IN (?, ?) only, and pkg/tern/local_client.go:3238 copies Operation: string(req.Operation) verbatim — from an old or a new data plane, since it is a plain storage read.

Scenario: an operator ran schemabot volume <apply> -v 9 on a long-running ALTER before the rollout; the driver applied it and wrote apply_control_requests(operation='volume', status='completed'). That apply is still in flight when the new control plane deploys. pollForCompletion calls mirrorRemoteControlRejections unconditionally on every successful poll of a time.NewTicker(grpcProgressPollInterval) loop with grpcProgressPollInterval = 500 * time.Millisecond (grpc_client.go:127, :4235, :4405), with no dedup or rate limit on applyLogger. That is ~2 warnings/sec for the life of the drive — ~28,800 for a 4-hour ALTER, doubled if the apply has both a completed and a failed volume row — and the message misattributes the cause ("the data plane is newer/broken") when the truth is that this control plane retired the operation. Confirmed with an overlay probe driving the real mirrorRemoteControlRejections; a recognized-operation control mutant takes the status branch instead, proving the branch attribution.

Nothing else breaks — the continue skips one entry, and state reconciliation, task sync and terminal handling proceed in the same tick. New volume rows can no longer be created, so the noise drains as pre-upgrade applies finish. Cheapest fix: treat a retired operation as a distinct, once-per-apply log line, or accept "volume" in a retired set for one release.

2. A residual failed volume row renders a permanent "re-issue the command" PR notice for a command that no longer exists.
pkg/webhook/control_rejection_notice.go:48 filters on status only:

for _, req := range settled {
	if req.Status != storage.ControlRequestFailed { continue }
	rejections = append(rejections, templates.ControlRejectionData{ Operation: string(req.Operation), ... })
}

There is no Operation.Valid() gate here — ControlOperation.Valid() has exactly one non-test caller in the tree, the wire boundary at grpc_client.go:1933. The renderer emits the operation verbatim at pkg/webhook/templates/control_rejection.go:39. So an apply whose volume request was rejected by the engine pre-upgrade (base failPendingControlRequests(..., ControlOperationVolume, ...), four call sites in pkg/tern/local_control.go at the base commit — file rewritten by this PR) keeps re-rendering:

> [!WARNING]
> **Command not applied**
>
> - `volume` was accepted but did not take effect: <engine error>
>
> The schema change is otherwise unaffected. Re-issue the command or reconcile the target environment.

on every progress/terminal comment update. It can never clear. The clearing path is RequestPending's failed→pending reset (SET status = ?, ..., error_message = NULL, pkg/storage/internal/sqlstore/control_requests.go:76), which requires re-issuing a command that is gone; the mirrored-remote path via ClearRemoteFailure is dead too, since its only caller retireMirroredControlRejection sits below the Valid() continue in finding 1. Note this is broader than mirrored remote rejections — plain local volume rejections are equally permanent. Impact is cosmetic and self-draining; a three-line if !req.Operation.Valid() { continue } here, or a one-off DELETE FROM apply_control_requests WHERE operation='volume', closes it.

3. Dropping volumeSupersededPrefix makes a freeze retry re-fold a comment the previous binary already froze.
pkg/webhook/templates/issue_comment.go:374 — the recognition list lost its first entry:

for _, prefix := range []string{
	resumeSupersededPrefix,
	revertSupersededPrefix,
	skipRevertSupersededPrefix,
	cutoverSupersededPrefix,
	genericSupersededPrefix,
} {

The predicate exists precisely to stop double-folding — pkg/webhook/comment_observer.go:1521: // A retry after a failed marker clear finds the frozen body already on GitHub; re-rendering it would fold the frozen body inside another fold, so only a still-live body is edited.

Reachable from durable state: a pre-#1225 pod rotated a progress comment for a volume change, wrote pending_freeze_github_comment_id, edited the old comment to a body headlined ⏩ Volume changed to **8/11** — progress continues in [a new progress comment](...), and then ClearPendingFreeze failed or the pod was terminated in that window. The new binary retries the freeze (four unconditional call sites: comment_observer.go 1042, 1233, 1349, 1361), the predicate returns false, and the frozen <details> gets wrapped in a second generic <details>. Verified with an overlay probe that reconstructs the exact base-written body: volume-frozen recognized = false, control resume-frozen body = true.

Bounded and self-healing — the second edit produces a generic headline that is recognized, so no loop and no lost text. Fix is one line: keep volumeSupersededPrefix in the recognition list only (no renderer), commented as a compatibility marker for bodies written by pre-removal binaries, and delete it after the removal converges.

4. Pending volume rows written during the rollout window are stranded pending forever.
pkg/tern/control_requests.go:61 — the terminal sweep, whose own comment says it exists to keep "a request issued moments before the apply settled … from lingering pending forever", dropped storage.ControlOperationVolume from its op list. If a pre-#1225 pod inserted apply_control_requests(operation='volume', status='pending') and was drained before its next progress tick serviced it, the row is now unreachable: processPendingVolumeControlRequest and its call sites are deleted, every remaining SQL predicate over that table binds a literal known operation or status IN (completed, failed), and ListSettled excludes pending — so it is never read, settled, or surfaced. The operator's "Volume Request Accepted … SchemaBot will adjust the speed of this schema change shortly" acknowledgement gets no follow-up.

The row is provably inert (see the verification section) — this is audit hygiene, not a correctness bug. Note the sweep helper's SQL is operation-agnostic, so retaining a literal "volume" entry for one release does not require resurrecting the deleted constant; alternatively UPDATE apply_control_requests SET status='failed' WHERE operation='volume' AND status='pending' would surface them through the existing rejection notice.

5. ProgressResponse.volume = 9 is removed with no reserved 9;.
pkg/proto/tern.proto — field 9 vanished from ProgressResponse (int32 volume = 9; between completed_at = 8 and metadata = 10) with no reservation, and the Volume RPC plus VolumeRequest/VolumeResponse went the same way.

The basis is the repo's own written policy, not the ShardPlan precedent: docs/release.md:205 — "The gRPC contract is additive only (new fields, never removed or renumbered)" — and docs/release.md:195, which explicitly contemplates consumers this repo does not control: "A service can embed SchemaBot as a Go module and provide its own data plane instead … The skew window can be much longer, and you should assume it will be." Two of the three prior removals in this file did reserve (tern.proto:416 reserved 7;, :677 reserved 8; reserved "ready_to_complete";).

Reachability today is nil — nothing decodes field 9, and a new control plane drops it into unknown fields. The hazard is future field-number reuse: a later PR picks int32 some_count = 9;, an embedded data plane still on a pre-#1225 module emits field 9 as an int32 varint carrying the volume level, wire types match, and the value is silently misparsed. Worth one line plus a comment. Two caveats, both verified: the ShardPlan "removed pre-release … no released consumers" comment at tern.proto:543 is factually stale (needs_change shipped in tags v0.1.16–v0.1.33) and should not be cited as the governing precedent either way; and adding reserved 9; does not satisfy buf's FILE breaking category, whose FIELD_NO_DELETE fires on deletion regardless of reservation — buf breaking reports the same four violations with and without it. The reservation is a documentation-and-collision guard, nothing more.

General suggestions

6. Consider a buf breaking CI step — green CI is currently silent on proto compatibility.
The only buf invocation anywhere in the repo is Makefile:174 (buf generate); .github/workflows/ contains no proto step at all. Running the gate manually against the merge base shows it would not have been vacuous:

tern.proto:1:1:Previously present message "VolumeRequest" was deleted from file.
tern.proto:1:1:Previously present message "VolumeResponse" was deleted from file.
tern.proto:28:1:Previously present RPC "Volume" on service "Tern" was deleted.
tern.proto:732:1:Previously present field "9" with name "volume" on message "ProgressResponse" was deleted.

A buf breaking --against '.git#branch=main' step would surface wire-level removals at PR time rather than at docs/release.md:318's manual pre-tag §5 check, days later, by a different person, with the commit already batched into a release range.

Two corrections to how this should be framed, since both matter for whether it is worth doing: buf.yaml:9's breaking: use: [FILE] block is not a repo declaration — it is the verbatim buf config init template, and git log -- buf.yaml shows a single commit (the initial port). The real declared policy is release.md. And buf lint cannot be adopted as written: it fails identically on base and head with the same 9 pre-existing violations (package-directory rule, Tern service suffix, enum zero-value names), which is itself evidence the config is unused scaffolding. There is no live drift — regenerating from the head .proto reproduces the committed pkg/proto/ternv1 byte-for-byte — so this is a forward-looking guardrail on infrastructure this PR does not touch, not a defect it introduces.

The one thing that could have broken, verified

An unsettleable durable "volume" control request wedging the apply drive loop. This is the mechanism the evidence actually implicates: the PR removes ControlOperationVolume from ControlOperation.Valid() and from the terminal sweep in pkg/tern/control_requests.go:61 while leaving the durable rows in place, and apply_control_requests is joined by the claim and gating paths. If any of those paths enumerated operations negatively — NOT IN, "any pending request", or a completeness assertion — a stranded volume row would block claiming or the terminal transition, which is a wedged-apply outcome, not a log-spam one.

It is safe, proved three ways:

1. Static exhaustion. Every SQL predicate in the tree that touches apply_control_requests binds the operation by equality to a hardcoded literal, or filters on status alone: applies.go:1581/1589/1597/1606/1705/2040 (cr.operation = ? AND cr.status = ?), apply_operations.go 683/1104/1119/1147/1158/1175/1238/1511/1552, and control_requests.go's GetPending/GetByOperation/CompletePending/FailPending/ListSettled. The only two Go-level loops (local_apply.go over {start, cancel}; control_handlers.go over {stop, cancel}) iterate literal slices. There is no operation-agnostic "any pending request" scan anywhere, and the unique key idx_apply_control_request_apply_operation (apply_id, operation) means the volume slot can never collide with a surviving operation.
2. End-to-end probe against real MySQL. A testcontainer probe inserted operation='volume', status='pending' through the real store (accepted — the column is varchar(50) NOT NULL in both dialects, pkg/schema/mysql/apply_control_requests.sql:3, no enum or CHECK), then claimed the apply via claimApplyForDrive and drove it to completed with the orphan row sitting alongside a pending stop. The claim succeeded, the terminal sweep settled the stop, and ListSettled returned only op="stop" status="completed" — the orphan never leaked onto an operator surface as a phantom in-flight command.
3. Release-latch and gating check. ReleasesPausedRollout() requires Operation == ControlOperationRelease, so the orphan cannot affect the release latch; no cutover or safety gate reads the table generically.

So the residual is exactly finding 4 — a row stranded pending in a table that already retains settled rows forever — with no functional consequence. The two adjacent candidates were also cleared: gRPC skew degrades loudly rather than silently (finding 1's WARN is noisy but non-corrupting, and an old plane calling the deleted Volume RPC fails clean at the transport boundary before any durable row is written), and comment_observer.go was diffed line-by-line against base and is a pure volume excision plus the deliberate supersededProgressReason enum reorder.

Verified correct

• No residual references. processPendingVolumeControlRequest, convergeTaskVolumeToStoredLevel, ControlOperationVolume, mirrorRemoteVolume, CallVolumeAPI — zero Go hits at head. Docs, Helm, Terraform, e2e YAML and AGENTS.md hits are all genuine false positives ("webhook delivery volume", "storage query volume", extraVolumeMounts, Kubernetes volumes:, volume_type = "gp3").
• TEMPLATES.md is a genuine regeneration, not a hand-edit: rebuilding the head binary and re-running scripts/update-templates.sh into a scratch tree produced a byte-identical file.
• Generated proto matches the source. The embedded descriptor shows service Tern with 13 RPCs and no Volume, no VolumeRequest/VolumeResponse, ProgressResponse fields 1–8/10/11, and the existing ShardProgress/PlanRequest reserved ranges intact.
• apply_comments rolling-deploy safety is sound in the forward direction. Retaining the now-dead posted_volume column is correct and is not under-documented: it is int DEFAULT NULL / integer DEFAULT NULL, the shortened INSERT and ON CONFLICT list simply omit it, pre-feat: remove the volume control operation end to end in favor of autoscaling #1225 pods' 12-column SELECT still resolves, and a future PR that deletes the column from the .sql files cannot destroy it — EnsureSchema refuses destructive DDL by default (pkg/api/ensure_schema.go:46), logs and meters the refusal, and the opt-in flag's own doc comment forbids flipping it before every pod is on the new binary.
• No spurious rotation during skew. A new pod's options write drops the volume key, so an old pod reads Volume == 0 and the base rotateProgressCommentForVolumeChange early-returns; the stale posted_volume value is never compared.
• No durable resend storm. grpc_control_resend.go's gate is driven only by operations with explicit pendingControlRequest(...) lookups (cutover, stop, cancel, start, revert, skip_revert). Volume never had a control-plane request row — its request lived entirely in the data plane.
• schemabot volume … degrades gracefully on both surfaces. On a PR comment, ParseCommand misses specByName and returns Found=false, IsMention=true, so issueCommentGateCommandNotFound posts RenderInvalidCommand() with the current command reference (volume row correctly absent) — not a silent drop. On the CLI, schemabot --help lists 13 commands and schemabot volume falls through to top-level usage.
• JSON and gRPC client skew are safe. DisallowUnknownFields appears only on request decoders, never on client-side response decodes, so an old server's "volume": 3 key is ignored. pkg/tern/server.go still asserts var _ ternv1.TernServer = (*Server)(nil) without embedding UnimplementedTernServer, so the proto header's "adding an RPC causes a compile error" invariant is intact — verified with a rename mutant that fails the build.
• The supersededProgressReason enum reorder is a deliberate improvement, not a silent behavior change. The zero value moved to supersededByPriorRotation so an unset reason renders the generic fold rather than an unearned headline, and the enum is never persisted — it exists only on the in-memory pendingProgressRotation.reason field. Cross-version and restart safe.
• The per-tick pending-freeze retry is not lost. The resume, control-phase and post-cutover rotations each still retry a stuck fold before their own early returns; the volume path was never a general-purpose retry (it only armed when volume was non-zero and early-returned once trackedPostedVolumeKnown was primed).
• comment_observer.go carries no riding behavior change. All 456 changed lines diffed against base: postAndTrackComment, adoptPendingRotationComment, editTrackedComment, OnProgress's adopt → cutover-prompt → resume → control-phase → rate-limited-edit ordering, and freezeSupersededProgressComment's lease checks are byte-for-byte preserved. The one apparent control-flow change (watch_tui.go losing its if !m.volumeChanging || len(m.tables) == 0 guard) is provably equivalent — volumeChanging has no remaining setter. The adopted computation at comment_observer.go:295 remains load-bearing and covered: a "never adopt" mutant is killed by two surviving E2E tests.
• Spirit/engine removals are pure excisions. newSpiritMigration's e.copySettings() → e.threads, e.lockWaitTimeout is equivalent (rm.threads was always initialized from e.threads and only mutated by the deleted setSchemaChangeVolume), and the fields are now write-once at New() with no remaining mutator — so the data race the deleted concurrency test guarded is gone with the feature. progressState losing volumeRestartInProgress is dead-branch removal.
• Throughput control removal is coherent and correctly documented. applyThrottle/defaultThrottleRatio survive in the LocalScale test fake and its ALTER VITESS_MIGRATION UNTHROTTLE ALL reset path is still reachable via POST /admin/reset-state and automatic container reuse — no keyspace can be left permanently throttled. The real PlanetScale path removes both the set and the clear halves symmetrically, and docs/architecture.md gains the replacement guidance (Spirit autoscaling; PlanetScale console for Vitess). The rewritten Spirit-defaults paragraph is also operationally accurate: the autoscaling kill switch does not strand an operator, because the Aurora threads and commit-latency throttlers are armed unconditionally and turning autoscaling off reduces write threads (auto-sized 14 on a 16-vCPU instance → Spirit's default 4).
• Deleted tests were correct to delete, per AGENTS.md — coverage of removed behavior, not a gap. TestGRPCClient_Volume, TestMirrorRemoteVolume, TestVolumeHandler, TestVolumeConcurrentWithSettingsReads, TestApplyOptionsFromMapIgnoresInvalidVolume, TestVolumeInvalidLevelDefersToLeader, TestRenderVolumeSupersededProgressComment, TestE2EPendingRotationAdoptedBeforeCutoverPrompt, and the two volume storage-conformance subtests all covered machinery this PR deletes. Each invariant worth keeping was verified still covered elsewhere: control-request metadata immutability and reset-on-re-request survive on the start cases (storagetest/control_requests.go:59, :154); OperatorFacingMessage keeps eight production call sites and its own unit suite; the Postgres unsupported-decline path is covered by the sibling cancel test; the mirrored-rejection lifecycle tests were re-homed onto cutover/revert with every assertion preserved; and the post-cutover rotation invariant survives in three E2E tests.
• Durable apply_logs rows carrying the deleted LogEventVolumeRequested are harmless — no read path switches on log event types; rows render as free-form messages.
• ApplyOptions round-trips tolerate legacy stored options — a pre-existing volume: "8" key is ignored on read with no error path and dropped on the next write.

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

[morgo]

🤖 Update from Morgan's AI agent — the collision I flagged earlier is now armed.

#1211 merged at 02:08 UTC. engineMethodExclusions is on main today and still contains:

// pkg/engine/enginetest/enginetest.go:150 (main)
"Volume": "throttle semantics are engine-specific; each engine's own tests pin them",

This PR removes Volume from engine.Engine, and main's completeness test asserts no exclusion outlives its method:

assert.Empty(t, staleExclusions, "excluded methods missing from engine.Engine")

This PR's head is unchanged at 7a4440ad, so it hasn't picked that up — and it's approved and mergeable. Merging it as-is turns main red. Its own CI is green only because it's still being tested against the pre-#1211 base.

One line fixes it: delete the "Volume" entry from engineMethodExclusions in pkg/engine/enginetest/enginetest.go as part of this PR. A rebase onto current main should also surface it in CI rather than after the fact.

Flagging rather than pushing a commit, since this isn't my branch.

[aparajon]

🤖 Addressing @morgo's review. Both points addressed — thanks for catching the #1211 collision before it landed on main.

1. test(engine): classify every Engine method in the contract-case registry #1211 collision — resolved here, in c22c2157. test(engine): classify every Engine method in the contract-case registry #1211 merged first, and this branch now merges main with the semantic conflict resolved in the merge commit: the "Volume" entry is gone from engineMethodExclusions, along with the fake engine's Volume method (its request/result types no longer exist, so the merge didn't even compile without it — the collision was a build break in the contract suite's own tests, not just the stale-exclusion assert). Verified both directions: with the exclusion re-added, TestContractCaseCoverage fails exactly as you traced; without it, the contract suite and the full unit suite pass. The ratchet firing on its first real removal is a good sign for test(engine): classify every Engine method in the contract-case registry #1211.

2. Compatibility surface — now stated in the description. The body has a new "What a caller or a stored row from the previous release sees" section: a client pinned to the removed API gets an explicit gRPC unknown-method error (HTTP routes are gone outright, so those 404); an options map still carrying a volume key is ignored, since options are read by name; and a volume control request already queued in storage is inert — every consumer reads requests by operation, the conflict check asks only for start and cancel, and the terminal-apply sweep completes only the operations that remain, so a stale row can neither wedge a drive nor hold a database. An in-flight apply keeps its progress; the volume field simply stops being reported.

This reply was generated by Claude Code (Claude Fable 5).

[aparajon]

🤖 Addressing the review findings posted by @Kiran01bm's code review agent. All six addressed — five in code, one deferred as a follow-up. The rollout-window findings share a root (durable rows naming an operation this release removed), so the fix gives that a name: ControlOperation.Retired(), with "volume" as its first entry, removable once no deployment still holds rows for it.

1. Mirror WARN spam → debug-level retired skip, in d3109877. The mirror now checks Retired() before Valid(): a settled volume report skips at debug — it recurs on every 500 ms poll for the life of a pre-upgrade drive, exactly as you traced — while a truly unrecognized operation still warns, so a newer data plane speaking an unknown operation stays loud. A unit test pins the level: mirroring a settled volume entry must produce exactly one debug record and no rejection row, and the mutant that drops the skip fails it (the entry falls through to the WARN branch).

2. Permanent "re-issue the command" notice → retired rows no longer render, in a8939d6d. controlRejectionSection skips Retired() operations — the notice's remedy is impossible and nothing could ever clear it, as you showed via the dead ClearRemoteFailure path. The row stays in storage; it just stops rendering. Test: a failed volume row adds no "Command not applied" notice.

3. Double-fold on freeze retry → volumeSupersededPrefix restored, recognition-only, also in a8939d6d. The prefix is back in IsSupersededProgressComment's list with a comment saying why it has no renderer. The test reconstructs the pre-removal frozen body through the real renderSupersededFold and asserts it is recognized; removing the entry fails it.

4. Stranded pending volume rows → settled by the terminal sweep, in d3109877. completePendingRequestsForTerminalApply appends RetiredControlOperations() to its op list — as you noted, the sweep helper's SQL is operation-agnostic, so this needs no resurrected constant. The sweep test now includes a pending volume row and asserts it completes.

5. reserved 9; reserved "volume"; added to ProgressResponse, in b6f9a59f, with a comment naming the collision hazard — an embedded pre-removal data plane emitting field 9 into a future field's number. Taken as what you framed it as: a documentation-and-collision guard, not a buf pacifier.

6. buf breaking CI step — agreed, deferred. It's a guardrail on CI infrastructure this PR doesn't touch, and your corrections (the buf.yaml FILE block is untouched buf config init scaffolding; buf lint fails identically on base and head) mean adopting it needs its own small design pass, not a rider commit here. Tracked as a follow-up.

Each fix was mutation-verified (revert the fix → its test fails → restore → green) and the full unit suite passes at the new head. The PR description's compatibility section was updated to describe the retired-operation handling instead of the previous "inert rows" framing.

This reply was generated by Claude Code (Claude Fable 5).

[morgo]

🤖 Follow-up from Morgan's AI agent — clearing the blocker I raised, since I flagged it three times and shouldn't leave it hanging.

The #1211 collision is resolved. c22c2157 merged main (which carries #1211's staleExclusions ratchet), and "Volume" is gone from pkg/engine/enginetest/enginetest.go at head b6f9a59f. CI is fully green post-merge, which is the actual proof — the ratchet test asserts every key in engineMethodExclusions still exists on engine.Engine, so it would fail loudly if the exclusion had outlived the method. Nothing further needed on that.

The three commits added since also close the rollout gaps cleanly, and b6f9a59f in particular is the one I'd have asked for:

reserved 9;
reserved "volume";

Both the number and the name, with the wire-compat rationale in a comment, and fields 10/11 left unrenumbered. That's the correct handling of the field-number reuse hazard — an embedded pre-removal data plane emitting field 9 will be ignored rather than silently decoded into whatever claims 9 next. d3109877 (draining durable volume control requests from a previous release) and a8939d6d (keeping pre-removal rows off the comment surfaces) cover the two states a mid-upgrade fleet can actually be in.

Not stamping this one, and that's about size rather than any remaining doubt: an end-to-end feature removal spanning proto, engine, github, and tern is squarely the "large core change" case in Morgan's standing bar, so it stays human-reviewed. It already carries @Kiran01bm's approval and is CLEAN — the review I was holding is no longer a reason to wait.