Repository navigation
fix(OMN-17887): retire the tenant schema; public is the TENANT domain's schema - #4079
Conversation
…'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.
jonahgabriel
added
the
hold:auto-merge
#11130) * evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#4079 * evidence: OCC companion self-bind for #11130 --------- Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
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): 7
Nit-level findings suppressed: 1
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: 7 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.
- [CRITICAL]
docker/application-domain-enforcement/seed.sql(glm-review) — Tenant owner now owns the schema containing internal-domain tables | ALTER SCHEMA public OWNER TO owner_onex_tenant consolidates ownership of thepublicschema in the tenant-domain owner. The omnino - [MAJOR]
docker/application-domain-enforcement/seed.sql(glm-review) — No deployable migration accompanies the schema-model change | The diff changes the proof seed, ownership manifests, topology instances and catalog sha256s for onex-dev and onex-prod, but no forward mi - [MAJOR]
src/omnibase_infra/validation/application_database_domain_enforcement.py(glm-review) — Tenant and internal families now share one namespace with collision-prone gates | Retiring thetenantschema collapses all tenant-domain declarations intopublic, which is also the physical home o - [MINOR]
scripts/validation/check_tenant_guc_domain_parity.py(glm-review) — Parity script resolution order can misclassify across databases | logical_domains resolves viarelation in internal_bridgebefore consulting the per-database declared domain, and the internal bridge - [MINOR]
scripts/validation/check_tenant_guc_domain_parity.py(glm-review) — Silent coercion of malformed schema spec entries | The new schema-domain map comprehension includes specs that are dicts or None and silently drops anything else. A schemas entry with a scalar value ( - [MINOR]
docker/application-domain-enforcement/seed.sql(glm-review) — USAGE on public granted where USAGE on tenant was scoped before | The seed rewrites GRANT USAGE ON SCHEMA tenant TO app_dashboard, tenant_projection_writer and onex_api into GRANT USAGE ON SCHEMA publ - [MINOR]
scripts/ci/prove_application_database_domain_enforcement.py(glm-review) — Proof cross-gate control relies on an untested error-string contract | The retired-tenant-schema control asserts the rejection reason "unknown topology schema" against lint_application_database_sql, b
|
| 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): 5
Nit-level findings suppressed: 1
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: 5 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]
src/omnibase_infra/validation/application_database_domain_enforcement.py:_record_sql_target(glm-review) — Removal of the blanket public-schema prohibition weakens a fail-closed control | Previously any application relation targeted atpublicwas refused outright, making public placement impossible regar - [MAJOR]
src/omnibase_infra/validation/application_database_domain_enforcement.py:validate_application_database_relation_states(glm-review) — No demonstrated RLS requirement for tenant relations now living in public | The deleted checkapplication relations are prohibited in publicpreviously forced every application relation out of publi - [MAJOR]
src/omnibase_infra/topology/application_database.py:_EXPECTED_SCHEMAS(glm-review) — Divergent treatment of public between the parity checker and application_database.py | check_tenant_guc_domain_parity.py was carefully reworked sopublicresolves to TENANT only per database, via ea - [MINOR]
scripts/validation/check_tenant_guc_domain_parity.py:_walk_database_table_blocks(glm-review) — Silent empty domain for malformed schemas entries in parity checker |_walk_database_table_blocksbuilds schema_domains with a comprehension acceptingspec is None, mapping a null spec to domain " - [MINOR]
scripts/ci/prove_application_database_domain_enforcement.py:_run_cross_gate_controls(glm-review) — Renamed cross-gate red control loses its former public-placement coverage | Thepublic-tablered control (CREATE TABLE public.events refused as prohibited) was replaced byretired-tenant-schema. T
…ted 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.
…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....
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): 5
Nit-level findings suppressed: 1
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: 5 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]
src/omnibase_infra/validation/application_database_domain_enforcement.py(glm-review) — SQL-target gate for public now fails open on ownership provenance | _record_sql_target previously refused every application relation target inpublic, failing closed regardless of census state. The - [MAJOR]
scripts/validation/check_tenant_guc_domain_parity.py(glm-review) — Per-database domain resolution not namespaced by database for relation keys | _walk_database_table_blocks correctly scopes the schema-to-domain map per database, but logical_domains accumulates result - [MINOR]
scripts/validation/check_tenant_guc_domain_parity.py(glm-review) — Internal bridge check precedes declared-domain check, contradicting per-database semantics | Resolution order in logical_domains is: NO_GUC schemas, then global internal_bridge membership, then the pe - [MINOR]
src/omnibase_infra/topology/application_database.py(glm-review) — No migration or verification path for existing tenant-schema objects during retirement | The diff removes thetenantschema from _EXPECTED_SCHEMAS, seed.sql, and every topology manifest, and rewrite - [MINOR]
scripts/validation/check_tenant_guc_domain_parity.py(glm-review) — Parity gate test coverage for schemas block with null or malformed entries | _walk_database_table_blocks builds schema_domains withif isinstance(spec, dict) or spec is None, mapping a null schemas
…ic, 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).
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): 5
Nit-level findings suppressed: 1
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: 5 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/application-domain-enforcement/seed.sql(glm-review) — Tenant data inpublicinherits default search_path resolution | Relocating the TENANT domain into thepublicschema removes a namespace defense-in-depth layer. Every role's default search_path inc - [MAJOR]
scripts/ci/prove_application_database_domain_enforcement.py(glm-review) — Public-prohibition red control deleted, replacement refusal unproven | The proof script's cross-gate controls previously included apublic-tablecontrol asserting a CREATE TABLE in public is refused - [MINOR]
src/omnibase_infra/validation/application_database_domain_enforcement.py(glm-review) — Public-target admission guard not demonstrably database-scoped | The new branch in _record_sql_target admits apublic.<name>target wheneverpublicappears inapplication_schemas. The excerpt do - [MINOR]
docker/application-domain-enforcement/seed.sql(glm-review) — Tenant owner now owns the defaultpublicschema | ALTER SCHEMA public OWNER TO owner_onex_tenant transfers ownership of the database's conventional default schema to the tenant-domain owner. Any too - [MINOR]
scripts/validation/check_tenant_guc_domain_parity.py(glm-review) — Parity resolution silently degrades when a database omits its schemas block | _walk_database_table_blocks defaults schema_domains to an empty dict when a database has noschemaskey. Under the old l
…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.
Conflict in .github/omnimarket-contract-pin.yaml: dev's refresh bot advanced the pin to 7112b8e8 (#4078); this branch pins 3441453c, omnimarket#2847's merge, which contains 7112b8e8. Kept 3441453c, the forward pin. After the merge: grant derivation --check --prove in sync on 3 instances, every rendered catalog passes --check, tenant GUC parity 0 violations; tests/unit/topology, tests/unit/validation, tests/ci: 6880 passed, 17 failed, all 17 also failing on untouched dev.
…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.
…ire-tenant-topology-r2 # Conflicts: # .github/omnimarket-contract-pin.yaml
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): 7
Nit-level findings suppressed: 1
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: 7 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]
src/omnibase_infra/validation/application_database_domain_enforcement.py, _TENANT_RELATIONS_REFERENCED_UNQUALIFIED and _record_sql_target(glm-review) — Unqualified-name lint allowance is global, not restricted to grandfathered files | The new _TENANT_RELATIONS_REFERENCED_UNQUALIFIED frozenset accepts unqualified references to 23 tenant-domain relatio - [MAJOR]
docker/application-domain-enforcement/seed.sql(glm-review) — Tenant data in the public schema collides with the default search_path namespace | Retiring the tenant schema and making public the TENANT domain means every role whose search_path defaults to "$user" - [MAJOR]
src/omnibase_infra/topology/application_database.py, _EXPECTED_SCHEMAS(glm-review) — No teardown or drift path for environments where a tenant schema already exists | The diff deletes the tenant schema from every topology catalog, _EXPECTED_SCHEMAS, and the proof seed, and the comment - [MINOR]
scripts/validation/check_tenant_guc_domain_parity.py, logical_domains(glm-review) — Internal bridge resolution now precedes the declared schema domain, changing precedence | In check_tenant_guc_domain_parity.logical_domains, the rewritten resolution order tests the internal bridge be - [MINOR]
scripts/ci/prove_application_database_domain_enforcement.py, _run_cross_gate_controls(glm-review) — Removed public-table red control narrows the cross-gate proof | _run_cross_gate_controls previously proved that CREATE TABLE public.events is refused. The replacement control proves CREATE TABLE tenan - [MINOR]
docker/application-domain-enforcement/seed.sql(glm-review) — RLS policies remain TO PUBLIC on tables in the public schema | The moved CREATE POLICY statements retain AS PERMISSIVE FOR ALL TO PUBLIC. Within the proof database this is belt-and-suspenders with FOR - [MINOR]
scripts/validation/check_tenant_guc_domain_parity.py, _walk_database_table_blocks(glm-review) — Per-database schema-domain resolution leaves databases without a schemas block unresolved | _walk_database_table_blocks yields an empty schema_domains map when a database lacks a schemas key. Every re
|
Close-out. Merged 2026-09-25T14:18:31Z as In short: I didn't answer the hostile reviewer's findings here before merge, and only one commit was made because of one of them: Point by pointThe reviewer made five passes: on 1. "Removing the blanket 2. "No demonstrated RLS requirement for tenant relations now in 3. "The tenant owner now owns the 4. "Tenant data in 5. "No deployable migration" / "no teardown or drift path where a 6. "The tenant and internal families now share one namespace, so the gates can collide" (major, 7. "The unqualified-name allowance is global, not limited to grandfathered files" / "the tenant name set is maintained twice" (major, final head; the second is the hint on the final head) 8. "The parity gate checks the internal bridge before the declared domain" (minor on 9. "The per-database result isn't keyed by database" (major, 10. Malformed or missing 11. " 12. "The 13. "The retired-tenant-schema control depends on an untested error string" (minor, 14. "RLS policies are still 15. "USAGE on 16. The five nit-level hints
The issue commentThe DEGRADED summary (09-24 23:09Z) belongs to the first pass, and everything in it is answered above. Thank you! |
2 participants
Summary
OMN-17887, step 3 of 4: retire the
tenantschema in omnibase_infra. Per the operator ruling on 2026-09-24, notenantPostgres schema will be built, and the TENANT domain's schema ispublicfor good.This builds on step 1, omnimarket#2847 (contracts and manifest declare
public; merged as3441453c), and step 2, omninode_infra#1660 (merged as8b8d5d3b).What changed
local,onex-devandonex-prodeach have 4schema: tenantUSAGE grant blocks, 12 in all. 6 are retargeted topublic(app_dashboard and onex_api, times 3), and 6 are dropped as duplicates of an existingpublicblock (tenant_projection_writer and validator_ro, times 3). The 3tenant:schemas entries are removed. The 9 rendered catalogs are regenerated, and_EXPECTED_SCHEMASand_EXPECTED_SCHEMA_OWNERSdroptenant.public. USAGE ontenantwas their only declared access to the TENANT domain, so it moves rather than goes. tenant_projection_writer and validator_ro already held USAGE onpublic, so theirtenantblock was a duplicate and is dropped. This was found while preparing step 4: omninode_infra's Kubernetes consumer check declares these two bindings' schema access, and the matching translation is[public, platform_catalog]. On the lakshman lane on .201, a read-only readback on 2026-09-24 at 19:16Z showsapp_dashboard=Ualready granted onpublic, and notenantschema.TENANT_TABLES_PHYSICALLY_IN_PUBLIC_UNTIL_OMN15359and its branch inphysical_grant_schema_for_tableare deleted, because tenant-domain relations are now declaredpublicdirectly. The 7 legacy migration declarations follow. The internal bridge is unchanged.publicoutright. Now:public.<name>SQL target is an ordinary application location, held to the exactly-one-ownership check, when the topology declarespublic.publicis refused unless its domain matches the schema's declared domain (TENANT), so a non-TENANT relation inpublicis still refused.tenant.<x>is refused as an unknown topology schema.schemas:block, instead of from the deleted bridge.safe_reporthash and audit id move topublic, and the obsoletepublic-tablered control becomesretired-tenant-schema.8b8d5d3b. The omnimarket contract pin moves forward from923b76bato step 1's merge,3441453c. Against that commit the grant check is in sync on all 9 profiles (69 pass, 0 fail, 0 residual each), so nothing is regenerated. After the 2026-09-25 merge of dev the pin is dev's8e5b1dbe(bot, chore(OMN-17292): advance omnimarket contract pin to 8e5b1dbeffb8 #4092), which contains3441453c; the 5 omnimarket commits between them change no contract, manifest or SQL file.pyproject.tomlanduv.lock.tenantnow pin the new truth. None were deleted or weakened, and new tests pin thattenantis refused.How it was verified
generate_application_database_table_grants.py --check --proveagainst step 1's contracts: 3 instances in sync, 69 passed and 0 failed on each of the 9 profiles. Against the oldtenantcontracts it fails (drift in 3 instances, 13 residuals per profile), which is the negative control.58b47971…from the old inputs.tenant: all 12 regressions fixed. The 3 that remain also fail on untouched dev.2d7ae9f62): the only conflict was the omnimarket pin, resolved to dev's8e5b1dbe. On the merged tree, the 14 test files CI runs in its application-database step, plus the two cohort-key test files, pass (632 tests), and the runner-image lock verifies. The SQL gate, run over a range that includes dev's new unqualified migrationnode_projection_savings/091, passes with 0 grandfathered.Local gates
--no-verify, noSKIP=, no--no-gpg-sign, nocore.hooksPathoverride. Every pre-commit and pre-push hook ran.Not in this change
tenantdrop migration. Onex-lab is read back first, per the ruling. Iftenantexists there and is empty, it is dropped with a node-owned migration, since a flat cross-database migration cannot be delivered on the k8s path.EXPECTED_SCHEMAS, and removing onex-lab'sCREATE SCHEMA tenant.tenantschema is pinned to historical revisions, independent of this topology.Evidence-Ticket: OMN-17887
Evidence-Source: OCC#11130