chore(deps): bump Spirit for quoted lint messages and render SQL fragments as code

[aparajon]

Bumps Spirit to pick up the lint-message quoting convention, and teaches the PR-comment renderer the two quoted shapes it could not convert. Together they finish the job started in the first quoting pass: every SQL token in a lint or unsafe message now renders as inline code on GitHub.

• Spirit bump: lint messages now %q-quote type names, collations, column lists (with number agreement — column "a" vs columns ("a", "b")), index/table names, and every unsafe-operation fragment through one unified shape.
• Renderer: CodeQuoteIdentifiers converts a quoted fragment led by a DDL keyword ("DROP COLUMN nickname") and a quoted token carrying attribute words ("int(11) unsigned", "created_at DESC") to inline code, sizing the code-span fence past the longest backtick run in the content so identifiers with embedded backticks cannot close the span early. The quoted-token pattern admits the comma and single quote that parameterised and enumerated types carry ("decimal(10,2)", "enum('active','archived')" each render as one code span), and quoted prose is untouched. The single-quote conversion pass is gone: no registered linter emits single-quoted tokens, and it corrupted the values inside enum/set types.
• Preview fixtures move to the shapes Spirit now emits, so TEMPLATES.md shows the full result.

Behavior changes in this bump (beyond message formatting):

• Spirit now refuses every ALTER against a table without a primary key at classification time (altering a table without a primary key is not supported) instead of failing mid-run at table setup. Plans for such tables route through the standard refused-statement handling; the refusal contract test pins both an add-column and a modify-column shape.
• The type_pedantic linter (default-on) now also checks collation consistency across tables, assuming utf8mb4 defaults, so plans can surface new advisory collation warnings on schemas with mixed collations.
• Transitive dependency moves beyond the Spirit module itself: golang.org/x/{crypto,mod,net,sys,text,tools} and testify advance to the versions Spirit now requires.

Plan comment with lint warnings

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

CREATE TABLE `users` (
    `id` bigint unsigned NOT NULL AUTO_INCREMENT,
    `email` varchar(255) NOT NULL,
    `created_at` timestamp DEFAULT current_timestamp(),
    PRIMARY KEY(`id`),
    INDEX `idx_email`(`email`)
) ENGINE InnoDB,
  CHARSET utf8mb4,
  COLLATE utf8mb4_0900_ai_ci;

CREATE TABLE `orders` (
    `id` bigint unsigned NOT NULL AUTO_INCREMENT,
    `user_id` bigint NOT NULL,
    `total_cents` bigint NOT NULL,
    `status` varchar(50) NOT NULL DEFAULT 'pending',
    PRIMARY KEY(`id`),
    INDEX `idx_user_id`(`user_id`)
) ENGINE InnoDB,
  CHARSET utf8mb4,
  COLLATE utf8mb4_0900_ai_ci;

ALTER TABLE `products` ADD INDEX `idx_category_price`(`category`, `price`);

💡 Lint Warnings: 2 advisory findings

• users: Column created_at uses TIMESTAMP which overflows on 2038-01-19. Consider using DATETIME instead.
• products: Index idx_category on column category is redundant - covered by index idx_category_price on columns (category, price)

📋 Plan: 2 tables to create, 1 table to alter

---

▶️ To apply all schema changes from this PR, comment:

schemabot apply -e staging

Unsafe change rejected

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 (Fable 5).

[Copilot]

Pull request overview

Updates SchemaBot’s Spirit dependency so lint messages quote type names/collations/allowed-value tokens with %q, allowing the existing quoted-token rendering pass (ui.CodeQuoteIdentifiers) to format them as inline code in GitHub PR comments without additional SchemaBot-side parsing logic.

Changes:

• Bump github.com/block/spirit to v0.16.1-0.20260828161121-d5c94537b5ff (root module + e2e/consumermodule) and refresh transitive deps.
• Update preview fixtures + tests to match Spirit’s new double-quoted token shape (rendered as backticked inline code in Markdown surfaces).
• Regenerate TEMPLATES.md to reflect the updated rendering.

Reviewed changes

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

Show a summary per file

File	Description
TEMPLATES.md	Updates rendered examples so type tokens appear as inline code (backticked) in lint warnings.
pkg/webhook/templates/preview.go	Adjusts preview fixture lint/unsafe messages to Spirit’s new %q-quoted token style.
pkg/webhook/plan_test.go	Updates assertions to expect backticked TIMESTAMP/DATETIME after ui.CodeQuoteIdentifiers rewriting.
pkg/cmd/internal/templates/pull_test.go	Updates CLI pull-schema lint output expectations to include new quoted tokens (kept as quotes on CLI surfaces).
pkg/cmd/internal/templates/preview_pull.go	Updates CLI preview response fixture lint message to new quoted token style.
pkg/cmd/internal/templates/plan_unsafe_test.go	Updates unsafe-change rendering expectations for the new quoted token style.
go.mod	Bumps Spirit + several dependencies (e.g., testify, x/*).
go.sum	Updates sums to match the dependency bumps.
e2e/consumermodule/go.mod	Mirrors dependency bumps in the consumer module.
e2e/consumermodule/go.sum	Updates sums for the consumer module dependency bumps.

---

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

[Kiran01bm]

🤖 Review findings - created by Kiran's code review agent - for schemabot/pull/1207, 3322a91.

Verdict: 8 findings — 1 blocking (an unnoticed refusal-routing change rides along with the Spirit bump), 4 non-blocking.

Blocking

1. The bump silently adds a new refusal verdict, and the guard test misses it. New Spirit registers a statement-scope check primarykeyexists, and StatementRefusal picks up scope members generically — so an ALTER on a PK-less table flips refused false→true, rerouting the plan to ExecutionModeBlocked or to native MySQL DDL via resolveRefusedMode. Probed both module versions: old returns refused=false for every ALTER shape on a PK-less table, new returns refused=true reason="altering a table without a primary key is not supported". TestStatementRefusalContract exists precisely to catch "a Spirit bump that moves a statement between routed, refused, and unclassifiable", but all 17 cases use refusalOrdersTable, which declares PRIMARY KEY (id) — the new verdict has zero coverage. The routing change is arguably an improvement (Spirit used to fail at table setup), but it should be an intentional, tested one, not a side effect of a chore(deps).

Non-blocking

2. singleQuotedIdentifier corrupts ENUM/SET types in the public plan comment. format.go:281 runs first and rewrites the SQL string literals that are part of the type: has type "enum('active','archived')" renders on GitHub as has type "enum(active,archived)", with the quotes deleted and the neighbouring varchar(32) correctly code-spanned in the same sentence. Pre-existing, but this PR makes it indefensible: after the bump no registered Spirit linter emits single-quoted identifiers any more, so the rule has no remaining upside — consider deleting it rather than extending it.

3. A new default-on Spirit lint rule starts emitting warnings, unmentioned. New type_pedantic adds collation-consistency checks (checkCollation=true, assumeCharset=utf8mb4); pkg/lint/lint.go:172 passes no Enabled map and Spirit enables every registered linter, so schemabot inherits it. Any managed schema with mixed collations — or a table with no declared DEFAULT CHARSET, now assumed utf8mb4 — starts showing new advisory findings, and crossing lintWarningsFoldThreshold also changes comment layout.

4. Incomplete fixture sweep: datetime_index_position. Spirit changed that message's type operand %s→%q, but preview.go:971-972 is the only pair of lines left unchanged in an otherwise rewritten LintViolations block. TEMPLATES.md:133-134 therefore documents has DATETIME column while production now renders has `DATETIME` column. (Unrelated: line 975 labels it column_type_consistency, not a real Spirit linter name.)

5. codeSpan's fixed two-backtick delimiter breaks on a doubled backtick. format.go:290 always uses , but GFM closes a code span at the *next equal-length* run — and Spirit escapes identifiers by doubling embedded backticks, so a column named ab `` yields `` DROP COLUMN ab` and the span closes mid-identifier, spilling the rest into raw markdown inside a <details> block. Exotic input, but it is exactly the case the helper was written for; size the fence to one more than the longest run in the content.

General suggestions

6. , is missing from the token class at format.go:282, so the most common parameterised types stay raw-quoted next to code-spanned neighbours: Column \amount` in table `orders` has type "decimal(10,2)"`. Cosmetic — no information is lost, unlike finding 2.
7. The MySQL index-drop guidance at plan.go:1122 is preview-only dead code: Spirit's lint_unsafe.go continues on both index-drop paths, so a real DROP INDEX is reported by invisible_index_before_drop, whose message has no DROP INDEX substring for unsafeDropIndexUsageTargets to count. Pre-existing, but the PR touches the very fixture that keeps it looking alive (preview.go:1062), and apply_test.go:354-466 still asserts the pre-bump unquoted shape.
8. The doc comment at format.go:273-275 still says messages quote identifiers 'idx_category' — no longer true after the bump. Also worth noting in the PR body that go.mod moves golang.org/x/{net,sys,crypto,mod,tools,text} and testify beyond what Spirit itself forces, so the blast radius isn't read as "Spirit only".

The one thing that could have broken, verified

Spirit's messages are the input to schemabot's own text parsing, so adding " around identifiers could have silently broken the unsafe-drop guidance and the finding counts. It didn't: unsafeDropApplicationUsageTarget/unsafeDropIndexUsageTargets use quote-agnostic strings.Count on the raw reason (plan.go:1135, :1169), cleanSingleLintReason strips only [ERROR] -style prefixes, and LintReasons splits on "; " which no new message shape introduces at error severity. Probed each consumer against real post-bump messages — all unchanged.

Verified correct

• TEMPLATES.md is fully regenerated: built the binary, ran scripts/update-templates.sh, diffed against the committed file — byte-identical.
• The quotedSQLFragment keyword alternation exactly covers Spirit's unsafe-message surface (DROP PRIMARY KEY, DROP PARTITION, TRUNCATE PARTITION, DISCARD …, COALESCE PARTITION, plus OriginalText() DROP/TRUNCATE TABLE, DROP DATABASE) — nothing falls outside it.
• Pass ordering is safe: [^"]* cannot cross a ", pass 2's output contains no " so passes 3-4 can't re-match it, and "a DESC", "b" / "a", "b DESC" all render correctly.
• \b correctly rejects "DROPPED" and "CREATE_TIME"; the new happy-path tests, including the padding-space strip, are genuinely correct.
• No double application of CodeQuoteIdentifiers — plan.go:1197 and :1216 are mutually exclusive branches.
• All 38 CI checks green, including E2E MySQL, Vitess, and gRPC suites; go build ./... and go test ./pkg/... pass on the head commit.

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

[aparajon]

🤖 All eight findings addressed in new commits fcc9e97b and be2f9b63:

1. PK-less ALTER refusal (blocking) — pinned in fcc9e97b: TestStatementRefusalContract gains a PK-less table fixture and two cases (ADD COLUMN, MODIFY COLUMN), both asserting altering a table without a primary key is not supported. I probe-verified the verdict against the bumped module before writing the cases: both shapes return refused=true at classification time. The PR body now documents this as an intentional behavior change (refusal at classification instead of a mid-run table-setup failure).
2. Single-quote pass corrupting enum/set types — the singleQuotedIdentifier pass is deleted in be2f9b63. I verified no registered linter emits single-quoted tokens after the bump (spirit#1178 unified the last three '%s' sites to %q), and no in-repo producer exists in pkg/engine/, pkg/tern/, or pkg/ddl/. Older stored reasons degrade to their raw quotes rather than being corrupted. New test: enum('active','archived') renders as one code span.
3. New default-on type_pedantic collation checks — noted in the PR body's new "Behavior changes in this bump" section.
4. Datetime preview fixture and linter name — fixed in be2f9b63: the fixture message now carries the quoted "DATETIME" operands Spirit emits, and the preview's LinterName is corrected to type_pedantic (the real linter name). TEMPLATES.md regenerated from a fresh build — the diff is exactly the two datetime lint lines gaining backticked DATETIME.
5. Doubled-backtick span breakage — codeSpan now sizes its fence to the longest backtick run in the content plus one (be2f9b63). Pinned by a test with a doubled embedded backtick; mutation check: reverting to the fixed double-backtick fence fails that test.
6. decimal(10,2) split by the comma — the quoted-token character classes admit , and ' (be2f9b63); tests cover decimal(10,2) and the enum shape. Mutation check: narrowing the class back fails both tests.
7. Stale unsafe-reason fixtures in apply_test.go — all swept to the post-bump quoted shape in be2f9b63 (the usage-target counting is quote-agnostic, so behavior is unchanged). The DROP INDEX guidance branch being preview-only dead code is real but out of scope for a deps bump — queued as a follow-up task to remove it against main.
8. Stale doc comment and dependency moves — the CodeQuoteIdentifiers doc comment now describes the double-quote-only convention (be2f9b63), and the PR body notes the transitive golang.org/x/{crypto,mod,net,sys,text,tools} + testify moves.

Verification: full make test-unit green (6778 tests), race tests on pkg/ui/pkg/webhook/pkg/webhook/templates green on push, both regex/fence mutants killed with restores verified.

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