mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(agents): persist system prompt server-side instead of localStorage (#22857)
## Problem The Admin → Agents → System Prompt textarea saved only to the browser's `localStorage`. The value was never sent to the backend, never stored in the database, and never injected into chats. Entering text, clicking Save, and refreshing the page showed no changes — the prompt was effectively a no-op. ## Root Cause Three disconnected layers: 1. **Frontend** wrote to `localStorage`, never called an API. 2. **`handleCreateChat`** never read `savedSystemPrompt`. 3. **Backend** hardcoded `chatd.DefaultSystemPrompt` on every chat creation — no field in `CreateChatRequest` accepted a custom prompt. ## Changes ### Database - Added `GetChatSystemPrompt` / `UpsertChatSystemPrompt` queries on the existing `site_configs` table (no migration needed). ### API - `GET /api/experimental/chats/system-prompt` — returns the configured prompt (any authenticated user). - `PUT /api/experimental/chats/system-prompt` — sets the prompt (admin-only, `rbac: deployment_config update`). - Input validation: max 32 KiB prompt length. ### Backend - `resolvedChatSystemPrompt(ctx)` checks for a custom prompt in the DB, falls back to `chatd.DefaultSystemPrompt` when empty/unset. - Logs a warning on DB errors instead of silently swallowing them. - Replaced the hardcoded `defaultChatSystemPrompt()` call in chat creation. ### Frontend - Replaced `localStorage` read/write with React Query `useQuery`/`useMutation` backed by the new endpoints. - Fixed `useEffect` draft sync to avoid clobbering in-progress user edits on refetch. - Added `try/catch` error handling on save (draft stays dirty for retry). - Save button disabled during mutation (`isSavingSystemPrompt`). - Query key follows kebab-case convention (`chat-system-prompt`). ### UX - Added hint: "When empty, the built-in default prompt is used." ### Tests - `TestChatSystemPrompt`: GET returns empty when unset, admin can set, non-admin gets 403. - dbauthz `TestMethodTestSuite` coverage for both new querier methods.
This commit is contained in:
@@ -2564,6 +2564,18 @@ func (q *querier) GetChatQueuedMessages(ctx context.Context, chatID uuid.UUID) (
|
||||
return q.db.GetChatQueuedMessages(ctx, chatID)
|
||||
}
|
||||
|
||||
func (q *querier) GetChatSystemPrompt(ctx context.Context) (string, error) {
|
||||
// The system prompt is a deployment-wide setting read during chat
|
||||
// creation by every authenticated user, so no RBAC policy check
|
||||
// is needed. We still verify that a valid actor exists in the
|
||||
// context to ensure this is never callable by an unauthenticated
|
||||
// or system-internal path without an explicit actor.
|
||||
if _, ok := ActorFromContext(ctx); !ok {
|
||||
return "", ErrNoActor
|
||||
}
|
||||
return q.db.GetChatSystemPrompt(ctx)
|
||||
}
|
||||
|
||||
func (q *querier) GetChatsByOwnerID(ctx context.Context, ownerID database.GetChatsByOwnerIDParams) ([]database.Chat, error) {
|
||||
return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetChatsByOwnerID)(ctx, ownerID)
|
||||
}
|
||||
@@ -6536,6 +6548,13 @@ func (q *querier) UpsertChatDiffStatusReference(ctx context.Context, arg databas
|
||||
return q.db.UpsertChatDiffStatusReference(ctx, arg)
|
||||
}
|
||||
|
||||
func (q *querier) UpsertChatSystemPrompt(ctx context.Context, value string) error {
|
||||
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceDeploymentConfig); err != nil {
|
||||
return err
|
||||
}
|
||||
return q.db.UpsertChatSystemPrompt(ctx, value)
|
||||
}
|
||||
|
||||
func (q *querier) UpsertConnectionLog(ctx context.Context, arg database.UpsertConnectionLogParams) (database.ConnectionLog, error) {
|
||||
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceConnectionLog); err != nil {
|
||||
return database.ConnectionLog{}, err
|
||||
|
||||
@@ -551,6 +551,10 @@ func (s *MethodTestSuite) TestChats() {
|
||||
dbm.EXPECT().GetChatQueuedMessages(gomock.Any(), chat.ID).Return(qms, nil).AnyTimes()
|
||||
check.Args(chat.ID).Asserts(chat, policy.ActionRead).Returns(qms)
|
||||
}))
|
||||
s.Run("GetChatSystemPrompt", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) {
|
||||
dbm.EXPECT().GetChatSystemPrompt(gomock.Any()).Return("prompt", nil).AnyTimes()
|
||||
check.Args().Asserts()
|
||||
}))
|
||||
s.Run("GetEnabledChatModelConfigs", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
|
||||
configA := testutil.Fake(s.T(), faker, database.ChatModelConfig{})
|
||||
configB := testutil.Fake(s.T(), faker, database.ChatModelConfig{})
|
||||
@@ -758,6 +762,10 @@ func (s *MethodTestSuite) TestChats() {
|
||||
dbm.EXPECT().UpsertChatDiffStatusReference(gomock.Any(), arg).Return(diffStatus, nil).AnyTimes()
|
||||
check.Args(arg).Asserts(chat, policy.ActionUpdate).Returns(diffStatus)
|
||||
}))
|
||||
s.Run("UpsertChatSystemPrompt", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) {
|
||||
dbm.EXPECT().UpsertChatSystemPrompt(gomock.Any(), "").Return(nil).AnyTimes()
|
||||
check.Args("").Asserts(rbac.ResourceDeploymentConfig, policy.ActionUpdate)
|
||||
}))
|
||||
}
|
||||
|
||||
func (s *MethodTestSuite) TestFile() {
|
||||
|
||||
@@ -1103,6 +1103,14 @@ func (m queryMetricsStore) GetChatQueuedMessages(ctx context.Context, chatID uui
|
||||
return r0, r1
|
||||
}
|
||||
|
||||
func (m queryMetricsStore) GetChatSystemPrompt(ctx context.Context) (string, error) {
|
||||
start := time.Now()
|
||||
r0, r1 := m.s.GetChatSystemPrompt(ctx)
|
||||
m.queryLatencies.WithLabelValues("GetChatSystemPrompt").Observe(time.Since(start).Seconds())
|
||||
m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "GetChatSystemPrompt").Inc()
|
||||
return r0, r1
|
||||
}
|
||||
|
||||
func (m queryMetricsStore) GetChatsByOwnerID(ctx context.Context, ownerID database.GetChatsByOwnerIDParams) ([]database.Chat, error) {
|
||||
start := time.Now()
|
||||
r0, r1 := m.s.GetChatsByOwnerID(ctx, ownerID)
|
||||
@@ -4526,6 +4534,14 @@ func (m queryMetricsStore) UpsertChatDiffStatusReference(ctx context.Context, ar
|
||||
return r0, r1
|
||||
}
|
||||
|
||||
func (m queryMetricsStore) UpsertChatSystemPrompt(ctx context.Context, value string) error {
|
||||
start := time.Now()
|
||||
r0 := m.s.UpsertChatSystemPrompt(ctx, value)
|
||||
m.queryLatencies.WithLabelValues("UpsertChatSystemPrompt").Observe(time.Since(start).Seconds())
|
||||
m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "UpsertChatSystemPrompt").Inc()
|
||||
return r0
|
||||
}
|
||||
|
||||
func (m queryMetricsStore) UpsertConnectionLog(ctx context.Context, arg database.UpsertConnectionLogParams) (database.ConnectionLog, error) {
|
||||
start := time.Now()
|
||||
r0, r1 := m.s.UpsertConnectionLog(ctx, arg)
|
||||
|
||||
@@ -2017,6 +2017,21 @@ func (mr *MockStoreMockRecorder) GetChatQueuedMessages(ctx, chatID any) *gomock.
|
||||
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetChatQueuedMessages", reflect.TypeOf((*MockStore)(nil).GetChatQueuedMessages), ctx, chatID)
|
||||
}
|
||||
|
||||
// GetChatSystemPrompt mocks base method.
|
||||
func (m *MockStore) GetChatSystemPrompt(ctx context.Context) (string, error) {
|
||||
m.ctrl.T.Helper()
|
||||
ret := m.ctrl.Call(m, "GetChatSystemPrompt", ctx)
|
||||
ret0, _ := ret[0].(string)
|
||||
ret1, _ := ret[1].(error)
|
||||
return ret0, ret1
|
||||
}
|
||||
|
||||
// GetChatSystemPrompt indicates an expected call of GetChatSystemPrompt.
|
||||
func (mr *MockStoreMockRecorder) GetChatSystemPrompt(ctx any) *gomock.Call {
|
||||
mr.mock.ctrl.T.Helper()
|
||||
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetChatSystemPrompt", reflect.TypeOf((*MockStore)(nil).GetChatSystemPrompt), ctx)
|
||||
}
|
||||
|
||||
// GetChatsByOwnerID mocks base method.
|
||||
func (m *MockStore) GetChatsByOwnerID(ctx context.Context, arg database.GetChatsByOwnerIDParams) ([]database.Chat, error) {
|
||||
m.ctrl.T.Helper()
|
||||
@@ -8463,6 +8478,20 @@ func (mr *MockStoreMockRecorder) UpsertChatDiffStatusReference(ctx, arg any) *go
|
||||
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertChatDiffStatusReference", reflect.TypeOf((*MockStore)(nil).UpsertChatDiffStatusReference), ctx, arg)
|
||||
}
|
||||
|
||||
// UpsertChatSystemPrompt mocks base method.
|
||||
func (m *MockStore) UpsertChatSystemPrompt(ctx context.Context, value string) error {
|
||||
m.ctrl.T.Helper()
|
||||
ret := m.ctrl.Call(m, "UpsertChatSystemPrompt", ctx, value)
|
||||
ret0, _ := ret[0].(error)
|
||||
return ret0
|
||||
}
|
||||
|
||||
// UpsertChatSystemPrompt indicates an expected call of UpsertChatSystemPrompt.
|
||||
func (mr *MockStoreMockRecorder) UpsertChatSystemPrompt(ctx, value any) *gomock.Call {
|
||||
mr.mock.ctrl.T.Helper()
|
||||
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertChatSystemPrompt", reflect.TypeOf((*MockStore)(nil).UpsertChatSystemPrompt), ctx, value)
|
||||
}
|
||||
|
||||
// UpsertConnectionLog mocks base method.
|
||||
func (m *MockStore) UpsertConnectionLog(ctx context.Context, arg database.UpsertConnectionLogParams) (database.ConnectionLog, error) {
|
||||
m.ctrl.T.Helper()
|
||||
|
||||
@@ -230,6 +230,7 @@ type sqlcQuerier interface {
|
||||
GetChatProviderByProvider(ctx context.Context, provider string) (ChatProvider, error)
|
||||
GetChatProviders(ctx context.Context) ([]ChatProvider, error)
|
||||
GetChatQueuedMessages(ctx context.Context, chatID uuid.UUID) ([]ChatQueuedMessage, error)
|
||||
GetChatSystemPrompt(ctx context.Context) (string, error)
|
||||
GetChatsByOwnerID(ctx context.Context, arg GetChatsByOwnerIDParams) ([]Chat, error)
|
||||
GetConnectionLogsOffset(ctx context.Context, arg GetConnectionLogsOffsetParams) ([]GetConnectionLogsOffsetRow, error)
|
||||
GetCoordinatorResumeTokenSigningKey(ctx context.Context) (string, error)
|
||||
@@ -840,6 +841,7 @@ type sqlcQuerier interface {
|
||||
UpsertBoundaryUsageStats(ctx context.Context, arg UpsertBoundaryUsageStatsParams) (bool, error)
|
||||
UpsertChatDiffStatus(ctx context.Context, arg UpsertChatDiffStatusParams) (ChatDiffStatus, error)
|
||||
UpsertChatDiffStatusReference(ctx context.Context, arg UpsertChatDiffStatusReferenceParams) (ChatDiffStatus, error)
|
||||
UpsertChatSystemPrompt(ctx context.Context, value string) error
|
||||
UpsertConnectionLog(ctx context.Context, arg UpsertConnectionLogParams) (ConnectionLog, error)
|
||||
UpsertCoordinatorResumeTokenSigningKey(ctx context.Context, value string) error
|
||||
// The default proxy is implied and not actually stored in the database.
|
||||
|
||||
@@ -14475,6 +14475,18 @@ func (q *sqlQuerier) GetApplicationName(ctx context.Context) (string, error) {
|
||||
return value, err
|
||||
}
|
||||
|
||||
const getChatSystemPrompt = `-- name: GetChatSystemPrompt :one
|
||||
SELECT
|
||||
COALESCE((SELECT value FROM site_configs WHERE key = 'agents_chat_system_prompt'), '') :: text AS chat_system_prompt
|
||||
`
|
||||
|
||||
func (q *sqlQuerier) GetChatSystemPrompt(ctx context.Context) (string, error) {
|
||||
row := q.db.QueryRowContext(ctx, getChatSystemPrompt)
|
||||
var chat_system_prompt string
|
||||
err := row.Scan(&chat_system_prompt)
|
||||
return chat_system_prompt, err
|
||||
}
|
||||
|
||||
const getCoordinatorResumeTokenSigningKey = `-- name: GetCoordinatorResumeTokenSigningKey :one
|
||||
SELECT value FROM site_configs WHERE key = 'coordinator_resume_token_signing_key'
|
||||
`
|
||||
@@ -14689,6 +14701,16 @@ func (q *sqlQuerier) UpsertApplicationName(ctx context.Context, value string) er
|
||||
return err
|
||||
}
|
||||
|
||||
const upsertChatSystemPrompt = `-- name: UpsertChatSystemPrompt :exec
|
||||
INSERT INTO site_configs (key, value) VALUES ('agents_chat_system_prompt', $1)
|
||||
ON CONFLICT (key) DO UPDATE SET value = $1 WHERE site_configs.key = 'agents_chat_system_prompt'
|
||||
`
|
||||
|
||||
func (q *sqlQuerier) UpsertChatSystemPrompt(ctx context.Context, value string) error {
|
||||
_, err := q.db.ExecContext(ctx, upsertChatSystemPrompt, value)
|
||||
return err
|
||||
}
|
||||
|
||||
const upsertCoordinatorResumeTokenSigningKey = `-- name: UpsertCoordinatorResumeTokenSigningKey :exec
|
||||
INSERT INTO site_configs (key, value) VALUES ('coordinator_resume_token_signing_key', $1)
|
||||
ON CONFLICT (key) DO UPDATE set value = $1 WHERE site_configs.key = 'coordinator_resume_token_signing_key'
|
||||
|
||||
@@ -153,3 +153,11 @@ DO UPDATE SET value = EXCLUDED.value WHERE site_configs.key = EXCLUDED.key;
|
||||
SELECT
|
||||
COALESCE((SELECT value FROM site_configs WHERE key = 'webpush_vapid_public_key'), '') :: text AS vapid_public_key,
|
||||
COALESCE((SELECT value FROM site_configs WHERE key = 'webpush_vapid_private_key'), '') :: text AS vapid_private_key;
|
||||
|
||||
-- name: GetChatSystemPrompt :one
|
||||
SELECT
|
||||
COALESCE((SELECT value FROM site_configs WHERE key = 'agents_chat_system_prompt'), '') :: text AS chat_system_prompt;
|
||||
|
||||
-- name: UpsertChatSystemPrompt :exec
|
||||
INSERT INTO site_configs (key, value) VALUES ('agents_chat_system_prompt', $1)
|
||||
ON CONFLICT (key) DO UPDATE SET value = $1 WHERE site_configs.key = 'agents_chat_system_prompt';
|
||||
|
||||
Reference in New Issue
Block a user