Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions coderd/apidoc/docs.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions coderd/apidoc/swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 17 additions & 2 deletions coderd/audit/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,19 @@ func NewMock() *MockAuditor {
return &MockAuditor{}
}

// NewMockWithDiffFn returns a MockAuditor whose entries carry diffs computed
// by the supplied function instead of the default empty diff. Tests use it to
// pin the Old/New pair a handler captured, which the empty diff cannot
// express. The function is test-supplied comparison logic; the mock never
// calls the production differ.
func NewMockWithDiffFn(fn func(old, newVal any) Map) *MockAuditor {
return &MockAuditor{diffFn: fn}
}

type MockAuditor struct {
mutex sync.Mutex
auditLogs []database.AuditLog
diffFn func(old, newVal any) Map
}

// ResetLogs removes all audit logs from the mock auditor.
Expand All @@ -70,8 +80,13 @@ func (a *MockAuditor) Export(_ context.Context, alog database.AuditLog) error {
return nil
}

func (*MockAuditor) diff(any, any) Map {
return Map{}
func (a *MockAuditor) diff(old, newVal any) Map {
a.mutex.Lock()
defer a.mutex.Unlock()
if a.diffFn == nil {
return Map{}
}
return a.diffFn(old, newVal)
}

// Contains returns true if, for each non-zero-valued field in expected,
Expand Down
3 changes: 2 additions & 1 deletion coderd/audit/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ type Auditable interface {
database.AuditableGroupAIBudget |
database.AuditableUserAIBudgetOverride |
database.UserSecret |
database.UserSkill
database.UserSkill |
database.ChatInstructionSettings
}

// Map is a map of changed fields in an audited resource. It maps field names to
Expand Down
26 changes: 26 additions & 0 deletions coderd/audit/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ func ResourceTarget[T Auditable](tgt T) string {
return typed.Name
case database.UserSkill:
return typed.Name
case database.ChatInstructionSettings:
return typed.Name
default:
panic(fmt.Sprintf("unknown resource %T for ResourceTarget", tgt))
}
Expand All @@ -168,6 +170,22 @@ func ResourceTarget[T Auditable](tgt T) string {
// 51A51C = "Static"
var noID = uuid.MustParse("51A51C00-0000-0000-0000-000000000000")

// Fixed IDs for the two chat instruction settings. History-by-setting works
// only if every change to one setting carries the same resource ID, so
// unlike the per-write artificial IDs of the other settings singletons,
// these never change. C1A7 = "Chat".
var (
ChatInstructionSystemPromptID = uuid.MustParse("C1A715C0-0000-0000-0000-000000000001")
ChatInstructionPlanModeID = uuid.MustParse("C1A715C0-0000-0000-0000-000000000002")
)

// Human-readable targets for the two chat instruction settings, so an audit
// row names the setting it concerns.
const (
ChatInstructionSystemPromptName = "System prompt"
ChatInstructionPlanModeName = "Plan mode instructions"
)

func ResourceID[T Auditable](tgt T) uuid.UUID {
switch typed := any(tgt).(type) {
case database.Template:
Expand Down Expand Up @@ -243,6 +261,9 @@ func ResourceID[T Auditable](tgt T) uuid.UUID {
return typed.ID
case database.UserSkill:
return typed.ID
case database.ChatInstructionSettings:
// Fixed ID per setting; see ChatInstructionSettings IDs.
return typed.ID
default:
panic(fmt.Sprintf("unknown resource %T for ResourceID", tgt))
}
Expand Down Expand Up @@ -318,6 +339,8 @@ func ResourceType[T Auditable](tgt T) database.ResourceType {
return database.ResourceTypeUserSecret
case database.UserSkill:
return database.ResourceTypeUserSkill
case database.ChatInstructionSettings:
return database.ResourceTypeChatInstructionSettings
default:
panic(fmt.Sprintf("unknown resource %T for ResourceType", typed))
}
Expand Down Expand Up @@ -408,6 +431,9 @@ func ResourceRequiresOrgID[T Auditable]() bool {
case database.UserSkill:
// User skills are global to the user across organizations.
return false
case database.ChatInstructionSettings:
// Deployment settings, not scoped to any organization.
return false
default:
panic(fmt.Sprintf("unknown resource %T for ResourceRequiresOrgID", tgt))
}
Expand Down
3 changes: 2 additions & 1 deletion coderd/database/dump.sql

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions coderd/database/lock.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,17 @@ const (
LockIDChatCapacityAdmission
)

// Per-setting advisory lock IDs for the chat instruction settings. These
// derive from the exact site_configs key with GenLockID (FNV-1a 64) instead
// of the sequential LockID* block above, so writers of different settings
// never contend and the IDs cannot collide with any sequentially allocated
// lock ID (different derivation space) or with another subsystem's
// GenLockID output (the key strings are unique to these settings).
var (
LockIDChatInstructionSystemPrompt = GenLockID("agents_chat_system_prompt")
LockIDChatInstructionPlanMode = GenLockID("agents_chat_plan_mode_instructions")
)

// GenLockID generates a unique and consistent lock ID from a given string.
func GenLockID(name string) int64 {
hash := fnv.New64()
Expand Down
50 changes: 50 additions & 0 deletions coderd/database/lock_internal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package database

import (
"testing"

"github.com/stretchr/testify/require"
)

// TestChatInstructionLockIDsDistinct proves the per-setting advisory lock IDs
// for the chat instruction settings cannot collide with each other or with
// any sequentially allocated LockID* constant. The constants are listed
// explicitly rather than enumerated programmatically (there is no registry of
// iota constants), so a future LockID* addition that collides fails here in
// review, not in a production deadlock.
func TestChatInstructionLockIDsDistinct(t *testing.T) {
t.Parallel()

generated := map[string]int64{
"LockIDChatInstructionSystemPrompt": LockIDChatInstructionSystemPrompt,
"LockIDChatInstructionPlanMode": LockIDChatInstructionPlanMode,
}

sequential := map[string]int64{
"LockIDDeploymentSetup": LockIDDeploymentSetup,
"LockIDEnterpriseDeploymentSetup": LockIDEnterpriseDeploymentSetup,
"LockIDDBRollup": LockIDDBRollup,
"LockIDDBPurge": LockIDDBPurge,
"LockIDNotificationsReportGenerator": LockIDNotificationsReportGenerator,
"LockIDCryptoKeyRotation": LockIDCryptoKeyRotation,
"LockIDReconcilePrebuilds": LockIDReconcilePrebuilds,
"LockIDReconcileSystemRoles": LockIDReconcileSystemRoles,
"LockIDBoundaryUsageStats": LockIDBoundaryUsageStats,
"LockIDAIProvidersEnvSeed": LockIDAIProvidersEnvSeed,
"LockIDChatModelConfigWrites": LockIDChatModelConfigWrites,
}

// The two generated IDs are pairwise distinct.
require.NotEqual(t,
LockIDChatInstructionSystemPrompt,
LockIDChatInstructionPlanMode,
"per-setting lock IDs must differ from each other")

// Neither generated ID collides with any sequential constant.
for name, id := range generated {
for seqName, seqID := range sequential {
require.NotEqualf(t, seqID, id,
"%s (%d) collides with sequential constant %s", name, id, seqName)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-- No-op, enum values can't be dropped.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TYPE resource_type
ADD VALUE IF NOT EXISTS 'chat_instruction_settings';
5 changes: 4 additions & 1 deletion coderd/database/models.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 10 additions & 4 deletions coderd/database/queries.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion coderd/database/queries/siteconfig.sql
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,12 @@ SELECT
WHERE key = 'agents_chat_system_prompt'
AND value != ''
)
) :: boolean AS include_default_system_prompt;
) :: boolean AS include_default_system_prompt,
EXISTS (
SELECT 1
FROM site_configs
WHERE key = 'agents_chat_include_default_system_prompt'
) :: boolean AS include_default_system_prompt_set;

-- name: UpsertChatSystemPrompt :exec
INSERT INTO site_configs (key, value) VALUES ('agents_chat_system_prompt', $1)
Expand Down
21 changes: 21 additions & 0 deletions coderd/database/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,27 @@ type OAuth2ProviderSettings struct {
DynamicClientRegistrationEnabled bool `db:"dynamic_client_registration_enabled" json:"dynamic_client_registration_enabled"`
}

// ChatInstructionSettings is the auditable shape of the deployment-wide
// chat instruction configuration, stored across the
// agents_chat_system_prompt, agents_chat_include_default_system_prompt and
// agents_chat_plan_mode_instructions site_configs keys. Both the
// system-prompt and plan-mode-instructions endpoints audit this one type;
// each populates only the fields its endpoint can change.
type ChatInstructionSettings struct {
ID uuid.UUID `db:"id" json:"id"`
// Name identifies which setting an audit row concerns (e.g. "System
// prompt"). It is ignored in diffs and set identically on Old and New.
Name string `db:"name" json:"name"`
SystemPrompt string `db:"system_prompt" json:"system_prompt"`
// IncludeDefaultSystemPromptSet records whether the override row
// exists, not only its effective value: writing explicit false over a
// legacy absent row does not move the effective value but changes
// future behavior, so presence must enter the diff.
IncludeDefaultSystemPromptSet bool `db:"include_default_system_prompt_set" json:"include_default_system_prompt_set"`
IncludeDefaultSystemPrompt bool `db:"include_default_system_prompt" json:"include_default_system_prompt"`
PlanModeInstructions string `db:"plan_mode_instructions" json:"plan_mode_instructions"`
}

type Actions []policy.Action

func (a *Actions) Scan(src interface{}) error {
Expand Down
Loading
Loading