Remove stale TODOs and fix chat test stubs for ChatClient protocol

_StubChatClient/_NoChatClient predated the ChatClient protocol (which
extends ServiceClient), so isinstance checks in ServiceAPI.chat_client
always failed. Also switches tests off the outdated ServiceAPI-name
patching in favor of ServiceAPI.set(), matching test_api.py.

room_cal/_periodic.py

@@ -37,7 +37,7 @@ def _register_all_periodics() -> None:
   import room_cal.workflows.prices        # noqa: F401
   import room_cal.workflows.closed        # noqa: F401
   import room_cal.workflows.local         # noqa: F401
-  import room_cal.workflows.remote        # noqa: F401  # TODO: uncomment register_periodic calls in remote.py when ready
+  import room_cal.workflows.remote        # noqa: F401
 
 
 async def run_due(api: ServiceAPI) -> None:

room_cal/workflows/ical_feeds.py

@@ -8,8 +8,6 @@ from room_cal.workflows import _bg, _push
 
 router = APIRouter(prefix='/workflow')
 
-# TODO: push_closed range may not cover all affected dates for multi-day feed events
-
 
 async def sync_ical_feeds(
   start_date: date,

tests/test_chat.py

@@ -7,8 +7,6 @@ from room_cal.api import create_app
 from room_cal.service_api import ServiceAPI
 from room_cal.service_api.types import ServiceAuthError
 
-pytestmark = pytest.mark.skip(reason="chat tests broken: _StubChatClient predates ChatClient protocol check; revisit when chat work resumes")
-
 
 # ── stubs ──────────────────────────────────────────────────────────────────────
 
@@ -18,6 +16,7 @@ class _Msg:
   created_at_ms: str
   body: str | None
   sender_account_id: str
+  sender_name: str | None = None
 
 @dataclass
 class _Thread:
@@ -25,12 +24,28 @@ class _Thread:
   messages: list
 
 class _StubChatClient:
-  supports_chat = True
+  """Satisfies ChatClient (which extends ServiceClient) so isinstance checks in
+  ServiceAPI.chat_client pass — the calendar/pricing methods are unused by these tests."""
 
   def __init__(self, thread=None, error=None):
     self._thread = thread
     self._error = error
 
+  async def get_calendar(self, date_from, date_to):
+    return {}
+
+  async def get_reservations(self, date_from, date_to):
+    return []
+
+  async def set_price(self, d, price):
+    pass
+
+  async def set_availability(self, d, available):
+    pass
+
+  async def probe(self):
+    pass
+
   async def get_thread(self, chat_id):
     if self._error:
       raise self._error
@@ -41,23 +56,37 @@ class _StubChatClient:
       raise self._error
 
 class _NoChatClient:
-  supports_chat = False
+  """Implements only ServiceClient, not ChatClient — no get_thread/send_message."""
+
+  async def get_calendar(self, date_from, date_to):
+    return {}
+
+  async def get_reservations(self, date_from, date_to):
+    return []
+
+  async def set_price(self, d, price):
+    pass
+
+  async def set_availability(self, d, available):
+    pass
+
+  async def probe(self):
+    pass
 
 _STUB_THREAD = _Thread(
   thread_id='t1',
   messages=[
-    _Msg(id='m1', created_at_ms='1000', body='Hello', sender_account_id='acc1'),
-    _Msg(id='m2', created_at_ms='2000', body=None, sender_account_id='acc2'),
+    _Msg(id='m1', created_at_ms='1000', body='Hello', sender_account_id='acc1', sender_name='acc1'),
+    _Msg(id='m2', created_at_ms='2000', body=None, sender_account_id='acc2', sender_name='acc2'),
   ],
 )
 
 _AUTHED = {'sub': 'u', 'email': 'u@test.com', 'name': 'User'}
 
 
-def _api_factory(**clients):
-  """Return a no-arg callable that yields a ServiceAPI with injected stub clients."""
-  instance = ServiceAPI(clients=clients)
-  return lambda: instance
+def _install_api(**clients):
+  """Install a ServiceAPI singleton with injected stub clients; conftest resets it after each test."""
+  ServiceAPI.set(ServiceAPI(clients=clients))
 
 
 # ── fixtures ───────────────────────────────────────────────────────────────────
@@ -76,43 +105,36 @@ async def client(app):
 
 @pytest.mark.asyncio
 async def test_get_thread_unknown_service(client):
-  factory = _api_factory(airbnb=_StubChatClient(thread=_STUB_THREAD))
-  with patch('room_cal.api._routes.chat.ServiceAPI', factory), \
-       patch('room_cal.api.get_user', return_value=_AUTHED):
+  _install_api(airbnb=_StubChatClient(thread=_STUB_THREAD))
+  with patch('room_cal.api.get_user', return_value=_AUTHED):
     r = await client.get('/chat/nonexistent/abc')
   assert r.status_code == 400
 
 @pytest.mark.asyncio
 async def test_get_thread_no_chat_support(client):
-  factory = _api_factory(booking=_NoChatClient())
-  with patch('room_cal.api._routes.chat.ServiceAPI', factory), \
-       patch('room_cal.api.get_user', return_value=_AUTHED):
+  _install_api(booking=_NoChatClient())
+  with patch('room_cal.api.get_user', return_value=_AUTHED):
     r = await client.get('/chat/booking/abc')
   assert r.status_code == 400
 
-@pytest.mark.skip(reason="ServiceAuthError → 503 mapping not yet implemented")
 @pytest.mark.asyncio
 async def test_get_thread_auth_error(client):
-  factory = _api_factory(airbnb=_StubChatClient(error=ServiceAuthError('stale')))
-  with patch('room_cal.api._routes.chat.ServiceAPI', factory), \
-       patch('room_cal.api.get_user', return_value=_AUTHED):
+  _install_api(airbnb=_StubChatClient(error=ServiceAuthError('stale')))
+  with patch('room_cal.api.get_user', return_value=_AUTHED):
     r = await client.get('/chat/airbnb/t1')
   assert r.status_code == 503
 
-@pytest.mark.skip(reason="RuntimeError → 502 mapping not yet implemented")
 @pytest.mark.asyncio
 async def test_get_thread_remote_error(client):
-  factory = _api_factory(airbnb=_StubChatClient(error=RuntimeError('network failure')))
-  with patch('room_cal.api._routes.chat.ServiceAPI', factory), \
-       patch('room_cal.api.get_user', return_value=_AUTHED):
+  _install_api(airbnb=_StubChatClient(error=RuntimeError('network failure')))
+  with patch('room_cal.api.get_user', return_value=_AUTHED):
     r = await client.get('/chat/airbnb/t1')
   assert r.status_code == 502
 
 @pytest.mark.asyncio
 async def test_get_thread_requires_auth(client):
-  factory = _api_factory(airbnb=_StubChatClient(thread=_STUB_THREAD))
-  with patch('room_cal.api._routes.chat.ServiceAPI', factory):
-    r = await client.get('/chat/airbnb/t1')
+  _install_api(airbnb=_StubChatClient(thread=_STUB_THREAD))
+  r = await client.get('/chat/airbnb/t1')
   assert r.status_code == 401
 
 
@@ -120,9 +142,8 @@ async def test_get_thread_requires_auth(client):
 
 @pytest.mark.asyncio
 async def test_get_thread_normalizes_response(client):
-  factory = _api_factory(airbnb=_StubChatClient(thread=_STUB_THREAD))
-  with patch('room_cal.api._routes.chat.ServiceAPI', factory), \
-       patch('room_cal.api.get_user', return_value=_AUTHED):
+  _install_api(airbnb=_StubChatClient(thread=_STUB_THREAD))
+  with patch('room_cal.api.get_user', return_value=_AUTHED):
     r = await client.get('/chat/airbnb/t1')
   assert r.status_code == 200
   data = r.json()
@@ -144,20 +165,16 @@ async def test_get_thread_includes_is_reply_needed_when_present(client):
     is_reply_needed: bool = True
 
   thread = _BookingThread(reservation_id='r1', messages=[], is_reply_needed=True)
-  factory = _api_factory(booking=_StubChatClient(thread=thread))
-  # Override supports_chat on the stub for this one test
-  factory().chat_client('booking')  # warm call to verify it resolves
-  with patch('room_cal.api._routes.chat.ServiceAPI', factory), \
-       patch('room_cal.api.get_user', return_value=_AUTHED):
+  _install_api(booking=_StubChatClient(thread=thread))
+  with patch('room_cal.api.get_user', return_value=_AUTHED):
     r = await client.get('/chat/booking/r1')
   assert r.status_code == 200
   assert r.json()['is_reply_needed'] is True
 
 @pytest.mark.asyncio
 async def test_get_thread_omits_is_reply_needed_when_absent(client):
-  factory = _api_factory(airbnb=_StubChatClient(thread=_STUB_THREAD))
-  with patch('room_cal.api._routes.chat.ServiceAPI', factory), \
-       patch('room_cal.api.get_user', return_value=_AUTHED):
+  _install_api(airbnb=_StubChatClient(thread=_STUB_THREAD))
+  with patch('room_cal.api.get_user', return_value=_AUTHED):
     r = await client.get('/chat/airbnb/t1')
   assert 'is_reply_needed' not in r.json()
 
@@ -166,41 +183,36 @@ async def test_get_thread_omits_is_reply_needed_when_absent(client):
 
 @pytest.mark.asyncio
 async def test_post_message_unknown_service(client):
-  factory = _api_factory(airbnb=_StubChatClient(thread=_STUB_THREAD))
-  with patch('room_cal.api._routes.chat.ServiceAPI', factory), \
-       patch('room_cal.api.get_user', return_value=_AUTHED):
+  _install_api(airbnb=_StubChatClient(thread=_STUB_THREAD))
+  with patch('room_cal.api.get_user', return_value=_AUTHED):
     r = await client.post('/chat/nonexistent/abc', json={'body': 'hi'})
   assert r.status_code == 400
 
 @pytest.mark.asyncio
 async def test_post_message_no_chat_support(client):
-  factory = _api_factory(booking=_NoChatClient())
-  with patch('room_cal.api._routes.chat.ServiceAPI', factory), \
-       patch('room_cal.api.get_user', return_value=_AUTHED):
+  _install_api(booking=_NoChatClient())
+  with patch('room_cal.api.get_user', return_value=_AUTHED):
     r = await client.post('/chat/booking/abc', json={'body': 'hi'})
   assert r.status_code == 400
 
 @pytest.mark.asyncio
 async def test_post_message_auth_error(client):
-  factory = _api_factory(airbnb=_StubChatClient(error=ServiceAuthError('stale')))
-  with patch('room_cal.api._routes.chat.ServiceAPI', factory), \
-       patch('room_cal.api.get_user', return_value=_AUTHED):
+  _install_api(airbnb=_StubChatClient(error=ServiceAuthError('stale')))
+  with patch('room_cal.api.get_user', return_value=_AUTHED):
     r = await client.post('/chat/airbnb/t1', json={'body': 'hi'})
   assert r.status_code == 503
 
 @pytest.mark.asyncio
 async def test_post_message_remote_error(client):
-  factory = _api_factory(airbnb=_StubChatClient(error=RuntimeError('send failed')))
-  with patch('room_cal.api._routes.chat.ServiceAPI', factory), \
-       patch('room_cal.api.get_user', return_value=_AUTHED):
+  _install_api(airbnb=_StubChatClient(error=RuntimeError('send failed')))
+  with patch('room_cal.api.get_user', return_value=_AUTHED):
     r = await client.post('/chat/airbnb/t1', json={'body': 'hi'})
   assert r.status_code == 502
 
 @pytest.mark.asyncio
 async def test_post_message_requires_auth(client):
-  factory = _api_factory(airbnb=_StubChatClient())
-  with patch('room_cal.api._routes.chat.ServiceAPI', factory):
-    r = await client.post('/chat/airbnb/t1', json={'body': 'hi'})
+  _install_api(airbnb=_StubChatClient())
+  r = await client.post('/chat/airbnb/t1', json={'body': 'hi'})
   assert r.status_code == 401
 
 
@@ -208,9 +220,8 @@ async def test_post_message_requires_auth(client):
 
 @pytest.mark.asyncio
 async def test_post_message_success(client):
-  factory = _api_factory(airbnb=_StubChatClient())
-  with patch('room_cal.api._routes.chat.ServiceAPI', factory), \
-       patch('room_cal.api.get_user', return_value=_AUTHED):
+  _install_api(airbnb=_StubChatClient())
+  with patch('room_cal.api.get_user', return_value=_AUTHED):
     r = await client.post('/chat/airbnb/t1', json={'body': 'Hello guest'})
   assert r.status_code == 204
 
@@ -223,6 +234,7 @@ async def test_get_thread_live():
   """Fetch a real thread using chat_id from a recent CalDAV reservation."""
   from datetime import date, timedelta
   from room_cal.reservations import Reservation
+  from room_cal.service_api._protocols import ChatClient
   from room_cal._utils import safe_json_loads
 
   res_cal = Reservation()
@@ -237,7 +249,7 @@ async def test_get_thread_live():
     if not service or not chat_id:
       continue
     probe = ServiceAPI()
-    if service in probe._clients and getattr(probe._clients[service], 'supports_chat', False):
+    if service in probe._clients and isinstance(probe._clients[service], ChatClient):
       target_service = service
       target_chat_id = chat_id
       break