fix: prevent stale Running status after job completion

agent/apiharness/src/main/java/com/intuit/tank/harness/APIMonitor.java

@@ -18,6 +18,7 @@
 import java.net.http.HttpClient;
 import java.net.http.HttpRequest;
 import java.net.http.HttpResponse;
+import java.time.Duration;
 import java.util.Date;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
@@ -144,43 +145,98 @@ public static void setDoMonitor(boolean monitor) {
         doMonitor = monitor;
     }
 
-    public synchronized static void setJobStatus(JobStatus jobStatus) {
+    public static void setJobStatus(JobStatus jobStatus) {
         LOG.debug(LogUtil.getLogMessage("Setting job status to: " + jobStatus));
-        if (status != null && status.getJobStatus() != JobStatus.Completed) {
+        // Capture immutable status snapshot inside lock, send outside to avoid
+        // blocking the synchronized section on network I/O (terminal retry can take ~6s)
+        String instanceId;
+        CloudVmStatus statusToSend;
+        synchronized (APIMonitor.class) {
+            if (status == null || status.getJobStatus() == JobStatus.Completed) {
+                return;
+            }
             try {
-            	VMStatus vmStatus =  jobStatus.equals(JobStatus.Stopped) ? VMStatus.stopping
+                VMStatus vmStatus = jobStatus.equals(JobStatus.Stopped) ? VMStatus.stopping
                         : jobStatus.equals(JobStatus.RampPaused) ? VMStatus.rampPaused
                         : jobStatus.equals(JobStatus.Running) ? VMStatus.running
                         : jobStatus.equals(JobStatus.Completed) ? VMStatus.terminated : status.getVmStatus();
                 WatsAgentStatusResponse stats = APITestHarness.getInstance().getStatus();
-                Date endTime = (jobStatus == JobStatus.Completed) ? new Date() : status
-                        .getEndTime();
-                status = new CloudVmStatus(status.getInstanceId(), status.getJobId(), status.getSecurityGroup(),
+                Date endTime = (jobStatus == JobStatus.Completed) ? new Date() : status.getEndTime();
+                // Build into local — only assign to static 'status' after full success
+                CloudVmStatus newStatus = new CloudVmStatus(status.getInstanceId(), status.getJobId(), status.getSecurityGroup(),
                         jobStatus, status.getRole(), status.getVmRegion(), vmStatus,
                         new ValidationStatus(stats.getKills(), stats.getAborts(),
                                 stats.getGotos(), stats.getSkips(), stats.getSkipGroups(), stats.getRestarts()),
                         stats.getMaxVirtualUsers(),
                         stats.getCurrentNumberUsers(), status.getStartTime(), endTime);
-                status.setUserDetails(APITestHarness.getInstance().getUserTracker().getSnapshot());
-                setInstanceStatus(status.getInstanceId(), status);
+                newStatus.setUserDetails(APITestHarness.getInstance().getUserTracker().getSnapshot());
+                status = newStatus;
+                instanceId = status.getInstanceId();
+                statusToSend = status;
             } catch (Exception e) {
-                LOG.error("Error sending status to controller: {}", e.toString(), e);
+                LOG.error("Error building status snapshot in setJobStatus: {}", e.toString(), e);
+                return;
             }
         }
+        // Network send is outside the lock — terminal retry won't block other threads
+        try {
+            setInstanceStatus(instanceId, statusToSend);
+        } catch (Exception e) {
+            LOG.error("Error sending status to controller: {}", e.toString(), e);
+        }
     }
 
-    protected static void setInstanceStatus(String instanceId, CloudVmStatus VmStatus) throws URISyntaxException, JsonProcessingException {
-        String json = objectWriter.writeValueAsString(VmStatus);
+    protected static void setInstanceStatus(String instanceId, CloudVmStatus vmStatus) throws URISyntaxException, JsonProcessingException {
+        // Force currentUsers=0 on terminal statuses — agent threads may not have fully exited
+        boolean isTerminal = vmStatus.getJobStatus() == JobStatus.Completed
+            || vmStatus.getJobStatus() == JobStatus.Stopped
+            || vmStatus.getVmStatus() == VMStatus.terminated;
+        if (isTerminal) {
+            vmStatus.setCurrentUsers(0);
+        }
+
+        String json = objectWriter.writeValueAsString(vmStatus);
         String token = APITestHarness.getInstance().getTankConfig().getAgentConfig().getAgentToken();
         HttpRequest request = HttpRequest.newBuilder()
                 .uri(new URI(APITestHarness.getInstance().getTankConfig().getControllerBase() + "/v2/agent/instance/status/" + instanceId))
                 .header(HttpHeaders.ACCEPT, ContentType.APPLICATION_JSON.getMimeType())
                 .header(HttpHeaders.CONTENT_TYPE, ContentType.APPLICATION_JSON.getMimeType())
                 .header(HttpHeaders.AUTHORIZATION, "bearer " + token)
+                .timeout(Duration.ofSeconds(10))
                 .PUT(HttpRequest.BodyPublishers.ofString(json))
                 .build();
 
         LOG.debug(LogUtil.getLogMessage("Sending instance status update for instance: " + instanceId + ", Status: " + json));
-        client.sendAsync(request, HttpResponse.BodyHandlers.discarding());
+
+        if (isTerminal) {
+            // Terminal statuses use synchronous send with retry — losing the final Completed
+            // report causes the controller to show stale "Running" status indefinitely
+            for (int attempt = 1; attempt <= 3; attempt++) {
+                try {
+                    HttpResponse<Void> response = client.send(request, HttpResponse.BodyHandlers.discarding());
+                    if (response.statusCode() == 200 || response.statusCode() == 202 || response.statusCode() == 204) {
+                        LOG.info(LogUtil.getLogMessage("Terminal status delivered on attempt " + attempt));
+                        return;
+                    }
+                    LOG.warn(LogUtil.getLogMessage("Terminal status delivery got " + response.statusCode() + " on attempt " + attempt));
+                } catch (InterruptedException ie) {
+                    Thread.currentThread().interrupt();
+                    LOG.warn(LogUtil.getLogMessage("Terminal status delivery interrupted on attempt " + attempt));
+                    return;
+                } catch (Exception e) {
+                    LOG.warn(LogUtil.getLogMessage("Terminal status delivery failed on attempt " + attempt + ": " + e.getMessage()));
+                }
+                if (attempt < 3) {
+                    try { Thread.sleep(2000); } catch (InterruptedException ignored) {
+                        Thread.currentThread().interrupt();
+                        return;
+                    }
+                }
+            }
+            LOG.error(LogUtil.getLogMessage("Failed to deliver terminal status after 3 attempts for instance " + instanceId));
+        } else {
+            // Non-terminal statuses remain async (fire-and-forget is fine for periodic updates)
+            client.sendAsync(request, HttpResponse.BodyHandlers.discarding());
+        }
     }
 }

rest-mvc/impl/src/main/java/com/intuit/tank/rest/mvc/rest/cloud/JobEventSender.java

@@ -274,6 +274,10 @@ public CloudVmStatus getVmStatus(String instanceId) {
     }
 
     public void setVmStatus(final String instanceId, final CloudVmStatus status) {
+        // Normalize: completed/terminated agents always have zero users
+        if (status.getJobStatus() == JobStatus.Completed || status.getVmStatus() == VMStatus.terminated) {
+            status.setCurrentUsers(0);
+        }
         vmTracker.setStatus(status);
         if (status.getJobStatus() == JobStatus.Completed
                 || status.getVmStatus() == VMStatus.terminated

rest-mvc/impl/src/test/java/com/intuit/tank/rest/mvc/rest/cloud/JobEventSenderTest.java

@@ -1,5 +1,6 @@
 package com.intuit.tank.rest.mvc.rest.cloud;
 
+import com.intuit.tank.vm.vmManager.VMTerminator;
 import com.intuit.tank.vm.vmManager.VMTracker;
 import com.intuit.tank.vm.vmManager.models.CloudVmStatus;
 import com.intuit.tank.vm.vmManager.models.CloudVmStatusContainer;
@@ -32,6 +33,9 @@ public class JobEventSenderTest {
     @Mock
     private VMTracker vmTracker;
 
+    @Mock
+    private VMTerminator terminator;
+
     @InjectMocks
     private JobEventSender jobEventSender;
 
@@ -263,6 +267,61 @@ void getInstancesForJob_excludesReplacedAndTerminated() throws Exception {
         assertFalse(instances.contains("i-replaced"));
     }
 
+    // ============ setVmStatus normalization tests (Fix 2) ============
+
+    @Test
+    @DisplayName("setVmStatus normalizes currentUsers to 0 for Completed agent before passing to vmTracker")
+    void setVmStatus_completedAgent_normalizesCurrentUsersToZero() {
+        // Given: An agent reports Completed but still has currentUsers=10
+        CloudVmStatus status = createStatus("i-agent1", JobStatus.Completed, VMStatus.running);
+        // Manually set currentUsers to simulate stale count
+        status.setCurrentUsers(10);
+
+        // When
+        jobEventSender.setVmStatus("i-agent1", status);
+
+        // Then: The status passed to vmTracker.setStatus should have currentUsers=0
+        assertEquals(0, status.getCurrentUsers(),
+            "Completed agent should have currentUsers normalized to 0 before reaching vmTracker");
+        verify(vmTracker).setStatus(status);
+        verify(terminator).terminate("i-agent1");
+    }
+
+    @Test
+    @DisplayName("setVmStatus normalizes currentUsers to 0 for terminated agent")
+    void setVmStatus_terminatedAgent_normalizesCurrentUsersToZero() {
+        // Given: An agent reports terminated but still has currentUsers=5
+        CloudVmStatus status = createStatus("i-agent1", JobStatus.Completed, VMStatus.terminated);
+        status.setCurrentUsers(5);
+
+        // When
+        jobEventSender.setVmStatus("i-agent1", status);
+
+        // Then
+        assertEquals(0, status.getCurrentUsers(),
+            "Terminated agent should have currentUsers normalized to 0");
+        verify(vmTracker).setStatus(status);
+        verify(terminator).terminate("i-agent1");
+    }
+
+    @Test
+    @DisplayName("setVmStatus does NOT normalize currentUsers for Running agent")
+    void setVmStatus_runningAgent_preservesCurrentUsers() {
+        // Given: A Running agent with 50 currentUsers
+        CloudVmStatus status = createStatus("i-agent1", JobStatus.Running, VMStatus.running);
+        status.setCurrentUsers(50);
+
+        // When
+        jobEventSender.setVmStatus("i-agent1", status);
+
+        // Then: currentUsers should remain 50
+        assertEquals(50, status.getCurrentUsers(),
+            "Running agent should keep its currentUsers unchanged");
+        verify(vmTracker).setStatus(status);
+    }
+
+    // ============ getVmStatus delegation tests ============
+
     @Test
     @DisplayName("getVmStatus delegates to vmTracker")
     void getVmStatus_delegatesToVmTracker() {

tank_vmManager/pom.xml

@@ -75,5 +75,11 @@
       <artifactId>serializer</artifactId>
     </dependency>
 
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-junit-jupiter</artifactId>
+      <scope>test</scope>
+    </dependency>
+
   </dependencies>
 </project>