fix: address CR feedback on CLI entry point

- Use 'if val is not None' instead of 'if val' for boolean flags
  to handle future default=True cases correctly
- Refactor cmd_run to delegate to cmd_execute and cmd_collect
  instead of reimplementing pipeline logic inline
- Add --attach flag to civ run for reusing existing infrastructure

Author: sshmulev

cli.py

@@ -9,7 +9,7 @@
 
 from core.config import CoreConfig
 from core.executor import execute
-from core.metadata import InstanceMetadata
+from core.metadata import InstanceMetadata, write_instances_json
 from core.provisioner import Provisioner
 from core.results import get_exit_code, merge_results
 
@@ -39,7 +39,7 @@ def _build_config(args: argparse.Namespace) -> CoreConfig:
 
     for attr in ("debug", "parallel", "stop_cleanup"):
         val = getattr(args, attr, None)
-        if val:
+        if val is not None:
             config_dict[attr] = val
 
     if getattr(args, "tags", None):
@@ -124,37 +124,32 @@ def cmd_run(args: argparse.Namespace) -> int:
     config = _build_config(args)
     provisioner = Provisioner(config)
     try:
-        instances = provisioner.provision()
-        provisioner.prepare_environment(instances)
+        if getattr(args, "attach", False):
+            instances = provisioner.get_instances()
+            write_instances_json(instances, config.instances_json)
+        else:
+            instances = provisioner.provision()
+            provisioner.prepare_environment(instances)
 
         command = getattr(args, "command", None) or _build_test_command(config)
         results_dir = os.path.dirname(config.output_file) or "/tmp"
-        exec_result = execute(
-            instances=instances,
+
+        exec_args = argparse.Namespace(
+            instances_json=config.instances_json,
             command=command,
             ssh_config=config.ssh_config_file,
             results_dir=results_dir,
             timeout=getattr(args, "timeout", 3600),
         )
+        exit_code = cmd_execute(exec_args)
 
-        for ir in exec_result.instance_results:
-            status = "PASS" if ir.exit_code == 0 else "FAIL"
-            print(f"  [{status}] {ir.instance_name} (exit={ir.exit_code})")
-
-        result_files = [
-            r.result_file for r in exec_result.instance_results if r.result_file
-        ]
-        if not result_files:
-            print("No result files collected")
-            return 1
-
-        merge_result = merge_results(result_files, config.output_file)
-        print(
-            f"Merged {merge_result.total_tests} tests: "
-            f"{merge_result.failures} failures, "
-            f"{merge_result.errors} errors"
+        collect_args = argparse.Namespace(
+            results_dir=results_dir,
+            output_file=config.output_file,
         )
-        return get_exit_code(merge_result)
+        collect_code = cmd_collect(collect_args)
+
+        return collect_code if collect_code != 0 else exit_code
 
     except Exception as exc:
         print(f"Error: {exc}")
@@ -250,6 +245,8 @@ def build_parser() -> argparse.ArgumentParser:
                        help="Custom command (overrides test flags)")
     p_run.add_argument("--timeout", type=int, default=3600,
                        help="Timeout in seconds (default: 3600)")
+    p_run.add_argument("-a", "--attach", action="store_true", default=False,
+                       help="Attach to existing infrastructure instead of provisioning")
     p_run.set_defaults(func=cmd_run)
 
     # --- cleanup ---

test/test_cli.py

@@ -55,6 +55,7 @@ def _run_args(**overrides):
         "tags": None,
         "command": None,
         "timeout": 3600,
+        "attach": False,
     }
     defaults.update(overrides)
     return Namespace(**defaults)
@@ -174,7 +175,7 @@ def test_run_subcommand_all_flags(self):
             "-t", "test_ssh", "--test-suites", "a/", "b/",
             "-m", "pub", "-p", "-d", "-s", "-e", "automated",
             "--tags", "env:prod", "--command", "echo hi",
-            "--timeout", "600",
+            "--timeout", "600", "-a",
         ])
         assert args.resources_file == "resources.json"
         assert args.test_filter == "test_ssh"
@@ -187,6 +188,7 @@ def test_run_subcommand_all_flags(self):
         assert args.tags == "env:prod"
         assert args.command == "echo hi"
         assert args.timeout == 600
+        assert args.attach is True
 
     def test_cleanup_subcommand(self):
         parser = build_parser()
@@ -310,70 +312,42 @@ def test_returns_two_when_no_files(self, mock_glob):
 
 
 class TestCmdRun:
-    @patch("cli.merge_results")
-    @patch("cli.execute")
+    @patch("cli.cmd_collect", return_value=0)
+    @patch("cli.cmd_execute", return_value=0)
     @patch("cli.Provisioner")
-    def test_full_pipeline_success(self, MockProv, mock_execute, mock_merge):
+    def test_full_pipeline_success(self, MockProv, mock_exec, mock_coll):
         mock_prov = MockProv.return_value
         mock_prov.provision.return_value = {"inst1": MagicMock()}
 
-        mock_ir = MagicMock()
-        mock_ir.exit_code = 0
-        mock_ir.instance_name = "inst1"
-        mock_ir.result_file = "/tmp/r/inst1.xml"
-        mock_exec_result = MagicMock()
-        mock_exec_result.instance_results = [mock_ir]
-        mock_execute.return_value = mock_exec_result
-
-        mock_merge.return_value = MergeResult(
-            total_tests=5, failures=0, errors=0, skipped=0, time=1.0,
-        )
-
         args = _run_args()
         assert cmd_run(args) == 0
         mock_prov.provision.assert_called_once()
         mock_prov.prepare_environment.assert_called_once()
+        mock_exec.assert_called_once()
+        mock_coll.assert_called_once()
         mock_prov.cleanup.assert_called_once()
 
-    @patch("cli.merge_results")
-    @patch("cli.execute")
+    @patch("cli.cmd_collect", return_value=1)
+    @patch("cli.cmd_execute", return_value=0)
     @patch("cli.Provisioner")
-    def test_returns_exit_code_from_merge(self, MockProv, mock_execute, mock_merge):
+    def test_returns_collect_exit_code(self, MockProv, mock_exec, mock_coll):
         mock_prov = MockProv.return_value
         mock_prov.provision.return_value = {"inst1": MagicMock()}
 
-        mock_ir = MagicMock()
-        mock_ir.exit_code = 0
-        mock_ir.instance_name = "inst1"
-        mock_ir.result_file = "/tmp/r/inst1.xml"
-        mock_exec_result = MagicMock()
-        mock_exec_result.instance_results = [mock_ir]
-        mock_execute.return_value = mock_exec_result
-
-        mock_merge.return_value = MergeResult(
-            total_tests=5, failures=2, errors=0, skipped=0, time=1.0,
-        )
-
         args = _run_args()
         assert cmd_run(args) == 1
 
-    @patch("cli.execute")
+    @patch("cli.cmd_collect", return_value=0)
+    @patch("cli.cmd_execute", return_value=1)
     @patch("cli.Provisioner")
-    def test_returns_one_when_no_result_files(self, MockProv, mock_execute):
+    def test_returns_execute_exit_code_when_collect_passes(
+        self, MockProv, mock_exec, mock_coll,
+    ):
         mock_prov = MockProv.return_value
         mock_prov.provision.return_value = {"inst1": MagicMock()}
 
-        mock_ir = MagicMock()
-        mock_ir.exit_code = 1
-        mock_ir.instance_name = "inst1"
-        mock_ir.result_file = None
-        mock_exec_result = MagicMock()
-        mock_exec_result.instance_results = [mock_ir]
-        mock_execute.return_value = mock_exec_result
-
         args = _run_args()
         assert cmd_run(args) == 1
-        mock_prov.cleanup.assert_called_once()
 
     @patch("cli.Provisioner")
     def test_returns_100_on_infra_error(self, MockProv):
@@ -384,48 +358,43 @@ def test_returns_100_on_infra_error(self, MockProv):
         assert cmd_run(args) == 100
         mock_prov.cleanup.assert_called_once()
 
-    @patch("cli.execute")
+    @patch("cli.cmd_collect", return_value=0)
+    @patch("cli.cmd_execute", return_value=0)
     @patch("cli.Provisioner")
-    def test_skips_cleanup_when_stop_cleanup(self, MockProv, mock_execute):
+    def test_skips_cleanup_when_stop_cleanup(self, MockProv, mock_exec, mock_coll):
         mock_prov = MockProv.return_value
         mock_prov.provision.return_value = {"inst1": MagicMock()}
 
-        mock_ir = MagicMock()
-        mock_ir.result_file = None
-        mock_ir.exit_code = 0
-        mock_ir.instance_name = "inst1"
-        mock_exec_result = MagicMock()
-        mock_exec_result.instance_results = [mock_ir]
-        mock_execute.return_value = mock_exec_result
-
         args = _run_args(stop_cleanup=True)
         cmd_run(args)
         mock_prov.cleanup.assert_not_called()
 
-    @patch("cli.merge_results")
-    @patch("cli.execute")
+    @patch("cli.cmd_collect", return_value=0)
+    @patch("cli.cmd_execute", return_value=0)
     @patch("cli.Provisioner")
-    def test_uses_custom_command(self, MockProv, mock_execute, mock_merge):
+    def test_passes_custom_command(self, MockProv, mock_exec, mock_coll):
         mock_prov = MockProv.return_value
         mock_prov.provision.return_value = {"inst1": MagicMock()}
 
-        mock_ir = MagicMock()
-        mock_ir.exit_code = 0
-        mock_ir.instance_name = "inst1"
-        mock_ir.result_file = "/tmp/r/inst1.xml"
-        mock_exec_result = MagicMock()
-        mock_exec_result.instance_results = [mock_ir]
-        mock_execute.return_value = mock_exec_result
-
-        mock_merge.return_value = MergeResult(
-            total_tests=1, failures=0, errors=0, skipped=0, time=0.5,
-        )
-
         args = _run_args(command="echo hi")
         cmd_run(args)
-        mock_execute.assert_called_once()
-        call_kwargs = mock_execute.call_args
-        assert call_kwargs[1]["command"] == "echo hi"
+        exec_call_args = mock_exec.call_args[0][0]
+        assert exec_call_args.command == "echo hi"
+
+    @patch("cli.cmd_collect", return_value=0)
+    @patch("cli.cmd_execute", return_value=0)
+    @patch("cli.write_instances_json")
+    @patch("cli.Provisioner")
+    def test_attach_skips_provision(self, MockProv, mock_write, mock_exec, mock_coll):
+        mock_prov = MockProv.return_value
+        mock_prov.get_instances.return_value = {"inst1": MagicMock()}
+
+        args = _run_args(attach=True)
+        assert cmd_run(args) == 0
+        mock_prov.provision.assert_not_called()
+        mock_prov.prepare_environment.assert_not_called()
+        mock_prov.get_instances.assert_called_once()
+        mock_write.assert_called_once()
 
 
 class TestCmdCleanup: