Fix Windows workspace deletion runtime resolution (#5888)
* Add test file for workspace delete bug Co-authored-by: Orca <help@stably.ai> * Fix Windows workspace deletion runtime resolution Resolve project-created workspace deletion through the selected project runtime so Windows paths are listed strictly without falsely tripping the unregistered worktree guard. Design doc: docs/delete-workspace-windows-unregistered.md --------- Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
parent
55456e0c2d
commit
553d47bf4f
|
|
@ -0,0 +1,152 @@
|
|||
# Delete Windows Workspace Without False Unregistered Error
|
||||
|
||||
## Problem
|
||||
|
||||
GitHub issue [#5864](https://github.com/stablyai/orca/issues/5864) reports that Orca on Windows v0.14.80 fails to delete a workspace created from a project `+` button:
|
||||
|
||||
`Error invoking remote method 'worktrees:remove': Error: Refusing to delete unregistered worktree path: C:/Users/andy/orca/workspaces/ops-tools/packaging-improvements-2`
|
||||
|
||||
Relevant flow:
|
||||
|
||||
- Renderer delete calls local IPC for local targets in `src/renderer/src/store/slices/worktrees.ts`.
|
||||
- Preload exposes that as `worktrees:remove` in `src/preload/index.ts`.
|
||||
- IPC delete lists Git worktrees, matches the requested path, and throws the unregistered error if no registered entry matches in `src/main/ipc/worktrees.ts`.
|
||||
- Runtime RPC delete has the same registered-worktree gate in `src/main/runtime/orca-runtime.ts`.
|
||||
- Windows create/list coverage exists in `src/main/ipc/worktrees-windows.test.ts`, but Windows delete coverage is missing.
|
||||
|
||||
## Root Cause
|
||||
|
||||
Delete is right to refuse arbitrary paths. This bug is a false negative in the proof step: Orca asks Git for the authoritative registered worktree list, but the list does not contain an entry equivalent to the project-created target.
|
||||
|
||||
Do not fix this by adding another path-normalization layer after the list. `findRegisteredDeletableWorktree` delegates to `areWorktreePathsEqual`, which already treats `C:/...`, `C:\...`, and drive-case variants as equal while keeping POSIX/WSL paths distinct. `git/worktree.removeWorktree` has a similar comparator for its fallback branch lookup.
|
||||
|
||||
The credible failure surfaces are before or around that comparator:
|
||||
|
||||
- Delete may list through a different local runtime than create/list/selector resolution, especially for project runtime settings on Windows.
|
||||
- `listWorktrees` currently returns `[]` for several Git/list failures. In delete, that collapses "could not prove registration" into the misleading unregistered-path error.
|
||||
- Runtime selector/list resolution calls `listRepoWorktreesForResolution(repo)`, which currently omits local project runtime options.
|
||||
- Runtime removal validates a registered row, but then calls `removeWorktree` without `knownRemovedWorktree`, allowing the Git helper to rescan under the supplied options.
|
||||
- Some runtime cleanup paths still omit or recompute local runtime options instead of using the option set captured for the delete.
|
||||
|
||||
Implementation must start with a failing regression. If an equivalent Windows row is present in the registered list under the right options, deletion should succeed without further path comparator changes.
|
||||
|
||||
## Non-goals
|
||||
|
||||
- Do not allow deletion of existing unregistered directories.
|
||||
- Do not bypass the main-worktree, nested-worktree, local dirty-worktree, archive-hook, branch-preservation, or concurrent-delete guards.
|
||||
- Do not change delete dialog UI/copy.
|
||||
- Do not change SSH provider semantics.
|
||||
- Do not add a broad path abstraction or metadata migration.
|
||||
|
||||
## Design
|
||||
|
||||
1. Capture one repo-scoped local Git option set for the delete.
|
||||
- For local repos, use the repo's project runtime options from `getLocalProjectWorktreeGitOptions(store, repo)`. This is already repo-scoped and surfaces repair-required project runtimes before any Git command.
|
||||
- Do not use `getLocalGitOptionsForRegisteredWorktree` for delete. It scans all repos and uses native `path.resolve`, which is not a safe Windows-equivalence test on macOS/Linux test hosts.
|
||||
- Do not choose options from a worktree path alone. The parsed `repoId` is the authority for local runtime selection; exact-ID fallback after selector failure should still use the owning repo's project runtime options.
|
||||
- Keep SSH paths on the existing provider branch.
|
||||
|
||||
2. Make the authoritative Git list strict enough for delete.
|
||||
- Delete must distinguish "Git listed zero matching worktrees" from "Git listing failed." A selected-runtime Git/list failure should surface the underlying failure, not turn into `Refusing to delete unregistered worktree path`.
|
||||
- If this requires a strict list API beside `listWorktrees`, keep it narrow and delete-only; do not change polling/list UI behavior that intentionally tolerates transient Git failures.
|
||||
- The registered row returned by Git remains the canonical removal target after `findRegisteredDeletableWorktree` succeeds.
|
||||
|
||||
3. Thread the captured option set through the full local removal path.
|
||||
- `listWorktrees`, archive hooks, orphan proof reads, missing-path checks, clean preflight, `git worktree remove`, branch cleanup, recursive orphan cleanup, filesystem delete, push-target cleanup, and `git worktree prune` must all use the same captured options.
|
||||
- Do not recompute project runtime options later in the operation. A project runtime setting change during an in-flight delete must not split one deletion across two runtimes.
|
||||
- IPC already passes `knownRemovedWorktree` to `removeWorktree`; keep that behavior.
|
||||
- Runtime removal must also pass `knownRemovedWorktree` so branch cleanup uses the validated row and avoids a second list.
|
||||
- Replace current runtime cleanup call sites that omit `localWorktreeGitOptions` for push-target remote cleanup.
|
||||
- Replace prune calls that recompute `getLocalProjectGitExecOptions(...)` with `{ cwd: repo.path, ...localWorktreeGitOptions }`.
|
||||
|
||||
4. Fix runtime resolution/listing.
|
||||
- Update `listRepoWorktreesForResolution(repo)` to call `listRepoWorktrees(repo, getLocalProjectWorktreeGitOptions(store, repo))` for local repos.
|
||||
- Runtime exact-ID deletes should re-list Git under the captured options before destructive work; selector caches are convenience only, not delete authority.
|
||||
- Runtime archive hooks should use the captured options rather than `this.getLocalGitExecutionOptionArgs(repo)[0]`.
|
||||
|
||||
## Data Flow
|
||||
|
||||
- Delete action -> `removeWorktree(worktreeId, force)` in renderer.
|
||||
- Local target -> `window.api.worktrees.remove({ worktreeId, force, skipArchive })`.
|
||||
- Main parses `repoId` and `worktreePath`.
|
||||
- Main resolves the repo and captures one local Git option set for that repo.
|
||||
- Main strictly lists registered Git worktrees with those options.
|
||||
- Main matches requested path to registered path with `areWorktreePathsEqual` via `findRegisteredDeletableWorktree`.
|
||||
- Main uses the registered canonical path for hooks, preflight, watcher close, Git removal, orphan cleanup, metadata cleanup, and sidebar refresh. PTY teardown remains keyed by the exact worktree ID.
|
||||
|
||||
Runtime RPC follows the same rule after selector resolution, and exact-ID fallback must still re-list Git before destructive work.
|
||||
|
||||
## Edge Cases
|
||||
|
||||
- `C:/...`, `C:\...`, and drive-letter case variants refer to the same Windows worktree.
|
||||
- POSIX WSL paths and Windows paths must not compare equal unless `listWorktrees` translated them through the selected WSL options.
|
||||
- UNC paths, drive-letter paths, and `/mnt/<drive>` paths need explicit tests because Node path behavior is platform-specific on macOS/Linux test hosts.
|
||||
- Main worktree deletion is still rejected.
|
||||
- Parent worktree deletion is still rejected if another registered worktree is nested inside it.
|
||||
- Existing unregistered directories are still rejected, even with `force`.
|
||||
- Already-missing Orca-known worktrees still clean metadata only.
|
||||
- Orphaned Orca-created worktree directories still require proof through the `.git` file before recursive deletion.
|
||||
- Multi-window IPC deletes coalesce only for the same exact worktree ID and options. Equivalent Windows paths with different IDs, or IPC/runtime deletes racing each other, must degrade to safe missing/orphan handling or a protected error.
|
||||
- External Git mutation between list and `git worktree remove` is handled by the existing missing/orphan branches; keep those branches under the same captured runtime options.
|
||||
- Project runtime setting changes during an in-flight delete affect only later deletes.
|
||||
- SSH deletes still use SSH Git/filesystem providers and do not touch local paths.
|
||||
|
||||
## Test Plan
|
||||
|
||||
- Unit:
|
||||
- `pnpm vitest run src/main/ipc/worktrees-windows.test.ts`
|
||||
- `pnpm vitest run src/main/ipc/worktrees.test.ts --testNamePattern "local worktree removal|selected WSL project runtime|unregistered delete|contains another registered|already-missing"`
|
||||
- `pnpm vitest run src/main/runtime/orca-runtime.test.ts --testNamePattern "worktree removal|selected WSL project runtime|unregistered delete|contains another registered|already-missing"`
|
||||
- Required new coverage:
|
||||
- IPC Windows delete regression: request path uses `C:/...`, Git registered row uses backslashes and/or different drive-case, delete succeeds, hooks/preflight/removal use the canonical registered path, `knownRemovedWorktree` is passed, metadata is removed, and `worktrees:changed` emits.
|
||||
- Runtime Windows delete regression with the same path mismatch. Assert selector/list resolution and final removal both use selected project runtime options, and `knownRemovedWorktree` is passed.
|
||||
- Strict-list failure regression: a selected-runtime list failure rejects with the list failure, not the unregistered-path error.
|
||||
- Negative Windows/WSL mismatch: POSIX `/mnt/c/...` or WSL-native paths must not match unrelated Windows paths unless translated by the selected WSL options.
|
||||
- Runtime cleanup regressions for already-missing/orphan/push-target cleanup under selected WSL options.
|
||||
- Integration/e2e:
|
||||
- Electron smoke with a disposable local repo/worktree: delete succeeds, row disappears, no new delete UI regressions.
|
||||
- Real Windows validation is preferred. macOS/Linux unit tests can cover comparator and option plumbing, but they cannot fully prove Node and Git path behavior on Windows.
|
||||
- Full checks:
|
||||
- `pnpm typecheck`
|
||||
- `pnpm lint`
|
||||
|
||||
## UI Quality Bar
|
||||
|
||||
No intentional UI change. Existing delete dialog, progress state, toast behavior, and sidebar row removal should remain visually unchanged and follow `docs/STYLEGUIDE.md`.
|
||||
|
||||
## Review Screenshots
|
||||
|
||||
No design screenshots are required for a backend-only fix. If the PR needs Electron smoke evidence, attach only:
|
||||
|
||||
1. Disposable workspace delete confirmation before confirming.
|
||||
2. Sidebar after successful deletion with the row gone.
|
||||
|
||||
Do not spend review time manufacturing a protected/error screenshot; cover that with unit tests.
|
||||
|
||||
## Rollout
|
||||
|
||||
1. Add focused Windows-path delete regression tests.
|
||||
2. Add strict delete listing or equivalent failure propagation.
|
||||
3. Update runtime selector/list resolution to use local project runtime options.
|
||||
4. Thread captured options and `knownRemovedWorktree` through IPC/runtime delete paths.
|
||||
5. Run focused tests, typecheck, lint.
|
||||
6. Electron-validate the unchanged delete UI on a disposable workspace and collect screenshots only if required.
|
||||
|
||||
## Lightweight Eng Review
|
||||
|
||||
- Scope: delete-only; no renderer changes, no new deletion authority, and no broad path-normalization rewrite.
|
||||
- Architecture/data flow: local IPC and runtime RPC both keep Git as the authority. The fix is to ask Git through the correct project runtime, treat list failures as failures, then use Git's registered row as the canonical removal target.
|
||||
- Failure modes covered:
|
||||
- Windows slash/drive-case mismatches.
|
||||
- Wrong or failed local project runtime listing.
|
||||
- Runtime selector resolution using host listings.
|
||||
- Runtime branch cleanup rescanning instead of using the validated row.
|
||||
- Metadata absent or stale during already-missing cleanup.
|
||||
- Existing unregistered directory remains protected.
|
||||
- Main and nested registered worktrees remain protected.
|
||||
- SSH paths stay provider-owned.
|
||||
- Performance/blast radius: no material concern if the delete path performs one strict authoritative list and passes `knownRemovedWorktree` to avoid the helper rescan.
|
||||
- Feasibility: this is not a "one comparator call" fix. The current APIs make `listWorktrees` failures look like empty lists, and runtime selector resolution currently omits local runtime options.
|
||||
- UI quality bar: no UI-visible design change; Electron should judge that existing delete dialog, progress, toast, and row removal still look unchanged against `docs/STYLEGUIDE.md`.
|
||||
- Required review screenshots: none for the backend fix; optional disposable-workspace smoke screenshots only if the PR process asks for visual evidence.
|
||||
- Residual risks: true Windows filesystem/Git spelling behavior still depends on a Windows runner or user validation; macOS/Linux tests cannot fully model it.
|
||||
|
|
@ -19,7 +19,8 @@ vi.mock('electron', () => ({
|
|||
}))
|
||||
|
||||
vi.mock('../git/worktree', () => ({
|
||||
listWorktrees: vi.fn().mockResolvedValue([])
|
||||
listWorktrees: vi.fn().mockResolvedValue([]),
|
||||
listWorktreesStrict: vi.fn().mockResolvedValue([])
|
||||
}))
|
||||
|
||||
import { BrowserManager } from './browser-manager'
|
||||
|
|
|
|||
|
|
@ -492,6 +492,17 @@ export async function listWorktrees(
|
|||
}
|
||||
}
|
||||
|
||||
export async function listWorktreesStrict(
|
||||
repoPath: string,
|
||||
options: GitWorktreeExecOptions = {}
|
||||
): Promise<GitWorktreeInfo[]> {
|
||||
const worktrees = (await readWorktreeList(repoPath, options)).map((worktree) => {
|
||||
const translatedPath = translateWorktreePath(worktree.path, repoPath, options)
|
||||
return translatedPath === worktree.path ? worktree : { ...worktree, path: translatedPath }
|
||||
})
|
||||
return annotateSparseCheckoutStatus(worktrees)
|
||||
}
|
||||
|
||||
async function annotateSparseCheckoutStatus(
|
||||
worktrees: GitWorktreeInfo[]
|
||||
): Promise<GitWorktreeInfo[]> {
|
||||
|
|
|
|||
|
|
@ -132,7 +132,8 @@ vi.mock('../git/check-ignored-paths', () => ({
|
|||
}))
|
||||
|
||||
vi.mock('../git/worktree', () => ({
|
||||
listWorktrees: listWorktreesMock
|
||||
listWorktrees: listWorktreesMock,
|
||||
listWorktreesStrict: listWorktreesMock
|
||||
}))
|
||||
|
||||
vi.mock('../providers/ssh-filesystem-dispatch', () => ({
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ const {
|
|||
handleMock,
|
||||
removeHandlerMock,
|
||||
listWorktreesMock,
|
||||
assertWorktreeCleanForRemovalMock,
|
||||
addWorktreeMock,
|
||||
removeWorktreeMock,
|
||||
getGitUsernameMock,
|
||||
|
|
@ -22,11 +23,16 @@ const {
|
|||
hasHooksFileMock,
|
||||
loadHooksMock,
|
||||
computeWorktreePathMock,
|
||||
ensurePathWithinWorkspaceMock
|
||||
ensurePathWithinWorkspaceMock,
|
||||
killAllProcessesForWorktreeMock,
|
||||
clearProviderPtyStateMock,
|
||||
getLocalPtyProviderMock,
|
||||
deleteWorktreeHistoryDirMock
|
||||
} = vi.hoisted(() => ({
|
||||
handleMock: vi.fn(),
|
||||
removeHandlerMock: vi.fn(),
|
||||
listWorktreesMock: vi.fn(),
|
||||
assertWorktreeCleanForRemovalMock: vi.fn(),
|
||||
addWorktreeMock: vi.fn(),
|
||||
removeWorktreeMock: vi.fn(),
|
||||
getGitUsernameMock: vi.fn(),
|
||||
|
|
@ -45,7 +51,11 @@ const {
|
|||
hasHooksFileMock: vi.fn(),
|
||||
loadHooksMock: vi.fn(),
|
||||
computeWorktreePathMock: vi.fn(),
|
||||
ensurePathWithinWorkspaceMock: vi.fn()
|
||||
ensurePathWithinWorkspaceMock: vi.fn(),
|
||||
killAllProcessesForWorktreeMock: vi.fn(),
|
||||
clearProviderPtyStateMock: vi.fn(),
|
||||
getLocalPtyProviderMock: vi.fn(),
|
||||
deleteWorktreeHistoryDirMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('electron', () => ({
|
||||
|
|
@ -57,6 +67,8 @@ vi.mock('electron', () => ({
|
|||
|
||||
vi.mock('../git/worktree', () => ({
|
||||
listWorktrees: listWorktreesMock,
|
||||
listWorktreesStrict: listWorktreesMock,
|
||||
assertWorktreeCleanForRemoval: assertWorktreeCleanForRemovalMock,
|
||||
addWorktree: addWorktreeMock,
|
||||
removeWorktree: removeWorktreeMock
|
||||
}))
|
||||
|
|
@ -93,6 +105,19 @@ vi.mock('../hooks', () => ({
|
|||
shouldRunSetupForCreate: shouldRunSetupForCreateMock
|
||||
}))
|
||||
|
||||
vi.mock('../runtime/worktree-teardown', () => ({
|
||||
killAllProcessesForWorktree: killAllProcessesForWorktreeMock
|
||||
}))
|
||||
|
||||
vi.mock('./pty', () => ({
|
||||
clearProviderPtyState: clearProviderPtyStateMock,
|
||||
getLocalPtyProvider: getLocalPtyProviderMock
|
||||
}))
|
||||
|
||||
vi.mock('../terminal-history', () => ({
|
||||
deleteWorktreeHistoryDir: deleteWorktreeHistoryDirMock
|
||||
}))
|
||||
|
||||
vi.mock('./worktree-logic', async (importOriginal) => {
|
||||
const actual = (await importOriginal()) as Record<string, unknown>
|
||||
return {
|
||||
|
|
@ -129,6 +154,7 @@ describe('registerWorktreeHandlers – Windows path handling', () => {
|
|||
handleMock.mockReset()
|
||||
removeHandlerMock.mockReset()
|
||||
listWorktreesMock.mockReset()
|
||||
assertWorktreeCleanForRemovalMock.mockReset()
|
||||
addWorktreeMock.mockReset()
|
||||
removeWorktreeMock.mockReset()
|
||||
getGitUsernameMock.mockReset()
|
||||
|
|
@ -148,6 +174,10 @@ describe('registerWorktreeHandlers – Windows path handling', () => {
|
|||
loadHooksMock.mockReset()
|
||||
computeWorktreePathMock.mockReset()
|
||||
ensurePathWithinWorkspaceMock.mockReset()
|
||||
killAllProcessesForWorktreeMock.mockReset()
|
||||
clearProviderPtyStateMock.mockReset()
|
||||
getLocalPtyProviderMock.mockReset()
|
||||
deleteWorktreeHistoryDirMock.mockReset()
|
||||
mainWindow.webContents.send.mockReset()
|
||||
store.getRepos.mockReset()
|
||||
store.getRepo.mockReset()
|
||||
|
|
@ -205,6 +235,13 @@ describe('registerWorktreeHandlers – Windows path handling', () => {
|
|||
computeWorktreePathMock.mockReturnValue('C:\\workspaces\\improve-dashboard')
|
||||
ensurePathWithinWorkspaceMock.mockReturnValue('C:\\workspaces\\improve-dashboard')
|
||||
listWorktreesMock.mockResolvedValue([])
|
||||
assertWorktreeCleanForRemovalMock.mockResolvedValue(undefined)
|
||||
killAllProcessesForWorktreeMock.mockResolvedValue({
|
||||
runtimeStopped: 0,
|
||||
providerStopped: 0,
|
||||
registryStopped: 0
|
||||
})
|
||||
getLocalPtyProviderMock.mockReturnValue({})
|
||||
|
||||
// Why: createLocalWorktree routes `git fetch` through
|
||||
// `runtime.fetchRemoteWithCache` (§3.3 Lifecycle). Stub it for path tests.
|
||||
|
|
@ -308,4 +345,43 @@ describe('registerWorktreeHandlers – Windows path handling', () => {
|
|||
}
|
||||
])
|
||||
})
|
||||
|
||||
it('deletes a Windows worktree when the requested path uses different separators and drive casing', async () => {
|
||||
const registeredWorktree = {
|
||||
path: 'c:\\workspaces\\Improve-Dashboard',
|
||||
head: 'feature-head',
|
||||
branch: 'refs/heads/improve-dashboard',
|
||||
isBare: false,
|
||||
isMainWorktree: false
|
||||
}
|
||||
listWorktreesMock.mockResolvedValue([
|
||||
{
|
||||
path: 'C:\\repo',
|
||||
head: 'main-head',
|
||||
branch: 'refs/heads/main',
|
||||
isBare: false,
|
||||
isMainWorktree: true
|
||||
},
|
||||
registeredWorktree
|
||||
])
|
||||
removeWorktreeMock.mockResolvedValue({})
|
||||
|
||||
await handlers['worktrees:remove'](null, {
|
||||
worktreeId: 'repo-1::C:/workspaces/improve-dashboard'
|
||||
})
|
||||
|
||||
expect(assertWorktreeCleanForRemovalMock).toHaveBeenCalledWith(registeredWorktree.path, false)
|
||||
expect(removeWorktreeMock).toHaveBeenCalledWith(
|
||||
'C:\\repo',
|
||||
registeredWorktree.path,
|
||||
false,
|
||||
expect.objectContaining({
|
||||
knownRemovedWorktree: registeredWorktree
|
||||
})
|
||||
)
|
||||
expect(store.removeWorktreeMeta).toHaveBeenCalledWith('repo-1::C:/workspaces/improve-dashboard')
|
||||
expect(mainWindow.webContents.send).toHaveBeenCalledWith('worktrees:changed', {
|
||||
repoId: 'repo-1'
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -112,6 +112,7 @@ vi.mock('electron', () => ({
|
|||
|
||||
vi.mock('../git/worktree', () => ({
|
||||
listWorktrees: listWorktreesMock,
|
||||
listWorktreesStrict: listWorktreesMock,
|
||||
parseWorktreeList: parseWorktreeListMock,
|
||||
assertWorktreeCleanForRemoval: assertWorktreeCleanForRemovalMock,
|
||||
addWorktree: addWorktreeMock,
|
||||
|
|
@ -6232,6 +6233,22 @@ describe('registerWorktreeHandlers', () => {
|
|||
)
|
||||
})
|
||||
|
||||
it('surfaces selected-runtime list failures during local worktree removal', async () => {
|
||||
mockSelectedWslProjectRuntime()
|
||||
const listError = new Error('wsl git list failed')
|
||||
listWorktreesMock.mockRejectedValue(listError)
|
||||
|
||||
await expect(
|
||||
handlers['worktrees:remove'](null, {
|
||||
worktreeId: 'repo-1::/workspace/feature-wt'
|
||||
})
|
||||
).rejects.toThrow('wsl git list failed')
|
||||
|
||||
expect(listWorktreesMock).toHaveBeenCalledWith('/workspace/repo', { wslDistro: 'Ubuntu' })
|
||||
expect(assertWorktreeCleanForRemovalMock).not.toHaveBeenCalled()
|
||||
expect(removeWorktreeMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('fails dirty non-force deletes before PTY teardown', async () => {
|
||||
mockKnownFeatureWorktree()
|
||||
getEffectiveHooksMock.mockReturnValue(null)
|
||||
|
|
|
|||
|
|
@ -37,7 +37,7 @@ import {
|
|||
import {
|
||||
assertWorktreeCleanForRemoval,
|
||||
forceDeleteLocalBranch,
|
||||
listWorktrees as listGitWorktrees,
|
||||
listWorktreesStrict as listGitWorktreesStrict,
|
||||
removeWorktree
|
||||
} from '../git/worktree'
|
||||
import { gitExecFileAsync } from '../git/runner'
|
||||
|
|
@ -1245,8 +1245,8 @@ export function registerWorktreeHandlers(
|
|||
const registeredWorktrees = repo.connectionId
|
||||
? await provider!.listWorktrees(repo.path)
|
||||
: hasLocalWorktreeGitOptions
|
||||
? await listGitWorktrees(repo.path, localWorktreeGitOptions)
|
||||
: await listGitWorktrees(repo.path)
|
||||
? await listGitWorktreesStrict(repo.path, localWorktreeGitOptions)
|
||||
: await listGitWorktreesStrict(repo.path)
|
||||
const removedMeta = store.getWorktreeMeta(args.worktreeId)
|
||||
const removedPushTarget = removedMeta?.pushTarget
|
||||
const registeredWorktree = findRegisteredDeletableWorktree(
|
||||
|
|
@ -1519,10 +1519,10 @@ export function registerWorktreeHandlers(
|
|||
// (`.git/worktrees/<name>`) is still intact. Without pruning, `git worktree
|
||||
// list` continues to show the stale entry and the branch it had checked out
|
||||
// remains locked — other worktrees cannot check it out.
|
||||
await gitExecFileAsync(
|
||||
['worktree', 'prune'],
|
||||
getLocalProjectGitExecOptions(store, repo)
|
||||
).catch(() => {})
|
||||
await gitExecFileAsync(['worktree', 'prune'], {
|
||||
cwd: repo.path,
|
||||
...localWorktreeGitOptions
|
||||
}).catch(() => {})
|
||||
await cleanupUnusedWorktreePushTargetRemote(
|
||||
repo.path,
|
||||
args.worktreeId,
|
||||
|
|
|
|||
|
|
@ -14,7 +14,8 @@ vi.mock('../git/worktree', () => ({
|
|||
isBare: false,
|
||||
isMainWorktree: false
|
||||
}
|
||||
])
|
||||
]),
|
||||
listWorktreesStrict: vi.fn().mockResolvedValue([])
|
||||
}))
|
||||
|
||||
vi.mock('../hooks', () => ({
|
||||
|
|
|
|||
|
|
@ -16,7 +16,8 @@ import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'
|
|||
import { OrcaRuntimeService } from './orca-runtime'
|
||||
|
||||
vi.mock('../git/worktree', () => ({
|
||||
listWorktrees: vi.fn().mockResolvedValue([])
|
||||
listWorktrees: vi.fn().mockResolvedValue([]),
|
||||
listWorktreesStrict: vi.fn().mockResolvedValue([])
|
||||
}))
|
||||
vi.mock('../hooks', () => ({
|
||||
createSetupRunnerScript: vi.fn(),
|
||||
|
|
|
|||
|
|
@ -16,7 +16,8 @@ vi.mock('../git/worktree', () => ({
|
|||
isBare: false,
|
||||
isMainWorktree: false
|
||||
}
|
||||
])
|
||||
]),
|
||||
listWorktreesStrict: vi.fn().mockResolvedValue([])
|
||||
}))
|
||||
|
||||
vi.mock('../hooks', () => ({
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ import {
|
|||
addWorktree,
|
||||
assertWorktreeCleanForRemoval,
|
||||
listWorktrees,
|
||||
listWorktreesStrict,
|
||||
removeWorktree
|
||||
} from '../git/worktree'
|
||||
import * as gitRunner from '../git/runner'
|
||||
|
|
@ -281,6 +282,7 @@ const {
|
|||
|
||||
vi.mock('../git/worktree', () => ({
|
||||
listWorktrees: vi.fn().mockResolvedValue(MOCK_GIT_WORKTREES),
|
||||
listWorktreesStrict: vi.fn().mockResolvedValue(MOCK_GIT_WORKTREES),
|
||||
assertWorktreeCleanForRemoval: vi.fn().mockResolvedValue(undefined),
|
||||
addWorktree: addWorktreeMock,
|
||||
removeWorktree: removeWorktreeMock,
|
||||
|
|
@ -491,6 +493,7 @@ function resetRuntimeTestMocks(): void {
|
|||
electronMocks.ipcMain.removeListener.mockClear()
|
||||
electronMocks.ipcMain.emit.mockClear()
|
||||
vi.mocked(listWorktrees).mockResolvedValue(MOCK_GIT_WORKTREES)
|
||||
vi.mocked(listWorktreesStrict).mockResolvedValue(MOCK_GIT_WORKTREES)
|
||||
vi.mocked(addWorktree).mockReset()
|
||||
vi.mocked(assertWorktreeCleanForRemoval).mockReset()
|
||||
vi.mocked(assertWorktreeCleanForRemoval).mockResolvedValue(undefined)
|
||||
|
|
@ -19523,7 +19526,14 @@ describe('OrcaRuntimeService', () => {
|
|||
const result = await runtime.removeManagedWorktree(TEST_WORKTREE_ID)
|
||||
|
||||
expect(runHook).not.toHaveBeenCalled()
|
||||
expect(removeWorktree).toHaveBeenCalledWith(TEST_REPO_PATH, TEST_WORKTREE_PATH, false)
|
||||
expect(removeWorktree).toHaveBeenCalledWith(
|
||||
TEST_REPO_PATH,
|
||||
TEST_WORKTREE_PATH,
|
||||
false,
|
||||
expect.objectContaining({
|
||||
knownRemovedWorktree: expect.objectContaining({ path: TEST_WORKTREE_PATH })
|
||||
})
|
||||
)
|
||||
expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith(TEST_WORKTREE_ID)
|
||||
expect(result.warning).toBe(
|
||||
`orca.yaml archive hook skipped for ${TEST_WORKTREE_PATH}; pass --run-hooks to run it.`
|
||||
|
|
@ -19560,10 +19570,124 @@ describe('OrcaRuntimeService', () => {
|
|||
wslDistro: 'Ubuntu'
|
||||
})
|
||||
expect(removeWorktree).toHaveBeenCalledWith(TEST_REPO_PATH, TEST_WORKTREE_PATH, false, {
|
||||
knownRemovedWorktree: expect.objectContaining({ path: TEST_WORKTREE_PATH }),
|
||||
wslDistro: 'Ubuntu'
|
||||
})
|
||||
})
|
||||
|
||||
it('deletes a Windows runtime worktree using the canonical registered path', async () => {
|
||||
setPlatform('win32')
|
||||
const repo = {
|
||||
id: TEST_REPO_ID,
|
||||
path: 'C:\\repo',
|
||||
displayName: 'repo',
|
||||
badgeColor: 'blue',
|
||||
addedAt: 1
|
||||
}
|
||||
const requestedWorktreeId = `${TEST_REPO_ID}::C:/workspaces/improve-dashboard`
|
||||
const registeredWorktree = {
|
||||
path: 'c:\\workspaces\\Improve-Dashboard',
|
||||
head: 'feature-head',
|
||||
branch: 'refs/heads/improve-dashboard',
|
||||
isBare: false,
|
||||
isMainWorktree: false
|
||||
}
|
||||
const runtimeStore = {
|
||||
...store,
|
||||
getRepos: () => [repo],
|
||||
getRepo: (id: string) => (id === TEST_REPO_ID ? repo : undefined),
|
||||
getAllWorktreeMeta: () => ({
|
||||
[requestedWorktreeId]: makeWorktreeMeta()
|
||||
}),
|
||||
getWorktreeMeta: (worktreeId: string) =>
|
||||
worktreeId === requestedWorktreeId ? makeWorktreeMeta() : undefined,
|
||||
getProjects: () => [
|
||||
{
|
||||
id: 'project-1',
|
||||
displayName: 'repo',
|
||||
badgeColor: 'blue',
|
||||
sourceRepoIds: [TEST_REPO_ID],
|
||||
localWindowsRuntimePreference: { kind: 'wsl', distro: 'Ubuntu' },
|
||||
createdAt: 0,
|
||||
updatedAt: 0
|
||||
}
|
||||
],
|
||||
getSettings: () => ({
|
||||
...store.getSettings(),
|
||||
localWindowsRuntimeDefault: { kind: 'windows-host' }
|
||||
})
|
||||
}
|
||||
const runtime = new OrcaRuntimeService(runtimeStore as never)
|
||||
vi.mocked(getEffectiveHooks).mockReturnValue(null)
|
||||
vi.mocked(listWorktrees).mockResolvedValue([
|
||||
{
|
||||
path: repo.path,
|
||||
head: 'main-head',
|
||||
branch: 'refs/heads/main',
|
||||
isBare: false,
|
||||
isMainWorktree: true
|
||||
},
|
||||
registeredWorktree
|
||||
])
|
||||
vi.mocked(listWorktreesStrict).mockResolvedValue([
|
||||
{
|
||||
path: repo.path,
|
||||
head: 'main-head',
|
||||
branch: 'refs/heads/main',
|
||||
isBare: false,
|
||||
isMainWorktree: true
|
||||
},
|
||||
registeredWorktree
|
||||
])
|
||||
vi.mocked(removeWorktree).mockResolvedValue({})
|
||||
|
||||
await runtime.removeManagedWorktree(requestedWorktreeId)
|
||||
|
||||
expect(listWorktrees).toHaveBeenCalledWith(repo.path, { wslDistro: 'Ubuntu' })
|
||||
expect(listWorktreesStrict).toHaveBeenCalledWith(repo.path, { wslDistro: 'Ubuntu' })
|
||||
expect(assertWorktreeCleanForRemoval).toHaveBeenCalledWith(registeredWorktree.path, false, {
|
||||
wslDistro: 'Ubuntu'
|
||||
})
|
||||
expect(removeWorktree).toHaveBeenCalledWith(repo.path, registeredWorktree.path, false, {
|
||||
knownRemovedWorktree: registeredWorktree,
|
||||
wslDistro: 'Ubuntu'
|
||||
})
|
||||
})
|
||||
|
||||
it('surfaces selected-runtime list failures during runtime worktree removal', async () => {
|
||||
setPlatform('win32')
|
||||
const runtimeStore = {
|
||||
...store,
|
||||
getProjects: () => [
|
||||
{
|
||||
id: 'project-1',
|
||||
displayName: 'repo',
|
||||
badgeColor: 'blue',
|
||||
sourceRepoIds: [TEST_REPO_ID],
|
||||
localWindowsRuntimePreference: { kind: 'wsl', distro: 'Ubuntu' },
|
||||
createdAt: 0,
|
||||
updatedAt: 0
|
||||
}
|
||||
],
|
||||
getSettings: () => ({
|
||||
...store.getSettings(),
|
||||
localWindowsRuntimeDefault: { kind: 'windows-host' }
|
||||
})
|
||||
}
|
||||
const runtime = new OrcaRuntimeService(runtimeStore as never)
|
||||
vi.mocked(listWorktrees).mockResolvedValue(MOCK_GIT_WORKTREES)
|
||||
vi.mocked(listWorktreesStrict).mockRejectedValue(new Error('wsl git list failed'))
|
||||
|
||||
await expect(runtime.removeManagedWorktree(TEST_WORKTREE_ID)).rejects.toThrow(
|
||||
'wsl git list failed'
|
||||
)
|
||||
|
||||
expect(listWorktrees).toHaveBeenCalledWith(TEST_REPO_PATH, { wslDistro: 'Ubuntu' })
|
||||
expect(listWorktreesStrict).toHaveBeenCalledWith(TEST_REPO_PATH, { wslDistro: 'Ubuntu' })
|
||||
expect(assertWorktreeCleanForRemoval).not.toHaveBeenCalled()
|
||||
expect(removeWorktree).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('force-deletes a branch that was preserved by runtime worktree removal', async () => {
|
||||
const runtime = new OrcaRuntimeService(store)
|
||||
vi.mocked(removeWorktree).mockResolvedValue({
|
||||
|
|
@ -19892,6 +20016,29 @@ describe('OrcaRuntimeService', () => {
|
|||
isMainWorktree: false
|
||||
}
|
||||
])
|
||||
vi.mocked(listWorktreesStrict).mockResolvedValue([
|
||||
{
|
||||
path: TEST_REPO_PATH,
|
||||
head: 'main',
|
||||
branch: 'refs/heads/main',
|
||||
isBare: false,
|
||||
isMainWorktree: true
|
||||
},
|
||||
{
|
||||
path: TEST_WORKTREE_PATH,
|
||||
head: 'parent',
|
||||
branch: 'refs/heads/parent',
|
||||
isBare: false,
|
||||
isMainWorktree: false
|
||||
},
|
||||
{
|
||||
path: `${TEST_WORKTREE_PATH}/child`,
|
||||
head: 'child',
|
||||
branch: 'refs/heads/child',
|
||||
isBare: false,
|
||||
isMainWorktree: false
|
||||
}
|
||||
])
|
||||
vi.mocked(getEffectiveHooks).mockReturnValue({
|
||||
scripts: {
|
||||
archive: 'pnpm worktree:archive'
|
||||
|
|
@ -19971,7 +20118,14 @@ describe('OrcaRuntimeService', () => {
|
|||
vi.spyOn(gitRunner, 'gitExecFileAsync').mockResolvedValue({ stdout: '', stderr: '' })
|
||||
|
||||
await expect(runtime.removeManagedWorktree(TEST_WORKTREE_ID)).resolves.toEqual({})
|
||||
expect(removeWorktree).toHaveBeenCalledWith(TEST_REPO_PATH, TEST_WORKTREE_PATH, false)
|
||||
expect(removeWorktree).toHaveBeenCalledWith(
|
||||
TEST_REPO_PATH,
|
||||
TEST_WORKTREE_PATH,
|
||||
false,
|
||||
expect.objectContaining({
|
||||
knownRemovedWorktree: expect.objectContaining({ path: TEST_WORKTREE_PATH })
|
||||
})
|
||||
)
|
||||
expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith(TEST_WORKTREE_ID)
|
||||
})
|
||||
|
||||
|
|
@ -19994,7 +20148,14 @@ describe('OrcaRuntimeService', () => {
|
|||
undefined,
|
||||
undefined
|
||||
)
|
||||
expect(removeWorktree).toHaveBeenCalledWith(TEST_REPO_PATH, TEST_WORKTREE_PATH, false)
|
||||
expect(removeWorktree).toHaveBeenCalledWith(
|
||||
TEST_REPO_PATH,
|
||||
TEST_WORKTREE_PATH,
|
||||
false,
|
||||
expect.objectContaining({
|
||||
knownRemovedWorktree: expect.objectContaining({ path: TEST_WORKTREE_PATH })
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('clears optimistic reconcile tokens when a CLI worktree removal succeeds', async () => {
|
||||
|
|
|
|||
|
|
@ -550,6 +550,7 @@ import {
|
|||
import { hasLocalCommitObject } from '../git/commit-object-ref'
|
||||
import {
|
||||
listWorktrees,
|
||||
listWorktreesStrict,
|
||||
addWorktree,
|
||||
addSparseWorktree,
|
||||
assertWorktreeCleanForRemoval,
|
||||
|
|
@ -13930,8 +13931,8 @@ export class OrcaRuntimeService {
|
|||
const registeredWorktrees = repo.connectionId
|
||||
? await provider!.listWorktrees(repo.path)
|
||||
: hasLocalWorktreeGitOptions
|
||||
? await listWorktrees(repo.path, localWorktreeGitOptions)
|
||||
: await listWorktrees(repo.path)
|
||||
? await listWorktreesStrict(repo.path, localWorktreeGitOptions)
|
||||
: await listWorktreesStrict(repo.path)
|
||||
const removedMeta = store.getWorktreeMeta(removalTarget.id)
|
||||
const removedPushTarget = removedMeta?.pushTarget ?? removalTarget.pushTarget
|
||||
const registeredWorktree = findRegisteredDeletableWorktree(
|
||||
|
|
@ -13996,7 +13997,8 @@ export class OrcaRuntimeService {
|
|||
repo.path,
|
||||
removalTarget.id,
|
||||
removedPushTarget,
|
||||
store
|
||||
store,
|
||||
localWorktreeGitOptions
|
||||
)
|
||||
}
|
||||
this.clearOptimisticReconcileToken(removalTarget.id)
|
||||
|
|
@ -14028,7 +14030,8 @@ export class OrcaRuntimeService {
|
|||
repo.path,
|
||||
removalTarget.id,
|
||||
removedPushTarget,
|
||||
store
|
||||
store,
|
||||
localWorktreeGitOptions
|
||||
))
|
||||
this.clearOptimisticReconcileToken(removalTarget.id)
|
||||
this.removeWorktreeMetadataAndHistory(store, removalTarget.id)
|
||||
|
|
@ -14079,7 +14082,7 @@ export class OrcaRuntimeService {
|
|||
canonicalWorktreePath,
|
||||
repo,
|
||||
undefined,
|
||||
this.getLocalGitExecutionOptionArgs(repo)[0]
|
||||
hasLocalWorktreeGitOptions ? localWorktreeGitOptions : undefined
|
||||
)
|
||||
if (!result.success) {
|
||||
console.error(`[hooks] archive hook failed for ${canonicalWorktreePath}:`, result.output)
|
||||
|
|
@ -14140,15 +14143,15 @@ export class OrcaRuntimeService {
|
|||
|
||||
let removalResult: RemoveWorktreeResult | undefined
|
||||
try {
|
||||
const removeOptions = hasLocalWorktreeGitOptions
|
||||
? { ...(!deleteBranch ? { deleteBranch } : {}), ...localWorktreeGitOptions }
|
||||
: !deleteBranch
|
||||
? { deleteBranch }
|
||||
: undefined
|
||||
const removeOptions = {
|
||||
...(!deleteBranch ? { deleteBranch } : {}),
|
||||
// Why: removal already validated the Git row under the selected
|
||||
// project runtime; keep branch cleanup on that same canonical row.
|
||||
knownRemovedWorktree: registeredWorktree,
|
||||
...localWorktreeGitOptions
|
||||
}
|
||||
removalResult = this.preserveBranchHeadFallback(
|
||||
await (removeOptions
|
||||
? removeWorktree(repo.path, canonicalWorktreePath, force, removeOptions)
|
||||
: removeWorktree(repo.path, canonicalWorktreePath, force)),
|
||||
await removeWorktree(repo.path, canonicalWorktreePath, force, removeOptions),
|
||||
registeredWorktree.head
|
||||
)
|
||||
} catch (error) {
|
||||
|
|
@ -14177,10 +14180,10 @@ export class OrcaRuntimeService {
|
|||
// (`.git/worktrees/<name>`) is still intact. Without pruning, `git worktree
|
||||
// list` continues to show the stale entry and the branch it had checked out
|
||||
// remains locked — other worktrees cannot check it out.
|
||||
await gitExecFileAsync(
|
||||
['worktree', 'prune'],
|
||||
getLocalProjectGitExecOptions(this.requireStore(), repo)
|
||||
).catch(() => {})
|
||||
await gitExecFileAsync(['worktree', 'prune'], {
|
||||
cwd: repo.path,
|
||||
...localWorktreeGitOptions
|
||||
}).catch(() => {})
|
||||
await cleanupUnusedWorktreePushTargetRemote(
|
||||
repo.path,
|
||||
removalTarget.id,
|
||||
|
|
@ -16236,7 +16239,13 @@ export class OrcaRuntimeService {
|
|||
|
||||
private async listRepoWorktreesForResolution(repo: Repo): Promise<RuntimeWorktreeScanResult> {
|
||||
if (!repo.connectionId) {
|
||||
return { ok: true, worktrees: await listRepoWorktrees(repo) }
|
||||
return {
|
||||
ok: true,
|
||||
worktrees: await listRepoWorktrees(
|
||||
repo,
|
||||
getLocalProjectWorktreeGitOptions(this.requireStore(), repo)
|
||||
)
|
||||
}
|
||||
}
|
||||
const provider = getSshGitProvider(repo.connectionId)
|
||||
if (!provider) {
|
||||
|
|
|
|||
|
|
@ -25,7 +25,8 @@ vi.mock('../git/worktree', () => ({
|
|||
isBare: false,
|
||||
isMainWorktree: false
|
||||
}
|
||||
])
|
||||
]),
|
||||
listWorktreesStrict: vi.fn().mockResolvedValue([])
|
||||
}))
|
||||
|
||||
async function sendRequest(
|
||||
|
|
|
|||
|
|
@ -2912,7 +2912,11 @@
|
|||
"e740f92596": "Refresh failed",
|
||||
"8418ec448d": "{{value0}} usage could not be refreshed. Agent sessions may still be signed in.",
|
||||
"45198c7d95": "1 rate-limit reset available",
|
||||
"bce421cba3": "{{value0}} rate-limit resets available"
|
||||
"bce421cba3": "{{value0}} rate-limit resets available",
|
||||
"7ec6e030a0": "Next expires now",
|
||||
"d1e442a9e5": "Expires now",
|
||||
"6cf9eaed10": "Next expires in {{value0}}",
|
||||
"20ad66aed1": "Expires in {{value0}}"
|
||||
},
|
||||
"SshTargetStatusRow": {
|
||||
"sshHost": "SSH Host"
|
||||
|
|
|
|||
|
|
@ -2912,7 +2912,11 @@
|
|||
"e740f92596": "Error al actualizar",
|
||||
"8418ec448d": "No se pudo actualizar el uso de {{value0}}. Es posible que las sesiones de agente sigan iniciadas.",
|
||||
"45198c7d95": "1 rate-limit reset available",
|
||||
"bce421cba3": "{{value0}} rate-limit resets available"
|
||||
"bce421cba3": "{{value0}} rate-limit resets available",
|
||||
"7ec6e030a0": "Next expires now",
|
||||
"d1e442a9e5": "Expires now",
|
||||
"6cf9eaed10": "Next expires in {{value0}}",
|
||||
"20ad66aed1": "Expires in {{value0}}"
|
||||
},
|
||||
"SshTargetStatusRow": {
|
||||
"sshHost": "SSH Host"
|
||||
|
|
|
|||
|
|
@ -2912,7 +2912,11 @@
|
|||
"e740f92596": "更新に失敗しました",
|
||||
"8418ec448d": "{{value0}} の使用状況を更新できませんでした。Agent セッションは引き続きサインイン済みの場合があります。",
|
||||
"45198c7d95": "1 rate-limit reset available",
|
||||
"bce421cba3": "{{value0}} rate-limit resets available"
|
||||
"bce421cba3": "{{value0}} rate-limit resets available",
|
||||
"7ec6e030a0": "Next expires now",
|
||||
"d1e442a9e5": "Expires now",
|
||||
"6cf9eaed10": "Next expires in {{value0}}",
|
||||
"20ad66aed1": "Expires in {{value0}}"
|
||||
},
|
||||
"SshTargetStatusRow": {
|
||||
"sshHost": "SSH Host"
|
||||
|
|
|
|||
|
|
@ -2912,7 +2912,11 @@
|
|||
"e740f92596": "새로 고침 실패",
|
||||
"8418ec448d": "{{value0}} 사용량을 새로 고칠 수 없습니다. Agent 세션은 여전히 로그인되어 있을 수 있습니다.",
|
||||
"45198c7d95": "rate-limit 재설정 1회 사용 가능",
|
||||
"bce421cba3": "rate-limit 재설정 {{value0}}회 사용 가능"
|
||||
"bce421cba3": "rate-limit 재설정 {{value0}}회 사용 가능",
|
||||
"7ec6e030a0": "Next expires now",
|
||||
"d1e442a9e5": "Expires now",
|
||||
"6cf9eaed10": "Next expires in {{value0}}",
|
||||
"20ad66aed1": "Expires in {{value0}}"
|
||||
},
|
||||
"SshTargetStatusRow": {
|
||||
"sshHost": "SSH 호스트"
|
||||
|
|
|
|||
|
|
@ -2912,7 +2912,11 @@
|
|||
"e740f92596": "刷新失败",
|
||||
"8418ec448d": "{{value0}} 用量无法刷新。智能体会话可能仍处于登录状态。",
|
||||
"45198c7d95": "1 rate-limit reset available",
|
||||
"bce421cba3": "{{value0}} rate-limit resets available"
|
||||
"bce421cba3": "{{value0}} rate-limit resets available",
|
||||
"7ec6e030a0": "Next expires now",
|
||||
"d1e442a9e5": "Expires now",
|
||||
"6cf9eaed10": "Next expires in {{value0}}",
|
||||
"20ad66aed1": "Expires in {{value0}}"
|
||||
},
|
||||
"SshTargetStatusRow": {
|
||||
"sshHost": "SSH 主机"
|
||||
|
|
|
|||
Loading…
Reference in New Issue