mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 00:03:45 +00:00
fix(worktrees): keep selector grammar intact when scoping by connection
resolveRepoSelectorForConnection matched the selector as a bare repo id, so an explicit connection identity silently changed the grammar: `path:` and `name:` selectors resolved to repo_not_found on that path alone, losing the whole sweep. A connection identity should only *narrow* the candidate set. Extract the selector matching both paths now share, and stop re-resolving an already-resolved repo: teardown rescanned via `id:<repo.id>`, which throws selector_ambiguous when an id is duplicated across hosts even though the caller's own selector was unambiguous. Reported as a P2 by Greptile (as redundant work); it is load-bearing. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -31389,7 +31389,10 @@ describe('OrcaRuntimeService', () => {
|
||||
const runtime = new OrcaRuntimeService(store, undefined, {
|
||||
getLocalProvider: () => localProvider as never
|
||||
})
|
||||
vi.spyOn(runtime, 'listDetectedManagedWorktrees').mockResolvedValue({
|
||||
vi.spyOn(
|
||||
runtime as unknown as { listDetectedWorktreesForResolvedRepo: () => unknown },
|
||||
'listDetectedWorktreesForResolvedRepo'
|
||||
).mockResolvedValue({
|
||||
repoId: TEST_REPO_ID,
|
||||
authoritative: true,
|
||||
source: 'git',
|
||||
@@ -31455,7 +31458,10 @@ describe('OrcaRuntimeService', () => {
|
||||
const runtime = new OrcaRuntimeService(store, undefined, {
|
||||
getLocalProvider: () => localProvider as never
|
||||
})
|
||||
vi.spyOn(runtime, 'listDetectedManagedWorktrees').mockResolvedValue({
|
||||
vi.spyOn(
|
||||
runtime as unknown as { listDetectedWorktreesForResolvedRepo: () => unknown },
|
||||
'listDetectedWorktreesForResolvedRepo'
|
||||
).mockResolvedValue({
|
||||
repoId: TEST_REPO_ID,
|
||||
authoritative: false,
|
||||
source: 'metadata-fallback',
|
||||
@@ -31469,6 +31475,47 @@ describe('OrcaRuntimeService', () => {
|
||||
expect(localProvider.listProcesses).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
// Why: an explicit connection identity only narrows the selector, it must not
|
||||
// change the grammar. Treating the selector as a bare repo id made every
|
||||
// `path:`/`name:` selector fail repo_not_found on this path alone.
|
||||
it('resolves non-id selectors when a connection identity is supplied', async () => {
|
||||
const deletedId = `${TEST_REPO_ID}::/tmp/deleted`
|
||||
const localProvider = {
|
||||
listProcesses: vi.fn(async () => [
|
||||
{ id: `${deletedId}@@deleted-session`, cwd: '/tmp/deleted', title: 'shell' }
|
||||
]),
|
||||
shutdown: vi.fn(async () => {})
|
||||
}
|
||||
const localRepo = store.getRepos()[0]
|
||||
const runtime = new OrcaRuntimeService(
|
||||
{ ...store, getRepos: () => [localRepo, { ...localRepo, connectionId: 'ssh-1' }] } as never,
|
||||
undefined,
|
||||
{ getLocalProvider: () => localProvider as never }
|
||||
)
|
||||
vi.spyOn(
|
||||
runtime as unknown as { listDetectedWorktreesForResolvedRepo: () => unknown },
|
||||
'listDetectedWorktreesForResolvedRepo'
|
||||
).mockResolvedValue({
|
||||
repoId: TEST_REPO_ID,
|
||||
authoritative: true,
|
||||
source: 'git',
|
||||
worktrees: []
|
||||
})
|
||||
|
||||
// `path:` is ambiguous across the two rows; connectionId null selects the local one.
|
||||
const result = await runtime.teardownMissingManagedWorktreeTerminals(
|
||||
`path:${localRepo.path}`,
|
||||
[deletedId],
|
||||
null
|
||||
)
|
||||
|
||||
expect(result).toEqual({ stoppedWorktreeIds: [deletedId] })
|
||||
expect(localProvider.shutdown).toHaveBeenCalledWith(
|
||||
`${deletedId}@@deleted-session`,
|
||||
expect.objectContaining({ immediate: true })
|
||||
)
|
||||
})
|
||||
|
||||
it('uses the connection-scoped repo when local and SSH repo ids collide', async () => {
|
||||
const deletedId = `${TEST_REPO_ID}::/tmp/deleted`
|
||||
const localProvider = {
|
||||
@@ -31495,7 +31542,10 @@ describe('OrcaRuntimeService', () => {
|
||||
connectionId === 'ssh-1' ? (sshProvider as never) : undefined
|
||||
}
|
||||
)
|
||||
vi.spyOn(runtime, 'listDetectedManagedWorktrees').mockResolvedValue({
|
||||
vi.spyOn(
|
||||
runtime as unknown as { listDetectedWorktreesForResolvedRepo: () => unknown },
|
||||
'listDetectedWorktreesForResolvedRepo'
|
||||
).mockResolvedValue({
|
||||
repoId: TEST_REPO_ID,
|
||||
authoritative: true,
|
||||
source: 'git',
|
||||
|
||||
@@ -17950,7 +17950,14 @@ export class OrcaRuntimeService {
|
||||
repoSelector: string,
|
||||
connectionId?: string | null
|
||||
): Promise<DetectedWorktreeListResult> {
|
||||
const repo = await this.resolveRepoSelectorForConnection(repoSelector, connectionId)
|
||||
return this.listDetectedWorktreesForResolvedRepo(
|
||||
await this.resolveRepoSelectorForConnection(repoSelector, connectionId)
|
||||
)
|
||||
}
|
||||
|
||||
private async listDetectedWorktreesForResolvedRepo(
|
||||
repo: Repo
|
||||
): Promise<DetectedWorktreeListResult> {
|
||||
const store = this.requireStore()
|
||||
if (isFolderRepo(repo)) {
|
||||
const worktrees = listRuntimeFolderWorkspaces(store, repo)
|
||||
@@ -18010,7 +18017,10 @@ export class OrcaRuntimeService {
|
||||
// (30s TTL) can still list a directory git already dropped, and the renderer
|
||||
// purges its state either way, so a stale miss strands those processes for good.
|
||||
this.invalidateWorktreeScanCacheForRepo(repo.id)
|
||||
const detected = await this.listDetectedManagedWorktrees(`id:${repo.id}`, connectionId)
|
||||
// Why: rescanning by `id:` would re-resolve the already-resolved repo, and a
|
||||
// duplicate id across hosts makes that second lookup throw selector_ambiguous
|
||||
// even though the caller's selector was unique — losing the sweep entirely.
|
||||
const detected = await this.listDetectedWorktreesForResolvedRepo(repo)
|
||||
if (!detected.authoritative) {
|
||||
return { stoppedWorktreeIds: [] }
|
||||
}
|
||||
@@ -18034,14 +18044,13 @@ export class OrcaRuntimeService {
|
||||
if (connectionId === undefined) {
|
||||
return this.resolveRepoSelector(repoSelector)
|
||||
}
|
||||
const repoId = repoSelector.startsWith('id:') ? repoSelector.slice(3) : repoSelector
|
||||
const matches = this.requireStore()
|
||||
.getRepos()
|
||||
.filter(
|
||||
(repo) =>
|
||||
repo.id === repoId &&
|
||||
(repo.connectionId?.trim() || null) === (connectionId?.trim() || null)
|
||||
)
|
||||
// Why: an explicit connection identity only *narrows* the selector; it must not
|
||||
// change the grammar. Matching the selector as a bare repo id would make
|
||||
// `path:`/`name:` selectors resolve to repo_not_found on this path alone.
|
||||
const wanted = connectionId?.trim() || null
|
||||
const matches = this.selectReposBySelector(repoSelector).filter(
|
||||
(repo) => (repo.connectionId?.trim() || null) === wanted
|
||||
)
|
||||
if (matches.length !== 1) {
|
||||
throw new Error(matches.length > 1 ? 'selector_ambiguous' : 'repo_not_found')
|
||||
}
|
||||
@@ -25223,27 +25232,32 @@ export class OrcaRuntimeService {
|
||||
return this.store?.getAllWorkspaceLineage?.() ?? {}
|
||||
}
|
||||
|
||||
// Why: one selector grammar, so connection-scoped resolution can narrow the same
|
||||
// candidate set instead of reimplementing (and diverging from) the matching rules.
|
||||
private selectReposBySelector(selector: string): Repo[] {
|
||||
const repos = this.store?.getRepos() ?? []
|
||||
if (selector.startsWith('id:')) {
|
||||
return repos.filter((repo) => repo.id === selector.slice(3))
|
||||
}
|
||||
if (selector.startsWith('path:')) {
|
||||
return repos.filter((repo) => runtimePathsEqual(repo.path, selector.slice(5)))
|
||||
}
|
||||
if (selector.startsWith('name:')) {
|
||||
return repos.filter((repo) => repo.displayName === selector.slice(5))
|
||||
}
|
||||
return repos.filter(
|
||||
(repo) =>
|
||||
repo.id === selector ||
|
||||
runtimePathsEqual(repo.path, selector) ||
|
||||
repo.displayName === selector
|
||||
)
|
||||
}
|
||||
|
||||
private async resolveRepoSelector(selector: string): Promise<Repo> {
|
||||
if (!this.store) {
|
||||
throw new Error('repo_not_found')
|
||||
}
|
||||
const repos = this.store.getRepos()
|
||||
let candidates: Repo[]
|
||||
|
||||
if (selector.startsWith('id:')) {
|
||||
candidates = repos.filter((repo) => repo.id === selector.slice(3))
|
||||
} else if (selector.startsWith('path:')) {
|
||||
candidates = repos.filter((repo) => runtimePathsEqual(repo.path, selector.slice(5)))
|
||||
} else if (selector.startsWith('name:')) {
|
||||
candidates = repos.filter((repo) => repo.displayName === selector.slice(5))
|
||||
} else {
|
||||
candidates = repos.filter(
|
||||
(repo) =>
|
||||
repo.id === selector ||
|
||||
runtimePathsEqual(repo.path, selector) ||
|
||||
repo.displayName === selector
|
||||
)
|
||||
}
|
||||
const candidates = this.selectReposBySelector(selector)
|
||||
|
||||
if (candidates.length === 1) {
|
||||
return candidates[0]
|
||||
|
||||
Reference in New Issue
Block a user