feat(altk_evolve): entity sharing — public/private visibility#201
Merged
visahak merged 18 commits intoAgentToolkit:mainfrom Apr 28, 2026
Merged
feat(altk_evolve): entity sharing — public/private visibility#201visahak merged 18 commits intoAgentToolkit:mainfrom
visahak merged 18 commits intoAgentToolkit:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds metadata-driven entity sharing (owner_id, visibility, published_at), backend helpers to patch/merge entity metadata, client APIs for public entity discovery and metadata patching, MCP tools to create/publish/unpublish entities, README updates, and new unit and e2e tests across filesystem, Milvus, and Postgres backends. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCP_Server
participant Backend
participant DB
Client->>MCP_Server: publish_entity(entity_id, user_id)
MCP_Server->>Backend: get_entity_by_id(namespace, entity_id)
Backend-->>MCP_Server: RecordedEntity or None
alt entity found
MCP_Server->>Backend: patch_entity(namespace, entity_id, metadata_updates with visibility/published_at)
Backend->>DB: load entity / validate existence
DB-->>Backend: entity row
Backend->>DB: update entity (merge metadata / upsert)
DB-->>Backend: updated row
Backend-->>MCP_Server: RecordedEntity (merged metadata)
MCP_Server-->>Client: JSON response (updated entity)
else not found
MCP_Server-->>Client: JSON error
end
sequenceDiagram
participant Client
participant EvolveClient
participant NamespaceService
participant StorageBackends
Client->>EvolveClient: get_public_entities(query,type,limit)
EvolveClient->>NamespaceService: search_namespaces()
NamespaceService-->>EvolveClient: list(namespaces)
loop per-namespace
EvolveClient->>StorageBackends: search_entities(filters={'metadata.visibility':'public', ...}, per_ns_limit)
StorageBackends-->>EvolveClient: list[RecordedEntity]
end
EvolveClient-->>Client: aggregated, deduplicated public results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
visahak
force-pushed
the
feature/entity-sharing-full-mcp
branch
from
df42b97 to
4876121
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
altk_evolve/frontend/mcp/mcp_server.py (1)
274-287:⚠️ Potential issue | 🟡 MinorValidate that
metadataparses to a JSON object before mutating it.A valid JSON value like
[]or"x"reaches Line 285 and then raises because it is not a dict. Return a structured validation error instead of falling through to the generic server-error path.Suggested fix
metadata_dict = {} if metadata: try: - metadata_dict = json.loads(metadata) + parsed_metadata = json.loads(metadata) except json.JSONDecodeError as e: logger.exception(f"Invalid JSON in metadata parameter: {str(e)}") return json.dumps({"error": "Invalid JSON", "message": f"Failed to parse metadata: {str(e)}", "invalid_metadata": metadata}) + if not isinstance(parsed_metadata, dict): + return json.dumps({"error": "Invalid metadata", "message": "metadata must be a JSON object"}) + metadata_dict = parsed_metadata🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@altk_evolve/frontend/mcp/mcp_server.py` around lines 274 - 287, The parsed metadata from json.loads(metadata) must be validated as a JSON object (dict) before mutating it; in the block around json.loads in mcp_server.py (the metadata_dict variable and subsequent mutations: metadata_dict.setdefault("creation_mode", ...), metadata_dict["visibility"], metadata_dict["owner_id"]), check isinstance(metadata_dict, dict) after parsing and if it's not a dict return a structured JSON error (e.g., {"error":"Invalid metadata type","message":"metadata must be a JSON object","invalid_metadata": metadata}) instead of proceeding, so non-object JSON like arrays or strings are rejected with a clear response rather than causing later exceptions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@altk_evolve/backend/base.py`:
- Around line 91-107: The update_entity_metadata implementation uses a new
current timestamp when calling patch_entity, which causes created_at to be
overwritten for metadata-only updates; change update_entity_metadata to reuse
the existing entity timestamp (e.g., entity.created_at or
entity.get("created_at")) instead of computing now, pass that timestamp into
patch_entity, and return RecordedEntity with the merged metadata and the
original timestamp so filesystem/backends won't persist a new created_at; locate
this logic in update_entity_metadata, search_entities result handling, the call
to patch_entity, and the RecordedEntity construction to make the change.
In `@altk_evolve/backend/filesystem.py`:
- Around line 173-176: Wrap the update sequence inside the critical section in a
try/finally so that if any exception occurs after loading namespace data the
instance attribute _active_data (set by _load_namespace_data(namespace_id)) is
cleared in the finally block to avoid stale data; additionally, validate
entity_id before calling _update_entity(namespace_id, entity_id, entity_type,
content_str, timestamp, metadata) and raise a clear exception (e.g., ValueError)
if entity_id is missing/invalid so the operation does not silently no-op; keep
the calls to _load_namespace_data, _update_entity, and _post_update unchanged
but ensure exceptions propagate after cleaning up.
In `@altk_evolve/backend/milvus.py`:
- Around line 307-313: When applying a metadata-only patch in the block that
parses the existing Milvus entity (parse_milvus_entity) and then calls
_update_entity, preserve the original entity creation timestamp instead of
generating a fresh timestamp: detect that only metadata is changing, fetch the
existing timestamp/created_at from the parsed entity (e.g., entity.created_at or
entity.timestamp), pass that original timestamp into _update_entity (instead of
the new int(datetime...timestamp() value) currently assigned to timestamp), and
ensure the returned RecordedEntity constructed via
RecordedEntity(**{**entity.model_dump(), "metadata": merged}) retains the
original created_at value.
In `@altk_evolve/frontend/mcp/mcp_server.py`:
- Around line 327-334: The patch currently sets "owner_id": user_id
unconditionally which will write null when user_id is absent; update the call to
get_client().patch_entity_metadata (the metadata_updates passed for entity_id)
so that "owner_id" is only added to the metadata_updates dict when user_id is
present (i.e., check user_id is not None before inserting owner_id), while still
always setting "visibility" to "public" and "published_at" to
datetime.now(UTC).isoformat().
In `@README.md`:
- Around line 152-172: The fenced code blocks in the README examples lack
language labels (MD040); update each fence to use a language tag `python` for
the examples: modify the blocks containing publish_entity(entity_id="42",
user_id="alice"), unpublish_entity(entity_id="42"), get_entities(task="write
safer code", include_public=True), and create_entity(content="...",
entity_type="guideline", visibility="public", owner_id="alice") so their opening
triple-backticks are changed to ```python to satisfy markdownlint.
In `@tests/e2e/conftest.py`:
- Around line 8-26: The mcp fixture mutates os.environ and
milvus_client_settings.uri without restoring them; capture original values for
EVOLVE_NAMESPACE_ID, EVOLVE_BACKEND, EVOLVE_SQLITE_PATH and EVOLVE_DATA_DIR (if
set) and original_milvus_uri before modifying, then register a teardown via
request.addfinalizer (or a try/finally around operations) that restores
milvus_client_settings.uri to original_milvus_uri (or None) and resets each
EVOLVE_* key to its previous value or deletes the key if it didn’t exist; also
ensure any milvus_db_file created is not left referenced in env after teardown.
- Around line 35-48: The teardown currently only closes the local setup
EvolveClient instance (evolve_client) leaving the actual MCP singleton client
(mcp_server_module._client created via get_client()) open; update the teardown
to locate and close the real singleton backend by calling get_client() or
checking mcp_server_module._client and invoking its backend.close() (with safe
try/except), so the MCP/Milvus Lite resources are properly released in tests.
In `@tests/unit/test_entity_sharing_e2e.py`:
- Around line 1-14: The tests are end-to-end but are incorrectly marked as unit
via the pytestmark variable; update the module-level marker from pytestmark =
pytest.mark.unit to pytestmark = pytest.mark.e2e (or remove and add pytestmark =
pytest.mark.e2e) so the file is classified as e2e tests; alternatively, if you
prefer path-based organization, move the file into the e2e test directory and
ensure the module-level pytestmark uses pytest.mark.e2e (refer to the pytestmark
symbol in this module).
---
Outside diff comments:
In `@altk_evolve/frontend/mcp/mcp_server.py`:
- Around line 274-287: The parsed metadata from json.loads(metadata) must be
validated as a JSON object (dict) before mutating it; in the block around
json.loads in mcp_server.py (the metadata_dict variable and subsequent
mutations: metadata_dict.setdefault("creation_mode", ...),
metadata_dict["visibility"], metadata_dict["owner_id"]), check
isinstance(metadata_dict, dict) after parsing and if it's not a dict return a
structured JSON error (e.g., {"error":"Invalid metadata
type","message":"metadata must be a JSON object","invalid_metadata": metadata})
instead of proceeding, so non-object JSON like arrays or strings are rejected
with a clear response rather than causing later exceptions.
🪄 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
Run ID: 182c2944-da14-4c08-b9e6-60e972a41985
📒 Files selected for processing (15)
README.mdaltk_evolve/backend/base.pyaltk_evolve/backend/filesystem.pyaltk_evolve/backend/milvus.pyaltk_evolve/backend/postgres.pyaltk_evolve/frontend/client/evolve_client.pyaltk_evolve/frontend/mcp/mcp_server.pyaltk_evolve/schema/core.pytests/e2e/conftest.pytests/e2e/test_mcp.pytests/e2e/test_sharing.pytests/unit/test_entity_sharing.pytests/unit/test_entity_sharing_e2e.pytests/unit/test_milvus_backend.pytests/unit/test_postgres_backend.py
💤 Files with no reviewable changes (1)
- tests/e2e/test_mcp.py
Collaborator
Author
|
This is ready for review. It applies the same sharing technique on the full mcp version. Separate from claude, bob, and codex versions. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
visahak
force-pushed
the
feature/entity-sharing-full-mcp
branch
from
9ecffbb to
f97adfb
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Collaborator
Author
|
Tested with the cuga agent and postgres. ready for review. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
visahak
force-pushed
the
feature/entity-sharing-full-mcp
branch
from
1022d43 to
f95b2f3
Compare
- Fixed test_uninstall_prunes_evolve_only_hook_groups by adding empty group pruning logic to _remove_user_prompt_hook in install.sh - Fixed test_skips_symlinked_entities by correcting test expectations - symlinks exist in git clones but are filtered by retrieve script - Added RETRIEVE_SCRIPT constant to test_sync.py for cleaner test code - All 230 platform integration tests now pass
…ng tests Fixes failing CI check: check-formatting (3.12) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Unit tests (postgres + milvus): - Verifies native metadata merge SQL / upsert path - Missing entity raises EvolveException - Non-numeric ID rejected early E2E tests (filesystem + milvus, via --run-e2e): - publish_entity / unpublish_entity full MCP flow - create_entity with visibility=public - Content preserved after metadata patch - Extracted shared mcp fixture into tests/e2e/conftest.py and extended params to include milvus
…ship mismatch tests
visahak
force-pushed
the
feature/entity-sharing-full-mcp
branch
from
f95b2f3 to
e7aaa5c
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Vatche Isahagian <vatchei@ibm.com>
Collaborator
Author
|
Rebased and resolved conflicts. This PR is ready for review. |
Collaborator
|
@visahak verified this works by running the mcp and test it with two different namespaced mcp servers, bob create entity, and publish it, alice can get the public entity, bob unpublish it, alice can't get bob's private entity |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses partly issue #182 in the mcp full version.
Summary
owner_id,visibility,published_at) — no schema migration required, all backends unaffectedpublish_entityandunpublish_entityMCP tools to control visibility per entityget_entitiesgainsinclude_public=Trueto merge public entities from all namespaces, annotated with[public: {owner_id}]create_entitygainsowner_idandvisibilityparams;save_trajectorygainsowner_idget_public_entities,get_entity_by_id,patch_entity_metadataonEvolveClientpatch_entityonBaseEntityBackend(+ filesystem override) enables metadata-only patches without changing entity ID or contentUnit Tests (291 total)
test_entity_sharing.pycreate_entity,publish_entity,unpublish_entity,get_entities(include_public)— all mockedtest_entity_sharing_e2e.pypatch_entity_metadatapersistence, type filtering — realFilesystemEntityBackendtest_postgres_backend.pyUPDATE ... metadata || jsonb, missing entity error, non-numeric ID rejectiontest_milvus_backend.pymilvus.queryfetch + upsert merge, missing entity error, non-numeric ID rejectionE2E Tests — 30 total (
--run-e2e)test_sharing.pycreate_entity(visibility=public), content preservation after metadata patch, not-found error handlingtest_mcp.pySummary by CodeRabbit
New Features
Documentation
Tests