From 32fd43a08b033ce8389885fb6505dc65bc8504ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 29 Jun 2026 13:26:14 +0200 Subject: [PATCH 1/2] feat: opt-in agent-cost wallClockMs behind --cost Add per-command wall-clock latency as a purely additive, opt-in response field (cost.wallClockMs) gated behind a new global --cost flag. The flag plumbs end to end mirroring --debug: cli-flags definition + GLOBAL_FLAG_KEYS, AgentDeviceClientConfig/overrides, buildClientConfig, buildMeta (meta.includeCost), the DaemonRequestMeta contract, and the boundary parse in daemonCommandRequestSchema so it survives the HTTP edge. The graft lives in request-router handleRequest (the seam that owns the outer wall-clock incl. lock + execute + finalize). It mirrors the conditional registerDownloadableArtifacts spread: when --cost is off OR the response is an error, the response is returned untouched. Only on an opted-in successful response is cost appended, so the default serialized DaemonResponse is byte-identical to today (Maestro .ad recompare safe). Proven by the parity test (flag-off identity, flag-on additive-only, error path, boundary survival). Additive / semver-minor. MCP exposure and richer signals (roundTrips, nodeCount) are deferred to follow-up slices. --- src/cli.ts | 1 + src/client-normalizers.ts | 1 + src/client-types.ts | 2 + src/contracts.ts | 5 + src/daemon-client.ts | 1 + .../__tests__/request-router-cost.test.ts | 157 ++++++++++++++++++ src/daemon/request-router.ts | 11 +- src/utils/cli-flags.ts | 17 +- 8 files changed, 193 insertions(+), 2 deletions(-) create mode 100644 src/daemon/__tests__/request-router-cost.test.ts diff --git a/src/cli.ts b/src/cli.ts index 89505716a3..762ab1c10e 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -252,6 +252,7 @@ export async function runCli(argv: string[], deps: CliDeps = DEFAULT_CLI_DEPS): lockPlatform: binding.defaultPlatform, cwd: process.cwd(), debug: debugOutputEnabled, + cost: currentFlags.cost, }); let parsedBatchSteps: BatchStep[] | undefined; if (command === 'batch') { diff --git a/src/client-normalizers.ts b/src/client-normalizers.ts index 513e5a071b..9df9c3de32 100644 --- a/src/client-normalizers.ts +++ b/src/client-normalizers.ts @@ -352,6 +352,7 @@ export function buildMeta(options: InternalRequestOptions): DaemonRequest['meta' cwd: options.cwd, sessionExplicit: options.session !== undefined, debug: options.debug, + includeCost: options.cost, lockPolicy: options.lockPolicy, lockPlatform: options.lockPlatform, ...leaseScopeToRequestMeta(leaseScope), diff --git a/src/client-types.ts b/src/client-types.ts index 1831bb14cd..f1676cf17b 100644 --- a/src/client-types.ts +++ b/src/client-types.ts @@ -69,6 +69,7 @@ export type AgentDeviceClientConfig = RemoteConnectionProfileFields & { runtime?: SessionRuntimeHints; cwd?: string; debug?: boolean; + cost?: boolean; iosXctestrunFile?: string; iosXctestDerivedDataPath?: string; iosXctestEnvDir?: string; @@ -95,6 +96,7 @@ export type AgentDeviceRequestOverrides = Pick< | 'leaseTtlMs' | 'cwd' | 'debug' + | 'cost' | 'iosXctestrunFile' | 'iosXctestDerivedDataPath' | 'iosXctestEnvDir' diff --git a/src/contracts.ts b/src/contracts.ts index 1b5d808e22..a6bbdf45cf 100644 --- a/src/contracts.ts +++ b/src/contracts.ts @@ -61,6 +61,7 @@ export type NetworkIncludeMode = (typeof NETWORK_INCLUDE_MODES)[number]; export type DaemonRequestMeta = { requestId?: string; debug?: boolean; + includeCost?: boolean; cwd?: string; sessionExplicit?: boolean; tenantId?: string; @@ -101,8 +102,11 @@ export type DaemonArtifact = { path?: string; }; +export type ResponseCost = { wallClockMs: number }; + export type DaemonResponseData = Record & { artifacts?: DaemonArtifact[]; + cost?: ResponseCost; }; export type DaemonError = { @@ -423,6 +427,7 @@ export const daemonCommandRequestSchema = schema((input, path) => : { requestId: optionalString(meta, 'requestId', `${path}.meta`), debug: optionalBoolean(meta, 'debug', `${path}.meta`), + includeCost: optionalBoolean(meta, 'includeCost', `${path}.meta`), cwd: optionalString(meta, 'cwd', `${path}.meta`), sessionExplicit: optionalBoolean(meta, 'sessionExplicit', `${path}.meta`), tenantId: optionalString(meta, 'tenantId', `${path}.meta`), diff --git a/src/daemon-client.ts b/src/daemon-client.ts index 4126405adf..abaa838078 100644 --- a/src/daemon-client.ts +++ b/src/daemon-client.ts @@ -48,6 +48,7 @@ export async function sendToDaemon(req: Omit): Promise { + const actual = await importOriginal(); + return { ...actual, dispatchCommand: vi.fn(async () => ({})) }; +}); + +vi.mock('../../platforms/ios/runner-client.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, stopIosRunnerSession: vi.fn(async () => {}) }; +}); + +vi.mock('../device-ready.ts', () => ({ ensureDeviceReady: vi.fn(async () => {}) })); + +import { dispatchCommand } from '../../core/dispatch.ts'; +import { createRequestHandler } from '../request-router.ts'; +import type { DaemonRequest, SessionState } from '../types.ts'; +import { LeaseRegistry } from '../lease-registry.ts'; +import { makeSessionStore } from '../../__tests__/test-utils/store-factory.ts'; +import { daemonCommandRequestSchema } from '../../contracts.ts'; + +const mockDispatch = vi.mocked(dispatchCommand); + +// A representative, structurally rich daemon payload so the parity assertions +// exercise nested objects/arrays rather than a trivial flat record. +const REPRESENTATIVE_PAYLOAD = { + message: 'home-ok', + detail: { nested: true, count: 3 }, + items: [1, 2, 3], +} as const; + +function makeIosSession(name: string): SessionState { + return { + name, + createdAt: 1_700_000_000_000, + actions: [], + device: { + platform: 'ios', + target: 'mobile', + id: 'SIM-001', + name: 'iPhone 16', + kind: 'simulator', + booted: true, + simulatorSetPath: '/tmp/tenant-a/set', + }, + }; +} + +function makeHandler(sessionStore = makeSessionStore('agent-device-router-cost-')) { + return { + sessionStore, + handler: createRequestHandler({ + logPath: path.join(os.tmpdir(), 'daemon.log'), + token: 'test-token', + sessionStore, + leaseRegistry: new LeaseRegistry(), + trackDownloadableArtifact: () => 'artifact-id', + }), + }; +} + +function baseRequest(overrides: Partial = {}): DaemonRequest { + return { + token: 'test-token', + session: 'cost-session', + command: 'home', + positionals: [], + flags: {}, + ...overrides, + }; +} + +beforeEach(() => { + mockDispatch.mockReset(); + mockDispatch.mockImplementation(async () => ({ ...REPRESENTATIVE_PAYLOAD })); +}); + +test('(a) flag-off identity: meta.includeCost absent === no meta at all, byte-identical and no cost', async () => { + const { sessionStore, handler } = makeHandler(); + sessionStore.set('cost-session', makeIosSession('cost-session')); + + const respNoMeta = await handler(baseRequest()); + const respMetaWithoutCost = await handler(baseRequest({ meta: {} })); + + // The serialized wire shape must be identical whether `meta` is omitted or + // present-without-includeCost. This is the Maestro `.ad` recompare invariant. + expect(JSON.stringify(respNoMeta)).toBe(JSON.stringify(respMetaWithoutCost)); + + expect(respNoMeta.ok).toBe(true); + expect(respMetaWithoutCost.ok).toBe(true); + if (respMetaWithoutCost.ok) { + expect('cost' in (respMetaWithoutCost.data ?? {})).toBe(false); + } + if (respNoMeta.ok) { + expect(respNoMeta.data).toEqual(REPRESENTATIVE_PAYLOAD); + } +}); + +test('(b) flag-on additive-only: cost.wallClockMs is the ONLY delta vs flag-off', async () => { + const { sessionStore, handler } = makeHandler(); + sessionStore.set('cost-session', makeIosSession('cost-session')); + + const respFlagOff = await handler(baseRequest()); + const respFlagOn = await handler(baseRequest({ meta: { includeCost: true } })); + + expect(respFlagOff.ok).toBe(true); + expect(respFlagOn.ok).toBe(true); + if (!respFlagOff.ok || !respFlagOn.ok) return; + + const cost = respFlagOn.data?.cost; + expect(typeof cost?.wallClockMs).toBe('number'); + expect(cost?.wallClockMs).toBeGreaterThanOrEqual(0); + + // Deleting the single added key must leave a payload deep-equal to flag-off. + delete respFlagOn.data?.cost; + expect(respFlagOn.data).toEqual(respFlagOff.data); +}); + +test('(c) error path: a failing request with includeCost:true produces NO cost', async () => { + const { sessionStore, handler } = makeHandler(); + sessionStore.set('cost-session', makeIosSession('cost-session')); + + // Conflicting explicit selector under a reject lock policy fails before dispatch. + const failingRequest = baseRequest({ + flags: { udid: 'SIM-999' }, + meta: { lockPolicy: 'reject', includeCost: true }, + }); + + const errOn = await handler(failingRequest); + expect(errOn.ok).toBe(false); + // The graft is gated on `response.ok`, so an error response is returned + // untouched: it carries an `error` (no `data`) and never a `cost` key. + expect('cost' in errOn).toBe(false); + expect((errOn as { data?: unknown }).data).toBeUndefined(); + if (!errOn.ok) { + expect(errOn.error.code).toBe('INVALID_ARGS'); + expect('cost' in errOn.error).toBe(false); + } +}); + +test('(d) boundary survival: meta.includeCost survives daemonCommandRequestSchema parsing', () => { + const parsed = daemonCommandRequestSchema.parse({ + command: 'home', + positionals: [], + meta: { includeCost: true }, + }); + expect(parsed.meta?.includeCost).toBe(true); + + const parsedOff = daemonCommandRequestSchema.parse({ + command: 'home', + positionals: [], + meta: {}, + }); + expect(parsedOff.meta?.includeCost).toBeUndefined(); +}); diff --git a/src/daemon/request-router.ts b/src/daemon/request-router.ts index a7379a81e8..1c7a14d497 100644 --- a/src/daemon/request-router.ts +++ b/src/daemon/request-router.ts @@ -4,6 +4,7 @@ import { } from '../core/dispatch-resolve.ts'; import { AppError, normalizeError } from '../utils/errors.ts'; import { timingSafeStringEqual } from '../utils/timing-safe-equal.ts'; +import type { ResponseCost } from '../contracts.ts'; import type { DaemonInvokeFn, DaemonRequest, DaemonResponse } from './types.ts'; import { SessionStore } from './session-store.ts'; import { noActiveSessionError } from './handlers/response.ts'; @@ -79,8 +80,9 @@ export function createRequestHandler(deps: RequestRouterDeps): DaemonInvokeFn { const { sessionStore, leaseRegistry } = deps; async function handleRequest(req: DaemonRequest): Promise { + const start = Date.now(); const debug = Boolean(req.meta?.debug || req.flags?.verbose); - return await withDiagnosticsScope( + const response = await withDiagnosticsScope( { session: req.session, requestId: req.meta?.requestId, @@ -107,6 +109,13 @@ export function createRequestHandler(deps: RequestRouterDeps): DaemonInvokeFn { } }, ); + // Phase 4 (agent-cost) graft: cost is purely additive and opt-in. With the + // flag off — or on an error response — the serialized DaemonResponse is + // byte-identical to today (Maestro `.ad` recompare diffs it). Mirrors the + // conditional `registerDownloadableArtifacts` spread in request-finalization. + if (!req.meta?.includeCost || !response.ok) return response; + const cost: ResponseCost = { wallClockMs: Date.now() - start }; + return { ok: true, data: { ...(response.data ?? {}), cost } }; } async function executeRequestScope( diff --git a/src/utils/cli-flags.ts b/src/utils/cli-flags.ts index efaf0b3ace..a1389b74ea 100644 --- a/src/utils/cli-flags.ts +++ b/src/utils/cli-flags.ts @@ -65,6 +65,7 @@ export type CliFlags = RemoteConfigMetroOptions & bundleUrl?: string; launchUrl?: string; verbose?: boolean; + cost?: boolean; snapshotInteractiveOnly?: boolean; snapshotDiff?: boolean; snapshotDepth?: number; @@ -775,6 +776,13 @@ const FLAG_DEFINITIONS: readonly FlagDefinition[] = [ usageDescription: 'Enable debug diagnostics; test --verbose prints per-test step timings without debug logs', }, + { + key: 'cost', + names: ['--cost'], + type: 'boolean', + usageLabel: '--cost', + usageDescription: 'Include per-command wall-clock latency (cost.wallClockMs) in the response', + }, { key: 'json', names: ['--json'], @@ -1131,7 +1139,14 @@ export const COMMON_COMMAND_SUPPORTED_FLAG_KEYS = flagKeys( 'noRecord', ); -export const GLOBAL_FLAG_KEYS = new Set(['json', 'config', 'help', 'version', 'verbose']); +export const GLOBAL_FLAG_KEYS = new Set([ + 'json', + 'config', + 'help', + 'version', + 'verbose', + 'cost', +]); const flagDefinitionByName = new Map(); for (const definition of FLAG_DEFINITIONS) { From a12c7af126fe7a316df188f8bfa0744dee4319ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 29 Jun 2026 13:36:40 +0200 Subject: [PATCH 2/2] test: classify --cost flag as outside provider-backed integration The integration progress guard (test:integration:progress:check) treats every public CLI flag as either device-observable (requiring provider-backed coverage) or intentionally excluded. --cost is a diagnostics/output flag (a purely additive response field, not device-observable), so it joins json/help/version/verbose in the 'config, output, diagnostics, and transport' exclusion bucket. --- scripts/integration-progress-model.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/integration-progress-model.ts b/scripts/integration-progress-model.ts index 58ce1034f2..e822abbcd1 100644 --- a/scripts/integration-progress-model.ts +++ b/scripts/integration-progress-model.ts @@ -221,6 +221,7 @@ function summarizeProviderScenarioFlagExclusions() { 'help', 'version', 'verbose', + 'cost', ], }, {