Uh oh!
There was an error while loading. Please reload this page.
fix(no-ticket): degrade gracefully when auth can't open a browser (#194) - #350
Open
BartoszBlizniak wants to merge 4 commits into
Open
fix(no-ticket): degrade gracefully when auth can't open a browser (#194)#350BartoszBlizniak wants to merge 4 commits into
BartoszBlizniak wants to merge 4 commits into
Conversation
…oudsmith-io#194) cloudsmith auth's SAML flow called webbrowser.open() with no error handling, so on Cygwin and similar environments without a runnable browser it crashed with an uncaught webbrowser.Error traceback instead of falling back to the already-printed IdP URL. Wrap the call and print a manual-open hint instead of crashing, and add a --no-browser flag to skip the auto-open step entirely for callers who know upfront they're in that kind of environment.
…th-io#194) Add CHANGELOG entries and update the README's SAML section, which still showed the "Opening your organization's SAML IDP URL in your browser" banner this change replaced. Also note the callback binds to 127.0.0.1:12400, so the browser must be on the same machine. Drop the dead pylint pragma (the repo runs ruff, which already ignores BLE001 repo-wide), trim the --no-browser help to one sentence, and remove a no-op mock assignment in the webbrowser.Error test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds graceful browser-launch fallback to SAML authentication.
Changes:
- Adds
--no-browsertoauth/authenticate. - Handles browser launch failures while preserving callback flow.
- Adds regression tests and documentation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
README.md | Documents manual browser authentication. |
cloudsmith_cli/cli/commands/auth.py | Implements browser fallback and option. |
cloudsmith_cli/cli/tests/commands/test_auth.py | Tests launch and fallback behavior. |
CHANGELOG.md | Records the feature and fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
BartoszBlizniak
marked this pull request as ready for review
August 14, 2026 12:08
cloudsmith-iduffy
approved these changes
Aug 14, 2026
cloudsmith-iduffy
left a comment
Contributor
There was a problem hiding this comment.
Nice addition and makes sense.
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 freeto 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.
Description
Addresses the Cygwin and same-host headless-shell portion of issue #194.
cloudsmith authnow handles both exceptions and falsey results fromwebbrowser.open()by showing a manual-open fallback and continuing to wait for the existing localhost SAML callback. A new--no-browseroption on bothcloudsmith authandcloudsmith authenticateskips automatic browser launch and presents the IdP URL for the user to open manually. The URL banner is neutral in every mode, and existing informational output routing is preserved. This does not add support for remote or fully browserless authentication: the callback still binds to127.0.0.1:12400, and a device-authorization-style flow requires backend support outside this change.The README's SAML section is updated for the new banner text, documents
--no-browserand the automatic fallback, and now states the same-machine constraint on the callback. CHANGELOG has Added/Fixed entries under Unreleased.Type of Change
Additional Notes
Regression coverage exercises the registered top-level Click command tree for
authandauthenticate, includingwebbrowser.Error, an unexpected browser-launch exception, a false return value,--no-browser, successful automatic launch, and continuation intoAuthenticationWebServer.handle_request(). Focused auth tests pass (14 passed), the full suite passes (632 passed, 40 skipped), andpre-commit run --all-filespasses. Both command names expose--no-browserin generated local help. A live SAML round trip was not performed because no SSO-enabled test organization was available; browser-launch behavior and callback continuation are covered with local mocks, and no external service was modified.