Repository navigation
Prompt for the host instead of failing when device code login has none - #7179
Merged
Merged
Conversation
maksim-zuev-db
marked this pull request as ready for review
October 26, 2026 09:42
maksim-zuev-db
requested review from
chrisst and
simonfaltum
and removed request for
chrisst
October 26, 2026 09:42
Collaborator
Integration test reportCommit: 45f4673
Top 6 slowest tests (at least 2 minutes):
|
simonfaltum
reviewed
Oct 26, 2026
simonfaltum
left a comment
simonfaltum
left a comment
Member
There was a problem hiding this comment.
Thanks for picking this up! Left a few comments inline. The main ones:
- The picker still lists profiles that device code login always rejects (L241).
- The new test touches the real
~/.databricks/token-cache.json(L225). - I think skipping discovery for device code would fix this more generally and remove the need for the picker special case (L245).
The rest are small suggestions.
One more that can't go inline since the file isn't in this PR: step 1 in docs/device-code-login.md says the host comes from an argument, --host, or an existing profile. It doesn't mention the picker, so someone running databricks auth login --use-device-code with no arguments won't find out why "Create a new profile" is missing. Might be worth a line there.
maksim-zuev-db
force-pushed
the
device-code-hide-create-profile
branch
from
October 26, 2026 11:58
f142176 to
7d59c27
Compare
Discovery login opens a browser on this machine and learns the workspace host only after sign-in. Device code login is meant for machines without a browser, but when no host was known it still went into the discovery branch and failed with "--use-device-code requires a workspace host". This broke "Create a new profile" in the login picker and "auth login --use-device-code --profile <new-profile>". Skip discovery for device code login so setHost resolves the host. It prompts for it when interactive and asks for --host otherwise. Co-authored-by: Isaac <no-reply@databricks.com>
maksim-zuev-db
force-pushed
the
device-code-hide-create-profile
branch
from
October 26, 2026 12:03
7d59c27 to
12f1366
Compare
maksim-zuev-db
changed the title
Contributor
|
Commit: Regression test reportSelected 1 changed tests (limit=20, 0 not selected) 1 tests: regression=1 unreleased=0 coverage=0 failing=0
TestAccept/cmd/auth/login/device-code/unsupported-host: branch ✅ | main (accb3df) ❌ | latest (v1.22.1) ❌main (accb3df): latest (v1.22.1): |
On Windows, bubbletea cannot cancel its read from the test's stdin pipe, so after the profile picker quits its reader swallows the input typed for the profile name prompt, and the test timed out waiting for the host prompt. Passing --profile makes the host prompt the only prompt while still covering the skipped discovery branch. Co-authored-by: Isaac <no-reply@databricks.com>
With --use-device-code, the login picker leaves out profiles on a classic account host and profiles saved with resources, which device code login always rejects. Profiles on unified hosts are still listed, because the discovery URL that identifies them is not saved on the profile. isDeviceCodeSupportedForHost replaces validateDeviceCodeHost so the picker and the check after setHost share the host check. Co-authored-by: Isaac <no-reply@databricks.com>
Share the setup of the two termtest tests in startDeviceCodeLogin. Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
simonfaltum
reviewed
Oct 26, 2026
simonfaltum
left a comment
simonfaltum
left a comment
Member
There was a problem hiding this comment.
Thanks, the discovery change looks good and the earlier comments are all covered. One issue with an empty host answer below, plus an optional suggestion.
| } | ||
| // an account ID for account and unified hosts. A workspace without | ||
| // device code login enabled is rejected later by DeviceCodeChallenge. | ||
| if useDeviceCode && !isDeviceCodeSupportedForHost(authArguments.Host, authArguments.DiscoveryURL) { |
Member
There was a problem hiding this comment.
With discovery skipped, setHost now prompts for the host, but promptForHost accepts an empty answer. If the user presses Enter at "Databricks host", Host stays "" and isDeviceCodeSupportedForHost("", "") returns true, so login continues:
ToOAuthArgumentresolves the config with theConfigAttributesloader, which reads env vars. WithDATABRICKS_HOSTset, login runs against that host (this workspace-only check never sees it), and the profile is saved without ahostkey.- Without
DATABRICKS_HOST, it fails later withhost must start with 'https://':.
Picker → "Enter a host URL manually" → empty input hits the same path, and the user gets the profile name prompt followed by a second host prompt. Before this PR, errDeviceCodeNeedsHost caught both cases.
I think the simplest fix is to reject empty input in promptForHost (cmdio.PromptOptions has Validate), or an empty host check here for device code.
Member
Author
There was a problem hiding this comment.
I kept the fix to device code login so other flows don't change in this PR. An empty host now fails with --use-device-code requires a workspace host, both at the setHost prompt and right after "Enter a host URL manually", without asking for the host again. Rejecting empty input in promptForHost would also change browser login and auth token, so I'd do that in a separate PR.
Comment on lines
+228
to
+230
| // (Create a new profile) and a manual host URL. Device code login skips | ||
| // discovery, so for it "Create a new profile" prompts for the host after | ||
| // the profile name. |
Member
There was a problem hiding this comment.
Optional, and mostly unrelated to this PR: after "Create a new profile" no host is known yet, so the name prompt suggests DEFAULT, and pressing Enter loads the existing DEFAULT profile if there is one. If that's an account profile the picker just hid, the user gets --use-device-code is only supported for workspace hosts for a profile they didn't pick. If it's a workspace profile, login reuses its host instead of prompting as this comment describes. Browser login reuses DEFAULT the same way, so fine to leave for a follow-up.
Member
Author
There was a problem hiding this comment.
Thanks, I updated the comment to mention that an existing profile's host is reused. I'll leave the fix for a follow-up PR.
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
simonfaltum
approved these changes
Oct 27, 2026
simonfaltum
left a comment
simonfaltum
left a comment
Member
There was a problem hiding this comment.
Thanks for addressing everything, LGTM.
Collaborator
Integration test reportCommit: e773191
8 interesting tests: 4 FAIL, 4 flaky
Top 50 slowest tests (at least 2 minutes):
|
idelafuente-oc
pushed a commit
to idelafuente-oc/cli
that referenced
this pull request
Nov 11, 2026
…s#7196) ## Changes After "Create a new profile" in the `databricks auth login` and `databricks auth token` pickers, the profile name prompt rejects the name of any existing config section with `profile X already exists. Choose a different name`. This includes the profiles the device code picker hides and sections without a `host`. A taken `DEFAULT` is no longer suggested: the prompt then has no default and requires a name. All profile name prompts now also trim spaces and pasted newlines, and reject the reserved `__settings__` name before the OAuth flow rather than after it. Saving a profile now rejects names that contain line breaks, for example a pasted `foo\nbar`. go-ini writes section headers unescaped, so such a name produced a config file that no longer parses. This applies to every command that saves a profile or sets the default profile, such as `auth login`, `auth token`, `auth switch` and `configure`. The `auth token` inline login can no longer target an existing profile, so its code for keeping that profile's scopes, client ID and resources is removed. ## Why Follow-up to a review comment on databricks#7179. With no host known yet, the prompt suggested `DEFAULT`, and pressing Enter logged in to the existing `DEFAULT` profile and reused its host. With `--use-device-code`, if that was an account profile the picker hid, login failed with `--use-device-code is only supported for workspace hosts`. A generated name like `DEFAULT-1` is not offered instead: the host is not known yet, so the name would not say which workspace the profile is for. ## Tests - `TestPromptForProfile*` cover the prompt: free and taken defaults, and empty, taken, reserved and pasted names. - `TestLoginCreateNewProfileRejectsTakenName`, `TestLoginDeviceCodeCreateNewProfileRejectsHiddenProfileName` and `TestResolveNoArgsTokenCreateNewProfileRejectsTakenName` cover the browser login, device code login and `auth token` flows. Like the other picker tests, they are skipped on Windows. - `TestGetSectionNames` covers sections without a host and empty sections. - `TestValidateProfileName` and `TestSaveToProfile_SquareBracketsRoundTrip` cover line breaks, and check that names with square brackets are still accepted and parse back. - `cmd/auth` unit tests and `TestAccept/cmd/auth` pass. This pull request and its description were written by Isaac. --------- Co-authored-by: Isaac <no-reply@databricks.com>
idelafuente-oc
pushed a commit
to idelafuente-oc/cli
that referenced
this pull request
Nov 11, 2026
## Changes `databricks auth login` and `databricks auth token` reject a host that is not an http(s) URL with a hostname, e.g. `/`, `?o=123`, `https://` or `ftp://x.test`, with `invalid host "<input>". Expected a URL like https://<databricks-instance>.cloud.databricks.com`. The check covers `--host`, the positional argument, profile hosts and the host prompt, which shows the error and asks again. `auth login` runs it before the keyring probe and the profile name prompt. Behavior changes: - An empty host after "Enter a host URL manually" in the login profile picker no longer starts discovery login. - `auth login --skip-workspace --host '?a=123'` fails. On main this started discovery login. - Spaces around a host are trimmed, so `" https://x.test/?o=1 "` works and keeps the workspace ID. ## Why Follow-up to a review comment on databricks#7179. An invalid host made login and token resolve the host from `DATABRICKS_HOST`, save a profile without a `host` key, connect to a host named `https`, or fail later with `host must start with 'https://': `. ## Tests Unit tests cover host validation for each host source, the host prompt, and the early rejection in browser, device code and `--skip-workspace` login. New acceptance tests check the error output of `auth login` and `auth token`. This pull request and its description were written by Isaac. --------- Co-authored-by: Isaac <no-reply@databricks.com>
tedjt
pushed a commit
to tedjt/cli
that referenced
this pull request
Nov 12, 2026
…tabricks#7506) ## Changes In `termtest.Run`, the goroutine running the cmdio call now sends `(value, err)` on `done` before it closes the stdin pipe's read end. Before this change it closed the pipe first. ## Why On Windows, bubbletea wraps a non-console input in cancelreader's fallback reader, and that reader cannot cancel a read in progress (`Cancel()` returns false). After the prompt quits, bubbletea's read loop can still be blocked in `ReadFile` on the pipe. Go's `(*os.File).Close` on Windows calls `CancelIoEx` on pipes, but it does nothing when the read loop hasn't entered `ReadFile` yet. Close then waits for the outstanding read, which only ends when `Term.Close` (run in `t.Cleanup`) closes the write end. That happens only after `Result()` has given up, so the test fails with `cmdio call did not finish in 5s`, even though the raw output shows bubbletea already restored the terminal. Whether it fails depends on goroutine scheduling, so on a retry it usually passes. Sending the result first lets `Result()` return as soon as the call finishes. `Close` still closes the write end and waits for the goroutine, so nothing leaks. Background: - databricks#5248 introduced the harness with this ordering. - databricks#7071 and later auth PRs (e.g. databricks#7093, databricks#7179) moved many more `cmd/auth` tests onto `termtest.Run`, which is why the flake rate rose. - Tests that start more than one bubbletea program are already skipped on Windows for the same root cause (`bubbletea cannot cancel reads on non-console input on Windows`). This PR does not change that. I decided not to raise `defaultTimeout`, because the wait isn't caused by load: it lasts until cleanup, however long the timeout is. ## Tests - `go test ./libs/cmdio/... ./cmd/auth/` (Linux, also `-race -count=5` for cmdiotest); `GOOS=windows go vet` on the same packages. - Windows unit tests in this PR's CI. Dashboard: | error | key failure lines | |---|---| | [error/2065](http://go/cli/dashboard/error/2065?main=1&pr=1&end=2026-11-11T19:40:00Z&last=30d) | `select_baseline_wrap_around_test.go:39: cmdio call did not finish in 5s; raw output:` … `\u001b[?2004l\u001b[?25h\u001b[?1002l\u001b[?1003l\u001b[?1006l` / `--- FAIL: TestSelectBaseline_WrapAround (5.05s)` | Affected (flaky on Windows, 59 tests), e.g. [cmdiotest.TestSelectBaseline_Scroll](http://go/cli/dashboard/test/libs/cmdio/cmdiotest.TestSelectBaseline_Scroll?main=1&pr=1&end=2026-11-11T19:40:00Z&last=30d), [cmdiotest.TestPromptBaseline_AltKeyNoop](http://go/cli/dashboard/test/libs/cmdio/cmdiotest.TestPromptBaseline_AltKeyNoop?main=1&pr=1&end=2026-11-11T19:40:00Z&last=30d), [auth.TestPromptForWorkspaceSelection/id_beyond_first_fifty](http://go/cli/dashboard/test/cmd/auth.TestPromptForWorkspaceSelection/id_beyond_first_fifty?main=1&pr=1&end=2026-11-11T19:40:00Z&last=30d), [auth.TestLoginDeviceCodeNewProfilePromptsForHost](http://go/cli/dashboard/test/cmd/auth.TestLoginDeviceCodeNewProfilePromptsForHost?main=1&pr=1&end=2026-11-11T19:40:00Z&last=30d). This PR was opened by an AI agent (harness_auto). This pull request and its description were written by Isaac. Co-authored-by: Isaac <no-reply@databricks.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
databricks auth login --use-device-codenow prompts for the workspace host when none is known, instead of failing. Device code login skips discovery, which opens a browser, and letssetHostprompt for the host instead. An empty answer still fails with--use-device-code requires a workspace host. This check only applies to device code login.The picker also hides profiles that device code login always rejects: account-host profiles and profiles saved with
resources. Unified-host profiles are still listed, because the profile doesn't store what identifies them.docs/device-code-login.mdis updated.Why
With no host known, device code login went into discovery and failed. That broke "Create a new profile" in the picker and
auth login --use-device-code --profile <new-profile>. The picker also listed profiles that only failed after you picked them.Tests
termtesttests check that the CLI prompts for the host, rejects an empty host (both after--profileand from the picker), and hides unsupported profiles. The picker empty-host test is skipped on Windows, because bubbletea can't cancel reads on piped stdin there.TestMatchDeviceCodeProfilesandTestIsDeviceCodeSupportedForHostcover the filter and the host check.device-code/unsupported-hostacceptance test now expects the non-interactive host error.No changelog fragment:
--use-device-codeis still behindDATABRICKS_CLI_DEVICE_CODE.This pull request and its description were written by Isaac.