Skip to content

feat(cli): always-on paste fallback for auth login (Ink UI)#4072

Merged
saddlepaddle merged 2 commits into
mainfrom
showy-lyric
May 5, 2026
Merged

feat(cli): always-on paste fallback for auth login (Ink UI)#4072
saddlepaddle merged 2 commits into
mainfrom
showy-lyric

Conversation

@saddlepaddle
Copy link
Copy Markdown
Collaborator

@saddlepaddle saddlepaddle commented May 5, 2026

Summary

  • superset auth login now races the loopback callback and the paste prompt in parallel — both UI affordances are always live, no env-var sniffing. Fixes login on remote machines where SSH_CONNECTION isn't set (tmux reattach, VS Code Remote, Codespaces, mosh).
  • The auth screen is now an Ink component so the URL copied to clipboard toast is a real 1.5s React-state-driven notification (no log spam on repeated c presses).
  • c copies the URL via OSC 52 — works cross-device over SSH because the escape passes through to the local terminal emulator. Native pbcopy/clip/wl-copy/xclip/xsel stays as a fallback for local users whose terminal blocks OSC 52.
  • Esc / Ctrl-C exit cleanly with Login interrupted instead of throwing.

Test plan

  • Local TTY (golden path): browser auto-opens with loopback redirect, callback fires, paste prompt is auto-cancelled, Authorized! prints, token written to config.
  • Local, decline browser: copy printed URL into a different browser session manually, paste <code>#<state> back; paste path wins, loopback server closes cleanly.
  • Remote ssh / tmux reattach (no SSH_CONNECTION): same UI shown, paste path completes successfully.
  • c keybinding: press c at the empty paste prompt; verify URL is on local clipboard (pbpaste on darwin, or paste into a local browser if testing over SSH); toast appears for 1.5s and disappears with no layout drift.
  • Esc cancellation: press Esc; expect Login interrupted and exit 0, no stack trace.
  • Ctrl-C cancellation: same as Esc.
  • Multi-org user: full flow continues to org picker after auth.
  • --organization <slug> non-interactive flow still works.
  • CI / non-TTY (piped stdin): falls back to plain Clack flow as before.

Summary by cubic

Makes superset auth login reliable on remote terminals by racing the loopback callback and paste flow with a new ink UI. Adds cross-device clipboard copy and clean cancellation; non‑TTY and CI behavior stay the same, fixes Ctrl‑C hangs, and improves redirect URL handling.

  • New Features

    • Paste fallback is always available; loopback and paste run in parallel to handle SSH/tmux/VS Code Remote/Codespaces/mosh.
    • ink UI with a live prompt and a 1.5s “URL copied” toast; press c to copy the URL via OSC 52 with native pbcopy/clip/wl-copy/xclip/xsel fallback.
    • Esc/Ctrl‑C now exit with “Login interrupted”; non‑TTY and CI sessions skip ink and use the plain Clack prompts.
  • Bug Fixes

    • Prevented Ctrl‑C hangs by rejecting the auth race from the outer signal.
    • Built the paste redirect with new URL() to avoid bad URIs when SUPERSET_WEB_URL has a trailing slash.

Written for commit c049520. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • Interactive CLI sign-in UI with real-time status, editable code input, paste handling, copy feedback and cancel/submit controls
    • Clipboard support for authentication codes (OSC 52 plus native fallbacks)
    • Concurrent sign-in flows: loopback callback and paste paths run in parallel for faster, more reliable auth
  • Dependencies

    • React and Ink promoted to runtime dependencies; type package added to devDependencies

Drop the SSH/CI env-var sniff that gated the paste-code flow.
`superset auth login` now races the loopback callback and the
paste prompt in parallel — both UI affordances are always live,
so remote (tmux, VS Code Remote, Codespaces, mosh) sessions get a
working fallback even when SSH_CONNECTION isn't set.

The auth screen renders via Ink so the "URL copied" toast is a
real 1.5s React-state-driven notification, not append-only log
spam. `c` copies the URL via OSC 52 (cross-device, works over
SSH) with native pbcopy/clip/wl-copy/xclip as a local fallback.
Esc / Ctrl-C now exit cleanly with "Login interrupted" instead
of throwing.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 5, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dbb64496-cee9-4009-ad68-18405ee25633

📥 Commits

Reviewing files that changed from the base of the PR and between 7086fec and c049520.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • packages/cli/package.json
  • packages/cli/src/commands/auth/login/LoginUI.tsx
  • packages/cli/src/commands/auth/login/command.ts
  • packages/cli/src/lib/auth.ts
✅ Files skipped from review due to trivial changes (1)
  • packages/cli/src/commands/auth/login/LoginUI.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/cli/src/commands/auth/login/command.ts

📝 Walkthrough

Walkthrough

Adds an Ink-based interactive login UI and clipboard helper, moves Ink/React to runtime dependencies, and refactors the CLI auth flow to concurrently race a loopback callback against an abortable pasted-code prompt with coordinated UI wiring and cleanup.

Changes

Interactive Login + Auth Flow Refactor

Layer / File(s) Summary
Dependencies
packages/cli/package.json
Adds ink (^7.0.1) and react (19.2.0) to dependencies; adds @types/react to devDependencies.
Public Contracts / Types
packages/cli/src/commands/auth/login/LoginUI.tsx, packages/cli/src/lib/auth.ts
Adds LoginStatus and LoginUIProps; updates LoginCallbacks so onAuthorizationUrl takes a single url and promptForPastedCode accepts an AbortSignal.
UI Component
packages/cli/src/commands/auth/login/LoginUI.tsx
New Ink React LoginUI component: typed/pasted input buffer, validation, copy feedback (idle/copy), keybindings (Esc/Ctrl+C cancel, Enter submit, c copy), paste handling, and 1.5s copy-success transient.
Clipboard Utility
packages/cli/src/commands/auth/login/copyToClipboard.ts
New copyToClipboard(text): Promise<boolean>: emits OSC 52 when TTY (base64-encoded) and falls back to platform candidates (pbcopy, clip, wl-copy, xclip, xsel), returning success boolean.
Command Integration / UI Wiring
packages/cli/src/commands/auth/login/command.ts
Conditionally mounts LoginUI in TTY/non-CI contexts, exposes a pastePromise resolved by UI submit/cancel, updates UI props from onAuthorizationUrl and onCopy, and adapts promptForPastedCode to await UI paste or fall back to non-Ink prompt.
Auth Flow Refactor
packages/cli/src/lib/auth.ts
login() always attempts loopback bind, builds paste and loopback URLs, notifies callbacks with the paste URL, opens browser to loopback URL when available, and races loopback callback vs. abortable pasted-code prompt; coordinates abort controllers and cleans up the loopback server; validates state and maps errors (including state mismatch) to CLI errors.
sequenceDiagram
    actor User
    participant CLI as CLI / LoginUI
    participant AuthLib as Auth Library
    participant Server as Loopback Server
    participant Browser as Browser / OAuth
    participant Clipboard as Clipboard / OSC52

    User->>CLI: start login (TTY)
    CLI->>AuthLib: call login() with callbacks
    AuthLib->>Server: bind loopback server
    AuthLib->>CLI: onAuthorizationUrl(pasteURL)
    CLI->>User: show URL + input UI

    par Loopback Path
        AuthLib->>Browser: open loopback authorization URL
        User->>Browser: authenticate
        Browser->>Server: redirect/callback with code
        Server-->>AuthLib: deliver code + redirectUri
    and Paste Path
        User->>Clipboard: copy authorization code
        User->>CLI: paste code (contains `#state`)
        CLI->>AuthLib: resolve promptForPastedCode(code)
        AuthLib->>AuthLib: parse & validate state
    and Copy Helper
        User->>CLI: press 'c' when input empty
        CLI->>Clipboard: call copyToClipboard()
        Clipboard-->>CLI: returns true/false
        CLI->>User: show "Copied!" transient
    end

    AuthLib->>AuthLib: race completes with winning result
    AuthLib->>AuthLib: exchange code for tokens
    AuthLib-->>CLI: auth complete
    CLI->>Server: close loopback server
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

"I hopped to ink and typed a code so spry,
A paste, a loopback, clipboard reaching high 🐇,
Keys and cursors blink and play,
Copy feedback twinkles, then goes away,
Now login races home beneath the sky."

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding an Ink-based UI with an always-on paste fallback for the auth login flow.
Description check ✅ Passed The PR description is comprehensive, covering summary, test plan, and additional context. All required template sections are present and well-documented.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch showy-lyric

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 5, 2026

Greptile Summary

  • Replaces the old env-var-sniffed paste fallback with an always-on parallel race between the loopback OAuth callback and a paste prompt rendered as an Ink React component, fixing login on remote environments (tmux, VS Code Remote, Codespaces) where SSH_CONNECTION is unset.
  • Adds c-key copy via OSC 52 (works over SSH) with native-binary fallback and a 1.5 s React-state-driven toast; Esc/Ctrl-C exit cleanly via exitOnCtrlC: false + useInput.
  • Three issues in command.ts: a duplicate p.intro banner on the Ink success path, a redundant second api.user.me.query() call, and fragile cancel detection via err.message === \"Login cancelled\" string comparison.

Confidence Score: 4/5

Safe to merge with minor UX and efficiency fixes recommended in command.ts

Only P2 findings: a redundant Clack intro banner on the Ink code path, a duplicate network call, and a fragile string-equality cancel check. Core auth logic, race management, and clipboard implementation are solid.

packages/cli/src/commands/auth/login/command.ts — success-path banner duplication and duplicate API call

Important Files Changed

Filename Overview
packages/cli/src/commands/auth/login/command.ts Orchestrates the Ink/Clack dual-path login flow; contains three issues: a redundant p.intro call on the Ink success path, a duplicate api.user.me.query() network call, and fragile error detection via message string comparison.
packages/cli/src/commands/auth/login/LoginUI.tsx New Ink React component for the auth login UI; correctly handles input, paste, clipboard copy keybinding (with empty-buffer guard), validation, and cancel via Esc/Ctrl-C.
packages/cli/src/commands/auth/login/copyToClipboard.ts Clean implementation: OSC 52 first (works cross-SSH), then falls back to native clipboard tools; error handling is solid with the child process close/error listeners.
packages/cli/src/lib/auth.ts Refactored to race loopback callback vs paste prompt using dual AbortControllers; loopback errors are silently swallowed to keep the paste path alive, which is intentional and correctly documented.
packages/cli/package.json Adds ink ^7.0.1, react ^19.2.5, and @types/react ^19.2.14 as expected for the new Ink UI component.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[superset auth login] --> B{stdout & stdin TTY?}
    B -- No --> C[Clack plain flow]
    B -- Yes --> D[Render Ink LoginUI]
    D --> E[bindLoopbackServer]
    E --> F[buildAuthorizeUrl paste redirect URI]
    F --> G[onAuthorizationUrl callback - update Ink UI with URL]
    G --> H{shouldOpenBrowser?}
    H -- Yes --> I[openBrowser loopback URL]
    H -- No --> J[Race: loopback vs paste]
    I --> J
    J --> K[waitForCallback loopback AbortController]
    J --> L[promptForPastedCode paste AbortController]
    K -- callback wins --> M[pasteController.abort - onAbort resolves pastePromise]
    L -- paste wins --> N[parsePastedCode - state check - callbackController.abort]
    M --> O[exchangeCodeForToken loopback redirectUri]
    N --> P[exchangeCodeForToken paste redirectUri]
    O --> Q[update status done - unmount Ink]
    P --> Q
    Q --> R[writeConfig tokens]
    R --> S[api.user.me.query - show name/email]
    S --> T{org selection}
    T --> U[writeConfig orgId - p.outro]
    K -- error/timeout --> V[swallowed - paste path continues]
    L -- user presses Esc --> W[pasteReject CLIError Login cancelled]
    W --> X[caught - cancelled=true - p.cancel Login interrupted]
Loading

Comments Outside Diff (1)

  1. packages/cli/src/commands/auth/login/command.ts, line 193 (link)

    P2 Duplicate api.user.me.query() call

    api.user.me.query() is already called at line 129 to display the user's name and email. This line issues a second identical request just to read .id from the result. The user variable from line 129 is unreachable here because it's declared inside a try block above. Lift it out of that block so the id can be reused without a second network round-trip.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: packages/cli/src/commands/auth/login/command.ts
    Line: 193
    
    Comment:
    **Duplicate `api.user.me.query()` call**
    
    `api.user.me.query()` is already called at line 129 to display the user's name and email. This line issues a second identical request just to read `.id` from the result. The `user` variable from line 129 is unreachable here because it's declared inside a `try` block above. Lift it out of that block so the id can be reused without a second network round-trip.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
packages/cli/src/commands/auth/login/command.ts:119-124
**Redundant `p.intro` banner printed twice on the Ink path**

When `inkInstance` is truthy, the Ink component already rendered `superset auth login` as its header. After `inkInstance.unmount()` in the `finally` block, this code calls `p.intro("superset auth login")` again, causing the banner to appear a second time in the terminal output before the `✔  Authorized!` line.

```suggestion
		if (!inkInstance) {
			p.log.success("Authorized!");
		} else {
			p.log.success("Authorized!");
		}
```

### Issue 2 of 3
packages/cli/src/commands/auth/login/command.ts:193
**Duplicate `api.user.me.query()` call**

`api.user.me.query()` is already called at line 129 to display the user's name and email. This line issues a second identical request just to read `.id` from the result. The `user` variable from line 129 is unreachable here because it's declared inside a `try` block above. Lift it out of that block so the id can be reused without a second network round-trip.

### Issue 3 of 3
packages/cli/src/commands/auth/login/command.ts:94-99
**Fragile error identification by message string**

The check `err.message === "Login cancelled"` is a stringly-typed sentinel. If the message is ever changed (e.g. for i18n or copy consistency), the catch block will silently re-throw as an unhandled error instead of producing the clean `Login interrupted` exit. Consider tagging `CLIError` with a machine-readable code or adding a custom subclass (e.g. `LoginCancelledError`) so the check is refactoring-safe.

Reviews (1): Last reviewed commit: "feat(cli): always-on paste fallback for ..." | Re-trigger Greptile

Comment on lines +119 to +124
if (!inkInstance) {
p.log.success("Authorized!");
} else {
p.intro("superset auth login");
p.log.success("Authorized!");
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Redundant p.intro banner printed twice on the Ink path

When inkInstance is truthy, the Ink component already rendered superset auth login as its header. After inkInstance.unmount() in the finally block, this code calls p.intro("superset auth login") again, causing the banner to appear a second time in the terminal output before the ✔ Authorized! line.

Suggested change
if (!inkInstance) {
p.log.success("Authorized!");
} else {
p.intro("superset auth login");
p.log.success("Authorized!");
}
if (!inkInstance) {
p.log.success("Authorized!");
} else {
p.log.success("Authorized!");
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cli/src/commands/auth/login/command.ts
Line: 119-124

Comment:
**Redundant `p.intro` banner printed twice on the Ink path**

When `inkInstance` is truthy, the Ink component already rendered `superset auth login` as its header. After `inkInstance.unmount()` in the `finally` block, this code calls `p.intro("superset auth login")` again, causing the banner to appear a second time in the terminal output before the `✔  Authorized!` line.

```suggestion
		if (!inkInstance) {
			p.log.success("Authorized!");
		} else {
			p.log.success("Authorized!");
		}
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +94 to +99
} catch (err) {
if (err instanceof CLIError && err.message === "Login cancelled") {
cancelled = true;
} else {
throw err;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Fragile error identification by message string

The check err.message === "Login cancelled" is a stringly-typed sentinel. If the message is ever changed (e.g. for i18n or copy consistency), the catch block will silently re-throw as an unhandled error instead of producing the clean Login interrupted exit. Consider tagging CLIError with a machine-readable code or adding a custom subclass (e.g. LoginCancelledError) so the check is refactoring-safe.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cli/src/commands/auth/login/command.ts
Line: 94-99

Comment:
**Fragile error identification by message string**

The check `err.message === "Login cancelled"` is a stringly-typed sentinel. If the message is ever changed (e.g. for i18n or copy consistency), the catch block will silently re-throw as an unhandled error instead of producing the clean `Login interrupted` exit. Consider tagging `CLIError` with a machine-readable code or adding a custom subclass (e.g. `LoginCancelledError`) so the check is refactoring-safe.

How can I resolve this? If you propose a fix, please make it concise.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ✅ Neon database branch

Thank you for your contribution! 🎉

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 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 `@packages/cli/src/commands/auth/login/command.ts`:
- Line 22: The current useInk variable (const useInk = process.stdout.isTTY &&
process.stdin.isTTY) will enable the Ink interactive UI in CI jobs that allocate
a pseudo-TTY; update its logic to explicitly opt-out when running in CI by
checking common CI environment flags (e.g., process.env.CI,
process.env.GITHUB_ACTIONS, process.env.GITLAB_CI, process.env.CIRCLECI) and
only enable Ink when both TTY checks pass and none of those CI indicators are
present (e.g., const useInk = process.stdout.isTTY && process.stdin.isTTY &&
!isCI). Locate the useInk declaration in command.ts and add a small helper or
inline check (isCI) to ensure CI environments take the plain fallback path.
- Around line 68-90: In promptForPastedCode, pass the abort signal into the
interactive prompt so p.text can be cancelled: update the p.text call inside
promptForPastedCode to include the signal option (so the prompt listens to the
provided AbortSignal), keeping the existing message and validate behavior and
still handling p.isCancel and signal.aborted checks; this affects the p.text
invocation used when inkInstance is falsy and ensures the prompt is torn down if
the passed-in signal fires.

In `@packages/cli/src/commands/auth/login/LoginUI.tsx`:
- Around line 103-105: The footer help text in LoginUI.tsx currently only shows
"Esc to cancel" but the input handler supports both Esc and Ctrl+C; update the
Text (dimColor italic) element in the LoginUI component so it mentions both keys
(e.g., "Esc or Ctrl+C to cancel" or "Esc / Ctrl+C to cancel") to accurately
reflect available shortcuts. Locate the Text node rendering the footer inside
LoginUI and replace the string while keeping styling and formatting intact.

In `@packages/cli/src/lib/auth.ts`:
- Around line 290-300: The pasteRedirectUri currently concatenates getWebUrl()
and PASTE_REDIRECT_PATH which can produce a double-slash when SUPERSET_WEB_URL
ends with '/' (breaking exact redirect matching). Update the code that builds
pasteRedirectUri (where pasteRedirectUri is assigned) to normalize the base URL
from getWebUrl() (or use the URL constructor) so there is exactly one slash
between the base and PASTE_REDIRECT_PATH; i.e., strip any trailing '/' from
getWebUrl() or build the redirect with new URL(PASTE_REDIRECT_PATH, webUrl)
before assigning pasteRedirectUri.
- Around line 329-398: The wrapper Promise that races waitForCallback and
callbacks.promptForPastedCode never rejects when the outer `signal` aborts;
modify the `onOuterAbort` handler (or add a listener inside the Promise) to call
the same `settle` routine and reject the Promise with an abort error while also
aborting `callbackController` and `pasteController`. In practice, inside the new
Promise that defines `settled/settle`, update `onOuterAbort` to call `settle(()
=> { callbackController.abort(); pasteController.abort(); reject(new
Error("aborted")); });` (or use a proper AbortError), ensuring the outer abort
triggers settling the Promise in addition to aborting the inner controllers so
`waitForCallback`/`promptForPastedCode` can't leave the Promise pending. Refer
to `callbackController`, `pasteController`, `onOuterAbort`, `waitForCallback`,
and `callbacks.promptForPastedCode` to locate the change.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4c56b101-0d91-4a19-b9a1-14f28587883d

📥 Commits

Reviewing files that changed from the base of the PR and between 8042fd2 and 36eeb71.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (5)
  • packages/cli/package.json
  • packages/cli/src/commands/auth/login/LoginUI.tsx
  • packages/cli/src/commands/auth/login/command.ts
  • packages/cli/src/commands/auth/login/copyToClipboard.ts
  • packages/cli/src/lib/auth.ts

Comment thread packages/cli/src/commands/auth/login/command.ts Outdated
Comment thread packages/cli/src/commands/auth/login/command.ts
Comment thread packages/cli/src/commands/auth/login/LoginUI.tsx
Comment thread packages/cli/src/lib/auth.ts Outdated
Comment thread packages/cli/src/lib/auth.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
packages/cli/src/commands/auth/login/command.ts (1)

25-38: 💤 Low value

Consider non-null typing for pasteResolve/pasteReject.

These are assigned synchronously inside the Promise executor before any consumer can run, so the | null union and the ?.() calls on lines 35-36 / 82 are defensive against an impossible state. A small let resolve!: … pattern (or capturing them via Promise.withResolvers()) removes the noise without changing behavior.

♻️ Optional cleanup
-		let pasteResolve: ((code: string) => void) | null = null;
-		let pasteReject: ((err: Error) => void) | null = null;
-		const pastePromise = new Promise<string>((resolve, reject) => {
-			pasteResolve = resolve;
-			pasteReject = reject;
-		});
+		let pasteResolve!: (code: string) => void;
+		let pasteReject!: (err: Error) => void;
+		const pastePromise = new Promise<string>((resolve, reject) => {
+			pasteResolve = resolve;
+			pasteReject = reject;
+		});

Then drop the ?. on lines 35-36 and 82.

🤖 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 `@packages/cli/src/commands/auth/login/command.ts` around lines 25 - 38, The
pasteResolve/pasteReject variables are unnecessarily typed as nullable even
though they’re assigned synchronously in the Promise executor; change their
declarations to non-null asserted locals (e.g., declare pasteResolve and
pasteReject with the appropriate function types using the !/definite assignment
pattern) so you can remove the | null unions and the safe-call operators; keep
the Promise creation (pastePromise) and the assignments inside its executor, and
update usages in LoginUIProps (onSubmit/onCancel) to call pasteResolve(...) and
pasteReject(...) directly without ?.().
packages/cli/src/commands/auth/login/LoginUI.tsx (1)

31-70: 💤 Low value

Minor: input is gated on status but not on url === null.

When status === "starting" (initial render before onAuthorizationUrl fires), useInput still accepts typed characters into value (lines 66-69) and usePaste still appends pasted content (lines 72-75), but the cursor (showCursor = status === "waiting", line 77) is hidden, so the user gets no visual feedback that their keystrokes are being captured. Consider also showing the cursor in "starting", or buffering input only after the URL arrives.

♻️ Suggested tweak
-	const showCursor = status === "waiting";
+	const showCursor = status === "waiting" || status === "starting";
🤖 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 `@packages/cli/src/commands/auth/login/LoginUI.tsx` around lines 31 - 70, The
input handlers accept keystrokes before an authorization URL is ready; update
the gating so input/paste are only processed after the URL arrives (or make the
cursor visible during "starting"). Specifically, in the useInput callback (and
the corresponding usePaste handler), add a guard that returns early when url ===
null || status === "starting" (instead of only checking status
"exchanging"/"done"), and/or change showCursor's expression (currently
showCursor = status === "waiting") to include "starting" (e.g. status ===
"waiting" || status === "starting") so users see feedback; touch symbols:
useInput, usePaste, showCursor, url, status, onAuthorizationUrl, setValue,
onCopy to implement this change.
🤖 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.

Nitpick comments:
In `@packages/cli/src/commands/auth/login/command.ts`:
- Around line 25-38: The pasteResolve/pasteReject variables are unnecessarily
typed as nullable even though they’re assigned synchronously in the Promise
executor; change their declarations to non-null asserted locals (e.g., declare
pasteResolve and pasteReject with the appropriate function types using the
!/definite assignment pattern) so you can remove the | null unions and the
safe-call operators; keep the Promise creation (pastePromise) and the
assignments inside its executor, and update usages in LoginUIProps
(onSubmit/onCancel) to call pasteResolve(...) and pasteReject(...) directly
without ?.().

In `@packages/cli/src/commands/auth/login/LoginUI.tsx`:
- Around line 31-70: The input handlers accept keystrokes before an
authorization URL is ready; update the gating so input/paste are only processed
after the URL arrives (or make the cursor visible during "starting").
Specifically, in the useInput callback (and the corresponding usePaste handler),
add a guard that returns early when url === null || status === "starting"
(instead of only checking status "exchanging"/"done"), and/or change
showCursor's expression (currently showCursor = status === "waiting") to include
"starting" (e.g. status === "waiting" || status === "starting") so users see
feedback; touch symbols: useInput, usePaste, showCursor, url, status,
onAuthorizationUrl, setValue, onCopy to implement this change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9363a470-8130-4a39-a725-3181269503e4

📥 Commits

Reviewing files that changed from the base of the PR and between 36eeb71 and 7086fec.

📒 Files selected for processing (3)
  • packages/cli/src/commands/auth/login/LoginUI.tsx
  • packages/cli/src/commands/auth/login/command.ts
  • packages/cli/src/lib/auth.ts

- auth.ts: outer signal now rejects the wrapper Promise.race instead
  of just aborting children, so Ctrl-C from the parent can no longer
  hang waiting on a Winner that nobody will resolve.
- auth.ts: build paste redirect via `new URL()` so a trailing slash
  on `SUPERSET_WEB_URL` doesn't corrupt the registered redirect URI.
- command.ts: skip the Ink UI in CI even with an allocated PTY.
- command.ts: drop the duplicate `p.intro` after Ink unmount —
  the component already rendered the header.
- LoginUI.tsx: footer now reads "Esc / Ctrl+C to cancel".
@saddlepaddle saddlepaddle merged commit af35e07 into main May 5, 2026
16 checks passed
@saddlepaddle saddlepaddle mentioned this pull request May 5, 2026
5 tasks
saddlepaddle added a commit that referenced this pull request May 5, 2026
Two CLI changes since v0.2.7, both fixing remote-auth UX:

- `superset auth login` now races the loopback callback and the paste
  prompt in parallel, with an Ink UI for the paste path. Fixes login
  on remote machines where SSH_CONNECTION isn't set (tmux reattach,
  VS Code Remote, Codespaces, mosh). `c` copies the auth URL via
  OSC 52 with native `pbcopy`/`clip`/`wl-copy`/`xclip`/`xsel`
  fallback. Esc / Ctrl-C exit cleanly. (#4072)
- Polished the `/cli/auth/code` web page to match Claude Code's
  paste-code UI (single-line code box, ghost copy button, click-to-
  copy with selection-on-release fallback). Wired standard readline
  shortcuts in the CLI paste prompt: option+backspace / ctrl+w
  delete the previous word; cmd+backspace / ctrl+u / ctrl+k clear
  the buffer. (#4075)

Push cli-v0.2.8 after this lands to fire the release pipeline.
saddlepaddle added a commit that referenced this pull request May 6, 2026
* feat(cli): always-on paste fallback for auth login (Ink UI)

Drop the SSH/CI env-var sniff that gated the paste-code flow.
`superset auth login` now races the loopback callback and the
paste prompt in parallel — both UI affordances are always live,
so remote (tmux, VS Code Remote, Codespaces, mosh) sessions get a
working fallback even when SSH_CONNECTION isn't set.

The auth screen renders via Ink so the "URL copied" toast is a
real 1.5s React-state-driven notification, not append-only log
spam. `c` copies the URL via OSC 52 (cross-device, works over
SSH) with native pbcopy/clip/wl-copy/xclip as a local fallback.
Esc / Ctrl-C now exit cleanly with "Login interrupted" instead
of throwing.

* fix(cli): address PR review on auth login Ink flow

- auth.ts: outer signal now rejects the wrapper Promise.race instead
  of just aborting children, so Ctrl-C from the parent can no longer
  hang waiting on a Winner that nobody will resolve.
- auth.ts: build paste redirect via `new URL()` so a trailing slash
  on `SUPERSET_WEB_URL` doesn't corrupt the registered redirect URI.
- command.ts: skip the Ink UI in CI even with an allocated PTY.
- command.ts: drop the duplicate `p.intro` after Ink unmount —
  the component already rendered the header.
- LoginUI.tsx: footer now reads "Esc / Ctrl+C to cancel".

---------

Co-authored-by: Town Hall <codetown@Town-Hall.local>
saddlepaddle added a commit that referenced this pull request May 6, 2026
Two CLI changes since v0.2.7, both fixing remote-auth UX:

- `superset auth login` now races the loopback callback and the paste
  prompt in parallel, with an Ink UI for the paste path. Fixes login
  on remote machines where SSH_CONNECTION isn't set (tmux reattach,
  VS Code Remote, Codespaces, mosh). `c` copies the auth URL via
  OSC 52 with native `pbcopy`/`clip`/`wl-copy`/`xclip`/`xsel`
  fallback. Esc / Ctrl-C exit cleanly. (#4072)
- Polished the `/cli/auth/code` web page to match Claude Code's
  paste-code UI (single-line code box, ghost copy button, click-to-
  copy with selection-on-release fallback). Wired standard readline
  shortcuts in the CLI paste prompt: option+backspace / ctrl+w
  delete the previous word; cmd+backspace / ctrl+u / ctrl+k clear
  the buffer. (#4075)

Push cli-v0.2.8 after this lands to fire the release pipeline.
@Kitenite Kitenite deleted the showy-lyric branch May 6, 2026 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant