Repository navigation
feat(OMN-18930): vendor node_projection_delegation 0046, the delegation_events cohort-key columns - #4062
Conversation
…on_events cohort-key columns K3 of OMN-18925, projection half. omnimarket's node_projection_delegation migration 0046 adds three nullable delegation_events columns (cohort_key JSONB, cohort_key_sha256, cohort_key_refusal) so every row carries the delegation cohort key its terminal carried. Vendored here first, per the node-migration vendor-parity ordering, ahead of the omnimarket source PR. - the vendored file, produced by scripts/sync-node-migrations.sh from the omnimarket branch (one file updated, nothing else drifted) - its application-migrations.tsv row (tenant, checksum 93306936...) - its migration class, expand-only (OMN-19344), which is the checker's own --suggest reading of the file - the manifest count 212 -> 213 - a vendor pin test: exact bytes and manifest binding, the class line against the bytes, and the OMN-15361 application-database SQL gate on every shipped profile, with a positive control that the linter can fail Onex-Lane: k3-projection-omn18930-r2
|
No OCC evidence companion was minted for this PR. product PR is a draft; companion suppressed (F-17) Matched in the state: To clear this: Mark the PR ready for review. The ready_for_review trigger re-fires the born path and the companion mints then. Reported by |
…ibase_infra#4062 (#11080) * evidence(OMN-18930): author OCC companion for OmniNode-ai/omnibase_infra#4062 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 233222ea0daf5005a19ff9b358dc4420042b19cf. * evidence(OMN-18930): self-bind OCC#11080 + rebind contract_sha256 --------- Co-authored-by: omnimarket-bot <bot@omninode.ai>
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: glm-review
Models failed: codex
New finding threads: 0
Deduped (already posted on this PR): 0
Below quorum (one model only, reported not threaded): 6
Nit-level findings suppressed: 2
The model is the FINDER, never the gate: merge is gated only by the
deterministic Hostile Review Thread Gate, which blocks while
hostile-reviewer threads are unresolved. Resolve each thread after
addressing (or rejecting, with a reply) its finding.
Below quorum: 6 finding(s) raised by one model only (OMN-18479)
These are reported and NOT dropped, but they get no thread and do not block: a single model's finding no other model reproduced is not evidence enough to stop a merge. Read them; act on them if they are right.
- [MAJOR]
tests/unit/migrations/test_omn18930_cohort_key_vendor.py(glm-review) — No runtime verification of the migration itself | All four tests are static: byte hashing, regex parsing, and a linter gate. No test applies the migration to a real Postgres instance, re-applies it (i - [MAJOR]
tests/unit/migrations/test_omn18930_cohort_key_vendor.py(glm-review) — Regex-based purity check can be bypassed and misreads types | test_migration_is_declared_expand_only_and_is_purely_additive strips comment lines by requiring line-leading '--' and then regex-matches o - [MAJOR]
docker/migrations/forward/nodes/node_projection_delegation/0046_delegation_events_cohort_key.sql(glm-review) — Expand-only declared with no contract or backfill phase | The migration is classed 'expand-only' and the comment promises the key and digest on every row, but the three columns are permanently NULL fo - [MINOR]
docker/migrations/forward/nodes/node_projection_delegation/0046_delegation_events_cohort_key.sql(glm-review) — Unqualified table name in migration SQL | ALTER TABLE delegation_events does not schema-qualify the target. The linter evidently requires qualification for other objects (the positive-control rewrites - [MINOR]
tests/unit/migrations/test_omn18930_cohort_key_vendor.py(glm-review) — Positive control validates only one profile | test_migration_passes_the_application_database_sql_gate runs all four profiles, but test_the_linter_is_live_positive_control asserts non-zero violations o - [MINOR]
tests/unit/migrations/test_omn18930_cohort_key_vendor.py(glm-review) — Manifest parser loses rows and misparses columns silently | _manifest_rows splits each line at the first tab for the key but builds the value from row.split("\t"), dropping nothing yet discarding any
|
| Surface | Meaning | Blocks merge? |
|---|---|---|
| Review threads | Per-finding, posted by the reviewer | No (informational) |
Hostile Review Thread Gate |
Deterministic: unresolved hostile-reviewer threads exist | Fails until resolved (not yet a required context) |
degraded verdict |
Fewer than 2 models succeeded (infra) | No |
Powered by omniintelligence.review_pairing.cli_review — multi-model adversarial review: qwen3-review, qwen3-review-b, glm-review (OMN-8468/OMN-8524/OMN-17492)
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: glm-review
Models failed: codex
New finding threads: 0
Deduped (already posted on this PR): 0
Below quorum (one model only, reported not threaded): 8
Nit-level findings suppressed: 2
The model is the FINDER, never the gate: merge is gated only by the
deterministic Hostile Review Thread Gate, which blocks while
hostile-reviewer threads are unresolved. Resolve each thread after
addressing (or rejecting, with a reply) its finding.
Below quorum: 8 finding(s) raised by one model only (OMN-18479)
These are reported and NOT dropped, but they get no thread and do not block: a single model's finding no other model reproduced is not evidence enough to stop a merge. Read them; act on them if they are right.
- [MAJOR]
docker/migrations/forward/nodes/node_projection_delegation/0046_delegation_events_cohort_key.sql(glm-review) — Digest semantics defined in a comment, enforced nowhere | The migration encodes the cohort_key_sha256 canonicalization (sorted members, compact separators) only as a SQL comment. The writer that produ - [MAJOR]
docker/migrations/forward/nodes/node_projection_delegation/0046_delegation_events_cohort_key.sql(glm-review) — No constraint enforcing the refusal invariant | The comment states that a refused key leaves cohort_key and cohort_key_sha256 NULL, and that all three columns stay NULL when no key was carried. These - [MINOR]
docker/migrations/forward/nodes/node_projection_delegation/0046_delegation_events_cohort_key.sql(glm-review) — Expand phase with no visible contract or backfill plan | The migration is expand-only and pre-existing rows will have NULL cohort keys forever, per the comment. Cross-run comparison across the migrati - [MINOR]
docker/migrations/forward/nodes/node_projection_delegation/0046_delegation_events_cohort_key.sql(glm-review) — No index supporting cohort_key_sha256 lookups | The stated use is row-to-row comparison keyed by digest. equality lookups on cohort_key_sha256 over delegation_events will require a sequential scan. Ad - [MINOR]
tests/unit/migrations/test_omn18930_cohort_key_vendor.py(glm-review) — Substring forbidden-token check will produce false positives | test_migration_is_declared_expand_only_and_is_purely_additive rejects any occurrence of the substrings NOT NULL, DROP, UPDATE, DELETE, CR - [MINOR]
tests/unit/migrations/test_omn18930_cohort_key_vendor.py(glm-review) — Negative control validates the linter on only one profile | test_migration_passes_the_application_database_sql_gate asserts zero violations across four profiles, but the live-positive-control only pro - [MINOR]
tests/unit/scripts/validation/test_application_migration_manifest.py(glm-review) — Manifest row count assertion is a maintenance hazard | The hard-coded equality len(result.declarations) == 213 requires a manual bump on every added migration across the entire repository. Every futur - [MINOR]
tests/unit/migrations/test_omn18930_cohort_key_vendor.py(glm-review) — Blank-line lines in manifest parser produce malformed entries | _manifest_rows skips blank rows via the strip() check, which is correct, but rows with trailing whitespace-only fields or a missing hash
…unqualified in the SQL lint Removing the tenant bridge also removed the qualification lint's allowance for those 24 relations, so deployed, append-only migrations that name them without a schema started failing: the Application Database Domain Enforcement SQL gate refused node_projection_delegation/0046's bare ALTER TABLE delegation_events (#4062, run 36075133104). The allowance comes back as exactly what it was: the same 24 names, as a lint-only set that maps nothing to a schema. A qualified public.<name> target is still held to the ownership check. It is not widened to every public relation: 36 grandfathered 'must be schema-qualified' entries in application_database_sql_baseline.yaml name public tables outside it, and widening would turn them stale at the dev->main promotion. Tests: 0046's statement lints clean; an unknown name and a public-granted but never-bridged name (dispatch_eval_results) are still refused; the set is pinned to the retired bridge. Removing the allowance turns the 0046 test red. Domain proof PASS (46 red controls); unit/validation plus the SQL-gate, pin and enforcement-contract suites: 1580 passed.
…ets the retired tenant schema tests/unit/migrations/test_omn18930_cohort_key_vendor.py (from #4062) proved the linter live by rewriting 0046's target to public.pg_class and expecting a lint violation. That relied on the old outright ban on public. With public the TENANT domain's schema, a public.<name> target passes the lint and is held to the SQL gate's exactly-one-ownership check instead, like every other application schema (Tests Split 7/15, run 36080758457). The control now rewrites the target to the retired tenant schema, which the lint refuses as unknown, and separately asserts that public.pg_class becomes an ownership requirement the gate must satisfy. 4 passed; Split 7/15 was the only failing split in that run.
…'s schema (#4079) * fix(OMN-17887): retire the tenant schema; public is the TENANT domain's schema Operator ruling 2026-09-24: no tenant Postgres schema will be built and the TENANT domain lives in public for good (ADR-0027 amendment). Topology: the local, onex-dev and onex-prod instances drop the 12 `schema: tenant` USAGE grant blocks and the 3 `tenant:` schemas entries; the 9 rendered catalogs are regenerated. The expected-schema maps drop tenant. The TENANT_TABLES_PHYSICALLY_IN_PUBLIC_UNTIL_OMN15359 bridge and its consumers are removed: tenant-domain relations are declared public directly (omnimarket OMN-17887 step 1), and the 7 legacy migration declarations follow. Domain enforcement follows the topology instead of banning public: a public.<name> target is an ordinary application location held to the exactly-one-ownership check when the topology declares public, and a relation in public is refused unless its domain matches the declared schema domain (TENANT). The retired tenant schema is refused as unknown. The tenant GUC parity gate resolves domains per database from the topology's own schemas block; its domain map is identical to dev's (55 internal / 23 tenant / 3 unresolved, 0 violations). The domain enforcement proof seed, ownership config, audited function hash and red controls move to public (proof: PASS, 6 relations, 46 red controls). CI checks the omninode_infra ownership manifests at the OMN-17887 step 2 merge (8b8d5d3b). Tests that pinned tenant now pin the new truth; none were deleted or weakened, and new tests pin that tenant is refused. * fix(OMN-17887): advance omnimarket contract pin * chore(OMN-17887): bump release version * fix(OMN-17887): app_dashboard and onex_api keep schema USAGE, retargeted to public The retire commit removed the `schema: tenant` USAGE blocks outright. For tenant_projection_writer and validator_ro that was right: both already held USAGE on public, so the tenant block was a duplicate claim on the same domain. For app_dashboard and onex_api it was not: USAGE on tenant was their only declared access to the TENANT domain, and removing it left them declaring platform_catalog alone. "Tenant lives in public" means the declaration moves, not that it goes. Both principals now declare USAGE on public in local, onex-dev and onex-prod, and the 9 rendered catalogs are regenerated. Found by the omninode_infra Kubernetes consumer check: its schema_access for these two bindings reads [tenant, platform_catalog], and the honest translation is [public, platform_catalog], which the projection has to carry. Verified: grant derivation --check --prove in sync on 3 instances; tenant GUC parity 0 violations; domain-enforcement proof PASS on PostgreSQL 16 (6 relations, 4 pools, 46 red controls); unit topology, validation, ci and scripts suites show no failure that untouched dev does not also show. * chore(OMN-17887): restore final newlines and sync uv.lock to 0.38.58 496cd1b and d043526 were written without a trailing newline, which the end-of-file-fixer hook rejects, and d043526 bumped pyproject.toml to 0.38.58 without the matching uv.lock line, which uv rewrites on the next run. Content is otherwise unchanged: the pin stays at 3441453c. * chore(OMN-17887): rebind the runner image identity to the 0.38.58 uv.lock uv.lock participates in the runner image's shared_env_digest as raw bytes, so the release-version line moving to 0.38.58 made the recorded digest stale and runner-image-build-smoke refused the build (run 36073948714). Regenerated with scripts/ci/runner_image_identity.py --mode generate, as the version bumps on dev do (#3996): --mode verify exits 1 before and 0 after. shared_env_digest 99110eda... -> 105217eb...; identity v9 9e6e5ba6... -> 08efb837.... * test(OMN-17887): the OMN-17292 upstream-addition replay declares public, not the retired tenant test_an_upstream_contract_addition_cannot_red_an_unrelated_infra_pr replays an omnimarket merge that adds a db_io table and expects the grant check to go red. Its replay contract declared schema: tenant; with tenant retired that declaration is an unmappable residual, derives no grant, and the red half of the proof passed vacuously (Application Database Domain Enforcement, run 36074333168). public is the TENANT domain's schema, so the replay now declares public and derives a tenant_projection_writer grant the checked-in topology lacks, which is the drift the proof needs. Verified with omnimarket at the pin (3441453c) as .proof-dependencies: the test passes, and with the old tenant replay it fails again. The other eight suites that read .proof-dependencies: 119 passed, 1 skipped (needs GNU realpath, Linux only). * fix(OMN-17887): keep accepting the formerly-bridged tenant relations unqualified in the SQL lint Removing the tenant bridge also removed the qualification lint's allowance for those 24 relations, so deployed, append-only migrations that name them without a schema started failing: the Application Database Domain Enforcement SQL gate refused node_projection_delegation/0046's bare ALTER TABLE delegation_events (#4062, run 36075133104). The allowance comes back as exactly what it was: the same 24 names, as a lint-only set that maps nothing to a schema. A qualified public.<name> target is still held to the ownership check. It is not widened to every public relation: 36 grandfathered 'must be schema-qualified' entries in application_database_sql_baseline.yaml name public tables outside it, and widening would turn them stale at the dev->main promotion. Tests: 0046's statement lints clean; an unknown name and a public-granted but never-bridged name (dispatch_eval_results) are still refused; the set is pinned to the retired bridge. Removing the allowance turns the 0046 test red. Domain proof PASS (46 red controls); unit/validation plus the SQL-gate, pin and enforcement-contract suites: 1580 passed. * test(OMN-17887): the cohort-key vendor's linter positive control targets the retired tenant schema tests/unit/migrations/test_omn18930_cohort_key_vendor.py (from #4062) proved the linter live by rewriting 0046's target to public.pg_class and expecting a lint violation. That relied on the old outright ban on public. With public the TENANT domain's schema, a public.<name> target passes the lint and is held to the SQL gate's exactly-one-ownership check instead, like every other application schema (Tests Split 7/15, run 36080758457). The control now rewrites the target to the retired tenant schema, which the lint refuses as unknown, and separately asserts that public.pg_class becomes an ownership requirement the gate must satisfy. 4 passed; Split 7/15 was the only failing split in that run.
Summary
OMN-18930 (K3 of the delegation-reliability epic), projection half, vendor side. omnimarket's
node_projection_delegationmigration 0046 adds three nullabledelegation_eventscolumns, so every delegation row carries the cohort key its terminal carried:cohort_key(JSONB)cohort_key_sha256(TEXT)cohort_key_refusal(TEXT)This PR vendors that migration here first. The omnimarket node-migration vendor-parity gate refuses the source PR until the vendored copy is on omnibase_infra dev. The source PR is on branch
jonah/omn-18930-k3-projection-cohort-keyin omnimarket and opens as a draft after this one.The typed key model is omnibase_infra#4054, which is separate, queued and untouched here.
Changes
docker/migrations/forward/nodes/node_projection_delegation/0046_delegation_events_cohort_key.sql: produced byscripts/sync-node-migrations.shfrom the omnimarket branch. It printed1 file(s) updatedand nothing else drifted.docker/migrations/forward/_ledger/application-migrations.tsv: one row. It declares the file astenantwith checksum933069368adc0ce83a627a859d5360e7970fba91bdfbd73848fde3820199dd1c.config/migration_classes.yaml: the migration-class declaration (merged as omnibase_infra#4039 and fix(OMN-19344): declare migration 0045's class so the declaration check passes on dev #4052), set toexpand-only. That is the checker's own--suggestreading of the file (# additive).tests/unit/scripts/validation/test_application_migration_manifest.py: the declaration count goes from 212 to 213.tests/unit/migrations/test_omn18930_cohort_key_vendor.py, a new test that checks:ADD COLUMN IF NOT EXISTS, with no NOT NULL, DROP, UPDATE, DELETE or CREATE OR REPLACE);local,onex-dev,onex-prodandstability-testprofiles;Test plan
uv run --frozen pytest -q tests/unit/migrations/test_omn18930_cohort_key_vendor.py tests/unit/migrations/test_omn19013_terminal_construction_vendor.py tests/unit/scripts/validation/test_application_migration_manifest.py=>28 passed.origin/dev(git cat-file -efails), so the pin test and the 213 count both fail there.Runtime-affecting
Yes. Run on this branch,
scripts/trigger_rebuild_on_merge.py --dry-runprintedRuntime change detected, matchingdocker/migrations/**on the manifest and the vendored file. A README-only control printedNo rebuild trigger. This PR is labelledhold:auto-merge, is never armed by this lane, and merges only through the runtime-train.Lab readback
omnibase-infra, databaseomnidash_analytics, tablepublic.delegation_events(fully migrated through 0045, 1139 rows before).omnimarket-projection-delegation-writer. Lease HOLD2026-09-24T13:09:15Z-k3-projection-omn18930-r2, released PASS withrestored=yes.psql -fdirectly, not through the migration runner, so no checksum row was recorded. The result wasALTER TABLE, three nullable columns, and the existing rows kept.origin/dev.delegate-skill-completedterminals were produced to partition 0, offsets 953 and 954. They are captured terminal A (build0ddd10ca) and captured terminal B (builda7ea811c), each carrying the key the omnibase_infra#4054 assembler builds from that capture, under fresh correlation ids.49ab6618-8ac8-42e1-8cae-4207d4dc3480: a 10-dimension key withcohort_key_sha25670236fdd....af5e8c68-6ec8-42e2-b666-78a24a85346d:3f8d5b95....build_identityand nothing else.c96acb18has all three columns NULL.e60d5b41,21fbee1dandc60afcb1, which equal dev. The fold file was removed and the writer restarted healthy.platform_catalog.schema_migrations.onex-api. The proof wrote as the lane writer and read aspostgres. This change adds columns only. It adds no policy and no grant, and it does not change the read path.Related issues
OMN-18930
Evidence-Ticket: OMN-18930
Evidence-Source: OCC#11080