Repository navigation
fix(OMN-13124): vendor pattern_learning node migration + always-run vendor-sync gate + CI gate - #1978
Conversation
…endor-sync gate + CI gate Makes the golden-chain pattern_learning chain durable across redeploys and closes the enforcement gap that let OMN-13124 merge with an un-vendored node migration (golden-chain sweep run12 Finding). - Vendor node_projection_pattern_learning/0000_create_pattern_learning_artifacts.sql and node_omnigate_projection/0000_create_gate_projection_tables.sql into docker/migrations/forward/nodes/ via scripts/sync-node-migrations.sh. The forward-migration runner applies forward/nodes/ to NODE_POSTGRES_DB= omnidash_analytics, so a clean redeploy now recreates pattern_learning_artifacts there (previously only present via a live hot-fix that reverts on redeploy). - Pre-commit hook onex-check-node-migration-sync: drop the files: scope, set always_run: true. The prior files: filter only fired when an infra path under forward/nodes/ (or the sync script) was staged; a node migration added in the omnimarket repo never touches those paths, so the hook silently passed and the un-vendored migration merged. - Add .github/workflows/node-migration-sync.yml: every infra PR + merge_group re-runs sync-node-migrations.sh --check against the omnimarket dev tip (sibling sparse-checkout), so an un-vendored/drifted node migration fails CI. Local proof: applied the migration against a throwaway postgres 14 — table + pattern_id/correlation_id/composite_score present, idempotent re-apply clean. sync --check fails (exit 1) when the vendored copy is removed, passes when present. migration_freeze + migration_sequence validators exit 0 (namespaced node: identity, no flat-sequence collision). pre-commit run green. Evidence-Source: OCC#<pending>
|
Warning Review limit reached
More reviews will be available in 55 minutes and 28 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…n (superseded by vendored node migration) The vendored node migration docker/migrations/forward/nodes/node_projection_pattern_learning/0000_create_pattern_learning_artifacts.sql (landed in #1978) is the canonical projection table for onex.evt.omniintelligence.pattern-stored.v1, applied to the omnidash_analytics DB and consumed by node_projection_pattern_learning (omnimarket). It supersedes the legacy flat migration 064_create_pattern_learning_artifacts.sql (OMN-8540), which created an orphan copy in the omnibase_infra DB with NO projection consumer. Both files defining pattern_learning_artifacts triggered a NAME_CONFLICT in the cross-repo Migration Conflict Check (the vendored 0000 adds correlation_id and relaxes NOT-NULL defaults vs 064), blocking the OCC pipeline (caught on OCC #2619). Retire the orphan the canonical way: delete the flat forward migration and its now-orphaned rollback. No skip-manifest tombstone is needed — the runner discovers files on disk, so a removed flat migration is simply never applied on fresh volumes; schema_migrations tracks by present-file migration_id with no dangling reference. The migration_sequence and migration_freeze validators both permit deletions. The vendored nodes/ tree is untouched (sync-node-migrations --check: in sync). Orphan-table data cleanup (non-prod only; do NOT run on prod): for already-applied environments the orphan table can be dropped from the omnibase_infra DB with: psql -d omnibase_infra -c 'DROP TABLE IF EXISTS pattern_learning_artifacts;' The canonical table in omnidash_analytics is unaffected. Verification: - Migration Conflict Check (OCC CI semantics, 5 repos + suppressions): No migration conflicts found; pattern_learning_artifacts conflict gone. - validate_migration_sequence: PASS (71 files, no duplicates) - validate_migration_freeze: inactive (no freeze) - Writer-Migration Coupling Check: passed - sync-node-migrations --check: in sync OMN-13124
OMN-13124 — make golden-chain pattern_learning durable (capstone Finding #1)
The original OMN-13124 PR (omnimarket #1210) shipped the node migration but it was never vendored into
omnibase_infradocker/migrations/forward/nodes/, so a clean redeploy did NOT recreatepattern_learning_artifactsinomnidash_analytics— the chain only passed via a live hot-fix that reverts on the next redeploy (golden-chain sweep run12 Finding).Changes
node_projection_pattern_learning/0000_create_pattern_learning_artifacts.sql(+node_omnigate_projection) intodocker/migrations/forward/nodes/viasync-node-migrations.sh. The forward-migration runner appliesforward/nodes/toNODE_POSTGRES_DB=omnidash_analytics, so a clean redeploy now recreates the table durably.onex-check-node-migration-sync: drop thefiles:scope, setalways_run: true. The priorfiles:filter only fired when an infra path underforward/nodes/was staged; a node migration added in the omnimarket repo never touches those paths, so the hook silently passed and the un-vendored migration merged..github/workflows/node-migration-sync.yml: every infra PR + merge_group re-runssync-node-migrations.sh --checkagainst the omnimarket dev tip (sibling sparse-checkout); an un-vendored/drifted node migration fails CI.Verification (local)
--checkpasses in sync, fails (exit 1) when a vendored copy is removed.pattern_id,correlation_id,composite_scorepresent).migration_freeze+migration_sequencevalidators exit 0 (namespacednode:identity, no flat-sequence collision).pre-commit rungreen.NO live deploy, NO restart, NO .201 mutation. The table becomes durably present on dev only after the next clean redeploy applies this committed vendored migration.
Evidence-Source: OCC#2618
Evidence-Ticket: OMN-13124
Do NOT merge — reported for the lead to enqueue.