refactor(planfix): route all requests through Client.Proxy

Previously only Stream read c.Proxy; Do/JSON got proxying from the default
transport, which reads the environment independently. Setting c.Proxy would
have governed downloads but not other requests -- an inconsistent knob.

New now clones the standard transport (keeping its connection-pool defaults)
for the shared Do/JSON client and points its Proxy at c.Proxy, resolved per
request; Stream uses the same helper. A single field now governs proxying for
every request, and nil disables it.

Author: a68366

AGENTS.md

@@ -96,7 +96,7 @@ Implemented:
 - Precedence: command-line flags > environment (`PFIX_DOMAIN`, `PFIX_TOKEN`, `PFIX_PROFILE`, `PFIX_CONFIG`) > config file. Profile name resolves through `config.ResolveProfileName` (`flag > PFIX_PROFILE > current_profile > "default"`) — use it everywhere a command needs the active profile, so the commands stay consistent.
 - Output: typed `task` commands default to a human-readable table (list) or key/value detail (single object), rendered by `internal/output` (stdlib `text/tabwriter`, no color). `--json` emits the API response unmodified (pretty-printed); `--fields` overrides the requested fields and table columns; `-q/--quiet` drops the header row (lists) or prints only the affected id (create/update/comment add). `--jq <expr>` filters the JSON output through a jq expression (implies `--json`). `api` always emits raw JSON. Errors go to stderr with a non-zero exit code. The Planfix layer stays thin — commands render generically from decoded `map[string]any` via dot-paths rather than typed structs, so unconfirmed nested shapes need no model.
 - Transport: `Client.Do` returns the HTTP response for any status (callers inspect `StatusCode` and use `planfix.ParseError` for detail). It retries connection errors + 5xx, never 4xx. Every request carries a `User-Agent` of `pfix/<version>` (from `buildinfo.Version`, set on the `Client.UserAgent` field in `New`); a caller-supplied `User-Agent` header — e.g. `api -H "User-Agent: ..."` — overrides it.
-- Proxy: the client follows the standard Go proxy environment variables (`HTTP(S)_PROXY`/`NO_PROXY`). `Do`/`JSON` inherit them via the default transport; `Stream` (file download) builds its own `http.Transport`, so it must set the proxy explicitly — it uses `Proxy: c.Proxy`, whose `New` default is `http.ProxyFromEnvironment` (a bare transport would disable proxying, so downloads would ignore the env and — where the only egress is a proxy — fail outright). `ALL_PROXY` is not honored (the Go stdlib `httpproxy` package does not read it).
+- Proxy: the client follows the standard Go proxy environment variables (`HTTP(S)_PROXY`/`NO_PROXY`) for every request. Both HTTP paths resolve the proxy through the `Client.Proxy` field (default `http.ProxyFromEnvironment`, set in `New`): `New` clones the standard transport for the shared `Do`/`JSON` client, and `Stream` (file download) builds its own timeout-free transport — both point `Proxy` at `c.Proxy`, resolved per request, so overriding the field (or nil-ing it to force a direct connection) governs all requests. A bare `http.Transport` would instead disable proxying outright, which is why `Stream` must not use one. `ALL_PROXY` is not honored (the Go stdlib `httpproxy` package does not read it).
 - API specifics: list endpoints are POST with `pageSize`/`offset`/`fields`/`filters`; fields must be requested explicitly — ship sensible per-resource defaults, overridable with `--fields`.
 - Files: `task files`/`contact files`/`project files --source inline` *composes* its JSON output (`{"result":"success","files":[...]}` built from resolved ids) rather than echoing an API response — there is no single endpoint for inline files, so `--json`/`--jq` there reflect pfix's aggregation, not one call's response body.
 - Files: `--fields` on the files commands (`task files`, `contact files`, `project files`) selects table columns only. No file-listing endpoint supports server-side field selection — `/file/{id}`'s own `fields` parameter is accepted and silently ignored — so unlike other typed commands, `--fields` there never changes what the API is asked for.

internal/planfix/client.go

@@ -25,24 +25,43 @@ type Client struct {
 	Retries   int
 	Backoff   func(attempt int) time.Duration
 	UserAgent string // sent as the User-Agent header unless a caller overrides it
-	// Proxy selects the proxy for Stream (file downloads); default
-	// http.ProxyFromEnvironment. Do/JSON go through HTTP, whose default
-	// transport reads the same environment variables independently.
+	// Proxy selects the proxy for every request; default
+	// http.ProxyFromEnvironment. Both the shared HTTP client and Stream's
+	// timeout-free client resolve it through this field at request time, so
+	// overriding it (or setting it to nil to disable proxying) governs all
+	// requests.
 	Proxy func(*http.Request) (*url.URL, error)
 }
 
 // New returns a Client with sane defaults (~5 req/s, 3 attempts).
 func New(domain, token string) *Client {
-	return &Client{
+	c := &Client{
 		Domain:    domain,
 		Token:     token,
-		HTTP:      &http.Client{Timeout: 30 * time.Second},
 		Limiter:   rate.NewLimiter(rate.Limit(5), 1),
 		Retries:   3,
 		Backoff:   defaultBackoff,
 		UserAgent: "pfix/" + buildinfo.Version,
 		Proxy:     http.ProxyFromEnvironment,
 	}
+	c.HTTP = &http.Client{Timeout: 30 * time.Second, Transport: c.newTransport(0)}
+	return c
+}
+
+// newTransport builds an HTTP transport that resolves its proxy through c.Proxy
+// at request time — so overriding c.Proxy governs every request — and clones the
+// standard transport for its connection-pool defaults. respHeaderTimeout caps
+// the wait for a response's headers (0 disables it).
+func (c *Client) newTransport(respHeaderTimeout time.Duration) *http.Transport {
+	t := http.DefaultTransport.(*http.Transport).Clone()
+	t.Proxy = func(req *http.Request) (*url.URL, error) {
+		if c.Proxy == nil {
+			return nil, nil
+		}
+		return c.Proxy(req)
+	}
+	t.ResponseHeaderTimeout = respHeaderTimeout
+	return t
 }
 
 func defaultBackoff(attempt int) time.Duration {
@@ -70,16 +89,12 @@ func (c *Client) Do(ctx context.Context, method, path string, body []byte, heade
 // Stream sends an authenticated GET and returns the response with its Body
 // unread; the caller must close it. Unlike Do/JSON it runs against a client
 // with no whole-request timeout (only a 30s response-header timeout), so
-// reading a large body is not cut off by a deadline. Its transport takes its
-// proxy from c.Proxy (default http.ProxyFromEnvironment), so downloads honor
-// HTTP(S)_PROXY/NO_PROXY like the default client — a bare http.Transport would
-// disable proxying. Redirects to object storage are followed by net/http, which
-// drops the Authorization header on the cross-host hop.
+// reading a large body is not cut off by a deadline. It shares Do's proxy
+// handling via c.Proxy (see the Proxy field). Redirects to object storage are
+// followed by net/http, which drops the Authorization header on the cross-host
+// hop.
 func (c *Client) Stream(ctx context.Context, path string) (*http.Response, error) {
-	hc := &http.Client{Transport: &http.Transport{
-		Proxy:                 c.Proxy,
-		ResponseHeaderTimeout: 30 * time.Second,
-	}}
+	hc := &http.Client{Transport: c.newTransport(30 * time.Second)}
 	return c.do(ctx, http.MethodGet, path, nil, nil, hc)
 }
 

internal/planfix/client_test.go

@@ -174,6 +174,34 @@ func TestDoDoesNotRetryOn4xx(t *testing.T) {
 	}
 }
 
+func TestDoRoutesThroughProxy(t *testing.T) {
+	var proxied int32
+	proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		atomic.AddInt32(&proxied, 1)
+		io.WriteString(w, `{"ok":true}`)
+	}))
+	defer proxy.Close()
+
+	proxyURL, err := url.Parse(proxy.URL)
+	if err != nil {
+		t.Fatalf("parse proxy URL: %v", err)
+	}
+
+	// Non-routable base: a success proves Do reaches the origin only via the
+	// proxy, i.e. that c.Proxy governs Do (not just Stream).
+	c := fastClient("http://origin.invalid/rest")
+	c.Proxy = func(*http.Request) (*url.URL, error) { return proxyURL, nil }
+
+	resp, err := c.Do(context.Background(), "GET", "task/1", nil, nil)
+	if err != nil {
+		t.Fatalf("Do: %v", err)
+	}
+	resp.Body.Close()
+	if got := atomic.LoadInt32(&proxied); got != 1 {
+		t.Fatalf("proxy calls = %d, want 1", got)
+	}
+}
+
 func TestStreamRoutesThroughProxy(t *testing.T) {
 	var proxied int32
 	proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {