diff --git a/Makefile b/Makefile index faf0c0f..3fc45b2 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,7 @@ $(LOCALBIN): mkdir -p "$(LOCALBIN)" GOLANGCI_LINT = $(LOCALBIN)/golangci-lint -GOLANGCI_LINT_VERSION ?= v2.5.0 +GOLANGCI_LINT_VERSION ?= v2.7.2 GO_JUNIT_REPORT = $(LOCALBIN)/go-junit-report GO_JUNIT_REPORT_VERSION ?= v1.0.0 GOPLS = $(LOCALBIN)/gopls diff --git a/designs/TASKS.md b/designs/TASKS.md index 0932634..e55cebf 100644 --- a/designs/TASKS.md +++ b/designs/TASKS.md @@ -25,6 +25,22 @@ ## Implementation Tasks +- [x] 2026-04-24: Added MCP skill publishing tool for registry creation from markdown (TDD-first): + - Added new MCP tool `skill_publish` in `internal/api/mcp/skill_publish.go` with scope-auth enforcement and `skills:write` permission. + - Made create+publish atomic for `skill_publish` by creating skills with initial `published` status in a single create operation (no follow-up status update step). + - Updated `internal/skills/store.go` API comments to document that `Store.Create` defaults to draft creation but may create published skills when `CreateInput.Status`/`PublishedAt` are set. + - Tool accepts either full markdown `content` (with optional `SKILL.md` frontmatter) or explicit `body` + metadata and derives missing slug/name fields. + - Tightened frontmatter parsing so invalid inline `agent_types` lists now return an explicit error instead of silently falling back to defaults. + - Improved supplementary file validation diagnostics to include the failing item index in `skill_publish` file-parse errors. + - Normalized MCP array schema declaration for `agent_types` to use `WithStringItems()` for consistency with other tool definitions and strict clients. + - Normalized `source_name` path handling for slug derivation to be separator-stable across platforms (supports both `/` and `\` inputs consistently). + - Extended tool input to support supplementary `files` (scripts/references) and persist them through the existing skills store validation/writes. + - Tool creates the skill in the target scope and marks it `published` so it is immediately discoverable by `skill_search` and installable via `skill_install`/CLI sync. + - Added parser/unit coverage in `internal/api/mcp/skill_publish_test.go`, integration coverage in `internal/api/mcp/mcp_integration_test.go`, and handler validation coverage in `internal/api/mcp/handlers_unit_test.go`. + - Extended MCP scope-auth inventory and integration coverage for `skill_publish`: + - `internal/api/mcp/scopeauth_inventory_test.go` + - `internal/api/mcp/scope_authz_integration_test.go` + - [x] 2026-04-24: Fixed social login duplicate principal creation when email slug already exists (TDD-first): - Added integration regression coverage in `internal/social/identity_test.go`: - `TestIdentityStore_FindOrCreate_ExistingEmailSlug_LinksPrincipal` diff --git a/internal/api/mcp/handlers_unit_test.go b/internal/api/mcp/handlers_unit_test.go index 00f0cce..a1182ee 100644 --- a/internal/api/mcp/handlers_unit_test.go +++ b/internal/api/mcp/handlers_unit_test.go @@ -528,3 +528,35 @@ func TestHandleSkillInvoke_NilPool_ReturnsToolError(t *testing.T) { }, s.handleSkillInvoke) assertToolError(t, result) } + +// ── handleSkillPublish ──────────────────────────────────────────────────────── + +func TestHandleSkillPublish_MissingScope_ReturnsToolError(t *testing.T) { + s := &Server{} + result := callTool(t, map[string]any{ + "content": "skill body", + }, s.handleSkillPublish) + assertToolError(t, result) +} + +func TestHandleSkillPublish_MissingContentAndBody_ReturnsToolError(t *testing.T) { + s := &Server{} + result := callTool(t, map[string]any{ + "scope": "project:acme/api", + }, s.handleSkillPublish) + assertToolError(t, result) +} + +func TestHandleSkillPublish_ServerNotConfigured_PrecedesFilesValidation(t *testing.T) { + s := &Server{} + result := callTool(t, map[string]any{ + "scope": "project:acme/api", + "body": "skill body", + "files": "not-an-array", + }, s.handleSkillPublish) + assertToolError(t, result) + msg := strings.ToLower(toolErrorText(t, result)) + if !strings.Contains(msg, "server not configured") { + t.Fatalf("expected server-not-configured error, got %q", msg) + } +} diff --git a/internal/api/mcp/mcp_integration_test.go b/internal/api/mcp/mcp_integration_test.go index fbb118a..1aaa48f 100644 --- a/internal/api/mcp/mcp_integration_test.go +++ b/internal/api/mcp/mcp_integration_test.go @@ -5,6 +5,7 @@ package mcp_test import ( "context" "encoding/json" + "strings" "testing" "github.com/google/uuid" @@ -187,6 +188,92 @@ func TestMCP_Publish_Endorse_AutoPublish(t *testing.T) { } } +func TestMCP_SkillPublish_WithFiles(t *testing.T) { + ctx := context.Background() + pool := testhelper.NewTestPool(t) + svc := testhelper.NewMockEmbeddingService() + cfg := &config.Config{} + + principal := testhelper.CreateTestPrincipal(t, pool, "user", "mcp-skill-publish-user") + scope := testhelper.CreateTestScope(t, pool, "project", "mcp-skill-publish-project", nil, principal.ID) + testhelper.CreateTestEmbeddingModel(t, pool) + + srv := mcpapi.NewServer(pool, svc, cfg) + mcpSrv := srv.MCPServer() + + scopeStr := "project:" + scope.ExternalID + ctx = withAuthContext(ctx, pool, principal.ID, scope.ID) + + tool := mcpSrv.GetTool("skill_publish") + if tool == nil { + t.Fatal("skill_publish tool not registered") + } + + req := mcpgo.CallToolRequest{} + req.Params.Name = "skill_publish" + req.Params.Arguments = map[string]any{ + "scope": scopeStr, + "source_name": "tox-verifier.md", + "content": strings.Join([]string{ + "---", + "name: Tox Verifier", + "description: Verify tox output and summarize failures", + "---", + "", + "Run tox and summarize failures.", + }, "\n"), + "files": []any{ + map[string]any{ + "path": "scripts/run.sh", + "content": "#!/bin/sh\ntox -e py\n", + "executable": true, + }, + map[string]any{ + "path": "references/tox-format.md", + "content": "Reference output format", + }, + }, + } + + result, err := tool.Handler(ctx, req) + if err != nil { + t.Fatalf("skill_publish failed: %v", err) + } + if result == nil || result.IsError { + t.Fatalf("skill_publish returned error result: %+v", result) + } + if len(result.Content) == 0 { + t.Fatal("expected non-empty result content") + } + text, ok := result.Content[0].(mcpgo.TextContent) + if !ok { + t.Fatalf("expected TextContent, got %T", result.Content[0]) + } + var out struct { + SkillID string `json:"skill_id"` + Slug string `json:"slug"` + Status string `json:"status"` + } + if err := json.Unmarshal([]byte(text.Text), &out); err != nil { + t.Fatalf("skill_publish output is not JSON: %v", err) + } + if out.Status != "published" { + t.Fatalf("status = %q, want published", out.Status) + } + skillID, err := uuid.Parse(out.SkillID) + if err != nil { + t.Fatalf("invalid skill_id %q: %v", out.SkillID, err) + } + + files, err := compat.ListSkillFiles(ctx, pool, skillID) + if err != nil { + t.Fatalf("ListSkillFiles: %v", err) + } + if len(files) != 2 { + t.Fatalf("len(files) = %d, want 2", len(files)) + } +} + func withAuthContext(ctx context.Context, pool *pgxpool.Pool, principalID, scopeID uuid.UUID) context.Context { return withAuthContextPerms(ctx, pool, principalID, scopeID, []string{"read", "write", "edit", "delete"}) } diff --git a/internal/api/mcp/scope_authz_integration_test.go b/internal/api/mcp/scope_authz_integration_test.go index 01de3e8..8a94770 100644 --- a/internal/api/mcp/scope_authz_integration_test.go +++ b/internal/api/mcp/scope_authz_integration_test.go @@ -226,6 +226,23 @@ func TestMCP_ScopeAuthz_ScopeTakingTools(t *testing.T) { } }, }, + { + name: "skill_publish", + argsFor: func(scope string) map[string]any { + return map[string]any{ + "scope": scope, + "source_name": "tox-verifier.md", + "content": strings.Join([]string{ + "---", + "name: Tox Verifier", + "description: Verify tox logs and summarize failures", + "---", + "", + "Run tox and summarize test failures.", + }, "\n"), + } + }, + }, } for _, tc := range cases { diff --git a/internal/api/mcp/scopeauth_inventory_test.go b/internal/api/mcp/scopeauth_inventory_test.go index dfa7931..bc1e68d 100644 --- a/internal/api/mcp/scopeauth_inventory_test.go +++ b/internal/api/mcp/scopeauth_inventory_test.go @@ -32,6 +32,7 @@ var mcpScopeToolInventory = []mcpScopeToolInventoryItem{ {File: "synthesize.go", Handler: "handleSynthesizeTopic", Tool: "synthesize_topic", Operation: "synthesize digest"}, {File: "skill_install.go", Handler: "handleSkillInstall", Tool: "skill_install", Operation: "install skill"}, {File: "skill_invoke.go", Handler: "handleSkillInvoke", Tool: "skill_invoke", Operation: "invoke skill"}, + {File: "skill_publish.go", Handler: "handleSkillPublish", Tool: "skill_publish", Operation: "publish skill"}, } func TestScopeTakingHandlersCallAuthorizeRequestedScope(t *testing.T) { diff --git a/internal/api/mcp/server.go b/internal/api/mcp/server.go index b3dde24..275811f 100644 --- a/internal/api/mcp/server.go +++ b/internal/api/mcp/server.go @@ -55,6 +55,7 @@ func (s *Server) registerTools() { s.registerCollect() s.registerKnowledgeDetail() s.registerSkillSearch() + s.registerSkillPublish() s.registerSkillInstall() s.registerSkillInvoke() s.registerListScopes() diff --git a/internal/api/mcp/skill_publish.go b/internal/api/mcp/skill_publish.go new file mode 100644 index 0000000..2c94a56 --- /dev/null +++ b/internal/api/mcp/skill_publish.go @@ -0,0 +1,322 @@ +package mcp + +import ( + "context" + "encoding/json" + "fmt" + "path" + "strings" + "time" + + "github.com/google/uuid" + mcpgo "github.com/mark3labs/mcp-go/mcp" + + "github.com/simplyblock/postbrain/internal/auth" + "github.com/simplyblock/postbrain/internal/db" + "github.com/simplyblock/postbrain/internal/skills" +) + +func (s *Server) registerSkillPublish() { + s.mcpServer.AddTool(mcpgo.NewTool("skill_publish", + mcpgo.WithReadOnlyHintAnnotation(false), + mcpgo.WithDestructiveHintAnnotation(false), + mcpgo.WithOpenWorldHintAnnotation(false), + mcpgo.WithDescription("Publish a skill to the registry from SKILL.md-style markdown or explicit fields"), + mcpgo.WithString("scope", mcpgo.Required(), mcpgo.Description("Scope as kind:external_id")), + mcpgo.WithString("content", mcpgo.Description("Raw skill markdown; supports optional YAML frontmatter")), + mcpgo.WithString("body", mcpgo.Description("Skill body (used when content is omitted)")), + mcpgo.WithString("slug", mcpgo.Description("Skill slug; if omitted derived from source_name or name")), + mcpgo.WithString("name", mcpgo.Description("Skill name; if omitted derived from frontmatter or slug")), + mcpgo.WithString("description", mcpgo.Description("Short description; if omitted derived from frontmatter or name")), + mcpgo.WithString("source_name", mcpgo.Description("Optional source filename, e.g. tox-verifier.md")), + mcpgo.WithString("visibility", mcpgo.Description("private|project|team|department|company (default: team)")), + mcpgo.WithArray("agent_types", mcpgo.Description("Compatible agent types (default: [\"any\"])"), + mcpgo.WithStringItems(), + ), + mcpgo.WithArray("files", mcpgo.Description("Optional supplementary files, e.g. scripts/* and references/*.md"), + mcpgo.Items(map[string]any{ + "type": "object", + "properties": map[string]any{ + "path": map[string]any{"type": "string"}, + "content": map[string]any{"type": "string"}, + "executable": map[string]any{"type": "boolean"}, + }, + "required": []string{"path", "content"}, + }), + ), + ), withToolMetrics("skill_publish", withToolPermission("skills:write", s.handleSkillPublish))) +} + +type skillDraft struct { + Slug string + Name string + Description string + Body string + AgentTypes []string +} + +func (s *Server) handleSkillPublish(ctx context.Context, req mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) { + args := req.GetArguments() + + scopeStr := argString(args, "scope") + if scopeStr == "" { + return mcpgo.NewToolResultError("skill_publish: 'scope' is required"), nil + } + content := argString(args, "content") + body := argString(args, "body") + if content == "" && body == "" { + return mcpgo.NewToolResultError("skill_publish: 'content' or 'body' is required"), nil + } + + if s.pool == nil || s.sklStore == nil { + return mcpgo.NewToolResultError("skill_publish: server not configured"), nil + } + + scopeID, errResult := s.resolveScope(ctx, "skill_publish", scopeStr) + if errResult != nil { + return errResult, nil + } + + draft := skillDraft{ + Slug: strings.TrimSpace(argString(args, "slug")), + Name: strings.TrimSpace(argString(args, "name")), + Description: strings.TrimSpace(argString(args, "description")), + Body: strings.TrimSpace(body), + AgentTypes: argStringSlice(args, "agent_types"), + } + if content != "" { + parsed, err := parseSkillMarkdownContent(content) + if err != nil { + return mcpgo.NewToolResultError(fmt.Sprintf("skill_publish: parse content: %v", err)), nil + } + if draft.Name == "" { + draft.Name = parsed.Name + } + if draft.Description == "" { + draft.Description = parsed.Description + } + if draft.Body == "" { + draft.Body = strings.TrimSpace(parsed.Body) + } + if len(draft.AgentTypes) == 0 { + draft.AgentTypes = parsed.AgentTypes + } + } + + draft.Slug = defaultSkillSlug(draft.Slug, draft.Name, argString(args, "source_name")) + if draft.Slug == "" { + return mcpgo.NewToolResultError("skill_publish: could not derive slug; provide 'slug', 'name', or 'source_name'"), nil + } + if err := skills.ValidateSlug(draft.Slug); err != nil { + return mcpgo.NewToolResultError(fmt.Sprintf("skill_publish: invalid slug: %v", err)), nil + } + + if draft.Name == "" { + draft.Name = titleFromSlug(draft.Slug) + } + if draft.Description == "" { + draft.Description = draft.Name + } + if draft.Body == "" { + return mcpgo.NewToolResultError("skill_publish: body must not be empty"), nil + } + + visibility := strings.TrimSpace(argString(args, "visibility")) + if visibility == "" { + visibility = "team" + } + + authorID, _ := ctx.Value(auth.ContextKeyPrincipalID).(uuid.UUID) + if authorID == uuid.Nil { + return mcpgo.NewToolResultError("skill_publish: missing caller principal"), nil + } + + var files []db.SkillFileInput + if rawFiles, ok := args["files"]; ok { + items, ok := rawFiles.([]any) + if !ok { + return mcpgo.NewToolResultError("skill_publish: 'files' must be an array"), nil + } + parsedFiles, err := parseSkillPublishFiles(items) + if err != nil { + return mcpgo.NewToolResultError(fmt.Sprintf("skill_publish: files: %v", err)), nil + } + files = parsedFiles + } + + created, err := s.sklStore.Create(ctx, skills.CreateInput{ + ScopeID: scopeID, + AuthorID: authorID, + Slug: draft.Slug, + Name: draft.Name, + Description: draft.Description, + AgentTypes: draft.AgentTypes, + Body: draft.Body, + Visibility: visibility, + Parameters: nil, + Files: files, + Status: "published", + PublishedAt: ptrTime(time.Now().UTC()), + ReviewRequired: 1, + }) + if err != nil { + return mcpgo.NewToolResultError(fmt.Sprintf("skill_publish: create: %v", err)), nil + } + + out, _ := json.Marshal(map[string]any{ + "skill_id": created.ID.String(), + "slug": created.Slug, + "status": created.Status, + "version": created.Version, + }) + return mcpgo.NewToolResultText(string(out)), nil +} + +func parseSkillMarkdownContent(raw string) (skillDraft, error) { + raw = strings.ReplaceAll(raw, "\r\n", "\n") + if !strings.HasPrefix(raw, "---\n") { + return skillDraft{Body: raw}, nil + } + + rest := strings.TrimPrefix(raw, "---\n") + idx := strings.Index(rest, "\n---\n") + delimiterLen := len("\n---\n") + if idx < 0 && strings.HasSuffix(rest, "\n---") { + idx = len(rest) - len("\n---") + delimiterLen = len("\n---") + } + if idx < 0 { + return skillDraft{}, fmt.Errorf("frontmatter opening found but closing '---' is missing") + } + + fm := rest[:idx] + body := rest[idx+delimiterLen:] + body = strings.TrimPrefix(body, "\n") + draft := skillDraft{Body: body} + + lines := strings.Split(fm, "\n") + inAgentTypes := false + for _, line := range lines { + trimmed := strings.TrimSpace(line) + if trimmed == "" { + continue + } + if inAgentTypes { + if strings.HasPrefix(trimmed, "- ") { + v := strings.TrimSpace(strings.TrimPrefix(trimmed, "- ")) + if v != "" { + draft.AgentTypes = append(draft.AgentTypes, v) + } + continue + } + inAgentTypes = false + } + + switch { + case strings.HasPrefix(trimmed, "name:"): + draft.Name = strings.TrimSpace(strings.TrimPrefix(trimmed, "name:")) + case strings.HasPrefix(trimmed, "description:"): + draft.Description = strings.TrimSpace(strings.TrimPrefix(trimmed, "description:")) + case strings.HasPrefix(trimmed, "agent_types:"): + rawValue := strings.TrimSpace(strings.TrimPrefix(trimmed, "agent_types:")) + if rawValue == "" { + inAgentTypes = true + continue + } + if strings.HasPrefix(rawValue, "[") { + var parsed []string + if err := json.Unmarshal([]byte(rawValue), &parsed); err != nil { + return skillDraft{}, fmt.Errorf("invalid agent_types inline list: %w", err) + } + draft.AgentTypes = append(draft.AgentTypes, parsed...) + } + } + } + + return draft, nil +} + +func defaultSkillSlug(slug, name, sourceName string) string { + if normalized := normalizeSlugCandidate(slug); normalized != "" { + return normalized + } + sourceName = strings.ReplaceAll(sourceName, "\\", "/") + if normalized := normalizeSlugCandidate(strings.TrimSuffix(path.Base(sourceName), path.Ext(sourceName))); normalized != "" { + return normalized + } + return normalizeSlugCandidate(name) +} + +func normalizeSlugCandidate(raw string) string { + raw = strings.ToLower(strings.TrimSpace(raw)) + if raw == "" { + return "" + } + var b strings.Builder + lastDash := false + for _, r := range raw { + if (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') { + b.WriteRune(r) + lastDash = false + continue + } + if r == '-' || r == '_' { + b.WriteRune(r) + lastDash = false + continue + } + if !lastDash { + b.WriteRune('-') + lastDash = true + } + } + out := strings.Trim(b.String(), "-_") + return out +} + +func titleFromSlug(slug string) string { + parts := strings.FieldsFunc(slug, func(r rune) bool { return r == '-' || r == '_' }) + for i, p := range parts { + if p == "" { + continue + } + parts[i] = strings.ToUpper(p[:1]) + p[1:] + } + if len(parts) == 0 { + return slug + } + return strings.Join(parts, " ") +} + +func parseSkillPublishFiles(raw []any) ([]db.SkillFileInput, error) { + files := make([]db.SkillFileInput, 0, len(raw)) + for i, item := range raw { + obj, ok := item.(map[string]any) + if !ok { + return nil, fmt.Errorf("item %d must be an object", i) + } + relativePath, ok := obj["path"].(string) + if !ok || strings.TrimSpace(relativePath) == "" { + return nil, fmt.Errorf("item %d: path is required", i) + } + content, ok := obj["content"].(string) + if !ok { + return nil, fmt.Errorf("item %d: content must be a string", i) + } + executable, _ := obj["executable"].(bool) + f := db.SkillFileInput{ + RelativePath: strings.TrimSpace(relativePath), + Content: content, + IsExecutable: executable, + } + if err := skills.ValidateSkillFile(f); err != nil { + return nil, fmt.Errorf("item %d: %w", i, err) + } + files = append(files, f) + } + return files, nil +} + +func ptrTime(t time.Time) *time.Time { + return &t +} diff --git a/internal/api/mcp/skill_publish_test.go b/internal/api/mcp/skill_publish_test.go new file mode 100644 index 0000000..f21a9a4 --- /dev/null +++ b/internal/api/mcp/skill_publish_test.go @@ -0,0 +1,201 @@ +package mcp + +import ( + "encoding/json" + "strings" + "testing" +) + +func TestParseSkillMarkdownContent_WithFrontmatter(t *testing.T) { + t.Parallel() + + content := strings.Join([]string{ + "---", + "name: Tox Verifier", + "description: Verify tox output and summarize failures", + "agent_types: [\"codex\", \"claude-code\"]", + "---", + "", + "Check tox output and report issues.", + }, "\n") + + draft, err := parseSkillMarkdownContent(content) + if err != nil { + t.Fatalf("parseSkillMarkdownContent: %v", err) + } + if draft.Name != "Tox Verifier" { + t.Fatalf("name = %q, want %q", draft.Name, "Tox Verifier") + } + if draft.Description != "Verify tox output and summarize failures" { + t.Fatalf("description = %q, want %q", draft.Description, "Verify tox output and summarize failures") + } + if draft.Body != "Check tox output and report issues." { + t.Fatalf("body = %q, want %q", draft.Body, "Check tox output and report issues.") + } + if len(draft.AgentTypes) != 2 || draft.AgentTypes[0] != "codex" || draft.AgentTypes[1] != "claude-code" { + t.Fatalf("agent_types = %#v, want [codex claude-code]", draft.AgentTypes) + } +} + +func TestParseSkillMarkdownContent_WithYamlAgentTypeList(t *testing.T) { + t.Parallel() + + content := strings.Join([]string{ + "---", + "name: Tox Verifier", + "agent_types:", + " - codex", + " - claude-code", + "---", + "", + "Body", + }, "\n") + + draft, err := parseSkillMarkdownContent(content) + if err != nil { + t.Fatalf("parseSkillMarkdownContent: %v", err) + } + if len(draft.AgentTypes) != 2 || draft.AgentTypes[0] != "codex" || draft.AgentTypes[1] != "claude-code" { + t.Fatalf("agent_types = %#v, want [codex claude-code]", draft.AgentTypes) + } +} + +func TestParseSkillMarkdownContent_InvalidInlineAgentTypes_ReturnsError(t *testing.T) { + t.Parallel() + + content := strings.Join([]string{ + "---", + "name: Tox Verifier", + `agent_types: [codex, "claude-code"]`, + "---", + "", + "Body", + }, "\n") + + _, err := parseSkillMarkdownContent(content) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "invalid agent_types inline list") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestParseSkillMarkdownContent_WithoutFrontmatter_UsesWholeBody(t *testing.T) { + t.Parallel() + + content := "Run tox -e py and summarize failures." + draft, err := parseSkillMarkdownContent(content) + if err != nil { + t.Fatalf("parseSkillMarkdownContent: %v", err) + } + if draft.Body != content { + t.Fatalf("body = %q, want %q", draft.Body, content) + } +} + +func TestDefaultSkillSlug_FromSourceName(t *testing.T) { + t.Parallel() + + got := defaultSkillSlug("", "", "tox-verifier.md") + if got != "tox-verifier" { + t.Fatalf("defaultSkillSlug = %q, want %q", got, "tox-verifier") + } +} + +func TestDefaultSkillSlug_FromSourceNamePath_SeparatorStable(t *testing.T) { + t.Parallel() + + gotForward := defaultSkillSlug("", "", "skills/tox-verifier.md") + if gotForward != "tox-verifier" { + t.Fatalf("defaultSkillSlug forward = %q, want %q", gotForward, "tox-verifier") + } + + gotBackward := defaultSkillSlug("", "", `skills\tox-verifier.md`) + if gotBackward != "tox-verifier" { + t.Fatalf("defaultSkillSlug backward = %q, want %q", gotBackward, "tox-verifier") + } +} + +func TestParseSkillPublishFiles_Valid(t *testing.T) { + t.Parallel() + + raw := []any{ + map[string]any{ + "path": "scripts/run.sh", + "content": "#!/bin/sh\necho hi\n", + "executable": true, + }, + map[string]any{ + "path": "references/usage.md", + "content": "Use it like this.", + }, + } + files, err := parseSkillPublishFiles(raw) + if err != nil { + t.Fatalf("parseSkillPublishFiles: %v", err) + } + if len(files) != 2 { + t.Fatalf("len(files) = %d, want 2", len(files)) + } + if !files[0].IsExecutable || files[0].RelativePath != "scripts/run.sh" { + t.Fatalf("file[0] = %#v", files[0]) + } + if files[1].IsExecutable || files[1].RelativePath != "references/usage.md" { + t.Fatalf("file[1] = %#v", files[1]) + } +} + +func TestParseSkillPublishFiles_InvalidPath_ReturnsError(t *testing.T) { + t.Parallel() + + raw := []any{ + map[string]any{ + "path": "../evil.sh", + "content": "oops", + }, + } + _, err := parseSkillPublishFiles(raw) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "item 0:") { + t.Fatalf("expected item index in error, got: %v", err) + } + if !strings.Contains(err.Error(), "relative_path") && !strings.Contains(err.Error(), "traversal") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestParseSkillPublishFiles_InvalidType_ReturnsError(t *testing.T) { + t.Parallel() + + _, err := parseSkillPublishFiles([]any{"not-an-object"}) + if err == nil { + t.Fatal("expected error, got nil") + } +} + +func TestParseSkillPublishFiles_CompatWithJSONDecodeShape(t *testing.T) { + t.Parallel() + + var decoded struct { + Files []any `json:"files"` + } + if err := json.Unmarshal([]byte(`{ + "files": [ + {"path":"scripts/run.sh","content":"#!/bin/sh","executable":true}, + {"path":"references/usage.md","content":"docs"} + ] + }`), &decoded); err != nil { + t.Fatalf("json unmarshal: %v", err) + } + + files, err := parseSkillPublishFiles(decoded.Files) + if err != nil { + t.Fatalf("parseSkillPublishFiles: %v", err) + } + if len(files) != 2 { + t.Fatalf("len(files) = %d, want 2", len(files)) + } +} diff --git a/internal/skills/store.go b/internal/skills/store.go index 1bc5347..4d553e6 100644 --- a/internal/skills/store.go +++ b/internal/skills/store.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "time" "github.com/google/uuid" "github.com/jackc/pgx/v5/pgxpool" @@ -53,6 +54,10 @@ func NewStore(pool *pgxpool.Pool, svc *providers.EmbeddingService) *Store { } // CreateInput holds the fields required to create a new skill. +// +// By default Create writes a draft skill (Status defaults to "draft"). +// Callers may optionally set Status to "published" and provide PublishedAt +// to create an already published skill in one operation. type CreateInput struct { ScopeID uuid.UUID AuthorID uuid.UUID @@ -64,7 +69,9 @@ type CreateInput struct { Body string Parameters []db.SkillParameter Visibility string - ReviewRequired int // default 1 if 0 + ReviewRequired int // default 1 if 0 + Status string // default "draft"; set to "published" for immediate publish + PublishedAt *time.Time // used when Status=="published"; ignored otherwise // Files are optional supplementary files to attach to the skill. // Each path must pass ValidateSkillFile; requires a non-nil pool. Files []db.SkillFileInput @@ -79,7 +86,11 @@ type UpdateInput struct { Files *[]db.SkillFileInput } -// Create persists a new skill in draft status, embedding its description+body. +// Create persists a new skill, embedding its description+body. +// +// Status defaults to "draft" when unset. If input.Status is "published", +// Create also persists a non-nil published_at timestamp in the same create +// operation to avoid a separate publish-status update step. // When Files are provided, the skill row and all supplementary files are written // in a single transaction so a mid-file failure cannot leave an orphaned skill. func (s *Store) Create(ctx context.Context, input CreateInput) (*db.Skill, error) { @@ -90,6 +101,17 @@ func (s *Store) Create(ctx context.Context, input CreateInput) (*db.Skill, error if input.ReviewRequired == 0 { input.ReviewRequired = 1 } + if input.Status == "" { + input.Status = "draft" + } + if input.Status == "published" { + if input.PublishedAt == nil { + now := time.Now().UTC() + input.PublishedAt = &now + } + } else { + input.PublishedAt = nil + } // Validate supplementary files before any expensive work or DB writes. for _, f := range input.Files { @@ -126,7 +148,8 @@ func (s *Store) Create(ctx context.Context, input CreateInput) (*db.Skill, error Body: input.Body, Parameters: paramsJSON, Visibility: input.Visibility, - Status: "draft", + Status: input.Status, + PublishedAt: input.PublishedAt, ReviewRequired: int32(input.ReviewRequired), Version: 1, Embedding: &embVec, diff --git a/internal/skills/store_test.go b/internal/skills/store_test.go index fb20da3..cfcdaa5 100644 --- a/internal/skills/store_test.go +++ b/internal/skills/store_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "testing" + "time" "github.com/google/uuid" "github.com/simplyblock/postbrain/internal/db" @@ -110,6 +111,58 @@ func TestCreate_DefaultReviewRequired(t *testing.T) { } } +func TestCreate_DefaultStatusDraft(t *testing.T) { + t.Parallel() + fdb := &fakeDB{} + s := newTestStore(fdb) + input := CreateInput{ + ScopeID: uuid.New(), + AuthorID: uuid.New(), + Slug: "test-skill", + Name: "Test Skill", + Body: "Do the thing.", + Visibility: "team", + } + skill, err := s.Create(context.Background(), input) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if skill.Status != "draft" { + t.Fatalf("status = %q, want %q", skill.Status, "draft") + } + if skill.PublishedAt != nil { + t.Fatalf("published_at = %v, want nil", skill.PublishedAt) + } +} + +func TestCreate_CustomPublishedStatus(t *testing.T) { + t.Parallel() + fdb := &fakeDB{} + s := newTestStore(fdb) + now := time.Now().UTC().Truncate(time.Second) + input := CreateInput{ + ScopeID: uuid.New(), + AuthorID: uuid.New(), + Slug: "test-skill", + Name: "Test Skill", + Body: "Do the thing.", + Visibility: "team", + Status: "published", + PublishedAt: &now, + ReviewRequired: 1, + } + skill, err := s.Create(context.Background(), input) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if skill.Status != "published" { + t.Fatalf("status = %q, want %q", skill.Status, "published") + } + if skill.PublishedAt == nil || !skill.PublishedAt.Equal(now) { + t.Fatalf("published_at = %v, want %v", skill.PublishedAt, now) + } +} + func TestCreate_ParametersSerialized(t *testing.T) { t.Parallel() fdb := &fakeDB{}