fix(skills): use DB file_path for directory resolution + list archived skills (#322)

* fix(skills): use DB file_path for skill directory resolution + list archived skills

Two fixes:

1. Skills store methods (ListSkills, ListAllSkills, GetSkill, GetSkillByID,
   LoadSkill) constructed BaseDir from s.baseDir + slug + version, ignoring
   the file_path column stored in the database. When GOCLAW_DATA_DIR differs
   from the directory used during initial skill seeding, this caused
   dependency rescans to look in the wrong directory, find no scripts, and
   silently skip — leaving missing_deps stale in the database even after
   successful pip/npm installs.

   Fix: buildSkillInfo now accepts an optional filePath parameter from the
   DB and uses it as the authoritative BaseDir when set.

2. ListSkills only returned skills with status='active', hiding archived
   skills from the admin UI. Skills with missing dependencies are archived
   on seed, making them invisible — admins couldn't see them to install
   deps and reactivate. Now includes archived skills in the listing.

* fix(skills): use 'deleted' status for soft-delete, distinct from 'archived'

DeleteSkill previously set status='archived', which is also used for skills
with missing dependencies. Now that ListSkills includes archived skills
(so admins can see and fix missing deps), soft-deleted skills were
reappearing after page refresh.

Use status='deleted' for user-deleted skills to keep them hidden while
allowing archived (missing deps) skills to remain visible in the admin UI.

* fix(skills): complete file_path resolution + exclude deleted from rescan

- ListAllSkills: add `status != 'deleted'` to prevent rescan from
  resurrecting user-deleted skills
- ListAccessible: add file_path to SELECT so runtime skill resolution
  uses DB path instead of stale baseDir
- SearchByEmbedding: use DB file_path when available for path resolution
- CreateSkillManaged: ON CONFLICT visibility reset now matches both
  'archived' and 'deleted' statuses
- Update delete result message and status comment

---------

Co-authored-by: Luvu182 <208665161+Luvu182@users.noreply.github.com>
Co-authored-by: viettranx <viettranx@gmail.com>

internal/store/pg/skills.go

@@ -62,9 +62,10 @@ func (s *PGSkillStore) ListSkills() []store.SkillInfo {
 	s.mu.RUnlock()
 
 	// Cache miss or TTL expired → query DB
-	// Returns active + system skills (and disabled ones — admin UI needs to see them to toggle back).
+	// Returns active + archived + system skills. Archived skills are shown dimmed in the UI
+	// so admins can see missing deps and re-activate after installing them.
 	rows, err := s.db.Query(
-		`SELECT id, name, slug, description, visibility, tags, version, is_system, status, enabled, deps, frontmatter FROM skills WHERE status = 'active' OR is_system = true ORDER BY name`)
+		`SELECT id, name, slug, description, visibility, tags, version, is_system, status, enabled, deps, frontmatter, file_path FROM skills WHERE status IN ('active', 'archived') OR is_system = true ORDER BY name`)
 	if err != nil {
 		return nil
 	}
@@ -79,10 +80,11 @@ func (s *PGSkillStore) ListSkills() []store.SkillInfo {
 		var version int
 		var isSystem, enabled bool
 		var depsRaw, fmRaw []byte
-		if err := rows.Scan(&id, &name, &slug, &desc, &visibility, pq.Array(&tags), &version, &isSystem, &status, &enabled, &depsRaw, &fmRaw); err != nil {
+		var filePath *string
+		if err := rows.Scan(&id, &name, &slug, &desc, &visibility, pq.Array(&tags), &version, &isSystem, &status, &enabled, &depsRaw, &fmRaw, &filePath); err != nil {
 			continue
 		}
-		info := buildSkillInfo(id.String(), name, slug, desc, version, s.baseDir)
+		info := buildSkillInfo(id.String(), name, slug, desc, version, s.baseDir, filePath)
 		info.Visibility = visibility
 		info.Tags = tags
 		info.IsSystem = isSystem
@@ -110,7 +112,7 @@ func (s *PGSkillStore) ListSkills() []store.SkillInfo {
 // Disabled skills are excluded — no point scanning or updating them.
 func (s *PGSkillStore) ListAllSkills() []store.SkillInfo {
 	rows, err := s.db.Query(
-		`SELECT id, name, slug, description, visibility, tags, version, is_system, status, enabled, deps FROM skills WHERE enabled = true ORDER BY name`)
+		`SELECT id, name, slug, description, visibility, tags, version, is_system, status, enabled, deps, file_path FROM skills WHERE enabled = true AND status != 'deleted' ORDER BY name`)
 	if err != nil {
 		return nil
 	}
@@ -125,10 +127,11 @@ func (s *PGSkillStore) ListAllSkills() []store.SkillInfo {
 		var version int
 		var isSystem, enabled bool
 		var depsRaw []byte
-		if err := rows.Scan(&id, &name, &slug, &desc, &visibility, pq.Array(&tags), &version, &isSystem, &status, &enabled, &depsRaw); err != nil {
+		var filePath *string
+		if err := rows.Scan(&id, &name, &slug, &desc, &visibility, pq.Array(&tags), &version, &isSystem, &status, &enabled, &depsRaw, &filePath); err != nil {
 			continue
 		}
-		info := buildSkillInfo(id.String(), name, slug, desc, version, s.baseDir)
+		info := buildSkillInfo(id.String(), name, slug, desc, version, s.baseDir, filePath)
 		info.Visibility = visibility
 		info.Tags = tags
 		info.IsSystem = isSystem

internal/store/pg/skills_content.go

@@ -18,13 +18,15 @@ var skillFrontmatterRe = regexp.MustCompile(`(?s)^---\n(.*?)\n---\n?`)
 func (s *PGSkillStore) LoadSkill(name string) (string, bool) {
 	var slug string
 	var version int
+	var filePath *string
 	err := s.db.QueryRow(
-		"SELECT slug, version FROM skills WHERE slug = $1 AND status = 'active'", name,
-	).Scan(&slug, &version)
+		"SELECT slug, version, file_path FROM skills WHERE slug = $1 AND status = 'active'", name,
+	).Scan(&slug, &version, &filePath)
 	if err != nil {
 		return "", false
 	}
-	content, err := readSkillContent(s.baseDir, slug, version)
+	info := buildSkillInfo("", "", slug, nil, version, s.baseDir, filePath)
+	content, err := readSkillFile(info.Path)
 	if err != nil {
 		return "", false
 	}
@@ -83,13 +85,14 @@ func (s *PGSkillStore) GetSkill(name string) (*store.SkillInfo, bool) {
 	var tags []string
 	var version int
 	var isSystem bool
+	var filePath *string
 	err := s.db.QueryRow(
-		"SELECT id, name, slug, description, visibility, tags, version, is_system FROM skills WHERE slug = $1 AND status = 'active'", name,
-	).Scan(&id, &skillName, &slug, &desc, &visibility, pq.Array(&tags), &version, &isSystem)
+		"SELECT id, name, slug, description, visibility, tags, version, is_system, file_path FROM skills WHERE slug = $1 AND status = 'active'", name,
+	).Scan(&id, &skillName, &slug, &desc, &visibility, pq.Array(&tags), &version, &isSystem, &filePath)
 	if err != nil {
 		return nil, false
 	}
-	info := buildSkillInfo(id.String(), skillName, slug, desc, version, s.baseDir)
+	info := buildSkillInfo(id.String(), skillName, slug, desc, version, s.baseDir, filePath)
 	info.Visibility = visibility
 	info.Tags = tags
 	info.IsSystem = isSystem
@@ -132,15 +135,16 @@ func (s *PGSkillStore) GetSkillByID(id uuid.UUID) (store.SkillInfo, bool) {
 	var version int
 	var isSystem, enabled bool
 	var depsRaw []byte
+	var filePath *string
 	err := s.db.QueryRow(
-		`SELECT name, slug, description, visibility, tags, version, is_system, status, enabled, deps
+		`SELECT name, slug, description, visibility, tags, version, is_system, status, enabled, deps, file_path
 		 FROM skills WHERE id = $1`,
 		id,
-	).Scan(&name, &slug, &desc, &visibility, pq.Array(&tags), &version, &isSystem, &status, &enabled, &depsRaw)
+	).Scan(&name, &slug, &desc, &visibility, pq.Array(&tags), &version, &isSystem, &status, &enabled, &depsRaw, &filePath)
 	if err != nil {
 		return store.SkillInfo{}, false
 	}
-	info := buildSkillInfo(id.String(), name, slug, desc, version, s.baseDir)
+	info := buildSkillInfo(id.String(), name, slug, desc, version, s.baseDir, filePath)
 	info.Visibility = visibility
 	info.Tags = tags
 	info.IsSystem = isSystem
@@ -172,30 +176,36 @@ func (s *PGSkillStore) GetSkillOwnerIDBySlug(slug string) (string, bool) {
 	return ownerID, true
 }
 
-func buildSkillInfo(id, name, slug string, desc *string, version int, baseDir string) store.SkillInfo {
+// buildSkillInfo constructs a SkillInfo. If filePath (from DB file_path column) is set,
+// it is used as BaseDir; otherwise BaseDir is constructed from baseDir/slug/version.
+// This ensures skills seeded under a previous data directory are still resolved correctly.
+func buildSkillInfo(id, name, slug string, desc *string, version int, baseDir string, filePath *string) store.SkillInfo {
 	d := ""
 	if desc != nil {
 		d = *desc
 	}
+	skillDir := fmt.Sprintf("%s/%s/%d", baseDir, slug, version)
+	if filePath != nil && *filePath != "" {
+		skillDir = *filePath
+	}
 	return store.SkillInfo{
 		ID:          id,
 		Name:        name,
 		Slug:        slug,
-		Path:        fmt.Sprintf("%s/%s/%d/SKILL.md", baseDir, slug, version),
-		BaseDir:     fmt.Sprintf("%s/%s/%d", baseDir, slug, version),
+		Path:        skillDir + "/SKILL.md",
+		BaseDir:     skillDir,
 		Source:      "managed",
 		Description: d,
 		Version:     version,
 	}
 }
 
-func readSkillContent(baseDir, slug string, version int) (string, error) {
-	path := fmt.Sprintf("%s/%s/%d/SKILL.md", baseDir, slug, version)
+// readSkillFile reads a SKILL.md file, normalizes line endings, and strips frontmatter.
+func readSkillFile(path string) (string, error) {
 	data, err := os.ReadFile(path)
 	if err != nil {
 		return "", err
 	}
-	// Normalize line endings (Windows CRLF → LF) and strip frontmatter
 	content := strings.ReplaceAll(string(data), "\r\n", "\n")
 	content = strings.ReplaceAll(content, "\r", "\n")
 	content = skillFrontmatterRe.ReplaceAllString(content, "")

internal/store/pg/skills_crud.go

@@ -18,7 +18,7 @@ type SkillCreateParams struct {
 	Description *string
 	OwnerID     string
 	Visibility  string
-	Status      string // "active" or "archived" (defaults to "active" if empty)
+	Status      string // "active", "archived" (missing deps), or "deleted" (user-deleted)
 	Version     int
 	FilePath    string
 	FileSize    int64
@@ -73,9 +73,9 @@ func (s *PGSkillStore) DeleteSkill(id uuid.UUID) error {
 		return fmt.Errorf("delete skill user grants: %w", err)
 	}
 
-	// Soft-delete the skill itself
-	if _, err := tx.Exec("UPDATE skills SET status = 'archived' WHERE id = $1", id); err != nil {
-		return fmt.Errorf("archive skill: %w", err)
+	// Soft-delete the skill (use 'deleted' status, distinct from 'archived' which means missing deps)
+	if _, err := tx.Exec("UPDATE skills SET status = 'deleted' WHERE id = $1", id); err != nil {
+		return fmt.Errorf("delete skill: %w", err)
 	}
 
 	if err := tx.Commit(); err != nil {
@@ -142,7 +142,7 @@ func (s *PGSkillStore) CreateSkillManaged(ctx context.Context, p SkillCreatePara
 		   version = EXCLUDED.version, frontmatter = EXCLUDED.frontmatter,
 		   file_path = EXCLUDED.file_path,
 		   file_size = EXCLUDED.file_size, file_hash = EXCLUDED.file_hash,
-		   visibility = CASE WHEN skills.status = 'archived' THEN 'private' ELSE skills.visibility END,
+		   visibility = CASE WHEN skills.status IN ('archived', 'deleted') THEN 'private' ELSE skills.visibility END,
 		   status = 'active', updated_at = NOW()
 		 RETURNING id`,
 		id, p.Name, p.Slug, p.Description, p.OwnerID, p.Visibility, version,

internal/store/pg/skills_embedding.go

@@ -23,7 +23,7 @@ func (s *PGSkillStore) SearchByEmbedding(ctx context.Context, embedding []float3
 	vecStr := vectorToString(embedding)
 
 	rows, err := s.db.QueryContext(ctx,
-		`SELECT name, slug, COALESCE(description, ''), version,
+		`SELECT name, slug, COALESCE(description, ''), version, file_path,
 				1 - (embedding <=> $1::vector) AS score
 			FROM skills
 			WHERE status = 'active' AND enabled = true AND embedding IS NOT NULL
@@ -41,10 +41,16 @@ func (s *PGSkillStore) SearchByEmbedding(ctx context.Context, embedding []float3
 	for rows.Next() {
 		var r store.SkillSearchResult
 		var version int
-		if err := rows.Scan(&r.Name, &r.Slug, &r.Description, &version, &r.Score); err != nil {
+		var filePath *string
+		if err := rows.Scan(&r.Name, &r.Slug, &r.Description, &version, &filePath, &r.Score); err != nil {
 			continue
 		}
-		r.Path = fmt.Sprintf("%s/%s/%d/SKILL.md", s.baseDir, r.Slug, version)
+		// Use DB file_path when available; fall back to baseDir construction.
+		if filePath != nil && *filePath != "" {
+			r.Path = *filePath + "/SKILL.md"
+		} else {
+			r.Path = fmt.Sprintf("%s/%s/%d/SKILL.md", s.baseDir, r.Slug, version)
+		}
 		results = append(results, r)
 	}
 	return results, nil

internal/store/pg/skills_grants.go

@@ -114,7 +114,7 @@ func (s *PGSkillStore) RevokeFromUser(ctx context.Context, skillID uuid.UUID, us
 // Access logic: public → all, private → owner only, internal → check grants.
 func (s *PGSkillStore) ListAccessible(ctx context.Context, agentID uuid.UUID, userID string) ([]store.SkillInfo, error) {
 	rows, err := s.db.QueryContext(ctx,
-		`SELECT DISTINCT s.name, s.slug, s.description, s.version FROM skills s
+		`SELECT DISTINCT s.name, s.slug, s.description, s.version, s.file_path FROM skills s
 		LEFT JOIN skill_agent_grants sag ON s.id = sag.skill_id AND sag.agent_id = $1
 		LEFT JOIN skill_user_grants sug ON s.id = sug.skill_id AND sug.user_id = $2
 		WHERE s.status = 'active' AND (
@@ -134,11 +134,12 @@ func (s *PGSkillStore) ListAccessible(ctx context.Context, agentID uuid.UUID, us
 		var name, slug string
 		var desc *string
 		var version int
-		if err := rows.Scan(&name, &slug, &desc, &version); err != nil {
+		var filePath *string
+		if err := rows.Scan(&name, &slug, &desc, &version, &filePath); err != nil {
 			slog.Warn("skill_grants: scan error in ListAccessible", "error", err)
 			continue
 		}
-		result = append(result, buildSkillInfo("", name, slug, desc, version, s.baseDir))
+		result = append(result, buildSkillInfo("", name, slug, desc, version, s.baseDir, filePath))
 	}
 	return result, rows.Err()
 }

internal/tools/skill_manage.go

@@ -350,7 +350,7 @@ func (t *SkillManageTool) executeDelete(ctx context.Context, args map[string]any
 		t.loader.BumpVersion()
 	}
 
-	return NewResult(fmt.Sprintf("Skill %q archived and removed from search.", slug))
+	return NewResult(fmt.Sprintf("Skill %q deleted and removed from search.", slug))
 }
 
 // maxCopySize limits total companion file copy to 20MB (matching publish_skill).