From d0b262ce5a56fb6ec9c6c61262d2c9406ee79a64 Mon Sep 17 00:00:00 2001 From: balgaly Date: Wed, 8 Apr 2026 16:52:56 +0300 Subject: [PATCH 1/2] fix(st2client): fix TypeError when sorting action parameters with mixed position types Parameters with a numeric 'position' attribute and those without (falling back to name) cannot be compared with '<' in Python 3, causing an unhelpful TypeError when running 'st2 run action -h' on actions with ordered params. Replace the flat sort key with a 3-tuple (tier, position, name) so that positioned params sort before unpositioned ones and no cross-type comparison is ever attempted. Fixes #5130 --- st2client/st2client/commands/action.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index 48964746a5..bfe9dccd74 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -1206,14 +1206,19 @@ def _get_parameter_sort_value(self, parameters, name): By default, parameters are sorted using "position" parameter attribute. If this attribute is not available, parameter is sorted based on the name. + + Returns a tuple (tier, value) so that parameters with a numeric position + sort before those without, and mixed int/str comparisons are avoided. """ parameter = parameters.get(name, None) if not parameter: - return None + return (1, 0, name) - sort_value = parameter.get("position", name) - return sort_value + position = parameter.get("position", None) + if position is not None: + return (0, int(position), "") + return (1, 0, name) def _get_inherited_env_vars(self): env_vars = os.environ.copy() From ac5077910ac321701d5dfdc8ba8c8d5b10edd999 Mon Sep 17 00:00:00 2001 From: Snir Balgaly Date: Thu, 9 Apr 2026 12:20:29 +0300 Subject: [PATCH 2/2] test(st2client): add unit tests for _sort_parameters mixed position types Regression tests for #5130. Covers the case where some parameters have a numeric 'position' attribute and others do not, which previously caused sorted() to raise TypeError in Python 3. Also covers all-positioned and all-unpositioned parameter lists to confirm existing sort behaviour is preserved. --- .../tests/unit/test_command_actionrun.py | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/st2client/tests/unit/test_command_actionrun.py b/st2client/tests/unit/test_command_actionrun.py index 805491f189..e567333869 100644 --- a/st2client/tests/unit/test_command_actionrun.py +++ b/st2client/tests/unit/test_command_actionrun.py @@ -393,3 +393,77 @@ def test_correctly_generate_empty_params_no_inherit_empty_parameters(self): ) self.assertDictEqual({}, param) + + def test_sort_parameters_mixed_position_types(self): + """Regression test for #5130. + + When some parameters have a numeric 'position' attribute and others do + not, sorted() previously raised TypeError because the sort key returned + either int or str depending on the parameter. Verify that sorting now + succeeds and that positioned parameters come first (by position), with + unpositioned parameters following in alphabetical order. + """ + action = Action() + action.ref = "test.action" + action.parameters = {} + + subparser = mock.Mock() + command = ActionRunCommand(action, self, subparser, name="test") + + parameters = { + "host": {"type": "string", "position": 0}, + "timeout": {"type": "integer", "position": 1}, + "verbose": {"type": "boolean"}, + "output": {"type": "string"}, + } + names = ["verbose", "output", "host", "timeout"] + + # Must not raise TypeError + result = command._sort_parameters(parameters=parameters, names=names) + + # Positioned params come first, in ascending position order + self.assertEqual(result[0], "host") + self.assertEqual(result[1], "timeout") + # Unpositioned params follow, alphabetically + self.assertEqual(result[2], "output") + self.assertEqual(result[3], "verbose") + + def test_sort_parameters_all_positioned(self): + """Parameters that all have a 'position' attribute sort by position.""" + action = Action() + action.ref = "test.action" + action.parameters = {} + + subparser = mock.Mock() + command = ActionRunCommand(action, self, subparser, name="test") + + parameters = { + "z_param": {"type": "string", "position": 2}, + "a_param": {"type": "string", "position": 0}, + "m_param": {"type": "string", "position": 1}, + } + names = ["z_param", "a_param", "m_param"] + + result = command._sort_parameters(parameters=parameters, names=names) + + self.assertEqual(result, ["a_param", "m_param", "z_param"]) + + def test_sort_parameters_none_positioned(self): + """Parameters with no 'position' attribute sort alphabetically by name.""" + action = Action() + action.ref = "test.action" + action.parameters = {} + + subparser = mock.Mock() + command = ActionRunCommand(action, self, subparser, name="test") + + parameters = { + "zebra": {"type": "string"}, + "apple": {"type": "string"}, + "mango": {"type": "string"}, + } + names = ["zebra", "apple", "mango"] + + result = command._sort_parameters(parameters=parameters, names=names) + + self.assertEqual(result, ["apple", "mango", "zebra"])