From 99e92384e0795af3aa51a30deb1aa354f3e70595 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arnaud=20H=C3=A9ritier?= Date: Thu, 3 Sep 2026 14:49:49 +0000 Subject: [PATCH 1/3] feat(modelsdev): add EmbeddedSnapshot and Fetch helpers EmbeddedSnapshot exposes the build-time snapshot.json catalog as a read-only singleton, and Fetch hits the live models.dev API directly, surfacing a network failure instead of silently degrading to a cache or the embedded snapshot. Together with the existing NewDatabaseStore these let callers build a Store that is either provably hermetic or provably live, instead of Store's usual best-effort network-then-cache-then-snapshot fallback. Groundwork for #4134. --- pkg/modelsdev/snapshot.go | 13 +++++++++++++ pkg/modelsdev/snapshot_test.go | 13 +++++++++++++ pkg/modelsdev/store.go | 21 +++++++++++++++++++++ 3 files changed, 47 insertions(+) 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. From 1a6d75c74b61015868df513f3e0b5aefbdb6d0b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arnaud=20H=C3=A9ritier?= Date: Thu, 3 Sep 2026 14:49:58 +0000 Subject: [PATCH 2/3] test: validate examples against the committed models.dev snapshot only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TestParseExamples used to resolve model references via modelsdev.NewStore(), which tries a live HTTP fetch to models.dev before falling back to the embedded snapshot. On CI's normal internet egress this meant the test validated against whatever the live catalog said at the instant it ran, not against the snapshot committed to the repo — so an unrelated, unmodified PR could start failing hours after a green run once the live catalog moved on (observed on #4121). TestParseExamples now resolves against modelsdev.NewDatabaseStore(modelsdev.EmbeddedSnapshot()) only, so it is hermetic and safe as a required, PR-blocking check: it can only fail when the PR's own diff introduces an inconsistency. The shared skip-and-resolve logic is extracted into catalogModelRefs() and reused by a new, opt-in TestExamplesAgainstLiveModelsDev (gated on CHECK_MODELS_DEV_LIVE, mirroring the existing CHECK_MODELS_SNAPSHOT_FRESHNESS pattern), which hits the live API and reports drift as explicitly external, not PR-caused. The bedrock TestDetectCachingSupport_* tests had the identical live NewStore() bug; switched to the same hermetic store for the same reason. Fixes #4134. --- pkg/config/examples_test.go | 100 ++++++++++++++++++---- pkg/model/provider/bedrock/client_test.go | 11 +-- 2 files changed, 89 insertions(+), 22 deletions(-) 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) From dff439fbedb70f7ad42831de8b7edc9c62aaa36f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arnaud=20H=C3=A9ritier?= Date: Thu, 3 Sep 2026 14:50:06 +0000 Subject: [PATCH 3/3] ci: add scheduled models.dev live-catalog drift check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `task check-models-live` and a new, non-blocking models-live-check workflow (workflow_dispatch + a daily schedule) that runs TestExamplesAgainstLiveModelsDev against the real models.dev API. Deliberately not part of ci.yml or any required check: a failure here means the external catalog drifted, which can strike any PR (or none) at any time and gives the PR author no actionable signal — see #4134. The failure step appends a summary explaining that explicitly. --- .github/workflows/models-live-check.yml | 59 +++++++++++++++++++++++++ Taskfile.yml | 5 +++ 2 files changed, 64 insertions(+) create mode 100644 .github/workflows/models-live-check.yml 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: