Repository navigation
Scope home launcher to caller workspace group - #17
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA new ChangesWorkspace Group Scope Feature
Sequence DiagramsequenceDiagram
actor TUI as TUI (App)
participant SubmitFlow as submit_new_workspace
participant ScopeResolver as caller_group_scope
participant API as workspace API
participant GroupAPI as workspace.group.list
participant RefreshWorker
participant LoadWorkspaces as load_workspaces
rect rgba(100, 149, 237, 0.5)
Note over TUI, GroupAPI: Submit path
TUI->>SubmitFlow: create new workspace
SubmitFlow->>ScopeResolver: resolve group_scope if unset
ScopeResolver->>GroupAPI: workspace.group.list
GroupAPI-->>ScopeResolver: group membership data
ScopeResolver-->>SubmitFlow: WorkspaceGroupScope
SubmitFlow->>TUI: upsert_optimistic_workspace(group_id)
SubmitFlow->>API: workspace.create(group_id, placement)
API-->>SubmitFlow: created workspace
SubmitFlow->>API: add_workspace_to_group_top (if group mismatch)
SubmitFlow->>TUI: apply_submit_success(group_scope)
end
rect rgba(60, 179, 113, 0.5)
Note over TUI, LoadWorkspaces: Refresh path
TUI->>RefreshWorker: trigger refresh
RefreshWorker->>ScopeResolver: caller_group_scope
ScopeResolver->>GroupAPI: workspace.group.list
GroupAPI-->>ScopeResolver: group membership data
ScopeResolver-->>RefreshWorker: WorkspaceGroupScope
RefreshWorker->>LoadWorkspaces: load_workspaces(group_scope)
LoadWorkspaces->>LoadWorkspaces: filter by group membership
LoadWorkspaces-->>RefreshWorker: WorkspaceStatus list with group_id
RefreshWorker->>TUI: apply_refresh(RefreshSnapshot with group_scope)
TUI->>TUI: update self.group_scope, filter pending rows
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main.rs`:
- Around line 643-650: The apply_refresh method and other group scope lookup
operations use `.ok().flatten()` on `caller_group_scope(...)` which silently
treats errors the same as "no group available". Instead of discarding errors,
check if the error is a known "groups unsupported" compatibility case and only
treat it as "no group" in that specific case; for other errors, preserve the
existing scope and surface the error appropriately. Apply this change to all
occurrences of the group scope lookup pattern in the apply_refresh function and
at the other locations mentioned (around lines 1072-1075 and 5586-5603) to
prevent unintended scope clearing or creation of ungrouped workspaces from
grouped launchers.
- Around line 5449-5462: The current implementation doesn't safely handle
parameter compatibility and post-create failures in the workspace creation flow.
When calling client.v2("workspace.create", params) with group_id and
group_placement, an older cmux version may reject these parameters causing the
entire operation to fail before retry. Additionally, if workspace.create
succeeds but the subsequent add_workspace_to_group_top call fails, the function
returns an error after the workspace already exists, leading to duplication on
retry. Fix this by implementing a retry mechanism that catches explicit
unsupported-parameter errors from the initial workspace.create call and retries
without the group_id and group_placement parameters, and handle any failures
from add_workspace_to_group_top after successful workspace creation as a
partial-success case (such as logging a warning or triggering a refresh) rather
than propagating the error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ce7b055b-4fee-4dd2-a07f-f5cea1b6e2c6
📒 Files selected for processing (3)
src/events.rssrc/main.rssrc/model.rs
Comment on lines
643
to
+650
| fn apply_refresh(&mut self, snapshot: RefreshSnapshot) { | ||
| let previously_selected_id = self.selected_workspace().map(|ws| ws.id.clone()); | ||
| self.group_scope = snapshot.group_scope; | ||
| let pending_workspaces = self | ||
| .workspaces | ||
| .iter() | ||
| .filter(|workspace| is_pending_workspace_id(&workspace.id)) | ||
| .filter(|workspace| self.workspace_is_in_scope(workspace)) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Don’t fail open when group scope lookup errors.
caller_group_scope(...).ok().flatten() treats a workspace.group.list failure the same as “no group”. On refresh, that loads the global workspace list and Line 645 clears the active scope; on submit, it can create an ungrouped workspace from a grouped launcher. Preserve the existing scope or surface the lookup error unless the response is a known “groups unsupported” compatibility case.
Also applies to: 1072-1075, 5586-5603
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main.rs` around lines 643 - 650, The apply_refresh method and other group
scope lookup operations use `.ok().flatten()` on `caller_group_scope(...)` which
silently treats errors the same as "no group available". Instead of discarding
errors, check if the error is a known "groups unsupported" compatibility case
and only treat it as "no group" in that specific case; for other errors,
preserve the existing scope and surface the error appropriately. Apply this
change to all occurrences of the group scope lookup pattern in the apply_refresh
function and at the other locations mentioned (around lines 1072-1075 and
5586-5603) to prevent unintended scope clearing or creation of ungrouped
workspaces from grouped launchers.
Comment on lines
+5449
to
+5462
| if let Some(group_scope) = &request.group_scope { | ||
| params["group_id"] = json!(&group_scope.group_id); | ||
| params["group_placement"] = json!("top"); | ||
| } | ||
|
|
||
| let mut client = CmuxClient::new(request.socket_path.clone()); | ||
| let created = client.v2("workspace.create", params)?; | ||
| let workspace_id = string_field(&created, "workspace_id") | ||
| .ok_or_else(|| anyhow!("workspace.create did not return workspace_id"))?; | ||
| if let Some(group_scope) = &request.group_scope { | ||
| let created_group_id = string_field(&created, "group_id"); | ||
| if created_group_id.as_deref() != Some(group_scope.group_id.as_str()) { | ||
| add_workspace_to_group_top(&mut client, &workspace_id, group_scope)?; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make the group fallback safe around the non-idempotent create.
The fallback only runs after workspace.create succeeds. If an older cmux rejects group_id/group_placement, submit fails before retrying without those params. Conversely, if create succeeds but workspace.group.add fails, this returns Err after the workspace already exists, so the UI restores the draft and a retry can duplicate the workspace. Retry only before a workspace is created for explicit unsupported-param errors, and treat post-create grouping failures as a partial-success path with a refresh/warning or a real compensating action.
Also applies to: 5478-5490
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main.rs` around lines 5449 - 5462, The current implementation doesn't
safely handle parameter compatibility and post-create failures in the workspace
creation flow. When calling client.v2("workspace.create", params) with group_id
and group_placement, an older cmux version may reject these parameters causing
the entire operation to fail before retry. Additionally, if workspace.create
succeeds but the subsequent add_workspace_to_group_top call fails, the function
returns an error after the workspace already exists, leading to duplication on
retry. Fix this by implementing a retry mechanism that catches explicit
unsupported-parameter errors from the initial workspace.create call and retries
without the group_id and group_placement parameters, and handle any failures
from add_workspace_to_group_top after successful workspace creation as a
partial-success case (such as logging a warning or triggering a refresh) rather
than propagating the error.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
Paired cmux app PR: manaflow-ai/cmux#6657
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Scopes the home launcher to the caller’s workspace group so you only see and create workspaces in that group. Group lookup is routed through the caller workspace; new workspaces are placed at the top of the group with a fallback for older
cmuxbuilds.CMUX_WORKSPACE_REF/CMUX_WORKSPACE_ID) usingworkspace.group.list, and builds aWorkspaceGroupScope.group_idandgroup_placement=toponworkspace.create; falls back toworkspace.group.add+workspace.reorderrelative to the group anchor if needed.group_idtoWorkspaceStatusand threads scope through refresh, submit, created-event handling, and optimistic rows.Written for commit 248453f. Summary will update on new commits.
Summary by CodeRabbit