Skip to content

feat(policy): support native PolicyPlugin exports from module files#141

Merged
digitarald merged 3 commits into
mainfrom
digitarald/agents/policyplugin-exports-review-update
May 10, 2026
Merged

feat(policy): support native PolicyPlugin exports from module files#141
digitarald merged 3 commits into
mainfrom
digitarald/agents/policyplugin-exports-review-update

Conversation

@digitarald
Copy link
Copy Markdown
Collaborator

Summary

Supersedes #115 — merges the original PR by @yxbh and adds review-driven improvements.

Allow policy modules to export a native PolicyPlugin object (with detectors, hooks, and recommenders) instead of being limited to the PolicyConfig criteria DSL.

Changes from #115 (original)

File Change
packages/core/src/services/policy/types.ts Added isNativePlugin() type guard, validateNativePlugin() with thorough hook/array/type checking
packages/core/src/services/policy/index.ts Re-exported new functions
packages/core/src/services/policy.ts loadPolicy() returns PolicyConfig | PolicyPlugin union, pathToFileURL fix for Windows
packages/core/src/services/policy/loader.ts loadPluginChain() detects and adds native plugins directly
packages/core/src/services/readiness/index.ts Auto-enables engine path when native plugins present (without mutating caller options)
Test files 30 new tests for type guards, validation, and loader integration

Review improvements (this PR)

  1. Hardened normalizeNativePlugin: Changed from conditional defaults (??) to unconditional forced values for sourceType: "module" and trust: "trusted-code". Previously a plugin could claim sourceType: "builtin" and normalizeNativePlugin would preserve it — now it's overwritten consistently with the loader's security enforcement.

  2. Added missing onError validation test: validateNativePlugin validates onError but had no test coverage for this path.

  3. Added isNativePlugin edge case test: Objects with both root-level name and meta (ambiguous shape) are correctly classified as PolicyConfig, not native plugins.

  4. Added direct loadPolicy() native plugin test: Verifies loadPolicy() returns a properly normalized PolicyPlugin when given a .mjs native export — previously only tested through loadPluginChain().

  5. Updated JSDoc to reflect the unconditional normalization behavior.

Why

The PolicyConfig DSL (criteria.add/disable/override) is great for simple checks but can't express:

  • Signal mutation via afterDetect hooks (SignalPatch)
  • Cross-signal recommendations (e.g. per-MCP-server denied signals)
  • Rich metadata on recommendations (reference URLs, impact levels)

Native PolicyPlugin exports unlock the full 5-stage engine pipeline for external policy authors.

Testing

Remaining items for follow-up

  • Readiness auto-shadow orchestration lacks dedicated tests (works via integration)
  • validateNativePlugin doesn't validate detector kind field (intentional — runtime flexibility)

yxbh and others added 3 commits April 8, 2026 11:52
Allow policy modules to export a native PolicyPlugin object (with detectors,
hooks, and recommenders) instead of being limited to the PolicyConfig DSL.

Changes:
- isNativePlugin() type guard with validation of meta fields
- validateNativePlugin() with thorough hook/array/type checking
- loadPolicy() returns PolicyConfig | PolicyPlugin union
- loadPluginChain() detects and adds native plugins directly
- readiness auto-enables engine path when native plugins are present
- pathToFileURL fix for Windows dynamic import

Tests: 671 passing (34 new tests for type guard, validation, and loader)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- normalizeNativePlugin now unconditionally forces sourceType/trust
  instead of conditional defaults, preventing plugins from claiming
  misleading sourceType (e.g. 'builtin') when loaded from modules
- Add missing onError validation test
- Add isNativePlugin test for ambiguous object with both name and meta
- Add direct loadPolicy() test for native plugin return path
- Update loadPolicy JSDoc to reflect normalization behavior

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 10, 2026 15:58
@digitarald digitarald requested a review from pierceboggan as a code owner May 10, 2026 15:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds support for policy module files (and npm specifiers) to export a native PolicyPlugin object directly, enabling full engine pipeline behavior (detectors/hooks/recommenders) beyond the PolicyConfig DSL.

Changes:

  • Extend policy loading to detect/validate native PolicyPlugin exports and normalize meta.sourceType / meta.trust.
  • Update loader/readiness flow to incorporate native plugins into the engine path and expose new type-guard/validation utilities.
  • Add/expand tests covering native plugin detection, validation, normalization, and loader/engine integration.
Show a summary per file
File Description
src/services/__tests__/policy.test.ts Adds helper to assert PolicyConfig loads, plus a direct native-plugin .mjs load test.
src/services/__tests__/policy-loader.test.ts Adds integration tests for native plugin loading/normalization and engine execution.
src/services/__tests__/policy-engine-types.test.ts Adds unit tests for isNativePlugin and validateNativePlugin edge cases.
packages/core/src/services/readiness/index.ts Skips native plugins in DSL resolution and auto-enables engine path when native plugins are present.
packages/core/src/services/policy/types.ts Introduces isNativePlugin and validateNativePlugin for detection and load-time validation.
packages/core/src/services/policy/loader.ts Updates plugin chain loader to accept native plugin exports directly.
packages/core/src/services/policy/index.ts Re-exports isNativePlugin / validateNativePlugin from the policy public API.
packages/core/src/services/policy.ts Updates loadPolicy() to return `PolicyConfig

Copilot's findings

  • Files reviewed: 8/8 changed files
  • Comments generated: 2

Comment on lines +273 to +276
obj.afterRecommend;
if (!hasHooks) {
throw new Error(
`Native plugin "${source}" is invalid: must implement at least one hook (detectors, afterDetect, beforeRecommend, recommenders, or afterRecommend)`
Comment on lines 211 to 214
if (jsonOnly) {
throw new Error(
`Policy "${source}" rejected: only JSON policies are allowed from agentrc.config.json. Module policies (.ts/.js) must be passed via --policy.`
);
@digitarald digitarald merged commit aa6ea2f into main May 10, 2026
21 checks passed
@digitarald digitarald deleted the digitarald/agents/policyplugin-exports-review-update branch May 10, 2026 16:04
github-actions Bot added a commit that referenced this pull request May 11, 2026
- Extract resolveNativePlugin() helper in policy.ts to deduplicate
  the repeated validateNativePlugin + normalizeNativePlugin call pair
- Remove redundant meta spread in loadPluginChain (loadPolicy already
  normalizes sourceType/trust via normalizeNativePlugin before returning)
- Replace options parameter reassignment in runReadinessReport with a
  local shadow boolean variable for clearer intent

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions Bot added a commit that referenced this pull request May 11, 2026
Add documentation for the native PolicyPlugin export feature introduced
in #141, which allows .ts/.js/.mjs module files and npm packages to
export a full PolicyPlugin object instead of being limited to the
PolicyConfig DSL.

- docs/dev/plugins.md: Add native plugin authoring section with a
  complete TypeScript example showing detectors, hooks, and recommenders.
  Update authoring API list from two to three options.
- docs/concepts.md: Mention native plugin support in the Policies
  overview and add a TypeScript policy example to the code block.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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.

3 participants