From 63f5cafa4f995555e670e3ebdc6de774bff61406 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 28 Jul 2026 13:49:57 +0200 Subject: [PATCH 1/5] fix(replay): secure Maestro failure diagnostics --- docs/adr/0012-interactive-replay.md | 11 +- scripts/integration-progress-model.ts | 3 +- .../__tests__/args-parse-session.test.ts | 13 +- .../cli-grammar/flag-definitions-workflow.ts | 9 ++ src/commands/cli-grammar/flag-groups.ts | 2 +- src/commands/command-flags.ts | 1 + src/commands/replay/index.test.ts | 6 + src/commands/replay/index.ts | 6 + .../maestro/__tests__/engine-context.test.ts | 44 ++++++- src/compat/maestro/engine-context.ts | 40 ++++-- src/compat/maestro/engine-types.ts | 12 +- src/compat/maestro/replay-plan-execution.ts | 9 +- .../maestro/replay-plan-step-execution.ts | 29 +++-- src/compat/maestro/replay-plan-steps.ts | 7 +- src/compat/maestro/replay-plan-types.ts | 10 +- src/compat/maestro/support-matrix.ts | 2 +- src/contracts/cli-flags.ts | 2 + src/contracts/client-request.ts | 1 + .../session-replay-maestro-failure.test.ts | 115 ++++++++++++++++-- .../__tests__/session-replay-vars.test.ts | 24 ++++ .../session-replay-maestro-failure.ts | 30 +++-- .../session-replay-maestro-response.ts | 39 ++++-- .../session-replay-maestro-runtime.ts | 41 ++++++- ...session-replay-runtime-failure-response.ts | 17 ++- src/daemon/handlers/session-replay-runtime.ts | 10 +- src/daemon/request-router.ts | 21 ++++ src/replay/__tests__/divergence.test.ts | 10 ++ src/replay/divergence.ts | 23 +++- src/replay/test/__tests__/progress.test.ts | 39 ++++++ src/replay/test/progress.ts | 92 +++++++++----- src/replay/test/reporters/default.ts | 2 +- src/replay/vars.ts | 28 +++++ 32 files changed, 589 insertions(+), 109 deletions(-) diff --git a/docs/adr/0012-interactive-replay.md b/docs/adr/0012-interactive-replay.md index 077c0c4d33..9d92a2d640 100644 --- a/docs/adr/0012-interactive-replay.md +++ b/docs/adr/0012-interactive-replay.md @@ -481,10 +481,13 @@ 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 fill text and arbitrary nested cause details +are never serialized. Maestro keeps expanded variables redacted in typed action/source provenance by default, +so a failure never serializes an injected or flow-local value. A caller may explicitly disclose a +non-sensitive runtime value with `--public-env KEY`; names and value shapes never imply sensitivity. Every +other resolved value is removed from rendered strings, optional-step warnings, diagnostics, and overflow +artifacts, including aliases and nested `runFlow` scopes, before the central diagnostics redactor and +truncation. 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/scripts/integration-progress-model.ts b/scripts/integration-progress-model.ts index e03220e141..162b984b85 100644 --- a/scripts/integration-progress-model.ts +++ b/scripts/integration-progress-model.ts @@ -298,7 +298,7 @@ function summarizeProviderScenarioFlagExclusions() { ], }, { - name: 'parser/client-only command flags', + name: 'parser, client, and diagnostic-only command flags', owner: 'args, CLI, debug-symbols, screenshot-diff, and batch tests', keys: [ 'artifact', @@ -311,6 +311,7 @@ function summarizeProviderScenarioFlagExclusions() { 'reporter', 'reportJunit', 'replayMaestro', + 'replayPublicEnv', 'recordVideo', 'shardAll', 'shardSplit', diff --git a/src/cli/parser/__tests__/args-parse-session.test.ts b/src/cli/parser/__tests__/args-parse-session.test.ts index 296df0c5ca..d9c0d0f10b 100644 --- a/src/cli/parser/__tests__/args-parse-session.test.ts +++ b/src/cli/parser/__tests__/args-parse-session.test.ts @@ -205,13 +205,24 @@ test('parseArgs recognizes command-specific flag combinations', async () => { }, { label: 'replay maestro flow', - argv: ['replay', './flow.yaml', '--maestro', '--env', 'USER=Ada', '--timeout', '240000'], + argv: [ + 'replay', + './flow.yaml', + '--maestro', + '--env', + 'USER=Ada', + '--public-env', + 'PASSWORD', + '--timeout', + '240000', + ], strictFlags: true, assertParsed: (parsed) => { assert.equal(parsed.command, 'replay'); assert.deepEqual(parsed.positionals, ['./flow.yaml']); assert.equal(parsed.flags.replayMaestro, true); assert.deepEqual(parsed.flags.replayEnv, ['USER=Ada']); + assert.deepEqual(parsed.flags.replayPublicEnv, ['PASSWORD']); assert.equal(parsed.flags.timeoutMs, 240000); }, }, diff --git a/src/commands/cli-grammar/flag-definitions-workflow.ts b/src/commands/cli-grammar/flag-definitions-workflow.ts index af6d3ec288..9280fb6a44 100644 --- a/src/commands/cli-grammar/flag-definitions-workflow.ts +++ b/src/commands/cli-grammar/flag-definitions-workflow.ts @@ -48,6 +48,15 @@ export const WORKFLOW_FLAG_DEFINITIONS: readonly FlagDefinition[] = [ usageDescription: 'Replay/Test: inject or override a ${KEY} variable for the script (repeatable)', }, + { + key: 'replayPublicEnv', + names: ['--public-env'], + type: 'string', + multiple: true, + usageLabel: '--public-env KEY', + usageDescription: + "Replay/Test: allow this Maestro variable's resolved value in failures (repeatable); all expanded values stay redacted unless explicitly public", + }, { key: 'failFast', names: ['--fail-fast'], diff --git a/src/commands/cli-grammar/flag-groups.ts b/src/commands/cli-grammar/flag-groups.ts index 1b3bd9a0e9..26b92344ef 100644 --- a/src/commands/cli-grammar/flag-groups.ts +++ b/src/commands/cli-grammar/flag-groups.ts @@ -43,7 +43,7 @@ export const REPEATED_TOUCH_FLAGS = flagKeys( // (flag-sourced budget on the interaction descriptors, mirroring wait's // positional budget). export const SETTLE_FLAGS = flagKeys('settle', 'settleQuietMs', 'timeoutMs'); -export const REPLAY_FLAGS = flagKeys('replayUpdate', 'replayEnv'); +export const REPLAY_FLAGS = flagKeys('replayUpdate', 'replayEnv', 'replayPublicEnv'); export const COMMON_COMMAND_SUPPORTED_FLAG_KEYS = flagKeys( 'remoteConfig', diff --git a/src/commands/command-flags.ts b/src/commands/command-flags.ts index c8177a4ef8..f13609ec76 100644 --- a/src/commands/command-flags.ts +++ b/src/commands/command-flags.ts @@ -92,6 +92,7 @@ function buildFlags(options: InternalRequestOptions): CommandFlags { replayUpdate: options.replayUpdate, replayBackend: options.replayBackend, replayEnv: options.replayEnv, + replayPublicEnv: options.publicEnv, replayShellEnv: options.replayShellEnv, replayFrom: options.replayFrom, replayPlanDigest: options.replayPlanDigest, diff --git a/src/commands/replay/index.test.ts b/src/commands/replay/index.test.ts index ef46e1e8db..e72ba79f32 100644 --- a/src/commands/replay/index.test.ts +++ b/src/commands/replay/index.test.ts @@ -50,6 +50,7 @@ describe('replay command interface', () => { replayUpdate: true, replayMaestro: true, replayEnv: ['FOO=bar'], + replayPublicEnv: ['TARGET'], metroHost: '127.0.0.1', metroPort: 8083, bundleUrl: 'http://127.0.0.1:8083/index.bundle', @@ -60,6 +61,7 @@ describe('replay command interface', () => { update: true, backend: 'maestro', env: ['FOO=bar'], + publicEnv: ['TARGET'], metroHost: '127.0.0.1', metroPort: 8083, bundleUrl: 'http://127.0.0.1:8083/index.bundle', @@ -78,6 +80,7 @@ describe('replay command interface', () => { replayUpdate: true, replayMaestro: true, replayEnv: ['FOO=bar'], + replayPublicEnv: ['TARGET'], metroHost: '127.0.0.1', metroPort: 8083, bundleUrl: 'http://127.0.0.1:8083/index.bundle', @@ -95,6 +98,7 @@ describe('replay command interface', () => { update: true, backend: 'maestro', env: ['FOO=bar'], + publicEnv: ['TARGET'], metroHost: '127.0.0.1', metroPort: 8083, bundleUrl: 'http://127.0.0.1:8083/index.bundle', @@ -116,6 +120,7 @@ describe('replay command interface', () => { update: true, backend: 'maestro', env: ['FOO=bar'], + publicEnv: ['TARGET'], }), ).toMatchObject({ command: 'replay', @@ -124,6 +129,7 @@ describe('replay command interface', () => { replayUpdate: true, replayBackend: 'maestro', replayEnv: ['FOO=bar'], + replayPublicEnv: ['TARGET'], replayShellEnv: { AD_VAR_REPLAY_TEST: 'enabled' }, }, }); diff --git a/src/commands/replay/index.ts b/src/commands/replay/index.ts index 7177d9b507..def5dbe0e0 100644 --- a/src/commands/replay/index.ts +++ b/src/commands/replay/index.ts @@ -39,6 +39,7 @@ export const replayCommandMetadata = defineFieldCommandMetadata( backend: stringField(), maestro: booleanField(), env: stringArrayField(), + publicEnv: stringArrayField(), metroHost: stringField('Metro/debug host hint inherited by replay-opened sessions.'), metroPort: integerField('Metro/debug port hint inherited by replay-opened sessions.'), bundleUrl: stringField('Bundle URL hint inherited by replay-opened sessions.'), @@ -68,6 +69,7 @@ export const testCommandMetadata = defineFieldCommandMetadata( backend: stringField(), maestro: booleanField(), env: stringArrayField(), + publicEnv: stringArrayField(), metroHost: stringField('Metro/debug host hint inherited by each test session.'), metroPort: integerField('Metro/debug port hint inherited by each test session.'), bundleUrl: stringField('Bundle URL hint inherited by each test session.'), @@ -139,6 +141,7 @@ export const replayCliReader: CliReader = (positionals, flags) => ({ update: flags.replayUpdate, backend: flags.replayMaestro ? 'maestro' : undefined, env: flags.replayEnv, + publicEnv: flags.replayPublicEnv, metroHost: flags.metroHost, metroPort: flags.metroPort, bundleUrl: flags.bundleUrl, @@ -154,6 +157,7 @@ export const testCliReader: CliReader = (positionals, flags) => ({ update: flags.replayUpdate, backend: flags.replayMaestro ? 'maestro' : undefined, env: flags.replayEnv, + publicEnv: flags.replayPublicEnv, metroHost: flags.metroHost, metroPort: flags.metroPort, bundleUrl: flags.bundleUrl, @@ -172,6 +176,7 @@ export const replayDaemonWriter: DaemonWriter = (input) => replayUpdate: input.update, replayBackend: readReplayBackend(input), replayEnv: input.env, + replayPublicEnv: input.publicEnv, replayShellEnv: collectReplayClientShellEnv(process.env), replayFrom: input.resumeFrom, replayPlanDigest: input.resumePlanDigest, @@ -184,6 +189,7 @@ export const testDaemonWriter: DaemonWriter = (input) => replayUpdate: input.update, replayBackend: readReplayBackend(input), replayEnv: input.env, + replayPublicEnv: input.publicEnv, replayShellEnv: collectReplayClientShellEnv(process.env), }); diff --git a/src/compat/maestro/__tests__/engine-context.test.ts b/src/compat/maestro/__tests__/engine-context.test.ts index d15e5cad58..ac1fb83d38 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('redacts transitive scoped variables unless explicitly public', () => { const context = createMaestroExecutionContext(); const leave = context.enter({ TARGET: '${NEXT}', @@ -13,11 +14,15 @@ test('resolves transitive scoped variables to their final value', () => { }); expect(context.resolve('${TARGET}')).toBe('Done'); - expect(context.expandedVariables).toEqual({ TARGET: 'Done' }); + expect(context.redactionVariables).toEqual([ + { name: 'FINAL', value: 'Done' }, + { name: 'NEXT', value: 'Done' }, + { name: 'TARGET', value: 'Done' }, + ]); leave(); }); -test('retains expanded values after nested scopes unwind', () => { +test('retains shadowed non-public values after nested scopes unwind', () => { const context = createMaestroExecutionContext(); const rootLeave = context.enter({ SECRET: 'nested-scope-secret' }); const nestedLeave = context.enter({ TARGET: '${SECRET}' }); @@ -26,9 +31,36 @@ test('retains expanded values after nested scopes unwind', () => { nestedLeave(); rootLeave(); - expect(context.expandedVariables).toEqual({ - TARGET: 'nested-scope-secret', - }); + expect(context.redactionVariables).toEqual([ + { name: 'SECRET', value: 'nested-scope-secret' }, + { name: 'TARGET', value: 'nested-scope-secret' }, + ]); +}); + +test('omits explicitly public variable values from the redaction set', () => { + const context = createMaestroExecutionContext({}, { TARGET: 'Continue checkout' }, ['TARGET']); + + expect(context.resolve('${TARGET}')).toBe('Continue checkout'); + expect(context.redactionVariables).toEqual([]); +}); + +test('scrubs optional-step warnings before they enter a successful result', async () => { + const sentinel = 'optional-maestro-secret'; + const program = parseMaestroProgram( + ['---', '- tapOn:', ' text: ${SECRET}', ' optional: true'].join('\n'), + { sourcePath: '/flows/optional.yaml' }, + ); + const port: MaestroRuntimePort = { + execute: vi.fn(async () => { + throw maestroTestFailure(`Missing ${sentinel}`); + }), + observe: vi.fn(async ({ generation }) => ({ generation, matched: true })), + }; + + const result = await executeMaestroProgram(program, port, { env: { SECRET: sentinel } }); + + expect(result.warnings).toEqual([expect.stringContaining('')]); + expect(JSON.stringify(result.warnings)).not.toContain(sentinel); }); test('rejects cyclic references instead of recursing indefinitely', () => { diff --git a/src/compat/maestro/engine-context.ts b/src/compat/maestro/engine-context.ts index 910a1e61b1..f0b2bc740b 100644 --- a/src/compat/maestro/engine-context.ts +++ b/src/compat/maestro/engine-context.ts @@ -1,21 +1,28 @@ import { AppError } from '../../kernel/errors.ts'; -import type { MaestroObservation } from './engine-types.ts'; +import type { MaestroObservation, MaestroRedactionVariable } from './engine-types.ts'; export type MaestroExecutionContext = ReturnType; export function createMaestroExecutionContext( defaults: Record = {}, runtimeOverrides: Record = {}, + publicVariableNames: Iterable = [], + onRedactionVariable?: (entry: MaestroRedactionVariable) => void, ) { const overrides = { ...runtimeOverrides }; + const publicNames = new Set(publicVariableNames); // Flow config and runFlow env values are stack-scoped; script output variables persist. let persistentValues = stringifyValues(defaults); const scopes: Record[] = []; - const expandedValues = new Map(); + const redactionValues = new Map>(); let cachedValues: Readonly> | undefined; let generation = 0; let observation: MaestroObservation | undefined; + for (const [name, value] of Object.entries(overrides)) { + recordSensitiveValue(name, value); + } + return { get values(): Readonly> { return currentValues(); @@ -26,8 +33,10 @@ export function createMaestroExecutionContext( get observation(): MaestroObservation | undefined { return observation?.generation === generation ? observation : undefined; }, - get expandedVariables(): Readonly> { - return Object.fromEntries(expandedValues); + get redactionVariables(): readonly MaestroRedactionVariable[] { + return [...redactionValues].flatMap(([name, values]) => + [...values].map((value) => ({ name, value })), + ); }, enter(scopedValues: Record = {}): () => void { const resolved = resolveScopedValues(scopedValues); @@ -62,11 +71,17 @@ export function createMaestroExecutionContext( observation = undefined; }, resolve(value: string): string { - return resolveValue(value, currentValues(), recordExpandedValue); + return resolveValue(value, currentValues(), recordSensitiveValue); }, resolveDeferred(value: string): string { return resolveValue(value, currentValues(), undefined, new Set(), false); }, + redact(value: string): string { + return [...redactionValues] + .flatMap(([name, values]) => [...values].map((entry) => ({ name, value: entry }))) + .sort((left, right) => right.value.length - left.value.length) + .reduce((result, entry) => result.replaceAll(entry.value, ``), value); + }, }; function currentValues(): Readonly> { @@ -92,16 +107,25 @@ export function createMaestroExecutionContext( ...resolved, ...overrides, }, - undefined, + recordSensitiveValue, new Set(), false, ); + recordSensitiveValue(key, resolved[key]); } return resolved; } - function recordExpandedValue(name: string, value: string): void { - expandedValues.set(name, value); + function recordSensitiveValue(name: string, value: string): void { + if (publicNames.has(name) || value.length === 0) return; + let values = redactionValues.get(name); + if (!values) { + values = new Set(); + redactionValues.set(name, values); + } + if (values.has(value)) return; + values.add(value); + onRedactionVariable?.({ name, value }); } } diff --git a/src/compat/maestro/engine-types.ts b/src/compat/maestro/engine-types.ts index 4af22839ff..2314978e81 100644 --- a/src/compat/maestro/engine-types.ts +++ b/src/compat/maestro/engine-types.ts @@ -5,6 +5,11 @@ import type { MaestroSelector, MaestroSourceLocation, } from './program-ir.ts'; + +export type MaestroRedactionVariable = { + readonly name: string; + readonly value: string; +}; export type MaestroControlCommand = Extract< MaestroCommand, { kind: 'runFlow' | 'repeat' | 'retry' } @@ -137,7 +142,8 @@ export type MaestroEngineObserver = { runtimeMetrics?: MaestroRuntimeMetrics; error: unknown; artifactPaths: readonly string[]; - expandedVariables: Readonly>; + /** Expanded non-public values that must be scrubbed from diagnostics. */ + redactionVariables: readonly MaestroRedactionVariable[]; }, ): void; }; @@ -145,6 +151,8 @@ export type MaestroEngineObserver = { export type MaestroEngineOptions = { /** Highest-precedence invocation values, normally CLI over shell. */ env?: Readonly>; + /** Names whose resolved values may be rendered in failure diagnostics. */ + publicVariableNames?: Iterable; /** Lowest-precedence defaults, normally replay built-ins. */ defaults?: Readonly>; platform?: MaestroPlatform; @@ -170,4 +178,6 @@ export type MaestroEngineResult = { generation: number; artifactPaths: string[]; warnings?: string[]; + /** Non-public values observed during execution, retained only for response scrubbing. */ + redactionVariables: readonly MaestroRedactionVariable[]; }; diff --git a/src/compat/maestro/replay-plan-execution.ts b/src/compat/maestro/replay-plan-execution.ts index edcef29cc0..1c58cc07fc 100644 --- a/src/compat/maestro/replay-plan-execution.ts +++ b/src/compat/maestro/replay-plan-execution.ts @@ -33,7 +33,11 @@ export async function executeMaestroReplayPlan( plan, port, options, - context: createMaestroExecutionContext(options.defaults, options.env ? { ...options.env } : {}), + context: createMaestroExecutionContext( + options.defaults, + options.env ? { ...options.env } : {}, + options.publicVariableNames, + ), timing: DEFAULT_MAESTRO_COMPATIBILITY_TIMING_POLICY, artifacts: new Set(), warnings: [], @@ -52,6 +56,7 @@ export async function executeMaestroReplayPlan( skipped: state.skipped, generation: state.context.generation, artifactPaths: [...state.artifacts], + redactionVariables: state.context.redactionVariables, ...(state.warnings.length > 0 ? { warnings: state.warnings } : {}), }; } @@ -96,7 +101,7 @@ async function executeObservedStep( ...runtimeMetricsDelta(metricsBefore, state.port.readMetrics?.()), error: failure.error, artifactPaths: [...state.artifacts], - expandedVariables: state.context.expandedVariables, + redactionVariables: state.context.redactionVariables, }), ); throw failure; diff --git a/src/compat/maestro/replay-plan-step-execution.ts b/src/compat/maestro/replay-plan-step-execution.ts index 0d16959720..1abbda5810 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(state.context.redact(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/replay-plan-steps.ts b/src/compat/maestro/replay-plan-steps.ts index a1265407b4..d96aff54c0 100644 --- a/src/compat/maestro/replay-plan-steps.ts +++ b/src/compat/maestro/replay-plan-steps.ts @@ -42,7 +42,12 @@ export async function compileMaestroReplayPlanSteps( const rootPath = sourcePathKey(program.source.path); const state: BuildState = { options, - context: createMaestroExecutionContext(options.defaults, options.env), + context: createMaestroExecutionContext( + options.defaults, + options.env, + options.publicVariableNames, + options.onRedactionVariable, + ), activeIncludePaths: new Set(rootPath === undefined ? [] : [rootPath]), staticallyExecutedControls: 0, staticallySkippedControls: 0, diff --git a/src/compat/maestro/replay-plan-types.ts b/src/compat/maestro/replay-plan-types.ts index 4cf9b6e894..9e65929a26 100644 --- a/src/compat/maestro/replay-plan-types.ts +++ b/src/compat/maestro/replay-plan-types.ts @@ -1,6 +1,10 @@ import type { MaestroProgramLoader } from './program-loader.ts'; import type { MaestroPlatform, MaestroSourceLocation } from './program-ir.ts'; -import type { MaestroControlCommandDescriptor, MaestroRuntimeCommand } from './engine-types.ts'; +import type { + MaestroControlCommandDescriptor, + MaestroRedactionVariable, + MaestroRuntimeCommand, +} from './engine-types.ts'; import type { SessionRuntimeHints } from '../../kernel/contracts.ts'; export type MaestroReplayPlanScope = Readonly>; @@ -49,6 +53,10 @@ export type MaestroReplayPlanOptions = { readonly platform?: MaestroPlatform; readonly target?: string; readonly runtimeHints?: Readonly; + /** Explicit names whose resolved values may appear in diagnostics. */ + readonly publicVariableNames?: Iterable; + /** Receives non-public values observed while resolving the static plan. */ + readonly onRedactionVariable?: (entry: MaestroRedactionVariable) => void; readonly loadProgram?: MaestroProgramLoader; readonly signal?: AbortSignal; }; diff --git a/src/compat/maestro/support-matrix.ts b/src/compat/maestro/support-matrix.ts index e38b2646b4..9de06fb418 100644 --- a/src/compat/maestro/support-matrix.ts +++ b/src/compat/maestro/support-matrix.ts @@ -8,7 +8,7 @@ export const MAESTRO_COMPAT_SUPPORTED_CAPABILITIES = [ 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.', + 'Environment: flow env is the default, AD_VAR_* overrides it, and CLI -e KEY=VALUE wins over both. Expanded values are redacted from Maestro failure diagnostics by default; use --public-env KEY only when a value is safe to disclose.', '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/contracts/cli-flags.ts b/src/contracts/cli-flags.ts index d44b6a95c7..5e95eaebfe 100644 --- a/src/contracts/cli-flags.ts +++ b/src/contracts/cli-flags.ts @@ -133,6 +133,8 @@ export type CliFlags = CloudProviderProfileFields & replayUpdate?: boolean; replayMaestro?: boolean; replayEnv?: string[]; + /** Replay/Test: names whose resolved values may appear in Maestro failure diagnostics. */ + replayPublicEnv?: string[]; replayShellEnv?: Record; replayFrom?: number; replayPlanDigest?: string; diff --git a/src/contracts/client-request.ts b/src/contracts/client-request.ts index 85be9ec15d..7d84608d42 100644 --- a/src/contracts/client-request.ts +++ b/src/contracts/client-request.ts @@ -53,6 +53,7 @@ export type CommandExecutionOptions = Partial & { replayUpdate?: boolean; replayBackend?: string; replayEnv?: string[]; + publicEnv?: string[]; replayShellEnv?: Record; replayFrom?: number; replayPlanDigest?: string; 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..3521f87389 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,7 @@ async function buildFailureResponse( durationMs: 12, error: new Error('typed Maestro action failed'), artifactPaths: [], - expandedVariables: {}, + redactionVariables: [], }, plan: makeMaestroPlan(), replayPath: path.join(root, 'flow.yaml'), @@ -75,7 +75,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 +92,12 @@ test('typed Maestro failure projection is report-only and preserves authored pro durationMs: 12, error: new Error('tap failed'), artifactPaths: [], - expandedVariables: {}, + redactionVariables: [], }, 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({ @@ -145,7 +145,10 @@ test('typed Maestro failure diagnostics scrub expanded selector values', async ( const response = await runReplayScriptFile({ req: baseReq({ positionals: [flowPath], - flags: { replayBackend: 'maestro', replayEnv: [`TARGET=${sentinel}`] }, + flags: { + replayBackend: 'maestro', + replayEnv: [`TARGET=${sentinel}`], + }, }), sessionName, logPath: path.join(root, 'daemon.log'), @@ -174,10 +177,79 @@ test('typed Maestro failure diagnostics scrub expanded selector values', async ( cause: { message: string; hint?: string }; suggestions: unknown[]; }; - expect(divergence.action).toBe('tapOn "${TARGET}"'); + expect(divergence.action).toBe('tapOn ""'); expect(divergence.cause.message).toContain(''); expect(divergence.cause.hint).toContain(''); - expect(divergence.suggestions).toEqual([]); + expect(divergence.suggestions).toEqual([ + expect.objectContaining({ + label: '', + selector: expect.stringContaining(''), + }), + ]); +}); + +test('typed Maestro failure diagnostics retain non-sensitive expanded selector values', 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 label = 'Continue checkout'; + fs.writeFileSync( + flowPath, + ['appId: com.example.app', '---', '- tapOn: ${TARGET}', ''].join('\n'), + ); + + const response = await runReplayScriptFile({ + req: baseReq({ + positionals: [flowPath], + flags: { + replayBackend: 'maestro', + replayEnv: [`TARGET=${label}`], + replayPublicEnv: ['TARGET'], + }, + }), + sessionName, + logPath: path.join(root, 'daemon.log'), + sessionStore, + invoke: async (req) => { + 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 ${label}`, + hint: `Find ${label}`, + }, + }; + } + return { ok: true, data: {} }; + }, + }); + + expect(response.ok).toBe(false); + if (response.ok) return; + 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 () => { @@ -276,6 +348,35 @@ test('typed Maestro nested scopes scrub failure values after unwind and keep ret ]); }); +test('typed Maestro scrubs 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 sentinel = 'flow-local-maestro-secret'; + fs.writeFileSync( + flowPath, + ['env:', ` SECRET: ${sentinel}`, '---', '- runFlow: ${SECRET}.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)).not.toContain(sentinel); + if (response.ok) return; + expect(response.error.message).toContain(''); +}); + 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-vars.test.ts b/src/daemon/handlers/__tests__/session-replay-vars.test.ts index f852d813f9..98257e9ec2 100644 --- a/src/daemon/handlers/__tests__/session-replay-vars.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-vars.test.ts @@ -51,6 +51,7 @@ import { buildReplayVarScope, collectReplayShellEnv, parseReplayCliEnvEntries, + parseReplayPublicEnvNames, resolveReplayAction, resolveReplayString, } from '../../../replay/vars.ts'; @@ -271,6 +272,15 @@ test('parseReplayCliEnvEntries splits KEY=VALUE and rejects invalid keys', () => assert.throws(() => parseReplayCliEnvEntries(['=value']), AppError); }); +test('parseReplayPublicEnvNames accepts replay variable names without inferring sensitivity', () => { + assert.deepEqual(parseReplayPublicEnvNames(['TARGET', 'API_TOKEN', 'TARGET']), [ + 'TARGET', + 'API_TOKEN', + ]); + assert.throws(() => parseReplayPublicEnvNames(['target']), AppError); + assert.throws(() => parseReplayPublicEnvNames(['AD_TOKEN']), AppError); +}); + test('resolveReplayAction walks positionals and string flags', () => { const action: SessionAction = { ts: 0, @@ -473,6 +483,20 @@ test('parseReplayCliEnvEntries error wording is user-friendly for invalid keys', ); }); +test('rejects --public-env outside Maestro YAML replay', async () => { + const { response } = await runReplayFixture({ + label: 'public-env-native-replay', + script: 'snapshot\n', + flags: { replayPublicEnv: ['TARGET'] }, + }); + + assert.equal(response.ok, false); + if (!response.ok) { + assert.equal(response.error.code, 'INVALID_ARGS'); + assert.match(response.error.message, /--public-env.*Maestro YAML/); + } +}); + // fallow-ignore-next-line complexity test('runReplayScriptFile dispatches resolved literals with file env overridden by CLI', async () => { const { response, calls } = await runReplayFixture({ diff --git a/src/daemon/handlers/session-replay-maestro-failure.ts b/src/daemon/handlers/session-replay-maestro-failure.ts index c15991d9b7..feac1a7c16 100644 --- a/src/daemon/handlers/session-replay-maestro-failure.ts +++ b/src/daemon/handlers/session-replay-maestro-failure.ts @@ -1,5 +1,6 @@ import { isMaestroControlCommandDescriptor, + type MaestroRedactionVariable, type MaestroEngineEvent, } from '../../compat/maestro/engine-types.ts'; import { formatMaestroCommandProgress } from '../../compat/maestro/progress.ts'; @@ -41,7 +42,7 @@ export type MaestroFailedEngineEvent = MaestroEngineEvent & { readonly durationMs: number; readonly error: unknown; readonly artifactPaths: readonly string[]; - readonly expandedVariables: Readonly>; + readonly redactionVariables: readonly MaestroRedactionVariable[]; }; export type MaestroFailureReportAction = Pick< @@ -50,7 +51,8 @@ export type MaestroFailureReportAction = Pick< >; export type MaestroFailureReportProjection = { - readonly authoredCommand: MaestroEngineEvent['command']; + /** Resolved command at the failing runtime boundary; source remains authored provenance. */ + readonly command: MaestroEngineEvent['command']; readonly source: MaestroEngineEvent['source']; readonly progress: ReturnType; readonly action: MaestroFailureReportAction; @@ -62,7 +64,7 @@ export function buildTypedMaestroFailureReportProjection( ): MaestroFailureReportProjection { const progress = formatMaestroCommandProgress(event.command); return { - authoredCommand: event.command, + command: event.command, source: event.source, progress, action: { @@ -88,8 +90,8 @@ export async function buildTypedMaestroFailureResponse(params: { const report = buildTypedMaestroFailureReportProjection(event, req); const cause = hoistReplayFailureCauseDiagnosticMeta(params.error); const scrubVars = [ - ...collectExpandedScrubVars(event.expandedVariables), - ...collectMaestroTextScrubVars(report.authoredCommand), + ...collectExpandedScrubVars(event.redactionVariables), + ...collectMaestroTextScrubVars(report.command), ].sort((left, right) => right.value.length - left.value.length); const sanitize = createReplayDivergenceSanitizer(scrubVars); const safeCause = { @@ -114,9 +116,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 +130,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 +175,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,10 +335,12 @@ 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 })) +function collectExpandedScrubVars( + values: readonly MaestroRedactionVariable[], +): ReplayVarScrubEntry[] { + return values + .filter(({ value }) => value.length > 0) + .map(({ name, value }) => ({ name, value })) .sort((left, right) => right.value.length - left.value.length); } diff --git a/src/daemon/handlers/session-replay-maestro-response.ts b/src/daemon/handlers/session-replay-maestro-response.ts index 30fda267c0..a5e7da8646 100644 --- a/src/daemon/handlers/session-replay-maestro-response.ts +++ b/src/daemon/handlers/session-replay-maestro-response.ts @@ -9,9 +9,18 @@ import { type MaestroFailedEngineEvent, } from './session-replay-maestro-failure.ts'; import { errorResponse } from './response.ts'; +import { + scrubReplayVarData, + scrubReplayVarValues, + type ReplayVarScrubEntry, +} from '../../replay/divergence.ts'; export function buildTypedMaestroSuccessResponse(params: { - result: { artifactPaths: string[]; warnings?: string[] }; + result: { + artifactPaths: string[]; + warnings?: string[]; + redactionVariables: readonly ReplayVarScrubEntry[]; + }; plan: MaestroReplayPlan; startIndex: number; startedAt: number; @@ -29,8 +38,16 @@ export function buildTypedMaestroSuccessResponse(params: { healed: 0, session: sessionName, sessionActive: sessionStore.get(sessionName) !== undefined, - artifactPaths: result.artifactPaths, - ...(result.warnings ? { warnings: result.warnings } : {}), + artifactPaths: result.artifactPaths.map((entry) => + scrubReplayVarValues(entry, result.redactionVariables), + ), + ...(result.warnings + ? { + warnings: result.warnings.map((entry) => + scrubReplayVarValues(entry, result.redactionVariables), + ), + } + : {}), ...(snapshotDiagnostics ? { snapshotDiagnostics } : {}), message: replaySuccessMessage(replayed, Date.now() - startedAt), } satisfies ReplayCommandResult, @@ -43,6 +60,7 @@ export async function buildTypedMaestroReplayErrorResponse(params: { state: { failedEvent?: MaestroFailedEngineEvent; plan?: MaestroReplayPlan; + redactionVariables: ReplayVarScrubEntry[]; snapshotStart: number; }; error: unknown; @@ -69,10 +87,17 @@ export async function buildTypedMaestroReplayErrorResponse(params: { ), }); } - return errorResponse(normalizedError.code, normalizedError.message, { - ...(normalizedError.details ?? {}), - ...buildErrorDetails(failedEvent), - }); + return errorResponse( + normalizedError.code, + scrubReplayVarValues(normalizedError.message, params.state.redactionVariables), + { + ...(scrubReplayVarData( + normalizedError.details ?? {}, + params.state.redactionVariables, + ) as Record), + ...buildErrorDetails(failedEvent), + }, + ); } function readSnapshotDiagnostics( diff --git a/src/daemon/handlers/session-replay-maestro-runtime.ts b/src/daemon/handlers/session-replay-maestro-runtime.ts index 0fafe3a865..45627461a5 100644 --- a/src/daemon/handlers/session-replay-maestro-runtime.ts +++ b/src/daemon/handlers/session-replay-maestro-runtime.ts @@ -12,7 +12,9 @@ import { stripUndefined } from '../../utils/parsing.ts'; import { collectReplayShellEnv, parseReplayCliEnvEntries, + parseReplayPublicEnvNames, readReplayCliEnvEntries, + readReplayPublicEnvNames, readReplayShellEnvSource, } from '../../replay/vars.ts'; import { createDaemonMaestroRuntimePort } from '../../compat/maestro/daemon-runtime-port.ts'; @@ -32,6 +34,7 @@ import { SessionStore } from '../session-store.ts'; import { errorResponse } from './response.ts'; import { buildReplayBuiltinVars } from './session-replay-vars.ts'; import type { MaestroFailedEngineEvent } from './session-replay-maestro-failure.ts'; +import type { ReplayVarScrubEntry } from '../../replay/divergence.ts'; import { createMaestroReplayObserver } from './session-replay-maestro-observer.ts'; import { buildTypedMaestroReplayErrorResponse, @@ -53,6 +56,7 @@ type TypedMaestroReplayParams = { type TypedMaestroReplayState = { failedEvent?: MaestroFailedEngineEvent; plan?: MaestroReplayPlan; + redactionVariables: ReplayVarScrubEntry[]; snapshotStart: number; }; @@ -65,6 +69,8 @@ type TypedMaestroReplayContext = { runtimeHints: ReturnType; defaults: Record; env: Record; + publicVariableNames: string[]; + redactionVariables: ReplayVarScrubEntry[]; signal: AbortSignal | undefined; loadProgram: ReturnType; }; @@ -87,7 +93,7 @@ export async function runTypedMaestroReplayFile( ); } const startedAt = Date.now(); - const state: TypedMaestroReplayState = { snapshotStart: 0 }; + const state: TypedMaestroReplayState = { snapshotStart: 0, redactionVariables: [] }; try { return await executeTypedMaestroReplay({ ...params, @@ -114,6 +120,7 @@ async function executeTypedMaestroReplay( ): Promise { const { req, sessionName, sessionStore, tracePath, invoke, state } = params; const context = await prepareTypedMaestroReplay(params); + state.redactionVariables = context.redactionVariables; const plan = await compileMaestroReplayPlan(context.program, { defaults: context.defaults, env: context.env, @@ -122,6 +129,8 @@ async function executeTypedMaestroReplay( runtimeHints: context.runtimeHints, loadProgram: context.loadProgram, signal: context.signal, + publicVariableNames: context.publicVariableNames, + onRedactionVariable: (entry) => addRedactionVariable(state.redactionVariables, entry), }); state.plan = plan; const startIndex = resolveMaestroReplayStartIndex(plan, { @@ -143,6 +152,7 @@ async function executeTypedMaestroReplay( const result = await executeMaestroPlan(plan, port, { defaults: context.defaults, env: context.env, + publicVariableNames: context.publicVariableNames, platform: context.platform, target: context.target, loadProgram: context.loadProgram, @@ -184,6 +194,7 @@ async function prepareTypedMaestroReplay( session, program, }); + const runtimeEnv = buildTypedMaestroEnv(req); return { filePath, program, @@ -195,7 +206,9 @@ async function prepareTypedMaestroReplay( platform: binding.platform, target: binding.target, }), - env: buildTypedMaestroEnv(req), + env: runtimeEnv.values, + publicVariableNames: runtimeEnv.publicVariableNames, + redactionVariables: runtimeEnv.redactionVariables, signal: getRequestSignal(req.meta?.requestId), loadProgram: createMaestroProgramLoader(path.dirname(filePath)), }; @@ -294,11 +307,31 @@ function buildTypedMaestroDefaults(params: { }; } -function buildTypedMaestroEnv(req: DaemonRequest): Record { - return { +function addRedactionVariable(entries: ReplayVarScrubEntry[], entry: ReplayVarScrubEntry): void { + if (entries.some((current) => current.name === entry.name && current.value === entry.value)) + return; + entries.push(entry); +} + +function buildTypedMaestroEnv(req: DaemonRequest): { + values: Record; + publicVariableNames: string[]; + redactionVariables: ReplayVarScrubEntry[]; +} { + const values = { ...collectReplayShellEnv(readReplayShellEnvSource(req.flags?.replayShellEnv)), ...parseReplayCliEnvEntries(readReplayCliEnvEntries(req.flags?.replayEnv)), }; + const publicVariableNames = parseReplayPublicEnvNames( + readReplayPublicEnvNames(req.flags?.replayPublicEnv), + ); + return { + values, + publicVariableNames, + redactionVariables: Object.entries(values) + .filter(([name, value]) => value.length > 0 && !publicVariableNames.includes(name)) + .map(([name, value]) => ({ name, value })), + }; } function createMaestroReplayPort(params: { diff --git a/src/daemon/handlers/session-replay-runtime-failure-response.ts b/src/daemon/handlers/session-replay-runtime-failure-response.ts index 7993d81ba7..712389935f 100644 --- a/src/daemon/handlers/session-replay-runtime-failure-response.ts +++ b/src/daemon/handlers/session-replay-runtime-failure-response.ts @@ -1,4 +1,8 @@ -import { scrubReplayVarValues, type ReplayVarScrubEntry } from '../../replay/divergence.ts'; +import { + scrubReplayVarData, + scrubReplayVarValues, + type ReplayVarScrubEntry, +} from '../../replay/divergence.ts'; import { formatDivergenceActionLabel } from '../../replay/script-utils.ts'; import type { SnapshotDiagnosticsSummary } from '../../contracts/snapshot-diagnostics.ts'; import { buildDisplayPositionals } from '../session-event-action.ts'; @@ -89,12 +93,15 @@ export function buildReplayDivergenceFailureResponseFromDescriptor(params: { ...(error.retriable !== undefined ? { retriable: error.retriable } : {}), ...(error.supportedOn !== undefined ? { supportedOn: error.supportedOn } : {}), details: { - ...pickSafeCauseDetails(error.details), - replayPath, + ...(scrubReplayVarData(pickSafeCauseDetails(error.details), scrubVars) as Record< + string, + unknown + >), + replayPath: scrubReplayVarValues(replayPath, scrubVars), step, action, - positionals, - artifactPaths, + positionals: positionals.map((value) => scrubReplayVarValues(value, scrubVars)), + artifactPaths: artifactPaths.map((value) => scrubReplayVarValues(value, scrubVars)), ...(snapshotDiagnostics ? { snapshotDiagnostics } : {}), divergence, }, diff --git a/src/daemon/handlers/session-replay-runtime.ts b/src/daemon/handlers/session-replay-runtime.ts index cde7a6181b..05d1fb5e3f 100644 --- a/src/daemon/handlers/session-replay-runtime.ts +++ b/src/daemon/handlers/session-replay-runtime.ts @@ -514,7 +514,15 @@ async function runTypedReplayIfNeeded(params: { invoke: DaemonInvokeFn; resolved: string; }): Promise { - if (!isTypedMaestroReplay(params.req, params.resolved)) return undefined; + if (!isTypedMaestroReplay(params.req, params.resolved)) { + if ((params.req.flags?.replayPublicEnv?.length ?? 0) > 0) { + return errorResponse( + 'INVALID_ARGS', + '--public-env is supported only with Maestro YAML replay (--maestro).', + ); + } + return undefined; + } if (params.sessionStore.get(params.sessionName)?.saveScriptBoundary !== undefined) { return errorResponse( 'INVALID_ARGS', diff --git a/src/daemon/request-router.ts b/src/daemon/request-router.ts index 497541c4b8..941940cdfd 100644 --- a/src/daemon/request-router.ts +++ b/src/daemon/request-router.ts @@ -48,6 +48,12 @@ import { canRunReplayScopedAction } from './daemon-command-registry.ts'; import { createAgentBrowserWebProvider } from '../platforms/web/agent-browser-provider.ts'; import { openWebSessionNames } from './web-session-names.ts'; import { inferFillText } from './action-utils.ts'; +import { + collectReplayShellEnv, + parseReplayCliEnvEntries, + readReplayCliEnvEntries, + readReplayShellEnvSource, +} from '../replay/vars.ts'; // --------------------------------------------------------------------------- // Request handler API @@ -141,6 +147,7 @@ export function createRequestHandler(deps: RequestRouterDeps): DaemonInvokeFn { return unauthorizedResponse(); } registerParameterizedFillDiagnosticValue(req); + registerReplayVariableDiagnosticValues(req); const invalidRecordingFlags = recordingFlagsResponse(req); if (invalidRecordingFlags) return invalidRecordingFlags; @@ -255,6 +262,7 @@ export function createRequestHandler(deps: RequestRouterDeps): DaemonInvokeFn { return unauthorizedResponse(); } registerParameterizedFillDiagnosticValue(req); + registerReplayVariableDiagnosticValues(req); let childScope: RequestExecutionScope | undefined; try { @@ -328,6 +336,19 @@ function registerParameterizedFillDiagnosticValue(req: DaemonRequest): void { ); } +/** + * Replay variables are fail-closed diagnostic values. Register them before + * request/session binding so nested dispatch and backend work inherit the + * redaction boundary even when no Maestro failure report is produced. + */ +function registerReplayVariableDiagnosticValues(req: DaemonRequest): void { + const values = { + ...collectReplayShellEnv(readReplayShellEnvSource(req.flags?.replayShellEnv)), + ...parseReplayCliEnvEntries(readReplayCliEnvEntries(req.flags?.replayEnv)), + }; + for (const value of Object.values(values)) registerDiagnosticSensitiveValue(value); +} + async function dispatchGenericForLockedScope(params: { lockedScope: LockedRequestScope; logPath: string; diff --git a/src/replay/__tests__/divergence.test.ts b/src/replay/__tests__/divergence.test.ts index b8c5f81eca..eede7e316a 100644 --- a/src/replay/__tests__/divergence.test.ts +++ b/src/replay/__tests__/divergence.test.ts @@ -9,6 +9,7 @@ import { REPLAY_DIVERGENCE_DIGEST_REF_LIMIT, REPLAY_DIVERGENCE_LEVEL_BYTE_LIMITS, REPLAY_DIVERGENCE_SUGGESTION_LIMIT, + scrubReplayVarData, truncateUtf8Field, type ReplayDivergence, } from '../divergence.ts'; @@ -236,6 +237,15 @@ test('sanitizeReplayDivergenceField redacts sensitive content even when no trunc assert.ok(sanitized.includes('[REDACTED]')); }); +test('scrubReplayVarData scrubs variable values from normalized detail values and keys', () => { + assert.deepEqual( + scrubReplayVarData({ 'secret-value': ['prefix secret-value', { nested: 'secret-value' }] }, [ + { name: 'TOKEN', value: 'secret-value' }, + ]), + { '': ['prefix ', { nested: '' }] }, + ); +}); + // --- Text report carries the repair data (bounded refs + unavailable hint) --- test('formatReplayDivergenceReport lists a bounded ref/role/label subset for an available screen', async () => { diff --git a/src/replay/divergence.ts b/src/replay/divergence.ts index 7164103221..1dd9db33a5 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; @@ -273,9 +276,25 @@ export function scrubReplayVarValues(value: string, entries: ReplayVarScrubEntry return output; } +/** Scrubs runtime values from arbitrary normalized error detail payloads, including object keys. */ +export function scrubReplayVarData( + value: unknown, + entries: readonly ReplayVarScrubEntry[], +): unknown { + if (typeof value === 'string') return scrubReplayVarValues(value, entries); + if (Array.isArray(value)) return value.map((entry) => scrubReplayVarData(entry, entries)); + if (!value || typeof value !== 'object') return value; + return Object.fromEntries( + Object.entries(value).map(([key, entry]) => [ + scrubReplayVarValues(key, entries), + scrubReplayVarData(entry, entries), + ]), + ); +} + /** 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..bec446c4a5 100644 --- a/src/replay/test/__tests__/progress.test.ts +++ b/src/replay/test/__tests__/progress.test.ts @@ -213,6 +213,45 @@ test('createReplayTestProgressRenderer trims live step progress by visible colum }); }); +test('createReplayTestProgressRenderer clears every reflowed row after a terminal resize', () => { + let columns = 80; + const renderer = createReplayTestProgressRenderer({ + liveProgress: true, + columns: () => columns, + }); + 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', + }, + }); + + columns = 20; + 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', + }, + }); + + assert.ok(rendered?.text.includes('\u001B[1A')); + assert.ok(rendered?.text.endsWith('...')); +}); + test('createReplayTestProgressRenderer colors completed result markers when color is enabled', () => { withForcedColor(() => { assert.equal( diff --git a/src/replay/test/progress.ts b/src/replay/test/progress.ts index fe56e117dd..909cb0557f 100644 --- a/src/replay/test/progress.ts +++ b/src/replay/test/progress.ts @@ -13,7 +13,8 @@ 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); }; export type ReplayTestProgressRender = { @@ -39,37 +40,55 @@ 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, + ); + 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)}${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 +282,24 @@ 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'], +): string { + const rows = Math.max(1, Math.ceil(previousWidth / resolveColumns(columns))); + 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..dd8714e74e 100644 --- a/src/replay/test/reporters/default.ts +++ b/src/replay/test/reporters/default.ts @@ -45,7 +45,7 @@ export function createDefaultReplayTestReporter(): ReplayTestReporter { progressRenderer ??= createReplayTestProgressRenderer({ verbose: context.verbose, liveProgress: shouldUseLiveProgress(context), - columns: context.stderr.columns, + columns: () => context.stderr.columns, }); const output = progressRenderer.render(event); if (!output) return; diff --git a/src/replay/vars.ts b/src/replay/vars.ts index 07c8f7c4cb..c7c7411cd8 100644 --- a/src/replay/vars.ts +++ b/src/replay/vars.ts @@ -93,12 +93,40 @@ export function parseReplayCliEnvEntries(entries: readonly string[]): Record(); + for (const entry of entries) { + if (!REPLAY_VAR_KEY_RE.test(entry)) { + throw new AppError( + 'INVALID_ARGS', + `Invalid --public-env name "${entry}": names must use uppercase letters, digits, and underscores (e.g. TARGET).`, + ); + } + if (isReservedNamespaceKey(entry)) { + throw reservedNamespaceError(entry); + } + names.add(entry); + } + return [...names]; +} + export function readReplayCliEnvEntries(raw: unknown): string[] { return Array.isArray(raw) ? raw.filter((value): value is string => typeof value === 'string') : []; } +export function readReplayPublicEnvNames(raw: unknown): string[] { + return Array.isArray(raw) + ? raw.filter((value): value is string => typeof value === 'string') + : []; +} + export function readReplayShellEnvSource(raw: unknown): NodeJS.ProcessEnv { if (raw && typeof raw === 'object' && !Array.isArray(raw)) { const result: NodeJS.ProcessEnv = {}; From 1b2a98c091a318560fca5523ab30bdd2e1a6462d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 28 Jul 2026 14:15:39 +0200 Subject: [PATCH 2/5] refactor(replay): keep Maestro env API compatible --- docs/adr/0012-interactive-replay.md | 16 ++- scripts/integration-progress-model.ts | 3 +- .../__tests__/args-parse-session.test.ts | 13 +- .../cli-grammar/flag-definitions-workflow.ts | 9 -- src/commands/cli-grammar/flag-groups.ts | 2 +- src/commands/command-flags.ts | 1 - src/commands/replay/index.test.ts | 6 - src/commands/replay/index.ts | 6 - .../__tests__/daemon-runtime-port.test.ts | 41 ++++++ .../maestro/__tests__/engine-context.test.ts | 39 ++---- src/compat/maestro/daemon-runtime-port.ts | 3 +- src/compat/maestro/engine-context.ts | 40 ++---- src/compat/maestro/engine-types.ts | 12 +- src/compat/maestro/replay-plan-execution.ts | 9 +- .../maestro/replay-plan-step-execution.ts | 2 +- src/compat/maestro/replay-plan-steps.ts | 7 +- src/compat/maestro/replay-plan-types.ts | 10 +- src/compat/maestro/support-matrix.ts | 2 +- src/contracts/cli-flags.ts | 2 - src/contracts/client-request.ts | 1 - .../session-replay-maestro-failure.test.ts | 124 ++++-------------- .../__tests__/session-replay-vars.test.ts | 24 ---- .../session-replay-maestro-failure.ts | 17 +-- .../session-replay-maestro-response.ts | 39 +----- .../session-replay-maestro-runtime.ts | 41 +----- src/daemon/handlers/session-replay-runtime.ts | 10 +- src/daemon/request-router.ts | 1 + src/replay/vars.ts | 28 ---- website/docs/docs/replay-e2e.md | 3 + 29 files changed, 129 insertions(+), 382 deletions(-) diff --git a/docs/adr/0012-interactive-replay.md b/docs/adr/0012-interactive-replay.md index 9d92a2d640..3f38d7ba66 100644 --- a/docs/adr/0012-interactive-replay.md +++ b/docs/adr/0012-interactive-replay.md @@ -481,13 +481,15 @@ 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 and arbitrary nested cause details -are never serialized. Maestro keeps expanded variables redacted in typed action/source provenance by default, -so a failure never serializes an injected or flow-local value. A caller may explicitly disclose a -non-sensitive runtime value with `--public-env KEY`; names and value shapes never imply sensitivity. Every -other resolved value is removed from rendered strings, optional-step warnings, diagnostics, and overflow -artifacts, including aliases and nested `runFlow` scopes, before the central diagnostics redactor and -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 registered with the request diagnostics redactor before platform work, independently of this +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. 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/scripts/integration-progress-model.ts b/scripts/integration-progress-model.ts index 162b984b85..e03220e141 100644 --- a/scripts/integration-progress-model.ts +++ b/scripts/integration-progress-model.ts @@ -298,7 +298,7 @@ function summarizeProviderScenarioFlagExclusions() { ], }, { - name: 'parser, client, and diagnostic-only command flags', + name: 'parser/client-only command flags', owner: 'args, CLI, debug-symbols, screenshot-diff, and batch tests', keys: [ 'artifact', @@ -311,7 +311,6 @@ function summarizeProviderScenarioFlagExclusions() { 'reporter', 'reportJunit', 'replayMaestro', - 'replayPublicEnv', 'recordVideo', 'shardAll', 'shardSplit', diff --git a/src/cli/parser/__tests__/args-parse-session.test.ts b/src/cli/parser/__tests__/args-parse-session.test.ts index d9c0d0f10b..296df0c5ca 100644 --- a/src/cli/parser/__tests__/args-parse-session.test.ts +++ b/src/cli/parser/__tests__/args-parse-session.test.ts @@ -205,24 +205,13 @@ test('parseArgs recognizes command-specific flag combinations', async () => { }, { label: 'replay maestro flow', - argv: [ - 'replay', - './flow.yaml', - '--maestro', - '--env', - 'USER=Ada', - '--public-env', - 'PASSWORD', - '--timeout', - '240000', - ], + argv: ['replay', './flow.yaml', '--maestro', '--env', 'USER=Ada', '--timeout', '240000'], strictFlags: true, assertParsed: (parsed) => { assert.equal(parsed.command, 'replay'); assert.deepEqual(parsed.positionals, ['./flow.yaml']); assert.equal(parsed.flags.replayMaestro, true); assert.deepEqual(parsed.flags.replayEnv, ['USER=Ada']); - assert.deepEqual(parsed.flags.replayPublicEnv, ['PASSWORD']); assert.equal(parsed.flags.timeoutMs, 240000); }, }, diff --git a/src/commands/cli-grammar/flag-definitions-workflow.ts b/src/commands/cli-grammar/flag-definitions-workflow.ts index 9280fb6a44..af6d3ec288 100644 --- a/src/commands/cli-grammar/flag-definitions-workflow.ts +++ b/src/commands/cli-grammar/flag-definitions-workflow.ts @@ -48,15 +48,6 @@ export const WORKFLOW_FLAG_DEFINITIONS: readonly FlagDefinition[] = [ usageDescription: 'Replay/Test: inject or override a ${KEY} variable for the script (repeatable)', }, - { - key: 'replayPublicEnv', - names: ['--public-env'], - type: 'string', - multiple: true, - usageLabel: '--public-env KEY', - usageDescription: - "Replay/Test: allow this Maestro variable's resolved value in failures (repeatable); all expanded values stay redacted unless explicitly public", - }, { key: 'failFast', names: ['--fail-fast'], diff --git a/src/commands/cli-grammar/flag-groups.ts b/src/commands/cli-grammar/flag-groups.ts index 26b92344ef..1b3bd9a0e9 100644 --- a/src/commands/cli-grammar/flag-groups.ts +++ b/src/commands/cli-grammar/flag-groups.ts @@ -43,7 +43,7 @@ export const REPEATED_TOUCH_FLAGS = flagKeys( // (flag-sourced budget on the interaction descriptors, mirroring wait's // positional budget). export const SETTLE_FLAGS = flagKeys('settle', 'settleQuietMs', 'timeoutMs'); -export const REPLAY_FLAGS = flagKeys('replayUpdate', 'replayEnv', 'replayPublicEnv'); +export const REPLAY_FLAGS = flagKeys('replayUpdate', 'replayEnv'); export const COMMON_COMMAND_SUPPORTED_FLAG_KEYS = flagKeys( 'remoteConfig', diff --git a/src/commands/command-flags.ts b/src/commands/command-flags.ts index f13609ec76..c8177a4ef8 100644 --- a/src/commands/command-flags.ts +++ b/src/commands/command-flags.ts @@ -92,7 +92,6 @@ function buildFlags(options: InternalRequestOptions): CommandFlags { replayUpdate: options.replayUpdate, replayBackend: options.replayBackend, replayEnv: options.replayEnv, - replayPublicEnv: options.publicEnv, replayShellEnv: options.replayShellEnv, replayFrom: options.replayFrom, replayPlanDigest: options.replayPlanDigest, diff --git a/src/commands/replay/index.test.ts b/src/commands/replay/index.test.ts index e72ba79f32..ef46e1e8db 100644 --- a/src/commands/replay/index.test.ts +++ b/src/commands/replay/index.test.ts @@ -50,7 +50,6 @@ describe('replay command interface', () => { replayUpdate: true, replayMaestro: true, replayEnv: ['FOO=bar'], - replayPublicEnv: ['TARGET'], metroHost: '127.0.0.1', metroPort: 8083, bundleUrl: 'http://127.0.0.1:8083/index.bundle', @@ -61,7 +60,6 @@ describe('replay command interface', () => { update: true, backend: 'maestro', env: ['FOO=bar'], - publicEnv: ['TARGET'], metroHost: '127.0.0.1', metroPort: 8083, bundleUrl: 'http://127.0.0.1:8083/index.bundle', @@ -80,7 +78,6 @@ describe('replay command interface', () => { replayUpdate: true, replayMaestro: true, replayEnv: ['FOO=bar'], - replayPublicEnv: ['TARGET'], metroHost: '127.0.0.1', metroPort: 8083, bundleUrl: 'http://127.0.0.1:8083/index.bundle', @@ -98,7 +95,6 @@ describe('replay command interface', () => { update: true, backend: 'maestro', env: ['FOO=bar'], - publicEnv: ['TARGET'], metroHost: '127.0.0.1', metroPort: 8083, bundleUrl: 'http://127.0.0.1:8083/index.bundle', @@ -120,7 +116,6 @@ describe('replay command interface', () => { update: true, backend: 'maestro', env: ['FOO=bar'], - publicEnv: ['TARGET'], }), ).toMatchObject({ command: 'replay', @@ -129,7 +124,6 @@ describe('replay command interface', () => { replayUpdate: true, replayBackend: 'maestro', replayEnv: ['FOO=bar'], - replayPublicEnv: ['TARGET'], replayShellEnv: { AD_VAR_REPLAY_TEST: 'enabled' }, }, }); diff --git a/src/commands/replay/index.ts b/src/commands/replay/index.ts index def5dbe0e0..7177d9b507 100644 --- a/src/commands/replay/index.ts +++ b/src/commands/replay/index.ts @@ -39,7 +39,6 @@ export const replayCommandMetadata = defineFieldCommandMetadata( backend: stringField(), maestro: booleanField(), env: stringArrayField(), - publicEnv: stringArrayField(), metroHost: stringField('Metro/debug host hint inherited by replay-opened sessions.'), metroPort: integerField('Metro/debug port hint inherited by replay-opened sessions.'), bundleUrl: stringField('Bundle URL hint inherited by replay-opened sessions.'), @@ -69,7 +68,6 @@ export const testCommandMetadata = defineFieldCommandMetadata( backend: stringField(), maestro: booleanField(), env: stringArrayField(), - publicEnv: stringArrayField(), metroHost: stringField('Metro/debug host hint inherited by each test session.'), metroPort: integerField('Metro/debug port hint inherited by each test session.'), bundleUrl: stringField('Bundle URL hint inherited by each test session.'), @@ -141,7 +139,6 @@ export const replayCliReader: CliReader = (positionals, flags) => ({ update: flags.replayUpdate, backend: flags.replayMaestro ? 'maestro' : undefined, env: flags.replayEnv, - publicEnv: flags.replayPublicEnv, metroHost: flags.metroHost, metroPort: flags.metroPort, bundleUrl: flags.bundleUrl, @@ -157,7 +154,6 @@ export const testCliReader: CliReader = (positionals, flags) => ({ update: flags.replayUpdate, backend: flags.replayMaestro ? 'maestro' : undefined, env: flags.replayEnv, - publicEnv: flags.replayPublicEnv, metroHost: flags.metroHost, metroPort: flags.metroPort, bundleUrl: flags.bundleUrl, @@ -176,7 +172,6 @@ export const replayDaemonWriter: DaemonWriter = (input) => replayUpdate: input.update, replayBackend: readReplayBackend(input), replayEnv: input.env, - replayPublicEnv: input.publicEnv, replayShellEnv: collectReplayClientShellEnv(process.env), replayFrom: input.resumeFrom, replayPlanDigest: input.resumePlanDigest, @@ -189,7 +184,6 @@ export const testDaemonWriter: DaemonWriter = (input) => replayUpdate: input.update, replayBackend: readReplayBackend(input), replayEnv: input.env, - replayPublicEnv: input.publicEnv, replayShellEnv: collectReplayClientShellEnv(process.env), }); 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 ac1fb83d38..5254ce2430 100644 --- a/src/compat/maestro/__tests__/engine-context.test.ts +++ b/src/compat/maestro/__tests__/engine-context.test.ts @@ -5,7 +5,7 @@ import type { MaestroRuntimePort } from '../engine-types.ts'; import { parseMaestroProgram } from '../program-ir-parser.ts'; import { executeMaestroProgram } from './runtime-port-fixtures.ts'; -test('redacts transitive scoped variables unless explicitly public', () => { +test('tracks expanded transitive scoped variables', () => { const context = createMaestroExecutionContext(); const leave = context.enter({ TARGET: '${NEXT}', @@ -14,15 +14,13 @@ test('redacts transitive scoped variables unless explicitly public', () => { }); expect(context.resolve('${TARGET}')).toBe('Done'); - expect(context.redactionVariables).toEqual([ - { name: 'FINAL', value: 'Done' }, - { name: 'NEXT', value: 'Done' }, - { name: 'TARGET', value: 'Done' }, - ]); + expect(context.expandedVariables).toEqual({ + TARGET: 'Done', + }); leave(); }); -test('retains shadowed non-public values after nested scopes unwind', () => { +test('retains expanded values after nested scopes unwind', () => { const context = createMaestroExecutionContext(); const rootLeave = context.enter({ SECRET: 'nested-scope-secret' }); const nestedLeave = context.enter({ TARGET: '${SECRET}' }); @@ -31,36 +29,27 @@ test('retains shadowed non-public values after nested scopes unwind', () => { nestedLeave(); rootLeave(); - expect(context.redactionVariables).toEqual([ - { name: 'SECRET', value: 'nested-scope-secret' }, - { name: 'TARGET', value: 'nested-scope-secret' }, - ]); -}); - -test('omits explicitly public variable values from the redaction set', () => { - const context = createMaestroExecutionContext({}, { TARGET: 'Continue checkout' }, ['TARGET']); - - expect(context.resolve('${TARGET}')).toBe('Continue checkout'); - expect(context.redactionVariables).toEqual([]); + expect(context.expandedVariables).toEqual({ + TARGET: 'nested-scope-secret', + }); }); -test('scrubs optional-step warnings before they enter a successful result', async () => { - const sentinel = 'optional-maestro-secret'; +test('renders resolved target variables in optional-step warnings', async () => { + const target = 'Missing checkout button'; const program = parseMaestroProgram( - ['---', '- tapOn:', ' text: ${SECRET}', ' optional: true'].join('\n'), + ['---', '- tapOn:', ' text: ${TARGET}', ' optional: true'].join('\n'), { sourcePath: '/flows/optional.yaml' }, ); const port: MaestroRuntimePort = { execute: vi.fn(async () => { - throw maestroTestFailure(`Missing ${sentinel}`); + throw maestroTestFailure(`Missing ${target}`); }), observe: vi.fn(async ({ generation }) => ({ generation, matched: true })), }; - const result = await executeMaestroProgram(program, port, { env: { SECRET: sentinel } }); + const result = await executeMaestroProgram(program, port, { env: { TARGET: target } }); - expect(result.warnings).toEqual([expect.stringContaining('')]); - expect(JSON.stringify(result.warnings)).not.toContain(sentinel); + 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 f0b2bc740b..910a1e61b1 100644 --- a/src/compat/maestro/engine-context.ts +++ b/src/compat/maestro/engine-context.ts @@ -1,28 +1,21 @@ import { AppError } from '../../kernel/errors.ts'; -import type { MaestroObservation, MaestroRedactionVariable } from './engine-types.ts'; +import type { MaestroObservation } from './engine-types.ts'; export type MaestroExecutionContext = ReturnType; export function createMaestroExecutionContext( defaults: Record = {}, runtimeOverrides: Record = {}, - publicVariableNames: Iterable = [], - onRedactionVariable?: (entry: MaestroRedactionVariable) => void, ) { const overrides = { ...runtimeOverrides }; - const publicNames = new Set(publicVariableNames); // Flow config and runFlow env values are stack-scoped; script output variables persist. let persistentValues = stringifyValues(defaults); const scopes: Record[] = []; - const redactionValues = new Map>(); + const expandedValues = new Map(); let cachedValues: Readonly> | undefined; let generation = 0; let observation: MaestroObservation | undefined; - for (const [name, value] of Object.entries(overrides)) { - recordSensitiveValue(name, value); - } - return { get values(): Readonly> { return currentValues(); @@ -33,10 +26,8 @@ export function createMaestroExecutionContext( get observation(): MaestroObservation | undefined { return observation?.generation === generation ? observation : undefined; }, - get redactionVariables(): readonly MaestroRedactionVariable[] { - return [...redactionValues].flatMap(([name, values]) => - [...values].map((value) => ({ name, value })), - ); + get expandedVariables(): Readonly> { + return Object.fromEntries(expandedValues); }, enter(scopedValues: Record = {}): () => void { const resolved = resolveScopedValues(scopedValues); @@ -71,17 +62,11 @@ export function createMaestroExecutionContext( observation = undefined; }, resolve(value: string): string { - return resolveValue(value, currentValues(), recordSensitiveValue); + return resolveValue(value, currentValues(), recordExpandedValue); }, resolveDeferred(value: string): string { return resolveValue(value, currentValues(), undefined, new Set(), false); }, - redact(value: string): string { - return [...redactionValues] - .flatMap(([name, values]) => [...values].map((entry) => ({ name, value: entry }))) - .sort((left, right) => right.value.length - left.value.length) - .reduce((result, entry) => result.replaceAll(entry.value, ``), value); - }, }; function currentValues(): Readonly> { @@ -107,25 +92,16 @@ export function createMaestroExecutionContext( ...resolved, ...overrides, }, - recordSensitiveValue, + undefined, new Set(), false, ); - recordSensitiveValue(key, resolved[key]); } return resolved; } - function recordSensitiveValue(name: string, value: string): void { - if (publicNames.has(name) || value.length === 0) return; - let values = redactionValues.get(name); - if (!values) { - values = new Set(); - redactionValues.set(name, values); - } - if (values.has(value)) return; - values.add(value); - onRedactionVariable?.({ name, value }); + function recordExpandedValue(name: string, value: string): void { + expandedValues.set(name, value); } } diff --git a/src/compat/maestro/engine-types.ts b/src/compat/maestro/engine-types.ts index 2314978e81..4af22839ff 100644 --- a/src/compat/maestro/engine-types.ts +++ b/src/compat/maestro/engine-types.ts @@ -5,11 +5,6 @@ import type { MaestroSelector, MaestroSourceLocation, } from './program-ir.ts'; - -export type MaestroRedactionVariable = { - readonly name: string; - readonly value: string; -}; export type MaestroControlCommand = Extract< MaestroCommand, { kind: 'runFlow' | 'repeat' | 'retry' } @@ -142,8 +137,7 @@ export type MaestroEngineObserver = { runtimeMetrics?: MaestroRuntimeMetrics; error: unknown; artifactPaths: readonly string[]; - /** Expanded non-public values that must be scrubbed from diagnostics. */ - redactionVariables: readonly MaestroRedactionVariable[]; + expandedVariables: Readonly>; }, ): void; }; @@ -151,8 +145,6 @@ export type MaestroEngineObserver = { export type MaestroEngineOptions = { /** Highest-precedence invocation values, normally CLI over shell. */ env?: Readonly>; - /** Names whose resolved values may be rendered in failure diagnostics. */ - publicVariableNames?: Iterable; /** Lowest-precedence defaults, normally replay built-ins. */ defaults?: Readonly>; platform?: MaestroPlatform; @@ -178,6 +170,4 @@ export type MaestroEngineResult = { generation: number; artifactPaths: string[]; warnings?: string[]; - /** Non-public values observed during execution, retained only for response scrubbing. */ - redactionVariables: readonly MaestroRedactionVariable[]; }; diff --git a/src/compat/maestro/replay-plan-execution.ts b/src/compat/maestro/replay-plan-execution.ts index 1c58cc07fc..edcef29cc0 100644 --- a/src/compat/maestro/replay-plan-execution.ts +++ b/src/compat/maestro/replay-plan-execution.ts @@ -33,11 +33,7 @@ export async function executeMaestroReplayPlan( plan, port, options, - context: createMaestroExecutionContext( - options.defaults, - options.env ? { ...options.env } : {}, - options.publicVariableNames, - ), + context: createMaestroExecutionContext(options.defaults, options.env ? { ...options.env } : {}), timing: DEFAULT_MAESTRO_COMPATIBILITY_TIMING_POLICY, artifacts: new Set(), warnings: [], @@ -56,7 +52,6 @@ export async function executeMaestroReplayPlan( skipped: state.skipped, generation: state.context.generation, artifactPaths: [...state.artifacts], - redactionVariables: state.context.redactionVariables, ...(state.warnings.length > 0 ? { warnings: state.warnings } : {}), }; } @@ -101,7 +96,7 @@ async function executeObservedStep( ...runtimeMetricsDelta(metricsBefore, state.port.readMetrics?.()), error: failure.error, artifactPaths: [...state.artifacts], - redactionVariables: state.context.redactionVariables, + 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 1abbda5810..349ecac04e 100644 --- a/src/compat/maestro/replay-plan-step-execution.ts +++ b/src/compat/maestro/replay-plan-step-execution.ts @@ -85,7 +85,7 @@ async function executeOptionalCommand( } catch (error) { checkpointMaestroCancellation(state.options.signal); if (isOptionalCommand(command) && isMaestroTestFailure(error)) { - state.warnings.push(state.context.redact(formatOptionalWarning(command, error))); + state.warnings.push(formatOptionalWarning(command, error)); state.skipped += 1; return undefined; } diff --git a/src/compat/maestro/replay-plan-steps.ts b/src/compat/maestro/replay-plan-steps.ts index d96aff54c0..a1265407b4 100644 --- a/src/compat/maestro/replay-plan-steps.ts +++ b/src/compat/maestro/replay-plan-steps.ts @@ -42,12 +42,7 @@ export async function compileMaestroReplayPlanSteps( const rootPath = sourcePathKey(program.source.path); const state: BuildState = { options, - context: createMaestroExecutionContext( - options.defaults, - options.env, - options.publicVariableNames, - options.onRedactionVariable, - ), + context: createMaestroExecutionContext(options.defaults, options.env), activeIncludePaths: new Set(rootPath === undefined ? [] : [rootPath]), staticallyExecutedControls: 0, staticallySkippedControls: 0, diff --git a/src/compat/maestro/replay-plan-types.ts b/src/compat/maestro/replay-plan-types.ts index 9e65929a26..4cf9b6e894 100644 --- a/src/compat/maestro/replay-plan-types.ts +++ b/src/compat/maestro/replay-plan-types.ts @@ -1,10 +1,6 @@ import type { MaestroProgramLoader } from './program-loader.ts'; import type { MaestroPlatform, MaestroSourceLocation } from './program-ir.ts'; -import type { - MaestroControlCommandDescriptor, - MaestroRedactionVariable, - MaestroRuntimeCommand, -} from './engine-types.ts'; +import type { MaestroControlCommandDescriptor, MaestroRuntimeCommand } from './engine-types.ts'; import type { SessionRuntimeHints } from '../../kernel/contracts.ts'; export type MaestroReplayPlanScope = Readonly>; @@ -53,10 +49,6 @@ export type MaestroReplayPlanOptions = { readonly platform?: MaestroPlatform; readonly target?: string; readonly runtimeHints?: Readonly; - /** Explicit names whose resolved values may appear in diagnostics. */ - readonly publicVariableNames?: Iterable; - /** Receives non-public values observed while resolving the static plan. */ - readonly onRedactionVariable?: (entry: MaestroRedactionVariable) => void; readonly loadProgram?: MaestroProgramLoader; readonly signal?: AbortSignal; }; diff --git a/src/compat/maestro/support-matrix.ts b/src/compat/maestro/support-matrix.ts index 9de06fb418..1b99b6a999 100644 --- a/src/compat/maestro/support-matrix.ts +++ b/src/compat/maestro/support-matrix.ts @@ -8,7 +8,7 @@ export const MAESTRO_COMPAT_SUPPORTED_CAPABILITIES = [ 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. Expanded values are redacted from Maestro failure diagnostics by default; use --public-env KEY only when a value is safe to disclose.', + 'Environment: flow env is the default, AD_VAR_* overrides it, and CLI -e KEY=VALUE wins over both. Failure diagnostics render resolved targets and paths but never inputText payloads; do not put 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/contracts/cli-flags.ts b/src/contracts/cli-flags.ts index 5e95eaebfe..d44b6a95c7 100644 --- a/src/contracts/cli-flags.ts +++ b/src/contracts/cli-flags.ts @@ -133,8 +133,6 @@ export type CliFlags = CloudProviderProfileFields & replayUpdate?: boolean; replayMaestro?: boolean; replayEnv?: string[]; - /** Replay/Test: names whose resolved values may appear in Maestro failure diagnostics. */ - replayPublicEnv?: string[]; replayShellEnv?: Record; replayFrom?: number; replayPlanDigest?: string; diff --git a/src/contracts/client-request.ts b/src/contracts/client-request.ts index 7d84608d42..85be9ec15d 100644 --- a/src/contracts/client-request.ts +++ b/src/contracts/client-request.ts @@ -53,7 +53,6 @@ export type CommandExecutionOptions = Partial & { replayUpdate?: boolean; replayBackend?: string; replayEnv?: string[]; - publicEnv?: string[]; replayShellEnv?: Record; replayFrom?: number; replayPlanDigest?: string; 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 3521f87389..2d52c0a02d 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,7 @@ async function buildFailureResponse( durationMs: 12, error: new Error('typed Maestro action failed'), artifactPaths: [], - redactionVariables: [], + expandedVariables: {}, }, plan: makeMaestroPlan(), replayPath: path.join(root, 'flow.yaml'), @@ -92,7 +92,7 @@ test('typed Maestro failure projection keeps the event command and source proven durationMs: 12, error: new Error('tap failed'), artifactPaths: [], - redactionVariables: [], + expandedVariables: {}, }, request, ); @@ -108,87 +108,7 @@ test('typed Maestro failure projection keeps the event command and source proven 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-')); - 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'; - 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}`], - }, - }), - sessionName, - logPath: path.join(root, 'daemon.log'), - sessionStore, - invoke: async (req) => { - if (req.command === 'snapshot') return { ok: true, data: { nodes } }; - if (req.command === 'click') { - return { - ok: false, - error: { - code: 'COMMAND_FAILED', - message: `tap failed for ${sentinel}`, - hint: `Find ${sentinel}`, - }, - }; - } - return { ok: true, data: {} }; - }, - }); - - 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 ""'); - expect(divergence.cause.message).toContain(''); - expect(divergence.cause.hint).toContain(''); - expect(divergence.suggestions).toEqual([ - expect.objectContaining({ - label: '', - selector: expect.stringContaining(''), - }), - ]); -}); - -test('typed Maestro failure diagnostics retain non-sensitive expanded selector values', async () => { +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'; @@ -206,7 +126,6 @@ test('typed Maestro failure diagnostics retain non-sensitive expanded selector v flags: { replayBackend: 'maestro', replayEnv: [`TARGET=${label}`], - replayPublicEnv: ['TARGET'], }, }), sessionName, @@ -252,14 +171,14 @@ test('typed Maestro failure diagnostics retain non-sensitive expanded selector v 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, [ @@ -270,7 +189,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}', '', @@ -289,7 +208,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, }, @@ -301,7 +220,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, @@ -315,8 +234,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}`, }, }; } @@ -325,7 +244,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() @@ -348,16 +267,16 @@ test('typed Maestro nested scopes scrub failure values after unwind and keep ret ]); }); -test('typed Maestro scrubs flow-local values when static include resolution fails', async () => { +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 sentinel = 'flow-local-maestro-secret'; + const flowName = 'checkout-details'; fs.writeFileSync( flowPath, - ['env:', ` SECRET: ${sentinel}`, '---', '- runFlow: ${SECRET}.yaml', ''].join('\n'), + ['env:', ` FLOW_NAME: ${flowName}`, '---', '- runFlow: ${FLOW_NAME}.yaml', ''].join('\n'), ); const response = await runReplayScriptFile({ @@ -372,9 +291,20 @@ test('typed Maestro scrubs flow-local values when static include resolution fail }); expect(response.ok).toBe(false); - expect(JSON.stringify(response)).not.toContain(sentinel); + expect(JSON.stringify(response)).toContain(flowName); if (response.ok) return; - expect(response.error.message).toContain(''); + 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 () => { diff --git a/src/daemon/handlers/__tests__/session-replay-vars.test.ts b/src/daemon/handlers/__tests__/session-replay-vars.test.ts index 98257e9ec2..f852d813f9 100644 --- a/src/daemon/handlers/__tests__/session-replay-vars.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-vars.test.ts @@ -51,7 +51,6 @@ import { buildReplayVarScope, collectReplayShellEnv, parseReplayCliEnvEntries, - parseReplayPublicEnvNames, resolveReplayAction, resolveReplayString, } from '../../../replay/vars.ts'; @@ -272,15 +271,6 @@ test('parseReplayCliEnvEntries splits KEY=VALUE and rejects invalid keys', () => assert.throws(() => parseReplayCliEnvEntries(['=value']), AppError); }); -test('parseReplayPublicEnvNames accepts replay variable names without inferring sensitivity', () => { - assert.deepEqual(parseReplayPublicEnvNames(['TARGET', 'API_TOKEN', 'TARGET']), [ - 'TARGET', - 'API_TOKEN', - ]); - assert.throws(() => parseReplayPublicEnvNames(['target']), AppError); - assert.throws(() => parseReplayPublicEnvNames(['AD_TOKEN']), AppError); -}); - test('resolveReplayAction walks positionals and string flags', () => { const action: SessionAction = { ts: 0, @@ -483,20 +473,6 @@ test('parseReplayCliEnvEntries error wording is user-friendly for invalid keys', ); }); -test('rejects --public-env outside Maestro YAML replay', async () => { - const { response } = await runReplayFixture({ - label: 'public-env-native-replay', - script: 'snapshot\n', - flags: { replayPublicEnv: ['TARGET'] }, - }); - - assert.equal(response.ok, false); - if (!response.ok) { - assert.equal(response.error.code, 'INVALID_ARGS'); - assert.match(response.error.message, /--public-env.*Maestro YAML/); - } -}); - // fallow-ignore-next-line complexity test('runReplayScriptFile dispatches resolved literals with file env overridden by CLI', async () => { const { response, calls } = await runReplayFixture({ diff --git a/src/daemon/handlers/session-replay-maestro-failure.ts b/src/daemon/handlers/session-replay-maestro-failure.ts index feac1a7c16..e09e19c568 100644 --- a/src/daemon/handlers/session-replay-maestro-failure.ts +++ b/src/daemon/handlers/session-replay-maestro-failure.ts @@ -1,6 +1,5 @@ import { isMaestroControlCommandDescriptor, - type MaestroRedactionVariable, type MaestroEngineEvent, } from '../../compat/maestro/engine-types.ts'; import { formatMaestroCommandProgress } from '../../compat/maestro/progress.ts'; @@ -42,7 +41,7 @@ export type MaestroFailedEngineEvent = MaestroEngineEvent & { readonly durationMs: number; readonly error: unknown; readonly artifactPaths: readonly string[]; - readonly redactionVariables: readonly MaestroRedactionVariable[]; + readonly expandedVariables: Readonly>; }; export type MaestroFailureReportAction = Pick< @@ -89,10 +88,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.redactionVariables), - ...collectMaestroTextScrubVars(report.command), - ].sort((left, right) => right.value.length - left.value.length); + const scrubVars = collectMaestroTextScrubVars(report.command); const sanitize = createReplayDivergenceSanitizer(scrubVars); const safeCause = { ...cause, @@ -335,15 +331,6 @@ function safeProgressPositionals(command: string, value: string | undefined): st return [value]; } -function collectExpandedScrubVars( - values: readonly MaestroRedactionVariable[], -): ReplayVarScrubEntry[] { - return 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/daemon/handlers/session-replay-maestro-response.ts b/src/daemon/handlers/session-replay-maestro-response.ts index a5e7da8646..30fda267c0 100644 --- a/src/daemon/handlers/session-replay-maestro-response.ts +++ b/src/daemon/handlers/session-replay-maestro-response.ts @@ -9,18 +9,9 @@ import { type MaestroFailedEngineEvent, } from './session-replay-maestro-failure.ts'; import { errorResponse } from './response.ts'; -import { - scrubReplayVarData, - scrubReplayVarValues, - type ReplayVarScrubEntry, -} from '../../replay/divergence.ts'; export function buildTypedMaestroSuccessResponse(params: { - result: { - artifactPaths: string[]; - warnings?: string[]; - redactionVariables: readonly ReplayVarScrubEntry[]; - }; + result: { artifactPaths: string[]; warnings?: string[] }; plan: MaestroReplayPlan; startIndex: number; startedAt: number; @@ -38,16 +29,8 @@ export function buildTypedMaestroSuccessResponse(params: { healed: 0, session: sessionName, sessionActive: sessionStore.get(sessionName) !== undefined, - artifactPaths: result.artifactPaths.map((entry) => - scrubReplayVarValues(entry, result.redactionVariables), - ), - ...(result.warnings - ? { - warnings: result.warnings.map((entry) => - scrubReplayVarValues(entry, result.redactionVariables), - ), - } - : {}), + artifactPaths: result.artifactPaths, + ...(result.warnings ? { warnings: result.warnings } : {}), ...(snapshotDiagnostics ? { snapshotDiagnostics } : {}), message: replaySuccessMessage(replayed, Date.now() - startedAt), } satisfies ReplayCommandResult, @@ -60,7 +43,6 @@ export async function buildTypedMaestroReplayErrorResponse(params: { state: { failedEvent?: MaestroFailedEngineEvent; plan?: MaestroReplayPlan; - redactionVariables: ReplayVarScrubEntry[]; snapshotStart: number; }; error: unknown; @@ -87,17 +69,10 @@ export async function buildTypedMaestroReplayErrorResponse(params: { ), }); } - return errorResponse( - normalizedError.code, - scrubReplayVarValues(normalizedError.message, params.state.redactionVariables), - { - ...(scrubReplayVarData( - normalizedError.details ?? {}, - params.state.redactionVariables, - ) as Record), - ...buildErrorDetails(failedEvent), - }, - ); + return errorResponse(normalizedError.code, normalizedError.message, { + ...(normalizedError.details ?? {}), + ...buildErrorDetails(failedEvent), + }); } function readSnapshotDiagnostics( diff --git a/src/daemon/handlers/session-replay-maestro-runtime.ts b/src/daemon/handlers/session-replay-maestro-runtime.ts index 45627461a5..0fafe3a865 100644 --- a/src/daemon/handlers/session-replay-maestro-runtime.ts +++ b/src/daemon/handlers/session-replay-maestro-runtime.ts @@ -12,9 +12,7 @@ import { stripUndefined } from '../../utils/parsing.ts'; import { collectReplayShellEnv, parseReplayCliEnvEntries, - parseReplayPublicEnvNames, readReplayCliEnvEntries, - readReplayPublicEnvNames, readReplayShellEnvSource, } from '../../replay/vars.ts'; import { createDaemonMaestroRuntimePort } from '../../compat/maestro/daemon-runtime-port.ts'; @@ -34,7 +32,6 @@ import { SessionStore } from '../session-store.ts'; import { errorResponse } from './response.ts'; import { buildReplayBuiltinVars } from './session-replay-vars.ts'; import type { MaestroFailedEngineEvent } from './session-replay-maestro-failure.ts'; -import type { ReplayVarScrubEntry } from '../../replay/divergence.ts'; import { createMaestroReplayObserver } from './session-replay-maestro-observer.ts'; import { buildTypedMaestroReplayErrorResponse, @@ -56,7 +53,6 @@ type TypedMaestroReplayParams = { type TypedMaestroReplayState = { failedEvent?: MaestroFailedEngineEvent; plan?: MaestroReplayPlan; - redactionVariables: ReplayVarScrubEntry[]; snapshotStart: number; }; @@ -69,8 +65,6 @@ type TypedMaestroReplayContext = { runtimeHints: ReturnType; defaults: Record; env: Record; - publicVariableNames: string[]; - redactionVariables: ReplayVarScrubEntry[]; signal: AbortSignal | undefined; loadProgram: ReturnType; }; @@ -93,7 +87,7 @@ export async function runTypedMaestroReplayFile( ); } const startedAt = Date.now(); - const state: TypedMaestroReplayState = { snapshotStart: 0, redactionVariables: [] }; + const state: TypedMaestroReplayState = { snapshotStart: 0 }; try { return await executeTypedMaestroReplay({ ...params, @@ -120,7 +114,6 @@ async function executeTypedMaestroReplay( ): Promise { const { req, sessionName, sessionStore, tracePath, invoke, state } = params; const context = await prepareTypedMaestroReplay(params); - state.redactionVariables = context.redactionVariables; const plan = await compileMaestroReplayPlan(context.program, { defaults: context.defaults, env: context.env, @@ -129,8 +122,6 @@ async function executeTypedMaestroReplay( runtimeHints: context.runtimeHints, loadProgram: context.loadProgram, signal: context.signal, - publicVariableNames: context.publicVariableNames, - onRedactionVariable: (entry) => addRedactionVariable(state.redactionVariables, entry), }); state.plan = plan; const startIndex = resolveMaestroReplayStartIndex(plan, { @@ -152,7 +143,6 @@ async function executeTypedMaestroReplay( const result = await executeMaestroPlan(plan, port, { defaults: context.defaults, env: context.env, - publicVariableNames: context.publicVariableNames, platform: context.platform, target: context.target, loadProgram: context.loadProgram, @@ -194,7 +184,6 @@ async function prepareTypedMaestroReplay( session, program, }); - const runtimeEnv = buildTypedMaestroEnv(req); return { filePath, program, @@ -206,9 +195,7 @@ async function prepareTypedMaestroReplay( platform: binding.platform, target: binding.target, }), - env: runtimeEnv.values, - publicVariableNames: runtimeEnv.publicVariableNames, - redactionVariables: runtimeEnv.redactionVariables, + env: buildTypedMaestroEnv(req), signal: getRequestSignal(req.meta?.requestId), loadProgram: createMaestroProgramLoader(path.dirname(filePath)), }; @@ -307,31 +294,11 @@ function buildTypedMaestroDefaults(params: { }; } -function addRedactionVariable(entries: ReplayVarScrubEntry[], entry: ReplayVarScrubEntry): void { - if (entries.some((current) => current.name === entry.name && current.value === entry.value)) - return; - entries.push(entry); -} - -function buildTypedMaestroEnv(req: DaemonRequest): { - values: Record; - publicVariableNames: string[]; - redactionVariables: ReplayVarScrubEntry[]; -} { - const values = { +function buildTypedMaestroEnv(req: DaemonRequest): Record { + return { ...collectReplayShellEnv(readReplayShellEnvSource(req.flags?.replayShellEnv)), ...parseReplayCliEnvEntries(readReplayCliEnvEntries(req.flags?.replayEnv)), }; - const publicVariableNames = parseReplayPublicEnvNames( - readReplayPublicEnvNames(req.flags?.replayPublicEnv), - ); - return { - values, - publicVariableNames, - redactionVariables: Object.entries(values) - .filter(([name, value]) => value.length > 0 && !publicVariableNames.includes(name)) - .map(([name, value]) => ({ name, value })), - }; } function createMaestroReplayPort(params: { diff --git a/src/daemon/handlers/session-replay-runtime.ts b/src/daemon/handlers/session-replay-runtime.ts index 05d1fb5e3f..cde7a6181b 100644 --- a/src/daemon/handlers/session-replay-runtime.ts +++ b/src/daemon/handlers/session-replay-runtime.ts @@ -514,15 +514,7 @@ async function runTypedReplayIfNeeded(params: { invoke: DaemonInvokeFn; resolved: string; }): Promise { - if (!isTypedMaestroReplay(params.req, params.resolved)) { - if ((params.req.flags?.replayPublicEnv?.length ?? 0) > 0) { - return errorResponse( - 'INVALID_ARGS', - '--public-env is supported only with Maestro YAML replay (--maestro).', - ); - } - return undefined; - } + if (!isTypedMaestroReplay(params.req, params.resolved)) return undefined; if (params.sessionStore.get(params.sessionName)?.saveScriptBoundary !== undefined) { return errorResponse( 'INVALID_ARGS', diff --git a/src/daemon/request-router.ts b/src/daemon/request-router.ts index 941940cdfd..70fb6079f5 100644 --- a/src/daemon/request-router.ts +++ b/src/daemon/request-router.ts @@ -342,6 +342,7 @@ function registerParameterizedFillDiagnosticValue(req: DaemonRequest): void { * redaction boundary even when no Maestro failure report is produced. */ function registerReplayVariableDiagnosticValues(req: DaemonRequest): void { + if (req.command !== 'replay' && req.command !== 'test') return; const values = { ...collectReplayShellEnv(readReplayShellEnvSource(req.flags?.replayShellEnv)), ...parseReplayCliEnvEntries(readReplayCliEnvEntries(req.flags?.replayEnv)), diff --git a/src/replay/vars.ts b/src/replay/vars.ts index c7c7411cd8..07c8f7c4cb 100644 --- a/src/replay/vars.ts +++ b/src/replay/vars.ts @@ -93,40 +93,12 @@ export function parseReplayCliEnvEntries(entries: readonly string[]): Record(); - for (const entry of entries) { - if (!REPLAY_VAR_KEY_RE.test(entry)) { - throw new AppError( - 'INVALID_ARGS', - `Invalid --public-env name "${entry}": names must use uppercase letters, digits, and underscores (e.g. TARGET).`, - ); - } - if (isReservedNamespaceKey(entry)) { - throw reservedNamespaceError(entry); - } - names.add(entry); - } - return [...names]; -} - export function readReplayCliEnvEntries(raw: unknown): string[] { return Array.isArray(raw) ? raw.filter((value): value is string => typeof value === 'string') : []; } -export function readReplayPublicEnvNames(raw: unknown): string[] { - return Array.isArray(raw) - ? raw.filter((value): value is string => typeof value === 'string') - : []; -} - export function readReplayShellEnvSource(raw: unknown): NodeJS.ProcessEnv { if (raw && typeof raw === 'object' && !Array.isArray(raw)) { const result: NodeJS.ProcessEnv = {}; diff --git a/website/docs/docs/replay-e2e.md b/website/docs/docs/replay-e2e.md index db869a6bbc..6a4811a0db 100644 --- a/website/docs/docs/replay-e2e.md +++ b/website/docs/docs/replay-e2e.md @@ -69,6 +69,9 @@ 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 render resolved variables used as targets and `runFlow` paths, while `inputText` + payloads remain hidden. Do not place secrets in selectors, links, filenames, or other diagnostic + identifiers that are expected to appear in failure output. - 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. From 8f6c1f0ac933e57aa965701b75fdea0f052c9fd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 28 Jul 2026 14:45:28 +0200 Subject: [PATCH 3/5] fix(replay): preserve diagnostic contracts --- docs/adr/0012-interactive-replay.md | 15 ++-- .../maestro/__tests__/engine-context.test.ts | 11 +-- src/compat/maestro/engine-context.ts | 16 +---- src/compat/maestro/engine-types.ts | 1 - src/compat/maestro/replay-plan-execution.ts | 1 - src/compat/maestro/support-matrix.ts | 3 +- .../request-router-replay-env.test.ts | 70 +++++++++++++++++++ .../session-replay-maestro-failure.test.ts | 2 - ...on-replay-runtime-failure-response.test.ts | 47 +++++++++++++ .../session-replay-maestro-failure.ts | 3 +- ...session-replay-runtime-failure-response.ts | 17 ++--- src/daemon/request-router.ts | 22 ------ src/replay/__tests__/divergence.test.ts | 10 --- src/replay/divergence.ts | 16 ----- src/replay/test/__tests__/progress.test.ts | 38 ++++++++++ src/replay/test/progress.ts | 9 ++- website/docs/docs/replay-e2e.md | 4 +- 17 files changed, 187 insertions(+), 98 deletions(-) create mode 100644 src/daemon/__tests__/request-router-replay-env.test.ts diff --git a/docs/adr/0012-interactive-replay.md b/docs/adr/0012-interactive-replay.md index 3f38d7ba66..05f91a601f 100644 --- a/docs/adr/0012-interactive-replay.md +++ b/docs/adr/0012-interactive-replay.md @@ -486,10 +486,17 @@ serialized. Maestro failure provenance renders resolved diagnostic identifiers, `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 registered with the request diagnostics redactor before platform work, independently of this -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. The report sets -truncation/redaction markers for every omission. +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__/engine-context.test.ts b/src/compat/maestro/__tests__/engine-context.test.ts index 5254ce2430..412a306747 100644 --- a/src/compat/maestro/__tests__/engine-context.test.ts +++ b/src/compat/maestro/__tests__/engine-context.test.ts @@ -5,7 +5,7 @@ import type { MaestroRuntimePort } from '../engine-types.ts'; import { parseMaestroProgram } from '../program-ir-parser.ts'; import { executeMaestroProgram } from './runtime-port-fixtures.ts'; -test('tracks expanded transitive scoped variables', () => { +test('resolves transitive scoped variables', () => { const context = createMaestroExecutionContext(); const leave = context.enter({ TARGET: '${NEXT}', @@ -14,13 +14,10 @@ test('tracks expanded transitive scoped variables', () => { }); 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}' }); @@ -28,10 +25,6 @@ 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 () => { 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/support-matrix.ts b/src/compat/maestro/support-matrix.ts index 1b99b6a999..132053e37a 100644 --- a/src/compat/maestro/support-matrix.ts +++ b/src/compat/maestro/support-matrix.ts @@ -8,7 +8,8 @@ export const MAESTRO_COMPAT_SUPPORTED_CAPABILITIES = [ 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 render resolved targets and paths but never inputText payloads; do not put secrets in diagnostic identifiers.', + '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 2d52c0a02d..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'), @@ -92,7 +91,6 @@ test('typed Maestro failure projection keeps the event command and source proven durationMs: 12, error: new Error('tap failed'), artifactPaths: [], - expandedVariables: {}, }, request, ); 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 e09e19c568..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,7 @@ export type MaestroFailureReportAction = Pick< >; export type MaestroFailureReportProjection = { - /** Resolved command at the failing runtime boundary; source remains authored provenance. */ + /** Failure command; runtime-command failures are resolved, while earlier failures stay authored. */ readonly command: MaestroEngineEvent['command']; readonly source: MaestroEngineEvent['source']; readonly progress: ReturnType; diff --git a/src/daemon/handlers/session-replay-runtime-failure-response.ts b/src/daemon/handlers/session-replay-runtime-failure-response.ts index 712389935f..7993d81ba7 100644 --- a/src/daemon/handlers/session-replay-runtime-failure-response.ts +++ b/src/daemon/handlers/session-replay-runtime-failure-response.ts @@ -1,8 +1,4 @@ -import { - scrubReplayVarData, - scrubReplayVarValues, - type ReplayVarScrubEntry, -} from '../../replay/divergence.ts'; +import { scrubReplayVarValues, type ReplayVarScrubEntry } from '../../replay/divergence.ts'; import { formatDivergenceActionLabel } from '../../replay/script-utils.ts'; import type { SnapshotDiagnosticsSummary } from '../../contracts/snapshot-diagnostics.ts'; import { buildDisplayPositionals } from '../session-event-action.ts'; @@ -93,15 +89,12 @@ export function buildReplayDivergenceFailureResponseFromDescriptor(params: { ...(error.retriable !== undefined ? { retriable: error.retriable } : {}), ...(error.supportedOn !== undefined ? { supportedOn: error.supportedOn } : {}), details: { - ...(scrubReplayVarData(pickSafeCauseDetails(error.details), scrubVars) as Record< - string, - unknown - >), - replayPath: scrubReplayVarValues(replayPath, scrubVars), + ...pickSafeCauseDetails(error.details), + replayPath, step, action, - positionals: positionals.map((value) => scrubReplayVarValues(value, scrubVars)), - artifactPaths: artifactPaths.map((value) => scrubReplayVarValues(value, scrubVars)), + positionals, + artifactPaths, ...(snapshotDiagnostics ? { snapshotDiagnostics } : {}), divergence, }, diff --git a/src/daemon/request-router.ts b/src/daemon/request-router.ts index 70fb6079f5..497541c4b8 100644 --- a/src/daemon/request-router.ts +++ b/src/daemon/request-router.ts @@ -48,12 +48,6 @@ import { canRunReplayScopedAction } from './daemon-command-registry.ts'; import { createAgentBrowserWebProvider } from '../platforms/web/agent-browser-provider.ts'; import { openWebSessionNames } from './web-session-names.ts'; import { inferFillText } from './action-utils.ts'; -import { - collectReplayShellEnv, - parseReplayCliEnvEntries, - readReplayCliEnvEntries, - readReplayShellEnvSource, -} from '../replay/vars.ts'; // --------------------------------------------------------------------------- // Request handler API @@ -147,7 +141,6 @@ export function createRequestHandler(deps: RequestRouterDeps): DaemonInvokeFn { return unauthorizedResponse(); } registerParameterizedFillDiagnosticValue(req); - registerReplayVariableDiagnosticValues(req); const invalidRecordingFlags = recordingFlagsResponse(req); if (invalidRecordingFlags) return invalidRecordingFlags; @@ -262,7 +255,6 @@ export function createRequestHandler(deps: RequestRouterDeps): DaemonInvokeFn { return unauthorizedResponse(); } registerParameterizedFillDiagnosticValue(req); - registerReplayVariableDiagnosticValues(req); let childScope: RequestExecutionScope | undefined; try { @@ -336,20 +328,6 @@ function registerParameterizedFillDiagnosticValue(req: DaemonRequest): void { ); } -/** - * Replay variables are fail-closed diagnostic values. Register them before - * request/session binding so nested dispatch and backend work inherit the - * redaction boundary even when no Maestro failure report is produced. - */ -function registerReplayVariableDiagnosticValues(req: DaemonRequest): void { - if (req.command !== 'replay' && req.command !== 'test') return; - const values = { - ...collectReplayShellEnv(readReplayShellEnvSource(req.flags?.replayShellEnv)), - ...parseReplayCliEnvEntries(readReplayCliEnvEntries(req.flags?.replayEnv)), - }; - for (const value of Object.values(values)) registerDiagnosticSensitiveValue(value); -} - async function dispatchGenericForLockedScope(params: { lockedScope: LockedRequestScope; logPath: string; diff --git a/src/replay/__tests__/divergence.test.ts b/src/replay/__tests__/divergence.test.ts index eede7e316a..b8c5f81eca 100644 --- a/src/replay/__tests__/divergence.test.ts +++ b/src/replay/__tests__/divergence.test.ts @@ -9,7 +9,6 @@ import { REPLAY_DIVERGENCE_DIGEST_REF_LIMIT, REPLAY_DIVERGENCE_LEVEL_BYTE_LIMITS, REPLAY_DIVERGENCE_SUGGESTION_LIMIT, - scrubReplayVarData, truncateUtf8Field, type ReplayDivergence, } from '../divergence.ts'; @@ -237,15 +236,6 @@ test('sanitizeReplayDivergenceField redacts sensitive content even when no trunc assert.ok(sanitized.includes('[REDACTED]')); }); -test('scrubReplayVarData scrubs variable values from normalized detail values and keys', () => { - assert.deepEqual( - scrubReplayVarData({ 'secret-value': ['prefix secret-value', { nested: 'secret-value' }] }, [ - { name: 'TOKEN', value: 'secret-value' }, - ]), - { '': ['prefix ', { nested: '' }] }, - ); -}); - // --- Text report carries the repair data (bounded refs + unavailable hint) --- test('formatReplayDivergenceReport lists a bounded ref/role/label subset for an available screen', async () => { diff --git a/src/replay/divergence.ts b/src/replay/divergence.ts index 1dd9db33a5..e93fbe155b 100644 --- a/src/replay/divergence.ts +++ b/src/replay/divergence.ts @@ -276,22 +276,6 @@ export function scrubReplayVarValues( return output; } -/** Scrubs runtime values from arbitrary normalized error detail payloads, including object keys. */ -export function scrubReplayVarData( - value: unknown, - entries: readonly ReplayVarScrubEntry[], -): unknown { - if (typeof value === 'string') return scrubReplayVarValues(value, entries); - if (Array.isArray(value)) return value.map((entry) => scrubReplayVarData(entry, entries)); - if (!value || typeof value !== 'object') return value; - return Object.fromEntries( - Object.entries(value).map(([key, entry]) => [ - scrubReplayVarValues(key, entries), - scrubReplayVarData(entry, entries), - ]), - ); -} - /** Per-report field sanitizer: variable scrub, then redact, then truncate. */ export function createReplayDivergenceSanitizer( scrubVars: readonly ReplayVarScrubEntry[], diff --git a/src/replay/test/__tests__/progress.test.ts b/src/replay/test/__tests__/progress.test.ts index bec446c4a5..3e5e969b16 100644 --- a/src/replay/test/__tests__/progress.test.ts +++ b/src/replay/test/__tests__/progress.test.ts @@ -252,6 +252,44 @@ test('createReplayTestProgressRenderer clears every reflowed row after a termina assert.ok(rendered?.text.endsWith('...')); }); +test('createReplayTestProgressRenderer bounds resize cleanup in non-reflowing terminals', () => { + let columns = 200; + const renderer = createReplayTestProgressRenderer({ + liveProgress: true, + columns: () => columns, + }); + 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, 3); +}); + test('createReplayTestProgressRenderer colors completed result markers when color is enabled', () => { withForcedColor(() => { assert.equal( diff --git a/src/replay/test/progress.ts b/src/replay/test/progress.ts index 909cb0557f..45e48833d6 100644 --- a/src/replay/test/progress.ts +++ b/src/replay/test/progress.ts @@ -32,6 +32,7 @@ const REPLAY_TEST_PROGRESS_SPINNER = { }; const ANSI_ESCAPE_PREFIX = `${String.fromCharCode(27)}[`; const ANSI_RESET = `${ANSI_ESCAPE_PREFIX}0m`; +const MAX_LIVE_PROGRESS_CLEAR_ROWS = 4; export const REPLAY_TEST_PROGRESS_SPINNER_INTERVAL_MS = REPLAY_TEST_PROGRESS_SPINNER.interval; @@ -286,7 +287,13 @@ function clearLiveProgressPrefix( previousWidth: number, columns: ReplayTestProgressFormatOptions['columns'], ): string { - const rows = Math.max(1, Math.ceil(previousWidth / resolveColumns(columns))); + // Reflowing terminals move the cursor to the final wrapped row after a shrink, + // while some multiplexers leave it on the original row. Bound the cursor-up + // cleanup so the latter cannot erase an arbitrary number of completed tests. + const rows = Math.min( + MAX_LIVE_PROGRESS_CLEAR_ROWS, + Math.max(1, Math.ceil(previousWidth / resolveColumns(columns))), + ); let output = '\r\x1B[2K'; for (let row = 1; row < rows; row += 1) { output += '\x1B[1A\r\x1B[2K'; diff --git a/website/docs/docs/replay-e2e.md b/website/docs/docs/replay-e2e.md index 6a4811a0db..5b5975a0cc 100644 --- a/website/docs/docs/replay-e2e.md +++ b/website/docs/docs/replay-e2e.md @@ -69,9 +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 render resolved variables used as targets and `runFlow` paths, while `inputText` - payloads remain hidden. Do not place secrets in selectors, links, filenames, or other diagnostic - identifiers that are expected to appear in failure output. +- 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. From f121ac0f75cfc98c073730c9f474f62156826a36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 28 Jul 2026 15:31:27 +0200 Subject: [PATCH 4/5] test(replay): verify resize cleanup rows --- src/replay/test/__tests__/progress.test.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/replay/test/__tests__/progress.test.ts b/src/replay/test/__tests__/progress.test.ts index 3e5e969b16..9280fe449e 100644 --- a/src/replay/test/__tests__/progress.test.ts +++ b/src/replay/test/__tests__/progress.test.ts @@ -214,12 +214,16 @@ test('createReplayTestProgressRenderer trims live step progress by visible colum }); test('createReplayTestProgressRenderer clears every reflowed row after a terminal resize', () => { - let columns = 80; + 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, }); - renderer.render({ + const initial = renderer.render({ type: 'test-step', test: { file: '/tmp/checkout.yaml', @@ -232,8 +236,11 @@ test('createReplayTestProgressRenderer clears every reflowed row after a termina stepValue: 'Confirmation', }, }); + assert.ok(initial?.text.startsWith(clearRow)); + const initialVisibleWidth = (initial?.text.length ?? clearRow.length) - clearRow.length; + assert.equal(initialVisibleWidth, initialColumns); - columns = 20; + columns = resizedColumns; const rendered = renderer.render({ type: 'test-step', test: { @@ -248,7 +255,11 @@ test('createReplayTestProgressRenderer clears every reflowed row after a termina }, }); - assert.ok(rendered?.text.includes('\u001B[1A')); + 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('...')); }); From 791c86d9d46f1408baeecddf1a507a5dd5960a28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 28 Jul 2026 15:51:48 +0200 Subject: [PATCH 5/5] fix(replay): distinguish terminal resize reflow --- src/replay/test/__tests__/progress.test.ts | 57 ++++++++++++++++- .../test/__tests__/reporters-default.test.ts | 61 ++++++++++++++++--- src/replay/test/progress.ts | 24 +++++--- src/replay/test/reporters/default.ts | 5 ++ 4 files changed, 129 insertions(+), 18 deletions(-) diff --git a/src/replay/test/__tests__/progress.test.ts b/src/replay/test/__tests__/progress.test.ts index 9280fe449e..f7fe03e3b1 100644 --- a/src/replay/test/__tests__/progress.test.ts +++ b/src/replay/test/__tests__/progress.test.ts @@ -263,11 +263,63 @@ test('createReplayTestProgressRenderer clears every reflowed row after a termina assert.ok(rendered?.text.endsWith('...')); }); -test('createReplayTestProgressRenderer bounds resize cleanup in non-reflowing terminals', () => { +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', @@ -298,7 +350,8 @@ test('createReplayTestProgressRenderer bounds resize cleanup in non-reflowing te }, }); - assert.equal((rendered?.text.split('\u001B[1A').length ?? 1) - 1, 3); + 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', () => { 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 45e48833d6..7202ba647e 100644 --- a/src/replay/test/progress.ts +++ b/src/replay/test/progress.ts @@ -15,6 +15,8 @@ export type ReplayTestProgressFormatOptions = { liveProgress?: boolean; /** 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 = { @@ -32,7 +34,6 @@ const REPLAY_TEST_PROGRESS_SPINNER = { }; const ANSI_ESCAPE_PREFIX = `${String.fromCharCode(27)}[`; const ANSI_RESET = `${ANSI_ESCAPE_PREFIX}0m`; -const MAX_LIVE_PROGRESS_CLEAR_ROWS = 4; export const REPLAY_TEST_PROGRESS_SPINNER_INTERVAL_MS = REPLAY_TEST_PROGRESS_SPINNER.interval; @@ -59,6 +60,7 @@ export function createReplayTestProgressRenderer( const clearPrefix = clearLiveProgressPrefix( hasLiveProgressLine ? liveProgressWidth : 0, options.columns, + options.terminalReflowsOnResize !== false, ); hasLiveProgressLine = true; liveProgressWidth = visibleLength(line); @@ -75,7 +77,11 @@ export function createReplayTestProgressRenderer( const line = formatReplayTestProgressEvent(event.test, options); if (!line) return undefined; const text = hasLiveProgressLine - ? `${clearLiveProgressPrefix(liveProgressWidth, options.columns)}${line}` + ? `${clearLiveProgressPrefix( + liveProgressWidth, + options.columns, + options.terminalReflowsOnResize !== false, + )}${line}` : line; hasLiveProgressLine = false; liveProgressWidth = 0; @@ -286,14 +292,14 @@ function replayTestCompletionProgressKey(event: ReplayTestResult): string { function clearLiveProgressPrefix( previousWidth: number, columns: ReplayTestProgressFormatOptions['columns'], + terminalReflowsOnResize: boolean, ): string { - // Reflowing terminals move the cursor to the final wrapped row after a shrink, - // while some multiplexers leave it on the original row. Bound the cursor-up - // cleanup so the latter cannot erase an arbitrary number of completed tests. - const rows = Math.min( - MAX_LIVE_PROGRESS_CLEAR_ROWS, - Math.max(1, Math.ceil(previousWidth / resolveColumns(columns))), - ); + // 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'; diff --git a/src/replay/test/reporters/default.ts b/src/replay/test/reporters/default.ts index dd8714e74e..56eee66328 100644 --- a/src/replay/test/reporters/default.ts +++ b/src/replay/test/reporters/default.ts @@ -46,6 +46,7 @@ export function createDefaultReplayTestReporter(): ReplayTestReporter { verbose: context.verbose, liveProgress: shouldUseLiveProgress(context), 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,