diff --git a/agent/agent.go b/agent/agent.go index 683070ab94..16600387d8 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -114,6 +114,7 @@ type Options struct { SocketServerEnabled bool SocketPath string // Path for the agent socket server socket BoundaryLogProxySocketPath string + ContextConfig agentcontextconfig.Config // DERPTLSConfig is an optional TLS config for DERP connections. DERPTLSConfig *tls.Config } @@ -233,6 +234,7 @@ func New(options Options) Agent { socketPath: options.SocketPath, socketServerEnabled: options.SocketServerEnabled, boundaryLogProxySocketPath: options.BoundaryLogProxySocketPath, + contextConfig: options.ContextConfig, derpTLSConfig: options.DERPTLSConfig, } // Initially, we have a closed channel, reflecting the fact that we are not initially connected. @@ -312,6 +314,7 @@ type agent struct { // It may be nil if there is a problem starting the server. boundaryLogProxy *boundarylogproxy.Server boundaryLogProxySocketPath string + contextConfig agentcontextconfig.Config prometheusRegistry *prometheus.Registry // metrics are prometheus registered metrics that will be collected and @@ -427,7 +430,7 @@ func (a *agent) init() { return m.Directory } return "" - }) + }, a.contextConfig) a.reconnectingPTYServer = reconnectingpty.NewServer( a.logger.Named("reconnecting-pty"), a.sshServer, diff --git a/agent/agent_internal_test.go b/agent/agent_internal_test.go index b6a904ea88..e49238e1b6 100644 --- a/agent/agent_internal_test.go +++ b/agent/agent_internal_test.go @@ -58,18 +58,10 @@ func TestReportConnectionEmpty(t *testing.T) { } func TestContextConfigAPI_InitOnce(t *testing.T) { - // Not parallel: uses t.Setenv to clear env vars. - - // Clear env vars so defaults are used and the test is - // hermetic regardless of the surrounding environment. - t.Setenv(agentcontextconfig.EnvInstructionsDirs, "") - t.Setenv(agentcontextconfig.EnvInstructionsFile, "") - t.Setenv(agentcontextconfig.EnvSkillsDirs, "") - t.Setenv(agentcontextconfig.EnvSkillMetaFile, "") - t.Setenv(agentcontextconfig.EnvMCPConfigFiles, "") + t.Parallel() // After the fix, contextConfigAPI is set once in init() and - // never reassigned. Config() evaluates lazily via the + // never reassigned. Resolve() evaluates lazily via the // manifest, so there is no concurrent write to race with. dir1 := platformAbsPath("dir1") dir2 := platformAbsPath("dir2") @@ -81,7 +73,7 @@ func TestContextConfigAPI_InitOnce(t *testing.T) { return m.Directory } return "" - }) + }, agentcontextconfig.Config{}) mcpFiles1 := a.contextConfigAPI.MCPConfigFiles() require.NotEmpty(t, mcpFiles1) diff --git a/agent/agentcontextconfig/api.go b/agent/agentcontextconfig/api.go index d82c98902f..5636f18fe3 100644 --- a/agent/agentcontextconfig/api.go +++ b/agent/agentcontextconfig/api.go @@ -68,43 +68,81 @@ const ( DefaultMCPConfigFile = ".mcp.json" ) +// Config holds the agent's context configuration. +// Defaults are applied by NewAPI, not by the zero value. +type Config struct { + InstructionsDirs string + InstructionsFile string + SkillsDirs string + SkillMetaFile string + MCPConfigFiles string +} + +// applyDefaults fills zero-valued fields with their defaults. +func (c Config) applyDefaults() Config { + c.InstructionsDirs = cmp.Or(c.InstructionsDirs, DefaultInstructionsDir) + c.InstructionsFile = cmp.Or(c.InstructionsFile, DefaultInstructionsFile) + c.SkillsDirs = cmp.Or(c.SkillsDirs, DefaultSkillsDir) + c.SkillMetaFile = cmp.Or(c.SkillMetaFile, DefaultSkillMetaFile) + c.MCPConfigFiles = cmp.Or(c.MCPConfigFiles, DefaultMCPConfigFile) + return c +} + +// ReadEnvConfig reads the CODER_AGENT_EXP_* environment +// variables, falling back to defaults for unset values. +func ReadEnvConfig() Config { + return Config{ + InstructionsDirs: strings.TrimSpace(os.Getenv(EnvInstructionsDirs)), + InstructionsFile: strings.TrimSpace(os.Getenv(EnvInstructionsFile)), + SkillsDirs: strings.TrimSpace(os.Getenv(EnvSkillsDirs)), + SkillMetaFile: strings.TrimSpace(os.Getenv(EnvSkillMetaFile)), + MCPConfigFiles: strings.TrimSpace(os.Getenv(EnvMCPConfigFiles)), + }.applyDefaults() +} + +// envVarKeys returns every CODER_AGENT_EXP_* env var key +// used by the context configuration subsystem. +func envVarKeys() []string { + return []string{ + EnvInstructionsDirs, EnvInstructionsFile, + EnvSkillsDirs, EnvSkillMetaFile, EnvMCPConfigFiles, + } +} + +// ClearEnvVars removes the CODER_AGENT_EXP_* environment +// variables from the current process so they are not +// inherited by child processes. +func ClearEnvVars() { + for _, key := range envVarKeys() { + _ = os.Unsetenv(key) + } +} + // API exposes the resolved context configuration through the // agent's HTTP API. type API struct { workingDir func() string + cfg Config } -// NewAPI accepts a closure that returns the working directory. -// The directory is evaluated lazily on each call to Config(), -// so the caller can update it after construction. -func NewAPI(workingDir func() string) *API { +// NewAPI creates a context configuration API. The working +// directory closure is evaluated lazily per request. +func NewAPI(workingDir func() string, cfg Config) *API { if workingDir == nil { workingDir = func() string { return "" } } - return &API{workingDir: workingDir} + return &API{workingDir: workingDir, cfg: cfg.applyDefaults()} } -// Config reads env vars, resolves paths, reads instruction files, -// and discovers skills. Returns the HTTP response and the resolved -// MCP config file paths (used only agent-internally). Exported -// for use by tests. -func Config(workingDir string) (workspacesdk.ContextConfigResponse, []string) { - // TrimSpace all env vars before cmp.Or so that a - // whitespace-only value falls through to the default - // consistently. ResolvePaths also trims each comma- - // separated entry, but without pre-trimming here a - // bare " " would bypass cmp.Or and produce nil. - instructionsDir := cmp.Or(strings.TrimSpace(os.Getenv(EnvInstructionsDirs)), DefaultInstructionsDir) - instructionsFile := cmp.Or(strings.TrimSpace(os.Getenv(EnvInstructionsFile)), DefaultInstructionsFile) - skillsDir := cmp.Or(strings.TrimSpace(os.Getenv(EnvSkillsDirs)), DefaultSkillsDir) - skillMetaFile := cmp.Or(strings.TrimSpace(os.Getenv(EnvSkillMetaFile)), DefaultSkillMetaFile) - mcpConfigFile := cmp.Or(strings.TrimSpace(os.Getenv(EnvMCPConfigFiles)), DefaultMCPConfigFile) - - resolvedInstructionsDirs := ResolvePaths(instructionsDir, workingDir) - resolvedSkillsDirs := ResolvePaths(skillsDir, workingDir) +// Resolve reads instruction files, discovers skills, and +// resolves MCP config file paths for the given config and +// working directory. +func Resolve(workingDir string, cfg Config) (workspacesdk.ContextConfigResponse, []string) { + resolvedInstructionsDirs := ResolvePaths(cfg.InstructionsDirs, workingDir) + resolvedSkillsDirs := ResolvePaths(cfg.SkillsDirs, workingDir) // Read instruction files from each configured directory. - parts := readInstructionFiles(resolvedInstructionsDirs, instructionsFile) + parts := readInstructionFiles(resolvedInstructionsDirs, cfg.InstructionsFile) // Also check the working directory for the instruction file, // unless it was already covered by InstructionsDirs. @@ -114,14 +152,14 @@ func Config(workingDir string) (workspacesdk.ContextConfigResponse, []string) { seenDirs[d] = struct{}{} } if _, ok := seenDirs[workingDir]; !ok { - if entry, found := readInstructionFileFromDir(workingDir, instructionsFile); found { + if entry, found := readInstructionFileFromDir(workingDir, cfg.InstructionsFile); found { parts = append(parts, entry) } } } // Discover skills from each configured skills directory. - skillParts := discoverSkills(resolvedSkillsDirs, skillMetaFile) + skillParts := discoverSkills(resolvedSkillsDirs, cfg.SkillMetaFile) parts = append(parts, skillParts...) // Guarantee non-nil slice to signal agent support. @@ -131,7 +169,7 @@ func Config(workingDir string) (workspacesdk.ContextConfigResponse, []string) { return workspacesdk.ContextConfigResponse{ Parts: parts, - }, ResolvePaths(mcpConfigFile, workingDir) + }, ResolvePaths(cfg.MCPConfigFiles, workingDir) } // ContextPartsFromDir reads instruction files and discovers skills @@ -164,7 +202,7 @@ func ContextPartsFromDir(dir string) []codersdk.ChatMessagePart { // MCPConfigFiles returns the resolved MCP configuration file // paths for the agent's MCP manager. func (api *API) MCPConfigFiles() []string { - _, mcpFiles := Config(api.workingDir()) + _, mcpFiles := Resolve(api.workingDir(), api.cfg) return mcpFiles } @@ -177,7 +215,7 @@ func (api *API) Routes() http.Handler { } func (api *API) handleGet(rw http.ResponseWriter, r *http.Request) { - response, _ := Config(api.workingDir()) + response, _ := Resolve(api.workingDir(), api.cfg) httpapi.Write(r.Context(), rw, http.StatusOK, response) } diff --git a/agent/agentcontextconfig/api_test.go b/agent/agentcontextconfig/api_test.go index be5075c6d9..eadd0196d1 100644 --- a/agent/agentcontextconfig/api_test.go +++ b/agent/agentcontextconfig/api_test.go @@ -157,13 +157,13 @@ func setupConfigTestEnv(t *testing.T, overrides map[string]string) string { return fakeHome } -func TestConfig(t *testing.T) { +func TestResolve(t *testing.T) { //nolint:paralleltest // Uses t.Setenv to mutate process-wide environment. t.Run("Defaults", func(t *testing.T) { setupConfigTestEnv(t, nil) workDir := platformAbsPath("work") - cfg, mcpFiles := agentcontextconfig.Config(workDir) + cfg, mcpFiles := agentcontextconfig.Resolve(workDir, agentcontextconfig.ReadEnvConfig()) // Parts is always non-nil. require.NotNil(t, cfg.Parts) @@ -197,7 +197,7 @@ func TestConfig(t *testing.T) { )) workDir := platformAbsPath("work") - cfg, mcpFiles := agentcontextconfig.Config(workDir) + cfg, mcpFiles := agentcontextconfig.Resolve(workDir, agentcontextconfig.ReadEnvConfig()) require.Equal(t, []string{optMCP}, mcpFiles) ctxFiles := filterParts(cfg.Parts, codersdk.ChatMessagePartTypeContextFile) @@ -220,7 +220,7 @@ func TestConfig(t *testing.T) { // Create a file matching the trimmed name. require.NoError(t, os.WriteFile(filepath.Join(fakeHome, "CLAUDE.md"), []byte("hello"), 0o600)) - cfg, _ := agentcontextconfig.Config(workDir) + cfg, _ := agentcontextconfig.Resolve(workDir, agentcontextconfig.ReadEnvConfig()) ctxFiles := filterParts(cfg.Parts, codersdk.ChatMessagePartTypeContextFile) require.Len(t, ctxFiles, 1) @@ -240,7 +240,7 @@ func TestConfig(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(b, "AGENTS.md"), []byte("from b"), 0o600)) workDir := t.TempDir() - cfg, _ := agentcontextconfig.Config(workDir) + cfg, _ := agentcontextconfig.Resolve(workDir, agentcontextconfig.ReadEnvConfig()) ctxFiles := filterParts(cfg.Parts, codersdk.ChatMessagePartTypeContextFile) require.Len(t, ctxFiles, 2) @@ -262,7 +262,7 @@ func TestConfig(t *testing.T) { 0o600, )) - cfg, _ := agentcontextconfig.Config(workDir) + cfg, _ := agentcontextconfig.Resolve(workDir, agentcontextconfig.ReadEnvConfig()) ctxFiles := filterParts(cfg.Parts, codersdk.ChatMessagePartTypeContextFile) require.NotNil(t, cfg.Parts) @@ -284,7 +284,7 @@ func TestConfig(t *testing.T) { 0o600, )) - cfg, _ := agentcontextconfig.Config(workDir) + cfg, _ := agentcontextconfig.Resolve(workDir, agentcontextconfig.ReadEnvConfig()) // Should find the working dir file (not in instruction dirs). ctxFiles := filterParts(cfg.Parts, codersdk.ChatMessagePartTypeContextFile) @@ -301,7 +301,7 @@ func TestConfig(t *testing.T) { largeContent := strings.Repeat("a", 64*1024+100) require.NoError(t, os.WriteFile(filepath.Join(workDir, "AGENTS.md"), []byte(largeContent), 0o600)) - cfg, _ := agentcontextconfig.Config(workDir) + cfg, _ := agentcontextconfig.Resolve(workDir, agentcontextconfig.ReadEnvConfig()) ctxFiles := filterParts(cfg.Parts, codersdk.ChatMessagePartTypeContextFile) require.Len(t, ctxFiles, 1) @@ -341,7 +341,7 @@ func TestConfig(t *testing.T) { 0o600, )) - cfg, _ := agentcontextconfig.Config(workDir) + cfg, _ := agentcontextconfig.Resolve(workDir, agentcontextconfig.ReadEnvConfig()) ctxFiles := filterParts(cfg.Parts, codersdk.ChatMessagePartTypeContextFile) require.Len(t, ctxFiles, 1) @@ -372,7 +372,7 @@ func TestConfig(t *testing.T) { 0o600, )) - cfg, _ := agentcontextconfig.Config(workDir) + cfg, _ := agentcontextconfig.Resolve(workDir, agentcontextconfig.ReadEnvConfig()) skillParts := filterParts(cfg.Parts, codersdk.ChatMessagePartTypeSkill) require.Len(t, skillParts, 1) @@ -391,7 +391,7 @@ func TestConfig(t *testing.T) { }) workDir := t.TempDir() - cfg, _ := agentcontextconfig.Config(workDir) + cfg, _ := agentcontextconfig.Resolve(workDir, agentcontextconfig.ReadEnvConfig()) // Non-nil empty slice (signals agent supports new format). require.NotNil(t, cfg.Parts) @@ -407,7 +407,7 @@ func TestConfig(t *testing.T) { t.Setenv(agentcontextconfig.EnvInstructionsDirs, fakeHome) workDir := t.TempDir() - _, mcpFiles := agentcontextconfig.Config(workDir) + _, mcpFiles := agentcontextconfig.Resolve(workDir, agentcontextconfig.ReadEnvConfig()) require.Equal(t, []string{optMCP}, mcpFiles) }) @@ -430,7 +430,7 @@ func TestConfig(t *testing.T) { 0o600, )) - cfg, _ := agentcontextconfig.Config(workDir) + cfg, _ := agentcontextconfig.Resolve(workDir, agentcontextconfig.ReadEnvConfig()) skillParts := filterParts(cfg.Parts, codersdk.ChatMessagePartTypeSkill) require.Empty(t, skillParts) }) @@ -456,7 +456,7 @@ func TestConfig(t *testing.T) { )) } - cfg, _ := agentcontextconfig.Config(workDir) + cfg, _ := agentcontextconfig.Resolve(workDir, agentcontextconfig.ReadEnvConfig()) skillParts := filterParts(cfg.Parts, codersdk.ChatMessagePartTypeSkill) require.Len(t, skillParts, 1) require.Equal(t, "from skills1", skillParts[0].SkillDescription) @@ -471,7 +471,7 @@ func TestNewAPI_LazyDirectory(t *testing.T) { t.Setenv(agentcontextconfig.EnvMCPConfigFiles, "") dir := "" - api := agentcontextconfig.NewAPI(func() string { return dir }) + api := agentcontextconfig.NewAPI(func() string { return dir }, agentcontextconfig.ReadEnvConfig()) // Before directory is set, MCP paths resolve to nothing. mcpFiles := api.MCPConfigFiles() @@ -483,3 +483,62 @@ func TestNewAPI_LazyDirectory(t *testing.T) { require.NotEmpty(t, mcpFiles) require.Equal(t, []string{filepath.Join(dir, ".mcp.json")}, mcpFiles) } + +// TestClearEnvVars verifies that ClearEnvVars removes every +// CODER_AGENT_EXP_* env var from the process. +// +//nolint:paralleltest // Mutates process-wide environment. +func TestClearEnvVars(t *testing.T) { + // Set every context config env var. + for _, key := range []string{ + agentcontextconfig.EnvInstructionsDirs, + agentcontextconfig.EnvInstructionsFile, + agentcontextconfig.EnvSkillsDirs, + agentcontextconfig.EnvSkillMetaFile, + agentcontextconfig.EnvMCPConfigFiles, + } { + t.Setenv(key, "some-value") + } + + agentcontextconfig.ClearEnvVars() + + // Every env var should be absent. + for _, key := range []string{ + agentcontextconfig.EnvInstructionsDirs, + agentcontextconfig.EnvInstructionsFile, + agentcontextconfig.EnvSkillsDirs, + agentcontextconfig.EnvSkillMetaFile, + agentcontextconfig.EnvMCPConfigFiles, + } { + _, ok := os.LookupEnv(key) + require.False(t, ok, "env var %s should be cleared", key) + } +} + +// TestResolve_ConfigOverridesEnv verifies that Resolve uses +// the Config struct, not environment variables. +// +//nolint:paralleltest // Uses t.Setenv to mutate process-wide environment. +func TestResolve_ConfigOverridesEnv(t *testing.T) { + // Set env vars to one value. + envDir := t.TempDir() + t.Setenv(agentcontextconfig.EnvInstructionsDirs, envDir) + + // Build a Config with a different value. + cfgDir := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(cfgDir, "AGENTS.md"), + []byte("from config"), + 0o600, + )) + + cfg := agentcontextconfig.ReadEnvConfig() + cfg.InstructionsDirs = cfgDir + + workDir := t.TempDir() + result, _ := agentcontextconfig.Resolve(workDir, cfg) + + ctxFiles := filterParts(result.Parts, codersdk.ChatMessagePartTypeContextFile) + require.Len(t, ctxFiles, 1) + require.Equal(t, "from config", ctxFiles[0].ContextFileContent) +} diff --git a/agent/agenttest/agent.go b/agent/agenttest/agent.go index bf7b9ac1a5..3428dbaf86 100644 --- a/agent/agenttest/agent.go +++ b/agent/agenttest/agent.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/coder/coder/v2/agent" + "github.com/coder/coder/v2/agent/agentcontextconfig" "github.com/coder/coder/v2/codersdk/agentsdk" "github.com/coder/coder/v2/testutil" ) @@ -47,3 +48,11 @@ func New(t testing.TB, coderURL *url.URL, agentToken string, opts ...func(*agent return agt } + +// WithContextConfigFromEnv returns an agent option that +// populates ContextConfig from the current environment. +func WithContextConfigFromEnv() func(*agent.Options) { + return func(o *agent.Options) { + o.ContextConfig = agentcontextconfig.ReadEnvConfig() + } +} diff --git a/cli/agent.go b/cli/agent.go index 588e332cf5..7e03f6fd6d 100644 --- a/cli/agent.go +++ b/cli/agent.go @@ -28,6 +28,7 @@ import ( "cdr.dev/slog/v3/sloggers/slogstackdriver" "github.com/coder/coder/v2/agent" "github.com/coder/coder/v2/agent/agentcontainers" + "github.com/coder/coder/v2/agent/agentcontextconfig" "github.com/coder/coder/v2/agent/agentexec" "github.com/coder/coder/v2/agent/agentssh" "github.com/coder/coder/v2/agent/boundarylogproxy" @@ -279,6 +280,11 @@ func workspaceAgent() *serpent.Command { defer reinitCancel() reinitEvents := agentsdk.WaitForReinitLoop(reinitCtx, logger, client) + // Read and strip env vars before the reinit + // loop so config survives agent restarts. + contextConfig := agentcontextconfig.ReadEnvConfig() + agentcontextconfig.ClearEnvVars() + var ( lastOwnerID uuid.UUID lastErr error @@ -335,6 +341,7 @@ func workspaceAgent() *serpent.Command { SocketPath: socketPath, SocketServerEnabled: socketServerEnabled, BoundaryLogProxySocketPath: boundaryLogProxySocketPath, + ContextConfig: contextConfig, }) if debugAddress != "" { diff --git a/coderd/x/chatd/chatd_test.go b/coderd/x/chatd/chatd_test.go index c2864ea637..f383495553 100644 --- a/coderd/x/chatd/chatd_test.go +++ b/coderd/x/chatd/chatd_test.go @@ -8324,7 +8324,7 @@ func TestAgentContextFilesAndSkillsLoadedIntoChat(t *testing.T) { workspace := coderdtest.CreateWorkspace(t, client, template.ID) coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID) - _ = agenttest.New(t, client.URL, agentToken) + _ = agenttest.New(t, client.URL, agentToken, agenttest.WithContextConfigFromEnv()) coderdtest.NewWorkspaceAgentWaiter(t, client, workspace.ID).Wait() // Capture LLM requests so we can inspect the system prompt.