fix: awxkit help flags for detailed & general help

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>

awxkit/awxkit/cli/client.py

@@ -80,6 +80,28 @@ class CLI(object):
     def help(self):
         return '--help' in self.argv or '-h' in self.argv
 
+    def _get_non_option_args(self, before_help=False):
+        """Extract non-option arguments from argv, optionally only those before help flag."""
+        if before_help and self.help:
+            # Find position of help flag
+            help_pos = next((i for i, arg in enumerate(self.argv) if arg in ('--help', '-h')), len(self.argv))
+            args_to_check = self.argv[:help_pos]
+        else:
+            args_to_check = self.argv
+
+        return [arg for arg in args_to_check if not arg.startswith('-') and arg != 'awx']
+
+    def _is_main_help_request(self):
+        """
+        Determine if help request is for main CLI (awx --help) vs subcommand (awx users create --help).
+        Returns True if this is a main CLI help request that should exit early.
+        """
+        if not self.help:
+            return False
+
+        # If there are non-option arguments before help flag, this is subcommand help
+        return len(self._get_non_option_args(before_help=True)) == 0
+
     def authenticate(self):
         """Configure the current session for authentication.
 
@@ -224,6 +246,16 @@ class CLI(object):
         subparsers = self.subparsers[self.resource].add_subparsers(dest='action', metavar='action')
         subparsers.required = True
 
+        # Add manual help handling for resource-level help
+        # since we disabled add_help=False for resource subparsers
+        if self.help:
+            # Check if this is resource-level help (no action specified)
+            non_option_args = self._get_non_option_args()
+            if len(non_option_args) == 1 and non_option_args[0] == self.resource:
+                # Only resource specified, no action - show resource-level help
+                self.subparsers[self.resource].print_help()
+                return
+
         # parse the action from OPTIONS
         parser = ResourceOptionsParser(self.v2, page, self.resource, subparsers)
         if parser.deprecated:
@@ -232,6 +264,18 @@ class CLI(object):
                 description = colored(description, 'yellow')
             self.subparsers[self.resource].description = description
 
+        # parse any action arguments FIRST before attempting to parse
+        if self.resource != 'settings':
+            for method in ('list', 'modify', 'create'):
+                if method in parser.parser.choices:
+                    if method == 'list':
+                        http_method = 'GET'
+                    elif method == 'modify' and 'PUT' in parser.options:
+                        http_method = 'PUT'
+                    else:
+                        http_method = 'POST'
+                    parser.build_query_arguments(method, http_method)
+
         if from_sphinx:
             # Our Sphinx plugin runs `parse_action` for *every* available
             # resource + action in the API so that it can generate usage
@@ -244,23 +288,15 @@ class CLI(object):
                 self.parser.parse_known_args(self.argv)[0]
             except SystemExit:
                 pass
-        else:
-            self.parser.parse_known_args()[0]
-
-        # parse any action arguments
-        if self.resource != 'settings':
-            for method in ('list', 'modify', 'create'):
-                if method in parser.parser.choices:
-                    if method == 'list':
-                        http_method = 'GET'
-                    elif method == 'modify' and 'PUT' in parser.options:
-                        http_method = 'PUT'
-                    else:
-                        http_method = 'POST'
-                    parser.build_query_arguments(method, http_method)
-        if from_sphinx:
             parsed, extra = self.parser.parse_known_args(self.argv)
         else:
+            # Try to parse to determine if help is requested for a specific action
+            try:
+                self.parser.parse_known_args()[0]
+            except SystemExit:
+                # This happens when required arguments are missing, which is expected behavior
+                # Let argparse handle it and show the appropriate help
+                raise
             parsed, extra = self.parser.parse_known_args()
 
         if extra and self.verbose:
@@ -313,6 +349,7 @@ class CLI(object):
         self.argv = argv
         self.parser = HelpfulArgumentParser(add_help=False)
         self.parser.add_argument(
+            '-h',
             '--help',
             action='store_true',
             help='prints usage information for the awx tool',
@@ -322,6 +359,13 @@ class CLI(object):
         add_output_formatting_arguments(self.parser, env)
 
         self.args = self.parser.parse_known_args(self.argv)[0]
+
+        # Early return for help to avoid server connection, but only for main CLI help
+        # Allow subcommand help (like 'awx users create --help') to continue processing
+        if self.help and self._is_main_help_request():
+            self.parser.print_help()
+            sys.exit(0)
+
         self.verbose = self.get_config('verbose')
         if self.verbose:
             logging.basicConfig(level='DEBUG')

awxkit/awxkit/cli/options.py

@@ -126,7 +126,7 @@ class ResourceOptionsParser(object):
             if method not in action_map:
                 continue
             method = action_map[method]
-            parser = self.parser.add_parser(method, help='')
+            parser = self.parser.add_parser(method, help='', add_help=True)
             if method == 'list':
                 parser.add_argument(
                     '--all',
@@ -152,7 +152,7 @@ class ResourceOptionsParser(object):
         if 'DELETE' in self.allowed_options:
             allowed.append('delete')
         for method in allowed:
-            parser = self.parser.add_parser(method, help='')
+            parser = self.parser.add_parser(method, help='', add_help=True)
             self.parser.choices[method].add_argument(
                 'id', type=functools.partial(pk_or_name, self.v2, self.resource), help='the ID (or unique name) of the resource'
             )

awxkit/awxkit/cli/resource.py

@@ -167,7 +167,9 @@ def parse_resource(client, skip_deprecated=False):
                 if k in DEPRECATED_RESOURCES:
                     kwargs['aliases'] = [DEPRECATED_RESOURCES[k]]
 
-            client.subparsers[k] = subparsers.add_parser(k, help='', **kwargs)
+            # Disable automatic help handling for resource subparsers to prevent
+            # premature help display before action subparsers are built
+            client.subparsers[k] = subparsers.add_parser(k, help='', add_help=False, **kwargs)
 
     resource = client.parser.parse_known_args()[0].resource
     if resource in DEPRECATED_RESOURCES.values():

awxkit/test/cli/test_client.py

@@ -1,5 +1,7 @@
 import pytest
 from requests.exceptions import ConnectionError
+import sys
+from unittest.mock import patch
 
 from awxkit.cli import run, CLI
 
@@ -53,3 +55,136 @@ def test_list_resources(capfd, resource):
     assert "usage:" in out
     for snippet in ('--conf.host https://example.awx.org]', '-v, --verbose'):
         assert snippet in out
+
+
+class TestHelpHandling:
+    """Test suite for improved help handling functionality"""
+
+    def test_get_non_option_args_basic(self):
+        """Test _get_non_option_args extracts non-option arguments correctly"""
+        cli = CLI()
+        cli.argv = ['awx', 'users', 'list', '--verbose']
+
+        result = cli._get_non_option_args()
+        assert result == ['users', 'list']
+
+    def test_get_non_option_args_with_flags(self):
+        """Test _get_non_option_args ignores option flags"""
+        cli = CLI()
+        cli.argv = ['awx', '--conf.host', 'example.com', 'jobs', 'create', '--name', 'test']
+
+        result = cli._get_non_option_args()
+        # Should include all non-option arguments (including flag values) except 'awx'
+        assert result == ['example.com', 'jobs', 'create', 'test']
+
+    def test_get_non_option_args_before_help(self):
+        """Test _get_non_option_args with before_help=True stops at help flag"""
+        cli = CLI()
+        cli.argv = ['awx', 'users', '--help', 'extra', 'args']
+
+        result = cli._get_non_option_args(before_help=True)
+        assert result == ['users']
+
+    def test_get_non_option_args_before_help_short_flag(self):
+        """Test _get_non_option_args with before_help=True stops at -h flag"""
+        cli = CLI()
+        cli.argv = ['awx', 'projects', '-h', 'should', 'not', 'appear']
+
+        result = cli._get_non_option_args(before_help=True)
+        assert result == ['projects']
+
+    def test_get_non_option_args_no_help_flag(self):
+        """Test _get_non_option_args when help flag not present"""
+        cli = CLI()
+        cli.argv = ['awx', 'organizations', 'list']
+
+        result = cli._get_non_option_args(before_help=True)
+        assert result == ['organizations', 'list']
+
+    def test_is_main_help_request_true(self):
+        """Test _is_main_help_request returns True for main CLI help"""
+        cli = CLI()
+        cli.argv = ['awx', '--help']
+
+        result = cli._is_main_help_request()
+        assert result is True
+
+    def test_is_main_help_request_short_flag(self):
+        """Test _is_main_help_request returns True for main CLI help with -h"""
+        cli = CLI()
+        cli.argv = ['awx', '-h']
+
+        result = cli._is_main_help_request()
+        assert result is True
+
+    def test_is_main_help_request_false_subcommand(self):
+        """Test _is_main_help_request returns False for subcommand help"""
+        cli = CLI()
+        cli.argv = ['awx', 'users', '--help']
+
+        result = cli._is_main_help_request()
+        assert result is False
+
+    def test_is_main_help_request_false_action(self):
+        """Test _is_main_help_request returns False for action help"""
+        cli = CLI()
+        cli.argv = ['awx', 'jobs', 'create', '--help']
+
+        result = cli._is_main_help_request()
+        assert result is False
+
+    def test_is_main_help_request_false_no_help(self):
+        """Test _is_main_help_request returns False when no help flag"""
+        cli = CLI()
+        cli.argv = ['awx', 'users', 'list']
+
+        result = cli._is_main_help_request()
+        assert result is False
+
+    def test_early_help_return_main_cli(self):
+        """Test that main CLI help exits early without server connection"""
+        cli = CLI()
+        # Verify that _is_main_help_request works correctly
+        cli.argv = ['awx', '--help']
+        assert cli._is_main_help_request() is True
+
+        # Test that parse_args with main help flag should exit
+        with patch.object(sys, 'exit') as mock_exit:
+            cli.parse_args(['awx', '--help'])
+            mock_exit.assert_called_once_with(0)
+
+    def test_no_early_exit_for_subcommand_help(self):
+        """Test that subcommand help does not exit early"""
+        with patch.object(sys, 'exit') as mock_exit:
+            cli = CLI()
+            # This should not exit early since it's subcommand help
+            cli.parse_args(['awx', 'users', '--help'])
+
+            mock_exit.assert_not_called()
+
+    def test_help_property_detection(self):
+        """Test that help property correctly detects help flags"""
+        cli = CLI()
+
+        cli.argv = ['awx', '--help']
+        assert cli.help is True
+
+        cli.argv = ['awx', '-h']
+        assert cli.help is True
+
+        cli.argv = ['awx', 'users', '--help']
+        assert cli.help is True
+
+        cli.argv = ['awx', 'users', 'list']
+        assert cli.help is False
+
+    def test_short_help_flag_added(self):
+        """Test that -h flag is properly added to argument parser"""
+        cli = CLI()
+        cli.parse_args(['awx'])
+
+        # Verify that both -h and --help are recognized
+        help_actions = [action for action in cli.parser._actions if '--help' in action.option_strings]
+        assert len(help_actions) == 1
+        assert '-h' in help_actions[0].option_strings
+        assert '--help' in help_actions[0].option_strings

awxkit/test/cli/test_options.py

@@ -251,3 +251,24 @@ class TestSettingsOptions(unittest.TestCase):
         out = StringIO()
         self.parser.choices['modify'].print_help(out)
         assert 'modify [-h] key value' in out.getvalue()
+
+
+class TestHelpParameterChanges(unittest.TestCase):
+    """Test that add_help parameter changes work correctly"""
+
+    def test_add_help_parameter_handling(self):
+        """Test that add_help=True and add_help=False work as expected"""
+        # Test add_help=True (for action parsers like list, create)
+        parser = argparse.ArgumentParser()
+        subparsers = parser.add_subparsers(dest='action')
+
+        list_parser = subparsers.add_parser('list', help='', add_help=True)
+        out = StringIO()
+        list_parser.print_help(out)
+        help_text = out.getvalue()
+        assert 'show this help message and exit' in help_text
+
+        # Test add_help=False (for resource parsers like users, jobs)
+        resource_parser = subparsers.add_parser('users', help='', add_help=False)
+        help_actions = [action for action in resource_parser._actions if '--help' in action.option_strings]
+        assert len(help_actions) == 0  # Should be 0 because add_help=False