diff --git a/docs/adr/0012-interactive-replay.md b/docs/adr/0012-interactive-replay.md index 077c0c4d33..05f91a601f 100644 --- a/docs/adr/0012-interactive-replay.md +++ b/docs/adr/0012-interactive-replay.md @@ -481,10 +481,22 @@ number of suggestions available at default/full) so a caller knows whether a re- has material; default and full carry at most **20** screen refs and **5** suggestions ranked per decision 1's total order. These counts are absolute, including error payloads. Individual labels, ids, selectors, source paths, mismatch values, cause messages, and hints are UTF-8 truncated to -**256 bytes**; an action summary has no positional array, and fill text, expanded variables, and arbitrary -nested cause details are never serialized. All rendered strings and any overflow artifact pass through the -central diagnostics redactor before truncation. The report sets truncation/redaction markers for every -omission. +**256 bytes**; an action summary has no positional array, and arbitrary nested cause details are never +serialized. Maestro failure provenance renders resolved diagnostic identifiers, including targets and +`runFlow` paths, so the report names what the runtime actually attempted instead of emitting an unresolved +`${VAR}` or synthetic `` token. Text-entry payloads remain semantic secrets: `inputText` progress, +failure messages, suggestions, and overflow artifacts never serialize the entered text. Injected replay +values are not registered as global sensitive literals: a short ordinary value such as `2` or `on` would +otherwise corrupt unrelated timestamps, paths, and typed error fields throughout the request log. +Text-entry values are registered at the actual dispatch boundary before platform work, independently of +the user-facing failure projection. Users must not place secrets in selectors, links, filenames, or other +diagnostic identifiers that are expected to appear in failure output. + +Native `.ad` replay retains its categorical `` replacement in human-readable divergence +messages, hints, and bounded diagnostic fields. That existing fail-closed policy is intentionally +separate from Maestro compatibility output; semantically masked positionals and daemon-owned +machine-readable fields and paths are never substring-rewritten. The report sets truncation/redaction +markers for every omission. When the bounded form would omit material, the daemon writes the same redacted, bounded-per-field detail to a session-scoped divergence artifact and returns its path plus `overflow: { omittedBytes, artifactPath diff --git a/src/compat/maestro/__tests__/daemon-runtime-port.test.ts b/src/compat/maestro/__tests__/daemon-runtime-port.test.ts index bee15d8fa8..3c14f46a00 100644 --- a/src/compat/maestro/__tests__/daemon-runtime-port.test.ts +++ b/src/compat/maestro/__tests__/daemon-runtime-port.test.ts @@ -4,12 +4,53 @@ import path from 'node:path'; import { expect, test, vi } from 'vitest'; import type { DaemonInvokeFn, DaemonRequest } from '../../../daemon/types.ts'; import { PNG } from '../../../utils/png.ts'; +import { + emitDiagnostic, + flushDiagnosticsToSessionFile, + withDiagnosticsScope, +} from '../../../utils/diagnostics.ts'; import { createDaemonMaestroRuntimePort } from '../daemon-runtime-port.ts'; import { MAESTRO_OBSERVATION_POLL_MS } from '../daemon-runtime-port-observation.ts'; import { parseMaestroProgram } from '../program-ir-parser.ts'; import { executeMaestroProgram } from './runtime-port-fixtures.ts'; import { makeBaseRequest, makeDependencies, makeSnapshot } from './daemon-runtime-port-fixtures.ts'; +test('registers Maestro inputText as sensitive before nested platform work', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-maestro-input-diagnostics-')); + const logPath = path.join(root, 'request.ndjson'); + const text = 'opaque-maestro-input'; + const port = createDaemonMaestroRuntimePort({ + baseReq: makeBaseRequest({ flags: { platform: 'android', replayBackend: 'maestro' } }), + invoke: async (request) => { + if (request.command === 'type') { + emitDiagnostic({ + phase: 'platform_echo', + data: { message: `Backend echoed ${request.positionals?.[0]}` }, + }); + } + return request.command === 'snapshot' + ? { ok: true, data: { nodes: [], createdAt: 0 } } + : { ok: true, data: {} }; + }, + dependencies: makeDependencies(), + platform: 'android', + }); + + await withDiagnosticsScope({ command: 'replay', logPath }, async () => { + await port.execute({ + command: { kind: 'inputText', source: { line: 2 }, text }, + generation: 0, + env: {}, + invalidateObservation() {}, + }); + flushDiagnosticsToSessionFile({ force: true }); + }); + + const diagnostics = fs.readFileSync(logPath, 'utf8'); + expect(diagnostics).not.toContain(text); + expect(diagnostics).toContain('Backend echoed [REDACTED]'); +}); + test('delegates lifecycle and coordinate gestures through public daemon commands', async () => { const requests: DaemonRequest[] = []; const invoke: DaemonInvokeFn = async (request) => { diff --git a/src/compat/maestro/__tests__/engine-context.test.ts b/src/compat/maestro/__tests__/engine-context.test.ts index d15e5cad58..412a306747 100644 --- a/src/compat/maestro/__tests__/engine-context.test.ts +++ b/src/compat/maestro/__tests__/engine-context.test.ts @@ -1,10 +1,11 @@ import { expect, test, vi } from 'vitest'; +import { maestroTestFailure } from '../compatibility-errors.ts'; import { createMaestroExecutionContext } from '../engine-context.ts'; import type { MaestroRuntimePort } from '../engine-types.ts'; import { parseMaestroProgram } from '../program-ir-parser.ts'; import { executeMaestroProgram } from './runtime-port-fixtures.ts'; -test('resolves transitive scoped variables to their final value', () => { +test('resolves transitive scoped variables', () => { const context = createMaestroExecutionContext(); const leave = context.enter({ TARGET: '${NEXT}', @@ -13,11 +14,10 @@ test('resolves transitive scoped variables to their final value', () => { }); expect(context.resolve('${TARGET}')).toBe('Done'); - expect(context.expandedVariables).toEqual({ TARGET: 'Done' }); leave(); }); -test('retains expanded values after nested scopes unwind', () => { +test('keeps nested scopes valid until they unwind', () => { const context = createMaestroExecutionContext(); const rootLeave = context.enter({ SECRET: 'nested-scope-secret' }); const nestedLeave = context.enter({ TARGET: '${SECRET}' }); @@ -25,10 +25,24 @@ test('retains expanded values after nested scopes unwind', () => { expect(context.resolve('${TARGET}')).toBe('nested-scope-secret'); nestedLeave(); rootLeave(); +}); - expect(context.expandedVariables).toEqual({ - TARGET: 'nested-scope-secret', - }); +test('renders resolved target variables in optional-step warnings', async () => { + const target = 'Missing checkout button'; + const program = parseMaestroProgram( + ['---', '- tapOn:', ' text: ${TARGET}', ' optional: true'].join('\n'), + { sourcePath: '/flows/optional.yaml' }, + ); + const port: MaestroRuntimePort = { + execute: vi.fn(async () => { + throw maestroTestFailure(`Missing ${target}`); + }), + observe: vi.fn(async ({ generation }) => ({ generation, matched: true })), + }; + + const result = await executeMaestroProgram(program, port, { env: { TARGET: target } }); + + expect(result.warnings).toEqual([expect.stringContaining(target)]); }); test('rejects cyclic references instead of recursing indefinitely', () => { diff --git a/src/compat/maestro/daemon-runtime-port.ts b/src/compat/maestro/daemon-runtime-port.ts index d78802714f..0fa8302649 100644 --- a/src/compat/maestro/daemon-runtime-port.ts +++ b/src/compat/maestro/daemon-runtime-port.ts @@ -1,6 +1,6 @@ import { AppError, asAppError } from '../../kernel/errors.ts'; import type { Rect } from '../../kernel/snapshot.ts'; -import { emitDiagnostic } from '../../utils/diagnostics.ts'; +import { emitDiagnostic, registerDiagnosticSensitiveValue } from '../../utils/diagnostics.ts'; import { stripUndefined } from '../../utils/parsing.ts'; import { executeRunScriptFile } from './run-script-execution.ts'; import { @@ -96,6 +96,7 @@ function createDaemonMaestroRuntimeParts(options: CreateDaemonMaestroRuntimeOper text: string, context: MaestroRuntimeOperationContext, ): Promise => { + registerDiagnosticSensitiveValue(text); await invokeMutation({ kind: 'typeText', text }, context); const stable = await waitForTypedSnapshotStability({ timeoutMs: MAESTRO_DEFAULT_SETTLE_TIMEOUT_MS, diff --git a/src/compat/maestro/engine-context.ts b/src/compat/maestro/engine-context.ts index 910a1e61b1..ba85739cc7 100644 --- a/src/compat/maestro/engine-context.ts +++ b/src/compat/maestro/engine-context.ts @@ -11,7 +11,6 @@ export function createMaestroExecutionContext( // Flow config and runFlow env values are stack-scoped; script output variables persist. let persistentValues = stringifyValues(defaults); const scopes: Record[] = []; - const expandedValues = new Map(); let cachedValues: Readonly> | undefined; let generation = 0; let observation: MaestroObservation | undefined; @@ -26,9 +25,6 @@ export function createMaestroExecutionContext( get observation(): MaestroObservation | undefined { return observation?.generation === generation ? observation : undefined; }, - get expandedVariables(): Readonly> { - return Object.fromEntries(expandedValues); - }, enter(scopedValues: Record = {}): () => void { const resolved = resolveScopedValues(scopedValues); scopes.push(resolved); @@ -62,10 +58,10 @@ export function createMaestroExecutionContext( observation = undefined; }, resolve(value: string): string { - return resolveValue(value, currentValues(), recordExpandedValue); + return resolveValue(value, currentValues()); }, resolveDeferred(value: string): string { - return resolveValue(value, currentValues(), undefined, new Set(), false); + return resolveValue(value, currentValues(), new Set(), false); }, }; @@ -92,17 +88,12 @@ export function createMaestroExecutionContext( ...resolved, ...overrides, }, - undefined, new Set(), false, ); } return resolved; } - - function recordExpandedValue(name: string, value: string): void { - expandedValues.set(name, value); - } } function stringifyValues( @@ -114,7 +105,6 @@ function stringifyValues( function resolveValue( value: string, values: Readonly>, - onExpanded?: (name: string, value: string) => void, resolving = new Set(), failOnUnresolved = true, ): string { @@ -130,11 +120,9 @@ function resolveValue( const resolved = resolveValue( values[key]!, values, - onExpanded, new Set([...resolving, key]), failOnUnresolved, ); - onExpanded?.(key, resolved); return resolved; }); if (failOnUnresolved) assertNoUnsupportedInterpolation(resolved); diff --git a/src/compat/maestro/engine-types.ts b/src/compat/maestro/engine-types.ts index 4af22839ff..f4ca3bbee7 100644 --- a/src/compat/maestro/engine-types.ts +++ b/src/compat/maestro/engine-types.ts @@ -137,7 +137,6 @@ export type MaestroEngineObserver = { runtimeMetrics?: MaestroRuntimeMetrics; error: unknown; artifactPaths: readonly string[]; - expandedVariables: Readonly>; }, ): void; }; diff --git a/src/compat/maestro/replay-plan-execution.ts b/src/compat/maestro/replay-plan-execution.ts index edcef29cc0..d449037510 100644 --- a/src/compat/maestro/replay-plan-execution.ts +++ b/src/compat/maestro/replay-plan-execution.ts @@ -96,7 +96,6 @@ async function executeObservedStep( ...runtimeMetricsDelta(metricsBefore, state.port.readMetrics?.()), error: failure.error, artifactPaths: [...state.artifacts], - expandedVariables: state.context.expandedVariables, }), ); throw failure; diff --git a/src/compat/maestro/replay-plan-step-execution.ts b/src/compat/maestro/replay-plan-step-execution.ts index 0d16959720..349ecac04e 100644 --- a/src/compat/maestro/replay-plan-step-execution.ts +++ b/src/compat/maestro/replay-plan-step-execution.ts @@ -75,18 +75,21 @@ async function executeStep( } async function executeOptionalCommand( - command: MaestroRuntimeCommand, + rawCommand: MaestroRuntimeCommand, appId: string | undefined, state: MaestroReplayPlanExecutionState, ): Promise { + const command = resolveCommand(rawCommand, state.context); try { - return await executeCommand(command, appId, state); + return await executeResolvedCommand(command, appId, state); } catch (error) { checkpointMaestroCancellation(state.options.signal); - if (!isOptionalCommand(command) || !isMaestroTestFailure(error)) throw error; - state.warnings.push(formatOptionalWarning(command, error)); - state.skipped += 1; - return undefined; + if (isOptionalCommand(command) && isMaestroTestFailure(error)) { + state.warnings.push(formatOptionalWarning(command, error)); + state.skipped += 1; + return undefined; + } + throw commandFailure(error, command); } } @@ -100,12 +103,11 @@ function isOptionalCommand(command: MaestroRuntimeCommand): boolean { return 'optional' in command && command.optional === true; } -async function executeCommand( - rawCommand: MaestroRuntimeCommand, +async function executeResolvedCommand( + command: MaestroRuntimeCommand, appId: string | undefined, state: MaestroReplayPlanExecutionState, ): Promise { - const command = resolveCommand(rawCommand, state.context); switch (command.kind) { case 'assertVisible': await requireObservation( @@ -303,6 +305,15 @@ export function asMaestroReplayPlanStepFailure( }; } +function commandFailure(error: unknown, command: MaestroRuntimeCommand): PlanStepFailure { + return { + kind: 'maestroPlanStepFailure', + error: withSource(error, command), + source: command.source, + command, + }; +} + function isPlanStepFailure(value: unknown): value is PlanStepFailure { return Boolean( value && diff --git a/src/compat/maestro/support-matrix.ts b/src/compat/maestro/support-matrix.ts index e38b2646b4..132053e37a 100644 --- a/src/compat/maestro/support-matrix.ts +++ b/src/compat/maestro/support-matrix.ts @@ -9,6 +9,7 @@ export const MAESTRO_COMPAT_LIMITATIONS = [ 'Runtime: iOS and Android only; launchApp.clearState supports Android and iOS simulators, launch arguments are Apple-only, and standalone device utility/state commands are unsupported.', 'Expressions: when.true supports boolean literals and maestro.platform comparisons; repeat.while, evalScript, and broader JavaScript expressions are unsupported.', 'Environment: flow env is the default, AD_VAR_* overrides it, and CLI -e KEY=VALUE wins over both.', + 'Failure diagnostics: resolved targets and runFlow paths are rendered, while inputText payloads remain hidden; do not place secrets in diagnostic identifiers.', 'Trust: runScript executes trusted scripts, may make http.post network requests, and is not a security sandbox; output keys cannot contain a dot.', 'Errors and tracking: unsupported commands and fields fail with source context when available; open a focused issue only when implementation work is planned.', ] as const; diff --git a/src/daemon/__tests__/request-router-replay-env.test.ts b/src/daemon/__tests__/request-router-replay-env.test.ts new file mode 100644 index 0000000000..6d107b35ae --- /dev/null +++ b/src/daemon/__tests__/request-router-replay-env.test.ts @@ -0,0 +1,70 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { expect, test } from 'vitest'; +import { makeSessionStore } from '../../__tests__/test-utils/index.ts'; +import { LeaseRegistry } from '../lease-registry.ts'; +import { createRequestHandler } from '../request-router.ts'; + +function createHarness() { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-router-replay-env-')); + return { + root, + handler: createRequestHandler({ + logPath: path.join(root, 'daemon.log'), + stateDir: root, + token: 'test-token', + sessionStore: makeSessionStore('agent-device-router-replay-env-store-'), + leaseRegistry: new LeaseRegistry(), + trackDownloadableArtifact: () => 'artifact-id', + }), + }; +} + +test('malformed replay env returns a normalized INVALID_ARGS response', async () => { + const { root, handler } = createHarness(); + const flowPath = path.join(root, 'flow.ad'); + fs.writeFileSync(flowPath, 'wait 1\n'); + + await expect( + handler({ + token: 'test-token', + session: 'default', + command: 'replay', + positionals: [flowPath], + flags: { replayEnv: ['NOEQUAL'] }, + meta: { requestId: 'req-invalid-replay-env' }, + }), + ).resolves.toMatchObject({ + ok: false, + error: { + code: 'INVALID_ARGS', + message: expect.stringContaining('expected KEY=VALUE'), + }, + }); +}); + +test('ordinary replay env values do not globally corrupt request diagnostics', async () => { + const { root, handler } = createHarness(); + const missingPath = path.join(root, '2-missing.ad'); + + const response = await handler({ + token: 'test-token', + session: 'default', + command: 'replay', + positionals: [missingPath], + flags: { replayEnv: ['RETRIES=2', 'USER=demo'] }, + meta: { requestId: 'req-ordinary-replay-env' }, + }); + + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.logPath).toBeTruthy(); + const diagnostics = fs.readFileSync(response.error.logPath!, 'utf8'); + expect(diagnostics).toContain(missingPath); + expect(diagnostics).not.toContain('[REDACTED]-missing.ad'); + for (const line of diagnostics.trim().split('\n')) { + const event = JSON.parse(line) as { ts: string }; + expect(event.ts).not.toContain('[REDACTED]'); + } +}); diff --git a/src/daemon/handlers/__tests__/session-replay-maestro-failure.test.ts b/src/daemon/handlers/__tests__/session-replay-maestro-failure.test.ts index 8106cd2927..dea8d4a334 100644 --- a/src/daemon/handlers/__tests__/session-replay-maestro-failure.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-maestro-failure.test.ts @@ -62,7 +62,6 @@ async function buildFailureResponse( durationMs: 12, error: new Error('typed Maestro action failed'), artifactPaths: [], - expandedVariables: {}, }, plan: makeMaestroPlan(), replayPath: path.join(root, 'flow.yaml'), @@ -75,7 +74,7 @@ async function buildFailureResponse( return response; } -test('typed Maestro failure projection is report-only and preserves authored provenance', () => { +test('typed Maestro failure projection keeps the event command and source provenance', () => { const command = { kind: 'tapOn' as const, source: { path: '/flows/login.yaml', line: 4 }, @@ -92,12 +91,11 @@ test('typed Maestro failure projection is report-only and preserves authored pro durationMs: 12, error: new Error('tap failed'), artifactPaths: [], - expandedVariables: {}, }, request, ); - expect(projection.authoredCommand).toBe(command); + expect(projection.command).toBe(command); expect(projection.source).toBe(command.source); expect(projection.progress).toEqual({ command: 'tapOn', value: 'save' }); expect(projection.action).toEqual({ @@ -108,57 +106,54 @@ test('typed Maestro failure projection is report-only and preserves authored pro expect(Object.keys(projection.action)).toEqual(['command', 'positionals', 'flags']); }); -test('typed Maestro failure diagnostics scrub expanded selector values', async () => { - const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-maestro-selector-redaction-')); +test('typed Maestro failure diagnostics render expanded selector values without extra flags', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-maestro-expanded-selector-')); const sessionStore = new SessionStore(path.join(root, 'sessions')); const sessionName = 'default'; sessionStore.set(sessionName, makeIosSession(sessionName)); const flowPath = path.join(root, 'flow.yaml'); - const sentinel = 'expanded-maestro-selector-secret'; + const label = 'Continue checkout'; fs.writeFileSync( flowPath, ['appId: com.example.app', '---', '- tapOn: ${TARGET}', ''].join('\n'), ); - const nodes = [ - { - index: 0, - depth: 0, - type: 'Application', - rect: { x: 0, y: 0, width: 402, height: 874 }, - }, - { - index: 1, - parentIndex: 0, - depth: 1, - type: 'Button', - label: sentinel, - rect: { x: 20, y: 40, width: 120, height: 44 }, - hittable: true, - }, - ]; - mockDispatchCommand.mockResolvedValue({ - nodes, - truncated: false, - backend: 'xctest', - }); const response = await runReplayScriptFile({ req: baseReq({ positionals: [flowPath], - flags: { replayBackend: 'maestro', replayEnv: [`TARGET=${sentinel}`] }, + flags: { + replayBackend: 'maestro', + replayEnv: [`TARGET=${label}`], + }, }), sessionName, logPath: path.join(root, 'daemon.log'), sessionStore, invoke: async (req) => { - if (req.command === 'snapshot') return { ok: true, data: { nodes } }; + if (req.command === 'snapshot') { + return { + ok: true, + data: { + nodes: [ + { + index: 0, + depth: 0, + type: 'Button', + label, + rect: { x: 20, y: 40, width: 120, height: 44 }, + hittable: true, + }, + ], + }, + }; + } if (req.command === 'click') { return { ok: false, error: { code: 'COMMAND_FAILED', - message: `tap failed for ${sentinel}`, - hint: `Find ${sentinel}`, + message: `tap failed for ${label}`, + hint: `Find ${label}`, }, }; } @@ -168,26 +163,20 @@ test('typed Maestro failure diagnostics scrub expanded selector values', async ( expect(response.ok).toBe(false); if (response.ok) return; - expect(JSON.stringify(response.error)).not.toContain(sentinel); - const divergence = response.error.details?.divergence as { - action: string; - cause: { message: string; hint?: string }; - suggestions: unknown[]; - }; - expect(divergence.action).toBe('tapOn "${TARGET}"'); - expect(divergence.cause.message).toContain(''); - expect(divergence.cause.hint).toContain(''); - expect(divergence.suggestions).toEqual([]); + expect(response.error.message).toContain(`tapOn "${label}"`); + expect(response.error.message).toContain(label); + expect(response.error.message).not.toContain(''); + expect(response.error.hint).toContain(label); }); -test('typed Maestro nested scopes scrub failure values after unwind and keep retry trace identity', async () => { +test('typed Maestro nested scopes retain resolved target values after unwind', async () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-maestro-nested-redaction-')); const sessionStore = new SessionStore(path.join(root, 'sessions')); const sessionName = 'default'; sessionStore.set(sessionName, makeIosSession(sessionName)); const flowPath = path.join(root, 'flow.yaml'); const tracePath = path.join(root, 'replay-timing.ndjson'); - const sentinel = 'nested-maestro-scope-secret'; + const targetLabel = 'Nested checkout target'; fs.writeFileSync( flowPath, [ @@ -198,7 +187,7 @@ test('typed Maestro nested scopes scrub failure values after unwind and keep ret ' commands:', ' - runFlow:', ' env:', - ' TARGET: ${SECRET}', + ' TARGET: ${SOURCE_LABEL}', ' commands:', ' - tapOn: ${TARGET}', '', @@ -217,7 +206,7 @@ test('typed Maestro nested scopes scrub failure values after unwind and keep ret parentIndex: 0, depth: 1, type: 'Button', - label: sentinel, + label: targetLabel, rect: { x: 20, y: 40, width: 120, height: 44 }, hittable: true, }, @@ -229,7 +218,7 @@ test('typed Maestro nested scopes scrub failure values after unwind and keep ret flags: { replayBackend: 'maestro', platform: 'ios', - replayEnv: [`SECRET=${sentinel}`], + replayEnv: [`SOURCE_LABEL=${targetLabel}`], }, }), sessionName, @@ -243,8 +232,8 @@ test('typed Maestro nested scopes scrub failure values after unwind and keep ret ok: false, error: { code: 'COMMAND_FAILED', - message: `tap failed for ${sentinel}`, - hint: `Find ${sentinel}`, + message: `tap failed for ${targetLabel}`, + hint: `Find ${targetLabel}`, }, }; } @@ -253,7 +242,7 @@ test('typed Maestro nested scopes scrub failure values after unwind and keep ret }); expect(response.ok).toBe(false); - expect(JSON.stringify(response)).not.toContain(sentinel); + expect(JSON.stringify(response)).toContain(targetLabel); const events = fs .readFileSync(tracePath, 'utf8') .trim() @@ -276,6 +265,46 @@ test('typed Maestro nested scopes scrub failure values after unwind and keep ret ]); }); +test('typed Maestro renders flow-local values when static include resolution fails', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-maestro-include-redaction-')); + const sessionName = 'default'; + const sessionStore = new SessionStore(path.join(root, 'sessions')); + sessionStore.set(sessionName, makeIosSession(sessionName)); + const flowPath = path.join(root, 'flow.yaml'); + const flowName = 'checkout-details'; + fs.writeFileSync( + flowPath, + ['env:', ` FLOW_NAME: ${flowName}`, '---', '- runFlow: ${FLOW_NAME}.yaml', ''].join('\n'), + ); + + const response = await runReplayScriptFile({ + req: baseReq({ + positionals: [flowPath], + flags: { replayBackend: 'maestro', platform: 'ios' }, + }), + sessionName, + logPath: path.join(root, 'daemon.log'), + sessionStore, + invoke: async () => ({ ok: true, data: {} }), + }); + + expect(response.ok).toBe(false); + expect(JSON.stringify(response)).toContain(flowName); + if (response.ok) return; + expect(response.error.message).toContain(`${flowName}.yaml`); +}); + +test('typed Maestro failure diagnostics never render inputText payloads', async () => { + const text = 'highly-sensitive-input'; + const response = await buildFailureResponse( + { kind: 'inputText', source: { path: '/flows/login.yaml', line: 4 }, text }, + [], + ); + + expect(JSON.stringify(response.error)).not.toContain(text); + expect(response.error.message).toContain('inputText'); +}); + test('typed Maestro suggestions rank visible childOf candidates and exclude out-of-scope nodes', async () => { const command = { kind: 'tapOn' as const, diff --git a/src/daemon/handlers/__tests__/session-replay-runtime-failure-response.test.ts b/src/daemon/handlers/__tests__/session-replay-runtime-failure-response.test.ts index 15c962727e..bd4eb46d95 100644 --- a/src/daemon/handlers/__tests__/session-replay-runtime-failure-response.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-runtime-failure-response.test.ts @@ -7,6 +7,7 @@ vi.mock('../../../core/dispatch.ts', async (importOriginal) => { import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; +import { buildReplayDivergenceFailureResponseFromDescriptor } from '../session-replay-runtime-failure-response.ts'; import { runReplayScriptFile } from '../session-replay-runtime.ts'; import { SessionStore } from '../../session-store.ts'; import { dispatchCommand } from '../../../core/dispatch.ts'; @@ -22,6 +23,52 @@ beforeEach(() => { mockDispatchCommand.mockReset(); mockDispatchCommand.mockResolvedValue({}); }); + +test('native replay failure metadata keeps machine fields and daemon-owned paths intact', () => { + const replayPath = '/tmp/flows/ios-login.ad'; + const artifactPath = '/tmp/sessions/default/screenshot-1.png'; + const response = buildReplayDivergenceFailureResponseFromDescriptor({ + error: { + code: 'COMMAND_FAILED', + message: 'Could not tap Continue on ios', + hint: 'Retry Continue on ios', + details: { + reason: 'not_found', + retriable: false, + supportedOn: 'ios', + }, + retriable: false, + supportedOn: 'ios', + }, + actionLabel: 'press Continue', + action: 'press', + positionals: ['Continue'], + step: 2, + replayPath, + artifactPaths: [artifactPath], + divergence: {}, + scrubVars: [ + { name: 'MODE', value: 'on' }, + { name: 'PLATFORM', value: 'ios' }, + { name: 'SESSION', value: 'default' }, + ], + }); + + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.retriable).toBe(false); + expect(response.error.supportedOn).toBe('ios'); + expect(response.error.details).toMatchObject({ + reason: 'not_found', + retriable: false, + supportedOn: 'ios', + replayPath, + positionals: ['Continue'], + artifactPaths: [artifactPath], + }); + expect(response.error.details).toHaveProperty('reason'); + expect(response.error.details).not.toHaveProperty('reas'); +}); test('divergence cause and action strings pass through the central redactor at construction', async () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-divergence-redact-')); const sessionStore = new SessionStore(path.join(root, 'sessions')); diff --git a/src/daemon/handlers/session-replay-maestro-failure.ts b/src/daemon/handlers/session-replay-maestro-failure.ts index c15991d9b7..a86903fd68 100644 --- a/src/daemon/handlers/session-replay-maestro-failure.ts +++ b/src/daemon/handlers/session-replay-maestro-failure.ts @@ -41,7 +41,6 @@ export type MaestroFailedEngineEvent = MaestroEngineEvent & { readonly durationMs: number; readonly error: unknown; readonly artifactPaths: readonly string[]; - readonly expandedVariables: Readonly>; }; export type MaestroFailureReportAction = Pick< @@ -50,7 +49,8 @@ export type MaestroFailureReportAction = Pick< >; export type MaestroFailureReportProjection = { - readonly authoredCommand: MaestroEngineEvent['command']; + /** Failure command; runtime-command failures are resolved, while earlier failures stay authored. */ + readonly command: MaestroEngineEvent['command']; readonly source: MaestroEngineEvent['source']; readonly progress: ReturnType; readonly action: MaestroFailureReportAction; @@ -62,7 +62,7 @@ export function buildTypedMaestroFailureReportProjection( ): MaestroFailureReportProjection { const progress = formatMaestroCommandProgress(event.command); return { - authoredCommand: event.command, + command: event.command, source: event.source, progress, action: { @@ -87,10 +87,7 @@ export async function buildTypedMaestroFailureResponse(params: { const { event, plan, replayPath, req, sessionName, sessionStore, logPath } = params; const report = buildTypedMaestroFailureReportProjection(event, req); const cause = hoistReplayFailureCauseDiagnosticMeta(params.error); - const scrubVars = [ - ...collectExpandedScrubVars(event.expandedVariables), - ...collectMaestroTextScrubVars(report.authoredCommand), - ].sort((left, right) => right.value.length - left.value.length); + const scrubVars = collectMaestroTextScrubVars(report.command); const sanitize = createReplayDivergenceSanitizer(scrubVars); const safeCause = { ...cause, @@ -114,9 +111,9 @@ export async function buildTypedMaestroFailureResponse(params: { const suggestions = session && observation.state === 'available' && - !isMaestroControlCommandDescriptor(report.authoredCommand) + !isMaestroControlCommandDescriptor(report.command) ? collectTypedMaestroSuggestions({ - command: report.authoredCommand, + command: report.command, platform: plan.platform, action: report.action, session, @@ -128,7 +125,7 @@ export async function buildTypedMaestroFailureResponse(params: { from: event.stepIndex, planDigest: plan.digest, }); - const actionLabel = [report.authoredCommand.kind, formatMaestroActionValue(report.progress.value)] + const actionLabel = [report.command.kind, formatMaestroActionValue(report.progress.value)] .filter(Boolean) .join(' '); const divergence: ReplayDivergence = { @@ -173,7 +170,7 @@ export async function buildTypedMaestroFailureResponse(params: { return buildReplayDivergenceFailureResponseFromDescriptor({ error: safeCause, actionLabel, - action: report.authoredCommand.kind, + action: report.command.kind, positionals: [...report.action.positionals], step: event.stepIndex, replayPath, @@ -333,13 +330,6 @@ function safeProgressPositionals(command: string, value: string | undefined): st return [value]; } -function collectExpandedScrubVars(values: Readonly>): ReplayVarScrubEntry[] { - return Object.entries(values) - .filter(([, value]) => value.length > 0) - .map(([name, value]) => ({ name, value })) - .sort((left, right) => right.value.length - left.value.length); -} - function collectMaestroTextScrubVars( command: MaestroEngineEvent['command'], ): ReplayVarScrubEntry[] { diff --git a/src/replay/divergence.ts b/src/replay/divergence.ts index 7164103221..e93fbe155b 100644 --- a/src/replay/divergence.ts +++ b/src/replay/divergence.ts @@ -264,7 +264,10 @@ export type ReplayVarScrubEntry = { name: string; value: string }; * replay-scope value is replaced with a `` marker, whatever the * value looks like — this is not shape-based secret redaction. */ -export function scrubReplayVarValues(value: string, entries: ReplayVarScrubEntry[]): string { +export function scrubReplayVarValues( + value: string, + entries: readonly ReplayVarScrubEntry[], +): string { let output = value; for (const entry of entries) { if (!entry.value) continue; @@ -275,7 +278,7 @@ export function scrubReplayVarValues(value: string, entries: ReplayVarScrubEntry /** Per-report field sanitizer: variable scrub, then redact, then truncate. */ export function createReplayDivergenceSanitizer( - scrubVars: ReplayVarScrubEntry[], + scrubVars: readonly ReplayVarScrubEntry[], ): (value: string, limit?: number) => string { return (value, limit) => sanitizeReplayDivergenceField(scrubReplayVarValues(value, scrubVars), limit); diff --git a/src/replay/test/__tests__/progress.test.ts b/src/replay/test/__tests__/progress.test.ts index c3af0d458f..f7fe03e3b1 100644 --- a/src/replay/test/__tests__/progress.test.ts +++ b/src/replay/test/__tests__/progress.test.ts @@ -213,6 +213,147 @@ test('createReplayTestProgressRenderer trims live step progress by visible colum }); }); +test('createReplayTestProgressRenderer clears every reflowed row after a terminal resize', () => { + const initialColumns = 80; + const resizedColumns = 20; + const clearRow = '\r\u001B[2K'; + const moveUpAndClearRow = '\u001B[1A\r\u001B[2K'; + let columns = initialColumns; + const renderer = createReplayTestProgressRenderer({ + liveProgress: true, + columns: () => columns, + }); + const initial = renderer.render({ + type: 'test-step', + test: { + file: '/tmp/checkout.yaml', + title: 'A long Maestro test title that is one row before terminal reflow', + index: 1, + total: 1, + stepIndex: 8, + stepTotal: 12, + stepCommand: 'assertVisible', + stepValue: 'Confirmation', + }, + }); + assert.ok(initial?.text.startsWith(clearRow)); + const initialVisibleWidth = (initial?.text.length ?? clearRow.length) - clearRow.length; + assert.equal(initialVisibleWidth, initialColumns); + + columns = resizedColumns; + const rendered = renderer.render({ + type: 'test-step', + test: { + file: '/tmp/checkout.yaml', + title: 'A long Maestro test title that is one row before terminal reflow', + index: 1, + total: 1, + stepIndex: 9, + stepTotal: 12, + stepCommand: 'tapOn', + stepValue: 'Continue', + }, + }); + + const reflowedRows = Math.ceil(initialVisibleWidth / resizedColumns); + const expectedClearPrefix = clearRow + moveUpAndClearRow.repeat(Math.max(0, reflowedRows - 1)); + assert.equal(reflowedRows, 4); + assert.equal(rendered?.text.slice(0, expectedClearPrefix.length), expectedClearPrefix); + assert.equal((rendered?.text.split('\u001B[2K').length ?? 1) - 1, reflowedRows); + assert.ok(rendered?.text.endsWith('...')); +}); + +test('createReplayTestProgressRenderer clears every row after a large terminal reflow', () => { + const initialColumns = 200; + const resizedColumns = 20; + const clearRow = '\r\u001B[2K'; + const moveUpAndClearRow = '\u001B[1A\r\u001B[2K'; + let columns = initialColumns; + const renderer = createReplayTestProgressRenderer({ + liveProgress: true, + columns: () => columns, + terminalReflowsOnResize: true, + }); + const initial = renderer.render({ + type: 'test-step', + test: { + file: '/tmp/checkout.yaml', + title: 'x'.repeat(180), + index: 1, + total: 1, + stepIndex: 8, + stepTotal: 12, + stepCommand: 'assertVisible', + stepValue: 'Confirmation', + }, + }); + assert.ok(initial?.text.startsWith(clearRow)); + const initialVisibleWidth = (initial?.text.length ?? clearRow.length) - clearRow.length; + assert.equal(initialVisibleWidth, initialColumns); + + columns = resizedColumns; + const rendered = renderer.render({ + type: 'test-step', + test: { + file: '/tmp/checkout.yaml', + title: 'Checkout', + index: 1, + total: 1, + stepIndex: 9, + stepTotal: 12, + stepCommand: 'tapOn', + stepValue: 'Continue', + }, + }); + + const reflowedRows = Math.ceil(initialVisibleWidth / resizedColumns); + const expectedClearPrefix = clearRow + moveUpAndClearRow.repeat(reflowedRows - 1); + assert.equal(reflowedRows, 10); + assert.equal(rendered?.text.slice(0, expectedClearPrefix.length), expectedClearPrefix); + assert.equal((rendered?.text.split('\u001B[1A').length ?? 1) - 1, 9); + assert.equal((rendered?.text.split('\u001B[2K').length ?? 1) - 1, 10); +}); + +test('createReplayTestProgressRenderer stays on one row in non-reflowing terminals', () => { + let columns = 200; + const renderer = createReplayTestProgressRenderer({ + liveProgress: true, + columns: () => columns, + terminalReflowsOnResize: false, + }); + renderer.render({ + type: 'test-step', + test: { + file: '/tmp/checkout.yaml', + title: 'x'.repeat(180), + index: 1, + total: 1, + stepIndex: 8, + stepTotal: 12, + stepCommand: 'assertVisible', + stepValue: 'Confirmation', + }, + }); + + columns = 20; + const rendered = renderer.render({ + type: 'test-step', + test: { + file: '/tmp/checkout.yaml', + title: 'Checkout', + index: 1, + total: 1, + stepIndex: 9, + stepTotal: 12, + stepCommand: 'tapOn', + stepValue: 'Continue', + }, + }); + + assert.equal((rendered?.text.split('\u001B[1A').length ?? 1) - 1, 0); + assert.equal((rendered?.text.split('\u001B[2K').length ?? 1) - 1, 1); +}); + test('createReplayTestProgressRenderer colors completed result markers when color is enabled', () => { withForcedColor(() => { assert.equal( diff --git a/src/replay/test/__tests__/reporters-default.test.ts b/src/replay/test/__tests__/reporters-default.test.ts index d0d6e4efc3..62c33ffa92 100644 --- a/src/replay/test/__tests__/reporters-default.test.ts +++ b/src/replay/test/__tests__/reporters-default.test.ts @@ -41,20 +41,20 @@ function emptySuite(): ReplaySuiteResult { }; } -function withCiEnv(value: string | undefined, run: () => T): T { - const original = process.env.CI; - if (value === undefined) delete process.env.CI; - else process.env.CI = value; +function withEnv(name: 'CI' | 'TMUX', value: string | undefined, run: () => T): T { + const original = process.env[name]; + if (value === undefined) delete process.env[name]; + else process.env[name] = value; try { return run(); } finally { - if (original === undefined) delete process.env.CI; - else process.env.CI = original; + if (original === undefined) delete process.env[name]; + else process.env[name] = original; } } test('default replay test reporter hides and restores cursor for tty progress', () => { - withCiEnv(undefined, () => { + withEnv('CI', undefined, () => { const reporter = createDefaultReplayTestReporter(); const { context, stderr, stdout } = createReporterContext({ stderrIsTty: true }); @@ -70,6 +70,53 @@ test('default replay test reporter hides and restores cursor for tty progress', }); }); +test('default replay test reporter avoids cursor-up cleanup inside tmux', () => { + withEnv('CI', undefined, () => + withEnv('TMUX', '/tmp/tmux-1000/default,1,0', () => { + const reporter = createDefaultReplayTestReporter(); + const { context, stderr } = createReporterContext({ stderrIsTty: true }); + context.stderr.columns = 200; + reporter.onSuiteStart?.( + { total: 1, runnable: 1, skipped: 0, artifactsDir: '/tmp/replay' }, + context, + ); + reporter.onTestStep?.( + { + file: '/tmp/checkout.yaml', + title: 'x'.repeat(180), + index: 1, + total: 1, + stepIndex: 1, + stepTotal: 2, + stepCommand: 'assertVisible', + stepValue: 'Confirmation', + }, + context, + ); + + context.stderr.columns = 20; + reporter.onTestStep?.( + { + file: '/tmp/checkout.yaml', + title: 'Checkout', + index: 1, + total: 1, + stepIndex: 2, + stepTotal: 2, + stepCommand: 'tapOn', + stepValue: 'Continue', + }, + context, + ); + reporter.onSuiteEnd?.(emptySuite(), context); + + const resizedProgress = stderr[2]; + assert.equal((resizedProgress?.split('\u001B[1A').length ?? 1) - 1, 0); + assert.equal((resizedProgress?.split('\u001B[2K').length ?? 1) - 1, 1); + }), + ); +}); + test('default replay test reporter leaves cursor alone for non-tty streams', () => { const reporter = createDefaultReplayTestReporter(); const { context, stderr } = createReporterContext({ stderrIsTty: false }); diff --git a/src/replay/test/progress.ts b/src/replay/test/progress.ts index fe56e117dd..7202ba647e 100644 --- a/src/replay/test/progress.ts +++ b/src/replay/test/progress.ts @@ -13,7 +13,10 @@ import { colorize, supportsColor } from '../../utils/output.ts'; export type ReplayTestProgressFormatOptions = { verbose?: boolean; liveProgress?: boolean; - columns?: number; + /** A live reader lets TTY progress adapt to terminal resize events. */ + columns?: number | (() => number | undefined); + /** Whether the terminal reflows existing rows when its width changes. */ + terminalReflowsOnResize?: boolean; }; export type ReplayTestProgressRender = { @@ -39,37 +42,60 @@ export function createReplayTestProgressRenderer( ): ReplayTestProgressRenderer { const completedKeys = new Set(); let hasLiveProgressLine = false; + let liveProgressWidth = 0; let spinnerFrameIndex = 0; + + const resetLiveProgress = () => { + completedKeys.clear(); + hasLiveProgressLine = false; + liveProgressWidth = 0; + }; + const renderLiveStep = ( + event: Extract, + ) => { + if (!options.liveProgress) return undefined; + const spinnerFrame = nextReplayTestProgressSpinnerFrame(spinnerFrameIndex); + spinnerFrameIndex += 1; + const line = formatReplayTestLiveProgressLine(event.test, options, spinnerFrame); + const clearPrefix = clearLiveProgressPrefix( + hasLiveProgressLine ? liveProgressWidth : 0, + options.columns, + options.terminalReflowsOnResize !== false, + ); + hasLiveProgressLine = true; + liveProgressWidth = visibleLength(line); + return { text: `${clearPrefix}${line}`, newline: false }; + }; + const renderTestResult = ( + event: Extract, + ) => { + if (isReplayTestCompletionProgressEvent(event.test)) { + const key = replayTestCompletionProgressKey(event.test); + if (completedKeys.has(key)) return undefined; + completedKeys.add(key); + } + const line = formatReplayTestProgressEvent(event.test, options); + if (!line) return undefined; + const text = hasLiveProgressLine + ? `${clearLiveProgressPrefix( + liveProgressWidth, + options.columns, + options.terminalReflowsOnResize !== false, + )}${line}` + : line; + hasLiveProgressLine = false; + liveProgressWidth = 0; + return { text, newline: true }; + }; + return { render(event) { if (event.type === 'suite-start') { - completedKeys.clear(); - hasLiveProgressLine = false; + resetLiveProgress(); return undefined; } - if (event.type === 'test-step') { - if (!options.liveProgress) return undefined; - hasLiveProgressLine = true; - const spinnerFrame = nextReplayTestProgressSpinnerFrame(spinnerFrameIndex); - spinnerFrameIndex += 1; - return { - text: clearLinePrefix( - formatReplayTestLiveProgressLine(event.test, options, spinnerFrame), - ), - newline: false, - }; - } - if (event.type !== 'test-result') return undefined; - if (isReplayTestCompletionProgressEvent(event.test)) { - const key = replayTestCompletionProgressKey(event.test); - if (completedKeys.has(key)) return undefined; - completedKeys.add(key); - } - const line = formatReplayTestProgressEvent(event.test, options); - if (!line) return undefined; - const text = hasLiveProgressLine ? clearLinePrefix(line) : line; - hasLiveProgressLine = false; - return { text, newline: true }; + if (event.type === 'test-step') return renderLiveStep(event); + return event.type === 'test-result' ? renderTestResult(event) : undefined; }, }; } @@ -263,17 +289,30 @@ function replayTestCompletionProgressKey(event: ReplayTestResult): string { return [event.status, event.index, event.total, event.file, event.title ?? '', shard].join('\0'); } -function clearLinePrefix(text: string): string { - return `\r\x1B[2K${text}`; +function clearLiveProgressPrefix( + previousWidth: number, + columns: ReplayTestProgressFormatOptions['columns'], + terminalReflowsOnResize: boolean, +): string { + // Reflowing terminals move the cursor to the final wrapped row after a shrink. + // Non-reflowing multiplexers keep it on the original row, where cursor-up + // cleanup would erase completed test output. + const rows = terminalReflowsOnResize + ? Math.max(1, Math.ceil(previousWidth / resolveColumns(columns))) + : 1; + let output = '\r\x1B[2K'; + for (let row = 1; row < rows; row += 1) { + output += '\x1B[1A\r\x1B[2K'; + } + return output; } -function resolveColumns(columns: number | undefined): number { - return typeof columns === 'number' && Number.isFinite(columns) && columns > 0 - ? Math.floor(columns) - : 80; +function resolveColumns(columns: ReplayTestProgressFormatOptions['columns']): number { + const value = typeof columns === 'function' ? columns() : columns; + return typeof value === 'number' && Number.isFinite(value) && value > 0 ? Math.floor(value) : 80; } -function trimToColumns(value: string, columns: number | undefined): string { +function trimToColumns(value: string, columns: ReplayTestProgressFormatOptions['columns']): string { const limit = resolveColumns(columns); if (visibleLength(value) <= limit) return value; if (limit <= 0) return ''; diff --git a/src/replay/test/reporters/default.ts b/src/replay/test/reporters/default.ts index be329f9eb8..56eee66328 100644 --- a/src/replay/test/reporters/default.ts +++ b/src/replay/test/reporters/default.ts @@ -45,7 +45,8 @@ export function createDefaultReplayTestReporter(): ReplayTestReporter { progressRenderer ??= createReplayTestProgressRenderer({ verbose: context.verbose, liveProgress: shouldUseLiveProgress(context), - columns: context.stderr.columns, + columns: () => context.stderr.columns, + terminalReflowsOnResize: terminalReflowsOnResize(), }); const output = progressRenderer.render(event); if (!output) return; @@ -104,6 +105,10 @@ function shouldUseLiveProgress(context: ReplayTestReporterContext): boolean { return context.stderr.isTTY && !process.env.CI; } +function terminalReflowsOnResize(): boolean { + return !process.env.TMUX && !process.env.STY; +} + function renderReplayTestSummary( data: ReplaySuiteResult, context: ReplayTestReporterContext, diff --git a/website/docs/docs/replay-e2e.md b/website/docs/docs/replay-e2e.md index db869a6bbc..5b5975a0cc 100644 --- a/website/docs/docs/replay-e2e.md +++ b/website/docs/docs/replay-e2e.md @@ -69,6 +69,7 @@ Boundaries: - Runtime: iOS and Android only; `launchApp.clearState` supports Android and iOS simulators, launch arguments are Apple-only, and standalone device utility/state commands are unsupported. - Expressions: `when.true` supports boolean literals and `maestro.platform` comparisons; `repeat.while`, `evalScript`, and broader JavaScript expressions are unsupported. - Environment: flow `env` is the default, `AD_VAR_*` overrides it, and CLI `-e KEY=VALUE` wins over both. +- Failure diagnostics: resolved targets and `runFlow` paths are rendered, while `inputText` payloads remain hidden; do not place secrets in diagnostic identifiers. - Trust: `runScript` executes trusted scripts, may make `http.post` network requests, and is not a security sandbox; output keys cannot contain a dot. - Errors and tracking: unsupported commands and fields fail with source context when available; open a focused issue only when implementation work is planned.