feat: support personal skills in chats (#25366)

> Mux updated this PR on behalf of Mike.

## Stack Context

This PR builds on #25365 in the experimental personal skills stack and
completes the chat integration.

Stack order:
1. #25362 personal skill resolver
2. #25363 storage, permissions, API, and SDK
3. #25365 API test coverage
4. #25366 chattool and chatd integration
5. #25066 settings UI and docs
6. #25386 personal skills slash menu

## What?

Updates chattool skill formatting and `read_skill` resolution so tools
can read personal skills from the database, then injects personal skill
metadata into chatd prompts and registers the skill-reading tools when
skills are available.

This branch has also been merged with current `origin/main` to resolve
merge conflicts.

## Why?

The chattool and chatd changes need to land together so the intermediate
stack state stays buildable. This completes personal skill availability
in chats without syncing personal skills into workspace filesystems.

## Validation

- `go test -count=1 ./coderd/x/chatd/chattool -run
'TestFormatResolvedSkillIndex|TestReadSkillTool|TestReadSkillFileTool'`
- `go test -count=1 ./coderd/x/chatd -run
'TestPersonalSkillsInSystemPrompt|TestPersonalAndWorkspaceSkillCollisionInSystemPrompt|TestSkillIndexRefreshReplacesStaleAliases|TestFetchPersonalSkillMetadata|TestLoadPersonalSkillBody'`
- `go test -count=1 ./coderd -run 'Test.*UserSkill'`
- `git diff --cached --check`
- `make lint`
- pre-commit hook
This commit is contained in:
Michael Suchacz
2026-05-20 19:50:50 +02:00
committed by GitHub
parent e27d917135
commit 63900d212d
5 changed files with 1217 additions and 93 deletions
+1 -2
View File
@@ -667,8 +667,7 @@ func userSkillMarkdown(name string, description string, body string) string {
func requireSDKErrorStatus(t *testing.T, err error, status int, msgAndArgs ...any) *codersdk.Error {
t.Helper()
require.Error(t, err, msgAndArgs...)
var sdkErr *codersdk.Error
require.ErrorAs(t, err, &sdkErr, msgAndArgs...)
sdkErr := coderdtest.SDKError(t, err)
require.Equal(t, status, sdkErr.StatusCode(), msgAndArgs...)
return sdkErr
}
+222 -27
View File
@@ -34,6 +34,7 @@ import (
"github.com/coder/coder/v2/coderd/database/dbauthz"
"github.com/coder/coder/v2/coderd/database/pubsub"
coderdpubsub "github.com/coder/coder/v2/coderd/pubsub"
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/coderd/util/ptr"
"github.com/coder/coder/v2/coderd/util/xjson"
"github.com/coder/coder/v2/coderd/webpush"
@@ -51,6 +52,7 @@ import (
"github.com/coder/coder/v2/coderd/x/chatd/chattool"
"github.com/coder/coder/v2/coderd/x/chatd/internal/agentselect"
"github.com/coder/coder/v2/coderd/x/chatd/mcpclient"
skillspkg "github.com/coder/coder/v2/coderd/x/skills"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/codersdk/workspacesdk"
"github.com/coder/quartz"
@@ -6476,6 +6478,31 @@ type systemPromptBehaviorContext struct {
isRootChat bool
}
func workspaceSkillsForResolution(workspaceSkills []chattool.SkillMeta) []skillspkg.Skill {
if len(workspaceSkills) == 0 {
return nil
}
resolved := make([]skillspkg.Skill, 0, len(workspaceSkills))
for _, skill := range workspaceSkills {
resolved = append(resolved, skillspkg.Skill{
Name: skill.Name,
Description: skill.Description,
Source: skillspkg.SourceWorkspace,
})
}
return resolved
}
func mergeTurnSkills(
personalSkills []skillspkg.Skill,
workspaceSkills []chattool.SkillMeta,
) []skillspkg.ResolvedSkill {
return skillspkg.MergeSkills(
personalSkills,
workspaceSkillsForResolution(workspaceSkills),
)
}
// buildSystemPrompt applies system-level prompt injections in the
// canonical order. It is used by both the initial prompt assembly
// and the ReloadMessages callback to keep them in sync.
@@ -6483,7 +6510,7 @@ func buildSystemPrompt(
prompt []fantasy.Message,
subagentInstruction string,
instruction string,
skills []chattool.SkillMeta,
resolvedSkills []skillspkg.ResolvedSkill,
userPrompt string,
behaviorContext systemPromptBehaviorContext,
) []fantasy.Message {
@@ -6493,7 +6520,7 @@ func buildSystemPrompt(
if instruction != "" {
prompt = chatprompt.InsertSystem(prompt, instruction)
}
if skillIndex := chattool.FormatSkillIndex(skills); skillIndex != "" {
if skillIndex := chattool.FormatResolvedSkillIndex(resolvedSkills); skillIndex != "" {
prompt = chatprompt.InsertSystem(prompt, skillIndex)
}
if userPrompt != "" {
@@ -6517,6 +6544,34 @@ func buildSystemPrompt(
return prompt
}
func removeSkillIndexMessages(prompt []fantasy.Message) []fantasy.Message {
out := make([]fantasy.Message, 0, len(prompt))
removed := false
for _, message := range prompt {
if isSkillIndexMessage(message) {
removed = true
continue
}
out = append(out, message)
}
if !removed {
return prompt
}
return out
}
func isSkillIndexMessage(message fantasy.Message) bool {
if message.Role != fantasy.MessageRoleSystem || len(message.Content) != 1 {
return false
}
textPart, ok := fantasy.AsMessagePart[fantasy.TextPart](message.Content[0])
if !ok {
return false
}
text := strings.TrimSpace(textPart.Text)
return strings.HasPrefix(text, chattool.AvailableSkillsOpenTag+"\n") && strings.HasSuffix(text, chattool.AvailableSkillsCloseTag)
}
type rootChatToolsOptions struct {
chat database.Chat
modelConfigID uuid.UUID
@@ -6559,6 +6614,88 @@ func (p *Server) loadPlanModeInstructions(
return fetched
}
func userSkillContext(ctx context.Context, userID uuid.UUID) context.Context {
actor := rbac.Subject{
Type: rbac.SubjectTypeUser,
ID: userID.String(),
Roles: rbac.RoleIdentifiers{rbac.RoleMember()},
Scope: rbac.ScopeAll,
}.WithCachedASTValue()
// Chat turns run asynchronously after admission, so the original request
// actor may no longer be available when a worker loads personal skills.
// We synthesize the chat owner as a member instead of reusing that actor.
// Hardcoding RoleMember is safe because dbauthz enforces
// ResourceUserSkill.WithOwner(userID), so this actor cannot read any other
// user's skills regardless of role. Org scoping is not needed because
// personal skills are user-scoped, not org-scoped.
//nolint:gocritic // The synthetic actor is intentional for the reasons above.
return dbauthz.As(ctx, actor)
}
func (p *Server) fetchPersonalSkillMetadata(
ctx context.Context,
userID uuid.UUID,
logger slog.Logger,
) []skillspkg.Skill {
rows, err := p.db.ListUserSkillMetadataByUserID(userSkillContext(ctx, userID), userID)
// See package coderd/x/skills (doc.go) for why metadata fetch failures
// intentionally degrade to an empty personal-skill list instead of
// failing the chat turn.
if err != nil {
logger.Warn(ctx, "failed to load personal skill metadata",
slog.F("owner_id", userID),
slog.Error(err),
)
return nil
}
personalSkills := make([]skillspkg.Skill, 0, len(rows))
for _, row := range rows {
personalSkills = append(personalSkills, skillspkg.Skill{
Name: row.Name,
Description: row.Description,
Source: skillspkg.SourcePersonal,
})
}
return personalSkills
}
func (p *Server) loadPersonalSkillBody(
ctx context.Context,
userID uuid.UUID,
name string,
) (skillspkg.ParsedSkill, error) {
row, err := p.db.GetUserSkillByUserIDAndName(
userSkillContext(ctx, userID),
database.GetUserSkillByUserIDAndNameParams{
UserID: userID,
Name: name,
},
)
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
return skillspkg.ParsedSkill{}, skillspkg.ErrSkillNotFound
}
p.logger.Error(ctx, "load personal skill body failed",
slog.F("user_id", userID),
slog.F("name", name),
slog.Error(err),
)
return skillspkg.ParsedSkill{}, xerrors.Errorf("load personal skill body: %w", err)
}
parsed, err := skillspkg.ParsePersonalSkillMarkdown([]byte(row.Content))
if err != nil {
p.logger.Error(ctx, "parse personal skill body failed",
slog.F("user_id", userID),
slog.F("name", name),
slog.Error(err),
)
return skillspkg.ParsedSkill{}, xerrors.Errorf("parse personal skill body: %w", err)
}
return parsed, nil
}
func (p *Server) appendRootChatTools(
ctx context.Context,
tools []fantasy.AgentTool,
@@ -7035,7 +7172,8 @@ func (p *Server) runChat(
mcpTools []fantasy.AgentTool
mcpCleanup func()
workspaceMCPTools []fantasy.AgentTool
skills []chattool.SkillMeta
workspaceSkills []chattool.SkillMeta
personalSkills []skillspkg.Skill
)
// Check if instruction files need to be (re-)persisted.
// This happens when no context-file parts exist yet, or when
@@ -7092,7 +7230,7 @@ func (p *Server) runChat(
return workspaceCtx.getWorkspaceConn(instructionCtx)
},
)
skills = selectSkillMetasForInstructionRefresh(
workspaceSkills = selectSkillMetasForInstructionRefresh(
persistedSkills,
discoveredSkills,
uuid.NullUUID{UUID: currentWorkspaceAgentID, Valid: hasCurrentWorkspaceAgent},
@@ -7112,8 +7250,12 @@ func (p *Server) runChat(
// re-injected via InsertSystem after compaction drops
// those messages. No workspace dial needed.
instruction = instructionFromContextFiles(messages)
skills = persistedSkills
workspaceSkills = persistedSkills
}
g2.Go(func() error {
personalSkills = p.fetchPersonalSkillMetadata(ctx, chat.OwnerID, logger)
return nil
})
g2.Go(func() error {
resolvedUserPrompt = p.resolveUserPrompt(ctx, chat.OwnerID)
return nil
@@ -7153,11 +7295,19 @@ func (p *Server) runChat(
if !isRootChat {
subagentInstruction = defaultSubagentInstruction
}
resolvedSkillsFor := func(workspaceSkills []chattool.SkillMeta) []skillspkg.ResolvedSkill {
return mergeTurnSkills(personalSkills, workspaceSkills)
}
resolveSkillAlias := func(alias string) (skillspkg.ResolvedSkill, error) {
return skillspkg.Lookup(resolvedSkillsFor(workspaceSkills), alias)
}
initialResolvedSkills := resolvedSkillsFor(workspaceSkills)
injectedSkillIndex := chattool.FormatResolvedSkillIndex(initialResolvedSkills)
prompt = buildSystemPrompt(
prompt,
subagentInstruction,
instruction,
skills,
initialResolvedSkills,
resolvedUserPrompt,
systemPromptBehaviorContext{
planMode: currentPlanMode,
@@ -7559,7 +7709,7 @@ func (p *Server) runChat(
workspaceCtx: &workspaceCtx,
workspaceMu: &workspaceMu,
instruction: &instruction,
skills: &skills,
skills: &workspaceSkills,
resolvePlanPath: resolvePlanPathForTools,
storeFile: storeChatAttachment,
isPlanModeTurn: isPlanModeTurn,
@@ -7567,19 +7717,43 @@ 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),
)
skillOpts := chattool.ReadSkillOptions{
GetWorkspaceConn: workspaceCtx.getWorkspaceConn,
GetSkills: func() []chattool.SkillMeta {
return workspaceSkills
},
ResolveAlias: resolveSkillAlias,
LoadPersonalSkillBody: func(ctx context.Context, name string) (skillspkg.ParsedSkill, error) {
return p.loadPersonalSkillBody(ctx, chat.OwnerID, name)
},
}
appendCurrentSkillTools := func(current []fantasy.AgentTool) ([]fantasy.AgentTool, bool) {
if len(personalSkills) == 0 && len(workspaceSkills) == 0 {
return current, false
}
updated := current
changed := false
appendTool := func(tool fantasy.AgentTool) {
name := tool.Info().Name
if slices.ContainsFunc(current, func(existing fantasy.AgentTool) bool {
return existing.Info().Name == name
}) {
return
}
if !changed {
updated = slices.Clone(current)
changed = true
}
updated = append(updated, tool)
}
appendTool(chattool.ReadSkill(skillOpts))
if len(workspaceSkills) > 0 {
appendTool(chattool.ReadSkillFile(skillOpts))
}
return updated, changed
}
tools, _ = appendCurrentSkillTools(tools)
if advisorRuntime != nil {
tools = append(tools, chatadvisor.Tool(chatadvisor.ToolOptions{
Runtime: advisorRuntime,
@@ -7853,14 +8027,16 @@ func (p *Server) runChat(
}
reloadedSkills := skillsFromParts(reloadedMsgs)
if len(reloadedSkills) == 0 {
reloadedSkills = skills
reloadedSkills = workspaceSkills
}
reloadedResolvedSkills := resolvedSkillsFor(reloadedSkills)
injectedSkillIndex = chattool.FormatResolvedSkillIndex(reloadedResolvedSkills)
reloadUserPrompt := p.resolveUserPrompt(reloadCtx, chat.OwnerID)
reloadedPrompt = buildSystemPrompt(
reloadedPrompt,
subagentInstruction,
reloadedInstruction,
reloadedSkills,
reloadedResolvedSkills,
reloadUserPrompt,
systemPromptBehaviorContext{
planMode: currentPlanMode,
@@ -7894,6 +8070,8 @@ func (p *Server) runChat(
chainModeActive = false
},
PrepareTools: func(currentTools []fantasy.AgentTool) []fantasy.AgentTool {
updatedTools, toolsChanged := appendCurrentSkillTools(currentTools)
// Mid-turn workspace MCP discovery for chats that bind a
// workspace via create_workspace or start_workspace after the
// turn has already started. The top-of-turn discovery path is
@@ -7907,10 +8085,16 @@ func (p *Server) runChat(
// always a cache hit. The primer's bounded wait means the
// dial fallback here only runs when priming itself failed.
if workspaceMCPDiscovered || isExploreSubagent {
if toolsChanged {
return updatedTools
}
return nil
}
snapshot := workspaceCtx.currentChatSnapshot()
if !snapshot.WorkspaceID.Valid {
if toolsChanged {
return updatedTools
}
return nil
}
discovered := p.discoverWorkspaceMCPTools(
@@ -7925,10 +8109,13 @@ func (p *Server) runChat(
// plus one ListMCPTools RPC, both fast against a live
// conn. The primer's 30s budget applies to its own
// loop only.
if toolsChanged {
return updatedTools
}
return nil
}
workspaceMCPDiscovered = true
return append(slices.Clone(currentTools), discovered...)
return append(slices.Clone(updatedTools), discovered...)
},
PrepareMessages: func(msgs []fantasy.Message) []fantasy.Message {
// Skip the snapshot update when chain mode is active;
@@ -7939,13 +8126,21 @@ func (p *Server) runChat(
if !chainModeActive {
setAdvisorPromptSnapshot(msgs)
}
if instructionInjected || instruction == "" {
return nil
result := msgs
changed := false
if !instructionInjected && instruction != "" {
instructionInjected = true
result = chatprompt.InsertSystem(result, instruction)
changed = true
}
instructionInjected = true
result := chatprompt.InsertSystem(msgs, instruction)
if skillIndex := chattool.FormatSkillIndex(skills); skillIndex != "" {
if skillIndex := chattool.FormatResolvedSkillIndex(resolvedSkillsFor(workspaceSkills)); skillIndex != "" && skillIndex != injectedSkillIndex {
result = removeSkillIndexMessages(result)
result = chatprompt.InsertSystem(result, skillIndex)
injectedSkillIndex = skillIndex
changed = true
}
if !changed {
return nil
}
if !chainModeActive {
setAdvisorPromptSnapshot(result)
+334
View File
@@ -4,6 +4,7 @@ import (
"context"
"database/sql"
"encoding/json"
"strings"
"sync"
"sync/atomic"
"testing"
@@ -23,12 +24,15 @@ import (
"github.com/coder/coder/v2/coderd/database/dbmock"
dbpubsub "github.com/coder/coder/v2/coderd/database/pubsub"
coderdpubsub "github.com/coder/coder/v2/coderd/pubsub"
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/coderd/x/chatd/chaterror"
"github.com/coder/coder/v2/coderd/x/chatd/chatloop"
openaicomputeruse "github.com/coder/coder/v2/coderd/x/chatd/chatopenai/computeruse"
"github.com/coder/coder/v2/coderd/x/chatd/chatprompt"
"github.com/coder/coder/v2/coderd/x/chatd/chatprovider"
"github.com/coder/coder/v2/coderd/x/chatd/chattest"
"github.com/coder/coder/v2/coderd/x/chatd/chattool"
skillspkg "github.com/coder/coder/v2/coderd/x/skills"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/codersdk/workspacesdk"
"github.com/coder/coder/v2/codersdk/workspacesdk/agentconnmock"
@@ -3092,6 +3096,336 @@ func TestSkillsFromParts(t *testing.T) {
})
}
func TestPersonalSkillsInSystemPrompt(t *testing.T) {
t.Parallel()
prompt := buildSystemPrompt(
nil,
"",
"",
mergeTurnSkills(
[]skillspkg.Skill{{
Name: "personal-review",
Description: "Personal review process",
Source: skillspkg.SourcePersonal,
}},
nil,
),
"",
systemPromptBehaviorContext{},
)
text := systemPromptText(t, prompt)
require.Contains(t, text, "<available-skills>")
require.Contains(t, text, "- personal-review: Personal review process")
require.NotContains(t, text, `"skill"`)
}
func TestPersonalAndWorkspaceSkillCollisionInSystemPrompt(t *testing.T) {
t.Parallel()
resolved := mergeTurnSkills(
[]skillspkg.Skill{{
Name: "deploy",
Description: "Personal deployment process",
Source: skillspkg.SourcePersonal,
}},
[]chattool.SkillMeta{{
Name: "deploy",
Description: "Workspace deployment process",
Dir: "/skills/deploy",
}},
)
prompt := buildSystemPrompt(
nil,
"",
"",
resolved,
"",
systemPromptBehaviorContext{},
)
text := systemPromptText(t, prompt)
require.Contains(t, text, "<available-skills>")
require.Contains(t, text, "- personal/deploy: Personal deployment process")
require.Contains(t, text, "- workspace/deploy: Workspace deployment process")
require.NotContains(t, text, "\n- deploy: ")
require.NotContains(t, text, "\n- deploy\n")
personal, err := skillspkg.Lookup(resolved, "personal/deploy")
require.NoError(t, err)
require.Equal(t, "deploy", personal.Name)
require.Equal(t, skillspkg.SourcePersonal, personal.Source)
workspace, err := skillspkg.Lookup(resolved, "workspace/deploy")
require.NoError(t, err)
require.Equal(t, "deploy", workspace.Name)
require.Equal(t, skillspkg.SourceWorkspace, workspace.Source)
_, err = skillspkg.Lookup(resolved, "deploy")
require.ErrorIs(t, err, skillspkg.ErrSkillAmbiguous)
require.ErrorContains(t, err, "personal/deploy")
require.ErrorContains(t, err, "workspace/deploy")
}
func TestSkillIndexRefreshReplacesStaleAliases(t *testing.T) {
t.Parallel()
initialResolved := mergeTurnSkills(
[]skillspkg.Skill{{
Name: "deploy",
Description: "Personal deployment process",
Source: skillspkg.SourcePersonal,
}},
nil,
)
prompt := buildSystemPrompt(
[]fantasy.Message{{
Role: fantasy.MessageRoleUser,
Content: []fantasy.MessagePart{
fantasy.TextPart{Text: "Create a workspace."},
},
}},
"",
"",
initialResolved,
"",
systemPromptBehaviorContext{},
)
mergedIndex := chattool.FormatResolvedSkillIndex(mergeTurnSkills(
[]skillspkg.Skill{{
Name: "deploy",
Description: "Personal deployment process",
Source: skillspkg.SourcePersonal,
}},
[]chattool.SkillMeta{{
Name: "deploy",
Description: "Workspace deployment process",
Dir: "/skills/deploy",
}},
))
prompt = removeSkillIndexMessages(prompt)
prompt = chatprompt.InsertSystem(prompt, mergedIndex)
text := systemPromptText(t, prompt)
require.Equal(t, 1, strings.Count(text, "<available-skills>"))
require.NotContains(t, text, "\n- deploy: Personal deployment process")
require.Contains(t, text, "- personal/deploy: Personal deployment process")
require.Contains(t, text, "- workspace/deploy: Workspace deployment process")
}
func requireUserSkillContextActor(ctx context.Context, t *testing.T, userID uuid.UUID) {
t.Helper()
actor, ok := dbauthz.ActorFromContext(ctx)
require.True(t, ok)
require.Equal(t, rbac.SubjectTypeUser, actor.Type)
require.Equal(t, userID.String(), actor.ID)
require.Equal(t, rbac.RoleIdentifiers{rbac.RoleMember()}, actor.Roles)
}
func TestFetchPersonalSkillMetadata(t *testing.T) {
t.Parallel()
t.Run("Success", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
db := dbmock.NewMockStore(ctrl)
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
server := &Server{db: db}
userID := uuid.New()
db.EXPECT().ListUserSkillMetadataByUserID(gomock.Any(), userID).DoAndReturn(
func(ctx context.Context, gotUserID uuid.UUID) ([]database.ListUserSkillMetadataByUserIDRow, error) {
requireUserSkillContextActor(ctx, t, userID)
require.Equal(t, userID, gotUserID)
return []database.ListUserSkillMetadataByUserIDRow{{
UserID: userID,
Name: "personal-review",
Description: "Personal review process",
}}, nil
},
)
got := server.fetchPersonalSkillMetadata(context.Background(), userID, logger)
require.Equal(t, []skillspkg.Skill{{
Name: "personal-review",
Description: "Personal review process",
Source: skillspkg.SourcePersonal,
}}, got)
})
t.Run("ListFailure", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
db := dbmock.NewMockStore(ctrl)
sink := testutil.NewFakeSink(t)
logger := sink.Logger().Leveled(slog.LevelDebug)
server := &Server{db: db}
userID := uuid.New()
db.EXPECT().ListUserSkillMetadataByUserID(gomock.Any(), userID).Return(nil, xerrors.New("boom"))
got := server.fetchPersonalSkillMetadata(context.Background(), userID, logger)
require.Empty(t, got)
warns := sink.Entries(func(e slog.SinkEntry) bool {
return e.Level == slog.LevelWarn && strings.Contains(e.Message, "personal skill metadata")
})
require.NotEmpty(t, warns)
})
}
func TestLoadPersonalSkillBody(t *testing.T) {
t.Parallel()
t.Run("ParsesCurrentContent", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
db := dbmock.NewMockStore(ctrl)
server := &Server{db: db}
userID := uuid.New()
params := database.GetUserSkillByUserIDAndNameParams{
UserID: userID,
Name: "personal-review",
}
db.EXPECT().GetUserSkillByUserIDAndName(gomock.Any(), params).DoAndReturn(
func(ctx context.Context, gotParams database.GetUserSkillByUserIDAndNameParams) (database.UserSkill, error) {
requireUserSkillContextActor(ctx, t, userID)
require.Equal(t, params, gotParams)
return database.UserSkill{
UserID: userID,
Name: "personal-review",
Content: "---\nname: personal-review\ndescription: Personal review process\n---\n\nUpdated instructions.\n",
}, nil
},
)
got, err := server.loadPersonalSkillBody(context.Background(), userID, "personal-review")
require.NoError(t, err)
require.Equal(t, "personal-review", got.Name)
require.Equal(t, "Personal review process", got.Description)
require.Equal(t, skillspkg.SourcePersonal, got.Source)
require.Contains(t, got.Body, "Updated instructions.")
})
t.Run("DeletedSkill", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
db := dbmock.NewMockStore(ctrl)
server := &Server{db: db}
userID := uuid.New()
params := database.GetUserSkillByUserIDAndNameParams{
UserID: userID,
Name: "missing-skill",
}
db.EXPECT().GetUserSkillByUserIDAndName(gomock.Any(), params).DoAndReturn(
func(ctx context.Context, gotParams database.GetUserSkillByUserIDAndNameParams) (database.UserSkill, error) {
requireUserSkillContextActor(ctx, t, userID)
require.Equal(t, params, gotParams)
return database.UserSkill{}, sql.ErrNoRows
},
)
_, err := server.loadPersonalSkillBody(context.Background(), userID, "missing-skill")
require.ErrorIs(t, err, skillspkg.ErrSkillNotFound)
})
t.Run("DatabaseError", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
db := dbmock.NewMockStore(ctrl)
sink := testutil.NewFakeSink(t)
server := &Server{db: db, logger: sink.Logger()}
userID := uuid.New()
params := database.GetUserSkillByUserIDAndNameParams{
UserID: userID,
Name: "error-skill",
}
dbErr := xerrors.New("database unavailable")
db.EXPECT().GetUserSkillByUserIDAndName(gomock.Any(), params).DoAndReturn(
func(ctx context.Context, gotParams database.GetUserSkillByUserIDAndNameParams) (database.UserSkill, error) {
requireUserSkillContextActor(ctx, t, userID)
require.Equal(t, params, gotParams)
return database.UserSkill{}, dbErr
},
)
_, err := server.loadPersonalSkillBody(context.Background(), userID, "error-skill")
require.ErrorContains(t, err, "load personal skill body")
require.ErrorIs(t, err, dbErr)
entries := sink.Entries(func(e slog.SinkEntry) bool {
return e.Level == slog.LevelError && e.Message == "load personal skill body failed"
})
require.Len(t, entries, 1)
requireFieldValue(t, entries[0], "error", dbErr)
})
t.Run("ParseError", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
db := dbmock.NewMockStore(ctrl)
sink := testutil.NewFakeSink(t)
server := &Server{db: db, logger: sink.Logger()}
userID := uuid.New()
params := database.GetUserSkillByUserIDAndNameParams{
UserID: userID,
Name: "broken-skill",
}
db.EXPECT().GetUserSkillByUserIDAndName(gomock.Any(), params).DoAndReturn(
func(ctx context.Context, gotParams database.GetUserSkillByUserIDAndNameParams) (database.UserSkill, error) {
requireUserSkillContextActor(ctx, t, userID)
require.Equal(t, params, gotParams)
return database.UserSkill{
UserID: userID,
Name: "broken-skill",
Content: "---\nname: broken-skill\ndescription: Broken\n---\n\n \n",
}, nil
},
)
_, err := server.loadPersonalSkillBody(context.Background(), userID, "broken-skill")
require.ErrorContains(t, err, "parse personal skill body")
require.ErrorIs(t, err, skillspkg.ErrSkillBodyRequired)
entries := sink.Entries(func(e slog.SinkEntry) bool {
return e.Level == slog.LevelError && e.Message == "parse personal skill body failed"
})
require.Len(t, entries, 1)
requireFieldValue(t, entries[0], "user_id", userID)
requireFieldValue(t, entries[0], "name", "broken-skill")
})
}
func systemPromptText(t *testing.T, prompt []fantasy.Message) string {
t.Helper()
var b strings.Builder
for _, msg := range prompt {
if msg.Role != fantasy.MessageRoleSystem {
continue
}
for _, part := range msg.Content {
textPart, ok := fantasy.AsMessagePart[fantasy.TextPart](part)
if ok {
_, _ = b.WriteString(textPart.Text)
_, _ = b.WriteString("\n")
}
}
}
return b.String()
}
func TestContextFileAgentID(t *testing.T) {
t.Parallel()
+196 -51
View File
@@ -11,12 +11,18 @@ import (
"charm.land/fantasy"
"golang.org/x/xerrors"
skillspkg "github.com/coder/coder/v2/coderd/x/skills"
"github.com/coder/coder/v2/codersdk/workspacesdk"
)
const (
maxSkillMetaBytes = workspacesdk.MaxSkillMetaBytes
maxSkillFileBytes = 512 * 1024
// AvailableSkillsOpenTag is the XML start tag for the skill index block.
AvailableSkillsOpenTag = "<available-skills>"
// AvailableSkillsCloseTag is the XML end tag for the skill index block.
AvailableSkillsCloseTag = "</available-skills>"
)
// SkillMeta is the frontmatter from a skill meta file discovered in a
@@ -45,32 +51,78 @@ type SkillContent struct {
Files []string
}
// 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 {
// FormatResolvedSkillIndex renders an XML block listing all source-aware
// skills. Aliases are the names the model should pass to the skill tools.
func FormatResolvedSkillIndex(resolved []skillspkg.ResolvedSkill) string {
if len(resolved) == 0 {
return ""
}
entries := make([]skillIndexEntry, 0, len(resolved))
hasQualifiedAlias := false
hasWorkspaceSkill := false
for _, s := range resolved {
entries = append(entries, skillIndexEntry{
Alias: s.Alias,
Description: s.Description,
})
if s.Source == skillspkg.SourceWorkspace {
hasWorkspaceSkill = true
}
if s.Alias == skillspkg.QualifiedAlias(s.Source, s.Name) {
hasQualifiedAlias = true
}
}
return renderSkillIndex(entries, skillIndexFormatOptions{
includeQualifiedAliasInstruction: hasQualifiedAlias,
includeReadSkillFileInstruction: hasWorkspaceSkill,
})
}
type skillIndexEntry struct {
Alias string
Description string
}
type skillIndexFormatOptions struct {
includeQualifiedAliasInstruction bool
includeReadSkillFileInstruction bool
}
func renderSkillIndex(entries []skillIndexEntry, opts skillIndexFormatOptions) string {
if len(entries) == 0 {
return ""
}
var b strings.Builder
_, _ = b.WriteString("<available-skills>\n")
_, _ = b.WriteString(AvailableSkillsOpenTag + "\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",
"before following them.\n",
)
for _, s := range skills {
if opts.includeReadSkillFileInstruction {
_, _ = b.WriteString(
"Use read_skill_file to read supporting files " +
"referenced by a workspace skill.\n",
)
}
if opts.includeQualifiedAliasInstruction {
_, _ = b.WriteString(
"When a skill is listed as personal/name or workspace/name, " +
"pass that qualified alias to read_skill.\n",
)
}
_, _ = b.WriteString("\n")
for _, s := range entries {
_, _ = b.WriteString("- ")
_, _ = b.WriteString(s.Name)
_, _ = b.WriteString(s.Alias)
if s.Description != "" {
_, _ = b.WriteString(": ")
_, _ = b.WriteString(s.Description)
}
_, _ = b.WriteString("\n")
}
_, _ = b.WriteString("</available-skills>")
_, _ = b.WriteString(AvailableSkillsCloseTag)
return b.String()
}
@@ -216,11 +268,14 @@ const DefaultSkillMetaFile = "SKILL.md"
type ReadSkillOptions struct {
GetWorkspaceConn func(context.Context) (workspacesdk.AgentConn, error)
GetSkills func() []SkillMeta
ResolveAlias func(string) (skillspkg.ResolvedSkill, error)
LoadPersonalSkillBody func(context.Context, string) (skillspkg.ParsedSkill, error)
}
// 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."`
Name string `json:"name" description:"The name or qualified alias of the skill to read."`
}
// ReadSkill returns an AgentTool that reads the full instructions
@@ -234,44 +289,51 @@ func ReadSkill(options ReadSkillOptions) fantasy.AgentTool {
"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
resolved, err := resolveSkillAlias(options, args.Name)
if err != nil {
return skillResolveErrorResponse(args.Name, err), nil
}
conn, err := options.GetWorkspaceConn(ctx)
if err != nil {
return fantasy.NewTextErrorResponse(
err.Error(),
), nil
switch resolved.Source {
case skillspkg.SourcePersonal:
if options.LoadPersonalSkillBody == nil {
return fantasy.NewTextErrorResponse(
"personal skill loader is not configured",
), nil
}
content, err := options.LoadPersonalSkillBody(ctx, resolved.Name)
if err != nil {
if xerrors.Is(err, skillspkg.ErrSkillNotFound) {
return skillNotFoundResponse(args.Name), nil
}
return fantasy.NewTextErrorResponse(
fmt.Sprintf("failed to load personal skill %q", args.Name),
), nil
}
return toolResponse(map[string]any{
"name": args.Name,
"body": content.Body,
"files": []string{},
}), nil
case skillspkg.SourceWorkspace:
content, response, ok := readWorkspaceSkillBody(ctx, options, args.Name, resolved.Name)
if ok {
return response, nil
}
return toolResponse(map[string]any{
"name": args.Name,
"body": content.Body,
"files": nonNilFiles(content.Files),
}), nil
default:
return skillNotFoundResponse(args.Name), nil
}
// Load the skill body from the workspace agent,
// respecting a custom meta file name if set.
content, err := LoadSkillBody(ctx, conn, skill, cmp.Or(skill.MetaFile, DefaultSkillMetaFile))
if err != nil {
return fantasy.NewTextErrorResponse(
err.Error(),
), nil
}
return toolResponse(map[string]any{
"name": content.Name,
"body": content.Body,
"files": content.Files,
}), nil
},
)
}
@@ -279,7 +341,7 @@ func ReadSkill(options ReadSkillOptions) fantasy.AgentTool {
// ReadSkillFileArgs are the parameters accepted by
// read_skill_file.
type ReadSkillFileArgs struct {
Name string `json:"name" description:"The kebab-case name of the skill."`
Name string `json:"name" description:"The name or qualified alias of the skill to read."`
Path string `json:"path" description:"Relative path to a file in the skill directory (e.g. roles/security-reviewer.md)."`
}
@@ -291,11 +353,6 @@ func ReadSkillFile(options ReadSkillOptions) fantasy.AgentTool {
"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",
@@ -307,12 +364,23 @@ func ReadSkillFile(options ReadSkillOptions) fantasy.AgentTool {
), nil
}
skill, ok := findSkill(options.GetSkills, args.Name)
if !ok {
resolved, err := resolveSkillAlias(options, args.Name)
if err != nil {
return skillResolveErrorResponse(args.Name, err), nil
}
if resolved.Source == skillspkg.SourcePersonal {
return fantasy.NewTextErrorResponse(
fmt.Sprintf("skill %q not found", args.Name),
"read_skill_file is not supported for personal skills (no supporting files)",
), nil
}
if resolved.Source != skillspkg.SourceWorkspace {
return skillNotFoundResponse(args.Name), nil
}
skill, ok := findSkill(options.GetSkills, resolved.Name)
if !ok {
return skillNotFoundResponse(args.Name), nil
}
// Validate the path early so we reject bad
// inputs before dialing the workspace agent.
@@ -322,6 +390,11 @@ func ReadSkillFile(options ReadSkillOptions) fantasy.AgentTool {
), nil
}
if options.GetWorkspaceConn == nil {
return fantasy.NewTextErrorResponse(
"workspace connection resolver is not configured",
), nil
}
conn, err := options.GetWorkspaceConn(ctx)
if err != nil {
return fantasy.NewTextErrorResponse(
@@ -345,6 +418,78 @@ func ReadSkillFile(options ReadSkillOptions) fantasy.AgentTool {
)
}
func resolveSkillAlias(options ReadSkillOptions, name string) (skillspkg.ResolvedSkill, error) {
if options.ResolveAlias != nil {
return options.ResolveAlias(name)
}
skill, ok := findSkill(options.GetSkills, name)
if !ok {
return skillspkg.ResolvedSkill{}, skillspkg.ErrSkillNotFound
}
return skillspkg.ResolvedSkill{
Skill: skillspkg.Skill{
Name: skill.Name,
Description: skill.Description,
Source: skillspkg.SourceWorkspace,
},
Alias: skill.Name,
}, nil
}
func readWorkspaceSkillBody(
ctx context.Context,
options ReadSkillOptions,
requestedName string,
canonicalName string,
) (SkillContent, fantasy.ToolResponse, bool) {
skill, ok := findSkill(options.GetSkills, canonicalName)
if !ok {
return SkillContent{}, skillNotFoundResponse(requestedName), true
}
if options.GetWorkspaceConn == nil {
return SkillContent{}, fantasy.NewTextErrorResponse(
"workspace connection resolver is not configured",
), true
}
conn, err := options.GetWorkspaceConn(ctx)
if err != nil {
return SkillContent{}, fantasy.NewTextErrorResponse(err.Error()), true
}
content, err := LoadSkillBody(ctx, conn, skill, cmp.Or(skill.MetaFile, DefaultSkillMetaFile))
if err != nil {
return SkillContent{}, fantasy.NewTextErrorResponse(err.Error()), true
}
return content, fantasy.ToolResponse{}, false
}
func skillResolveErrorResponse(name string, err error) fantasy.ToolResponse {
if xerrors.Is(err, skillspkg.ErrSkillNotFound) {
return skillNotFoundResponse(name)
}
if xerrors.Is(err, skillspkg.ErrSkillAmbiguous) {
return fantasy.NewTextErrorResponse(err.Error())
}
return fantasy.NewTextErrorResponse(
fmt.Sprintf("failed to resolve skill %q", name),
)
}
func skillNotFoundResponse(name string) fantasy.ToolResponse {
return fantasy.NewTextErrorResponse(
fmt.Sprintf("skill %q not found", name),
)
}
func nonNilFiles(files []string) []string {
if files == nil {
return []string{}
}
return files
}
// findSkill looks up a skill by name in the current skill list.
func findSkill(
getSkills func() []SkillMeta,
+464 -13
View File
@@ -2,6 +2,7 @@ package chattool_test
import (
"context"
"encoding/json"
"io"
"strings"
"testing"
@@ -13,6 +14,7 @@ import (
"golang.org/x/xerrors"
"github.com/coder/coder/v2/coderd/x/chatd/chattool"
skillspkg "github.com/coder/coder/v2/coderd/x/skills"
"github.com/coder/coder/v2/codersdk/workspacesdk"
"github.com/coder/coder/v2/codersdk/workspacesdk/agentconnmock"
)
@@ -23,27 +25,101 @@ 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) {
func responseName(t *testing.T, resp fantasy.ToolResponse) string {
t.Helper()
var payload struct {
Name string `json:"name"`
}
require.NoError(t, json.Unmarshal([]byte(resp.Content), &payload))
return payload.Name
}
func TestFormatResolvedSkillIndex(t *testing.T) {
t.Parallel()
t.Run("Empty", func(t *testing.T) {
t.Parallel()
assert.Empty(t, chattool.FormatSkillIndex(nil))
assert.Empty(t, chattool.FormatResolvedSkillIndex(nil))
})
t.Run("RendersIndex", func(t *testing.T) {
t.Run("PersonalOnly", 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")
idx := chattool.FormatResolvedSkillIndex([]skillspkg.ResolvedSkill{{
Skill: skillspkg.Skill{
Name: "personal-review",
Description: "Personal review process",
Source: skillspkg.SourcePersonal,
},
Alias: "personal-review",
}})
assert.Contains(t, idx, "- personal-review: Personal review process")
assert.NotContains(t, idx, "read_skill_file")
assert.NotContains(t, idx, "qualified alias")
})
t.Run("WorkspaceOnlyMatchesLegacy", func(t *testing.T) {
t.Parallel()
resolved := []skillspkg.ResolvedSkill{{
Skill: skillspkg.Skill{
Name: "deep-review",
Description: "Review",
Source: skillspkg.SourceWorkspace,
},
Alias: "deep-review",
}}
assert.Equal(t,
"<available-skills>\n"+
"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 workspace skill.\n"+
"\n"+
"- deep-review: Review\n"+
"</available-skills>",
chattool.FormatResolvedSkillIndex(resolved),
)
})
t.Run("MixedNonColliding", func(t *testing.T) {
t.Parallel()
idx := chattool.FormatResolvedSkillIndex([]skillspkg.ResolvedSkill{
{
Skill: skillspkg.Skill{
Name: "personal-review",
Description: "Personal review process",
Source: skillspkg.SourcePersonal,
},
Alias: "personal-review",
},
{
Skill: skillspkg.Skill{
Name: "deep-review",
Description: "Workspace review process",
Source: skillspkg.SourceWorkspace,
},
Alias: "deep-review",
},
})
assert.Contains(t, idx, "- personal-review: Personal review process")
assert.Contains(t, idx, "- deep-review: Workspace review process")
assert.Contains(t, idx, "read_skill_file")
assert.NotContains(t, idx, "personal/personal-review")
assert.NotContains(t, idx, "workspace/deep-review")
})
t.Run("CollidingNames", func(t *testing.T) {
t.Parallel()
resolved := skillspkg.MergeSkills(
[]skillspkg.Skill{{Name: "review", Description: "Personal", Source: skillspkg.SourcePersonal}},
[]skillspkg.Skill{{Name: "review", Description: "Workspace", Source: skillspkg.SourceWorkspace}},
)
idx := chattool.FormatResolvedSkillIndex(resolved)
assert.Contains(t, idx, "- personal/review: Personal")
assert.Contains(t, idx, "- workspace/review: Workspace")
assert.Contains(t, idx, "pass that qualified alias to read_skill")
})
}
@@ -277,6 +353,326 @@ func TestReadSkillTool(t *testing.T) {
assert.Contains(t, resp.Content, "Do the thing.")
})
t.Run("PersonalSkill", func(t *testing.T) {
t.Parallel()
tool := chattool.ReadSkill(chattool.ReadSkillOptions{
ResolveAlias: func(alias string) (skillspkg.ResolvedSkill, error) {
require.Equal(t, "my-skill", alias)
return skillspkg.ResolvedSkill{
Skill: skillspkg.Skill{
Name: "my-skill",
Description: "test",
Source: skillspkg.SourcePersonal,
},
Alias: "my-skill",
}, nil
},
LoadPersonalSkillBody: func(context.Context, string) (skillspkg.ParsedSkill, error) {
return skillspkg.ParsedSkill{
Skill: skillspkg.Skill{
Name: "my-skill",
Description: "test",
Source: skillspkg.SourcePersonal,
},
Body: "Personal instructions.",
}, nil
},
})
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, "Personal instructions.")
assert.Contains(t, resp.Content, `"files":[]`)
})
t.Run("PersonalQualifiedAliasPreservesAlias", func(t *testing.T) {
t.Parallel()
var loadedName string
tool := chattool.ReadSkill(chattool.ReadSkillOptions{
ResolveAlias: func(alias string) (skillspkg.ResolvedSkill, error) {
require.Equal(t, "personal/my-skill", alias)
return skillspkg.ResolvedSkill{
Skill: skillspkg.Skill{
Name: "my-skill",
Description: "test",
Source: skillspkg.SourcePersonal,
},
Alias: "personal/my-skill",
}, nil
},
LoadPersonalSkillBody: func(_ context.Context, name string) (skillspkg.ParsedSkill, error) {
loadedName = name
return skillspkg.ParsedSkill{
Skill: skillspkg.Skill{
Name: "my-skill",
Description: "test",
Source: skillspkg.SourcePersonal,
},
Body: "Personal instructions.",
}, nil
},
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "read_skill",
Input: `{"name":"personal/my-skill"}`,
})
require.NoError(t, err)
assert.False(t, resp.IsError)
assert.Equal(t, "personal/my-skill", responseName(t, resp))
assert.Equal(t, "my-skill", loadedName)
})
t.Run("WorkspaceQualifiedAlias", 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{}, nil,
)
tool := chattool.ReadSkill(chattool.ReadSkillOptions{
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
return conn, nil
},
GetSkills: func() []chattool.SkillMeta { return skills },
ResolveAlias: func(alias string) (skillspkg.ResolvedSkill, error) {
require.Equal(t, "workspace/my-skill", alias)
return skillspkg.ResolvedSkill{
Skill: skillspkg.Skill{
Name: "my-skill",
Description: "test",
Source: skillspkg.SourceWorkspace,
},
Alias: "workspace/my-skill",
}, nil
},
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "read_skill",
Input: `{"name":"workspace/my-skill"}`,
})
require.NoError(t, err)
assert.False(t, resp.IsError)
assert.Equal(t, "workspace/my-skill", responseName(t, resp))
assert.Contains(t, resp.Content, "Do the thing.")
})
t.Run("CollisionAliasRoundTrip", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
conn := agentconnmock.NewMockAgentConn(ctrl)
workspaceSkills := []chattool.SkillMeta{{
Name: "deploy",
Description: "workspace deploy",
Dir: "/work/.agents/skills/deploy",
}}
conn.EXPECT().ReadFile(
gomock.Any(), gomock.Any(), int64(0), gomock.Any(),
).Return(
io.NopCloser(strings.NewReader(validSkillMD("deploy", "workspace deploy"))),
"text/markdown",
nil,
)
conn.EXPECT().LS(gomock.Any(), "", gomock.Any()).Return(
workspacesdk.LSResponse{}, nil,
)
resolveAlias := func(alias string) (skillspkg.ResolvedSkill, error) {
switch alias {
case "personal/deploy":
return skillspkg.ResolvedSkill{
Skill: skillspkg.Skill{
Name: "deploy",
Description: "personal deploy",
Source: skillspkg.SourcePersonal,
},
Alias: "personal/deploy",
}, nil
case "workspace/deploy":
return skillspkg.ResolvedSkill{
Skill: skillspkg.Skill{
Name: "deploy",
Description: "workspace deploy",
Source: skillspkg.SourceWorkspace,
},
Alias: "workspace/deploy",
}, nil
default:
return skillspkg.ResolvedSkill{}, skillspkg.ErrSkillNotFound
}
}
tool := chattool.ReadSkill(chattool.ReadSkillOptions{
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
return conn, nil
},
GetSkills: func() []chattool.SkillMeta { return workspaceSkills },
ResolveAlias: resolveAlias,
LoadPersonalSkillBody: func(_ context.Context, name string) (skillspkg.ParsedSkill, error) {
require.Equal(t, "deploy", name)
return skillspkg.ParsedSkill{
Skill: skillspkg.Skill{
Name: "deploy",
Description: "personal deploy",
Source: skillspkg.SourcePersonal,
},
Body: "Personal deploy instructions.",
}, nil
},
})
workspaceResp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "read_skill",
Input: `{"name":"workspace/deploy"}`,
})
require.NoError(t, err)
assert.False(t, workspaceResp.IsError)
workspaceName := responseName(t, workspaceResp)
assert.Equal(t, "workspace/deploy", workspaceName)
workspaceResolved, err := resolveAlias(workspaceName)
require.NoError(t, err)
assert.Equal(t, skillspkg.SourceWorkspace, workspaceResolved.Source)
personalResp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-2",
Name: "read_skill",
Input: `{"name":"personal/deploy"}`,
})
require.NoError(t, err)
assert.False(t, personalResp.IsError)
personalName := responseName(t, personalResp)
assert.Equal(t, "personal/deploy", personalName)
personalResolved, err := resolveAlias(personalName)
require.NoError(t, err)
assert.Equal(t, skillspkg.SourcePersonal, personalResolved.Source)
_, err = resolveAlias("deploy")
require.ErrorIs(t, err, skillspkg.ErrSkillNotFound)
})
t.Run("MissingPersonalSkill", func(t *testing.T) {
t.Parallel()
tool := chattool.ReadSkill(chattool.ReadSkillOptions{
ResolveAlias: func(alias string) (skillspkg.ResolvedSkill, error) {
return skillspkg.ResolvedSkill{
Skill: skillspkg.Skill{Name: alias, Source: skillspkg.SourcePersonal},
Alias: alias,
}, nil
},
LoadPersonalSkillBody: func(context.Context, string) (skillspkg.ParsedSkill, error) {
return skillspkg.ParsedSkill{}, skillspkg.ErrSkillNotFound
},
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "read_skill",
Input: `{"name":"missing-skill"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Contains(t, resp.Content, `skill "missing-skill" not found`)
})
t.Run("PersonalSkillLoaderErrorIsSanitized", func(t *testing.T) {
t.Parallel()
tool := chattool.ReadSkill(chattool.ReadSkillOptions{
ResolveAlias: func(alias string) (skillspkg.ResolvedSkill, error) {
return skillspkg.ResolvedSkill{
Skill: skillspkg.Skill{Name: alias, Source: skillspkg.SourcePersonal},
Alias: alias,
}, nil
},
LoadPersonalSkillBody: func(context.Context, string) (skillspkg.ParsedSkill, error) {
return skillspkg.ParsedSkill{}, xerrors.New("synthetic private storage failure")
},
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "read_skill",
Input: `{"name":"my-skill"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Contains(t, resp.Content, `failed to load personal skill "my-skill"`)
assert.NotContains(t, resp.Content, "synthetic private storage failure")
})
t.Run("ResolveAliasErrorIsSanitized", func(t *testing.T) {
t.Parallel()
tool := chattool.ReadSkill(chattool.ReadSkillOptions{
ResolveAlias: func(string) (skillspkg.ResolvedSkill, error) {
return skillspkg.ResolvedSkill{}, xerrors.New("synthetic private resolver failure")
},
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "read_skill",
Input: `{"name":"my-skill"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Contains(t, resp.Content, `failed to resolve skill "my-skill"`)
assert.NotContains(t, resp.Content, "synthetic private resolver failure")
})
t.Run("AmbiguousLookupSurfacesAliases", func(t *testing.T) {
t.Parallel()
tool := chattool.ReadSkill(chattool.ReadSkillOptions{
ResolveAlias: ambiguousResolveAliasForTest,
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "read_skill",
Input: `{"name":"deploy"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Contains(t, resp.Content, "skill lookup is ambiguous")
assert.Contains(t, resp.Content, "personal/deploy")
assert.Contains(t, resp.Content, "workspace/deploy")
})
t.Run("UnknownSkill", func(t *testing.T) {
t.Parallel()
@@ -320,6 +716,19 @@ func TestReadSkillTool(t *testing.T) {
})
}
func ambiguousResolveAliasForTest(alias string) (skillspkg.ResolvedSkill, error) {
return skillspkg.Lookup([]skillspkg.ResolvedSkill{
{
Skill: skillspkg.Skill{Name: "deploy", Source: skillspkg.SourcePersonal},
Alias: "personal/deploy",
},
{
Skill: skillspkg.Skill{Name: "deploy", Source: skillspkg.SourceWorkspace},
Alias: "workspace/deploy",
},
}, alias)
}
func TestReadSkillFileTool(t *testing.T) {
t.Parallel()
@@ -362,6 +771,48 @@ func TestReadSkillFileTool(t *testing.T) {
assert.Contains(t, resp.Content, "reviewer guide")
})
t.Run("PersonalSkillUnsupported", func(t *testing.T) {
t.Parallel()
tool := chattool.ReadSkillFile(chattool.ReadSkillOptions{
ResolveAlias: func(alias string) (skillspkg.ResolvedSkill, error) {
return skillspkg.ResolvedSkill{
Skill: skillspkg.Skill{Name: alias, Source: skillspkg.SourcePersonal},
Alias: alias,
}, nil
},
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "read_skill_file",
Input: `{"name":"my-skill","path":"helper.md"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Contains(t, resp.Content, "not supported for personal skills")
})
t.Run("AmbiguousLookupSurfacesAliases", func(t *testing.T) {
t.Parallel()
tool := chattool.ReadSkillFile(chattool.ReadSkillOptions{
ResolveAlias: ambiguousResolveAliasForTest,
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "read_skill_file",
Input: `{"name":"deploy","path":"helper.md"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Contains(t, resp.Content, "skill lookup is ambiguous")
assert.Contains(t, resp.Content, "personal/deploy")
assert.Contains(t, resp.Content, "workspace/deploy")
})
t.Run("TraversalRejected", func(t *testing.T) {
t.Parallel()