WS-XINT-003-02D: publish REV authorization integration contracts - #257
Conversation
|
Warning Review limit reached
Next review available in: 33 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughPR ChangesREV authorization contracts
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
backend/app/modules/authorization/review_contracts.py (1)
221-229: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse one closed enum for every
execution_mode.Four models declare
execution_modeas a bare string literal ("due_lease"here,"due_preference"at Line 265,"artifact_reference"at Line 437,"projection_rebuild"at Line 451). The reconcile pair uses theReconciliationModeenum instead. Doc line 96 describes one closed server-derivedexecution_mode, so REV consumers benefit from a single importable enum.♻️ Sketch of the consolidation
+class ServiceExecutionMode(StrEnum): + """Closed server-derived execution modes for fixed-service contracts.""" + + AUTHORITY_INVALIDATION = "authority_invalidation" + GENERAL = "general" + DUE_LEASE = "due_lease" + DUE_PREFERENCE = "due_preference" + ARTIFACT_REFERENCE = "artifact_reference" + PROJECTION_REBUILD = "projection_rebuild"- execution_mode: Literal["due_lease"] + execution_mode: Literal[ServiceExecutionMode.DUE_LEASE]🤖 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 `@backend/app/modules/authorization/review_contracts.py` around lines 221 - 229, Define or reuse one closed enum for all server-derived execution modes, then update ReviewLeaseExpiryContract.execution_mode and the corresponding declarations in the due-preference, artifact-reference, and projection-rebuild contract models to use that enum instead of bare string literals. Preserve the existing mode values and keep the reconciliation contracts aligned with the same importable enum.backend/tests/test_review_authorization_contracts.py (1)
182-198: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert on field types, not field names.
Two inertness proofs are weaker than the acceptance criteria at doc lines 140-142.
forbidden_namescompares exact field names. A field namedevidence_bytes,raw_content, orcallback_urlpasses. The test name also promises an unbounded-map check, but no annotation is inspected.- Line 514 asserts
"PreparedAuthorizationHandle" not in contract.model_dump_json(). A class name never appears as a serialized value, so that assertion always passes.Check the allowed annotation set across every manifest model instead.
♻️ Proposed type-based check
for spec in REVIEW_AUTHORIZATION_CONTRACT_BY_ACTION.values(): for model in spec.resource_models: assert forbidden_names.isdisjoint(model.model_fields) - assert all( - field.annotation is not PreparedAuthorizationHandle - for field in model.model_fields.values() - ) + for name, field in model.model_fields.items(): + members = { + arg for arg in get_args(field.annotation) or (field.annotation,) + } + assert PreparedAuthorizationHandle not in members, name + assert not any( + isinstance(member, type) + and issubclass(member, (bytes, bytearray, dict, list, set)) + for member in members + ), name + assert not any(callable(member) and not isinstance(member, type) for member in members), name🤖 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 `@backend/tests/test_review_authorization_contracts.py` around lines 182 - 198, Update test_contract_models_exclude_handles_callbacks_bytes_and_unbounded_maps to validate annotations rather than field names: inspect every field in every resource model from REVIEW_AUTHORIZATION_CONTRACT_BY_ACTION and assert each uses the allowed annotation set, excluding PreparedAuthorizationHandle, bytes, callback-related types, and unbounded mappings. Replace the inert contract.model_dump_json() class-name assertion near the authorization contract checks with the same type-based validation.
🤖 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
@.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02D-auth-prep-integration-readiness.md:
- Around line 71-86: Split review.queue.inspect into a separate operator-queue
row, documenting REQUEST_READ and only its published facts: project/shard,
filter digest, bounded cursor, and redaction state; keep
review.queue.routing.override, review.queue.routing.correct, and
review.queue.close grouped with their PREPARED_OPERATOR facts. In the
review.revision_context.repair row, change the guide requirement from an
identity triple to the published guide_id and guide_activation_sequence fields,
unless the contract is intentionally extended with a guide digest.
In `@backend/app/modules/authorization/review_contracts.py`:
- Around line 144-154: Update the contract model’s no_self_review field to use
Literal[True], matching the module’s other server-proven markers. In
require_distinct_reviewer, validate the no_self_review proof and
reviewer/contributor identity as separate conditions with distinct error
messages, while preserving successful validation for valid distinct-reviewer
contexts.
---
Nitpick comments:
In `@backend/app/modules/authorization/review_contracts.py`:
- Around line 221-229: Define or reuse one closed enum for all server-derived
execution modes, then update ReviewLeaseExpiryContract.execution_mode and the
corresponding declarations in the due-preference, artifact-reference, and
projection-rebuild contract models to use that enum instead of bare string
literals. Preserve the existing mode values and keep the reconciliation
contracts aligned with the same importable enum.
In `@backend/tests/test_review_authorization_contracts.py`:
- Around line 182-198: Update
test_contract_models_exclude_handles_callbacks_bytes_and_unbounded_maps to
validate annotations rather than field names: inspect every field in every
resource model from REVIEW_AUTHORIZATION_CONTRACT_BY_ACTION and assert each uses
the allowed annotation set, excluding PreparedAuthorizationHandle, bytes,
callback-related types, and unbounded mappings. Replace the inert
contract.model_dump_json() class-name assertion near the authorization contract
checks with the same type-based validation.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8abd0a13-18e8-4faa-96da-6a0b1b2a8c65
📒 Files selected for processing (12)
.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/ACTION_CUSTODY.md.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/STATUS.md.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02D-auth-prep-integration-readiness.md.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02D-external-review-response.md.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02D-internal-review.md.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02D-pr-trust-bundle.mdbackend/app/modules/authorization/review_contracts.pybackend/scripts/run_test_lanes.pybackend/tests/test_review_authorization_contracts.pydocs/spec_authorization_service.mddocs/spec_review_lifecycle.md
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
2 participants
Intent
Publish the complete inert typed AUTH contract surface REV can implement against without per-chunk authorization discovery.
Scope
review.*actionsEvidence
Human review focus
Confirm the manifest is complete enough for REV composers, concealed
noneresults cannot leak lineage, revision decisions cannot bypass predecessor/response binding, and publishing these types creates no runtime authority.Full hosted Backend and Agent Gates are required on the exact head before merge readiness.
Summary by CodeRabbit
New Features
Documentation
Tests