fix(cli): relativize absolute --path for file open and file diff before the runtime RPC (#9429) (#9824)

This commit is contained in:
Rod Boev 2026-07-24 02:43:15 -04:00 committed by GitHub
parent 877bbdebf8
commit 108a2ad41b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 192 additions and 5 deletions

View File

@ -0,0 +1,161 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const callMock = vi.fn()
vi.mock('../runtime-client', () => {
class RuntimeClient {
readonly isRemote = false
call = callMock
getCliStatus = vi.fn()
openOrca = vi.fn()
}
class RuntimeClientError extends Error {
readonly code: string
constructor(code: string, message: string) {
super(message)
this.code = code
}
}
class RuntimeRpcFailureError extends RuntimeClientError {
readonly response: unknown
constructor(response: unknown) {
super('runtime_error', 'runtime_error')
this.response = response
}
}
return { RuntimeClient, RuntimeClientError, RuntimeRpcFailureError }
})
import { main } from '../index'
import { buildWorktree, okFixture, queueFixtures, worktreeListFixture } from '../test-fixtures'
describe('absolute file CLI paths', () => {
beforeEach(() => {
vi.restoreAllMocks()
callMock.mockReset()
process.exitCode = undefined
vi.spyOn(console, 'log').mockImplementation(() => {})
vi.spyOn(console, 'error').mockImplementation(() => {})
})
it('reproduces the issue positional WSL command without invalid_relative_path', async () => {
const issuePath = '/root/orca/workspaces/xxx/xxx/xxx.ts'
callMock.mockImplementation(async (method: string, params: { relativePath?: string }) => {
if (method === 'worktree.list') {
return worktreeListFixture([buildWorktree('/root/orca/workspaces/xxx', 'feature')])
}
if (method === 'worktree.show') {
return okFixture('req_show', {
worktree: buildWorktree('/root/orca/workspaces/xxx', 'feature')
})
}
if (method === 'files.open' && params.relativePath?.startsWith('/')) {
throw new Error('invalid_relative_path')
}
return okFixture('req_open', {
worktree: 'wt-1',
relativePath: params.relativePath,
kind: 'text',
opened: true
})
})
await main(['file', 'open', issuePath], '/root/orca/workspaces/xxx')
expect(process.exitCode).toBeUndefined()
expect(callMock).toHaveBeenNthCalledWith(1, 'worktree.list', { limit: 10_000 })
expect(callMock).toHaveBeenNthCalledWith(2, 'worktree.show', {
worktree: 'id:repo::/root/orca/workspaces/xxx'
})
expect(callMock).toHaveBeenNthCalledWith(3, 'files.open', {
worktree: 'id:repo::/root/orca/workspaces/xxx',
relativePath: 'xxx/xxx.ts'
})
})
it('relativizes absolute file diff paths', async () => {
queueFixtures(
callMock,
okFixture('req_show', { worktree: buildWorktree('/tmp/repo', 'feature') }),
okFixture('req_diff', {
worktree: 'wt-1',
relativePath: 'src/App.tsx',
kind: 'text',
opened: true
})
)
await main(
['file', 'diff', '--path', '/tmp/repo/src/App.tsx', '--worktree', 'id:wt-1', '--staged'],
'/tmp'
)
expect(callMock).toHaveBeenNthCalledWith(1, 'worktree.show', { worktree: 'id:wt-1' })
expect(callMock).toHaveBeenNthCalledWith(2, 'files.openDiff', {
worktree: 'id:wt-1',
relativePath: 'src/App.tsx',
staged: true
})
})
it('keeps relative paths on the single-rpc path', async () => {
queueFixtures(
callMock,
okFixture('req_open', {
worktree: 'wt-1',
relativePath: 'src/App.tsx',
kind: 'text',
opened: true
})
)
await main(['file', 'open', '--path', 'src/App.tsx', '--worktree', 'id:wt-1'], '/tmp')
expect(callMock).toHaveBeenCalledTimes(1)
expect(callMock).toHaveBeenCalledWith('files.open', {
worktree: 'id:wt-1',
relativePath: 'src/App.tsx'
})
})
it('leaves outside-worktree absolute paths for the runtime guard', async () => {
const absolutePath = '/tmp/elsewhere/App.tsx'
queueFixtures(
callMock,
okFixture('req_show', { worktree: buildWorktree('/tmp/repo', 'feature') }),
okFixture('req_open', {
worktree: 'wt-1',
relativePath: absolutePath,
kind: 'text',
opened: true
})
)
await main(['file', 'open', '--path', absolutePath, '--worktree', 'id:wt-1'], '/tmp')
expect(callMock).toHaveBeenNthCalledWith(2, 'files.open', {
worktree: 'id:wt-1',
relativePath: absolutePath
})
})
it('rejects the worktree root as a file-open target', async () => {
queueFixtures(
callMock,
okFixture('req_show', { worktree: buildWorktree('/tmp/repo', 'feature') })
)
await main(['file', 'open', '--path', '/tmp/repo', '--worktree', 'id:wt-1'], '/tmp')
expect(process.exitCode).toBe(1)
expect(console.error).toHaveBeenCalledWith(
'The selected worktree root is a directory, not a file-open target.'
)
expect(callMock).toHaveBeenCalledTimes(1)
})
})

View File

@ -1,5 +1,6 @@
import type { GitStatusEntry, GitStatusResult } from '../../shared/git-status-types'
import type { RuntimeFileOpenResult } from '../../shared/runtime-types'
import type { RuntimeFileOpenResult, RuntimeWorktreeRecord } from '../../shared/runtime-types'
import { isRuntimePathAbsolute, relativePathInsideRoot } from '../../shared/cross-platform-path'
import type { CommandHandler, HandlerContext } from '../dispatch'
import { getOptionalStringFlag, getRequiredStringFlag } from '../flags'
import { printResult } from '../format'
@ -45,6 +46,28 @@ async function getFileWorktreeSelector({ flags, cwd, client }: HandlerContext):
return await resolveCurrentWorktreeSelector(cwd, client)
}
async function resolveFilePath(
ctx: HandlerContext,
worktree: string,
path: string
): Promise<string> {
if (!isRuntimePathAbsolute(path)) {
return path
}
// Why: only in-worktree absolute paths should be relativized here; outside paths must reach the runtime guard unchanged.
const result = await ctx.client.call<{ worktree: RuntimeWorktreeRecord }>('worktree.show', {
worktree
})
const relativePath = relativePathInsideRoot(result.result.worktree.path, path)
if (relativePath === '') {
throw new RuntimeClientError(
'invalid_argument',
'The selected worktree root is a directory, not a file-open target.'
)
}
return relativePath ?? path
}
function getOpenChangedMode(flags: Map<string, string | boolean>): OpenChangedMode {
const value = flags.get('mode')
if (flags.has('mode') && (typeof value !== 'string' || value.length === 0)) {
@ -137,8 +160,9 @@ function formatFileDiff(result: RuntimeFileOpenResult): string {
export const FILE_HANDLERS: Record<string, CommandHandler> = {
'file open': async (ctx) => {
const relativePath = getRequiredStringFlag(ctx.flags, 'path')
const path = getRequiredStringFlag(ctx.flags, 'path')
const worktree = await getFileWorktreeSelector(ctx)
const relativePath = await resolveFilePath(ctx, worktree, path)
const result = await ctx.client.call<RuntimeFileOpenResult>('files.open', {
worktree,
relativePath
@ -146,9 +170,10 @@ export const FILE_HANDLERS: Record<string, CommandHandler> = {
printResult(result, ctx.json, formatFileOpen)
},
'file diff': async (ctx) => {
const relativePath = getRequiredStringFlag(ctx.flags, 'path')
const path = getRequiredStringFlag(ctx.flags, 'path')
const staged = ctx.flags.get('staged') === true
const worktree = await getFileWorktreeSelector(ctx)
const relativePath = await resolveFilePath(ctx, worktree, path)
const result = await ctx.client.call<RuntimeFileOpenResult>('files.openDiff', {
worktree,
relativePath,

View File

@ -9,7 +9,7 @@ export const FILE_COMMAND_SPECS: CommandSpec[] = [
allowedFlags: [...GLOBAL_FLAGS, 'path', 'worktree'],
positionalArgs: ['path'],
notes: [
'The path is relative to the selected worktree. When --worktree is omitted, local CLI calls infer the current Orca worktree from cwd.'
'The path may be relative to the selected worktree or an absolute path inside that worktree. When --worktree is omitted, local CLI calls infer the current Orca worktree from cwd.'
],
examples: [
'orca file open src/App.tsx',
@ -23,7 +23,8 @@ export const FILE_COMMAND_SPECS: CommandSpec[] = [
allowedFlags: [...GLOBAL_FLAGS, 'path', 'staged', 'worktree'],
positionalArgs: ['path'],
notes: [
'Diffs default to unstaged changes. Pass --staged to open the staged source-control diff.'
'Diffs default to unstaged changes. Pass --staged to open the staged source-control diff.',
'The path may be relative to the selected worktree or an absolute path inside that worktree.'
],
examples: [
'orca file diff src/App.tsx',