fix(tools): scope file state by session
Made-with: Cursor
This commit is contained in:
+39
-32
@@ -28,7 +28,7 @@ from nanobot.agent.tools.ask import (
|
|||||||
pending_ask_user_id,
|
pending_ask_user_id,
|
||||||
)
|
)
|
||||||
from nanobot.agent.tools.cron import CronTool
|
from nanobot.agent.tools.cron import CronTool
|
||||||
from nanobot.agent.tools.file_state import FileStates
|
from nanobot.agent.tools.file_state import FileStates, bind_file_states, reset_file_states
|
||||||
from nanobot.agent.tools.filesystem import EditFileTool, ListDirTool, ReadFileTool, WriteFileTool
|
from nanobot.agent.tools.filesystem import EditFileTool, ListDirTool, ReadFileTool, WriteFileTool
|
||||||
from nanobot.agent.tools.message import MessageTool
|
from nanobot.agent.tools.message import MessageTool
|
||||||
from nanobot.agent.tools.notebook import NotebookEditTool
|
from nanobot.agent.tools.notebook import NotebookEditTool
|
||||||
@@ -248,10 +248,9 @@ class AgentLoop:
|
|||||||
self.context = ContextBuilder(workspace, timezone=timezone, disabled_skills=disabled_skills)
|
self.context = ContextBuilder(workspace, timezone=timezone, disabled_skills=disabled_skills)
|
||||||
self.sessions = session_manager or SessionManager(workspace)
|
self.sessions = session_manager or SessionManager(workspace)
|
||||||
self.tools = ToolRegistry()
|
self.tools = ToolRegistry()
|
||||||
# Per-session file-read/write tracker (issue #3571) — shared across
|
# One file-read/write tracker per logical session. The tool registry is
|
||||||
# the filesystem tools registered below so this AgentLoop does not
|
# shared by this loop, so tools resolve the active state via contextvars.
|
||||||
# leak read-dedup state into another loop's tools.
|
self._file_states_by_session: dict[str, FileStates] = {}
|
||||||
self._file_states = FileStates()
|
|
||||||
self.runner = AgentRunner(provider)
|
self.runner = AgentRunner(provider)
|
||||||
self.subagents = SubagentManager(
|
self.subagents = SubagentManager(
|
||||||
provider=provider,
|
provider=provider,
|
||||||
@@ -313,6 +312,14 @@ class AgentLoop:
|
|||||||
self.commands = CommandRouter()
|
self.commands = CommandRouter()
|
||||||
register_builtin_commands(self.commands)
|
register_builtin_commands(self.commands)
|
||||||
|
|
||||||
|
def _file_states_for_session(self, session_key: str | None) -> FileStates:
|
||||||
|
key = session_key or "__default__"
|
||||||
|
states = self._file_states_by_session.get(key)
|
||||||
|
if states is None:
|
||||||
|
states = FileStates()
|
||||||
|
self._file_states_by_session[key] = states
|
||||||
|
return states
|
||||||
|
|
||||||
def _sync_subagent_runtime_limits(self) -> None:
|
def _sync_subagent_runtime_limits(self) -> None:
|
||||||
"""Keep subagent runtime limits aligned with mutable loop settings."""
|
"""Keep subagent runtime limits aligned with mutable loop settings."""
|
||||||
self.subagents.max_iterations = self.max_iterations
|
self.subagents.max_iterations = self.max_iterations
|
||||||
@@ -353,24 +360,19 @@ class AgentLoop:
|
|||||||
self.workspace if (self.restrict_to_workspace or self.exec_config.sandbox) else None
|
self.workspace if (self.restrict_to_workspace or self.exec_config.sandbox) else None
|
||||||
)
|
)
|
||||||
extra_read = [BUILTIN_SKILLS_DIR] if allowed_dir else None
|
extra_read = [BUILTIN_SKILLS_DIR] if allowed_dir else None
|
||||||
# One FileStates per session so read-dedup and read-before-edit
|
|
||||||
# tracking does not leak across sessions sharing this process
|
|
||||||
# (issue #3571).
|
|
||||||
file_states = self._file_states
|
|
||||||
self.tools.register(AskUserTool())
|
self.tools.register(AskUserTool())
|
||||||
self.tools.register(
|
self.tools.register(
|
||||||
ReadFileTool(
|
ReadFileTool(
|
||||||
workspace=self.workspace,
|
workspace=self.workspace,
|
||||||
allowed_dir=allowed_dir,
|
allowed_dir=allowed_dir,
|
||||||
extra_allowed_dirs=extra_read,
|
extra_allowed_dirs=extra_read,
|
||||||
file_states=file_states,
|
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
for cls in (WriteFileTool, EditFileTool, ListDirTool):
|
for cls in (WriteFileTool, EditFileTool, ListDirTool):
|
||||||
self.tools.register(cls(workspace=self.workspace, allowed_dir=allowed_dir, file_states=file_states))
|
self.tools.register(cls(workspace=self.workspace, allowed_dir=allowed_dir))
|
||||||
for cls in (GlobTool, GrepTool):
|
for cls in (GlobTool, GrepTool):
|
||||||
self.tools.register(cls(workspace=self.workspace, allowed_dir=allowed_dir, file_states=file_states))
|
self.tools.register(cls(workspace=self.workspace, allowed_dir=allowed_dir))
|
||||||
self.tools.register(NotebookEditTool(workspace=self.workspace, allowed_dir=allowed_dir, file_states=file_states))
|
self.tools.register(NotebookEditTool(workspace=self.workspace, allowed_dir=allowed_dir))
|
||||||
if self.exec_config.enable:
|
if self.exec_config.enable:
|
||||||
self.tools.register(
|
self.tools.register(
|
||||||
ExecTool(
|
ExecTool(
|
||||||
@@ -630,25 +632,30 @@ class AgentLoop:
|
|||||||
|
|
||||||
return items
|
return items
|
||||||
|
|
||||||
result = await self.runner.run(AgentRunSpec(
|
active_session_key = session.key if session else session_key
|
||||||
initial_messages=initial_messages,
|
file_state_token = bind_file_states(self._file_states_for_session(active_session_key))
|
||||||
tools=self.tools,
|
try:
|
||||||
model=self.model,
|
result = await self.runner.run(AgentRunSpec(
|
||||||
max_iterations=self.max_iterations,
|
initial_messages=initial_messages,
|
||||||
max_tool_result_chars=self.max_tool_result_chars,
|
tools=self.tools,
|
||||||
hook=hook,
|
model=self.model,
|
||||||
error_message="Sorry, I encountered an error calling the AI model.",
|
max_iterations=self.max_iterations,
|
||||||
concurrent_tools=True,
|
max_tool_result_chars=self.max_tool_result_chars,
|
||||||
workspace=self.workspace,
|
hook=hook,
|
||||||
session_key=session.key if session else None,
|
error_message="Sorry, I encountered an error calling the AI model.",
|
||||||
context_window_tokens=self.context_window_tokens,
|
concurrent_tools=True,
|
||||||
context_block_limit=self.context_block_limit,
|
workspace=self.workspace,
|
||||||
provider_retry_mode=self.provider_retry_mode,
|
session_key=session.key if session else None,
|
||||||
progress_callback=on_progress,
|
context_window_tokens=self.context_window_tokens,
|
||||||
retry_wait_callback=on_retry_wait,
|
context_block_limit=self.context_block_limit,
|
||||||
checkpoint_callback=_checkpoint,
|
provider_retry_mode=self.provider_retry_mode,
|
||||||
injection_callback=_drain_pending,
|
progress_callback=on_progress,
|
||||||
))
|
retry_wait_callback=on_retry_wait,
|
||||||
|
checkpoint_callback=_checkpoint,
|
||||||
|
injection_callback=_drain_pending,
|
||||||
|
))
|
||||||
|
finally:
|
||||||
|
reset_file_states(file_state_token)
|
||||||
self._last_usage = result.usage
|
self._last_usage = result.usage
|
||||||
if result.stop_reason == "max_iterations":
|
if result.stop_reason == "max_iterations":
|
||||||
logger.warning("Max iterations ({}) reached", self.max_iterations)
|
logger.warning("Max iterations ({}) reached", self.max_iterations)
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import hashlib
|
import hashlib
|
||||||
import os
|
import os
|
||||||
|
from contextvars import ContextVar, Token
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
@@ -129,6 +130,26 @@ class FileStates:
|
|||||||
self._state.clear()
|
self._state.clear()
|
||||||
|
|
||||||
|
|
||||||
|
_current_file_states: ContextVar[FileStates | None] = ContextVar(
|
||||||
|
"nanobot_file_states",
|
||||||
|
default=None,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def current_file_states(default: FileStates) -> FileStates:
|
||||||
|
"""Return the FileStates bound to the current agent task, or a fallback."""
|
||||||
|
return _current_file_states.get() or default
|
||||||
|
|
||||||
|
|
||||||
|
def bind_file_states(file_states: FileStates) -> Token[FileStates | None]:
|
||||||
|
"""Bind file read/write state for the current async task."""
|
||||||
|
return _current_file_states.set(file_states)
|
||||||
|
|
||||||
|
|
||||||
|
def reset_file_states(token: Token[FileStates | None]) -> None:
|
||||||
|
_current_file_states.reset(token)
|
||||||
|
|
||||||
|
|
||||||
# Module-level default instance, retained for backward compatibility with
|
# Module-level default instance, retained for backward compatibility with
|
||||||
# tests and callers that reach in directly. Per-session callers should hold
|
# tests and callers that reach in directly. Per-session callers should hold
|
||||||
# their own FileStates instance instead of touching this one.
|
# their own FileStates instance instead of touching this one.
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ from typing import Any
|
|||||||
|
|
||||||
from nanobot.agent.tools.base import Tool, tool_parameters
|
from nanobot.agent.tools.base import Tool, tool_parameters
|
||||||
from nanobot.agent.tools.schema import BooleanSchema, IntegerSchema, StringSchema, tool_parameters_schema
|
from nanobot.agent.tools.schema import BooleanSchema, IntegerSchema, StringSchema, tool_parameters_schema
|
||||||
from nanobot.agent.tools.file_state import FileStates, _hash_file
|
from nanobot.agent.tools.file_state import FileStates, _hash_file, current_file_states
|
||||||
from nanobot.utils.helpers import build_image_content_blocks, detect_image_mime
|
from nanobot.utils.helpers import build_image_content_blocks, detect_image_mime
|
||||||
from nanobot.config.paths import get_media_dir
|
from nanobot.config.paths import get_media_dir
|
||||||
|
|
||||||
@@ -54,10 +54,17 @@ class _FsTool(Tool):
|
|||||||
self._workspace = workspace
|
self._workspace = workspace
|
||||||
self._allowed_dir = allowed_dir
|
self._allowed_dir = allowed_dir
|
||||||
self._extra_allowed_dirs = extra_allowed_dirs
|
self._extra_allowed_dirs = extra_allowed_dirs
|
||||||
# Read-dedup / read-before-edit state is scoped to one FileStates
|
# Explicit state is used by isolated runners like Dream/subagents.
|
||||||
# so it does not leak across sessions sharing this process
|
# Main AgentLoop tools leave this unset and resolve state from the
|
||||||
# (issue #3571). Bare constructions get a private instance.
|
# current async task, which keeps shared tool instances session-safe.
|
||||||
self._file_states: FileStates = file_states if file_states is not None else FileStates()
|
self._explicit_file_states = file_states
|
||||||
|
self._fallback_file_states = FileStates()
|
||||||
|
|
||||||
|
@property
|
||||||
|
def _file_states(self) -> FileStates:
|
||||||
|
if self._explicit_file_states is not None:
|
||||||
|
return self._explicit_file_states
|
||||||
|
return current_file_states(self._fallback_file_states)
|
||||||
|
|
||||||
def _resolve(self, path: str) -> Path:
|
def _resolve(self, path: str) -> Path:
|
||||||
return _resolve_path(path, self._workspace, self._allowed_dir, self._extra_allowed_dirs)
|
return _resolve_path(path, self._workspace, self._allowed_dir, self._extra_allowed_dirs)
|
||||||
|
|||||||
@@ -127,6 +127,36 @@ class TestReadDedupSessionIsolation:
|
|||||||
)
|
)
|
||||||
assert "line 0" in second
|
assert "line 0" in second
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_shared_loop_tool_uses_bound_session_state(self, tmp_path):
|
||||||
|
f = tmp_path / "shared.txt"
|
||||||
|
f.write_text("\n".join(f"line {i}" for i in range(10)), encoding="utf-8")
|
||||||
|
|
||||||
|
# AgentLoop registers one shared ReadFileTool instance. The session
|
||||||
|
# boundary is the task-local FileStates binding, not the tool object.
|
||||||
|
shared_tool = ReadFileTool(workspace=tmp_path)
|
||||||
|
session_a = file_state.FileStates()
|
||||||
|
session_b = file_state.FileStates()
|
||||||
|
|
||||||
|
token = file_state.bind_file_states(session_a)
|
||||||
|
try:
|
||||||
|
first = await shared_tool.execute(path=str(f))
|
||||||
|
repeat = await shared_tool.execute(path=str(f))
|
||||||
|
finally:
|
||||||
|
file_state.reset_file_states(token)
|
||||||
|
|
||||||
|
assert "line 0" in first
|
||||||
|
assert "unchanged" in repeat.lower()
|
||||||
|
|
||||||
|
token = file_state.bind_file_states(session_b)
|
||||||
|
try:
|
||||||
|
second_session_read = await shared_tool.execute(path=str(f))
|
||||||
|
finally:
|
||||||
|
file_state.reset_file_states(token)
|
||||||
|
|
||||||
|
assert "unchanged" not in second_session_read.lower()
|
||||||
|
assert "line 0" in second_session_read
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# PDF support
|
# PDF support
|
||||||
|
|||||||
Reference in New Issue
Block a user