Skip to content

fix(rollback): plan rollback from selected apply#119

Draft
aparajon wants to merge 2 commits into
mainfrom
armand/rollback-fixes
Draft

fix(rollback): plan rollback from selected apply#119
aparajon wants to merge 2 commits into
mainfrom
armand/rollback-fixes

Conversation

@aparajon
Copy link
Copy Markdown
Collaborator

@aparajon aparajon commented May 18, 2026

Summary

Rollback now starts from the apply the operator selected, instead of rediscovering whatever schema state happens to be newest on the PR. schemabot rollback <apply-id> validates that the apply belongs to the PR/database/environment, builds the rollback plan from that apply's stored original schema, and pins that rollback plan to the lock so rollback-confirm executes the plan the operator reviewed.

This keeps rollback deterministic when a PR has multiple applies, newer plans, or multi-database configuration. It also keeps unsafe cases fail-closed: older applies without captured original schema cannot be rolled back automatically, rollback plans cannot be consumed by apply-confirm, and Vitess VSchema rollback remains blocked until original VSchema capture is supported.

Generated with Amp

Copilot AI review requested due to automatic review settings May 18, 2026 19:19
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates rollback planning to be based on the requested apply’s captured pre-apply schema, routes rollback planning through the normal Plan flow so local and gRPC clients share behavior, and extends Tern plan responses to include the captured “original schema” for later rollback planning and remote plan storage.

Changes:

  • Add original_schema to SchemaChange in the Tern proto and propagate it through LocalClient Plan responses and SchemaBot plan storage.
  • Replace the prior client-level RollbackPlan flow with service-level rollback planning (ExecuteRollbackPlanForApply) that calls the standard Plan path and stores the resulting plan.
  • Update webhook/API rollback handlers and tests to use apply-scoped rollback planning.

Reviewed changes

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

Show a summary per file
File Description
pkg/webhook/rollback.go Rollback plan generation now uses the requested apply; rollback-confirm re-plans via the service.
pkg/tern/local_control.go Removes LocalClient RollbackPlan implementation (no longer needed).
pkg/tern/local_client.go Attaches stored OriginalSchema to plan response schema changes.
pkg/tern/local_client_integration_test.go Asserts OriginalSchema is present in Plan responses.
pkg/tern/grpc_client.go Removes unsupported RollbackPlan method.
pkg/tern/client.go Removes RollbackPlan from the tern.Client interface.
pkg/proto/tern.proto Adds original_schema field to SchemaChange.
pkg/proto/ternv1/tern.pb.go Regenerated protobuf bindings for original_schema.
pkg/proto/ternv1/tern_grpc.pb.go Regenerated gRPC bindings (version bump).
pkg/api/rollback_plan_integration_test.go Integration test ensuring rollback uses the requested apply’s original schema and stores rollback plan original schema.
pkg/api/proto_helpers.go Persists SchemaChange.OriginalSchema into stored plan namespaces.
pkg/api/plan_handlers.go Introduces ExecuteRollbackPlanForApply, factors out storePlanResponse, adds latest-completed-apply lookup.
pkg/api/handlers_test.go Enhances mock Tern client to capture/return Plan requests/responses.
pkg/api/control_handlers.go Rollback plan endpoint now plans from the requested apply (apply-scoped).
Files not reviewed (2)
  • pkg/proto/ternv1/tern.pb.go: Language not supported
  • pkg/proto/ternv1/tern_grpc.pb.go: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/tern/local_client.go Outdated
Comment thread pkg/webhook/rollback.go Outdated
@aparajon aparajon force-pushed the armand/rollback-fixes branch from 152eb57 to 8c18ebb Compare May 18, 2026 19:39
@aparajon aparajon changed the title fix(rollback): plan from requested apply state fix(rollback): require explicit apply ID May 18, 2026
@aparajon aparajon force-pushed the armand/rollback-fixes branch from 8c18ebb to ddca597 Compare May 18, 2026 19:55
@aparajon
Copy link
Copy Markdown
Collaborator Author

🤖 Note for whoever picks this up after #137: PR #137 does not obsolete this rollback PR. #137 only changes admin/control operations (cutover, stop, start, volume, revert, skip-revert) to require apply_id + environment and derive database/routing from storage.

This PR still owns rollback-specific behavior: building rollback plans from the requested apply's captured original schema, preventing fallback to the latest completed apply, requiring/validating the same apply ID through rollback-confirm, preserving original schema in Tern plan responses, and keeping the rollback webhook/integration coverage.

Expect a rebase/conflict cleanup with #137 in shared files like pkg/api/control_handlers.go, pkg/api/handlers_test.go, and TEMPLATES.md. A clearer post-rebase title might be fix(rollback): bind rollback to requested apply snapshot.

@aparajon aparajon force-pushed the armand/rollback-fixes branch from ddca597 to 04bb632 Compare May 21, 2026 21:09
@aparajon aparajon changed the title fix(rollback): require explicit apply ID fix(rollback): bind rollback to requested apply snapshot May 21, 2026
@aparajon aparajon marked this pull request as ready for review May 21, 2026 21:16
@aparajon aparajon requested review from Kiran01bm and morgo as code owners May 21, 2026 21:16
@aparajon aparajon force-pushed the armand/rollback-fixes branch 3 times, most recently from 3310e73 to 19b123d Compare May 26, 2026 20:30
@aparajon aparajon force-pushed the armand/rollback-fixes branch 8 times, most recently from d2be34c to f249984 Compare June 7, 2026 16:08
@aparajon aparajon changed the title fix(rollback): bind rollback to requested apply snapshot fix(rollback): plan rollback from selected apply Jun 7, 2026
@aparajon aparajon marked this pull request as draft June 7, 2026 16:24
aparajon and others added 2 commits June 8, 2026 11:11
Rollback now plans from the exact completed apply being rolled back instead of whichever completed apply happens to be latest for the PR/database. It stores whether original schema capture succeeded separately from the schema map so create-table rollbacks can target an empty original schema, while older plans without capture still fail closed.

The confirmation path also keeps rollback locks distinct from normal apply locks, routes observers through the stored rollback deployment, makes rollback-confirm database-explicit, and rejects unsupported planning-time defer-cutover flags.

Vitess VSchema rollback remains fail-closed until original VSchema capture is supported.

Co-authored-by: Amp <amp@ampcode.com>
@aparajon aparajon force-pushed the armand/rollback-fixes branch from f249984 to 47ce802 Compare June 8, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants