feat(agent): refine skills support

This commit is contained in:
ParthSareen
2026-07-16 17:14:44 -07:00
parent 944ab0aca5
commit 0593581f97
11 changed files with 210 additions and 49 deletions

View File

@@ -24,7 +24,7 @@ func testSkillCatalog(t *testing.T) *SkillCatalog {
if err := os.Mkdir(path, 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(path, "SKILL.md"), []byte("Use concise bullets."), 0o644); err != nil {
if err := os.WriteFile(filepath.Join(path, "SKILL.md"), []byte("---\nname: release-notes\ndescription: Draft release notes.\n---\nUse concise bullets."), 0o644); err != nil {
t.Fatal(err)
}
catalog, err := DiscoverSkills(dir)

View File

@@ -9,6 +9,8 @@ import (
"regexp"
"sort"
"strings"
"gopkg.in/yaml.v3"
)
const (
@@ -22,7 +24,7 @@ const (
maxSkillBytes = 1 << 20
)
var skillName = regexp.MustCompile(`^[a-z0-9][a-z0-9_-]{0,63}$`)
var skillName = regexp.MustCompile(`^[a-z0-9]+(?:-[a-z0-9]+)*$`)
// SkillsDir returns the canonical runtime-owned skill directory.
func SkillsDir() (string, error) {
@@ -160,7 +162,8 @@ func LoadDefaultSkills(projectDir string) (*SkillCatalog, error) {
for _, root := range roots {
sub, err := DiscoverSkills(root.path)
if err != nil {
return nil, fmt.Errorf("discover skills in %s: %w", root.path, err)
catalog.diagnostics = append(catalog.diagnostics, fmt.Errorf("discover skills in %s: %w", root.path, err))
continue
}
catalog.diagnostics = append(catalog.diagnostics, sub.diagnostics...)
for _, skill := range sub.skills {
@@ -285,17 +288,27 @@ func parseSkill(path, directoryName string) (Skill, error) {
if instructions == "" {
return Skill{}, fmt.Errorf("skill %q: %s is empty", directoryName, skillFilename)
}
skill := Skill{Name: directoryName, Path: path}
if strings.HasPrefix(instructions, "---\n") || strings.HasPrefix(instructions, "---\r\n") {
metadata, body, err := skillFrontMatter(instructions)
if err != nil {
return Skill{}, fmt.Errorf("skill %q: %w", directoryName, err)
}
// The directory name is the canonical skill name; a front matter "name"
// field is accepted for compatibility but is not required to match.
skill.Description = metadata["description"]
instructions = body
if !strings.HasPrefix(instructions, "---\n") && !strings.HasPrefix(instructions, "---\r\n") {
return Skill{}, fmt.Errorf("skill %q: missing YAML front matter", directoryName)
}
metadata, body, err := skillFrontMatter(instructions)
if err != nil {
return Skill{}, fmt.Errorf("skill %q: %w", directoryName, err)
}
if metadata.Name == "" {
return Skill{}, fmt.Errorf("skill %q: front matter requires name", directoryName)
}
if metadata.Description == "" {
return Skill{}, fmt.Errorf("skill %q: front matter requires description", directoryName)
}
if !skillName.MatchString(metadata.Name) {
return Skill{}, fmt.Errorf("skill %q: invalid front matter name %q", directoryName, metadata.Name)
}
if metadata.Name != directoryName {
return Skill{}, fmt.Errorf("skill %q: front matter name %q must match directory name", directoryName, metadata.Name)
}
skill := Skill{Name: metadata.Name, Description: metadata.Description, Path: path}
instructions = body
if strings.TrimSpace(instructions) == "" {
return Skill{}, fmt.Errorf("skill %q: instructions are empty", directoryName)
}
@@ -303,25 +316,28 @@ func parseSkill(path, directoryName string) (Skill, error) {
return skill, nil
}
func skillFrontMatter(input string) (map[string]string, string, error) {
type skillFrontMatterMetadata struct {
Name string `yaml:"name"`
Description string `yaml:"description"`
Metadata map[string]any `yaml:"metadata"`
}
func skillFrontMatter(input string) (skillFrontMatterMetadata, string, error) {
input = strings.ReplaceAll(input, "\r\n", "\n")
lines := strings.Split(input, "\n")
if len(lines) < 3 || lines[0] != "---" {
return nil, "", errors.New("invalid front matter")
return skillFrontMatterMetadata{}, "", errors.New("invalid front matter")
}
metadata := make(map[string]string)
for i := 1; i < len(lines); i++ {
if lines[i] == "---" {
var metadata skillFrontMatterMetadata
if err := yaml.Unmarshal([]byte(strings.Join(lines[1:i], "\n")), &metadata); err != nil {
return skillFrontMatterMetadata{}, "", fmt.Errorf("parse YAML front matter: %w", err)
}
metadata.Name = strings.TrimSpace(metadata.Name)
metadata.Description = strings.TrimSpace(metadata.Description)
return metadata, strings.Join(lines[i+1:], "\n"), nil
}
key, value, ok := strings.Cut(lines[i], ":")
if !ok || strings.TrimSpace(key) == "" || strings.TrimSpace(value) == "" {
return nil, "", fmt.Errorf("invalid front matter line %q", lines[i])
}
if _, exists := metadata[key]; exists {
return nil, "", fmt.Errorf("duplicate front matter field %q", key)
}
metadata[key] = strings.Trim(strings.TrimSpace(value), `"`)
}
return nil, "", errors.New("front matter is not closed")
return skillFrontMatterMetadata{}, "", errors.New("front matter is not closed")
}

View File

@@ -13,6 +13,9 @@ func writeCatalogSkill(t *testing.T, dir, name, content string) {
if err := os.MkdirAll(path, 0o755); err != nil {
t.Fatal(err)
}
if !strings.HasPrefix(content, "---") {
content = "---\nname: " + name + "\ndescription: Test skill.\n---\n" + content
}
if err := os.WriteFile(filepath.Join(path, skillFilename), []byte(content), 0o644); err != nil {
t.Fatal(err)
}
@@ -20,7 +23,7 @@ func writeCatalogSkill(t *testing.T, dir, name, content string) {
func TestDiscoverAndLoadSkills(t *testing.T) {
dir := t.TempDir()
writeCatalogSkill(t, dir, "release-notes", "---\nname: release-notes\ndescription: Draft concise release notes.\n---\n# Release notes\n\nUse short bullets.")
writeCatalogSkill(t, dir, "release-notes", "---\nname: release-notes\ndescription: Draft concise release notes.\nmetadata:\n author: Ollama\n labels:\n - release\n - docs\n---\n# Release notes\n\nUse short bullets.")
catalog, err := DiscoverSkills(dir)
if err != nil {
t.Fatal(err)
@@ -44,20 +47,28 @@ func TestDiscoverAndLoadSkills(t *testing.T) {
func TestDiscoverSkillsSkipsMalformedEntries(t *testing.T) {
dir := t.TempDir()
writeCatalogSkill(t, dir, "valid", "do the useful thing")
// A mismatched front matter "name" no longer rejects a skill: the directory
// name is canonical, so this loads fine.
writeCatalogSkill(t, dir, "mismatched", "---\nname: whatever\ndescription: still loads\n---\nbody")
writeCatalogSkill(t, dir, "mismatched", "---\nname: whatever\ndescription: wrong name\n---\nbody")
// Genuinely malformed front matter (a line without a key:value pair) is still rejected.
writeCatalogSkill(t, dir, "broken", "---\nname: broken\ndescription\n---\nnope")
writeCatalogSkill(t, dir, "missing-name", "---\ndescription: missing name\n---\nbody")
writeCatalogSkill(t, dir, "missing-description", "---\nname: missing-description\n---\nbody")
writeCatalogSkill(t, dir, "bad-name", "---\nname: bad_name\ndescription: invalid name\n---\nbody")
writeCatalogSkill(t, dir, "under_score", "---\nname: under_score\ndescription: invalid directory\n---\nbody")
if err := os.MkdirAll(filepath.Join(dir, "no-front-matter"), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(dir, "no-front-matter", skillFilename), []byte("body"), 0o644); err != nil {
t.Fatal(err)
}
catalog, err := DiscoverSkills(dir)
if err != nil {
t.Fatal(err)
}
if got, want := len(catalog.List()), 2; got != want {
if got, want := len(catalog.List()), 1; got != want {
t.Fatalf("valid skills = %d, want %d", got, want)
}
if got := len(catalog.Diagnostics()); got != 1 {
t.Fatalf("diagnostics = %d, want 1", got)
if got, want := len(catalog.Diagnostics()), 7; got != want {
t.Fatalf("diagnostics = %d, want %d: %#v", got, want, catalog.Diagnostics())
}
if _, err := catalog.Load("broken"); err == nil || !strings.Contains(err.Error(), "not found") {
t.Fatalf("load broken error = %v", err)
@@ -70,7 +81,7 @@ func TestDiscoverSkillsSkipsMalformedEntries(t *testing.T) {
func TestDiscoverSkillsFollowsSymlinks(t *testing.T) {
dir := t.TempDir()
target := t.TempDir()
writeCatalogSkill(t, target, "shared", "---\ndescription: From a linked repo.\n---\nshared instructions")
writeCatalogSkill(t, target, "shared", "---\nname: shared\ndescription: From a linked repo.\n---\nshared instructions")
if err := os.Symlink(filepath.Join(target, "shared"), filepath.Join(dir, "shared")); err != nil {
t.Skipf("symlink not supported: %v", err)
}
@@ -87,6 +98,38 @@ func TestDiscoverSkillsFollowsSymlinks(t *testing.T) {
}
}
func TestLoadDefaultSkillsContinuesAfterBadRoot(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
project := t.TempDir()
writeCatalogSkill(t, filepath.Join(project, ".ollama", "skills"), "release-notes", "project instructions")
badRoot := filepath.Join(t.TempDir(), "not-a-directory")
if err := os.WriteFile(badRoot, []byte("not a directory"), 0o644); err != nil {
t.Fatal(err)
}
t.Setenv(SkillsDirEnv, badRoot)
catalog, err := LoadDefaultSkills(project)
if err != nil {
t.Fatal(err)
}
if _, err := catalog.Load("release-notes"); err != nil {
t.Fatalf("valid skill was hidden by bad root: %v", err)
}
var foundDiagnostic bool
for _, diagnostic := range catalog.Diagnostics() {
if strings.Contains(diagnostic.Error(), badRoot) {
foundDiagnostic = true
break
}
}
if !foundDiagnostic {
t.Fatalf("diagnostics = %#v, want bad root %q", catalog.Diagnostics(), badRoot)
}
}
func TestSkillsDirUsesOverrideAndXDG(t *testing.T) {
base := t.TempDir()
@@ -167,7 +210,7 @@ func TestSkillContentListsDirectoryAndResources(t *testing.T) {
if err := os.MkdirAll(filepath.Join(skillDir, "references"), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("Handle PDFs."), 0o644); err != nil {
if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("---\nname: pdf-processing\ndescription: Handle PDFs.\n---\nHandle PDFs."), 0o644); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(skillDir, "scripts", "extract.py"), []byte("#!/usr/bin/env python3"), 0o755); err != nil {

View File

@@ -16,7 +16,7 @@ func TestSkillLoadsCoreCatalogWithoutApproval(t *testing.T) {
if err := os.Mkdir(path, 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(path, "SKILL.md"), []byte("Use concise bullets."), 0o644); err != nil {
if err := os.WriteFile(filepath.Join(path, "SKILL.md"), []byte("---\nname: release-notes\ndescription: Draft release notes.\n---\nUse concise bullets."), 0o644); err != nil {
t.Fatal(err)
}
catalog, err := agent.DiscoverSkills(dir)

View File

@@ -227,8 +227,6 @@ func GenerateAgentTUI(cmd *cobra.Command, client *api.Client, opts agentTUIOptio
for _, diagnostic := range skillCatalog.Diagnostics() {
fmt.Fprintf(os.Stderr, "\033[1mwarning:\033[0m ignored invalid agent skill: %v\n", diagnostic)
}
skillContext := skillCatalog.SystemContext()
var registry *coreagent.Registry
registryForModel := func(ctx context.Context, model string) *coreagent.Registry {
return agentToolsRegistry(ctx, client, model, skillCatalog)
@@ -236,7 +234,7 @@ func GenerateAgentTUI(cmd *cobra.Command, client *api.Client, opts agentTUIOptio
if opts.Model != "" {
registry = agentToolsRegistry(cmd.Context(), client, opts.Model, skillCatalog)
}
systemPrompt := agentSystemPromptWithWorkingDir(opts.Model, opts.System, skillContext, cwd)
systemPrompt := agentSystemPromptWithWorkingDir(opts.Model, opts.System, agentSkillSystemContext(skillCatalog, registry, opts.ToolsDisabled), cwd)
_, err = agentchat.Run(cmd.Context(), agentchat.Options{
Model: opts.Model,
@@ -253,8 +251,8 @@ func GenerateAgentTUI(cmd *cobra.Command, client *api.Client, opts agentTUIOptio
OnModelSelected: func(_ context.Context, model string) error {
return config.SetLastModel(model)
},
SystemPromptForModel: func(ctx context.Context, model string, registry *coreagent.Registry) string {
return agentSystemPromptWithWorkingDir(model, agentSystemFromShow(ctx, client, model), skillContext, cwd)
SystemPromptForModel: func(ctx context.Context, model string, registry *coreagent.Registry, toolsDisabled bool) string {
return agentSystemPromptWithWorkingDir(model, agentSystemFromShow(ctx, client, model), agentSkillSystemContext(skillCatalog, registry, toolsDisabled), cwd)
},
Skills: skillCatalog,
SystemPrompt: systemPrompt,
@@ -294,6 +292,16 @@ func GenerateAgentTUI(cmd *cobra.Command, client *api.Client, opts agentTUIOptio
return err
}
func agentSkillSystemContext(catalog *coreagent.SkillCatalog, registry *coreagent.Registry, toolsDisabled bool) string {
if toolsDisabled || registry == nil {
return ""
}
if _, ok := registry.Get("skill"); !ok {
return ""
}
return catalog.SystemContext()
}
func selectAgentModel(ctx context.Context, client *api.Client, current string) (string, error) {
models, err := agentModelOptions(ctx, client)
if err != nil {

View File

@@ -12,6 +12,7 @@ import (
"github.com/spf13/cobra"
coreagent "github.com/ollama/ollama/agent"
agenttools "github.com/ollama/ollama/agent/tools"
"github.com/ollama/ollama/api"
"github.com/ollama/ollama/cmd/config"
agentchat "github.com/ollama/ollama/cmd/tui/chat"
@@ -70,6 +71,32 @@ func TestAgentSystemPromptIncludesSkillCatalog(t *testing.T) {
}
}
func TestAgentSkillSystemContextRequiresAvailableEnabledSkillTool(t *testing.T) {
dir := t.TempDir()
if err := os.Mkdir(filepath.Join(dir, "release-notes"), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(dir, "release-notes", "SKILL.md"), []byte("---\nname: release-notes\ndescription: Draft releases.\n---\nUse bullets."), 0o644); err != nil {
t.Fatal(err)
}
catalog, err := coreagent.DiscoverSkills(dir)
if err != nil {
t.Fatal(err)
}
registry := &coreagent.Registry{}
registry.Register(&agenttools.Skill{Catalog: catalog})
if got := agentSkillSystemContext(catalog, registry, false); !strings.Contains(got, "release-notes: Draft releases.") {
t.Fatalf("enabled skill context = %q", got)
}
if got := agentSkillSystemContext(catalog, registry, true); got != "" {
t.Fatalf("disabled tools should omit skill context, got %q", got)
}
if got := agentSkillSystemContext(catalog, &coreagent.Registry{}, false); got != "" {
t.Fatalf("unavailable skill tool should omit skill context, got %q", got)
}
}
func TestAgentSelectionItemsUseLaunchSections(t *testing.T) {
items := agentSelectionItems([]agentchat.ModelOption{
{Name: "glm-5.2:cloud", Description: "cloud", Recommended: true, Cloud: true},

View File

@@ -60,7 +60,7 @@ type Options struct {
MultiModalForModel func(context.Context, string) bool
ModelOptions func(context.Context) ([]ModelOption, error)
OnModelSelected func(context.Context, string) error
SystemPromptForModel func(context.Context, string, *coreagent.Registry) string
SystemPromptForModel func(context.Context, string, *coreagent.Registry, bool) string
ApprovalPrompter coreagent.ApprovalPrompter
EventSinks []coreagent.EventSink
AllowAllTools bool

View File

@@ -74,11 +74,6 @@ func (m *chatModel) handleSubmit() (tea.Model, tea.Cmd) {
return *m, nil
}
_, _, hasSlashCommand := slashCommandInvocation(input)
if !hasSlashCommand {
if _, _, ok := m.skillSlashInvocation(input); ok {
hasSlashCommand = true
}
}
if (m.running || m.compacting) && !hasSlashCommand {
m.status = "wait for current response"
return *m, nil
@@ -227,6 +222,9 @@ func (m *chatModel) handleToolsCommand(args string) (tea.Model, tea.Cmd) {
m.opts.ToolsDisabled = true
m.status = "tools off"
}
if m.opts.SystemPromptForModel != nil {
m.opts.SystemPrompt = m.opts.SystemPromptForModel(m.ctx, m.opts.Model, m.opts.Tools, m.opts.ToolsDisabled)
}
return *m, nil
}

View File

@@ -569,6 +569,46 @@ func TestSkillSlashCommandPromptBecomesUserMessage(t *testing.T) {
}
}
func TestChatSkillSubmitWhileActiveRunKeepsActiveState(t *testing.T) {
catalog := writeTestSkillCatalog(t)
for _, state := range []struct {
name string
running bool
compacting bool
}{
{name: "running", running: true},
{name: "compacting", compacting: true},
} {
t.Run(state.name, func(t *testing.T) {
events := make(chan tea.Msg)
cancel := func() {}
m := chatModel{
opts: Options{Skills: catalog},
input: []rune("/release-notes draft notes"),
running: state.running,
compacting: state.compacting,
events: events,
cancel: cancel,
}
updated, cmd := m.handleSubmit()
if cmd != nil {
t.Fatal("skill submit should not start another run while active")
}
got := updated.(chatModel)
if got.events != events || got.cancel == nil || got.running != state.running || got.compacting != state.compacting {
t.Fatalf("active run state changed: %#v", got)
}
if string(got.input) != "/release-notes draft notes" {
t.Fatalf("input = %q, want skill invocation preserved", got.input)
}
if got.status != "wait for current response" {
t.Fatalf("status = %q", got.status)
}
})
}
}
func writeTestSkillCatalog(t *testing.T) *coreagent.SkillCatalog {
t.Helper()
dir := t.TempDir()
@@ -720,6 +760,35 @@ func TestChatToolsCommandTogglesToolRegistry(t *testing.T) {
}
}
func TestChatToolsCommandRefreshesCapabilityAwareSystemPrompt(t *testing.T) {
registry := &coreagent.Registry{}
registry.Register(chatTestTool{})
m := chatModel{
ctx: context.Background(),
opts: Options{
Model: "test",
Tools: registry,
SystemPromptForModel: func(_ context.Context, _ string, _ *coreagent.Registry, disabled bool) string {
if disabled {
return "tools disabled"
}
return "tools enabled"
},
},
}
updated, _ := m.handleToolsCommand("")
m = updated.(chatModel)
if m.opts.SystemPrompt != "tools disabled" {
t.Fatalf("system prompt = %q, want disabled prompt", m.opts.SystemPrompt)
}
updated, _ = m.handleToolsCommand("")
m = updated.(chatModel)
if m.opts.SystemPrompt != "tools enabled" {
t.Fatalf("system prompt = %q, want enabled prompt", m.opts.SystemPrompt)
}
}
func TestChatToolsCommandUsage(t *testing.T) {
m := chatModel{input: []rune("/tools off")}

View File

@@ -213,7 +213,7 @@ func (m *chatModel) applyModelSelection(modelName string, persist bool) error {
m.opts.Tools = m.opts.ToolRegistryForModel(m.ctx, modelName)
}
if m.opts.SystemPromptForModel != nil {
m.opts.SystemPrompt = m.opts.SystemPromptForModel(m.ctx, modelName, m.opts.Tools)
m.opts.SystemPrompt = m.opts.SystemPromptForModel(m.ctx, modelName, m.opts.Tools, m.opts.ToolsDisabled)
}
if m.opts.MultiModalForModel != nil {
ctx := m.ctx

View File

@@ -289,7 +289,7 @@ func TestChatModelPickerFiltersAndSwitchesModel(t *testing.T) {
}
return 262144
},
SystemPromptForModel: func(ctx context.Context, model string, registry *coreagent.Registry) string {
SystemPromptForModel: func(ctx context.Context, model string, registry *coreagent.Registry, toolsDisabled bool) string {
if model != "qwen3.5:cloud" {
t.Fatalf("system prompt model = %q, want qwen3.5:cloud", model)
}
@@ -397,7 +397,7 @@ func TestChatModelSwitchNextRunKeepsHistory(t *testing.T) {
opts: Options{
Model: "llama3.2",
Client: client,
SystemPromptForModel: func(_ context.Context, model string, _ *coreagent.Registry) string {
SystemPromptForModel: func(_ context.Context, model string, _ *coreagent.Registry, _ bool) string {
return "system for " + model
},
},