Skip to content

fix(ci): preserve trusted visual run identity - #5587

Merged
cixzhang merged 1 commit into
mainfrom
fix/visual-recapture-identity
Aug 27, 2026
Merged

fix(ci): preserve trusted visual run identity#5587
cixzhang merged 1 commit into
mainfrom
fix/visual-recapture-identity

Conversation

@cixzhang

@cixzhang cixzhang commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Why

Accepted visual PRs such as #5214 and #5558 cannot merge because immutable visual evidence rejects its own trusted recapture.

The reproduced publisher logs show:

RUN_ID: 33039803616
RUN_ATTEMPT: 2
Error: visual evidence rejected: trusted capture identity mismatch

RUN_ID: 33039815281
RUN_ATTEMPT: 2
Error: visual evidence rejected: trusted capture identity mismatch

pr-comment.yml attempted to replace GITHUB_RUN_ID and GITHUB_RUN_ATTEMPT with those triggering CI identities. GitHub's default GITHUB_* variables are immutable, so gate.mjs instead recorded the PR Comment workflow identities in trusted-capture/manifest.json; publish-pr-report.mjs then correctly rejected the mismatch.

The same required gate had a second live permission blocker on #5162:

DELETE /repos/facebook/astryx/issues/5162/labels/visual-approved - 403
Resource not accessible by integration

Both affected jobs mutate a label on a pull request, so they need job-scoped pull-requests: write; issues: write alone did not authorize that API call.

What

  • Pass the triggering CI identity through dedicated ASTRYX_VISUAL_RUN_ID / ASTRYX_VISUAL_RUN_ATTEMPT variables.
  • Prefer those variables only when present; normal GitHub and local capture behavior is unchanged.
  • Grant PR mutation permission only to the two acceptance jobs that add/remove visual-approved.
  • Cover default, trusted override, missing, invalid, workflow wiring, and permission behavior.

Tests

  • pnpm exec vitest run .github/scripts/visual-gate — 13 files, 117 tests
  • actionlint on the changed workflows (ignoring the existing custom-runner label warning)
  • targeted ESLint and Prettier checks
  • repository pre-commit checks

@vercel

vercel Bot commented Aug 27, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
astryx Ready Ready Preview Aug 27, 2026 6:08am

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Aug 27, 2026
@cixzhang
cixzhang force-pushed the fix/visual-recapture-identity branch 2 times, most recently from ad92471 to d852ab0 Compare August 27, 2026 05:45
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

PR Analysis Report

📚 Storybook Preview

View Storybook for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

🧪 Sandbox Preview

View Sandbox for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

No new or modified components detected.

Bundle Size Summary

No component packages changed.

Accessibility Audit

Status: No accessibility violations detected.


Generated by PR Enrichment workflow | Storybook | Sandbox | View full report

@cixzhang cixzhang left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the run-identity fix reproduces cleanly. One blocker remains: the current head no longer grants pull-requests: write to initialize or accept, so #5162 still gets a 403 deleting visual-approved and required acceptance cannot finish (visual-acceptance.yml:28,210). Please restore the job-scoped permissions and their regression assertion before merge.

[Reviewed by Robohands]

@cixzhang
cixzhang force-pushed the fix/visual-recapture-identity branch from d852ab0 to 1b1e3d8 Compare August 27, 2026 06:05
github-actions Bot added a commit that referenced this pull request Aug 27, 2026

@cixzhang cixzhang left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks—the blocker is fixed at 1b1e3d89808: both mutation jobs have scoped PR write access, authorization stays read-only, and the identity repro passes. Good to merge.

[Reviewed by Robohands]

@cixzhang
cixzhang merged commit c0c7f8b into main Aug 27, 2026
26 of 27 checks passed
@cixzhang
cixzhang deleted the fix/visual-recapture-identity branch August 27, 2026 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant