Repository navigation
fix(OMN-16249): re-vendor 0005 watermarks migration with the schema-exists assert - #2808
Merged
jonahgabriel merged 1 commit intoAug 20, 2026
Conversation
…ssert This is the copy that actually runs. The migrate image builds from docker/migrations/forward/nodes/, not from omnimarket's source tree, so the companion fix in omnimarket#2110 does not reach the cluster on its own -- this vendored file is what the migrate Job executes. Its `CREATE SCHEMA IF NOT EXISTS omninode_internal` fails on the deployed lane with `ERROR: permission denied for database omnidash_analytics`: CREATE SCHEMA needs CREATE on the DATABASE, which the migration role does not hold, and IF NOT EXISTS does not save it because Postgres checks the privilege before it checks existence. The migrate Job then exceeded its backoff limit on deploy run 32301533344 and every post-migration step was skipped -- including the runtime image pin -- so nothing merged could reach onex-dev at all. Regenerated with scripts/sync-node-migrations.sh against the omnimarket branch carrying the fix (one file updated, no other vendored migration touched), so source and vendored copy are byte-identical and node-migration-vendor-parity-gate passes. Landing this first is what that gate's own error message instructs, and it is also the correct order: the vendored copy is the deployable artifact. Verified live before writing the fix rather than assuming: `DB=omnidash_analytics omninode_internal_schema_count=1 projection_watermarks_exists=0` -- the schema exists, so the assert passes where CREATE SCHEMA cannot, and the table genuinely does not exist yet, so the migration still has real work to do. Refs OMN-16249, OMN-16146. Also updates the declared checksum for this migration in docker/migrations/forward/_ledger/application-migrations.tsv (67aecf3c -> 61102cd1). Amending a declared checksum is only legitimate for a migration that has not yet been applied anywhere, which was confirmed live before touching it: the staging probe above reports projection_watermarks_exists=0, so no database has executed the old bytes. validate_application_migration_manifest passes (102 active, 0 blocked). The pre-commit vendor-sync hook resolves omnimarket through $OMNIMARKET_SRC (its own documented resolution order #1) and was pointed at the branch carrying the paired source change, because the canonical clone still sits on dev where that change has not landed yet. That is the override existing for exactly this in-flight case, not a bypass: it makes the local check compare the vendored copy against the source it is actually mirroring, and the file-level result is byte-identical to omnimarket#2110 (sha256 61102cd1 on both sides).
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 33 minutes Limit details: You’ve used the included review currently available. Your 120 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. You’re in a promotional period — use the checkbox below to run this review for free:
On-demand reviews are free for the next 30 days. After that, they cost $0.25 per reviewed file. How can I continue?Run this review now using the option above, or comment You can also wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
Comment |
Contributor
|
OCC autobind did not mint a companion for this PR: no changed-file candidate could be proven RED against the merge base, and emitting a PR-existence probe instead would be non-falsifiable evidence (OMN-15247). Hand-authored evidence is required. |
jonahgabriel
enabled auto-merge (squash)
August 19, 2026 23:00
Contributor
✅ Hostile Reviewer — PASSEDBlocking findings (critical): 0 Gate semantics (pilot phase)
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8468/OMN-8524) |
jonahgabriel
added a commit
to OmniNode-ai/onex_change_control
that referenced
this pull request
Aug 20, 2026
#6757) * evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#2808 * evidence: OCC companion self-bind for #6757 * evidence(OMN-16249): resolve contract conflict and rebind contract_sha256 #6755 merged while this companion was open; both appended to contracts/OMN-16249.yaml, so the merge conflicted. Resolved by keeping BOTH sides -- dev's entries (dod-OmniNode-ai-omnimarket-pr-2110, occ-self-bind-pr-6755) plus this branch's occ-self-bind-pr-6757. 16 entries, no marker residue, validate-yaml passes. Rebinding contract_sha256 across this PR's own receipts, because the merge changed the contract file's raw bytes. Only receipts in THIS PR's diff were touched: dod-OmniNode-ai-omnimarket-pr-2110/command.yaml is already merged on dev and is therefore immutable, so it is deliberately left alone. That merged receipt is now carrying a stale contract_sha256, and it is worth naming as a structural issue rather than hiding: a whole-file contract_sha256 is invalidated by ANY later append to the same contract, even an append that touches no part of the evidence it attests. The gate's own message points at the fix -- mint per-entry contract_entry_sha256 (OMN-13888), which is append-safe. The receipt-hardening hook only inspects a PR's changed files, so this does not block anything today; it is latent, not silent. Refs OMN-16249. --------- Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai> Co-authored-by: jonahgabriel <jonah@omninode.ai>
jonahgabriel
deleted the
jonah/omn-16249-vendor-watermarks-schema-precondition
branch
August 20, 2026 10:27
This was referenced Aug 20, 2026
jonahgabriel
added a commit
that referenced
this pull request
Aug 21, 2026
…2813) * feat(OMN-16289): sync main to release tag SHA on successful release After a release publishes (non-rc), fast-forward main to the tag's commit SHA via a non-force push. Uses the same GITHUB_TOKEN identity already persisted by the checkout step in this job -- no new secret class introduced. Part of the release-synced-main policy (main = the last published release, promotion PRs retired). Main branch protection is updated separately (empty required_status_checks contexts on main, push restricted to the release-automation identity). Committed with OMNIMARKET_SRC pointed at a local staging overlay (per scripts/sync-node-migrations.sh's own documented override) because the always-run onex-check-node-migration-sync hook is currently blocking EVERY infra commit on a real, pre-existing, unrelated drift: infra's vendored node_projection_registration/0005_create_projection_watermarks.sql (dev 381333d / OMN-16249 #2808) already carries a real permission-denied bugfix that was never backported to omnimarket's own source. Details + recommended fix posted to OMN-14975 (open umbrella ticket for this recurring drift class); not fixed here (different repo, DB migration content, deserves its own careful landing). OMN-16289 * fix(OMN-16289): remove stale doc_freshness_sweep skill mapping omnimarket#2100 (OMN-16191) deleted node_doc_freshness_sweep and its onex.nodes entry point -- the node never had a real implementation of its own, only a try/except fallback into onex_change_control's scanner that always reported a clean sweep. skill_mapping.yaml here still referenced the deleted node_name, which fails test_every_mapped_node_resolves_in_omnimarket_catalog (skill-node-mapping-sync) with the mapped catalog resolved against a live sibling omnimarket checkout -- the class of bug OMN-13531 exists to catch. Removes the doc_freshness_sweep skill_mapping.yaml entry and its _EXPECTED_SKILLS test fixture entry. onex skill doc_freshness_sweep was already broken (Unknown node at dispatch time); this just makes the catalog-sync gate agree.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the copy that actually runs
The migrate image builds from
docker/migrations/forward/nodes/, not from omnimarket's source tree. So the companion fix in omnimarket#2110 does not reach the cluster on its own — this vendored file is what the migrate Job executes. Landing this first is both whatnode-migration-vendor-parity-gateinstructs and the correct order, because the vendored copy is the deployable artifact.The defect
fails on the deployed lane:
CREATE SCHEMAneedsCREATEon the database, which the migration role does not hold, andIF NOT EXISTSdoes not save it — Postgres checks the privilege before it checks existence, so it fails even though the schema is already present.Blast radius was the whole deploy: on run 32301533344 the migrate Job exceeded its backoff limit and every post-migration step was skipped, including "Pin runtime plane deployments to the runtime digest". Nothing merged could reach onex-dev.
The fix
Regenerated with
scripts/sync-node-migrations.shagainst the omnimarket branch carrying the paired source change — one vendored file updated, no other migration touched — so source and vendored copy are byte-identical (sha256 61102cd1on both sides) and the parity gate passes. TheCREATEbecomes the assert pattern this codebase already documents for exactly this hazard (node_projection_live_events/0002, "THE SCHEMA TRAP THIS FILE ASSERTS, NOT WORKS AROUND"): readpg_catalog.pg_namespace, which needs no schema-level privilege, and fail loudly by integer division if the schema is genuinely absent.Verified live before writing it
The schema exists, so the assert passes where
CREATE SCHEMAcannot. The table genuinely does not exist yet, so the migration still has real work to do — converting CREATE→ASSERT against a database lacking the schema would only move the failure one line down.Declared-checksum update
docker/migrations/forward/_ledger/application-migrations.tsvcarries a declared checksum per migration; this one moves67aecf3c→61102cd1.Amending a declared checksum is only legitimate for a migration that has not been applied anywhere, which is exactly what the live probe above establishes (
projection_watermarks_exists=0— no database has executed the old bytes).validate_application_migration_manifestpasses: 102 active, 0 blocked, 2 historical, 30 cloud aliases.Note on the local hook override
The pre-commit vendor-sync hook resolves omnimarket through
$OMNIMARKET_SRC(its own documented resolution order #1) and was pointed at the worktree carrying the paired source change, because the canonical clone still sits ondevwhere that change has not landed. That override exists for precisely this in-flight case and is recorded in the commit message — it makes the local check compare the vendored copy against the source it is actually mirroring, and the file-level result is byte-identical to omnimarket#2110.How this got here
OMN-16249's repin had to rebuild the migrate image at a newer source rev to satisfy
SINGLE_SOURCE_REV_BUNDLE, which pulled this OMN-16146 migration into the deploy bundle for the first time. Courtesy note posted on OMN-16146.Refs OMN-16249, OMN-16146.
Evidence-Ticket: OMN-16249
Evidence-Source: OCC#6757