Repository navigation
feat: run the Playwright test healer on demand on PRs [ED-25918] - #65
Conversation
Adds a reusable playwright-pr-test-healer.yml that Core and Pro call when the `heal` label is added to a PR. Every failed test in the PR's latest Playwright run gets one verdict in a single PR comment: broken by this PR (linked to the diff, never re-run), flaky (passed a re-run; fixed by a Cloud agent, verified on main, and opened as one PR against main), already failing on main, or needs a human look. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| - name: Check out the healer | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| repository: ${{ job.workflow_repository }} | ||
| ref: ${{ job.workflow_sha }} | ||
| path: .healer | ||
| sparse-checkout: actions/playwright-test-healer | ||
| persist-credentials: false | ||
| - uses: actions/setup-node@v6 |
There was a problem hiding this comment.
False positive: this checks out the healer's own scripts from this workflow's commit (job.workflow_repository at job.workflow_sha), not the pull request. Same pattern as the nightly healer, whose identical alerts are open on main.
| - name: Check out the PR head | ||
| if: steps.failures.outputs.failure_count != '0' && steps.failures.outputs.failure_count != '' | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| ref: ${{ needs.resolve-input.outputs.head_sha }} | ||
| path: pr | ||
| persist-credentials: false | ||
|
|
||
| - name: List the PR's changed files |
There was a problem hiding this comment.
Intended, and read-only: the PR head is checked out only so triage can read its spec files and imports. Nothing from it is executed in this job, and credentials are not persisted.
| - name: Check out the healer | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| repository: ${{ job.workflow_repository }} | ||
| ref: ${{ job.workflow_sha }} | ||
| path: .healer | ||
| sparse-checkout: | | ||
| actions/playwright-test-healer | ||
| actions/resolve-core-nightly | ||
| persist-credentials: false | ||
|
|
||
| - uses: actions/setup-node@v6 |
There was a problem hiding this comment.
False positive: this checks out the healer's own scripts from this workflow's commit (job.workflow_repository at job.workflow_sha), not the pull request. Same pattern as the nightly healer, whose identical alerts are open on main.
| - uses: actions/checkout@v6 | ||
| with: | ||
| ref: ${{ needs.resolve-input.outputs.head_sha }} | ||
| persist-credentials: false | ||
|
|
||
| - name: Check out the healer |
There was a problem hiding this comment.
Intended: re-running the PR's failed tests means running the PR's code, as its own pr.yml run does with the same licence. Only same-repository PRs reach this job (writers only); the job has a read token, no persisted credentials and no cache saves.
| - name: Check out the healer | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| repository: ${{ job.workflow_repository }} | ||
| ref: ${{ job.workflow_sha }} | ||
| path: .healer | ||
| sparse-checkout: actions/playwright-test-healer | ||
| persist-credentials: false | ||
| - uses: actions/setup-node@v6 |
There was a problem hiding this comment.
False positive: this checks out the healer's own scripts from this workflow's commit (job.workflow_repository at job.workflow_sha), not the pull request. Same pattern as the nightly healer, whose identical alerts are open on main.
| - uses: actions/checkout@v6 | ||
| if: steps.agent.outputs.branch != '' | ||
| with: | ||
| ref: ${{ steps.agent.outputs.branch }} | ||
| fetch-depth: 0 | ||
| persist-credentials: false | ||
|
|
||
| - name: Check out the healer |
There was a problem hiding this comment.
Intended: this verifies the agent's fix branch, which the healer itself had the Cloud agent push (heal/..., name validated in the step above). Same trust model as the nightly healer's verify-fix: read token, no persisted credentials, no caches, and open-fix-pr re-checks the branch is still at the verified SHA before opening a PR, which then goes through normal review.
| - name: Check out the healer | ||
| if: steps.agent.outputs.branch != '' | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| repository: ${{ job.workflow_repository }} | ||
| ref: ${{ job.workflow_sha }} | ||
| path: .healer | ||
| sparse-checkout: actions/playwright-test-healer | ||
| persist-credentials: false | ||
|
|
||
| - name: Enforce fix scope |
There was a problem hiding this comment.
False positive: this checks out the healer's own scripts from this workflow's commit (job.workflow_repository at job.workflow_sha), not the pull request. Same pattern as the nightly healer, whose identical alerts are open on main.
| - name: Check out the healer | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| repository: ${{ job.workflow_repository }} | ||
| ref: ${{ job.workflow_sha }} | ||
| path: .healer | ||
| sparse-checkout: actions/playwright-test-healer | ||
| persist-credentials: false | ||
| - uses: actions/setup-node@v6 |
There was a problem hiding this comment.
False positive: this checks out the healer's own scripts from this workflow's commit (job.workflow_repository at job.workflow_sha), not the pull request. Same pattern as the nightly healer, whose identical alerts are open on main.
| - name: Check out the healer | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| repository: ${{ job.workflow_repository }} | ||
| ref: ${{ job.workflow_sha }} | ||
| path: .healer | ||
| sparse-checkout: actions/playwright-test-healer | ||
| persist-credentials: false | ||
| - uses: actions/setup-node@v6 |
There was a problem hiding this comment.
False positive: this checks out the healer's own scripts from this workflow's commit (job.workflow_repository at job.workflow_sha), not the pull request. Same pattern as the nightly healer, whose identical alerts are open on main.
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try the Wiz Code extension for VS Code, JetBrains, or Visual Studio. |
…ED-25918] Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gitstream-cm
Bot
added
🚨 14 Unresolved Thread(s)
and removed
🚨 15 Unresolved Thread(s)
labels
… [ED-25918] Bots merge main into PR branches, leaving the head on a commit with no finished run; the healer now falls back to the newest finished run on the branch and names that commit in its comment. `label` is a jq keyword, which broke removing the label. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ED-25918] Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…up sources [ED-25918] Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ntnelbaba
left a comment
Ntnelbaba
left a comment
There was a problem hiding this comment.
Thanks for the detailed write-up. This is large (about 4.5k lines), so it would be easier to review and safer to land in three parts: (1) the shared pieces (profile fields, the rank-nightly-failures / run-healer-attempts exports, HEAL_CONTEXT in dispatch-cloud-agent); (2) the PR healer scripts and their tests; (3) the workflow YAML on its own, since that is where the security-sensitive wiring is.
npx vitest run passes (327 tests) and root lint is clean.
Findings on lines outside the diff hunks, or without a single line:
.github/workflows/playwright-pr-test-healer.yml: Medium: There is no top-levelconcurrency:group keyed by PR number. Two overlappinghealruns can race on comment upsert (tail -1on matching comments), duplicate agent dispatches, and push competingheal/pr-*branches. Addconcurrency: group: pr-healer-${{ inputs.pr_number }}, cancel-in-progress: false(or true, if you prefer superseding).actions/playwright-test-healer/scripts/open-pr-fix-pr.js: Nit: No unit tests formergeVerifiedFixes, branch move detection, or conflict abort paths—onlyreadOutcomesis exported/tested indirectly. Add tests with a fakegit/ghlayer.actions/playwright-test-healer/tests/find-pr-playwright-run.test.js: Nit: No test formain()integration: workflow file name from profile, pagination gap, or head-SHA wait vs older completed run—only unit tests for helpers.actions/playwright-test-healer/tests/pr-healer-report.test.js: Nit: Covers verdict matrix and Slack/fix body well, but no test for comment upsert marker edge cases orresolvePrVerdictswhenmainRerunis empty JSON vs missing file (report job artifact merge).
| if: inputs.product == 'pro' | ||
| timeout-minutes: 30 | ||
| env: | ||
| E2E_PRO_LICENSE: ${{ secrets.E2E_PRO_LICENSE }} |
There was a problem hiding this comment.
High (security): rerun-on-pr checks out the PR head and runs setup-pro-playwright.sh with E2E_PRO_LICENSE in the environment, so arbitrary PR branch code can read the licence. That matches the stated same-repo-only model, but callers should treat this like full privileged E2E CI: restrict who can push branches and add heal, and keep a non-production licence as documented.
There was a problem hiding this comment.
No change: this is policy, not code. It is the same exposure the PR's own pr.yml run has with the licence. The workflow header now states it: anyone who can push a branch and add the label can reach the licence, so pass a non-production one.
| rerun-on-pr: | ||
| name: Re-run on the PR build | ||
| needs: [resolve-input, collect, prepare-builds] | ||
| if: always() && needs.collect.outputs.has_reruns == 'true' |
There was a problem hiding this comment.
Medium (security): rerun-on-pr uses if: always() && needs.collect.outputs.has_reruns == 'true' and does not require needs.prepare-builds.result == 'success'. If the PR build job failed, this job still starts, fails on missing healer-builds-pr, and burns runner time while relying on PR_RERUN_RESULT for verdicts. Gate on needs.prepare-builds.result == 'success' (and optionally skip when prepare-builds was cancelled).
There was a problem hiding this comment.
Fixed in 4e6d0be: rerun-on-pr and the baseline re-runs now require needs.prepare-builds.result == 'success'. A failed build leaves the unlinked failures as needing a human look.
| rerun-on-pr: | ||
| name: Re-run on the PR build | ||
| needs: [resolve-input, collect, prepare-builds] | ||
| if: always() && needs.collect.outputs.has_reruns == 'true' |
There was a problem hiding this comment.
Medium: Same condition lets plan-fixes / fix-agent / verify-fix proceed when flaky JSON outputs exist but main/PR build artifacts never uploaded, producing long failing verify paths instead of a clear early exit. Consider requiring successful prepare-builds anywhere tests or licences run.
There was a problem hiding this comment.
Fixed in 4e6d0be: plan-fixes and everything after it depend on has_flaky, which is only set when rerun-on-pr ran, and that job now requires a successful build.
| - name: Collect the failures | ||
| id: failures | ||
| if: steps.run.outputs.found == 'true' | ||
| run: node "$HEALER_DIR/scripts/collect-pr-failures.js" |
There was a problem hiding this comment.
Medium (security): The collect job downloads playwright-test-results-* artifacts from the source CI run (produced under PR/test code) and feeds error-context.md into triage. A crafted error string could steer token-based linkage (false broken vs false flake). Risk is verdict quality, not RCE; mitigations already exclude snapshot sections—consider capping token matches to quoted locator strings only, or ignoring tokens not present in the # Error details block after sanity checks.
There was a problem hiding this comment.
Partly addressed in 4e6d0be: tokens now match whole words only, which narrows what a crafted error string can link. I did not restrict them to quoted locators. As you say the risk is verdict quality, and a linked failure is only labelled, not acted on.
| evidence.push(fileEvidence(first, changedFiles)); | ||
| } | ||
|
|
||
| const globalChanges = [...changed].filter(isGlobalTestFile); |
There was a problem hiding this comment.
Medium: Any change to a GLOBAL_TEST_FILES path links every failure as broken by this PR. Touching shared Playwright config on a large PR will skip re-runs for all other failures. Worth a comment in the PR table or splitting global-config linkage into a softer signal unless you truly want a hard stop.
There was a problem hiding this comment.
No change: this is deliberate. A change to the shared config can break any test, and re-running would risk a real regression looking flaky. The reason in the comment names the file, so the author can see why every failure was linked.
| return { error: 'The test could not be found to re-run it.' }; | ||
| } | ||
|
|
||
| if (listed > MAX_LISTED_MATCHES) { |
There was a problem hiding this comment.
Low: Re-run aborts when --list matches more than 3 tests, marking the test as needs-human via rerun.error. Document that shared titles across specs require a unique title or a known spec file in error-context Location.
There was a problem hiding this comment.
No change: the reason in the PR comment already says the title matches N tests and can't be re-run on its own, so the verdict is needs a human look.
| ? `Failed shard: ${shardIndex} — the evidence is in that run's playwright-test-results artifact for this shard ("playwright-test-results-${shardIndex}", named as the Repository notes describe). Download that one; do not guess from the Allure title.` | ||
| : 'Failed shard: unknown — find the failed job in that run and download its playwright-test-results-* artifact.', | ||
| `Matching test-results directories inside that artifact to inspect for error-context.md/trace.zip: ${matchedDirs}`, | ||
| ...(context ? ['', context] : []), |
There was a problem hiding this comment.
Low: Optional HEAL_CONTEXT is appended to the agent prompt with no length cap; a huge context env could bloat dispatch or hit API limits. Truncate in buildPrompt (e.g. 4k chars) with a note that details are in the linked run.
There was a problem hiding this comment.
Fixed in 4e6d0be: context is capped at 4,000 characters, with a note pointing to the evidence run.
| # One agent per flaky test, each on its own branch off main. Nothing from | ||
| # the agent runs here: this job holds the Cursor key. | ||
| fix-agent: | ||
| name: Fix ${{ matrix.fix.testName }} |
There was a problem hiding this comment.
Nit: Matrix job name Fix ${{ matrix.fix.testName }} can break Actions UI or logs when titles contain : or very long strings. Use matrix.fix.index in the name and keep the title in step output only.
There was a problem hiding this comment.
Fixed in 4e6d0be: the matrix jobs are now named by index (Fix flaky test N, Verify fix N).
| flattenAllureFailures, | ||
| hasTraceInDirectory, | ||
| matchResultDirectories, | ||
| readHardFailureSource, |
There was a problem hiding this comment.
Nit: Exporting readHardFailureSource / readResultDirs is a pure refactor for PR collect—no nightly behavior change—but nightly and PR now share ranking helpers; add a one-line comment in rank-nightly-failures.js that PR healer depends on these exports so refactors run both test suites.
There was a problem hiding this comment.
Fixed in 4e6d0be: added the note above the exports.
| echo "base_sha=$(jq -r '.base.sha' <<< "$PR")" | ||
| echo "base_ref=$(jq -r '.base.ref' <<< "$PR")" | ||
| echo "pr_url=$(jq -r '.html_url' <<< "$PR")" | ||
| echo "main_sha=$(gh api "repos/${GITHUB_REPOSITORY}/commits/main" --jq '.sha')" |
There was a problem hiding this comment.
High: The comparison baseline is always main (commits/main here), not the PR's base branch. base_ref/base_sha are read on L191-192 but only used for triage and the PR build leg. The non-PR build leg (L341-345, L374, L381), rerun-on-main (L509-564), the agents (HEAL_STARTING_REF/HEAL_BASE_REF: main, L670/L690), verify (BASE_REF: main, L778) and the fix PR (L939, open-pr-fix-pr.js L74/L150-151) all target main. For a PR into release/*, a failure can be labelled 'already failing on main' when it is new on the release line (or the reverse), and flaky fixes get verified and opened against the wrong branch. Nothing refuses non-main bases either.
Until releases move to main-only, this should check both: when base_ref != 'main', add a build and re-run leg on base_sha next to the main one, let the verdict say which baseline(s) it failed on, and pass the fix target ref (the PR base) through the agent env, verify-fix, gh pr list --base (L595, pr-healer-report.js L599) and open-pr-fix-pr.js --base. When the base is main, the current single leg is enough.
There was a problem hiding this comment.
Fixed in 4e6d0be: resolve-input now builds the baselines: main always, plus the PR's base when it isn't main. A PR into a release branch is judged against its base, and the reason says what main did. The agent, verify, gh pr list --base and fix PR all use that target. check_pr_base: 'false' goes back to main only, for when PRs only target main. Unit tests only so far, not run live yet.
Ntnelbaba
left a comment
Ntnelbaba
left a comment
There was a problem hiding this comment.
A few more findings on how artifacts and agent input are trusted. The first one should be fixed before the healer is enabled.
- Info: the head SHA is resolved when the workflow starts rather than pinned at label time, so a push between labelling and
resolve-inputchanges what gets healed. Worth noting in the PR comment which SHA was used.
|
|
||
| return { | ||
| testName: fix.testName, | ||
| branch: agent?.branch || '', |
There was a problem hiding this comment.
Medium (security): open-fix-pr trusts the branch, SHA and pass counts read from the pr-healer-agent-N / pr-healer-verify-N artifacts. Those names are predictable, and PR code running earlier in the same workflow run (the pr leg of prepare-builds, rerun-on-pr) can reach ACTIONS_RUNTIME_TOKEN and upload artifacts with those names first. A same-repo author could then get the bot to merge an arbitrary branch into a "verified" fix PR to main, and opened with GITHUB_TOKEN, that PR triggers no CI. Before merging, rebuild the expected heal/pr-<N>-<slug>-<run_id>-<index> name from trusted inputs and require an exact match, fetch by the verified SHA and assert FETCH_HEAD equals it, and re-run check-fix-scope.js against origin/main here. Please fix this before TEST_HEALER_ENABLED is turned on.
There was a problem hiding this comment.
Fixed in 4e6d0be: the branch must match the one rebuilt from the plan, which is a job output rather than an artifact. The commit is fetched by the verified SHA and FETCH_HEAD must equal it, and check-fix-scope.js runs again against the target. Remaining limit: a forged record can still claim the test passed, so review of the fix PR stays mandatory. Unit tests only so far.
|
|
||
| for (const outcome of merged) { | ||
| try { | ||
| git(['push', 'origin', '--delete', outcome.branch]); |
There was a problem hiding this comment.
Low (security): branch cleanup should only delete branches that match the exact expected heal/pr-… pattern. Otherwise a poisoned outcome artifact can name an unprotected branch to delete.
There was a problem hiding this comment.
Fixed in 4e6d0be: cleanup only uses the branch from the plan, never one named in an artifact.
| @@ -135,6 +137,7 @@ function buildPrompt({ | |||
| ? `Failed shard: ${shardIndex} — the evidence is in that run's playwright-test-results artifact for this shard ("playwright-test-results-${shardIndex}", named as the Repository notes describe). Download that one; do not guess from the Allure title.` | |||
| : 'Failed shard: unknown — find the failed job in that run and download its playwright-test-results-* artifact.', | |||
| `Matching test-results directories inside that artifact to inspect for error-context.md/trace.zip: ${matchedDirs}`, | |||
There was a problem hiding this comment.
Medium (security): test names, matched dirs and error-context.md text from the PR's own run go straight into the agent prompt (also workflow L673-678). Crafted text can steer the agent, for example into adding a helper under tests/playwright/ that still passes the scope check. Mark this block in the prompt as untrusted data the agent must not follow as instructions, and keep human review of the fix PR mandatory.
There was a problem hiding this comment.
Fixed in 4e6d0be: the prompt now marks the test title, directory names and everything in the evidence run as untrusted data not to be followed, and titles and names are kept on one line. This line is also in the nightly healer's prompt, since they share buildPrompt. It lowers the risk rather than removing it, so review of the fix PR stays mandatory.
| path: ${{ runner.temp }}/healer-builds | ||
|
|
||
| - name: Unpack the build | ||
| run: tar -xzf "$RUNNER_TEMP/healer-builds/healer-builds.tgz" |
There was a problem hiding this comment.
Low (security): the healer-builds-main tarball is unpacked into the workspace root (also L804). A poisoned artifact with the same name could overwrite files and forge results. Unpack into RUNNER_TEMP with a path allowlist or check a hash recorded by the trusted build job.
There was a problem hiding this comment.
Fixed in 4e6d0be: unpack-healer-build.sh checks every path against an allowlist, unpacks to a temp folder, then moves only the allowed entries. It checks paths only, not a hash, so a replaced build with valid paths would still get through. That code would have the same access the PR's code already has in those jobs.
| outputs: | ||
| fix_pr_url: ${{ steps.open.outputs.fix_pr_url }} | ||
| steps: | ||
| - uses: actions/checkout@v6 |
There was a problem hiding this comment.
Low (security): this checkout keeps the default persist-credentials, unlike every other checkout here. Set persist-credentials: false and pass the token only to the push step.
There was a problem hiding this comment.
Fixed in 4e6d0be: persist-credentials: false. The token goes only to the git calls that reach the remote (ls-remote, fetch, push), not just the push, because private repos need it for those too.
| exit 1 | ||
| fi | ||
| for N in "$MAX_RERUNS" "$MAX_FIXES" "$VERIFY_REPEAT"; do | ||
| if ! [[ "$N" =~ ^[0-9]{1,2}$ ]]; then |
There was a problem hiding this comment.
Low: verify_repeat accepts 0, which would mark a fix as verified without running it. Validate it as at least 1, like the other limits.
There was a problem hiding this comment.
Fixed in 4e6d0be: verify_repeat must now be 1-99.
| } | ||
|
|
||
| function buildSlackPayload({ repo, prNumber, prUrl, fixPrUrl, fixes }) { | ||
| const tests = fixes.map((fix) => `• \`${fix.testName}\``).join('\n'); |
There was a problem hiding this comment.
Low: testName goes into the Slack mrkdwn unescaped, so &, <, > or backticks in a test title can break the message or inject links. Escape them before building the payload.
There was a problem hiding this comment.
Fixed in 4e6d0be: &, <, > and backticks in test titles are now escaped. Covered by a test.
…ed artifacts [ED-25918] A PR into a release branch is now judged against that branch as well as checked on main, and its fixes go there. check_pr_base=false goes back to main only, for when PRs only target main. The fix PR step no longer trusts the agent and verify artifacts: the branch must match the plan, the fetched commit must be the verified SHA, and the scope check runs again. Also marks evidence as untrusted in the agent prompt, checks build archive paths before unpacking, keeps credentials out of the fix PR checkout, refuses verify_repeat=0, escapes Slack text, matches error tokens as whole words, and skips re-runs when a build failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| - uses: actions/checkout@v6 | ||
| with: | ||
| ref: ${{ matrix.build.sha }} | ||
| persist-credentials: false | ||
|
|
||
| - name: Check out the healer |
| - uses: actions/checkout@v6 | ||
| with: | ||
| ref: ${{ matrix.baseline.sha }} | ||
| persist-credentials: false | ||
|
|
||
| - name: Check out the healer |
| - uses: actions/checkout@v6 | ||
| with: | ||
| ref: ${{ needs.resolve-input.outputs.target_ref }} | ||
| fetch-depth: 0 | ||
| # The script gives the token to the git calls that reach the remote. | ||
| persist-credentials: false | ||
|
|
||
| - name: Check out the healer |
gitstream-cm
Bot
added
🚨 45 Unresolved Thread(s)
🚨 46 Unresolved Thread(s)
and removed
🚨 14 Unresolved Thread(s)
🚨 45 Unresolved Thread(s)
labels
|
Thanks for the review. The fixes are in 4e6d0be, and I've replied on each inline comment with what changed or why it didn't. Your two big points
Findings without a line
Testing: 348 unit tests pass. The live run on elementor/elementor-pro#7860 came before these changes, so the base-branch check, the artifact checks and the unpack check have only unit tests so far. |
Ntnelbaba
left a comment
Ntnelbaba
left a comment
There was a problem hiding this comment.
Re-reviewed 4e6d0be. The base-branch check, the build gate, the artifact checks and the escaping fixes look right, and the 348 unit tests pass.
Still open from the earlier reviews:
- Security: the Pro licence reaching PR code in
rerun-on-pr(documented, not changed); prompt injection into the agent, which is mitigated but not removed; a forged verify record can still claim a pass, so review of the fix PR stays the gate. - Partly fixed: collect artifact trust in triage, and label removal on
disabled/fork/error/no-run. - Test gaps:
find-pr-playwright-runmain()and pagination, and comment marker plus empty-vs-missing main re-run inpr-healer-report. - Splitting the PR.
| rerun-on-pr: | ||
| name: Re-run on the PR build | ||
| needs: [resolve-input, collect, prepare-builds] | ||
| if: always() && needs.collect.outputs.has_reruns == 'true' && needs.prepare-builds.result == 'success' |
There was a problem hiding this comment.
Medium: needs.prepare-builds.result == 'success' is false when any matrix leg fails. On a release-branch PR there are now three builds, so a failing main build (which only adds a side note there) skips the PR re-runs and every baseline, and every unlinked failure ends up as needing a human look. Gate each job on the build it actually uses, e.g. per-build outputs or checking for the artifact, and let a missing baseline build become an error on that baseline only.
| PR=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}") | ||
| HEAD_REPOSITORY=$(jq -r '.head.repo.full_name // ""' <<< "$PR") | ||
| HEAD_SHA=$(jq -r '.head.sha' <<< "$PR") | ||
| BASE_SHA=$(jq -r '.base.sha' <<< "$PR") |
There was a problem hiding this comment.
Low: The base baseline uses the PR's base.sha, while main uses the live tip. base.sha can lag behind the branch, but the agent branches from target_ref and the fix PR targets its tip, so verify-fix can test against a different base than the fix lands on. Resolve it with commits/${BASE_REF} like main.
| BASELINES=$(jq -cn --arg sha "$MAIN_SHA" '[{key: "main", ref: "main", sha: $sha}]') | ||
| TARGET_REF=main | ||
| TARGET_KEY=main | ||
| if [ "$CHECK_PR_BASE" = "true" ] && [ "$BASE_REF" != "main" ]; then |
There was a problem hiding this comment.
Nit: Any value of check_pr_base other than exactly true (True, yes, a typo) quietly switches to main only. Validate it as true or false in the input check step, like the other inputs.
| HEAL_BRANCH_NAME: ${{ matrix.fix.branch }} | ||
| HEAL_SHARD_INDEX: ${{ matrix.fix.shardIndex }} | ||
| HEAL_MATCHED_DIRS: ${{ matrix.fix.matchedDirs }} | ||
| HEAL_STARTING_REF: ${{ needs.resolve-input.outputs.target_ref }} |
There was a problem hiding this comment.
Low: For a PR into a release branch, the flaky fix goes only to that branch. If the same flake exists on main, nothing fixes it there while both are supported. Say so in the PR comment and the fix PR body, or note it as a known limit in the header.
| fi | ||
|
|
||
| UNPACKED=$(mktemp -d "${RUNNER_TEMP:-/tmp}/healer-build.XXXXXX") | ||
| tar -xzf "$ARCHIVE" -C "$UNPACKED" |
There was a problem hiding this comment.
Low (security): The check covers paths only, so symbolic links pass it. An archive holding build/x -> /etc (or a link to $HOME) unpacks and is moved into the workspace as is. Reject any entry that is not a regular file or a directory, either from tar -tvzf or with find "$UNPACKED" ! -type f ! -type d before the move, and add a symlink case to unpack-healer-build.test.js.
gitstream-cm
Bot
added
🚨 51 Unresolved Thread(s)
and removed
🚨 46 Unresolved Thread(s)
labels
…37907) ## What Adds `.github/workflows/pr-test-healer.yml`: adding the `heal` label to a PR calls the shared [Playwright PR test healer](elementor/elementor-editor-github-actions#65) ([ED-25918](https://elementor.atlassian.net/browse/ED-25918)). Every test that failed in the PR's latest Playwright run gets one verdict — ❌ broken by this PR, 🔁 flaky, 🟠 already failing on main, or 👀 needs a human look — with a reason and evidence, in one comment on the PR. Tests linked to the PR's diff are never re-run or changed. Flaky tests are fixed by an agent, verified on main, and opened as a separate PR against `main`, never pushed to the PR's branch. The label is removed when the run ends; add it again to re-run. - Fork PRs are skipped. - Job-level concurrency per PR, so adding another label does not cancel a heal in progress. - Identical to the Pro caller (elementor/elementor-pro#7876) except `product` and the Pro licence secret. ## Before it works - [ ] elementor/elementor-editor-github-actions#65 merged (this calls it at `@main`). - [ ] `TEST_HEALER_ENABLED` is `manual` or `all` (already used by the nightly healer). - [ ] A `heal` label exists. - [ ] Confirm the Slack channel (`C0C0TEZN45C`, the nightly healer's) is #flaky-fixes. [ED-25918]: https://elementor.atlassian.net/browse/ED-25918?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ ## Visual proof #skip_proof This PR only adds a GitHub Actions workflow; there is no editor UI to verify on Playground. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: ElementorBot <48412871+elementorbot@users.noreply.github.com>
What
Adds PR mode to the Playwright test healer: a reusable
.github/workflows/playwright-pr-test-healer.ymlthat Core and Pro call from a thin workflow when theheallabel is added to a PR (ED-25918).Every test that failed in the PR's latest Playwright run gets exactly one verdict, with a reason and evidence, in one comment on the PR (updated in place on each run):
mainbuild.Flaky tests: one Cloud agent per test (existing prompt + skill, starting from
main), each fix verified 10x on themainbuild with the existing scope check, then the verified fixes are merged into one branch and opened as one PR against main — never pushed to the author's branch. The comment links it; Slack announces it. No failures → "nothing to heal", no PR. The label is removed at the end so re-adding it starts a new run.Pieces
scripts/find-pr-playwright-run.js— newest PR-workflow run on the head SHA that ran Playwright, excluding the run thelabeledevent itself starts (pr.yml listens tolabeled).scripts/collect-pr-failures.js— every hard failure, with shard, result dirs, spec fromerror-context.md, error text.scripts/triage-pr-failures.js— diff linkage + evidence lines.scripts/rerun-pr-failures.js— N re-runs per test on its shard's command, retries off.scripts/pr-healer-report.js— verdicts, PR comment, fix-PR body, Slack payload.scripts/open-pr-fix-pr.js— combines verified fixes into one PR.prWorkflowandmoduleRoots;dispatch-cloud-agent.jstakes an optionalHEAL_CONTEXT.Safety
verify-fix(read token + licence).TEST_HEALER_ENABLEDmust bemanualorall.Testing
vitest: 348 passing (new tests for every new script).npm run lintclean; actionlint clean except the pre-existingjob.workflow_*false positives.check_pr_base), the fix PR's artifact checks and the build unpack check. Those came after the live run.[ED-25918]: https://elementor.atlassian.net/browse/ED-25918?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
✨ PR Description
1. Problem & Context
Implements on-demand Playwright test healing for PRs (ED-25918) to differentiate between regressions and flakes. This allows developers to trigger a triage and automated fix process via a GitHub label.
2. What Changed (Where)
.github/workflows/playwright-pr-test-healer.ymlscripts/triage-pr-failures.jsscripts/rerun-pr-failures.jsscripts/collect-pr-failures.jsscripts/find-pr-playwright-run.jsscripts/open-pr-fix-pr.jsscripts/pr-healer-report.jsscripts/dispatch-cloud-agent.jsscripts/unpack-healer-build.shprofiles/*.jsonprWorkflowandmoduleRootsfor triage mapping.tests/*.test.js3. How It Works
The workflow is triggered by a label, initiating a pipeline that:
4. Risks
unpack-healer-build.shand verifying commit SHAs before merging fixes.Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Description using Guidelines Learn how