refactor(github): retry GitHub requests through the fleet's shared pRetry

[John-David Dalton (jdalton)]

What this changes

src/utils/github-errors.mts had its own retry loop: a for (let attempt = 1; ; attempt += 1) counter, a backoffMs() function computing Math.min(1000 * 2 ** (attempt - 1), 10_000), and a hand-written sleep() promise. The fleet already ships that exact helper as pRetry, and @socketsecurity/registry is already a dependency of this branch, so the loop was a second copy of code we maintain elsewhere. Two copies of a backoff policy for the same API will drift apart, and the local copy had no way to make the delay short in a test.

This PR hands the transient-failure retries to pRetry and deletes the local loop. MAX_TRANSIENT_ATTEMPTS, MAX_BACKOFF_MS, backoffMs(), and sleep() are gone.

Nothing about the rate-limit detection changes. classifyGitHubResponse, getRateLimitWaitSeconds, and isGitHubBlockingError behave exactly as before, and the scan loop still short-circuits on a blocking error rather than reporting a silent success on a throttled run. That fix is the reason this file exists and it is fully preserved.

The retry policy is now the shared one, which changes two things a user can notice

Transient failures now back off on the fleet's 5s/10s schedule instead of the local 1s/2s one

The retry policy adopted here matches GITHUB_RETRY_CONFIG in @socketsecurity/lib, which is the fleet's chosen policy for the GitHub API: two retries on top of the initial attempt, delay doubling each time, capped at 10 seconds, with jitter.

	Before	After
Total attempts on a 5xx or network failure	3	3, unchanged
Delay before retry 1	1s	5s, plus jitter
Delay before retry 2	2s	10s, plus jitter
Delay cap	10s	10s, unchanged

So a GitHub outage that never recovers now costs up to about 15 seconds of waiting instead of 3. That is deliberate: a GitHub 5xx frequently needs more than a second to clear, and one second of backoff mostly buys you a second failure. The attempt count is unchanged, so no request is made that was not made before.

The rate-limit path picks up one extra backoff delay it did not have. When GitHub reports a short reset window, the code still waits out that window and retries; that retry is now driven by pRetry, so the configured backoff is added on top of the server's window. In practice that is a few extra seconds on an already-throttled run.

The backoff delay is now settable through SOCKET_GITHUB_RETRY_BASE_DELAY_MS, which is what makes the retry path testable

The base delay is read from SOCKET_GITHUB_RETRY_BASE_DELAY_MS on every call, falling back to 5000. The name and the default are deliberately identical to the override in @socketsecurity/lib's releases/github-retry-config, so both socket-cli lines answer to the same knob and cannot drift on the value.

This is not a convenience. The old tests reached for vi.useFakeTimers() plus vi.runAllTimersAsync(), which worked only because the local sleep() used a plain setTimeout. pRetry sleeps through node:timers/promises, which fake timers do not reliably intercept, so the first run of the converted tests hung for the full real delay and timed out. Setting the override to 0 is the fix, and it is fake-timer-independent. The tests now do that in a beforeEach and restore the previous value afterwards.

Why the classifier is still local rather than imported from socket-lib, and what happens next

The classification logic in this file — deciding from a status, headers, and body that a response is a rate limit, abuse detection, or an auth failure, and the idea of a blocking error that should stop a loop over repositories — has been contributed upstream to socket-lib as github/error-classification in SocketDev/socket-lib#221, so there is now one canonical copy to converge on.

This branch cannot import it yet, and the reason is a hard one rather than a matter of taste. @socketsecurity/lib declares engines.node: ">=22". This branch declares engines.node: ">=18.20.8" and its CI compatibility matrix actively tests Node 20, 22, and 24. Adding that library here would either put an unsupported-on-Node-20 dependency into the line that owns the latest npm dist-tag, or force a Node floor raise, and neither belongs inside a retry refactor. This branch is also pinned to @socketsecurity/registry@1.1.17, the pre-split package, so pulling in @socketsecurity/lib@6.x alongside it would ship two generations of the same utility library in one bundle.

pRetry was available without any of that, because @socketsecurity/registry@1.1.17 already exports it with the identical option shape that socket-lib's copy uses. That is the part of the deduplication this branch can take today, so that is what this PR does.

One thing did improve on the way through. The test named flags a 403 with x-ratelimit-remaining: 0 as a rate limit used a response body that also said "rate limit", so the body detector was quietly covering for the header detector: deleting the header check left that test green. The body is now one that says nothing about throttling, so the test finally proves what its name claims. That header-only 403 is precisely the shape that used to be misread as "this repository has no manifests".

Testing

Every detector touched was broken on purpose and confirmed to turn a named test red

Mutation applied	Named test that went red
x-ratelimit-remaining: 0 detector removed	classifyGitHubResponse > flags a 403 with x-ratelimit-remaining: 0 as a rate limit
classifier always returns "not a blocking error"	10 tests, including githubApiRequest > surfaces a long-window rate limit immediately without retrying and githubApiRequest > never retries an auth failure
auth failure marked retryable	githubApiRequest > never retries an auth failure
transient 5xx marked non-retryable	githubApiRequest > retries transient 5xx with bounded backoff, then surfaces a server error, githubApiRequest > honors SOCKET_GITHUB_RETRY_BASE_DELAY_MS for the backoff delay
env override no longer reaches the retry policy	githubApiRequest > honors SOCKET_GITHUB_RETRY_BASE_DELAY_MS for the backoff delay
retry loop no longer stops early on a non-retryable failure	githubApiRequest > surfaces a long-window rate limit immediately without retrying, githubApiRequest > never retries an auth failure, githubApiRequest > waits out the reset window only once, then surfaces the rate limit
caller shown the first failure instead of the last	5 tests, including githubApiRequest > retries a network-level failure, then surfaces it

The source was restored and diffed byte-for-byte against the pre-mutation copy after each run.

Three tests are new: one that a short reset window is waited out only once before the rate limit is surfaced, one that a network-level failure is retried and then reported, and one that the env override actually reaches the backoff by measuring elapsed time.

Ran, all from a clean worktree off this branch's base, with a baseline captured first:

Gate	Baseline on the unmodified base	With this change
vitest run src/utils/github-errors.test.mts src/commands/scan/create-scan-from-github.test.mts	25 passed, exit 0	28 passed, exit 0
vitest run src/utils src/commands/scan	not captured	40 files, 530 passed, exit 0
pnpm run lint	369 warnings, 0 errors, exit 0	369 warnings, 0 errors, exit 0 — identical count, no finding names this file
pnpm run check:lint	exit 0	exit 0, after fixing the import-order and function-scoping errors it raised on the first pass
pnpm run check:tsc	exit 0	exit 0

Did not run

• The full test suite and the end-to-end suites. The change is scoped to one utility module and its one consumer, both of which are covered above.
• A live call against the real GitHub API. The retry paths are exercised through the module's existing injectable fetchImpl parameter, which is what it is there for.

---

Note

Medium Risk
Changes how long socket scan github waits on GitHub 5xx/network and short rate-limit retries without altering rate-limit/auth classification, so outage behavior shifts but silent-success misreads should not return.

Overview
Replaces the hand-written retry loop in github-errors.mts (local backoffMs, sleep, and attempt counter) with the shared pRetry helper from @socketsecurity/registry, aligned with the fleet GITHUB_RETRY_CONFIG (two retries, doubling delay capped at 10s).

Transient 5xx and network failures still get three attempts total, but backoff now starts at 5s (not 1s) unless SOCKET_GITHUB_RETRY_BASE_DELAY_MS overrides it—read per request so tests can set 0. GitHubRequestFailure tells pRetry when to stop early (auth, long rate limits) and keeps the last failure for the caller. Short-window rate-limit handling still waits once on Retry-After / reset headers; classification and blocking-error behavior are unchanged.

Tests drop fake timers (incompatible with node:timers/promises), zero the env delay in beforeEach, and add coverage for single rate-limit wait, network retries, and env-driven backoff timing. One classifier test now uses a 403 + x-ratelimit-remaining: 0 body that does not mention rate limits, so the header path is actually exercised.

^{Reviewed by Cursor Bugbot for commit 5a28451. Configure here.}

[John-David Dalton (jdalton)]

[agent] The one red check was a flake, not this branch. e2e-tests (20, ubuntu-latest) failed on cmd-fix.e2e.test.mts in the Python project case, asserting django should move off 3.0.0 and getting 3.0.0 back. The CLI output in that job says "The Socket backend resolved 0 artifacts, so vulnerability discovery may be incomplete" — the backend returned nothing to fix, so there was nothing to upgrade.

Three things say it is not the code here. The same commit passed e2e-tests on node 22 and node 24 in that same run, and nothing in this diff is node-version dependent. This branch only touches src/utils/github-errors.mts, which is the GitHub API retry path and has nothing to do with resolving artifacts from the Socket backend. And rerunning the failed job on the identical commit passed: https://github.com/SocketDev/socket-cli/actions/runs/30920466565/job/92032579788

All 12 checks are green now, including the three required e2e-tests jobs. The branch is a single signed commit on the v1.x tip and auto-merge is armed, so it needs one approving review to go in.