diff --git a/packages/cli-app/src/exec.js b/packages/cli-app/src/exec.js index ccec76328..10b54de21 100644 --- a/packages/cli-app/src/exec.js +++ b/packages/cli-app/src/exec.js @@ -1,3 +1,6 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; import command from '@percy/cli-command'; import * as ExecPlugin from '@percy/cli-exec'; @@ -15,6 +18,168 @@ export const start = command('start', { } }, ExecPlugin.start.callback); +// Locate the `test` subcommand in argv. Maestro accepts global parent +// flags before the subcommand, e.g.: +// maestro test flow.yaml +// maestro --udid test flow.yaml +// maestro --platform=android test flow.yaml +// maestro --verbose --no-ansi test flow.yaml +// We must find `test` by scanning rather than checking args[1] === 'test', +// or our injects silently no-op when the customer pins a device. +// +// Returns the index of the `test` literal, or -1 if not present. Skips +// over the value of known value-taking parent flags so a literal `test` +// supplied as a flag value (e.g. `--udid test`) isn't mistaken for the +// subcommand. Equals-form (`--flag=value`) doesn't need a skip — the +// value is part of the same argv token. +const MAESTRO_PARENT_VALUE_FLAGS = new Set([ + '--udid', '--device', '-p', '--platform', + '--host', '--port', '--driver-host-port' +]); +function findTestSubcommandIdx(args) { + for (let i = 1; i < args.length; i++) { + const tok = args[i]; + if (tok === 'test') return i; + if (typeof tok === 'string' && MAESTRO_PARENT_VALUE_FLAGS.has(tok)) { + i++; // skip the value of this flag + } + } + return -1; +} + +function hasExistingPercyServerFlag(args, testIdx) { + for (let i = testIdx + 1; i < args.length - 1; i++) { + if (args[i] === '-e' && /^PERCY_SERVER=/.test(args[i + 1])) return true; + } + return false; +} + +// Returns the customer-supplied value of `--test-output-dir` (whether the +// space-form `--test-output-dir ` or the equals-form +// `--test-output-dir=` — both are valid picocli syntax), or null if +// absent. Returning the value (not just an index) lets the screenshot-dir +// helper align PERCY_MAESTRO_SCREENSHOT_DIR with the customer's value in +// either form without re-deriving it. +// +// An empty value (space-form value === '' or equals-form `--test-output-dir=` +// with nothing after the `=`) is treated as ABSENT — returning the empty +// string would tell the caller "customer set this" and bypass the +// auto-resolve fallback, leaving Maestro to default its output location +// while PERCY_MAESTRO_SCREENSHOT_DIR stays unset. That mismatch silently +// produces all-404 snapshots, so fall through to the auto-resolve path +// instead. +const TEST_OUTPUT_DIR_EQ_PREFIX = '--test-output-dir='; +function findTestOutputDirValue(args, testIdx) { + for (let i = testIdx + 1; i < args.length; i++) { + const tok = args[i]; + if (tok === '--test-output-dir' && i + 1 < args.length) { + const val = args[i + 1]; + if (typeof val === 'string' && val.length > 0) return val; + } else if (typeof tok === 'string' && tok.startsWith(TEST_OUTPUT_DIR_EQ_PREFIX)) { + const val = tok.slice(TEST_OUTPUT_DIR_EQ_PREFIX.length); + if (val.length > 0) return val; + } + } + return null; +} + +// Maestro's GraalJS sandbox does NOT inherit the parent process's env, +// so `PERCY_SERVER_ADDRESS` exported by app:exec is invisible to the +// SDK. When wrapping `maestro test`, surface the CLI address through +// Maestro's only env channel — `-e KEY=VALUE` flags — so the SDK +// healthcheck can find the local CLI without the customer having to +// pair ports manually. No-op when the customer already supplied their +// own `-e PERCY_SERVER=...`. +// +// When percy?.address() is falsy (percy disabled, start failed), emit a +// WARN so the customer is not surprised by a silent zero-snapshot build. +// The customer-override skip case (their own `-e PERCY_SERVER=...` is in +// argv) does NOT warn — that's intentional flow control, not a problem. +export function maybeInjectMaestroServer(ctx, log) { + const args = ctx?.argv; + if (!Array.isArray(args) || args.length < 2) return; + if (path.basename(args[0]) !== 'maestro') return; + const testIdx = findTestSubcommandIdx(args); + if (testIdx < 0) return; + if (hasExistingPercyServerFlag(args, testIdx)) return; + const addr = ctx.percy?.address(); + if (!addr) { + log?.warn( + 'app:exec did not start the Percy CLI server (percy disabled or start ' + + 'failed); -e PERCY_SERVER not injected into maestro test. Snapshots will ' + + 'NOT be uploaded. Set PERCY_TOKEN and re-run, or check the percy log above.' + ); + return; + } + // Inject after `test` so `-e KEY=VAL` is bound to the `test` subcommand + // (the only Maestro subcommand that accepts `-e`). + args.splice(testIdx + 1, 0, '-e', `PERCY_SERVER=${addr}`); +} + +// Auto-resolve the Maestro screenshot output directory so customers don't +// have to pair `export PERCY_MAESTRO_SCREENSHOT_DIR=...` with a matching +// `--test-output-dir ` in their maestro test command. +// +// Resolution order: +// 1. Customer set BOTH process.env.PERCY_MAESTRO_SCREENSHOT_DIR and +// --test-output-dir in argv → trust them, do nothing. +// 2. Customer set PERCY_MAESTRO_SCREENSHOT_DIR only → use it, inject +// `--test-output-dir ` into argv. +// 3. Customer set --test-output-dir only → use that value, mirror it +// into process.env.PERCY_MAESTRO_SCREENSHOT_DIR (so the SDK + +// CLI relay see the same path). +// 4. Neither set → pick `${process.cwd()}/.percy-out`. On any mkdir +// failure (read-only CWD, EACCES, EEXIST as a file), fall back to +// `${os.tmpdir()}/percy-maestro-` with a WARN log. +// +// The env-var update and argv splice always keep both sources of truth +// (SDK reads env var; Maestro reads the flag) aligned to the same path. +export function maybeInjectScreenshotDir(ctx, log) { + const args = ctx?.argv; + if (!Array.isArray(args) || args.length < 2) return; + if (path.basename(args[0]) !== 'maestro') return; + const testIdx = findTestSubcommandIdx(args); + if (testIdx < 0) return; + + const envSet = !!process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + const existingFlagValue = findTestOutputDirValue(args, testIdx); + const flagSet = existingFlagValue !== null; + + // Fully customer-controlled — nothing to do. + if (envSet && flagSet) return; + + let resolved; + if (envSet) { + resolved = process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + } else if (flagSet) { + resolved = existingFlagValue; + } else { + const preferred = path.join(process.cwd(), '.percy-out'); + try { + fs.mkdirSync(preferred, { recursive: true }); + resolved = preferred; + } catch (err) { + const fallback = path.join(os.tmpdir(), `percy-maestro-${process.pid}`); + try { + fs.mkdirSync(fallback, { recursive: true }); + } catch (_) { + // tmpdir mkdir failure is exceedingly rare; fall through and let + // downstream code surface a clearer error than this helper can. + } + resolved = fallback; + log?.warn( + `Could not create ${preferred} (${err.code || err.message}); ` + + `falling back to ${fallback}. Set PERCY_MAESTRO_SCREENSHOT_DIR to ` + + 'pick a specific location.' + ); + } + } + + if (!envSet) process.env.PERCY_MAESTRO_SCREENSHOT_DIR = resolved; + // Inject right after `test` (the subcommand that owns `--test-output-dir`). + if (!flagSet) args.splice(testIdx + 1, 0, '--test-output-dir', resolved); +} + export const exec = command('exec', { description: 'Start and stop Percy around a supplied command for native apps', usage: '[options] -- ', @@ -29,6 +194,18 @@ export const exec = command('exec', { projectType: 'app', skipDiscovery: true } -}, ExecPlugin.default.callback); +}, async function*(ctx) { + // The two helpers splice their flag groups at argv index 2 (between `test` + // and the flow file) because `-e` and `--test-output-dir` are + // `test`-subcommand options. Resulting argv for `maestro test flow.yaml`: + // maestro test --test-output-dir -e PERCY_SERVER= flow.yaml + // iOS driver port: not prescribed from this side — the @percy/core relay + // reads `PERCY_IOS_DRIVER_HOST_PORT` (BS-host-injected on production + // hosts) and probes the documented Maestro 2.4.0 single-simulator default + // (`127.0.0.1:7001`) when it isn't set. See `packages/core/src/maestro-hierarchy.js`. + maybeInjectMaestroServer(ctx, ctx.log); + maybeInjectScreenshotDir(ctx, ctx.log); + yield* ExecPlugin.default.callback(ctx); +}); export default exec; diff --git a/packages/cli-app/src/index.js b/packages/cli-app/src/index.js index 49012033f..d1c8d4ffd 100644 --- a/packages/cli-app/src/index.js +++ b/packages/cli-app/src/index.js @@ -1,2 +1,2 @@ export { default, app } from './app.js'; -export { exec, start, stop, ping } from './exec.js'; +export { exec, start, stop, ping, maybeInjectMaestroServer, maybeInjectScreenshotDir } from './exec.js'; diff --git a/packages/cli-app/test/exec.test.js b/packages/cli-app/test/exec.test.js index 100c1fd5e..b114676af 100644 --- a/packages/cli-app/test/exec.test.js +++ b/packages/cli-app/test/exec.test.js @@ -1,14 +1,24 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; import { setupTest } from '@percy/cli-command/test/helpers'; import * as ExecPlugin from '@percy/cli-exec'; -import { exec, start, stop, ping } from '@percy/cli-app'; +import { + exec, start, stop, ping, + maybeInjectMaestroServer, maybeInjectScreenshotDir +} from '@percy/cli-app'; describe('percy app:exec', () => { beforeEach(async () => { await setupTest(); }); - it('has shared exec commands with differing definitions', async () => { - expect(exec.callback).toEqual(ExecPlugin.default.callback); + it('wraps cli-exec callbacks while preserving differing definitions', async () => { + // exec.callback wraps cli-exec's callback (auto-injects -e PERCY_SERVER + // and --test-output-dir for `maestro test`), so it is no longer reference- + // equal — but start, stop, and ping remain straight delegations. + expect(typeof exec.callback).toBe('function'); + expect(exec.callback).not.toEqual(ExecPlugin.default.callback); expect(exec.definition).not.toEqual(ExecPlugin.default.definition); expect(start.callback).toEqual(ExecPlugin.start.callback); expect(start.definition).not.toEqual(ExecPlugin.start.definition); @@ -23,4 +33,503 @@ describe('percy app:exec', () => { await expectAsync(start(['--network-idle-timeout', '500'])) .toBeRejectedWithError("Unknown option '--network-idle-timeout'"); }); + + describe('maybeInjectMaestroServer', () => { + function ctxFor(argv, addr = 'http://localhost:5338') { + return { + argv: [...argv], + percy: { address: () => addr } + }; + } + + it('injects -e PERCY_SERVER at index 2 for `maestro test`', () => { + const ctx = ctxFor(['maestro', 'test', 'flow.yaml']); + maybeInjectMaestroServer(ctx); + expect(ctx.argv).toEqual([ + 'maestro', 'test', '-e', 'PERCY_SERVER=http://localhost:5338', 'flow.yaml' + ]); + }); + + it('preserves other -e flags and customer args after injection', () => { + const ctx = ctxFor([ + 'maestro', 'test', '--test-output-dir', 'out', + '-e', 'PERCY_DEVICE_NAME=Pixel 10', 'flow.yaml' + ]); + maybeInjectMaestroServer(ctx); + expect(ctx.argv).toEqual([ + 'maestro', 'test', + '-e', 'PERCY_SERVER=http://localhost:5338', + '--test-output-dir', 'out', + '-e', 'PERCY_DEVICE_NAME=Pixel 10', + 'flow.yaml' + ]); + }); + + it('skips when customer already supplied -e PERCY_SERVER (adjacent to test)', () => { + const argv = ['maestro', 'test', '-e', 'PERCY_SERVER=http://custom:9999', 'flow.yaml']; + const ctx = ctxFor(argv); + maybeInjectMaestroServer(ctx); + expect(ctx.argv).toEqual(argv); + }); + + it('skips when customer supplied -e PERCY_SERVER deeper in args (R3 scan)', () => { + const argv = [ + 'maestro', 'test', '--test-output-dir', 'out', + '-e', 'PERCY_SERVER=http://custom:9999', 'flow.yaml' + ]; + const ctx = ctxFor(argv); + maybeInjectMaestroServer(ctx); + expect(ctx.argv).toEqual(argv); + }); + + it('skips for `maestro hierarchy` (not a test command)', () => { + const argv = ['maestro', 'hierarchy', '--udid', 'X']; + const ctx = ctxFor(argv); + maybeInjectMaestroServer(ctx); + expect(ctx.argv).toEqual(argv); + }); + + it('skips for `maestro list-devices` (not a test command)', () => { + const argv = ['maestro', 'list-devices']; + const ctx = ctxFor(argv); + maybeInjectMaestroServer(ctx); + expect(ctx.argv).toEqual(argv); + }); + + it('skips when args has fewer than two elements', () => { + const argv = ['maestro']; + const ctx = ctxFor(argv); + maybeInjectMaestroServer(ctx); + expect(ctx.argv).toEqual(argv); + }); + + it('matches by basename when the command is an absolute path', () => { + const ctx = ctxFor(['/Users/foo/.maestro/bin/maestro', 'test', 'flow.yaml']); + maybeInjectMaestroServer(ctx); + expect(ctx.argv).toEqual([ + '/Users/foo/.maestro/bin/maestro', 'test', + '-e', 'PERCY_SERVER=http://localhost:5338', + 'flow.yaml' + ]); + }); + + it('skips for `npx maestro test` (argv[0] is npx, not maestro)', () => { + const argv = ['npx', 'maestro', 'test', 'flow.yaml']; + const ctx = ctxFor(argv); + maybeInjectMaestroServer(ctx); + expect(ctx.argv).toEqual(argv); + }); + + it('skips for non-maestro commands (python, appium, etc.)', () => { + const pyArgv = ['python', 'test.py']; + const pyCtx = ctxFor(pyArgv); + maybeInjectMaestroServer(pyCtx); + expect(pyCtx.argv).toEqual(pyArgv); + + const apiumArgv = ['appium', '--port', '4723']; + const apiumCtx = ctxFor(apiumArgv); + maybeInjectMaestroServer(apiumCtx); + expect(apiumCtx.argv).toEqual(apiumArgv); + }); + + it('flows --port through into the injected address (multi-device)', () => { + const ctx = ctxFor(['maestro', 'test', 'flow.yaml'], 'http://localhost:5339'); + maybeInjectMaestroServer(ctx); + expect(ctx.argv).toEqual([ + 'maestro', 'test', '-e', 'PERCY_SERVER=http://localhost:5339', 'flow.yaml' + ]); + }); + + it('skips when percy is disabled (no address)', () => { + const argv = ['maestro', 'test', 'flow.yaml']; + const ctx = { argv: [...argv], percy: { address: () => undefined } }; + maybeInjectMaestroServer(ctx); + expect(ctx.argv).toEqual(argv); + }); + + it('skips when ctx has no percy at all', () => { + const argv = ['maestro', 'test', 'flow.yaml']; + const ctx = { argv: [...argv] }; + maybeInjectMaestroServer(ctx); + expect(ctx.argv).toEqual(argv); + }); + + it('two concurrent contexts pick their own addresses (multi-device isolation)', () => { + const a = ctxFor(['maestro', 'test', 'flow.yaml'], 'http://localhost:5338'); + const b = ctxFor(['maestro', 'test', 'flow.yaml'], 'http://localhost:5339'); + maybeInjectMaestroServer(a); + maybeInjectMaestroServer(b); + expect(a.argv).toContain('PERCY_SERVER=http://localhost:5338'); + expect(b.argv).toContain('PERCY_SERVER=http://localhost:5339'); + expect(a.argv).not.toContain('PERCY_SERVER=http://localhost:5339'); + expect(b.argv).not.toContain('PERCY_SERVER=http://localhost:5338'); + }); + + it('emits WARN log when percy address is falsy and there is no customer override', () => { + const log = { warn: jasmine.createSpy('warn') }; + const ctx = { argv: ['maestro', 'test', 'flow.yaml'], percy: { address: () => undefined } }; + maybeInjectMaestroServer(ctx, log); + expect(log.warn).toHaveBeenCalledTimes(1); + expect(log.warn.calls.argsFor(0)[0]).toContain('-e PERCY_SERVER not injected'); + }); + + it('does NOT emit WARN when customer-supplied -e PERCY_SERVER override is present', () => { + const log = { warn: jasmine.createSpy('warn') }; + const ctx = { + argv: ['maestro', 'test', '-e', 'PERCY_SERVER=http://custom:9999', 'flow.yaml'], + percy: { address: () => undefined } + }; + maybeInjectMaestroServer(ctx, log); + expect(log.warn).not.toHaveBeenCalled(); + }); + + // Maestro accepts global flags BEFORE the `test` subcommand + // (picocli convention): `maestro --udid X test flow.yaml`, + // `maestro --platform=android test flow.yaml`, etc. The injection + // must locate `test` and splice the -e pair right after it, not + // assume args[1] === 'test'. + it('injects after `test` when --udid precedes the subcommand', () => { + const ctx = ctxFor(['maestro', '--udid', '61031VDCR0004B', 'test', 'flow.yaml']); + maybeInjectMaestroServer(ctx); + expect(ctx.argv).toEqual([ + 'maestro', '--udid', '61031VDCR0004B', 'test', + '-e', 'PERCY_SERVER=http://localhost:5338', + 'flow.yaml' + ]); + }); + + it('injects after `test` when --device precedes the subcommand', () => { + const ctx = ctxFor(['maestro', '--device', 'Pixel-10', 'test', 'flow.yaml']); + maybeInjectMaestroServer(ctx); + expect(ctx.argv).toEqual([ + 'maestro', '--device', 'Pixel-10', 'test', + '-e', 'PERCY_SERVER=http://localhost:5338', + 'flow.yaml' + ]); + }); + + it('injects after `test` when --platform=android (= form) precedes the subcommand', () => { + const ctx = ctxFor(['maestro', '--platform=android', 'test', 'flow.yaml']); + maybeInjectMaestroServer(ctx); + expect(ctx.argv).toEqual([ + 'maestro', '--platform=android', 'test', + '-e', 'PERCY_SERVER=http://localhost:5338', + 'flow.yaml' + ]); + }); + + it('injects after `test` when multiple parent flags precede the subcommand', () => { + const ctx = ctxFor([ + 'maestro', '--udid', '61031VDCR0004B', '--platform', 'android', + 'test', 'flow.yaml' + ]); + maybeInjectMaestroServer(ctx); + expect(ctx.argv).toEqual([ + 'maestro', '--udid', '61031VDCR0004B', '--platform', 'android', 'test', + '-e', 'PERCY_SERVER=http://localhost:5338', + 'flow.yaml' + ]); + }); + + it('detects deeper -e PERCY_SERVER override when global flags precede `test`', () => { + const argv = [ + 'maestro', '--udid', '61031VDCR0004B', 'test', + '-e', 'PERCY_SERVER=http://custom:9999', 'flow.yaml' + ]; + const ctx = ctxFor(argv); + maybeInjectMaestroServer(ctx); + expect(ctx.argv).toEqual(argv); + }); + }); + + describe('maybeInjectScreenshotDir', () => { + let originalEnvValue; + + beforeEach(() => { + originalEnvValue = process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + delete process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + }); + + afterEach(() => { + if (originalEnvValue === undefined) { + delete process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + } else { + process.env.PERCY_MAESTRO_SCREENSHOT_DIR = originalEnvValue; + } + }); + + function ctxFor(argv) { + return { argv: [...argv] }; + } + + it('injects --test-output-dir and sets env var to /.percy-out on happy path', () => { + const mkdir = spyOn(fs, 'mkdirSync').and.callFake(() => {}); + const expectedDir = path.join(process.cwd(), '.percy-out'); + const ctx = ctxFor(['maestro', 'test', 'flow.yaml']); + maybeInjectScreenshotDir(ctx); + expect(mkdir).toHaveBeenCalledWith(expectedDir, { recursive: true }); + expect(process.env.PERCY_MAESTRO_SCREENSHOT_DIR).toBe(expectedDir); + expect(ctx.argv).toEqual([ + 'maestro', 'test', '--test-output-dir', expectedDir, 'flow.yaml' + ]); + }); + + it('honors customer-set PERCY_MAESTRO_SCREENSHOT_DIR and injects flag aligned to it', () => { + process.env.PERCY_MAESTRO_SCREENSHOT_DIR = '/custom/screenshot/dir'; + const mkdir = spyOn(fs, 'mkdirSync').and.callFake(() => {}); + const ctx = ctxFor(['maestro', 'test', 'flow.yaml']); + maybeInjectScreenshotDir(ctx); + expect(mkdir).not.toHaveBeenCalled(); + expect(process.env.PERCY_MAESTRO_SCREENSHOT_DIR).toBe('/custom/screenshot/dir'); + expect(ctx.argv).toEqual([ + 'maestro', 'test', '--test-output-dir', '/custom/screenshot/dir', 'flow.yaml' + ]); + }); + + it('honors customer-supplied --test-output-dir and mirrors value into env var', () => { + const mkdir = spyOn(fs, 'mkdirSync').and.callFake(() => {}); + const ctx = ctxFor(['maestro', 'test', '--test-output-dir', '/custom/path', 'flow.yaml']); + maybeInjectScreenshotDir(ctx); + expect(mkdir).not.toHaveBeenCalled(); + expect(process.env.PERCY_MAESTRO_SCREENSHOT_DIR).toBe('/custom/path'); + // argv unchanged — customer's flag stays where they put it + expect(ctx.argv).toEqual(['maestro', 'test', '--test-output-dir', '/custom/path', 'flow.yaml']); + }); + + it('skips entirely when both env var and --test-output-dir are customer-set', () => { + process.env.PERCY_MAESTRO_SCREENSHOT_DIR = '/from/env'; + const mkdir = spyOn(fs, 'mkdirSync').and.callFake(() => {}); + const argv = ['maestro', 'test', '--test-output-dir', '/from/flag', 'flow.yaml']; + const ctx = ctxFor(argv); + maybeInjectScreenshotDir(ctx); + expect(mkdir).not.toHaveBeenCalled(); + expect(process.env.PERCY_MAESTRO_SCREENSHOT_DIR).toBe('/from/env'); + expect(ctx.argv).toEqual(argv); + }); + + it('falls back to /percy-maestro- on EACCES and emits WARN', () => { + const mkdir = spyOn(fs, 'mkdirSync').and.callFake((dirPath) => { + if (dirPath === path.join(process.cwd(), '.percy-out')) { + const err = new Error('EACCES'); err.code = 'EACCES'; + throw err; + } + }); + const log = { warn: jasmine.createSpy('warn') }; + const ctx = ctxFor(['maestro', 'test', 'flow.yaml']); + maybeInjectScreenshotDir(ctx, log); + const fallback = path.join(os.tmpdir(), `percy-maestro-${process.pid}`); + expect(mkdir).toHaveBeenCalledWith(fallback, { recursive: true }); + expect(process.env.PERCY_MAESTRO_SCREENSHOT_DIR).toBe(fallback); + expect(ctx.argv).toEqual([ + 'maestro', 'test', '--test-output-dir', fallback, 'flow.yaml' + ]); + expect(log.warn).toHaveBeenCalledTimes(1); + expect(log.warn.calls.argsFor(0)[0]).toContain('EACCES'); + expect(log.warn.calls.argsFor(0)[0]).toContain(fallback); + }); + + it('falls back on EROFS (read-only filesystem)', () => { + spyOn(fs, 'mkdirSync').and.callFake((dirPath) => { + if (dirPath === path.join(process.cwd(), '.percy-out')) { + const err = new Error('EROFS'); err.code = 'EROFS'; + throw err; + } + }); + const log = { warn: jasmine.createSpy('warn') }; + const ctx = ctxFor(['maestro', 'test', 'flow.yaml']); + maybeInjectScreenshotDir(ctx, log); + const fallback = path.join(os.tmpdir(), `percy-maestro-${process.pid}`); + expect(process.env.PERCY_MAESTRO_SCREENSHOT_DIR).toBe(fallback); + expect(log.warn).toHaveBeenCalled(); + }); + + it('falls back on EEXIST (.percy-out collides with a non-directory)', () => { + spyOn(fs, 'mkdirSync').and.callFake((dirPath) => { + if (dirPath === path.join(process.cwd(), '.percy-out')) { + const err = new Error('EEXIST'); err.code = 'EEXIST'; + throw err; + } + }); + const log = { warn: jasmine.createSpy('warn') }; + const ctx = ctxFor(['maestro', 'test', 'flow.yaml']); + maybeInjectScreenshotDir(ctx, log); + const fallback = path.join(os.tmpdir(), `percy-maestro-${process.pid}`); + expect(process.env.PERCY_MAESTRO_SCREENSHOT_DIR).toBe(fallback); + expect(log.warn).toHaveBeenCalled(); + }); + + it('skips for `maestro hierarchy` (not a test command)', () => { + const mkdir = spyOn(fs, 'mkdirSync').and.callFake(() => {}); + const argv = ['maestro', 'hierarchy', '--udid', 'X']; + const ctx = ctxFor(argv); + maybeInjectScreenshotDir(ctx); + expect(mkdir).not.toHaveBeenCalled(); + expect(process.env.PERCY_MAESTRO_SCREENSHOT_DIR).toBeUndefined(); + expect(ctx.argv).toEqual(argv); + }); + + it('skips for `npx maestro test` (argv[0] is npx, not maestro)', () => { + const mkdir = spyOn(fs, 'mkdirSync').and.callFake(() => {}); + const argv = ['npx', 'maestro', 'test', 'flow.yaml']; + const ctx = ctxFor(argv); + maybeInjectScreenshotDir(ctx); + expect(mkdir).not.toHaveBeenCalled(); + expect(ctx.argv).toEqual(argv); + }); + + it('skips for non-maestro commands', () => { + const mkdir = spyOn(fs, 'mkdirSync').and.callFake(() => {}); + const argv = ['python', 'test.py']; + const ctx = ctxFor(argv); + maybeInjectScreenshotDir(ctx); + expect(mkdir).not.toHaveBeenCalled(); + expect(ctx.argv).toEqual(argv); + }); + + it('skips when args has fewer than two elements', () => { + const mkdir = spyOn(fs, 'mkdirSync').and.callFake(() => {}); + const argv = ['maestro']; + const ctx = ctxFor(argv); + maybeInjectScreenshotDir(ctx); + expect(mkdir).not.toHaveBeenCalled(); + expect(ctx.argv).toEqual(argv); + }); + + it('matches by basename when the command is an absolute path', () => { + const mkdir = spyOn(fs, 'mkdirSync').and.callFake(() => {}); + const expectedDir = path.join(process.cwd(), '.percy-out'); + const ctx = ctxFor(['/Users/foo/.maestro/bin/maestro', 'test', 'flow.yaml']); + maybeInjectScreenshotDir(ctx); + expect(mkdir).toHaveBeenCalledWith(expectedDir, { recursive: true }); + expect(ctx.argv).toEqual([ + '/Users/foo/.maestro/bin/maestro', 'test', + '--test-output-dir', expectedDir, + 'flow.yaml' + ]); + }); + + // Mirror the maestro-server tests: `test` may follow global parent + // flags (--udid, --device, --platform, ...), so the helper must + // locate `test` and splice --test-output-dir right after it. + it('injects --test-output-dir after `test` when --udid precedes the subcommand', () => { + const mkdir = spyOn(fs, 'mkdirSync').and.callFake(() => {}); + const expectedDir = path.join(process.cwd(), '.percy-out'); + const ctx = ctxFor(['maestro', '--udid', '61031VDCR0004B', 'test', 'flow.yaml']); + maybeInjectScreenshotDir(ctx); + expect(mkdir).toHaveBeenCalledWith(expectedDir, { recursive: true }); + expect(ctx.argv).toEqual([ + 'maestro', '--udid', '61031VDCR0004B', 'test', + '--test-output-dir', expectedDir, + 'flow.yaml' + ]); + }); + + it('injects --test-output-dir after `test` when --platform=android precedes the subcommand', () => { + const mkdir = spyOn(fs, 'mkdirSync').and.callFake(() => {}); + const expectedDir = path.join(process.cwd(), '.percy-out'); + const ctx = ctxFor(['maestro', '--platform=android', 'test', 'flow.yaml']); + maybeInjectScreenshotDir(ctx); + expect(mkdir).toHaveBeenCalledWith(expectedDir, { recursive: true }); + expect(ctx.argv).toEqual([ + 'maestro', '--platform=android', 'test', + '--test-output-dir', expectedDir, + 'flow.yaml' + ]); + }); + + it('detects deeper customer --test-output-dir when global flags precede `test`', () => { + const mkdir = spyOn(fs, 'mkdirSync').and.callFake(() => {}); + const ctx = ctxFor([ + 'maestro', '--udid', '61031VDCR0004B', 'test', + '--test-output-dir', '/custom/path', 'flow.yaml' + ]); + maybeInjectScreenshotDir(ctx); + expect(mkdir).not.toHaveBeenCalled(); + expect(process.env.PERCY_MAESTRO_SCREENSHOT_DIR).toBe('/custom/path'); + expect(ctx.argv).toEqual([ + 'maestro', '--udid', '61031VDCR0004B', 'test', + '--test-output-dir', '/custom/path', 'flow.yaml' + ]); + }); + + // --test-output-dir= is valid picocli syntax (single token, `=`). + // Without equals-form handling, the helper would think no flag is set, + // inject a SECOND --test-output-dir, and overwrite PERCY_MAESTRO_SCREENSHOT_DIR + // to the auto-resolved .percy-out — Maestro then writes screenshots to one + // dir while the relay reads from another → silent all-404 snapshots. + it('honors customer-supplied --test-output-dir= equals-form', () => { + const mkdir = spyOn(fs, 'mkdirSync').and.callFake(() => {}); + const ctx = ctxFor(['maestro', 'test', '--test-output-dir=/custom/eq', 'flow.yaml']); + maybeInjectScreenshotDir(ctx); + expect(mkdir).not.toHaveBeenCalled(); + expect(process.env.PERCY_MAESTRO_SCREENSHOT_DIR).toBe('/custom/eq'); + // argv unchanged — no duplicate --test-output-dir injection + expect(ctx.argv).toEqual([ + 'maestro', 'test', '--test-output-dir=/custom/eq', 'flow.yaml' + ]); + }); + + it('honors --test-output-dir= when global flags precede `test`', () => { + const mkdir = spyOn(fs, 'mkdirSync').and.callFake(() => {}); + const ctx = ctxFor([ + 'maestro', '--udid', '61031VDCR0004B', 'test', + '--test-output-dir=/custom/eq', 'flow.yaml' + ]); + maybeInjectScreenshotDir(ctx); + expect(mkdir).not.toHaveBeenCalled(); + expect(process.env.PERCY_MAESTRO_SCREENSHOT_DIR).toBe('/custom/eq'); + expect(ctx.argv).toEqual([ + 'maestro', '--udid', '61031VDCR0004B', 'test', + '--test-output-dir=/custom/eq', 'flow.yaml' + ]); + }); + + it('treats env var as override when both env and --test-output-dir= equals-form are set', () => { + process.env.PERCY_MAESTRO_SCREENSHOT_DIR = '/from/env'; + const mkdir = spyOn(fs, 'mkdirSync').and.callFake(() => {}); + const argv = ['maestro', 'test', '--test-output-dir=/from/eq', 'flow.yaml']; + const ctx = ctxFor(argv); + maybeInjectScreenshotDir(ctx); + expect(mkdir).not.toHaveBeenCalled(); + // env wins on read; argv untouched (both customer-set → fully passive) + expect(process.env.PERCY_MAESTRO_SCREENSHOT_DIR).toBe('/from/env'); + expect(ctx.argv).toEqual(argv); + }); + + // An empty equals-form value `--test-output-dir=` (and the space-form + // equivalent `--test-output-dir ''`) is a user error / typo. Without + // the empty-string guard the helper would treat the empty string as a + // customer-set value, leaving PERCY_MAESTRO_SCREENSHOT_DIR unset AND + // skipping the auto-resolve fallback — Maestro defaults its own output + // dir while the SDK reads an empty env var. Producing all-404 snapshots + // silently. The helper instead falls through to the auto-resolve path + // as if the flag were absent. + it('treats empty --test-output-dir= equals-form value as absent (falls through to auto-resolve)', () => { + const mkdir = spyOn(fs, 'mkdirSync').and.callFake(() => {}); + const expectedDir = path.join(process.cwd(), '.percy-out'); + const ctx = ctxFor(['maestro', 'test', '--test-output-dir=', 'flow.yaml']); + maybeInjectScreenshotDir(ctx); + expect(mkdir).toHaveBeenCalledWith(expectedDir, { recursive: true }); + expect(process.env.PERCY_MAESTRO_SCREENSHOT_DIR).toBe(expectedDir); + // argv: original empty --test-output-dir= preserved AND the resolved + // value spliced in (Maestro picocli will accept the latter override). + expect(ctx.argv).toEqual([ + 'maestro', 'test', '--test-output-dir', expectedDir, + '--test-output-dir=', 'flow.yaml' + ]); + }); + + it('treats empty space-form --test-output-dir "" as absent (falls through to auto-resolve)', () => { + const mkdir = spyOn(fs, 'mkdirSync').and.callFake(() => {}); + const expectedDir = path.join(process.cwd(), '.percy-out'); + const ctx = ctxFor(['maestro', 'test', '--test-output-dir', '', 'flow.yaml']); + maybeInjectScreenshotDir(ctx); + expect(mkdir).toHaveBeenCalledWith(expectedDir, { recursive: true }); + expect(process.env.PERCY_MAESTRO_SCREENSHOT_DIR).toBe(expectedDir); + expect(ctx.argv).toEqual([ + 'maestro', 'test', '--test-output-dir', expectedDir, + '--test-output-dir', '', 'flow.yaml' + ]); + }); + }); }); diff --git a/packages/core/src/api.js b/packages/core/src/api.js index 6a4ec27e6..c3dd2d6a8 100644 --- a/packages/core/src/api.js +++ b/packages/core/src/api.js @@ -425,15 +425,19 @@ export function createPercyServer(percy, port) { let { name, sessionId } = req.body || {}; if (!name) throw new ServerError(400, 'Missing required field: name'); - if (!sessionId) throw new ServerError(400, 'Missing required field: sessionId'); + + // Self-hosted vs BrowserStack is signaled by sessionId presence: BS + // host-injection always supplies it; self-hosted runs never do. + let selfHosted = !sessionId; // Strict character-class validation — rejects path separators, shell metacharacters, // NUL, newlines, and anything else that could confuse the glob or the filesystem. + // `name` is load-bearing for the recursive glob — must not be loosened. const SAFE_ID = /^[a-zA-Z0-9_-]+$/; if (!SAFE_ID.test(name)) { throw new ServerError(400, 'Invalid screenshot name'); } - if (!SAFE_ID.test(sessionId)) { + if (sessionId && !SAFE_ID.test(sessionId)) { throw new ServerError(400, 'Invalid sessionId'); } @@ -473,6 +477,49 @@ export function createPercyServer(percy, port) { suppliedFilePath = req.body.filePath; } + // Resolve the file-find scope root. On BrowserStack (sessionId present), + // the root is the BS host's /tmp/{sessionId}{_test_suite} convention. + // Self-hosted (sessionId absent) requires PERCY_MAESTRO_SCREENSHOT_DIR + // (read from process.env, never the request body) to be an absolute, + // existing directory — typically the customer's + // `maestro test --test-output-dir ` path. The realpath + prefix + // check below enforces the security invariant at whichever root applies; + // the boundary is relocated, not removed. + let scopeRoot; + if (selfHosted) { + // Reject filePath outright in self-hosted mode. The SDK never emits + // it (it sends a relative SCREENSHOT_NAME); honoring an absolute + // filePath against a caller-influenceable root would re-open + // arbitrary in-root reads. + if (suppliedFilePath) { + throw new ServerError(400, 'filePath is not accepted in self-hosted mode (omit it; PERCY_MAESTRO_SCREENSHOT_DIR + relative SCREENSHOT_NAME is the supported path)'); + } + let dir = process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + if (!dir) { + throw new ServerError(400, 'Missing required env: PERCY_MAESTRO_SCREENSHOT_DIR (set it to your `maestro test --test-output-dir` path)'); + } + if (!path.isAbsolute(dir)) { + throw new ServerError(400, 'PERCY_MAESTRO_SCREENSHOT_DIR must be an absolute path'); + } + // UX guard ONLY: surface an actionable 400 ("dir not found") instead + // of the opaque 404 the realpath+prefix containment check below would + // emit for a missing dir. There is a small TOCTOU window between this + // stat and the realpath at line 647 — that window is acceptable here + // because realpath (not stat) is the security invariant: even if the + // dir is replaced with a symlink in between, realpath resolves the + // target and the sep-prefix check rejects anything outside scopeRoot. + let stat; + try { stat = await fs.promises.stat(dir); } catch { stat = null; } + if (!stat || !stat.isDirectory()) { + throw new ServerError(400, `PERCY_MAESTRO_SCREENSHOT_DIR is not an existing directory: ${dir}`); + } + scopeRoot = dir; + } else { + scopeRoot = platform === 'ios' + ? `/tmp/${sessionId}` + : `/tmp/${sessionId}_test_suite`; + } + // Validate regions input shape early (before file I/O and ADB work) so // malformed requests don't consume resolver/relay work. Three parallel // input arrays share the same per-item shape; algorithm semantics differ @@ -529,21 +576,44 @@ export function createPercyServer(percy, port) { // {device}_maestro_debug_ root. The `**` recursive match handles any depth. // Exact {name}.png match at the leaf filters out Maestro's emoji-prefixed // debug frames (e.g., `screenshot-❌--(flow).png`). - let searchPattern = platform === 'ios' - ? `/tmp/${sessionId}/*_maestro_debug_*/**/${name}.png` - : `/tmp/${sessionId}_test_suite/logs/*/screenshots/${name}.png`; + let searchPattern; + if (selfHosted) { + // Self-hosted: recursive glob under the customer's --test-output-dir + // (PERCY_MAESTRO_SCREENSHOT_DIR). Recursive depth handles arbitrary + // Maestro layouts; `name` is SAFE_ID-validated above so it cannot + // contain separators or traversal characters. + // + // fast-glob requires forward-slashes in patterns on every platform + // (per its docs: "Always use forward-slashes in glob expressions"). + // On Windows the scopeRoot from path.resolve contains backslashes, + // so we normalize before embedding into the pattern. Production- + // code Windows portability — verified by the CI Windows runner. + const globRoot = scopeRoot.replace(/\\/g, '/'); + searchPattern = `${globRoot}/**/${name}.png`; + } else { + searchPattern = platform === 'ios' + ? `/tmp/${sessionId}/*_maestro_debug_*/**/${name}.png` + : `/tmp/${sessionId}_test_suite/logs/*/screenshots/${name}.png`; + } let files; try { let { default: glob } = await import('fast-glob'); - files = await glob(searchPattern); + // Self-hosted needs `dot: true` because Maestro's default output + // directory is `.maestro/` — a dot-prefixed entry that fast-glob + // hides by default. BS layouts have no dot-prefixed segments, so + // omitting the option there keeps the byte-identical behavior. + files = await glob(searchPattern, selfHosted ? { dot: true } : undefined); } catch { // Fast-glob import / glob call failed — fall back to manual walker. // See manualScreenshotWalk() at file top for the rationale + the // file-level .semgrepignore covering path-traversal sinks inside. + // Self-hosted has no walker fallback (no fixed-layout convention) — + // empty files → 404 with the actionable PERCY_MAESTRO_SCREENSHOT_DIR + // guidance above. /* istanbul ignore next — only fires when fast-glob import throws (broken install / FS corruption); integration-test territory. */ - files = await manualScreenshotWalk(platform, sessionId, name); + files = selfHosted ? [] : await manualScreenshotWalk(platform, sessionId, name); } if (!files || files.length === 0) { @@ -568,22 +638,27 @@ export function createPercyServer(percy, port) { } } - // Canonicalize and confirm the resolved path still lives under the sessionId-owned dir. - // Defeats symlink swaps where a sessionId-named dir points elsewhere. - // We resolve both the file and the expected prefix because /tmp is a symlink on macOS - // (iOS hosts run macOS, where /tmp → /private/tmp). - let expectedSessionRoot = platform === 'ios' - ? `/tmp/${sessionId}` - : `/tmp/${sessionId}_test_suite`; + // Canonicalize and confirm the resolved path still lives under scopeRoot. + // Defeats symlink swaps where the root points elsewhere. Both ends are + // realpath'd because /tmp is a symlink on macOS (where iOS hosts run). + // The trailing `/` on the prefix is load-bearing — it prevents + // sibling-prefix bypass (e.g. /x/.maestro vs /x/.maestro-secrets). + // + // Normalize both sides to forward-slashes before the prefix check so + // the same code works on Windows (real-fs returns backslashes) AND on + // POSIX (no-op) AND on memfs in tests (POSIX-style virtual paths + // regardless of host OS). let realPath, realPrefix; try { realPath = await fs.promises.realpath(chosenFile); - realPrefix = await fs.promises.realpath(expectedSessionRoot); + realPrefix = await fs.promises.realpath(scopeRoot); } catch { throw new ServerError(404, `Screenshot not found: ${name}.png (path resolution failed)`); } - if (!realPath.startsWith(`${realPrefix}/`)) { - throw new ServerError(404, `Screenshot not found: ${name}.png (resolved outside session dir)`); + const realPathFwd = realPath.replace(/\\/g, '/'); + const realPrefixFwd = realPrefix.replace(/\\/g, '/'); + if (!realPathFwd.startsWith(`${realPrefixFwd}/`)) { + throw new ServerError(404, `Screenshot not found: ${name}.png (resolved outside ${selfHosted ? 'PERCY_MAESTRO_SCREENSHOT_DIR' : 'session dir'})`); } // Read and base64-encode the screenshot @@ -715,7 +790,10 @@ export function createPercyServer(percy, port) { if (cachedDump === null) { // Thread the per-Percy gRPC client cache so the Android gRPC // primary path can reuse channels across snapshots in the same - // session (D9 of 2026-05-07-002 plan). iOS path ignores it. + // session (D9 of 2026-05-07-002 plan). iOS path ignores it + // (the iOS resolver reads PERCY_IOS_DRIVER_HOST_PORT directly; + // no per-session port cache needed since the port is prescribed + // upstream by `@percy/cli-app`'s `maybeInjectDriverHostPort`). cachedDump = await maestroDump({ platform, sessionId, diff --git a/packages/core/src/maestro-hierarchy.js b/packages/core/src/maestro-hierarchy.js index b7a8ae35a..aec35f696 100644 --- a/packages/core/src/maestro-hierarchy.js +++ b/packages/core/src/maestro-hierarchy.js @@ -101,10 +101,16 @@ const SELECTOR_KEYS_UNION = ['resource-id', 'text', 'content-desc', 'class', 'id // past the socket timeout. const IOS_HTTP_HEALTHY_DEADLINE_MS = 1500; const IOS_HTTP_CIRCUIT_BREAKER_MS = 5000; -// Maestro iOS driver-host-port is realmobile-derived as wda_port + 2700. -// WDA ports are 8400-8410 → driver host ports are 11100-11110. -const IOS_DRIVER_HOST_PORT_MIN = 11100; -const IOS_DRIVER_HOST_PORT_MAX = 11110; +// `parseIosDriverHostPort` accepts any valid TCP port (1-65535). The +// BrowserStack-realmobile canonical range (11100-11110, derived as +// wda_port + 2700 from WDA ports 8400-8410) is a strict subset. Self-hosted +// callers prescribe the port via `@percy/cli-app`'s `maybeInjectDriverHostPort` +// helper, which auto-injects `--driver-host-port ` into the +// `maestro test` argv and mirrors the chosen value to +// `PERCY_IOS_DRIVER_HOST_PORT`. This relay reads the env var unconditionally +// and no longer auto-discovers the port via probe / lsof — see Phase 3 of +// the self-hosted Maestro architecture doc and brainstorm +// `2026-06-11-ios-port-discovery-simplification-requirements.md`. // HTTP response cap before parse — sized for WebView-heavy iOS apps. const IOS_HTTP_RESPONSE_MAX_BYTES = 20 * 1024 * 1024; @@ -902,17 +908,23 @@ function defaultHttpRequest({ host, port, method, path: requestPath, headers, bo }); } -// Validate PERCY_IOS_DRIVER_HOST_PORT env value as integer in the realmobile -// range (wda_port + 2700 = 11100-11110). Out-of-range values return null. +// Validate PERCY_IOS_DRIVER_HOST_PORT env value as an integer in the valid +// TCP range (1-65535). Was previously clamped to the BS-specific realmobile +// range (wda_port+2700 = 11100-11110); the relaxed range is a superset so BS +// continues to accept its canonical port unchanged. Self-hosted callers can +// pass any port they pinned via `maestro test --driver-host-port

` (e.g. +// the deterministic `7001` on a simulator, or a forwarded port like ~6001 on +// a real device). function parseIosDriverHostPort(raw) { /* istanbul ignore if — undefined/null/empty raw value branch; iOS dispatch pre-checks PERCY_IOS_DRIVER_HOST_PORT before calling, so these never fire. */ if (raw === undefined || raw === null || raw === '') return null; const n = Number(raw); /* istanbul ignore if — non-integer port (e.g. NaN from non-numeric env); - env var is set by realmobile as the canonical wda_port+2700 integer. */ + env var is set by realmobile as the canonical wda_port+2700 integer or by + a self-hosted customer as their explicit `--driver-host-port` value. */ if (!Number.isInteger(n)) return null; - if (n < IOS_DRIVER_HOST_PORT_MIN || n > IOS_DRIVER_HOST_PORT_MAX) return null; + if (n < 1 || n > 65535) return null; return n; } @@ -1260,15 +1272,51 @@ export async function dump(options) { const started = Date.now(); if (platform === 'ios') { - // iOS dispatch: read realmobile-injected env vars; warn-skip if absent. const udid = getEnv('PERCY_IOS_DEVICE_UDID'); - const driverHostPortRaw = getEnv('PERCY_IOS_DRIVER_HOST_PORT'); - if (!udid || !driverHostPortRaw) { - log.warn(`iOS resolver env-missing: udid=${udid ? 'set' : 'unset'} driver_port=${driverHostPortRaw ? 'set' : 'unset'}`); + let driverHostPortRaw = getEnv('PERCY_IOS_DRIVER_HOST_PORT'); + + // Self-hosted env-absent path: probe the Maestro 2.4.0 documented + // single-simulator default port (127.0.0.1:7001) before giving up. + // Maestro 2.4.0's TestCommand binds the driver to 7001 deterministically + // for the single-simulator case, so a single HTTP probe resolves the + // dominant self-hosted scenario without any port discovery from outside + // the process. Older releases used the same default; newer releases + // (2.6+, ephemeral port via ServerSocket(0)) require the customer to + // pin via `PERCY_IOS_DRIVER_HOST_PORT` (or `maestro --driver-host-port`, + // which 2.6+ accepts at the parent level). + // + // `PERCY_IOS_DRIVER_HOST_PORT` is the only signal the explicit branch + // below reads. Set by: + // * BrowserStack hosts (canonical 11100-11110 range, via + // `cli_manager.rb#start_percy_cli` env injection) + // * Customers who pin the driver host port themselves + // + // If neither the env var nor a live driver on 7001 is found, element + // regions degrade gracefully with `env-missing` — coordinate regions + // and the snapshot itself continue to upload via the relay's other + // paths. + if (!driverHostPortRaw) { + const probe = await runIosHttpDump({ port: 7001, sessionId, httpRequest }); + if (probe.kind === 'hierarchy' || probe.kind === 'no-aut-tree') { + log.debug(`dump took ${Date.now() - started}ms via maestro-http (self-hosted probe :7001, ${probe.kind === 'hierarchy' ? `${probe.nodes.length} nodes` : probe.reason})`); + if (probe.kind === 'hierarchy') { + recordResolverSuccess({ platform: 'ios', via: 'maestro-http' }); + } else { + recordResolverFinalFailure({ platform: 'ios', failureClass: failureClassFromReason(probe.reason) }); + } + return probe; + } + log.warn('[percy] iOS element regions unavailable — no Maestro driver responded on 127.0.0.1:7001. Set PERCY_IOS_DRIVER_HOST_PORT if your driver binds a non-default port (e.g. Maestro 2.6+ ephemeral or sharded runs).'); recordResolverFinalFailure({ platform: 'ios', failureClass: 'other' }); return { kind: 'unavailable', reason: 'env-missing' }; } + // ─── Explicit — PERCY_IOS_DRIVER_HOST_PORT present ───────────────────── + // BrowserStack (host-injected) and self-hosted-with-explicit-port both + // take this path. The HTTP primary runs against the supplied port; the + // CLI fallback runs only when UDID is also present (without UDID + // there's no way to target a specific device via `maestro --udid`). + // D3 kill switch (PERCY_MAESTRO_GRPC=0): same env name gates BOTH Maestro // primaries. On iOS this skips runIosHttpDump and routes straight to the // maestro-CLI fallback below. Read every call so toggling at runtime is @@ -1278,8 +1326,10 @@ export async function dump(options) { log.warn('PERCY_MAESTRO_GRPC=0 kill switch active; skipping iOS HTTP primary'); } - // Validate driver-host-port range before attempting HTTP. Out-of-range - // values skip the HTTP path entirely and fall through to maestro-CLI. + // Validate driver-host-port — integer 1-65535 (relaxed from the BS + // 11100-11110 range; BS values stay valid as a subset). Out-of-range + // values skip the HTTP path entirely and fall through to maestro-CLI + // (when UDID is present) or warn-skip (when not). const driverHostPort = parseIosDriverHostPort(driverHostPortRaw); let httpResult = null; if (!iosKillSwitch && driverHostPort !== null) { @@ -1308,6 +1358,15 @@ export async function dump(options) { log.info(`[percy] hierarchy: maestro-http failed (other: ${oorReason}) → falling back to maestro-cli-fallback`); } + // CLI fallback requires UDID. Without it (port-only self-hosted), the + // HTTP attempt above was the only available path — emit warn-skip with + // an actionable message rather than running maestro without a target. + if (!udid) { + log.warn('[percy] iOS resolver: PERCY_IOS_DEVICE_UDID absent — no CLI fallback. HTTP primary either succeeded above and returned, or failed and there is no further path. Set UDID to enable the CLI fallback.'); + recordResolverFinalFailure({ platform: 'ios', failureClass: 'other' }); + return { kind: 'unavailable', reason: 'self-hosted-no-udid' }; + } + const cliResult = await runMaestroIosDump(udid, driverHostPort ?? driverHostPortRaw, execMaestro, getEnv); const httpReason = httpResult ? `${httpResult.kind}/${httpResult.reason}` : 'out-of-range-port'; log.debug(`dump took ${Date.now() - started}ms via maestro-cli-fallback (${httpReason}) kind=${cliResult.kind}`); diff --git a/packages/core/test/api.test.js b/packages/core/test/api.test.js index e22a4c2c7..88f2a6dc1 100644 --- a/packages/core/test/api.test.js +++ b/packages/core/test/api.test.js @@ -1,3 +1,4 @@ +import os from 'os'; import path from 'path'; import PercyConfig from '@percy/config'; import { logger, setupTest, fs } from './helpers/index.js'; @@ -17,7 +18,13 @@ describe('API Server', () => { beforeEach(async () => { process.env.PERCY_FORCE_PKG_VALUE = JSON.stringify({ name: '@percy/client', version: '1.0.0' }); jasmine.DEFAULT_TIMEOUT_INTERVAL = 150000; - await setupTest(); + // The self-hosted maestro tests exercise a recursive `**` + `dot:true` + // fast-glob against a `.maestro/` directory. fast-glob's recursive-dot + // traversal is unreliable against memfs across volume resets in the full + // suite (works in isolation; returns [] mid-suite), so route the + // self-hosted root to the REAL filesystem — this also tests the true + // production glob path. Only paths under this unique root are affected. + await setupTest({ filesystem: { $bypass: [p => typeof p === 'string' && p.includes('percy-self-hosted-real')] } }); percy = new Percy({ token: 'PERCY_TOKEN', @@ -1214,9 +1221,21 @@ describe('API Server', () => { await expectAsync(postMaestro({ sessionId: SID })).toBeRejectedWithError(/Missing required field: name/); }); - it('rejects missing sessionId with 400', async () => { - await percy.start(); - await expectAsync(postMaestro({ name: SS_NAME })).toBeRejectedWithError(/Missing required field: sessionId/); + it('400s missing sessionId + missing PERCY_MAESTRO_SCREENSHOT_DIR (self-hosted mode requires the env var)', async () => { + // `sessionId` absent is the self-hosted detection signal. Without + // PERCY_MAESTRO_SCREENSHOT_DIR set, the relay 400s with actionable + // guidance rather than 404'ing on a glob it cannot scope. The + // self-hosted happy path is covered in the dedicated describe below. + let prior = process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + delete process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + try { + await percy.start(); + await expectAsync(postMaestro({ name: SS_NAME })) + .toBeRejectedWithError(/Missing required env: PERCY_MAESTRO_SCREENSHOT_DIR/); + } finally { + if (prior === undefined) delete process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + else process.env.PERCY_MAESTRO_SCREENSHOT_DIR = prior; + } }); it('rejects invalid platform with 400', async () => { @@ -1851,6 +1870,187 @@ describe('API Server', () => { expect(payload.tag.width).toBe(1179); expect(payload.tag.height).toBe(2556); }); + + // ───────────────────────────────────────────────────────────────────── + // Self-hosted mode: `sessionId` absent triggers PERCY_MAESTRO_SCREENSHOT_DIR + // resolution. The BS path (sessionId present) is byte-identical and + // covered above; these tests lock the new branches. + // ───────────────────────────────────────────────────────────────────── + describe('self-hosted (sessionId absent)', () => { + // Real-fs root (matched by the $bypass registered in the top-level + // beforeEach) so the recursive `**` + `dot:true` glob runs against the + // real filesystem — the production path — rather than memfs. + // Use os.tmpdir() (not hardcoded `/tmp/`) so the fixtures work on + // Windows runners too — Windows has no `/tmp/`; the CI fails 404 on + // every glob when the root never gets created. + const SELF_HOSTED_ROOT = path.join(os.tmpdir(), 'percy-self-hosted-real-root'); + const NESTED_SUBDIR = path.join(SELF_HOSTED_ROOT, '.maestro', 'run-x', 'screenshots'); + const SELF_HOSTED_NAME = 'SelfHostedScreen'; + let priorEnv; + + beforeEach(() => { + priorEnv = process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + process.env.PERCY_MAESTRO_SCREENSHOT_DIR = SELF_HOSTED_ROOT; + fs.rmSync(SELF_HOSTED_ROOT, { recursive: true, force: true }); + fs.mkdirSync(NESTED_SUBDIR, { recursive: true }); + // Valid 24-byte PNG header (1080 x 2400) exercises PNG-fill on the + // self-hosted path too. + const pngHeader = Buffer.alloc(24); + Buffer.from([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]).copy(pngHeader, 0); + pngHeader.writeUInt32BE(13, 8); + Buffer.from('IHDR', 'ascii').copy(pngHeader, 12); + pngHeader.writeUInt32BE(1080, 16); + pngHeader.writeUInt32BE(2400, 20); + fs.writeFileSync(path.join(NESTED_SUBDIR, `${SELF_HOSTED_NAME}.png`), pngHeader); + }); + + afterEach(() => { + if (priorEnv === undefined) delete process.env.PERCY_MAESTRO_SCREENSHOT_DIR; + else process.env.PERCY_MAESTRO_SCREENSHOT_DIR = priorEnv; + // Clean up the real-fs fixture root (see beforeEach / top-level $bypass). + fs.rmSync(SELF_HOSTED_ROOT, { recursive: true, force: true }); + }); + + it('finds screenshot via recursive glob under PERCY_MAESTRO_SCREENSHOT_DIR and uploads without sessionId', async () => { + await percy.start(); + spyOn(percy, 'upload').and.resolveTo(); + let res = await postMaestro({ name: SELF_HOSTED_NAME, platform: 'android' }); + expect(res.success).toBe(true); + let [payload] = percy.upload.calls.mostRecent().args; + expect(payload.name).toBe(SELF_HOSTED_NAME); + expect(payload.tag.width).toBe(1080); + expect(payload.tag.height).toBe(2400); + expect(payload.tiles[0].content).toBeDefined(); + // sessionId is never forwarded into the upload payload (relay only + // used it for scoping; self-hosted has no equivalent). + expect(payload.sessionId).toBeUndefined(); + }); + + it('400s when PERCY_MAESTRO_SCREENSHOT_DIR is not absolute', async () => { + process.env.PERCY_MAESTRO_SCREENSHOT_DIR = 'relative/path'; + await percy.start(); + await expectAsync(postMaestro({ name: SELF_HOSTED_NAME })) + .toBeRejectedWithError(/PERCY_MAESTRO_SCREENSHOT_DIR must be an absolute path/); + }); + + it('400s when PERCY_MAESTRO_SCREENSHOT_DIR does not exist', async () => { + process.env.PERCY_MAESTRO_SCREENSHOT_DIR = path.join(os.tmpdir(), 'this-path-does-not-exist-percy-self-hosted'); + await percy.start(); + await expectAsync(postMaestro({ name: SELF_HOSTED_NAME })) + .toBeRejectedWithError(/PERCY_MAESTRO_SCREENSHOT_DIR is not an existing directory/); + }); + + it('400s when PERCY_MAESTRO_SCREENSHOT_DIR points to a file, not a directory', async () => { + const notADir = path.join(os.tmpdir(), 'percy-self-hosted-not-a-dir'); + fs.writeFileSync(notADir, 'plain-file'); + process.env.PERCY_MAESTRO_SCREENSHOT_DIR = notADir; + await percy.start(); + await expectAsync(postMaestro({ name: SELF_HOSTED_NAME })) + .toBeRejectedWithError(/PERCY_MAESTRO_SCREENSHOT_DIR is not an existing directory/); + }); + + it('rejects a supplied filePath in self-hosted mode (security invariant)', async () => { + // The SDK never emits filePath self-hosted; honoring it against a + // caller-influenceable root would re-open arbitrary in-root reads. + await percy.start(); + await expectAsync(postMaestro({ + name: SELF_HOSTED_NAME, + filePath: `${NESTED_SUBDIR}/${SELF_HOSTED_NAME}.png` + })).toBeRejectedWithError(/filePath is not accepted in self-hosted mode/); + }); + + it('404s when the screenshot is missing under the configured root', async () => { + await percy.start(); + await expectAsync(postMaestro({ name: 'NoSuchScreenshot' })) + .toBeRejectedWithError(/Screenshot not found/); + }); + + it('404s when a globbed file resolves outside the root (symlink escape)', async () => { + // A symlink inside the root that points outside it must not exfiltrate + // the target — the realpath + prefix check rejects it (self-hosted arm + // of the "resolved outside" guard). Uses real fs via the $bypass. + const outside = path.join(os.tmpdir(), 'percy-self-hosted-real-OUTSIDE.png'); + fs.writeFileSync(outside, 'OUTSIDE'); + fs.symlinkSync(outside, path.join(NESTED_SUBDIR, 'EscapeScreen.png')); + await percy.start(); + await expectAsync(postMaestro({ name: 'EscapeScreen', platform: 'android' })) + .toBeRejectedWithError(/resolved outside PERCY_MAESTRO_SCREENSHOT_DIR/); + fs.rmSync(outside, { force: true }); + }); + + it('rejects name with traversal characters (SAFE_ID is load-bearing for the recursive glob)', async () => { + await percy.start(); + await expectAsync(postMaestro({ name: '../etc/passwd' })) + .toBeRejectedWithError(/Invalid screenshot name/); + }); + + it('coordinate regions still pass through on the self-hosted path', async () => { + await percy.start(); + spyOn(percy, 'upload').and.resolveTo(); + await postMaestro({ + name: SELF_HOSTED_NAME, + platform: 'android', + regions: [{ top: 10, bottom: 50, left: 0, right: 100, algorithm: 'ignore' }] + }); + let [payload] = percy.upload.calls.mostRecent().args; + expect(payload.regions).toBeDefined(); + expect(payload.regions.length).toBe(1); + expect(payload.regions[0].elementSelector.boundingBox) + .toEqual({ x: 0, y: 10, width: 100, height: 40 }); + expect(payload.regions[0].algorithm).toBe('ignore'); + }); + + // Multi-match mtime selection — Maestro re-runs in the same root leave + // older fixtures behind under different timestamped sub-dirs. The relay + // picks the most-recently-modified match. This locks that determinism + // for the self-hosted code path (the BS arm has been exercising this + // branch via integration tests). + it('selects the most-recently-modified PNG when multiple matches exist under the root', async () => { + // Two PNG fixtures at different depths with distinct dimensions and + // mtimes. The newer one (larger size) must win. + const OLD_DIR = path.join(SELF_HOSTED_ROOT, '.maestro', 'run-old', 'screenshots'); + const NEW_DIR = path.join(SELF_HOSTED_ROOT, '.maestro', 'run-new', 'screenshots'); + fs.mkdirSync(OLD_DIR, { recursive: true }); + fs.mkdirSync(NEW_DIR, { recursive: true }); + + // Helper: a valid 24-byte PNG header with the supplied dimensions. + const pngOf = (w, h) => { + const buf = Buffer.alloc(24); + Buffer.from([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]).copy(buf, 0); + buf.writeUInt32BE(13, 8); + Buffer.from('IHDR', 'ascii').copy(buf, 12); + buf.writeUInt32BE(w, 16); + buf.writeUInt32BE(h, 20); + return buf; + }; + + // Same `name` under both — recursive glob will find both. + const NAME = 'MultiMatch'; + const oldFile = path.join(OLD_DIR, `${NAME}.png`); + const newFile = path.join(NEW_DIR, `${NAME}.png`); + fs.writeFileSync(oldFile, pngOf(720, 1280)); + fs.writeFileSync(newFile, pngOf(1080, 2400)); + + // Stamp explicit mtimes so the assertion doesn't depend on fs write + // order. The 60-second gap is chosen to survive CI filesystems with + // 1-second mtime resolution (ext4 without `relatime`, common older + // CI images) — at that floor the two writes could otherwise round + // to the same mtime and the ordering assertion would be racy. + // `fs.utimesSync` takes atime/mtime in SECONDS (POSIX `time_t`). + const now = Date.now() / 1000; + fs.utimesSync(oldFile, now - 60, now - 60); + fs.utimesSync(newFile, now, now); + + await percy.start(); + spyOn(percy, 'upload').and.resolveTo(); + let res = await postMaestro({ name: NAME, platform: 'android' }); + expect(res.success).toBe(true); + let [payload] = percy.upload.calls.mostRecent().args; + // Newer fixture's dimensions reach the payload. + expect(payload.tag.width).toBe(1080); + expect(payload.tag.height).toBe(2400); + }); + }); }); }); diff --git a/packages/core/test/unit/maestro-hierarchy.parity.test.js b/packages/core/test/unit/maestro-hierarchy.parity.test.js index 6ab5ceb86..8aff2abd6 100644 --- a/packages/core/test/unit/maestro-hierarchy.parity.test.js +++ b/packages/core/test/unit/maestro-hierarchy.parity.test.js @@ -90,10 +90,21 @@ describe('Unit / maestro-hierarchy / cross-platform parity', () => { }); it('iOS env-missing returns { kind: "unavailable", reason: "env-missing" }', async () => { - // Same envelope shape as Android-failure paths, just a different reason tag. - const res = await dump({ platform: 'ios', getEnv: () => undefined }); + // Same envelope shape as Android-failure paths, just a different reason + // tag. When PERCY_IOS_DRIVER_HOST_PORT is absent, the iOS dispatch + // probes the deterministic 127.0.0.1:7001 fallback once; if the probe + // is rejected (no Maestro driver listening) the dispatch returns + // env-missing and the snapshot continues to upload via other paths + // — element regions degrade gracefully. + const httpRequest = jasmine.createSpy('httpRequest').and.callFake(async () => { + throw Object.assign(new Error('econnrefused'), { code: 'ECONNREFUSED' }); + }); + const res = await dump({ platform: 'ios', getEnv: () => undefined, httpRequest }); expect(res.kind).toBe('unavailable'); expect(res.reason).toBe('env-missing'); + // The 7001 probe must be attempted exactly once before the warn-skip. + expect(httpRequest).toHaveBeenCalledTimes(1); + expect(httpRequest.calls.mostRecent().args[0].port).toBe(7001); }); it('iOS env-set with no http/maestro reachable returns same envelope kinds as Android failure paths', async () => { diff --git a/packages/core/test/unit/maestro-hierarchy.test.js b/packages/core/test/unit/maestro-hierarchy.test.js index 6597bfce9..59145b975 100644 --- a/packages/core/test/unit/maestro-hierarchy.test.js +++ b/packages/core/test/unit/maestro-hierarchy.test.js @@ -476,27 +476,103 @@ describe('Unit / maestro-hierarchy', () => { }); describe('iOS dispatch — env handling', () => { - it('returns env-missing when PERCY_IOS_DEVICE_UDID is unset', async () => { + // Self-hosted-with-explicit-port-but-no-UDID: enters the EXPLICIT branch + // (port present), runs the HTTP primary. With UDID absent, the CLI + // fallback is unavailable — on HTTP success the dump returns; on HTTP + // failure (connection-fail here), warn-skip with the new + // `self-hosted-no-udid` reason. + it('warn-skips with self-hosted-no-udid when port is set, udid is unset, and HTTP primary fails', async () => { const getEnv = key => { if (key === 'PERCY_IOS_DRIVER_HOST_PORT') return '11100'; return undefined; }; - const res = await dump({ platform: 'ios', getEnv }); - expect(res).toEqual({ kind: 'unavailable', reason: 'env-missing' }); + const httpRequest = async () => { throw Object.assign(new Error('econnrefused'), { code: 'ECONNREFUSED' }); }; + const res = await dump({ platform: 'ios', getEnv, httpRequest }); + expect(res).toEqual({ kind: 'unavailable', reason: 'self-hosted-no-udid' }); + }); + + // Self-hosted env-absent path: with PERCY_IOS_DRIVER_HOST_PORT unset, the + // relay probes Maestro 2.4.0's documented single-simulator default port + // (127.0.0.1:7001) before degrading. When the probe finds a live Maestro + // driver, element regions resolve through the same `runIosHttpDump` + // transport the BS path uses. When the probe fails, we warn-skip with + // env-missing — coordinate regions and the snapshot itself still upload. + // Maestro 2.6+ (ephemeral port) customers set PERCY_IOS_DRIVER_HOST_PORT + // explicitly to bypass the probe. + + it('probes 127.0.0.1:7001 when env is unset, returns the dump on hit', async () => { + let observedPort = null; + const minimalAxElement = JSON.stringify({ + axElement: { + elementType: 1, + identifier: 'com.example.app', + frame: { X: 0, Y: 0, Width: 100, Height: 100 }, + children: [] + } + }); + const httpRequest = async opts => { + observedPort = opts.port; + return { statusCode: 200, headers: { 'content-type': 'application/json' }, body: minimalAxElement }; + }; + const res = await dump({ platform: 'ios', getEnv: () => undefined, httpRequest }); + expect(observedPort).toBe(7001); + expect(res.kind).toBe('hierarchy'); }); - it('returns env-missing when PERCY_IOS_DRIVER_HOST_PORT is unset', async () => { + it('warn-skips with env-missing when port is unset and the 7001 probe finds no driver', async () => { const getEnv = key => { if (key === 'PERCY_IOS_DEVICE_UDID') return '00008110-000065081404401E'; return undefined; }; - const res = await dump({ platform: 'ios', getEnv }); + const httpRequest = jasmine.createSpy('httpRequest').and.callFake(async () => { + throw Object.assign(new Error('econnrefused'), { code: 'ECONNREFUSED' }); + }); + const res = await dump({ platform: 'ios', getEnv, httpRequest }); expect(res).toEqual({ kind: 'unavailable', reason: 'env-missing' }); + // The probe must have been attempted exactly once (port 7001). + expect(httpRequest).toHaveBeenCalledTimes(1); + expect(httpRequest.calls.mostRecent().args[0].port).toBe(7001); }); - it('returns env-missing when both env vars are unset', async () => { - const res = await dump({ platform: 'ios', getEnv: () => undefined }); + it('warn-skips with env-missing when both env vars are unset and the 7001 probe finds no driver', async () => { + const httpRequest = jasmine.createSpy('httpRequest').and.callFake(async () => { + throw Object.assign(new Error('econnrefused'), { code: 'ECONNREFUSED' }); + }); + const res = await dump({ platform: 'ios', getEnv: () => undefined, httpRequest }); expect(res).toEqual({ kind: 'unavailable', reason: 'env-missing' }); + expect(httpRequest).toHaveBeenCalledTimes(1); + expect(httpRequest.calls.mostRecent().args[0].port).toBe(7001); + }); + + // When env is unset but the 7001 probe responds with a SpringBoard-only + // hierarchy (no AUT subtree — the user app isn't foregrounded), the dump + // returns kind: 'no-aut-tree'. The dispatch records a final failure with + // the classified reason and returns the probe result directly so the + // caller surfaces the actionable AUT-not-running diagnostic instead of + // the generic env-missing warn-skip. + it('returns no-aut-tree via the 7001 probe when env is unset and the driver reports SpringBoard-only', async () => { + // findAxAutRoot returns null when it walks an axElement tree that + // contains only the SpringBoard application (identifier + // 'com.apple.springboard') with no AUT child — i.e. the user app + // isn't foregrounded. runIosHttpDump then returns kind: 'no-aut-tree' + // and the dispatch's self-hosted-probe branch returns that result + // directly after recording the classified failure (line 1305). + const httpRequest = jasmine.createSpy('httpRequest').and.callFake(async () => ({ + statusCode: 200, + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ + axElement: { + elementType: 1, + identifier: 'com.apple.springboard', + children: [] + } + }) + })); + const res = await dump({ platform: 'ios', getEnv: () => undefined, httpRequest }); + expect(res.kind).toBe('no-aut-tree'); + expect(res.reason).toBe('springboard-only'); + expect(httpRequest).toHaveBeenCalledTimes(1); + expect(httpRequest.calls.mostRecent().args[0].port).toBe(7001); }); it('does not invoke adb on iOS dispatch', async () => {