{{ message }}
fix(mcp): validate navigation pagination and fix recent_activity project display - #915
Merged
Conversation
…ect display From the integration-test bug hunt. Regression tests live under test-int/bughunt_fixes/ and exercise the real MCP server, DB, and ASGI routing (no mocks). Bugs fixed: - #9 search_notes accepted non-positive page_size and returned a misleading has_more=true with zero rows (a negative page_size became an uncapped SQLite LIMIT). Added the same page/page_size guard recent_activity uses so all three navigation tools reject invalid pagination consistently. - #10 build_context with page_size<=0 silently dropped the requested primary entity (primary_count=0 for a valid memory:// URL) because the context service does primary = primary[:limit]. Added the same page/page_size validation. - #13 recent_activity text output printed the raw project UUID instead of the project name when routed via project_id. Use active_project.name (the canonical, display-safe name) for both the log line and the formatted header. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes three navigation/pagination bugs found by the integration-test bug hunt in the MCP tools
search_notes,build_context, andrecent_activity. Two of them (#9, #10) make pagination validation consistent across all three navigation tools — previously onlyrecent_activityrejected non-positivepage/page_size. The third (#13) stopsrecent_activity's text header from leaking a raw project UUID.Bugs fixed
search_notesaccepted non-positivepage_sizeand returned a misleadinghas_more=truewith zero rows. Apage_size=0returned{results: [], total: 15, has_more: True}— no reachable page could ever fetch those rows. A negativepage_sizebecame an uncapped SQLiteLIMITand returned an arbitrary slice. Added the samepage < 1/page_size < 1guard thatrecent_activityalready uses, so the tool fails fast with a clearValueError.build_contextwithpage_size<=0silently dropped the requested primary entity. A validmemory://lookup returnedprimary_count=0because the context service doesprimary = primary[:limit], andlimit=0truncates the resolved entity to[]. Added the samepage/page_sizevalidation so a valid direct lookup never silently returns nothing.recent_activitytext output showed the raw project UUID instead of the project name when routed viaproject_id. When callers pass onlyproject_id(a UUID),resolved_projectis the verbatim UUID. The fix usesactive_project.name(the canonical, display-safe name already in scope fromget_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.pyand use the real MCP server (FastMCPClient), real DB, and ASGI routing — no mocks. The CLI portion uses the real TyperCliRunner.Commands and results:
uv run pytest test-int/bughunt_fixes/test_navigation_pagination_integration.py -q --no-cov→ 4 passedBASIC_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 (includestest_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 passeduv 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 passeduv run pytest tests/cli/ -k "tool or search" -q --no-cov→ 48 passeduv run ruff check .../uv run ruff format ...→ cleanuv run ty check src tests test-int→ All checks passedRisk
Low. No tool signatures changed. The fix tightens existing pagination validation to match
recent_activity's established pattern (raiseValueErrorforpage<1/page_size<1), so callers that previously got silent bad payloads now get an explicit error. The CLIsearch-notescommand already mapsValueErrorto a non-zero exit with a clear message. Internalsearch_notescallers (chatgpt_tools, read_note, ui_sdk, ci) all use positive defaults. The #13 change is a display-only fix.🤖 Generated with Claude Code