mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-29 06:31:32 +00:00
fix(acp): avoid duplicate edit approval diffs
This commit is contained in:
parent
9592e595a2
commit
49b28d1646
3 changed files with 29 additions and 44 deletions
|
|
@ -895,7 +895,7 @@ def _build_tool_complete_content(
|
|||
if len(display_result) > 5000:
|
||||
display_result = display_result[:4900] + f"\n... ({len(result)} chars total, truncated)"
|
||||
|
||||
if tool_name in {"write_file", "patch", "skill_manage"}:
|
||||
if tool_name == "skill_manage":
|
||||
try:
|
||||
from agent.display import extract_edit_diff
|
||||
|
||||
|
|
@ -936,22 +936,15 @@ def build_tool_start(
|
|||
|
||||
if tool_name == "patch":
|
||||
mode = arguments.get("mode", "replace")
|
||||
if mode == "replace":
|
||||
path = arguments.get("path", "")
|
||||
old = arguments.get("old_string", "")
|
||||
new = arguments.get("new_string", "")
|
||||
content = [acp.tool_diff_content(path=path, new_text=new, old_text=old)]
|
||||
else:
|
||||
patch_text = arguments.get("patch", "")
|
||||
content = _build_patch_mode_content(patch_text)
|
||||
path = arguments.get("path") or "patch input"
|
||||
content = [_text(f"Preparing {mode} edit for {path}. Approval prompt shows the diff.")]
|
||||
return acp.start_tool_call(
|
||||
tool_call_id, title, kind=kind, content=content, locations=locations,
|
||||
)
|
||||
|
||||
if tool_name == "write_file":
|
||||
path = arguments.get("path", "")
|
||||
file_content = arguments.get("content", "")
|
||||
content = [acp.tool_diff_content(path=path, new_text=file_content)]
|
||||
content = [_text(f"Preparing write to {path}. Approval prompt shows the diff." if path else "Preparing file write. Approval prompt shows the diff.")]
|
||||
return acp.start_tool_call(
|
||||
tool_call_id, title, kind=kind, content=content, locations=locations,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -183,7 +183,7 @@ class TestMcpRegistrationE2E:
|
|||
assert "hello" in complete_event.content[0].content.text
|
||||
assert complete_event.raw_output is None
|
||||
|
||||
def test_patch_mode_tool_start_emits_diff_blocks_for_v4a_patch(self):
|
||||
def test_patch_mode_tool_start_defers_diff_to_edit_approval_prompt(self):
|
||||
update = build_tool_start(
|
||||
"tc-1",
|
||||
"patch",
|
||||
|
|
@ -193,14 +193,9 @@ class TestMcpRegistrationE2E:
|
|||
},
|
||||
)
|
||||
|
||||
assert len(update.content) == 2
|
||||
assert update.content[0].type == "diff"
|
||||
assert update.content[0].path == "src/app.py"
|
||||
assert update.content[0].old_text == "old line"
|
||||
assert update.content[0].new_text == "new line"
|
||||
assert update.content[1].type == "diff"
|
||||
assert update.content[1].path == "src/new.py"
|
||||
assert update.content[1].new_text == "hello"
|
||||
assert len(update.content) == 1
|
||||
assert update.content[0].type == "content"
|
||||
assert "Approval prompt shows the diff" in update.content[0].content.text
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_prompt_tool_results_paired_by_call_id(self, acp_agent, mock_manager):
|
||||
|
|
|
|||
|
|
@ -147,7 +147,7 @@ class TestBuildToolTitle:
|
|||
|
||||
class TestBuildToolStart:
|
||||
def test_build_tool_start_for_patch(self):
|
||||
"""patch should produce a FileEditToolCallContent (diff)."""
|
||||
"""patch start should not duplicate the edit-approval diff."""
|
||||
args = {
|
||||
"path": "src/main.py",
|
||||
"old_string": "print('hello')",
|
||||
|
|
@ -156,24 +156,23 @@ class TestBuildToolStart:
|
|||
result = build_tool_start("tc-1", "patch", args)
|
||||
assert isinstance(result, ToolCallStart)
|
||||
assert result.kind == "edit"
|
||||
# The first content item should be a diff
|
||||
assert len(result.content) >= 1
|
||||
diff_item = result.content[0]
|
||||
assert isinstance(diff_item, FileEditToolCallContent)
|
||||
assert diff_item.path == "src/main.py"
|
||||
assert diff_item.new_text == "print('world')"
|
||||
assert diff_item.old_text == "print('hello')"
|
||||
item = result.content[0]
|
||||
assert isinstance(item, ContentToolCallContent)
|
||||
assert "Approval prompt shows the diff" in item.content.text
|
||||
assert "src/main.py" in item.content.text
|
||||
|
||||
def test_build_tool_start_for_write_file(self):
|
||||
"""write_file should produce a FileEditToolCallContent (diff)."""
|
||||
"""write_file start should not duplicate the edit-approval diff."""
|
||||
args = {"path": "new_file.py", "content": "print('hello')"}
|
||||
result = build_tool_start("tc-w1", "write_file", args)
|
||||
assert isinstance(result, ToolCallStart)
|
||||
assert result.kind == "edit"
|
||||
assert len(result.content) >= 1
|
||||
diff_item = result.content[0]
|
||||
assert isinstance(diff_item, FileEditToolCallContent)
|
||||
assert diff_item.path == "new_file.py"
|
||||
item = result.content[0]
|
||||
assert isinstance(item, ContentToolCallContent)
|
||||
assert "Approval prompt shows the diff" in item.content.text
|
||||
assert "new_file.py" in item.content.text
|
||||
|
||||
def test_build_tool_start_for_terminal(self):
|
||||
"""terminal should produce text content with the command."""
|
||||
|
|
@ -452,8 +451,8 @@ class TestBuildToolComplete:
|
|||
assert len(display_text) < 6000
|
||||
assert "truncated" in display_text
|
||||
|
||||
def test_build_tool_complete_for_patch_uses_diff_blocks(self):
|
||||
"""Completed patch calls should keep structured diff content for Zed."""
|
||||
def test_build_tool_complete_for_patch_summarizes_without_repeating_diff(self):
|
||||
"""Completed patch calls should not duplicate the edit-approval diff."""
|
||||
patch_result = (
|
||||
'{"success": true, "diff": "--- a/README.md\\n+++ b/README.md\\n@@ -1 +1,2 @@\\n old line\\n+new line\\n", '
|
||||
'"files_modified": ["README.md"]}'
|
||||
|
|
@ -461,18 +460,17 @@ class TestBuildToolComplete:
|
|||
result = build_tool_complete("tc-p1", "patch", patch_result)
|
||||
assert isinstance(result, ToolCallProgress)
|
||||
assert len(result.content) == 1
|
||||
diff_item = result.content[0]
|
||||
assert isinstance(diff_item, FileEditToolCallContent)
|
||||
assert diff_item.path == "README.md"
|
||||
assert diff_item.old_text == "old line"
|
||||
assert diff_item.new_text == "old line\nnew line"
|
||||
item = result.content[0]
|
||||
assert isinstance(item, ContentToolCallContent)
|
||||
assert "✅ patch completed" in item.content.text
|
||||
assert "README.md" in item.content.text
|
||||
|
||||
def test_build_tool_complete_for_patch_falls_back_to_text_when_no_diff(self):
|
||||
result = build_tool_complete("tc-p2", "patch", '{"success": true}')
|
||||
assert isinstance(result, ToolCallProgress)
|
||||
assert isinstance(result.content[0], ContentToolCallContent)
|
||||
|
||||
def test_build_tool_complete_for_write_file_uses_snapshot_diff(self, tmp_path):
|
||||
def test_build_tool_complete_for_write_file_summarizes_without_repeating_diff(self, tmp_path):
|
||||
target = tmp_path / "diff-test.txt"
|
||||
snapshot = type("Snapshot", (), {"paths": [target], "before": {str(target): None}})()
|
||||
target.write_text("hello from hermes\n", encoding="utf-8")
|
||||
|
|
@ -486,11 +484,10 @@ class TestBuildToolComplete:
|
|||
)
|
||||
assert isinstance(result, ToolCallProgress)
|
||||
assert len(result.content) == 1
|
||||
diff_item = result.content[0]
|
||||
assert isinstance(diff_item, FileEditToolCallContent)
|
||||
assert diff_item.path.endswith("diff-test.txt")
|
||||
assert diff_item.old_text is None
|
||||
assert diff_item.new_text == "hello from hermes"
|
||||
item = result.content[0]
|
||||
assert isinstance(item, ContentToolCallContent)
|
||||
assert "✅ write_file completed" in item.content.text
|
||||
assert "diff-test.txt" in item.content.text
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue