feat: track applied migrations as a list instead of a scalar schema_version#71
Draft
effron wants to merge 10 commits into
Draft
feat: track applied migrations as a list instead of a scalar schema_version#71effron wants to merge 10 commits into
effron wants to merge 10 commits into
Conversation
…version 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. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The previous test computed its expected order with hashVersion itself, so it was circular -- a no-op or buggy hashVersion could still pass. Pin the sha1 ordering as a literal so the test fails if the ordering changes or regresses to a lexical/timestamp sort. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The old 'migrations newer than the schema file' wording came from the scalar high-water-mark model, where any unrecorded migration was strictly newer. With the schema_versions set-membership check it's just 'on disk but not recorded in the schema file' -- not necessarily newer, and not the same as unapplied (this loop never checks server-applied state). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Asserts that load marks only schema-recorded versions applied on the server, skips migration files on disk that aren't recorded, and prints the warning exactly once regardless of how many are unrecorded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Comment on lines
+46
to
+51
| if schema.SerializerVersion > serializers.SerializerVersion { | ||
| return nil, fmt.Errorf( | ||
| "%s was written by a newer testtrack CLI (serializer_version %d, this CLI supports %d). Please upgrade your testtrack CLI", | ||
| schemaPath, schema.SerializerVersion, serializers.SerializerVersion, | ||
| ) | ||
| } |
Comment on lines
+228
to
+230
| sort.Slice(schema.SchemaVersions, func(i, j int) bool { | ||
| return hashVersion(schema.SchemaVersions[i]) < hashVersion(schema.SchemaVersions[j]) | ||
| }) |
|
|
||
| // SerializerVersion is the current version of the migration file format so we can evolve over time | ||
| const SerializerVersion = 1 | ||
| // SerializerVersion is the current version of the migration file format so we can evolve over time. |
Comment on lines
+33
to
+37
| func TestReadRejectsNewerSerializerVersion(t *testing.T) { | ||
| withSchemaFile(t, ` | ||
| serializer_version: 99 | ||
| schema_versions: | ||
| - "2020011712345" |
- Read() now rejects an older-format schema (serializer_version below current) and points at `testtrack schema generate`, instead of silently round-tripping a v1 file into a lossy v2 write that drops migration history. generate doesn't read the file, so it remains the upgrade path. - Schema load warning now points at `testtrack schema generate` (which updates the file) rather than `testtrack migrate` (which never persists schema_versions, so the warning would recur). - Split version hash-ordering out of SortAlphabetically into sortSchemaVersions so the function name stays honest, and precompute each version's hash once (comparing raw sha1 bytes) instead of recomputing on every comparison. - Update fakeserver fixtures to the v2 schema_versions format. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review caught two inaccuracies: the upgrade sequence (bump CI first, then commit schema) would break testtrack migrate in CI in the interim window, so the CI pin and the v2 schema commit must land together; and the list of commands that error on a v1 schema was incomplete (decide and destroy also go through Read). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`schema generate` rebuilds the schema by replaying every migration, which fails on long-lived apps whose testtrack/migrate predates some splits or references splits created out of band (no create migration to replay). That made the documented v1->v2 upgrade path a dead end for real repos. `schema upgrade` converts in place instead: it keeps the already-materialized body and only rebuilds schema_versions from the migration filenames on disk, then restamps serializer_version. It reads the old file directly, bypassing the read-time version guard. Point the guard's error and UPGRADING.md at it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.