fix skills registry install/search validation and github URLs

cmd/picoclaw/internal/skills/helpers.go

@@ -104,6 +104,11 @@ func skillsInstallFromRegistry(cfg *config.Config, registryName, target string)
 		fmt.Printf("\u26a0\ufe0f  Warning: skill '%s' is flagged as suspicious.\n", target)
 	}
 
+	if !workspaceHasValidSkillDirectory(workspace, dirName) {
+		_ = os.RemoveAll(targetDir)
+		return fmt.Errorf("✗ failed to install skill: registry archive for %q is not a valid skill", target)
+	}
+
 	normalizedSlug := skills.NormalizeInstallTargetForRegistry(cfg.Tools.Skills, registry.Name(), target)
 	installedAt := time.Now().UnixMilli()
 	if err := writeInstalledSkillOriginMeta(targetDir, installedSkillOriginMeta{
@@ -135,6 +140,19 @@ func writeInstalledSkillOriginMeta(targetDir string, meta installedSkillOriginMe
 	return fileutil.WriteFileAtomic(filepath.Join(targetDir, ".skill-origin.json"), data, 0o600)
 }
 
+func workspaceHasValidSkillDirectory(workspace, directory string) bool {
+	loader := skills.NewSkillsLoader(workspace, "", "")
+	for _, skill := range loader.ListSkills() {
+		if skill.Source != "workspace" {
+			continue
+		}
+		if filepath.Base(filepath.Dir(skill.Path)) == directory {
+			return true
+		}
+	}
+	return false
+}
+
 func skillsRemoveFromWorkspace(workspace, skillName string) error {
 	name := strings.TrimSpace(skillName)
 	name = strings.Trim(name, "/")

cmd/picoclaw/internal/skills/helpers_test.go

@@ -60,3 +60,40 @@ func TestSkillsInstallFromRegistryWritesOriginMetadata(t *testing.T) {
 	assert.Equal(t, "master", meta.InstalledVersion)
 	assert.NotZero(t, meta.InstalledAt)
 }
+
+func TestSkillsInstallFromRegistryRejectsInvalidSkillArchive(t *testing.T) {
+	workspace := t.TempDir()
+	cfg := config.DefaultConfig()
+	cfg.Agents.Defaults.Workspace = workspace
+
+	var server *httptest.Server
+	server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/api/v3/repos/foo/bar":
+			require.NoError(t, json.NewEncoder(w).Encode(map[string]any{"default_branch": "master"}))
+		case "/api/v3/repos/foo/bar/contents/.agents/skills/pr-review":
+			require.NoError(t, json.NewEncoder(w).Encode([]map[string]any{{
+				"type":         "file",
+				"name":         "SKILL.md",
+				"download_url": server.URL + "/raw/foo/bar/master/.agents/skills/pr-review/SKILL.md",
+			}}))
+		case "/raw/foo/bar/master/.agents/skills/pr-review/SKILL.md":
+			_, _ = w.Write([]byte("---\nname: bad_skill\ndescription: Invalid skill name\n---\n# Invalid\n"))
+		default:
+			http.NotFound(w, r)
+		}
+	}))
+	defer server.Close()
+
+	githubRegistry, ok := cfg.Tools.Skills.Registries.Get("github")
+	require.True(t, ok)
+	githubRegistry.BaseURL = server.URL
+	cfg.Tools.Skills.Registries.Set("github", githubRegistry)
+
+	target := server.URL + "/foo/bar/tree/master/.agents/skills/pr-review"
+	err := skillsInstallFromRegistry(cfg, "github", target)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "is not a valid skill")
+	_, statErr := os.Stat(filepath.Join(workspace, "skills", "pr-review"))
+	assert.True(t, os.IsNotExist(statErr))
+}

pkg/skills/github_registry.go

@@ -93,10 +93,10 @@ func (r *GitHubRegistry) SkillURL(target, version string) string {
 		return fmt.Sprintf("%s/%s", base, urlPath)
 	}
 	if ref.SubPath != "" {
-		return fmt.Sprintf("%s/%s/tree/%s/%s", base, urlPath, url.PathEscape(ref.Ref), ref.SubPath)
+		return fmt.Sprintf("%s/%s/tree/%s/%s", base, urlPath, ref.Ref, ref.SubPath)
 	}
 	if ref.Ref != "main" {
-		return fmt.Sprintf("%s/%s/tree/%s", base, urlPath, url.PathEscape(ref.Ref))
+		return fmt.Sprintf("%s/%s/tree/%s", base, urlPath, ref.Ref)
 	}
 	return fmt.Sprintf("%s/%s", base, urlPath)
 }

pkg/skills/github_registry_test.go

@@ -171,6 +171,11 @@ func TestGitHubRegistrySkillURLUsesProvidedVersionAndBasePath(t *testing.T) {
 		"https://ghe.example.com/git/org/repo/tree/dev/skills/pr-review",
 		registry.SkillURL("https://ghe.example.com/git/org/repo/tree/dev/skills/pr-review", ""),
 	)
+	assert.Equal(
+		t,
+		"https://ghe.example.com/git/org/repo/tree/feature/skills-registry/skills/pr-review",
+		registry.SkillURL("org/repo/skills/pr-review", "feature/skills-registry"),
+	)
 }
 
 func TestGitHubRegistryResolveInstallDirNameSupportsFullURLs(t *testing.T) {

web/backend/api/skills.go

@@ -242,6 +242,15 @@ func (h *Handler) handleSearchSkills(w http.ResponseWriter, r *http.Request) {
 	response := make([]skillSearchResultItem, 0, len(pageResults))
 	for _, result := range pageResults {
 		installedSkill, installed := installedSkills[result.Slug]
+		if !installed {
+			registry := registryMgr.GetRegistry(result.RegistryName)
+			if registry != nil {
+				dirName, err := registry.ResolveInstallDirName(result.Slug)
+				if err == nil {
+					installedSkill, installed = installedSkills[dirName]
+				}
+			}
+		}
 		item := skillSearchResultItem{
 			Score:        result.Score,
 			Slug:         result.Slug,
@@ -569,18 +578,20 @@ func buildOccupiedWorkspaceSkillsByDirectory(cfg *config.Config) (map[string]ski
 			continue
 		}
 
-		key := filepath.Base(filepath.Dir(skill.Path))
+		dirName := filepath.Base(filepath.Dir(skill.Path))
+		if dirName != "" {
+			result[dirName] = skill
+		}
 		if meta, err := readInstalledSkillOriginMeta(skill.Path); err == nil && meta != nil && meta.Slug != "" {
-			key = skills.NormalizeInstallTargetForRegistry(cfg.Tools.Skills, meta.Registry, meta.Slug)
+			key := skills.NormalizeInstallTargetForRegistry(cfg.Tools.Skills, meta.Registry, meta.Slug)
 			if key == "" {
 				key = meta.Slug
 			}
-		}
-		if key == "" {
-			continue
-		}
+			if key != "" {
 				result[key] = skill
 			}
+		}
+	}
 	return result, nil
 }
 

web/backend/api/skills_test.go

@@ -1311,6 +1311,89 @@ func TestHandleInstallSkillTracksGitHubURLInstallsAsInstalled(t *testing.T) {
 	}
 }
 
+func TestHandleSearchSkillsMarksDirectoryCollisionAsInstalled(t *testing.T) {
+	configPath, cleanup := setupOAuthTestEnv(t)
+	defer cleanup()
+
+	cfg, loadErr := config.LoadConfig(configPath)
+	if loadErr != nil {
+		t.Fatalf("LoadConfig() error = %v", loadErr)
+	}
+	workspace := filepath.Join(t.TempDir(), "workspace")
+	cfg.Agents.Defaults.Workspace = workspace
+
+	skillDir := filepath.Join(workspace, "skills", "pr-review")
+	if err := os.MkdirAll(skillDir, 0o755); err != nil {
+		t.Fatalf("MkdirAll() error = %v", err)
+	}
+	if err := os.WriteFile(
+		filepath.Join(skillDir, "SKILL.md"),
+		[]byte("---\nname: pr-review\ndescription: Workspace PR review skill\n---\n# PR Review\n"),
+		0o644,
+	); err != nil {
+		t.Fatalf("WriteFile(SKILL.md) error = %v", err)
+	}
+	if err := writeSkillOriginMeta(skillDir, installedSkillOriginMeta{
+		Version:          1,
+		OriginKind:       "third_party",
+		Registry:         "github",
+		Slug:             "foo/bar/.agents/skills/pr-review",
+		RegistryURL:      "https://github.com/foo/bar/tree/master/.agents/skills/pr-review",
+		InstalledVersion: "master",
+		InstalledAt:      time.Now().UnixMilli(),
+	}); err != nil {
+		t.Fatalf("writeSkillOriginMeta() error = %v", err)
+	}
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/api/v1/search":
+			json.NewEncoder(w).Encode(map[string]any{
+				"results": []map[string]any{{
+					"slug":        "pr-review",
+					"displayName": "PR Review",
+					"summary":     "ClawHub PR review skill",
+					"version":     "1.2.3",
+				}},
+			})
+		default:
+			http.NotFound(w, r)
+		}
+	}))
+	defer server.Close()
+
+	setClawHubBaseURL(cfg, server.URL)
+	githubRegistry, _ := cfg.Tools.Skills.Registries.Get("github")
+	githubRegistry.Enabled = false
+	cfg.Tools.Skills.Registries.Set("github", githubRegistry)
+	if saveErr := config.SaveConfig(configPath, cfg); saveErr != nil {
+		t.Fatalf("SaveConfig() error = %v", saveErr)
+	}
+
+	h := NewHandler(configPath)
+	mux := http.NewServeMux()
+	h.RegisterRoutes(mux)
+
+	rec := httptest.NewRecorder()
+	req := httptest.NewRequest(http.MethodGet, "/api/skills/search?q=pr+review&limit=5", nil)
+	mux.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusOK {
+		t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String())
+	}
+
+	var resp skillSearchResponse
+	if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("Unmarshal() error = %v", err)
+	}
+	if len(resp.Results) != 1 {
+		t.Fatalf("results count = %d, want 1", len(resp.Results))
+	}
+	if !resp.Results[0].Installed || resp.Results[0].InstalledName != "pr-review" {
+		t.Fatalf("search result should be treated as installed when directory is occupied, got %#v", resp.Results[0])
+	}
+}
+
 func TestHandleInstallSkillRollsBackOnOriginMetadataWriteFailure(t *testing.T) {
 	configPath, cleanup := setupOAuthTestEnv(t)
 	defer cleanup()