Skip to content

Commit a1804a9

Browse files
GoodbyePlanetclaude
andcommitted
fix: guard prune_orphaned_services against empty config, add test coverage
An empty configured-services set (e.g. config.yaml with services: []) would previously cause every indexed service to be treated as orphaned and deleted. Also ensures the collection exists before pruning, filters the "unknown" service placeholder out of get_indexed_services, and adds tests for the new pruning behavior across cleanup.py, pipeline.py, git_history.py, and qdrant.py. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent b1c998d commit a1804a9

8 files changed

Lines changed: 183 additions & 15 deletions

File tree

server/indexer/cleanup.py

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,29 @@ async def prune_orphaned_services(
2020
label: str = "data",
2121
) -> list[str]:
2222
"""Delete all stored data for services that exist in the store but not in
23-
the configured set. Returns the list of orphaned service names that were pruned."""
23+
the configured set. Returns the list of orphaned service names that were pruned.
24+
25+
Refuses to prune when `configured_names` is empty — an empty configured set
26+
is far more likely to be a load error or operator mistake than a genuine
27+
intent to wipe every indexed service, and deletion is not reversible.
28+
"""
29+
if not configured_names:
30+
logger.warning(
31+
"No configured services; skipping prune of %s to avoid wiping the store.",
32+
label,
33+
)
34+
return []
35+
2436
indexed_names = await store.get_indexed_services()
25-
orphaned = set(indexed_names) - configured_names
37+
orphaned = sorted(set(indexed_names) - configured_names)
2638

27-
for name in sorted(orphaned):
39+
for name in orphaned:
2840
logger.warning("Pruning orphaned service %r from %s", name, label)
2941
await store.delete_by_service(name)
3042

31-
return list(orphaned)
43+
if orphaned:
44+
logger.info(
45+
"Pruned %d orphaned service(s) from %s: %s", len(orphaned), label, orphaned
46+
)
47+
48+
return orphaned

server/indexer/git_history.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -198,12 +198,10 @@ async def index_all(
198198
progress_callback: Callable[[ProgressEvent], Awaitable[None]] | None = None,
199199
) -> dict[str, Any]:
200200
services = settings.load_services()
201-
configured_names = {s.name for s in services}
202-
orphaned = await prune_orphaned_services(
203-
self._store, configured_names, label="git commits"
201+
await self._store.ensure_collection()
202+
await prune_orphaned_services(
203+
self._store, {s.name for s in services}, label="git commits"
204204
)
205-
if orphaned:
206-
logger.info("Pruned %d orphaned service(s): %s", len(orphaned), orphaned)
207205

208206
results: dict[str, Any] = {}
209207
for svc in services:

server/indexer/pipeline.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -282,12 +282,10 @@ async def index_all(
282282
progress_callback: Callable[[ProgressEvent], Awaitable[None]] | None = None,
283283
) -> dict[str, Any]:
284284
services = settings.load_services()
285-
configured_names = {s.name for s in services}
286-
orphaned = await prune_orphaned_services(
287-
self._store, configured_names, label="code symbols"
285+
await self._store.ensure_collection()
286+
await prune_orphaned_services(
287+
self._store, {s.name for s in services}, label="code symbols"
288288
)
289-
if orphaned:
290-
logger.info("Pruned %d orphaned service(s): %s", len(orphaned), orphaned)
291289

292290
results: dict[str, Any] = {}
293291
for svc in services:

server/store/qdrant.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,9 @@ async def get_service_stats(self) -> list[dict[str, Any]]:
312312
async def get_indexed_services(self) -> list[str]:
313313
"""Return distinct service names that have indexed code symbols."""
314314
stats = await self.get_service_stats()
315-
return sorted(s["service"] for s in stats)
315+
return sorted(
316+
s["service"] for s in stats if s["service"] and s["service"] != "unknown"
317+
)
316318

317319
async def collection_info(self) -> dict[str, Any]:
318320
info = await self._client.get_collection(self._collection)

tests/test_cleanup.py

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
from __future__ import annotations
2+
3+
from server.indexer.cleanup import prune_orphaned_services
4+
5+
6+
class _FakeStore:
7+
def __init__(self, indexed_names: list[str]) -> None:
8+
self.indexed_names = indexed_names
9+
self.deleted: list[str] = []
10+
11+
async def get_indexed_services(self) -> list[str]:
12+
return self.indexed_names
13+
14+
async def delete_by_service(self, service: str) -> None:
15+
self.deleted.append(service)
16+
17+
18+
async def test_service_absent_from_config_is_deleted() -> None:
19+
store = _FakeStore(["kept", "renamed-away"])
20+
21+
orphaned = await prune_orphaned_services(store, {"kept"})
22+
23+
assert orphaned == ["renamed-away"]
24+
assert store.deleted == ["renamed-away"]
25+
26+
27+
async def test_configured_service_is_never_deleted() -> None:
28+
store = _FakeStore(["kept"])
29+
30+
orphaned = await prune_orphaned_services(store, {"kept"})
31+
32+
assert orphaned == []
33+
assert store.deleted == []
34+
35+
36+
async def test_empty_configured_names_skips_prune_entirely() -> None:
37+
store = _FakeStore(["a", "b", "c"])
38+
39+
orphaned = await prune_orphaned_services(store, set())
40+
41+
assert orphaned == []
42+
assert store.deleted == []
43+
44+
45+
async def test_no_indexed_services_is_a_noop() -> None:
46+
store = _FakeStore([])
47+
48+
orphaned = await prune_orphaned_services(store, {"kept"})
49+
50+
assert orphaned == []
51+
assert store.deleted == []
52+
53+
54+
async def test_multiple_orphans_are_returned_sorted() -> None:
55+
store = _FakeStore(["zeta", "alpha", "kept", "beta"])
56+
57+
orphaned = await prune_orphaned_services(store, {"kept"})
58+
59+
assert orphaned == ["alpha", "beta", "zeta"]
60+
assert store.deleted == ["alpha", "beta", "zeta"]

tests/test_git_history.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
from __future__ import annotations
22

3+
from unittest.mock import AsyncMock, patch
4+
5+
import server.indexer.git_history as git_history_module
6+
from server.config import ServiceConfig
37
from server.indexer.git_history import (
48
_MAX_PATCH_CHARS,
9+
GitHistoryPipeline,
510
_build_embedding_text,
611
_commit_to_payload,
712
)
@@ -27,6 +32,42 @@ def _file(filename: str = "src/Foo.java", patch: str | None = None) -> CommitFil
2732
)
2833

2934

35+
async def test_index_all_prunes_orphaned_services_before_indexing() -> None:
36+
store = AsyncMock()
37+
store.ensure_collection = AsyncMock()
38+
store.get_indexed_services = AsyncMock(return_value=["kept", "renamed-away"])
39+
store.delete_by_service = AsyncMock()
40+
pipeline = GitHistoryPipeline(store)
41+
pipeline.index_service = AsyncMock(return_value={"commits": 0})
42+
43+
with patch.object(
44+
type(git_history_module.settings),
45+
"load_services",
46+
return_value=[ServiceConfig(name="kept", github_repo="org/kept", exclude=[])],
47+
):
48+
await pipeline.index_all()
49+
50+
store.delete_by_service.assert_awaited_once_with("renamed-away")
51+
pipeline.index_service.assert_awaited_once_with(
52+
"kept", force=False, progress_callback=None
53+
)
54+
55+
56+
async def test_index_all_skips_prune_when_no_services_configured() -> None:
57+
store = AsyncMock()
58+
store.ensure_collection = AsyncMock()
59+
store.get_indexed_services = AsyncMock(return_value=["kept"])
60+
store.delete_by_service = AsyncMock()
61+
pipeline = GitHistoryPipeline(store)
62+
63+
with patch.object(
64+
type(git_history_module.settings), "load_services", return_value=[]
65+
):
66+
await pipeline.index_all()
67+
68+
store.delete_by_service.assert_not_awaited()
69+
70+
3071
def test_embedding_text_contains_service_and_author() -> None:
3172
text = _build_embedding_text(_commit(), "auth-server")
3273
assert "auth-server" in text

tests/test_pipeline.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,42 @@ def _sym(docstring: str) -> CodeSymbol:
5757
)
5858

5959

60+
async def test_index_all_prunes_orphaned_services_before_indexing() -> None:
61+
store = AsyncMock()
62+
store.ensure_collection = AsyncMock()
63+
store.get_indexed_services = AsyncMock(return_value=["kept", "renamed-away"])
64+
store.delete_by_service = AsyncMock()
65+
pipeline = _make_pipeline(store)
66+
pipeline.index_service = AsyncMock(
67+
return_value={"files": 1, "chunks": 1, "skipped": 0}
68+
)
69+
70+
with patch.object(
71+
type(pipeline_module.settings),
72+
"load_services",
73+
return_value=[ServiceConfig(name="kept", github_repo="org/kept", exclude=[])],
74+
):
75+
await pipeline.index_all()
76+
77+
store.delete_by_service.assert_awaited_once_with("renamed-away")
78+
pipeline.index_service.assert_awaited_once_with(
79+
"kept", force=False, progress_callback=None
80+
)
81+
82+
83+
async def test_index_all_skips_prune_when_no_services_configured() -> None:
84+
store = AsyncMock()
85+
store.ensure_collection = AsyncMock()
86+
store.get_indexed_services = AsyncMock(return_value=["kept"])
87+
store.delete_by_service = AsyncMock()
88+
pipeline = _make_pipeline(store)
89+
90+
with patch.object(type(pipeline_module.settings), "load_services", return_value=[]):
91+
await pipeline.index_all()
92+
93+
store.delete_by_service.assert_not_awaited()
94+
95+
6096
def test_python_triple_double_quote_stripped() -> None:
6197
text = _build_embedding_text(_sym('"""Hello world"""'), "svc")
6298
assert "Hello world" in text

tests/test_store.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,22 @@ def test_close_is_coroutine() -> None:
1818
assert asyncio.iscoroutinefunction(QdrantStore.close)
1919

2020

21+
async def test_get_indexed_services_excludes_unknown_placeholder() -> None:
22+
"""The 'unknown' placeholder from unlabeled payloads must never look like a
23+
real service name that pruning could delete."""
24+
store = QdrantStore.__new__(QdrantStore)
25+
store.get_service_stats = AsyncMock(
26+
return_value=[
27+
{"service": "billing", "symbols": 3},
28+
{"service": "unknown", "symbols": 1},
29+
]
30+
)
31+
32+
services = await store.get_indexed_services()
33+
34+
assert services == ["billing"]
35+
36+
2137
async def test_find_by_name_fuzzy_scans_all_pages() -> None:
2238
"""Non-exact search must paginate instead of relying on the first 20 results."""
2339
store = QdrantStore.__new__(QdrantStore)

0 commit comments

Comments
 (0)