fix(github): address sonnet review feedback on PR #113
PR Ready Gate / clear-labels (pull_request) Successful in 2s
CI / test (pull_request) Successful in 18s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 31s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m45s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 2m27s
PR Ready Gate / clear-labels (pull_request) Successful in 2s
CI / test (pull_request) Successful in 18s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 31s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m45s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 2m27s
- hasHTTPSScheme: use strings.EqualFold instead of ToLower+HasPrefix - slog.Warn: move hint into structured attribute for idiomatic usage - client_test.go: fix blank line formatting between test functions - clientConfig.testBypass: strengthen comment to reference export_test.go
This commit is contained in:
+3
-3
@@ -92,7 +92,7 @@ func asAPIError(err error) (*APIError, bool) {
|
||||
// clientConfig holds optional configuration for NewClient.
|
||||
type clientConfig struct {
|
||||
allowInsecureHTTP bool
|
||||
testBypass bool // skip env gate (for tests only)
|
||||
testBypass bool // skip env gate; only WithAllowInsecureHTTPForTest (export_test.go) should set this
|
||||
}
|
||||
|
||||
// ClientOption configures optional behavior of NewClient.
|
||||
@@ -187,7 +187,7 @@ func NewClient(token, baseURL string, opts ...ClientOption) *Client {
|
||||
slog.Warn("WithAllowInsecureHTTP enabled — credentials may be sent over plaintext",
|
||||
"env", envAllowInsecure+"=1")
|
||||
} else {
|
||||
slog.Warn("WithAllowInsecureHTTP option ignored: set "+envAllowInsecure+"=1 to enable")
|
||||
slog.Warn("WithAllowInsecureHTTP option ignored", "hint", "set "+envAllowInsecure+"=1 to enable")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -269,7 +269,7 @@ func (c *Client) parseRetryAfter(value string) (time.Duration, bool) {
|
||||
// It avoids the allocation of url.Parse for a simple scheme check.
|
||||
func hasHTTPSScheme(rawURL string) bool {
|
||||
const prefix = "https://"
|
||||
return len(rawURL) >= len(prefix) && strings.HasPrefix(strings.ToLower(rawURL[:len(prefix)]), prefix)
|
||||
return len(rawURL) >= len(prefix) && strings.EqualFold(rawURL[:len(prefix)], prefix)
|
||||
}
|
||||
|
||||
// doRequest performs an HTTP request with retry on 429 rate limit responses.
|
||||
|
||||
@@ -530,7 +530,6 @@ func TestDoRequest_RejectsHTTPWithToken(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
func TestDoRequest_RejectsHTTPWithToken_RedactsQueryParams(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(200)
|
||||
@@ -551,6 +550,7 @@ func TestDoRequest_RejectsHTTPWithToken_RedactsQueryParams(t *testing.T) {
|
||||
t.Errorf("error message should contain redacted marker, got: %v", errMsg)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDoRequest_AllowsHTTPWithoutToken(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(200)
|
||||
|
||||
Reference in New Issue
Block a user