fix(github): address review findings from round 2880/2883

Sonnet MINOR #1: Stop timer after <-timer.C fires for idiomatic cleanup.
Sonnet MINOR #2: Document that empty array from contents API is valid (empty dir).
Sonnet MINOR #3: Document that GetPullRequestFiles returns nil for no files.
Sonnet NIT #4: Strengthen SetHTTPClient/SetRetryBackoff docs to clarify test-only intent.
Sonnet NIT #5: Document GetCommitStatuses fail-fast behavior.
Sonnet NIT #6: Document double-slash collapsing in escapePath.
Security MINOR #1: Document redirect policy responsibility when providing custom client.
Security MINOR #2: Reduce maxErrorBodyBytes from 64KB to 4KB to limit sensitive data exposure.

github/client.go

@@ -27,9 +27,10 @@ const (
 // It carries the status code so callers can distinguish between
 // different failure modes (e.g. 404 vs 500).
 //
-// Note: Error() includes up to 200 bytes of the response body for debugging.
-// Callers should avoid logging raw error messages in production if the upstream
-// server may return sensitive details in error responses.
+// The Body field stores up to 4 KiB of the raw response for programmatic
+// inspection. Error() truncates to 200 bytes for safe logging, but callers
+// should avoid logging or propagating Body directly in production since it may
+// contain sensitive details from the upstream server.
 type APIError struct {
 	StatusCode int
 	Body       string
@@ -87,8 +88,9 @@ func AllowInsecureHTTP() ClientOption {
 }
 
 // Client interacts with the GitHub API.
-// A Client is safe for concurrent use by multiple goroutines;
-// however, SetHTTPClient and SetRetryBackoff must not be called concurrently with requests.
+// A Client is safe for concurrent use by multiple goroutines.
+// SetHTTPClient and SetRetryBackoff are intended for test setup only and must
+// be called before any goroutines issue requests; they have no synchronization.
 type Client struct {
 	baseURL           string
 	token             string
@@ -141,9 +143,15 @@ func NewClient(token, baseURL string, opts ...ClientOption) *Client {
 }
 
 // SetHTTPClient sets the underlying HTTP client used for requests.
-// This is intended for testing to inject mock transports.
+// This is intended for test setup only to inject mock transports; it must be
+// called before any goroutines issue requests.
+//
 // Passing nil restores the default client (30s timeout + auth-stripping
 // CheckRedirect policy matching NewClient).
+//
+// Callers providing a non-nil client are responsible for configuring a safe
+// CheckRedirect policy. Without one, the default net/http behavior will follow
+// redirects and may forward the Authorization header to untrusted hosts.
 func (c *Client) SetHTTPClient(hc *http.Client) {
 	if hc == nil {
 		hc = &http.Client{
@@ -155,6 +163,7 @@ func (c *Client) SetHTTPClient(hc *http.Client) {
 }
 
 // SetRetryBackoff configures the retry backoff durations for testing.
+// It must be called before any goroutines issue requests.
 // In production the default {1s, 2s} applies.
 func (c *Client) SetRetryBackoff(d []time.Duration) {
 	c.retryBackoff = d
@@ -175,7 +184,10 @@ func (c *Client) doRequest(ctx context.Context, method, reqURL string, accept st
 		backoff = []time.Duration{1 * time.Second, 2 * time.Second}
 	}
 
-	const maxErrorBodyBytes = 64 * 1024
+	// maxErrorBodyBytes limits how much of an error response body is stored.
+	// Kept small (4 KiB) to reduce the risk of sensitive data leakage if callers
+	// log APIError.Body directly. Error() further truncates to 200 bytes.
+	const maxErrorBodyBytes = 4 * 1024
 
 	// Reject non-HTTPS URLs early since the URL is immutable across retries.
 	if c.token != "" && !c.allowInsecureHTTP {
@@ -199,7 +211,7 @@ func (c *Client) doRequest(ctx context.Context, method, reqURL string, accept st
 				timer := time.NewTimer(delay)
 				select {
 				case <-timer.C:
-					// Backoff elapsed, proceed with retry.
+					timer.Stop() // no-op after fire, releases runtime resources promptly
 				case <-ctx.Done():
 					timer.Stop()
 					return nil, ctx.Err()