mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
fix(cli): orca file open and diff open name their owner (#27154)
* fix(cli): orca file open and diff open name their owner The desktop's own catalog resolves the worktree for files.open/files.openDiff, so the renderer notification now says so (null) instead of leaving the owner unset, which let the focused server claim a tab the renderer could not place yet. * fix(cli): keep a paired server's ownership on diff open A catalog row stamped runtime:<env> names that server; null would route its diff through this desktop's git. --------- Co-authored-by: m4air <m4air@m4airs-MacBook-Air.local>
This commit is contained in:
@@ -86,7 +86,7 @@ describe('RuntimeFileCommands', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('opens source control diffs through the renderer host (inheriting active runtime env)', async () => {
|
||||
it('opens source control diffs through the renderer host as owned by this desktop, not the focused server', async () => {
|
||||
const openDiff = vi.fn()
|
||||
const { commands } = createRuntimeFileCommands({ openDiff })
|
||||
|
||||
@@ -97,7 +97,7 @@ describe('RuntimeFileCommands', () => {
|
||||
'/repo/docs/readme.md',
|
||||
'docs/readme.md',
|
||||
true,
|
||||
undefined,
|
||||
null,
|
||||
undefined
|
||||
)
|
||||
expect(result).toEqual({
|
||||
@@ -108,7 +108,7 @@ describe('RuntimeFileCommands', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('opens text files through the renderer host (inheriting active runtime env)', async () => {
|
||||
it('opens text files through the renderer host as owned by this desktop, not the focused server', async () => {
|
||||
const openFile = vi.fn()
|
||||
const { commands } = createRuntimeFileCommands({ openFile })
|
||||
resolveAuthorizedPathMock.mockResolvedValue('/repo/docs/readme.md')
|
||||
@@ -120,7 +120,7 @@ describe('RuntimeFileCommands', () => {
|
||||
'wt-1',
|
||||
'/repo/docs/readme.md',
|
||||
'docs/readme.md',
|
||||
undefined,
|
||||
null,
|
||||
undefined
|
||||
)
|
||||
expect(result).toEqual({
|
||||
@@ -143,7 +143,7 @@ describe('RuntimeFileCommands', () => {
|
||||
'wt-1',
|
||||
'/repo/assets/logo.png',
|
||||
'assets/logo.png',
|
||||
undefined,
|
||||
null,
|
||||
undefined
|
||||
)
|
||||
expect(result).toEqual({
|
||||
@@ -168,7 +168,7 @@ describe('RuntimeFileCommands', () => {
|
||||
'wt-1',
|
||||
'/repo/docs/readme.md',
|
||||
'docs/readme.md',
|
||||
undefined,
|
||||
null,
|
||||
'all'
|
||||
)
|
||||
expect(openDiff).toHaveBeenCalledWith(
|
||||
@@ -176,11 +176,73 @@ describe('RuntimeFileCommands', () => {
|
||||
'/repo/docs/readme.md',
|
||||
'docs/readme.md',
|
||||
false,
|
||||
undefined,
|
||||
null,
|
||||
'host'
|
||||
)
|
||||
})
|
||||
|
||||
it('names this desktop as the owner of SSH file and diff opens', async () => {
|
||||
const openFile = vi.fn()
|
||||
const openDiff = vi.fn()
|
||||
const resolveRuntimeFileTarget = vi.fn(async () => ({
|
||||
worktree: { id: 'wt-1', repoId: 'repo-1', path: '/remote/repo' },
|
||||
executionHostId: 'ssh:ssh-1'
|
||||
}))
|
||||
const { commands } = createRuntimeFileCommands({
|
||||
openFile,
|
||||
openDiff,
|
||||
path: '/remote/repo',
|
||||
resolveRuntimeFileTarget
|
||||
})
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the open path only calls `stat`.
|
||||
vi.mocked(getSshFilesystemProvider).mockReturnValue({
|
||||
stat: vi.fn().mockResolvedValue({ type: 'file', size: 1, mtime: 0 })
|
||||
} as never)
|
||||
|
||||
await commands.openMobileFile('id:wt-1', 'docs/readme.md')
|
||||
await commands.openMobileDiff('id:wt-1', 'docs/readme.md', false)
|
||||
|
||||
expect(openFile).toHaveBeenCalledWith(
|
||||
'wt-1',
|
||||
'/remote/repo/docs/readme.md',
|
||||
'docs/readme.md',
|
||||
null,
|
||||
undefined
|
||||
)
|
||||
expect(openDiff).toHaveBeenCalledWith(
|
||||
'wt-1',
|
||||
'/remote/repo/docs/readme.md',
|
||||
'docs/readme.md',
|
||||
false,
|
||||
null,
|
||||
undefined
|
||||
)
|
||||
})
|
||||
|
||||
it('names a paired server as the owner of a diff its catalog row is stamped with', async () => {
|
||||
const openDiff = vi.fn()
|
||||
const resolveRuntimeFileTarget = vi.fn(async () => ({
|
||||
worktree: { id: 'wt-1', repoId: 'repo-1', path: '/remote/repo' },
|
||||
executionHostId: 'runtime:env-a'
|
||||
}))
|
||||
const { commands } = createRuntimeFileCommands({
|
||||
openDiff,
|
||||
path: '/remote/repo',
|
||||
resolveRuntimeFileTarget
|
||||
})
|
||||
|
||||
await commands.openMobileDiff('id:wt-1', 'docs/readme.md', true)
|
||||
|
||||
expect(openDiff).toHaveBeenCalledWith(
|
||||
'wt-1',
|
||||
'/remote/repo/docs/readme.md',
|
||||
'docs/readme.md',
|
||||
true,
|
||||
'env-a',
|
||||
undefined
|
||||
)
|
||||
})
|
||||
|
||||
it.each(['docs/example.pdf', 'dist/bundle.zip'])(
|
||||
'opens binary %s in the desktop editor like the File Explorer does',
|
||||
async (relativePath) => {
|
||||
@@ -195,7 +257,7 @@ describe('RuntimeFileCommands', () => {
|
||||
'wt-1',
|
||||
`/repo/${relativePath}`,
|
||||
relativePath,
|
||||
undefined,
|
||||
null,
|
||||
undefined
|
||||
)
|
||||
expect(result).toEqual({ worktree: 'wt-1', relativePath, kind: 'binary', opened: true })
|
||||
|
||||
@@ -30,7 +30,7 @@ import {
|
||||
requireRuntimeFileProvider,
|
||||
runtimeFileRouteForTarget
|
||||
} from './runtime-file-command-target'
|
||||
import type { ExecutionHostId } from '../../shared/execution-host'
|
||||
import { parseExecutionHostId, type ExecutionHostId } from '../../shared/execution-host'
|
||||
import type { FileStat } from '../providers/types'
|
||||
|
||||
export class RuntimeFileCommandsWithConstructor extends RuntimeFileCommandsWithActiveRuntimeTextSearches {
|
||||
@@ -210,8 +210,13 @@ export class RuntimeFileCommandsWithConstructor extends RuntimeFileCommandsWithA
|
||||
const filePath = joinWorktreeRelativePath(worktree.path, relativePath)
|
||||
// Why: CLI/agents treat opened:true as success; stat first so missing paths and directories fail the RPC instead of opening a ghost tab.
|
||||
await this.assertOpenTargetIsFile(filePath, target)
|
||||
// Why: the internal runtimeId isn't a valid env selector; pass undefined so openFile falls back to activeRuntimeEnvironmentId.
|
||||
this.host.openFile(worktree.id, filePath, relativePath, undefined, navigation)
|
||||
this.host.openFile(
|
||||
worktree.id,
|
||||
filePath,
|
||||
relativePath,
|
||||
runtimeOwnerForTarget(target),
|
||||
navigation
|
||||
)
|
||||
return { worktree: worktree.id, relativePath, kind, opened: true }
|
||||
}
|
||||
|
||||
@@ -243,7 +248,8 @@ export class RuntimeFileCommandsWithConstructor extends RuntimeFileCommandsWithA
|
||||
staged: boolean,
|
||||
navigation?: RuntimeNavigationTarget
|
||||
): Promise<RuntimeFileOpenResult> {
|
||||
const { worktree } = await this.host.resolveRuntimeFileTarget(worktreeSelector)
|
||||
const target = await this.host.resolveRuntimeFileTarget(worktreeSelector)
|
||||
const { worktree } = target
|
||||
if (!isSafeMobileRelativePath(relativePath)) {
|
||||
throw new Error('invalid_relative_path')
|
||||
}
|
||||
@@ -253,8 +259,20 @@ export class RuntimeFileCommandsWithConstructor extends RuntimeFileCommandsWithA
|
||||
? 'markdown'
|
||||
: 'text'
|
||||
const filePath = joinWorktreeRelativePath(worktree.path, relativePath)
|
||||
// Why: see openMobileFile; avoid stamping internal runtimeId as runtimeEnvironmentId.
|
||||
this.host.openDiff(worktree.id, filePath, relativePath, staged, undefined, navigation)
|
||||
this.host.openDiff(
|
||||
worktree.id,
|
||||
filePath,
|
||||
relativePath,
|
||||
staged,
|
||||
runtimeOwnerForTarget(target),
|
||||
navigation
|
||||
)
|
||||
return { worktree: worktree.id, relativePath, kind, opened: true }
|
||||
}
|
||||
}
|
||||
|
||||
// Why: name the resolved owner; undefined would let the renderer fall back to the focused server.
|
||||
function runtimeOwnerForTarget(target: { executionHostId: ExecutionHostId }): string | null {
|
||||
const host = parseExecutionHostId(target.executionHostId)
|
||||
return host?.kind === 'runtime' ? host.environmentId : null
|
||||
}
|
||||
|
||||
@@ -23,7 +23,7 @@ export const uiClipboardAndWindowControlsApi = {
|
||||
filePath: string
|
||||
relativePath: string
|
||||
staged: boolean
|
||||
runtimeEnvironmentId?: string
|
||||
runtimeEnvironmentId?: string | null
|
||||
navigation?: RuntimeNavigationTarget
|
||||
}) => void
|
||||
): (() => void) => {
|
||||
@@ -34,7 +34,7 @@ export const uiClipboardAndWindowControlsApi = {
|
||||
filePath: string
|
||||
relativePath: string
|
||||
staged: boolean
|
||||
runtimeEnvironmentId?: string
|
||||
runtimeEnvironmentId?: string | null
|
||||
navigation?: RuntimeNavigationTarget
|
||||
}
|
||||
) => callback(data)
|
||||
|
||||
@@ -227,7 +227,7 @@ export const uiTerminalAndSessionTabsApi = {
|
||||
worktreeId: string
|
||||
filePath: string
|
||||
relativePath: string
|
||||
runtimeEnvironmentId?: string
|
||||
runtimeEnvironmentId?: string | null
|
||||
navigation?: RuntimeNavigationTarget
|
||||
}) => void
|
||||
): (() => void) => {
|
||||
@@ -237,7 +237,7 @@ export const uiTerminalAndSessionTabsApi = {
|
||||
worktreeId: string
|
||||
filePath: string
|
||||
relativePath: string
|
||||
runtimeEnvironmentId?: string
|
||||
runtimeEnvironmentId?: string | null
|
||||
navigation?: RuntimeNavigationTarget
|
||||
}
|
||||
) => callback(data)
|
||||
|
||||
@@ -232,7 +232,7 @@ export type UiCommandEventApi = {
|
||||
worktreeId: string
|
||||
filePath: string
|
||||
relativePath: string
|
||||
runtimeEnvironmentId?: string
|
||||
runtimeEnvironmentId?: string | null
|
||||
navigation?: RuntimeNavigationTarget
|
||||
}) => void
|
||||
) => () => void
|
||||
@@ -242,7 +242,7 @@ export type UiCommandEventApi = {
|
||||
filePath: string
|
||||
relativePath: string
|
||||
staged: boolean
|
||||
runtimeEnvironmentId?: string
|
||||
runtimeEnvironmentId?: string | null
|
||||
navigation?: RuntimeNavigationTarget
|
||||
}) => void
|
||||
) => () => void
|
||||
|
||||
@@ -41,7 +41,7 @@ type OpenFilePayload = {
|
||||
worktreeId: string
|
||||
filePath: string
|
||||
relativePath: string
|
||||
runtimeEnvironmentId?: string
|
||||
runtimeEnvironmentId?: string | null
|
||||
navigation?: RuntimeNavigationTarget
|
||||
}
|
||||
type OpenDiffPayload = OpenFilePayload & { staged: boolean }
|
||||
@@ -401,4 +401,55 @@ describe('runtime file opens on the host desktop', () => {
|
||||
expect(state.openFiles.find((file) => file.id === state.activeFileId)?.mode).toBe('diff')
|
||||
expect(state.pendingRevealWorktree?.worktreeId).toBe(BACKGROUND)
|
||||
})
|
||||
|
||||
describe('with another server focused', () => {
|
||||
// A worktree the CLI just created, before this renderer's catalog lists it.
|
||||
const FRESH = 'repo9::/repo9/fresh'
|
||||
const FRESH_TS = { filePath: '/repo1/fresh/src/app.ts', relativePath: 'src/app.ts' }
|
||||
|
||||
function focusServer(store: TestStore): void {
|
||||
store.setState({
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: owner routing reads only these fields.
|
||||
settings: { activeRuntimeEnvironmentId: 'srv-focused' } as never,
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: owner routing reads only the saved id.
|
||||
runtimeEnvironments: [{ id: 'srv-focused' } as never]
|
||||
})
|
||||
}
|
||||
|
||||
it('keeps a desktop-owned file open on this desktop', () => {
|
||||
const { openFile, store } = setup('editor')
|
||||
focusServer(store)
|
||||
|
||||
openFile({ worktreeId: FRESH, ...FRESH_TS, runtimeEnvironmentId: null, navigation: 'caller' })
|
||||
|
||||
const opened = store.getState().openFiles.find((file) => file.worktreeId === FRESH)
|
||||
expect(opened?.runtimeEnvironmentId).toBeNull()
|
||||
})
|
||||
|
||||
it('keeps a desktop-owned diff open on this desktop', () => {
|
||||
const { openDiff, store } = setup('editor')
|
||||
focusServer(store)
|
||||
|
||||
openDiff({
|
||||
worktreeId: FRESH,
|
||||
...FRESH_TS,
|
||||
staged: false,
|
||||
runtimeEnvironmentId: null,
|
||||
navigation: 'caller'
|
||||
})
|
||||
|
||||
const opened = store.getState().openFiles.find((file) => file.worktreeId === FRESH)
|
||||
expect(opened?.runtimeEnvironmentId).toBeNull()
|
||||
})
|
||||
|
||||
it('an unnamed owner would hand the file to the focused server', () => {
|
||||
const { openFile, store } = setup('editor')
|
||||
focusServer(store)
|
||||
|
||||
openFile({ worktreeId: FRESH, ...FRESH_TS, navigation: 'caller' })
|
||||
|
||||
const opened = store.getState().openFiles.find((file) => file.worktreeId === FRESH)
|
||||
expect(opened?.runtimeEnvironmentId).toBe('srv-focused')
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user