fix(miner): bound oauth-device-flow.js's GitHub fetches with a request timeout#7048
Conversation
…t timeout requestDeviceCode and pollForAccessToken's per-attempt fetch had no AbortSignal.timeout, unlike every other GitHub-facing fetch in this package -- a stalled connection could hang indefinitely instead of reaching init-wizard.js's documented pasted-token fallback. A timed-out poll attempt is now caught and treated as a per-attempt failure that still retries, bounded by the existing deadline, rather than an unhandled rejection.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7048 +/- ##
==========================================
- Coverage 93.75% 93.74% -0.01%
==========================================
Files 692 692
Lines 68760 68709 -51
Branches 18769 18760 -9
==========================================
- Hits 64463 64412 -51
Misses 3302 3302
Partials 995 995
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-17 19:53:19 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
packages/loopover-miner/lib/oauth-device-flow.js's two GitHub-facing fetches —requestDeviceCodeandpollForAccessToken's per-attempt fetch — had noAbortSignal.timeout, unlike every other GitHub-facing fetch in this package (github-token-resolution.js'sGITHUB_TOKEN_FETCH_TIMEOUT_MSprecedent, and everything routed throughhttp-retry.js).pollForAccessToken's owndeadlinecheck only fires between polling attempts, so a single stalled connection mid-fetch could hang indefinitely rather than being caught by the deadline — undermininginit-wizard.js's documented "never a hard dependency ... automatic fallback on any device-flow failure", since a hang never reaches thecatchmeant to trigger the pasted-token fallback.DEVICE_FLOW_FETCH_TIMEOUT_MS = 10_000constant (matching the package's existing per-file constant convention) and wiressignal: AbortSignal.timeout(...)into both fetches.pollForAccessToken's per-attempt fetch in try/catch: a timeout/abort is now a per-attempt failure thatcontinues the polling loop (still bounded by the existingdeadlinecheck), not an unhandled rejection that crashes the flow.Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #6988
Validation
git diff --checknpm run actionlintnpm run typecheck— the whole-repotsc --noEmitreliably OOMs on this shared, memory-constrained sandbox regardless of what changed;node --checkon the modified file (passed) plus the full targeted vitest run below stand in as the local proxy, and CI's isolated runner performs the authoritativetsc --noEmit. This module's public function signatures are unchanged (verified against its hand-maintainedoauth-device-flow.d.tscompanion — only internal fetch-call bodies changed), so no.d.tsupdate was needed.npm run test:coverage—test/unit/miner-oauth-device-flow.test.ts(27 tests, incl. 4 new: theAbortSignaltimeout on both fetches, a rejected/timed-out attempt still retrying to a successful poll, and a permanently-stalled connection still respecting the deadline) — 27/27 passing.npm run test:workers/npm run build:mcp/npm run test:mcp-pack/npm run ui:openapi:check(not applicable — this is a CLI-package module with no Worker/MCP-packaging/API-schema surface)npm audit --audit-level=moderate(no dependency changes)If any required check was skipped, explain why:
npm run typecheckOOMs on this specific sandbox under current memory pressure regardless of diff size;node --check(syntax) plus the full targeted test run above are the local proxy for this JS module, and CI's isolated runner performs the realtsc --noEmit(which type-checks the unchanged, hand-maintained.d.tscompanion against this file's actual exports).npm run test:workersand the MCP/OpenAPI checks have no surface to exercise for a change scoped to two fetch calls in one miner CLI module.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots. (N/A — backend CLI-only change, no UI surface.)