feat(core): self-hosted (non-BrowserStack) Maestro relay support#2248
Closed
Sriram567 wants to merge 4 commits into
Closed
feat(core): self-hosted (non-BrowserStack) Maestro relay support#2248Sriram567 wants to merge 4 commits into
Sriram567 wants to merge 4 commits into
Conversation
…ESTRO_SCREENSHOT_DIR scope
`/percy/maestro-screenshot` now supports non-BrowserStack ("self-hosted")
Maestro runs by making `sessionId` optional. When the request omits it
(the BrowserStack host's exclusive marker), the relay enters self-hosted
mode and resolves the file-find scope root from a new required env var:
- PERCY_MAESTRO_SCREENSHOT_DIR — process.env only, never request body.
Must be an absolute, existing directory (typically the customer's
`maestro test --test-output-dir <DIR>` path). Missing/invalid → 400
with actionable guidance, emitted before realpath so it isn't masked
as a 404.
Self-hosted file-find uses a recursive `<root>/**/<name>.png` glob and
goes through the same realpath + trailing-separator prefix check as the
BS path, just rooted at the env-supplied dir. The security boundary is
relocated, not removed:
- filePath is rejected outright self-hosted (SDK never emits it; an
absolute filePath against a caller-influenceable root would re-open
arbitrary in-root reads).
- SAFE_ID on `name` stays load-bearing for the recursive glob.
- Trailing-separator guard preserved (prevents /x/.maestro vs
/x/.maestro-secrets bypass at the relocated root).
BS path (sessionId present) is byte-identical — the existing
`/tmp/{sessionId}{_test_suite}` glob, walker fallback, and realpath
check all run exactly as before. R7 from the plan.
Tests: BS-path tests pass unchanged. New self-hosted describe locks the
new branches: happy path (recursive find, payload omits sessionId), env
var validation (unset/relative/non-existent/file-not-dir → 400),
filePath rejected, file missing → 404, name traversal rejected,
coordinate region pass-through.
Plan: docs/plans/2026-05-27-001-feat-self-hosted-maestro-percy-plan.md
(Unit 1 of 4; iOS port resolver + SDK + docs follow).
…o/` default Maestro's default output directory is `.maestro/` (dot-prefixed). When a self-hosted customer runs `maestro test` without `--test-output-dir`, screenshots land under `<cwd>/.maestro/...`. fast-glob's default is `dot: false`, so the recursive `<root>/**/<name>.png` glob silently skipped dot-prefixed segments and 404'd the file. CI surfaced this on two self-hosted tests (happy path + coordinate regions pass-through), whose fixtures correctly mirror Maestro's `.maestro/run-x/screenshots/` real-world layout. `dot: true` is applied only on the self-hosted glob; the BS glob is unchanged (BS layouts have no dot-prefixed segments — byte-identical R7).
…of + warn-skip
Adds non-BrowserStack support to the iOS element-region resolver. Before
this change, `dump({ platform: 'ios' })` required BOTH
`PERCY_IOS_DEVICE_UDID` AND `PERCY_IOS_DRIVER_HOST_PORT` to be set
(host-injected by realmobile) — if either was absent (the normal
self-hosted state) the resolver bailed with `unavailable/env-missing`
and element regions silently dropped.
The dispatch now splits into two branches by `PERCY_IOS_DRIVER_HOST_PORT`
presence:
EXPLICIT (port set) — BS and customer-supplied-port self-hosted both
take this path. Existing HTTP-primary → CLI-fallback cascade runs
unchanged when UDID is also set. New: when UDID is absent (a
self-hosted-with-explicit-port scenario), HTTP is the only path; on
HTTP failure the resolver warn-skips with the new `self-hosted-no-udid`
reason rather than running maestro CLI without a target. `parseIos
DriverHostPort` is relaxed from the BS-specific 11100-11110 range to
any valid TCP port (1-65535) — BS values remain a strict subset.
IMPLICIT (port unset) — self-hosted discovery cascade, all HTTP-based:
1. Cache hit (iosPortCache, per-Percy-instance, mirroring grpcClientCache).
2. Probe 127.0.0.1:7001 — source-verified deterministic single-
simulator port on Maestro ≤2.4.0 (cli-2.4.0 TestCommand.kt#selectPort).
3. `lsof -nP -iTCP -sTCP:LISTEN | grep maestro-driver-ios…xctrunner` —
exactly-one-match guard (zero or multiple → warn-skip, no guess).
4. Warn-skip with actionable log (`self-hosted-no-driver` reason).
"Probe" reuses runIosHttpDump as both liveness check AND dump — a
`hierarchy` or `no-aut-tree` result confirms the port is a valid
Maestro driver (the existing axElement-root schema check rejects wrong
services); `dump-error`/`connection-fail` advances to the next
candidate (no drift-bit flip in cascade — drift is reserved for the
explicit-port BS path).
No cold-start `maestro hierarchy` tier — cut per plan after the spike
verified 7001 is deterministic on current GA Maestro and lsof covers
the future ephemeral-port case (Maestro 2.6+ `ServerSocket(0)`).
BS path R7: the EXPLICIT branch with both UDID + valid PORT runs
byte-identical to today. Existing iOS-HTTP, schema-drift, kill-switch,
and healthcheck-drift tests pass unchanged. Three previously-expected
`env-missing` tests are updated to the new branching:
- PORT set + UDID unset → enters EXPLICIT, HTTP fails → `self-hosted-no-udid`.
- PORT unset (regardless of UDID) → enters IMPLICIT, cascade →
`self-hosted-no-driver` on no resolution.
New describe `iOS self-hosted port cascade` (7 tests) covers:
probe-7001 hit + caching, lsof single-match discovery, lsof zero/multi-
match warn-skip, explicit out-of-legacy-range port (e.g., 6001 for real
device) bypassing the cascade, wrong-service response (no axElement)
falling through without caching, and cache-hit reuse on subsequent
snapshots.
Files:
- packages/core/src/maestro-hierarchy.js: new constants
(IOS_SELF_HOSTED_PROBE_PORT), three new helpers (execLsofDefault,
lsofXctrunnerPort, resolveSelfHostedIosPort), iOS dispatch refactor,
dump() signature extended with execLsof + iosPortCache injectables.
- packages/core/src/api.js: thread `iosPortCache: percy.iosPortCache`
through the maestroDump call (one line; mirrors grpcClientCache).
- packages/core/src/percy.js: initialize `this.iosPortCache = { port: null }`
next to grpcClientCache (per-Percy-instance scope, D9 of the maestro
4-PR plan).
- packages/core/test/unit/maestro-hierarchy.test.js: 3 existing tests
updated for the new branching, 7 new cascade tests added.
Plan: docs/plans/2026-05-27-001-feat-self-hosted-maestro-percy-plan.md
(Unit 2 of 4; Unit 1 already in ef66ccc/8ac60b87; companion SDK + docs
in percy/percy-maestro-app#7).
…hosted-no-driver Caught by CI on PR #2248: the cross-platform parity test asserted the old `env-missing` reason tag, which Unit 2 replaced with `self-hosted-no-driver` (PERCY_IOS_DRIVER_HOST_PORT absent enters the new self-hosted IMPLICIT cascade and warn-skips with the new reason when no driver is found). Test now injects fakes for httpRequest + execLsof so the cascade deterministically exhausts and returns `self-hosted-no-driver`. The envelope-shape parity invariant (kind = 'unavailable') is preserved. Fix-up to commit ccb9bc0 (Unit 2).
Sriram567
added a commit
to percy/percy-maestro-app
that referenced
this pull request
May 28, 2026
…vice run
Marks status: validated-2026-05-28. The runbook stub anticipated this
backfill ("Backfill on first real run") — these are the verified
commands, build IDs, and the two gotchas discovered during the first
real-device acceptance run on a Pixel 10 / Android 16 / Maestro 2.4.0
host.
Validated:
- /percy/maestro-screenshot relay resolves PERCY_MAESTRO_SCREENSHOT_DIR-
scoped screenshots with no sessionId (Unit 1).
- SDK uploads successfully self-hosted with PERCY_SESSION_ID omitted from
the payload (Unit 3).
- Coordinate regions are forwarded SDK → relay → Percy comparison engine.
Baseline build #50215463 (7×8=56) approved; comparison build #50215516
(9×9=81) reported `02_CalcResult` as Unchanged — masked area's diff
ignored, canonical regions-working demonstration.
Two gotchas captured for the next person validating self-hosted on a
fresh machine:
1. Maestro hangs silently on the driver-APK install step without
JAVA_TOOL_OPTIONS="-Djava.net.preferIPv4Stack=true". The hang is in
dadb trying to talk to adb over IPv6 loopback. Reproduces with both
Maestro 2.6.0 and 2.4.0, with Play Protect disabled, on Pixel 10 +
Android 16 + macOS 15.5 (Apple Silicon). Independent of Percy —
`maestro test` alone hangs identically without the env var.
2. Maestro's GraalJS runScript context does NOT inherit the parent
process's environment. PERCY_SERVER_ADDRESS exported by percy
app:exec is invisible to the SDK. Customer must pass
`-e PERCY_SERVER=http://localhost:5338` as a Maestro -e flag on every
`maestro test` invocation. This is the one extra config item required
for self-hosted vs. BrowserStack.
iOS validation pending (no iOS device at the time of this run); the
runbook captures the runtime-verify items for that next pass.
Plan: docs/plans/2026-05-27-001-feat-self-hosted-maestro-percy-plan.md
Companion CLI PR: percy/cli#2248
This was referenced May 29, 2026
Contributor
Author
|
Consolidated into #2261 (single self-hosted Maestro+Percy V1 PR — all 6 commits, same content). Closing this stack-base in favor of the unified review surface. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds non-BrowserStack ("self-hosted") support to the Maestro screenshot relay. Customers running
percy app:exec -- maestro test ...on their own devices, CI, or device farms can now produce Percy builds without BS host injection.Architecture, in one sentence:
sessionIdabsent on/percy/maestro-screenshotis the self-hosted detection signal (it's a BrowserStack host-injected marker that lives nowhere else in the CLI); when absent, the relay resolves a file-find scope root fromPERCY_MAESTRO_SCREENSHOT_DIR(process env, required, absolute existing directory — typically the customer'smaestro test --test-output-dir <DIR>path), globs<root>/**/<name>.png, and applies the same realpath + trailing-separator prefix check that protects the BS path today. The BrowserStack path is byte-identical — the existing/tmp/{sessionId}{_test_suite}glob, manual-walker fallback, and security check all run exactly as before for any request withsessionId.What changed
packages/core/src/api.js—sessionIdis optional;selfHosted = !sessionIdderives the mode. New scope-root resolution block validatesPERCY_MAESTRO_SCREENSHOT_DIR(absolute + existing dir, 400 with actionable message otherwise, emitted before realpath so it isn't masked as a 404). File-find branches onselfHostedfor the glob pattern; the realpath/prefix check is now rooted at the resolvedscopeRoot.filePathis rejected in self-hosted mode (the SDK never emits it; honoring it against a caller-influenceable root would re-open arbitrary in-root reads).packages/core/test/api.test.js— newself-hosted (sessionId absent)describe locks the new branches: happy path (recursive glob find + payload omits sessionId + PNG-fill works), env-var validation (unset/relative/non-existent/file-not-dir → 400),filePathrejected, file-missing → 404,name-traversal rejected, coordinate region pass-through. The existing "rejects missing sessionId" test is updated to reflect the new behavior (missing sessionId + missing env var → 400 with the new actionable message).Security invariants (relocated, not removed)
SAFE_IDonname(^[a-zA-Z0-9_-]+$) is load-bearing for the recursive glob — separators and traversal characters rejected./x/.maestrovs/x/.maestro-secrets).PERCY_MAESTRO_SCREENSHOT_DIRread fromprocess.envonly, never the request body — root cannot be moved per-call.filePathrejected self-hosted.What this PR does not include yet
This is the first of two cli commits for the feature; the iOS element-region resolver work (auto-discover port
7001→ BS-proven HTTP/viewHierarchyattach +lsofself-discovery fallback + graceful warn-skip, no cold-start tier) lands as a follow-up commit on this same branch. Marking the PR draft until it's in. SDK changes (@percy/maestro-appserver default + session-id gate relaxation) and docs land in a companion PR againstpercy/percy-maestro— also pending.Testing
/percy/maestro-screenshotdescribe) stays green — locks R7 (no BrowserStack regression).1536376returns 404 fromchromium-browser-snapshots, breaking unrelatedServer+ browser tests). Buildkite CI runs in a clean environment.Post-Deploy Monitoring & Validation
bs-nixpkgs.service.name=percy-apiwithbuild.client=maestro(host-side BS sessions) —snapshot_countshould stay at baseline;"Snapshot command was not called"should not appear.400from/percy/maestro-screenshoton BS sessions.bs-nixpkgsderivation bump. The change is gated so the BS path is byte-identical — any regression points at this PR.bs-nixpkgscli bump that includes this commit.percy app:exec -- maestro testoff-BS). Acceptance is the real-device gate in the plan (Android + iOS device builds with coordinate + element regions).Plan reference
Plan:
percy-maestro/docs/plans/2026-05-27-001-feat-self-hosted-maestro-percy-plan.md(cross-repo plan, not in this repo — Unit 1 of 4). Jira: PER-8599.🤖 Generated with Claude Opus 4.7 (200K context, extended thinking) via Claude Code