Repository navigation
id: residency policy vocabulary and evaluator (#3398 step 1) - #5629
Conversation
![developing-today[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 40 minutes. View limit details
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. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 448019bbfc
ℹ️ 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".
| #[derive(Clone, Debug, Default, PartialEq, Eq)] | ||
| pub struct Policy { | ||
| pub planes: BTreeMap<Plane, PlanePolicy>, | ||
| /// Planes a path must prove. Defaults to all four through [`Policy::new`]. | ||
| pub required: BTreeSet<Plane>, |
There was a problem hiding this comment.
Constructing Scope::Scoped(Policy::default()) and evaluating an empty hop list returns Decision::Allow: the derived default leaves required empty, so neither evaluation loop records a denial, even though no planes, trust roots, or evidence are configured. This differs from Policy::new and makes the default construction of a scoped policy bypass the intended missing-evidence denial. Implement Default with all four required planes, or remove the derived implementation so callers must explicitly construct a policy.
Useful? React with 👍 / 👎.
| #[test] | ||
| fn full_evidence_allows() { | ||
| assert!(run(&evs(), 50, &OK).is_allow()); |
There was a problem hiding this comment.
The new evaluator has only hand-written example tests, despite having explicit invariants concerning required-plane coverage, evidence binding, expiry, and rejection of invalid hops. Add property-based coverage against these invariants, or generated comparisons with an independent decision model, varying required planes, hop multiplicity, bindings, and time boundaries. Root AGENTS.md requires property/model-based testing where meaningful invariants or a design model exist; fixed examples alone do not satisfy that requirement.
AGENTS.md reference: AGENTS.md:L41-L45
Useful? React with 👍 / 👎.
|
Update: the policy/evaluator is now in |
Partial implementation of #5529 (design
doc/2026-11-01_design_id3398.md§7 step 1). Adds pure, internalresidencymodule: planes, scope, per-plane policy, operator trust roots, evidence, fail-closed evaluator with reason codes, 14 unit tests. No network/store behaviour changes. Steps 2-5 (storage/processing admission, sync+bridge, federation/relay/metadata, spill) remain; step 5 is blocked on the single-writer lease design. Refs #5529, does not close it.Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.