feat(core): add authority policy obstruction vocabulary#335
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR extends the capability grant intent boundary by introducing an obstruction-only ChangesCapability Grant Intent Obstruction Expansion
🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
🔍 Static Audit Results | Filepath | Line(s) | Severity | Classification | Description |
| :--- | :--- | :--- | :--- | :--- |
| `crates/warp-core/src/optic_artifact.rs` | `L435-449` | Major | Logic Error / Admission Semantics | Empty `AuthorityPolicy.policy_id` is not rejected before policy evaluation, so an incomplete policy context can still classify as `InvalidDelegation` or `ScopeEscalation` instead of unsupported policy. |
| `crates/warp-core/tests/capability_grant_intent_tests.rs` | `L216-253` | Minor | Test Coverage Drift | The broad `never_makes_grant_authority` invariant does not exercise the new `InvalidDelegation`, `ScopeEscalation`, or `ReplayOrDuplicateIntent` obstruction surfaces added by this branch. |
| `docs/design/optic-capability-grant-registry.md` | `L4-7` | Nit | Naming / Documentation Drift | The file path still says `registry` while the document title and current doctrine describe an intent boundary, preserving stale CRUD vocabulary in a design-map location. |Cc: @codex — please review the evidence bundles above and confirm validation alignment. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5205e750c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if authority_context.policy.is_none() { | ||
| return CapabilityGrantIntentObstruction::UnsupportedAuthorityPolicy; |
There was a problem hiding this comment.
In classify_capability_grant_intent, policy presence is validated only with authority_context.policy.is_none(), so Some(AuthorityPolicy { policy_id: "" }) falls through to policy_evaluation and can be classified as InvalidDelegation or ScopeEscalation instead of an unsupported/malformed policy context. This changes admission semantics (the intent ID is then recorded for replay blocking via records_submitted_intent) and produces misleading obstruction outcomes for incomplete authority contexts; validate non-empty policy_id before entering the evaluation match.
Useful? React with 👍 / 👎.
|
🔧 Static Audit Resolution | Issue | Severity | Addressed by | Regression coverage | Outcome |
| :--- | :--- | :--- | :--- | :--- |
| Empty `AuthorityPolicy.policy_id` could over-classify as `InvalidDelegation` / `ScopeEscalation` instead of unsupported policy shape. | Major | d70ef0b | `capability_grant_intent_obstructs_missing_policy_identity_as_unsupported_policy`; `cargo test -p warp-core capability_grant_intent` | Missing policy identity now obstructs as `UnsupportedAuthorityPolicy` before policy evaluation posture is considered. |
| Broad never-authority invariant omitted new obstruction surfaces. | Minor | d70ef0b | `capability_grant_intent_never_makes_grant_authority`; `cargo test -p warp-core capability_grant_intent` | Invariant now covers malformed, missing issuer, invalid delegation, scope escalation, replay/duplicate, and unsupported policy. |
| Design doc path preserved stale registry vocabulary. | Nit | d70ef0b | `test ! -e docs/design/optic-capability-grant-registry.md && test -e docs/design/optic-capability-grant-intent-boundary.md`; `npx markdownlint-cli2 docs/design/optic-capability-grant-intent-boundary.md CHANGELOG.md` | Doc renamed to `docs/design/optic-capability-grant-intent-boundary.md`; no stale path references remain. |Validation also passed: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/design/optic-capability-grant-intent-boundary.md (2)
250-253: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
AuthorityContextdocumentation is missingpolicy_evaluation.Line 250-253 (ER model) and Line 308-309 (narrative) still describe
AuthorityContextas issuer + policy only, but this spec now introducespolicy_evaluation(Line 165) and uses it in obstruction classification. Please add it to keep the model deterministic and unambiguous across sections.As per coding guidelines, "Documentation accuracy matters — especially anything touching determinism guarantees, hash stability, or canonical ordering. Flag factual errors and stale cross-references."
Also applies to: 308-309
🤖 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 `@docs/design/optic-capability-grant-intent-boundary.md` around lines 250 - 253, The AuthorityContext ER model and narrative are missing the new policy_evaluation field; update the AuthorityContext definition and accompanying narrative to include policy_evaluation (the same structure introduced earlier) so all references (e.g., AuthorityContext, policy_evaluation, obstruction classification) are consistent and deterministic across the document; ensure the ER block adds policy_evaluation with its type and the explanatory paragraph mentions how policy_evaluation is used for evaluation/hash stability and ties into obstruction classification semantics.
318-318:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse replay/duplicate terminology consistently.
Line 318 says
classifies duplicate grant intents, but the updated obstruction vocabulary isReplayOrDuplicateIntent. This is stale wording and narrows the documented behavior.As per coding guidelines, "Documentation accuracy matters — especially anything touching determinism guarantees, hash stability, or canonical ordering. Flag factual errors and stale cross-references."
🤖 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 `@docs/design/optic-capability-grant-intent-boundary.md` at line 318, The phrase "classifies duplicate grant intents" is stale — update the wording to use the new obstruction vocabulary (e.g., "ReplayOrDuplicateIntent" or "replay/duplicate intent") and broaden the description so it no longer narrows behavior; specifically, replace the phrase with language that references ReplayOrDuplicateIntent and clarifies it covers both replayed and duplicate intents rather than only "duplicate grant intents" (search for the exact phrase and edit the surrounding sentence to match the current terminology and scope).
🤖 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.
Outside diff comments:
In `@docs/design/optic-capability-grant-intent-boundary.md`:
- Around line 250-253: The AuthorityContext ER model and narrative are missing
the new policy_evaluation field; update the AuthorityContext definition and
accompanying narrative to include policy_evaluation (the same structure
introduced earlier) so all references (e.g., AuthorityContext,
policy_evaluation, obstruction classification) are consistent and deterministic
across the document; ensure the ER block adds policy_evaluation with its type
and the explanatory paragraph mentions how policy_evaluation is used for
evaluation/hash stability and ties into obstruction classification semantics.
- Line 318: The phrase "classifies duplicate grant intents" is stale — update
the wording to use the new obstruction vocabulary (e.g.,
"ReplayOrDuplicateIntent" or "replay/duplicate intent") and broaden the
description so it no longer narrows behavior; specifically, replace the phrase
with language that references ReplayOrDuplicateIntent and clarifies it covers
both replayed and duplicate intents rather than only "duplicate grant intents"
(search for the exact phrase and edit the surrounding sentence to match the
current terminology and scope).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ec580a6d-406f-4e66-8128-72a2d022f23c
📒 Files selected for processing (5)
CHANGELOG.mdcrates/warp-core/src/lib.rscrates/warp-core/src/optic_artifact.rscrates/warp-core/tests/capability_grant_intent_tests.rsdocs/design/optic-capability-grant-intent-boundary.md
|
🔧 CodeRabbit docs feedback resolved | Issue | Severity | Addressed by | Regression coverage | Outcome |
| :--- | :--- | :--- | :--- | :--- |
| `AuthorityContext` ER/narrative omitted `policy_evaluation`. | Major | e3ac10d | `sed -n '250,254p' docs/design/optic-capability-grant-intent-boundary.md \| rg -n "policy_evaluation"`; `npx markdownlint-cli2 docs/design/optic-capability-grant-intent-boundary.md` | ER model and narrative now include `policy_evaluation` as obstruction-classification posture, not trusted governance. |
| Stale “duplicate grant intents” wording did not match `ReplayOrDuplicateIntent`. | Minor | e3ac10d | `rg -n "classifies duplicate grant intents" docs/design/optic-capability-grant-intent-boundary.md` returns no matches; `git diff --check` | Wording now uses replay/duplicate terminology and names `ReplayOrDuplicateIntent`. | |
1 participant
Summary
Adds the next Echo authority-ladder slice: obstruction vocabulary for authority policy evaluation.
This PR keeps the grant-intent path obstruction-only while naming the policy failure surfaces Echo must eventually witness:
InvalidDelegationScopeEscalationReplayOrDuplicateIntentUnsupportedAuthorityPolicyThe core doctrine remains:
What this proves
What this is not
AdmissionTicketemissionLawWitnessemissionVerification
RED:
GREEN / validation:
Push gate also passed: fmt, guards, clippy-core, tests-warp-core, rustdoc.
Summary by CodeRabbit
New Features
Documentation
Tests