Fix R01-N22: move migrations to deploy time, refuse-to-serve safety net

Migrations now run from the Docker entrypoint
(bin/docker-entrypoint.sh → php bin/migrate.php) before Apache binds the
port, so a failure aborts container start instead of leaking through to
the request path mid-DDL. The web request path only calls
Migrator::pendingFiles() and emits 503 + Retry-After when something is
out of date — it no longer applies SQL itself.

- src/Db/Migrator.php: add pendingFiles() (read-only diff between disk
  and schema_version); migrate() unchanged for the CLI's apply path.
- bin/migrate.php: PHP CLI runner used by the entrypoint and by
  operators on bare-metal deploys. Exit 0 on no-op or success, 1 on
  failure with the message on stderr.
- bin/docker-entrypoint.sh: runs the CLI then exec apache2-foreground.
  set -euo pipefail so a migration failure surfaces in `docker logs`.
- Dockerfile: install the entrypoint to /usr/local/bin and switch to
  ENTRYPOINT + CMD so docker compose picks it up unchanged.
- public/index.php: replace the migrate() call with a pendingFiles()
  check; 503 + structured error_log when pending. Bootstrap-phase
  failures still return 500 as before.
- tests/Db/MigratorTest.php (6 cases) pins pendingFiles() against the
  real migrations/ folder, the virgin-DB path, the post-migrate empty
  state, the ignore-non-NNN_*.sql rule, and the schema_version creation
  side-effect on first call.
- README.md / SPEC.md / doc/admin-manual.md §4.1, §5.4, §5.5 updated to
  document the new bootstrap, the bare-metal CLI invocation, and the
  503 behaviour. Drift fence kept narrow (no behavioural change in the
  CLI's output beyond the new "schema already current" line).

Tests: 311 / 823 (was 305 / 814).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Dockerfile

@@ -70,5 +70,12 @@ RUN mkdir -p /var/www/data /var/www/data/sessions /var/www/html/data/twig-cache
         '</VirtualHost>' \
         > /etc/apache2/sites-available/000-default.conf
 
+# R01-N22: run migrations at container start, before Apache binds the port.
+# The request path no longer auto-migrates — it only checks and refuses to
+# serve when something is pending — so a missed entrypoint produces a loud
+# 503, not silent stale-schema serving.
+RUN install -m 0755 /var/www/html/bin/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh
+
 EXPOSE 80
+ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"]
 CMD ["apache2-foreground"]

README.md

@@ -66,10 +66,14 @@ of them set the OIDC path will not auto-promote anyone (this closes
 finding R01-N03 in `doc/REVIEW_01.md`). Subsequent admin promotions
 happen through the **Users** page in the hamburger menu.
 
-The SQLite database is created on first request at `./data/app.sqlite` on
-the host (mounted into the container at `/var/www/data/app.sqlite`).
-Migrations run automatically on container start. Wiping `./data/` resets
-the application to a blank slate.
+The SQLite database is created at `./data/app.sqlite` on the host
+(mounted into the container at `/var/www/data/app.sqlite`). Migrations
+run from the Docker entrypoint (`bin/docker-entrypoint.sh` → `php
+bin/migrate.php`) BEFORE Apache binds the port — a failed migration
+aborts container start instead of leaving a half-applied schema
+(R01-N22). The web request path only checks `schema_version` and
+returns 503 Service Unavailable when something is pending. Wiping
+`./data/` resets the application to a blank slate.
 
 For everything else — Entra app registration, backups, troubleshooting,
 upgrades — see [`doc/admin-manual.md`](doc/admin-manual.md).

SPEC.md

@@ -1607,7 +1607,9 @@ npm run watch:css   # rebuilds public/assets/css/app.css on change
 ```
 
 The SQLite file lives at `./data/app.sqlite` on the host; nuking it resets
-the app to a blank slate (migrations run on the next request).
+the app to a blank slate (migrations run from the Docker entrypoint —
+`bin/docker-entrypoint.sh` → `php bin/migrate.php` — before Apache starts;
+the request path only checks and 503s if pending, never auto-migrates).
 
 Syntax-check PHP without Docker:
 ```bash

bin/docker-entrypoint.sh

@@ -0,0 +1,19 @@
+#!/usr/bin/env bash
+# R01-N22: deploy-time migrations.
+#
+# Apply any pending SQL migrations before Apache starts to serve traffic, so
+# the request path can simply CHECK the schema state and refuse to serve when
+# something is unexpectedly out of date — no half-applied DDL hazard.
+#
+# Failure here aborts the container start (we exit non-zero) so the operator
+# notices in `docker logs`. Apache otherwise picks up the trailing args
+# verbatim (CMD `apache2-foreground`).
+set -euo pipefail
+
+APP_ROOT="${APP_ROOT:-/var/www/html}"
+
+echo "[entrypoint] running deploy-time migrations…"
+php "${APP_ROOT}/bin/migrate.php"
+
+echo "[entrypoint] starting: $*"
+exec "$@"

bin/migrate.php

@@ -0,0 +1,47 @@
+#!/usr/bin/env php
+<?php
+
+declare(strict_types=1);
+
+/**
+ * R01-N22: deploy-time migration runner.
+ *
+ * Called by the Docker entrypoint before Apache starts, and by operators
+ * directly when applying a new release outside Docker. The web request path
+ * (`public/index.php`) only checks `Migrator::pendingFiles()` and refuses
+ * to serve when there is anything pending — it does NOT apply SQL itself.
+ *
+ * Exits 0 on success (including the no-op "already current" case) and 1 on
+ * any failure, with a one-line message on stderr. Stdout reports what was
+ * applied so the entrypoint log shows it.
+ */
+
+use App\Db\Connection;
+use App\Db\Migrator;
+
+$root = dirname(__DIR__);
+require $root . '/vendor/autoload.php';
+
+if (is_file($root . '/.env')) {
+    Dotenv\Dotenv::createImmutable($root)->safeLoad();
+}
+
+try {
+    $pdo = Connection::pdo();
+    $migrator = new Migrator($pdo);
+    $applied = $migrator->migrate();
+} catch (Throwable $e) {
+    fwrite(STDERR, 'migrate: ' . $e->getMessage() . PHP_EOL);
+    exit(1);
+}
+
+if ($applied === []) {
+    fwrite(STDOUT, 'migrate: schema already current (version ' . $migrator->currentVersion() . ')' . PHP_EOL);
+} else {
+    fwrite(STDOUT, 'migrate: applied ' . count($applied) . ' migration(s):' . PHP_EOL);
+    foreach ($applied as $f) {
+        fwrite(STDOUT, '  - ' . $f . PHP_EOL);
+    }
+    fwrite(STDOUT, 'migrate: schema now at version ' . $migrator->currentVersion() . PHP_EOL);
+}
+exit(0);

doc/admin-manual.md

@@ -280,10 +280,19 @@ What happens:
    `assets/css/input.css` into `public/assets/css/app.css`. The PHP
    stage installs Composer dependencies and copies the application
    source into `/var/www/html`.
-2. The container starts Apache on port 80, exposed on the host as
-   port **8088** (see `docker-compose.yml`).
-3. On the first request, the migration runner creates `app.sqlite`
-   inside the volume and applies all migrations under `migrations/`.
+2. The container's entrypoint (`bin/docker-entrypoint.sh`) runs
+   `php bin/migrate.php` to create `app.sqlite` inside the volume and
+   apply every file in `migrations/` BEFORE Apache binds the port. If
+   a migration fails, the container exits non-zero and Apache never
+   starts — check `docker compose logs` for the migration's stderr.
+3. Apache then starts on port 80, exposed on the host as port
+   **8088** (see `docker-compose.yml`).
+
+The web request path itself never applies SQL — it only checks that
+`schema_version` matches the on-disk migration set and refuses to serve
+(503 Service Unavailable, `Retry-After: 30`) when something is pending.
+A forgotten deploy step therefore produces a loud, fast-failing 503
+instead of silent stale-schema serving (R01-N22).
 
 Open `http://<host>:8088`. If you used the local-admin fallback, sign
 in at `/auth/local`. Otherwise click the Entra sign-in CTA on `/`.
@@ -410,10 +419,10 @@ rm -rf ./data
 docker compose up -d
 ```
 
-Migrations run again on the next request. The first administrator is
-re-seeded as described in §3.6 (OIDC bootstrap) or §3.7 (local-admin
-fallback) — there is no longer an unconditional "first sign-in becomes
-admin" path.
+Migrations run on the next container start (entrypoint), before
+Apache binds the port. The first administrator is re-seeded as
+described in §3.6 (OIDC bootstrap) or §3.7 (local-admin fallback) —
+there is no longer an unconditional "first sign-in becomes admin" path.
 
 ### 5.5 Updating to a new release
 
@@ -423,9 +432,28 @@ docker compose build --no-cache
 docker compose up -d
 ```
 
-Schema migrations under `migrations/` run automatically on the next
-request after restart. Always take a backup of `./data/app.sqlite`
-before pulling.
+Schema migrations under `migrations/` run from the entrypoint as the
+container starts (R01-N22) — `bin/docker-entrypoint.sh` calls
+`php bin/migrate.php` before `apache2-foreground`. Apache only binds
+the port if migrations succeed. Always take a backup of
+`./data/app.sqlite` before pulling.
+
+**Running migrations outside Docker.** If you deploy the code without
+the container (e.g. a bare PHP-FPM host), apply migrations manually as
+part of the release procedure, BEFORE restarting the web server:
+
+```bash
+php bin/migrate.php
+# → migrate: schema already current (version N)
+# → or: migrate: applied K migration(s):  …  schema now at version N
+```
+
+If a deploy ever skips this step, the request path will return
+`503 Service Unavailable` with `Retry-After: 30` until the operator
+runs `bin/migrate.php`. The application server log carries a single
+line listing the pending filenames. The 503 is intentional: the
+alternative — auto-migrating on a request that may then crash mid-DDL
+— is the failure mode this design rules out.
 
 **Composer dependency cadence (R01-N16).** The XLSX import wizard is
 backed by [PhpSpreadsheet](https://github.com/PHPOffice/PhpSpreadsheet),

public/index.php

@@ -82,12 +82,19 @@ if ($appEnv !== 'production') {
 FatalErrorHandler::register($appEnv, false);
 
 // ---------------------------------------------------------------------------
-// Migrations — cheap no-op when already current
+// R01-N22: schema sanity check (NOT migration).
+//
+// The request path never applies SQL — `bin/migrate.php` (run by the Docker
+// entrypoint or by the operator at deploy time) is the only path that can
+// move the schema forward. Here we only OPEN the connection and verify there
+// are no pending migration files. If there are, refuse to serve with 503 so
+// stale-schema serving cannot happen silently after a forgotten deploy step.
 // ---------------------------------------------------------------------------
 try {
     $pdo = Connection::pdo();
-    (new Migrator($pdo))->migrate();
+    $pending = (new Migrator($pdo))->pendingFiles();
 } catch (\Throwable $e) {
+    error_log('database bootstrap failed: ' . $e->getMessage());
     http_response_code(500);
     header('Content-Type: text/plain; charset=utf-8');
     echo "Database bootstrap failed.\n";
@@ -97,6 +104,23 @@ try {
     exit;
 }
 
+if ($pending !== []) {
+    error_log(
+        'schema out of date — pending migrations: '
+        . implode(', ', $pending)
+        . '. Run `php bin/migrate.php` to apply.'
+    );
+    http_response_code(503);
+    header('Content-Type: text/plain; charset=utf-8');
+    header('Retry-After: 30');
+    echo "Service Unavailable: database schema is out of date.\n";
+    if ($appEnv !== 'production') {
+        echo "Pending migrations: " . implode(', ', $pending) . "\n";
+        echo "Run: php bin/migrate.php\n";
+    }
+    exit;
+}
+
 // ---------------------------------------------------------------------------
 // Shared services
 // ---------------------------------------------------------------------------

src/Db/Migrator.php

@@ -14,7 +14,11 @@ use RuntimeException;
  * - Records applied versions in `schema_version(version, filename, applied_at)`.
  * - Each file is executed in a single transaction. No down-migrations.
  *
- * Safe to call on every request; becomes a cheap no-op once current.
+ * R01-N22: invoked at deploy time by `bin/migrate.php` (Docker entrypoint).
+ * The web request path now only CHECKS via `pendingFiles()` and refuses to
+ * serve when the on-disk migration set is ahead of `schema_version` — never
+ * applying SQL itself, so a request that crashes mid-DDL can no longer leave
+ * a half-migrated schema behind.
  */
 final class Migrator
 {
@@ -46,6 +50,28 @@ final class Migrator
         return $ran;
     }
 
+    /**
+     * @return list<string> filenames present on disk whose version is not yet
+     * recorded in `schema_version`. Empty list ⇒ schema is current.
+     *
+     * Cheap read-only check: opens `schema_version` (creates it if missing —
+     * a brand-new database needs the table even for an empty `pendingFiles()`
+     * result to make sense), reads applied versions, diffs against the file
+     * scan. No DDL beyond `CREATE TABLE IF NOT EXISTS schema_version`.
+     */
+    public function pendingFiles(): array
+    {
+        $this->ensureVersionTable();
+        $applied = $this->appliedVersions();
+        $out = [];
+        foreach ($this->discover() as [$version, $filename, $_]) {
+            if (!isset($applied[$version])) {
+                $out[] = $filename;
+            }
+        }
+        return $out;
+    }
+
     public function currentVersion(): int
     {
         $this->ensureVersionTable();

tests/Db/MigratorTest.php

@@ -0,0 +1,116 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Db;
+
+use App\Db\Migrator;
+use PDO;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * R01-N22: pin the contract that `Migrator::pendingFiles()` lets the request
+ * path detect a deploy-skipped migration without applying SQL itself, and
+ * that `migrate()` is still the apply path used by `bin/migrate.php`.
+ */
+final class MigratorTest extends TestCase
+{
+    private string $tmpDir;
+
+    protected function setUp(): void
+    {
+        $this->tmpDir = sys_get_temp_dir() . '/spw-migrator-' . bin2hex(random_bytes(4));
+        if (!mkdir($this->tmpDir) && !is_dir($this->tmpDir)) {
+            $this->fail('cannot mkdir tmp');
+        }
+    }
+
+    protected function tearDown(): void
+    {
+        foreach (glob($this->tmpDir . '/*') ?: [] as $f) {
+            @unlink($f);
+        }
+        @rmdir($this->tmpDir);
+    }
+
+    private function makePdo(): PDO
+    {
+        $pdo = new PDO('sqlite::memory:', null, null, [
+            PDO::ATTR_ERRMODE            => PDO::ERRMODE_EXCEPTION,
+            PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
+            PDO::ATTR_EMULATE_PREPARES   => false,
+        ]);
+        $pdo->exec('PRAGMA foreign_keys = ON');
+        return $pdo;
+    }
+
+    private function writeMigration(string $name, string $sql): void
+    {
+        file_put_contents($this->tmpDir . DIRECTORY_SEPARATOR . $name, $sql);
+    }
+
+    public function testPendingFilesReturnsEverythingOnVirginDb(): void
+    {
+        $this->writeMigration('001_init.sql', 'CREATE TABLE a (x INTEGER);');
+        $this->writeMigration('002_more.sql', 'CREATE TABLE b (x INTEGER);');
+
+        $migrator = new Migrator($this->makePdo(), $this->tmpDir);
+        $this->assertSame(['001_init.sql', '002_more.sql'], $migrator->pendingFiles());
+    }
+
+    public function testPendingFilesIsEmptyAfterMigrate(): void
+    {
+        $this->writeMigration('001_init.sql', 'CREATE TABLE a (x INTEGER);');
+        $this->writeMigration('002_more.sql', 'CREATE TABLE b (x INTEGER);');
+
+        $migrator = new Migrator($this->makePdo(), $this->tmpDir);
+        $migrator->migrate();
+        $this->assertSame([], $migrator->pendingFiles());
+        $this->assertSame(2, $migrator->currentVersion());
+    }
+
+    public function testPendingFilesReportsOnlyNewlyAdded(): void
+    {
+        $this->writeMigration('001_init.sql', 'CREATE TABLE a (x INTEGER);');
+        $pdo = $this->makePdo();
+        (new Migrator($pdo, $this->tmpDir))->migrate();
+
+        // Operator dropped a new file in but didn't re-run migrate.
+        $this->writeMigration('002_more.sql', 'CREATE TABLE b (x INTEGER);');
+        $migrator = new Migrator($pdo, $this->tmpDir);
+        $this->assertSame(['002_more.sql'], $migrator->pendingFiles());
+    }
+
+    public function testPendingFilesIgnoresFilesWithoutVersionPrefix(): void
+    {
+        $this->writeMigration('001_init.sql',  'CREATE TABLE a (x INTEGER);');
+        $this->writeMigration('README.md',     '# notes');
+        $this->writeMigration('foo_bar.sql',   'CREATE TABLE c (x INTEGER);');
+
+        $migrator = new Migrator($this->makePdo(), $this->tmpDir);
+        $this->assertSame(['001_init.sql'], $migrator->pendingFiles());
+    }
+
+    public function testPendingFilesCreatesSchemaVersionTable(): void
+    {
+        // Empty dir: even without any migrations the call must not blow up
+        // and the version table must exist (idempotent ensure).
+        $pdo = $this->makePdo();
+        $migrator = new Migrator($pdo, $this->tmpDir);
+        $this->assertSame([], $migrator->pendingFiles());
+
+        $row = $pdo->query("SELECT name FROM sqlite_master WHERE type='table' AND name='schema_version'")->fetch();
+        $this->assertIsArray($row);
+        $this->assertSame('schema_version', $row['name']);
+    }
+
+    public function testRealMigrationsDirectoryHasNoPendingAfterFullApply(): void
+    {
+        // Smoke test against the real migrations/ folder so a future reviewer
+        // can't drop a malformed file in without the suite noticing.
+        $pdo = $this->makePdo();
+        $migrator = new Migrator($pdo);
+        $migrator->migrate();
+        $this->assertSame([], $migrator->pendingFiles(), 'real migrations apply cleanly');
+    }
+}