From 58f295059c4fac0ccca9a3540dc9f172f2c5841c Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 11 Mar 2026 15:07:46 -0700 Subject: [PATCH] fix: grant chatd ActionReadPersonal on User and parallelize runChat DB calls (#22970) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem 1. **Personal behavior prompt not applied**: The chatd background worker was missing `ActionReadPersonal` on `ResourceUser` in its RBAC subject. When `resolveUserPrompt` calls `GetUserChatCustomPrompt`, the dbauthz layer checks `ActionReadPersonal` on the user — which the chatd role didn't have. The error was silently swallowed (returns `""`), so the user's custom prompt was never injected into the system messages. 2. **Sequential DB calls on chat startup**: Several independent database queries in `runChat` and `resolveChatModel` were running sequentially, adding unnecessary latency before the LLM stream begins. ## Changes ### RBAC fix (`dbauthz.go`) - Add `rbac.ResourceUser.Type: {policy.ActionReadPersonal}` to `subjectChatd` site permissions - This is the minimal permission needed — `ActionRead` on User remains denied ### Parallelization (`chatd.go`) Three parallelization points using `errgroup.Group`: 1. **`resolveChatModel`**: `resolveModelConfig` and `GetEnabledChatProviders` run concurrently (both needed for `ModelFromConfig`, which stays sequential after the wait) 2. **`runChat` startup**: `resolveChatModel` and `GetChatMessagesForPromptByChatID` run concurrently (completely independent) 3. **`runChat` prompt assembly**: `resolveInstructions` and `resolveUserPrompt` run concurrently (both produce strings; `InsertSystem` calls maintain correct order after the wait) Same pattern applied to the `ReloadMessages` callback. ### Test (`dbauthz_test.go`) - Add assertion in `TestAsChatd/AllowedActions` that `ActionReadPersonal` on `ResourceUser` is permitted --- coderd/chatd/chatd.go | 116 +++++++++++++++++------- coderd/database/dbauthz/dbauthz.go | 1 + coderd/database/dbauthz/dbauthz_test.go | 4 + 3 files changed, 90 insertions(+), 31 deletions(-) diff --git a/coderd/chatd/chatd.go b/coderd/chatd/chatd.go index 5a8f0aaa14..de3c2676b9 100644 --- a/coderd/chatd/chatd.go +++ b/coderd/chatd/chatd.go @@ -15,6 +15,7 @@ import ( "charm.land/fantasy/providers/anthropic" "github.com/google/uuid" "github.com/sqlc-dev/pqtype" + "golang.org/x/sync/errgroup" "golang.org/x/xerrors" "cdr.dev/slog/v3" @@ -2100,21 +2101,38 @@ func (p *Server) runChat( chat database.Chat, logger slog.Logger, ) error { - model, modelConfig, providerKeys, err := p.resolveChatModel(ctx, chat) - if err != nil { - return err - } + var ( + model fantasy.LanguageModel + modelConfig database.ChatModelConfig + providerKeys chatprovider.ProviderAPIKeys + callConfig codersdk.ChatModelCallConfig + messages []database.ChatMessage + ) - var callConfig codersdk.ChatModelCallConfig - if len(modelConfig.Options) > 0 { - if err := json.Unmarshal(modelConfig.Options, &callConfig); err != nil { - return xerrors.Errorf("parse model call config: %w", err) + var g errgroup.Group + g.Go(func() error { + var err error + model, modelConfig, providerKeys, err = p.resolveChatModel(ctx, chat) + if err != nil { + return err } - } - - messages, err := p.db.GetChatMessagesForPromptByChatID(ctx, chat.ID) - if err != nil { - return xerrors.Errorf("get chat messages: %w", err) + if len(modelConfig.Options) > 0 { + if err := json.Unmarshal(modelConfig.Options, &callConfig); err != nil { + return xerrors.Errorf("parse model call config: %w", err) + } + } + return nil + }) + g.Go(func() error { + var err error + messages, err = p.db.GetChatMessagesForPromptByChatID(ctx, chat.ID) + if err != nil { + return xerrors.Errorf("get chat messages: %w", err) + } + return nil + }) + if err := g.Wait(); err != nil { + return err } // Fire title generation asynchronously so it doesn't block the // chat response. It uses a detached context so it can finish @@ -2237,11 +2255,23 @@ func (p *Server) runChat( return currentConn, nil } - if instruction := p.resolveInstructions(ctx, chat, getWorkspaceConn); instruction != "" { + var instruction, resolvedUserPrompt string + var g2 errgroup.Group + g2.Go(func() error { + instruction = p.resolveInstructions(ctx, chat, getWorkspaceConn) + return nil + }) + g2.Go(func() error { + resolvedUserPrompt = p.resolveUserPrompt(ctx, chat.OwnerID) + return nil + }) + _ = g2.Wait() + + if instruction != "" { prompt = chatprompt.InsertSystem(prompt, instruction) } - if userPrompt := p.resolveUserPrompt(ctx, chat.OwnerID); userPrompt != "" { - prompt = chatprompt.InsertSystem(prompt, userPrompt) + if resolvedUserPrompt != "" { + prompt = chatprompt.InsertSystem(prompt, resolvedUserPrompt) } // Use the model config's context_limit as a fallback when the LLM // provider doesn't include context_limit in its response metadata @@ -2532,11 +2562,23 @@ func (p *Server) runChat( if chat.ParentChatID.Valid { reloadedPrompt = chatprompt.InsertSystem(reloadedPrompt, defaultSubagentInstruction) } - if instruction := p.resolveInstructions(reloadCtx, chat, getWorkspaceConn); instruction != "" { - reloadedPrompt = chatprompt.InsertSystem(reloadedPrompt, instruction) + var reloadInstruction, reloadUserPrompt string + var rg errgroup.Group + rg.Go(func() error { + reloadInstruction = p.resolveInstructions(reloadCtx, chat, getWorkspaceConn) + return nil + }) + rg.Go(func() error { + reloadUserPrompt = p.resolveUserPrompt(reloadCtx, chat.OwnerID) + return nil + }) + _ = rg.Wait() + + if reloadInstruction != "" { + reloadedPrompt = chatprompt.InsertSystem(reloadedPrompt, reloadInstruction) } - if userPrompt := p.resolveUserPrompt(reloadCtx, chat.OwnerID); userPrompt != "" { - reloadedPrompt = chatprompt.InsertSystem(reloadedPrompt, userPrompt) + if reloadUserPrompt != "" { + reloadedPrompt = chatprompt.InsertSystem(reloadedPrompt, reloadUserPrompt) } return reloadedPrompt, nil }, @@ -2764,18 +2806,30 @@ func (p *Server) resolveChatModel( ctx context.Context, chat database.Chat, ) (fantasy.LanguageModel, database.ChatModelConfig, chatprovider.ProviderAPIKeys, error) { - dbConfig, err := p.resolveModelConfig(ctx, chat) - if err != nil { - return nil, database.ChatModelConfig{}, chatprovider.ProviderAPIKeys{}, xerrors.Errorf( - "resolve model config: %w", err, - ) - } + var ( + dbConfig database.ChatModelConfig + providers []database.ChatProvider + ) - providers, err := p.db.GetEnabledChatProviders(ctx) - if err != nil { - return nil, database.ChatModelConfig{}, chatprovider.ProviderAPIKeys{}, xerrors.Errorf( - "get enabled chat providers: %w", err, - ) + var g errgroup.Group + g.Go(func() error { + var err error + dbConfig, err = p.resolveModelConfig(ctx, chat) + if err != nil { + return xerrors.Errorf("resolve model config: %w", err) + } + return nil + }) + g.Go(func() error { + var err error + providers, err = p.db.GetEnabledChatProviders(ctx) + if err != nil { + return xerrors.Errorf("get enabled chat providers: %w", err) + } + return nil + }) + if err := g.Wait(); err != nil { + return nil, database.ChatModelConfig{}, chatprovider.ProviderAPIKeys{}, err } dbProviders := make( []chatprovider.ConfiguredProvider, 0, len(providers), diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 36f963abdb..50d4c9c36f 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -707,6 +707,7 @@ var ( rbac.ResourceChat.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, rbac.ResourceWorkspace.Type: {policy.ActionRead}, rbac.ResourceDeploymentConfig.Type: {policy.ActionRead}, + rbac.ResourceUser.Type: {policy.ActionReadPersonal}, }), User: []rbac.Permission{}, ByOrgID: map[string]rbac.OrgPermissions{}, diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 5a7e47c2af..a765909dad 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -5459,6 +5459,10 @@ func TestAsChatd(t *testing.T) { // DeploymentConfig read. err = auth.Authorize(ctx, actor, policy.ActionRead, rbac.ResourceDeploymentConfig) require.NoError(t, err, "deployment config read should be allowed") + + // User read_personal (needed for GetUserChatCustomPrompt). + err = auth.Authorize(ctx, actor, policy.ActionReadPersonal, rbac.ResourceUser) + require.NoError(t, err, "user read_personal should be allowed") }) t.Run("DeniedActions", func(t *testing.T) {