Skip to content

Commit 37290ca

Browse files
authored
fix: isolate raw tool payloads in session snapshots (#171)
1 parent bdb9ac7 commit 37290ca

3 files changed

Lines changed: 68 additions & 6 deletions

File tree

‎docs/contrib.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ The helpers under `acp.contrib` package recurring patterns we saw in integration
1313

1414
- `SessionAccumulator.apply(notification)` reconciles `tool_call` and `tool_call_update` payloads, even if the start event arrives late.
1515
- `snapshot()` returns an immutable `SessionSnapshot` Pydantic model containing the plan, current mode, commands, and ordered message history.
16+
- Raw tool input/output payloads are copied when applying notifications and creating snapshots, so nested payload mutations do not change the accumulated state.
1617
- `subscribe(callback)` lets you push snapshots into stores or UI components whenever state changes.
1718
- Automatic reset on session-change (toggle with `auto_reset_on_session_change`).
1819

‎src/acp/contrib/session_state.py‎

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

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

78
from pydantic import BaseModel, ConfigDict
@@ -62,8 +63,8 @@ def apply_start(self, update: ToolCallStart) -> None:
6263
self.status = update.status
6364
self.content = _copy_model_list(update.content)
6465
self.locations = _copy_model_list(update.locations)
65-
self.raw_input = update.raw_input
66-
self.raw_output = update.raw_output
66+
self.raw_input = deepcopy(update.raw_input)
67+
self.raw_output = deepcopy(update.raw_output)
6768

6869
def apply_progress(self, update: ToolCallProgress) -> None:
6970
if update.title is not None:
@@ -77,9 +78,9 @@ def apply_progress(self, update: ToolCallProgress) -> None:
7778
if update.locations is not None:
7879
self.locations = _copy_model_list(update.locations)
7980
if update.raw_input is not None:
80-
self.raw_input = update.raw_input
81+
self.raw_input = deepcopy(update.raw_input)
8182
if update.raw_output is not None:
82-
self.raw_output = update.raw_output
83+
self.raw_output = deepcopy(update.raw_output)
8384

8485
def snapshot(self) -> ToolCallView:
8586
return ToolCallView(
@@ -89,8 +90,8 @@ def snapshot(self) -> ToolCallView:
8990
status=self.status,
9091
content=tuple(item.model_copy(deep=True) for item in self.content) if self.content else None,
9192
locations=tuple(loc.model_copy(deep=True) for loc in self.locations) if self.locations else None,
92-
raw_input=self.raw_input,
93-
raw_output=self.raw_output,
93+
raw_input=deepcopy(self.raw_input),
94+
raw_output=deepcopy(self.raw_output),
9495
)
9596

9697

‎tests/contrib/test_contrib_session_state.py‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,66 @@ def notification(session_id: str, update):
2222
return SessionNotification(session_id=session_id, update=update)
2323

2424

25+
@pytest.fixture(params=["start", "progress"])
26+
def raw_tool_notification(request: pytest.FixtureRequest) -> SessionNotification:
27+
raw_input = {"options": {"paths": ["input.txt"]}}
28+
raw_output = {"result": {"lines": ["original"]}}
29+
update: ToolCallStart | ToolCallProgress
30+
if request.param == "start":
31+
update = ToolCallStart(
32+
session_update="tool_call",
33+
tool_call_id="call-1",
34+
title="Read file",
35+
raw_input=raw_input,
36+
raw_output=raw_output,
37+
)
38+
else:
39+
update = ToolCallProgress(
40+
session_update="tool_call_update",
41+
tool_call_id="call-1",
42+
raw_input=raw_input,
43+
raw_output=raw_output,
44+
)
45+
return notification("s", update)
46+
47+
48+
def test_raw_notification_mutation_does_not_change_accumulated_state(
49+
raw_tool_notification: SessionNotification,
50+
) -> None:
51+
acc = SessionAccumulator()
52+
first = acc.apply(raw_tool_notification)
53+
update = raw_tool_notification.update
54+
assert isinstance(update, ToolCallStart | ToolCallProgress)
55+
assert update.raw_input is not None
56+
assert update.raw_output is not None
57+
58+
update.raw_input["options"]["paths"].append("external.txt")
59+
update.raw_output["result"]["lines"].append("external")
60+
61+
for snapshot in (first, acc.snapshot()):
62+
assert snapshot.tool_calls["call-1"].raw_input == {"options": {"paths": ["input.txt"]}}
63+
assert snapshot.tool_calls["call-1"].raw_output == {"result": {"lines": ["original"]}}
64+
65+
66+
def test_raw_snapshot_mutation_does_not_change_other_snapshots_or_state(
67+
raw_tool_notification: SessionNotification,
68+
) -> None:
69+
acc = SessionAccumulator()
70+
first = acc.apply(raw_tool_notification)
71+
second = acc.snapshot()
72+
73+
first.tool_calls["call-1"].raw_input["options"]["paths"].append("external.txt")
74+
first.tool_calls["call-1"].raw_output["result"]["lines"].append("external")
75+
76+
for snapshot in (second, acc.snapshot()):
77+
assert snapshot.tool_calls["call-1"].raw_input == {"options": {"paths": ["input.txt"]}}
78+
assert snapshot.tool_calls["call-1"].raw_output == {"result": {"lines": ["original"]}}
79+
update = raw_tool_notification.update
80+
assert isinstance(update, ToolCallStart | ToolCallProgress)
81+
assert update.raw_input == {"options": {"paths": ["input.txt"]}}
82+
assert update.raw_output == {"result": {"lines": ["original"]}}
83+
84+
2585
def test_session_accumulator_merges_tool_calls():
2686
acc = SessionAccumulator()
2787
start = ToolCallStart(

0 commit comments

Comments
 (0)