mirror of
https://github.com/NCBM/plyngent.git
synced 2026-11-07 23:47:54 +08:00
core/agent+tools: apply a batch's same-file writes in call order
This commit is contained in:
@@ -22,7 +22,7 @@ from plyngent.lmproto.openai_compatible.model import (
|
||||
)
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import AsyncIterator
|
||||
from collections.abc import AsyncIterator, Mapping
|
||||
|
||||
|
||||
def _response(message: AssistantChatMessage) -> ChatCompletionResponse:
|
||||
@@ -156,3 +156,102 @@ async def test_parallel_tool_calls_run_concurrently() -> None:
|
||||
tool_msgs = [e for e in agent.messages if isinstance(e, ToolChatMessage)]
|
||||
assert {m.content for m in tool_msgs} == {"a", "b"}
|
||||
assert any(getattr(e, "content", None) == "done" for e in events)
|
||||
|
||||
|
||||
async def test_same_file_writes_apply_in_call_order() -> None:
|
||||
"""A batch's writes to one file run one at a time, in call order."""
|
||||
order: list[str] = []
|
||||
|
||||
@tool(register=False, writes=("path",))
|
||||
async def write_a(path: str) -> str:
|
||||
order.append(f"a start {path}")
|
||||
await asyncio.sleep(0) # an unserialized sibling would interleave here
|
||||
order.append(f"a end {path}")
|
||||
return "a"
|
||||
|
||||
@tool(register=False, writes=("path",))
|
||||
async def write_b(path: str) -> str:
|
||||
order.append(f"b start {path}")
|
||||
await asyncio.sleep(0)
|
||||
order.append(f"b end {path}")
|
||||
return "b"
|
||||
|
||||
registry = ToolRegistry([write_a, write_b])
|
||||
results = await asyncio.gather(
|
||||
registry.execute("write_a", '{"path": "a.txt"}'),
|
||||
registry.execute("write_b", '{"path": "./a.txt"}'),
|
||||
)
|
||||
assert results == ["a", "b"]
|
||||
assert order == ["a start a.txt", "a end a.txt", "b start ./a.txt", "b end ./a.txt"]
|
||||
|
||||
|
||||
async def test_different_files_still_write_concurrently() -> None:
|
||||
"""Serialization is per file: two files of one batch still overlap."""
|
||||
overlaps = 0
|
||||
gate = asyncio.Event()
|
||||
|
||||
async def _rendezvous() -> None:
|
||||
nonlocal overlaps
|
||||
overlaps += 1
|
||||
if overlaps == 2:
|
||||
_ = gate.set()
|
||||
_ = await asyncio.wait_for(gate.wait(), timeout=2.0)
|
||||
|
||||
@tool(register=False, writes=("path",))
|
||||
async def write_a(path: str) -> str:
|
||||
del path
|
||||
await _rendezvous()
|
||||
return "a"
|
||||
|
||||
@tool(register=False, writes=("path",))
|
||||
async def write_b(path: str) -> str:
|
||||
del path
|
||||
await _rendezvous()
|
||||
return "b"
|
||||
|
||||
registry = ToolRegistry([write_a, write_b])
|
||||
results = await asyncio.gather(
|
||||
registry.execute("write_a", '{"path": "a.txt"}'),
|
||||
registry.execute("write_b", '{"path": "b.txt"}'),
|
||||
)
|
||||
assert results == ["a", "b"]
|
||||
|
||||
|
||||
async def test_a_write_holds_its_file_across_the_confirm() -> None:
|
||||
"""The file is locked before the prompt, so a sibling write cannot overtake it."""
|
||||
order: list[str] = []
|
||||
prompt_shown = asyncio.Event()
|
||||
answered = asyncio.Event()
|
||||
|
||||
@tool(register=False, writes=("path",))
|
||||
async def write_a(path: str) -> str:
|
||||
del path
|
||||
order.append("a")
|
||||
return "a"
|
||||
|
||||
@tool(register=False, writes=("path",))
|
||||
async def write_b(path: str) -> str:
|
||||
del path
|
||||
order.append("b")
|
||||
return "b"
|
||||
|
||||
async def on_confirm(name: str, args: Mapping[str, object], reason: str) -> bool:
|
||||
del args, reason
|
||||
order.append(f"confirm {name}")
|
||||
prompt_shown.set()
|
||||
await answered.wait()
|
||||
return True
|
||||
|
||||
def danger(name: str, args: Mapping[str, object]) -> str | None:
|
||||
del args
|
||||
return "overwrite" if name == "write_a" else None
|
||||
|
||||
registry = ToolRegistry([write_a, write_b], danger=danger, on_confirm=on_confirm)
|
||||
first = asyncio.create_task(registry.execute("write_a", '{"path": "a.txt"}'))
|
||||
await prompt_shown.wait()
|
||||
second = asyncio.create_task(registry.execute("write_b", '{"path": "a.txt"}'))
|
||||
await asyncio.sleep(0) # the second write waits for the file, not for the human
|
||||
assert order == ["confirm write_a"]
|
||||
answered.set()
|
||||
assert await asyncio.gather(first, second) == ["a", "b"]
|
||||
assert order == ["confirm write_a", "a", "b"]
|
||||
|
||||
@@ -1,11 +1,18 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import importlib
|
||||
from pathlib import Path
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from plyngent.agent import ToolRegistry
|
||||
from plyngent.tools.danger import classify_danger
|
||||
from plyngent.tools.file import edit_replace, listdir, read_file, write_file
|
||||
from tests.test_tools.helpers import call_sync
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Mapping
|
||||
|
||||
get_truncated_module = importlib.import_module("plyngent.tools.file.get_truncated")
|
||||
|
||||
|
||||
@@ -37,6 +44,37 @@ def test_write_file_refuses_existing_file_without_overwrite(workspace: Path) ->
|
||||
assert (workspace / "keep.txt").read_text(encoding="utf-8") == "second"
|
||||
|
||||
|
||||
async def test_same_file_writes_in_one_batch_keep_call_order(workspace: Path) -> None:
|
||||
"""The second call writes onto the first one's text, not onto the stale text."""
|
||||
target = workspace / "notes.txt"
|
||||
_ = target.write_text("one", encoding="utf-8")
|
||||
confirmed = asyncio.Event()
|
||||
|
||||
async def on_confirm(name: str, args: Mapping[str, object], reason: str) -> bool:
|
||||
del name, args, reason
|
||||
await confirmed.wait()
|
||||
return True
|
||||
|
||||
registry = ToolRegistry([write_file, edit_replace], danger=classify_danger, on_confirm=on_confirm)
|
||||
rewrite = asyncio.create_task(
|
||||
registry.execute("write_file", '{"path": "notes.txt", "content": "two", "overwrite": true}')
|
||||
)
|
||||
await asyncio.sleep(0) # the rewrite is waiting on its overwrite confirm
|
||||
edit = asyncio.create_task(
|
||||
registry.execute(
|
||||
"edit_replace",
|
||||
'{"path": "notes.txt", "old_string": "two", "new_string": "three"}',
|
||||
)
|
||||
)
|
||||
await asyncio.sleep(0) # the edit waits for the file instead of editing stale text
|
||||
assert target.read_text(encoding="utf-8") == "one"
|
||||
confirmed.set()
|
||||
rewrite_out, edit_out = await asyncio.gather(rewrite, edit)
|
||||
assert "wrote" in rewrite_out
|
||||
assert "replaced" in edit_out
|
||||
assert target.read_text(encoding="utf-8") == "three"
|
||||
|
||||
|
||||
def test_read_file_max_chars_embeds_token(workspace: Path) -> None:
|
||||
(workspace / "big.txt").write_text("a" * 1000, encoding="utf-8")
|
||||
out = call_sync(read_file, "big.txt", max_chars=200)
|
||||
|
||||
Reference in New Issue
Block a user