fix: run E2E tests after i18n completes on release PRs #8091
+107
−8
Conversation
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
- Add workflow_call trigger to ci-tests-e2e.yaml - Add skip condition for version-bump PRs on pull_request trigger - Create ci-tests-e2e-release.yaml that triggers after i18n completes - Calls original E2E workflow via workflow_call to avoid job duplication Fixes the issue where locale commits would cancel in-progress E2E tests.
dosubot
bot
added
the
size:L
Contributor
📝 WalkthroughWalkthroughThe changes introduce a new GitHub Actions workflow for orchestrating E2E tests on release PRs and modify the existing E2E test workflow to support reusable invocation via workflow_call with configurable inputs for Git ref and PR number. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub Actions
participant I18N as i18n Workflow
participant Release as ci-tests-e2e-release
participant E2E as ci-tests-e2e (Reusable)
GitHub->>I18N: Trigger workflow
I18N->>GitHub: Workflow completes
GitHub->>Release: Trigger on workflow_run (version-bump branch)
Release->>Release: eligibility: Check branch pattern & SHA match
rect rgba(100, 150, 200, 0.5)
Release->>GitHub: get-pr-number: Query PRs by head SHA
GitHub-->>Release: Return matching PR number
end
Release->>E2E: run-e2e-tests: Call workflow with<br/>ref & pr_number inputs
rect rgba(150, 100, 200, 0.5)
E2E->>E2E: Checkout at provided ref
E2E->>E2E: Run E2E tests
E2E->>GitHub: Comment on PR (using pr_number)
end
Possibly related PRs
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎭 Playwright Tests:
|
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/16/2026, 02:18:13 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.35 MB (baseline 3.35 MB) • ⚪ 0 BMain entry bundles and manifests
Graph Workspace — 1.15 MB (baseline 1.15 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Views & Navigation — 6.66 kB (baseline 6.66 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 372 kB (baseline 372 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
UI Components — 209 kB (baseline 209 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 1.41 kB (baseline 1.41 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 9.34 MB (baseline 9.34 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 5.38 MB (baseline 5.38 MB) • ⚪ 0 BBundles that do not match a named category
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/ci-tests-e2e.yaml (2)
220-229: Script injection vulnerability withgithub.head_ref.
github.head_ref(the branch name) is user-controlled and used directly in the inline script, which could allow command injection via maliciously crafted branch names. Pass it through an environment variable instead.🔒 Suggested fix
- name: Post starting comment env: GITHUB_TOKEN: ${{ github.token }} + HEAD_REF: ${{ github.head_ref || inputs.ref }} run: | chmod +x scripts/cicd/pr-playwright-deploy-and-comment.sh ./scripts/cicd/pr-playwright-deploy-and-comment.sh \ "${{ inputs.pr_number || github.event.pull_request.number }}" \ - "${{ github.head_ref || inputs.ref }}" \ + "$HEAD_REF" \ "starting" \ "${{ steps.start-time.outputs.time }}"
255-265: Same script injection vulnerability withgithub.head_ref.Apply the same fix here by passing
github.head_ref || inputs.refthrough an environment variable.🔒 Suggested fix
- name: Deploy reports and comment on PR env: CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} GITHUB_TOKEN: ${{ github.token }} GITHUB_SHA: ${{ inputs.ref || github.event.pull_request.head.sha }} + HEAD_REF: ${{ github.head_ref || inputs.ref }} run: | bash ./scripts/cicd/pr-playwright-deploy-and-comment.sh \ "${{ inputs.pr_number || github.event.pull_request.number }}" \ - "${{ github.head_ref || inputs.ref }}" \ + "$HEAD_REF" \ "completed"
🤖 Fix all issues with AI agents
In @.github/workflows/ci-tests-e2e-release.yaml:
- Around line 42-53: The pulls.list call may miss the target PR due to default
pagination; update the code that calls github.rest.pulls.list (and the
subsequent pr lookup using pr.head.sha and
context.payload.workflow_run.head_sha) to fetch all open PRs by either adding a
larger per_page (e.g., per_page: 100) or switching to pagination (e.g., use
github.paginate or octokit.paginate over github.rest.pulls.list) so the prs
array contains every open PR before running prs.find; ensure the ref remains the
same (pr variable and head.sha comparison) after changing the fetch logic.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci-tests-e2e-release.yaml.github/workflows/ci-tests-e2e.yaml
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Myestery
Repo: Comfy-Org/ComfyUI_frontend PR: 7422
File: .github/workflows/pr-update-playwright-expectations.yaml:131-135
Timestamp: 2025-12-12T23:02:37.473Z
Learning: In the `.github/workflows/pr-update-playwright-expectations.yaml` workflow in the Comfy-Org/ComfyUI_frontend repository, the snapshot update process is intentionally scoped to only add and update snapshot images. Deletions of snapshot files are handled explicitly outside this workflow and should not be suggested as part of this automation.
📚 Learning: 2025-12-12T23:02:37.473Z
Learnt from: Myestery
Repo: Comfy-Org/ComfyUI_frontend PR: 7422
File: .github/workflows/pr-update-playwright-expectations.yaml:131-135
Timestamp: 2025-12-12T23:02:37.473Z
Learning: In the `.github/workflows/pr-update-playwright-expectations.yaml` workflow in the Comfy-Org/ComfyUI_frontend repository, the snapshot update process is intentionally scoped to only add and update snapshot images. Deletions of snapshot files are handled explicitly outside this workflow and should not be suggested as part of this automation.
Applied to files:
.github/workflows/ci-tests-e2e-release.yaml.github/workflows/ci-tests-e2e.yaml
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test user workflows in browser tests
Applied to files:
.github/workflows/ci-tests-e2e-release.yaml
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Run quality gates before PR: `pnpm lint`, `pnpm typecheck`, `pnpm knip`, and ensure relevant tests pass
Applied to files:
.github/workflows/ci-tests-e2e-release.yaml
🪛 actionlint (1.7.10)
.github/workflows/ci-tests-e2e.yaml
261-261: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: deploy-and-comment
- GitHub Check: test
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (4)
.github/workflows/ci-tests-e2e-release.yaml (1)
58-65: LGTM!The job dependency chain and conditional execution are well-structured. The string comparison
!= 'null'is correct since GitHub Actions outputs are stringified, and the workflow_call properly passes the required inputs..github/workflows/ci-tests-e2e.yaml (3)
10-19: LGTM!The
workflow_callinputs are well-defined, withrefas required andpr_numberas optional. This allows the workflow to be reused by the release workflow while maintaining backward compatibility with directpull_requesttriggers.
28-33: LGTM!The conditional logic correctly implements the skip behavior for version-bump PRs while ensuring the workflow runs for push events, workflow_call invocations, and non-version-bump PRs.
37-38: LGTM!The checkout steps consistently use
inputs.ref || ''pattern, which correctly falls back to the default ref when not invoked viaworkflow_call.Also applies to: 73-74, 122-123, 168-169
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Comment on lines
+42
to
+53
| const { data: prs } = await github.rest.pulls.list({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| state: 'open', | ||
| }); | ||
|
|
||
| const pr = prs.find(p => p.head.sha === context.payload.workflow_run.head_sha); | ||
|
|
||
| if (!pr) { | ||
| console.log('No PR found for SHA:', context.payload.workflow_run.head_sha); | ||
| return null; | ||
| } |
Contributor
There was a problem hiding this comment.
Potential pagination issue with pulls.list API.
The pulls.list call uses default pagination (30 PRs per page). If there are more than 30 open PRs, the version-bump PR might not be found, causing E2E tests to silently skip.
Consider increasing per_page or using pagination:
🔧 Suggested fix
const { data: prs } = await github.rest.pulls.list({
owner: context.repo.owner,
repo: context.repo.repo,
state: 'open',
+ per_page: 100,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const { data: prs } = await github.rest.pulls.list({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| state: 'open', | |
| }); | |
| const pr = prs.find(p => p.head.sha === context.payload.workflow_run.head_sha); | |
| if (!pr) { | |
| console.log('No PR found for SHA:', context.payload.workflow_run.head_sha); | |
| return null; | |
| } | |
| const { data: prs } = await github.rest.pulls.list({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| state: 'open', | |
| per_page: 100, | |
| }); | |
| const pr = prs.find(p => p.head.sha === context.payload.workflow_run.head_sha); | |
| if (!pr) { | |
| console.log('No PR found for SHA:', context.payload.workflow_run.head_sha); | |
| return null; | |
| } |
🤖 Prompt for AI Agents
In @.github/workflows/ci-tests-e2e-release.yaml around lines 42 - 53, The
pulls.list call may miss the target PR due to default pagination; update the
code that calls github.rest.pulls.list (and the subsequent pr lookup using
pr.head.sha and context.payload.workflow_run.head_sha) to fetch all open PRs by
either adding a larger per_page (e.g., per_page: 100) or switching to pagination
(e.g., use github.paginate or octokit.paginate over github.rest.pulls.list) so
the prs array contains every open PR before running prs.find; ensure the ref
remains the same (pr variable and head.sha comparison) after changing the fetch
logic.
snomiao
approved these changes
Jan 16, 2026
Member
snomiao
left a comment
snomiao
left a comment
There was a problem hiding this comment.
Nice job! the code changes prevents i18n commits cancel e2e tests, SGTM.
and also workflow_run: seems will only works after merge to main.
so I think we can merge it to see if it works, (and revert+fix if it fails).
DrJKL
reviewed
Jan 16, 2026
Comment on lines
+5
to
+7
| workflow_run: | ||
| workflows: ['i18n: Update Core'] | ||
| types: [completed] |
Contributor
There was a problem hiding this comment.
Could this be added as a trigger to the existing e2e workflow?
Contributor
Author
There was a problem hiding this comment.
no not really, that workflow already has enough triggers
DrJKL
added a commit
that referenced
this pull request
Jan 16, 2026
Reverts #8091 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8105-Revert-fix-run-E2E-tests-after-i18n-completes-on-release-PRs-2ea6d73d365081f2bd26cdc932acf7fb) by [Unito](https://www.unito.io)
3 tasks
This was referenced Jan 28, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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
Changes
Modified
ci-tests-e2e.yaml:workflow_calltrigger withrefandpr_numberinputspull_requesttriggerinputs.refwhen called viaworkflow_callCreated
ci-tests-e2e-release.yaml:workflow_runcompletion ofi18n: Update Coreworkflow_call(no job duplication)How it works
Regular PRs:
CI: Tests E2Eruns normally viapull_requesttriggerVersion-bump PRs:
CI: Tests E2Eskips (setup job condition fails)i18n: Update Coreruns and commits locale updatesCI: Tests E2E (Release PRs)triggers after i18n completesCI: Tests E2Eviaworkflow_callTest plan
Fixes the issue identified during v1.38.2 release where locale commits caused E2E tests to restart multiple times.
┆Issue is synchronized with this Notion page by Unito