fix(skills): an unreadable skills dir must not abort the run (OPE-216)

coworker/providers/errors.py

@@ -20,12 +20,18 @@
 #   you do not have access to it."}} (404/403) and {"code": "insufficient_quota"} (429).
 # Anthropic: {"type": "not_found_error", "message": "model: X"} (404),
 #   {"type": "permission_error"} (403), and "credit balance is too low" (400).
+# NOTE: markers must be specific to a PROVIDER error body. "permission denied" (with a
+# space) was listed here once and was far too broad: every OS PermissionError stringifies
+# as "[Errno 13] Permission denied: <path>", so an unreadable file anywhere in the turn was
+# reported to the user as "your account doesn't have access to <model>" — a billing problem
+# that did not exist, sending debugging after a provider outage that never happened while
+# the real cause sat in the raw traceback. Anthropic's genuine marker is the underscored
+# error TYPE, which is already listed.
 _NO_ACCESS = (
     "model_not_found",
     "does not exist or you do not have access",
     "does not have access to model",
     "permission_error",
-    "permission denied",
 )
 _NO_QUOTA = (
     "insufficient_quota",

coworker/skills/base.py

@@ -40,13 +40,34 @@ def rescan(self) -> None:
             self._discover(directory)
 
     def _discover(self, directory: Path) -> None:
-        if not directory.is_dir():
+        """Read one skill dir, best-effort.
+
+        Every filesystem call here is guarded, because `rescan()` runs on EVERY TURN to
+        rebuild the live skill menu: a dir that is merely unreadable must degrade to "no
+        skills here", never abort the run. Seen in the wild with an agent running headless
+        as an unprivileged user — a root-owned `.coworker/` appeared in the workspace
+        mid-session and `is_dir()` raised EACCES (pathlib ignores ENOENT/ENOTDIR/EBADF/
+        ELOOP but propagates EACCES). Long sessions died outright, some ~100 turns deep,
+        over an OPTIONAL dir that was empty anyway.
+
+        Per-skill parsing is guarded separately so one unreadable or malformed folder
+        costs only that skill, not the rest of the dir.
+        """
+        try:
+            if not directory.is_dir():
+                return
+            entries = sorted(directory.iterdir())
+        except OSError:
             return
-        for sub in sorted(directory.iterdir()):
-            md = sub / "SKILL.md"
-            if md.is_file():
+        for sub in entries:
+            try:
+                md = sub / "SKILL.md"
+                if not md.is_file():
+                    continue
                 skill = _parse_skill(md)
-                self._skills[skill.name] = skill
+            except (OSError, UnicodeDecodeError, ValueError):
+                continue
+            self._skills[skill.name] = skill
 
     def names(self) -> list[str]:
         return list(self._skills)

tests/test_model_errors.py

@@ -84,3 +84,21 @@ def test_unrelated_errors_pass_through_raw():
         friendly_model_error("gpt-5.6-sol", RuntimeError("connection reset by peer"))
         is None
     )
+
+
+def test_os_permission_error_is_not_a_model_access_error():
+    """An unreadable FILE is not a billing problem.
+
+    Every OS PermissionError stringifies as "[Errno 13] Permission denied: <path>". The
+    marker list used to contain "permission denied" with a space, so any such error was
+    reported as "your account doesn't have access to <model>", sending debugging after a
+    provider outage that never happened. Unrecognized errors must return None so the
+    caller surfaces the raw message.
+    """
+    exc = PermissionError(13, "Permission denied", "/src/.coworker/skills")
+    assert friendly_model_error("openrouter:moonshotai/kimi-k3", exc) is None
+
+    # The genuine provider marker (Anthropic's underscored error type) still translates.
+    real = RuntimeError("Error code: 403 - {'type': 'permission_error', 'message': '...'}")
+    msg = friendly_model_error("anthropic:claude-fable-5", real)
+    assert msg and "doesn't have access to anthropic:claude-fable-5" in msg

tests/test_skills.py

@@ -102,3 +102,64 @@ def test_build_engine_code_has_agents_md_and_skills(tmp_path):
         assert engine.agent_name == "code"
     finally:
         engine.executor.close()
+
+
+def test_unreadable_skill_dir_does_not_abort_the_scan(tmp_path, monkeypatch):
+    """An unreadable skills dir degrades to "no skills", it never raises.
+
+    `rescan()` runs every turn to rebuild the live skill menu, so an EACCES from
+    `is_dir()` on the workspace's optional `.coworker/skills` propagated out of the turn
+    and killed long headless sessions outright, some ~100 turns deep.
+    """
+    from pathlib import Path
+
+    from coworker.skills.base import SkillLoader
+
+    good = tmp_path / "good"
+    (good / "helper").mkdir(parents=True)
+    (good / "helper" / "SKILL.md").write_text(
+        "---\nname: helper\ndescription: works\n---\nbody\n", encoding="utf-8"
+    )
+    denied = tmp_path / "denied"
+    denied.mkdir()
+
+    real_is_dir = Path.is_dir
+
+    def fake_is_dir(self):
+        if self == denied:
+            raise PermissionError(13, "Permission denied", str(self))
+        return real_is_dir(self)
+
+    monkeypatch.setattr(Path, "is_dir", fake_is_dir)
+
+    # The unreadable dir is skipped; the readable one is still loaded.
+    loader = SkillLoader([denied, good])
+    assert loader.names() == ["helper"]
+    loader.rescan()  # the per-turn call must stay quiet too
+    assert loader.get("helper") is not None
+
+
+def test_unreadable_single_skill_folder_is_skipped(tmp_path, monkeypatch):
+    """One bad folder costs only that skill, not the whole dir."""
+    from pathlib import Path
+
+    from coworker.skills.base import SkillLoader
+
+    root = tmp_path / "skills"
+    for name in ("alpha", "beta"):
+        (root / name).mkdir(parents=True)
+        (root / name / "SKILL.md").write_text(
+            "---\nname: %s\ndescription: d\n---\nbody\n" % name, encoding="utf-8"
+        )
+
+    real_read_text = Path.read_text
+
+    def fake_read_text(self, *a, **kw):
+        if self.parent.name == "alpha":
+            raise PermissionError(13, "Permission denied", str(self))
+        return real_read_text(self, *a, **kw)
+
+    monkeypatch.setattr(Path, "read_text", fake_read_text)
+
+    loader = SkillLoader([root])
+    assert loader.names() == ["beta"]