From 85e7a7e7e97149cf63bc66a4219a32e1a183247a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 11 Oct 2026 17:29:57 +0200 Subject: [PATCH 1/3] test(cli): cover auth, connection status, proxy startup, and replay shard formatting Drives the five CRAP findings in #3420 through their owning command entry points: auth status/login/logout rendering, the connection status human block, the proxy command's daemon-backed startup and loopback bind, and replay-test shard device-name suffixes. --- src/__tests__/cli-auth-command.test.ts | 173 ++++++++++++++++++ src/cli/commands/__tests__/connection.test.ts | 159 ++++++++++++++++ src/cli/commands/__tests__/proxy.test.ts | 144 +++++++++++++++ .../reporters/__tests__/format.test.ts | 66 +++++++ 4 files changed, 542 insertions(+) create mode 100644 src/__tests__/cli-auth-command.test.ts create mode 100644 src/cli/commands/__tests__/connection.test.ts create mode 100644 src/cli/commands/__tests__/proxy.test.ts create mode 100644 src/cli/replay-test/reporters/__tests__/format.test.ts diff --git a/src/__tests__/cli-auth-command.test.ts b/src/__tests__/cli-auth-command.test.ts new file mode 100644 index 0000000000..4b1f05ab71 --- /dev/null +++ b/src/__tests__/cli-auth-command.test.ts @@ -0,0 +1,173 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { afterEach, test, vi } from 'vitest'; +import { runCliCapture } from './cli-capture.ts'; +import { writeCliSession } from '../cli/auth-session.ts'; +import { mkdtempForTestSync } from './test-utils/tmp-dir.ts'; + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +function writeStoredCliSession( + stateDir: string, + overrides: Partial[0]['session']> = {}, +) { + writeCliSession({ + stateDir, + session: { + version: 1, + id: 'cli-1', + cloudBaseUrl: 'https://cloud.example.test', + refreshCredential: 'refresh-credential', + createdAt: '2026-01-01T00:00:00.000Z', + ...overrides, + }, + }); +} + +test('auth with no subcommand renders an unauthenticated status', async () => { + const stateDir = mkdtempForTestSync('agent-device-auth-status-'); + const result = await runCliCapture(['auth', '--state-dir', stateDir]); + + assert.equal(result.code, null); + assert.equal(result.stdout, 'Not authenticated.\n'); +}); + +test('auth status renders every stored session field', async () => { + const stateDir = mkdtempForTestSync('agent-device-auth-status-full-'); + writeStoredCliSession(stateDir, { + id: 'cli-42', + workspaceId: 'ws-7', + accountId: 'acct-9', + expiresAt: '2020-01-01T00:00:00.000Z', + }); + + const result = await runCliCapture(['auth', 'status', '--state-dir', stateDir]); + + assert.equal(result.code, null); + assert.deepEqual(result.stdout.trim().split('\n'), [ + 'Authenticated with cloud CLI session.', + 'cloud=https://cloud.example.test', + 'session=cli-42', + 'workspace=ws-7', + 'account=acct-9', + 'expiresAt=2020-01-01T00:00:00.000Z', + 'status=expired', + ]); +}); + +test('auth status omits optional fields a minimal session does not carry', async () => { + const stateDir = mkdtempForTestSync('agent-device-auth-status-minimal-'); + writeStoredCliSession(stateDir); + + const result = await runCliCapture(['auth', 'status', '--state-dir', stateDir]); + + assert.equal(result.code, null); + assert.deepEqual(result.stdout.trim().split('\n'), [ + 'Authenticated with cloud CLI session.', + 'cloud=https://cloud.example.test', + 'session=cli-1', + ]); +}); + +test('auth login completes a device-code flow and stores the CLI session', async () => { + const stateDir = mkdtempForTestSync('agent-device-auth-login-'); + const requestUrls: string[] = []; + vi.stubGlobal( + 'fetch', + vi.fn(async (input: URL | RequestInfo) => { + const url = new URL(String(input)); + requestUrls.push(url.pathname); + if (url.pathname === '/api/control-plane/device-auth/start') { + return new Response( + JSON.stringify({ + deviceCode: 'device-code-1', + userCode: 'ABCD-1234', + verificationUri: 'https://cloud.example.test/login', + interval: 1, + }), + { status: 200, headers: { 'content-type': 'application/json' } }, + ); + } + return new Response( + JSON.stringify({ + status: 'approved', + accessToken: 'access-token-1', + expiresAt: '2026-02-01T00:00:00.000Z', + cliSession: { + id: 'cli-7', + refreshCredential: 'refresh-credential-7', + workspaceId: 'ws-7', + accountId: 'acct-7', + expiresAt: '2026-06-01T00:00:00.000Z', + }, + }), + { status: 200, headers: { 'content-type': 'application/json' } }, + ); + }), + ); + + const previousStdinIsTTY = process.stdin.isTTY; + const previousStdoutIsTTY = process.stdout.isTTY; + (process.stdin as { isTTY?: boolean }).isTTY = true; + (process.stdout as { isTTY?: boolean }).isTTY = true; + try { + const result = await runCliCapture(['auth', 'login', '--state-dir', stateDir], { + env: { + AGENT_DEVICE_CLOUD_BASE_URL: 'https://cloud.example.test', + // A remote shell with interactive TTYs takes the device-code branch, + // which prints the URL instead of launching a browser. + SSH_CONNECTION: '10.0.0.1 50000 10.0.0.2 22', + CI: undefined, + GITHUB_ACTIONS: undefined, + BUILDKITE: undefined, + }, + }); + + assert.equal(result.code, null); + assert.equal(result.stdout, 'Authenticated with cloud CLI session.\n'); + assert.match(result.stderr, /Open this URL on your machine/); + assert.deepEqual(requestUrls, [ + '/api/control-plane/device-auth/start', + '/api/control-plane/device-auth/poll', + ]); + const stored = JSON.parse( + fs.readFileSync(path.join(stateDir, 'auth', 'cli-session.json'), 'utf8'), + ) as { id: string; workspaceId?: string }; + assert.equal(stored.id, 'cli-7'); + assert.equal(stored.workspaceId, 'ws-7'); + } finally { + (process.stdin as { isTTY?: boolean }).isTTY = previousStdinIsTTY; + (process.stdout as { isTTY?: boolean }).isTTY = previousStdoutIsTTY; + } +}); + +test('auth logout removes a stored session and reports it', async () => { + const stateDir = mkdtempForTestSync('agent-device-auth-logout-removed-'); + writeStoredCliSession(stateDir); + + const result = await runCliCapture(['auth', 'logout', '--state-dir', stateDir]); + + assert.equal(result.code, null); + assert.equal(result.stdout, 'Removed stored cloud CLI session.\n'); + assert.equal(fs.existsSync(path.join(stateDir, 'auth', 'cli-session.json')), false); +}); + +test('auth logout without a stored session reports nothing to remove', async () => { + const stateDir = mkdtempForTestSync('agent-device-auth-logout-none-'); + const result = await runCliCapture(['auth', 'logout', '--state-dir', stateDir]); + + assert.equal(result.code, null); + assert.equal(result.stdout, 'No stored cloud CLI session.\n'); +}); + +test('auth rejects a subcommand outside status, login, and logout', async () => { + const stateDir = mkdtempForTestSync('agent-device-auth-invalid-'); + const result = await runCliCapture(['auth', 'rotate', '--state-dir', stateDir]); + + assert.equal(result.code, 1); + assert.match(result.stderr, /INVALID_ARGS/); + assert.match(result.stderr, /auth accepts only: status, login, logout/); +}); diff --git a/src/cli/commands/__tests__/connection.test.ts b/src/cli/commands/__tests__/connection.test.ts new file mode 100644 index 0000000000..e5b5c202a6 --- /dev/null +++ b/src/cli/commands/__tests__/connection.test.ts @@ -0,0 +1,159 @@ +// Human rendering of `connection status` (#3420): the shipped status block is the operator- +// facing projection of a stored remote connection, so its pending-vs-bound lease axes, the +// metro prepared marker, and the deferred notices are pinned through the CLI route. +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { test } from 'vitest'; +import { runCliCapture } from '../../../__tests__/cli-capture.ts'; +import { + hashRemoteConfigFile, + writeRemoteConnectionState, + type RemoteConnectionState, +} from '../../../remote/remote-connection-state.ts'; +import { mkdtempForTestSync } from '../../../__tests__/test-utils/tmp-dir.ts'; + +type ConnectionFixture = { + stateDir: string; + remoteConfigPath: string; +}; + +function connectionFixture(prefix: string): ConnectionFixture { + const tempRoot = mkdtempForTestSync(prefix); + return { + stateDir: path.join(tempRoot, '.state'), + remoteConfigPath: path.join(tempRoot, 'remote.json'), + }; +} + +function seedConnection( + fixture: ConnectionFixture, + remoteConfig: Record, + state: Partial & Pick, +): void { + fs.writeFileSync(fixture.remoteConfigPath, JSON.stringify(remoteConfig)); + const now = new Date().toISOString(); + writeRemoteConnectionState({ + stateDir: fixture.stateDir, + state: { + version: 1, + remoteConfigPath: fixture.remoteConfigPath, + remoteConfigHash: hashRemoteConfigFile(fixture.remoteConfigPath), + tenant: 'acme', + runId: 'run-123', + connectedAt: now, + updatedAt: now, + ...state, + }, + }); +} + +test('connection status without a stored connection says so for the default session', async () => { + const fixture = connectionFixture('agent-device-connection-status-none-'); + const result = await runCliCapture(['connection', 'status', '--state-dir', fixture.stateDir]); + + assert.equal(result.code, null); + assert.equal(result.stdout, 'No remote connection for "default".\n'); +}); + +test('connection status rejects a subcommand other than status', async () => { + const fixture = connectionFixture('agent-device-connection-status-invalid-'); + const result = await runCliCapture(['connection', 'rotate', '--state-dir', fixture.stateDir]); + + assert.equal(result.code, 1); + assert.match(result.stderr, /INVALID_ARGS/); + assert.match(result.stderr, /connection accepts only: status/); +}); + +test('connection status renders a pending lease and the deferred allocation notice', async () => { + const fixture = connectionFixture('agent-device-connection-status-pending-'); + seedConnection( + fixture, + { daemonBaseUrl: 'https://daemon.example.test' }, + { + session: 'adc-pending', + leaseProvider: 'proxy', + }, + ); + + const result = await runCliCapture([ + 'connection', + 'status', + '--state-dir', + fixture.stateDir, + '--session', + 'adc-pending', + ]); + + assert.equal(result.code, null); + assert.match(result.stdout, /Configured remote session "adc-pending"\./); + assert.match(result.stdout, /tenant=acme runId=run-123 leaseId=pending backend=pending/); + assert.match(result.stdout, new RegExp(`remoteConfig=${fixture.remoteConfigPath}`)); + assert.match(result.stdout, /metro=not-prepared/); + assert.match(result.stdout, /No live device session has been created/); +}); + +test('connection status renders a bound lease and a prepared runtime without notices', async () => { + const fixture = connectionFixture('agent-device-connection-status-bound-'); + seedConnection( + fixture, + { daemonBaseUrl: 'https://daemon.example.test' }, + { + session: 'adc-bound', + leaseId: 'lease-1', + leaseBackend: 'android-instance', + leaseProvider: 'proxy', + runtime: { platform: 'android' }, + }, + ); + + const result = await runCliCapture([ + 'connection', + 'status', + '--state-dir', + fixture.stateDir, + '--session', + 'adc-bound', + ]); + + assert.equal(result.code, null); + assert.match(result.stdout, /tenant=acme runId=run-123 leaseId=lease-1 backend=android-instance/); + assert.match(result.stdout, /metro=prepared/); + assert.doesNotMatch(result.stdout, /No live device session has been created/); +}); + +test('connection status re-emits the deferred Metro notice for a bound lease', async () => { + const fixture = connectionFixture('agent-device-connection-status-metro-'); + seedConnection( + fixture, + { + daemonBaseUrl: 'https://daemon.example.test', + metroPublicBaseUrl: 'https://sandbox.example.test', + }, + { + session: 'adc-metro', + leaseId: 'lease-2', + leaseBackend: 'android-instance', + leaseProvider: 'proxy', + }, + ); + + const result = await runCliCapture([ + 'connection', + 'status', + '--state-dir', + fixture.stateDir, + '--session', + 'adc-metro', + ]); + + assert.equal(result.code, null); + assert.match(result.stdout, /metro=not-prepared/); + assert.match(result.stdout, /Metro runtime is not prepared yet/); + assert.match( + result.stdout, + new RegExp( + `agent-device metro prepare --remote-config ${fixture.remoteConfigPath} --session adc-metro`, + ), + ); +}); diff --git a/src/cli/commands/__tests__/proxy.test.ts b/src/cli/commands/__tests__/proxy.test.ts new file mode 100644 index 0000000000..9c058114d4 --- /dev/null +++ b/src/cli/commands/__tests__/proxy.test.ts @@ -0,0 +1,144 @@ +// #3420: the proxy command's startup path — daemon-backed upstream resolution, client-token +// sourcing, and real loopback binding — is pinned through the command handler itself. The +// process-spawn seam (`ensureDaemon`) and the output sink are mocked at the module boundary the +// command consumes; the proxy server and its listen are the real ones. +import assert from 'node:assert/strict'; +import http from 'node:http'; +import { afterEach, test, vi } from 'vitest'; +import { + skipWhenLoopbackUnavailable, + closeLoopbackServer, +} from '../../../__tests__/test-utils/loopback.ts'; +import { mkdtempForTestSync } from '../../../__tests__/test-utils/tmp-dir.ts'; +import { resolveDaemonPaths } from '@agent-device/daemon-contracts/daemon-resolution'; + +const mocks = vi.hoisted(() => ({ + ensureDaemon: vi.fn(), + writeCommandOutput: vi.fn(async (..._args: unknown[]) => undefined), + createdServers: [] as http.Server[], +})); + +vi.mock('../../../daemon-client/daemon-client-lifecycle.ts', async (importOriginal) => ({ + ...(await importOriginal()), + ensureDaemon: mocks.ensureDaemon, +})); +vi.mock('@agent-device/proxy', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + createDaemonProxyServer: (options: Parameters[0]) => { + const server = actual.createDaemonProxyServer(options); + mocks.createdServers.push(server); + return server; + }, + }; +}); +vi.mock('../shared.ts', () => ({ writeCommandOutput: mocks.writeCommandOutput })); + +import { proxyCommand } from '../proxy.ts'; + +afterEach(async () => { + vi.clearAllMocks(); + const servers = mocks.createdServers.splice(0, mocks.createdServers.length); + for (const server of servers) { + await closeLoopbackServer(server); + } +}); + +function runProxy(flags: Partial[0]['flags']> = {}) { + // The command streams its startup card and then serves until interrupted, so the handler + // promise intentionally never settles; the test observes the emitted output and lets it float. + void proxyCommand({ + positionals: [], + flags: { json: true, help: false, version: false, ...flags }, + client: {} as never, + }); +} + +async function waitForStartupOutput(): Promise<{ + data: Record; + render: () => string; +}> { + await vi.waitFor(() => { + assert.equal(mocks.writeCommandOutput.mock.calls.length, 1); + }); + const call = mocks.writeCommandOutput.mock.calls[0]!; + return { + data: call[1] as Record, + render: call[2] as () => string, + }; +} + +test('proxy rejects positional arguments', async () => { + await assert.rejects( + proxyCommand({ + positionals: ['extra'], + flags: { json: true, help: false, version: false }, + client: {} as never, + }), + (error: { code?: string }) => error.code === 'INVALID_ARGS', + ); + assert.equal(mocks.ensureDaemon.mock.calls.length, 0); +}); + +test('proxy refuses to start when the local daemon has no HTTP endpoint', async () => { + mocks.ensureDaemon.mockResolvedValue({ + info: { token: 'daemon-secret', pid: 4242 }, + startedByClient: false, + }); + + await assert.rejects( + proxyCommand({ + positionals: [], + flags: { json: true, help: false, version: false }, + client: {} as never, + }), + (error: { code?: string; details?: { hint?: string } }) => + error.code === 'COMMAND_FAILED' && /fresh --state-dir/.test(String(error.details?.hint)), + ); + assert.equal(mocks.writeCommandOutput.mock.calls.length, 0); +}); + +test('proxy binds a loopback listener and reports the supplied client token', async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; + const stateDir = mkdtempForTestSync('agent-device-proxy-start-'); + mocks.ensureDaemon.mockResolvedValue({ + info: { httpPort: 54321, token: 'daemon-secret', pid: 4242 }, + startedByClient: false, + }); + + runProxy({ + stateDir, + // The trimmed flag value becomes the client token instead of a generated one. + daemonAuthToken: ' proxy-token ', + }); + const { data, render } = await waitForStartupOutput(); + + assert.equal(data.upstreamBaseUrl, 'http://127.0.0.1:54321'); + assert.equal(data.token, 'proxy-token'); + assert.equal(data.stateDir, resolveDaemonPaths(stateDir).baseDir); + const proxyBaseUrl = String(data.proxyBaseUrl); + assert.match(proxyBaseUrl, /^http:\/\/(127\.0\.0\.1|\[::1\]):\d+$/); + assert.equal(data.agentDeviceBaseUrl, `${proxyBaseUrl}/agent-device`); + const startupCard = render(); + assert.match(startupCard, /Proxy listening at/); + assert.ok(startupCard.includes(proxyBaseUrl)); + // The daemon the proxy fronts is the local one of the resolved state dir, never a remote URL. + const settings = mocks.ensureDaemon.mock.calls[0]![0]; + assert.equal(settings.paths.baseDir, resolveDaemonPaths(stateDir).baseDir); + assert.equal(settings.remoteBaseUrl, undefined); +}); + +test('proxy honors proxy host and port flags and mints a token when none is supplied', async (t) => { + if (await skipWhenLoopbackUnavailable(t)) return; + mocks.ensureDaemon.mockResolvedValue({ + info: { httpPort: 54322, token: 'daemon-secret', pid: 4242 }, + startedByClient: false, + }); + + runProxy({ proxyHost: ' localhost ', proxyPort: 0 }); + const { data } = await waitForStartupOutput(); + + assert.match(String(data.proxyBaseUrl), /^http:\/\/(localhost|\[::1\]|127\.0\.0\.1):\d+$/); + assert.match(String(data.token), /^[0-9a-f]{64}$/); +}); diff --git a/src/cli/replay-test/reporters/__tests__/format.test.ts b/src/cli/replay-test/reporters/__tests__/format.test.ts new file mode 100644 index 0000000000..05bded6547 --- /dev/null +++ b/src/cli/replay-test/reporters/__tests__/format.test.ts @@ -0,0 +1,66 @@ +// Shard suffix rendering for replay-test display names (#3420): the `[n/m device]` suffix is +// what operators scan in sharded runs, so device-name preference, whitespace collapse, and the +// missing-count fallback are pinned at the owning module's entry point. +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import type { ReplaySuiteTestResult } from '@agent-device/contracts/replay'; +import { + formatReplayTestShardSuffix, + replayTestCaseName, + replayTestDisplayNameWithFile, +} from '../format.ts'; + +function shardedResult( + overrides: Partial> = {}, +): ReplaySuiteTestResult { + return { + file: '/flows/login.ad', + title: 'Login', + session: 'default', + status: 'passed', + durationMs: 10, + attempts: 1, + replayed: 0, + healed: 0, + ...overrides, + }; +} + +test('shard suffix renders the index, count, and device name', () => { + assert.equal( + formatReplayTestShardSuffix( + shardedResult({ shardIndex: 0, shardCount: 2, deviceName: 'Pixel 8', deviceId: 'em-1' }), + ), + ' [1/2 Pixel 8]', + ); +}); + +test('shard suffix falls back to the device id when no name is carried', () => { + assert.equal( + formatReplayTestShardSuffix(shardedResult({ shardIndex: 1, shardCount: 3, deviceId: 'em-5' })), + ' [2/3 em-5]', + ); +}); + +test('shard suffix drops blank device fields and unknown counts', () => { + assert.equal(formatReplayTestShardSuffix(shardedResult({ shardIndex: 0 })), ' [1/?]'); + assert.equal( + formatReplayTestShardSuffix( + shardedResult({ shardIndex: 2, shardCount: 4, deviceName: ' ', deviceId: ' ' }), + ), + ' [3/4]', + ); +}); + +test('display names append the shard suffix to the title and fall back to the filename', () => { + const sharded = shardedResult({ shardIndex: 0, shardCount: 2, deviceName: 'Pixel 8' }); + assert.equal(replayTestDisplayNameWithFile(sharded), 'Login [1/2 Pixel 8]'); + assert.equal( + replayTestDisplayNameWithFile({ + ...sharded, + title: ' ', + }), + 'login.ad [1/2 Pixel 8]', + ); + assert.equal(replayTestCaseName(sharded), 'Login [1/2 Pixel 8]'); +}); From f78c2a197ffec3f68971a5f4484dacdd4ab4d921 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 11 Oct 2026 17:36:22 +0200 Subject: [PATCH 2/3] test(cli): tighten proxy host/port and connection notice assertions Review follow-up: record the arguments the command passes to the proxy server's listen so the --proxy-host/--proxy-port test cannot pass on defaults, and pin the absence of the deferred Metro notice in the bound-lease connection status case. --- src/cli/commands/__tests__/connection.test.ts | 1 + src/cli/commands/__tests__/proxy.test.ts | 22 +++++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/cli/commands/__tests__/connection.test.ts b/src/cli/commands/__tests__/connection.test.ts index e5b5c202a6..a698c04e4d 100644 --- a/src/cli/commands/__tests__/connection.test.ts +++ b/src/cli/commands/__tests__/connection.test.ts @@ -120,6 +120,7 @@ test('connection status renders a bound lease and a prepared runtime without not assert.match(result.stdout, /tenant=acme runId=run-123 leaseId=lease-1 backend=android-instance/); assert.match(result.stdout, /metro=prepared/); assert.doesNotMatch(result.stdout, /No live device session has been created/); + assert.doesNotMatch(result.stdout, /Metro runtime is not prepared yet/); }); test('connection status re-emits the deferred Metro notice for a bound lease', async () => { diff --git a/src/cli/commands/__tests__/proxy.test.ts b/src/cli/commands/__tests__/proxy.test.ts index 9c058114d4..94365d5d8a 100644 --- a/src/cli/commands/__tests__/proxy.test.ts +++ b/src/cli/commands/__tests__/proxy.test.ts @@ -8,6 +8,7 @@ import { afterEach, test, vi } from 'vitest'; import { skipWhenLoopbackUnavailable, closeLoopbackServer, + listenOnLoopback, } from '../../../__tests__/test-utils/loopback.ts'; import { mkdtempForTestSync } from '../../../__tests__/test-utils/tmp-dir.ts'; import { resolveDaemonPaths } from '@agent-device/daemon-contracts/daemon-resolution'; @@ -16,6 +17,7 @@ const mocks = vi.hoisted(() => ({ ensureDaemon: vi.fn(), writeCommandOutput: vi.fn(async (..._args: unknown[]) => undefined), createdServers: [] as http.Server[], + listenArgs: [] as { port: number; host: string }[], })); vi.mock('../../../daemon-client/daemon-client-lifecycle.ts', async (importOriginal) => ({ @@ -28,6 +30,12 @@ vi.mock('@agent-device/proxy', async (importOriginal) => { ...actual, createDaemonProxyServer: (options: Parameters[0]) => { const server = actual.createDaemonProxyServer(options); + // Record what the command asks the real server to bind; the bind itself stays real. + const listen = server.listen.bind(server); + server.listen = ((port?: number, host?: string, callback?: () => void) => { + mocks.listenArgs.push({ port: port ?? 0, host: String(host) }); + return listen(port, host, callback); + }) as typeof server.listen; mocks.createdServers.push(server); return server; }, @@ -39,6 +47,7 @@ import { proxyCommand } from '../proxy.ts'; afterEach(async () => { vi.clearAllMocks(); + mocks.listenArgs.splice(0, mocks.listenArgs.length); const servers = mocks.createdServers.splice(0, mocks.createdServers.length); for (const server of servers) { await closeLoopbackServer(server); @@ -114,6 +123,7 @@ test('proxy binds a loopback listener and reports the supplied client token', as }); const { data, render } = await waitForStartupOutput(); + assert.deepEqual(mocks.listenArgs, [{ port: 0, host: '127.0.0.1' }]); assert.equal(data.upstreamBaseUrl, 'http://127.0.0.1:54321'); assert.equal(data.token, 'proxy-token'); assert.equal(data.stateDir, resolveDaemonPaths(stateDir).baseDir); @@ -135,10 +145,18 @@ test('proxy honors proxy host and port flags and mints a token when none is supp info: { httpPort: 54322, token: 'daemon-secret', pid: 4242 }, startedByClient: false, }); + // Reserve a real port through an ephemeral bind so the flag value is distinguishable + // from the `0` default the command would otherwise pass to listen. + const sentinel = http.createServer(); + const reservedPort = await listenOnLoopback(sentinel); + await closeLoopbackServer(sentinel); - runProxy({ proxyHost: ' localhost ', proxyPort: 0 }); + runProxy({ proxyHost: ' localhost ', proxyPort: reservedPort }); const { data } = await waitForStartupOutput(); - assert.match(String(data.proxyBaseUrl), /^http:\/\/(localhost|\[::1\]|127\.0\.0\.1):\d+$/); + assert.deepEqual(mocks.listenArgs, [{ port: reservedPort, host: 'localhost' }]); + // The reported URL carries the bound address, which names the reserved port either way. + assert.match(String(data.proxyBaseUrl), /^http:\/\/(localhost|127\.0\.0\.1|\[::1\]):\d+$/); + assert.ok(String(data.proxyBaseUrl).endsWith(`:${reservedPort}`)); assert.match(String(data.token), /^[0-9a-f]{64}$/); }); From af9efc5b2c95d25e52cc3ec7ffca73070299a022 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 11 Oct 2026 18:53:59 +0200 Subject: [PATCH 3/3] test(cli): address review layout and assertion nits Move the auth command test beside connection and proxy, assert the command's render closure against the exported renderProxyStartup payload, and replace the proxy test's blind client cast with a typed stub that throws on any access. --- .../commands/__tests__/auth.test.ts} | 6 ++--- src/cli/commands/__tests__/proxy.test.ts | 22 +++++++++++++++---- 2 files changed, 21 insertions(+), 7 deletions(-) rename src/{__tests__/cli-auth-command.test.ts => cli/commands/__tests__/auth.test.ts} (96%) diff --git a/src/__tests__/cli-auth-command.test.ts b/src/cli/commands/__tests__/auth.test.ts similarity index 96% rename from src/__tests__/cli-auth-command.test.ts rename to src/cli/commands/__tests__/auth.test.ts index 4b1f05ab71..39c8002737 100644 --- a/src/__tests__/cli-auth-command.test.ts +++ b/src/cli/commands/__tests__/auth.test.ts @@ -2,9 +2,9 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import path from 'node:path'; import { afterEach, test, vi } from 'vitest'; -import { runCliCapture } from './cli-capture.ts'; -import { writeCliSession } from '../cli/auth-session.ts'; -import { mkdtempForTestSync } from './test-utils/tmp-dir.ts'; +import { runCliCapture } from '../../../__tests__/cli-capture.ts'; +import { writeCliSession } from '../../auth-session.ts'; +import { mkdtempForTestSync } from '../../../__tests__/test-utils/tmp-dir.ts'; afterEach(() => { vi.unstubAllGlobals(); diff --git a/src/cli/commands/__tests__/proxy.test.ts b/src/cli/commands/__tests__/proxy.test.ts index 94365d5d8a..821ce3af03 100644 --- a/src/cli/commands/__tests__/proxy.test.ts +++ b/src/cli/commands/__tests__/proxy.test.ts @@ -43,7 +43,16 @@ vi.mock('@agent-device/proxy', async (importOriginal) => { }); vi.mock('../shared.ts', () => ({ writeCommandOutput: mocks.writeCommandOutput })); -import { proxyCommand } from '../proxy.ts'; +import { proxyCommand, renderProxyStartup } from '../proxy.ts'; +import type { AgentDeviceClient } from '../../../agent-device-client.ts'; + +// The proxy command never reads the client; a typed stub that throws on any access proves it +// without the `{} as never` blind cast. +const unreferencedClient = new Proxy({} as AgentDeviceClient, { + get: () => { + throw new Error('proxy must not touch the client'); + }, +}); afterEach(async () => { vi.clearAllMocks(); @@ -60,7 +69,7 @@ function runProxy(flags: Partial[0]['flags']> = void proxyCommand({ positionals: [], flags: { json: true, help: false, version: false, ...flags }, - client: {} as never, + client: unreferencedClient, }); } @@ -83,7 +92,7 @@ test('proxy rejects positional arguments', async () => { proxyCommand({ positionals: ['extra'], flags: { json: true, help: false, version: false }, - client: {} as never, + client: unreferencedClient, }), (error: { code?: string }) => error.code === 'INVALID_ARGS', ); @@ -100,7 +109,7 @@ test('proxy refuses to start when the local daemon has no HTTP endpoint', async proxyCommand({ positionals: [], flags: { json: true, help: false, version: false }, - client: {} as never, + client: unreferencedClient, }), (error: { code?: string; details?: { hint?: string } }) => error.code === 'COMMAND_FAILED' && /fresh --state-dir/.test(String(error.details?.hint)), @@ -131,6 +140,11 @@ test('proxy binds a loopback listener and reports the supplied client token', as assert.match(proxyBaseUrl, /^http:\/\/(127\.0\.0\.1|\[::1\]):\d+$/); assert.equal(data.agentDeviceBaseUrl, `${proxyBaseUrl}/agent-device`); const startupCard = render(); + // The human render is the exported startup-card renderer applied to the emitted payload. + assert.equal( + startupCard, + renderProxyStartup(data as unknown as Parameters[0]), + ); assert.match(startupCard, /Proxy listening at/); assert.ok(startupCard.includes(proxyBaseUrl)); // The daemon the proxy fronts is the local one of the resolved state dir, never a remote URL.