Address Copilot review feedback on PR #4

README.md

@@ -228,9 +228,11 @@ components := []discord.Component{
 
 ```go
 // Parse the decimal string Discord sends for members and roles.
+// An empty member.Permissions returns (0, nil) - only a non-numeric
+// or out-of-range string produces a non-nil error.
 perms, err := discord.ParsePermission(member.Permissions)
 if err != nil {
-    // member.Permissions was empty or malformed
+    // member.Permissions was malformed (not a base-10 uint64).
 }
 
 // Check whether a member has both KickMembers and BanMembers.

example/basic/main.go

@@ -122,7 +122,14 @@ func main() {
 				return
 			}
 
-			perms, _ := discord.ParsePermission(member.Permissions)
+			// member.Permissions can be empty when Discord doesn't include a
+			// resolved permission bitfield (e.g. /guilds/{id}/members), so
+			// distinguish the empty case (no error, perms == 0) from the
+			// malformed case (error, fall back to "unknown").
+			adminValue := "unknown"
+			if perms, perr := discord.ParsePermission(member.Permissions); perr == nil {
+				adminValue = fmt.Sprintf("%v", perms.IsAdmin())
+			}
 
 			embed := discord.Embed{
 				Title: member.User.Tag(),
@@ -132,7 +139,7 @@ func main() {
 					{Name: "Joined", Value: member.JoinedAt, Inline: true},
 					{Name: "Roles", Value: fmt.Sprintf("%d roles", len(member.Roles)), Inline: true},
 					{Name: "Bot", Value: fmt.Sprintf("%v", member.User.Bot), Inline: true},
-					{Name: "Admin", Value: fmt.Sprintf("%v", perms.IsAdmin()), Inline: true},
+					{Name: "Admin", Value: adminValue, Inline: true},
 				},
 				Footer: &discord.EmbedFooter{Text: "GoDiscord basic example"},
 			}

example/slash/main.go

@@ -130,10 +130,16 @@ func handleCommand(b *discord.Bot, i *discord.Interaction) {
 		if max < 1 {
 			max = 1
 		}
-		// Derive a deterministic demo result from the interaction ID's last digit.
-		// A real implementation should use math/rand or crypto/rand.
-		lastDigit := i.ID[len(i.ID)-1]
-		result := int64(lastDigit)%max + 1
+		// Derive a deterministic demo result from the interaction ID's last
+		// decimal digit. We subtract '0' so we get 0-9 (the integer value)
+		// instead of the byte's ASCII code (e.g. '7' == 55), which would
+		// produce a skewed and unintuitive modulus result. A real
+		// implementation should use math/rand or crypto/rand.
+		var lastDigit int64
+		if last := i.ID[len(i.ID)-1]; last >= '0' && last <= '9' {
+			lastDigit = int64(last - '0')
+		}
+		result := lastDigit%max + 1
 		b.Rest.CreateInteractionResponse(i.ID, i.Token, discord.InteractionResponse{
 			Type: discord.InteractionCallbackTypeChannelMessage,
 			Data: &discord.InteractionResponseData{

permissions.go

@@ -8,8 +8,12 @@ package discord
 //
 // Quick usage:
 //
-//	// Check whether a member can send messages and embed links.
-//	perms := discord.Permission(member.Permissions)
+//	// Discord delivers member permissions as a base-10 string. Use
+//	// ParsePermission to convert it to a Permission bitfield safely.
+//	perms, err := discord.ParsePermission(member.Permissions)
+//	if err != nil {
+//	    // member.Permissions was malformed (not a base-10 uint64).
+//	}
 //	if perms.Has(discord.PermSendMessages, discord.PermEmbedLinks) {
 //	    // ...
 //	}

rest_test.go

@@ -1,16 +1,58 @@
 package discord
 
 import (
+	"encoding/json"
+	"io"
+	"net/http"
+	"strings"
 	"testing"
 )
 
 // ---------------------------------------------------------------------------
-// Input validation helpers (BanMember days clamping, BulkDelete count check)
+// Test helpers - custom RoundTripper that records the request and returns a
+// canned response, so we can drive the real RestClient methods end-to-end
+// without touching discord.com.
+// ---------------------------------------------------------------------------
+
+type recordingTransport struct {
+	lastReq  *http.Request
+	lastBody []byte
+	respBody string
+	respCode int
+}
+
+func (t *recordingTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	t.lastReq = req
+	if req.Body != nil {
+		t.lastBody, _ = io.ReadAll(req.Body)
+		_ = req.Body.Close()
+	}
+	code := t.respCode
+	if code == 0 {
+		code = http.StatusOK
+	}
+	body := t.respBody
+	if body == "" {
+		body = "[]"
+	}
+	return &http.Response{
+		StatusCode: code,
+		Body:       io.NopCloser(strings.NewReader(body)),
+		Header:     make(http.Header),
+		Request:    req,
+	}, nil
+}
+
+func newTestRest(rt *recordingTransport) *RestClient {
+	return &RestClient{token: "test", client: &http.Client{Transport: rt}}
+}
+
+// ---------------------------------------------------------------------------
+// Input validation - BulkDeleteMessages
 // ---------------------------------------------------------------------------
 
 // TestBulkDeleteMessages_Validation verifies the in-process guard before any
-// HTTP call is made. We call the method with a nil RestClient to confirm the
-// validation returns an error rather than panicking.
+// HTTP call is made.
 func TestBulkDeleteMessages_Validation(t *testing.T) {
 	r := &RestClient{token: "x", client: nil}
 
@@ -24,66 +66,100 @@ func TestBulkDeleteMessages_Validation(t *testing.T) {
 		t.Error("BulkDeleteMessages with 0 IDs should return an error")
 	}
 
-	// Exactly at maximum — should NOT fail locally (would fail at HTTP layer,
-	// but we cannot test that without a live server).
+	// 101 IDs is above the documented Discord maximum (100), so the local
+	// validator must reject it before any HTTP call is made.
 	ids := make([]string, 101)
 	if err := r.BulkDeleteMessages("chan", ids); err == nil {
 		t.Error("BulkDeleteMessages with 101 IDs should return an error")
 	}
-}
-
-// TestGetMessages_LimitClamping uses a nil HTTP client so the function panics
-// only when it tries to actually perform the HTTP call, not during validation.
-// We test only that the clamping math is correct by inspecting the formatted path.
-func TestGetMessages_LimitClamping(t *testing.T) {
-	// Verify that a limit of 0 is clamped to 1 and a limit of 200 is clamped
-	// to 100. We do this by testing the internal clamping logic directly
-	// (since we can't make a real HTTP call in unit tests).
-
-	clamp := func(limit int) int {
-		if limit < 1 {
-			limit = 1
-		}
-		if limit > 100 {
-			limit = 100
-		}
-		return limit
-	}
 
-	if got := clamp(0); got != 1 {
-		t.Errorf("clamp(0) = %d, want 1", got)
+	// Exactly 100 IDs is within the documented Discord maximum. The local
+	// validator must not reject it; the actual request goes through the
+	// recording transport so we don't touch discord.com.
+	rt := &recordingTransport{respCode: http.StatusNoContent}
+	rOK := newTestRest(rt)
+	hundred := make([]string, 100)
+	for i := range hundred {
+		hundred[i] = "id"
 	}
-	if got := clamp(-5); got != 1 {
-		t.Errorf("clamp(-5) = %d, want 1", got)
+	if err := rOK.BulkDeleteMessages("chan", hundred); err != nil {
+		t.Errorf("BulkDeleteMessages with 100 IDs returned err: %v", err)
 	}
-	if got := clamp(200); got != 100 {
-		t.Errorf("clamp(200) = %d, want 100", got)
+	if rt.lastReq == nil {
+		t.Fatal("expected an HTTP request for 100 IDs")
 	}
-	if got := clamp(50); got != 50 {
-		t.Errorf("clamp(50) = %d, want 50", got)
+	if !strings.HasSuffix(rt.lastReq.URL.Path, "/channels/chan/messages/bulk-delete") {
+		t.Errorf("unexpected request path: %s", rt.lastReq.URL.Path)
 	}
 }
 
-// TestBanMember_DaysClamping verifies the deleteMessageDays clamping logic.
-func TestBanMember_DaysClamping(t *testing.T) {
-	// Same approach: test the clamping logic in isolation.
-	clamp := func(days int) int {
-		if days < 0 {
-			days = 0
+// ---------------------------------------------------------------------------
+// GetMessages clamping - exercises the real method, not a re-implementation
+// ---------------------------------------------------------------------------
+
+func TestGetMessages_LimitClamping(t *testing.T) {
+	cases := []struct {
+		input int
+		want  string // expected ?limit= value
+	}{
+		{0, "limit=1"},
+		{-5, "limit=1"},
+		{50, "limit=50"},
+		{100, "limit=100"},
+		{200, "limit=100"},
+	}
+	for _, tc := range cases {
+		rt := &recordingTransport{respBody: "[]"}
+		r := newTestRest(rt)
+		if _, err := r.GetMessages("chan", tc.input); err != nil {
+			t.Errorf("GetMessages(%d) error: %v", tc.input, err)
+			continue
 		}
-		if days > 7 {
-			days = 7
+		if rt.lastReq == nil {
+			t.Errorf("GetMessages(%d): no request observed", tc.input)
+			continue
+		}
+		query := rt.lastReq.URL.RawQuery
+		if query != tc.want {
+			t.Errorf("GetMessages(%d) sent query %q, want %q", tc.input, query, tc.want)
 		}
-		return days
 	}
+}
 
-	if got := clamp(-1); got != 0 {
-		t.Errorf("clamp(-1) = %d, want 0", got)
-	}
-	if got := clamp(8); got != 7 {
-		t.Errorf("clamp(8) = %d, want 7", got)
+// ---------------------------------------------------------------------------
+// BanMember clamping - exercises the real method via the recorded body
+// ---------------------------------------------------------------------------
+
+func TestBanMember_DaysClamping(t *testing.T) {
+	cases := []struct {
+		input int
+		want  int
+	}{
+		{-1, 0},
+		{0, 0},
+		{3, 3},
+		{7, 7},
+		{8, 7},
+		{99, 7},
 	}
-	if got := clamp(3); got != 3 {
-		t.Errorf("clamp(3) = %d, want 3", got)
+	for _, tc := range cases {
+		rt := &recordingTransport{respCode: http.StatusNoContent}
+		r := newTestRest(rt)
+		if err := r.BanMember("guild", "user", tc.input); err != nil {
+			t.Errorf("BanMember(%d) error: %v", tc.input, err)
+			continue
+		}
+		if len(rt.lastBody) == 0 {
+			t.Errorf("BanMember(%d): no body observed", tc.input)
+			continue
+		}
+		var payload map[string]int
+		if err := json.Unmarshal(rt.lastBody, &payload); err != nil {
+			t.Errorf("BanMember(%d): body unmarshal: %v (body=%q)", tc.input, err, string(rt.lastBody))
+			continue
+		}
+		if payload["delete_message_days"] != tc.want {
+			t.Errorf("BanMember(%d) sent delete_message_days=%d, want %d", tc.input, payload["delete_message_days"], tc.want)
+		}
 	}
 }

websocket_test.go

@@ -45,13 +45,6 @@ func buildFakeFrameHeader(length uint64) []byte {
 	return hdr
 }
 
-// TestMaxFramePayload_Constant ensures the constant is in fact 64 MiB.
-func TestMaxFramePayload_Value(t *testing.T) {
-	if maxFramePayload != 64*1024*1024 {
-		t.Errorf("maxFramePayload should be 64 MiB, got %d", maxFramePayload)
-	}
-}
-
 // TestFrameHeaderBuilding verifies that our helper builds headers correctly,
 // which indirectly tests our understanding of the WebSocket frame format used
 // in readFrame.