Merge pull request #746 from microsoft/security-code-quality-fix

refactor: clean up code across multiple components

Author: Roopan-Microsoft

infra/scripts/index_datasets.py

@@ -118,7 +118,7 @@ def extract_docx_text(docx_bytes):
 
     print("Creating or updating Azure Search index...")
     search_index_client = SearchIndexClient(endpoint=ai_search_endpoint, credential=credential)
-    index_result = search_index_client.create_or_update_index(index=index)
+    search_index_client.create_or_update_index(index=index)
     print(f"Index '{ai_search_index_name}' created or updated successfully.")
 except Exception as e:
     print(f"Error creating/updating index: {e}")
@@ -163,7 +163,9 @@ def extract_docx_text(docx_bytes):
     print("Uploading documents to the index...")
     search_client = SearchClient(endpoint=ai_search_endpoint, index_name=ai_search_index_name, credential=credential)
     result = search_client.upload_documents(documents=data_list)
-    print(f"Uploaded {len(data_list)} documents.")
+    successes = sum(1 for r in result if getattr(r, "succeeded", False))
+    failures = len(data_list) - successes
+    print(f"Uploaded documents. Requested: {len(data_list)}, Succeeded: {successes}, Failed: {failures}")
 except Exception as e:
     print(f"Error uploading documents: {e}")
     sys.exit(1)

package-lock.json

@@ -87,8 +87,16 @@ def mock_dependencies(monkeypatch):
 def test_input_task_invalid_json():
     """Test the case where the input JSON is invalid."""
     headers = {"Authorization": "Bearer mock-token"}
+    invalid_json = "{invalid: json"  # deliberately malformed JSON
     response = client.post("/input_task", data=invalid_json, headers=headers)
 
+    # Assert that the API responds with a client error for invalid JSON
+    assert response.status_code == 400
+    # Optionally, check that an error message is present in the response body
+    # Adjust these assertions to match the actual error schema if needed
+    body = response.json()
+    assert "error" in body or "detail" in body
+
 
 def test_process_request_endpoint_success():
     """Test the /api/process_request endpoint with valid input."""

src/backend/tests/test_app.py

@@ -78,7 +78,11 @@ async def start_comms(
                 message = await websocket.receive_text()
                 logging.debug(f"Received WebSocket message from {user_id}: {message}")
             except asyncio.TimeoutError:
-                pass
+                # Ignore timeouts to keep the WebSocket connection open, but avoid a tight loop.
+                logging.debug(
+                    f"WebSocket receive timeout for user {user_id}, process {process_id}"
+                )
+                await asyncio.sleep(0.1)
             except WebSocketDisconnect:
                 track_event_if_configured(
                     "WebSocketDisconnect",

src/backend/v4/api/router.py

@@ -481,8 +481,6 @@ async def validate_team_search_indexes(
                     error_msg = "Team configuration references search indexes but no Azure Search endpoint is configured"
                     self.logger.warning(error_msg)
                     return False, [error_msg]
-                else:
-                    return True, []
 
             if not index_names:
                 self.logger.info(

src/backend/v4/common/services/team_service.py

@@ -87,8 +87,14 @@ async def open(self) -> "MCPEnabledBase":
         # Register agent (best effort)
         try:
             agent_registry.register_agent(self)
-        except Exception:
-            pass
+        except Exception as exc:
+            # Best-effort registration; log and continue without failing open()
+            self.logger.warning(
+                "Failed to register agent %s in agent_registry: %s",
+                type(self).__name__,
+                exc,
+                exc_info=True,
+            )
 
         return self
 
@@ -100,13 +106,25 @@ async def close(self) -> None:
             if self._agent and hasattr(self._agent, "close"):
                 try:
                     await self._agent.close()  # AzureAIAgentClient has async close
-                except Exception:
-                    pass
+                except Exception as exc:
+                    # Best-effort close; log failure but continue teardown
+                    self.logger.warning(
+                        "Error while closing underlying agent %s: %s",
+                        type(self._agent).__name__ if self._agent else "Unknown",
+                        exc,
+                        exc_info=True,
+                    )
             # Unregister from registry if present
             try:
                 agent_registry.unregister_agent(self)
-            except Exception:
-                pass
+            except Exception as exc:
+                # Best-effort unregister; log and continue teardown
+                self.logger.warning(
+                    "Failed to unregister agent %s from agent_registry: %s",
+                    type(self).__name__,
+                    exc,
+                    exc_info=True,
+                )
             await self._stack.aclose()
         finally:
             self._stack = None
@@ -407,26 +425,26 @@ async def close(self) -> None:
             if self._agent and hasattr(self._agent, "close"):
                 try:
                     await self._agent.close()
-                except Exception:
-                    pass
+                except Exception as exc:
+                    logging.warning("Failed to close underlying agent %r: %s", self._agent, exc, exc_info=True)
 
             # Unregister from registry
             try:
                 agent_registry.unregister_agent(self)
-            except Exception:
-                pass
+            except Exception as exc:
+                logging.warning("Failed to unregister agent %r from registry: %s", self, exc, exc_info=True)
 
             # Close credential and project client
             if self.client:
                 try:
                     await self.client.close()
-                except Exception:
-                    pass
+                except Exception as exc:
+                    logging.warning("Failed to close Azure AgentsClient %r: %s", self.client, exc, exc_info=True)
             if self.creds:
                 try:
                     await self.creds.close()
-                except Exception:
-                    pass
+                except Exception as exc:
+                    logging.warning("Failed to close credentials %r: %s", self.creds, exc, exc_info=True)
 
         finally:
             await super().close()

src/backend/v4/magentic_agents/common/lifecycle.py

@@ -33,13 +33,6 @@ def __init__(self, team_service: Optional[TeamService] = None):
         self._agent_list: List = []
         self.team_service = team_service
 
-    # @staticmethod
-    # def parse_team_config(file_path: Union[str, Path]) -> SimpleNamespace:
-    #     """Parse JSON file into objects using SimpleNamespace."""
-    #     with open(file_path, 'r') as f:
-    #         data = json.load(f)
-    #     return json.loads(json.dumps(data), object_hook=lambda d: SimpleNamespace(**d))
-
     # Ensure only an explicit boolean True in the source sets this flag.
     def extract_use_reasoning(self, agent_obj):
         # Support both dict and attribute-style objects

src/backend/v4/magentic_agents/magentic_agent_factory.py

@@ -36,24 +36,6 @@ def from_env(cls) -> "MCPConfig":
         )
 
 
-# @dataclass(slots=True)
-# class BingConfig:
-#     """Configuration for connecting to Bing Search."""
-#     connection_name: str = "Bing"
-
-#     @classmethod
-#     def from_env(cls) -> "BingConfig":
-#         connection_name = config.BING_CONNECTION_NAME
-
-#         # Raise exception if required environment variable is missing
-#         if not connection_name:
-#             raise ValueError(f"{cls.__name__} Missing required environment variables")
-
-#         return cls(
-#             connection_name=connection_name,
-#         )
-
-
 @dataclass(slots=True)
 class SearchConfig:
     """Configuration for connecting to Azure AI Search."""
@@ -65,7 +47,6 @@ class SearchConfig:
     @classmethod
     def from_env(cls, index_name: str) -> "SearchConfig":
         connection_name = config.AZURE_AI_SEARCH_CONNECTION_NAME
-        index_name = index_name
         endpoint = config.AZURE_AI_SEARCH_ENDPOINT
 
         # Raise exception if any required environment variable is missing
@@ -76,6 +57,6 @@ def from_env(cls, index_name: str) -> "SearchConfig":
 
         return cls(
             connection_name=connection_name,
-            index_name=index_name,
-            endpoint=endpoint
+            endpoint=endpoint,
+            index_name=index_name
         )

src/backend/v4/magentic_agents/models/agent_models.py

@@ -1,11 +1,10 @@
-import html
 import os
 
 import uvicorn
 from dotenv import load_dotenv
 from fastapi import FastAPI
 from fastapi.middleware.cors import CORSMiddleware
-from fastapi.responses import FileResponse, HTMLResponse
+from fastapi.responses import FileResponse
 from fastapi.staticfiles import StaticFiles
 
 # Load environment variables from .env file