fix: auto-install deps on skill upload before archiving (#559)

Upload handler previously archived skills immediately when deps were
missing. Now calls InstallDeps() first (owner/master tenant only) and
falls back to archive on failure.

Changes:
- Auto-install missing deps during skill upload (same flow as seeder)
- Atomic DB persist: deps state written with CreateSkillManaged in one call
- Per-slug upload mutex prevents concurrent race conditions
- Frontend: warning state (amber triangle) instead of throwing error
- Non-cancellable context for DB write after dep install
- SQLite StoreMissingDeps now works for custom skills (not just system)
- Comprehensive unit + integration tests

Closes #468
This commit is contained in:
Kai (Tam Nhu) Tran authored and GitHub committed 2026-03-30 21:58:29 +07:00
1 parent 4c60dd021e
commit 3fe0633d35
13 files changed
+889 -58

No files matched your search

+14 -2
View File
@@ -8,6 +8,7 @@ import (
"net/http"
"os"
"path/filepath"
"sync"
"github.com/google/uuid"
@@ -22,6 +23,11 @@ import (
const maxSkillUploadSize = 20 << 20 // 20 MB
var (
aggregateInstallDeps = skills.AggregateMissingDeps
installManagedDeps = skills.InstallDeps
)
// SkillsHandler handles skill management HTTP endpoints.
type SkillsHandler struct {
skills store.SkillManageStore
@@ -32,6 +38,7 @@ type SkillsHandler struct {
tenantCfgStore store.SkillTenantConfigStore
tenantStore store.TenantStore
db *sql.DB // for export/import direct queries
uploadLocks sync.Map // per-slug mutex; bounded by validated slug set, entries are tiny (*sync.Mutex)
}
// NewSkillsHandler creates a handler for skill management endpoints.
@@ -47,6 +54,11 @@ func (h *SkillsHandler) tenantSkillsDir(r *http.Request) string {
return config.TenantSkillsStoreDir(h.dataDir, tid, slug)
}
func (h *SkillsHandler) skillUploadLock(scopeKey string) *sync.Mutex {
actual, _ := h.uploadLocks.LoadOrStore(scopeKey, &sync.Mutex{})
return actual.(*sync.Mutex)
}
// emitCacheInvalidate broadcasts a cache invalidation event if msgBus is set.
func (h *SkillsHandler) emitCacheInvalidate(kind, key string) {
if h.msgBus == nil {
@@ -249,7 +261,7 @@ func (h *SkillsHandler) handleInstallDeps(w http.ResponseWriter, r *http.Request
return
}
manifest, missing := skills.AggregateMissingDeps(dirs)
manifest, missing := aggregateInstallDeps(dirs)
if len(missing) == 0 {
writeJSON(w, http.StatusOK, map[string]string{"message": "all deps satisfied"})
return
@@ -262,7 +274,7 @@ func (h *SkillsHandler) handleInstallDeps(w http.ResponseWriter, r *http.Request
})
}
result, err := skills.InstallDeps(r.Context(), manifest, missing)
result, err := installManagedDeps(r.Context(), manifest, missing)
if err != nil {
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
return
+169 -20
View File
@@ -2,6 +2,7 @@ package http
import (
"archive/zip"
"context"
"crypto/sha256"
"fmt"
"io"
@@ -10,10 +11,20 @@ import (
"os"
"path/filepath"
"strings"
"time"
"github.com/nextlevelbuilder/goclaw/internal/bus"
"github.com/nextlevelbuilder/goclaw/internal/i18n"
"github.com/nextlevelbuilder/goclaw/internal/skills"
"github.com/nextlevelbuilder/goclaw/internal/store"
"github.com/nextlevelbuilder/goclaw/pkg/protocol"
)
const uploadDepsInstallTimeout = 5 * time.Minute
var (
installUploadedSkillDeps = skills.InstallDeps
checkUploadedSkillDeps = skills.CheckSkillDeps
)
// handleUpload processes a ZIP file upload containing a skill (must have SKILL.md at root).
@@ -113,11 +124,15 @@ func (h *SkillsHandler) handleUpload(w http.ResponseWriter, r *http.Request) {
return
}
tenantSkillsBase := h.tenantSkillsDir(r)
uploadLock := h.skillUploadLock(filepath.Join(tenantSkillsBase, slug))
uploadLock.Lock()
defer uploadLock.Unlock()
// Determine version (always increment — includes archived skills so re-upload gets v2+)
version := h.skills.GetNextVersion(r.Context(), slug)
// Extract to filesystem: tenant-scoped skills-store/slug/version/
tenantSkillsBase := h.tenantSkillsDir(r)
destDir := filepath.Join(tenantSkillsBase, slug, fmt.Sprintf("%d", version))
if err := os.MkdirAll(destDir, 0755); err != nil {
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgInternalError, "failed to create skill directory")})
@@ -178,32 +193,166 @@ func (h *SkillsHandler) handleUpload(w http.ResponseWriter, r *http.Request) {
Frontmatter: frontmatter,
}
id, err := h.skills.CreateSkillManaged(r.Context(), skill)
// Scan and check dependencies
response := map[string]any{"slug": slug, "version": version, "name": name, "status": "active"}
depState := uploadSkillDepState{}
depsCtx, cancel := context.WithTimeout(context.WithoutCancel(r.Context()), uploadDepsInstallTimeout)
defer cancel()
manifest := skills.ScanSkillDeps(destDir)
if manifest != nil && !manifest.IsEmpty() {
if ok, missing := checkUploadedSkillDeps(manifest); !ok {
depState = h.reconcileUploadedSkillDeps(
depsCtx,
slug,
manifest,
missing,
canAutoInstallUploadedSkillDeps(r.Context()),
)
skill.Status = depState.status
skill.MissingDeps = depState.missing
for key, value := range depState.response {
response[key] = value
}
}
}
// Use depsCtx (non-cancellable) so the DB write completes even if the
// client disconnects during the dep-install window.
id, err := h.skills.CreateSkillManaged(depsCtx, skill)
if err != nil {
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgFailedToCreate, "skill", err.Error())})
return
}
response["id"] = id
h.skills.BumpVersion()
emitAudit(h.msgBus, r, "skill.uploaded", "skill", slug)
slog.Info("skill uploaded", "id", id, "slug", slug, "version", version, "size", header.Size)
// Scan and check dependencies
response := map[string]any{
"id": id,
"slug": slug,
"version": version,
"name": name,
}
manifest := skills.ScanSkillDeps(destDir)
if manifest != nil && !manifest.IsEmpty() {
ok, missing := skills.CheckSkillDeps(manifest)
if !ok {
// Set skill to archived due to missing deps
_ = h.skills.UpdateSkill(r.Context(), id, map[string]any{"status": "archived"})
response["deps_warning"] = "missing dependencies: " + skills.FormatMissing(missing)
}
}
slog.Info("skill uploaded", "id", id, "slug", slug, "version", version, "size", header.Size, "status", skill.Status)
depState.emit(h, slug)
writeJSON(w, http.StatusCreated, response)
}
func canAutoInstallUploadedSkillDeps(ctx context.Context) bool {
return store.IsOwnerRole(ctx) || store.TenantIDFromContext(ctx) == store.MasterTenantID
}
func uploadDepErrors(result *skills.InstallResult, installErr error) []string {
var errors []string
if installErr != nil {
errors = append(errors, installErr.Error())
}
if result != nil && len(result.Errors) > 0 {
errors = append(errors, result.Errors...)
}
return errors
}
func (h *SkillsHandler) emitUploadDepInstalling(slug string, count int) {
if h.msgBus == nil {
return
}
h.msgBus.Broadcast(bus.Event{
Name: protocol.EventSkillDepsInstalling,
Payload: map[string]any{"skill": slug, "count": count},
})
}
func (h *SkillsHandler) emitUploadDepChecked(slug, status string, missing []string) {
if h.msgBus == nil {
return
}
payload := map[string]any{
"slug": slug,
"status": status,
}
if len(missing) > 0 {
payload["missing"] = missing
}
h.msgBus.Broadcast(bus.Event{
Name: protocol.EventSkillDepsChecked,
Payload: payload,
})
}
func (h *SkillsHandler) emitUploadDepInstalled(slug string, result *skills.InstallResult) {
if h.msgBus == nil {
return
}
payload := map[string]any{"skill": slug}
if result != nil {
payload["result"] = result
}
h.msgBus.Broadcast(bus.Event{
Name: protocol.EventSkillDepsInstalled,
Payload: payload,
})
}
func (h *SkillsHandler) reconcileUploadedSkillDeps(
ctx context.Context,
slug string,
manifest *skills.SkillManifest,
missing []string,
allowAutoInstall bool,
) uploadSkillDepState {
response := map[string]any{}
finalStatus := "archived"
finalMissing := append([]string(nil), missing...)
state := uploadSkillDepState{installCount: len(missing), checked: true}
var installResult *skills.InstallResult
var installErr error
if allowAutoInstall {
installResult, installErr = installUploadedSkillDeps(ctx, manifest, missing)
if ok, checkedMissing := checkUploadedSkillDeps(manifest); ok {
finalStatus = "active"
finalMissing = nil
response["deps_installed"] = true
slog.Info("skill deps auto-installed", "skill", slug, "installed", missing)
} else {
finalMissing = checkedMissing
slog.Warn("skill deps auto-install failed", "skill", slug, "missing", finalMissing, "errors", uploadDepErrors(installResult, installErr))
}
state.installResult = installResult
} else {
response["deps_warning"] = "missing dependencies: " + skills.FormatMissing(finalMissing)
state.installCount = 0
}
if finalStatus == "archived" {
if _, exists := response["deps_warning"]; !exists {
response["deps_warning"] = "auto-install failed for: " + skills.FormatMissing(finalMissing)
}
response["missing_deps"] = finalMissing
if errors := uploadDepErrors(installResult, installErr); len(errors) > 0 {
response["deps_errors"] = errors
}
}
response["status"] = finalStatus
state.status = finalStatus
state.missing = finalMissing
state.response = response
return state
}
type uploadSkillDepState struct {
status string
missing []string
response map[string]any
installCount int
installResult *skills.InstallResult
checked bool
}
func (s uploadSkillDepState) emit(h *SkillsHandler, slug string) {
if !s.checked {
return
}
if s.installCount > 0 {
h.emitUploadDepInstalling(slug, s.installCount)
h.emitUploadDepInstalled(slug, s.installResult)
}
h.emitUploadDepChecked(slug, s.status, s.missing)
}
+531
View File
@@ -0,0 +1,531 @@
package http
import (
"archive/zip"
"bytes"
"context"
"encoding/json"
"mime/multipart"
"net/http"
"net/http/httptest"
"path/filepath"
"reflect"
"testing"
"github.com/google/uuid"
"github.com/nextlevelbuilder/goclaw/internal/bus"
"github.com/nextlevelbuilder/goclaw/internal/skills"
"github.com/nextlevelbuilder/goclaw/internal/store"
"github.com/nextlevelbuilder/goclaw/pkg/protocol"
)
func captureEventNames(msgBus *bus.MessageBus) *[]string {
names := []string{}
msgBus.Subscribe("test", func(event bus.Event) { names = append(names, event.Name) })
return &names
}
func stubUploadDepFns(
t *testing.T,
installFn func(context.Context, *skills.SkillManifest, []string) (*skills.InstallResult, error),
checkFn func(*skills.SkillManifest) (bool, []string),
) {
t.Helper()
prevInstall := installUploadedSkillDeps
prevCheck := checkUploadedSkillDeps
installUploadedSkillDeps = installFn
checkUploadedSkillDeps = checkFn
t.Cleanup(func() {
installUploadedSkillDeps = prevInstall
checkUploadedSkillDeps = prevCheck
})
}
func TestReconcileUploadedSkillDeps_SkipsAutoInstallOutsideMasterTenant(t *testing.T) {
msgBus := bus.New()
handler := &SkillsHandler{msgBus: msgBus}
events := captureEventNames(msgBus)
called := false
stubUploadDepFns(t, func(context.Context, *skills.SkillManifest, []string) (*skills.InstallResult, error) {
called = true
return nil, nil
}, func(*skills.SkillManifest) (bool, []string) { return false, nil })
state := handler.reconcileUploadedSkillDeps(context.Background(), "demo", &skills.SkillManifest{}, []string{"pip:requests"}, false)
if called {
t.Fatal("expected auto-install to be skipped")
}
if got := state.status; got != "archived" {
t.Fatalf("status = %v, want archived", got)
}
if !reflect.DeepEqual(state.missing, []string{"pip:requests"}) {
t.Fatalf("missing = %#v", state.missing)
}
response := state.response
state.emit(handler, "demo")
if got := response["deps_warning"]; got != "missing dependencies: pip:requests" {
t.Fatalf("deps_warning = %v", got)
}
if !reflect.DeepEqual(response["missing_deps"], []string{"pip:requests"}) {
t.Fatalf("missing_deps = %#v", response["missing_deps"])
}
if !reflect.DeepEqual(*events, []string{protocol.EventSkillDepsChecked}) {
t.Fatalf("events = %v", *events)
}
}
func TestReconcileUploadedSkillDeps_AutoInstallSuccessClearsMissingDeps(t *testing.T) {
msgBus := bus.New()
handler := &SkillsHandler{msgBus: msgBus}
events := captureEventNames(msgBus)
stubUploadDepFns(t,
func(context.Context, *skills.SkillManifest, []string) (*skills.InstallResult, error) {
return &skills.InstallResult{Pip: []string{"requests"}}, nil
},
func(*skills.SkillManifest) (bool, []string) { return true, nil },
)
state := handler.reconcileUploadedSkillDeps(context.Background(), "demo", &skills.SkillManifest{}, []string{"pip:requests"}, true)
if got := state.status; got != "active" {
t.Fatalf("status = %v, want active", got)
}
if len(state.missing) != 0 {
t.Fatalf("missing = %v, want none", state.missing)
}
response := state.response
state.emit(handler, "demo")
if got := response["deps_installed"]; got != true {
t.Fatalf("deps_installed = %v, want true", got)
}
wantEvents := []string{
protocol.EventSkillDepsInstalling,
protocol.EventSkillDepsInstalled,
protocol.EventSkillDepsChecked,
}
if !reflect.DeepEqual(*events, wantEvents) {
t.Fatalf("events = %v, want %v", *events, wantEvents)
}
}
func TestReconcileUploadedSkillDeps_AutoInstallFailureArchivesSkill(t *testing.T) {
msgBus := bus.New()
handler := &SkillsHandler{msgBus: msgBus}
events := captureEventNames(msgBus)
stubUploadDepFns(t,
func(context.Context, *skills.SkillManifest, []string) (*skills.InstallResult, error) {
return &skills.InstallResult{Errors: []string{"pip failed"}}, nil
},
func(*skills.SkillManifest) (bool, []string) { return false, []string{"pip:requests"} },
)
state := handler.reconcileUploadedSkillDeps(context.Background(), "demo", &skills.SkillManifest{}, []string{"pip:requests"}, true)
if got := state.status; got != "archived" {
t.Fatalf("status = %v, want archived", got)
}
if !reflect.DeepEqual(state.missing, []string{"pip:requests"}) {
t.Fatalf("missing = %#v", state.missing)
}
response := state.response
state.emit(handler, "demo")
if got := response["deps_warning"]; got != "auto-install failed for: pip:requests" {
t.Fatalf("deps_warning = %v", got)
}
if !reflect.DeepEqual(response["deps_errors"], []string{"pip failed"}) {
t.Fatalf("deps_errors = %#v", response["deps_errors"])
}
wantEvents := []string{
protocol.EventSkillDepsInstalling,
protocol.EventSkillDepsInstalled,
protocol.EventSkillDepsChecked,
}
if !reflect.DeepEqual(*events, wantEvents) {
t.Fatalf("events = %v, want %v", *events, wantEvents)
}
}
func TestHandleUpload_AutoInstallsMissingDepsAndKeepsSkillActive(t *testing.T) {
handler, skillStore, ctx, _ := newTestUploadHandler(t)
installCalls := 0
checkCalls := 0
stubUploadDepFns(t,
func(context.Context, *skills.SkillManifest, []string) (*skills.InstallResult, error) {
installCalls++
return &skills.InstallResult{Pip: []string{"requests"}}, nil
},
func(*skills.SkillManifest) (bool, []string) {
checkCalls++
if checkCalls == 1 {
return false, []string{"pip:requests"}
}
return true, nil
},
)
req := newZipUploadRequest(t, ctx, map[string]string{
"SKILL.md": skillMarkdown("Pip Skill", "pip-skill"),
"scripts/run.py": "import requests\n",
})
w := httptest.NewRecorder()
handler.handleUpload(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("status = %d, body = %s", w.Code, w.Body.String())
}
if installCalls != 1 {
t.Fatalf("install calls = %d, want 1", installCalls)
}
var resp struct {
ID string `json:"id"`
Status string `json:"status"`
DepsInstalled bool `json:"deps_installed"`
DepsErrors []string `json:"deps_errors"`
MissingDeps []string `json:"missing_deps"`
}
if err := json.NewDecoder(w.Body).Decode(&resp); err != nil {
t.Fatalf("decode response: %v", err)
}
if resp.Status != "active" {
t.Fatalf("response status = %q, want active", resp.Status)
}
if !resp.DepsInstalled {
t.Fatal("expected deps_installed=true")
}
if len(resp.DepsErrors) != 0 {
t.Fatalf("deps_errors = %v, want none", resp.DepsErrors)
}
if len(resp.MissingDeps) != 0 {
t.Fatalf("missing_deps = %v, want none", resp.MissingDeps)
}
id := uuid.MustParse(resp.ID)
info, ok := skillStore.GetSkillByID(ctx, id)
if !ok {
t.Fatal("GetSkillByID returned !ok")
}
if info.Status != "active" {
t.Fatalf("stored status = %q, want active", info.Status)
}
if len(info.MissingDeps) != 0 {
t.Fatalf("stored missing_deps = %v, want none", info.MissingDeps)
}
}
func TestHandleUpload_UninstallableDepArchivesSkillWithErrors(t *testing.T) {
handler, skillStore, ctx, _ := newTestUploadHandler(t)
installCalls := 0
stubUploadDepFns(t,
func(context.Context, *skills.SkillManifest, []string) (*skills.InstallResult, error) {
installCalls++
return &skills.InstallResult{Errors: []string{"pip failed"}}, nil
},
func(*skills.SkillManifest) (bool, []string) { return false, []string{"pip:requests"} },
)
req := newZipUploadRequest(t, ctx, map[string]string{
"SKILL.md": skillMarkdown("Broken Pip Skill", "broken-pip-skill"),
"scripts/run.py": "import requests\n",
})
w := httptest.NewRecorder()
handler.handleUpload(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("status = %d, body = %s", w.Code, w.Body.String())
}
if installCalls != 1 {
t.Fatalf("install calls = %d, want 1", installCalls)
}
var resp struct {
ID string `json:"id"`
Status string `json:"status"`
DepsErrors []string `json:"deps_errors"`
MissingDeps []string `json:"missing_deps"`
}
if err := json.NewDecoder(w.Body).Decode(&resp); err != nil {
t.Fatalf("decode response: %v", err)
}
if resp.Status != "archived" {
t.Fatalf("response status = %q, want archived", resp.Status)
}
if !reflect.DeepEqual(resp.DepsErrors, []string{"pip failed"}) {
t.Fatalf("deps_errors = %v", resp.DepsErrors)
}
if !reflect.DeepEqual(resp.MissingDeps, []string{"pip:requests"}) {
t.Fatalf("missing_deps = %v", resp.MissingDeps)
}
id := uuid.MustParse(resp.ID)
info, ok := skillStore.GetSkillByID(ctx, id)
if !ok {
t.Fatal("GetSkillByID returned !ok")
}
if info.Status != "archived" {
t.Fatalf("stored status = %q, want archived", info.Status)
}
if !reflect.DeepEqual(info.MissingDeps, []string{"pip:requests"}) {
t.Fatalf("stored missing_deps = %v", info.MissingDeps)
}
}
func TestHandleInstallDeps_ExistingEndpointStillReturnsInstallResult(t *testing.T) {
handler, skillStore, ctx, root := newTestUploadHandler(t)
systemDir := filepath.Join(root, "skills-store", "system-skill", "1")
skillStore.seedSystemSkill("system-skill", systemDir)
prevAggregate := aggregateInstallDeps
prevInstall := installManagedDeps
aggregateInstallDeps = func(dirs map[string]string) (*skills.SkillManifest, []string) {
if got := dirs["system-skill"]; got != systemDir {
t.Fatalf("system dir = %q, want %q", got, systemDir)
}
return &skills.SkillManifest{RequiresPython: []string{"requests"}}, []string{"pip:requests"}
}
installManagedDeps = func(context.Context, *skills.SkillManifest, []string) (*skills.InstallResult, error) {
return &skills.InstallResult{Pip: []string{"requests"}}, nil
}
t.Cleanup(func() {
aggregateInstallDeps = prevAggregate
installManagedDeps = prevInstall
})
req := httptest.NewRequest(http.MethodPost, "/v1/skills/install-deps", http.NoBody).WithContext(ctx)
w := httptest.NewRecorder()
handler.handleInstallDeps(w, req)
if w.Code != http.StatusOK {
t.Fatalf("status = %d, body = %s", w.Code, w.Body.String())
}
var resp skills.InstallResult
if err := json.NewDecoder(w.Body).Decode(&resp); err != nil {
t.Fatalf("decode response: %v", err)
}
if !reflect.DeepEqual(resp.Pip, []string{"requests"}) {
t.Fatalf("pip installs = %v, want [requests]", resp.Pip)
}
}
func newTestUploadHandler(t *testing.T) (*SkillsHandler, *skillManageStoreStub, context.Context, string) {
t.Helper()
root := t.TempDir()
baseDir := filepath.Join(root, "skills-store")
skillStore := newSkillManageStoreStub(baseDir)
handler := NewSkillsHandler(skillStore, baseDir, root, "", bus.New(), nil, nil)
ctx := store.WithLocale(
store.WithTenantID(
store.WithUserID(context.Background(), "user-1"),
store.MasterTenantID,
),
"en",
)
return handler, skillStore, ctx, root
}
func newZipUploadRequest(t *testing.T, ctx context.Context, files map[string]string) *http.Request {
t.Helper()
var zipBuf bytes.Buffer
zw := zip.NewWriter(&zipBuf)
for name, content := range files {
w, err := zw.Create(name)
if err != nil {
t.Fatalf("zip create %s: %v", name, err)
}
if _, err := w.Write([]byte(content)); err != nil {
t.Fatalf("zip write %s: %v", name, err)
}
}
if err := zw.Close(); err != nil {
t.Fatalf("zip close: %v", err)
}
var body bytes.Buffer
mw := multipart.NewWriter(&body)
part, err := mw.CreateFormFile("file", "skill.zip")
if err != nil {
t.Fatalf("multipart file: %v", err)
}
if _, err := part.Write(zipBuf.Bytes()); err != nil {
t.Fatalf("multipart write: %v", err)
}
if err := mw.Close(); err != nil {
t.Fatalf("multipart close: %v", err)
}
req := httptest.NewRequest(http.MethodPost, "/v1/skills/upload", &body)
req.Header.Set("Content-Type", mw.FormDataContentType())
return req.WithContext(ctx)
}
func skillMarkdown(name, slug string) string {
return "---\nname: " + name + "\nslug: " + slug + "\n---\nSkill body\n"
}
type skillManageStoreStub struct {
baseDir string
version int64
nextBySlug map[string]int
skills map[uuid.UUID]store.SkillInfo
systemDirs map[string]string
}
func newSkillManageStoreStub(baseDir string) *skillManageStoreStub {
return &skillManageStoreStub{
baseDir: baseDir,
nextBySlug: map[string]int{},
skills: map[uuid.UUID]store.SkillInfo{},
systemDirs: map[string]string{},
}
}
func (s *skillManageStoreStub) seedSystemSkill(slug, dir string) {
id := uuid.New()
s.skills[id] = store.SkillInfo{
ID: id.String(),
Name: "System Skill",
Slug: slug,
Path: filepath.Join(dir, "SKILL.md"),
BaseDir: dir,
Version: 1,
Status: "active",
Enabled: true,
IsSystem: true,
}
s.systemDirs[slug] = dir
}
func (s *skillManageStoreStub) ListSkills(context.Context) []store.SkillInfo {
return s.ListAllSkills(context.Background())
}
func (s *skillManageStoreStub) LoadSkill(context.Context, string) (string, bool) { return "", false }
func (s *skillManageStoreStub) LoadForContext(context.Context, []string) string { return "" }
func (s *skillManageStoreStub) BuildSummary(context.Context, []string) string { return "" }
func (s *skillManageStoreStub) GetSkill(_ context.Context, name string) (*store.SkillInfo, bool) {
for _, skill := range s.skills {
if skill.Slug == name {
copy := skill
return &copy, true
}
}
return nil, false
}
func (s *skillManageStoreStub) FilterSkills(context.Context, []string) []store.SkillInfo {
return s.ListAllSkills(context.Background())
}
func (s *skillManageStoreStub) Version() int64 { return s.version }
func (s *skillManageStoreStub) BumpVersion() { s.version++ }
func (s *skillManageStoreStub) Dirs() []string { return []string{s.baseDir} }
func (s *skillManageStoreStub) CreateSkillManaged(_ context.Context, p store.SkillCreateParams) (uuid.UUID, error) {
id := uuid.New()
status := p.Status
if status == "" {
status = "active"
}
version := p.Version
if version == 0 {
version = s.nextBySlug[p.Slug] + 1
}
if version > s.nextBySlug[p.Slug] {
s.nextBySlug[p.Slug] = version
}
s.skills[id] = store.SkillInfo{
ID: id.String(),
Name: p.Name,
Slug: p.Slug,
Path: filepath.Join(p.FilePath, "SKILL.md"),
BaseDir: p.FilePath,
Version: version,
Status: status,
Enabled: true,
MissingDeps: append([]string(nil), p.MissingDeps...),
}
return id, nil
}
func (s *skillManageStoreStub) UpdateSkill(_ context.Context, id uuid.UUID, updates map[string]any) error {
skill, ok := s.skills[id]
if !ok {
return nil
}
if status, ok := updates["status"].(string); ok {
skill.Status = status
}
s.skills[id] = skill
return nil
}
func (s *skillManageStoreStub) DeleteSkill(context.Context, uuid.UUID) error { return nil }
func (s *skillManageStoreStub) ToggleSkill(context.Context, uuid.UUID, bool) error { return nil }
func (s *skillManageStoreStub) GetSkillByID(_ context.Context, id uuid.UUID) (store.SkillInfo, bool) {
info, ok := s.skills[id]
return info, ok
}
func (s *skillManageStoreStub) GetSkillOwnerID(context.Context, uuid.UUID) (string, bool) {
return "", false
}
func (s *skillManageStoreStub) GetSkillOwnerIDBySlug(context.Context, string) (string, bool) {
return "", false
}
func (s *skillManageStoreStub) GetNextVersion(_ context.Context, slug string) int {
return s.nextBySlug[slug] + 1
}
func (s *skillManageStoreStub) GetNextVersionLocked(_ context.Context, slug string) (int, func() error, error) {
return s.GetNextVersion(context.Background(), slug), func() error { return nil }, nil
}
func (s *skillManageStoreStub) IsSystemSkill(slug string) bool {
_, ok := s.systemDirs[slug]
return ok
}
func (s *skillManageStoreStub) ListAllSkills(context.Context) []store.SkillInfo {
out := make([]store.SkillInfo, 0, len(s.skills))
for _, skill := range s.skills {
out = append(out, skill)
}
return out
}
func (s *skillManageStoreStub) ListAllSystemSkills(context.Context) []store.SkillInfo {
var out []store.SkillInfo
for _, skill := range s.skills {
if skill.IsSystem {
out = append(out, skill)
}
}
return out
}
func (s *skillManageStoreStub) ListSystemSkillDirs(context.Context) map[string]string {
out := make(map[string]string, len(s.systemDirs))
for slug, dir := range s.systemDirs {
out[slug] = dir
}
return out
}
func (s *skillManageStoreStub) StoreMissingDeps(_ context.Context, id uuid.UUID, missing []string) error {
skill, ok := s.skills[id]
if !ok {
return nil
}
skill.MissingDeps = append([]string(nil), missing...)
s.skills[id] = skill
return nil
}
func (s *skillManageStoreStub) GrantToAgent(context.Context, uuid.UUID, uuid.UUID, int, string) error {
return nil
}
func (s *skillManageStoreStub) RevokeFromAgent(context.Context, uuid.UUID, uuid.UUID) error {
return nil
}
func (s *skillManageStoreStub) GrantToUser(context.Context, uuid.UUID, string, string) error {
return nil
}
func (s *skillManageStoreStub) RevokeFromUser(context.Context, uuid.UUID, string) error { return nil }
func (s *skillManageStoreStub) ListWithGrantStatus(context.Context, uuid.UUID) ([]store.SkillWithGrantStatus, error) {
return nil, nil
}
func (s *skillManageStoreStub) GetSkillFilePath(context.Context, uuid.UUID) (string, string, int, bool, bool) {
return "", "", 0, false, false
}
+8 -4
View File
@@ -190,10 +190,7 @@ func (s *PGSkillStore) scanSkillInfoList(rows *sql.Rows) []store.SkillInfo {
// Works for both system and custom skills. System skills bypass tenant filter;
// custom skills require tenant_id match for cross-tenant safety.
func (s *PGSkillStore) StoreMissingDeps(ctx context.Context, id uuid.UUID, missing []string) error {
if missing == nil {
missing = []string{}
}
encoded, err := json.Marshal(map[string]any{"missing": missing})
encoded, err := marshalMissingDeps(missing)
if err != nil {
return err
}
@@ -207,3 +204,10 @@ func (s *PGSkillStore) StoreMissingDeps(ctx context.Context, id uuid.UUID, missi
}
return err
}
func marshalMissingDeps(missing []string) ([]byte, error) {
if missing == nil {
missing = []string{}
}
return json.Marshal(map[string]any{"missing": missing})
}
+13 -5
View File
@@ -123,6 +123,14 @@ func (s *PGSkillStore) CreateSkillManaged(ctx context.Context, p store.SkillCrea
fmJSON = b
}
}
depsJSON, err := marshalMissingDeps(p.MissingDeps)
if err != nil {
return uuid.Nil, fmt.Errorf("marshal deps: %w", err)
}
status := p.Status
if status == "" {
status = "active"
}
tx, err := s.db.BeginTx(ctx, nil)
if err != nil {
@@ -154,18 +162,18 @@ func (s *PGSkillStore) CreateSkillManaged(ctx context.Context, p store.SkillCrea
id := store.GenNewID()
var returnedID uuid.UUID
err = tx.QueryRowContext(ctx,
`INSERT INTO skills (id, name, slug, description, owner_id, tenant_id, visibility, version, status, frontmatter, file_path, file_size, file_hash, created_at, updated_at)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, 'active', $9, $10, $11, $12, NOW(), NOW())
`INSERT INTO skills (id, name, slug, description, owner_id, tenant_id, visibility, version, status, deps, frontmatter, file_path, file_size, file_hash, created_at, updated_at)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, NOW(), NOW())
ON CONFLICT (tenant_id, slug) DO UPDATE SET
name = EXCLUDED.name, description = EXCLUDED.description,
version = EXCLUDED.version, frontmatter = EXCLUDED.frontmatter,
file_path = EXCLUDED.file_path,
file_path = EXCLUDED.file_path, deps = EXCLUDED.deps,
file_size = EXCLUDED.file_size, file_hash = EXCLUDED.file_hash,
visibility = CASE WHEN skills.status IN ('archived', 'deleted') THEN 'private' ELSE skills.visibility END,
status = 'active', updated_at = NOW()
status = EXCLUDED.status, updated_at = NOW()
RETURNING id`,
id, p.Name, p.Slug, p.Description, p.OwnerID, tenantID, p.Visibility, version,
fmJSON, p.FilePath, p.FileSize, p.FileHash,
status, depsJSON, fmJSON, p.FilePath, p.FileSize, p.FileHash,
).Scan(&returnedID)
if err != nil {
return uuid.Nil, fmt.Errorf("upsert skill: %w", err)
+1
View File
@@ -71,6 +71,7 @@ type SkillCreateParams struct {
OwnerID string
Visibility string
Status string // "active", "archived" (missing deps), or "deleted" (user-deleted)
MissingDeps []string
Version int
FilePath string
FileSize int64
+9 -5
View File
@@ -174,15 +174,12 @@ func (s *SQLiteSkillStore) scanSkillInfoList(rows *sql.Rows) []store.SkillInfo {
}
func (s *SQLiteSkillStore) StoreMissingDeps(ctx context.Context, id uuid.UUID, missing []string) error {
if missing == nil {
missing = []string{}
}
encoded, err := json.Marshal(map[string]any{"missing": missing})
encoded, err := marshalMissingDeps(missing)
if err != nil {
return err
}
_, err = s.db.ExecContext(ctx,
`UPDATE skills SET deps = ?, updated_at = ? WHERE id = ? AND is_system = 1`,
`UPDATE skills SET deps = ?, updated_at = ? WHERE id = ?`,
encoded, time.Now().UTC(), id,
)
if err == nil {
@@ -191,6 +188,13 @@ func (s *SQLiteSkillStore) StoreMissingDeps(ctx context.Context, id uuid.UUID, m
return err
}
func marshalMissingDeps(missing []string) ([]byte, error) {
if missing == nil {
missing = []string{}
}
return json.Marshal(map[string]any{"missing": missing})
}
// --- helpers shared with other skill files ---
func buildSkillInfo(id, name, slug string, desc *string, version int, baseDir string, filePath *string) store.SkillInfo {
+14 -6
View File
@@ -98,6 +98,14 @@ func (s *SQLiteSkillStore) CreateSkillManaged(ctx context.Context, p store.Skill
}
fmJSON := marshalFrontmatter(p.Frontmatter)
depsJSON, err := marshalMissingDeps(p.MissingDeps)
if err != nil {
return uuid.Nil, fmt.Errorf("marshal deps: %w", err)
}
status := p.Status
if status == "" {
status = "active"
}
tenantID := store.TenantIDFromContext(ctx)
if tenantID == uuid.Nil {
@@ -131,11 +139,11 @@ func (s *SQLiteSkillStore) CreateSkillManaged(ctx context.Context, p store.Skill
// Update existing.
_, err = tx.ExecContext(ctx,
`UPDATE skills SET name = ?, description = ?, version = ?, frontmatter = ?,
file_path = ?, file_size = ?, file_hash = ?,
file_path = ?, file_size = ?, file_hash = ?, deps = ?,
visibility = CASE WHEN status IN ('archived', 'deleted') THEN 'private' ELSE visibility END,
status = 'active', updated_at = ? WHERE id = ?`,
status = ?, updated_at = ? WHERE id = ?`,
p.Name, p.Description, version, fmJSON,
p.FilePath, p.FileSize, p.FileHash, now, existingID,
p.FilePath, p.FileSize, p.FileHash, depsJSON, status, now, existingID,
)
if err != nil {
return uuid.Nil, fmt.Errorf("update skill: %w", err)
@@ -145,10 +153,10 @@ func (s *SQLiteSkillStore) CreateSkillManaged(ctx context.Context, p store.Skill
// Insert new.
newID := store.GenNewID()
_, err = tx.ExecContext(ctx,
`INSERT INTO skills (id, name, slug, description, owner_id, tenant_id, visibility, version, status, frontmatter, file_path, file_size, file_hash, created_at, updated_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, 'active', ?, ?, ?, ?, ?, ?)`,
`INSERT INTO skills (id, name, slug, description, owner_id, tenant_id, visibility, version, status, deps, frontmatter, file_path, file_size, file_hash, created_at, updated_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
newID, p.Name, p.Slug, p.Description, p.OwnerID, tenantID, p.Visibility, version,
fmJSON, p.FilePath, p.FileSize, p.FileHash, now, now,
status, depsJSON, fmJSON, p.FilePath, p.FileSize, p.FileHash, now, now,
)
if err != nil {
return uuid.Nil, fmt.Errorf("insert skill: %w", err)
+83
View File
@@ -0,0 +1,83 @@
//go:build sqlite || sqliteonly
package sqlitestore
import (
"context"
"path/filepath"
"reflect"
"testing"
"github.com/nextlevelbuilder/goclaw/internal/store"
)
func TestSQLiteSkillStore_StoreMissingDeps_PersistsForCustomSkills(t *testing.T) {
ctx, skillStore := newTestSQLiteSkillStore(t)
skillID, err := skillStore.CreateSkillManaged(ctx, store.SkillCreateParams{
Name: "Custom Skill",
Slug: "custom-skill",
OwnerID: "user-1",
Visibility: "private",
FilePath: filepath.Join(t.TempDir(), "custom-skill", "1"),
})
if err != nil {
t.Fatalf("CreateSkillManaged error: %v", err)
}
missing := []string{"pip:requests", "npm:tsx"}
if err := skillStore.StoreMissingDeps(ctx, skillID, missing); err != nil {
t.Fatalf("StoreMissingDeps error: %v", err)
}
info, ok := skillStore.GetSkillByID(ctx, skillID)
if !ok {
t.Fatal("GetSkillByID returned !ok")
}
if !reflect.DeepEqual(info.MissingDeps, missing) {
t.Fatalf("MissingDeps = %v, want %v", info.MissingDeps, missing)
}
}
func TestSQLiteSkillStore_CreateSkillManaged_PersistsArchivedDependencyState(t *testing.T) {
ctx, skillStore := newTestSQLiteSkillStore(t)
missing := []string{"pip:requests"}
skillID, err := skillStore.CreateSkillManaged(ctx, store.SkillCreateParams{
Name: "Archived Skill",
Slug: "archived-skill",
OwnerID: "user-1",
Visibility: "private",
Status: "archived",
MissingDeps: missing,
FilePath: filepath.Join(t.TempDir(), "archived-skill", "1"),
})
if err != nil {
t.Fatalf("CreateSkillManaged error: %v", err)
}
info, ok := skillStore.GetSkillByID(ctx, skillID)
if !ok {
t.Fatal("GetSkillByID returned !ok")
}
if info.Status != "archived" {
t.Fatalf("Status = %q, want archived", info.Status)
}
if !reflect.DeepEqual(info.MissingDeps, missing) {
t.Fatalf("MissingDeps = %v, want %v", info.MissingDeps, missing)
}
}
func newTestSQLiteSkillStore(t *testing.T) (context.Context, *SQLiteSkillStore) {
t.Helper()
db, err := OpenDB(filepath.Join(t.TempDir(), "skills.db"))
if err != nil {
t.Fatalf("OpenDB error: %v", err)
}
t.Cleanup(func() { _ = db.Close() })
if err := EnsureSchema(db); err != nil {
t.Fatalf("EnsureSchema error: %v", err)
}
return store.WithTenantID(context.Background(), store.MasterTenantID), NewSQLiteSkillStore(db, t.TempDir())
}
@@ -48,9 +48,13 @@ func (s *stubAgentStore) MigrateUserDataOnMerge(_ context.Context, _ []string, _
}
// Remaining interface methods — not exercised in these tests.
func (s *stubAgentStore) Create(_ context.Context, _ *store.AgentData) error { return nil }
func (s *stubAgentStore) GetByKey(_ context.Context, _ string) (*store.AgentData, error) { return nil, nil }
func (s *stubAgentStore) GetByID(_ context.Context, _ uuid.UUID) (*store.AgentData, error) { return nil, nil }
func (s *stubAgentStore) Create(_ context.Context, _ *store.AgentData) error { return nil }
func (s *stubAgentStore) GetByKey(_ context.Context, _ string) (*store.AgentData, error) {
return nil, nil
}
func (s *stubAgentStore) GetByID(_ context.Context, _ uuid.UUID) (*store.AgentData, error) {
return nil, nil
}
func (s *stubAgentStore) GetByIDUnscoped(_ context.Context, _ uuid.UUID) (*store.AgentData, error) {
return nil, nil
}
@@ -60,10 +64,12 @@ func (s *stubAgentStore) GetByKeys(_ context.Context, _ []string) ([]store.Agent
func (s *stubAgentStore) GetByIDs(_ context.Context, _ []uuid.UUID) ([]store.AgentData, error) {
return nil, nil
}
func (s *stubAgentStore) GetDefault(_ context.Context) (*store.AgentData, error) { return nil, nil }
func (s *stubAgentStore) Update(_ context.Context, _ uuid.UUID, _ map[string]any) error { return nil }
func (s *stubAgentStore) Delete(_ context.Context, _ uuid.UUID) error { return nil }
func (s *stubAgentStore) List(_ context.Context, _ string) ([]store.AgentData, error) { return nil, nil }
func (s *stubAgentStore) GetDefault(_ context.Context) (*store.AgentData, error) { return nil, nil }
func (s *stubAgentStore) Update(_ context.Context, _ uuid.UUID, _ map[string]any) error { return nil }
func (s *stubAgentStore) Delete(_ context.Context, _ uuid.UUID) error { return nil }
func (s *stubAgentStore) List(_ context.Context, _ string) ([]store.AgentData, error) {
return nil, nil
}
func (s *stubAgentStore) ShareAgent(_ context.Context, _ uuid.UUID, _, _, _ string) error { return nil }
func (s *stubAgentStore) RevokeShare(_ context.Context, _ uuid.UUID, _ string) error { return nil }
func (s *stubAgentStore) ListShares(_ context.Context, _ uuid.UUID) ([]store.AgentShareData, error) {
+13 -1
View File
@@ -11,6 +11,18 @@ import type { SkillInfo, SkillFile, SkillVersions } from "@/types/skill";
export type { SkillInfo, SkillFile, SkillVersions };
export type SkillUploadResponse = {
id: string;
slug: string;
version: number;
name: string;
status?: string;
deps_warning?: string;
deps_errors?: string[];
missing_deps?: string[];
deps_installed?: boolean;
};
export function useSkills() {
const ws = useWs();
const http = useHttp();
@@ -50,7 +62,7 @@ export function useSkills() {
async (file: File) => {
const formData = new FormData();
formData.append("file", file);
const res = await http.upload<{ id: string; slug: string; version: number; name: string }>(
const res = await http.upload<SkillUploadResponse>(
"/v1/skills/upload",
formData,
);
@@ -1,6 +1,6 @@
import { useState, useRef } from "react";
import { useTranslation } from "react-i18next";
import { Upload, CheckCircle2, XCircle, Loader2, X } from "lucide-react";
import { Upload, CheckCircle2, XCircle, Loader2, TriangleAlert, X } from "lucide-react";
import {
Dialog,
DialogContent,
@@ -12,8 +12,9 @@ import {
import { Button } from "@/components/ui/button";
import { validateSkillZip } from "./lib/validate-skill-zip";
import { uniqueId } from "@/lib/utils";
import type { SkillUploadResponse } from "./hooks/use-skills";
type FileStatus = "validating" | "valid" | "invalid" | "uploading" | "success" | "error";
type FileStatus = "validating" | "valid" | "invalid" | "uploading" | "success" | "warning" | "error";
interface FileEntry {
id: string;
@@ -27,7 +28,7 @@ interface FileEntry {
interface SkillUploadDialogProps {
open: boolean;
onOpenChange: (open: boolean) => void;
onUpload: (file: File) => Promise<unknown>;
onUpload: (file: File) => Promise<SkillUploadResponse>;
}
export function SkillUploadDialog({ open, onOpenChange, onUpload }: SkillUploadDialogProps) {
@@ -93,9 +94,18 @@ export function SkillUploadDialog({ open, onOpenChange, onUpload }: SkillUploadD
prev.map((e) => (e.id === entry.id ? { ...e, status: "uploading" } : e)),
);
try {
await onUpload(entry.file);
const result = await onUpload(entry.file);
const detail = result.deps_warning
? result.deps_errors?.length
? `${result.deps_warning}: ${result.deps_errors.join("; ")}`
: result.deps_warning
: undefined;
setEntries((prev) =>
prev.map((e) => (e.id === entry.id ? { ...e, status: "success" } : e)),
prev.map((e) => (e.id === entry.id ? {
...e,
status: result.deps_warning ? "warning" : "success",
error: detail,
} : e)),
);
} catch (err) {
setEntries((prev) =>
@@ -136,7 +146,7 @@ export function SkillUploadDialog({ open, onOpenChange, onUpload }: SkillUploadD
};
const validCount = entries.filter((e) => e.status === "valid").length;
const successCount = entries.filter((e) => e.status === "success").length;
const successCount = entries.filter((e) => e.status === "success" || e.status === "warning").length;
return (
<Dialog open={open} onOpenChange={handleClose}>
@@ -248,6 +258,8 @@ function FileEntryRow({
</div>
{entry.status === "invalid" || entry.status === "error" ? (
<p className="text-xs text-destructive truncate">{entry.error ? t(entry.error) : t("upload.failed")}</p>
) : entry.status === "warning" ? (
<p className="text-xs text-amber-600 truncate">{entry.error ?? t("upload.failed")}</p>
) : entry.status === "validating" ? (
<p className="text-xs text-muted-foreground">{t("upload.validating")}</p>
) : entry.name && entry.status !== "success" ? (
@@ -278,6 +290,8 @@ function StatusIcon({ status }: { status: FileStatus }) {
return <CheckCircle2 className="h-4 w-4 shrink-0 text-primary" />;
case "success":
return <CheckCircle2 className="h-4 w-4 shrink-0 text-green-600" />;
case "warning":
return <TriangleAlert className="h-4 w-4 shrink-0 text-amber-600" />;
case "invalid":
case "error":
return <XCircle className="h-4 w-4 shrink-0 text-destructive" />;
+1 -2
View File
@@ -77,8 +77,7 @@ export function SkillsPage() {
};
const handleUpload = async (file: File) => {
await uploadSkill(file);
refresh();
return uploadSkill(file);
};
const handleCycleVisibility = async (skill: SkillInfo) => {