24: first decision: trusted principal, memory facts, can, and decide - #40
Conversation
Refs #24 - Add SourceResolver interface for adapter-backed source registration - Add PrincipalEvaluator class with can() and decide() methods - Add Mizan.registerSource() and Mizan.forPrincipal() for source registration and principal binding - Implement collectFacts() with contract validation and error handling - Implement evaluate() with exact-match, denial-overrides-grant, deny-by-default logic - Implement useMemoryAdapter() to register the memory source on Mizan - Add end-to-end tests demonstrating the complete principal-to-memory-source-to-decision path
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe core package now supports registered source resolvers, principal-bound authorization decisions, outcome validation, and deny-overrides-grant evaluation. The memory package registers its adapter and adds integration coverage for grants, denials, roles, and default denial. ChangesAuthorization decision flow
Repository maintenance
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant PrincipalEvaluator
participant Mizan
participant MemoryAdapter
Caller->>PrincipalEvaluator: decide(permission)
PrincipalEvaluator->>Mizan: collect facts for principalId
Mizan->>MemoryAdapter: resolve(context)
MemoryAdapter-->>Mizan: facts outcome
Mizan-->>PrincipalEvaluator: aggregated facts
PrincipalEvaluator-->>Caller: allow or deny result
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@packages/core/__tests__/decision.test.ts`:
- Around line 163-164: Update the async assertions for auth.can and auth.decide
to await the promise-aware matchers, replacing synchronous not.toThrow checks
with awaited rejects.not.toThrow assertions so both promise results and
rejection behavior are exercised.
In `@packages/core/src/index.ts`:
- Around line 253-266: Update the resolver outcome validation before status
evaluation to reject null or non-object payloads, and validate every entry in
outcome.facts as an object with a string permission and supported effect. Use
the existing source name and contract-violation TypeError pattern for these
failures, while preserving valid status and fact processing in the surrounding
resolver flow.
- Around line 223-225: Update the register method to detect when the sources map
already contains the given name and reject the registration instead of
overwriting the existing resolver. Preserve the current insertion behavior for
unique names, and use an appropriate error or rejection consistent with the
surrounding API.
- Around line 265-267: Update the authorization outcome aggregation around the
outcome.status check so an "unavailable" source cannot be discarded: immediately
reject the decision or return the existing structured denial when any source is
unavailable, while preserving the current facts collection for available "facts"
outcomes.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 02f603da-3a45-43b4-a645-522739ff19a8
📒 Files selected for processing (6)
.gitignorepackages/core/__tests__/decision.test.tspackages/core/src/index.tspackages/memory/__tests__/decision.test.tspackages/memory/__tests__/smoke.test.tspackages/memory/src/index.ts
…me validation, unavailable = fail closed, async test assertions
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/core/__tests__/decision.test.ts`:
- Line 211: Await both async rejection assertions in
packages/core/__tests__/decision.test.ts at lines 211-211 and 223-223 by adding
await before expect(auth.can("x")).rejects.toThrow(...), matching the existing
fix at lines 163-164.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f5a07a9f-4ef0-45ca-8ddf-a39634f2cdac
📒 Files selected for processing (2)
packages/core/__tests__/decision.test.tspackages/core/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/index.ts
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.agents/wisdom/process.md:
- Around line 28-30: Add a language identifier to the fenced code block in the
issue template example, using text (or another appropriate language) while
preserving the example content.
In @.agents/wisdom/testing.md:
- Around line 19-25: Update the malformed-output test guidance in the relevant
testing specification so an empty facts array is treated as valid. Split
“missing/empty facts array” into separate cases: test missing or non-array facts
as malformed, and add a separate test confirming facts: [] is accepted by
SourceOutcome.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cf423387-a722-472f-a71d-55cd349b49c3
📒 Files selected for processing (3)
.agents/wisdom/process.md.agents/wisdom/security.md.agents/wisdom/testing.md
…mpty facts clarification
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Refs #24
Summary
Implements the first authorization decision path: a trusted principal can be evaluated against normalized facts supplied by the memory adapter.
Changes
packages/core
SourceResolverinterface for adapter-backed source registrationMizan.registerSource(name, resolver)for registering named sourcesMizan.forPrincipal(principalId)— returns aPrincipalEvaluatorbound to the principalPrincipalEvaluatorclass withcan()anddecide()collectFacts()with contract validationevaluate()— exact match, denial-overrides-grant, deny-by-defaultpackages/memory
useMemoryAdapter()now registers the adapter's source on MizanTests
Acceptance criteria
Summary by CodeRabbit
can()/decide()with structured allow/deny results and reasons..scratch/.