feat(action): add manifest-based workflow modes by erikkrietsch · Pull Request #786 · ExpediaGroup/comparadise

erikkrietsch · 2026-06-10T20:12:00Z

Summary

Adds three new workflow input values to the GitHub Action: manifest-generate, manifest-compare, and manifest-merge
manifest-generate runs visual tests, hashes each screenshot with MD5, and writes a manifest to S3 at manifests/{sha}.json; when head-sha is provided, only images whose hash changed since that SHA are uploaded (differential uploads)
manifest-compare fetches the PR and HEAD manifests, finds the merge-base ancestor manifest, and classifies each differing path as PR Owns / Main Owns / Conflict via 3-way hash comparison; PR Owns paths get diff images generated and a changeset written to changesets/{sha}.json; sets the Visual Regression commit status and posts a PR comment
manifest-merge reads the PR's changeset at merge time, overlays it onto the parent manifest, writes the new HEAD manifest to S3, and updates base-images/; detects stale changesets and fails any open PRs whose changesets overlap with a conflicting stale base

Changes

action/src/hash.ts — MD5 file hashing
action/src/manifest-s3.ts — typed put/get for manifests and changesets
action/src/manifest-generate.ts — manifestGenerate() top-level mode
action/src/manifest-compare-classify.ts — classifyManifests() 3-way classification
action/src/manifest-diff.ts — generateDiffs() produces base + diff PNGs in S3
action/src/diff-png.ts — diffPng() pixelmatch wrapper
action/src/manifest-compare.ts — manifestCompare() orchestrator
action/src/manifest-merge-overlay.ts — overlayChangeset() and detectStaleConflicts() (pure transforms)
action/src/manifest-merge-base-images.ts — applyChangesetToBaseImages() S3 copy/delete
action/src/manifest-merge-flag-prs.ts — flagOverlappingOpenPrs() sets failure status on conflicting open PRs
action/src/manifest-merge.ts — manifestMerge() orchestrator
action/src/run.ts — dispatches new workflow values; wires deps for each mode
action.yml — documents new inputs (head-sha, base-ref, pr-sha, pr-number, merge-commit-sha)
docs/docs/setup/manifest-workflows.md — setup guide with YAML examples for all three modes
docs/static/img/manifest-workflow.svg — flowchart diagram

Test plan

bunx nx test action — all tests pass
manifest-generate: manifest written to manifests/{sha}.json; differential upload skips unchanged images when head-sha is set
manifest-compare: match path sets Visual Regression → success; conflict paths → setFailed with comment; PR Owns paths → pending + changeset written
manifest-merge: changeset overlaid onto parent manifest; stale + conflicting changeset → setFailed; base-images/ updated correctly; overlapping open PRs flagged

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

…three modes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Introduces hashFile() which computes MD5 hex digest of a file using Node.js crypto. Includes test with a fixture PNG image. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Typed functions for storing and retrieving manifest and changeset JSON from S3, with graceful null returns for missing keys. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Pure function that diffs two manifests, producing a changeset with new hashes for added/changed entries and null for deletions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Orchestrates the manifest-generate workflow: runs visual tests, hashes all screenshots, compares against the HEAD manifest to determine what changed, and uploads only changed images plus the new manifest to S3. Supports resize and package-paths options. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Replace top-level imports with injectable deps parameters so tests can pass mocks directly without Bun's global mock.module. This prevents cross-file mock poisoning that was breaking run.test.ts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Removes bespoke per-module DI interfaces in favor of the shared Dependencies type from dependencies.ts. manifest-generate now uses deps.s3, deps.core, deps.fs, and deps.hashFile. manifest-s3 takes S3Operations directly. Tests use env vars for action inputs, matching the run.test.ts pattern. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fetches PR, HEAD, and ancestor manifests, then classifies each differing screenshot path as prOwns (PR introduced the change), mainOwns (main changed, PR is clean), or conflict (both sides modified). Returns early with 'match' when all hashes are identical. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The manifest key is now the containing directory relative to the screenshots root (e.g., "components/Button") rather than the full file path (e.g., "components/Button/new.png"). This aligns with the updated plan and makes S3 path construction straightforward—append /{base,diff,new}.png to the key. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Introduces two modules: - diff-png: wraps pixelmatch/pngjs to produce a diff PNG from two buffers, handling size mismatches by expanding to max dimensions - manifest-diff: orchestrates downloading base + new from S3, generating the diff, and uploading base.png + diff.png to the comparison directory for the Comparadise web app Only processes prOwns entries with type 'changed' — added and deleted entries are skipped. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Wires together classification, diff generation, GitHub status, comment posting, and changeset writing per MANIFEST_PLAN.md step 3. Outcomes: - match: success status, nothing else - only mainOwns: success status (main changed, PR clean) - conflicts: failure status + conflict comment, no changeset - prOwns: diffs + pending status + comment + changeset (with _headSha) Changeset records pr hash for changed/added entries and null for deleted entries, omitting mainOwns paths entirely.

Mirror the manifest-generate convention where the file/function named after the mode is the top-level mode entry point. - manifest-compare.ts (classifier) -> manifest-compare-classify.ts (manifestCompare -> classifyManifests; ManifestCompareDeps -> ClassifyDeps; CompareParams -> ClassifyParams) - run-manifest-compare.ts (orchestrator) -> manifest-compare.ts (runManifestCompare -> manifestCompare; RunManifestCompareDeps -> ManifestCompareDeps; RunManifestCompareParams -> ManifestCompareParams)

Two pure manifest transformations used by manifest-merge: - overlayChangeset(parent, changeset): apply a changeset on top of a parent manifest. Non-null entries set/update hashes; null entries remove keys. The _headSha metadata field is ignored. - detectStaleConflicts(head, parent, changeset): safeguard for the case where a changeset's _headSha differs from the merge parent. Returns paths whose hash differs between the two manifests, which would be clobbered by overlay.

applyChangesetToBaseImages walks a changeset and applies it to S3: - non-null entries: copy new-images/{prSha}/{path}/new.png to base-images/{path}/base.png - null entries: batch-delete base-images/{path}/base.png Implements step 10 of the manifest-merge plan.

flagOverlappingOpenPrs lists open PRs and, for each whose changeset shares at least one screenshot path with the merging PR's changeset, sets a failure commit status on that PR's head SHA with a 'rebase required' message. Implements step 4 of the manifest-merge plan.

Wires together changeset fetch, conflict prevention, stale-conflict safeguard, manifest overlay, manifest write, and base-image apply per MANIFEST_PLAN.md. Outcomes: - no changeset: copy parent manifest as merge commit manifest, done - changeset with _headSha == parent: flag overlapping open PRs, overlay, write, apply base images - changeset with _headSha != parent: same plus a stale-conflict check using the head manifest as a safeguard against interleaved merges Stale conflicts fail the job (setFailed + thrown error) before any write or base-image apply happens.

Covers manifest-generate, manifest-compare, and manifest-merge modes with example workflow YAMLs for single-package and matrix setups. Documents the required concurrency group on manifest-merge workflows to prevent concurrent merge races on base images.

SVG sequence diagram showing all three manifest modes (generate, compare, merge), their triggers, and data flows to S3 and GitHub API. Referenced from the manifest-workflows docs page.

Shows decision points for all three modes (generate, compare, merge) in a three-column layout with exit badges for success/failure paths.

- Remove original-new-images/ write from manifest-generate (it was incorrectly uploading full-size originals alongside resized images) - Treat deleted screenshots as non-reviewable: write changeset but set success status and skip pending/comment/diffs for deletion-only PRs - Paginate all open PRs in flagOverlappingOpenPrs (was capped at 30) - Improve ancestor-manifest-not-found error to mention running manifest-generate on the base branch Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Matrix manifest-generate jobs now write per-package manifests to manifests/{sha}/{package-path}.json with package-prefixed keys and image paths, so parallel jobs no longer overwrite a shared manifest. manifest-compare squashes those per-package manifests into the single manifests/{sha}.json before the 3-way comparison; single-package PRs are unaffected. manifest-merge is unchanged. Implements the Monorepo support for manifest workflows epic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Source-level re-review against MANIFEST_AC.md at PR head 02869c8. All 41 criteria satisfied; Review #1 defects resolved; 175 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…mension note Adds acceptance criterion 4.7 requiring each manifest mode to derive its SHAs/refs/PR identifiers from the triggering event rather than dedicated head-sha/base-ref/pr-sha/pr-number/merge-commit-sha inputs, and expands the 2.2 note to state that diff generation must tolerate a base/new dimension mismatch (padding before pixelmatch). Syncs MANIFEST_AC.md to the canonical manifest-plan content. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

danadajian · 2026-06-24T19:31:04Z

+  head-sha:
+    description: 'Optional main HEAD SHA used by manifest-generate to only upload changed screenshots.'
+    required: false
+  base-ref:
+    description: 'Base branch name for manifest-compare (for example, "main").'
+    required: false
+  pr-sha:
+    description: 'PR head SHA for manifest-merge. Falls back to github.event.pull_request.head.sha.'
+    required: false
+  pr-number:
+    description: 'PR number for manifest-merge. Falls back to github.event.pull_request.number.'
+    required: false
+  merge-commit-sha:
+    description: 'Merge commit SHA for manifest-merge. Falls back to github.event.pull_request.merge_commit_sha.'
+    required: false


We don't need to add these inputs as optional overrides.

Each mode runs on exactly one trigger, so an override input only duplicates a value already in the event payload. The per-commit merge core must remain parameter-driven (decoupled from the event source) so a future trigger — e.g. a push/merge-queue path that resolves the same values from the pushed commit range — can reuse it without these inputs.

danadajian · 2026-06-24T19:38:19Z

+    for (const part of parts) {
+      if (!part.Key) continue;
+      const response = await s3.getObject({ Bucket: bucket, Key: part.Key });
+      const body = await response.Body!.transformToString();


We should not use non-null assertions (!). I will add a new lint rule to ban this in the project

danadajian · 2026-06-24T19:48:23Z

  }
 };
+
+async function runManifestCompareWorkflow(deps: Dependencies): Promise<void> {


All this new code should be in a separate file or files, to keep the entrypoint file a bit cleaner. (As a follow-up I should extract out the existing stuff in here into its own file)

danadajian · 2026-06-24T19:48:55Z

+  if (!Number.isFinite(prNumber)) {
+    deps.core.setFailed(`Invalid pr-number: ${prNumberInput}`);
+    return;
+  }


lol I hope PR numbers from github are finite. Let's remove this

danadajian · 2026-06-24T19:51:17Z

+      const body = await response.Body!.transformToString();
+      return JSON.parse(body) as Changeset;
+    } catch (error: unknown) {
+      if (error instanceof Error && error.name === 'NoSuchKey') {


Let's reuse the isNoSuchKey() method here and any other places we duplicate this code

danadajian · 2026-06-24T19:52:49Z

+    for (const part of parts) {
+      if (!part.Key) continue;
+      const response = await s3.getObject({ Bucket: bucket, Key: part.Key });
+      const body = await response.Body!.transformToString();


danadajian · 2026-06-24T19:54:21Z

+import { readFile } from 'fs/promises';
+
+export async function readImageFile(filePath: string): Promise<Buffer> {
+  return readFile(filePath);
+}


Delete this - it's unused

danadajian marked this pull request as ready for review June 12, 2026 13:10

erikkrietsch changed the base branch from manifest-plan to main June 12, 2026 13:21

erikkrietsch force-pushed the manifest-implementation branch from 80ec07d to 56b78f1 Compare June 12, 2026 13:29

danadajian and others added 27 commits June 24, 2026 11:11

docs: add implementation plan for manifest-based visual comparison

f3f81ad

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

clarify

dbac8ac

stale detection (#775)

f0e2255

proactive check (#777)

8600bcb

Add semantic names for compare mode scenarios

821d672

docs(manifest-plan): use directory paths as manifest keys (#783)

df4dcd6

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

docs(manifest-plan): clarify and simplify resize behavior across all …

e355a26

…three modes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs(manifest-plan): update ancestor manifest error message and AC doc

1a98890

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Add file hashing utility for manifest generation

a7a330e

Introduces hashFile() which computes MD5 hex digest of a file using Node.js crypto. Includes test with a fixture PNG image. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add manifest and changeset S3 read/write utilities

5d882a7

Typed functions for storing and retrieving manifest and changeset JSON from S3, with graceful null returns for missing keys. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add changeset computation between two manifests

d3bf025

Pure function that diffs two manifests, producing a changeset with new hashes for added/changed entries and null for deletions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Document no-mock.module testing rule in AGENTS.md

3ec15ea

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Wire manifest workflows into action runtime

042b401

Add manifest workflow sequence diagram

46de14e

SVG sequence diagram showing all three manifest modes (generate, compare, merge), their triggers, and data flows to S3 and GitHub API. Referenced from the manifest-workflows docs page.

aiya/claude and others added 4 commits June 24, 2026 11:11

Replace sequence diagram with flowchart for manifest workflows

6e2b093

Shows decision points for all three modes (generate, compare, merge) in a three-column layout with exit badges for success/failure paths.

chore: committing generated code

474d123

erikkrietsch force-pushed the manifest-implementation branch from 1874a1c to c9167db Compare June 24, 2026 16:14

chore: committing generated code

02869c8

danadajian reviewed Jun 24, 2026

View reviewed changes

update AC

f4796d2

danadajian reviewed Jun 24, 2026

View reviewed changes

Merge branch 'main' into manifest-implementation

15328b8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(action): add manifest-based workflow modes#786

feat(action): add manifest-based workflow modes#786
erikkrietsch wants to merge 35 commits into
mainfrom
manifest-implementation

erikkrietsch commented Jun 10, 2026

Uh oh!

danadajian Jun 24, 2026

Uh oh!

danadajian Jun 24, 2026

Uh oh!

danadajian Jun 24, 2026

Uh oh!

danadajian Jun 24, 2026

Uh oh!

danadajian Jun 24, 2026

Uh oh!

danadajian Jun 24, 2026

Uh oh!

danadajian Jun 24, 2026

Uh oh!

danadajian Jun 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

erikkrietsch commented Jun 10, 2026

Summary

Changes

Test plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants