mirror of
https://github.com/coder/coder.git
synced 2026-06-03 04:58:23 +00:00
919dc299fc
Piggybacks on #23878. Moves instruction file reading and skill discovery from `chatd` (server-side, via multiple `LS`/`ReadFile` round-trips through the agent connection) to the agent itself (local filesystem access). This intentionally drops backward compatibility with older agents that don't support the context-config endpoint. Agents and server are deployed together; there is no rolling-update contract to maintain here. ## What changed The agent's `GET /api/v0/context-config` response now returns `[]ChatMessagePart` directly — the same types chatd persists. This eliminates intermediate type conversions and makes the protocol extensible. | Field | Type | Description | |---|---|---| | `parts` | `[]ChatMessagePart` | Context-file and skill parts, ready to persist | | `working_dir` | `string` | Agent's resolved working directory | Removed from the response: `instructions_dirs`, `instructions_file`, `skills_dirs`, `skill_meta_file`, `mcp_config_files` — the agent reads files locally and returns their content as parts. Removed from chatd: all legacy `LS`/`ReadFile` fallback code (`readHomeInstructionFile`, `readInstructionDirFile`, `DiscoverSkills` via LS, etc). ## Why The previous architecture had the agent resolve paths, serve them over HTTP, then `chatd` make N+1 round-trips back through the agent connection to read files. The agent has direct filesystem access and should just read the files. ## Key design decisions - **Agent returns `ChatMessagePart` directly** — same types chatd persists. No intermediate `InstructionFileEntry`/`SkillEntry` types needed. - **`SkillMeta.MetaFile`** — persisted via `ContextFileSkillMetaFile` on the skill part, so custom meta file names (`CODER_AGENT_EXP_SKILL_META_FILE`) survive across chat turns. - **No pre-read body** — `read_skill` always dials the workspace to fetch the skill body on demand. Simpler than caching the body in the response. - **MCP config paths kept agent-internal** — `MCPConfigFiles()` getter, not sent over the wire. - **No backward compat fallback** — old agents that don't support context-config get no instruction files. This is acceptable since agent and server deploy together.
391 lines
9.5 KiB
Go
391 lines
9.5 KiB
Go
package chattool_test
|
|
|
|
import (
|
|
"context"
|
|
"io"
|
|
"strings"
|
|
"testing"
|
|
|
|
"charm.land/fantasy"
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
"go.uber.org/mock/gomock"
|
|
"golang.org/x/xerrors"
|
|
|
|
"github.com/coder/coder/v2/coderd/x/chatd/chattool"
|
|
"github.com/coder/coder/v2/codersdk/workspacesdk"
|
|
"github.com/coder/coder/v2/codersdk/workspacesdk/agentconnmock"
|
|
)
|
|
|
|
// validSkillMD returns a valid SKILL.md with the given name and
|
|
// description.
|
|
func validSkillMD(name, description string) string {
|
|
return "---\nname: " + name + "\ndescription: " + description + "\n---\n\n# Instructions\n\nDo the thing.\n"
|
|
}
|
|
|
|
func TestFormatSkillIndex(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
t.Run("Empty", func(t *testing.T) {
|
|
t.Parallel()
|
|
assert.Empty(t, chattool.FormatSkillIndex(nil))
|
|
})
|
|
|
|
t.Run("RendersIndex", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
skills := []chattool.SkillMeta{
|
|
{Name: "alpha", Description: "First"},
|
|
{Name: "beta", Description: "Second"},
|
|
}
|
|
idx := chattool.FormatSkillIndex(skills)
|
|
assert.Contains(t, idx, "<available-skills>")
|
|
assert.Contains(t, idx, "- alpha: First")
|
|
assert.Contains(t, idx, "- beta: Second")
|
|
assert.Contains(t, idx, "</available-skills>")
|
|
assert.Contains(t, idx, "read_skill")
|
|
})
|
|
}
|
|
|
|
func TestLoadSkillBody(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
t.Run("ReturnsBodyAndFiles", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
ctrl := gomock.NewController(t)
|
|
conn := agentconnmock.NewMockAgentConn(ctrl)
|
|
|
|
skill := chattool.SkillMeta{
|
|
Name: "my-skill",
|
|
Description: "desc",
|
|
Dir: "/work/.agents/skills/my-skill",
|
|
}
|
|
|
|
// Read the full SKILL.md.
|
|
conn.EXPECT().ReadFile(
|
|
gomock.Any(),
|
|
"/work/.agents/skills/my-skill/SKILL.md",
|
|
int64(0),
|
|
int64(64*1024+1),
|
|
).Return(
|
|
io.NopCloser(strings.NewReader(validSkillMD("my-skill", "desc"))),
|
|
"text/markdown",
|
|
nil,
|
|
)
|
|
|
|
// List supporting files.
|
|
conn.EXPECT().LS(gomock.Any(), "", gomock.Any()).Return(
|
|
workspacesdk.LSResponse{
|
|
Contents: []workspacesdk.LSFile{
|
|
{Name: "SKILL.md"},
|
|
{Name: "helper.md"},
|
|
{Name: "roles", IsDir: true},
|
|
},
|
|
}, nil,
|
|
)
|
|
|
|
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)
|
|
})
|
|
}
|
|
|
|
func TestLoadSkillFile(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
t.Run("ValidFile", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
ctrl := gomock.NewController(t)
|
|
conn := agentconnmock.NewMockAgentConn(ctrl)
|
|
|
|
skill := chattool.SkillMeta{
|
|
Name: "my-skill",
|
|
Dir: "/work/.agents/skills/my-skill",
|
|
}
|
|
|
|
conn.EXPECT().ReadFile(
|
|
gomock.Any(),
|
|
"/work/.agents/skills/my-skill/roles/reviewer.md",
|
|
int64(0),
|
|
int64(512*1024+1),
|
|
).Return(
|
|
io.NopCloser(strings.NewReader("review instructions")),
|
|
"text/markdown",
|
|
nil,
|
|
)
|
|
|
|
content, err := chattool.LoadSkillFile(
|
|
context.Background(), conn, skill, "roles/reviewer.md",
|
|
)
|
|
require.NoError(t, err)
|
|
assert.Equal(t, "review instructions", content)
|
|
})
|
|
|
|
t.Run("PathTraversalRejected", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
ctrl := gomock.NewController(t)
|
|
conn := agentconnmock.NewMockAgentConn(ctrl)
|
|
|
|
skill := chattool.SkillMeta{
|
|
Name: "my-skill",
|
|
Dir: "/work/.agents/skills/my-skill",
|
|
}
|
|
|
|
_, err := chattool.LoadSkillFile(
|
|
context.Background(), conn, skill, "../../etc/passwd",
|
|
)
|
|
require.Error(t, err)
|
|
assert.Contains(t, err.Error(), "traversal")
|
|
})
|
|
|
|
t.Run("AbsolutePathRejected", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
ctrl := gomock.NewController(t)
|
|
conn := agentconnmock.NewMockAgentConn(ctrl)
|
|
|
|
skill := chattool.SkillMeta{
|
|
Name: "my-skill",
|
|
Dir: "/work/.agents/skills/my-skill",
|
|
}
|
|
|
|
_, err := chattool.LoadSkillFile(
|
|
context.Background(), conn, skill, "/etc/passwd",
|
|
)
|
|
require.Error(t, err)
|
|
assert.Contains(t, err.Error(), "absolute")
|
|
})
|
|
|
|
t.Run("HiddenFileRejected", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
ctrl := gomock.NewController(t)
|
|
conn := agentconnmock.NewMockAgentConn(ctrl)
|
|
|
|
skill := chattool.SkillMeta{
|
|
Name: "my-skill",
|
|
Dir: "/work/.agents/skills/my-skill",
|
|
}
|
|
|
|
_, err := chattool.LoadSkillFile(
|
|
context.Background(), conn, skill, ".git/config",
|
|
)
|
|
require.Error(t, err)
|
|
assert.Contains(t, err.Error(), "hidden")
|
|
})
|
|
|
|
t.Run("EmptyPathRejected", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
ctrl := gomock.NewController(t)
|
|
conn := agentconnmock.NewMockAgentConn(ctrl)
|
|
|
|
skill := chattool.SkillMeta{
|
|
Name: "my-skill",
|
|
Dir: "/work/.agents/skills/my-skill",
|
|
}
|
|
|
|
_, err := chattool.LoadSkillFile(
|
|
context.Background(), conn, skill, "",
|
|
)
|
|
require.Error(t, err)
|
|
assert.Contains(t, err.Error(), "required")
|
|
})
|
|
|
|
t.Run("OversizedFileTruncated", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
ctrl := gomock.NewController(t)
|
|
conn := agentconnmock.NewMockAgentConn(ctrl)
|
|
|
|
skill := chattool.SkillMeta{
|
|
Name: "my-skill",
|
|
Dir: "/work/.agents/skills/my-skill",
|
|
}
|
|
|
|
// Build a file that exceeds maxSkillFileBytes (512KB).
|
|
bigContent := strings.Repeat("x", 512*1024+100)
|
|
|
|
conn.EXPECT().ReadFile(
|
|
gomock.Any(),
|
|
"/work/.agents/skills/my-skill/large.txt",
|
|
int64(0),
|
|
int64(512*1024+1),
|
|
).Return(
|
|
io.NopCloser(strings.NewReader(bigContent)),
|
|
"text/plain",
|
|
nil,
|
|
)
|
|
|
|
content, err := chattool.LoadSkillFile(
|
|
context.Background(), conn, skill, "large.txt",
|
|
)
|
|
require.NoError(t, err)
|
|
assert.Equal(t, 512*1024, len(content),
|
|
"content should be truncated to maxSkillFileBytes")
|
|
})
|
|
}
|
|
|
|
func TestReadSkillTool(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
t.Run("ValidSkill", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
ctrl := gomock.NewController(t)
|
|
conn := agentconnmock.NewMockAgentConn(ctrl)
|
|
|
|
skills := []chattool.SkillMeta{{
|
|
Name: "my-skill",
|
|
Description: "test",
|
|
Dir: "/work/.agents/skills/my-skill",
|
|
}}
|
|
|
|
conn.EXPECT().ReadFile(
|
|
gomock.Any(), gomock.Any(), int64(0), gomock.Any(),
|
|
).Return(
|
|
io.NopCloser(strings.NewReader(validSkillMD("my-skill", "test"))),
|
|
"text/markdown",
|
|
nil,
|
|
)
|
|
conn.EXPECT().LS(gomock.Any(), "", gomock.Any()).Return(
|
|
workspacesdk.LSResponse{
|
|
Contents: []workspacesdk.LSFile{
|
|
{Name: "SKILL.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.")
|
|
})
|
|
|
|
t.Run("UnknownSkill", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
tool := chattool.ReadSkill(chattool.ReadSkillOptions{
|
|
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
|
|
t.Fatal("unexpected call to GetWorkspaceConn")
|
|
return nil, xerrors.New("unreachable")
|
|
},
|
|
GetSkills: func() []chattool.SkillMeta { return nil },
|
|
})
|
|
|
|
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
|
ID: "call-1",
|
|
Name: "read_skill",
|
|
Input: `{"name":"nonexistent"}`,
|
|
})
|
|
require.NoError(t, err)
|
|
assert.True(t, resp.IsError)
|
|
assert.Contains(t, resp.Content, "not found")
|
|
})
|
|
|
|
t.Run("EmptyName", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
tool := chattool.ReadSkill(chattool.ReadSkillOptions{
|
|
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
|
|
t.Fatal("unexpected call to GetWorkspaceConn")
|
|
return nil, xerrors.New("unreachable")
|
|
},
|
|
GetSkills: func() []chattool.SkillMeta { return nil },
|
|
})
|
|
|
|
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
|
ID: "call-1",
|
|
Name: "read_skill",
|
|
Input: `{"name":""}`,
|
|
})
|
|
require.NoError(t, err)
|
|
assert.True(t, resp.IsError)
|
|
assert.Contains(t, resp.Content, "required")
|
|
})
|
|
}
|
|
|
|
func TestReadSkillFileTool(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
t.Run("ValidFile", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
ctrl := gomock.NewController(t)
|
|
conn := agentconnmock.NewMockAgentConn(ctrl)
|
|
|
|
skills := []chattool.SkillMeta{{
|
|
Name: "my-skill",
|
|
Dir: "/work/.agents/skills/my-skill",
|
|
}}
|
|
|
|
conn.EXPECT().ReadFile(
|
|
gomock.Any(),
|
|
"/work/.agents/skills/my-skill/roles/reviewer.md",
|
|
int64(0),
|
|
int64(512*1024+1),
|
|
).Return(
|
|
io.NopCloser(strings.NewReader("reviewer guide")),
|
|
"text/markdown",
|
|
nil,
|
|
)
|
|
|
|
tool := chattool.ReadSkillFile(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_file",
|
|
Input: `{"name":"my-skill","path":"roles/reviewer.md"}`,
|
|
})
|
|
require.NoError(t, err)
|
|
assert.False(t, resp.IsError)
|
|
assert.Contains(t, resp.Content, "reviewer guide")
|
|
})
|
|
|
|
t.Run("TraversalRejected", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
skills := []chattool.SkillMeta{{
|
|
Name: "my-skill",
|
|
Dir: "/work/.agents/skills/my-skill",
|
|
}}
|
|
|
|
tool := chattool.ReadSkillFile(chattool.ReadSkillOptions{
|
|
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
|
|
t.Fatal("unexpected call to GetWorkspaceConn")
|
|
return nil, xerrors.New("unreachable")
|
|
},
|
|
GetSkills: func() []chattool.SkillMeta { return skills },
|
|
})
|
|
|
|
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
|
ID: "call-1",
|
|
Name: "read_skill_file",
|
|
Input: `{"name":"my-skill","path":"../../etc/passwd"}`,
|
|
})
|
|
require.NoError(t, err)
|
|
assert.True(t, resp.IsError)
|
|
assert.Contains(t, resp.Content, "traversal")
|
|
})
|
|
}
|