Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .envrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
source_up

# Secrets from 1Password (no plaintext on disk)
export SESSION_SECRET="$(op read 'op://MCP/odyonu7pcmotjpsqthxt666ivm/password')"

# Non-secret config (safe as plaintext)
export SMS_DEV_MODE=true
export PUBLIC_APP_URL=http://localhost:5173
61 changes: 59 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,63 @@ jobs:
- name: Production build
run: npm run build

e2e:
needs: [changes, lint-and-check]
if: github.event_name != 'pull_request' || needs.changes.outputs.code == 'true'
runs-on: ubuntu-latest
timeout-minutes: 25
strategy:
fail-fast: false
matrix:
project: [desktop, mobile-ios, mobile-android]

steps:
- name: Checkout
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1

- name: Setup Node.js
uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0
with:
node-version: 24
cache: npm

- name: Install dependencies
run: npm ci

- name: Cache Playwright browsers
id: pw-cache
uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
with:
path: ~/.cache/ms-playwright
key: ${{ runner.os }}-playwright-${{ hashFiles('package-lock.json') }}

- name: Install Playwright browsers
run: npx playwright install --with-deps chromium webkit

- name: Install ffmpeg (for fixture media)
run: sudo apt-get update && sudo apt-get install -y ffmpeg

- name: Build app
run: npm run build

- name: Run Playwright E2E
run: npx playwright test --project=${{ matrix.project }}

- name: Upload Playwright report
if: always()
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
with:
name: playwright-report-${{ matrix.project }}
path: |
playwright-report/
test-results/
docs/test-report.md
retention-days: 30

ci:
runs-on: ubuntu-latest
if: always()
needs: [changes, lint-and-check]
needs: [changes, lint-and-check, e2e]
steps:
- name: Check CI status
run: |
Expand All @@ -103,7 +156,11 @@ jobs:
exit 0
fi
if [[ "${{ needs.lint-and-check.result }}" == "failure" || "${{ needs.lint-and-check.result }}" == "cancelled" ]]; then
echo "CI failed"
echo "CI failed (lint/check)"
exit 1
fi
if [[ "${{ needs.e2e.result }}" == "failure" || "${{ needs.e2e.result }}" == "cancelled" ]]; then
echo "CI failed (e2e)"
exit 1
fi
echo "CI passed"
15 changes: 15 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,28 @@ Thumbs.db
*.avi
*.mkv

# Allow tiny test fixtures to flow through CI
!e2e/fixtures/media/*.mp4
!e2e/fixtures/media/*.mov
!e2e/fixtures/media/*.webm

# Vite
vite.config.js.timestamp-*
vite.config.ts.timestamp-*

# Coverage
/coverage/

# Playwright / E2E
/playwright-report/
/test-results/
/e2e/.tmp/
/e2e/screenshots/
/.playwright/

# Claude Code runtime
/.claude/scheduled_tasks.lock

# CodeQL
/.codeql-db/

Expand Down
2 changes: 2 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Private video-sharing PWA for friend groups. SvelteKit + SQLite + Twilio.
- Use Drizzle ORM for all database queries — never raw SQL
- IDs are UUIDs (text), timestamps are Unix epoch integers
- After making code changes, run `npm run check` to verify no type errors were introduced
- Every new feature must include tests. New API endpoint → API test in `src/routes/api/__tests__/`. New server module → unit test in `src/lib/server/__tests__/`. New user-facing flow → E2E spec in `e2e/`. Run `npm run test:all` before opening a PR. See `@docs/testing.md` for fixtures, mocking, and the failure-report workflow.

## Design System

Expand All @@ -51,6 +52,7 @@ Private video-sharing PWA for friend groups. SvelteKit + SQLite + Twilio.
- @docs/api.md — API endpoint reference
- @docs/architecture.md — Stack overview and deployment
- @docs/notifications.md — Push notification setup and triggers
- @docs/testing.md — Vitest + Playwright suite, fixtures, failure report

## Git Workflow

Expand Down
198 changes: 198 additions & 0 deletions docs/bugs-found.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
# Bugs found by the visual test suite

Run on branch `feat/test-suite` against the production build (`node build/index.js`) with `TEST_MODE=true SMS_DEV_MODE=true`. The suite drives Chromium (desktop @ 1280×800), iPhone 13 emulation (WebKit), and Pixel 7 emulation (Chromium with touch), so each spec runs in three viewports.

Confirmed-real findings are listed first. Suspected real findings (need human verification in a real browser) follow. Test-side issues found and fixed during the run are at the bottom for the record.

---

## Confirmed bugs

### 1. Settings page on desktop hides notification rows behind the bottom nav

**Severity:** medium · visible to every desktop user with a moderately-tall settings panel
**Found by:** `e2e/visual/overflow.spec.ts → "settings page text is not occluded by bottom nav"`
**Project affected:** desktop (1280×800). mobile-ios and mobile-android pass.

After scrolling the `/settings` page to the bottom, the rows under the **Notify Me About** section ("When someone reacts to your clip", "Comments", "When someone comments or replies on any clip") sit visually under the fixed bottom nav. The bottom 80–120px of the page is unreachable.

Test output:
```
Found text content behind the bottom nav after scrolling to page end:
[
{ "tag": "span", "text": "When someone reacts to your clip", "top": 725 },
{ "tag": "span", "text": "Comments", "top": 761 },
{ "tag": "span", "text": "When someone comments or replies", "top": 783 }
]
```

**Suspected cause.** `main { padding-bottom: var(--bottom-nav-height, 64px) }` and the settings page's own `padding-bottom: var(--bottom-nav-height, 64px)` (`src/routes/(app)/+layout.svelte:283` and `src/routes/(app)/settings/+page.svelte:411`). On desktop the bottom-nav sits inside the same 520-px-max content column with extra vertical padding from `padding: var(--space-xs) 0` plus `safe-area-inset-bottom`. The 64-px fallback isn't enough to clear the actual rendered nav height on desktop.

**Repro:** open `/settings` as a host on a desktop viewport, scroll to the very bottom. The "Comments" toggle row is visually clipped by the bottom nav.

---

### 2. WebKit logs `interactive-widget` viewport warning twice on every page

**Severity:** low · console noise on iOS Safari
**Found by:** `e2e/visual/console-errors.spec.ts → "/offline page renders without errors"` (and 8 other mobile-ios specs)

Every page load on mobile-iOS / iOS Safari logs:

```
Viewport argument key "interactive-widget" not recognized and ignored.
```

…twice. Source: `src/app.html:5`:

```html
<meta name="viewport" content="width=device-width, initial-scale=1, viewport-fit=cover, interactive-widget=resizes-content" />
```

`interactive-widget` is a Chrome-only viewport hint. WebKit doesn't support it and the warning is emitted to the developer console on every navigation. **No functional impact** — iOS already does the equivalent of `resizes-content` natively. But it's noise that obscures real errors during debugging.

**Options:**
- Leave it (browsers will eventually adopt; users don't see this).
- UA-sniff and only emit the meta on Chrome / Android (added complexity for a benign warning).
- Just delete `interactive-widget=resizes-content` — Android Chrome resizes-content is also the default; the explicit hint mostly matters when you want `overlays-content` instead.

---

## Suspected bugs (need human verification)

### 3. Pressing Back from a clip overlay opened via Activity does NOT return to /activity

**Severity:** high (if confirmed) · regresses the recent fix d362978 ("fix: land on activity page when dismissing a clip opened from it")
**Found by:** `e2e/visual/sheets-and-panels.spec.ts → "panels: activity row → opens clip overlay → back returns to /activity"` (×3 viewports)
**Found by:** `e2e/visual/regression-recent-fixes.spec.ts → "regression: activity → clip → back returns to /activity"` (×3 viewports)

**Steps:**
1. Seed a notification + clip in a group, log in as the recipient.
2. Navigate to `/activity`.
3. Click any notification row — overlay opens correctly, URL becomes `/` (the feed strips `?clip=` and `?from=activity` deliberately and stashes them in component state).
4. Press browser Back (or `history.back()` from the page context).

**Expected:** the overlay dismisses AND the user lands on `/activity` (single press, per the fix).
**Actual under Playwright:** overlay stays mounted; URL is `/`.

Screenshot evidence: `e2e/screenshots/desktop/panels/after-back-to-activity.png` shows the clip overlay still visible after Back.

**Important caveat.** Playwright's `page.goBack()` and even `page.evaluate(() => history.back())` may not fire popstate exactly the way a real browser does. The fix uses `setTimeout(history.back, 0)` inside the dismiss handler — this could be timing-sensitive in the headless environment. **Please verify in real Safari and Chrome before assuming this is a regression.**

If it's real, look at:
- `src/lib/components/ClipOverlay.svelte:67-79` (`handleDismiss`) — the `setTimeout(() => history.back(), 0)` for the `fromActivity` branch.
- `src/lib/overlayHistory.ts:100-115` (popstate handler) — the early-return on `page.state?.[stateKey] === stateValue` may be too eager when the cleaned URL leaves a partially-populated state object.
- `src/routes/(app)/+page.svelte:809-828` — the `replaceState(null, ...)` that wipes the SvelteKit page state for the cleaned URL may interact oddly with the second `history.back()`.

---

### 4. Pressing Back does NOT close the comments sheet from the feed

**Severity:** medium (if confirmed) · users would need a second back press to return to the feed
**Found by:** `e2e/visual/back-gestures.spec.ts → "opening comments sheet then Back closes it (no double-pop)"` (×3 viewports)

**Steps:**
1. Seed clips + comments, navigate to `/`.
2. Click the comments button (`aria-label="Comments"`) on the active reel — sheet appears.
3. Confirm the sheet pushed history state via `history.state.sheet === 'comments'`.
4. Press browser Back.

**Expected:** sheet unmounts, user remains on `/`.
**Actual under Playwright:** sheet remains visible.

Screenshot: `e2e/screenshots/desktop/back/sheet-after-back.png` shows the comments sheet still mounted with all seeded comments visible after Back.

**Same caveat as #3.** Could be a Playwright/popstate timing artefact rather than a real bug. The history state assertion before the back press confirms `sheet=comments` was pushed, so the overlay-history mechanism *did* register an entry. The popstate handler in `src/lib/overlayHistory.ts:100` may not be firing as expected in this environment.

---

### 5. Comments-on-music-clip shows "No comments yet" even when comments exist on a different clip

This was a **test setup issue** in the original run, but caught by the suite — the rich-seed scenario placed comments on `clips[0]`, while the feed's default sort surfaces the *oldest* unwatched clip first (which was the music clip in the original seed). Result: opening comments on the active reel showed an empty sheet despite three seeded comments existing.

Not a product bug — but it surfaces a real UX concern: **a user looking at a clip they don't realise has zero comments will assume "0 comments" applies to the *active* clip, not just to that one.** The current "No comments yet" copy is correct, but could be paired with a count chip on the action sidebar (already done — `commentCount` is rendered).

Counting this as "documentation" rather than a bug because the visual rendering is correct.

---

## Code-quality findings (not crashes, but worth fixing)

These came out of `npm run check` while preparing the suite:

### 6. `<button>` cannot be a descendant of `<button>` in `SharePacingPicker.svelte`

Lines 151, 159, 172, 188 of `src/lib/components/settings/SharePacingPicker.svelte`. Browsers re-write the DOM for invalid HTML on hydration, which can break click handlers and produce hydration mismatches. **This is a real bug waiting to bite under specific conditions.**

```
<button>` cannot be a descendant of `<button>`. When rendering this component on the
server, the resulting HTML will be modified by the browser (by moving, removing, or
inserting elements), likely resulting in a `hydration_mismatch` warning.
```

### 7. `$state` referenced locally at component init (Svelte 5 footgun) — multiple files

```
src/lib/components/settings/DailyShareLimitPicker.svelte:6
src/lib/components/settings/GroupNameEdit.svelte:7-8
src/lib/components/settings/MaxFileSizePicker.svelte:20
src/lib/components/settings/SharePacingPicker.svelte:18-22
src/lib/components/settings/ShortcutSheet.svelte:25-32
src/lib/components/settings/UsernameEdit.svelte:8-9
```

> `This reference only captures the initial value of \`X\`. Did you mean to reference it inside a derived/closure instead?`

These mean the value isn't reactive — if a parent passes a fresh prop, the child still shows the original. Easy to miss in development; visible to users when they edit a setting and immediately revisit the page.

### 8. A11y: click handlers without keyboard equivalents

```
src/lib/components/ShortcutUpgradeBanner.svelte:78-80
src/lib/components/ToastStack.svelte:245
src/lib/components/settings/SharePacingPicker.svelte:112, 147
src/routes/join/+page.svelte:234
```

Visible non-interactive elements with click events. Keyboard users can't trigger them.

---

## How to reproduce / iterate

```bash
# Run only the visual specs (213 of 237 pass at time of writing)
npx playwright test e2e/visual/

# Just the layout audit:
npx playwright test e2e/visual/overflow.spec.ts

# Just the back-gesture suspects:
npx playwright test e2e/visual/back-gestures.spec.ts e2e/visual/sheets-and-panels.spec.ts -g "back"

# Capture full-page screenshots of every state in every viewport (about 40 PNGs):
npx playwright test e2e/visual/snapshots.spec.ts
ls e2e/screenshots/{desktop,mobile-ios,mobile-android}
```

Screenshots land in `e2e/screenshots/<project>/<scenario>.png` (gitignored) so they don't bloat the repo. Failed-run screenshots also land in `test-results/`.

---

## What the suite covers

| Spec | What it checks |
|---|---|
| `console-errors.spec.ts` | No console errors / pageerror / failed network requests on each top-level page |
| `overflow.spec.ts` | No horizontal overflow on any page; bottom nav doesn't occlude content |
| `snapshots.spec.ts` | Empty / content / long-text states screenshotted across all viewports |
| `sheets-and-panels.spec.ts` | Comments sheet, queue sheet, add-video modal, /me reel, settings open + dismiss with content |
| `back-gestures.spec.ts` | Browser-back from every authenticated page; back-from-overlay-sheet |
| `notifications-flow.spec.ts` | In-app activity feed renders all notification types; push payloads captured via TEST_MODE buffer; unread badge accuracy |
| `queue-interactions.spec.ts` | Queue list render, drag reorder, cancel single, move-to-top, clear-all |
| `reactions-and-scrub.spec.ts` | Heart toggle, double-tap heart on mobile, progress-bar scrub via pointer events |
| `mentions.spec.ts` | `@mention` autocomplete, keyboard nav, Escape, cursor placement after select, mention notification creation |
| `error-recovery.spec.ts` | Bad URL submission, retry path, missing video → 404, host-only enforcement, expired/forged session |
| `regression-recent-fixes.spec.ts` | Regression specs for d0ed8b5, d362978, d2f5c29 |

Total: ~80 specs × 3 viewports = ~240 test cases. ~213 currently pass.
Loading