docs(mysql): add mysql.md and link guides from plans

[aparajon]

Why

Primary key lint findings need a clear path to understanding the tradeoffs. MySQL users also need practical progress guidance without reading engine internals.

What

• Add docs/mysql.md for primary key choices, why index adds copy the table, and progress displays; move implementation details into docs/architecture.md
• Explain automatic write-thread scaling and capacity planning in the throttle guide
• Illustrate instant, native in-place, and table-copy routing with DDL examples; give lifecycle phases more reading time
• Show how SchemaBot coordinates Spirit checkpoint recovery and document resume requirements
• Add a deduplicated Related guidance list to plan comments, starting with primary key and column rename guides
• Link a guide only from a comment that shows the finding behind it, and carry the lint fold and the guides onto the comment that blocks an apply for unsafe changes

How

Lint rule	Related guidance
primary_key	Choosing a primary key
rename_column	Renaming a column or table

Map lint rule IDs to guides in a shared registry. Collect findings across severities and environments, then show each guide once outside collapsed findings. Preserve rule IDs in rollback plans too.

Issues + Lint Warnings (all environments)
                   |
           Match rules to guides
                   |
            One link per guide

Risk

Low risk: changes affect documentation, comment rendering, and the GIF renderer. Schema execution, lint severity, and approval requirements are unchanged.

Testing

Inspected the workflow GIFs at copy, interruption, recovery, and cutover boundaries. Regenerated the plan previews below. Smoke-tested the renderer's Chrome launch options with default channel discovery and an explicit CHROME path on macOS; both passed.

Choosing an execution path

Table-copy lifecycle

SchemaBot orchestration and Spirit checkpoint recovery

Primary key animation

Existing varchar primary key: advisory warning

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` ADD INDEX `idx_created_at`(`created_at`);

💡 Lint Warnings: 1 advisory finding

• customers: Primary key column id has type varchar

📖 Related guidance:

• Choosing a primary key

📋 Plan: 1 table to alter

---

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

schemabot apply -e staging

New varchar primary key: issue requiring acknowledgement

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 `customers` (
    `id` varchar(64) NOT NULL,
    `created_at` datetime(3) NOT NULL,
    PRIMARY KEY(`id`)
) ENGINE InnoDB,
  CHARSET utf8mb4,
  COLLATE utf8mb4_0900_ai_ci;

⚠️ Issues: 1 unsafe change detected

1. customers: Primary key column id has type varchar

📖 Related guidance:

• Choosing a primary key

📋 Plan: 1 table to create

---

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

schemabot apply -e staging

Aggregated guidance: six warnings and one issue, two guides

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 `users` RENAME COLUMN `email` TO `email_address`;

ALTER TABLE `customers` ADD INDEX `idx_created_at`(`created_at`);

ALTER TABLE `orders` ADD INDEX `idx_created_at`(`created_at`);

ALTER TABLE `invoices` ADD INDEX `idx_created_at`(`created_at`);

ALTER TABLE `shipments` ADD INDEX `idx_created_at`(`created_at`);

ALTER TABLE `sessions` ADD INDEX `idx_created_at`(`created_at`);

ALTER TABLE `events` ADD INDEX `idx_created_at`(`created_at`);

⚠️ Issues: 1 unsafe change detected

1. users: Column rename detected in table users: email to email_address. Renaming a column cannot be done atomically across application pods, and ORMs that generate column names at compile time (e.g. jOOQ) will break until code is recompiled

💡 Lint Warnings: 6 advisory findings

customers

• Primary key column id has type varchar

orders

• Primary key column id has type varchar

invoices

• Primary key column id has type varchar

shipments

• Primary key column id has type varchar

sessions

• Primary key column id has type varchar

events

• Primary key column id has type varchar

📖 Related guidance:

• Choosing a primary key
• Renaming a column or table

📋 Plan: 7 tables to alter

---

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

schemabot apply -e staging

Apply blocked for unsafe changes: the findings and their guides, above the flag

📋 Plan: 2 tables to alter

💡 Lint Warnings: 1 advisory finding

• orders: Column created_at uses TIMESTAMP which overflows on 2038-01-19. Consider using DATETIME instead.

📖 Related guidance:

• Choosing a primary key
• Renaming a column or table

---

⛔ Apply rejected: 3 unsafe changes detected

1. orders: Primary key column id has type int
2. orders: Column created_at uses TIMESTAMP which overflows on 2038-01-19. Consider using DATETIME instead.
3. users: Column rename detected in table users: email to email_address. Renaming a column cannot be done atomically across application pods, and ORMs that generate column names at compile time (e.g. jOOQ) will break until code is recompiled

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

schemabot apply -e staging --allow-unsafe

A rollback comment renders the lint fold and no unsafe section, so its error-severity findings go unshown there and their guides stay unlinked rather than arriving as a link with nothing above it naming the rule.

Bigger picture

Keep engine guides useful for everyday work, with architecture deep dives and upstream references one click away. Additional lint rules can share guides without repeating links.

Generated with Codex

[Copilot]

🟡 Changes recommended

Two moderate findings block approval, with three documentation nits outstanding.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a consolidated MySQL/Spirit guide covering primary-key tradeoffs and progress behavior, with supporting animation assets and plan-comment links.

Changes:

• Adds Spirit documentation, animation sources, and cross-references.
• Links primary-key findings to guidance in plan comments.
• Consolidates progress documentation and updates tests/templates.

Review findings:

• Moderate (3 votes): Rollback comments do not populate HasPrimaryKeyFindings.
• Moderate (3 votes): The animation renderer uses a macOS-only Chrome path by default.
• Nits: Correct the Spirit URL, progress label, and canonical collation example.

File summaries

File	Summary
TEMPLATES.md	Updates rendered plan guidance.
scripts/render-spirit-primary-keys.cjs	Generates the primary-key animation.
README.md	Links to the Spirit guide.
pkg/webhook/templates/rollback.go	Adds rollback guidance rendering.
pkg/webhook/templates/preview.go	Updates preview lint metadata.
pkg/webhook/templates/plan.go	Renders primary-key guidance.
pkg/webhook/templates/lint_test.go	Tests guidance visibility and folding.
pkg/webhook/plan.go	Detects primary-key findings.
pkg/webhook/plan_test.go	Tests finding detection and error handling.
docs/spirit.md	Adds the consolidated Spirit guide.
docs/spirit_progress.md	Redirects to the consolidated guide.
docs/lint-and-safety-levels.md	Links primary-key guidance.
docs/engines.md	Updates Spirit documentation links.
docs/architecture.md	Updates the progress documentation reference.
docs/.toc-manifest	Registers the consolidated guide.
assets/src/spirit-primary-keys.html	Defines the animation source.
assets/src/README-spirit-primary-keys.md	Documents animation generation.

Review details

Suppressed comments (3)

docs/engines.md:24

• The Built on column now links to SchemaBot's guide rather than the Spirit dependency; unlike the other rows, it no longer takes the reader to the engine this column names. Keep the upstream Spirit URL here and link the new guide separately.

| Spirit | MySQL | [Spirit](spirit.md) |

docs/spirit.md:434

• The estimate-exceeded renderer does not label rows Active: the standard CLI/TUI path defaults to Finalizing copy (pkg/cmd/internal/templates/progress.go:247), and the PR renderer uses the same label. This example therefore does not match the user-visible output; change the label to Finalizing copy.
  docs/spirit.md:54
• This copyable CREATE TABLE example uses utf8mb4_0900_bin, but the repository's canonical Spirit-compatible SHOW CREATE TABLE format uses utf8mb4_0900_ai_ci (as required by the schema conventions and the other SQL examples). Keep the guide's example in that canonical format.

• Files reviewed: 17/18 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 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/1376, ed540df.
Verdict: 3 findings — 2 non-blocking (guidance link surfaces), 1 suggestion.

Non-blocking

Rollback comment can show a "Related guidance" link with no matching finding text. rollback.go:43 calls writeRelatedGuidance unconditionally, but LintRuleNames is filled from all severities (rollback.go:346) while LintViolations comes from LintNonErrors() and the template has no unsafe/Issues section. An error-severity primary_key finding (rollback re-declares an old varchar PK) therefore renders a bare "📖 Related guidance: Choosing a primary key" with nothing on the comment mentioning a primary key — cosmetic, and TestRollbackPlanCommentRelatedGuidance/error currently pins that behaviour rather than catching it.

The guidance sweep missed RenderUnsafeChangesBlocked. apply_commands.go:184 builds its comment from a PlanCommentData that already carries LintRuleNames, but writeRelatedGuidance has only three call sites (plan.go:366, plan.go:1609, rollback.go:43). So a new-table varchar PK blocked at apply time — the exact moment the operator decides whether to pass --allow-unsafe — never links the new primary-key guide; same for the confirm path at apply_execute.go:228. Mitigated only by the plan comment on the same PR carrying it.

General suggestions

The LintViolations match arm is unreachable in production. lint_guidance.go:37 scans LintViolations[].LinterName, but both builders populate LintRuleNames from every LintResults entry, a strict superset of the LintNonErrors()-derived violations. Only hand-built preview/test data reaches it, so it is dead defensive code that must be kept in sync as guides are added — consider dropping it or covering it deliberately.

The one thing that could have broken, verified

Guide ordering could have varied with environment or finding order, making the comment non-deterministic across re-plans. It does not: writeRelatedGuidance iterates lintGuides in the outer loop and plans in the inner one, deduping on seen[guide.url] — confirmed by the round-trip assertion in lint_guidance_test.go and by TEMPLATES.md output emitting primary_key before rename_column even though rename_column came first in the input data.

Verified correct

• primary_key and rename_column are the real Spirit linter names (lint_primary_key_type.go:38, lint_rename_column.go:35), not invented IDs.
• Both guide URLs resolve: "Choosing a primary key" and "Renaming a column or table"; every md link+anchor in the touched docs checks out.
• Linter survives every conversion into PlanResponse.LintResults (proto path proto_helpers.go:207, pull path plan_handlers.go:418), so LintRuleNames is populated in production.
• The multi-env filter excludes errored environments (data.Errors[env] == "", plan.go:1605) and nil plans, so a failed env cannot contribute a stale link.
• The "no changes" short-circuits (plan.go:298, plan.go:1707) return before guidance and cannot hide a finding — spirit.go:557 returns NoChanges before any lint violation is collected.
• IsLocked suppresses guidance both at the call site (plan.go:365) and inside writeRelatedGuidance, matching the existing rule for lint findings.
• Nil-safety holds: both collection loops guard finding != nil && finding.Linter != "", and indexing a possibly-nil data.Errors map is legal Go.

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

[Kiran01bm]

🤖 Approved on Kiran's (@kmuddukrishna) behalf by the scheduled review agent — no blocking findings at ed540df. See the review comment above; non-blocking findings and suggestions, if any, are not merge gates.

[Kiran01bm]

🤖 Review findings - created by Kiran's code review agent - for schemabot/pull/1376, 52bf243.

Re-review of the delta only: ed540dfd..52bf2436 (the one commit added since the previous review). Findings from that earlier review are not repeated here.

Verdict: 2 findings — 0 blocking, 2 non-blocking (stale doc comment + dead write on the rollback path).

Non-blocking

The buildRollbackPlanCommentData doc comment now states the opposite of what the code does.
rollback.go:329 still says it "preserves lint rule IDs from every severity for related guidance", but the only consumer, RenderRollbackPlanComment, now calls writeRelatedGuidance(&sb, data.disclosesNonErrorsOnly()) (templates/rollback.go:44), whose scope is built solely from LintViolations (lint_guidance.go:47-52). The comment was true at 52bf2436^; a maintainer registering a guide for an error-severity rule would now expect a rollback link that can never appear. Rendering is correct by design — this is a documentation fix.

LintRuleNames is still populated on the rollback path but never read.
rollback.go:348 appends into commentData.LintRuleNames, whose sole production reader is disclosesEverySeverity (lint_guidance.go:42) — reachable from RenderPlanComment / multi-plan / RenderUnsafeChangesBlocked, never from the rollback renderer. rollback_test.go:39 still asserts the dead value []string{"primary_key","primary_key"}, which keeps the write looking load-bearing. Drop the write and the assertion together with the comment fix above.

The one thing that could have broken, verified

Removing the IsLocked guard from writeRelatedGuidance — previously if !data.IsLocked { … }, now unconditional. It is safe on all three surfaces: for plan comments disclosesEverySeverity returns a zero-value guidanceScope (nil rules) when IsLocked, so the loop matches nothing; IsLocked is only ever set in postAutoConfirmDowngrade (apply_execute.go:387) and apply_handlers.go:391, both of which render RenderPlanComment, so no locked data reaches RenderUnsafeChangesBlocked; and the rollback path builds its own PlanCommentData that never sets IsLocked while writing its lint fold unconditionally, so guidance stays aligned with what is displayed.

Verified correct

• RenderPlanComment refactor is behaviour-preserving: the old guarded call and the new unconditional one render identically.
• Dropping the LintViolations arm from the match loses no production link — LintRuleNames is a superset built from every planResp.LintResults entry with a non-empty Linter (plan.go:1063-1076).
• Dedup and ordering unchanged: guide-registry-outer / scope-inner with a per-URL seen map, so display order stays registry order and each URL appears once.
• TestRelatedGuidanceAggregatesRulesAcrossEnvironments still proves forward and reverse environment order render identically.
• mysqlOnly guides remain gated per scope (guide.mysqlOnly && !scope.isMySQL), so Vitess/PostgreSQL plans get no MySQL-only links, including on the new apply-blocked surface.
• TEMPLATES.md drift is complete: only PreviewCommentLintErrorsBlocked changes output; the other RenderUnsafeChangesBlocked previews carry no lint data.
• The multi-env lint preview's rules (has_timestamp, redundant_indexes) have no registered guide, so its golden is correctly untouched.
• go test ./pkg/webhook/templates/ and go test ./pkg/webhook/ both pass at 52bf243.

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

[Kiran01bm]

🤖 Approved on Kiran's (@kmuddukrishna) behalf by the scheduled review agent — no blocking findings at 52bf243. See the review comment above; non-blocking findings and suggestions, if any, are not merge gates.

[Kiran01bm]

🤖 Review findings - created by Kiran's code review agent - for schemabot/pull/1376, de858ca.

Re-review of the delta only: 52bf2436..de858ca5, this PR's one new commit since the previous review. It addresses the two non-blocking findings raised there (stale buildRollbackPlanCommentData doc comment, dead LintRuleNames write on the rollback path); findings from that earlier review are not repeated here.

Verdict: clean — approve.

The one thing that could have broken, verified

Dropping the LintRuleNames population in buildRollbackPlanCommentData could have silently stripped the "related guidance" links from rollback comments. It doesn't: the rollback renderer calls writeRelatedGuidance(&sb, data.disclosesNonErrorsOnly()) at templates/rollback.go:44, and disclosesNonErrorsOnly derives its rule set from LintViolations, never LintRuleNames. A grep over pkg/ confirms LintRuleNames is read in exactly one place — disclosesEverySeverity — which the rollback path never reaches.

Verified correct

• The removed loop was a dead store: buildRollbackPlanCommentData has one caller (rollback.go:323) and its result feeds only RenderRollbackPlanComment.
• plan.go:1063 still populates LintRuleNames for plan/apply comments, which do render error-severity findings, so the every-severity scope stays correct where the field is still used.
• The deleted finding != nil guard adds no nil-panic risk: LintWarnings/LintInfos (apitypes.go:803) already dereference w.Severity unguarded, so a nil element panics identically before and after.
• rollback_test.go still renders the comment and asserts the primary-key guide appears once for warning severity and not at all for error — coverage of the shown-finding/linked-guide invariant is unchanged.
• Both assert and require remain used after the assertion removal, so no unused-import breakage.
• The test deletion matches AGENTS.md's "do not add negative regression tests for removed behavior": it drops an assertion over a now-unpopulated field, not over live behaviour.
• The new doc comment on buildRollbackPlanCommentData states intent (why error findings are withheld) rather than history, so it clears the "no 'what changed' comments" rule.
• The LintRuleNames field comment at templates/plan.go:114 stays accurate — plan.go is now its only producer and it does collect every severity.

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

[Kiran01bm]

🤖 Approved on Kiran's (@kmuddukrishna) behalf by the scheduled review agent — no blocking findings at de858ca. See the review comment above; non-blocking findings and suggestions, if any, are not merge gates.

[aparajon]

🤖 Both addressed in de858ca5, and they were two halves of the same leftover.

The doc comment did state the opposite of the code. buildRollbackPlanCommentData promised it preserved rule IDs "from every severity for related guidance" — true at 52bf2436^, and exactly the wrong thing to leave in front of someone registering a guide for an error-severity rule. It now says why error-severity findings are left out: the rollback comment has no unsafe section to show them in, and a guide linked for a finding the comment never shows is advice about nothing.

The LintRuleNames write went with it, along with the rollback_test.go assertion on the dead value — you're right that keeping the assertion is what made the write look load-bearing.

Thanks for walking the IsLocked guard removal; that was the change I was least sure read as obviously safe from the diff alone.

Replied by Claude Code (claude-opus-5).

[Kiran01bm]

🤖 Review findings - created by Kiran's code review agent - for schemabot/pull/1376, 33092bf.

Re-review of the delta only: de858ca5..33092bf9 (the one docs commit added since the previous review). Findings from that earlier review are not repeated here.

Verdict: 1 finding — 1 non-blocking (engine-gated guidance link).

Non-blocking

The rename_column lint guide is gated mysqlOnly: true, so Vitess/Strata plans never link the (engine-agnostic) rename guidance. lint_guidance.go:22 registers the guide MySQL-only, but plan.go:173 copies the same Spirit rename_column violation through for Vitess, so the Issue renders while lint_guidance.go:63 drops the link because IsMySQL: schema.Type == "mysql" (plan.go:908) is false. Rename users.email → users.email_address on a type: vitess database and the reader gets a bare unsafe-change Issue, while the byte-identical MySQL plan gets the doc link — and the linked section is entirely engine-agnostic, unlike the primary-key guide whose content is Spirit-chunker specific.

The one thing that could have broken, verified

The Active → Finalizing copy doc rename could have documented a label the code never emits. It does not: progress.go:247 and :441, apply.go:1163, sharded_apply.go:451 and watch_tui_view.go:314 already ship Finalizing copy, so the docs were the stale side and no code change was needed — consistent with green CI.

Verified correct

• Sweep is complete: no renderer or template still emits the old Active copy label; the only Active left in TEMPLATES.md (line 2549) is the unrelated 🔒 Active Locks header.
• No doc carries the stale wording: remaining Active hits are cli.md:72 (profile), github-app-setup.md:40 (GitHub App field) and apply-lifecycle.md:94-98 (apply states).
• mysql.md:157-160 matches formatEstimateExceededTable (progress.go:830) and the tooltip literal in progressbar.go:19 byte-for-byte.
• engines.md:24 repointing Spirit's "Built on" cell at block/spirit makes the column uniform and does not orphan mysql.md (still linked from README.md:117, architecture.md:958, lint-and-safety-levels.md:67, .toc-manifest:18).
• No TOC/manifest drift: the delta adds no headings, so the generated TOC blocks and docs/.toc-manifest stay valid.
• No "migration" or hyphenated "schema-change" text on changed lines, so make check-terminology is unaffected.
• The collation edit brings mysql.md:54 in line with the canonical CREATE TABLE form used in every other doc example.

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

[Kiran01bm]

🤖 Approved on Kiran's (@kmuddukrishna) behalf by the scheduled review agent — no blocking findings at 33092bf. See the review comment above; non-blocking findings and suggestions, if any, are not merge gates.

[Kiran01bm]

🤖 Review findings - created by Kiran's code review agent - for schemabot/pull/1376, 15906af.

Verdict: 2 findings — both general suggestions (CLI sweep gap, renderer error handling).

General suggestions

The CLI plan output was left out of the guidance sweep. pkg/cmd/internal/templates/plan.go#L434 renders warnings from Table/Message only and drops w.Linter, because the lintGuides registry is unexported in pkg/webhook/templates. Running schemabot plan -e staging against a varchar primary key prints • customers: Primary key column "id" has type "varchar" with no link, while the PR comment for the same plan now links docs/mysql.md#choosing-a-primary-key. Exporting the registry (or a GuidanceFor(linter) helper) would let the CLI surface match.

The pageerror listener rethrows from inside a Playwright callback, escaping the try/finally. scripts/render-spirit-workflows.cjs#L17 throws from an event handler dispatched via setImmediate, so node dies on an uncaught exception without running finally { await browser.close(); fs.rmSync(framesDir, …) } — contradicting the new README's "Temporary frames are cleaned up on exit". Impact is small in practice (processLauncher still SIGKILLs Chrome, and the leaked tmpdir is empty), but the sibling render-spirit-primary-keys.cjs#L11 uses console.error for the same event, so the two renderers diverge on the same failure.

The one thing that could have broken, verified

disclosesEverySeverity asserts that error-severity findings are always shown, which is what lets the guidance block skip them. Traced on MySQL: every error-severity Spirit violation sets change.IsUnsafe + UnsafeReason at pkg/engine/spirit/spirit.go#L586, which becomes data.UnsafeChanges/HasUnsafeChanges in buildPlanCommentData — so the claim holds at every call site. The one place it could have leaked is the locked-apply path, and there RenderPlanComment suppresses both the unsafe section and the lint fold while disclosesEverySeverity returns an empty scope, so no guide is linked (matching lint_test.go's "locked apply" case).

Verified correct

• writeRelatedGuidance iterates lintGuides outer / scopes inner and dedups on guide.url, so link order is registry order, independent of environment and finding order.
• RenderUnsafeChangesBlocked renders its unsafe list unconditionally, so disclosesEverySeverity is the right scope there; the new writeLintViolations shows only non-error findings and cannot duplicate it.
• RenderRollbackPlanComment genuinely has no unsafe section, so disclosesNonErrorsOnly is correct; both LintViolationData construction sites (plan.go:1071, rollback.go:349) now carry LinterName.
• buildRollbackPlanCommentData is a faithful extraction — keyspace loop, lint loop, and commentData.Errors unchanged, with the post/return moved above the new function.
• Every surface calling writeLintViolations (apply_commands.go, plan.go x2, rollback.go) also calls writeRelatedGuidance — no lint-showing comment lost the hook.
• Both registered rule IDs match the real Spirit linter names (primary_key, rename_column), and the preview fixture's stale pk_type was corrected.

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

[Kiran01bm]

🤖 Approved on Kiran's (@kmuddukrishna) behalf by the scheduled review agent — no blocking findings at 15906af. See the review comment above; non-blocking findings and suggestions, if any, are not merge gates.

[morgo]

🤖 Approving on Morgan's behalf (agent review).

Reviewed the whole PR at bdea57a0, not just the delta since @Kiran01bm's approval (I had not reviewed this one before). The delta since 15906af9 is docs/mysql.md +5/-2: narrowing the native-DDL claim to "for a single-table ALTER, Spirit first tries instant DDL, then its supported subset of native in-place operations", and adding the talk link. Both accurate.

The invariant this PR rests on holds. The claim is "link a lint guide only where the finding is shown". Traced it rather than taking it on faith:

• rename_column is Error severity in block/spirit (pkg/lint/lint_rename_column.go); primary_key is warning. So the registry exercises both severity paths.
• pkg/engine/spirit/spirit.go:589 sets change.IsUnsafe = true on the same change the error-severity violations were attached to, so an error-severity finding always materialises as an UnsafeChange — it cannot be in LintRuleNames without the unsafe section showing it.
• RenderPlanComment gates both the unsafe section and the lint fold on !IsLocked, and disclosesEverySeverity() returns an empty scope on IsLocked. Consistent.
• writeEnvironmentPlanSection renders both sections unconditionally, and buildPlanCommentData never sets IsLocked, so the multi-env and unsafe-blocked paths are consistent too.
• Rollback correctly uses disclosesNonErrorsOnly(): that comment has no unsafe section, so rename_column gets no link there.
• The totalChanges == 0 early returns can't strand a link — lint findings attach to planned changes, so zero changes implies zero findings.

Docs verified, not assumed:

• docs/mysql.md:90 ## Choosing a primary key and docs/pre-merge-workflow.md:304 ### Renaming a column or table both exist, and slugify to the anchors the guides point at.
• https://www.youtube.com/watch?v=-d-NOzKZxdI resolves to "MySQL Belgian Days 2024 - Introducing Spirit by Morgan Tocker". Right video.
• The docs/spirit_progress.md deletion (-447) is fully accounted for, not lost: all 11 architecture headings relocated under #### Spirit progress architecture in docs/architecture.md (demoted one level), and the TUI rendering reference relocated into docs/mysql.md. docs/.toc-manifest swaps the two correctly and there are no dangling references to the old path anywhere at this head.

CI at bdea57a0: 41/41 SUCCESS, no failures and no cancelled-run phantoms.

Two nits, neither blocking:

1. pkg/webhook/templates/plan.go:1603 — the new guidance loop tests data.Errors[env] == "" (value emptiness) while the render branch immediately above at :1585 tests errMsg, hasErr := data.Errors[env] (key presence), as does the pre-existing :1792. They diverge only if an empty-string error is ever stored; all three writers go through userFacingError(err) on a non-nil error, so it's unreachable today. Matching the existing predicate would keep the two in lockstep by construction.

2. The IsLocked short-circuit lives inside disclosesEverySeverity(), but RenderUnsafeChangesBlocked calls writeLintViolations with no IsLocked gate of its own. Inert now (buildPlanCommentData never sets IsLocked), but a future caller that did would render findings with the guidance silently dropped — the one direction the registry's design is trying to rule out.

One observation, no action needed: the guide URLs are absolute against blob/main, so they only resolve once this merges. Expected for the merge-to-main flow; just means a build deployed from this branch would 404 those links.

No github.com/squareup/ references in any added line — public-repo separation clean.