diff --git a/docs/tools.md b/docs/tools.md index 549eaf3..ed699ee 100644 --- a/docs/tools.md +++ b/docs/tools.md @@ -145,11 +145,21 @@ Required: `project_id`, `issue_id`. POST `/projects/{project_id}/issues/{issue_id}/issue-attachments/`. -Low-level: returns `{ id, upload_data: { url, fields: {...} } }`. Use -`plane_attachment_upload` for the full init → s3 → complete flow. +Advanced low-level step for custom upload clients. Use `plane_attachment_upload` +for normal uploads. Required: `project_id`, `issue_id`, `name`. -Optional: `file_type` (mime type), `size` (number, bytes). +Optional: `file_type` (mime type), `size` (number, bytes), `raw` (boolean). + +By default, returns compact metadata such as: + +```json +{ "asset_id": "...", "asset_url": "...", "raw_upload_credentials_omitted": true } +``` + +With `raw: true`, returns the full Plane init payload, including +`upload_data.url` and temporary S3 form credential fields needed by custom +multipart clients. ### `plane_attachment_complete_upload` diff --git a/internal/mcpserver/server_test.go b/internal/mcpserver/server_test.go index e914263..4eb5e59 100644 --- a/internal/mcpserver/server_test.go +++ b/internal/mcpserver/server_test.go @@ -407,6 +407,91 @@ func TestHealthReturnsStructuredEndpointErrors(t *testing.T) { require.Contains(t, issues["error"], "403") } +func TestAttachmentInitUploadSchemaWarnsAboutRawCredentials(t *testing.T) { + srv := newTestServer(t, func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{}`)) + }) + c, err := client.NewInProcessClient(srv.MCPServer()) + require.NoError(t, err) + require.NoError(t, c.Start(context.Background())) + t.Cleanup(func() { _ = c.Close() }) + + _, err = c.Initialize(context.Background(), mcp.InitializeRequest{}) + require.NoError(t, err) + list, err := c.ListTools(context.Background(), mcp.ListToolsRequest{}) + require.NoError(t, err) + + for _, tool := range list.Tools { + if tool.Name == "plane_attachment_init_upload" { + require.Contains(t, tool.Description, "Prefer plane_attachment_upload") + require.Contains(t, tool.Description, "temporary S3 form credentials") + raw, ok := tool.InputSchema.Properties["raw"].(map[string]any) + require.True(t, ok) + require.Equal(t, "boolean", raw["type"]) + require.Contains(t, raw["description"], "temporary S3 upload form credentials") + return + } + } + require.Fail(t, "plane_attachment_init_upload should be registered") +} + +func TestAttachmentInitUploadDefaultsToCompactResult(t *testing.T) { + srv := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/api/v1/workspaces/ws/projects/P/issues/I/issue-attachments/", r.URL.Path) + _, _ = w.Write([]byte(`{ + "id": "asset-1", + "asset_url": "https://cdn.example.com/file.txt", + "name": "file.txt", + "upload_data": { + "url": "https://uploads.example.com/", + "fields": { + "key": "tmp/file.txt", + "policy": "secret-policy", + "x-amz-signature": "secret-signature" + } + } + }`)) + }) + + payload := callToolPayload(t, srv, "plane_attachment_init_upload", map[string]any{ + "project_id": "P", + "issue_id": "I", + "name": "file.txt", + }) + require.Equal(t, "asset-1", payload["asset_id"]) + require.Equal(t, "https://cdn.example.com/file.txt", payload["asset_url"]) + require.Equal(t, "file.txt", payload["name"]) + require.Equal(t, true, payload["raw_upload_credentials_omitted"]) + require.Equal(t, true, payload["upload_credentials_available_with_raw"]) + require.NotContains(t, payload, "upload_data") +} + +func TestAttachmentInitUploadRawReturnsFullPayload(t *testing.T) { + srv := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/api/v1/workspaces/ws/projects/P/issues/I/issue-attachments/", r.URL.Path) + _, _ = w.Write([]byte(`{ + "id": "asset-1", + "upload_data": { + "url": "https://uploads.example.com/", + "fields": {"policy": "secret-policy"} + } + }`)) + }) + + payload := callToolPayload(t, srv, "plane_attachment_init_upload", map[string]any{ + "project_id": "P", + "issue_id": "I", + "name": "file.txt", + "raw": true, + }) + require.Equal(t, "asset-1", payload["id"]) + uploadData, ok := payload["upload_data"].(map[string]any) + require.True(t, ok) + fields, ok := uploadData["fields"].(map[string]any) + require.True(t, ok) + require.Equal(t, "secret-policy", fields["policy"]) +} + func TestAttachmentMimeTypeStripsParameters(t *testing.T) { require.Equal(t, "text/plain", attachmentMimeType("notes.txt")) require.Equal(t, "application/octet-stream", attachmentMimeType("archive.unknownext")) diff --git a/internal/mcpserver/tools_attachments.go b/internal/mcpserver/tools_attachments.go index 532440b..b812e70 100644 --- a/internal/mcpserver/tools_attachments.go +++ b/internal/mcpserver/tools_attachments.go @@ -23,12 +23,13 @@ func (s *Server) registerAttachmentTools() { ), s.handleAttachmentList) s.mcp.AddTool(mcp.NewTool("plane_attachment_init_upload", - mcp.WithDescription("Low-level: request S3 upload credentials for a new attachment."), + mcp.WithDescription("Advanced/low-level: request attachment upload metadata. Prefer plane_attachment_upload; raw=true returns temporary S3 form credentials."), mcp.WithString("project_id", mcp.Required()), mcp.WithString("issue_id", mcp.Required()), mcp.WithString("name", mcp.Required()), mcp.WithString("file_type"), mcp.WithNumber("size"), + mcp.WithBoolean("raw", mcp.Description("Return the full Plane payload, including temporary S3 upload form credentials.")), ), s.handleAttachmentInitUpload) s.mcp.AddTool(mcp.NewTool("plane_attachment_complete_upload", @@ -90,7 +91,10 @@ func (s *Server) handleAttachmentInitUpload(ctx context.Context, req mcp.CallToo if err != nil { return toolError(err), nil } - return asTextResult(out) + if raw := argBoolPtr(args, "raw"); raw != nil && *raw { + return asTextResult(out) + } + return asTextResult(compactAttachmentInitUpload(out)) } func (s *Server) handleAttachmentCompleteUpload(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -270,6 +274,89 @@ func parseInitPayload(init map[string]any) (assetID, uploadURL string, fields ma return assetID, uploadURL, fields, nil } +func compactAttachmentInitUpload(init map[string]any) map[string]any { + out := map[string]any{ + "raw_upload_credentials_omitted": true, + } + if assetID := initUploadAssetID(init); assetID != "" { + out["asset_id"] = assetID + } + if assetURL := initUploadAssetURL(init); assetURL != "" { + out["asset_url"] = assetURL + } + if name := initUploadName(init); name != "" { + out["name"] = name + } + if _, ok := init["upload_data"]; ok { + out["upload_credentials_available_with_raw"] = true + } + return out +} + +func initUploadAssetID(init map[string]any) string { + for _, key := range []string{"id", "asset_id"} { + if value, ok := init[key].(string); ok && value != "" { + return value + } + } + if attachment, ok := init["attachment"].(map[string]any); ok { + for _, key := range []string{"id", "asset_id"} { + if value, ok := attachment[key].(string); ok && value != "" { + return value + } + } + } + return "" +} + +func initUploadAssetURL(init map[string]any) string { + for _, key := range []string{"asset_url", "asset"} { + if value, ok := init[key].(string); ok && value != "" { + return value + } + } + if attachment, ok := init["attachment"].(map[string]any); ok { + for _, key := range []string{"asset_url", "asset"} { + if value, ok := attachment[key].(string); ok && value != "" { + return value + } + } + if attrs, ok := attachment["attributes"].(map[string]any); ok { + if value, ok := attrs["url"].(string); ok && value != "" { + return value + } + } + } + if attrs, ok := init["attributes"].(map[string]any); ok { + if value, ok := attrs["url"].(string); ok && value != "" { + return value + } + } + return "" +} + +func initUploadName(init map[string]any) string { + if value, ok := init["name"].(string); ok && value != "" { + return value + } + if attachment, ok := init["attachment"].(map[string]any); ok { + if value, ok := attachment["name"].(string); ok && value != "" { + return value + } + if attrs, ok := attachment["attributes"].(map[string]any); ok { + if value, ok := attrs["name"].(string); ok && value != "" { + return value + } + } + } + if attrs, ok := init["attributes"].(map[string]any); ok { + if value, ok := attrs["name"].(string); ok && value != "" { + return value + } + } + return "" +} + func attachmentURL(att plane.Attachment, fallback string) string { if attrs, ok := att["attributes"].(map[string]any); ok { if u, ok := attrs["url"].(string); ok && isDownloadableAttachmentURL(u) {