A trigger created by the previous admin is not working correctly - it works sometimes but not always, and I can't figure out any pattern.

The trigger is meant to allow salespeople to take over old contacts by adding a task to the contact record. If the contact hasn't had any activity in a year and the salesperson has the right user role, the contact is supposed to change to Active and ownership should change to the new salesperson.

I know almost nothing about Apex and am still learning Salesforce, so any help would be appreciated. This is the code:

    trigger Task_After_Update on Task (after update) {
//Update Contact Fields: Last Activity Trigger, Contact Owner
Set<Id> cIds = new Set<Id>();
 for(Task t: [SELECT Subject,WhoId,Status,OwnerId,Owner_role__c,Made_Contact__c FROM TASK WHERE ID IN:trigger.new]){
   String wId = t.WhoId;
   Task oldTask = Trigger.oldMap.get(t.ID);
   if( oldTask.Made_Contact__c !='Yes' && wId!=null && !cIds.contains(t.WhoId) && t.Status == 'Completed' && t.Made_Contact__c == 'Yes' && t.owner_role__c.Contains('Broker')){
   cIds.add(t.WhoId);
    for(Contact c:[SELECT ownerid, type__c, Last_Activity_Trigger__c  FROM Contact WHERE ( type__c = 'Active' AND ID IN:cIds) ]){
        if( c.ownerid == t.ownerid ){
                 c.Last_Activity_Trigger__c=System.now();
                 c.Last_Activity_Completed_By__c=t.ownerid;}
        if( c.ownerid != t.ownerid ){
                 c.Last_Activity_Completed_By__c=t.ownerid;}         
        update c;
    }
    for(Contact c:[SELECT ownerid, type__c, Last_Activity_Trigger__c  FROM Contact WHERE ( type__c != 'Active' AND ID IN:cIds) ]){
        if( c.ownerid == t.ownerid ){
                 c.Last_Activity_Trigger__c=System.now();
                 c.Last_Activity_Completed_By__c=t.ownerid;}
        if( c.ownerid != t.ownerid ){
                 c.Last_Activity_Trigger__c=System.now();
                 c.Last_Activity_Completed_By__c=t.ownerid;
                 c.ownerid=t.ownerid;}
        update c;
    }
   }
 }  
}