From e04c2f9e6200b74a71eeffe107005b2c8b709f6d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 19 Oct 2026 12:18:31 -0700 Subject: [PATCH] fix(cli): --environment connects to the server a differently-cased name resolved to (#30834) --- src/cli/execution-host-flag.test.ts | 56 ++++++++++++++----- src/cli/execution-host-flag.ts | 16 ++++-- .../index-local-command-routing-flags.test.ts | 13 +++++ src/cli/index.ts | 13 ++--- 4 files changed, 72 insertions(+), 26 deletions(-) diff --git a/src/cli/execution-host-flag.test.ts b/src/cli/execution-host-flag.test.ts index b387558b595..4cb9d9b9a36 100644 --- a/src/cli/execution-host-flag.test.ts +++ b/src/cli/execution-host-flag.test.ts @@ -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' + ) }) }) diff --git a/src/cli/execution-host-flag.ts b/src/cli/execution-host-flag.ts index 7e87634f49c..ba4cd93287d 100644 --- a/src/cli/execution-host-flag.ts +++ b/src/cli/execution-host-flag.ts @@ -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 -): Promise { +): Promise { 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() diff --git a/src/cli/index-local-command-routing-flags.test.ts b/src/cli/index-local-command-routing-flags.test.ts index 22cb6cc9c3e..aecd984668f 100644 --- a/src/cli/index-local-command-routing-flags.test.ts +++ b/src/cli/index-local-command-routing-flags.test.ts @@ -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 = '' diff --git a/src/cli/index.ts b/src/cli/index.ts index 932786d24e1..663a29b3fc9 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -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: 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: 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,