Skip to content

Commit cb2efad

Browse files
fix: reviver() uses explicit chat= instead of global singleton
reviver() was calling register_singleton() and using bare from_json() calls, reintroducing ambient global state even though the 3-level resolver exists. Now uses chat= parameter for explicit resolution without side effects. Also adds 18 tests for the resolver: - Global singleton register/resolve/clear - ContextVar activation scoping and nesting - Explicit chat= beats context and global - Concurrent tasks don't bleed across ContextVars - reviver() doesn't register singleton, binds to specific chat Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 7dbfc29 commit cb2efad

2 files changed

Lines changed: 258 additions & 4 deletions

File tree

src/chat_sdk/chat.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -759,17 +759,19 @@ def get_adapter_by_name(self, name: str) -> Adapter | None:
759759
def reviver(self) -> Callable[[str, Any], Any]:
760760
"""Return a JSON reviver that deserializes Thread/Channel/Message objects.
761761
762-
Ensures this Chat instance is the registered singleton.
762+
Uses explicit ``chat=self`` for deserialization instead of mutating
763+
process-global state. Each Chat instance produces a reviver bound
764+
to itself.
763765
"""
764-
self.register_singleton()
766+
chat = self
765767

766768
def _reviver(key: str, value: Any) -> Any:
767769
if isinstance(value, dict) and "_type" in value:
768770
t = value["_type"]
769771
if t == "chat:Thread":
770-
return ThreadImpl.from_json(value)
772+
return ThreadImpl.from_json(value, chat=chat)
771773
if t == "chat:Channel":
772-
return ChannelImpl.from_json(value)
774+
return ChannelImpl.from_json(value, chat=chat)
773775
if t == "chat:Message":
774776
return _message_from_json(value)
775777
return value

tests/test_chat_resolver.py

Lines changed: 252 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
1+
"""Tests for the 3-level Chat resolver (ContextVar → global → error).
2+
3+
Verifies:
4+
- chat.activate() scopes resolution to the current context
5+
- Explicit chat= parameter beats context and global
6+
- Multiple concurrent chats don't bleed across tasks
7+
- reviver() uses explicit chat= instead of global singleton
8+
- clear_chat_singleton() resets both levels
9+
"""
10+
11+
from __future__ import annotations
12+
13+
import asyncio
14+
15+
import pytest
16+
17+
from chat_sdk import Chat, MemoryStateAdapter
18+
from chat_sdk.testing import create_mock_adapter
19+
from chat_sdk.thread import (
20+
ThreadImpl,
21+
clear_chat_singleton,
22+
get_chat_singleton,
23+
has_chat_singleton,
24+
)
25+
26+
27+
def _make_chat(name: str = "slack") -> Chat:
28+
adapter = create_mock_adapter(name)
29+
state = MemoryStateAdapter()
30+
return Chat(adapters={name: adapter}, state=state, user_name=f"{name}-bot")
31+
32+
33+
def _thread_json(adapter_name: str = "slack") -> dict:
34+
return {
35+
"_type": "chat:Thread",
36+
"id": "t1",
37+
"channelId": f"{adapter_name}:C1",
38+
"channelVisibility": "public",
39+
"isDM": False,
40+
"adapterName": adapter_name,
41+
}
42+
43+
44+
class TestGlobalSingleton:
45+
"""Existing register_singleton pattern still works."""
46+
47+
def setup_method(self):
48+
clear_chat_singleton()
49+
50+
def teardown_method(self):
51+
clear_chat_singleton()
52+
53+
def test_register_and_resolve(self):
54+
chat = _make_chat()
55+
chat.register_singleton()
56+
assert get_chat_singleton() is chat
57+
58+
def test_has_singleton_false_before_register(self):
59+
assert not has_chat_singleton()
60+
61+
def test_has_singleton_true_after_register(self):
62+
chat = _make_chat()
63+
chat.register_singleton()
64+
assert has_chat_singleton()
65+
66+
def test_clear_resets(self):
67+
chat = _make_chat()
68+
chat.register_singleton()
69+
clear_chat_singleton()
70+
assert not has_chat_singleton()
71+
72+
def test_error_when_no_singleton(self):
73+
with pytest.raises(RuntimeError, match="No Chat instance available"):
74+
get_chat_singleton()
75+
76+
77+
class TestContextVarActivation:
78+
"""chat.activate() scopes resolution to the current context."""
79+
80+
def setup_method(self):
81+
clear_chat_singleton()
82+
83+
def teardown_method(self):
84+
clear_chat_singleton()
85+
86+
def test_activate_scopes_to_context(self):
87+
chat = _make_chat()
88+
with chat.activate():
89+
assert get_chat_singleton() is chat
90+
# Outside context, no singleton
91+
assert not has_chat_singleton()
92+
93+
def test_activate_overrides_global(self):
94+
global_chat = _make_chat("global")
95+
local_chat = _make_chat("local")
96+
global_chat.register_singleton()
97+
98+
assert get_chat_singleton() is global_chat
99+
100+
with local_chat.activate():
101+
assert get_chat_singleton() is local_chat
102+
103+
# After exit, global is restored
104+
assert get_chat_singleton() is global_chat
105+
106+
def test_nested_activate(self):
107+
chat_a = _make_chat("a")
108+
chat_b = _make_chat("b")
109+
110+
with chat_a.activate():
111+
assert get_chat_singleton() is chat_a
112+
with chat_b.activate():
113+
assert get_chat_singleton() is chat_b
114+
# Back to chat_a after inner exit
115+
assert get_chat_singleton() is chat_a
116+
117+
def test_from_json_resolves_via_activate(self):
118+
chat = _make_chat("slack")
119+
data = _thread_json("slack")
120+
121+
with chat.activate():
122+
thread = ThreadImpl.from_json(data)
123+
assert thread.adapter is not None
124+
assert thread.adapter.name == "slack"
125+
126+
127+
class TestExplicitChatParameter:
128+
"""Explicit chat= parameter on from_json beats all fallbacks."""
129+
130+
def setup_method(self):
131+
clear_chat_singleton()
132+
133+
def teardown_method(self):
134+
clear_chat_singleton()
135+
136+
def test_explicit_chat_beats_global(self):
137+
global_chat = _make_chat("global")
138+
explicit_chat = _make_chat("explicit")
139+
global_chat.register_singleton()
140+
141+
data = _thread_json("explicit")
142+
thread = ThreadImpl.from_json(data, chat=explicit_chat)
143+
assert thread.adapter.name == "explicit"
144+
145+
def test_explicit_chat_beats_contextvar(self):
146+
context_chat = _make_chat("context")
147+
explicit_chat = _make_chat("explicit")
148+
149+
data = _thread_json("explicit")
150+
with context_chat.activate():
151+
thread = ThreadImpl.from_json(data, chat=explicit_chat)
152+
assert thread.adapter.name == "explicit"
153+
154+
def test_explicit_chat_works_without_any_singleton(self):
155+
chat = _make_chat("solo")
156+
data = _thread_json("solo")
157+
thread = ThreadImpl.from_json(data, chat=chat)
158+
assert thread.adapter.name == "solo"
159+
160+
161+
class TestConcurrentIsolation:
162+
"""Multiple concurrent chats don't bleed across tasks."""
163+
164+
def setup_method(self):
165+
clear_chat_singleton()
166+
167+
def teardown_method(self):
168+
clear_chat_singleton()
169+
170+
async def test_concurrent_tasks_isolated(self):
171+
chat_a = _make_chat("adapter_a")
172+
chat_b = _make_chat("adapter_b")
173+
results: dict[str, str] = {}
174+
175+
async def task(chat: Chat, name: str) -> None:
176+
with chat.activate():
177+
# Yield to let the other task run
178+
await asyncio.sleep(0.01)
179+
resolved = get_chat_singleton()
180+
results[name] = resolved._user_name
181+
182+
await asyncio.gather(
183+
task(chat_a, "a"),
184+
task(chat_b, "b"),
185+
)
186+
187+
assert results["a"] == "adapter_a-bot"
188+
assert results["b"] == "adapter_b-bot"
189+
190+
async def test_concurrent_from_json_isolated(self):
191+
chat_a = _make_chat("aa")
192+
chat_b = _make_chat("bb")
193+
results: dict[str, str] = {}
194+
195+
async def task(chat: Chat, adapter_name: str) -> None:
196+
data = _thread_json(adapter_name)
197+
with chat.activate():
198+
await asyncio.sleep(0.01)
199+
thread = ThreadImpl.from_json(data)
200+
results[adapter_name] = thread.adapter.name
201+
202+
await asyncio.gather(
203+
task(chat_a, "aa"),
204+
task(chat_b, "bb"),
205+
)
206+
207+
assert results["aa"] == "aa"
208+
assert results["bb"] == "bb"
209+
210+
211+
class TestReviver:
212+
"""reviver() uses explicit chat= instead of global singleton."""
213+
214+
def setup_method(self):
215+
clear_chat_singleton()
216+
217+
def teardown_method(self):
218+
clear_chat_singleton()
219+
220+
def test_reviver_does_not_register_singleton(self):
221+
chat = _make_chat()
222+
_reviver = chat.reviver()
223+
# reviver() should NOT register a global singleton
224+
assert not has_chat_singleton()
225+
226+
def test_reviver_deserializes_thread(self):
227+
chat = _make_chat("slack")
228+
reviver = chat.reviver()
229+
data = _thread_json("slack")
230+
thread = reviver("key", data)
231+
assert isinstance(thread, ThreadImpl)
232+
assert thread.adapter.name == "slack"
233+
234+
def test_reviver_bound_to_specific_chat(self):
235+
chat_a = _make_chat("a")
236+
chat_b = _make_chat("b")
237+
238+
reviver_a = chat_a.reviver()
239+
reviver_b = chat_b.reviver()
240+
241+
thread_a = reviver_a("k", _thread_json("a"))
242+
thread_b = reviver_b("k", _thread_json("b"))
243+
244+
assert thread_a.adapter.name == "a"
245+
assert thread_b.adapter.name == "b"
246+
247+
def test_reviver_passes_through_non_typed_values(self):
248+
chat = _make_chat()
249+
reviver = chat.reviver()
250+
assert reviver("key", "plain string") == "plain string"
251+
assert reviver("key", 42) == 42
252+
assert reviver("key", {"no_type": True}) == {"no_type": True}

0 commit comments

Comments
 (0)