fix(serde): make idempotent rebind transactional on adapter-lookup failure

Codex P2 (both ThreadImpl and ChannelImpl): `from_json(existing_inst,
chat=Y)` was invalidating caches BEFORE calling `chat.get_adapter(name)`.
If the lookup failed and raised `RuntimeError`, the caller catching the
exception was left with a partially-mutated instance — `_state_adapter_
instance`, `_channel_cache`, `_message_history`, `_recent_messages`,
`_is_subscribed_context` all silently wiped, but `_adapter` unchanged.
Subsequent operations on the thread would re-resolve state/channel via
the singleton instead of the previously-bound chat.

Fix: pre-flight the adapter lookup BEFORE invalidating caches. If the
name doesn't resolve in the new chat, raise immediately with all
original state intact. The binding block below repeats the lookup
when it actually applies the result; that's a cheap duplicate in
exchange for transactional semantics.

Same fix in both `ThreadImpl.from_json` and `ChannelImpl.from_json`.

2 regression tests, each captures a snapshot of every cache-bearing
attribute before the rebind, calls `from_json(instance, chat=bad_chat)`
expecting RuntimeError, then asserts every snapshotted attribute is
byte-identical on the catch side:

- test_should_leave_thread_unchanged_when_chat_rebind_lookup_fails
- test_should_leave_channel_unchanged_when_chat_rebind_lookup_fails

3468 tests pass.

https://claude.ai/code/session_01XE1bMoQ5BjCvgi1iLGKKhG

Author: claude

src/chat_sdk/channel.py

@@ -423,11 +423,26 @@ def from_json(
         """
         if isinstance(data, ChannelImpl):
             channel = data
-            # Invalidate caches derived from the previous binding so the
-            # rebind block below resolves them fresh against the new
-            # adapter/chat. Both `_state_adapter_instance` (old chat's
-            # state backend) and `_message_history` (old chat's cache)
-            # would route to the previous context otherwise.
+            # Transactional rebind: if chat= is passed without adapter=,
+            # we'll need to resolve the adapter from the new chat by name.
+            # Pre-flight that lookup and raise (leaving the channel
+            # unchanged) BEFORE invalidating caches, so a caller that
+            # catches the RuntimeError isn't left with a partially-reset
+            # instance. The binding block below repeats the lookup when
+            # it actually applies the result (cheap).
+            if chat is not None and adapter is None:
+                _lookup_name = channel._adapter_name or (
+                    channel._adapter.name if channel._adapter is not None else None
+                )
+                if _lookup_name and chat.get_adapter(_lookup_name) is None:
+                    raise RuntimeError(
+                        f'Adapter "{_lookup_name}" not found in the provided Chat instance'
+                    )
+
+            # Validation passed — safe to invalidate.
+            # Both `_state_adapter_instance` (old chat's state backend)
+            # and `_message_history` (old chat's cache) would route to
+            # the previous context otherwise.
             if adapter is not None or chat is not None:
                 channel._state_adapter_instance = None
                 channel._message_history = None

src/chat_sdk/thread.py

@@ -843,6 +843,24 @@ def from_json(
             # operations to the previous context — the classic split-
             # routing bug.
             #
+            # Transactional rebind: if the caller passes only `chat=`, we
+            # need to resolve the adapter from the new chat by name. If that
+            # lookup fails, we must raise BEFORE mutating any cache, so a
+            # caller that catches the RuntimeError is left with an
+            # unchanged thread. Pre-flight the lookup here; the binding
+            # block below repeats it (cheap dict access) when it actually
+            # applies the result.
+            if chat is not None and adapter is None:
+                _lookup_name = thread._adapter_name or (
+                    thread._adapter.name if thread._adapter is not None else None
+                )
+                if _lookup_name and chat.get_adapter(_lookup_name) is None:
+                    raise RuntimeError(
+                        f'Adapter "{_lookup_name}" not found in the provided Chat instance'
+                    )
+
+            # Validation passed — safe to invalidate caches.
+            #
             # Every attribute that embeds a reference to the old context
             # gets cleared here: `_state_adapter_instance` (old chat's
             # state backend), `_channel_cache` (old adapter's channel),

tests/test_channel_faithful.py

@@ -722,6 +722,48 @@ def test_should_invalidate_state_cache_on_idempotent_rebind(self):
         assert rebound._state_adapter_instance is None
         assert rebound.adapter.name == "teams"
 
+    def test_should_leave_channel_unchanged_when_chat_rebind_lookup_fails(self):
+        """Transactional rebind: `from_json(existing_channel, chat=Y)` must
+        raise BEFORE mutating any cache when the adapter lookup against
+        the new chat fails. Callers catching the exception get their
+        original channel back unchanged. Regression for a Codex P2."""
+        from chat_sdk.chat import Chat, ChatConfig
+        from chat_sdk.testing import create_mock_adapter as _create
+        from chat_sdk.testing import create_mock_state as _create_state
+
+        slack_a = _create("slack")
+        state_a = _create_state()
+        chat_b = Chat(
+            ChatConfig(
+                user_name="bot-b",
+                adapters={"teams": _create("teams")},  # no 'slack'
+                state=_create_state(),
+            )
+        )
+
+        original = ChannelImpl(
+            _ChannelImplConfigWithAdapter(
+                id="C123",
+                adapter=slack_a,
+                state_adapter=state_a,
+            )
+        )
+        snapshot = {
+            "_adapter": original._adapter,
+            "_adapter_name": original._adapter_name,
+            "_state_adapter_instance": original._state_adapter_instance,
+            "_message_history": original._message_history,
+        }
+
+        with pytest.raises(RuntimeError, match='Adapter "slack" not found'):
+            ChannelImpl.from_json(original, chat=chat_b)
+
+        for attr, before in snapshot.items():
+            after = getattr(original, attr)
+            assert after == before, f"{attr}: expected {before!r}, got {after!r}"
+        assert original._adapter is snapshot["_adapter"]
+        assert original._state_adapter_instance is snapshot["_state_adapter_instance"]
+
     def test_should_rebind_adapter_on_idempotent_chat_rebind_for_direct_constructed_channel(self):
         """When `from_json(existing_channel, chat=Y)` rebinds a channel that
         was constructed directly (so `_adapter_name` is None), the new

tests/test_serialization.py

@@ -665,6 +665,61 @@ def test_should_reset_is_subscribed_context_on_rebind(self, mock_state):
         rebound = ThreadImpl.from_json(original, adapter=second_adapter)
         assert rebound._is_subscribed_context is False
 
+    def test_should_leave_thread_unchanged_when_chat_rebind_lookup_fails(self, mock_state):
+        """Transactional rebind: `from_json(existing_thread, chat=Y)` must
+        either fully apply the rebind or leave the thread untouched. If
+        `chat.get_adapter(name)` returns None, the RuntimeError must fire
+        BEFORE any cache invalidation — callers that catch the exception
+        should be able to keep using the thread with its original
+        bindings intact. Regression for a Codex P2."""
+        from chat_sdk.chat import Chat, ChatConfig
+        from chat_sdk.testing import create_mock_adapter, create_mock_state, create_test_message
+
+        slack_a = create_mock_adapter("slack")
+        state_a = create_mock_state()
+        # Chat B has a different adapter name, so looking up "slack" will fail.
+        chat_b = Chat(
+            ChatConfig(
+                user_name="bot-b",
+                adapters={"teams": create_mock_adapter("teams")},
+                state=create_mock_state(),
+            )
+        )
+
+        original = ThreadImpl(
+            _ThreadImplConfig(
+                id="slack:C123:1234.5678",
+                adapter=slack_a,
+                state_adapter=state_a,
+                channel_id="C123",
+                initial_message=create_test_message("m1", "hello"),
+                is_subscribed_context=True,
+            )
+        )
+        # Capture every state-bearing attribute before the rebind.
+        snapshot = {
+            "_adapter": original._adapter,
+            "_adapter_name": original._adapter_name,
+            "_state_adapter_instance": original._state_adapter_instance,
+            "_channel_cache": original._channel_cache,
+            "_message_history": original._message_history,
+            "_recent_messages": list(original._recent_messages),
+            "_is_subscribed_context": original._is_subscribed_context,
+        }
+
+        with pytest.raises(RuntimeError, match='Adapter "slack" not found'):
+            ThreadImpl.from_json(original, chat=chat_b)
+
+        # Every cached attribute must be exactly as it was before. A
+        # non-transactional implementation would have nulled some of these
+        # before the raise, making recovery unsafe.
+        for attr, before in snapshot.items():
+            after = getattr(original, attr)
+            assert after == before, f"{attr}: expected {before!r}, got {after!r}"
+        # Extra identity check for objects where equality might be loose.
+        assert original._adapter is snapshot["_adapter"]
+        assert original._state_adapter_instance is snapshot["_state_adapter_instance"]
+
     def test_should_invalidate_recent_messages_and_message_history_on_rebind(self, mock_state):
         """Two additional caches that carry previous-binding references must
         also drop on idempotent rebind: `_recent_messages` (populated from