fix: improve local path validation error messages

src/apm_cli/commands/install.py

@@ -155,9 +155,11 @@ def _validate_and_add_packages_to_apm_yml(packages, dry_run=False, dev=False, lo
                 validated_packages.append(canonical)
                 existing_identities.add(identity)  # prevent duplicates within batch
         else:
-            reason = "not accessible or doesn't exist"
-            if not verbose:
-                reason += " -- run with --verbose for auth details"
+            reason = _local_path_failure_reason(dep_ref)
+            if not reason:
+                reason = "not accessible or doesn't exist"
+                if not verbose:
+                    reason += " -- run with --verbose for auth details"
             invalid_outcomes.append((package, reason))
             if logger:
                 logger.validation_fail(package, reason)
@@ -212,6 +214,50 @@ def _validate_and_add_packages_to_apm_yml(packages, dry_run=False, dev=False, lo
     return validated_packages, outcome
 
 
+def _local_path_failure_reason(dep_ref):
+    """Return a specific failure reason for local path deps, or None for remote."""
+    if not (dep_ref.is_local and dep_ref.local_path):
+        return None
+    local = Path(dep_ref.local_path).expanduser()
+    if not local.is_absolute():
+        local = Path.cwd() / local
+    local = local.resolve()
+    if not local.exists():
+        return "path does not exist"
+    if not local.is_dir():
+        return "path is not a directory"
+    # Directory exists but has no package markers
+    return "no apm.yml, SKILL.md, or plugin.json found"
+
+
+def _local_path_no_markers_hint(local_dir, verbose_log=None):
+    """Scan two levels for sub-packages and print a hint if any are found."""
+    from apm_cli.utils.helpers import find_plugin_json
+
+    markers = ("apm.yml", "SKILL.md")
+    found = []
+    for child in sorted(local_dir.iterdir()):
+        if not child.is_dir():
+            continue
+        if any((child / m).exists() for m in markers) or find_plugin_json(child) is not None:
+            found.append(child)
+        # Also check one more level (e.g. skills/<name>/)
+        for grandchild in sorted(child.iterdir()) if child.is_dir() else []:
+            if not grandchild.is_dir():
+                continue
+            if any((grandchild / m).exists() for m in markers) or find_plugin_json(grandchild) is not None:
+                found.append(grandchild)
+
+    if not found:
+        return
+
+    _rich_info("  [i] Found installable package(s) inside this directory:")
+    for p in found[:5]:
+        _rich_echo(f"      apm install {p}", color="dim")
+    if len(found) > 5:
+        _rich_echo(f"      ... and {len(found) - 5} more", color="dim")
+
+
 def _validate_package_exists(package, verbose=False):
     """Validate that a package exists and is accessible on GitHub, Azure DevOps, or locally."""
     import os
@@ -241,7 +287,11 @@ def _validate_package_exists(package, verbose=False):
             if (local / "apm.yml").exists() or (local / "SKILL.md").exists():
                 return True
             from apm_cli.utils.helpers import find_plugin_json
-            return find_plugin_json(local) is not None
+            if find_plugin_json(local) is not None:
+                return True
+            # Directory exists but lacks package markers -- surface a hint
+            _local_path_no_markers_hint(local, verbose_log)
+            return False
 
         # For virtual packages, use the downloader's validation method
         if dep_ref.is_virtual:

tests/unit/compilation/test_agents_compiler_coverage.py

@@ -256,8 +256,9 @@ def test_validate_primitives_outside_base_dir_uses_absolute_path(self):
             compiler.validate_primitives(primitives)
 
             self.assertEqual(len(compiler.warnings), 1)
-            # Should use absolute path (contains tmp2 path, not relative).
-            self.assertIn(str(tmp2), compiler.warnings[0])
+            # portable_relpath resolves and returns POSIX paths
+            resolved_tmp2 = Path(tmp2).resolve().as_posix()
+            self.assertIn(resolved_tmp2, compiler.warnings[0])
         finally:
             import shutil
 
@@ -448,7 +449,9 @@ def test_generate_placement_summary_outside_base_uses_absolute(self):
             result = self._make_distributed_result([(outside_path, 2)])
 
             summary = compiler._generate_placement_summary(result)
-            self.assertIn(outside_path, summary)
+            # portable_relpath resolves and returns POSIX paths
+            resolved_path = (Path(other_tmp) / "AGENTS.md").resolve().as_posix()
+            self.assertIn(resolved_path, summary)
         finally:
             import shutil
 
@@ -477,7 +480,9 @@ def test_generate_distributed_summary_outside_base(self):
             summary = compiler._generate_distributed_summary(
                 result, CompilationConfig()
             )
-            self.assertIn(outside_path, summary)
+            # portable_relpath resolves and returns POSIX paths
+            resolved_path = (Path(other_tmp) / "AGENTS.md").resolve().as_posix()
+            self.assertIn(resolved_path, summary)
         finally:
             import shutil
 

tests/unit/policy/test_discovery.py

@@ -133,34 +133,24 @@ class TestLoadFromFile(unittest.TestCase):
     """Test _load_from_file with real filesystem."""
 
     def test_valid_policy_file(self):
-        with tempfile.NamedTemporaryFile(
-            mode="w", suffix=".yml", delete=False
-        ) as f:
-            f.write(VALID_POLICY_YAML)
-            f.flush()
-            try:
-                result = _load_from_file(Path(f.name))
-                self.assertTrue(result.found)
-                self.assertIsInstance(result.policy, ApmPolicy)
-                self.assertEqual(result.policy.name, "test-policy")
-                self.assertIn("file:", result.source)
-                self.assertIsNone(result.error)
-            finally:
-                os.unlink(f.name)
+        with tempfile.TemporaryDirectory() as tmpdir:
+            p = Path(tmpdir) / "policy.yml"
+            p.write_text(VALID_POLICY_YAML, encoding="utf-8")
+            result = _load_from_file(p)
+            self.assertTrue(result.found)
+            self.assertIsInstance(result.policy, ApmPolicy)
+            self.assertEqual(result.policy.name, "test-policy")
+            self.assertIn("file:", result.source)
+            self.assertIsNone(result.error)
 
     def test_invalid_yaml(self):
-        with tempfile.NamedTemporaryFile(
-            mode="w", suffix=".yml", delete=False
-        ) as f:
-            f.write("enforcement: invalid-value\n")
-            f.flush()
-            try:
-                result = _load_from_file(Path(f.name))
-                self.assertFalse(result.found)
-                self.assertIsNotNone(result.error)
-                self.assertIn("Invalid policy file", result.error)
-            finally:
-                os.unlink(f.name)
+        with tempfile.TemporaryDirectory() as tmpdir:
+            p = Path(tmpdir) / "bad-policy.yml"
+            p.write_text("enforcement: invalid-value\n", encoding="utf-8")
+            result = _load_from_file(p)
+            self.assertFalse(result.found)
+            self.assertIsNotNone(result.error)
+            self.assertIn("Invalid policy file", result.error)
 
     def test_unreadable_file(self):
         result = _load_from_file(Path("/nonexistent/file.yml"))
@@ -494,19 +484,14 @@ class TestDiscoverPolicy(unittest.TestCase):
     """Integration-level tests for discover_policy."""
 
     def test_override_local_file(self):
-        with tempfile.NamedTemporaryFile(
-            mode="w", suffix=".yml", delete=False
-        ) as f:
-            f.write(VALID_POLICY_YAML)
-            f.flush()
-            try:
-                result = discover_policy(
-                    Path("/fake"), policy_override=f.name
-                )
-                self.assertTrue(result.found)
-                self.assertIn("file:", result.source)
-            finally:
-                os.unlink(f.name)
+        with tempfile.TemporaryDirectory() as tmpdir:
+            p = Path(tmpdir) / "override-policy.yml"
+            p.write_text(VALID_POLICY_YAML, encoding="utf-8")
+            result = discover_policy(
+                Path("/fake"), policy_override=str(p)
+            )
+            self.assertTrue(result.found)
+            self.assertIn("file:", result.source)
 
     @patch("apm_cli.policy.discovery.requests")
     def test_override_url(self, mock_requests):

tests/unit/test_install_command.py

@@ -489,3 +489,114 @@ def tracking_callback(dep_ref, mods_dir, parent_chain=""):
         assert ">" in chain, (
             f"Expected '>' separator in chain, got: '{chain}'"
         )
+
+
+class TestLocalPathValidationMessages:
+    """Tests for improved local path validation error messages."""
+
+    def test_local_path_failure_reason_nonexistent(self, tmp_path):
+        """Non-existent path returns 'path does not exist'."""
+        from apm_cli.commands.install import _local_path_failure_reason
+        from apm_cli.models.apm_package import DependencyReference
+
+        dep_ref = DependencyReference.parse(str(tmp_path / "does-not-exist-xyz-9999"))
+        reason = _local_path_failure_reason(dep_ref)
+        assert reason == "path does not exist"
+
+    def test_local_path_failure_reason_file_not_dir(self, tmp_path):
+        """A file (not directory) returns 'path is not a directory'."""
+        from apm_cli.commands.install import _local_path_failure_reason
+        from apm_cli.models.apm_package import DependencyReference
+
+        f = tmp_path / "somefile.txt"
+        f.write_text("hello")
+        dep_ref = DependencyReference.parse(str(f))
+        reason = _local_path_failure_reason(dep_ref)
+        assert reason == "path is not a directory"
+
+    def test_local_path_failure_reason_no_markers(self, tmp_path):
+        """Directory without markers returns specific message."""
+        from apm_cli.commands.install import _local_path_failure_reason
+        from apm_cli.models.apm_package import DependencyReference
+
+        empty_dir = tmp_path / "empty-pkg"
+        empty_dir.mkdir()
+        dep_ref = DependencyReference.parse(str(empty_dir))
+        reason = _local_path_failure_reason(dep_ref)
+        assert reason == "no apm.yml, SKILL.md, or plugin.json found"
+
+    def test_local_path_failure_reason_valid_apm_yml(self, tmp_path):
+        """Directory with apm.yml still returns 'no markers' message.
+
+        _local_path_failure_reason is only called when _validate_package_exists
+        already returned False, so it doesn't re-check markers. We verify it
+        returns a string (not None) and doesn't crash.
+        """
+        from apm_cli.commands.install import _local_path_failure_reason
+        from apm_cli.models.apm_package import DependencyReference
+
+        pkg = tmp_path / "valid-pkg"
+        pkg.mkdir()
+        (pkg / "apm.yml").write_text("name: test\nversion: 1.0.0\n")
+        dep_ref = DependencyReference.parse(str(pkg))
+        reason = _local_path_failure_reason(dep_ref)
+        assert reason == "no apm.yml, SKILL.md, or plugin.json found"
+
+    def test_local_path_failure_reason_remote_ref(self):
+        """Remote refs return None (not a local path)."""
+        from apm_cli.commands.install import _local_path_failure_reason
+        from apm_cli.models.apm_package import DependencyReference
+
+        dep_ref = DependencyReference.parse("owner/repo")
+        reason = _local_path_failure_reason(dep_ref)
+        assert reason is None
+
+    def test_hint_finds_skill_in_subdirectory(self, tmp_path, capsys):
+        """Hint discovers SKILL.md in a child directory."""
+        from apm_cli.commands.install import _local_path_no_markers_hint
+
+        skill_dir = tmp_path / "my-skill"
+        skill_dir.mkdir()
+        (skill_dir / "SKILL.md").write_text("---\nname: my-skill\n---\n")
+
+        _local_path_no_markers_hint(tmp_path)
+        captured = capsys.readouterr()
+        # Rich may wrap long paths across lines; collapse before asserting
+        flat = captured.out.replace("\n", "")
+        assert "my-skill" in flat
+
+    def test_hint_finds_nested_skill(self, tmp_path, capsys):
+        """Hint discovers SKILL.md two levels deep (skills/<name>/)."""
+        from apm_cli.commands.install import _local_path_no_markers_hint
+
+        nested = tmp_path / "skills" / "deep-skill"
+        nested.mkdir(parents=True)
+        (nested / "SKILL.md").write_text("---\nname: deep-skill\n---\n")
+
+        _local_path_no_markers_hint(tmp_path)
+        captured = capsys.readouterr()
+        flat = captured.out.replace("\n", "")
+        assert "deep-skill" in flat
+
+    def test_hint_silent_when_no_packages(self, tmp_path, capsys):
+        """Hint produces no output when no sub-packages found."""
+        from apm_cli.commands.install import _local_path_no_markers_hint
+
+        (tmp_path / "random-file.txt").write_text("nothing here")
+        _local_path_no_markers_hint(tmp_path)
+        captured = capsys.readouterr()
+        assert captured.out == ""
+
+    def test_hint_caps_at_five(self, tmp_path, capsys):
+        """Hint shows at most 5 packages then a '... and N more' line."""
+        from apm_cli.commands.install import _local_path_no_markers_hint
+
+        for i in range(8):
+            d = tmp_path / f"skill-{i:02d}"
+            d.mkdir()
+            (d / "SKILL.md").write_text(f"---\nname: skill-{i:02d}\n---\n")
+
+        _local_path_no_markers_hint(tmp_path)
+        captured = capsys.readouterr()
+        assert "apm install" in captured.out
+        assert "... and 3 more" in captured.out