fix(planetscale): delete the branch an apply created when it fails before its deploy request

pkg/engine/planetscale/apply.go

@@ -49,7 +49,7 @@ func vschemaDiffsFromChanges(changes []engine.SchemaChange) []vschemaKeyspaceDif
 // Apply starts executing a schema change plan.
 // Creates a PlanetScale branch, applies DDL via MySQL connection to the branch,
 // then creates and starts a deploy request.
-func (e *Engine) Apply(ctx context.Context, req *engine.ApplyRequest) (*engine.ApplyResult, error) {
+func (e *Engine) Apply(ctx context.Context, req *engine.ApplyRequest) (result *engine.ApplyResult, retErr error) {
 	e.logger.Info("applying plan",
 		"plan_id", req.PlanID,
 		"database", req.Database,
@@ -128,6 +128,24 @@ func (e *Engine) Apply(ctx context.Context, req *engine.ApplyRequest) (*engine.A
 	var branchName string
 	branchStart := time.Now()
 
+	// A branch SchemaBot creates exists only to carry this apply's DDL into a
+	// deploy request. Once that deploy request exists it owns the teardown
+	// (AutoDeleteBranch); until then nothing does, so an apply that fails while
+	// preparing the branch would strand it. Branches are quota'd, and the
+	// failures in this window are the ordinary ones — DDL the engine refuses —
+	// so the strand accumulates until branch creation itself starts failing for
+	// unrelated schema changes. ownedBranch names the branch this apply is
+	// responsible for; it is cleared once the deploy request takes ownership,
+	// and it is never set for an operator-supplied branch, which SchemaBot does
+	// not own.
+	ownedBranch := ""
+	defer func() {
+		if retErr == nil || ownedBranch == "" {
+			return
+		}
+		e.deleteOwnedBranch(ctx, client, org, req.Database, ownedBranch, retErr)
+	}()
+
 	if existingBranch != "" {
 		// Reuse existing branch: wait for ready, refresh schema from main, wait again
 		branchName = existingBranch
@@ -187,6 +205,7 @@ func (e *Engine) Apply(ctx context.Context, req *engine.ApplyRequest) (*engine.A
 		if err != nil {
 			return nil, fmt.Errorf("create branch: %w", err)
 		}
+		ownedBranch = branchName
 
 		// Wait for branch to be ready
 		if err := e.waitForBranchReady(ctx, client, org, req.Database, branchName); err != nil {
@@ -280,6 +299,8 @@ func (e *Engine) Apply(ctx context.Context, req *engine.ApplyRequest) (*engine.A
 	if err != nil {
 		return nil, fmt.Errorf("create deploy request: %w", err)
 	}
+	// The deploy request now owns the branch's teardown.
+	ownedBranch = ""
 	emitEvent(engine.ApplyEvent{
 		Message: fmt.Sprintf("Deploy request #%d created, validating...", dr.Number),
 		Metadata: map[string]string{

pkg/engine/planetscale/branch_cleanup.go

@@ -0,0 +1,42 @@
+package planetscale
+
+import (
+	"context"
+	"time"
+
+	ps "github.com/planetscale/planetscale-go/planetscale"
+
+	"github.com/block/schemabot/pkg/psclient"
+)
+
+// branchDeleteTimeout bounds the cleanup delete so a slow or unreachable
+// PlanetScale API cannot hold the apply's failure path open.
+const branchDeleteTimeout = 30 * time.Second
+
+// deleteOwnedBranch removes a branch this apply created and no deploy request
+// took ownership of, so a failure while preparing the branch does not strand
+// quota. cause is the failure that triggered the cleanup; it is logged with the
+// outcome so the two are readable together.
+//
+// The delete runs on its own deadline, detached from the apply's context, so a
+// cancelled or timed-out apply still cleans up after itself. A branch that could
+// not be deleted is logged at error level with the identifiers needed to remove
+// it by hand — an undeletable branch must be visible, not silently retried.
+func (e *Engine) deleteOwnedBranch(ctx context.Context, client psclient.PSClient, org, database, branch string, cause error) {
+	ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), branchDeleteTimeout)
+	defer cancel()
+
+	err := client.DeleteBranch(ctx, &ps.DeleteDatabaseBranchRequest{
+		Organization: org,
+		Database:     database,
+		Branch:       branch,
+	})
+	if err != nil {
+		e.logger.Error("failed to delete the branch left behind by a failed apply; delete it manually to reclaim branch quota",
+			"organization", org, "database", database, "branch", branch,
+			"apply_error", cause, "error", err)
+		return
+	}
+	e.logger.Info("deleted the branch created for an apply that failed before its deploy request",
+		"organization", org, "database", database, "branch", branch, "apply_error", cause)
+}

pkg/engine/planetscale/branch_cleanup_test.go

@@ -0,0 +1,95 @@
+package planetscale
+
+import (
+	"context"
+	"errors"
+	"sync"
+	"testing"
+
+	ps "github.com/planetscale/planetscale-go/planetscale"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/block/schemabot/pkg/engine"
+	"github.com/block/schemabot/pkg/psclient"
+)
+
+// branchLifecycleClient serves a branch that exists and is ready, and fails the
+// credential request that follows it — the shape of an apply that dies while
+// preparing its branch, before any deploy request exists. Deletions are
+// recorded so the test can assert on the cleanup.
+type branchLifecycleClient struct {
+	psclient.PSClient
+
+	mu      sync.Mutex
+	created []string
+	deleted []string
+}
+
+func (c *branchLifecycleClient) GetBranch(context.Context, *ps.GetDatabaseBranchRequest) (*ps.DatabaseBranch, error) {
+	return &ps.DatabaseBranch{Ready: true, SafeMigrations: true}, nil
+}
+
+func (c *branchLifecycleClient) CreateBranch(_ context.Context, req *ps.CreateDatabaseBranchRequest) (*ps.DatabaseBranch, error) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	c.created = append(c.created, req.Name)
+	return &ps.DatabaseBranch{Name: req.Name, Ready: true}, nil
+}
+
+func (c *branchLifecycleClient) DeleteBranch(_ context.Context, req *ps.DeleteDatabaseBranchRequest) error {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	c.deleted = append(c.deleted, req.Branch)
+	return nil
+}
+
+func (c *branchLifecycleClient) RefreshSchema(context.Context, string, string, string) error {
+	return nil
+}
+
+func (c *branchLifecycleClient) CreateBranchPassword(context.Context, *ps.DatabaseBranchPasswordRequest) (*ps.DatabaseBranchPassword, error) {
+	return nil, errors.New("branch credentials unavailable")
+}
+
+func (c *branchLifecycleClient) snapshot() (created, deleted []string) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	return append([]string(nil), c.created...), append([]string(nil), c.deleted...)
+}
+
+// A branch SchemaBot creates is stranded if the apply fails before a deploy
+// request exists to own its teardown, and PlanetScale branches are quota'd —
+// the strand eventually blocks unrelated schema changes. The apply must clean up
+// after itself, while leaving an operator-supplied branch alone: SchemaBot did
+// not create it and must not remove it.
+func TestApplyDeletesItsOwnBranchWhenItFailsBeforeTheDeployRequest(t *testing.T) {
+	applyRequest := func(options map[string]string) *engine.ApplyRequest {
+		return &engine.ApplyRequest{
+			PlanID:      "plan-0123456789abcdef",
+			Database:    "commerce",
+			Credentials: conformanceCredentials(),
+			Options:     options,
+		}
+	}
+
+	t.Run("a branch this apply created is deleted", func(t *testing.T) {
+		client := &branchLifecycleClient{}
+		_, err := conformanceEngine(client).Apply(t.Context(), applyRequest(nil))
+		require.Error(t, err)
+
+		created, deleted := client.snapshot()
+		require.Len(t, created, 1)
+		assert.Equal(t, created, deleted, "the apply deletes exactly the branch it created")
+	})
+
+	t.Run("an operator-supplied branch is left alone", func(t *testing.T) {
+		client := &branchLifecycleClient{}
+		_, err := conformanceEngine(client).Apply(t.Context(), applyRequest(map[string]string{"branch": "operator-branch"}))
+		require.Error(t, err)
+
+		created, deleted := client.snapshot()
+		assert.Empty(t, created)
+		assert.Empty(t, deleted)
+	})
+}