mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
fix: implement project-specific sync status checks for MCP tools (#183)
Signed-off-by: phernandez <paul@basicmachines.co> Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -82,10 +82,15 @@ async def build_context(
|
||||
logger.info(f"Building context from {url}")
|
||||
# URL is already validated and normalized by MemoryUrl type annotation
|
||||
|
||||
# Get the active project first to check project-specific sync status
|
||||
active_project = get_active_project(project)
|
||||
|
||||
# Check migration status and wait briefly if needed
|
||||
from basic_memory.mcp.tools.utils import wait_for_migration_or_return_status
|
||||
|
||||
migration_status = await wait_for_migration_or_return_status(timeout=5.0)
|
||||
migration_status = await wait_for_migration_or_return_status(
|
||||
timeout=5.0, project_name=active_project.name
|
||||
)
|
||||
if migration_status: # pragma: no cover
|
||||
# Return a proper GraphContext with status message
|
||||
from basic_memory.schemas.memory import MemoryMetadata
|
||||
@@ -102,8 +107,6 @@ async def build_context(
|
||||
uri=migration_status, # Include status in metadata
|
||||
),
|
||||
)
|
||||
|
||||
active_project = get_active_project(project)
|
||||
project_url = active_project.project_url
|
||||
|
||||
response = await call_get(
|
||||
|
||||
@@ -52,14 +52,17 @@ async def read_note(
|
||||
read_note("Meeting Notes", project="work-project")
|
||||
"""
|
||||
|
||||
# Get the active project first to check project-specific sync status
|
||||
active_project = get_active_project(project)
|
||||
|
||||
# Check migration status and wait briefly if needed
|
||||
from basic_memory.mcp.tools.utils import wait_for_migration_or_return_status
|
||||
|
||||
migration_status = await wait_for_migration_or_return_status(timeout=5.0)
|
||||
migration_status = await wait_for_migration_or_return_status(
|
||||
timeout=5.0, project_name=active_project.name
|
||||
)
|
||||
if migration_status: # pragma: no cover
|
||||
return f"# System Status\n\n{migration_status}\n\nPlease wait for migration to complete before reading notes."
|
||||
|
||||
active_project = get_active_project(project)
|
||||
project_url = active_project.project_url
|
||||
|
||||
# Get the file via REST API - first try direct permalink lookup
|
||||
|
||||
@@ -525,11 +525,16 @@ def check_migration_status() -> Optional[str]:
|
||||
return None
|
||||
|
||||
|
||||
async def wait_for_migration_or_return_status(timeout: float = 5.0) -> Optional[str]:
|
||||
async def wait_for_migration_or_return_status(
|
||||
timeout: float = 5.0, project_name: Optional[str] = None
|
||||
) -> Optional[str]:
|
||||
"""Wait briefly for sync/migration to complete, or return status message.
|
||||
|
||||
Args:
|
||||
timeout: Maximum time to wait for sync completion
|
||||
project_name: Optional project name to check specific project status.
|
||||
If provided, only checks that project's readiness.
|
||||
If None, uses global status check (legacy behavior).
|
||||
|
||||
Returns:
|
||||
Status message if sync is still in progress, None if ready
|
||||
@@ -538,18 +543,36 @@ async def wait_for_migration_or_return_status(timeout: float = 5.0) -> Optional[
|
||||
from basic_memory.services.sync_status_service import sync_status_tracker
|
||||
import asyncio
|
||||
|
||||
if sync_status_tracker.is_ready:
|
||||
# Check if we should use project-specific or global status
|
||||
def is_ready() -> bool:
|
||||
if project_name:
|
||||
return sync_status_tracker.is_project_ready(project_name)
|
||||
return sync_status_tracker.is_ready
|
||||
|
||||
if is_ready():
|
||||
return None
|
||||
|
||||
# Wait briefly for sync to complete
|
||||
start_time = asyncio.get_event_loop().time()
|
||||
while (asyncio.get_event_loop().time() - start_time) < timeout:
|
||||
if sync_status_tracker.is_ready:
|
||||
if is_ready():
|
||||
return None
|
||||
await asyncio.sleep(0.1) # Check every 100ms
|
||||
|
||||
# Still not ready after timeout
|
||||
return sync_status_tracker.get_summary()
|
||||
if project_name:
|
||||
# For project-specific checks, get project status details
|
||||
project_status = sync_status_tracker.get_project_status(project_name)
|
||||
if project_status and project_status.status.value == "failed":
|
||||
error_msg = project_status.error or "Unknown sync error"
|
||||
return f"❌ Sync failed for project '{project_name}': {error_msg}"
|
||||
elif project_status:
|
||||
return f"🔄 Project '{project_name}' is still syncing: {project_status.message}"
|
||||
else:
|
||||
return f"⚠️ Project '{project_name}' status unknown"
|
||||
else:
|
||||
# Fall back to global summary for legacy calls
|
||||
return sync_status_tracker.get_summary()
|
||||
except Exception: # pragma: no cover
|
||||
# If there's any error, assume ready
|
||||
return None
|
||||
|
||||
@@ -72,10 +72,15 @@ async def write_note(
|
||||
"""
|
||||
logger.info(f"MCP tool call tool=write_note folder={folder}, title={title}, tags={tags}")
|
||||
|
||||
# Get the active project first to check project-specific sync status
|
||||
active_project = get_active_project(project)
|
||||
|
||||
# Check migration status and wait briefly if needed
|
||||
from basic_memory.mcp.tools.utils import wait_for_migration_or_return_status
|
||||
|
||||
migration_status = await wait_for_migration_or_return_status(timeout=5.0)
|
||||
migration_status = await wait_for_migration_or_return_status(
|
||||
timeout=5.0, project_name=active_project.name
|
||||
)
|
||||
if migration_status: # pragma: no cover
|
||||
return f"# System Status\n\n{migration_status}\n\nPlease wait for migration to complete before creating notes."
|
||||
|
||||
@@ -91,7 +96,6 @@ async def write_note(
|
||||
content=content,
|
||||
entity_metadata=metadata,
|
||||
)
|
||||
active_project = get_active_project(project)
|
||||
project_url = active_project.project_url
|
||||
|
||||
# Create or update via knowledge API
|
||||
|
||||
@@ -123,31 +123,31 @@ class SearchRepository:
|
||||
|
||||
def _prepare_boolean_query(self, query: str) -> str:
|
||||
"""Prepare a Boolean query by quoting individual terms while preserving operators.
|
||||
|
||||
|
||||
Args:
|
||||
query: A Boolean query like "tier1-test AND unicode" or "(hello OR world) NOT test"
|
||||
|
||||
|
||||
Returns:
|
||||
A properly formatted Boolean query with quoted terms that need quoting
|
||||
"""
|
||||
# Define Boolean operators and their boundaries
|
||||
boolean_pattern = r'(\bAND\b|\bOR\b|\bNOT\b)'
|
||||
|
||||
boolean_pattern = r"(\bAND\b|\bOR\b|\bNOT\b)"
|
||||
|
||||
# Split the query by Boolean operators, keeping the operators
|
||||
parts = re.split(boolean_pattern, query)
|
||||
|
||||
|
||||
processed_parts = []
|
||||
for part in parts:
|
||||
part = part.strip()
|
||||
if not part:
|
||||
continue
|
||||
|
||||
|
||||
# If it's a Boolean operator, keep it as is
|
||||
if part in ['AND', 'OR', 'NOT']:
|
||||
if part in ["AND", "OR", "NOT"]:
|
||||
processed_parts.append(part)
|
||||
else:
|
||||
# Handle parentheses specially - they should be preserved for grouping
|
||||
if '(' in part or ')' in part:
|
||||
if "(" in part or ")" in part:
|
||||
# Parse parenthetical expressions carefully
|
||||
processed_part = self._prepare_parenthetical_term(part)
|
||||
processed_parts.append(processed_part)
|
||||
@@ -155,15 +155,15 @@ class SearchRepository:
|
||||
# This is a search term - for Boolean queries, don't add prefix wildcards
|
||||
prepared_term = self._prepare_single_term(part, is_prefix=False)
|
||||
processed_parts.append(prepared_term)
|
||||
|
||||
|
||||
return " ".join(processed_parts)
|
||||
|
||||
|
||||
def _prepare_parenthetical_term(self, term: str) -> str:
|
||||
"""Prepare a term that contains parentheses, preserving the parentheses for grouping.
|
||||
|
||||
|
||||
Args:
|
||||
term: A term that may contain parentheses like "(hello" or "world)" or "(hello OR world)"
|
||||
|
||||
|
||||
Returns:
|
||||
A properly formatted term with parentheses preserved
|
||||
"""
|
||||
@@ -171,16 +171,16 @@ class SearchRepository:
|
||||
result = ""
|
||||
i = 0
|
||||
while i < len(term):
|
||||
if term[i] in '()':
|
||||
if term[i] in "()":
|
||||
# Preserve parentheses as-is
|
||||
result += term[i]
|
||||
i += 1
|
||||
else:
|
||||
# Find the next parenthesis or end of string
|
||||
start = i
|
||||
while i < len(term) and term[i] not in '()':
|
||||
while i < len(term) and term[i] not in "()":
|
||||
i += 1
|
||||
|
||||
|
||||
# Extract the content between parentheses
|
||||
content = term[start:i].strip()
|
||||
if content:
|
||||
@@ -191,43 +191,71 @@ class SearchRepository:
|
||||
result += f'"{escaped_content}"'
|
||||
else:
|
||||
result += content
|
||||
|
||||
|
||||
return result
|
||||
|
||||
|
||||
def _needs_quoting(self, term: str) -> bool:
|
||||
"""Check if a term needs to be quoted for FTS5 safety.
|
||||
|
||||
|
||||
Args:
|
||||
term: The term to check
|
||||
|
||||
|
||||
Returns:
|
||||
True if the term should be quoted
|
||||
"""
|
||||
if not term or not term.strip():
|
||||
return False
|
||||
|
||||
|
||||
# Characters that indicate we should quote (excluding parentheses which are valid syntax)
|
||||
needs_quoting_chars = [" ", ".", ":", ";", ",", "<", ">", "?", "/", "-", "'", '"',
|
||||
"[", "]", "{", "}", "+", "!", "@", "#", "$", "%", "^", "&",
|
||||
"=", "|", "\\", "~", "`"]
|
||||
|
||||
needs_quoting_chars = [
|
||||
" ",
|
||||
".",
|
||||
":",
|
||||
";",
|
||||
",",
|
||||
"<",
|
||||
">",
|
||||
"?",
|
||||
"/",
|
||||
"-",
|
||||
"'",
|
||||
'"',
|
||||
"[",
|
||||
"]",
|
||||
"{",
|
||||
"}",
|
||||
"+",
|
||||
"!",
|
||||
"@",
|
||||
"#",
|
||||
"$",
|
||||
"%",
|
||||
"^",
|
||||
"&",
|
||||
"=",
|
||||
"|",
|
||||
"\\",
|
||||
"~",
|
||||
"`",
|
||||
]
|
||||
|
||||
return any(c in term for c in needs_quoting_chars)
|
||||
|
||||
|
||||
def _prepare_single_term(self, term: str, is_prefix: bool = True) -> str:
|
||||
"""Prepare a single search term (no Boolean operators).
|
||||
|
||||
|
||||
Args:
|
||||
term: A single search term
|
||||
is_prefix: Whether to add prefix search capability (* suffix)
|
||||
|
||||
|
||||
Returns:
|
||||
A properly formatted single term
|
||||
"""
|
||||
if not term or not term.strip():
|
||||
return term
|
||||
|
||||
|
||||
term = term.strip()
|
||||
|
||||
|
||||
# Check if term is already a proper wildcard pattern (alphanumeric + *)
|
||||
# e.g., "hello*", "test*world" - these should be left alone
|
||||
if "*" in term and all(c.isalnum() or c in "*_-" for c in term):
|
||||
|
||||
@@ -131,6 +131,23 @@ class SyncStatusTracker:
|
||||
"""Check if system is ready (no sync in progress)."""
|
||||
return self._global_status in (SyncStatus.IDLE, SyncStatus.COMPLETED)
|
||||
|
||||
def is_project_ready(self, project_name: str) -> bool:
|
||||
"""Check if a specific project is ready for operations.
|
||||
|
||||
Args:
|
||||
project_name: Name of the project to check
|
||||
|
||||
Returns:
|
||||
True if the project is ready (completed, watching, or not tracked),
|
||||
False if the project is syncing, scanning, or failed
|
||||
"""
|
||||
project_status = self._project_statuses.get(project_name)
|
||||
if not project_status:
|
||||
# Project not tracked = ready (likely hasn't been synced yet)
|
||||
return True
|
||||
|
||||
return project_status.status in (SyncStatus.COMPLETED, SyncStatus.WATCHING, SyncStatus.IDLE)
|
||||
|
||||
def get_project_status(self, project_name: str) -> Optional[ProjectSyncStatus]:
|
||||
"""Get status for a specific project."""
|
||||
return self._project_statuses.get(project_name)
|
||||
|
||||
@@ -334,15 +334,30 @@ class TestSearchTermPreparation:
|
||||
# Test the specific case from the GitHub issue
|
||||
result = search_repository._prepare_search_term("tier1-test AND unicode")
|
||||
assert result == '"tier1-test" AND unicode'
|
||||
|
||||
|
||||
# Test other hyphenated Boolean combinations
|
||||
assert search_repository._prepare_search_term("multi-word OR single") == '"multi-word" OR single'
|
||||
assert search_repository._prepare_search_term("well-formed NOT badly-formed") == '"well-formed" NOT "badly-formed"'
|
||||
assert search_repository._prepare_search_term("test-case AND (hello OR world)") == '"test-case" AND (hello OR world)'
|
||||
|
||||
assert (
|
||||
search_repository._prepare_search_term("multi-word OR single")
|
||||
== '"multi-word" OR single'
|
||||
)
|
||||
assert (
|
||||
search_repository._prepare_search_term("well-formed NOT badly-formed")
|
||||
== '"well-formed" NOT "badly-formed"'
|
||||
)
|
||||
assert (
|
||||
search_repository._prepare_search_term("test-case AND (hello OR world)")
|
||||
== '"test-case" AND (hello OR world)'
|
||||
)
|
||||
|
||||
# Test mixed special characters with Boolean operators
|
||||
assert search_repository._prepare_search_term("config.json AND test-file") == '"config.json" AND "test-file"'
|
||||
assert search_repository._prepare_search_term("C++ OR python-script") == '"C++" OR "python-script"'
|
||||
assert (
|
||||
search_repository._prepare_search_term("config.json AND test-file")
|
||||
== '"config.json" AND "test-file"'
|
||||
)
|
||||
assert (
|
||||
search_repository._prepare_search_term("C++ OR python-script")
|
||||
== '"C++" OR "python-script"'
|
||||
)
|
||||
|
||||
def test_programming_terms_should_work(self, search_repository):
|
||||
"""Programming-related terms with special chars should be searchable."""
|
||||
|
||||
@@ -17,6 +17,9 @@ def test_sync_tracker_initial_state(sync_tracker):
|
||||
assert sync_tracker.global_status == SyncStatus.IDLE
|
||||
assert sync_tracker.get_summary() == "✅ System ready"
|
||||
|
||||
# Test project-specific ready check for unknown project
|
||||
assert sync_tracker.is_project_ready("unknown-project")
|
||||
|
||||
|
||||
def test_start_project_sync(sync_tracker):
|
||||
"""Test starting project sync."""
|
||||
@@ -211,3 +214,49 @@ def test_summary_without_file_counts(sync_tracker):
|
||||
summary = sync_tracker.get_summary()
|
||||
assert "🔄 Syncing 2 projects" in summary
|
||||
assert "files" not in summary # Should not show file progress
|
||||
|
||||
|
||||
def test_is_project_ready_functionality(sync_tracker):
|
||||
"""Test project-specific ready checks."""
|
||||
# Unknown project should be ready
|
||||
assert sync_tracker.is_project_ready("unknown-project")
|
||||
|
||||
# Project in different states
|
||||
sync_tracker.start_project_sync("scanning-project")
|
||||
assert not sync_tracker.is_project_ready("scanning-project") # SCANNING = not ready
|
||||
|
||||
sync_tracker.update_project_progress("scanning-project", SyncStatus.SYNCING, "Processing")
|
||||
assert not sync_tracker.is_project_ready("scanning-project") # SYNCING = not ready
|
||||
|
||||
sync_tracker.fail_project_sync("scanning-project", "Test error")
|
||||
assert not sync_tracker.is_project_ready("scanning-project") # FAILED = not ready
|
||||
|
||||
sync_tracker.complete_project_sync("scanning-project")
|
||||
assert sync_tracker.is_project_ready("scanning-project") # COMPLETED = ready
|
||||
|
||||
# Test watching project
|
||||
sync_tracker.start_project_watch("watching-project")
|
||||
assert sync_tracker.is_project_ready("watching-project") # WATCHING = ready
|
||||
|
||||
|
||||
def test_project_isolation_scenario(sync_tracker):
|
||||
"""Test the specific bug scenario: project isolation with mixed sync states."""
|
||||
# Set up the bug scenario: one failed project, one healthy project
|
||||
sync_tracker.start_project_sync("main")
|
||||
sync_tracker.fail_project_sync(
|
||||
"main", "UNIQUE constraint failed: entity.file_path, entity.project_id"
|
||||
)
|
||||
|
||||
sync_tracker.start_project_sync("basic-memory-testing-20250626-1009")
|
||||
sync_tracker.complete_project_sync("basic-memory-testing-20250626-1009")
|
||||
sync_tracker.start_project_watch("basic-memory-testing-20250626-1009")
|
||||
|
||||
# Global status should be failed due to "main" project
|
||||
assert sync_tracker.global_status == SyncStatus.FAILED
|
||||
assert not sync_tracker.is_ready
|
||||
|
||||
# But the healthy project should be ready for operations
|
||||
assert sync_tracker.is_project_ready("basic-memory-testing-20250626-1009")
|
||||
assert not sync_tracker.is_project_ready("main")
|
||||
|
||||
# This demonstrates the fix: project-specific checks allow isolation
|
||||
|
||||
Reference in New Issue
Block a user