Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ See `docs/architecture.md` for the full system architecture.
| TypeScript build | `pnpm build` | All 3 packages must compile cleanly |
| Unit tests | `pnpm test` | Vitest across shared, api, web (518+ tests) |
| Integration tests | `pnpm --filter api exec vitest run --config vitest.integration.config.ts` | 69 tests, requires the test Postgres container (`pnpm test:db:up`) |
| E2E tests | `pnpm test:e2e` | 195 tests, Playwright — full runs are sharded across 4 isolated stacks (see `docs/e2e-sharding.md`); `pnpm test:e2e e2e/foo.spec.ts` runs a single spec unsharded |
| E2E tests | `pnpm test:e2e` | 195 tests, Playwright — full runs are sharded across up to 4 isolated stacks, scaled to the machine (see `docs/e2e-sharding.md`); `pnpm test:e2e e2e/foo.spec.ts` runs a single spec unsharded |
| E2E tests (serial) | `pnpm test:e2e:serial` | Escape hatch: plain single-stack `playwright test` |
| Test coverage | `pnpm test:coverage` | V8 coverage reports (HTML + LCOV) |
| Lint | `pnpm lint` | ESLint |
Expand Down Expand Up @@ -90,7 +90,7 @@ This is a TDD-first project. All new features and bug fixes must be developed te

- **Unit tests**: `packages/*/src/__tests__/*.test.ts` — Vitest, jsdom (web) / node (api). Fast, deterministic, no database.
- **Integration tests**: `packages/api/src/__tests__/integration/*.test.ts` — Vitest against the dedicated `postgres-oxygen-test` container on `:5433`. The harness (`helpers/load-env.ts`) refuses to run if `DATABASE_URL` resolves to port 5432 (dev DB) — set `INTEGRATION_DATABASE_URL` to override. Per-suite isolation comes from giving each suite its own `Event` row and relying on `ON DELETE CASCADE`.
- **E2E tests**: `e2e/*.spec.ts` — Playwright, Chromium, single worker, sequential within a stack. Full user flows through the browser. Full runs are parallelized by `scripts/e2e-sharded.mjs`, which launches 4 isolated stacks (own DB `oxygen_e2e_<n>`, own API, own Vite server) and splits the spec files across them — serial semantics are preserved inside each shard. See `docs/e2e-sharding.md`.
- **E2E tests**: `e2e/*.spec.ts` — Playwright, Chromium, single worker, sequential within a stack. Full user flows through the browser. Full runs are parallelized by `scripts/e2e-sharded.mjs`, which launches up to 4 isolated stacks — `min(4, cores/2)` — (own DB `oxygen_e2e_<n>`, own API, own Vite server) and splits the spec files across them — serial semantics are preserved inside each shard. See `docs/e2e-sharding.md`.

## 5. Flaky Test Policy

Expand All @@ -108,7 +108,7 @@ in your final message and explain why.
1. **`pnpm build`** — Zero TypeScript errors across all three packages.
2. **`pnpm test`** — All unit tests pass. Always required.
3. **Integration tests** — Run for any DB-related changes. Always required for features: `pnpm --filter api exec vitest run --config vitest.integration.config.ts`
4. **E2E tests** — During iterative development, run only the spec files covering the affected area (`pnpm test:e2e e2e/specific-file.spec.ts` — runs unsharded against the default stack). Before declaring a task complete, run the full suite once: `pnpm test:e2e` (sharded across 4 isolated stacks, ~2-3 min; see `docs/e2e-sharding.md`). For minor fixes confined to `docs/` or other non-shipping files, E2E can be skipped — state so in your final message.
4. **E2E tests** — During iterative development, run only the spec files covering the affected area (`pnpm test:e2e e2e/specific-file.spec.ts` — runs unsharded against the default stack). Before declaring a task complete, run the full suite once: `pnpm test:e2e` (sharded across up to 4 isolated stacks, ~3-5 min; see `docs/e2e-sharding.md`). For minor fixes confined to `docs/` or other non-shipping files, E2E can be skipped — state so in your final message.
5. **Rebuild Docker** — Run `docker compose -f docker-compose.host-db.yml up --build -d` so the running stack reflects the latest code. **Required for every change that touches `packages/api/`, `packages/web/`, `packages/shared/`, `docker/`, any `Dockerfile`, `docker-compose*.yml`, or `pnpm-lock.yaml`.** You may skip it only for changes confined to `docs/`, `AGENTS.md`, `.claude/`, or test fixtures that don't ship in either image — and when you skip it, state so in your final message. Verify the output ends with both `Image oxygen-api Built` / `Image oxygen-web Built` and both containers `Started`; treat anything else as a failure.
6. **Major-version drift report** — After all other steps pass, run `pnpm outdated -r --long` and list any **direct** dependencies (production or dev) with a major-version update available. Format each as `package: current → latest — one-line note on what changes / "no notable changes documented"`. Informational only; do not bump majors as part of an unrelated PR. The user decides whether to act.

Expand Down
16 changes: 14 additions & 2 deletions docs/e2e-sharding.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ semantics the suite has always had.
```
pnpm test:e2e
│
└── scripts/e2e-sharded.mjs (default N=4, override with E2E_SHARDS)
└── scripts/e2e-sharded.mjs (N = min(4, cores/2) by default, override with E2E_SHARDS)
├── shard 1: playwright test <files> → vite :4201 → api :4101 → db oxygen_e2e_1 (+ eventor stub :4301)
├── shard 2: playwright test <files> → vite :4202 → api :4102 → db oxygen_e2e_2 (+ eventor stub :4302)
├── shard 3: playwright test <files> → vite :4203 → api :4103 → db oxygen_e2e_3 (+ eventor stub :4303)
Expand All @@ -30,7 +30,7 @@ reference events — nothing to provision manually.

| Command | What happens |
|---------|--------------|
| `pnpm test:e2e` | Full suite, sharded across 4 stacks (~2-3 min) |
| `pnpm test:e2e` | Full suite, sharded across `min(4, floor(cores/2))` stacks — 4 on a 16-core dev box (~3-5 min), 2 on a 4 vCPU GitHub runner |
| `pnpm test:e2e e2e/kiosk.spec.ts` | Selective run — single plain Playwright process on its own isolated stack (ports 4100/4200, db `oxygen_e2e`), no sharding. Isolated ports mean it works while `pnpm dev` is running |
| `E2E_SHARDS=2 pnpm test:e2e` | Fewer shards (lower peak CPU/RAM) |
| `pnpm test:e2e:serial` | Escape hatch: plain `playwright test`, identical to the pre-sharding behavior |
Expand Down Expand Up @@ -117,3 +117,15 @@ deliberately avoid the dev servers (3002/5173) and the Docker stack
- **Port already in use** — a previous run crashed without cleanup; kill
leftover `tsx`/`vite` processes bound to 41xx/42xx ports.
- **Machine too loaded** — `E2E_SHARDS=2 pnpm test:e2e`.
- **Timeouts only in CI** — each shard is a whole stack (API + Vite +
eventor stub + Chromium) and wants about two cores. Four shards on a
4 vCPU runner ran every shard at half speed (6-10 min instead of 3-5)
and pushed tests with 5 s `expect` budgets over the edge; the default
now scales with `os.availableParallelism()`. Tests that are
legitimately long (many round trips, tile rendering in the background)
should say so with `test.slow()` rather than the whole suite growing
its timeouts. Tests that create named records must also be
**retry-safe**: Playwright retries twice in CI, and a retry that finds
the previous attempt's runner / card in place fails for a different
reason than the first attempt did (`phase2.spec.ts` derives its names
and card number from `testInfo.retry`).
2 changes: 1 addition & 1 deletion docs/map-tile-rendering.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ map bug: unrelated queries time out while someone pans a map.
| `map_tiles` table | Shared | The real cache. Written with `ON CONFLICT DO NOTHING`, so concurrent renders of the same block across instances are harmless. |
| SVG cache | Per process | Parsed map SVGs, `MAP_SVG_CACHE_EVENTS` of them. Amortises the ~70 ms parse and the OCAD read. |
| In-flight block map | Per process | A viewport fetches ~20 tiles at once; they collapse onto one render per block. |
| Render semaphore | Per process | Bounds concurrent block renders (`MAP_RENDER_CONCURRENCY`). One permit covers a block's composite **and** ink rasters, run side by side. Foreground requests are served before pre-cache work, so background rendering never queues a user behind a whole sweep. |
| Render semaphore | Per process | `renderGate()` in `map-render-limits.ts` bounds concurrent rasterisations (`MAP_RENDER_CONCURRENCY`). One permit covers a block's composite **and** ink rasters, run side by side. The course-map layout preview (`/api/maps/:nameId/window.png`) takes permits from the same pool. Foreground requests are served before pre-cache work, so background rendering never queues a user behind a whole sweep. |
| Queue bound | Per process | At most `MAP_RENDER_MAX_QUEUE` foreground blocks may wait for a permit. Beyond that the tile route answers **503 + `Retry-After: 5`** immediately instead of letting the request sit until the platform kills it. |

Everything above the database is a per-process optimisation, and losing
Expand Down
69 changes: 69 additions & 0 deletions docs/perf-polling-and-auth-round-trips.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# Performance: polling endpoints and per-request auth round trips

**Date:** 2026-09-24
**Context:** follow-up sweep after
[bugfix-map-tile-cold-render-cloud-timeouts.md](bugfix-map-tile-cold-render-cloud-timeouts.md).
Cloud Run request logs since 2026-09-10 (requests ≥ 0.5 s, tiles
excluded) were grouped by endpoint; the recurring entries were all
polls or per-request plumbing, not user actions.

The scarce resource on the cloud deployment is Cloud SQL
(`db-f1-micro`, ~22 usable connections, non-trivial per-query latency
through the connector). Everything below trades sequential round trips
for single statements or for not asking at all.

## What changed

| Endpoint / path | Before | After |
|---|---|---|
| `competition.counterState` (every 5 s per tab) | 9 sequential queries — one `MAX(updated_at)` per table in a loop, plus punches, events and a Prisma aggregate | 1 statement with scalar subselects |
| `competition.dbStatus` (every 3 s per tab, public) | 3 queries including `pg_database_size()` each time | 1 statement; database size memoised for 60 s (`ttl-memo.ts`) |
| `assertRestAccess` + route handler | Guard resolved the event, handler resolved it again; `countFinishedRunners` on every authenticated request | Guard returns the `EventRef`; handlers reuse it. The finished-runner count runs only when it can change the capability set (`finishedCountMatters`) |
| `/api/maps/:nameId/window.png` | Full print-window rasterisation outside the render semaphore | Shares `renderGate()` with tiles; refused with `503 Retry-After` when the queue is full |
| `course.controlCompletionStatus` (every 15 s) | Loaded every classed runner and every card's full `punches_raw` for the event | Scoped to the requested course's runners and to the cards those runners carry; one regex per control instead of one per runner per code |

### Why the finished-runner count is usually redundant

`resolveEventCapabilities` adds `event.view` / `results.view` /
`courses.view` for every signed-in user once the event is *completed*,
and completion is `date < today || finishedRunners > 0`. The count is
therefore irrelevant when the date is already in the past, or when the
user's grants already include all three capabilities — which is every
club member with a view role. Only a user without those grants, on a
current or future event, still triggers the count. Semantics are
unchanged; `permissions.test.ts` covers the decision table.

### REST guard contract

`assertRestAccess()` now returns `EventRef | null` instead of `boolean`.
Existing `if (!(await assertRestAccess(...)))` call sites keep working;
new handlers should take the returned event rather than looking the slug
up again. Pass `event:` when the handler had to resolve it first (the
tile-progress poll does, because an unknown event is not an error
there).

## Verification

- `permissions.test.ts` — `finishedCountMatters` decision table.
- `ttl-memo.test.ts` — TTL, shared in-flight computation, failures not cached.
- `integration/event.test.ts` — `counterState` returns all nine legacy
keys; `oClub` ignores removed runners while `oRunner` does not.
- `integration/control-completion.test.ts` — course-scoped vs. aggregate
counting, direct course assignment overriding the class course,
alternate punch codes, removed / unclassed runners, cards nobody
carries. The same suite passes against the previous implementation, so
the narrowing changed nothing observable.
- `integration/map-tiles.test.ts`, `integration/course-maps.test.ts` —
routes through the changed guard (404 / 200 / 503 paths).

## Not changed, and why

- `/api/version`, `/sw.js`, first `users.me` at 4–12 s: instance cold
start (Node + Prisma + Cloud SQL connector handshake). Only
`--min-instances=1` removes it.
- `lease.status` every 10 s: two indexed reads; its slow tail tracks
instance saturation, not the query.
- `identityFromRequest` still looks the user up per request. Caching
identity is an auth decision, not a performance one.
- `maps.pdf` still fetches the full event row (needs `name` /
`organizerName`); it is a one-off export.
7 changes: 7 additions & 0 deletions e2e/course-editor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,10 @@ test.describe("Course editor", () => {
});

test("build a course by clicking, reorder, undo/redo, persist", async ({ page }) => {
// Many round trips (place, reorder, undo, redo, reload) plus tile
// rendering in the background: legitimately long, and it runs into
// the 30 s test timeout on a loaded CI runner.
test.slow();
await selectCompetition(page);
await ensureCoursesAndMap(page);
await openEditor(page);
Expand Down Expand Up @@ -763,6 +767,9 @@ test.describe("Course editor", () => {
});

test("suggests a description from the base map for a placed control", async ({ page }) => {
// Waits on the base-map object lookup after placing a control; slow
// under CI load.
test.slow();
page.on("dialog", (dialog) => dialog.accept());

await selectCompetition(page);
Expand Down
7 changes: 5 additions & 2 deletions e2e/online-input-config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,17 @@ test.describe("Online Input panel", () => {
await page.getByTestId("online-input-new-target").selectOption("2"); // PunchFinish
await page.getByTestId("online-input-add-mapping").click();

// The row appears after the mutation round trip *and* the config
// refetch it triggers; under a full sharded run that can exceed the
// default 5 s expect timeout.
const mapping = page.getByTestId("online-input-mapping-100");
await expect(mapping).toBeVisible();
await expect(mapping).toBeVisible({ timeout: 15000 });
await expect(mapping).toContainText("100");
await expect(mapping).toContainText("Finish");

// Remove it again
await mapping.getByRole("button", { name: "Remove mapping" }).click();
await expect(mapping).toHaveCount(0);
await expect(mapping).toHaveCount(0, { timeout: 15000 });
});

test("Reset button on lastId becomes enabled after a poll has advanced it", async ({
Expand Down
32 changes: 22 additions & 10 deletions e2e/phase2.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,16 @@ test.describe("Runner Management", () => {
await expect(page.locator("span", { hasText: "runners" })).toBeVisible();
});

test("should create, edit, and delete a runner", async ({ page }) => {
test("should create, edit, and delete a runner", async ({ page }, testInfo) => {
// Unique per attempt: a retry after a slow first attempt must not
// find the previous attempt's runner (the dialog would then show the
// duplicate-card warning and never close — the deterministic CI
// failure of September 2026).
const tag = `${testInfo.retry}-${Date.now().toString(36).slice(-4)}`;
const createdName = `Test Runner E2E ${tag}`;
const updatedName = `Test Runner Updated ${tag}`;
const cardNo = String(990000 + (Date.now() % 9000) + testInfo.retry);

await goToTab(page, "Runners");
await expect(page.locator("span", { hasText: "runners" })).toBeVisible({ timeout: 10000 });

Expand All @@ -80,33 +89,36 @@ test.describe("Runner Management", () => {
).toBeVisible({ timeout: 3000 });

const dialog = page.getByTestId("registration-dialog");
await dialog.locator("input[placeholder='First Last']").fill("Test Runner E2E");
await dialog.locator("input[placeholder='First Last']").fill(createdName);
await dialog.getByTestId("reg-class").click();
await expect(dialog.getByText("Öppen 1", { exact: true })).toBeVisible({ timeout: 3000 });
await dialog.getByText("Öppen 1", { exact: true }).click();
await dialog.locator("input[placeholder='e.g. 500123']").fill("999999");
await dialog.locator("input[placeholder='e.g. 500123']").fill(cardNo);
await dialog.getByTestId("reg-submit").click();

await expect(page.getByText("Test Runner E2E")).toBeVisible({ timeout: 5000 });
// Dialog closes, then the list refetches — two round trips on a
// loaded CI runner.
await expect(dialog).not.toBeVisible({ timeout: 15000 });
await expect(page.getByText(createdName)).toBeVisible({ timeout: 15000 });

// EDIT (inline autosave)
await page.getByRole("cell", { name: "Test Runner E2E" }).click();
await page.getByRole("cell", { name: createdName }).click();
const expandedPanel = page.locator(".bg-blue-50\\/60");
await expect(expandedPanel).toBeVisible({ timeout: 3000 });

const nameInput = expandedPanel.locator("input").first();
await nameInput.clear();
await nameInput.fill("Test Runner Updated");
await expect(page.getByText("Saved")).toBeVisible({ timeout: 3000 });
await nameInput.fill(updatedName);
await expect(page.getByText("Saved")).toBeVisible({ timeout: 10000 });

await page.getByRole("cell", { name: "Test Runner Updated", exact: true }).first().click();
await page.getByRole("cell", { name: updatedName, exact: true }).first().click();
await expect(expandedPanel).not.toBeVisible({ timeout: 3000 });

// DELETE
page.on("dialog", (dialog) => dialog.accept());
const updatedRow = page.locator("tr").filter({ hasText: "Test Runner Updated" }).first();
const updatedRow = page.locator("tr").filter({ hasText: updatedName }).first();
await updatedRow.getByTitle("Remove runner").click();
await expect(page.getByText("Test Runner Updated")).not.toBeVisible({ timeout: 5000 });
await expect(page.getByText(updatedName)).not.toBeVisible({ timeout: 10000 });
await expect(page.locator("span", { hasText: "runners" })).toBeVisible();
});

Expand Down
Loading
Loading