Skip to content

fix(skills): align dev skill source discovery with stable Homebrew layout #61

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
drpedapati merged 1 commit into from
Feb 23, 2026
Merged

fix(skills): align dev skill source discovery with stable Homebrew layout #61

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 90 additions & 46 deletions cmd/picoclaw/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,9 @@ func main() {

workspace := cfg.WorkspacePath()
installer := skills.NewSkillInstaller(workspace)
// 获取全局配置目录和内置 skills 目录
globalDir := filepath.Dir(getConfigPath())
globalSkillsDir := filepath.Join(globalDir, "skills")
builtinSkillsDir := filepath.Join(globalDir, "picoclaw", "skills")
builtinSkillsDir := resolveBuiltinSkillsDir(workspace)
skillsLoader := skills.NewSkillsLoader(workspace, globalSkillsDir, builtinSkillsDir)

switch subcommand {
Expand Down Expand Up @@ -840,11 +839,7 @@ func baselineSkillSourceDirs(workspace string) []string {
}

if exePath, err := os.Executable(); err == nil {
exeDir := filepath.Dir(exePath)
candidates = append(candidates,
filepath.Clean(filepath.Join(exeDir, "..", "share", "sciclaw", "skills")),
filepath.Clean(filepath.Join(exeDir, "..", "share", "picoclaw", "skills")),
)
candidates = append(candidates, skillSourceDirsForExecutable(exePath)...)
}

// User-local fallback, e.g. ~/.picoclaw/skills
Expand All @@ -865,6 +860,51 @@ func baselineSkillSourceDirs(workspace string) []string {
return dirs
}

func skillSourceDirsForExecutable(exePath string) []string {
exeDir := filepath.Dir(exePath)
shareDir := filepath.Clean(filepath.Join(exeDir, "..", "share"))

dirs := []string{
filepath.Join(shareDir, "sciclaw", "skills"),
filepath.Join(shareDir, "picoclaw", "skills"),
filepath.Join(shareDir, "sciclaw-dev", "skills"),
}

// Homebrew formula installs resources under share/<formula>/...
// e.g. sciclaw, sciclaw-dev.
formulaName := filepath.Base(filepath.Dir(filepath.Dir(shareDir)))
if formulaName != "" && formulaName != "." && formulaName != string(filepath.Separator) {
dirs = append(dirs, filepath.Join(shareDir, formulaName, "skills"))
}

return dirs
}

func resolveBuiltinSkillsDir(workspace string) string {
for _, dir := range baselineSkillSourceDirs(workspace) {
if dirHasSkillMarkdown(dir) {
return dir
}
}
return ""
}

func dirHasSkillMarkdown(dir string) bool {
entries, err := os.ReadDir(dir)
if err != nil {
return false
}
for _, entry := range entries {
if !entry.IsDir() {
continue
}
if _, err := os.Stat(filepath.Join(dir, entry.Name(), "SKILL.md")); err == nil {
return true
}
}
return false
}

func migrateCmd() {
if len(os.Args) > 2 && (os.Args[2] == "--help" || os.Args[2] == "-h") {
migrateHelp()
Expand Down Expand Up @@ -2262,26 +2302,33 @@ func skillsRemoveCmd(installer *skills.SkillInstaller, skillName string) {
}

func skillsInstallBuiltinCmd(workspace string) {
builtinSkillsDir := "./picoclaw/skills"
builtinSkillsDir := resolveBuiltinSkillsDir(workspace)
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Exclude workspace skills when resolving builtin source

skillsInstallBuiltinCmd now trusts resolveBuiltinSkillsDir(workspace), which currently scans baselineSkillSourceDirs where <cwd>/skills is the highest-priority candidate; when the command is run from the workspace root, this resolves to the same workspace/skills tree being written. In that case copyDirectory(builtinPath, workspacePath) is invoked with identical source/destination, and copyDirectory opens destination files with O_TRUNC, so existing SKILL.md content can be truncated during the self-copy. Please ensure builtin resolution excludes workspace/cwd skill directories (or guard against src == dst) before copying.

Useful? React with 👍 / 👎.

if strings.TrimSpace(builtinSkillsDir) == "" {
fmt.Println("✗ No builtin skills source detected.")
fmt.Println(" Reinstall sciclaw via Homebrew or run from a repo checkout that contains ./skills.")
return
}
workspaceSkillsDir := filepath.Join(workspace, "skills")

fmt.Printf("Copying builtin skills to workspace...\n")
fmt.Printf("Copying builtin skills to workspace from %s...\n", builtinSkillsDir)

skillsToInstall := []string{
"weather",
"news",
"stock",
"calculator",
entries, err := os.ReadDir(builtinSkillsDir)
if err != nil {
fmt.Printf("✗ Failed to read builtin skills: %v\n", err)
return
}

for _, skillName := range skillsToInstall {
installed := 0
for _, entry := range entries {
if !entry.IsDir() {
continue
}
skillName := entry.Name()
builtinPath := filepath.Join(builtinSkillsDir, skillName)
workspacePath := filepath.Join(workspaceSkillsDir, skillName)

if _, err := os.Stat(builtinPath); err != nil {
fmt.Printf("⊘ Builtin skill '%s' not found: %v\n", skillName, err)
if _, err := os.Stat(filepath.Join(builtinPath, "SKILL.md")); err != nil {
continue
}
workspacePath := filepath.Join(workspaceSkillsDir, skillName)

if err := os.MkdirAll(workspacePath, 0755); err != nil {
fmt.Printf("✗ Failed to create directory for %s: %v\n", skillName, err)
Expand All @@ -2290,10 +2337,18 @@ func skillsInstallBuiltinCmd(workspace string) {

if err := copyDirectory(builtinPath, workspacePath); err != nil {
fmt.Printf("✗ Failed to copy %s: %v\n", skillName, err)
continue
}
fmt.Printf(" ✓ %s\n", skillName)
installed++
}

if installed == 0 {
fmt.Println("⊘ No builtin skills found to install.")
return
}

fmt.Println("\n✓ All builtin skills installed!")
fmt.Printf("\n✓ Installed %d builtin skills.\n", installed)
fmt.Println("Now you can use them in your workspace.")
}

Expand All @@ -2303,10 +2358,17 @@ func skillsListBuiltinCmd() {
fmt.Printf("Error loading config: %v\n", err)
return
}
builtinSkillsDir := filepath.Join(filepath.Dir(cfg.WorkspacePath()), "picoclaw", "skills")
builtinSkillsDir := resolveBuiltinSkillsDir(cfg.WorkspacePath())
if strings.TrimSpace(builtinSkillsDir) == "" {
fmt.Println("\nAvailable Builtin Skills:")
fmt.Println("-----------------------")
fmt.Println("No builtin skills source detected.")
return
}

fmt.Println("\nAvailable Builtin Skills:")
fmt.Println("-----------------------")
fmt.Printf("Source: %s\n", builtinSkillsDir)

entries, err := os.ReadDir(builtinSkillsDir)
if err != nil {
Expand All @@ -2320,32 +2382,14 @@ func skillsListBuiltinCmd() {
}

for _, entry := range entries {
if entry.IsDir() {
skillName := entry.Name()
skillFile := filepath.Join(builtinSkillsDir, skillName, "SKILL.md")

description := "No description"
if _, err := os.Stat(skillFile); err == nil {
data, err := os.ReadFile(skillFile)
if err == nil {
content := string(data)
if idx := strings.Index(content, "\n"); idx > 0 {
firstLine := content[:idx]
if strings.Contains(firstLine, "description:") {
descLine := strings.Index(content[idx:], "\n")
if descLine > 0 {
description = strings.TrimSpace(content[idx+descLine : idx+descLine])
}
}
}
}
}
status := "✓"
fmt.Printf(" %s %s\n", status, entry.Name())
if description != "" {
fmt.Printf(" %s\n", description)
}
if !entry.IsDir() {
continue
}
skillFile := filepath.Join(builtinSkillsDir, entry.Name(), "SKILL.md")
if _, err := os.Stat(skillFile); err != nil {
continue
}
fmt.Printf(" ✓ %s\n", entry.Name())
}
}

Expand Down
47 changes: 47 additions & 0 deletions cmd/picoclaw/main_workspace_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,3 +155,50 @@ func TestEnsureBaselineScienceSkillsFromSourcesDoesNotOverwriteExistingSkill(t *
t.Fatalf("existing baseline skill was overwritten unexpectedly")
}
}

func TestSkillSourceDirsForExecutableIncludesFormulaScopedShareDir(t *testing.T) {
exePath := "/opt/homebrew/Cellar/sciclaw-dev/0.1.53-dev.26/bin/sciclaw"
dirs := skillSourceDirsForExecutable(exePath)
want := filepath.Clean("/opt/homebrew/Cellar/sciclaw-dev/0.1.53-dev.26/share/sciclaw-dev/skills")
if !sliceContainsPath(dirs, want) {
t.Fatalf("expected %q in %v", want, dirs)
}
}

func TestSkillSourceDirsForExecutableIncludesStableShareDir(t *testing.T) {
exePath := "/opt/homebrew/Cellar/sciclaw/0.1.53/bin/sciclaw"
dirs := skillSourceDirsForExecutable(exePath)
want := filepath.Clean("/opt/homebrew/Cellar/sciclaw/0.1.53/share/sciclaw/skills")
if !sliceContainsPath(dirs, want) {
t.Fatalf("expected %q in %v", want, dirs)
}
}

func TestDirHasSkillMarkdown(t *testing.T) {
root := t.TempDir()
if dirHasSkillMarkdown(root) {
t.Fatalf("expected empty dir to return false")
}

skillDir := filepath.Join(root, "example-skill")
if err := os.MkdirAll(skillDir, 0755); err != nil {
t.Fatalf("mkdir: %v", err)
}
if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("# example"), 0644); err != nil {
t.Fatalf("write SKILL.md: %v", err)
}

if !dirHasSkillMarkdown(root) {
t.Fatalf("expected dir with SKILL.md to return true")
}
}

func sliceContainsPath(paths []string, target string) bool {
cleanTarget := filepath.Clean(target)
for _, p := range paths {
if filepath.Clean(p) == cleanTarget {
return true
}
}
return false
}
Loading