fix(cli): project list MCP column shows transport type instead of DB presence

[groksrc]

Summary

• Rename "MCP (stdio)" column to "MCP" in bm project list
• Show actual transport type: stdio (local) or https (cloud) based on project routing mode
• Clear local path display for cloud-mode projects

Previously the column only checked whether a project existed in the local DB, ignoring the configured routing mode entirely.

Test plan

• Updated assertions in test_project_list_and_ls.py for new column name and values
• Verified test_json_output.py still passes (key name mcp_stdio unchanged)
• All 242 CLI tests pass
• ruff check, ruff format, pyright all clean

Fixes #659

🤖 Generated with Claude Code

[github-actions]

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

---

PR Review: Fix project list MCP column to show transport type

• Read changed files
• Review code quality & standards
• Check testing & documentation
• Verify architecture compliance
• Post final review

---

Summary

This PR fixes a legitimate bug: the old code set mcp_stdio_target = "local" (or "n/a") based solely on whether a project existed in the local DB, completely ignoring the configured routing mode. The new code correctly maps routing mode to transport: stdio for local projects, https for cloud projects. Clean, focused change.

---

Team Checklist

Code Quality & Standards

• Follows Basic Memory's coding conventions in CLAUDE.md
• Python 3.12+ type annotations and async patterns
• SQLAlchemy 2.0 best practices — N/A (no DB changes)
• FastAPI and Typer conventions followed
• 100-character line length limit maintained
• No commented-out code blocks

Testing & Documentation

• Unit tests for new functions/methods — updated assertions in test_project_list_and_ls.py
• Integration tests for new MCP tools — N/A
• Test coverage for edge cases — see minor gap below
• 100% test coverage maintained — JSON key preserved, test_json_output.py still passes
• Documentation updated — N/A (small display fix)
• CLAUDE.md updated if conventions change — N/A

Basic Memory Architecture

• MCP tools follow atomic, composable design — N/A
• Database changes include Alembic migrations — N/A
• Preserves local-first architecture principles — ✅
• Knowledge graph operations maintain consistency — N/A
• Markdown file handling preserves integrity — N/A
• AI-human collaboration patterns followed — ✅

Security & Performance

• No hardcoded secrets or credentials
• Input validation for MCP tools — N/A
• Proper error handling and logging
• Performance considerations addressed — N/A
• No sensitive data in logs or commits

Compatibility

• File path comparisons must be Windows compatible — N/A for this change
• Avoid using emojis and unicode characters in console output — ✅ (pre-existing Unicode bars not touched)

---

Issues Found

Minor: Variable name inconsistency

project.py:209 — The variable mcp_stdio_target still carries stdio in its name but now holds either "stdio" or "https". The JSON key mcp_stdio is intentionally kept for backward compatibility (good), but the local variable name could be mcp_transport to better reflect the new meaning. This is cosmetic only.

Fix this →

Minor: Edge case not directly tested

The second branch in the transport logic:

# project.py:193-194
elif cloud_project is not None and local_project is None:
    mcp_stdio_target = "https"

This handles a cloud-only project with no local config entry (entry is None). The existing test covers the entry.mode == ProjectMode.CLOUD path, but not this fallback. It's a minor gap since the fallback is simple, but a dedicated test scenario would be belt-and-suspenders.

Observation: local_path clearing is intentional and correct

New block at project.py:167-170:

# Clear local path for cloud-mode projects — only local projects
# should display a local path
if entry and entry.mode == ProjectMode.CLOUD:
    local_path = ""

This overrides the earlier population from entry.local_sync_path for cloud projects. The PR description explicitly calls this out as intentional ("Clear local path display for cloud-mode projects"). The comment is clear. ✅

---

Verdict

Approve — The fix is correct and well-targeted. The old behavior was clearly wrong (using DB presence as a proxy for transport type). The new behavior accurately reflects the configured routing mode. Tests validate both the column header rename and the transport values. The two minor issues above are cosmetic / nice-to-have and don't block merging.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 851015f8b2

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Gate HTTPS fallback on missing project config

The elif cloud_project is not None and local_project is None branch marks MCP transport as https even when the project is explicitly configured as local (entry.mode == local). This happens when bm project list --cloud is used, because that path only fetches cloud projects and leaves local_project unset, so local-mode projects that also exist in cloud are mislabeled as HTTPS. The fallback should only apply when no local routing configuration exists for the project (for example, entry is None).

Useful? React with 👍 / 👎.

[github-actions]

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

---

PR Review: Fix project list MCP column to show transport type

• Read changed files
• Review code quality & standards
• Check testing & documentation
• Verify architecture compliance
• Post final review

---

Summary

This PR was updated to address the variable rename from the previous review (mcp_stdio_target → mcp_transport). The core fix — showing actual transport type (stdio vs https) instead of a fixed "local" string — is correct and well-targeted.

---

Team Checklist

Code Quality & Standards

• Follows Basic Memory's coding conventions in CLAUDE.md
• Python 3.12+ type annotations and async patterns
• SQLAlchemy 2.0 best practices — N/A (no DB changes)
• FastAPI and Typer conventions followed
• 100-character line length limit maintained
• No commented-out code blocks

Testing & Documentation

• Unit tests updated — test_project_list_and_ls.py assertions updated
• Integration tests — N/A
• 100% test coverage maintained — JSON key mcp_stdio preserved, test_json_output.py still passes
• Documentation updated — N/A (display-only fix)
• CLAUDE.md updated if conventions change — N/A

Basic Memory Architecture

• Preserves local-first architecture principles ✅
• No DB changes — N/A for migrations

Security & Performance

• No hardcoded secrets or credentials
• No sensitive data in logs or commits

Compatibility

• No Unicode additions (pre-existing bars untouched)

---

Issues Found

✅ Previous review concern addressed

The mcp_stdio_target → mcp_transport rename requested in the prior review is complete (project.py:191–196). The JSON dict key mcp_stdio is intentionally preserved for backward compatibility. ✅

---

Bug: --cloud flag causes wrong transport for local-mode projects

project.py:191–196 — the transport logic has a hole when bm project list --cloud is used:

# Current logic
if entry and entry.mode == ProjectMode.CLOUD:
    mcp_transport = "https"
elif cloud_project is not None and local_project is None:   # ← problem branch
    mcp_transport = "https"
else:
    mcp_transport = "stdio"

When --cloud is passed, only cloud_result is populated — local_result stays None. So local_project is None for every project in the loop. A project configured as mode: local in config that also exists in cloud would hit the elif branch (cloud_project is not None and local_project is None) and be incorrectly labeled https.

The cli_route logic just above (lines 176–185) avoids this by checking entry first as a catch-all before falling back to cloud-only inference:

elif entry:
    cli_route = entry.mode.value     # covers both LOCAL and CLOUD via entry
elif cloud_project is not None and local_project is None:
    ...

The mcp_transport block should mirror this pattern:

if entry and entry.mode == ProjectMode.CLOUD:
    mcp_transport = "https"
elif entry and entry.mode == ProjectMode.LOCAL:   # ← add this guard
    mcp_transport = "stdio"
elif cloud_project is not None and local_project is None:
    mcp_transport = "https"
else:
    mcp_transport = "stdio"

This matches how cli_route is computed and ensures entry.mode always wins over inferred routing.

Fix this →

---

Minor: Edge case test gap remains

The elif cloud_project is not None and local_project is None branch (no entry at all) is not directly tested. Now that the --cloud flag scenario above is identified, it would also be worth a test specifically for the local-mode-in-cloud-list scenario. The existing tests only cover the main path (local project local, cloud project cloud).

---

Verdict

Request changes — The core fix is correct and the variable rename was applied cleanly. However, the transport logic has an edge case bug where bm project list --cloud can mislabel local-mode projects as https. The cli_route logic just 10 lines above correctly handles this by checking entry first; the mcp_transport block should do the same. The fix is a one-line addition.

[github-actions]

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

---

PR Review: Fix project list MCP column to show transport type

• Read changed files
• Review code quality & standards
• Check testing & documentation
• Verify architecture compliance
• Post final review

---

Summary

This PR has been through two prior review cycles and the key issues have been addressed well. The core fix (showing actual transport type instead of DB presence) is correct, the variable rename from mcp_stdio_target → mcp_transport is clean, and the --cloud flag bug from the second review has been resolved with a tighter condition.

---

Team Checklist

Code Quality & Standards

• Follows Basic Memory's coding conventions in CLAUDE.md
• Python 3.12+ type annotations and async patterns
• SQLAlchemy 2.0 best practices — N/A (no DB changes)
• FastAPI and Typer conventions followed
• 100-character line length limit maintained
• No commented-out code blocks

Testing & Documentation

• Unit tests updated — test_project_list_and_ls.py assertions check stdio and https column values
• Integration tests — N/A (display-only fix)
• Test coverage for edge cases — see minor gap below
• 100% test coverage maintained — JSON key mcp_stdio preserved, test_json_output.py unaffected
• Documentation updated — N/A
• CLAUDE.md updated if conventions change — N/A

Basic Memory Architecture

• MCP tools follow atomic, composable design — N/A
• Database changes include Alembic migrations — N/A
• Preserves local-first architecture principles ✅
• Knowledge graph operations maintain consistency — N/A
• Markdown file handling preserves integrity — N/A
• AI-human collaboration patterns followed ✅

Security & Performance

• No hardcoded secrets or credentials
• Input validation for MCP tools — N/A
• Proper error handling and logging
• Performance considerations addressed — N/A
• No sensitive data in logs or commits

Compatibility

• File path comparisons must be Windows compatible — N/A
• Avoid using emojis and unicode characters in console output — ✅ (pre-existing Unicode bars not touched)

---

Issues Found

✅ Previous bug fix verified: --cloud flag no longer mislabels local projects

The second review identified that the original elif cloud_project is not None and local_project is None branch would mislabel local-mode projects as https when --cloud was passed (because local_project would be None for every row in that path).

The current code correctly fixes this by using entry is None instead of local_project is None:

# project.py:191-196
if entry and entry.mode == ProjectMode.CLOUD:
    mcp_transport = "https"
elif entry is None and cloud_project is not None:   # ← gated on config entry, not local_project
    mcp_transport = "https"
else:
    mcp_transport = "stdio"

entry comes from config.projects.get(project_name) (line 157) — the local config file — which is unaffected by --cloud. A local-mode project will always have entry.mode == ProjectMode.LOCAL, so it correctly falls through to stdio. ✅

Minor: Edge case test gap (cosmetic, not blocking)

The elif entry is None and cloud_project is not None branch (a cloud project with zero local config entry) has no direct test. The main test covers entry.mode == CLOUD for the beta project. Worth a dedicated test scenario for future robustness, but not a blocker.

---

Verdict

Approve — The fix is correct and all issues from prior reviews have been properly addressed. The --cloud flag bug from the second review is resolved by gating the HTTPS fallback on entry is None rather than local_project is None, which is the right approach. Tests validate the column header rename and correct transport values for both local and cloud projects. Clean, focused change.

[bm-clawd]

Clean fix. The old column was misleading — showing DB presence when what matters is the transport. Logic looks correct: cloud-mode → https, everything else → stdio. Cloud projects also correctly clear local_path now.

One minor note: the JSON key stays mcp_stdio which is technically a misnomer now (could be https), but changing it would break consumers so leaving it is the right call.

Ship it. 🚢