fix(api): cross-platform delete_kb + endpoint OSError handling (xhigh review #1,#2)

- delete_kb no longer holds the ingest lock DURING rmtree (the lock file lives
  inside the KB dir; Windows cannot delete an open file — the prior review-fix
  regressed this). It now takes the lock as a BARRIER (drain + wait out any
  in-flight mutation), releases it, re-checks existence, then rmtrees. [#1]
- delete-KB endpoint maps FileNotFoundError to an idempotent success (a
  concurrent delete already removed the tree) and other OSError to a clean 500
  with a message, instead of an uncaught 500 stack trace. [#2]

Tests: endpoint OSError -> clean 500, FileNotFoundError -> 200 deleted.

Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG

Author: KylinMountain

openkb/api_kbs_router.py

@@ -42,4 +42,10 @@ async def delete_kb_endpoint(
         await run_in_threadpool(delete_kb, kb_dir)
     except ValueError as exc:  # resolved to an existing path that is not a KB
         raise HTTPException(status_code=400, detail=str(exc)) from exc
+    except FileNotFoundError:
+        pass  # a concurrent delete already removed it — idempotent success
+    except OSError as exc:  # rmtree failed (permission/disk; a still-open file on Windows)
+        raise HTTPException(
+            status_code=500, detail=f"Failed to delete the knowledge base: {exc}"
+        ) from exc
     return KbDeleteResponse(deleted=True, kb=request.kb, path=str(kb_dir))

openkb/kb_admin.py

@@ -56,9 +56,14 @@ def delete_kb(kb_dir: Path) -> None:
     if kb_dir.exists():
         if not config._is_kb_dir(kb_dir):
             raise ValueError(f"Refusing to delete: not a knowledge base directory: {kb_dir}")
-        # Serialize against in-flight ingest/recompile on this KB (like `openkb
-        # remove`). POSIX: rmtree unlinks the held lock file, but the open fd
-        # stays valid, so the funlock on context-exit is safe.
+        # Serialize against in-flight ingest/recompile by acquiring the ingest
+        # lock as a BARRIER (it drains pending journals and waits out any active
+        # mutation), then RELEASING it before rmtree. The lock file lives INSIDE
+        # kb_dir and Windows cannot delete a still-open file, so it must not be
+        # held during rmtree. Re-check existence in case a concurrent delete won
+        # the race while we waited on the barrier.
         with kb_ingest_lock(kb_dir / ".openkb"):
+            pass
+        if kb_dir.exists():
             shutil.rmtree(kb_dir)
     unregister_kb(kb_dir)

tests/test_api.py

@@ -3316,3 +3316,45 @@ def test_summary_page_is_editable_but_not_deletable(monkeypatch, kb_dir):
         "/api/v1/page/delete", json={"kb": kb, "path": "summaries/doc"}, headers=_auth()
     )
     assert r.status_code == 400
+
+
+def test_delete_kb_maps_oserror_to_clean_500(monkeypatch, tmp_path):
+    # An rmtree failure (permission/disk, or a still-open lock file on Windows)
+    # surfaces as a clean 500 with a message, not an uncaught stack trace.
+    from openkb.config import register_kb_alias
+
+    _isolate_global(monkeypatch, tmp_path)
+    kb = _make_kb(tmp_path / "mykb")
+    register_kb_alias("boom-kb", kb)
+
+    def boom(_):
+        raise OSError("disk on fire")
+
+    monkeypatch.setattr("openkb.api_kbs_router.delete_kb", boom)
+    client = _client(monkeypatch)
+    r = client.post(
+        "/api/v1/kb/delete", json={"kb": "boom-kb", "confirm_name": "boom-kb"}, headers=_auth()
+    )
+    assert r.status_code == 500
+    assert "Failed to delete" in r.json()["detail"]
+
+
+def test_delete_kb_filenotfound_is_idempotent(monkeypatch, tmp_path):
+    # A concurrent delete that already removed the tree (FileNotFoundError) is a
+    # success, not a 500 — deleting an already-gone KB is idempotent.
+    from openkb.config import register_kb_alias
+
+    _isolate_global(monkeypatch, tmp_path)
+    kb = _make_kb(tmp_path / "mykb")
+    register_kb_alias("gone-kb", kb)
+
+    def already_gone(_):
+        raise FileNotFoundError()
+
+    monkeypatch.setattr("openkb.api_kbs_router.delete_kb", already_gone)
+    client = _client(monkeypatch)
+    r = client.post(
+        "/api/v1/kb/delete", json={"kb": "gone-kb", "confirm_name": "gone-kb"}, headers=_auth()
+    )
+    assert r.status_code == 200
+    assert r.json()["deleted"] is True