Report Skill upload and update field errors

contracts/agents-api/core.openapi.yaml

@@ -6311,7 +6311,7 @@ paths:
           required: true
           type: string
       produces:
-        - application/octet-stream
+        - application/zip
       responses:
         "200":
           description: OK
@@ -6447,7 +6447,7 @@ paths:
           required: true
           type: string
       produces:
-        - application/octet-stream
+        - application/zip
       responses:
         "200":
           description: OK

contracts/agents-api/source-files.md

@@ -62,11 +62,11 @@ A copy into a workspace reads a consistent snapshot of the File and can finish a
 | Operation | Behavior |
 | --- | --- |
 | `POST /skills` | Uploads a new Skill. Its first version is both default and latest |
-| `POST /skills/{skill_id}/versions` | Uploads a new version. A `default` form field of `true` makes it the default; `false` or omitted leaves the default unchanged |
+| `POST /skills/{skill_id}/versions` | Uploads a new version. A `default` form field of `true` or `false`, in any letter case, makes it the default or leaves the default unchanged; omitted leaves it unchanged |
 | `GET /skills`, `GET /skills/{skill_id}` | Skill metadata, without decrypting any bundle |
-| `POST /skills/{skill_id}` | `{"default_version": "<n>"}` changes the default version |
+| `POST /skills/{skill_id}` | `{"default_version": "<n>"}` changes the default version. Body errors are field errors with the member as `param`; a version the Skill does not have returns 404 with `param: "default_version"` |
 | `DELETE /skills/{skill_id}` | Deletes the Skill and every version |
-| `GET /skills/{skill_id}/content` | ZIP of the default version |
+| `GET /skills/{skill_id}/content` | ZIP of the default version, `Content-Type: application/zip`, filename `<name>-<version>.zip` |
 | `GET /skills/{skill_id}/versions`, `GET /skills/{skill_id}/versions/{version}` | Version metadata. The list orders by version number, and `after` is a version ID (`skillver_…`), not a number |
 | `GET /skills/{skill_id}/versions/{version}/content` | ZIP of that version |
 | `DELETE /skills/{skill_id}/versions/{version}` | See [Delete a version](#delete-a-version) |
@@ -87,6 +87,8 @@ A bundle has one top-level folder containing `SKILL.md` and any supporting files
 - Entries are regular files or directories with clean relative paths. Links, special files, absolute paths, `..` components and duplicates are rejected.
 - Limits: 5 MiB compressed, 20 MiB expanded, 500 files, and 1,000 ZIP entries including directories.
 
+A rejected upload returns 400 `invalid_request_error` with `param: "files"` and code `invalid_value`, and a message that states the cause. A missing `files` part, an unknown part and `default` on create are `Missing required parameter` and `Unknown parameter` errors for that member; an invalid `default` names `param: "default"`. The upload routes also reject a `name` that contains `_` and a `description` longer than 1024 characters. Skills already stored, plugin Skills and Environment installation keep accepting `_` in the name and any non-empty description.
+
 Core encrypts each version's bundle bound to its Project, Skill and version. ZIP uploads keep executable bits; directory uploads store files with mode 0644.
 
 ### Versions and metadata

contracts/agents-api/zh/source-files.md

@@ -1,7 +1,7 @@
 ---
 title: "文件与 Skill"
 source: contracts/agents-api/source-files.md
-source_hash: 32041f334a2bf3141731fab69901fc97665da225e4e03328df0db7300faa18ff
+source_hash: a87f5745f8d96e7399726fa99cb8c69fc083ee03faf122bce7c4fc33ad95b5d0
 ---
 
 Files（`/v1/files`）和 Skills（`/v1/skills`）是具有独立生命周期的项目资源，不依赖 Session。File 保存上传字节，Environment 通过 ID 复制它们。Skill 保存不可变、版本化的包，由模板和 Session 引用。项目的所有 API 密钥共享这些资源。
@@ -64,11 +64,11 @@ Core 在自身数据库中以 PostgreSQL 大对象存储 File 字节。上传和
 | 操作 | 行为 |
 | --- | --- |
 | `POST /skills` | 上传新 Skill。首版同时为默认和最新版本 |
-| `POST /skills/{skill_id}/versions` | 上传新版本。表单 `default` 为 `true` 时设为默认；`false` 或省略时默认不变 |
+| `POST /skills/{skill_id}/versions` | 上传新版本。表单 `default` 取 `true` 或 `false`（不区分大小写），分别设为默认或保持默认不变；省略时默认不变 |
 | `GET /skills`, `GET /skills/{skill_id}` | Skill 元数据，不解密任何包 |
-| `POST /skills/{skill_id}` | `{"default_version": "<n>"}` 修改默认版本 |
+| `POST /skills/{skill_id}` | `{"default_version": "<n>"}` 修改默认版本。请求体错误是以该成员为 `param` 的字段错误；Skill 没有的版本返回 404，`param: "default_version"` |
 | `DELETE /skills/{skill_id}` | 删除 Skill 与所有版本 |
-| `GET /skills/{skill_id}/content` | 默认版本 ZIP |
+| `GET /skills/{skill_id}/content` | 默认版本 ZIP，`Content-Type: application/zip`，文件名 `<name>-<version>.zip` |
 | `GET /skills/{skill_id}/versions`, `GET /skills/{skill_id}/versions/{version}` | 版本元数据。列表按版本号排序，`after` 是版本 ID（`skillver_…`），不是数字 |
 | `GET /skills/{skill_id}/versions/{version}/content` | 指定版本 ZIP |
 | `DELETE /skills/{skill_id}/versions/{version}` | 见[删除版本](#delete-a-version) |
@@ -89,6 +89,8 @@ curl "$OPENAI_BASE_URL/skills" -H "Authorization: Bearer $OPENAI_API_KEY" -F fil
 - 条目为普通文件或目录，使用规范相对路径。链接、特殊文件、绝对路径、`..` 组件和重复项被拒绝。
 - 限制：压缩后 5 MiB，展开后 20 MiB，500 个文件，以及含目录在内的 1,000 个 ZIP 条目。
 
+被拒绝的上传返回 400 `invalid_request_error`，`param: "files"`，code 为 `invalid_value`，message 说明原因。缺少 `files` 部分、未知部分以及创建时携带 `default` 分别返回针对该成员的 `Missing required parameter` 与 `Unknown parameter` 错误；无效的 `default` 的 `param: "default"`。上传路由还会拒绝包含 `_` 的 `name` 以及超过 1024 个字符的 `description`。已存储的 Skill、插件 Skill 和 Environment 安装仍接受名称中的 `_` 和任何非空 `description`。
+
 Core 将每个版本的包加密并绑定到项目、Skill 与版本。ZIP 上传保留可执行位；目录上传以 mode 0644 存储文件。
 
 ### 版本与元数据 {#versions-and-metadata}

services/core/internal/api/admin_skills_routes.go

@@ -46,7 +46,7 @@ func (h *Handler) adminDeleteSkill(w http.ResponseWriter, r *http.Request) {
 // @Summary Download Skill content in a Project
 // @Description Core key only. Reuses the public resource projection and operation rules; the Project ID selects the target space and does not authenticate.
 // @Tags Skills
-// @Produce octet-stream
+// @Produce application/zip
 // @Security DeploymentAdminAuth
 // @Param skill_id path string true "Skill ID"
 // @Success 200 {file} binary
@@ -103,7 +103,7 @@ func (h *Handler) adminDeleteSkillVersion(w http.ResponseWriter, r *http.Request
 // @Summary Download immutable Skill version content in a Project
 // @Description Core key only. Reuses the public resource projection and operation rules; the Project ID selects the target space and does not authenticate.
 // @Tags Skills
-// @Produce octet-stream
+// @Produce application/zip
 // @Security DeploymentAdminAuth
 // @Param skill_id path string true "Skill ID"
 // @Param version path string true "Concrete version number"

services/core/internal/api/errors.go

@@ -61,9 +61,10 @@ func writeContentTooLarge(w http.ResponseWriter) {
 const unstorableTextMessage = "Request text contains characters this service cannot store or compare, such as U+0000 or invalid UTF-8."
 
 // fieldError is a request validation failure reported with the official
-// invalid_request_error code. An empty param serializes as null.
+// invalid_request_error code unless it names another. An empty param
+// serializes as null.
 type fieldError struct {
-	param, message string
+	param, message, code string
 }
 
 func (e *fieldError) Error() string { return e.message }
@@ -75,10 +76,14 @@ func writeFieldError(w http.ResponseWriter, err error) bool {
 	if !errors.As(err, &field) {
 		return false
 	}
+	code := field.code
+	if code == "" {
+		code = "invalid_request_error"
+	}
 	if field.param == "" {
-		writeError(w, http.StatusBadRequest, "invalid_request_error", field.message)
+		writeError(w, http.StatusBadRequest, code, field.message)
 	} else {
-		writeError(w, http.StatusBadRequest, "invalid_request_error", field.message, field.param)
+		writeError(w, http.StatusBadRequest, code, field.message, field.param)
 	}
 	return true
 }

services/core/internal/api/errors_skills.go

@@ -23,12 +23,15 @@ func writeSkillsError(w http.ResponseWriter, r *http.Request, err error) {
 		} else {
 			writeError(w, http.StatusBadRequest, "invalid_request_error", cursor.Message)
 		}
-	case errors.Is(err, skills.ErrNotFound):
-		code := "not_found_error"
+	case errors.Is(err, skills.ErrNotFound), errors.Is(err, skills.ErrVersionNotFound):
+		code, param := "not_found_error", []string(nil)
 		if skillsRoute {
 			code = ""
 		}
-		writeError(w, http.StatusNotFound, code, "Resource not found.")
+		if errors.Is(err, skills.ErrVersionNotFound) {
+			param = []string{"default_version"}
+		}
+		writeError(w, http.StatusNotFound, code, "Resource not found.", param...)
 	case errors.Is(err, skills.ErrInvalidInput):
 		writeError(w, http.StatusBadRequest, "invalid_request", invalidInputMessage)
 	case writeAuditSourceError(w, r, err) || writeTextValueError(w, r, err):

services/core/internal/api/errors_skills_test.go

@@ -34,6 +34,8 @@ func TestWriteSkillsError(t *testing.T) {
 			`{"error":{"message":"Resource not found.","type":"invalid_request_error","code":null,"param":null}}`},
 		{"project not found", project, skills.ErrNotFound, http.StatusNotFound,
 			`{"error":{"message":"Resource not found.","type":"not_found_error","code":"not_found_error","param":null}}`},
+		{"default version missing", "/v1/skills/skill_missing", skills.ErrVersionNotFound, http.StatusNotFound,
+			`{"error":{"message":"Resource not found.","type":"invalid_request_error","code":null,"param":"default_version"}}`},
 		{"invalid input", "/v1/skills", skills.ErrInvalidInput, http.StatusBadRequest,
 			`{"error":{"message":"Invalid resource identifier or request limits.","type":"invalid_request_error","code":"invalid_request","param":null}}`},
 		{"unstorable text", "/v1/skills", fmt.Errorf("create: %w", textvalue.ErrUnstorable), http.StatusBadRequest,

services/core/internal/api/session_artifacts.go

@@ -60,7 +60,7 @@ func (h *Handler) deleteSessionArtifact(w http.ResponseWriter, r *http.Request)
 }
 
 func (h *Handler) sessionArtifactContent(w http.ResponseWriter, r *http.Request) {
-	err := serveStoredContent(w, r, func(ctx context.Context, consume func(string, int64, io.Reader) error) error {
+	err := serveStoredContent(w, r, "application/octet-stream", func(ctx context.Context, consume func(string, int64, io.Reader) error) error {
 		return h.ArtifactsReader.ReadSessionArtifact(ctx, tenantID(r), chi.URLParam(r, "session_id"), chi.URLParam(r, "artifact_id"), func(a sessions.Artifact, body io.Reader) error {
 			return consume(path.Base(a.Path), a.SizeBytes, body)
 		})

services/core/internal/api/skills.go

@@ -2,6 +2,7 @@ package api
 
 import (
 	"context"
+	"encoding/json"
 	"net/http"
 	"strconv"
 
@@ -56,13 +57,23 @@ func (h *Handler) getSkill(w http.ResponseWriter, r *http.Request) {
 	writeJSON(w, http.StatusOK, skillResponseResource(value))
 }
 
+var skillUpdateShape = shape{kind: objectValue, members: []member{{"default_version", requiredString}}}
+
 func (h *Handler) updateSkill(w http.ResponseWriter, r *http.Request) {
 	body, ok := readJSONBodyLimit(w, r, 64<<10, "Request exceeds 64 KiB.")
 	if !ok {
 		return
 	}
+	// The Skills routes keep their own body rules; a well-formed object reports
+	// its field errors first.
+	if json.Valid(body) && jsonValueKind(body) == "an object" {
+		if err := checkValue("", body, skillUpdateShape); err != nil {
+			writeFieldError(w, err)
+			return
+		}
+	}
 	var input v1.SkillUpdateRequest
-	if decodeInputObject(body, &input, "default_version") != nil || input.DefaultVersion == "" {
+	if decodeInputObject(body, &input, "default_version") != nil {
 		writeSkillsError(w, r, skills.ErrInvalidInput)
 		return
 	}

services/core/internal/api/skills_field_errors_test.go

@@ -0,0 +1,154 @@
+package api
+
+import (
+	"archive/zip"
+	"bytes"
+	"context"
+	"io"
+	"mime/multipart"
+	"net/http"
+	"net/http/httptest"
+	"net/textproto"
+	"strings"
+	"testing"
+
+	"github.com/MiniMax-AI/OpenAgentCore/services/core/internal/skills"
+	"github.com/go-chi/chi/v5"
+)
+
+func skillZip(t *testing.T, name, description string) []byte {
+	t.Helper()
+	var buffer bytes.Buffer
+	writer := zip.NewWriter(&buffer)
+	file, err := writer.Create("proof/SKILL.md")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err = file.Write([]byte("---\nname: " + name + "\ndescription: " + description + "\n---\nbody")); err != nil {
+		t.Fatal(err)
+	}
+	if err = writer.Close(); err != nil {
+		t.Fatal(err)
+	}
+	return buffer.Bytes()
+}
+
+// skillServer serves the Skill routes over HTTP, which the bounded transfers need.
+func skillServer(t *testing.T, fake *fakeSkills) *httptest.Server {
+	t.Helper()
+	router := chi.NewRouter()
+	router.Use(func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			next.ServeHTTP(w, r.WithContext(context.WithValue(r.Context(), adminTenantContextKey{}, "tenant")))
+		})
+	})
+	(&Handler{Dependencies: Dependencies{Skills: fake}}).registerSkillRoutes(router)
+	server := httptest.NewServer(router)
+	t.Cleanup(server.Close)
+	return server
+}
+
+func TestSkillUploadFieldErrors(t *testing.T) {
+	type part struct {
+		field, filename string
+		body            []byte
+	}
+	good := skillZip(t, "proof", "Verify an upload.")
+	for _, test := range []struct {
+		name    string
+		version bool
+		parts   []part
+		want    string
+	}{
+		{"no files", false, nil, `"message":"Missing required parameter: 'files'.","type":"invalid_request_error","code":"invalid_request_error","param":"files"`},
+		{"default on create", false, []part{{"files", "p.zip", good}, {"default", "", []byte("true")}}, `"message":"Unknown parameter: 'default'.","type":"invalid_request_error","code":"invalid_request_error","param":"default"`},
+		{"bad default", true, []part{{"files", "p.zip", good}, {"default", "", []byte("yes")}}, `"code":"invalid_request_error","param":"default"`},
+		{"bad bundle", false, []part{{"files", "p.zip", []byte("not a zip")}}, `"type":"invalid_request_error","code":"invalid_value","param":"files"`},
+		{"underscore name", false, []part{{"files", "p.zip", skillZip(t, "my_skill", "Verify an upload.")}}, `"message":"The Skill name must not contain '_'.","type":"invalid_request_error","code":"invalid_value","param":"files"`},
+		{"long description", false, []part{{"files", "p.zip", skillZip(t, "proof", strings.Repeat("d", 1025))}}, `"message":"The Skill description must be at most 1024 characters.","type":"invalid_request_error","code":"invalid_value","param":"files"`},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			var buffer bytes.Buffer
+			writer := multipart.NewWriter(&buffer)
+			for _, entry := range test.parts {
+				header := textproto.MIMEHeader{}
+				disposition := `form-data; name="` + entry.field + `"`
+				if entry.filename != "" {
+					disposition += `; filename="` + entry.filename + `"`
+				}
+				header.Set("Content-Disposition", disposition)
+				p, err := writer.CreatePart(header)
+				if err != nil {
+					t.Fatal(err)
+				}
+				_, _ = p.Write(entry.body)
+			}
+			_ = writer.Close()
+			path := "/v1/skills"
+			if test.version {
+				path += "/skill_x/versions"
+			}
+			response, err := http.Post(skillServer(t, &fakeSkills{t: t}).URL+path, writer.FormDataContentType(), &buffer)
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer response.Body.Close()
+			body, _ := io.ReadAll(response.Body)
+			if response.StatusCode != http.StatusBadRequest || !strings.Contains(string(body), test.want) {
+				t.Fatalf("%d %s", response.StatusCode, body)
+			}
+		})
+	}
+}
+
+func TestSkillUploadAcceptsCapitalizedDefault(t *testing.T) {
+	var buffer bytes.Buffer
+	writer := multipart.NewWriter(&buffer)
+	file, _ := writer.CreateFormFile("files", "p.zip")
+	_, _ = file.Write(skillZip(t, "proof", "Verify an upload."))
+	_ = writer.WriteField("default", "True")
+	_ = writer.Close()
+	request := httptest.NewRequest(http.MethodPost, "/v1/skills/skill_x/versions", &buffer)
+	request.Header.Set("Content-Type", writer.FormDataContentType())
+	if _, makeDefault, err := readSkillUpload(request, true); err != nil || !makeDefault {
+		t.Fatal(makeDefault, err)
+	}
+}
+
+func TestSkillUpdateErrors(t *testing.T) {
+	fake := &fakeSkills{t: t, setDefaultVersion: func(context.Context, skills.SetDefaultVersion) (skills.Skill, error) {
+		return skills.Skill{}, skills.ErrVersionNotFound
+	}}
+	router := chi.NewRouter()
+	router.Post("/v1/skills/{skill_id}", (&Handler{Dependencies: Dependencies{Skills: fake}}).updateSkill)
+	for _, test := range []struct{ name, body, want string }{
+		{"unknown key", `{"default_version":"1","x":1}`, `"message":"Unknown parameter: 'x'.","type":"invalid_request_error","code":"invalid_request_error","param":"x"`},
+		{"missing", `{}`, `"message":"Missing required parameter: 'default_version'.","type":"invalid_request_error","code":"invalid_request_error","param":"default_version"`},
+		{"wrong type", `{"default_version":1}`, `"message":"Invalid type for 'default_version': expected a string, but got an integer instead.","type":"invalid_request_error","code":"invalid_request_error","param":"default_version"`},
+		{"missing version", `{"default_version":"9"}`, `"message":"Resource not found.","type":"invalid_request_error","code":null,"param":"default_version"`},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			request := httptest.NewRequest(http.MethodPost, "/v1/skills/skill_x", strings.NewReader(test.body))
+			request = request.WithContext(context.WithValue(request.Context(), adminTenantContextKey{}, "tenant"))
+			response := httptest.NewRecorder()
+			router.ServeHTTP(response, request)
+			if !strings.Contains(response.Body.String(), test.want) || (test.name == "missing version") != (response.Code == http.StatusNotFound) {
+				t.Fatalf("%d %s", response.Code, response.Body)
+			}
+		})
+	}
+}
+
+func TestSkillContentDownloadHeaders(t *testing.T) {
+	fake := &fakeSkills{t: t, readDefaultVersion: func(context.Context, skills.ReadDefaultVersion) (skills.Content, error) {
+		return skills.Content{Version: skills.Version{Name: "proof", Version: 3}, Archive: []byte("zip")}, nil
+	}}
+	response, err := http.Get(skillServer(t, fake).URL + "/v1/skills/skill_x/content")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer response.Body.Close()
+	if response.Header.Get("Content-Type") != "application/zip" || response.Header.Get("Content-Disposition") != `attachment; filename=proof-3.zip` {
+		t.Fatal(response.Header)
+	}
+}

services/core/internal/api/skills_transfer.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"fmt"
 	"io"
 	"net/http"
 	"time"
@@ -34,7 +35,7 @@ func (h *Handler) uploadSkill(w http.ResponseWriter, r *http.Request, version bo
 		var limit *http.MaxBytesError
 		if errors.As(err, &limit) {
 			writeContentTooLarge(w)
-		} else {
+		} else if !writeFieldError(w, err) {
 			writeSkillsError(w, r, skills.ErrInvalidInput)
 		}
 		return
@@ -57,7 +58,7 @@ func (h *Handler) uploadSkill(w http.ResponseWriter, r *http.Request, version bo
 }
 
 func (h *Handler) skillContent(w http.ResponseWriter, r *http.Request) {
-	err := serveStoredContent(w, r, func(ctx context.Context, consume func(string, int64, io.Reader) error) error {
+	err := serveStoredContent(w, r, "application/zip", func(ctx context.Context, consume func(string, int64, io.Reader) error) error {
 		skill := skills.PathID(chi.URLParam(r, "skill_id"))
 		var content skills.Content
 		var err error
@@ -69,7 +70,7 @@ func (h *Handler) skillContent(w http.ResponseWriter, r *http.Request) {
 		if err != nil {
 			return err
 		}
-		return consume(content.Version.Name+".zip", int64(len(content.Archive)), bytes.NewReader(content.Archive))
+		return consume(fmt.Sprintf("%s-%d.zip", content.Version.Name, content.Version.Version), int64(len(content.Archive)), bytes.NewReader(content.Archive))
 	})
 	if err != nil {
 		writeSkillsError(w, r, err)