Repository navigation
contracts(OMN-12154): add OCC contract for Valkey rate limiter fix - #1652
Merged
Merged
Conversation
Documents the ValkeyRateLimiter implementation that replaces InMemoryRateLimiter in omninode_infra. DoD evidence: PR #472, 11 unit tests, 2 integration tests, 171 existing tests with no regressions.
|
Warning Review limit reached
More reviews will be available in 43 minutes and 16 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ 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 (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
jonahgabriel
force-pushed
the
jonah/omn-12154-occ-contract
branch
from
May 26, 2026 02:02
9995f26 to
356e262
Compare
andywu42
pushed a commit
to andywu42/onex_change_control
that referenced
this pull request
Jun 10, 2026
…platform PRs (OmniNode-ai#1580) * evidence(OMN-11982): add OCC contracts and receipts for knowledge platform PRs Adds contracts and DoD receipts for three knowledge platform tickets that are blocking omniclaude PRs OmniNode-ai#1650, OmniNode-ai#1651, and OmniNode-ai#1652: - OMN-11938: recall skill --scope flag and knowledge federation (omniclaude OmniNode-ai#1650) - OMN-11940: design_to_plan Phase 0 knowledge preload (omniclaude OmniNode-ai#1651) - OMN-11941: ticket-work research enrichment documentation (omniclaude OmniNode-ai#1652) Lane 1 (LLM topology) and Lane 5 (hardcoded model cleanup) are already covered by prior merged OCC PRs (OmniNode-ai#1570 OMN-11970, OmniNode-ai#1569 OMN-11971). * evidence(OMN-11982): add self-reference contract for Wave 2 OCC PR Add contracts/OMN-11982.yaml and receipt so the OCC eligibility check passes for this PR. The receipt gate requires every OCC PR to have its own ticket contract present at the pinned commit SHA. * fix(OMN-11982): set real contract_sha256 digests in all receipts The ModelDodReceipt validator requires contract_sha256 to be a real sha256:<64-hex> digest. Update all four receipts with actual file hashes computed from the contract YAML files in this PR.
jonahgabriel
added a commit
that referenced
this pull request
Sep 7, 2026
* evidence(OMN-17554): repoint the evidence contract at the merged PR #1658 The contract was autobound to omnibase_core#1652, which was CLOSED without merging after review rejected its doctor boundary (mergeCommit null, mergedAt null). Every check on it therefore rested on a pull request that never landed, and dod_verify reported 2/8 with 2 failed against work that is in fact merged and green: one failure was "#1652 not merged", the other a behavior proof naming tests/test_doctor/test_checks_env_typed_binding.py and tests/unit/models/configuration/test_extra_forbid_omn17554.py, two candidate-era paths that never existed in the landed tree. Repointed to #1658, squash 90148c953dd1d02cdeb08d14df52a35178f5af6c, merged 2026-09-07T13:11:28Z with 165 SUCCESS / 1 SKIPPED / 0 FAILURE. Each content check was selected because its exit status FLIPS across the merge, verified in both directions before being written here rather than assumed: load_declared_user_config is present at the merge commit and absent at the parent; constants_workflow.py holds zero raw env reads at the merge commit and one at the parent; the AST pin test is in the PR file list and 404s at the parent. A fourth candidate check, asserting the rejected _build_env_vars_check helper is absent, was dropped after its negative control showed the helper was absent at the parent too, so it would have been a vacuous check of exactly the kind this ticket exists to remove. The behavior proof now names the five test targets the PR actually adds or changes: 100 passed in 5.78s. Refs: OMN-17554 * evidence(OMN-17554): bind the merged PR #1658, add-only Reduces this branch to a strictly additive change. Its first commit removed dod_evidence item 'dod-OmniNode-ai-omnibase_core-pr-1652' and edited 'dod-occ-diff-derived-behavior-proof'; merged entries are immutable and the append-only gate correctly rejected both. `git diff origin/dev` on this branch now has zero removed lines. Adds one net-new item, 'dod-OmniNode-ai-omnibase_core-pr-1658'. Its receipt was already on dev, minted by the autobind at drift/dod_receipts/OMN-17554/ dod-OmniNode-ai-omnibase_core-pr-1658/command.yaml, with no contract item to bind to; this supplies it. #1658 is the change that actually landed: merged 2026-09-07T13:11:28Z, squash 90148c953dd1d02cdeb08d14df52a35178f5af6c, 165 SUCCESS / 1 SKIPPED / 0 FAILURE. Each of its four checks was verified to FLIP across the merge before being written here, so none can pass with the product change reverted: at merge commit vs parent 5b27eb5c, load_declared_user_config counts 1 vs 0; the raw env-read count in constants_workflow.py is 0 vs 1; the AST pin test is in the PR file list and 404s at the parent; and the merge parent assertion is exact. Verified locally before pushing: the append-only validator reports "No append-only violations", and check_receipt_hardening.py --supersession-corpus reports 2344 against a 2344 baseline, "Corpus matches the frozen baseline exactly". NOT fixed here, and filed as a residual on the ticket instead: the autobind's two supersession records for this ticket are stale, and repairing them is currently blocked by two mutually exclusive rules inside scripts/validation/check_receipt_hardening.py. _repaired_targets (~line 1800) requires a repair record whose `supersedes:` names the mis-paired .supersede. record; _supersession_chain_violations (line 1302) rejects any receipt whose `supersedes:` names a .supersede. record, unconditionally and fail-closed. A record satisfying either rule is refused by the other, so the documented OMN-15459 repair path cannot currently be taken. Refs: OMN-17554 * evidence(OMN-17554): add the OCC self-bind and rebind the 1658 receipt hash Two additions the eligibility and hardening gates asked for by name, both strictly add-only. Net change for this ticket is 92 insertions, 0 deletions. occ-preflight reported reason=missing_occ_self_bind: OCC evidence for OMN-17554 verifies (receipts PASS, hashes bound, missing_or_nonpass_receipts empty) but no receipt bound to this evidence PR itself. Adds item occ-self-bind-pr-8596 and its PASS receipt with pr_number 8596. Appending that item moved the contract's whole-file hash, which invalidated dod-OmniNode-ai-omnibase_core-pr-1658/command.yaml — an autobind receipt minted 2026-09-06 carrying only contract_sha256 and no per-entry hash (OMN-13888). That receipt is merged and immutable, so it is corrected by a net-new supersession rather than an edit. The record names the BASE receipt, not another supersession, so it is not a chain; it mints contract_entry_sha256 so future appends to other entries cannot invalidate it again, and it re-probes at the merge commit 90148c95 rather than the superseded intermediate head abae24cf. Verified locally before pushing, each gate run on the exact staged tree: receipt hardening on all three touched receipts reports no violations; --supersession-corpus reports 2344 against a 2344 baseline, "Corpus matches the frozen baseline exactly"; the append-only validator reports "No append-only violations: existing entries and receipts unchanged." Refs: OMN-17554
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
Adds OCC contract for OMN-12154. The contract documents the ValkeyRateLimiter implementation in omninode_infra PR #472 that replaces InMemoryRateLimiter to fix multi-worker rate limiting correctness.
DoD Evidence
Ticket: OMN-12154