From ee855f9618e41d86c80d4267527674e11e5d1230 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 1 Apr 2026 12:28:47 -0400 Subject: [PATCH] feat: make agent context paths configurable via env vars (#23878) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace hardcoded paths for instruction files, skills, and MCP config with values read from `CODER_AGENT_EXP_*` environment variables. Template authors configure paths via the existing `coder_agent` `env` block. The agent resolves `~`, relative, and absolute paths locally, then serves the resolved config over `GET /api/v0/context-config`. `chatd` fetches this once per workspace attach and falls back to today's defaults for older agents. All path env vars are comma-separated, allowing multiple directories: | Env Var | Default | Controls | |---|---|---| | `CODER_AGENT_EXP_INSTRUCTIONS_DIRS` | `~/.coder` | Dirs containing the instruction file | | `CODER_AGENT_EXP_INSTRUCTIONS_FILE` | `AGENTS.md` | Instruction file name | | `CODER_AGENT_EXP_SKILLS_DIRS` | `.agents/skills` | Skills directories | | `CODER_AGENT_EXP_SKILL_META_FILE` | `SKILL.md` | Skill metadata file name | | `CODER_AGENT_EXP_MCP_CONFIG_FILES` | `.mcp.json` | MCP config files | ### Example ```hcl resource "coder_agent" "main" { os = "linux" arch = "amd64" env = { CODER_AGENT_EXP_INSTRUCTIONS_DIRS = "/opt/company/agent-config,~/.coder" CODER_AGENT_EXP_INSTRUCTIONS_FILE = "CLAUDE.md" CODER_AGENT_EXP_SKILLS_DIRS = "/opt/company/ai-skills,.agents/skills" CODER_AGENT_EXP_MCP_CONFIG_FILES = "/opt/company/mcp.json,.mcp.json" } } ```
Implementation Details ### Architecture Follows the same pattern as MCP tool discovery: agent resolves locally → exposes via HTTP → chatd consumes. **Agent-side** (`agent/agentcontextconfig/`): - `ResolvePath` / `ResolvePaths` handle `~`, relative, and absolute path forms; returns `""` for relative paths when baseDir is empty - `Config` reads env vars, falls back to defaults, resolves all paths - `GET /api/v0/context-config` serves the resolved config as JSON **chatd-side** (`coderd/x/chatd/`): - Calls `conn.ContextConfig()` once on first workspace attach - Falls back to hardcoded defaults on 404 (older agents) - Iterates instruction dirs, skills dirs using resolved absolute paths - `LSRelativityRoot` everywhere — no more home/root juggling ### Key design decisions - **`EXP_` prefix**: env vars use `CODER_AGENT_EXP_*` to indicate experimental status - **Plural names**: comma-separated vars use plural names (`DIRS`, `FILES`); single-value vars use singular (`FILE`) - **Defaults in `workspacesdk`**: default constants live in `codersdk/workspacesdk/` so both agent and server reference them without cross-layer imports - **`skillMetaFile` persistence**: stored on context-file parts via `ContextFileSkillMetaFile` and restored on subsequent chat turns so custom values survive across turns - **Working dir dedup**: `slices.Contains` guard prevents reading the same instruction file from both `InstructionsDirs` and the working directory - **MCP server dedup**: first-occurrence-wins dedup prevents leaking duplicate connections from overlapping config files - **ResolvePath safety**: returns `""` for relative paths when `baseDir` is empty, so `ResolvePaths` filters them out ### Files changed | File | Change | |---|---| | `agent/agentcontextconfig/` | New package — path resolution + HTTP endpoint | | `codersdk/workspacesdk/agentconn.go` | `ContextConfigResponse` type, default constants, client method | | `agent/agent.go` + `agent/api.go` | Wire up endpoint, pass config to MCP | | `agent/x/agentmcp/manager.go` | Accept `[]string` MCP config paths, dedup by name | | `coderd/x/chatd/chatd.go` | Fetch config, thread through, named returns | | `coderd/x/chatd/instruction.go` | Accept configurable dir + file name, `skillMetaFileFromParts` | | `coderd/x/chatd/chattool/skill.go` | Accept configurable dirs + meta file | | `codersdk/chats.go` | `ContextFileSkillMetaFile` field for persistence | ### Test coverage - `TestConfig` (4 cases): defaults, custom env vars, whitespace trimming, comma-separated dirs - `TestResolvePath` / `TestResolvePaths`: including empty baseDir edge case - `TestPersistInstructionFilesFallbackOnOlderAgent`: backward-compat path when `ContextConfig` returns 404 - `TestChatMessagePartVariantTags`: updated exclusion list for new internal field ### Backward compatibility Older agents return 404 for the new endpoint. `chatd` catches this and falls back to today's defaults via `readHomeInstructionFile` (using `LSRelativityHome`). Existing workspaces work with no changes.
--- Makefile | 1 + agent/agent.go | 22 +- agent/agentcontextconfig/api.go | 83 ++++++++ agent/agentcontextconfig/api_test.go | 95 +++++++++ agent/agentcontextconfig/resolve.go | 55 +++++ agent/agentcontextconfig/resolve_test.go | 152 ++++++++++++++ agent/api.go | 1 + agent/x/agentmcp/manager.go | 45 +++- coderd/x/chatd/chatd.go | 129 ++++++++---- coderd/x/chatd/chatd_internal_test.go | 192 +++++++++++++++++- coderd/x/chatd/chatd_test.go | 22 +- coderd/x/chatd/chattool/skill.go | 173 +++++++++------- coderd/x/chatd/chattool/skill_test.go | 23 ++- coderd/x/chatd/instruction.go | 101 +++++++-- coderd/x/chatd/instruction_test.go | 64 +++++- codersdk/chats.go | 7 + codersdk/chats_test.go | 31 +-- codersdk/workspacesdk/agentconn.go | 42 ++++ .../agentconnmock/agentconnmock.go | 28 ++- 19 files changed, 1070 insertions(+), 196 deletions(-) create mode 100644 agent/agentcontextconfig/api.go create mode 100644 agent/agentcontextconfig/api_test.go create mode 100644 agent/agentcontextconfig/resolve.go create mode 100644 agent/agentcontextconfig/resolve_test.go diff --git a/Makefile b/Makefile index 6d08353fb7..ce46e8ab4d 100644 --- a/Makefile +++ b/Makefile @@ -988,6 +988,7 @@ coderd/httpmw/loggermw/loggermock/loggermock.go: coderd/httpmw/loggermw/logger.g codersdk/workspacesdk/agentconnmock/agentconnmock.go: codersdk/workspacesdk/agentconn.go go generate ./codersdk/workspacesdk/agentconnmock/ + ./scripts/format_go_file.sh "$@" touch "$@" $(AIBRIDGED_MOCKS): enterprise/aibridged/client.go enterprise/aibridged/pool.go diff --git a/agent/agent.go b/agent/agent.go index 9ee013b084..44c79ab549 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -38,6 +38,7 @@ import ( "cdr.dev/slog/v3" "github.com/coder/clistat" "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/agentfiles" "github.com/coder/coder/v2/agent/agentgit" @@ -308,12 +309,13 @@ type agent struct { containerAPI *agentcontainers.API gitAPIOptions []agentgit.Option - filesAPI *agentfiles.API - gitAPI *agentgit.API - processAPI *agentproc.API - desktopAPI *agentdesktop.API - mcpManager *agentmcp.Manager - mcpAPI *agentmcp.API + filesAPI *agentfiles.API + gitAPI *agentgit.API + processAPI *agentproc.API + desktopAPI *agentdesktop.API + mcpManager *agentmcp.Manager + mcpAPI *agentmcp.API + contextConfigAPI *agentcontextconfig.API socketServerEnabled bool socketPath string @@ -1263,6 +1265,12 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context, return xerrors.Errorf("update workspace agent startup: %w", err) } + // Initialize the context config API with the expanded + // working directory so that it is ready before the HTTP + // handler is created (which happens after manifestOK). + a.contextConfigAPI = agentcontextconfig.NewAPI( + manifest.Directory, + ) oldManifest := a.manifest.Swap(&manifest) manifestOK.complete(nil) sentResult = true @@ -1358,7 +1366,7 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context, // lifecycle transition to avoid delaying Ready. // This runs inside the tracked goroutine so it // is properly awaited on shutdown. - if mcpErr := a.mcpManager.Connect(a.gracefulCtx, manifest.Directory); mcpErr != nil { + if mcpErr := a.mcpManager.Connect(a.gracefulCtx, a.contextConfigAPI.Config().MCPConfigFiles); mcpErr != nil { a.logger.Warn(ctx, "failed to connect to workspace MCP servers", slog.Error(mcpErr)) } }) diff --git a/agent/agentcontextconfig/api.go b/agent/agentcontextconfig/api.go new file mode 100644 index 0000000000..a7a3fc0c59 --- /dev/null +++ b/agent/agentcontextconfig/api.go @@ -0,0 +1,83 @@ +package agentcontextconfig + +import ( + "cmp" + "net/http" + "os" + "strings" + + "github.com/go-chi/chi/v5" + + "github.com/coder/coder/v2/coderd/httpapi" + "github.com/coder/coder/v2/codersdk/workspacesdk" +) + +// Env var names for context configuration. Prefixed with EXP_ +// to indicate these are experimental and may change. +const ( + EnvInstructionsDirs = "CODER_AGENT_EXP_INSTRUCTIONS_DIRS" + EnvInstructionsFile = "CODER_AGENT_EXP_INSTRUCTIONS_FILE" + EnvSkillsDirs = "CODER_AGENT_EXP_SKILLS_DIRS" + EnvSkillMetaFile = "CODER_AGENT_EXP_SKILL_META_FILE" + EnvMCPConfigFiles = "CODER_AGENT_EXP_MCP_CONFIG_FILES" +) + +// Defaults are defined in codersdk/workspacesdk so both +// the agent and server can reference them without a +// cross-layer import. + +// API exposes the resolved context configuration through the +// agent's HTTP API. +type API struct { + config workspacesdk.ContextConfigResponse +} + +// NewAPI reads context configuration from environment variables, +// resolves all paths relative to workingDir, and returns an API +// handler that serves the result. +func NewAPI(workingDir string) *API { + return &API{ + config: Config(workingDir), + } +} + +// Config reads env vars and resolves paths. Exported for use +// by the MCP manager and tests. +func Config(workingDir string) workspacesdk.ContextConfigResponse { + // 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)), workspacesdk.DefaultInstructionsDir) + instructionsFile := cmp.Or(strings.TrimSpace(os.Getenv(EnvInstructionsFile)), workspacesdk.DefaultInstructionsFile) + skillsDir := cmp.Or(strings.TrimSpace(os.Getenv(EnvSkillsDirs)), workspacesdk.DefaultSkillsDir) + skillMetaFile := cmp.Or(strings.TrimSpace(os.Getenv(EnvSkillMetaFile)), workspacesdk.DefaultSkillMetaFile) + mcpConfigFile := cmp.Or(strings.TrimSpace(os.Getenv(EnvMCPConfigFiles)), workspacesdk.DefaultMCPConfigFile) + + return workspacesdk.ContextConfigResponse{ + InstructionsDirs: ResolvePaths(instructionsDir, workingDir), + InstructionsFile: instructionsFile, + SkillsDirs: ResolvePaths(skillsDir, workingDir), + SkillMetaFile: skillMetaFile, + MCPConfigFiles: ResolvePaths(mcpConfigFile, workingDir), + } +} + +// Config returns the resolved config for use by other agent +// components (e.g. MCP manager). +func (api *API) Config() workspacesdk.ContextConfigResponse { + return api.config +} + +// Routes returns the HTTP handler for the context config +// endpoint. +func (api *API) Routes() http.Handler { + r := chi.NewRouter() + r.Get("/", api.handleGet) + return r +} + +func (api *API) handleGet(rw http.ResponseWriter, r *http.Request) { + httpapi.Write(r.Context(), rw, http.StatusOK, api.config) +} diff --git a/agent/agentcontextconfig/api_test.go b/agent/agentcontextconfig/api_test.go new file mode 100644 index 0000000000..459b447b40 --- /dev/null +++ b/agent/agentcontextconfig/api_test.go @@ -0,0 +1,95 @@ +package agentcontextconfig_test + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/agent/agentcontextconfig" + "github.com/coder/coder/v2/codersdk/workspacesdk" +) + +func TestConfig(t *testing.T) { + t.Run("Defaults", func(t *testing.T) { + fakeHome := t.TempDir() + t.Setenv("HOME", fakeHome) + t.Setenv("USERPROFILE", fakeHome) + + // Clear all env vars so defaults are used. + t.Setenv(agentcontextconfig.EnvInstructionsDirs, "") + t.Setenv(agentcontextconfig.EnvInstructionsFile, "") + t.Setenv(agentcontextconfig.EnvSkillsDirs, "") + t.Setenv(agentcontextconfig.EnvSkillMetaFile, "") + t.Setenv(agentcontextconfig.EnvMCPConfigFiles, "") + + workDir := platformAbsPath("work") + cfg := agentcontextconfig.Config(workDir) + + require.Equal(t, workspacesdk.DefaultInstructionsFile, cfg.InstructionsFile) + require.Equal(t, workspacesdk.DefaultSkillMetaFile, cfg.SkillMetaFile) + // Default instructions dir is "~/.coder" which resolves + // to the home directory. + require.Equal(t, []string{filepath.Join(fakeHome, ".coder")}, cfg.InstructionsDirs) + // Default skills dir is ".agents/skills" (relative), + // resolved against the working directory. + require.Equal(t, []string{filepath.Join(workDir, ".agents", "skills")}, cfg.SkillsDirs) + // Default MCP config file is ".mcp.json" (relative), + // resolved against the working directory. + require.Equal(t, []string{filepath.Join(workDir, ".mcp.json")}, cfg.MCPConfigFiles) + }) + + t.Run("CustomEnvVars", func(t *testing.T) { + fakeHome := t.TempDir() + t.Setenv("HOME", fakeHome) + t.Setenv("USERPROFILE", fakeHome) + + optInstructions := platformAbsPath("opt", "instructions") + optSkills := platformAbsPath("opt", "skills") + optMCP := platformAbsPath("opt", "mcp.json") + + t.Setenv(agentcontextconfig.EnvInstructionsDirs, optInstructions) + t.Setenv(agentcontextconfig.EnvInstructionsFile, "CUSTOM.md") + t.Setenv(agentcontextconfig.EnvSkillsDirs, optSkills) + t.Setenv(agentcontextconfig.EnvSkillMetaFile, "META.yaml") + t.Setenv(agentcontextconfig.EnvMCPConfigFiles, optMCP) + + workDir := platformAbsPath("work") + cfg := agentcontextconfig.Config(workDir) + + require.Equal(t, "CUSTOM.md", cfg.InstructionsFile) + require.Equal(t, "META.yaml", cfg.SkillMetaFile) + require.Equal(t, []string{optInstructions}, cfg.InstructionsDirs) + require.Equal(t, []string{optSkills}, cfg.SkillsDirs) + require.Equal(t, []string{optMCP}, cfg.MCPConfigFiles) + }) + + t.Run("WhitespaceInFileNames", func(t *testing.T) { + t.Setenv(agentcontextconfig.EnvInstructionsDirs, "") + t.Setenv(agentcontextconfig.EnvInstructionsFile, " CLAUDE.md ") + t.Setenv(agentcontextconfig.EnvSkillsDirs, "") + t.Setenv(agentcontextconfig.EnvSkillMetaFile, "") + t.Setenv(agentcontextconfig.EnvMCPConfigFiles, "") + + workDir := platformAbsPath("work") + cfg := agentcontextconfig.Config(workDir) + + require.Equal(t, "CLAUDE.md", cfg.InstructionsFile) + }) + + t.Run("CommaSeparatedDirs", func(t *testing.T) { + a := platformAbsPath("opt", "a") + b := platformAbsPath("opt", "b") + + t.Setenv(agentcontextconfig.EnvInstructionsDirs, a+","+b) + t.Setenv(agentcontextconfig.EnvInstructionsFile, "") + t.Setenv(agentcontextconfig.EnvSkillsDirs, "") + t.Setenv(agentcontextconfig.EnvSkillMetaFile, "") + t.Setenv(agentcontextconfig.EnvMCPConfigFiles, "") + + workDir := platformAbsPath("work") + cfg := agentcontextconfig.Config(workDir) + + require.Equal(t, []string{a, b}, cfg.InstructionsDirs) + }) +} diff --git a/agent/agentcontextconfig/resolve.go b/agent/agentcontextconfig/resolve.go new file mode 100644 index 0000000000..a92bd1d192 --- /dev/null +++ b/agent/agentcontextconfig/resolve.go @@ -0,0 +1,55 @@ +package agentcontextconfig + +import ( + "os" + "path/filepath" + "strings" +) + +// ResolvePath resolves a single path that may be absolute, +// home-relative (~/ or ~), or relative to the given base +// directory. Returns an absolute path. Empty input returns empty. +func ResolvePath(raw, baseDir string) string { + raw = strings.TrimSpace(raw) + if raw == "" { + return "" + } + switch { + case raw == "~": + home, err := os.UserHomeDir() + if err != nil { + return "" + } + return home + case strings.HasPrefix(raw, "~/"): + home, err := os.UserHomeDir() + if err != nil { + return "" + } + return filepath.Join(home, raw[2:]) + case filepath.IsAbs(raw): + return raw + default: + if baseDir == "" { + return "" + } + return filepath.Join(baseDir, raw) + } +} + +// ResolvePaths splits a comma-separated list of paths and +// resolves each entry independently. Empty entries and entries +// that resolve to empty strings are skipped. +func ResolvePaths(raw, baseDir string) []string { + if strings.TrimSpace(raw) == "" { + return nil + } + parts := strings.Split(raw, ",") + out := make([]string, 0, len(parts)) + for _, p := range parts { + if resolved := ResolvePath(p, baseDir); resolved != "" { + out = append(out, resolved) + } + } + return out +} diff --git a/agent/agentcontextconfig/resolve_test.go b/agent/agentcontextconfig/resolve_test.go new file mode 100644 index 0000000000..ac57e59b0e --- /dev/null +++ b/agent/agentcontextconfig/resolve_test.go @@ -0,0 +1,152 @@ +package agentcontextconfig_test + +import ( + "path/filepath" + "runtime" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/agent/agentcontextconfig" +) + +// platformAbsPath constructs an absolute path that is valid +// on the current platform. On Windows paths must include a +// drive letter to be considered absolute. +func platformAbsPath(parts ...string) string { + if runtime.GOOS == "windows" { + return `C:\` + filepath.Join(parts...) + } + return "/" + filepath.Join(parts...) +} + +func TestResolvePath(t *testing.T) { //nolint:tparallel // subtests using t.Setenv cannot be parallel + t.Run("EmptyInput", func(t *testing.T) { + t.Parallel() + require.Equal(t, "", agentcontextconfig.ResolvePath("", platformAbsPath("base"))) + }) + + t.Run("WhitespaceOnly", func(t *testing.T) { + t.Parallel() + require.Equal(t, "", agentcontextconfig.ResolvePath(" ", platformAbsPath("base"))) + }) + + // Tests that use t.Setenv cannot be parallel. + t.Run("TildeAlone", func(t *testing.T) { + fakeHome := t.TempDir() + t.Setenv("HOME", fakeHome) + t.Setenv("USERPROFILE", fakeHome) + got := agentcontextconfig.ResolvePath("~", platformAbsPath("base")) + require.Equal(t, fakeHome, got) + }) + + t.Run("TildeSlashPath", func(t *testing.T) { + fakeHome := t.TempDir() + t.Setenv("HOME", fakeHome) + t.Setenv("USERPROFILE", fakeHome) + got := agentcontextconfig.ResolvePath("~/docs/readme", platformAbsPath("base")) + require.Equal(t, filepath.Join(fakeHome, "docs", "readme"), got) + }) + + t.Run("AbsolutePath", func(t *testing.T) { + t.Parallel() + p := platformAbsPath("etc", "coder") + got := agentcontextconfig.ResolvePath(p, platformAbsPath("base")) + require.Equal(t, p, got) + }) + + t.Run("RelativePath", func(t *testing.T) { + t.Parallel() + base := platformAbsPath("work") + got := agentcontextconfig.ResolvePath("foo/bar", base) + require.Equal(t, filepath.Join(base, "foo", "bar"), got) + }) + + t.Run("RelativePathWithWhitespace", func(t *testing.T) { + t.Parallel() + base := platformAbsPath("work") + got := agentcontextconfig.ResolvePath(" foo/bar ", base) + require.Equal(t, filepath.Join(base, "foo", "bar"), got) + }) + + t.Run("RelativePathWithEmptyBaseDir", func(t *testing.T) { + t.Parallel() + got := agentcontextconfig.ResolvePath(".agents/skills", "") + require.Equal(t, "", got) + }) +} + +func TestResolvePath_HomeUnset(t *testing.T) { + // Cannot be parallel — modifies HOME env var. + t.Setenv("HOME", "") + // Also clear USERPROFILE for Windows compatibility. + t.Setenv("USERPROFILE", "") + + require.Equal(t, "", agentcontextconfig.ResolvePath("~", platformAbsPath("base"))) + require.Equal(t, "", agentcontextconfig.ResolvePath("~/docs", platformAbsPath("base"))) +} + +func TestResolvePaths(t *testing.T) { //nolint:tparallel // subtests using t.Setenv cannot be parallel + t.Run("EmptyString", func(t *testing.T) { + t.Parallel() + require.Nil(t, agentcontextconfig.ResolvePaths("", platformAbsPath("base"))) + }) + + t.Run("WhitespaceOnly", func(t *testing.T) { + t.Parallel() + require.Nil(t, agentcontextconfig.ResolvePaths(" ", platformAbsPath("base"))) + }) + + t.Run("SingleEntry", func(t *testing.T) { + t.Parallel() + p := platformAbsPath("abs", "path") + got := agentcontextconfig.ResolvePaths(p, platformAbsPath("base")) + require.Equal(t, []string{p}, got) + }) + + // Tests that use t.Setenv cannot be parallel. + t.Run("MultipleEntries", func(t *testing.T) { + fakeHome := t.TempDir() + t.Setenv("HOME", fakeHome) + t.Setenv("USERPROFILE", fakeHome) + b := platformAbsPath("b") + base := platformAbsPath("base") + got := agentcontextconfig.ResolvePaths("~/a,"+b+",rel", base) + require.Equal(t, []string{ + filepath.Join(fakeHome, "a"), + b, + filepath.Join(base, "rel"), + }, got) + }) + + t.Run("TrimsWhitespace", func(t *testing.T) { + t.Parallel() + a := platformAbsPath("a") + b := platformAbsPath("b") + got := agentcontextconfig.ResolvePaths(" "+a+" , "+b+" ", platformAbsPath("base")) + require.Equal(t, []string{a, b}, got) + }) + + t.Run("SkipsEmptyEntries", func(t *testing.T) { + t.Parallel() + a := platformAbsPath("a") + b := platformAbsPath("b") + got := agentcontextconfig.ResolvePaths(a+",,"+b+",", platformAbsPath("base")) + require.Equal(t, []string{a, b}, got) + }) + + t.Run("TrailingComma", func(t *testing.T) { + t.Parallel() + p := platformAbsPath("only") + got := agentcontextconfig.ResolvePaths(p+",", platformAbsPath("base")) + require.Equal(t, []string{p}, got) + }) + + t.Run("RelativePathSkippedWhenBaseDirEmpty", func(t *testing.T) { + fakeHome := t.TempDir() + t.Setenv("HOME", fakeHome) + t.Setenv("USERPROFILE", fakeHome) + got := agentcontextconfig.ResolvePaths("~/.coder,.agents/skills", "") + require.Equal(t, []string{filepath.Join(fakeHome, ".coder")}, got) + }) +} diff --git a/agent/api.go b/agent/api.go index c74533e669..0258d410cd 100644 --- a/agent/api.go +++ b/agent/api.go @@ -32,6 +32,7 @@ func (a *agent) apiHandler() http.Handler { r.Mount("/api/v0/processes", a.processAPI.Routes()) r.Mount("/api/v0/desktop", a.desktopAPI.Routes()) r.Mount("/api/v0/mcp", a.mcpAPI.Routes()) + r.Mount("/api/v0/context-config", a.contextConfigAPI.Routes()) if a.devcontainers { r.Mount("/api/v0/containers", a.containerAPI.Routes()) diff --git a/agent/x/agentmcp/manager.go b/agent/x/agentmcp/manager.go index f8c4445cce..c1ae470417 100644 --- a/agent/x/agentmcp/manager.go +++ b/agent/x/agentmcp/manager.go @@ -6,7 +6,6 @@ import ( "fmt" "io/fs" "os" - "path/filepath" "slices" "strings" "sync" @@ -70,16 +69,40 @@ func NewManager(logger slog.Logger) *Manager { } } -// Connect discovers .mcp.json in dir and connects to all -// configured servers. Failed servers are logged and skipped. -func (m *Manager) Connect(ctx context.Context, dir string) error { - path := filepath.Join(dir, ".mcp.json") - configs, err := ParseConfig(path) - if err != nil { - if errors.Is(err, fs.ErrNotExist) { - return nil +// Connect reads MCP config files at the given absolute paths and +// connects to all configured servers. Failed servers are logged +// and skipped. Missing config files are silently skipped. +func (m *Manager) Connect(ctx context.Context, mcpConfigFiles []string) error { + var allConfigs []ServerConfig + for _, configPath := range mcpConfigFiles { + configs, err := ParseConfig(configPath) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + continue + } + m.logger.Warn(ctx, "failed to parse MCP config", + slog.F("path", configPath), + slog.Error(err), + ) + continue } - return xerrors.Errorf("parse mcp config: %w", err) + allConfigs = append(allConfigs, configs...) + } + + // Deduplicate by server name; first occurrence wins. + seen := make(map[string]struct{}) + deduped := make([]ServerConfig, 0, len(allConfigs)) + for _, cfg := range allConfigs { + if _, ok := seen[cfg.Name]; ok { + continue + } + seen[cfg.Name] = struct{}{} + deduped = append(deduped, cfg) + } + allConfigs = deduped + + if len(allConfigs) == 0 { + return nil } // Connect to servers in parallel without holding the @@ -95,7 +118,7 @@ func (m *Manager) Connect(ctx context.Context, dir string) error { connected []connectedServer ) var eg errgroup.Group - for _, cfg := range configs { + for _, cfg := range allConfigs { eg.Go(func() error { c, err := m.connectServer(ctx, cfg) if err != nil { diff --git a/coderd/x/chatd/chatd.go b/coderd/x/chatd/chatd.go index 854dced8b4..aa6b759321 100644 --- a/coderd/x/chatd/chatd.go +++ b/coderd/x/chatd/chatd.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "net/http" + "path" "slices" "strconv" "strings" @@ -3958,6 +3959,7 @@ func (p *Server) runChat( mcpCleanup func() workspaceMCPTools []fantasy.AgentTool skills []chattool.SkillMeta + skillMetaFile = workspacesdk.DefaultSkillMetaFile ) // Check if instruction files need to be (re-)persisted. // This happens when no context-file parts exist yet, or when @@ -3980,7 +3982,7 @@ func (p *Server) runChat( if needsInstructionPersist { g2.Go(func() error { var persistErr error - instruction, skills, persistErr = p.persistInstructionFiles( + instruction, skills, skillMetaFile, persistErr = p.persistInstructionFiles( ctx, chat, modelConfig.ID, @@ -4007,6 +4009,9 @@ func (p *Server) runChat( // those messages. No workspace dial needed. instruction = instructionFromContextFiles(messages) skills = skillsFromParts(messages) + if restored := skillMetaFileFromParts(messages); restored != "" { + skillMetaFile = restored + } } g2.Go(func() error { resolvedUserPrompt = p.resolveUserPrompt(ctx, chat.OwnerID) @@ -4497,6 +4502,7 @@ func (p *Server) runChat( GetSkills: func() []chattool.SkillMeta { return skills }, + SkillMetaFile: skillMetaFile, } tools = append(tools, chattool.ReadSkill(skillOpts), @@ -4949,22 +4955,22 @@ func contextFileAgentID(messages []database.ChatMessage) (uuid.UUID, bool) { // skills from the workspace agent, persisting both as message // parts. This is called once when a workspace is first attached // to a chat (or when the agent changes). Returns the formatted -// instruction string and skill index for injection into the -// current turn's prompt. +// instruction string, skill index, and the skill meta file name +// for injection into the current turn's prompt. func (p *Server) persistInstructionFiles( ctx context.Context, chat database.Chat, modelConfigID uuid.UUID, getWorkspaceAgent func(context.Context) (database.WorkspaceAgent, error), getWorkspaceConn func(context.Context) (workspacesdk.AgentConn, error), -) (string, []chattool.SkillMeta, error) { +) (instruction string, skills []chattool.SkillMeta, skillMetaFile string, err error) { if !chat.WorkspaceID.Valid || getWorkspaceAgent == nil { - return "", nil, nil + return "", nil, workspacesdk.DefaultSkillMetaFile, nil } agent, err := getWorkspaceAgent(ctx) if err != nil { - return "", nil, nil + return "", nil, workspacesdk.DefaultSkillMetaFile, nil } directory := agent.ExpandedDirectory @@ -4977,7 +4983,17 @@ func (p *Server) persistInstructionFiles( sections []instructionFileSection workspaceConnOK bool ) - if getWorkspaceConn != nil { + + // Fetch context configuration from the agent. This tells + // us where instruction files, skills, and MCP configs live. + // Fall back to the pre-context-config behavior for older + // agents that don't support the endpoint. + agentCfg := workspacesdk.ContextConfigResponse{ + InstructionsFile: workspacesdk.DefaultInstructionsFile, + SkillMetaFile: workspacesdk.DefaultSkillMetaFile, + } + + if getWorkspaceConn != nil { //nolint:nestif // Existing high-complexity block; config fallback logic adds unavoidable branches. instructionCtx, cancel := context.WithTimeout(ctx, homeInstructionLookupTimeout) defer cancel() @@ -4989,14 +5005,57 @@ func (p *Server) persistInstructionFiles( ) } else { workspaceConnOK = true - if content, source, truncated, readErr := readHomeInstructionFile(instructionCtx, conn); readErr != nil { - p.logger.Debug(ctx, "failed to load home instruction file", - slog.F("chat_id", chat.ID), slog.Error(readErr)) - } else if content != "" { - sections = append(sections, instructionFileSection{content, source, truncated}) + + // Fetch resolved context config from agent. + var cfgErr error + agentCfg, cfgErr = conn.ContextConfig(instructionCtx) + if cfgErr != nil { + p.logger.Debug(ctx, "agent does not support context-config endpoint, using defaults", + slog.F("chat_id", chat.ID), slog.Error(cfgErr)) + // Fall back to the pre-context-config behavior: + // read instruction file from home dir using + // LSRelativityHome and discover skills from the + // working directory. + agentCfg = workspacesdk.ContextConfigResponse{ + InstructionsFile: workspacesdk.DefaultInstructionsFile, + SkillMetaFile: workspacesdk.DefaultSkillMetaFile, + } + if content, source, truncated, readErr := readHomeInstructionFile( + instructionCtx, conn, ".coder", agentCfg.InstructionsFile, + ); readErr != nil { + p.logger.Debug(ctx, "failed to load home instruction file", + slog.F("chat_id", chat.ID), slog.Error(readErr)) + } else if content != "" { + sections = append(sections, instructionFileSection{content, source, truncated}) + } + if directory != "" { + agentCfg.SkillsDirs = []string{path.Join(directory, ".agents/skills")} + } } - if pwdPath := pwdInstructionFilePath(directory); pwdPath != "" { + // Read instruction files from each configured + // instruction directory. Track seen paths to + // avoid reading the same file twice when the + // user duplicates entries. + seenDirs := make(map[string]struct{}, len(agentCfg.InstructionsDirs)) + for _, absDir := range agentCfg.InstructionsDirs { + if _, ok := seenDirs[absDir]; ok { + continue + } + seenDirs[absDir] = struct{}{} + if content, source, truncated, readErr := readInstructionDirFile(instructionCtx, conn, absDir, agentCfg.InstructionsFile); readErr != nil { + p.logger.Debug(ctx, "failed to load instruction file from dir", + slog.F("chat_id", chat.ID), slog.F("dir", absDir), slog.Error(readErr)) + } else if content != "" { + sections = append(sections, instructionFileSection{content, source, truncated}) + } + } + + // Also check the working directory for the + // instruction file, unless it was already + // covered by InstructionsDirs. + _, pwdSeen := seenDirs[directory] + if pwdPath := pwdInstructionFilePath(directory, agentCfg.InstructionsFile); pwdPath != "" && !pwdSeen { if content, source, truncated, readErr := readInstructionFile(instructionCtx, conn, pwdPath); readErr != nil { p.logger.Debug(ctx, "failed to load working directory instruction file", slog.F("chat_id", chat.ID), slog.F("directory", directory), slog.Error(readErr)) @@ -5007,15 +5066,15 @@ func (p *Server) persistInstructionFiles( } } - // Discover skills from the workspace while we have a - // connection. Errors are non-fatal — a chat without skills - // still works, it just won't list them in the prompt. + // Discover skills from each configured skills directory. + // Errors are non-fatal. A chat without skills still works, + // it just won't list them in the prompt. var discoveredSkills []chattool.SkillMeta - if workspaceConnOK { + if workspaceConnOK && len(agentCfg.SkillsDirs) > 0 { conn, connErr := getWorkspaceConn(ctx) if connErr == nil { var discoverErr error - discoveredSkills, discoverErr = chattool.DiscoverSkills(ctx, conn, directory) + discoveredSkills, discoverErr = chattool.DiscoverSkills(ctx, p.logger, conn, agentCfg.SkillsDirs, agentCfg.SkillMetaFile) if discoverErr != nil { p.logger.Debug(ctx, "failed to discover skills", slog.F("chat_id", chat.ID), @@ -5027,14 +5086,15 @@ func (p *Server) persistInstructionFiles( if len(sections) == 0 { if !workspaceConnOK { - return "", nil, nil + return "", nil, agentCfg.SkillMetaFile, nil } // Persist a sentinel (plus any discovered skill parts) // so subsequent turns skip the workspace agent dial. parts := []codersdk.ChatMessagePart{{ - Type: codersdk.ChatMessagePartTypeContextFile, - ContextFilePath: "", - ContextFileAgentID: uuid.NullUUID{UUID: agent.ID, Valid: true}, + Type: codersdk.ChatMessagePartTypeContextFile, + ContextFilePath: "", + ContextFileAgentID: uuid.NullUUID{UUID: agent.ID, Valid: true}, + ContextFileSkillMetaFile: agentCfg.SkillMetaFile, }} for _, s := range discoveredSkills { parts = append(parts, codersdk.ChatMessagePart{ @@ -5047,7 +5107,7 @@ func (p *Server) persistInstructionFiles( } content, err := chatprompt.MarshalParts(parts) if err != nil { - return "", nil, nil + return "", nil, agentCfg.SkillMetaFile, nil } msgParams := database.InsertChatMessagesParams{ //nolint:exhaustruct // Fields populated by appendChatMessage. ChatID: chat.ID, @@ -5077,20 +5137,21 @@ func (p *Server) persistInstructionFiles( } else { p.updateLastInjectedContext(ctx, chat.ID, nil) } - return "", discoveredSkills, nil + return "", discoveredSkills, agentCfg.SkillMetaFile, nil } // Build context-file parts (one per instruction file) and // skill parts (one per discovered skill). parts := make([]codersdk.ChatMessagePart, 0, len(sections)+len(discoveredSkills)) for _, s := range sections { parts = append(parts, codersdk.ChatMessagePart{ - Type: codersdk.ChatMessagePartTypeContextFile, - ContextFilePath: s.source, - ContextFileContent: s.content, - ContextFileTruncated: s.truncated, - ContextFileAgentID: uuid.NullUUID{UUID: agent.ID, Valid: true}, - ContextFileOS: agent.OperatingSystem, - ContextFileDirectory: directory, + Type: codersdk.ChatMessagePartTypeContextFile, + ContextFilePath: s.source, + ContextFileContent: s.content, + ContextFileTruncated: s.truncated, + ContextFileAgentID: uuid.NullUUID{UUID: agent.ID, Valid: true}, + ContextFileOS: agent.OperatingSystem, + ContextFileDirectory: directory, + ContextFileSkillMetaFile: agentCfg.SkillMetaFile, }) } for _, s := range discoveredSkills { @@ -5105,7 +5166,7 @@ func (p *Server) persistInstructionFiles( content, err := chatprompt.MarshalParts(parts) if err != nil { - return "", nil, xerrors.Errorf("marshal context-file parts: %w", err) + return "", nil, agentCfg.SkillMetaFile, xerrors.Errorf("marshal context-file parts: %w", err) } msgParams := database.InsertChatMessagesParams{ //nolint:exhaustruct // Fields populated by appendChatMessage. @@ -5119,7 +5180,7 @@ func (p *Server) persistInstructionFiles( chatprompt.CurrentContentVersion, )) if _, err := p.db.InsertChatMessages(ctx, msgParams); err != nil { - return "", nil, xerrors.Errorf("persist instruction files: %w", err) + return "", nil, agentCfg.SkillMetaFile, xerrors.Errorf("persist instruction files: %w", err) } // Build stripped copies for the cache column so internal // fields (full file content, OS, directory, skill paths) @@ -5134,7 +5195,7 @@ func (p *Server) persistInstructionFiles( // Return the formatted instruction text and discovered skills // so the caller can inject them into this turn's prompt (since // the prompt was built before we persisted). - return formatSystemInstructions(agent.OperatingSystem, directory, sections), discoveredSkills, nil + return formatSystemInstructions(agent.OperatingSystem, directory, sections), discoveredSkills, agentCfg.SkillMetaFile, nil } // updateLastInjectedContext persists the injected context diff --git a/coderd/x/chatd/chatd_internal_test.go b/coderd/x/chatd/chatd_internal_test.go index 25cff273a3..98624f6729 100644 --- a/coderd/x/chatd/chatd_internal_test.go +++ b/coderd/x/chatd/chatd_internal_test.go @@ -513,6 +513,12 @@ func TestPersistInstructionFilesIncludesAgentMetadata(t *testing.T) { conn := agentconnmock.NewMockAgentConn(ctrl) conn.EXPECT().SetExtraHeaders(gomock.Any()).Times(1) + conn.EXPECT().ContextConfig(gomock.Any()).Return(workspacesdk.ContextConfigResponse{ + InstructionsDirs: []string{"/home/coder/.coder"}, + InstructionsFile: "AGENTS.md", + SkillsDirs: []string{"/home/coder/project/.agents/skills"}, + SkillMetaFile: "SKILL.md", + }, nil).AnyTimes() conn.EXPECT().LS(gomock.Any(), "", gomock.Any()).Return( workspacesdk.LSResponse{}, codersdk.NewTestError(404, "POST", "/api/v0/list-directory"), @@ -520,8 +526,7 @@ func TestPersistInstructionFilesIncludesAgentMetadata(t *testing.T) { conn.EXPECT().ReadFile(gomock.Any(), "/home/coder/project/AGENTS.md", int64(0), - int64(maxInstructionFileBytes+1), - ).Return( + int64(maxInstructionFileBytes+1)).Return( io.NopCloser(strings.NewReader("# Project instructions")), "", nil, @@ -546,7 +551,7 @@ func TestPersistInstructionFilesIncludesAgentMetadata(t *testing.T) { } t.Cleanup(workspaceCtx.close) - instruction, _, err := server.persistInstructionFiles( + instruction, _, _, err := server.persistInstructionFiles( ctx, chat, uuid.New(), @@ -558,6 +563,157 @@ func TestPersistInstructionFilesIncludesAgentMetadata(t *testing.T) { require.Contains(t, instruction, "Working Directory: /home/coder/project") } +func TestPersistInstructionFilesFallbackOnOlderAgent(t *testing.T) { + t.Parallel() + + // When the agent doesn't support the context-config endpoint + // (returns an error), the fallback path should: + // 1. Read instruction files from ~/.coder using LSRelativityHome + // 2. Discover skills from the working directory + // 3. Return the default skill meta file name + + ctx := context.Background() + ctrl := gomock.NewController(t) + db := dbmock.NewMockStore(ctrl) + + workspaceID := uuid.New() + agentID := uuid.New() + chat := database.Chat{ + ID: uuid.New(), + WorkspaceID: uuid.NullUUID{ + UUID: workspaceID, + Valid: true, + }, + AgentID: uuid.NullUUID{ + UUID: agentID, + Valid: true, + }, + } + workspaceAgent := database.WorkspaceAgent{ + ID: agentID, + OperatingSystem: "linux", + Directory: "/home/coder/project", + ExpandedDirectory: "/home/coder/project", + } + + db.EXPECT().GetWorkspaceAgentByID( + gomock.Any(), + agentID, + ).Return(workspaceAgent, nil).Times(1) + db.EXPECT().InsertChatMessages(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + db.EXPECT().UpdateChatLastInjectedContext(gomock.Any(), gomock.Any()).Return(database.Chat{}, nil).AnyTimes() + + conn := agentconnmock.NewMockAgentConn(ctrl) + conn.EXPECT().SetExtraHeaders(gomock.Any()).Times(1) + + // ContextConfig returns error — simulating an older agent. + conn.EXPECT().ContextConfig(gomock.Any()).Return( + workspacesdk.ContextConfigResponse{}, + codersdk.NewTestError(404, "GET", "/api/v0/context-config"), + ).Times(1) + + // Fallback: readHomeInstructionFile uses LSRelativityHome + // to read from ~/.coder directory. + conn.EXPECT().LS(gomock.Any(), "", + gomock.Cond(func(x any) bool { + req, ok := x.(workspacesdk.LSRequest) + return ok && req.Relativity == workspacesdk.LSRelativityHome && + len(req.Path) == 1 && req.Path[0] == ".coder" + }), + ).Return(workspacesdk.LSResponse{ + Contents: []workspacesdk.LSFile{{ + Name: "AGENTS.md", + AbsolutePathString: "/home/user/.coder/AGENTS.md", + IsDir: false, + }}, + }, nil).Times(1) + + // ReadFile for the home instruction file. + conn.EXPECT().ReadFile(gomock.Any(), + "/home/user/.coder/AGENTS.md", + int64(0), + int64(maxInstructionFileBytes+1), + ).Return( + io.NopCloser(strings.NewReader("# Home instructions")), + "", + nil, + ).Times(1) + + // Working directory instruction file: 404. + conn.EXPECT().ReadFile(gomock.Any(), + "/home/coder/project/AGENTS.md", + int64(0), + int64(maxInstructionFileBytes+1), + ).Return( + nil, "", + codersdk.NewTestError(404, "GET", "/api/v0/read-file"), + ).Times(1) + + // Skills directory: fallback constructs path from working dir. + conn.EXPECT().LS(gomock.Any(), "", + gomock.Cond(func(x any) bool { + req, ok := x.(workspacesdk.LSRequest) + return ok && req.Relativity == workspacesdk.LSRelativityRoot && + len(req.Path) == 1 && req.Path[0] == "/home/coder/project/.agents/skills" + }), + ).Return(workspacesdk.LSResponse{ + Contents: []workspacesdk.LSFile{{ + Name: "fallback-skill", + AbsolutePathString: "/home/coder/project/.agents/skills/fallback-skill", + IsDir: true, + }}, + }, nil).Times(1) + + skillContent := "---\nname: fallback-skill\ndescription: Discovered via fallback\n---\nBody" + conn.EXPECT().ReadFile(gomock.Any(), + "/home/coder/project/.agents/skills/fallback-skill/SKILL.md", + int64(0), + int64(64*1024+1), + ).Return( + io.NopCloser(strings.NewReader(skillContent)), + "", + nil, + ).Times(1) + + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}) + server := &Server{ + db: db, + logger: logger, + agentConnFn: func(context.Context, uuid.UUID) (workspacesdk.AgentConn, func(), error) { + return conn, func() {}, nil + }, + } + + chatStateMu := &sync.Mutex{} + currentChat := chat + workspaceCtx := turnWorkspaceContext{ + server: server, + chatStateMu: chatStateMu, + currentChat: ¤tChat, + loadChatSnapshot: func(context.Context, uuid.UUID) (database.Chat, error) { return database.Chat{}, nil }, + } + t.Cleanup(workspaceCtx.close) + + instruction, skills, skillMeta, err := server.persistInstructionFiles( + ctx, + chat, + uuid.New(), + workspaceCtx.getWorkspaceAgent, + workspaceCtx.getWorkspaceConn, + ) + require.NoError(t, err) + // Instruction should contain the home instruction file content. + require.Contains(t, instruction, "Home instructions") + // OS and directory metadata should be present. + require.Contains(t, instruction, "Operating System: linux") + require.Contains(t, instruction, "Working Directory: /home/coder/project") + // Skills should be discovered from the working directory. + require.Len(t, skills, 1) + require.Equal(t, "fallback-skill", skills[0].Name) + // Skill meta file should be the default. + require.Equal(t, workspacesdk.DefaultSkillMetaFile, skillMeta) +} + func TestPersistInstructionFilesSkipsSentinelWhenWorkspaceUnavailable(t *testing.T) { t.Parallel() @@ -577,7 +733,7 @@ func TestPersistInstructionFilesSkipsSentinelWhenWorkspaceUnavailable(t *testing logger: slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}), } - instruction, _, err := server.persistInstructionFiles( + instruction, _, _, err := server.persistInstructionFiles( ctx, chat, uuid.New(), @@ -655,13 +811,20 @@ func TestPersistInstructionFilesSentinelWithSkills(t *testing.T) { conn := agentconnmock.NewMockAgentConn(ctrl) conn.EXPECT().SetExtraHeaders(gomock.Any()).Times(1) + conn.EXPECT().ContextConfig(gomock.Any()).Return(workspacesdk.ContextConfigResponse{ + InstructionsDirs: []string{"/home/coder/.coder"}, + InstructionsFile: "AGENTS.md", + SkillsDirs: []string{"/home/coder/project/.agents/skills"}, + SkillMetaFile: "SKILL.md", + }, nil).AnyTimes() - // Home LS (.coder directory): return 404 so no home - // instruction file is found. + // Instruction dir LS (.coder directory): return 404 so no + // instruction file is found from the configured dir. conn.EXPECT().LS(gomock.Any(), "", gomock.Cond(func(x any) bool { req, ok := x.(workspacesdk.LSRequest) - return ok && req.Relativity == workspacesdk.LSRelativityHome + return ok && req.Relativity == workspacesdk.LSRelativityRoot && + len(req.Path) == 1 && req.Path[0] == "/home/coder/.coder" }), ).Return( workspacesdk.LSResponse{}, @@ -684,7 +847,8 @@ func TestPersistInstructionFilesSentinelWithSkills(t *testing.T) { conn.EXPECT().LS(gomock.Any(), "", gomock.Cond(func(x any) bool { req, ok := x.(workspacesdk.LSRequest) - return ok && req.Relativity == workspacesdk.LSRelativityRoot + return ok && req.Relativity == workspacesdk.LSRelativityRoot && + len(req.Path) == 1 && req.Path[0] == "/home/coder/project/.agents/skills" }), ).Return(workspacesdk.LSResponse{ Contents: []workspacesdk.LSFile{{ @@ -725,7 +889,7 @@ func TestPersistInstructionFilesSentinelWithSkills(t *testing.T) { } t.Cleanup(workspaceCtx.close) - instruction, skills, err := server.persistInstructionFiles( + instruction, skills, _, err := server.persistInstructionFiles( ctx, chat, uuid.New(), @@ -786,8 +950,14 @@ func TestPersistInstructionFilesSentinelNoSkillsClearsColumn(t *testing.T) { conn := agentconnmock.NewMockAgentConn(ctrl) conn.EXPECT().SetExtraHeaders(gomock.Any()).Times(1) + conn.EXPECT().ContextConfig(gomock.Any()).Return(workspacesdk.ContextConfigResponse{ + InstructionsDirs: []string{"/home/coder/.coder"}, + InstructionsFile: "AGENTS.md", + SkillsDirs: []string{"/home/coder/project/.agents/skills"}, + SkillMetaFile: "SKILL.md", + }, nil).AnyTimes() - // All LS calls return 404: no home .coder directory and no + // All LS calls return 404: no .coder directory and no // .agents/skills directory. conn.EXPECT().LS(gomock.Any(), "", gomock.Any()).Return( workspacesdk.LSResponse{}, @@ -823,7 +993,7 @@ func TestPersistInstructionFilesSentinelNoSkillsClearsColumn(t *testing.T) { } t.Cleanup(workspaceCtx.close) - instruction, skills, err := server.persistInstructionFiles( + instruction, skills, _, err := server.persistInstructionFiles( ctx, chat, uuid.New(), diff --git a/coderd/x/chatd/chatd_test.go b/coderd/x/chatd/chatd_test.go index 69fb7d0a94..a149fe70cc 100644 --- a/coderd/x/chatd/chatd_test.go +++ b/coderd/x/chatd/chatd_test.go @@ -1746,6 +1746,10 @@ func TestPersistToolResultWithBinaryData(t *testing.T) { mockConn.EXPECT(). SetExtraHeaders(gomock.Any()). AnyTimes() + mockConn.EXPECT(). + ContextConfig(gomock.Any()). + Return(workspacesdk.ContextConfigResponse{}, xerrors.New("not supported")). + AnyTimes() mockConn.EXPECT(). ListMCPTools(gomock.Any()). Return(workspacesdk.ListMCPToolsResponse{}, nil). @@ -3576,6 +3580,10 @@ func TestComputerUseSubagentToolsAndModel(t *testing.T) { mockConn.EXPECT(). SetExtraHeaders(gomock.Any()). AnyTimes() + mockConn.EXPECT(). + ContextConfig(gomock.Any()). + Return(workspacesdk.ContextConfigResponse{}, xerrors.New("not supported")). + AnyTimes() mockConn.EXPECT(). LS(gomock.Any(), gomock.Any(), gomock.Any()). Return(workspacesdk.LSResponse{}, xerrors.New("not found")). @@ -4009,6 +4017,8 @@ func TestMCPServerToolInvocation(t *testing.T) { ctrl := gomock.NewController(t) mockConn := agentconnmock.NewMockAgentConn(ctrl) mockConn.EXPECT().SetExtraHeaders(gomock.Any()).AnyTimes() + mockConn.EXPECT().ContextConfig(gomock.Any()). + Return(workspacesdk.ContextConfigResponse{}, xerrors.New("not supported")).AnyTimes() mockConn.EXPECT().ListMCPTools(gomock.Any()). Return(workspacesdk.ListMCPToolsResponse{}, nil).AnyTimes() mockConn.EXPECT().LS(gomock.Any(), gomock.Any(), gomock.Any()). @@ -4024,11 +4034,10 @@ func TestMCPServerToolInvocation(t *testing.T) { }) chat, err := server.CreateChat(ctx, chatd.CreateOptions{ - OwnerID: user.ID, - Title: "mcp-tool-test", - ModelConfigID: model.ID, - WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true}, - MCPServerIDs: []uuid.UUID{mcpConfig.ID}, + OwnerID: user.ID, + Title: "mcp-tool-test", ModelConfigID: model.ID, + WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true}, + MCPServerIDs: []uuid.UUID{mcpConfig.ID}, InitialUserContent: []codersdk.ChatMessagePart{ codersdk.ChatMessageText("Echo something via MCP."), }, @@ -4252,13 +4261,14 @@ func TestMCPServerOAuth2TokenRefresh(t *testing.T) { ctrl := gomock.NewController(t) mockConn := agentconnmock.NewMockAgentConn(ctrl) mockConn.EXPECT().SetExtraHeaders(gomock.Any()).AnyTimes() + mockConn.EXPECT().ContextConfig(gomock.Any()). + Return(workspacesdk.ContextConfigResponse{}, xerrors.New("not supported")).AnyTimes() mockConn.EXPECT().ListMCPTools(gomock.Any()). Return(workspacesdk.ListMCPToolsResponse{}, nil).AnyTimes() mockConn.EXPECT().LS(gomock.Any(), gomock.Any(), gomock.Any()). Return(workspacesdk.LSResponse{}, nil).AnyTimes() mockConn.EXPECT().ReadFile(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). Return(io.NopCloser(strings.NewReader("")), "", nil).AnyTimes() - server := newActiveTestServer(t, db, ps, func(cfg *chatd.Config) { cfg.AgentConn = func(_ context.Context, agentID uuid.UUID) (workspacesdk.AgentConn, func(), error) { require.Equal(t, dbAgent.ID, agentID) diff --git a/coderd/x/chatd/chattool/skill.go b/coderd/x/chatd/chattool/skill.go index 757cf96d14..1ae54a6c07 100644 --- a/coderd/x/chatd/chattool/skill.go +++ b/coderd/x/chatd/chattool/skill.go @@ -11,12 +11,11 @@ import ( "charm.land/fantasy" "golang.org/x/xerrors" + "cdr.dev/slog/v3" "github.com/coder/coder/v2/codersdk/workspacesdk" ) const ( - agentsSkillsDir = ".agents/skills" - skillMetaFile = "SKILL.md" maxSkillMetaBytes = 64 * 1024 maxSkillFileBytes = 512 * 1024 ) @@ -33,7 +32,7 @@ var skillNamePattern = regexp.MustCompile( // used by instruction.go in the parent package. var markdownCommentRe = regexp.MustCompile(``) -// SkillMeta is the frontmatter from a SKILL.md discovered in a +// SkillMeta is the frontmatter from a skill meta file discovered in a // workspace. It carries just enough information to list the skill // in the prompt index without reading the full body. type SkillMeta struct { @@ -52,91 +51,109 @@ type SkillContent struct { // delimiters have been stripped. Body string // Files lists relative paths of supporting files in the - // skill directory (everything except SKILL.md itself). + // skill directory (everything except the skill meta file). Files []string } -// DiscoverSkills walks the .agents/skills directory inside the -// workspace and returns metadata for every valid skill it finds. -// Missing directories or individual read errors are silently -// skipped so that a partially broken skills tree never blocks the -// conversation. +// DiscoverSkills walks the given skills directories and returns +// metadata for every valid skill it finds. Missing directories +// or individual read errors are silently skipped so that a +// partially broken skills tree never blocks the conversation. +// Skill names must be unique across directories; first +// occurrence wins. func DiscoverSkills( ctx context.Context, + logger slog.Logger, conn workspacesdk.AgentConn, - workingDir string, + skillsDirs []string, + metaFile string, ) ([]SkillMeta, error) { - skillsDirPath := path.Join(workingDir, agentsSkillsDir) - - lsResp, err := conn.LS(ctx, "", workspacesdk.LSRequest{ - Path: []string{skillsDirPath}, - Relativity: workspacesdk.LSRelativityRoot, - }) - if err != nil { - // The skills directory is entirely optional. Return - // nil for any error so skill discovery never blocks - // the conversation. - return nil, nil - } - + seen := make(map[string]struct{}) var skills []SkillMeta - for _, entry := range lsResp.Contents { - if !entry.IsDir { - continue - } - metaPath := path.Join( - entry.AbsolutePathString, skillMetaFile, - ) - reader, _, err := conn.ReadFile( - ctx, metaPath, 0, maxSkillMetaBytes+1, - ) - if err != nil { - // The directory may have been removed between the - // LS and this read, or it simply lacks a SKILL.md. - // Any error is non-fatal. - continue - } - raw, err := io.ReadAll(io.LimitReader(reader, maxSkillMetaBytes+1)) - reader.Close() - if err != nil { - continue - } - - // Silently truncate oversized metadata files so a - // single large file cannot exhaust memory. - if int64(len(raw)) > maxSkillMetaBytes { - raw = raw[:maxSkillMetaBytes] - } - - name, description, _, err := parseSkillFrontmatter( - string(raw), - ) - if err != nil { - continue - } - - // The directory name must match the declared name so - // skill references are unambiguous. - if name != entry.Name { - continue - } - if !skillNamePattern.MatchString(name) { - continue - } - - skills = append(skills, SkillMeta{ - Name: name, - Description: description, - Dir: entry.AbsolutePathString, + for _, skillsDirPath := range skillsDirs { + lsResp, err := conn.LS(ctx, "", workspacesdk.LSRequest{ + Path: []string{skillsDirPath}, + Relativity: workspacesdk.LSRelativityRoot, }) + if err != nil { + // The skills directory is entirely optional. + // Skip on any error. + continue + } + + for _, entry := range lsResp.Contents { + if !entry.IsDir { + continue + } + + metaPath := path.Join( + entry.AbsolutePathString, metaFile, + ) + reader, _, err := conn.ReadFile( + ctx, metaPath, 0, maxSkillMetaBytes+1, + ) + if err != nil { + // The directory may have been removed between + // the LS and this read, or it simply lacks the + // meta file. Any error is non-fatal. + continue + } + raw, err := io.ReadAll(io.LimitReader(reader, maxSkillMetaBytes+1)) + reader.Close() + if err != nil { + logger.Debug(ctx, "failed to read skill meta file", + slog.F("path", metaPath), slog.Error(err)) + continue + } + + // Silently truncate oversized metadata files so + // a single large file cannot exhaust memory. + if int64(len(raw)) > maxSkillMetaBytes { + raw = raw[:maxSkillMetaBytes] + } + + name, description, _, err := parseSkillFrontmatter( + string(raw), + ) + if err != nil { + logger.Debug(ctx, "failed to parse skill frontmatter", + slog.F("path", metaPath), slog.Error(err)) + continue + } + + // The directory name must match the declared name + // so skill references are unambiguous. + if name != entry.Name { + logger.Debug(ctx, "skill name does not match directory", + slog.F("dir", entry.Name), slog.F("declared_name", name)) + continue + } + if !skillNamePattern.MatchString(name) { + logger.Debug(ctx, "skill name does not match pattern", + slog.F("name", name)) + continue + } + + // First occurrence wins across directories. + if _, ok := seen[name]; ok { + continue + } + seen[name] = struct{}{} + + skills = append(skills, SkillMeta{ + Name: name, + Description: description, + Dir: entry.AbsolutePathString, + }) + } } return skills, nil } // parseSkillFrontmatter extracts name, description, and the -// markdown body from a SKILL.md file. The frontmatter uses a +// markdown body from a skill meta file. The frontmatter uses a // simple `key: value` format between `---` delimiters, and no // full YAML parser is needed. func parseSkillFrontmatter( @@ -228,15 +245,16 @@ func FormatSkillIndex(skills []SkillMeta) string { return b.String() } -// LoadSkillBody reads the full SKILL.md for a discovered skill +// LoadSkillBody reads the full skill meta file for a discovered skill // and lists the supporting files in its directory. The caller // should have already obtained the SkillMeta from DiscoverSkills. func LoadSkillBody( ctx context.Context, conn workspacesdk.AgentConn, skill SkillMeta, + metaFile string, ) (SkillContent, error) { - metaPath := path.Join(skill.Dir, skillMetaFile) + metaPath := path.Join(skill.Dir, metaFile) reader, _, err := conn.ReadFile( ctx, metaPath, 0, maxSkillMetaBytes+1, @@ -279,7 +297,7 @@ func LoadSkillBody( var files []string for _, entry := range lsResp.Contents { - if entry.Name == skillMetaFile { + if entry.Name == metaFile { continue } name := entry.Name @@ -366,6 +384,7 @@ func validateSkillFilePath(p string) error { type ReadSkillOptions struct { GetWorkspaceConn func(context.Context) (workspacesdk.AgentConn, error) GetSkills func() []SkillMeta + SkillMetaFile string } // ReadSkillArgs are the parameters accepted by read_skill. @@ -380,7 +399,7 @@ func ReadSkill(options ReadSkillOptions) fantasy.AgentTool { return fantasy.NewAgentTool( "read_skill", "Read the full instructions for a skill by name. "+ - "Returns the SKILL.md body and a list of "+ + "Returns the skill meta file body and a list of "+ "supporting files. Use read_skill before "+ "following a skill's instructions.", func(ctx context.Context, args ReadSkillArgs, _ fantasy.ToolCall) (fantasy.ToolResponse, error) { @@ -409,7 +428,7 @@ func ReadSkill(options ReadSkillOptions) fantasy.AgentTool { ), nil } - content, err := LoadSkillBody(ctx, conn, skill) + content, err := LoadSkillBody(ctx, conn, skill, options.SkillMetaFile) if err != nil { return fantasy.NewTextErrorResponse( err.Error(), diff --git a/coderd/x/chatd/chattool/skill_test.go b/coderd/x/chatd/chattool/skill_test.go index 66e57099fe..3f9775726f 100644 --- a/coderd/x/chatd/chattool/skill_test.go +++ b/coderd/x/chatd/chattool/skill_test.go @@ -12,6 +12,7 @@ import ( "go.uber.org/mock/gomock" "golang.org/x/xerrors" + "cdr.dev/slog/v3/sloggers/slogtest" "github.com/coder/coder/v2/coderd/x/chatd/chattool" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/workspacesdk" @@ -70,7 +71,7 @@ func TestDiscoverSkills(t *testing.T) { nil, ) - skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work") + skills, err := chattool.DiscoverSkills(context.Background(), slogtest.Make(t, nil), conn, []string{"/work/.agents/skills"}, "SKILL.md") require.NoError(t, err) require.Len(t, skills, 2) assert.Equal(t, "my-skill", skills[0].Name) @@ -89,7 +90,7 @@ func TestDiscoverSkills(t *testing.T) { codersdk.NewTestError(404, "POST", "/api/v0/list-directory"), ) - skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work") + skills, err := chattool.DiscoverSkills(context.Background(), slogtest.Make(t, nil), conn, []string{"/work/.agents/skills"}, "SKILL.md") require.NoError(t, err) require.Empty(t, skills) }) @@ -119,7 +120,7 @@ func TestDiscoverSkills(t *testing.T) { codersdk.NewTestError(404, "GET", "/api/v0/read-file"), ) - skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work") + skills, err := chattool.DiscoverSkills(context.Background(), slogtest.Make(t, nil), conn, []string{"/work/.agents/skills"}, "SKILL.md") require.NoError(t, err) require.Empty(t, skills) }) @@ -147,7 +148,7 @@ func TestDiscoverSkills(t *testing.T) { nil, ) - skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work") + skills, err := chattool.DiscoverSkills(context.Background(), slogtest.Make(t, nil), conn, []string{"/work/.agents/skills"}, "SKILL.md") require.NoError(t, err) require.Empty(t, skills) }) @@ -175,7 +176,7 @@ func TestDiscoverSkills(t *testing.T) { nil, ) - skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work") + skills, err := chattool.DiscoverSkills(context.Background(), slogtest.Make(t, nil), conn, []string{"/work/.agents/skills"}, "SKILL.md") require.NoError(t, err) require.Empty(t, skills) }) @@ -202,7 +203,7 @@ func TestDiscoverSkills(t *testing.T) { nil, ) - skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work") + skills, err := chattool.DiscoverSkills(context.Background(), slogtest.Make(t, nil), conn, []string{"/work/.agents/skills"}, "SKILL.md") require.NoError(t, err) require.Empty(t, skills) }) @@ -221,7 +222,7 @@ func TestDiscoverSkills(t *testing.T) { }, nil, ) - skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work") + skills, err := chattool.DiscoverSkills(context.Background(), slogtest.Make(t, nil), conn, []string{"/work/.agents/skills"}, "SKILL.md") require.NoError(t, err) require.Empty(t, skills) }) @@ -250,7 +251,7 @@ func TestDiscoverSkills(t *testing.T) { nil, ) - skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work") + skills, err := chattool.DiscoverSkills(context.Background(), slogtest.Make(t, nil), conn, []string{"/work/.agents/skills"}, "SKILL.md") require.NoError(t, err) require.Len(t, skills, 1) assert.Equal(t, "A quoted description", skills[0].Description) @@ -282,7 +283,7 @@ func TestDiscoverSkills(t *testing.T) { nil, ) - skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work") + skills, err := chattool.DiscoverSkills(context.Background(), slogtest.Make(t, nil), conn, []string{"/work/.agents/skills"}, "SKILL.md") require.NoError(t, err) // The skill should still be discovered since the // frontmatter fits within the truncation limit. @@ -314,7 +315,7 @@ func TestDiscoverSkills(t *testing.T) { nil, ) - skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work") + skills, err := chattool.DiscoverSkills(context.Background(), slogtest.Make(t, nil), conn, []string{"/work/.agents/skills"}, "SKILL.md") require.NoError(t, err) require.Len(t, skills, 1) assert.Equal(t, "bom-skill", skills[0].Name) @@ -383,7 +384,7 @@ func TestLoadSkillBody(t *testing.T) { }, nil, ) - content, err := chattool.LoadSkillBody(context.Background(), conn, skill) + content, err := chattool.LoadSkillBody(context.Background(), conn, skill, "SKILL.md") require.NoError(t, err) assert.Contains(t, content.Body, "Do the thing.") assert.Equal(t, []string{"helper.md", "roles/"}, content.Files) diff --git a/coderd/x/chatd/instruction.go b/coderd/x/chatd/instruction.go index fe8da3d4e3..0ba7bd83a2 100644 --- a/coderd/x/chatd/instruction.go +++ b/coderd/x/chatd/instruction.go @@ -19,25 +19,27 @@ import ( ) const ( - coderHomeInstructionDir = ".coder" - coderHomeInstructionFile = "AGENTS.md" - maxInstructionFileBytes = 64 * 1024 + maxInstructionFileBytes = 64 * 1024 ) var markdownCommentPattern = regexp.MustCompile(``) -// readHomeInstructionFile reads the ~/.coder/AGENTS.md file from the -// workspace agent's home directory. +// readHomeInstructionFile reads an instruction file from the +// agent's home directory using home-relative path resolution. +// This is the fallback for older agents that don't support +// the context-config endpoint. func readHomeInstructionFile( ctx context.Context, conn workspacesdk.AgentConn, + homeDir string, + fileName string, ) (content string, sourcePath string, truncated bool, err error) { if conn == nil { return "", "", false, nil } - coderDir, err := conn.LS(ctx, "", workspacesdk.LSRequest{ - Path: []string{coderHomeInstructionDir}, + dirListing, err := conn.LS(ctx, "", workspacesdk.LSRequest{ + Path: []string{homeDir}, Relativity: workspacesdk.LSRelativityHome, }) if err != nil { @@ -48,11 +50,52 @@ func readHomeInstructionFile( } var filePath string - for _, entry := range coderDir.Contents { + for _, entry := range dirListing.Contents { if entry.IsDir { continue } - if strings.EqualFold(strings.TrimSpace(entry.Name), coderHomeInstructionFile) { + if strings.EqualFold(strings.TrimSpace(entry.Name), fileName) { + filePath = strings.TrimSpace(entry.AbsolutePathString) + break + } + } + if filePath == "" { + return "", "", false, nil + } + + return readInstructionFile(ctx, conn, filePath) +} + +// readInstructionDirFile reads an instruction file from the given +// absolute directory path. The directory is listed and scanned +// for a file matching fileName (case-insensitive). +func readInstructionDirFile( + ctx context.Context, + conn workspacesdk.AgentConn, + absDir string, + fileName string, +) (content string, sourcePath string, truncated bool, err error) { + if conn == nil { + return "", "", false, nil + } + + dirListing, err := conn.LS(ctx, "", workspacesdk.LSRequest{ + Path: []string{absDir}, + Relativity: workspacesdk.LSRelativityRoot, + }) + if err != nil { + if isCodersdkStatusCode(err, http.StatusNotFound) { + return "", "", false, nil + } + return "", "", false, xerrors.Errorf("list instruction directory: %w", err) + } + + var filePath string + for _, entry := range dirListing.Contents { + if entry.IsDir { + continue + } + if strings.EqualFold(strings.TrimSpace(entry.Name), fileName) { filePath = strings.TrimSpace(entry.AbsolutePathString) break } @@ -234,13 +277,43 @@ func skillsFromParts( return skills } -// pwdInstructionFilePath returns the absolute path to the AGENTS.md -// file in the given working directory, or empty if directory is empty. -func pwdInstructionFilePath(directory string) string { - if directory == "" { +// pwdInstructionFilePath returns the absolute path to the +// instruction file in the given working directory, or empty if +// directory is empty. +func pwdInstructionFilePath(directory, fileName string) string { + if directory == "" || fileName == "" { return "" } - return path.Join(directory, coderHomeInstructionFile) + return path.Join(directory, fileName) +} + +// skillMetaFileFromParts scans persisted context-file parts for +// the stored skill meta file name. Uses last-wins semantics to +// match contextFileAgentID, so after an agent change the newest +// agent's value is returned. +func skillMetaFileFromParts( + messages []database.ChatMessage, +) string { + var result string + for _, msg := range messages { + if !msg.Content.Valid || + !bytes.Contains(msg.Content.RawMessage, []byte(`"context-file"`)) { + continue + } + var parts []codersdk.ChatMessagePart + if err := json.Unmarshal(msg.Content.RawMessage, &parts); err != nil { + continue + } + for _, part := range parts { + if part.Type != codersdk.ChatMessagePartTypeContextFile { + continue + } + if part.ContextFileSkillMetaFile != "" { + result = part.ContextFileSkillMetaFile + } + } + } + return result } func isCodersdkStatusCode(err error, statusCode int) bool { diff --git a/coderd/x/chatd/instruction_test.go b/coderd/x/chatd/instruction_test.go index 2cb2f25ff4..d666a67d64 100644 --- a/coderd/x/chatd/instruction_test.go +++ b/coderd/x/chatd/instruction_test.go @@ -2,14 +2,17 @@ package chatd //nolint:testpackage // Uses internal symbols. import ( "context" + "encoding/json" "io" "strings" "testing" "charm.land/fantasy" + "github.com/sqlc-dev/pqtype" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" + "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/x/chatd/chatprompt" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/workspacesdk" @@ -56,7 +59,7 @@ func TestReadHomeInstructionFileNotFound(t *testing.T) { }, ) - content, sourcePath, truncated, err := readHomeInstructionFile(context.Background(), conn) + content, sourcePath, truncated, err := readHomeInstructionFile(context.Background(), conn, ".coder", "AGENTS.md") require.NoError(t, err) require.Empty(t, content) require.Empty(t, sourcePath) @@ -90,7 +93,7 @@ func TestReadHomeInstructionFileSuccess(t *testing.T) { nil, ) - content, sourcePath, truncated, err := readHomeInstructionFile(context.Background(), conn) + content, sourcePath, truncated, err := readHomeInstructionFile(context.Background(), conn, ".coder", "AGENTS.md") require.NoError(t, err) require.Equal(t, "base\n\nlocal", content) require.Equal(t, "/home/coder/.coder/AGENTS.md", sourcePath) @@ -120,7 +123,7 @@ func TestReadHomeInstructionFileTruncates(t *testing.T) { int64(maxInstructionFileBytes+1), ).Return(io.NopCloser(strings.NewReader(content)), "text/markdown", nil) - got, _, truncated, err := readHomeInstructionFile(context.Background(), conn) + got, _, truncated, err := readHomeInstructionFile(context.Background(), conn, ".coder", "AGENTS.md") require.NoError(t, err) require.True(t, truncated) require.Len(t, got, maxInstructionFileBytes) @@ -300,6 +303,57 @@ func TestFormatSystemInstructions(t *testing.T) { func TestPwdInstructionFilePath(t *testing.T) { t.Parallel() - require.Equal(t, "/home/coder/project/AGENTS.md", pwdInstructionFilePath("/home/coder/project")) - require.Empty(t, pwdInstructionFilePath("")) + require.Equal(t, "/home/coder/project/AGENTS.md", pwdInstructionFilePath("/home/coder/project", "AGENTS.md")) + require.Empty(t, pwdInstructionFilePath("", "AGENTS.md")) +} + +func TestSkillMetaFileFromParts(t *testing.T) { + t.Parallel() + + makeMsg := func(parts []codersdk.ChatMessagePart) database.ChatMessage { + raw, err := json.Marshal(parts) + require.NoError(t, err) + return database.ChatMessage{ + Content: pqtype.NullRawMessage{RawMessage: raw, Valid: true}, + } + } + + t.Run("EmptyMessages", func(t *testing.T) { + t.Parallel() + require.Equal(t, "", skillMetaFileFromParts(nil)) + }) + + t.Run("NoContextFileParts", func(t *testing.T) { + t.Parallel() + msg := makeMsg([]codersdk.ChatMessagePart{{ + Type: codersdk.ChatMessagePartTypeText, + Text: "hello", + }}) + require.Equal(t, "", skillMetaFileFromParts([]database.ChatMessage{msg})) + }) + + t.Run("ReturnsLastMatch", func(t *testing.T) { + t.Parallel() + msg1 := makeMsg([]codersdk.ChatMessagePart{{ + Type: codersdk.ChatMessagePartTypeContextFile, + ContextFilePath: "/old/path", + ContextFileSkillMetaFile: "OLD.md", + }}) + msg2 := makeMsg([]codersdk.ChatMessagePart{{ + Type: codersdk.ChatMessagePartTypeContextFile, + ContextFilePath: "/new/path", + ContextFileSkillMetaFile: "NEW.md", + }}) + require.Equal(t, "NEW.md", skillMetaFileFromParts([]database.ChatMessage{msg1, msg2})) + }) + + t.Run("SkipsEmptyField", func(t *testing.T) { + t.Parallel() + msg := makeMsg([]codersdk.ChatMessagePart{{ + Type: codersdk.ChatMessagePartTypeContextFile, + ContextFilePath: "/some/path", + ContextFileSkillMetaFile: "", + }}) + require.Equal(t, "", skillMetaFileFromParts([]database.ChatMessage{msg})) + }) } diff --git a/codersdk/chats.go b/codersdk/chats.go index aaacb1a034..15ad632dd6 100644 --- a/codersdk/chats.go +++ b/codersdk/chats.go @@ -228,6 +228,12 @@ type ChatMessagePart struct { // the workspace filesystem. Internal only: used by // read_skill/read_skill_file tools to locate skill files. SkillDir string `json:"skill_dir,omitempty" typescript:"-"` + // ContextFileSkillMetaFile is the basename of the skill + // meta file (e.g. "SKILL.md") at the time of persistence. + // Internal only: restored on subsequent turns so the + // read_skill tool uses the correct filename even when the + // agent configured a non-default value. + ContextFileSkillMetaFile string `json:"context_file_skill_meta_file,omitempty" typescript:"-"` } // StripInternal removes internal-only fields that must not be @@ -245,6 +251,7 @@ func (p *ChatMessagePart) StripInternal() { p.ContextFileOS = "" p.ContextFileDirectory = "" p.SkillDir = "" + p.ContextFileSkillMetaFile = "" } // ChatMessageText builds a text chat message part. diff --git a/codersdk/chats_test.go b/codersdk/chats_test.go index 0d8c2f6a2f..219f8696f5 100644 --- a/codersdk/chats_test.go +++ b/codersdk/chats_test.go @@ -188,18 +188,20 @@ func TestChatMessagePart_StripInternal(t *testing.T) { t.Parallel() agentID := uuid.New() part := codersdk.ChatMessagePart{ - Type: codersdk.ChatMessagePartTypeContextFile, - ContextFilePath: "/home/coder/AGENTS.md", - ContextFileContent: "large content", - ContextFileAgentID: uuid.NullUUID{UUID: agentID, Valid: true}, - ContextFileOS: "linux", - ContextFileDirectory: "/home/coder/project", + Type: codersdk.ChatMessagePartTypeContextFile, + ContextFilePath: "/home/coder/AGENTS.md", + ContextFileContent: "large content", + ContextFileAgentID: uuid.NullUUID{UUID: agentID, Valid: true}, + ContextFileOS: "linux", + ContextFileDirectory: "/home/coder/project", + ContextFileSkillMetaFile: "CUSTOM.md", } part.StripInternal() // Internal fields stripped. assert.Empty(t, part.ContextFileContent) assert.Empty(t, part.ContextFileOS) assert.Empty(t, part.ContextFileDirectory) + assert.Empty(t, part.ContextFileSkillMetaFile) // Public fields preserved. assert.Equal(t, "/home/coder/AGENTS.md", part.ContextFilePath) assert.Equal(t, agentID, part.ContextFileAgentID.UUID) @@ -231,14 +233,15 @@ func TestChatMessagePartVariantTags(t *testing.T) { // If you add a new field to ChatMessagePart, either add a // variants tag or add it here with a comment explaining why. excludedFields := map[string]string{ - "type": "discriminant, added automatically by codegen", - "signature": "added in #22290, never populated by any code path", - "result_delta": "added in #22290, never populated by any code path", - "provider_metadata": "internal only, stripped by db2sdk before API responses", - "context_file_content": "internal only, stripped before API responses (typescript:\"-\")", - "context_file_os": "internal only, used during prompt expansion (typescript:\"-\")", - "context_file_directory": "internal only, used during prompt expansion (typescript:\"-\")", - "skill_dir": "internal only, used by read_skill tools (typescript:\"-\")", + "type": "discriminant, added automatically by codegen", + "signature": "added in #22290, never populated by any code path", + "result_delta": "added in #22290, never populated by any code path", + "provider_metadata": "internal only, stripped by db2sdk before API responses", + "context_file_content": "internal only, stripped before API responses (typescript:\"-\")", + "context_file_os": "internal only, used during prompt expansion (typescript:\"-\")", + "context_file_directory": "internal only, used during prompt expansion (typescript:\"-\")", + "skill_dir": "internal only, used by read_skill tools (typescript:\"-\")", + "context_file_skill_meta_file": "internal only, restored on subsequent turns (typescript:\"-\")", } knownTypes := make(map[codersdk.ChatMessagePartType]bool) for _, pt := range codersdk.AllChatMessagePartTypes() { diff --git a/codersdk/workspacesdk/agentconn.go b/codersdk/workspacesdk/agentconn.go index c8ee83894d..9c54018ab0 100644 --- a/codersdk/workspacesdk/agentconn.go +++ b/codersdk/workspacesdk/agentconn.go @@ -61,6 +61,7 @@ type AgentConn interface { AwaitReachable(ctx context.Context) bool CallMCPTool(ctx context.Context, req CallMCPToolRequest) (CallMCPToolResponse, error) Close() error + ContextConfig(ctx context.Context) (ContextConfigResponse, error) DebugLogs(ctx context.Context) ([]byte, error) DebugMagicsock(ctx context.Context) ([]byte, error) DebugManifest(ctx context.Context) ([]byte, error) @@ -946,6 +947,30 @@ type MCPToolInfo struct { Required []string `json:"required"` } +// Default values for context configuration. These are used +// by the agent when env vars are unset and by the server as +// fallbacks for older agents that don't support the +// context-config endpoint. +const ( + DefaultInstructionsDir = "~/.coder" + DefaultInstructionsFile = "AGENTS.md" + DefaultSkillsDir = ".agents/skills" + DefaultSkillMetaFile = "SKILL.md" + DefaultMCPConfigFile = ".mcp.json" +) + +// ContextConfigResponse is the response from the agent's +// context configuration endpoint. Directory fields contain +// fully resolved absolute paths. File name fields contain +// basenames. +type ContextConfigResponse struct { + InstructionsDirs []string `json:"instructions_dirs"` + InstructionsFile string `json:"instructions_file"` + SkillsDirs []string `json:"skills_dirs"` + SkillMetaFile string `json:"skill_meta_file"` + MCPConfigFiles []string `json:"mcp_config_files"` +} + // CallMCPToolRequest is the request body for proxying an MCP // tool call through the workspace agent. type CallMCPToolRequest struct { @@ -1018,6 +1043,23 @@ func (c *agentConn) ListMCPTools(ctx context.Context) (ListMCPToolsResponse, err return resp, json.NewDecoder(res.Body).Decode(&resp) } +// ContextConfig returns the resolved context configuration from +// the workspace agent. +func (c *agentConn) ContextConfig(ctx context.Context) (ContextConfigResponse, error) { + ctx, span := tracing.StartSpan(ctx) + defer span.End() + res, err := c.apiRequest(ctx, http.MethodGet, "/api/v0/context-config", nil) + if err != nil { + return ContextConfigResponse{}, xerrors.Errorf("do request: %w", err) + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return ContextConfigResponse{}, codersdk.ReadBodyAsError(res) + } + var resp ContextConfigResponse + return resp, json.NewDecoder(res.Body).Decode(&resp) +} + // CallMCPTool proxies a tool call to an MCP server running in // the workspace. func (c *agentConn) CallMCPTool(ctx context.Context, req CallMCPToolRequest) (CallMCPToolResponse, error) { diff --git a/codersdk/workspacesdk/agentconnmock/agentconnmock.go b/codersdk/workspacesdk/agentconnmock/agentconnmock.go index f3860e70c6..66b725b1b6 100644 --- a/codersdk/workspacesdk/agentconnmock/agentconnmock.go +++ b/codersdk/workspacesdk/agentconnmock/agentconnmock.go @@ -17,18 +17,19 @@ import ( reflect "reflect" time "time" - slog "cdr.dev/slog/v3" - codersdk "github.com/coder/coder/v2/codersdk" - healthsdk "github.com/coder/coder/v2/codersdk/healthsdk" - workspacesdk "github.com/coder/coder/v2/codersdk/workspacesdk" - wsjson "github.com/coder/coder/v2/codersdk/wsjson" - tailnet "github.com/coder/coder/v2/tailnet" uuid "github.com/google/uuid" gomock "go.uber.org/mock/gomock" ssh "golang.org/x/crypto/ssh" gonet "gvisor.dev/gvisor/pkg/tcpip/adapters/gonet" ipnstate "tailscale.com/ipn/ipnstate" speedtest "tailscale.com/net/speedtest" + + slog "cdr.dev/slog/v3" + codersdk "github.com/coder/coder/v2/codersdk" + healthsdk "github.com/coder/coder/v2/codersdk/healthsdk" + workspacesdk "github.com/coder/coder/v2/codersdk/workspacesdk" + wsjson "github.com/coder/coder/v2/codersdk/wsjson" + tailnet "github.com/coder/coder/v2/tailnet" ) // MockAgentConn is a mock of AgentConn interface. @@ -113,6 +114,21 @@ func (mr *MockAgentConnMockRecorder) ConnectDesktopVNC(ctx any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConnectDesktopVNC", reflect.TypeOf((*MockAgentConn)(nil).ConnectDesktopVNC), ctx) } +// ContextConfig mocks base method. +func (m *MockAgentConn) ContextConfig(ctx context.Context) (workspacesdk.ContextConfigResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ContextConfig", ctx) + ret0, _ := ret[0].(workspacesdk.ContextConfigResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ContextConfig indicates an expected call of ContextConfig. +func (mr *MockAgentConnMockRecorder) ContextConfig(ctx any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ContextConfig", reflect.TypeOf((*MockAgentConn)(nil).ContextConfig), ctx) +} + // DebugLogs mocks base method. func (m *MockAgentConn) DebugLogs(ctx context.Context) ([]byte, error) { m.ctrl.T.Helper()