fix(graph): skip malformed tunnel records

CHANGELOG.md

@@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 
 ### Bug Fixes
 
+- **Malformed records in `tunnels.json` no longer crash tunnel operations.** An otherwise valid JSON list could contain `null`, strings, or partial endpoint objects; `_load_tunnels` returned them verbatim, allowing one malformed value to crash create, list, follow, or delete during attribute or key access. The loader now keeps only records with a usable string ID and complete string wing/room endpoints, preserves optional legacy fields, and warns with the skipped count without logging record contents. Invalid top-level JSON types also produce a content-free warning instead of being silently treated as empty. (#2266)
 - **`sweep` books a failed `stat` as a failure again.** The non-regular-file gate added in 3.7.1 reads `stat.S_ISREG(f.stat().st_mode)` inside a `try`, and its `except OSError` printed `SKIP` and moved on. A dangling symlink, a symlink loop and a file unlinked between `rglob` and the gate all raise there, and before the gate existed every one of them reached `sweep()` and was booked in `failures` — so `sweep` went from reporting a transcript it could not read to reporting success. A failed probe is now an error, not a benign file type: it is logged, printed as `WARNING`, and appended to `failures`, while a probe that succeeds and says "not regular" still skips silently. (#2221)
 - **`mempalace init` no longer tracebacks on a directory it cannot enter.** `_parse_gradle`'s `is_file()` gate sat in front of the `try` that the parser's own `except OSError` provides, so a manifest under a directory with `r` but no `x` raised `PermissionError` out of a call that used to answer "no manifest name". The gate moved inside that `try`, and `_collect_manifest_names` stats through `os.path.isfile`, which reports rather than raises. (#2221)
 - **`split` no longer blocks on a FIFO at its own output name, nor writes through a broken link.** The type gate in `main` covers the files the glob listed; `split_file` builds its output names itself, so a pre-existing named pipe at one of them wedged `write_text` in the kernel waiting for a reader. The check asks about the link itself rather than its target, because a dangling symlink at an output name reads as "nothing there" and `write_text` would create the target — landing a chunk outside the output directory. Output names that are anything but a regular file are now skipped with a `SKIP` line. (#2221)

mempalace/palace_graph.py

@@ -352,11 +352,34 @@ def _legacy_tunnel_file() -> str:
     return os.path.join(os.path.expanduser("~"), ".mempalace", "tunnels.json")
 
 
+def _is_valid_tunnel_endpoint(endpoint) -> bool:
+    """Return whether an endpoint has the minimum traversable shape."""
+    if not isinstance(endpoint, dict):
+        return False
+    return all(
+        isinstance(endpoint.get(field), str) and endpoint[field].strip()
+        for field in ("wing", "room")
+    )
+
+
+def _is_valid_tunnel_record(record) -> bool:
+    """Return whether a decoded JSON value is safe for every tunnel consumer."""
+    return (
+        isinstance(record, dict)
+        and isinstance(record.get("id"), str)
+        and bool(record["id"].strip())
+        and _is_valid_tunnel_endpoint(record.get("source"))
+        and _is_valid_tunnel_endpoint(record.get("target"))
+    )
+
+
 def _load_tunnels(config=None):
     """Load explicit tunnels from disk.
 
     Returns an empty list if the file is missing or corrupt (e.g. truncated
     by a crash mid-write on a system that lacks atomic-rename semantics).
+    Structurally invalid records inside an otherwise valid JSON list are
+    ignored so one hand-edited value cannot break every tunnel operation.
 
     Backwards-compatibility: prior to 3.3.6 the tunnel file was hardcoded at
     ``~/.mempalace/tunnels.json`` regardless of the configured palace_path.
@@ -381,7 +404,23 @@ def _load_tunnels(config=None):
                 current_tunnel_file,
             )
             return []
-        return data if isinstance(data, list) else []
+        if not isinstance(data, list):
+            logger.warning(
+                "Mempalace tunnels file '%s' has invalid root type %s; "
+                "expected a JSON list; starting empty.",
+                current_tunnel_file,
+                type(data).__name__,
+            )
+            return []
+        tunnels = [record for record in data if _is_valid_tunnel_record(record)]
+        invalid_count = len(data) - len(tunnels)
+        if invalid_count:
+            logger.warning(
+                "Mempalace tunnels file '%s': ignored %d invalid record(s).",
+                current_tunnel_file,
+                invalid_count,
+            )
+        return tunnels
 
     legacy = _legacy_tunnel_file()
     if legacy != current_tunnel_file and os.path.exists(legacy):

tests/test_palace_graph_tunnels.py

@@ -1,5 +1,6 @@
 """Tests for explicit tunnel helpers in mempalace.palace_graph."""
 
+import json
 import logging
 import os
 import stat
@@ -33,6 +34,42 @@ def _use_tmp_tunnel_file(monkeypatch, tmp_path):
     return tunnel_file
 
 
+def _mixed_valid_and_malformed_tunnels():
+    """Return one legacy-compatible tunnel mixed with unusable JSON values."""
+    source = {"wing": "wing_a", "room": "r1"}
+    target = {"wing": "wing_b", "room": "r2"}
+    valid = {
+        "id": "valid-tunnel",
+        "source": source,
+        "target": target,
+        "label": "legacy record without optional fields",
+    }
+    malformed = [
+        None,
+        "junk",
+        [],
+        {},
+        42,
+        {"id": 42, "source": source, "target": target},
+        {"source": source, "target": target},
+        {"id": "missing-source", "target": target},
+        {"id": "missing-target", "source": source},
+        {"id": "null-source", "source": None, "target": target},
+        {"id": "wrong-source-type", "source": "secret-value", "target": target},
+        {
+            "id": "missing-wing",
+            "source": {"room": "r1"},
+            "target": target,
+        },
+        {
+            "id": "blank-target-room",
+            "source": source,
+            "target": {"wing": "wing_b", "room": "   "},
+        },
+    ]
+    return [*malformed, valid], valid, len(malformed)
+
+
 class TestTunnelStorage:
     def test_load_tunnels_missing_file_returns_empty_list(self, tmp_path, monkeypatch):
         _use_tmp_tunnel_file(monkeypatch, tmp_path)
@@ -56,6 +93,70 @@ def test_save_and_load_round_trip(self, tmp_path, monkeypatch):
         palace_graph._save_tunnels(tunnels)
         assert palace_graph._load_tunnels() == tunnels
 
+    def test_load_tunnels_filters_malformed_records_and_warns(self, tmp_path, monkeypatch, caplog):
+        tunnel_file = _use_tmp_tunnel_file(monkeypatch, tmp_path)
+        records, valid, malformed_count = _mixed_valid_and_malformed_tunnels()
+        palace_graph._save_tunnels(records)
+
+        with caplog.at_level(logging.WARNING, logger="mempalace_graph"):
+            loaded = palace_graph._load_tunnels()
+
+        assert loaded == [valid]
+        assert str(tunnel_file) in caplog.text
+        assert f"ignored {malformed_count} invalid record(s)" in caplog.text
+        assert "secret-value" not in caplog.text
+
+    def test_load_tunnels_warns_on_non_list_root_without_logging_contents(
+        self, tmp_path, monkeypatch, caplog
+    ):
+        tunnel_file = _use_tmp_tunnel_file(monkeypatch, tmp_path)
+        palace_graph._save_tunnels(
+            {
+                "id": "single-record",
+                "source": {"wing": "wing_a", "room": "r1"},
+                "target": {"wing": "wing_b", "room": "r2"},
+                "label": "secret-root-label",
+            }
+        )
+
+        with caplog.at_level(logging.WARNING, logger="mempalace_graph"):
+            loaded = palace_graph._load_tunnels()
+
+        assert loaded == []
+        assert str(tunnel_file) in caplog.text
+        assert "invalid root type dict" in caplog.text
+        assert "secret-root-label" not in caplog.text
+
+    @pytest.mark.parametrize("operation", ["create", "list", "follow", "delete"])
+    def test_tunnel_operations_tolerate_malformed_records(self, operation, tmp_path, monkeypatch):
+        tunnel_file = _use_tmp_tunnel_file(monkeypatch, tmp_path)
+        records, valid, _ = _mixed_valid_and_malformed_tunnels()
+        palace_graph._save_tunnels(records)
+
+        if operation == "create":
+            created = palace_graph.create_tunnel(
+                "wing_c", "topic:new", "wing_d", "topic:new", kind="topic"
+            )
+            assert {t["id"] for t in palace_graph._load_tunnels()} == {
+                valid["id"],
+                created["id"],
+            }
+            assert {t["id"] for t in json.loads(tunnel_file.read_text(encoding="utf-8"))} == {
+                valid["id"],
+                created["id"],
+            }
+        elif operation == "list":
+            assert palace_graph.list_tunnels("wing_a") == [valid]
+            assert json.loads(tunnel_file.read_text(encoding="utf-8")) == records
+        elif operation == "follow":
+            connections = palace_graph.follow_tunnels("wing_a", "r1")
+            assert [c["tunnel_id"] for c in connections] == [valid["id"]]
+            assert json.loads(tunnel_file.read_text(encoding="utf-8")) == records
+        else:
+            assert palace_graph.delete_tunnel("missing") == {"deleted": "missing"}
+            assert palace_graph._load_tunnels() == [valid]
+            assert json.loads(tunnel_file.read_text(encoding="utf-8")) == [valid]
+
     @pytest.mark.skipif(
         sys.platform == "win32",
         reason="POSIX file-permission bits only apply on Unix-like systems",