fix(install): stop destroying a config we cannot parse (#34)

_load returned {} on any parse failure, so install merged its fragment into an empty object and wrote it back, destroying the user's whole config (starkest for Junie, whose config.json is the entire CLI configuration). _load now tolerates a UTF-8 BOM (utf-8-sig) and raises ConfigUnreadable on anything still unreadable, so install/uninstall preserve the file. installed() stays a query that never raises -- its TOML branch now guards (OSError, UnicodeError) too, matching the JSON branch's ConfigUnreadable handling.

Author: jothimani-rajendran

CHANGELOG.md

@@ -7,6 +7,17 @@ versioning: [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 ## [Unreleased]
 
 ### Fixed
+- `install` could **destroy a user's entire config**. `_load` returned `{}` on any parse
+  failure, so the fragment was merged into an empty object and written back, discarding
+  everything the file held. For Junie, whose `config.json` is the whole CLI configuration
+  rather than a hooks-only file, a single stray byte -- a UTF-8 BOM from a Windows editor, a
+  trailing comma, a half-saved edit -- cost the user their entire config. `_load` now
+  tolerates a BOM (utf-8-sig, mirroring the runtime stdin fix) and raises `ConfigUnreadable`
+  on anything still unparseable rather than returning `{}`, so `install` and `uninstall`
+  stop and preserve the file instead of overwriting it. `installed()` stays a safe query and
+  reports "not present" rather than raising -- including its TOML branch, which read the file
+  with no guard and so crashed the query on a non-UTF-8 or unreadable config. This protected
+  every JSON-config agent, not just Junie.
 - Claude Code's `NotebookEdit` is listed in `WRITE_TOOLS`, so the adapter claims to gate it,
   but its cell body arrives as `new_source` -- a field `parse()` never read -- so
   `Event.content` was `None` and a content policy (secret scan, memory guard) saw nothing at

src/agentseam/install.py

@@ -21,14 +21,51 @@
 END = "# <<< agentseam <<<"
 
 
+class ConfigUnreadable(Exception):
+    """An existing config file could not be read or parsed, so it must not be overwritten.
+
+    Returning {} here -- as this used to -- meant install merged its fragment into an empty
+    object and wrote that back, silently DESTROYING everything the user had. For Junie, whose
+    config.json is the whole CLI configuration rather than a hooks-only file, a single stray
+    byte (a UTF-8 BOM, a trailing comma, a half-saved edit) cost the user their entire
+    config. Wiping a file we cannot understand is the exact silent-destruction this library
+    exists to refuse; the honest move is to stop and say so.
+    """
+
+
 def _load(path):
+    """The existing config as a dict. {} only when the file is genuinely absent.
+
+    A BOM is tolerated (utf-8-sig), mirroring the runtime's stdin fix -- Windows editors add
+    one and it is not corruption. Anything still unparseable, or unreadable, raises rather
+    than returning {}, because the caller is about to write this file back.
+    """
     if not os.path.exists(path):
         return {}
     try:
-        with open(path) as fh:
-            return json.load(fh)
-    except (ValueError, OSError):
-        return {}
+        with open(path, encoding="utf-8-sig") as fh:
+            text = fh.read()
+    except OSError as exc:
+        raise ConfigUnreadable("cannot read %s: %s" % (path, exc)) from exc
+    except UnicodeError as exc:
+        # A file that is not UTF-8/UTF-8-BOM (e.g. UTF-16) raises while decoding here. Wrap it
+        # as ConfigUnreadable, like a JSON error, so it is preserved and reported through the
+        # one path -- keeping the exception type consistent -- rather than crashing
+        # install/uninstall, and installed()'s JSON branch (which catches only
+        # ConfigUnreadable), with a raw UnicodeDecodeError.
+        raise ConfigUnreadable("%s exists but is not UTF-8 text (%s); refusing to overwrite it." % (path, exc)) from exc
+    if not text.strip():
+        return {}  # an empty file is not corruption; treat it as a fresh config
+    try:
+        loaded = json.loads(text)
+    except ValueError as exc:
+        raise ConfigUnreadable(
+            "%s exists but is not valid JSON (%s); refusing to overwrite it. "
+            "Fix or move the file, then re-run." % (path, exc)
+        ) from exc
+    if not isinstance(loaded, dict):
+        raise ConfigUnreadable("%s is valid JSON but not an object; refusing to overwrite it." % path)
+    return loaded
 
 
 def _dump(path, data):
@@ -183,6 +220,18 @@ def installed(agent, repo_root=".", owner="agentseam"):
     if getattr(mod, "CONFIG_FORMAT", "json") == "toml":
         if not os.path.exists(path):
             return False
-        with open(path) as fh:
-            return _block_bounds(fh.read(), owner) is not None
-    return owner in json.dumps(_load(path))
+        try:
+            with open(path, encoding="utf-8-sig") as fh:
+                text = fh.read()
+        except (OSError, UnicodeError):
+            # A query never raises: an unreadable or non-UTF-8 file means our witness is not
+            # known to be there. This mirrors the JSON branch's ConfigUnreadable handling;
+            # uninstall() is where an unreadable file must stop, not here.
+            return False
+        return _block_bounds(text, owner) is not None
+    try:
+        return owner in json.dumps(_load(path))
+    except ConfigUnreadable:
+        # A query never raises: if the file cannot be read, our witness is not known to be
+        # there. uninstall() is where an unreadable file must stop, not here.
+        return False

tests/test_install.py

@@ -4,6 +4,8 @@
 import sys
 from pathlib import Path
 
+import pytest
+
 sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src"))
 
 from agentseam import adapters
@@ -111,3 +113,63 @@ def test_a_user_scoped_config_path_is_not_nested_under_the_repo(tmp_path, monkey
     assert not (tmp_path / "~").exists(), "created a directory literally named ~"
     assert Path(written) == tmp_path / ".junie" / "config.json"
     assert I.installed("junie", str(tmp_path))
+
+
+def test_install_never_destroys_a_config_it_cannot_parse(tmp_path, monkeypatch):
+    """The data-loss bug: _load returned {} on any parse failure, so install merged its
+    fragment into an empty object and wrote that back -- wiping the user's whole config.
+
+    For Junie, config.json IS the CLI configuration, so a stray byte cost everything. A
+    UTF-8 BOM (Windows editors add one) is the common trigger and is not corruption, so it
+    must be tolerated; genuine corruption must stop install, never overwrite.
+    """
+    monkeypatch.setenv("HOME", str(tmp_path))
+    cfg = tmp_path / ".junie" / "config.json"
+    cfg.parent.mkdir()
+
+    # A BOM-prefixed real config: tolerated, preserved, and wired.
+    cfg.write_bytes(b"\xef\xbb\xbf" + json.dumps({"theme": "dark", "customModel": "keep-me"}).encode())
+    I.install("junie", ["pre_tool"], "guard.py")
+    after = json.loads(cfg.read_text())
+    assert after["customModel"] == "keep-me" and after["theme"] == "dark", "user settings were destroyed"
+    assert "hooks" in after, "the hook was not wired"
+
+    # Genuine corruption: install refuses rather than clobbering.
+    cfg.write_text("{ this is not json ,,, }")
+    with pytest.raises(I.ConfigUnreadable):
+        I.install("junie", ["pre_tool"], "guard.py")
+    assert cfg.read_text() == "{ this is not json ,,, }", "a config we could not parse was overwritten"
+
+    # An undecodable encoding (UTF-16) is unreadable, not corruption-in-JSON, but must take
+    # the same preserve-and-report path rather than crashing with a raw UnicodeDecodeError.
+    cfg.write_bytes(json.dumps({"keep": "me"}).encode("utf-16"))
+    with pytest.raises(I.ConfigUnreadable):
+        I.install("junie", ["pre_tool"], "guard.py")
+    assert cfg.read_bytes()[:2] == b"\xff\xfe", "a UTF-16 config was overwritten"
+
+
+def test_a_query_never_raises_on_an_unparseable_config(tmp_path, monkeypatch):
+    """installed() is a read-only question; a corrupt file means "not known to be there",
+    not a crash. uninstall() is where an unreadable file must stop instead."""
+    monkeypatch.setenv("HOME", str(tmp_path))
+    cfg = tmp_path / ".junie" / "config.json"
+    cfg.parent.mkdir()
+    cfg.write_text("{ broken ,,, }")
+
+    assert I.installed("junie") is False
+    with pytest.raises(I.ConfigUnreadable):
+        I.uninstall("junie")
+    assert cfg.read_text() == "{ broken ,,, }", "uninstall must not rewrite a file it cannot parse"
+
+
+def test_a_query_never_raises_on_an_undecodable_toml_config(tmp_path, monkeypatch):
+    """The TOML branch of installed() must uphold the same "never raises" contract as the
+    JSON branch: a config that exists but is not UTF-8 (a UTF-16 file, a stray byte) means
+    "not known to be there", not a crash. It read the file with no guard before."""
+    monkeypatch.setenv("HOME", str(tmp_path))
+    cfg = tmp_path / ".kimi-code" / "config.toml"
+    cfg.parent.mkdir()
+    cfg.write_bytes('event = "x"'.encode("utf-16"))
+
+    assert I.installed("kimi_code") is False
+    assert cfg.read_bytes()[:2] == b"\xff\xfe", "a read-only query must not touch the file"