Skip to content

fix(sidebar): draw a project group once when two hosts publish it under one id - #30128

Merged
nwparker merged 5 commits into
mainfrom
nwparker/sidebar-group-dup-remote
Oct 17, 2026
Merged

nwparker merged 5 commits into
mainfrom
nwparker/sidebar-group-dup-remote

Conversation

@nwparker

@nwparker nwparker commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​168 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​167
Prod 8 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​30 $\color{#cf222e}{\Huge{\mathbf{−}}}$​8 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​22

ELI5

When you move a project to a managed server, the server gets a copy of the project's group with the same id, and this computer keeps its own copy. The sidebar drew a header for each copy, so the group showed up twice: one header with the project, one empty, and because both headers had the same row key they could pile up on top of each other. Now each group is drawn once.

What Changed

Before: after moving a grouped project to a managed server (local copy kept), with the server connected and the sidebar grouped by project, the group header appeared twice. The second was empty. Both had the row key hdr:project-group:<id>, so the virtual list could overlap them and leave ghost rows after quick updates (for example, removing several workspaces in a row). Cmd+J also listed the group twice.

After: one header for the group, holding the project's workspaces from both hosts. Cmd+J lists it once. Reveal (Cmd+J, jump to workspace) opens the parents of that same header.

Mechanism: a group id that two hosts publish now resolves to one copy everywhere the sidebar reads it: the last copy in the catalog, which is the copy every existing id-keyed lookup already used.

  • src/shared/project-groups.ts: new getDistinctProjectGroups, one row per id in first-seen order with the last copy winning.
  • src/shared/sidebar-list/project-group-sections.ts: builds group headers and their nesting from the distinct list, so a second copy no longer emits an empty header with the same key. getProjectGroupOccurrencePaths uses the same list.
  • cmd-j/palette-project-results.ts: lists groups from the distinct list, so search finds the name the sidebar shows.
  • reveal-ancestors.ts and folder-reveal.ts: walk the parents of the copy that is drawn. Folder-workspace reveal still opens the folder's own host section.
  • WorktreeList passes the host-filtered group list (the one the rows are built from) to reveal, so a host filter cannot make reveal open a hidden host's parent.

Why

Root cause: the sidebar fetches every host's groups into one list (fetchProjectGroupsForAllHosts), and a managed-server move copies a group to the server under the same id. Group membership in the row builder is keyed by id only (repo.projectGroupId), but appendProjectGroupSections emitted one header per catalog row. The first header took the project (groupByProjectGroupId.delete), the second was empty, and both headers had the same key.

Alternatives considered:

  • Draw one header per host (add the host to the key). That fixes the key collision but keeps the empty duplicate the user reported, because the project section is shared across hosts and can only sit in one header.
  • Give the copy a new id during the move. It does not help existing installs that already hold same-id copies, and the move code is being changed by other work.
  • Merge groups by name. Open PR fix(sidebar): render one project-group header per group across paired hosts #22023 does that for different-id groups from separate folder scans. That is a different cause; this PR only handles copies that share an id.

Linked Issue

Fixes #30045

Visual Proof

Real Orca dev app (background launch, isolated profile), with one project in a group that the local catalog and a runtime server both publish under the same id. Before uses main's project-group-sections.ts, after uses this branch's:

Before After
before-a.png after-a.png

Before: "Platform" twice, the second one empty. After: one "Platform" holding both hosts' workspaces.

Testing

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

  • New worktree-list-groups-same-id-host-groups.test.ts: one header holding the project; unique render keys; nesting of a same-id child drawn once; reveal paths follow the drawn copy when copies have different parents. These fail on main.

  • palette-results.test.ts: a same-id group is listed once, found by the name the sidebar shows.

  • folder-reveal.test.ts: folder reveal follows the drawn copy's parents. The existing host-owner case now lists the copies in the order where that host's copy is the drawn one, because the drawn copy decides which parents exist.

  • Ran src/renderer/src/components/sidebar (370 files), src/shared, cmd-j, pnpm tc, and the static-analysis job steps from pr.yml locally.

  • Verified live, on macOS only: the rendered sidebar in the real app with a seeded two-host catalog, before and after (screenshots above). Not verified live: a real managed-server move. The seeded state matches the on-disk data in the issue (same group id on both hosts).

Review

Codex review (gpt-6.1-sol, high), four rounds. Each finding was fixed:

  1. Cmd+J kept the first copy while the sidebar drew the last. Both now use getDistinctProjectGroups.
  2. Group reveal and occurrence paths resolved the first copy. They now use the drawn copy.
  3. Folder-workspace reveal walked only its host's copy. It now walks the drawn hierarchy and keeps the host section key.
  4. Reveal read the unfiltered catalog while rows used the host-filtered one. Reveal now gets the filtered list.

Final round: no findings.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Overlap: #29877 (open, conflicting) changes which row places a multi-host project inside appendProjectGroupSections. It touches the same function, but its rule is independent of this one. #29863 (open) filters Move to group by host. #22023 handles different-id groups that share a name. SSH and folder workspaces: folder workspaces in a same-id group render under the single header with their own host identity, and the host section key used by reveal is unchanged. No wire change.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

…er one id

Moving a project to a managed server copies its project group there under the same id while the desktop keeps its copy. The row builder emitted a header per copy, so the second sat empty and both shared one virtual-row key.

Fixes #30045
@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

📝 Walkthrough

Project-group section construction and search now use one group per distinct ID. The sidebar passes its visible project groups to pending reveal logic. Folder reveal checks the filtered group set for the owning group, then traverses ancestors using all project groups. Tests cover duplicate group headers, search results, row keys, and reveal paths for groups shared across hosts.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: 🟡 Moderate · up to 2874f

If two hosts publish the same project groups with conflicting parent relationships, the affected groups and their projects can disappear from the sidebar. The rest of the change looks sound, but this edge case should be handled or accepted before merge.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 11 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary fix: preventing duplicate project-group rendering when multiple hosts publish the same group ID.
Description check Passed The description is complete and focused. It includes the required explanation, change details, rationale, linked issue, visual proof, testing, review notes, compatibility considerations, and checklist…
Linked Issues check Passed Direct issue [#30045] requires the repo-grouped sidebar to stop emitting duplicate same-ID headers and to preserve correct navigation behavior. getDistinctProjectGroups keeps one group per ID with t…
Out of Scope Changes check Passed The changes stay within [#30045]. The shared distinct-group helper, sidebar section changes, Cmd+J alignment, host-filtered reveal propagation, and reveal tests all support consistent rendering and na…
Full details: Description check

Explanation

The description is complete and focused. It includes the required explanation, change details, rationale, linked issue, visual proof, testing, review notes, compatibility considerations, and checklist. The AI Disclosure section is omitted, which is acceptable if the author is an internal contributor.

Full details: Linked Issues check

Explanation

Direct issue [#30045] requires the repo-grouped sidebar to stop emitting duplicate same-ID headers and to preserve correct navigation behavior. getDistinctProjectGroups keeps one group per ID with the last catalog copy. appendProjectGroupSections and getProjectGroupOccurrencePaths use that list, so the sidebar emits one header and one occurrence path. Cmd+J uses the same list. Reveal logic uses the drawn copy and the host-filtered group list. Added tests cover one header, unique row keys, nested groups, Cmd+J results, and reveal paths.

Full details: Out of Scope Changes check

Explanation

The changes stay within [#30045]. The shared distinct-group helper, sidebar section changes, Cmd+J alignment, host-filtered reveal propagation, and reveal tests all support consistent rendering and navigation for same-ID groups across hosts. No unrelated product behavior or unrelated files appear in the pull-request diff.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: bd9a5997-40ad-4685-8488-b03a845e5139
📥 Commits

Reviewing files that changed from the base of the PR and between d9a6143 and 2874fbb.

📒 Files selected for processing: 11
  • src/renderer/src/components/cmd-j/palette-project-results.ts
  • src/renderer/src/components/cmd-j/palette-results.test.ts
  • src/renderer/src/components/sidebar/WorktreeList.tsx
  • src/renderer/src/components/sidebar/worktree-list-groups-same-id-host-groups.test.ts
  • src/renderer/src/components/sidebar/worktree-list/navigation/folder-reveal.test.ts
  • src/renderer/src/components/sidebar/worktree-list/navigation/folder-reveal.ts
  • src/renderer/src/components/sidebar/worktree-list/navigation/reveal-ancestors.ts
  • src/renderer/src/components/sidebar/worktree-list/viewport/VirtualizedWorktreeViewport.tsx
  • src/renderer/src/components/sidebar/worktree-list/viewport/viewport-props.ts
  • src/shared/project-groups.ts
  • src/shared/sidebar-list/project-group-sections.ts

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

const childGroupsByParentId = new Map<string | null, ProjectGroup[]>()
for (const group of projectGroups) {
// Why distinct: membership is id-keyed, so a same-id copy's header would sit empty (#30045).
for (const group of getDistinctProjectGroups(projectGroups)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Keep conflicting host trees from hiding grouped projects.

If same-ID copies from two hosts interleave, selecting the last copy of each ID can create a parent cycle. For example, local A is a root and local B is its child, while remote B is a root and remote A is its child. In the order [local A, remote B, remote A, local B], this loop selects A → B → A. Neither group reaches the root traversal at Line 132. The fallback at Lines 137-139 then skips their grouped repos because both IDs exist. Resolve conflicting parents or detect cycles before building the tree so those projects remain visible.

@nwparker
nwparker merged commit 965f881 into main Oct 17, 2026
33 checks passed
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.

[Bug]: Sidebar shows a project group twice/overlapping when a remote runtime shares the group id with local (after orcad migration)

1 participant