feat: track applied migrations as a list instead of a scalar schema_version#71
Draft
effron wants to merge 7 commits into
Draft
feat: track applied migrations as a list instead of a scalar schema_version#71effron wants to merge 7 commits into
effron wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the TestTrack CLI’s schema serialization format to avoid frequent merge conflicts caused by a single scalar schema_version field. It replaces the high-water-mark with a schema_versions list of applied migration versions (sorted by a stable hash), bumps serializer_version to 2, and updates schema loading to use set membership rather than timestamp comparisons.
Changes:
- Bump
SerializerVersionto2and replaceschema_versionwithschema_versions []string. - Update schema load behavior to sync/mark only versions explicitly recorded in
schema_versions, warning once if migrations exist on disk but aren’t recorded. - Add tests for schema-version ordering and serializer-version gating, plus schema loader warning behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
serializers/serializers.go |
Bumps serializer version and changes schema representation to schema_versions with an AddVersion helper. |
schemaloaders/schemaloaders.go |
Switches schema load logic from high-water-mark comparisons to set membership and single warning. |
schemaloaders/schemaloaders_test.go |
Adds coverage ensuring only recorded versions are synced and warning is emitted once. |
schema/schema.go |
Adds forward-version rejection, records all applied versions, and sorts recorded versions by hash for conflict reduction. |
schema/schema_test.go |
Adds tests for ordering-by-hash and rejecting schemas written by newer CLIs, plus generation coverage. |
migrationmanagers/migrationmanagers.go |
Updates schema mutation to record applied versions in the new list format. |
Makefile |
Bumps release version to 2.0.0 to reflect the breaking format change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| break | ||
| if !appliedVersions[version] { | ||
| if !warnedOfUnrecordedMigrations { | ||
| fmt.Println("Schema load complete, but there are migrations on disk not recorded in the schema file - run testtrack schema generate to update it.") |
Comment on lines
163
to
164
| m.schema.AddVersion(*m.migration.MigrationVersion()) | ||
|
|
Comment on lines
+79
to
+83
| var unrecorded []string | ||
| for _, version := range s.migrationRepo.SortedVersions() { | ||
| if !appliedVersions[version] { | ||
| unrecorded = append(unrecorded, version) | ||
| } |
The two-key comparison (Split < Split && Reason < Reason) is not a strict weak ordering, so remote_kills order in the written schema depended on input order — a source of spurious diffs on every schema write. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extract Filenames (dir walk: skip hidden files and directories) and Versions (unique versions from filenames, no content parsing) into migrationloaders, and rebuild Load on Filenames. Anything that needs to agree with the loader about what counts as a migration can now share the walk instead of re-implementing it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Migration files and the schema file are separate formats that happen to share one constant. The schema format bumps to v2 in the next commit while the migration format is unchanged, so give migration files their own constant now — otherwise every new migration would claim a version whose format never changed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The v1 schema recorded applied migrations as a single scalar high-water mark, so every new migration rewrote the same schema_version line and any two branches adding migrations conflicted on it. Replace the scalar with a schema_versions list of every applied version, ordered by SHA-1 of the version so concurrent additions scatter through the file instead of clustering on adjacent lines. Schema load switches from the high-water-mark comparison to set membership, and warns - enumerating the affected versions and both repair paths - when disk migrations aren't recorded in the schema. (The `schema upgrade` command the warning references lands in the next commit.) SyncVersion's schema mutation is dropped rather than ported: both call paths construct the manager with a throwaway schema that is never persisted. BREAKING CHANGE: serializer_version bumps to 2; pre-2.0 CLIs rewrite a v2 schema back into a v1-shaped hybrid. The next commit makes 2.0 detect and repair that. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A 2.0 CLI reading an old or damaged schema must fail loudly, not silently round-trip a lossy read. Read now rejects, with a pointer to the fix: - files written by a newer CLI (upgrade your CLI); - v1 files (serializer_version 1 - run `schema upgrade`); - hybrids left by a pre-2.0 CLI rewriting a v2 schema: those round-trip serializer_version: 2 while writing the v1 shape, so detection is by presence of the legacy schema_version key (a *string - 1.x write paths like `sync` emit schema_version: "", which must still trip the guard); - v2 files whose schema_versions list is empty while testtrack/migrate contains migrations: no machine write produces that state, so the list was lost to a hand-edit or merge resolution, and reading it would ratify the truncated list on the next write. `schema upgrade` converts in place without replaying migrations: it keeps the materialized body (which generate can't rebuild on apps whose history doesn't replay) and rebuilds only the version list, from filenames alone - tolerating unparsable migrations and a missing migrate dir (legacy repos), and clearing the legacy scalar so repaired files pass the guards. ReadMerged deliberately keeps bypassing these guards: linked schemas belong to other apps mid-rollout, and the body fields it consumes are shape-stable across serializer versions - now documented at the function. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The switch is a flag day: a 2.0.0 CLI refuses a v1 schema and a 1.x CLI mangles a v2 schema into a guard-detected hybrid, so the schema conversion and every pinned CLI must move together. Documents what changed, the atomic rollout order, and why `schema upgrade` (not `generate`) is the conversion path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
effron
force-pushed
the
schema-versions-list
branch
from
65e4018 to
b6fbec6
Compare
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.
Every new migration rewrote the single schema_version line to its own timestamp, so any two branches adding migrations conflicted on it. Replace the scalar with a schema_versions list of every applied version, ordered by a hash of each version so concurrently-added migrations scatter through the file instead of clustering on one line.
Bumps serializer_version to 2. schema load now uses set membership against the list, and Read rejects schema files written by a newer CLI. This is a breaking format change and needs a coordinated CLI rollout per consumer.