From 80893a1e6c7366d42b9de359a0adfaf2ecd710ab Mon Sep 17 00:00:00 2001 From: Duy /zuey/ Date: Sun, 31 May 2026 12:44:29 +0700 Subject: [PATCH] feat(skills): add selected skill downloads Squash merge PR #110 after resolving dev changelog conflict. Local checks covered Go http/store, full web test/build; PR CI run 26704448786 passed release-versioning, go, and web. --- docs/18-http-api.md | 16 + docs/project-changelog.md | 29 ++ internal/http/agents_export_handlers.go | 9 +- internal/http/skills_export.go | 337 ++++++++++++++++-- internal/http/skills_export_test.go | 192 ++++++++++ internal/store/pg/skills_export_queries.go | 40 ++- .../store/pg/skills_export_queries_test.go | 83 +++++ internal/store/pg/skills_scan_rows.go | 2 + ...hase-01-api-contract-and-export-writers.md | 152 ++++++++ .../phase-02-skills-ui-download-actions.md | 141 ++++++++ ...regression-validation-and-issue-handoff.md | 132 +++++++ .../plan.md | 95 +++++ ui/web/src/i18n/locales/en/skills.json | 8 + ui/web/src/i18n/locales/vi/skills.json | 8 + ui/web/src/i18n/locales/zh/skills.json | 8 + ui/web/src/pages/skills/hooks/use-skills.ts | 21 +- .../skills/lib/skill-export-download.test.ts | 41 +++ .../pages/skills/lib/skill-export-download.ts | 58 +++ .../skills/skill-bulk-actions-toolbar.tsx | 38 +- .../src/pages/skills/skill-detail-dialog.tsx | 63 ++-- ui/web/src/pages/skills/skills-page.tsx | 25 +- 21 files changed, 1438 insertions(+), 60 deletions(-) create mode 100644 internal/http/skills_export_test.go create mode 100644 internal/store/pg/skills_export_queries_test.go create mode 100644 plans/260528-1759-skill-download-and-selective-export-web-ui/phase-01-api-contract-and-export-writers.md create mode 100644 plans/260528-1759-skill-download-and-selective-export-web-ui/phase-02-skills-ui-download-actions.md create mode 100644 plans/260528-1759-skill-download-and-selective-export-web-ui/phase-03-regression-validation-and-issue-handoff.md create mode 100644 plans/260528-1759-skill-download-and-selective-export-web-ui/plan.md create mode 100644 ui/web/src/pages/skills/lib/skill-export-download.test.ts create mode 100644 ui/web/src/pages/skills/lib/skill-export-download.ts diff --git a/docs/18-http-api.md b/docs/18-http-api.md index e5227b98..fbabdba6 100644 --- a/docs/18-http-api.md +++ b/docs/18-http-api.md @@ -376,6 +376,22 @@ The default prefix is `/`; supported prompt forms are `/ prompt`, | `GET` | `/v1/skills/export` | Export skills bundle | | `POST` | `/v1/skills/import` | Import skills bundle | +`GET /v1/skills/export` supports direct download and the existing SSE token flow: + +| Query | Description | +|-------|-------------| +| `stream=true` | Start the existing SSE export flow and return a temporary `download_url` | +| `format=tar.gz\|tgz\|zip` | Archive format; defaults to `tar.gz`. `tgz` is an alias for gzip tar output | +| `id=` | Select one skill ID. Can be repeated for multiple selected skills | +| `ids=,` | Select multiple skill IDs as a comma-separated list | +| `include_system=true` | Include system skills in a full export when no selected IDs are provided | + +Without `id` or `ids`, export remains backward-compatible and includes tenant +custom skills by default. Explicitly selected IDs may include system/core skills +without `include_system=true`; tenant-scoped custom skills are still filtered by +the request tenant. ZIP support is export/download only; the import endpoint +continues to accept the existing skills bundle format. + --- ## 6. Providers diff --git a/docs/project-changelog.md b/docs/project-changelog.md index 029f674c..51fc006c 100644 --- a/docs/project-changelog.md +++ b/docs/project-changelog.md @@ -6,6 +6,35 @@ Significant changes, features, and fixes in reverse chronological order. ## 2026-05-29 +### Skill selected download export (issue #80) + +**Features** + +- Extended `GET /v1/skills/export` with selected skill IDs via repeated `id` + or comma-separated `ids`, while preserving the no-selection full export as + tenant custom-skill only by default. +- Added archive format selection: `tar.gz`, `tgz` alias, and `zip`. Direct and + SSE download paths now return matching archive filenames and content types. +- Selected exports can include system/core skills explicitly without changing + full backup defaults. +- Archive output now preserves skill directory content such as `SKILL.md`, + `references/`, `scripts/`, and `assets/`, while skipping unsafe paths and + symlinks. Nested files named `metadata.json` or `grants.jsonl` remain + exportable; only generated root archive artifacts are skipped. +- Archive assembly streams file contents into the writer and revalidates opened + files against the resolved skill root to avoid large in-memory reads and + symlink-swap escapes. +- Web Skills page now supports Download from the detail dialog and Download + selected from the bulk toolbar, with format selection and EN/VI/ZH labels. + +**Tests** + +- Added backend coverage for export request parsing, archive writers, safe + directory walking, selected system/custom scope, and full export defaults. +- Added web helper coverage for selected export URLs and archive filenames. + +--- + ### Discord thread history and attachment backfill (issue #69) **Fixes** diff --git a/internal/http/agents_export_handlers.go b/internal/http/agents_export_handlers.go index 24348b47..c5431ad2 100644 --- a/internal/http/agents_export_handlers.go +++ b/internal/http/agents_export_handlers.go @@ -119,11 +119,18 @@ func (h *AgentsHandler) handleExportDownload(w http.ResponseWriter, r *http.Requ } defer f.Close() - w.Header().Set("Content-Type", "application/gzip") + w.Header().Set("Content-Type", exportDownloadContentType(entry.fileName)) w.Header().Set("Content-Disposition", fmt.Sprintf(`attachment; filename="%s"`, entry.fileName)) io.Copy(w, f) //nolint:errcheck } +func exportDownloadContentType(fileName string) string { + if strings.HasSuffix(strings.ToLower(fileName), ".zip") { + return "application/zip" + } + return "application/gzip" +} + // handleExportSSE streams build progress as SSE events then sends a download token on completion. func (h *AgentsHandler) handleExportSSE(w http.ResponseWriter, r *http.Request, ag *store.AgentData, sections map[string]bool) { flusher := initSSE(w) diff --git a/internal/http/skills_export.go b/internal/http/skills_export.go index 8f8a1e23..b2aa5d61 100644 --- a/internal/http/skills_export.go +++ b/internal/http/skills_export.go @@ -2,14 +2,19 @@ package http import ( "archive/tar" + "archive/zip" + "bytes" "compress/gzip" "context" "database/sql" + "errors" "fmt" "io" "log/slog" "net/http" "os" + "path/filepath" + "strings" "time" "github.com/google/uuid" @@ -52,8 +57,23 @@ func (h *SkillsHandler) handleSkillsExport(w http.ResponseWriter, r *http.Reques return } + exportReq, err := parseSkillExportRequest(r) + if err != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": err.Error()}) + return + } + skills, err := pg.ExportSkills(r.Context(), h.db, pg.SkillExportSelection{ + IDs: exportReq.IDs, + IncludeSystem: exportReq.IncludeSystem, + }) + if err != nil { + slog.Error("skills.export.query", "error", err) + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgInternalError)}) + return + } + stream := r.URL.Query().Get("stream") == "true" - fileName := fmt.Sprintf("skills-%s.tar.gz", time.Now().UTC().Format("20060102")) + fileName := skillExportFileName(skills, exportReq.Format, time.Now().UTC()) if stream { flusher := initSSE(w) @@ -62,7 +82,7 @@ func (h *SkillsHandler) handleSkillsExport(w http.ResponseWriter, r *http.Reques return } - tmpFile, err := os.CreateTemp("", "goclaw-skills-export-*.tar.gz") + tmpFile, err := os.CreateTemp("", "goclaw-skills-export-*"+exportReq.Format.Extension) if err != nil { sendSSE(w, flusher, "error", ProgressEvent{Phase: "init", Status: "error", Detail: "failed to create temp file"}) return @@ -70,7 +90,7 @@ func (h *SkillsHandler) handleSkillsExport(w http.ResponseWriter, r *http.Reques tmpPath := tmpFile.Name() progressFn := func(ev ProgressEvent) { sendSSE(w, flusher, "progress", ev) } - buildErr := h.writeSkillsExportArchive(r.Context(), tmpFile, progressFn) + buildErr := h.writeSkillsExportArchive(r.Context(), tmpFile, progressFn, exportReq, skills) tmpFile.Close() if buildErr != nil { @@ -83,29 +103,26 @@ func (h *SkillsHandler) handleSkillsExport(w http.ResponseWriter, r *http.Reques token := storeExportToken("skills", userID, tmpPath, fileName) sendSSE(w, flusher, "complete", map[string]string{ "download_url": "/v1/export/download/" + token, + "file_name": fileName, }) return } - w.Header().Set("Content-Type", "application/gzip") + w.Header().Set("Content-Type", exportReq.Format.ContentType) w.Header().Set("Content-Disposition", fmt.Sprintf(`attachment; filename="%s"`, fileName)) - if err := h.writeSkillsExportArchive(r.Context(), w, nil); err != nil { + if err := h.writeSkillsExportArchive(r.Context(), w, nil, exportReq, skills); err != nil { slog.Error("skills.export.direct", "error", err) } } -// writeSkillsExportArchive builds the skills tar.gz: skills/{slug}/metadata.json + SKILL.md + grants.jsonl per skill. -func (h *SkillsHandler) writeSkillsExportArchive(ctx context.Context, w io.Writer, progressFn func(ProgressEvent)) error { +// writeSkillsExportArchive builds the skills archive. +func (h *SkillsHandler) writeSkillsExportArchive(ctx context.Context, w io.Writer, progressFn func(ProgressEvent), req skillExportRequest, skills []pg.CustomSkillExport) error { lw := &limitedWriter{w: w, limit: maxExportSize} - gw := gzip.NewWriter(lw) - tw := tar.NewWriter(gw) - - skills, err := pg.ExportCustomSkills(ctx, h.db) + archive, err := newSkillArchiveWriter(lw, req.Format) if err != nil { - tw.Close() - gw.Close() - return fmt.Errorf("query custom skills: %w", err) + return err } + defer archive.Close() for i, sk := range skills { if progressFn != nil { @@ -138,21 +155,13 @@ func (h *SkillsHandler) writeSkillsExportArchive(ctx context.Context, w io.Write slog.Warn("skills.export: marshal metadata", "slug", sk.Slug, "error", err) continue } - if err := addToTar(tw, prefix+"metadata.json", metaJSON); err != nil { - tw.Close() - gw.Close() + if err := archive.AddFile(prefix+"metadata.json", metaJSON); err != nil { return fmt.Errorf("write %smetadata.json: %w", prefix, err) } - // SKILL.md — read from filesystem - if sk.FilePath != "" { - fullPath := config.ExpandHome(store.SkillMarkdownPath(sk.FilePath)) - if data, err := os.ReadFile(fullPath); err == nil { - if err := addToTar(tw, prefix+"SKILL.md", data); err != nil { - slog.Warn("skills.export: write SKILL.md", "slug", sk.Slug, "error", err) - } - } else { - slog.Warn("skills.export: read SKILL.md", "slug", sk.Slug, "path", fullPath, "error", err) + for _, root := range h.skillExportRoots(sk) { + if err := addSkillDirectoryToArchive(archive, root, prefix); err != nil { + slog.Warn("skills.export: write skill directory", "slug", sk.Slug, "root", root, "error", err) } } @@ -170,9 +179,7 @@ func (h *SkillsHandler) writeSkillsExportArchive(ctx context.Context, w io.Write data, err := marshalJSONL(grants) if err != nil { slog.Warn("skills.export: marshal grants", "slug", sk.Slug, "error", err) - } else if err := addToTar(tw, prefix+"grants.jsonl", data); err != nil { - tw.Close() - gw.Close() + } else if err := archive.AddFile(prefix+"grants.jsonl", data); err != nil { return fmt.Errorf("write %sgrants.jsonl: %w", prefix, err) } } @@ -182,9 +189,273 @@ func (h *SkillsHandler) writeSkillsExportArchive(ctx context.Context, w io.Write progressFn(ProgressEvent{Phase: "skills", Status: "done", Current: len(skills), Total: len(skills), Detail: fmt.Sprintf("%d skills exported", len(skills))}) } - if err := tw.Close(); err != nil { - gw.Close() - return fmt.Errorf("close tar: %w", err) + return archive.Close() +} + +func (h *SkillsHandler) skillExportRoots(sk pg.CustomSkillExport) []string { + if sk.FilePath == "" { + return nil } - return gw.Close() + root := config.ExpandHome(store.SkillBaseDir(sk.FilePath)) + return readableSkillRoots(root, sk.Slug, sk.IsSystem, h.bundledDir) +} + +type skillExportFormat struct { + Canonical string + Extension string + ContentType string +} + +type skillExportRequest struct { + Format skillExportFormat + IDs []uuid.UUID + IncludeSystem bool +} + +func parseSkillExportRequest(r *http.Request) (skillExportRequest, error) { + format, err := parseSkillExportFormat(r.URL.Query().Get("format")) + if err != nil { + return skillExportRequest{}, err + } + ids, err := parseSkillExportIDs(r) + if err != nil { + return skillExportRequest{}, err + } + return skillExportRequest{ + Format: format, + IDs: ids, + IncludeSystem: strings.EqualFold(r.URL.Query().Get("include_system"), "true"), + }, nil +} + +func parseSkillExportFormat(raw string) (skillExportFormat, error) { + switch strings.ToLower(strings.TrimSpace(raw)) { + case "", "tar.gz", "tgz": + return skillExportFormat{Canonical: "tar.gz", Extension: ".tar.gz", ContentType: "application/gzip"}, nil + case "zip": + return skillExportFormat{Canonical: "zip", Extension: ".zip", ContentType: "application/zip"}, nil + default: + return skillExportFormat{}, fmt.Errorf("unsupported skills export format %q", raw) + } +} + +func parseSkillExportIDs(r *http.Request) ([]uuid.UUID, error) { + raw := append([]string{}, r.URL.Query()["id"]...) + raw = append(raw, r.URL.Query()["ids"]...) + var ids []uuid.UUID + seen := map[uuid.UUID]bool{} + for _, group := range raw { + for part := range strings.SplitSeq(group, ",") { + part = strings.TrimSpace(part) + if part == "" { + continue + } + id, err := uuid.Parse(part) + if err != nil { + return nil, fmt.Errorf("invalid skill id %q", part) + } + if !seen[id] { + seen[id] = true + ids = append(ids, id) + } + } + } + return ids, nil +} + +type skillArchiveWriter interface { + AddFile(name string, data []byte) error + AddFileReader(name string, size int64, modTime time.Time, r io.Reader) error + Close() error + ContentType() string + Extension() string +} + +func newSkillArchiveWriter(w io.Writer, format skillExportFormat) (skillArchiveWriter, error) { + switch format.Canonical { + case "zip": + return &skillZipArchiveWriter{zw: zip.NewWriter(w)}, nil + case "tar.gz": + gw := gzip.NewWriter(w) + return &skillTarGzArchiveWriter{gw: gw, tw: tar.NewWriter(gw)}, nil + default: + return nil, fmt.Errorf("unsupported skills export format %q", format.Canonical) + } +} + +type skillTarGzArchiveWriter struct { + gw *gzip.Writer + tw *tar.Writer + closed bool +} + +func (w *skillTarGzArchiveWriter) AddFile(name string, data []byte) error { + return w.AddFileReader(name, int64(len(data)), time.Now(), bytes.NewReader(data)) +} + +func (w *skillTarGzArchiveWriter) AddFileReader(name string, size int64, modTime time.Time, r io.Reader) error { + if err := validateArchivePath(name); err != nil { + return err + } + hdr := &tar.Header{ + Name: name, + Mode: 0o644, + Size: size, + ModTime: modTime, + } + if err := w.tw.WriteHeader(hdr); err != nil { + return err + } + _, err := io.Copy(w.tw, r) + return err +} + +func (w *skillTarGzArchiveWriter) Close() error { + if w.closed { + return nil + } + w.closed = true + if err := w.tw.Close(); err != nil { + w.gw.Close() + return err + } + return w.gw.Close() +} + +func (w *skillTarGzArchiveWriter) ContentType() string { return "application/gzip" } +func (w *skillTarGzArchiveWriter) Extension() string { return ".tar.gz" } + +type skillZipArchiveWriter struct { + zw *zip.Writer + closed bool +} + +func (w *skillZipArchiveWriter) AddFile(name string, data []byte) error { + return w.AddFileReader(name, int64(len(data)), time.Now(), bytes.NewReader(data)) +} + +func (w *skillZipArchiveWriter) AddFileReader(name string, size int64, modTime time.Time, r io.Reader) error { + if err := validateArchivePath(name); err != nil { + return err + } + hdr := &zip.FileHeader{Name: name, Method: zip.Deflate} + hdr.SetModTime(modTime) + hdr.UncompressedSize64 = uint64(size) + fw, err := w.zw.CreateHeader(hdr) + if err != nil { + return err + } + _, err = io.Copy(fw, r) + return err +} + +func (w *skillZipArchiveWriter) Close() error { + if w.closed { + return nil + } + w.closed = true + return w.zw.Close() +} + +func (w *skillZipArchiveWriter) ContentType() string { return "application/zip" } +func (w *skillZipArchiveWriter) Extension() string { return ".zip" } + +func addSkillDirectoryToArchive(archive skillArchiveWriter, root, prefix string) error { + root = filepath.Clean(root) + if root == "." || root == "" { + return nil + } + rootReal, err := filepath.EvalSymlinks(root) + if err != nil { + return nil + } + rootReal = filepath.Clean(rootReal) + return filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { + if err != nil { + return nil + } + rel, err := filepath.Rel(root, path) + if err != nil || rel == "." { + return nil + } + rel = filepath.ToSlash(rel) + if skillsExportArtifact(rel) { + if d.IsDir() { + return filepath.SkipDir + } + return nil + } + if d.IsDir() || d.Type()&os.ModeSymlink != 0 { + return nil + } + archivePath := prefix + sanitizeRelPath(rel) + if archivePath == prefix { + return nil + } + return addValidatedSkillFileToArchive(archive, rootReal, path, archivePath) + }) +} + +func skillsExportArtifact(rel string) bool { + name := filepath.Base(rel) + return name == ".DS_Store" || name == "Thumbs.db" || rel == "metadata.json" || rel == "grants.jsonl" +} + +func addValidatedSkillFileToArchive(archive skillArchiveWriter, rootReal, path, archivePath string) error { + file, err := os.Open(path) + if err != nil { + return nil + } + defer file.Close() + + realPath, err := filepath.EvalSymlinks(path) + if err != nil { + slog.Warn("security.skills_export_path_unresolved", "path", path, "error", err) + return nil + } + realPath = filepath.Clean(realPath) + if !pathWithinDir(realPath, rootReal) || hasDeniedFilePrefix(realPath) { + slog.Warn("security.skills_export_path_escape", "path", path, "resolved", realPath, "root", rootReal) + return nil + } + + realInfo, err := os.Stat(realPath) + if err != nil { + return nil + } + fileInfo, err := file.Stat() + if err != nil { + slog.Warn("security.skills_export_open_race", "path", realPath, "error", err) + return nil + } + if fileInfo.IsDir() || realInfo.IsDir() || !fileInfo.Mode().IsRegular() || !os.SameFile(realInfo, fileInfo) { + slog.Warn("security.skills_export_open_race", "path", realPath) + return nil + } + + return archive.AddFileReader(archivePath, fileInfo.Size(), fileInfo.ModTime(), file) +} + +func validateArchivePath(name string) error { + if name == "" || strings.ContainsRune(name, 0) || strings.HasPrefix(name, "/") || filepath.IsAbs(name) { + return errors.New("invalid archive path") + } + name = filepath.ToSlash(name) + if strings.Contains(name, "\\") || strings.Contains(name, ":") { + return errors.New("invalid archive path") + } + for part := range strings.SplitSeq(name, "/") { + if part == "" || part == "." || part == ".." { + return errors.New("invalid archive path") + } + } + return nil +} + +func skillExportFileName(skills []pg.CustomSkillExport, format skillExportFormat, now time.Time) string { + if len(skills) == 1 { + sk := skills[0] + return fmt.Sprintf("goclaw-skill-%s-v%d%s", sanitizeName(sk.Slug), sk.Version, format.Extension) + } + return fmt.Sprintf("goclaw-skills-export-%s%s", now.Format("20060102-1504"), format.Extension) } diff --git a/internal/http/skills_export_test.go b/internal/http/skills_export_test.go new file mode 100644 index 00000000..868d3291 --- /dev/null +++ b/internal/http/skills_export_test.go @@ -0,0 +1,192 @@ +package http + +import ( + "archive/tar" + "archive/zip" + "bytes" + "compress/gzip" + "io" + "net/http/httptest" + "os" + "path/filepath" + "slices" + "testing" +) + +func TestParseSkillExportRequestFormatsAndIDs(t *testing.T) { + req := httptest.NewRequest("GET", "/v1/skills/export?format=tgz&id=11111111-1111-1111-1111-111111111111&ids=22222222-2222-2222-2222-222222222222,33333333-3333-3333-3333-333333333333&include_system=true", nil) + + parsed, err := parseSkillExportRequest(req) + if err != nil { + t.Fatalf("parseSkillExportRequest() error = %v", err) + } + if parsed.Format.Canonical != "tar.gz" || parsed.Format.Extension != ".tar.gz" { + t.Fatalf("format = %#v, want tar.gz alias", parsed.Format) + } + if len(parsed.IDs) != 3 { + t.Fatalf("ids = %v, want 3 ids", parsed.IDs) + } + if !parsed.IncludeSystem { + t.Fatal("include_system was not parsed") + } +} + +func TestParseSkillExportRequestRejectsUnsupportedFormat(t *testing.T) { + req := httptest.NewRequest("GET", "/v1/skills/export?format=rar", nil) + + if _, err := parseSkillExportRequest(req); err == nil { + t.Fatal("parseSkillExportRequest() error = nil, want unsupported format error") + } +} + +func TestSkillExportArchiveWriters(t *testing.T) { + for _, tc := range []struct { + name string + format string + contentTyp string + wantEntry func(t *testing.T, data []byte) + }{ + { + name: "zip", + format: "zip", + contentTyp: "application/zip", + wantEntry: func(t *testing.T, data []byte) { + t.Helper() + zr, err := zip.NewReader(bytes.NewReader(data), int64(len(data))) + if err != nil { + t.Fatalf("zip.NewReader: %v", err) + } + names := make([]string, 0, len(zr.File)) + for _, f := range zr.File { + names = append(names, f.Name) + } + if !slices.Contains(names, "skills/demo/SKILL.md") { + t.Fatalf("zip entries = %v, want SKILL.md", names) + } + }, + }, + { + name: "tar.gz", + format: "tar.gz", + contentTyp: "application/gzip", + wantEntry: func(t *testing.T, data []byte) { + t.Helper() + gr, err := gzip.NewReader(bytes.NewReader(data)) + if err != nil { + t.Fatalf("gzip.NewReader: %v", err) + } + defer gr.Close() + tr := tar.NewReader(gr) + for { + hdr, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + t.Fatalf("tar.Next: %v", err) + } + if hdr.Name == "skills/demo/SKILL.md" { + return + } + } + t.Fatal("tar missing skills/demo/SKILL.md") + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + var buf bytes.Buffer + writer, err := newSkillArchiveWriter(&buf, mustSkillExportFormat(t, tc.format)) + if err != nil { + t.Fatalf("newSkillArchiveWriter() error = %v", err) + } + if writer.ContentType() != tc.contentTyp { + t.Fatalf("ContentType() = %q, want %q", writer.ContentType(), tc.contentTyp) + } + if err := writer.AddFile("skills/demo/SKILL.md", []byte("# Demo")); err != nil { + t.Fatalf("AddFile: %v", err) + } + if err := writer.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + tc.wantEntry(t, buf.Bytes()) + }) + } +} + +func TestAddSkillDirectoryToArchiveSkipsSymlinksAndResources(t *testing.T) { + root := t.TempDir() + mustWriteFile(t, filepath.Join(root, "SKILL.md"), "# Demo") + mustWriteFile(t, filepath.Join(root, "references", "guide.md"), "guide") + mustWriteFile(t, filepath.Join(root, "references", "grants.jsonl"), "nested grants") + mustWriteFile(t, filepath.Join(root, "scripts", "run.sh"), "#!/bin/sh") + mustWriteFile(t, filepath.Join(root, "assets", "logo.txt"), "logo") + mustWriteFile(t, filepath.Join(root, "assets", "metadata.json"), "nested metadata") + mustWriteFile(t, filepath.Join(root, ".DS_Store"), "junk") + mustWriteFile(t, filepath.Join(root, "metadata.json"), "generated metadata placeholder") + mustWriteFile(t, filepath.Join(root, "grants.jsonl"), "generated grants placeholder") + if err := os.Symlink(filepath.Join(root, "SKILL.md"), filepath.Join(root, "linked.md")); err != nil { + t.Fatalf("symlink: %v", err) + } + + var buf bytes.Buffer + writer, err := newSkillArchiveWriter(&buf, mustSkillExportFormat(t, "zip")) + if err != nil { + t.Fatalf("newSkillArchiveWriter() error = %v", err) + } + if err := addSkillDirectoryToArchive(writer, root, "skills/demo/"); err != nil { + t.Fatalf("addSkillDirectoryToArchive() error = %v", err) + } + if err := writer.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + + zr, err := zip.NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len())) + if err != nil { + t.Fatalf("zip.NewReader: %v", err) + } + names := make([]string, 0, len(zr.File)) + for _, f := range zr.File { + names = append(names, f.Name) + } + for _, want := range []string{ + "skills/demo/SKILL.md", + "skills/demo/references/guide.md", + "skills/demo/references/grants.jsonl", + "skills/demo/scripts/run.sh", + "skills/demo/assets/logo.txt", + "skills/demo/assets/metadata.json", + } { + if !slices.Contains(names, want) { + t.Fatalf("entries = %v, missing %s", names, want) + } + } + for _, skipped := range []string{ + "skills/demo/.DS_Store", + "skills/demo/linked.md", + "skills/demo/metadata.json", + "skills/demo/grants.jsonl", + } { + if slices.Contains(names, skipped) { + t.Fatalf("entries include skipped artifact %s: %v", skipped, names) + } + } +} + +func mustSkillExportFormat(t *testing.T, raw string) skillExportFormat { + t.Helper() + format, err := parseSkillExportFormat(raw) + if err != nil { + t.Fatalf("parseSkillExportFormat(%q): %v", raw, err) + } + return format +} + +func mustWriteFile(t *testing.T, path, content string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + t.Fatalf("mkdir %s: %v", path, err) + } + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatalf("write %s: %v", path, err) + } +} diff --git a/internal/store/pg/skills_export_queries.go b/internal/store/pg/skills_export_queries.go index d8dbee14..aa8b903e 100644 --- a/internal/store/pg/skills_export_queries.go +++ b/internal/store/pg/skills_export_queries.go @@ -6,6 +6,9 @@ import ( "encoding/json" "github.com/google/uuid" + "github.com/lib/pq" + + "github.com/nextlevelbuilder/goclaw/internal/store" ) // CustomSkillExport holds portable skill metadata (no internal UUIDs in references). @@ -16,6 +19,7 @@ type CustomSkillExport struct { Description *string `json:"description,omitempty" db:"description"` Visibility string `json:"visibility" db:"visibility"` Version int `json:"version" db:"version"` + IsSystem bool `json:"is_system" db:"is_system"` Frontmatter json.RawMessage `json:"frontmatter,omitempty" db:"frontmatter"` Tags []string `json:"tags,omitempty" db:"tags"` Deps json.RawMessage `json:"deps,omitempty" db:"deps"` @@ -34,18 +38,50 @@ type SkillsExportPreview struct { TotalGrants int `json:"total_grants" db:"total_grants"` } +// SkillExportSelection describes the skill set requested by the export API. +type SkillExportSelection struct { + IDs []uuid.UUID + IncludeSystem bool +} + // ExportCustomSkills returns all non-system skills scoped to the current tenant. func ExportCustomSkills(ctx context.Context, db *sql.DB) ([]CustomSkillExport, error) { + return ExportSkills(ctx, db, SkillExportSelection{}) +} + +// ExportSkills returns exportable skill rows for the requested selection. +// No IDs preserves the legacy custom-skills-only export unless IncludeSystem is true. +// Explicit IDs may include system skills, while custom skills remain tenant-scoped. +func ExportSkills(ctx context.Context, db *sql.DB, selection SkillExportSelection) ([]CustomSkillExport, error) { tc, tcArgs, _, err := scopeClause(ctx, 1) if err != nil { return nil, err } + where := " WHERE is_system = false" + tc + args := tcArgs + if len(selection.IDs) > 0 { + if store.IsCrossTenant(ctx) { + where = " WHERE id = ANY($1)" + args = []any{pq.Array(selection.IDs)} + } else { + scope, err := store.ScopeFromContext(ctx) + if err != nil { + return nil, err + } + where = " WHERE id = ANY($1) AND (is_system = true OR tenant_id = $2)" + args = []any{pq.Array(selection.IDs), scope.TenantID} + } + } else if selection.IncludeSystem { + where = " WHERE (is_system = true OR (is_system = false" + tc + "))" + args = tcArgs + } var scanned []customSkillExportRow if err := pkgSqlxDB.SelectContext(ctx, &scanned, "SELECT id, name, slug, description, visibility, version, frontmatter, tags, deps, file_path"+ - " FROM skills WHERE is_system = false"+tc+ + ", is_system"+ + " FROM skills"+where+ " ORDER BY name", - tcArgs..., + args..., ); err != nil { return nil, err } diff --git a/internal/store/pg/skills_export_queries_test.go b/internal/store/pg/skills_export_queries_test.go new file mode 100644 index 00000000..4991a61d --- /dev/null +++ b/internal/store/pg/skills_export_queries_test.go @@ -0,0 +1,83 @@ +package pg + +import ( + "database/sql" + "slices" + "testing" + + "github.com/google/uuid" + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +func TestExportSkillsSelectionIncludesExplicitSystemAndTenantCustom(t *testing.T) { + db := hooksTestDB(t) + tenantA, _ := seedTenantAndAgent(t, db) + tenantB, _ := seedTenantAndAgent(t, db) + customA := seedExportSkill(t, db, tenantA, "custom-a", false) + customB := seedExportSkill(t, db, tenantB, "custom-b", false) + system := seedExportSkill(t, db, tenantB, "system-global", true) + + ctx := store.WithTenantID(t.Context(), tenantA) + got, err := ExportSkills(ctx, db, SkillExportSelection{ + IDs: []uuid.UUID{customA, customB, system}, + }) + if err != nil { + t.Fatalf("ExportSkills() error = %v", err) + } + slugs := exportSkillSlugs(got) + if !containsSlug(slugs, "system-global") || !containsSlug(slugs, "custom-a") { + t.Fatalf("slugs = %v, want tenant custom + explicit system", slugs) + } + if containsSlug(slugs, "custom-b") { + t.Fatalf("slugs = %v, cross-tenant custom leaked", slugs) + } +} + +func TestExportSkillsDefaultRemainsCustomOnly(t *testing.T) { + db := hooksTestDB(t) + tenantID, _ := seedTenantAndAgent(t, db) + seedExportSkill(t, db, tenantID, "custom-default", false) + seedExportSkill(t, db, tenantID, "system-default", true) + + ctx := store.WithTenantID(t.Context(), tenantID) + got, err := ExportSkills(ctx, db, SkillExportSelection{}) + if err != nil { + t.Fatalf("ExportSkills() error = %v", err) + } + slugs := exportSkillSlugs(got) + if !containsSlug(slugs, "custom-default") { + t.Fatalf("slugs = %v, missing custom skill", slugs) + } + if containsSlug(slugs, "system-default") { + t.Fatalf("slugs = %v, default export included system skill", slugs) + } +} + +func seedExportSkill(t *testing.T, db *sql.DB, tenantID uuid.UUID, slug string, isSystem bool) uuid.UUID { + t.Helper() + id := uuid.Must(uuid.NewV7()) + _, err := db.ExecContext(store.WithTenantID(t.Context(), tenantID), + `INSERT INTO skills (id, name, slug, description, owner_id, visibility, version, status, frontmatter, file_path, file_size, tags, is_system, deps, enabled, tenant_id) + VALUES ($1,$2,$3,$4,$5,'private',1,'active','{}',$6,0,ARRAY['export-test']::text[],$7,'{}',true,$8)`, + id, slug, slug, slug+" desc", "owner", "/tmp/"+slug+"/1", isSystem, tenantID, + ) + if err != nil { + t.Fatalf("seed skill %s: %v", slug, err) + } + t.Cleanup(func() { + _, _ = db.Exec("DELETE FROM skills WHERE id=$1", id) + }) + return id +} + +func exportSkillSlugs(skills []CustomSkillExport) []string { + out := make([]string, 0, len(skills)) + for _, skill := range skills { + out = append(out, skill.Slug) + } + return out +} + +func containsSlug(slugs []string, want string) bool { + return slices.Contains(slugs, want) +} diff --git a/internal/store/pg/skills_scan_rows.go b/internal/store/pg/skills_scan_rows.go index 5b6b6c83..5d8e256d 100644 --- a/internal/store/pg/skills_scan_rows.go +++ b/internal/store/pg/skills_scan_rows.go @@ -84,6 +84,7 @@ type customSkillExportRow struct { Description *string `db:"description"` Visibility string `db:"visibility"` Version int `db:"version"` + IsSystem bool `db:"is_system"` FmRaw []byte `db:"frontmatter"` Tags pq.StringArray `db:"tags"` DepsRaw []byte `db:"deps"` @@ -99,6 +100,7 @@ func (r *customSkillExportRow) toCustomSkillExport() CustomSkillExport { Description: r.Description, Visibility: r.Visibility, Version: r.Version, + IsSystem: r.IsSystem, Tags: []string(r.Tags), } if len(r.FmRaw) > 0 { diff --git a/plans/260528-1759-skill-download-and-selective-export-web-ui/phase-01-api-contract-and-export-writers.md b/plans/260528-1759-skill-download-and-selective-export-web-ui/phase-01-api-contract-and-export-writers.md new file mode 100644 index 00000000..edacd685 --- /dev/null +++ b/plans/260528-1759-skill-download-and-selective-export-web-ui/phase-01-api-contract-and-export-writers.md @@ -0,0 +1,152 @@ +--- +phase: 1 +title: "API Contract and Export Writers" +status: complete +priority: P1 +effort: "1d" +dependencies: [] +--- + +# Phase 1: API Contract and Export Writers + +## Context Links + +- Issue: `digitopvn/goclaw#80` +- Backend route registration: `internal/http/skills.go` +- Existing export handler: `internal/http/skills_export.go` +- Existing export queries: `internal/store/pg/skills_export_queries.go` +- Skill file/version helpers: `internal/http/skills_versions.go` +- Existing import parser: `internal/http/skills_import.go` + +## Overview + +Extend the backend export path to support selected skills, system skills when explicitly selected, and multiple archive formats while keeping existing all-custom-skills export compatible. + +## Requirements + +- Functional: admin can export selected skill IDs, including system/core skills. +- Functional: admin can choose `zip`, `tar.gz`, or `tgz`. +- Functional: archive preserves skill directory content, including `SKILL.md`, `references/`, `scripts/`, `assets/`, and metadata. +- Functional: no `ids` preserves current tenant-scoped custom-skill export behavior. +- Non-functional: no shelling out to `zip`, `tar`, `gzip`, or external tools. +- Non-functional: keep path traversal, symlink escape, and export size protections. + +## Architecture + +Add a small archive-writer abstraction: + +```go +type skillArchiveWriter interface { + AddFile(name string, data []byte) error + Close() error + ContentType() string + Extension() string +} +``` + +Use two implementations: +- `tar.gz`: existing `gzip.Writer` + `tar.Writer` +- `zip`: Go standard library `archive/zip` + +Skill selection flow: +1. Parse `format`, `ids`, and `include_system`. +2. Resolve admin auth through existing `adminMiddleware`. +3. Query custom skills by tenant for full export. +4. Query selected skills by IDs for selected export; enforce tenant scope and allow system skills only when selected. +5. For each skill, resolve a readable root: + - managed custom root from `file_path` + - system root from version directory or bundled fallback, mirroring file-browser behavior +6. Walk allowed files and add normalized archive paths. + +## Related Code Files + +- Modify: `internal/http/skills_export.go` +- Modify: `internal/store/pg/skills_export_queries.go` +- Possibly modify: `internal/http/skills_versions.go` to share root-resolution helper +- Create: `internal/http/skills_export_test.go` +- Create or extend: `internal/store/pg/skills_export_queries_test.go` + +## Tests Before + +1. Backend handler contract tests fail first: + - `GET /v1/skills/export?format=zip&id=` returns ZIP content type and contains `skills//SKILL.md`. + - `GET /v1/skills/export?format=tgz&id=` returns gzip tar content type. + - unsupported `format=rar` returns `400`. + - non-admin caller gets `403`. +2. Store query tests fail first: + - selected skill query returns system and custom rows when IDs are explicit. + - full export query remains custom-only by default. + - tenant scope blocks selected skills from another tenant. + +## Refactor + +1. Extract current tar writer into a reusable archive-writer helper. +2. Add ZIP writer using standard library only. +3. Add format parser with aliases: + - `tar.gz`, `tgz`, empty -> gzip tar + - `zip` -> ZIP +4. Add selected skill resolver query in `pg`. +5. Add directory walker that: + - normalizes slash paths + - skips symlinks and OS junk + - rejects `..`, absolute paths, Windows drives, and null bytes + - enforces existing `maxExportSize` +6. Include `metadata.json` and `grants.jsonl` as generated files. + +## Tests After + +1. Archive content tests: + - nested `references/`, `scripts/`, and `assets/` files are present. + - generated `metadata.json` excludes internal file path. + - `grants.jsonl` remains included for custom skill grants. +2. Compatibility tests: + - no `ids` still exports custom skills only. + - existing SSE path still returns `download_url`. + - direct download sets correct `Content-Disposition` filename. + +## Implementation Steps + +1. Write failing tests for format parsing, selected IDs, system inclusion, and folder walking. +2. Add `parseSkillExportRequest(r)` to centralize query parsing. +3. Add `ExportSelectedSkills(ctx, db, ids, includeSystem)` in `internal/store/pg/skills_export_queries.go`. +4. Replace direct `tar.Writer` code with archive-writer interface. +5. Add ZIP implementation. +6. Replace single `SKILL.md` read with safe directory walk from resolved root. +7. Keep existing SSE token flow and direct response flow. +8. Update `docs/18-http-api.md` only if implementation changes the documented API shape. + +## Todo List + +- [x] Backend tests written before implementation. +- [x] Store selected-skill query is tenant-scoped. +- [x] Archive writer abstraction supports ZIP and gzip tar. +- [x] System/core selected skills export with full directory contents. +- [x] Full export backward-compatible. +- [x] HTTP docs updated if needed. + +## Success Criteria + +- [x] `format=zip`, `format=tar.gz`, and `format=tgz` work. +- [x] Selected system/core skill archive contains `SKILL.md` and resource dirs when present. +- [x] Full export without `ids` does not accidentally include bundled system skills. +- [x] Unauthorized/non-admin export is rejected. +- [x] `go test ./internal/http ./internal/store/pg` passes. + +## Risk Assessment + +- Risk: including system skills in full backups bloats archives and changes import semantics. + Mitigation: only include system skills when selected or `include_system=true`. +- Risk: path traversal or symlink escape during directory walk. + Mitigation: reuse strict archive path sanitizer and skip symlinks. +- Risk: ZIP export breaks import compatibility. + Mitigation: document that issue #80 download supports ZIP; existing full import/export remains tar.gz-compatible unless implementation adds ZIP import support later. + +## Security Considerations + +- Admin-only gate stays in `RegisterRoutes`. +- Tenant scope must be enforced in SQL, not only UI filtering. +- Generated metadata must not leak server filesystem paths. + +## Next Steps + +Phase 2 consumes the backend API from the Skills page and should not duplicate archive assembly client-side. diff --git a/plans/260528-1759-skill-download-and-selective-export-web-ui/phase-02-skills-ui-download-actions.md b/plans/260528-1759-skill-download-and-selective-export-web-ui/phase-02-skills-ui-download-actions.md new file mode 100644 index 00000000..6aee03bc --- /dev/null +++ b/plans/260528-1759-skill-download-and-selective-export-web-ui/phase-02-skills-ui-download-actions.md @@ -0,0 +1,141 @@ +--- +phase: 2 +title: "Skills UI Download Actions" +status: complete +priority: P2 +effort: "0.75d" +dependencies: [1] +--- + +# Phase 2: Skills UI Download Actions + +## Context Links + +- Skills page: `ui/web/src/pages/skills/skills-page.tsx` +- Skills hook: `ui/web/src/pages/skills/hooks/use-skills.ts` +- Detail dialog: `ui/web/src/pages/skills/skill-detail-dialog.tsx` +- Bulk toolbar: `ui/web/src/pages/skills/skill-bulk-actions-toolbar.tsx` +- Row actions: `ui/web/src/pages/skills/skill-table-row.tsx` +- Existing export hook pattern: `ui/web/src/pages/import-export/hooks/use-capabilities-export.ts` +- i18n: `ui/web/src/i18n/locales/{en,vi,zh}/skills.json` + +## Overview + +Expose admin-only download actions where users already manage skills: one action in skill detail and one bulk action for selected rows. + +## Requirements + +- Functional: admin can download current skill from detail dialog. +- Functional: admin can select multiple skills and download one archive. +- Functional: selected system/core skills are eligible. +- Functional: UI supports archive format choice. +- Functional: empty selection is disabled and surfaced clearly. +- Non-functional: use existing `HttpClient.downloadBlob` and auth headers. +- Non-functional: no client-side ZIP assembly. + +## Architecture + +Use backend as source of truth. UI constructs download URLs only: + +```text +SkillDetailDialog + -> useSkills().downloadSkills({ ids: [skill.id], format }) + -> /v1/skills/export?id=&format= + +SkillBulkActionsToolbar + -> useSkills().downloadSkills({ ids: selectedIds, format }) + -> /v1/skills/export?ids=&format= +``` + +Format control: +- Keep default `zip` in the Skills page because browser users expect ZIP. +- Offer `zip`, `tar.gz`, `tgz` in a compact select/menu. +- Import/export page can continue defaulting to tar.gz for compatibility. + +## Related Code Files + +- Modify: `ui/web/src/pages/skills/hooks/use-skills.ts` +- Modify: `ui/web/src/pages/skills/skills-page.tsx` +- Modify: `ui/web/src/pages/skills/skill-detail-dialog.tsx` +- Modify: `ui/web/src/pages/skills/skill-bulk-actions-toolbar.tsx` +- Modify: `ui/web/src/i18n/locales/en/skills.json` +- Modify: `ui/web/src/i18n/locales/vi/skills.json` +- Modify: `ui/web/src/i18n/locales/zh/skills.json` +- Create or extend tests near `ui/web/src/pages/skills/lib/` if logic is extracted. + +## Tests Before + +1. Add UI logic tests for: + - export URL params from one ID and many IDs. + - filename extension from selected format. + - no selected IDs -> disabled bulk download. +2. Add component tests only if existing test setup makes it low-friction; otherwise keep logic in pure helpers. + +## Refactor + +1. Add a small helper: + - `buildSkillExportParams(ids, format)` + - `skillExportDownloadName(skills, format)` +2. Add `downloadSkills(ids, format)` to `useSkills`. +3. Add local `exportFormat` state in `SkillsPage` or small child component. +4. Wire bulk toolbar download button. +5. Wire detail dialog download button near Copy Link/version controls. +6. Add i18n keys in all three locale files before rendering strings. + +## Tests After + +1. Verify pure helper tests pass. +2. Verify TypeScript build catches prop wiring: + - `pnpm -C ui/web build` +3. Manual UI check if dev server is available: + - custom tab selected skills -> download enabled. + - core tab selected skills -> download enabled. + - no selected skills -> no bulk toolbar. + +## Implementation Steps + +1. Write failing helper tests for URL/query and filename behavior. +2. Add helper module only if it prevents bloating `skills-page.tsx`; use kebab-case filename. +3. Add `downloadSkills` in `useSkills` using `http.downloadBlob`. +4. Pass download callbacks into `SkillBulkActionsToolbar` and `SkillDetailDialog`. +5. Add format selector using existing UI select/menu patterns. +6. Add loading state so repeated clicks are disabled. +7. Add success/error toasts. +8. Update all `skills.json` locale files. + +## Todo List + +- [x] URL/filename helper tests first. +- [x] `useSkills` exposes `downloadSkills`. +- [x] Detail dialog has single-skill Download. +- [x] Bulk toolbar has Download selected. +- [x] Core/system selected rows are not filtered out. +- [x] EN/VI/ZH strings added. +- [x] Web build passes. + +## Success Criteria + +- [x] Admin can download one skill from detail. +- [x] Admin can download selected custom and system skills from list. +- [x] UI can choose `zip`, `tar.gz`, or `tgz`. +- [x] Empty selection and loading states are correct. +- [x] UI does not assemble archives client-side. +- [x] `pnpm -C ui/web test -- skills` and `pnpm -C ui/web build` pass. + +## Risk Assessment + +- Risk: adding too much state to `skills-page.tsx`. + Mitigation: extract helpers and keep UI state minimal. +- Risk: "Download" icon conflicts with dependency install icon. + Mitigation: labels and tooltip text clarify skill export vs dependency install. +- Risk: admin-only feature visible to non-admin due route assumptions. + Mitigation: Skills page route/admin context should be checked; backend remains source of enforcement. + +## Security Considerations + +- UI visibility is convenience only; backend admin gate is authoritative. +- Do not expose raw archive token or filesystem path in UI state. + +## Next Steps + +Phase 3 validates backend and UI together, then comments the issue with plan path and implementation summary. diff --git a/plans/260528-1759-skill-download-and-selective-export-web-ui/phase-03-regression-validation-and-issue-handoff.md b/plans/260528-1759-skill-download-and-selective-export-web-ui/phase-03-regression-validation-and-issue-handoff.md new file mode 100644 index 00000000..9d2accda --- /dev/null +++ b/plans/260528-1759-skill-download-and-selective-export-web-ui/phase-03-regression-validation-and-issue-handoff.md @@ -0,0 +1,132 @@ +--- +phase: 3 +title: "Regression Validation and Issue Handoff" +status: complete +priority: P2 +effort: "0.5d" +dependencies: [1, 2] +--- + +# Phase 3: Regression Validation and Issue Handoff + +## Context Links + +- API docs: `docs/18-http-api.md` +- Changelog: `docs/project-changelog.md` +- GitHub issue: `digitopvn/goclaw#80` +- Existing beta workflow note: `dev-beta-release.yaml` is the release check surface for later implementation, not for this plan-only branch. + +## Overview + +Run the TDD regression gates, update docs if API changed, and prepare the implementation handoff for issue #80. + +## Requirements + +- Functional: all planned acceptance criteria are validated by tests or manual checks. +- Functional: issue #80 gets a concise summary and implementation PR path. +- Non-functional: ship by PR only; do not merge implementation in this phase. +- Non-functional: keep reports concise and list unresolved questions last. + +## Architecture + +Validation is layered: + +```text +Backend tests + -> archive formats + -> selected/system skill scope + -> permissions + +Web tests/build + -> URL params and filename helpers + -> component prop wiring + -> locale completeness + +Docs/handoff + -> HTTP API docs if route contract changed + -> issue comment with plan path +``` + +## Related Code Files + +- Modify if needed: `docs/18-http-api.md` +- Modify if implementation lands: `docs/project-changelog.md` +- No planned source changes in this phase beyond docs/test fixes. + +## Tests Before + +Before implementing final polish, confirm expected failures from Phases 1-2 are real: +- backend selected export tests fail before backend changes. +- UI helper tests fail before UI helper changes. + +## Refactor + +Keep refactor minimal: +- remove duplicated archive filename construction. +- remove duplicated query param building. +- keep docs updated only for changed API behavior. + +## Tests After + +Run: + +```bash +go test ./internal/http ./internal/store/pg +pnpm -C ui/web test -- skills +pnpm -C ui/web build +``` + +If archive writer changes wider shared helpers, also run: + +```bash +go build ./... +``` + +## Implementation Steps + +1. Run backend focused tests. +2. Run UI focused tests. +3. Run UI build. +4. Run Go build if shared backend helpers changed. +5. Update `docs/18-http-api.md` with `format`, `ids`, and system-skill selection semantics. +6. Update `docs/project-changelog.md` after implementation lands. +7. Comment issue #80 with: + - selected approach + - archive formats + - admin-only permission model + - plan path + - validation commands to expect. + +## Todo List + +- [x] Backend focused tests pass. +- [x] Web focused tests pass. +- [x] Web build passes. +- [x] Go build runs if needed. +- [x] HTTP API docs match actual query params. +- [x] Issue #80 comment posted with plan path. + +## Success Criteria + +- [x] All acceptance criteria from issue #80 mapped to phases. +- [x] No unresolved API questions remain. +- [x] Issue #80 has an implementation summary comment. +- [x] Implementation is ready for PR review and beta shipping label. + +## Risk Assessment + +- Risk: docs claim ZIP import support when implementation only supports ZIP export. + Mitigation: docs must say export/download explicitly unless ZIP import is actually implemented. +- Risk: selected export tests pass but import/export page behavior regresses. + Mitigation: include no-ids tar.gz compatibility test. +- Risk: issue comment over-promises implementation. + Mitigation: comment lists completed validation commands and links the PR. + +## Security Considerations + +- Issue comment should not include local secrets, env paths, or private tokens. +- Commit only implementation, plan, and docs artifacts for this issue branch. + +## Next Steps + +Create PR, wait for CI, then comment and relabel issue #80. diff --git a/plans/260528-1759-skill-download-and-selective-export-web-ui/plan.md b/plans/260528-1759-skill-download-and-selective-export-web-ui/plan.md new file mode 100644 index 00000000..91f9a106 --- /dev/null +++ b/plans/260528-1759-skill-download-and-selective-export-web-ui/plan.md @@ -0,0 +1,95 @@ +--- +title: "Skill Download and Selective Export Web UI" +description: "Add admin-only skill download actions on /skills with selected/system skill support and zip/tar.gz export formats." +status: implemented +priority: P2 +issue: 80 +branch: "codex/issue-80-skill-download-plan" +tags: [skills, export, web-ui, tdd, issue-80] +blockedBy: [] +blocks: [] +created: "2026-05-28T11:00:02.557Z" +createdBy: "ck:plan" +source: skill +--- + +# Skill Download and Selective Export Web UI + +## Overview + +Implement issue #80 by extending the existing skills export surface instead of creating a parallel subsystem. + +User decisions locked: +- Archive formats: support popular server-generated formats: `zip`, `tar.gz`, and `tgz` alias. +- Scope: downloadable skills include custom and system/core skills when explicitly selected. +- Permission model: admin-only. + +Backward compatibility: +- Existing full skills export remains compatible with `/import-export`: no `ids` means tenant-scoped custom skills by default. +- `/skills` detail and bulk actions use selected IDs, so system/core skills can be exported without changing full backup defaults. + +## Current Code Context + +- Existing backend export: `internal/http/skills_export.go` +- Existing export routes: `internal/http/skills.go` +- Existing export queries: `internal/store/pg/skills_export_queries.go` +- Existing file/version helpers: `internal/http/skills_versions.go` +- Existing import/export UI: `ui/web/src/pages/import-export/hooks/use-capabilities-export.ts` +- Existing skills UI selection: `ui/web/src/pages/skills/skills-page.tsx` +- Existing bulk toolbar: `ui/web/src/pages/skills/skill-bulk-actions-toolbar.tsx` +- Existing i18n namespace: `ui/web/src/i18n/locales/{en,vi,zh}/skills.json` + +## Phases + +| Phase | Name | Status | +|-------|------|--------| +| 1 | [API Contract and Export Writers](./phase-01-api-contract-and-export-writers.md) | Complete | +| 2 | [Skills UI Download Actions](./phase-02-skills-ui-download-actions.md) | Complete | +| 3 | [Regression Validation and Issue Handoff](./phase-03-regression-validation-and-issue-handoff.md) | Complete | + +## Dependencies + +- GitHub issue: `digitopvn/goclaw#80` +- No blocking plan found. Existing pending merge-train plan touches release flow, not this feature area. + +## API Contract Target + +`GET /v1/skills/export` + +Query params: +- `stream=true|false`: keep existing behavior. +- `format=tar.gz|tgz|zip`: default `tar.gz`; `tgz` aliases to gzip tar writer. +- `ids=,` or repeated `id=`: optional selected skill IDs. +- `include_system=true|false`: optional for full export only. Selected `ids` can include system skills without this flag. + +Response filenames: +- Single selected skill: `goclaw-skill--v.` +- Multi/full export: `goclaw-skills-export-YYYYMMDD-HHmm.` + +Archive layout: +```text +skills//metadata.json +skills//SKILL.md +skills//references/** +skills//scripts/** +skills//assets/** +skills//grants.jsonl +``` + +## Validation Commands + +- `go test ./internal/http ./internal/store/pg` +- `pnpm -C ui/web test -- skills` +- `pnpm -C ui/web build` +- If backend code changes touch wider store contracts: `go build ./...` + +## Out of Scope + +- Viewer/operator downloads. +- RAR/7z export creation. +- Changing skills import to accept ZIP in this plan unless implementation discovers import/export mismatch that blocks issue #80 acceptance. +- Auto-publishing release or merging implementation PR. + +## Unresolved Questions + +None for planning. RAR/7z intentionally excluded from "popular archive formats" because they require extra dependencies or non-standard writers; ZIP and tar.gz cover browser/download and Unix portability. diff --git a/ui/web/src/i18n/locales/en/skills.json b/ui/web/src/i18n/locales/en/skills.json index e8f01366..90daadab 100644 --- a/ui/web/src/i18n/locales/en/skills.json +++ b/ui/web/src/i18n/locales/en/skills.json @@ -89,6 +89,14 @@ "customOnlyReason": "This action only applies to custom skills", "noAgentsReason": "No current agents available" }, + "export": { + "format": "Archive format", + "download": "Download", + "downloadSelected": "Download selected", + "downloading": "Downloading...", + "downloadSuccess": "Download started", + "downloadFailed": "Download failed" + }, "visibility": { "clickToCycle": "Click to change access mode" }, diff --git a/ui/web/src/i18n/locales/vi/skills.json b/ui/web/src/i18n/locales/vi/skills.json index 329d18e4..918eb60f 100644 --- a/ui/web/src/i18n/locales/vi/skills.json +++ b/ui/web/src/i18n/locales/vi/skills.json @@ -227,5 +227,13 @@ "skippedSystem": "Bỏ qua {{count}} skill hệ thống", "customOnlyReason": "Thao tác này chỉ áp dụng cho custom skill", "noAgentsReason": "Chưa có agent hiện tại" + }, + "export": { + "format": "Định dạng archive", + "download": "Tải xuống", + "downloadSelected": "Tải xuống mục đã chọn", + "downloading": "Đang tải...", + "downloadSuccess": "Đã bắt đầu tải xuống", + "downloadFailed": "Tải xuống thất bại" } } diff --git a/ui/web/src/i18n/locales/zh/skills.json b/ui/web/src/i18n/locales/zh/skills.json index e5db5acb..a44236c9 100644 --- a/ui/web/src/i18n/locales/zh/skills.json +++ b/ui/web/src/i18n/locales/zh/skills.json @@ -227,5 +227,13 @@ "skippedSystem": "已跳过 {{count}} 个系统 Skill", "customOnlyReason": "此操作仅适用于自定义 Skill", "noAgentsReason": "暂无当前 Agent" + }, + "export": { + "format": "归档格式", + "download": "下载", + "downloadSelected": "下载所选项", + "downloading": "下载中...", + "downloadSuccess": "已开始下载", + "downloadFailed": "下载失败" } } diff --git a/ui/web/src/pages/skills/hooks/use-skills.ts b/ui/web/src/pages/skills/hooks/use-skills.ts index 5809180e..acf7a333 100644 --- a/ui/web/src/pages/skills/hooks/use-skills.ts +++ b/ui/web/src/pages/skills/hooks/use-skills.ts @@ -8,6 +8,11 @@ import { toast } from "@/stores/use-toast-store"; import i18next from "i18next"; import { userFriendlyError } from "@/lib/error-utils"; import type { SkillInfo, SkillFile, SkillVersions, SkillAgentGrant } from "@/types/skill"; +import { + buildSkillExportPath, + skillExportDownloadName, + type SkillExportFormat, +} from "../lib/skill-export-download"; export type { SkillInfo, SkillFile, SkillVersions }; @@ -186,6 +191,20 @@ export function useSkills() { [http, invalidate], ); + const downloadSkills = useCallback( + async (targetSkills: SkillInfo[], format: SkillExportFormat) => { + const ids = targetSkills.map((skill) => skill.id).filter((id): id is string => !!id); + const blob = await http.downloadBlob(buildSkillExportPath(ids, format)); + const url = URL.createObjectURL(blob); + const a = document.createElement("a"); + a.href = url; + a.download = skillExportDownloadName(targetSkills, format); + a.click(); + URL.revokeObjectURL(url); + }, + [http], + ); + const getSkillVersions = useCallback( async (id: string) => { return http.get(`/v1/skills/${id}/versions`); @@ -302,7 +321,7 @@ export function useSkills() { skills, loading, refresh: invalidate, getSkill, uploadSkill, updateSkill, deleteSkill, listAgentGrants, grantSkillToAgent, grantSkillToAgents, revokeSkillFromAgent, - deleteSkills, toggleSkills, + deleteSkills, toggleSkills, downloadSkills, getSkillVersions, getSkillFiles, getSkillFileContent, rescanDeps, installDeps, installSingleDep, toggleSkill, setTenantConfig, deleteTenantConfig, }; diff --git a/ui/web/src/pages/skills/lib/skill-export-download.test.ts b/ui/web/src/pages/skills/lib/skill-export-download.test.ts new file mode 100644 index 00000000..93b7001d --- /dev/null +++ b/ui/web/src/pages/skills/lib/skill-export-download.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from "vitest"; +import { + buildSkillExportPath, + skillExportDownloadName, + skillExportExtension, + type SkillExportFormat, +} from "./skill-export-download"; + +describe("skill export download helpers", () => { + it("builds a selected export URL with repeated id params", () => { + const path = buildSkillExportPath(["skill-a", "skill-b"], "zip"); + const url = new URL(path, "https://goclaw.test"); + + expect(url.pathname).toBe("/v1/skills/export"); + expect(url.searchParams.get("format")).toBe("zip"); + expect(url.searchParams.getAll("id")).toEqual(["skill-a", "skill-b"]); + }); + + it("rejects empty selections", () => { + expect(() => buildSkillExportPath([], "zip")).toThrow("at least one skill"); + }); + + it.each(["zip", "tar.gz", "tgz"])("maps extension for %s", (format) => { + expect(skillExportExtension(format)).toMatch(/^\.(zip|tar\.gz)$/); + }); + + it("names single skill archives by slug and version", () => { + const name = skillExportDownloadName([{ slug: "my-skill", version: 3 }], "zip"); + + expect(name).toBe("goclaw-skill-my-skill-v3.zip"); + }); + + it("names multi skill archives with selected extension", () => { + const name = skillExportDownloadName([ + { slug: "one", version: 1 }, + { slug: "two", version: 2 }, + ], "tar.gz", new Date("2026-05-29T09:30:00Z")); + + expect(name).toBe("goclaw-skills-export-20260529-0930.tar.gz"); + }); +}); diff --git a/ui/web/src/pages/skills/lib/skill-export-download.ts b/ui/web/src/pages/skills/lib/skill-export-download.ts new file mode 100644 index 00000000..7f4e0c7e --- /dev/null +++ b/ui/web/src/pages/skills/lib/skill-export-download.ts @@ -0,0 +1,58 @@ +export type SkillExportFormat = "zip" | "tar.gz" | "tgz"; + +export type SkillExportSummary = { + id?: string; + slug?: string; + name?: string; + version?: number; +}; + +export function buildSkillExportPath(ids: string[], format: SkillExportFormat): string { + const cleanIds = Array.from(new Set(ids.map((id) => id.trim()).filter(Boolean))); + if (cleanIds.length === 0) { + throw new Error("at least one skill is required"); + } + + const params = new URLSearchParams(); + params.set("format", format); + for (const id of cleanIds) params.append("id", id); + + return `/v1/skills/export?${params.toString()}`; +} + +export function skillExportExtension(format: SkillExportFormat): ".zip" | ".tar.gz" { + return format === "zip" ? ".zip" : ".tar.gz"; +} + +export function skillExportDownloadName( + skills: SkillExportSummary[], + format: SkillExportFormat, + now = new Date(), +): string { + const extension = skillExportExtension(format); + if (skills.length === 1) { + const skill = skills[0]!; + const label = sanitizeFilePart(skill.slug || skill.name || skill.id || "skill"); + const version = skill.version ? `-v${skill.version}` : ""; + return `goclaw-skill-${label}${version}${extension}`; + } + + return `goclaw-skills-export-${formatUtcTimestamp(now)}${extension}`; +} + +function sanitizeFilePart(value: string): string { + return value.trim().toLowerCase().replace(/[^a-z0-9._-]+/g, "-").replace(/^-+|-+$/g, "") || "skill"; +} + +function formatUtcTimestamp(date: Date): string { + const year = date.getUTCFullYear(); + const month = pad(date.getUTCMonth() + 1); + const day = pad(date.getUTCDate()); + const hour = pad(date.getUTCHours()); + const minute = pad(date.getUTCMinutes()); + return `${year}${month}${day}-${hour}${minute}`; +} + +function pad(value: number): string { + return String(value).padStart(2, "0"); +} diff --git a/ui/web/src/pages/skills/skill-bulk-actions-toolbar.tsx b/ui/web/src/pages/skills/skill-bulk-actions-toolbar.tsx index fe6c8fdd..44d7603d 100644 --- a/ui/web/src/pages/skills/skill-bulk-actions-toolbar.tsx +++ b/ui/web/src/pages/skills/skill-bulk-actions-toolbar.tsx @@ -1,6 +1,14 @@ -import { CheckCircle2, ShieldCheck, Trash2, XCircle } from "lucide-react"; +import { CheckCircle2, Download, ShieldCheck, Trash2, XCircle } from "lucide-react"; import { useTranslation } from "react-i18next"; import { Button } from "@/components/ui/button"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui/select"; +import type { SkillExportFormat } from "./lib/skill-export-download"; interface SkillBulkActionsToolbarProps { selectedCount: number; @@ -8,6 +16,10 @@ interface SkillBulkActionsToolbarProps { skippedSystemCount: number; agentCount: number; loading: boolean; + downloadLoading: boolean; + exportFormat: SkillExportFormat; + onExportFormatChange: (format: SkillExportFormat) => void; + onDownload: () => void; onEnable: () => void; onDisable: () => void; onGrantAllAgents: () => void; @@ -21,6 +33,10 @@ export function SkillBulkActionsToolbar({ skippedSystemCount, agentCount, loading, + downloadLoading, + exportFormat, + onExportFormatChange, + onDownload, onEnable, onDisable, onGrantAllAgents, @@ -49,6 +65,26 @@ export function SkillBulkActionsToolbar({
+ + +
+ + + + {versions && versions.versions.length > 1 ? ( + <> {t("detail.version")} -
- ) : headerVersion ? ( -
- + + ) : headerVersion ? ( v{headerVersion} -
- ) : ( - - )} + ) : null} +
{skill.description && (

{skill.description}

diff --git a/ui/web/src/pages/skills/skills-page.tsx b/ui/web/src/pages/skills/skills-page.tsx index 8c604997..4ccfc54f 100644 --- a/ui/web/src/pages/skills/skills-page.tsx +++ b/ui/web/src/pages/skills/skills-page.tsx @@ -36,6 +36,7 @@ import { type SkillsPageState, } from "./lib/skills-page-state"; import { getNextSkillAccessMode } from "./lib/skill-access-mode"; +import type { SkillExportFormat } from "./lib/skill-export-download"; const MASTER_TENANT_ID = "0193a5b0-7000-7000-8000-000000000001"; @@ -46,7 +47,7 @@ export function SkillsPage() { const { skills, loading, refresh, getSkill, uploadSkill, updateSkill, deleteSkill, listAgentGrants, grantSkillToAgent, grantSkillToAgents, revokeSkillFromAgent, - deleteSkills, toggleSkills, + deleteSkills, toggleSkills, downloadSkills, getSkillVersions, getSkillFiles, getSkillFileContent, rescanDeps, installDeps, installSingleDep, toggleSkill, setTenantConfig, deleteTenantConfig, } = useSkills(); @@ -71,6 +72,8 @@ export function SkillsPage() { const [selectedIds, setSelectedIds] = useState>(new Set()); const [deleteLoading, setDeleteLoading] = useState(false); const [bulkLoading, setBulkLoading] = useState(false); + const [downloadLoading, setDownloadLoading] = useState(false); + const [exportFormat, setExportFormat] = useState("zip"); const [rescanning, setRescanning] = useState(false); const [installingDeps, setInstallingDeps] = useState(false); const [toggling, setToggling] = useState(null); @@ -252,6 +255,18 @@ export function SkillsPage() { } }; + const handleDownloadSkills = async (targetSkills: SkillInfo[]) => { + setDownloadLoading(true); + try { + await downloadSkills(targetSkills, exportFormat); + toast.success(t("export.downloadSuccess")); + } catch (err) { + toast.error(t("export.downloadFailed"), err instanceof Error ? err.message : String(err)); + } finally { + setDownloadLoading(false); + } + }; + const handleRescanDeps = async () => { setRescanning(true); try { await rescanDeps(); } finally { setRescanning(false); } @@ -327,6 +342,10 @@ export function SkillsPage() { skippedSystemCount={skippedSystemCount} agentCount={agents.length} loading={bulkLoading || deleteLoading} + downloadLoading={downloadLoading} + exportFormat={exportFormat} + onExportFormatChange={setExportFormat} + onDownload={() => handleDownloadSkills(selectedSkills)} onEnable={() => handleBulkToggle(true)} onDisable={() => handleBulkToggle(false)} onGrantAllAgents={handleBulkGrantAllAgents} @@ -420,6 +439,10 @@ export function SkillsPage() { selectedFilePath={params.get("file")} onStateChange={setParamValues} onClose={closeDetail} + exportFormat={exportFormat} + downloadLoading={downloadLoading} + onExportFormatChange={setExportFormat} + onDownloadSkill={() => handleDownloadSkills([selectedSkill])} getSkillVersions={getSkillVersions} getSkillFiles={getSkillFiles} getSkillFileContent={getSkillFileContent}