xcad vor 4 Monaten
Ursprung
Commit
968269fba2
7 geänderte Dateien mit 336 neuen und 62 gelöschten Zeilen
  1. 4 12
      AGENTS.md
  2. 169 4
      cli/core/config.py
  3. 57 20
      cli/core/module.py
  4. 25 5
      cli/core/prompt.py
  5. 24 7
      cli/core/template.py
  6. 47 4
      cli/core/variables.py
  7. 10 10
      library/compose/authentik/template.yaml

+ 4 - 12
AGENTS.md

@@ -248,21 +248,13 @@ After creating the issue, update the TODO line in the `AGENTS.md` file with the
 
 ### Work in Progress
 
-* FIXME We need proper validation to ensure all variable names are unique across all sections (currently allowed but could cause conflicts)
-* FIXME Insufficient Error Messages for Template Loading
-* FIXME Excessive Generic Exception Catching
-* FIXME No Rollback on Config Write Failures: If writing config fails partway through, the config file can be left in a corrupted state. There's no atomic write operation.
-* FIXME Inconsistent Logging Levels: Some important operations use `DEBUG` when they should use `INFO`, and vice versa.
-* TODO Memory Inefficiency in Template File Collection: The template loads all file paths into memory immediately, even when only metadata is needed (like for `list` command). This is wasteful when listing many templates.
-* TODO Missing Input Validation in ConfigManager
+* FIXME Insufficient Error Messages for Template Loading: Error messages during template loading need improvement for better context and debugging.
+* FIXME Excessive Generic Exception Catching: Too much generic exception catching reduces debugging capability. Need to audit and make exception handlers more specific.
+* FIXME Inconsistent Logging Levels: Some important operations use `DEBUG` when they should use `INFO`, and vice versa. Need to audit all logging statements.
 * TODO Add compose deploy command to deploy a generated compose project to a local or remote docker environment
-* TODO No Caching for Module Specs: Each template loads module specs independently. If listing 50 compose templates, the compose module spec is imported 50 times.
 * TODO Missing Type Hints in Some Functions: While most code has type hints, some functions are missing them, reducing IDE support and static analysis capability.
-* TODO No Dry-Run Mode for Generate Command: A dry-run mode would allow users to see what files would be generated without actually writing them to disk.
-* TODO Template Validation Command: A command to validate the structure and variable definitions of a template without generating it.
 * TODO Interactive Variable Prompt Improvements: The interactive prompt could be improved with better navigation, help text, and validation feedback.
-* TODO Better Error Recovery in Jinja2 Rendering
-* FIXME Make sure all outputs in module.py and display.py use the IconManager for consistent icons
+* TODO Better Error Recovery in Jinja2 Rendering: Improve error handling during Jinja2 template rendering with better context and suggestions.
 
 ## Best Practices for Template Development
 

+ 169 - 4
cli/core/config.py

@@ -2,6 +2,9 @@ from __future__ import annotations
 
 import logging
 import os
+import re
+import shutil
+import tempfile
 from pathlib import Path
 from typing import Any, Dict, Optional, Union
 
@@ -13,6 +16,9 @@ from .variables import Variable, VariableSection, VariableCollection
 logger = logging.getLogger(__name__)
 console = Console()
 
+# Valid Python identifier pattern for variable names
+VALID_IDENTIFIER_PATTERN = re.compile(r'^[a-zA-Z_][a-zA-Z0-9_]*$')
+
 class ConfigManager:
     """Manages configuration for the CLI application."""
     
@@ -55,32 +61,131 @@ class ConfigManager:
             
         Raises:
             yaml.YAMLError: If YAML parsing fails.
+            ValueError: If configuration structure is invalid.
         """
         try:
             with open(self.config_path, 'r') as f:
                 config = yaml.safe_load(f) or {}
+            
+            # Validate config structure
+            self._validate_config_structure(config)
+            
             return config
         except yaml.YAMLError as e:
             logger.error(f"Failed to parse YAML configuration: {e}")
             raise
+        except ValueError as e:
+            logger.error(f"Invalid configuration structure: {e}")
+            raise
         except Exception as e:
             logger.error(f"Failed to read configuration file: {e}")
             raise
     
     def _write_config(self, config: Dict[str, Any]) -> None:
-        """Write configuration to file.
+        """Write configuration to file atomically using temp file + rename pattern.
+        
+        This prevents config file corruption if write operation fails partway through.
         
         Args:
             config: Dictionary containing the configuration to write.
+            
+        Raises:
+            ValueError: If configuration structure is invalid.
         """
         try:
-            with open(self.config_path, 'w') as f:
-                yaml.dump(config, f, default_flow_style=False)
-            logger.debug(f"Configuration written to {self.config_path}")
+            # Validate config structure before writing
+            self._validate_config_structure(config)
+            
+            # Ensure parent directory exists
+            self.config_path.parent.mkdir(parents=True, exist_ok=True)
+            
+            # Write to temporary file in same directory for atomic rename
+            with tempfile.NamedTemporaryFile(
+                mode='w',
+                delete=False,
+                dir=self.config_path.parent,
+                prefix='.config_',
+                suffix='.tmp'
+            ) as tmp_file:
+                yaml.dump(config, tmp_file, default_flow_style=False)
+                tmp_path = tmp_file.name
+            
+            # Atomic rename (overwrites existing file on POSIX systems)
+            shutil.move(tmp_path, self.config_path)
+            logger.debug(f"Configuration written atomically to {self.config_path}")
+            
+        except ValueError as e:
+            logger.error(f"Invalid configuration structure: {e}")
+            raise
         except Exception as e:
+            # Clean up temp file if it exists
+            if 'tmp_path' in locals():
+                try:
+                    Path(tmp_path).unlink(missing_ok=True)
+                except Exception:
+                    pass
             logger.error(f"Failed to write configuration file: {e}")
             raise
     
+    def _validate_config_structure(self, config: Dict[str, Any]) -> None:
+        """Validate the configuration structure.
+        
+        Args:
+            config: Configuration dictionary to validate.
+            
+        Raises:
+            ValueError: If configuration structure is invalid.
+        """
+        if not isinstance(config, dict):
+            raise ValueError("Configuration must be a dictionary")
+        
+        # Check top-level structure
+        if "defaults" in config and not isinstance(config["defaults"], dict):
+            raise ValueError("'defaults' must be a dictionary")
+        
+        if "preferences" in config and not isinstance(config["preferences"], dict):
+            raise ValueError("'preferences' must be a dictionary")
+        
+        # Validate defaults structure
+        if "defaults" in config:
+            for module_name, module_defaults in config["defaults"].items():
+                if not isinstance(module_name, str):
+                    raise ValueError(f"Module name must be a string, got {type(module_name).__name__}")
+                
+                if not isinstance(module_defaults, dict):
+                    raise ValueError(f"Defaults for module '{module_name}' must be a dictionary")
+                
+                # Validate variable names are valid Python identifiers
+                for var_name in module_defaults.keys():
+                    if not isinstance(var_name, str):
+                        raise ValueError(f"Variable name must be a string, got {type(var_name).__name__}")
+                    
+                    if not VALID_IDENTIFIER_PATTERN.match(var_name):
+                        raise ValueError(
+                            f"Invalid variable name '{var_name}' in module '{module_name}'. "
+                            f"Variable names must be valid Python identifiers (letters, numbers, underscores, "
+                            f"cannot start with a number)"
+                        )
+        
+        # Validate preferences structure and types
+        if "preferences" in config:
+            preferences = config["preferences"]
+            
+            # Validate known preference types
+            if "editor" in preferences and not isinstance(preferences["editor"], str):
+                raise ValueError("Preference 'editor' must be a string")
+            
+            if "output_dir" in preferences:
+                if preferences["output_dir"] is not None and not isinstance(preferences["output_dir"], str):
+                    raise ValueError("Preference 'output_dir' must be a string or null")
+            
+            if "library_paths" in preferences:
+                if not isinstance(preferences["library_paths"], list):
+                    raise ValueError("Preference 'library_paths' must be a list")
+                
+                for path in preferences["library_paths"]:
+                    if not isinstance(path, str):
+                        raise ValueError(f"Library path must be a string, got {type(path).__name__}")
     
     def get_config_path(self) -> Path:
         """Get the path to the configuration file.
@@ -116,7 +221,29 @@ class ConfigManager:
             module_name: Name of the module
             defaults: Dictionary of defaults (flat key-value pairs):
                       {"var_name": "value", "var2_name": "value2"}
+                      
+        Raises:
+            ValueError: If module name or variable names are invalid.
         """
+        # Validate module name
+        if not isinstance(module_name, str) or not module_name:
+            raise ValueError("Module name must be a non-empty string")
+        
+        # Validate defaults dictionary
+        if not isinstance(defaults, dict):
+            raise ValueError("Defaults must be a dictionary")
+        
+        # Validate variable names
+        for var_name in defaults.keys():
+            if not isinstance(var_name, str):
+                raise ValueError(f"Variable name must be a string, got {type(var_name).__name__}")
+            
+            if not VALID_IDENTIFIER_PATTERN.match(var_name):
+                raise ValueError(
+                    f"Invalid variable name '{var_name}'. Variable names must be valid Python identifiers "
+                    f"(letters, numbers, underscores, cannot start with a number)"
+                )
+        
         config = self._read_config()
         
         if "defaults" not in config:
@@ -133,7 +260,23 @@ class ConfigManager:
             module_name: Name of the module
             var_name: Name of the variable
             value: Default value to set
+            
+        Raises:
+            ValueError: If module name or variable name is invalid.
         """
+        # Validate inputs
+        if not isinstance(module_name, str) or not module_name:
+            raise ValueError("Module name must be a non-empty string")
+        
+        if not isinstance(var_name, str):
+            raise ValueError(f"Variable name must be a string, got {type(var_name).__name__}")
+        
+        if not VALID_IDENTIFIER_PATTERN.match(var_name):
+            raise ValueError(
+                f"Invalid variable name '{var_name}'. Variable names must be valid Python identifiers "
+                f"(letters, numbers, underscores, cannot start with a number)"
+            )
+        
         defaults = self.get_defaults(module_name)
         defaults[var_name] = value
         self.set_defaults(module_name, defaults)
@@ -184,7 +327,29 @@ class ConfigManager:
         Args:
             key: Preference key
             value: Preference value
+            
+        Raises:
+            ValueError: If key or value is invalid for known preference types.
         """
+        # Validate key
+        if not isinstance(key, str) or not key:
+            raise ValueError("Preference key must be a non-empty string")
+        
+        # Validate known preference types
+        if key == "editor" and not isinstance(value, str):
+            raise ValueError("Preference 'editor' must be a string")
+        
+        if key == "output_dir":
+            if value is not None and not isinstance(value, str):
+                raise ValueError("Preference 'output_dir' must be a string or null")
+        
+        if key == "library_paths":
+            if not isinstance(value, list):
+                raise ValueError("Preference 'library_paths' must be a list")
+            for path in value:
+                if not isinstance(path, str):
+                    raise ValueError(f"Library path must be a string, got {type(path).__name__}")
+        
         config = self._read_config()
         
         if "preferences" not in config:

+ 57 - 20
cli/core/module.py

@@ -11,7 +11,7 @@ from rich.panel import Panel
 from rich.prompt import Confirm
 from typer import Argument, Context, Option, Typer, Exit
 
-from .display import DisplayManager
+from .display import DisplayManager, IconManager
 from .library import LibraryManager
 from .prompt import PromptHandler
 from .template import Template
@@ -179,8 +179,9 @@ class Module(ABC):
     id: str = Argument(..., help="Template ID"),
     directory: Optional[str] = Argument(None, help="Output directory (defaults to template ID)"),
     interactive: bool = Option(True, "--interactive/--no-interactive", "-i/-n", help="Enable interactive prompting for variables"),
-    var: Optional[list[str]] = Option(None, "--var", "-v", help="Variable override (repeatable). Use KEY=VALUE or --var KEY VALUE"),
+    var: Optional[list[str]] = Option(None, "--var", "-v", help="Variable override (repeatable). Supports: KEY=VALUE or KEY VALUE"),
     dry_run: bool = Option(False, "--dry-run", help="Preview template generation without writing files"),
+    show_files: bool = Option(False, "--show-files", help="Display generated file contents in plain text (use with --dry-run)"),
     ctx: Context = None,
   ) -> None:
     """Generate from template.
@@ -203,6 +204,9 @@ class Module(ABC):
         
         # Preview without writing files (dry run)
         cli compose generate traefik --dry-run
+        
+        # Preview and show generated file contents
+        cli compose generate traefik --dry-run --show-files
     """
 
     logger.info(f"Starting generation for template '{id}' from module '{self.name}'")
@@ -283,7 +287,7 @@ class Module(ABC):
       # Warn if directory is not empty (both interactive and non-interactive)
       if dir_not_empty:
         if interactive:
-          console.print(f"\n[yellow] Warning: Directory '{output_dir}' is not empty.[/yellow]")
+          console.print(f"\n[yellow]{IconManager.get_status_icon('warning')} Warning: Directory '{output_dir}' is not empty.[/yellow]")
           if existing_files:
             console.print(f"[yellow]  {len(existing_files)} file(s) will be overwritten.[/yellow]")
           
@@ -312,7 +316,18 @@ class Module(ABC):
       
       # Skip file writing in dry-run mode
       if dry_run:
-        console.print(f"\n[yellow]✓ Dry run complete - no files were written[/yellow]")
+        # Display file contents if requested
+        if show_files:
+          console.print()
+          console.print("[bold blue]Generated Files:[/bold blue]")
+          console.print()
+          for file_path, content in sorted(rendered_files.items()):
+            console.print(f"[cyan]File:[/cyan] {file_path}")
+            print(f"{'─'*80}")
+            print(content)
+            print()  # Add blank line after content
+        
+        console.print(f"[yellow]{IconManager.get_status_icon('success')} Dry run complete - no files were written[/yellow]")
         console.print(f"[dim]Files would have been generated in '{output_dir}'[/dim]")
         logger.info(f"Dry run completed for template '{id}'")
       else:
@@ -327,7 +342,7 @@ class Module(ABC):
             f.write(content)
           console.print(f"[green]Generated file: {file_path}[/green]")
         
-        console.print(f"\n[green] Template generated successfully in '{output_dir}'[/green]")
+        console.print(f"\n[green]{IconManager.get_status_icon('success')} Template generated successfully in '{output_dir}'[/green]")
         logger.info(f"Template written to directory: {output_dir}")
       
       # Display next steps if provided in template metadata
@@ -375,27 +390,49 @@ class Module(ABC):
 
   def config_set(
     self,
-    var_name: str = Argument(..., help="Variable name to set default for"),
-    value: str = Argument(..., help="Default value"),
+    var_name: str = Argument(..., help="Variable name or var=value format"),
+    value: Optional[str] = Argument(None, help="Default value (not needed if using var=value format)"),
   ) -> None:
     """Set a default value for a variable.
     
     This only sets the DEFAULT VALUE, not the variable spec.
     The variable must be defined in the module or template spec.
     
+    Supports both formats:
+      - var_name value
+      - var_name=value
+    
     Examples:
-        # Set default value
+        # Set default value (format 1)
         cli compose defaults set service_name my-awesome-app
         
+        # Set default value (format 2)
+        cli compose defaults set service_name=my-awesome-app
+        
         # Set author for all compose templates
         cli compose defaults set author "Christian Lempa"
     """
     from .config import ConfigManager
     config = ConfigManager()
     
+    # Parse var_name and value - support both "var value" and "var=value" formats
+    if '=' in var_name and value is None:
+      # Format: var_name=value
+      parts = var_name.split('=', 1)
+      actual_var_name = parts[0]
+      actual_value = parts[1]
+    elif value is not None:
+      # Format: var_name value
+      actual_var_name = var_name
+      actual_value = value
+    else:
+      console_err.print(f"[red]Error: Missing value for variable '{var_name}'[/red]")
+      console_err.print(f"[dim]Usage: defaults set VAR_NAME VALUE or defaults set VAR_NAME=VALUE[/dim]")
+      raise Exit(code=1)
+    
     # Set the default value
-    config.set_default_value(self.name, var_name, value)
-    console.print(f"[green] Set default:[/green] [cyan]{var_name}[/cyan] = [yellow]{value}[/yellow]")
+    config.set_default_value(self.name, actual_var_name, actual_value)
+    console.print(f"[green]{IconManager.get_status_icon('success')} Set default:[/green] [cyan]{actual_var_name}[/cyan] = [yellow]{actual_value}[/yellow]")
     console.print(f"\n[dim]This will be used as the default value when generating templates with this module.[/dim]")
 
   def config_remove(
@@ -419,7 +456,7 @@ class Module(ABC):
     if var_name in defaults:
       del defaults[var_name]
       config.set_defaults(self.name, defaults)
-      console.print(f"[green] Removed default for '{var_name}'[/green]")
+      console.print(f"[green]{IconManager.get_status_icon('success')} Removed default for '{var_name}'[/green]")
     else:
       console.print(f"[red]No default found for variable '{var_name}'[/red]")
 
@@ -450,13 +487,13 @@ class Module(ABC):
       if var_name in defaults:
         del defaults[var_name]
         config.set_defaults(self.name, defaults)
-        console.print(f"[green] Cleared default for '{var_name}'[/green]")
+        console.print(f"[green]{IconManager.get_status_icon('success')} Cleared default for '{var_name}'[/green]")
       else:
         console.print(f"[red]No default found for variable '{var_name}'[/red]")
     else:
       # Clear all defaults
       if not force:
-        console.print(f"[bold yellow] Warning:[/bold yellow] This will clear ALL defaults for module '[cyan]{self.name}[/cyan]'")
+        console.print(f"[bold yellow]{IconManager.get_status_icon('warning')} Warning:[/bold yellow] This will clear ALL defaults for module '[cyan]{self.name}[/cyan]'")
         console.print()
         # Show what will be cleared
         for var_name, var_value in defaults.items():
@@ -467,7 +504,7 @@ class Module(ABC):
           return
       
       config.clear_defaults(self.name)
-      console.print(f"[green] Cleared all defaults for module '{self.name}'[/green]")
+      console.print(f"[green]{IconManager.get_status_icon('success')} Cleared all defaults for module '{self.name}'[/green]")
 
   def config_list(self) -> None:
     """Display the defaults for this specific module in YAML format.
@@ -533,13 +570,13 @@ class Module(ABC):
           _ = template.used_variables
           # Trigger variable definition validation by accessing variables
           _ = template.variables
-          console.print(f"[green] Template '{template_id}' is valid[/green]")
+          console.print(f"[green]{IconManager.get_status_icon('success')} Template '{template_id}' is valid[/green]")
           
           if verbose:
             console.print(f"\n[dim]Template path: {template.template_dir}[/dim]")
             console.print(f"[dim]Found {len(template.used_variables)} variables[/dim]")
         except ValueError as e:
-          console.print(f"[red] Validation failed for '{template_id}':[/red]")
+          console.print(f"[red]{IconManager.get_status_icon('error')} Validation failed for '{template_id}':[/red]")
           console.print(f"\n{e}")
           raise Exit(code=1)
           
@@ -565,17 +602,17 @@ class Module(ABC):
           _ = template.variables
           valid_count += 1
           if verbose:
-            console.print(f"[green][/green] {template_id}")
+            console.print(f"[green]{IconManager.get_status_icon('success')}[/green] {template_id}")
         except ValueError as e:
           invalid_count += 1
           errors.append((template_id, str(e)))
           if verbose:
-            console.print(f"[red][/red] {template_id}")
+            console.print(f"[red]{IconManager.get_status_icon('error')}[/red] {template_id}")
         except Exception as e:
           invalid_count += 1
           errors.append((template_id, f"Load error: {e}"))
           if verbose:
-            console.print(f"[yellow]?[/yellow] {template_id}")
+            console.print(f"[yellow]{IconManager.get_status_icon('warning')}[/yellow] {template_id}")
       
       # Summary
       console.print(f"\n[bold]Validation Summary:[/bold]")
@@ -595,7 +632,7 @@ class Module(ABC):
           console.print(f"[dim]{error_msg}[/dim]")
         raise Exit(code=1)
       else:
-        console.print(f"\n[green] All templates are valid![/green]")
+        console.print(f"\n[green]{IconManager.get_status_icon('success')} All templates are valid![/green]")
 
   @classmethod
   def register_cli(cls, app: Typer) -> None:

+ 25 - 5
cli/core/prompt.py

@@ -42,8 +42,16 @@ class PromptHandler:
       # Check if dependencies are satisfied
       if not variables.is_section_satisfied(section_key):
         # Get list of unsatisfied dependencies for better user feedback
-        unsatisfied = [dep for dep in section.needs if not variables.is_section_satisfied(dep)]
-        dep_names = ", ".join(unsatisfied) if unsatisfied else "unknown"
+        unsatisfied_keys = [dep for dep in section.needs if not variables.is_section_satisfied(dep)]
+        # Convert section keys to titles for user-friendly display
+        unsatisfied_titles = []
+        for dep_key in unsatisfied_keys:
+          dep_section = variables.get_section(dep_key)
+          if dep_section:
+            unsatisfied_titles.append(dep_section.title)
+          else:
+            unsatisfied_titles.append(dep_key)
+        dep_names = ", ".join(unsatisfied_titles) if unsatisfied_titles else "unknown"
         self.console.print(
           f"\n[dim]{IconManager.get_status_icon('skipped')} {section.title} (skipped - requires {dep_names} to be enabled)[/dim]"
         )
@@ -83,7 +91,11 @@ class PromptHandler:
         # Pass section.required so _prompt_variable can enforce required inputs
         new_value = self._prompt_variable(variable, required=section.required)
         
-        if new_value != current_value:
+        # For autogenerated variables, always update even if None (signals autogeneration)
+        if variable.autogenerated and new_value is None:
+          collected[var_name] = None
+          variable.value = None
+        elif new_value != current_value:
           collected[var_name] = new_value
           variable.value = new_value
 
@@ -106,6 +118,12 @@ class PromptHandler:
     prompt_text = variable.get_prompt_text()
     default_value = variable.get_normalized_default()
 
+    # Add lock icon before default value for sensitive or autogenerated variables
+    if variable.sensitive or variable.autogenerated:
+      # Format: "Prompt text 🔒 (default)"
+      # The lock icon goes between the text and the default value in parentheses
+      prompt_text = f"{prompt_text} {IconManager.lock()}"
+
     # Check if this specific variable is required (has no default and not autogenerated)
     var_is_required = variable.is_required()
     
@@ -118,7 +136,8 @@ class PromptHandler:
     # Add validation hint (includes both extra text and enum options)
     hint = variable.get_validation_hint()
     if hint:
-      prompt_text = f"{prompt_text} [dim]{hint}[/dim]"
+      # Show options/extra inline inside parentheses, before the default
+      prompt_text = f"{prompt_text} [dim]({hint})[/dim]"
 
     while True:
       try:
@@ -127,7 +146,8 @@ class PromptHandler:
         converted = variable.convert(raw)
 
         # Allow empty values for autogenerated variables
-        if variable.autogenerated and (converted is None or (isinstance(converted, str) and converted == "")):
+        # Also treat the "*auto" marker as a signal for autogeneration
+        if variable.autogenerated and (converted is None or (isinstance(converted, str) and (converted == "" or converted == "*auto"))):
           return None  # Return None to indicate auto-generation should happen
         
         # If this variable is required, do not accept None/empty values

+ 24 - 7
cli/core/template.py

@@ -4,6 +4,7 @@ from .variables import Variable, VariableCollection
 from pathlib import Path
 from typing import Any, Dict, List, Set, Optional, Literal
 from dataclasses import dataclass, field
+from functools import lru_cache
 import logging
 import os
 import yaml
@@ -144,9 +145,8 @@ class Template:
       # Validate 'kind' field (always needed)
       self._validate_kind(self._template_data)
 
-      # Collect file paths (relatively lightweight, needed for various lazy loads)
-      # This will now populate self.template_files
-      self._collect_template_files()
+      # NOTE: File collection is now lazy-loaded via the template_files property
+      # This significantly improves performance when listing many templates
 
       logger.info(f"Loaded template '{self.id}' (v{self.metadata.version})")
 
@@ -165,14 +165,31 @@ class Template:
         return path
     raise FileNotFoundError(f"Main template file (template.yaml or template.yml) not found in {self.template_dir}")
 
-  def _load_module_specs(self, kind: str) -> dict:
-    """Load specifications from the corresponding module."""
+  @staticmethod
+  @lru_cache(maxsize=32)
+  def _load_module_specs(kind: str) -> dict:
+    """Load specifications from the corresponding module with caching.
+    
+    Uses LRU cache to avoid re-loading the same module spec multiple times.
+    This significantly improves performance when listing many templates of the same kind.
+    
+    Args:
+        kind: The module kind (e.g., 'compose', 'terraform')
+        
+    Returns:
+        Dictionary containing the module's spec, or empty dict if kind is empty
+        
+    Raises:
+        ValueError: If module cannot be loaded or spec is invalid
+    """
     if not kind:
       return {}
     try:
       import importlib
-      module = importlib.import_module(f"..modules.{kind}", package=__package__)
-      return getattr(module, 'spec', {})
+      module = importlib.import_module(f"cli.modules.{kind}")
+      spec = getattr(module, 'spec', {})
+      logger.debug(f"Loaded and cached module spec for kind '{kind}'")
+      return spec
     except Exception as e:
       raise ValueError(f"Error loading module specifications for kind '{kind}': {e}")
 

+ 47 - 4
cli/core/variables.py

@@ -230,9 +230,9 @@ class Variable:
         Formatted string representation of the value
     """
     if self.value is None or self.value == "":
-      # Show (auto-generated) for autogenerated variables instead of (none)
+      # Show (*auto) for autogenerated variables instead of (none)
       if self.autogenerated:
-        return "[dim](auto-generated)[/dim]" if show_none else ""
+        return "[dim](*auto)[/dim]" if show_none else ""
       return "[dim](none)[/dim]" if show_none else ""
     
     # Mask sensitive values
@@ -254,6 +254,8 @@ class Variable:
     Handles type conversion and provides sensible defaults for different types.
     Especially useful for enum, bool, and int types in interactive prompts.
     
+    For autogenerated variables, returns "autogenerated" as a display hint.
+    
     Returns:
         Normalized default value appropriate for the variable type
     """
@@ -262,6 +264,10 @@ class Variable:
     except Exception:
       typed = self.value
     
+    # Autogenerated: return display hint
+    if self.autogenerated and (typed is None or typed == ""):
+      return "*auto"
+    
     # Enum: ensure default is valid option
     if self.type == "enum":
       if not self.options:
@@ -577,6 +583,9 @@ class VariableCollection:
       vars_data = section_data.get("vars") or {}
       self._initialize_variables(section, vars_data)
       self._sections[section_key] = section
+    
+    # Validate all variable names are unique across sections
+    self._validate_unique_variable_names()
 
   def _create_section(self, key: str, data: dict[str, Any]) -> VariableSection:
     """Create a VariableSection from data."""
@@ -605,13 +614,47 @@ class VariableCollection:
     
     # Validate toggle variable after all variables are added
     self._validate_section_toggle(section)
-    # FIXME: Add more section-level validation here as needed:
-    #   - Validate that variable names don't conflict across sections (currently allowed but could be confusing)
+    # TODO: Add more section-level validation:
     #   - Validate that required sections have at least one non-toggle variable
     #   - Validate that enum variables have non-empty options lists
     #   - Validate that variable names follow naming conventions (e.g., lowercase_with_underscores)
     #   - Validate that default values are compatible with their type definitions
 
+  def _validate_unique_variable_names(self) -> None:
+    """Validate that all variable names are unique across all sections.
+    
+    This prevents variable name conflicts that could cause confusion when:
+    - Building Jinja2 context (later variables overwrite earlier ones)
+    - Using --var CLI overrides (unclear which section is affected)
+    - Reading/setting defaults (ambiguous which variable is referenced)
+    
+    Raises:
+        ValueError: If duplicate variable names are found across sections
+    """
+    var_to_sections: Dict[str, List[str]] = {}
+    
+    # Build mapping of variable names to sections they appear in
+    for section_key, section in self._sections.items():
+      for var_name in section.variables.keys():
+        if var_name not in var_to_sections:
+          var_to_sections[var_name] = []
+        var_to_sections[var_name].append(section_key)
+    
+    # Find duplicates
+    duplicates = {var: sections for var, sections in var_to_sections.items() if len(sections) > 1}
+    
+    if duplicates:
+      error_lines = [
+        "Variable names must be unique across all sections, but found duplicates:"
+      ]
+      for var_name, sections in sorted(duplicates.items()):
+        error_lines.append(f"  - '{var_name}' appears in sections: {', '.join(sections)}")
+      error_lines.append("\nPlease rename variables to be unique or consolidate them into a single section.")
+      
+      error_msg = "\n".join(error_lines)
+      logger.error(error_msg)
+      raise ValueError(error_msg)
+  
   def _validate_section_toggle(self, section: VariableSection) -> None:
     """Validate that toggle variable is of type bool if it exists.
     

+ 10 - 10
library/compose/authentik/template.yaml

@@ -59,24 +59,24 @@ spec:
   general:
     vars:
       service_name:
-        default: "authentik"
+        default: authentik
       container_name:
-        default: "authentik-server"
+        default: authentik-server
   database:
     required: true
     vars:
       database_name:
-        default: "authentik"
+        default: authentik
       database_user:
-        default: "authentik"
+        default: authentik
   ports:
     vars:
       ports_http:
-        description: "Host port for HTTP (80)"
+        description: Host port for HTTP
         type: int
         default: 8000
       ports_https:
-        description: "Host port for HTTPS (443)"
+        description: Host port for HTTPS
         type: int
         default: 8443
   traefik:
@@ -84,16 +84,16 @@ spec:
       traefik_host:
         default: authentik.home.arpa
   authentik:
-    description: "Configure Authentik application settings"
+    description: Configure Authentik application settings
     required: true
     vars:
       authentik_error_reporting:
-        description: "Enable error reporting to Authentik developers"
+        description: Enable error reporting to Authentik developers
         type: bool
         default: false
       authentik_secret_key:
-        description: "Secret key used for cookie signing and unique user IDs"
-        extra: "Leave empty for auto-generated 50-character secure key"
+        description: Secret Key
+        extra: Used for cookie signing and unique user IDs
         type: str
         sensitive: true
         autogenerated: true