diff --git a/AGENTS.md b/AGENTS.md index f43c8ee52..3e2de45d5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,6 +37,7 @@ Single-context repo. Read `CONTEXT.md` for domain language and testing/architect - Match existing style. Remove imports/variables your change made unused. - Test through public interfaces when possible. Do not add unrelated exports just to make tests easier. - Prefer type-level checks when TypeScript can enforce a contract or invalid shape. +- Use `unknown` only at trust boundaries: parsed JSON, daemon/runtime payloads, catch values, generic I/O, or parser callbacks. Once a value is validated or its producer has a known contract, narrow to a domain type or focused parser/helper instead of carrying `unknown` through internal helper and formatter signatures. - Keep modules small for agent context safety: - target <= 300 LOC per implementation file when practical. - if a file grows past 500 LOC, plan/extract focused submodules before adding new behavior. diff --git a/scripts/write-xcuitest-cache-metadata.mjs b/scripts/write-xcuitest-cache-metadata.mjs index e09e11f17..ea26864b9 100644 --- a/scripts/write-xcuitest-cache-metadata.mjs +++ b/scripts/write-xcuitest-cache-metadata.mjs @@ -362,7 +362,9 @@ function collectConfiguredTestTargets(parsed) { if (!Array.isArray(testConfigurations)) return []; const targets = []; for (const config of testConfigurations) { - if (!isRecord(config) || !Array.isArray(config.TestTargets)) continue; + if (!isRecord(config) || !Array.isArray(config.TestTargets)) { + continue; + } targets.push(...config.TestTargets.filter(isRecord)); } return targets; @@ -373,6 +375,10 @@ function collectLegacyTestTargets(parsed) { return Object.values(parsed).filter((value) => isRecord(value) && 'TestBundlePath' in value); } +function isRecord(value) { + return value !== null && typeof value === 'object' && !Array.isArray(value); +} + function collectXctestrunProductReferenceValuesFromTarget(target) { const values = new Set(); const productReferenceKeys = new Set([ @@ -452,7 +458,3 @@ function readFileSize(filePath) { return null; } } - -function isRecord(value) { - return Boolean(value) && typeof value === 'object' && !Array.isArray(value); -} diff --git a/src/__tests__/client.test.ts b/src/__tests__/client.test.ts index ec7be2cd9..3c8020e49 100644 --- a/src/__tests__/client.test.ts +++ b/src/__tests__/client.test.ts @@ -121,6 +121,47 @@ test('apps.open forwards explicit runtime hints through the daemon request', asy }); }); +test('client close normalizes target shutdown results', async () => { + const setup = createTransport(async () => ({ + ok: true, + data: + setup.calls.length === 1 + ? { + shutdown: { + success: false, + exitCode: -1, + stdout: '', + stderr: 'simctl shutdown failed', + error: { + code: 'COMMAND_FAILED', + message: 'simctl shutdown failed', + details: { retryable: false }, + }, + }, + } + : { + shutdown: { success: true }, + }, + })); + const client = createAgentDeviceClient(setup.config, { transport: setup.transport }); + + const sessionClose = await client.sessions.close({ shutdown: true }); + const appClose = await client.apps.close({ shutdown: true }); + + assert.deepEqual(sessionClose.shutdown, { + success: false, + exitCode: -1, + stdout: '', + stderr: 'simctl shutdown failed', + error: { + code: 'COMMAND_FAILED', + message: 'simctl shutdown failed', + details: { retryable: false }, + }, + }); + assert.equal(appClose.shutdown, undefined); +}); + test('observability.perf projects structured frame area to daemon positionals', async () => { const setup = createTransport(async (req) => { if (req.command === 'perf') { @@ -701,6 +742,53 @@ test('client capture.snapshot forwards force-full as snapshotForceFull flag', as assert.equal(setup.calls[0]?.flags?.snapshotForceFull, true); }); +test('client capture.screenshot normalizes overlay refs from daemon response data', async () => { + const setup = createTransport(async () => ({ + ok: true, + data: { + path: '/tmp/screenshot.png', + overlayRefs: [ + { + ref: '@e1', + label: 'Continue', + rect: { x: 10, y: 20, width: 30, height: 40 }, + overlayRect: { x: 12, y: 22, width: 34, height: 44 }, + center: { x: 25, y: 40 }, + }, + { + ref: '@missing-center', + rect: { x: 1, y: 2, width: 3, height: 4 }, + overlayRect: { x: 1, y: 2, width: 3, height: 4 }, + }, + { + ref: '@array-rect', + rect: [], + overlayRect: { x: 1, y: 2, width: 3, height: 4 }, + center: { x: 2, y: 3 }, + }, + 'not-an-overlay-ref', + ], + }, + })); + const client = createAgentDeviceClient(setup.config, { transport: setup.transport }); + + const result = await client.capture.screenshot({ overlayRefs: true }); + + assert.deepEqual(result, { + path: '/tmp/screenshot.png', + overlayRefs: [ + { + ref: '@e1', + label: 'Continue', + rect: { x: 10, y: 20, width: 30, height: 40 }, + overlayRect: { x: 12, y: 22, width: 34, height: 44 }, + center: { x: 25, y: 40 }, + }, + ], + identifiers: { session: 'qa' }, + }); +}); + test('sessions.stateDir resolves locally without contacting the daemon', async () => { const setup = createTransport(async () => { throw new Error('unexpected daemon call'); diff --git a/src/backend.ts b/src/backend.ts index 1902dcb9b..193f65da8 100644 --- a/src/backend.ts +++ b/src/backend.ts @@ -1,12 +1,6 @@ import type { AlertAction, AlertInfo } from './alert-contract.ts'; import type { AppsFilter } from './contracts/app-inventory.ts'; -import type { - Point, - ScreenshotOverlayRef, - SnapshotNode, - SnapshotOptions, - SnapshotState, -} from './utils/snapshot.ts'; +import type { Point, SnapshotNode, SnapshotOptions, SnapshotState } from './utils/snapshot.ts'; import type { NetworkIncludeMode } from './contracts.ts'; import type { DeviceTarget, Platform, PlatformSelector } from './utils/device.ts'; import type { BackMode } from './core/back-mode.ts'; @@ -22,6 +16,7 @@ import type { SnapshotCaptureAnnotations, SnapshotCaptureFreshness, } from './snapshot-capture-annotations.ts'; +import type { ScreenshotResultData } from './utils/screenshot-result.ts'; export type AgentDeviceBackendPlatform = Platform; @@ -78,10 +73,7 @@ export type BackendScreenshotOptions = { surface?: SessionSurface; }; -export type BackendScreenshotResult = { - path?: string; - overlayRefs?: ScreenshotOverlayRef[]; -}; +export type BackendScreenshotResult = ScreenshotResultData; export type BackendActionResult = Record | void; diff --git a/src/batch.ts b/src/batch.ts index a9b2ec4b7..016a0b03e 100644 --- a/src/batch.ts +++ b/src/batch.ts @@ -11,6 +11,8 @@ export type { BatchFlags, BatchInvoke, BatchRequest, + BatchRunResponse, + BatchRunResult, DaemonBatchStep, BatchStepResult, NormalizedBatchStep, diff --git a/src/cli/batch-steps.ts b/src/cli/batch-steps.ts index 783ea12f6..7c5bc24ea 100644 --- a/src/cli/batch-steps.ts +++ b/src/cli/batch-steps.ts @@ -1,14 +1,16 @@ import type { BatchStep } from '../client-types.ts'; +import { daemonRuntimeSchema, type SessionRuntimeHints } from '../contracts.ts'; import { readInputFromCli } from '../commands/cli-grammar.ts'; import { isCommandName, type CommandName } from '../commands/command-metadata.ts'; import type { CliFlags } from '../utils/cli-flags.ts'; import { AppError } from '../utils/errors.ts'; +import { isRecord } from '../utils/parsing.ts'; type LegacyCliBatchStep = { command: CommandName; positionals?: string[]; flags?: Record; - runtime?: unknown; + runtime?: SessionRuntimeHints; }; export function readCliBatchStepsJson(raw: string): BatchStep[] { @@ -27,7 +29,7 @@ export function readCliBatchStepsJson(raw: string): BatchStep[] { function normalizeCliBatchSteps(steps: unknown[]): BatchStep[] { let sawLegacyStep = false; const normalized = steps.map((step, index) => { - if (isStructuredBatchStep(step)) return step; + if (isStructuredBatchStepShape(step)) return readStructuredBatchStep(step, index + 1); const legacyStep = readLegacyCliBatchStep(step, index + 1); sawLegacyStep = true; return legacyStepToStructuredStep(legacyStep); @@ -53,31 +55,36 @@ function legacyStepToStructuredStep(legacyStep: LegacyCliBatchStep): BatchStep { }; } -function isStructuredBatchStep(step: unknown): step is BatchStep { - return ( - step !== null && - typeof step === 'object' && - !Array.isArray(step) && - 'input' in step && - !('positionals' in step) && - !('flags' in step) - ); +function isStructuredBatchStepShape(step: unknown): step is Record & BatchStep { + return isRecord(step) && 'input' in step && !('positionals' in step) && !('flags' in step); +} + +function readStructuredBatchStep( + step: Record & BatchStep, + stepNumber: number, +): BatchStep { + const runtime = readRuntimeHints(step.runtime, stepNumber); + const { runtime: _runtime, ...rest } = step; + return { + ...rest, + ...(runtime === undefined ? {} : { runtime }), + }; } function readLegacyCliBatchStep(step: unknown, stepNumber: number): LegacyCliBatchStep { - if (!step || typeof step !== 'object' || Array.isArray(step)) { + if (!isRecord(step)) { throw new AppError('INVALID_ARGS', `Invalid batch step ${stepNumber}.`); } - const record = step as Record; - assertLegacyBatchStepKeys(record, stepNumber); - const command = readLegacyCommand(record.command, stepNumber); - const positionals = readLegacyPositionals(record.positionals, stepNumber); - const flags = readLegacyFlags(record.flags, stepNumber); + assertLegacyBatchStepKeys(step, stepNumber); + const command = readLegacyCommand(step.command, stepNumber); + const positionals = readLegacyPositionals(step.positionals, stepNumber); + const flags = readLegacyFlags(step.flags, stepNumber); + const runtime = readRuntimeHints(step.runtime, stepNumber); return { command, ...(positionals === undefined ? {} : { positionals }), ...(flags === undefined ? {} : { flags }), - ...(record.runtime === undefined ? {} : { runtime: record.runtime }), + ...(runtime === undefined ? {} : { runtime }), }; } @@ -116,10 +123,22 @@ function readLegacyPositionals(value: unknown, stepNumber: number): string[] | u function readLegacyFlags(value: unknown, stepNumber: number): Record | undefined { if (value === undefined) return undefined; - if (!value || typeof value !== 'object' || Array.isArray(value)) { + if (!isRecord(value)) { throw new AppError('INVALID_ARGS', `Batch step ${stepNumber} flags must be an object.`); } - return value as Record; + return value; +} + +function readRuntimeHints(value: unknown, stepNumber: number): SessionRuntimeHints | undefined { + if (value === undefined) return undefined; + try { + return daemonRuntimeSchema.parse(value); + } catch (error) { + throw new AppError( + 'INVALID_ARGS', + `Batch step ${stepNumber} runtime is invalid: ${error instanceof Error ? error.message : String(error)}`, + ); + } } function cliFlagsFromBatchStep(flags: Record | undefined): CliFlags { diff --git a/src/client-normalizers.ts b/src/client-normalizers.ts index 0424edcb0..213fa90ab 100644 --- a/src/client-normalizers.ts +++ b/src/client-normalizers.ts @@ -1,8 +1,8 @@ import type { CommandFlags } from './core/dispatch.ts'; import { screenshotFlagsFromOptions } from './contracts/screenshot.ts'; import type { DaemonRequest, SessionRuntimeHints } from './daemon/types.ts'; -import { AppError } from './utils/errors.ts'; -import type { ScreenshotOverlayRef, SnapshotNode } from './utils/snapshot.ts'; +import { AppError, type NormalizedError } from './utils/errors.ts'; +import type { SnapshotNode } from './utils/snapshot.ts'; import { buildAppIdentifiers, buildDeviceIdentifiers } from './client-shared.ts'; import type { AgentDeviceDevice, @@ -13,6 +13,7 @@ import type { InternalRequestOptions, MaterializationReleaseResult, StartupPerfSample, + TargetShutdownResult, } from './client-types.ts'; import { asRecord, @@ -20,8 +21,6 @@ import { readDeviceTarget, readNullableString, readOptionalString, - readPoint, - readRect, readRequiredDeviceKind, readRequiredNumber, readRequiredPlatform, @@ -228,35 +227,44 @@ export function normalizeStartupSample(value: unknown): StartupPerfSample | unde }; } +export function normalizeTargetShutdownResult(value: unknown): TargetShutdownResult | undefined { + if (!isRecord(value)) return undefined; + if ( + typeof value.success !== 'boolean' || + typeof value.exitCode !== 'number' || + typeof value.stdout !== 'string' || + typeof value.stderr !== 'string' + ) { + return undefined; + } + const error = normalizeTargetShutdownError(value.error); + return { + success: value.success, + exitCode: value.exitCode, + stdout: value.stdout, + stderr: value.stderr, + ...(error ? { error } : {}), + }; +} + +function normalizeTargetShutdownError(value: unknown): NormalizedError | undefined { + if (!isRecord(value)) return undefined; + if (typeof value.code !== 'string' || typeof value.message !== 'string') return undefined; + return { + code: value.code, + message: value.message, + ...(typeof value.hint === 'string' ? { hint: value.hint } : {}), + ...(typeof value.diagnosticId === 'string' ? { diagnosticId: value.diagnosticId } : {}), + ...(typeof value.logPath === 'string' ? { logPath: value.logPath } : {}), + ...(isRecord(value.details) ? { details: value.details } : {}), + }; +} + export function readSnapshotNodes(value: unknown): SnapshotNode[] { // Snapshot nodes are produced by the daemon snapshot pipeline and treated as trusted here. return Array.isArray(value) ? (value as SnapshotNode[]) : []; } -export function readScreenshotOverlayRefs( - record: Record, -): ScreenshotOverlayRef[] | undefined { - const value = record.overlayRefs; - if (!Array.isArray(value)) return undefined; - const refs: ScreenshotOverlayRef[] = []; - for (const entry of value) { - if (!isRecord(entry)) continue; - const ref = readOptionalString(entry, 'ref'); - const rect = readRect(entry, 'rect'); - const overlayRect = readRect(entry, 'overlayRect'); - const center = readPoint(entry, 'center'); - if (!ref || !rect || !overlayRect || !center) continue; - refs.push({ - ref, - label: readOptionalString(entry, 'label'), - rect, - overlayRect, - center, - }); - } - return refs; -} - export function buildFlags(options: InternalRequestOptions): CommandFlags { return stripUndefined({ stateDir: options.stateDir, diff --git a/src/client-types.ts b/src/client-types.ts index 4885e47f6..f414478f4 100644 --- a/src/client-types.ts +++ b/src/client-types.ts @@ -28,12 +28,8 @@ import type { ScrollInputDirection } from './commands/interaction/runtime/gestur import type { LogAction } from './contracts/logs.ts'; import type { SessionSurface } from './core/session-surface.ts'; import type { FindLocator } from './utils/finders.ts'; -import type { - ScreenshotOverlayRef, - SnapshotNode, - SnapshotUnchanged, - SnapshotVisibility, -} from './utils/snapshot.ts'; +import type { SnapshotNode, SnapshotUnchanged, SnapshotVisibility } from './utils/snapshot.ts'; +import type { ScreenshotResultData } from './utils/screenshot-result.ts'; import type { MetroPrepareKind, PrepareMetroRuntimeResult, @@ -42,8 +38,11 @@ import type { import type { MetroBridgeScope } from './client-companion-tunnel-contract.ts'; import type { AppsFilter } from './contracts/app-inventory.ts'; import type { ScreenshotRequestFlags } from './contracts/screenshot.ts'; +import type { BatchRunResult, DaemonBatchStep } from './core/batch.ts'; +export type { BatchRunResult } from './core/batch.ts'; +import type { TargetShutdownResult } from './target-shutdown-contract.ts'; +export type { TargetShutdownResult } from './target-shutdown-contract.ts'; import type { PerfAction, PerfArea, PerfKind, PerfSubject } from './contracts/perf.ts'; -import type { DaemonBatchStep } from './core/batch.ts'; import type { AlertAction, AlertInfo } from './alert-contract.ts'; import type { DebugSymbolsOptions, DebugSymbolsResult } from './contracts/debug-symbols.ts'; @@ -173,7 +172,7 @@ export type StartupPerfSample = { export type SessionCloseResult = { session: string; - shutdown?: Record; + shutdown?: TargetShutdownResult; identifiers: AgentDeviceIdentifiers; }; @@ -230,7 +229,7 @@ export type AppCloseOptions = AgentDeviceRequestOverrides & { export type AppCloseResult = { session: string; closedApp?: string; - shutdown?: Record; + shutdown?: TargetShutdownResult; identifiers: AgentDeviceIdentifiers; }; @@ -357,9 +356,8 @@ export type CaptureScreenshotOptions = AgentDeviceRequestOverrides & { surface?: SessionSurface; }; -export type CaptureScreenshotResult = { +export type CaptureScreenshotResult = ScreenshotResultData & { path: string; - overlayRefs?: ScreenshotOverlayRef[]; identifiers: AgentDeviceIdentifiers; }; @@ -749,7 +747,7 @@ export type ReplayTestOptions = AgentDeviceRequestOverrides & export type BatchStep = { command: string; input: Record; - runtime?: unknown; + runtime?: SessionRuntimeHints; }; export type BatchRunOptions = AgentDeviceRequestOverrides & { @@ -1001,7 +999,7 @@ export type AgentDeviceClient = { test: (options: ReplayTestOptions) => Promise; }; batch: { - run: (options: BatchRunOptions) => Promise; + run: (options: BatchRunOptions) => Promise; }; observability: { perf: (options?: PerfOptions) => Promise; diff --git a/src/client.ts b/src/client.ts index efca6d455..6b6b95826 100644 --- a/src/client.ts +++ b/src/client.ts @@ -16,15 +16,17 @@ import { normalizeInstallFromSourceResult, normalizeMaterializationReleaseResult, normalizeOpenDevice, - readScreenshotOverlayRefs, normalizeRuntimeHints, normalizeSession, normalizeStartupSample, + normalizeTargetShutdownResult, readOptionalString, readRequiredString, readSnapshotNodes, resolveSessionName, } from './client-normalizers.ts'; +import { readScreenshotResultData } from './utils/screenshot-result.ts'; +import { isRecord } from './utils/parsing.ts'; import type { AgentDeviceClient, AgentDeviceClientConfig, @@ -130,13 +132,9 @@ export function createAgentDeviceClient( close: async (options = {}) => { const session = resolveRequestSession(options); const data = await executeCommand>('close', options); - const shutdown = data.shutdown; return { session, - shutdown: - typeof shutdown === 'object' && shutdown !== null - ? (shutdown as Record) - : undefined, + shutdown: normalizeTargetShutdownResult(data.shutdown), identifiers: { session }, }; }, @@ -192,14 +190,10 @@ export function createAgentDeviceClient( close: async (options: AppCloseOptions = {}) => { const session = resolveRequestSession(options); const data = await executeCommand>('close', options); - const shutdown = data.shutdown; return { session, closedApp: options.app, - shutdown: - typeof shutdown === 'object' && shutdown !== null - ? (shutdown as Record) - : undefined, + shutdown: normalizeTargetShutdownResult(data.shutdown), identifiers: { session }, }; }, @@ -276,9 +270,10 @@ export function createAgentDeviceClient( screenshot: async (options: CaptureScreenshotOptions = {}) => { const session = resolveRequestSession(options); const data = await executeCommand>('screenshot', options); + const screenshot = readScreenshotResultData(data); return { path: readRequiredString(data, 'path'), - overlayRefs: readScreenshotOverlayRefs(data), + overlayRefs: screenshot?.overlayRefs, identifiers: { session }, }; }, @@ -373,9 +368,7 @@ function optionalSnapshotResponseFields( } function readObject(value: unknown): Record | undefined { - return typeof value === 'object' && value !== null - ? (value as Record) - : undefined; + return isRecord(value) ? value : undefined; } function mergeClientOptions( @@ -387,18 +380,17 @@ function mergeClientOptions( function normalizeLease(data: Record): Lease { const rawLease = data.lease; - if (!rawLease || typeof rawLease !== 'object' || Array.isArray(rawLease)) { + if (!isRecord(rawLease)) { throw new Error('Invalid lease response from daemon'); } - const lease = rawLease as Record; return { - leaseId: readRequiredString(lease, 'leaseId'), - tenantId: readRequiredString(lease, 'tenantId'), - runId: readRequiredString(lease, 'runId'), - backend: readRequiredString(lease, 'backend') as Lease['backend'], - createdAt: typeof lease.createdAt === 'number' ? lease.createdAt : undefined, - heartbeatAt: typeof lease.heartbeatAt === 'number' ? lease.heartbeatAt : undefined, - expiresAt: typeof lease.expiresAt === 'number' ? lease.expiresAt : undefined, + leaseId: readRequiredString(rawLease, 'leaseId'), + tenantId: readRequiredString(rawLease, 'tenantId'), + runId: readRequiredString(rawLease, 'runId'), + backend: readRequiredString(rawLease, 'backend') as Lease['backend'], + createdAt: typeof rawLease.createdAt === 'number' ? rawLease.createdAt : undefined, + heartbeatAt: typeof rawLease.heartbeatAt === 'number' ? rawLease.heartbeatAt : undefined, + expiresAt: typeof rawLease.expiresAt === 'number' ? rawLease.expiresAt : undefined, }; } diff --git a/src/commands/batch/cli.test.ts b/src/commands/batch/cli.test.ts index 5a1761285..3b324145f 100644 --- a/src/commands/batch/cli.test.ts +++ b/src/commands/batch/cli.test.ts @@ -13,7 +13,7 @@ import { const batchDefaultResponse: DaemonResponse = { ok: true, - data: { total: 1, executed: 1, totalDurationMs: 1 }, + data: { total: 0, executed: 0, totalDurationMs: 1, results: [] }, }; function runCliCapture( @@ -120,6 +120,19 @@ test('batch rejects structured replay steps before daemon dispatch', async () => assert.match(result.stderr, /not available through command batch/); }); +test('batch rejects invalid structured runtime without falling back to legacy parsing', async () => { + const result = await runCliCapture([ + 'batch', + '--steps', + '[{"command":"open","input":{"app":"settings"},"runtime":null}]', + ]); + + assert.equal(result.code, 1); + assert.equal(result.calls.length, 0); + assert.match(result.stderr, /Batch step 1 runtime is invalid/); + assert.doesNotMatch(result.stderr, /unknown legacy field\(s\): input/); +}); + test('batch accepts legacy positionals/flags steps with deprecation warning', async () => { const result = await runCliCapture([ 'batch', @@ -300,37 +313,3 @@ test('batch human output renders per-step results', async () => { assert.match(result.stdout, /1\. OK Opened: Settings \(7ms\)/); assert.match(result.stdout, /2\. OK Typed 5 chars \(8ms\)/); }); - -test('batch human output renders failed steps distinctly', async () => { - const result = await runCliCapture( - ['batch', '--steps', '[{"command":"open","input":{}}]'], - async () => ({ - ok: true, - data: { - total: 2, - executed: 1, - totalDurationMs: 15, - results: [ - { - step: 1, - command: 'open', - ok: true, - data: { appName: 'Settings', message: 'Opened: Settings' }, - durationMs: 7, - }, - { - step: 2, - command: 'type', - ok: false, - error: { message: 'type requires text' }, - durationMs: 8, - }, - ], - }, - }), - ); - - assert.equal(result.code, null); - assert.match(result.stdout, /1\. OK Opened: Settings \(7ms\)/); - assert.match(result.stdout, /2\. FAILED type requires text \(8ms\)/); -}); diff --git a/src/commands/batch/metadata.ts b/src/commands/batch/metadata.ts index 7c439c6b5..1902ab3a0 100644 --- a/src/commands/batch/metadata.ts +++ b/src/commands/batch/metadata.ts @@ -1,4 +1,5 @@ import { DEFAULT_BATCH_MAX_STEPS } from '../../batch-contract.ts'; +import { daemonRuntimeSchema, type SessionRuntimeHints } from '../../contracts.ts'; import { STRUCTURED_BATCH_COMMAND_NAMES, readStructuredBatchCommandName, @@ -20,11 +21,12 @@ import { type CommandFieldMap, type InferCommandInput, } from '../command-input.ts'; +import { isRecord } from '../../utils/parsing.ts'; export type BatchCommandStep = { command: string; input: Record; - runtime?: unknown; + runtime?: SessionRuntimeHints; }; export type BatchInput = InferCommandInput & { @@ -145,18 +147,18 @@ function readBatchStepCommand( } function readBatchStepRecord(step: unknown, stepNumber: number): Record { - if (!step || typeof step !== 'object' || Array.isArray(step)) { + if (!isRecord(step)) { throw new Error(`Invalid batch step ${stepNumber}.`); } - return step as Record; + return step; } function readBatchStepInput(record: Record, stepNumber: number) { const input = record.input; - if (!input || typeof input !== 'object' || Array.isArray(input)) { + if (!isRecord(input)) { throw new Error(`Batch step ${stepNumber} input must be an object.`); } - return input as Record; + return input; } function readBatchStepRuntimeProperty( @@ -164,11 +166,12 @@ function readBatchStepRuntimeProperty( stepNumber: number, ): Pick { const runtime = record.runtime; - if ( - runtime !== undefined && - (!runtime || typeof runtime !== 'object' || Array.isArray(runtime)) - ) { - throw new Error(`Batch step ${stepNumber} runtime must be an object.`); + if (runtime === undefined) return {}; + try { + return { runtime: daemonRuntimeSchema.parse(runtime) }; + } catch (error) { + throw new Error( + `Batch step ${stepNumber} runtime is invalid: ${error instanceof Error ? error.message : String(error)}`, + ); } - return runtime === undefined ? {} : { runtime }; } diff --git a/src/commands/batch/output.ts b/src/commands/batch/output.ts index f8e762b4f..f6b9448ae 100644 --- a/src/commands/batch/output.ts +++ b/src/commands/batch/output.ts @@ -1,50 +1,23 @@ -import type { CommandRequestResult } from '../../client-types.ts'; +import type { BatchRunResult, BatchStepResult } from '../../core/batch.ts'; import { readCommandMessage } from '../../utils/success-text.ts'; import type { CliOutput } from '../command-contract.ts'; -import { readRecord, resultOutput, type CliOutputFormatter } from '../output-common.ts'; +import { resultOutput, type CliOutputFormatter } from '../output-common.ts'; -function batchCliOutput(result: CommandRequestResult): CliOutput { - const data = result as Record; - const total = typeof data.total === 'number' ? data.total : 0; - const executed = typeof data.executed === 'number' ? data.executed : 0; - const durationMs = typeof data.totalDurationMs === 'number' ? data.totalDurationMs : undefined; +function batchCliOutput(result: BatchRunResult): CliOutput { const lines = [ - `Batch completed: ${executed}/${total} steps${durationMs !== undefined ? ` in ${durationMs}ms` : ''}`, + `Batch completed: ${result.executed}/${result.total} steps in ${result.totalDurationMs}ms`, ]; - const results = Array.isArray(data.results) ? data.results : []; - for (const entry of results) { - const line = renderBatchStepLine(entry); - if (line) lines.push(line); + for (const entry of result.results) { + lines.push(renderBatchStepLine(entry)); } - return { data, text: lines.join('\n') }; + return { data: result, text: lines.join('\n') }; } export const batchCliOutputFormatters = { - batch: resultOutput(batchCliOutput), + batch: resultOutput(batchCliOutput), } as const satisfies Record; -function renderBatchStepLine(entry: unknown): string | undefined { - const result = readRecord(entry); - if (!result) return undefined; - const step = typeof result.step === 'number' ? result.step : undefined; - const command = typeof result.command === 'string' ? result.command : 'step'; - const stepOk = result.ok !== false; - const description = readBatchStepDescription(result, stepOk, command); - const prefix = step !== undefined ? `${step}. ` : '- '; - const durationMs = typeof result.durationMs === 'number' ? result.durationMs : undefined; - const durationSuffix = durationMs !== undefined ? ` (${durationMs}ms)` : ''; - return `${prefix}${stepOk ? 'OK' : 'FAILED'} ${description}${durationSuffix}`; -} - -function readBatchStepDescription( - result: Record, - stepOk: boolean, - command: string, -): string { - if (stepOk) return readCommandMessage(readRecord(result.data)) ?? command; - return readBatchStepFailure(readRecord(result.error)) ?? command; -} - -function readBatchStepFailure(error: Record | undefined): string | null { - return typeof error?.message === 'string' && error.message.length > 0 ? error.message : null; +function renderBatchStepLine(result: BatchStepResult): string { + const description = readCommandMessage(result.data) ?? result.command; + return `${result.step}. OK ${description} (${result.durationMs}ms)`; } diff --git a/src/commands/batch/projection.ts b/src/commands/batch/projection.ts index b124b7c15..6a96bbb45 100644 --- a/src/commands/batch/projection.ts +++ b/src/commands/batch/projection.ts @@ -1,10 +1,12 @@ import { PUBLIC_COMMANDS } from '../../command-catalog.ts'; +import { daemonRuntimeSchema, type DaemonRequest } from '../../contracts.ts'; import { STRUCTURED_BATCH_COMMAND_NAMES, readStructuredBatchCommandName, } from '../../batch-policy.ts'; import type { DaemonBatchStep } from '../../core/batch.ts'; import { AppError } from '../../utils/errors.ts'; +import { isRecord } from '../../utils/parsing.ts'; import { request } from '../cli-grammar/common.ts'; import type { CommandInput, DaemonCommandRequest, DaemonWriter } from '../cli-grammar/types.ts'; import { buildRequestFlags } from '../command-flags.ts'; @@ -63,10 +65,10 @@ function readBatchDaemonStep( } function readBatchStepRecord(step: unknown, stepNumber: number): Record { - if (!step || typeof step !== 'object' || Array.isArray(step)) { + if (!isRecord(step)) { throw new AppError('INVALID_ARGS', `Invalid batch step ${stepNumber}.`); } - return step as Record; + return step; } function readBatchStepCommand( @@ -78,7 +80,7 @@ function readBatchStepCommand( function readBatchStepInput(record: Record, stepNumber: number): CommandInput { const input = record.input; - if (!input || typeof input !== 'object' || Array.isArray(input)) { + if (!isRecord(input)) { throw new AppError('INVALID_ARGS', `Batch step ${stepNumber} input must be an object.`); } return input as CommandInput; @@ -87,13 +89,15 @@ function readBatchStepInput(record: Record, stepNumber: number) function readBatchStepRuntime( record: Record, stepNumber: number, -): Record | undefined { +): DaemonRequest['runtime'] { const runtime = record.runtime; - if ( - runtime !== undefined && - (!runtime || typeof runtime !== 'object' || Array.isArray(runtime)) - ) { - throw new AppError('INVALID_ARGS', `Batch step ${stepNumber} runtime must be an object.`); + if (runtime === undefined) return undefined; + try { + return daemonRuntimeSchema.parse(runtime); + } catch (error) { + throw new AppError( + 'INVALID_ARGS', + `Batch step ${stepNumber} runtime is invalid: ${error instanceof Error ? error.message : String(error)}`, + ); } - return runtime as Record | undefined; } diff --git a/src/commands/batch/public.test.ts b/src/commands/batch/public.test.ts index 3a1f75dee..8ead6f9fe 100644 --- a/src/commands/batch/public.test.ts +++ b/src/commands/batch/public.test.ts @@ -7,7 +7,6 @@ import { buildBatchStepFlags, runBatch, validateAndNormalizeBatchSteps, - type BatchStepResult, } from '../../batch.ts'; import type { DaemonRequest } from '../../contracts.ts'; @@ -43,9 +42,8 @@ test('public batch entrypoint exports daemon-compatible orchestration helpers', assert.equal(response.ok, true); assert.deepEqual(seenCommands, ['open', 'wait']); if (response.ok) { - assert.equal(response.data?.total, 2); - const results = response.data?.results as BatchStepResult[]; - assert.equal(results[0]?.command, 'open'); + assert.equal(response.data.total, 2); + assert.equal(response.data.results[0]?.command, 'open'); } }); diff --git a/src/commands/observability/output.ts b/src/commands/observability/output.ts index 430f0bb8e..a6cdb6169 100644 --- a/src/commands/observability/output.ts +++ b/src/commands/observability/output.ts @@ -1,28 +1,72 @@ -import type { CommandRequestResult } from '../../client-types.ts'; +import type { BackendNetworkEntry } from '../../backend.ts'; +import type { NetworkIncludeMode } from '../../contracts.ts'; +import type { NetworkEntry } from '../../daemon/network-log.ts'; import type { CliOutput } from '../command-contract.ts'; -import { readRecord, resultOutput, type CliOutputFormatter } from '../output-common.ts'; +import { resultOutput, type CliOutputFormatter } from '../output-common.ts'; -function logsCliOutput(result: CommandRequestResult): CliOutput { - const data = result as Record; - const pathOut = typeof data.path === 'string' ? data.path : ''; +type LogsActionFields = { + started?: true; + stopped?: true; + marked?: true; + cleared?: true; + restarted?: true; + removedRotatedFiles?: number; +}; + +type LogsCliResult = LogsActionFields & { + path: string; + active?: boolean; + state?: string; + backend?: string; + sizeBytes?: number; + hint?: string; + notes?: readonly string[]; +}; + +const LOG_ACTION_FIELD_KEYS = [ + 'started', + 'stopped', + 'marked', + 'cleared', + 'restarted', + 'removedRotatedFiles', +] as const satisfies readonly (keyof LogsActionFields)[]; + +type NetworkCliEntry = (BackendNetworkEntry | NetworkEntry) & { + headers?: string; + requestHeaders?: Record; + responseHeaders?: Record; +}; + +type NetworkCliResult = { + path?: string; + active?: boolean; + state?: string; + backend?: string; + include?: NetworkIncludeMode; + scannedLines?: number; + matchedLines?: number; + entries?: readonly NetworkCliEntry[]; + notes?: readonly string[]; +}; + +function logsCliOutput(data: LogsCliResult): CliOutput { return { data, - text: pathOut, + text: data.path, stderr: joinDefinedLines([ - formatKeyValueFields(data, ['active', 'state', 'backend', 'sizeBytes']), + formatKeyValueFields(data, ['active', 'state', 'backend', 'sizeBytes'] as const), formatActionFields(data), - typeof data.hint === 'string' ? data.hint : undefined, + data.hint, formatNotes(data.notes), ]), }; } -function networkCliOutput(result: CommandRequestResult): CliOutput { - const data = result as Record; +function networkCliOutput(data: NetworkCliResult): CliOutput { const lines: string[] = []; - const pathOut = typeof data.path === 'string' ? data.path : ''; - if (pathOut) lines.push(pathOut); - const entries = Array.isArray(data.entries) ? data.entries : []; + const entries = data.entries ?? []; + if (data.path) lines.push(data.path); if (entries.length === 0) { lines.push('No recent HTTP(s) entries found.'); } else { @@ -41,62 +85,64 @@ function networkCliOutput(result: CommandRequestResult): CliOutput { 'include', 'scannedLines', 'matchedLines', - ]), + ] as const), formatNotes(data.notes), ]), }; } export const observabilityCliOutputFormatters = { - logs: resultOutput(logsCliOutput), - network: resultOutput(networkCliOutput), + logs: resultOutput(logsCliOutput), + network: resultOutput(networkCliOutput), } as const satisfies Record; -function formatActionFields(data: Record): string | undefined { +function formatActionFields(data: LogsActionFields): string | undefined { return ( - ['started', 'stopped', 'marked', 'cleared', 'restarted', 'removedRotatedFiles'] - .map((key) => formatActionField(key, data[key])) + LOG_ACTION_FIELD_KEYS.map((key) => formatActionField(key, data[key])) .filter(Boolean) .join(' ') || undefined ); } -function formatActionField(key: string, value: unknown): string { - if (value === true) return `${key}=true`; - return typeof value === 'number' ? `${key}=${value}` : ''; +function formatActionField(key: string, value: true | number | null | undefined): string { + return value == null ? '' : `${key}=${value}`; } -function formatNetworkEntry(entry: unknown): string[] { - const record = readRecord(entry) ?? {}; - const method = typeof record.method === 'string' ? record.method : 'HTTP'; - const url = typeof record.url === 'string' ? record.url : ''; - const status = typeof record.status === 'number' ? ` status=${record.status}` : ''; - const timestamp = typeof record.timestamp === 'string' ? `${record.timestamp} ` : ''; - const durationMs = - typeof record.durationMs === 'number' ? ` durationMs=${record.durationMs}` : ''; +function formatNetworkEntry(entry: NetworkCliEntry): string[] { + const method = entry.method ?? 'HTTP'; + const url = entry.url ?? ''; + const status = entry.status !== undefined ? ` status=${entry.status}` : ''; + const timestamp = entry.timestamp ? `${entry.timestamp} ` : ''; + const durationMs = entry.durationMs !== undefined ? ` durationMs=${entry.durationMs}` : ''; const lines = [`${timestamp}${method} ${url}${status}${durationMs}`]; - const hasFormattedHeaders = typeof record.headers === 'string'; - appendNetworkEntryBody(lines, 'headers', record.headers); - if (!hasFormattedHeaders) { - appendNetworkEntryHeaders(lines, 'request headers', record.requestHeaders); - appendNetworkEntryHeaders(lines, 'response headers', record.responseHeaders); + if (entry.headers) { + appendNetworkEntryBody(lines, 'headers', entry.headers); + } else { + appendNetworkEntryHeaders(lines, 'request headers', entry.requestHeaders); + appendNetworkEntryHeaders(lines, 'response headers', entry.responseHeaders); } - appendNetworkEntryBody(lines, 'request', record.requestBody); - appendNetworkEntryBody(lines, 'response', record.responseBody); + appendNetworkEntryBody(lines, 'request', entry.requestBody); + appendNetworkEntryBody(lines, 'response', entry.responseBody); return lines; } -function appendNetworkEntryHeaders(lines: string[], label: string, value: unknown): void { - const headers = readRecord(value); +function appendNetworkEntryHeaders( + lines: string[], + label: string, + headers: Record | undefined, +): void { if (!headers || Object.keys(headers).length === 0) return; lines.push(` ${label}: ${JSON.stringify(headers)}`); } -function appendNetworkEntryBody(lines: string[], label: string, value: unknown): void { - if (typeof value === 'string') lines.push(` ${label}: ${value}`); +function appendNetworkEntryBody(lines: string[], label: string, value: string | undefined): void { + if (value !== undefined) lines.push(` ${label}: ${value}`); } -function formatKeyValueFields(data: Record, fields: string[]): string | undefined { +function formatKeyValueFields>( + data: T, + fields: readonly K[], +): string | undefined { const text = fields .map((key) => (data[key] !== undefined && data[key] !== null ? `${key}=${data[key]}` : '')) .filter(Boolean) @@ -104,9 +150,8 @@ function formatKeyValueFields(data: Record, fields: string[]): return text || undefined; } -function formatNotes(notes: unknown): string | undefined { - if (!Array.isArray(notes)) return undefined; - const lines = notes.filter((note): note is string => typeof note === 'string' && note.length > 0); +function formatNotes(notes: readonly string[] | undefined): string | undefined { + const lines = notes?.filter((note) => note.length > 0) ?? []; return lines.length > 0 ? lines.join('\n') : undefined; } diff --git a/src/commands/output-common.ts b/src/commands/output-common.ts index 42ff2e337..e8dbe6c28 100644 --- a/src/commands/output-common.ts +++ b/src/commands/output-common.ts @@ -17,17 +17,3 @@ export const messageOutput = resultOutput(messageCliOutput); export function messageCliOutput(result: Record): CliOutput { return { data: result, text: readCommandMessage(result) }; } - -export function readRecord(value: unknown): Record | undefined { - return value && typeof value === 'object' && !Array.isArray(value) - ? (value as Record) - : undefined; -} - -export function readRecordArray(value: unknown): Array> { - return Array.isArray(value) ? value.filter(isRecord) : []; -} - -function isRecord(value: unknown): value is Record { - return Boolean(value) && typeof value === 'object' && !Array.isArray(value); -} diff --git a/src/commands/perf/output.ts b/src/commands/perf/output.ts index bb944da16..ec39cc40e 100644 --- a/src/commands/perf/output.ts +++ b/src/commands/perf/output.ts @@ -1,11 +1,7 @@ import type { CommandRequestResult } from '../../client-types.ts'; +import { isRecord } from '../../utils/parsing.ts'; import type { CliOutput } from '../command-contract.ts'; -import { - readRecord, - readRecordArray, - resultOutput, - type CliOutputFormatter, -} from '../output-common.ts'; +import { resultOutput, type CliOutputFormatter } from '../output-common.ts'; function perfCliOutput(result: CommandRequestResult): CliOutput { const data = result as Record; @@ -19,12 +15,16 @@ export const perfCliOutputFormatters = { function formatPerfCliOutput(data: Record): string { const nativeOutput = formatNativePerfOutput(data); if (nativeOutput) return nativeOutput; - const artifact = readRecord(data.artifact); + const artifact = isRecord(data.artifact) ? data.artifact : undefined; if (artifact) { return formatMemoryArtifactSummary(artifact); } - const metrics = readRecord(data.metrics); - const fps = readRecord(metrics?.fps); + const metrics = isRecord(data.metrics) ? data.metrics : undefined; + return formatFramePerfOutput(metrics); +} + +function formatFramePerfOutput(metrics: Record | undefined): string { + const fps = isRecord(metrics?.fps) ? metrics.fps : undefined; const resourceSummary = buildResourcePerfSummary(metrics); if (!fps) { return formatPerfUnavailable(resourceSummary, 'missing frame metric'); @@ -37,6 +37,10 @@ function formatPerfCliOutput(data: Record): string { const frameSummary = formatFrameHealthSummary(fps); if (!frameSummary) return formatPerfUnavailable(resourceSummary, 'missing dropped-frame summary'); + return formatFrameHealthOutput(fps, frameSummary); +} + +function formatFrameHealthOutput(fps: Record, frameSummary: string): string { const lines = [`Frame health: ${frameSummary}`]; const worstWindows = formatWorstFrameWindows(fps); if (worstWindows.length > 0) { @@ -125,8 +129,8 @@ function readString(value: unknown): string | undefined { } function formatNativePerfFrameHealth(data: Record): string { - const summary = readRecord(data.summary); - const frameHealth = readRecord(summary?.frameHealth); + const summary = isRecord(data.summary) ? data.summary : undefined; + const frameHealth = isRecord(summary?.frameHealth) ? summary.frameHealth : undefined; if (!frameHealth || frameHealth.available !== true) return ''; const droppedFramePercent = readFiniteNumber(frameHealth.droppedFramePercent); const droppedFrameCount = readFiniteNumber(frameHealth.droppedFrameCount); @@ -177,8 +181,10 @@ function formatSampleWindow(sampleWindowMs: number | undefined): string { } function formatWorstFrameWindows(fps: Record): string[] { - return readRecordArray(fps.worstWindows).flatMap((window) => { - const line = formatWorstFrameWindow(window); + if (!Array.isArray(fps.worstWindows)) return []; + return fps.worstWindows.flatMap((entry) => { + if (!isRecord(entry)) return []; + const line = formatWorstFrameWindow(entry); return line ? [line] : []; }); } @@ -199,10 +205,11 @@ function formatWorstFrameWindow(window: Record): string | undef function buildResourcePerfSummary( metrics: Record | undefined, ): string | undefined { - const parts = [ - formatCpuPerfSummary(readRecord(metrics?.cpu)), - formatMemoryPerfSummary(readRecord(metrics?.memory)), - ].filter((part): part is string => Boolean(part)); + const cpu = isRecord(metrics?.cpu) ? metrics.cpu : undefined; + const memory = isRecord(metrics?.memory) ? metrics.memory : undefined; + const parts = [formatCpuPerfSummary(cpu), formatMemoryPerfSummary(memory)].filter( + (part): part is string => Boolean(part), + ); return parts.length > 0 ? parts.join(', ') : undefined; } diff --git a/src/core/__tests__/batch.test.ts b/src/core/__tests__/batch.test.ts index 37dcbdaf6..d61676445 100644 --- a/src/core/__tests__/batch.test.ts +++ b/src/core/__tests__/batch.test.ts @@ -25,3 +25,14 @@ test('validateAndNormalizeBatchSteps blocks replay daemon steps', () => { /cannot run replay/i, ); }); + +test('validateAndNormalizeBatchSteps validates runtime hints', () => { + assert.throws( + () => + validateAndNormalizeBatchSteps( + [{ command: 'open', runtime: { platform: 'web' } } as unknown as DaemonBatchStep], + 10, + ), + /runtime is invalid/i, + ); +}); diff --git a/src/core/batch.ts b/src/core/batch.ts index 0cb8ea7d3..34b98cc31 100644 --- a/src/core/batch.ts +++ b/src/core/batch.ts @@ -1,5 +1,6 @@ -import type { DaemonRequest, DaemonResponse } from '../contracts.ts'; +import { daemonRuntimeSchema, type DaemonRequest, type DaemonResponse } from '../contracts.ts'; import { AppError, asAppError } from '../utils/errors.ts'; +import { isRecord } from '../utils/parsing.ts'; import { DEFAULT_BATCH_MAX_STEPS } from '../batch-contract.ts'; import { BATCH_BLOCKED_COMMANDS, @@ -18,7 +19,7 @@ export type DaemonBatchStep = { command: string; positionals?: string[]; flags?: Record; - runtime?: unknown; + runtime?: DaemonRequest['runtime']; }; export type BatchFlags = Record & { @@ -37,7 +38,7 @@ export type NormalizedBatchStep = { command: string; positionals: string[]; flags: Record; - runtime?: unknown; + runtime?: DaemonRequest['runtime']; }; export type BatchStepResult = { @@ -48,11 +49,25 @@ export type BatchStepResult = { durationMs: number; }; +export type BatchRunResult = Record & { + total: number; + executed: number; + totalDurationMs: number; + results: BatchStepResult[]; +}; + +export type BatchRunResponse = + | { + ok: true; + data: BatchRunResult; + } + | Extract; + export async function runBatch( req: BatchRequest, sessionName: string, invoke: BatchInvoke, -): Promise { +): Promise { const flags = readBatchFlags(req.flags); const batchOnError = flags?.batchOnError ?? 'stop'; if (batchOnError !== 'stop') { @@ -94,14 +109,15 @@ export async function runBatch( } partialResults.push(stepResponse.result); } + const data: BatchRunResult = { + total: steps.length, + executed: steps.length, + totalDurationMs: Date.now() - startedAt, + results: partialResults, + }; return { ok: true, - data: { - total: steps.length, - executed: steps.length, - totalDurationMs: Date.now() - startedAt, - results: partialResults, - }, + data, }; } catch (error) { const appErr = asAppError(error); @@ -125,8 +141,8 @@ export function validateAndNormalizeBatchSteps( const normalized: NormalizedBatchStep[] = []; for (let index = 0; index < steps.length; index += 1) { - const step = steps[index] as Partial; - if (!step || typeof step !== 'object') { + const step = steps[index]; + if (!isRecord(step)) { throw new AppError('INVALID_ARGS', `Invalid batch step at index ${index}.`); } const unknownKeys = Object.keys(step).filter((key) => !batchAllowedStepKeys.has(key)); @@ -152,28 +168,31 @@ export function validateAndNormalizeBatchSteps( `Batch step ${index + 1} positionals must contain only strings.`, ); } - if ( - step.flags !== undefined && - (typeof step.flags !== 'object' || Array.isArray(step.flags) || !step.flags) - ) { + if (step.flags !== undefined && !isRecord(step.flags)) { throw new AppError('INVALID_ARGS', `Batch step ${index + 1} flags must be an object.`); } - if ( - step.runtime !== undefined && - (typeof step.runtime !== 'object' || Array.isArray(step.runtime) || !step.runtime) - ) { - throw new AppError('INVALID_ARGS', `Batch step ${index + 1} runtime must be an object.`); - } normalized.push({ command, positionals: positionals as string[], flags: (step.flags ?? {}) as Record, - runtime: step.runtime, + runtime: readBatchStepRuntime(step.runtime, index + 1), }); } return normalized; } +function readBatchStepRuntime(value: unknown, stepNumber: number): DaemonRequest['runtime'] { + if (value === undefined) return undefined; + try { + return daemonRuntimeSchema.parse(value); + } catch (error) { + throw new AppError( + 'INVALID_ARGS', + `Batch step ${stepNumber} runtime is invalid: ${error instanceof Error ? error.message : String(error)}`, + ); + } +} + export function buildBatchStepFlags( parentFlags: BatchFlags | Record | undefined, stepFlags: DaemonBatchStep['flags'] | Record | undefined, @@ -233,7 +252,7 @@ async function runBatchStep( command: step.command, positionals: step.positionals, flags: stepFlags, - runtime: (step.runtime === undefined ? req.runtime : step.runtime) as DaemonRequest['runtime'], + runtime: step.runtime === undefined ? req.runtime : step.runtime, meta: req.meta, }); const durationMs = Date.now() - stepStartedAt; diff --git a/src/daemon/screenshot-runtime.ts b/src/daemon/screenshot-runtime.ts index ca5520330..4cb950e09 100644 --- a/src/daemon/screenshot-runtime.ts +++ b/src/daemon/screenshot-runtime.ts @@ -1,12 +1,13 @@ import { promises as fs } from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import type { AgentDeviceBackend, BackendScreenshotResult } from '../backend.ts'; +import type { AgentDeviceBackend } from '../backend.ts'; import type { ArtifactAdapter } from '../io.ts'; import { createAgentDevice, localCommandPolicy } from '../runtime.ts'; import { dispatchCommand } from '../core/dispatch.ts'; import { screenshotFlagsFromOptions, screenshotOptionsFromFlags } from '../contracts/screenshot.ts'; import { AppError } from '../utils/errors.ts'; +import { readScreenshotResultData } from '../utils/screenshot-result.ts'; import type { DaemonCommandContext } from './context.ts'; import type { SessionState } from './types.ts'; import { createDaemonRuntimeSessionStore } from './runtime-session.ts'; @@ -58,28 +59,17 @@ function createDispatchScreenshotBackend(params: { surface: options?.surface, }; if (outputPlacement === 'out') { - return toBackendScreenshotResult( + return readScreenshotResultData( await dispatchCommand(session.device, 'screenshot', [], outPath, context), ); } - return toBackendScreenshotResult( + return readScreenshotResultData( await dispatchCommand(session.device, 'screenshot', [outPath], undefined, context), ); }, }; } -function toBackendScreenshotResult(data: unknown): BackendScreenshotResult | void { - if (typeof data !== 'object' || data === null) return; - const record = data as Record; - return { - ...(typeof record.path === 'string' ? { path: record.path } : {}), - ...(Array.isArray(record.overlayRefs) - ? { overlayRefs: record.overlayRefs as NonNullable } - : {}), - }; -} - function createDaemonScreenshotArtifactAdapter(): ArtifactAdapter { return { resolveInput: async () => { diff --git a/src/daemon/target-shutdown.ts b/src/daemon/target-shutdown.ts index e4a20c2c7..eb8ce5e07 100644 --- a/src/daemon/target-shutdown.ts +++ b/src/daemon/target-shutdown.ts @@ -1,16 +1,11 @@ import { runAndroidAdb } from '../platforms/android/adb.ts'; import { getSimulatorState, shutdownSimulator } from '../platforms/ios/simulator.ts'; +import type { TargetShutdownResult } from '../target-shutdown-contract.ts'; import type { DeviceInfo } from '../utils/device.ts'; import { normalizeError } from '../utils/errors.ts'; import { isAndroidEmulator, isIosSimulator } from './device-targets.ts'; -export type DeviceTargetShutdownResult = { - success: boolean; - exitCode: number; - stdout: string; - stderr: string; - error?: ReturnType; -}; +export type DeviceTargetShutdownResult = TargetShutdownResult; export function canShutdownDeviceTarget(device: DeviceInfo): boolean { return isIosSimulator(device) || isAndroidEmulator(device); diff --git a/src/index.ts b/src/index.ts index 907950b94..c5c188869 100644 --- a/src/index.ts +++ b/src/index.ts @@ -54,6 +54,7 @@ export type { AppTriggerEventOptions, BackCommandOptions, BackCommandResult, + BatchRunResult, BatchRunOptions, BatchStep, CaptureDiffOptions, @@ -102,6 +103,7 @@ export type { SettingsUpdateOptions, StartupPerfSample, SwipeOptions, + TargetShutdownResult, TraceOptions, TypeTextOptions, WaitCommandOptions, diff --git a/src/platforms/ios/debug-symbols/crash-artifact.ts b/src/platforms/ios/debug-symbols/crash-artifact.ts index 7b4b6f2f5..44963f623 100644 --- a/src/platforms/ios/debug-symbols/crash-artifact.ts +++ b/src/platforms/ios/debug-symbols/crash-artifact.ts @@ -1,4 +1,5 @@ import path from 'node:path'; +import { isRecord } from '../../../utils/parsing.ts'; import type { AppleImage, CrashArtifact, @@ -9,7 +10,6 @@ import type { } from './types.ts'; import { addressKey, - isRecord, normalizeUuid, parseAtosSymbol, readJsonRecord, @@ -65,11 +65,8 @@ function readIpsFrameMatches(rawThreads: unknown[], images: AppleImage[]): IpsFr } function readIpsFrameRecords(thread: unknown): Record[] { - if (!thread || typeof thread !== 'object') return []; - const frames = (thread as Record).frames; - return Array.isArray(frames) - ? frames.filter((frame): frame is Record => isRecord(frame)) - : []; + if (!isRecord(thread) || !Array.isArray(thread.frames)) return []; + return thread.frames.filter(isRecord); } function readIpsFrameMatch( @@ -111,18 +108,17 @@ function writeIpsFrameSymbol(frame: Record, symbol: string): vo } function readIpsImage(value: unknown, index: number): AppleImage[] { - if (!value || typeof value !== 'object') return []; - const record = value as Record; - const uuid = normalizeUuid(readString(record.uuid)); - const base = readBigIntField(record, 'base', 'IPS usedImages'); + if (!isRecord(value)) return []; + const uuid = normalizeUuid(readString(value.uuid)); + const base = readBigIntField(value, 'base', 'IPS usedImages'); if (!uuid || base === undefined) return []; - const pathValue = readString(record.path); + const pathValue = readString(value.path); return [ { index, - name: readString(record.name) ?? (pathValue ? path.basename(pathValue) : `image-${index}`), + name: readString(value.name) ?? (pathValue ? path.basename(pathValue) : `image-${index}`), uuid, - arch: readString(record.arch), + arch: readString(value.arch), base, path: pathValue, }, diff --git a/src/platforms/ios/debug-symbols/report.ts b/src/platforms/ios/debug-symbols/report.ts index 1827552e9..313166f20 100644 --- a/src/platforms/ios/debug-symbols/report.ts +++ b/src/platforms/ios/debug-symbols/report.ts @@ -7,6 +7,7 @@ import type { SymbolicatedAddress, TextFrameMatch, } from './types.ts'; +import { isRecord } from '../../../utils/parsing.ts'; import { addressKey, compactJoin, @@ -14,7 +15,6 @@ import { hex, readNumber, readJsonRecord, - readRecord, readString, } from './utils.ts'; @@ -74,17 +74,16 @@ function readIpsBundleId( payload: Record, header: Record | null, ): string | undefined { - return firstString(readRecord(payload.bundleInfo)?.CFBundleIdentifier, header?.bundleID); + const bundleInfo = isRecord(payload.bundleInfo) ? payload.bundleInfo : undefined; + return firstString(bundleInfo?.CFBundleIdentifier, header?.bundleID); } function readIpsVersion( payload: Record, header: Record | null, ): string | undefined { - return firstString( - readRecord(payload.bundleInfo)?.CFBundleShortVersionString, - header?.app_version, - ); + const bundleInfo = isRecord(payload.bundleInfo) ? payload.bundleInfo : undefined; + return firstString(bundleInfo?.CFBundleShortVersionString, header?.app_version); } function readIpsIncident( @@ -102,7 +101,7 @@ function readIpsTimestamp( } function readIpsExceptionType(exception: unknown): string | undefined { - return readString(readRecord(exception)?.type); + return isRecord(exception) ? readString(exception.type) : undefined; } function readIpsHeader(header: string | undefined): Record | null { @@ -113,23 +112,22 @@ function readIpsCrashedThread(payload: Record): number | undefi const faultingThread = readNumber(payload.faultingThread); if (faultingThread !== undefined) return faultingThread; const threads = Array.isArray(payload.threads) ? payload.threads : []; - const triggeredIndex = threads.findIndex((thread) => readRecord(thread)?.triggered === true); + const triggeredIndex = threads.findIndex( + (thread) => isRecord(thread) && thread.triggered === true, + ); return triggeredIndex === -1 ? undefined : triggeredIndex; } function readIpsExceptionCodes(exception: unknown): string | undefined { - const record = readRecord(exception); - if (!record) return undefined; - return firstString(record.codes, record.rawCodes); + return isRecord(exception) ? firstString(exception.codes, exception.rawCodes) : undefined; } function readIpsTerminationReason(termination: unknown): string | undefined { - const record = readRecord(termination); - if (!record) return undefined; + if (!isRecord(termination)) return undefined; return compactJoin([ - readString(record.namespace), - readString(record.code), - readString(record.reason), + readString(termination.namespace), + readString(termination.code), + readString(termination.reason), ]); } diff --git a/src/platforms/ios/debug-symbols/utils.ts b/src/platforms/ios/debug-symbols/utils.ts index c6d927422..e08835374 100644 --- a/src/platforms/ios/debug-symbols/utils.ts +++ b/src/platforms/ios/debug-symbols/utils.ts @@ -1,4 +1,5 @@ import { AppError } from '../../../utils/errors.ts'; +import { isRecord } from '../../../utils/parsing.ts'; const UUID_RE = /^[0-9a-fA-F-]{32,36}$/; @@ -32,14 +33,6 @@ export function compactJoin(values: (string | undefined)[]): string | undefined return compact.length > 0 ? compact.join(' ') : undefined; } -export function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null; -} - -export function readRecord(value: unknown): Record | undefined { - return isRecord(value) ? value : undefined; -} - export function readJsonRecord(text: string): Record | null { try { const value = JSON.parse(text); diff --git a/src/platforms/ios/runner-xctestrun-products.ts b/src/platforms/ios/runner-xctestrun-products.ts index 737bc0440..80a0d0959 100644 --- a/src/platforms/ios/runner-xctestrun-products.ts +++ b/src/platforms/ios/runner-xctestrun-products.ts @@ -2,6 +2,7 @@ import fs from 'node:fs'; import path from 'node:path'; import { readApplePlistJson } from './tool-provider.ts'; import { parseXmlDocumentSync, visitXmlPlistEntries, type XmlNode } from './xml.ts'; +import { isRecord } from '../../utils/parsing.ts'; const XCTESTRUN_PRODUCT_REFERENCE_KEYS = new Set([ 'ProductPaths', @@ -108,7 +109,11 @@ async function resolveXctestrunProductReferences(xctestrunPath: string): Promise function resolveXctestrunProductReferencesFromJson(parsed: Record): string[] { const values = new Set(); - for (const target of collectXctestrunProductReferenceTargets(parsed)) { + for (const target of [ + parsed, + ...collectConfiguredTestTargets(parsed), + ...collectLegacyTestTargets(parsed), + ]) { for (const value of collectXctestrunProductReferenceValuesFromTarget(target)) { values.add(value); } @@ -117,12 +122,6 @@ function resolveXctestrunProductReferencesFromJson(parsed: Record, -): Record[] { - return [parsed, ...collectConfiguredTestTargets(parsed), ...collectLegacyTestTargets(parsed)]; -} - function collectConfiguredTestTargets(parsed: Record): Record[] { const testConfigurations = parsed.TestConfigurations; if (!Array.isArray(testConfigurations)) { @@ -131,9 +130,7 @@ function collectConfiguredTestTargets(parsed: Record): Record[] = []; for (const config of testConfigurations) { - if (!isRecord(config)) { - continue; - } + if (!isRecord(config)) continue; const testTargets = config.TestTargets; if (Array.isArray(testTargets)) { targets.push(...testTargets.filter(isRecord)); @@ -148,10 +145,6 @@ function collectLegacyTestTargets(parsed: Record): Record { - return Boolean(value) && typeof value === 'object'; -} - function collectXctestrunProductReferenceValuesFromTarget( target: Record, ): string[] { diff --git a/src/snapshot-diagnostics.ts b/src/snapshot-diagnostics.ts index 5dad88c78..64ec2dd2e 100644 --- a/src/snapshot-diagnostics.ts +++ b/src/snapshot-diagnostics.ts @@ -1,5 +1,6 @@ import type { SnapshotBackend } from './utils/snapshot.ts'; import type { Platform } from './utils/device.ts'; +import { isRecord } from './utils/parsing.ts'; const SLOW_SNAPSHOT_P95_WARNING_MS = 1_500; @@ -78,11 +79,10 @@ export function mergeSnapshotDiagnostics( export function readSnapshotDiagnosticsSummary( value: unknown, ): SnapshotDiagnosticsSummary | undefined { - if (!value || typeof value !== 'object' || Array.isArray(value)) return undefined; - const record = value as Record; - const stats = readSnapshotTimingStats(record.stats); + if (!isRecord(value)) return undefined; + const stats = readSnapshotTimingStats(value.stats); if (!stats) return undefined; - const warning = typeof record.warning === 'string' ? record.warning : undefined; + const warning = typeof value.warning === 'string' ? value.warning : undefined; return { stats, ...(warning ? { warning } : {}) }; } @@ -128,22 +128,15 @@ function formatSlowSnapshotWarning(stats: SnapshotTimingStats): string { } function readSnapshotTimingStats(value: unknown): SnapshotTimingStats | undefined { - const record = readRecord(value); - if (!record) return undefined; - const required = readRequiredSnapshotTimingStats(record); + if (!isRecord(value)) return undefined; + const required = readRequiredSnapshotTimingStats(value); if (!required) return undefined; return { ...required, - ...readOptionalSnapshotTimingStats(record), + ...readOptionalSnapshotTimingStats(value), }; } -function readRecord(value: unknown): Record | undefined { - return value && typeof value === 'object' && !Array.isArray(value) - ? (value as Record) - : undefined; -} - function readRequiredSnapshotTimingStats( record: Record, ): @@ -174,8 +167,7 @@ function readOptionalSnapshotTimingStats( record: Record, ): Pick { const platform = typeof record.platform === 'string' ? record.platform : undefined; - const backendRecord = readRecord(record.backends); - const backends = backendRecord ? readBackendCounts(backendRecord) : undefined; + const backends = isRecord(record.backends) ? readBackendCounts(record.backends) : undefined; return { ...(platform ? { platform: platform as SnapshotTimingStats['platform'] } : {}), ...(backends ? { backends } : {}), diff --git a/src/target-shutdown-contract.ts b/src/target-shutdown-contract.ts new file mode 100644 index 000000000..eed1398dc --- /dev/null +++ b/src/target-shutdown-contract.ts @@ -0,0 +1,9 @@ +import type { NormalizedError } from './utils/errors.ts'; + +export type TargetShutdownResult = { + success: boolean; + exitCode: number; + stdout: string; + stderr: string; + error?: NormalizedError; +}; diff --git a/src/utils/cli-flags.ts b/src/utils/cli-flags.ts index 6a1f99703..b2116edea 100644 --- a/src/utils/cli-flags.ts +++ b/src/utils/cli-flags.ts @@ -10,6 +10,7 @@ import type { DaemonTransportPreference, LeaseBackend, NetworkIncludeMode, + SessionRuntimeHints, SessionIsolationMode, } from '../contracts.ts'; import type { RemoteConfigMetroOptions } from '../remote-config-schema.ts'; @@ -129,7 +130,7 @@ export type CliFlags = RemoteConfigMetroOptions & batchSteps?: Array<{ command: string; input: Record; - runtime?: unknown; + runtime?: SessionRuntimeHints; }>; out?: string; help: boolean; diff --git a/src/utils/parsing.ts b/src/utils/parsing.ts index b22863f4d..c85c71e2f 100644 --- a/src/utils/parsing.ts +++ b/src/utils/parsing.ts @@ -71,8 +71,7 @@ export function readDeviceTarget(record: Record, key: string): return readOptional(record, key, parseDeviceTarget) ?? 'mobile'; } -export function readRect(record: Record, key: string): Rect | undefined { - const value = record[key]; +export function parseRect(value: unknown): Rect | undefined { if (!isRecord(value)) return undefined; const x = readNumberField(value, 'x'); const y = readNumberField(value, 'y'); @@ -84,8 +83,7 @@ export function readRect(record: Record, key: string): Rect | u return { x, y, width, height }; } -export function readPoint(record: Record, key: string): Point | undefined { - const value = record[key]; +export function parsePoint(value: unknown): Point | undefined { if (!isRecord(value)) return undefined; const x = readNumberField(value, 'x'); const y = readNumberField(value, 'y'); @@ -136,7 +134,7 @@ export function asRecord(value: unknown): Record { } export function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null; + return value !== null && typeof value === 'object' && !Array.isArray(value); } export function stripUndefined>(value: T): T { diff --git a/src/utils/screenshot-result.ts b/src/utils/screenshot-result.ts new file mode 100644 index 000000000..cd912f012 --- /dev/null +++ b/src/utils/screenshot-result.ts @@ -0,0 +1,64 @@ +import type { ScreenshotOverlayRef } from './snapshot.ts'; +import { isRecord, parsePoint, parseRect } from './parsing.ts'; + +export type ScreenshotResultData = { + path?: string; + overlayRefs?: ScreenshotOverlayRef[]; +}; + +type ScreenshotOverlayRefData = { + ref?: unknown; + label?: unknown; + rect?: unknown; + overlayRect?: unknown; + center?: unknown; +}; + +export function readScreenshotResultData(value: unknown): ScreenshotResultData | undefined { + if (!isRecord(value)) return undefined; + const path = typeof value.path === 'string' ? value.path : undefined; + const overlayRefs = Array.isArray(value.overlayRefs) + ? value.overlayRefs.filter(isScreenshotOverlayRefData).flatMap((entry) => { + const overlayRef = readScreenshotOverlayRef(entry); + return overlayRef ? [overlayRef] : []; + }) + : undefined; + return { + ...(path ? { path } : {}), + ...(overlayRefs ? { overlayRefs } : {}), + }; +} + +function readScreenshotOverlayRef( + record: ScreenshotOverlayRefData, +): ScreenshotOverlayRef | undefined { + if (typeof record.ref !== 'string' || record.ref.length === 0) return undefined; + const geometry = readScreenshotOverlayGeometry(record); + if (!geometry) return undefined; + return { + ref: record.ref, + ...readScreenshotOverlayLabel(record), + ...geometry, + }; +} + +function readScreenshotOverlayGeometry( + record: ScreenshotOverlayRefData, +): Pick | undefined { + const rect = parseRect(record.rect); + if (!rect) return undefined; + const overlayRect = parseRect(record.overlayRect); + if (!overlayRect) return undefined; + const center = parsePoint(record.center); + return center ? { rect, overlayRect, center } : undefined; +} + +function readScreenshotOverlayLabel( + record: ScreenshotOverlayRefData, +): Pick { + return typeof record.label === 'string' && record.label.length > 0 ? { label: record.label } : {}; +} + +function isScreenshotOverlayRefData(value: unknown): value is ScreenshotOverlayRefData { + return isRecord(value); +} diff --git a/test/integration/provider-scenarios/android-lifecycle.test.ts b/test/integration/provider-scenarios/android-lifecycle.test.ts index 025149847..08349bfab 100644 --- a/test/integration/provider-scenarios/android-lifecycle.test.ts +++ b/test/integration/provider-scenarios/android-lifecycle.test.ts @@ -1104,10 +1104,7 @@ async function runAndroidCaptureInteractionAndReplayWorkflow( ...selection, }); assert.equal(batch.executed, 1); - assert.equal( - (batch.results as Array<{ data?: { count?: number } }> | undefined)?.[0]?.data?.count, - 2, - ); + assert.equal(batch.results[0]?.data.count, 2); const replayPath = path.join(tempRoot, 'settings-search.ad'); fs.writeFileSync( @@ -1155,7 +1152,7 @@ async function runAndroidCaptureInteractionAndReplayWorkflow( }); assert.equal(fastScreenshot.path, fastScreenshotPath); assert.ok( - Array.isArray(fastScreenshot.overlayRefs) && fastScreenshot.overlayRefs.length > 0, + fastScreenshot.overlayRefs && fastScreenshot.overlayRefs.length > 0, JSON.stringify(fastScreenshot), ); assertPngFile(fastScreenshotPath);