fix(mcp): pass workspace parameter through client factory

[phernandez]

Summary

• Fix factory mode silently dropping the workspace parameter, making per-tool workspace switching impossible in the cloud MCP server
• Every MCP tool already accepts workspace and passes it to get_project_client(), but in factory mode the value was discarded at two points: get_client() and get_project_client()
• Update factory callable type to accept optional workspace kwarg
• Update test factories to match new signature

Context

Addresses basicmachines-co/basic-memory-cloud#460 — users connecting a single MCP remote endpoint cannot switch between personal and team workspaces. The companion PR in basic-memory-cloud updates the transport to read workspace from inner request headers.

Test plan

• tests/mcp/test_async_client_modes.py — 20/20 passing (factory tests updated)
• tests/mcp/test_project_context.py — factory test updated
• Integration test with cloud transport (companion PR)

🤖 Generated with Claude Code

[github-actions]

Claude finished @phernandez's task in 1m 14s —— View job

---

Code Review

• Gather context
• Read changed files
• Review against team checklist
• Post review summary

---

The fix is correct and minimal. Both drop sites are patched (get_client() line 179 and get_project_client() line 638), the comment is updated to explain the new contract, and tests are updated. A few things worth noting:

---

Issues

1. Type annotation weakened without need (async_client.py:135, 138)

Callable[..., ...] accepts any signature — argument constraints are completely gone. The old Callable[[], ...] was too strict (no kwargs), but Callable[..., ...] is too loose. A Protocol preserves safety:

from typing import Protocol

class ClientFactory(Protocol):
    def __call__(self, *, workspace: Optional[str] = None) -> AbstractAsyncContextManager[AsyncClient]: ...

Or at minimum the docstring for set_client_factory should say "factory must accept an optional workspace keyword argument". Right now there's nothing stopping a caller from registering a factory that ignores workspace, and the type checker won't catch it. Fix this →

2. No test asserting workspace is actually forwarded (test_async_client_modes.py)

test_get_client_uses_injected_factory checks the factory is called but not that workspace is forwarded. The core fix (passing workspace=workspace) has no direct coverage. A captured-workspace assertion would make this regression-proof:

@pytest.mark.asyncio
async def test_get_client_forwards_workspace_to_factory(monkeypatch):
    received = {}

    @asynccontextmanager
    async def factory(workspace=None):
        received["workspace"] = workspace
        async with httpx.AsyncClient(base_url="https://example.test") as client:
            yield client

    set_client_factory(factory)
    async with get_client(workspace="tenant-abc") as client:
        pass

    assert received["workspace"] == "tenant-abc"

Same gap exists in test_factory_mode_skips_workspace_resolution — the test proves workspace resolution is skipped, but doesn't assert the workspace value was forwarded to the factory. Fix this →

3. Minor: double space in comment (async_client.py:134)

connection time.  See basic-memory-cloud main.py

Two spaces before "See". Cosmetic but easy to fix.

---

Checklist

Item	Status
Follows Basic Memory coding conventions (decision-point comments updated)	✅
Python 3.12+ type annotations	⚠️ Callable[..., ...] loses argument constraints
No SQLAlchemy / Alembic changes needed	✅
100-char line limit maintained	✅
No commented-out code	✅
Unit tests updated	✅
100% coverage maintained	⚠️ workspace-forwarding path has no assertion coverage
No DB schema changes	✅
No hardcoded secrets or credentials	✅
Windows path compatibility	✅
No emoji/unicode in console output	✅

---

Summary

The core fix is right and the diff is appropriately small. Two things worth addressing before merge: the type annotation (a Protocol is the clean solution) and a test that actually asserts the workspace value reaches the factory. Everything else looks good.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: db5014fd85

ℹ️ 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]

Invalidate cached project when workspace override changes

This new factory-mode call path now forwards workspace, but get_active_project() still reuses active_project cache entries keyed only by project identifier, not workspace. In one MCP session, a call for project="..." in workspace A followed by workspace B can return the cached project from A, so downstream tools use the wrong external_id under the new workspace header and may operate on the wrong project (or fail with confusing 404/authorization errors). The cache should be re-keyed or bypassed when the requested workspace differs.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Preserve compatibility with no-arg client factories

get_client() now unconditionally calls the injected factory with workspace=..., which breaks existing factories that implement the previous zero-argument contract and are still valid according to prior versions of set_client_factory. Those integrations will now fail at runtime with an unexpected-keyword TypeError before any request handling. Please keep a compatibility path for factories that do not accept the new keyword.

Useful? React with 👍 / 👎.