fix(user-flow): close offline-queue, export, session, ban & realtime gaps
Some checks failed
E2E / Playwright E2E (chromium-desktop) (push) Failing after 6m52s
E2E / Cross-UA smoke matrix (push) Failing after 3m50s

Comprehensive user-flow review across guest/host/admin roles, then fixes with
e2e regression guards. Highlights:

- Offline upload queue auto-resumes on reconnect: network errors keep items
  pending (not error), 4xx are terminal (no infinite retry), quota exhaustion
  returns a distinct 413; queue cap + dedup.
- Export lifecycle: release atomically locks uploads; reopen invalidates and
  re-release regenerates the keepsake; workers claim their job atomically so a
  reopen->re-release can't corrupt the ZIP; startup re-spawns interrupted
  exports.
- Sessions slide on activity (no 30-day cliff); JWT expiry deferred to the
  revocable session row; sign-out-everywhere + revoke-on-PIN-reset.
- Ban always hides content (v_feed / find_visible_media / export filter
  is_banned) but stays a read-only ban per USER_JOURNEYS §10 — sessions are
  not revoked, read access + keepsake download preserved.
- Realtime: server-clock SSE delta cursor; event-closed/opened drive the UI
  live; feed_delta rate-limited; like returns {liked, like_count} to fix
  multi-device drift; lightbox live comments; diashow delta backfill.
- Forgotten-PIN in-app request flow; simultaneous same-name join returns 409;
  quota increment is transactional; operator floor.

Adds e2e/specs/10-flow-review/ (offline resume, export integrity, deterministic
anti-race guard) and updates existing specs for the new contracts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-07-10 23:05:37 +02:00
parent 16d1f356be
commit 641174717c
38 changed files with 1400 additions and 153 deletions

View File

@@ -31,21 +31,21 @@ test.describe('Feed — like + comment', () => {
expect(row.liked_by_me).toBe(false);
// First like → counted exactly once.
expect(await like(liker.jwt, uploadId)).toBe(204);
expect(await like(liker.jwt, uploadId)).toBe(200);
row = findFeedRow(await api.getFeed(liker.jwt), uploadId);
expect(row.like_count).toBe(1);
expect(row.liked_by_me).toBe(true);
// Liking again is a toggle → back to zero (guards against a regression that
// double-counts a repeated like instead of removing it).
expect(await like(liker.jwt, uploadId)).toBe(204);
expect(await like(liker.jwt, uploadId)).toBe(200);
row = findFeedRow(await api.getFeed(liker.jwt), uploadId);
expect(row.like_count).toBe(0);
expect(row.liked_by_me).toBe(false);
// A second distinct user's like is counted independently (per-user semantics).
expect(await like(liker.jwt, uploadId)).toBe(204); // liker likes again → 1
expect(await like(author.jwt, uploadId)).toBe(204); // author likes too → 2
expect(await like(liker.jwt, uploadId)).toBe(200); // liker likes again → 1
expect(await like(author.jwt, uploadId)).toBe(200); // author likes too → 2
row = findFeedRow(await api.getFeed(liker.jwt), uploadId);
expect(row.like_count).toBe(2);
});

View File

@@ -61,7 +61,7 @@ test.describe('Host — event lock', () => {
method: 'POST',
headers: { Authorization: `Bearer ${g.jwt}` },
});
expect(likeRes.status).toBe(204);
expect(likeRes.status).toBe(200);
// Comments stay open on a locked event.
const commentRes = await fetch(`${BASE}/api/v1/upload/${id}/comments`, {

View File

@@ -16,13 +16,15 @@ test.describe('Host — moderation API', () => {
expect(row?.uploads_hidden).toBe(true);
});
test('ban without hiding leaves uploads_hidden=false', async ({ api, host, guest }) => {
test('ban always hides — the hide_uploads flag is ignored (USER_JOURNEYS §10.5)', async ({ api, host, guest }) => {
// Ban is now unconditionally a hide: even asking NOT to hide still hides, because a
// banned user's content is "gone" everywhere. The legacy hide_uploads arg is ignored.
const target = await guest('Banned2');
await api.banUser(host.jwt, target.userId, false);
const users = await api.listUsers(host.jwt);
const row = users.find((u: any) => u.id === target.userId);
expect(row?.is_banned).toBe(true);
expect(row?.uploads_hidden).toBe(false);
expect(row?.uploads_hidden).toBe(true);
});
test('banned user cannot call /upload', async ({ api, host, guest }) => {
@@ -102,7 +104,7 @@ test.describe('Host — live role/ban revocation (H1)', () => {
// H1 / USER_JOURNEYS §10: a banned user keeps *read* access but every write is
// rejected with 403. The ban is enforced on write handlers + Require{Host,Admin},
// NOT in the base extractor (which would wrongly block reads too).
test('a banned user keeps read access but is blocked from writes', async ({ api, host, guest }) => {
test('a banned user keeps read access but is blocked from writes', async ({ api, host, guest, db }) => {
const base = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101';
const target = await guest('BannedRW');
const auth = (jwt: string) => ({ Authorization: `Bearer ${jwt}` });
@@ -147,10 +149,11 @@ test.describe('Host — live role/ban revocation (H1)', () => {
});
expect(delComment.status).toBe(403);
// The upload survived every blocked write (still fetchable via the feed).
const feed = await fetch(base + '/api/v1/feed', { headers: auth(host.jwt) });
const body = await feed.json();
const rows = body.uploads ?? body.items ?? body;
expect(Array.isArray(rows) && rows.some((u: any) => u.id === ownUpload)).toBe(true);
// The upload survived every blocked write — the delete was rejected, so the row still
// exists (deleted_at IS NULL). We check the DB directly rather than the feed: ban now
// ALWAYS hides, so a banned user's own upload is (correctly) filtered out of every feed
// even though the row is intact. Read access to *other* content is proven by the 200 on
// /me/context above.
expect(await db.countUploadsForUser(target.userId)).toBe(1);
});
});

View File

@@ -0,0 +1,198 @@
/**
* Re-review regression guard — Chain #2 ("export corruption on reopen→re-release").
*
* Bug that was fixed: `spawn_export_jobs` ran the worker unconditionally and the
* worker wrote a FIXED temp path (`Gallery.zip.tmp` / `viewer_tmp_{event}`).
* The new reopen→re-release flow could therefore spawn a second worker while the
* first was still running — two workers stomping the same temp file → a corrupt
* keepsake ZIP served to guests.
*
* The fix: `claim_job` is an atomic `UPDATE ... WHERE status='pending'`; a worker
* that doesn't win the claim bails without touching the temp file. The re-enqueue
* `ON CONFLICT DO UPDATE ... WHERE status <> 'running'` leaves an in-flight job
* alone. Net: exactly one worker per (event,type).
*
* This test seeds real uploads, releases, then churns reopen→re-release (stressing
* the claim guard), waits for the export to settle, and asserts the produced ZIP is
* INTACT (passes `unzip -t`) with exactly one entry per upload — plus that no
* export_job row is left stuck `running`. A temp-file race would corrupt/truncate
* the archive or drop entries, failing these assertions.
*
* Note on scope: a Dockerised export over small fixtures completes in well under a
* second, so this cannot *deterministically* force two workers to overlap in time.
* It stresses the claim/ON-CONFLICT guard under rapid churn and hard-checks archive
* integrity; the atomic single-worker guarantee itself is additionally proven by
* code review + SQL PREPARE. Together they cover the regression.
*/
import { test, expect } from '../../fixtures/test';
import { Client } from 'pg';
import { execFileSync } from 'node:child_process';
import { writeFileSync, mkdtempSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { seedUpload } from '../../helpers/seed';
const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101';
const SLUG = 'e2e-test-event';
const N_UPLOADS = 6;
const PG = {
host: process.env.E2E_DB_HOST ?? 'localhost',
port: Number(process.env.E2E_DB_PORT ?? '55432'),
user: process.env.E2E_DB_USER ?? 'eventsnap_test',
password: process.env.E2E_DB_PASSWORD ?? 'eventsnap_test',
database: process.env.E2E_DB_NAME ?? 'eventsnap_test',
};
async function pgQuery<T = any>(sql: string): Promise<T[]> {
const c = new Client(PG);
await c.connect();
try {
return (await c.query(sql)).rows as T[];
} finally {
await c.end();
}
}
function post(path: string, jwt: string) {
return fetch(BASE + path, { method: 'POST', headers: { Authorization: `Bearer ${jwt}` } });
}
async function exportStatus(jwt: string): Promise<any> {
const res = await fetch(BASE + '/api/v1/export/status', { headers: { Authorization: `Bearer ${jwt}` } });
return res.json();
}
async function mintTicket(jwt: string): Promise<string> {
const res = await post('/api/v1/export/ticket', jwt);
return (await res.json()).ticket;
}
test.describe('Flow re-review — reopen→re-release export integrity (#2)', () => {
// The real export job runs image processing; give it head-room over the tiny fixtures.
test.setTimeout(90_000);
test('rapid release/reopen/re-release yields exactly one INTACT ZIP and no stuck jobs', async ({
host,
}) => {
// Seed real, decodable uploads while the event is open.
for (let i = 0; i < N_UPLOADS; i++) {
await seedUpload(host.jwt, { caption: `pic ${i}` });
}
// First release → export starts and uploads lock (release ⇒ lock).
expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204);
// Post-release uploads must be rejected (belt-and-suspenders on top of the lock).
const { uploadRaw } = await import('../../helpers/upload-client');
const { readFileSync } = await import('node:fs');
const rejected = await uploadRaw(host.jwt, readFileSync(join(process.cwd(), 'fixtures', 'media', 'sample.jpg')), {
filename: 'late.jpg',
contentType: 'image/jpeg',
});
expect(rejected.status).toBe(403);
// Churn: reopen (clears release + ready flags) then re-release, several times in
// quick succession. This is the path that used to be able to double-spawn workers
// over the shared temp file. End on a release so the gallery is left released.
for (let i = 0; i < 4; i++) {
expect((await post('/api/v1/host/event/open', host.jwt)).status).toBe(204);
const rel = await post('/api/v1/host/gallery/release', host.jwt);
expect(rel.status).toBe(204);
}
// Wait for the export to settle: both jobs done and the ZIP marked ready.
await expect
.poll(async () => {
const s = await exportStatus(host.jwt);
return s.released === true && s.zip?.status === 'done' && s.html?.status === 'done';
}, { timeout: 60_000, intervals: [500] })
.toBe(true);
// No export_job may be left stuck 'running' (would mean a worker that never
// completed — the failure mode the claim guard exists to avoid).
const jobs = await pgQuery<{ type: string; status: string }>(
`SELECT ej.type::text AS type, ej.status::text AS status
FROM export_job ej JOIN event e ON e.id = ej.event_id
WHERE e.slug = '${SLUG}'`
);
expect(jobs.length).toBe(2);
for (const j of jobs) expect(j.status, `${j.type} job status`).toBe('done');
// Download the ZIP and prove it is INTACT — a temp-file race would corrupt or
// truncate it. `unzip -t` verifies every entry's CRC; a non-zero exit throws.
const ticket = await mintTicket(host.jwt);
const res = await fetch(BASE + '/api/v1/export/zip?ticket=' + encodeURIComponent(ticket));
expect(res.status).toBe(200);
const bytes = Buffer.from(await res.arrayBuffer());
// ZIP local-file-header magic — a stomped/half-written file fails this immediately.
expect(bytes.subarray(0, 2).toString('latin1')).toBe('PK');
const dir = mkdtempSync(join(tmpdir(), 'es-zip-'));
const zipPath = join(dir, 'Gallery.zip');
writeFileSync(zipPath, bytes);
// Integrity: `unzip -t` exits 0 only if all CRCs check out and the archive is whole.
execFileSync('unzip', ['-t', zipPath], { stdio: 'pipe' });
// Completeness: exactly one entry per seeded upload — no entry lost to a stomped
// temp file, none duplicated by a second worker.
const listing = execFileSync('unzip', ['-Z1', zipPath], { encoding: 'utf8' })
.split('\n')
.map((l) => l.trim())
.filter((l) => l.length > 0 && !l.endsWith('/'));
expect(listing.length, `zip entries:\n${listing.join('\n')}`).toBe(N_UPLOADS);
});
test('re-release does NOT disturb an in-flight (running) export job — the anti-race guard', async ({
host,
}) => {
// Deterministic proof of the fix, independent of export timing. We simulate a worker
// that is still mid-export by pinning the zip job to `running` with a sentinel
// progress value, then drive reopen→re-release. The fix's
// ON CONFLICT ... DO UPDATE ... WHERE export_job.status <> 'running'
// must leave that row untouched (and the freshly-spawned worker's
// claim_job: UPDATE ... WHERE status = 'pending'
// finds nothing to claim, so it bails without racing the temp file).
//
// On the OLD code (unconditional ON CONFLICT DO UPDATE) the row would be reset to
// pending/progress=0 and a second worker would claim and run concurrently — so the
// sentinel below would be wiped. This assertion therefore fails on the bug and passes
// on the fix.
await seedUpload(host.jwt, { caption: 'a' });
await seedUpload(host.jwt, { caption: 'b' });
// Release and let the first export fully finish so no real worker is active.
expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204);
await expect
.poll(async () => {
const s = await exportStatus(host.jwt);
return s.zip?.status === 'done' && s.html?.status === 'done';
}, { timeout: 60_000, intervals: [500] })
.toBe(true);
// Simulate an in-flight worker owning the zip job, with a sentinel we can check.
await pgQuery(
`UPDATE export_job ej SET status = 'running', progress_pct = 77
FROM event e
WHERE e.id = ej.event_id AND e.slug = '${SLUG}' AND ej.type = 'zip'`
);
// Reopen (clears release + ready flags, does NOT touch job rows) then re-release.
expect((await post('/api/v1/host/event/open', host.jwt)).status).toBe(204);
expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204);
// Give any spawned worker a beat to attempt (and, correctly, bail) its claim.
await new Promise((r) => setTimeout(r, 750));
// The guard must have preserved the 'running' sentinel — untouched by both the
// re-enqueue and the bailing worker.
const [row] = await pgQuery<{ status: string; progress_pct: number }>(
`SELECT ej.status::text AS status, ej.progress_pct
FROM export_job ej JOIN event e ON e.id = ej.event_id
WHERE e.slug = '${SLUG}' AND ej.type = 'zip'`
);
expect(row.status, 'zip job status').toBe('running');
expect(Number(row.progress_pct), 'zip job sentinel progress').toBe(77);
});
});

View File

@@ -0,0 +1,100 @@
/**
* Re-review regression guard — Chain B1 ("offline queue must auto-resume").
*
* Bug that was fixed: a file staged while OFFLINE was attempted immediately;
* the XHR network error marked the queue item `error`, and every resume path
* (the `online` listener, `processQueue`, `loadQueue`) only picks up `pending`
* items — so the item was stranded until a manual "Erneut" tap.
*
* The fix: network failures are a distinct `NetworkError` that keeps the item
* `pending`; `processQueue` bails while `navigator.onLine === false` (so an
* offline-staged item is never burned to `error`); and `requeueRetriable()`
* flips transient `error`→`pending` on the `online` event and on mount.
*
* This test drives a REAL browser going offline → stage+submit → reconnect and
* asserts the upload lands with NO manual interaction. It fails on the old code
* (item errors, never resumes) and passes on the fix.
*/
import { test, expect } from '../../fixtures/test';
import { FeedPage, UploadSheet } from '../../page-objects';
import { join } from 'node:path';
const SAMPLE_JPG = join(process.cwd(), 'fixtures', 'media', 'sample.jpg');
// White-box: read the upload queue straight out of IndexedDB so we can assert
// the item's status without depending on the (currently unrendered) queue widget.
async function queueStatuses(page: import('@playwright/test').Page): Promise<string[]> {
return page.evaluate(async () => {
const statuses = await new Promise<string[]>((resolve, reject) => {
const req = indexedDB.open('eventsnap-uploads', 3);
req.onerror = () => reject(req.error);
req.onsuccess = () => {
const dbh = req.result;
const tx = dbh.transaction('queue', 'readonly');
const all = tx.objectStore('queue').getAll();
all.onsuccess = () => resolve(all.result.map((r: any) => r.status));
all.onerror = () => reject(all.error);
};
});
return statuses;
});
}
test.describe('Flow re-review — offline upload auto-resume (B1)', () => {
test('offline-staged upload stays pending, then flushes on reconnect with no manual tap', async ({
page,
guest,
signIn,
db,
}) => {
const g = await guest('OfflineResume');
await signIn(page, g);
await page.goto('/feed');
expect(await db.countUploadsForUser(g.userId)).toBe(0);
// Warm the code-split `/upload` route chunk while still ONLINE. Without this, the
// client-side navigation the UploadSheet triggers would try to fetch the chunk with
// no connectivity and hit SvelteKit's error boundary — a test artifact unrelated to
// the upload-queue fix under test. (A production PWA service worker would cache it.)
await page.goto('/upload');
await page.goto('/feed');
// Go offline BEFORE staging so the first attempt happens with no connectivity.
await page.context().setOffline(true);
const feed = new FeedPage(page);
const sheet = new UploadSheet(page);
await feed.openUploadSheet();
await sheet.stageFiles([SAMPLE_JPG]);
await sheet.captionInput.waitFor({ state: 'visible', timeout: 10_000 });
await sheet.fillCaption('shot with no signal');
await sheet.submit();
// handleSubmit navigates to /feed after enqueuing even while offline.
await page.waitForURL('**/feed', { timeout: 15_000 });
// While offline: nothing may have reached the server, and — the crux of the
// fix — the item must be parked as `pending`, NOT burned to `error`/`blocked`
// (which on the old code would require a manual retry tap).
await expect
.poll(() => queueStatuses(page), { timeout: 8_000 })
.toEqual(['pending']);
expect(await db.countUploadsForUser(g.userId)).toBe(0);
// Reconnect. The `online` listener must resume the queue automatically — we do
// NOT navigate, tap retry, or otherwise touch the page.
await page.context().setOffline(false);
// The queued upload is now created server-side without any manual interaction.
await expect
.poll(() => db.countUploadsForUser(g.userId), { timeout: 20_000 })
.toBe(1);
// And the queue item ends terminal-good (done) or is drained — never stuck in error.
await expect
.poll(async () => {
const s = await queueStatuses(page);
return s.every((x) => x === 'done') || s.length === 0;
}, { timeout: 10_000 })
.toBe(true);
});
});