fix: add workspace routing to cloud upload and API client

[groksrc]

Summary

• get_cloud_control_plane_client() now accepts a workspace param and sends X-Workspace-ID header
• cloud_utils functions (fetch_cloud_projects, create_cloud_project, project_exists) now accept and pass workspace
• upload command resolves workspace from per-project workspace_id or default_workspace
• get_client() per-project routing auto-resolves workspace from project config entry

Test plan

• bm cloud upload ~/aitext --project aitext --dry-run shows correct workspace
• bm cloud upload ~/aitext --project aitext --create-project creates project in teams workspace
• Files visible and searchable in teams workspace via MCP after upload
• Existing personal workspace uploads still work (no regression)
• Unit tests pass

Fixes #703

🤖 Generated with Claude Code

[github-actions]

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

---

PR Review: fix/cloud-upload-workspace-routing

• Gather context and read changed files
• Review code quality and standards
• Review testing and documentation
• Review architecture patterns
• Review security and performance
• Post full review summary

---

Overview

This PR correctly routes X-Workspace-ID headers through cloud upload and per-project API client calls. The general approach is sound and the workspace resolution priority order (per-project workspace_id → default_workspace) is consistent across both changed files.

---

Issues Found

🔴 Breaking test: fake_project_exists missing workspace kwarg

File: tests/cli/cloud/test_upload_command_routing.py:23

async def fake_project_exists(_project_name: str) -> bool:
    return True

The PR changes upload_command.py to call:

if not await project_exists(project, workspace=workspace):

The monkeypatched fake_project_exists does not accept a workspace keyword argument. This will raise a TypeError at runtime, breaking the existing test.

Fix this →

---

🟡 Missing tests for new workspace behavior

The PR adds workspace propagation but no new tests verify:

1. _workspace_headers sends the X-Workspace-ID header — test_cloud_utils_fetch_and_exists_and_create_project doesn't assert the header is included when workspace is passed.
2. get_cloud_control_plane_client(workspace=...) sets the header — existing tests call it without a workspace; no test calls it with a workspace and asserts X-Workspace-ID.
3. get_client() auto-resolves workspace from entry.workspace_id — test_get_client_cloud_adds_workspace_header passes workspace explicitly; there's no test that sets entry.workspace_id in config and verifies auto-resolution.
4. upload_command.py workspace resolution — the new workspace-selection logic (project entry → default_workspace fallback) isn't exercised in tests.

Per CLAUDE.md: "Coverage must stay at 100%".

---

🟡 Duplicated workspace resolution logic

The same resolution pattern appears in two places:

upload_command.py:78-84:

entry = config.projects.get(project)
if entry and entry.workspace_id:
    workspace = entry.workspace_id
elif config.default_workspace:
    workspace = config.default_workspace

async_client.py:188-194:

entry = config.projects.get(project_name)
if entry and entry.workspace_id:
    effective_workspace = entry.workspace_id
elif config.default_workspace:
    effective_workspace = config.default_workspace

This is acceptable under the project's "no speculative abstraction" rule (two callsites), but worth noting for future reference if a third site appears.

---

🟡 Missing docstring workspace param in fetch_cloud_projects

create_cloud_project and project_exists both have workspace documented in their docstrings, but fetch_cloud_projects does not.

File: src/basic_memory/cli/commands/cloud/cloud_utils.py:26-49

---

🟢 Minor: Inconsistent Optional vs union type syntax

async_client.py uses Optional[str] (line 91, 143) while cloud_utils.py uses str | None (lines 19, 28, 55, 114). Both are valid in Python 3.12+ but the file-level inconsistency is minor.

---

Checklist

Category	Status	Notes
Python 3.12+ type annotations	✅	Both str | None and Optional[str] used
Async patterns	✅	asynccontextmanager usage is correct
FastAPI/Typer conventions	✅	Typer CLI patterns followed
100-char line length	✅	Within limits
No commented-out code	✅
Unit tests for new code	❌	Missing workspace header assertions and auto-resolve tests
100% test coverage	⚠️	New branches in workspace resolution uncovered
No hardcoded secrets	✅
No sensitive data in logs	✅	workspace ID is logged, which seems acceptable
Input validation	✅	Truthy check on workspace is appropriate
Local-first architecture preserved	✅	Workspace only affects cloud routing
Windows path compatibility	✅	No path comparisons in changed code
No emoji/unicode in console output	✅

---

Summary

The workspace routing logic itself is correct and well-structured. The critical blocker is that the existing test test_cloud_upload_uses_control_plane_client will fail because fake_project_exists doesn't accept the new workspace keyword argument. Additional tests for the new workspace header behavior are needed to maintain 100% coverage.

[github-actions]

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

---

PR Review: fix/cloud-upload-workspace-routing

• Gather context and read changed files
• Review code quality and standards
• Review testing and documentation
• Review architecture patterns
• Review security and performance
• Post full review summary

---

Overview

This revision addresses all critical issues from the previous review. The workspace routing logic is sound, the broken test is fixed, and comprehensive workspace coverage has been added for the async_client and cloud_utils layers.

---

Fixed Since Last Review ✅

Issue	Status
fake_project_exists missing workspace kwarg	Fixed — workspace: str | None = None added
_workspace_headers untested	Fixed — test_workspace_headers_with_workspace + test_workspace_headers_without_workspace added
get_cloud_control_plane_client(workspace=...) untested	Fixed — test_get_cloud_control_plane_client_with_workspace added
get_client() auto-resolve from entry.workspace_id untested	Fixed — test_get_client_auto_resolves_workspace_from_project_config and test_get_client_explicit_workspace_overrides_config added
get_client() auto-resolve from default_workspace untested	Fixed — test_get_client_auto_resolves_workspace_from_default added
fetch_cloud_projects missing workspace docstring	Fixed — workspace: Cloud workspace tenant_id to list projects from added
fetch_cloud_projects/project_exists/create_cloud_project workspace header not asserted	Fixed — test_cloud_utils_pass_workspace_header asserts x-workspace-id for all three

---

Remaining Issues

🟡 Upload command workspace resolution branches not exercised

File: src/basic_memory/cli/commands/cloud/upload_command.py:80-84

entry = config.projects.get(project)
if entry and entry.workspace_id:
    workspace = entry.workspace_id
elif config.default_workspace:
    workspace = config.default_workspace

The sole test in test_upload_command_routing.py invokes --project routing-test against a blank config_manager, so config.projects.get("routing-test") returns None and config.default_workspace is None. The two workspace = assignment lines (82 and 84) are never executed. Per CLAUDE.md the 100% coverage requirement is strict, so these branches need tests.

Two tests would cover this:

1. Set cfg.projects["routing-test"].workspace_id = "tenant-from-entry" → assert fake_get_client receives workspace="tenant-from-entry".
2. Set cfg.default_workspace = "default-tenant" (no per-project entry) → assert fake_get_client receives workspace="default-tenant".

Fix this →

---

🟢 Minor: Optional[str] vs str | None type syntax

async_client.py uses Optional[str] in three function signatures (lines 72, 92, 143) while cloud_utils.py and the rest of the project use str | None (Python 3.12+ style). Pre-existing in the file, but the new additions follow the same pattern. Low priority.

---

Checklist

Category	Status	Notes
Python 3.12+ type annotations	⚠️	Optional[str] in async_client.py — pre-existing
Async patterns	✅	asynccontextmanager usage is correct throughout
FastAPI/Typer conventions	✅	Followed correctly
100-char line length	✅	Within limits
No commented-out code	✅
_workspace_headers unit tests	✅	Both with/without workspace
Cloud utils workspace header tests	✅	All three functions asserted
get_client() workspace auto-resolve tests	✅	Project entry, default_workspace, and explicit override all covered
get_cloud_control_plane_client workspace test	✅	Both with/without workspace
Upload command workspace resolution	❌	entry.workspace_id and default_workspace branches uncovered
No hardcoded secrets	✅
No sensitive data in logs	✅	workspace ID in dim console output is acceptable
Input validation	✅	Truthy check on workspace is appropriate
Local-first architecture preserved	✅	Workspace only affects cloud routing
Windows path compatibility	✅	No path comparisons in changed code

---

Summary

All critical issues from the previous review are resolved. The one remaining gap is test coverage for the upload command's workspace resolution logic — the entry.workspace_id and default_workspace branches in upload_command.py:80-84 aren't exercised by any test. Adding two focused tests there would satisfy the 100% coverage requirement and make this PR ready to merge.