diff --git a/TEMPLATES.md b/TEMPLATES.md index 8539f5567..fc8214f46 100644 --- a/TEMPLATES.md +++ b/TEMPLATES.md @@ -1418,39 +1418,6 @@ schemabot apply -e staging --allow-unsafe -
-Drop Index Blocked - - -## Schema Change Plan โ€” Staging - -**Database**: `testapp` | **Type**: `MySQL` | **Schema Name**: `testapp` - -*Requested by @jackjackbits at 2026-01-01 00:00:00 UTC ยท planned from [`abcdef1`](https://github.com/block/schemabot/commit/abcdef1234567890abcdef1234567890abcdef12)* - -```sql -ALTER TABLE `customers` DROP INDEX `idx_customers_email`; -``` - -๐Ÿ“‹ **Plan**: **1** table to alter - ---- - -**โ›” Apply rejected**: 1 unsafe change detected -- `customers`: Unsafe operation detected: DROP INDEX `idx_customers_email` - -**Destructive drop guidance:** - -Before dropping an index in MySQL, first make the dropped index invisible and verify application queries no longer rely on it for safe performance. - -**๐Ÿšจ To proceed with these destructive changes, re-run with `--allow-unsafe`:** -``` -schemabot apply -e staging --allow-unsafe -``` - - -
-
Schema Lint Errors Blocked @@ -7906,8 +7873,8 @@ schemabot apply -e production | --- | --- | | `-40` | โœ… completed | | `40-80` | โœ… completed | -| `80-c0` | โ›” cancelled | -| `c0-` | โ›” cancelled | +| `80-c0` | ๐Ÿšซ cancelled | +| `c0-` | ๐Ÿšซ cancelled | --- diff --git a/pkg/cmd/commands/preview.go b/pkg/cmd/commands/preview.go index a167dbee2..c3a63ca65 100644 --- a/pkg/cmd/commands/preview.go +++ b/pkg/cmd/commands/preview.go @@ -91,7 +91,7 @@ func (cmd *PreviewCmd) Run(g *Globals) error { templates.PreviewCommentMySQLMultiSchema, templates.PreviewCommentHelp, templates.PreviewCommentSupportChannel, templates.PreviewCommentErrors, templates.PreviewCommentUnsafeBlocked, - templates.PreviewCommentDropColumnBlocked, templates.PreviewCommentDropIndexBlocked, + templates.PreviewCommentDropColumnBlocked, templates.PreviewCommentLintErrorsBlocked, templates.PreviewCommentApplyPlan, templates.PreviewCommentApplyPlanOptions, templates.PreviewCommentApplyPlanUnsafe, @@ -282,7 +282,6 @@ Comment Templates (GitHub PR comments): comment_errors All error comment templates comment_unsafe_blocked Unsafe changes blocked (no --allow-unsafe) comment_drop_column_blocked Drop column blocked with destructive-drop guidance - comment_drop_index_blocked Drop index blocked with destructive-drop guidance comment_lint_errors_blocked Error-severity schema lint violations block apply comment_single_progress Single table: running (most common case) comment_single_complete Single table: completed diff --git a/pkg/cmd/internal/templates/preview.go b/pkg/cmd/internal/templates/preview.go index 081d1a9e8..be29ef10f 100644 --- a/pkg/cmd/internal/templates/preview.go +++ b/pkg/cmd/internal/templates/preview.go @@ -144,7 +144,6 @@ const ( PreviewCommentErrors PreviewType = "comment_errors" // All error comment templates PreviewCommentUnsafeBlocked PreviewType = "comment_unsafe_blocked" // Unsafe changes blocked (no --allow-unsafe) PreviewCommentDropColumnBlocked PreviewType = "comment_drop_column_blocked" // Drop column blocked with destructive-drop guidance - PreviewCommentDropIndexBlocked PreviewType = "comment_drop_index_blocked" // Drop index blocked with destructive-drop guidance PreviewCommentLintErrorsBlocked PreviewType = "comment_lint_errors_blocked" // Error-severity schema lint violations block apply PreviewCommentApplyPlan PreviewType = "comment_apply_plan" // Locked apply-plan comment PreviewCommentApplyPlanOptions PreviewType = "comment_apply_plan_options" // Locked apply-plan with options diff --git a/pkg/cmd/internal/templates/preview_comment.go b/pkg/cmd/internal/templates/preview_comment.go index 43215f128..3909194b8 100644 --- a/pkg/cmd/internal/templates/preview_comment.go +++ b/pkg/cmd/internal/templates/preview_comment.go @@ -71,7 +71,6 @@ func previewCommentAllOutput() { {"SCHEMA CHANGE APPLY (UNSAFE + ALLOWED)", func() { fmt.Print(webhooktemplates.PreviewCommentApplyPlanUnsafe()) }}, {"UNSAFE CHANGES BLOCKED", func() { fmt.Print(webhooktemplates.PreviewCommentUnsafeBlocked()) }}, {"DROP COLUMN BLOCKED", func() { fmt.Print(webhooktemplates.PreviewCommentDropColumnBlocked()) }}, - {"DROP INDEX BLOCKED", func() { fmt.Print(webhooktemplates.PreviewCommentDropIndexBlocked()) }}, {"SCHEMA LINT ERRORS BLOCKED", func() { fmt.Print(webhooktemplates.PreviewCommentLintErrorsBlocked()) }}, {"MULTI-ENV PLAN (IDENTICAL)", func() { fmt.Print(webhooktemplates.PreviewCommentMultiEnvPlan()) }}, {"MULTI-ENV PLAN (DIFFERENT)", func() { fmt.Print(webhooktemplates.PreviewCommentMultiEnvPlanDiff()) }}, @@ -188,7 +187,6 @@ func previewCommentPlanAllOutput() { {"DEPLOYMENT DRIFT (DETECTED)", func() { fmt.Print(webhooktemplates.PreviewCommentPlanDriftDetected()) }}, {"DEPLOYMENT DRIFT (COULD NOT VERIFY)", func() { fmt.Print(webhooktemplates.PreviewCommentPlanDriftUnverified()) }}, {"DROP COLUMN BLOCKED", func() { fmt.Print(webhooktemplates.PreviewCommentDropColumnBlocked()) }}, - {"DROP INDEX BLOCKED", func() { fmt.Print(webhooktemplates.PreviewCommentDropIndexBlocked()) }}, {"SCHEMA LINT ERRORS BLOCKED", func() { fmt.Print(webhooktemplates.PreviewCommentLintErrorsBlocked()) }}, {"HELP COMMENT", func() { fmt.Print(webhooktemplates.PreviewCommentHelp()) }}, {"SUPPORT CHANNEL FOOTER", func() { fmt.Print(webhooktemplates.PreviewCommentSupportChannel()) }}, diff --git a/pkg/cmd/internal/templates/preview_dispatch.go b/pkg/cmd/internal/templates/preview_dispatch.go index a12cd74ae..909cdaf91 100644 --- a/pkg/cmd/internal/templates/preview_dispatch.go +++ b/pkg/cmd/internal/templates/preview_dispatch.go @@ -182,8 +182,6 @@ func PreviewCLIOutput(previewType PreviewType) { fmt.Print(webhooktemplates.PreviewCommentUnsafeBlocked()) case PreviewCommentDropColumnBlocked: fmt.Print(webhooktemplates.PreviewCommentDropColumnBlocked()) - case PreviewCommentDropIndexBlocked: - fmt.Print(webhooktemplates.PreviewCommentDropIndexBlocked()) case PreviewCommentLintErrorsBlocked: fmt.Print(webhooktemplates.PreviewCommentLintErrorsBlocked()) case PreviewCommentApplyPlan: diff --git a/pkg/webhook/templates/apply_commands.go b/pkg/webhook/templates/apply_commands.go index 60355081f..564d60ae4 100644 --- a/pkg/webhook/templates/apply_commands.go +++ b/pkg/webhook/templates/apply_commands.go @@ -180,7 +180,7 @@ func RenderUnsafeChangesBlocked(data PlanCommentData) string { writeUnsafeChangeItem(&sb, "`"+c.Table+"`", c.Reason) } sb.WriteString("\n") - writeUnsafeDropGuidance(&sb, data.UnsafeChanges, data.IsMySQL) + writeUnsafeDropGuidance(&sb, data.UnsafeChanges) // Attribution comes before the opt-in this comment coaches: --allow-unsafe // is consent to destroy the data, and whether the change is this pull diff --git a/pkg/webhook/templates/apply_test.go b/pkg/webhook/templates/apply_test.go index b8534b8d0..b700dd64a 100644 --- a/pkg/webhook/templates/apply_test.go +++ b/pkg/webhook/templates/apply_test.go @@ -339,67 +339,7 @@ func TestRenderApplyStatusComment_ThrottledChecksumming(t *testing.T) { assert.Contains(t, result, "- โ„น๏ธ _Throttled: threads-running 21 > 18 ยท backing off while the database's active threads exceed its budget ([docs](https://github.com/block/schemabot/blob/main/docs/throttle.md))_") } -func TestUnsafeDropIndexUsageTargets(t *testing.T) { - tests := []struct { - name string - changes []UnsafeChangeData - wantActionTarget string - wantInvisibleTarget string - wantQueryTarget string - wantOK bool - }{ - { - name: "drop index", - changes: []UnsafeChangeData{ - {Table: "customers", Reason: "Unsafe operation detected: DROP INDEX `idx_customers_email`"}, - }, - wantActionTarget: "an index", - wantInvisibleTarget: "the dropped index", - wantQueryTarget: "it", - wantOK: true, - }, - { - name: "multiple drop indexes", - changes: []UnsafeChangeData{ - {Table: "customers", Reason: "Unsafe operation detected: DROP INDEX `idx_customers_email`; Unsafe operation detected: DROP INDEX `idx_customers_phone`"}, - }, - wantActionTarget: "indexes", - wantInvisibleTarget: "any dropped indexes", - wantQueryTarget: "them", - wantOK: true, - }, - { - name: "drop index with drop column", - changes: []UnsafeChangeData{ - {Table: "customers", Reason: "Unsafe operation detected: DROP COLUMN `nickname`; Unsafe operation detected: DROP INDEX `idx_customers_email`"}, - }, - wantActionTarget: "an index", - wantInvisibleTarget: "the dropped index", - wantQueryTarget: "it", - wantOK: true, - }, - { - name: "other unsafe change", - changes: []UnsafeChangeData{ - {Table: "customers", Reason: "Unsafe operation detected: MODIFY COLUMN"}, - }, - wantOK: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - gotActionTarget, gotInvisibleTarget, gotQueryTarget, ok := unsafeDropIndexUsageTargets(tt.changes) - - assert.Equal(t, tt.wantOK, ok) - assert.Equal(t, tt.wantActionTarget, gotActionTarget) - assert.Equal(t, tt.wantInvisibleTarget, gotInvisibleTarget) - assert.Equal(t, tt.wantQueryTarget, gotQueryTarget) - }) - } -} - -func TestRenderUnsafeChangesBlockedIncludesDropIndexGuidance(t *testing.T) { +func TestRenderUnsafeChangesBlockedIncludesDropColumnGuidance(t *testing.T) { rendered := RenderUnsafeChangesBlocked(PlanCommentData{ Database: "testapp", SchemaName: "testapp", @@ -409,66 +349,18 @@ func TestRenderUnsafeChangesBlockedIncludesDropIndexGuidance(t *testing.T) { { Keyspace: "testapp", Statements: []string{ - "ALTER TABLE `customers` DROP COLUMN `nickname`, DROP INDEX `idx_customers_email`;", + "ALTER TABLE `customers` DROP COLUMN `nickname`;", }, }, }, HasUnsafeChanges: true, UnsafeChanges: []UnsafeChangeData{ - {Table: "customers", Reason: "Unsafe operation detected: DROP COLUMN `nickname`; Unsafe operation detected: DROP INDEX `idx_customers_email`"}, + {Table: "customers", Reason: "Unsafe operation detected: DROP COLUMN `nickname`"}, }, }) + assert.Contains(t, rendered, "**Destructive drop guidance:**") assert.Contains(t, rendered, "Before allowing a destructive drop, first deploy application code that no longer reads from or writes to the dropped column.") - assert.Contains(t, rendered, "Before dropping an index in MySQL, first make the dropped index invisible and verify application queries no longer rely on it for safe performance.") - assert.NotContains(t, rendered, "reads from or writes to the dropped index") -} - -func TestRenderUnsafeChangesBlockedUsesPluralMySQLDropIndexGuidance(t *testing.T) { - rendered := RenderUnsafeChangesBlocked(PlanCommentData{ - Database: "testapp", - SchemaName: "testapp", - Environment: "staging", - IsMySQL: true, - Changes: []KeyspaceChangeData{ - { - Keyspace: "testapp", - Statements: []string{ - "ALTER TABLE `customers` DROP INDEX `idx_customers_email`, DROP INDEX `idx_customers_phone`;", - }, - }, - }, - HasUnsafeChanges: true, - UnsafeChanges: []UnsafeChangeData{ - {Table: "customers", Reason: "Unsafe operation detected: DROP INDEX `idx_customers_email`; Unsafe operation detected: DROP INDEX `idx_customers_phone`"}, - }, - }) - - assert.Contains(t, rendered, "Before dropping indexes in MySQL, first make any dropped indexes invisible and verify application queries no longer rely on them for safe performance.") - assert.NotContains(t, rendered, "Before dropping an index in MySQL, first make any dropped indexes invisible") -} - -func TestRenderUnsafeChangesBlockedDoesNotMentionInvisibleIndexesForVitess(t *testing.T) { - rendered := RenderUnsafeChangesBlocked(PlanCommentData{ - Database: "testapp", - Environment: "staging", - IsMySQL: false, - Changes: []KeyspaceChangeData{ - { - Keyspace: "testapp", - Statements: []string{ - "ALTER TABLE `customers` DROP INDEX `idx_customers_email`;", - }, - }, - }, - HasUnsafeChanges: true, - UnsafeChanges: []UnsafeChangeData{ - {Table: "customers", Reason: "Unsafe operation detected: DROP INDEX `idx_customers_email`"}, - }, - }) - - assert.Contains(t, rendered, "Before allowing a destructive drop, verify application queries no longer rely on the dropped index for safe performance.") - assert.NotContains(t, rendered, "invisible") } func TestRenderApplyStatusComment_Running(t *testing.T) { diff --git a/pkg/webhook/templates/plan.go b/pkg/webhook/templates/plan.go index 09ecb5747..9f3c237a2 100644 --- a/pkg/webhook/templates/plan.go +++ b/pkg/webhook/templates/plan.go @@ -335,7 +335,7 @@ func RenderPlanComment(data PlanCommentData) string { // operator acknowledged them with --allow-unsafe (apply-confirm re-checks // and blocks otherwise), so repeating them there is noise. if data.HasUnsafeChanges && len(data.UnsafeChanges) > 0 && !data.IsLocked { - writeUnsafeWarning(&sb, data.UnsafeChanges, data.IsMySQL) + writeUnsafeWarning(&sb, data.UnsafeChanges) } // Lint violations โ€” shown on the plan comment for review, omitted on the @@ -1057,7 +1057,7 @@ func writeDirectChanges(sb *strings.Builder, changes []DirectChangeData, databas sb.WriteString("\n" + footer + "\n\n") } -func writeUnsafeWarning(sb *strings.Builder, changes []UnsafeChangeData, isMySQL bool) { +func writeUnsafeWarning(sb *strings.Builder, changes []UnsafeChangeData) { n := countUnsafeFindings(changes) fmt.Fprintf(sb, glyph.Attention+" **Issues**: %d unsafe %s detected\n", n, pluralize("change", n)) for _, c := range changes { @@ -1068,7 +1068,7 @@ func writeUnsafeWarning(sb *strings.Builder, changes []UnsafeChangeData, isMySQL writeUnsafeChangeItem(sb, table, c.Reason) } sb.WriteString("\n") - writeUnsafeDropGuidance(sb, changes, isMySQL) + writeUnsafeDropGuidance(sb, changes) } // writeUnsafeChangeItem writes one table's unsafe findings as a list item: @@ -1108,24 +1108,14 @@ func countUnsafeFindings(changes []UnsafeChangeData) int { return n } -func writeUnsafeDropGuidance(sb *strings.Builder, changes []UnsafeChangeData, isMySQL bool) { +func writeUnsafeDropGuidance(sb *strings.Builder, changes []UnsafeChangeData) { applicationUsageTarget, hasApplicationUsageTarget := unsafeDropApplicationUsageTarget(changes) - indexActionTarget, indexInvisibleTarget, indexQueryTarget, hasIndexUsageTarget := unsafeDropIndexUsageTargets(changes) - if !hasApplicationUsageTarget && !hasIndexUsageTarget { + if !hasApplicationUsageTarget { return } sb.WriteString("**Destructive drop guidance:**\n\n") - if hasApplicationUsageTarget { - fmt.Fprintf(sb, "Before allowing a destructive drop, first deploy application code that no longer reads from or writes to %s.\n\n", applicationUsageTarget) - } - if hasIndexUsageTarget { - if isMySQL { - fmt.Fprintf(sb, "Before dropping %s in MySQL, first make %s invisible and verify application queries no longer rely on %s for safe performance.\n\n", indexActionTarget, indexInvisibleTarget, indexQueryTarget) - } else { - fmt.Fprintf(sb, "Before allowing a destructive drop, verify application queries no longer rely on %s for safe performance.\n\n", indexInvisibleTarget) - } - } + fmt.Fprintf(sb, "Before allowing a destructive drop, first deploy application code that no longer reads from or writes to %s.\n\n", applicationUsageTarget) } func unsafeDropApplicationUsageTarget(changes []UnsafeChangeData) (string, bool) { @@ -1164,21 +1154,6 @@ func unsafeDropApplicationUsageTarget(changes []UnsafeChangeData) (string, bool) return "", false } -func unsafeDropIndexUsageTargets(changes []UnsafeChangeData) (actionTarget, invisibleTarget, queryTarget string, ok bool) { - dropIndexes := 0 - for _, change := range changes { - dropIndexes += strings.Count(strings.ToUpper(change.Reason), "DROP INDEX") - } - - if dropIndexes == 1 { - return "an index", "the dropped index", "it", true - } - if dropIndexes > 1 { - return "indexes", "any dropped indexes", "them", true - } - return "", "", "", false -} - // lintWarningsFoldThreshold is the warning count above which the lint section // collapses into a details block grouped by table. Short lists stay inline so // a single advisory finding never needs a click; long lists stop dominating @@ -1518,7 +1493,7 @@ func writeEnvironmentPlanSection(sb *strings.Builder, plan *PlanCommentData) { // Unsafe changes warning if plan.HasUnsafeChanges && len(plan.UnsafeChanges) > 0 { - writeUnsafeWarning(sb, plan.UnsafeChanges, plan.IsMySQL) + writeUnsafeWarning(sb, plan.UnsafeChanges) } // Lint violations diff --git a/pkg/webhook/templates/preview.go b/pkg/webhook/templates/preview.go index 692cf69a4..d08b73d23 100644 --- a/pkg/webhook/templates/preview.go +++ b/pkg/webhook/templates/preview.go @@ -1035,36 +1035,6 @@ func PreviewCommentDropColumnBlocked() string { }) } -// PreviewCommentDropIndexBlocked renders a sample plan where a destructive -// index drop is blocked until query performance has been reviewed. -func PreviewCommentDropIndexBlocked() string { - return RenderUnsafeChangesBlocked(PlanCommentData{ - Database: "testapp", - SchemaName: "testapp", - Environment: "staging", - HeadSHA: previewHeadSHA, - Repository: previewRepository, - RequestedBy: previewRequestedBy, - IsMySQL: true, - DatabaseType: "mysql", - Changes: []KeyspaceChangeData{ - { - Keyspace: "testapp", - Statements: []string{ - "ALTER TABLE `customers` DROP INDEX `idx_customers_email`;", - }, - }, - }, - HasUnsafeChanges: true, - UnsafeChanges: []UnsafeChangeData{ - { - Table: "customers", - Reason: "Unsafe operation detected: DROP INDEX `idx_customers_email`", - }, - }, - }) -} - // PreviewCommentLintErrorsBlocked renders a sample plan where error-severity // schema lint violations block the apply. The reasons carry the raw // "[ERROR] linter:" prefixes engines report; the renderer strips them.