Keep a dead server explainable after its record is pruned

declib/api/server_registry.py

@@ -72,6 +72,83 @@ def unregister_server(server_id: str) -> bool:
         return False
 
 
+# A pruned record is the only evidence that a server ever existed. Deleting it
+# outright means the *next* command can explain what happened but every command
+# after that cannot -- and the command agents run most is `list`, which prunes.
+# Measured on a 40-binary benchmark: only 10 of 79 "missing server" errors
+# carried the useful diagnosis; 69 had already had the corpse eaten by an
+# intervening `list`. So pruning leaves a tombstone behind instead.
+_TOMBSTONE_SUFFIX = ".dead.json"
+_TOMBSTONE_KEEP = 16
+
+
+def _tombstone_path(server_id: str) -> Path:
+    return _registry_dir() / f"{server_id}{_TOMBSTONE_SUFFIX}"
+
+
+def _write_tombstone(record: Dict) -> None:
+    """Record that a server died, so later commands can still say so."""
+    server_id = record.get("id")
+    if not server_id:
+        return
+    payload = dict(record)
+    payload["died_at"] = time.time()
+    try:
+        path = _tombstone_path(str(server_id))
+        tmp_path = path.with_suffix(".tmp")
+        with open(tmp_path, "w") as f:
+            json.dump(payload, f, indent=2, default=str)
+        os.replace(tmp_path, path)
+    except Exception as exc:
+        _l.debug("Failed to write tombstone for %s: %s", server_id, exc)
+        return
+    _trim_tombstones()
+
+
+def _trim_tombstones() -> None:
+    """Keep only the most recent tombstones; they are a hint, not a log."""
+    try:
+        stones = sorted(
+            _registry_dir().glob(f"*{_TOMBSTONE_SUFFIX}"),
+            key=lambda p: p.stat().st_mtime,
+            reverse=True,
+        )
+    except Exception:
+        return
+    for old in stones[_TOMBSTONE_KEEP:]:
+        try:
+            old.unlink()
+        except Exception:
+            pass
+
+
+def list_tombstones() -> List[Dict]:
+    """Recently-died servers, newest first."""
+    out: List[Dict] = []
+    try:
+        stones = sorted(_registry_dir().glob(f"*{_TOMBSTONE_SUFFIX}"))
+    except FileNotFoundError:
+        return []
+    for entry in stones:
+        try:
+            with open(entry, "r") as f:
+                out.append(json.load(f))
+        except Exception:
+            continue
+    out.sort(key=lambda r: r.get("died_at") or 0, reverse=True)
+    return out
+
+
+def clear_tombstone(server_id: str) -> None:
+    """Forget a death -- called when a binary is successfully reloaded."""
+    try:
+        _tombstone_path(str(server_id)).unlink()
+    except FileNotFoundError:
+        pass
+    except Exception:
+        pass
+
+
 def _is_record_live(record: Dict) -> bool:
     pid = record.get("pid")
     socket_path = record.get("socket_path")
@@ -96,7 +173,10 @@ def list_servers(prune_stale: bool = True, pruned: Optional[List[Dict]] = None)
     """
     records: List[Dict] = []
     try:
-        entries = sorted(_registry_dir().glob("*.json"))
+        entries = sorted(
+            e for e in _registry_dir().glob("*.json")
+            if not e.name.endswith(_TOMBSTONE_SUFFIX)
+        )
     except FileNotFoundError:
         return []
 
@@ -111,6 +191,7 @@ def list_servers(prune_stale: bool = True, pruned: Optional[List[Dict]] = None)
         if prune_stale and not _is_record_live(record):
             if pruned is not None:
                 pruned.append(record)
+            _write_tombstone(record)
             try:
                 entry.unlink()
             except FileNotFoundError:

declib/cli/decompiler_cli.py

@@ -213,6 +213,16 @@ def _select_server(
             if (not server_id or r.get("id") == server_id)
             and (not backend or r.get("backend") == backend)
         ]
+        # `reaped` only holds servers *this* call pruned. Whichever command ran
+        # first after the death already pruned it -- usually `list` -- so for
+        # every later command the corpse is only in the tombstones.
+        if not dead:
+            dead = [
+                r for r in server_registry.list_tombstones()
+                if (not server_id or r.get("id") == server_id)
+                and (not backend or r.get("backend") == backend)
+                and (not binary_path or r.get("binary_path") == binary_path)
+            ]
         if dead:
             corpse = dead[0]
             binary = corpse.get("binary_path") or "<unknown binary>"
@@ -600,6 +610,10 @@ def cmd_load(args) -> int:
         timeout=args.timeout,
         backend=backend,
     )
+    # The binary is usable again; stop reporting the old death.
+    for stone in server_registry.list_tombstones():
+        if stone.get("binary_path") == str(binary_path):
+            server_registry.clear_tombstone(str(stone.get("id")))
     _emit(args, {
         "status": "started",
         "id": record["id"],
@@ -642,6 +656,7 @@ def cmd_list(args) -> int:
         return 0
     if not records:
         print(f"No running decompiler servers.  (registry: {registry_dir})")
+        _report_recent_deaths()
         return 0
     print(f"{'ID':<12} {'BACKEND':<8} {'PID':<8} BINARY")
     for r in records:
@@ -650,6 +665,20 @@ def cmd_list(args) -> int:
     return 0
 
 
+
+def _report_recent_deaths() -> None:
+    """Tell the caller a server died, rather than letting it read as never-started."""
+    stones = server_registry.list_tombstones()
+    if not stones:
+        return
+    print("\nRecently died (analysis is gone; reload to continue):")
+    for stone in stones[:5]:
+        binary = stone.get("binary_path") or "<unknown binary>"
+        cmd = f"decompiler load {binary}"
+        if stone.get("backend"):
+            cmd += f" --backend {stone['backend']}"
+        print(f"  {stone.get('id','?'):<12} {binary}\n      reload: {cmd}")
+
 def _stop_server_by_record(record: Dict, save_mode: Optional[str] = None) -> bool:
     """Shut down the server process backing `record`.
 

tests/test_decompiler_cli.py

@@ -2525,6 +2525,83 @@ def test_pruned_records_are_reported(self):
                 self.assertEqual(reaped[0]["binary_path"], "/tmp/gone")
             finally:
                 os.environ.pop("DECLIB_SERVER_REGISTRY", None)
+class TestServerTombstones(unittest.TestCase):
+    """A death must stay explainable after the corpse is pruned.
+
+    `list` prunes, and `list` is the command agents run most, so without a
+    tombstone the useful "your server died" diagnosis is destroyed by the very
+    command used to check state -- and every later command reads as though no
+    server was ever started.
+    """
+
+    def setUp(self):
+        import tempfile
+        self._tmp = tempfile.TemporaryDirectory()
+        os.environ["DECLIB_SERVER_REGISTRY"] = self._tmp.name
+
+    def tearDown(self):
+        os.environ.pop("DECLIB_SERVER_REGISTRY", None)
+        self._tmp.cleanup()
+
+    def _dead_record(self):
+        from declib.api import server_registry
+        server_registry.register_server({
+            "id": "deadbeef01", "socket_path": "/nonexistent/x.sock",
+            "binary_path": "/tmp/gone", "backend": "ida", "pid": 999999,
+        })
+
+    def test_pruning_leaves_a_tombstone(self):
+        from declib.api import server_registry
+
+        self._dead_record()
+        self.assertEqual(server_registry.list_servers(), [])
+        stones = server_registry.list_tombstones()
+        self.assertEqual([s["id"] for s in stones], ["deadbeef01"])
+        self.assertEqual(stones[0]["binary_path"], "/tmp/gone")
+        self.assertIn("died_at", stones[0])
+
+    def test_diagnosis_survives_a_second_lookup(self):
+        """The regression: the first call pruned, so the second went blind."""
+        from declib.api import server_registry
+
+        self._dead_record()
+        server_registry.list_servers()          # first call eats the corpse
+        stones = server_registry.list_tombstones()
+        self.assertTrue(stones, "second lookup must still be able to explain")
+
+    def test_tombstones_are_not_mistaken_for_live_records(self):
+        from declib.api import server_registry
+
+        self._dead_record()
+        server_registry.list_servers()
+        # The tombstone is a *.json file in the same directory; it must never
+        # come back as a server.
+        self.assertEqual(server_registry.list_servers(), [])
+        self.assertIsNone(server_registry.find_server(server_id="deadbeef01"))
+
+    def test_reload_clears_the_tombstone(self):
+        from declib.api import server_registry
+
+        self._dead_record()
+        server_registry.list_servers()
+        self.assertTrue(server_registry.list_tombstones())
+        server_registry.clear_tombstone("deadbeef01")
+        self.assertEqual(server_registry.list_tombstones(), [])
+
+    def test_tombstones_are_capped(self):
+        """A hint, not a log -- they must not grow without bound."""
+        from declib.api import server_registry
+
+        for i in range(server_registry._TOMBSTONE_KEEP + 10):
+            server_registry.register_server({
+                "id": f"corpse{i:04d}", "socket_path": "/nonexistent/x.sock",
+                "binary_path": f"/tmp/gone{i}", "backend": "ida", "pid": 999999,
+            })
+            server_registry.list_servers()
+        self.assertLessEqual(len(server_registry.list_tombstones()),
+                             server_registry._TOMBSTONE_KEEP)
+
+
 class TestDisassembleRangeArgs(unittest.TestCase):
     """Range-disassembly plumbing that needs no backend or fixture binary."""