Skip to content

feat(core): explicit runtime field gates maestro-screenshot self-hosted detection (R2)#2264

Merged
Sriram567 merged 1 commit into
feat/self-hosted-maestro-percy-v1from
feat/maestro-relay-runtime-field
Jun 2, 2026
Merged

feat(core): explicit runtime field gates maestro-screenshot self-hosted detection (R2)#2264
Sriram567 merged 1 commit into
feat/self-hosted-maestro-percy-v1from
feat/maestro-relay-runtime-field

Conversation

@Sriram567
Copy link
Copy Markdown
Contributor

Summary

Stacks on #2261. Promotes the self-hosted-vs-BrowserStack discriminator in the /percy/maestro-screenshot relay handler from an implicit signal (sessionId absence, introduced by #2261) to an explicit one — runtime: "selfhosted" | "browserstack" in the SDK payload. The sessionId-absent fallback stays for backward compatibility with SDKs that predate the runtime field.

Companion SDK change ships the field in a future @percy/maestro-app release (tracked under percy-maestro/docs/plans/2026-06-02-001-feat-explicit-runtime-field-plan.md Unit 1, blocked behind percy-maestro-app#7). The relay works correctly today with older SDKs via the backward-compat fallback.

Why now

cli#2261's relay handler infers the runtime from a field's absence:

let { name, sessionId } = req.body || {};
let selfHosted = !sessionId;

This works today (R7 verified on BS canary builds #22 and #23). The brittleness: if BS ever omits sessionId in a future code path — a new session type, a retry that doesn't re-inject — the self-hosted branch activates accidentally → wrong file-find scope → 404. Moving the declaration to the SDK (where the knowledge originates — the SDK knows whether PERCY_SESSION_ID was injected) eliminates that future-proofing risk.

The field is also semantically clearer for code review: today sessionId in this handler means "BS App Automate session" but in sibling Appium SDKs it means "Appium driver session UUID" (which exists in both runtimes). Adding runtime makes the wire contract self-documenting.

What changes

packages/core/src/api.js:

let { name, sessionId, runtime } = req.body || {};

// Type validation — but unknown values are NOT rejected (fall back to sessionId).
if (runtime !== undefined && typeof runtime !== 'string') {
  throw new ServerError(400, 'Invalid runtime: must be a string');
}

// Prefer explicit declaration; fall back to sessionId-absent for older SDKs.
let selfHosted = (runtime === 'selfhosted') || (!runtime && !sessionId);

// Diagnostic surface for inconsistencies (debug-only, never breaks the request).
if (runtime === 'browserstack' && !sessionId) {
  percy.log.debug('maestro-screenshot: runtime=browserstack but sessionId missing — trusting runtime field');
}
if (runtime === 'selfhosted' && sessionId) {
  percy.log.debug('maestro-screenshot: runtime=selfhosted but sessionId present — trusting runtime field');
}

The selfHosted boolean propagates through the existing handler logic unchanged — only its source-of-truth shifts from implicit to explicit.

Testing

9 new scenarios under a new describe('runtime field gating') block. Covers the 8-way matrix of (runtime ∈ {undefined, "selfhosted", "browserstack", "unknown-value"}) × (sessionId ∈ {present, absent}) plus the type-validation 400. Pre-existing /percy/maestro-screenshot tests are unchanged.

runtime sessionId Expected branch Verification
"selfhosted" absent self-hosted 400 Missing required env: PERCY_MAESTRO_SCREENSHOT_DIR
"selfhosted" present self-hosted (trust runtime) 400 same (with debug log)
"browserstack" present BS 200 success
"browserstack" absent BS (trust runtime) 404 (debug log fires)
"unknown-value" absent falls back to self-hosted 400 same
"unknown-value" present falls back to BS 200 success
missing present BS (back-compat) 200 success
missing absent self-hosted (back-compat) 400 same
non-string reject early 400 Invalid runtime: must be a string

R7 (BS regression) preserved — the explicit runtime: "browserstack" + sessionId path runs byte-identically to today's sessionId-only path.

Origin

  • Brainstorm: percy-maestro/docs/brainstorms/2026-06-02-selfhosted-followup-bundle-requirements.md (R2)
  • Plan: percy-maestro/docs/plans/2026-06-02-001-feat-explicit-runtime-field-plan.md (Unit 2)

Stacking

Post-Deploy Monitoring & Validation

  • What to monitor
    • The two new percy.log.debug lines — they appear when SDK declarations and sessionId presence disagree. Non-zero rate in production would indicate a customer misconfiguration or an upstream BS bug.
    • [BROWSERSTACK_TESTSUITE_PARSE_ERROR] or Screenshot not found rate post-deploy. Should be unchanged.
  • Validation checks
    • BS canary regression on both Android (183.177.55.134) and iOS (103.234.68.130) Maestro v2 hosts. Expected: snapshots produce identically pre/post deploy.
    • Self-hosted Pixel + iPhone simulator quickstart from the example repo. Expected: unchanged behavior (SDK ships no runtime field yet; relay falls back to sessionId-absent path).
  • Expected healthy signals
    • Zero new [percy] Screenshot not found errors on BS or self-hosted builds compared to baseline.
    • Debug log lines remain quiet in production (no SDK is sending the new field yet).
  • Failure signal / rollback trigger
    • Any BS App Automate Maestro build that previously passed reports Snapshot command was not called post-deploy → revert immediately (back-compat path broken).
    • Self-hosted runs that previously worked now 404 → revert (sessionId-absent fallback broken).
  • Validation window & owner

🤖 Generated with Claude Opus 4.7 via Claude Code + Compound Engineering v2.54.0

…lf-hosted detection (R2)

Promotes the self-hosted-vs-BrowserStack discriminator in the relay's
/percy/maestro-screenshot handler from an implicit signal (sessionId
absence) to an explicit one (runtime: "selfhosted" | "browserstack"
in the SDK payload). The sessionId-absent fallback stays for backward
compatibility with SDKs that predate the runtime field.

Why: cli#2261's `let selfHosted = !sessionId;` derives the runtime
from a field's absence. If BS ever omits sessionId in a future code
path (retry, new session type), the self-hosted branch activates by
accident → wrong file-find scope → 404. Moving the declaration to the
SDK (where the knowledge originates — the SDK knows whether
PERCY_SESSION_ID was injected) eliminates that future-proofing risk.

Wire contract (additive, fully back-compat):

  selfHosted = (runtime === "selfhosted") || (!runtime && !sessionId)

Unknown runtime values are NOT rejected — the relay falls back to the
sessionId check, so SDKs experimenting with future values
("maestro-cloud", "saucelabs", etc.) don't break. Two `percy.log.debug`
lines surface bidirectional inconsistencies (runtime + sessionId
disagree) for diagnostic surface, never failing the request.

R7 (BS regression) preserved — when SDK emits runtime: "browserstack"
+ sessionId, the BS branch runs byte-identically to today.

Tests:
- 9 new scenarios for the runtime field gating (8-way matrix of
  runtime × sessionId presence, plus the type-validation 400).
- Existing /percy/maestro-screenshot tests unchanged.

Stacks on cli#2261; companion SDK PR (percy-maestro-app) ships the
runtime field in a future @percy/maestro-app release. Relay works
today with older SDKs via the back-compat fallback.

Origin: percy-maestro/docs/brainstorms/2026-06-02-selfhosted-followup-bundle-requirements.md (R2)
Plan:   percy-maestro/docs/plans/2026-06-02-001-feat-explicit-runtime-field-plan.md (Unit 2)
@Sriram567 Sriram567 requested a review from a team as a code owner June 2, 2026 11:37
@Sriram567 Sriram567 requested review from Shivanshu-07 and prklm10 and removed request for a team June 2, 2026 11:37
Sriram567 added a commit to percy/percy-maestro-app that referenced this pull request Jun 2, 2026
…ayload

Promotes the self-hosted-vs-BrowserStack discriminator from an implicit
relay-side check (sessionId absence) to an explicit SDK-side declaration.
The SDK now emits `runtime: "browserstack" | "selfhosted"` in every
maestro-screenshot POST, derived from PERCY_SESSION_ID presence — the
same discriminator that drove the implicit relay-side check before this
release.

Why: cli#2261's relay handler infers the runtime from a field's absence
(`let selfHosted = !sessionId;`). If BS ever omits sessionId in a future
code path — a new session type, a retry that doesn't re-inject — the
self-hosted branch activates by accident → wrong file-find scope → 404.
Moving the declaration to the SDK (which has perfect knowledge of
whether PERCY_SESSION_ID was injected) eliminates that future-proofing
risk.

Wire-contract: additive, non-breaking.
- CLIs that predate the runtime-field gating (cli ≤ #2261) ignore the
  field; the SDK and relay are decoupled in their roll-out.
- CLIs with the field gating in place (cli#2264+) prefer this field;
  fall back to sessionId-absent for older SDKs.

Companion CLI PR: percy/cli#2264.

Origin: docs/brainstorms/2026-06-02-selfhosted-followup-bundle-requirements.md (R2)
Plan:   docs/plans/2026-06-02-001-feat-explicit-runtime-field-plan.md (Unit 1)
@Sriram567 Sriram567 merged commit ac86447 into feat/self-hosted-maestro-percy-v1 Jun 2, 2026
38 of 40 checks passed
@Sriram567 Sriram567 deleted the feat/maestro-relay-runtime-field branch June 2, 2026 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant