Skip to content

fix(github): remove dead MySQL DROP INDEX guidance from plan comments - #1223

Draft
aparajon wants to merge 1 commit into
mainfrom
claude/charming-easley-aeb23a
Draft

fix(github): remove dead MySQL DROP INDEX guidance from plan comments#1223
aparajon wants to merge 1 commit into
mainfrom
claude/charming-easley-aeb23a

Conversation

@aparajon

Copy link
Copy Markdown
Collaborator

Spirit's unsafe classification treats index drops as safe (no data loss), so unsafe-change reasons never contain a "DROP INDEX" fragment — index drops surface through the invisible_index_before_drop linter, whose message carries no such fragment either. That made the plan-comment guidance branch keyed off counting "DROP INDEX" in unsafe reasons reachable only from a hand-written preview fixture, never from a real plan.

  • Remove the dead guidance branch and its unsafeDropIndexUsageTargets helper from the plan-comment renderer; writeUnsafeWarning/writeUnsafeDropGuidance drop their now-unused isMySQL parameter.
  • Remove the Drop Index Blocked preview scenario and its CLI registrations, and the tests asserting the removed guidance.
  • The DROP COLUMN / DROP TABLE application-usage guidance is unchanged and keeps a rendered-output test of its own.
  • TEMPLATES.md regenerated; the regen also picks up the cancelled-shard glyph from an earlier change that had not been regenerated into the file.
Drop Column Blocked (guidance that remains, unchanged)

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

ALTER TABLE `customers` DROP COLUMN `nickname`;

📋 Plan: 1 table to alter


⛔ Apply rejected: 1 unsafe change detected

  • customers: Unsafe operation detected: DROP COLUMN nickname

Destructive drop guidance:

Before allowing a destructive drop, first deploy application code that no longer reads from or writes to the dropped column.

🚨 To proceed with these destructive changes, re-run with --allow-unsafe:

schemabot apply -e staging --allow-unsafe

This PR was written by Claude Code (Claude Fable 5).

🤖 Generated with Claude Code

Spirit's unsafe classification treats index drops as safe (no data
loss), so unsafe-change reasons never contain a "DROP INDEX" fragment;
index drops surface through the invisible_index_before_drop linter,
whose message carries no such fragment either. The plan-comment
guidance branch keyed off counting "DROP INDEX" in unsafe reasons was
therefore reachable only from a hand-written preview fixture, never
from a real plan.

Remove the guidance branch, its unsafeDropIndexUsageTargets helper,
the Drop Index Blocked preview scenario, and the tests asserting the
guidance. The DROP COLUMN / DROP TABLE application-usage guidance is
unchanged, and keeps a rendered-output test of its own. TEMPLATES.md
is regenerated; the regen also picks up the cancelled-shard glyph from
an earlier change that had not been regenerated into the file.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 31, 2026 17:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Removes unreachable “DROP INDEX” destructive-drop guidance from the GitHub plan/blocked-unsafe comment rendering, along with the associated preview scenario and tests, because real unsafe-change reasons do not include DROP INDEX.

Changes:

  • Removed the dead DROP INDEX guidance branch and helper (unsafeDropIndexUsageTargets) from the plan comment renderer; simplified writeUnsafeWarning/writeUnsafeDropGuidance signatures accordingly.
  • Removed the “Drop Index Blocked” preview scenario and all CLI preview registrations for it.
  • Regenerated TEMPLATES.md to drop the removed scenario and to reflect the updated cancelled-shard glyph.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
TEMPLATES.md Removes the “Drop Index Blocked” rendered template; updates cancelled glyph rendering in regenerated output.
pkg/webhook/templates/preview.go Deletes the PreviewCommentDropIndexBlocked fixture.
pkg/webhook/templates/plan.go Removes dead drop-index guidance logic; simplifies unsafe-warning/guidance helpers and call sites.
pkg/webhook/templates/apply_test.go Deletes tests for removed drop-index guidance; keeps drop-column guidance test.
pkg/webhook/templates/apply_commands.go Updates call to writeUnsafeDropGuidance after signature change.
pkg/cmd/internal/templates/preview.go Removes the comment_drop_index_blocked preview type constant.
pkg/cmd/internal/templates/preview_dispatch.go Removes CLI dispatch case for the deleted drop-index preview.
pkg/cmd/internal/templates/preview_comment.go Removes the drop-index preview from “preview all comment templates” output lists.
pkg/cmd/commands/preview.go Removes the drop-index preview from the supported preview set and help text.

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

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