From 3fe0633d35ed616b2a02a9309ba1923ceda09559 Mon Sep 17 00:00:00 2001 From: "Kai (Tam Nhu) Tran" <61256810+kaitranntt@users.noreply.github.com> Date: Mon, 30 Mar 2026 10:58:29 -0400 Subject: [PATCH] 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 --- internal/http/skills.go | 16 +- internal/http/skills_upload.go | 189 ++++++- internal/http/skills_upload_test.go | 531 ++++++++++++++++++ internal/store/pg/skills.go | 12 +- internal/store/pg/skills_crud.go | 18 +- internal/store/skill_store.go | 1 + internal/store/sqlitestore/skills.go | 14 +- internal/store/sqlitestore/skills_crud.go | 20 +- internal/store/sqlitestore/skills_test.go | 83 +++ .../tools/context_file_interceptor_test.go | 20 +- ui/web/src/pages/skills/hooks/use-skills.ts | 14 +- .../src/pages/skills/skill-upload-dialog.tsx | 26 +- ui/web/src/pages/skills/skills-page.tsx | 3 +- 13 files changed, 889 insertions(+), 58 deletions(-) create mode 100644 internal/http/skills_upload_test.go create mode 100644 internal/store/sqlitestore/skills_test.go diff --git a/internal/http/skills.go b/internal/http/skills.go index f1df671e..872303ac 100644 --- a/internal/http/skills.go +++ b/internal/http/skills.go @@ -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 diff --git a/internal/http/skills_upload.go b/internal/http/skills_upload.go index 9d53f539..26a95a74 100644 --- a/internal/http/skills_upload.go +++ b/internal/http/skills_upload.go @@ -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) +} diff --git a/internal/http/skills_upload_test.go b/internal/http/skills_upload_test.go new file mode 100644 index 00000000..87108c42 --- /dev/null +++ b/internal/http/skills_upload_test.go @@ -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 ©, 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 +} diff --git a/internal/store/pg/skills.go b/internal/store/pg/skills.go index 188bc186..1e550575 100644 --- a/internal/store/pg/skills.go +++ b/internal/store/pg/skills.go @@ -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}) +} diff --git a/internal/store/pg/skills_crud.go b/internal/store/pg/skills_crud.go index 28e742f5..082afd74 100644 --- a/internal/store/pg/skills_crud.go +++ b/internal/store/pg/skills_crud.go @@ -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) diff --git a/internal/store/skill_store.go b/internal/store/skill_store.go index 8624a3db..ea59d3d9 100644 --- a/internal/store/skill_store.go +++ b/internal/store/skill_store.go @@ -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 diff --git a/internal/store/sqlitestore/skills.go b/internal/store/sqlitestore/skills.go index c0327209..f2cc1bfe 100644 --- a/internal/store/sqlitestore/skills.go +++ b/internal/store/sqlitestore/skills.go @@ -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 { diff --git a/internal/store/sqlitestore/skills_crud.go b/internal/store/sqlitestore/skills_crud.go index ba46eb2c..21bf50f2 100644 --- a/internal/store/sqlitestore/skills_crud.go +++ b/internal/store/sqlitestore/skills_crud.go @@ -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) diff --git a/internal/store/sqlitestore/skills_test.go b/internal/store/sqlitestore/skills_test.go new file mode 100644 index 00000000..d72ba7c9 --- /dev/null +++ b/internal/store/sqlitestore/skills_test.go @@ -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()) +} diff --git a/internal/tools/context_file_interceptor_test.go b/internal/tools/context_file_interceptor_test.go index bf2bb5a8..cbbf7020 100644 --- a/internal/tools/context_file_interceptor_test.go +++ b/internal/tools/context_file_interceptor_test.go @@ -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) { diff --git a/ui/web/src/pages/skills/hooks/use-skills.ts b/ui/web/src/pages/skills/hooks/use-skills.ts index 9faf7094..68e727b5 100644 --- a/ui/web/src/pages/skills/hooks/use-skills.ts +++ b/ui/web/src/pages/skills/hooks/use-skills.ts @@ -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( "/v1/skills/upload", formData, ); diff --git a/ui/web/src/pages/skills/skill-upload-dialog.tsx b/ui/web/src/pages/skills/skill-upload-dialog.tsx index 5bb29d71..1e32297a 100644 --- a/ui/web/src/pages/skills/skill-upload-dialog.tsx +++ b/ui/web/src/pages/skills/skill-upload-dialog.tsx @@ -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; + onUpload: (file: File) => Promise; } 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 ( @@ -248,6 +258,8 @@ function FileEntryRow({ {entry.status === "invalid" || entry.status === "error" ? (

{entry.error ? t(entry.error) : t("upload.failed")}

+ ) : entry.status === "warning" ? ( +

{entry.error ?? t("upload.failed")}

) : entry.status === "validating" ? (

{t("upload.validating")}

) : entry.name && entry.status !== "success" ? ( @@ -278,6 +290,8 @@ function StatusIcon({ status }: { status: FileStatus }) { return ; case "success": return ; + case "warning": + return ; case "invalid": case "error": return ; diff --git a/ui/web/src/pages/skills/skills-page.tsx b/ui/web/src/pages/skills/skills-page.tsx index 2da2d28c..193ddfde 100644 --- a/ui/web/src/pages/skills/skills-page.tsx +++ b/ui/web/src/pages/skills/skills-page.tsx @@ -77,8 +77,7 @@ export function SkillsPage() { }; const handleUpload = async (file: File) => { - await uploadSkill(file); - refresh(); + return uploadSkill(file); }; const handleCycleVisibility = async (skill: SkillInfo) => {