mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
Fix SSH worktree delete routing after host reconnect (#7997)
* Fix SSH worktree delete routing * Harden host-scoped worktree deletion Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Jinwoo-H <jinwoo0825@gmail.com> Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
co-authored by
Orca
Jinwoo-H
parent
c633a634ba
commit
8976bd969c
@@ -6935,6 +6935,169 @@ describe('registerWorktreeHandlers', () => {
|
||||
expect(provider.removeWorktree).toHaveBeenCalledWith('/remote/feature-wt', undefined)
|
||||
})
|
||||
|
||||
it('uses the workspace host when duplicate repo ids exist across local and SSH', async () => {
|
||||
const localRepo = {
|
||||
id: 'repo-shared',
|
||||
path: '/local/repo',
|
||||
displayName: 'local',
|
||||
badgeColor: '#000',
|
||||
addedAt: 0,
|
||||
worktreeBaseRef: null
|
||||
}
|
||||
const sshRepo = {
|
||||
...localRepo,
|
||||
path: '/remote/repo',
|
||||
displayName: 'ssh',
|
||||
connectionId: 'conn-1'
|
||||
}
|
||||
const provider = {
|
||||
listWorktrees: vi.fn().mockResolvedValue([
|
||||
{
|
||||
path: sshRepo.path,
|
||||
head: 'main',
|
||||
branch: 'main',
|
||||
isBare: false,
|
||||
isMainWorktree: true
|
||||
},
|
||||
{
|
||||
path: '/remote/feature-wt',
|
||||
head: 'feature',
|
||||
branch: 'feature',
|
||||
isBare: false,
|
||||
isMainWorktree: false
|
||||
}
|
||||
]),
|
||||
removeWorktree: vi.fn().mockResolvedValue(undefined),
|
||||
worktreeIsClean: vi.fn().mockResolvedValue({ clean: true })
|
||||
}
|
||||
store.getRepo.mockReturnValue(localRepo)
|
||||
store.getRepos.mockReturnValue([localRepo, sshRepo])
|
||||
getSshGitProviderMock.mockReturnValue(provider)
|
||||
|
||||
await handlers['worktrees:remove'](null, {
|
||||
worktreeId: 'repo-shared::/remote/feature-wt',
|
||||
hostId: 'ssh:conn-1'
|
||||
})
|
||||
|
||||
expect(provider.removeWorktree).toHaveBeenCalledWith('/remote/feature-wt', undefined)
|
||||
expect(removeWorktreeMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('fails closed when duplicate repo ids are deleted without a host', async () => {
|
||||
const localRepo = {
|
||||
id: 'repo-shared',
|
||||
path: '/local/repo',
|
||||
displayName: 'local',
|
||||
badgeColor: '#000',
|
||||
addedAt: 0
|
||||
}
|
||||
const sshRepo = { ...localRepo, path: '/remote/repo', connectionId: 'conn-1' }
|
||||
store.getRepos.mockReturnValue([localRepo, sshRepo])
|
||||
|
||||
await expect(
|
||||
handlers['worktrees:remove'](null, {
|
||||
worktreeId: 'repo-shared::/remote/feature-wt'
|
||||
})
|
||||
).rejects.toThrow('Repo not found: repo-shared')
|
||||
|
||||
expect(removeWorktreeMock).not.toHaveBeenCalled()
|
||||
expect(getSshGitProviderMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('inspects hooks on the requested host when repo ids collide', async () => {
|
||||
const localRepo = {
|
||||
id: 'repo-shared',
|
||||
path: '/local/repo',
|
||||
displayName: 'local',
|
||||
badgeColor: '#000',
|
||||
addedAt: 0
|
||||
}
|
||||
const sshRepo = { ...localRepo, path: '/remote/repo', connectionId: 'conn-1' }
|
||||
const fsProvider = {
|
||||
readFile: vi.fn().mockResolvedValue({
|
||||
content: 'scripts:\n archive: remote-cleanup',
|
||||
isBinary: false
|
||||
})
|
||||
}
|
||||
store.getRepos.mockReturnValue([localRepo, sshRepo])
|
||||
getSshFilesystemProviderMock.mockReturnValue(fsProvider)
|
||||
parseOrcaYamlMock.mockReturnValue({ scripts: { archive: 'remote-cleanup' } })
|
||||
|
||||
await expect(
|
||||
handlers['hooks:check'](null, {
|
||||
repoId: 'repo-shared',
|
||||
hostId: 'ssh:conn-1'
|
||||
})
|
||||
).resolves.toEqual({
|
||||
status: 'ok',
|
||||
hasHooks: true,
|
||||
hooks: { scripts: { archive: 'remote-cleanup' } },
|
||||
mayNeedUpdate: false
|
||||
})
|
||||
expect(fsProvider.readFile).toHaveBeenCalledWith('/remote/repo/orca.yaml')
|
||||
expect(hasHooksFileMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('fails hook inspection closed when duplicate repo ids omit the host', async () => {
|
||||
const localRepo = {
|
||||
id: 'repo-shared',
|
||||
path: '/local/repo',
|
||||
displayName: 'local',
|
||||
badgeColor: '#000',
|
||||
addedAt: 0
|
||||
}
|
||||
const sshRepo = { ...localRepo, path: '/remote/repo', connectionId: 'conn-1' }
|
||||
store.getRepos.mockReturnValue([localRepo, sshRepo])
|
||||
|
||||
await expect(handlers['hooks:check'](null, { repoId: 'repo-shared' })).resolves.toEqual({
|
||||
status: 'error',
|
||||
hasHooks: false,
|
||||
hooks: null,
|
||||
mayNeedUpdate: false
|
||||
})
|
||||
expect(getSshFilesystemProviderMock).not.toHaveBeenCalled()
|
||||
expect(hasHooksFileMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not coalesce forget requests for the same id on different hosts', async () => {
|
||||
const localRepo = {
|
||||
id: 'repo-shared',
|
||||
path: '/local/repo',
|
||||
displayName: 'local',
|
||||
badgeColor: '#000',
|
||||
addedAt: 0
|
||||
}
|
||||
const sshRepo = { ...localRepo, path: '/remote/repo', connectionId: 'conn-1' }
|
||||
store.getRepos.mockReturnValue([localRepo, sshRepo])
|
||||
let finishFirst!: () => void
|
||||
killAllProcessesForWorktreeMock
|
||||
.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
finishFirst = () =>
|
||||
resolve({ runtimeStopped: 0, providerStopped: 0, registryStopped: 0 })
|
||||
})
|
||||
)
|
||||
.mockResolvedValueOnce({ runtimeStopped: 0, providerStopped: 0, registryStopped: 0 })
|
||||
|
||||
const first = handlers['worktrees:forgetLocal'](null, {
|
||||
worktreeId: 'repo-shared::/same/path',
|
||||
hostId: 'local'
|
||||
}) as Promise<unknown>
|
||||
await vi.waitFor(() => expect(killAllProcessesForWorktreeMock).toHaveBeenCalledTimes(1))
|
||||
|
||||
await expect(
|
||||
handlers['worktrees:forgetLocal'](null, {
|
||||
worktreeId: 'repo-shared::/same/path',
|
||||
hostId: 'ssh:conn-1'
|
||||
})
|
||||
).resolves.toEqual({})
|
||||
expect(killAllProcessesForWorktreeMock).toHaveBeenCalledTimes(2)
|
||||
|
||||
finishFirst()
|
||||
await expect(first).resolves.toEqual({})
|
||||
})
|
||||
|
||||
it('preserves the branch on remove for worktrees created from an existing local branch', async () => {
|
||||
mockKnownFeatureWorktree()
|
||||
removeWorktreeMock.mockResolvedValue(undefined)
|
||||
@@ -7257,14 +7420,16 @@ describe('registerWorktreeHandlers', () => {
|
||||
await mkdir(adminWorktreePath, { recursive: true })
|
||||
await writeFile(join(orphanPath, '.git'), `gitdir: ${adminWorktreePath}\n`)
|
||||
await writeFile(join(adminWorktreePath, 'gitdir'), `${join(orphanPath, '.git')}\n`)
|
||||
store.getRepo.mockReturnValue({
|
||||
const repo = {
|
||||
id: 'repo-1',
|
||||
path: repoPath,
|
||||
displayName: 'repo',
|
||||
badgeColor: '#000',
|
||||
addedAt: 0,
|
||||
worktreeBaseRef: null
|
||||
})
|
||||
}
|
||||
store.getRepo.mockReturnValue(repo)
|
||||
store.getRepos.mockReturnValue([repo])
|
||||
mockKnownFeatureWorktree(join(parentDir, 'real-feature'), repoPath)
|
||||
store.getWorktreeMeta.mockReturnValue(makeWorktreeMeta({ createdAt: Date.now() }))
|
||||
|
||||
@@ -7298,14 +7463,16 @@ describe('registerWorktreeHandlers', () => {
|
||||
await mkdir(adminWorktreePath, { recursive: true })
|
||||
await writeFile(join(orphanPath, '.git'), `gitdir: ${adminWorktreePath}\n`)
|
||||
await writeFile(join(adminWorktreePath, 'gitdir'), `${join(orphanPath, '.git')}\n`)
|
||||
store.getRepo.mockReturnValue({
|
||||
const repo = {
|
||||
id: 'repo-1',
|
||||
path: repoPath,
|
||||
displayName: 'repo',
|
||||
badgeColor: '#000',
|
||||
addedAt: 0,
|
||||
worktreeBaseRef: null
|
||||
})
|
||||
}
|
||||
store.getRepo.mockReturnValue(repo)
|
||||
store.getRepos.mockReturnValue([repo])
|
||||
mockKnownFeatureWorktree(join(parentDir, 'real-feature'), repoPath)
|
||||
store.getWorktreeMeta.mockReturnValue(
|
||||
makeWorktreeMeta({ orcaCreatedAt: Date.now(), orcaCreationSource: 'runtime' })
|
||||
@@ -7527,6 +7694,7 @@ describe('registerWorktreeHandlers', () => {
|
||||
}) as Promise<unknown>
|
||||
const second = handlers['worktrees:remove'](null, {
|
||||
worktreeId: 'repo-1::/workspace/feature-wt',
|
||||
hostId: 'local',
|
||||
force: true
|
||||
}) as Promise<unknown>
|
||||
|
||||
@@ -7564,6 +7732,7 @@ describe('registerWorktreeHandlers', () => {
|
||||
await expect(
|
||||
handlers['worktrees:remove'](null, {
|
||||
worktreeId: 'repo-1::/workspace/feature-wt',
|
||||
hostId: 'local',
|
||||
force: true
|
||||
})
|
||||
).rejects.toThrow('Worktree deletion already in progress')
|
||||
|
||||
+114
-55
@@ -29,6 +29,7 @@ import type {
|
||||
Worktree,
|
||||
WorktreeMeta
|
||||
} from '../../shared/types'
|
||||
import { getRepoExecutionHostId, type ExecutionHostId } from '../../shared/execution-host'
|
||||
import {
|
||||
buildKnownOrcaWorkspaceLayouts,
|
||||
isLegacyRepoForExternalWorktreeVisibility,
|
||||
@@ -101,6 +102,35 @@ import {
|
||||
type CreateWorktreeArgsWithSystemProvenance = CreateWorktreeArgs & {
|
||||
automationProvenance?: AutomationWorkspaceProvenance
|
||||
}
|
||||
|
||||
type RemoveWorktreeArgs = {
|
||||
worktreeId: string
|
||||
hostId?: ExecutionHostId
|
||||
force?: boolean
|
||||
skipArchive?: boolean
|
||||
}
|
||||
|
||||
function getRepoForWorktreeRemoval(
|
||||
store: Store,
|
||||
repoId: string,
|
||||
hostId?: ExecutionHostId
|
||||
): Repo | undefined {
|
||||
const matches = store
|
||||
.getRepos()
|
||||
.filter((repo) => repo.id === repoId && (!hostId || getRepoExecutionHostId(repo) === hostId))
|
||||
// Why: deletion must never guess between host owners. Legacy unscoped calls
|
||||
// remain compatible only while the repo id still has one unique owner.
|
||||
if (matches.length === 1) {
|
||||
return matches[0]
|
||||
}
|
||||
if (matches.length > 1) {
|
||||
return undefined
|
||||
}
|
||||
const legacyMatch = store.getRepo(repoId)
|
||||
return legacyMatch && (!hostId || getRepoExecutionHostId(legacyMatch) === hostId)
|
||||
? legacyMatch
|
||||
: undefined
|
||||
}
|
||||
import { classifyWorkspaceCreateError } from './workspace-create-error-classifier'
|
||||
import { advertisedUrlWatcher } from '../ports/advertised-url-watcher'
|
||||
import { localhostWorktreeLabelProxy } from '../localhost-worktree-label-proxy'
|
||||
@@ -300,6 +330,10 @@ function getWorktreeRemovalOptionsKey(args: { force?: boolean; skipArchive?: boo
|
||||
return `${forceKey}:${archiveKey}`
|
||||
}
|
||||
|
||||
function getWorktreeRemovalInFlightKey(worktreeId: string, hostId?: ExecutionHostId): string {
|
||||
return `${hostId ?? ''}\0${worktreeId}`
|
||||
}
|
||||
|
||||
async function getArchiveHooksForRemoval(repo: Repo): Promise<OrcaHooks | null> {
|
||||
if (!repo.connectionId) {
|
||||
return getEffectiveHooks(repo)
|
||||
@@ -1345,9 +1379,18 @@ export function registerWorktreeHandlers(
|
||||
|
||||
ipcMain.handle(
|
||||
'worktrees:remove',
|
||||
async (_event, args: { worktreeId: string; force?: boolean; skipArchive?: boolean }) => {
|
||||
async (_event, args: RemoveWorktreeArgs): Promise<RemoveWorktreeResult> => {
|
||||
const { repoId, worktreePath } = parseWorktreeId(args.worktreeId)
|
||||
const repo = getRepoForWorktreeRemoval(store, repoId, args.hostId)
|
||||
if (!repo) {
|
||||
throw new Error(`Repo not found: ${repoId}`)
|
||||
}
|
||||
const inFlightKey = getWorktreeRemovalInFlightKey(
|
||||
args.worktreeId,
|
||||
getRepoExecutionHostId(repo)
|
||||
)
|
||||
const optionsKey = getWorktreeRemovalOptionsKey(args)
|
||||
const inFlightRemoval = worktreeRemovalsInFlight.get(args.worktreeId)
|
||||
const inFlightRemoval = worktreeRemovalsInFlight.get(inFlightKey)
|
||||
if (inFlightRemoval) {
|
||||
if (inFlightRemoval.optionsKey === optionsKey) {
|
||||
return inFlightRemoval.promise
|
||||
@@ -1359,11 +1402,6 @@ export function registerWorktreeHandlers(
|
||||
// target the same worktree concurrently. Share the destructive backend
|
||||
// operation so only one path touches Git and the filesystem.
|
||||
const removal = (async (): Promise<RemoveWorktreeResult> => {
|
||||
const { repoId, worktreePath } = parseWorktreeId(args.worktreeId)
|
||||
const repo = store.getRepo(repoId)
|
||||
if (!repo) {
|
||||
throw new Error(`Repo not found: ${repoId}`)
|
||||
}
|
||||
if (isFolderRepo(repo)) {
|
||||
if (args.worktreeId === getFolderWorkspaceRootId(repo)) {
|
||||
throw new Error(
|
||||
@@ -1797,12 +1835,12 @@ export function registerWorktreeHandlers(
|
||||
notifyWorktreesChanged(mainWindow, repoId)
|
||||
return removalResult ?? {}
|
||||
})()
|
||||
worktreeRemovalsInFlight.set(args.worktreeId, { optionsKey, promise: removal })
|
||||
worktreeRemovalsInFlight.set(inFlightKey, { optionsKey, promise: removal })
|
||||
try {
|
||||
return await removal
|
||||
} finally {
|
||||
if (worktreeRemovalsInFlight.get(args.worktreeId)?.promise === removal) {
|
||||
worktreeRemovalsInFlight.delete(args.worktreeId)
|
||||
if (worktreeRemovalsInFlight.get(inFlightKey)?.promise === removal) {
|
||||
worktreeRemovalsInFlight.delete(inFlightKey)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1816,12 +1854,24 @@ export function registerWorktreeHandlers(
|
||||
// no branches, no files are deleted there.
|
||||
ipcMain.handle(
|
||||
'worktrees:forgetLocal',
|
||||
async (_event, args: { worktreeId: string }): Promise<RemoveWorktreeResult> => {
|
||||
async (
|
||||
_event,
|
||||
args: Pick<RemoveWorktreeArgs, 'worktreeId' | 'hostId'>
|
||||
): Promise<RemoveWorktreeResult> => {
|
||||
const { repoId } = parseWorktreeId(args.worktreeId)
|
||||
const repo = getRepoForWorktreeRemoval(store, repoId, args.hostId)
|
||||
if (!repo) {
|
||||
throw new Error(`Repo not found: ${repoId}`)
|
||||
}
|
||||
// Why: share the removal in-flight map (not a separate one) so a concurrent
|
||||
// worktrees:remove and worktrees:forgetLocal on the same id cannot both
|
||||
// mutate metadata. A forget takes no force/skipArchive options.
|
||||
const inFlightKey = getWorktreeRemovalInFlightKey(
|
||||
args.worktreeId,
|
||||
getRepoExecutionHostId(repo)
|
||||
)
|
||||
const optionsKey = 'forget-local'
|
||||
const inFlight = worktreeRemovalsInFlight.get(args.worktreeId)
|
||||
const inFlight = worktreeRemovalsInFlight.get(inFlightKey)
|
||||
if (inFlight) {
|
||||
if (inFlight.optionsKey === optionsKey) {
|
||||
return inFlight.promise
|
||||
@@ -1830,11 +1880,6 @@ export function registerWorktreeHandlers(
|
||||
}
|
||||
|
||||
const forget = (async (): Promise<RemoveWorktreeResult> => {
|
||||
const { repoId } = parseWorktreeId(args.worktreeId)
|
||||
const repo = store.getRepo(repoId)
|
||||
if (!repo) {
|
||||
throw new Error(`Repo not found: ${repoId}`)
|
||||
}
|
||||
if (isFolderRepo(repo) && args.worktreeId === getFolderWorkspaceRootId(repo)) {
|
||||
throw new Error(
|
||||
'Cannot delete the project root workspace. Remove the folder project instead.'
|
||||
@@ -1858,12 +1903,12 @@ export function registerWorktreeHandlers(
|
||||
notifyWorktreesChanged(mainWindow, repoId)
|
||||
return {}
|
||||
})()
|
||||
worktreeRemovalsInFlight.set(args.worktreeId, { optionsKey, promise: forget })
|
||||
worktreeRemovalsInFlight.set(inFlightKey, { optionsKey, promise: forget })
|
||||
try {
|
||||
return await forget
|
||||
} finally {
|
||||
if (worktreeRemovalsInFlight.get(args.worktreeId)?.promise === forget) {
|
||||
worktreeRemovalsInFlight.delete(args.worktreeId)
|
||||
if (worktreeRemovalsInFlight.get(inFlightKey)?.promise === forget) {
|
||||
worktreeRemovalsInFlight.delete(inFlightKey)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1996,49 +2041,63 @@ export function registerWorktreeHandlers(
|
||||
}
|
||||
})
|
||||
|
||||
ipcMain.handle('hooks:check', async (_event, args: { repoId: string }) => {
|
||||
const repo = store.getRepo(args.repoId)
|
||||
if (!repo || isFolderRepo(repo)) {
|
||||
return { status: 'ok', hasHooks: false, hooks: null, mayNeedUpdate: false }
|
||||
}
|
||||
|
||||
if (repo.connectionId) {
|
||||
const fsProvider = getSshFilesystemProvider(repo.connectionId)
|
||||
if (!fsProvider) {
|
||||
return { status: 'error', hasHooks: false, hooks: null, mayNeedUpdate: false }
|
||||
}
|
||||
try {
|
||||
const result = await fsProvider.readFile(joinWorktreeRelativePath(repo.path, 'orca.yaml'))
|
||||
ipcMain.handle(
|
||||
'hooks:check',
|
||||
async (_event, args: { repoId: string; hostId?: ExecutionHostId }) => {
|
||||
const repo = getRepoForWorktreeRemoval(store, args.repoId, args.hostId)
|
||||
if (!repo) {
|
||||
const repoIdExists = store.getRepos().some((candidate) => candidate.id === args.repoId)
|
||||
// Why: a requested or ambiguous host must not be reported as hook-free;
|
||||
// callers treat inspection errors as "skip", which keeps hook execution fail closed.
|
||||
return {
|
||||
status: 'ok',
|
||||
hasHooks: !result.isBinary,
|
||||
hooks: result.isBinary ? null : parseOrcaYaml(result.content),
|
||||
mayNeedUpdate: false
|
||||
}
|
||||
} catch (error) {
|
||||
return {
|
||||
status: isENOENT(error) ? 'ok' : 'error',
|
||||
status: args.hostId || repoIdExists ? 'error' : 'ok',
|
||||
hasHooks: false,
|
||||
hooks: null,
|
||||
mayNeedUpdate: false
|
||||
}
|
||||
}
|
||||
}
|
||||
if (isFolderRepo(repo)) {
|
||||
return { status: 'ok', hasHooks: false, hooks: null, mayNeedUpdate: false }
|
||||
}
|
||||
|
||||
const has = hasHooksFile(repo.path)
|
||||
const hooks = has ? loadHooks(repo.path) : null
|
||||
// Why: when a newer Orca version adds a top-level key to `orca.yaml`, older
|
||||
// versions that don't recognise it return null and show "could not be parsed".
|
||||
// Detecting well-formed but unrecognised keys lets the UI suggest updating
|
||||
// instead of implying the file is broken.
|
||||
const mayNeedUpdate = has && !hooks && hasUnrecognizedOrcaYamlKeys(repo.path)
|
||||
return {
|
||||
status: 'ok',
|
||||
hasHooks: has,
|
||||
hooks,
|
||||
mayNeedUpdate
|
||||
if (repo.connectionId) {
|
||||
const fsProvider = getSshFilesystemProvider(repo.connectionId)
|
||||
if (!fsProvider) {
|
||||
return { status: 'error', hasHooks: false, hooks: null, mayNeedUpdate: false }
|
||||
}
|
||||
try {
|
||||
const result = await fsProvider.readFile(joinWorktreeRelativePath(repo.path, 'orca.yaml'))
|
||||
return {
|
||||
status: 'ok',
|
||||
hasHooks: !result.isBinary,
|
||||
hooks: result.isBinary ? null : parseOrcaYaml(result.content),
|
||||
mayNeedUpdate: false
|
||||
}
|
||||
} catch (error) {
|
||||
return {
|
||||
status: isENOENT(error) ? 'ok' : 'error',
|
||||
hasHooks: false,
|
||||
hooks: null,
|
||||
mayNeedUpdate: false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const has = hasHooksFile(repo.path)
|
||||
const hooks = has ? loadHooks(repo.path) : null
|
||||
// Why: when a newer Orca version adds a top-level key to `orca.yaml`, older
|
||||
// versions that don't recognise it return null and show "could not be parsed".
|
||||
// Detecting well-formed but unrecognised keys lets the UI suggest updating
|
||||
// instead of implying the file is broken.
|
||||
const mayNeedUpdate = has && !hooks && hasUnrecognizedOrcaYamlKeys(repo.path)
|
||||
return {
|
||||
status: 'ok',
|
||||
hasHooks: has,
|
||||
hooks,
|
||||
mayNeedUpdate
|
||||
}
|
||||
}
|
||||
})
|
||||
)
|
||||
|
||||
ipcMain.handle(
|
||||
'hooks:createIssueCommandRunner',
|
||||
|
||||
@@ -1130,12 +1130,16 @@ export type PreloadApi = {
|
||||
>
|
||||
remove: (args: {
|
||||
worktreeId: string
|
||||
hostId?: ExecutionHostId
|
||||
force?: boolean
|
||||
skipArchive?: boolean
|
||||
}) => Promise<RemoveWorktreeResult>
|
||||
// Forget a workspace from Orca only — no remote Git/filesystem work. Used
|
||||
// for workspaces pinned to a removed/disconnected SSH host.
|
||||
forgetLocal: (args: { worktreeId: string }) => Promise<RemoveWorktreeResult>
|
||||
forgetLocal: (args: {
|
||||
worktreeId: string
|
||||
hostId?: ExecutionHostId
|
||||
}) => Promise<RemoveWorktreeResult>
|
||||
forceDeletePreservedBranch: (args: {
|
||||
worktreeId: string
|
||||
branchName: string
|
||||
@@ -2131,7 +2135,7 @@ export type PreloadApi = {
|
||||
browser: BrowserApi
|
||||
emulator: EmulatorApi
|
||||
hooks: {
|
||||
check: (args: { repoId: string }) => Promise<{
|
||||
check: (args: { repoId: string; hostId?: ExecutionHostId }) => Promise<{
|
||||
status?: 'ok' | 'error'
|
||||
hasHooks: boolean
|
||||
hooks: OrcaHooks | null
|
||||
|
||||
@@ -142,6 +142,7 @@ import type { TelemetryConsentState } from '../shared/telemetry-consent-types'
|
||||
import type { PreflightRuntimeContext, RefreshAgentsResult } from './api-types'
|
||||
import type { AgentKind, LaunchSource, RequestKind } from '../shared/telemetry-events'
|
||||
import type { AppStarSource } from '../shared/gh-star-source'
|
||||
import type { ExecutionHostId } from '../shared/execution-host'
|
||||
import type {
|
||||
Automation,
|
||||
AutomationCreateInput,
|
||||
@@ -2478,6 +2479,7 @@ const api = {
|
||||
hooks: {
|
||||
check: (args: {
|
||||
repoId: string
|
||||
hostId?: ExecutionHostId
|
||||
}): Promise<{
|
||||
status?: 'ok' | 'error'
|
||||
hasHooks: boolean
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { toast } from 'sonner'
|
||||
import { useAppStore } from '@/store'
|
||||
import { getWorktreeMapFromState } from '@/store/selectors'
|
||||
import { findRepoForHost } from '@/store/slices/repo-host-identity'
|
||||
import { activateAndRevealWorktree } from '@/lib/worktree-activation'
|
||||
import { prepareActiveWorktreeFocusAfterDelete } from './active-worktree-focus-after-delete'
|
||||
import { showDeleteWorktreeFailureToast } from './delete-worktree-failure-toast'
|
||||
@@ -262,7 +263,12 @@ export function runWorktreeDelete(worktreeId: string): void {
|
||||
// misclassify every SSH repo as a ghost, routing to a forget dialog whose
|
||||
// local-only backend is unavailable there. Their normal worktree.rm RPC path
|
||||
// already handles the delete against the desktop runtime.
|
||||
const repo = state.repos.find((entry) => entry.id === target.repoId) ?? null
|
||||
const matchingRepos = state.repos.filter((entry) => entry.id === target.repoId)
|
||||
const repo = target.hostId
|
||||
? findRepoForHost(matchingRepos, target.repoId, { hostId: target.hostId })
|
||||
: matchingRepos.length === 1
|
||||
? matchingRepos[0]
|
||||
: null
|
||||
const sshResolution = isPairedWebClientWindow()
|
||||
? { kind: 'not-ssh' as const }
|
||||
: resolveSshWorkspaceForget({
|
||||
|
||||
@@ -225,6 +225,28 @@ describe('ensureHooksConfirmed', () => {
|
||||
expect(pending).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('inspects the requested host when duplicate repo ids exist', async () => {
|
||||
const { state } = createTestState({
|
||||
settings: { activeRuntimeEnvironmentId: 'env-1' },
|
||||
trustedOrcaHooks: { 'repo-1': { all: { approvedAt: 1 } } },
|
||||
repos: [
|
||||
{ id: 'repo-1', displayName: 'Runtime', executionHostId: 'runtime:env-1' },
|
||||
{ id: 'repo-1', displayName: 'SSH', connectionId: 'ssh-1' }
|
||||
]
|
||||
} as unknown as Partial<AppState>)
|
||||
hooksCheckMock.mockResolvedValue({
|
||||
hasHooks: true,
|
||||
hooks: { scripts: {} },
|
||||
mayNeedUpdate: false
|
||||
})
|
||||
|
||||
const decision = await ensureHooksConfirmed(state, 'repo-1', 'archive', 'ssh:ssh-1')
|
||||
|
||||
expect(decision).toBe('run')
|
||||
expect(hooksCheckMock).toHaveBeenCalledWith({ repoId: 'repo-1', hostId: 'ssh:ssh-1' })
|
||||
expect(runtimeEnvironmentCallMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('checks runtime-owned repo hooks through the repo owner runtime', async () => {
|
||||
const { state, pending } = createTestState({
|
||||
settings: { activeRuntimeEnvironmentId: 'focused-env' },
|
||||
|
||||
@@ -4,6 +4,11 @@ import { resolveHookCommandSourcePolicy } from '../../../shared/hook-command-sou
|
||||
import { hashOrcaHookScript, type OrcaHookScriptKind } from './orca-hook-trust'
|
||||
import { checkRuntimeHooks, readRuntimeIssueCommand } from '@/runtime/runtime-hooks-client'
|
||||
import { getRuntimeEnvironmentIdForRepo } from './repo-runtime-owner'
|
||||
import {
|
||||
getRepoExecutionHostId,
|
||||
parseExecutionHostId,
|
||||
type ExecutionHostId
|
||||
} from '../../../shared/execution-host'
|
||||
|
||||
export type HookScriptKind = OrcaHookScriptKind
|
||||
|
||||
@@ -56,8 +61,23 @@ function getVmRecipeTrustContent(yamlHooks: OrcaHooks | null): string {
|
||||
.join('\n\n')
|
||||
}
|
||||
|
||||
function settingsForHookRepoOwner(state: AppState, repoId: string): AppState['settings'] {
|
||||
const runtimeEnvironmentId = getRuntimeEnvironmentIdForRepo(state, repoId)
|
||||
function findHookRepo(state: AppState, repoId: string, hostId?: ExecutionHostId) {
|
||||
return hostId
|
||||
? state.repos.find((repo) => repo.id === repoId && getRepoExecutionHostId(repo) === hostId)
|
||||
: state.repos.find((repo) => repo.id === repoId)
|
||||
}
|
||||
|
||||
function settingsForHookRepoOwner(
|
||||
state: AppState,
|
||||
repoId: string,
|
||||
hostId?: ExecutionHostId
|
||||
): AppState['settings'] {
|
||||
const parsedHost = hostId ? parseExecutionHostId(hostId) : null
|
||||
const runtimeEnvironmentId = hostId
|
||||
? parsedHost?.kind === 'runtime'
|
||||
? parsedHost.environmentId
|
||||
: null
|
||||
: getRuntimeEnvironmentIdForRepo(state, repoId)
|
||||
// Why: hook inspection must follow the repo owner. SSH/local repos execute
|
||||
// through desktop IPC, while runtime repos may differ from the focused host.
|
||||
return state.settings
|
||||
@@ -68,10 +88,12 @@ function settingsForHookRepoOwner(state: AppState, repoId: string): AppState['se
|
||||
export async function ensureHooksConfirmed(
|
||||
state: AppState,
|
||||
repoId: string,
|
||||
scriptKind: HookScriptKind
|
||||
scriptKind: HookScriptKind,
|
||||
hostId?: ExecutionHostId
|
||||
): Promise<'run' | 'skip'> {
|
||||
return enqueueTrustPrompt(async () => {
|
||||
if (state.trustedOrcaHooks[repoId]?.all) {
|
||||
const hasDuplicateRepoId = state.repos.filter((repo) => repo.id === repoId).length > 1
|
||||
if (state.trustedOrcaHooks[repoId]?.all && !(hostId && hasDuplicateRepoId)) {
|
||||
return 'run'
|
||||
}
|
||||
|
||||
@@ -80,7 +102,7 @@ export async function ensureHooksConfirmed(
|
||||
if (scriptKind === 'issueCommand') {
|
||||
// Local overrides are user-owned; only shared orca.yaml commands need repo trust.
|
||||
const result = await readRuntimeIssueCommand(
|
||||
settingsForHookRepoOwner(state, repoId),
|
||||
settingsForHookRepoOwner(state, repoId, hostId),
|
||||
repoId
|
||||
)
|
||||
if (result.source === 'local') {
|
||||
@@ -94,7 +116,7 @@ export async function ensureHooksConfirmed(
|
||||
}
|
||||
scriptContent = (result.sharedContent ?? '').trim()
|
||||
} else {
|
||||
const repo = state.repos.find((r) => r.id === repoId)
|
||||
const repo = findHookRepo(state, repoId, hostId)
|
||||
const localScript = repo?.hookSettings?.scripts?.[scriptKind]?.trim()
|
||||
const sourcePolicy = resolveHookCommandSourcePolicy(
|
||||
repo?.hookSettings?.commandSourcePolicy,
|
||||
@@ -105,7 +127,11 @@ export async function ensureHooksConfirmed(
|
||||
if (sourcePolicy === 'local-only') {
|
||||
return 'run'
|
||||
}
|
||||
const result = await checkRuntimeHooks(settingsForHookRepoOwner(state, repoId), repoId)
|
||||
const result = await checkRuntimeHooks(
|
||||
settingsForHookRepoOwner(state, repoId, hostId),
|
||||
repoId,
|
||||
hostId
|
||||
)
|
||||
if (result.status === 'error') {
|
||||
return 'skip'
|
||||
}
|
||||
@@ -132,7 +158,7 @@ export async function ensureHooksConfirmed(
|
||||
return 'run'
|
||||
}
|
||||
|
||||
const repo = state.repos.find((r) => r.id === repoId)
|
||||
const repo = findHookRepo(state, repoId, hostId)
|
||||
const repoName = repo?.displayName ?? 'this repository'
|
||||
// A non-empty existingHash that didn't match means the user approved a previous
|
||||
// version of this script; the prompt is reappearing because orca.yaml changed.
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { GlobalSettings, OrcaHooks } from '../../../shared/types'
|
||||
import type { ExecutionHostId } from '../../../shared/execution-host'
|
||||
import type { SetupScriptImportCandidate } from '../../../shared/setup-script-imports'
|
||||
import { callRuntimeRpc, getActiveRuntimeTarget } from './runtime-rpc-client'
|
||||
|
||||
@@ -20,11 +21,12 @@ export type IssueCommandReadResult = {
|
||||
|
||||
export async function checkRuntimeHooks(
|
||||
settings: Pick<GlobalSettings, 'activeRuntimeEnvironmentId'> | null | undefined,
|
||||
repoId: string
|
||||
repoId: string,
|
||||
hostId?: ExecutionHostId
|
||||
): Promise<HookCheckResult> {
|
||||
const target = getActiveRuntimeTarget(settings)
|
||||
if (target.kind !== 'environment') {
|
||||
return window.api.hooks.check({ repoId })
|
||||
return window.api.hooks.check({ repoId, ...(hostId ? { hostId } : {}) })
|
||||
}
|
||||
return callRuntimeRpc<HookCheckResult>(
|
||||
target,
|
||||
|
||||
@@ -4101,6 +4101,7 @@ describe('worktree remote runtime mutations', () => {
|
||||
expect(result).toEqual({ ok: true })
|
||||
expect(mockApi.worktrees.remove).toHaveBeenCalledWith({
|
||||
worktreeId: wt.id,
|
||||
hostId: 'ssh:ssh-1',
|
||||
force: undefined,
|
||||
skipArchive: false
|
||||
})
|
||||
@@ -4108,6 +4109,39 @@ describe('worktree remote runtime mutations', () => {
|
||||
expect(store.getState().worktreesByRepo['repo-ssh']).toEqual([])
|
||||
})
|
||||
|
||||
it('fails closed before deleting an exact worktree id owned by multiple hosts', async () => {
|
||||
const store = createTestStore()
|
||||
const worktreeId = 'repo-shared::/same/path'
|
||||
store.setState({
|
||||
repos: [
|
||||
{ id: 'repo-shared', path: '/local', displayName: 'Local', badgeColor: '#000', addedAt: 0 },
|
||||
{
|
||||
id: 'repo-shared',
|
||||
path: '/remote',
|
||||
displayName: 'SSH',
|
||||
badgeColor: '#111',
|
||||
addedAt: 1,
|
||||
connectionId: 'ssh-1'
|
||||
}
|
||||
],
|
||||
worktreesByRepo: {
|
||||
'repo-shared': [
|
||||
makeWorktree({ id: worktreeId, repoId: 'repo-shared', hostId: 'local' }),
|
||||
makeWorktree({ id: worktreeId, repoId: 'repo-shared', hostId: 'ssh:ssh-1' })
|
||||
]
|
||||
}
|
||||
} as Partial<AppState>)
|
||||
|
||||
const result = await store.getState().removeWorktree(worktreeId)
|
||||
|
||||
expect(result).toEqual({
|
||||
ok: false,
|
||||
error: 'Workspace identity is ambiguous across hosts. Refresh projects and try again.'
|
||||
})
|
||||
expect(mockApi.worktrees.remove).not.toHaveBeenCalled()
|
||||
expect(runtimeEnvironmentCall).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('persists worktree metadata through the active remote runtime environment', async () => {
|
||||
const store = createTestStore()
|
||||
const wt = makeWorktree({ id: 'repo1::/path/wt1', repoId: 'repo1', path: '/path/wt1' })
|
||||
|
||||
@@ -71,6 +71,8 @@ export type { WorktreeSlice, WorktreeDeleteState } from './worktree-helpers'
|
||||
// Why: old runtime servers only have `worktree.list`; preserve the large-list
|
||||
// UI hydration parity this slice used before `worktree.detectedList` existed.
|
||||
const REMOTE_WORKTREE_LIST_PARITY_LIMIT = 10_000
|
||||
const WORKTREE_REMOVAL_AMBIGUOUS_ERROR =
|
||||
'Workspace identity is ambiguous across hosts. Refresh projects and try again.'
|
||||
const ACTIVE_WORKTREE_TERMINAL_PREP_DELAY_MS = 300
|
||||
const ACTIVE_WORKTREE_TERMINAL_PREP_INPUT_QUIET_MS = 450
|
||||
const ACTIVE_WORKTREE_TERMINAL_PREP_IDLE_TIMEOUT_MS = 180
|
||||
@@ -1158,6 +1160,41 @@ function getWorktreeHostId(
|
||||
return repo ? getRepoExecutionHostId(repo) : null
|
||||
}
|
||||
|
||||
function resolveWorktreeRemovalHost(
|
||||
state: Pick<AppState, 'repos' | 'settings' | 'worktreesByRepo' | 'detectedWorktreesByRepo'>,
|
||||
worktreeId: string
|
||||
): { hostId: ExecutionHostId | null; ambiguous: boolean } {
|
||||
const hostIds = new Set<ExecutionHostId>()
|
||||
for (const worktrees of Object.values(state.worktreesByRepo)) {
|
||||
for (const worktree of worktrees) {
|
||||
if (worktree.id === worktreeId && worktree.hostId) {
|
||||
hostIds.add(worktree.hostId)
|
||||
}
|
||||
}
|
||||
}
|
||||
for (const result of Object.values(state.detectedWorktreesByRepo)) {
|
||||
for (const worktree of result.worktrees) {
|
||||
if (worktree.id === worktreeId && worktree.hostId) {
|
||||
hostIds.add(worktree.hostId)
|
||||
}
|
||||
}
|
||||
}
|
||||
if (hostIds.size > 1) {
|
||||
return { hostId: null, ambiguous: true }
|
||||
}
|
||||
if (hostIds.size === 1) {
|
||||
return { hostId: hostIds.values().next().value ?? null, ambiguous: false }
|
||||
}
|
||||
|
||||
const repoId = getRepoIdFromWorktreeId(worktreeId)
|
||||
const repoHostIds = new Set(
|
||||
state.repos.filter((repo) => repo.id === repoId).map(getRepoExecutionHostId)
|
||||
)
|
||||
return repoHostIds.size > 1
|
||||
? { hostId: null, ambiguous: true }
|
||||
: { hostId: repoHostIds.values().next().value ?? null, ambiguous: false }
|
||||
}
|
||||
|
||||
function mergeLineageForHost(
|
||||
state: Pick<
|
||||
AppState,
|
||||
@@ -3145,6 +3182,11 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
|
||||
},
|
||||
|
||||
removeWorktree: async (worktreeId, force, options) => {
|
||||
const removalOwner = resolveWorktreeRemovalHost(get(), worktreeId)
|
||||
if (removalOwner.ambiguous) {
|
||||
return { ok: false, error: WORKTREE_REMOVAL_AMBIGUOUS_ERROR }
|
||||
}
|
||||
const hostId = removalOwner.hostId ?? undefined
|
||||
const forgetLocalOnly = options?.mode === 'forget-local'
|
||||
set((s) => ({
|
||||
deleteStateByWorktreeId: {
|
||||
@@ -3163,20 +3205,35 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
|
||||
// and no need to prompt for hook trust.
|
||||
const skipArchive = forgetLocalOnly
|
||||
? true
|
||||
: (await ensureHooksConfirmed(get(), getRepoIdFromWorktreeId(worktreeId), 'archive')) ===
|
||||
'skip'
|
||||
: (await ensureHooksConfirmed(
|
||||
get(),
|
||||
getRepoIdFromWorktreeId(worktreeId),
|
||||
'archive',
|
||||
hostId
|
||||
)) === 'skip'
|
||||
|
||||
const worktreeBeforeRemoval = get()
|
||||
.allWorktrees()
|
||||
.find((entry) => entry.id === worktreeId)
|
||||
const currentOwner = resolveWorktreeRemovalHost(get(), worktreeId)
|
||||
if (
|
||||
currentOwner.ambiguous ||
|
||||
(hostId && currentOwner.hostId && currentOwner.hostId !== hostId)
|
||||
) {
|
||||
throw new Error(WORKTREE_REMOVAL_AMBIGUOUS_ERROR)
|
||||
}
|
||||
// Why: forget-local always clears Orca's own records via the local IPC
|
||||
// handler regardless of the workspace's execution host — the whole point
|
||||
// is that the remote (SSH relay / runtime) is gone or unreachable.
|
||||
const target = getActiveRuntimeTarget(settingsForWorktreeOwner(get(), worktreeId))
|
||||
const target = getActiveRuntimeTarget(
|
||||
hostId
|
||||
? settingsForExecutionHostOwner(get().settings, hostId)
|
||||
: settingsForWorktreeOwner(get(), worktreeId)
|
||||
)
|
||||
const removalResult = await (forgetLocalOnly
|
||||
? window.api.worktrees.forgetLocal({ worktreeId })
|
||||
? window.api.worktrees.forgetLocal({ worktreeId, hostId })
|
||||
: target.kind === 'local'
|
||||
? window.api.worktrees.remove({ worktreeId, force, skipArchive })
|
||||
? window.api.worktrees.remove({ worktreeId, hostId, force, skipArchive })
|
||||
: callRuntimeRpc<RemoveWorktreeResult>(
|
||||
target,
|
||||
'worktree.rm',
|
||||
|
||||
Reference in New Issue
Block a user