From 0611900f8e9d61f042baf7a8b3822ce66b790afc Mon Sep 17 00:00:00 2001 From: TurtleWolfe Date: Thu, 14 May 2026 03:23:29 +0000 Subject: [PATCH] chore: gitignore .screenshots/ + capture round 10 lessons in CLAUDE.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two small hygiene changes that lock in this session's learnings. 1. .gitignore — add `.screenshots/`. The existing `screenshots/` entry on line 120 didn't match the dot-prefixed variant, so .screenshots/ has been showing up in every `git status` run for weeks. One-line fix. 2. CLAUDE.md — new section "CI & E2E Stability (Round 10 Lessons, 2026-05-13)" between "Supabase Database Migrations" and "Important Notes", capturing: - NEVER merge a PR while another PR's CI is running against the same backend. The concurrency mutex in e2e.yml protects this now, but the rule still applies for any other shared backend introduced later. - NEVER bypass commit hooks. Husky + lint-staged + gitleaks catch real bugs; --no-verify is explicitly forbidden without user opt-in. - Programmatic `el.scrollTop = N` does NOT fire scroll events in WebKit reliably. The fix pattern (dispatch the scroll event explicitly) is now applied at 4 sites in tests/e2e/messaging/, and any future test code touching scrollTop should follow it. - Branch hygiene — delete_branch_on_merge=true, fetch --prune after merge, no floating unmerged branches, avoid stacked PRs (PR #87 auto-close footgun documented). - CI logs API ≠ UI. The Actions list page is misleading for in-progress matrix runs; use `gh run view --json` for authoritative state. Plus a small fix on the existing "Important Notes" bullet — the line claiming "E2E tests are local-only, not in CI pipeline" was stale. E2E now runs on every push and PR across 28 shards (chromium + firefox + webkit). Updated to reflect reality and point at the new CI section. Verification: - `git check-ignore .screenshots/` confirms it's now matched by the new line 121 entry - `git status` no longer shows .screenshots/ as untracked - Pre-commit hooks ran clean (prettier + gitleaks) Refs: PR #89 (concurrency mutex), PR #87 (stacked-PR autoclose footgun), memory entries feedback_branch_hygiene.md and lesson_concurrent_ci_shared_backend.md Co-Authored-By: Claude Opus 4.7 (1M context) --- .gitignore | 1 + CLAUDE.md | 48 +++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index c796c112..2454591c 100644 --- a/.gitignore +++ b/.gitignore @@ -118,6 +118,7 @@ docs/visual-testing/ # Test screenshots screenshots/ +.screenshots/ mobile-check.png # Runtime test fixtures — created by node:test beforeEach, deleted by afterEach. diff --git a/CLAUDE.md b/CLAUDE.md index a5b3e4e2..29ae5397 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -359,11 +359,57 @@ ADD COLUMN IF NOT EXISTS encryption_salt TEXT; - Suggest running SQL snippets piecemeal - Use Supabase CLI migrations +## CI & E2E Stability (Round 10 Lessons, 2026-05-13) + +The E2E suite ran against a single shared Supabase project for months and accumulated **9 rounds of "flake mitigation"** that all attacked symptoms. Round 10 (PR #89, commit `996211e`) finally identified the underlying root cause and fixed it structurally. These rules exist so future contributors don't re-derive the same painful path. + +### NEVER merge a PR while another PR's CI is running against the same backend + +**Why**: this is what caused issue #85 to compound. Two concurrent E2E runs against the same Supabase project race each other's `cleanupOldMessages` `beforeAll` hooks across 11+ messaging specs. Each run wipes data the other run is polling for. One run hits the 60-min GitHub Actions job cap and gets cancelled. + +**Now protected by**: `.github/workflows/e2e.yml` has a repo-wide `concurrency:` mutex (`group: e2e-supabase-${{ github.repository }}`, `cancel-in-progress: false`). Concurrent E2E runs queue; they never race. But the rule still applies to other shared backends in the future. + +**Verification it's active**: trigger two pushes within 1 minute; the second workflow shows "Queued, waiting for another workflow run" in the Actions UI. + +### NEVER bypass commit hooks (no `--no-verify`) + +**Why**: husky + lint-staged + gitleaks all run pre-commit and catch real bugs. Yesterday's session almost shipped secrets in a doc PR; gitleaks caught it. The user explicitly forbids `--no-verify` unless they ask for it. + +**If a hook fails**: investigate. Hook output names the file + line. Fix the underlying issue, re-stage, commit. Never `git commit --no-verify` as an escape hatch. + +### Programmatic `el.scrollTop = N` does NOT fire scroll events reliably in WebKit + +**Why this matters**: Chromium and Firefox auto-fire the `scroll` event when JavaScript assigns to `scrollTop`. WebKit (Playwright's Linux build) does not always do this. Yesterday's `messaging-scroll.spec.ts:261` test ("T007-T008: Jump button appears when scrolled") failed all 3 retries on webkit-msg because the React `handleScroll` listener at `src/components/molecular/MessageThread/MessageThread.tsx:194` never ran. + +**The fix pattern** (now applied at 4 sites in `tests/e2e/messaging/`): + +```typescript +await el.evaluate((el) => { + el.scrollTop = N; + el.dispatchEvent(new Event('scroll', { bubbles: true })); // <-- explicit dispatch for WebKit +}); +``` + +Apply this any time test code sets `scrollTop` and expects a scroll-event-driven UI side effect. + +### Branch hygiene — NON-NEGOTIABLE + +- **`delete_branch_on_merge=true`** is set on the repo. Every merged PR auto-deletes its head branch. Don't undo this. +- **After merging**, always `git fetch --prune origin` to drop the dead remote-tracking ref locally. +- **Never leave unmerged branches or open PRs sitting around** between work items. Merge or close one before starting the next. +- **Avoid stacked PRs** unless dependency is unavoidable. When a parent PR merges with `delete_branch_on_merge=true`, GitHub auto-closes any child PR using that parent as its base (known footgun — happened to PR #87 yesterday). Re-target the child to `main` and reopen. + +### CI logs API ≠ UI + +- **Authoritative state**: `gh run view --json status,conclusion,jobs` or REST API +- **UI is misleading**: the workflow-run-list page shows the workflow's _overall_ status with the most-recent activity timestamp. That timestamp is when the _last queued sub-job started_, not when the run as a whole started. Reading "In progress 10:35 PM" as "nothing started yet" is wrong but easy to do. +- **For per-job status**: click into the run itself (job-graph view) or use the API. Don't trust the list page. + ## Important Notes - Never create components manually - use the generator - All PRs must pass component structure validation -- E2E tests are local-only, not in CI pipeline +- **E2E tests DO run in CI** (chromium + firefox + webkit across 28 shards) — was previously local-only but is now part of the GitHub Actions pipeline; see "CI & E2E Stability" above - Docker-first development is mandatory - Use `min-h-11 min-w-11` for 44px touch targets (mobile-first)