fix(skills): read undecodable skill state as missing (#7963)

* fix(skills): read undecodable skill state as missing

`_read_manifest` and `_read_skill_states` document that an unreadable file
means "no manifest" / "no overrides", and both catch OSError and
JSONDecodeError. A file that is not valid UTF-8 raises UnicodeDecodeError,
which is a ValueError and matches neither, so the projection and the skill
state read raise instead of falling back.

Catch it alongside the existing exceptions.

* fix(skills): keep the reason when profile state cannot be decoded

Review feedback on #7963: the state reader dropped the exception, so a decode
failure that silently discards the user's disabled set left nothing to diagnose,
and the next toggle overwrites those bytes. Log the exception, and log a debug
line when an unreadable projection manifest forces a rebuild.

* docs(skills): state that undecodable skill state fails open

_read_skill_states already degraded a malformed file to {} before this change;
making undecodable bytes take the same path made that explicit, so say so next to
the sandbox check that documents the opposite verdict for a single named skill.

---------

Co-authored-by: lxcxjxhx <lxcxjxhx@users.noreply.github.com>

backend/packages/harness/deerflow/skills/projection.py

@@ -422,7 +422,10 @@ def _manifest_path(scope_root: Path) -> Path:
 def _read_manifest(scope_root: Path) -> dict | None:
     try:
         value = json.loads(_manifest_path(scope_root).read_text(encoding="utf-8"))
-    except (OSError, json.JSONDecodeError):
+    except (OSError, UnicodeDecodeError, json.JSONDecodeError) as exc:
+        # Discarding the manifest forces a full rebuild on the next ensure; log
+        # once so "why did my projection rebuild" has an answer.
+        logger.debug("Ignoring unreadable projection manifest %s: %s", _manifest_path(scope_root), exc)
         return None
     return value if isinstance(value, dict) else None
 

backend/packages/harness/deerflow/skills/storage/user_scoped_skill_storage.py

@@ -101,7 +101,16 @@ class UserScopedSkillStorage(LocalSkillStorage):
 
         Returns a dict keyed by skill name, each value being
         ``{"enabled": True/False}``.  Returns an empty dict if the file
-        does not exist or is unreadable.
+        does not exist or cannot be decoded.
+
+        Undecodable bytes degrade to "no overrides", i.e. FAIL OPEN: every
+        skill the user had disabled reads as enabled again.  That matches what
+        malformed JSON already did, and it is deliberate - the alternative,
+        denying every custom skill, locks a user out of their own tree over one
+        stray byte.  ``sandbox/tools.py::_is_disabled_skill_path`` says corrupt
+        state must fail CLOSED on purpose: it decides access for one already
+        named skill, where denying is the safe verdict.  The two are not in
+        conflict; do not "align" them without deciding which side loses data.
         """
         if not self._skill_states_file.exists():
             return {}
@@ -110,8 +119,10 @@ class UserScopedSkillStorage(LocalSkillStorage):
                 data = json.load(f)
             if isinstance(data, dict):
                 return data
-        except (json.JSONDecodeError, OSError):
-            logger.warning("Failed to read skill states file %s", self._skill_states_file)
+        except (json.JSONDecodeError, OSError, UnicodeDecodeError) as exc:
+            # A decode failure means the user's disabled set is discarded, and the
+            # next toggle rewrites this file: keep the reason in the log.
+            logger.warning("Failed to read skill states file %s: %s", self._skill_states_file, exc)
         return {}
 
     def _write_skill_states(self, states: dict[str, dict[str, bool]]) -> None:

backend/tests/test_skill_projection.py

@@ -1080,3 +1080,21 @@ def test_thread_projection_upgrades_metadata_only_manifest(projection_env) -> No
     ensure_thread_skill_projection(env.storage, "thread-old-manifest", {"alpha"})
 
     assert target.read_bytes() == source.read_bytes()
+
+
+def test_undecodable_manifest_is_treated_as_missing(tmp_path: Path) -> None:
+    """A manifest we cannot decode must not stop the projection from being rebuilt."""
+    from deerflow.skills.projection import _read_manifest
+
+    (tmp_path / ".projection-manifest.json").write_bytes(b"\xff\xfe")
+
+    assert _read_manifest(tmp_path) is None
+
+
+def test_undecodable_skill_states_file_is_treated_as_empty(projection_env) -> None:
+    """An undecodable states file reads as 'no overrides', not as a crash."""
+    state_file = projection_env.paths.user_skills_dir("alice") / "_skill_states.json"
+    state_file.parent.mkdir(parents=True, exist_ok=True)
+    state_file.write_bytes(b"\xff\xfe")
+
+    assert projection_env.storage._read_skill_states() == {}