From c130bcff334f305cbbeec655cd5377c939e89810 Mon Sep 17 00:00:00 2001 From: Viktor Barzin Date: Sun, 22 Mar 2026 23:36:13 +0200 Subject: [PATCH] feat: sharing tests, property tests, tag-share UI, inline errors, route fix - Add 10 sharing endpoint tests (share/unshare memory, tag shares, shared-with-me, my-shares, recall with shared, update permission checks) - Add hypothesis property-based tests for model validation (roundtrip, bounds, enum, sort_by, limit) - Tighten model validation: sort_by Literal, limit ge=1/le=500, tags/keywords max_length - Fix dashboard shared_with_me stat to include tag-based shares - Add tag-sharing management UI (share/remove tags, user typeahead) - Replace alert() with inline error messages - Fix route ordering bug: share-tag routes before {memory_id} parameterized routes --- .gitignore | 1 + pyproject.toml | 2 +- src/claude_memory/api/app.py | 72 ++++--- src/claude_memory/api/models.py | 8 +- src/claude_memory/ui/static/index.html | 63 ++++++ src/claude_memory/ui/static/js/api.js | 8 +- src/claude_memory/ui/static/js/memories.js | 64 +++++- tests/test_api.py | 217 +++++++++++++++++++++ tests/test_properties.py | 111 +++++++++++ 9 files changed, 503 insertions(+), 43 deletions(-) create mode 100644 tests/test_properties.py diff --git a/.gitignore b/.gitignore index 9605f28..1662af9 100644 --- a/.gitignore +++ b/.gitignore @@ -43,3 +43,4 @@ docker/pgdata/ # Database *.db *.sqlite3 +.hypothesis/ diff --git a/pyproject.toml b/pyproject.toml index 9728689..99888cb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,7 +18,7 @@ classifiers = [ [project.optional-dependencies] api = ["fastapi>=0.115", "asyncpg>=0.30", "uvicorn>=0.34", "pydantic>=2.0", "alembic>=1.14", "sqlalchemy>=2.0", "psycopg2-binary>=2.9", "mcp>=1.0.0"] vault = ["hvac>=2.0"] -dev = ["pytest>=8.0", "pytest-asyncio>=0.24", "ruff>=0.8", "mypy>=1.13", "httpx>=0.28", "cryptography>=43.0"] +dev = ["pytest>=8.0", "pytest-asyncio>=0.24", "ruff>=0.8", "mypy>=1.13", "httpx>=0.28", "cryptography>=43.0", "hypothesis>=6.0"] [project.scripts] claude-memory-server = "claude_memory.mcp_server:main" diff --git a/src/claude_memory/api/app.py b/src/claude_memory/api/app.py index 2f804a7..86d8c8b 100644 --- a/src/claude_memory/api/app.py +++ b/src/claude_memory/api/app.py @@ -454,7 +454,12 @@ async def get_stats(user: AuthUser = Depends(get_current_user)) -> dict[str, Any user.user_id, ) shared_with_me = await conn.fetchval( - "SELECT COUNT(*) FROM memory_shares WHERE shared_with = $1", + """SELECT COUNT(DISTINCT m.id) FROM memories m + WHERE m.deleted_at IS NULL AND ( + EXISTS (SELECT 1 FROM memory_shares ms WHERE ms.memory_id = m.id AND ms.shared_with = $1) + OR EXISTS (SELECT 1 FROM tag_shares ts WHERE ts.owner_id = m.user_id AND ts.shared_with = $1 + AND EXISTS (SELECT 1 FROM unnest(string_to_array(m.tags, ',')) t WHERE trim(t) = ts.tag)) + )""", user.user_id, ) @@ -467,6 +472,41 @@ async def get_stats(user: AuthUser = Depends(get_current_user)) -> dict[str, Any } +# NOTE: Literal-path routes (share-tag, shared-with-me, my-shares) must be +# registered before parameterized routes ({memory_id}) to prevent FastAPI +# from matching the literal segment as a path parameter. + + +@app.post("/api/memories/share-tag") +async def share_tag(body: ShareTag, user: AuthUser = Depends(get_current_user)) -> dict[str, Any]: + pool = await get_pool() + if body.shared_with == user.user_id: + raise HTTPException(status_code=400, detail="Cannot share with yourself") + + async with pool.acquire() as conn: + await conn.execute( + """ + INSERT INTO tag_shares (owner_id, tag, shared_with, permission) + VALUES ($1, $2, $3, $4) + ON CONFLICT (owner_id, tag, shared_with) + DO UPDATE SET permission = EXCLUDED.permission + """, + user.user_id, body.tag, body.shared_with, body.permission, + ) + return {"shared_tag": body.tag, "with": body.shared_with, "permission": body.permission} + + +@app.delete("/api/memories/share-tag") +async def unshare_tag(body: UnshareTag, user: AuthUser = Depends(get_current_user)) -> dict[str, Any]: + pool = await get_pool() + async with pool.acquire() as conn: + await conn.execute( + "DELETE FROM tag_shares WHERE owner_id = $1 AND tag = $2 AND shared_with = $3", + user.user_id, body.tag, body.shared_with, + ) + return {"unshared_tag": body.tag, "from": body.shared_with} + + @app.delete("/api/memories/{memory_id}") async def delete_memory(memory_id: int, user: AuthUser = Depends(get_current_user)) -> dict[str, Any]: pool = await get_pool() @@ -665,36 +705,6 @@ async def unshare_memory(memory_id: int, target_user: str, user: AuthUser = Depe return {"unshared": memory_id, "from": target_user} -@app.post("/api/memories/share-tag") -async def share_tag(body: ShareTag, user: AuthUser = Depends(get_current_user)) -> dict[str, Any]: - pool = await get_pool() - if body.shared_with == user.user_id: - raise HTTPException(status_code=400, detail="Cannot share with yourself") - - async with pool.acquire() as conn: - await conn.execute( - """ - INSERT INTO tag_shares (owner_id, tag, shared_with, permission) - VALUES ($1, $2, $3, $4) - ON CONFLICT (owner_id, tag, shared_with) - DO UPDATE SET permission = EXCLUDED.permission - """, - user.user_id, body.tag, body.shared_with, body.permission, - ) - return {"shared_tag": body.tag, "with": body.shared_with, "permission": body.permission} - - -@app.delete("/api/memories/share-tag") -async def unshare_tag(body: UnshareTag, user: AuthUser = Depends(get_current_user)) -> dict[str, Any]: - pool = await get_pool() - async with pool.acquire() as conn: - await conn.execute( - "DELETE FROM tag_shares WHERE owner_id = $1 AND tag = $2 AND shared_with = $3", - user.user_id, body.tag, body.shared_with, - ) - return {"unshared_tag": body.tag, "from": body.shared_with} - - @app.get("/api/memories/shared-with-me") async def shared_with_me(user: AuthUser = Depends(get_current_user)) -> dict[str, Any]: pool = await get_pool() diff --git a/src/claude_memory/api/models.py b/src/claude_memory/api/models.py index a0a0c6c..61b9704 100644 --- a/src/claude_memory/api/models.py +++ b/src/claude_memory/api/models.py @@ -9,8 +9,8 @@ MAX_MEMORY_CHARS = 800 class MemoryStore(BaseModel): content: str = Field(..., max_length=MAX_MEMORY_CHARS) category: str = "facts" - tags: str = "" - expanded_keywords: str = "" + tags: str = Field(default="", max_length=500) + expanded_keywords: str = Field(default="", max_length=500) importance: float = Field(default=0.5, ge=0.0, le=1.0) force_sensitive: bool = False @@ -19,8 +19,8 @@ class MemoryRecall(BaseModel): context: str expanded_query: str = "" category: Optional[str] = None - sort_by: str = "importance" - limit: int = 10 + sort_by: Literal["importance", "relevance", "recency"] = "importance" + limit: int = Field(default=10, ge=1, le=500) class MemoryResponse(BaseModel): diff --git a/src/claude_memory/ui/static/index.html b/src/claude_memory/ui/static/index.html index 80ef133..ecb26fc 100644 --- a/src/claude_memory/ui/static/index.html +++ b/src/claude_memory/ui/static/index.html @@ -158,6 +158,11 @@ + + +
My Memories
@@ -325,6 +330,64 @@
+ + +
+
+
Tag Shares
+ +
+ + +
+
+
+ + +
+
+ + +
+ +
+
+
+ + +
+
+ +
+ + + + +
diff --git a/src/claude_memory/ui/static/js/api.js b/src/claude_memory/ui/static/js/api.js index 5b8b4ab..9920f7d 100644 --- a/src/claude_memory/ui/static/js/api.js +++ b/src/claude_memory/ui/static/js/api.js @@ -63,8 +63,12 @@ const api = { return this.fetch(url, { method: 'PUT', body: JSON.stringify(data) }); }, - del(url) { - return this.fetch(url, { method: 'DELETE' }); + del(url, data) { + const options = { method: 'DELETE' }; + if (data) { + options.body = JSON.stringify(data); + } + return this.fetch(url, options); }, async login(key) { diff --git a/src/claude_memory/ui/static/js/memories.js b/src/claude_memory/ui/static/js/memories.js index 167eff7..57424a9 100644 --- a/src/claude_memory/ui/static/js/memories.js +++ b/src/claude_memory/ui/static/js/memories.js @@ -23,9 +23,15 @@ function memoriesBrowser() { showShareForm: null, shareForm: { user: '', permission: 'read' }, shareUserFilter: '', + // Tag shares + tagShares: [], + showTagShareForm: false, + tagShareForm: { tag: '', user: '', permission: 'read' }, + tagShareUserFilter: '', + errorMsg: '', async init() { - await Promise.all([this.loadMemories(), this.loadShared(), this.loadCategories(), this.loadUsers()]); + await Promise.all([this.loadMemories(), this.loadShared(), this.loadCategories(), this.loadUsers(), this.loadTagShares()]); }, async loadCategories() { @@ -100,6 +106,7 @@ function memoriesBrowser() { }, async saveEdit(id) { + this.errorMsg = ''; try { await api.put(`/api/memories/${id}`, { content: this.editForm.content, @@ -110,7 +117,7 @@ function memoriesBrowser() { this.offset = 0; await this.loadMemories(); } catch (e) { - alert('Save failed: ' + e.message); + this.errorMsg = 'Save failed: ' + e.message; } }, @@ -119,13 +126,14 @@ function memoriesBrowser() { }, async doDelete(id) { + this.errorMsg = ''; try { await api.del(`/api/memories/${id}`); this.deleteConfirm = null; this.offset = 0; await this.loadMemories(); } catch (e) { - alert('Delete failed: ' + e.message); + this.errorMsg = 'Delete failed: ' + e.message; } }, @@ -153,6 +161,7 @@ function memoriesBrowser() { async addMemory() { if (!this.addForm.content.trim()) return; + this.errorMsg = ''; try { await api.post('/api/memories', { content: this.addForm.content, @@ -164,7 +173,7 @@ function memoriesBrowser() { this.offset = 0; await Promise.all([this.loadMemories(), this.loadCategories()]); } catch (e) { - alert('Failed to add memory: ' + e.message); + this.errorMsg = 'Failed to add memory: ' + e.message; } }, @@ -191,6 +200,7 @@ function memoriesBrowser() { async shareMemory(memId) { if (!this.shareForm.user.trim()) return; + this.errorMsg = ''; try { await api.post(`/api/memories/${memId}/share`, { shared_with: this.shareForm.user, @@ -203,7 +213,51 @@ function memoriesBrowser() { await this.toggleShares(memId); } } catch (e) { - alert('Share failed: ' + e.message); + this.errorMsg = 'Share failed: ' + e.message; + } + }, + + async loadTagShares() { + try { + const data = await api.get('/api/memories/my-shares'); + this.tagShares = data.tag_shares || []; + } catch (e) { console.error('Failed to load tag shares:', e); } + }, + + get filteredTagShareUsers() { + const q = this.tagShareForm.user.toLowerCase(); + if (!q) return this.allUsers.slice(0, 10); + return this.allUsers.filter(u => u.toLowerCase().includes(q)); + }, + + selectTagShareUser(user) { + this.tagShareForm.user = user; + }, + + async addTagShare() { + if (!this.tagShareForm.tag.trim() || !this.tagShareForm.user.trim()) return; + this.errorMsg = ''; + try { + await api.post('/api/memories/share-tag', { + tag: this.tagShareForm.tag, + shared_with: this.tagShareForm.user, + permission: this.tagShareForm.permission, + }); + this.tagShareForm = { tag: '', user: '', permission: 'read' }; + this.showTagShareForm = false; + await this.loadTagShares(); + } catch (e) { + this.errorMsg = 'Failed to share tag: ' + e.message; + } + }, + + async removeTagShare(tag, sharedWith) { + this.errorMsg = ''; + try { + await api.del('/api/memories/share-tag', { tag, shared_with: sharedWith }); + await this.loadTagShares(); + } catch (e) { + this.errorMsg = 'Failed to remove tag share: ' + e.message; } }, diff --git a/tests/test_api.py b/tests/test_api.py index debd6fc..b79bb7d 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -454,3 +454,220 @@ async def test_delete_excludes_already_deleted(client): call_args = conn.fetchrow.call_args query = call_args[0][0] assert "deleted_at IS NULL" in query + + +# ─── Sharing endpoint tests ────────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_share_memory_creates_record(client): + """POST /api/memories/{id}/share creates a sharing record.""" + ac, conn, app_mod = client + conn.fetchrow.return_value = _make_memory_row(id=10, user_id="testuser") + conn.execute.return_value = None + + async with ac: + resp = await ac.post( + "/api/memories/10/share", + json={"shared_with": "otheruser", "permission": "read"}, + headers={"Authorization": "Bearer test-key"}, + ) + + assert resp.status_code == 200 + data = resp.json() + assert data["shared"] == 10 + assert data["with"] == "otheruser" + assert data["permission"] == "read" + + +@pytest.mark.asyncio +async def test_share_memory_nonexistent_returns_404(client): + """POST /api/memories/{id}/share returns 404 for non-existent memory.""" + ac, conn, app_mod = client + conn.fetchrow.return_value = None + + async with ac: + resp = await ac.post( + "/api/memories/999/share", + json={"shared_with": "otheruser", "permission": "read"}, + headers={"Authorization": "Bearer test-key"}, + ) + + assert resp.status_code == 404 + + +@pytest.mark.asyncio +async def test_unshare_memory(client): + """DELETE /api/memories/{id}/share/{user} removes sharing.""" + ac, conn, app_mod = client + conn.fetchrow.return_value = _make_memory_row(id=10, user_id="testuser") + conn.execute.return_value = None + + async with ac: + resp = await ac.delete( + "/api/memories/10/share/otheruser", + headers={"Authorization": "Bearer test-key"}, + ) + + assert resp.status_code == 200 + + +@pytest.mark.asyncio +async def test_share_tag_creates_record(client): + """POST /api/memories/share-tag creates a tag sharing record.""" + ac, conn, app_mod = client + conn.execute.return_value = None + + async with ac: + resp = await ac.post( + "/api/memories/share-tag", + json={"tag": "python", "shared_with": "otheruser", "permission": "read"}, + headers={"Authorization": "Bearer test-key"}, + ) + + assert resp.status_code == 200 + data = resp.json() + assert data["shared_tag"] == "python" + assert data["with"] == "otheruser" + assert data["permission"] == "read" + + +@pytest.mark.asyncio +async def test_unshare_tag(client): + """DELETE /api/memories/share-tag removes tag sharing.""" + ac, conn, app_mod = client + conn.execute.return_value = None + + async with ac: + resp = await ac.request( + "DELETE", + "/api/memories/share-tag", + json={"tag": "python", "shared_with": "otheruser"}, + headers={"Authorization": "Bearer test-key"}, + ) + + assert resp.status_code == 200 + data = resp.json() + assert data["unshared_tag"] == "python" + + +@pytest.mark.asyncio +async def test_shared_with_me_returns_shared_memories(client): + """GET /api/memories/shared-with-me returns individually and tag-shared memories.""" + ac, conn, app_mod = client + # Mock conn.fetch called twice: individual shares, then tag shares + # Need to include permission field for the sharing queries + conn.fetch.side_effect = [ + [_make_memory_row(id=1, content="shared memory", user_id="owner1", shared_by="owner1", permission="read")], # individual + [_make_memory_row(id=2, content="tag shared", user_id="owner2", shared_by="owner2", permission="write")], # tag-shared + ] + + async with ac: + resp = await ac.get( + "/api/memories/shared-with-me", + headers={"Authorization": "Bearer test-key"}, + ) + + assert resp.status_code == 200 + data = resp.json() + assert len(data["memories"]) == 2 + assert data["memories"][0]["id"] == 1 + assert data["memories"][1]["id"] == 2 + + +@pytest.mark.asyncio +async def test_my_shares_returns_outgoing_shares(client): + """GET /api/memories/my-shares returns outgoing memory and tag shares.""" + ac, conn, app_mod = client + now = datetime.now(timezone.utc) + # Mock conn.fetch called twice: memory_shares, then tag_shares + conn.fetch.side_effect = [ + [MockRow({"memory_id": 1, "shared_with": "user1", "permission": "read", "preview": "memory preview", "created_at": now})], # memory_shares + [MockRow({"tag": "python", "shared_with": "user2", "permission": "write", "created_at": now})], # tag_shares + ] + + async with ac: + resp = await ac.get( + "/api/memories/my-shares", + headers={"Authorization": "Bearer test-key"}, + ) + + assert resp.status_code == 200 + data = resp.json() + assert len(data["memory_shares"]) == 1 + assert len(data["tag_shares"]) == 1 + assert data["memory_shares"][0]["memory_id"] == 1 + assert data["tag_shares"][0]["tag"] == "python" + + +@pytest.mark.asyncio +async def test_recall_includes_shared_memories(client): + """POST /api/memories/recall includes shared memories with shared_by field.""" + ac, conn, app_mod = client + # recall calls fetch multiple times: own, shared, tag-shared, OR-fallback + conn.fetch.side_effect = [ + [_make_memory_row(id=1, content="own memory", user_id="testuser", shared_by=None)], # own + [_make_memory_row(id=2, content="shared memory", user_id="owner1", shared_by="owner1")], # shared + [_make_memory_row(id=3, content="tag shared", user_id="owner2", shared_by="owner2")], # tag-shared + [], # OR-fallback + ] + + async with ac: + resp = await ac.post( + "/api/memories/recall", + json={"context": "test query"}, + headers={"Authorization": "Bearer test-key"}, + ) + + assert resp.status_code == 200 + data = resp.json() + results = data["memories"] + assert len(results) == 3 + # Check that shared_by field appears in shared memories + assert results[0]["shared_by"] is None + assert results[1]["shared_by"] == "owner1" + assert results[2]["shared_by"] == "owner2" + + +@pytest.mark.asyncio +async def test_update_shared_memory_with_write_permission(client): + """PUT /api/memories/{id} succeeds when user has write permission.""" + ac, conn, app_mod = client + + # Mock check_memory_permission to return (True, "owner") + async def mock_check_permission(conn, memory_id, user_id, perm): + return (True, "owner") + + with patch("claude_memory.api.app.check_memory_permission", side_effect=mock_check_permission): + conn.execute.return_value = None + + async with ac: + resp = await ac.put( + "/api/memories/10", + json={"content": "updated content"}, + headers={"Authorization": "Bearer test-key"}, + ) + + assert resp.status_code == 200 + data = resp.json() + assert data["updated"] == 10 + + +@pytest.mark.asyncio +async def test_update_shared_memory_without_write_fails(client): + """PUT /api/memories/{id} returns 403 when user lacks write permission.""" + ac, conn, app_mod = client + + # Mock check_memory_permission to return (False, "owner") + async def mock_check_permission(conn, memory_id, user_id, perm): + return (False, "owner") + + with patch("claude_memory.api.app.check_memory_permission", side_effect=mock_check_permission): + async with ac: + resp = await ac.put( + "/api/memories/10", + json={"content": "updated content"}, + headers={"Authorization": "Bearer test-key"}, + ) + + assert resp.status_code == 403 diff --git a/tests/test_properties.py b/tests/test_properties.py new file mode 100644 index 0000000..38390b2 --- /dev/null +++ b/tests/test_properties.py @@ -0,0 +1,111 @@ +"""Property-based tests for Claude Memory models.""" + +from hypothesis import given, settings +from hypothesis import strategies as st +from pydantic import ValidationError + +from claude_memory.api.models import MemoryStore, MemoryRecall, ShareMemory + + +# Strategy for valid MemoryStore +valid_memory_store = st.builds( + MemoryStore, + content=st.text(min_size=1, max_size=800), + category=st.text(min_size=1, max_size=50), + tags=st.text(max_size=500), + expanded_keywords=st.text(max_size=500), + importance=st.floats(min_value=0.0, max_value=1.0, allow_nan=False), +) + + +@given(mem=valid_memory_store) +@settings(max_examples=50) +def test_roundtrip_memory_store(mem): + """Any valid MemoryStore can be serialized and deserialized identically.""" + data = mem.model_dump() + restored = MemoryStore(**data) + assert restored.content == mem.content + assert restored.importance == mem.importance + assert restored.tags == mem.tags + + +@given(content=st.text(min_size=801, max_size=1000)) +@settings(max_examples=20) +def test_content_over_max_rejected(content): + """Content exceeding 800 chars is rejected.""" + try: + MemoryStore(content=content) + assert False, "Should have raised ValidationError" + except ValidationError: + pass + + +@given(importance=st.floats().filter(lambda x: x < 0.0 or x > 1.0).filter(lambda x: x == x)) # exclude NaN +@settings(max_examples=20) +def test_importance_out_of_bounds_rejected(importance): + """Importance outside [0.0, 1.0] is rejected.""" + try: + MemoryStore(content="test", importance=importance) + assert False, "Should have raised ValidationError" + except ValidationError: + pass + + +@given(permission=st.text(min_size=1, max_size=20).filter(lambda x: x not in ("read", "write"))) +@settings(max_examples=20) +def test_invalid_permission_rejected(permission): + """Only 'read' or 'write' accepted for ShareMemory.permission.""" + try: + ShareMemory(shared_with="user", permission=permission) + assert False, "Should have raised ValidationError" + except ValidationError: + pass + + +@given(tags=st.text(max_size=200)) +@settings(max_examples=50) +def test_tags_splitting_consistency(tags): + """Tags splitting produces consistent results.""" + result1 = [t.strip() for t in tags.split(",") if t.strip()] + result2 = [t.strip() for t in tags.split(",") if t.strip()] + assert result1 == result2 + + +@given(sort_by=st.sampled_from(["importance", "relevance", "recency"])) +def test_valid_sort_by_accepted(sort_by): + """Valid sort_by values are accepted.""" + recall = MemoryRecall(context="test", sort_by=sort_by) + assert recall.sort_by == sort_by + + +@given(sort_by=st.text(min_size=1, max_size=20).filter(lambda x: x not in ("importance", "relevance", "recency"))) +@settings(max_examples=20) +def test_invalid_sort_by_rejected(sort_by): + """Invalid sort_by values are rejected after model update.""" + try: + MemoryRecall(context="test", sort_by=sort_by) + assert False, "Should have raised ValidationError" + except ValidationError: + pass + + +@given(limit=st.integers(min_value=501, max_value=10000)) +@settings(max_examples=10) +def test_limit_too_high_rejected(limit): + """Limit above 500 is rejected after model update.""" + try: + MemoryRecall(context="test", limit=limit) + assert False, "Should have raised ValidationError" + except ValidationError: + pass + + +@given(limit=st.integers(min_value=-100, max_value=0)) +@settings(max_examples=10) +def test_limit_zero_or_negative_rejected(limit): + """Limit <= 0 is rejected.""" + try: + MemoryRecall(context="test", limit=limit) + assert False, "Should have raised ValidationError" + except ValidationError: + pass