mirror of
https://github.com/stablyai/orca.git
synced 2026-11-01 01:27:35 +00:00
fix(cli): stop suggesting --host to listings that reject it (#30012)
The hostScope annotation on terminal list, worktree list and worktree ps named --host local / --host ssh:<id> as the follow-up for an omitted host, but none of those commands accept --host. Those hosts are reached through the runtime that answered, so no flag selects them: report a null selector and say so in the human scope line. Paired servers keep --environment. Fixes #29901
This commit is contained in:
@@ -74,7 +74,7 @@ export const TERMINAL_HANDLERS: Record<string, CommandHandler> = {
|
||||
includeVisualLayouts: !json || flags.has('include-visual-layouts')
|
||||
}
|
||||
)
|
||||
await annotateOmittedHostScope(client, result.result)
|
||||
await annotateOmittedHostScope(result.result)
|
||||
printResult(result, json, formatTerminalList)
|
||||
},
|
||||
'terminal show': async ({ flags, client, cwd, json }) => {
|
||||
|
||||
@@ -104,7 +104,7 @@ export const WORKTREE_HANDLERS: Record<string, CommandHandler> = {
|
||||
'worktree.ps',
|
||||
{ limit: getOptionalPositiveIntegerFlag(flags, 'limit') }
|
||||
)
|
||||
await annotateOmittedHostScope(client, result.result)
|
||||
await annotateOmittedHostScope(result.result)
|
||||
const worktrees = result.result.worktrees.map(projectWorktreePsTerminalVerdict)
|
||||
printResult({ ...result, result: { ...result.result, worktrees } }, json, () =>
|
||||
formatWorktreePs(result.result)
|
||||
@@ -118,7 +118,7 @@ export const WORKTREE_HANDLERS: Record<string, CommandHandler> = {
|
||||
limit: getOptionalPositiveIntegerFlag(flags, 'limit')
|
||||
}
|
||||
)
|
||||
await annotateOmittedHostScope(client, result.result)
|
||||
await annotateOmittedHostScope(result.result)
|
||||
printResult(result, json, formatWorktreeList)
|
||||
},
|
||||
'worktree show': async ({ flags, client, cwd, json }) => {
|
||||
|
||||
@@ -41,6 +41,7 @@ vi.mock('child_process', async () => {
|
||||
})
|
||||
|
||||
import { main } from './index'
|
||||
import { COMMAND_SPECS } from './specs'
|
||||
import { okFixture, queueFixtures } from './test-fixtures'
|
||||
import { pairRuntimeEnvironment, useWorktreeAwarenessEnvironment } from './index-test-harness'
|
||||
|
||||
@@ -125,27 +126,69 @@ describe('omittedHostIds selector annotation', () => {
|
||||
expect(printed).toContain('runtime:env-retired (not selectable from this machine)')
|
||||
})
|
||||
|
||||
it('resolves an omitted SSH host against the targets the runtime actually knows', async () => {
|
||||
listEnvironmentsMock.mockReturnValue([])
|
||||
it.each([
|
||||
['terminal', 'list', 'req_terminal_list', { terminals: [TERMINAL_ROW] }],
|
||||
['worktree', 'list', 'req_worktree_list', { worktrees: [] }],
|
||||
['worktree', 'ps', 'req_worktree_ps', { worktrees: [] }]
|
||||
] as const)(
|
||||
'only suggests flags that `%s %s` accepts',
|
||||
async (group, action, requestId, rows) => {
|
||||
// Why: `--host local` / `--host ssh:<id>` were suggested here, and the listing rejected
|
||||
// them as unknown flags, breaking the agent loop the annotation exists for (#29901).
|
||||
pairRuntimeEnvironment(listEnvironmentsMock, 'env-paired', 'm4air')
|
||||
queueFixtures(
|
||||
callMock,
|
||||
okFixture(requestId, {
|
||||
...rows,
|
||||
totalCount: 1,
|
||||
truncated: false,
|
||||
hostScope: {
|
||||
hostIds: [],
|
||||
omittedHostIds: ['local', 'runtime:env-paired', 'ssh:box-1']
|
||||
}
|
||||
})
|
||||
)
|
||||
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
|
||||
|
||||
await main([group, action, '--json'], '/tmp/repo')
|
||||
|
||||
const printed = JSON.parse(String(logSpy.mock.calls[0]?.[0]))
|
||||
expect(printed.result.hostScope.omittedHostSelectors).toEqual([
|
||||
{ hostId: 'local', selector: null },
|
||||
{ hostId: 'runtime:env-paired', selector: '--environment m4air' },
|
||||
{ hostId: 'ssh:box-1', selector: null }
|
||||
])
|
||||
const spec = COMMAND_SPECS.find(
|
||||
(candidate) => candidate.path.join(' ') === `${group} ${action}`
|
||||
)
|
||||
for (const { selector } of printed.result.hostScope.omittedHostSelectors) {
|
||||
if (selector) {
|
||||
expect(spec?.allowedFlags).toContain(selector.split(' ')[0].replace(/^--/, ''))
|
||||
}
|
||||
}
|
||||
// An SSH host is reached through the answering runtime, so no target lookup is needed.
|
||||
expect(callMock).toHaveBeenCalledTimes(1)
|
||||
}
|
||||
)
|
||||
|
||||
it('says this runtime reaches an omitted local or SSH host in the human listing', async () => {
|
||||
queueFixtures(
|
||||
callMock,
|
||||
okFixture('req_terminal_list', {
|
||||
terminals: [TERMINAL_ROW],
|
||||
totalCount: 1,
|
||||
truncated: false,
|
||||
hostScope: { hostIds: ['local'], omittedHostIds: ['ssh:box-1', 'ssh:box-gone'] }
|
||||
}),
|
||||
okFixture('req_ssh_targets', { targets: [{ id: 'box-1', label: 'openclaw' }] })
|
||||
hostScope: { hostIds: [], omittedHostIds: ['local', 'ssh:box-1'] }
|
||||
})
|
||||
)
|
||||
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
|
||||
|
||||
await main(['terminal', 'list', '--json'], '/tmp/repo')
|
||||
await main(['terminal', 'list'], '/tmp/repo')
|
||||
|
||||
const printed = JSON.parse(String(logSpy.mock.calls[0]?.[0]))
|
||||
expect(printed.result.hostScope.omittedHostSelectors).toEqual([
|
||||
{ hostId: 'ssh:box-1', selector: '--host ssh:box-1' },
|
||||
{ hostId: 'ssh:box-gone', selector: null }
|
||||
])
|
||||
const printed = String(logSpy.mock.calls[0]?.[0])
|
||||
expect(printed).toContain('local (reached through this runtime; no flag selects it)')
|
||||
expect(printed).toContain('ssh:box-1 (reached through this runtime; no flag selects it)')
|
||||
expect(printed).not.toContain('--host')
|
||||
})
|
||||
|
||||
it('never keeps a host id out of omittedHostIds', async () => {
|
||||
|
||||
@@ -89,9 +89,9 @@ describe('orca terminal list host scope', () => {
|
||||
expect(printed.result.hostScope).toEqual({
|
||||
hostIds: ['ssh:box-1'],
|
||||
omittedHostIds: ['local'],
|
||||
// The CLI annotates each omitted host with the flag that reaches it; see
|
||||
// index-omitted-host-scope-selectors.test.ts.
|
||||
omittedHostSelectors: [{ hostId: 'local', selector: '--host local' }]
|
||||
// `terminal list` takes no --host, so no flag reaches the answering runtime's own hosts
|
||||
// (#29901); see index-omitted-host-scope-selectors.test.ts.
|
||||
omittedHostSelectors: [{ hostId: 'local', selector: null }]
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -4,13 +4,7 @@ import {
|
||||
type ParsedExecutionHost
|
||||
} from '../shared/execution-host'
|
||||
import type { RuntimeListingHostScope } from '../shared/runtime-listing-host-scope'
|
||||
import {
|
||||
findEnvironmentByName,
|
||||
findSshTargetByName,
|
||||
listSshTargets,
|
||||
type SshTargetSummary
|
||||
} from './host-selector-alternatives'
|
||||
import type { RuntimeClient } from './runtime-client'
|
||||
import { findEnvironmentByName } from './host-selector-alternatives'
|
||||
|
||||
export type OmittedHostScopeSelector = {
|
||||
hostId: ExecutionHostId
|
||||
@@ -38,11 +32,13 @@ export type WithAnnotatedHostScope<TResult> = Omit<TResult, 'hostScope'> & {
|
||||
* The ids are kept rather than filtered: dropping one would shrink what the listing admits it did
|
||||
* not cover, and `docs/reference/ssh-execution-boundary.md` requires a listing to name its gaps.
|
||||
* A `null` selector marks the ones this machine cannot name, which is the part a caller needs.
|
||||
* Only the local pairing store and SSH-target registry are consulted, so this answers "can I
|
||||
* select it", never "is it up" — no host is claimed live or exited on this path.
|
||||
* Only the local pairing store is consulted, so this answers "can I select it", never "is it up" —
|
||||
* no host is claimed live or exited on this path.
|
||||
*
|
||||
* `local` and `ssh:` hosts get no selector: the listing commands take no `--host` (#29901), and
|
||||
* the runtime that answered is the one that reaches them, so no flag would route elsewhere.
|
||||
*/
|
||||
export async function resolveOmittedHostScopeSelectors(
|
||||
client: RuntimeClient,
|
||||
omittedHostIds: readonly ExecutionHostId[]
|
||||
): Promise<OmittedHostScopeSelector[]> {
|
||||
const parsed = omittedHostIds.map((hostId) => ({
|
||||
@@ -52,13 +48,9 @@ export async function resolveOmittedHostScopeSelectors(
|
||||
const environments = parsed.some((entry) => entry.host?.kind === 'runtime')
|
||||
? await listPairedEnvironments()
|
||||
: []
|
||||
// Why: SSH targets need a round trip, so only pay for it when an ssh host was actually omitted.
|
||||
const sshTargets = parsed.some((entry) => entry.host?.kind === 'ssh')
|
||||
? await listSshTargets(client)
|
||||
: []
|
||||
return parsed.map(({ hostId, host }) => ({
|
||||
hostId,
|
||||
selector: resolveSelector(host, environments, sshTargets)
|
||||
selector: resolveSelector(host, environments)
|
||||
}))
|
||||
}
|
||||
|
||||
@@ -75,15 +67,8 @@ async function listPairedEnvironments(): Promise<{ id: string; name: string }[]>
|
||||
|
||||
function resolveSelector(
|
||||
host: ParsedExecutionHost | null,
|
||||
environments: readonly { id: string; name: string }[],
|
||||
sshTargets: readonly SshTargetSummary[]
|
||||
environments: readonly { id: string; name: string }[]
|
||||
): string | null {
|
||||
if (host?.kind === 'local') {
|
||||
return '--host local'
|
||||
}
|
||||
if (host?.kind === 'ssh') {
|
||||
return findSshTargetByName(sshTargets, host.targetId) ? `--host ssh:${host.targetId}` : null
|
||||
}
|
||||
if (host?.kind === 'runtime') {
|
||||
const environment = findEnvironmentByName(environments, host.environmentId)
|
||||
return environment ? `--environment ${environment.name}` : null
|
||||
@@ -108,19 +93,24 @@ export function formatListingHostScope(scope: ListingHostScopeWithSelectors | un
|
||||
return hostId
|
||||
}
|
||||
const selector = selectorByHostId.get(hostId)
|
||||
return selector ? `${hostId} (${selector})` : `${hostId} (not selectable from this machine)`
|
||||
if (selector) {
|
||||
return `${hostId} (${selector})`
|
||||
}
|
||||
const kind = parseExecutionHostId(hostId)?.kind
|
||||
return kind === 'local' || kind === 'ssh'
|
||||
? `${hostId} (reached through this runtime; no flag selects it)`
|
||||
: `${hostId} (not selectable from this machine)`
|
||||
})
|
||||
return `scope: ${covered} — not covered: ${omitted.join(', ')}`
|
||||
}
|
||||
|
||||
/** Attaches the resolved selectors in place; a listing with no omitted hosts pays nothing. */
|
||||
export async function annotateOmittedHostScope(
|
||||
client: RuntimeClient,
|
||||
result: { hostScope?: ListingHostScopeWithSelectors }
|
||||
): Promise<void> {
|
||||
export async function annotateOmittedHostScope(result: {
|
||||
hostScope?: ListingHostScopeWithSelectors
|
||||
}): Promise<void> {
|
||||
const scope = result.hostScope
|
||||
if (!scope || scope.omittedHostIds.length === 0) {
|
||||
return
|
||||
}
|
||||
scope.omittedHostSelectors = await resolveOmittedHostScopeSelectors(client, scope.omittedHostIds)
|
||||
scope.omittedHostSelectors = await resolveOmittedHostScopeSelectors(scope.omittedHostIds)
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
/** Shared by `worktree list` and `worktree ps`, which report host coverage the same way. */
|
||||
export const WORKTREE_LISTING_SCOPE_NOTES: readonly string[] = [
|
||||
'Each row carries the execution host that owns it (`host=`), and the trailing `scope:` line names every host the page covers plus the ones it does not.',
|
||||
'A host named under `not covered` may still have workspaces; an empty answer for it is not evidence that it has none. Each is annotated with the flag that reaches it, or marked not selectable from this machine.',
|
||||
'A host named under `not covered` may still have workspaces; an empty answer for it is not evidence that it has none. A paired server is annotated with the `--environment` flag that reaches it; this machine and its SSH targets are reached through the same runtime, so no flag selects them.',
|
||||
'The row cap is shared across hosts, so a host whose rows sort last is not starved out of the page.'
|
||||
]
|
||||
|
||||
Reference in New Issue
Block a user