Repository navigation
fix(OMN-15312): wrap vendored delegation reconcile migration 0009a in an explicit transaction - #2524
Conversation
… an explicit transaction Vendored-copy half of the fix; canonical source is omnimarket src/omnimarket/nodes/node_projection_delegation/migrations/ (OMN-12559 auto-discovery). Both copies are byte-identical (sha256 b2e66b31...) so the next vendor sync is a no-op rather than a silent revert. The forward-migration runner applies node migrations with psql -f in autocommit, so LOCK TABLE errored with ERROR: LOCK TABLE can only be used in transaction blocks exiting 3 and aborting every cold-lane bring-up (OMN-13414), the only documented recovery for the GC-reclaimed dev lane (OMN-15190). Proven RED then GREEN against a cold omnidash_analytics on the .201 dev lane.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
MERGE ORDER: this PR is blocked on omnimarket#1930 by design, not by a defect.
That is the vendoring seam working correctly — it is the same gate that would have caught an infra-only fix being silently reverted by the next sync. Land omnimarket#1930 first; |
|
| 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)
|
Temporary close/reopen by codex-a merge sweep to emit a fresh pull_request.reopened event after adding Omnimarket-Source-Ref. No head change; OCC binding remains valid. |
|
Temporarily closing/reopening to refresh a scheduler-stuck CI run (30351396694 remained queued after cancellation request). No head change; OCC binding remains intact. |
Problem (proven RED, live)
0009a_delegation_events_legacy_schema_reconcile.sqlopens with three statements that are only legal inside a transaction block:The forward-migration runner (
omnibase_infra/scripts/run-forward-migrations.sh, node loop ~L418) applies node migrations withpsql -v ON_ERROR_STOP=1 -f "$migration_file"— no-1/--single-transaction, and the file carries noBEGIN;. psql runs each statement in autocommit, so theSET LOCALs degrade to warnings andLOCK TABLEhard-errors. Containeromnibase-infra-forward-migrationexits 3.Blast radius
This aborts the entire cold-lane bring-up path (OMN-13414,
docs/runbooks/cold-lane-full-bringup.md) — the only documented recovery for the GC-reclaimed.201dev lane (OMN-15190). While that lane is absent, every repo'socc-autobind/occ-companion-effectpublish fails connection-refused againstomninode-pc.tail75df5e.ts.net:19092and cascades intoocc-preflight/Receipt Gate org-wide. A migration-source defect was holding the org CI receipt path hostage.Warm lanes were unaffected only because
0009ais already recorded inschema_migrationsthere and is skipped — so this was invisible until a cold rebuild.Fix
Wrap the body in
BEGIN; ... COMMIT;. This also supplies the atomicity the file's own header already claims ("retry without leaving a partial conversion"), which did not exist under autocommit.All 80 node migrations under
docker/migrations/forward/nodes/were checked: zero containBEGIN;. Statement-level autocommit is the established runner contract and0009awas the sole violator, so this is fixed in the migration, not the shared runner — no blast radius onto the other 79.Seam (two-repo, matched byte-for-byte)
The canonical source is omnimarket
src/omnimarket/nodes/node_projection_delegation/migrations/; omnibase_infra vendors it via OMN-12559 auto-discovery. Pre-fix both copies hashedae71fec2aaab6626…. Fixing only one would let the next vendor sync silently revert it, so both land together and both now hash:The artifact that was executed in the RED/GREEN transcripts below is byte-identical (same sha256) to the file in both PRs — the test ran against the artifact that ships, not a surrogate.
RED / GREEN transcript (executed,
.201dev lane, coldomnidash_analytics)RED — pristine file, scratch DB seeded with
0007_delegation_events.sql:GREEN — patched file, same DB:
Idempotent re-apply (warm no-op, acceptance test 4):
No
SET LOCALwarnings remain post-fix.Acceptance tests (OMN-15312)
LOCK TABLESET LOCALwarningsdeploy-runtime.sh --execute --cold --forceclears migration preflightAcceptance test 3 cannot be satisfied pre-merge and this is not a gap in the evidence — it is the OMN-13415 gate working.
deploy-runtime.shasserts the deployed migration tree byte-matches the canonical clone at its committed SHA, so an uncommitted working-tree patch is correctly rejected:That gate was not bypassed, skip-tokened, or weakened. Test 3 runs as the post-merge verification.
proof_class
replay-provenfor tests 1/2/4 (RED and GREEN executed against the shipping artifact, verified by sha256 identity).code-onlyfor test 3 until merge.Refs: OMN-15190 (dev-lane GC-reclaim), OMN-13414 (cold-lane runbook), OMN-14974 (introduced the migration, #2507/#2509/#2510), OMN-12559 (vendor auto-discovery seam), OMN-15291 (adjacent runner advisory-lock defect — same file, different failure).
Evidence-Source: OCC#5275
Evidence-Ticket: OMN-15312
Omnimarket-Source-Ref: jonah/omn-15312-migration-0009a-transaction-block