From 012ca5036c2df325ba47e4843601d646eb502d60 Mon Sep 17 00:00:00 2001 From: jonahgabriel Date: Fri, 12 Jun 2026 11:13:12 -0400 Subject: [PATCH 1/2] =?UTF-8?q?fix(OMN-13062):=20migration-gate=20vacuity?= =?UTF-8?q?=20fix=20=E2=80=94=20sentinel=20discipline,=20wait-for-postgres?= =?UTF-8?q?,=20skip-manifest,=20sync-check=20nonzero?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes three bugs identified in retro A-10 (recurrences OMN-12885, OMN-12934): (1) RUNNER SENTINEL DISCIPLINE run-forward-migrations.sh now clears migrations_complete=FALSE at the start of every run and sets it TRUE only as its FINAL act after all infra and node migrations succeed. Any mid-run failure leaves the gate UNHEALTHY. runner_completed_at is stamped at the same final step as durable evidence of a successful completion run. (2) SYNC-NODE-MIGRATIONS VACUOUS GATE sync-node-migrations.sh --check now exits 2 (not 0) when the omnimarket source tree is unresolvable. Silent exit-0 was hiding drift. The single opt-out is SYNC_NODE_MIGRATIONS_SKIP_UNRESOLVABLE=1 for environments that intentionally run without the source. (3) WAIT-FOR-POSTGRES GUARD run-forward-migrations.sh now waits up to PG_WAIT_RETRIES (default 30) x 2s for Postgres to accept connections before proceeding, guarding the first-boot initdb race. (4) SKIP-MANIFEST docker/migrations/skip-manifest.yaml introduced as the sole committed escape for intentionally-skipped migrations. The runner reads this at startup; listed migrations are recorded in schema_migrations with checksum "skip-manifest" without executing the SQL. (5) MIGRATION 085 Adds runner_completed_at TIMESTAMPTZ column to db_metadata so the runner's final stamp is durable in the schema (idempotent ADD COLUMN IF NOT EXISTS). Rollback included. 22 regression tests added covering all five fix surfaces. --- ...add_runner_completed_at_to_db_metadata.sql | 38 ++ ...add_runner_completed_at_to_db_metadata.sql | 9 + docker/migrations/skip-manifest.yaml | 34 ++ scripts/run-forward-migrations.sh | 117 +++++ scripts/sync-node-migrations.sh | 21 +- .../test_migration_gate_vacuity_fix.py | 398 ++++++++++++++++++ 6 files changed, 608 insertions(+), 9 deletions(-) create mode 100644 docker/migrations/forward/085_add_runner_completed_at_to_db_metadata.sql create mode 100644 docker/migrations/rollback/rollback_085_add_runner_completed_at_to_db_metadata.sql create mode 100644 docker/migrations/skip-manifest.yaml create mode 100644 tests/unit/migrations/test_migration_gate_vacuity_fix.py diff --git a/docker/migrations/forward/085_add_runner_completed_at_to_db_metadata.sql b/docker/migrations/forward/085_add_runner_completed_at_to_db_metadata.sql new file mode 100644 index 0000000000..0e3cc78ec2 --- /dev/null +++ b/docker/migrations/forward/085_add_runner_completed_at_to_db_metadata.sql @@ -0,0 +1,38 @@ +-- ============================================================================= +-- MIGRATION: Add runner_completed_at column to db_metadata (OMN-13062) +-- ============================================================================= +-- Ticket: OMN-13062 (migration-gate vacuity fix — retro A-10) +-- Recurrences: OMN-12885, OMN-12934 +-- Version: 1.0.0 +-- +-- PURPOSE: +-- Extends db_metadata with runner_completed_at TIMESTAMPTZ. The forward +-- migration runner (run-forward-migrations.sh) stamps this column as its +-- FINAL act after every migration in the infra and node sets succeeds. +-- +-- This timestamp is the durable evidence that the runner reached +-- successful completion. Combined with the migrations_complete=TRUE +-- sentinel (OMN-3737), the migration-gate healthcheck now has two +-- independent signals: +-- 1. migrations_complete=TRUE — the flag was set without error +-- 2. runner_completed_at IS NOT NULL — the runner reached its final act +-- +-- The runner clears migrations_complete to FALSE at the start of every run +-- and sets it (together with runner_completed_at) only after all migrations +-- succeed. Any nonzero exit from any migration leaves the gate UNHEALTHY. +-- +-- IDEMPOTENCY: +-- ALTER TABLE ... ADD COLUMN IF NOT EXISTS is safe to re-run. +-- +-- ROLLBACK: +-- See rollback/rollback_085_add_runner_completed_at_to_db_metadata.sql +-- ============================================================================= + +ALTER TABLE public.db_metadata + ADD COLUMN IF NOT EXISTS runner_completed_at TIMESTAMPTZ; + +COMMENT ON COLUMN public.db_metadata.runner_completed_at IS + 'Stamped by run-forward-migrations.sh as its final act after all ' + 'infra and node migrations succeed. NULL means the runner never ' + 'completed successfully (partial run, first boot, or mid-run failure). ' + 'OMN-13062 (retro A-10).'; diff --git a/docker/migrations/rollback/rollback_085_add_runner_completed_at_to_db_metadata.sql b/docker/migrations/rollback/rollback_085_add_runner_completed_at_to_db_metadata.sql new file mode 100644 index 0000000000..5608069cae --- /dev/null +++ b/docker/migrations/rollback/rollback_085_add_runner_completed_at_to_db_metadata.sql @@ -0,0 +1,9 @@ +-- ============================================================================= +-- ROLLBACK: Remove runner_completed_at column from db_metadata (OMN-13062) +-- ============================================================================= +-- Ticket: OMN-13062 (migration-gate vacuity fix — retro A-10) +-- Version: 1.0.0 +-- ============================================================================= + +ALTER TABLE public.db_metadata + DROP COLUMN IF EXISTS runner_completed_at; diff --git a/docker/migrations/skip-manifest.yaml b/docker/migrations/skip-manifest.yaml new file mode 100644 index 0000000000..cccfbad11a --- /dev/null +++ b/docker/migrations/skip-manifest.yaml @@ -0,0 +1,34 @@ +# skip-manifest.yaml — Intentionally-skipped migration IDs +# +# Ticket: OMN-13062 (migration-gate vacuity fix — retro A-10) +# +# PURPOSE +# The forward migration runner (run-forward-migrations.sh) checks this +# file before applying each migration. A migration listed here is treated +# as already-applied (a skip record is written to schema_migrations with +# checksum "skip-manifest") without executing the SQL. +# +# WHEN TO USE +# - A migration was discovered to be a no-op on warm volumes (already +# applied via docker-entrypoint-initdb.d on first boot). +# - A migration contains a known-safe but environment-specific statement +# that must not re-run on warm volumes. +# - Never use this to hide a broken migration — fix the SQL instead. +# +# HOW TO ADD AN ENTRY +# 1. Confirm the migration is safe to skip (e.g., idempotent but already +# applied, or intentionally empty). +# 2. Add the entry to skipped_migrations below. +# 3. Commit the manifest change in the SAME PR that deems the migration +# unrunnable. Operator env cannot inject skips. +# +# ENTRY FORMAT +# id: "docker/" — migration_id as recorded in schema_migrations +# reason: "human-readable reason" +# ticket: "OMN-XXXX" — Linear ticket authorising the skip +# +# EXIT CODES for run-forward-migrations.sh +# 0 — all migrations applied (or skipped by this manifest) +# 1 — migration failure (nonzero psql exit) + +skipped_migrations: [] diff --git a/scripts/run-forward-migrations.sh b/scripts/run-forward-migrations.sh index 7b045c5991..3f11dbe90f 100755 --- a/scripts/run-forward-migrations.sh +++ b/scripts/run-forward-migrations.sh @@ -14,6 +14,27 @@ # mechanism for keeping warm Postgres volumes up-to-date with new migrations. # # Ticket: OMN-4175 (Forward migration runner for warm Postgres volumes) +# Ticket: OMN-13062 (migration-gate vacuity fix — retro A-10) +# +# --------------------------------------------------------------------------- +# Sentinel discipline (OMN-13062) +# --------------------------------------------------------------------------- +# migrations_complete is cleared to FALSE at the start of every runner +# invocation and set to TRUE only as the FINAL act after all infra and +# node migrations apply without error. Any nonzero exit from any migration +# leaves the flag FALSE, making the migration-gate healthcheck UNHEALTHY. +# +# The committed per-migration skip-manifest is the SOLE escape for migrations +# that must be intentionally skipped: +# docker/migrations/skip-manifest.yaml +# Format: +# skipped_migrations: +# - id: "docker/NNN_name.sql" +# reason: "..." +# ticket: "OMN-XXXX" +# The runner reads this at startup; a listed migration_id is treated as +# already-applied without executing the SQL. New entries must be committed +# in the same PR that deems the migration unrunnable. # # --------------------------------------------------------------------------- # Node-owned migration auto-discovery (OMN-12559) @@ -40,6 +61,7 @@ # MIGRATIONS_DIR (default: /migrations/forward) # NODE_MIGRATIONS_DIR (default: ${MIGRATIONS_DIR}/nodes) # NODE_POSTGRES_DB (default: POSTGRES_DB; compose sets omnidash_analytics) +# PG_WAIT_RETRIES (default: 30 — number of 2s waits for postgres ready) set -e @@ -50,9 +72,50 @@ PGDB="${POSTGRES_DB:-omnibase_infra}" MIGRATIONS_DIR="${MIGRATIONS_DIR:-/migrations/forward}" NODE_MIGRATIONS_DIR="${NODE_MIGRATIONS_DIR:-${MIGRATIONS_DIR}/nodes}" NODE_PGDB="${NODE_POSTGRES_DB:-${PGDB}}" +PG_WAIT_RETRIES="${PG_WAIT_RETRIES:-30}" export PGPASSWORD="${POSTGRES_PASSWORD}" +# --------------------------------------------------------------------------- +# Skip-manifest: load intentionally-skipped migration ids (OMN-13062) +# --------------------------------------------------------------------------- +# Format: YAML file with a top-level list "skipped_migrations" each entry has +# "id" (e.g. "docker/038_placeholder.sql") and optionally "reason" / "ticket". +# Only a committed manifest is honoured — operator env cannot inject skips. +SKIP_MANIFEST="${MIGRATIONS_DIR%/forward}/skip-manifest.yaml" +SKIPPED_IDS="" +if [ -f "${SKIP_MANIFEST}" ]; then + echo "[forward-migration] Loading skip-manifest: ${SKIP_MANIFEST}" + # Extract quoted id: values from YAML using portable sed (no yq/python/gawk). + # Handles lines of the form: - id: "docker/NNN_name.sql" + SKIPPED_IDS="$(sed -n 's/^[[:space:]]*-[[:space:]]*id:[[:space:]]*"\([^"]*\)".*/\1/p' \ + "${SKIP_MANIFEST}" 2>/dev/null || true)" +fi + +is_skipped_by_manifest() { + migration_id="$1" + if [ -z "${SKIPPED_IDS}" ]; then + return 1 + fi + echo "${SKIPPED_IDS}" | grep -Fxq "${migration_id}" +} + +# --------------------------------------------------------------------------- +# 0. Wait for Postgres to be ready (first-boot initdb race guard, OMN-13062) +# --------------------------------------------------------------------------- +echo "[forward-migration] Waiting for Postgres to accept connections..." +retries=0 +until psql -h "$PGHOST" -p "$PGPORT" -U "$PGUSER" -d "$PGDB" -c "SELECT 1" >/dev/null 2>&1; do + retries=$((retries + 1)) + if [ "$retries" -ge "$PG_WAIT_RETRIES" ]; then + echo "[forward-migration] ERROR: Postgres not ready after ${PG_WAIT_RETRIES} retries. Aborting." >&2 + exit 1 + fi + echo "[forward-migration] postgres not ready (attempt ${retries}/${PG_WAIT_RETRIES}), retrying in 2s..." + sleep 2 +done +echo "[forward-migration] Postgres is ready." + validate_database_identifier() { database="$1" if ! printf '%s' "$database" | grep -Eq '^[A-Za-z_][A-Za-z0-9_-]*$'; then @@ -96,6 +159,33 @@ CREATE TABLE IF NOT EXISTS public.schema_migrations ( ); " +# --------------------------------------------------------------------------- +# 1a. Clear the sentinel at the start of every run (OMN-13062) +# --------------------------------------------------------------------------- +# This ensures that any mid-run failure leaves migrations_complete=FALSE so +# the migration-gate healthcheck stays UNHEALTHY. The sentinel is only set +# TRUE as the very last act of this script (after all migrations succeed). +# We use a conditional UPDATE so this is a no-op on volumes that have not +# yet applied migration 037 (migrations_complete column may not exist yet). +echo "[forward-migration] Clearing migration sentinel (will be re-set on successful completion)..." +psql -h "$PGHOST" -p "$PGPORT" -U "$PGUSER" -d "$PGDB" -c " +DO \$\$ +BEGIN + IF EXISTS ( + SELECT 1 FROM information_schema.columns + WHERE table_schema = 'public' + AND table_name = 'db_metadata' + AND column_name = 'migrations_complete' + ) THEN + UPDATE public.db_metadata + SET migrations_complete = FALSE, + updated_at = NOW() + WHERE id = TRUE; + END IF; +END; +\$\$; +" 2>/dev/null || true + # --------------------------------------------------------------------------- # 2. Apply pending migrations in sorted order # --------------------------------------------------------------------------- @@ -108,6 +198,18 @@ for migration_file in $(ls "${MIGRATIONS_DIR}"/*.sql | sort); do filename=$(basename "$migration_file") migration_id="docker/${filename}" + # Honour skip-manifest: treat manifest-listed migrations as already applied + if is_skipped_by_manifest "${migration_id}"; then + echo "[forward-migration] skip ${filename} (skip-manifest)" + SKIPPED=$((SKIPPED + 1)) + # Record in schema_migrations so the table stays consistent. + psql -h "$PGHOST" -p "$PGPORT" -U "$PGUSER" -d "$PGDB" \ + -c "INSERT INTO public.schema_migrations (migration_id, checksum, source_set) + VALUES ('${migration_id}', 'skip-manifest', 'docker') + ON CONFLICT (migration_id) DO NOTHING;" + continue + fi + # Check if already applied already_applied=$(psql -h "$PGHOST" -p "$PGPORT" -U "$PGUSER" -d "$PGDB" \ -tAc "SELECT 1 FROM public.schema_migrations WHERE migration_id = '${migration_id}'" 2>/dev/null || true) @@ -199,3 +301,18 @@ else fi echo "[forward-migration] Complete: ${APPLIED} infra applied, ${SKIPPED} infra skipped; ${NODE_APPLIED} node applied, ${NODE_SKIPPED} node skipped." + +# --------------------------------------------------------------------------- +# 4. Set the sentinel TRUE only after ALL migrations succeed (OMN-13062) +# --------------------------------------------------------------------------- +# This is the FINAL act. Any earlier failure leaves migrations_complete=FALSE. +# runner_completed_at records the timestamp of this successful completion. +echo "[forward-migration] All migrations complete. Setting sentinel TRUE..." +psql -h "$PGHOST" -p "$PGPORT" -U "$PGUSER" -d "$PGDB" -v ON_ERROR_STOP=1 -c " +UPDATE public.db_metadata +SET migrations_complete = TRUE, + runner_completed_at = NOW(), + updated_at = NOW() +WHERE id = TRUE; +" +echo "[forward-migration] Sentinel set. Migration gate will report HEALTHY." diff --git a/scripts/sync-node-migrations.sh b/scripts/sync-node-migrations.sh index 6cffcd8254..4f2d3ce5d0 100755 --- a/scripts/sync-node-migrations.sh +++ b/scripts/sync-node-migrations.sh @@ -95,17 +95,20 @@ PY OMK_RESOLVED="$(resolve_omnimarket_src || true)" if [ -z "${OMK_RESOLVED}" ]; then - # In --check mode (pre-commit / CI), the omnimarket source tree is frequently - # not checked out. The vendored files are the durable source of truth and are - # validated independently by the migration tests; a drift check is only - # meaningful when the source is present. Skip (success) rather than fail so the - # gate never produces false negatives on runners without omnimarket. - if [ "${CHECK_MODE}" -eq 1 ]; then - echo "[sync-node-migrations] omnimarket source not resolvable — skipping drift check." >&2 - exit 0 - fi echo "[sync-node-migrations] ERROR: could not resolve omnimarket source tree." >&2 echo " Set OMNIMARKET_SRC=, or OMNI_HOME=, or pip install omnimarket." >&2 + if [ "${CHECK_MODE}" -eq 1 ]; then + # OMN-13062 (retro A-10): --check with unresolvable source is a vacuous gate. + # Silently passing when the source is absent means drift is never detected. + # Exit 2 (source-unresolvable) so the CI gate fires and the operator is + # required to either supply OMNIMARKET_SRC / OMNI_HOME or explicitly skip + # this check via SYNC_NODE_MIGRATIONS_SKIP_UNRESOLVABLE=1. + if [ "${SYNC_NODE_MIGRATIONS_SKIP_UNRESOLVABLE:-0}" = "1" ]; then + echo "[sync-node-migrations] SYNC_NODE_MIGRATIONS_SKIP_UNRESOLVABLE=1 — skipping unresolvable-source error." >&2 + exit 0 + fi + exit 2 + fi exit 2 fi diff --git a/tests/unit/migrations/test_migration_gate_vacuity_fix.py b/tests/unit/migrations/test_migration_gate_vacuity_fix.py new file mode 100644 index 0000000000..d8a1f619d7 --- /dev/null +++ b/tests/unit/migrations/test_migration_gate_vacuity_fix.py @@ -0,0 +1,398 @@ +# SPDX-FileCopyrightText: 2025 OmniNode.ai Inc. +# SPDX-License-Identifier: MIT + +"""Regression tests for OMN-13062 — migration-gate vacuity fix (retro A-10). + +Three bugs fixed: + +(1) RUNNER SENTINEL DISCIPLINE + run-forward-migrations.sh must clear migrations_complete to FALSE at the + start of every run, and set it TRUE only as the FINAL act after all + migrations succeed. Any mid-run failure must leave the gate UNHEALTHY. + +(2) SYNC-NODE-MIGRATIONS VACUOUS GATE + sync-node-migrations.sh --check must exit nonzero (2) when the + omnimarket source tree is unresolvable. Silently passing hides drift. + SYNC_NODE_MIGRATIONS_SKIP_UNRESOLVABLE=1 provides the single opt-out. + +(3) WAIT-FOR-POSTGRES GUARD + run-forward-migrations.sh must wait for Postgres to be ready before + proceeding, guarding against the first-boot initdb race. + +(4) SKIP-MANIFEST + A committed docker/migrations/skip-manifest.yaml is the sole escape for + intentionally-skipped migrations. The runner reads this at startup. + +Recurrences: OMN-12885, OMN-12934 +""" + +from __future__ import annotations + +import os +import subprocess +import tempfile +import textwrap +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[3] +RUNNER = REPO_ROOT / "scripts" / "run-forward-migrations.sh" +SYNC_SCRIPT = REPO_ROOT / "scripts" / "sync-node-migrations.sh" +SKIP_MANIFEST = REPO_ROOT / "docker" / "migrations" / "skip-manifest.yaml" +MIGRATION_085 = ( + REPO_ROOT + / "docker" + / "migrations" + / "forward" + / "085_add_runner_completed_at_to_db_metadata.sql" +) +ROLLBACK_085 = ( + REPO_ROOT + / "docker" + / "migrations" + / "rollback" + / "rollback_085_add_runner_completed_at_to_db_metadata.sql" +) + +pytestmark = pytest.mark.unit + + +# --------------------------------------------------------------------------- +# (1) Runner sentinel discipline +# --------------------------------------------------------------------------- +class TestRunnerSentinelDiscipline: + """run-forward-migrations.sh clears sentinel at start, sets at end only.""" + + def test_runner_clears_sentinel_at_start(self) -> None: + script = RUNNER.read_text(encoding="utf-8") + # The runner must set migrations_complete = FALSE before any migration. + assert "migrations_complete = FALSE" in script, ( + "run-forward-migrations.sh must clear migrations_complete=FALSE " + "at the start of every run (OMN-13062)" + ) + + def test_runner_sets_sentinel_only_at_end(self) -> None: + script = RUNNER.read_text(encoding="utf-8") + # The sentinel-set TRUE block must come AFTER the node-migration loop. + node_discovery_pos = script.index("Node-owned migration auto-discovery") + sentinel_set_pos = script.index("Setting sentinel TRUE") + assert sentinel_set_pos > node_discovery_pos, ( + "The sentinel TRUE set must be the runner's final act, " + "after all infra and node migrations complete (OMN-13062)" + ) + + def test_runner_sets_runner_completed_at(self) -> None: + script = RUNNER.read_text(encoding="utf-8") + assert "runner_completed_at = NOW()" in script, ( + "run-forward-migrations.sh must stamp runner_completed_at " + "as its final act (OMN-13062)" + ) + + def test_runner_uses_on_error_stop_for_sentinel_set(self) -> None: + script = RUNNER.read_text(encoding="utf-8") + # Find the sentinel-set block at the end. + sentinel_pos = script.rindex("Setting sentinel TRUE") + block_after = script[sentinel_pos:] + assert "ON_ERROR_STOP=1" in block_after, ( + "The final sentinel-set psql call must use -v ON_ERROR_STOP=1 " + "so a DB error at that point still fails the runner (OMN-13062)" + ) + + def test_runner_migration_failure_leaves_gate_unhealthy(self) -> None: + """A failing migration must exit nonzero (via set -e + ON_ERROR_STOP).""" + script = RUNNER.read_text(encoding="utf-8") + # The runner uses set -e; psql uses -v ON_ERROR_STOP=1 for each apply. + assert "set -e" in script, ( + "run-forward-migrations.sh must use 'set -e' so any psql failure " + "aborts the runner before the sentinel is set (OMN-13062)" + ) + assert "-v ON_ERROR_STOP=1 -f" in script, ( + "Each migration apply must use -v ON_ERROR_STOP=1 so a SQL error " + "causes psql to exit nonzero and the runner to abort (OMN-13062)" + ) + + +# --------------------------------------------------------------------------- +# (2) sync-node-migrations --check vacuous gate +# --------------------------------------------------------------------------- +class TestSyncNodeMigrationsCheckVacuity: + """sync-node-migrations.sh --check must exit nonzero when source is absent.""" + + def test_check_mode_exits_nonzero_when_source_unresolvable(self) -> None: + """--check with no source must exit 2, not 0.""" + result = subprocess.run( + ["bash", str(SYNC_SCRIPT), "--check"], + cwd=str(REPO_ROOT), + env={ + k: v + for k, v in os.environ.items() + # Unset OMNI_HOME and OMNIMARKET_SRC to make source unresolvable. + if k not in ("OMNI_HOME", "OMNIMARKET_SRC") + }, + capture_output=True, + text=True, + timeout=30, + check=False, + ) + assert result.returncode == 2, ( + f"sync-node-migrations.sh --check must exit 2 when omnimarket " + f"source is unresolvable (OMN-13062 vacuous-gate sibling fix). " + f"Got returncode={result.returncode}.\n" + f"stderr: {result.stderr}\nstdout: {result.stdout}" + ) + + def test_check_mode_skip_env_allows_exit_zero(self) -> None: + """SYNC_NODE_MIGRATIONS_SKIP_UNRESOLVABLE=1 provides the single opt-out.""" + result = subprocess.run( + ["bash", str(SYNC_SCRIPT), "--check"], + cwd=str(REPO_ROOT), + env={ + k: v + for k, v in os.environ.items() + if k not in ("OMNI_HOME", "OMNIMARKET_SRC") + } + | {"SYNC_NODE_MIGRATIONS_SKIP_UNRESOLVABLE": "1"}, + capture_output=True, + text=True, + timeout=30, + check=False, + ) + assert result.returncode == 0, ( + f"sync-node-migrations.sh --check with " + f"SYNC_NODE_MIGRATIONS_SKIP_UNRESOLVABLE=1 must exit 0. " + f"Got returncode={result.returncode}.\n" + f"stderr: {result.stderr}\nstdout: {result.stdout}" + ) + + def test_sync_script_documents_exit_code_2_for_unresolvable(self) -> None: + script = SYNC_SCRIPT.read_text(encoding="utf-8") + assert "exit 2" in script, ( + "sync-node-migrations.sh must document and implement exit code 2 " + "for unresolvable omnimarket source (OMN-13062)" + ) + + def test_sync_script_has_skip_unresolvable_env_escape(self) -> None: + script = SYNC_SCRIPT.read_text(encoding="utf-8") + assert "SYNC_NODE_MIGRATIONS_SKIP_UNRESOLVABLE" in script, ( + "sync-node-migrations.sh must honour " + "SYNC_NODE_MIGRATIONS_SKIP_UNRESOLVABLE=1 as the single escape " + "for --check when source is absent (OMN-13062)" + ) + + +# --------------------------------------------------------------------------- +# (3) wait-for-postgres guard +# --------------------------------------------------------------------------- +class TestWaitForPostgresGuard: + """run-forward-migrations.sh must wait for Postgres before proceeding.""" + + def test_runner_has_postgres_wait_loop(self) -> None: + script = RUNNER.read_text(encoding="utf-8") + assert "Waiting for Postgres" in script, ( + "run-forward-migrations.sh must wait for Postgres before " + "applying migrations (OMN-13062 first-boot initdb race guard)" + ) + + def test_runner_wait_loop_has_retry_limit(self) -> None: + script = RUNNER.read_text(encoding="utf-8") + assert "PG_WAIT_RETRIES" in script, ( + "The Postgres wait loop must honour PG_WAIT_RETRIES so the " + "runner eventually exits nonzero on a stuck Postgres (OMN-13062)" + ) + + def test_runner_postgres_wait_exits_nonzero_on_timeout( + self, tmp_path: Path + ) -> None: + """A fake psql that always fails triggers the retry limit and exits 1.""" + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + fake_psql = bin_dir / "psql" + fake_psql.write_text("#!/bin/sh\nexit 1\n", encoding="utf-8") + fake_psql.chmod(0o755) + + # Use an empty migrations dir so the runner stops after the wait loop. + migrations_dir = tmp_path / "migrations" + migrations_dir.mkdir() + + result = subprocess.run( + ["sh", str(RUNNER)], + check=False, + env={ + # Minimal env — no real Postgres vars so fake psql handles all calls. + "POSTGRES_PASSWORD": "test", + "POSTGRES_HOST": "127.0.0.1", + "POSTGRES_PORT": "65534", # unused port + "MIGRATIONS_DIR": str(migrations_dir), + "NODE_MIGRATIONS_DIR": str(tmp_path / "no-nodes"), + "PG_WAIT_RETRIES": "2", # fast timeout for tests + "PATH": f"{bin_dir}{os.pathsep}{os.environ.get('PATH', '/usr/bin:/bin')}", + "HOME": os.environ.get("HOME", str(tmp_path)), + }, + text=True, + capture_output=True, + timeout=30, + ) + + assert result.returncode != 0, ( + "run-forward-migrations.sh must exit nonzero when Postgres is not " + "reachable within PG_WAIT_RETRIES retries (OMN-13062)" + ) + assert "not ready" in result.stdout or "Aborting" in result.stdout, ( + f"Expected 'not ready' or 'Aborting' in runner output.\n" + f"stdout: {result.stdout}\nstderr: {result.stderr}" + ) + + +# --------------------------------------------------------------------------- +# (4) Skip-manifest +# --------------------------------------------------------------------------- +class TestSkipManifest: + """The committed skip-manifest.yaml is the sole escape for skipped migrations.""" + + def test_skip_manifest_exists(self) -> None: + assert SKIP_MANIFEST.exists(), ( + f"docker/migrations/skip-manifest.yaml must exist (OMN-13062): {SKIP_MANIFEST}" + ) + + def test_skip_manifest_has_skipped_migrations_key(self) -> None: + content = SKIP_MANIFEST.read_text(encoding="utf-8") + assert "skipped_migrations:" in content, ( + "skip-manifest.yaml must contain a 'skipped_migrations:' key (OMN-13062)" + ) + + def test_runner_reads_skip_manifest(self) -> None: + script = RUNNER.read_text(encoding="utf-8") + assert "skip-manifest.yaml" in script, ( + "run-forward-migrations.sh must read skip-manifest.yaml (OMN-13062)" + ) + + def test_runner_has_is_skipped_by_manifest_function(self) -> None: + script = RUNNER.read_text(encoding="utf-8") + assert "is_skipped_by_manifest" in script, ( + "run-forward-migrations.sh must implement is_skipped_by_manifest " + "to check each migration against the skip-manifest (OMN-13062)" + ) + + def test_runner_honours_skip_manifest_in_migration_loop( + self, tmp_path: Path + ) -> None: + """A migration listed in skip-manifest.yaml must not be executed.""" + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + + # psql that exits 0 on SELECT 1 (wait guard), and records calls. + call_log = tmp_path / "psql_calls.log" + fake_psql = bin_dir / "psql" + fake_psql.write_text( + textwrap.dedent(f"""\ + #!/bin/sh + printf '%s\\n' "$*" >> "{call_log}" + # If the -c arg contains SELECT 1, succeed (wait guard). + if echo "$*" | grep -q 'SELECT 1'; then + exit 0 + fi + # CREATE TABLE for schema_migrations — succeed. + if echo "$*" | grep -q 'CREATE TABLE'; then + exit 0 + fi + # INSERT INTO schema_migrations — succeed. + if echo "$*" | grep -q 'INSERT INTO'; then + exit 0 + fi + # DO block for sentinel clear — succeed. + if echo "$*" | grep -q 'BEGIN'; then + exit 0 + fi + # Actual -f migration file — succeed. + exit 0 + """), + encoding="utf-8", + ) + fake_psql.chmod(0o755) + + migrations_dir = tmp_path / "migrations" / "forward" + migrations_dir.mkdir(parents=True) + + # One migration that is listed in the skip-manifest. + skip_sql = migrations_dir / "001_should_be_skipped.sql" + skip_sql.write_text("SELECT 'THIS SHOULD NOT RUN';\n", encoding="utf-8") + + # skip-manifest one level above forward/. + skip_manifest_path = tmp_path / "migrations" / "skip-manifest.yaml" + skip_manifest_path.write_text( + textwrap.dedent("""\ + skipped_migrations: + - id: "docker/001_should_be_skipped.sql" + reason: "test skip" + ticket: "OMN-13062" + """), + encoding="utf-8", + ) + + result = subprocess.run( + ["sh", str(RUNNER)], + check=False, + env={ + # Minimal env — deliberately exclude real POSTGRES_HOST so + # the runner uses its default "postgres" (fake psql handles it). + "POSTGRES_PASSWORD": "test", + "POSTGRES_HOST": "localhost", + "POSTGRES_PORT": "5432", + "MIGRATIONS_DIR": str(migrations_dir), + "NODE_MIGRATIONS_DIR": str(tmp_path / "no-nodes"), + "PG_WAIT_RETRIES": "1", + # Put fake psql first so it shadows any real psql. + "PATH": f"{bin_dir}{os.pathsep}{os.environ.get('PATH', '/usr/bin:/bin')}", + "HOME": os.environ.get("HOME", str(tmp_path)), + }, + text=True, + capture_output=True, + timeout=30, + ) + + assert "skip-manifest" in result.stdout, ( + f"Runner must log 'skip-manifest' for the skipped migration.\n" + f"stdout: {result.stdout}\nstderr: {result.stderr}" + ) + # The SQL file must not have been passed to psql with -f. + # The fake psql logs every call; check that the migration file never + # appears in a -f argument (it was skipped, not executed). + calls = call_log.read_text(encoding="utf-8") if call_log.exists() else "" + file_executed = f"-f {skip_sql}" in calls or f"-f {skip_sql.name}" in calls + assert not file_executed, ( + f"Skip-manifest migration must not be passed to psql with -f.\n" + f"psql calls logged:\n{calls}" + ) + + +# --------------------------------------------------------------------------- +# (5) Migration 085 — runner_completed_at schema +# --------------------------------------------------------------------------- +class TestMigration085RunnerCompletedAt: + """Migration 085 adds runner_completed_at column to db_metadata.""" + + def test_migration_085_exists(self) -> None: + assert MIGRATION_085.exists(), f"Migration 085 must exist: {MIGRATION_085}" + + def test_rollback_085_exists(self) -> None: + assert ROLLBACK_085.exists(), ( + f"Rollback for migration 085 must exist: {ROLLBACK_085}" + ) + + def test_migration_085_is_idempotent(self) -> None: + sql = MIGRATION_085.read_text(encoding="utf-8") + assert "IF NOT EXISTS" in sql, ( + "Migration 085 must use ADD COLUMN IF NOT EXISTS for idempotency" + ) + + def test_migration_085_adds_runner_completed_at(self) -> None: + sql = MIGRATION_085.read_text(encoding="utf-8") + assert "runner_completed_at" in sql + assert "TIMESTAMPTZ" in sql.upper() + + def test_migration_085_does_not_create_tables(self) -> None: + sql = MIGRATION_085.read_text(encoding="utf-8").upper() + assert "CREATE TABLE" not in sql, ( + "Migration 085 must only ALTER db_metadata, not create new tables" + ) From 5bb2504c6592c573786d34a5639e717a1b5fda06 Mon Sep 17 00:00:00 2001 From: jonahgabriel Date: Fri, 12 Jun 2026 12:44:05 -0400 Subject: [PATCH 2/2] fix(OMN-13062): stamp schema fingerprint for migration 085 Migration 085 (085_add_runner_completed_at_to_db_metadata.sql) was added in the initial commit but schema_fingerprint.sha256 was not regenerated. Running `python scripts/check_schema_fingerprint.py stamp` updates the artifact from the stale hash to match the 71 migration files. Evidence-Source: OCC#2563 Evidence-Ticket: OMN-13062 --- docker/migrations/schema_fingerprint.sha256 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docker/migrations/schema_fingerprint.sha256 b/docker/migrations/schema_fingerprint.sha256 index 20a5206436..32ec458bb9 100644 --- a/docker/migrations/schema_fingerprint.sha256 +++ b/docker/migrations/schema_fingerprint.sha256 @@ -1,6 +1,6 @@ # Schema migration fingerprint for omnibase_infra (auto-generated) # Regenerate: python scripts/check_schema_fingerprint.py stamp # Verify: python scripts/check_schema_fingerprint.py verify -sha256:dc4b265ba91d2d28ca02b75546cf73f4c05be55e8a3ecd27585653959a19c734 -generated_at: 2026-06-09T17:45:31Z -migration_file_count: 70 +sha256:b24f163de9b716a0db87d15af329ddeb008f3d6531c6404867b844aed304451a +generated_at: 2026-06-12T16:41:04Z +migration_file_count: 71