mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat(coderd/x/chatd): serve workspace MCP tools and read_skill from pinned context (#26581)
This wires the last two consumer-side gaps of the agent-pushed workspace
context refactor. coderd already hydrates each chat's pinned context
(`chat_context_resources`), and `resolveTurnWorkspaceContext` already
prefers the pin for instruction files and skill metadata. This change
extends that preference to workspace MCP tools and the `read_skill`
body.
Workspace MCP tools are now built from the chat's pinned `mcp_server`
resources instead of a live `ListMCPTools` pull.
`resolveWorkspaceMCPTools` prefers the pin and falls back to live
discovery for chats whose agent has not reported context yet, gated the
same way as the instruction/skills pin: the pin wins whenever the chat
has any pinned rows, so a workspace with no MCP servers contributes no
tools rather than resurrecting stale ones. Because the agent reports
tool names unprefixed, each tool is re-prefixed to the
`{server}__{tool}` form and the pushed JSON Schema is split into
`properties` and `required` so the result matches what live discovery
produced. Calls still proxy through the workspace agent connection; the
snapshot carries tool definitions, not a way to execute them.
`read_skill` now serves a workspace skill's `SKILL.md` body from the
pinned snapshot (`SkillMeta.Meta`) instead of dialing the agent, so a
pinned chat keeps returning the same instructions even when the
workspace is unreachable. The supporting-file list stays a best-effort
live lookup, since the snapshot carries only the meta file per the agent
push contract.
The legacy live paths remain as the fallback for agents that have not
pushed context; RFC Release-5 cleanup of those paths is out of scope
here.
<details>
<summary>Implementation plan and decisions</summary>
### Background
The agentcontext refactor is mostly shipped across earlier PRs (#25983,
#26526, #26533, #26577, #26570, #26573): the agent resolves instruction
files, skills, and MCP servers into a snapshot, pushes it via
`PushContextState`, and coderd hydrates each chat's pinned context
(`chat_context_resources`). This PR closes the two remaining
consumer-side gaps.
### Key facts established from the code
- Pushed MCP tool names are **unprefixed** (`mcprunner` stores
`tool.Name`); the agent MCP proxy and `CallMCPTool` expect the
`{server}__{tool}` form (`agentmcp.ToolNameSep`). The pinned path
reconstructs the prefix for execution, matching the model-facing names
the legacy path produced.
- Legacy `agentmcp` sets `MCPToolInfo.Schema = InputSchema.Properties`
and `Required = InputSchema.Required` separately. The pushed
`input_schema` is the full JSON Schema object, so the pinned builder
extracts `properties` and `required` to match that shape.
- `SkillMetaBody.meta` is the verbatim SKILL.md. The supporting-file
list is **not** in the snapshot, so it is fetched live on demand
(best-effort).
- Gating mirrors `resolveTurnWorkspaceContext`: the pin wins when the
chat has any pinned rows; otherwise the live path is used.
### Changes
1. `chattool/skill.go`: add `SkillMeta.Meta []byte`; extract
`listSkillFiles` from `LoadSkillBody`; in `readWorkspaceSkillBody`, when
`Meta` is present, parse the body from it without dialing and list files
best-effort, else use the legacy live read.
2. `context_prompt.go`: populate `SkillMeta.Meta` in
`contextResourcesToPrompt`; add `workspaceMCPToolInfosFromResources`
(pinned `mcp_server` rows to `[]workspacesdk.MCPToolInfo` with prefixed
names and split properties/required) and `splitMCPInputSchema`.
3. `chatd.go`: add `pinnedWorkspaceMCPTools` (build tools from the pin,
ok-gated) and `resolveWorkspaceMCPTools` (pin-first, fall back to
`discoverWorkspaceMCPTools`).
4. `generation_preparer.go`: call `resolveWorkspaceMCPTools` instead of
`discoverWorkspaceMCPTools`.
### Tests
- `chattool/skill_test.go`: read_skill serves the pinned body without
dialing, lists files via LS, and still returns the body when the
workspace is unreachable.
- `context_prompt_internal_test.go`: `SkillMeta.Meta` is populated;
`workspaceMCPToolInfosFromResources` prefixing/properties/required/skip
behavior; `pinnedWorkspaceMCPTools` ok-gating and fallback dispatch.
</details>
---
*Opened by Coder Agents on behalf of @kylecarbs.*
This commit is contained in:
@@ -552,6 +552,65 @@ func (p *Server) discoverWorkspaceMCPTools(
|
||||
return tools
|
||||
}
|
||||
|
||||
// resolveWorkspaceMCPTools selects the workspace MCP tool set for a turn. It
|
||||
// prefers the chat's pinned context snapshot and falls back to the per-turn
|
||||
// live discovery path for chats whose agent has not reported context yet. The
|
||||
// two paths are mutually exclusive, mirroring resolveTurnWorkspaceContext for
|
||||
// instructions and skills.
|
||||
func (p *Server) resolveWorkspaceMCPTools(
|
||||
ctx context.Context,
|
||||
logger slog.Logger,
|
||||
chat database.Chat,
|
||||
workspaceCtx *turnWorkspaceContext,
|
||||
) []fantasy.AgentTool {
|
||||
pinned, ok, err := p.pinnedWorkspaceMCPTools(ctx, chat, workspaceCtx.getWorkspaceConn)
|
||||
if err != nil {
|
||||
// A pinned-read failure should not be more fatal than a live
|
||||
// discovery failure (which returns nil tools), so log and fall back
|
||||
// rather than aborting the turn.
|
||||
logger.Warn(ctx, "failed to read pinned workspace MCP tools; falling back to live discovery",
|
||||
slog.F("chat_id", chat.ID), slog.Error(err))
|
||||
} else if ok {
|
||||
return pinned
|
||||
}
|
||||
return p.discoverWorkspaceMCPTools(ctx, logger, chat.ID, workspaceCtx)
|
||||
}
|
||||
|
||||
// pinnedWorkspaceMCPTools builds workspace MCP tools from the chat's pinned
|
||||
// context snapshot (chat_context_resources) instead of dialing the agent for
|
||||
// a live tool list. ok reports whether the caller should use these tools
|
||||
// instead of the live discovery path. It is false when the chat has no pinned
|
||||
// rows (an older agent that never reported context, or a chat not yet
|
||||
// hydrated), so the caller falls back. When rows exist ok is true even if none
|
||||
// are MCP servers, because the pin is then authoritative: a workspace with no
|
||||
// MCP servers contributes no tools.
|
||||
//
|
||||
// Each tool still proxies its calls back through the workspace agent
|
||||
// connection; the snapshot carries tool definitions, not a way to execute
|
||||
// them, so execution requires a reachable agent. There is no per-chat cache to
|
||||
// invalidate on the pinned path: a server removed or renamed in the workspace
|
||||
// surfaces as a dirty chat on the agent's next push, and the user refreshes to
|
||||
// re-pin, so a nil invalidate callback (a 404 no-op) is correct here.
|
||||
func (p *Server) pinnedWorkspaceMCPTools(
|
||||
ctx context.Context,
|
||||
chat database.Chat,
|
||||
getConn func(context.Context) (workspacesdk.AgentConn, error),
|
||||
) (tools []fantasy.AgentTool, ok bool, err error) {
|
||||
resources, err := p.db.ListChatContextResourcesByChatID(ctx, chat.ID)
|
||||
if err != nil {
|
||||
return nil, false, xerrors.Errorf("list chat context resources: %w", err)
|
||||
}
|
||||
if len(resources) == 0 {
|
||||
return nil, false, nil
|
||||
}
|
||||
infos := workspaceMCPToolInfosFromResources(resources)
|
||||
tools = make([]fantasy.AgentTool, 0, len(infos))
|
||||
for _, info := range infos {
|
||||
tools = append(tools, chattool.NewWorkspaceMCPTool(info, getConn, nil))
|
||||
}
|
||||
return tools, true, nil
|
||||
}
|
||||
|
||||
// primeWorkspaceMCPCache populates workspaceMCPToolsCache after the
|
||||
// create_workspace or start_workspace tool finishes waiting for the
|
||||
// workspace agent to become reachable. By the time it runs the agent
|
||||
|
||||
@@ -37,6 +37,15 @@ type SkillMeta struct {
|
||||
// MetaFile is the basename of the skill meta file (e.g.
|
||||
// "SKILL.md"). When empty, DefaultSkillMetaFile is used.
|
||||
MetaFile string
|
||||
// Meta is the verbatim skill meta file (SKILL.md) content the
|
||||
// agent pushed in the workspace context snapshot: front-matter
|
||||
// plus body. When present, read_skill serves the body from it
|
||||
// instead of reading the file back over the workspace
|
||||
// connection, so a pinned chat keeps returning the same
|
||||
// instructions even when the workspace is unreachable. It is
|
||||
// empty on the legacy per-turn discovery path, where the body is
|
||||
// read live.
|
||||
Meta []byte
|
||||
}
|
||||
|
||||
// SkillContent is the full body of a skill, loaded on demand
|
||||
@@ -165,14 +174,32 @@ func LoadSkillBody(
|
||||
|
||||
// List supporting files so the model knows what it can
|
||||
// request via read_skill_file.
|
||||
files, err := listSkillFiles(ctx, conn, skill.Dir, metaFile)
|
||||
if err != nil {
|
||||
return SkillContent{}, err
|
||||
}
|
||||
|
||||
return SkillContent{
|
||||
SkillMeta: skill,
|
||||
Body: body,
|
||||
Files: files,
|
||||
}, nil
|
||||
}
|
||||
|
||||
// listSkillFiles lists the supporting files in a skill directory,
|
||||
// excluding the skill meta file itself. Directory entries are
|
||||
// suffixed with "/" so the model can tell them apart from files.
|
||||
func listSkillFiles(
|
||||
ctx context.Context,
|
||||
conn workspacesdk.AgentConn,
|
||||
dir, metaFile string,
|
||||
) ([]string, error) {
|
||||
lsResp, err := conn.LS(ctx, "", workspacesdk.LSRequest{
|
||||
Path: []string{skill.Dir},
|
||||
Path: []string{dir},
|
||||
Relativity: workspacesdk.LSRelativityRoot,
|
||||
})
|
||||
if err != nil {
|
||||
return SkillContent{}, xerrors.Errorf(
|
||||
"list skill directory: %w", err,
|
||||
)
|
||||
return nil, xerrors.Errorf("list skill directory: %w", err)
|
||||
}
|
||||
|
||||
var files []string
|
||||
@@ -186,14 +213,64 @@ func LoadSkillBody(
|
||||
}
|
||||
files = append(files, name)
|
||||
}
|
||||
return files, nil
|
||||
}
|
||||
|
||||
// loadPinnedWorkspaceSkillContent builds skill content from the
|
||||
// SKILL.md the agent pushed in the workspace context snapshot. The
|
||||
// body is parsed from the pinned bytes without dialing the workspace,
|
||||
// so it keeps working when the workspace is unreachable. The
|
||||
// supporting-file list is a best-effort live lookup, because the
|
||||
// snapshot carries only the meta file (per the agent push contract):
|
||||
// a missing connection or LS failure yields an empty file list rather
|
||||
// than failing the read.
|
||||
func loadPinnedWorkspaceSkillContent(
|
||||
ctx context.Context,
|
||||
options ReadSkillOptions,
|
||||
skill SkillMeta,
|
||||
) (SkillContent, error) {
|
||||
raw := skill.Meta
|
||||
if int64(len(raw)) > maxSkillMetaBytes {
|
||||
raw = raw[:maxSkillMetaBytes]
|
||||
}
|
||||
|
||||
_, _, body, err := workspacesdk.ParseSkillFrontmatter(string(raw))
|
||||
if err != nil {
|
||||
return SkillContent{}, xerrors.Errorf(
|
||||
"parse skill frontmatter: %w", err,
|
||||
)
|
||||
}
|
||||
|
||||
return SkillContent{
|
||||
SkillMeta: skill,
|
||||
Body: body,
|
||||
Files: files,
|
||||
Files: bestEffortSkillFiles(ctx, options, skill),
|
||||
}, nil
|
||||
}
|
||||
|
||||
// bestEffortSkillFiles lists a pinned skill's supporting files over the
|
||||
// workspace connection, returning nil when the connection or listing
|
||||
// fails so an unreachable workspace never blocks read_skill from
|
||||
// returning the pinned body.
|
||||
func bestEffortSkillFiles(
|
||||
ctx context.Context,
|
||||
options ReadSkillOptions,
|
||||
skill SkillMeta,
|
||||
) []string {
|
||||
if options.GetWorkspaceConn == nil {
|
||||
return nil
|
||||
}
|
||||
conn, err := options.GetWorkspaceConn(ctx)
|
||||
if err != nil {
|
||||
return nil
|
||||
}
|
||||
files, err := listSkillFiles(ctx, conn, skill.Dir, cmp.Or(skill.MetaFile, DefaultSkillMetaFile))
|
||||
if err != nil {
|
||||
return nil
|
||||
}
|
||||
return files
|
||||
}
|
||||
|
||||
// LoadSkillFile reads a supporting file from a skill's directory.
|
||||
// The relativePath is validated to prevent directory traversal and
|
||||
// access to hidden files.
|
||||
@@ -446,6 +523,19 @@ func readWorkspaceSkillBody(
|
||||
if !ok {
|
||||
return SkillContent{}, skillNotFoundResponse(requestedName), true
|
||||
}
|
||||
|
||||
// Pinned path: the SKILL.md body travels in the workspace context
|
||||
// snapshot, so it is served without dialing the workspace.
|
||||
if len(skill.Meta) > 0 {
|
||||
content, err := loadPinnedWorkspaceSkillContent(ctx, options, skill)
|
||||
if err != nil {
|
||||
return SkillContent{}, fantasy.NewTextErrorResponse(err.Error()), true
|
||||
}
|
||||
return content, fantasy.ToolResponse{}, false
|
||||
}
|
||||
|
||||
// Legacy path: read the SKILL.md body live over the workspace
|
||||
// connection for agents that have not pushed context.
|
||||
if options.GetWorkspaceConn == nil {
|
||||
return SkillContent{}, fantasy.NewTextErrorResponse(
|
||||
"workspace connection resolver is not configured",
|
||||
|
||||
@@ -353,6 +353,78 @@ func TestReadSkillTool(t *testing.T) {
|
||||
assert.Contains(t, resp.Content, "Do the thing.")
|
||||
})
|
||||
|
||||
t.Run("PinnedBodyFromMeta", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
conn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
|
||||
// Meta carries the pushed SKILL.md, so the body is served from the
|
||||
// pin: the conn must never be asked to ReadFile the SKILL.md. The
|
||||
// supporting-file list is still a live, best-effort LS.
|
||||
skills := []chattool.SkillMeta{{
|
||||
Name: "my-skill",
|
||||
Description: "test",
|
||||
Dir: "/work/.agents/skills/my-skill",
|
||||
Meta: []byte(validSkillMD("my-skill", "test")),
|
||||
}}
|
||||
|
||||
conn.EXPECT().LS(gomock.Any(), "", gomock.Any()).Return(
|
||||
workspacesdk.LSResponse{
|
||||
Contents: []workspacesdk.LSFile{
|
||||
{Name: "SKILL.md"},
|
||||
{Name: "helper.md"},
|
||||
},
|
||||
}, nil,
|
||||
)
|
||||
|
||||
tool := chattool.ReadSkill(chattool.ReadSkillOptions{
|
||||
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
|
||||
return conn, nil
|
||||
},
|
||||
GetSkills: func() []chattool.SkillMeta { return skills },
|
||||
})
|
||||
|
||||
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
||||
ID: "call-1",
|
||||
Name: "read_skill",
|
||||
Input: `{"name":"my-skill"}`,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.False(t, resp.IsError)
|
||||
assert.Contains(t, resp.Content, "Do the thing.")
|
||||
assert.Contains(t, resp.Content, "helper.md")
|
||||
})
|
||||
|
||||
t.Run("PinnedBodyServedWhenWorkspaceUnreachable", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// With the body pinned, an unreachable workspace must not block
|
||||
// read_skill: the body is returned and the file list degrades to empty.
|
||||
skills := []chattool.SkillMeta{{
|
||||
Name: "my-skill",
|
||||
Dir: "/work/.agents/skills/my-skill",
|
||||
Meta: []byte(validSkillMD("my-skill", "test")),
|
||||
}}
|
||||
|
||||
tool := chattool.ReadSkill(chattool.ReadSkillOptions{
|
||||
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
|
||||
return nil, xerrors.New("workspace is stopped")
|
||||
},
|
||||
GetSkills: func() []chattool.SkillMeta { return skills },
|
||||
})
|
||||
|
||||
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
||||
ID: "call-1",
|
||||
Name: "read_skill",
|
||||
Input: `{"name":"my-skill"}`,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.False(t, resp.IsError)
|
||||
assert.Contains(t, resp.Content, "Do the thing.")
|
||||
assert.Contains(t, resp.Content, `"files":[]`)
|
||||
})
|
||||
|
||||
t.Run("PersonalSkill", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -7,12 +7,14 @@ import (
|
||||
|
||||
"golang.org/x/xerrors"
|
||||
"google.golang.org/protobuf/encoding/protojson"
|
||||
"google.golang.org/protobuf/types/known/structpb"
|
||||
|
||||
"cdr.dev/slog/v3"
|
||||
agentproto "github.com/coder/coder/v2/agent/proto"
|
||||
"github.com/coder/coder/v2/coderd/database"
|
||||
"github.com/coder/coder/v2/coderd/x/chatd/chattool"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/codersdk/workspacesdk"
|
||||
)
|
||||
|
||||
// contextBodyUnmarshalOptions reads the protojson resource bodies written by
|
||||
@@ -41,6 +43,13 @@ func decodeSkillMetaBody(body json.RawMessage) (*agentproto.SkillMetaBody, bool)
|
||||
return &decoded, true
|
||||
}
|
||||
|
||||
// mcpToolNameSeparator joins a server name and a tool name into the
|
||||
// flattened "<server>__<tool>" form. The agent reports MCP tool names
|
||||
// unprefixed alongside the server name; the workspace agent's MCP proxy
|
||||
// expects this flattened form to route a call back to the owning server
|
||||
// (see agent/x/agentmcp ToolNameSep).
|
||||
const mcpToolNameSeparator = "__"
|
||||
|
||||
// mcpToolsFromServerBody decodes a stored mcp_server resource body and returns
|
||||
// its tool list for the chat response. The agent prefixes each tool name with
|
||||
// "<server>__"; that prefix is stripped so the name reads as the server
|
||||
@@ -54,7 +63,7 @@ func mcpToolsFromServerBody(server string, body json.RawMessage) []codersdk.Chat
|
||||
if len(tools) == 0 {
|
||||
return nil
|
||||
}
|
||||
prefix := server + "__"
|
||||
prefix := server + mcpToolNameSeparator
|
||||
out := make([]codersdk.ChatContextTool, 0, len(tools))
|
||||
for _, t := range tools {
|
||||
name := strings.TrimPrefix(t.GetName(), prefix)
|
||||
@@ -72,6 +81,68 @@ func mcpToolsFromServerBody(server string, body json.RawMessage) []codersdk.Chat
|
||||
return out
|
||||
}
|
||||
|
||||
// workspaceMCPToolInfosFromResources decodes a chat's pinned mcp_server
|
||||
// resources into execution-ready tool infos. Only OK mcp_server rows
|
||||
// contribute. The agent reports tool names unprefixed alongside the server
|
||||
// name, so each tool is re-prefixed to "<server>__<tool>", the model-facing
|
||||
// and proxy-routable form the live discovery path also produces. The pushed
|
||||
// input schema is a full JSON Schema object; its "properties" and "required"
|
||||
// are split out to match the shape the workspace agent's live tool list
|
||||
// produces (see agent/x/agentmcp). Tools with an empty name are skipped.
|
||||
func workspaceMCPToolInfosFromResources(resources []database.ChatContextResource) []workspacesdk.MCPToolInfo {
|
||||
var out []workspacesdk.MCPToolInfo
|
||||
for _, r := range resources {
|
||||
if r.BodyKind != database.WorkspaceAgentContextBodyKindMcpServer ||
|
||||
r.Status != database.WorkspaceAgentContextResourceStatusOk {
|
||||
continue
|
||||
}
|
||||
var decoded agentproto.MCPServerBody
|
||||
if err := contextBodyUnmarshalOptions.Unmarshal(r.Body, &decoded); err != nil {
|
||||
continue
|
||||
}
|
||||
server := decoded.GetServerName()
|
||||
if server == "" {
|
||||
server = r.Source
|
||||
}
|
||||
for _, t := range decoded.GetTools() {
|
||||
name := t.GetName()
|
||||
if name == "" {
|
||||
continue
|
||||
}
|
||||
properties, required := splitMCPInputSchema(t.GetInputSchema())
|
||||
out = append(out, workspacesdk.MCPToolInfo{
|
||||
ServerName: server,
|
||||
Name: server + mcpToolNameSeparator + name,
|
||||
Description: t.GetDescription(),
|
||||
Schema: properties,
|
||||
Required: required,
|
||||
})
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// splitMCPInputSchema splits a pushed JSON Schema object into the properties
|
||||
// map and required list the workspace MCP tool wrapper expects. A nil schema,
|
||||
// or one missing these keys, yields nil for the absent part.
|
||||
func splitMCPInputSchema(schema *structpb.Struct) (properties map[string]any, required []string) {
|
||||
if schema == nil {
|
||||
return nil, nil
|
||||
}
|
||||
m := schema.AsMap()
|
||||
if props, ok := m["properties"].(map[string]any); ok {
|
||||
properties = props
|
||||
}
|
||||
if raw, ok := m["required"].([]any); ok {
|
||||
for _, v := range raw {
|
||||
if s, ok := v.(string); ok {
|
||||
required = append(required, s)
|
||||
}
|
||||
}
|
||||
}
|
||||
return properties, required
|
||||
}
|
||||
|
||||
// decodeInstructionContent decodes an instruction-file resource body and
|
||||
// returns its sanitized content. decoded is false when the body cannot be
|
||||
// decoded, letting the prompt path count it as malformed; content is empty
|
||||
@@ -221,23 +292,26 @@ func contextResourcesToPrompt(
|
||||
ContextFileContent: content,
|
||||
})
|
||||
case database.WorkspaceAgentContextBodyKindSkill:
|
||||
name, description, decoded := decodeSkillIdentity(r.Body)
|
||||
if !decoded {
|
||||
decodedBody, ok := decodeSkillMetaBody(r.Body)
|
||||
if !ok {
|
||||
malformed++
|
||||
continue
|
||||
}
|
||||
if name == "" {
|
||||
if decodedBody.GetName() == "" {
|
||||
continue
|
||||
}
|
||||
// source is the skill directory. MetaFile is left empty so
|
||||
// chattool falls back to DefaultSkillMetaFile ("SKILL.md").
|
||||
// SkillMetaBody carries no meta file name, so a non-default
|
||||
// CODER_AGENT_EXP_SKILL_META_FILE is not preserved on this
|
||||
// path, unlike the per-turn discovery path.
|
||||
// path, unlike the per-turn discovery path. Meta carries the
|
||||
// verbatim SKILL.md so read_skill serves the body from the
|
||||
// pin instead of dialing the workspace.
|
||||
skills = append(skills, chattool.SkillMeta{
|
||||
Name: name,
|
||||
Description: description,
|
||||
Name: decodedBody.GetName(),
|
||||
Description: decodedBody.GetDescription(),
|
||||
Dir: r.Source,
|
||||
Meta: decodedBody.GetMeta(),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -13,6 +13,7 @@ import (
|
||||
"golang.org/x/xerrors"
|
||||
"google.golang.org/protobuf/encoding/protojson"
|
||||
"google.golang.org/protobuf/proto"
|
||||
"google.golang.org/protobuf/types/known/structpb"
|
||||
|
||||
"cdr.dev/slog/v3"
|
||||
"cdr.dev/slog/v3/sloggers/slogtest"
|
||||
@@ -23,6 +24,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/database/dbtestutil"
|
||||
"github.com/coder/coder/v2/coderd/database/dbtime"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/codersdk/workspacesdk"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
)
|
||||
|
||||
@@ -57,6 +59,23 @@ func skillResource(t *testing.T, source, name, description string, status databa
|
||||
}
|
||||
}
|
||||
|
||||
func mcpServerResource(t *testing.T, source string, body *agentproto.MCPServerBody, status database.WorkspaceAgentContextResourceStatus) database.ChatContextResource {
|
||||
t.Helper()
|
||||
return database.ChatContextResource{
|
||||
Source: source,
|
||||
BodyKind: database.WorkspaceAgentContextBodyKindMcpServer,
|
||||
Body: mustMarshalContextBody(t, body),
|
||||
Status: status,
|
||||
}
|
||||
}
|
||||
|
||||
func mustStruct(t *testing.T, m map[string]any) *structpb.Struct {
|
||||
t.Helper()
|
||||
s, err := structpb.NewStruct(m)
|
||||
require.NoError(t, err)
|
||||
return s
|
||||
}
|
||||
|
||||
func TestContextResourcesToPrompt(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -93,6 +112,9 @@ func TestContextResourcesToPrompt(t *testing.T) {
|
||||
require.Equal(t, "/home/coder/.coder/skills/deploy", skills[0].Dir)
|
||||
// MetaFile is left empty so chattool defaults to SKILL.md.
|
||||
require.Empty(t, skills[0].MetaFile)
|
||||
// Meta carries the pushed SKILL.md so read_skill serves the body
|
||||
// from the pin without dialing the workspace.
|
||||
require.Equal(t, []byte("# deploy"), skills[0].Meta)
|
||||
})
|
||||
|
||||
t.Run("SkipsNonOKStatus", func(t *testing.T) {
|
||||
@@ -700,3 +722,167 @@ func TestContextResources(t *testing.T) {
|
||||
require.Error(t, err)
|
||||
})
|
||||
}
|
||||
|
||||
func TestWorkspaceMCPToolInfosFromResources(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
t.Run("BuildsPrefixedToolsFromMCPServers", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
schema := mustStruct(t, map[string]any{
|
||||
"type": "object",
|
||||
"properties": map[string]any{
|
||||
"title": map[string]any{"type": "string"},
|
||||
"body": map[string]any{"type": "string"},
|
||||
},
|
||||
"required": []any{"title"},
|
||||
})
|
||||
resources := []database.ChatContextResource{
|
||||
// Skipped: a config resource carries no tools.
|
||||
{
|
||||
Source: "/home/coder/.mcp.json",
|
||||
BodyKind: database.WorkspaceAgentContextBodyKindMcpConfig,
|
||||
Body: mustMarshalContextBody(t, &agentproto.MCPConfigBody{}),
|
||||
Status: database.WorkspaceAgentContextResourceStatusOk,
|
||||
},
|
||||
mcpServerResource(t, "github", &agentproto.MCPServerBody{
|
||||
ServerName: "github",
|
||||
Tools: []*agentproto.MCPTool{
|
||||
{Name: "create_issue", Description: "Create an issue", InputSchema: schema},
|
||||
// Skipped: a tool with no name cannot be addressed.
|
||||
{Name: "", Description: "nameless"},
|
||||
},
|
||||
}, database.WorkspaceAgentContextResourceStatusOk),
|
||||
// Skipped: a server that failed to connect is not OK.
|
||||
mcpServerResource(t, "broken", &agentproto.MCPServerBody{ServerName: "broken"},
|
||||
database.WorkspaceAgentContextResourceStatusUnreadable),
|
||||
}
|
||||
|
||||
infos := workspaceMCPToolInfosFromResources(resources)
|
||||
require.Len(t, infos, 1)
|
||||
require.Equal(t, "github", infos[0].ServerName)
|
||||
// Tool names are re-prefixed with the server name so the workspace
|
||||
// agent's MCP proxy routes the call to the owning server.
|
||||
require.Equal(t, "github__create_issue", infos[0].Name)
|
||||
require.Equal(t, "Create an issue", infos[0].Description)
|
||||
require.Equal(t, []string{"title"}, infos[0].Required)
|
||||
// Schema is the JSON Schema "properties" sub-map, matching the shape the
|
||||
// live discovery path produces; "required" travels separately.
|
||||
require.Contains(t, infos[0].Schema, "title")
|
||||
require.Contains(t, infos[0].Schema, "body")
|
||||
require.NotContains(t, infos[0].Schema, "required")
|
||||
})
|
||||
|
||||
t.Run("FallsBackToSourceWhenServerNameEmpty", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
resources := []database.ChatContextResource{
|
||||
mcpServerResource(t, "playwright", &agentproto.MCPServerBody{
|
||||
Tools: []*agentproto.MCPTool{{Name: "navigate"}},
|
||||
}, database.WorkspaceAgentContextResourceStatusOk),
|
||||
}
|
||||
infos := workspaceMCPToolInfosFromResources(resources)
|
||||
require.Len(t, infos, 1)
|
||||
require.Equal(t, "playwright", infos[0].ServerName)
|
||||
require.Equal(t, "playwright__navigate", infos[0].Name)
|
||||
})
|
||||
|
||||
t.Run("NoMCPServersYieldsNil", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
resources := []database.ChatContextResource{
|
||||
instructionResource(t, "/home/coder/AGENTS.md", "be helpful", database.WorkspaceAgentContextResourceStatusOk),
|
||||
}
|
||||
require.Empty(t, workspaceMCPToolInfosFromResources(resources))
|
||||
})
|
||||
}
|
||||
|
||||
func TestPinnedWorkspaceMCPTools(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// getConn is never dialed by these tests: pinnedWorkspaceMCPTools builds
|
||||
// tool definitions from the snapshot and only wires the connection for
|
||||
// later execution.
|
||||
getConn := func(context.Context) (workspacesdk.AgentConn, error) {
|
||||
return nil, xerrors.New("not dialed in this test")
|
||||
}
|
||||
|
||||
t.Run("NoRowsFallsBack", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
db := dbmock.NewMockStore(ctrl)
|
||||
chatID := uuid.New()
|
||||
db.EXPECT().ListChatContextResourcesByChatID(gomock.Any(), chatID).
|
||||
Return([]database.ChatContextResource{}, nil)
|
||||
server := newPinServer(t, db)
|
||||
|
||||
tools, ok, err := server.pinnedWorkspaceMCPTools(context.Background(), database.Chat{ID: chatID}, getConn)
|
||||
require.NoError(t, err)
|
||||
require.False(t, ok)
|
||||
require.Empty(t, tools)
|
||||
})
|
||||
|
||||
t.Run("ListError", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
db := dbmock.NewMockStore(ctrl)
|
||||
chatID := uuid.New()
|
||||
db.EXPECT().ListChatContextResourcesByChatID(gomock.Any(), chatID).
|
||||
Return(nil, xerrors.New("boom"))
|
||||
server := newPinServer(t, db)
|
||||
|
||||
_, ok, err := server.pinnedWorkspaceMCPTools(context.Background(), database.Chat{ID: chatID}, getConn)
|
||||
require.Error(t, err)
|
||||
require.False(t, ok)
|
||||
})
|
||||
|
||||
t.Run("BuildsToolsFromMCPServers", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
db := dbmock.NewMockStore(ctrl)
|
||||
chatID := uuid.New()
|
||||
db.EXPECT().ListChatContextResourcesByChatID(gomock.Any(), chatID).
|
||||
Return([]database.ChatContextResource{
|
||||
instructionResource(t, "/home/coder/AGENTS.md", "be helpful", database.WorkspaceAgentContextResourceStatusOk),
|
||||
mcpServerResource(t, "github", &agentproto.MCPServerBody{
|
||||
ServerName: "github",
|
||||
Tools: []*agentproto.MCPTool{
|
||||
{Name: "create_issue", Description: "Create an issue"},
|
||||
{Name: "search", Description: "Search code"},
|
||||
},
|
||||
}, database.WorkspaceAgentContextResourceStatusOk),
|
||||
}, nil)
|
||||
server := newPinServer(t, db)
|
||||
|
||||
tools, ok, err := server.pinnedWorkspaceMCPTools(context.Background(), database.Chat{ID: chatID}, getConn)
|
||||
require.NoError(t, err)
|
||||
require.True(t, ok)
|
||||
require.Len(t, tools, 2)
|
||||
require.Equal(t, "github__create_issue", tools[0].Info().Name)
|
||||
require.Equal(t, "github__search", tools[1].Info().Name)
|
||||
})
|
||||
|
||||
t.Run("PinWithoutMCPServersIsAuthoritative", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
db := dbmock.NewMockStore(ctrl)
|
||||
chatID := uuid.New()
|
||||
// The chat is pinned (an instruction file is present) but the agent
|
||||
// reported no MCP servers: ok is true with zero tools so the caller does
|
||||
// not fall back to a live pull that could resurrect stale tools.
|
||||
db.EXPECT().ListChatContextResourcesByChatID(gomock.Any(), chatID).
|
||||
Return([]database.ChatContextResource{
|
||||
instructionResource(t, "/home/coder/AGENTS.md", "be helpful", database.WorkspaceAgentContextResourceStatusOk),
|
||||
}, nil)
|
||||
server := newPinServer(t, db)
|
||||
|
||||
tools, ok, err := server.pinnedWorkspaceMCPTools(context.Background(), database.Chat{ID: chatID}, getConn)
|
||||
require.NoError(t, err)
|
||||
require.True(t, ok)
|
||||
require.Empty(t, tools)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -267,7 +267,7 @@ func (server *Server) prepareGeneration(
|
||||
}
|
||||
if chat.WorkspaceID.Valid && !isPlanModeTurn && !isExploreSubagent {
|
||||
g2.Go(func() error {
|
||||
workspaceMCPTools = server.discoverWorkspaceMCPTools(ctx, logger, chat.ID, &workspaceCtx)
|
||||
workspaceMCPTools = server.resolveWorkspaceMCPTools(ctx, logger, chat, &workspaceCtx)
|
||||
return nil
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user