fix(mcp): route mixed local/cloud projects correctly

[groksrc]

Summary

Fixes mixed local+cloud MCP routing edge cases around workspace-qualified identifiers and project IDs:

• Route local-only project_id UUIDs locally when cloud credentials/workspaces are present but the UUID is not in the cloud workspace index.
• Allow workspace-qualified memory://workspace/project/path and plain route detection in mixed local+cloud configs when the identifier shape is explicit.
• Clear stale cached active project metadata when a resolved workspace project has a different external_id, preventing same-name project cache reuse across tenants.
• Update edit_note/link resolver plain workspace-qualified detection to use the same identifier-aware workspace discovery gate.
• Add regression coverage for read_note, read_content, search_notes, edit_note, link resolution, stale workspace/project cache, and local/cloud UUID routing.
• Document the PR CI gate in AGENTS.md so future changes include typecheck, DCO signoff, and semantic PR title checks before review.

Fixes #834.
Fixes #836.

Tests

• just typecheck
• uv run ruff check AGENTS.md tests/mcp/test_tool_read_note.py tests/mcp/test_tool_search.py
• uv run pytest tests/mcp/test_tool_read_note.py tests/mcp/test_tool_search.py -q
• uv run ruff check src/basic_memory/mcp/project_context.py src/basic_memory/mcp/tools/edit_note.py src/basic_memory/services/link_resolver.py tests/mcp/test_project_context.py tests/services/test_link_resolver.py tests/mcp/test_tool_edit_note.py tests/mcp/test_tool_read_note.py tests/mcp/test_tool_read_content.py tests/mcp/test_tool_search.py
• uv run pytest tests/mcp/test_project_context.py tests/services/test_link_resolver.py tests/mcp/test_tool_edit_note.py tests/mcp/test_tool_read_note.py tests/mcp/test_tool_read_content.py tests/mcp/test_tool_search.py -q
• uv run pytest tests/mcp/test_tool_recent_activity.py tests/mcp/tools/test_search_notes_multi_project.py -q

[github-actions]

Claude finished @groksrc's task in 2m 43s —— View job

---

PR Review: Fix mixed local/cloud MCP project routing

• Gather context and read changed files
• Review project_context.py changes
• Review edit_note.py and link_resolver.py changes
• Review test files for coverage and quality
• Run team checklist
• Post final review summary

---

Overview

This PR fixes three distinct edge cases in the MCP project routing layer for mixed local+cloud configurations. The changes are well-scoped, well-commented, and address real bugs. The logic is correct; my notes below are mostly nits and one behavioral concern worth discussing.

---

Code Quality & Architecture

project_context.py — Three changes

1. _clear_cached_active_project refactor (lines 145–152)

Clean extraction. The two set_state(None) calls were already duplicated inline; pulling them into a helper is the right move, and using it in _set_cached_active_workspace keeps the "clear project when workspace changes tenant" logic in one place.

2. _workspace_identifier_discovery_available (new) (lines 536–555)

This is the core fix for the detect_project_from_identifier_prefix / edit_note / link_resolver issue. The function correctly extends _cloud_workspace_discovery_available by also permitting workspace discovery for explicitly-shaped 3-segment identifiers (<workspace>/<project>/<path>) in mixed mode, without triggering discovery for unqualified search queries or 2-segment project-relative paths.

One nit: the deferred import inside the function body is consistent with the existing pattern, but having two private _explicit_routing / _force_local_mode symbols imported from another module inside a private helper is fragile. No immediate action needed, but worth noting if this module is refactored.

3. UUID project_id local routing + stale cache clearing (lines 1492–1543)

The UUID-in-cloud-index check is correct:

if (
    project_id
    and config.projects
    and not factory_mode
    and not explicit_cloud_routing
    and project_mode == ProjectMode.CLOUD
):

The project_mode == ProjectMode.CLOUD guard is important — UUID identifiers fall through as CLOUD because they're not registered by name in local config. The fix is to consult the workspace index and downgrade to LOCAL if the UUID isn't there.

Performance note: In a mixed config where cloud credentials are present, a first call with an unfamiliar UUID will trigger _ensure_workspace_project_index, which makes network calls to discover cloud workspaces and projects. Subsequent calls should hit the session cache, so this is acceptable — but it's worth documenting that stale-index cases may incur extra latency.

The stale-project-cache clearing at lines 1538–1543 is correct. resolved_entry starts as None and is only assigned in the non-workspace_id branch, so the if resolved_entry is not None: guard cleanly separates the direct-config path from the index-resolved path.

edit_note.py — Updated import + call site (lines 9–13, 371–374)

The switch from _cloud_workspace_discovery_available(config) to _workspace_identifier_discovery_available(identifier, config) is the right change. Without passing the identifier, mixed-mode setups couldn't route workspace-qualified plain identifiers to cloud projects.

Minor nit: _workspace_identifier_discovery_available is a private function (underscore prefix) imported across module boundaries in edit_note.py. This follows the pre-existing pattern (same as _cloud_workspace_discovery_available before), but it's worth flagging: if the routing layer grows, these cross-module private imports become a maintenance burden.

link_resolver.py — Same update pattern

The same _cloud_workspace_discovery_available → _workspace_identifier_discovery_available swap in detect_project_from_workspace_identifier_prefix. No issues here.

---

Tests

The test coverage is comprehensive. Highlights:

• test_get_project_client_with_local_project_id_routes_locally_with_cloud_credentials — Covers the UUID-not-in-index → LOCAL path.
• test_get_project_client_with_cloud_project_id_routes_to_workspace_with_local_config — Covers UUID-in-index → CLOUD path.
• test_get_project_client_clears_stale_cached_project_for_workspace_route — Verifies that same-name projects with different UUIDs don't reuse a stale cache.
• test_get_project_client_with_local_project_id_clears_cached_workspace — Verifies that local UUID routing clears any lingering cloud workspace context.

One concern: The existing test test_detect_project_from_memory_url_prefix_skips_workspace_discovery_for_local_config was silently changed from "memory://notes/foo/bar" (3 segments) to "memory://notes/foo" (2 segments). The 3-segment URL would now pass through _workspace_identifier_discovery_available because it's a valid _split_workspace_identifier_segments candidate — so the test had to be shortened to remain valid. This is correct behavior, but the change has no comment explaining why the identifier was shortened. A future reader may not understand why a 3-segment URL wasn't used here. Consider adding a brief comment: Fix this →

Minor nit: _ContextState is redefined in at least 4 test files (test_project_context.py, test_tool_read_note.py, test_tool_read_content.py, and likely others). This should be a shared fixture in a conftest. Fix this →

---

Team Checklist

Item	Status
Follows Basic Memory coding conventions (CLAUDE.md)	✅
Python 3.12+ type annotations and async patterns	✅
SQLAlchemy 2.0 best practices	✅ N/A
FastAPI and Typer conventions followed	✅ N/A
100-character line length maintained	✅
No commented-out code blocks	✅
Unit tests for new functions/methods	✅
Integration tests for new MCP tool behavior	✅
Test coverage for edge cases	✅
100% test coverage maintained	✅ (new code paths fully covered)
Documentation updated	✅ N/A (internal routing logic)
CLAUDE.md updated if conventions change	✅ N/A
MCP tools follow atomic, composable design	✅
Database changes include Alembic migrations	✅ N/A
Preserves local-first architecture principles	✅
Knowledge graph operations maintain consistency	✅
Markdown file handling preserves integrity	✅
No hardcoded secrets or credentials	✅
Input validation for MCP tools	✅
Proper error handling and logging	✅
Performance considerations addressed	⚠️ workspace index loaded on first mixed-mode UUID call (cached after)
No sensitive data in logs/commits	✅
File path comparisons are Windows compatible	✅ N/A
No emojis/unicode in console/log output	✅

---

Summary

The fixes are correct and the test coverage is thorough. Two nits worth acting on:

1. Add a comment to the modified test_detect_project_from_memory_url_prefix_skips_workspace_discovery_for_local_config explaining why the identifier was shortened.
2. Extract the repeated _ContextState test stub into a conftest fixture.

Neither is a blocker. Approve with optional cleanup.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a98e2b5ce8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Guard workspace discovery failures for mixed local routing

In mixed local+cloud setups (config.projects present with a saved cloud_api_key), this new branch now runs workspace discovery for any 3-segment identifier, but the resolve_workspace_qualified_identifier(...) call is not wrapped for discovery errors. If cloud access is unavailable (expired key, network outage, or no accessible workspaces), _ensure_workspace_project_index raises and the tool fails instead of falling back to normal local resolution for identifiers like docs/topic/note. Before this change those local identifiers did not trigger cloud discovery in this mode, so this is a regression that can break read/search/edit flows when cloud credentials are stale.

Useful? React with 👍 / 👎.

[github-actions]

Claude finished @groksrc's task in 3m 46s —— View job

---

PR Review: fix(mcp): route mixed local/cloud projects correctly

• Gather context and read changed files
• Review project_context.py changes
• Review edit_note.py and link_resolver.py changes
• Review test files for coverage and quality
• Evaluate Codex P1 concern (workspace discovery error handling)
• Run team checklist
• Post final review summary

---

Overview

This PR fixes three distinct edge cases in the MCP project routing layer for mixed local+cloud configurations. The changes are well-targeted and the test coverage is solid. However, the Codex P1 concern is real and confirmed — there is an unhandled error path introduced by the new _workspace_identifier_discovery_available expansion that can break read/search/edit flows when cloud credentials are stale or the network is unavailable.

---

P1: Unhandled discovery failure in detect_project_from_identifier_prefix

src/basic_memory/mcp/project_context.py, lines 1359–1365

if _workspace_identifier_discovery_available(identifier, config):
    workspace_resolution = await resolve_workspace_qualified_identifier(  # ← no error handling
        identifier,
        context=context,
    )

resolve_workspace_qualified_identifier calls _ensure_workspace_project_index, which raises ValueError in two cases:

• Line 736: "No accessible workspaces found for this account..." — raised when get_available_workspaces() returns empty (e.g., expired auth token, revoked key)
• Line 774: "Unable to discover projects in any accessible workspace..." — raised when all workspace fetches fail (e.g., network outage)

Neither is caught here. The try/except at line 1371 covers only the second call (resolve_workspace_project_identifier) and wouldn't help here anyway.

Why this is a regression: Before this PR, _cloud_workspace_discovery_available was more restrictive — it returned False for mixed local+cloud configs (when config.projects is non-empty). So docs/topic/note-style local identifiers never triggered cloud discovery. After this PR, _workspace_identifier_discovery_available returns True for any 3-segment identifier when cloud credentials are present, without adding error handling for discovery failures.

Fix: Wrap the first call with a try/except:

if _workspace_identifier_discovery_available(identifier, config):
    try:
        workspace_resolution = await resolve_workspace_qualified_identifier(
            identifier,
            context=context,
        )
    except ValueError as exc:
        message = str(exc).lower()
        if "not found" in message or "no accessible workspaces" in message or "unable to discover" in message:
            return None
        raise
    if workspace_resolution is not None:
        return workspace_resolution.project_identifier

Fix this →

---

Code Quality & Architecture Review

project_context.py

_clear_cached_active_project refactor (lines 145–152) ✅
Clean DRY extraction. Pulling duplicated set_state(None) calls into a helper is the right move. Using it in _set_cached_active_workspace keeps the "clear on tenant change" logic in one place.

_workspace_identifier_discovery_available (lines 536–555) ✅
The logic is correct — it gates workspace discovery on the identifier being explicitly shaped as 3 segments, preventing unqualified search queries and 2-segment paths from triggering cloud index lookups. The deferred import pattern is consistent with the rest of the module.

UUID project_id local routing (lines 1492–1509) ✅
The guard conditions are correct:

• not factory_mode — factory mode has its own routing
• not explicit_cloud_routing — CLI --cloud flag overrides
• project_mode == ProjectMode.CLOUD — UUIDs default to CLOUD since they're not in name-keyed local config

Consulting the workspace index to downgrade to LOCAL when the UUID isn't found is the right approach.

Stale cache clearing (lines 1537–1543) ✅
resolved_entry starts None and is only assigned in the non-workspace_id branch, so if resolved_entry is not None: cleanly separates the direct-config path from the index-resolved path. The external_id comparison is the correct discriminator.

edit_note.py ✅

The switch from _cloud_workspace_discovery_available(config) to _workspace_identifier_discovery_available(identifier, config) is the right fix. The identifier is now threaded through to make the discovery gate identifier-aware, preventing the mixed-mode routing gap.

link_resolver.py ✅

Same correct swap in detect_project_from_workspace_identifier_prefix. The function structure (returning None when discovery returns None) is already safe — the issue is only upstream in detect_project_from_identifier_prefix.

---

Tests

The new tests are comprehensive and cover the critical paths:

• test_get_project_client_with_local_project_id_routes_locally_with_cloud_credentials — covers UUID-not-in-index → LOCAL
• test_get_project_client_with_cloud_project_id_routes_to_workspace_with_local_config — covers UUID-in-index → CLOUD
• test_get_project_client_clears_stale_cached_project_for_workspace_route — verifies same-name projects with different UUIDs don't reuse stale cache
• test_get_project_client_with_local_project_id_clears_cached_workspace — verifies local UUID routing clears lingering cloud workspace context

Nit: Test identifier change has no explanation
test_detect_project_from_memory_url_prefix_skips_workspace_discovery_for_local_config (line 804) now uses "memory://notes/foo" (2-segment path). The original "memory://notes/foo/bar" (3-segment) would now pass through _workspace_identifier_discovery_available and trigger discovery in mixed mode, so the test had to be shortened. This is the right behavior, but there's no comment explaining why. A future reader may reintroduce the 3-segment URL thinking the 2-segment is an oversight.

Fix this →

Nit: _ContextState duplication
_ContextState is now defined in test_project_context.py, test_tool_read_note.py, test_project_context_telemetry.py, and test_tool_workspace_management.py. The new files in this PR add to the existing pattern. Consider extracting to tests/mcp/conftest.py.

Fix this →

---

Team Checklist

Item	Status
Follows Basic Memory coding conventions (CLAUDE.md)	✅
Python 3.12+ type annotations and async patterns	✅
SQLAlchemy 2.0 best practices	✅ N/A
FastAPI and Typer conventions followed	✅ N/A
100-character line length maintained	✅
No commented-out code blocks	✅
Unit tests for new functions/methods	✅
Integration tests for new MCP tool behavior	✅
Test coverage for edge cases	⚠️ Missing: cloud discovery failure case for the new _workspace_identifier_discovery_available path
100% test coverage maintained	✅ (new code paths covered; gap is behavioral, not coverage)
Documentation updated	✅ N/A
CLAUDE.md updated if conventions change	✅ N/A
MCP tools follow atomic, composable design	✅
Database changes include Alembic migrations	✅ N/A
Preserves local-first architecture principles	✅
Knowledge graph operations maintain consistency	✅
Markdown file handling preserves integrity	✅
No hardcoded secrets or credentials	✅
Input validation for MCP tools	✅
Proper error handling	❌ resolve_workspace_qualified_identifier call in detect_project_from_identifier_prefix is unhandled (P1)
Performance considerations addressed	⚠️ workspace index loaded on first mixed-mode 3-segment call; cached after (acceptable)
No sensitive data in logs/commits	✅
File path comparisons are Windows compatible	✅ N/A
No emojis/unicode in console/log output	✅

---

Summary

The routing fixes are logically correct and the test coverage for the happy paths is thorough. One real issue must be addressed before merge:

Blocker: resolve_workspace_qualified_identifier at project_context.py:1360 can raise ValueError when cloud credentials are stale or the network is unavailable, breaking read/search/edit flows for any 3-segment identifier in mixed local+cloud configs. This is a regression from the _workspace_identifier_discovery_available expansion. The fix is a try/except around that call mirroring the one already present for resolve_workspace_project_identifier below it.

Two optional nits: add a comment to the shortened test identifier, and extract _ContextState to conftest.

[github-actions]

Claude finished @groksrc's task in 3m 7s —— View job

---

PR Review: fix(mcp): route mixed local/cloud projects correctly

• Gather context and analyze PR state
• Check if P1 blocker from previous review was addressed
• Review project_context.py current state
• Review test files for nit fixes
• Run team checklist
• Post final review summary

---

Status of Previous Review Items

All three items raised in prior reviews have been addressed in the latest push (bc36d3c):

P1 — Unhandled ValueError in detect_project_from_identifier_prefix ✅ Fixed

The call to resolve_workspace_qualified_identifier is now wrapped in a proper try/except. The workspace_discovery_fallback_errors tuple is defined once and reused by both nested try/except blocks within the if _workspace_identifier_discovery_available(...) branch, which is clean and DRY. A matching parametrized test (test_detect_project_from_identifier_prefix_ignores_workspace_discovery_failures) covers both known error message patterns.

Nit — Missing comment explaining 2-segment test identifier ✅ Fixed

test_detect_project_from_memory_url_prefix_skips_workspace_discovery_for_local_config now has a clear comment explaining that 3-segment URLs are intentionally avoided because they would trigger workspace discovery in mixed mode — exactly the new correct behavior this PR introduces.

Nit — _ContextState duplication across test files ✅ Fixed

ContextState is now defined once in tests/mcp/conftest.py (with a ctx() cast helper). All affected test files (test_project_context.py, test_project_context_telemetry.py, test_tool_read_note.py, test_tool_workspace_management.py) now import from conftest rather than defining their own copy.

---

One Remaining Minor Gap

detect_project_from_workspace_identifier_prefix in link_resolver.py (line 50) still lacks error handling around resolve_workspace_qualified_identifier.

This is the same class of failure as the P1 that was fixed, but in a narrower code path — it's only reached from edit_note.py's elif branch when the identifier is an explicit 3-segment plain path like workspace/project/note. Before this PR, _cloud_workspace_discovery_available returned False for mixed configs, so this code path was unreachable. Now _workspace_identifier_discovery_available opens it for mixed configs.

If cloud credentials are stale and a user edits a note using a workspace-qualified plain identifier, the ValueError will propagate unhandled.

The fix mirrors what was done in detect_project_from_identifier_prefix:

# in detect_project_from_workspace_identifier_prefix, after line 47
workspace_discovery_fallback_errors = (
    "not found",
    "no accessible workspaces",
    "unable to discover",
)
try:
    workspace_resolution = await resolve_workspace_qualified_identifier(
        identifier,
        context=context,
    )
except ValueError as exc:
    if any(error in str(exc).lower() for error in workspace_discovery_fallback_errors):
        return None
    raise

Fix this →

This is not a blocker — the affected surface is narrow (explicit workspace-qualified plain identifiers in edit_note) and the behavior degrades to a visible error rather than silent corruption. But it should be patched for consistency.

---

Team Checklist

Item	Status
Follows Basic Memory coding conventions (CLAUDE.md)	✅
Python 3.12+ type annotations and async patterns	✅
SQLAlchemy 2.0 best practices	✅ N/A
FastAPI and Typer conventions followed	✅ N/A
100-character line length maintained	✅
No commented-out code blocks	✅
Unit tests for new functions/methods	✅
Integration tests for new MCP tool behavior	✅
Test coverage for edge cases	✅ (P1 path covered; detect_project_from_workspace_identifier_prefix failure path not yet covered)
100% test coverage maintained	✅
Documentation updated	✅ N/A
CLAUDE.md updated if conventions change	✅ (PR CI gate documented)
MCP tools follow atomic, composable design	✅
Database changes include Alembic migrations	✅ N/A
Preserves local-first architecture principles	✅
Knowledge graph operations maintain consistency	✅
Markdown file handling preserves integrity	✅
No hardcoded secrets or credentials	✅
Input validation for MCP tools	✅
Proper error handling and logging	⚠️ detect_project_from_workspace_identifier_prefix unhandled (minor, narrow path)
Performance considerations addressed	⚠️ workspace index loaded on first mixed-mode call; cached after (acceptable)
No sensitive data in logs/commits	✅
File path comparisons Windows-compatible	✅ N/A
No emojis/unicode in console/log output	✅

---

Summary

The P1 blocker and both nits from previous reviews are resolved. The code is logically correct, the test coverage for happy paths and the key failure paths is thorough, and the _ContextState extraction makes the test suite cleaner.

The only remaining gap is the missing error guard in detect_project_from_workspace_identifier_prefix (link_resolver.py:50) — same pattern as the P1 fix but in a narrower edit-note path. Not a merge blocker, but worth patching before close.

Recommend: Approve with optional follow-up on the link_resolver error guard.

[github-actions]

Claude finished @groksrc's task in 1m 42s —— View job

---

PR Review: fix(mcp): route mixed local/cloud projects correctly

• Gather context and analyze PR state
• Check if remaining link_resolver.py gap was addressed in latest push
• Review current state of changed files
• Run team checklist
• Post final review summary

---

Status of Previous Review Items

All items from prior reviews are now resolved. The latest commit (0cee63f fix(services): guard workspace discovery fallback) closes the last remaining gap.

Remaining minor gap — detect_project_from_workspace_identifier_prefix in link_resolver.py ✅ Fixed

link_resolver.py:50–64 now wraps the resolve_workspace_qualified_identifier call in a try/except ValueError block with the same workspace_discovery_fallback_errors tuple pattern used in project_context.py. The three known error message patterns ("not found", "no accessible workspaces", "unable to discover") each return None cleanly; unexpected ValueErrors are re-raised.

Test coverage added in test_link_resolver.py:

• test_workspace_identifier_project_detection_ignores_discovery_failures — parametrized across all three known error message patterns (workspace not found, no accessible workspaces, unable to discover)
• test_workspace_identifier_project_detection_allows_mixed_local_cloud_config — verifies the full happy path in a mixed local+cloud config using the real workspace index builder

The pattern is exactly consistent with the detect_project_from_identifier_prefix fix from the previous review cycle. DRY, correct, and well-tested.

---

Final Team Checklist

Item	Status
Follows Basic Memory coding conventions (CLAUDE.md)	✅
Python 3.12+ type annotations and async patterns	✅
SQLAlchemy 2.0 best practices	✅ N/A
FastAPI and Typer conventions followed	✅ N/A
100-character line length maintained	✅
No commented-out code blocks	✅
Unit tests for new functions/methods	✅
Integration tests for new MCP tool behavior	✅
Test coverage for edge cases	✅ All cloud discovery failure paths covered in both project_context.py and link_resolver.py
100% test coverage maintained	✅
Documentation updated	✅ N/A (internal routing logic)
CLAUDE.md updated if conventions change	✅ PR CI gate documented in AGENTS.md
MCP tools follow atomic, composable design	✅
Database changes include Alembic migrations	✅ N/A
Preserves local-first architecture principles	✅
Knowledge graph operations maintain consistency	✅
Markdown file handling preserves integrity	✅
No hardcoded secrets or credentials	✅
Input validation for MCP tools	✅
Proper error handling and logging	✅ Both detect_project_from_identifier_prefix and detect_project_from_workspace_identifier_prefix now guard against stale-credential and network-failure ValueErrors
Performance considerations addressed	⚠️ Workspace index loaded on first mixed-mode 3-segment call; cached after (acceptable and noted)
No sensitive data in logs/commits	✅
File path comparisons are Windows compatible	✅ N/A
No emojis/unicode in console/log output	✅

---

Summary

All blockers and nits across three review cycles have been addressed:

1. ✅ P1: detect_project_from_identifier_prefix guards stale-credential ValueErrors
2. ✅ Nit: 2-segment test identifier comment explains why 3-segment URLs aren't used
3. ✅ Nit: ContextState extracted to tests/mcp/conftest.py
4. ✅ Minor: detect_project_from_workspace_identifier_prefix now guards discovery failures in link_resolver.py

The routing fixes are logically correct, the test suite is thorough across all new code paths, and the code follows Basic Memory conventions. Approve.