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>
This commit is contained in:
claude[bot]
2025-12-31 04:33:07 +00:00
parent 4ce21984a4
commit cdc28eeefa
3 changed files with 26 additions and 11 deletions
+4 -4
View File
@@ -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
+10 -3
View File
@@ -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,
+12 -4
View File
@@ -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}")