diff --git a/internal/store/pg/skills_content.go b/internal/store/pg/skills_content.go index df3aca0c..ac4d6948 100644 --- a/internal/store/pg/skills_content.go +++ b/internal/store/pg/skills_content.go @@ -135,13 +135,17 @@ func (s *PGSkillStore) GetSkill(ctx context.Context, name string) (*store.SkillI return &enriched[0], true } + // "archived" means a system skill has missing dependencies (see seeder.go); it does not + // mean the skill is hidden from lookup. ListSkills() and GetSkillByID() both include + // active + archived skills, so name/slug lookup must match to avoid 404s on archived + // skills that are otherwise still enabled. if id, err := uuid.Parse(name); err == nil { return scan(baseSelect+"id = $"+fmt.Sprint(len(args)+1)+" AND status IN ('active', 'archived')"+scope, append(args, id)...) } - if info, ok := scan(baseSelect+"slug = $"+fmt.Sprint(len(args)+1)+" AND status = 'active'"+scope, append(args, name)...); ok { + if info, ok := scan(baseSelect+"slug = $"+fmt.Sprint(len(args)+1)+" AND status IN ('active', 'archived')"+scope, append(args, name)...); ok { return info, true } - return scan(baseSelect+"name = $"+fmt.Sprint(len(args)+1)+" AND status = 'active'"+scope+" ORDER BY id LIMIT 1", append(args, name)...) + return scan(baseSelect+"name = $"+fmt.Sprint(len(args)+1)+" AND status IN ('active', 'archived')"+scope+" ORDER BY id LIMIT 1", append(args, name)...) } func (s *PGSkillStore) FilterSkills(ctx context.Context, allowList []string) []store.SkillInfo { diff --git a/internal/store/sqlitestore/skills_content.go b/internal/store/sqlitestore/skills_content.go index 1c497660..8520a74b 100644 --- a/internal/store/sqlitestore/skills_content.go +++ b/internal/store/sqlitestore/skills_content.go @@ -129,13 +129,17 @@ func (s *SQLiteSkillStore) GetSkill(ctx context.Context, name string) (*store.Sk return &enriched[0], true } + // "archived" means a system skill has missing dependencies (see seeder.go); it does not + // mean the skill is hidden from lookup. ListSkills() and GetSkillByID() both include + // active + archived skills, so name/slug lookup must match to avoid 404s on archived + // skills that are otherwise still enabled. if id, err := uuid.Parse(name); err == nil { return scan(baseSelect+"id = ? AND status IN ('active', 'archived')"+scope, append([]any{id}, args...)...) } - if info, ok := scan(baseSelect+"slug = ? AND status = 'active'"+scope, append([]any{name}, args...)...); ok { + if info, ok := scan(baseSelect+"slug = ? AND status IN ('active', 'archived')"+scope, append([]any{name}, args...)...); ok { return info, true } - return scan(baseSelect+"name = ? AND status = 'active'"+scope+" ORDER BY id LIMIT 1", append([]any{name}, args...)...) + return scan(baseSelect+"name = ? AND status IN ('active', 'archived')"+scope+" ORDER BY id LIMIT 1", append([]any{name}, args...)...) } func (s *SQLiteSkillStore) FilterSkills(ctx context.Context, allowList []string) []store.SkillInfo { diff --git a/internal/store/sqlitestore/skills_test.go b/internal/store/sqlitestore/skills_test.go index 93be73cf..5e8749ff 100644 --- a/internal/store/sqlitestore/skills_test.go +++ b/internal/store/sqlitestore/skills_test.go @@ -70,7 +70,12 @@ func TestSQLiteSkillStore_CreateSkillManaged_PersistsArchivedDependencyState(t * } } -func TestSQLiteSkillStore_GetSkill_UUIDCanReadArchivedSlugStaysActiveOnly(t *testing.T) { +func TestSQLiteSkillStore_GetSkill_ArchivedFoundByUUIDAndSlugAndName(t *testing.T) { + // "archived" means a system skill has missing dependencies (see internal/skills/seeder.go); + // it is still enabled and must be discoverable the same way as via ListSkills() and + // GetSkillByID(), which both include active + archived skills. Regression test for a bug + // where archived-but-enabled skills (e.g. docx, pdf, pptx, skill-creator, xlsx when deps + // are missing) 404'd when looked up by name/slug while UUID lookup worked fine. ctx, skillStore := newTestSQLiteSkillStore(t) skillID, err := skillStore.CreateSkillManaged(ctx, store.SkillCreateParams{ Name: "Archived Detail", @@ -84,9 +89,6 @@ func TestSQLiteSkillStore_GetSkill_UUIDCanReadArchivedSlugStaysActiveOnly(t *tes t.Fatalf("CreateSkillManaged error: %v", err) } - if _, ok := skillStore.GetSkill(ctx, "archived-detail"); ok { - t.Fatal("GetSkill by slug returned archived skill; want active-only slug lookup") - } info, ok := skillStore.GetSkill(ctx, skillID.String()) if !ok { t.Fatal("GetSkill by UUID returned !ok for archived skill") @@ -94,6 +96,22 @@ func TestSQLiteSkillStore_GetSkill_UUIDCanReadArchivedSlugStaysActiveOnly(t *tes if info.Status != "archived" { t.Fatalf("Status = %q, want archived", info.Status) } + + slugInfo, ok := skillStore.GetSkill(ctx, "archived-detail") + if !ok { + t.Fatal("GetSkill by slug returned !ok for archived skill; want consistency with ListSkills/GetSkillByID inclusion policy") + } + if slugInfo.Status != "archived" { + t.Fatalf("Status = %q, want archived", slugInfo.Status) + } + + nameInfo, ok := skillStore.GetSkill(ctx, "Archived Detail") + if !ok { + t.Fatal("GetSkill by name returned !ok for archived skill; want consistency with ListSkills/GetSkillByID inclusion policy") + } + if nameInfo.Status != "archived" { + t.Fatalf("Status = %q, want archived", nameInfo.Status) + } } func TestSQLiteSkillStore_ListSkills_ResolvesCreatorAgentWithinTenant(t *testing.T) {