Repository navigation
fix(OMN-10972,OMN-11149,OMN-11150,OMN-11152,OMN-11153,OMN-11155): flip advisory gates to blocking in omniclaude - #1604
Merged
Conversation
…ry gates to blocking in omniclaude - OMN-11149: validate-pr-contracts.yml — remove exit 0 warn-only gate; use exit_code from validation step - OMN-11150: mypy CI step — remove || true; add blocking error exit; fix 9 mypy strict errors in delegation/runner.py, delegation/inprocess_runner.py, and runtime/introspection.py - OMN-11152: DOD_ENFORCEMENT advisory → hard in CI dod-pre-push-check step - OMN-11153: boundary-parity — remove continue-on-error; set warn-only: false - OMN-11155: model_router_hook.yaml — mode advisory → enforce OMN-11151 (skill-node boundary) deferred: 140 violations across 45 skills require upstream OMN-8088 porting work not yet complete.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
| 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)
jonahgabriel
changed the title
jonahgabriel
removed this pull request from the merge queue due to a manual request
May 20, 2026
jonahgabriel
enabled auto-merge
May 20, 2026 11:05
jonahgabriel
added this pull request to the merge queue
May 20, 2026
jonahgabriel
added a commit
that referenced
this pull request
Aug 28, 2026
…sion class in the CR-thread gate (#2057) Closes OMN-16823 ## Problem `scripts/check-unresolved-threads.sh` backs the required `gate / CodeRabbit Thread Check` context. It counted a CodeRabbit review thread as **blocking** even after CodeRabbit had conceded the finding in-thread and agreed to defer it to a ticket, because its concession match only recognized `you're right` / `apologize` / `correct behavior` / `retract` / `understood ... reasonable`. The live case is **omnibase_core#1604**, thread 2 of 3 on `src/omnibase_core/models/cli/model_cli_user_config_logging.py`. CodeRabbit's own reply, verbatim: > `@jonahgabriel`, agreed that `EnumLogLevel` cannot represent this YAML contract. Its lowercase values would cause an unintended serialized-value migration. > > The finding requested a separate uppercase-valued enum, not reuse of `EnumLogLevel`. However, deferring the validation design to OMN-16037 is reasonable because the current PR must avoid silent rewrites of existing configuration files. An unambiguous concession plus an explicit deferral — and the required context stayed red, with no path to green short of making a change the reviewer itself agreed was not required. Same family as OMN-15815 / OMN-15532: a required context unsatisfiable by construction. ## Fix — detection only The widened class is a **conjunction**, not a keyword list: - an agreement token (`agreed`, `agree`, `concur`, `fair point`, ...), **AND** - a deferral/acceptance token (`defer*`, `reasonable`, `pragmatic`, `tradeoff`, `out of scope`, `separate ticket`, `OMN-####`, ...), **AND** - **no** negation-or-escalation marker (`disagree`, `do not agree`, `still stands`, `reiterate`, `escalate`, `finding stands`, ...), evaluated over the **salient** reply body — auto-generated `<details>` blocks and HTML comments are stripped first, so CodeRabbit's own "Learnings" boilerplate cannot supply either token. The pre-existing **human-rebuttal precondition** and the **truncated-thread conservatism** both still gate it. The gate cannot degrade into "any CodeRabbit reply clears the thread". Also collapses the previously byte-duplicated concession regex in `CONCESSION_JQ` and `BLOCKING_JQ` into one shared `CR_CONCEDED_JQ` predicate. That duplication is precisely how a widened match lands in the audit line but not in the count. ## No auto-resolution — `pr_review_bot` remains the sole resolver **This change introduces no auto-resolution and no resolve path of any kind.** It changes only the wording match that decides whether a thread still *counts*. Asserted mechanically, not just in prose: the suite greps the gate script for `resolveReviewThread` / `unresolveReviewThread` / `--resolve` and fails if any appears (`PASS: no thread-resolution mutation present in the gate script`). No `[skip-*]` token, no bypass, no required-check removal is used anywhere in this change. ## Evidence — TDD red-first, negative controls proven by execution RED before the fix, with the verbatim #1604 reply as the fixture: ``` FAIL Case K: expected 0 got 1 ``` GREEN after, 24 assertions. The OMN-16823 surface: ``` PASS: Case K blocking=0 for the verbatim omnibase_core#1604 agreement-plus-deferral concession PASS: Case K concession ack names class=agreement_deferral PASS: Case L blocking=1 for an unanswered CodeRabbit finding PASS: Case M blocking=1 when CodeRabbit escalates while using deferral vocabulary PASS: Case N blocking=1 for a negated agreement (do not agree ... is reasonable) PASS: Case O blocking=1 for agreement on a sub-point with the finding kept PASS: Case P blocking=1 when concession vocabulary only appears inside a details block PASS: Case Q blocking=1 — agreement-plus-deferral without a human rebuttal is not a concession PASS: Case R blocking=1 — truncated thread stays conservative despite a concession in the fetched slice PASS: no thread-resolution mutation present in the gate script ALL TESTS PASSED ``` Cases **L–R are the permanent negative controls** required by AC3, each proven red by execution rather than argued. M is the one a bare keyword search gets wrong: CodeRabbit escalating while reusing the entire deferral vocabulary ("I disagree that deferring the validation design to OMN-16037 is reasonable"). P proves the salient-body stripping: the concession vocabulary sits only inside a `<details>Learnings</details>` block while the visible reply keeps the finding. Falsifiability is by discrimination, not assertion: restoring **only** `scripts/check-unresolved-threads.sh` to its `origin/dev` content in the same worktree and re-running reproduces `FAIL Case K: expected 0 got 1`. ## Enforcement (Operating Rule 5) The filter suite was an opt-in script wired to nothing. It now runs in two enforced places: - pre-commit hook `cr-thread-gate-filters` (verified firing: `CR-thread-gate jq filter tests (OMN-16823)....Passed`) - a step in the CI `quality` job, which feeds the required `quality-gate` context plus `tests/scripts/test_check_unresolved_threads_filters.py`, a pytest entrypoint that executes the real suite and additionally asserts by name that the positive case and all seven negative controls are still present — so a regression that quietly deletes a control fails rather than silently passing. ## dod_evidence OCC evidence companion: `contracts/OMN-16823.yaml` + `drift/dod_receipts/OMN-16823/dod-omn-16823-concession-detection-behavior/` in `onex_change_control` (merges first). Both bound checks are BEHAVIOR class and both execute the filters rather than reading a file. Evidence-Ticket: OMN-16823 Evidence-Source: OCC#7390 --------- Co-authored-by: jonahgabriel <jonahgabriel@users.noreply.github.com>
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
Graduates five advisory CI gates to blocking in omniclaude.
validate-pr-contracts.yml— removeexit 0warn-only gate; useexit ${{ steps.validate.outputs.exit_code }}so failures block merge.|| true; add blocking error exit. Fix 9 mypy strict errors:delegation/runner.py: AddTYPE_CHECKINGimport forModelBifrostResponse; type_emit_auditand_handle_routing_decisionproperly;cast(object, ...)on_build_env_configreturn (lazy import returnsAny);# type: ignore[misc]on_InProcessTransportsubclass (mixin loaded via lazy import).delegation/inprocess_runner.py: Usetask_type.valueinModelDelegationRequestconstructor (StrEnum value satisfiesLiteral["test", "document", "research"]).runtime/introspection.py:# type: ignore[misc]on_SkillNodeProxy(MixinNodeIntrospection)— pre-existing pattern, mixin loaded via lazy import.DOD_ENFORCEMENT: advisory→hardin CI dod-pre-push-check step.continue-on-error: true; setwarn-only: "false".model_router_hook.yaml—mode: advisory→mode: enforce.OMN-11151 deferred: skill-node boundary validator shows 140 violations across 45 skills. Prerequisites (OMN-8088 thin-trigger porting for remaining skills) not complete. Cannot make blocking without fixing violations.
Part of OMN-10972 (22-gate hardening sweep).
Evidence-Source: c38413e9ef595b88039e83334ec2b400d3805f0e
Evidence-Ticket: OMN-10972
Implements OMN-10972
Implements OMN-11149
Implements OMN-11150
Implements OMN-11152
Implements OMN-11153
Implements OMN-11155