From 91650469684a4b0809b786d70bf14f71d8b3374c Mon Sep 17 00:00:00 2001 From: Djordje Lukic Date: Fri, 22 May 2026 01:13:11 +0200 Subject: [PATCH] Show the path from where the skill is loaded Signed-off-by: Djordje Lukic --- pkg/path/display.go | 67 +++++++++++++++++++ pkg/path/display_test.go | 37 ++++++++++ pkg/sandbox/kit/kit.go | 22 +----- pkg/skills/local.go | 31 +++++++-- pkg/skills/skills_test.go | 12 ++++ .../tool/directorytree/directorytree.go | 3 +- pkg/tui/components/tool/editfile/editfile.go | 5 +- .../tool/listdirectory/listdirectory.go | 3 +- .../tool/listdirectory/listdirectory_test.go | 39 ----------- pkg/tui/components/tool/readfile/readfile.go | 3 +- .../readmultiplefiles/readmultiplefiles.go | 5 +- .../searchfilescontent/searchfilescontent.go | 3 +- pkg/tui/components/toolcommon/common.go | 13 ---- pkg/tui/dialog/skills.go | 32 +++++++++ pkg/tui/dialog/skills_test.go | 38 +++++++++++ 15 files changed, 229 insertions(+), 84 deletions(-) create mode 100644 pkg/path/display.go create mode 100644 pkg/path/display_test.go diff --git a/pkg/path/display.go b/pkg/path/display.go new file mode 100644 index 000000000..ad14b952d --- /dev/null +++ b/pkg/path/display.go @@ -0,0 +1,67 @@ +package path + +import ( + "os" + "path/filepath" + "strings" +) + +// RelativeTo returns p relative to baseDir when both paths are absolute and +// filepath.Rel can compute a relative path. Otherwise it returns p cleaned. +func RelativeTo(p, baseDir string) string { + if p == "" { + return p + } + + p = filepath.Clean(p) + if baseDir == "" || !filepath.IsAbs(p) || !filepath.IsAbs(baseDir) { + return p + } + + rel, err := filepath.Rel(filepath.Clean(baseDir), p) + if err != nil { + return p + } + return rel +} + +// ShortenHome replaces the current user's home directory prefix with "~". +func ShortenHome(p string) string { + homeDir, err := os.UserHomeDir() + if err != nil || homeDir == "" { + return p + } + return ShortenHomeDir(p, homeDir) +} + +// ShortenHomeDir replaces a leading homeDir prefix with "~". +func ShortenHomeDir(p, homeDir string) string { + if p == "" || homeDir == "" { + return p + } + + p = filepath.Clean(p) + homeDir = filepath.Clean(homeDir) + if !IsWithin(p, homeDir) { + return p + } + + rel, err := filepath.Rel(homeDir, p) + if err != nil { + return p + } + if rel == "." { + return "~" + } + return filepath.Join("~", rel) +} + +// IsWithin reports whether p is equal to dir or contained by dir. +func IsWithin(p, dir string) bool { + if p == "" || dir == "" { + return false + } + + rel, err := filepath.Rel(filepath.Clean(dir), filepath.Clean(p)) + return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) +} diff --git a/pkg/path/display_test.go b/pkg/path/display_test.go new file mode 100644 index 000000000..e72ee917f --- /dev/null +++ b/pkg/path/display_test.go @@ -0,0 +1,37 @@ +package path + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestRelativeTo(t *testing.T) { + t.Parallel() + base := t.TempDir() + + assert.Equal(t, filepath.Join("dir", "file.txt"), RelativeTo(filepath.Join(base, "dir", "file.txt"), base)) + assert.Equal(t, filepath.Join("..", "sibling"), RelativeTo(filepath.Join(filepath.Dir(base), "sibling"), base)) + assert.Equal(t, "relative/path", RelativeTo("relative/path", base)) + assert.Equal(t, filepath.Join(base, "file.txt"), RelativeTo(filepath.Join(base, "file.txt"), "relative/base")) +} + +func TestShortenHomeDir(t *testing.T) { + t.Parallel() + home := t.TempDir() + + assert.Equal(t, "~", ShortenHomeDir(home, home)) + assert.Equal(t, filepath.Join("~", ".agents", "skills", "commit"), ShortenHomeDir(filepath.Join(home, ".agents", "skills", "commit"), home)) + assert.Equal(t, filepath.Join(filepath.Dir(home), "elsewhere"), ShortenHomeDir(filepath.Join(filepath.Dir(home), "elsewhere"), home)) +} + +func TestIsWithin(t *testing.T) { + t.Parallel() + base := t.TempDir() + + assert.True(t, IsWithin(base, base)) + assert.True(t, IsWithin(filepath.Join(base, "child"), base)) + assert.False(t, IsWithin(filepath.Dir(base), base)) + assert.False(t, IsWithin(filepath.Join(filepath.Dir(base), filepath.Base(base)+"-suffix"), base)) +} diff --git a/pkg/sandbox/kit/kit.go b/pkg/sandbox/kit/kit.go index 3babc0029..4981f71f7 100644 --- a/pkg/sandbox/kit/kit.go +++ b/pkg/sandbox/kit/kit.go @@ -45,6 +45,7 @@ import ( "github.com/docker/docker-agent/pkg/config" latestcfg "github.com/docker/docker-agent/pkg/config/latest" "github.com/docker/docker-agent/pkg/environment" + pathx "github.com/docker/docker-agent/pkg/path" "github.com/docker/docker-agent/pkg/paths" "github.com/docker/docker-agent/pkg/promptfiles" "github.com/docker/docker-agent/pkg/skills" @@ -715,7 +716,7 @@ func (r *Result) PrintSummary(w io.Writer) { fmt.Fprintln(w, " prompt files:") for _, e := range promptEntries { name := promptFileName(e) - notes := []string{"from " + displayHostPath(e.Source)} + notes := []string{"from " + pathx.ShortenHome(e.Source)} if !e.IsStaged() { notes = append(notes, "workspace mount") } else if redacted[e.Target] { @@ -781,24 +782,7 @@ func displaySkillHeader(e Entry) string { if e.Source == "" { return name } - return fmt.Sprintf("%s (from %s)", name, displayHostPath(e.Source)) -} - -// displayHostPath replaces the user's $HOME prefix with "~" so the -// printed paths stay short and don't reveal the local username when -// shared in screenshots. -func displayHostPath(p string) string { - home, err := os.UserHomeDir() - if err != nil || home == "" { - return p - } - if strings.HasPrefix(p, home+string(filepath.Separator)) { - return "~" + p[len(home):] - } - if p == home { - return "~" - } - return p + return fmt.Sprintf("%s (from %s)", name, pathx.ShortenHome(e.Source)) } // summaryCounts formats the trailing line of PrintSummary. diff --git a/pkg/skills/local.go b/pkg/skills/local.go index e55b0a291..343188106 100644 --- a/pkg/skills/local.go +++ b/pkg/skills/local.go @@ -7,6 +7,7 @@ import ( "path/filepath" "slices" + pathx "github.com/docker/docker-agent/pkg/path" "github.com/docker/docker-agent/pkg/paths" ) @@ -51,11 +52,7 @@ func localSearchPaths() []localSearchPath { var searchPaths []localSearchPath if home := paths.GetHomeDir(); home != "" { - searchPaths = append(searchPaths, - localSearchPath{filepath.Join(home, ".codex", "skills"), true}, - localSearchPath{filepath.Join(home, ".claude", "skills"), false}, - localSearchPath{filepath.Join(home, ".agents", "skills"), true}, - ) + searchPaths = append(searchPaths, homeSkillSearchPaths(home)...) } cwd, err := os.Getwd() @@ -80,6 +77,30 @@ func loadLocalSkillsInto(skillMap map[string]Skill) { } } +// IsHomeSkillPath reports whether path is under one of the global skill +// directories in the user's home directory. +func IsHomeSkillPath(path string) bool { + home := paths.GetHomeDir() + if home == "" { + return false + } + + for _, p := range homeSkillSearchPaths(home) { + if pathx.IsWithin(path, p.dir) { + return true + } + } + return false +} + +func homeSkillSearchPaths(home string) []localSearchPath { + return []localSearchPath{ + {filepath.Join(home, ".codex", "skills"), true}, + {filepath.Join(home, ".claude", "skills"), false}, + {filepath.Join(home, ".agents", "skills"), true}, + } +} + // projectSearchDirs returns directories from the enclosing git root down to // cwd (inclusive), ordered from root to cwd. If cwd is not inside a git // repository, it returns just cwd. diff --git a/pkg/skills/skills_test.go b/pkg/skills/skills_test.go index ac5b86f8d..4bbd42c6a 100644 --- a/pkg/skills/skills_test.go +++ b/pkg/skills/skills_test.go @@ -754,6 +754,18 @@ func TestLoad_KitDirOverridesEverything(t *testing.T) { assert.NotContains(t, names, "host-only", "host paths must be ignored when a kit is set") } +func TestIsHomeSkillPath(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + + assert.True(t, IsHomeSkillPath(filepath.Join(home, ".codex", "skills", "skill-a"))) + assert.True(t, IsHomeSkillPath(filepath.Join(home, ".claude", "skills", "skill-b"))) + assert.True(t, IsHomeSkillPath(filepath.Join(home, ".agents", "skills", "skill-c"))) + assert.False(t, IsHomeSkillPath(filepath.Join(home, "work", "repo", ".agents", "skills", "project-skill"))) + assert.False(t, IsHomeSkillPath(filepath.Join(filepath.Dir(home), "elsewhere", "skill"))) +} + func TestSkill_IsFork(t *testing.T) { assert.True(t, (&Skill{Context: "fork"}).IsFork()) assert.False(t, (&Skill{Context: ""}).IsFork()) diff --git a/pkg/tui/components/tool/directorytree/directorytree.go b/pkg/tui/components/tool/directorytree/directorytree.go index 69e0934a0..1be65fabb 100644 --- a/pkg/tui/components/tool/directorytree/directorytree.go +++ b/pkg/tui/components/tool/directorytree/directorytree.go @@ -3,6 +3,7 @@ package directorytree import ( "strings" + pathx "github.com/docker/docker-agent/pkg/path" "github.com/docker/docker-agent/pkg/tools/builtin/filesystem" "github.com/docker/docker-agent/pkg/tui/components/toolcommon" "github.com/docker/docker-agent/pkg/tui/core/layout" @@ -12,7 +13,7 @@ import ( func New(msg *types.Message, sessionState service.SessionStateReader) layout.Model { return toolcommon.NewBase(msg, sessionState, toolcommon.SimpleRendererWithResult( - toolcommon.ExtractField(func(a filesystem.DirectoryTreeArgs) string { return toolcommon.ShortenPath(a.Path) }), + toolcommon.ExtractField(func(a filesystem.DirectoryTreeArgs) string { return pathx.ShortenHome(a.Path) }), extractResult, )) } diff --git a/pkg/tui/components/tool/editfile/editfile.go b/pkg/tui/components/tool/editfile/editfile.go index c5440f370..f3801158c 100644 --- a/pkg/tui/components/tool/editfile/editfile.go +++ b/pkg/tui/components/tool/editfile/editfile.go @@ -3,6 +3,7 @@ package editfile import ( "fmt" + pathx "github.com/docker/docker-agent/pkg/path" "github.com/docker/docker-agent/pkg/tools/builtin/filesystem" "github.com/docker/docker-agent/pkg/tui/components/spinner" "github.com/docker/docker-agent/pkg/tui/components/toolcommon" @@ -72,7 +73,7 @@ func render( "%s%s %s", toolcommon.Icon(msg, s), styles.ToolName.Render(msg.ToolDefinition.DisplayName()), - styles.ToolMessageStyle.Render(toolcommon.ShortenPath(args.Path)), + styles.ToolMessageStyle.Render(pathx.ShortenHome(args.Path)), ) } @@ -142,7 +143,7 @@ func renderCollapsed( "%s%s %s%s", toolcommon.Icon(msg, s), styles.ToolName.Render(msg.ToolDefinition.DisplayName()), - styles.ToolMessageStyle.Render(toolcommon.ShortenPath(args.Path)), + styles.ToolMessageStyle.Render(pathx.ShortenHome(args.Path)), diffSummary, ) diff --git a/pkg/tui/components/tool/listdirectory/listdirectory.go b/pkg/tui/components/tool/listdirectory/listdirectory.go index f0bd47ef7..c9432c4d4 100644 --- a/pkg/tui/components/tool/listdirectory/listdirectory.go +++ b/pkg/tui/components/tool/listdirectory/listdirectory.go @@ -3,6 +3,7 @@ package listdirectory import ( "strings" + pathx "github.com/docker/docker-agent/pkg/path" "github.com/docker/docker-agent/pkg/tools/builtin/filesystem" "github.com/docker/docker-agent/pkg/tui/components/toolcommon" "github.com/docker/docker-agent/pkg/tui/core/layout" @@ -12,7 +13,7 @@ import ( func New(msg *types.Message, sessionState service.SessionStateReader) layout.Model { return toolcommon.NewBase(msg, sessionState, toolcommon.SimpleRendererWithResult( - toolcommon.ExtractField(func(a filesystem.ListDirectoryArgs) string { return toolcommon.ShortenPath(a.Path) }), + toolcommon.ExtractField(func(a filesystem.ListDirectoryArgs) string { return pathx.ShortenHome(a.Path) }), extractResult, )) } diff --git a/pkg/tui/components/tool/listdirectory/listdirectory_test.go b/pkg/tui/components/tool/listdirectory/listdirectory_test.go index 61b8a28da..994c561ee 100644 --- a/pkg/tui/components/tool/listdirectory/listdirectory_test.go +++ b/pkg/tui/components/tool/listdirectory/listdirectory_test.go @@ -5,7 +5,6 @@ import ( "github.com/docker/docker-agent/pkg/tools" "github.com/docker/docker-agent/pkg/tools/builtin/filesystem" - "github.com/docker/docker-agent/pkg/tui/components/toolcommon" "github.com/docker/docker-agent/pkg/tui/types" ) @@ -70,41 +69,3 @@ func TestExtractResult(t *testing.T) { }) } } - -func TestShortenPath(t *testing.T) { - tests := []struct { - name string - path string - expected string - }{ - { - name: "empty path", - path: "", - expected: "", - }, - { - name: "current directory", - path: ".", - expected: ".", - }, - { - name: "absolute path", - path: "/usr/local/bin", - expected: "/usr/local/bin", - }, - { - name: "relative path", - path: "src/components", - expected: "src/components", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := toolcommon.ShortenPath(tt.path) - if result != tt.expected { - t.Errorf("ShortenPath(%q) = %q, want %q", tt.path, result, tt.expected) - } - }) - } -} diff --git a/pkg/tui/components/tool/readfile/readfile.go b/pkg/tui/components/tool/readfile/readfile.go index ae0e728b4..e277a4da7 100644 --- a/pkg/tui/components/tool/readfile/readfile.go +++ b/pkg/tui/components/tool/readfile/readfile.go @@ -3,6 +3,7 @@ package readfile import ( "fmt" + pathx "github.com/docker/docker-agent/pkg/path" "github.com/docker/docker-agent/pkg/tools/builtin/filesystem" "github.com/docker/docker-agent/pkg/tui/components/toolcommon" "github.com/docker/docker-agent/pkg/tui/core/layout" @@ -12,7 +13,7 @@ import ( func New(msg *types.Message, sessionState service.SessionStateReader) layout.Model { return toolcommon.NewBase(msg, sessionState, toolcommon.SimpleRendererWithResult( - toolcommon.ExtractField(func(a filesystem.ReadFileArgs) string { return toolcommon.ShortenPath(a.Path) }), + toolcommon.ExtractField(func(a filesystem.ReadFileArgs) string { return pathx.ShortenHome(a.Path) }), extractResult, )) } diff --git a/pkg/tui/components/tool/readmultiplefiles/readmultiplefiles.go b/pkg/tui/components/tool/readmultiplefiles/readmultiplefiles.go index 1f3c32f80..82d2a1ec4 100644 --- a/pkg/tui/components/tool/readmultiplefiles/readmultiplefiles.go +++ b/pkg/tui/components/tool/readmultiplefiles/readmultiplefiles.go @@ -7,6 +7,7 @@ import ( "charm.land/lipgloss/v2" + pathx "github.com/docker/docker-agent/pkg/path" "github.com/docker/docker-agent/pkg/tools/builtin/filesystem" "github.com/docker/docker-agent/pkg/tui/components/spinner" "github.com/docker/docker-agent/pkg/tui/components/toolcommon" @@ -94,7 +95,7 @@ func formatSummaryLines(meta *filesystem.ReadMultipleFilesMeta) []fileSummary { var summaries []fileSummary for _, file := range meta.Files { - path := toolcommon.ShortenPath(file.Path) + path := pathx.ShortenHome(file.Path) var output string if file.Error != "" { output = " " + file.Error @@ -120,7 +121,7 @@ func formatFilesList(filePaths []string) string { shortened := make([]string, len(filePaths)) for i, p := range filePaths { - shortened[i] = toolcommon.ShortenPath(p) + shortened[i] = pathx.ShortenHome(p) } if len(shortened) == 1 { diff --git a/pkg/tui/components/tool/searchfilescontent/searchfilescontent.go b/pkg/tui/components/tool/searchfilescontent/searchfilescontent.go index 26ed0dca9..d99e9c8db 100644 --- a/pkg/tui/components/tool/searchfilescontent/searchfilescontent.go +++ b/pkg/tui/components/tool/searchfilescontent/searchfilescontent.go @@ -3,6 +3,7 @@ package searchfilescontent import ( "fmt" + pathx "github.com/docker/docker-agent/pkg/path" "github.com/docker/docker-agent/pkg/tools/builtin/filesystem" "github.com/docker/docker-agent/pkg/tui/components/toolcommon" "github.com/docker/docker-agent/pkg/tui/core/layout" @@ -23,7 +24,7 @@ func extractArgs(args string) string { return "" } - path := toolcommon.ShortenPath(parsed.Path) + path := pathx.ShortenHome(parsed.Path) query := parsed.Query if len(query) > 30 { query = query[:27] + "..." diff --git a/pkg/tui/components/toolcommon/common.go b/pkg/tui/components/toolcommon/common.go index e40538dba..1e5e110e8 100644 --- a/pkg/tui/components/toolcommon/common.go +++ b/pkg/tui/components/toolcommon/common.go @@ -9,7 +9,6 @@ import ( "charm.land/lipgloss/v2" - "github.com/docker/docker-agent/pkg/paths" "github.com/docker/docker-agent/pkg/tools" "github.com/docker/docker-agent/pkg/tui/components/spinner" "github.com/docker/docker-agent/pkg/tui/styles" @@ -253,18 +252,6 @@ func RenderTool(msg *types.Message, inProgress spinner.Spinner, args, result str return styles.RenderComposite(styles.ToolMessageStyle.Width(width), content) } -// ShortenPath replaces home directory with ~ for cleaner display. -func ShortenPath(path string) string { - if path == "" { - return path - } - homeDir := paths.GetHomeDir() - if homeDir != "" && strings.HasPrefix(path, homeDir) { - return "~" + strings.TrimPrefix(path, homeDir) - } - return path -} - // RenderFriendlyHeader renders a friendly description header if present in the tool call arguments. // Returns the rendered header string and true if a friendly description was found, empty string and false otherwise. // Custom renderers can use this to show the friendly description before their custom content. diff --git a/pkg/tui/dialog/skills.go b/pkg/tui/dialog/skills.go index 58a69086f..febab20d2 100644 --- a/pkg/tui/dialog/skills.go +++ b/pkg/tui/dialog/skills.go @@ -2,10 +2,12 @@ package dialog import ( "fmt" + "os" "strings" "charm.land/lipgloss/v2" + pathx "github.com/docker/docker-agent/pkg/path" "github.com/docker/docker-agent/pkg/skills" "github.com/docker/docker-agent/pkg/tui/components/toolcommon" "github.com/docker/docker-agent/pkg/tui/styles" @@ -66,10 +68,40 @@ func formatSkill(s *skills.Skill, contentWidth int) []string { out = append(out, indent+styles.MutedStyle.Render(toolcommon.TruncateText(desc, availableWidth))) } } + + if path := skillLoadedFrom(s); path != "" { + indent := " " + prefix := "from: " + availableWidth := contentWidth - lipgloss.Width(indent) - lipgloss.Width(prefix) + if availableWidth > 0 { + out = append(out, indent+styles.MutedStyle.Render(prefix+toolcommon.TruncateText(path, availableWidth))) + } + } + out = append(out, "") return out } +func skillLoadedFrom(s *skills.Skill) string { + path := s.BaseDir + if path == "" { + path = s.FilePath + } + if path == "" { + return "" + } + + if s.Local { + if skills.IsHomeSkillPath(path) { + return pathx.ShortenHome(path) + } + cwd, _ := os.Getwd() + return pathx.RelativeTo(path, cwd) + } + + return pathx.ShortenHome(path) +} + func skillSourceBadge(s *skills.Skill) string { if s.Local { return styles.SuccessStyle.Render("[local]") diff --git a/pkg/tui/dialog/skills_test.go b/pkg/tui/dialog/skills_test.go index f667f264a..ee34fa8c1 100644 --- a/pkg/tui/dialog/skills_test.go +++ b/pkg/tui/dialog/skills_test.go @@ -1,10 +1,13 @@ package dialog import ( + "os" + "path/filepath" "strings" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/docker/docker-agent/pkg/skills" ) @@ -23,12 +26,14 @@ func TestNewSkillsDialog_RendersSkills(t *testing.T) { { Name: "commit", Description: "Commit local changes", + BaseDir: "skills/commit", Local: true, Context: "fork", }, { Name: "poem", Description: "Prints a poem", + BaseDir: "cache/skills/poem", Local: false, }, } @@ -37,9 +42,42 @@ func TestNewSkillsDialog_RendersSkills(t *testing.T) { assert.Contains(t, out, "Skills (2)") assert.Contains(t, out, "commit") assert.Contains(t, out, "Commit local changes") + assert.Contains(t, out, "from: skills/commit") assert.Contains(t, out, "local") assert.Contains(t, out, "fork") assert.Contains(t, out, "poem") assert.Contains(t, out, "Prints a poem") + assert.Contains(t, out, "from: cache/skills/poem") assert.Contains(t, out, "remote") } + +func TestSkillLoadedFrom_LocalHomeSkillUsesTilde(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + + skill := &skills.Skill{ + BaseDir: filepath.Join(home, ".agents", "skills", "commit"), + Local: true, + } + + assert.Equal(t, filepath.Join("~", ".agents", "skills", "commit"), skillLoadedFrom(skill)) +} + +func TestSkillLoadedFrom_LocalProjectSkillUsesRelativePath(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + + repo := t.TempDir() + cwd := filepath.Join(repo, "subdir") + require.NoError(t, os.MkdirAll(cwd, 0o755)) + t.Chdir(cwd) + + skill := &skills.Skill{ + BaseDir: filepath.Join(repo, ".agents", "skills", "commit"), + Local: true, + } + + assert.Equal(t, filepath.Join("..", ".agents", "skills", "commit"), skillLoadedFrom(skill)) +}