⚠️ Potential issue | 🔴 Critical

Critical: deploy_staging condition doesn't gate on VR test failures.

The job depends on vr_tests (line 270) but the condition (line 271) only checks needs.build_test_pack.result == 'success'. When VR tests fail, the vr_tests job outcome is 'failure', but this is not checked in the condition, so deploy_staging will still run and deploy sites with failed tests.

Update the condition to explicitly check both jobs:

-    if: needs.build_test_pack.result == 'success'
+    if: needs.build_test_pack.result == 'success' && needs.vr_tests.result == 'success'

Alternatively, if deployments should proceed regardless of VR test status, remove the vr_tests dependency and clarify the intent in a comment.

.github/workflows/ci.yml around lines 268 to 276: the deploy_staging job
currently only gates on needs.build_test_pack.result == 'success' while still
declaring a dependency on vr_tests, so deployments can proceed even if VR tests
failed; update the if condition to explicitly require both successes (e.g.,
needs.build_test_pack.result == 'success' && needs.vr_tests.result == 'success')
so deploy_staging only runs when both jobs pass, or if the intent is to deploy
regardless, remove vr_tests from the needs array and add a comment clarifying
that VR failures should not block deployment.

This is intentional. Failed VR tests will still produce the HTML report that we want to upload.

@PavelVanecek, thank you for clarifying the intention! I understand you want to deploy the staging site with the VR report even when tests fail, so the report can be reviewed.

However, there's a subtle issue with the current implementation. In GitHub Actions, when a job in the needs array fails, dependent jobs are skipped by default, even if the if condition doesn't explicitly check that job's result. Since the vr_tests job will fail when tests fail (due to the "Fail workflow if tests failed" step on line 263), the deploy_staging job will be skipped.

To achieve your intended behavior, you need to explicitly allow deploy_staging to run when vr_tests fails:

if: always() && needs.build_test_pack.result == 'success' && (needs.vr_tests.result == 'success' || needs.vr_tests.result == 'failure')

This ensures:

• The job always evaluates the condition (via always())
• It only runs if the build succeeds
• It runs regardless of whether VR tests pass or fail
• It still skips if vr_tests is cancelled or skipped

Would you like me to create an issue to track this fix?

Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6748
File: .github/workflows/ci.yml:268-276
Timestamp: 2025-12-07T01:38:46.677Z
Learning: In the recharts/recharts repository, the staging deployment pipeline should deploy the website and VR test report even when VR tests fail, so that the HTML report can be reviewed. This requires using `if: always() && needs.build_test_pack.result == 'success' && (needs.vr_tests.result == 'success' || needs.vr_tests.result == 'failure')` in the deploy_staging job condition.

Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-12-06T03:36:59.377Z
Learning: Applies to test-vr/__snapshots__/** : Visual regression test snapshots should be committed to the repository

⚠️ Potential issue | 🟠 Major

PR comment assumes successful deployment.

The "Comment on PR" step posts staging URLs unconditionally, but it runs after the deploy step. If the deploy step fails (git push, etc.), the comment will still post URLs pointing to non-existent or stale deployments, creating a poor user experience.

Add a condition to only comment if the deploy step succeeded, or add a check within the script:

      - name: Comment on PR
        if: github.event_name == 'pull_request'
+       # Add a condition to ensure deploy succeeded before commenting

Alternatively, validate that the deployment completed before posting the URLs.

Committable suggestion skipped: line range outside the PR's diff.

.github/workflows/ci.yml around lines 327 to 345: the "Comment on PR" step posts
staging URLs unconditionally even when the deploy may have failed; modify the
job so the comment runs only on successful deploy (e.g., add an if: ${{
success() }} or if: ${{ needs.deploy_job.result == 'success' }} conditioned on
the deploy job name), or add a lightweight runtime check before posting (curl
the WEBSITE_URL/REPORT_URL or verify the deploy job outputs a success
flag/artifact and only proceed if the check passes); update the step to bail out
(or post a failure note) when the deployment verification fails.