Skip to content

Unify DefaultPolicyEngine decision and explanation rule chain - #286

Merged
dgenio merged 25 commits into
mainfrom
agent/unify-policy-rule-chain-219
Aug 14, 2026
Merged

Unify DefaultPolicyEngine decision and explanation rule chain#286
dgenio merged 25 commits into
mainfrom
agent/unify-policy-rule-chain-219

Conversation

@dgenio

@dgenio dgenio commented Aug 14, 2026

Copy link
Copy Markdown
Owner

What changed

  • replace the duplicated DefaultPolicyEngine.evaluate() / .explain() condition trees with one ordered _DEFAULT_RULES chain
  • keep decision semantics short-circuiting while explanation traverses the same rules in collect-all mode
  • add RateLimiter.peek() so explanation can predict rate-limit denials without creating, pruning, or consuming limiter state
  • preserve denial reason codes and decision trace semantics while returning constraints from the shared traversal
  • split rule helpers by concern so every new module remains below the repository's 300-line module ceiling
  • update architecture and agent guidance so future rules are registered once in the shared chain
  • add agreement regressions across safety, sensitivity, memory, constraint, and rate-limit cases, plus strict no-mutation tests for explain()

Verification

Authoritative make ci passed on the final architecture:

  • Ruff format: clean
  • Ruff lint: clean
  • mypy: 70 source files, no issues
  • pytest: 835 passed, 1 skipped
  • branch coverage: 94.23% (90% required)
  • all repository examples completed successfully

The new policy modules are all below the mandatory 300-line ceiling. policy.py itself is reduced substantially by removing the two hand-maintained rule copies.

Closes #219.

Copilot AI lite review requested due to automatic review settings August 14, 2026 22:05
Comment thread src/weaver_kernel/policy.py Fixed
Comment thread src/weaver_kernel/policy.py Fixed
Comment thread src/weaver_kernel/policy.py Fixed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors DefaultPolicyEngine so both authorization decisions (evaluate()) and denial explanations (explain()) are driven by a single canonical ordered rule chain, eliminating the prior duplicated condition trees. It also adds a read-only rate-limit path (RateLimiter.peek()) so explanations can predict rate-limit denials without mutating limiter state, and adds regression tests to enforce evaluate/explain agreement and strict no-mutation behavior.

Changes:

  • Introduce a shared _DEFAULT_RULES rule chain (DefaultPolicyRuleChain) and route DefaultPolicyEngine.evaluate() / .explain() through it (short-circuit vs collect-all).
  • Add RateLimiter.peek() and use it for read-only rate-limit evaluation during explanation.
  • Add targeted tests for decision/explanation agreement and read-only invariants; update docs and contributor guidance accordingly.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_policy_rule_chain.py Adds agreement + read-only invariant tests to prevent drift and limiter mutation in explain().
src/weaver_kernel/rate_limit.py Adds RateLimiter.peek() for non-mutating rate-limit prediction.
src/weaver_kernel/policy.py Replaces duplicated evaluate/explain logic with shared chain traversal; preserves trace + reason-code behavior.
src/weaver_kernel/default_policy_rules.py Defines canonical ordered _DEFAULT_RULES and DefaultPolicyRuleChain.run() traversal.
src/weaver_kernel/default_policy_rule_types.py Adds shared context/result/failure datatypes and shared constants for the rule chain.
src/weaver_kernel/default_policy_limit_rules.py Implements row-cap and rate-limit rules, including read-only limiter behavior.
src/weaver_kernel/default_policy_access_rules.py Implements safety/sensitivity/secrets/memory access rules as chain components.
docs/architecture.md Documents the unified chain and read-only rate-limit explanation behavior.
CHANGELOG.md Notes the behavior-preserving refactor and new agreement/no-mutation coverage.
AGENTS.md Updates “Adding a policy rule” guidance to register rules once in _DEFAULT_RULES + add agreement coverage.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@dgenio
dgenio merged commit 7c4443e into main Aug 14, 2026
11 checks passed
dgenio pushed a commit that referenced this pull request Aug 17, 2026
Red-team follow-up (PR #259 review by dgenio):

- enforce_arg_constraints now validates inner rule values, not just the outer
  containers: a non-string `prefix` value (e.g. {"prefix":{"path":123}}) and a
  non-string / unhashable `allowed_keys` element (e.g. a nested list) previously
  reached value.startswith()/set(allowed) and raised an untyped TypeError that
  escaped the audited denial path. Both now fail closed with
  TokenScopeError(arg_constraint_violation). Regression cases added.

Rebase integration onto current main (was 6 commits behind):
- Reconciled with the #286 shared policy rule-chain refactor: DefaultPolicyEngine
  keeps max_ttl_s (#203) on top of the new DefaultPolicyRuleChain structure.
- Merged #284 (actionable secret warning) with the #185 rotation keyring in
  _secrets.py; merged docs/security.md and CHANGELOG.
- coding_agent_demo.py (#253) now imports HMACTokenProvider from _hmac_provider,
  since tokens.py no longer re-exports it (cycle fix).
- CHANGELOG lifecycle entries moved under [Unreleased]; dropped the stale
  "removed rate-limit aliases" note (main keeps them) and the stale re-export
  claim.

make ci green: 912 passed, 1 skipped.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019WRxQL8t2Uusa6845jWtVV
dgenio added a commit that referenced this pull request Aug 19, 2026
* feat: harden the capability-token lifecycle — rotation, TTL, invoke-time enforcement

Groups six issues that share one implementation path (a CapabilityToken from
issuance through invoke-time enforcement) and one code area.

- #185 Signing-key rotation: HMACTokenProvider(secrets={key_id: secret},
  active_key_id=...) verifies previous-key tokens during an overlap window;
  key_id is signed into the payload; unknown key id fails closed. New
  WEAVER_KERNEL_SECRETS / WEAVER_KERNEL_ACTIVE_KEY env resolution.
- #200 CapabilityToken.from_dict raises typed TokenInvalid on malformed input
  instead of leaking KeyError/ValueError.
- #203 Per-grant TTL: grant_capability(ttl_s=...) with DefaultPolicyEngine(
  max_ttl_s=...); non-positive or over-max is denied (not clamped), audited.
- #183 Signed argument constraints (constraints["args"]: allowed_keys/pinned/
  prefix) enforced at invoke() and invoke_stream() before the driver runs,
  raising TokenScopeError with a failure trace; dry-run predicts the same.
- #170 Opt-in per-invocation rate limiting (Kernel(invoke_rate_limits=...)),
  default off, race-free check-then-record; dry-run never consumes.
- #224 ADR docs/adr/0001-token-signing-evolution.md (HMAC vs macaroon vs
  Biscuit, measured); recommends HMAC + re-issuance now. No code/deps change.

To stay within the AGENTS.md 300-line module budget, HMACTokenProvider moved to
_hmac_provider.py (re-exported; logger name unchanged) and helpers were split
into _token_signing.py, policy_ttl.py, kernel/_grant.py, kernel/_constraints.py.
Removed the private policy._DEFAULT_RATE_LIMITS/_SERVICE_RATE_MULTIPLIER aliases
(part of #196). Breaking: the signed payload now includes key_id, so
pre-upgrade tokens no longer verify (accepted pre-1.0; legacy config unchanged).

make ci: fmt-check, lint, mypy --strict (67 files), 856 passed / 1 skipped,
94% branch coverage, all 13 examples.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019WRxQL8t2Uusa6845jWtVV

* fix: break token-module import cycle (CodeQL) and harden fail-closed inputs

Addresses PR #259 review:

- CodeQL cyclic-import errors: the tokens <-> _hmac_provider re-export and the
  tokens <-> _token_signing TYPE_CHECKING import formed cycles. Made the graph
  acyclic — _token_signing no longer references tokens (build_signable_payload
  inlined into CapabilityToken._signable_payload), and tokens no longer
  re-exports HMACTokenProvider. Importers (__init__, kernel, cli/_doctor) now
  import HMACTokenProvider from _hmac_provider; the public
  `from weaver_kernel import HMACTokenProvider` is unchanged.
- Copilot: from_dict now coerces naive ISO timestamps to UTC, so a malformed
  token can't cause a naive-vs-aware TypeError at verify() time.
- Copilot: DefaultPolicyEngine(max_ttl_s=...) now rejects non-SafetyClass dict
  keys at construction (fail closed instead of silently ignoring the cap).
- Copilot: enforce_arg_constraints fails closed (TokenScopeError,
  arg_constraint_violation) on malformed args.allowed_keys/pinned/prefix types
  instead of raising an untyped TypeError or silently ignoring them.

Tests added for each. make ci green: 861 passed, 1 skipped, 94% branch coverage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019WRxQL8t2Uusa6845jWtVV

* fix: close remaining arg-constraint type gaps; rebase onto main

Red-team follow-up (PR #259 review by dgenio):

- enforce_arg_constraints now validates inner rule values, not just the outer
  containers: a non-string `prefix` value (e.g. {"prefix":{"path":123}}) and a
  non-string / unhashable `allowed_keys` element (e.g. a nested list) previously
  reached value.startswith()/set(allowed) and raised an untyped TypeError that
  escaped the audited denial path. Both now fail closed with
  TokenScopeError(arg_constraint_violation). Regression cases added.

Rebase integration onto current main (was 6 commits behind):
- Reconciled with the #286 shared policy rule-chain refactor: DefaultPolicyEngine
  keeps max_ttl_s (#203) on top of the new DefaultPolicyRuleChain structure.
- Merged #284 (actionable secret warning) with the #185 rotation keyring in
  _secrets.py; merged docs/security.md and CHANGELOG.
- coding_agent_demo.py (#253) now imports HMACTokenProvider from _hmac_provider,
  since tokens.py no longer re-exports it (cycle fix).
- CHANGELOG lifecycle entries moved under [Unreleased]; dropped the stale
  "removed rate-limit aliases" note (main keeps them) and the stale re-export
  claim.

make ci green: 912 passed, 1 skipped.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019WRxQL8t2Uusa6845jWtVV

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unify the duplicated rule chains in DefaultPolicyEngine.evaluate() and .explain()

3 participants