Serialize shared-connection access across skill tools and the sandbox

This commit is contained in:
Yiorgis Gozadinos 2026-06-05 17:22:11 +03:00
parent b47583258e
commit e59ee56002
No known key found for this signature in database
7 changed files with 109 additions and 37 deletions

View file

@ -3,7 +3,7 @@
### Fixed ### Fixed
- Analysis-skill `execute_code` document VFS reads (`content.txt`, `items.jsonl`, `toc.json`) run on the calling event loop through the skill's connection, instead of a separate connection on a background loop. - Skill tools (`search`/`cite`/`list_documents`/`get_document`) and the analysis sandbox serialize access to the shared LanceDB connection through one lock, so a turn's concurrently executed tool calls no longer trigger `RuntimeError: Already borrowed`.
## [0.55.0] - 2026-06-05 ## [0.55.0] - 2026-06-05

View file

@ -133,6 +133,7 @@ class Sandbox:
_config: AppConfig _config: AppConfig
_context: AnalysisContext _context: AnalysisContext
_rag: "HaikuRAG | None" _rag: "HaikuRAG | None"
_lock: "asyncio.Lock | None"
_search_results: "list[SearchResult]" _search_results: "list[SearchResult]"
_doc_items: dict[str, list["DocumentItem"]] _doc_items: dict[str, list["DocumentItem"]]
_doc_chunk_index: dict[str, dict[str, list[str]]] _doc_chunk_index: dict[str, dict[str, list[str]]]
@ -148,11 +149,13 @@ class Sandbox:
config: AppConfig, config: AppConfig,
context: AnalysisContext, context: AnalysisContext,
rag: "HaikuRAG | None" = None, rag: "HaikuRAG | None" = None,
lock: "asyncio.Lock | None" = None,
): ):
self._db_path = db_path self._db_path = db_path
self._config = config self._config = config
self._context = context self._context = context
self._rag = rag self._rag = rag
self._lock = lock
self._search_results = [] self._search_results = []
self._doc_items = {} self._doc_items = {}
self._doc_chunk_index = {} self._doc_chunk_index = {}
@ -164,8 +167,14 @@ class Sandbox:
@asynccontextmanager @asynccontextmanager
async def _connection(self) -> "AsyncIterator[HaikuRAG]": async def _connection(self) -> "AsyncIterator[HaikuRAG]":
"""Yield the supplied connection, or an ephemeral read-only one.""" """Yield the shared connection (serialized by the lock), or an ephemeral
read-only one. The lock guards the whole block so a read's awaits cannot
interleave with another task's operation on the same connection."""
if self._rag is not None: if self._rag is not None:
if self._lock is not None:
async with self._lock:
yield self._rag
else:
yield self._rag yield self._rag
return return
from haiku.rag.client import HaikuRAG from haiku.rag.client import HaikuRAG

View file

@ -1,3 +1,4 @@
import asyncio
from collections.abc import AsyncIterator from collections.abc import AsyncIterator
from contextlib import asynccontextmanager from contextlib import asynccontextmanager
from dataclasses import dataclass from dataclasses import dataclass
@ -15,6 +16,10 @@ if TYPE_CHECKING:
@dataclass @dataclass
class RAGRunDeps(SkillRunDeps): class RAGRunDeps(SkillRunDeps):
rag: "HaikuRAG | None" = None rag: "HaikuRAG | None" = None
# pydantic-ai runs a turn's tool calls concurrently; LanceDB's per-connection
# state cannot take two in-flight operations at once, so every use of ``rag``
# (skill tools and the analysis sandbox) serializes through this lock.
rag_lock: "asyncio.Lock | None" = None
search_count: int = 0 search_count: int = 0
@ -50,6 +55,7 @@ def make_rag_lifespan(db_path: Path, config: AppConfig):
async with HaikuRAG(db_path, config=config, read_only=True) as rag: async with HaikuRAG(db_path, config=config, read_only=True) as rag:
deps.rag = rag deps.rag = rag
deps.rag_lock = asyncio.Lock()
deps.search_count = 0 deps.search_count = 0
_reset_invocation_state(deps.state) _reset_invocation_state(deps.state)
yield yield
@ -66,12 +72,14 @@ def make_analysis_lifespan(db_path: Path, config: AppConfig):
doc_filter = getattr(deps.state, "document_filter", None) doc_filter = getattr(deps.state, "document_filter", None)
async with HaikuRAG(db_path, config=config, read_only=True) as rag: async with HaikuRAG(db_path, config=config, read_only=True) as rag:
deps.rag = rag deps.rag = rag
deps.rag_lock = asyncio.Lock()
deps.search_count = 0 deps.search_count = 0
sandbox = Sandbox( sandbox = Sandbox(
db_path=db_path, db_path=db_path,
config=config, config=config,
context=AnalysisContext(filter=doc_filter), context=AnalysisContext(filter=doc_filter),
rag=rag, rag=rag,
lock=deps.rag_lock,
) )
deps.sandbox = sandbox deps.sandbox = sandbox
_reset_invocation_state(deps.state) _reset_invocation_state(deps.state)

View file

@ -1,3 +1,4 @@
import asyncio
from pathlib import Path from pathlib import Path
from typing import Any from typing import Any
@ -84,6 +85,13 @@ def _require_rag(ctx: RunContext[RAGRunDeps]) -> HaikuRAG:
return ctx.deps.rag return ctx.deps.rag
def _require_lock(ctx: RunContext[RAGRunDeps]) -> asyncio.Lock:
assert ctx.deps is not None and ctx.deps.rag_lock is not None, (
"RAGRunDeps.rag_lock is not set — skill lifespan must run before tools."
)
return ctx.deps.rag_lock
def _register_citations(state: Any, citations: "list[Citation]") -> None: def _register_citations(state: Any, citations: "list[Citation]") -> None:
"""Add citations to the index and record cited chunk IDs for this invocation.""" """Add citations to the index and record cited chunk IDs for this invocation."""
next_index = len(state.citation_index) + 1 next_index = len(state.citation_index) + 1
@ -195,6 +203,7 @@ def create_skill_tools(
) )
state = _get_state(ctx, state_type) state = _get_state(ctx, state_type)
async with _require_lock(ctx):
formatted, results = await skill_search( formatted, results = await skill_search(
_require_rag(ctx), _require_rag(ctx),
query, query,
@ -221,6 +230,7 @@ def create_skill_tools(
) -> list[dict[str, Any]]: ) -> list[dict[str, Any]]:
"""List all documents in the knowledge base.""" """List all documents in the knowledge base."""
state = _get_state(ctx, state_type) state = _get_state(ctx, state_type)
async with _require_lock(ctx):
return await skill_list_documents( return await skill_list_documents(
_require_rag(ctx), _require_rag(ctx),
filter=state.document_filter if state else None, filter=state.document_filter if state else None,
@ -238,6 +248,7 @@ def create_skill_tools(
Args: Args:
query: Document ID, title, or URI to look up. query: Document ID, title, or URI to look up.
""" """
async with _require_lock(ctx):
return await skill_get_document(_require_rag(ctx), query) return await skill_get_document(_require_rag(ctx), query)
tools["get_document"] = get_document tools["get_document"] = get_document
@ -325,6 +336,7 @@ def create_skill_tools(
] ]
if missing: if missing:
async with _require_lock(ctx):
rag = _require_rag(ctx) rag = _require_rag(ctx)
synthetic: list[SearchResult] = [] synthetic: list[SearchResult] = []
doc_cache: dict[str, Any] = {} doc_cache: dict[str, Any] = {}

View file

@ -492,8 +492,8 @@ class TestSandboxHeldConnection:
async def test_vfs_read_concurrent_with_connection_read(self, temp_db_path): async def test_vfs_read_concurrent_with_connection_read(self, temp_db_path):
"""A VFS read and a direct read on the shared connection run together. """A VFS read and a direct read on the shared connection run together.
Both go through one connection on one loop, so the concurrency that Both serialize through the shared lock, so concurrent tasks never have
used to cross two event loops no longer borrows the same LanceDB state. two operations in flight on the one connection at once.
""" """
config = AppConfig() config = AppConfig()
async with HaikuRAG(temp_db_path, create=True) as client: async with HaikuRAG(temp_db_path, create=True) as client:
@ -505,19 +505,26 @@ class TestSandboxHeldConnection:
assert doc.id assert doc.id
async with HaikuRAG(temp_db_path, config=config, read_only=True) as rag: async with HaikuRAG(temp_db_path, config=config, read_only=True) as rag:
lock = asyncio.Lock()
sb = Sandbox( sb = Sandbox(
db_path=temp_db_path, db_path=temp_db_path,
config=config, config=config,
context=AnalysisContext(), context=AnalysisContext(),
rag=rag, rag=rag,
lock=lock,
) )
read_code = ( read_code = (
"from pathlib import Path\n" "from pathlib import Path\n"
f"print(Path('/documents/{doc.id}/content.txt').read_text())" f"print(Path('/documents/{doc.id}/content.txt').read_text())"
) )
async def direct_read() -> str | None:
async with lock:
return await rag.document_repository.get_content(doc.id)
exec_result, content = await asyncio.gather( exec_result, content = await asyncio.gather(
sb.execute(read_code), sb.execute(read_code),
rag.document_repository.get_content(doc.id), direct_read(),
) )
assert exec_result.success, exec_result.stderr assert exec_result.success, exec_result.stderr
assert "Foxes" in exec_result.stdout assert "Foxes" in exec_result.stdout

View file

@ -1,3 +1,4 @@
import asyncio
import random import random
from unittest.mock import MagicMock from unittest.mock import MagicMock
@ -17,10 +18,11 @@ def _make_ctx(state=None, rag=None, sandbox=None):
from haiku.rag.skills.analysis import AnalysisState from haiku.rag.skills.analysis import AnalysisState
ctx = MagicMock(spec=RunContext) ctx = MagicMock(spec=RunContext)
lock = asyncio.Lock()
if isinstance(state, AnalysisState) or sandbox is not None: if isinstance(state, AnalysisState) or sandbox is not None:
ctx.deps = AnalysisRunDeps(state=state, rag=rag, sandbox=sandbox) ctx.deps = AnalysisRunDeps(state=state, rag=rag, sandbox=sandbox, rag_lock=lock)
else: else:
ctx.deps = RAGRunDeps(state=state, rag=rag) ctx.deps = RAGRunDeps(state=state, rag=rag, rag_lock=lock)
return ctx return ctx

View file

@ -1,3 +1,5 @@
import asyncio
import pytest import pytest
from haiku.rag.config.models import AppConfig from haiku.rag.config.models import AppConfig
@ -169,6 +171,38 @@ class TestSearchTool:
assert isinstance(result, str) assert isinstance(result, str)
assert len(result) > 0 assert len(result) > 0
async def test_concurrent_searches_serialize_on_lock(
self, rag_db, rag_client, monkeypatch
):
"""Tool calls in a turn run concurrently; the shared lock keeps only one
connection operation in flight (pydantic-ai borrow guard)."""
from haiku.rag.skills import _tools
from haiku.rag.skills.rag import create_skill
original = _tools.skill_search
inflight = {"now": 0, "max": 0}
async def tracking_search(*args, **kwargs):
inflight["now"] += 1
inflight["max"] = max(inflight["max"], inflight["now"])
try:
await asyncio.sleep(0.05) # widen the window an overlap would use
return await original(*args, **kwargs)
finally:
inflight["now"] -= 1
monkeypatch.setattr(_tools, "skill_search", tracking_search)
skill = create_skill(db_path=rag_db)
search = _get_tool(skill, "search")
ctx = _make_ctx(rag=rag_client) # one ctx -> one shared lock, as in a turn
results = await asyncio.gather(
search(ctx, query="artificial intelligence"),
search(ctx, query="machine learning"),
)
assert all(isinstance(r, str) and r for r in results)
assert inflight["max"] == 1
async def test_search_updates_state(self, rag_db, rag_client): async def test_search_updates_state(self, rag_db, rag_client):
from haiku.rag.skills.rag import RAGState, create_skill from haiku.rag.skills.rag import RAGState, create_skill