Repository navigation
feat(OMN-1614, OMN-1616): Add canonical publish interface ADR and test adapter - #201
Merged
jonahgabriel merged 10 commits intoJan 27, 2026
Merged
Conversation
…t adapter
- Add ADR documenting canonical publish interface policy (OMN-1614)
- Handlers depend on ProtocolEventPublisher from SPI
- Handler-local protocols (ProtocolKafkaPublisher) are forbidden
- Include compliance grep patterns for CI enforcement
- Add TestProtocolEventPublisherAdapter for EventBusInmemory (OMN-1616)
- Implements ProtocolEventPublisher bridging to in-memory bus
- Includes decode_inmemory_event() helper for test assertions
- 31 golden tests covering topic routing, partition keys,
correlation IDs, payload integrity, and metadata preservation
- Update dependencies: omnibase-core 0.9.7, omnibase-spi 0.6.2
📝 WalkthroughWalkthroughAdds an ADR mandating a canonical ProtocolEventPublisher, implements an in‑memory ProtocolEventPublisher test adapter with envelope serialization, metrics, and helpers, adds unit and integration tests for the adapter and metrics model, re-exports a TypedDict, and bumps two internal Python dependency versions. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Code
participant Adapter as AdapterProtocolEventPublisherInmemory
participant Envelope as Envelope Builder
participant Bus as EventBusInmemory
participant Metrics as ModelPublisherMetrics
Test->>Adapter: publish(event_type, payload, correlation_id, ...)
Adapter->>Envelope: _build_envelope(params)
Envelope->>Envelope: normalize UUIDs, merge metadata/tags
Envelope-->>Adapter: ModelEventEnvelope
Adapter->>Adapter: serialize envelope -> JSON bytes
Adapter->>Adapter: encode partition_key -> UTF-8 bytes
Adapter->>Bus: publish(topic, payload_bytes, key)
Bus-->>Adapter: success/failure
Adapter->>Metrics: update counts & timing
Adapter-->>Test: return success (bool)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@docs/decisions/adr-canonical-publish-interface-policy.md`:
- Around line 174-176: The fenced code block containing the file path
"omnibase_spi/src/omnibase_spi/protocols/event_bus/protocol_event_publisher.py"
is missing a language specifier; update the markdown fenced block to include the
language tag `text` (i.e., change the opening ``` to ```text) so the block is
treated as plain text in rendered docs.
- Around line 52-74: Update the fenced ASCII diagram in the
docs/decisions/adr-canonical-publish-interface-policy.md by adding an explicit
language specifier (e.g., change the opening ``` to ```text) so linters
recognize the block as plain text; locate the ASCII diagram block (the
multi-line box diagram) and modify its opening fence accordingly without
changing the diagram content.
In
`@tests/integration/event_bus/test_adapter_protocol_event_publisher_inmemory.py`:
- Around line 199-219: The test test_partition_key_unicode_encoding currently
uses an ASCII-only partition_key so it doesn't validate UTF-8 handling; update
the partition_key in that test (which runs against
AdapterProtocolEventPublisherInmemory and EventBusInmemory and later checks
event_bus.get_event_history) to include non-ASCII characters (e.g.,
Chinese/Japanese/Cyrillic or emoji) and keep the existing assertion that
history[0].key == partition_key.encode("utf-8") so the roundtrip encoding is
actually verified.
- Around line 543-548: The test's payload uses ASCII placeholders instead of
real Unicode; update the payload dict defined as payload in
tests/integration/event_bus/test_adapter_protocol_event_publisher_inmemory.py to
include real Unicode characters (e.g., "greeting_ja": "こんにちは", "greeting_ar":
"مرحبا", "emoji": "😊", "special": "çåß©—" or similar) so the test actually
verifies Unicode preservation; keep the same keys and ensure any assertions that
compare or serialize this payload continue to use the updated payload variable.
🧹 Nitpick comments (4)
src/omnibase_infra/event_bus/testing/model_publisher_metrics.py (1)
45-61: Consider usingmodel_dump()instead of customto_dict().Pydantic's built-in
model_dump(mode="json")already provides JSON-serializable output. The customto_dict()method duplicates this functionality and requires manual maintenance when fields are added.However, the explicit
dict[str, JsonType]return type does provide stronger typing guarantees thanmodel_dump(), so this is acceptable if consistency withJsonTypeis preferred.tests/integration/event_bus/test_adapter_protocol_event_publisher_inmemory.py (1)
784-789: Consider catchingpydantic.ValidationErrorexplicitly.The comment indicates a Pydantic ValidationError is expected, but the test catches the generic
Exception. Using the specific exception type improves test precision and documentation.♻️ Proposed fix
+from pydantic import ValidationError + def test_decode_non_envelope_json_raises(self) -> None: """Verify decode_inmemory_event raises on non-envelope JSON.""" non_envelope_json = b'{"random": "data"}' - with pytest.raises(Exception): # Pydantic ValidationError + with pytest.raises(ValidationError): decode_inmemory_event(non_envelope_json)src/omnibase_infra/event_bus/testing/adapter_protocol_event_publisher_inmemory.py (2)
167-168: Consider usingOnexErrorsubclass instead ofRuntimeError.Per coding guidelines, adapters should "only raise
OnexErroror its subclasses." For a closed publisher state,ProtocolConfigurationErroror a custom error would align better with the codebase conventions.♻️ Suggested change
+from omnibase_spi.errors import ProtocolConfigurationError + ... if self._closed: - raise RuntimeError("Publisher has been closed") + raise ProtocolConfigurationError("Publisher has been closed")Based on coding guidelines requiring
OnexErroror subclasses.
224-238: Broad exception handling may mask underlying issues.The
except Exceptionblock catches all errors and returnsFalse, which could silently hide unexpected bugs during test development. Consider either:
- Re-raising after logging for critical/unexpected exceptions
- Being more specific about expected exception types
For a test adapter, this fail-safe approach is reasonable, but be aware it could mask configuration or usage errors.
Comment on lines
+52
to
+74
| ``` | ||
| ┌─────────────────────────────────────────────────────────┐ | ||
| │ HANDLERS │ | ||
| │ Business logic, orchestration, event processing │ | ||
| │ Depends on: omnibase_spi.ProtocolEventPublisher │ | ||
| └─────────────────────────────────────────────────────────┘ | ||
| │ | ||
| │ Semantic publish interface | ||
| ▼ | ||
| ┌─────────────────────────────────────────────────────────┐ | ||
| │ INFRA LAYER │ | ||
| │ Transport adapters, connection pools, bytes encoding │ | ||
| │ Implements: ProtocolEventPublisher │ | ||
| │ Owns: Kafka producer, retry logic, circuit breaker │ | ||
| └─────────────────────────────────────────────────────────┘ | ||
| │ | ||
| │ Wire protocol (bytes) | ||
| ▼ | ||
| ┌─────────────────────────────────────────────────────────┐ | ||
| │ KAFKA/REDPANDA │ | ||
| │ Message broker, partitions, topics │ | ||
| └─────────────────────────────────────────────────────────┘ | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to fenced code block.
The ASCII diagram code block lacks a language specifier. While this is an ASCII diagram, adding text or leaving it as a plain block is acceptable, but linters prefer explicit language hints.
🔧 Proposed fix
-```
+```text
┌─────────────────────────────────────────────────────────┐
│ HANDLERS │🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
52-52: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/decisions/adr-canonical-publish-interface-policy.md` around lines 52 -
74, Update the fenced ASCII diagram in the
docs/decisions/adr-canonical-publish-interface-policy.md by adding an explicit
language specifier (e.g., change the opening ``` to ```text) so linters
recognize the block as plain text; locate the ASCII diagram block (the
multi-line box diagram) and modify its opening fence accordingly without
changing the diagram content.
Comment on lines
+174
to
+176
| ``` | ||
| omnibase_spi/src/omnibase_spi/protocols/event_bus/protocol_event_publisher.py | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to fenced code block.
The file path code block lacks a language specifier. Use text for plain text content.
🔧 Proposed fix
-```
+```text
omnibase_spi/src/omnibase_spi/protocols/event_bus/protocol_event_publisher.py</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
174-174: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/decisions/adr-canonical-publish-interface-policy.md` around lines 174 -
176, The fenced code block containing the file path
"omnibase_spi/src/omnibase_spi/protocols/event_bus/protocol_event_publisher.py"
is missing a language specifier; update the markdown fenced block to include the
language tag `text` (i.e., change the opening ``` to ```text) so the block is
treated as plain text in rendered docs.
added 4 commits
January 27, 2026 13:10
- Fix metadata parameter type to use ContextValue instead of object, ensuring AdapterProtocolEventPublisherInmemory conforms to ProtocolEventPublisher protocol - Update test_unicode_payload_preserved to use actual Unicode characters (Japanese, Arabic, emoji, Latin-1 extended) instead of ASCII placeholders
- Remove unused `datetime` import from test file (only UTC is used) - Add `__all__` export list to model_publisher_metrics.py for consistency
Enhance TestProtocolCompliance with: - isinstance() check against @runtime_checkable ProtocolEventPublisher - Method signature verification (params, defaults) - Async method verification via iscoroutinefunction() - Return type verification for publish() and get_metrics() - Required metrics keys verification This follows the ONEX protocol conformance pattern where implementations use duck typing (structural subtyping) verified through test assertions rather than explicit inheritance.
- Add protocol compliance test verifying adapter assignable to ProtocolEventPublisher - Replace type: ignore with explicit cast() for type safety - Add unit tests for ModelPublisherMetrics (6 tests covering defaults, to_dict, mutability, constraints)
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@src/omnibase_infra/event_bus/testing/adapter_protocol_event_publisher_inmemory.py`:
- Around line 165-169: Replace the RuntimeError raised when self._closed is true
with an OnexError subclass that includes
ModelInfraErrorContext.with_correlation(...) and preserves error chaining;
specifically, where the code currently does "if self._closed: raise
RuntimeError('Publisher has been closed')", construct a contextual Onex error
(e.g., raise
YourOnexAdapterClosedError(ModelInfraErrorContext.with_correlation(self._correlation_id),
"Publisher has been closed") from RuntimeError("Publisher has been closed")) so
the adapter follows the OnexError-only rule and retains the original exception
as the cause; add any required imports for the chosen Onex error class and
ModelInfraErrorContext.
- Around line 264-304: The code currently generates a new UUID when
correlation_id is invalid but only logs the original value; update the logic in
the method that builds envelope_metadata (where corr_uuid is computed and
ModelEnvelopeMetadata(tags=tags) is created) to persist the original invalid
correlation_id into the tags/metadata so it is not dropped: when catching
ValueError, add tags["original_correlation_id"]=str(correlation_id) (or similar
key), and ensure tags["correlation_id"]=str(corr_uuid) is set so the generated
UUID is used as the canonical correlation_id while the original is preserved;
keep using ModelEnvelopeMetadata(tags=tags) to store both values.
In
`@tests/integration/event_bus/test_adapter_protocol_event_publisher_inmemory.py`:
- Around line 811-825: Remove the test method test_isinstance_protocol_check
from the tests file: delete the entire function definition that asserts
isinstance(adapter, ProtocolEventPublisher); instead rely on existing tests
test_publish_signature_matches_protocol, test_close_signature_matches_protocol,
and test_adapter_assignable_to_protocol to validate protocol conformance for
AdapterProtocolEventPublisherInmemory against ProtocolEventPublisher.
Comment on lines
+165
to
+169
| Raises: | ||
| RuntimeError: If adapter has been closed. | ||
| """ | ||
| if self._closed: | ||
| raise RuntimeError("Publisher has been closed") |
There was a problem hiding this comment.
Use OnexError (with context) instead of RuntimeError for closed publisher.
The closed-path raises RuntimeError, which violates the “OnexError-only” rule and error context requirements for adapters. Please raise the appropriate OnexError subclass with ModelInfraErrorContext.with_correlation(...) and error chaining. As per coding guidelines, ...
🤖 Prompt for AI Agents
In
`@src/omnibase_infra/event_bus/testing/adapter_protocol_event_publisher_inmemory.py`
around lines 165 - 169, Replace the RuntimeError raised when self._closed is
true with an OnexError subclass that includes
ModelInfraErrorContext.with_correlation(...) and preserves error chaining;
specifically, where the code currently does "if self._closed: raise
RuntimeError('Publisher has been closed')", construct a contextual Onex error
(e.g., raise
YourOnexAdapterClosedError(ModelInfraErrorContext.with_correlation(self._correlation_id),
"Publisher has been closed") from RuntimeError("Publisher has been closed")) so
the adapter follows the OnexError-only rule and retains the original exception
as the cause; add any required imports for the chosen Onex error class and
ModelInfraErrorContext.
Comment on lines
+264
to
+304
| # Convert correlation_id string to UUID if provided | ||
| corr_uuid: UUID | None = None | ||
| if correlation_id is not None: | ||
| try: | ||
| corr_uuid = UUID(str(correlation_id)) | ||
| except ValueError: | ||
| # If not a valid UUID, generate one and store original in metadata | ||
| corr_uuid = uuid4() | ||
| logger.warning( | ||
| "correlation_id is not a valid UUID, using generated UUID", | ||
| extra={ | ||
| "original_correlation_id": correlation_id, | ||
| "generated_uuid": str(corr_uuid), | ||
| }, | ||
| ) | ||
|
|
||
| # Build metadata tags | ||
| tags: dict[str, str] = { | ||
| "event_type": event_type, | ||
| "service_name": self._service_name, | ||
| "instance_id": self._instance_id, | ||
| } | ||
|
|
||
| # Store causation_id in tags (ModelEventEnvelope doesn't have dedicated field) | ||
| if causation_id is not None: | ||
| tags["causation_id"] = str(causation_id) | ||
|
|
||
| # Merge additional metadata context values into tags | ||
| if metadata is not None and isinstance(metadata, dict): | ||
| for key, value in metadata.items(): | ||
| # Context values may have a serialize_for_context method or be simple types | ||
| if hasattr(value, "serialize_for_context"): | ||
| serialized = value.serialize_for_context() | ||
| tags[key] = json.dumps(serialized) | ||
| elif hasattr(value, "value"): | ||
| # ProtocolContext*Value types have a value attribute | ||
| tags[key] = str(value.value) | ||
| else: | ||
| tags[key] = str(value) | ||
|
|
||
| envelope_metadata = ModelEnvelopeMetadata(tags=tags) |
There was a problem hiding this comment.
Invalid correlation_id is dropped despite comment.
The comment says the original ID is stored in metadata, but it’s only logged. Either persist it in tags or update the comment to match behavior.
💡 Suggested fix to preserve original correlation_id
- corr_uuid: UUID | None = None
+ corr_uuid: UUID | None = None
+ original_correlation_id: str | None = None
if correlation_id is not None:
try:
corr_uuid = UUID(str(correlation_id))
except ValueError:
# If not a valid UUID, generate one and store original in metadata
corr_uuid = uuid4()
+ original_correlation_id = str(correlation_id)
logger.warning(
"correlation_id is not a valid UUID, using generated UUID",
extra={
"original_correlation_id": correlation_id,
"generated_uuid": str(corr_uuid),
},
)
@@
tags: dict[str, str] = {
"event_type": event_type,
"service_name": self._service_name,
"instance_id": self._instance_id,
}
+ if original_correlation_id is not None:
+ tags["original_correlation_id"] = original_correlation_id🤖 Prompt for AI Agents
In
`@src/omnibase_infra/event_bus/testing/adapter_protocol_event_publisher_inmemory.py`
around lines 264 - 304, The code currently generates a new UUID when
correlation_id is invalid but only logs the original value; update the logic in
the method that builds envelope_metadata (where corr_uuid is computed and
ModelEnvelopeMetadata(tags=tags) is created) to persist the original invalid
correlation_id into the tags/metadata so it is not dropped: when catching
ValueError, add tags["original_correlation_id"]=str(correlation_id) (or similar
key), and ensure tags["correlation_id"]=str(corr_uuid) is set so the generated
UUID is used as the canonical correlation_id while the original is preserved;
keep using ModelEnvelopeMetadata(tags=tags) to store both values.
Comment on lines
+811
to
+825
| def test_isinstance_protocol_check( | ||
| self, | ||
| adapter: AdapterProtocolEventPublisherInmemory, | ||
| ) -> None: | ||
| """Verify adapter passes isinstance check for ProtocolEventPublisher. | ||
|
|
||
| This test uses @runtime_checkable to verify structural subtyping. | ||
| The adapter must implement the same method signatures as the protocol. | ||
| """ | ||
| from omnibase_spi.protocols.event_bus import ProtocolEventPublisher | ||
|
|
||
| assert isinstance(adapter, ProtocolEventPublisher), ( | ||
| "AdapterProtocolEventPublisherInmemory must implement ProtocolEventPublisher. " | ||
| "Check that all required methods are present with correct signatures." | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "test_adapter_protocol_event_publisher_inmemory.py" --type fRepository: OmniNode-ai/omnibase_infra
Length of output: 147
🏁 Script executed:
wc -l tests/integration/event_bus/test_adapter_protocol_event_publisher_inmemory.pyRepository: OmniNode-ai/omnibase_infra
Length of output: 151
🏁 Script executed:
sed -n '805,830p' tests/integration/event_bus/test_adapter_protocol_event_publisher_inmemory.pyRepository: OmniNode-ai/omnibase_infra
Length of output: 1136
🏁 Script executed:
grep -n "def test_" tests/integration/event_bus/test_adapter_protocol_event_publisher_inmemory.py | head -20Repository: OmniNode-ai/omnibase_infra
Length of output: 1134
🏁 Script executed:
grep -n "def test_" tests/integration/event_bus/test_adapter_protocol_event_publisher_inmemory.py | grep -i "protocol\|signature\|assignment"Repository: OmniNode-ai/omnibase_infra
Length of output: 266
🏁 Script executed:
sed -n '841,880p' tests/integration/event_bus/test_adapter_protocol_event_publisher_inmemory.pyRepository: OmniNode-ai/omnibase_infra
Length of output: 1337
🏁 Script executed:
sed -n '919,978p' tests/integration/event_bus/test_adapter_protocol_event_publisher_inmemory.pyRepository: OmniNode-ai/omnibase_infra
Length of output: 2181
🏁 Script executed:
sed -n '980,988p' tests/integration/event_bus/test_adapter_protocol_event_publisher_inmemory.pyRepository: OmniNode-ai/omnibase_infra
Length of output: 549
Remove test_isinstance_protocol_check test method.
The isinstance() check violates coding guidelines which require duck typing for protocols. The method at lines 811-825 is redundant; protocol conformance is already verified by test_publish_signature_matches_protocol, test_close_signature_matches_protocol, and test_adapter_assignable_to_protocol which use signature validation and static type assignment respectively.
🤖 Prompt for AI Agents
In
`@tests/integration/event_bus/test_adapter_protocol_event_publisher_inmemory.py`
around lines 811 - 825, Remove the test method test_isinstance_protocol_check
from the tests file: delete the entire function definition that asserts
isinstance(adapter, ProtocolEventPublisher); instead rely on existing tests
test_publish_signature_matches_protocol, test_close_signature_matches_protocol,
and test_adapter_assignable_to_protocol to validate protocol conformance for
AdapterProtocolEventPublisherInmemory against ProtocolEventPublisher.
added 2 commits
January 27, 2026 14:06
- Remove unused UTC import from test file - Add NOTE comment explaining intentional broad exception catch
- Fix Unicode test to use actual Unicode characters (Japanese, emoji, accented) instead of ASCII-only partition key - Fix misleading comment about storing invalid correlation_id in metadata (it's logged for debugging, not stored)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@tests/integration/event_bus/test_adapter_protocol_event_publisher_inmemory.py`:
- Around line 721-740: Update the test_close_prevents_further_publishes test to
expect the OnexError subclass that AdapterProtocolEventPublisherInmemory.publish
raises after AdapterProtocolEventPublisherInmemory.close, not RuntimeError;
specifically replace the pytest.raises(RuntimeError, match="Publisher has been
closed") with pytest.raises(OnexError or the exact OnexError subclass your
adapter uses) (and add the appropriate import) to align the test with the
OnexError-only rule.
🧹 Nitpick comments (1)
src/omnibase_infra/event_bus/testing/adapter_protocol_event_publisher_inmemory.py (1)
356-383: Use JsonType in the decode helper signature.This helper returns JSON-compatible payloads, so annotate it with JsonType instead of object for consistency and downstream typing. As per coding guidelines.
♻️ Proposed refactor
-def decode_inmemory_event(value: bytes) -> ModelEventEnvelope[object]: +def decode_inmemory_event(value: bytes) -> ModelEventEnvelope[JsonType]: @@ - return ModelEventEnvelope[object].model_validate(decoded_dict) + return ModelEventEnvelope[JsonType].model_validate(decoded_dict)
Comment on lines
+721
to
+740
| @pytest.mark.asyncio | ||
| async def test_close_prevents_further_publishes( | ||
| self, | ||
| event_bus: EventBusInmemory, | ||
| adapter: AdapterProtocolEventPublisherInmemory, | ||
| ) -> None: | ||
| """Verify close() prevents further publish calls.""" | ||
| # Publish one event first | ||
| success = await adapter.publish( | ||
| event_type="before.close", | ||
| payload={"data": "test"}, | ||
| ) | ||
| assert success is True | ||
|
|
||
| # Close the adapter | ||
| await adapter.close() | ||
|
|
||
| # Attempting to publish after close should raise | ||
| with pytest.raises(RuntimeError, match="Publisher has been closed"): | ||
| await adapter.publish( |
There was a problem hiding this comment.
Align close() error expectation with OnexError-only rule.
Once the adapter raises an OnexError subclass for closed state, this test should assert that error type instead of RuntimeError to avoid locking in non-compliant behavior. As per coding guidelines.
🤖 Prompt for AI Agents
In
`@tests/integration/event_bus/test_adapter_protocol_event_publisher_inmemory.py`
around lines 721 - 740, Update the test_close_prevents_further_publishes test to
expect the OnexError subclass that AdapterProtocolEventPublisherInmemory.publish
raises after AdapterProtocolEventPublisherInmemory.close, not RuntimeError;
specifically replace the pytest.raises(RuntimeError, match="Publisher has been
closed") with pytest.raises(OnexError or the exact OnexError subclass your
adapter uses) (and add the appropriate import) to align the test with the
OnexError-only rule.
added 3 commits
January 27, 2026 14:40
- Add TypedDictEnvelopeBuildParams for _build_envelope parameters - Improves type safety by replacing dict[str, object] with proper TypedDict - Add test for get_metrics() behavior after adapter close - Documents that metrics remain accessible after close for debugging
- Add reset_metrics() method to AdapterProtocolEventPublisherInmemory for test isolation when reusing adapter across test cases - Fix TypedDictEnvelopeBuildParams to use NotRequired for optional fields instead of total=False which incorrectly made all fields optional - Add 3 tests for reset_metrics() covering counter clearing, closed state preservation, and fresh counting after reset Addresses code review feedback from PR #201.
Resolves conflict in docs/decisions/README.md by including both: - adr-canonical-publish-interface-policy (this branch) - adr-consumer-group-naming (from main) Both ADRs are listed in alphabetical order.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/omnibase_infra/event_bus/testing/adapter_protocol_event_publisher_inmemory.py`:
- Around line 111-128: Change the constructor to follow project DI by accepting
a ModelONEXContainer (ModelONEXContainer) instead of directly requiring an
EventBusInmemory; update __init__ in adapter_protocol_event_publisher_inmemory
(the class constructor shown) to accept container: ModelONEXContainer and
optionally keep bus: EventBusInmemory | None for backward compatibility, then
resolve the EventBusInmemory from the container when bus is None and preserve
existing assignments for _service_name, _instance_id, _metrics and _closed;
alternatively, if you intend a test-only exemption, add a small factory function
that takes ModelONEXContainer and returns the adapter instance and document the
exemption instead of changing DI signature inline.
Comment on lines
+111
to
+128
| def __init__( | ||
| self, | ||
| bus: EventBusInmemory, | ||
| service_name: str = "test-service", | ||
| instance_id: str | None = None, | ||
| ) -> None: | ||
| """Initialize the adapter with an EventBusInmemory instance. | ||
|
|
||
| Args: | ||
| bus: The EventBusInmemory instance to bridge to. | ||
| service_name: Service name for envelope metadata. Defaults to "test-service". | ||
| instance_id: Optional instance identifier. Defaults to a generated UUID. | ||
| """ | ||
| self._bus = bus | ||
| self._service_name = service_name | ||
| self._instance_id = instance_id or str(uuid4()) | ||
| self._metrics = ModelPublisherMetrics() | ||
| self._closed = False |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use ModelONEXContainer-based DI in adapter __init__.
adapter_*.py constructors are required to accept ModelONEXContainer for dependency injection. Please align the signature (or document an explicit test-only exemption and add a factory) so adapter initialization is consistent with project DI conventions. As per coding guidelines, ...
🤖 Prompt for AI Agents
In
`@src/omnibase_infra/event_bus/testing/adapter_protocol_event_publisher_inmemory.py`
around lines 111 - 128, Change the constructor to follow project DI by accepting
a ModelONEXContainer (ModelONEXContainer) instead of directly requiring an
EventBusInmemory; update __init__ in adapter_protocol_event_publisher_inmemory
(the class constructor shown) to accept container: ModelONEXContainer and
optionally keep bus: EventBusInmemory | None for backward compatibility, then
resolve the EventBusInmemory from the container when bus is None and preserve
existing assignments for _service_name, _instance_id, _metrics and _closed;
alternatively, if you intend a test-only exemption, add a small factory function
that takes ModelONEXContainer and returns the adapter instance and document the
exemption instead of changing DI signature inline.
jonahgabriel
deleted the
jonah/omn-1614-omn-1616-publish-interface-policy
branch
January 27, 2026 20:57
6 of 9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OMN-1614: Add ADR documenting canonical publish interface policy
ProtocolEventPublisherfrom SPIProtocolKafkaPublisher) are forbiddenOMN-1616: Add
TestProtocolEventPublisherAdapterforEventBusInmemoryProtocolEventPublisherbridging to in-memory busdecode_inmemory_event()helper for test assertionsUpdate dependencies:
omnibase-core0.9.7,omnibase-spi0.6.2Files Changed
docs/decisions/adr-canonical-publish-interface-policy.mddocs/decisions/README.mdsrc/omnibase_infra/event_bus/testing/__init__.pysrc/omnibase_infra/event_bus/testing/adapter_protocol_event_publisher_inmemory.pysrc/omnibase_infra/event_bus/testing/model_publisher_metrics.pytests/integration/event_bus/test_adapter_protocol_event_publisher_inmemory.pypyproject.tomlpoetry.lockTest Plan
Linear Tickets
Summary by CodeRabbit
Documentation
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.