Repository navigation
fix(sidebar): render folder workspaces under every Group by mode (#15362) - #15404
brennanb2025 merged 2 commits into
Conversation
) Folder-workspace rows were emitted in exactly one place, inside the repo-grouping branch of buildRows, so switching the sidebar to Status, PR status or None dropped them from every lane. Membership is now decided once, above the groupBy switch; a mode only chooses which lane a workspace lands in. Lane assignment is an exhaustive WorktreeGroupBy switch with no default clause, so a future grouping mode is a compile error here rather than a silent regression. Not a regression: the groupBy !== 'repo' gate predates folder workspaces by two weeks (#2866), and #5172 added folder-workspace emission below it, unreachable for non-repo grouping from its first commit. Also fixes four further paths where the same row kind went missing: - Flat mode gated its whole section on worktree count, so an account with only folder workspaces rendered nothing at all. - A collapsed folder-only lane header rendered globally instead of under its host section, because empty worktree lists yield undefined host counts. Host id maps now carry explicit empty arrays so the host fallback cannot leak global worktree ids. - Reveal resolved folder workspaces to project-group header keys only, so revealing one inside a collapsed lane never expanded it. Reveal now reuses the same lane function as grouping, expands the host header, and covers the agent-send path and the host-qualified row lookup. - The Clear Filters empty state ignored folder workspaces, so a folder-only account lost them whenever any filter was active. This one reproduced under Project grouping too. Behaviour deliberately unchanged: archived folder workspaces are still not filtered, matching current behaviour rather than making membership mode-dependent. Pinned folder workspaces still render in their natural lane rather than the Pinned section, now uniformly across modes.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughFolder workspaces now render in flat, workspace-status, PR-status, and project-grouped sidebar views. The change adds shared host resolution, lane metadata, sorting, row construction, filtering, collapsed-group expansion, reveal-key generation, and render-row lookup. Tests cover empty-state precedence, grouping behavior, host metadata, reveal keys, and folder workspace row matching. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/renderer/src/components/sidebar/folder-workspace-host-id.ts (1)
5-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShorten the new implementation-history comments.
Keep only the non-obvious invariant where a comment is necessary. Remove issue history and behavior that the code or test name already states.
src/renderer/src/components/sidebar/folder-workspace-host-id.ts#L5-L13: reduce this block to the resolver consistency invariant.src/renderer/src/components/sidebar/sidebar-empty-state-gate.ts#L1-L9: reduce this block to the empty-state rule.src/renderer/src/components/sidebar/sidebar-empty-state-gate.test.ts#L12-L14: remove the duplicated test rationale.src/renderer/src/components/sidebar/worktree-list/grouping/host-labels.ts#L117-L124: reduce this block to the folder-only host-section invariant.src/renderer/src/components/sidebar/worktree-list/grouping/host-labels.ts#L142-L149: reduce this block to the empty host-ID-array invariant.src/renderer/src/components/sidebar/worktree-list/navigation/render-row-lookup.ts#L112-L113: remove this comment because the branch states the behavior.src/renderer/src/components/sidebar/worktree-list/navigation/render-row-lookup.folder-workspace.test.ts#L39-L41: remove the duplicated test rationale.src/renderer/src/components/sidebar/worktree-list/listing/use-collapsed-groups.ts#L58-L59: reduce this block to the folder-workspace lookup invariant.As per coding guidelines, “Comments must be concise, non-obvious, and brief—prefer one line; do not explain obvious behavior or walk through code.”
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ffbf5cba-cb1f-4cba-be89-7dd77ac943af
📒 Files selected for processing (21)
src/renderer/src/components/sidebar/WorktreeList.tsxsrc/renderer/src/components/sidebar/folder-workspace-host-id.tssrc/renderer/src/components/sidebar/host-section-rows.tssrc/renderer/src/components/sidebar/sidebar-empty-state-gate.test.tssrc/renderer/src/components/sidebar/sidebar-empty-state-gate.tssrc/renderer/src/components/sidebar/worktree-list/grouping/build-rows.folder-workspace-lanes.test.tssrc/renderer/src/components/sidebar/worktree-list/grouping/build-rows.tssrc/renderer/src/components/sidebar/worktree-list/grouping/folder-workspace-lanes.tssrc/renderer/src/components/sidebar/worktree-list/grouping/group-keys.tssrc/renderer/src/components/sidebar/worktree-list/grouping/group-sections.tssrc/renderer/src/components/sidebar/worktree-list/grouping/host-labels.tssrc/renderer/src/components/sidebar/worktree-list/grouping/project-group-sections.tssrc/renderer/src/components/sidebar/worktree-list/grouping/project-grouping.tssrc/renderer/src/components/sidebar/worktree-list/grouping/row-builders.tssrc/renderer/src/components/sidebar/worktree-list/grouping/worktree-grouping.tssrc/renderer/src/components/sidebar/worktree-list/listing/use-collapsed-groups.tssrc/renderer/src/components/sidebar/worktree-list/navigation/folder-reveal.test.tssrc/renderer/src/components/sidebar/worktree-list/navigation/folder-reveal.tssrc/renderer/src/components/sidebar/worktree-list/navigation/pending-reveal-inputs.tssrc/renderer/src/components/sidebar/worktree-list/navigation/render-row-lookup.folder-workspace.test.tssrc/renderer/src/components/sidebar/worktree-list/navigation/render-row-lookup.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.
| if (owningGroup && options?.defaultHostId) { | ||
| keys.push( | ||
| `host:${getFolderWorkspaceHostId(folderWorkspace, owningGroup, options.defaultHostId)}` | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Honor ProjectGroup.executionHostId during folder workspace host resolution.
getFolderWorkspaceHostId only reads connectionId. It ignores ProjectGroup.executionHostId, which is the runtime-host stamp. A folder workspace in a runtime-host group without a connection ID resolves to the default host. Its row metadata and reveal key then target the wrong host section.
src/renderer/src/components/sidebar/worktree-list/navigation/folder-reveal.ts#L102-L105: Generate the host key from a resolver that honors the effective runtime host.src/renderer/src/components/sidebar/worktree-list/grouping/group-sections.ts#L107-L143: Ensure grouped lane host metadata uses that same effective host.src/renderer/src/components/sidebar/worktree-list/grouping/build-rows.ts#L148-L159: Ensure flat-mode host metadata uses that same effective host.src/renderer/src/components/sidebar/worktree-list/grouping/build-rows.folder-workspace-lanes.test.ts#L185-L245: Add a folder-only runtime-host case withexecutionHostId: 'runtime:...'.src/renderer/src/components/sidebar/worktree-list/navigation/folder-reveal.test.ts#L139-L146: Assert that reveal returns the matchinghost:runtime:...key.
📍 Affects 5 files
src/renderer/src/components/sidebar/worktree-list/navigation/folder-reveal.ts#L102-L105(this comment)src/renderer/src/components/sidebar/worktree-list/grouping/group-sections.ts#L107-L143src/renderer/src/components/sidebar/worktree-list/grouping/build-rows.ts#L148-L159src/renderer/src/components/sidebar/worktree-list/grouping/build-rows.folder-workspace-lanes.test.ts#L185-L245src/renderer/src/components/sidebar/worktree-list/navigation/folder-reveal.test.ts#L139-L146
Electron UI validation (head
|
| Mode (visible control) | Store groupBy |
Sidebar lane | Folder workspace visible |
|---|---|---|---|
| Project | repo |
QA Folder Group 15404 | yes |
| Status | workspace-status |
In progress | yes |
| PR (PR status) | pr-status |
In progress | yes |
| None | none |
All | yes |
Uncropped full-app screenshots (no repo commit):
Project
Status
PR
None
Final review status: ready to landReviewed the focused fix for #15362 through repeated same-model review/readiness passes, performance review, and established UI-state precedent. The closing readiness pass found no remaining P0/P1/P2 issues. Meaningful fixes pushed in
Validation:
No mobile, Windows/WSL, or SSH rig was required because this is a renderer-only sidebar grouping change with no platform, Git, or remote-wire behavior. The isolated dev harness needed a temporary Vite alias for an unrelated existing Verdict: land. |
…blyai#15362) (stablyai#15404) * fix(sidebar): render folder workspaces under every Group by mode (stablyai#15362) Folder-workspace rows were emitted in exactly one place, inside the repo-grouping branch of buildRows, so switching the sidebar to Status, PR status or None dropped them from every lane. Membership is now decided once, above the groupBy switch; a mode only chooses which lane a workspace lands in. Lane assignment is an exhaustive WorktreeGroupBy switch with no default clause, so a future grouping mode is a compile error here rather than a silent regression. Not a regression: the groupBy !== 'repo' gate predates folder workspaces by two weeks (stablyai#2866), and stablyai#5172 added folder-workspace emission below it, unreachable for non-repo grouping from its first commit. Also fixes four further paths where the same row kind went missing: - Flat mode gated its whole section on worktree count, so an account with only folder workspaces rendered nothing at all. - A collapsed folder-only lane header rendered globally instead of under its host section, because empty worktree lists yield undefined host counts. Host id maps now carry explicit empty arrays so the host fallback cannot leak global worktree ids. - Reveal resolved folder workspaces to project-group header keys only, so revealing one inside a collapsed lane never expanded it. Reveal now reuses the same lane function as grouping, expands the host header, and covers the agent-send path and the host-qualified row lookup. - The Clear Filters empty state ignored folder workspaces, so a folder-only account lost them whenever any filter was active. This one reproduced under Project grouping too. Behaviour deliberately unchanged: archived folder workspaces are still not filtered, matching current behaviour rather than making membership mode-dependent. Pinned folder workspaces still render in their natural lane rather than the Pinned section, now uniformly across modes. * fix(sidebar): count folder workspaces in host badges
…blyai#15362) (stablyai#15404) * fix(sidebar): render folder workspaces under every Group by mode (stablyai#15362) Folder-workspace rows were emitted in exactly one place, inside the repo-grouping branch of buildRows, so switching the sidebar to Status, PR status or None dropped them from every lane. Membership is now decided once, above the groupBy switch; a mode only chooses which lane a workspace lands in. Lane assignment is an exhaustive WorktreeGroupBy switch with no default clause, so a future grouping mode is a compile error here rather than a silent regression. Not a regression: the groupBy !== 'repo' gate predates folder workspaces by two weeks (stablyai#2866), and stablyai#5172 added folder-workspace emission below it, unreachable for non-repo grouping from its first commit. Also fixes four further paths where the same row kind went missing: - Flat mode gated its whole section on worktree count, so an account with only folder workspaces rendered nothing at all. - A collapsed folder-only lane header rendered globally instead of under its host section, because empty worktree lists yield undefined host counts. Host id maps now carry explicit empty arrays so the host fallback cannot leak global worktree ids. - Reveal resolved folder workspaces to project-group header keys only, so revealing one inside a collapsed lane never expanded it. Reveal now reuses the same lane function as grouping, expands the host header, and covers the agent-send path and the host-qualified row lookup. - The Clear Filters empty state ignored folder workspaces, so a folder-only account lost them whenever any filter was active. This one reproduced under Project grouping too. Behaviour deliberately unchanged: archived folder workspaces are still not filtered, matching current behaviour rather than making membership mode-dependent. Pinned folder workspaces still render in their natural lane rather than the Pinned section, now uniformly across modes. * fix(sidebar): count folder workspaces in host badges
…blyai#15362) (stablyai#15404) * fix(sidebar): render folder workspaces under every Group by mode (stablyai#15362) Folder-workspace rows were emitted in exactly one place, inside the repo-grouping branch of buildRows, so switching the sidebar to Status, PR status or None dropped them from every lane. Membership is now decided once, above the groupBy switch; a mode only chooses which lane a workspace lands in. Lane assignment is an exhaustive WorktreeGroupBy switch with no default clause, so a future grouping mode is a compile error here rather than a silent regression. Not a regression: the groupBy !== 'repo' gate predates folder workspaces by two weeks (stablyai#2866), and stablyai#5172 added folder-workspace emission below it, unreachable for non-repo grouping from its first commit. Also fixes four further paths where the same row kind went missing: - Flat mode gated its whole section on worktree count, so an account with only folder workspaces rendered nothing at all. - A collapsed folder-only lane header rendered globally instead of under its host section, because empty worktree lists yield undefined host counts. Host id maps now carry explicit empty arrays so the host fallback cannot leak global worktree ids. - Reveal resolved folder workspaces to project-group header keys only, so revealing one inside a collapsed lane never expanded it. Reveal now reuses the same lane function as grouping, expands the host header, and covers the agent-send path and the host-qualified row lookup. - The Clear Filters empty state ignored folder workspaces, so a folder-only account lost them whenever any filter was active. This one reproduced under Project grouping too. Behaviour deliberately unchanged: archived folder workspaces are still not filtered, matching current behaviour rather than making membership mode-dependent. Pinned folder workspaces still render in their natural lane rather than the Pinned section, now uniformly across modes. * fix(sidebar): count folder workspaces in host badges




ELI5
If you use a folder workspace on a multi-repo project group, it only showed up in the sidebar when "Group by" was set to Project. Switch to Status, PR status or None and it silently disappeared. Now it shows up in all of them.
What Changed
Folder-workspace rows were emitted in exactly one place — inside the repo-grouping branch of
buildRows— so every other Group by mode dropped them.Membership is now decided once, above the
groupByswitch; a mode only chooses which lane a workspace lands in, never whether it exists. Lane assignment is an exhaustiveWorktreeGroupByswitch with nodefault:clause, so adding a fifth grouping mode is a compile error here instead of silently re-introducing this bug.Placement per mode:
workspaceStatuslaneThe drop-site sweep found four more paths where the same row kind went missing. All are fixed here because this change makes folder-only non-repo results ordinary:
maintoday, including under Project grouping.Two behaviours are deliberately left alone, and both are pinned by tests:
Why
AGENTS.mdsays all changes must consider folder workspaces as well as git worktrees. This is one of the spots that predates that rule, and the shape of it is the reason it stayed hidden: membership was computed inside a mode branch, so each new grouping mode forgot the second workspace kind by default. Hoisting membership above the switch and making the mode switch exhaustive fixes the instance and the class.This is not a regression. The
groupBy !== 'repo'gate landed in #2866 (2026-05-27), two weeks before folder workspaces existed. #5172 (2026-06-11) then added folder-workspace emission below that gate, unreachable for non-repo grouping from its very first commit.workspaceStatushas been onFolderWorkspacesince day one and status grouping shipped 2026-05-14, so the data was always there — only the wiring was missing.Bisect hazard for reviewers:
build-rows.tslooks like August code, but that is only the #14486 module split out ofworktree-list-groups.ts.git log --followbottoms out at the split; the logic is byte-for-byte the June original.Linked Issue
Fixes #15362
Visual Proof
Outstanding — needs an Electron session. This is a real UI change and the template requires before/after. I could not capture it in this pass: Electron validation here runs through the
$electronskill, which is user-invoked only, so I could not self-drive a capture. Happy to attach a before/after the moment someone runs it.In the meantime the behaviour is pinned by unit-level evidence with proven pre-fix differentials, described below.
Testing
Four new/extended suites. What matters is not that they pass, but that each was run against pre-fix source and observed to fail — a test that cannot fail measures nothing:
build-rows.folder-workspace-lanes.test.tsfolder-reveal.test.ts(extended)render-row-lookup.folder-workspace.test.tssidebar-empty-state-gate.test.tsThe four intentionally-passing entries are guards, not evidence: the
repoarm of the parametrized test, the repo no-duplication guard, the host-filtered negative arm, and a pure lane-key assertion. They are labelled as such in the test file so nobody later mistakes them for acceptance coverage.Coverage includes: all four Group by modes; a folder workspace as the sole occupant of a status lane; a folder-only account in flat mode; ordering across both emitters (grouped lanes and the separate flat path); host placement for a collapsed folder-only lane; explicit empty
hostWorktreeIds; archived parity across all four modes; and no duplication under Project grouping.pnpm typecheck(web) and theoxlintcode-quality gate are clean locally.One honest caveat on the local suite: the full sidebar run reported 23 failing tests, but this machine was at load average 119–172 with other worktrees running vitest concurrently. Every named failure was re-run individually at
--maxWorkers=1and passed — includingWorktreeList.folder-workspace-rows.test.ts(5/5), the existing suite covering exactly these rows. Two others (WorktreeCard.lineage,WorktreeCard.pinned-repo-icon) were confirmed failing on pristinemainwith none of this branch's changes present, so they are pre-existing and untouched here. CI is the real gate.Review
Platform/compat notes: renderer-only change. No RPC params, stream opcodes, or published host content are touched, so there is no remote wire-compatibility surface. No new platform-dependent behaviour, no Git invocations, no provider-specific logic. SSH/remote folder workspaces are covered — the host resolver is now centralised in one function shared by header counting and reveal, so the two cannot drift.
Agent skill upstream boundary
Notes
Follow-ups deliberately not bundled here, to keep this focused:
emitPinnedGrouponly receives worktrees. Pre-existing and orthogonal.buildRows, well beyond this fix.getFolderWorkspaceHostIdignoresexecutionHostIdwhile the host filter resolver prefers it, so a runtime-hosted folder workspace passes the filter but buckets under the default host. Pre-existing; this change surfaces it in more modes without causing it.Checklist
pnpm typecheckand lint pass locally; CI covers the restAuthor