Skip to content

PER-8985 feat: Playwright drop-in baseline seeding — build attributes + exec orchestration#2335

Open
Shivanshu-07 wants to merge 16 commits into
masterfrom
feat/playwright-dropin-baseline
Open

PER-8985 feat: Playwright drop-in baseline seeding — build attributes + exec orchestration#2335
Shivanshu-07 wants to merge 16 commits into
masterfrom
feat/playwright-dropin-baseline

Conversation

@Shivanshu-07

Copy link
Copy Markdown
Contributor

What

CLI-side support for the @percy/playwright toHaveScreenshot drop-in's baseline flow:

  • @percy/clientcreateBuild accepts allow-listed drop-in build sources via PERCY_BUILD_SOURCE (playwright-dropin / playwright-dropin-baseline) plus dropin-baseline-candidate / dropin-baseline-setup attributes (env or explicit options). The API keys baseline auto-approval on these.
  • @percy/cli-exec — generic baseline-provider discovery: any installed SDK package can declare "@percy/cli": { "baselineProvider": "..." } (no framework code enters the CLI, same scaling model as command discovery). On an EMPTY project, percy exec uploads the committed baseline screenshots as an auto-approved build ⬆️ Bump @babel/register from 7.10.4 to 7.10.5 #1 before the user's command runs (the suite never executes for the baseline), then starts the head build. Established projects get a notice pointing at the SDK's explicit playwright:setup-baseline command — the API's no-build-persisted sentinel guarantees no stray builds. Seeding dispatches by project type: web → rendered web snapshots (root DOM + image resource, the percy upload shape); app → comparison ingest (tag + tile, no render).
  • @percy/core — the server-decided build source is kept on percy.build so SDKs can key on it through the healthcheck.

Depends on

Testing

Client + exec suites green incl. 11 new seeding specs; E2E verified against a local percy-api and on staging (empty web/app seed → head diff flow, established-project sentinel, explicit setup command).

🤖 Generated with Claude Code

Shivanshu-07 and others added 3 commits July 13, 2026 18:31
…orchestration

- @percy/client createBuild: allow-listed drop-in build sources via
  PERCY_BUILD_SOURCE, plus dropin-baseline-candidate / dropin-baseline-setup
  attributes (env or explicit options) the API keys baseline auto-approval on.
- @percy/cli-exec: generic baseline-provider discovery (any installed SDK
  package can declare "@percy/cli".baselineProvider). On an empty project,
  percy exec uploads the committed baseline screenshots as an auto-approved
  build #1 before the user's command runs; established projects get a notice
  pointing at the SDK's explicit setup command. Seeding helpers exported for
  SDK-contributed commands.
- @percy/core: expose the server-decided build source on percy.build so SDKs
  can key on it through the healthcheck.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Verified E2E against a local percy-api: a web project's snapshot create
requires a root resource, so raw-PNG comparison tiles are rejected. Seed
uploads now use client.sendSnapshot with the same generated root-DOM +
image-resource shape 'percy upload' uses for web projects — Percy renders
the baseline in the project's own browsers, so it pairs with the head
run's DOM snapshots by (name, browser, width).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Seeding dispatches by project type: web projects seed rendered snapshots
(root DOM + image resource); app projects seed raw comparison uploads
(tag + tile), exactly how App Percy ingests screenshots. Exec seeding now
engages for app tokens too.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Shivanshu-07
Shivanshu-07 requested a review from a team as a code owner July 13, 2026 13:03
Comment thread packages/cli-exec/src/baseline.js Fixed
Comment thread packages/cli-exec/src/baseline.js Fixed
Comment thread packages/cli-exec/src/baseline.js Fixed
Comment thread packages/cli-exec/src/baseline.js Fixed
Comment thread packages/cli-exec/src/baseline.js Fixed
Comment thread packages/cli-exec/src/baseline.js Fixed
Comment thread packages/cli-exec/src/baseline.js Fixed
Comment thread packages/cli-exec/src/baseline.js Fixed
Comment thread packages/cli-exec/src/baseline.js Fixed
Comment thread packages/cli-exec/src/baseline.js Fixed
Shivanshu-07 and others added 12 commits July 14, 2026 10:28
The same env var the SDK override honors — one switch turns off both the
toHaveScreenshot override and exec's baseline provider discovery.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- core: only set percy.build.source when the API returned one — an own
  'source: undefined' key survives in-process but drops out of JSON
  responses, breaking the API-server shape specs.
- cli-exec: cover the exec seeding block (real provider-package fixture)
  and the remaining baseline.js branches for the 100% coverage gate.
- cli-exec: sanitize dirent names / paths at the fs boundary in the
  provider walk (also clears semgrep's path-join-resolve-traversal
  findings on the diff).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Direct sanitizer unit tests, walk fixtures for every discovery skip branch
(empty dir, ESM named-export provider without discoverBaselines), and the
non-web/app token exec branch; istanbul-ignore the two defense-in-depth
dirent guards unreachable via readdir.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The seed build keeps rendering and only auto-approves once it reaches
finished; head snapshots select their baseline as they process, so
starting the head immediately raced the approval and a lost race made
the whole first run report as new instead of diffing. Poll the seed
build up to the pipeline latency budget (10m) with a 30s heartbeat,
degrade to a warning instead of blocking on timeout or status errors.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…pin-baseline

# Conflicts:
#	packages/client/src/client.js
#	packages/client/test/client.test.js
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Master-inherited coverage gap from the metadata-SSRF change — the catch
path for a colon-containing host that fails URL parsing was never
exercised, holding @percy/core under the 100% gate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every caller (matchMetadataHost) null-guards before calling, so the
branch cannot be exercised by tests; it held @percy/core under the
100% gate on master too.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The seed upload omitted browserName, so seed and head comparisons
landed on different canonical tag rows and screenshot pairing missed —
the first app run diffed nothing. One tag shape, defined by the SDK.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- discovery walk now mirrors @percy/cli's findModulePackages exactly:
  stop at the nearest node_modules, never cross the home directory, and
  degrade to [] on any filesystem error — a throw here previously
  escaped to exec's outer catch and disabled Percy for the whole run
- provider modules are confined to their declaring package (a
  baselineProvider path escaping the package root is skipped)
- only ever seed build #1: an API that predates the candidate attribute
  hands back a normal build, which previously seeded stray baseline
  builds into established projects on every run
- falsy provider baseline entries are filtered instead of silently
  terminating an upload worker

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment thread packages/cli-exec/src/baseline.js Fixed
Comment thread packages/cli-exec/src/baseline.js Fixed
… idiom)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Shivanshu-07

Copy link
Copy Markdown
Contributor Author

Claude Code PR Review

PR: #2335Head: 0a38a90Reviewers: fallback inline checklist (deep-review agent)

Summary

CLI-side drop-in baseline seeding: generic baselineProvider package discovery, seed-build creation/upload (percy exec seeds an empty project's build #1 from committed screenshots, then waits for it to finish before the head build starts), new allow-listed build source + candidate/setup attributes in @percy/client, plus two @percy/core coverage fixes inherited from master.

Review Table

Priority Category Check Status Notes
High Security No hardcoded secrets or credentials Pass
High Security Authentication/authorization checks present N/A token handled by @percy/client env
High Security Input validation and sanitization Pass sources allow-listed (DROPIN_BUILD_SOURCES); env flags strict === 'true'
High Security No IDOR — resource ownership validated N/A
High Security No SQL injection (parameterized queries) N/A
High Correctness Logic is correct, handles edge cases Pass 2 High findings fixed in 0a38a90 lineage: discovery walk now degrades to [] on fs errors (a throw previously escaped to exec's catch and disabled Percy for the run); seeding is bounded to build #1 so a pre-candidate API can no longer accrete stray baseline builds on established projects
High Correctness Error handling is explicit, no swallowed exceptions Pass never-throw seeding posture is by design; walk failures logged at debug
High Correctness No race conditions or concurrency issues Pass seed-wait closes the head-baseline race; parallel shards skip seeding
Medium Testing New code has corresponding tests Pass walk-error, containment-escape, homedir-bound, non-first-build, falsy-entry cases added
Medium Testing Error paths and edge cases tested Pass
Medium Testing Existing tests still pass (no regressions) Pass all package suites + 100% coverage gates green
Medium Performance No N+1 queries or unbounded data fetching Pass walk now stops at the nearest node_modules (was: every ancestor to /)
Medium Performance Long-running tasks use background jobs N/A
Medium Quality Follows existing codebase patterns Pass walk mirrors @percy/cli findModulePackages semantics exactly
Medium Quality Changes are focused (single concern) Pass the two @percy/core fixes unblock master's own red Test workflow
Low Quality Meaningful names, no dead code Pass
Low Quality Comments explain why, not what Pass
Low Quality No unnecessary dependencies added Pass

Findings

All review findings were fixed on this PR before this report:

  • High packages/cli-exec/src/baseline.js: the node_modules walk could throw (EACCES / node_modules-as-file) out of findBaselineProvider into exec's outer catch — silently disabling Percy for the whole run. Walk now try/caught → [], matching the command-discovery pattern.
  • High packages/cli-exec/src/baseline.js: against an API that predates the candidate attribute, every run would seed a stray "baseline" build into established projects. Seeding now requires build-number === 1.
  • Medium waitForSeedBuild uses client.getBuild — verified working with standard write project tokens against percy-api (staging E2E), so the read-token doc caveat does not bite; kept as-is.
  • Low: walk bounded at the home directory and stops at the nearest node_modules (narrower import surface); provider modules confined to their declaring package; falsy provider entries filtered instead of terminating an upload worker; containment paths wrapped in the sanctioned sanitizer idiom (semgrep).

Verdict: PASS

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.

2 participants