From 2cbc464c7218d165d98518dcd9a852cd6ea6ae40 Mon Sep 17 00:00:00 2001 From: Ethan <39577870+ethanndickson@users.noreply.github.com> Date: Tue, 7 Jul 2026 00:20:54 +1000 Subject: [PATCH] fix(coderd): invalidate chatd provider cache on AI provider changes (#26987) chatd subscribes to `ChatConfigEventChannel` and invalidates its provider cache on a `providers` event kind, but nothing ever published that kind. AI provider CRUD only publishes on `AIProvidersChangedChannel` (consumed by aibridged and aibridgeproxyd), so chatd's provider cache only converged via its 10 second TTL. Subscribe chatd to the same `AIProvidersChangedChannel` publish instead of adding a second publish, per the review feedback on #26207: one publish, multiple subscribers. The now-unused `ChatConfigEventProviders` kind is removed so `ChatConfigEvent` stays scoped to model configs, user prompts, and advisor config, and can't regrow a dead subscriber. Follow-up to CRF-5 from the review of #25673. Supersedes #26207. Closes CODAGT-499 --- coderd/ai_providers.go | 4 ++-- coderd/pubsub/aiproviderschangedevent.go | 4 ++-- coderd/pubsub/chatconfigevent.go | 9 +++----- coderd/x/chatd/chatd.go | 23 +++++++++++++++++++-- coderd/x/chatd/configcache_internal_test.go | 19 +++++++++++++++++ 5 files changed, 47 insertions(+), 12 deletions(-) diff --git a/coderd/ai_providers.go b/coderd/ai_providers.go index a00e026800..d05c1680af 100644 --- a/coderd/ai_providers.go +++ b/coderd/ai_providers.go @@ -492,8 +492,8 @@ func (api *API) aiProvidersDelete(rw http.ResponseWriter, r *http.Request) { } // publishAIProvidersChanged notifies subscribers (aibridged, -// aibridgeproxyd) that the live provider set changed and they should -// refetch from the database. Pubsub failures are logged but not +// aibridgeproxyd, chatd) that the live provider set changed and they +// should refetch from the database. Pubsub failures are logged but not // propagated: subscribers refresh authoritatively from the DB, so a // dropped notification only delays convergence. func (api *API) publishAIProvidersChanged(ctx context.Context) { diff --git a/coderd/pubsub/aiproviderschangedevent.go b/coderd/pubsub/aiproviderschangedevent.go index a0ff20f960..5d61b3b7fa 100644 --- a/coderd/pubsub/aiproviderschangedevent.go +++ b/coderd/pubsub/aiproviderschangedevent.go @@ -2,8 +2,8 @@ package pubsub // AIProvidersChangedChannel is the pubsub channel that carries AI // provider lifecycle events: provider create / update / soft-delete -// and key insert / delete. Subscribers (aibridged, aibridgeproxyd) -// reload their in-memory provider snapshot on receipt. +// and key insert / delete. Subscribers (aibridged, aibridgeproxyd, +// chatd) reload their in-memory provider snapshot on receipt. // // The payload is an empty invalidation hint; subscribers refetch the // authoritative state from the database, so dropped messages only diff --git a/coderd/pubsub/chatconfigevent.go b/coderd/pubsub/chatconfigevent.go index 734bfb39cc..c5bb5190bf 100644 --- a/coderd/pubsub/chatconfigevent.go +++ b/coderd/pubsub/chatconfigevent.go @@ -9,7 +9,7 @@ import ( ) // ChatConfigEventChannel is the pubsub channel for chat config -// changes (providers, model configs, user prompts, advisor config). +// changes (model configs, user prompts, advisor config). // All replicas subscribe to this channel to invalidate their local // caches. const ChatConfigEventChannel = "chat:config_change" @@ -33,13 +33,11 @@ func HandleChatConfigEvent(cb func(ctx context.Context, payload ChatConfigEvent, } // ChatConfigEvent is published when chat configuration changes -// (provider CRUD, model config CRUD, user prompt updates, or advisor -// config updates). Subscribers use this to invalidate their local -// caches. +// (model config CRUD, user prompt updates, or advisor config +// updates). Subscribers use this to invalidate their local caches. type ChatConfigEvent struct { Kind ChatConfigEventKind `json:"kind"` // EntityID carries context for the invalidation: - // - For providers: uuid.Nil (all providers are invalidated). // - For model configs: the specific config ID. // - For user prompts: the user ID. // - For advisor config: uuid.Nil (singleton site-config row). @@ -49,7 +47,6 @@ type ChatConfigEvent struct { type ChatConfigEventKind string const ( - ChatConfigEventProviders ChatConfigEventKind = "providers" ChatConfigEventModelConfig ChatConfigEventKind = "model_config" ChatConfigEventUserPrompt ChatConfigEventKind = "user_prompt" ChatConfigEventAdvisorConfig ChatConfigEventKind = "advisor_config" diff --git a/coderd/x/chatd/chatd.go b/coderd/x/chatd/chatd.go index 29eabaad24..5c85a2968f 100644 --- a/coderd/x/chatd/chatd.go +++ b/coderd/x/chatd/chatd.go @@ -179,6 +179,7 @@ type Server struct { debugSvcInit sync.Once configCache *chatConfigCache configCacheUnsubscribe func() + providerCacheUnsubscribe func() usageTracker *workspacestats.UsageTracker clock quartz.Clock @@ -3300,8 +3301,6 @@ func New(ps pubsub.Pubsub, cfg Config) *Server { return } switch ev.Kind { - case coderdpubsub.ChatConfigEventProviders: - p.configCache.InvalidateProviders() case coderdpubsub.ChatConfigEventModelConfig: p.configCache.InvalidateModelConfig(ev.EntityID) case coderdpubsub.ChatConfigEventUserPrompt: @@ -3317,6 +3316,22 @@ func New(ps pubsub.Pubsub, cfg Config) *Server { p.configCacheUnsubscribe = cancelConfigSub } + cancelProviderSub, err := p.pubsub.SubscribeWithErr( + coderdpubsub.AIProvidersChangedChannel, + func(cbCtx context.Context, _ []byte, err error) { + if err != nil { + p.logger.Warn(cbCtx, "ai providers changed event error", slog.Error(err)) + return + } + p.configCache.InvalidateProviders() + }, + ) + if err != nil { + p.logger.Error(ctx, "subscribe to ai providers changed events", slog.Error(err)) + } else { + p.providerCacheUnsubscribe = cancelProviderSub + } + p.ctx = ctx // Spawn background goroutines that all servers need. @@ -4903,6 +4918,10 @@ func (p *Server) Close() error { p.configCacheUnsubscribe = nil unsub() } + if unsub := p.providerCacheUnsubscribe; unsub != nil { + p.providerCacheUnsubscribe = nil + unsub() + } if p.chatWorker != nil { if err := p.chatWorker.Close(); err != nil { p.logger.Warn(context.Background(), "failed to close chat worker", slog.Error(err)) diff --git a/coderd/x/chatd/configcache_internal_test.go b/coderd/x/chatd/configcache_internal_test.go index 375a307f94..305e213311 100644 --- a/coderd/x/chatd/configcache_internal_test.go +++ b/coderd/x/chatd/configcache_internal_test.go @@ -14,6 +14,9 @@ import ( "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + coderdpubsub "github.com/coder/coder/v2/coderd/pubsub" + "github.com/coder/coder/v2/coderd/x/chatd/chatprovider" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" "github.com/coder/quartz" @@ -1198,3 +1201,19 @@ func TestConfigCache_InvalidateAdvisorConfig_BlocksStaleInFlight(t *testing.T) { require.EqualValues(t, 2, third.MaxUsesPerRun) require.Equal(t, int32(2), store.advisorConfigCalls.Load()) } + +func TestConfigCache_InvalidatesProvidersOnAIProvidersChangedEvent(t *testing.T) { + t.Parallel() + + db, ps := dbtestutil.NewDB(t) + server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{}) + + // The generation counter only advances through InvalidateProviders, + // so this cannot false-pass via TTL expiry. + gen := server.configCache.providersGeneration() + require.NoError(t, ps.Publish(coderdpubsub.AIProvidersChangedChannel, nil)) + + require.Eventually(t, func() bool { + return server.configCache.providersGeneration() > gen + }, testutil.WaitShort, testutil.IntervalFast) +}