fix(OMN-15336): compose migration runner honours the k8s operator fence (skip+record, never apply)

[jonahgabriel]

OMN-15336 — the compose migration runner had no operator fence

The fence gating the tenant-RLS node migrations lived only in omninode_infra/k8s/migrations/omnibase-infra-migrate.yaml. scripts/run-forward-migrations.sh — the runner every compose lane executes (dev, stability-test, judge, prod all mount it; docker/catalog/services/forward-migration.yaml, docker/docker-compose.{infra,stability-test,judge,prod}.yml) — had no fence and no skip branch of any kind in its node loop. It applied every nodes/*/*.sql it discovered. Two sanctioned runners, one shared id space, disagreeing about what is gated.

This PR ports the fence. It changes no live database state anywhere — see "What this does NOT do".

Live readback (read-only, .201, four compose lanes, db omnidash_analytics, 2026-07-29 ~00:15Z)

lane	container	gated ids present in schema_migrations	tables with relforcerowsecurity
dev	omnibase-infra-postgres	all 6, plus registration:0002	6
stability-test	omnibase-infra-stability-test-postgres	all 6, plus registration:0002	6
prod	omnibase-infra-prod-postgres	registration:0000/0001 only	0
judge	omnibase-infra-judge-postgres	registration:0000/0001 only	0

Two facts the ticket did not have:

1. The breach is wider than reported. stability-test — the designated proof lane for synthetic integration evidence — is also over the fence, with the same six FORCE-RLS tables. The ticket named only the dev lane.
2. prod and judge are clean but pending. They have not applied delegation:0023-0026, so without this fence the next forward-migration run on either would apply the un-proven tenant-RLS design unattended. That is the concrete hazard this closes.

---

The seam (stated explicitly, matched field-by-field)

Both runners walk the same vendored SQL tree (docker/migrations/forward/nodes/<node>/*.sql, synced by scripts/sync-node-migrations.sh) and mint the same id node:<node>:<filename>. The fence is a shared list over a shared id space.

field	omninode_infra k8s Job	this repo's compose runner
id grammar	node:${node_name}:$(basename "$sql_file")	node:${node_name}:${filename}
list contents and order	FENCED_NODE_MIGRATION_IDS=( … ) (6 ids)	FENCED_NODE_MIGRATION_IDS="…" (same 6, same order)
match	exact string equality per element	grep -Fxq (exact whole-line)
position in loop	before the already-applied probe	before the already-applied probe
on match	log + node_skipped++ + continue	log + NODE_SKIPPED++ + continue
ledger row written	NO	NO
env-overridable	NO (literal array)	NO (unconditional assignment)

Source read at build time: omninode_infra@dev = f436ca6 (fix(OMN-15335): … fence the registration pair (#740)), re-read immediately before push — still six ids, registration:0002 not yet fenced there.

Two deliberate deviations, both stated rather than smuggled:

• Shell dialect differs and must. The k8s Job is bash (arrays, set -euo pipefail); this runner is #!/bin/sh under busybox ash in the migration container, where arrays do not exist. The seam is the id list and the semantics, not the syntax. Asserted by test_fence_predicate_is_posix_sh.
• The ticket asked for the check "between the already-applied check and the psql -f"; it is placed BEFORE the already-applied probe instead — matching the k8s runner, and strictly safer: a fenced id then cannot be reached by any path through the loop, including a ledger probe that fails open. Asserted by test_fence_is_checked_before_the_ledger_probe_and_the_apply.

"Skip + record" means record the SKIP — counted into NODE_SKIPPED, named on stdout. The id is deliberately not inserted into schema_migrations, matching the k8s runner: a ledger row would make the eventual un-fencing a silent no-op (the runner would read the row, call it already applied, and never run the migration). Asserted by test_fenced_skip_never_records_a_ledger_row and proven live.

---

Cross-boundary regression test (drives the artifact that actually runs)

tests/scripts/test_node_migration_fence_parity.py — 10 tests, reusing the OMN-15291 scratch-Postgres harness.

Live proofs execute the shipped scripts/run-forward-migrations.sh against a real Postgres with a vendored-shaped node tree in which every fenced id creates a distinctly-named marker table, so "was this applied?" is answered by the database, not by parsing the runner's own log:

• test_shipped_runner_skips_and_records_fenced_node_migrations (GREEN) — exit 0; each fenced id reported SKIP (operator-gated …); no fenced marker table exists; no fenced id in schema_migrations; summary line reports 6 node skipped. An unfenced control migration in the same node directory IS applied and IS recorded — without it, "no fenced marker exists" would pass vacuously on a runner that never reached the node loop.
• test_fence_free_runner_applies_the_fenced_migration (RED control) — the same harness against a copy of the runner with both fence blocks mechanically stripped (derived from the shipped file, so the control cannot drift from what it controls). Asserts the fence-free runner applies every gated migration and writes every ledger row — i.e. it reproduces the exact .201/stability-test breach. Without this, the GREEN result is unfalsifiable.

Static assertions (no DB, so they gate every PR): fence present; exact ordered list vs the pinned EXPECTED_FENCE, with the delegation half and the registration half asserted separately so a failure names which half moved; every fenced id resolves to a real vendored .sql file (a typo'd id fences nothing and fails open, silently); the list is not env-overridable; ordering vs probe vs apply; skip writes nothing to the DB; POSIX-sh only.

RED-vs-base, proven not asserted

Against origin/dev's runner (byte-swapped in, then restored — sha256 verified identical after restore): 9 of 10 fail. The one that passes is test_every_fenced_id_names_a_real_vendored_sql_file, which is about the vendored SQL tree, not the runner — correct that it is green on base.

---

Cross-repo drift hazard — flagged, NOT unilaterally re-architected

This fix duplicates the list: the same ids are now hand-maintained in two repos. That is the same shape as the defect being fixed, just slower to bite.

• Always on, gates every PR: the pinned EXPECTED_FENCE tuple (exact + ordered).
• Opt-in: test_fence_matches_omninode_infra_k8s_runner diffs against the live k8s manifest, resolved from that repo's origin/dev, under REQUIRE_CROSS_REPO_FENCE_PARITY=1.
• Still uncovered: a fence entry added to the k8s runner alone lands unnoticed in this repo's CI until someone runs the opt-in check or updates the pin.

The opt-in-ness was measured, not assumed: an always-on version of that test went RED on the .200 build host purely because that host's omninode_infra clone sat two commits behind dev (working tree 294850e, origin/dev ref 6e09ec3, true tip f436ca6). Gating every PR on another repo's local checkout freshness manufactures false REDs; the failure message now names the resolved source commit so a stale clone is diagnosable instead of being mistaken for real drift.

Follow-up filed: OMN-15349 — "Single-source the node-migration operator fence across omnibase_infra + omninode_infra". Marked ARCHITECTURE / needs operator approval, with three costed options (committed manifest consumed by both runners / generated block + drift gate / cross-repo CI diff only). Not started here: single-sourcing a value consumed by two runners in two repos is an architecture change, and those are operator-gated.

Standing obligation until that lands: any change to FENCED_NODE_MIGRATION_IDS must edit both runners in the same change, plus the EXPECTED_FENCE pin. Live example: if the OMN-15345 lane adds node:node_projection_registration:0002_node_service_registry_tenant_rls.sql to the k8s fence, this list and the pin must move with it.

---

What this does NOT do (deliberate; operator-owned)

• Does not change any lane's live FORCE-RLS state. No NO FORCE, no DISABLE, no DROP POLICY, anywhere.
• Does not roll back the six tables on dev or stability-test, and does not delete their ledger rows. Bless-the-lab-as-FORCE-proving-ground vs roll-back is an outstanding operator decision.
• Does not un-fence anything, and does not add ids the k8s runner does not carry — notably registration:0002, delegation:0027, inference_response:0003, flat 081, which the ticket flags as carrying the same FORCE hazard while being ungated on both runners. Widening the fence beyond parity is a separate decision on OMN-15088; parity is this ticket.
• Touches no prod state. The only prod contact was a read-only SELECT (table above).

Behaviour change to expect: on prod and judge (which have not applied delegation:0023-0026), these ids now report SKIP (operator-gated …) on every run instead of being applied. That is the point. On dev and stability-test, where they are already applied, the fence short-circuits ahead of the already-applied probe, so the line changes from already applied to operator-gated — no state change.

---

Gates

Run on .200 (stickybeatz-studio) per rule 11a, patch-transfer with sha256 verified in both directions before any remote gate ran (scripts/run-forward-migrations.sh 058c11d7…, test file 5bcb3327…):

• ruff format --check + ruff check tests/scripts/ scripts/ — clean.
• pytest tests/scripts/test_node_migration_fence_parity.py tests/scripts/test_forward_migration_advisory_lock.py tests/unit/migrations/ with REQUIRE_CROSS_REPO_FENCE_PARITY=1 against a freshly-fetched omninode_infra@f436ca6 — 79 passed, 0 skipped (-rs). The live Postgres proofs executed there, not skipped.
• pre-commit run --all-files — two failures, both proven pre-existing on origin/dev and untouched by this PR: SPDX year 2026 in tests/ci/test_runner_routing_audit.py and tests/scripts/test_deploy_runtime_core_contracts_resolution.py (confirmed via git show origin/dev:<file> | head -1), and check-required-env-vars reporting GITHUB_TOKEN missing from that host's ~/.omnibase/.env (reproduces identically on the pristine canonical clone at dev — host env, not repo state).
• shellcheck -S warning -x scripts/run-forward-migrations.sh — clean, and clean on the base version too (the three SC2012 info findings are pre-existing ls-in-for lines this PR did not touch; the repo's shellcheck hook is scoped to deploy-runtime.sh and does not cover this file).
• Also run on this Mac (which has psql/initdb): the same fence module, 10/10 including both live proofs, fail-closed under REQUIRE_MIGRATION_LOCK_DB=1.

dod_evidence

• scripts/run-forward-migrations.sh — fence block + in-loop skip branch, both marker-delimited.
• tests/scripts/test_node_migration_fence_parity.py — 10 tests: 8 static, 2 live-Postgres (GREEN + RED control).
• RED-vs-base: 9/10 fail against origin/dev's runner.
• Live four-lane .201 readback table above (read-only).
• Follow-up OMN-15349 filed for the cross-repo single-source decision.

Closes OMN-15336.

Evidence-Ticket: OMN-15336
Evidence-Source: OCC#5386

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 34 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e4716313-c162-4ed9-9e86-95ad5cca7e04

📥 Commits

Reviewing files that changed from the base of the PR and between 9796b76 and a81f91a.

📒 Files selected for processing (2)

• scripts/run-forward-migrations.sh
• tests/scripts/test_node_migration_fence_parity.py

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

⚠️ Hostile Reviewer — DEGRADED (informational)

Blocking findings (critical): 0
Total findings: 0
Models succeeded: none

Note: All reviewer models failed or were unavailable. Degraded results are informational during the pilot phase (OMN-8468/OMN-8524) and do not block merge. Error: all review endpoints [192.168.86.201:8000 192.168.86.201:8001 ] unreachable — preflight short-circuit (no models available)

---

Gate semantics (pilot phase)

Verdict	Meaning	Blocks merge?
passed	No critical findings	No
blocked	CRITICAL findings found	Yes
degraded	All models unavailable (infra)	No (pilot)

Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8468/OMN-8524)