From cdc28eeefa3a1aa17836be6cf94e0e96cac09a55 Mon Sep 17 00:00:00 2001
From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com>
Date: Wed, 31 Dec 2025 04:33:07 +0000
Subject: [PATCH] fix: resolve three correctness footguns in MCP and sync code

- Fix mutable default args in search_notes() MCP tool
  Changed types and entity_types from List[str] = [] to Optional[List[str]] = None
  to prevent shared state across calls

- Fix Pydantic defaults in WatchServiceState
  Moved datetime.now() and os.getpid() from field defaults to model_post_init()
  to prevent evaluation at import time vs instance creation time

- Fix list mutation during iteration in WatchService
  Changed 'for added_path in adds' to 'for added_path in list(adds)'
  to avoid skipping items when removing from list during iteration

Fixes #487

Co-authored-by: Paul Hernandez <phernandez@users.noreply.github.com>
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
---
 src/basic_memory/mcp/tools/search.py     |  8 ++++----
 src/basic_memory/schemas/project_info.py | 13 ++++++++++---
 src/basic_memory/sync/watch_service.py   | 16 ++++++++++++----
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/basic_memory/mcp/tools/search.py b/src/basic_memory/mcp/tools/search.py
index 01f6298e..8d897d9e 100644
--- a/src/basic_memory/mcp/tools/search.py
+++ b/src/basic_memory/mcp/tools/search.py
@@ -206,8 +206,8 @@ async def search_notes(
     page: int = 1,
     page_size: int = 10,
     search_type: str = "text",
-    types: List[str] = [],
-    entity_types: List[str] = [],
+    types: Optional[List[str]] = None,
+    entity_types: Optional[List[str]] = None,
     after_date: Optional[str] = None,
     context: Context | None = None,
 ) -> SearchResponse | str:
@@ -348,9 +348,9 @@ async def search_notes(
         search_query.text = query  # Default to text search
 
     # Add optional filters if provided (empty lists are treated as no filter)
-    if entity_types:
+    if entity_types is not None and entity_types:
         search_query.entity_types = [SearchItemType(t) for t in entity_types]
-    if types:
+    if types is not None and types:
         search_query.types = types
     if after_date:
         search_query.after_date = after_date
diff --git a/src/basic_memory/schemas/project_info.py b/src/basic_memory/schemas/project_info.py
index 70fe1ecf..a2199a5c 100644
--- a/src/basic_memory/schemas/project_info.py
+++ b/src/basic_memory/schemas/project_info.py
@@ -120,8 +120,8 @@ class WatchEvent(BaseModel):
 class WatchServiceState(BaseModel):
     # Service status
     running: bool = False
-    start_time: datetime = datetime.now()  # Use directly with Pydantic model
-    pid: int = os.getpid()  # Use directly with Pydantic model
+    start_time: Optional[datetime] = None
+    pid: Optional[int] = None
 
     # Stats
     error_count: int = 0
@@ -132,7 +132,14 @@ class WatchServiceState(BaseModel):
     synced_files: int = 0
 
     # Recent activity
-    recent_events: List[WatchEvent] = []  # Use directly with Pydantic model
+    recent_events: List[WatchEvent] = []
+
+    def model_post_init(self, __context) -> None:
+        """Initialize dynamic defaults after model creation."""
+        if self.start_time is None:
+            self.start_time = datetime.now()
+        if self.pid is None:
+            self.pid = os.getpid()
 
     def add_event(
         self,
diff --git a/src/basic_memory/sync/watch_service.py b/src/basic_memory/sync/watch_service.py
index 5cdd2e1c..b2efd780 100644
--- a/src/basic_memory/sync/watch_service.py
+++ b/src/basic_memory/sync/watch_service.py
@@ -34,8 +34,8 @@ class WatchEvent(BaseModel):
 class WatchServiceState(BaseModel):
     # Service status
     running: bool = False
-    start_time: datetime = datetime.now()  # Use directly with Pydantic model
-    pid: int = os.getpid()  # Use directly with Pydantic model
+    start_time: Optional[datetime] = None
+    pid: Optional[int] = None
 
     # Stats
     error_count: int = 0
@@ -46,7 +46,14 @@ class WatchServiceState(BaseModel):
     synced_files: int = 0
 
     # Recent activity
-    recent_events: List[WatchEvent] = []  # Use directly with Pydantic model
+    recent_events: List[WatchEvent] = []
+
+    def model_post_init(self, __context) -> None:
+        """Initialize dynamic defaults after model creation."""
+        if self.start_time is None:
+            self.start_time = datetime.now()
+        if self.pid is None:
+            self.pid = os.getpid()
 
     def add_event(
         self,
@@ -299,7 +306,8 @@ class WatchService:
         )
 
         # because of our atomic writes on updates, an add may be an existing file
-        for added_path in adds:  # pragma: no cover TODO add test
+        # Iterate over a copy to avoid mutation during iteration
+        for added_path in list(adds):  # pragma: no cover TODO add test
             entity = await sync_service.entity_repository.get_by_file_path(added_path)
             if entity is not None:
                 logger.debug(f"Existing file will be processed as modified, path={added_path}")