fix(mcp): validate navigation pagination and fix recent_activity project display

[phernandez]

Summary

Fixes three navigation/pagination bugs found by the integration-test bug hunt in the MCP tools search_notes, build_context, and recent_activity. Two of them (#9, #10) make pagination validation consistent across all three navigation tools — previously only recent_activity rejected non-positive page/page_size. The third (#13) stops recent_activity's text header from leaking a raw project UUID.

Bugs fixed

• feat: import chatgpt conversation data #9 — search_notes accepted non-positive page_size and returned a misleading has_more=true with zero rows. A page_size=0 returned {results: [], total: 15, has_more: True} — no reachable page could ever fetch those rows. A negative page_size became an uncapped SQLite LIMIT and returned an arbitrary slice. Added the same page < 1 / page_size < 1 guard that recent_activity already uses, so the tool fails fast with a clear ValueError.
• fix: Re do enhanced read note format #10 — build_context with page_size<=0 silently dropped the requested primary entity. A valid memory:// lookup returned primary_count=0 because the context service does primary = primary[:limit], and limit=0 truncates the resolved entity to []. Added the same page/page_size validation so a valid direct lookup never silently returns nothing.
• feat: Incremental sync on watch #13 — recent_activity text output showed the raw project UUID instead of the project name when routed via project_id. When callers pass only project_id (a UUID), resolved_project is the verbatim UUID. The fix uses active_project.name (the canonical, display-safe name already in scope from get_project_client) for both the log line and the formatted header — matching every other tool's display behavior.

Testing

All regression tests live in test-int/bughunt_fixes/test_navigation_pagination_integration.py and use the real MCP server (FastMCP Client), real DB, and ASGI routing — no mocks. The CLI portion uses the real Typer CliRunner.

Commands and results:

• uv run pytest test-int/bughunt_fixes/test_navigation_pagination_integration.py -q --no-cov → 4 passed
• BASIC_MEMORY_TEST_POSTGRES=1 uv run pytest test-int/bughunt_fixes/... -q --no-cov → 4 passed (Postgres)
• uv run pytest tests/mcp/ -q --no-cov → 697 passed, 8 skipped (includes test_tool_contracts.py + test_tool_telemetry.py)
• uv run pytest test-int/mcp/test_pagination_integration.py test-int/mcp/test_build_context_validation.py test-int/mcp/test_ui_sdk_integration.py test-int/mcp/test_search_integration.py test-int/mcp/test_output_format_json_integration.py test-int/mcp/test_chatgpt_tools_integration.py test-int/mcp/test_param_aliases_integration.py -q --no-cov → all passed
• uv run pytest test-int/cli/test_cli_tool_json_integration.py test-int/cli/test_search_notes_meta_integration.py test-int/cli/test_cli_tool_json_failure_integration.py -q --no-cov → all passed
• uv run pytest tests/cli/ -k "tool or search" -q --no-cov → 48 passed
• uv run ruff check ... / uv run ruff format ... → clean
• uv run ty check src tests test-int → All checks passed

Risk

Low. No tool signatures changed. The fix tightens existing pagination validation to match recent_activity's established pattern (raise ValueError for page<1/page_size<1), so callers that previously got silent bad payloads now get an explicit error. The CLI search-notes command already maps ValueError to a non-zero exit with a clear message. Internal search_notes callers (chatgpt_tools, read_note, ui_sdk, ci) all use positive defaults. The #13 change is a display-only fix.

🤖 Generated with Claude Code

[github-actions]

Claude finished @phernandez's task in 2m 6s —— View job

---

Code Review

• Gather context and understand the PR
• Read changed source files
• Read integration test file
• Review against team checklist
• Post final review

---

Summary

Three focused, well-scoped bug fixes with thorough integration tests. The changes are minimal and correct. A few observations below — none are blocking.

---

Code Quality & Standards

Item	Status	Notes
Coding conventions (CLAUDE.md)	✅	Trigger/Why/Outcome comments on all three guards — exactly as specified
Python 3.12+ type annotations	✅	No new type signatures; existing annotations unchanged
SQLAlchemy / FastAPI / Typer conventions	✅	No ORM changes; CLI ValueError → non-zero exit behavior already wired
100-char line length	✅	All added lines are under the limit
No commented-out code	✅

---

Bug Fixes

Bug #9 — search_notes (search.py:877-880) ✅

The guard is placed before the try/except Exception block at line 1109, so a ValueError propagates as a raw ToolError rather than being swallowed by the search-error formatter. That's the right behavior (fail-fast, explicit signal). One thing worth noting: _search_all_projects (line 510-511) still has max(page, 1) / max(page_size, 1) clamps that are now unreachable via normal call paths (the outer search_notes rejects invalid values before the all-projects branch). They're harmless defensive code, but could be cleaned up later.

Bug #10 — build_context (build_context.py:219-222) ✅

Guard is placed before any async work, which is correct. The comment accurately explains the primary[:limit] truncation root cause.

Bug #13 — recent_activity (recent_activity.py:300-328) ✅

The fix moves the logger.info call inside the context manager so active_project.name is available, and updates the _format_project_output call from resolved_project to active_project.name. Clean and accurate.

---

Minor Observations

1. Upper-bound inconsistency: recent_activity has if page_size > 100: raise ValueError(...) (recent_activity.py:143-144) that build_context and search_notes do not gain in this PR. Not a regression — it's pre-existing — but a follow-up to make the cap consistent across all three tools would help. Fix this →

2. Test gap — build_context with page < 1: test_build_context_nonpositive_page_size_drops_primary tests page_size=0 but not page=0. The search_notes test covers both cases. Minor gap since the code paths are symmetric. Fix this →

3. Pre-existing emojis in output: _format_discovery_output and _format_project_output use emojis (🔧, 📋, ✅, 🧪, 📄, 🔍, 🔗). Not introduced by this PR, but they conflict with the CLAUDE.md "avoid emojis and unicode in console and log output" guideline. Worth a follow-up cleanup pass on those functions.

---

Testing & Documentation

Item	Status	Notes
Integration tests with real server/DB/ASGI	✅	All 4 tests use FastMCP Client, real DB, real routing
Negative cases (invalid inputs rejected)	✅	page_size=0, page_size=-3, page=0 all tested for search_notes; page_size=0 for build_context
Positive baseline (valid args still work)	✅	Happy-path assertions present in both MCP and CLI tests
Bug #13 regression test	✅	Asserts UUID absent and project name present in output text
100% coverage	✅	New guards are all exercised by the integration tests
bughunt_fixes directory structure	✅	No __init__.py needed — consistent with mcp/ and cli/ subdirs

---

Checklist

• Follows Basic Memory's coding conventions in CLAUDE.md
• Python 3.12+ type annotations and async patterns
• SQLAlchemy 2.0 best practices (N/A)
• FastAPI and Typer conventions followed
• 100-character line length limit maintained
• No commented-out code blocks
• Integration tests for MCP tools
• Test coverage for edge cases (minor gap: build_context page<1)
• 100% test coverage maintained
• Docstrings: build_context and search_notes docstrings say Raises: ToolError generically — could be tightened to mention pagination, but non-blocking
• MCP tools follow atomic, composable design
• Database changes include Alembic migrations (N/A)
• Preserves local-first architecture principles
• No hardcoded secrets or credentials
• Input validation for MCP tools ✅ (the whole point of this PR)
• Proper error handling and logging
• No sensitive data in logs or commits
• File path comparisons windows-compatible (PurePosixPath used explicitly)
• Emojis in console output — pre-existing, not introduced here, but worth a follow-up

---

Verdict: Approved with minor follow-ups noted above. The fixes are correct, minimal, and well-tested. The three observations are non-blocking and can be addressed in follow-up PRs.