Skip to content

fix(cli): clear default_workspace on cloud logout#773

Merged
phernandez merged 1 commit intomainfrom
fix/cloud-logout-clears-default-workspace
Apr 29, 2026
Merged

fix(cli): clear default_workspace on cloud logout#773
phernandez merged 1 commit intomainfrom
fix/cloud-logout-clears-default-workspace

Conversation

@phernandez
Copy link
Copy Markdown
Member

Summary

  • bm cloud logout only invalidated OAuth tokens — it left config.default_workspace pointing at the previously selected tenant.
  • A subsequent bm cloud login (e.g. switching from an org workspace back to personal) inherited that stale selection and silently routed every cloud-mode project through the wrong tenant. The user had no signal that the active workspace hadn't reset.
  • Reporter (bm cloud login/logout don't reset default_workspace; no workspace picker on login #755) hit this while migrating a project out of a Teams workspace; only bm cloud workspace set-default <personal> recovered.

Change

  • cli/commands/cloud/core_commands.pylogout() now clears config.default_workspace and saves the config when a non-None value was cleared. Skips the write when there's nothing to clear, so we don't touch the file unnecessarily.

Out of scope

  • The reporter also suggested login should prompt to pick a default workspace when multiple are available. That's a UX enhancement (needs to fetch workspaces, render a chooser, validate selection) and worth a separate PR. This change focuses on the smaller invariant: ending a session must invalidate the cached selection.

Test plan

  • New TestLogoutCommand class in tests/cli/test_cloud_authentication.py:
    • test_logout_clears_default_workspace — confirms the value goes from set → None and config is saved exactly once
    • test_logout_skips_save_when_no_default_workspace — confirms no save call when there was nothing to clear
  • uv run pytest tests/cli/test_cloud_authentication.py --no-cov — green

Closes #755

🤖 Generated with Claude Code

`bm cloud logout` previously only invalidated OAuth tokens, leaving
config.default_workspace pointing at whatever tenant was selected before
the session ended. A subsequent `bm cloud login` (often as a different
user, or returning from an org workspace to personal) would silently
inherit the stale selection and route every cloud-mode project through
the wrong tenant.

Clear default_workspace on logout, and only rewrite the config file
when there was a value to clear (avoid touching the file when nothing
needs to change).

Closes #755

Signed-off-by: phernandez <paul@basicmachines.co>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 29, 2026

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


PR Review: fix(cli): clear default_workspace on cloud logout

Tasks:

  • Gather context and read changed files
  • Review implementation against team checklist
  • Post final review

Summary

Clean, well-scoped fix. The bug is real, the change is minimal, and the tests cover both branches. A few notes below.


Code Quality & Standards

  • Follows Basic Memory's coding conventions
  • Python 3.12+ type annotations and async patterns — no type changes needed
  • SQLAlchemy 2.0 best practices — N/A
  • FastAPI and Typer conventions followed
  • 100-character line length limit maintained
  • No commented-out code blocks
  • Decision-point comment follows the Trigger/Why/Outcome literate style from CLAUDE.md

Minor inconsistency (non-blocking): logout() accesses config_manager.config (the property alias) while the adjacent status() command at line 102 calls config_manager.load_config() directly. Both resolve to the same thing since config is a @property delegating to load_config(), but aligning to load_config() would be more consistent with the rest of the file.

Fix this →


Testing & Documentation

  • Unit tests for new behavior — TestLogoutCommand covers both code paths
  • 100% coverage maintained — both if config.default_workspace is not None branches are exercised
  • No integration tests needed (pure config-file mutation, no DB or MCP involvement)
  • No README/docstring changes needed

Test structure note (non-blocking): FakeConfigManager.config is a class-level attribute rather than an instance-level one. This works because the test only ever creates one instance, but it's worth knowing that if logout() ever mutated config_manager.config directly (rather than through config), the reference semantics would differ from production. The current test accurately captures the real behavior.


Basic Memory Architecture

  • No DB changes, no Alembic migration needed
  • No MCP tool changes
  • Local-first principles preserved

Security & Performance

  • No hardcoded secrets or credentials
  • Skip-save optimization (if config.default_workspace is not None) is correct — avoids a disk write when there's nothing to clear
  • Exception ordering is correct: auth.logout() runs before default_workspace is cleared, so a failed logout leaves the workspace selection intact (safe fail)

Compatibility

  • No file path operations — Windows compat not a concern here
  • No emojis or non-ASCII characters in console output

UX Nit (optional)

When default_workspace is non-None and gets cleared, the user sees no acknowledgment. A single [dim] line like:

console.print("[dim]Default workspace selection cleared.[/dim]")

inside the if block would give the user a signal that something changed. Worth considering for discoverability, but not a blocker given the PR's stated scope.


Overall: LGTM with the optional nit above. The fix is correct, minimal, well-tested, and properly documented. Ready to merge.

@phernandez phernandez merged commit 128c2da into main Apr 29, 2026
55 of 57 checks passed
@phernandez phernandez deleted the fix/cloud-logout-clears-default-workspace branch April 29, 2026 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bm cloud login/logout don't reset default_workspace; no workspace picker on login

1 participant