feat(addon-loader): strengthen manifest validation at parse time (#388)

- Change AddonManifest.error (str) to .errors (list[str]) so all
  problems are accumulated in a single pass instead of masking
  subsequent failures.

- Validate load_priority type, version string format (dotted-numeric),
  and context ID syntax (alphanumeric + dots/underscores) during
  parse_manifest().

- Add validate_dependencies() cross-addon check that verifies all
  declared dependencies reference discovered addon names, called
  between parsing and validation in the pipeline.

- Rewrite validate_manifest() to run all checks (version bounds,
  workbench path, Init.py presence) and collect errors instead of
  early-returning on the first failure.

- Simplify resolve_load_order() by removing the inline
  unknown-dependency check (now handled earlier by
  validate_dependencies()).

- Update _print_load_summary() to join multiple errors with semicolons.

Closes #388

src/Mod/Create/addon_loader.py

@@ -6,6 +6,7 @@
 
 import enum
 import os
+import re
 import sys
 import time
 import xml.etree.ElementTree as ET
@@ -50,12 +51,14 @@ class AddonManifest:
 
     # Runtime state
     state: AddonState = AddonState.DISCOVERED
-    error: str = ""
+    errors: list[str] = field(default_factory=list)
     load_time_ms: float = 0.0
     contexts: list[str] = field(default_factory=list)
 
     def __repr__(self):
-        return f"AddonManifest(name={self.name!r}, version={self.version!r}, state={self.state.value})"
+        return (
+            f"AddonManifest(name={self.name!r}, version={self.version!r}, state={self.state.value})"
+        )
 
 
 # ---------------------------------------------------------------------------
@@ -191,6 +194,16 @@ def scan_addons(mods_dir: str) -> list[AddonManifest]:
 # Parsing
 # ---------------------------------------------------------------------------
 
+# Validation patterns
+_VERSION_RE = re.compile(r"^\d+(\.\d+)*$")
+_CONTEXT_ID_RE = re.compile(r"^[a-zA-Z0-9][a-zA-Z0-9_.]*$")
+
+
+def _valid_version(v: str) -> bool:
+    """Check that a version string is dotted-numeric (e.g. '0.1.0')."""
+    return bool(_VERSION_RE.match(v))
+
+
 # FreeCAD package.xml namespace
 _PKG_NS = "https://wiki.freecad.org/Package_Metadata"
 
@@ -224,10 +237,8 @@ def parse_manifest(manifest: AddonManifest):
         root = tree.getroot()
     except ET.ParseError as e:
         manifest.state = AddonState.FAILED
-        manifest.error = f"XML parse error: {e}"
-        FreeCAD.Console.PrintWarning(
-            f"Create: Failed to parse {manifest.package_xml_path}: {e}\n"
-        )
+        manifest.errors.append(f"XML parse error: {e}")
+        FreeCAD.Console.PrintWarning(f"Create: Failed to parse {manifest.package_xml_path}: {e}\n")
         return
 
     # Standard fields
@@ -259,22 +270,43 @@ def parse_manifest(manifest: AddonManifest):
         manifest.has_kindred_element = True
         manifest.min_create_version = _text(kindred, "min_create_version") or None
         manifest.max_create_version = _text(kindred, "max_create_version") or None
+
+        # Validate version string formats
+        if manifest.min_create_version and not _valid_version(manifest.min_create_version):
+            manifest.errors.append(
+                f"min_create_version is not a valid version: {manifest.min_create_version!r}"
+            )
+        if manifest.max_create_version and not _valid_version(manifest.max_create_version):
+            manifest.errors.append(
+                f"max_create_version is not a valid version: {manifest.max_create_version!r}"
+            )
+
+        # Validate load_priority is an integer
         priority_str = _text(kindred, "load_priority")
         if priority_str:
             try:
                 manifest.load_priority = int(priority_str)
             except ValueError:
-                pass
+                manifest.errors.append(f"load_priority must be an integer, got: {priority_str!r}")
+
         deps = _find(kindred, "dependencies")
         if deps is not None:
             for dep in _findall(deps, "dependency"):
                 if dep.text and dep.text.strip():
                     manifest.dependencies.append(dep.text.strip())
+
+        # Validate context ID syntax
         ctxs = _find(kindred, "contexts")
         if ctxs is not None:
             for ctx in _findall(ctxs, "context"):
-                if ctx.text and ctx.text.strip():
-                    manifest.contexts.append(ctx.text.strip())
+                cid = ctx.text.strip() if ctx.text else ""
+                if cid:
+                    if _CONTEXT_ID_RE.match(cid):
+                        manifest.contexts.append(cid)
+                    else:
+                        manifest.errors.append(
+                            f"Invalid context ID {cid!r}: must be alphanumeric, dots, or underscores"
+                        )
 
     FreeCAD.Console.PrintLog(
         f"Create: Parsed {manifest.name} v{manifest.version} from {manifest.package_xml_path}\n"
@@ -294,8 +326,27 @@ def _parse_version(v: str) -> tuple:
         return (0, 0, 0)
 
 
+def validate_dependencies(manifests: list[AddonManifest]):
+    """Check that all declared dependencies reference discovered addons.
+
+    Called after parsing all manifests so the full set of names is known.
+    Errors are accumulated on each manifest.
+    """
+    known = {m.name for m in manifests}
+    for m in manifests:
+        if m.state == AddonState.FAILED:
+            continue
+        for dep in m.dependencies:
+            if dep not in known:
+                m.errors.append(f"Unknown dependency: {dep!r}")
+
+
 def validate_manifest(manifest: AddonManifest, create_version: str) -> bool:
-    """Check version compatibility and path existence. Returns True if valid."""
+    """Check version compatibility and path existence.
+
+    All checks run to completion so that every problem is reported.
+    Returns True if the manifest is valid for loading.
+    """
     if manifest.state == AddonState.FAILED:
         return False
 
@@ -303,39 +354,29 @@ def validate_manifest(manifest: AddonManifest, create_version: str) -> bool:
 
     if manifest.min_create_version:
         if cv < _parse_version(manifest.min_create_version):
-            manifest.state = AddonState.SKIPPED
-            manifest.error = f"Requires Create >= {manifest.min_create_version}, running {create_version}"
-            FreeCAD.Console.PrintWarning(
-                f"Create: Skipping {manifest.name}: {manifest.error}\n"
+            manifest.errors.append(
+                f"Requires Create >= {manifest.min_create_version}, running {create_version}"
             )
-            return False
 
     if manifest.max_create_version:
         if cv > _parse_version(manifest.max_create_version):
-            manifest.state = AddonState.SKIPPED
-            manifest.error = f"Requires Create <= {manifest.max_create_version}, running {create_version}"
-            FreeCAD.Console.PrintWarning(
-                f"Create: Skipping {manifest.name}: {manifest.error}\n"
+            manifest.errors.append(
+                f"Requires Create <= {manifest.max_create_version}, running {create_version}"
             )
-            return False
 
     if not os.path.isdir(manifest.workbench_path):
-        manifest.state = AddonState.SKIPPED
-        manifest.error = f"Workbench path not found: {manifest.workbench_path}"
-        FreeCAD.Console.PrintWarning(
-            f"Create: Skipping {manifest.name}: {manifest.error}\n"
-        )
-        return False
+        manifest.errors.append(f"Workbench path not found: {manifest.workbench_path}")
+    else:
+        # Only check Init files if the directory exists
+        has_init = os.path.isfile(os.path.join(manifest.workbench_path, "Init.py"))
+        has_gui = os.path.isfile(os.path.join(manifest.workbench_path, "InitGui.py"))
+        if not has_init and not has_gui:
+            manifest.errors.append(f"No Init.py or InitGui.py in {manifest.workbench_path}")
 
-    # At least one of Init.py or InitGui.py must exist
-    has_init = os.path.isfile(os.path.join(manifest.workbench_path, "Init.py"))
-    has_gui = os.path.isfile(os.path.join(manifest.workbench_path, "InitGui.py"))
-    if not has_init and not has_gui:
+    if manifest.errors:
         manifest.state = AddonState.SKIPPED
-        manifest.error = f"No Init.py or InitGui.py in {manifest.workbench_path}"
-        FreeCAD.Console.PrintWarning(
-            f"Create: Skipping {manifest.name}: {manifest.error}\n"
-        )
+        for err in manifest.errors:
+            FreeCAD.Console.PrintWarning(f"Create: {manifest.name}: {err}\n")
         return False
 
     manifest.state = AddonState.VALIDATED
@@ -347,9 +388,7 @@ def validate_manifest(manifest: AddonManifest, create_version: str) -> bool:
 # ---------------------------------------------------------------------------
 
 
-def resolve_load_order(
-    manifests: list[AddonManifest], mods_dir: str
-) -> list[AddonManifest]:
+def resolve_load_order(manifests: list[AddonManifest], mods_dir: str) -> list[AddonManifest]:
     """Sort addons by dependencies, then by (load_priority, name).
 
     If no addons declare a <kindred> element, fall back to the legacy
@@ -370,15 +409,10 @@ def resolve_load_order(
 
     ts = TopologicalSorter()
     for m in manifests:
-        # Only include dependencies that are actually discovered
+        if m.state in (AddonState.SKIPPED, AddonState.FAILED):
+            continue
         known_deps = [d for d in m.dependencies if d in by_name]
-        unknown_deps = [d for d in m.dependencies if d not in by_name]
-        for dep in unknown_deps:
-            m.state = AddonState.SKIPPED
-            m.error = f"Missing dependency: {dep}"
-            FreeCAD.Console.PrintWarning(f"Create: Skipping {m.name}: {m.error}\n")
-        if m.state != AddonState.SKIPPED:
-            ts.add(m.name, *known_deps)
+        ts.add(m.name, *known_deps)
 
     try:
         # Process level by level so we can sort within each topological level
@@ -387,11 +421,7 @@ def resolve_load_order(
         while ts.is_active():
             ready = list(ts.get_ready())
             # Sort each level by (priority, name) for determinism
-            ready.sort(
-                key=lambda n: (
-                    (by_name[n].load_priority, n) if n in by_name else (999, n)
-                )
-            )
+            ready.sort(key=lambda n: (by_name[n].load_priority, n) if n in by_name else (999, n))
             for name in ready:
                 ts.done(name)
             order.extend(ready)
@@ -400,7 +430,7 @@ def resolve_load_order(
             f"Create: Dependency cycle detected: {e}. Falling back to priority order.\n"
         )
         return sorted(
-            [m for m in manifests if m.state != AddonState.SKIPPED],
+            [m for m in manifests if m.state not in (AddonState.SKIPPED, AddonState.FAILED)],
             key=lambda m: (m.load_priority, m.name),
         )
 
@@ -408,7 +438,7 @@ def resolve_load_order(
     result = []
     for name in order:
         m = by_name.get(name)
-        if m is not None and m.state != AddonState.SKIPPED:
+        if m is not None and m.state not in (AddonState.SKIPPED, AddonState.FAILED):
             result.append(m)
 
     return result
@@ -465,12 +495,10 @@ def _load_addon(manifest: AddonManifest, gui: bool = False):
         else:
             manifest.load_time_ms += elapsed
         manifest.state = AddonState.LOADED
-        FreeCAD.Console.PrintLog(
-            f"Create: Loaded {manifest.name} {init_file} ({elapsed:.0f}ms)\n"
-        )
+        FreeCAD.Console.PrintLog(f"Create: Loaded {manifest.name} {init_file} ({elapsed:.0f}ms)\n")
     except Exception as e:
         manifest.state = AddonState.FAILED
-        manifest.error = str(e)
+        manifest.errors.append(str(e))
         FreeCAD.Console.PrintWarning(f"Create: Failed to load {manifest.name}: {e}\n")
 
 
@@ -500,8 +528,8 @@ def _print_load_summary(registry: AddonRegistry, phase: str):
         state_str = m.state.value.upper()
         time_str = f"{m.load_time_ms:.0f}ms" if m.load_time_ms > 0 else "-"
         line = f"  {m.name:<{max_name}}  {state_str:<12}  {time_str:>6}"
-        if m.error:
-            line += f"  ({m.error})"
+        if m.errors:
+            line += f"  ({'; '.join(m.errors)})"
         lines.append(line)
 
     FreeCAD.Console.PrintLog("\n".join(lines) + "\n")
@@ -527,6 +555,8 @@ def load_addons(gui: bool = False):
         for m in manifests:
             parse_manifest(m)
 
+        validate_dependencies(manifests)
+
         create_version = _get_create_version()
         validated = [m for m in manifests if validate_manifest(m, create_version)]
         ordered = resolve_load_order(validated, mods_dir)