Classify provider refusals once for every chat action

agent/cmd/chat.go

@@ -99,15 +99,37 @@ func registerChat(api *callable.Server, s *chatRoutes) {
 			default:
 				return nil, invalid("invalid provider")
 			}
-			// A provider rate limit is the user acting too fast, not a server error.
-			var status *clients.HTTPError
-			if errors.As(err, &status) && status.Status == http.StatusTooManyRequests {
-				return nil, &callable.Error{Code: "resource-exhausted", Message: "too many requests, try again shortly"}
+			if rejection := providerRejection(r.Provider, name, err); rejection != nil {
+				return nil, rejection
 			}
 			return result, err
 		})
 	}
 }
+
+// providerRejection classifies a provider's HTTP error that is about the user's
+// request rather than the agent. Every chat action and provider shares it, so a
+// new refusal wording never surfaces as an internal error. The request shapes
+// these actions send are pinned by tests, so an explained rejection is a refusal.
+func providerRejection(p, action string, err error) error {
+	var status *clients.HTTPError
+	if !errors.As(err, &status) {
+		return nil
+	}
+	switch status.Status {
+	case http.StatusTooManyRequests:
+		// A provider rate limit is the user acting too fast, not a server error.
+		return &callable.Error{Code: "resource-exhausted", Message: "too many requests, try again shortly"}
+	case http.StatusBadRequest, http.StatusNotFound, http.StatusConflict, http.StatusUnprocessableEntity:
+		if status.Explained {
+			return &callable.Error{Code: "failed-precondition", Message: providerNames[p] + " rejected this " + action + " request."}
+		}
+	}
+	return nil
+}
+
+var providerNames = map[string]string{"twitch": "Twitch", "kick": "Kick", "youtube": "YouTube"}
+
 func (r chatRequest) validate(action string) error {
 	if r.Provider == "" {
 		return invalid("missing provider")

agent/cmd/chat_moderation_test.go

@@ -22,20 +22,18 @@ func TestTwitchModerationRejectionReporting(t *testing.T) {
 		{"protected user", "ban", "user may not be banned", 400, 400, "FAILED_PRECONDITION", "Twitch does not allow banning this user."},
 		{"protected timeout", "timeout", "user may not be put in a timeout", 400, 400, "FAILED_PRECONDITION", "Twitch does not allow timing out this user."},
 		{"already unbanned", "unban", "user is not banned", 400, 400, "FAILED_PRECONDITION", "This user is not banned."},
+		{"unban refusal naming its parameter", "unban", "The user specified in the user_id query parameter is not banned or timed out.", 400, 400, "FAILED_PRECONDITION", "Twitch rejected this unban request."},
 		{"unrecognized unban rejection", "unban", "private-provider-detail", 400, 400, "FAILED_PRECONDITION", "Twitch rejected this unban request."},
 		{"empty unban rejection", "unban", "", 400, 500, "INTERNAL", "INTERNAL"},
 		{"blank unban rejection", "unban", "  ", 400, 500, "INTERNAL", "INTERNAL"},
 		{"unban server failure", "unban", "user is not banned", 503, 500, "INTERNAL", "INTERNAL"},
 		{"unban forbidden", "unban", "private-provider-detail", 403, 500, "INTERNAL", "INTERNAL"},
 		{"documented wording", "ban", "The user specified in the user_id field is already banned.", 400, 400, "FAILED_PRECONDITION", "This user is already banned."},
-		{"missing provider parameter", "ban", "user_id is required", 400, 500, "INTERNAL", "INTERNAL"},
 		{"unrecognized ban rejection", "ban", "private-provider-detail", 400, 400, "FAILED_PRECONDITION", "Twitch rejected this ban request."},
 		{"unrecognized timeout rejection", "timeout", "private-provider-detail", 400, 400, "FAILED_PRECONDITION", "Twitch rejected this timeout request."},
 		{"not a matching reason", "ban", "user is already banned private-provider-detail", 400, 400, "FAILED_PRECONDITION", "Twitch rejected this ban request."},
 		{"empty ban rejection", "ban", "", 400, 500, "INTERNAL", "INTERNAL"},
-		{"missing query parameter", "ban", `Missing required parameter "moderator_id"`, 400, 500, "INTERNAL", "INTERNAL"},
-		{"malformed parameter", "timeout", "The value in the duration field is not valid.", 400, 500, "INTERNAL", "INTERNAL"},
-		{"malformed unban", "unban", "The user_id query parameter is required.", 400, 500, "INTERNAL", "INTERNAL"},
+		{"ban refusal naming its field", "ban", "The user_id field may not be banned while required by policy.", 400, 400, "FAILED_PRECONDITION", "Twitch rejected this ban request."},
 		{"forbidden", "ban", "private-provider-detail", 403, 500, "INTERNAL", "INTERNAL"},
 		{"server failure with known text", "ban", "user is already banned", 503, 500, "INTERNAL", "INTERNAL"},
 		{"rate limited", "ban", "private-provider-detail", 429, 429, "RESOURCE_EXHAUSTED", "too many requests, try again shortly"},
@@ -145,3 +143,48 @@ func TestTwitchTimeoutBoundsCheckedLocally(t *testing.T) {
 		})
 	}
 }
+
+// Every chat action shares one rejection policy, so a refusal Twitch or Kick
+// explains is a user-facing rejection on any endpoint, while an unexplained
+// rejection or a server failure still alerts.
+func TestProviderRejectionsAcrossEndpoints(t *testing.T) {
+	for _, tc := range []struct {
+		name, action, provider string
+		status                 int
+		body                   string
+		wantStatus             int
+		wantMessage            string
+	}{
+		{"twitch delete refusal", "deleteMessage", "twitch", 400, `{"message":"private-provider-detail"}`, 400, "Twitch rejected this deleteMessage request."},
+		{"twitch delete missing message", "deleteMessage", "twitch", 404, `{"message":"private-provider-detail"}`, 400, "Twitch rejected this deleteMessage request."},
+		{"twitch clear refusal", "clear", "twitch", 400, `{"message":"private-provider-detail"}`, 400, "Twitch rejected this clear request."},
+		{"kick ban refusal", "ban", "kick", 400, `{"message":"private-provider-detail"}`, 400, "Kick rejected this ban request."},
+		{"kick unban refusal", "unban", "kick", 422, `{"message":"private-provider-detail"}`, 400, "Kick rejected this unban request."},
+		{"kick delete conflict", "deleteMessage", "kick", 409, `{"message":"private-provider-detail"}`, 400, "Kick rejected this deleteMessage request."},
+		{"unexplained rejection", "deleteMessage", "twitch", 400, `{"message":" "}`, 500, "INTERNAL"},
+		{"non-JSON rejection", "ban", "kick", 400, `private-provider-detail`, 500, "INTERNAL"},
+		{"non-string message", "clear", "twitch", 400, `{"message":42}`, 500, "INTERNAL"},
+		{"explained server failure", "deleteMessage", "twitch", 502, `{"message":"private-provider-detail"}`, 500, "INTERNAL"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			core, logs := observer.New(zap.InfoLevel)
+			defer zap.ReplaceGlobals(zap.New(observability.Wrap(core)))()
+			s, _ := newMock(func(r *http.Request) (*http.Response, error) {
+				return respond(tc.status, tc.body), nil
+			})
+			code, body := call(t, s, tc.action, map[string]any{"provider": tc.provider, "channelId": "123", "userId": "456", "messageId": tc.provider + ":message"}, true)
+			if code != tc.wantStatus || object(body["error"])["message"] != tc.wantMessage {
+				t.Fatal(code, body)
+			}
+			entries := logs.FilterMessage("http request").All()
+			if len(entries) != 1 || (tc.wantStatus < 500) != (entries[0].Level == zap.InfoLevel) {
+				t.Fatalf("rejection alerting changed: %v", entries)
+			}
+			logged, _ := json.Marshal(entries[0].ContextMap())
+			encoded, _ := json.Marshal(body)
+			if strings.Contains(string(logged)+string(encoded), "private-provider-detail") {
+				t.Fatal("provider details leaked")
+			}
+		})
+	}
+}

agent/internal/clients/client.go

@@ -26,17 +26,32 @@ type Response struct {
 }
 
 // HTTPError deliberately excludes response bodies, which may contain credentials.
-type HTTPError struct{ Status int }
+// Explained records only whether the provider gave a reason for the rejection.
+type HTTPError struct {
+	Status    int
+	Explained bool
+}
 
 func (e *HTTPError) Error() string { return fmt.Sprintf("provider returned HTTP %d", e.Status) }
 
 func (r *Response) Err() error {
 	if r.StatusCode < 200 || r.StatusCode >= 300 {
-		return &HTTPError{Status: r.StatusCode}
+		return &HTTPError{Status: r.StatusCode, Explained: explained(r.Body)}
 	}
 	return nil
 }
 
+// explained reports whether a JSON error body carries a non-blank "message",
+// the field Twitch and Kick use to say why they rejected a request.
+func explained(body []byte) bool {
+	var result struct{ Message any }
+	if json.Unmarshal(body, &result) != nil {
+		return false
+	}
+	message, _ := result.Message.(string)
+	return strings.TrimSpace(message) != ""
+}
+
 func (r *Response) Decode(out any) error {
 	if out != nil && json.Unmarshal(r.Body, out) != nil {
 		return errors.New("invalid provider response")

agent/internal/clients/twitch/chat.go

@@ -3,7 +3,6 @@ package twitch
 import (
 	"context"
 	"net/url"
-	"regexp"
 	"strings"
 	"time"
 )
@@ -68,28 +67,20 @@ func (c *Client) Ban(ctx context.Context, channel, moderator, user string, durat
 	if duration != 0 {
 		data["duration"], data["reason"] = duration, reason
 	}
-	action := "ban"
-	if duration != 0 {
-		action = "timeout"
-	}
-	return c.writeBan(ctx, action, "POST", moderationPath("/moderation/bans", channel, moderator, nil), map[string]any{"data": data})
+	return c.writeBan(ctx, "POST", moderationPath("/moderation/bans", channel, moderator, nil), map[string]any{"data": data})
 }
 
 func (c *Client) Unban(ctx context.Context, channel, moderator, user string) error {
-	return c.writeBan(ctx, "unban", "DELETE", moderationPath("/moderation/bans", channel, moderator, url.Values{"user_id": {user}}), nil)
+	return c.writeBan(ctx, "DELETE", moderationPath("/moderation/bans", channel, moderator, url.Values{"user_id": {user}}), nil)
 }
 
-// ModerationRejected is a refusal of a moderation action by Twitch.
-// Other provider failures retain their redacted HTTP error for reporting.
+// ModerationRejected is a recognized refusal of a moderation action by Twitch.
+// Other refusals keep their redacted HTTP error and are classified by the caller.
 type ModerationRejected struct{ Message string }
 
 func (e *ModerationRejected) Error() string { return e.Message }
 
-// Helix describes a malformed request in terms of its parameters, while a
-// refusal describes the target user. Malformed requests are agent bugs.
-var malformedRequest = regexp.MustCompile(`parameter|required|malformed|valid`)
-
-func (c *Client) writeBan(ctx context.Context, action, method, path string, body any) error {
+func (c *Client) writeBan(ctx context.Context, method, path string, body any) error {
 	response, err := c.Do(ctx, method, path, body)
 	if err != nil {
 		return err
@@ -108,13 +99,6 @@ func (c *Client) writeBan(ctx context.Context, action, method, path string, body
 			case "user may not be put in a timeout", "the user specified in the user_id field may not be put in a timeout":
 				return &ModerationRejected{Message: "Twitch does not allow timing out this user."}
 			}
-			// The request carries only resolved IDs and locally validated timeout
-			// fields, so any other rejection is Twitch refusing the action, even
-			// when it changes its wording. Never expose raw text.
-			message := strings.ToLower(strings.TrimSpace(result.Message))
-			if message != "" && !malformedRequest.MatchString(message) {
-				return &ModerationRejected{Message: "Twitch rejected this " + action + " request."}
-			}
 		}
 	}
 	return response.Err()