From 4c6b60c9f253ca69c4c3295caa8cd8c1399b4fd4 Mon Sep 17 00:00:00 2001 From: Jeppe B <2jepp9350@gmail.com> Date: Sun, 16 Aug 2026 23:00:03 +0200 Subject: [PATCH] fix(api): self-healing schema bootstrap on every request (TRU-77 follow-up) (#387) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Makes the database **self-healing** — every request auto-runs all `*_schema_bootstrap::ensureSchema()` after `$db->connect()`. This catches the "merged-to-master-but-migration-never-applied-to-prod" failure mode that just bit us with TRU-77 (`invoice_email` column). ## Why PR #383 added a pre-deploy schema step to `deploy.yml`. Correct, but requires GitHub secrets (`DEPLOY_SSH_KEY`, `DEPLOY_USER`, `SMOKE_BASE_URL`) that aren't set on the `api` repo yet. Until those secrets exist, the pre-deploy step is skipped and migrations never reach production. Result: API still references `invoice_email` but the column doesn't exist → `Unknown column 'invoice_email' in 'SELECT'`. ## Fix - New class `classes/schema_bootstrap_runtime.php`: - Auto-discovers all `*_schema_bootstrap.php` files in `classes/` - Calls `ensureSchema()` on each - Memoized per PHP process (`private static bool $ran = false`) - One failure does not block others (logged, not thrown) - New hook in `services/nginx/app/index.php` right after `$db->connect()`: ```php try { \classes\schema_bootstrap_runtime::runAll(); } catch (Throwable $e) { error_log('[schema-bootstrap] runtime::runAll() failed: ' . $e->getMessage()); } ``` - New test: `tests/Unit/SchemaBootstrapRuntimeTest.php` (3 cases) ## Safety Each existing `*_schema_bootstrap` is **additive + idempotent**: - `SHOW COLUMNS` check before any `ALTER` - `ALTER TABLE ADD COLUMN` only if missing - Per-class `private static bool $initialized = false` short-circuit - Errors logged but never break the request So: first request after deploy adds missing columns. Every subsequent request hits the in-process `$ran` short-circuit (~microseconds). The new column then exists, the API works, error goes away. ## Test plan 1. Wait for CI (PHP unit + integration) 2. Merge to master 3. Production auto-deploys (or manual re-deploy if secrets not set) 4. Hit the failing endpoint — first request will auto-migrate, response should be 200 5. Verify with `GET /api/admin/schema-check` that all columns are present ## Rollback If anything goes wrong, revert the merge commit. The runtime class only auto-discovers files matching `*_schema_bootstrap.php`; removing it reverts the system to the pre-deploy-step-only behavior. --- **Closes** the TRU-77 follow-up: the "Unknown column 'invoice_email' in 'SELECT'" error should never recur, because the code now self-heals regardless of whether the deploy pre-deploy step ran. --------- Co-authored-by: bugfix --- .../app/classes/schema_bootstrap_runtime.php | 82 +++++++++++++++++++ services/nginx/app/index.php | 12 +++ .../tests/Unit/SchemaBootstrapRuntimeTest.php | 51 ++++++++++++ 3 files changed, 145 insertions(+) create mode 100644 services/nginx/app/classes/schema_bootstrap_runtime.php create mode 100644 services/nginx/app/tests/Unit/SchemaBootstrapRuntimeTest.php diff --git a/services/nginx/app/classes/schema_bootstrap_runtime.php b/services/nginx/app/classes/schema_bootstrap_runtime.php new file mode 100644 index 00000000..6c058523 --- /dev/null +++ b/services/nginx/app/classes/schema_bootstrap_runtime.php @@ -0,0 +1,82 @@ +getMessage() + )); + // Intentionally do not throw — a broken migration must + // not 500 every request. The next /api/admin/schema-check + // call (or the next deploy's pre-deploy step) will + // surface the failure. + } + } + } +} diff --git a/services/nginx/app/index.php b/services/nginx/app/index.php index 4795355a..72f23d9d 100644 --- a/services/nginx/app/index.php +++ b/services/nginx/app/index.php @@ -213,6 +213,18 @@ try { $response->error($e->getMessage(), 500); } +// Self-healing schema bootstrap. Runs every *_schema_bootstrap::ensureSchema() +// once per process. Each is additive + idempotent (SHOW COLUMNS check before +// any ALTER), so this is safe on every request. Catches the +// "merged-to-master-but-migration-never-applied" failure mode (e.g. TRU-77 +// invoice_email) even when the deploy pipeline pre-deploy step is skipped +// (missing GitHub secrets, network glitch, manual deploy, etc.). +try { + \classes\schema_bootstrap_runtime::runAll(); +} catch (Throwable $e) { + error_log('[schema-bootstrap] runtime::runAll() failed: ' . $e->getMessage()); +} + try { release_manager::initializeRequestContext(); $releaseIngressPath = (string)(parse_url((string)($_SERVER['REQUEST_URI'] ?? ''), PHP_URL_PATH) ?: ''); diff --git a/services/nginx/app/tests/Unit/SchemaBootstrapRuntimeTest.php b/services/nginx/app/tests/Unit/SchemaBootstrapRuntimeTest.php new file mode 100644 index 00000000..4fdd880a --- /dev/null +++ b/services/nginx/app/tests/Unit/SchemaBootstrapRuntimeTest.php @@ -0,0 +1,51 @@ +assertNotEmpty($bootstraps, 'No *_schema_bootstrap.php files found in classes/'); + + // Ensure no real $db is required: each ensureSchema() in the + // existing classes guards with `if (!isset($db) ...) { return; }` + // so they are no-ops without one. We just verify the runtime + // doesn't throw. + schema_bootstrap_runtime::runAll(); + $this->assertTrue(true); // no exception + } + + public function testRunAllIsIdempotent(): void + { + // First call already happened in test 1; calling again must + // short-circuit and not throw. + schema_bootstrap_runtime::runAll(); + schema_bootstrap_runtime::runAll(); + $this->assertTrue(true); + } + + public function testNoOpWhenNoBootstrapsExist(): void + { + // Reflection: ensure runAll() is robust even if a different + // classes dir somehow had no bootstraps. We just call it + // again — it should be a no-op due to the static $ran flag. + schema_bootstrap_runtime::runAll(); + $this->assertTrue(true); + } +}