Skip to content

fix(sidebar): place a runtime-stamped project group on its own host - #24363

Open
innocarpe wants to merge 2 commits into
stablyai:mainfrom
innocarpe:fix-13944-project-group-host
Open

innocarpe wants to merge 2 commits into
stablyai:mainfrom
innocarpe:fix-13944-project-group-host

Conversation

@innocarpe

Copy link
Copy Markdown
Contributor

Description

A project group that belongs to one Orca server was rendered under whichever server was focused. Runtime-owned groups usually carry executionHostId and no SSH connectionId. The sidebar filter already treated that stamp as the group's host, but placement did not, so the group (and its folder workspaces) jumped to the focused server.

Focused fix

In:

  • Folder-workspace placement now uses the same host resolver as the visibility filter, including a connectionless executionHostId.
  • A repo-less project-group header with an explicit runtime stamp or SSH connection is placed on that host.

Out:

  • Creating a project group on a chosen server.
  • Host filters, reveal, and pinned-folder features.

Preserves

Ungrouped, All, and Pinned headers, and any project group with neither stamp, stay in the pending buffer and are still copied onto each following host run. A header that already has a repo still follows that repo's host. SSH folder workspaces still follow their connection.

Evidence

node node_modules/vitest/vitest.mjs run --config config/vitest.config.ts --cache false src/renderer/src/components/sidebar/host-section-rows.runtime-group.test.ts src/renderer/src/components/sidebar/host-section-rows.test.ts

The new file pins three cases: a connectionless runtime group stays on its owner while another server is focused, a connectionless folder workspace follows that stamp, and an unstamped group header stays buffered across host runs. The existing host-section tests still pass.

User-regression-tradeoffs

A runtime-stamped group no longer follows the focused server. That is the reported bug. Groups with no host stamp keep the previous shared-header behavior.

Fixes #13944

@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Changes how project groups are assigned to host sections in the sidebar.

The PR appears safe to merge; no new actionable issue or outstanding finding remains.

Summary

The PR places runtime-stamped project-group headers and their folder workspaces under their owning host while preserving shared-header behavior for unstamped groups. Two added tests cover the filtered and unfiltered project views.

Reviews (2) · Last reviewed commit: "test(sidebar): cover a runtime project g..."

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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

🧰 Additional context used
📚 Code guidelines (1)
AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 1fe1d499-289d-41bf-9cb4-10a616bb0b28

📥 Commits

Reviewing files that changed from the base of the PR and between 82741b46819bcc34cdbd7fbcbc919df02249f96d and af9ecb1.

📒 Files selected for processing (1)
  • src/renderer/src/components/sidebar/host-section-rows.runtime-group.test.ts

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


📝 Walkthrough

Walkthrough

Folder workspaces and project-group headers now use host resolution that accounts for runtime host stamps and connection IDs. This changes where connectionless, stamped groups and their folder workspaces appear in host sections. Tests cover stamped groups under their owner host, folder workspaces under the stamped group host, and unstamped group headers across host runs.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to af9ec

Stamped project groups and their folder workspaces now appear under the server that owns them. Unstamped headers keep their previous behavior. The change is a small sidebar placement fix with tests, and no merge-blocking risk was found.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description clearly explains the bug, focused fix, scope, preserved behavior, testing, and linked issue. However, it does not follow the repository template and omits required sections such as ELI… Restructure the description using the repository template. Add the required headings and complete each section. Include visual proof or write N/A with a reason, document testing and platform coverage, complete the checklist, and address A…
Docstring Coverage ⚠️ Warning Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: placing runtime-stamped project groups on their owning host.
Linked Issues check ✅ Passed The changes satisfy #13944. getFolderWorkspaceHostId uses the shared execution-host resolver, so a connectionless folder workspace follows its project group's executionHostId. getHeaderHostId as…
Out of Scope Changes check ✅ Passed The changes stay within #13944. They modify sidebar host resolution and add focused automated tests. No group creation, host-filter, reveal, or pinned-folder feature changes are present.
Full details: Description check

Explanation

The description clearly explains the bug, focused fix, scope, preserved behavior, testing, and linked issue. However, it does not follow the repository template and omits required sections such as ELI5, What Changed, Why, Visual Proof or N/A, Testing checkboxes, Review, Notes, Agent skill upstream boundary, and Checklist.

Resolution

Restructure the description using the repository template. Add the required headings and complete each section. Include visual proof or write N/A with a reason, document testing and platform coverage, complete the checklist, and address AI Disclosure and Agent skill upstream boundary as applicable.

  • Fix all pre-merge checks with AI
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

@pullfrog pullfrog 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.

✅ No new issues found.

Reviewed changes

  • Folder-workspace placement now matches the visibility filter. getFolderWorkspaceHostId (src/renderer/src/components/sidebar/folder-workspace-host-id.ts) delegates to getFolderWorkspaceExecutionHostIdForRows, so an executionHostId-first stamp (including a connectionless runtime stamp) decides the section instead of falling back to the focused host.
  • Repo-less project-group headers get a host. New getHeaderHostId (src/renderer/src/components/sidebar/host-section-rows.ts) places a group header with an explicit runtime/SSH stamp on its owner host, while Ungrouped, All, Pinned, and groups with neither stamp stay pending-buffered (return null); repo headers still resolve through getRepoHostId.
  • Regression tests added. host-section-rows.runtime-group.test.ts pins three cases with exact row arrays; all three fail against the pre-fix buffering path, so they are real coverage rather than theatre.
  • Scope is broader than the title. project-group-catalog.ts stamps every fetched group via projectGroupWithFetchedOwner (local → local, SSH → ssh:<conn>, runtime → runtime:<env>), so in practice nearly every group header becomes host-owned now, not only runtime-stamped ones. The direction is correct — a local group no longer duplicates under remote host sections — and the unstamped branch is reserved for synthetic/transient groups.

Verification: the four host-section/filter files pass (33 tests), the broader folder-workspace/reveal/host-label/grouping set passes (71 tests), the full src/renderer/src/components/sidebar suite passes (2698 tests), tsc -p config/tsconfig.tc.web.json is clean, and check-changed-code-quality.mjs reports 0 new findings.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

innocarpe and others added 2 commits October 1, 2026 22:04
A connectionless executionHostId is the group's host. Placement still
fell back to the focused server, so the group moved when focus changed.
Visibility already honored the stamp.

Fixes stablyai#13944
The host-section cases skipped the project-grouping flags the sidebar passes, so they could pass without the filtered view placing the group under its owner.

Co-authored-by: Cursor <cursoragent@cursor.com>
@innocarpe
innocarpe force-pushed the fix-13944-project-group-host branch from 82741b4 to af9ecb1 Compare October 1, 2026 13:08
@innocarpe

Copy link
Copy Markdown
Contributor Author

Sync update (af9ecb1a32)

The runtime-group cases now call host sectioning the way the sidebar does. An explicit two-host filter still places the stamped group under its owner, and the unfiltered projects view leaves those rows without host headers.

@pullfrog pullfrog 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.

✅ No new issues found.

Reviewed changes

This run re-reviewed the incremental delta on the PR branch (test-only changes since the prior pullfrog review; production resolver code is unchanged).

  • Added renderedHostFilter test helper matching the production call. It passes visibleWorkspaceHostIds and preferProjectGrouping: true, mirroring use-section-rows.ts:220, so the new cases exercise the path the sidebar actually uses rather than the bare resolver.
  • Added a filtered-both-hosts placement case. Confirms a connectionless runtime-stamped group stays under its owner host when both hosts are visible. This is real coverage: with getHeaderHostId reverted, the unbuffered group duplicates under every host run, so the assertion fails against the pre-fix code.
  • Added an unfiltered-projects-view case. Confirms host headers are not inserted when preferProjectGrouping is set and no host filter is active — it exercises the early pass-through return at host-section-rows.ts:205-211. This one is a guard, not bug coverage (it passes before and after the fix).

This directly addresses the prior greptile review thread on the test helper; that thread is now resolved. The new test file passes (5/5) and pnpm tc:web is clean.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

This branch has not been deployed

No deployments
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.

Projects sidebar: project group renders under whichever Orca server is focused, not its own host section

1 participant