fix(skills): allow archived skills to be found by slug/name lookup, matching UUID lookup and ListSkills behavior (#1368)

Co-authored-by: Bruno Clermont <bruno.clermont@gmail.com>
This commit is contained in:
Bruno ClermontandBruno Clermont authored and GitHub committed 2026-07-06 03:09:08 +07:00
1 parent db8ac12b16
commit ca097052b6
3 files changed
+34 -8

No files matched your search

+6 -2
View File
@@ -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 {
+6 -2
View File
@@ -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 {
+22 -4
View File
@@ -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) {