Repository navigation
feat(runtime): type database ownership wiring (OMN-15418) - #2548
jonahgabriel merged 9 commits into
Conversation
📝 WalkthroughWalkthroughThis change adds PostgreSQL 16 ACL generation and proof workflows, typed application database topology and ownership validation, cryptographic tenant authority for projection writes, topology-backed adapter wiring, dynamic contract materialization, and extensive unit and integration coverage. ChangesDatabase governance and proofs
Runtime topology and projection dispatch
Validation and integration coverage
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant ServiceKernel
participant RuntimeHostProcess
participant KafkaContractSource
participant HandlerWiring
participant ProjectionAdapter
participant PostgreSQL
ServiceKernel->>RuntimeHostProcess: provide deployment topology
RuntimeHostProcess->>KafkaContractSource: materialize cached db_io contract
KafkaContractSource->>HandlerWiring: authorize dynamic topology wiring
HandlerWiring->>ProjectionAdapter: resolve target and tenant authority
ProjectionAdapter->>PostgreSQL: attest identity and execute RLS-scoped operation
PostgreSQL-->>ProjectionAdapter: return database result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
a0bced6 to
f02cfc8
Compare
9236ab4 to
ec71d67
Compare
|
| 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)
#5635) * evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#2548 * evidence: OCC companion self-bind for #5635 --------- Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
* feat(runtime): split projection database domains (OMN-15421) * fix(runtime): register tenant binding resolver protocol (OMN-15421) --------- Co-authored-by: jonahgabriel <jonahgabriel@users.noreply.github.com>
* feat(database): generate application ACL matrix * fix(database): satisfy ACL type gates * test(database): register ACL role-state protocol --------- Co-authored-by: t <t@t.t>
* test(runtime): isolate config prefetch environment * fix(database): preserve ACL proof credentials --------- Co-authored-by: t <t@t.t>
7228ce0 to
b65dd28
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/integration/runtime/test_projection_handler_db_injection_integration.py (1)
391-406: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winThese two tests no longer exercise
_wire_handler_entry.Both docstrings state that
_wire_handler_entryselects the projection callback or the standard path. Neither body calls_wire_handler_entry. They now assert only thatModelDiscoveredContract.db_ioholds the values passed to the constructor, which Pydantic already guarantees. The branch selection that this PR changes is left uncovered. Either call the wiring function and assert which callback it produces, or rename the tests to describe the contract-shape assertion they now make.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/runtime/test_projection_handler_db_injection_integration.py` around lines 391 - 406, Update test_wire_handler_entry_uses_projection_path_when_db_io_declared and test_wire_handler_entry_uses_standard_path_when_no_db_io to invoke _wire_handler_entry and assert the selected projection versus standard callback, covering the changed branch. Alternatively, if these tests should remain contract-shape checks, rename them and revise their docstrings to describe only the ModelDiscoveredContract.db_io assertions.
🧹 Nitpick comments (29)
tests/unit/validation/test_application_relation_ownership.py (1)
141-143: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTighten the
matchpattern for the strict-declaration test.
match="database"accepts anyValidationErrorwhose text contains "database". The test then passes even ifparallel-location.yamlfails for an unrelated reason, such as a different missing field. Assert the specific field and error type that the strictModelDbTableDeclarationproduces.♻️ Suggested tightening
def test_service_loader_reuses_strict_typed_table_declaration() -> None: - with pytest.raises(ValidationError, match="database"): - load_service_ownership_manifest(_FIXTURES / "parallel-location.yaml") + with pytest.raises(ValidationError) as excinfo: + load_service_ownership_manifest(_FIXTURES / "parallel-location.yaml") + + assert {error["loc"][-1] for error in excinfo.value.errors()} == {"database_ref"}Replace
database_refwith the exact field thatparallel-location.yamlviolates.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/validation/test_application_relation_ownership.py` around lines 141 - 143, Update test_service_loader_reuses_strict_typed_table_declaration to replace the broad match="database" pattern with the exact database_ref field and validation error text produced by ModelDbTableDeclaration for parallel-location.yaml, ensuring unrelated ValidationError messages cannot satisfy the test.src/omnibase_infra/validation/application_database_acl.py (1)
4-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the module docstring with the hardcoded database scope.
Lines 6-9 state that no allowlist is maintained in this module. Lines 108-119 define
_DEPLOYMENT_CONNECT_DATABASES, a fixed eight-database CONNECT scope that gates deployment authorization at line 916. State this exception in the docstring, or derive the set from the locked topology so the two cannot drift.Also applies to: 108-119
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/omnibase_infra/validation/application_database_acl.py` around lines 4 - 10, The module docstring incorrectly claims no database allowlist is maintained while `_DEPLOYMENT_CONNECT_DATABASES` defines the fixed CONNECT scope used by deployment authorization. Update the docstring to explicitly document this hardcoded eight-database exception, or replace the constant with the locked topology’s derived database set so the scope cannot drift.src/omnibase_infra/runtime/auto_wiring/handler_wiring.py (2)
5530-5548: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the typed-then-YAML DLQ resolution into one helper.
Both sites resolve DLQ topics with the same rule: prefer
contract.event_bus.dlq_topics, then fall back to_read_dlq_topics(contract.contract_path). The docstring of_contract_provision_topicsstates that the provisioned string must stay byte-identical to the routing target "by construction". Two copies of the rule can drift. Extract one helper, for example_resolve_dlq_topics(contract) -> tuple[str, ...], and call it from both places.Also applies to: 6846-6857
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/omnibase_infra/runtime/auto_wiring/handler_wiring.py` around lines 5530 - 5548, The typed-then-YAML DLQ topic resolution is duplicated and must be centralized. Add a helper such as _resolve_dlq_topics(contract) that prefers contract.event_bus.dlq_topics and otherwise reads contract.contract_path, preserving the existing fallback/error behavior, then replace the resolution logic at the shown site and _contract_provision_topics with calls to that helper so both provisioning and routing use the same result.
2762-2782: 🚀 Performance & Scalability | 🔵 TrivialConsider a pooled or reused connection path for projection dispatch.
The wiring builds a new
ProjectionDatabaseOperationsfor every dispatched event, and thefinallyblock closes every per-binding connection after each event. Each event therefore pays a TCP/TLS connect, an authentication round trip, and theSELECT current_user, current_database()attestation query per binding. On a high-volume projection topic this adds a fixed per-message latency floor and increases backend churn on PostgreSQL.A per-binding pool with attestation performed once per physical connection would keep the identity guarantee and remove the per-event connect cost. This is deployment guidance, not a blocker for this PR.
Also applies to: 2908-2912, 2922-2954
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/omnibase_infra/runtime/auto_wiring/handler_wiring.py` around lines 2762 - 2782, Reuse pooled database connections for projection dispatch instead of creating and closing a new ProjectionDatabaseOperations connection for every event. Update the projection wiring and cleanup paths around the dispatch flow and its finally block to acquire connections from a per-binding pool, while retaining attestation (SELECT current_user, current_database()) once for each physical connection before reuse.src/omnibase_infra/runtime/projection_tenant_authority.py (2)
30-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInfra errors are raised without
ModelInfraErrorContext. Both new failure paths raise a typed infrastructure error with a message only, so neither failure carries a correlation id into logs. The coding guidelines require the narrowest infrastructure error class together with a context built byModelInfraErrorContext.with_correlation(...).
src/omnibase_infra/runtime/projection_tenant_authority.py#L30-L33: build the context fromenvelope.trace_idand pass it toProjectionTenantContextError, and keep tenant identifiers and signature material out of the message.src/omnibase_infra/runtime/service_kernel.py#L2637-L2648: passcontext=ModelInfraErrorContext.with_correlation(correlation_id=correlation_id, ...)to theRuntimeHostError, matching the otherRuntimeHostErrorraises inbootstrap().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/omnibase_infra/runtime/projection_tenant_authority.py` around lines 30 - 33, Update _tenant_context_error in src/omnibase_infra/runtime/projection_tenant_authority.py#L30-L33 to build ModelInfraErrorContext with envelope.trace_id and pass it to ProjectionTenantContextError, keeping tenant identifiers and signature material out of the message. Update the RuntimeHostError raise in src/omnibase_infra/runtime/service_kernel.py#L2637-L2648 to pass context=ModelInfraErrorContext.with_correlation(correlation_id=correlation_id, ...), matching the other RuntimeHostError raises in bootstrap().Source: Coding guidelines
180-190: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider an explicit freshness window on
emitted_at.The minted capability carries
emitted_atbut no code rejects an old envelope. A captured signed envelope stays acceptable forever. Theevent_envelope_idand payload-hash binding restricts a replay to the same event, so the current exposure is limited. An explicit maximum age still narrows the window and makes the intent visible.🔒️ Sketch of an age check
+_MAX_AUTHORITY_AGE = timedelta(minutes=5) + def verify_signed_projection_tenant_authority( envelope: object, key_provider: ProtocolKeyProvider, tenant_binding_resolver: ProtocolProjectionTenantBindingResolver, ) -> VerifiedProjectionTenantAuthority: @@ + if datetime.now(UTC) - envelope.emitted_at > _MAX_AUTHORITY_AGE: + raise _tenant_context_error( + "Projection tenant envelope is outside the accepted freshness window" + ) tenant_id = parse_canonical_tenant_uuid(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/omnibase_infra/runtime/projection_tenant_authority.py` around lines 180 - 190, Update the projection tenant authority verification flow that constructs VerifiedProjectionTenantAuthority to validate envelope.emitted_at against an explicit maximum freshness window before minting the capability. Reject stale envelopes while preserving the existing event_envelope_id and payload-hash bindings, and define or reuse a clearly named configuration constant for the allowed age.tests/unit/runtime/test_kafka_contract_source.py (1)
1229-1237: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that both string replacements changed the YAML.
Both
str.replacecalls depend on the exact indentation and quoting emitted by_market_contract_yaml. If that helper changes, each call becomes a silent no-op. The test then still passes while no longer exercising the version-less routing payload or the stringevent_modelform. Add explicit preconditions.♻️ Proposed precondition assertions
- contract_yaml = _market_contract_yaml( + base_yaml = _market_contract_yaml( routing_module=self._MODULE, routing_class=self._CLASS, - ).replace( + ) + contract_yaml = base_yaml.replace( " version:\n major: 1\n minor: 0\n patch: 0\n", "", ) - contract_yaml = contract_yaml.replace( + assert contract_yaml != base_yaml, "handler_routing version block not removed" + with_event_model = contract_yaml.replace( ' - operation: "sweep"\n', ' - operation: "sweep"\n' ' event_model: "omnimarket.models.ModelSweepRequested"\n', ) + assert with_event_model != contract_yaml, "event_model not injected" + contract_yaml = with_event_model🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/runtime/test_kafka_contract_source.py` around lines 1229 - 1237, Update the test around the two contract_yaml str.replace calls to assert that each expected source fragment is present before replacement, ensuring both replacements actually modify the YAML. Add explicit precondition assertions for the version block removed by the first replace and the sweep operation line replaced by the second, while preserving the existing replacement behavior.src/omnibase_infra/runtime/runtime_host_process.py (1)
3446-3453: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFail loudly when a
db_iocontract has no deployment topology.
topologyresolves toNonewhen the runtime was constructed withoutdeployment_topology. The docstring ofmaterialize_cached_contractstates that topology is required when the contract declaresdb_io. In that case the call still runs and the result is reported only as a generic rejection at line 3459, so operators cannot tell a missing-topology misconfiguration from a wiring failure. Add an explicit precondition check with a named reason.♻️ Proposed guard
source = getattr(self, "_kafka_contract_source", None) dispatch_engine = getattr(self, "_dispatch_engine", None) if source is None or dispatch_engine is None: logger.warning( "Cannot materialize db_io contract without Kafka source and " "dispatch engine", extra={ "node_name": node_name, "correlation_id": str(correlation_id), }, ) return False + + topology = getattr(self, "_deployment_topology", None) + if topology is None: + logger.warning( + "Cannot materialize db_io contract without deployment topology", + extra={ + "node_name": node_name, + "correlation_id": str(correlation_id), + }, + ) + return False @@ materialization = await source.materialize_cached_contract( node_name=node_name, dispatch_engine=dispatch_engine, event_bus=getattr(self, "_event_bus", None), environment=source.environment, container=getattr(self, "_container", None), - topology=getattr(self, "_deployment_topology", None), + topology=topology, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/omnibase_infra/runtime/runtime_host_process.py` around lines 3446 - 3453, Before calling materialize_cached_contract in the surrounding runtime flow, detect when the contract declares db_io and deployment topology is unavailable; fail immediately with an explicit, named reason identifying the missing topology. Preserve the existing materialization path when topology is present or the contract does not require db_io.src/omnibase_infra/runtime/auto_wiring/models/model_db_table_validation_warning.py (1)
20-20: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueRemove the
schemashadowing warning.The locked Pydantic 2.13.4 accepts this field but emits a
UserWarningbecause it shadowsBaseModel.schema(). UseField(alias="schema")if import-time warnings must be clean.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/omnibase_infra/runtime/auto_wiring/models/model_db_table_validation_warning.py` at line 20, Update the schema field declaration in the model to use a non-shadowing Python attribute with Pydantic Field(alias="schema"), preserving the external alias while eliminating the BaseModel.schema() warning; adjust any related model configuration or references as needed to support population and serialization through the alias.tests/unit/runtime/auto_wiring/test_projection_domain_adapters.py (1)
61-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAnnotate the helpers with
ProjectionDatabaseTargetand drop the suppressions.
_adapter,_verified_adapter, and_targettypetargetasobject, which forces the twotype: ignore[arg-type]comments at lines 70-71. The concrete type is already importable from the same module you import_build_projection_db_adapterfrom.♻️ Proposed refactor
from omnibase_infra.runtime.auto_wiring.handler_wiring import ( + ProjectionDatabaseTarget, _build_projection_db_adapter, _resolve_projection_database_target, )def _adapter( - target: object, + target: ProjectionDatabaseTarget, *, authority: VerifiedProjectionTenantAuthority | None = None, tenant_event: object | None = None, default_url: str = "postgresql://fixture", **binding_urls: str, ) -> object: return _build_projection_db_adapter( - projection_database_urls(target, default_url, **binding_urls), # type: ignore[arg-type] - target, # type: ignore[arg-type] + projection_database_urls(target, default_url, **binding_urls), + target, authority, tenant_event, )Apply the same annotation to
_verified_adapterand the_targetreturn type.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/runtime/auto_wiring/test_projection_domain_adapters.py` around lines 61 - 96, Annotate the target parameters and helper return values in _adapter, _verified_adapter, and _target with ProjectionDatabaseTarget, importing it from the module that provides _build_projection_db_adapter. Remove the unnecessary type: ignore[arg-type] suppressions from _adapter once the concrete type is used.tests/helpers/application_db_topology.py (1)
125-126: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePreserve sibling database entries when you copy the topology.
model_copy(update={"databases": {"application": database}})replaces the whole mapping. Todaytopology.yamldeclares onlyapplication, so behavior is correct. If the fixture later declares a second database, every target that references it fails withUnknown database_ref, and the cause is this helper. Merge instead of replace.♻️ Proposed refactor
database = database.model_copy(update={"principals": principals}) - return topology.model_copy(update={"databases": {"application": database}}) + databases = dict(topology.databases) + databases["application"] = database + return topology.model_copy(update={"databases": databases})Apply the same merge at lines 156-160.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/helpers/application_db_topology.py` around lines 125 - 126, Update the topology-copy logic in the helper to merge the updated application database into the existing topology.databases mapping instead of replacing the mapping, preserving all sibling database entries. Apply the same merge behavior in the related topology-copy logic around the second database update referenced by the comment.tests/integration/projectors/test_handler_wiring_projection_dispatch.py (1)
49-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the stale
os.environ.getpatch and rename the local variable.
_make_projection_dispatch_callbacknow reads DSNs at construction time, and the construction happens outside thepatch(_PATCH_ENVIRON_GET, ...)block. The autouse fixture supplies the DSN. The_PATCH_ENVIRON_GETcontext therefore has no effect on these tests. Also,db_tablesnow holds aProjectionDatabaseTarget, not a table list;db_targetstates the type.♻️ Proposed refactor for lines 49-66
- db_tables = projection_database_target("node_service_registry") + db_target = projection_database_target("node_service_registry") handler = FakeProjectionHandler() callback = _make_projection_dispatch_callback( - handler, db_tables, ("onex.evt.platform.node-heartbeat.v1",) + handler, db_target, ("onex.evt.platform.node-heartbeat.v1",) ) @@ fake_adapter = MagicMock() - with patch( - _PATCH_ENVIRON_GET, - return_value="postgresql://postgres:test@localhost:5436/omnidash_analytics", - ): - with patch(_PATCH_BUILD_ADAPTER, return_value=fake_adapter): - asyncio.run(callback(envelope)) + with patch(_PATCH_BUILD_ADAPTER, return_value=fake_adapter): + asyncio.run(callback(envelope))Apply the same change at line 96 and in
tests/integration/projectors/test_projection_terminal_event_emission.py.Also applies to: 96-96
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/projectors/test_handler_wiring_projection_dispatch.py` at line 49, Remove the stale patch(_PATCH_ENVIRON_GET, ...) context from the affected tests because DSNs are resolved during _make_projection_dispatch_callback construction outside that block and the autouse fixture provides them. Rename the local variable db_tables to db_target at both referenced locations and apply the same updates in test_projection_terminal_event_emission.py.tests/unit/runtime/test_service_kernel_database_topology_wiring.py (1)
82-86: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the fail-closed behavior instead of the source text.
These assertions compare exact substrings of
service_kernel.py, including"if db_io_contracts and deployment_topology is None:". A formatter that wraps that condition, or a rename ofdb_io_contracts, breaks the test without changing behavior. The test then reports a defect that does not exist.The guarantee under test is that a
db_iocontract without a resolved topology fails closed.tests/unit/runtime/test_handler_wiring_state_io.pylines 197-202 already assert that guarantee behaviorally withpytest.raises(ModelOnexError, match="no checked-in ModelDeploymentTopology"). Prefer the same approach here, and keep a source assertion only for the operator-facing message text.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/runtime/test_service_kernel_database_topology_wiring.py` around lines 82 - 86, Update test_db_io_boot_requires_explicit_profile to exercise service-kernel startup with a db_io contract and no resolved deployment topology, asserting pytest.raises(ModelOnexError, match="no checked-in ModelDeploymentTopology") instead of matching implementation source text; retain only a source assertion for the operator-facing "ONEX_DATABASE_TOPOLOGY_PROFILE is not set" message.tests/unit/runtime/auto_wiring/test_handler_wiring_db_injection.py (1)
447-456: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the mock-connection setup into one helper.
Lines 447-456, 483-492, 522-531, 588-597, and 709-718 repeat the same six-line connection double, including the
("omninode_runtime", "omnidash_analytics")attestation tuple. If the attestation query or its expected shape changes, you must edit five copies.tests/unit/runtime/auto_wiring/test_projection_domain_adapters.pylines 45-58 already defines an equivalent_connectionhelper.♻️ Proposed refactor
Add one helper next to
_internal_db_adapter:def _attested_connection() -> tuple[MagicMock, MagicMock]: """Return a psycopg2 connection double that passes identity attestation.""" cursor = MagicMock() cursor.fetchone.return_value = ("omninode_runtime", "omnidash_analytics") cursor_context = MagicMock() cursor_context.__enter__.return_value = cursor conn = MagicMock() conn.closed = False conn.cursor.return_value = cursor_context return conn, cursorThen reduce each call site:
- cursor = MagicMock() - cursor.fetchone.return_value = ("omninode_runtime", "omnidash_analytics") - cursor_context = MagicMock() - cursor_context.__enter__.return_value = cursor - conn = MagicMock() - conn.closed = False - conn.cursor.return_value = cursor_context - - with patch("psycopg2.connect", return_value=conn): + conn, cursor = _attested_connection() + + with patch("psycopg2.connect", return_value=conn): adapter = _internal_db_adapter("savings_estimates")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/runtime/auto_wiring/test_handler_wiring_db_injection.py` around lines 447 - 456, Extract the repeated attested psycopg2 mock setup into an _attested_connection helper adjacent to _internal_db_adapter, returning both the connection and cursor while preserving the existing identity tuple and context-manager behavior. Replace all five duplicated setup blocks in this test module with calls to the helper, keeping each patch and test-specific assertion unchanged.tests/integration/db/test_projection_writer_tenant_rls_omn15301.py (1)
284-297: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the normalized tenant value, not only the return code.
The test name states that a matching tenant string becomes a UUID. The body only asserts that
upsertreturnsTrue, so the normalization it names is never observed. Read the owner-side row and assert the stored type and value.♻️ Proposed assertion
def test_equal_tenant_string_is_replaced_with_uuid(domain_dsns: dict[str, str]) -> None: adapter = _tenant_adapter(domain_dsns["tenant"], TENANT_A) + correlation_id = uuid4() try: assert adapter.upsert( TENANT_TABLE, "correlation_id", { - "correlation_id": uuid4(), + "correlation_id": correlation_id, "task_type": "proof", "tenant_id": str(TENANT_A), }, ) finally: adapter.close() + assert _tenant_rows(domain_dsns["owner"]) == [(correlation_id, TENANT_A)]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/db/test_projection_writer_tenant_rls_omn15301.py` around lines 284 - 297, Update test_equal_tenant_string_is_replaced_with_uuid to read the owner-side row after the upsert and assert that the stored tenant_id is a UUID with the expected TENANT_A value, rather than only asserting the upsert return value.tests/integration/runtime/test_projection_handler_db_injection_integration.py (1)
54-66: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe written
contract.yamlomits thedb_ioblock that the model declares.
_make_contractwrites YAML withoutdb_io, then constructsModelDiscoveredContractwith aModelDbOwnershipSubcontractwhendeclare_db_io=True. Any code that re-readscontract_pathsees a contract with no database ownership. Add thedb_ioblock to the written text so the file and the model agree.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/runtime/test_projection_handler_db_injection_integration.py` around lines 54 - 66, Update _make_contract so the YAML written to contract_path includes the db_io ownership block whenever declare_db_io is true, matching the ModelDbOwnershipSubcontract supplied to ModelDiscoveredContract. Preserve the existing YAML for contracts that do not declare database I/O.tests/unit/runtime/auto_wiring/test_db_table_validator.py (1)
39-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd the return annotation for consistency.
_contractdeclares its return type._tabledoes not. Annotate it as-> ModelDbTableDeclaration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/runtime/auto_wiring/test_db_table_validator.py` at line 39, Update the _table helper signature to include the return annotation -> ModelDbTableDeclaration, matching the existing annotation on _contract while leaving its behavior unchanged..github/workflows/docker-build.yml (1)
81-114: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHoist
COMPOSE_PROJECT_NAMEto job level.
COMPOSE_PROJECT_NAMEis set separately in two steps (Lines 99-100 and 108-109). Declare it once as job-levelenv:, as therebuilt-postgres16-proofjob inapplication-acl-postgres16-proof.ymldoes. This removes duplication and keeps both workflows consistent.♻️ Proposed refactor
domain-adapter-proof: name: Projection Domain Adapter Proof (OMN-15421) + env: + COMPOSE_PROJECT_NAME: omn15421-${{ github.run_id }}-${{ github.run_attempt }} runs-on: >- ${{ (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository) && fromJSON(vars.OMNI_PUBLIC_PR_RUNS_ON_JSON || '["ubuntu-latest"]') || fromJSON(vars.OMNI_DOCKER_CI_RUNS_ON_JSON || vars.OMNI_TRUSTED_CI_RUNS_ON_JSON || '["self-hosted","omnibase-ci"]') }} timeout-minutes: 15 steps: - name: Checkout code uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Verify Docker socket access run: docker info - name: Run rebuilt-image PostgreSQL 16 proof - env: - COMPOSE_PROJECT_NAME: omn15421-${{ github.run_id }}-${{ github.run_attempt }} run: >- docker compose -f docker/domain-adapter-proof/compose.yml up --build --abort-on-container-exit --exit-code-from proof - name: Remove proof containers and volumes if: always() - env: - COMPOSE_PROJECT_NAME: omn15421-${{ github.run_id }}-${{ github.run_attempt }} run: >- docker compose -f docker/domain-adapter-proof/compose.yml down --volumes --remove-orphans🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/docker-build.yml around lines 81 - 114, Move COMPOSE_PROJECT_NAME from the env blocks of the “Run rebuilt-image PostgreSQL 16 proof” and “Remove proof containers and volumes” steps to a single job-level env declaration in the Projection Domain Adapter Proof job, preserving the existing value and ensuring both steps inherit it.src/omnibase_infra/validation/models/model_application_inventory_relation.py (1)
20-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
from_attributes=Trueto the newmodel_*.pyclasses. All six new models in this cohort setConfigDict(frozen=True, extra="forbid")but omitfrom_attributes=True, which the coding guideline forsrc/omnibase_infra/**/model_*.pycalls for. Add the setting to each class'sConfigDictso the models can be built from attribute-bearing objects, not only from dicts/JSON.
src/omnibase_infra/validation/models/model_application_inventory_relation.py#L20-L53: addfrom_attributes=TruetoModelApplicationInventoryRelation.model_config.src/omnibase_infra/validation/models/model_application_inventory_relation_counts.py#L9-L22: addfrom_attributes=TruetoModelApplicationInventoryRelationCounts.model_config.src/omnibase_infra/validation/models/model_application_relation_declaration.py#L22-L44: addfrom_attributes=TruetoModelApplicationRelationDeclaration.model_config.src/omnibase_infra/validation/models/model_application_relation_evidence_inventory.py#L25-L58: addfrom_attributes=TruetoModelApplicationRelationEvidenceInventory.model_config.src/omnibase_infra/validation/models/model_internal_tenant_column_transform.py#L13-L24: addfrom_attributes=TruetoModelInternalTenantColumnTransform.model_config.src/omnibase_infra/validation/models/model_live_application_relation.py#L21-L45: addfrom_attributes=TruetoModelLiveApplicationRelation.model_config.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/omnibase_infra/validation/models/model_application_inventory_relation.py` around lines 20 - 53, Enable attribute-based validation by adding from_attributes=True to the ConfigDict for ModelApplicationInventoryRelation in src/omnibase_infra/validation/models/model_application_inventory_relation.py:20-53, ModelApplicationInventoryRelationCounts in src/omnibase_infra/validation/models/model_application_inventory_relation_counts.py:9-22, ModelApplicationRelationDeclaration in src/omnibase_infra/validation/models/model_application_relation_declaration.py:22-44, ModelApplicationRelationEvidenceInventory in src/omnibase_infra/validation/models/model_application_relation_evidence_inventory.py:25-58, ModelInternalTenantColumnTransform in src/omnibase_infra/validation/models/model_internal_tenant_column_transform.py:13-24, and ModelLiveApplicationRelation in src/omnibase_infra/validation/models/model_live_application_relation.py:21-45, preserving their existing frozen and extra-forbid settings.Source: Coding guidelines
tests/ci/test_application_database_acl_contract.py (1)
19-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
ModelApplicationDatabaseAclSourcefrom its own module.
model_application_database_acl_matrixbinds this name only because it imports it for its own field types, and its__all__lists justModelApplicationDatabaseAclMatrix. The import works today, but it breaks if that transitive import is ever removed or restructured. Import the symbol from its defining module.♻️ Proposed import fix
from omnibase_infra.validation.models.model_application_database_acl_matrix import ( ModelApplicationDatabaseAclMatrix, - ModelApplicationDatabaseAclSource, +) +from omnibase_infra.validation.models.model_application_database_acl_source import ( + ModelApplicationDatabaseAclSource, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/ci/test_application_database_acl_contract.py` around lines 19 - 22, Update the imports in test_application_database_acl_contract.py so ModelApplicationDatabaseAclSource is imported from its defining module rather than model_application_database_acl_matrix; keep ModelApplicationDatabaseAclMatrix imported from the matrix module.src/omnibase_infra/validation/models/model_application_database_acl_matrix.py (1)
46-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
from_attributes=Trueto the model config.The repository guideline requires
model_<name>.pymodels to generally use frozen, strict PydanticConfigDictsettings withfrom_attributes=True. This config setsfrozen=Trueandextra="forbid"only. The same omission repeats in the sibling models added by this PR (model_application_database_acl_object.py,model_application_database_acl_source.py,model_application_database_default_acl_row.py,model_application_database_role_membership.py), so apply the change consistently or record the deviation.As per coding guidelines: "Model files named
model_<name>.pymust defineModel<Name>and generally use frozen, strict PydanticConfigDictsettings withfrom_attributes=True."♻️ Proposed config change
- model_config = ConfigDict(frozen=True, extra="forbid") + model_config = ConfigDict(frozen=True, extra="forbid", from_attributes=True)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/omnibase_infra/validation/models/model_application_database_acl_matrix.py` at line 46, Update the ConfigDict declarations in the affected Model classes, including the ACL matrix and sibling model files, to retain frozen and extra="forbid" while adding from_attributes=True consistently.Source: Coding guidelines
src/omnibase_infra/validation/models/model_application_database_principal_inventory.py (1)
158-173: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse the set computed at lines 119-123.
classified_present_rolesrepeats the exact expression already assigned toclassified_activity_roles. Keep one name and one computation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/omnibase_infra/validation/models/model_application_database_principal_inventory.py` around lines 158 - 173, In the validation logic around classified_activity_roles, reuse that existing set for the unclassified_catalog_owners subtraction instead of recomputing it as classified_present_roles. Remove the duplicate assignment and preserve the existing owner classification and ValueError behavior.src/omnibase_infra/validation/types/type_application_database_function_signature.py (1)
32-36: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winReject all control characters inside quoted identifiers.
The quoted branch rejects only
\n,\r, and\x00. Characters such as\t,\x0b,\x0c, and\x1bstill pass into rendered SQL and into evidence log lines. Test one property instead of three members.🛡️ Proposed fix to widen the control-character check
if quoted: - if character in {"\n", "\r", "\x00"}: + if not character.isprintable(): raise ValueError( "function_signature quoted identifiers cannot contain controls" )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/omnibase_infra/validation/types/type_application_database_function_signature.py` around lines 32 - 36, Update the quoted-identifier validation in the function-signature parser to reject any control character, replacing the explicit newline, carriage-return, and NUL membership check with a single character-control predicate. Preserve the existing ValueError and message for invalid quoted identifiers, and ensure tabs and other control characters are rejected before SQL rendering or evidence logging.src/omnibase_infra/validation/models/model_application_database_activity_evidence.py (1)
29-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the
fromisoformat(isoformat())round-trip in both window validators.AwareDatetimefields are alreadydatetimeinstances, so both round-trips are no-ops that only add parse cost and hide the comparison. Subtract the fields directly.
src/omnibase_infra/validation/models/model_application_database_activity_evidence.py#L29-L31: replace the two local round-trips withif self.window_ended_at - self.window_started_at < timedelta(hours=24):.src/omnibase_infra/validation/models/model_application_database_activity_result_evidence.py#L40-L42: apply the same replacement, keeping the existing error text.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/omnibase_infra/validation/models/model_application_database_activity_evidence.py` around lines 29 - 31, Remove the datetime.fromisoformat/isoformat round-trips in both window validators and subtract the AwareDatetime fields directly in the duration checks. Update model_application_database_activity_evidence.py lines 29-31 and model_application_database_activity_result_evidence.py lines 40-42; preserve the existing validation behavior and error text.src/omnibase_infra/validation/models/model_application_database_catalog_object_evidence.py (1)
23-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDefine a dedicated enum for the catalog-kind vocabulary.
EnumApplicationDatabaseObjectKindomitstable,view, andmaterialized_view.EnumApplicationInventoryObjectKindaddsextension. Add an enum with exactly these seven values and reuse it in both catalog models.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/omnibase_infra/validation/models/model_application_database_catalog_object_evidence.py` around lines 23 - 31, Define a dedicated enum containing exactly table, view, materialized_view, sequence, function, procedure, and type, then replace the inline catalog_kind Literal annotations in both catalog models with EnumApplicationDatabaseObjectKind. Do not include extension or omit any of the seven catalog kinds.src/omnibase_infra/validation/application_relation_ownership.py (3)
772-784: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unreachable duplicate-owner branch.
The loop at Lines 704-708 iterates every identity in
declarations_by_identityand adds each identity with more than one owner toduplicate_owner_identities.candidatesat Line 762 comes from the same mapping, solen(owners) > 1always impliesidentity in duplicate_owner_identities. Thiselifbody never runs.Delete the branch, or narrow the earlier loop if the live-relation duplicate message is the one you want to keep.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/omnibase_infra/validation/application_relation_ownership.py` around lines 772 - 784, Remove the unreachable `elif len(owners) > 1 and identity not in duplicate_owner_identities` branch that appends `DUPLICATE_OWNER` violations. Keep the surrounding ownership-validation logic unchanged, unless intentionally narrowing the earlier population of `duplicate_owner_identities` to make this live-relation message reachable.
312-320: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffSplit
_append_service_declarationsinto focused helpers.The function spans Lines 312-626 and mixes census checks, blocked relations, table declarations, relation evidence, view counts, and function objects. Extraction into helpers such as
_append_manifest_census_violations,_append_manifest_tables,_append_relation_evidence, and_append_function_objectsreduces the branching depth and makes each rule testable alone.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/omnibase_infra/validation/application_relation_ownership.py` around lines 312 - 320, Refactor _append_service_declarations into focused helper functions that separately handle census violations, blocked relations, table declarations, relation evidence, view counts, and function objects, using helpers such as _append_manifest_census_violations, _append_manifest_tables, _append_relation_evidence, and _append_function_objects. Preserve the existing declaration and violation behavior while reducing branching and keeping each rule independently testable.
157-160: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winClassify topology lookup failures from topology data, not exception text.
_lookup_violation_codemaps everyValueErrorthat does not start with"Unknown database_ref"toUNKNOWN_SCHEMA. Checkdatabase_refmembership intopology.databases, then check schema membership in that database'sschemas. Use the exception only for human-readable detail.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/omnibase_infra/validation/application_relation_ownership.py` around lines 157 - 160, Update _lookup_violation_code to classify the failure using topology data: check whether database_ref exists in topology.databases, then whether the schema exists in that database’s schemas, returning the corresponding violation code. Stop using exception-message prefixes for classification, while retaining the exception only for human-readable error details.src/omnibase_infra/validation/models/model_application_database_acl_policy.py (1)
30-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
from_attributes=Trueto the newmodel_*.pyConfigDictsettings. All six new validation models setfrozen=Trueand anextrapolicy but omitfrom_attributes=True, which the repository requires formodel_<name>.pyfiles. Keep each existingextravalue.
src/omnibase_infra/validation/models/model_application_database_acl_policy.py#L30-L30: change toConfigDict(frozen=True, extra="forbid", from_attributes=True).src/omnibase_infra/validation/models/model_application_database_connection_policy.py#L18-L18: change toConfigDict(frozen=True, extra="forbid", from_attributes=True).src/omnibase_infra/validation/models/model_application_database_role_state.py#L16-L16: change toConfigDict(frozen=True, extra="forbid", from_attributes=True).src/omnibase_infra/validation/models/model_application_relation_ownership_report.py#L22-L22: change toConfigDict(frozen=True, extra="forbid", from_attributes=True).src/omnibase_infra/validation/models/model_application_relation_violation.py#L16-L16: change toConfigDict(frozen=True, extra="forbid", from_attributes=True).src/omnibase_infra/validation/models/model_node_ownership_document.py#L16-L16: change toConfigDict(frozen=True, extra="ignore", from_attributes=True).As per coding guidelines: "Model files named
model_<name>.pymust defineModel<Name>and generally use frozen, strict PydanticConfigDictsettings withfrom_attributes=True."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/omnibase_infra/validation/models/model_application_database_acl_policy.py` at line 30, Update the ConfigDict settings in all six listed model files to include from_attributes=True while preserving each existing extra value: use extra="forbid" for the five application policy/report/violation models and extra="ignore" for ModelNodeOwnershipDocument. Apply the changes at the model_config definitions in model_application_database_acl_policy.py:30-30, model_application_database_connection_policy.py:18-18, model_application_database_role_state.py:16-16, model_application_relation_ownership_report.py:22-22, model_application_relation_violation.py:16-16, and model_node_ownership_document.py:16-16.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6fb0e3a6-05b0-4d19-b050-cd104015c1c0
⛔ Files ignored due to path filters (2)
docker/application-acl-proof/generated/candidate-matrix.yamlis excluded by!**/generated/**docker/application-acl-proof/generated/prechange-fixture-acl.jsonis excluded by!**/generated/**
📒 Files selected for processing (128)
.github/workflows/application-acl-postgres16-proof.yml.github/workflows/docker-build.yml.onex_ratchets/noncanonical_class_allowlist.yamldocker/application-acl-proof/Dockerfiledocker/application-acl-proof/README.mddocker/application-acl-proof/compose.ymldocker/application-acl-proof/seed.sqldocker/application-acl-proof/source-lock.yamldocker/domain-adapter-proof/Dockerfiledocker/domain-adapter-proof/compose.ymldocker/domain-adapter-proof/prove.pyscripts/ci/prove_application_database_acl.pyscripts/generate_application_database_acl.pysrc/omnibase_infra/errors/error_projection.pysrc/omnibase_infra/runtime/auto_wiring/db_table_validator.pysrc/omnibase_infra/runtime/auto_wiring/discovery.pysrc/omnibase_infra/runtime/auto_wiring/handler_wiring.pysrc/omnibase_infra/runtime/auto_wiring/models/model_db_table_validation_warning.pysrc/omnibase_infra/runtime/auto_wiring/models/model_discovered_contract.pysrc/omnibase_infra/runtime/auto_wiring/models/model_event_bus_wiring.pysrc/omnibase_infra/runtime/auto_wiring/protocol_db_table_catalog_connection.pysrc/omnibase_infra/runtime/dispatch_envelope_context.pysrc/omnibase_infra/runtime/kafka_contract_source.pysrc/omnibase_infra/runtime/message_dispatch_engine.pysrc/omnibase_infra/runtime/projection_tenant_authority.pysrc/omnibase_infra/runtime/runtime_host_process.pysrc/omnibase_infra/runtime/service_kernel.pysrc/omnibase_infra/runtime/state_io/state_store_adapter.pysrc/omnibase_infra/validation/application_database_acl.pysrc/omnibase_infra/validation/application_relation_ownership.pysrc/omnibase_infra/validation/enums/enum_application_database_acl_authorization_scope.pysrc/omnibase_infra/validation/enums/enum_application_database_acl_policy_source_kind.pysrc/omnibase_infra/validation/enums/enum_application_database_acl_render_phase.pysrc/omnibase_infra/validation/enums/enum_application_database_object_kind.pysrc/omnibase_infra/validation/enums/enum_application_database_principal_inventory_source_kind.pysrc/omnibase_infra/validation/enums/enum_application_inventory_object_kind.pysrc/omnibase_infra/validation/enums/enum_application_relation_kind.pysrc/omnibase_infra/validation/enums/enum_application_relation_purpose.pysrc/omnibase_infra/validation/enums/enum_application_relation_violation.pysrc/omnibase_infra/validation/enums/enum_internal_tenant_column_transform_status.pysrc/omnibase_infra/validation/enums/enum_ownership_evidence_status.pysrc/omnibase_infra/validation/models/model_application_database_acl_matrix.pysrc/omnibase_infra/validation/models/model_application_database_acl_object.pysrc/omnibase_infra/validation/models/model_application_database_acl_policy.pysrc/omnibase_infra/validation/models/model_application_database_acl_row.pysrc/omnibase_infra/validation/models/model_application_database_acl_source.pysrc/omnibase_infra/validation/models/model_application_database_activity_evidence.pysrc/omnibase_infra/validation/models/model_application_database_activity_principal_observation.pysrc/omnibase_infra/validation/models/model_application_database_activity_result_evidence.pysrc/omnibase_infra/validation/models/model_application_database_catalog_object_evidence.pysrc/omnibase_infra/validation/models/model_application_database_catalog_result_evidence.pysrc/omnibase_infra/validation/models/model_application_database_connection_policy.pysrc/omnibase_infra/validation/models/model_application_database_default_acl_row.pysrc/omnibase_infra/validation/models/model_application_database_observed_role_state.pysrc/omnibase_infra/validation/models/model_application_database_principal_inventory.pysrc/omnibase_infra/validation/models/model_application_database_role_membership.pysrc/omnibase_infra/validation/models/model_application_database_role_state.pysrc/omnibase_infra/validation/models/model_application_inventory_relation.pysrc/omnibase_infra/validation/models/model_application_inventory_relation_counts.pysrc/omnibase_infra/validation/models/model_application_inventory_runtime_evidence.pysrc/omnibase_infra/validation/models/model_application_relation_declaration.pysrc/omnibase_infra/validation/models/model_application_relation_evidence_inventory.pysrc/omnibase_infra/validation/models/model_application_relation_inventory.pysrc/omnibase_infra/validation/models/model_application_relation_ownership_report.pysrc/omnibase_infra/validation/models/model_application_relation_violation.pysrc/omnibase_infra/validation/models/model_blocked_relation.pysrc/omnibase_infra/validation/models/model_database_object_evidence.pysrc/omnibase_infra/validation/models/model_internal_tenant_column_transform.pysrc/omnibase_infra/validation/models/model_live_application_relation.pysrc/omnibase_infra/validation/models/model_migration_ownership_manifest.pysrc/omnibase_infra/validation/models/model_node_ownership_document.pysrc/omnibase_infra/validation/models/model_relation_evidence.pysrc/omnibase_infra/validation/models/model_retained_live_census.pysrc/omnibase_infra/validation/models/model_runtime_evidence_status.pysrc/omnibase_infra/validation/types/__init__.pysrc/omnibase_infra/validation/types/type_application_database_function_signature.pytests/ci/test_application_database_acl_contract.pytests/fixtures/application_database_acl/acl-policy-postgres16.yamltests/fixtures/application_database_acl/acl-policy.yamltests/fixtures/application_database_acl/inventory.yamltests/fixtures/application_database_acl/principal-inventories-external.yamltests/fixtures/application_database_acl/principal-inventory-postgres16.yamltests/fixtures/application_database_acl/principal-inventory.yamltests/fixtures/application_database_acl/topology.yamltests/fixtures/application_relation_ownership/blocked-service.yamltests/fixtures/application_relation_ownership/conflicting-location.yamltests/fixtures/application_relation_ownership/duplicate-owner.yamltests/fixtures/application_relation_ownership/false-complete-census.yamltests/fixtures/application_relation_ownership/incomplete-census.yamltests/fixtures/application_relation_ownership/inventory-unknown-schema.yamltests/fixtures/application_relation_ownership/inventory-with-missing.yamltests/fixtures/application_relation_ownership/inventory-without-delegation.yamltests/fixtures/application_relation_ownership/inventory.yamltests/fixtures/application_relation_ownership/node-owner.yamltests/fixtures/application_relation_ownership/node-reader.yamltests/fixtures/application_relation_ownership/parallel-location.yamltests/fixtures/application_relation_ownership/rich-inventory.yamltests/fixtures/application_relation_ownership/service-owner.yamltests/fixtures/application_relation_ownership/topology.yamltests/fixtures/application_relation_ownership/unknown-schema.yamltests/fixtures/omn15547/application-acl-prechange-fixture.json.capturedtests/fixtures/omn15547/application-acl-source-lock.yaml.capturedtests/helpers/application_db_topology.pytests/helpers/projection_tenant_authority.pytests/incident_replays/registry.yamltests/integration/db/test_projection_writer_tenant_rls_omn15301.pytests/integration/projectors/test_handler_wiring_projection_dispatch.pytests/integration/projectors/test_projection_terminal_event_emission.pytests/integration/runtime/test_projection_dispatch_tenant_authority_context.pytests/integration/runtime/test_projection_handler_db_injection_integration.pytests/integration/test_application_acl_postgres16_proof.pytests/integration/test_projection_handler_wiring_runtime_dispatch.pytests/integration/test_sync_db_adapter_composite_conflict_key.pytests/unit/contracts/test_protocol_ownership.pytests/unit/runtime/auto_wiring/test_db_table_validator.pytests/unit/runtime/auto_wiring/test_handler_wiring_db_injection.pytests/unit/runtime/auto_wiring/test_projection_domain_adapters.pytests/unit/runtime/auto_wiring/test_typed_db_io_loading.pytests/unit/runtime/test_handler_wiring_state_io.pytests/unit/runtime/test_health_config_prefetch_status.pytests/unit/runtime/test_kafka_contract_source.pytests/unit/runtime/test_kafka_contract_source_materialization.pytests/unit/runtime/test_live_contract_materialization.pytests/unit/runtime/test_message_dispatch_engine.pytests/unit/runtime/test_service_kernel_database_topology_wiring.pytests/unit/scripts/test_prove_application_database_acl.pytests/unit/validation/test_application_database_acl.pytests/unit/validation/test_application_relation_ownership.py
💤 Files with no reviewable changes (1)
- .onex_ratchets/noncanonical_class_allowlist.yaml
| domain-adapter-proof: | ||
| name: Projection Domain Adapter Proof (OMN-15421) | ||
| runs-on: >- | ||
| ${{ | ||
| (github.event_name == 'pull_request' && | ||
| github.event.pull_request.head.repo.full_name != github.repository) | ||
| && fromJSON(vars.OMNI_PUBLIC_PR_RUNS_ON_JSON || '["ubuntu-latest"]') | ||
| || fromJSON(vars.OMNI_DOCKER_CI_RUNS_ON_JSON || vars.OMNI_TRUSTED_CI_RUNS_ON_JSON || '["self-hosted","omnibase-ci"]') | ||
| }} | ||
| timeout-minutes: 15 | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the top-level and per-job permissions declared in docker-build.yml
rg -n -B2 -A2 'permissions:' .github/workflows/docker-build.ymlRepository: OmniNode-ai/omnibase_infra
Length of output: 527
🏁 Script executed:
#!/bin/bash
# Inspect workflow structure and permission scope around the affected jobs.
sed -n '1,155p' .github/workflows/docker-build.yml
printf '\n--- permission declarations ---\n'
rg -n '^[[:space:]]{2}permissions:|^[[:space:]]{4}permissions:|^[[:space:]]{6}permissions:' .github/workflows/docker-build.yml
printf '\n--- comparable workflow permissions ---\n'
rg -n -B3 -A5 'permissions:' .github/workflows/application-acl-postgres16-proof.ymlRepository: OmniNode-ai/omnibase_infra
Length of output: 6415
🏁 Script executed:
#!/bin/bash
sed -n '1,155p' .github/workflows/docker-build.yml
printf '\n--- permission declarations ---\n'
rg -n '^[[:space:]]{2}permissions:|^[[:space:]]{4}permissions:|^[[:space:]]{6}permissions:' .github/workflows/docker-build.yml
printf '\n--- comparable workflow permissions ---\n'
rg -n -B3 -A5 'permissions:' .github/workflows/application-acl-postgres16-proof.ymlRepository: OmniNode-ai/omnibase_infra
Length of output: 6415
Add explicit GITHUB_TOKEN permissions to domain-adapter-proof.
docker-build.yml has no workflow-level permissions: block. Add permissions: contents: read to this job so it does not inherit broader repository defaults.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/docker-build.yml around lines 80 - 90, Add a job-level
permissions configuration to domain-adapter-proof granting only contents: read,
ensuring the job does not inherit broader repository permissions. Keep the
existing runner and timeout settings unchanged.
| dlq_topics: tuple[str, ...] = Field( | ||
| default_factory=tuple, | ||
| description="Contract-declared dead-letter topics", | ||
| ) | ||
| consumer_group: str | None = Field( | ||
| default=None, | ||
| description="Optional contract-declared consumer group identifier", | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Collect every key used under `event_bus:` in contract YAML files and compare with the model fields.
fd -t f -e yaml -e yml . | xargs -r rg -l '^\s*event_bus\s*:' | while IFS= read -r f; do
python - "$f" <<'PY'
import sys, yaml
path = sys.argv[1]
try:
doc = yaml.safe_load(open(path, encoding="utf-8"))
except Exception:
sys.exit(0)
if isinstance(doc, dict) and isinstance(doc.get("event_bus"), dict):
print(path, sorted(doc["event_bus"]))
PY
done
# Show the declared field set for comparison.
fd -t f 'model_event_bus_wiring.py' -x cat -n {}Repository: OmniNode-ai/omnibase_infra
Length of output: 10892
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- ModelEventBusWiring ---'
cat -n src/omnibase_infra/runtime/auto_wiring/models/model_event_bus_wiring.py
printf '%s\n' '--- kafka_contract_source validation sites ---'
rg -n -C 12 'ModelEventBusWiring|event_bus' src/omnibase_infra/runtime/kafka_contract_source.py
printf '%s\n' '--- discovery event_bus parsing ---'
rg -n -C 12 'event_bus|subscribe_topics|publish_topics' src/omnibase_infra/runtime/auto_wiring/discovery.py
printf '%s\n' '--- representative contract shapes ---'
for f in \
src/omnibase_infra/nodes/node_build_loop_projection_compute/contract.yaml \
src/omnibase_infra/nodes/node_runtime_orchestrator/contract.yaml \
contracts/runtime/runtime_config.yaml \
contracts/runtime/event_bus_wiring_effect.yaml
do
if [ -f "$f" ]; then
echo "--- $f"
cat -n "$f" | sed -n '1,100p'
fi
done
printf '%s\n' '--- all event_bus mappings, including nested mappings ---'
python3 - <<'PY'
from pathlib import Path
import yaml
def walk(value, path=()):
if isinstance(value, dict):
for key, child in value.items():
if key == "event_bus" and isinstance(child, dict):
yield path + (key,), child
yield from walk(child, path + (str(key),))
elif isinstance(value, list):
for i, child in enumerate(value):
yield from walk(child, path + (f"[{i}]",))
for path in sorted(Path(".").rglob("*.yaml")) + sorted(Path(".").rglob("*.yml")):
try:
data = yaml.safe_load(path.read_text(encoding="utf-8"))
except Exception:
continue
for location, mapping in walk(data):
print(f"{path}:{'.'.join(location)}\t{','.join(sorted(map(str, mapping)))}")
PYRepository: OmniNode-ai/omnibase_infra
Length of output: 40041
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- References to strict event_bus validation and contract loading ---'
rg -n -C 8 'on_contract_registered|contract_data|contract_config|runtime_config|_parse_contract|ModelEventBusWiring' \
src/omnibase_infra/runtime src/omnibase_infra/service_kernel.py tests 2>/dev/null || true
printf '%s\n' '--- ModelEventBusWiring tests and direct validation tests ---'
rg -n -C 10 'ModelEventBusWiring|auto_offset_reset|event_bus\.(version|type|environment)|event_bus:' \
tests src/omnibase_infra | head -n 1200
printf '%s\n' '--- Event-bus key blocks in checked-in YAML files ---'
python3 - <<'PY'
from pathlib import Path
import re
fields = {
"subscribe_topics", "publish_topics", "dlq_topics", "consumer_group",
"consumer_purpose", "plugin_managed", "tenant_scoped_ingress",
"terminal_event",
}
for path in sorted(list(Path(".").rglob("*.yaml")) + list(Path(".").rglob("*.yml"))):
lines = path.read_text(encoding="utf-8", errors="replace").splitlines()
in_event_bus = False
event_indent = None
keys = []
for line_no, line in enumerate(lines, 1):
if not line.strip() or line.lstrip().startswith("#"):
continue
indent = len(line) - len(line.lstrip())
match = re.match(r"^(\s*)event_bus\s*:\s*(?:#.*)?$", line)
if match:
in_event_bus = True
event_indent = len(match.group(1))
keys = []
continue
if in_event_bus:
if indent <= event_indent:
if keys:
extra = sorted(set(keys) - fields)
if extra:
print(f"{path}: event_bus keys={sorted(set(keys))}; extra={extra}")
in_event_bus = False
event_indent = None
else:
key_match = re.match(r"^\s+([A-Za-z_][A-Za-z0-9_-]*)\s*:", line)
if key_match and indent == event_indent + 2:
keys.append(key_match.group(1))
if in_event_bus and keys:
extra = sorted(set(keys) - fields)
if extra:
print(f"{path}: event_bus keys={sorted(set(keys))}; extra={extra}")
PYRepository: OmniNode-ai/omnibase_infra
Length of output: 50383
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- Kafka registration validation and failure handling ---'
sed -n '220,335p' src/omnibase_infra/runtime/kafka_contract_source.py
rg -n -C 18 'def on_contract_registered|on_contract_registered\(' src/omnibase_infra/runtime/kafka_contract_source.py src/omnibase_infra/runtime tests/unit/runtime | head -n 500
printf '%s\n' '--- Targeted tests that include extra event_bus keys ---'
rg -l 'event_bus.*version|version:.*major|auto_offset_reset|consumer_purpose' \
tests/unit/runtime tests/integration 2>/dev/null | while IFS= read -r f; do
echo "--- $f"
rg -n -C 6 'event_bus|auto_offset_reset|consumer_purpose|version:' "$f" | head -n 120
done
printf '%s\n' '--- Checked-in node contracts with known extra event_bus keys ---'
for f in \
src/omnibase_infra/nodes/node_build_loop_projection_compute/contract.yaml \
src/omnibase_infra/nodes/node_ledger_projection_compute/contract.yaml \
src/omnibase_infra/nodes/node_validation_ledger_projection_compute/contract.yaml
do
if [ -f "$f" ]; then
echo "--- $f"
sed -n '/^event_bus:/,/^[^ #].*:/p' "$f" | head -n 30
fi
doneRepository: OmniNode-ai/omnibase_infra
Length of output: 50383
Add all contract-declared event_bus fields before enabling extra="forbid".
Checked-in node contracts and existing tests declare event_bus.version and auto_offset_reset, but ModelEventBusWiring does not. Kafka registration validates the raw mapping and rejects these contracts, while filesystem discovery ignores the extra keys. Align the model with the contract schema, or strip unsupported fields before validation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/omnibase_infra/runtime/auto_wiring/models/model_event_bus_wiring.py`
around lines 23 - 30, Update ModelEventBusWiring to declare every
contract-supported event_bus field, including version and auto_offset_reset,
before strict extra-field validation runs. Match the existing contract
types/defaults and preserve validation of genuinely unsupported keys; do not
strip declared fields from the raw mapping.
| accepted = materialization.status in { | ||
| EnumMaterializationStatus.MATERIALIZED, | ||
| EnumMaterializationStatus.ALREADY_MATERIALIZED, | ||
| } | ||
| if not accepted: | ||
| logger.warning( | ||
| "Typed db_io contract materialization rejected", | ||
| extra={ | ||
| "node_name": node_name, | ||
| "status": materialization.status.value, | ||
| "reason": ( | ||
| materialization.reason.value | ||
| if materialization.reason is not None | ||
| else None | ||
| ), | ||
| "correlation_id": str(correlation_id), | ||
| }, | ||
| ) | ||
| return accepted |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find consumers of CAPABILITY_CHANGE and of registered_handlers to size the behavior change.
rg -nP --type=py -C3 'CAPABILITY_CHANGE' -g '!**/tests/**'
rg -nP --type=py -C3 '\bregistered_handlers\b'
rg -nP --type=py -C5 '_publish_capability_change|_announced_capabilities'Repository: OmniNode-ai/omnibase_infra
Length of output: 164
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate file ---'
wc -l src/omnibase_infra/runtime/runtime_host_process.py
ast-grep outline src/omnibase_infra/runtime/runtime_host_process.py | rg -n 'publish|capability|handler|health|material|dispatch|register' || true
printf '%s\n' '--- relevant symbols and references ---'
rg -n -C4 'CAPABILITY_CHANGE|registered_handlers|_publish_capability_change|_announced_capabilities|db_io|dispatch.engine|dispatch_engine' src/omnibase_infra/runtime/runtime_host_process.py src -g '*.py' -g '*.yaml' || true
printf '%s\n' '--- all tracked paths containing likely consumers ---'
git ls-files | rg -i 'capabil|health|dispatch|handler|introspect' | head -200Repository: OmniNode-ai/omnibase_infra
Length of output: 50383
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- runtime_host_process.py target ---'
sed -n '3360,3505p' src/omnibase_infra/runtime/runtime_host_process.py
printf '%s\n' '--- exact capability symbols ---'
rg -n -C5 'CAPABILITY_CHANGE|registered_handlers|_publish_capability_change|_announced_capabilities' src/omnibase_infra/runtime/runtime_host_process.py src --glob '*.py' --glob '*.yaml' | head -300
printf '%s\n' '--- dispatch registration and report symbols ---'
rg -n -C4 'register_dispatch|register.*dispatcher|dispatcher.*register|ModelContractWiringResult|wired.*handler|handler.*wired|health_check' src/omnibase_infra/runtime src/omnibase_infra --glob '*.py' | head -400Repository: OmniNode-ai/omnibase_infra
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- materialize_cached_contract definition and callers ---'
rg -n -C10 'def materialize_cached_contract|materialize_cached_contract\(' src --glob '*.py' | head -250
printf '%s\n' '--- db_io commit registration ---'
rg -n -C15 'def _commit_handler_wiring|def _commit_contract_wiring|dispatcher_id|route_ids|db_io' src/omnibase_infra/runtime/auto_wiring/handler_wiring.py | head -350
printf '%s\n' '--- health and dispatch-engine projections ---'
sed -n '5280,5505p' src/omnibase_infra/runtime/runtime_host_process.py
rg -n -C5 'dispatch_engine|dispatchers|routes|wired_contract|wiring_details|registered_handlers' src/omnibase_infra/runtime/runtime_host_process.py src/omnibase_infra/services/health_checker.py --glob '*.py' | head -300
printf '%s\n' '--- focused tests and contract expectations ---'
rg -n -C4 'CAPABILITY_CHANGE|materialize_cached_contract|db_io.*material|registered_handlers|dispatch.*engine' tests src --glob '*.py' | head -400Repository: OmniNode-ai/omnibase_infra
Length of output: 50383
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- KafkaContractSource.materialize_cached_contract ---'
sed -n '1320,1495p' src/omnibase_infra/runtime/kafka_contract_source.py
printf '%s\n' '--- commit path ---'
sed -n '6159,6295p' src/omnibase_infra/runtime/auto_wiring/handler_wiring.py
printf '%s\n' '--- dynamic registration authorization ---'
sed -n '6990,7075p' src/omnibase_infra/runtime/auto_wiring/handler_wiring.py
printf '%s\n' '--- focused test filenames and matching lines ---'
rg -l 'CAPABILITY_CHANGE|materialize_cached_contract|db_io.*material|registered_handlers|EnumMaterializationStatus' tests src --glob '*.py' | sort
rg -n -C3 'CAPABILITY_CHANGE|materialize_cached_contract|db_io.*material|EnumMaterializationStatus' tests --glob '*.py' | head -300Repository: OmniNode-ai/omnibase_infra
Length of output: 49645
Preserve capability and health projections for live db_io materialization. The dispatch engine registers the dispatcher and routes, but the early return skips _publish_capability_change() and leaves self._handlers unchanged. Therefore, CAPABILITY_CHANGE consumers and health_check()["registered_handlers"] do not observe the materialized contract. Add an equivalent capability and health projection for this path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/omnibase_infra/runtime/runtime_host_process.py` around lines 3454 - 3472,
The typed db_io materialization path returns before updating capability and
handler projections. In the dispatch flow around the shown accepted-status
check, publish the equivalent capability change via _publish_capability_change()
and update self._handlers consistently with the normal materialization path
before returning, while preserving rejection logging and the accepted result.
| profile = os.environ.get("ONEX_DATABASE_TOPOLOGY_PROFILE") | ||
| if profile is None: | ||
| return None | ||
|
|
||
| from omnibase_infra.topology import load_topology_profile | ||
|
|
||
| return load_topology_profile(profile) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Treat a blank profile value as unset.
os.environ.get("ONEX_DATABASE_TOPOLOGY_PROFILE") returns "" when the variable is exported empty, which is common in compose and CI environment injection. The function then calls load_topology_profile("") and bootstrap fails with a profile-resolution error instead of the clear "profile is not set" path. Normalize the value first.
🐛 Proposed fix
- profile = os.environ.get("ONEX_DATABASE_TOPOLOGY_PROFILE")
- if profile is None:
+ profile = (os.environ.get("ONEX_DATABASE_TOPOLOGY_PROFILE") or "").strip()
+ if not profile:
return None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| profile = os.environ.get("ONEX_DATABASE_TOPOLOGY_PROFILE") | |
| if profile is None: | |
| return None | |
| from omnibase_infra.topology import load_topology_profile | |
| return load_topology_profile(profile) | |
| profile = (os.environ.get("ONEX_DATABASE_TOPOLOGY_PROFILE") or "").strip() | |
| if not profile: | |
| return None | |
| from omnibase_infra.topology import load_topology_profile | |
| return load_topology_profile(profile) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/omnibase_infra/runtime/service_kernel.py` around lines 373 - 379,
Normalize the value read in the profile-loading function before checking it,
treating an empty or whitespace-only ONEX_DATABASE_TOPOLOGY_PROFILE as unset and
returning None. Only call load_topology_profile with a nonblank normalized
profile value.
| def _quote_identifier(identifier: str) -> str: | ||
| if _SQL_IDENTIFIER.fullmatch(identifier) is None: | ||
| raise ValueError(f"Unsafe or non-canonical SQL identifier {identifier!r}") | ||
| return f'"{identifier}"' | ||
|
|
||
|
|
||
| def _quote_sql_literal(value: str) -> str: | ||
| """Quote one trusted typed value as a PostgreSQL string literal.""" | ||
| return "'" + value.replace("'", "''") + "'" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate every name that is interpolated into rendered SQL.
_quote_identifier rejects a non-canonical identifier, but the renderer bypasses it for many string literals and for name lists:
- Line 2720 and the surrounding guard blocks embed
role_name,schema_name,object_ref, andphysical_databasedirectly in single quotes with an f-string. - Lines 2861, 2942, 3028, 3029, and 3080 build SQL literals with Python
repr().repr()escapes an embedded quote as\', which PostgreSQL does not accept as an escaped quote in a standard-conforming string._quote_sql_literalexists for exactly this and doubles the quote instead.
A name that contains ', a backslash, or a newline therefore produces a broken or attacker-shaped script that runs with owner privileges. The evidence YAML is checked in, so this is a posture gap rather than a live exploit, but the generated artifact is privileged SQL.
The cheapest complete fix is to validate every role, schema, and object name once while building the matrix, and to replace each repr() literal with _quote_sql_literal.
🔒️ Sketch of the validation and quoting fix
+def _assert_canonical_name(kind: str, name: str) -> str:
+ if _SQL_IDENTIFIER.fullmatch(name) is None:
+ raise ValueError(f"Unsafe or non-canonical {kind} name {name!r}")
+ return name- member_literals = ", ".join(repr(name) for name in membership_members)
- parent_literals = ", ".join(repr(name) for name in governed_parents)
+ member_literals = ", ".join(_quote_sql_literal(name) for name in membership_members)
+ parent_literals = ", ".join(_quote_sql_literal(name) for name in governed_parents)- database_literals = ", ".join(
- repr(database) for database in matrix.required_connect_databases
- )
+ database_literals = ", ".join(
+ _quote_sql_literal(database) for database in matrix.required_connect_databases
+ )Also applies to: 2716-2724, 3028-3029, 3079-3081
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/omnibase_infra/validation/application_database_acl.py` around lines 2466
- 2474, Validate every role, schema, object, and physical database name when
constructing the matrix, reusing _quote_identifier or the appropriate canonical
validation before rendering. In the SQL renderer and surrounding guard blocks,
replace direct single-quoted f-string interpolation of role_name, schema_name,
object_ref, and physical_database, plus every repr()-generated SQL literal
around the identified rendering sites, with _quote_sql_literal; preserve the
existing generated SQL structure while ensuring all interpolated names are
safely escaped.
| result = subprocess.run( | ||
| [ | ||
| *command, | ||
| "up", | ||
| "--build", | ||
| "--abort-on-container-exit", | ||
| "--exit-code-from", | ||
| "proof", | ||
| ], | ||
| cwd=_ROOT, | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=1_500, | ||
| check=False, | ||
| ) | ||
| try: | ||
| assert result.returncode == 0, result.stdout + result.stderr | ||
| output = result.stdout + result.stderr | ||
| assert "acl_phase=scaffold_wrong_database_guard status=PASS" in output | ||
| assert "acl_phase=scaffold_fresh_additive_round_trip status=PASS" in output | ||
| assert "acl_phase=rollback_atomic_failure" in output | ||
| assert "acl_status=PASS postgres_major=16" in output | ||
| finally: | ||
| subprocess.run( | ||
| [*command, "down", "--volumes", "--remove-orphans"], | ||
| cwd=_ROOT, | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=120, | ||
| check=False, | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
A timeout on up skips the compose cleanup.
subprocess.run(... "up" ...) runs outside the try block. If the build and proof exceed timeout=1_500, subprocess.run raises TimeoutExpired before the try is entered, so the finally never runs docker compose down --volumes --remove-orphans. The containers, network, and volumes of project application-acl-proof-<pid> then survive the failed run. The project name embeds the PID, so a later run does not reclaim them. Move the up call inside the try.
🐛 Proposed fix
- result = subprocess.run(
- [
- *command,
- "up",
- "--build",
- "--abort-on-container-exit",
- "--exit-code-from",
- "proof",
- ],
- cwd=_ROOT,
- capture_output=True,
- text=True,
- timeout=1_500,
- check=False,
- )
try:
+ result = subprocess.run(
+ [
+ *command,
+ "up",
+ "--build",
+ "--abort-on-container-exit",
+ "--exit-code-from",
+ "proof",
+ ],
+ cwd=_ROOT,
+ capture_output=True,
+ text=True,
+ timeout=1_500,
+ check=False,
+ )
assert result.returncode == 0, result.stdout + result.stderr📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| result = subprocess.run( | |
| [ | |
| *command, | |
| "up", | |
| "--build", | |
| "--abort-on-container-exit", | |
| "--exit-code-from", | |
| "proof", | |
| ], | |
| cwd=_ROOT, | |
| capture_output=True, | |
| text=True, | |
| timeout=1_500, | |
| check=False, | |
| ) | |
| try: | |
| assert result.returncode == 0, result.stdout + result.stderr | |
| output = result.stdout + result.stderr | |
| assert "acl_phase=scaffold_wrong_database_guard status=PASS" in output | |
| assert "acl_phase=scaffold_fresh_additive_round_trip status=PASS" in output | |
| assert "acl_phase=rollback_atomic_failure" in output | |
| assert "acl_status=PASS postgres_major=16" in output | |
| finally: | |
| subprocess.run( | |
| [*command, "down", "--volumes", "--remove-orphans"], | |
| cwd=_ROOT, | |
| capture_output=True, | |
| text=True, | |
| timeout=120, | |
| check=False, | |
| ) | |
| try: | |
| result = subprocess.run( | |
| [ | |
| *command, | |
| "up", | |
| "--build", | |
| "--abort-on-container-exit", | |
| "--exit-code-from", | |
| "proof", | |
| ], | |
| cwd=_ROOT, | |
| capture_output=True, | |
| text=True, | |
| timeout=1_500, | |
| check=False, | |
| ) | |
| assert result.returncode == 0, result.stdout + result.stderr | |
| output = result.stdout + result.stderr | |
| assert "acl_phase=scaffold_wrong_database_guard status=PASS" in output | |
| assert "acl_phase=scaffold_fresh_additive_round_trip status=PASS" in output | |
| assert "acl_phase=rollback_atomic_failure" in output | |
| assert "acl_status=PASS postgres_major=16" in output | |
| finally: | |
| subprocess.run( | |
| [*command, "down", "--volumes", "--remove-orphans"], | |
| cwd=_ROOT, | |
| capture_output=True, | |
| text=True, | |
| timeout=120, | |
| check=False, | |
| ) |
🧰 Tools
🪛 ast-grep (0.45.0)
[error] 53-60: Command coming from incoming request
Context: subprocess.run(
[*command, "down", "--volumes", "--remove-orphans"],
cwd=_ROOT,
capture_output=True,
text=True,
timeout=120,
check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/integration/test_application_acl_postgres16_proof.py` around lines 31 -
61, Move the `subprocess.run` invocation for the compose `"up"` command inside
the existing `try` block in the test flow, so `TimeoutExpired` and other
execution failures still reach the `finally` cleanup. Preserve the current
assertions and ensure the compose `"down" --volumes --remove-orphans"` call
always executes.
| output = result.stdout + result.stderr | ||
| assert "acl_phase=scaffold_wrong_database_guard status=PASS" in output | ||
| assert "acl_phase=scaffold_fresh_additive_round_trip status=PASS" in output | ||
| assert "acl_phase=rollback_atomic_failure" in output |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert a verdict for the rollback phase.
Lines 49-50 assert status=PASS for their phases. Line 51 asserts only that the phase name appears, so the rollback proof passes even when that phase reports a failure verdict. Add the expected status suffix if the proof emits one for this phase.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/integration/test_application_acl_postgres16_proof.py` at line 51,
Update the rollback-phase assertion near the existing status checks to require
the proof’s expected success verdict, not just the phase name. Extend the
assertion for `acl_phase=rollback_atomic_failure` with the emitted `status=PASS`
suffix, preserving the existing phase identifier.
| def test_structural_security_context_is_not_tenant_authority() -> None: | ||
| tenant_id = uuid4() | ||
| envelope = ModelEventEnvelope[dict[str, object]]( | ||
| payload={"tenant_id": str(tenant_id)}, | ||
| security_context=ModelSecurityContext( | ||
| user_id=uuid4(), | ||
| auth_method="oidc", | ||
| auth_timestamp=datetime.now(UTC), | ||
| security_labels={"tenant_id": str(tenant_id)}, | ||
| ), | ||
| ) | ||
| target = projection_database_target("tenant_events", schema="tenant") | ||
| conn, _ = _connection("tenant_projection_writer") | ||
|
|
||
| with patch("psycopg2.connect", return_value=conn): | ||
| adapter = _adapter(target, authority=None) | ||
| with pytest.raises(ProjectionTenantContextError, match="verified authority"): | ||
| adapter.upsert("tenant_events", "event_id", {"event_id": uuid4()}) | ||
|
|
||
| conn.cursor.assert_not_called() | ||
| assert ( | ||
| envelope.security_context is not None | ||
| ) # proves the rejected shape was present |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
The structural-security-context test never gives the context to the adapter.
The test builds envelope with a ModelSecurityContext that carries security_labels["tenant_id"], but line 208 passes authority=None and no tenant_event. The adapter never sees that envelope. The test therefore proves only that a missing authority is rejected. The final assertion at lines 213-215 restates a local construction and cannot fail.
The stated guarantee is that a structural security context is not accepted as tenant authority. To cover that guarantee, pass the envelope as the dispatch event while authority stays None.
🐛 Proposed fix
target = projection_database_target("tenant_events", schema="tenant")
conn, _ = _connection("tenant_projection_writer")
with patch("psycopg2.connect", return_value=conn):
- adapter = _adapter(target, authority=None)
+ adapter = _adapter(target, authority=None, tenant_event=envelope)
with pytest.raises(ProjectionTenantContextError, match="verified authority"):
adapter.upsert("tenant_events", "event_id", {"event_id": uuid4()})
conn.cursor.assert_not_called()
- assert (
- envelope.security_context is not None
- ) # proves the rejected shape was present
+ # The rejected envelope did carry a structurally valid tenant label.
+ assert envelope.security_context is not None
+ assert envelope.security_context.security_labels["tenant_id"] == str(tenant_id)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_structural_security_context_is_not_tenant_authority() -> None: | |
| tenant_id = uuid4() | |
| envelope = ModelEventEnvelope[dict[str, object]]( | |
| payload={"tenant_id": str(tenant_id)}, | |
| security_context=ModelSecurityContext( | |
| user_id=uuid4(), | |
| auth_method="oidc", | |
| auth_timestamp=datetime.now(UTC), | |
| security_labels={"tenant_id": str(tenant_id)}, | |
| ), | |
| ) | |
| target = projection_database_target("tenant_events", schema="tenant") | |
| conn, _ = _connection("tenant_projection_writer") | |
| with patch("psycopg2.connect", return_value=conn): | |
| adapter = _adapter(target, authority=None) | |
| with pytest.raises(ProjectionTenantContextError, match="verified authority"): | |
| adapter.upsert("tenant_events", "event_id", {"event_id": uuid4()}) | |
| conn.cursor.assert_not_called() | |
| assert ( | |
| envelope.security_context is not None | |
| ) # proves the rejected shape was present | |
| def test_structural_security_context_is_not_tenant_authority() -> None: | |
| tenant_id = uuid4() | |
| envelope = ModelEventEnvelope[dict[str, object]]( | |
| payload={"tenant_id": str(tenant_id)}, | |
| security_context=ModelSecurityContext( | |
| user_id=uuid4(), | |
| auth_method="oidc", | |
| auth_timestamp=datetime.now(UTC), | |
| security_labels={"tenant_id": str(tenant_id)}, | |
| ), | |
| ) | |
| target = projection_database_target("tenant_events", schema="tenant") | |
| conn, _ = _connection("tenant_projection_writer") | |
| with patch("psycopg2.connect", return_value=conn): | |
| adapter = _adapter(target, authority=None, tenant_event=envelope) | |
| with pytest.raises(ProjectionTenantContextError, match="verified authority"): | |
| adapter.upsert("tenant_events", "event_id", {"event_id": uuid4()}) | |
| conn.cursor.assert_not_called() | |
| # The rejected envelope did carry a structurally valid tenant label. | |
| assert envelope.security_context is not None | |
| assert envelope.security_context.security_labels["tenant_id"] == str(tenant_id) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/runtime/auto_wiring/test_projection_domain_adapters.py` around
lines 193 - 215, Update test_structural_security_context_is_not_tenant_authority
to pass the constructed envelope as the adapter’s dispatch event (tenant_event)
while keeping authority=None. Remove the redundant final assertion that only
rechecks the locally constructed security context, and retain the expectation
that ProjectionTenantContextError is raised without opening a cursor.
| def test_cold_boot_wiring_receives_resolved_topology() -> None: | ||
| keyword = _keyword_for_call("wire_from_manifest", "topology") | ||
|
|
||
| assert keyword is not None | ||
| assert isinstance(keyword.value, ast.Name) | ||
| assert keyword.value.id == "deployment_topology" | ||
|
|
||
|
|
||
| def test_dynamic_runtime_receives_the_same_resolved_topology() -> None: | ||
| keyword = _keyword_for_call("RuntimeHostProcess", "deployment_topology") | ||
|
|
||
| assert keyword is not None | ||
| assert isinstance(keyword.value, ast.Name) | ||
| assert keyword.value.id == "deployment_topology" | ||
|
|
||
|
|
||
| def test_db_io_boot_requires_explicit_profile() -> None: | ||
| source = SERVICE_KERNEL_PATH.read_text(encoding="utf-8") | ||
|
|
||
| assert "if db_io_contracts and deployment_topology is None:" in source | ||
| assert "ONEX_DATABASE_TOPOLOGY_PROFILE is not set" in source |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Locate real startup tests that invoke wire_from_manifest and check for the new topology argument.
set -uo pipefail
echo "=== wire_from_manifest usage in tests ==="
rg -nP -C 8 '\bwire_from_manifest\s*\(' tests
echo "=== wire_from_manifest signature ==="
ast-grep run --pattern 'async def wire_from_manifest($$$):
$$$' --lang python src/omnibase_infra/runtime/auto_wiring
ast-grep run --pattern 'def wire_from_manifest($$$):
$$$' --lang python src/omnibase_infra/runtime/auto_wiringRepository: OmniNode-ai/omnibase_infra
Length of output: 50383
🏁 Script executed:
#!/bin/bash
set -u
echo "=== topology references in tests ==="
rg -n -C 4 '\b(topology|deployment_topology|ONEX_DATABASE_TOPOLOGY_PROFILE|_load_runtime_database_topology)\b' tests/unit/runtime tests/integration/runtime tests/integration 2>/dev/null | head -n 1200
echo "=== topology references in runtime source ==="
rg -n -C 3 '\b(topology|deployment_topology|ONEX_DATABASE_TOPOLOGY_PROFILE|wire_from_manifest)\b' src/omnibase_infra/runtime/auto_wiring src/omnibase_infra/runtime/service_kernel.py | head -n 1000
echo "=== candidate tests by filename ==="
git ls-files 'tests/**/*.py' | rg -i '(startup|boot|kernel|wiring|manifest|topology|database)'
echo "=== exact topology keyword call contexts ==="
rg -n -U -P -C 8 'wire_from_manifest\s*\([^)]*\btopology\s*=' tests || trueRepository: OmniNode-ai/omnibase_infra
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -u
echo "=== live contract materialization test outline ==="
ast-grep outline tests/unit/runtime/test_live_contract_materialization.py
echo "=== live contract materialization focused source ==="
sed -n '1,235p' tests/unit/runtime/test_live_contract_materialization.py
echo "=== wire_from_manifest definition and topology handling ==="
rg -n -C 10 'def wire_from_manifest|topology:|deployment_topology|_prepare_handler_wiring' src/omnibase_infra/runtime/auto_wiring src/omnibase_infra/runtime/service_kernel.py | head -n 1200
echo "=== AST inventory of test calls passing topology ==="
python3 - <<'PY'
import ast
from pathlib import Path
for path in Path("tests").rglob("*.py"):
try:
tree = ast.parse(path.read_text(encoding="utf-8"))
except Exception:
continue
for node in ast.walk(tree):
if isinstance(node, ast.Call):
name = (
node.func.id if isinstance(node.func, ast.Name)
else node.func.attr if isinstance(node.func, ast.Attribute)
else ""
)
if name == "wire_from_manifest":
topology = [kw for kw in node.keywords if kw.arg == "topology"]
if topology:
print(f"{path}:{node.lineno}: topology argument present")
print(ast.get_source_segment(path.read_text(encoding="utf-8"), node))
PYRepository: OmniNode-ai/omnibase_infra
Length of output: 48720
🏁 Script executed:
#!/bin/bash
set -u
echo "=== live contract materialization test outline ==="
ast-grep outline tests/unit/runtime/test_live_contract_materialization.py
echo "=== live contract materialization focused source ==="
sed -n '1,235p' tests/unit/runtime/test_live_contract_materialization.py
echo "=== wire_from_manifest definition and topology handling ==="
rg -n -C 10 'def wire_from_manifest|topology:|deployment_topology|_prepare_handler_wiring' src/omnibase_infra/runtime/auto_wiring src/omnibase_infra/runtime/service_kernel.py | head -n 1200
echo "=== AST inventory of test calls passing topology ==="
python3 - <<'PY'
import ast
from pathlib import Path
for path in Path("tests").rglob("*.py"):
text = path.read_text(encoding="utf-8")
try:
tree = ast.parse(text)
except Exception:
continue
for node in ast.walk(tree):
if isinstance(node, ast.Call):
name = (
node.func.id if isinstance(node.func, ast.Name)
else node.func.attr if isinstance(node.func, ast.Attribute)
else ""
)
if name == "wire_from_manifest" and any(
kw.arg == "topology" for kw in node.keywords
):
print(f"{path}:{node.lineno}: topology argument present")
print(ast.get_source_segment(text, node))
PYRepository: OmniNode-ai/omnibase_infra
Length of output: 48720
🏁 Script executed:
#!/bin/bash
set -u
echo "=== real manifest startup test ==="
sed -n '1,180p' tests/integration/runtime/test_auto_wiring_real_manifest.py
echo "=== runtime boot tests ==="
for f in \
tests/integration/runtime/test_build_loop_auto_wiring_boot.py \
tests/integration/runtime/test_pr_state_auto_wiring_boot.py \
tests/integration/runtime/test_ledger_write_effect_dispatch_seam.py
do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,145p' "$f"
fi
done
echo "=== contract discovery/loading and handler assertions in startup candidates ==="
rg -n -C 3 'discover_contracts|discover_contracts_from_paths|contract_path|handler|total_wired|dispatcher_count|registered' \
tests/integration/runtime/test_auto_wiring_real_manifest.py \
tests/integration/runtime/test_build_loop_auto_wiring_boot.py \
tests/integration/runtime/test_pr_state_auto_wiring_boot.py \
tests/integration/runtime/test_ledger_write_effect_dispatch_seam.py | head -n 1000
echo "=== all wire_from_manifest calls with topology keyword or keyword unpacking ==="
python3 - <<'PY'
import ast
from pathlib import Path
for path in Path("tests").rglob("*.py"):
text = path.read_text(encoding="utf-8")
try:
tree = ast.parse(text)
except SyntaxError:
continue
for node in ast.walk(tree):
if isinstance(node, ast.Call):
fn = node.func
name = fn.id if isinstance(fn, ast.Name) else fn.attr if isinstance(fn, ast.Attribute) else None
if name == "wire_from_manifest":
kws = [kw.arg for kw in node.keywords]
unpacked = [ast.unparse(kw.value) for kw in node.keywords if kw.arg is None]
print(f"{path}:{node.lineno}: keywords={kws}; unpacked={unpacked}")
PYRepository: OmniNode-ai/omnibase_infra
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -u
echo "=== db_io declarations in checked-in contracts ==="
rg -l -n '(^|[[:space:]])db_io:' src --glob 'contract.yaml' | sort | head -n 200
echo "=== database declarations in boot-test contracts ==="
for f in \
src/omnibase_infra/nodes/node_build_loop_projection_compute/contract.yaml \
src/omnibase_infra/nodes/node_build_loop_write_effect/contract.yaml \
src/omnibase_infra/nodes/node_pr_state_projection_compute/contract.yaml \
src/omnibase_infra/nodes/node_pr_state_write_effect/contract.yaml
do
if [ -f "$f" ]; then
echo "--- $f ---"
rg -n -C 3 'db_io|db_tables|database_ref|schema:' "$f" || true
fi
done
echo "=== tests that use disk db_io contracts or topology with real wiring ==="
rg -n -U -P -C 5 \
'discover_contracts(?:_from_paths)?\s*\([^)]*|wire_from_manifest\s*\([^)]*' \
tests/integration tests/unit/runtime 2>/dev/null \
| rg -n -C 2 'db_io|db_tables|topology|discover_contracts|wire_from_manifest' \
| head -n 1200Repository: OmniNode-ai/omnibase_infra
Length of output: 50383
Add startup coverage for topology.
The existing disk-contract startup tests call wire_from_manifest and verify handler wiring, but none passes topology. Extend one test to pass a resolved topology and exercise a db_io contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/runtime/test_service_kernel_database_topology_wiring.py` around
lines 66 - 86, Extend an existing disk-contract startup test that calls
wire_from_manifest to pass a resolved topology argument and include a db_io
contract in the startup scenario. Verify the resulting handler wiring still
succeeds, ensuring the test exercises topology propagation through db_io startup
rather than only asserting AST argument names.
Source: Coding guidelines
| def test_prechange_capture_pins_durable_acl_drift_predicate( | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| ) -> None: | ||
| monkeypatch.setenv("EXPECTED_PRECHANGE", str(_PRECHANGE_CAPTURE)) | ||
| proof = _load_proof(monkeypatch) | ||
| expected_path = cast("Path", proof.EXPECTED_PRECHANGE) | ||
|
|
||
| expected = json.loads(expected_path.read_text(encoding="utf-8")) | ||
| observed = json.loads(_PRECHANGE_CAPTURE.read_text(encoding="utf-8")) | ||
| assert expected == observed | ||
|
|
||
| drifted = dict(observed) | ||
| drifted["roles"] = observed["roles"][1:] | ||
| with pytest.raises(AssertionError, match="durable pre-change ACL artifact drift"): | ||
| assert expected == drifted, "durable pre-change ACL artifact drift" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the real pre-change drift comparison in the proof script so the test can target it.
fd -t f 'prove_application_database_acl.py' | while IFS= read -r f; do
echo "== $f"
ast-grep outline "$f" --items all
rg -n -C6 'EXPECTED_PRECHANGE|prechange|drift' "$f"
doneRepository: OmniNode-ai/omnibase_infra
Length of output: 13691
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '== production drift-check context =='
sed -n '2280,2315p' scripts/ci/prove_application_database_acl.py
printf '%s\n' '== test context =='
sed -n '1,125p' tests/unit/scripts/test_prove_application_database_acl.py
printf '%s\n' '== fixture shape =='
python3 - <<'PY'
import json
from pathlib import Path
path = Path("tests/fixtures/omn15547/application-acl-prechange-fixture.json.captured")
data = json.loads(path.read_text(encoding="utf-8"))
roles = data.get("roles")
print("top_level_keys:", sorted(data))
print("roles_type:", type(roles).__name__)
print("roles_count:", len(roles) if isinstance(roles, list) else None)
PY
printf '%s\n' '== AST call/compare summary =='
python3 - <<'PY'
import ast
from pathlib import Path
for name in (
"scripts/ci/prove_application_database_acl.py",
"tests/unit/scripts/test_prove_application_database_acl.py",
):
tree = ast.parse(Path(name).read_text(encoding="utf-8"), filename=name)
print(name)
for node in ast.walk(tree):
if isinstance(node, ast.Assert):
print(
" assert",
node.lineno,
ast.unparse(node.test),
"message=" + (ast.unparse(node.msg) if node.msg else "None"),
)
elif isinstance(node, ast.Call) and isinstance(node.func, ast.Name):
if node.func.id in {"main", "_load_proof"}:
print(" call", node.lineno, node.func.id)
PYRepository: OmniNode-ai/omnibase_infra
Length of output: 11100
Test the production drift predicate.
This test reads EXPECTED_PRECHANGE and observed from the same fixture, then checks only Python’s assert behavior. It does not execute the inline predicate in main() at scripts/ci/prove_application_database_acl.py:2310. Extract that comparison into a pure helper and test the helper with an explicitly valid roles payload and a drifted payload.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/scripts/test_prove_application_database_acl.py` around lines 101 -
115, Extract the inline durable pre-change ACL comparison from main() into a
pure helper, preserving the “durable pre-change ACL artifact drift” failure
behavior. Update test_prechange_capture_pins_durable_acl_drift_predicate to call
this helper with an explicitly valid roles payload, then verify it rejects the
payload with one role removed instead of testing only Python assert semantics.
2 participants
Summary
db_io.db_tablesdictionary loading with the exact typedModelDbOwnershipSubcontracton filesystem and Kafka registration pathsdb_io, canonicalhandler_routing, and runtimeevent_busfields through real registry eventsExact stack
This is a landing-blocked draft.
1f4549d71d4d39560ac5a162ac1d39e54d86e688; strictdatabase_ref+schemacontract owner.d7f1ac464a54696ac9ed05fd8f3766463a0a7eea; this branch is directly stacked on that seven-profile topology owner.1a1fff92cbf8e02e488ac6e6e89b1087ca396657; deploys the explicitONEX_DATABASE_TOPOLOGY_PROFILEconsumer seam.a0f65e9471da06b88eecb91bbc1c847989cb10d8; typed node declarations and rich relation inventory.39033d55147ef22a061b665345b506246d3aa543; service ownership manifests.e66a70bdbeedf0f4fabd4f9c50f4625bad78c5ee; registry producer preservation and strict declaration migration, pinned to this PR's exact head.OMN-15421 still owns separate tenant/internal/catalog adapters. Until it lands, non-tenant projection declarations fail closed before the existing tenant adapter can be constructed.
Runtime behavior
ModelDiscoveredContract.db_iois the typed core subcontract; raw YAML table-location reads are removed.ONEX_DATABASE_TOPOLOGY_PROFILE; event namespaces never select database topology.db_iomanifest when no explicit topology profile is loaded.Ownership validator
database:location, blocked evidence, and incomplete censuspassedwith missing rowsVerification
.200full unit suite on exact9e9e62c3177f568b7e5110f653ee4be6dcda965c: 22,380 passed, 40 skipped in 400.79sKnown landing holds
ORPHANED_CONSUMERfindings from cumulative infra audit subscriptions added after omnimarket's old pin (OMN-15006/OMN-15168). That cross-package graph stays fail-closed; this ticket adds no exemption or baseline expansion.OMN-13842process-boundary timeout aftertest_local_dispatch_evidence.py(4,450 passed, 8 skipped, 1 failed); the isolated test passes and the two-file predecessor reproducer preserves the same single failure. This is pre-existing and unrelated to the sole final infra compatibility delta.Runtime startup gate
RestartCount == 0Scope fence
No merge, OCC authoring, deploy, restart,
.201, live database access, adapter split, grant, RLS, migration application, data copy, cutover, or destructive change.Linear: OMN-15418
Evidence-Ticket: OMN-15418
Evidence-Source: OCC#5635
Summary by CodeRabbit
New Features
Bug Fixes