fix(matrix): correct sync result-object comment and route it through the classifier

The comment above the result-object branch in _sync_loop claimed mautrix's
Client.sync() returns an object carrying a message string for auth failures.
That is wrong. In the pinned mautrix 0.21.0, HTTPAPI._send raises
make_request_error() for any non-2xx and otherwise returns parsed JSON, so a
real M_FORBIDDEN arrives as an exception and is handled by the except branch.
The claim was introduced by this PR, which rewrote an accurate comment about
the earlier matrix-nio client (whose SyncError result objects were genuine).

The branch itself is kept as defense in depth against a future client swap,
but it now classifies with the same errcode/http_status logic as the
exception path instead of a lone "unknown_token" substring test, which
silently missed M_MISSING_TOKEN and M_FORBIDDEN and resynced forever
against a credential that can never succeed.

A structured errcode/http_status is authoritative; the message text is only
consulted when the object exposes neither, since str(object) is an opaque
repr. The text scan deliberately cannot override a structured verdict, so a
transient 502 whose HTML body contains "Forbidden" is still retried.

Adds four tests. Three are discriminating RED/GREEN cases that fail against
the old substring branch (M_MISSING_TOKEN errcode, http_status=401 with no
keyword in the message, and an unstructured object whose only signal is
.message). The fourth pins the precedence rule and passes either way.

Verified: 136 passed / 1 failed in tests/gateway/test_matrix.py; the single
failure (test_password_login_uses_device_id) fails identically at the
pristine PR head and is unrelated.

(cherry picked from commit bc9e6a8dafcf349a4e6b20a261fb2449603c0239)

plugins/platforms/matrix/adapter.py

@@ -1837,32 +1837,19 @@ class MatrixAdapter(BasePlatformAdapter):
         next_batch = await client.sync_store.get_next_batch()  # resume from the initial sync
         while not self._closing:
             try:
-<<<<<<< HEAD
-                # 45s outer cap guards TCP-level hangs the 30s long-poll timeout can't catch.
+                # 45s outer cap guards TCP-level hangs the 30s long-poll timeout cannot catch.
                 sync_data = await asyncio.wait_for(client.sync(since=next_batch, timeout=30000), timeout=45.0)
-                # Auth failures (M_UNKNOWN_TOKEN) arrive as SyncError objects, not exceptions.
-=======
-                # Wrap in asyncio.wait_for to guard against TCP-level hangs
-                # that the Matrix long-poll timeout cannot catch. Long-poll
-                # is 30s, so 45s gives 15s slack for network drain.
-                sync_data = await asyncio.wait_for(
-                    client.sync(
-                        since=next_batch,
-                        timeout=30000,
-                    ),
-                    timeout=45.0,
-                )
 
-                # mautrix's Client.sync() returns a plain dict on success but
-                # an object carrying a "message" string (not a raised
-                # exception) for auth failures like M_UNKNOWN_TOKEN. Detect
-                # and stop immediately rather than falling through to the
-                # dict-shaped handling below.
->>>>>>> 4e16313582 (fix(matrix): use a genuinely discriminating fixture for the sync-loop test)
+                # Route result objects through the same classifier as raised errors.
                 _sync_msg = getattr(sync_data, "message", None)
-                if isinstance(_sync_msg, str) and "unknown_token" in _sync_msg.lower():
-                    logger.error("Matrix: permanent auth error from sync: %s — stopping", _sync_msg)
-                    return
+                if isinstance(_sync_msg, str):
+                    structured = isinstance(getattr(sync_data, "errcode", None), str) or isinstance(getattr(sync_data, "http_status", None), int)
+                    permanent = _is_permanent_matrix_auth_error(sync_data) if structured else _is_permanent_matrix_auth_error(_sync_msg)
+                    if permanent:
+                        logger.error("Matrix: permanent auth error from sync: %s, stopping", _sync_msg)
+                        return
+
+
                 if isinstance(sync_data, dict):
                     next_batch = await self._absorb_sync(client, sync_data) or next_batch
                     await asyncio.sleep(0)  # let fresh invite joins start before the next sync

tests/gateway/test_matrix.py

@@ -1437,6 +1437,112 @@ class TestMatrixSyncLoop:
         assert fake_client.sync.await_count == 1
         assert 5 not in [call.args[0] for call in mock_sleep.await_args_list]
 
+    # ------------------------------------------------------------------
+    # Result-object (non-exception) auth failures.
+    #
+    # The pinned mautrix 0.21.0 never returns an error object from sync():
+    # HTTPAPI._send raises make_request_error() for any non-2xx and otherwise
+    # returns parsed JSON, so a real auth failure arrives as an exception and
+    # is covered by the two tests above. The result-object branch in
+    # _sync_loop is inherited from the earlier matrix-nio client (whose
+    # SyncError objects were genuine) and is kept as defense in depth. These
+    # tests pin it to the same classifier the exception path uses so a future
+    # client swap cannot silently reintroduce the infinite-resync bug.
+    # ------------------------------------------------------------------
+
+    @staticmethod
+    def _result_obj(**attrs):
+        """Build an error *result object*, not a MagicMock.
+
+        A MagicMock would auto-create ``errcode``/``http_status`` attributes
+        and make the structured-vs-text branch untestable, so these fixtures
+        expose exactly the attributes a real client object would.
+        """
+        return type("SyncErrorResult", (), attrs)()
+
+    async def _run_loop_with_sync_result(self, result_obj):
+        """Drive _sync_loop with sync() returning result_obj, then a clean dict."""
+        adapter = _make_adapter()
+        adapter._closing = False
+
+        calls = {"n": 0}
+
+        async def _sync_side_effect(**kwargs):
+            calls["n"] += 1
+            if calls["n"] == 1:
+                return result_obj
+            # If the loop treated the object as transient it comes back here;
+            # stop cleanly so the test can assert the retry happened.
+            adapter._closing = True
+            return {"next_batch": "s1"}
+
+        mock_sync_store = MagicMock()
+        mock_sync_store.get_next_batch = AsyncMock(return_value=None)
+        mock_sync_store.put_next_batch = AsyncMock()
+
+        fake_client = MagicMock()
+        fake_client.sync = AsyncMock(side_effect=_sync_side_effect)
+        fake_client.sync_store = mock_sync_store
+        adapter._client = fake_client
+
+        with patch("asyncio.sleep", new=AsyncMock()):
+            await adapter._sync_loop()
+
+        return fake_client.sync.await_count
+
+    @pytest.mark.asyncio
+    async def test_sync_loop_stops_on_result_object_with_permanent_errcode(self):
+        """An M_MISSING_TOKEN result object must stop the loop.
+
+        This is the discriminating RED/GREEN case for routing the branch
+        through _is_permanent_matrix_auth_error. The old code tested only
+        ``"m_unknown_token" in msg or "unknown_token" in msg``, and this
+        message ("Missing access token") contains neither, so the old branch
+        fell through and resynced forever against a credential that can
+        never succeed. Reading the structured errcode catches it.
+        """
+        obj = self._result_obj(
+            message="Missing access token", errcode="M_MISSING_TOKEN"
+        )
+        assert await self._run_loop_with_sync_result(obj) == 1
+
+    @pytest.mark.asyncio
+    async def test_sync_loop_stops_on_result_object_with_401_http_status(self):
+        """A result object exposing only http_status=401 must stop the loop.
+
+        Also discriminating: the message text carries no auth keyword at all,
+        so only the structured status read reaches the right verdict.
+        """
+        obj = self._result_obj(message="Sync request failed", http_status=401)
+        assert await self._run_loop_with_sync_result(obj) == 1
+
+    @pytest.mark.asyncio
+    async def test_sync_loop_stops_on_unstructured_result_object_via_message(self):
+        """With no errcode and no http_status, the message text is the only signal.
+
+        str(obj) on a result object is an opaque repr like
+        ``<SyncErrorResult object at 0x...>``, so the classifier must be
+        handed ``.message`` explicitly or this auth failure is missed.
+        """
+        obj = self._result_obj(message="M_FORBIDDEN: access token rejected")
+        assert await self._run_loop_with_sync_result(obj) == 1
+
+    @pytest.mark.asyncio
+    async def test_sync_loop_retries_transient_result_object_despite_auth_keyword(self):
+        """A structured 502 must be retried even though its body says "Forbidden".
+
+        This pins the precedence rule. A homeserver behind a reverse proxy can
+        return a 502 HTML error page containing the word "Forbidden"; if the
+        message-text scan were allowed to override the structured status, a
+        passing outage would permanently kill the sync loop. That is the same
+        false-positive class the classifier rework exists to prevent.
+        """
+        obj = self._result_obj(
+            message="<html><body><h1>502 Bad Gateway</h1>Forbidden</body></html>",
+            http_status=502,
+        )
+        assert await self._run_loop_with_sync_result(obj) == 2
+
     @pytest.mark.asyncio
     async def test_connect_receives_dm_from_initial_sync_dispatch(self):
         """A DM delivered by initial sync should reach the message handler after connect."""