feat: #76 myCrate query with keyset paging

.github/workflows/ci.yml

@@ -76,9 +76,101 @@ jobs:
             exit 1
           fi
 
+  # The database-backed suites are skipped when TEST_DATABASE_URL is unset, so
+  # until this job existed CI reported them as "27 skipped" and stayed green.
+  # The tests guard the things unit tests cannot see: SQL that only fails once
+  # PostgreSQL parses it, keyset paging across ties and NULLs, facet counts
+  # against the filters, and the MusicBrainz import preserving reviews.
+  #
+  # The cluster is created here and thrown away at the end. Deliberately not the
+  # application database and not a shared test database: no secret to hand out,
+  # no state to drift, and running the destructive seed is impossible by
+  # construction. It listens on a unix socket only, so it cannot accept a TCP
+  # connection even while it exists.
+  integration:
+    name: PostgreSQL integration tests
+    runs-on: [self-hosted, linux, x64]
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Install pnpm
+        uses: pnpm/action-setup@v4
+        with:
+          version: 10.33.0
+
+      - name: Set up Node.js
+        uses: actions/setup-node@v4
+        with:
+          node-version: 24
+          cache: pnpm
+
+      - name: Install dependencies
+        run: pnpm install --frozen-lockfile
+
+      - name: Start a throwaway PostgreSQL cluster
+        run: |
+          set -euo pipefail
+          # Debian installs the server binaries under /usr/lib/postgresql/<major>/bin
+          # and only puts the client on PATH, so `initdb` is not found even with
+          # PostgreSQL 18 installed. Use PATH when it works, otherwise the newest
+          # installed major version.
+          if ! command -v initdb >/dev/null 2>&1; then
+            pg_bindir=''
+            for dir in /usr/lib/postgresql/*/bin; do
+              if [ -x "${dir}/initdb" ]; then
+                pg_bindir="${dir}"
+              fi
+            done
+            if [ -z "${pg_bindir}" ]; then
+              echo "::error::initdb was not found. Install a PostgreSQL server package on the runner."
+              exit 1
+            fi
+            export PATH="${pg_bindir}:${PATH}"
+          fi
+          initdb --version
+          export PGDATA="${RUNNER_TEMP}/pgdata"
+          rm -rf "${PGDATA}"
+          initdb -D "${PGDATA}" --auth=trust --no-sync
+          # -k puts the socket in the data directory and `listen_addresses=''`
+          # keeps it off the network entirely.
+          pg_ctl -D "${PGDATA}" -l "${PGDATA}/server.log" \
+            -o "-c listen_addresses='' -k ${PGDATA}" start
+          createdb -h "${PGDATA}" crate_test
+          echo "PGDATA=${PGDATA}" >> "${GITHUB_ENV}"
+          echo "TEST_DATABASE_URL=postgres:///crate_test?host=${PGDATA}" >> "${GITHUB_ENV}"
+          psql -h "${PGDATA}" -d crate_test -tAc 'select version()'
+
+      - name: Apply migrations
+        run: DATABASE_URL="$TEST_DATABASE_URL" pnpm --filter @t19/server db:migrate
+
+      - name: Run the server test suite
+        # The JSON report is only there for the next step; the default reporter
+        # still prints the normal output a reader expects.
+        run: |
+          pnpm --filter @t19/server exec vitest run \
+            --reporter=default \
+            --reporter=json \
+            --outputFile="${RUNNER_TEMP}/vitest.json"
+
+      # A green run is not evidence on its own: without a database every
+      # PostgreSQL suite skips and vitest still exits 0. See the script header.
+      - name: Fail if the PostgreSQL tests were skipped
+        run: node server/scripts/assert-postgres-tests.mjs "${RUNNER_TEMP}/vitest.json"
+
+      - name: Remove the cluster
+        if: always()
+        run: |
+          if [ -f "${PGDATA}/postmaster.pid" ]; then
+            pg_ctl -D "${PGDATA}" -m immediate stop || true
+          fi
+          rm -rf "${PGDATA}"
+
   deploy:
     name: Deploy to the group VM
-    needs: quality
+    # Both jobs have to pass. Deploying with the database layer untested is the
+    # same gap as deploying a server that was never built (#44).
+    needs: [quality, integration]
     if: github.ref == 'refs/heads/main' && github.event_name == 'push'
     # Pinned to the `project2` label on purpose. This job writes to
     # /var/www/html/project2 and restarts crate-api, both of which only exist

README.md

@@ -83,8 +83,21 @@ CI also builds the project and checks the client bundle size.
 | `pnpm db:fetch:musicbrainz`            | Rebuild the local MusicBrainz snapshot (network required) |
 | `pnpm db:seed:large`                   | Reset to a large synthetic catalog                        |
 
-Tests cover frontend components and backend logic. Additional PostgreSQL tests cover search, sorting, rating updates and filters. These run when `TEST_DATABASE_URL` is configured and require a separate test database with migrations applied. The
-[VM setup guide](docs/VM-setup.md) includes API smoke tests.
+Tests cover frontend components and backend logic. The database-backed suites
+(search, sorting, filtering and facet counts, and the MusicBrainz import) need a
+real PostgreSQL server and are skipped unless `TEST_DATABASE_URL` is set.
+
+CI starts a temporary PostgreSQL cluster for them, applies the migrations, runs
+the suites and fails if any of them was skipped instead of executed
+(`server/scripts/assert-postgres-tests.mjs`). To run them locally, point
+`TEST_DATABASE_URL` at a database you do not mind losing - the tests insert and
+delete fixture rows:
+
+```bash
+TEST_DATABASE_URL=postgres://t19:password@127.0.0.1:5432/crate_test pnpm --filter @t19/server test
+```
+
+The [VM setup guide](docs/VM-setup.md) includes API smoke tests.
 
 ## Database seed data
 

docs/VM-setup.md

@@ -41,6 +41,12 @@ migrations, imports the committed MusicBrainz catalog snapshot and restarts
 `crate-api.service`. The import is repeatable and does not reset the database or
 delete user data. Deployment does not run either destructive seed command.
 
+The `integration` job runs the database-backed test suites. It starts a
+temporary PostgreSQL cluster in the runner's temp directory, listening on a unix
+socket only, applies the migrations to it, runs the tests and deletes it
+afterwards. It never connects to the `crate` database, so the names, fixtures
+and cleanup in those tests cannot affect the application data.
+
 Apache serves the frontend at `/project2/` and forwards `/graphql` requests to
 the API on port 3001:
 

docs/architecture.md

@@ -96,11 +96,10 @@ brief explicitly wants simulated users rather than login. Not an
 `Authorization` header either, because nothing here is authenticated and
 calling it that would be misleading.
 
-It goes on **every** request, not only mutations: `myReview` and `isInCrate`
-are query fields. The same transport is intended for `myCrate`, which is declared
-in the schema but has no resolver yet.
+It goes on **every** request, not only mutations: `myReview`, `isInCrate` and
+`myCrate` are query fields.
 
-**Missing or invalid user ID.** If the `X-User-Id` header is missing or invalid, the application treats the user as anonymous. Their own review is returned as `null`, and albums are treated as not being in their crate. Adding albums or saving reviews requires a valid user ID.
+**Missing or invalid user ID.** If the `X-User-Id` header is missing or invalid, the application treats the user as anonymous. Their own review is returned as `null`, albums are treated as not being in their crate, and `myCrate` returns an empty connection rather than an error: the field is non-null, and a read should degrade instead of failing. Adding albums or saving reviews requires a valid user ID.
 
 Reads never fail because of identity state, so a bad header cannot take the
 search page down. Writes require it and say so rather than attributing the

server/scripts/assert-postgres-tests.mjs

@@ -0,0 +1,86 @@
+// Fails when the PostgreSQL test files were skipped instead of executed.
+//
+// Every PostgreSQL suite is wrapped in `describe.skipIf(!process.env.TEST_DATABASE_URL)`,
+// so a CI job that forgets the variable still exits 0 with the whole database
+// layer missing. `pnpm test` would report "61 passed | 27 skipped" and nothing
+// would be wrong by its own account. This script reads the JSON report from
+// that run and requires every database-backed file to have executed, which is
+// the difference between "the tests pass" and "the tests ran".
+//
+// Usage: node server/scripts/assert-postgres-tests.mjs <vitest-report.json>
+
+import { readFileSync } from 'node:fs';
+
+// Files that cannot run without a database. Adding a new one here is deliberate:
+// the list is the guard, so it should grow with the suite.
+const databaseBackedFiles = [
+  'src/crate.postgres.test.ts',
+  'src/db/musicBrainzCatalog.test.ts',
+  'src/search/facets.postgres.test.ts',
+  'src/search/search.postgres.test.ts',
+  'src/search/sort.postgres.test.ts',
+];
+
+const reportPath = process.argv[2];
+if (!reportPath) {
+  console.error('::error::Usage: node assert-postgres-tests.mjs <vitest-report.json>');
+  process.exit(1);
+}
+
+let report;
+try {
+  report = JSON.parse(readFileSync(reportPath, 'utf8'));
+} catch {
+  console.error(`::error::Could not read the vitest report at ${reportPath}`);
+  process.exit(1);
+}
+
+// The reporter names files by absolute path. Keep only the part after `server/`.
+function relativeName(name) {
+  const normalized = name.replaceAll('\\', '/');
+  const marker = '/server/';
+  const index = normalized.lastIndexOf(marker);
+  return index === -1 ? normalized : normalized.slice(index + marker.length);
+}
+
+const resultsByFile = new Map(
+  (report.testResults ?? []).map((file) => [relativeName(file.name), file]),
+);
+
+let failed = false;
+for (const file of databaseBackedFiles) {
+  const result = resultsByFile.get(file);
+
+  if (!result) {
+    console.error(`::error::${file} did not run at all`);
+    failed = true;
+    continue;
+  }
+
+  // A skipped suite still produces a "passed" file containing only pending
+  // assertions, so count what actually ran rather than trusting the file
+  // status. A file with some tests running and some skipped also fails, which
+  // catches `describe.skipIf` on one block of a mixed file.
+  const assertions = result.assertionResults ?? [];
+  const executed = assertions.filter((assertion) => assertion.status === 'passed').length;
+  const pending = assertions.filter((assertion) => assertion.status === 'pending').length;
+
+  if (result.status !== 'passed' || executed === 0 || pending > 0) {
+    console.error(
+      `::error::${file} did not fully run (${result.status}, ${executed} passed, ${pending} skipped) - is TEST_DATABASE_URL set?`,
+    );
+    failed = true;
+    continue;
+  }
+
+  console.log(`${file} ${executed} passed`);
+}
+
+if (failed) {
+  console.error(
+    '::error::The PostgreSQL tests did not run. Point TEST_DATABASE_URL at a migrated test database.',
+  );
+  process.exit(1);
+}
+
+console.log(`All ${databaseBackedFiles.length} database-backed test files executed.`);