Handle generate config errors cleanly (#169)

Author: httpdss

structkit/commands/generate.py

@@ -10,6 +10,10 @@
 import subprocess
 
 
+class GenerateConfigError(ValueError):
+  """Expected generate config load/shape error shown without traceback."""
+
+
 # Generate command class
 class GenerateCommand(Command):
   def __init__(self, parser):
@@ -111,8 +115,7 @@ def _load_yaml_config(self, structure_definition, structures_path):
       structure_definition = f"file://{structure_definition}"
 
     if structure_definition.startswith("file://") and structure_definition.endswith(".yaml"):
-      with open(structure_definition[7:], 'r') as f:
-        return yaml.safe_load(f)
+      file_path = structure_definition[7:]
     else:
       this_file = os.path.dirname(os.path.realpath(__file__))
       contribs_path = os.path.join(this_file, "..", "contribs")
@@ -121,11 +124,23 @@ def _load_yaml_config(self, structure_definition, structures_path):
         file_path = os.path.join(structures_path, f"{structure_definition}.yaml")
       if not os.path.exists(file_path):
         file_path = os.path.join(contribs_path, f"{structure_definition}.yaml")
-      if not os.path.exists(file_path):
-        self.logger.error(f"❗ File not found: {file_path}")
-        return None
+
+    try:
       with open(file_path, 'r') as f:
         return yaml.safe_load(f)
+    except FileNotFoundError:
+      raise GenerateConfigError(f"File not found: {file_path}") from None
+    except yaml.YAMLError as exc:
+      raise GenerateConfigError(f"Invalid YAML in {file_path}: {exc}") from None
+    except OSError as exc:
+      raise GenerateConfigError(f"Failed to read {file_path}: {exc}") from None
+
+  def _validate_loaded_config(self, config):
+    if config is None:
+      return {}
+    if not isinstance(config, dict):
+      raise GenerateConfigError("Top-level YAML content must be a mapping.")
+    return config
 
   def execute(self, args):
     try:
@@ -164,19 +179,23 @@ def execute(self, args):
           self.logger.error(f"Mappings file not found: {mappings_file_path}")
           return
 
+    # Load and validate config before creating output/backup paths, so config
+    # errors do not leave partial filesystem side effects behind.
+    try:
+      config = self._validate_loaded_config(
+        self._load_yaml_config(args.structure_definition, args.structures_path)
+      )
+    except GenerateConfigError as exc:
+      self.logger.error(f"❗ {exc}")
+      raise SystemExit(1) from None
+
     if args.backup and not os.path.exists(args.backup):
       os.makedirs(args.backup)
 
     if args.base_path and not os.path.exists(args.base_path) and "console" not in args.output:
       self.logger.info(f"Creating base path: {args.base_path}")
       os.makedirs(args.base_path)
 
-    # Load config to check for hooks
-    config = None
-    config = self._load_yaml_config(args.structure_definition, args.structures_path)
-    if config is None:
-      return
-
     pre_hooks = config.get('pre_hooks', [])
     post_hooks = config.get('post_hooks', [])
 
@@ -188,7 +207,7 @@ def execute(self, args):
     # Actually generate structure
     try:
       self._create_structure(args, mappings)
-    except TemplateVariableError as exc:
+    except (GenerateConfigError, TemplateVariableError) as exc:
       self.logger.error(f"❗ {exc}")
       raise SystemExit(1) from None
 
@@ -201,9 +220,9 @@ def _create_structure(self, args, mappings=None, summary=None, print_summary=Tru
     if isinstance(args, dict):
         args = argparse.Namespace(**args)
 
-    config = self._load_yaml_config(args.structure_definition, args.structures_path)
-    if config is None:
-      return summary if summary is not None else None
+    config = self._validate_loaded_config(
+      self._load_yaml_config(args.structure_definition, args.structures_path)
+    )
 
     # Safely parse template variables
     template_vars = self._parse_template_vars(args.vars) if getattr(args, 'vars', None) else {}

structkit/mcp_server.py

@@ -178,13 +178,19 @@ class Args:
                 GenerateCommand(dummy_parser).execute(args)
                 text = buf.getvalue()
                 return text.strip() or "Structure generation completed successfully"
+            except SystemExit as exc:
+                text = buf.getvalue().strip()
+                return text or f"Error: structure generation failed with exit code {exc.code}"
             finally:
                 sys.stdout = old
         else:
             # Create a dummy parser for GenerateCommand
             import argparse
             dummy_parser = argparse.ArgumentParser()
-            GenerateCommand(dummy_parser).execute(args)
+            try:
+                GenerateCommand(dummy_parser).execute(args)
+            except SystemExit as exc:
+                return f"Error: structure generation failed with exit code {exc.code}"
             if dry_run:
                 return f"Dry run completed for structure '{structure_definition}' at '{base_path}'"
             return f"Structure '{structure_definition}' generated successfully at '{base_path}'"

tests/test_commands_more.py

@@ -134,6 +134,53 @@ def test_generate_reports_template_variable_error_without_traceback(parser, tmp_
     assert "Traceback" not in caplog.text
 
 
+def test_generate_invalid_yaml_reports_clean_error_without_side_effect(parser, tmp_path, caplog):
+    command = GenerateCommand(parser)
+    invalid = tmp_path / 'invalid.yaml'
+    invalid.write_text('files: [\n')
+    out_dir = tmp_path / 'out-invalid'
+    args = parser.parse_args(['--non-interactive', str(invalid), str(out_dir)])
+
+    with pytest.raises(SystemExit) as excinfo:
+        command.execute(args)
+
+    assert excinfo.value.code == 1
+    assert f"Invalid YAML in {invalid}" in caplog.text
+    assert "Traceback" not in caplog.text
+    assert not out_dir.exists()
+
+
+def test_generate_top_level_non_mapping_reports_clean_error_without_side_effect(parser, tmp_path, caplog):
+    command = GenerateCommand(parser)
+    non_mapping = tmp_path / 'list.yaml'
+    non_mapping.write_text('- item\n')
+    out_dir = tmp_path / 'out-list'
+    args = parser.parse_args(['--non-interactive', str(non_mapping), str(out_dir)])
+
+    with pytest.raises(SystemExit) as excinfo:
+        command.execute(args)
+
+    assert excinfo.value.code == 1
+    assert "Top-level YAML content must be a mapping." in caplog.text
+    assert "Traceback" not in caplog.text
+    assert not out_dir.exists()
+
+
+def test_generate_missing_file_reports_clean_error_without_side_effect(parser, tmp_path, caplog):
+    command = GenerateCommand(parser)
+    missing = tmp_path / 'missing.yaml'
+    out_dir = tmp_path / 'out-missing'
+    args = parser.parse_args(['--non-interactive', str(missing), str(out_dir)])
+
+    with pytest.raises(SystemExit) as excinfo:
+        command.execute(args)
+
+    assert excinfo.value.code == 1
+    assert f"File not found: {missing}" in caplog.text
+    assert "Traceback" not in caplog.text
+    assert not out_dir.exists()
+
+
 def test_info_nonexistent_file_logs_error(parser):
     command = InfoCommand(parser)
     args = parser.parse_args(['does-not-exist'])