fix: replace broad except Exception with specific exception types

src/napoln/commands/add.py

@@ -20,7 +20,13 @@
     resolve_git,
     resolve_local,
 )
-from napoln.errors import MultipleSkillsError, ResolverError
+from napoln.errors import (
+    ManifestError,
+    MultipleSkillsError,
+    PlacementError,
+    ResolverError,
+    StoreError,
+)
 from napoln.prompts import SkillChoice, pick_skills
 
 
@@ -297,7 +303,7 @@ def _install_single_skill(
         store_path, content_hash = store.store_skill(
             resolved.skill_dir, install_id, version, napoln_home
         )
-    except Exception as e:
+    except (StoreError, OSError) as e:
         output.error(f"Failed to store skill '{install_id}': {e}")
         return 1
 
@@ -320,7 +326,7 @@ def _install_single_skill(
                     link_mode=link_mode,
                     scope=scope,
                 )
-        except Exception as e:
+        except (PlacementError, OSError) as e:
             output.error(f"Failed to place '{install_id}' for {path_agents[0].display_name}: {e}")
             return 1
 
@@ -363,7 +369,7 @@ def _pick_from_multi_skill_repo(
             continue
         try:
             mf = manifest.read_manifest(mf_path)
-        except Exception:
+        except (ManifestError, OSError):
             continue
         for entry in mf.skills.values():
             installed_sources.add(entry.source)

src/napoln/commands/enable.py

@@ -9,6 +9,7 @@
 from napoln.core import agents as agents_mod
 from napoln.core import linker, manifest, store
 from napoln.core.home import get_napoln_home
+from napoln.errors import PlacementError, StoreError
 from napoln.prompts import SkillChoice, pick_agents, pick_skills
 
 
@@ -64,7 +65,7 @@ def _place_skill_for_agent(
             skill_entry.source,
             napoln_home,
         )
-    except Exception as e:
+    except (StoreError, OSError) as e:
         output.error(f"Failed to retrieve '{skill_name}' from store: {e}")
         return None
 
@@ -82,7 +83,7 @@ def _place_skill_for_agent(
             link_mode,
         )
         return link_mode
-    except Exception as e:
+    except (PlacementError, OSError) as e:
         output.error(f"Failed to place '{skill_name}': {e}")
         return None
 

src/napoln/commands/install.py

@@ -7,7 +7,7 @@
 from napoln import output
 from napoln.core import linker, manifest, store
 from napoln.core.home import NAPOLN_DIR, get_napoln_home
-from napoln.errors import NapolnError
+from napoln.errors import NapolnError, PlacementError
 
 
 def _sync_manifest(
@@ -51,7 +51,7 @@ def _sync_manifest(
                     if result is not None:
                         output.success(f"Restored '{skill_name}' to {placement_path}")
                         synced += 1
-                except Exception as e:
+                except (PlacementError, OSError) as e:
                     output.error(f"Failed to restore '{skill_name}' to {placement_path}: {e}")
                     errors += 1
 

src/napoln/core/manifest.py

@@ -81,7 +81,7 @@ def read_manifest(path: Path) -> Manifest:
 
     try:
         data = tomllib.loads(path.read_text(encoding="utf-8"))
-    except Exception as e:
+    except (OSError, tomllib.TOMLDecodeError) as e:
         raise ManifestError(
             f"Could not read manifest: {path}",
             cause=str(e),
@@ -159,13 +159,12 @@ def write_manifest(manifest: Manifest, path: Path) -> None:
     try:
         tmp_path.write_text(serialized, encoding="utf-8")
         os.replace(tmp_path, path)
-    except Exception:
+    finally:
         if tmp_path.exists():
             try:
                 tmp_path.unlink()
             except OSError:
                 pass
-        raise
 
 
 def add_skill_to_manifest(

src/napoln/core/merger.py

@@ -229,5 +229,5 @@ def has_conflict_markers(file_path: Path) -> bool:
     try:
         content = file_path.read_text(encoding="utf-8")
         return "<<<<<<< " in content and "=======" in content and ">>>>>>> " in content
-    except Exception:
+    except (OSError, UnicodeDecodeError):
         return False

src/napoln/core/resolver.py

@@ -525,7 +525,7 @@ def _extract_version(skill_dir: Path) -> str:
                 return str(metadata["version"])
             if "version" in frontmatter:
                 return str(frontmatter["version"])
-    except Exception:
+    except (OSError, yaml.YAMLError, UnicodeDecodeError):
         pass
 
     return DEFAULT_VERSION
@@ -549,7 +549,7 @@ def _extract_description(skill_dir: Path) -> str:
         frontmatter = yaml.safe_load(content[3:end])
         if isinstance(frontmatter, dict):
             return str(frontmatter.get("description", ""))
-    except Exception:
+    except (OSError, yaml.YAMLError, UnicodeDecodeError):
         pass
 
     return ""

src/napoln/core/store.py

@@ -71,7 +71,7 @@ def store_skill(
 
         # Atomic rename
         temp_path.rename(store_path)
-    except Exception:
+    except OSError:
         if temp_path.exists():
             shutil.rmtree(temp_path)
         raise

tests/unit/test_add_logic.py

@@ -3,8 +3,19 @@
 from __future__ import annotations
 
 import tomllib
+from pathlib import Path
+from unittest.mock import MagicMock
 
-from napoln.commands.add import _ensure_initialized
+import pytest
+
+from napoln.commands.add import (
+    _ensure_initialized,
+    _install_single_skill,
+    _pick_from_multi_skill_repo,
+)
+from napoln.core import manifest as manifest_mod
+from napoln.core.resolver import ResolvedSource, SourceType
+from napoln.errors import ManifestError, MultipleSkillsError, PlacementError, StoreError
 
 
 class TestEnsureInitialized:
@@ -45,3 +56,206 @@ def test_creates_nested_parent_dirs(self, tmp_path):
 
         assert home.is_dir()
         assert (home / "store").is_dir()
+
+
+class TestInstallSingleSkillExceptions:
+    """Regression: broad except Exception swallows programming bugs."""
+
+    @pytest.fixture
+    def resolved(self, tmp_path):
+        skill_dir = tmp_path / "skill"
+        skill_dir.mkdir()
+        (skill_dir / "SKILL.md").write_text(
+            '---\nname: test-skill\ndescription: x\nmetadata:\n  version: "1.0.0"\n---\n# Hello'
+        )
+        return ResolvedSource(
+            source_type=SourceType.LOCAL,
+            source_id=str(skill_dir),
+            skill_dir=skill_dir,
+            version="1.0.0",
+        )
+
+    @pytest.fixture
+    def agent_config(self):
+        cfg = MagicMock()
+        cfg.id = "claude-code"
+        cfg.display_name = "Claude Code"
+        return cfg
+
+    @pytest.fixture
+    def manifest_and_path(self, tmp_path):
+        path = tmp_path / "manifest.toml"
+        mf = manifest_mod.Manifest()
+        return mf, path
+
+    def test_store_error_outputs_and_returns_1(
+        self, resolved, agent_config, manifest_and_path, monkeypatch
+    ):
+        mf, path = manifest_and_path
+        errors = []
+
+        def capture_error(msg, **_kwargs):
+            errors.append(msg)
+
+        monkeypatch.setattr("napoln.commands.add.output.error", capture_error)
+        monkeypatch.setattr(
+            "napoln.commands.add.store.store_skill",
+            lambda *_a, **_k: (_ for _ in ()).throw(StoreError("store failed")),
+        )
+
+        code = _install_single_skill(
+            resolved,
+            "test-skill",
+            [agent_config],
+            Path.home(),
+            Path.home(),
+            "global",
+            None,
+            mf,
+            path,
+            False,
+        )
+        assert code == 1
+        assert any("Failed to store skill" in e for e in errors)
+
+    def test_store_type_error_propagates(
+        self, resolved, agent_config, manifest_and_path, monkeypatch
+    ):
+        """Programming bugs must not be swallowed."""
+        mf, path = manifest_and_path
+
+        monkeypatch.setattr(
+            "napoln.commands.add.store.store_skill",
+            lambda *_a, **_k: (_ for _ in ()).throw(TypeError("programming bug")),
+        )
+
+        with pytest.raises(TypeError, match="programming bug"):
+            _install_single_skill(
+                resolved,
+                "test-skill",
+                [agent_config],
+                Path.home(),
+                Path.home(),
+                "global",
+                None,
+                mf,
+                path,
+                False,
+            )
+
+    def test_placement_error_outputs_and_returns_1(
+        self, resolved, agent_config, manifest_and_path, monkeypatch, tmp_path
+    ):
+        mf, path = manifest_and_path
+        errors = []
+
+        def capture_error(msg, **_kwargs):
+            errors.append(msg)
+
+        monkeypatch.setattr("napoln.commands.add.output.error", capture_error)
+        monkeypatch.setattr(
+            "napoln.commands.add.store.store_skill",
+            lambda *_a, **_k: (tmp_path / "store", "hash123"),
+        )
+        monkeypatch.setattr(
+            "napoln.commands.add.linker.place_skill",
+            lambda *_a, **_k: (_ for _ in ()).throw(PlacementError("placement failed")),
+        )
+
+        code = _install_single_skill(
+            resolved,
+            "test-skill",
+            [agent_config],
+            Path.home(),
+            Path.home(),
+            "global",
+            None,
+            mf,
+            path,
+            False,
+        )
+        assert code == 1
+        assert any("Failed to place" in e for e in errors)
+
+    def test_placement_type_error_propagates(
+        self, resolved, agent_config, manifest_and_path, monkeypatch, tmp_path
+    ):
+        """Programming bugs must not be swallowed."""
+        mf, path = manifest_and_path
+
+        monkeypatch.setattr(
+            "napoln.commands.add.store.store_skill",
+            lambda *_a, **_k: (tmp_path / "store", "hash123"),
+        )
+        monkeypatch.setattr(
+            "napoln.commands.add.linker.place_skill",
+            lambda *_a, **_k: (_ for _ in ()).throw(TypeError("programming bug")),
+        )
+
+        with pytest.raises(TypeError, match="programming bug"):
+            _install_single_skill(
+                resolved,
+                "test-skill",
+                [agent_config],
+                Path.home(),
+                Path.home(),
+                "global",
+                None,
+                mf,
+                path,
+                False,
+            )
+
+
+class TestPickFromMultiSkillRepoExceptions:
+    """Regression: broad except Exception swallows programming bugs."""
+
+    def test_manifest_error_continues(self, tmp_path, monkeypatch):
+        repo_dir = tmp_path / "repo"
+        repo_dir.mkdir()
+        skill_dir = repo_dir / "skill-a"
+        skill_dir.mkdir()
+        (skill_dir / "SKILL.md").write_text("---\nname: skill-a\n---\n")
+
+        err = MultipleSkillsError(repo_dir, [skill_dir])
+        parsed = MagicMock()
+        parsed.host = "github.com"
+        parsed.owner = "owner"
+        parsed.repo = "repo"
+        parsed.version = ""
+
+        monkeypatch.setattr(
+            "napoln.commands.add.manifest.read_manifest",
+            lambda *_a, **_k: (_ for _ in ()).throw(ManifestError("bad manifest")),
+        )
+        monkeypatch.setattr("napoln.commands.add.pick_skills", lambda _choices: [])
+
+        result = _pick_from_multi_skill_repo(err, parsed, "owner/repo", None, tmp_path / ".napoln")
+        assert result is None
+
+    def test_manifest_type_error_propagates(self, tmp_path, monkeypatch):
+        """Programming bugs must not be swallowed."""
+        repo_dir = tmp_path / "repo"
+        repo_dir.mkdir()
+        skill_dir = repo_dir / "skill-a"
+        skill_dir.mkdir()
+        (skill_dir / "SKILL.md").write_text("---\nname: skill-a\n---\n")
+
+        err = MultipleSkillsError(repo_dir, [skill_dir])
+        parsed = MagicMock()
+        parsed.host = "github.com"
+        parsed.owner = "owner"
+        parsed.repo = "repo"
+        parsed.version = ""
+
+        napoln_home = tmp_path / ".napoln"
+        napoln_home.mkdir()
+        (napoln_home / "manifest.toml").write_text("[napoln]\nschema = 1\n", encoding="utf-8")
+
+        monkeypatch.setattr(
+            "napoln.commands.add.manifest.read_manifest",
+            lambda *_a, **_k: (_ for _ in ()).throw(TypeError("programming bug")),
+        )
+
+        with pytest.raises(TypeError, match="programming bug"):
+            _pick_from_multi_skill_repo(err, parsed, "owner/repo", None, napoln_home)