Skip to content

ci: add build + e2e gate on PRs, pin node 20 - #300

Merged
leobragaz merged 3 commits into
mainfrom
ci/build-e2e-gate
Jul 29, 2026
Merged

ci: add build + e2e gate on PRs, pin node 20#300
leobragaz merged 3 commits into
mainfrom
ci/build-e2e-gate

Conversation

@leobragaz

@leobragaz leobragaz commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

What

  • lint.ymlCI workflow with three jobs, triggered on pull_request → main and push → main:
    • lint (renamed from submit): prettier + eslint + tsc, unchanged steps
    • build (new): npm run build — real WXT production build, catches build breaks a typecheck misses
    • e2e (new): builds the extension, npx playwright install --with-deps chromium, then xvfb-run npm run e2e — the fixtures launch headed Chrome with the MV3 extension loaded, so xvfb provides the display
  • package.json: engines.node: 20.x so local and CI agree

e2e status: wired in AND green ✅

The test had never run anywhere, and it had rotted: onboarding was rebuilt as a step-based wizard, so #onboarding / #pass-onboarding / #setup-password-screen no longer existed and the #/setup route was gone. Fixed in this PR (12 lines): restored the three ids on WelcomePage / CreatePasswordPage, and SetupPasswordScreen no longer navigates to the removed route. Passes locally (~6s) and in CI.

Branch protection (Leo, run after merge)

gh api -X PUT repos/forbole/kastle/branches/main/protection \
  -f 'required_status_checks[strict]=true' \
  -f 'required_status_checks[checks][][context]=lint' \
  -f 'required_status_checks[checks][][context]=build' \
  -f 'required_status_checks[checks][][context]=e2e' \
  -F 'enforce_admins=false' \
  -F 'required_pull_request_reviews=null' \
  -F 'restrictions=null'

e2e ran green on first fixed attempt; if it ever turns flaky, drop its line from the command above and re-run — lint + build stay the hard gate.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved onboarding flow validation, including welcome and password setup screens.
    • Updated password setup navigation to work correctly within the onboarding experience.
  • Tests

    • Expanded automated checks to cover linting, builds, and end-to-end onboarding scenarios.
  • Chores

    • Continuous integration now runs for pull requests and pushes to the main branch.
    • Standardized supported runtime to Node.js 20.x.

- lint.yml -> CI workflow: lint (renamed from submit), build (wxt build), e2e (Playwright via xvfb)
- triggers on pull_request -> main and push -> main
- engines.node 20.x so local and CI agree

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 28, 2026 15:52
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

Note

.coderabbit.yml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized keys: "labels", "include_paths", "exclude_paths", "filters", "review", "pull_request", "limits", "commands", "messages"
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4d88a4e8-eda5-44ef-b708-079ee7643607

📥 Commits

Reviewing files that changed from the base of the PR and between 38d1152 and a24e70d.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (5)
  • .github/workflows/lint.yml
  • tests/onboarding.spec.ts
  • tests/pages/setup-password.ts
  • ui/full-page/create-password/CreatePasswordPage.tsx
  • ui/full-page/welcome/WelcomePage.tsx

📝 Walkthrough

Walkthrough

The CI workflow now runs lint, build, and end-to-end jobs with Node.js 20.x. Onboarding elements receive stable IDs, and the Playwright password setup flow no longer navigates to a dedicated setup route.

Changes

CI runtime and onboarding validation

Layer / File(s) Summary
Runtime and CI validation
package.json, .github/workflows/lint.yml
Declares Node.js 20.x, updates CI triggers and linting, and adds build and Playwright end-to-end jobs.
Onboarding selectors and password setup
ui/full-page/create-password/CreatePasswordPage.tsx, ui/full-page/welcome/WelcomePage.tsx, tests/pages/setup-password.ts, tests/onboarding.spec.ts
Adds stable onboarding IDs and changes password setup tests to wait for the in-place screen without using an extension route.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: adding build and e2e CI gates for PRs and pinning Node.js 20.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/build-e2e-gate

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

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
.github/workflows/lint.yml (1)

42-43: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Use npm ci for CI dependency installs.

package-lock.json is committed, so replace the npm install steps in .github/workflows/lint.yml with npm ci at lines 21, 43, and 60. This keeps CI installs deterministic and lockfile/package-consistent without rewriting the lockfile.

♻️ Proposed fix
-        run: npm install
+        run: npm ci
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/lint.yml around lines 42 - 43, Replace every npm install
dependency-install step in the workflow, including the steps near lines 21, 43,
and 60, with npm ci. Keep the existing step structure unchanged so CI uses the
committed package-lock.json deterministically.

Source: MCP tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/lint.yml:
- Around line 1-10: Add a workflow-level permissions block to the CI workflow,
granting only contents: read for the GITHUB_TOKEN. Place it alongside the
top-level name/on configuration, before jobs, and leave the lint job behavior
unchanged.
- Line 35: Update both actions/checkout@v4 steps in the workflow to set
persist-credentials to false, ensuring the GitHub token is not retained for
subsequent job commands.

---

Nitpick comments:
In @.github/workflows/lint.yml:
- Around line 42-43: Replace every npm install dependency-install step in the
workflow, including the steps near lines 21, 43, and 60, with npm ci. Keep the
existing step structure unchanged so CI uses the committed package-lock.json
deterministically.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: 80b4acd8-fccd-4521-97f7-6c2321488a05

📥 Commits

Reviewing files that changed from the base of the PR and between d8fcfbf and 38d1152.

📒 Files selected for processing (2)
  • .github/workflows/lint.yml
  • package.json

Comment thread .github/workflows/lint.yml
Comment thread .github/workflows/lint.yml
leobragaz and others added 2 commits July 28, 2026 18:09
- restore #onboarding / #pass-onboarding ids on WelcomePage
- add #setup-password-screen id on CreatePasswordPage
- SetupPasswordScreen no longer navigates to removed #/setup route

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- permissions: contents: read at workflow level
- persist-credentials: false on all checkout steps
- npm install -> npm ci; sync lockfile engines field

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@leobragaz

Copy link
Copy Markdown
Contributor Author

@coderabbitai All review comments addressed in a24e70d:

Comment Classification Action
Declare least-privilege GITHUB_TOKEN permissions Valid (security) Added workflow-level permissions: contents: read
Disable checkout credential persistence Valid (security) persist-credentials: false on all three checkout steps
Use npm ci for CI installs Valid (nitpick) Replaced npm install × 3; also synced package-lock.json (engines field) so npm ci is deterministic

Verified: npm ci, prettier, and the onboarding e2e all pass locally; all four PR checks (lint, build, e2e, CodeRabbit) green after the fix commit.

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@leobragaz Thanks for addressing these. The workflow now applies least-privilege token access, avoids persisting checkout credentials, and uses deterministic installs with npm ci.

🐇 ✅


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

@leobragaz
leobragaz merged commit c75f69e into main Jul 29, 2026
4 checks passed
@leobragaz
leobragaz deleted the ci/build-e2e-gate branch July 29, 2026 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants