[cristim]

The default-branch ruleset requires only CI Success and the pre-commit
check, and the Playwright job ran in its own workflow behind a paths
filter, so a red or missing run gated nothing. Requiring that context
directly would block every PR that never reports it. The job now runs in
ci.yml as frontend-e2e (still named Playwright Chromium) with its steps,
caps and cache handling unchanged, and CI Success needs it. ci.yml runs
on every PR and on push to main, which keeps the default-branch browser
cache populated. The cost is about 70s per PR that touches nothing under
frontend/. No ruleset change is needed.

Closes #212

Verification

Local: actionlint and every pre-commit hook pass on ci.yml. This PR's own CI run is the end-to-end check: the Playwright Chromium job must run inside ci.yml and CI Success must list it in needs.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 40 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: LeanerCloud/cloud-commitments-platform
• Branch: ci/require-playwright
• File: .coderabbit.yaml
• Commit: 3c8d969
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 075a50e3-cac7-4291-ae3e-adec9c00db95

Commits

Reviewing files that changed from the base of the PR and between 97c7d59 and 3c8d969.

Files selected for processing: 2

• .github/workflows/ci.yml
• .github/workflows/frontend-e2e.yml

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[cristim]

APPROVED ae43597 by claude-cc-platform

Cold review, CI only (ci.yml +170, frontend-e2e.yml deleted).

• actionlint: v1.7.12 on every workflow at this head (scratch worktree) passes with no findings. The PR's own Lint Workflows job (the repo-pinned actionlint) was still pending when I checked.
• Steps unchanged: I diffed the old playwright job's steps: block against the new frontend-e2e job's. They're identical apart from one trailing blank line. timeout-minutes: 20, working-directory: frontend, CI: "true", the per-step 8-minute caps and the split cache restore/save all carry over.
• Pinned SHAs: every uses: in ci.yml is pinned to a 40-character SHA (grep for any non-SHA ref: none). The moved job reuses the same pins as the deleted workflow (checkout v7.0.1, setup-node v7.1.0, cache/restore v6.1.0).
• No required check dropped or renamed: the job keeps the name Playwright Chromium. CI Success adds frontend-e2e to needs and allowlists success over toJSON(needs), so a failed, cancelled or skipped Playwright run now fails CI Success. Nothing else in the repo references frontend-e2e.yml or Frontend E2E (git grep at this head).
• Triggers: ci.yml runs on every PR and push to main/develop, so the Playwright check now reports on every PR. The main-push cache population that the deleted workflow existed for is kept.

Non-blocking:

• The old workflow had concurrency with cancel-in-progress for PRs. ci.yml has no workflow-level concurrency, so a superseded PR push now runs Playwright to completion. That's a cost, not a correctness issue.
• The existing # Summary job - all checks must pass comment now sits above the new frontend-e2e job instead of ci-success. The new job could go above that comment.

Merge after this PR's own run shows Playwright Chromium inside "CI - Build & Test" and CI Success green (34 checks were pending when I looked).

[cristim]

APPROVED ae43597 by claude-nibblets

Review of #1821 (#212), CI only: ci.yml +170, frontend-e2e.yml deleted.

• actionlint: v1.7.12, with shellcheck on PATH, so the run: scripts are linted too. It reports .github/workflows/ci.yml at this SHA clean.

• No required check is dropped or renamed. The only ruleset, protect-default-branch (active, ~DEFAULT_BRANCH), requires exactly CI Success and Run pre-commit hooks, and main has no classic branch protection.

  • CI Success keeps its name and gains frontend-e2e in needs (ci.yml:2188). Its check allowlists success over toJSON(needs), so a failed, cancelled or skipped Playwright job now fails it.
  • The job keeps its context name, Playwright Chromium. Nothing at this SHA still references frontend-e2e.yml or "Frontend E2E".

• Action pins: every uses: in the moved job is a full 40-character SHA with a version comment:

  • checkout@3d3c42e5 (v7.0.1);
  • setup-node@949feb24 (v7.1.0);
  • cache/restore and cache/save@55cc8345 (v6.1.0);
  • upload-artifact@cf430e03 (v7.0.2).

  These are byte-for-byte the pins the deleted workflow used. I diffed the job's steps: block against the old file, and the only difference is one trailing blank line.

• Permissions don't widen: ci.yml has top-level permissions: contents: read (:29-30), the same as the deleted file, and the job adds none.

• Triggers and concurrency: ci.yml runs on pull_request and push to main and develop, plus workflow_dispatch, with no paths filter. So the job now runs on every PR, at the ~70s cost the body states, and main pushes still populate the default-branch cache. ci.yml has no concurrency block, so a main push is never cancelled. That keeps the old file's intent (it cancelled PR runs only); PR runs simply stop cancelling superseded ones.

Nit (non-blocking): the existing # Summary job - all checks must pass comment (ci.yml:1988) now sits above frontend-e2e instead of ci-success. Move the new block above it.

CI at this SHA: when I posted, Run pre-commit hooks was green, and Playwright Chromium was running inside the CI - Build & Test workflow, which is what the body asks for. CI Success hadn't reported yet, so it must go green before merge. Review only, not merging.

[cristim]

Merged main in to clear the Unit Tests failure. That failure was main's TestIdempotencyKeyCopyHasSingleWriter break, which #1823 fixed. The new head is 3c8d969, a merge-only commit. The PR's own diff against the new merge base 97c7d59 is unchanged: ci.yml +170 and frontend-e2e.yml -193, and git range-diff shows the commit as identical. Delta approvals at the new head are needed from @claude-cc-platform and @claude-nibblets (both approved ae43597).

[cristim]

APPROVED 3c8d969 by claude-cc-platform

Merge-only delta from my review at ae435973. 3c8d9698 has two parents: ae435973 (my reviewed head) and main 97c7d591, which includes #1823's scanner-pin fix. The PR diff against its new merge base (git diff 97c7d591 3c8d9698) is byte-identical to the reviewed diff (ae435973 against its old base), so my earlier verdict carries over unchanged. Most CI checks were still pending when I looked (1 pass, 36 pending); merge once Playwright Chromium and CI Success are green.

[cristim]

APPROVED 3c8d969 by claude-nibblets

Merge-only delta from my approval at ae435973. 3c8d969 is a two-parent merge of ae435973 and main 97c7d591, which brings in #1239, #1823, #1813 and #1816.

• The merge is clean: git show --remerge-diff 3c8d969 is empty, so there are no conflicts and no hand edits.
• The PR diff is unchanged: diff(merge-base, ae435973) and diff(merge-base, 3c8d969) are byte-identical: ci.yml +170 and frontend-e2e.yml deleted.
• Nothing from main touches .github/ in ae435973..3c8d969, so the new frontend-e2e job and the CI Success needs entry have nothing to collide with.
• actionlint v1.7.12, with shellcheck, reports ci.yml at this SHA clean.
• Required checks: the only ruleset (protect-default-branch) still requires CI Success and Run pre-commit hooks, and neither is renamed or dropped. The action pins are unchanged from my earlier review.

Note: main has since gained e8a6c625 (#1811, docs only, nothing under .github/), so the PR is 1 behind. It'll need another update before merge, and that's merge-only for workflow purposes.

CI at this SHA: 36 checks were still running when I posted, including Playwright Chromium and CI Success, and they must go green before merge. Review only, not merging.