Merge pull request #3810 from rudycelekli/fix/file-observations-by-conversation-20261021

fix(files): keep file-read observations within their conversation
This commit is contained in:
zhayujie authored and GitHub committed 2026-10-25 10:43:23 +08:00
commit 26be412aa4
2 files changed
+128 -6

No files matched your search

+18 -6
View File
@@ -2,7 +2,9 @@
Cross-tool record of when the agent last read each file.
The tool manager builds a fresh tool instance per call, so this state has to
live at module level to be shared between read, edit and write.
live at module level to be shared between read, edit and write. Observations
are scoped to the current conversation: another conversation reading the same
file must not replace the snapshot this agent actually saw.
It powers a staleness warning: if a file changed on disk after the agent read
it, the agent is probably about to overwrite someone else's change (the user
@@ -17,12 +19,12 @@ import threading
from collections import OrderedDict
from typing import Optional
# Plenty for one conversation; bounded so a long-running process can't grow
# Bound all conversations together so a long-running process cannot grow
# without limit.
_MAX_TRACKED = 512
_lock = threading.Lock()
_read_mtimes = OrderedDict() # realpath -> mtime at the time we read it
_read_mtimes = OrderedDict() # conversation + realpath -> observed mtime
def _key(path: str) -> Optional[str]:
@@ -32,6 +34,15 @@ def _key(path: str) -> Optional[str]:
return None
def _observation_key(path: str) -> tuple:
from common.runtime_identity import current_identity
identity = current_identity()
# Keep observations across turns of the same conversation. A fresh run id
# must not discard a file view still present in that conversation's context.
return identity.agent_id, identity.user_id, identity.session_id, path
def _current_mtime(path: str) -> Optional[float]:
try:
return os.path.getmtime(path)
@@ -47,9 +58,10 @@ def note_read(path: str) -> None:
mtime = _current_mtime(key)
if mtime is None:
return
observation = _observation_key(key)
with _lock:
_read_mtimes[key] = mtime
_read_mtimes.move_to_end(key)
_read_mtimes[observation] = mtime
_read_mtimes.move_to_end(observation)
while len(_read_mtimes) > _MAX_TRACKED:
_read_mtimes.popitem(last=False)
@@ -70,7 +82,7 @@ def staleness_warning(path: str) -> Optional[str]:
if not key:
return None
with _lock:
seen = _read_mtimes.get(key)
seen = _read_mtimes.get(_observation_key(key))
if seen is None:
return None
current = _current_mtime(key)
+110
View File
@@ -0,0 +1,110 @@
"""One conversation's fresh read must not replace another's stale file view."""
import os
import pytest
from agent.tools.edit.edit import Edit
from agent.tools.read.read import Read
from agent.tools.utils import file_state
from agent.tools.write.write import Write
from common.runtime_identity import identity_scope
@pytest.fixture(autouse=True)
def isolated_observations():
file_state.reset()
yield
file_state.reset()
def observed_file(tmp_path):
path = tmp_path / "report.txt"
path.write_text("first line\nold second line\n", encoding="utf-8")
os.utime(path, (1700000000, 1700000000))
with identity_scope(agent_id="writer", session_id="chat-A"):
assert (
Read({"cwd": str(tmp_path)}).execute({"path": str(path)}).status
== "success"
)
path.write_text("first line\nupdated outside\n", encoding="utf-8")
os.utime(path, (1700000002, 1700000002))
return path
@pytest.mark.parametrize(
"other",
[
{"agent_id": "writer", "session_id": "chat-B"},
{"agent_id": "teammate", "session_id": "chat-A"},
],
)
@pytest.mark.parametrize("tool", ["edit", "write"])
@pytest.mark.parametrize("other_action", ["read", "write"])
def test_other_observation_does_not_hide_original_conversations_warning(
tmp_path, other, tool, other_action
):
path = observed_file(tmp_path)
with identity_scope(**other):
if other_action == "read":
result = Read({"cwd": str(tmp_path)}).execute({"path": str(path)})
assert "updated outside" in result.result["content"]
else:
result = Write({"cwd": str(tmp_path)}).execute(
{"path": str(path), "content": "first line\nwritten by other actor\n"}
)
assert result.status == "success"
with identity_scope(agent_id="writer", session_id="chat-A"):
if tool == "edit":
result = Edit({"cwd": str(tmp_path)}).execute(
{
"path": str(path),
"oldText": "first line",
"newText": "revised first line",
}
)
else:
result = Write({"cwd": str(tmp_path)}).execute(
{"path": str(path), "content": "revised first line\nold second line\n"}
)
assert result.status == "success", result.result
assert "modified after you last read it" in result.result.get("warning", "")
assert file_state.staleness_warning(str(path)) is None
def test_unread_conversation_has_no_borrowed_expectation(tmp_path):
path = observed_file(tmp_path)
with identity_scope(agent_id="writer", session_id="chat-B"):
result = Write({"cwd": str(tmp_path)}).execute(
{"path": str(path), "content": "new document\n"}
)
assert result.status == "success"
assert "warning" not in result.result
def test_same_conversation_keeps_observation_across_turns(tmp_path):
path = tmp_path / "report.txt"
path.write_text("initial\n", encoding="utf-8")
os.utime(path, (1700000000, 1700000000))
with identity_scope(agent_id="writer", session_id="chat-A", run_id="first-turn"):
Read({"cwd": str(tmp_path)}).execute({"path": str(path)})
path.write_text("outside edit\n", encoding="utf-8")
os.utime(path, (1700000002, 1700000002))
with identity_scope(agent_id="writer", session_id="chat-A", run_id="next-turn"):
assert "modified after you last read it" in file_state.staleness_warning(
str(path)
)
def test_same_conversation_reread_refreshes_its_own_view(tmp_path):
path = observed_file(tmp_path)
with identity_scope(agent_id="writer", session_id="chat-A"):
assert (
Read({"cwd": str(tmp_path)}).execute({"path": str(path)}).status
== "success"
)
result = Write({"cwd": str(tmp_path)}).execute(
{"path": str(path), "content": "new content based on fresh read\n"}
)
assert result.status == "success"
assert "warning" not in result.result