persist tool removal in the index — uninstalls stayed in-memory only

remove_tool_by_id popped the entry from every in-memory map, but the
persisted singleton index still carried it. Every populate broadcasts a
reload_tool_source_cache invalidation, so the very next reload handed
the entry back and the uninstalled tool resolved again
(test_repository_uninstall expected err_msg, got the full tool).

ToolSourceStore grows remove_index_entry — the uninstall counterpart of
update_index_entry — and remove_tool_by_id writes the removal through.
This commit is contained in:
mvdbeek
2026-07-28 17:27:24 +02:00
parent efd598802e
commit 0cf910ddac
4 changed files with 53 additions and 0 deletions
+21
View File
@@ -176,6 +176,27 @@ class ToolSourceStore(ABC):
entry: The index entry to update.
"""
def remove_index_entry(self, tool_id: str) -> None:
"""Remove a tool's entry from the persisted index.
Counterpart of :meth:`update_index_entry` for uninstalls: the lazy
toolbox pops the entry from its in-memory index, but the persisted
singleton would hand it right back on the next cache invalidation
unless the removal is written through.
"""
index = self.load_index()
if index is None:
return
removed = index.entries.pop(tool_id, None)
removed_versions = index.entries_by_version.pop(tool_id, None)
if removed is None and removed_versions is None:
return
for section_tool_ids in index.by_section.values():
if tool_id in section_tool_ids:
section_tool_ids.remove(tool_id)
index.invalidate_caches()
self.store_index(index)
def invalidate_index_cache(self) -> None: # noqa: B027 — intentional empty default
"""Drop any in-memory cached index so the next load_index() reads fresh.
+10
View File
@@ -1229,6 +1229,16 @@ class LazyToolBox(ToolBox):
with self._cache_lock:
for key in [k for k in self._tool_object_cache.keys() if k.startswith(f"{tool_id}:")]:
self._tool_object_cache.pop(key, None)
if self._store is not None:
# The pops above are in-memory. The persisted singleton index
# still carries the entry, and any later cache invalidation
# (every populate broadcasts one) would reload it —
# resurrecting an uninstalled tool.
try:
self._store.remove_index_entry(tool_id)
self._store.commit()
except Exception as e:
log.warning("Persisting index removal of %s raised: %s", tool_id, e)
return result
# === Override has_tool to check index ===
@@ -73,6 +73,9 @@ class CompositeToolSourceStore(ToolSourceStore):
def update_index_entry(self, entry: ToolIndexEntry) -> None:
self._default_store.update_index_entry(entry)
def remove_index_entry(self, tool_id: str) -> None:
self._default_store.remove_index_entry(tool_id)
# --- read ops: priority order --------------------------------------
def get(self, hash: str) -> StoredToolSource | None:
@@ -194,6 +194,25 @@ class TestDatabaseBackendPathRows:
store.delete("edited_hash_v2")
app.model.context.commit()
def test_remove_index_entry_persists_removal(self):
app = MockApp()
store = DatabaseToolSourceStore(app.model.context) # type: ignore[arg-type]
index = ToolIndex()
entry = ToolIndexEntry(id="removable", version="1.0", name="Removable", panel_section_id="sec1")
index.add_entry(entry)
index.by_section["sec1"] = ["removable"]
store.store_index(index)
app.model.context.commit()
store.remove_index_entry("removable")
app.model.context.commit()
store.invalidate_index_cache()
reloaded = store.load_index()
assert reloaded is not None
assert reloaded.get("removable") is None
assert "removable" not in reloaded.by_section.get("sec1", [])
def test_pathless_sources_dedupe_on_hash(self):
app = MockApp()
store = DatabaseToolSourceStore(app.model.context) # type: ignore[arg-type]