Skip to content

Commit fb74a7f

Browse files
committed
fix(contrib): isolate tracked raw tool payloads
1 parent 11f8a4c commit fb74a7f

3 files changed

Lines changed: 115 additions & 13 deletions

File tree

‎src/acp/contrib/tool_calls.py‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import uuid
44
from collections.abc import Callable, Sequence
5+
from copy import deepcopy
56
from typing import Any, cast
67

78
from pydantic import BaseModel, ConfigDict
@@ -86,8 +87,8 @@ def __init__(
8687
self.status = status
8788
self.content = _copy_model_list(content)
8889
self.locations = _copy_model_list(locations)
89-
self.raw_input = raw_input
90-
self.raw_output = raw_output
90+
self.raw_input = deepcopy(raw_input)
91+
self.raw_output = deepcopy(raw_output)
9192
self._stream_buffer: str | None = None
9293

9394
def to_view(self) -> TrackedToolCallView:
@@ -99,8 +100,8 @@ def to_view(self) -> TrackedToolCallView:
99100
status=self.status,
100101
content=tuple(item.model_copy(deep=True) for item in self.content) if self.content else None,
101102
locations=tuple(loc.model_copy(deep=True) for loc in self.locations) if self.locations else None,
102-
raw_input=self.raw_input,
103-
raw_output=self.raw_output,
103+
raw_input=deepcopy(self.raw_input),
104+
raw_output=deepcopy(self.raw_output),
104105
)
105106

106107
def to_tool_call_model(self) -> ToolCallUpdate:
@@ -112,8 +113,8 @@ def to_tool_call_model(self) -> ToolCallUpdate:
112113
status=self.status,
113114
content=_copy_model_list(self.content),
114115
locations=_copy_model_list(self.locations),
115-
raw_input=self.raw_input,
116-
raw_output=self.raw_output,
116+
raw_input=deepcopy(self.raw_input),
117+
raw_output=deepcopy(self.raw_output),
117118
)
118119

119120
def to_start_model(self) -> ToolCallStart:
@@ -128,8 +129,8 @@ def to_start_model(self) -> ToolCallStart:
128129
status=self.status,
129130
content=_copy_model_list(self.content),
130131
locations=_copy_model_list(self.locations),
131-
raw_input=self.raw_input,
132-
raw_output=self.raw_output,
132+
raw_input=deepcopy(self.raw_input),
133+
raw_output=deepcopy(self.raw_output),
133134
)
134135

135136
def update(
@@ -169,11 +170,11 @@ def update(
169170
)
170171
kwargs["locations"] = _copy_model_list(seq_locations)
171172
if raw_input is not UNSET:
172-
self.raw_input = raw_input
173-
kwargs["rawInput"] = raw_input
173+
self.raw_input = deepcopy(raw_input)
174+
kwargs["rawInput"] = deepcopy(self.raw_input)
174175
if raw_output is not UNSET:
175-
self.raw_output = raw_output
176-
kwargs["raw_output"] = raw_output
176+
self.raw_output = deepcopy(raw_output)
177+
kwargs["raw_output"] = deepcopy(self.raw_output)
177178
return ToolCallProgress(session_update="tool_call_update", tool_call_id=self.tool_call_id, **kwargs)
178179

179180
def append_stream_text(
@@ -189,7 +190,11 @@ def append_stream_text(
189190

190191

191192
class ToolCallTracker:
192-
"""Utility for generating ACP tool call updates on the server side."""
193+
"""Utility for generating ACP tool call updates on the server side.
194+
195+
Raw input/output payloads are copied on entry and export so caller, view,
196+
notification, and permission-request mutations cannot change tracked state.
197+
"""
193198

194199
def __init__(self, *, id_factory: Callable[[], str] | None = None) -> None:
195200
self._id_factory = id_factory or (lambda: uuid.uuid4().hex)

‎tests/contrib/test_contrib_permissions.py‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,3 +116,22 @@ def test_default_permission_options_shape():
116116
options = default_permission_options()
117117
assert len(options) == 3
118118
assert {opt.option_id for opt in options} == {"approve", "approve_for_session", "reject"}
119+
120+
121+
@pytest.mark.asyncio
122+
async def test_permission_request_raw_payloads_are_detached_from_tracker() -> None:
123+
tracker = ToolCallTracker()
124+
tracker.start("external", title="Run", raw_input={"args": ["original"]}, raw_output={"result": ["original"]})
125+
126+
async def requester(request: RequestPermissionRequest) -> RequestPermissionResponse:
127+
assert isinstance(request.tool_call.raw_input, dict)
128+
assert isinstance(request.tool_call.raw_output, dict)
129+
request.tool_call.raw_input["args"].append("request mutation")
130+
request.tool_call.raw_output["result"].append("request mutation")
131+
return RequestPermissionResponse(outcome=AllowedOutcome(option_id="approve", outcome="selected"))
132+
133+
broker = PermissionBroker("session", requester, tracker=tracker)
134+
await broker.request_for("external")
135+
136+
assert tracker.view("external").raw_input == {"args": ["original"]}
137+
assert tracker.view("external").raw_output == {"result": ["original"]}

‎tests/contrib/test_contrib_tool_calls.py‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
from __future__ import annotations
22

3+
import pytest
4+
35
from acp.contrib.tool_calls import ToolCallTracker
46
from acp.schema import ContentToolCallContent, TextContentBlock, ToolCallProgress
57

@@ -33,3 +35,79 @@ def test_tool_call_tracker_streaming_text_updates_content():
3335
assert isinstance(second_content, ContentToolCallContent)
3436
assert isinstance(second_content.content, TextContentBlock)
3537
assert second_content.content.text == "hello, world"
38+
39+
40+
@pytest.mark.parametrize("field", ["raw_input", "raw_output"])
41+
@pytest.mark.parametrize("operation", ["start", "progress"])
42+
def test_tracker_snapshots_incoming_raw_payloads(field: str, operation: str) -> None:
43+
tracker = ToolCallTracker()
44+
payload = {"nested": ["original"]}
45+
if operation == "start":
46+
notification = tracker.start(
47+
"external",
48+
title="Run",
49+
raw_input=payload if field == "raw_input" else None,
50+
raw_output=payload if field == "raw_output" else None,
51+
)
52+
else:
53+
tracker.start("external", title="Run")
54+
notification = tracker.progress(
55+
"external",
56+
raw_input=payload if field == "raw_input" else None,
57+
raw_output=payload if field == "raw_output" else None,
58+
)
59+
60+
payload["nested"].append("caller mutation")
61+
62+
assert getattr(tracker.view("external"), field) == {"nested": ["original"]}
63+
assert getattr(notification, field) == {"nested": ["original"]}
64+
65+
66+
@pytest.mark.parametrize("field", ["raw_input", "raw_output"])
67+
@pytest.mark.parametrize("output", ["start", "progress", "view", "tool_call_model"])
68+
def test_tracker_raw_payload_exports_cannot_mutate_state(field: str, output: str) -> None:
69+
tracker = ToolCallTracker()
70+
payload = {"nested": ["original"]}
71+
notification = tracker.start(
72+
"external",
73+
title="Run",
74+
raw_input=payload if field == "raw_input" else None,
75+
raw_output=payload if field == "raw_output" else None,
76+
)
77+
if output == "progress":
78+
exported = tracker.progress(
79+
"external",
80+
raw_input=payload if field == "raw_input" else None,
81+
raw_output=payload if field == "raw_output" else None,
82+
)
83+
elif output == "view":
84+
exported = tracker.view("external")
85+
elif output == "tool_call_model":
86+
exported = tracker.tool_call_model("external")
87+
else:
88+
exported = notification
89+
90+
getattr(exported, field)["nested"].append("consumer mutation")
91+
92+
assert getattr(tracker.view("external"), field) == {"nested": ["original"]}
93+
94+
95+
@pytest.mark.parametrize("payload", [None, False, 0, "text", ["item"]])
96+
def test_tracker_preserves_json_raw_payload_values(payload: object) -> None:
97+
tracker = ToolCallTracker()
98+
tracker.start("external", title="Run", raw_input=payload, raw_output=payload)
99+
100+
assert tracker.view("external").raw_input == payload
101+
assert tracker.tool_call_model("external").raw_output == payload
102+
103+
104+
def test_tracker_progress_can_clear_raw_payloads() -> None:
105+
tracker = ToolCallTracker()
106+
tracker.start("external", title="Run", raw_input={"old": True}, raw_output={"old": True})
107+
108+
progress = tracker.progress("external", raw_input=None, raw_output=None)
109+
110+
assert progress.raw_input is None
111+
assert progress.raw_output is None
112+
assert tracker.view("external").raw_input is None
113+
assert tracker.view("external").raw_output is None

0 commit comments

Comments
 (0)