diff --git a/hindsight-api-slim/hindsight_api/api/http.py b/hindsight-api-slim/hindsight_api/api/http.py index 08896f355d..e2f05facbc 100644 --- a/hindsight-api-slim/hindsight_api/api/http.py +++ b/hindsight-api-slim/hindsight_api/api/http.py @@ -1371,6 +1371,24 @@ class DeleteResponse(BaseModel): deleted_count: int | None = None +class DeleteMemoryUnitResponse(BaseModel): + """Response model for deleting a single memory unit.""" + + model_config = ConfigDict( + json_schema_extra={ + "example": { + "success": True, + "memory_id": "abc123", + "message": "Memory unit and all its links deleted successfully", + } + } + ) + + success: bool + memory_id: str | None = None + message: str | None = None + + class ClearMemoryObservationsResponse(BaseModel): """Response model for clearing observations for a specific memory.""" @@ -2412,6 +2430,55 @@ async def api_get_memory( logger.error(f"Error in /v1/default/banks/{bank_id}/memories/{memory_id}: {error_detail}") raise HTTPException(status_code=500, detail=str(e)) + @app.delete( + "/v1/default/banks/{bank_id}/memories/{memory_id}", + response_model=DeleteMemoryUnitResponse, + summary="Delete memory unit", + description="Delete a single memory unit and all its associated links, entity associations, " + "and derived observations. Triggers re-consolidation for any affected observations.", + operation_id="delete_memory", + tags=["Memory"], + ) + @audited("delete_memory", request_param=None) + async def api_delete_memory( + bank_id: str, + memory_id: str, + request_context: RequestContext = Depends(get_request_context), + ): + """Delete a single memory unit by ID.""" + try: + import uuid as _uuid + + try: + _uuid.UUID(memory_id) + except ValueError: + raise HTTPException( + status_code=400, detail=f"Invalid memory_id format: '{memory_id}' is not a valid UUID" + ) + + result = await app.state.memory.delete_memory_unit( + unit_id=memory_id, + bank_id=bank_id, + request_context=request_context, + ) + if not result.get("success"): + raise HTTPException(status_code=404, detail=f"Memory unit '{memory_id}' not found in bank '{bank_id}'") + return DeleteMemoryUnitResponse( + success=result["success"], + memory_id=result.get("memory_id"), + message=result.get("message"), + ) + except OperationValidationError as e: + raise HTTPException(status_code=e.status_code, detail=e.reason) + except (AuthenticationError, HTTPException): + raise + except Exception as e: + import traceback + + error_detail = f"{str(e)}\n\nTraceback:\n{traceback.format_exc()}" + logger.error(f"Error in DELETE /v1/default/banks/{bank_id}/memories/{memory_id}: {error_detail}") + raise HTTPException(status_code=500, detail=str(e)) + @app.get( "/v1/default/banks/{bank_id}/memories/{memory_id}/history", summary="Get observation history", diff --git a/hindsight-api-slim/hindsight_api/engine/interface.py b/hindsight-api-slim/hindsight_api/engine/interface.py index 296dfde287..b2b934f9ac 100644 --- a/hindsight-api-slim/hindsight_api/engine/interface.py +++ b/hindsight-api-slim/hindsight_api/engine/interface.py @@ -291,6 +291,7 @@ async def delete_memory_unit( self, unit_id: str, *, + bank_id: str, request_context: "RequestContext", ) -> dict[str, Any]: """ @@ -298,6 +299,7 @@ async def delete_memory_unit( Args: unit_id: The memory unit ID. + bank_id: The bank that owns the memory unit (enforced in query). request_context: Request context for authentication. Returns: diff --git a/hindsight-api-slim/hindsight_api/engine/memory_engine.py b/hindsight-api-slim/hindsight_api/engine/memory_engine.py index db1aacbe73..33b93495b9 100644 --- a/hindsight-api-slim/hindsight_api/engine/memory_engine.py +++ b/hindsight-api-slim/hindsight_api/engine/memory_engine.py @@ -3730,6 +3730,7 @@ async def delete_memory_unit( self, unit_id: str, *, + bank_id: str, request_context: "RequestContext", ) -> dict[str, Any]: """ @@ -3745,39 +3746,47 @@ async def delete_memory_unit( Args: unit_id: UUID of the memory unit to delete + bank_id: The bank that owns the memory unit (enforced in query). request_context: Request context for authentication. Returns: Dictionary with deletion result """ await self._authenticate_tenant(request_context) + if self._operation_validator: + from hindsight_api.extensions import BankWriteContext + + ctx = BankWriteContext(bank_id=bank_id, operation="delete_memory_unit", request_context=request_context) + await self._validate_operation(self._operation_validator.validate_bank_write(ctx)) pool = await self._get_pool() invalidated_obs = 0 bank_id_for_consolidation: str | None = None async with acquire_with_retry(pool) as conn: async with conn.transaction(): - # Get bank_id and fact_type before deletion + # Get fact_type before deletion (scoped to bank) row = await conn.fetchrow( - f"SELECT bank_id, fact_type FROM {fq_table('memory_units')} WHERE id = $1", + f"SELECT fact_type FROM {fq_table('memory_units')} WHERE id = $1 AND bank_id = $2", unit_id, + bank_id, ) - bank_id = row["bank_id"] if row else None fact_type = row["fact_type"] if row else None # Invalidate observations before deletion (only for source memory types) - if bank_id and fact_type in ("experience", "world"): + if fact_type in ("experience", "world"): invalidated_obs = await self._delete_stale_observations_for_memories(conn, bank_id, [unit_id]) if invalidated_obs > 0: bank_id_for_consolidation = bank_id - # Delete the memory unit (cascades to links and associations) + # Delete the memory unit (scoped to bank, cascades to links and associations) deleted = await conn.fetchval( - f"DELETE FROM {fq_table('memory_units')} WHERE id = $1 RETURNING id", unit_id + f"DELETE FROM {fq_table('memory_units')} WHERE id = $1 AND bank_id = $2 RETURNING id", + unit_id, + bank_id, ) result = { "success": deleted is not None, - "unit_id": str(deleted) if deleted else None, + "memory_id": str(deleted) if deleted else None, "message": "Memory unit and all its links deleted successfully" if deleted else "Memory unit not found", diff --git a/hindsight-api-slim/hindsight_api/mcp_tools.py b/hindsight-api-slim/hindsight_api/mcp_tools.py index f73e5a5c5d..368596a152 100644 --- a/hindsight-api-slim/hindsight_api/mcp_tools.py +++ b/hindsight-api-slim/hindsight_api/mcp_tools.py @@ -2033,7 +2033,7 @@ async def delete_memory( Args: memory_id: The ID of the memory to delete - bank_id: Optional bank (accepted for consistency, not used in deletion). + bank_id: Optional bank to scope the deletion to. """ try: target_bank = bank_id or config.bank_id_resolver() @@ -2042,6 +2042,7 @@ async def delete_memory( result = await memory.delete_memory_unit( unit_id=memory_id, + bank_id=target_bank, request_context=_get_request_context(config), ) return json.dumps({"status": "deleted", "memory_id": memory_id, **result}, default=str) @@ -2073,6 +2074,7 @@ async def delete_memory( result = await memory.delete_memory_unit( unit_id=memory_id, + bank_id=target_bank, request_context=_get_request_context(config), ) return {"status": "deleted", "memory_id": memory_id, **result} diff --git a/hindsight-api-slim/tests/test_http_api_integration.py b/hindsight-api-slim/tests/test_http_api_integration.py index a2b2b54377..e8b5bb2b91 100644 --- a/hindsight-api-slim/tests/test_http_api_integration.py +++ b/hindsight-api-slim/tests/test_http_api_integration.py @@ -1229,3 +1229,86 @@ async def test_retain_with_timestamp_async_complete_processing(api_client, test_ assert response.status_code == 200 items = response.json()["items"] assert len(items) > 0, "Should have stored memories after async processing" + + +@pytest.mark.asyncio +async def test_delete_individual_memory_unit(api_client): + """ + Test DELETE /v1/default/banks/{bank_id}/memories/{memory_id}. + + Verifies: + 1. Deleting an existing memory returns 200 with success=True + 2. The memory is no longer listed after deletion + 3. Deleting a non-existent memory returns 404 + """ + test_bank_id = f"delete_unit_test_{datetime.now().timestamp()}" + + # 1. Create a memory + response = await api_client.post( + f"/v1/default/banks/{test_bank_id}/memories", + json={"items": [{"content": "This is a test memory that will be deleted.", "context": "delete test"}]}, + ) + assert response.status_code == 200 + + # 2. List memories to get the ID + response = await api_client.get(f"/v1/default/banks/{test_bank_id}/memories/list", params={"limit": 10}) + assert response.status_code == 200 + items = response.json()["items"] + assert len(items) > 0, "Should have at least one memory" + memory_id = items[0]["id"] + + # 3. Delete the memory + response = await api_client.delete(f"/v1/default/banks/{test_bank_id}/memories/{memory_id}") + assert response.status_code == 200 + result = response.json() + assert result["success"] is True + assert result["memory_id"] == memory_id + assert result["message"] is not None + + # 4. Verify the memory is gone + response = await api_client.get(f"/v1/default/banks/{test_bank_id}/memories/{memory_id}") + assert response.status_code == 404 + + # 5. Verify it's not in the list + response = await api_client.get(f"/v1/default/banks/{test_bank_id}/memories/list", params={"limit": 10}) + assert response.status_code == 200 + remaining_ids = [item["id"] for item in response.json()["items"]] + assert memory_id not in remaining_ids, "Deleted memory should not appear in list" + + # 6. Deleting again should return 404 + response = await api_client.delete(f"/v1/default/banks/{test_bank_id}/memories/{memory_id}") + assert response.status_code == 404 + + +@pytest.mark.asyncio +async def test_delete_memory_unit_cross_bank_isolation(api_client): + """Deleting a memory via a different bank's path should return 404 (not delete it).""" + bank_a = f"delete_bank_a_{datetime.now().timestamp()}" + bank_b = f"delete_bank_b_{datetime.now().timestamp()}" + + # Create a memory in bank A + response = await api_client.post( + f"/v1/default/banks/{bank_a}/memories", + json={"items": [{"content": "Memory in bank A.", "context": "isolation test"}]}, + ) + assert response.status_code == 200 + + # Get the memory ID from bank A + response = await api_client.get(f"/v1/default/banks/{bank_a}/memories/list", params={"limit": 10}) + assert response.status_code == 200 + memory_id = response.json()["items"][0]["id"] + + # Try to delete it via bank B's path — should fail with 404 + response = await api_client.delete(f"/v1/default/banks/{bank_b}/memories/{memory_id}") + assert response.status_code == 404, "Cross-bank deletion should be rejected" + + # Verify memory still exists in bank A + response = await api_client.get(f"/v1/default/banks/{bank_a}/memories/{memory_id}") + assert response.status_code == 200, "Memory should still exist in its original bank" + + +@pytest.mark.asyncio +async def test_delete_memory_unit_invalid_uuid(api_client): + """Deleting with a non-UUID memory_id should return 400, not 500.""" + response = await api_client.delete(f"/v1/default/banks/test/memories/not-a-uuid") + assert response.status_code == 400 diff --git a/hindsight-api-slim/tests/test_observation_invalidation.py b/hindsight-api-slim/tests/test_observation_invalidation.py index de21f4b238..4dcde81997 100644 --- a/hindsight-api-slim/tests/test_observation_invalidation.py +++ b/hindsight-api-slim/tests/test_observation_invalidation.py @@ -95,7 +95,7 @@ async def test_deleting_source_memory_removes_observation( m2 = await _insert_memory(conn, bank_id, "Alice goes hiking every weekend.") obs_id = await _insert_observation(conn, bank_id, "Alice enjoys hiking regularly.", [m1, m2]) - await memory.delete_memory_unit(str(m1), request_context=request_context) + await memory.delete_memory_unit(str(m1), bank_id=bank_id, request_context=request_context) async with pool.acquire() as conn: obs_ids = await _get_observation_ids(conn, bank_id) @@ -122,7 +122,7 @@ async def test_deleting_source_memory_resets_remaining_source_consolidated_at( # Patch out consolidation so it doesn't re-set consolidated_at before we can check it with patch.object(memory, "submit_async_consolidation", new=AsyncMock()): - await memory.delete_memory_unit(str(m1), request_context=request_context) + await memory.delete_memory_unit(str(m1), bank_id=bank_id, request_context=request_context) async with pool.acquire() as conn: # m2 should have consolidated_at reset to NULL @@ -146,7 +146,7 @@ async def test_deleting_non_source_memory_leaves_observations_intact( unrelated = await _insert_memory(conn, bank_id, "Bob likes cycling.") obs_id = await _insert_observation(conn, bank_id, "Alice enjoys hiking regularly.", [m1, m2]) - await memory.delete_memory_unit(str(unrelated), request_context=request_context) + await memory.delete_memory_unit(str(unrelated), bank_id=bank_id, request_context=request_context) async with pool.acquire() as conn: obs_ids = await _get_observation_ids(conn, bank_id) @@ -170,7 +170,7 @@ async def test_deleting_sole_source_memory_removes_observation_no_remaining_rese m1 = await _insert_memory(conn, bank_id, "Alice loves hiking.") obs_id = await _insert_observation(conn, bank_id, "Alice enjoys hiking.", [m1]) - await memory.delete_memory_unit(str(m1), request_context=request_context) + await memory.delete_memory_unit(str(m1), bank_id=bank_id, request_context=request_context) async with pool.acquire() as conn: obs_ids = await _get_observation_ids(conn, bank_id) @@ -192,7 +192,7 @@ async def test_deleting_observation_type_memory_does_not_trigger_invalidation( obs_id = await _insert_observation(conn, bank_id, "Alice enjoys hiking.", [m1]) # Delete the observation directly (not the source memory) - await memory.delete_memory_unit(str(obs_id), request_context=request_context) + await memory.delete_memory_unit(str(obs_id), bank_id=bank_id, request_context=request_context) async with pool.acquire() as conn: # Source memory should still be consolidated (not reset)