fix(python): isolate per-entry parse failures in list_models() (#1302)

python/copilot/client.py

@@ -2091,6 +2091,11 @@ async def list_models(self) -> list[ModelInfo]:
         it is called instead of querying the CLI server. The handler may be sync
         or async.
 
+        Malformed entries in the server response (for example, a single model
+        whose schema drifts in a backwards-incompatible way) are logged at
+        ``WARNING`` level and skipped, so one bad entry will not fail the
+        entire call.
+
         Returns:
             A list of ModelInfo objects with model details.
 
@@ -2123,7 +2128,21 @@ async def list_models(self) -> list[ModelInfo]:
                 # Cache miss - fetch from backend while holding lock
                 response = await self._client.request("models.list", {})
                 models_data = response.get("models", [])
-                models = [ModelInfo.from_dict(model) for model in models_data]
+                models = []
+                for model_data in models_data:
+                    try:
+                        models.append(ModelInfo.from_dict(model_data))
+                    except Exception as e:
+                        # Isolate per-entry parse failures so a single
+                        # malformed model entry (e.g. backend schema drift
+                        # on one model) does not take down list_models()
+                        # for all consumers. See issue #1302.
+                        model_id = model_data.get("id") if isinstance(model_data, dict) else None
+                        logger.warning(
+                            "Skipping malformed model entry%s in models.list response: %s",
+                            f" (id={model_id!r})" if model_id else "",
+                            e,
+                        )
 
             # Update cache before releasing lock (copy to prevent external mutation)
             self._models_cache = list(models)

python/test_client.py

@@ -491,6 +491,94 @@ def handler():
         assert models == custom_models
 
 
+class TestListModelsParserResilience:
+    """Per-entry parse failures in models.list response should not take down
+    the whole call. See https://github.com/github/copilot-sdk/issues/1302."""
+
+    def _make_client_with_fake_rpc(self, models_data):
+        """Build a CopilotClient whose RPC layer returns the given models payload.
+
+        Avoids spawning the CLI subprocess; we drive list_models() directly
+        against a stand-in for the JSON-RPC client.
+        """
+
+        class _FakeRpcClient:
+            async def request(self, method, params):
+                assert method == "models.list", f"unexpected RPC: {method}"
+                return {"models": models_data}
+
+        client = CopilotClient(SubprocessConfig(cli_path=CLI_PATH))
+        client._client = _FakeRpcClient()  # type: ignore[assignment]
+        return client
+
+    def _good_model(self, model_id: str) -> dict:
+        return {
+            "id": model_id,
+            "name": model_id,
+            "capabilities": {
+                "supports": {"vision": False, "reasoning_effort": False},
+                "limits": {"max_context_window_tokens": 128000},
+            },
+        }
+
+    @pytest.mark.asyncio
+    async def test_skips_malformed_entry_and_returns_valid_ones(self, caplog):
+        # Mix of valid + malformed entries. Malformed ones are missing
+        # required fields (id/name/capabilities) which would otherwise raise
+        # ValueError from ModelInfo.from_dict and abort the whole call.
+        models_data = [
+            self._good_model("good-1"),
+            {"id": "bad-no-name", "capabilities": self._good_model("x")["capabilities"]},
+            self._good_model("good-2"),
+            {"name": "missing-everything-else"},
+        ]
+
+        client = self._make_client_with_fake_rpc(models_data)
+
+        with caplog.at_level("WARNING", logger="copilot.client"):
+            models = await client.list_models()
+
+        ids = [m.id for m in models]
+        assert ids == ["good-1", "good-2"]
+
+        warnings = [r for r in caplog.records if r.levelname == "WARNING"]
+        assert len(warnings) == 2, (
+            f"expected one warning per malformed entry, got {len(warnings)}: "
+            f"{[r.getMessage() for r in warnings]}"
+        )
+        # The first warning should mention the id we did manage to extract.
+        assert "bad-no-name" in warnings[0].getMessage()
+
+    @pytest.mark.asyncio
+    async def test_all_malformed_returns_empty_list_without_raising(self):
+        models_data = [
+            {"id": "broken-1"},
+            {"id": "broken-2"},
+        ]
+        client = self._make_client_with_fake_rpc(models_data)
+
+        models = await client.list_models()
+        assert models == []
+
+    @pytest.mark.asyncio
+    async def test_empty_models_payload_returns_empty_list(self):
+        client = self._make_client_with_fake_rpc([])
+        models = await client.list_models()
+        assert models == []
+
+    @pytest.mark.asyncio
+    async def test_non_dict_entry_is_skipped_with_warning(self, caplog):
+        models_data = [self._good_model("ok"), "not-a-dict", 42]
+        client = self._make_client_with_fake_rpc(models_data)
+
+        with caplog.at_level("WARNING", logger="copilot.client"):
+            models = await client.list_models()
+
+        assert [m.id for m in models] == ["ok"]
+        warnings = [r for r in caplog.records if r.levelname == "WARNING"]
+        assert len(warnings) == 2
+
+
 class TestSessionConfigForwarding:
     @pytest.mark.asyncio
     async def test_create_session_forwards_client_name(self):