fix(cli): --environment connects to the server a differently-cased name resolved to (#30834)

This commit is contained in:
Neil
2026-10-19 12:18:31 -07:00
committed by GitHub
parent dd85af1434
commit e04c2f9e62
4 changed files with 72 additions and 26 deletions
+43 -13
View File
@@ -20,7 +20,7 @@ vi.mock('./runtime-client', () => ({
}))
import {
assertEnvironmentSelectorResolvable,
resolveEnvironmentSelectorForStore,
hostFilterMatchesHostId,
parseHostFlag,
resolveHostFlagEnvironmentId,
@@ -181,6 +181,23 @@ describe('resolveHostFlagEnvironmentId', () => {
})
})
it('accepts ORCA_ENVIRONMENT naming the runtime host in another case', async () => {
listEnvironmentsMock.mockReturnValue([environment('env-1', 'Build')])
resolveEnvironmentMock.mockImplementation(() => {
throw new Error('Unknown environment: build')
})
await expect(
resolveHostFlagEnvironmentId(flags({ host: 'runtime:env-1' }), {
listSshTargets: listSshTargetsMock,
verifySshTargets: listSshTargetsMock,
pairingCode: null,
environmentSelector: { value: 'build', label: 'ORCA_ENVIRONMENT' }
})
).resolves.toBe('env-1')
resolveEnvironmentMock.mockReset()
})
it('accepts --environment naming the same server as the runtime host', async () => {
listEnvironmentsMock.mockReturnValue([environment('env-1', 'gpu')])
resolveEnvironmentMock.mockReturnValue(environment('env-1', 'gpu'))
@@ -434,7 +451,7 @@ describe('ambiguous environment names', () => {
environment('env-2', 'awin')
])
await expect(assertEnvironmentSelectorResolvable('awin', listSshTargetsMock)).rejects.toThrow(
await expect(resolveEnvironmentSelectorForStore('awin', listSshTargetsMock)).rejects.toThrow(
'2 paired servers are named awin'
)
})
@@ -451,20 +468,33 @@ describe('ambiguous environment names', () => {
})
})
describe('assertEnvironmentSelectorResolvable', () => {
describe('resolveEnvironmentSelectorForStore', () => {
it('accepts a paired server by name or id', async () => {
listEnvironmentsMock.mockReturnValue([environment('env-1', 'awin')])
listSshTargetsMock.mockClear()
await expect(
assertEnvironmentSelectorResolvable('awin', listSshTargetsMock)
).resolves.toBeUndefined()
await expect(
assertEnvironmentSelectorResolvable('env-1', listSshTargetsMock)
).resolves.toBeUndefined()
await expect(resolveEnvironmentSelectorForStore('awin', listSshTargetsMock)).resolves.toBe(
'awin'
)
await expect(resolveEnvironmentSelectorForStore('env-1', listSshTargetsMock)).resolves.toBe(
'env-1'
)
expect(listSshTargetsMock).not.toHaveBeenCalled()
})
// Why: the store matches names exactly, so a loosely matched name has to leave here as the id;
// an exact one stays as typed, which `orca status` shows as the target.
it('hands back the id for a name typed in another case or quoted', async () => {
listEnvironmentsMock.mockReturnValue([environment('env-1', 'Build')])
await expect(resolveEnvironmentSelectorForStore('build', listSshTargetsMock)).resolves.toBe(
'env-1'
)
await expect(resolveEnvironmentSelectorForStore('"BUILD"', listSshTargetsMock)).resolves.toBe(
'env-1'
)
})
// Why: the inverse of the --host case, and the one the report actually hit — a name that is
// an SSH target dead-ended with a bare "unknown environment".
it('names the SSH target when the selector is one', async () => {
@@ -472,7 +502,7 @@ describe('assertEnvironmentSelectorResolvable', () => {
listSshTargetsMock.mockResolvedValue([{ id: 'ssh-1-a', label: 'openclaw' }])
await expect(
assertEnvironmentSelectorResolvable('openclaw', listSshTargetsMock)
resolveEnvironmentSelectorForStore('openclaw', listSshTargetsMock)
).rejects.toMatchObject({
code: 'invalid_argument',
data: {
@@ -485,8 +515,8 @@ describe('assertEnvironmentSelectorResolvable', () => {
listEnvironmentsMock.mockReturnValue([])
listSshTargetsMock.mockResolvedValue([])
await expect(
assertEnvironmentSelectorResolvable('nowhere', listSshTargetsMock)
).rejects.toThrow('no paired Orca server is named or has id nowhere')
await expect(resolveEnvironmentSelectorForStore('nowhere', listSshTargetsMock)).rejects.toThrow(
'no paired Orca server is named or has id nowhere'
)
})
})
+10 -6
View File
@@ -150,7 +150,9 @@ export async function resolveHostFlagEnvironmentId(
)
}
if (selection.environmentSelector) {
const selected = resolveEnvironment(userDataPath, selection.environmentSelector.value)
const { value } = selection.environmentSelector
const selected =
findEnvironmentByName(environments, value) ?? resolveEnvironment(userDataPath, value)
if (selected.id !== environment.id) {
throw new RuntimeClientError(
'invalid_argument',
@@ -314,11 +316,12 @@ function assertEnvironmentNameUnambiguous(
// Why: the inverse of the --host case. `--environment openclaw` failed with a bare "Unknown
// environment", when openclaw is very often an SSH target — a different axis, not a typo. The
// store's own error cannot carry the hint (translateStoreError forwards code and message only,
// dropping data), so resolve the selector here where the payload survives.
export async function assertEnvironmentSelectorResolvable(
// dropping data), so resolve the selector here where the payload survives. Returns what the store
// resolves to the same server: the store matches names exactly, so a loose match leaves as the id.
export async function resolveEnvironmentSelectorForStore(
selector: string,
listSshTargetsForSuggestion: () => Promise<SshTargetSummary[]>
): Promise<void> {
): Promise<string> {
const [{ listEnvironments }, { getDefaultUserDataPath }] = await Promise.all([
import('./runtime/environments.js'),
import('./runtime-client.js')
@@ -327,8 +330,9 @@ export async function assertEnvironmentSelectorResolvable(
id: candidate.id,
name: candidate.name
}))
if (findEnvironmentByName(environments, selector)) {
return
const found = findEnvironmentByName(environments, selector)
if (found) {
return found.id === selector || found.name === selector ? selector : found.id
}
assertEnvironmentNameUnambiguous(environments, selector, `--environment ${selector}`)
const sshTargets = await listSshTargetsForSuggestion()
@@ -156,6 +156,19 @@ describe('runtime-selector flags on locally pinned CLI commands', () => {
expect(runtimeClientConstructorMock).toHaveBeenCalledWith(undefined, 'm4air')
})
// Why: the pre-check matches names loosely but the connection lookup is exact, so `build` for a
// server saved as `Build` passed the check and then failed with "Unknown environment: build".
it('connects to the server an --environment name in another case resolved to', async () => {
pairRuntimeEnvironment(listEnvironmentsMock, 'env-m4air', 'M4Air')
fakeMachineNameRuntime('M4 Air')
vi.spyOn(console, 'log').mockImplementation(() => {})
runtimeClientConstructorMock.mockClear()
await main(['host', 'name', '--environment', 'm4air', '--json'], '/tmp/repo')
expect(runtimeClientConstructorMock).toHaveBeenCalledWith(undefined, 'env-m4air')
})
/** The selectors an agent's environment carries: set empty, which the CLI reads as unset. */
function agentSelectors(): void {
process.env.ORCA_ENVIRONMENT = ''
+6 -7
View File
@@ -11,7 +11,7 @@ import {
import { readOrcaCliVersion } from './cli-version'
import { dispatch } from './dispatch'
import {
assertEnvironmentSelectorResolvable,
resolveEnvironmentSelectorForStore,
resolveHostFlagEnvironmentId
} from './execution-host-flag'
import { fetchSshTargetSummaries, listSshTargets } from './host-selector-alternatives'
@@ -124,9 +124,10 @@ export async function main(
// with the cross-kind hint rather than a bare store error at first use.
const listSshTargetsForSuggestion = async (): Promise<{ id: string; label: string }[]> =>
listSshTargets(new RuntimeClientClass(undefined, undefined, null, null))
if (typeof environmentSelector === 'string') {
await assertEnvironmentSelectorResolvable(environmentSelector, listSshTargetsForSuggestion)
}
const storeEnvironmentSelector =
typeof environmentSelector === 'string'
? await resolveEnvironmentSelectorForStore(environmentSelector, listSshTargetsForSuggestion)
: null
// Why: --host runtime:<id> names a paired server, not a filter over this
// runtime's rows, so it has to pick the connection before the client exists.
// An ambient ORCA_ENVIRONMENT is checked for disagreement too — silently
@@ -167,9 +168,7 @@ export async function main(
// An explicit --host runtime:<id> outranks an ambient pairing code or environment.
const remotePairingCode =
hostEnvironmentId !== null ? null : typeof pairingCode === 'string' ? pairingCode : suppressed
const remoteEnvironment =
hostEnvironmentId ??
(typeof environmentSelector === 'string' ? environmentSelector : suppressed)
const remoteEnvironment = hostEnvironmentId ?? storeEnvironmentSelector ?? suppressed
let client: RuntimeClient | undefined
await dispatch(parsed.commandPath, {
flags: parsed.flags,