sidebar: keep the selected row clear of the footer

[teamleaderleo]

Summary

In the AppKit workspace list, the selected row can end up behind the sidebar footer (account, help, Upgrade), drawn under the buttons and faded by the bottom scrim:

scrollSelectedRowToVisibleIfNeeded checked the row against table.visibleRect and scrolled with scrollRowToVisible. Both count the strips under the scroll view's content insets as visible: the titlebar scrim on top and the footer overlay at the bottom. So when a reorder pushed the selected row down (here, another workspace jumped to the top when its notification arrived), the row sat behind the footer and was treated as already visible.

It now computes the area between the insets and scrolls by exactly the overlap, clamped to the list's scroll range (SidebarWorkspaceTableController.selectedRowScrollOrigin).

Testing

• New cmuxTests/SidebarSelectedRowScrollTests.swift: a row between the insets does not scroll; a row behind the footer scrolls up just enough; a row under the titlebar scrim scrolls down; both ends clamp. Running on CI (not locally, per the no-local-tests rule on this Mac).
• The screenshot above is from CI UI run 36319472969 before the fix.

Changelog

Fixed: The selected workspace no longer hides behind the sidebar footer after the list reorders.

🤖 Generated with Claude Code

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

---

Summary by cubic

Fixes the selected workspace row scrolling behind the sidebar footer after a reorder, such as when another workspace jumps to the top on a notification.

Bug Fixes

• Previously visibleRect and scrollRowToVisible counted the strips under the content insets as visible, so a selected row behind the footer never scrolled.
• The selection now scrolls fully between the top and bottom insets, clamped to the list's scroll range; rows taller than the clear area top-align instead.
• Adds tests for rows already visible, behind the footer, under the titlebar, tall rows, and clamping at both ends.

^{Written for commit e2384e6. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes
  • Selecting a workspace now scrolls it into the unobscured portion of the sidebar, keeping it clear of the title bar and footer.
  • Scrolling is adjusted for rows taller than the visible area and is limited at the top and bottom of the workspace list.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 7de96d3c-a2fb-4054-a11a-a7a94be37a5a

📥 Commits

Reviewing files that changed from the base of the PR and between 8efe28d and e2384e6.

📒 Files selected for processing (3)

• Sources/Sidebar/AppKitList/SidebarWorkspaceTableController.swift
• cmux.xcodeproj/project.pbxproj
• cmuxTests/SidebarSelectedRowScrollTests.swift

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

---

📝 Walkthrough

Walkthrough

Selected workspace rows are scrolled within the unobscured clip-view area. The scroll origin accounts for top and bottom insets, oversized rows, and document boundaries. Tests cover these cases.

Changes

Sidebar row scrolling

Layer / File(s)	Summary
Inset-aware selected-row scrolling Sources/Sidebar/AppKitList/SidebarWorkspaceTableController.swift	The controller calculates scrolling from the selected row’s clip-view frame. It uses the unobscured area and clamps the origin to document bounds.
Scroll-origin tests and target wiring cmuxTests/SidebarSelectedRowScrollTests.swift, cmux.xcodeproj/project.pbxproj	Tests cover rows within and outside the insets, oversized rows, and scroll limits. The Xcode project adds the test file to the test target.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to e2384

No actionable issue remains identified for the selected-row scrolling change. Merge after normal checks.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 2 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (24 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main behavior change: keeping the selected sidebar row clear of the footer.
Description check	✅ Passed	The description is mostly complete. It explains the problem, implementation, testing scope, CI-only execution, changelog entry, and includes a screenshot for the UI behavior change. The template's Dem…
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 Cloud Persistent Session And Early Input	✅ Passed	PASS: The pull request changes only sidebar row scrolling, project registration, and sidebar scroll tests. The authoritative diff introduces no Cloud terminal creation, cmux-tui client, transport, PTY…
Cmux Swift Actor Isolation	✅ Passed	PASS: The production change remains inside the pre-existing @MainActor SidebarWorkspaceTableController, and its AppKit access runs through the main-actor controller method. The new pure `selectedR…
Cmux Swift Blocking Runtime	✅ Passed	PASS: The production diff replaces row-visibility logic with deterministic geometry calculations and AppKit scrolling. It adds no semaphore, blocking wait, sleep, delayed dispatch, polling loop, main-…
Cmux Browser Automation Off-Main	✅ Passed	PASS. The pull request changes only sidebar row scrolling, its tests, and Xcode project registration. The diff contains no browser socket commands, WebKit waits, mainActor/processV2Command routing, so…
Cmux Expensive Synchronous Load	✅ Passed	The production diff changes only selected-row geometry and scrolling. It adds no RestorableAgentSessionIndex.load(), agent store/history read, JSON/JSONL parsing, directory scan, per-record syscall,…
Cmux Cache Substitution Correctness	✅ Passed	PASS. The production diff changes transient AppKit scrolling behavior from visibleRect/scrollRowToVisible to inset-aware clip-view geometry. It does not replace a fresh persistence, history, undo,…
Cmux No Hacky Sleeps	✅ Passed	PASS. The authoritative diff changes only Swift source/tests and Xcode project metadata. The configured rule applies to TypeScript, JavaScript, shell, and non-Swift build/runtime scripts; Swift timing…
Cmux Algorithmic Complexity	✅ Passed	The production change performs one linear rows.firstIndex(where:) lookup over the workspace array, which is an appropriate O(n) design for the stated scale. The new selectedRowScrollOrigin method …
Cmux Swift Concurrency	✅ Passed	PASS — The Swift diff adds synchronous AppKit scrolling and a pure nonisolated geometry helper. It adds no DispatchQueue, Task, Combine state, completion-handler API, or fire-and-forget asynchro…
Cmux Swift @Concurrent	✅ Passed	PASS: The Swift diff adds only nonisolated static func selectedRowScrollOrigin, which is synchronous and pure, an explicitly allowed case. The changed caller remains synchronous and @MainActor-iso…
Cmux Swift Package Boundaries	✅ Passed	PASS. The production change is AppKit sidebar glue, not reusable domain logic. SidebarWorkspaceTableController remains a @MainActor NSObject/NSTableViewDelegate and the new helper is used only…
Cmux Swiftpm Lockfiles	✅ Passed	The PR changes only the sidebar controller, adds a Swift test, and adds that test to cmux.xcodeproj/project.pbxproj. The project-file additions are only a PBXFileReference and PBXBuildFile; no S…
Cmux Swift Logging	✅ Passed	The changed production Swift code only replaces row-scrolling logic and adds comments and a helper. The added test contains no logging. The diff adds or materially changes no print, debugPrint, `d…
Cmux User-Facing Error Privacy	✅ Passed	PASS: The production change only adjusts sidebar row scrolling. Added text is limited to developer comments and test names/comments about the footer and scrim. No user-facing error, alert, command out…
Cmux Full Internationalization	✅ Passed	The pull request changes scrolling geometry in SidebarWorkspaceTableController and adds tests plus Xcode project wiring. The changed Swift lines add no user-facing text, and the added English phrase…
Cmux Swiftui State Layout	✅ Passed	PASS: The pull request does not introduce a SwiftUI state or layout pattern covered by the check. The changed source is an AppKit SidebarWorkspaceTableController and an AppKit test; the added behavi…
Cmux Architecture Rethink	✅ Passed	PASS. The diff is a small local correctness fix in the existing @MainActor SidebarWorkspaceTableController. It replaces AppKit's broad visibility check with a pure selectedRowScrollOrigin calcul…
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PASS: The PR changes SidebarWorkspaceTableController scrolling logic and adds a test-only SidebarSelectedRowScrollTests fixture. The diff adds no NSWindow, NSPanel, NSWindowController, Swift…
Cmux Source Artifacts	✅ Passed	All three changed paths are intentional source-control entries: one Swift product source file, one Xcode project configuration update, and one checked-in Swift test file. The patch adds no logs, scree…
Cmux No Test Or Debug Seam In Production Source	✅ Passed	PASS. The production diff adds selectedRowScrollOrigin as a normal internal, nonisolated helper in Sources/Sidebar/AppKitList/SidebarWorkspaceTableController.swift. The production method `scrollSe…

Full details: Docstring Coverage

Explanation

Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

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

[github-actions]

CI failure attribution

CI passes on e2384e6fba (run 36337235292 attempt 1).

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's.

[github-actions]

CI fast guards passes on e2384e6fba (https://github.com/manaflow-ai/cmux/actions/runs/36337234990).

[github-actions]

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