diff --git a/.github/workflows/models-live-check.yml b/.github/workflows/models-live-check.yml new file mode 100644 index 0000000000..6ac0023974 --- /dev/null +++ b/.github/workflows/models-live-check.yml @@ -0,0 +1,59 @@ +# Validates every example's model reference against the LIVE models.dev API, +# as opposed to ci.yml's `task test` (which runs TestParseExamples against +# only the committed pkg/modelsdev/snapshot.json and is fully hermetic). +# +# This check is deliberately NOT part of ci.yml and NOT a required PR check: +# a failure here means the external models.dev catalog has drifted since the +# snapshot was last refreshed, which is unrelated to any PR's diff and can +# strike any PR (or none) at any time. See issue #4134. +name: models-live-check + +permissions: + contents: read + +concurrency: + group: models-live-check + cancel-in-progress: false + +on: + workflow_dispatch: + schedule: + # Daily, 07:00 UTC (an hour after update-models' Monday 06:00 refresh). + - cron: "0 7 * * *" + +jobs: + check: + if: github.repository == 'docker/docker-agent' + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + + - name: Set up Go + uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 + with: + go-version-file: go.mod + cache-dependency-path: go.sum + + - name: Install Task + uses: go-task/setup-task@01a4adf9db2d14c1de7a560f09170b6e0df736aa # v2.1.0 + with: + version: 3.51.1 + + - name: Check examples against the live models.dev catalog + run: task check-models-live + + - name: Explain failure + if: failure() + run: | + { + echo "### models.dev live-catalog drift detected" + echo + echo "This is **not** caused by any pull request's diff. This scheduled check" + echo "validates example model references against the live models.dev API, which" + echo "has moved on since \`pkg/modelsdev/snapshot.json\` was last refreshed." + echo + echo "The committed snapshot and \`TestParseExamples\` (the PR-blocking check) are" + echo "unaffected. Fix by refreshing the snapshot (\`task update-models\`) and/or" + echo "updating the affected example(s); see the failed step's log above for which." + } >> "$GITHUB_STEP_SUMMARY" diff --git a/Taskfile.yml b/Taskfile.yml index 392712668f..60fbf9cfc4 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -59,6 +59,11 @@ tasks: cmds: - CHECK_MODELS_SNAPSHOT_FRESHNESS=1 go test -run TestSnapshotDateIsFresh ./pkg/modelsdev/ + check-models-live: + desc: Validate example model references against the live models.dev API (hits the network; not part of `task test` since drift here is external and must not block PRs — see issue #4134) + cmds: + - CHECK_MODELS_DEV_LIVE=1 go test -count=1 -run TestExamplesAgainstLiveModelsDev ./pkg/config/ + check-plan-cross: desc: Cross-compile the plan storage and host plans packages for every file-lock build-tag variant (plus the plan-consuming CLI and TUI packages on windows) cmds: diff --git a/pkg/config/examples_test.go b/pkg/config/examples_test.go index 6bee2d009c..edd686fe5a 100644 --- a/pkg/config/examples_test.go +++ b/pkg/config/examples_test.go @@ -1,9 +1,11 @@ package config import ( + "fmt" "io/fs" "os" "path/filepath" + "sort" "strings" "testing" @@ -53,11 +55,45 @@ func collectExamples(t *testing.T) []string { return files } +// catalogModelRefs returns the models.dev IDs that TestParseExamples and +// TestExamplesAgainstLiveModelsDev must resolve for cfg, applying the same +// skip rules to both: first_available selectors are resolved at runtime +// from the environment's credentials, routed models span multiple +// providers, custom providers are self-contained (already validated via +// cfg.Providers), and modelsDevAbsentProviders lists providers models.dev +// deliberately does not catalog. +func catalogModelRefs(cfg *latest.Config) []modelsdev.ID { + var ids []modelsdev.ID + for _, model := range cfg.Models { + if model.IsFirstAvailable() { + continue + } + if model.Provider == "" || model.Model == "" { + continue + } + if modelsDevAbsentProviders[model.Provider] { + continue + } + if len(model.Routing) > 0 { + continue + } + if _, isCustomProvider := cfg.Providers[model.Provider]; isCustomProvider { + continue + } + ids = append(ids, modelsdev.NewID(model.Provider, model.Model)) + } + return ids +} + func TestParseExamples(t *testing.T) { t.Parallel() - modelsStore, err := modelsdev.NewStore() - require.NoError(t, err) + // Resolved against the embedded/committed snapshot.json only — never the + // network — so this is safe as a required, PR-blocking check: it can only + // fail when the PR's own diff (an example or the committed snapshot) + // introduces an inconsistency, never because the live models.dev catalog + // moved on since the snapshot was last refreshed. See issue #4134. + modelsStore := modelsdev.NewDatabaseStore(modelsdev.EmbeddedSnapshot()) for _, file := range collectExamples(t) { t.Run(file, func(t *testing.T) { @@ -85,20 +121,10 @@ func TestParseExamples(t *testing.T) { } require.NotEmpty(t, model.Provider) require.NotEmpty(t, model.Model) - // Skip providers that don't have entries in models.dev. - if modelsDevAbsentProviders[model.Provider] { - continue - } - // Skip models with routing rules - they use multiple providers - if len(model.Routing) > 0 { - continue - } - // Skip models that use custom providers (defined in cfg.Providers) - if _, isCustomProvider := cfg.Providers[model.Provider]; isCustomProvider { - continue - } + } - model, err := modelsStore.GetModel(t.Context(), modelsdev.NewID(model.Provider, model.Model)) + for _, id := range catalogModelRefs(cfg) { + model, err := modelsStore.GetModel(t.Context(), id) require.NoError(t, err) require.NotNil(t, model) } @@ -106,6 +132,50 @@ func TestParseExamples(t *testing.T) { } } +// TestExamplesAgainstLiveModelsDev validates the same example model +// references as TestParseExamples, but against the live models.dev API +// instead of the committed snapshot. It is opt-in (gated on +// CHECK_MODELS_DEV_LIVE, mirroring TestSnapshotDateIsFresh) and skipped by +// default: a failure here means the *external* catalog has drifted since +// the snapshot was last refreshed, which is unrelated to any PR's diff and +// must never gate a merge (issue #4134). It is intended to run on a +// schedule; see .github/workflows/models-live-check.yml. +func TestExamplesAgainstLiveModelsDev(t *testing.T) { + t.Parallel() + + if os.Getenv("CHECK_MODELS_DEV_LIVE") == "" { + t.Skip("set CHECK_MODELS_DEV_LIVE=1 to validate examples against the live models.dev catalog") + } + + db, err := modelsdev.Fetch(t.Context()) + require.NoError(t, err, "failed to fetch the live models.dev catalog") + modelsStore := modelsdev.NewDatabaseStore(db) + + var drifted []string + for _, file := range collectExamples(t) { + cfg, err := Load(t.Context(), NewFileSource(file)) + require.NoError(t, err) + + for _, id := range catalogModelRefs(cfg) { + if _, err := modelsStore.GetModel(t.Context(), id); err != nil { + drifted = append(drifted, fmt.Sprintf("%s: %s (%v)", file, id.String(), err)) + } + } + } + + if len(drifted) == 0 { + return + } + sort.Strings(drifted) + t.Fatalf("live models.dev catalog drift detected (NOT caused by this PR's diff — the\n"+ + "committed snapshot and TestParseExamples are unaffected): the following example model\n"+ + "references resolve against the committed snapshot (dated %s) but not against the\n"+ + "live models.dev API:\n %s\n\n"+ + "Fix by refreshing the snapshot (`task update-models`) and/or updating the affected\n"+ + "example(s) to reference a model the live catalog still carries.", + modelsdev.SnapshotDate().Format("2006-01-02"), strings.Join(drifted, "\n ")) +} + func TestParseExamplesAfterMarshalling(t *testing.T) { t.Parallel() diff --git a/pkg/model/provider/bedrock/client_test.go b/pkg/model/provider/bedrock/client_test.go index 9b6321892f..ecddb55c2e 100644 --- a/pkg/model/provider/bedrock/client_test.go +++ b/pkg/model/provider/bedrock/client_test.go @@ -1382,10 +1382,9 @@ func TestPromptCachingEnabled_TypeMismatch(t *testing.T) { func TestDetectCachingSupport_SupportedModel(t *testing.T) { t.Parallel() - store, err := modelsdev.NewStore() - require.NoError(t, err) + store := modelsdev.NewDatabaseStore(modelsdev.EmbeddedSnapshot()) - // Uses real models.dev lookup to verify Claude models support caching + // Uses the committed models.dev snapshot to verify Claude models support caching supported := detectCachingSupport(t.Context(), "anthropic.claude-opus-4-7", store) assert.True(t, supported) } @@ -1393,8 +1392,7 @@ func TestDetectCachingSupport_SupportedModel(t *testing.T) { func TestDetectCachingSupport_UnsupportedModel(t *testing.T) { t.Parallel() - store, err := modelsdev.NewStore() - require.NoError(t, err) + store := modelsdev.NewDatabaseStore(modelsdev.EmbeddedSnapshot()) // Llama doesn't have cache pricing in models.dev supported := detectCachingSupport(t.Context(), "meta.llama3-8b-instruct-v1:0", store) @@ -1404,8 +1402,7 @@ func TestDetectCachingSupport_UnsupportedModel(t *testing.T) { func TestDetectCachingSupport_UnknownModel(t *testing.T) { t.Parallel() - store, err := modelsdev.NewStore() - require.NoError(t, err) + store := modelsdev.NewDatabaseStore(modelsdev.EmbeddedSnapshot()) // Unknown model should gracefully return false, not panic supported := detectCachingSupport(t.Context(), "nonexistent.model.that.does.not.exist:v1", store) diff --git a/pkg/modelsdev/snapshot.go b/pkg/modelsdev/snapshot.go index 430c98f1ec..17f1214837 100644 --- a/pkg/modelsdev/snapshot.go +++ b/pkg/modelsdev/snapshot.go @@ -50,6 +50,19 @@ func embeddedSnapshot() *Database { return snapshotDB } +// EmbeddedSnapshot returns the models.dev catalog committed to the repo at +// pkg/modelsdev/snapshot.json and baked into the binary at build time. It +// never touches the network or the filesystem cache, which makes it the +// right catalog for hermetic tests that must validate against exactly what +// is checked in — e.g. TestParseExamples — rather than whatever the live +// models.dev API happens to say at the instant the test runs. +// +// The returned *Database is a shared, process-wide singleton: callers must +// treat it as read-only. +func EmbeddedSnapshot() *Database { + return embeddedSnapshot() +} + // SnapshotDate returns the time the embedded models.dev snapshot was // generated. The zero value is returned when the recorded date can't be // parsed. diff --git a/pkg/modelsdev/snapshot_test.go b/pkg/modelsdev/snapshot_test.go index a015836b9f..d3db8d684f 100644 --- a/pkg/modelsdev/snapshot_test.go +++ b/pkg/modelsdev/snapshot_test.go @@ -74,6 +74,19 @@ func TestEmbeddedSnapshotParses(t *testing.T) { assert.NotEmpty(t, openai.Models, "openai provider must list models") } +// TestEmbeddedSnapshotExported verifies the exported EmbeddedSnapshot wrapper +// returns the same singleton as the internal embeddedSnapshot accessor, so +// callers that need a guaranteed-hermetic catalog (e.g. NewDatabaseStore in +// tests) get exactly the committed snapshot with no extra copying. +func TestEmbeddedSnapshotExported(t *testing.T) { + t.Parallel() + + db := EmbeddedSnapshot() + require.NotNil(t, db) + assert.NotEmpty(t, db.Providers, "EmbeddedSnapshot must contain providers") + assert.Same(t, embeddedSnapshot(), db, "EmbeddedSnapshot must return the same singleton as embeddedSnapshot") +} + // TestSnapshotDateParses verifies the embedded snapshot date is a valid, // non-zero RFC3339 timestamp. This always runs: a malformed date is a build // artefact bug, not a function of wall-clock time. diff --git a/pkg/modelsdev/store.go b/pkg/modelsdev/store.go index 1cbbd26d9a..20d73bafda 100644 --- a/pkg/modelsdev/store.go +++ b/pkg/modelsdev/store.go @@ -363,6 +363,27 @@ func loadDatabase(ctx context.Context, cacheFile string, allowFetch bool, fetch return database, true } +// Fetch retrieves the models.dev catalog directly from the live API, +// bypassing any on-disk cache or the embedded build-time snapshot. Unlike a +// Store lookup, a failure here is always returned to the caller rather than +// silently degraded to cached or embedded data — callers that need to detect +// live-catalog drift (as opposed to resolving a model as best-effort) should +// use this instead of GetDatabase. +func Fetch(ctx context.Context) (*Database, error) { + db, _, err := fetchFromAPI(ctx, "") + if err != nil { + return nil, err + } + if db == nil { + // fetchFromAPI returns (nil, etag, nil) only for a 304 response, which + // requires a non-empty If-None-Match — unreachable with the empty etag + // passed above. Guard it anyway so a future change to fetchFromAPI + // can't silently turn this into a nil-database success. + return nil, errors.New("models.dev fetch returned no data") + } + return db, nil +} + // fetchFromAPI fetches the models.dev database. // If etag is non-empty it is sent as If-None-Match; a 304 response // returns (nil, etag, nil) to indicate no change.