[manuaudio]

Closes #13071.

Summary

Removing a project from the sidebar can no longer destroy a different host's project.

When a repo id exists on two hosts and the owning host's row has dropped out of the renderer catalog, the sidebar remove flow resolved the id to whichever row was left — the other host's live one. Removal then killed that host's PTYs, purged its worktrees and dropped its row, while the orphan rows it was supposed to clean up survived. You lose the working project and keep the broken one.

Root cause

findRepoForHost (store/slices/repo-host-identity.ts:45-47) has a unique-candidate fallback: when no hostId is supplied and exactly one row carries the id, it returns that row regardless of which host it belongs to.

removeProject already accepts options.hostId and already scopes its RPC target and local row deletion to the owning host — the sidebar callers simply never passed it, even though every one of them had the host in hand:

site	had	passed
RemoveFolderDialog.tsx:62	modal data	—
WorktreeList.tsx:6159	the full Repo	—
delete-worktree-flow.ts:224	target.hostId	—

ssh-host-remove-workspaces.ts:45-51 documents this exact hazard and defends against it with an explicit hostId; the sidebar flow predates that defense.

What changed

• delete-worktree-flow.ts — carries target.hostId into the modal, and resolves the display name with findRepoForHost instead of a bare repos.find.
• WorktreeList.tsx — handleRemoveProject carries getRepoExecutionHostId(repo).
• RemoveFolderDialog.tsx — reads the host from modal data, forwards it to removeProject, and resolves the SSH host label via findRepoForHost so the confirmation text names the row that will actually be removed.
• worktrees.ts — the orphaned per-workspace-env purge keeps each row's own host instead of mapping down to bare ids.

The host is narrowed out of modal data with normalizeExecutionHostId rather than cast, so a malformed value degrades to today's behavior instead of lying to the type system.

Screenshots

No visual change. The only user-visible difference in text is that the removal confirmation can now name the correct SSH host.

Testing

• pnpm lint
• pnpm typecheck
• pnpm test — 5 pre-existing failures, evidence below
• pnpm build
• Added or updated high-quality tests that would catch regressions

Rebased onto current main and re-run from scratch: pnpm lint, pnpm typecheck and pnpm build all clean, and 4331 tests across the touched trees (components/sidebar/, store/slices/) pass in 370 files.

On the full-suite failures

pnpm test exits 1 on this branch. Rather than assert that's unrelated, I ran a full-suite baseline on clean main and a second full run on the branch:

	clean main	branch, run 1	branch, run 2
check-root-directory-entries	3 fail	3 fail	3 fail
agent-exec-handler	2 fail	2 fail	2 fail
run-electron-vite-dev-web	pass	1 fail	pass
remote-runtime-shared-control-connection	pass	1 fail	pass
orchestration-adopted-run-binding	pass	1 fail	pass
ai-vault-session-worktree-map	pass	pass	1 fail

Only the first two are constant, and they reproduce identically on clean main, so they are local-environment artifacts rather than regressions: the root-directory guard shells out to a script using declare -A (bash 4+) while macOS ships bash 3.2, and agent-exec-handler asserts the spawned env contains all of process.env, which fails on any dev machine whose shell exports a var the handler overrides (e.g. GIT_ASKPASS).

The rest reshuffle between runs, which is the tell — a regression fails deterministically. Every one of them is a wall-clock or throughput budget, not a behavioural assertion:

• orchestration-adopted-run-binding — expected 0.107 to be less than 0.0887 (scaling ratio)
• ai-vault-session-worktree-map — expected 156.2 to be less than 150 (a 4% miss on a 150ms budget; runs in 25ms in isolation)
• remote-runtime-shared-control-connection — throughput under a stalled socket

Run 1 took 389s against the baseline's 294s because a full pnpm build had just finished on the same machine; run 2, unloaded, came in at 298s and cleared all three. Each passes in isolation on this branch.

None of these files import any of the six this PR changes — it is renderer-only, four files under components/sidebar/ plus one store slice. Flagging the pattern rather than burying it, in case the timing budgets are worth loosening on CI runners.

New regression test in delete-worktree-flow.test.ts models the issue's scenario directly: host A's row dropped from the catalog, host B's live row the only one left under the id. It asserts the modal carries hostId: 'ssh:host-a' and that the confirmation names host A's project — host B's name appearing is the tell that resolution went to the wrong row.

Verified non-blind: reverting the plumbing makes it fail with exactly the bug —

-     "displayName": "orca on host A",
-     "hostId": "ssh:host-a",
+     "displayName": "orca on host B",

On a full pnpm test run, the handful that don't pass are local-environment artifacts that reproduce on a clean checkout with this branch stashed: check-root-directory-entries ×3 (the guard shells out to a script using declare -A, and macOS ships bash 3.2) and agent-exec-handler ×2 (asserts the spawned env contains all of process.env, which fails on any dev machine whose shell exports a var the handler overrides, e.g. GIT_ASKPASS).

AI Review Report

Reviewed with Claude Opus 5 against the CONTRIBUTING checklist.

• Cross-platform (macOS/Linux/Windows) — no platform-dependent behavior touched: no path construction, no shell invocation, no accelerator or shortcut label, no metaKey/ctrlKey branch. This is identity plumbing between renderer modules, so there is nothing for a platform check to guard.
• SSH/remote/local — this is the SSH-relevant path. Local rows resolve to the local host id and behave as before; SSH rows now resolve to their own ssh:<target> host. removeProject's existing per-host RPC targeting and removeForHost branch are unchanged — they simply now receive the host the user actually meant. No wire format changes, so mixed client/host versions are unaffected.
• Folder workspaces — unchanged; a folder workspace's main worktree routes through the same isMainWorktree branch and now carries its host.
• Agent/integration/git provider — none touched. Nothing here is provider-specific.
• Performance — findRepoForHost replaces Array.find with a filter over the same list, on a user-initiated removal. Not a hot path.
• UI quality — no layout, token or component change.

What it flagged and I acted on: the first cut resolved the host with a cast out of modal data. That silently accepts a malformed value, so it now goes through normalizeExecutionHostId and degrades to the previous behavior instead.

Security Audit

Reviewed for input handling, command execution, path handling, auth, secrets, dependency and IPC risk.

• Destructive-operation targeting — this is the substance of the change. It narrows a destructive operation (PTY teardown, worktree purge, row deletion) to an explicitly identified host instead of an inferred one, and removes a path where a dropped catalog row causes an unintended teardown of a live project. Net reduction in blast radius.
• Input handling — the one new untrusted-ish input is the host id read out of modal data. It is narrowed with normalizeExecutionHostId, not cast, so a malformed value falls back rather than propagating into an RPC target.
• Command execution / path handling — none added. No new spawn, shell, or filesystem path is constructed.
• IPC — no new IPC surface. removeProject's existing channel and its hostId parameter are unchanged; this only populates a parameter that was already there and already validated on the other side.
• Auth, secrets, dependencies — untouched. No new dependency.

No follow-up needed for this PR. The related gap worth its own change is noted below.

Notes

On the worktrees.ts change. Flagged as possibly out of scope, so to be explicit: this is the same defect, not adjacent cleanup. #13071's title is "removeProject without a hostId can delete the wrong host's project". The orphaned per-workspace-env purge calls removeProject(repoId) with no host while iterating full Repo objects — so it hits that exact fallback and can purge another host's row. The issue body enumerates the three sidebar sites; this is a fourth caller of the same function with the same omission, and it had the host in hand. Leaving it out would fix the bug on three paths and leave it live on a fourth. Happy to split it into its own PR if you'd rather keep this one to the enumerated sites.

What I deliberately did not do. The issue also suggests making removeProject refuse the unique-candidate / focused-host fallbacks for removals. I left that alone because it would silently break group removal: selectProjectGroupRemovalTargets (store/slices/project-group-removal-targets.ts:26-30) iterates full Repo objects and pushes repo.id, discarding the host — so removeProject is legitimately called there with no host available. A blanket refusal would turn those calls into silent no-ops and strand the projects. That site is a fifth instance of the same defect class and deserves its own fix (carry the host through ProjectGroupRemovalTargets), but it changes a shared selector's shape and its callers, so it does not belong here. Glad to follow up with it, or to add the guard here once that site can supply a host.

X: @manuaudiomix

[coderabbitai]

📝 Walkthrough

Walkthrough

Project removal now carries the repository execution host ID through sidebar flows and confirmation dialogs. Repository and SSH lookups use host-aware resolution. Main worktree deletion passes the target host to the confirmation modal. Orphaned repository cleanup tracks host-qualified repository identities and passes each repository’s host ID to removeProject. Tests cover duplicate repository IDs across hosts.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR passes owning host IDs through the listed sidebar removal paths and uses host-aware lookup, with regression coverage for duplicate repository IDs.
Out of Scope Changes check	✅ Passed	The orphan purge change addresses the same host-scoping defect, and all modified files support issue #13071.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title clearly and concisely describes the main fix: removing a project on its owning host instead of another host's row.
Description check	✅ Passed	The description covers the required summary, screenshots, testing, AI review, security audit, and notes sections with detailed, relevant information.

---

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.}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cd6f3c7a-71ca-4d69-94b9-186f508bec99

📥 Commits

Reviewing files that changed from the base of the PR and between ce20a10 and 60dca470b504a4584be2685cfca398cc7d758e7d.

📒 Files selected for processing (5)

• src/renderer/src/components/sidebar/RemoveFolderDialog.tsx
• src/renderer/src/components/sidebar/WorktreeList.tsx
• src/renderer/src/components/sidebar/delete-worktree-flow.test.ts
• src/renderer/src/components/sidebar/delete-worktree-flow.ts
• src/renderer/src/store/slices/worktrees.ts

[manuaudio]

Adding my X handle per CONTRIBUTING: @manuaudiomix

[manuaudio]

This one is the same defect a level up from the fix, and you're right. removeProject became host-aware but purgedRepoIds still compared bare ids, so with one repo id on two destroyed SSH hosts the first purge would suppress the second host's orphan sweep and strand that row.

Fixed in 8d825a9915 by keying the set with getRepoHostIdentityForParts, the same composite identity the host-aware lookups already use.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[manuaudio]

Closing this as superseded by the broader root-cause fix in #26340, now merged through #26483. That change makes the host mandatory for every project removal, covers the orphan purge and group deletion, and closed #13071. This PR remains useful prior-art and is referenced in #26340, but rebasing it would duplicate the merged behavior.