Skip to content

Keep a collapsed sidebar group folded when the workspace below it closes - #10169

Merged
teamleaderleo merged 3 commits into
manaflow-ai:mainfrom
AvoChang:fix-collapsed-group-expands-on-close
Sep 26, 2026
Merged

teamleaderleo merged 3 commits into
manaflow-ai:mainfrom
AvoChang:fix-collapsed-group-expands-on-close

Conversation

@AvoChang

@AvoChang AvoChang commented Aug 14, 2026 •

Copy link
Copy Markdown
Contributor

Problem

A collapsed sidebar group force-expands when the workspace sitting directly below it is closed.

Cause

The close path's selection fallback indexes into the flat tabs[] array:

let newIndex = min(index, max(0, tabs.count - 1))
selectedTabId = tabs[newIndex].id

tabs[] holds every workspace, including members a collapsed group hides. The sidebar's actual rows are group anchors plus ungrouped workspaces (sidebarTopLevelWorkspaceIds()), so this can select a workspace that has no row.

That assignment then runs selectedTabId's didSet → selectedWorkspaceIdDidChange → expandWorkspaceGroupForSelectionIfNeeded(), which sees the selection inside a collapsed group it is not the anchor of, and clears isCollapsed. The auto-expand is correct in itself — "a selected row must be visible". The defect is selecting an invisible row in the first place.

Why it needs the workspace to be last

newIndex = min(index, count - 1):

  • Closing a non-last workspace walks forwards to whatever moved up into the slot. A group run always starts with its anchor (anchorFirst), and the anchor is exempt from the auto-expand — no bug.
  • Closing the last workspace walks backwards to the preceding entry, which is the collapsed group's last member — bug.

Since the ordering invariant puts groups above ungrouped-unpinned workspaces, a lone ungrouped workspace under a collapsed group is exactly this shape.

Fix

Resolve the slot's occupant to a row the sidebar renders. A non-anchor member of a collapsed group hands off to its group's anchor; members of an expanded group have their own rows and are selected as before.

This mirrors toggleWorkspaceGroupCollapsed, which already moves focus to the anchor when collapsing would hide the selected member, and SidebarState.scrollTargetWorkspaceId, which already maps hidden members to the anchor for scrolling.

The fallback moved to WorkspacesModel.selectionTargetAfterClose(closedIndex:) so it is reachable from the package's tests.

Commits

Per the regression policy, the test lands first so CI proves it catches the bug:

  1. Failing tests — the extraction is behavior-preserving, so the two collapsed-group tests fail here.
  2. Fix — the anchor hand-off.
Test 1 2
closingLastWorkspaceSelectsAnchorNotHiddenMemberOfCollapsedGroup ❌ ✅
closingLastWorkspaceKeepsPrecedingGroupCollapsed (end-to-end) ❌ ✅
closingLastWorkspaceSelectsAdjacentMemberOfExpandedGroup (guards over-correction) ✅ ✅
closingNonLastWorkspaceSelectsTheWorkspaceThatMovedUp ✅ ✅

Also: CmuxWorkspacesTests was not running in CI

CmuxWorkspaces is absent from the swift test allowlist in .github/workflows/ci.yml, so that whole suite — including the existing group/reorder coordinator tests — has never been a CI gate. Without adding it, commit 1 could not go red and the regression policy would be vacuous. Its dependencies (CmuxSettings, bonsplit, CMUXDebugLog, CmuxTestSupport) already resolve headlessly in that job.

Out of scope

index is captured before promoteAnchorOrRemoveGroupsAnchoredBy, which calls normalizeWorkspaceGroupContiguity and can reorder tabs. On an anchor close the index may not match the post-reorder array. That is pre-existing and untouched here, though the anchor hand-off softens its symptom.

Notes

No user-facing strings changed, so there is nothing to localize.

I could not build locally (no Swift toolchain available in my environment), so compilation and the test results rest on CI. Happy to fix up whatever it reports.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Keeps a collapsed sidebar group folded when the workspace beneath it closes by fixing the close-path selection fallback. Previously, closing the last workspace selected a hidden group member and auto-expanded the group; now it selects the group anchor so the group stays collapsed.

  • Extracts the selection fallback to WorkspacesModel.selectionTargetAfterClose(closedIndex:) (in WorkspacesModel+CloseSelection.swift) and calls it from TabManager's close path.
  • Maps a hidden member of a collapsed group to its anchor; expanded groups and non-last closes retain the prior selection behavior.
  • Adds targeted tests in CmuxWorkspacesTests and adds the package to the swift test allowlist so they run in CI; the list now lives in ci-macos.yml after merging main.

Written for commit 16af016. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Improved workspace selection after closing a workspace.
    • Closing the final workspace in a group now selects the appropriate visible workspace.
    • Closing a workspace preserves collapsed groups and selects the adjacent workspace when applicable.
  • Tests

    • Added coverage for workspace closing and selection behavior, including grouped and ungrouped workspaces.
    • Included the workspace package in the automated test suite.

AvoChang and others added 2 commits August 14, 2026 21:13
Closing the last workspace walks selection backwards into the tail of the
preceding group's member run. `tabs[]` holds every workspace, including
members a collapsed group hides, so the fallback can select a workspace
that has no sidebar row. That fires the selection auto-expand and unfolds
a group the user deliberately collapsed.

Lift the fallback out of TabManager.closeWorkspace into
WorkspacesModel.selectionTargetAfterClose with its behavior unchanged, so
the rule is reachable from the package's tests, and cover it.
closingLastWorkspaceSelectsAnchorNotHiddenMemberOfCollapsedGroup and
closingLastWorkspaceKeepsPrecedingGroupCollapsed fail on this commit; the
expanded-group and non-last-close tests pin the behavior that must not
change.

CmuxWorkspacesTests was not a CI gate — the package is absent from the
swift-test allowlist in ci.yml, so the whole suite never ran. Add it; its
dependencies are already headless-resolvable there.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Resolve the close-path selection slot to a row the sidebar renders: a
non-anchor member of a collapsed group now hands off to its group's
anchor instead of being selected directly, so the selection auto-expand
no longer unfolds the group.

Members of an expanded group have their own rows and are still selected
as before.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 14, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

Next included review available in 1 minute.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: fa3e8744-ae5d-4422-b81c-8d6b0af13fab

📥 Commits

Reviewing files that changed from the base of the PR and between 313b417 and 16af016.

📒 Files selected for processing (1)
  • Sources/TabManager.swift

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8c96736e-a10d-401e-b2ca-bfc82b129623

📥 Commits

Reviewing files that changed from the base of the PR and between 6cf7f5f and 313b417.

📒 Files selected for processing (4)
  • .github/workflows/ci.yml
  • Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Model/WorkspacesModel+CloseSelection.swift
  • Packages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCloseSelectionTests.swift
  • Sources/TabManager.swift

📝 Walkthrough

Walkthrough

Workspace closing now calculates replacement selection in WorkspacesModel. Collapsed groups resolve to their visible anchor. Tests cover grouped and ungrouped workspace cases, and CI runs the package tests.

Changes

Workspace close selection

Layer / File(s) Summary
Selection target resolution
Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Model/WorkspacesModel+CloseSelection.swift, Sources/TabManager.swift
WorkspacesModel calculates the replacement workspace after closure. TabManager uses the calculated target when the closed workspace was selected.
Selection behavior validation
Packages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCloseSelectionTests.swift, .github/workflows/ci.yml
Tests cover collapsed and expanded groups, final and non-final workspace closure, and preservation of collapsed state. CI includes CmuxWorkspaces in the Swift package test loop.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: lawrencecchen, austinywang

Merge Risk: ⚪ Minimal · up to 313b4

This PR localizes workspace-close selection to visible sidebar rows and adds regression coverage plus CI inclusion; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the primary fix: preserving a collapsed sidebar group when the workspace below it closes.
Description check ✅ Passed The description clearly explains the problem, cause, fix, tests, and CI change, but it omits the template's Demo Video, review trigger, and checklist sections.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed The new method extends explicit @MainActor WorkspacesModel and is called from @MainActor TabManager.closeWorkspace; no Sendable or background-context access was added.
Cmux Swift Blocking Runtime ✅ Passed The production diff adds only deterministic workspace/group selection logic and delegates to it; added Swift lines contain no semaphore, wait, sleep, timer, polling, sync, or lock primitive.
Cmux Browser Automation Off-Main ✅ Passed The diff changes CI, workspace close-selection code/tests, and TabManager only; no browser socket automation or policy-target files changed.
Cmux Expensive Synchronous Load ✅ Passed The diff only replaces in-memory close-selection logic; the existing agent-index load is byte-for-byte unchanged, and the new method scans tabs/groups without disk or JSON I/O.
Cmux Cache Substitution Correctness ✅ Passed The diff changes only transient workspace selection and group-row resolution; it adds no cache substitution in persistence, history, undo, or snapshot paths. The existing history cache read is unch...
Cmux No Hacky Sleeps ✅ Passed The PR changes only Swift workspace logic/tests and a GitHub Actions allowlist entry; no covered TypeScript, JavaScript, shell, or build/runtime delay code is introduced.
Cmux Algorithmic Complexity ✅ Passed The changed production path uses indexed tabs access plus one linear workspaceGroups.first(where:) scan; it adds no loop, nested scan, batch rescan, sorting, or filtering.
Cmux Swift Concurrency ✅ Passed The PR adds a synchronous selection helper and @MainActor tests; the changed Swift diff adds no Dispatch queues, Tasks, Combine state, completion handlers, or fire-and-forget work.
Cmux Swift @Concurrent ✅ Passed The PR adds only synchronous selection logic and calls it from @MainActor TabManager; no nonisolated async work, @concurrent annotation, or heavy async UI call was introduced.
Cmux Swift Package Boundaries ✅ Passed The new selection logic is in the CmuxWorkspaces SwiftPM target and has package tests; Sources/TabManager.swift only delegates during app close lifecycle, so no boundary violation is introduced.
Cmux Swiftpm Lockfiles ✅ Passed The PR diff only adds CmuxWorkspaces to the SwiftPM test list; it changes no Package.swift, Package.resolved, .gitignore, or Xcode package references.
Cmux Swift Logging ✅ Passed The PR diff adds no print, debugPrint, dump, NSLog, ad hoc logging, Logger, or sensitive-data logging; changed Swift code only updates selection behavior and tests.
Cmux User-Facing Error Privacy ✅ Passed The diff adds workspace-selection logic, tests, CI configuration, and comments only; it adds no user-facing errors, alerts, command output, or recovery copy.
Cmux Full Internationalization ✅ Passed The PR diff adds workspace-selection logic, tests, CI coverage, and developer comments/fixtures only; it adds no user-facing text, localization keys, catalogs, or locale data.
Cmux Swiftui State Layout ✅ Passed The PR diff adds model/test logic and a close-event selection assignment only; it introduces no new SwiftUI state, geometry, lazy-row store reference, or render-time mutation.
Cmux Architecture Rethink ✅ Passed The diff adds a local WorkspacesModel selection invariant and one TabManager call path; it adds no sleeps, polling, locks, observers, side channels, or split UI lifecycle ownership.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR changes workspace selection, TabManager close behavior, tests, and CI. It adds no standalone NSWindow, NSPanel, SwiftUI Window, identifier, or close-shortcut routing.
Cmux Source Artifacts ✅ Passed All four changed paths are intentional Swift source, tests, or CI configuration; the diff adds no artifact directories, binary files, logs, caches, or build output.
Cmux No Test Or Debug Seam In Production Source ✅ Passed No test/debug guard or seam-named member was added; selectionTargetAfterClose is production behavior called by Sources/TabManager.swift, while test scaffolding remains under Tests/.
Cmux No Ambient Global State ✅ Passed The new API is an instance method on WorkspacesModel at WorkspacesModel+CloseSelection.swift:24, called via workspaces; the diff adds no global var, static namespace, or singleton.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

The swift test package list moved from ci.yml to ci-macos.yml on main,
so take main's ci.yml as is.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Merged main in and resolved the ci.yml conflict so this can land (main moved the package test list out of ci.yml, so that line is dropped here and we'll add CmuxWorkspaces to CI separately). One last thing: the CLA check needs your signature, the CLA Assistant comment has the link. Thanks for the really clear write-up :)

@AvoChang

Copy link
Copy Markdown
Contributor Author

I have read the CLA Document v2.2 and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request Sep 26, 2026
@teamleaderleo
teamleaderleo merged commit 45815f7 into manaflow-ai:main Sep 26, 2026
63 of 64 checks passed
@teamleaderleo

Copy link
Copy Markdown
Collaborator

Merged, thanks @AvoChang :)

@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for 16af01629e: every check was green at merge (17 verified; 16 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 26, 2026
2123231 fix(cloud): offer Pi in the cloud agent menu and vm.cloud_agent_open (manaflow-ai#14819)
909fcc7 fix(settings): stop promising a Tailscale QR the pairing window no longer shows (manaflow-ai#14817)
5d2bc04 fix(custom-sidebar): render Menu nodes so context-menu submenus appear (manaflow-ai#14808)
45815f7 Keep a collapsed sidebar group folded when the workspace below it closes (manaflow-ai#10169)
4bf0ea0 perf(shell): stop spawning tmux and rm on every prompt when idle (manaflow-ai#14833)
443d050 perf: skip the per-flush stat and mkdir in the event log writer (manaflow-ai#14828)
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.

2 participants