Skip to content

fix(login): allow ctrl-c to cancel browser login#1126

Open
runeb wants to merge 2 commits into
mainfrom
fix/login-ctrl-c-spinner
Open

fix(login): allow ctrl-c to cancel browser login#1126
runeb wants to merge 2 commits into
mainfrom
fix/login-ctrl-c-spinner

Conversation

@runeb
Copy link
Copy Markdown
Member

@runeb runeb commented May 22, 2026

Description

sanity login (browser-callback flow) used a spinner with ora's default discardStdin: true. The spinner text invites the user to cancel ("Press Ctrl + C to cancel"), but Ctrl+C did nothing.

This is a documented ora gotcha. From the ora README:

Note: discardStdin puts stdin into raw mode. In raw mode, Ctrl+C no longer generates SIGINT from the terminal. Ora re-emits Ctrl+C from stdin input, but if your code blocks the event loop with synchronous work, Ctrl+C handling is delayed until the blocking work ends. Use async APIs, a worker thread, or a child process to keep Ctrl+C responsive, or set discardStdin to false.

discardStdin: false is one of the two officially recommended workarounds. See also ora#156 — ctrl + c not working (closed with no clean fix; the README note above is effectively the resolution).

In our specific case the re-emit path also breaks for a more interesting reason: getProvider() runs its own spinner immediately before this one (packages/@sanity/cli/src/actions/auth/login/getProvider.ts:44). Both spinners drive the same stdin-discarder singleton, and the back-to-back start → stop → start cycle leaves stdin in a state where the second spinner's data listener never sees the 0x03 byte. Adding any process.stdin.on('data', …) listener after the second spinner starts unsticks it — which is consistent with the TTY-state-corruption family of bugs around raw-mode toggling. Untangling that is out of scope here; discardStdin: false sidesteps the whole thing for this call site.

The same pattern (sequential spinners) likely affects other commands; we should sweep them in a follow-up.

What to review

  • packages/@sanity/cli/src/actions/auth/login/login.ts — one option added to the spinner.

Testing

Manual:

  1. pnpm build:cli
  2. ./packages/@sanity/cli/bin/run.js login --provider google (with or without --no-open — both reproduce)
  3. Press Ctrl+C while the spinner is running → process exits cleanly.

Reverting the change reproduces the hang.

No new automated test — the bug depends on TTY/stdin raw-mode interaction that's awkward to simulate in unit tests.

The spinner during `sanity login` was started with ora's default
`discardStdin: true`, which puts stdin in raw mode. In our particular
setup this could leave Ctrl+C unhandled and hang the command despite
the spinner text inviting the user to cancel.

Passing `discardStdin: false` keeps stdin in cooked mode so the terminal
delivers SIGINT and the process exits as expected.
@runeb runeb requested a review from a team as a code owner May 22, 2026 00:12
@claude
Copy link
Copy Markdown

claude Bot commented May 22, 2026

Claude finished @runeb's task in 56s —— View job


Review

  • Read PR context
  • Inspect the diff
  • Check related code (other spinners / signal handling)
  • Check for changeset
  • Post review

Missing changeset. This is a user-facing bug fix in packages/@sanity/cli that affects runtime behavior, so a changeset is required. Add .changeset/<name>.md:

---
"@sanity/cli": patch
---

Allow Ctrl+C to cancel `sanity login` while the browser callback is pending.

Fix this →


Nit (optional). login.ts:89 — comment typo: dontdon't. Trivial; leave or fix as you like.


No other issues. The change is minimal and scoped to the one call site that prompts the user to press Ctrl+C, which is the right place to opt out of discardStdin. Other spinners in the codebase (e.g. undeploy.ts, installPackages.ts) keep the default and are unaffected. The PR description adequately justifies not adding an automated test for this TTY-dependent behavior.
· branch: fix/login-ctrl-c-spinner

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 22, 2026

📦 Bundle Stats — @sanity/cli

Compared against main (cd110e7c)

@sanity/cli

Metric Value vs main (cd110e7)
Internal (raw) 2.1 KB -
Internal (gzip) 799 B -
Bundled (raw) 10.97 MB -
Bundled (gzip) 2.06 MB -
Import time 810ms -2ms, -0.2%

bin:sanity

Metric Value vs main (cd110e7)
Internal (raw) 1023 B -
Internal (gzip) 486 B -
Bundled (raw) 9.84 MB -
Bundled (gzip) 1.77 MB -
Import time 1.90s +2ms, +0.1%

🗺️ View treemap · Artifacts

Details
  • Import time regressions over 10% are flagged with ⚠️
  • Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.

📦 Bundle Stats — @sanity/cli-core

Compared against main (cd110e7c)

Metric Value vs main (cd110e7)
Internal (raw) 97.0 KB -
Internal (gzip) 22.7 KB -
Bundled (raw) 21.61 MB -
Bundled (gzip) 3.42 MB -
Import time 772ms -2ms, -0.3%

🗺️ View treemap · Artifacts

Details
  • Import time regressions over 10% are flagged with ⚠️
  • Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.

📦 Bundle Stats — create-sanity

Compared against main (cd110e7c)

Metric Value vs main (cd110e7)
Internal (raw) 908 B -
Internal (gzip) 483 B -
Bundled (raw) 931 B -
Bundled (gzip) 491 B -
Import time ❌ ChildProcess denied: node -
Details
  • Import time regressions over 10% are flagged with ⚠️
  • Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.

@runeb runeb requested a review from rexxars May 22, 2026 00:13
@github-actions
Copy link
Copy Markdown
Contributor

Coverage Delta

File Statements
packages/@sanity/cli/src/actions/auth/login/login.ts 100.0% (±0%)

Comparing 1 changed file against main @ cd110e7c2b0cdad7438f53d958a0831fea2a3773

Overall Coverage

Metric Coverage
Statements 84.3% (±0%)
Branches 74.3% (±0%)
Functions 84.2% (±0%)
Lines 84.8% (±0%)

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.

2 participants