From 18325cc602f28bc914cf0c3fde4b417ab43a6c69 Mon Sep 17 00:00:00 2001 From: jdjioe5-cpu <257431731+jdjioe5-cpu@users.noreply.github.com> Date: Thu, 4 Jun 2026 17:09:40 +0800 Subject: [PATCH 1/4] feat: add memory edit command --- memanto/app/routes/memory.py | 68 +++++++++++++++++++++++++++++ memanto/cli/client/direct_client.py | 34 +++++++++++++++ memanto/cli/client/sdk_client.py | 34 +++++++++++++++ memanto/cli/commands/memory.py | 64 +++++++++++++++++++++++++++ tests/test_api.py | 64 +++++++++++++++++++++++++++ tests/test_cli.py | 36 +++++++++++++++ 6 files changed, 300 insertions(+) diff --git a/memanto/app/routes/memory.py b/memanto/app/routes/memory.py index 5c004b9a..e4b0d705 100644 --- a/memanto/app/routes/memory.py +++ b/memanto/app/routes/memory.py @@ -120,6 +120,18 @@ class RecallRecentRequest(BaseModel): type: list[str] | None = Field(default=None, description="Memory type filters") +class MemoryEditRequest(BaseModel): + title: str | None = Field(default=None, max_length=100) + content: str | None = Field(default=None, max_length=10000) + type: str | None = None + confidence: float | None = Field(default=None, ge=0.0, le=1.0) + tags: list[str] | None = None + source: str | None = None + + def to_updates(self) -> dict[str, object]: + return self.model_dump(exclude_none=True) + + @router.post("/{agent_id}/remember") async def remember( agent_id: str, @@ -299,6 +311,62 @@ async def batch_remember( raise map_error_to_http_exception(e) +@router.patch("/{agent_id}/memories/{memory_id}") +async def edit_memory( + agent_id: str, + memory_id: str, + request: MemoryEditRequest = Body(...), + session: Session = Depends(get_current_session), + client=Depends(get_moorcheh_client), +): + """ + Update one memory in the active agent's namespace (Session-based). + + Requires: + - X-Session-Token: {session_token} + + The session must be for the specified agent_id. + """ + if session.agent_id != agent_id: + raise map_error_to_http_exception( + Exception( + f"Session is for agent '{session.agent_id}', cannot access '{agent_id}'" + ) + ) + + updates = request.to_updates() + if not updates: + raise HTTPException( + status_code=400, + detail="Provide at least one field to update.", + ) + + if content := updates.get("content"): + CostGuard.validate_text_length(str(content), "Memory content") + + try: + write_service = MemoryWriteService(client) + result = await asyncio.to_thread( + write_service.update_memory, memory_id, session.namespace, updates + ) + return { + "agent_id": agent_id, + "session_id": session.session_id, + "namespace": session.namespace, + "memory_id": memory_id, + "status": result.get("status", "updated"), + "action": result.get("action", "updated"), + "updated_fields": result.get("updated_fields", list(updates.keys())), + } + + except Exception as e: + if "not found" in str(e).lower(): + raise HTTPException( + status_code=404, detail=f"Memory '{memory_id}' was not found." + ) + raise map_error_to_http_exception(e) + + @router.post("/{agent_id}/upload-file") async def upload_file( agent_id: str, diff --git a/memanto/cli/client/direct_client.py b/memanto/cli/client/direct_client.py index d784edd6..e7fa869e 100644 --- a/memanto/cli/client/direct_client.py +++ b/memanto/cli/client/direct_client.py @@ -729,6 +729,40 @@ def batch_remember( return result + def update_memory( + self, agent_id: str, memory_id: str, updates: dict[str, Any] + ) -> dict[str, Any]: + """ + Update a single memory in the active agent namespace. + + Args: + agent_id: Target agent. + memory_id: Memory document ID to update. + updates: Fields to update. + + Returns: + Dict with update result metadata. + + Raises: + ValueError: If no update fields are provided. + """ + session = self._get_validated_session_for_agent(agent_id) + if not updates: + raise ValueError("Provide at least one field to update") + + result = self._get_write_service().update_memory( + memory_id, session.namespace, updates + ) + + return { + "agent_id": agent_id, + "namespace": session.namespace, + "memory_id": memory_id, + "status": result.get("status", "updated"), + "action": result.get("action", "updated"), + "updated_fields": result.get("updated_fields", list(updates.keys())), + } + def recall( self, agent_id: str, diff --git a/memanto/cli/client/sdk_client.py b/memanto/cli/client/sdk_client.py index c47d019c..d29e51d4 100644 --- a/memanto/cli/client/sdk_client.py +++ b/memanto/cli/client/sdk_client.py @@ -566,6 +566,40 @@ def batch_remember( return result + def update_memory( + self, agent_id: str, memory_id: str, updates: dict[str, Any] + ) -> dict[str, Any]: + """ + Update a single memory in the active agent namespace. + + Args: + agent_id: Target agent. + memory_id: Memory document ID to update. + updates: Fields to update. + + Returns: + Dict with update result metadata. + + Raises: + ValueError: If no update fields are provided. + """ + session = self._get_validated_session_for_agent(agent_id) + if not updates: + raise ValueError("Provide at least one field to update") + + result = self._get_write_service().update_memory( + memory_id, session.namespace, updates + ) + + return { + "agent_id": agent_id, + "namespace": session.namespace, + "memory_id": memory_id, + "status": result.get("status", "updated"), + "action": result.get("action", "updated"), + "updated_fields": result.get("updated_fields", list(updates.keys())), + } + def upload_file(self, agent_id: str, file_path: str) -> dict[str, Any]: """ Upload a file directly to the agent's memory namespace. diff --git a/memanto/cli/commands/memory.py b/memanto/cli/commands/memory.py index 2b542d9b..fcd84bd5 100644 --- a/memanto/cli/commands/memory.py +++ b/memanto/cli/commands/memory.py @@ -172,6 +172,70 @@ def remember( _error(f"Failed to store memory: {e}") +@app.command() +def edit( + memory_id: str = typer.Argument(..., help="Memory ID to update"), + title: str | None = typer.Option(None, "--title", help="New memory title"), + content: str | None = typer.Option(None, "--content", help="New memory content"), + memory_type: str | None = typer.Option( + None, "--type", "-t", help="New memory type" + ), + confidence: float | None = typer.Option( + None, "--confidence", "-c", help="New confidence score (0.0-1.0)" + ), + tags: str | None = typer.Option(None, "--tags", help="New comma-separated tags"), + source: str | None = typer.Option(None, "--source", "-s", help="New memory source"), +): + """Update fields on an existing memory for the active agent.""" + start = time.perf_counter() + active_agent_id, active_session_token = config_manager.get_active_session() + + if not active_agent_id or not active_session_token: + _error( + "No active agent.", hint="Run 'memanto agent activate ' first." + ) + + updates: dict[str, object] = {} + if title is not None: + updates["title"] = title + if content is not None: + updates["content"] = content + if memory_type is not None: + updates["type"] = memory_type + if confidence is not None: + updates["confidence"] = confidence + if tags is not None: + updates["tags"] = [t.strip() for t in tags.split(",") if t.strip()] + if source is not None: + updates["source"] = source + + if not updates: + _error( + "No update fields provided.", + hint="Pass at least one of: --title, --content, --type, --confidence, --tags, --source.", + ) + + client = get_client() + + try: + with console.status("[cyan]Updating memory...", spinner="dots"): + result = client.update_memory( + agent_id=active_agent_id, + memory_id=memory_id, + updates=updates, + ) + elapsed = time.perf_counter() - start + + updated_fields = ", ".join(result.get("updated_fields", updates.keys())) + console.print("[green]Memory updated successfully![/green]") + console.print(f"[dim]Memory ID: {result.get('memory_id', memory_id)}[/dim]") + console.print(f"[dim]Updated fields: {updated_fields}[/dim]") + console.print(f"[dim]Completed in {elapsed:.2f}s[/dim]") + + except Exception as e: + _error(f"Failed to update memory: {e}") + + @app.command() def upload( file_path: str = typer.Argument(..., help="Path to the file to upload"), diff --git a/tests/test_api.py b/tests/test_api.py index 566de350..40db9e53 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1,6 +1,7 @@ import os import shutil import tempfile +from datetime import datetime, timedelta from pathlib import Path from unittest.mock import AsyncMock, MagicMock, patch @@ -9,6 +10,8 @@ from memanto.app.config import settings from memanto.app.main import app +from memanto.app.models.session import Session +from memanto.app.routes.auth_deps import get_current_session # Set test environment os.environ["MOORCHEH_API_KEY"] = "test-api-key" @@ -246,6 +249,67 @@ async def test_remember_with_session(self, client, auth_headers, mock_moorcheh): assert response.status_code == 200 assert response.json()["status"] == "queued" + @pytest.mark.asyncio + async def test_edit_memory_with_session(self, client, auth_headers): + """Test updating one memory with session token.""" + app.dependency_overrides[get_current_session] = lambda: Session( + session_id="sess-test", + session_token="token-test", + agent_id=self.TEST_AGENT_ID, + namespace=f"memanto_agent_{self.TEST_AGENT_ID}", + started_at=datetime.utcnow(), + expires_at=datetime.utcnow() + timedelta(hours=1), + ) + try: + with patch("memanto.app.routes.memory.MemoryWriteService") as mock_cls: + write_service = mock_cls.return_value + write_service.update_memory.return_value = { + "status": "success", + "action": "updated", + "updated_fields": ["title", "content"], + } + + response = await client.patch( + f"/api/v2/agents/{self.TEST_AGENT_ID}/memories/mem-123", + headers=auth_headers, + json={"title": "New title", "content": "New content"}, + ) + finally: + app.dependency_overrides.clear() + + assert response.status_code == 200 + data = response.json() + assert data["action"] == "updated" + assert data["updated_fields"] == ["title", "content"] + write_service.update_memory.assert_called_once_with( + "mem-123", + f"memanto_agent_{self.TEST_AGENT_ID}", + {"title": "New title", "content": "New content"}, + ) + + @pytest.mark.asyncio + async def test_edit_memory_rejects_empty_update(self, client, auth_headers): + """Test update endpoint requires at least one field.""" + app.dependency_overrides[get_current_session] = lambda: Session( + session_id="sess-test", + session_token="token-test", + agent_id=self.TEST_AGENT_ID, + namespace=f"memanto_agent_{self.TEST_AGENT_ID}", + started_at=datetime.utcnow(), + expires_at=datetime.utcnow() + timedelta(hours=1), + ) + try: + response = await client.patch( + f"/api/v2/agents/{self.TEST_AGENT_ID}/memories/mem-123", + headers=auth_headers, + json={}, + ) + finally: + app.dependency_overrides.clear() + + assert response.status_code == 400 + assert "at least one field" in response.json()["detail"] + @pytest.mark.asyncio async def test_answer_with_session(self, client, auth_headers, mock_moorcheh): """Test RAG answer with session token""" diff --git a/tests/test_cli.py b/tests/test_cli.py index 37bd5922..e1f8cfed 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -209,6 +209,42 @@ def test_remember(self, mock_all_clients): assert "stored successfully" in result.stdout.lower() assert "mem-123" in result.stdout + def test_edit(self, mock_all_clients): + """Test 'memanto edit' updates selected memory fields.""" + mock_all_clients.update_memory.return_value = { + "memory_id": "mem-123", + "status": "success", + "updated_fields": ["title", "tags"], + } + + result = runner.invoke( + app, + [ + "edit", + "mem-123", + "--title", + "Updated title", + "--tags", + "alpha,beta", + ], + ) + + assert result.exit_code == 0 + assert "updated successfully" in result.stdout.lower() + mock_all_clients.update_memory.assert_called_once_with( + agent_id="test-agent", + memory_id="mem-123", + updates={"title": "Updated title", "tags": ["alpha", "beta"]}, + ) + + def test_edit_requires_field(self, mock_all_clients): + """Test 'memanto edit' rejects an empty update.""" + result = runner.invoke(app, ["edit", "mem-123"]) + + assert result.exit_code != 0 + assert "No update fields provided" in result.stdout + mock_all_clients.update_memory.assert_not_called() + def test_recall(self, mock_all_clients): """Test 'memanto recall'""" mock_all_clients.recall.return_value = { From b5467619dd9d578f42c39a2d1143b065196734b1 Mon Sep 17 00:00:00 2001 From: jdjioe5-cpu Date: Sun, 14 Jun 2026 21:59:06 +0800 Subject: [PATCH 2/4] fix(memory): validate edit fields on API + direct-client paths Address the two actionable CodeRabbit review items flagged on PR #736 (which tests this PR): 1. routes/memory.py edit endpoint previously used 'if content := updates.get("content")' which silently allowed blank/whitespace-only content. Replaced with explicit non-empty + length checks. Added parallel range checks for confidence (0.0-1.0) and membership check for memory_type, matching the API contract used for the create endpoint. 2. cli/client/direct_client.py update_memory previously only checked that 'updates' was non-empty, allowing invalid edit payloads (out-of-range confidence, invalid type, blank content, unknown field names) to flow into the write service. Added the same field validation on the direct-client path so the CLI behavior matches the REST API contract. Both fixes align the edit path with the existing create path's validation strategy and keep the test coverage in PR #736 honest. Refs PR #736 (test follow-up) Refs Issue #540 (good-first-issue) Refs BountyHub bounty #508 (Memanto + mattpocock skills challenge) --- memanto/app/routes/memory.py | 31 +++++++++++++++++++++- memanto/cli/client/direct_client.py | 41 +++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/memanto/app/routes/memory.py b/memanto/app/routes/memory.py index e4b0d705..0be6ca1b 100644 --- a/memanto/app/routes/memory.py +++ b/memanto/app/routes/memory.py @@ -17,6 +17,7 @@ from memanto.app.clients.backend import get_active_llm_model from memanto.app.clients.moorcheh import get_moorcheh_client from memanto.app.config import settings +from memanto.app.constants import VALID_MEMORY_TYPES from memanto.app.core import MemoryRecord from memanto.app.models import ( AnswerRequest, @@ -341,8 +342,36 @@ async def edit_memory( detail="Provide at least one field to update.", ) - if content := updates.get("content"): + if "content" in updates: + content = updates["content"] + if content is None or not str(content).strip(): + raise HTTPException( + status_code=400, + detail="Memory content must be a non-empty string.", + ) CostGuard.validate_text_length(str(content), "Memory content") + if "confidence" in updates: + confidence = updates["confidence"] + try: + confidence_value = float(confidence) # type: ignore[arg-type] + except (TypeError, ValueError): + raise HTTPException( + status_code=400, + detail=f"Confidence must be a number between 0.0 and 1.0, got {confidence!r}.", + ) + if not 0.0 <= confidence_value <= 1.0: + raise HTTPException( + status_code=400, + detail=f"Confidence must be between 0.0 and 1.0, got {confidence_value}.", + ) + if "type" in updates and updates["type"] not in VALID_MEMORY_TYPES: + raise HTTPException( + status_code=400, + detail=( + f"Invalid memory_type '{updates['type']}'. " + f"Must be one of: {', '.join(sorted(VALID_MEMORY_TYPES))}." + ), + ) try: write_service = MemoryWriteService(client) diff --git a/memanto/cli/client/direct_client.py b/memanto/cli/client/direct_client.py index e7fa869e..cb865a77 100644 --- a/memanto/cli/client/direct_client.py +++ b/memanto/cli/client/direct_client.py @@ -749,6 +749,47 @@ def update_memory( session = self._get_validated_session_for_agent(agent_id) if not updates: raise ValueError("Provide at least one field to update") + _ALLOWED_UPDATE_FIELDS = { + "title", "content", "type", "confidence", "tags", "source", + } + unknown_fields = set(updates) - _ALLOWED_UPDATE_FIELDS + if unknown_fields: + raise ValueError( + f"Unknown update fields: {', '.join(sorted(unknown_fields))}. " + f"Allowed fields: {', '.join(sorted(_ALLOWED_UPDATE_FIELDS))}." + ) + if "content" in updates: + content = updates["content"] + if content is None or not str(content).strip(): + raise ValueError("Memory content must be a non-empty string") + if len(str(content)) > _MAX_CONTENT_LENGTH: + raise ValueError( + f"Memory content exceeds {_MAX_CONTENT_LENGTH} characters" + ) + if "title" in updates: + title = updates["title"] + if title is not None and len(str(title)) > _MAX_TITLE_LENGTH: + raise ValueError( + f"Memory title exceeds {_MAX_TITLE_LENGTH} characters" + ) + if "type" in updates: + memory_type = updates["type"] + if memory_type not in _VALID_MEMORY_TYPES: + raise ValueError( + f"Invalid memory_type '{memory_type}'. " + f"Must be one of: {', '.join(sorted(_VALID_MEMORY_TYPES))}" + ) + if "confidence" in updates: + try: + confidence_value = float(updates["confidence"]) # type: ignore[arg-type] + except (TypeError, ValueError): + raise ValueError( + f"Confidence must be a number between 0.0 and 1.0, got {updates['confidence']!r}" + ) + if not 0.0 <= confidence_value <= 1.0: + raise ValueError( + f"Confidence must be between 0.0 and 1.0, got {confidence_value}" + ) result = self._get_write_service().update_memory( memory_id, session.namespace, updates From 004ba833c622cfcf426f7796b53d305bf04772fe Mon Sep 17 00:00:00 2001 From: jdjioe5-cpu Date: Sun, 14 Jun 2026 21:29:09 +0800 Subject: [PATCH 3/4] test: add edit failure-path and cross-agent PATCH rejection tests (Refs PR #633, BountyHub #508) This follow-up adds the parallel coverage to PR #633 that PR #733 already added for PR #632: - test_edit_memory_returns_404_when_missing: PATCH /memories/{id} returns 404 when the underlying update_memory raises a 'not found' error - test_edit_memory_rejected_for_cross_agent: PATCH is rejected (403) when session.agent_id != URL agent_id; the write service is never invoked - test_edit_nonexistent_memory: `memanto edit ` surfaces a non-zero exit + the missing memory id when update_memory raises Same pattern as PR #733; both can be merged independently of PR #633. --- tests/test_api.py | 70 +++++++++++++++++++++++++++++++++++++++++++++++ tests/test_cli.py | 20 ++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/tests/test_api.py b/tests/test_api.py index 40db9e53..721ae6d4 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -8,6 +8,7 @@ import pytest from httpx import ASGITransport, AsyncClient +from memanto.app.clients.moorcheh import get_moorcheh_client from memanto.app.config import settings from memanto.app.main import app from memanto.app.models.session import Session @@ -310,6 +311,75 @@ async def test_edit_memory_rejects_empty_update(self, client, auth_headers): assert response.status_code == 400 assert "at least one field" in response.json()["detail"] + @pytest.mark.asyncio + async def test_edit_memory_returns_404_when_missing( + self, client, auth_headers, mock_moorcheh + ): + """Test PATCH /memories/{id} returns 404 when the underlying update + raises a 'not found' error (the canonical failure-path for #540).""" + app.dependency_overrides[get_current_session] = lambda: Session( + session_id="sess-test", + session_token="token-test", + agent_id=self.TEST_AGENT_ID, + namespace=f"memanto_agent_{self.TEST_AGENT_ID}", + started_at=datetime.utcnow(), + expires_at=datetime.utcnow() + timedelta(hours=1), + ) + app.dependency_overrides[get_moorcheh_client] = lambda: mock_moorcheh + try: + with patch( + "memanto.app.routes.memory.MemoryWriteService" + ) as mock_cls: + write_service = mock_cls.return_value + write_service.update_memory.side_effect = Exception( + "Memory 'missing-memory' not found in namespace" + ) + + response = await client.patch( + f"/api/v2/agents/{self.TEST_AGENT_ID}/memories/missing-memory", + headers=auth_headers, + json={"title": "New title"}, + ) + finally: + app.dependency_overrides.clear() + + assert response.status_code == 404 + assert "missing-memory" in response.json()["detail"] + + @pytest.mark.asyncio + async def test_edit_memory_rejected_for_cross_agent( + self, client, auth_headers, mock_moorcheh + ): + """Test PATCH /memories/{id} is rejected when session.agent_id != URL + agent_id, and the write service is never invoked.""" + other_agent = "different-agent" + + app.dependency_overrides[get_current_session] = lambda: Session( + session_id="sess-test", + session_token="token-test", + agent_id=other_agent, + namespace=f"memanto_agent_{other_agent}", + started_at=datetime.utcnow(), + expires_at=datetime.utcnow() + timedelta(hours=1), + ) + app.dependency_overrides[get_moorcheh_client] = lambda: mock_moorcheh + try: + with patch( + "memanto.app.routes.memory.MemoryWriteService" + ) as mock_cls: + write_service = mock_cls.return_value + + response = await client.patch( + f"/api/v2/agents/{self.TEST_AGENT_ID}/memories/mem-123", + headers=auth_headers, + json={"title": "New title"}, + ) + finally: + app.dependency_overrides.clear() + + assert response.status_code == 403 + write_service.update_memory.assert_not_called() + @pytest.mark.asyncio async def test_answer_with_session(self, client, auth_headers, mock_moorcheh): """Test RAG answer with session token""" diff --git a/tests/test_cli.py b/tests/test_cli.py index e1f8cfed..447f8a03 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -245,6 +245,26 @@ def test_edit_requires_field(self, mock_all_clients): assert "No update fields provided" in result.stdout mock_all_clients.update_memory.assert_not_called() + def test_edit_nonexistent_memory(self, mock_all_clients): + """Test 'memanto edit' surfaces a non-zero exit when the underlying + update_memory raises (e.g. memory not found).""" + mock_all_clients.update_memory.side_effect = ValueError( + "Memory 'missing-memory' not found in namespace" + ) + + result = runner.invoke( + app, + ["edit", "missing-memory", "--title", "Updated title"], + ) + + assert result.exit_code != 0 + assert "missing-memory" in result.stdout + mock_all_clients.update_memory.assert_called_once_with( + agent_id="test-agent", + memory_id="missing-memory", + updates={"title": "Updated title"}, + ) + def test_recall(self, mock_all_clients): """Test 'memanto recall'""" mock_all_clients.recall.return_value = { From c86eb8072b36e7847ef885b719bd23cdfffe63bb Mon Sep 17 00:00:00 2001 From: Hermes Bounty Ops Date: Mon, 15 Jun 2026 00:23:59 +0800 Subject: [PATCH 4/4] style: ruff format on PR #736 tests (post-rebase) --- tests/test_api.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index 721ae6d4..69f4cbe1 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -327,9 +327,7 @@ async def test_edit_memory_returns_404_when_missing( ) app.dependency_overrides[get_moorcheh_client] = lambda: mock_moorcheh try: - with patch( - "memanto.app.routes.memory.MemoryWriteService" - ) as mock_cls: + with patch("memanto.app.routes.memory.MemoryWriteService") as mock_cls: write_service = mock_cls.return_value write_service.update_memory.side_effect = Exception( "Memory 'missing-memory' not found in namespace" @@ -364,9 +362,7 @@ async def test_edit_memory_rejected_for_cross_agent( ) app.dependency_overrides[get_moorcheh_client] = lambda: mock_moorcheh try: - with patch( - "memanto.app.routes.memory.MemoryWriteService" - ) as mock_cls: + with patch("memanto.app.routes.memory.MemoryWriteService") as mock_cls: write_service = mock_cls.return_value response = await client.patch(