feat: add workspace select command #1856
Conversation
📝 WalkthroughWalkthroughThe changes implement persistent tenant state management by adding storage and retrieval APIs for the current workspace, updating the default tenant initialization to read from persistent state, and introducing a new workspace selection command that allows users to switch between workspaces with automatic state persistence. Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as CLI Command
participant Client as Client
participant State as Persistent State
User->>CLI: workspace select <workspace-id>
CLI->>Client: GetTenant (lookup workspace)
Client-->>CLI: TenantNameOrID
CLI->>Client: SetCurrentTenant(tenant)
Client->>State: Write Tenant to state
State-->>Client: Success
Client-->>CLI: Success
CLI->>Client: Reconnect with new tenant
Client-->>CLI: Connection established
CLI-->>User: Workspace switched & persisted
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/pkg/cli/client/state.go`:
- Around line 69-72: SetCurrentTenant currently assigns to the zero-valued
package-level state and writes it, which can erase existing fields; before
mutating, read/initialize the existing state from disk (e.g., call the state
reader like state.read(statePath) or use the package getter that ensures state
is loaded), handle any read error, then set state.Tenant and call
state.write(statePath) so existing AnonID and TermsAcceptedAt are preserved;
update SetCurrentTenant to perform this load-before-write flow and propagate
errors.
🧹 Nitpick comments (1)
src/cmd/cli/command/workspace.go (1)
80-94: Remove redundant newline from log output and clarify commented annotation.The explicit
\nin line 91 is unnecessary—standard Go logging functions likeInfofautomatically append a newline if missing, soterm.Infof("Switched to workspace %q\n", ...)will produce a double newline. Remove the\nfrom the format string.Additionally, the commented-out
authNeededAnnotationon line 84 should either be uncommented (if auth is required) or removed entirely (if intentionally not needed). If authentication is delegated toConnectWithTenanton line 88, the comment should be deleted to avoid confusion.Suggested cleanup
var workspaceSelectCmd = &cobra.Command{ Use: "select WORKSPACE", Aliases: []string{"use", "switch"}, Args: cobra.ExactArgs(1), - // Annotations: authNeededAnnotation, Short: "Select a workspace to use", RunE: func(cmd *cobra.Command, args []string) error { global.Tenant = types.TenantNameOrID(args[0]) if _, err := cli.ConnectWithTenant(cmd.Context(), global.Cluster, global.Tenant); err != nil { return err } - term.Infof("Switched to workspace %q\n", global.Tenant) + term.Infof("Switched to workspace %q", global.Tenant) return client.SetCurrentTenant(global.Tenant) }, }
| func SetCurrentTenant(tenant types.TenantNameOrID) error { | ||
| state.Tenant = tenant | ||
| return state.write(statePath) | ||
| } |
There was a problem hiding this comment.
Potential data loss: SetCurrentTenant may overwrite uninitialized state.
If SetCurrentTenant is called before any getter (like GetAnonID or GetCurrentTenant), the package-level state variable will still be zero-valued. Writing it will overwrite any existing AnonID and TermsAcceptedAt in the state file.
Consider initializing the state before modifying it:
Proposed fix
func SetCurrentTenant(tenant types.TenantNameOrID) error {
+ state = initState(statePath)
state.Tenant = tenant
return state.write(statePath)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func SetCurrentTenant(tenant types.TenantNameOrID) error { | |
| state.Tenant = tenant | |
| return state.write(statePath) | |
| } | |
| func SetCurrentTenant(tenant types.TenantNameOrID) error { | |
| state = initState(statePath) | |
| state.Tenant = tenant | |
| return state.write(statePath) | |
| } |
🤖 Prompt for AI Agents
In `@src/pkg/cli/client/state.go` around lines 69 - 72, SetCurrentTenant currently
assigns to the zero-valued package-level state and writes it, which can erase
existing fields; before mutating, read/initialize the existing state from disk
(e.g., call the state reader like state.read(statePath) or use the package
getter that ensures state is loaded), handle any read error, then set
state.Tenant and call state.write(statePath) so existing AnonID and
TermsAcceptedAt are preserved; update SetCurrentTenant to perform this
load-before-write flow and propagate errors.
Description
Using our (existing) state file for tracking the current workspace.
Linked Issues
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.