From fce745fe5e4f0c7741ca46fc5484c9471c997d00 Mon Sep 17 00:00:00 2001 From: Parth Sareen Date: Wed, 22 Jul 2026 22:40:25 -0700 Subject: [PATCH] agent: import skills from coding agents (#17294) --- agent/skills.go | 350 +++++++++++++++++++ agent/skills_test.go | 199 +++++++++++ agent/testdata/import/release-notes/SKILL.md | 8 + cmd/agent_tui.go | 22 +- cmd/tui/chat/chat.go | 2 + cmd/tui/chat/input.go | 113 +++++- cmd/tui/chat/input_test.go | 95 +++++ 7 files changed, 779 insertions(+), 10 deletions(-) create mode 100644 agent/testdata/import/release-notes/SKILL.md diff --git a/agent/skills.go b/agent/skills.go index 1d863b03f..6766f2bbb 100644 --- a/agent/skills.go +++ b/agent/skills.go @@ -1,8 +1,10 @@ package agent import ( + "bytes" "errors" "fmt" + "io" "io/fs" "os" "path/filepath" @@ -271,6 +273,354 @@ type skillRoot struct { path string } +// SkillImportResult describes one import attempt. Failed skills do not prevent +// other valid skills in the same source root from being imported. +type SkillImportResult struct { + Source string + SourceDir string + Destination string + Imported []string + Existing []string + Failures []SkillImportFailure +} + +// SkillImportFailure identifies a source skill that was deliberately skipped. +// The destination is never changed for a failed skill. +type SkillImportFailure struct { + Name string + Err error +} + +// ImportSkills imports skills from a conventional coding-agent source into the +// canonical Ollama skills directory. Supported sources are codex, claude, and +// pi. Existing skills are left untouched: an identical directory is reported +// as existing, and a differing one is reported as a conflict. +func ImportSkills(source string) (SkillImportResult, error) { + home, err := os.UserHomeDir() + if err != nil { + return SkillImportResult{}, fmt.Errorf("resolve home directory: %w", err) + } + + destination, err := SkillsDir() + if err != nil { + return SkillImportResult{}, fmt.Errorf("resolve Ollama skills directory: %w", err) + } + return importSkillsFromRoots(source, conventionalSkillImportRoots(home), destination) +} + +func conventionalSkillImportRoots(home string) map[string]string { + return map[string]string{ + "codex": filepath.Join(home, ".codex", "skills"), + "claude": filepath.Join(home, ".claude", "skills"), + "pi": filepath.Join(home, ".pi", "agent", "skills"), + } +} + +func importSkillsFromRoots(source string, roots map[string]string, destination string) (SkillImportResult, error) { + source = strings.ToLower(strings.TrimSpace(source)) + sourceDir, ok := roots[source] + if !ok { + return SkillImportResult{}, fmt.Errorf("unknown skill source %q", source) + } + return importSkillsFromDir(source, sourceDir, destination) +} + +func importSkillsFromDir(source, sourceDir, destination string) (SkillImportResult, error) { + result := SkillImportResult{Source: source, SourceDir: sourceDir, Destination: destination} + info, err := os.Lstat(sourceDir) + if errors.Is(err, fs.ErrNotExist) { + return result, nil + } + if err != nil { + return result, fmt.Errorf("inspect %s skills directory: %w", source, err) + } + if info.Mode()&os.ModeSymlink != 0 { + return result, fmt.Errorf("inspect %s skills directory: symlinks are not supported", source) + } + if !info.IsDir() { + return result, fmt.Errorf("inspect %s skills directory: not a directory", source) + } + + entries, err := os.ReadDir(sourceDir) + if err != nil { + return result, fmt.Errorf("read %s skills directory: %w", source, err) + } + for _, entry := range entries { + name := entry.Name() + path := filepath.Join(sourceDir, name) + if entry.Type()&os.ModeSymlink != 0 { + result.Failures = append(result.Failures, SkillImportFailure{Name: name, Err: errors.New("symlinked skill directories are not supported")}) + continue + } + info, err := entry.Info() + if err != nil { + result.Failures = append(result.Failures, SkillImportFailure{Name: name, Err: fmt.Errorf("inspect source: %w", err)}) + continue + } + if !info.IsDir() { + continue + } + if !skillName.MatchString(name) { + result.Failures = append(result.Failures, SkillImportFailure{Name: name, Err: errors.New("invalid skill directory name")}) + continue + } + if err := validateImportSkill(path, name); err != nil { + result.Failures = append(result.Failures, SkillImportFailure{Name: name, Err: err}) + continue + } + + state, err := importSkillDirectory(path, filepath.Join(destination, name)) + if err != nil { + result.Failures = append(result.Failures, SkillImportFailure{Name: name, Err: err}) + continue + } + if state == skillImportExisting { + result.Existing = append(result.Existing, name) + } else { + result.Imported = append(result.Imported, name) + } + } + return result, nil +} + +func validateImportSkill(dir, name string) error { + manifest := filepath.Join(dir, skillFilename) + info, err := os.Lstat(manifest) + if err != nil { + return fmt.Errorf("inspect %s: %w", skillFilename, err) + } + if info.Mode()&os.ModeSymlink != 0 || !info.Mode().IsRegular() { + return fmt.Errorf("%s must be a regular, non-symlinked file", skillFilename) + } + if _, err := parseSkill(manifest, name); err != nil { + return err + } + return walkImportTree(dir, func(path string, entry fs.DirEntry, info fs.FileInfo) error { + if info.IsDir() || path == dir { + return nil + } + if !info.Mode().IsRegular() { + return fmt.Errorf("only regular files may be imported: %s", path) + } + file, err := os.Open(path) + if err != nil { + return fmt.Errorf("read %s: %w", path, err) + } + return file.Close() + }) +} + +func walkImportTree(root string, visit func(string, fs.DirEntry, fs.FileInfo) error) error { + return filepath.WalkDir(root, func(path string, entry fs.DirEntry, err error) error { + if err != nil { + return err + } + rel, err := filepath.Rel(root, path) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return fmt.Errorf("unsafe skill path %q", path) + } + if entry.Type()&os.ModeSymlink != 0 { + return fmt.Errorf("symlinks may not be imported: %s", path) + } + info, err := entry.Info() + if err != nil { + return err + } + return visit(path, entry, info) + }) +} + +type skillImportState int + +const ( + skillImportCopied skillImportState = iota + skillImportExisting +) + +func importSkillDirectory(source, destination string) (skillImportState, error) { + if info, err := os.Lstat(destination); err == nil { + if info.Mode()&os.ModeSymlink != 0 || !info.IsDir() { + return 0, errors.New("destination exists but is not a regular directory") + } + same, err := sameImportTree(source, destination) + if err != nil { + return 0, fmt.Errorf("inspect existing destination: %w", err) + } + if same { + return skillImportExisting, nil + } + return 0, errors.New("destination skill already exists with different contents") + } else if !errors.Is(err, fs.ErrNotExist) { + return 0, fmt.Errorf("inspect destination: %w", err) + } + + if err := ensureImportDestination(filepath.Dir(destination)); err != nil { + return 0, err + } + stage, err := os.MkdirTemp(filepath.Dir(destination), "."+filepath.Base(destination)+".import-") + if err != nil { + return 0, fmt.Errorf("create import staging directory: %w", err) + } + defer os.RemoveAll(stage) + if err := copyImportTree(source, stage); err != nil { + return 0, err + } + if _, err := os.Lstat(destination); err == nil { + return 0, errors.New("destination skill was created during import") + } else if !errors.Is(err, fs.ErrNotExist) { + return 0, fmt.Errorf("inspect destination before install: %w", err) + } + if err := os.Rename(stage, destination); err != nil { + return 0, fmt.Errorf("install imported skill: %w", err) + } + return skillImportCopied, nil +} + +func ensureImportDestination(dir string) error { + if err := os.MkdirAll(dir, 0o755); err != nil { + return fmt.Errorf("create Ollama skills directory: %w", err) + } + info, err := os.Lstat(dir) + if err != nil { + return fmt.Errorf("inspect Ollama skills directory: %w", err) + } + if info.Mode()&os.ModeSymlink != 0 || !info.IsDir() { + return errors.New("Ollama skills directory must be a regular, non-symlinked directory") + } + return nil +} + +func copyImportTree(source, destination string) error { + return walkImportTree(source, func(path string, entry fs.DirEntry, info fs.FileInfo) error { + rel, err := filepath.Rel(source, path) + if err != nil { + return err + } + target := destination + if rel != "." { + target = filepath.Join(destination, rel) + } + if info.IsDir() { + if rel == "." { + return nil + } + return os.Mkdir(target, info.Mode().Perm()) + } + if !info.Mode().IsRegular() { + return fmt.Errorf("only regular files may be imported: %s", path) + } + return copyImportFile(path, target, info.Mode().Perm()) + }) +} + +func copyImportFile(source, destination string, mode fs.FileMode) error { + in, err := os.Open(source) + if err != nil { + return fmt.Errorf("read %s: %w", source, err) + } + defer in.Close() + out, err := os.OpenFile(destination, os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode) + if err != nil { + return fmt.Errorf("create %s: %w", destination, err) + } + _, copyErr := io.Copy(out, in) + closeErr := out.Close() + if copyErr != nil { + return fmt.Errorf("copy %s: %w", source, copyErr) + } + if closeErr != nil { + return fmt.Errorf("write %s: %w", destination, closeErr) + } + return nil +} + +func sameImportTree(source, destination string) (bool, error) { + seen := make(map[string]struct{}) + same := true + err := walkImportTree(source, func(path string, entry fs.DirEntry, info fs.FileInfo) error { + rel, err := filepath.Rel(source, path) + if err != nil { + return err + } + seen[rel] = struct{}{} + other := destination + if rel != "." { + other = filepath.Join(destination, rel) + } + otherInfo, err := os.Lstat(other) + if errors.Is(err, fs.ErrNotExist) { + same = false + return nil + } + if err != nil { + return err + } + if otherInfo.Mode()&os.ModeSymlink != 0 || otherInfo.IsDir() != info.IsDir() || (!info.IsDir() && !otherInfo.Mode().IsRegular()) { + same = false + return nil + } + if info.Mode().IsRegular() { + equal, err := sameImportFile(path, other) + if err != nil { + return err + } + if !equal { + same = false + } + } + return nil + }) + if err != nil || !same { + return same, err + } + err = walkImportTree(destination, func(path string, entry fs.DirEntry, info fs.FileInfo) error { + rel, err := filepath.Rel(destination, path) + if err != nil { + return err + } + if _, ok := seen[rel]; !ok { + same = false + } + return nil + }) + return same, err +} + +func sameImportFile(first, second string) (bool, error) { + a, err := os.Open(first) + if err != nil { + return false, err + } + defer a.Close() + b, err := os.Open(second) + if err != nil { + return false, err + } + defer b.Close() + + left := make([]byte, 32*1024) + right := make([]byte, len(left)) + for { + n, errA := a.Read(left) + m, errB := b.Read(right) + if n != m || !bytes.Equal(left[:n], right[:m]) { + return false, nil + } + if errA == io.EOF && errB == io.EOF { + return true, nil + } + if errA != nil && errA != io.EOF { + return false, errA + } + if errB != nil && errB != io.EOF { + return false, errB + } + if errA == io.EOF || errB == io.EOF { + return false, nil + } + } +} + // defaultSkillRoots returns skill directories ordered lowest- to // highest-precedence. Non-existent directories are scanned harmlessly // (DiscoverSkills skips them). diff --git a/agent/skills_test.go b/agent/skills_test.go index 12059812f..767c044ea 100644 --- a/agent/skills_test.go +++ b/agent/skills_test.go @@ -21,6 +21,21 @@ func writeCatalogSkill(t *testing.T, dir, name, content string) { } } +func writeImportFixtureSkill(t *testing.T, dir string) { + t.Helper() + contents, err := os.ReadFile(filepath.Join("testdata", "import", "release-notes", skillFilename)) + if err != nil { + t.Fatal(err) + } + path := filepath.Join(dir, "release-notes", skillFilename) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, contents, 0o644); err != nil { + t.Fatal(err) + } +} + func TestDiscoverAndLoadSkills(t *testing.T) { dir := t.TempDir() 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.") @@ -291,3 +306,187 @@ func TestSkillContentListsDirectoryAndResources(t *testing.T) { t.Fatalf("content missing resource listing: %q", content) } } + +func TestImportSkillsCopiesFixtureAndIsIdempotent(t *testing.T) { + source := t.TempDir() + destination := t.TempDir() + writeImportFixtureSkill(t, source) + writeCatalogSkill(t, source, "broken", "---\nname: another-skill\ndescription: Deliberately invalid.\n---\nIgnore this.") + if err := os.MkdirAll(filepath.Join(source, "release-notes", "references"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(source, "release-notes", "references", "style.txt"), []byte("Keep it short.\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(source, "release-notes", "scripts"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(source, "release-notes", "scripts", "prepare.sh"), []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(source, "ignored.md"), []byte("Ignored root file.\n"), 0o644); err != nil { + t.Fatal(err) + } + + result, err := importSkillsFromDir("codex", source, destination) + if err != nil { + t.Fatal(err) + } + if got, want := strings.Join(result.Imported, ","), "release-notes"; got != want { + t.Fatalf("imported = %q, want %q", got, want) + } + catalog, err := DiscoverSkills(destination) + if err != nil { + t.Fatal(err) + } + skill, err := catalog.Load("release-notes") + if err != nil || skill.Description != "Draft concise release notes." { + t.Fatalf("imported skill = %#v, %v", skill, err) + } + if got := len(result.Failures); got != 1 || result.Failures[0].Name != "broken" { + t.Fatalf("failures = %#v, want broken fixture failure", result.Failures) + } + for _, file := range []string{skillFilename, filepath.Join("references", "style.txt"), filepath.Join("scripts", "prepare.sh")} { + if _, err := os.Stat(filepath.Join(destination, "release-notes", file)); err != nil { + t.Fatalf("imported fixture file %q: %v", file, err) + } + } + + result, err = importSkillsFromDir("codex", source, destination) + if err != nil { + t.Fatal(err) + } + if got, want := strings.Join(result.Existing, ","), "release-notes"; got != want { + t.Fatalf("existing = %q, want %q", got, want) + } + if len(result.Imported) != 0 { + t.Fatalf("repeated import copied skills: %#v", result.Imported) + } +} + +func TestImportSkillsLeavesConflictsAndUnsafeSourcesUntouched(t *testing.T) { + source := t.TempDir() + destination := t.TempDir() + writeCatalogSkill(t, source, "release-notes", "source instructions") + writeCatalogSkill(t, destination, "release-notes", "existing instructions") + writeCatalogSkill(t, source, "nested-link", "safe manifest") + if err := os.Symlink(filepath.Join(source, "release-notes", skillFilename), filepath.Join(source, "nested-link", "reference")); err != nil { + t.Skipf("symlink not supported: %v", err) + } + if err := os.Symlink(filepath.Join(source, "release-notes"), filepath.Join(source, "linked-skill")); err != nil { + t.Skipf("symlink not supported: %v", err) + } + + result, err := importSkillsFromDir("codex", source, destination) + if err != nil { + t.Fatal(err) + } + if len(result.Imported) != 0 || len(result.Existing) != 0 { + t.Fatalf("unexpected successful import: %#v", result) + } + if got, err := os.ReadFile(filepath.Join(destination, "release-notes", skillFilename)); err != nil || !strings.Contains(string(got), "existing instructions") { + t.Fatalf("conflicting destination changed: %q, %v", got, err) + } + failed := make(map[string]bool) + for _, failure := range result.Failures { + failed[failure.Name] = true + } + for _, name := range []string{"release-notes", "nested-link", "linked-skill"} { + if !failed[name] { + t.Fatalf("missing failure for %q: %#v", name, result.Failures) + } + } +} + +func TestImportSkillsRejectsSymlinkedRoot(t *testing.T) { + root := t.TempDir() + source := filepath.Join(t.TempDir(), "codex-skills") + if err := os.Symlink(root, source); err != nil { + t.Skipf("symlink not supported: %v", err) + } + result, err := importSkillsFromDir("codex", source, t.TempDir()) + if err == nil || !strings.Contains(err.Error(), "symlinks are not supported") { + t.Fatalf("symlinked root error = %v", err) + } + if len(result.Imported) != 0 || len(result.Existing) != 0 || len(result.Failures) != 0 { + t.Fatalf("symlinked root result = %#v", result) + } +} + +func TestImportSkillsMissingRootAndConfiguredRoots(t *testing.T) { + result, err := importSkillsFromDir("codex", filepath.Join(t.TempDir(), "missing"), t.TempDir()) + if err != nil { + t.Fatal(err) + } + if len(result.Imported) != 0 || len(result.Existing) != 0 || len(result.Failures) != 0 { + t.Fatalf("missing root result = %#v", result) + } + + destination := t.TempDir() + rootBase := t.TempDir() + roots := map[string]string{ + "codex": filepath.Join(rootBase, "codex"), + "claude": filepath.Join(rootBase, "claude"), + "pi": filepath.Join(rootBase, "pi"), + } + for _, test := range []struct { + source string + root string + name string + }{ + {source: "codex", root: roots["codex"], name: "from-codex"}, + {source: "claude", root: roots["claude"], name: "from-claude"}, + {source: "pi", root: roots["pi"], name: "from-pi"}, + } { + t.Run(test.source, func(t *testing.T) { + writeCatalogSkill(t, test.root, test.name, "from "+test.source) + result, err = importSkillsFromRoots(test.source, roots, destination) + if err != nil { + t.Fatal(err) + } + if result.SourceDir != test.root { + t.Fatalf("source dir = %q, want %q", result.SourceDir, test.root) + } + if _, err := os.Stat(filepath.Join(destination, test.name, skillFilename)); err != nil { + t.Fatalf("conventional source was not imported: %v", err) + } + }) + } + if _, err := importSkillsFromRoots("unknown", roots, destination); err == nil || !strings.Contains(err.Error(), "unknown skill source") { + t.Fatalf("unknown source error = %v", err) + } +} + +func TestConventionalSkillImportRoots(t *testing.T) { + home := t.TempDir() + roots := conventionalSkillImportRoots(home) + for source, want := range map[string]string{ + "codex": filepath.Join(home, ".codex", "skills"), + "claude": filepath.Join(home, ".claude", "skills"), + "pi": filepath.Join(home, ".pi", "agent", "skills"), + } { + if got := roots[source]; got != want { + t.Fatalf("%s root = %q, want %q", source, got, want) + } + } +} + +func TestImportSkillsRejectsUnreadableManifest(t *testing.T) { + source := t.TempDir() + writeCatalogSkill(t, source, "private", "do not read") + manifest := filepath.Join(source, "private", skillFilename) + if err := os.Chmod(manifest, 0); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(manifest, 0o644) }) + if _, err := os.ReadFile(manifest); err == nil { + t.Skip("test user can read a mode-000 file") + } + result, err := importSkillsFromDir("codex", source, t.TempDir()) + if err != nil { + t.Fatal(err) + } + if len(result.Failures) != 1 || result.Failures[0].Name != "private" { + t.Fatalf("failures = %#v", result.Failures) + } +} diff --git a/agent/testdata/import/release-notes/SKILL.md b/agent/testdata/import/release-notes/SKILL.md new file mode 100644 index 000000000..dae33c015 --- /dev/null +++ b/agent/testdata/import/release-notes/SKILL.md @@ -0,0 +1,8 @@ +--- +name: release-notes +description: Draft concise release notes. +--- + +# Release notes + +Use short bullets. diff --git a/cmd/agent_tui.go b/cmd/agent_tui.go index 447ecbeb2..b2121517a 100644 --- a/cmd/agent_tui.go +++ b/cmd/agent_tui.go @@ -83,12 +83,20 @@ func GenerateAgentTUI(cmd *cobra.Command, client *api.Client, opts agentTUIOptio return agentContextWindowForModel(ctx, client, model, fallback) } - skillCatalog, err := coreagent.LoadDefaultSkills(cwd) - if err != nil { - return fmt.Errorf("load agent skills: %w", err) + var skillCatalog *coreagent.SkillCatalog + reloadSkills := func() (*coreagent.SkillCatalog, error) { + catalog, err := coreagent.LoadDefaultSkills(cwd) + if err != nil { + return nil, err + } + for _, diagnostic := range catalog.Diagnostics() { + fmt.Fprintf(os.Stderr, "\033[1mwarning:\033[0m ignored invalid agent skill: %v\n", diagnostic) + } + skillCatalog = catalog + return catalog, nil } - for _, diagnostic := range skillCatalog.Diagnostics() { - fmt.Fprintf(os.Stderr, "\033[1mwarning:\033[0m ignored invalid agent skill: %v\n", diagnostic) + if _, err := reloadSkills(); err != nil { + return fmt.Errorf("load agent skills: %w", err) } var registry *coreagent.Registry registryForModel := func(ctx context.Context, model string) *coreagent.Registry { @@ -99,7 +107,7 @@ func GenerateAgentTUI(cmd *cobra.Command, client *api.Client, opts agentTUIOptio } systemPrompt := agentSystemPromptWithWorkingDir(opts.Model, opts.System, agentSkillSystemContext(skillCatalog, registry, opts.ToolsDisabled), cwd) - _, err = agentchat.Run(cmd.Context(), agentchat.Options{ + _, err := agentchat.Run(cmd.Context(), agentchat.Options{ Model: opts.Model, Client: client, Tools: registry, @@ -118,6 +126,8 @@ func GenerateAgentTUI(cmd *cobra.Command, client *api.Client, opts agentTUIOptio return agentSystemPromptWithWorkingDir(model, agentSystemFromShow(ctx, client, model), agentSkillSystemContext(skillCatalog, registry, toolsDisabled), cwd) }, Skills: skillCatalog, + ImportSkills: coreagent.ImportSkills, + ReloadSkills: reloadSkills, SystemPrompt: systemPrompt, WorkingDir: cwd, Format: opts.Format, diff --git a/cmd/tui/chat/chat.go b/cmd/tui/chat/chat.go index 700241722..db1a8bea7 100644 --- a/cmd/tui/chat/chat.go +++ b/cmd/tui/chat/chat.go @@ -55,6 +55,8 @@ type Options struct { Client coreagent.ChatClient Tools *coreagent.Registry Skills *coreagent.SkillCatalog + ImportSkills func(string) (coreagent.SkillImportResult, error) + ReloadSkills func() (*coreagent.SkillCatalog, error) ToolRegistryForModel func(context.Context, string) *coreagent.Registry ToolsDisabled bool MultiModalForModel func(context.Context, string) bool diff --git a/cmd/tui/chat/input.go b/cmd/tui/chat/input.go index 178b07164..d032d74c0 100644 --- a/cmd/tui/chat/input.go +++ b/cmd/tui/chat/input.go @@ -55,7 +55,7 @@ var chatSlashCommands = []chatSlashCommand{ {name: "/new", description: "start a new chat"}, {name: "/think", description: "set thinking mode"}, {name: "/tools", description: "toggle tools on or off"}, - {name: "/skills", description: "list available skills"}, + {name: "/skills", usage: "/skills [import codex|claude|pi]", description: "list or import skills"}, {name: "/compact", description: "summarize older context"}, {name: "/help", description: "show commands", aliases: []string{"/?"}}, {name: "/bye", description: "exit", aliases: []string{"/exit"}}, @@ -63,6 +63,12 @@ var chatSlashCommands = []chatSlashCommand{ {name: "/save", usage: "/save ", description: "save request JSON; saved as .json"}, } +var skillsImportCompletions = []chatCompletion{ + {value: "/skills import codex", label: "/skills import codex", description: "import from ~/.codex/skills"}, + {value: "/skills import claude", label: "/skills import claude", description: "import from ~/.claude/skills"}, + {value: "/skills import pi", label: "/skills import pi", description: "import from ~/.pi/agent/skills"}, +} + func (m *chatModel) handleSubmit() (tea.Model, tea.Cmd) { m.syncInputPlaceholders() input := strings.TrimSpace(string(m.input)) @@ -159,8 +165,10 @@ func (m *chatModel) submitInput(input string) (tea.Model, tea.Cmd) { } func (m *chatModel) handleSkillsCommand(args string) (tea.Model, tea.Cmd) { - if strings.TrimSpace(args) != "" { - m.entries = append(m.entries, newChatEntry(chatEntry{role: "error", content: "usage: /skills"})) + if fields := strings.Fields(args); len(fields) == 2 && fields[0] == "import" { + return m.handleSkillsImport(fields[1]) + } else if len(fields) != 0 { + m.entries = append(m.entries, newChatEntry(chatEntry{role: "error", content: "usage: /skills [import codex|claude|pi]"})) return *m, nil } skills := m.opts.Skills.List() @@ -181,6 +189,61 @@ func (m *chatModel) handleSkillsCommand(args string) (tea.Model, tea.Cmd) { return *m, nil } +func (m *chatModel) handleSkillsImport(source string) (tea.Model, tea.Cmd) { + importSkills := m.opts.ImportSkills + if importSkills == nil { + importSkills = coreagent.ImportSkills + } + result, err := importSkills(source) + if err != nil { + m.status = "error" + m.entries = append(m.entries, newChatEntry(chatEntry{role: "error", content: fmt.Sprintf("Could not import %s skills: %v", source, err)})) + return *m, nil + } + + if len(result.Imported) != 0 || len(result.Existing) != 0 { + reload := m.opts.ReloadSkills + if reload == nil { + reload = func() (*coreagent.SkillCatalog, error) { + return coreagent.LoadDefaultSkills(m.currentWorkingDir()) + } + } + catalog, err := reload() + if err != nil { + m.status = "error" + m.entries = append(m.entries, newChatEntry(chatEntry{role: "error", content: fmt.Sprintf("%s\n\nCould not reload skills: %v", skillsImportSummary(result), err)})) + return *m, nil + } + m.opts.Skills = catalog + if m.opts.ToolRegistryForModel != nil && m.opts.Model != "" { + m.opts.Tools = m.opts.ToolRegistryForModel(m.ctx, m.opts.Model) + } + if m.opts.SystemPromptForModel != nil { + m.opts.SystemPrompt = m.opts.SystemPromptForModel(m.ctx, m.opts.Model, m.opts.Tools, m.opts.ToolsDisabled) + } + m.status = "skills reloaded" + } + m.entries = append(m.entries, newSlashEntry(skillsImportSummary(result))) + return *m, nil +} + +func skillsImportSummary(result coreagent.SkillImportResult) string { + if len(result.Imported) == 0 && len(result.Existing) == 0 && len(result.Failures) == 0 { + return fmt.Sprintf("No %s skills found at %s.", result.Source, result.SourceDir) + } + var lines []string + if len(result.Imported) != 0 { + lines = append(lines, fmt.Sprintf("Imported %d skill%s from %s.", len(result.Imported), pluralSuffix(len(result.Imported)), result.SourceDir)) + } + if len(result.Existing) != 0 { + lines = append(lines, "Already present (left unchanged): "+strings.Join(result.Existing, ", ")+".") + } + for _, failure := range result.Failures { + lines = append(lines, fmt.Sprintf("Skipped %s: %v.", failure.Name, failure.Err)) + } + return strings.Join(lines, "\n") +} + func skillsDirForDisplay(catalog *coreagent.SkillCatalog) string { if catalog != nil && catalog.Dir() != "" { return catalog.Dir() @@ -1117,13 +1180,16 @@ func (m chatModel) completions() []chatCompletion { func (m chatModel) slashCompletions() []chatCompletion { rawInput := string(m.input) - input := strings.TrimSpace(rawInput) + input := strings.TrimLeftFunc(rawInput, unicode.IsSpace) if !strings.HasPrefix(input, "/") { return nil } if m.skillSlashPromptStarted(rawInput) { return nil } + if completions := matchingSkillsImportCompletions(input); completions != nil { + return completions + } commands := matchingSlashCommands(input) completions := make([]chatCompletion, 0, len(commands)) @@ -1134,6 +1200,13 @@ func (m chatModel) slashCompletions() []chatCompletion { description: command.description, }) } + if strings.EqualFold(input, "/skills") { + completions = append(completions, chatCompletion{ + value: "/skills import", + label: "/skills import", + description: "import skills from Codex, Claude, or Pi", + }) + } // Each catalog skill is also invocable as "/"; surface them as // completions so they are discoverable by typing. if m.opts.Skills != nil { @@ -1163,6 +1236,38 @@ func (m chatModel) slashCompletions() []chatCompletion { return completions } +func matchingSkillsImportCompletions(input string) []chatCompletion { + const importCommand = "/skills import" + lower := strings.ToLower(input) + if lower == "/skills" { + return nil // Preserve Enter on /skills as the listing command. + } + if !strings.HasPrefix(lower, "/skills ") { + return nil + } + if strings.HasPrefix(importCommand, lower) { + return []chatCompletion{{ + value: importCommand, + label: importCommand, + description: "import skills from Codex, Claude, or Pi", + }} + } + if !strings.HasPrefix(lower, importCommand) { + return nil + } + prefix := strings.TrimSpace(strings.TrimPrefix(lower, importCommand)) + completions := make([]chatCompletion, 0, len(skillsImportCompletions)) + for _, completion := range skillsImportCompletions { + if strings.HasPrefix(strings.TrimPrefix(completion.value, importCommand+" "), prefix) { + completions = append(completions, completion) + } + } + if len(completions) == 0 { + return []chatCompletion{{label: "No matching skill sources"}} + } + return completions +} + func (m chatModel) skillSlashPromptStarted(input string) bool { input = strings.TrimLeftFunc(input, unicode.IsSpace) end := strings.IndexFunc(input, unicode.IsSpace) diff --git a/cmd/tui/chat/input_test.go b/cmd/tui/chat/input_test.go index df7c50edc..26ddafa32 100644 --- a/cmd/tui/chat/input_test.go +++ b/cmd/tui/chat/input_test.go @@ -570,6 +570,76 @@ func TestSkillCommandsListAndPersistSyntheticToolCall(t *testing.T) { } } +func TestSkillsImportReloadsCatalogRegistryAndSystemPrompt(t *testing.T) { + before := writeTestSkillCatalog(t) + dir := t.TempDir() + if err := os.Mkdir(filepath.Join(dir, "from-codex"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "from-codex", "SKILL.md"), []byte("---\nname: from-codex\ndescription: Imported skill.\n---\nImported instructions."), 0o644); err != nil { + t.Fatal(err) + } + after, err := coreagent.DiscoverSkills(dir) + if err != nil { + t.Fatal(err) + } + registry := &coreagent.Registry{} + var reloaded, rebuilt, prompted bool + m := chatModel{ + ctx: context.Background(), + opts: Options{ + Model: "test", + Skills: before, + ImportSkills: func(source string) (coreagent.SkillImportResult, error) { + if source != "codex" { + t.Fatalf("source = %q", source) + } + return coreagent.SkillImportResult{Source: source, SourceDir: "/source", Imported: []string{"from-codex"}}, nil + }, + ReloadSkills: func() (*coreagent.SkillCatalog, error) { + reloaded = true + return after, nil + }, + ToolRegistryForModel: func(context.Context, string) *coreagent.Registry { + rebuilt = true + return registry + }, + SystemPromptForModel: func(_ context.Context, _ string, got *coreagent.Registry, _ bool) string { + prompted = got == registry + return after.SystemContext() + }, + }, + input: []rune("/skills import codex"), + } + + updated, cmd := m.handleSubmit() + if cmd != nil { + t.Fatal("skills import should not start a model run") + } + m = updated.(chatModel) + if !reloaded || !rebuilt || !prompted { + t.Fatalf("reload=%v rebuilt=%v prompted=%v", reloaded, rebuilt, prompted) + } + if m.opts.Skills != after || m.opts.Tools != registry || !strings.Contains(m.opts.SystemPrompt, "from-codex") { + t.Fatalf("reloaded options = %#v", m.opts) + } + if m.status != "skills reloaded" || len(m.entries) != 1 || !strings.Contains(m.entries[0].content, "Imported 1 skill") { + t.Fatalf("import result = status %q entries %#v", m.status, m.entries) + } +} + +func TestSkillsImportUsage(t *testing.T) { + m := chatModel{input: []rune("/skills import")} + updated, cmd := m.handleSubmit() + if cmd != nil { + t.Fatal("invalid skills import should not start a model run") + } + m = updated.(chatModel) + if len(m.entries) != 1 || m.entries[0].role != "error" || !strings.Contains(m.entries[0].content, "usage: /skills [import codex|claude|pi]") { + t.Fatalf("entries = %#v", m.entries) + } +} + func TestSkillSlashCommandPromptBecomesUserMessage(t *testing.T) { catalog := writeTestSkillCatalog(t) m := chatModel{ctx: context.Background(), opts: Options{Model: "test", Skills: catalog, Client: chatTestClient{}}, input: []rune("/release-notes draft the v1.2 notes")} @@ -665,6 +735,31 @@ func TestSkillSlashCommandAppearsInCompletions(t *testing.T) { } } +func TestSkillsImportSlashCompletions(t *testing.T) { + for _, test := range []struct { + input string + want []string + }{ + {input: "/skills", want: []string{"/skills", "/skills import"}}, + {input: "/skills impo", want: []string{"/skills import"}}, + {input: "/skills import ", want: []string{"/skills import codex", "/skills import claude", "/skills import pi"}}, + {input: "/skills import c", want: []string{"/skills import codex", "/skills import claude"}}, + {input: "/skills import pi", want: []string{"/skills import pi"}}, + } { + t.Run(test.input, func(t *testing.T) { + m := chatModel{input: []rune(test.input)} + completions := m.slashCompletions() + got := make([]string, 0, len(completions)) + for _, completion := range completions { + got = append(got, completion.value) + } + if strings.Join(got, "\n") != strings.Join(test.want, "\n") { + t.Fatalf("completions = %#v, want %#v", got, test.want) + } + }) + } +} + func TestSkillSlashPromptHidesCommandCompletions(t *testing.T) { catalog := writeTestSkillCatalog(t) for _, input := range []string{"/release-notes ", "/release-notes draft the release notes"} {