Repository navigation
fix(OMN-13990): recognize occ-preflight as an OCC-evidence-only failure signature - #1647
Conversation
…re signature
The receipt-gate autobind arm (OMN-13987/OMN-13990) only recognized the
Receipt Gate check name ("verify / verify") as the "green-except-OCC-
companion" signature. occ-preflight (omnibase_core occ-preflight.yml, job
"eligibility") is a SEPARATE required check that fails on the identical
signature but was never recognized — a PR whose only red check is
occ-preflight fell through to the generic RED/CODE_FAILURE branch and was
misrouted to a code-fix agent instead of the OCC evidence autobind path.
Verified live: omnibase_infra#2238 fails only "occ-preflight / eligibility"
(Resolve Evidence-Source step — missing Evidence-Source line), which this
classifier blind spot silently misclassified as CODE_FAILURE.
Widen both parallel classifiers (triage compute's _is_receipt_only_failure
and the orchestrator's _block_reason_for_fix) from an exact match on
"verify / verify" to a subset check against {"verify / verify",
"occ-preflight / eligibility"} — either check alone, or both together, with
nothing else failing, routes to OCC_DEPENDENCY / RECEIPT_EVIDENCE_SOURCE_AUTOBIND
via the existing OccAutobindAdapter (bot-authored, not implementer-self-authored
— compliant with OMN-14055's no-self-authored-evidence rule). A genuine
code-signal check alongside occ-preflight still routes to CODE_FAILURE.
Adds regression coverage for: occ-preflight-only, both-checks-together, and
occ-preflight-plus-real-failure (negative/anti-over-broaden) cases in both
the triage compute golden chain and the orchestrator's CP1 classifier suite.
Cites OMN-13990 (In Progress — "drive the OCC emitter at NORMAL closeout,
not only on a deploy-gate contract-miss"; this closes a residual blind spot
in that same effort discovered while diagnosing why omninode_infra#578/#579
and omnibase_infra#2238 had no OCC companion).
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 14 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 (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…e classification Round 2 of the occ-preflight widening (previous commit 4af42f1). A live sweep of the still-blocked PRs found the subset match still never fires: omninode_infra#579's failed_check_names is {"verify / verify", "Enable Auto-Merge"} — the cosmetic 2nd check breaks the subset comparison. "Enable Auto-Merge" fails BY DESIGN on every PR today (org-wide auto-merge is off) and is verified live NOT a required status check on any repo's dev branch protection (checked 2026-07-08: omnimarket, omnibase_infra, omniclaude, omninode_infra all omit it from branches/dev/protection/required_status_checks). Both classifiers now strip this cosmetic name before comparing the failed-check set against the OCC-evidence signature; the raw failed_check_names tuple is otherwise unchanged everywhere else in each function. Deliberately narrow: omninode_infra#578's second failing check (call-reject-skip-token / scan / reject-skip-gate-token) is NOT denylisted — verified live it IS a required context on omninode_infra's branch protection, and its current failure is a transient runner/action-download timeout, not a real skip-token match. A required-but-flaky check needs a CI rerun, not a classifier exclusion; new tests assert this case still routes to CODE_FAILURE (anti-over-broaden guard, same as any other genuine required-check failure). New tests (8 total): receipt-gate/occ-preflight each paired with the cosmetic Enable-Auto-Merge check -> autobind; the required skip-token gate paired with the receipt gate -> stays CODE_FAILURE (both orchestrator CP1 suite and triage golden-chain); cosmetic-check-plus-real-code-failure stays CODE_FAILURE (anti-over-broaden). Verified: 390 targeted unit/golden-chain tests pass (1 unrelated system-load-flaky subprocess test in test_main_cli.py independently reproduced as flaky on a clean pre-round-2 tree — 12.57s pass once load dropped, well under its 30s timeout). ruff format/check clean, mypy --strict clean, full pre-commit --files green. Cites OMN-13990.
|
| 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_pr_lifecycle_orchestrator/handlers/handler_pr_lifecycle_orchestrator.py:1163: inv_handler = HandlerPrLifecycleInventory()
src/omnimarket/nodes/node_pr_lifecycle_orchestrator/handlers/handler_pr_lifecycle_orchestrator.py:1181: triage_handler = HandlerPrLifecycleTriage()
src/omnimarket/nodes/node_pr_lifecycle_orchestrator/handlers/handler_pr_lifecycle_orchestrator.py:1199: reducer_handler = HandlerPrLifecycleStateReducer()
src/omnimarket/nodes/node_pr_lifecycle_orchestrator/handlers/handler_pr_lifecycle_orchestrator.py:1221: merge_handler = HandlerPrLifecycleMerge(
src/omnimarket/nodes/node_pr_lifecycle_orchestrator/handlers/handler_pr_lifecycle_orchestrator.py:1264: fix_handler = HandlerPrLifecycleFix(
src/omnimarket/nodes/node_pr_lifecycle_orchestrator/handlers/handler_pr_lifecycle_orchestrator.py:1294: prune_handler = HandlerWorktreePrune(git_adapter=GitWorktreeAdapter())
src/omnimarket/nodes/node_pr_lifecycle_orchestrator/handlers/handler_pr_lifecycle_orchestrator.py:2491: 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
Powered by node_pr_review_bot
✅ PR Review Bot — CLEANFindings: 0
Verdict semantics
Powered by node_pr_review_bot — FSM pipeline with multi-model review fan-out |
❓ Hostile Reviewer — UNKNOWNBlocking findings (critical/error): 0 Gate semantics (pilot phase)
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524) |
Evidence-Source: OCC#3726
Evidence-Ticket: OMN-13990
Summary
The OCC evidence-autobind arm (OMN-13987/OMN-13990) recognizes exactly one
check name —
verify / verify(the Receipt Gate) — as the "green-except-OCC-companion" failure signature that routes to
RECEIPT_EVIDENCE_SOURCE_AUTOBIND/
OCC_DEPENDENCY.occ-preflight / eligibility(omnibase_coreocc-preflight.yml, jobeligibility) is a separate required check thatfails on the exact same signature (missing/invalid
Evidence-Sourceline) butwas never recognized by either of the two parallel classifiers
(
node_pr_lifecycle_triage_compute's_is_receipt_only_failureandnode_pr_lifecycle_orchestrator's_block_reason_for_fix). A PR whose onlyred check is occ-preflight fell through to the generic RED category and was
misrouted to
CODE_FAILURE— a code-fix agent dispatch for a problem thatisn't a code problem.
Verified live, not hypothetical:
omnibase_infra#2238currently failsonly
occ-preflight / eligibility(the gate's "Resolve Evidence-Source" step— no
Evidence-Source:line in the PR body); the "verify" job itself reportsskippingfor that diff. This is exactly the blind spot above.Fix
Widen both classifiers from an exact match on
{"verify / verify"}to asubset check against
{"verify / verify", "occ-preflight / eligibility"}—either check alone, or both together, with nothing else failing, now routes
through the existing (already-built, bot-authored)
OccAutobindAdapterpath.A genuine code-signal check (lint/type/test/etc.) alongside occ-preflight
still routes to
CODE_FAILURE— this only widens the evidence-onlysignature, it does not broaden what counts as "nothing else is wrong."
This keeps evidence authorship on the sanctioned automated path
(
omnimarket-bot-authored companion viaOccAutobindAdapter, run from themerge-sweep tick) — it does not let an implementing agent self-author its own
companion, preserving OMN-14055's no-self-authored-evidence rule.
Test plan
occ-preflight-plus-real-code-failure (negative/anti-over-broaden) in
both
tests/test_golden_chain_pr_lifecycle_triage_compute.pyandtests/test_omn_13987_pr_lifecycle_arm_activation.pyuv run pytest tests/unit/nodes/node_pr_lifecycle_orchestrator tests/unit/nodes/node_pr_lifecycle_fix_effect tests/test_golden_chain_pr_lifecycle_orchestrator.py tests/test_omn_13987_pr_lifecycle_arm_activation.py tests/test_golden_chain_pr_lifecycle_triage_compute.py tests/test_pr_lifecycle_repo_health_fold.py— 385 passedruff format --check/ruff checkclean on touched filesmypy --strictclean on touched nodespre-commit run --files <touched>— full hook suite green (topic-naming-lint requiresOMNIBASE_INFRA_PATH, verified green when set; unrelated to this diff)CI note:
verify / verifyandocc-preflight / eligibilityare expectedred on THIS PR — it requires an
Evidence-Source: OCC#<PR>line citing agenuine, PR-bound OCC companion in
onex_change_control. No companion hasbeen authored yet; per OMN-14055 the implementing agent does not self-author
that evidence. This PR's own fix should make the sanctioned autobind path
pick itself up once merge-sweep ticks against it — a known, tracked gap, not
a defect in this diff.
Cites OMN-13990.