Remove the --strict flag#910
Merged
Merged
Conversation
Removes the --strict CLI flag and the Migration.Strict field, along with the strict branch in setup(), the WithStrict() test helper, the four TestResumeFromCheckpoint*Strict* tests, and the corresponding sections of docs/migrate.md and resume-from-checkpoint.md. With --strict gone, a failed resume-from-checkpoint always logs the reason and falls back to a fresh migration. The typed status.Err* values (ErrMismatchedAlter, ErrBinlogNotFound, ErrCheckpointTooOld, ErrCheckpointCollision) are still produced and logged internally; only the strict consumer of them is removed. This is a breaking change for anyone passing --strict: Kong will error on the unknown flag. Per prior discussion the flag was already not-recommended. Forked out of block#879. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Removes Spirit’s --strict migration mode end-to-end (CLI surface, Migration config, runner behavior branch, tests, and documentation) so resume-from-checkpoint is always best-effort: on any resume failure, Spirit logs the reason and falls back to starting a fresh migration.
Changes:
- Deleted the
--strictCLI flag andMigration.Strictfield, and removed the strict-only error-return branch inRunner.setup(). - Removed strict-mode test helper and strict-only resume tests; adjusted remaining resume tests/comments to match best-effort semantics.
- Updated docs to remove strict-mode guidance and describe the single fallback behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/migration/runner.go | Removes strict-mode branching in setup and updates resume-related comments. |
| pkg/migration/resume-from-checkpoint.md | Updates checkpoint resume docs to reflect always-best-effort behavior and removes strict-mode section. |
| pkg/migration/resume_test.go | Deletes strict-mode resume tests and updates remaining test comments accordingly. |
| pkg/migration/migration.go | Removes the Strict field / --strict flag definition from the migration config struct. |
| pkg/migration/helpers_test.go | Removes WithStrict() test helper. |
| docs/migrate.md | Removes --strict documentation and updates resume-related text to match new behavior. |
| AGENTS.md | Removes WithStrict() from the documented test runner options list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
aparajon
approved these changes
Jun 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #909.
Summary
Removes the
--strictmigration flag (and theMigration.Strictfield) entirely. This is the strict-removal portion of #879, forked out into its own change so it can land independently of the status-API work in that PR.With
--strictgone, a failed resume-from-checkpoint always logs the reason and falls back to a fresh migration — Spirit's default best-effort behavior. The flag was already documented as not-recommended and had a surprising gap (it does not hard-fail on a checkpoint-table schema mismatch), so it was a footgun rather than a safety net. See #909 for the full rationale.Changes
--strictCLI flag /Migration.Strictfield (pkg/migration/migration.go)setup()and update a stale comment (pkg/migration/runner.go)WithStrict()test helper (pkg/migration/helpers_test.go) and the fourTestResumeFromCheckpoint*Strict*tests (pkg/migration/resume_test.go)--strictdocumentation indocs/migrate.mdandpkg/migration/resume-from-checkpoint.mdWithStrict()reference inAGENTS.mdThe typed
status.Err*resume errors (ErrMismatchedAlter,ErrBinlogNotFound,ErrCheckpointTooOld,ErrCheckpointCollision) are retained — they are still produced and logged internally; only the strict consumer of them is removed.Breaking change
Anyone passing
--strictwill now get a Kong error on the unknown flag.🤖 Generated with Claude Code