feat: device flow-login (QR / annan enhet) i CLI:n by bergetjakob · Pull Request #90 · berget-ai/cli

bergetjakob · 2026-09-14T09:22:32Z

Vad

Samma device flow som i opencode-pluginen (#39/#40) och pi-provider (#55), nu i CLI:n:

berget code init: prompt Browser vs Device code innan login (browser = oförändrat PKCE-flöde)
berget auth login --device: flagga för skript/CI
src/auth/oauth/device-flow.ts: RFC 8628 — QR i terminalen (halvblock, kvadratiska pixlar, EC 'L'), verification_uri_complete så koden är inbäddad i länken, poll med slow_down-backoff, icke-JSON-svar som retrybara, deadline från expires_in

Tester

8 nya; hela sviten 265 gröna. auth-sync/init-testskript uppdaterade för den nya select-prompten.

Manuellt verifierat

BERGET_AUTH_URL=https://auth.stage.berget.ai ./bin/berget.js auth login --device (QR scannas, inloggning går igenom)
berget code init → välja Device code

- src/auth/oauth/device-flow.ts: device authorization grant against Keycloak — terminal QR (half-block, EC 'L'), verification_uri_complete link, RFC 8628 polling (slow_down backoff, non-JSON gateway responses retryable, deadline from expires_in) - loginInteractive({method}) + berget auth login --device flag - berget code init prompts browser vs device before login - 8 new tests; all 265 green

berget-ai

Review

Summary

This PR adds RFC 8628 device authorization grant login (berget auth login --device, plus a browser/device prompt in the code init wizard) with a terminal QR renderer and poll loop, and adapts existing init/auth-sync tests to the new prompt.

Risk

MEDIUM — the standalone auth login --device path works, but I verified in the vendored @clack/prompts@1.5.1 dist that the spinner repaints every 80 ms with cursor.up() + erase.down() on stdout and does not buffer other output; consequently the QR/link/user-code that startDeviceFlow prints after s.start() inside ensureCliAuth is erased within ~160 ms, making device login unusable in berget code init, and the new module ships with zero tests.

Issues

critical src/commands/code/auth-sync.ts:121 — The QR/link/user-code are printed to stdout while the clack spinner is running; the spinner's 80 ms repaint (erase.down()) wipes everything below its frame, so device mode is unusable through the wizard.
warning src/services/auth-service.ts:77 — loginInteractive still documents "Does NOT print to stdout", but the new device branch prints the whole QR block, breaking that documented contract (which is the direct cause of the spinner conflict).
warning src/auth/oauth/device-flow.ts:8 — Realm is hardcoded (/realms/berget/…) even though AuthConfig.realm exists and issuer.ts composes the realm dynamically; this also bypasses the OIDC discovery (getConfiguration) already used for PKCE, so the two flows can drift to different endpoints.
warning src/auth/oauth/device-flow.ts:141 — https://${new URL(keycloakUrl).host} forces https and drops any path prefix, breaking the BERGET_KEYCLOAK_URL=http://localhost:8080 local-dev override that getAuthConfig() explicitly supports (PKCE handles those via discovery).
warning src/auth/oauth/device-flow.ts:60 — No unit tests for the new logic (extractTokenResult, handlePollError, renderTerminalQrCode, polling loop) despite "Exported for tests" comments and the sibling pkce-flow.test.ts precedent.
nit src/auth/oauth/device-flow.ts:10 — Device scope adds device-email-otp, absent from the PKCE scope; if that custom scope isn't assigned to the berget-code client, the device authorization request fails outright with invalid_scope — confirm it exists on both stage and prod realms.
nit src/auth/oauth/device-flow.ts:217 — The authorization response is only checked via an as cast; a missing expires_in produces a NaN deadline so the poll loop never executes and the user instantly gets "timed out".
nit src/commands/auth.ts:19 — The new --device flag (headlined for SSH/headless) is undocumented in the README's Authentication section.

Suggestions

src/commands/code/auth-sync.ts — Stop the spinner before the device flow prints its instructions (or route the QR through the prompter), e.g.:

if (method === 'device') s.stop('Scan the code to continue:');
const loginResult = await authService.loginInteractive({ debug: ..., method });

src/auth/oauth/device-flow.ts — Build endpoints from config like issuer.ts does (${config.keycloakUrl}/realms/${config.realm}/protocol/openid-connect/auth/device) or read the discovered device_authorization_endpoint, which also fixes the https-forcing URL at line 141.
src/auth/oauth/device-flow.ts — Add unit tests for handlePollError (pending/slow_down/access_denied/expired), extractTokenResult, and the QR renderer.

Architecture

The device flow hand-builds Keycloak endpoint URLs instead of reusing the openid-client discovery stack (getConfiguration), creating a second source of truth for realm/endpoint config, and couples a documented-stdout-free library API (loginInteractive) directly to process.stdout.

Code quality score: 7/10 — clean decomposition into documented, testable pure helpers, but the module is untested, duplicates endpoint/realm construction, and breaks its caller contract by printing to stdout.

Inline findings

_{Berget AI (berget/zai-org/GLM-5.3-Flash) | PR #90}

berget-ai · 2026-09-14T09:26:33Z

+
  const s = prompter.spinner();
-  s.start('Waiting for browser login...');
+  s.start(


🔴 blocker — Device flow prints the QR to stdout while this clack spinner is active; the spinner's 80ms erase.down() repaint wipes the QR/link/user-code, making device login unusable in berget code init.

berget-ai · 2026-09-14T09:26:33Z

@@ -72,7 +77,12 @@ export class AuthService {
   * Does NOT print to stdout — returns tokens so callers can display


🟠 warning — loginInteractive's documented 'Does NOT print to stdout' contract is now violated by the device branch, which prints the QR block and causes the spinner conflict.

berget-ai · 2026-09-14T09:26:33Z

+
+import { logger, LogLevel } from '../../utils/logger.js';
+
+const DEVICE_AUTHORIZATION_PATH = '/realms/berget/protocol/openid-connect/auth/device';


🟠 warning — Realm hardcoded ('/realms/berget/…') although AuthConfig.realm exists and issuer.ts composes it dynamically; also bypasses the OIDC discovery already used for PKCE.

berget-ai · 2026-09-14T09:26:33Z

+ */
+export async function startDeviceFlow(options: DeviceFlowOptions): Promise<BrowserAuthResult> {
+  const debug = options.debug || logger.getLogLevel() >= LogLevel.DEBUG;
+  const baseUrl = `https://${new URL(options.config.keycloakUrl).host}`;


🟠 warning — Forcing https://${URL.host} drops URL path prefixes and breaks BERGET_KEYCLOAK_URL=http://localhost:8080 local-dev overrides that getAuthConfig() explicitly supports.

berget-ai · 2026-09-14T09:26:33Z

+ * @returns `{ intervalSeconds }` to adjust polling (slow_down), or throws a
+ *          user-readable Error for terminal states.
+ */
+export function handlePollError(


🟠 warning — No tests added for any device-flow logic (extractTokenResult, handlePollError, QR renderer, poll loop) despite 'Exported for tests' comments and pkce-flow.test.ts precedent.

berget-ai · 2026-09-14T09:26:33Z

+
+const DEVICE_AUTHORIZATION_PATH = '/realms/berget/protocol/openid-connect/auth/device';
+const TOKEN_PATH = '/realms/berget/protocol/openid-connect/token';
+const DEVICE_FLOW_SCOPE = 'openid email profile offline_access device-email-otp';


🟡 nit — Adds custom scope device-email-otp absent from the PKCE scope; if it's not assigned to the berget-code Keycloak client the device request fails with invalid_scope — confirm it exists.

berget-ai · 2026-09-14T09:26:33Z

+  deviceInfo: DeviceAuthorizationResponse;
+}): Promise<BrowserAuthResult> {
+  const { baseUrl, clientId, deviceInfo } = input;
+  const deadline = Date.now() + deviceInfo.expires_in * 1000;


🟡 nit — Authorization response trusted via as-cast; a missing expires_in yields a NaN deadline so the poll loop never runs and login instantly 'times out'.

berget-ai · 2026-09-14T09:26:33Z

    .command(AuthService.COMMANDS.LOGIN)
    .description('Log in to Berget')
-    .action(async () => {
+    .option('--device', 'Sign in with a device code (QR / another machine) instead of the browser')


🟡 nit — New --device flag is undocumented in the README's Authentication section, despite SSH/headless being its stated use case.

- berget auth login prompts method on TTY (this device / other device with QR); --device and non-TTY skip the prompt - device flow output: OSC 8 clickable link (cyan, underlined), bold user code, breathing room around the QR block - drop static 'Waiting for approval' line — the spinner owns status

berget-ai

Follow-up review

New changes: 1 commit (80b71d3) — adds an interactive browser/device method prompt to berget auth login and polishes device-flow output (chalk styling, OSC 8 hyperlink, removes the redundant "Waiting for approval…" print, QR indent).

Previous findings:

⏳ critical auth-sync.ts (QR/link wiped by clack spinner in code init) — still present: s.start() still runs before loginInteractive, and startDeviceFlow still console.logs the QR under the live spinner (confirmed the wizard spinner is clack's, clack-prompter.ts:55); only the device-flow's own "Waiting…" line was removed.
⏳ warning auth-service.ts (loginInteractive documented "Does NOT print to stdout" but prints) — still present.
⏳ warning device-flow.ts (hardcoded /realms/berget) — still present.
⏳ warning device-flow.ts:142 (https://...host breaks local-dev http override) — still present.
⏳ warning (no unit tests for device flow) — still present; helpers say "Exported for tests" but no test file references device-flow.
⏳ warning (device-email-otp scope / invalid_scope risk) — still present.
⏳ nit (NaN deadline when expires_in missing) — still present.
⏳ nit (README lacks --device / QR docs) — still present.

New findings:

nit src/services/auth-service.ts:51 — the login-method p.select block is a verbatim copy of ensureCliAuth's prompt (identical labels/values/hint); extract a shared const/helper — this commit already shows the two must be edited in lockstep.
nit src/auth/oauth/device-flow.ts:171 — OSC 8 hyperlink is emitted with no terminal-capability check; chalk's color-level detection doesn't strip/wrap OSC 8, so unsupported terminals (conhost, tmux <3.3) and piped output render raw ESC]8;; garbage where the previous plain URL worked.

Done right: the new --device-skip logic (!method && process.stdin.isTTY) keeps non-TTY/CI behavior on the old silent browser default, and both prompt copies were updated together in this commit.

Inline findings

_{Berget AI (berget/zai-org/GLM-5.3-Flash) | PR #90}

berget-ai · 2026-09-14T09:32:39Z

+      // --device skip the prompt).
+      let method = options?.method;
+      if (!method && process.stdin.isTTY) {
+        const choice = await p.select<'browser' | 'device'>({


🟡 nit — Method-choice prompt is a verbatim copy of ensureCliAuth's select in auth-sync.ts — extract a shared helper to avoid drift.

berget-ai · 2026-09-14T09:32:39Z

+
+  const verificationUri = deviceInfo.verification_uri_complete ?? deviceInfo.verification_uri;
+
+  const linkedUrl = `\u001B]8;;${verificationUri}\u0007${verificationUri}\u001B]8;;\u0007`;


🟡 nit — OSC 8 hyperlink emitted without terminal-capability detection; chalk color-level checks don't cover OSC 8, so unsupported terminals/pipes show raw \u001B]8;; escape garbage.

- CRITICAL: device output was erased by the clack spinner (80ms repaint); browser path keeps the spinner, device path logs via prompter instead - endpoints built from config realm (keycloakUrl + realm) — supports BERGET_KEYCLOAK_URL=http://localhost:8080 and custom realms - validate device authorization response (missing expires_in gave a NaN deadline and instant 'timed out') - poll errors (denied/expired/server) become error results, not throws - loginInteractive doc updated: device method prints to stdout - 7 unit tests (poll errors, token extraction, QR renderer, happy path, endpoint construction); vitest excludes .pi/** (local tool artifacts) - README documents --device

berget-ai

Follow-up review

New changes: 1 commit (a401527) — "fix: review feedback on device flow": spinner removed from the device path, endpoint built from config.realm/keycloakUrl, response validation added, handlePollError throws contained, doc comment corrected, 7 new unit tests, --device documented in README.

Previous findings:

✅ critical spinner wiping QR in code init — fixed: device path no longer starts a spinner (auth-sync.ts:124 logs a step then runs the flow bare; browser spinner kept, else-branch returns early).
✅ warning loginInteractive doc mismatch — fixed (src/services/auth-service.ts:99-103 now states the device method prints to stdout).
✅ warning hardcoded /realms/berget — fixed (device-flow.ts:141 builds from config.realm, with a regression test).
✅ warning https:// forcing breaking local dev — fixed; keycloakUrl used as-is and the tests run against http://localhost:8080.
✅ warning no unit tests — fixed: device-flow.test.ts (7 tests, verified passing locally; tsc --noEmit also clean).
⏳ warning device-email-otp invalid_scope risk — still present: scope unchanged; the new comment only asserts the scope is assigned on stage/prod, and a rejection still surfaces as a generic Keycloak rejected device authorization (400).
✅ nit NaN deadline when expires_in missing — fixed via response validation.
✅ nit README lacks --device — fixed; flag verified wired (src/commands/auth.ts:19).
⏳ nit duplicated login-method prompt — still present (verbatim in auth-service.ts:51 and auth-sync.ts:110); this commit's if/else restructure even duplicated the post-login handling inside ensureCliAuth, strengthening the case for a shared helper.
⏳ nit OSC 8 hyperlink without terminal-capability check — still present (device-flow.ts:179 unchanged).

New findings:

nit src/auth/oauth/device-flow.ts:166 — the new validation checks device_code/user_code/expires_in but not interval, which is consumed unvalidated at line 229: a non-numeric interval makes sleep(NaN) fire immediately, hot-polling the token endpoint until the deadline; validate/clamp it (e.g. const interval = Number.isFinite(deviceInfo.interval) && deviceInfo.interval >= 1 ? deviceInfo.interval : DEFAULT_POLL_INTERVAL_SECONDS), and consider also requiring verification_uri so a malformed response can't render an "undefined" QR.

Done right: the critical spinner fix is correctly structured with a regression-safe early return, and README's --device matches the actual CLI flag.

Inline findings

_{Berget AI (berget/zai-org/GLM-5.3-Flash) | PR #90}

berget-ai · 2026-09-14T09:36:38Z

+    if (
+      typeof deviceInfo.device_code !== 'string' ||
+      typeof deviceInfo.user_code !== 'string' ||
+      typeof deviceInfo.expires_in !== 'number'


🟡 nit — Validation skips interval — a non-numeric value reaches sleep(NaN) and hot-polls the token endpoint until expiry; validate/clamp it (and require verification_uri).

berget-ai Bot requested changes Sep 14, 2026

View reviewed changes

berget-ai Bot previously approved these changes Sep 14, 2026

View reviewed changes

bergetjakob dismissed berget-ai[bot]’s stale review via a401527 September 14, 2026 09:34

berget-ai Bot approved these changes Sep 14, 2026

View reviewed changes

bergetjakob merged commit 85a15b6 into main Sep 14, 2026
2 checks passed

		@@ -72,7 +77,12 @@ export class AuthService {
		* Does NOT print to stdout — returns tokens so callers can display


		import { logger, LogLevel } from '../../utils/logger.js';

		const DEVICE_AUTHORIZATION_PATH = '/realms/berget/protocol/openid-connect/auth/device';


		const verificationUri = deviceInfo.verification_uri_complete ?? deviceInfo.verification_uri;

		const linkedUrl = `\u001B]8;;${verificationUri}\u0007${verificationUri}\u001B]8;;\u0007`;

Conversation

bergetjakob commented Sep 14, 2026

Vad

Tester

Manuellt verifierat

Uh oh!

berget-ai Bot left a comment

Choose a reason for hiding this comment

Review

Summary

Risk

Issues

Suggestions

Architecture

Inline findings

Uh oh!

berget-ai Bot Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

berget-ai Bot Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

berget-ai Bot Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

berget-ai Bot Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

berget-ai Bot Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

berget-ai Bot Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

berget-ai Bot Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

berget-ai Bot Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

berget-ai Bot left a comment

Choose a reason for hiding this comment

Follow-up review

Inline findings

Uh oh!

berget-ai Bot Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

berget-ai Bot Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

berget-ai Bot left a comment

Choose a reason for hiding this comment

Follow-up review

Inline findings

Uh oh!

berget-ai Bot Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant