From 0ac9f168e6785daccee18ac9874379a4f8eb2471 Mon Sep 17 00:00:00 2001 From: Harris Effron Date: Tue, 14 Jul 2026 15:17:40 -0400 Subject: [PATCH 1/7] Fix SortAlphabetically's remote-kill comparator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- schema/schema.go | 6 ++++-- schema/schema_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 schema/schema_test.go diff --git a/schema/schema.go b/schema/schema.go index 9916da8..175547d 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -213,8 +213,10 @@ func applyAllMigrationsToSchema(schema *serializers.Schema) error { // SortAlphabetically sorts the schema's resource slices by their natural keys func SortAlphabetically(schema *serializers.Schema) { sort.Slice(schema.RemoteKills, func(i, j int) bool { - return schema.RemoteKills[i].Split < schema.RemoteKills[j].Split && - schema.RemoteKills[i].Reason < schema.RemoteKills[j].Reason + if schema.RemoteKills[i].Split != schema.RemoteKills[j].Split { + return schema.RemoteKills[i].Split < schema.RemoteKills[j].Split + } + return schema.RemoteKills[i].Reason < schema.RemoteKills[j].Reason }) sort.Slice(schema.FeatureCompletions, func(i, j int) bool { return schema.FeatureCompletions[i].FeatureGate < schema.FeatureCompletions[j].FeatureGate diff --git a/schema/schema_test.go b/schema/schema_test.go new file mode 100644 index 0000000..65a2c65 --- /dev/null +++ b/schema/schema_test.go @@ -0,0 +1,28 @@ +package schema + +import ( + "testing" + + "github.com/Betterment/testtrack-cli/serializers" + "github.com/stretchr/testify/require" +) + +func TestSortAlphabeticallyRemoteKillsIsDeterministic(t *testing.T) { + // These three kills are mutually incomparable under the old buggy + // comparator (Split < Split && Reason < Reason), which made the written + // order depend on input order. + kills := []serializers.RemoteKill{ + {Split: "a_split", Reason: "z_reason"}, + {Split: "b_split", Reason: "m_reason"}, + {Split: "c_split", Reason: "a_reason"}, + } + forward := &serializers.Schema{RemoteKills: []serializers.RemoteKill{kills[0], kills[1], kills[2]}} + reversed := &serializers.Schema{RemoteKills: []serializers.RemoteKill{kills[2], kills[1], kills[0]}} + + SortAlphabetically(forward) + SortAlphabetically(reversed) + + require.Equal(t, forward.RemoteKills, reversed.RemoteKills, + "remote kills must sort to the same order regardless of input order") + require.Equal(t, []serializers.RemoteKill{kills[0], kills[1], kills[2]}, forward.RemoteKills) +} From 41193e097cb2d3000d1a3dae8de6b988809ad358 Mon Sep 17 00:00:00 2001 From: Harris Effron Date: Tue, 14 Jul 2026 15:17:42 -0400 Subject: [PATCH 2/7] Give migration-dir walking a single source of truth 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 --- migrationloaders/migrationloaders.go | 58 +++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/migrationloaders/migrationloaders.go b/migrationloaders/migrationloaders.go index 8310ec5..033b474 100644 --- a/migrationloaders/migrationloaders.go +++ b/migrationloaders/migrationloaders.go @@ -17,25 +17,65 @@ import ( "gopkg.in/yaml.v2" ) -// Load loads a set of migrations -func Load() (migrations.Repository, error) { +// Filenames lists the migration filenames in testtrack/migrate, skipping +// hidden files and directories. It is the single source of truth for which +// directory entries count as migrations — Load and `schema upgrade` must +// agree on that, or the schema's recorded versions drift from what the +// loader sees. +func Filenames() ([]string, error) { files, err := os.ReadDir("testtrack/migrate") if err != nil { return nil, err } - - migrationRepo := make(migrations.Repository) + filenames := make([]string, 0, len(files)) for _, file := range files { - if strings.HasPrefix(file.Name(), ".") { - continue // Skip hidden files + if file.IsDir() || strings.HasPrefix(file.Name(), ".") { + continue + } + filenames = append(filenames, file.Name()) + } + return filenames, nil +} + +// Versions returns the unique migration versions recorded in the filenames in +// testtrack/migrate, without reading file contents — usable even on repos +// whose migrations can't be parsed or replayed. +func Versions() ([]string, error) { + filenames, err := Filenames() + if err != nil { + return nil, err + } + seen := make(map[string]bool, len(filenames)) + versions := make([]string, 0, len(filenames)) + for _, filename := range filenames { + version, err := migrations.ExtractVersionFromFilename(filename) + if err != nil { + return nil, fmt.Errorf("%w - delete or rename it if it isn't a testtrack migration", err) + } + if seen[version] { + continue } + seen[version] = true + versions = append(versions, version) + } + return versions, nil +} - migrationVersion, err := migrations.ExtractVersionFromFilename(file.Name()) +// Load loads a set of migrations +func Load() (migrations.Repository, error) { + filenames, err := Filenames() + if err != nil { + return nil, err + } + + migrationRepo := make(migrations.Repository) + for _, filename := range filenames { + migrationVersion, err := migrations.ExtractVersionFromFilename(filename) if err != nil { return nil, err } - fileBytes, err := os.ReadFile(path.Join("testtrack/migrate", file.Name())) + fileBytes, err := os.ReadFile(path.Join("testtrack/migrate", filename)) if err != nil { return nil, err } @@ -62,7 +102,7 @@ func Load() (migrations.Repository, error) { } else if migrationFile.IdentifierType != nil { migrationRepo[migrationVersion] = identifiertypes.FromFile(&migrationVersion, migrationFile.IdentifierType) } else { - return nil, fmt.Errorf("testtrack/migrate/%s didn't match a known migration type", file.Name()) + return nil, fmt.Errorf("testtrack/migrate/%s didn't match a known migration type", filename) } } return migrationRepo, nil From 327c1796d5d2401b8bc5043935fb85e196c306cd Mon Sep 17 00:00:00 2001 From: Harris Effron Date: Tue, 14 Jul 2026 15:18:12 -0400 Subject: [PATCH 3/7] Version migration files independently of the schema file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- featurecompletions/featurecompletions.go | 2 +- identifiertypes/identifiertypes.go | 2 +- remotekills/remotekills.go | 2 +- serializers/serializers.go | 9 ++++++++- splitdecisions/splitdecisions.go | 2 +- splitretirements/splitretirements.go | 2 +- splits/splits.go | 2 +- 7 files changed, 14 insertions(+), 7 deletions(-) diff --git a/featurecompletions/featurecompletions.go b/featurecompletions/featurecompletions.go index 6131059..ae5d01f 100644 --- a/featurecompletions/featurecompletions.go +++ b/featurecompletions/featurecompletions.go @@ -67,7 +67,7 @@ func (f *FeatureCompletion) Filename() *string { // File returns a serializable MigrationFile for this migration func (f *FeatureCompletion) File() *serializers.MigrationFile { return &serializers.MigrationFile{ - SerializerVersion: serializers.SerializerVersion, + SerializerVersion: serializers.MigrationSerializerVersion, FeatureCompletion: f.serializable(), } } diff --git a/identifiertypes/identifiertypes.go b/identifiertypes/identifiertypes.go index 106398a..bee0190 100644 --- a/identifiertypes/identifiertypes.go +++ b/identifiertypes/identifiertypes.go @@ -54,7 +54,7 @@ func (i *IdentifierType) Filename() *string { // File returns a serializable MigrationFile for this migration func (i *IdentifierType) File() *serializers.MigrationFile { return &serializers.MigrationFile{ - SerializerVersion: serializers.SerializerVersion, + SerializerVersion: serializers.MigrationSerializerVersion, IdentifierType: i.serializable(), } } diff --git a/remotekills/remotekills.go b/remotekills/remotekills.go index d921b0e..e7ddf29 100644 --- a/remotekills/remotekills.go +++ b/remotekills/remotekills.go @@ -91,7 +91,7 @@ func (r *RemoteKill) Filename() *string { // File returns a serializable MigrationFile for this migration func (r *RemoteKill) File() *serializers.MigrationFile { return &serializers.MigrationFile{ - SerializerVersion: serializers.SerializerVersion, + SerializerVersion: serializers.MigrationSerializerVersion, RemoteKill: r.serializable(), } } diff --git a/serializers/serializers.go b/serializers/serializers.go index 34f06ae..951e1cb 100644 --- a/serializers/serializers.go +++ b/serializers/serializers.go @@ -4,9 +4,16 @@ import ( "gopkg.in/yaml.v2" ) -// SerializerVersion is the current version of the migration file format so we can evolve over time +// SerializerVersion is the current version of the schema file format so we +// can evolve it over time. const SerializerVersion = 1 +// MigrationSerializerVersion is the current version of the migration file +// format. It is versioned independently of the schema file: the schema format +// changed in v2 but migration files didn't, and stamping them with the schema's +// version would burn version numbers for a format that hasn't evolved. +const MigrationSerializerVersion = 1 + // MigrationVersion is a JSON-marshalable representation of migration version (timestamp) type MigrationVersion struct { Version string `json:"version"` diff --git a/splitdecisions/splitdecisions.go b/splitdecisions/splitdecisions.go index 3d9fb8d..4d5df8e 100644 --- a/splitdecisions/splitdecisions.go +++ b/splitdecisions/splitdecisions.go @@ -53,7 +53,7 @@ func (s *SplitDecision) Filename() *string { // File returns a serializable MigrationFile for this migration func (s *SplitDecision) File() *serializers.MigrationFile { return &serializers.MigrationFile{ - SerializerVersion: serializers.SerializerVersion, + SerializerVersion: serializers.MigrationSerializerVersion, SplitDecision: &serializers.SplitDecision{ Split: *s.split, Variant: *s.variant, diff --git a/splitretirements/splitretirements.go b/splitretirements/splitretirements.go index c9c5b2e..eeb5b55 100644 --- a/splitretirements/splitretirements.go +++ b/splitretirements/splitretirements.go @@ -53,7 +53,7 @@ func (s *SplitRetirement) Filename() *string { // File returns a serializable MigrationFile for this migration func (s *SplitRetirement) File() *serializers.MigrationFile { return &serializers.MigrationFile{ - SerializerVersion: serializers.SerializerVersion, + SerializerVersion: serializers.MigrationSerializerVersion, SplitRetirement: &serializers.SplitRetirement{ Split: *s.split, Decision: *s.decision, diff --git a/splits/splits.go b/splits/splits.go index ddd6316..49e576f 100644 --- a/splits/splits.go +++ b/splits/splits.go @@ -109,7 +109,7 @@ func (s *Split) Filename() *string { // File returns a serializable MigrationFile for this migration func (s *Split) File() *serializers.MigrationFile { return &serializers.MigrationFile{ - SerializerVersion: serializers.SerializerVersion, + SerializerVersion: serializers.MigrationSerializerVersion, Split: &serializers.SplitYAML{ Name: *s.name, Weights: *s.weights, From 6d269083c12a2a3b22b93af894a46617bf64772b Mon Sep 17 00:00:00 2001 From: Harris Effron Date: Tue, 14 Jul 2026 15:19:55 -0400 Subject: [PATCH 4/7] feat!: track applied migrations as a schema_versions list 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 --- fakeserver/server_test.go | 9 +- migrationmanagers/migrationmanagers.go | 9 +- schema/schema.go | 27 ++++- schema/schema_test.go | 52 ++++++++++ schemaloaders/schemaloaders.go | 32 +++++- schemaloaders/schemaloaders_test.go | 132 +++++++++++++++++++++++++ serializers/serializers.go | 21 +++- 7 files changed, 262 insertions(+), 20 deletions(-) create mode 100644 schemaloaders/schemaloaders_test.go diff --git a/fakeserver/server_test.go b/fakeserver/server_test.go index a0a554a..de5147a 100644 --- a/fakeserver/server_test.go +++ b/fakeserver/server_test.go @@ -20,8 +20,9 @@ import ( ) var testSchema = ` -serializer_version: 1 -schema_version: "2020011774023" +serializer_version: 2 +schema_versions: +- "2020011774023" splits: - name: test.test_experiment weights: @@ -34,8 +35,8 @@ splits: ` var otherTestSchema = `{ - "serializer_version": 1, - "schema_version": "2020011774023", + "serializer_version": 2, + "schema_versions": ["2020011774023"], "splits": [ { "name": "test.json_experiment", diff --git a/migrationmanagers/migrationmanagers.go b/migrationmanagers/migrationmanagers.go index 2d43455..b4b9131 100644 --- a/migrationmanagers/migrationmanagers.go +++ b/migrationmanagers/migrationmanagers.go @@ -94,8 +94,8 @@ func (m *MigrationManager) ApplyToSchema(migrationRepo migrations.Repository, id } appliedVersion := m.migration.MigrationVersion() - if appliedVersion != nil && m.schema.SchemaVersion < *appliedVersion { - m.schema.SchemaVersion = *appliedVersion + if appliedVersion != nil { + m.schema.AddVersion(*appliedVersion) } return nil } @@ -160,10 +160,5 @@ func (m *MigrationManager) SyncVersion() error { return fmt.Errorf("got %d status code", resp.StatusCode) } - appliedVersion := m.migration.MigrationVersion() - if m.schema.SchemaVersion < *appliedVersion { - m.schema.SchemaVersion = *appliedVersion - } - return nil } diff --git a/schema/schema.go b/schema/schema.go index 175547d..f9c2c83 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -1,6 +1,8 @@ package schema import ( + "bytes" + "crypto/sha1" "encoding/json" "errors" "fmt" @@ -63,9 +65,10 @@ func Generate() (*serializers.Schema, error) { return schema, nil } -// Write a schema to disk after alpha-sorting its resources +// Write a schema to disk after sorting its resources into a stable order func Write(schema *serializers.Schema) error { SortAlphabetically(schema) + sortSchemaVersions(schema) schemaPath, _ := findSchemaPath() @@ -204,12 +207,28 @@ func applyAllMigrationsToSchema(schema *serializers.Schema) error { return err } } - if len(versions) != 0 { - schema.SchemaVersion = versions[len(versions)-1] - } + schema.SchemaVersions = versions return nil } +// sortSchemaVersions orders the applied-version list by the SHA-1 of each +// version. The order is otherwise meaningless; hashing scatters newly added +// versions through the list instead of clustering them by timestamp, so two +// branches that each append a migration rarely touch the same lines. Hashes +// are precomputed so each version is hashed once rather than on every +// comparison. +func sortSchemaVersions(schema *serializers.Schema) { + hashes := make(map[string][sha1.Size]byte, len(schema.SchemaVersions)) + for _, version := range schema.SchemaVersions { + hashes[version] = sha1.Sum([]byte(version)) + } + sort.Slice(schema.SchemaVersions, func(i, j int) bool { + a := hashes[schema.SchemaVersions[i]] + b := hashes[schema.SchemaVersions[j]] + return bytes.Compare(a[:], b[:]) < 0 + }) +} + // SortAlphabetically sorts the schema's resource slices by their natural keys func SortAlphabetically(schema *serializers.Schema) { sort.Slice(schema.RemoteKills, func(i, j int) bool { diff --git a/schema/schema_test.go b/schema/schema_test.go index 65a2c65..b4b2412 100644 --- a/schema/schema_test.go +++ b/schema/schema_test.go @@ -1,12 +1,64 @@ package schema import ( + "os" + "path/filepath" + "strconv" "testing" "github.com/Betterment/testtrack-cli/serializers" "github.com/stretchr/testify/require" ) +func TestSortSchemaVersionsByHash(t *testing.T) { + schema := &serializers.Schema{SchemaVersions: []string{ + "2020011712345", + "2020011712346", + "2020011712347", + "2020011712348", + "2020011712349", + }} + sortSchemaVersions(schema) + + expected := []string{ + "2020011712349", // 0dfad45993f8e2122515814d8e62f676fcb33841 + "2020011712347", // 13af4b988d91f57f71abeb0d657c24a18fba27d7 + "2020011712346", // 529bb729c31584a72d98669cf1daa0b9509379ce + "2020011712348", // a467c4bdcdbdac7030e77951932029e03a8d0920 + "2020011712345", // bef98a4d8faa7aea1b347fb834a25c01be3c44d2 + } + require.Equal(t, expected, schema.SchemaVersions, "versions should be ordered by sha1 of the version") +} + +func TestGenerateRecordsAllMigrationVersions(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + require.NoError(t, os.MkdirAll("testtrack/migrate", 0755)) + + versions := []string{"2020011712345", "2020011712346", "2020011712347"} + for i, version := range versions { + writeSplitMigration(t, version, []string{"alpha_experiment", "bravo_experiment", "charlie_experiment"}[i]) + } + + schema, err := Generate() + require.NoError(t, err) + + require.ElementsMatch(t, versions, schema.SchemaVersions, + "every migration version should be recorded in schema_versions") +} + +func writeSplitMigration(t *testing.T, version, name string) { + t.Helper() + contents := "serializer_version: " + strconv.Itoa(serializers.MigrationSerializerVersion) + "\n" + + "split:\n" + + " name: " + name + "\n" + + " weights:\n" + + " control: 50\n" + + " treatment: 50\n" + path := filepath.Join("testtrack", "migrate", version+"_"+name+".yml") + require.NoError(t, os.WriteFile(path, []byte(contents), 0644)) +} + func TestSortAlphabeticallyRemoteKillsIsDeterministic(t *testing.T) { // These three kills are mutually incomparable under the old buggy // comparator (Split < Split && Reason < Reason), which made the written diff --git a/schemaloaders/schemaloaders.go b/schemaloaders/schemaloaders.go index cbf5d22..26ad5e3 100644 --- a/schemaloaders/schemaloaders.go +++ b/schemaloaders/schemaloaders.go @@ -71,10 +71,36 @@ func (s *SchemaLoader) Load() error { } } + appliedVersions := make(map[string]bool, len(s.schema.SchemaVersions)) + for _, version := range s.schema.SchemaVersions { + appliedVersions[version] = true + } + + var unrecorded []string + for _, version := range s.migrationRepo.SortedVersions() { + if !appliedVersions[version] { + unrecorded = append(unrecorded, version) + } + } + if len(unrecorded) > 0 { + // An unrecorded version can mean two different states, and the safe + // remedy differs: if only the version list was damaged (the schema body + // already reflects the migration), `schema upgrade` records it; but if + // the body change never landed (hand-copied file, merge dropped the + // schema hunk), upgrade would mark it applied WITHOUT applying it and + // permanently silence this warning. Enumerate the versions so the user + // can check, and print before syncing so the hint isn't lost if a + // SyncVersion call fails below. + fmt.Println("Warning: there are migrations on disk not recorded in the schema file:") + for _, version := range unrecorded { + fmt.Printf(" %s\n", version) + } + fmt.Println("If the schema file already reflects their changes (e.g. a merge lost only the version list), run `testtrack schema upgrade` to record them. If it does not, `schema upgrade` would mark them applied WITHOUT applying them - instead run `testtrack schema generate` to rebuild the schema from migrations (if your history replays cleanly), or re-create the change with the CLI.") + } + for _, version := range s.migrationRepo.SortedVersions() { - if version > s.schema.SchemaVersion { - fmt.Println("Schema load complete, but there are migrations newer than the schema file - run testtrack migrate to apply them.") - break + if !appliedVersions[version] { + continue } err := migrationmanagers.NewWithServer((*s.migrationRepo)[version], s.server).SyncVersion() if err != nil { diff --git a/schemaloaders/schemaloaders_test.go b/schemaloaders/schemaloaders_test.go new file mode 100644 index 0000000..7737351 --- /dev/null +++ b/schemaloaders/schemaloaders_test.go @@ -0,0 +1,132 @@ +package schemaloaders + +import ( + "io" + "net/http" + "os" + "strings" + "testing" + + "github.com/Betterment/testtrack-cli/migrations" + "github.com/Betterment/testtrack-cli/serializers" + "github.com/Betterment/testtrack-cli/splits" + "github.com/stretchr/testify/require" +) + +const unrecordedWarning = "migrations on disk not recorded in the schema file" + +// fakeServer records the paths it's asked to POST and succeeds unless +// failAfter is set, in which case Posts beyond that count return a 500. +type fakeServer struct { + postedPaths []string + failAfter int +} + +func (f *fakeServer) Get(string, interface{}) error { return nil } + +func (f *fakeServer) Delete(string) error { return nil } + +func (f *fakeServer) Post(path string, _ interface{}) (*http.Response, error) { + f.postedPaths = append(f.postedPaths, path) + if f.failAfter > 0 && len(f.postedPaths) > f.failAfter { + return &http.Response{StatusCode: 500, Body: io.NopCloser(strings.NewReader(""))}, nil + } + return &http.Response{StatusCode: 204, Body: io.NopCloser(strings.NewReader(""))}, nil +} + +func TestLoadSyncsOnlyRecordedVersionsAndWarnsOnce(t *testing.T) { + server := &fakeServer{} + // Two versions are recorded in the schema; two migration files on disk are not. + schema := &serializers.Schema{SchemaVersions: []string{"2020011712345", "2020011712346"}} + repo := migrations.Repository{ + "2020011712345": newSplitMigration(t, "2020011712345"), + "2020011712346": newSplitMigration(t, "2020011712346"), + "2020011712347": newSplitMigration(t, "2020011712347"), + "2020011712348": newSplitMigration(t, "2020011712348"), + } + loader := &SchemaLoader{server: server, schema: schema, migrationRepo: &repo} + + out := captureStdout(t, func() { + require.NoError(t, loader.Load()) + }) + + require.Equal(t, 1, strings.Count(out, unrecordedWarning), + "warning should print exactly once no matter how many unrecorded migrations exist") + // The warning enumerates exactly the unrecorded versions so the user can + // inspect them, and explains both repair paths. + require.Contains(t, out, "2020011712347") + require.Contains(t, out, "2020011712348") + require.NotContains(t, out, " 2020011712345\n") + require.Contains(t, out, "testtrack schema upgrade") + require.Contains(t, out, "WITHOUT applying them") + // Only the two recorded versions get marked applied on the server; the + // schema here has no splits/etc., so these are the only posts. + require.Equal(t, []string{"api/v2/migrations", "api/v2/migrations"}, server.postedPaths) +} + +func TestLoadWarningSurvivesSyncVersionFailure(t *testing.T) { + // The first SyncVersion succeeds, the second gets a 500 — the warning must + // already be on stdout, not swallowed by the error return. + server := &fakeServer{failAfter: 1} + schema := &serializers.Schema{SchemaVersions: []string{"2020011712345", "2020011712346"}} + repo := migrations.Repository{ + "2020011712345": newSplitMigration(t, "2020011712345"), + "2020011712346": newSplitMigration(t, "2020011712346"), + "2020011712347": newSplitMigration(t, "2020011712347"), + } + loader := &SchemaLoader{server: server, schema: schema, migrationRepo: &repo} + + out := captureStdout(t, func() { + require.Error(t, loader.Load()) + }) + + require.Equal(t, 1, strings.Count(out, unrecordedWarning), + "warning must print even when a later SyncVersion fails") + require.Contains(t, out, "2020011712347") +} + +func TestLoadDoesNotWarnWhenEveryMigrationIsRecorded(t *testing.T) { + server := &fakeServer{} + schema := &serializers.Schema{SchemaVersions: []string{"2020011712345", "2020011712346"}} + repo := migrations.Repository{ + "2020011712345": newSplitMigration(t, "2020011712345"), + "2020011712346": newSplitMigration(t, "2020011712346"), + } + loader := &SchemaLoader{server: server, schema: schema, migrationRepo: &repo} + + out := captureStdout(t, func() { + require.NoError(t, loader.Load()) + }) + + require.NotContains(t, out, unrecordedWarning) + require.Equal(t, []string{"api/v2/migrations", "api/v2/migrations"}, server.postedPaths) +} + +func newSplitMigration(t *testing.T, version string) migrations.IMigration { + t.Helper() + migration, err := splits.FromFile(&version, &serializers.SplitYAML{ + Name: "split_" + version, + Weights: map[string]int{"control": 50, "treatment": 50}, + }) + require.NoError(t, err) + return migration +} + +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + original := os.Stdout + r, w, err := os.Pipe() + require.NoError(t, err) + os.Stdout = w + defer func() { + os.Stdout = original + r.Close() + }() + + fn() + require.NoError(t, w.Close()) + + out, err := io.ReadAll(r) + require.NoError(t, err) + return string(out) +} diff --git a/serializers/serializers.go b/serializers/serializers.go index 951e1cb..c0b8e50 100644 --- a/serializers/serializers.go +++ b/serializers/serializers.go @@ -6,7 +6,13 @@ import ( // SerializerVersion is the current version of the schema file format so we // can evolve it over time. -const SerializerVersion = 1 +// +// Version 2 replaced the single scalar schema_version high-water-mark with a +// schema_versions list of every applied migration version, eliminating the +// merge-conflict hotspot that the scalar created (every new migration rewrote +// the same line). The list is ordered by a hash of each version so concurrently +// added migrations scatter through the file instead of clustering. +const SerializerVersion = 2 // MigrationSerializerVersion is the current version of the migration file // format. It is versioned independently of the schema file: the schema format @@ -97,13 +103,24 @@ type SchemaSplit struct { // migration validation and bootstrapping of new ecosystems type Schema struct { SerializerVersion int `yaml:"serializer_version" json:"serializer_version"` - SchemaVersion string `yaml:"schema_version" json:"schema_version"` + SchemaVersions []string `yaml:"schema_versions,omitempty" json:"schema_versions,omitempty"` Splits []SchemaSplit `yaml:"splits,omitempty" json:"splits,omitempty"` IdentifierTypes []IdentifierType `yaml:"identifier_types,omitempty" json:"identifier_types,omitempty"` RemoteKills []RemoteKill `yaml:"remote_kills,omitempty" json:"remote_kills,omitempty"` FeatureCompletions []FeatureCompletion `yaml:"feature_completions,omitempty" json:"feature_completions,omitempty"` } +// AddVersion records a migration version as applied in the schema, ignoring +// duplicates. Ordering is handled at write time, so callers needn't sort. +func (s *Schema) AddVersion(version string) { + for _, v := range s.SchemaVersions { + if v == version { + return + } + } + s.SchemaVersions = append(s.SchemaVersions, version) +} + // LegacySchema represents the Rails migration-piggybacked testtrack schema files of old type LegacySchema struct { IdentifierTypes []string `yaml:"identifier_types"` From 93639ee3bc949656604da50e55379fb27c918657 Mon Sep 17 00:00:00 2001 From: Harris Effron Date: Tue, 14 Jul 2026 15:20:19 -0400 Subject: [PATCH 5/7] Refuse v1 and damaged schemas; add `schema upgrade` to convert them 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 --- cmds/schema_upgrade.go | 40 ++++++ schema/schema.go | 116 +++++++++++++++- schema/schema_test.go | 269 ++++++++++++++++++++++++++++++++++--- serializers/serializers.go | 21 ++- 4 files changed, 416 insertions(+), 30 deletions(-) create mode 100644 cmds/schema_upgrade.go diff --git a/cmds/schema_upgrade.go b/cmds/schema_upgrade.go new file mode 100644 index 0000000..de83502 --- /dev/null +++ b/cmds/schema_upgrade.go @@ -0,0 +1,40 @@ +package cmds + +import ( + "github.com/Betterment/testtrack-cli/schema" + "github.com/spf13/cobra" +) + +var schemaUpgradeDoc = ` +Upgrades testtrack/schema.{json,yml} to the schema format this CLI writes, +in place, without replaying migrations. + +It keeps the materialized state already in the file (splits, decisions, +retirements, feature completions, identifier types) and only rebuilds the +applied-migration version list from the files in testtrack/migrate. This is the +upgrade path to use when a newer CLI refuses to read an older-format schema. + +Unlike 'schema generate', upgrade does not rebuild the schema from scratch, so +it works on apps whose migrations can't be replayed cleanly - e.g. when +testtrack/migrate predates some splits, or references splits that were created +out of band in the TestTrack admin. +` + +func init() { + schemaCmd.AddCommand(schemaUpgradeCmd) +} + +var schemaUpgradeCmd = &cobra.Command{ + Use: "upgrade", + Short: "Upgrade schema.{json,yml} to the current format in place", + Long: schemaUpgradeDoc, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + return schemaUpgrade() + }, +} + +func schemaUpgrade() error { + _, err := schema.Upgrade() + return err +} diff --git a/schema/schema.go b/schema/schema.go index f9c2c83..6219ed9 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -29,22 +29,117 @@ func findSchemaPath() (string, bool) { return "testtrack/schema.json", false } +// readSchemaFile locates and unmarshals the schema file, rejecting files +// written by a newer CLI. exists is false (with a nil schema and nil error) +// when no schema file is present; callers decide how to handle that. +func readSchemaFile() (schema *serializers.Schema, schemaPath string, exists bool, err error) { + schemaPath, exists = findSchemaPath() + if !exists { + return nil, schemaPath, false, nil + } + schemaBytes, err := os.ReadFile(schemaPath) + if err != nil { + return nil, schemaPath, true, err + } + var s serializers.Schema + err = yaml.Unmarshal(schemaBytes, &s) + if err != nil { + return nil, schemaPath, true, err + } + if s.SerializerVersion > serializers.SerializerVersion { + return nil, schemaPath, true, fmt.Errorf( + "%s was written by a newer testtrack CLI (serializer_version %d, this CLI supports %d). Please upgrade your testtrack CLI", + schemaPath, s.SerializerVersion, serializers.SerializerVersion, + ) + } + return &s, schemaPath, true, nil +} + // Read a schema from disk or generate one func Read() (*serializers.Schema, error) { - schemaPath, exists := findSchemaPath() + schema, schemaPath, exists, err := readSchemaFile() + if err != nil { + return nil, err + } if !exists { return Generate() } - schemaBytes, err := os.ReadFile(schemaPath) + if schema.SerializerVersion < serializers.SerializerVersion { + // An older file predates a format change and may be missing data that + // can't be reconstructed by re-reading it (e.g. the v1 scalar + // schema_version carried no per-migration list). Refuse to read it + // rather than silently round-trip a lossy upgrade; `schema upgrade` + // converts it in place, preserving the materialized state. + return nil, fmt.Errorf( + "%s uses an older schema format (serializer_version %d, this CLI writes %d). Run `testtrack schema upgrade` to upgrade it", + schemaPath, schema.SerializerVersion, serializers.SerializerVersion, + ) + } + if schema.LegacySchemaVersion != nil { + // A pre-2.0 CLI rewriting a v2 schema produces a hybrid: it round-trips + // serializer_version: 2 but writes the v1 shape (scalar schema_version, + // no schema_versions list), so the version check above can't catch it. + // Reading it as-is would silently continue with an empty applied-version + // list. Key presence, not value, is the tell: 1.x write paths that never + // set the scalar (e.g. `sync`) emit schema_version: "". + return nil, fmt.Errorf( + "%s has serializer_version %d but contains the legacy schema_version field - it was likely rewritten by a pre-2.0 testtrack CLI. Run `testtrack schema upgrade` to repair it, and make sure no older CLI touches it again", + schemaPath, schema.SerializerVersion, + ) + } + if len(schema.SchemaVersions) == 0 { + // No machine write produces an empty version list alongside migrations + // on disk — that state means the schema_versions block was lost to a + // hand-edit or merge resolution (or the legacy scalar was nulled out, + // which also evades the presence check above). Reading it as-is would + // ratify the truncated list on the next write. A missing or unreadable + // migrate dir counts as no migrations. + if filenames, err := migrationloaders.Filenames(); err == nil && len(filenames) > 0 { + return nil, fmt.Errorf( + "%s has no schema_versions but testtrack/migrate contains migrations - the applied-version list was likely lost in a merge. Run `testtrack schema upgrade` to rebuild it", + schemaPath, + ) + } + } + return schema, nil +} + +// Upgrade converts an existing schema file to the current serializer format in +// place. Unlike Generate it does not replay migrations, so it preserves the +// already-materialized state (splits, decisions, retirements, etc.) and works +// even on schemas that can't be rebuilt from scratch — e.g. apps whose +// testtrack/migrate predates some splits or references ones created out of +// band. The only thing it rebuilds is the applied-version list, which it reads +// from the migration filenames on disk. +func Upgrade() (*serializers.Schema, error) { + schema, _, exists, err := readSchemaFile() if err != nil { return nil, err } - var schema serializers.Schema - err = yaml.Unmarshal(schemaBytes, &schema) + if !exists { + return nil, errors.New("no testtrack schema file to upgrade. Run testtrack schema generate to create one") + } + + versions, err := migrationloaders.Versions() + if err != nil { + if os.IsNotExist(err) { + // Legacy repos can have a schema with no testtrack/migrate dir at + // all (git doesn't track empty dirs); there are no versions to + // record, which is a valid v2 state. + versions = nil + } else { + return nil, err + } + } + schema.SchemaVersions = versions + schema.SerializerVersion = serializers.SerializerVersion + schema.LegacySchemaVersion = nil + + err = Write(schema) if err != nil { return nil, err } - return &schema, nil + return schema, nil } // Generate a schema from migrations on the filesystem and write it to disk @@ -118,7 +213,16 @@ func Link(force bool) error { return os.Symlink(dir+"/"+schemaPath, path) } -// ReadMerged merges schemas linked at ~/testtrack/schemas into a single virtual schema +// ReadMerged merges schemas linked at ~/testtrack/schemas into a single virtual schema. +// +// It deliberately bypasses Read's serializer-version and hybrid guards: the +// linked schemas belong to *other* apps that upgrade on their own schedule, +// and hard-failing on a neighbor's stale schema would break assign/fakeserver +// for every app on the machine. This tolerance is safe because the body +// fields consumed here (splits, identifier_types, remote_kills, +// feature_completions) have never changed shape across serializer versions — +// a future version that reshapes them must add per-file version handling +// here. func ReadMerged() (*serializers.Schema, error) { configDir, err := paths.FakeServerConfigDir() if err != nil { diff --git a/schema/schema_test.go b/schema/schema_test.go index b4b2412..08fe648 100644 --- a/schema/schema_test.go +++ b/schema/schema_test.go @@ -30,33 +30,215 @@ func TestSortSchemaVersionsByHash(t *testing.T) { require.Equal(t, expected, schema.SchemaVersions, "versions should be ordered by sha1 of the version") } -func TestGenerateRecordsAllMigrationVersions(t *testing.T) { +func TestReadRejectsNewerSerializerVersion(t *testing.T) { + withSchemaFile(t, ` +serializer_version: 99 +schema_versions: +- "2020011712345" +`) + + _, err := Read() + require.Error(t, err) + require.Contains(t, err.Error(), "upgrade your testtrack CLI") +} + +func TestReadRejectsOlderSerializerVersion(t *testing.T) { + withSchemaFile(t, ` +serializer_version: 1 +schema_version: "2020011712345" +`) + + _, err := Read() + require.Error(t, err) + require.Contains(t, err.Error(), "testtrack schema upgrade") +} + +func TestReadRejectsHybridSchemaFromPre2CLIRewrite(t *testing.T) { + // A pre-2.0 CLI rewriting a v2 schema round-trips serializer_version: 2 but + // writes the v1 shape: scalar schema_version, no schema_versions list. + withSchemaFile(t, ` +serializer_version: 2 +schema_version: "2020011712345" +splits: +- name: some_split + weights: + "false": 100 + "true": 0 +`) + + _, err := Read() + require.Error(t, err) + require.Contains(t, err.Error(), "pre-2.0 testtrack CLI") + require.Contains(t, err.Error(), "testtrack schema upgrade") +} + +func TestReadRejectsHybridSchemaWithEmptyScalar(t *testing.T) { + // 1.x write paths that never set the scalar (e.g. `sync`) emit + // schema_version: "" — the v1 field has no omitempty. Detection must be by + // key presence, not non-empty value, or this hybrid silently reads as a v2 + // schema with an empty applied-version list. + withSchemaFile(t, ` +serializer_version: 2 +schema_version: "" +splits: +- name: some_split + weights: + "false": 100 + "true": 0 +`) + + _, err := Read() + require.Error(t, err) + require.Contains(t, err.Error(), "pre-2.0 testtrack CLI") + require.Contains(t, err.Error(), "testtrack schema upgrade") +} + +func TestReadRejectsEmptyVersionListWithMigrationsOnDisk(t *testing.T) { + // A merge resolution that deletes the schema_versions block (or nulls the + // legacy scalar, which evades the presence guard) leaves a v2 file with no + // applied-version list. Reading it as-is would ratify the truncated list on + // the next write. + for name, contents := range map[string]string{ + "missing list": ` +serializer_version: 2 +splits: +- name: some_split + weights: + "false": 100 + "true": 0 +`, + "nulled legacy scalar": ` +serializer_version: 2 +schema_version: +splits: +- name: some_split + weights: + "false": 100 + "true": 0 +`, + } { + t.Run(name, func(t *testing.T) { + withSchemaFile(t, contents) + require.NoError(t, os.MkdirAll("testtrack/migrate", 0755)) + writeSplitMigration(t, "2020011712345", "some_split") + + _, err := Read() + require.Error(t, err) + require.Contains(t, err.Error(), "no schema_versions but testtrack/migrate contains migrations") + require.Contains(t, err.Error(), "testtrack schema upgrade") + }) + } +} + +func TestReadAcceptsEmptyVersionListWithoutMigrations(t *testing.T) { + // A schema with no recorded versions is valid when there are no migrations + // on disk (fresh projects, legacy repos with no migrate dir). + withSchemaFile(t, ` +serializer_version: 2 +splits: +- name: some_split + weights: + "false": 100 + "true": 0 +`) + + _, err := Read() + require.NoError(t, err) +} + +func TestUpgradeRepairsHybridSchema(t *testing.T) { dir := t.TempDir() t.Chdir(dir) require.NoError(t, os.MkdirAll("testtrack/migrate", 0755)) + require.NoError(t, os.WriteFile(filepath.Join("testtrack", "schema.yml"), []byte(` +serializer_version: 2 +schema_version: "2020011712345" +splits: +- name: some_split + weights: + "false": 100 + "true": 0 +`), 0644)) + writeSplitMigration(t, "2020011712345", "some_split") - versions := []string{"2020011712345", "2020011712346", "2020011712347"} - for i, version := range versions { - writeSplitMigration(t, version, []string{"alpha_experiment", "bravo_experiment", "charlie_experiment"}[i]) - } + schema, err := Upgrade() + require.NoError(t, err) + require.Equal(t, []string{"2020011712345"}, schema.SchemaVersions) - schema, err := Generate() + // The legacy scalar must not survive the rewrite, so the repaired file + // passes Read's hybrid guard. + contents, err := os.ReadFile(filepath.Join("testtrack", "schema.yml")) require.NoError(t, err) + require.NotContains(t, string(contents), "schema_version:") - require.ElementsMatch(t, versions, schema.SchemaVersions, - "every migration version should be recorded in schema_versions") + reread, err := Read() + require.NoError(t, err) + require.Equal(t, []string{"2020011712345"}, reread.SchemaVersions) } -func writeSplitMigration(t *testing.T, version, name string) { - t.Helper() - contents := "serializer_version: " + strconv.Itoa(serializers.MigrationSerializerVersion) + "\n" + - "split:\n" + - " name: " + name + "\n" + - " weights:\n" + - " control: 50\n" + - " treatment: 50\n" - path := filepath.Join("testtrack", "migrate", version+"_"+name+".yml") - require.NoError(t, os.WriteFile(path, []byte(contents), 0644)) +func TestUpgradeConvertsInPlacePreservingBodyWithoutReplaying(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + require.NoError(t, os.MkdirAll("testtrack/migrate", 0755)) + + // A v1 schema whose body references a split that has NO create migration on + // disk — i.e. it could not be rebuilt by replaying (a common real-world case). + require.NoError(t, os.WriteFile(filepath.Join("testtrack", "schema.yml"), []byte(` +serializer_version: 1 +schema_version: "2020011712346" +splits: +- name: legacy_split_created_out_of_band + weights: + "false": 100 + "true": 0 +`), 0644)) + + // Migration files on disk decide that orphan split — replaying these from + // scratch would fail, but upgrade doesn't replay. + writeSplitMigration(t, "2020011712345", "some_other_split") + require.NoError(t, os.WriteFile(filepath.Join("testtrack", "migrate", "2020011712346_create_split_decision_legacy_split_created_out_of_band.yml"), + []byte("serializer_version: 1\nsplit_decision:\n split: legacy_split_created_out_of_band\n variant: \"false\"\n"), 0644)) + + schema, err := Upgrade() + require.NoError(t, err) + + require.Equal(t, serializers.SerializerVersion, schema.SerializerVersion) + // Version list rebuilt from the migration filenames. + require.ElementsMatch(t, []string{"2020011712345", "2020011712346"}, schema.SchemaVersions) + // Materialized body preserved as-is (not replayed/rebuilt). + require.Len(t, schema.Splits, 1) + require.Equal(t, "legacy_split_created_out_of_band", schema.Splits[0].Name) + + // And the file is now readable by the normal guarded Read(). + reread, err := Read() + require.NoError(t, err) + require.Equal(t, serializers.SerializerVersion, reread.SerializerVersion) +} + +func TestUpgradeUsesFilenamesOnlyAndDedupes(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + require.NoError(t, os.MkdirAll("testtrack/migrate", 0755)) + require.NoError(t, os.WriteFile(filepath.Join("testtrack", "schema.yml"), []byte(` +serializer_version: 1 +schema_version: "2020011712347" +`), 0644)) + + // Unparsable contents must not break upgrade — it only needs filenames. + require.NoError(t, os.WriteFile(filepath.Join("testtrack", "migrate", "2020011712345_create_split_broken.yml"), + []byte("{{{ not yaml"), 0644)) + // Two files sharing a version (it happens in real repos) record it once. + require.NoError(t, os.WriteFile(filepath.Join("testtrack", "migrate", "2020011712346_create_split_twin_a.yml"), + []byte("also garbage"), 0644)) + require.NoError(t, os.WriteFile(filepath.Join("testtrack", "migrate", "2020011712346_create_split_twin_b.yml"), + []byte("also garbage"), 0644)) + // Hidden files are skipped, matching the migration loader. + require.NoError(t, os.WriteFile(filepath.Join("testtrack", "migrate", ".DS_Store"), + []byte{0}, 0644)) + + schema, err := Upgrade() + require.NoError(t, err) + require.ElementsMatch(t, []string{"2020011712345", "2020011712346"}, schema.SchemaVersions) } func TestSortAlphabeticallyRemoteKillsIsDeterministic(t *testing.T) { @@ -78,3 +260,54 @@ func TestSortAlphabeticallyRemoteKillsIsDeterministic(t *testing.T) { "remote kills must sort to the same order regardless of input order") require.Equal(t, []serializers.RemoteKill{kills[0], kills[1], kills[2]}, forward.RemoteKills) } + +func TestReadAcceptsCurrentSerializerVersion(t *testing.T) { + withSchemaFile(t, ` +serializer_version: 2 +schema_versions: +- "2020011712345" +- "2020011712346" +`) + + schema, err := Read() + require.NoError(t, err) + require.Equal(t, []string{"2020011712345", "2020011712346"}, schema.SchemaVersions) +} + +func TestGenerateRecordsAllMigrationVersions(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + require.NoError(t, os.MkdirAll("testtrack/migrate", 0755)) + + versions := []string{"2020011712345", "2020011712346", "2020011712347"} + for i, version := range versions { + writeSplitMigration(t, version, []string{"alpha_experiment", "bravo_experiment", "charlie_experiment"}[i]) + } + + schema, err := Generate() + require.NoError(t, err) + + require.ElementsMatch(t, versions, schema.SchemaVersions, + "every migration version should be recorded in schema_versions") +} + +// withSchemaFile writes a schema file into a temp testtrack dir and chdirs there. +func withSchemaFile(t *testing.T, contents string) { + t.Helper() + dir := t.TempDir() + t.Chdir(dir) + require.NoError(t, os.MkdirAll("testtrack", 0755)) + require.NoError(t, os.WriteFile(filepath.Join("testtrack", "schema.yml"), []byte(contents), 0644)) +} + +func writeSplitMigration(t *testing.T, version, name string) { + t.Helper() + contents := "serializer_version: " + strconv.Itoa(serializers.MigrationSerializerVersion) + "\n" + + "split:\n" + + " name: " + name + "\n" + + " weights:\n" + + " control: 50\n" + + " treatment: 50\n" + path := filepath.Join("testtrack", "migrate", version+"_"+name+".yml") + require.NoError(t, os.WriteFile(path, []byte(contents), 0644)) +} diff --git a/serializers/serializers.go b/serializers/serializers.go index c0b8e50..b0591be 100644 --- a/serializers/serializers.go +++ b/serializers/serializers.go @@ -102,12 +102,21 @@ type SchemaSplit struct { // Schema is the YAML-marshalable representation of the TestTrack schema for // migration validation and bootstrapping of new ecosystems type Schema struct { - SerializerVersion int `yaml:"serializer_version" json:"serializer_version"` - SchemaVersions []string `yaml:"schema_versions,omitempty" json:"schema_versions,omitempty"` - Splits []SchemaSplit `yaml:"splits,omitempty" json:"splits,omitempty"` - IdentifierTypes []IdentifierType `yaml:"identifier_types,omitempty" json:"identifier_types,omitempty"` - RemoteKills []RemoteKill `yaml:"remote_kills,omitempty" json:"remote_kills,omitempty"` - FeatureCompletions []FeatureCompletion `yaml:"feature_completions,omitempty" json:"feature_completions,omitempty"` + SerializerVersion int `yaml:"serializer_version" json:"serializer_version"` + SchemaVersions []string `yaml:"schema_versions,omitempty" json:"schema_versions,omitempty"` + // LegacySchemaVersion is the v1 scalar high-water mark. It only exists so + // reads can detect a file that carries it — either a plain v1 schema, or a + // hybrid produced by a pre-2.0 CLI rewriting a v2 schema (those round-trip + // serializer_version: 2 while writing the v1 shape). It's a pointer because + // detection is by key presence, not value: 1.x always writes the key (no + // omitempty), and rewrite paths that never set the scalar (e.g. 1.x `sync`) + // emit schema_version: "", which must still trip the guard. It is never + // written: `schema upgrade` clears it and omitempty drops nil from output. + LegacySchemaVersion *string `yaml:"schema_version,omitempty" json:"schema_version,omitempty"` + Splits []SchemaSplit `yaml:"splits,omitempty" json:"splits,omitempty"` + IdentifierTypes []IdentifierType `yaml:"identifier_types,omitempty" json:"identifier_types,omitempty"` + RemoteKills []RemoteKill `yaml:"remote_kills,omitempty" json:"remote_kills,omitempty"` + FeatureCompletions []FeatureCompletion `yaml:"feature_completions,omitempty" json:"feature_completions,omitempty"` } // AddVersion records a migration version as applied in the schema, ignoring From a522a1bf7ab47c6347d3609ecb2c4026b84c4cab Mon Sep 17 00:00:00 2001 From: Harris Effron Date: Tue, 14 Jul 2026 15:20:38 -0400 Subject: [PATCH 6/7] Add UPGRADING.md for the 1.x -> 2.0.0 schema format change 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 --- UPGRADING.md | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 UPGRADING.md diff --git a/UPGRADING.md b/UPGRADING.md new file mode 100644 index 0000000..c269488 --- /dev/null +++ b/UPGRADING.md @@ -0,0 +1,100 @@ +# Upgrading testtrack-cli + +## 1.x to 2.0.0 + +2.0.0 changes the on-disk schema format (`testtrack/schema.{yml,json}`). + +### What changed + +The schema's applied-migration high-water mark used to be a single scalar +field: + +```yaml +serializer_version: 1 +schema_version: "2020011543200" +``` + +Every new migration rewrote that one line, so any two branches that added +migrations conflicted on it. 2.0.0 replaces the scalar with a list of every +applied migration version (`serializer_version` bumps to `2`): + +```yaml +serializer_version: 2 +schema_versions: +- "2020060151730" +- "2020011543200" +``` + +The list is ordered by a hash of each version (not chronologically), so +concurrently added migrations scatter through the file instead of clustering +on one line. The ordering is otherwise meaningless — don't rely on it. + +### This is a breaking format change + +A 2.0.0 CLI **refuses to read** an old (`serializer_version: 1`) schema file. +Any command that reads the schema errors and tells you to run `testtrack schema +upgrade` — `create`, `migrate`, `sync`, `decide`, the `destroy` commands, and +`schema load` (essentially everything except `assign`/`unassign`, which use a +different read path). This is deliberate: the old format can't be losslessly +read into the new struct, so the CLI makes you convert it explicitly rather than +silently round-tripping a lossy upgrade. + +Conversely, a pre-2.0.0 CLI does **not** understand `schema_versions`. If it +reads a v2 schema it ignores the field, and any command that rewrites the +schema produces a hybrid: the file keeps `serializer_version: 2` (the old CLI +round-trips whatever number it read) but is otherwise back in the v1 shape — a +scalar `schema_version`, no `schema_versions` list. A 2.0.0 CLI detects the +leftover `schema_version` field, refuses to read the hybrid, and tells you to +run `testtrack schema upgrade`, which repairs it by rebuilding the version list +from `testtrack/migrate`. So the whole team — and every CI/build environment — +has to be on 2.0.0 before you commit a v2 schema, or the format will flip-flop +between commits. + +Because a 2.0.0 CLI rejects a v1 schema and a 1.x CLI mangles a v2 schema, the +switch is a flag day: the moment the v2 schema lands in the repo, every consumer +of it must already be on 2.0.0. Order the rollout so the CLI bump and the schema +commit line up, not in separate steps. + +1. **Make 2.0.0 available, but don't switch anything to it yet.** Cut the + `v2.0.0` release, update the brew formula, and make the pinned binary + available to CI — without yet changing the CI pin or asking devs to upgrade. + +2. **Land the schema conversion and the CI bump together, in one change.** In + your app root: + + ```sh + testtrack schema upgrade + ``` + + This converts `testtrack/schema.{yml,json}` to the v2 format **in place**: it + keeps the materialized state already in the file (splits, decisions, + retirements, etc.) and only rebuilds the `schema_versions` list from the + migration filenames in `testtrack/migrate`. In the **same** PR, bump the + CI/build pin to `v2.0.0`. They must be atomic: if CI runs 2.0.0 against a + still-v1 schema, `testtrack migrate` errors; if the v2 schema lands while CI + still runs 1.x, the old binary rewrites it into the hybrid described above, + and every 2.0.0 user is blocked until someone re-runs `schema upgrade`. + Expect a large + one-time diff — the new `schema_versions` block lists every migration — but + the splits body is left untouched. + +3. **Developers upgrade as that change lands** (`brew upgrade testtrack-cli`). A + developer must be on 2.0.0 before running any testtrack command against the + upgraded schema — an older binary silently rewrites it into the hybrid, + which 2.0.0 CLIs then refuse to read until it's repaired with + `schema upgrade`. + +### Notes + +- **Use `schema upgrade`, not `schema generate`, to convert.** `generate` + rebuilds the schema by replaying every migration from scratch, which fails on + many long-lived apps — e.g. when `testtrack/migrate` predates some splits, or + contains a decision/retirement for a split that was created out of band in the + TestTrack admin (so there's no create migration to replay). `upgrade` doesn't + replay; it preserves the already-correct materialized state and just restamps + the format, so it works regardless. +- `schema upgrade` reads the old file directly (it's the one command that + bypasses the version guard), so it's always the recovery path if you hit the + "older schema format" error. +- No server-side change is required. The TestTrack server tracks applied + migrations itself; `schema_version`/`schema_versions` is a CLI-only artifact. From b6fbec6d0494cabec37405d7731497a99c806dbe Mon Sep 17 00:00:00 2001 From: Harris Effron Date: Tue, 14 Jul 2026 15:20:38 -0400 Subject: [PATCH 7/7] chore: bump CLI version to 2.0.0 for the schema format change Co-Authored-By: Claude Fable 5 --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 48d6742..725cd85 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ SHELL = /bin/sh -VERSION=1.9.0 +VERSION=2.0.0 BUILD=`git rev-parse HEAD` LDFLAGS=-ldflags "-w -s \