mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat(coderd/x/chatd): add skills discovery and tools for chatd (#23715)
Adds skill discovery and tools to chatd so the agent can discover and load `.agents/skills/` from workspaces, following the same pattern as AGENTS.md instruction loading and MCP tool discovery. ## What changed ### `chattool/skill.go` — discovery, loading, and tools - **DiscoverSkills** — walks `.agents/skills/` via `conn.LS()` + `conn.ReadFile()`, parses SKILL.md frontmatter (name + description), validates kebab-case names match directory names, silently skips broken/missing entries. - **FormatSkillIndex** — renders a compact `<available-skills>` XML block for system prompt injection (~60 tokens for 3 skills). Progressive disclosure: only names + descriptions in context, full body loaded on demand. - **LoadSkillBody** / **LoadSkillFile** — on-demand loading with path traversal protection and size caps (64KB for SKILL.md, 512KB for supporting files). - **read_skill** / **read_skill_file** tools — `fantasy.AgentTool` implementations following the same pattern as ReadFile and WorkspaceMCPTool. Receive pre-discovered `[]SkillMeta` via closure to avoid re-scanning on every call. ### `chatd.go` — integration into runChat - Skills discovered in the `g2` errgroup parallel with instructions and MCP tools. - `skillsCache` (sync.Map) per chat+agent, same invalidation pattern as MCP tools cache. - Skill index injected via `InsertSystem` after workspace instructions. - Re-injected in `ReloadMessages` callback so it survives compaction. - `read_skill` + `read_skill_file` tools registered when skills are present (for both root and subagent chats). - Cache cleaned up in `cleanupStreamIfIdle` alongside MCP tools cache. ## Format compatibility Uses the same `.agents/skills/<name>/SKILL.md` format as [coder/mux](https://github.com/coder/mux) and [openai/codex](https://github.com/openai/codex).
This commit is contained in:
Kyle Carberry
+125
-18
@@ -125,6 +125,10 @@ type Server struct {
|
||||
// keyed by chat ID and invalidated when the agent changes.
|
||||
workspaceMCPToolsCache sync.Map // uuid.UUID -> *cachedWorkspaceMCPTools
|
||||
|
||||
// skillsCache caches discovered skill metadata per chat so
|
||||
// we avoid re-scanning .agents/skills/ on every turn.
|
||||
skillsCache sync.Map // uuid.UUID -> *cachedSkills
|
||||
|
||||
usageTracker *workspacestats.UsageTracker
|
||||
clock quartz.Clock
|
||||
|
||||
@@ -172,6 +176,86 @@ type cachedWorkspaceMCPTools struct {
|
||||
tools []workspacesdk.MCPToolInfo
|
||||
}
|
||||
|
||||
// cachedSkills stores discovered skill metadata from a workspace
|
||||
// agent, keyed by the agent ID that provided them.
|
||||
type cachedSkills struct {
|
||||
agentID uuid.UUID
|
||||
skills []chattool.SkillMeta
|
||||
}
|
||||
|
||||
// discoverWorkspaceSkills returns cached skill metadata for a chat
|
||||
// or discovers them fresh using the provided agent connection. The
|
||||
// result is cached per chat+agent so subsequent turns skip the
|
||||
// filesystem scan.
|
||||
func (p *Server) discoverWorkspaceSkills(
|
||||
ctx context.Context,
|
||||
chatID uuid.UUID,
|
||||
agent database.WorkspaceAgent,
|
||||
conn workspacesdk.AgentConn,
|
||||
logger slog.Logger,
|
||||
) []chattool.SkillMeta {
|
||||
// Check cache first.
|
||||
if cached, ok := p.skillsCache.Load(chatID); ok {
|
||||
if entry, ok2 := cached.(*cachedSkills); ok2 {
|
||||
if entry.agentID == agent.ID {
|
||||
return entry.skills
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
dir := agent.ExpandedDirectory
|
||||
if dir == "" {
|
||||
dir = agent.Directory
|
||||
}
|
||||
discovered, err := chattool.DiscoverSkills(ctx, conn, dir)
|
||||
if err != nil {
|
||||
logger.Warn(ctx, "failed to discover skills",
|
||||
slog.Error(err))
|
||||
return nil
|
||||
}
|
||||
// Cache the result. Unlike MCP tools, an empty skills
|
||||
// list is a valid stable state (the workspace simply has
|
||||
// no skills), so we always cache.
|
||||
p.skillsCache.Store(chatID, &cachedSkills{
|
||||
agentID: agent.ID,
|
||||
skills: discovered,
|
||||
})
|
||||
return discovered
|
||||
}
|
||||
|
||||
// loadCachedWorkspaceContext checks the MCP tools and skills caches
|
||||
// for the given chat and agent. Returns non-nil tools when the MCP
|
||||
// cache hits, which signals the caller to skip the slow discovery
|
||||
// path. Skills may also be populated from the skills cache.
|
||||
func (p *Server) loadCachedWorkspaceContext(
|
||||
chatID uuid.UUID,
|
||||
agent database.WorkspaceAgent,
|
||||
getConn func(context.Context) (workspacesdk.AgentConn, error),
|
||||
) ([]fantasy.AgentTool, []chattool.SkillMeta) {
|
||||
cached, ok := p.workspaceMCPToolsCache.Load(chatID)
|
||||
if !ok {
|
||||
return nil, nil
|
||||
}
|
||||
entry, ok := cached.(*cachedWorkspaceMCPTools)
|
||||
if !ok || entry.agentID != agent.ID {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
var tools []fantasy.AgentTool
|
||||
for _, t := range entry.tools {
|
||||
tools = append(tools, chattool.NewWorkspaceMCPTool(t, getConn))
|
||||
}
|
||||
|
||||
var skills []chattool.SkillMeta
|
||||
if sc, ok := p.skillsCache.Load(chatID); ok {
|
||||
if se, ok := sc.(*cachedSkills); ok && se.agentID == agent.ID {
|
||||
skills = se.skills
|
||||
}
|
||||
}
|
||||
|
||||
return tools, skills
|
||||
}
|
||||
|
||||
type turnWorkspaceContext struct {
|
||||
server *Server
|
||||
chatStateMu *sync.Mutex
|
||||
@@ -2572,6 +2656,7 @@ func (p *Server) cleanupStreamIfIdle(chatID uuid.UUID, state *chatStreamState) {
|
||||
if !state.buffering && len(state.subscribers) == 0 {
|
||||
p.chatStreams.Delete(chatID)
|
||||
p.workspaceMCPToolsCache.Delete(chatID)
|
||||
p.skillsCache.Delete(chatID)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3804,6 +3889,7 @@ func (p *Server) runChat(
|
||||
mcpTools []fantasy.AgentTool
|
||||
mcpCleanup func()
|
||||
workspaceMCPTools []fantasy.AgentTool
|
||||
skills []chattool.SkillMeta
|
||||
)
|
||||
// Check if instruction files need to be (re-)persisted.
|
||||
// This happens when no context-file parts exist yet, or when
|
||||
@@ -3872,21 +3958,14 @@ func (p *Server) runChat(
|
||||
// agent (ensureWorkspaceAgent is free when already
|
||||
// loaded). This avoids a per-turn latest-build DB
|
||||
// query on the common subsequent-turn path.
|
||||
if agent, err := workspaceCtx.getWorkspaceAgent(ctx); err == nil {
|
||||
if cached, ok := p.workspaceMCPToolsCache.Load(chat.ID); ok {
|
||||
entry, ok := cached.(*cachedWorkspaceMCPTools)
|
||||
if ok && entry.agentID == agent.ID {
|
||||
for _, t := range entry.tools {
|
||||
workspaceMCPTools = append(workspaceMCPTools,
|
||||
chattool.NewWorkspaceMCPTool(t, workspaceCtx.getWorkspaceConn),
|
||||
)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
agent, agentErr := workspaceCtx.getWorkspaceAgent(ctx)
|
||||
if agentErr == nil {
|
||||
if workspaceMCPTools, skills = p.loadCachedWorkspaceContext(
|
||||
chat.ID, agent, workspaceCtx.getWorkspaceConn,
|
||||
); workspaceMCPTools != nil {
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
||||
// Cache miss, agent changed, or no cache — validate
|
||||
} // Cache miss, agent changed, or no cache: validate
|
||||
// that the workspace still has a live agent before
|
||||
// attempting a dial.
|
||||
workspaceMCPCtx, cancel := context.WithTimeout(
|
||||
@@ -3895,12 +3974,11 @@ func (p *Server) runChat(
|
||||
)
|
||||
defer cancel()
|
||||
|
||||
_, _, agentErr := workspaceCtx.workspaceAgentIDForConn(
|
||||
workspaceMCPCtx,
|
||||
)
|
||||
_, _, agentErr = workspaceCtx.workspaceAgentIDForConn(workspaceMCPCtx)
|
||||
if agentErr != nil {
|
||||
if xerrors.Is(agentErr, errChatHasNoWorkspaceAgent) {
|
||||
p.workspaceMCPToolsCache.Delete(chat.ID)
|
||||
p.skillsCache.Delete(chat.ID)
|
||||
return nil
|
||||
}
|
||||
logger.Warn(ctx, "failed to resolve workspace agent for MCP tools",
|
||||
@@ -3908,13 +3986,22 @@ func (p *Server) runChat(
|
||||
return nil
|
||||
}
|
||||
|
||||
// Fetch fresh tools from the workspace agent.
|
||||
// Discover skills and MCP tools using the
|
||||
// same conn to avoid a second dial attempt.
|
||||
conn, connErr := workspaceCtx.getWorkspaceConn(workspaceMCPCtx)
|
||||
if connErr != nil {
|
||||
logger.Warn(ctx, "failed to get workspace conn for MCP tools",
|
||||
slog.Error(connErr))
|
||||
return nil
|
||||
}
|
||||
|
||||
agent, agentErr = workspaceCtx.getWorkspaceAgent(workspaceMCPCtx)
|
||||
if agentErr == nil {
|
||||
skills = p.discoverWorkspaceSkills(
|
||||
workspaceMCPCtx, chat.ID, agent, conn, logger,
|
||||
)
|
||||
}
|
||||
|
||||
toolsResp, listErr := conn.ListMCPTools(workspaceMCPCtx)
|
||||
if listErr != nil {
|
||||
logger.Warn(ctx, "failed to list workspace MCP tools",
|
||||
@@ -3963,6 +4050,9 @@ func (p *Server) runChat(
|
||||
if instruction != "" {
|
||||
prompt = chatprompt.InsertSystem(prompt, instruction)
|
||||
}
|
||||
if skillIndex := chattool.FormatSkillIndex(skills); skillIndex != "" {
|
||||
prompt = chatprompt.InsertSystem(prompt, skillIndex)
|
||||
}
|
||||
if resolvedUserPrompt != "" {
|
||||
prompt = chatprompt.InsertSystem(prompt, resolvedUserPrompt)
|
||||
}
|
||||
@@ -4341,6 +4431,20 @@ func (p *Server) runChat(
|
||||
})...)
|
||||
}
|
||||
|
||||
// Append skill tools when the workspace has skills.
|
||||
if len(skills) > 0 {
|
||||
skillOpts := chattool.ReadSkillOptions{
|
||||
GetWorkspaceConn: workspaceCtx.getWorkspaceConn,
|
||||
GetSkills: func() []chattool.SkillMeta {
|
||||
return skills
|
||||
},
|
||||
}
|
||||
tools = append(tools,
|
||||
chattool.ReadSkill(skillOpts),
|
||||
chattool.ReadSkillFile(skillOpts),
|
||||
)
|
||||
}
|
||||
|
||||
// Append tools from external MCP servers. These appear
|
||||
// after the built-in tools so the LLM sees them as
|
||||
// additional capabilities.
|
||||
@@ -4430,6 +4534,9 @@ func (p *Server) runChat(
|
||||
if instruction != "" {
|
||||
reloadedPrompt = chatprompt.InsertSystem(reloadedPrompt, instruction)
|
||||
}
|
||||
if skillIndex := chattool.FormatSkillIndex(skills); skillIndex != "" {
|
||||
reloadedPrompt = chatprompt.InsertSystem(reloadedPrompt, skillIndex)
|
||||
}
|
||||
reloadUserPrompt := p.resolveUserPrompt(reloadCtx, chat.OwnerID)
|
||||
if reloadUserPrompt != "" {
|
||||
reloadedPrompt = chatprompt.InsertSystem(reloadedPrompt, reloadUserPrompt)
|
||||
|
||||
@@ -0,0 +1,511 @@
|
||||
package chattool
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"io"
|
||||
"path"
|
||||
"regexp"
|
||||
"strings"
|
||||
|
||||
"charm.land/fantasy"
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"github.com/coder/coder/v2/codersdk/workspacesdk"
|
||||
)
|
||||
|
||||
const (
|
||||
agentsSkillsDir = ".agents/skills"
|
||||
skillMetaFile = "SKILL.md"
|
||||
maxSkillMetaBytes = 64 * 1024
|
||||
maxSkillFileBytes = 512 * 1024
|
||||
)
|
||||
|
||||
// skillNamePattern validates kebab-case skill names. Each segment
|
||||
// must start with a lowercase letter or digit, and segments are
|
||||
// separated by single hyphens.
|
||||
var skillNamePattern = regexp.MustCompile(
|
||||
`^[a-z0-9]+(-[a-z0-9]+)*$`,
|
||||
)
|
||||
|
||||
// markdownCommentRe strips HTML comments from skill bodies so
|
||||
// they don't leak into the prompt. Matches the same pattern
|
||||
// used by instruction.go in the parent package.
|
||||
var markdownCommentRe = regexp.MustCompile(`<!--[\s\S]*?-->`)
|
||||
|
||||
// SkillMeta is the frontmatter from a SKILL.md 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 {
|
||||
Name string
|
||||
Description string
|
||||
// Dir is the absolute path to the skill directory inside
|
||||
// the workspace filesystem.
|
||||
Dir string
|
||||
}
|
||||
|
||||
// SkillContent is the full body of a skill, loaded on demand
|
||||
// when the model calls read_skill.
|
||||
type SkillContent struct {
|
||||
SkillMeta
|
||||
// Body is the markdown content after the frontmatter
|
||||
// delimiters have been stripped.
|
||||
Body string
|
||||
// Files lists relative paths of supporting files in the
|
||||
// skill directory (everything except SKILL.md itself).
|
||||
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.
|
||||
func DiscoverSkills(
|
||||
ctx context.Context,
|
||||
conn workspacesdk.AgentConn,
|
||||
workingDir 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
|
||||
}
|
||||
|
||||
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,
|
||||
})
|
||||
}
|
||||
|
||||
return skills, nil
|
||||
}
|
||||
|
||||
// parseSkillFrontmatter extracts name, description, and the
|
||||
// markdown body from a SKILL.md file. The frontmatter uses a
|
||||
// simple `key: value` format between `---` delimiters, and no
|
||||
// full YAML parser is needed.
|
||||
func parseSkillFrontmatter(
|
||||
content string,
|
||||
) (name, description, body string, err error) {
|
||||
content = strings.TrimPrefix(content, "\xef\xbb\xbf")
|
||||
lines := strings.Split(content, "\n")
|
||||
if len(lines) == 0 || strings.TrimSpace(lines[0]) != "---" {
|
||||
return "", "", "", xerrors.New(
|
||||
"missing opening frontmatter delimiter",
|
||||
)
|
||||
}
|
||||
|
||||
closingIdx := -1
|
||||
for i := 1; i < len(lines); i++ {
|
||||
if strings.TrimSpace(lines[i]) == "---" {
|
||||
closingIdx = i
|
||||
break
|
||||
}
|
||||
}
|
||||
if closingIdx < 0 {
|
||||
return "", "", "", xerrors.New(
|
||||
"missing closing frontmatter delimiter",
|
||||
)
|
||||
}
|
||||
|
||||
for _, line := range lines[1:closingIdx] {
|
||||
key, value, ok := strings.Cut(line, ":")
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
key = strings.TrimSpace(key)
|
||||
value = strings.TrimSpace(value)
|
||||
// Strip surrounding quotes from YAML string values.
|
||||
if len(value) >= 2 {
|
||||
if (value[0] == '"' && value[len(value)-1] == '"') ||
|
||||
(value[0] == '\'' && value[len(value)-1] == '\'') {
|
||||
value = value[1 : len(value)-1]
|
||||
}
|
||||
}
|
||||
switch strings.ToLower(key) {
|
||||
case "name":
|
||||
name = value
|
||||
case "description":
|
||||
description = value
|
||||
}
|
||||
}
|
||||
|
||||
if name == "" {
|
||||
return "", "", "", xerrors.New(
|
||||
"frontmatter missing required 'name' field",
|
||||
)
|
||||
}
|
||||
|
||||
// Everything after the closing delimiter is the body.
|
||||
body = strings.Join(lines[closingIdx+1:], "\n")
|
||||
body = markdownCommentRe.ReplaceAllString(body, "")
|
||||
body = strings.TrimSpace(body)
|
||||
|
||||
return name, description, body, nil
|
||||
}
|
||||
|
||||
// FormatSkillIndex renders an XML block listing all discovered
|
||||
// skills. This block is injected into the system prompt so the
|
||||
// model knows which skills are available and how to load them.
|
||||
func FormatSkillIndex(skills []SkillMeta) string {
|
||||
if len(skills) == 0 {
|
||||
return ""
|
||||
}
|
||||
|
||||
var b strings.Builder
|
||||
_, _ = b.WriteString("<available-skills>\n")
|
||||
_, _ = b.WriteString(
|
||||
"Use read_skill to load a skill's full instructions " +
|
||||
"before following them.\n" +
|
||||
"Use read_skill_file to read supporting files " +
|
||||
"referenced by a skill.\n\n",
|
||||
)
|
||||
for _, s := range skills {
|
||||
_, _ = b.WriteString("- ")
|
||||
_, _ = b.WriteString(s.Name)
|
||||
if s.Description != "" {
|
||||
_, _ = b.WriteString(": ")
|
||||
_, _ = b.WriteString(s.Description)
|
||||
}
|
||||
_, _ = b.WriteString("\n")
|
||||
}
|
||||
_, _ = b.WriteString("</available-skills>")
|
||||
return b.String()
|
||||
}
|
||||
|
||||
// LoadSkillBody reads the full SKILL.md 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,
|
||||
) (SkillContent, error) {
|
||||
metaPath := path.Join(skill.Dir, skillMetaFile)
|
||||
|
||||
reader, _, err := conn.ReadFile(
|
||||
ctx, metaPath, 0, maxSkillMetaBytes+1,
|
||||
)
|
||||
if err != nil {
|
||||
return SkillContent{}, xerrors.Errorf(
|
||||
"read skill body: %w", err,
|
||||
)
|
||||
}
|
||||
raw, err := io.ReadAll(io.LimitReader(reader, maxSkillMetaBytes+1))
|
||||
reader.Close()
|
||||
if err != nil {
|
||||
return SkillContent{}, xerrors.Errorf(
|
||||
"read skill body bytes: %w", err,
|
||||
)
|
||||
}
|
||||
|
||||
if int64(len(raw)) > maxSkillMetaBytes {
|
||||
raw = raw[:maxSkillMetaBytes]
|
||||
}
|
||||
|
||||
_, _, body, err := parseSkillFrontmatter(string(raw))
|
||||
if err != nil {
|
||||
return SkillContent{}, xerrors.Errorf(
|
||||
"parse skill frontmatter: %w", err,
|
||||
)
|
||||
}
|
||||
|
||||
// List supporting files so the model knows what it can
|
||||
// request via read_skill_file.
|
||||
lsResp, err := conn.LS(ctx, "", workspacesdk.LSRequest{
|
||||
Path: []string{skill.Dir},
|
||||
Relativity: workspacesdk.LSRelativityRoot,
|
||||
})
|
||||
if err != nil {
|
||||
return SkillContent{}, xerrors.Errorf(
|
||||
"list skill directory: %w", err,
|
||||
)
|
||||
}
|
||||
|
||||
var files []string
|
||||
for _, entry := range lsResp.Contents {
|
||||
if entry.Name == skillMetaFile {
|
||||
continue
|
||||
}
|
||||
name := entry.Name
|
||||
if entry.IsDir {
|
||||
name += "/"
|
||||
}
|
||||
files = append(files, name)
|
||||
}
|
||||
|
||||
return SkillContent{
|
||||
SkillMeta: skill,
|
||||
Body: body,
|
||||
Files: files,
|
||||
}, nil
|
||||
}
|
||||
|
||||
// LoadSkillFile reads a supporting file from a skill's directory.
|
||||
// The relativePath is validated to prevent directory traversal and
|
||||
// access to hidden files.
|
||||
func LoadSkillFile(
|
||||
ctx context.Context,
|
||||
conn workspacesdk.AgentConn,
|
||||
skill SkillMeta,
|
||||
relativePath string,
|
||||
) (string, error) {
|
||||
if err := validateSkillFilePath(relativePath); err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
||||
fullPath := path.Join(skill.Dir, relativePath)
|
||||
|
||||
reader, _, err := conn.ReadFile(
|
||||
ctx, fullPath, 0, maxSkillFileBytes+1,
|
||||
)
|
||||
if err != nil {
|
||||
return "", xerrors.Errorf(
|
||||
"read skill file: %w", err,
|
||||
)
|
||||
}
|
||||
raw, err := io.ReadAll(io.LimitReader(reader, maxSkillFileBytes+1))
|
||||
reader.Close()
|
||||
if err != nil {
|
||||
return "", xerrors.Errorf(
|
||||
"read skill file bytes: %w", err,
|
||||
)
|
||||
}
|
||||
|
||||
if int64(len(raw)) > maxSkillFileBytes {
|
||||
raw = raw[:maxSkillFileBytes]
|
||||
}
|
||||
|
||||
return string(raw), nil
|
||||
}
|
||||
|
||||
// validateSkillFilePath rejects paths that could escape the skill
|
||||
// directory or access hidden files. Only forward-relative,
|
||||
// non-hidden paths are allowed.
|
||||
func validateSkillFilePath(p string) error {
|
||||
if p == "" {
|
||||
return xerrors.New("path is required")
|
||||
}
|
||||
if strings.HasPrefix(p, "/") {
|
||||
return xerrors.New(
|
||||
"absolute paths are not allowed",
|
||||
)
|
||||
}
|
||||
for _, component := range strings.Split(p, "/") {
|
||||
if component == ".." {
|
||||
return xerrors.New(
|
||||
"path traversal is not allowed",
|
||||
)
|
||||
}
|
||||
if strings.HasPrefix(component, ".") {
|
||||
return xerrors.New(
|
||||
"hidden file components are not allowed",
|
||||
)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// ReadSkillOptions configures the read_skill and read_skill_file
|
||||
// tools.
|
||||
type ReadSkillOptions struct {
|
||||
GetWorkspaceConn func(context.Context) (workspacesdk.AgentConn, error)
|
||||
GetSkills func() []SkillMeta
|
||||
}
|
||||
|
||||
// ReadSkillArgs are the parameters accepted by read_skill.
|
||||
type ReadSkillArgs struct {
|
||||
Name string `json:"name" description:"The kebab-case name of the skill to read."`
|
||||
}
|
||||
|
||||
// ReadSkill returns an AgentTool that reads the full instructions
|
||||
// for a skill by name. The model should call this before
|
||||
// following any skill's instructions.
|
||||
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 "+
|
||||
"supporting files. Use read_skill before "+
|
||||
"following a skill's instructions.",
|
||||
func(ctx context.Context, args ReadSkillArgs, _ fantasy.ToolCall) (fantasy.ToolResponse, error) {
|
||||
if options.GetWorkspaceConn == nil {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
"workspace connection resolver is not configured",
|
||||
), nil
|
||||
}
|
||||
if args.Name == "" {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
"name is required",
|
||||
), nil
|
||||
}
|
||||
|
||||
skill, ok := findSkill(options.GetSkills, args.Name)
|
||||
if !ok {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
fmt.Sprintf("skill %q not found", args.Name),
|
||||
), nil
|
||||
}
|
||||
|
||||
conn, err := options.GetWorkspaceConn(ctx)
|
||||
if err != nil {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
err.Error(),
|
||||
), nil
|
||||
}
|
||||
|
||||
content, err := LoadSkillBody(ctx, conn, skill)
|
||||
if err != nil {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
err.Error(),
|
||||
), nil
|
||||
}
|
||||
|
||||
return toolResponse(map[string]any{
|
||||
"name": content.Name,
|
||||
"body": content.Body,
|
||||
"files": content.Files,
|
||||
}), nil
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
// ReadSkillFileArgs are the parameters accepted by
|
||||
// read_skill_file.
|
||||
type ReadSkillFileArgs struct {
|
||||
Name string `json:"name" description:"The kebab-case name of the skill."`
|
||||
Path string `json:"path" description:"Relative path to a file in the skill directory (e.g. roles/security-reviewer.md)."`
|
||||
}
|
||||
|
||||
// ReadSkillFile returns an AgentTool that reads a supporting file
|
||||
// from a skill's directory.
|
||||
func ReadSkillFile(options ReadSkillOptions) fantasy.AgentTool {
|
||||
return fantasy.NewAgentTool(
|
||||
"read_skill_file",
|
||||
"Read a supporting file from a skill's directory "+
|
||||
"(e.g. roles/security-reviewer.md).",
|
||||
func(ctx context.Context, args ReadSkillFileArgs, _ fantasy.ToolCall) (fantasy.ToolResponse, error) {
|
||||
if options.GetWorkspaceConn == nil {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
"workspace connection resolver is not configured",
|
||||
), nil
|
||||
}
|
||||
if args.Name == "" {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
"name is required",
|
||||
), nil
|
||||
}
|
||||
if args.Path == "" {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
"path is required",
|
||||
), nil
|
||||
}
|
||||
|
||||
skill, ok := findSkill(options.GetSkills, args.Name)
|
||||
if !ok {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
fmt.Sprintf("skill %q not found", args.Name),
|
||||
), nil
|
||||
}
|
||||
|
||||
// Validate the path early so we reject bad
|
||||
// inputs before dialing the workspace agent.
|
||||
if err := validateSkillFilePath(args.Path); err != nil {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
err.Error(),
|
||||
), nil
|
||||
}
|
||||
|
||||
conn, err := options.GetWorkspaceConn(ctx)
|
||||
if err != nil {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
err.Error(),
|
||||
), nil
|
||||
}
|
||||
|
||||
content, err := LoadSkillFile(
|
||||
ctx, conn, skill, args.Path,
|
||||
)
|
||||
if err != nil {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
err.Error(),
|
||||
), nil
|
||||
}
|
||||
|
||||
return toolResponse(map[string]any{
|
||||
"content": content,
|
||||
}), nil
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
// findSkill looks up a skill by name in the current skill list.
|
||||
func findSkill(
|
||||
getSkills func() []SkillMeta,
|
||||
name string,
|
||||
) (SkillMeta, bool) {
|
||||
if getSkills == nil {
|
||||
return SkillMeta{}, false
|
||||
}
|
||||
for _, s := range getSkills() {
|
||||
if s.Name == name {
|
||||
return s, true
|
||||
}
|
||||
}
|
||||
return SkillMeta{}, false
|
||||
}
|
||||
@@ -0,0 +1,688 @@
|
||||
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"
|
||||
"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 TestDiscoverSkills(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
t.Run("FindsSkillsInWorkspace", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
conn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
|
||||
// List the skills directory: returns two skill dirs.
|
||||
conn.EXPECT().LS(gomock.Any(), "", gomock.Any()).DoAndReturn(
|
||||
func(_ context.Context, _ string, req workspacesdk.LSRequest) (workspacesdk.LSResponse, error) {
|
||||
require.Equal(t, []string{"/work/.agents/skills"}, req.Path)
|
||||
return workspacesdk.LSResponse{
|
||||
Contents: []workspacesdk.LSFile{
|
||||
{Name: "my-skill", IsDir: true, AbsolutePathString: "/work/.agents/skills/my-skill"},
|
||||
{Name: "other-skill", IsDir: true, AbsolutePathString: "/work/.agents/skills/other-skill"},
|
||||
},
|
||||
}, nil
|
||||
},
|
||||
)
|
||||
|
||||
// Read SKILL.md for my-skill.
|
||||
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", "first skill"))),
|
||||
"text/markdown",
|
||||
nil,
|
||||
)
|
||||
|
||||
// Read SKILL.md for other-skill.
|
||||
conn.EXPECT().ReadFile(
|
||||
gomock.Any(),
|
||||
"/work/.agents/skills/other-skill/SKILL.md",
|
||||
int64(0),
|
||||
int64(64*1024+1),
|
||||
).Return(
|
||||
io.NopCloser(strings.NewReader(validSkillMD("other-skill", "second skill"))),
|
||||
"text/markdown",
|
||||
nil,
|
||||
)
|
||||
|
||||
skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work")
|
||||
require.NoError(t, err)
|
||||
require.Len(t, skills, 2)
|
||||
assert.Equal(t, "my-skill", skills[0].Name)
|
||||
assert.Equal(t, "first skill", skills[0].Description)
|
||||
assert.Equal(t, "other-skill", skills[1].Name)
|
||||
})
|
||||
|
||||
t.Run("SkillsDirMissing", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
conn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
|
||||
conn.EXPECT().LS(gomock.Any(), "", gomock.Any()).Return(
|
||||
workspacesdk.LSResponse{},
|
||||
codersdk.NewTestError(404, "POST", "/api/v0/list-directory"),
|
||||
)
|
||||
|
||||
skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work")
|
||||
require.NoError(t, err)
|
||||
require.Empty(t, skills)
|
||||
})
|
||||
|
||||
t.Run("SkipsMissingSKILLmd", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
conn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
|
||||
conn.EXPECT().LS(gomock.Any(), "", gomock.Any()).Return(
|
||||
workspacesdk.LSResponse{
|
||||
Contents: []workspacesdk.LSFile{
|
||||
{Name: "broken", IsDir: true, AbsolutePathString: "/work/.agents/skills/broken"},
|
||||
},
|
||||
}, nil,
|
||||
)
|
||||
|
||||
// SKILL.md doesn't exist.
|
||||
conn.EXPECT().ReadFile(
|
||||
gomock.Any(),
|
||||
"/work/.agents/skills/broken/SKILL.md",
|
||||
int64(0),
|
||||
int64(64*1024+1),
|
||||
).Return(
|
||||
nil, "",
|
||||
codersdk.NewTestError(404, "GET", "/api/v0/read-file"),
|
||||
)
|
||||
|
||||
skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work")
|
||||
require.NoError(t, err)
|
||||
require.Empty(t, skills)
|
||||
})
|
||||
|
||||
t.Run("SkipsInvalidFrontmatter", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
conn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
|
||||
conn.EXPECT().LS(gomock.Any(), "", gomock.Any()).Return(
|
||||
workspacesdk.LSResponse{
|
||||
Contents: []workspacesdk.LSFile{
|
||||
{Name: "bad", IsDir: true, AbsolutePathString: "/work/.agents/skills/bad"},
|
||||
},
|
||||
}, nil,
|
||||
)
|
||||
|
||||
// No frontmatter delimiters.
|
||||
conn.EXPECT().ReadFile(
|
||||
gomock.Any(), gomock.Any(), int64(0), gomock.Any(),
|
||||
).Return(
|
||||
io.NopCloser(strings.NewReader("just some markdown")),
|
||||
"text/markdown",
|
||||
nil,
|
||||
)
|
||||
|
||||
skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work")
|
||||
require.NoError(t, err)
|
||||
require.Empty(t, skills)
|
||||
})
|
||||
|
||||
t.Run("SkipsMismatchedDirName", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
conn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
|
||||
conn.EXPECT().LS(gomock.Any(), "", gomock.Any()).Return(
|
||||
workspacesdk.LSResponse{
|
||||
Contents: []workspacesdk.LSFile{
|
||||
{Name: "dir-name", IsDir: true, AbsolutePathString: "/work/.agents/skills/dir-name"},
|
||||
},
|
||||
}, nil,
|
||||
)
|
||||
|
||||
// name in frontmatter doesn't match dir name.
|
||||
conn.EXPECT().ReadFile(
|
||||
gomock.Any(), gomock.Any(), int64(0), gomock.Any(),
|
||||
).Return(
|
||||
io.NopCloser(strings.NewReader(validSkillMD("different-name", "desc"))),
|
||||
"text/markdown",
|
||||
nil,
|
||||
)
|
||||
|
||||
skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work")
|
||||
require.NoError(t, err)
|
||||
require.Empty(t, skills)
|
||||
})
|
||||
|
||||
t.Run("SkipsNonKebabCase", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
conn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
|
||||
conn.EXPECT().LS(gomock.Any(), "", gomock.Any()).Return(
|
||||
workspacesdk.LSResponse{
|
||||
Contents: []workspacesdk.LSFile{
|
||||
{Name: "UPPER", IsDir: true, AbsolutePathString: "/work/.agents/skills/UPPER"},
|
||||
},
|
||||
}, nil,
|
||||
)
|
||||
|
||||
conn.EXPECT().ReadFile(
|
||||
gomock.Any(), gomock.Any(), int64(0), gomock.Any(),
|
||||
).Return(
|
||||
io.NopCloser(strings.NewReader(validSkillMD("UPPER", "bad name"))),
|
||||
"text/markdown",
|
||||
nil,
|
||||
)
|
||||
|
||||
skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work")
|
||||
require.NoError(t, err)
|
||||
require.Empty(t, skills)
|
||||
})
|
||||
|
||||
t.Run("SkipsNonDirectories", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
conn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
|
||||
conn.EXPECT().LS(gomock.Any(), "", gomock.Any()).Return(
|
||||
workspacesdk.LSResponse{
|
||||
Contents: []workspacesdk.LSFile{
|
||||
{Name: "README.md", IsDir: false, AbsolutePathString: "/work/.agents/skills/README.md"},
|
||||
},
|
||||
}, nil,
|
||||
)
|
||||
|
||||
skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work")
|
||||
require.NoError(t, err)
|
||||
require.Empty(t, skills)
|
||||
})
|
||||
|
||||
t.Run("QuotedDescription", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
conn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
|
||||
conn.EXPECT().LS(gomock.Any(), "", gomock.Any()).Return(
|
||||
workspacesdk.LSResponse{
|
||||
Contents: []workspacesdk.LSFile{
|
||||
{Name: "my-skill", IsDir: true, AbsolutePathString: "/work/.agents/skills/my-skill"},
|
||||
},
|
||||
}, nil,
|
||||
)
|
||||
|
||||
// Description uses YAML-style quotes.
|
||||
md := "---\nname: my-skill\ndescription: \"A quoted description\"\n---\n\nBody.\n"
|
||||
conn.EXPECT().ReadFile(
|
||||
gomock.Any(), gomock.Any(), int64(0), gomock.Any(),
|
||||
).Return(
|
||||
io.NopCloser(strings.NewReader(md)),
|
||||
"text/markdown",
|
||||
nil,
|
||||
)
|
||||
|
||||
skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work")
|
||||
require.NoError(t, err)
|
||||
require.Len(t, skills, 1)
|
||||
assert.Equal(t, "A quoted description", skills[0].Description)
|
||||
})
|
||||
|
||||
t.Run("OversizedSKILLmdTruncated", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
conn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
|
||||
conn.EXPECT().LS(gomock.Any(), "", gomock.Any()).Return(
|
||||
workspacesdk.LSResponse{
|
||||
Contents: []workspacesdk.LSFile{
|
||||
{Name: "big-skill", IsDir: true, AbsolutePathString: "/work/.agents/skills/big-skill"},
|
||||
},
|
||||
}, nil,
|
||||
)
|
||||
|
||||
// Build a SKILL.md larger than 64KB. The frontmatter is
|
||||
// at the start so it survives truncation.
|
||||
bigBody := strings.Repeat("x", 70*1024)
|
||||
md := "---\nname: big-skill\ndescription: large\n---\n" + bigBody
|
||||
conn.EXPECT().ReadFile(
|
||||
gomock.Any(), gomock.Any(), int64(0), gomock.Any(),
|
||||
).Return(
|
||||
io.NopCloser(strings.NewReader(md)),
|
||||
"text/markdown",
|
||||
nil,
|
||||
)
|
||||
|
||||
skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work")
|
||||
require.NoError(t, err)
|
||||
// The skill should still be discovered since the
|
||||
// frontmatter fits within the truncation limit.
|
||||
require.Len(t, skills, 1)
|
||||
assert.Equal(t, "big-skill", skills[0].Name)
|
||||
})
|
||||
|
||||
t.Run("BOMHandled", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
conn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
|
||||
conn.EXPECT().LS(gomock.Any(), "", gomock.Any()).Return(
|
||||
workspacesdk.LSResponse{
|
||||
Contents: []workspacesdk.LSFile{
|
||||
{Name: "bom-skill", IsDir: true, AbsolutePathString: "/work/.agents/skills/bom-skill"},
|
||||
},
|
||||
}, nil,
|
||||
)
|
||||
|
||||
// UTF-8 BOM prefix before the frontmatter.
|
||||
md := "\xef\xbb\xbf---\nname: bom-skill\ndescription: has BOM\n---\n\nBody.\n"
|
||||
conn.EXPECT().ReadFile(
|
||||
gomock.Any(), gomock.Any(), int64(0), gomock.Any(),
|
||||
).Return(
|
||||
io.NopCloser(strings.NewReader(md)),
|
||||
"text/markdown",
|
||||
nil,
|
||||
)
|
||||
|
||||
skills, err := chattool.DiscoverSkills(context.Background(), conn, "/work")
|
||||
require.NoError(t, err)
|
||||
require.Len(t, skills, 1)
|
||||
assert.Equal(t, "bom-skill", skills[0].Name)
|
||||
})
|
||||
}
|
||||
|
||||
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)
|
||||
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")
|
||||
})
|
||||
}
|
||||
Reference in New Issue
Block a user