diff --git a/src/cli/handlers/terminal.ts b/src/cli/handlers/terminal.ts index 012c06d6a99..a77810809d8 100644 --- a/src/cli/handlers/terminal.ts +++ b/src/cli/handlers/terminal.ts @@ -74,7 +74,7 @@ export const TERMINAL_HANDLERS: Record = { 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 }) => { diff --git a/src/cli/handlers/worktree.ts b/src/cli/handlers/worktree.ts index f6842dacfbb..2836528193e 100644 --- a/src/cli/handlers/worktree.ts +++ b/src/cli/handlers/worktree.ts @@ -104,7 +104,7 @@ export const WORKTREE_HANDLERS: Record = { '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 = { limit: getOptionalPositiveIntegerFlag(flags, 'limit') } ) - await annotateOmittedHostScope(client, result.result) + await annotateOmittedHostScope(result.result) printResult(result, json, formatWorktreeList) }, 'worktree show': async ({ flags, client, cwd, json }) => { diff --git a/src/cli/index-omitted-host-scope-selectors.test.ts b/src/cli/index-omitted-host-scope-selectors.test.ts index 1fbd77289f5..794fe327a82 100644 --- a/src/cli/index-omitted-host-scope-selectors.test.ts +++ b/src/cli/index-omitted-host-scope-selectors.test.ts @@ -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:` 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 () => { diff --git a/src/cli/index-terminal-list-host-scope.test.ts b/src/cli/index-terminal-list-host-scope.test.ts index 04b486df671..083e3b4f5fd 100644 --- a/src/cli/index-terminal-list-host-scope.test.ts +++ b/src/cli/index-terminal-list-host-scope.test.ts @@ -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 }] }) }) diff --git a/src/cli/omitted-host-scope-selectors.ts b/src/cli/omitted-host-scope-selectors.ts index 2666b116375..965eb11ee72 100644 --- a/src/cli/omitted-host-scope-selectors.ts +++ b/src/cli/omitted-host-scope-selectors.ts @@ -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 = Omit & { * 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 { 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 { +export async function annotateOmittedHostScope(result: { + hostScope?: ListingHostScopeWithSelectors +}): Promise { const scope = result.hostScope if (!scope || scope.omittedHostIds.length === 0) { return } - scope.omittedHostSelectors = await resolveOmittedHostScopeSelectors(client, scope.omittedHostIds) + scope.omittedHostSelectors = await resolveOmittedHostScopeSelectors(scope.omittedHostIds) } diff --git a/src/cli/specs/worktree-listing-scope-notes.ts b/src/cli/specs/worktree-listing-scope-notes.ts index 91449cc6584..b9de89356d1 100644 --- a/src/cli/specs/worktree-listing-scope-notes.ts +++ b/src/cli/specs/worktree-listing-scope-notes.ts @@ -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.' ]