Address code review: fix transport error handling, header casing, import grouping

Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/29c289a0-52c0-4c27-98f3-8eef3b574559

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>

Author: Copilot

internal/config/rate_limit_config_test.go

@@ -1,9 +1,10 @@
 package config
 
 import (
+	"testing"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"testing"
 )
 
 func TestServerConfig_RateLimitFields(t *testing.T) {

internal/proxy/handler.go

@@ -422,13 +422,15 @@ func copyResponseHeaders(w http.ResponseWriter, resp *http.Response) {
 }
 
 // injectRetryAfterIfRateLimited inspects the upstream response for rate-limit signals
-// (HTTP 429 or X-RateLimit-Remaining == 0). When detected it:
+// (HTTP 429 or X-Ratelimit-Remaining == 0). When detected it:
 //  1. Injects a Retry-After header so the client knows when to retry.
 //  2. Logs the event at ERROR level so operators can monitor rate-limit incidents.
 func injectRetryAfterIfRateLimited(w http.ResponseWriter, resp *http.Response) {
 	is429 := resp.StatusCode == http.StatusTooManyRequests
-	remaining := resp.Header.Get("X-RateLimit-Remaining")
-	resetHeader := resp.Header.Get("X-RateLimit-Reset")
+	// Use Go's canonical header key form (textproto.CanonicalMIMEHeaderKey produces
+	// "X-Ratelimit-Remaining", matching GitHub's actual response headers).
+	remaining := resp.Header.Get("X-Ratelimit-Remaining")
+	resetHeader := resp.Header.Get("X-Ratelimit-Reset")
 
 	isRateLimited := is429 || remaining == "0"
 	if !isRateLimited {
@@ -441,7 +443,7 @@ func injectRetryAfterIfRateLimited(w http.ResponseWriter, resp *http.Response) {
 	w.Header().Set("Retry-After", strconv.Itoa(retryAfter))
 
 	logger.LogError("client",
-		"upstream rate limit hit: status=%d X-RateLimit-Remaining=%s X-RateLimit-Reset=%s retry-after=%ds",
+		"upstream rate limit hit: status=%d X-Ratelimit-Remaining=%s X-Ratelimit-Reset=%s retry-after=%ds",
 		resp.StatusCode, remaining, resetHeader, retryAfter)
 }
 

internal/proxy/rate_limit_test.go

@@ -33,7 +33,7 @@ func TestInjectRetryAfterIfRateLimited(t *testing.T) {
 		assert.Greater(t, secs, 0, "Retry-After should be positive")
 	})
 
-	t.Run("X-RateLimit-Remaining 0 injects Retry-After", func(t *testing.T) {
+	t.Run("X-Ratelimit-Remaining 0 injects Retry-After", func(t *testing.T) {
 		t.Parallel()
 		w := httptest.NewRecorder()
 		future := time.Now().Add(60 * time.Second)

internal/server/unified.go

@@ -574,7 +574,9 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName
 
 	backendResult, err := executeBackendToolCall(execCtx, us.launcher, serverID, sessionID, toolName, args)
 	if err != nil {
-		cb.RecordSuccess() // transport error ≠ rate limit; reset consecutive counter
+		// Transport errors (connection failure, JSON parse error, etc.) are not rate-limit
+		// events and must not affect the consecutive rate-limit counter. Leave the circuit
+		// breaker state unchanged so genuine rate-limit history is preserved.
 		execSpan.RecordError(err)
 		execSpan.SetStatus(codes.Error, err.Error())
 		httpStatusCode = 500