Repository navigation
fix(OMN-14793): single-producer enforcement lease for OccCompanionEmitter (OMN-14783 rec #2) - #1822
Conversation
…tter (OMN-14783 rec #2) Two live OccCompanionEmitter instances (the local merge_sweep mint path and the .201 effects lane) run on different hosts with no shared in-process state and force-push the SAME deterministic auto/<repo>-pr-<pr>-occ-autobind branch, so "branch exists" is not a discriminator. The OMN-14783 incident was a 5-minute sequential race (OCC b9251c78 force-pushed over by 28d50bf7) where a stale mint landed first. Adds an atomic create-if-absent lease on a git-ref in the shared OCC repo (onex_change_control) — the only durable surface both producers share. Keyed on the product PR head SHA (refs/occ-companion-leases/<repo>-pr-<pr>-<head>): first-acquirer-wins regardless of host; a second concurrent producer no-ops with ZERO side effects. TTL-based steal (server-authoritative committer timestamp, skew-free) self-heals a crashed producer's stale lease. Always-on, fail-closed (no enable/disable flag). Released in the mint's finally so a failed mint does not wedge the next attempt. - occ_git_transport: acquire_occ_companion_lease / release_occ_companion_lease (shared transport, reusable by node_occ_companion_effect when promoted). - OccCompanionEmitter._emit_companion_sync: guard before the clone/mutate block, wrapped in try/finally (covers both public entry points + all live call sites). - node_occ_companion_effect: pointer comment for future wiring (no behavior change). - Fixtures F-C1..F-C7 incl. RED-vs-EXISTS-but-WRONG (guard reverted -> both author) and fail-closed (lease transport error -> no author). Parent OMN-14783. OCC companion owed to Codex. PR-only.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
| Rule | Description |
|---|---|
| ARCH-TOPIC-001 | No hardcoded Kafka topic strings in handler code |
| ARCH-DI-001 | No event_bus=None bypass |
| ARCH-DI-002 | No direct Handler instantiation outside workflow_runner/adapters |
| ARCH-DI-003 | No reinvented DI containers |
| ARCH-TOPIC-002 | contract.yaml topics follow `onex.{cmd |
ARCH-DI-002: Direct Handler instantiation (warning)\n\n```\nsrc/omnimarket/nodes/node_occ_companion_effect/handlers/handler_occ_companion_effect.py:107: self._state_handler = state_handler or HandlerOccStateEffect()
src/omnimarket/nodes/node_pr_lifecycle_orchestrator/handlers/handler_pr_lifecycle_orchestrator.py:1301: inv_handler = HandlerPrLifecycleInventory()
src/omnimarket/nodes/node_pr_lifecycle_orchestrator/handlers/handler_pr_lifecycle_orchestrator.py:1319: triage_handler = HandlerPrLifecycleTriage()
src/omnimarket/nodes/node_pr_lifecycle_orchestrator/handlers/handler_pr_lifecycle_orchestrator.py:1337: reducer_handler = HandlerPrLifecycleStateReducer()
src/omnimarket/nodes/node_pr_lifecycle_orchestrator/handlers/handler_pr_lifecycle_orchestrator.py:1359: merge_handler = HandlerPrLifecycleMerge(
src/omnimarket/nodes/node_pr_lifecycle_orchestrator/handlers/handler_pr_lifecycle_orchestrator.py:1406: fix_handler = HandlerPrLifecycleFix(
src/omnimarket/nodes/node_pr_lifecycle_orchestrator/handlers/handler_pr_lifecycle_orchestrator.py:1441: prune_handler = HandlerWorktreePrune(git_adapter=GitWorktreeAdapter())
src/omnimarket/nodes/node_pr_lifecycle_orchestrator/handlers/handler_pr_lifecycle_orchestrator.py:1457: arm_gate_handler = HandlerPrArmGate()
src/omnimarket/nodes/node_pr_lifecycle_orchestrator/handlers/handler_pr_lifecycle_orchestrator.py:2916: verify_handler = HandlerVerify()\n```\n\nHandler classes should be wired only in workflow_runner.py or adapter modules. Direct instantiation elsewhere bypasses the contract system.\n\n
Static architectural lint — no model inference (OMN-14176).
✅ Hostile Reviewer — PASSEDBlocking findings (critical/error): 0 Gate semantics (pilot phase)
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524) |
OMN-14793 — Single-producer enforcement lease for
OccCompanionEmitterImplements recommendation #2 of OMN-14783 (parent): enforce single-producer OCC-companion minting via an idempotency/lease guard. Leaf ticket: OMN-14793.
Problem (from OMN-14783 hard evidence)
Two live
OccCompanionEmitterinstances — the localmerge_sweepmint path and the.201effects lane — run on different hosts with no shared in-process state, commit as the sameomnimarket-botidentity, render deterministic content, and force-push the SAMEauto/<repo>-pr-<pr>-occ-autobindbranch. "Branch exists" is therefore not a discriminator, and force-push is last-writer-wins with no CAS. The incident was a 5-minute sequential race: OCCb9251c78@16:40:51Z was force-pushed over by28d50bf7@16:45:53Z, letting a stale mint land first.Mechanism
An atomic create-if-absent lease on a git-ref in the shared OCC repo (
onex_change_control) — the only durable surface both producers share:refs/occ-companion-leases/<repo>-pr-<pr>-<head_sha>(keyed on the product PR head SHA, the one value both producers observe identically; producer-identity is useless since both commit asomnimarket-bot).POST /git/refs-> 201 create (won) or 422 "Reference already exists" (held). Server-side atomic; no dependence on per-host remote-tracking refs (so--force-with-leasecross-host unreliability is avoided).committer.dateis the acquisition clock — read back on 422 for a skew-free TTL decision (no host wall-clock).False).finallyso a failed/crashed mint frees the head immediately (TTL steal is only the backstop for a hard kill).Changes
src/omnimarket/occ_git_transport.py—acquire_occ_companion_lease/release_occ_companion_lease(+_create_lease_commit,_create_lease_ref,_lease_is_stale,_lease_key). Shared transport, reusable by the canonicalnode_occ_companion_effectwhen promoted; zero new dependencies (reusesrest_json/rest_no_content/split_repo, whoseGitHubApiError.status_codegives the 201/422 discrimination)..../node_pr_lifecycle_fix_effect/handlers/occ_companion_emitter.py— guard call in the CORE_emit_companion_sync(covers both public entry pointsautobind_evidence_source/create_occ_contractand every live call site), wrapped intry/finally. New ctor paramsproducer_id(forensic metadata only) +lease_ttl_seconds=900..../node_occ_companion_effect/handlers/handler_occ_companion_effect.py— pointer comment for future wiring (no behavior change; node is unwired).dod_evidence
Fixtures ride the required
occ-emitter-goldengate + pre-commit hook (no new gate — net-neutral):skip:LEASE_HELDwith empty git_calls (no clone/branch/commit/push) and no companion tree.synchronizeon a NEW head -> new key -> granted -> mints; asserts the lease is keyed on the current head SHA.finallyand re-raises.tests/unit/test_occ_git_transport_lease.py) — helper unit tests: 201->True / 422->False / 500->raise; TTL stale-steal + lost-race + delete-404-tolerated + delete-500-propagates; release swallows 404/422/5xx; exact key formatrefs/occ-companion-leases/omninode-ai-omnimarket-pr-321-<head>.Local gates (worktree venv):
ruff format+ruff checkclean ·mypy --strict src/omnimarketclean (2878 files) · new + golden + parity + effect-handler + golden-chain suites green ·pre-commiton changed files all green incl.occ-emitter-goldenandOne OCC producer per contract (OMN-14285)· runtime-sweep import-check clean (369 entry points).Scope (NOT in this PR — separate gated steps)
Disabling the local
merge_sweepmint path (the ticket's alternative), emitter retirement, cutover toggle-flip, shadow-wiringnode_occ_companion_effect, emitter-provenance stamping (rec #3), the F-06 runtime-probe remainder, and any.201/runtime/deploy work (rec #1).Closes OMN-14793.
Evidence-Source: OCC#4421
Evidence-Commit: 752d0c148d23746ecbc8e5f11c7584675429e546
Evidence-Ticket: OMN-14793