mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: gate chat advisor and virtual desktop behind experiments, delete experiments page (#26809)
This commit is contained in:
@@ -190,6 +190,7 @@ type Server struct {
|
||||
|
||||
aibridgeTransportFactory *atomic.Pointer[aibridge.TransportFactory]
|
||||
aiGatewayRoutingEnabled bool
|
||||
experiments codersdk.Experiments
|
||||
|
||||
// Configuration
|
||||
pendingChatAcquireInterval time.Duration
|
||||
@@ -3166,6 +3167,7 @@ type Config struct {
|
||||
Clock quartz.Clock
|
||||
AIBridgeTransportFactory *atomic.Pointer[aibridge.TransportFactory]
|
||||
AIGatewayRoutingEnabled bool
|
||||
Experiments codersdk.Experiments
|
||||
|
||||
PrometheusRegistry prometheus.Registerer
|
||||
|
||||
@@ -3262,6 +3264,7 @@ func New(ps pubsub.Pubsub, cfg Config) *Server {
|
||||
},
|
||||
aibridgeTransportFactory: cfg.AIBridgeTransportFactory,
|
||||
aiGatewayRoutingEnabled: cfg.AIGatewayRoutingEnabled,
|
||||
experiments: cfg.Experiments,
|
||||
pendingChatAcquireInterval: pendingChatAcquireInterval,
|
||||
maxChatsPerAcquire: maxChatsPerAcquire,
|
||||
inFlightChatStaleAfter: inFlightChatStaleAfter,
|
||||
|
||||
@@ -5240,6 +5240,7 @@ func newTestServer(
|
||||
Database: db,
|
||||
ReplicaID: replicaID,
|
||||
PendingChatAcquireInterval: testutil.WaitLong,
|
||||
Experiments: codersdk.ExperimentsKnown,
|
||||
})
|
||||
t.Cleanup(func() {
|
||||
require.NoError(t, server.Close())
|
||||
@@ -8135,6 +8136,7 @@ func newActiveTestServer(
|
||||
ReplicaID: uuid.New(),
|
||||
PendingChatAcquireInterval: 10 * time.Millisecond,
|
||||
InFlightChatStaleAfter: testutil.WaitSuperLong,
|
||||
Experiments: codersdk.ExperimentsKnown,
|
||||
}
|
||||
for _, o := range overrides {
|
||||
o(&cfg)
|
||||
@@ -9319,9 +9321,6 @@ func TestComputerUseSubagentToolsAndModel(t *testing.T) {
|
||||
BaseUrl: anthropicSrv.URL,
|
||||
})
|
||||
|
||||
err := db.UpsertChatDesktopEnabled(ctx, true)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Build workspace + agent records so getWorkspaceConn can
|
||||
// resolve the agent for the computer use child.
|
||||
ws, dbAgent := seedWorkspaceWithAgent(t, db, user.ID)
|
||||
@@ -9514,6 +9513,7 @@ func TestInterruptChatPersistsPartialResponse(t *testing.T) {
|
||||
ReplicaID: uuid.New(),
|
||||
PendingChatAcquireInterval: 10 * time.Millisecond,
|
||||
InFlightChatStaleAfter: testutil.WaitSuperLong,
|
||||
Experiments: codersdk.ExperimentsKnown,
|
||||
})
|
||||
server.Start()
|
||||
t.Cleanup(func() {
|
||||
@@ -11602,51 +11602,61 @@ func TestAcquireChatsSkipsArchivedPendingChat(t *testing.T) {
|
||||
require.Equal(t, activeChat.ID, acquired[0].ID)
|
||||
}
|
||||
|
||||
func TestAdvisorGating_Disabled(t *testing.T) {
|
||||
// TestAdvisorGating_ExperimentDisabled verifies that the advisor tool is
|
||||
// not attached when the chat-advisor experiment is absent from the
|
||||
// experiments list, even if the DB-stored advisor config has Enabled=true.
|
||||
func TestAdvisorGating_ExperimentDisabled(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
var toolsMu sync.Mutex
|
||||
var capturedTools []string
|
||||
var capturedMessages []chattest.OpenAIMessage
|
||||
var streamedCallCount atomic.Int32
|
||||
var streamedCallsMu sync.Mutex
|
||||
var firstCallTools []string
|
||||
|
||||
openAIURL := chattest.NewOpenAI(t, func(req *chattest.OpenAIRequest) chattest.OpenAIResponse {
|
||||
if !req.Stream {
|
||||
return chattest.OpenAINonStreamingResponse("title")
|
||||
}
|
||||
|
||||
names := make([]string, 0, len(req.Tools))
|
||||
for _, tool := range req.Tools {
|
||||
names = append(names, tool.Function.Name)
|
||||
if streamedCallCount.Add(1) == 1 {
|
||||
names := make([]string, 0, len(req.Tools))
|
||||
for _, tool := range req.Tools {
|
||||
names = append(names, tool.Function.Name)
|
||||
}
|
||||
streamedCallsMu.Lock()
|
||||
firstCallTools = names
|
||||
streamedCallsMu.Unlock()
|
||||
}
|
||||
toolsMu.Lock()
|
||||
capturedTools = names
|
||||
capturedMessages = append([]chattest.OpenAIMessage(nil), req.Messages...)
|
||||
toolsMu.Unlock()
|
||||
|
||||
return chattest.OpenAIStreamingResponse(
|
||||
chattest.OpenAITextChunks("advisor is not available")...,
|
||||
chattest.OpenAITextChunks("done")...,
|
||||
)
|
||||
})
|
||||
|
||||
user, org, model := seedChatDependenciesWithProvider(t, db, "openai-compat", openAIURL)
|
||||
seedAdvisorConfig(ctx, t, db, codersdk.AdvisorConfig{
|
||||
Enabled: false,
|
||||
Enabled: true,
|
||||
MaxUsesPerRun: 3,
|
||||
MaxOutputTokens: 16384,
|
||||
})
|
||||
server := newActiveTestServer(t, db, ps)
|
||||
experiments := slices.DeleteFunc(
|
||||
slices.Clone(codersdk.ExperimentsKnown),
|
||||
func(e codersdk.Experiment) bool { return e == codersdk.ExperimentChatAdvisor },
|
||||
)
|
||||
server := newActiveTestServer(t, db, ps, func(cfg *chatd.Config) {
|
||||
cfg.Experiments = experiments
|
||||
})
|
||||
|
||||
chat, err := server.CreateChat(ctx, chatd.CreateOptions{
|
||||
OrganizationID: org.ID,
|
||||
OwnerID: user.ID,
|
||||
APIKeyID: testAPIKeyID(t, db, user.ID),
|
||||
Title: "advisor-disabled",
|
||||
Title: "advisor-experiment-disabled",
|
||||
ModelConfigID: model.ID,
|
||||
InitialUserContent: []codersdk.ChatMessagePart{
|
||||
codersdk.ChatMessageText("hello"),
|
||||
codersdk.ChatMessageText("help me plan this"),
|
||||
},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
@@ -11656,22 +11666,20 @@ func TestAdvisorGating_Disabled(t *testing.T) {
|
||||
if getErr != nil {
|
||||
return false
|
||||
}
|
||||
return got.Status == database.ChatStatusWaiting ||
|
||||
got.Status == database.ChatStatusError
|
||||
if got.Status != database.ChatStatusWaiting &&
|
||||
got.Status != database.ChatStatusError {
|
||||
return false
|
||||
}
|
||||
return streamedCallCount.Load() >= 1
|
||||
}, testutil.WaitLong, testutil.IntervalFast)
|
||||
|
||||
toolsMu.Lock()
|
||||
tools := append([]string(nil), capturedTools...)
|
||||
messages := append([]chattest.OpenAIMessage(nil), capturedMessages...)
|
||||
toolsMu.Unlock()
|
||||
streamedCallsMu.Lock()
|
||||
tools := append([]string(nil), firstCallTools...)
|
||||
streamedCallsMu.Unlock()
|
||||
|
||||
require.NotEmpty(t, messages, "expected a streamed LLM request")
|
||||
require.NotEmpty(t, tools, "expected at least one streamed LLM request")
|
||||
require.NotContains(t, tools, "advisor",
|
||||
"advisor tool should not be registered when disabled")
|
||||
for _, msg := range messages {
|
||||
require.NotContains(t, msg.Content, chatadvisor.ParentGuidanceBlock,
|
||||
"advisor guidance should not be injected when disabled")
|
||||
}
|
||||
"advisor tool must not be registered when the chat-advisor experiment is absent")
|
||||
}
|
||||
|
||||
func TestAdvisorGating_RootChat(t *testing.T) {
|
||||
|
||||
@@ -118,6 +118,8 @@ func (server *Server) prepareGeneration(
|
||||
|
||||
planModeInstructions := server.loadPlanModeInstructions(ctx, currentPlanMode, logger)
|
||||
advisorCfg := server.loadAdvisorConfig(ctx, logger)
|
||||
// Force Enabled from the experiment; the stored DB value is ignored.
|
||||
advisorCfg.Enabled = server.experiments.Enabled(codersdk.ExperimentChatAdvisor)
|
||||
|
||||
var advisorRuntime *chatadvisor.Runtime
|
||||
if advisorCfg.Enabled && isRootChat && !isPlanModeTurn && !isExploreSubagent {
|
||||
|
||||
@@ -114,14 +114,6 @@ type listAgentsArgs struct {
|
||||
Offset *int `json:"offset,omitempty"`
|
||||
}
|
||||
|
||||
func (p *Server) isDesktopEnabled(ctx context.Context) bool {
|
||||
enabled, err := p.db.GetChatDesktopEnabled(ctx)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
return enabled
|
||||
}
|
||||
|
||||
func subagentModelOverrideLogLabel(
|
||||
overrideContext codersdk.ChatModelOverrideContext,
|
||||
) string {
|
||||
|
||||
@@ -114,8 +114,8 @@ func allSubagentDefinitions() []subagentDefinition {
|
||||
if currentChat.PlanMode.Valid && currentChat.PlanMode.ChatPlanMode == database.ChatPlanModePlan {
|
||||
return `type "computer_use" is unavailable in plan mode`
|
||||
}
|
||||
if !p.isDesktopEnabled(ctx) {
|
||||
return `type "computer_use" is unavailable because desktop access is not enabled`
|
||||
if !p.experiments.Enabled(codersdk.ExperimentChatVirtualDesktop) {
|
||||
return `type "computer_use" is unavailable because the chat-virtual-desktop experiment is not enabled`
|
||||
}
|
||||
_, _, _, err := p.computerUseProviderAndModelFromConfig(ctx)
|
||||
if err != nil {
|
||||
|
||||
@@ -96,7 +96,6 @@ func TestSpawnComputerUseAgentInheritsPinnedContext(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
require.NoError(t, db.UpsertChatDesktopEnabled(chatdTestContext(t), true))
|
||||
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
|
||||
|
||||
ctx := chatdTestContext(t)
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"slices"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -73,6 +74,7 @@ type internalTestServerConfig struct {
|
||||
logger slog.Logger
|
||||
clock quartz.Clock
|
||||
startWorker bool
|
||||
experiments codersdk.Experiments
|
||||
}
|
||||
|
||||
type internalTestServerOpt func(*internalTestServerConfig)
|
||||
@@ -95,6 +97,19 @@ func withInternalTestServerWorker() internalTestServerOpt {
|
||||
}
|
||||
}
|
||||
|
||||
func withInternalTestServerExperiments(experiments codersdk.Experiments) internalTestServerOpt {
|
||||
return func(cfg *internalTestServerConfig) {
|
||||
cfg.experiments = experiments
|
||||
}
|
||||
}
|
||||
|
||||
func experimentsOrDefault(experiments codersdk.Experiments) codersdk.Experiments {
|
||||
if experiments == nil {
|
||||
return codersdk.ExperimentsKnown
|
||||
}
|
||||
return experiments
|
||||
}
|
||||
|
||||
// newInternalTestServer creates a passive Server for internal tests with
|
||||
// custom provider API keys. Pass withInternalTestServerWorker to start the
|
||||
// background chat worker for tests that need real execution.
|
||||
@@ -123,6 +138,7 @@ func newInternalTestServer(
|
||||
// does not interfere with test assertions.
|
||||
PendingChatAcquireInterval: testutil.WaitLong,
|
||||
ProviderAPIKeys: keys,
|
||||
Experiments: experimentsOrDefault(cfg.experiments),
|
||||
})
|
||||
if cfg.startWorker {
|
||||
server.Start()
|
||||
@@ -2150,7 +2166,6 @@ func TestSpawnAgent_DescriptionListsAllAvailableTypes(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
require.NoError(t, db.UpsertChatDesktopEnabled(chatdTestContext(t), true))
|
||||
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{
|
||||
Anthropic: "test-anthropic-key",
|
||||
})
|
||||
@@ -2198,7 +2213,6 @@ func TestSpawnAgent_DescriptionIncludesComputerUseWithMissingProviderKey(t *test
|
||||
t.Parallel()
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
require.NoError(t, db.UpsertChatDesktopEnabled(chatdTestContext(t), true))
|
||||
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
|
||||
|
||||
ctx := chatdTestContext(t)
|
||||
@@ -2220,7 +2234,6 @@ func TestSpawnAgent_PlanModeDescriptionOmitsComputerUse(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
require.NoError(t, db.UpsertChatDesktopEnabled(chatdTestContext(t), true))
|
||||
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{
|
||||
Anthropic: "test-anthropic-key",
|
||||
})
|
||||
@@ -2261,7 +2274,6 @@ func TestSpawnAgent_PlanModeRejectsComputerUse(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
require.NoError(t, db.UpsertChatDesktopEnabled(chatdTestContext(t), true))
|
||||
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{
|
||||
Anthropic: "test-anthropic-key",
|
||||
})
|
||||
@@ -2312,7 +2324,6 @@ func TestSpawnAgent_InvalidTypeAndCredentialErrorAreDistinct(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
require.NoError(t, db.UpsertChatDesktopEnabled(chatdTestContext(t), true))
|
||||
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
|
||||
|
||||
ctx := chatdTestContext(t)
|
||||
@@ -2353,7 +2364,6 @@ func TestSpawnAgent_ComputerUseAvailabilityUsesConfiguredProvider(t *testing.T)
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
ctx := chatdTestContext(t)
|
||||
require.NoError(t, db.UpsertChatDesktopEnabled(ctx, true))
|
||||
require.NoError(t, db.UpsertChatComputerUseProvider(
|
||||
ctx,
|
||||
chattool.ComputerUseProviderOpenAI,
|
||||
@@ -2374,7 +2384,6 @@ func TestSpawnAgent_ComputerUseRejectsMissingConfiguredProvider(t *testing.T) {
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
ctx := chatdTestContext(t)
|
||||
require.NoError(t, db.UpsertChatDesktopEnabled(ctx, true))
|
||||
require.NoError(t, db.UpsertChatComputerUseProvider(
|
||||
ctx,
|
||||
chattool.ComputerUseProviderOpenAI,
|
||||
@@ -2434,7 +2443,6 @@ func TestSpawnAgent_ComputerUseRejectsInvalidConfiguredProviderWithStableReason(
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
ctx := chatdTestContext(t)
|
||||
require.NoError(t, db.UpsertChatDesktopEnabled(ctx, true))
|
||||
require.NoError(t, db.UpsertChatComputerUseProvider(ctx, "bogus"))
|
||||
logSink := &subagentTestLogSink{}
|
||||
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).AppendSinks(logSink)
|
||||
@@ -2463,9 +2471,13 @@ func TestSpawnAgent_ComputerUseRejectsDesktopDisabled(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
experiments := slices.DeleteFunc(
|
||||
slices.Clone(codersdk.ExperimentsKnown),
|
||||
func(e codersdk.Experiment) bool { return e == codersdk.ExperimentChatVirtualDesktop },
|
||||
)
|
||||
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{
|
||||
Anthropic: "test-anthropic-key",
|
||||
})
|
||||
}, withInternalTestServerExperiments(experiments))
|
||||
|
||||
ctx := chatdTestContext(t)
|
||||
user, org, model := seedInternalChatDeps(t, db)
|
||||
@@ -2478,14 +2490,13 @@ func TestSpawnAgent_ComputerUseRejectsDesktopDisabled(t *testing.T) {
|
||||
Prompt: "open the browser",
|
||||
})
|
||||
require.True(t, resp.IsError)
|
||||
require.Contains(t, resp.Content, `type "computer_use" is unavailable because desktop access is not enabled`)
|
||||
require.Contains(t, resp.Content, `type "computer_use" is unavailable because the chat-virtual-desktop experiment is not enabled`)
|
||||
}
|
||||
|
||||
func TestSpawnAgent_BlankTypeReturnsValidOptions(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
require.NoError(t, db.UpsertChatDesktopEnabled(chatdTestContext(t), true))
|
||||
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{
|
||||
Anthropic: "test-anthropic-key",
|
||||
})
|
||||
@@ -2527,7 +2538,6 @@ func TestSpawnAgent_NotAvailableForChildChats(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
require.NoError(t, db.UpsertChatDesktopEnabled(chatdTestContext(t), true))
|
||||
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{
|
||||
Anthropic: "test-anthropic-key",
|
||||
})
|
||||
@@ -2609,9 +2619,6 @@ func TestSubagentLifecycleToolsIncludePersistedSubagentTypeAcrossVariants(t *tes
|
||||
t.Parallel()
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
if tt.variant == subagentTypeComputerUse {
|
||||
require.NoError(t, db.UpsertChatDesktopEnabled(chatdTestContext(t), true))
|
||||
}
|
||||
|
||||
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
|
||||
|
||||
@@ -2750,7 +2757,6 @@ func TestSpawnAgent_ComputerUseUsesComputerUseModelNotParent(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
require.NoError(t, db.UpsertChatDesktopEnabled(chatdTestContext(t), true))
|
||||
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
|
||||
|
||||
ctx := chatdTestContext(t)
|
||||
@@ -2810,7 +2816,6 @@ func TestSpawnAgent_ComputerUseInheritsMCPServerIDs(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
require.NoError(t, db.UpsertChatDesktopEnabled(chatdTestContext(t), true))
|
||||
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
|
||||
|
||||
ctx := chatdTestContext(t)
|
||||
|
||||
Reference in New Issue
Block a user