Add fail-closed operational decision gates - #12
Open
Deathcharge wants to merge 8 commits into
Open
Conversation
|
Warning Review limit reached
Next review available in: 47 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
Summary by CodeRabbit
WalkthroughChangesThe package adds immutable, auditable decision policies with fail-closed evaluation. It exports the policy APIs, hardens consensus validation and hashing, adds release-gate examples, and updates integration, release, CI, and product documentation. Decision policy and release integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseGate
participant ConsensusEngine
participant evaluate_decision
participant DecisionPolicy
ReleaseGate->>ConsensusEngine: gather weighted participant responses
ConsensusEngine-->>ReleaseGate: return ConsensusResult
ReleaseGate->>evaluate_decision: evaluate consensus
evaluate_decision->>DecisionPolicy: apply policy constraints
evaluate_decision-->>ReleaseGate: return DecisionVerdict
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deathcharge
marked this pull request as ready for review
August 11, 2026 03:33
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 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 19-20: Set persist-credentials: false on all three
actions/checkout steps in .github/workflows/ci.yml at lines 19-20, 44-45, and
58-59; the actions/setup-python step at lines 19-20 requires no change.
In `@agent_consensus/models.py`:
- Around line 281-293: Update _validate_decision_settings to accept a
caller-facing quorum_field label and use it in both min_successful validation
error messages. Pass quorum_field="min_votes" from evaluate_votes while
preserving the existing label for the other public entry point.
In `@agent_consensus/policy.py`:
- Around line 278-279: Update the minimum-weight check in the policy evaluation
logic to use math.isclose with the existing tolerance pattern, treating
successful_weight as meeting min_successful_weight when equal within
floating-point tolerance; only append SUCCESSFUL_WEIGHT_BELOW_MINIMUM when it is
genuinely below the threshold.
- Around line 131-136: Parenthesize the combined set expression in the
allowed_choices validation within the configuration checks, making the subset
comparison explicitly evaluate (pass_choices | veto_choices) against
allowed_choices while preserving the existing error behavior.
- Around line 229-232: Update evaluate_decision’s input validation to use an
exception within the package hierarchy, preferably by adding DecisionInputError
in errors.py as a ConsensusError and TypeError subclass, then raising it for
invalid consensus and policy arguments. Preserve existing TypeError
compatibility while ensuring callers catching ConsensusError handle these
rejections.
- Around line 288-298: Replace the any-based membership check in the incomplete
calculation with an intersection against a module-level frozenset of incomplete
DecisionReason values, defined alongside the enums. Preserve the existing reason
set exactly and derive incomplete from whether the intersection with reasons is
non-empty.
In `@docs/API_REFERENCE.md`:
- Around line 102-111: Update the DecisionPolicy signature documentation so
veto_choices and required_participants use empty frozenset defaults, matching
the actual defaults in policy.py; leave pass_choices unchanged.
In `@docs/INTEGRATIONS.md`:
- Around line 32-33: Update docs/INTEGRATIONS.md lines 32-33 to state that
callers must pass panel_normalizer into consensus evaluation before configuring
allowed_choices={"approve", "hold", "reject"}, ensuring aliases normalize before
unknown-value validation. Update SECURITY.md lines 33-34 to replace the claim
that adapters cannot introduce unreviewed choices with wording that an
unreviewed normalized choice cannot pass.
- Around line 41-43: Update the adapter recipes around authorization_review and
ethics_review to define and pass the per-request request/context values
explicitly. Prefer showing a factory or per-run binding that supplies fresh
inputs to each callback, or clearly mark the snippets as illustrative pseudocode
so they do not imply undefined or stale closure state.
In `@docs/PRODUCTIZATION.md`:
- Around line 343-345: Update the CI verification record near the
implementation-head reference to replace the abbreviated hash 4ff499b with the
complete commit SHA, preserving the surrounding job results and
publication-scope details.
In `@tests/test_policy.py`:
- Around line 212-216: Update test_policy_digest_has_a_stable_schema_v1_contract
to document that a digest mismatch must preserve the existing golden value and
treat _content_dict changes as a schema or compatibility migration requiring
deliberate handling of stored verdicts, rather than replacing the expected hash.
- Around line 219-239: Type the test_policy parameter as a zero-argument
Callable returning DecisionPolicy, import Callable from collections.abc, and
remove the type: ignore from policy(). Keep the existing parametrized factories
and invalid-policy assertions unchanged.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d9c4d74d-c8ce-4ddb-b521-efcd22bc8793
📒 Files selected for processing (24)
.github/workflows/ci.ymlCHANGELOG.mdREADME.mdROADMAP.mdSECURITY.mdagent_consensus/__init__.pyagent_consensus/core.pyagent_consensus/models.pyagent_consensus/policy.pydocs/API_REFERENCE.mddocs/CONSENSUS_ALGORITHMS.mddocs/DECISION_GATES.mddocs/GETTING_STARTED.mddocs/INTEGRATIONS.mddocs/PRODUCTIZATION.mddocs/RELEASING.mdexamples/06_release_gate.pyexamples/07_policy_panel.pypyproject.tomlrequirements-dev.txttests/test_consensus.pytests/test_engine.pytests/test_package.pytests/test_policy.py
Comment on lines
+281
to
+293
| def _validate_decision_settings(threshold: float, min_successful: int) -> None: | ||
| """Validate the decision settings shared by both public entry points.""" | ||
| if ( | ||
| isinstance(threshold, bool) | ||
| or not isinstance(threshold, (int, float)) | ||
| or not math.isfinite(threshold) | ||
| or not 0 < threshold <= 1 | ||
| ): | ||
| raise ConfigurationError("threshold must be greater than 0 and at most 1") | ||
| if isinstance(min_successful, bool) or not isinstance(min_successful, int): | ||
| raise ConfigurationError("min_successful must be an integer") | ||
| if min_successful < 1: | ||
| raise ConfigurationError("min_successful must be at least 1") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Report the caller's parameter name in the quorum error message.
evaluate_votes exposes the quorum parameter as min_votes (agent_consensus/core.py Line 169). This validator always raises "min_successful must be an integer". A caller who passes min_votes=0 receives a message that names a parameter absent from the evaluate_votes signature.
Pass the caller-facing label into the validator.
🔧 Proposed fix
-def _validate_decision_settings(threshold: float, min_successful: int) -> None:
+def _validate_decision_settings(
+ threshold: float,
+ min_successful: int,
+ *,
+ quorum_field: str = "min_successful",
+) -> None:
"""Validate the decision settings shared by both public entry points."""
if (
isinstance(threshold, bool)
or not isinstance(threshold, (int, float))
or not math.isfinite(threshold)
or not 0 < threshold <= 1
):
raise ConfigurationError("threshold must be greater than 0 and at most 1")
if isinstance(min_successful, bool) or not isinstance(min_successful, int):
- raise ConfigurationError("min_successful must be an integer")
+ raise ConfigurationError(f"{quorum_field} must be an integer")
if min_successful < 1:
- raise ConfigurationError("min_successful must be at least 1")
+ raise ConfigurationError(f"{quorum_field} must be at least 1")Then update the call in agent_consensus/core.py:
_validate_decision_settings(threshold, min_votes, quorum_field="min_votes")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _validate_decision_settings(threshold: float, min_successful: int) -> None: | |
| """Validate the decision settings shared by both public entry points.""" | |
| if ( | |
| isinstance(threshold, bool) | |
| or not isinstance(threshold, (int, float)) | |
| or not math.isfinite(threshold) | |
| or not 0 < threshold <= 1 | |
| ): | |
| raise ConfigurationError("threshold must be greater than 0 and at most 1") | |
| if isinstance(min_successful, bool) or not isinstance(min_successful, int): | |
| raise ConfigurationError("min_successful must be an integer") | |
| if min_successful < 1: | |
| raise ConfigurationError("min_successful must be at least 1") | |
| def _validate_decision_settings( | |
| threshold: float, | |
| min_successful: int, | |
| *, | |
| quorum_field: str = "min_successful", | |
| ) -> None: | |
| """Validate the decision settings shared by both public entry points.""" | |
| if ( | |
| isinstance(threshold, bool) | |
| or not isinstance(threshold, (int, float)) | |
| or not math.isfinite(threshold) | |
| or not 0 < threshold <= 1 | |
| ): | |
| raise ConfigurationError("threshold must be greater than 0 and at most 1") | |
| if isinstance(min_successful, bool) or not isinstance(min_successful, int): | |
| raise ConfigurationError(f"{quorum_field} must be an integer") | |
| if min_successful < 1: | |
| raise ConfigurationError(f"{quorum_field} must be at least 1") |
🤖 Prompt for 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.
In `@agent_consensus/models.py` around lines 281 - 293, Update
_validate_decision_settings to accept a caller-facing quorum_field label and use
it in both min_successful validation error messages. Pass
quorum_field="min_votes" from evaluate_votes while preserving the existing label
for the other public entry point.
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
passed,blocked, orindeterminateverdicts with stable reason codes, the applied policy snapshot, and complete consensus evidenceWhy
The core tally engine was credible but stopped at “what did the group agree on?” Real release, output-safety, authorization, and routing workflows also need a deterministic answer to “may the application act on this evidence?” Keeping policy evaluation separate preserves the small provider-neutral core while giving the package concrete operational use cases.
Blocking evidence takes precedence: a configured veto or agreed non-pass choice is
blocked. Missing reviewers, insufficient successful weight, unknown choices, quorum failure, or no consensus isindeterminate. Only a fully satisfied policy ispassed.Read-only inspection of optional Samsarix producer contracts informed the policy-panel recipe. The package imports none of them, so this repository remains independently installable and useful.
Developer impact
New public API:
DecisionPolicyDecisionStatusDecisionReasonDecisionVerdictevaluate_decision()DecisionPolicy.digestcovers normalized schema-v1 policy content using canonical UTF-8 JSON and SHA-256. A fixed contract vector protects the encoding from drift. The digest is not a signature.There are still no runtime dependencies, network calls, persistence, provider SDKs, sibling-repository imports, or protected-action side effects. Hosts remain responsible for authenticated identity, authorization, redaction, policy distribution, and enforcement.
Exact-head validation
Head:
37c10fa54afa7261f00ef8eefd6e2d595490a6ecFresh Windows Python 3.14.6 environment installed solely from
requirements-dev.txt:9.0.3confirmedpython -m ruff format --check .— passed; 18 files already formattedpython -m ruff check .— passedpython -m mypy agent_consensus— passed; 6 source filespython -m pytest— 79 passed; 99.62% branch-aware coveragepython -m build --outdir <fresh-temp>/artifacts— sdist built, then wheel built from the sdistpython -m twine check <fresh-temp>/artifacts/*— wheel and sdist passedpip install --no-deps <wheel>— passed; imported version0.2.0Final local artifact snapshot:
3A28A87AF84F0889D2FC925E07183A56A1B6FA4DDCF6CAB3BF568EA7E1CDCFBD8213BB92E36F7BD9BF7B1B66CFB7CCF37EF2619FCBE4086B6A61C79732F7477BHosted CI for the final head passed twice across quality, Linux Python 3.10–3.14, Windows Python 3.14, package build/check, isolated wheel install, and all examples.
Security review
daf0dc5..310372a): 5/5 changed source-like rows closed, 0 findings, 0 deferred work.310372a..37c10fa): 1/1 source-like row closed, 0 findings, 0 deferred work.Review and release gates