Repository navigation
feat(sidebar): nest project groups from the group menu - #26194
Open
LINDECKER-Charles wants to merge 6 commits into
Open
LINDECKER-Charles wants to merge 6 commits into
LINDECKER-Charles wants to merge 6 commits into
Conversation
Folder imports already create nested groups and the sidebar renders them, but nothing let a user build or change that hierarchy by hand. Group headers now offer "New subgroup" and "Move to group" (capped at five levels for manual changes), and the project "Move to group" submenu shows the tree indented. Moves are validated on the host that owns the group (no self, descendant, cross-host, or too-deep parents). Clients only send a move to remote hosts that advertise projectGroup.nesting.v1, because older hosts drop the field and still return the unmoved group. Refs stablyai#8785
stablyai#8785 suggested two or three levels: deeper than that, the indentation eats the sidebar width. Folder imports stay exempt, and imported trees deeper than the cap can still be moved shallower.
LINDECKER-Charles
marked this pull request as ready for review
October 7, 2026 11:31
Contributor
|
Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 30.65% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 62 functions across 40 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Title check ✅ Passed The title clearly and concisely describes the main change: adding nested project groups through the sidebar menu. Description check ✅ Passed The description covers the required sections with clear user-facing changes, rationale, linked issue, visual proof, testing details, and review notes. It is detailed and mostly complete. Linked Issues check ✅ Passed Issue #8785requests manual subgroup creation and changing a group’s parent, with a suggested depth cap. The PR adds “New subgroup” and group “Move to group” menus, including a top-level destination, …Out of Scope Changes check ✅ Passed The changes support nested sidebar groups. Owner-host routing and host validation enable subgroup creation and group moves. Cycle handling and host-scoped parent resolution support rendering the neste…
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
ba6e4437-3e39-40a4-affe-9a20ecdcecf6
📒 Files selected for processing (46)
src/main/ipc/repos-execution-host-catalog.test.tssrc/main/ipc/repos/repo-ipc-arg-schemas.tssrc/main/persistence-project-group-nesting.test.tssrc/main/persistence/tracking-repos/project-group-operations.tssrc/main/runtime/rpc/methods/repo.test.tssrc/main/runtime/runtime-project-group-controller.tssrc/preload/api/repository-api.tssrc/renderer/src/components/sidebar/ProjectGroupDeleteDialog.test.tsxsrc/renderer/src/components/sidebar/ProjectGroupDeleteDialog.tsxsrc/renderer/src/components/sidebar/ProjectGroupMoveSubmenu.tsxsrc/renderer/src/components/sidebar/WorktreeContextMenuView.tsxsrc/renderer/src/components/sidebar/WorktreeList.tsxsrc/renderer/src/components/sidebar/project-group-move-targets.test.tssrc/renderer/src/components/sidebar/project-group-move-targets.tssrc/renderer/src/components/sidebar/worktree-list-groups-nested-project-groups.test.tssrc/renderer/src/components/sidebar/worktree-list/grouping/project-group-sections.tssrc/renderer/src/components/sidebar/worktree-list/rows/ProjectGroupDialogs.tsxsrc/renderer/src/components/sidebar/worktree-list/rows/SectionHeader.tsxsrc/renderer/src/components/sidebar/worktree-list/rows/project-group-header-actions.test.tsxsrc/renderer/src/components/sidebar/worktree-list/rows/project-group-header-actions.tsxsrc/renderer/src/components/sidebar/worktree-list/rows/repo-header-project-actions.tsxsrc/renderer/src/components/sidebar/worktree-list/rows/use-project-group-dialogs-owner-host.test.tsxsrc/renderer/src/components/sidebar/worktree-list/rows/use-project-group-dialogs.tssrc/renderer/src/components/sidebar/worktree-list/viewport/viewport-props.tssrc/renderer/src/components/sidebar/worktree-list/viewport/virtual-row-context.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/fr.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/renderer/src/store/project-groups/project-group-mutations.tssrc/renderer/src/store/project-groups/project-group-owner-update.tssrc/renderer/src/store/repos/repo-state.tssrc/renderer/src/store/slices/project-group-owner-routing.tssrc/renderer/src/store/slices/repos-project-group-nesting.test.tssrc/renderer/src/store/slices/repos.tssrc/shared/project-group-nesting-capability.tssrc/shared/project-group-nesting.test.tssrc/shared/project-group-nesting.tssrc/shared/project-group-types.tssrc/shared/project-groups.test.tssrc/shared/project-groups.tssrc/shared/protocol-version.tssrc/shared/rpc-contract/repo-params.tstests/e2e/project-group-nesting.spec.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
A create call that named another connection than the parent's stored the mismatch, so a later move under a valid group on the parent's host failed with host-mismatch. Reject it up front; an omitted or null connection still inherits the parent's.
Roughriver74
added a commit
to Roughriver74/orca
that referenced
this pull request
Oct 11, 2026
Upstream now routes project group catalog calls by their owner host (stablyai#27288): createProjectGroup takes the seeding project's host, and rows are adopted through adoptFromEndpoint. Subgroup creation moves to its own createProjectSubgroup action, routed by the parent group's owner like moveProjectGroup, and the update/move helper uses the new owner routing. protocol-version.ts re-exports the attention capabilities with a star, as stablyai#27493 did for the resume ones, to stay within the line limit.
Author
|
@brennanb2025 friendly ping on this one when you have a moment. I merged Since this comes from a fork, the CI workflows are still waiting for a maintainer to approve the run. |
Upstream renamed runtimeTargetForOwnerHostId to ipcOrRuntimeTargetForHostId and dropped the RuntimeClientTarget re-export (stablyai#27992), so createProjectGroup and the shared createOnTarget helper use the new name and import the type from shared. ProjectGroupingModel moved to shared/project-section-grouping (stablyai#28850). Locale conflicts were sibling additions; both sides are kept.
The workspace-list builder moved to src/shared/sidebar-list (stablyai#29906), and the sidebar now draws one header per project group id when two hosts publish the same group (stablyai#30128). The branch linked each host's copy to a parent on its own host, which assumed every copy was drawn; with copies collapsed, parents now resolve by id over the drawn groups, the same links reveal walks. Same-id copies from two hosts can still close a parent loop, so section emission and getProjectGroupOccurrencePaths share one parent map that cuts it with breakProjectGroupParentCycles. compareProjectGroupSiblings adopts the device-independent sidebar order from stablyai#29946 instead of localeCompare. createOnTarget moves to project-group-create-on-host.ts, and two repo RPC test expectations fold into one, to keep both files under max-lines.
Author
|
@brennanb2025 one follow-up since my last ping, no rush. I merged
|
Roughriver74
added a commit
to Roughriver74/orca
that referenced
this pull request
Oct 22, 2026
Roughriver74
added a commit
to Roughriver74/orca
that referenced
this pull request
Oct 22, 2026
…s C+E) Two distinct bugs in the project-group rebase stack (PR stablyai#26194 nest-groups layered on top of upstream's stablyai#30045/stablyai#30128 same-id-across-hosts fix): 1. appendProjectGroupSections() fed getDistinctProjectGroups()'s bare-id last-wins dedup straight into the per-host catalogGroupKey nesting walk. That dedup is correct for stablyai#30045's real scenario (one group literally republished under the same id to a managed server, so only one copy should render), but when two DIFFERENT duplicated ids cross-reference each other as parents across hosts (host A's copy of X parents host B's copy of Y, while host B's copy of Y parents host A's copy of X), the dedup silently drops an entire host's half of the hierarchy instead of looping. Added findDuplicatedIdsFormingParentCycles() to detect that specific cross-reference and exempt only those ids from the dedup, so every host keeps its own acyclic copy; ids with no such cross-reference keep the existing stablyai#30045 merge-to-one-copy behavior unchanged. 2. host-section-rows.ts's getRowHostId() pinned every project-group header to a concrete host id via getProjectGroupExecutionHostIdForRows(), which defaults to defaultHostId when a group has no executionHostId/ connectionId of its own. That conflated 'this group explicitly belongs to one host' with 'this group has no host and spans whichever hosts its repos live on' — so a catalog-wide group's header got bucketed into the default host's section only, instead of floating as a pending header repeated in every host section its member repos actually appear under. Added projectGroupHasExplicitExecutionHost() and only pin the header to a host when the group actually names one.
This branch has not been deployed
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
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.
ELI5
You can already put projects into groups, but only one level deep. This lets you put a group inside another group (for example a "Clients" group holding one group per client) straight from the group's
⋯menu: New subgroup creates one inside it, and Move to group moves an existing group under another one, or back to the top level.What Changed
Before: a group's
⋯menu only offered Rename and Delete. Nesting existed only when a folder import created it, so people encoded hierarchy in names ("acme / web", "acme / infra"). A project's Move to group submenu was a flat list, so two groups with the same name in different branches were indistinguishable.After:
Top level+ every valid parent, indented as a tree). The current parent is shown but disabled; the group itself and its own subgroups are never offered.Mechanism:
projectGroup.update(IPC + runtime RPC) accepts an optionalparentGroupId(null= top level). The host that owns the group validates every move insrc/shared/project-group-nesting.ts: no self, no descendant, no other host, no deeper than the cap. A manual subgroup inherits its parent's host (connectionId).projectGroup.nesting.v1. Otherwise it shows a "could not confirm the move" toast instead of pretending it worked. Creating a subgroup needs no gate, becauseprojectGroup.createhas acceptedparentGroupIdsince Add nested repo group imports #2866; the client also sends the parent'sconnectionIdso older hosts keep the subgroup on the right SSH target. This follows Rule 1 ofdocs/reference/remote-wire-compatibility.md.normalizeProjectGroupsnow breaks parent cycles in persisted data, since the sidebar renders from roots and a cycle would hide every group in it. The sidebar also resolves parent links inside each host's catalog, so equal group ids on two hosts can't link into a loop.Why
Folder imports already produce nested groups and the sidebar already renders them; the missing piece was a way to build or change that structure by hand (#8785).
MAX_PROJECT_GROUP_LEVELS). Folder imports stay exempt, and an imported tree that is already deeper can still be moved shallower, never deeper, so the cap never traps existing data.Prior work: #8939 (drag-and-drop, by the issue author) has had merge conflicts since July, and #19125 bundles a broader "client" model. This PR is the narrow, menu-only slice that can land on its own. Drag-to-nest from #8939 can build on top of it.
Linked Issue
Fixes #8785
Visual Proof
Headless Electron build, macOS, same seeded data on both sides (left:
mainat 2584ed9, right: this branch).Sidebar tree: groups can now nest.
Group
⋯menu: adds New subgroup and Move to group.Project → Move to group: an indented tree instead of a flat list.
New: Group → Move to group. Top level is offered, the current parent is shown but disabled, and the group's own subtree is hidden.
Testing
pnpm typecheck: passes.electron-vite build(the e2e build of the desktop app): succeeds.pnpm lint: every step that covers this change passes, includingverify:rpc-params-catalog,verify:localization-*,check:max-lines-ratchetand the changed-lines code-quality and design-system gate (0 new findings across 40 files). The only failure is 5 existingimport/no-cyclewarnings inmobile/files this PR doesn't touch.Unit tests (Vitest, Node) across every directory touched (
components/sidebar,renderer/store,shared,main/persistence,main/runtime/rpc,main/ipc,preload): 29,821 pass. The single failure,bounded-output-sink.test.ts, fails identically onmainand is unrelated.New tests:
shared/project-group-nesting.test.ts: depth, subtree span, move rejections, and the rule that imported deep trees can still move shallower.shared/project-groups.test.ts: parent-cycle breaking.main/persistence-project-group-nesting.test.ts: host-side validation, parent-host inheritance, and renames inside over-deep imported trees.main/runtime/rpc/methods/repo.test.ts,main/ipc/repos-execution-host-catalog.test.ts:parentGroupIdround-trip.store/slices/repos-project-group-nesting.test.ts: owner-host routing, capability gate, and the old host that ignores the move.sidebar/project-group-move-targets.test.ts,project-group-header-actions.test.tsx,use-project-group-dialogs-owner-host.test.tsx,ProjectGroupDeleteDialog.test.tsx,worktree-list-groups-nested-project-groups.test.ts: menu targets, tree order, host isolation, delete copy.tests/e2e/project-group-nesting.spec.ts(headless Electron, 4 tests passing): create a subgroup, cap at level 3, move in and out, own subtree never offered (and the submenu disappears when no valid parent is left).Platforms: macOS (unit + headless e2e). No platform-specific code: no shortcuts, paths or process spawning. SSH and remote behaviour is covered by the routing and capability unit tests above.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
AI Disclosure
AI coding agents were used for this PR. Claude (Anthropic, Claude Opus 5.5, via Claude Code) verified the change, rebased it onto
main, ran the checks and e2e tests, captured the screenshots, did the review below, and drafted this description. All of it was reviewed by me before submission.Review
AI review summary of the final diff:
PROJECT_GROUP_HEADER_INDENTconstant (inline style, because computed padding classes on menu primitives are lint-gated).projectGroup.nesting.v1capability gate, and an unreachable host or timeout ends in a "could not confirm" toast, never a false success.DropdownMenu*primitives and tokens, with no new colors or restyles (design-system gate: 0 findings). Long trees scroll inside the submenu (max-h-80). New strings go throughtranslate()with hashedauto.*keys, seeded in all six locales.parentGroupIdis validated with zod (min(1), nullable, optional) on both IPC and RPC, and existence, host and cycle checks run on the host before anything is persisted. There are no new filesystem, shell or network surfaces.Suggested reading order (28 source files, +816/−190; tests +1,178; locales +198):
src/shared/project-group-nesting.ts: the rules.src/main/persistence/tracking-repos/project-group-operations.ts: host-side enforcement.src/renderer/src/store/project-groups/project-group-owner-update.tsandproject-group-mutations.ts: routing and the capability gate.src/renderer/src/components/sidebar/project-group-move-targets.ts,ProjectGroupMoveSubmenu.tsxandproject-group-header-actions.tsx: the UI.Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Ensure no issues in: Security, Cross-platform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance. All of these are covered in Review above. Mobile has no group-editing UI; it only reads
parentGroupIdto resolve group subtrees (agent-history-resume-target.ts), which already handles imported nested trees and is cycle-safe.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)