feat: Allow passing the server URL positionally for login & logout (#796)

This commit adjusts the `login` and `logout` subcommands to accept the
Connect server URL as a positional argument (in addition to the
-s/--server option), making them more intuitive to use:

    $ rsconnect login https://connect.company.internal

rather than

    $ rsconnect login -s https://connect.company.internal

Unit tests are included.

Signed-off-by: Aaron Jacobs <aaron.jacobs@posit.co>

Author: atheriel

rsconnect/main.py

@@ -1042,7 +1042,8 @@ def remove(
     ),
     no_args_is_help=True,
 )
-@click.option("--server", "-s", envvar="CONNECT_SERVER", required=True, help="The URL of the Posit Connect server.")
+@click.argument("server_arg", metavar="SERVER", required=False)
+@click.option("--server", "-s", envvar="CONNECT_SERVER", help="The URL of the Posit Connect server.")
 @click.option("--name", "-n", help="Nickname for the server (defaults to server hostname).")
 @click.option("--insecure", "-i", envvar="CONNECT_INSECURE", is_flag=True, help="Disable TLS certificate verification.")
 @click.option(
@@ -1068,7 +1069,8 @@ def remove(
 @click.option("--verbose", "-v", count=True, help="Enable verbose output. Use -vv for very verbose (debug) output.")
 @cli_exception_handler
 def login(
-    server: str,
+    server_arg: Optional[str],
+    server: Optional[str],
     name: Optional[str],
     insecure: bool,
     cacert: Optional[str],
@@ -1079,6 +1081,17 @@ def login(
 ):
     set_verbosity(verbose)
 
+    # Only treat --server as conflicting with the positional argument when it
+    # was given explicitly on the command line. A value sourced from the
+    # CONNECT_SERVER environment variable should not block the positional form.
+    server_source = validation.get_parameter_source_name_from_ctx("server", click.get_current_context())
+    server_from_option = server_source == "COMMANDLINE"
+    if server_arg and server and server_from_option:
+        raise RSConnectException("You must specify only one of SERVER or -s/--server.")
+    server = server_arg or server
+    if not server:
+        raise RSConnectException("You must specify the server as a SERVER argument or with -s/--server.")
+
     if not server.startswith("http"):
         raise RSConnectException("Server URL must begin with http or https.")
 
@@ -1175,27 +1188,33 @@ def _do_login(cid: str) -> dict[str, Any]:
     short_help="Remove stored OAuth credentials for a Posit Connect server.",
     help=(
         "Remove locally-stored OAuth credentials for a Posit Connect server. "
-        "One of --name or --server is required. "
+        "The server is identified by a positional SERVER argument, -s/--server, or -n/--name. "
         "The server entry is preserved (for re-login without re-registration); "
         "use 'rsconnect remove' to delete the entry entirely."
     ),
     no_args_is_help=True,
 )
+@click.argument("server_arg", metavar="SERVER", required=False)
 @click.option("--name", "-n", help="The nickname of the Posit Connect server to log out from.")
 @click.option("--server", "-s", help="The URL of the Posit Connect server to log out from.")
 @click.option("--verbose", "-v", count=True, help="Enable verbose output. Use -vv for very verbose (debug) output.")
 @cli_exception_handler
 def logout(
+    server_arg: Optional[str],
     name: Optional[str],
     server: Optional[str],
     verbose: int,
 ):
     set_verbosity(verbose)
 
+    if server_arg and server:
+        raise RSConnectException("Specify only one of SERVER or -s/--server.")
+    server = server_arg or server
+
     if name and server:
-        raise RSConnectException("Specify only one of --name or --server.")
+        raise RSConnectException("Specify only one of --name, --server, or SERVER.")
     if not name and not server:
-        raise RSConnectException("Specify one of --name or --server.")
+        raise RSConnectException("Specify one of --name, --server, or SERVER.")
 
     entry = None
     if name:

tests/test_oauth.py

@@ -433,6 +433,71 @@ def test_login_success(
         assert result.exit_code == 0, result.output
         assert "Logged in" in result.output
 
+    @patch("rsconnect.oauth.keyring_store_token", return_value=True)
+    @patch("rsconnect.oauth.login_with_browser")
+    @patch("rsconnect.oauth.register_client", return_value="new-client-id")
+    @patch("rsconnect.oauth.discover_oauth_metadata")
+    def test_login_positional_server(
+        self,
+        mock_discover: MagicMock,
+        mock_register: MagicMock,
+        mock_login: MagicMock,
+        mock_keyring: MagicMock,
+    ):
+        from click.testing import CliRunner
+
+        from rsconnect.main import cli
+
+        mock_discover.return_value = FAKE_METADATA
+        mock_login.return_value = {"access_token": "at-1", "refresh_token": "rt-1", "expires_in": 3600}
+
+        runner = CliRunner()
+        result = runner.invoke(cli, ["login", FAKE_URL, "--name", "test-server"])
+
+        assert result.exit_code == 0, result.output
+        assert "Logged in" in result.output
+
+    @patch("rsconnect.oauth.keyring_store_token", return_value=True)
+    @patch("rsconnect.oauth.login_with_browser")
+    @patch("rsconnect.oauth.register_client", return_value="new-client-id")
+    @patch("rsconnect.oauth.discover_oauth_metadata")
+    def test_login_positional_server_overrides_connect_server_env(
+        self,
+        mock_discover: MagicMock,
+        mock_register: MagicMock,
+        mock_login: MagicMock,
+        mock_keyring: MagicMock,
+    ):
+        from click.testing import CliRunner
+
+        from rsconnect.main import cli
+
+        mock_discover.return_value = FAKE_METADATA
+        mock_login.return_value = {"access_token": "at-1", "refresh_token": "rt-1", "expires_in": 3600}
+
+        runner = CliRunner()
+        result = runner.invoke(
+            cli,
+            ["login", FAKE_URL, "--name", "test-server"],
+            env={"CONNECT_SERVER": "https://env-server.example.com"},
+        )
+
+        assert result.exit_code == 0, result.output
+        assert "Logged in" in result.output
+        # The positional argument should win over the CONNECT_SERVER envvar.
+        assert mock_discover.call_args.args[0] == FAKE_URL
+
+    def test_login_positional_and_option_server_conflict(self):
+        from click.testing import CliRunner
+
+        from rsconnect.main import cli
+
+        runner = CliRunner()
+        result = runner.invoke(cli, ["login", FAKE_URL, "--server", FAKE_URL])
+
+        assert result.exit_code != 0
+        assert "only one of SERVER" in result.output
+
     def test_login_missing_server(self):
         from click.testing import CliRunner
 
@@ -478,6 +543,26 @@ def test_logout_success(self, mock_store: MagicMock, mock_keyring_del: MagicMock
         assert result.exit_code == 0, result.output
         mock_keyring_del.assert_called_once()
 
+    @patch("rsconnect.oauth.keyring_delete_tokens")
+    @patch("rsconnect.main.server_store")
+    def test_logout_positional_server(self, mock_store: MagicMock, mock_keyring_del: MagicMock):
+        from click.testing import CliRunner
+
+        from rsconnect.main import cli
+
+        mock_store.get_by_url.return_value = {
+            "name": "myserver",
+            "url": FAKE_URL,
+            "oauth_client_id": "client-123",
+        }
+        mock_store.update_oauth_tokens = MagicMock()
+
+        runner = CliRunner()
+        result = runner.invoke(cli, ["logout", FAKE_URL])
+
+        assert result.exit_code == 0, result.output
+        mock_keyring_del.assert_called_once()
+
 
 class TestListCommand:
     @patch("rsconnect.oauth.keyring_get_tokens", return_value=("at-from-keyring", None))