feat: implement and refactor network request retries

[scott-ray-wilson]

Description 📣

Nothing in the CLI retried 429 or 504. Resty's built-in default only retries transport errors, and it is silently replaced the moment a retry condition is added, so status codes were never inspected anywhere.

State before this PR:

Surface	Retry behavior
util.GetRestyClientWithCustomHeaders(), 48 call sites	none at all
9 direct resty.New() sites (agentproxy/*, pam/*, cmd/agent_proxy*.go)	none, and they also skipped INFISICAL_CUSTOM_HEADERS
util/helper.go, cmd/agent.go	SetRetryCount(10000) with no condition
cmd/login.go	SetRetryCount(5), same

The 10000 is the tell: a connection refused retried ten thousand times while a 429 failed on the first attempt.

Fix: one policy in packages/util/retry.go, applied inside GetRestyClientWithCustomHeaders(). That single edit covers the 48 existing sites and every future api.Call*. The 9 direct constructions were converted, so there is now exactly one construction path, and all 5 ad-hoc overrides are gone.

Policy

• Retries 429, 502, 503, 504, plus typed transport errors (net.Error, ECONNRESET/ECONNREFUSED/EPIPE/EHOSTUNREACH/ENETUNREACH/ETIMEDOUT, io.EOF). Type and errno checks, not substring matching on error text.
• Honors Retry-After in both RFC 9110 forms, capped at MaxDelay. Nothing read that header before.
• POST retries only on 429. A 502/503/504 can mean the server did process the write and only the response was lost, so replaying a POST risks double-applying it, for instance minting a second dynamic secret lease. 429 is safe because the server states it rejected the request outright.
• Does not retry 4xx, bare 500, TLS trust failures, or context.Canceled/DeadlineExceeded. Allow-list, so anything unrecognised surfaces immediately.
• Three named policies, all honoring INFISICAL_RETRY_*: DefaultRetryPolicy (3 retries / 10s), AgentRetryPolicy (30 / 30s, for the agent's token lifecycle), BestEffortRetryPolicy (1 / 1s, for agent-proxy usage reporting, which also runs on the shutdown path).

TestNoDirectRestyConstruction walks packages/ and fails on any new resty.New(). Without it the choke point silently decays: a direct construction compiles, runs, and looks fine in review, it just has no retries.

Type ✨

• Bug fix
• New feature
• Improvement
• Breaking change
• Documentation

Behavior changes worth a look

• Agent token refresh: effectively-infinite retries capped at 30 (AgentRetryPolicy).
• cmd/login.go: 5 retries becomes 3, but now covers 429/504 where it previously covered neither.
• Malformed INFISICAL_RETRY_* warns and falls back to the default instead of os.Exit(1), since this now runs while building a client for any command. The agent's stricter SDK-side validation is unchanged.
• The 9 converted sites now honor INFISICAL_CUSTOM_HEADERS. Latent fix, but a behavior change.

Known gap

17 sites build infisicalSdk.NewInfisicalClient (ssh, dynamic-secrets, gateway, pam, agent templating). The SDK's own retry condition never inspects status codes either, so 429/504 there still fail on the first attempt. Closing that needs a go-sdk change, since its opt-in RetryRequestsConfig retries any IsError() including 401 and 404. Out of scope here. The high-traffic paths are covered: infisical run, secrets get, and export all go through packages/util/secrets.go, which uses the shared constructor.

Also still outside resty entirely: packages/pam/session/chunk_uploader.go:458 uses http.DefaultClient.

Tests 🛠️

Unit tests in packages/util/retry_test.go cover the status-code matrix, POST-vs-GET method safety, Retry-After parsing and capping, transport-error classification (including that TLS trust failures are not retried), env overrides, and the choke-point guard.

go build ./...
go test ./packages/... -count=1 -vet=off
go vet ./packages/util/ ./packages/agentproxy/ ./packages/pam/...

The guard test was verified to actually fail on a planted resty.New(), not just pass vacuously.

Manual verification

Drove the built binary against a mock API, counting requests server-side rather than trusting the CLI's own logs.

cat > /tmp/mock.py <<'PY'
import json, os, time
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
SEQ = [int(x) for x in os.environ.get("SEQUENCE", "200").split(",")]  # last value repeats
RA, PORT = os.environ.get("RETRY_AFTER", ""), int(os.environ.get("PORT", "8899"))
hits, start = [], time.monotonic()
class H(BaseHTTPRequestHandler):
    protocol_version = "HTTP/1.1"
    def _h(self):
        n = int(self.headers.get("Content-Length") or 0)
        if n: self.rfile.read(n)   # must drain, or the next keep-alive request is mangled
        i = len(hits); s = SEQ[i] if i < len(SEQ) else SEQ[-1]; hits.append(s)
        print(f"HIT #{i+1} t=+{time.monotonic()-start:5.2f}s {self.command} -> {s}", flush=True)
        b = json.dumps({"accessToken": "renewed", "accessTokenTTL": 3600}).encode()
        self.send_response(s)
        if RA and s in (429, 503): self.send_header("Retry-After", RA)
        self.send_header("Content-Type", "application/json"); self.send_header("Content-Length", str(len(b)))
        self.end_headers(); self.wfile.write(b)
    do_GET = do_POST = do_PUT = do_DELETE = do_PATCH = _h
    def log_message(self, *a): pass
ThreadingHTTPServer(("127.0.0.1", PORT), H).serve_forever()
PY

go build -o /tmp/infisical .

# 1. POST + 429 twice then 200 -> retries, then succeeds
SEQUENCE=429,429,200 PORT=8901 python3 /tmp/mock.py &
/tmp/infisical token renew ua.mock --domain http://127.0.0.1:8901 --log-level debug

# 2. POST + 504 -> must NOT retry (unsafe to replay a write)
SEQUENCE=504 PORT=8902 python3 /tmp/mock.py &
/tmp/infisical token renew ua.mock --domain http://127.0.0.1:8902 --log-level debug

# 3. Retry-After honored (gaps ~2s, not the 500ms base delay)
SEQUENCE=429,429,200 RETRY_AFTER=2 PORT=8903 python3 /tmp/mock.py &
/tmp/infisical token renew ua.mock --domain http://127.0.0.1:8903 --log-level debug

# 4. GET + 504 -> must retry (safe to replay a read); this is the `infisical run` path
SEQUENCE=504 PORT=8904 python3 /tmp/mock.py &
INFISICAL_TOKEN=ua.mock /tmp/infisical secrets --projectId p --env dev --domain http://127.0.0.1:8904 --log-level debug

# 5. Env overrides
SEQUENCE=504 PORT=8905 python3 /tmp/mock.py &
INFISICAL_TOKEN=ua.mock INFISICAL_RETRY_MAX_RETRIES=6 INFISICAL_RETRY_BASE_DELAY=100ms \
  /tmp/infisical secrets --projectId p --env dev --domain http://127.0.0.1:8905 --log-level debug

# 6. Transport error: bind and release a port so nothing is listening
/tmp/infisical token renew ua.mock --domain http://127.0.0.1:59889 --log-level debug

Results:

#	Scenario	Expected	Observed
1	POST, 429 ×2 then 200	retry then succeed	3 hits at 0.05 / 0.55 / 1.38s, succeeded
2	POST, 504	no retry	1 hit
3	POST, 429 + Retry-After: 2	~2s gaps	2.00s and 2.01s
4	GET, 504	4 attempts	4 hits, 0.15 / 0.65 / 1.50 / 3.20s
5	6 retries, 100ms base	7 attempts, ~100ms first gap	7 hits, first gap 0.11s
6	connection refused	4 attempts	4 attempts, then "retries are exhausted"
-	POST, 401	no retry	1 hit

Not run: the e2e/ suite. It builds and vets clean (cd e2e && go build ./... && go vet ./...), but running it needs INFISICAL_BACKEND_DIR and an e2e/.env. Nothing was tested against a real Infisical instance.

---

• I have read the contributing guide, agreed and acknowledged the code of conduct. 📝

🤖 Generated with Claude Code

This PR adds/unifies request retry attempts

Type ✨

• Bug fix
• New feature
• Improvement
• Breaking change
• Documentation

Tests 🛠️

# Here's some code block to paste some code snippets

---

• I have read the contributing guide, agreed and acknowledged the code of conduct. 📝

[linear]

SECRETS-576

[infisical-review-police]

💬 Discussion in Slack: #pr-review-cli-378-feat-implement-and-refactor-network-request-retries

Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel.

[veria-ai]

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 1 · PR risk: 0/10

[greptile-apps]

Greptile Summary

The PR centralizes Resty client construction and applies bounded retry policies to CLI, agent-proxy, and PAM API requests.

• Adds retry handling for selected transient HTTP statuses and transport failures.
• Restricts ambiguous retries according to HTTP method safety and honors bounded Retry-After values.
• Migrates direct Resty construction sites to the shared client factory.
• Adds policy, environment-override, transport-safety, and construction-guard tests.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
packages/util/retry.go	Defines centralized retry policies, transient-failure classification, method-safety controls, bounded server-directed delays, and retry logging.
packages/util/retry_test.go	Covers retry status selection, transport failures, cancellation, TLS errors, method safety, environment overrides, and centralized client construction.
packages/util/common.go	Routes shared Resty client construction through the selected retry policy while retaining custom-header parsing.
packages/agentproxy/leases.go	Migrates dynamic lease creation and revocation to the shared retry-enabled client.
packages/cmd/agent.go	Replaces the agent token-refresh retry override with the centralized long-running agent policy.
packages/agentproxy/proxy.go	Applies the bounded best-effort retry policy to proxy usage reporting.

_{Reviews (2): Last reviewed commit: "improvement: address feedback" | Re-trigger Greptile}

[scott-ray-wilson]

@greptile review

[Thiago-AS]

Maybe we should tone down the comments lol. There are a lot of them, and they can get stale pretty easily. Most of them are just stating the obvious.

Claude, go over the comments that were added and leave only what’s necessary. Remove anything obvious or self-documented by the code. Keep the comments concise. No em dashes XD