feat(login): --device login via RFC 8628 device flow - #199
Conversation
Adds `insforge login --device` for environments where the browser can never reach a loopback callback (agent sandboxes like the ChatGPT app, SSH, containers, CI): - requestDeviceAuthorization: POST /oauth/v1/device_authorization -> device_code + user_code (with a clear error when the server/client doesn't support the grant yet). - pollForDeviceTokens: polls the token endpoint per RFC 8628 §3.5 — waits on authorization_pending, backs off +5s on slow_down, stops on access_denied/expired_token, rides out transient network errors, and gives up at the expires_in deadline. - performDeviceLogin: prints the verification URL + code (opens the browser best-effort), polls, saves credentials + profile. The default browser flow is unchanged. Requires backend support (insforge-cloud-backend#733) and the dashboard approve page (insforge-cloud#590). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GcszudKMZBSm5RMP14h7fu
WalkthroughChangesDevice login
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Caught by live e2e against a local backend: the poll sleep was the only thing on the event loop (no callback server in this flow), so unref'ing it let Node exit mid-poll with an unsettled top-level await (exit 13). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GcszudKMZBSm5RMP14h7fu
|
Live e2e completed against a local cloud-backend running InsForge/insforge-cloud-backend#733: happy path (code issued → dashboard-API approve → authenticated in one poll cycle → 🐛 Found & fixed by the e2e (ceb1bca): the poll sleep timer was unref'd, and since this flow has no callback server, the event loop drained and Node exited mid-poll with an unsettled top-level await. Unit tests couldn't catch this (vitest keeps the loop alive) — only the real binary did. |
jwfing
left a comment
jwfing
left a comment
There was a problem hiding this comment.
Review: feat(login): --device login via RFC 8628 device flow
Summary: A clean, RFC 8628-compliant device authorization grant that mirrors the existing browser flow's structure and error handling; no blocking issues found.
Requirements context
No matching spec/plan found under docs/specs/ (the only specs present cover the diagnose and db-migrations commands, not login). Assessed against the PR description, linked backend dependency (insforge-cloud-backend#733), and RFC 8628 §3.5. Implementation intent is clearly documented in the PR body and matches the code.
Findings
Critical
(none) — no correctness, security, or data-loss issues, and the default browser flow is untouched.
Suggestion
-
Functionality / Performance —
intervalmay beundefined→ tight polling loop.src/lib/auth.ts(pollForDeviceTokens):let intervalMs = Math.max(params.interval, 1) * 1000;. RFC 8628 §3.2 makes theintervalfield optional in the device-authorization response (default 5s). TheDeviceAuthorizationinterface typesintervalas a requirednumber, but that is not enforced at runtime — if a compliant-but-minimal server omits it,device.intervalisundefined,Math.max(undefined, 1)isNaN, andsetTimeout(fn, NaN)fires immediately, producing a zero-delay poll loop that hammers/api/oauth/v1/token(and likely trips server rate-limiting). The paired backend (#733) presumably always sendsinterval: 5, so this is a robustness gap rather than a live bug, but a defensive default would close it:let intervalMs = Math.max(params.interval || 5, 1) * 1000;
A test feeding a response without
intervalwould lock this in. -
Software engineering — orchestration path is untested. The new tests cover the
requestDeviceAuthorization/pollForDeviceTokensstate machine well (pending, slow_down backoff, denial, server + local expiry, transient network faults — nice).performDeviceLogin(save-before-profile ordering, spinner/JSON branching, best-effort browser open) has no test. This is consistent with the existingperformOAuthLogin, which also has no test, so it isn't a regression — noting it only as a coverage opportunity for the credential-save ordering, which is the one part with real failure modes.
Information
- Dead parameter.
pollForDeviceTokensdeclaresonPoll?: () => voidand callsparams.onPoll?.(), but no caller ever passes it (performDeviceLoginomits it, tests omit it). Either wire it to the spinner or drop it. - Error-message style inconsistency.
requestDeviceAuthorizationthrowsnew Error(formatFetchError(err, url), { cause: err })on network failure, whereasexchangeCodeForTokens/refreshOAuthTokenprefix the context (`Token exchange failed — ${formatFetchError(...)}`). Minor consistency nit. - Spinner wording.
s?.start('Waiting for approval in the browser (code ...)')— in the device flow the user may approve on a different device than the CLI host, so "in the browser" is slightly narrow. The non-interactive stderr copy ("ask the user to open ...") is clearer.
Dimension coverage
- Security: No new secrets exposed —
user_code/verification_uriare printed by design; the sensitivedevice_codeis never logged, and access/refresh tokens are not surfaced. No new dependencies (openalready present, pinned). No auth checks weakened; device flow correctly omits PKCE/state (no redirect, thedevice_codeis the bearer secret). No concerns. - Performance: Polling respects the server interval with a 1s floor and honors
slow_down(+5s) and the localexpires_indeadline. Only concern is theNaNedge above.
Verdict
approved (informational — human approval via the standard flow). Zero Critical findings; the interval default is the one worth addressing before/at merge, but it's non-blocking given the backend contract. Note this PR is still a draft and depends on insforge-cloud-backend#733 landing first.
- Default the poll interval to 5s when the server omits it (RFC 8628 §3.2 makes it optional) — a missing value previously became NaN and a zero-delay poll loop. Locked in with a fake-timer test. - Drop the never-used onPoll callback parameter. - Prefix the device-authorization network error like the sibling token helpers; broaden the waiting-spinner wording (approval may happen on another device). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GcszudKMZBSm5RMP14h7fu
jwfing
left a comment
jwfing
left a comment
There was a problem hiding this comment.
Review: feat(login): --device login via RFC 8628 device flow
Summary: A clean, well-tested RFC 8628 device-authorization login path that mirrors the existing browser flow's conventions; no blocking issues found.
Requirements context
No matching spec/plan found — docs/superpowers/ does not exist in this repo, and docs/specs/ only contains the diagnose and db-migrations designs (nothing on device/OAuth login). Assessed against the PR description (RFC 8628 device flow) and the existing performOAuthLogin implementation as the convention baseline.
I verified the new tests locally: vitest run src/lib/auth.device.test.ts → 9/9 pass. Coverage of the poll state machine is genuinely good (pending→success, slow_down backoff, access_denied, server-side expired_token, local deadline, missing-interval default via fake timers, transient network faults). Telemetry (method: 'oauth_device' via trackTopLevelUsage) follows the existing login convention, and the method ternary refactor in login.ts:23-29 is behavior-preserving for the existing paths.
Critical
(none)
Suggestion
Functionality — hard dependency on the optional verification_uri_complete — src/lib/auth.ts:423,430,437
performDeviceLogin displays and open()s device.verification_uri_complete exclusively. Per RFC 8628 §3.2 that field is OPTIONAL; only verification_uri + user_code are guaranteed. If the backend ever omits it, the user sees Open undefined and confirm this code: … and the browser-open silently opens undefined. Since paired backend #733 controls the response the blast radius is low today, but a small fallback (use verification_uri and always show user_code) would make the CLI robust to a spec-compliant server. Note the declared DeviceAuthorization.verification_uri field is currently never read.
Security — opening a server-provided URL — src/lib/auth.ts:435-438
Unlike the browser flow, which open()s a locally-built authUrl, this path passes a server-returned string straight to the OS opener. The platform is trusted over HTTPS so risk is low, but validating the scheme is https: (and ideally that the origin matches platformUrl) before calling open() would prevent a compromised/misconfigured response from launching an unexpected handler.
Software engineering — orchestration is untested — src/lib/auth.ts:411-478
requestDeviceAuthorization and pollForDeviceTokens are well covered, but performDeviceLogin itself (credential save, profile-fetch fallback, best-effort browser open) has no test. A single test asserting saveCredentials is called and the profile-fetch-failure branch still returns creds would guard the "logged in with blank identity" fallback path it shares with the browser flow.
Information
- First-poll latency —
src/lib/auth.ts:362-366: the loop sleepsinterval(default 5s) before the first poll, so even an instant approval waits one interval. This is intentional and explicitly tested; noting only for awareness. --jsonon a TTY —performDeviceLoginbranches onisInteractive(TTY-based) independent of thejsonflag, sologin --device --jsonon a TTY still emits clack spinner/log lines that can interleave with the final JSON stdout line. This exactly matches the existingperformOAuthLogin/loginWithOAuthbehavior, so it's consistent — flagging only for completeness.- Performance: no concerns. Single bounded polling loop (interval floor of 1s, hard local deadline from
expires_in), no N+1, no unbounded loop, no blocking work on the event loop. The deliberate non-unref'd timer is well-justified in the comment.
Verdict
approved (informational — human approval is still a separate action). Zero Critical findings; the Suggestions above are worth a look before merge but none block. Reminder that this is a draft and per the PR body it depends on backend #733 landing first for real e2e.
…g state Agent sandboxes (e.g. the ChatGPT app) can kill the polling process before the user approves. Persist the in-flight device code to ~/.insforge/pending-device.json (0600) and have the next `login --device` resume polling the SAME code instead of minting a new one — if the user approved while no poller was alive, the rerun completes immediately. Cleared on success/denial/expiry and on logout; kept on transient failures. Ignored when nearly expired or for a different server/client. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GcszudKMZBSm5RMP14h7fu
jwfing
left a comment
jwfing
left a comment
There was a problem hiding this comment.
Review: feat(login): --device login via RFC 8628 device flow
Summary: Well-scoped, cleanly-mirrored implementation of the RFC 8628 device authorization grant with a thoughtful resume capability; no blocking issues found.
Requirements context
No matching spec/plan found under docs/specs/ (only diagnose-* and db-migrations-* designs exist there; the repo has no docs/superpowers/). Assessed against the PR description and RFC 8628. The grant type (urn:ietf:params:oauth:grant-type:device_code), the authorization_pending / slow_down (+5s) / access_denied / expired_token handling, and the optional-interval default all match RFC 8628 §3.4–§3.5. Verified locally: npm ci → the two new test files pass (14/14) and eslint is clean on all changed files.
Findings
Critical
(none) — no correctness bug, security hole, data-loss risk, or break to the existing browser/email/api-key login paths. The new flow is additive and the default browser flow is untouched.
Suggestion
- Functionality / UX — resume is not actually "immediate".
pollForDeviceTokenssleeps at the top of the loop (src/lib/auth.ts:334-347) before the first fetch, andintervalMsis floored at 1s (Math.max(params.interval || 5, 1),src/lib/auth.ts:330). So a resumed attempt where the user already approved redeems after one interval (default 5s), not "immediately" as the type-doc (src/types.ts) and stderr message claim. Consider a single poll before the first sleep so an already-approved resume completes on the first tick. - Software engineering — new test suite runs ~22s of real wall-clock. The 1s interval floor overrides the
interval: 0.01used throughoutauth.device.test.ts/auth.device.resume.test.ts, so those cases can't exercise the intended sub-second cadence and spend real time sleeping. Thedefaults to a 5s intervaltest already demonstrates the fix —vi.useFakeTimers()+advanceTimersByTimeAsync. Adopting fake timers (or injecting the sleep fn) for the polling-loop tests would keep the suite fast and make timing assertions deterministic.
Information
- Response shape is not runtime-validated.
requestDeviceAuthorizationreturnsawait res.json() as DeviceAuthorization(src/lib/auth.ts:290) andgetPendingDeviceLoginreturnsJSON.parse(...)cast toPendingDeviceLogin(src/lib/config.ts:57-62) with no field checks. This degrades safely —getResumableDeviceLogincatches parse throws, and a badexpires_atyieldsNaN, failing the> 60_000guard so it falls through to a fresh attempt — so it's informational, not a defect. --device --jsonin a TTY may interleave output.performDeviceLogingates its spinner/log onisInteractive(TTY), not thejsonflag, sologin --device --jsonfrom a terminal would write clack output to stdout alongside the final JSON. This is consistent with the existingperformOAuthLogin(same pattern), so it's a pre-existing behavior the new flow inherits rather than a regression.- Only
verification_uri_completeis displayed. RFC 8628 §3.3 treats the plainverification_uri+user_codeas the canonical fallback display; the flow relies solely on the_completeform. Fine as long as the backend always returns it. - Security review:
device_codeis correctly treated as a bearer secret — persisted at0600, never logged/printed, and cleared on success/denial/expiry (and onclearCredentials), mirroringcredentials.jsonhandling. No new dependencies, no SQL/shell, HTTP bodies are parameterized JSON to fixed endpoints. Nothing security-blocking. - Performance review: polling is bounded by the local deadline, sleeps between polls, and network errors
continuewithout hammering; the intentional non-unref'd timer is documented. No N+1, unbounded loops, or blocking work introduced. - Draft PR that depends on backend #733 (endpoints/grant) and dashboard #590; e2e is reasonably deferred until those land. Fails fast with a clear message when the grant isn't enabled (
unauthorized_client).
Verdict
approved (informational — the human still gives the explicit GitHub approval). Zero Critical findings; the Suggestions and Information items are non-blocking. Nice work on the resume-after-kill design and the RFC-faithful state machine.
Replaces the paste-back (--no-browser/--callback-url) guidance per Tony's call — device flow only: agent runs `login --device`, relays the pre-filled link + code, user clicks Authorize, CLI polls to completion; rerun resumes the same pending code after sandbox timeouts. Retrieval-tested: an agent given only this doc chose the right command, relayed the link, handled the killed-process resume, and knew no paste-back is needed. Companion to InsForge/CLI#199 + insforge-cloud-backend#733; requires the CLI release that ships --device. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GcszudKMZBSm5RMP14h7fu
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
jwfing
left a comment
jwfing
left a comment
There was a problem hiding this comment.
Review: feat(login): --device login via RFC 8628 device flow
Summary: Adds insforge login --device implementing the OAuth 2.0 device authorization grant (RFC 8628) with a resumable pending-code persistence layer; the implementation is careful, closely mirrors the existing OAuth flow, and is well tested — no blocking issues found.
Requirements context
No matching spec/plan found — docs/specs/ contains only the diagnose-command and db-migrations designs, and there is no docs/superpowers/ directory. Assessed against the PR description, RFC 8628, the repo's DEVELOPMENT.md, and the existing performOAuthLogin implementation as the source of intent. Repo conventions (ESM .js imports, CLIError + top-level handleError, formatFetchError, 0600 credential files, type-sanitized PostHog telemetry) are all respected. The new method: 'oauth_device' string flows through sanitizeTelemetry (which filters by type, not an enum allow-list), so it needs no telemetry-side change.
Findings
Critical
(none)
Suggestion
Functionality
-
pollForDeviceTokenssleeps before its first request (src/lib/auth.ts:369-373). For a fresh login that's ideal (avoids a guaranteedauthorization_pendinground-trip), but on resume it contradicts the documented behavior —performDeviceLogin's comment (src/lib/auth.ts:435-437) and the stderr message ("If they already approved, this completes immediately") promise immediate redemption, yet a resumed, already-approved code still waits a full interval (≥1s, up to 5s) before redeeming. Consider polling once before the first sleep when resuming. -
The poll loop tolerates fetch-level network errors (
catch { continue },src/lib/auth.ts:388-391) but a token-endpoint HTTP 5xx falls through to thedefault:case (src/lib/auth.ts:406-408) and throws, aborting the whole attempt. So a transient network blip is survived, but a transient server-side blip ends the run (the user must re-invoke to resume). Treating 5xx (and unknownerrorwith no body) as retryable-within-the-loop would make the two transient-failure paths consistent. Thekeeps pending state on transient poll failuretest (auth.device.resume.test.ts:172-177) shows this is a deliberate "abort-but-keep-state" choice, so it's non-blocking — but the asymmetry is worth reconsidering.
Security
await open(device.verification_uri_complete)(src/lib/auth.ts:483-484) hands a fully server-provided string to the OS URL opener. Trust in the platform makes this low-risk (andopenspawns without a shell), but unlike the browser flow — which opens a locally-builtbuildAuthorizeUrl— this URL is entirely controlled by thedevice_authorizationresponse. Validating that it parses as anhttps:URL before opening would harden it against a misconfigured/spoofed--api-url.
Software engineering / process
DEVELOPMENT.md §3asks that a new user-facing flag be reflected in theinsforge-cliskill inInsForge/agent-skillsin the same change set, cross-referenced from this PR. The PR defers this ("skill docs update follows the release"). Non-blocking, but please track it so agents learn about--device.
Information
-
Both the request and poll paths use only
verification_uri_complete; if the server ever omits it, the printed instruction becomesundefined(src/lib/auth.ts:469-471). RFC 8628 §3.3.1 suggests falling back toverification_uri+user_code. TheDeviceAuthorizationtype marks it required, so this is only a defensive note. -
intervalMs = Math.max(params.interval || 5, 1) * 1000(src/lib/auth.ts:367) clamps to a 1s floor. The unit tests passinterval: 0.01expecting sub-second polling, but they actually run at the 1s floor — they still pass (within Vitest timeouts) but not as fast as the literal implies. Harmless; noting for accuracy. -
The pre-existing
--client-id <id>option is not wired into any login path (all readconfig.oauth_client_id ?? DEFAULT_CLIENT_ID); the device flow follows that same pattern. Out of scope for this PR — flagging only for awareness.
Positives worth calling out: the device_code is treated as a bearer secret — persisted 0600, never printed (only the non-secret user_code is shown), and cleared on success/denial/expiry and on logout (clearCredentials now also clears pending state, src/lib/config.ts). Test coverage is strong: request/poll state machine, slow_down backoff, denial, server- and local-deadline expiry, transient network faults, default-interval hammer-loop guard, plus a full resume lifecycle (fresh→persist→clear, resume-same-code, ignore near-expiry/other-server, clear-on-denial, keep-on-transient). Non-TTY output correctly goes to stderr to keep --json stdout clean.
Verdict
approved — zero Critical findings. The Suggestions and Information items are non-blocking; the human reviewer still gives the explicit GitHub approval. Note the two depends-on backend/dashboard PRs (#733, insforge-cloud#590) must land before this is usable end-to-end.
Greptile SummaryThis PR adds
Confidence Score: 3/5Safe to merge once the empty-user credential issue is addressed; the rest of the changes are well-structured and tested. The core flow and resume logic are correct and well-tested, but performDeviceLogin saves credentials with a placeholder empty user before the profile fetch and silently swallows profile errors, meaning --json consumers can get { success: true, user: { id: '', email: '' } } — breaking the contract agent integrations rely on. src/lib/auth.ts — specifically the two-phase credential save around the getProfile call in performDeviceLogin Important Files Changed
Reviews (1): Last reviewed commit: "chore: bump version to 0.2.0" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const creds: StoredCredentials = { | ||
| access_token: tokens.access_token, | ||
| refresh_token: tokens.refresh_token, | ||
| user: { id: '', name: '', email: '', avatar_url: null, email_verified: true }, | ||
| }; | ||
| saveCredentials(creds); | ||
|
|
||
| try { | ||
| const profile = await getProfile(apiUrl); | ||
| creds.user = profile; | ||
| saveCredentials(creds); | ||
| s?.stop(`Authenticated as ${profile.email}`); | ||
| if (!isInteractive) process.stderr.write(`Authenticated as ${profile.email}\n`); | ||
| } catch { | ||
| s?.stop('Authenticated successfully'); | ||
| if (!isInteractive) process.stderr.write('Authenticated successfully\n'); | ||
| } | ||
|
|
||
| return creds; |
There was a problem hiding this comment.
getProfile failure
The code saves credentials with a placeholder user (id: '', email: '', name: '') before fetching the profile, then silently swallows any getProfile error. When getProfile fails (network blip, 5xx, rate-limit), performDeviceLogin returns and loginWithDevice outputs { success: true, user: { id: '', email: '', name: '' } } in JSON mode. Agent integrations that check user.id or user.email to verify the login succeeded will get empty strings despite success: true, breaking the contract the --json flag is meant to provide.
| export interface DeviceAuthorization { | ||
| device_code: string; | ||
| user_code: string; | ||
| verification_uri: string; | ||
| verification_uri_complete: string; | ||
| expires_in: number; | ||
| interval: number; | ||
| } |
There was a problem hiding this comment.
DeviceAuthorization.interval is typed as a required number, but the code immediately falls back with fresh.interval ?? 5, and the pollForDeviceTokens parameter is interval?: number. RFC 8628 §3.2 explicitly makes interval optional in the server response. Marking it optional aligns the type with reality.
| export interface DeviceAuthorization { | |
| device_code: string; | |
| user_code: string; | |
| verification_uri: string; | |
| verification_uri_complete: string; | |
| expires_in: number; | |
| interval: number; | |
| } | |
| export interface DeviceAuthorization { | |
| device_code: string; | |
| user_code: string; | |
| verification_uri: string; | |
| verification_uri_complete: string; | |
| expires_in: number; | |
| /** RFC 8628 §3.2: OPTIONAL in the server response; defaults to 5 s when absent. */ | |
| interval?: number; | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| switch (err.error) { | ||
| case 'authorization_pending': | ||
| continue; | ||
| case 'slow_down': | ||
| intervalMs += 5000; | ||
| continue; |
There was a problem hiding this comment.
slow_down backoff can prevent polling before deadline
Each slow_down response adds 5 s to intervalMs with no upper cap. After N consecutive slow_down responses, if the accumulated interval exceeds the remaining time before deadline, the setTimeout resolves after the deadline has already passed, the while-loop condition fails, and the function throws "device code expired" even though the user may have approved. Consider capping with Math.min(intervalMs + 5000, 60_000) so backoff stays bounded.
jwfing
left a comment
jwfing
left a comment
There was a problem hiding this comment.
Review: feat(login): --device login via RFC 8628 device flow
Summary: A clean, well-tested implementation of the OAuth 2.0 device authorization grant that follows the existing auth.ts/login.ts conventions closely and correctly leaves the default browser flow untouched — no Critical findings.
Requirements context
No matching spec/plan found. docs/superpowers/ does not exist in this repo; the spec directory is docs/specs/, which contains only diagnose and db-migrations design docs — nothing covering login/device flow. Assessed against the PR description, RFC 8628, and the existing OAuth implementation in src/lib/auth.ts.
Critical
(none)
Suggestion
Functionality — reliance on optional verification_uri_complete (src/lib/auth.ts:315-318,458,470,477,484). RFC 8628 §3.2 makes verification_uri_complete OPTIONAL; only verification_uri + user_code are guaranteed. The interface declares it required and every display/open() path uses it exclusively. If the backend ever omits it (or an older server responds), the user sees Open undefined and the browser-open targets undefined. Since this PR explicitly depends on backend #733 which presumably always returns it, this is non-blocking — but a fallback to verification_uri + separately-displayed user_code would harden it against contract drift.
Functionality — HTTP-level 5xx aborts the whole poll loop (src/lib/auth.ts:395-408). The PR body states "transient network errors → keep polling," but that resilience only covers fetch throwing (src/lib/auth.ts:386-389). A transient HTTP 5xx / gateway error that returns non-JSON falls through to the default case and throws Token polling failed (HTTP 5xx), ending the login. The device code stays valid so a rerun resumes, but consider treating 5xx (and unknown non-terminal errors) as continue-and-retry to match the stated intent, reserving hard failure for access_denied/expired_token.
Software engineering — poll sleeps before the first request, so "completes immediately" isn't (src/lib/auth.ts:369-373, message at src/lib/auth.ts:477). pollForDeviceTokens awaits intervalMs before the first fetch. On a resumed attempt where the user already approved, the stderr message promises "If they already approved, this completes immediately," but the first redemption is delayed by up to interval (≥1s, default 5s). Polling once immediately, then sleeping between subsequent polls, would honor the message and is RFC-compliant.
Functionality — no shape validation on persisted pending state (src/lib/config.ts:60-66, src/lib/auth.ts:421-436). getPendingDeviceLogin casts JSON.parse(...) straight to PendingDeviceLogin. getResumableDeviceLogin's guard only checks platform_url, client_id, and expires_at; a syntactically-valid but truncated file missing device_code would pass the resumable check and poll with device_code: undefined. Low blast radius (self-authored file), but a minimal field check before trusting the resume path would be safer. (The try/catch correctly handles corrupt JSON.)
Information
- Unused / mismatched interface fields (
src/lib/auth.ts:312-319).DeviceAuthorization.verification_uriis declared but never read, andintervalis typed as required yet handled as optional (fresh.interval ?? 5atsrc/lib/auth.ts:459). RFC 8628 makesintervaloptional — marking itinterval?: numberwould match reality. Math.max(params.interval || 5, 1)floors the interval at 1s (src/lib/auth.ts:367). Sensible anti-hammer floor; just note it means the sub-secondinterval: 0.01used in tests effectively polls at 1s in production, which is fine.- Telemetry method value
oauth_device(src/commands/login.ts:23-29). Consistent with the existingoauth/email/user_api_keyvalues on the sametrackTopLevelUsage('login', …)call — good, follows the established login-command pattern.
Dimension coverage
- Software engineering — Strong. New behavior is covered by 12 focused unit tests across two files: request success +
unauthorized_client, poll pending→success,slow_downbackoff, denial, server-side expiry, local-deadline expiry, default-interval (fake timers, asserts no zero-delay hammer loop), transient network fault, and the full resume lifecycle (persist-while-polling, clear-on-success, resume-same-code, ignore near-expiry/other-server, clear-on-denial, retain-on-transient). Import style, error-handling shape, andisInteractive/stderr handling all mirrorperformOAuthLogin. (I could not execute the suite in the review sandbox — assessed by inspection.) - Functionality — Solves the stated problem (browserless/loopback-unreachable login). RFC 8628 §3.5 state machine is correct. Gaps noted above are edge/robustness, not core-path.
- Security — No new secrets logged or returned;
device_code(the bearer secret for the attempt) is persisted0o600, matchingcredentials.json, and cleared on success/denial/expiry and on logout (clearCredentials→clearPendingDeviceLogin,src/lib/config.ts:75-80). No new user input reaches SQL/shell. No auth checks weakened. No new dependencies (openalready used by the browser flow). Good. - Performance — Poll loop is bounded by the local deadline and gated by
intervalMs; no busy loop (verified by the fake-timer test). Deliberate non-unref()of the poll timer is documented and correct for this flow. No hot-path concerns.
Verdict
approved (informational — zero Critical findings; the four Suggestions and Information notes are non-blocking). A human still provides the explicit GitHub approval.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
src/lib/auth.device.test.ts (1)
107-127: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy liftAdd a process-liveness regression test for the referenced timer.
Fake timers validate the default interval but cannot detect the prior failure mode: Node exiting when the polling timeout is
unref()’d. Add a child-process/integration regression that verifies a pending poll keeps the process alive until it resolves or expires.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/auth.device.test.ts` around lines 107 - 127, Add a child-process or integration regression test alongside the existing pollForDeviceTokens timer tests that starts a pending poll and verifies the process remains alive until the poll resolves or expires. Use real timers and a scenario exercising the referenced polling timeout, ensuring the test would fail if that timer is unref()’d while preserving the existing fake-timer interval test.src/lib/auth.device.resume.test.ts (1)
147-169: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winActually test server and client mismatches.
This test’s title claims another-server coverage, but only changes
expires_at. Add cases overridingplatform_urlandclient_id, asserting that each mints a fresh code rather than polling the persisted device code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/auth.device.resume.test.ts` around lines 147 - 169, Expand the test around performDeviceLogin to cover pending state mismatches for both platform_url and client_id, overriding each value in separate cases while retaining valid expiration. Assert each mismatch mints a fresh device code and does not poll the persisted device code; keep the existing nearly-expired case intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/login.ts`:
- Around line 18-37: Validate the authentication options in the login command
before selecting a flow: reject when more than one of opts.userApiKey,
opts.email, and opts.device is provided, report the conflicting flags, and stop
execution without invoking any login function. Preserve the existing
single-option method selection and login behavior.
- Around line 18-37: Thread opts.clientId through the device-login branch into
loginWithDevice and then performDeviceLogin, updating both function signatures
and their call sites. Use the forwarded client ID when creating or resuming
device authorization, and validate pending login state against that value
instead of the global/default client ID.
- Around line 121-126: Update loginWithDevice to forward its json flag into
performDeviceLogin, using the existing output-mode parameter or adding one if
needed. Ensure JSON-mode progress output is routed to stderr rather than stdout,
while preserving the current interactive output behavior for non-JSON logins.
In `@src/lib/auth.device.test.ts`:
- Around line 74-77: Update the request-body assertions in the device-token test
for pollForDeviceTokens to also verify that body.client_id matches the expected
client identifier, alongside the existing grant_type and device_code checks.
- Around line 79-90: Update the “backs off on slow_down and still completes”
test to use fake timers and explicitly verify that the second fetch is not
triggered before the additional 5-second slow_down delay, then advance the
timers and assert the successful token response. Preserve the existing response
sequence and completion assertions while ensuring the test fails if slow_down is
ignored.
In `@src/lib/auth.ts`:
- Around line 369-389: Update the polling loop around the tokenUrl fetch to
enforce a per-request AbortController timeout bounded by the remaining deadline,
passing its signal to fetch and stopping or continuing appropriately when
aborted. Track transport failures in this loop and increase the next polling
delay with backoff as required by RFC 8628 §3.5, while preserving normal polling
and deadline behavior.
- Around line 395-408: Update performDeviceLogin’s terminal-error cleanup to
identify denied, expired, redeemed, and other non-retryable OAuth failures by
the parsed err.error code rather than matching the thrown message text. Ensure
pending-device.json is removed for every terminal token error while preserving
retry behavior for authorization_pending and slow_down, and add coverage for a
redeemed or other terminal error.
---
Nitpick comments:
In `@src/lib/auth.device.resume.test.ts`:
- Around line 147-169: Expand the test around performDeviceLogin to cover
pending state mismatches for both platform_url and client_id, overriding each
value in separate cases while retaining valid expiration. Assert each mismatch
mints a fresh device code and does not poll the persisted device code; keep the
existing nearly-expired case intact.
In `@src/lib/auth.device.test.ts`:
- Around line 107-127: Add a child-process or integration regression test
alongside the existing pollForDeviceTokens timer tests that starts a pending
poll and verifies the process remains alive until the poll resolves or expires.
Use real timers and a scenario exercising the referenced polling timeout,
ensuring the test would fail if that timer is unref()’d while preserving the
existing fake-timer interval test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 938c8527-c19a-49f1-9023-c5dad08b2f87
📒 Files selected for processing (7)
package.jsonsrc/commands/login.tssrc/lib/auth.device.resume.test.tssrc/lib/auth.device.test.tssrc/lib/auth.tssrc/lib/config.tssrc/types.ts
| .option('--device', 'Device login: approve a short code on the dashboard while the CLI polls — for sandboxes/SSH/containers where the browser cannot reach this process') | ||
| .action(async (opts, cmd) => { | ||
| const { json, apiUrl } = getRootOpts(cmd); | ||
| // Which auth path was taken — user_api_key logins are the signal the | ||
| // dashboard's connect-agent onboarding funnel is measured by. | ||
| const method = opts.userApiKey ? 'user_api_key' : opts.email ? 'email' : 'oauth'; | ||
| const method = opts.userApiKey | ||
| ? 'user_api_key' | ||
| : opts.email | ||
| ? 'email' | ||
| : opts.device | ||
| ? 'oauth_device' | ||
| : 'oauth'; | ||
|
|
||
| try { | ||
| if (opts.userApiKey) { | ||
| await loginWithUserApiKey(opts.userApiKey, json, apiUrl); | ||
| } else if (opts.email) { | ||
| await loginWithEmail(json, apiUrl); | ||
| } else if (opts.device) { | ||
| await loginWithDevice(json, apiUrl); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject conflicting authentication flags.
--device is silently ignored when combined with --email or --user-api-key, because the earlier branch wins. Reject multiple auth-mode flags instead of unexpectedly launching a different login flow.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/login.ts` around lines 18 - 37, Validate the authentication
options in the login command before selecting a flow: reject when more than one
of opts.userApiKey, opts.email, and opts.device is provided, report the
conflicting flags, and stop execution without invoking any login function.
Preserve the existing single-option method selection and login behavior.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Forward --client-id into device authorization.
The device branch only passes apiUrl; performDeviceLogin therefore uses the global/default client ID instead of opts.clientId. insforge login --device --client-id ... can mint or resume a device code for the wrong client. Thread the value through loginWithDevice and performDeviceLogin, including pending-login validation.
Suggested wiring
- await loginWithDevice(json, apiUrl);
+ await loginWithDevice(json, apiUrl, opts.clientId);Update both function signatures and the pending-state client check accordingly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/login.ts` around lines 18 - 37, Thread opts.clientId through the
device-login branch into loginWithDevice and then performDeviceLogin, updating
both function signatures and their call sites. Use the forwarded client ID when
creating or resuming device authorization, and validate pending login state
against that value instead of the global/default client ID.
| const creds = await performDeviceLogin(apiUrl); | ||
|
|
||
| if (!json) { | ||
| clack.outro('Done'); | ||
| } else { | ||
| console.log(JSON.stringify({ success: true, user: creds.user })); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
# First, explore the file structure to understand the codebase
fd -type f -name "*.ts" | grep -E "(login|device)" | head -20Repository: InsForge/CLI
Length of output: 321
🏁 Script executed:
# Read the login.ts file, focusing on lines 121–126 and surrounding context
cat -n src/commands/login.ts | sed -n '100,140p'Repository: InsForge/CLI
Length of output: 1601
🏁 Script executed:
# Find the performDeviceLogin function definition
rg -A 20 "function performDeviceLogin|const performDeviceLogin|export.*performDeviceLogin" --type tsRepository: InsForge/CLI
Length of output: 150
🏁 Script executed:
# Search for performDeviceLogin more broadly in the codebase
rg "performDeviceLogin" --type ts -B 2 -A 15Repository: InsForge/CLI
Length of output: 150
🏁 Script executed:
# List all TypeScript files to understand the structure
find . -name "*.ts" | head -30Repository: InsForge/CLI
Length of output: 959
🏁 Script executed:
# Search for performDeviceLogin without type filtering
rg "performDeviceLogin" -B 2 -A 15Repository: InsForge/CLI
Length of output: 150
🏁 Script executed:
# Check the imports and top of login.ts
cat -n src/commands/login.ts | head -50Repository: InsForge/CLI
Length of output: 2670
🏁 Script executed:
# Find the auth.js file and read the performDeviceLogin function
find . -name "auth.js" -o -name "auth.ts"Repository: InsForge/CLI
Length of output: 168
🏁 Script executed:
# Read the auth file containing performDeviceLogin
cat -n src/lib/auth.ts 2>/dev/null || cat -n src/lib/auth.js 2>/dev/nullRepository: InsForge/CLI
Length of output: 22541
Pass json flag to performDeviceLogin to prevent progress output from corrupting JSON.
loginWithDevice receives the json parameter but does not forward it to performDeviceLogin, which only checks isInteractive (TTY detection) to decide between Clack output and stderr. In a TTY with --json, the spinner and progress logs will be written to stdout before the success JSON object, breaking machine-readable output. Forward the json flag to performDeviceLogin (or a new parameter controlling output mode) so it routes progress messages to stderr when JSON output is requested.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/login.ts` around lines 121 - 126, Update loginWithDevice to
forward its json flag into performDeviceLogin, using the existing output-mode
parameter or adding one if needed. Ensure JSON-mode progress output is routed to
stderr rather than stdout, while preserving the current interactive output
behavior for non-JSON logins.
| const body = JSON.parse((fetchMock.mock.calls[0][1] as RequestInit).body as string); | ||
| expect(body.grant_type).toBe('urn:ietf:params:oauth:grant-type:device_code'); | ||
| expect(body.device_code).toBe('dvc_abc'); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files ==\n'
git ls-files 'src/lib/auth.device.*' 'src/lib/*auth*' | sed 's#^`#-` #'
printf '\n== outline auth.device.ts ==\n'
ast-grep outline src/lib/auth.device.ts --view expanded || true
printf '\n== outline auth.device.test.ts ==\n'
ast-grep outline src/lib/auth.device.test.ts --view expanded || true
printf '\n== relevant snippets ==\n'
sed -n '1,240p' src/lib/auth.device.ts
printf '\n--- test ---\n'
sed -n '1,220p' src/lib/auth.device.test.tsRepository: InsForge/CLI
Length of output: 622
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== outline auth.ts ==\n'
ast-grep outline src/lib/auth.ts --view expanded || true
printf '\n== auth.ts relevant ==\n'
rg -n "pollForDeviceTokens|device_code|client_id|grant_type" src/lib/auth.ts src/lib/auth.device.test.ts src/lib/auth.device.resume.test.ts -n -A 4 -B 4 || true
printf '\n== auth.ts snippet ==\n'
sed -n '1,260p' src/lib/auth.ts
printf '\n== auth.device.test.ts snippet ==\n'
sed -n '1,260p' src/lib/auth.device.test.ts
printf '\n== auth.device.resume.test.ts snippet ==\n'
sed -n '1,260p' src/lib/auth.device.resume.test.tsRepository: InsForge/CLI
Length of output: 42028
Assert client_id in the device-token request.
pollForDeviceTokens sends client_id along with grant_type and device_code; this test should cover that so a regression dropping the field doesn't slip through.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/auth.device.test.ts` around lines 74 - 77, Update the request-body
assertions in the device-token test for pollForDeviceTokens to also verify that
body.client_id matches the expected client identifier, alongside the existing
grant_type and device_code checks.
| it('backs off on slow_down and still completes', async () => { | ||
| const responses = [ | ||
| jsonResponse({ error: 'slow_down' }, 400), | ||
| jsonResponse({ access_token: 'at', refresh_token: 'rt', expires_in: 3600 }), | ||
| ]; | ||
| vi.stubGlobal('fetch', vi.fn(async () => responses.shift()!)); | ||
|
|
||
| // slow_down adds 5s; keep the test fast by capping expiresIn so failure | ||
| // would surface as an expiry error if backoff logic breaks the loop. | ||
| const tokens = await pollForDeviceTokens({ ...params, expiresIn: 30 }); | ||
| expect(tokens.access_token).toBe('at'); | ||
| }, 15_000); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the test and the implementation it exercises.
git ls-files src/lib/auth.device.test.ts src/lib/auth.device.ts src/lib/auth.device.* src/lib/*device* | sed 's#^`#FILE` #'
echo '--- src/lib/auth.device.test.ts ---'
cat -n src/lib/auth.device.test.ts | sed -n '1,220p'
echo '--- candidate implementation files ---'
for f in src/lib/auth.device.ts src/lib/auth.device.js src/lib/auth.device.mts src/lib/auth.device.cts; do
if [ -f "$f" ]; then
echo "--- $f ---"
cat -n "$f" | sed -n '1,260p'
fi
doneRepository: InsForge/CLI
Length of output: 6628
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the polling implementation and any slow_down-specific logic.
rg -n "pollForDeviceTokens|slow_down|authorization_pending|expired_token|device_code" src test . --glob '!**/node_modules/**'Repository: InsForge/CLI
Length of output: 7835
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the polling implementation around pollForDeviceTokens.
sed -n '340,430p' src/lib/auth.ts | cat -nRepository: InsForge/CLI
Length of output: 4330
Assert the slow_down backoff here
This still only checks eventual success, so it passes even if slow_down is ignored. Use fake timers and assert the second poll doesn’t happen until the extra 5s elapses.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/auth.device.test.ts` around lines 79 - 90, Update the “backs off on
slow_down and still completes” test to use fake timers and explicitly verify
that the second fetch is not triggered before the additional 5-second slow_down
delay, then advance the timers and assert the successful token response.
Preserve the existing response sequence and completion assertions while ensuring
the test fails if slow_down is ignored.
| while (Date.now() < deadline) { | ||
| // Deliberately NOT unref'd: this timer is often the only thing on the | ||
| // event loop (no callback server in this flow), and unref'ing it lets | ||
| // the process exit mid-poll with an unsettled top-level await. | ||
| await new Promise((resolve) => setTimeout(resolve, intervalMs)); | ||
|
|
||
| let res: Response; | ||
| try { | ||
| res = await fetch(tokenUrl, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ | ||
| grant_type: DEVICE_CODE_GRANT, | ||
| device_code: params.deviceCode, | ||
| client_id: params.clientId, | ||
| }), | ||
| }); | ||
| } catch { | ||
| // Transient network error mid-poll — keep polling until the deadline. | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file structure first
ast-grep outline src/lib/auth.ts --view expanded
# Read the relevant section with line numbers
sed -n '300,430p' src/lib/auth.ts | cat -nRepository: InsForge/CLI
Length of output: 7445
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any timeout/abort handling tied to the OAuth/device flow
rg -n --no-heading -S "AbortController|AbortSignal|timeout|setTimeout\\(|slow_down|authorization_pending|expired_token|pollForDeviceTokens|requestDeviceAuthorization|performDeviceLogin" src/lib/auth.ts src || true
# Read the caller around device login to see whether a higher-level timeout exists
sed -n '430,520p' src/lib/auth.ts | cat -nRepository: InsForge/CLI
Length of output: 15886
🌐 Web query:
RFC 8628 section 3.5 connection timeout polling frequency slow_down network error
💡 Result:
In RFC 8628 section 3.5, the requirements for polling frequency in response to error conditions are distinct for "slow_down" errors and general network connection timeouts [1][2]. Slow_down Error Handling When a client receives a "slow_down" error, it must continue polling, but it is required to increase the polling interval by at least 5 seconds for that request and all subsequent requests [1][3]. This is a mandatory protocol requirement designed to reduce the rate of requests to the token endpoint [4]. Connection Timeout Handling In the event of a connection timeout, the specification imposes a different requirement: 1. Clients MUST unilaterally reduce their polling frequency before retrying [1][5]. 2. The use of an exponential backoff algorithm is recommended (e.g., doubling the polling interval on each such timeout) [3][2]. While both mechanisms serve to throttle requests, the "slow_down" error is a server-directed signal to adjust the interval by a fixed increment, whereas connection timeout handling is a client-side recovery strategy for transport-level issues [1][2].
Citations:
- 1: https://www.rfc-editor.org/rfc/rfc8628.txt
- 2: https://datatracker.ietf.org/doc/rfc8628/
- 3: https://www.rfc-editor.org/info/rfc8628/
- 4: fix(providers): increase slow_down back-off to 5 s per RFC 8628 openclaw/openclaw#22656
- 5: https://www.rfc-editor.org/rfc/rfc8628.html
Bound each poll request and back off transport failures.
fetch() can stall past the deadline, and connection errors retry at the same interval. Add a per-request abort tied to the remaining time, and increase the delay after network timeouts per RFC 8628 §3.5.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/auth.ts` around lines 369 - 389, Update the polling loop around the
tokenUrl fetch to enforce a per-request AbortController timeout bounded by the
remaining deadline, passing its signal to fetch and stopping or continuing
appropriately when aborted. Track transport failures in this loop and increase
the next polling delay with backoff as required by RFC 8628 §3.5, while
preserving normal polling and deadline behavior.
| const err = await res.json().catch(() => ({})) as { error?: string }; | ||
| switch (err.error) { | ||
| case 'authorization_pending': | ||
| continue; | ||
| case 'slow_down': | ||
| intervalMs += 5000; | ||
| continue; | ||
| case 'access_denied': | ||
| throw new Error('Login request was denied in the dashboard.'); | ||
| case 'expired_token': | ||
| throw new Error('The device code expired before the login was approved. Run `insforge login --device` again.'); | ||
| default: | ||
| throw new Error(err.error ?? `Token polling failed (HTTP ${res.status})`); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== auth.ts around polling ==\n'
nl -ba src/lib/auth.ts | sed -n '330,460p'
printf '\n== search pending-state cleanup ==\n'
rg -n "pending state|pendingState|device code|authorization_pending|slow_down|access_denied|expired_token|single-use|redeemed|resumable|local expiry" src test testsRepository: InsForge/CLI
Length of output: 220
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== auth.ts around polling ==\n'
sed -n '330,460p' src/lib/auth.ts | cat -n
printf '\n== search pending-state cleanup ==\n'
rg -n "pending state|pendingState|device code|authorization_pending|slow_down|access_denied|expired_token|single-use|redeemed|resumable|local expiry" src test testsRepository: InsForge/CLI
Length of output: 8900
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== auth.ts performDeviceLogin slice ==\n'
sed -n '114,260p' src/lib/auth.ts | cat -n
printf '\n== auth.device.resume.test.ts slice ==\n'
sed -n '1,260p' src/lib/auth.device.resume.test.ts | cat -n
printf '\n== auth.device.test.ts slice ==\n'
sed -n '1,180p' src/lib/auth.device.test.ts | cat -nRepository: InsForge/CLI
Length of output: 21016
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== clearPendingDeviceLogin references ==\n'
rg -n "clearPendingDeviceLogin|getPendingDeviceLogin|savePendingDeviceLogin|deletePendingDeviceLogin|pending device" src/lib/auth.ts src/lib/config.ts src/lib/auth.device.resume.test.ts
printf '\n== auth.ts later slice ==\n'
sed -n '260,420p' src/lib/auth.ts | cat -n
printf '\n== config.ts pending helpers ==\n'
sed -n '1,260p' src/lib/config.ts | cat -nRepository: InsForge/CLI
Length of output: 16362
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== auth.ts around performDeviceLogin cleanup ==\n'
sed -n '420,550p' src/lib/auth.ts | cat -n
printf '\n== auth.device.resume.test.ts around resume/error cases ==\n'
sed -n '160,230p' src/lib/auth.device.resume.test.ts | cat -nRepository: InsForge/CLI
Length of output: 6279
Clear pending state for all terminal token errors.
performDeviceLogin only deletes pending-device.json when the thrown message matches /denied|expired/i, so other terminal OAuth errors can leave a dead device code resumable until local expiry. Match the OAuth error code instead of the message text, and add coverage for a redeemed/other terminal error path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/auth.ts` around lines 395 - 408, Update performDeviceLogin’s
terminal-error cleanup to identify denied, expired, redeemed, and other
non-retryable OAuth failures by the parsed err.error code rather than matching
the thrown message text. Ensure pending-device.json is removed for every
terminal token error while preserving retry behavior for authorization_pending
and slow_down, and add coverage for a redeemed or other terminal error.
2 participants
Why
In sandboxed environments (ChatGPT app plugin, SSH, containers) browser login hangs forever: the OAuth redirect targets a loopback listener the host browser can never reach. The industry norm (gh, az, Codex
--device-auth, Stripe, Railway) is a device/pairing flow — the user approves a short code in the browser and the CLI polls; nothing is pasted, no callback needed.What
insforge login --device:device_code+user_codefromPOST /oauth/v1/device_authorization.insforge.dev/auth/device?user_code=XXXX-XXXX; the user just clicks Authorize.authorization_pending→ keep waiting,slow_down→ +5s backoff,access_denied/expired_token→ clear errors, transient network errors → keep polling, local deadline atexpires_in).Default browser flow unchanged — on a normal desktop it remains the best UX;
--deviceis for everywhere else. Telemetry method:oauth_device.Depends on
Branched from main, independent of #198 (paste-back) so that PR can ship or be dropped freely.
Verification
🤖 Generated with Claude Code
https://claude.ai/code/session_01GcszudKMZBSm5RMP14h7fu
Note
Add
--devicelogin option to the CLI using RFC 8628 device authorization flow--deviceflag to thelogincommand that triggers the OAuth 2.0 device authorization flow (RFC 8628), allowing login on devices without a browser.requestDeviceAuthorizationPOSTs to/api/oauth/v1/device_authorizationand returns a user code and verification URI;pollForDeviceTokenspolls/api/oauth/v1/tokenwith backoff and error handling forslow_down,access_denied, andexpired_token.performDeviceLoginorchestrates the full flow: displays the user code, attempts to open a browser, polls for tokens, saves credentials, and fetches the user profile.{ success, user }JSON when--jsonis passed; telemetry recordsmethod: oauth_device.Changes since #199 opened
intervalparameter optional inpollForDeviceTokenswith a default of 5 seconds [6f23cb7]onPollcallback parameter frompollForDeviceTokens[6f23cb7]performDeviceLoginfunction [7632f21]configmodule and definedPendingDeviceLogintype [7632f21]auth.device.resume.test.ts[7632f21]0.1.100to0.2.0inpackage.json[edf4407]Macroscope summarized d02189b. (Automatic summaries will resume when PR exits draft mode or review begins).
Summary by cubic
Adds
insforge login --deviceusing the OAuth 2.0 device authorization flow (RFC 8628) for sandboxes, SSH, and containers where loopback browser login fails. Default browser login is unchanged; bumps@insforge/clito 0.2.0.New Features
insforge login --deviceprints a verification URL with a short code, tries to open it in the browser, stores tokens, fetches the user profile, supports--json, and records telemetrymethod: oauth_device./api/oauth/v1/tokenper RFC 8628: waits onauthorization_pending, backs off+5sonslow_down, stops onaccess_denied/expired_token, tolerates transient network errors, respectsexpires_in, defaults to a 5s interval when omitted, and keeps the process alive during polling.~/.insforge/pending-device.jsonand resumes the same code on the next run if it’s for the same server/client and not near expiry; cleared on success/denial/expiry and on logout, retained on transient failures. Unit tests cover resume lifecycle in addition to interval, backoff, denial, expiry, and network faults.Dependencies
Written for commit edf4407. Summary will update on new commits.
Summary by CodeRabbit
New Features
--devicelogin support using a device authorization flow.Bug Fixes
Tests
Chores
0.2.0.