Use DefaultTransport for http.Client by mattn · Pull Request #422 · sipeed/picoclaw

mattn · 2026-02-18T14:34:54Z

Fixes #413

📝 Description

web.go set nil to http.Client.Transport. This will cause the http.Client to use the default transport, this disable HTTP_PROXY use.

🗣️ Type of Change

🐞 Bug fix (non-breaking change which fixes an issue)
✨ New feature (non-breaking change which adds functionality)
📖 Documentation update
⚡ Code refactoring (no functional changes, no api changes)

🤖 AI Code Generation

🤖 Fully AI-generated (100% AI, 0% Human)
🛠️ Mostly AI-generated (AI draft, Human verified/modified)
👨‍💻 Mostly Human-written (Human lead, AI assisted or none)

🔗 Related Issue

📚 Technical Context (Skip for Docs)

Reference URL:
Reasoning:

🧪 Test Environment

Hardware:
OS:
Model/Provider:
Channels:

📸 Evidence (Optional)

Click to view Logs/Screenshots

☑️ Checklist

My code/docs follow the style of this project.
I have performed a self-review of my own changes.
I have updated the documentation accordingly.

Copilot

Pull request overview

This pull request attempts to enable HTTP_PROXY support for web tools by explicitly setting Transport: http.DefaultTransport on the http.Client used by BraveSearchProvider. The change is motivated by issue #413, which requests that the web_fetch tool honor HTTP_PROXY environment variables to allow operation in proxy-required environments.

Changes:

Modified BraveSearchProvider to explicitly set Transport: http.DefaultTransport on its http.Client

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pkg/tools/web.go

nikolasdehor

Thanks for the PR, mattn! The intent is right, but I believe this change doesn't actually address issue #413.

When Transport is nil on http.Client, Go already uses http.DefaultTransport (which includes Proxy: http.ProxyFromEnvironment). So the Brave, DuckDuckGo, and Perplexity search providers already respect HTTP_PROXY/HTTPS_PROXY — this line is effectively a no-op.

The actual problem is in WebFetchTool.Execute() (~line 416 of web.go), which constructs a custom http.Transport{} without setting the Proxy field:

Transport: &http.Transport{
    MaxIdleConns:        10,
    IdleConnTimeout:     30 * time.Second,
    DisableCompression:  false,
    TLSHandshakeTimeout: 15 * time.Second,
},

When you manually construct an http.Transport{}, the Proxy field defaults to nil (no proxy) — unlike http.DefaultTransport which sets Proxy: http.ProxyFromEnvironment.

The fix for #413 should be:

Transport: &http.Transport{
    Proxy:               http.ProxyFromEnvironment,  // ADD THIS
    MaxIdleConns:        10,
    IdleConnTimeout:     30 * time.Second,
    DisableCompression:  false,
    TLSHandshakeTimeout: 15 * time.Second,
},

Would you be open to updating the PR to fix the WebFetchTool transport instead? That's the code path that issue #413 is specifically about. The Brave search client change here can be kept for explicitness, or dropped since it's a no-op — your call.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pkg/tools/web.go

mattn · 2026-02-19T00:24:40Z

I changed PR to use http.NewClient().

Initializing &http.Client{} or &http.Transport{} directly may set default values that differ from the official defaults or omit required field settings.

Fixes sipeed#413

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

pkg/tools/web.go:42

Setting Transport: http.DefaultTransport here is redundant because leaving Transport as nil already uses http.DefaultTransport (which honors ProxyFromEnvironment). This doesn’t match the PR description that says default transport disables HTTP_PROXY; if the proxy bug is in WebFetchTool’s custom transport, consider removing this to avoid confusion or update the PR description accordingly.

	client := http.NewClient()
	client.Timeout = 10 * time.Second
	resp, err := client.Do(req)
	if err != nil {

pkg/tools/web.go:420

There are existing tests for WebFetchTool, but none cover proxy behavior. Since this PR is intended to ensure HTTP_PROXY/HTTPS_PROXY is honored, add a focused test that asserts requests are routed through a proxy when the env var is set (or refactor client creation into a helper that can be unit-tested).

	client.Transport.MaxIdleConns = 10
	client.Transport.IdleConnTimeout = 30 * time.Second
	client.Transport.DisableCompression = false

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-19T00:32:38Z

pkg/tools/web.go

 	resp, err := client.Do(req)
 	if err != nil {


net/http does not provide http.NewClient(), so this will not compile. Use &http.Client{Timeout: ...} (Transport left nil to use the default transport) or construct a client via an existing project helper if one exists.

Copilot · 2026-02-19T00:32:39Z

pkg/tools/web.go

+	client := http.NewClient()
+	client.Timeout = 30 * time.Second


net/http has no http.NewClient() function; this change introduces a compilation error. Revert to &http.Client{Timeout: ...} (or use a valid helper) to keep proxy behavior intact.

Suggested change

client := http.NewClient()

client.Timeout = 30 * time.Second

client := &http.Client{Timeout: 30 * time.Second}

Copilot · 2026-02-19T00:32:39Z

pkg/tools/web.go

+	client := http.NewClient()
+	client.Timeout = 60 * time.Second
+	client.Transport.MaxIdleConns = 10
+	client.Transport.IdleConnTimeout = 30 * time.Second
+	client.Transport.DisableCompression = false
+	client.Transport.TLSHandshakeTimeout = 15 * time.Second
+	client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
+		if len(via) >= 5 {
+			return fmt.Errorf("stopped after 5 redirects")
+		}
+		return nil
 	}



This block will not compile: http.NewClient() doesn’t exist in net/http, and http.Client.Transport is a RoundTripper interface so you can’t set MaxIdleConns/IdleConnTimeout fields on it directly. If the goal is to honor HTTP_PROXY, create a *http.Transport that uses http.ProxyFromEnvironment (or clone http.DefaultTransport and adjust the fields), then assign it to client.Transport.

Suggested change

client := http.NewClient()

client.Timeout = 60 * time.Second

client.Transport.MaxIdleConns = 10

client.Transport.IdleConnTimeout = 30 * time.Second

client.Transport.DisableCompression = false

client.Transport.TLSHandshakeTimeout = 15 * time.Second

client.CheckRedirect = func(req *http.Request, via []*http.Request) error {

if len(via) >= 5 {

return fmt.Errorf("stopped after 5 redirects")

}

return nil

}

transport := &http.Transport{

Proxy: http.ProxyFromEnvironment,

MaxIdleConns: 10,

IdleConnTimeout: 30 * time.Second,

DisableCompression: false,

TLSHandshakeTimeout: 15 * time.Second,

}

client := &http.Client{

Timeout: 60 * time.Second,

Transport: transport,

CheckRedirect: func(req *http.Request, via []*http.Request) error {

if len(via) >= 5 {

return fmt.Errorf("stopped after 5 redirects")

}

return nil

},

}

Copilot AI review requested due to automatic review settings February 18, 2026 14:34

Copilot started reviewing on behalf of mattn February 18, 2026 14:43 View session

Copilot AI reviewed Feb 18, 2026

View reviewed changes

pkg/tools/web.go Outdated Show resolved Hide resolved

nikolasdehor reviewed Feb 18, 2026

View reviewed changes

This was referenced Feb 18, 2026

[Feature] allow web_fetch tool to use proxy #413

Open

fix(tools): add proxy support to web_fetch HTTP transport #447

Closed

Copilot AI review requested due to automatic review settings February 19, 2026 00:21

Copilot started reviewing on behalf of mattn February 19, 2026 00:21 View session

Copilot AI reviewed Feb 19, 2026

View reviewed changes

pkg/tools/web.go Show resolved Hide resolved

Copilot AI review requested due to automatic review settings February 19, 2026 00:29

Copilot started reviewing on behalf of mattn February 19, 2026 00:29 View session

mattn force-pushed the fix/use-default-transport branch from 9075d18 to 57d66f8 Compare February 19, 2026 00:30

Use http.NewClient()

4bfa10f

Fixes sipeed#413

mattn force-pushed the fix/use-default-transport branch from 57d66f8 to 4bfa10f Compare February 19, 2026 00:31

Copilot AI reviewed Feb 19, 2026

View reviewed changes

mattn closed this Feb 19, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use DefaultTransport for http.Client#422

Use DefaultTransport for http.Client#422
mattn wants to merge 1 commit intosipeed:mainfrom
mattn:fix/use-default-transport

mattn commented Feb 18, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

nikolasdehor left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

mattn commented Feb 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	client := http.NewClient()
	client.Timeout = 30 * time.Second
	client := &http.Client{Timeout: 30 * time.Second}

Conversation

mattn commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Description

🗣️ Type of Change

🤖 AI Code Generation

🔗 Related Issue

📚 Technical Context (Skip for Docs)

🧪 Test Environment

📸 Evidence (Optional)

☑️ Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

nikolasdehor left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

mattn commented Feb 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mattn commented Feb 18, 2026 •

edited

Loading