From 83048cee9b15f794d98682c91f00e9843c2d890d Mon Sep 17 00:00:00 2001 From: "Christoph Engelbert (noctarius)" Date: Fri, 24 Apr 2026 16:21:23 +0200 Subject: [PATCH 01/12] Add MCP skill_publish tool for markdown-based skill creation Add a new MCP tool, skill_publish, so agents can create and publish skills into a scope directly from SKILL.md-style markdown or explicit fields. The handler enforces scope authorization, derives slug/name defaults, parses optional frontmatter, creates the skill, and marks it published so it is immediately discoverable/installable. Also add parser/unit tests, handler validation tests, and scope-auth inventory/integration coverage for the new scope-taking tool. Co-authored-by: Codex --- designs/TASKS.md | 9 + internal/api/mcp/handlers_unit_test.go | 18 ++ .../api/mcp/scope_authz_integration_test.go | 17 ++ internal/api/mcp/scopeauth_inventory_test.go | 1 + internal/api/mcp/server.go | 1 + internal/api/mcp/skill_publish.go | 262 ++++++++++++++++++ internal/api/mcp/skill_publish_test.go | 82 ++++++ 7 files changed, 390 insertions(+) create mode 100644 internal/api/mcp/skill_publish.go create mode 100644 internal/api/mcp/skill_publish_test.go diff --git a/designs/TASKS.md b/designs/TASKS.md index 0932634e..625a5b1e 100644 --- a/designs/TASKS.md +++ b/designs/TASKS.md @@ -25,6 +25,15 @@ ## 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. + - Tool accepts either full markdown `content` (with optional `SKILL.md` frontmatter) or explicit `body` + metadata and derives missing slug/name fields. + - 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` 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 00f0cce0..6a0804bf 100644 --- a/internal/api/mcp/handlers_unit_test.go +++ b/internal/api/mcp/handlers_unit_test.go @@ -528,3 +528,21 @@ 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) +} diff --git a/internal/api/mcp/scope_authz_integration_test.go b/internal/api/mcp/scope_authz_integration_test.go index 01de3e8d..8a94770d 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 dfa79310..bc1e68d4 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 b3dde243..275811f7 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 00000000..8f67a40a --- /dev/null +++ b/internal/api/mcp/skill_publish.go @@ -0,0 +1,262 @@ +package mcp + +import ( + "context" + "encoding/json" + "fmt" + "path/filepath" + "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/compat" + "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.Items(map[string]any{"type": "string"}), + ), + ), 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 + } + + 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: nil, + ReviewRequired: 1, + }) + if err != nil { + return mcpgo.NewToolResultError(fmt.Sprintf("skill_publish: create: %v", err)), nil + } + + // MCP publish should result in a consumable registry skill immediately. + now := time.Now().UTC() + if err := compat.UpdateSkillStatus(ctx, s.pool, created.ID, "published", &now, nil); err != nil { + return mcpgo.NewToolResultError(fmt.Sprintf("skill_publish: set published status: %v", err)), nil + } + + out, _ := json.Marshal(map[string]any{ + "skill_id": created.ID.String(), + "slug": created.Slug, + "status": "published", + "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") + if idx < 0 { + return skillDraft{}, fmt.Errorf("frontmatter opening found but closing '---' is missing") + } + + fm := rest[:idx] + body := rest[idx+len("\n---\n"):] + 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 { + draft.AgentTypes = append(draft.AgentTypes, parsed...) + } + } + } + } + + return draft, nil +} + +func defaultSkillSlug(slug, name, sourceName string) string { + if normalized := normalizeSlugCandidate(slug); normalized != "" { + return normalized + } + if normalized := normalizeSlugCandidate(strings.TrimSuffix(filepath.Base(sourceName), filepath.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, " ") +} diff --git a/internal/api/mcp/skill_publish_test.go b/internal/api/mcp/skill_publish_test.go new file mode 100644 index 00000000..b94f1ba4 --- /dev/null +++ b/internal/api/mcp/skill_publish_test.go @@ -0,0 +1,82 @@ +package mcp + +import ( + "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_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") + } +} From db6ac4b01119d11a66f51ee58440f1638542ea76 Mon Sep 17 00:00:00 2001 From: "Christoph Engelbert (noctarius)" Date: Fri, 24 Apr 2026 16:26:31 +0200 Subject: [PATCH 02/12] Add multi-file support to MCP skill_publish Extend skill_publish to accept a files array and persist supplementary skill files (scripts/references) through skills.Store.Create. Add validation/parsing for file payloads using existing skill file safety rules and return input errors for malformed file structures. Add unit tests for file payload parsing and an integration test verifying published skills store multiple files successfully. Co-authored-by: Codex --- designs/TASKS.md | 3 +- internal/api/mcp/handlers_unit_test.go | 10 +++ internal/api/mcp/mcp_integration_test.go | 87 ++++++++++++++++++++++++ internal/api/mcp/skill_publish.go | 56 ++++++++++++++- internal/api/mcp/skill_publish_test.go | 81 ++++++++++++++++++++++ 5 files changed, 235 insertions(+), 2 deletions(-) diff --git a/designs/TASKS.md b/designs/TASKS.md index 625a5b1e..e71f6cc2 100644 --- a/designs/TASKS.md +++ b/designs/TASKS.md @@ -28,8 +28,9 @@ - [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. - Tool accepts either full markdown `content` (with optional `SKILL.md` frontmatter) or explicit `body` + metadata and derives missing slug/name fields. + - 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` and handler validation coverage in `internal/api/mcp/handlers_unit_test.go`. + - 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` diff --git a/internal/api/mcp/handlers_unit_test.go b/internal/api/mcp/handlers_unit_test.go index 6a0804bf..378b309f 100644 --- a/internal/api/mcp/handlers_unit_test.go +++ b/internal/api/mcp/handlers_unit_test.go @@ -546,3 +546,13 @@ func TestHandleSkillPublish_MissingContentAndBody_ReturnsToolError(t *testing.T) }, s.handleSkillPublish) assertToolError(t, result) } + +func TestHandleSkillPublish_InvalidFilesType_ReturnsToolError(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) +} diff --git a/internal/api/mcp/mcp_integration_test.go b/internal/api/mcp/mcp_integration_test.go index fbb118af..1aaa48fa 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/skill_publish.go b/internal/api/mcp/skill_publish.go index 8f67a40a..3ddca872 100644 --- a/internal/api/mcp/skill_publish.go +++ b/internal/api/mcp/skill_publish.go @@ -12,6 +12,7 @@ import ( mcpgo "github.com/mark3labs/mcp-go/mcp" "github.com/simplyblock/postbrain/internal/auth" + "github.com/simplyblock/postbrain/internal/db" "github.com/simplyblock/postbrain/internal/db/compat" "github.com/simplyblock/postbrain/internal/skills" ) @@ -33,6 +34,17 @@ func (s *Server) registerSkillPublish() { mcpgo.WithArray("agent_types", mcpgo.Description("Compatible agent types (default: [\"any\"])"), mcpgo.Items(map[string]any{"type": "string"}), ), + 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))) } @@ -120,6 +132,19 @@ func (s *Server) handleSkillPublish(ctx context.Context, req mcpgo.CallToolReque 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, @@ -130,7 +155,7 @@ func (s *Server) handleSkillPublish(ctx context.Context, req mcpgo.CallToolReque Body: draft.Body, Visibility: visibility, Parameters: nil, - Files: nil, + Files: files, ReviewRequired: 1, }) if err != nil { @@ -260,3 +285,32 @@ func titleFromSlug(slug string) string { } 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) + } + path, ok := obj["path"].(string) + if !ok || strings.TrimSpace(path) == "" { + 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(path), + Content: content, + IsExecutable: executable, + } + if err := skills.ValidateSkillFile(f); err != nil { + return nil, err + } + files = append(files, f) + } + return files, nil +} diff --git a/internal/api/mcp/skill_publish_test.go b/internal/api/mcp/skill_publish_test.go index b94f1ba4..88bd9291 100644 --- a/internal/api/mcp/skill_publish_test.go +++ b/internal/api/mcp/skill_publish_test.go @@ -1,6 +1,7 @@ package mcp import ( + "encoding/json" "strings" "testing" ) @@ -80,3 +81,83 @@ func TestDefaultSkillSlug_FromSourceName(t *testing.T) { t.Fatalf("defaultSkillSlug = %q, want %q", got, "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(), "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)) + } +} From b90f3e265a256a090a8e5d12f6cc03e0db445bde Mon Sep 17 00:00:00 2001 From: "Christoph Engelbert (noctarius)" Date: Fri, 24 Apr 2026 16:27:46 +0200 Subject: [PATCH 03/12] Updated golangci --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index faf0c0f1..3fc45b29 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 From a12b317a8b62e1343a014bb53e806cc99cc3b730 Mon Sep 17 00:00:00 2001 From: noctarius aka Christoph Engelbert Date: Fri, 24 Apr 2026 17:06:55 +0200 Subject: [PATCH 04/12] Update internal/api/mcp/skill_publish.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- internal/api/mcp/skill_publish.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/api/mcp/skill_publish.go b/internal/api/mcp/skill_publish.go index 3ddca872..c0e70106 100644 --- a/internal/api/mcp/skill_publish.go +++ b/internal/api/mcp/skill_publish.go @@ -185,12 +185,17 @@ func parseSkillMarkdownContent(raw string) (skillDraft, error) { 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+len("\n---\n"):] + body := rest[idx+delimiterLen:] body = strings.TrimPrefix(body, "\n") draft := skillDraft{Body: body} From c0e948b37a2b3e3978c695e89e350741c56b7a6a Mon Sep 17 00:00:00 2001 From: "Christoph Engelbert (noctarius)" Date: Fri, 24 Apr 2026 17:12:03 +0200 Subject: [PATCH 05/12] Normalize skill_publish source_name path handling Make default slug derivation from source_name separator-stable across platforms by normalizing backslashes to forward slashes and using the path package for basename/extension extraction. This keeps behavior consistent for client-provided names regardless of server OS. Add regression coverage for both slash styles in skill_publish tests. Co-authored-by: Codex --- designs/TASKS.md | 1 + internal/api/mcp/skill_publish.go | 5 +++-- internal/api/mcp/skill_publish_test.go | 14 ++++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/designs/TASKS.md b/designs/TASKS.md index e71f6cc2..f82b3b90 100644 --- a/designs/TASKS.md +++ b/designs/TASKS.md @@ -28,6 +28,7 @@ - [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. - Tool accepts either full markdown `content` (with optional `SKILL.md` frontmatter) or explicit `body` + metadata and derives missing slug/name fields. + - 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`. diff --git a/internal/api/mcp/skill_publish.go b/internal/api/mcp/skill_publish.go index c0e70106..b86bac89 100644 --- a/internal/api/mcp/skill_publish.go +++ b/internal/api/mcp/skill_publish.go @@ -4,7 +4,7 @@ import ( "context" "encoding/json" "fmt" - "path/filepath" + "path" "strings" "time" @@ -244,7 +244,8 @@ func defaultSkillSlug(slug, name, sourceName string) string { if normalized := normalizeSlugCandidate(slug); normalized != "" { return normalized } - if normalized := normalizeSlugCandidate(strings.TrimSuffix(filepath.Base(sourceName), filepath.Ext(sourceName))); normalized != "" { + sourceName = strings.ReplaceAll(sourceName, "\\", "/") + if normalized := normalizeSlugCandidate(strings.TrimSuffix(path.Base(sourceName), path.Ext(sourceName))); normalized != "" { return normalized } return normalizeSlugCandidate(name) diff --git a/internal/api/mcp/skill_publish_test.go b/internal/api/mcp/skill_publish_test.go index 88bd9291..30005da5 100644 --- a/internal/api/mcp/skill_publish_test.go +++ b/internal/api/mcp/skill_publish_test.go @@ -82,6 +82,20 @@ func TestDefaultSkillSlug_FromSourceName(t *testing.T) { } } +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() From b40b7f92556bd5711e861f43de565a81da1057da Mon Sep 17 00:00:00 2001 From: "Christoph Engelbert (noctarius)" Date: Fri, 24 Apr 2026 17:15:08 +0200 Subject: [PATCH 06/12] Make MCP skill publish atomic in a single create operation Avoid partial publish failures in skill_publish by creating skills directly with published status and published_at, instead of doing Create followed by UpdateSkillStatus in a second DB operation. Extend skills.CreateInput with optional initial status/published_at fields (defaulting to draft for existing callers) and add unit coverage for default and published create paths. Co-authored-by: Codex --- designs/TASKS.md | 1 + internal/api/mcp/skill_publish.go | 15 ++++----- internal/skills/store.go | 9 +++++- internal/skills/store_test.go | 53 +++++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 9 deletions(-) diff --git a/designs/TASKS.md b/designs/TASKS.md index f82b3b90..a9318064 100644 --- a/designs/TASKS.md +++ b/designs/TASKS.md @@ -27,6 +27,7 @@ - [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). - Tool accepts either full markdown `content` (with optional `SKILL.md` frontmatter) or explicit `body` + metadata and derives missing slug/name fields. - 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. diff --git a/internal/api/mcp/skill_publish.go b/internal/api/mcp/skill_publish.go index b86bac89..cb821816 100644 --- a/internal/api/mcp/skill_publish.go +++ b/internal/api/mcp/skill_publish.go @@ -13,7 +13,6 @@ import ( "github.com/simplyblock/postbrain/internal/auth" "github.com/simplyblock/postbrain/internal/db" - "github.com/simplyblock/postbrain/internal/db/compat" "github.com/simplyblock/postbrain/internal/skills" ) @@ -156,22 +155,18 @@ func (s *Server) handleSkillPublish(ctx context.Context, req mcpgo.CallToolReque 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 } - // MCP publish should result in a consumable registry skill immediately. - now := time.Now().UTC() - if err := compat.UpdateSkillStatus(ctx, s.pool, created.ID, "published", &now, nil); err != nil { - return mcpgo.NewToolResultError(fmt.Sprintf("skill_publish: set published status: %v", err)), nil - } - out, _ := json.Marshal(map[string]any{ "skill_id": created.ID.String(), "slug": created.Slug, - "status": "published", + "status": created.Status, "version": created.Version, }) return mcpgo.NewToolResultText(string(out)), nil @@ -320,3 +315,7 @@ func parseSkillPublishFiles(raw []any) ([]db.SkillFileInput, error) { } return files, nil } + +func ptrTime(t time.Time) *time.Time { + return &t +} diff --git a/internal/skills/store.go b/internal/skills/store.go index 1bc53474..2cbdedd2 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" @@ -65,6 +66,8 @@ type CreateInput struct { Parameters []db.SkillParameter Visibility string ReviewRequired int // default 1 if 0 + Status string + PublishedAt *time.Time // Files are optional supplementary files to attach to the skill. // Each path must pass ValidateSkillFile; requires a non-nil pool. Files []db.SkillFileInput @@ -90,6 +93,9 @@ 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" + } // Validate supplementary files before any expensive work or DB writes. for _, f := range input.Files { @@ -126,7 +132,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 fb20da35..cfcdaa55 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{} From 694a7c090f9f7a25b7de4a2d3a338c73b580b6a9 Mon Sep 17 00:00:00 2001 From: "Christoph Engelbert (noctarius)" Date: Fri, 24 Apr 2026 17:17:24 +0200 Subject: [PATCH 07/12] Clarify skill_publish guard-order unit test intent Rename the skill_publish unit test that passed a malformed files value under an unconfigured server and assert the returned error text explicitly reflects the early server-not-configured guard. This aligns test naming with actual behavior and avoids implying files validation was reached. Co-authored-by: Codex --- internal/api/mcp/handlers_unit_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/api/mcp/handlers_unit_test.go b/internal/api/mcp/handlers_unit_test.go index 378b309f..a1182eef 100644 --- a/internal/api/mcp/handlers_unit_test.go +++ b/internal/api/mcp/handlers_unit_test.go @@ -547,7 +547,7 @@ func TestHandleSkillPublish_MissingContentAndBody_ReturnsToolError(t *testing.T) assertToolError(t, result) } -func TestHandleSkillPublish_InvalidFilesType_ReturnsToolError(t *testing.T) { +func TestHandleSkillPublish_ServerNotConfigured_PrecedesFilesValidation(t *testing.T) { s := &Server{} result := callTool(t, map[string]any{ "scope": "project:acme/api", @@ -555,4 +555,8 @@ func TestHandleSkillPublish_InvalidFilesType_ReturnsToolError(t *testing.T) { "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) + } } From 7a198f3b7fced8bae89b419ea3786bd105654c83 Mon Sep 17 00:00:00 2001 From: noctarius aka Christoph Engelbert Date: Fri, 24 Apr 2026 17:24:29 +0200 Subject: [PATCH 08/12] Update internal/skills/store.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- internal/skills/store.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/skills/store.go b/internal/skills/store.go index 2cbdedd2..96382991 100644 --- a/internal/skills/store.go +++ b/internal/skills/store.go @@ -96,6 +96,14 @@ func (s *Store) Create(ctx context.Context, input CreateInput) (*db.Skill, error 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 { From 7581f57b6383982032165bbab6a5b9d669a3ec9a Mon Sep 17 00:00:00 2001 From: "Christoph Engelbert (noctarius)" Date: Fri, 24 Apr 2026 17:27:02 +0200 Subject: [PATCH 09/12] Clarify Store.Create draft vs published creation semantics Update skills store API documentation to reflect current behavior: Create defaults to draft status but may create published skills when CreateInput.Status and PublishedAt are provided. Adjust comments on CreateInput and Store.Create to make this explicit for callers. Co-authored-by: Codex --- designs/TASKS.md | 1 + internal/skills/store.go | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/designs/TASKS.md b/designs/TASKS.md index a9318064..57cb97f7 100644 --- a/designs/TASKS.md +++ b/designs/TASKS.md @@ -28,6 +28,7 @@ - [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. - 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. diff --git a/internal/skills/store.go b/internal/skills/store.go index 96382991..4d553e6e 100644 --- a/internal/skills/store.go +++ b/internal/skills/store.go @@ -54,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 @@ -65,9 +69,9 @@ type CreateInput struct { Body string Parameters []db.SkillParameter Visibility string - ReviewRequired int // default 1 if 0 - Status string - PublishedAt *time.Time + 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 @@ -82,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) { From 626f5d2d3747643b01486172cc7a647bf7a7b743 Mon Sep 17 00:00:00 2001 From: "Christoph Engelbert (noctarius)" Date: Fri, 24 Apr 2026 17:28:51 +0200 Subject: [PATCH 10/12] Fail fast on invalid inline agent_types in skill frontmatter Change skill_publish frontmatter parsing to return an explicit error when agent_types is provided as an inline list but is not valid JSON array syntax, instead of silently ignoring parse failures and falling back to default compatibility. Add regression coverage for invalid inline agent_types parsing and update TASKS.md tracking. Co-authored-by: Codex --- designs/TASKS.md | 1 + internal/api/mcp/skill_publish.go | 5 +++-- internal/api/mcp/skill_publish_test.go | 21 +++++++++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/designs/TASKS.md b/designs/TASKS.md index 57cb97f7..4e187c62 100644 --- a/designs/TASKS.md +++ b/designs/TASKS.md @@ -30,6 +30,7 @@ - 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. - 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. diff --git a/internal/api/mcp/skill_publish.go b/internal/api/mcp/skill_publish.go index cb821816..bc6dfcf8 100644 --- a/internal/api/mcp/skill_publish.go +++ b/internal/api/mcp/skill_publish.go @@ -225,9 +225,10 @@ func parseSkillMarkdownContent(raw string) (skillDraft, error) { } if strings.HasPrefix(rawValue, "[") { var parsed []string - if err := json.Unmarshal([]byte(rawValue), &parsed); err == nil { - draft.AgentTypes = append(draft.AgentTypes, parsed...) + 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...) } } } diff --git a/internal/api/mcp/skill_publish_test.go b/internal/api/mcp/skill_publish_test.go index 30005da5..f4d070a1 100644 --- a/internal/api/mcp/skill_publish_test.go +++ b/internal/api/mcp/skill_publish_test.go @@ -60,6 +60,27 @@ func TestParseSkillMarkdownContent_WithYamlAgentTypeList(t *testing.T) { } } +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() From 96517e975be87c4fb12a5a3d2179197c8e225f99 Mon Sep 17 00:00:00 2001 From: "Christoph Engelbert (noctarius)" Date: Fri, 24 Apr 2026 17:30:42 +0200 Subject: [PATCH 11/12] Use WithStringItems for skill_publish agent_types schema Switch skill_publish agent_types array schema declaration to mcpgo.WithStringItems() for consistency with other MCP tools and stricter client schema handling. Update TASKS.md to track the MCP schema consistency adjustment. Co-authored-by: Codex --- designs/TASKS.md | 1 + internal/api/mcp/skill_publish.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/designs/TASKS.md b/designs/TASKS.md index 4e187c62..4638c3f3 100644 --- a/designs/TASKS.md +++ b/designs/TASKS.md @@ -31,6 +31,7 @@ - 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. + - 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. diff --git a/internal/api/mcp/skill_publish.go b/internal/api/mcp/skill_publish.go index bc6dfcf8..b8747e98 100644 --- a/internal/api/mcp/skill_publish.go +++ b/internal/api/mcp/skill_publish.go @@ -31,7 +31,7 @@ func (s *Server) registerSkillPublish() { 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.Items(map[string]any{"type": "string"}), + mcpgo.WithStringItems(), ), mcpgo.WithArray("files", mcpgo.Description("Optional supplementary files, e.g. scripts/* and references/*.md"), mcpgo.Items(map[string]any{ From 8be8fab5aa2497b3fab5d48a707c24831c9ff1c0 Mon Sep 17 00:00:00 2001 From: "Christoph Engelbert (noctarius)" Date: Fri, 24 Apr 2026 17:32:17 +0200 Subject: [PATCH 12/12] Clarify skill_publish file-parse naming and error context Rename the local file path variable in parseSkillPublishFiles from path to relativePath to avoid confusion with the path package, and wrap ValidateSkillFile failures with the item index so multi-file payload errors are easier to diagnose. Add/adjust tests to assert indexed error context and update TASKS.md. Co-authored-by: Codex --- designs/TASKS.md | 1 + internal/api/mcp/skill_publish.go | 8 ++++---- internal/api/mcp/skill_publish_test.go | 3 +++ 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/designs/TASKS.md b/designs/TASKS.md index 4638c3f3..e55cebf3 100644 --- a/designs/TASKS.md +++ b/designs/TASKS.md @@ -31,6 +31,7 @@ - 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. diff --git a/internal/api/mcp/skill_publish.go b/internal/api/mcp/skill_publish.go index b8747e98..2c94a56f 100644 --- a/internal/api/mcp/skill_publish.go +++ b/internal/api/mcp/skill_publish.go @@ -295,8 +295,8 @@ func parseSkillPublishFiles(raw []any) ([]db.SkillFileInput, error) { if !ok { return nil, fmt.Errorf("item %d must be an object", i) } - path, ok := obj["path"].(string) - if !ok || strings.TrimSpace(path) == "" { + 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) @@ -305,12 +305,12 @@ func parseSkillPublishFiles(raw []any) ([]db.SkillFileInput, error) { } executable, _ := obj["executable"].(bool) f := db.SkillFileInput{ - RelativePath: strings.TrimSpace(path), + RelativePath: strings.TrimSpace(relativePath), Content: content, IsExecutable: executable, } if err := skills.ValidateSkillFile(f); err != nil { - return nil, err + return nil, fmt.Errorf("item %d: %w", i, err) } files = append(files, f) } diff --git a/internal/api/mcp/skill_publish_test.go b/internal/api/mcp/skill_publish_test.go index f4d070a1..f21a9a44 100644 --- a/internal/api/mcp/skill_publish_test.go +++ b/internal/api/mcp/skill_publish_test.go @@ -159,6 +159,9 @@ func TestParseSkillPublishFiles_InvalidPath_ReturnsError(t *testing.T) { 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) }