Repository navigation
feat(OMN-13092): register durable-capture event types + cross-registry consistency gate - #1751
Conversation
|
Warning Review limit reached
More reviews will be available in 3 hours, 26 minutes, and 33 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds two new durable capture event types to omniclaude's hook-emitted event system by updating topic definitions, event registries (YAML and Python), client support, allowlisting, and cross-registry validation. A new CI job enforces consistency between the local registries and the canonical omnimarket daemon registry. ChangesDurable Capture Events Implementation
Sequence Diagram(s)sequenceDiagram
participant Client as emit_event CLI
participant SupportedTypes as SUPPORTED_EVENT_TYPES
participant Registry as EVENT_REGISTRY
participant Kafka as Kafka Topics
participant DaemonRegistry as Daemon YAML
Client->>SupportedTypes: validate artifact.captured
SupportedTypes-->>Client: type recognized
Client->>Registry: resolve fan-out routes
Registry->>Kafka: publish to ARTIFACT_CAPTURED topic
Registry->>Registry: partition by correlation_id
Kafka-->>DaemonRegistry: topic must exist in fan_out
DaemonRegistry-->>Kafka: tier: duty_critical
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…y consistency gate Phase 0c (omniclaude side) of the skill-output-suppression slice (epic OMN-13089): - emit_client_wrapper SUPPORTED_EVENT_TYPES += artifact.captured, tool.output.captured - TopicBase += ARTIFACT_CAPTURED / TOOL_OUTPUT_CAPTURED (onex.evt.omnimarket.*-captured.v1) - EVENT_REGISTRY registrations (duty-critical capture trail, no-hidden-loss invariant) - live daemon launch registry (plugins/onex/lib/event_registry/omniclaude.yaml) registers both events so the --event-registry path accepts them at runtime - topic_allowlist.yaml entries (producer-side contract ownership is omnimarket node_emit_daemon contract.yaml + registries/topics.yaml) - NEW blocking gate: tests/hooks/test_registry_consistency.py — hook frozenset ⊆ omniclaude EVENT_REGISTRY ⊆ omnimarket daemon YAML registry, capture topics duty_critical; dedicated registry-consistency CI job checks out omnimarket@dev and is wired into the Tests Gate aggregator
b93d8b3 to
a432027
Compare
|
| Verdict | Meaning | Blocks merge? |
|---|---|---|
passed |
No critical/error findings | No |
blocked |
CRITICAL or ERROR findings found | Yes |
degraded |
All models unavailable (infra) | No (pilot) |
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 1225-1233: Both checkout steps ("Checkout code" and "Checkout
canonical omnimarket registry") leave Git credentials in the repo by default;
update each actions/checkout@v6 step to include persist-credentials: false in
their with: block so the runner does not persist the token to local git config
before running repository-controlled commands like uv sync and pytest.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9ab88a4e-9c09-49b0-a651-46bd91f5ffe6
📒 Files selected for processing (9)
.github/workflows/ci.ymlplugins/onex/hooks/lib/emit_client_wrapper.pyplugins/onex/lib/event_registry/omniclaude.yamlsrc/omniclaude/hooks/event_registry.pysrc/omniclaude/hooks/topic_allowlist.yamlsrc/omniclaude/hooks/topics.pytests/hooks/test_emit_client_wrapper.pytests/hooks/test_event_registry.pytests/hooks/test_registry_consistency.py
Summary
Phase 0c (omniclaude side) of the skill-output-suppression slice (epic OMN-13089), ticket OMN-13092. Pairs with merged omnimarket PR #1201 (daemon-side topic registration).
Registers the two durable-capture event types end-to-end on the hook side and adds the blocking cross-registry consistency gate:
artifact.captured→onex.evt.omnimarket.artifact-captured.v1(duty_critical)tool.output.captured→onex.evt.omnimarket.tool-output-captured.v1(duty_critical)Changes
plugins/onex/hooks/lib/emit_client_wrapper.pySUPPORTED_EVENT_TYPES+= both event types (closes F10: registered nowhere)src/omniclaude/hooks/topics.pyTopicBase.ARTIFACT_CAPTURED/TOOL_OUTPUT_CAPTUREDsrc/omniclaude/hooks/event_registry.pyEventRegistrationentries (partitioncorrelation_id, required fields mirror the daemon registry)plugins/onex/lib/event_registry/omniclaude.yaml--event-registrylaunch registry registers both events (the daemon rejects unregistered event types with "Unknown event type")src/omniclaude/hooks/topic_allowlist.yamlnode_emit_daemon(contract.yaml publish_topics + registries/topics.yaml)tests/hooks/test_registry_consistency.pyEVENT_REGISTRY⊆ omnimarket daemon YAML; per-event fan-out topic subset; capture topics exactly duty_critical.github/workflows/ci.ymlregistry-consistencyjob: checks outomnimarket@devand runs the gate against the live routing authority (wired into the Tests Gate aggregator)Pre-existing gate violations
The pre-existing pre-commit violations originally fixed in this branch (demo skill hardcoded topics, legacy CLAUDE_SESSION_ID read, stale SPDX years) landed on dev independently while this PR was in flight; this branch rebased onto them and now carries only the OMN-13092 registration + gate changes.
dod_evidence
uv run pytest tests/hooks/test_registry_consistency.py -v→ 4 passed (frozenset⊆EVENT_REGISTRY, frozenset⊆daemon YAML, duty_critical assertion, per-event topic subset)uv run pytest tests/ -m "not integration" -q→ 11,117 passed, 0 failed (integration tests require the local Redpanda bus, unavailable on this machine by design; CI runs them)uv run mypy src/ --config-file mypy.ini→ Success, no issues in 711 filesuv run ruff format/ruff check→ cleanpre-commit run --all-files→ exit 0Evidence-Ticket: OMN-13092
Evidence-Source: OCC#2588
Paired OCC receipt PRs: onex_change_control #2575 (contract + registration receipts, merged), #2581 (deploy verification receipt, merged), #2589 (omniclaude PR receipt, open), #2588 (omniclaude PR binding receipt, merged)
Summary by CodeRabbit
New Features
Tests