Skip to content

feat(mcp): expose compliance and retention controls#296

Open
illeatmyhat wants to merge 5 commits into
mainfrom
codex/mcp-compliance
Open

feat(mcp): expose compliance and retention controls#296
illeatmyhat wants to merge 5 commits into
mainfrom
codex/mcp-compliance

Conversation

@illeatmyhat

@illeatmyhat illeatmyhat commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • add structured MCP tools for memory inventory, detail, metadata patching, and explicit access recording
  • expose retention policy validation and dry-run/apply execution with entity-linked audit reports
  • report backend, retention, hook coverage, and configured protection-plugin health
  • keep retention and administrative inventory scans from refreshing last_accessed
  • document the compliance UI integration surface

Why

This supplies the Evolve-side API surface needed by the CUGA stakeholder compliance PoC and supports the eventing case discussed in #275. Evolve owns policy evaluation, hook enforcement, and referenced results; an external scheduler/eventing system can invoke runs and persist or deliver those results.

MCP tools

  • list_entities
  • get_entity
  • patch_entity_metadata
  • record_access
  • validate_retention_policy
  • run_retention
  • get_compliance_status

Validation

  • uv run --extra milvus pytest -q: 961 passed, 8 skipped, 279 deselected
  • focused MCP/retention/hooks tests: 89 passed
  • pre-commit: mypy, Ruff check/format, large-file check, detect-secrets passed
  • live SSE smoke from the CUGA adapter: create, list/detail with access stamp, metadata patch, policy validation, retention preview, and compliance status passed

Notes

  • User-scoped detail and mutation calls fail closed when ownership metadata is absent.
  • Retention remains a scheduled sweep, not an automatic expiry hook.
  • run_retention accepts as_of for deterministic stakeholder demonstrations and returns pre-action snapshots so applied deletions remain explainable.

Summary by CodeRabbit

  • New Features
    • Added entity inventory, retrieval, metadata update, and access-recording tools.
    • Added retention policy validation, dry-run/enforced execution, and compliance health reporting.
    • Added support for importing entities with an explicit creation timestamp.
    • Added a legal-hold plugin that prevents deletion of protected records.
  • Bug Fixes
    • Improved retention filtering and cascade-delete handling.
    • Access recording now reports successfully updated entity IDs.
  • Documentation
    • Added guidance for compliance tools, administrative scans, and the legal-hold plugin.

@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@illeatmyhat, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 13 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d39e1d60-fe3e-4960-82d2-ae4d857b853a

📥 Commits

Reviewing files that changed from the base of the PR and between 247b278 and af3e131.

📒 Files selected for processing (2)
  • altk_evolve/frontend/mcp/mcp_server.py
  • tests/unit/test_mcp_compliance_tools.py
📝 Walkthrough

Walkthrough

Adds MCP tools for entity administration, access recording, retention policy validation and execution, compliance status reporting, and legal-hold enforcement. The client, backend, and retention engine gain administrative scan and timestamp paths, with documentation, configuration, dependency, and unit-test updates.

Changes

Administrative MCP and retention controls

Layer / File(s) Summary
Entity administration and access tracking
altk_evolve/backend/base.py, altk_evolve/frontend/client/evolve_client.py, altk_evolve/frontend/mcp/mcp_server.py, tests/unit/test_mcp_compliance_tools.py, tests/unit/test_filesystem_backend.py, tests/unit/test_combine_guidelines.py
Adds hook-free scans, paginated entity inventory, ownership-enforced retrieval and metadata patching, access recording, and imported creation timestamps.
Retention validation and execution
altk_evolve/retention/engine.py, altk_evolve/frontend/mcp/mcp_server.py, tests/unit/test_retention.py, tests/unit/test_mcp_compliance_tools.py, docs/guides/retention.md
Adds policy normalization, filtered retention execution, deterministic reports, administrative scan selection, and cascade-delete precedence handling.
Compliance status reporting
altk_evolve/frontend/mcp/mcp_server.py, tests/unit/test_mcp_compliance_tools.py, examples/cuga_compliance_poc_hooks.yaml, altk_evolve/frontend/ui/package.json
Adds plugin configuration discovery, health and coverage reporting, retention capability checks, and supporting integration configuration.
Legal-hold deletion enforcement
altk_evolve/hooks/plugin.py, altk_evolve/hooks/manager.py, altk_evolve/hooks/plugins/legal_hold.py, examples/hooks_plugins.yaml, docs/guides/memory-hooks.md, tests/unit/test_hooks_seam.py
Adds a policy violation contract, native adapter handling, and a shipped plugin that blocks deletion when legal_hold metadata is true.

Estimated code review effort: 4 (Complex) | ~75 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MCPClient
  participant MCPServer
  participant RetentionEngine
  participant EvolveClient
  participant EntityBackend
  MCPClient->>MCPServer: run_retention(policy, filters)
  MCPServer->>RetentionEngine: apply normalized policy
  RetentionEngine->>EvolveClient: scan_entities(namespace_id, filters, limit)
  EvolveClient->>EntityBackend: administrative entity search
  EntityBackend-->>RetentionEngine: entity snapshot
  RetentionEngine-->>MCPServer: retention outcomes and report
  MCPServer-->>MCPClient: structured JSON response
Loading

Possibly related PRs

Suggested reviewers: visahak

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 31.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main MCP additions around compliance and retention controls.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/mcp-compliance

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 4

🧹 Nitpick comments (5)
tests/unit/test_mcp_compliance_tools.py (1)

98-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for the default record_access=True denial path.

Both parametrized cases use record_access=False, so the ordering issue flagged in altk_evolve/frontend/mcp/mcp_server.py (Lines 521-534) — the access-stamping read happening before the ownership check — is untested. A case asserting client.get_entity_by_id/client.record_access are not called for a non-owner would lock the fix in.

🤖 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 `@tests/unit/test_mcp_compliance_tools.py` around lines 98 - 111, Add a test
case in test_get_entity_enforces_attributed_owner that uses the default
record_access=True for a non-owner, then assert the denial response and verify
client.get_entity_by_id and client.record_access are not called. Keep the
existing parametrized record_access=False coverage unchanged.
altk_evolve/frontend/mcp/mcp_server.py (3)

687-701: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Snapshot scan duplicates the engine's full namespace scan.

run_retention scans up to FETCH_LIMIT entities for snapshots and RetentionEngine.evaluate scans the namespace again, doubling I/O and memory on large namespaces. Consider building snapshots lazily for only the ids in the report, or having the engine expose the entities it already fetched.

🤖 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 `@altk_evolve/frontend/mcp/mcp_server.py` around lines 687 - 701, Eliminate the
duplicate namespace scan in run_retention by reusing entities fetched by
RetentionEngine.apply/evaluate when constructing snapshots. Prefer exposing the
engine’s fetched entities or lazily resolving only report-referenced IDs, while
preserving retention report behavior and existing scan limits.

780-797: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

healthy and retention_available are structurally derived, not probed.

plugin["healthy"] is just manager is not None and enabled, and retention_available is a literal True, so a plugin that failed to load still reports healthy. Consider deriving per-plugin health from the manager's registered hooks (e.g. has_hooks_for) so the report can actually fail.

🤖 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 `@altk_evolve/frontend/mcp/mcp_server.py` around lines 780 - 797, Update the
health computation in the MCP health response so each enabled plugin’s health is
derived from its manager’s registered hooks using the existing hook-checking API
such as has_hooks_for, rather than only checking manager existence and enabled
state. Replace the literal retention_available value with the actual retention
capability or probe result, and ensure failed or unregistered plugins cause the
overall healthy result to be false.

583-594: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

One backend scan per id; consider a single scan for the batch.

record_access with a large entity_ids list issues N sequential scan_entities round-trips. A single scan (or an id-filtered batch fetch) then an in-memory ownership partition keeps this O(1) in backend calls. Also worth short-circuiting when allowed is empty.

🤖 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 `@altk_evolve/frontend/mcp/mcp_server.py` around lines 583 - 594, The entity
validation loop performs one scan_entities call per unique entity ID; replace it
with a single batch or ID-filtered scan, then partition the returned entities
into missing, denied, and allowed using _entity_owned_by while preserving
input-ID handling. In the record_access flow, skip the backend call when allowed
is empty while retaining the existing timestamp behavior for non-empty updates.
altk_evolve/frontend/client/evolve_client.py (1)

117-126: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider a named backend seam instead of reaching into _search_entities_impl.

This is the second caller of the private impl (see consolidate_guidelines, Line 273). A documented BaseEntityBackend.scan_entities (or search_entities_internal) would make the no-hook read path part of the backend contract instead of relying on a private method every backend must keep.

🤖 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 `@altk_evolve/frontend/client/evolve_client.py` around lines 117 - 126, The
scan_entities path should use a documented backend seam rather than directly
calling the private _search_entities_impl method. Add or expose a named method
such as BaseEntityBackend.scan_entities (or search_entities_internal), implement
it across backends as needed, and update both scan_entities and
consolidate_guidelines to call that contract while preserving the no-hook read
behavior.
🤖 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 `@altk_evolve/frontend/mcp/mcp_server.py`:
- Around line 744-747: Validate the parsed YAML root in the plugin-loading path
before calling loaded.get, ensuring it is a mapping and treating list or scalar
roots as malformed input. Raise the same handled validation/error type used by
RetentionPolicy.from_file so get_compliance_status returns its structured
healthy: False payload instead of leaking AttributeError.
- Around line 488-496: Update the pagination flow around record_access and
next_offset to capture the original page length before per-entity re-fetching,
then advance the cursor using that pre-filter length rather than len(page).
Preserve the transformed page for returned results and access recording while
ensuring vanished entities do not cause already-consumed rows to be re-served.
- Around line 521-534: Update the entity lookup flow around _resolve_namespace
and _entity_owned_by so ownership is always determined using the non-access scan
path before any access-stamping read occurs. After confirming the entity exists
and _entity_owned_by(entity, user_id) succeeds, perform record_access when
requested; preserve the existing not-found and permission-denied responses.

In `@altk_evolve/retention/engine.py`:
- Around line 213-214: The entity scan selection in the retention engine must
not eagerly access get_all_entities when scan_entities is available. Update the
scan assignment to lazily choose the preferred scan_entities method and only
resolve get_all_entities as the fallback, preserving the existing
scan(namespace_id, limit=limit) invocation.

---

Nitpick comments:
In `@altk_evolve/frontend/client/evolve_client.py`:
- Around line 117-126: The scan_entities path should use a documented backend
seam rather than directly calling the private _search_entities_impl method. Add
or expose a named method such as BaseEntityBackend.scan_entities (or
search_entities_internal), implement it across backends as needed, and update
both scan_entities and consolidate_guidelines to call that contract while
preserving the no-hook read behavior.

In `@altk_evolve/frontend/mcp/mcp_server.py`:
- Around line 687-701: Eliminate the duplicate namespace scan in run_retention
by reusing entities fetched by RetentionEngine.apply/evaluate when constructing
snapshots. Prefer exposing the engine’s fetched entities or lazily resolving
only report-referenced IDs, while preserving retention report behavior and
existing scan limits.
- Around line 780-797: Update the health computation in the MCP health response
so each enabled plugin’s health is derived from its manager’s registered hooks
using the existing hook-checking API such as has_hooks_for, rather than only
checking manager existence and enabled state. Replace the literal
retention_available value with the actual retention capability or probe result,
and ensure failed or unregistered plugins cause the overall healthy result to be
false.
- Around line 583-594: The entity validation loop performs one scan_entities
call per unique entity ID; replace it with a single batch or ID-filtered scan,
then partition the returned entities into missing, denied, and allowed using
_entity_owned_by while preserving input-ID handling. In the record_access flow,
skip the backend call when allowed is empty while retaining the existing
timestamp behavior for non-empty updates.

In `@tests/unit/test_mcp_compliance_tools.py`:
- Around line 98-111: Add a test case in
test_get_entity_enforces_attributed_owner that uses the default
record_access=True for a non-owner, then assert the denial response and verify
client.get_entity_by_id and client.record_access are not called. Keep the
existing parametrized record_access=False coverage unchanged.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7915f270-3566-4894-b5cd-ebb096b4a53e

📥 Commits

Reviewing files that changed from the base of the PR and between f24d05a and 0d9d382.

📒 Files selected for processing (7)
  • README.md
  • altk_evolve/frontend/client/evolve_client.py
  • altk_evolve/frontend/mcp/mcp_server.py
  • altk_evolve/retention/engine.py
  • docs/guides/retention.md
  • tests/unit/test_mcp_compliance_tools.py
  • tests/unit/test_retention.py

Comment thread altk_evolve/frontend/mcp/mcp_server.py Outdated
Comment thread altk_evolve/frontend/mcp/mcp_server.py
Comment thread altk_evolve/frontend/mcp/mcp_server.py
Comment thread altk_evolve/retention/engine.py Outdated
@agent-toolkit-owl

agent-toolkit-owl commented Jul 25, 2026

Copy link
Copy Markdown

No vulnerabilities found.

@illeatmyhat

Copy link
Copy Markdown
Collaborator Author

Review follow-up is in 1643bca.

Addressed:

  • fixed cursor advancement when an access-time re-fetch loses an entity
  • checks ownership before any access-stamping read
  • validates malformed hook YAML as a structured unhealthy response
  • lazily selects the retention scan fallback
  • added a documented backend administrative-scan seam
  • removed the duplicate retention snapshot scan
  • reports configured plugin health from registered hooks and probes retention client capabilities
  • added denial-path and regression coverage
  • pinned react-router and react-router-dom to 7.18.1, removing the vulnerable 7.17.0 lock entry for CVE-2026-55685

I retained per-ID ownership lookups in record_access instead of scanning up to 100,000 unrelated entities: the current backends do not expose an ID-list batch lookup, and a bounded namespace scan could incorrectly report valid IDs beyond the bound as missing. The empty-allowed-set write is now short-circuited.

Validation: 966 Python tests passed (8 skipped, 279 deselected), 90 focused tests passed, pre-commit passed, and the clean UI install, 6 UI tests, and production build passed.

Note: npm now reports a separate newer RSC advisory whose suggested fix is react-router 8.3.0, but there is currently no matching react-router-dom 8.3.0 release. The CVEs reported by this PR compliance failure are fixed by the aligned 7.18.1 upgrade.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 2

🧹 Nitpick comments (1)
altk_evolve/frontend/mcp/mcp_server.py (1)

1149-1165: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse _parse_datetime instead of re-implementing ISO-8601/UTC-fallback parsing.

This inline datetime.fromisoformat(created_at.replace("Z", "+00:00")) + naive-tz-fallback logic duplicates what _parse_datetime (already used by record_access and run_retention) does, including handling None/empty input.

♻️ Proposed refactor
-        parsed_created_at = None
-        if created_at:
-            from datetime import UTC, datetime
-
-            try:
-                parsed_created_at = datetime.fromisoformat(created_at.replace("Z", "+00:00"))
-            except ValueError:
-                return json.dumps({"error": "Invalid created_at", "message": "created_at must be ISO-8601"})
-            if parsed_created_at.tzinfo is None:
-                parsed_created_at = parsed_created_at.replace(tzinfo=UTC)
+        try:
+            parsed_created_at = _parse_datetime(created_at, field_name="created_at")
+        except ValueError:
+            return json.dumps({"error": "Invalid created_at", "message": "created_at must be ISO-8601"})
🤖 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 `@altk_evolve/frontend/mcp/mcp_server.py` around lines 1149 - 1165, The entity
creation flow should reuse the existing _parse_datetime helper instead of
duplicating ISO-8601 parsing and UTC fallback logic. Replace the inline
parsed_created_at conversion in the entity creation function with
_parse_datetime, preserving the existing invalid-created_at error response and
handling of empty or missing values.
🤖 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 `@altk_evolve/frontend/mcp/mcp_server.py`:
- Around line 806-821: Update the healthy aggregate in the MCP health response
to require hook_engine_available whenever any plugin is enabled, while
preserving the existing plugin health and retention checks. Use the existing
plugins collection and hook_engine_available value so enabled plugins report
unhealthy when the hook engine is unavailable; leave the behavior unchanged when
no plugins are enabled.

In `@altk_evolve/hooks/manager.py`:
- Around line 447-458: Preserve structured violation details in the synchronous
_invoke path: add a details field to MemoryPolicyViolation, pass
HookPolicyViolation.details when reconstructing it around the _invoke handling,
and ensure the resulting PluginViolation retains that data for
delete_entity_by_id callers. Update the regression test to assert the
LegalHoldMemoryPlugin entity_id is preserved.

---

Nitpick comments:
In `@altk_evolve/frontend/mcp/mcp_server.py`:
- Around line 1149-1165: The entity creation flow should reuse the existing
_parse_datetime helper instead of duplicating ISO-8601 parsing and UTC fallback
logic. Replace the inline parsed_created_at conversion in the entity creation
function with _parse_datetime, preserving the existing invalid-created_at error
response and handling of empty or missing values.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e0aa054d-670e-4469-a035-6501c7134bac

📥 Commits

Reviewing files that changed from the base of the PR and between 0d9d382 and 247b278.

⛔ Files ignored due to path filters (1)
  • altk_evolve/frontend/ui/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (17)
  • altk_evolve/backend/base.py
  • altk_evolve/frontend/client/evolve_client.py
  • altk_evolve/frontend/mcp/mcp_server.py
  • altk_evolve/frontend/ui/package.json
  • altk_evolve/hooks/manager.py
  • altk_evolve/hooks/plugin.py
  • altk_evolve/hooks/plugins/legal_hold.py
  • altk_evolve/retention/engine.py
  • docs/guides/memory-hooks.md
  • examples/cuga_compliance_poc_hooks.yaml
  • examples/hooks_plugins.yaml
  • tests/unit/test_combine_guidelines.py
  • tests/unit/test_filesystem_backend.py
  • tests/unit/test_hooks_seam.py
  • tests/unit/test_mcp_compliance_tools.py
  • tests/unit/test_mcp_server.py
  • tests/unit/test_retention.py

Comment on lines +806 to +821
hook_coverage = {hook.value: hooks_active(hook) for hook in HookType}
hook_engine_available = engine_available()
retention_available = (callable(getattr(client, "scan_entities", None)) or callable(getattr(client, "get_all_entities", None))) and all(
callable(getattr(client, method, None)) for method in ("patch_entity_metadata", "delete_entity_by_id")
)
healthy = client.ready() and retention_available and all(not plugin["enabled"] or plugin["healthy"] for plugin in plugins)
return _json_response(
{
"healthy": healthy,
"evolve_version": package_version,
"backend": evolve_config.backend,
"namespace_id": resolved_ns,
"retention_available": retention_available,
"hooks_enabled": manager is not None,
"hook_engine_available": hook_engine_available,
"hook_coverage": hook_coverage,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check whether has_hooks_for already gates on engine availability.
ast-grep outline altk_evolve/hooks/manager.py --items all
rg -n -A10 'def has_hooks_for' altk_evolve/hooks/manager.py
rg -n 'HAS_CPEX|engine_available' altk_evolve/hooks/manager.py altk_evolve/hooks/types.py

Repository: AgentToolkit/altk-evolve

Length of output: 3522


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== mcp_server health snippet =="
sed -n '780,840p' altk_evolve/frontend/mcp/mcp_server.py

echo
echo "== hooks types =="
sed -n '1,90p' altk_evolve/hooks/types.py
sed -n '200,260p' altk_evolve/hooks/types.py

echo
echo "== hook manager relevant methods =="
sed -n '100,210p' altk_evolve/hooks/manager.py
sed -n '320,360p' altk_evolve/hooks/manager.py
sed -n '300,310p' altk_evolve/hooks/manager.py

echo
echo "== health / plugin health usages =="
rg -n -C3 '"plugin_health"|plugin_health|hooks_active\(|has_hooks_for\(|engine_available\(' altk_evolve/frontend/mcp/mcp_server.py altk_evolve/hooks/manager.py altk_evolve/hooks/types.py

Repository: AgentToolkit/altk-evolve

Length of output: 23104


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== health function context =="
rg -n -C20 'def .*hook.*health|def .*health|hook_engine_available|hook_coverage|healthy = client.ready' altk_evolve/frontend/mcp/mcp_server.py

echo
echo "== hook activation gates =="
rg -n -C5 'hooks_active\(HookType\.(MEMORY_PRE_DELETE|MEMORY_PRE_WRITE|MEMORY_PRE_METADATA_PATCH|MEMORY_PRE_NAMESPACE_DELETE)\)' altk_evolve backend -g '*.py'

echo
echo "== static verifier: health expression vs engine dependency =="
python3 - <<'PY'
from pathlib import Path
import re

mcp = Path("altk_evolve/frontend/mcp/mcp_server.py").read_text()
m = re.search(r'healthy = (.+?)(?:\n    return _json_response|$)', mcp, re.S)
if not m:
    raise SystemExit("healthy assignment not found")
expr = " ".join(m.group(1).split())
print(expr)
print("depends_on_hook_engine_available=", "hook_engine_available" in expr)

hooks = Path("altk_evolve/hooks/manager.py").read_text()
hp = re.search(r'''def hooks_active\([^)]*\):\s*""".*?"""(?:\n\s*if not .*?\n\s*return .*?\n)?''', hooks, re.S)
hp2 = re.search(r'''def hooks_active\(hook_type: HookType\).\s*(?:""".*?""")?\s*(\s*if not.*?\n\s*return.*?\n\s*return.*?)''', hooks, re.S)
print("\\nhooks_active body:", hp2.group(1).strip() if hp2 else "not parsed")
print("hooks_active_gates_engine=", "not _plugins_enabled or _plugin_manager is None" in (hp2.group(1) if hp2 else ""))
PY

Repository: AgentToolkit/altk-evolve

Length of output: 6807


Include hook_engine_available in the MCP health flag when hooks are enabled.

hooks_active() only checks _plugins_enabled/_plugin_manager and delegates to has_hooks_for(), so an enabled plugin reporting subscribed hooks still reports healthy: true even when cpex is unavailable and dispatches no-op. If any plugin is enabled, require hook_engine_available in the aggregate health before retention operations are allowed to report healthy.

🛡️ Proposed fix
-    healthy = client.ready() and retention_available and all(not plugin["enabled"] or plugin["healthy"] for plugin in plugins)
+    any_plugin_enabled = any(plugin["enabled"] for plugin in plugins)
+    healthy = (
+        client.ready()
+        and retention_available
+        and (not any_plugin_enabled or hook_engine_available)
+        and all(not plugin["enabled"] or plugin["healthy"] for plugin in plugins)
+    )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
hook_coverage = {hook.value: hooks_active(hook) for hook in HookType}
hook_engine_available = engine_available()
retention_available = (callable(getattr(client, "scan_entities", None)) or callable(getattr(client, "get_all_entities", None))) and all(
callable(getattr(client, method, None)) for method in ("patch_entity_metadata", "delete_entity_by_id")
)
healthy = client.ready() and retention_available and all(not plugin["enabled"] or plugin["healthy"] for plugin in plugins)
return _json_response(
{
"healthy": healthy,
"evolve_version": package_version,
"backend": evolve_config.backend,
"namespace_id": resolved_ns,
"retention_available": retention_available,
"hooks_enabled": manager is not None,
"hook_engine_available": hook_engine_available,
"hook_coverage": hook_coverage,
hook_coverage = {hook.value: hooks_active(hook) for hook in HookType}
hook_engine_available = engine_available()
retention_available = (callable(getattr(client, "scan_entities", None)) or callable(getattr(client, "get_all_entities", None))) and all(
callable(getattr(client, method, None)) for method in ("patch_entity_metadata", "delete_entity_by_id")
)
any_plugin_enabled = any(plugin["enabled"] for plugin in plugins)
healthy = (
client.ready()
and retention_available
and (not any_plugin_enabled or hook_engine_available)
and all(not plugin["enabled"] or plugin["healthy"] for plugin in plugins)
)
return _json_response(
{
"healthy": healthy,
"evolve_version": package_version,
"backend": evolve_config.backend,
"namespace_id": resolved_ns,
"retention_available": retention_available,
"hooks_enabled": manager is not None,
"hook_engine_available": hook_engine_available,
"hook_coverage": hook_coverage,
🤖 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 `@altk_evolve/frontend/mcp/mcp_server.py` around lines 806 - 821, Update the
healthy aggregate in the MCP health response to require hook_engine_available
whenever any plugin is enabled, while preserving the existing plugin health and
retention checks. Use the existing plugins collection and hook_engine_available
value so enabled plugins report unhealthy when the hook engine is unavailable;
leave the behavior unchanged when no plugins are enabled.

Comment on lines +447 to +458
try:
out = method(plain, hook_ctx)
except HookPolicyViolation as violation:
return PluginResult(
continue_processing=False,
violation=PluginViolation(
reason=violation.reason,
description=violation.reason,
code=violation.code,
details=violation.details,
),
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Preserve structured violation details through the public exception.

The adapter copies HookPolicyViolation.details into PluginViolation, but the synchronous _invoke path at Line 577-584 reconstructs MemoryPolicyViolation with only reason, code, and plugin name. Consequently, LegalHoldMemoryPlugin’s entity_id never reaches delete_entity_by_id callers. Add a details field to MemoryPolicyViolation, pass it through, and assert it in the regression test.

🤖 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 `@altk_evolve/hooks/manager.py` around lines 447 - 458, Preserve structured
violation details in the synchronous _invoke path: add a details field to
MemoryPolicyViolation, pass HookPolicyViolation.details when reconstructing it
around the _invoke handling, and ensure the resulting PluginViolation retains
that data for delete_entity_by_id callers. Update the regression test to assert
the LegalHoldMemoryPlugin entity_id is preserved.

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.

2 participants