Repository navigation
fix(workspaces): delete on the confirmed host, and make both hosts' rows selectable (STA-4343) - #15013
Conversation
|
Too many files changed for review (242 files, 100 file limit). Bypass the limit by tagging |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Walkthrough<hidden_new_layer> Additional CLI, runtime, renderer, cleanup, persistence, routing, activation, and host-collision tests validate host-qualified worktree behavior.range_bf05ae654400 range_b889cb2c730c range_55bbfa789537 range_9a5f49722ac1 range_1e977a7bab5c range_28bcb84a1ed2 range_aee3c2b3e0b0 range_a68f6948c40a range_0cbe0a220768 range_29ef6399dedf range_7226fe5b7974 range_8cf7797ba0a6 range_b2aad33d3dd8 range_cb7383f6e5c4 range_7d598730361a range_be3e96bb55f8 range_fcc16f0c4d54 range_49e0d9c8ddf6 range_70fdc1e4a3cd range_5faa6faaadcb range_b8bdffd83a80 range_513f1afbf758 range_0b39ce6429be range_bff0a44e49cd range_7f8034387799 range_be182453d60f range_5ed80e5d3c89 range_a3f6b7e40fb5 range_23d90fc0db4d range_da4af0471e91 range_d705cdf44650 range_46497f0c90de range_42c6d3f18b69 range_b21d362fc746 range_6516d6012163 range_36fc89821e88 range_1386eb30d902 range_bdbfab774c0a range_62d8cf599546 range_9ad7b2ef0173 range_62084e43ed04 range_a2133d6aaf2e range_b92569e96df1 range_d20ef9de0b10 range_8df08273dab5 range_4fd892422fdc range_8ae91a423352 range_e3d52398cfad range_7c9d8a7d0ee6 range_108a06402c3c range_3e7add45d17b range_c895498f101d range_371acdc58e9a range_d8b0c5100428 range_1c4d220c0d70 range_ccb94de74415 range_fe9c0ee9038d range_9ea9c5b7693f range_358d5b5f3036 range_fc6b88963818 range_0d30326a19ba range_31d3cf3a22bc range_cbc8c1772633 range_96a951fe46c7 range_7b84b8e1ee61 range_05b5243567e6 range_b84b8fec4123 range_3645c26ea6f0 range_12df1e215529 range_cc1ee3c7b872 range_0350f3cfc189 range_c2adc34a5f17 range_8006f3329492 range_16d60450fbf8 range_055968ecad10 range_834e34c84f01 range_9c354fc689d2 range_aa6ec406f857 range_40ea086f01a6 range_30574f8a40e1 range_27a4a054f2e9 range_b358e3078c93 range_0ad757a10120 range_a1cf9ac8ce08 range_16c49b59dc69 range_c8c6630b72a0 range_1ca49bcdf33c range_c8fce18d5390 range_d247a069d376 range_c96edd2fcdda range_5f0ab3f69e86 range_ed513d9bc843 range_20d08deea53b range_21c111baa9da range_26c7c747ab0a range_8dc7835b9302 range_206944b165ec range_5b4ff008381f range_a57237faaa55 range_e3c92009b1c4 range_489218fdb9d4 range_1026df8f4e59 range_fc6dbb2e650a range_de9d9516e8b9 range_cfaa45b5dbb1 range_4e81d8db4b69 range_1c026aa88d27 range_b98ced13b9c0 range_89600e4f5f90 range_0dac924237dd range_3686c6b54483 range_d1ec375d49bb range_b546f5edc803 range_9dca7b28b8ba range_cc9788c98ed6 range_20576152e133 range_6e7ef3ed831a range_75bad01244ee range_682523f68e07 range_6d95d6f679b8 range_f274e4c58fe7 range_b36adcf338a0 range_1a943578b695 range_2c2811eb73f1 range_c4cd079c4368 range_7229f289bde7 range_e5fede49a6a5 range_525a48dbe01c range_97be9f1cdeba range_64be14d5d1e2 range_493c328fe1b6 range_a028627a4c72 range_641dbdfefddb range_2e305127b6a9 range_a109936f14bd range_a21b619c01be range_bf07576caf46 range_eb5d26f2430a range_09cd5a4f3ab7 range_dc130c66557b range_38599cb51596 range_5513f7655cd2 range_1fd72d6014e0 range_b106198d4972 range_eebbca0ea64e range_d3c916726f56 range_5124d51d6d0d range_2b4eb0cb1081 range_136db7451519 range_44b01577cb2a range_7466aba7987a range_eefe410f091b range_40d782838c15 range_a041374cd591 range_78a724e1590c range_1c2063e223a6 range_1ee79c746550 range_4a3d5b812f3b range_747d486af14f range_13db48fdbbc8 range_4acc74016e35 range_b02baaaca1b9 range_182f2319247b range_60487c09f859 range_02ee001e380e range_1f989421d77e range_06f805cd496a range_d34b4f17c358 range_60932f018f04 range_1bb695f1bac4 range_b00799325b02 range_2ba0339d8cc0 range_e19b024329df range_5ce8569ed2f8 range_c953f2cd7033 range_be2af656dc9d range_f84796edfd80 range_b394b14aff3b range_638037dbade7 range_0d44811fb9cf range_5ae20bd86d7f range_2f51cede8108 range_b2cb4ac160d8 range_b620d2116652 range_b71d5c5259fa range_792b8305cacd range_1d821899b0d9 range_2dbe533f88cb range_9608e7965e4b range_5d2e16f1ca22 range_243bb51f2f92🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/renderer/src/components/sidebar/worktree-list/grouping/row-builders.ts (1)
211-226: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse host-qualified keys for lineage nesting and collapse.
When two parent rows share an ID,
childrenByParentId,visibleIds, andgetLineageGroupKey(worktree.id)merge host-specific state, whileemitteddistinguishes hosts. A child can render under the wrong parent, and collapsing one parent collapses both. Key lineage state by host-qualified parent identity and add a collision test with a child on one host that asserts correct nesting and independent collapse.src/main/runtime/orca-runtime.ts (3)
22429-22456: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPass host-scoped metadata to visibility detection.
isRuntimeWorktreeVisibleomitsprovidedMeta, sotoRuntimeDetectedWorktreefalls back to unscopedgetWorktreeMeta(worktree.id). On multiple hosts, ownership classification can use another host's metadata and produce an incorrect.visibleresult. Fetch metadata withgetRepoOwnedWorktreeMetaand pass it asprovidedMeta.
2334-2376: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPersist host ownership when initializing runtime folder workspaces
listRuntimeFolderWorkspacesdoes not persisthostId. After a second repo owner appears, the host filter rejects metadata initialized by this function. The forced root row then has no persisted metadata orinstanceId.Persist
hostId: expectedHostIdand initialize the root without bypassing the multi-host ownership rules.
25556-25596: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftScope non-ID selectors by
requiredHostIdbefore removal.branch:,path:,name:, and bare selectors use unscopedresolveWorktreeSelector, which can return another host's path whilecleanupHostIdselects a different repo/provider. Reject host mismatches before any destructive operation.src/renderer/src/components/sidebar/worktree-delete-request.ts (1)
43-72: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy liftReject hostless identities before destructive resolution.
Line 64 resolves a hostless request through the first row for that ID. Line 87 converts an invalid
hostIdinto that same hostless request. When two hosts own the ID, this can delete the other host's checkout.Fail closed when a deletion identity has no valid
hostId. Updatesrc/renderer/src/components/sidebar/worktree-delete-request.test.tsLines 96-100 to expect rejection instead of first-wins routing.Proposed direction
- const target = lookupTarget(worktreeId, typeof request === 'string' ? undefined : request.hostId) ?? null + if (typeof request === 'string' || request.hostId === undefined) { + return null + } + const target = lookupTarget(worktreeId, request.hostId) ?? nullAlso applies to: 75-90
🧹 Nitpick comments (13)
src/main/worktree-removal-repo-owner.test.ts (1)
70-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the
as neverstore casts with a typed metadata-store helper.The metadata store literals already supply
getReposandgetWorktreeMeta, so they satisfy the parameter type without a cast.as neverdisables type checking on these call sites. If theresolveWorktreeRemovalMetadatasignature changes, these tests will still compile and the contract drift will pass unnoticed. A typed helper also validateshostId: 'local'againstExecutionHostIdinstead of accepting an arbitrary string.♻️ Proposed typed metadata store helper
+function makeMetadataStore( + repos: readonly Repo[], + meta: WorktreeMeta | undefined +): Parameters<typeof resolveWorktreeRemovalMetadata>[0] { + return { getRepos: () => repos, getWorktreeMeta: () => meta } +} + it('does not borrow destructive metadata from a colliding host', () => { - const localMeta = { hostId: 'local', preserveBranchOnDelete: true } - const metadataStore = { - getRepos: () => [localRepo, sshRepo], - getWorktreeMeta: () => localMeta - } + const localMeta = makeWorktreeMeta({ hostId: 'local', preserveBranchOnDelete: true }) + const metadataStore = makeMetadataStore([localRepo, sshRepo], localMeta) expect( - resolveWorktreeRemovalMetadata( - metadataStore as never, - 'repo1', - 'repo1::/shared/worktree', - 'ssh:ssh-1' - ) + resolveWorktreeRemovalMetadata(metadataStore, 'repo1', 'repo1::/shared/worktree', 'ssh:ssh-1') ).toBeUndefined()
makeWorktreeMetais a local fixture builder you add for the requiredWorktreeMetafields, mirroringmakeWorktreeMetainsrc/main/ipc/worktrees-test-fixtures.ts.src/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal-late-settlement.test.ts (1)
66-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBuild the fixture
removedIdentitieswith the identity helper instead of bare worktree ids.Every fixture sets
removedIdentitiesto the same bare worktree ids asremovedIds. A production identity is${hostId}<separator>${worktreeId}, so it never equals a bare id. The late-settlement reconciliation only needs set membership, so these tests pass either way. They would also pass if the implementation swappedremovedIdsandremovedIdentities, which is the exact confusion this PR introduces the two fields to prevent.Compose the fixture values with
getWorkspaceCleanupHostIdentity(or the candidate identity helper) so the reconciliation path is exercised with realistic keys.♻️ Example for one fixture
+import { getWorkspaceCleanupCandidateIdentity } from '../../../../shared/workspace-cleanup-host-identity' ... .mockResolvedValueOnce({ removedIds: [parent.worktreeId], - removedIdentities: [parent.worktreeId], + removedIdentities: [getWorkspaceCleanupCandidateIdentity(parent)], failures: [] })Apply the same change to the child, unrelated, first-child, second-child, and second-parent fixtures, and to the matching
onResult/onLateResultassertions.Also applies to: 84-102, 150-154, 169-194, 226-260
src/renderer/src/store/slices/workspace-cleanup-removal-targets.ts (1)
135-141: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the always-empty
approvedCandidatespread.Line 114 returns when
confirmedCandidateandconfirmedHostIdare both set. Line 125 returns whenconfirmedCandidateis set. Execution reaches Line 135 only whenconfirmedCandidateisundefined. The conditional spread on Line 140 therefore always produces{}.The dead condition states a false invariant. A later reader can conclude that a confirmed candidate reaches this branch and can build logic on that assumption.
♻️ Proposed simplification
return { kind: 'target', worktreeId, executionHostId: ownerHostIds[0] ?? null, - displayName, - ...(confirmedCandidate ? { approvedCandidate: confirmedCandidate } : {}) + displayName }src/renderer/src/store/slices/workspace-cleanup.ts (1)
55-58: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winThe
removedIdentitiesdoc claims a parallel relation that the code does not enforce.Line 57 states that
removedIdentitiesis "parallel to removedIds".removedIdsis a push-ordered array built at Line 431.removedIdentitiesis aSetbuilt at Line 432 and spread to an array at Line 499. ASetdrops duplicates, so the two arrays are parallel only while no batch removes the sameworktreeIdon two hosts.Today
resolveWorkspaceCleanupRemovalTargetsfails such a batch closed through its ambiguity gate, so the relation holds by accident rather than by construction. If that gate is ever relaxed, index-based pairing of the two arrays breaks silently.Either make the relation structural, or document the two fields as independent membership sets.
♻️ Option: build both arrays together
- /** Host-qualified keys of the rows actually removed, parallel to removedIds. */ + /** Host-qualified keys of the rows actually removed, one per removedIds entry. */ removedIdentities: string[]- const removedIdentities = new Set<string>() + const removedIdentities: string[] = [] ... removedIds.push(candidate.worktreeId) - removedIdentities.add(getWorkspaceCleanupCandidateIdentity(candidate)) + removedIdentities.push(getWorkspaceCleanupCandidateIdentity(candidate))Then use a local
Setonly where membership is tested, at Line 459, and returnremovedIdentitiesdirectly at Line 499.src/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-timeout-recovery.test.ts (1)
21-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared
settleBackgroundRemovalhelper.This helper is identical to the one in
src/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal.test.tsandsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal-snapshot-batch.test.ts. The flush count10is an arbitrary constant. If the removal chain gains anotherawait, all three copies must change together, and a missed copy produces an intermittent failure.Move the helper into a shared cleanup test fixture module, for example
workspace-cleanup-removal-test-settlement.ts, and import it in all three suites.♻️ Proposed extraction
New file
src/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-test-settlement.ts:/** Flushes enough microtasks for the cleanup removal batch to settle. */ export async function settleBackgroundRemoval(): Promise<void> { for (let index = 0; index < 10; index += 1) { await Promise.resolve() } }Then in this file:
-async function settleBackgroundRemoval(): Promise<void> { - for (let index = 0; index < 10; index += 1) { - await Promise.resolve() - } -} +import { settleBackgroundRemoval } from './workspace-cleanup-removal-test-settlement'src/main/ipc/worktrees.ts (1)
222-226: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove this import into the top-level import block.
The
importstatement at lines 222-226 sits after thegetRepoForWorktreeRemovalbody. Module imports hoist, so behavior is unchanged, but a mid-file import reads as a patch artifact and can trip import-ordering lint rules on this file.♻️ Proposed relocation
function getRepoForWorktreeRemoval( store: Store, repoId: string, hostId?: ExecutionHostId ): Repo | undefined { // Why: deletion must never guess between host owners; legacy unscoped calls work only while the repo id has one unique owner. const owner = resolveWorktreeRemovalRepoOwner(store, repoId, hostId) return owner.kind === 'resolved' ? owner.repo : undefined } -import { - hasWorktreeRemovalRepoOwnerOnOtherHost, - resolveWorktreeRemovalMetadata, - resolveWorktreeRemovalRepoOwner -} from '../worktree-removal-repo-owner'Add the same specifiers to the existing import block at the top of the file:
import { hasWorktreeRemovalRepoOwnerOnOtherHost, resolveWorktreeRemovalMetadata, resolveWorktreeRemovalRepoOwner } from '../worktree-removal-repo-owner'src/main/worktree-removal-repo-owner.ts (1)
15-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCompress the explanatory comments.
Keep only the non-obvious host-collision invariant near the code. Move historical issue details and extended policy rationale to an ADR or issue if they must remain available.
src/main/worktree-removal-repo-owner.ts#L15-L21: Reduce the resolver contract to the host-qualified ownership rule.src/main/worktree-removal-repo-owner.ts#L42-L59: Keep a short statement that over-preservation is intentional for unpartitioned metadata.src/renderer/src/store/slices/worktree-by-id-index.ts#L8-L16: State that bare-ID lookup is ambiguous across hosts.src/renderer/src/store/worktree-host-collision-index.test.ts#L1-L13: Use the test suite name and one sentence to describe the collision fixture.src/renderer/src/store/worktree-repo-index.ts#L26-L33: State the host-qualified deduplication rule.src/renderer/src/store/worktree-repo-index.ts#L43-L52: State the getter-read constraint without restating index behavior.src/renderer/src/store/worktree-repo-index.ts#L72-L78: State that destructive paths require an explicit host.As per coding guidelines, comments must be concise, non-obvious, and brief—prefer one line.
Source: Coding guidelines
src/shared/worktree/removal.ts (1)
127-145: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShorten the new explanatory comments.
Keep one concise statement of the host-qualified deletion invariant. Remove implementation walkthroughs that the types, tests, and calls already show.
src/shared/worktree/removal.ts#L127-L145: reduce the background and type documentation to the required identity invariant.src/renderer/src/components/sidebar/delete-worktree-flow.ts#L36-L39: retain one short reason for host-aware lookup.src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx#L1622-L1626: retain one short reason for resolving the scanned row on its host.src/renderer/src/components/sidebar/worktree-delete-host-qualification.test.ts#L1-L19: reduce the preamble to the tested collision scenario.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
src/renderer/src/store/slices/workspace-cleanup-host-qualified-list-state.test.ts (1)
108-136: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the persisted dismissal payload carries the host.
The test verifies in-memory keys only.
dismissMockrecords what crosses the IPC boundary, andexecutionHostIdis the new field that makes a persisted dismissal host-scoped. Add an assertion on the mock arguments so a regression that drops the field before persistence is caught.♻️ Suggested addition
expect(store.getState().workspaceCleanupScan?.candidates).toEqual([ expect.objectContaining({ blockers: expect.arrayContaining(['dismissed']) }), expect.objectContaining({ blockers: expect.arrayContaining(['dismissed']) }) ]) + expect(dismissMock).toHaveBeenCalledWith( + expect.arrayContaining([ + expect.objectContaining({ worktreeId: WORKTREE_ID, executionHostId: 'local' }), + expect.objectContaining({ worktreeId: WORKTREE_ID, executionHostId: 'ssh:ssh-1' }) + ]) + )src/shared/worktree/host-qualified-identity.ts (1)
34-51: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winEnforce the separator-free host invariant instead of documenting it.
ExecutionHostIdislocal | \ssh:${string}` | `runtime:${string}`, so the type permits a hand-built id such as ``ssh:${rawAlias}`` that was never passed throughtoSshExecutionHostId. Such an id can contain|, andgetWorktreeIdFromHostIdentity` would then return a truncated workspace id. These identities key the removal-routing indices, so a truncated id fails silently rather than loudly.Encoding the host half inside
composeWorktreeHostIdentitykeeps the split exact for every input and does not change the round-trip contract.♻️ Suggested hardening
export function composeWorktreeHostIdentity( hostId: ExecutionHostId | undefined, worktreeId: string ): string { - return `${hostId ?? ''}${HOST_SEPARATOR}${worktreeId}` + // Encoding here keeps the split exact even for a host id built without the + // `toSshExecutionHostId` / `toRuntimeExecutionHostId` factories. + return `${hostId ? encodeURIComponent(hostId) : ''}${HOST_SEPARATOR}${worktreeId}` }If any caller compares an identity's host half against a raw
hostId, decode it there, or add agetHostIdFromWorktreeHostIdentityhelper.src/renderer/src/lib/worktree-operation-catalog-route.ts (1)
101-109: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCompare routes by field instead of
JSON.stringify.
JSON.stringifyequality depends on key insertion order. Both routes come fromrouteForOwner, so today the order matches. A future field added on one path would make equal routes read as ambiguous. Compare the two fields directly.♻️ Proposed structural comparison
const current = next.get(repoId) if (!current) { next.set(repoId, { kind: 'resolved', route }) } else if ( current.kind === 'resolved' && - JSON.stringify(current.route) !== JSON.stringify(route) + (current.route.executionHostId !== route.executionHostId || + current.route.runtimeEnvironmentId !== route.runtimeEnvironmentId) ) { next.set(repoId, { kind: 'ambiguous' }) }Also note that the loop variable
repoIdon line 93 shadows the function parameterrepoId. Rename the inner one toownerRepoIdfor clarity.src/renderer/src/store/slices/worktree-removal-host-collision-fixture.ts (1)
64-89: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRestrict the destructive delete to roots this rig created.
deleteOnHostrunsfs.rmSync(root, { recursive: true, force: true })on any string found inrootsByHostId. A test that passes a non-temporary path by mistake would delete real files on the developer machine. Track the roots created bycreateHostCheckoutand delete only those.🛡️ Proposed guard
const hostDirCleanup: (() => void)[] = [] +const createdRoots = new Set<string>() /** One real directory per host; its marker file is that host's uncommitted work. */ export function createHostCheckout(hostId: ExecutionHostId): HostCheckout { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'sta4343-')) + createdRoots.add(root) hostDirCleanup.push(() => fs.rmSync(root, { recursive: true, force: true }))const deleteOnHost = (hostId: string | undefined): void => { routedHostIds.push(hostId ?? '<missing>') const root = hostId ? rootsByHostId[hostId as ExecutionHostId] : undefined - if (root) { + if (root && createdRoots.has(root)) { fs.rmSync(root, { recursive: true, force: true }) } }src/renderer/src/store/slices/worktrees/teardown/host-qualified-worktree-removal.ts (1)
145-163: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueHoist the state read and match options out of the
findpredicate.
get()andworktreeHostMatchOptions(...)run once for every candidate entry. Both values are constant for the call. Compute them once before the scan.♻️ Proposed refactor
export function findWorktreeOnConfirmedHost( get: WorktreeSliceGet, worktreeId: string, requiredExecutionHostId: ExecutionHostId | null ): PreservedBranchWorktree { const repoId = getRepoIdFromWorktreeId(worktreeId) - return get() - .allWorktrees() - .find( - (entry) => - entry.id === worktreeId && - (!requiredExecutionHostId || - worktreeMatchesHost( - entry, - requiredExecutionHostId, - worktreeHostMatchOptions(get(), repoId, requiredExecutionHostId) - )) - ) + const state = get() + const matchOptions = requiredExecutionHostId + ? worktreeHostMatchOptions(state, repoId, requiredExecutionHostId) + : null + return state + .allWorktrees() + .find( + (entry) => + entry.id === worktreeId && + (!requiredExecutionHostId || + worktreeMatchesHost(entry, requiredExecutionHostId, matchOptions!)) + ) }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f2b7ec50-cc6c-4337-9ef5-0359003a4dc8
📒 Files selected for processing (130)
src/cli/handlers/worktree.tssrc/cli/index.test.tssrc/main/ipc/workspace-cleanup-broad-scan.test.tssrc/main/ipc/workspace-cleanup-disconnected-ssh.tssrc/main/ipc/workspace-cleanup-folder-workspaces.tssrc/main/ipc/workspace-cleanup-scan.tssrc/main/ipc/workspace-cleanup-snapshot-ipc.test.tssrc/main/ipc/workspace-cleanup.tssrc/main/ipc/worktrees-preserved-branch-fork-remote.test.tssrc/main/ipc/worktrees-removal-recovery.test.tssrc/main/ipc/worktrees-remove-host-disambiguation.test.tssrc/main/ipc/worktrees.tssrc/main/persistence-host-partitioned-sessions.test.tssrc/main/persistence/loading-store/store.tssrc/main/runtime/orca-runtime.test.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/repo-worktree-row-resolution.test.tssrc/main/runtime/repo-worktree-row-resolution.tssrc/main/runtime/rpc/methods/workspace-cleanup-ui-schema.test.tssrc/main/runtime/rpc/methods/workspace-cleanup-ui-schema.tssrc/main/runtime/rpc/methods/worktree-rm-host-qualification.test.tssrc/main/runtime/rpc/methods/worktree-rm-pty-waiver.test.tssrc/main/runtime/rpc/methods/worktree.test.tssrc/main/runtime/rpc/methods/worktree.tssrc/main/runtime/runtime-folder-workspace.tssrc/main/worktree-metadata-ownership.tssrc/main/worktree-removal-repo-owner.test.tssrc/main/worktree-removal-repo-owner.tssrc/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsxsrc/renderer/src/components/sidebar/DeleteWorktreeDialog.tsxsrc/renderer/src/components/sidebar/ForgetSshWorkspaceDialog.tsxsrc/renderer/src/components/sidebar/WorktreeList.lineage-agent-expansion-coupling.test.tsxsrc/renderer/src/components/sidebar/WorktreeList.lineage-child-card.test.tssrc/renderer/src/components/sidebar/WorktreeList.lineage-child-real-card.test.tsxsrc/renderer/src/components/sidebar/delete-worktree-flow.test.tssrc/renderer/src/components/sidebar/delete-worktree-flow.tssrc/renderer/src/components/sidebar/delete-worktree-parallel-flow.test.tssrc/renderer/src/components/sidebar/ssh-host-remove-workspaces.tssrc/renderer/src/components/sidebar/use-confirmed-worktree-delete-targets.tssrc/renderer/src/components/sidebar/use-worktree-card-workspace-actions.tssrc/renderer/src/components/sidebar/workspace-delete-lineage.test.tssrc/renderer/src/components/sidebar/workspace-delete-lineage.tssrc/renderer/src/components/sidebar/worktree-context-menu-delete-intent.tssrc/renderer/src/components/sidebar/worktree-delete-execution.tssrc/renderer/src/components/sidebar/worktree-delete-host-qualification.test.tssrc/renderer/src/components/sidebar/worktree-delete-request.test.tssrc/renderer/src/components/sidebar/worktree-delete-request.tssrc/renderer/src/components/sidebar/worktree-list-card-markup-queries.tssrc/renderer/src/components/sidebar/worktree-list-groups-host-collision.test.tssrc/renderer/src/components/sidebar/worktree-list/grouping/build-rows.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/row-builders.tssrc/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsxsrc/renderer/src/components/status-bar/workspace-space-delete-host-routing.test.tssrc/renderer/src/components/status-bar/workspace-space-delete-selection.tssrc/renderer/src/components/status-bar/workspace-space-presentation.test.tssrc/renderer/src/components/status-bar/workspace-space-presentation.tssrc/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.mount-gating.test.tsxsrc/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.stale-while-revalidate.test.tsxsrc/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.tsxsrc/renderer/src/components/workspace-cleanup/use-workspace-cleanup-dialog-lifecycle.tssrc/renderer/src/components/workspace-cleanup/use-workspace-cleanup-facet-rows.test.tsxsrc/renderer/src/components/workspace-cleanup/use-workspace-cleanup-facet-rows.tssrc/renderer/src/components/workspace-cleanup/use-workspace-cleanup-git-evidence.test.tsxsrc/renderer/src/components/workspace-cleanup/use-workspace-cleanup-git-evidence.tssrc/renderer/src/components/workspace-cleanup/use-workspace-cleanup-removal.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-ancestor-skips.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal-late-settlement.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal-snapshot-batch.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.test.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-confirm-remove.test.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-confirm-remove.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-deletion-phases.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-deletion-phases.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-facet-filter.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-facet-row-caches.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-facet.test.fixture.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-facets.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-flat-list.test.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-git-evidence.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-git-evidence.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-host-identity.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-host-label.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-post-batch-late-settlement.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-query.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-settlement.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-settlement.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-timeout-recovery.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-row-list.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-scanned-host-confirmation-removal.test.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-selection-model.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/lib/worktree-operation-catalog-route.tssrc/renderer/src/lib/worktree-operation-route.tssrc/renderer/src/store/selectors.tssrc/renderer/src/store/slices/editor-state-worktree-purge-leak.test.tssrc/renderer/src/store/slices/generation-records-worktree-removal-leak.test.tssrc/renderer/src/store/slices/native-chat-launch-draft-teardown.test.tssrc/renderer/src/store/slices/store-worktree-removal-cascade.test.tssrc/renderer/src/store/slices/tab-worktree-orphan-map-purge-leak.test.tssrc/renderer/src/store/slices/workspace-cleanup-host-qualified-list-state.test.tssrc/renderer/src/store/slices/workspace-cleanup-removal-preflight.test.tssrc/renderer/src/store/slices/workspace-cleanup-removal-targets.tssrc/renderer/src/store/slices/workspace-cleanup-scan-progress.test.tssrc/renderer/src/store/slices/workspace-cleanup-wrong-host-removal-guard.test.tssrc/renderer/src/store/slices/workspace-cleanup-wrong-host-removal.test.tssrc/renderer/src/store/slices/workspace-cleanup.tssrc/renderer/src/store/slices/worktree-by-id-index.tssrc/renderer/src/store/slices/worktree-helpers.tssrc/renderer/src/store/slices/worktree-removal-host-collision-fixture.tssrc/renderer/src/store/slices/worktree-removal-maps-leak.test.tssrc/renderer/src/store/slices/worktree-removal-options.tssrc/renderer/src/store/slices/worktrees-remote-runtime-removal.test.tssrc/renderer/src/store/slices/worktrees-removal-state-cleanup.test.tssrc/renderer/src/store/slices/worktrees/listing/worktree-slice-constants.tssrc/renderer/src/store/slices/worktrees/teardown/dispatch-worktree-removal.tssrc/renderer/src/store/slices/worktrees/teardown/host-qualified-worktree-removal.tssrc/renderer/src/store/slices/worktrees/teardown/remove-worktree.tssrc/renderer/src/store/slices/worktrees/teardown/removed-worktree-snapshot-prune.tssrc/renderer/src/store/worktree-host-collision-index.test.tssrc/renderer/src/store/worktree-repo-index.tssrc/shared/workspace-cleanup-host-identity.tssrc/shared/workspace-cleanup.tssrc/shared/worktree/host-qualified-identity.test.tssrc/shared/worktree/host-qualified-identity.tssrc/shared/worktree/removal.ts
💤 Files with no reviewable changes (2)
- src/main/runtime/rpc/methods/worktree.test.ts
- src/renderer/src/components/status-bar/workspace-space-presentation.ts
Included review availability: Your plan includes up to 10 reviews per rolling hour; 0 remain after this review.
| if (!canQueueWorkspaceCleanupCandidate(candidate)) { | ||
| return failure( | ||
| candidate.blockers.length | ||
| ? candidate.blockers.join(', ') | ||
| : 'Workspace needs another look before removal.' | ||
| ) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find an existing user-facing label mapping for cleanup blockers.
rg -nP -C3 'WorkspaceCleanupBlocker' src/renderer/src src/shared | rg -nP -i 'label|translate|t\(' -C2
# Confirm which cleanup blocker strings already exist in the locale file.
rg -nP -C1 'cleanup' src/renderer/src/i18n/locales/en.json | rg -nP -i 'blocker|dirty|unpushed'Repository: stablyai/orca
Length of output: 12602
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- target file ---'
sed -n '1,45p;220,255p' src/renderer/src/store/slices/workspace-cleanup-removal-targets.ts
printf '%s\n' '--- blocker label helper ---'
cat -n src/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-labels.ts | sed -n '1,75p'
printf '%s\n' '--- translation keys and implementation ---'
rg -n -C2 'workspace\.cleanup|needsAnotherLook|dirtyFiles|unpushedCommits|mainWorktree|folderRepo' src/renderer/src/i18n/locales/en.json src/renderer/src/i18n
printf '%s\n' '--- failure consumers ---'
rg -n -C3 'canQueueWorkspaceCleanupCandidate|removal-targets|failure\(' src/renderer/src/store src/renderer/src/components/workspace-cleanupRepository: stablyai/orca
Length of output: 23984
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- complete blocker label helper ---'
cat -n src/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-labels.ts | sed -n '1,115p'
printf '%s\n' '--- candidate blocker type and queue predicate ---'
rg -n -C4 'type WorkspaceCleanupBlocker|export type WorkspaceCleanupBlocker|function canQueueWorkspaceCleanupCandidate|export function canQueueWorkspaceCleanupCandidate' src/shared/workspace-cleanup.ts
printf '%s\n' '--- all direct expectations for preflight failure text ---'
rg -n -C3 'dirty-files|unpushed-commits|Workspace needs another look|changedSinceConfirmation|failure\.message|preflightWorkspaceCleanupCandidates' src/renderer/src --glob '*.{ts,tsx}'
printf '%s\n' '--- translation entries for every blocker label ---'
sed -n '2738,2765p' src/renderer/src/i18n/locales/en.jsonRepository: stablyai/orca
Length of output: 36257
Use translated blocker labels in the preflight failure. Map candidate.blockers through getWorkspaceCleanupBlockerLabel instead of joining raw identifiers, and wrap the fallback message in translate(...).
Squashed integration of PR #14606 + the codex review-loop output, replayed onto current main. Granular history preserved on brennanb2025/sta-4343-review-full. Fixes the regression from #13413: a workspace id is repoId::path with no host component, so the same repo at the same path on two hosts published one id for two workspaces, and deletion routed by that id landed on whichever host routing preferred - usually the ACTIVE one, not the row the user confirmed. - removeWorktree takes a REQUIRED host-qualified WorktreeRemovalTarget; omitting the host is a type error. All destructive callers migrated. - Projections dedup on (host, id), so two hosts render as two selectable rows while the createWorktree/fetchWorktrees race duplicate still collapses. - Ephemeral VM cleanup is host-scoped. It matched on bare workspaceId, so the host-scoped delete path destroyed the SURVIVING host's VM and its unpushed filesystem - a leak fix that had become data destruction. - Selection, keyboard routing, lineage grouping and Space rows carry host identity end to end; fixing the executor dedupe alone would have turned one-row intent into deleting both hosts. Files split to stay under max-lines rather than raising any cap.
The review-loop commits used --no-verify, so the pre-commit hook never enforced the caps. Extracted cohesive units rather than raising any limit: renderer teardown, delete-with-toast, pinned-group rows, host-scope helpers, workspace-kind predicates, filter actions, kanban drag selection, the renderer removal result type, and the native-chat persistence tests.
Clears the last max-lines violation and the import-type side effect the changed-code gate flagged.
c5b916d to
c7cd107
Compare
|
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. |
Live Electron QA — round 2 (dev build of
|
| before (host dropped from the identity) | after (this PR) |
|---|---|
 |
 |
Before: one row, no host chip — the second host's workspace is invisible and unreachable, so any delete on it can only route by bare id.
After: two rows, each labelled with its host (Local Mac / qa-linux).
Scaled up to two workspaces × two hosts, all four stay distinct:
❌ Two defects found, not yet fixed
A (P0) — the sidebar context menu drops the host, so delete still routes to the wrong one.
WorktreeContextMenu.tsx handleDelete builds the intent as { id, instanceId } with no hostId (both the single and the batch path). runWorktreeContextMenuDeleteIntent only forwards expectedHostId when hostId is truthy, so getWorktreeOnHostFromState(state, id, undefined) falls through to its first-wins rows[0].
Reproduced live: right-clicking the qa-linux row and confirming Delete Workspace produced a removal target of { id: "…::/…/qa-collision", executionHostId: "local" }.
The props are already in scope (worktree: Worktree, and batchDeleteWorktrees are full rows), so it is a small change. A sweep of every other delete-identity construction found no other site dropping the host — the Space Manager batch path resolves the row with its host correctly.
B (P1) — the host identity leaked into the jump palette's cmdk value; this is the CI red.
WorktreeJumpPalette.tsx changed the item id from worktree:${worktree.id} to worktree:${identity}, and that id is passed straight to cmdk as value=. cmdk scores against value, so every workspace row's search value now carries a host prefix and a |. Six tests fail on shard 9/16 (deterministically, on both node 24 and node 26) — and one of them is not a renamed string: the worktree entry drops out of the result set entirely. That is a ranking regression on the jump palette for all users, not only the collision case.
Not a defect in this PR
Opening a terminal on a row whose host does not resolve shows "Workspace identity is ambiguous across hosts." That guard (terminalOwnerUnresolved, pty-connection.ts) is pre-existing on main; it fired only because the QA fixture's ssh:qa-linux host does not exist.
CI
30 of 32 test shards green, plus package (mac + windows), static analysis, git compatibility, shell contracts, cross-version wire compatibility and native smoke. The only red is shard 9/16 on both node versions — defect B above.
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/renderer/src/components/sidebar/worktree-list/drag/use-pointer-drag.ts (1)
157-185: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPreserve host identity in the drag payload.
Line 183 identifies selected rows by host-qualified identity. Line 184 then discards that identity.
If two selected worktrees have the same
idon different hosts,draggedIdsbecomes two identical strings. Reorder and workspace-board drop processing cannot target the selected hosts independently.Carry host-qualified identities, or
Worktreetargets, throughWorktreePointerDrag, reorder resolution, and status-drop actions.src/renderer/src/components/sidebar/worktree-list/navigation/pending-reveal-inputs.ts (1)
73-100: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not let a hostless row satisfy a host-qualified reveal.
When
executionHostIdis set, lines 73-77 can select a legacy hostless worktree before the exact-host worktree with the same ID. Lines 87-100 can also overwrite an exact-host map entry with that legacy row. The reveal then expands the wrong lineage, while later host-qualified row lookup searches for the requested host and cannot find it.Require an exact host match for a host-qualified reveal. Apply the same rule in
sidebarWorkspaceStillExistsinfolder-reveal.ts. Add a regression case with a hostless row preceding an exact-host row with the same ID.
🧹 Nitpick comments (15)
src/main/persistence-native-chat-tab-view-mode.test.ts (1)
12-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCorrect the stale rationale comment.
This file contains no SSH-import test. The comment describes a test that lives in another suite. Keep the stub if
createStore()reads the real~/.ssh/config, and state that reason instead.♻️ Proposed comment fix
-// Stub the ~/.ssh/config parser so the SSH-import test drives the real Store with deterministic hosts, not the operator's actual ~/.ssh/config. +// Stub the ~/.ssh/config parser so constructing the real Store never reads the operator's actual ~/.ssh/config.src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx (1)
1470-1479: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBound the runtime Git status request with a timeout.
getRuntimeGitStatushas no timeout. If the resolved host stops responding, the promise never settles. The row then keepsisRefreshing: trueand holds one of the six worker slots for the lifetime of the panel. The row degrades to "Keep: git not checked", so deletion stays fail-closed, but the state never recovers without a rescan.Wrap the call with the repository's renderer-side timeout helper so the row reports a concrete error instead.
Based on learnings that renderer-side IPC calls in this repository should carry an explicit timeout rather than being issued bare, so a hang cannot block UI state indefinitely.
Source: Learnings
src/renderer/src/components/status-bar/workspace-space-delete-host-routing.test.ts (1)
66-74: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExtract the Space delete resolution so this test covers production code.
identitiesForSpaceDeleteduplicates the panel's resolution loop atWorkspaceSpaceManagerPanel.tsxLines 1745-1754. The test therefore asserts against a copy. IfdeleteWorktreesregresses to passing a bare id, every case here still passes, and the regression the header comment documents goes undetected.Move the "Space row -> store row on its host -> identity" mapping into a named module, call it from
deleteWorktrees, and import it here. KeeplookupOnHostas the injected lookup.src/renderer/src/components/status-bar/workspace-space-presentation.ts (1)
266-290: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the id-shaped names that now carry host-qualified identities. These values changed from bare worktree ids to host-qualified identities, but the function names, prop names, and callback parameter names still say
worktreeIdorIds. Both a bare id and an identity arestring, so passing a bareworktreeIdtype-checks and silently reintroduces the STA-4343 collision.
src/renderer/src/components/status-bar/workspace-space-presentation.ts#L266-L290: renameresolveWorkspaceSpaceInspectedWorktreeIdandresolveWorkspaceSpaceTreemapZoomWorktreeIdto...Identity, and renamepruneWorkspaceSpaceSelectedIdsand itsselectedIdsparameter at Line 300 to identity terms.src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx#L832-L848: rename theWorkspaceTreemappropselectedWorktreeIdtoselectedWorktreeIdentity, and rename theonSelectandonZoomChangeparameters fromworktreeIdtoidentity.Consider a branded
WorktreeHostIdentitystring type inshared/worktree/host-qualified-identity.tsso the compiler rejects a bare id at these boundaries.src/renderer/src/components/status-bar/workspace-space-delete-selection.ts (1)
34-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the bare-ID selection helper.
Production deletion already uses
getSelectedDeletableWorkspaceRows. RemovegetSelectedDeletableWorkspaceIdsand update its two tests to exercise the row-returning helper instead.src/renderer/src/components/sidebar/workspace-kanban-pointer-drag-selection.ts (1)
30-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the redundant comment.
The type name already states its purpose. As per coding guidelines, comments must be concise, non-obvious, and brief.
Source: Coding guidelines
src/renderer/src/components/sidebar/delete-worktree-dialog-force-delete.ts (1)
36-46: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winMatch the force target on host identity, not the bare id.
The comment states that a bare id could force delete another host's checkout at the same path. The predicate at Line 39 still matches on
entry.id === worktreeId. The host guarantee comes only from the caller, which passes a host-qualifiedcurrentWorktrees. Force delete is currently single-target, so no collision is reachable today. Force delete is also irreversible, so make the invariant local to this function.Pass the confirmed host and match both fields.
♻️ Proposed hardening
export function runDialogForceDelete(args: { worktreeId: string + worktreeHostId?: ExecutionHostId currentWorktrees: readonly Worktree[]- const forceTarget = currentWorktrees.find((entry) => entry.id === worktreeId) + const forceTarget = currentWorktrees.find( + (entry) => + entry.id === worktreeId && (!args.worktreeHostId || entry.hostId === args.worktreeHostId) + )src/renderer/src/components/sidebar/delete-worktree-dirty-change-counts.ts (1)
26-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize the
dirtyChangeCountsByWorktreeIdkey rule. The expressionworktree.hostId ? getWorktreeHostIdentity(worktree) : worktree.idis repeated in the producer and in all three readers. The producer and every reader must compute the same string, or the dirty-change hint silently renders no count. Export one key helper next to the producer and call it at each site.
src/renderer/src/components/sidebar/delete-worktree-dirty-change-counts.ts#L26-L41: export agetDirtyChangeCountKey(worktree: Pick<Worktree, 'id' | 'hostId'>)helper and use it forresultKeyand for thegitStatusByWorktreeIdentitylookup.src/renderer/src/components/sidebar/DeleteWorktreeTargetPreview.tsx#L39-L41: replace the inline conditional withgetDirtyChangeCountKey(item).src/renderer/src/components/sidebar/DeleteWorktreeTargetPreview.tsx#L66-L68: replace the inline conditional withgetDirtyChangeCountKey(worktree).src/renderer/src/components/sidebar/DeleteWorktreeLineageNotice.tsx#L53-L55: replace the inline conditional withgetDirtyChangeCountKey(child).src/renderer/src/store/slices/worktrees/teardown/removed-worktree-renderer-teardown.ts (1)
19-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument why
hostIdandrequiredExecutionHostIdare separate parameters.The signature takes two host values.
requiredExecutionHostIdselects the host-scoped shape forcleanupEphemeralVmRuntimesForDeletedat Line 38.hostIdselects the host-scoped shape forremoveWorkspaceSpaceWorktreesat Line 63. The types differ only byundefinedversusnull. A caller cannot tell the two apart from the signature.This is a destructive teardown path. Add a short doc line for each parameter, or collapse them into one host value if the callers always pass the same host.
src/renderer/src/store/slices/workspace-cleanup-wrong-host-removal.test.ts (1)
126-166: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the per-host temp directory rig; it now exists in three copies.
parseIdentityPath,toHostRelativePath,createHostDirectories, and thehostDirCleanupregistry duplicate the same helpers insrc/renderer/src/store/slices/workspace-cleanup-wrong-host-removal-guard.test.tsat Lines 125-151. The sidebar test in this same PR already imports an equivalent rig fromworktree-removal-host-collision-fixture. Move this rig into one shared fixture module and import it in all three tests, so a fix to the Windows-path mapping applies everywhere.#!/bin/bash # Locate every copy of the per-host temp directory rig in the cohort. rg -nP -C3 '\b(toHostRelativePath|createHostDirectories|createHostCheckout|hostDirCleanup)\b' srcsrc/renderer/src/store/slices/workspace-space.ts (1)
204-227: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the target-partitioning logic; it is duplicated on a destructive path.
Lines 209-218 rebuild the same
deletedIdsanddeletedIdentitiessets thatremoveDeletedWorktreesFromAnalysisbuilds at Lines 33-43. If the two copies drift,workspaceSpaceMeasurementsandworkspaceSpaceAnalysiswill disagree about which rows were removed. Extract one helper and call it from both places.Also document the two modes of the union parameter at Lines 24-26. A bare
stringtarget removes every row with that id on any host. A{ id, executionHostId: null }target removes only the row that carries no host qualifier. A caller could reasonably expect those to be equivalent.♻️ Proposed extraction
+/** + * A bare string id matches every host's row; a target matches only the row whose + * host equals `executionHostId` (`null` meaning the unqualified row). + */ +function partitionRemovalTargets(targets: readonly (string | WorktreeRemovalTarget)[]): { + ids: ReadonlySet<string> + identities: ReadonlySet<string> +} { + const ids = new Set<string>() + const identities = new Set<string>() + for (const target of targets) { + if (typeof target === 'string') { + ids.add(target) + } else { + identities.add(composeWorktreeHostIdentity(target.executionHostId ?? undefined, target.id)) + } + } + return { ids, identities } +} + function removeDeletedWorktreesFromAnalysis( analysis: WorkspaceSpaceAnalysis, deletedWorktreeTargets: readonly (string | WorktreeRemovalTarget)[] ): WorkspaceSpaceAnalysis { - const deletedIds = new Set<string>() - const deletedIdentities = new Set<string>() - for (const target of deletedWorktreeTargets) { - if (typeof target === 'string') { - deletedIds.add(target) - } else { - deletedIdentities.add( - composeWorktreeHostIdentity(target.executionHostId ?? undefined, target.id) - ) - } - } + const { ids: deletedIds, identities: deletedIdentities } = + partitionRemovalTargets(deletedWorktreeTargets)set((state) => { - const deletedIds = new Set( - worktreeTargets.flatMap((target) => (typeof target === 'string' ? [target] : [])) - ) - const deletedIdentities = new Set( - worktreeTargets.flatMap((target) => - typeof target !== 'string' - ? [composeWorktreeHostIdentity(target.executionHostId ?? undefined, target.id)] - : [] - ) - ) + const { ids: deletedIds, identities: deletedIdentities } = + partitionRemovalTargets(worktreeTargets)src/renderer/src/components/sidebar/worktree-keyboard-cycle.ts (1)
9-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove
getCyclableWorktreeIdsand its tests. No production consumer remains; keyboard navigation usesgetCyclableWorktrees. The ID-only helper can also return duplicate IDs for the same worktree ID on different hosts, whichresolveCycledWorktreeIdcannot distinguish.src/renderer/src/components/sidebar/worktree-list/grouping/row-builders.ts (1)
96-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused
worktreeMapparameters from the row emitters
appendWorktreeRowsdoes not readworktreeMap, andemitPinnedGrouponly forwards it. Remove the parameter from both signatures and their call sites. KeepbuildRows’sworktreeMapparameter because other lineage functions still use it.src/renderer/src/store/slices/worktrees/teardown/host-qualified-worktree-row-removal.ts (1)
143-157: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollapse the pass-through wrapper.
prepareHostScopedRemovalCompletionforwards every argument todropConfirmedHostRowand returns its value. The signatures are identical, so the wrapper adds an indirection without behavior. ExportdropConfirmedHostRowunder one name, or keep an alias export instead of a second function body.♻️ Proposed refactor
-export function prepareHostScopedRemovalCompletion( - set: WorktreeSliceSet, - worktreeId: string, - requiredExecutionHostId: ExecutionHostId, - fallbackSurvivingHostId: ExecutionHostId | null, - ignoreWorkspaceCleanupScanSurvivors = false -): boolean { - return dropConfirmedHostRow( - set, - worktreeId, - requiredExecutionHostId, - fallbackSurvivingHostId, - ignoreWorkspaceCleanupScanSurvivors - ) -} +export { dropConfirmedHostRow as prepareHostScopedRemovalCompletion }src/renderer/src/components/workspace-cleanup/workspace-cleanup-deletion-phases.ts (1)
51-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd and use a shared host-qualification predicate.
host-qualified-identity.tsexposes only a parser. The parser is unsafe for detection because bare worktree IDs can contain|. Add a predicate that accepts the host ID and use it instead of derivinghostPrefixwith an empty worktree ID.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5fd726e7-6a93-4c4e-8778-4d5cd58bdc21
📒 Files selected for processing (237)
src/cli/handlers/worktree.tssrc/cli/index.test.tssrc/main/ipc/workspace-cleanup-broad-scan.test.tssrc/main/ipc/workspace-cleanup-disconnected-ssh.tssrc/main/ipc/workspace-cleanup-folder-workspaces.tssrc/main/ipc/workspace-cleanup-scan.tssrc/main/ipc/workspace-cleanup-snapshot-ipc.test.tssrc/main/ipc/workspace-cleanup.tssrc/main/ipc/worktrees-forget-local.test.tssrc/main/ipc/worktrees-preserved-branch-fork-remote.test.tssrc/main/ipc/worktrees-removal-recovery.test.tssrc/main/ipc/worktrees-remove-host-disambiguation.test.tssrc/main/ipc/worktrees.tssrc/main/persistence-host-partitioned-sessions.test.tssrc/main/persistence-native-chat-tab-view-mode.test.tssrc/main/persistence/loading-store/store.tssrc/main/runtime/orca-runtime.test.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/repo-worktree-row-resolution.test.tssrc/main/runtime/repo-worktree-row-resolution.tssrc/main/runtime/rpc/methods/workspace-cleanup-ui-schema.test.tssrc/main/runtime/rpc/methods/workspace-cleanup-ui-schema.tssrc/main/runtime/rpc/methods/worktree-rm-host-qualification.test.tssrc/main/runtime/rpc/methods/worktree-rm-pty-waiver.test.tssrc/main/runtime/rpc/methods/worktree-schemas.tssrc/main/runtime/rpc/methods/worktree.test.tssrc/main/runtime/rpc/methods/worktree.tssrc/main/runtime/runtime-folder-workspace.tssrc/main/worktree-metadata-ownership.tssrc/main/worktree-removal-repo-owner.test.tssrc/main/worktree-removal-repo-owner.tssrc/renderer/src/components/WorktreeJumpPalette.test.tsxsrc/renderer/src/components/WorktreeJumpPalette.tsxsrc/renderer/src/components/right-sidebar/HostedReviewActions.tsxsrc/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsxsrc/renderer/src/components/sidebar/DeleteWorktreeDialog.tsxsrc/renderer/src/components/sidebar/DeleteWorktreeLineageNotice.tsxsrc/renderer/src/components/sidebar/DeleteWorktreeTargetPreview.tsxsrc/renderer/src/components/sidebar/ForgetSshWorkspaceDialog.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanCard.host-identity.test.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanCard.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanDrawer.search.test.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanDrawer.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanLaneCardList.test.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanLaneCardList.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanLaneGrid.test.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanLaneGrid.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanStatusLane.test.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanStatusLane.tsxsrc/renderer/src/components/sidebar/WorktreeContextMenu.test.tssrc/renderer/src/components/sidebar/WorktreeContextMenu.tsxsrc/renderer/src/components/sidebar/WorktreeList.lineage-agent-expansion-coupling.test.tsxsrc/renderer/src/components/sidebar/WorktreeList.lineage-child-card.test.tssrc/renderer/src/components/sidebar/WorktreeList.lineage-child-real-card.test.tsxsrc/renderer/src/components/sidebar/WorktreeList.tsxsrc/renderer/src/components/sidebar/active-worktree-focus-after-delete.tssrc/renderer/src/components/sidebar/delete-worktree-dialog-force-delete.tssrc/renderer/src/components/sidebar/delete-worktree-dirty-change-counts.tssrc/renderer/src/components/sidebar/delete-worktree-flow.test.tssrc/renderer/src/components/sidebar/delete-worktree-flow.tssrc/renderer/src/components/sidebar/delete-worktree-lineage-delete-all.tssrc/renderer/src/components/sidebar/delete-worktree-parallel-flow.test.tssrc/renderer/src/components/sidebar/rendered-sidebar-worktree-order.test.tssrc/renderer/src/components/sidebar/rendered-sidebar-worktree-order.tssrc/renderer/src/components/sidebar/run-worktree-delete-with-toast.tssrc/renderer/src/components/sidebar/sidebar-filter-actions.tssrc/renderer/src/components/sidebar/ssh-host-remove-workspaces.tssrc/renderer/src/components/sidebar/use-confirmed-worktree-delete-targets.tssrc/renderer/src/components/sidebar/use-delete-worktree-status-hydration.tssrc/renderer/src/components/sidebar/use-visible-workspace-kanban-worktree-ids.test.tsxsrc/renderer/src/components/sidebar/use-visible-workspace-kanban-worktree-ids.tssrc/renderer/src/components/sidebar/use-workspace-kanban-card-pointer-drag.test.tssrc/renderer/src/components/sidebar/use-workspace-kanban-card-pointer-drag.tssrc/renderer/src/components/sidebar/use-workspace-kanban-selection.test.tsxsrc/renderer/src/components/sidebar/use-workspace-kanban-selection.tssrc/renderer/src/components/sidebar/use-worktree-card-activation-actions.tssrc/renderer/src/components/sidebar/use-worktree-card-foundation.tssrc/renderer/src/components/sidebar/use-worktree-card-workspace-actions.tssrc/renderer/src/components/sidebar/visible-worktree-host-scope.tssrc/renderer/src/components/sidebar/visible-worktree-kinds.tssrc/renderer/src/components/sidebar/visible-worktrees.tssrc/renderer/src/components/sidebar/workspace-delete-lineage.test.tssrc/renderer/src/components/sidebar/workspace-delete-lineage.tssrc/renderer/src/components/sidebar/workspace-kanban-card-drag-preview-dom.tssrc/renderer/src/components/sidebar/workspace-kanban-pointer-drag-selection.tssrc/renderer/src/components/sidebar/workspace-kanban-search.test.tssrc/renderer/src/components/sidebar/workspace-kanban-search.tssrc/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.tssrc/renderer/src/components/sidebar/workspace-kanban-virtual-lane-layout.tssrc/renderer/src/components/sidebar/workspace-kanban-worktree-groups.test.tssrc/renderer/src/components/sidebar/workspace-kanban-worktree-groups.tssrc/renderer/src/components/sidebar/worktree-card-model.tssrc/renderer/src/components/sidebar/worktree-context-menu-delete-intent.tssrc/renderer/src/components/sidebar/worktree-delete-execution.tssrc/renderer/src/components/sidebar/worktree-delete-host-qualification.test.tssrc/renderer/src/components/sidebar/worktree-delete-request.test.tssrc/renderer/src/components/sidebar/worktree-delete-request.tssrc/renderer/src/components/sidebar/worktree-delete-state-host-match.tssrc/renderer/src/components/sidebar/worktree-keyboard-cycle.test.tssrc/renderer/src/components/sidebar/worktree-keyboard-cycle.tssrc/renderer/src/components/sidebar/worktree-list-card-markup-queries.tssrc/renderer/src/components/sidebar/worktree-list-groups-host-collision.test.tssrc/renderer/src/components/sidebar/worktree-list-groups-lineage-nesting.test.tssrc/renderer/src/components/sidebar/worktree-list/drag/use-pointer-drag.tssrc/renderer/src/components/sidebar/worktree-list/grouping/build-rows.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/pinned-group-rows.tssrc/renderer/src/components/sidebar/worktree-list/grouping/row-builders.tssrc/renderer/src/components/sidebar/worktree-list/listing/renderable-rows.tssrc/renderer/src/components/sidebar/worktree-list/listing/use-visible-worktrees.test.tsxsrc/renderer/src/components/sidebar/worktree-list/listing/use-visible-worktrees.tssrc/renderer/src/components/sidebar/worktree-list/navigation/active-descendant-option.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.tssrc/renderer/src/components/sidebar/worktree-list/navigation/use-active-row.tssrc/renderer/src/components/sidebar/worktree-list/navigation/use-keyboard.tssrc/renderer/src/components/sidebar/worktree-list/navigation/use-pending-reveal.tssrc/renderer/src/components/sidebar/worktree-list/navigation/use-reveal-requests.tssrc/renderer/src/components/sidebar/worktree-list/navigation/use-selection-host-collision.test.tsxsrc/renderer/src/components/sidebar/worktree-list/navigation/use-selection.tssrc/renderer/src/components/sidebar/worktree-list/rows/folder-row.tsxsrc/renderer/src/components/sidebar/worktree-list/rows/item-row.tsxsrc/renderer/src/components/sidebar/worktree-list/rows/option-dom-host-collision.test.tssrc/renderer/src/components/sidebar/worktree-list/rows/option-dom.tssrc/renderer/src/components/sidebar/worktree-list/viewport/VirtualizedWorktreeViewport.tsxsrc/renderer/src/components/sidebar/worktree-list/viewport/viewport-props.tssrc/renderer/src/components/sidebar/worktree-list/viewport/virtual-row-context.tssrc/renderer/src/components/sidebar/worktree-list/viewport/virtual-rows.tssrc/renderer/src/components/sidebar/worktree-sidebar-row-preference.tssrc/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsxsrc/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsxsrc/renderer/src/components/status-bar/resource-manager-worktree-target.test.tssrc/renderer/src/components/status-bar/resource-manager-worktree-target.tssrc/renderer/src/components/status-bar/workspace-space-delete-host-routing.test.tssrc/renderer/src/components/status-bar/workspace-space-delete-selection.tssrc/renderer/src/components/status-bar/workspace-space-presentation.test.tssrc/renderer/src/components/status-bar/workspace-space-presentation.tssrc/renderer/src/components/terminal-pane/TerminalSshReconnectOverlay.test.tsxsrc/renderer/src/components/terminal-pane/TerminalSshReconnectOverlay.tsxsrc/renderer/src/components/terminal-pane/pty-connection.tssrc/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.mount-gating.test.tsxsrc/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.stale-while-revalidate.test.tsxsrc/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.tsxsrc/renderer/src/components/workspace-cleanup/use-workspace-cleanup-dialog-lifecycle.tssrc/renderer/src/components/workspace-cleanup/use-workspace-cleanup-facet-rows.test.tsxsrc/renderer/src/components/workspace-cleanup/use-workspace-cleanup-facet-rows.tssrc/renderer/src/components/workspace-cleanup/use-workspace-cleanup-git-evidence.test.tsxsrc/renderer/src/components/workspace-cleanup/use-workspace-cleanup-git-evidence.tssrc/renderer/src/components/workspace-cleanup/use-workspace-cleanup-removal.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-ancestor-skips.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal-late-settlement.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal-snapshot-batch.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.test.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-confirm-remove.test.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-confirm-remove.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-deletion-phases.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-deletion-phases.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-facet-filter.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-facet-row-caches.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-facet.test.fixture.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-facets.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-flat-list.test.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-git-evidence.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-git-evidence.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-host-identity.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-host-label.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-post-batch-late-settlement.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-query.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-candidates.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-candidates.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-settlement.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-settlement.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-timeout-recovery.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-row-list.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-scanned-host-confirmation-removal.test.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-selection-model.tssrc/renderer/src/hooks/ipc-events-test-harness.tssrc/renderer/src/hooks/useIpcEvents-cmd-j-digit-chord.test.tssrc/renderer/src/hooks/useIpcEvents.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.tssrc/renderer/src/lib/ephemeral-vm-runtime-cleanup.tssrc/renderer/src/lib/workspace-activation-path-gate.test.tssrc/renderer/src/lib/worktree-activation-reveal.test.tssrc/renderer/src/lib/worktree-activation.tssrc/renderer/src/lib/worktree-operation-catalog-route.tssrc/renderer/src/lib/worktree-operation-route.tssrc/renderer/src/lib/worktree-palette-search.tssrc/renderer/src/lib/worktree-palette-task-url-match.test.tssrc/renderer/src/lib/worktree-palette-task-url-match.tssrc/renderer/src/store/selectors.tssrc/renderer/src/store/slices/editor-state-worktree-purge-leak.test.tssrc/renderer/src/store/slices/generation-records-worktree-removal-leak.test.tssrc/renderer/src/store/slices/native-chat-launch-draft-teardown.test.tssrc/renderer/src/store/slices/renderer-remove-worktree-result.tssrc/renderer/src/store/slices/store-worktree-removal-cascade.test.tssrc/renderer/src/store/slices/tab-worktree-orphan-map-purge-leak.test.tssrc/renderer/src/store/slices/ui.tssrc/renderer/src/store/slices/workspace-cleanup-host-qualified-list-state.test.tssrc/renderer/src/store/slices/workspace-cleanup-removal-preflight.test.tssrc/renderer/src/store/slices/workspace-cleanup-removal-targets.tssrc/renderer/src/store/slices/workspace-cleanup-scan-progress.test.tssrc/renderer/src/store/slices/workspace-cleanup-wrong-host-removal-guard.test.tssrc/renderer/src/store/slices/workspace-cleanup-wrong-host-removal.test.tssrc/renderer/src/store/slices/workspace-cleanup.tssrc/renderer/src/store/slices/workspace-space.test.tssrc/renderer/src/store/slices/workspace-space.tssrc/renderer/src/store/slices/worktree-by-id-index.tssrc/renderer/src/store/slices/worktree-helpers.tssrc/renderer/src/store/slices/worktree-removal-host-collision-fixture.tssrc/renderer/src/store/slices/worktree-removal-maps-leak.test.tssrc/renderer/src/store/slices/worktree-removal-options.tssrc/renderer/src/store/slices/worktrees-remote-runtime-removal.test.tssrc/renderer/src/store/slices/worktrees-removal-state-cleanup.test.tssrc/renderer/src/store/slices/worktrees/listing/worktree-slice-constants.tssrc/renderer/src/store/slices/worktrees/teardown/dispatch-worktree-removal.tssrc/renderer/src/store/slices/worktrees/teardown/host-qualified-worktree-removal.tssrc/renderer/src/store/slices/worktrees/teardown/host-qualified-worktree-row-removal.tssrc/renderer/src/store/slices/worktrees/teardown/remove-worktree-store-cleanup.tssrc/renderer/src/store/slices/worktrees/teardown/remove-worktree.tssrc/renderer/src/store/slices/worktrees/teardown/removed-worktree-renderer-teardown.tssrc/renderer/src/store/slices/worktrees/teardown/removed-worktree-snapshot-prune.tssrc/renderer/src/store/slices/worktrees/teardown/worktree-delete-state.tssrc/renderer/src/store/slices/worktrees/teardown/worktree-purge-state.tssrc/renderer/src/store/worktree-host-collision-index.test.tssrc/renderer/src/store/worktree-repo-index.tssrc/shared/workspace-cleanup-host-identity.tssrc/shared/workspace-cleanup.tssrc/shared/worktree/host-qualified-identity.test.tssrc/shared/worktree/host-qualified-identity.tssrc/shared/worktree/removal.ts
💤 Files with no reviewable changes (1)
- src/main/runtime/rpc/methods/worktree.test.ts
🚧 Files skipped from review as they are similar to previous changes (103)
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal-snapshot-batch.test.ts
- src/main/ipc/workspace-cleanup-snapshot-ipc.test.ts
- src/cli/handlers/worktree.ts
- src/main/runtime/runtime-folder-workspace.ts
- src/main/runtime/rpc/methods/workspace-cleanup-ui-schema.test.ts
- src/renderer/src/store/worktree-host-collision-index.test.ts
- src/main/runtime/rpc/methods/worktree-rm-pty-waiver.test.ts
- src/renderer/src/store/slices/workspace-cleanup-scan-progress.test.ts
- src/renderer/src/store/slices/generation-records-worktree-removal-leak.test.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-facet.test.fixture.ts
- src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-dialog-lifecycle.ts
- src/renderer/src/lib/worktree-operation-catalog-route.ts
- src/renderer/src/store/slices/editor-state-worktree-purge-leak.test.ts
- src/renderer/src/components/sidebar/WorktreeList.lineage-child-card.test.ts
- src/shared/worktree/host-qualified-identity.test.ts
- src/main/ipc/workspace-cleanup.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-flat-list.test.tsx
- src/renderer/src/components/sidebar/worktree-list/grouping/group-sections.ts
- src/renderer/src/components/sidebar/worktree-list-groups-host-collision.test.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal.ts
- src/main/ipc/worktrees-preserved-branch-fork-remote.test.ts
- src/renderer/src/store/slices/worktrees/teardown/removed-worktree-snapshot-prune.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-facet-filter.test.ts
- src/main/runtime/rpc/methods/workspace-cleanup-ui-schema.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-settlement.test.ts
- src/renderer/src/components/sidebar/use-worktree-card-workspace-actions.ts
- src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-facet-rows.test.tsx
- src/renderer/src/store/slices/worktree-by-id-index.ts
- src/renderer/src/store/slices/tab-worktree-orphan-map-purge-leak.test.ts
- src/renderer/src/components/sidebar/WorktreeList.lineage-child-real-card.test.tsx
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal-late-settlement.test.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-settlement.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-deletion-phases.test.ts
- src/renderer/src/store/slices/worktree-removal-options.ts
- src/renderer/src/i18n/locales/en.json
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-confirm-remove.test.tsx
- src/main/ipc/worktrees-remove-host-disambiguation.test.ts
- src/cli/index.test.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-post-batch-late-settlement.ts
- src/renderer/src/store/slices/worktrees/listing/worktree-slice-constants.ts
- src/renderer/src/components/sidebar/ForgetSshWorkspaceDialog.tsx
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.test.tsx
- src/main/ipc/worktrees-removal-recovery.test.ts
- src/renderer/src/store/slices/worktree-removal-maps-leak.test.ts
- src/renderer/src/components/sidebar/WorktreeList.lineage-agent-expansion-coupling.test.tsx
- src/renderer/src/components/sidebar/worktree-list/grouping/build-rows.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-scanned-host-confirmation-removal.test.tsx
- src/renderer/src/store/slices/workspace-cleanup-removal-targets.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-facet-row-caches.ts
- src/renderer/src/components/sidebar/worktree-context-menu-delete-intent.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-facets.ts
- src/renderer/src/store/worktree-repo-index.ts
- src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-facet-rows.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal.test.ts
- src/main/worktree-removal-repo-owner.test.ts
- src/main/ipc/workspace-cleanup-folder-workspaces.ts
- src/renderer/src/store/slices/worktrees-removal-state-cleanup.test.ts
- src/main/runtime/rpc/methods/worktree-rm-host-qualification.test.ts
- src/renderer/src/components/sidebar/worktree-list/grouping/host-labels.ts
- src/main/runtime/repo-worktree-row-resolution.test.ts
- src/main/ipc/worktrees.ts
- src/renderer/src/store/slices/worktrees/teardown/dispatch-worktree-removal.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-git-evidence.test.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-host-identity.ts
- src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.mount-gating.test.tsx
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-ancestor-skips.ts
- src/renderer/src/store/slices/native-chat-launch-draft-teardown.test.ts
- src/main/runtime/rpc/methods/worktree.ts
- src/renderer/src/components/sidebar/use-confirmed-worktree-delete-targets.ts
- src/renderer/src/store/slices/workspace-cleanup-host-qualified-list-state.test.ts
- src/main/ipc/workspace-cleanup-disconnected-ssh.ts
- src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-git-evidence.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-query.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-timeout-recovery.test.ts
- src/main/runtime/repo-worktree-row-resolution.ts
- src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-removal.ts
- src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-git-evidence.test.tsx
- src/renderer/src/store/slices/worktrees/teardown/remove-worktree.ts
- src/renderer/src/components/sidebar/worktree-delete-request.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-git-evidence.ts
- src/main/persistence/loading-store/store.ts
- src/renderer/src/store/selectors.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-selection-model.ts
- src/renderer/src/store/slices/workspace-cleanup-removal-preflight.test.ts
- src/renderer/src/lib/worktree-operation-route.ts
- src/renderer/src/components/sidebar/ssh-host-remove-workspaces.ts
- src/main/runtime/orca-runtime.test.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-host-label.ts
- src/main/ipc/workspace-cleanup-broad-scan.test.ts
- src/main/worktree-metadata-ownership.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-confirm-remove.tsx
- src/main/worktree-removal-repo-owner.ts
- src/shared/worktree/removal.ts
- src/main/ipc/workspace-cleanup-scan.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-row-list.tsx
- src/shared/workspace-cleanup.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.tsx
- src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.tsx
- src/shared/workspace-cleanup-host-identity.ts
- src/renderer/src/components/sidebar/worktree-delete-execution.ts
- src/renderer/src/store/slices/workspace-cleanup.ts
- src/shared/worktree/host-qualified-identity.ts
- src/main/runtime/orca-runtime.ts
Included review availability: Your plan includes up to 10 reviews per rolling hour; 1 remains after this review.
| // Why: tabs persisted before viewMode existed default to 'terminal' so older sessions stay backward-compatible. | ||
| it('round-trips viewMode for unified tabs and defaults legacy tabs to terminal', async () => { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align the test title with the assertion.
The title states that legacy tabs default to terminal. The assertion at Line 129 requires viewMode to stay undefined. Persistence preserves absence; the renderer applies the fallback. Rename the test so it describes the persisted behavior.
🐛 Proposed fix
- // Why: tabs persisted before viewMode existed default to 'terminal' so older sessions stay backward-compatible.
- it('round-trips viewMode for unified tabs and defaults legacy tabs to terminal', async () => {
+ // Why: persistence must not invent a viewMode; the renderer treats absent as 'terminal'.
+ it('round-trips viewMode for unified tabs and leaves legacy tabs without one', async () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Why: tabs persisted before viewMode existed default to 'terminal' so older sessions stay backward-compatible. | |
| it('round-trips viewMode for unified tabs and defaults legacy tabs to terminal', async () => { | |
| // Why: persistence must not invent a viewMode; the renderer treats absent as 'terminal'. | |
| it('round-trips viewMode for unified tabs and leaves legacy tabs without one', async () => { |
| /** | ||
| * Shared pure utility that computes the ordered list of visible (non-archived, | ||
| * non-filtered) worktree IDs. Both the App-level Cmd+1–9 handler and | ||
| * WorktreeList's render pipeline consume this function so the numbering and | ||
| * card order can never diverge. | ||
| * | ||
| * Why a shared function: if the filter/sort pipeline lived in two places, a | ||
| * new filter added in one but not the other would silently break the mapping | ||
| * between badge numbers and the Cmd+N shortcut target. | ||
| */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the stale trailing comment.
Lines 69-78 describe an ID-ordering function that this module does not declare. Remove the comment or move it above the declared function it documents.
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
| useEffect(() => { | ||
| generationRef.current = generation | ||
| setStatusByIdentity(new Map()) | ||
| if (!isOpen) { | ||
| return | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
Scope the reset and refetch to generation.
The effect depends on repos and settings. Both are broad store objects. If any unrelated setting changes while the delete dialog is open, the effect re-runs. It then clears the whole map at Line 37 and re-issues one getRuntimeGitStatus call per target. The loaded dirty-change counts disappear from the dialog, and the IPC work repeats.
generation already encodes the target set, so it is the correct reset key. Read repos and settings inside the effect through useAppStore.getState(), as the code already does for gitStatusByWorktree and worktreesByRepo, and drop them from the dependency list.
♻️ Proposed change
useEffect(() => {
+ const { repos, settings } = useAppStore.getState()
generationRef.current = generation
setStatusByIdentity(new Map())- }, [deleteTargets, generation, isOpen, repoMap, repos, settings])
+ }, [deleteTargets, generation, isOpen, repoMap])Remove the now-unused repos and settings selector subscriptions at Lines 25-26.
Also applies to: 89-89
| const cleanupWorktreeIds = new Set( | ||
| Object.keys(cleanupPhases).map(getWorkspaceCleanupIdentityWorktreeId) | ||
| ) | ||
| const phases = { ...cleanupPhases } | ||
| for (const candidate of candidates) { | ||
| if (cleanupWorktreeIds.has(candidate.worktreeId)) { | ||
| continue | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Skip the candidate by identity, not by bare worktree id.
cleanupWorktreeIds collapses every cleanup-phase key to its bare worktree id. When two hosts publish the same worktreeId, one host's cleanup phase suppresses the generic-phase lookup for the other host's candidate. The second row then shows no deletion phase while its delete state is active.
Compare the candidate's full identity against the cleanup-phase keys instead.
🐛 Proposed fix
- const cleanupWorktreeIds = new Set(
- Object.keys(cleanupPhases).map(getWorkspaceCleanupIdentityWorktreeId)
- )
+ const cleanupIdentities = new Set(Object.keys(cleanupPhases))
const phases = { ...cleanupPhases }
for (const candidate of candidates) {
- if (cleanupWorktreeIds.has(candidate.worktreeId)) {
+ if (cleanupIdentities.has(getWorkspaceCleanupCandidateIdentity(candidate))) {
continue
}getWorkspaceCleanupIdentityWorktreeId then becomes unused in this file.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const cleanupWorktreeIds = new Set( | |
| Object.keys(cleanupPhases).map(getWorkspaceCleanupIdentityWorktreeId) | |
| ) | |
| const phases = { ...cleanupPhases } | |
| for (const candidate of candidates) { | |
| if (cleanupWorktreeIds.has(candidate.worktreeId)) { | |
| continue | |
| } | |
| const cleanupIdentities = new Set(Object.keys(cleanupPhases)) | |
| const phases = { ...cleanupPhases } | |
| for (const candidate of candidates) { | |
| if (cleanupIdentities.has(getWorkspaceCleanupCandidateIdentity(candidate))) { | |
| continue | |
| } |
| const nextDeleteState = { ...state.deleteStateByWorktreeId } | ||
| delete nextDeleteState[composeWorktreeHostIdentity(requiredExecutionHostId, worktreeId)] |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Also clear a compatible legacy delete-state key.
This deletes only the host-qualified key. markWorktreesDeleting still accepts bare worktree ids, so an in-flight delete can be recorded under the bare worktreeId. After a host-scoped drop, that bare entry survives. The surviving host's row then keeps rendering a deleting state that no operation will ever clear.
Delete the bare key when it belongs to the confirmed host or names no host. Do not delete it when it names a different host.
🐛 Proposed fix
const nextDeleteState = { ...state.deleteStateByWorktreeId }
delete nextDeleteState[composeWorktreeHostIdentity(requiredExecutionHostId, worktreeId)]
+ const legacyDeleteState = nextDeleteState[worktreeId]
+ if (
+ legacyDeleteState &&
+ (legacyDeleteState.executionHostId == null ||
+ legacyDeleteState.executionHostId === requiredExecutionHostId)
+ ) {
+ delete nextDeleteState[worktreeId]
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const nextDeleteState = { ...state.deleteStateByWorktreeId } | |
| delete nextDeleteState[composeWorktreeHostIdentity(requiredExecutionHostId, worktreeId)] | |
| const nextDeleteState = { ...state.deleteStateByWorktreeId } | |
| delete nextDeleteState[composeWorktreeHostIdentity(requiredExecutionHostId, worktreeId)] | |
| const legacyDeleteState = nextDeleteState[worktreeId] | |
| if ( | |
| legacyDeleteState && | |
| (legacyDeleteState.executionHostId == null || | |
| legacyDeleteState.executionHostId === requiredExecutionHostId) | |
| ) { | |
| delete nextDeleteState[worktreeId] | |
| } |
Review round 2 complete —
|
| clicked row | before | after |
|---|---|---|
qa-linux / qa-collision |
executionHostId: "local" ❌ |
executionHostId: "ssh:qa-linux" ✅ |
Local Mac / qa-collision |
— | executionHostId: "local" ✅ |
Deleting the qa-linux row removed only that row and left the local one standing. Zero console errors across the HMR round-trip.
Pinned-section root fix — my own probe, which failed against the earlier partial fix, now passes.
Its new pinned-section-worktrees.test.ts mutation-tested by me: 10/10 green with the fix; with the fix reverted, × does not include an unpinned row with the same id on another host — expected [ 'local', 'ssh:box' ] to deeply equal [ 'local' ]. The test is load-bearing.
The four PTY suites the report flagged as failing in isolation actually pass on this head (4 files, 56 tests). That was a load flake in a large batch run, not a baseline failure.
Mutation matrix
The initial pass over the whole production delta found 79 red, 7 weak/green across 86 changed/added test files. Five weak feature areas were strengthened and the sweep repeated: 84 red, 2 N/A. The two N/A files have no production-coupled assertion to mutate (one relocates an unchanged body for max-lines, one is deletion-only cleanup). Spot-checked: the Kanban lane-grid test gained a real assertion that the host-qualified identity propagates through virtualized lanes.
Gates — all green, confirmed by exit code
typecheck (0 errors) · audit:code-quality:native · audit:code-quality:type-aware (exit 0) · check:react-doctor:changed (exit 0) · check:max-lines-ratchet · check:reliability-gates · check:zustand-selector-fanout · verify:localization-coverage · changed-code quality 0 findings across 241 files.
Local suites on the integrated head: 342 files / 2,827 tests green across sidebar, status-bar, workspace-cleanup and cmd-j; the three jump-palette suites that were the only real CI failure are now 56/56 green.
Known gaps — reconfirmed, deliberately out of scope
Permissive collision-minting fallbacks in runtimeRepoMatchesExecutionHost; stale meta.hostId stranding persisted metadata/lineage; non-host-partitioned worktreeMeta (status/pin drag is another manifestation of this, not new scope); the repo-level proxy in hasWorktreeRemovalRepoOwnerOnOtherHost; the old-host CLI hard throw; bare-id create-worktree matching. Plus a non-destructive forced-reveal path in visible-worktrees.ts:161 left unchanged — its caller has no host identity to disambiguate, and fixing it needs a separate host-qualified agent-target contract. None can route or delete a workspace.
…5170) #15170 replaced the palette result model (singular matchedField/ranges -> plural arrays, options-object search signature), which is the same surface this PR's jump-palette change occupied. Took main's rewritten palette wholesale and re-applied only what is load-bearing here: - deleteActiveWorkspaceAction again passes expectedHostId, or a colliding id deletes whichever row wins first-wins. This is delete routing, the core of the PR, and taking main's side silently reverted it. - Dropped cmd-j/worktree-palette-item-id.* — orphaned by the rewrite. Palette item ids are bare again, which is main's own behaviour, so collision disambiguation there is deferred, not regressed.
#15170 replaced PaletteSearchResult while this PR was open. Re-applied the host qualification on top of the new model instead of taking either side: results carry worktreeHostId again, and the board filter keys its matched set on host identity rather than the bare id. Known gap, documented in the board test rather than deleted: searchWorktrees resolves evidence through a `documents` map keyed by BARE worktree id, so two same-id host rows collapse before this code sees them. Closing that belongs with the palette work.
|
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. |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/lib/worktree-palette-task-url-match.ts (1)
303-313: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPropagate the host ID on the Jira branch too.
The GitHub, Linear, and GitLab branches now pass
worktreeHostId. The final Jira branch at lines 317-321 still builds the result without it. A Jira-URL match therefore returns a host-less result, and two rows that sharerepoId::pathon different hosts collapse for every consumer that keys onworktreeHostId.🐛 Proposed fix for the Jira branch
return buildWorktreePaletteTaskUrlResult({ worktreeId: worktree.id, + ...(worktree.hostId ? { worktreeHostId: worktree.hostId } : {}), labelKind: 'issue', text: intent.parsed.issueKey })Consider extracting the repeated
...(worktree.hostId ? { worktreeHostId: worktree.hostId } : {})spread into one local constant so a future provider branch cannot omit it.
🧹 Nitpick comments (10)
src/main/persistence-native-chat-tab-view-mode.test.ts (1)
12-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the stale SSH-config comment and unused mocks.
The comment describes an "SSH-import test", but this suite contains only the
viewModeround-trip test. The./ssh/ssh-config-parsermock is also unused here. Drop both, or keep the mock and correct the comment to state why the real~/.ssh/configmust not be read duringStoreconstruction.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
src/renderer/src/store/slices/worktrees/teardown/remove-worktree.ts (1)
73-88: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueUse the host-aware lookup in the test harness.
worktree-list-lineage-card-test-harness.ts:114reads only the bare worktree ID, so list tests cannot represent host-qualified deletion state. Production readers already handle both key shapes.src/renderer/src/components/sidebar/worktree-list/rows/option-dom-host-collision.test.ts (1)
12-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClose the inner
divso the option rows stay siblings.
<div data-worktree-card-surface />is not a void element. The HTML parser treats it as an open tag, and the single following</div>closes it. Therole="option"elements therefore nest inside each other instead of forming siblings, which does not match the rendered sidebar. Any ancestor-based logic inoption-dom.ts(for exampleclosest('[role="option"]')) would then see a different tree than production.♻️ Proposed fixture fix
- <div role="option" data-worktree-id="shared" data-worktree-host-identity="local|shared" data-worktree-row-key="pinned:local|shared"><div data-worktree-card-surface /></div> - <div role="option" data-worktree-id="shared" data-worktree-host-identity="local|shared" data-worktree-row-key="all:local|shared"><div data-worktree-card-surface /></div> - <div role="option" data-worktree-id="shared" data-worktree-host-identity="ssh:host-b|shared" data-worktree-row-key="all:ssh:host-b|shared"><div data-worktree-card-surface /></div> + <div role="option" data-worktree-id="shared" data-worktree-host-identity="local|shared" data-worktree-row-key="pinned:local|shared"><div data-worktree-card-surface></div></div> + <div role="option" data-worktree-id="shared" data-worktree-host-identity="local|shared" data-worktree-row-key="all:local|shared"><div data-worktree-card-surface></div></div> + <div role="option" data-worktree-id="shared" data-worktree-host-identity="ssh:host-b|shared" data-worktree-row-key="all:ssh:host-b|shared"><div data-worktree-card-surface></div></div>src/renderer/src/store/slices/workspace-cleanup-wrong-host-removal.test.ts (1)
126-203: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftTwo parallel implementations of the same STA-4343 removal rig. Both files define host temp-directory creation, a module-level cleanup array, and a function named
installRemovalTransportsthat records routed host ids and deletes real directories. The duplication means a fix to the routing fake must be applied twice, and the two versions already differ (root-level delete versus path-scoped delete, object versus positional runtime-RPC arguments).
src/renderer/src/store/slices/workspace-cleanup-wrong-host-removal.test.ts#L126-L203: import the rig fromworktree-removal-host-collision-fixture.tsinstead of redefiningcreateHostDirectories,hostDirCleanup, andinstallRemovalTransports.src/renderer/src/store/slices/worktree-removal-host-collision-fixture.ts#L34-L89: extend the shared rig to cover the path-scoped delete and the runtime-RPC argument shape this test needs, so it can serve both suites.src/renderer/src/store/slices/worktree-removal-host-collision-fixture.ts (1)
64-79: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
deleteOnHostremoves the whole host root, so path-scoped routing is untestable.
fs.rmSync(root, ...)deletes the temp root itself. A test cannot distinguish "the correct workspace inside the host was removed" from "the whole host was wiped". If future cases need per-workspace granularity inside one host, pass the workspace-relative path and delete only that subtree, asworkspace-cleanup-wrong-host-removal.test.tsdoes withtoHostRelativePath.src/renderer/src/components/sidebar/worktree-list/navigation/active-descendant-option.test.ts (1)
23-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the mirrored local-host case.
The test only asserts the
ssh:boxrow. A lookup that returns the last matching row would also pass. Assert thatactiveWorkspaceExecutionHostId: 'local'resolves tolocal.rowKeyas well.♻️ Proposed additional assertion
).toBe(getWorktreeOptionId(ssh.rowKey)) + + expect( + getActiveDescendantOptionId({ + activeWorktreeId: 'shared', + activeWorkspaceExecutionHostId: 'local', + pinnedDisplayPolicy: 'single-location', + renderRows: [local, ssh], + virtualItems: [{ index: 0 }, { index: 1 }] + }) + ).toBe(getWorktreeOptionId(local.rowKey)) })src/renderer/src/components/sidebar/worktree-list/navigation/render-row-lookup.ts (1)
81-127: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider collapsing the two preferred-row lookups.
findPreferredRenderRowIndexForWorktreeandfindPreferredRenderRowIndexForWorktreeIdentityshare the same loop, fallback rule, and pinned-policy rule. Only the match predicate differs. One function with an injected predicate, or one function that accepts an optionalexecutionHostId, would remove the duplicate control flow.src/renderer/src/components/workspace-cleanup/workspace-cleanup-scanned-host-confirmation-removal.test.tsx (1)
55-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExplain the joined module path.
SCANNER_MODULE_PATHis assembled from an array to keep the specifier non-literal. The reason is not stated, so a later reader can "simplify" it to a literal and break thevi.importActualbehavior. Add a one-line comment that names the reason.src/renderer/src/store/slices/workspace-cleanup.ts (1)
419-435: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer building the removal entries once instead of mutating them.
The second pass reassigns
pendingRemoval.ignoreWorkspaceCleanupScanSurvivorsand callsdelete pendingRemoval.sameIdSurvivingHostId. Thedeleteoperator on a typed object is easy to misread and the entries are already local. Build the final entries in one pass afterscheduledRemovalIdentitiesis known.src/renderer/src/store/slices/workspace-cleanup-wrong-host-removal-guard.test.ts (1)
445-478: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the duplicate local-workspace test. Keep the SSH and snapshot tests because they cover distinct cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d66336f1-4961-4050-8f2f-b7b098a836c6
📒 Files selected for processing (242)
src/cli/handlers/worktree.tssrc/cli/index.test.tssrc/main/ipc/workspace-cleanup-broad-scan.test.tssrc/main/ipc/workspace-cleanup-disconnected-ssh.tssrc/main/ipc/workspace-cleanup-folder-workspaces.tssrc/main/ipc/workspace-cleanup-scan.tssrc/main/ipc/workspace-cleanup-snapshot-ipc.test.tssrc/main/ipc/workspace-cleanup.tssrc/main/ipc/worktrees-forget-local.test.tssrc/main/ipc/worktrees-preserved-branch-fork-remote.test.tssrc/main/ipc/worktrees-removal-recovery.test.tssrc/main/ipc/worktrees-remove-host-disambiguation.test.tssrc/main/ipc/worktrees.tssrc/main/persistence-host-partitioned-sessions.test.tssrc/main/persistence-native-chat-tab-view-mode.test.tssrc/main/persistence/loading-store/store.tssrc/main/runtime/orca-runtime.test.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/repo-worktree-row-resolution.test.tssrc/main/runtime/repo-worktree-row-resolution.tssrc/main/runtime/rpc/methods/workspace-cleanup-ui-schema.test.tssrc/main/runtime/rpc/methods/workspace-cleanup-ui-schema.tssrc/main/runtime/rpc/methods/worktree-rm-host-qualification.test.tssrc/main/runtime/rpc/methods/worktree-rm-pty-waiver.test.tssrc/main/runtime/rpc/methods/worktree-schemas.tssrc/main/runtime/rpc/methods/worktree.test.tssrc/main/runtime/rpc/methods/worktree.tssrc/main/runtime/runtime-folder-workspace.tssrc/main/worktree-metadata-ownership.tssrc/main/worktree-removal-repo-owner.test.tssrc/main/worktree-removal-repo-owner.tssrc/renderer/src/components/WorktreeJumpPalette.test.tsxsrc/renderer/src/components/WorktreeJumpPalette.tsxsrc/renderer/src/components/right-sidebar/HostedReviewActions.tsxsrc/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsxsrc/renderer/src/components/sidebar/DeleteWorktreeDialog.tsxsrc/renderer/src/components/sidebar/DeleteWorktreeLineageNotice.tsxsrc/renderer/src/components/sidebar/DeleteWorktreeTargetPreview.tsxsrc/renderer/src/components/sidebar/ForgetSshWorkspaceDialog.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanCard.host-identity.test.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanCard.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanDrawer.search.test.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanDrawer.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanLaneCardList.test.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanLaneCardList.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanLaneGrid.test.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanLaneGrid.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanStatusLane.test.tsxsrc/renderer/src/components/sidebar/WorkspaceKanbanStatusLane.tsxsrc/renderer/src/components/sidebar/WorktreeContextMenu.test.tssrc/renderer/src/components/sidebar/WorktreeContextMenu.tsxsrc/renderer/src/components/sidebar/WorktreeList.lineage-agent-expansion-coupling.test.tsxsrc/renderer/src/components/sidebar/WorktreeList.lineage-child-card.test.tssrc/renderer/src/components/sidebar/WorktreeList.lineage-child-real-card.test.tsxsrc/renderer/src/components/sidebar/WorktreeList.tsxsrc/renderer/src/components/sidebar/active-worktree-focus-after-delete.tssrc/renderer/src/components/sidebar/delete-worktree-dialog-force-delete.tssrc/renderer/src/components/sidebar/delete-worktree-dirty-change-counts.tssrc/renderer/src/components/sidebar/delete-worktree-flow.test.tssrc/renderer/src/components/sidebar/delete-worktree-flow.tssrc/renderer/src/components/sidebar/delete-worktree-lineage-delete-all.tssrc/renderer/src/components/sidebar/delete-worktree-parallel-flow.test.tssrc/renderer/src/components/sidebar/pinned-section-worktrees.test.tssrc/renderer/src/components/sidebar/pinned-section-worktrees.tssrc/renderer/src/components/sidebar/rendered-sidebar-worktree-order.test.tssrc/renderer/src/components/sidebar/rendered-sidebar-worktree-order.tssrc/renderer/src/components/sidebar/run-worktree-delete-with-toast.tssrc/renderer/src/components/sidebar/sidebar-filter-actions.tssrc/renderer/src/components/sidebar/ssh-host-remove-workspaces.tssrc/renderer/src/components/sidebar/use-confirmed-worktree-delete-targets.tssrc/renderer/src/components/sidebar/use-delete-worktree-status-hydration.tssrc/renderer/src/components/sidebar/use-visible-workspace-kanban-worktree-ids.test.tsxsrc/renderer/src/components/sidebar/use-visible-workspace-kanban-worktree-ids.tssrc/renderer/src/components/sidebar/use-workspace-kanban-card-pointer-drag.test.tssrc/renderer/src/components/sidebar/use-workspace-kanban-card-pointer-drag.tssrc/renderer/src/components/sidebar/use-workspace-kanban-selection.test.tsxsrc/renderer/src/components/sidebar/use-workspace-kanban-selection.tssrc/renderer/src/components/sidebar/use-worktree-card-activation-actions.tssrc/renderer/src/components/sidebar/use-worktree-card-foundation.tssrc/renderer/src/components/sidebar/use-worktree-card-workspace-actions.tssrc/renderer/src/components/sidebar/visible-worktree-host-scope.tssrc/renderer/src/components/sidebar/visible-worktree-kinds.tssrc/renderer/src/components/sidebar/visible-worktrees.tssrc/renderer/src/components/sidebar/workspace-delete-lineage.test.tssrc/renderer/src/components/sidebar/workspace-delete-lineage.tssrc/renderer/src/components/sidebar/workspace-kanban-card-drag-preview-dom.tssrc/renderer/src/components/sidebar/workspace-kanban-pointer-drag-selection.tssrc/renderer/src/components/sidebar/workspace-kanban-search.test.tssrc/renderer/src/components/sidebar/workspace-kanban-search.tssrc/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.tssrc/renderer/src/components/sidebar/workspace-kanban-virtual-lane-layout.tssrc/renderer/src/components/sidebar/workspace-kanban-worktree-groups.test.tssrc/renderer/src/components/sidebar/workspace-kanban-worktree-groups.tssrc/renderer/src/components/sidebar/worktree-card-model.tssrc/renderer/src/components/sidebar/worktree-context-menu-delete-intent.test.tssrc/renderer/src/components/sidebar/worktree-context-menu-delete-intent.tssrc/renderer/src/components/sidebar/worktree-delete-execution.tssrc/renderer/src/components/sidebar/worktree-delete-host-qualification.test.tssrc/renderer/src/components/sidebar/worktree-delete-request.test.tssrc/renderer/src/components/sidebar/worktree-delete-request.tssrc/renderer/src/components/sidebar/worktree-delete-state-host-match.tssrc/renderer/src/components/sidebar/worktree-keyboard-cycle.test.tssrc/renderer/src/components/sidebar/worktree-keyboard-cycle.tssrc/renderer/src/components/sidebar/worktree-list-card-markup-queries.tssrc/renderer/src/components/sidebar/worktree-list-groups-host-collision.test.tssrc/renderer/src/components/sidebar/worktree-list-groups-lineage-nesting.test.tssrc/renderer/src/components/sidebar/worktree-list/drag/use-pointer-drag.tssrc/renderer/src/components/sidebar/worktree-list/grouping/build-rows.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/pinned-group-rows.tssrc/renderer/src/components/sidebar/worktree-list/grouping/row-builders.tssrc/renderer/src/components/sidebar/worktree-list/listing/renderable-rows.tssrc/renderer/src/components/sidebar/worktree-list/listing/use-visible-worktrees.test.tsxsrc/renderer/src/components/sidebar/worktree-list/listing/use-visible-worktrees.tssrc/renderer/src/components/sidebar/worktree-list/navigation/active-descendant-option.test.tssrc/renderer/src/components/sidebar/worktree-list/navigation/active-descendant-option.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.tssrc/renderer/src/components/sidebar/worktree-list/navigation/use-active-row.tssrc/renderer/src/components/sidebar/worktree-list/navigation/use-keyboard.tssrc/renderer/src/components/sidebar/worktree-list/navigation/use-pending-reveal.tssrc/renderer/src/components/sidebar/worktree-list/navigation/use-reveal-requests.tssrc/renderer/src/components/sidebar/worktree-list/navigation/use-selection-host-collision.test.tsxsrc/renderer/src/components/sidebar/worktree-list/navigation/use-selection.tssrc/renderer/src/components/sidebar/worktree-list/rows/folder-row.tsxsrc/renderer/src/components/sidebar/worktree-list/rows/item-row.tsxsrc/renderer/src/components/sidebar/worktree-list/rows/option-dom-host-collision.test.tssrc/renderer/src/components/sidebar/worktree-list/rows/option-dom.tssrc/renderer/src/components/sidebar/worktree-list/viewport/VirtualizedWorktreeViewport.tsxsrc/renderer/src/components/sidebar/worktree-list/viewport/viewport-props.tssrc/renderer/src/components/sidebar/worktree-list/viewport/virtual-row-context.tssrc/renderer/src/components/sidebar/worktree-list/viewport/virtual-rows.tssrc/renderer/src/components/sidebar/worktree-sidebar-row-preference.tssrc/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsxsrc/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsxsrc/renderer/src/components/status-bar/resource-manager-worktree-target.test.tssrc/renderer/src/components/status-bar/resource-manager-worktree-target.tssrc/renderer/src/components/status-bar/workspace-space-delete-host-routing.test.tssrc/renderer/src/components/status-bar/workspace-space-delete-selection.tssrc/renderer/src/components/status-bar/workspace-space-presentation.test.tssrc/renderer/src/components/status-bar/workspace-space-presentation.tssrc/renderer/src/components/terminal-pane/TerminalSshReconnectOverlay.test.tsxsrc/renderer/src/components/terminal-pane/TerminalSshReconnectOverlay.tsxsrc/renderer/src/components/terminal-pane/pty-connection.tssrc/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.mount-gating.test.tsxsrc/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.stale-while-revalidate.test.tsxsrc/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.tsxsrc/renderer/src/components/workspace-cleanup/use-workspace-cleanup-dialog-lifecycle.tssrc/renderer/src/components/workspace-cleanup/use-workspace-cleanup-facet-rows.test.tsxsrc/renderer/src/components/workspace-cleanup/use-workspace-cleanup-facet-rows.tssrc/renderer/src/components/workspace-cleanup/use-workspace-cleanup-git-evidence.test.tsxsrc/renderer/src/components/workspace-cleanup/use-workspace-cleanup-git-evidence.tssrc/renderer/src/components/workspace-cleanup/use-workspace-cleanup-removal.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-ancestor-skips.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal-late-settlement.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal-snapshot-batch.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.test.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-confirm-remove.test.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-confirm-remove.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-deletion-phases.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-deletion-phases.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-facet-filter.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-facet-row-caches.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-facet.test.fixture.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-facets.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-flat-list.test.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-git-evidence.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-git-evidence.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-host-identity.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-host-label.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-post-batch-late-settlement.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-query.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-candidates.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-candidates.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-settlement.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-settlement.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-timeout-recovery.test.tssrc/renderer/src/components/workspace-cleanup/workspace-cleanup-row-list.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-scanned-host-confirmation-removal.test.tsxsrc/renderer/src/components/workspace-cleanup/workspace-cleanup-selection-model.tssrc/renderer/src/hooks/ipc-events-test-harness.tssrc/renderer/src/hooks/useIpcEvents-cmd-j-digit-chord.test.tssrc/renderer/src/hooks/useIpcEvents.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.tssrc/renderer/src/lib/ephemeral-vm-runtime-cleanup.tssrc/renderer/src/lib/workspace-activation-path-gate.test.tssrc/renderer/src/lib/worktree-activation-reveal.test.tssrc/renderer/src/lib/worktree-activation.tssrc/renderer/src/lib/worktree-operation-catalog-route.tssrc/renderer/src/lib/worktree-operation-route.tssrc/renderer/src/lib/worktree-palette-search.tssrc/renderer/src/lib/worktree-palette-task-url-match.test.tssrc/renderer/src/lib/worktree-palette-task-url-match.tssrc/renderer/src/lib/worktree-palette-task-url-result.tssrc/renderer/src/store/selectors.tssrc/renderer/src/store/slices/editor-state-worktree-purge-leak.test.tssrc/renderer/src/store/slices/generation-records-worktree-removal-leak.test.tssrc/renderer/src/store/slices/native-chat-launch-draft-teardown.test.tssrc/renderer/src/store/slices/renderer-remove-worktree-result.tssrc/renderer/src/store/slices/store-worktree-removal-cascade.test.tssrc/renderer/src/store/slices/tab-worktree-orphan-map-purge-leak.test.tssrc/renderer/src/store/slices/ui.tssrc/renderer/src/store/slices/workspace-cleanup-host-qualified-list-state.test.tssrc/renderer/src/store/slices/workspace-cleanup-removal-preflight.test.tssrc/renderer/src/store/slices/workspace-cleanup-removal-targets.tssrc/renderer/src/store/slices/workspace-cleanup-scan-progress.test.tssrc/renderer/src/store/slices/workspace-cleanup-wrong-host-removal-guard.test.tssrc/renderer/src/store/slices/workspace-cleanup-wrong-host-removal.test.tssrc/renderer/src/store/slices/workspace-cleanup.tssrc/renderer/src/store/slices/workspace-space.test.tssrc/renderer/src/store/slices/workspace-space.tssrc/renderer/src/store/slices/worktree-by-id-index.tssrc/renderer/src/store/slices/worktree-helpers.tssrc/renderer/src/store/slices/worktree-removal-host-collision-fixture.tssrc/renderer/src/store/slices/worktree-removal-maps-leak.test.tssrc/renderer/src/store/slices/worktree-removal-options.tssrc/renderer/src/store/slices/worktrees-remote-runtime-removal.test.tssrc/renderer/src/store/slices/worktrees-removal-state-cleanup.test.tssrc/renderer/src/store/slices/worktrees/listing/worktree-slice-constants.tssrc/renderer/src/store/slices/worktrees/teardown/dispatch-worktree-removal.tssrc/renderer/src/store/slices/worktrees/teardown/host-qualified-worktree-removal.tssrc/renderer/src/store/slices/worktrees/teardown/host-qualified-worktree-row-removal.tssrc/renderer/src/store/slices/worktrees/teardown/remove-worktree-store-cleanup.tssrc/renderer/src/store/slices/worktrees/teardown/remove-worktree.tssrc/renderer/src/store/slices/worktrees/teardown/removed-worktree-renderer-teardown.tssrc/renderer/src/store/slices/worktrees/teardown/removed-worktree-snapshot-prune.tssrc/renderer/src/store/slices/worktrees/teardown/worktree-delete-state.tssrc/renderer/src/store/slices/worktrees/teardown/worktree-purge-state.tssrc/renderer/src/store/worktree-host-collision-index.test.tssrc/renderer/src/store/worktree-repo-index.tssrc/shared/workspace-cleanup-host-identity.tssrc/shared/workspace-cleanup.tssrc/shared/worktree/host-qualified-identity.test.tssrc/shared/worktree/host-qualified-identity.tssrc/shared/worktree/removal.ts
💤 Files with no reviewable changes (1)
- src/main/runtime/rpc/methods/worktree.test.ts
🚧 Files skipped from review as they are similar to previous changes (214)
- src/renderer/src/store/slices/generation-records-worktree-removal-leak.test.ts
- src/renderer/src/components/sidebar/WorkspaceKanbanStatusLane.tsx
- src/renderer/src/components/status-bar/resource-manager-worktree-target.test.ts
- src/renderer/src/components/sidebar/DeleteWorktreeLineageNotice.tsx
- src/renderer/src/lib/workspace-activation-path-gate.test.ts
- src/main/ipc/worktrees-remove-host-disambiguation.test.ts
- src/renderer/src/components/sidebar/WorktreeContextMenu.test.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-facet-filter.test.ts
- src/renderer/src/lib/worktree-palette-task-url-match.test.ts
- src/renderer/src/i18n/locales/en.json
- src/renderer/src/store/slices/worktree-removal-maps-leak.test.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-facet.test.fixture.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-flat-list.test.tsx
- src/renderer/src/store/slices/workspace-cleanup-scan-progress.test.ts
- src/renderer/src/components/sidebar/worktree-list/rows/option-dom.ts
- src/renderer/src/components/sidebar/active-worktree-focus-after-delete.ts
- src/renderer/src/store/slices/worktrees/listing/worktree-slice-constants.ts
- src/main/runtime/rpc/methods/workspace-cleanup-ui-schema.ts
- src/main/runtime/rpc/methods/workspace-cleanup-ui-schema.test.ts
- src/main/ipc/workspace-cleanup-snapshot-ipc.test.ts
- src/main/ipc/worktrees-forget-local.test.ts
- src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.mount-gating.test.tsx
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-host-label.ts
- src/renderer/src/components/sidebar/worktree-list/listing/use-visible-worktrees.test.tsx
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-deletion-phases.test.ts
- src/renderer/src/store/slices/workspace-space.test.ts
- src/main/runtime/rpc/methods/worktree.ts
- src/renderer/src/components/sidebar/use-workspace-kanban-selection.ts
- src/renderer/src/components/sidebar/WorkspaceKanbanCard.tsx
- src/renderer/src/components/sidebar/worktree-list/grouping/group-keys.ts
- src/renderer/src/components/sidebar/worktree-list/navigation/use-selection-host-collision.test.tsx
- src/renderer/src/components/terminal-pane/TerminalSshReconnectOverlay.tsx
- src/renderer/src/components/sidebar/WorktreeList.lineage-child-card.test.ts
- src/renderer/src/components/sidebar/use-worktree-card-foundation.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-selection-model.ts
- src/renderer/src/components/sidebar/workspace-delete-lineage.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-confirm-remove.tsx
- src/main/runtime/rpc/methods/worktree-schemas.ts
- src/renderer/src/components/sidebar/use-visible-workspace-kanban-worktree-ids.test.tsx
- src/main/runtime/runtime-folder-workspace.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal-snapshot-batch.test.ts
- src/shared/worktree/host-qualified-identity.test.ts
- src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx
- src/renderer/src/components/sidebar/worktree-list/viewport/virtual-row-context.ts
- src/cli/handlers/worktree.ts
- src/renderer/src/components/sidebar/use-visible-workspace-kanban-worktree-ids.ts
- src/renderer/src/store/slices/worktrees/teardown/worktree-purge-state.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-facet-row-caches.ts
- src/renderer/src/components/sidebar/WorktreeList.lineage-child-real-card.test.tsx
- src/renderer/src/components/sidebar/workspace-delete-lineage.test.ts
- src/renderer/src/components/sidebar/delete-worktree-lineage-delete-all.ts
- src/renderer/src/components/sidebar/worktree-list/viewport/viewport-props.ts
- src/main/ipc/worktrees-removal-recovery.test.ts
- src/renderer/src/components/WorktreeJumpPalette.tsx
- src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-facet-rows.test.tsx
- src/renderer/src/components/sidebar/worktree-list/listing/renderable-rows.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-deletion-phases.ts
- src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-dialog-lifecycle.ts
- src/renderer/src/components/terminal-pane/TerminalSshReconnectOverlay.test.tsx
- src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-git-evidence.test.tsx
- src/renderer/src/lib/worktree-activation-reveal.test.ts
- src/main/ipc/workspace-cleanup-folder-workspaces.ts
- src/main/ipc/workspace-cleanup-scan.ts
- src/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.ts
- src/renderer/src/components/sidebar/worktree-list/viewport/virtual-rows.ts
- src/main/persistence-host-partitioned-sessions.test.ts
- src/renderer/src/components/sidebar/worktree-list-groups-lineage-nesting.test.ts
- src/renderer/src/components/sidebar/rendered-sidebar-worktree-order.ts
- src/renderer/src/components/sidebar/workspace-kanban-worktree-groups.test.ts
- src/renderer/src/components/sidebar/worktree-list/grouping/group-sections.ts
- src/renderer/src/components/status-bar/workspace-space-delete-host-routing.test.ts
- src/renderer/src/store/worktree-repo-index.ts
- src/renderer/src/components/sidebar/delete-worktree-dirty-change-counts.ts
- src/renderer/src/components/status-bar/resource-manager-worktree-target.ts
- src/shared/worktree/removal.ts
- src/renderer/src/store/slices/worktrees/teardown/remove-worktree-store-cleanup.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-git-evidence.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-post-batch-late-settlement.ts
- src/renderer/src/store/slices/worktrees/teardown/removed-worktree-snapshot-prune.ts
- src/renderer/src/components/sidebar/worktree-sidebar-row-preference.ts
- src/renderer/src/store/slices/editor-state-worktree-purge-leak.test.ts
- src/renderer/src/components/sidebar/worktree-delete-request.test.ts
- src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts
- src/renderer/src/components/sidebar/workspace-kanban-worktree-groups.ts
- src/renderer/src/components/sidebar/worktree-list/listing/use-visible-worktrees.ts
- src/renderer/src/components/sidebar/use-delete-worktree-status-hydration.ts
- src/renderer/src/store/slices/workspace-cleanup-removal-preflight.test.ts
- src/renderer/src/components/sidebar/worktree-list/grouping/pinned-group-rows.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.test.tsx
- src/renderer/src/components/sidebar/WorkspaceKanbanDrawer.search.test.tsx
- src/renderer/src/store/slices/worktrees/teardown/removed-worktree-renderer-teardown.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-git-evidence.test.ts
- src/renderer/src/components/sidebar/worktree-list-groups-host-collision.test.ts
- src/renderer/src/components/sidebar/use-worktree-card-workspace-actions.ts
- src/main/ipc/workspace-cleanup-disconnected-ssh.ts
- src/renderer/src/store/slices/worktree-by-id-index.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-facets.ts
- src/renderer/src/components/sidebar/worktree-list/viewport/VirtualizedWorktreeViewport.tsx
- src/renderer/src/components/sidebar/worktree-keyboard-cycle.test.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-confirm-remove.test.tsx
- src/renderer/src/store/selectors.ts
- src/renderer/src/components/status-bar/workspace-space-presentation.test.ts
- src/renderer/src/components/sidebar/worktree-keyboard-cycle.ts
- src/main/ipc/workspace-cleanup.ts
- src/main/persistence/loading-store/store.ts
- src/renderer/src/lib/worktree-operation-route.ts
- src/renderer/src/hooks/useIpcEvents-cmd-j-digit-chord.test.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-candidates.ts
- src/renderer/src/store/slices/renderer-remove-worktree-result.ts
- src/renderer/src/hooks/useIpcEvents.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-ancestor-skips.ts
- src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.stale-while-revalidate.test.tsx
- src/shared/workspace-cleanup.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-candidates.test.ts
- src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-removal.ts
- src/renderer/src/components/sidebar/worktree-delete-host-qualification.test.ts
- src/renderer/src/components/terminal-pane/pty-connection.ts
- src/cli/index.test.ts
- src/renderer/src/store/slices/worktrees/teardown/host-qualified-worktree-row-removal.ts
- src/main/ipc/worktrees.ts
- src/renderer/src/lib/ephemeral-vm-runtime-cleanup.ts
- src/renderer/src/components/sidebar/rendered-sidebar-worktree-order.test.ts
- src/renderer/src/components/sidebar/visible-worktree-host-scope.ts
- src/renderer/src/components/sidebar/use-confirmed-worktree-delete-targets.ts
- src/renderer/src/components/sidebar/sidebar-filter-actions.ts
- src/main/ipc/worktrees-preserved-branch-fork-remote.test.ts
- src/renderer/src/components/sidebar/worktree-delete-request.ts
- src/renderer/src/components/right-sidebar/HostedReviewActions.tsx
- src/renderer/src/store/slices/store-worktree-removal-cascade.test.ts
- src/renderer/src/store/slices/worktree-removal-options.ts
- src/renderer/src/components/sidebar/worktree-list/navigation/active-descendant-option.ts
- src/renderer/src/store/slices/worktrees-removal-state-cleanup.test.ts
- src/renderer/src/components/sidebar/DeleteWorktreeTargetPreview.tsx
- src/renderer/src/components/sidebar/WorkspaceKanbanLaneCardList.tsx
- src/renderer/src/components/sidebar/worktree-list/navigation/folder-reveal.ts
- src/renderer/src/components/sidebar/visible-worktree-kinds.ts
- src/renderer/src/components/sidebar/worktree-list/navigation/use-selection.ts
- src/renderer/src/components/sidebar/delete-worktree-flow.test.ts
- src/renderer/src/components/sidebar/WorktreeList.lineage-agent-expansion-coupling.test.tsx
- src/renderer/src/components/sidebar/use-workspace-kanban-card-pointer-drag.ts
- src/renderer/src/store/slices/native-chat-launch-draft-teardown.test.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.tsx
- src/renderer/src/components/sidebar/worktree-delete-execution.ts
- src/renderer/src/components/sidebar/delete-worktree-parallel-flow.test.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-timeout-recovery.test.ts
- src/renderer/src/store/slices/worktrees/teardown/host-qualified-worktree-removal.ts
- src/main/worktree-removal-repo-owner.test.ts
- src/renderer/src/components/sidebar/workspace-kanban-search.ts
- src/main/runtime/repo-worktree-row-resolution.test.ts
- src/renderer/src/store/slices/workspace-cleanup-removal-targets.ts
- src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx
- src/main/runtime/orca-runtime.test.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-settlement.test.ts
- src/renderer/src/store/slices/tab-worktree-orphan-map-purge-leak.test.ts
- src/renderer/src/components/sidebar/WorktreeList.tsx
- src/renderer/src/store/slices/workspace-cleanup-host-qualified-list-state.test.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal-late-settlement.test.ts
- src/renderer/src/hooks/ipc-events-test-harness.ts
- src/renderer/src/components/status-bar/workspace-space-delete-selection.ts
- src/renderer/src/components/sidebar/delete-worktree-dialog-force-delete.ts
- src/main/runtime/rpc/methods/worktree-rm-host-qualification.test.ts
- src/renderer/src/components/sidebar/workspace-kanban-virtual-lane-layout.ts
- src/renderer/src/components/sidebar/worktree-delete-state-host-match.ts
- src/renderer/src/components/sidebar/ForgetSshWorkspaceDialog.tsx
- src/renderer/src/store/slices/workspace-space.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-query.ts
- src/renderer/src/lib/worktree-operation-catalog-route.ts
- src/renderer/src/components/sidebar/worktree-list/navigation/use-keyboard.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-removal-settlement.ts
- src/renderer/src/components/sidebar/worktree-list/navigation/use-pending-reveal.ts
- src/renderer/src/components/status-bar/workspace-space-presentation.ts
- src/main/ipc/workspace-cleanup-broad-scan.test.ts
- src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx
- src/renderer/src/components/sidebar/use-workspace-kanban-selection.test.tsx
- src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-git-evidence.ts
- src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-facet-rows.ts
- src/renderer/src/components/sidebar/use-worktree-card-activation-actions.ts
- src/renderer/src/components/sidebar/workspace-kanban-card-drag-preview-dom.ts
- src/main/runtime/repo-worktree-row-resolution.ts
- src/renderer/src/store/slices/ui.ts
- src/renderer/src/components/sidebar/WorkspaceKanbanDrawer.tsx
- src/renderer/src/components/sidebar/worktree-list/navigation/use-active-row.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal.ts
- src/renderer/src/components/sidebar/workspace-kanban-pointer-drag-selection.ts
- src/renderer/src/components/sidebar/delete-worktree-flow.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-host-identity.ts
- src/renderer/src/store/slices/worktrees/teardown/worktree-delete-state.ts
- src/renderer/src/components/sidebar/worktree-card-model.ts
- src/renderer/src/components/sidebar/ssh-host-remove-workspaces.ts
- src/renderer/src/store/slices/worktrees-remote-runtime-removal.test.ts
- src/renderer/src/store/worktree-host-collision-index.test.ts
- src/shared/worktree/host-qualified-identity.ts
- src/renderer/src/components/sidebar/WorkspaceKanbanLaneCardList.test.tsx
- src/renderer/src/components/sidebar/visible-worktrees.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-background-removal.test.ts
- src/renderer/src/components/sidebar/use-workspace-kanban-card-pointer-drag.test.ts
- src/main/worktree-removal-repo-owner.ts
- src/renderer/src/components/sidebar/worktree-list/rows/item-row.tsx
- src/renderer/src/store/slices/worktree-helpers.ts
- src/renderer/src/components/sidebar/WorkspaceKanbanCard.host-identity.test.tsx
- src/renderer/src/components/sidebar/worktree-list/navigation/use-reveal-requests.ts
- src/renderer/src/components/sidebar/worktree-list/drag/use-pointer-drag.ts
- src/renderer/src/components/sidebar/WorkspaceKanbanLaneGrid.tsx
- src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.tsx
- src/renderer/src/components/sidebar/worktree-list/navigation/pending-reveal-inputs.ts
- src/renderer/src/lib/worktree-activation.ts
- src/shared/workspace-cleanup-host-identity.ts
- src/renderer/src/store/slices/worktrees/teardown/dispatch-worktree-removal.ts
- src/renderer/src/components/workspace-cleanup/workspace-cleanup-row-list.tsx
- src/main/runtime/orca-runtime.ts
- src/renderer/src/components/sidebar/worktree-list/grouping/host-labels.ts
- src/renderer/src/components/sidebar/worktree-list/grouping/row-builders.ts
- src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx
- src/renderer/src/components/sidebar/worktree-list/rows/folder-row.tsx
Included review availability: Your plan includes up to 10 reviews per rolling hour; 6 remain after this review.
| const commitFocus = prepareActiveWorktreeFocusAfterDelete(worktreeId) | ||
| const focusSuccessor = options.focusSuccessorOnDelete !== false |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
ast-grep outline src/renderer/src/components/sidebar/active-worktree-focus-after-delete.ts --items all
rg -n -C 5 'prepareActiveWorktreeFocusAfterDelete\s*\(' src/renderer/src/components/sidebar
rg -n -C 5 'activeWorktree(Id|Host)|setActiveWorktree' \
src/renderer/src/components/sidebar/active-worktree-focus-after-delete.tsRepository: stablyai/orca
Length of output: 24818
🏁 Script executed:
#!/bin/bash
set -eu
cat -n src/renderer/src/components/sidebar/active-worktree-focus-after-delete.ts | sed -n '1,125p'
printf '\n-- target and host identity definitions/usages --\n'
rg -n -C 4 'type .*Worktree.*Target|interface .*Worktree.*Target|executionHostId|hostId|getWorktreeHostIdentity|prepareActiveWorktreeFocusAfterDelete' \
src/renderer/src/components/sidebar/run-worktree-delete-with-toast.ts \
src/renderer/src/components/sidebar/worktree-delete-execution.ts \
src/renderer/src/components/sidebar/delete-worktree-dialog-force-delete.ts \
src/renderer/src/components/sidebar/active-worktree-focus-after-delete.test.tsRepository: stablyai/orca
Length of output: 47425
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '-- selectors and worktree identity --'
rg -n -C 8 'function getWorktreeMapFromState|const getWorktreeMapFromState|worktreeMap|worktreesByRepo|activeWorktreeId' \
src/renderer/src/store src/renderer/src/lib src/shared \
-g '*.ts' -g '*.tsx' | head -n 240
printf '%s\n' '-- host-qualified identity helpers --'
fd -i 'host-qualified-identity|activation' src/renderer/src shared
rg -n -C 8 'composeWorktreeHostIdentity|getWorktreeHostIdentity|function activateAndRevealWorktree|const activateAndRevealWorktree' \
src/renderer/src shared -g '*.ts' -g '*.tsx' | head -n 240
printf '%s\n' '-- removal target definition --'
cat -n src/shared/worktree/removal.ts | sed -n '1,180p'Repository: stablyai/orca
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -eu
cat -n src/renderer/src/store/worktree-repo-index.ts | sed -n '24,78p'
rg -n -C 10 'export function activateAndRevealWorktree|function activateAndRevealWorktree' \
src/renderer/src/lib/worktree-activation.ts
cat -n src/shared/worktree/removal.ts | sed -n '1,100p'
python3 - <<'PY'
# Read-only model of the relevant selector behavior with a same-ID host collision.
rows = [
{"id": "repo::/shared", "hostId": "local", "repoId": "repo"},
{"id": "repo::/shared", "hostId": "ssh:box", "repoId": "repo"},
]
bare_map = {}
for row in rows:
bare_map[row["id"]] = row
deleted = rows[0]
active_id = rows[0]["id"]
active_host = rows[0]["hostId"]
print("collision_rows:", [(r["id"], r["hostId"]) for r in rows])
print("bare_map_resolution:", bare_map[deleted["id"]]["hostId"])
print("raw_active_id_match:", active_id == deleted["id"])
print("host_qualified_active_match:", active_id == deleted["id"] and active_host == deleted["hostId"])
print("host_qualified_other_row_match:", active_id == deleted["id"] and active_host == rows[1]["hostId"])
PYRepository: stablyai/orca
Length of output: 9159
Make focus restoration host-qualified.
The store supports distinct worktrees with the same ID on different hosts, but this helper uses the legacy bare-ID map and worktree.id !== deletedWorktreeId. Pass the target host identity through prepareActiveWorktreeFocusAfterDelete and compare host-qualified identities when selecting the successor.
| let prunableWorktreeIds = new Set(removedIds) | ||
| set((state) => { | ||
| const remainingCandidates = state.workspaceCleanupScan?.candidates.filter( | ||
| (candidate) => !removedIdentities.has(getWorkspaceCleanupCandidateIdentity(candidate)) | ||
| ) | ||
| })) | ||
| void window.api.workspaceCleanup | ||
| .dismiss({ dismissals: [], removedWorktreeIds: removedIds }) | ||
| .catch((error: unknown) => { | ||
| console.warn('Failed to prune persisted cleanup dismissals', error) | ||
| }) | ||
| // Why: a same-id row on another host survives this removal, and its | ||
| // dismissal/viewed marks are keyed by worktree id alone — dropping them | ||
| // would resurrect a row the user already ignored. | ||
| const survivingWorktreeIds = new Set( | ||
| (remainingCandidates ?? []).map((candidate) => candidate.worktreeId) | ||
| ) | ||
| prunableWorktreeIds = new Set( | ||
| removedIds.filter((worktreeId) => !survivingWorktreeIds.has(worktreeId)) | ||
| ) | ||
| return { | ||
| workspaceCleanupLoading: false, | ||
| workspaceCleanupScan: | ||
| state.workspaceCleanupScan && remainingCandidates | ||
| ? { ...state.workspaceCleanupScan, candidates: remainingCandidates } | ||
| : state.workspaceCleanupScan, | ||
| // Why: dismissals and viewed marks for removed worktrees are dead | ||
| // weight in the store and in every persisted-dismissals write. | ||
| workspaceCleanupDismissals: pruneWorkspaceCleanupDismissals( | ||
| state.workspaceCleanupDismissals, | ||
| prunableWorktreeIds | ||
| ), | ||
| workspaceCleanupViewedCandidates: pruneWorkspaceCleanupRecord( | ||
| state.workspaceCleanupViewedCandidates, | ||
| prunableWorktreeIds | ||
| ) | ||
| } | ||
| }) | ||
| if (prunableWorktreeIds.size > 0) { | ||
| void window.api.workspaceCleanup | ||
| .dismiss({ dismissals: [], removedWorktreeIds: [...prunableWorktreeIds] }) | ||
| .catch((error: unknown) => { | ||
| console.warn('Failed to prune persisted cleanup dismissals', error) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Do not prune host-scoped metadata when the scan is absent.
remainingCandidates is undefined when state.workspaceCleanupScan is null. survivingWorktreeIds is then empty, so prunableWorktreeIds becomes every removed ID. Dismissals and viewed marks for a same-ID row on another host are pruned, and the persisted-dismissal prune IPC fires for them. That resurrects a row the user already ignored, which is the case the comment on lines 494-496 protects.
Skip pruning when there is no scan to prove that no same-ID row survives.
Separately, prunableWorktreeIds is assigned inside the set updater and read after it. The updater should stay pure. Compute the surviving IDs from get() before set.
🛡️ Proposed fix
- let prunableWorktreeIds = new Set(removedIds)
- set((state) => {
- const remainingCandidates = state.workspaceCleanupScan?.candidates.filter(
- (candidate) => !removedIdentities.has(getWorkspaceCleanupCandidateIdentity(candidate))
- )
- // Why: a same-id row on another host survives this removal, and its
- // dismissal/viewed marks are keyed by worktree id alone — dropping them
- // would resurrect a row the user already ignored.
- const survivingWorktreeIds = new Set(
- (remainingCandidates ?? []).map((candidate) => candidate.worktreeId)
- )
- prunableWorktreeIds = new Set(
- removedIds.filter((worktreeId) => !survivingWorktreeIds.has(worktreeId))
- )
- return {
+ const scanBeforePrune = get().workspaceCleanupScan
+ const remainingCandidates = scanBeforePrune?.candidates.filter(
+ (candidate) => !removedIdentities.has(getWorkspaceCleanupCandidateIdentity(candidate))
+ )
+ // Why: a same-id row on another host survives this removal, and its
+ // dismissal/viewed marks are keyed by worktree id alone — dropping them
+ // would resurrect a row the user already ignored. Without a scan we
+ // cannot prove no such row exists, so prune nothing.
+ const survivingWorktreeIds = new Set(
+ (remainingCandidates ?? []).map((candidate) => candidate.worktreeId)
+ )
+ const prunableWorktreeIds = remainingCandidates
+ ? new Set(removedIds.filter((worktreeId) => !survivingWorktreeIds.has(worktreeId)))
+ : new Set<string>()
+ set((state) => {
+ return {
workspaceCleanupLoading: false,
workspaceCleanupScan:
state.workspaceCleanupScan && remainingCandidates
? { ...state.workspaceCleanupScan, candidates: remainingCandidates }
: state.workspaceCleanupScan,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let prunableWorktreeIds = new Set(removedIds) | |
| set((state) => { | |
| const remainingCandidates = state.workspaceCleanupScan?.candidates.filter( | |
| (candidate) => !removedIdentities.has(getWorkspaceCleanupCandidateIdentity(candidate)) | |
| ) | |
| })) | |
| void window.api.workspaceCleanup | |
| .dismiss({ dismissals: [], removedWorktreeIds: removedIds }) | |
| .catch((error: unknown) => { | |
| console.warn('Failed to prune persisted cleanup dismissals', error) | |
| }) | |
| // Why: a same-id row on another host survives this removal, and its | |
| // dismissal/viewed marks are keyed by worktree id alone — dropping them | |
| // would resurrect a row the user already ignored. | |
| const survivingWorktreeIds = new Set( | |
| (remainingCandidates ?? []).map((candidate) => candidate.worktreeId) | |
| ) | |
| prunableWorktreeIds = new Set( | |
| removedIds.filter((worktreeId) => !survivingWorktreeIds.has(worktreeId)) | |
| ) | |
| return { | |
| workspaceCleanupLoading: false, | |
| workspaceCleanupScan: | |
| state.workspaceCleanupScan && remainingCandidates | |
| ? { ...state.workspaceCleanupScan, candidates: remainingCandidates } | |
| : state.workspaceCleanupScan, | |
| // Why: dismissals and viewed marks for removed worktrees are dead | |
| // weight in the store and in every persisted-dismissals write. | |
| workspaceCleanupDismissals: pruneWorkspaceCleanupDismissals( | |
| state.workspaceCleanupDismissals, | |
| prunableWorktreeIds | |
| ), | |
| workspaceCleanupViewedCandidates: pruneWorkspaceCleanupRecord( | |
| state.workspaceCleanupViewedCandidates, | |
| prunableWorktreeIds | |
| ) | |
| } | |
| }) | |
| if (prunableWorktreeIds.size > 0) { | |
| void window.api.workspaceCleanup | |
| .dismiss({ dismissals: [], removedWorktreeIds: [...prunableWorktreeIds] }) | |
| .catch((error: unknown) => { | |
| console.warn('Failed to prune persisted cleanup dismissals', error) | |
| }) | |
| } | |
| const scanBeforePrune = get().workspaceCleanupScan | |
| const remainingCandidates = scanBeforePrune?.candidates.filter( | |
| (candidate) => !removedIdentities.has(getWorkspaceCleanupCandidateIdentity(candidate)) | |
| ) | |
| // Why: a same-id row on another host survives this removal, and its | |
| // dismissal/viewed marks are keyed by worktree id alone — dropping them | |
| // would resurrect a row the user already ignored. Without a scan we | |
| // cannot prove no such row exists, so prune nothing. | |
| const survivingWorktreeIds = new Set( | |
| (remainingCandidates ?? []).map((candidate) => candidate.worktreeId) | |
| ) | |
| const prunableWorktreeIds = remainingCandidates | |
| ? new Set(removedIds.filter((worktreeId) => !survivingWorktreeIds.has(worktreeId))) | |
| : new Set<string>() | |
| set((state) => { | |
| return { | |
| workspaceCleanupLoading: false, | |
| workspaceCleanupScan: | |
| state.workspaceCleanupScan && remainingCandidates | |
| ? { ...state.workspaceCleanupScan, candidates: remainingCandidates } | |
| : state.workspaceCleanupScan, | |
| // Why: dismissals and viewed marks for removed worktrees are dead | |
| // weight in the store and in every persisted-dismissals write. | |
| workspaceCleanupDismissals: pruneWorkspaceCleanupDismissals( | |
| state.workspaceCleanupDismissals, | |
| prunableWorktreeIds | |
| ), | |
| workspaceCleanupViewedCandidates: pruneWorkspaceCleanupRecord( | |
| state.workspaceCleanupViewedCandidates, | |
| prunableWorktreeIds | |
| ) | |
| } | |
| }) | |
| if (prunableWorktreeIds.size > 0) { | |
| void window.api.workspaceCleanup | |
| .dismiss({ dismissals: [], removedWorktreeIds: [...prunableWorktreeIds] }) | |
| .catch((error: unknown) => { | |
| console.warn('Failed to prune persisted cleanup dismissals', error) | |
| }) | |
| } |
…ld model The palette collision test asserted two host-qualified rows, which #15170's rewrite made unreachable: item ids are bare again and worktreeMap is id-keyed. Rewritten to assert what holds — activation always names a host — and to pin the defect it exposes: two same-id rows render on ONE command value, so React sees duplicate keys and a click on the first row activates the second row's host. That reproduces on main, so it is pre-existing, not from this PR. Pinned rather than deleted so fixing it must update this test.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/components/WorktreeJumpPalette.test.tsx (1)
364-402: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftAssert host-specific activation for each collision row.
worktreeItemsresolves every match through the bare-idworktreeMap, whosesetcall overwrites the local worktree with the SSH worktree. Both rows therefore capture SSH before React reconciliation. Preserve the matched host in each item and assert that the local and SSH rows activate with their respectiveexecutionHostIdvalues.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0674eb07-ff96-4d64-8d81-e3fc4c493cbe
📒 Files selected for processing (1)
src/renderer/src/components/WorktreeJumpPalette.test.tsx
Included review availability: Your plan includes up to 10 reviews per rolling hour; 5 remain after this review.
* fix(sidebar): clear the delete state when a removal is refused Callers mark rows deleting up front for immediate sidebar feedback, but a refusal in beginHostQualifiedRemoval returns before removeWorktree's try/catch — the only other place that clears the flag. The failure toast auto-dismisses after 10s, so the workspace was left on a 'Deleting…' spinner indefinitely with no explanation still on screen. Also removes workspace-cleanup-removal-host-guard.ts: #14731 refused a colliding cleanup removal, #15013 replaced refusing with routing to the confirmed host, and nothing imports the module any more — not even its own tests. * test(sidebar): pin the two states that actually reach the removal refusal The existing case used an empty store, which proved the clear but not that a user can land there. Cover a stale folder-workspace id and a hostless row under an ambiguous legacy runtime — both resolve to no route from populated state. --------- Co-authored-by: QA <qa@local>
…ows selectable (STA-4343) (stablyai#15013) * fix(workspaces): host-qualified workspace deletion (STA-4343, STA-4448) Squashed integration of PR stablyai#14606 + the codex review-loop output, replayed onto current main. Granular history preserved on brennanb2025/sta-4343-review-full. Fixes the regression from stablyai#13413: a workspace id is repoId::path with no host component, so the same repo at the same path on two hosts published one id for two workspaces, and deletion routed by that id landed on whichever host routing preferred - usually the ACTIVE one, not the row the user confirmed. - removeWorktree takes a REQUIRED host-qualified WorktreeRemovalTarget; omitting the host is a type error. All destructive callers migrated. - Projections dedup on (host, id), so two hosts render as two selectable rows while the createWorktree/fetchWorktrees race duplicate still collapses. - Ephemeral VM cleanup is host-scoped. It matched on bare workspaceId, so the host-scoped delete path destroyed the SURVIVING host's VM and its unpushed filesystem - a leak fix that had become data destruction. - Selection, keyboard routing, lineage grouping and Space rows carry host identity end to end; fixing the executor dedupe alone would have turned one-row intent into deleting both hosts. Files split to stay under max-lines rather than raising any cap. * refactor: split files that crossed max-lines The review-loop commits used --no-verify, so the pre-commit hook never enforced the caps. Extracted cohesive units rather than raising any limit: renderer teardown, delete-with-toast, pinned-group rows, host-scope helpers, workspace-kind predicates, filter actions, kanban drag selection, the renderer removal result type, and the native-chat persistence tests. * refactor(workspaces): extract cleanup deletion-phase selector Clears the last max-lines violation and the import-type side effect the changed-code gate flagged. * refactor(sidebar): track the delete-dialog extraction modules * fix(workspaces): preserve host identity across remaining surfaces * fix(sidebar): re-carry host through the rewritten palette result model stablyai#15170 replaced PaletteSearchResult while this PR was open. Re-applied the host qualification on top of the new model instead of taking either side: results carry worktreeHostId again, and the board filter keys its matched set on host identity rather than the bare id. Known gap, documented in the board test rather than deleted: searchWorktrees resolves evidence through a `documents` map keyed by BARE worktree id, so two same-id host rows collapse before this code sees them. Closing that belongs with the palette work. * test(cmd-j): pin the palette collision gap instead of asserting the old model The palette collision test asserted two host-qualified rows, which stablyai#15170's rewrite made unreachable: item ids are bare again and worktreeMap is id-keyed. Rewritten to assert what holds — activation always names a host — and to pin the defect it exposes: two same-id rows render on ONE command value, so React sees duplicate keys and a click on the first row activates the second row's host. That reproduces on main, so it is pre-existing, not from this PR. Pinned rather than deleted so fixing it must update this test. --------- Co-authored-by: QA <qa@local>
…yai#15187) * fix(sidebar): clear the delete state when a removal is refused Callers mark rows deleting up front for immediate sidebar feedback, but a refusal in beginHostQualifiedRemoval returns before removeWorktree's try/catch — the only other place that clears the flag. The failure toast auto-dismisses after 10s, so the workspace was left on a 'Deleting…' spinner indefinitely with no explanation still on screen. Also removes workspace-cleanup-removal-host-guard.ts: stablyai#14731 refused a colliding cleanup removal, stablyai#15013 replaced refusing with routing to the confirmed host, and nothing imports the module any more — not even its own tests. * test(sidebar): pin the two states that actually reach the removal refusal The existing case used an empty store, which proved the clear but not that a user can land there. Cover a stale folder-workspace id and a hostless row under an ambiguous legacy runtime — both resolve to no route from populated state. --------- Co-authored-by: QA <qa@local>
…yai#15187) * fix(sidebar): clear the delete state when a removal is refused Callers mark rows deleting up front for immediate sidebar feedback, but a refusal in beginHostQualifiedRemoval returns before removeWorktree's try/catch — the only other place that clears the flag. The failure toast auto-dismisses after 10s, so the workspace was left on a 'Deleting…' spinner indefinitely with no explanation still on screen. Also removes workspace-cleanup-removal-host-guard.ts: stablyai#14731 refused a colliding cleanup removal, stablyai#15013 replaced refusing with routing to the confirmed host, and nothing imports the module any more — not even its own tests. * test(sidebar): pin the two states that actually reach the removal refusal The existing case used an empty store, which proved the clear but not that a user can land there. Cover a stale folder-workspace id and a hostless row under an ambiguous legacy runtime — both resolve to no route from populated state. --------- Co-authored-by: QA <qa@local>
…ows selectable (STA-4343) (stablyai#15013) * fix(workspaces): host-qualified workspace deletion (STA-4343, STA-4448) Squashed integration of PR stablyai#14606 + the codex review-loop output, replayed onto current main. Granular history preserved on brennanb2025/sta-4343-review-full. Fixes the regression from stablyai#13413: a workspace id is repoId::path with no host component, so the same repo at the same path on two hosts published one id for two workspaces, and deletion routed by that id landed on whichever host routing preferred - usually the ACTIVE one, not the row the user confirmed. - removeWorktree takes a REQUIRED host-qualified WorktreeRemovalTarget; omitting the host is a type error. All destructive callers migrated. - Projections dedup on (host, id), so two hosts render as two selectable rows while the createWorktree/fetchWorktrees race duplicate still collapses. - Ephemeral VM cleanup is host-scoped. It matched on bare workspaceId, so the host-scoped delete path destroyed the SURVIVING host's VM and its unpushed filesystem - a leak fix that had become data destruction. - Selection, keyboard routing, lineage grouping and Space rows carry host identity end to end; fixing the executor dedupe alone would have turned one-row intent into deleting both hosts. Files split to stay under max-lines rather than raising any cap. * refactor: split files that crossed max-lines The review-loop commits used --no-verify, so the pre-commit hook never enforced the caps. Extracted cohesive units rather than raising any limit: renderer teardown, delete-with-toast, pinned-group rows, host-scope helpers, workspace-kind predicates, filter actions, kanban drag selection, the renderer removal result type, and the native-chat persistence tests. * refactor(workspaces): extract cleanup deletion-phase selector Clears the last max-lines violation and the import-type side effect the changed-code gate flagged. * refactor(sidebar): track the delete-dialog extraction modules * fix(workspaces): preserve host identity across remaining surfaces * fix(sidebar): re-carry host through the rewritten palette result model stablyai#15170 replaced PaletteSearchResult while this PR was open. Re-applied the host qualification on top of the new model instead of taking either side: results carry worktreeHostId again, and the board filter keys its matched set on host identity rather than the bare id. Known gap, documented in the board test rather than deleted: searchWorktrees resolves evidence through a `documents` map keyed by BARE worktree id, so two same-id host rows collapse before this code sees them. Closing that belongs with the palette work. * test(cmd-j): pin the palette collision gap instead of asserting the old model The palette collision test asserted two host-qualified rows, which stablyai#15170's rewrite made unreachable: item ids are bare again and worktreeMap is id-keyed. Rewritten to assert what holds — activation always names a host — and to pin the defect it exposes: two same-id rows render on ONE command value, so React sees duplicate keys and a click on the first row activates the second row's host. That reproduces on main, so it is pre-existing, not from this PR. Pinned rather than deleted so fixing it must update this test. --------- Co-authored-by: QA <qa@local>
…yai#15187) * fix(sidebar): clear the delete state when a removal is refused Callers mark rows deleting up front for immediate sidebar feedback, but a refusal in beginHostQualifiedRemoval returns before removeWorktree's try/catch — the only other place that clears the flag. The failure toast auto-dismisses after 10s, so the workspace was left on a 'Deleting…' spinner indefinitely with no explanation still on screen. Also removes workspace-cleanup-removal-host-guard.ts: stablyai#14731 refused a colliding cleanup removal, stablyai#15013 replaced refusing with routing to the confirmed host, and nothing imports the module any more — not even its own tests. * test(sidebar): pin the two states that actually reach the removal refusal The existing case used an empty store, which proved the clear but not that a user can land there. Cover a stale folder-workspace id and a hostless row under an ambiguous legacy runtime — both resolve to no route from populated state. --------- Co-authored-by: QA <qa@local>
Supersedes #14606 and #14930. Same repo at the same path on two hosts is two workspaces — route the delete to the confirmed host instead of refusing, and make both rows selectable in the first place.
The defect
A workspace id is
repoId::pathwith no host component, so the local host, an SSH host and a paired runtime can all publish the same id for different workspaces. Removal routed by that id landed on whichever host routing preferred — usually the active workspace's, not the row the user confirmed. Deleting the SSH row destroyed the local checkout and its uncommitted work.#14731 shipped the interim fail-closed refusal. #14930 added the type contract. Neither made the second workspace reachable: the sidebar still collapsed two hosts into one row, so the other host's workspace could not be selected at all.
What this does
1. Host-qualified target, required by type.
removeWorktreetakesWorktreeRemovalTarget { id, executionHostId }instead of a bare id plus an optional option. Required-but-nullable: omitting it is a compile error, and "this row declares no host" becomes a deliberate claim. Migrating the callers produced 59 compile errors — all in tests, zero in production.2. All 8 destructive callers migrated (#14606 touched zero sidebar files).
store/slices/workspace-cleanup.tssidebar/worktree-delete-execution.ts(normal ×2, force retry)toWorktreeRemovalTarget(target)sidebar/DeleteWorktreeDialog.tsx(force)sidebar/ForgetSshWorkspaceDialog.tsx(forget-local)sidebar/ssh-host-remove-workspaces.tsstatus-bar/WorkspaceSpaceManagerPanel.tsx(force)worktree.executionHostId3. Projections no longer collapse two hosts. Dedup keys on
(host, id), so the createWorktree/fetchWorktrees race duplicate still collapses while two hosts survive as two rows. Row keys, host labels and the lineage emit sets are host-qualified — the lineage set mattered most, since keyed by bare id the second host's row was never emitted at all.4. The two id indices reconciled.
worktree-repo-index(last-wins) andworktree-by-id-index(first-wins) flattened the same data and disagreed, so one caller resolved to the local row and another to the SSH row. Both are first-wins now, documented, and marked as not evidence of ownership.Also fixed, found while writing the end-to-end test: the two sidebar row entry points passed a bare id (so with two rows rendered, either click deleted the same one); the batch path re-checked staleness through the id-keyed map and silently dropped confirmed deletes; the confirm dialog could only ever resolve the first-wins host's row; the Space Manager's ordinary Delete was still a live wrong-host deletion; ephemeral VM teardown was skipped on the host-scoped path, leaving VMs running and billing.
Testing
Real temp directories per host; the transports actually
rm -rfwhichever host they route to. Every assertion names both sides — which directory had to survive and which had to go — so neither a rig that deletes nothing nor one that deletes everything can pass.activeWorktreeIdis set to the colliding id, which is the trigger.The case #14930 could not prove: with the local workspace active and the remote row confirmed, the remote directory is deleted and the local one survives. Plus normal and force (force driven by real dirt, not a flag), local / SSH / paired runtime / folder workspaces / Windows-path ids / pre-host-qualified rows, no-false-refusal controls, and a sidebar test that two hosts render as two selectable rows.
Non-vacuity: 4 independent production mutations → 37 tests RED.
Known gaps
runtimeRepoMatchesExecutionHost's permissive fallbacks are what mint collisions; deliberately out of scope.meta.hostId(stamped once, never updated) can strand persisted metadata after a successful delete. The obvious narrowing is wrong — it discards session-partition evidence — so it needs its own change.worktreeMetais not host-partitioned.hostId. Proper fix is aworktree.rmcapability; marked TODO. Local and single-owner deletes route normally.STA-4343